Re: [PATCH v4 01/20] driver core: Split devres APIs to device/devres.h

Arnd Bergmann
2025-02-11T09:39:38Z

On Tue, Feb 11, 2025, at 10:27, Andy Shevchenko wrote:
> On Tue, Feb 11, 2025 at 08:36:47AM +0100, Arnd Bergmann wrote:
>> On Mon, Feb 10, 2025, at 16:23, Andy Shevchenko wrote:
>> >
>> > TBH I have no quick idea how to address this. It seems that io.h
>> > includes device.h
>> > for no reason (but I haven't checked that carefully). OTOH, we need only
>> > IOMEM_IS_ERR() definition which can simply be moved from io.h to err.h
>> > as the
>> > former includes the latter and the definition depends only on
>> > compiler_types.h.
>> >
>> > Arnd?
>>
>> Removing linux/device.h from asm/io.h is probably the right step,
>> it really has no business in there and no other architecture
>> includes it. I don't see an IOMEM_IS_ERR() definition, do you
>> mean EEH_POSSIBLE_ERROR?
>
> The definition is in the generic header and patch here relies on
> that definition to fix the sparse warning. The simplest solution
> is to add another patch that simply moves the macro from
> linux/io.h to linux/err.h.

Ah, IOMEM_ERR_PTR(), not IOMEM_IS_ERR().

I don't mind moving that if it helps you, but don't see what
the problem is here. Is this missing because of a circular
#include list with linux/device.h including asm/io.h and vice
versa? If that is the root cause, then I assume there will be
additional problems either way until the loop can be broken.

>> Most of asm/eeh.h probably shouldn't be included by asm/io.h
>> either, my guess is that we can get away with the
>> eeh_{s,}{b,w,l,q}{_be} helpers, eeh_memcpy_fromio() and
>> eeh_check_failure(), which have no dependency on 'struct
>> device' in the header.
>>
>> Removing a giant header inclusion from another one likely causes
>> build regressions in drivers that should have included the
>> header (linux/device.h or something included by that) themselves,
>> so ideally there should be some separate build testing of
>> powerpc kernels.
>
> I believe this might be far out of scope for this series due to potential
> fallouts here and there. But would be good to have it separately.

It certainly gets towards yak-shaving, but it does look like
the best solution. It really depends on how much breaks -- if there
are only a couple of missing #include statements, I can see those
get merged early as a bugfix or as part of another series. If there
are a lot of them, it is probably not worth it.

Arnd