[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMpxmJWziqW-PiJPSm6aH5aXbYktMJfVjJfvfGxv8fdbWKydqg@mail.gmail.com>
Date: Wed, 12 Aug 2020 20:07:04 +0200
From: Bartosz Golaszewski <bgolaszewski@...libre.com>
To: Sakari Ailus <sakari.ailus@....fi>
Cc: Sakari Ailus <sakari.ailus@...ux.intel.com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
linux-i2c <linux-i2c@...r.kernel.org>,
Wolfram Sang <wsa@...-dreams.de>, linux-acpi@...r.kernel.org,
Bingbu Cao <bingbu.cao@...el.com>,
linux-media <linux-media@...r.kernel.org>,
Chiranjeevi Rapolu <chiranjeevi.rapolu@...el.com>,
Hyungwoo Yang <hyungwoo.yang@...el.com>,
Arnd Bergmann <arnd@...db.de>,
LKML <linux-kernel@...r.kernel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Rajmohan Mani <rajmohan.mani@...el.com>,
Tomasz Figa <tfiga@...omium.org>
Subject: Re: [PATCH v4 5/6] at24: Support probing while off
On Tue, Aug 11, 2020 at 10:00 AM Sakari Ailus <sakari.ailus@....fi> 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@....fi> 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
Powered by blists - more mailing lists