netdev - Re: [PATCH net-next v4 10/15] net: ravb: Move delay mode set in the driver's ndo

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1c680be4-67b7-435b-9609-b5f7a45ca675@tuxon.dev>
Date: Mon, 29 Jan 2024 15:55:54 +0200
From: claudiu beznea <claudiu.beznea@...on.dev>
To: Sergey Shtylyov <s.shtylyov@....ru>, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
 richardcochran@...il.com, p.zabel@...gutronix.de, geert+renesas@...der.be
Cc: netdev@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: Re: [PATCH net-next v4 10/15] net: ravb: Move delay mode set in the
 driver's ndo_open API



On 24.01.2024 22:18, Sergey Shtylyov wrote:
> On 1/23/24 3:58 PM, Claudiu wrote:
> 
>> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
>>
>> Delay parsing and setting were done in the driver's probe API. As some IP
>> variants switch to reset mode (and thus registers content is lost) when
> 
>    s/registers content/the register contents/, perhaps?
> 
>> setting clocks (due to module standby functionality) to be able to
>> implement runtime PM keep the delay parsing in the driver's probe function
>> and move the delay applying function to the driver's ndo_open API.
>>
>> Along with it, both delay specific functions were kept together.
>>
>> Reviewed-by: Sergey Shtylyov <s.shtylyov@....ru>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> [...]
> 
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
>> index 8956c33c60e2..69d4854d6738 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -1800,6 +1800,59 @@ static int ravb_compute_gti(struct net_device *ndev)
>>  	return 0;
>>  }
>>  
>> +/* Set tx and rx clock internal delay modes */
>> +static void ravb_parse_delay_mode(struct device_node *np, struct net_device *ndev)
> 
>    Wait, why are you moving this? Its call site didn't move -- it's still
> down in the probe() method...

As explained in the patch description ("Along with it, both delay specific
functions were kept together.") functions were moved closed to each other
to have the code dealing with delay in the same place.

> 
> [...]
>> @@ -2785,10 +2793,7 @@ static int ravb_probe(struct platform_device *pdev)
>>  	if (error)
>>  		goto out_rpm_put;
>>  
>> -	if (info->internal_delay) {
>> -		ravb_parse_delay_mode(np, ndev);
>> -		ravb_set_delay_mode(ndev);
>> -	}
>> +	ravb_parse_delay_mode(np, ndev);
>>  
>>  	/* Allocate descriptor base address table */
>>  	priv->desc_bat_size = sizeof(struct ravb_desc) * DBAT_ENTRY_NUM;
>>
> 
> MBR, Sergey