Re: [PATCH v4 5/6] at24: Support probing while off

[Bartosz Golaszewski]

On Tue, Aug 11, 2020 at 10:00 AM Sakari Ailus <sakari.ailus@xxxxxx> wrote:
>
> Hi Bartosz,
>
> On Mon, Aug 10, 2020 at 08:12:00PM +0200, Bartosz Golaszewski wrote:
> > On Mon, Aug 10, 2020 at 10:26 AM Sakari Ailus <sakari.ailus@xxxxxx> wrote:
> > >
> >
> > [snip]
> >
> > > >
> > > > Rafael: I think that there are two issues with patch 1/5:
> > > > 1. It adds a very specific boolean flag to a structure that's meant to
> > > > be very general. As I pointed out in the i2c patch: at the very least
> > > > this could be made into an int storing flag values, instead of a
> > > > boolean field. But rather than that - it looks to me more like a
> > > > device (or bus) feature than a driver feature. Is there any ACPI flag
> > > > we could use to pass this information to the driver model without
> > > > changing the driver structure?
> > >
> > > To my knowledge there isn't. The fact that I涎 devices are powered on for
> > > probe in ACPI based systems is specific to Linux kernel and not ACPI as
> > > such.
> > >
> > > The reason this needs to be in a generic struct is that the device's power
> > > state will be changed before any interaction with the driver takes place as
> > > it's the I涎 framework that powers on the device.
> > >
> >
> > I'm not sure I'm following. Looking at patch 1/6 struct device already
> > exists so why can't this information be conveyed "per device" as
> > opposed to "per driver"?
>
> It's both driver and device.
>
> Suppose there's no indication of driver support. If you add the property
> telling the device shouldn't be powered on for probe, it won't be. And if
> the driver doesn't support that, probe will fail. That could happen e.g.
> when running an older kernel on a system that happens to specify this
> property for a given device.
>
> You could view this as a driver bug of course. I still think it's better to
> make driver support for this explicit, and avoid making this a practical
> problem anywhere.
>

I see. I'm not sure this is the correct solution but let's see what
Wolfram says. From my side: I'd prefer to see the
disable_i2c_core_irq_mapping converted to flags first and then the
flags extended with whatever you need. disable_i2c_core_irq_mapping
could also be removed AFAICT - nobody uses it.

Bart