[PATCH] mfd: Add ACPI support to Kontron PLD driver

[PATCH] mfd: Add ACPI support to Kontron PLD driver

[Michael Brunner]

Recent Kontron COMe modules identify the PLD device using the hardware
id KEM0001 in the ACPI table.
This patch adds support for probing the device using the HID and also
retrieving the resources.

As this is not available for all products, the DMI based detection still
needs to be around for older systems. It is executed if no matching ACPI
HID is found during registering the platform driver or no specific
device id is forced.
If a device is detected using ACPI and no resource information is
available, the default io resource is used.

Forcing a device id with the force_device_id parameter and therefore
manually generating a platform device takes precedence over ACPI during
probing.

Signed-off-by: Michael Brunner <michael.brunner@kontron.com>
---
 drivers/mfd/kempld-core.c | 97 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 91 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/kempld-core.c b/drivers/mfd/kempld-core.c
index f48e21d8b97c..408cad1958d9 100644
--- a/drivers/mfd/kempld-core.c
+++ b/drivers/mfd/kempld-core.c
@@ -13,6 +13,7 @@
 #include <linux/dmi.h>
 #include <linux/io.h>
 #include <linux/delay.h>
+#include <linux/acpi.h>
 
 #define MAX_ID_LEN 4
 static char force_device_id[MAX_ID_LEN + 1] = "";
@@ -132,6 +133,7 @@ static const struct kempld_platform_data kempld_platform_data_generic = {
 };
 
 static struct platform_device *kempld_pdev;
+static bool kempld_acpi_mode;
 
 static int kempld_create_platform_device(const struct dmi_system_id *id)
 {
@@ -434,13 +436,87 @@ static int kempld_detect_device(struct kempld_device_data *pld)
 	return ret;
 }
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id kempld_acpi_table[] = {
+	{ "KEM0001", (kernel_ulong_t)&kempld_platform_data_generic },
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, kempld_acpi_table);
+
+static int kempld_get_acpi_data(struct platform_device *pdev)
+{
+	struct list_head resource_list;
+	struct resource *resources;
+	struct resource_entry *rentry;
+	struct device *dev = &pdev->dev;
+	struct acpi_device *acpi_dev = ACPI_COMPANION(dev);
+	const struct kempld_platform_data *pdata;
+	int ret;
+	int count;
+
+	pdata = acpi_device_get_match_data(dev);
+	ret = platform_device_add_data(pdev, pdata,
+				       sizeof(struct kempld_platform_data));
+	if (ret)
+		return ret;
+
+	INIT_LIST_HEAD(&resource_list);
+	ret = acpi_dev_get_resources(acpi_dev, &resource_list, NULL, NULL);
+	if (ret < 0)
+		goto out;
+
+	count = ret;
+
+	if (count > 0) {
+		resources = devm_kcalloc(&acpi_dev->dev, count,
+					  sizeof(struct resource), GFP_KERNEL);
+		if (!resources) {
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		count = 0;
+		list_for_each_entry(rentry, &resource_list, node) {
+			memcpy(&resources[count], rentry->res,
+			       sizeof(*resources));
+			count++;
+		}
+		ret = platform_device_add_resources(pdev, resources, count);
+		if (ret)
+			goto out;
+	} else
+		ret = platform_device_add_resources(pdev, pdata->ioresource, 1);
+
+out:
+	acpi_dev_free_resource_list(&resource_list);
+
+	return ret;
+}
+#else
+static int kempld_get_acpi_data(struct platform_device *pdev)
+{
+	return -ENODEV;
+}
+#endif /* CONFIG_ACPI */
+
 static int kempld_probe(struct platform_device *pdev)
 {
-	const struct kempld_platform_data *pdata =
-		dev_get_platdata(&pdev->dev);
+	const struct kempld_platform_data *pdata;
 	struct device *dev = &pdev->dev;
 	struct kempld_device_data *pld;
 	struct resource *ioport;
+	int ret;
+
+	if (kempld_pdev == NULL) {
+		ret = kempld_get_acpi_data(pdev);
+		if (ret < 0)
+			return ret;
+		kempld_acpi_mode = true;
+	} else if (kempld_pdev != pdev) {
+		dev_notice(dev, "platform device exists - not using ACPI\n");
+		return -ENODEV;
+	}
+	pdata = dev_get_platdata(dev);
 
 	pld = devm_kzalloc(dev, sizeof(*pld), GFP_KERNEL);
 	if (!pld)
@@ -482,6 +558,7 @@ static int kempld_remove(struct platform_device *pdev)
 static struct platform_driver kempld_driver = {
 	.driver		= {
 		.name	= "kempld",
+		.acpi_match_table = ACPI_PTR(kempld_acpi_table),
 	},
 	.probe		= kempld_probe,
 	.remove		= kempld_remove,
@@ -800,6 +877,7 @@ MODULE_DEVICE_TABLE(dmi, kempld_dmi_table);
 static int __init kempld_init(void)
 {
 	const struct dmi_system_id *id;
+	int ret;
 
 	if (force_device_id[0]) {
 		for (id = kempld_dmi_table;
@@ -809,12 +887,19 @@ static int __init kempld_init(void)
 					break;
 		if (id->matches[0].slot == DMI_NONE)
 			return -ENODEV;
-	} else {
-		if (!dmi_check_system(kempld_dmi_table))
-			return -ENODEV;
 	}
 
-	return platform_driver_register(&kempld_driver);
+	ret = platform_driver_register(&kempld_driver);
+	if (ret)
+		return ret;
+
+	if (!kempld_pdev && !kempld_acpi_mode)
+		if (!dmi_check_system(kempld_dmi_table)) {
+			platform_driver_unregister(&kempld_driver);
+			return -ENODEV;
+		}
+
+	return 0;
 }
 
 static void __exit kempld_exit(void)
-- 
2.25.1

Re: [PATCH] mfd: Add ACPI support to Kontron PLD driver

[Lee Jones]

On Wed, 12 Aug 2020, Michael Brunner wrote:

> Recent Kontron COMe modules identify the PLD device using the hardware
> id KEM0001 in the ACPI table.
> This patch adds support for probing the device using the HID and also
> retrieving the resources.
> 
> As this is not available for all products, the DMI based detection still
> needs to be around for older systems. It is executed if no matching ACPI
> HID is found during registering the platform driver or no specific
> device id is forced.
> If a device is detected using ACPI and no resource information is
> available, the default io resource is used.
> 
> Forcing a device id with the force_device_id parameter and therefore
> manually generating a platform device takes precedence over ACPI during
> probing.
> 
> Signed-off-by: Michael Brunner <michael.brunner@kontron.com>
> ---
>  drivers/mfd/kempld-core.c | 97 ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 91 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mfd/kempld-core.c b/drivers/mfd/kempld-core.c
> index f48e21d8b97c..408cad1958d9 100644
> --- a/drivers/mfd/kempld-core.c
> +++ b/drivers/mfd/kempld-core.c
> @@ -13,6 +13,7 @@
>  #include <linux/dmi.h>
>  #include <linux/io.h>
>  #include <linux/delay.h>
> +#include <linux/acpi.h>
>  
>  #define MAX_ID_LEN 4
>  static char force_device_id[MAX_ID_LEN + 1] = "";
> @@ -132,6 +133,7 @@ static const struct kempld_platform_data kempld_platform_data_generic = {
>  };
>  
>  static struct platform_device *kempld_pdev;
> +static bool kempld_acpi_mode;
>  
>  static int kempld_create_platform_device(const struct dmi_system_id *id)
>  {
> @@ -434,13 +436,87 @@ static int kempld_detect_device(struct kempld_device_data *pld)
>  	return ret;
>  }
>  
> +#ifdef CONFIG_ACPI

Not keen on #ifdefery if at all avoidable.

Can you use if (IS_ENABLED(CONFIG_ACPI)) at the call-site instead?

The compiler should take care of the rest, no?

> +static const struct acpi_device_id kempld_acpi_table[] = {
> +	{ "KEM0001", (kernel_ulong_t)&kempld_platform_data_generic },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(acpi, kempld_acpi_table);

I'd prefer if this was moved down to just above where it's used
i.e. where we usually place the of_device_id tables.

> +static int kempld_get_acpi_data(struct platform_device *pdev)
> +{
> +	struct list_head resource_list;
> +	struct resource *resources;
> +	struct resource_entry *rentry;
> +	struct device *dev = &pdev->dev;
> +	struct acpi_device *acpi_dev = ACPI_COMPANION(dev);
> +	const struct kempld_platform_data *pdata;
> +	int ret;
> +	int count;
> +
> +	pdata = acpi_device_get_match_data(dev);
> +	ret = platform_device_add_data(pdev, pdata,
> +				       sizeof(struct kempld_platform_data));
> +	if (ret)
> +		return ret;
> +
> +	INIT_LIST_HEAD(&resource_list);
> +	ret = acpi_dev_get_resources(acpi_dev, &resource_list, NULL, NULL);
> +	if (ret < 0)
> +		goto out;
> +
> +	count = ret;

  	if (count == 0) {
		ret = platform_device_add_resources(pdev, pdata->ioresource, 1);
		goto out;
	}

Then drop the next check and pull the indented code back:

> +	if (count > 0) {
> +		resources = devm_kcalloc(&acpi_dev->dev, count,
> +					  sizeof(struct resource), GFP_KERNEL);

sizeof(*resources) is preferred.

> +		if (!resources) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
> +
> +		count = 0;
> +		list_for_each_entry(rentry, &resource_list, node) {
> +			memcpy(&resources[count], rentry->res,
> +			       sizeof(*resources));
> +			count++;
> +		}
> +
> +		ret = platform_device_add_resources(pdev, resources, count);
> +		if (ret)
> +			goto out;
> +	} else
> +		ret = platform_device_add_resources(pdev, pdata->ioresource, 1);
> +
> +out:
> +	acpi_dev_free_resource_list(&resource_list);
> +
> +	return ret;
> +}
> +#else
> +static int kempld_get_acpi_data(struct platform_device *pdev)
> +{
> +	return -ENODEV;
> +}
> +#endif /* CONFIG_ACPI */
> +
>  static int kempld_probe(struct platform_device *pdev)
>  {
> -	const struct kempld_platform_data *pdata =
> -		dev_get_platdata(&pdev->dev);
> +	const struct kempld_platform_data *pdata;
>  	struct device *dev = &pdev->dev;
>  	struct kempld_device_data *pld;
>  	struct resource *ioport;
> +	int ret;
> +
> +	if (kempld_pdev == NULL) {

Comment please.  What does !kempld_pdev actually imply?

> +		ret = kempld_get_acpi_data(pdev);
> +		if (ret < 0)
> +			return ret;

Is 'ret > 0' valid?

If not, then just 'if (ret)'.

> +		kempld_acpi_mode = true;
> +	} else if (kempld_pdev != pdev) {

> +		dev_notice(dev, "platform device exists - not using ACPI\n");

Why dev_notice() and not dev_err()?

Is that what 'kempld_pdev != pdev' means?

Could you explain this to me in more depth please?

> +		return -ENODEV;
> +	}
> +	pdata = dev_get_platdata(dev);
>  
>  	pld = devm_kzalloc(dev, sizeof(*pld), GFP_KERNEL);
>  	if (!pld)
> @@ -482,6 +558,7 @@ static int kempld_remove(struct platform_device *pdev)
>  static struct platform_driver kempld_driver = {
>  	.driver		= {
>  		.name	= "kempld",
> +		.acpi_match_table = ACPI_PTR(kempld_acpi_table),
>  	},
>  	.probe		= kempld_probe,
>  	.remove		= kempld_remove,
> @@ -800,6 +877,7 @@ MODULE_DEVICE_TABLE(dmi, kempld_dmi_table);
>  static int __init kempld_init(void)
>  {
>  	const struct dmi_system_id *id;
> +	int ret;
>  
>  	if (force_device_id[0]) {
>  		for (id = kempld_dmi_table;
> @@ -809,12 +887,19 @@ static int __init kempld_init(void)
>  					break;
>  		if (id->matches[0].slot == DMI_NONE)
>  			return -ENODEV;
> -	} else {
> -		if (!dmi_check_system(kempld_dmi_table))
> -			return -ENODEV;
>  	}
>  
> -	return platform_driver_register(&kempld_driver);
> +	ret = platform_driver_register(&kempld_driver);
> +	if (ret)
> +		return ret;

Is it guaranteed that the child device has probed at this point?

> +	if (!kempld_pdev && !kempld_acpi_mode)

Again, comment please.  What has gone on to get to this point?

> +		if (!dmi_check_system(kempld_dmi_table)) {
> +			platform_driver_unregister(&kempld_driver);
> +			return -ENODEV;
> +		}
> +
> +	return 0;
>  }
>  
>  static void __exit kempld_exit(void)

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] mfd: Add ACPI support to Kontron PLD driver

[Michael Brunner]

Thank you for taking the time to review the patch!
Additional comments below, patch v2 will follow.

On Wed, 2020-08-19 at 11:15 +0100, Lee Jones wrote:
> On Wed, 12 Aug 2020, Michael Brunner wrote:
... 
> > +#ifdef CONFIG_ACPI
> 
> Not keen on #ifdefery if at all avoidable.
> 
> Can you use if (IS_ENABLED(CONFIG_ACPI)) at the call-site instead?
> 
> The compiler should take care of the rest, no?

Unfortunately acpi_dev_get_resources is not defined when compiling with
CONFIG_ACPI disabled, therefore #ifdef seems to be the only choice.

> > +static const struct acpi_device_id kempld_acpi_table[] = {
> > +	{ "KEM0001", (kernel_ulong_t)&kempld_platform_data_generic },
> > +	{}
> > +};
> > +MODULE_DEVICE_TABLE(acpi, kempld_acpi_table);
>
> I'd prefer if this was moved down to just above where it's used
> i.e. where we usually place the of_device_id tables.

No problem, will update this.

...
> > +	INIT_LIST_HEAD(&resource_list);
> > +	ret = acpi_dev_get_resources(acpi_dev, &resource_list, NULL,
> > NULL);
> > +	if (ret < 0)
> > +		goto out;
> > +
> > +	count = ret;
> 
>   	if (count == 0) {
> 		ret = platform_device_add_resources(pdev, pdata-
> >ioresource, 1);
> 		goto out;
> 	}
> 
> Then drop the next check and pull the indented code back:

Agreed, looks better.

> > +	if (count > 0) {
> > +		resources = devm_kcalloc(&acpi_dev->dev, count,
> > +					  sizeof(struct resource),
> > GFP_KERNEL);
> 
> sizeof(*resources) is preferred.

Ok

...
> >  static int kempld_probe(struct platform_device *pdev)
> >  {
> > -	const struct kempld_platform_data *pdata =
> > -		dev_get_platdata(&pdev->dev);
> > +	const struct kempld_platform_data *pdata;
> >  	struct device *dev = &pdev->dev;
> >  	struct kempld_device_data *pld;
> >  	struct resource *ioport;
> > +	int ret;
> > +
> > +	if (kempld_pdev == NULL) {
> 
> Comment please.  What does !kempld_pdev actually imply?

If kempld_pdev is not defined, this means no platform device has been
created using the DMI id table and we are currently probing the ACPI
based platform device.
Will add a comment to the code.

> > +		ret = kempld_get_acpi_data(pdev);
> > +		if (ret < 0)
> > +			return ret;
> 
> Is 'ret > 0' valid?
> 
> If not, then just 'if (ret)'.

Ok

> > +		kempld_acpi_mode = true;
> > +	} else if (kempld_pdev != pdev) {
> > +		dev_notice(dev, "platform device exists - not using
> > ACPI\n");
> 
> Why dev_notice() and not dev_err()?
> 
> Is that what 'kempld_pdev != pdev' means?
> 
> Could you explain this to me in more depth please?

kempld_pdev is the DMI based platform device created in kempld_init
(through force_device_id or dmi_check_system). pdev is the one passed
by the probe function. (kempdl_pdev != pdev) means pdev is the device
created using the ACPI table. As there is only one physical device and
the DMI based version should have priority, the probe is aborted at
this point. As it is not an error condition, only a notice is created
to indicate that ACPI is not used for probing the device.

> > ...
> > @@ -809,12 +887,19 @@ static int __init kempld_init(void)
> >  					break;
> >  		if (id->matches[0].slot == DMI_NONE)
> >  			return -ENODEV;
> > -	} else {
> > -		if (!dmi_check_system(kempld_dmi_table))
> > -			return -ENODEV;
> >  	}
> >  
> > -	return platform_driver_register(&kempld_driver);
> > +	ret = platform_driver_register(&kempld_driver);
> > +	if (ret)
> > +		return ret;
>
> Is it guaranteed that the child device has probed at this point?

To my understanding, with synchronous probing it should. According to
the definition of the probe_type enum this is still the default for all
drivers.
I guess it would make sense to enforce this for the case the default is
changed in the future.
Added this to v2:
	.probe_type = PROBE_FORCE_SYNCHRONOUS,

Only exception, to my knowledge, is if drivers_autoprobe is disabled
for platform drivers during kempld_init. As DMI has priority, the
driver will later only allow to manually bind the DMI platform device,
if ACPI and DMI are both supported on a platform.

> > +	if (!kempld_pdev && !kempld_acpi_mode)
> 
> Again, comment please.  What has gone on to get to this point?

At this point it is checked if the kempld_pdev has been already created
(force_device_id parameter) and if the ACPI based probe has been
already successfull. If not, the DMI table is searched for a matching
device ID. Doing this at this point should prevent unnecessarily
checking the DMI table.

> > +		if (!dmi_check_system(kempld_dmi_table)) {
> > +			platform_driver_unregister(&kempld_driver);
> > +			return -ENODEV;
> > +		}

While writing the above comments I noticed that unregistering the
driver at this point prevents manually binding the driver to a device
if drivers_autoprobe is set to 0. This only made sense as long as DMI
based detection was the only method supported. Therefore I removed it
in v2.

  Michael

[PATCH v2] mfd: Add ACPI support to Kontron PLD driver

[Michael Brunner]

Recent Kontron COMe modules identify the PLD device using the hardware
id KEM0001 in the ACPI table.
This patch adds support for probing the device using the HID and also
retrieving the resources.

As this is not available for all products, the DMI based detection still
needs to be around for older systems. It is executed if no matching ACPI
HID is found during registering the platform driver or no specific
device id is forced.
If a device is detected using ACPI and no resource information is
available, the default io resource is used.

Forcing a device id with the force_device_id parameter and therefore
manually generating a platform device takes precedence over ACPI during
probing.

v2: - Implemented code changes suggested by Lee Jones
    - Added comments explaining some critical code
    - Driver is no longer unregistered if probing is skipped during
      platform_driver_register and device is not found in DMI table
    - Set probe type to PROBE_FORCE_SYNCHRONOUS to make clear this is
      the expected behaviour for this code to work as expected

Signed-off-by: Michael Brunner <michael.brunner@kontron.com>
---
 drivers/mfd/kempld-core.c | 115 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 109 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/kempld-core.c b/drivers/mfd/kempld-core.c
index f48e21d8b97c..0bb6d621112c 100644
--- a/drivers/mfd/kempld-core.c
+++ b/drivers/mfd/kempld-core.c
@@ -13,6 +13,7 @@
 #include <linux/dmi.h>
 #include <linux/io.h>
 #include <linux/delay.h>
+#include <linux/acpi.h>
 
 #define MAX_ID_LEN 4
 static char force_device_id[MAX_ID_LEN + 1] = "";
@@ -132,6 +133,7 @@ static const struct kempld_platform_data kempld_platform_data_generic = {
 };
 
 static struct platform_device *kempld_pdev;
+static bool kempld_acpi_mode;
 
 static int kempld_create_platform_device(const struct dmi_system_id *id)
 {
@@ -434,13 +436,93 @@ static int kempld_detect_device(struct kempld_device_data *pld)
 	return ret;
 }
 
+#ifdef CONFIG_ACPI
+static int kempld_get_acpi_data(struct platform_device *pdev)
+{
+	struct list_head resource_list;
+	struct resource *resources;
+	struct resource_entry *rentry;
+	struct device *dev = &pdev->dev;
+	struct acpi_device *acpi_dev = ACPI_COMPANION(dev);
+	const struct kempld_platform_data *pdata;
+	int ret;
+	int count;
+
+	pdata = acpi_device_get_match_data(dev);
+	ret = platform_device_add_data(pdev, pdata,
+				       sizeof(struct kempld_platform_data));
+	if (ret)
+		return ret;
+
+	INIT_LIST_HEAD(&resource_list);
+	ret = acpi_dev_get_resources(acpi_dev, &resource_list, NULL, NULL);
+	if (ret < 0)
+		goto out;
+
+	count = ret;
+
+	if (count == 0) {
+		ret = platform_device_add_resources(pdev, pdata->ioresource, 1);
+		goto out;
+	}
+
+	resources = devm_kcalloc(&acpi_dev->dev, count, sizeof(*resources),
+				 GFP_KERNEL);
+	if (!resources) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	count = 0;
+	list_for_each_entry(rentry, &resource_list, node) {
+		memcpy(&resources[count], rentry->res,
+		       sizeof(*resources));
+		count++;
+	}
+	ret = platform_device_add_resources(pdev, resources, count);
+
+out:
+	acpi_dev_free_resource_list(&resource_list);
+
+	return ret;
+}
+#else
+static int kempld_get_acpi_data(struct platform_device *pdev)
+{
+	return -ENODEV;
+}
+#endif /* CONFIG_ACPI */
+
 static int kempld_probe(struct platform_device *pdev)
 {
-	const struct kempld_platform_data *pdata =
-		dev_get_platdata(&pdev->dev);
+	const struct kempld_platform_data *pdata;
 	struct device *dev = &pdev->dev;
 	struct kempld_device_data *pld;
 	struct resource *ioport;
+	int ret;
+
+	if (kempld_pdev == NULL) {
+		/*
+		 * No kempld_pdev device has been registered in kempld_init,
+		 * so we seem to be probing an ACPI platform device.
+		 */
+		ret = kempld_get_acpi_data(pdev);
+		if (ret)
+			return ret;
+
+		kempld_acpi_mode = true;
+	} else if (kempld_pdev != pdev) {
+		/*
+		 * The platform device we are probing is not the one we
+		 * registered in kempld_init using the DMI table, so this one
+		 * comes from ACPI.
+		 * As we can only probe one - abort here and use the DMI
+		 * based one instead.
+		 */
+		dev_notice(dev, "platform device exists - not using ACPI\n");
+		return -ENODEV;
+	}
+	pdata = dev_get_platdata(dev);
 
 	pld = devm_kzalloc(dev, sizeof(*pld), GFP_KERNEL);
 	if (!pld)
@@ -479,9 +561,17 @@ static int kempld_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct acpi_device_id kempld_acpi_table[] = {
+	{ "KEM0001", (kernel_ulong_t)&kempld_platform_data_generic },
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, kempld_acpi_table);
+
 static struct platform_driver kempld_driver = {
 	.driver		= {
 		.name	= "kempld",
+		.acpi_match_table = ACPI_PTR(kempld_acpi_table),
+		.probe_type = PROBE_FORCE_SYNCHRONOUS,
 	},
 	.probe		= kempld_probe,
 	.remove		= kempld_remove,
@@ -800,6 +890,7 @@ MODULE_DEVICE_TABLE(dmi, kempld_dmi_table);
 static int __init kempld_init(void)
 {
 	const struct dmi_system_id *id;
+	int ret;
 
 	if (force_device_id[0]) {
 		for (id = kempld_dmi_table;
@@ -809,12 +900,24 @@ static int __init kempld_init(void)
 					break;
 		if (id->matches[0].slot == DMI_NONE)
 			return -ENODEV;
-	} else {
-		if (!dmi_check_system(kempld_dmi_table))
-			return -ENODEV;
 	}
 
-	return platform_driver_register(&kempld_driver);
+	ret = platform_driver_register(&kempld_driver);
+	if (ret)
+		return ret;
+
+	/*
+	 * With synchronous probing the device should already be probed now.
+	 * If no device id is forced and also no ACPI definition for the
+	 * device was found, scan DMI table as fallback.
+	 *
+	 * If drivers_autoprobing is disabled and the device is found here,
+	 * only that device can be bound manually later.
+	 */
+	if (!kempld_pdev && !kempld_acpi_mode)
+		dmi_check_system(kempld_dmi_table);
+
+	return 0;
 }
 
 static void __exit kempld_exit(void)
-- 
2.25.1

Re: [PATCH v2] mfd: Add ACPI support to Kontron PLD driver

[Lee Jones]

On Thu, 20 Aug 2020, Michael Brunner wrote:

> Recent Kontron COMe modules identify the PLD device using the hardware
> id KEM0001 in the ACPI table.
> This patch adds support for probing the device using the HID and also
> retrieving the resources.
> 
> As this is not available for all products, the DMI based detection still
> needs to be around for older systems. It is executed if no matching ACPI
> HID is found during registering the platform driver or no specific
> device id is forced.
> If a device is detected using ACPI and no resource information is
> available, the default io resource is used.
> 
> Forcing a device id with the force_device_id parameter and therefore
> manually generating a platform device takes precedence over ACPI during
> probing.
> 
> v2: - Implemented code changes suggested by Lee Jones
>     - Added comments explaining some critical code
>     - Driver is no longer unregistered if probing is skipped during
>       platform_driver_register and device is not found in DMI table
>     - Set probe type to PROBE_FORCE_SYNCHRONOUS to make clear this is
>       the expected behaviour for this code to work as expected
> 
> Signed-off-by: Michael Brunner <michael.brunner@kontron.com>
> ---
>  drivers/mfd/kempld-core.c | 115 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 109 insertions(+), 6 deletions(-)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog