[v1] ACPI: PNP: Introduce list of known non-PNP devices

Message ID 12149290.O9o76ZdvQC@kreacher
State New
Headers
Series [v1] ACPI: PNP: Introduce list of known non-PNP devices |

Commit Message

Rafael J. Wysocki Jan. 10, 2023, 5:58 p.m. UTC
  From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

In some cases, PNP device IDs from acpi_pnp_device_ids[] are returned by
_CID for devices for which matching platform drivers are present in the
kernel and should be bound to them.  However, the IDs coming from _CID
cause the PNP scan handler to attach to those devices which prevents
platform device objects from being created for them.

Address this by introducing a list of known non-PNP device IDs into
acpi_pnp.c such that if a device ID is there in that list, it cannot be
attached to by the PNP scan handler and add the platform runtime update
and telemetry device IDs to that list to start with.

Reported-by: Chen Yu <yu.c.chen@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_pnp.c |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
  

Comments

Zhang, Rui Jan. 11, 2023, 7:25 a.m. UTC | #1
On Tue, 2023-01-10 at 18:58 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> In some cases, PNP device IDs from acpi_pnp_device_ids[] are returned
> by
> _CID for devices for which matching platform drivers are present in
> the
> kernel and should be bound to them.  However, the IDs coming from
> _CID
> cause the PNP scan handler to attach to those devices which prevents
> platform device objects from being created for them.
> 
> Address this by introducing a list of known non-PNP device IDs into
> acpi_pnp.c such that if a device ID is there in that list, it cannot
> be
> attached to by the PNP scan handler and add the platform runtime
> update
> and telemetry device IDs to that list to start with.
> 
> Reported-by: Chen Yu <yu.c.chen@intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/acpi/acpi_pnp.c |   14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> Index: linux-pm/drivers/acpi/acpi_pnp.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/acpi_pnp.c
> +++ linux-pm/drivers/acpi/acpi_pnp.c
> @@ -348,10 +348,22 @@ static bool acpi_pnp_match(const char *i
>  	return false;
>  }
>  
> +/*
> + * If one of the device IDs below is present in the list of device
> IDs of a
> + * given ACPI device object, the PNP scan handler will not attach to
> that
> + * object, because there is a proper non-PNP driver in the kernel
> for the
> + * device represented by it.
> + */
> +static const struct acpi_device_id acpi_nonpnp_device_ids[] = {
> +	{"INTC1080"},
> +	{"INTC1081"},
> +	{""},
> +};
> +
>  static int acpi_pnp_attach(struct acpi_device *adev,
>  			   const struct acpi_device_id *id)
>  {
> -	return 1;
> +	return !!acpi_match_device_ids(adev, acpi_nonpnp_device_ids);

acpi_match_device_ids() returns True if the id matches, and in this
case, acpi_pnp_attach() should return false, right?

thanks,
rui

>  }
>  
>  static struct acpi_scan_handler acpi_pnp_handler = {
> 
> 
>
  
Zhang, Rui Jan. 11, 2023, 8:02 a.m. UTC | #2
CC Zhang Yang who has tested the patch.

On Wed, 2023-01-11 at 07:25 +0000, Zhang, Rui wrote:
> On Tue, 2023-01-10 at 18:58 +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > In some cases, PNP device IDs from acpi_pnp_device_ids[] are
> > returned
> > by
> > _CID for devices for which matching platform drivers are present in
> > the
> > kernel and should be bound to them.  However, the IDs coming from
> > _CID
> > cause the PNP scan handler to attach to those devices which
> > prevents
> > platform device objects from being created for them.
> > 
> > Address this by introducing a list of known non-PNP device IDs into
> > acpi_pnp.c such that if a device ID is there in that list, it
> > cannot
> > be
> > attached to by the PNP scan handler and add the platform runtime
> > update
> > and telemetry device IDs to that list to start with.
> > 
> > Reported-by: Chen Yu <yu.c.chen@intel.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/acpi/acpi_pnp.c |   14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > Index: linux-pm/drivers/acpi/acpi_pnp.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/acpi_pnp.c
> > +++ linux-pm/drivers/acpi/acpi_pnp.c
> > @@ -348,10 +348,22 @@ static bool acpi_pnp_match(const char *i
> >  	return false;
> >  }
> >  
> > +/*
> > + * If one of the device IDs below is present in the list of device
> > IDs of a
> > + * given ACPI device object, the PNP scan handler will not attach
> > to
> > that
> > + * object, because there is a proper non-PNP driver in the kernel
> > for the
> > + * device represented by it.
> > + */
> > +static const struct acpi_device_id acpi_nonpnp_device_ids[] = {
> > +	{"INTC1080"},
> > +	{"INTC1081"},
> > +	{""},
> > +};
> > +
> >  static int acpi_pnp_attach(struct acpi_device *adev,
> >  			   const struct acpi_device_id *id)
> >  {
> > -	return 1;
> > +	return !!acpi_match_device_ids(adev, acpi_nonpnp_device_ids);
> 
> acpi_match_device_ids() returns True if the id matches, and in this
> case, acpi_pnp_attach() should return false, right?
> 
It is __acpi_match_device() that returns True when matches, and
acpi_match_device_ids() returns 0 in this case.
So this is not a bug, sorry for the noise.

thanks,
rui

> thanks,
> rui
> 
> >  }
> >  
> >  static struct acpi_scan_handler acpi_pnp_handler = {
> > 
> > 
> >
  
Zhang, Yang5 Jan. 11, 2023, 8:07 a.m. UTC | #3
-----Original Message-----
From: Zhang, Rui <rui.zhang@intel.com> 
Sent: Wednesday, January 11, 2023 4:02 PM
To: rjw@rjwysocki.net; linux-acpi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org; Chen, Yu C <yu.c.chen@intel.com>; Zhang, Yang5 <yang5.zhang@intel.com>
Subject: Re: [PATCH v1] ACPI: PNP: Introduce list of known non-PNP devices

CC Zhang Yang who has tested the patch.

On Wed, 2023-01-11 at 07:25 +0000, Zhang, Rui wrote:
> On Tue, 2023-01-10 at 18:58 +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > In some cases, PNP device IDs from acpi_pnp_device_ids[] are 
> > returned by _CID for devices for which matching platform drivers are 
> > present in the kernel and should be bound to them.  However, the IDs 
> > coming from _CID cause the PNP scan handler to attach to those 
> > devices which prevents platform device objects from being created 
> > for them.
> > 
> > Address this by introducing a list of known non-PNP device IDs into 
> > acpi_pnp.c such that if a device ID is there in that list, it cannot 
> > be attached to by the PNP scan handler and add the platform runtime 
> > update and telemetry device IDs to that list to start with.
> > 
> > Reported-by: Chen Yu <yu.c.chen@intel.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Tested-by: Zhang Yang <Yang5.zhang@intel.com>

Thanks 
Yang 
> > ---
> >  drivers/acpi/acpi_pnp.c |   14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > Index: linux-pm/drivers/acpi/acpi_pnp.c 
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/acpi_pnp.c
> > +++ linux-pm/drivers/acpi/acpi_pnp.c
> > @@ -348,10 +348,22 @@ static bool acpi_pnp_match(const char *i
> >  	return false;
> >  }
> >  
> > +/*
> > + * If one of the device IDs below is present in the list of device
> > IDs of a
> > + * given ACPI device object, the PNP scan handler will not attach
> > to
> > that
> > + * object, because there is a proper non-PNP driver in the kernel
> > for the
> > + * device represented by it.
> > + */
> > +static const struct acpi_device_id acpi_nonpnp_device_ids[] = {
> > +	{"INTC1080"},
> > +	{"INTC1081"},
> > +	{""},
> > +};
> > +
> >  static int acpi_pnp_attach(struct acpi_device *adev,
> >  			   const struct acpi_device_id *id)  {
> > -	return 1;
> > +	return !!acpi_match_device_ids(adev, acpi_nonpnp_device_ids);
> 
> acpi_match_device_ids() returns True if the id matches, and in this 
> case, acpi_pnp_attach() should return false, right?
> 
It is __acpi_match_device() that returns True when matches, and
acpi_match_device_ids() returns 0 in this case.
So this is not a bug, sorry for the noise.

thanks,
rui

> thanks,
> rui
> 
> >  }
> >  
> >  static struct acpi_scan_handler acpi_pnp_handler = {
> > 
> > 
> >
  

Patch

Index: linux-pm/drivers/acpi/acpi_pnp.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_pnp.c
+++ linux-pm/drivers/acpi/acpi_pnp.c
@@ -348,10 +348,22 @@  static bool acpi_pnp_match(const char *i
 	return false;
 }
 
+/*
+ * If one of the device IDs below is present in the list of device IDs of a
+ * given ACPI device object, the PNP scan handler will not attach to that
+ * object, because there is a proper non-PNP driver in the kernel for the
+ * device represented by it.
+ */
+static const struct acpi_device_id acpi_nonpnp_device_ids[] = {
+	{"INTC1080"},
+	{"INTC1081"},
+	{""},
+};
+
 static int acpi_pnp_attach(struct acpi_device *adev,
 			   const struct acpi_device_id *id)
 {
-	return 1;
+	return !!acpi_match_device_ids(adev, acpi_nonpnp_device_ids);
 }
 
 static struct acpi_scan_handler acpi_pnp_handler = {