[PATCH v1] platform/surface: surfacepro3_button: Check ACPI_COMPANION()

Posted by Rafael J. Wysocki 1 month ago

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Every platform driver can be forced to match a device that doesn't match
its list of device IDs because of device_match_driver_override(), so
platform drivers that rely on the existence of a device's ACPI companion
object need to verify its presence.

Accordingly, add a requisite ACPI_COMPANION() check against NULL to the
surfacepro3_button driver.

Fixes: d913a5a12b40 ("platform/surface: surfacepro3_button: Convert to a platform driver")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/surface/surfacepro3_button.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

--- a/drivers/platform/surface/surfacepro3_button.c
+++ b/drivers/platform/surface/surfacepro3_button.c
@@ -185,12 +185,15 @@ static bool surface_button_check_MSHW004
 
 static int surface_button_probe(struct platform_device *pdev)
 {
-	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
 	struct surface_button *button;
+	struct acpi_device *device;
 	struct input_dev *input;
-	const char *hid = acpi_device_hid(device);
 	int error;
 
+	device = ACPI_COMPANION(&pdev->dev);
+	if (!device)
+		return -ENODEV;
+
 	if (strncmp(acpi_device_bid(device), SURFACE_BUTTON_OBJ_NAME,
 	    strlen(SURFACE_BUTTON_OBJ_NAME)))
 		return -ENODEV;
@@ -210,7 +213,8 @@ static int surface_button_probe(struct p
 	}
 
 	strscpy(acpi_device_name(device), SURFACE_BUTTON_DEVICE_NAME);
-	snprintf(button->phys, sizeof(button->phys), "%s/buttons", hid);
+	snprintf(button->phys, sizeof(button->phys), "%s/buttons",
+		 acpi_device_hid(device));
 
 	input->name = acpi_device_name(device);
 	input->phys = button->phys;

Re: [PATCH v1] platform/surface: surfacepro3_button: Check ACPI_COMPANION()

Posted by Ilpo Järvinen 3 weeks, 3 days ago

On Tue, 12 May 2026 18:30:26 +0200, Rafael J. Wysocki wrote:

> Every platform driver can be forced to match a device that doesn't match
> its list of device IDs because of device_match_driver_override(), so
> platform drivers that rely on the existence of a device's ACPI companion
> object need to verify its presence.
> 
> Accordingly, add a requisite ACPI_COMPANION() check against NULL to the
> surfacepro3_button driver.
> 
> [...]


Thank you for your contribution, it has been applied to my local
review-ilpo-fixes branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-fixes branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/1] platform/surface: surfacepro3_button: Check ACPI_COMPANION()
      commit: 5798b46271e229dab05e47537ac30b76f81728da

--
 i.

Re: [PATCH v1] platform/surface: surfacepro3_button: Check ACPI_COMPANION()

Posted by Chen, Yu C 4 weeks ago

On 5/13/2026 12:30 AM, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> Every platform driver can be forced to match a device that doesn't match
> its list of device IDs because of device_match_driver_override(), so
> platform drivers that rely on the existence of a device's ACPI companion
> object need to verify its presence.
> 
> Accordingly, add a requisite ACPI_COMPANION() check against NULL to the
> surfacepro3_button driver.
> 
> Fixes: d913a5a12b40 ("platform/surface: surfacepro3_button: Convert to a platform driver")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Thanks for the fix, also learned that an override triggers a force attach.

Reviewed-by: Chen Yu <yu.c.chen@intel.com>

thanks,
Chenyu

Re: [PATCH v1] platform/surface: surfacepro3_button: Check ACPI_COMPANION()

Posted by Andy Shevchenko 1 month ago

On Tue, May 12, 2026 at 06:30:26PM +0200, Rafael J. Wysocki wrote:

> Every platform driver can be forced to match a device that doesn't match
> its list of device IDs because of device_match_driver_override(), so
> platform drivers that rely on the existence of a device's ACPI companion
> object need to verify its presence.
> 
> Accordingly, add a requisite ACPI_COMPANION() check against NULL to the
> surfacepro3_button driver.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

>  	strscpy(acpi_device_name(device), SURFACE_BUTTON_DEVICE_NAME);
> -	snprintf(button->phys, sizeof(button->phys), "%s/buttons", hid);
> +	snprintf(button->phys, sizeof(button->phys), "%s/buttons",
> +		 acpi_device_hid(device));

I'm wondering if some CIs will now generate a snprintf() warning
(this was in the original code if the compiler can prove no truncation
 of the string, though).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1] platform/surface: surfacepro3_button: Check ACPI_COMPANION()

Posted by Rafael J. Wysocki 1 month ago

On Tue, May 12, 2026 at 6:42 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, May 12, 2026 at 06:30:26PM +0200, Rafael J. Wysocki wrote:
>
> > Every platform driver can be forced to match a device that doesn't match
> > its list of device IDs because of device_match_driver_override(), so
> > platform drivers that rely on the existence of a device's ACPI companion
> > object need to verify its presence.
> >
> > Accordingly, add a requisite ACPI_COMPANION() check against NULL to the
> > surfacepro3_button driver.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thanks!

> ...
>
> >       strscpy(acpi_device_name(device), SURFACE_BUTTON_DEVICE_NAME);
> > -     snprintf(button->phys, sizeof(button->phys), "%s/buttons", hid);
> > +     snprintf(button->phys, sizeof(button->phys), "%s/buttons",
> > +              acpi_device_hid(device));
>
> I'm wondering if some CIs will now generate a snprintf() warning
> (this was in the original code if the compiler can prove no truncation
>  of the string, though).

Then I can send another patch to make that warning go away ...