On Tue, Feb 11, 2025 at 10:39:16AM +0100, Arnd Bergmann wrote:
> 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().
Oh, yes, sorry for the confusion.
> 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.
I don't see how. io.h already includes err.h, so whoever includes io.h should
have that as previously.
> >> 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.
--
With Best Regards,
Andy Shevchenko
