Re: [PATCH v4 2/3] net: ethernet: renesas: Add Ethernet Switch driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Arnd,

On Mon, Oct 24, 2022 at 9:55 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
> On Mon, Oct 24, 2022, at 17:27, Geert Uytterhoeven wrote:
> > On Wed, Oct 19, 2022 at 1:17 PM kernel test robot <lkp@xxxxxxxxx> wrote:
> >>    drivers/net/ethernet/renesas/rswitch.c: In function 'rswitch_ext_desc_get_dptr':
> >> >> drivers/net/ethernet/renesas/rswitch.c:355:71: warning: left shift count >= width of type [-Wshift-count-overflow]
> >>      355 |         return __le32_to_cpu(desc->dptrl) | (dma_addr_t)(desc->dptrh) << 32;
> >>          |                                                                       ^~
> >>    drivers/net/ethernet/renesas/rswitch.c: In function 'rswitch_ext_ts_desc_get_dptr':
> >>    drivers/net/ethernet/renesas/rswitch.c:367:71: warning: left shift count >= width of type [-Wshift-count-overflow]
> >>      367 |         return __le32_to_cpu(desc->dptrl) | (dma_addr_t)(desc->dptrh) << 32;
> >>          |                                                                       ^~
> >>
> >>
> >> vim +355 drivers/net/ethernet/renesas/rswitch.c
> >>
> >>    352
> >>    353  static dma_addr_t rswitch_ext_desc_get_dptr(struct rswitch_ext_desc *desc)
> >>    354  {
> >>  > 355          return __le32_to_cpu(desc->dptrl) | (dma_addr_t)(desc->dptrh) << 32;
> >
> > A simple fix would be to replace the cast to "dma_addr_t" by a cast to "u64".
> > A more convoluted fix would be:
> >
> >     dma_addr_t dma;
> >
> >     dma = __le32_to_cpu(desc->dptrl);
> >     if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT))
> >             dma |= (u64)desc->dptrh << 32;
> >     return dma;
> >
> > Looking at the gcc compiler output, the both cases are optimized to the
> > exact same code, for both arm32 and arm64, so I'd go for the simple fix.
> >
> > BTW, if struct rswitch_ext_desc would just extend struct rswitch_desc,
> > you could use rswitch_ext_desc_get_dptr() for both.
> >
>
> Regardless of which way this is expressed, it looked like there is
> a missing __le32_to_cpu() around the high word.

I think it's OK, because desc->dptrh is u8:

    struct rswitch_desc {
            __le16 info_ds; /* Descriptor size */
            u8 die_dt;      /* Descriptor interrupt enable and type */
            __u8  dptrh;    /* Descriptor pointer MSB */
            __le32 dptrl;   /* Descriptor pointer LSW */
    } __packed;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux