[PATCH v1 0/1] ACPI enumeration support for i2c mux clients

[PATCH v1 0/1] ACPI enumeration support for i2c mux clients

[Kuppuswamy Sathyanarayanan]

Added ACPI enumeration support for i2c mux clients.

Please let me know your review comments.

Kuppuswamy Sathyanarayanan (1):
  i2c: Add acpi support to enumerate i2c mux clients

 drivers/i2c/i2c-core.c |  3 ++-
 drivers/i2c/i2c-mux.c  | 26 ++++++++++++++++++++++++++
 include/linux/i2c.h    |  5 +++++
 3 files changed, 33 insertions(+), 1 deletion(-)

-- 
1.9.1

[PATCH v1 1/1] i2c: Add acpi support to enumerate i2c mux clients

[Kuppuswamy Sathyanarayanan]

Added support to enumerate i2c mux clients via ACPI.

Currently we don't have any fixed way for enumerating
mux clients via acpi. This patch adds support to scan
for all client i2c devices under the mux device in DSDT
table and enumerate them in order.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/i2c-core.c |  3 ++-
 drivers/i2c/i2c-mux.c  | 26 ++++++++++++++++++++++++++
 include/linux/i2c.h    |  5 +++++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index edf274c..94f76e3 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -118,7 +118,7 @@ static int acpi_i2c_add_resource(struct acpi_resource *ares, void *data)
 	return 1;
 }
 
-static acpi_status acpi_i2c_add_device(acpi_handle handle, u32 level,
+acpi_status acpi_i2c_add_device(acpi_handle handle, u32 level,
 				       void *data, void **return_value)
 {
 	struct i2c_adapter *adapter = data;
@@ -155,6 +155,7 @@ static acpi_status acpi_i2c_add_device(acpi_handle handle, u32 level,
 
 	return AE_OK;
 }
+EXPORT_SYMBOL_GPL(acpi_i2c_add_device);
 
 /**
  * acpi_i2c_register_devices - enumerate I2C slave devices behind adapter
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 593f7ca..32cf7a2 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -100,6 +100,29 @@ static unsigned int i2c_mux_parent_classes(struct i2c_adapter *parent)
 	return class;
 }
 
+#ifdef CONFIG_ACPI
+static void acpi_i2c_mux_register_devices(struct i2c_adapter *adap,
+					  struct device *mux_dev)
+{
+	acpi_handle handle;
+	acpi_status status;
+
+	handle = ACPI_HANDLE(mux_dev);
+	if (!handle)
+		return;
+
+	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
+				     acpi_i2c_add_device, NULL,
+				     adap, NULL);
+
+	if (ACPI_FAILURE(status))
+		dev_warn(mux_dev, "mux adapter slave enumeration fails\n");
+}
+#else /* !CONFIG_ACPI */
+static inline void acpi_i2c_mux_register_devices(struct i2c_adapter *adap,
+						 struct device *mux_dev) { }
+#endif /* CONFIG_ACPI */
+
 struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
 				struct device *mux_dev,
 				void *mux_priv, u32 force_nr, u32 chan_id,
@@ -193,6 +216,9 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
 	dev_info(&parent->dev, "Added multiplexed i2c bus %d\n",
 		 i2c_adapter_id(&priv->adap));
 
+	/* scan for mux clients */
+	acpi_i2c_mux_register_devices(&priv->adap, mux_dev);
+
 	return &priv->adap;
 }
 EXPORT_SYMBOL_GPL(i2c_add_mux_adapter);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index f17da50..67b1412 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -33,6 +33,7 @@
 #include <linux/of.h>		/* for struct device_node */
 #include <linux/swab.h>		/* for swab16 */
 #include <uapi/linux/i2c.h>
+#include <linux/acpi.h>
 
 extern struct bus_type i2c_bus_type;
 extern struct device_type i2c_adapter_type;
@@ -608,4 +609,8 @@ static inline struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node
 }
 #endif /* CONFIG_OF */
 
+#ifdef CONFIG_ACPI
+extern acpi_status acpi_i2c_add_device(acpi_handle handle, u32 level,
+				       void *data, void **return_value);
+#endif /* CONFIG_ACPI */
 #endif /* _LINUX_I2C_H */
