2023-06-23 13:42:04

by Nicolas Ferre

[permalink] [raw]
Subject: Re: net: macb: sparse warning fixes

Hi Ben,

On 22/06/2023 at 15:05, Ben Dooks wrote:
> These are 3 hopefully easy patches for fixing sparse errors due to
> endian-ness warnings. There are still some left, but there are not
> as easy as they mix host and network fields together.
>
> For example, gem_prog_cmp_regs() has two u32 variables that it does
> bitfield manipulation on for the tcp ports and these are __be16 into
> u32, so not sure how these are meant to be changed. I've also no hardware
> to test on, so even if these did get changed then I can't check if it is
> working pre/post change.

Do you know if there could be any impact on performance (even if limited)?

Best regards,
Nicolas

> Also gem_writel and gem_writel_n, it is not clear if both of these are
> meant to be host order or not.
>
> Ben Dooks (3):
> net: macb: check constant to define and fix __be32 warnings
> net: macb: add port constant to fix __be16 warnings
> net: macb: fix __be32 warnings in debug code
>
> drivers/net/ethernet/cadence/macb_main.c | 25 +++++++++++++-----------
> 1 file changed, 14 insertions(+), 11 deletions(-)

--
Nicolas Ferre



2023-06-23 16:04:57

by Andrew Lunn

[permalink] [raw]
Subject: Re: net: macb: sparse warning fixes

On Fri, Jun 23, 2023 at 03:16:23PM +0200, Nicolas Ferre wrote:
> Hi Ben,
>
> On 22/06/2023 at 15:05, Ben Dooks wrote:
> > These are 3 hopefully easy patches for fixing sparse errors due to
> > endian-ness warnings. There are still some left, but there are not
> > as easy as they mix host and network fields together.
> >
> > For example, gem_prog_cmp_regs() has two u32 variables that it does
> > bitfield manipulation on for the tcp ports and these are __be16 into
> > u32, so not sure how these are meant to be changed. I've also no hardware
> > to test on, so even if these did get changed then I can't check if it is
> > working pre/post change.
>
> Do you know if there could be any impact on performance (even if limited)?

Hi Nicolas

This is inside a netdev_dbg(). So 99% of the time it is compiled
out. The other 1% of the time, your 115200 baud serial port is
probably the bottleneck, not an endianness swap.

Andrew

2023-06-23 16:37:21

by Nicolas Ferre

[permalink] [raw]
Subject: Re: net: macb: sparse warning fixes

On 23/06/2023 at 17:38, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Fri, Jun 23, 2023 at 03:16:23PM +0200, Nicolas Ferre wrote:
>> Hi Ben,
>>
>> On 22/06/2023 at 15:05, Ben Dooks wrote:
>>> These are 3 hopefully easy patches for fixing sparse errors due to
>>> endian-ness warnings. There are still some left, but there are not
>>> as easy as they mix host and network fields together.
>>>
>>> For example, gem_prog_cmp_regs() has two u32 variables that it does
>>> bitfield manipulation on for the tcp ports and these are __be16 into
>>> u32, so not sure how these are meant to be changed. I've also no hardware
>>> to test on, so even if these did get changed then I can't check if it is
>>> working pre/post change.
>>
>> Do you know if there could be any impact on performance (even if limited)?
>
> Hi Nicolas
>
> This is inside a netdev_dbg(). So 99% of the time it is compiled
> out. The other 1% of the time, your 115200 baud serial port is
> probably the bottleneck, not an endianness swap.

Yeah, sure thing: I was not talking about the 3/3 patch where, yes,
indeed it doesn't have an importance.

Others in gem_enable_flow_filters() and gem_prog_cmp_regs() look like
they are away from hot path anyway, so here again probably no impact.

Regards,
Nicolas

--
Nicolas Ferre