Re: [PATCH net-next 5/6] net: macb: Implement TAPRIO TC offload command interface

[Claudiu Beznea <claudiu.beznea@tuxon.dev>]

On 29.07.2025 12:38, Karumanchi, Vineeth wrote:
> Hi Claudiu,
> 
> 
> On 7/26/2025 5:59 PM, claudiu beznea (tuxon) wrote:
> <...>
>>> +    int err = 0;
>>> +
>>> +    switch (taprio->cmd) {
>>> +    case TAPRIO_CMD_REPLACE:
>>> +        err = macb_taprio_setup_replace(ndev, taprio);
>>> +        break;
>>> +    case TAPRIO_CMD_DESTROY:
>>> +        macb_taprio_destroy(ndev);
>>
>> macb_taprio_setup_replace() along with macb_taprio_destroy() touch HW
>> registers. Could macb_setup_taprio() be called when the interface is
>> runtime suspended?
>>
>>
> 
> Nice catch!
> 
> I will leverage pm_runtime_suspended(&pdev->dev) check before configuring.
> 
>>> +        break;
>>> +    default:
>>> +        err = -EOPNOTSUPP;
>>> +    }
>>> +
>>> +    return err;
>>> +}
>>> +
>>> +static int macb_setup_tc(struct net_device *dev, enum tc_setup_type
>>> type, void *type_data)
>>> +{
>>> +    if (!dev || !type_data)
>>> +        return -EINVAL;
>>> +
>>> +    switch (type) {
>>> +    case TC_SETUP_QDISC_TAPRIO:
>>> +        return macb_setup_taprio(dev, type_data);
>>
>> Same here.
>>
>>> +    default:
>>> +        return -EOPNOTSUPP;
>>> +    }
>>> +}
>>> +
>>>   static const struct net_device_ops macb_netdev_ops = {
>>>       .ndo_open        = macb_open,
>>>       .ndo_stop        = macb_close,
>>> @@ -4284,6 +4316,7 @@ static const struct net_device_ops macb_netdev_ops
>>> = {
>>>       .ndo_features_check    = macb_features_check,
>>>       .ndo_hwtstamp_set    = macb_hwtstamp_set,
>>>       .ndo_hwtstamp_get    = macb_hwtstamp_get,
>>> +    .ndo_setup_tc        = macb_setup_tc,
>>
>> This patch (or parts of it) should be merged with the previous ones.
>> Otherwise you introduce patches with code that is unused.
>>
> 
> Clubbing all comments on patch organization:
> I see that patch series gets merged into 2 set only.
> 
> 1/6 + 2/6 + 3/6 + 4/6 + 5/6 ==> 1/2
> 6/6 ==> 2/2

That should be good.

Thank you,
Claudiu

> 
> Please let me know your thoughts or suggestions.
> 
> 
> Thanks