On Wed, Nov 17, 2021 at 03:52:21PM +0100, Rafael J. Wysocki wrote:
> On Wed, Nov 10, 2021 at 9:09 AM Sakari Ailus
> <sakari.ailus@xxxxxxxxxxxxxxx> wrote:
> >
> > Hi Andy,
> >
> > Thanks for the review.
> >
> > On Tue, Nov 09, 2021 at 02:19:13PM +0200, Andy Shevchenko wrote:
> > > On Tue, Nov 09, 2021 at 01:19:34PM +0200, Sakari Ailus wrote:
> > > > Printk modifier %pfw is used to print the full path of the device name.
> > > > This is obtained device by device until a device no longer has a parent.
> > > >
> > > > On ACPI getting the parent fwnode is done by calling acpi_get_parent()
> > > > which tries to down() a semaphore. But local IRQs are now disabled in
> > > > vprintk_store() before the mutex is acquired. This is obviously a problem.
> > > >
> > > > Luckily struct device, embedded in struct acpi_device, has a parent field
> > > > already. Use that field to get the parent instead of relying on
> > > > acpi_get_parent().
> > >
> > > Thanks, with the below addressed
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > >
> > > > Fixes: 3bd32d6a2ee6 ("lib/vsprintf: Add %pfw conversion specifier for printing fwnode names")
> > > > Cc: stable@xxxxxxxxxxxxxxx # v5.5+
> > > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > > > ---
> > > > drivers/acpi/property.c | 14 ++++++--------
> > > > 1 file changed, 6 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> > > > index e312ebaed8db4..dc97711ba8081 100644
> > > > --- a/drivers/acpi/property.c
> > > > +++ b/drivers/acpi/property.c
> > > > @@ -1089,16 +1089,14 @@ struct fwnode_handle *acpi_node_get_parent(const struct fwnode_handle *fwnode)
> > > > if (is_acpi_data_node(fwnode)) {
> > > > /* All data nodes have parent pointer so just return that */
> > > > return to_acpi_data_node(fwnode)->parent;
> > >
> > > ...
> > >
> > > > - } else if (is_acpi_device_node(fwnode)) {
> > > > + }
> > >
> > > > + if (is_acpi_device_node(fwnode)) {
> > >
> > > Unneeded change. Yes I know that 'else' here can be skipped. But in such cases
> > > it's a trade-off between changes, code readability and maintenance. Since here
> > > it's a fix, backporting concerns are also play role.
> >
> > The patch applies cleanly to 5.5, the oldest kernel where it's needed.
>
> Which doesn't matter too much.
>
> The change above is not needed and there is no point making it in
> which otherwise is a fix, not just because of the backporting
> concerns, but also for the sake of cleanliness in general.
>
> Have you posted the v3 already? If not, please update the patch as
> requested by Andy.
I'll post v3 soon.
--
Sakari Ailus
