Hi Rafael, On Mon, Nov 20, 2023 at 01:52:39PM +0100, Rafael J. Wysocki wrote: > On Mon, Nov 20, 2023 at 10:31 AM Sakari Ailus > <sakari.ailus@xxxxxxxxxxxxxxx> wrote: > > > > Hi Laurent, > > > > On Sat, Nov 18, 2023 at 08:50:49PM +0200, Laurent Pinchart wrote: > > > Hi Sakari, > > > > > > Thank you for the patch. > > > > > > On Fri, Nov 17, 2023 at 01:14:29PM +0200, Sakari Ailus wrote: > > > > Document that acpi_dev_state_d0() can be used to tell if the device was > > > > powered on for probe. > > > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > > > > --- > > > > Documentation/firmware-guide/acpi/non-d0-probe.rst | 8 ++++++++ > > > > 1 file changed, 8 insertions(+) > > > > > > > > diff --git a/Documentation/firmware-guide/acpi/non-d0-probe.rst b/Documentation/firmware-guide/acpi/non-d0-probe.rst > > > > index 7afd16701a02..815bcc8db69f 100644 > > > > --- a/Documentation/firmware-guide/acpi/non-d0-probe.rst > > > > +++ b/Documentation/firmware-guide/acpi/non-d0-probe.rst > > > > @@ -24,6 +24,14 @@ there's a problem with the device, the driver likely probes just fine but the > > > > first user will find out the device doesn't work, instead of a failure at probe > > > > time. This feature should thus be used sparingly. > > > > > > > > +ACPI framework > > > > +-------------- > > > > + > > > > +Use the Linux ACPI framework function :c:func:`acpi_dev_state_d0()` to tell > > > > +whether the device was powered on for probe. :c:func:`acpi_dev_state_d0()` > > > > +returns true if the device is powered on, false otherwise. For non-ACPI backed > > > > +devices it returns true always. > > > > + > > > > > > While this is true, I don't want to see drivers having to call > > > ACPI-specific functions, the same way you dislike OF-specific functions > > > in drivers. Please find a better way to handle this. > > > > The functionality is only available on ACPI and the function does the right > > thing on non-ACPI platforms. I don't see an issue here. > > The issue would be calling an ACPI-specific function from code that's > otherwise firmware-agnostic, AFAICS. > > It would be good to have a more generic way of checking whether or not > a device is operational. In DT case it's up to the driver to do that, so the device is powered off. > > > Feel free to post DT binding patches on suggested device power state during > > probe. :-) I think DT would benefit from this as well: the at24 driver is > > widely used and suddenly making probe() not talk to the chip (or even power > > it up) at all would probably be seen as a regression. > > In the DT case it is more complicated, though, at least in general, > because there may be multiple clocks and regulators the device depends > on and you may need to toggle a GPIO line too. I don't think it is as what's missing is the desired power state during probe, i.e. whether or not the device will be powered on. It wasn't there in ACPI either before it was added. The problem is slightly lesser on DT though as it's up to the driver whether or not to power on the device. In the example I gave above, however, e.g. the at24 driver can't be modified to keep the device powered off and at the same time expected people would remain content with it. So this information should come from firmware. -- Regards, Sakari Ailus
