Re: [PATCH net v4 3/3] net: ethernet: oa_tc6: Disabled tx queues when disable_traffic is set

[Parthiban Veerasooran <parthiban.veerasooran@microchip.com>]

Imperative mood for the subject: "Disable tx queues when disable_traffic 
is set" (netdev convention).

On 21/07/26 7:50 am, Selvamani Rajagopal via B4 Relay wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> From: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
> 
> Previously, TX queue interface was stopped when
> disable_traffic flag was set. It is more appropriate to
> disable the queue as there is no recovery, once
> disable_traffic is set. Carrier is also marked off
"Carrier is also marked off" contradicts both the code and your own v4 
changelog: the patch only adds netif_tx_disable(), there is no 
netif_carrier_off(), and v4 says you reverted the carrier change. Please 
drop this sentence — the message currently describes behavior the patch 
doesn't implement.
> 
> Fixes: b542d13fab0f ("net: ethernet: oa_tc6: Interrupt is active low, level triggered.")
> Signed-off-by: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
> 
--- is missing
> changes in v4
>    - Reverted the the statement that turned carrier off on
typo.
>      disble_traffic, as it may have side effects
typo.
> changes in v3
>    - New patch. Carrier marked off once disable_traffic is set
> ---
>   drivers/net/ethernet/oa_tc6.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
> index 9e6850ccfe6c..4b8f6280be51 100644
> --- a/drivers/net/ethernet/oa_tc6.c
> +++ b/drivers/net/ethernet/oa_tc6.c
> @@ -730,6 +730,11 @@ static void oa_tc6_disable_traffic(struct oa_tc6 *tc6)
>          skb = oa_tc6_detach_waiting_tx_skb(tc6);
>          spin_unlock_bh(&tc6->tx_skb_lock);
> 
> +       /* disable_traffic, when set, is a point of no
> +        * return to working state. TX queues are
> +        * disabled.
> +        */
Comment wraps at ~40 columns; fill to ~80 like the rest of the file.

Best regards,
Parthiban V
> +       netif_tx_disable(tc6->netdev);
>          oa_tc6_drop_tx_skb(tc6, skb);
>          oa_tc6_free_ongoing_skbs(tc6);
>          oa_tc6_write_register(tc6, OA_TC6_REG_INT_MASK0, regval);
> 
> --
> 2.43.0
> 
>