[PATCH 1/2] mfd: intel_soc_pmic_chtwc: Remove unneeded I2C device ID table

[PATCH 1/2] mfd: intel_soc_pmic_chtwc: Remove unneeded I2C device ID table

[Javier Martinez Canillas]

The driver has an empty I2C device table as a workaround for a previous
bug in the I2C core that was returning -ENODEV in i2c_device_probe() if
the driver didn't have an I2C device ID table. Even when is for an ACPI
only device and so the driver shouldn't need an I2C ID table in theory.

But now this issue has been fixed by commit c64ffff7a9d1 ("i2c: core:
Allow empty id_table in ACPI case as well"), so the empty I2C device
ID table can be removed.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 drivers/mfd/intel_soc_pmic_chtwc.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_chtwc.c b/drivers/mfd/intel_soc_pmic_chtwc.c
index b35da01d5bcf..ca01ecd1e546 100644
--- a/drivers/mfd/intel_soc_pmic_chtwc.c
+++ b/drivers/mfd/intel_soc_pmic_chtwc.c
@@ -208,10 +208,6 @@ static int __maybe_unused cht_wc_resume(struct device *dev)
 }
 static SIMPLE_DEV_PM_OPS(cht_wc_pm_ops, cht_wc_suspend, cht_wc_resume);
 
-static const struct i2c_device_id cht_wc_i2c_id[] = {
-	{ }
-};
-
 static const struct acpi_device_id cht_wc_acpi_ids[] = {
 	{ "INT34D3", },
 	{ }
@@ -225,6 +221,5 @@ static struct i2c_driver cht_wc_driver = {
 	},
 	.probe_new = cht_wc_probe,
 	.shutdown = cht_wc_shutdown,
-	.id_table = cht_wc_i2c_id,
 };
 builtin_i2c_driver(cht_wc_driver);
-- 
2.13.3

[PATCH 2/2] mfd: intel_soc_pmic_chtwc: Fix module autoload

[Javier Martinez Canillas]

The driver has a tristate Kconfig symbol so it can be built as a module,
but it doesn't export the device aliases in the module. So if the driver
is built as module, autoload won't work since udev/kmod won't be able to
match the registered ACPI device with its corresponding driver module.

Use the MODULE_DEVICE_TABLE() macro to export the ACPI device as alias.

Before this patch:

$ modinfo drivers/mfd/intel_soc_pmic_chtwc.ko | grep alias
$

After this patch

$ modinfo drivers/mfd/intel_soc_pmic_chtwc.ko | grep alias
alias:          acpi*:INT34D3:*

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

---

 drivers/mfd/intel_soc_pmic_chtwc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/intel_soc_pmic_chtwc.c b/drivers/mfd/intel_soc_pmic_chtwc.c
index ca01ecd1e546..b8b38d164981 100644
--- a/drivers/mfd/intel_soc_pmic_chtwc.c
+++ b/drivers/mfd/intel_soc_pmic_chtwc.c
@@ -212,6 +212,7 @@ static const struct acpi_device_id cht_wc_acpi_ids[] = {
 	{ "INT34D3", },
 	{ }
 };
+MODULE_DEVICE_TABLE(acpi, cht_wc_acpi_ids);
 
 static struct i2c_driver cht_wc_driver = {
 	.driver	= {
-- 
2.13.3

Re: [PATCH 1/2] mfd: intel_soc_pmic_chtwc: Remove unneeded I2C device ID table

[Hans de Goede]

Hi,

On 09-08-17 10:44, Javier Martinez Canillas wrote:
> The driver has an empty I2C device table as a workaround for a previous
> bug in the I2C core that was returning -ENODEV in i2c_device_probe() if
> the driver didn't have an I2C device ID table. Even when is for an ACPI
> only device and so the driver shouldn't need an I2C ID table in theory.
> 
> But now this issue has been fixed by commit c64ffff7a9d1 ("i2c: core:
> Allow empty id_table in ACPI case as well"), so the empty I2C device
> ID table can be removed.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

Thank you. Patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
> 
>   drivers/mfd/intel_soc_pmic_chtwc.c | 5 -----
>   1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/mfd/intel_soc_pmic_chtwc.c b/drivers/mfd/intel_soc_pmic_chtwc.c
> index b35da01d5bcf..ca01ecd1e546 100644
> --- a/drivers/mfd/intel_soc_pmic_chtwc.c
> +++ b/drivers/mfd/intel_soc_pmic_chtwc.c
> @@ -208,10 +208,6 @@ static int __maybe_unused cht_wc_resume(struct device *dev)
>   }
>   static SIMPLE_DEV_PM_OPS(cht_wc_pm_ops, cht_wc_suspend, cht_wc_resume);
>   
> -static const struct i2c_device_id cht_wc_i2c_id[] = {
> -	{ }
> -};
> -
>   static const struct acpi_device_id cht_wc_acpi_ids[] = {
>   	{ "INT34D3", },
>   	{ }
> @@ -225,6 +221,5 @@ static struct i2c_driver cht_wc_driver = {
>   	},
>   	.probe_new = cht_wc_probe,
>   	.shutdown = cht_wc_shutdown,
> -	.id_table = cht_wc_i2c_id,
>   };
>   builtin_i2c_driver(cht_wc_driver);
> 

Re: [PATCH 2/2] mfd: intel_soc_pmic_chtwc: Fix module autoload

[Hans de Goede]

Hi,

On 09-08-17 10:44, Javier Martinez Canillas wrote:
> The driver has a tristate Kconfig symbol so it can be built as a module,
> but it doesn't export the device aliases in the module. So if the driver
> is built as module, autoload won't work since udev/kmod won't be able to
> match the registered ACPI device with its corresponding driver module.
> 
> Use the MODULE_DEVICE_TABLE() macro to export the ACPI device as alias.
> 
> Before this patch:
> 
> $ modinfo drivers/mfd/intel_soc_pmic_chtwc.ko | grep alias
> $
> 
> After this patch
> 
> $ modinfo drivers/mfd/intel_soc_pmic_chtwc.ko | grep alias
> alias:          acpi*:INT34D3:*
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

As the Kconfig help text mentions this driver should always be
builtin if enabled. But I somehow made a mistake and it became a
tristate  in Kconfig instead of a bool. A patch to fix this
(make it a bool) is pending, so this patch is not necessary:

NACK.

Regards,

Hans



> 
> ---
> 
>   drivers/mfd/intel_soc_pmic_chtwc.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mfd/intel_soc_pmic_chtwc.c b/drivers/mfd/intel_soc_pmic_chtwc.c
> index ca01ecd1e546..b8b38d164981 100644
> --- a/drivers/mfd/intel_soc_pmic_chtwc.c
> +++ b/drivers/mfd/intel_soc_pmic_chtwc.c
> @@ -212,6 +212,7 @@ static const struct acpi_device_id cht_wc_acpi_ids[] = {
>   	{ "INT34D3", },
>   	{ }
>   };
> +MODULE_DEVICE_TABLE(acpi, cht_wc_acpi_ids);
>   
>   static struct i2c_driver cht_wc_driver = {
>   	.driver	= {
> 

Re: [PATCH 2/2] mfd: intel_soc_pmic_chtwc: Fix module autoload

[Javier Martinez Canillas]

Hello Hans,

On 08/09/2017 02:02 PM, Hans de Goede wrote:
> Hi,
> 
> On 09-08-17 10:44, Javier Martinez Canillas wrote:
>> The driver has a tristate Kconfig symbol so it can be built as a module,
>> but it doesn't export the device aliases in the module. So if the driver
>> is built as module, autoload won't work since udev/kmod won't be able to
>> match the registered ACPI device with its corresponding driver module.
>>
>> Use the MODULE_DEVICE_TABLE() macro to export the ACPI device as alias.
>>
>> Before this patch:
>>
>> $ modinfo drivers/mfd/intel_soc_pmic_chtwc.ko | grep alias
>> $
>>
>> After this patch
>>
>> $ modinfo drivers/mfd/intel_soc_pmic_chtwc.ko | grep alias
>> alias:          acpi*:INT34D3:*
>>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> 
> As the Kconfig help text mentions this driver should always be
> builtin if enabled. But I somehow made a mistake and it became a
> tristate  in Kconfig instead of a bool. A patch to fix this
> (make it a bool) is pending, so this patch is not necessary:
> 
> NACK.
>

I see, sorry for the noise then. I just looked at the Kconfig symbol but didn't
read the Kconfig help text...

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat