linux-kernel - Re: [net-next v7 4/7] net: mtip: The L2 switch driver for imx287

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: <20250425094907.27740d07@wsk>
Date: Fri, 25 Apr 2025 09:49:07 +0200
From: Lukasz Majewski <lukma@...x.de>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew+netdev@...n.ch>,
 davem@...emloft.net, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni
 <pabeni@...hat.com>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Shawn Guo
 <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix
 Kernel Team <kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>,
 Richard Cochran <richardcochran@...il.com>, netdev@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org, Stefan Wahren
 <wahrenst@....net>, Simon Horman <horms@...nel.org>, Andrew Lunn
 <andrew@...n.ch>
Subject: Re: [net-next v7 4/7] net: mtip: The L2 switch driver for imx287

Hi Krzysztof, Jakub

> On 25/04/2025 08:05, Lukasz Majewski wrote:
> > Hi Krzysztof, Jakub,
> >   
> >> On 25/04/2025 03:11, Jakub Kicinski wrote:  
> >>> On Wed, 23 Apr 2025 09:29:08 +0200 Lukasz Majewski wrote:    
> >>>> This patch series provides support for More Than IP L2 switch
> >>>> embedded in the imx287 SoC.
> >>>>
> >>>> This is a two port switch (placed between uDMA[01] and
> >>>> MAC-NET[01]), which can be used for offloading the network
> >>>> traffic.
> >>>>
> >>>> It can be used interchangeably with current FEC driver - to be
> >>>> more specific: one can use either of it, depending on the
> >>>> requirements.
> >>>>
> >>>> The biggest difference is the usage of DMA - when FEC is used,
> >>>> separate DMAs are available for each ENET-MAC block.
> >>>> However, with switch enabled - only the DMA0 is used to
> >>>> send/receive data to/form switch (and then switch sends them to
> >>>> respecitive ports).    
> >>>
> >>> Lots of sparse warnings and build issues here, at least on x86.
> >>>
> >>> Could you make sure it's clean with an allmodconfig config, 
> >>> something like:
> >>>
> >>> make C=1 W=1 drivers/net/ethernet/freescale/mtipsw/     
> >>
> >> ... and W=1 with clang as well.
> >>  
> > 
> > The sparse warnings are because of struct switch_t casting and
> > register  
> 
> clang W=1 fails on errors, so it is not only sparse:
> 
> error: cast to smaller integer type 'uint' (aka 'unsigned int') from
> 'struct cbd_t *' [-Werror,-Wpointer-to-int-cast]
> 
> You probably wanted there kenel_ulong_t.

This I did not catch earlier (probably because of my testing on
imx287). Thanks for spotting it.

> 
> > access with this paradigm (as it is done with other drivers).  
> 
> I don't understand. I see code like:
> 
> 	struct switch_t *fecp = fep->hwp;
> 
> But this is not a cast - the same types.

For example:

The warning:

mtipl2sw.c:208:30: warning: incorrect type in argument 1 (different
address spaces) mtipl2sw.c:208:30:    expected void const volatile
[noderef] __iomem *addr mtipl2sw.c:208:30:    got unsigned int *

corresponds to:
 info->maclo = readl(&fecp->ESW_LREC0);   [*]

where:

struct switch_t {
        u32 ESW_REVISION;
        u32 ESW_SCRATCH;
	...
        /*from 0x420-0x4FC*/
        u32 esw_reserved9[57];
        /*0xFC0DC500---0xFC0DC508*/
        u32 ESW_LREC0;
        u32 ESW_LREC1;
        u32 ESW_LSR;
};


The 'u32' type seems to be valid here as this register is 32 bit wide.

To fix the sparse warnings - I think that I will replace [*] with:

info->maclo = readl((u32 __iomem *)&fecp->ESW_LREC0);

as such solution is used in a wide way in the mainline kernel.

Is this the acceptable solution?

> > 
> > What is the advise here from the community?
> >   
> >> Best regards,
> >> Krzysztof  
> > 
> > 
> > 
> > 
> > Best regards,
> > 
> > Lukasz Majewski
> > 
> > --
> > 
> > DENX Software Engineering GmbH,      Managing Director: Erika Unter
> > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > lukma@...x.de  
> 
> 
> Best regards,
> Krzysztof




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@...x.de

Content of type "application/pgp-signature" skipped