netdev - Re: [PATCH net-next v3 09/10] net: ti: icssg-prueth: Modify common functions for SR1.0

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]

Date: Tue, 27 Feb 2024 12:42:55 +0000
From: Diogo Ivo <diogo.ivo@...mens.com>
To: Roger Quadros <rogerq@...nel.org>, danishanwar@...com,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, andrew@...n.ch, dan.carpenter@...aro.org,
 linux-arm-kernel@...ts.infradead.org, netdev@...r.kernel.org
Cc: jan.kiszka@...mens.com, diogo.ivo@...mens.com
Subject: Re: [PATCH net-next v3 09/10] net: ti: icssg-prueth: Modify common
 functions for SR1.0

On 2/26/24 17:33, Roger Quadros wrote:
> 
> 
> On 21/02/2024 17:24, Diogo Ivo wrote:
>> Some parts of the logic differ only slightly between Silicon Revisions.
>> In these cases add the bits that differ to a common function that
>> executes those bits conditionally based on the Silicon Revision.
>>
>> Based on the work of Roger Quadros, Vignesh Raghavendra and
>> Grygorii Strashko in TI's 5.10 SDK [1].
>>
>> [1]: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.ti.com%2Fcgit%2Fti-linux-kernel%2Fti-linux-kernel%2Ftree%2F%3Fh%3Dti-linux-5.10.y&data=05%7C02%7Cdiogo.ivo%40siemens.com%7C44253ecc112a4f939f5e08dc36f11754%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C638445656302550240%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=kXkC6HubTh0yeftDSzmIy47tJmlnISQjMbWoOqvEAx0%3D&reserved=0
>>
>> Co-developed-by: Jan Kiszka <jan.kiszka@...mens.com>
>> Signed-off-by: Jan Kiszka <jan.kiszka@...mens.com>
>> Signed-off-by: Diogo Ivo <diogo.ivo@...mens.com>
>> ---
>>   drivers/net/ethernet/ti/icssg/icssg_common.c | 46 +++++++++++++++-----
>>   drivers/net/ethernet/ti/icssg/icssg_prueth.c |  4 +-
>>   drivers/net/ethernet/ti/icssg/icssg_prueth.h |  2 +-
>>   3 files changed, 38 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c

...

>> -	emac->rx_flow_id_base = k3_udma_glue_rx_get_flow_id_base(rx_chn->rx_chn);
>> -	netdev_dbg(ndev, "flow id base = %d\n", emac->rx_flow_id_base);
>> +	flow_id_base = k3_udma_glue_rx_get_flow_id_base(rx_chn->rx_chn);
>> +	if (!strcmp(name, "rxmgm")) {
> 
> if (emac->is_sr1 && !strcmp(name, "rxmgm")) ?

Since technically the DT should only contain "rxmgm" for SR1.0 I did not
add the emac->is_sr1 check but it can't hurt to be safer so I'll add it.

...

>>   	if (eth0_node) {
>> -		ret = prueth_get_cores(prueth, ICSS_SLICE0);
>> +		ret = prueth_get_cores(prueth, ICSS_SLICE0, true);
> 
> Isn't this SR2.0 device driver? so is_sr1 parameter should be false?
> 
>>   		if (ret)
>>   			goto put_cores;
>>   	}
>>   
>>   	if (eth1_node) {
>> -		ret = prueth_get_cores(prueth, ICSS_SLICE1);
>> +		ret = prueth_get_cores(prueth, ICSS_SLICE1, true);
> 
> here too?

Yes, you are correct, thank you for catching that.

Best regards,
Diogo