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
