ACPI core in conjunction with platform driver core provides
an infrastructure to enumerate ACPI devices. Use it in order
to remove a lot of boilerplate code.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/i2c/busses/i2c-scmi.c | 48 ++++++++++++++---------------------
1 file changed, 19 insertions(+), 29 deletions(-)
diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
index 6746aa46d96c..0239e134b90f 100644
--- a/drivers/i2c/busses/i2c-scmi.c
+++ b/drivers/i2c/busses/i2c-scmi.c
@@ -6,15 +6,13 @@
*/
#include <linux/module.h>
+#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/kernel.h>
#include <linux/stddef.h>
#include <linux/i2c.h>
#include <linux/acpi.h>
-#define ACPI_SMBUS_HC_CLASS "smbus"
-#define ACPI_SMBUS_HC_DEVICE_NAME "cmi"
-
/* SMBUS HID definition as supported by Microsoft Windows */
#define ACPI_SMBUS_MS_HID "SMB0001"
@@ -30,7 +28,7 @@ struct acpi_smbus_cmi {
u8 cap_info:1;
u8 cap_read:1;
u8 cap_write:1;
- struct smbus_methods_t *methods;
+ const struct smbus_methods_t *methods;
};
static const struct smbus_methods_t smbus_methods = {
@@ -358,29 +356,25 @@ static acpi_status acpi_smbus_cmi_query_methods(acpi_handle handle, u32 level,
return AE_OK;
}
-static int acpi_smbus_cmi_add(struct acpi_device *device)
+static int smbus_cmi_probe(struct platform_device *device)
{
+ struct device *dev = &device->dev;
struct acpi_smbus_cmi *smbus_cmi;
- const struct acpi_device_id *id;
int ret;
smbus_cmi = kzalloc(sizeof(struct acpi_smbus_cmi), GFP_KERNEL);
if (!smbus_cmi)
return -ENOMEM;
- smbus_cmi->handle = device->handle;
- strcpy(acpi_device_name(device), ACPI_SMBUS_HC_DEVICE_NAME);
- strcpy(acpi_device_class(device), ACPI_SMBUS_HC_CLASS);
- device->driver_data = smbus_cmi;
+ smbus_cmi->handle = ACPI_HANDLE(dev);
+ smbus_cmi->methods = device_get_match_data(dev);
+
+ platform_set_drvdata(device, smbus_cmi);
+
smbus_cmi->cap_info = 0;
smbus_cmi->cap_read = 0;
smbus_cmi->cap_write = 0;
- for (id = acpi_smbus_cmi_ids; id->id[0]; id++)
- if (!strcmp(id->id, acpi_device_hid(device)))
- smbus_cmi->methods =
- (struct smbus_methods_t *) id->driver_data;
-
acpi_walk_namespace(ACPI_TYPE_METHOD, smbus_cmi->handle, 1,
acpi_smbus_cmi_query_methods, NULL, smbus_cmi, NULL);
@@ -390,8 +384,7 @@ static int acpi_smbus_cmi_add(struct acpi_device *device)
}
snprintf(smbus_cmi->adapter.name, sizeof(smbus_cmi->adapter.name),
- "SMBus CMI adapter %s",
- acpi_device_name(device));
+ "SMBus CMI adapter %s", dev_name(dev));
smbus_cmi->adapter.owner = THIS_MODULE;
smbus_cmi->adapter.algo = &acpi_smbus_cmi_algorithm;
smbus_cmi->adapter.algo_data = smbus_cmi;
@@ -408,31 +401,28 @@ static int acpi_smbus_cmi_add(struct acpi_device *device)
err:
kfree(smbus_cmi);
- device->driver_data = NULL;
return ret;
}
-static int acpi_smbus_cmi_remove(struct acpi_device *device)
+static int smbus_cmi_remove(struct platform_device *device)
{
- struct acpi_smbus_cmi *smbus_cmi = acpi_driver_data(device);
+ struct acpi_smbus_cmi *smbus_cmi = platform_get_drvdata(device);
i2c_del_adapter(&smbus_cmi->adapter);
kfree(smbus_cmi);
- device->driver_data = NULL;
return 0;
}
-static struct acpi_driver acpi_smbus_cmi_driver = {
- .name = ACPI_SMBUS_HC_DEVICE_NAME,
- .class = ACPI_SMBUS_HC_CLASS,
- .ids = acpi_smbus_cmi_ids,
- .ops = {
- .add = acpi_smbus_cmi_add,
- .remove = acpi_smbus_cmi_remove,
+static struct platform_driver smbus_cmi_driver = {
+ .probe = smbus_cmi_probe,
+ .remove = smbus_cmi_remove,
+ .driver = {
+ .name = "smbus_cmi",
+ .acpi_match_table = acpi_smbus_cmi_ids,
},
};
-module_acpi_driver(acpi_smbus_cmi_driver);
+module_platform_driver(smbus_cmi_driver);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Crane Cai <[email protected]>");
--
2.35.1
On Tue, Sep 06, 2022 at 06:55:07PM +0300, Andy Shevchenko wrote:
> ACPI core in conjunction with platform driver core provides
> an infrastructure to enumerate ACPI devices. Use it in order
> to remove a lot of boilerplate code.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
Josef, do you have resources to test this patch before I apply it?
On Thu, Sep 08, 2022 at 09:48:29AM +0200, Josef Johansson wrote:
> On 9/8/22 08:07, Josef Johansson wrote:
> > On 9/7/22 21:47, Wolfram Sang wrote:
> > > On Tue, Sep 06, 2022 at 06:55:07PM +0300, Andy Shevchenko wrote:
> > > > ACPI core in conjunction with platform driver core provides
> > > > an infrastructure to enumerate ACPI devices. Use it in order
> > > > to remove a lot of boilerplate code.
> > > >
> > > > Signed-off-by: Andy Shevchenko <[email protected]>
> > > Josef, do you have resources to test this patch before I apply it?
> > >
> > Yes, I'll make that happen today.
> Hi,
>
> I compiled with linux-6.0.0-rc4 with your patch on top.
>
> Have been running flawless so far. Boot showed no problems.
>
> Thanks!
Thank you!
--
With Best Regards,
Andy Shevchenko
On Tue, Sep 06, 2022 at 06:55:07PM +0300, Andy Shevchenko wrote:
> ACPI core in conjunction with platform driver core provides
> an infrastructure to enumerate ACPI devices. Use it in order
> to remove a lot of boilerplate code.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
Doesn't apply anymore after the revert. Could you respin this?
> > Signed-off-by: Andy Shevchenko <[email protected]>
>
> Doesn't apply anymore after the revert. Could you respin this?
Sorry, my fault. It was the other way around. I missed the revert in
the for-mergewindow branch.
On Thu, 2022-09-08 at 13:02 +0300, Andy Shevchenko wrote:
> On Thu, Sep 08, 2022 at 09:48:29AM +0200, Josef Johansson wrote:
> > On 9/8/22 08:07, Josef Johansson wrote:
> > > On 9/7/22 21:47, Wolfram Sang wrote:
> > > > On Tue, Sep 06, 2022 at 06:55:07PM +0300, Andy Shevchenko
> > > > wrote:
> > > > > ACPI core in conjunction with platform driver core provides
> > > > > an infrastructure to enumerate ACPI devices. Use it in order
> > > > > to remove a lot of boilerplate code.
> > > > >
> > > > > Signed-off-by: Andy Shevchenko <
> > > > > [email protected]>
> > > > Josef, do you have resources to test this patch before I apply
> > > > it?
> > > >
> > > Yes, I'll make that happen today.
> > Hi,
> >
> > I compiled with linux-6.0.0-rc4 with your patch on top.
> >
> > Have been running flawless so far. Boot showed no problems.
> >
> > Thanks!
>
> Thank you!
>
Hi,
We just noticed that this change prevents the usage of the i2c-scmi
driver on basically all Kontron COMe based boards.
The reason is the patch "ACPI / platform: Add SMB0001 HID to
forbidden_id_list" submitted in November 2018 by Hans de Goede. The
patch blacklists the SMB0001 HID that is also used by the COMe boards.
This was due to issues with HP AMD based laptops according to the
commit message.
Ironically the commit message there states that it is OK to blacklist
the HID as the device directly binds to the acpi_bus and therefore the
platform_device is not needed anyway. This changed with this patch.
As this affects all systems using this HID, applying a patch that
whitelists specific boards again in the acpi-platform driver doesn't
seem to be a good solution to me.
Therefore I would request to remove this patch again, unless someone
has a better idea.
Thanks,
Michael Brunner
On Mon, May 15, 2023 at 07:51:55AM +0000, Michael Brunner wrote:
> On Thu, 2022-09-08 at 13:02 +0300, Andy Shevchenko wrote:
> > On Thu, Sep 08, 2022 at 09:48:29AM +0200, Josef Johansson wrote:
> > > On 9/8/22 08:07, Josef Johansson wrote:
> > > > On 9/7/22 21:47, Wolfram Sang wrote:
> > > > > On Tue, Sep 06, 2022 at 06:55:07PM +0300, Andy Shevchenko
> > > > > wrote:
First of all, sorry for so-o lo-o-ong delay. Too many emails in a backlog.
...
> > > I compiled with linux-6.0.0-rc4 with your patch on top.
> > >
> > > Have been running flawless so far. Boot showed no problems.
> We just noticed that this change prevents the usage of the i2c-scmi
> driver on basically all Kontron COMe based boards.
Does this device have resources defined in DSDT? Can you show all variants?
> The reason is the patch "ACPI / platform: Add SMB0001 HID to
> forbidden_id_list" submitted in November 2018 by Hans de Goede.
> The
> patch blacklists the SMB0001 HID that is also used by the COMe boards.
> This was due to issues with HP AMD based laptops according to the
> commit message.
> Ironically the commit message there states that it is OK to blacklist
> the HID as the device directly binds to the acpi_bus and therefore the
> platform_device is not needed anyway. This changed with this patch.
>
> As this affects all systems using this HID, applying a patch that
> whitelists specific boards again in the acpi-platform driver doesn't
> seem to be a good solution to me.
> Therefore I would request to remove this patch again, unless someone
> has a better idea.
I have a better idea if the DSDT has no resources. See the Q above.
--
With Best Regards,
Andy Shevchenko
On Thu, 2023-06-15 at 18:50 +0300, [email protected]
wrote:
> On Mon, May 15, 2023 at 07:51:55AM +0000, Michael Brunner wrote:
> > On Thu, 2022-09-08 at 13:02 +0300, Andy Shevchenko wrote:
> > > On Thu, Sep 08, 2022 at 09:48:29AM +0200, Josef Johansson wrote:
> > > > On 9/8/22 08:07, Josef Johansson wrote:
> > > > > On 9/7/22 21:47, Wolfram Sang wrote:
> > > > > > On Tue, Sep 06, 2022 at 06:55:07PM +0300, Andy Shevchenko
> > > > > > wrote:
>
> First of all, sorry for so-o lo-o-ong delay. Too many emails in a
> backlog.
>
> ...
No problem, thanks a lot for taking the time.
> > > > I compiled with linux-6.0.0-rc4 with your patch on top.
> > > >
> > > > Have been running flawless so far. Boot showed no problems.
> > We just noticed that this change prevents the usage of the i2c-scmi
> > driver on basically all Kontron COMe based boards.
>
> Does this device have resources defined in DSDT? Can you show all
> variants?
According to our BIOS department there are no resources defined in any
variant. The device uses operation regions only.
> > The reason is the patch "ACPI / platform: Add SMB0001 HID to
> > forbidden_id_list" submitted in November 2018 by Hans de Goede.
> > The
> > patch blacklists the SMB0001 HID that is also used by the COMe
> > boards.
> > This was due to issues with HP AMD based laptops according to the
> > commit message.
> > Ironically the commit message there states that it is OK to
> > blacklist
> > the HID as the device directly binds to the acpi_bus and therefore
> > the
> > platform_device is not needed anyway. This changed with this patch.
> >
> > As this affects all systems using this HID, applying a patch that
> > whitelists specific boards again in the acpi-platform driver
> > doesn't
> > seem to be a good solution to me.
> > Therefore I would request to remove this patch again, unless
> > someone
> > has a better idea.
>
> I have a better idea if the DSDT has no resources. See the Q above.
Sounds good for me. Please let me know if you need additional
information or if we should test something.
Best regards,
Michael Brunner