Re: [PATCH v6 net-next 2/7] net: ethernet: am65-cpsw: cleanup TAPRIO handling

[Roger Quadros]

On 21/11/2023 00:56, Vladimir Oltean wrote:
> On Mon, Nov 20, 2023 at 04:01:42PM +0200, Roger Quadros wrote:
>> -static int am65_cpsw_configure_taprio(struct net_device *ndev,
>> -				      struct am65_cpsw_est *est_new)
>> +static void am65_cpsw_cp_taprio(struct tc_taprio_qopt_offload *from,
>> +				struct tc_taprio_qopt_offload *to)
>> +{
>> +	int i;
>> +
>> +	*to = *from;
>> +	for (i = 0; i < from->num_entries; i++)
>> +		to->entries[i] = from->entries[i];
>> +}
> 
> I think I mentioned this before: have you looked at taprio_offload_get()
> and taprio_offload_put()?

I'm sorry that I missed this. I'll take a look.

> 
>> +
>> +static int am65_cpsw_taprio_replace(struct net_device *ndev,
>> +				    struct tc_taprio_qopt_offload *taprio)
>>  {
>>  	struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
>> +	struct netlink_ext_ack *extack = taprio->mqprio.extack;
>> +	struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
>>  	struct am65_cpts *cpts = common->cpts;
>>  	int ret = 0, tact = TACT_PROG;
>> +	struct am65_cpsw_est *est_new;
>>  
>> -	am65_cpsw_est_update_state(ndev);
>> +	if (!netif_running(ndev)) {
>> +		NL_SET_ERR_MSG_MOD(extack, "interface is down, link speed unknown\n");
> 
> The extack message doesn't need a \n.

OK.

> 
>> +		return -ENETDOWN;
>> +	}
>>  
>> -	if (est_new->taprio.cmd == TAPRIO_CMD_DESTROY) {
>> -		am65_cpsw_stop_est(ndev);
>> -		return ret;
>> +	if (common->pf_p0_rx_ptype_rrobin) {
>> +		NL_SET_ERR_MSG_MOD(extack,
>> +				   "p0-rx-ptype-rrobin flag conflicts with taprio qdisc\n");
> 
> Also here.
> 
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (port->qos.link_speed == SPEED_UNKNOWN)
>> +		return -ENOLINK;
>> +
>> +	if (taprio->cycle_time_extension) {
>> +		NL_SET_ERR_MSG_MOD(extack,
>> +				   "cycle time extension not supported");
> 
> Here it's ok.
> 
>> +		return -EOPNOTSUPP;
>>  	}

Thanks for the detailed review!

-- 
cheers,
-roger