-- 
1.9.1

Re: [PATCH v1 1/1] i2c: Add acpi support to enumerate i2c mux clients

[Mika Westerberg]

On Thu, Apr 09, 2015 at 12:15:42PM -0700, Kuppuswamy Sathyanarayanan wrote:
> Added support to enumerate i2c mux clients via ACPI.
> 
> Currently we don't have any fixed way for enumerating
> mux clients via acpi. This patch adds support to scan
> for all client i2c devices under the mux device in DSDT
> table and enumerate them in order.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

>From ACPI standpoint I don't see problems with this code.

Reviewed-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Re: [PATCH v1 1/1] i2c: Add acpi support to enumerate i2c mux clients

[Sathyanarayanan Kuppuswamy]

Thanks Mika.

On 04/10/2015 01:42 AM, Mika Westerberg wrote:
> On Thu, Apr 09, 2015 at 12:15:42PM -0700, Kuppuswamy Sathyanarayanan wrote:
>> Added support to enumerate i2c mux clients via ACPI.
>>
>> Currently we don't have any fixed way for enumerating
>> mux clients via acpi. This patch adds support to scan
>> for all client i2c devices under the mux device in DSDT
>> table and enumerate them in order.
>>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> >From ACPI standpoint I don't see problems with this code.
>
> Reviewed-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>

-- 
--
Sathyanarayanan KN
Android Kernel Developer

Re: [PATCH v1 1/1] i2c: Add acpi support to enumerate i2c mux clients

[sathyanarayanan kuppuswamy]

ping.

On 04/10/2015 01:42 AM, Mika Westerberg wrote:
> On Thu, Apr 09, 2015 at 12:15:42PM -0700, Kuppuswamy Sathyanarayanan wrote:
>> Added support to enumerate i2c mux clients via ACPI.
>>
>> Currently we don't have any fixed way for enumerating
>> mux clients via acpi. This patch adds support to scan
>> for all client i2c devices under the mux device in DSDT
>> table and enumerate them in order.
>>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> >From ACPI standpoint I don't see problems with this code.
>
> Reviewed-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>

-- 
Sathyanarayanan Kuppuswamy
Android kernel developer

Re: [PATCH v1 1/1] i2c: Add acpi support to enumerate i2c mux clients

[sathyanarayanan kuppuswamy]

gentle ping.

On 04/10/2015 01:42 AM, Mika Westerberg wrote:
> On Thu, Apr 09, 2015 at 12:15:42PM -0700, Kuppuswamy Sathyanarayanan wrote:
>> Added support to enumerate i2c mux clients via ACPI.
>>
>> Currently we don't have any fixed way for enumerating
>> mux clients via acpi. This patch adds support to scan
>> for all client i2c devices under the mux device in DSDT
>> table and enumerate them in order.
>>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> >From ACPI standpoint I don't see problems with this code.
>
> Reviewed-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>

-- 
Sathyanarayanan Kuppuswamy
Android kernel developer

Re: [PATCH v1 1/1] i2c: Add acpi support to enumerate i2c mux clients

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 985 bytes --]

> +#ifdef CONFIG_ACPI
> +static void acpi_i2c_mux_register_devices(struct i2c_adapter *adap,
> +					  struct device *mux_dev)
> +{
> +	acpi_handle handle;
> +	acpi_status status;
> +
> +	handle = ACPI_HANDLE(mux_dev);
> +	if (!handle)
> +		return;
> +
> +	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
> +				     acpi_i2c_add_device, NULL,
> +				     adap, NULL);
> +
> +	if (ACPI_FAILURE(status))
> +		dev_warn(mux_dev, "mux adapter slave enumeration fails\n");
> +}
> +#else /* !CONFIG_ACPI */
> +static inline void acpi_i2c_mux_register_devices(struct i2c_adapter *adap,
> +						 struct device *mux_dev) { }
> +#endif /* CONFIG_ACPI */

IMO, this shares too much code with acpi_i2c_register_devices(). And it
pulls in ACPI into mux.c which is not really needed.

What about naming the above function
acpi_i2c_register_devices_from_dev() and let acpi_i2c_register_devices()
then call it as a helper function, all this in i2c-core.c?

Thanks for your patience BTW...


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v1 1/1] i2c: Add acpi support to enumerate i2c mux clients

[sathyanarayanan kuppuswamy]

Hi Sang

Thanks for your comments. Please find my reply inline.

On 06/02/2015 09:19 AM, Wolfram Sang wrote:
>> +#ifdef CONFIG_ACPI
>> +static void acpi_i2c_mux_register_devices(struct i2c_adapter *adap,
>> +					  struct device *mux_dev)
>> +{
>> +	acpi_handle handle;
>> +	acpi_status status;
>> +
>> +	handle = ACPI_HANDLE(mux_dev);
>> +	if (!handle)
>> +		return;
>> +
>> +	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
>> +				     acpi_i2c_add_device, NULL,
>> +				     adap, NULL);
>> +
>> +	if (ACPI_FAILURE(status))
>> +		dev_warn(mux_dev, "mux adapter slave enumeration fails\n");
>> +}
>> +#else /* !CONFIG_ACPI */
>> +static inline void acpi_i2c_mux_register_devices(struct i2c_adapter *adap,
>> +						 struct device *mux_dev) { }
>> +#endif /* CONFIG_ACPI */
> IMO, this shares too much code with acpi_i2c_register_devices(). And it
> pulls in ACPI into mux.c which is not really needed.
Even though mux is a just a virtual adapter without any ACPI ID, Its 
slaves are a still actual devices and needs to be enumerated by ACPI. So 
think its accptable to include ACPI in mux code. Don't you agree ?

Also ACPI handle code in this function is slightly different from 
acpi_i2c_register_devices() code.

>
> What about naming the above function
> acpi_i2c_register_devices_from_dev() and let acpi_i2c_register_devices()
If you call this function from i2c-core register device function, then 
how will you maintain the hierarchy ? We need some way to indicate that 
these devices are under mux adapter right ?
> then call it as a helper function, all this in i2c-core.c?
>
> Thanks for your patience BTW...
>

-- 
Sathyanarayanan Kuppuswamy
Android kernel developer

Re: [PATCH v1 1/1] i2c: Add acpi support to enumerate i2c mux clients

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1716 bytes --]

On Tue, Jun 02, 2015 at 05:38:20PM -0700, sathyanarayanan kuppuswamy wrote:
> Hi Sang
> 
> Thanks for your comments. Please find my reply inline.
> 
> On 06/02/2015 09:19 AM, Wolfram Sang wrote:
> >>+#ifdef CONFIG_ACPI
> >>+static void acpi_i2c_mux_register_devices(struct i2c_adapter *adap,
> >>+					  struct device *mux_dev)
> >>+{
> >>+	acpi_handle handle;
> >>+	acpi_status status;
> >>+
> >>+	handle = ACPI_HANDLE(mux_dev);
> >>+	if (!handle)
> >>+		return;
> >>+
> >>+	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
> >>+				     acpi_i2c_add_device, NULL,
> >>+				     adap, NULL);
> >>+
> >>+	if (ACPI_FAILURE(status))
> >>+		dev_warn(mux_dev, "mux adapter slave enumeration fails\n");
> >>+}
> >>+#else /* !CONFIG_ACPI */
> >>+static inline void acpi_i2c_mux_register_devices(struct i2c_adapter *adap,
> >>+						 struct device *mux_dev) { }
> >>+#endif /* CONFIG_ACPI */
> >IMO, this shares too much code with acpi_i2c_register_devices(). And it
> >pulls in ACPI into mux.c which is not really needed.
> Even though mux is a just a virtual adapter without any ACPI ID, Its slaves
> are a still actual devices and needs to be enumerated by ACPI. So think its
> accptable to include ACPI in mux code. Don't you agree ?

The resulting function should be called from mux code. But the function
itself should be in the core.

> Also ACPI handle code in this function is slightly different from
> acpi_i2c_register_devices() code.

The only difference I see is that one takes a device as parameter, the
other an adapter. So, you can read out the device from the adapter in
acpi_i2c_register_devices() and then call
acpi_i2c_register_devices_from_dev().


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]