Re: [PATCH 1/2] ACPI: Get acpi_device's parent from the parent field

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux