Re: [PATCH 0/3] PCI: dwc: Improve code readability

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

 



Hi Krzysztof,

On Mon, Nov 13, 2023 at 1:22 PM Krzysztof Wilczyński <kw@xxxxxxxxx> wrote:
> > > > Now, while you are looking at things, can you also take care about the following:
> > > >
> > > >   drivers/pci/controller/dwc/pcie-rcar-gen4.c:439:15: warning: cast to smaller integer type 'enum dw_pcie_device_mode'
> > > from 'const void *' [-Wvoid-pointer-to-enum-cast]
> >
> > Thank you for the report!
> >
> > > > This requires adding structs for each data member of the of_device_id type.
> > >
> > > That sounds like overkill to me.
> > > An intermediate cast to uintptr_t should fix the issue as well.
> >
> > I confirmed that the uintptr_t fixed the issue.
>
> We declined a similar fix in the past[1] ...
>
> > I also think that adding a new struct with the mode is overkill.
>
> ... with the hopes that a driver could drop the switch statements in place
> of using the other pattern.  Also, to be consistent with other drivers that
> do this already.

Note that the issue of casting is something we cannot fix easily:
some *_device_id structs use "kernel_ulong_t" for the "data" member,
others use "void *".

git grep -W "_device_id" -- include/linux/mod_devicetable.h | grep data

In addition, several drivers use multiple types of device IDs, so you
cannot settle on one type to avoid casts.

Also, putting enum values in instances of that struct, as suggested,
increases kernel size, for IMHO no additional gain.  If there is more
data to put in the struct, I agree it makes sense to use a struct.

> > So, I would like to fix the issue by using the cast of uintptr_t.
>
> Sure.  I appreciate that this would be more work.  When you send your
> patch, can you include an update to the iproc driver (and credit the
> original author from [1])?  I would appreciate it.
>
> 1. https://lore.kernel.org/linux-pci/20230814230008.GA196797@bhelgaas/

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]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux