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

Geert Uytterhoeven geert at linux-m68k.org
Mon Nov 13 23:57:32 AEDT 2023


Hi Krzysztof,

On Mon, Nov 13, 2023 at 1:22 PM Krzysztof Wilczyński <kw at linux.com> 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 at linux-m68k.org

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


More information about the Linuxppc-dev mailing list