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

[Lukasz Majewski <lukma@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 3494 bytes --]

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@denx.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@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]