[PATCH 0/2] ACPI match adc128s052 to ADC on UP Squared

[PATCH 0/2] ACPI match adc128s052 to ADC on UP Squared

[Javier Arteaga]

Hi all,

On AAEON's UP Squared board, there is an ADC124S101 enumerated via ACPI.
This patchset enables ACPI matching by compatible string and by AAEON's
allocated HID.

Dan O'Donovan (1):
  iio: adc128s052: allow driver to be matched using ACPI

Nicola Lunghi (1):
  iio: adc128s052: add ACPI _HID AANT1280

 drivers/iio/adc/ti-adc128s052.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

-- 
2.17.0

[PATCH 1/2] iio: adc128s052: allow driver to be matched using ACPI

[Javier Arteaga]

From: Dan O'Donovan <dan@emutex.com>

Allow driver to be matched by compatible string in
ACPI device properties.

Signed-off-by: Dan O'Donovan <dan@emutex.com>
---
 drivers/iio/adc/ti-adc128s052.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
index 7cf39b3e2416..56ec04d1c68f 100644
--- a/drivers/iio/adc/ti-adc128s052.c
+++ b/drivers/iio/adc/ti-adc128s052.c
@@ -202,7 +202,7 @@ MODULE_DEVICE_TABLE(spi, adc128_id);
 static struct spi_driver adc128_driver = {
 	.driver = {
 		.name = "adc128s052",
-		.of_match_table = of_match_ptr(adc128_of_match),
+		.of_match_table = adc128_of_match,
 	},
 	.probe = adc128_probe,
 	.remove = adc128_remove,
-- 
2.17.0

Re: [PATCH 1/2] iio: adc128s052: allow driver to be matched using ACPI

[Jonathan Cameron]

On Thu, 19 Apr 2018 14:20:35 +0100
Javier Arteaga <javier@emutex.com> wrote:

> From: Dan O'Donovan <dan@emutex.com>
> 
> Allow driver to be matched by compatible string in
> ACPI device properties.
> 
> Signed-off-by: Dan O'Donovan <dan@emutex.com>
Hi Javier,

I don't really see the connection between the change in here
and what the description says...

If you are probing from ACPI then there is no need to ensure
a valid of table is supplied (even if we aren't building with OF)
which is what I think this patch is trying to do...

Jonathan

> ---
>  drivers/iio/adc/ti-adc128s052.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
> index 7cf39b3e2416..56ec04d1c68f 100644
> --- a/drivers/iio/adc/ti-adc128s052.c
> +++ b/drivers/iio/adc/ti-adc128s052.c
> @@ -202,7 +202,7 @@ MODULE_DEVICE_TABLE(spi, adc128_id);
>  static struct spi_driver adc128_driver = {
>  	.driver = {
>  		.name = "adc128s052",
> -		.of_match_table = of_match_ptr(adc128_of_match),
> +		.of_match_table = adc128_of_match,
>  	},
>  	.probe = adc128_probe,
>  	.remove = adc128_remove,

Re: [PATCH 1/2] iio: adc128s052: allow driver to be matched using ACPI

[Javier Arteaga]

Hi Jonathan,

On Sat, Apr 21, 2018 at 04:54:41PM +0100, Jonathan Cameron wrote:
> I don't really see the connection between the change in here
> and what the description says...

I think you're right, we didn't make our intent clear here.

> If you are probing from ACPI then there is no need to ensure
> a valid of table is supplied (even if we aren't building with OF)
> which is what I think this patch is trying to do...

The patch enables ACPI _DSD to reuse existing DT "compatible" strings,
as described in Documentation/acpi/enumeration.txt, even without OF.
This kind of patch has some precedent, like for example:

    01427fe7c4b9 ("Input: adxl34x - make it enumerable in ACPI environment")

To clarify, for the UP2 board we don't actually need this patch as we
have an ACPI _HID - just thought it would might be an improvement for
others.

I'll improve the description and perhaps reorder this patch last for v2.
Or I can send separately if you prefer.

Thanks for your review!

Re: [PATCH 1/2] iio: adc128s052: allow driver to be matched using ACPI

[Jonathan Cameron]

On Sat, 21 Apr 2018 21:41:57 +0100
Javier Arteaga <javier@emutex.com> wrote:

> Hi Jonathan,
> 
> On Sat, Apr 21, 2018 at 04:54:41PM +0100, Jonathan Cameron wrote:
> > I don't really see the connection between the change in here
> > and what the description says...  
> 
> I think you're right, we didn't make our intent clear here.
> 
> > If you are probing from ACPI then there is no need to ensure
> > a valid of table is supplied (even if we aren't building with OF)
> > which is what I think this patch is trying to do...  
> 
> The patch enables ACPI _DSD to reuse existing DT "compatible" strings,
> as described in Documentation/acpi/enumeration.txt, even without OF.
> This kind of patch has some precedent, like for example:
> 
>     01427fe7c4b9 ("Input: adxl34x - make it enumerable in ACPI environment")
> 
> To clarify, for the UP2 board we don't actually need this patch as we
> have an ACPI _HID - just thought it would might be an improvement for
> others.
> 
> I'll improve the description and perhaps reorder this patch last for v2.
> Or I can send separately if you prefer.
> 
> Thanks for your review!
Unless we know of ACPI firmwares out there that are doing this, drop the
patch for now.  I don't want to see a flood of these based on a 'you can
do it that' way argument.

Jonathan

> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] iio: adc128s052: allow driver to be matched using ACPI

[Javier Arteaga]

On Tue, Apr 24, 2018 at 06:23:30PM +0100, Jonathan Cameron wrote:
> Unless we know of ACPI firmwares out there that are doing this, drop the
> patch for now.  I don't want to see a flood of these based on a 'you can
> do it that' way argument.

Fair enough. I'll drop it for v3.

[PATCH 2/2] iio: adc128s052: add ACPI _HID AANT1280

[Javier Arteaga]

From: Nicola Lunghi <nicola.lunghi@emutex.com>

ACPI _HID AANT1280 matches an ADC124S101 present on the UP Squared board
that is compatible with adc128s052.

Add it to the driver.

Signed-off-by: Nicola Lunghi <nicola.lunghi@emutex.com>
[javier@emutex.com: fix up commit message and one checkpatch warning]
Signed-off-by: Javier Arteaga <javier@emutex.com>
---
 drivers/iio/adc/ti-adc128s052.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
index 56ec04d1c68f..917273f1268c 100644
--- a/drivers/iio/adc/ti-adc128s052.c
+++ b/drivers/iio/adc/ti-adc128s052.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/iio/iio.h>
 #include <linux/regulator/consumer.h>
+#include <linux/acpi.h>
 
 struct adc128_configuration {
 	const struct iio_chan_spec	*channels;
@@ -136,9 +137,22 @@ static int adc128_probe(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev;
 	struct adc128 *adc;
-	int config = spi_get_device_id(spi)->driver_data;
+	int config;
 	int ret;
 
+	if (ACPI_COMPANION(&spi->dev)) {
+		const struct acpi_device_id *ad_id;
+
+		ad_id = acpi_match_device(spi->dev.driver->acpi_match_table,
+					  &spi->dev);
+		if (!ad_id)
+			return -ENODEV;
+
+		config = ad_id->driver_data;
+	} else {
+		config = spi_get_device_id(spi)->driver_data;
+	}
+
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
 	if (!indio_dev)
 		return -ENOMEM;
@@ -199,10 +213,19 @@ static const struct spi_device_id adc128_id[] = {
 };
 MODULE_DEVICE_TABLE(spi, adc128_id);
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id adc128_acpi_match[] = {
+	{ "AANT1280", 2 }, /* ADC124S021 compatible ACPI ID */
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, adc128_acpi_match);
+#endif
+
 static struct spi_driver adc128_driver = {
 	.driver = {
 		.name = "adc128s052",
 		.of_match_table = adc128_of_match,
+		.acpi_match_table = ACPI_PTR(adc128_acpi_match),
 	},
 	.probe = adc128_probe,
 	.remove = adc128_remove,
-- 
2.17.0

Re: [PATCH 2/2] iio: adc128s052: add ACPI _HID AANT1280

[Jonathan Cameron]

On Thu, 19 Apr 2018 14:20:36 +0100
Javier Arteaga <javier@emutex.com> wrote:

> From: Nicola Lunghi <nicola.lunghi@emutex.com>
> 
> ACPI _HID AANT1280 matches an ADC124S101 present on the UP Squared board
> that is compatible with adc128s052.
> 
> Add it to the driver.
> 
> Signed-off-by: Nicola Lunghi <nicola.lunghi@emutex.com>
> [javier@emutex.com: fix up commit message and one checkpatch warning]
> Signed-off-by: Javier Arteaga <javier@emutex.com>
Hi Javier,

This is in principle fine, but I'd like to see it done somewhat more
explicitly.

1) Add the adc124s101 and similar families to the driver first. I think
   this is just a case of IDs for all of
   adc124s051 and adc124s101.

2) Add the ACPI ID as you have done here.

That way we at least reflect clearly that the part is supported rather than
just in a comment on the ACPI ID.

Hmm. Looking at it, we should perhaps revisit whether some of these
TI parts are similar enough that their drivers can be cleanly combined.
A job for another day!

Jonathan


> ---
>  drivers/iio/adc/ti-adc128s052.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
> index 56ec04d1c68f..917273f1268c 100644
> --- a/drivers/iio/adc/ti-adc128s052.c
> +++ b/drivers/iio/adc/ti-adc128s052.c
> @@ -17,6 +17,7 @@
>  #include <linux/module.h>
>  #include <linux/iio/iio.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/acpi.h>
>  
>  struct adc128_configuration {
>  	const struct iio_chan_spec	*channels;
> @@ -136,9 +137,22 @@ static int adc128_probe(struct spi_device *spi)
>  {
>  	struct iio_dev *indio_dev;
>  	struct adc128 *adc;
> -	int config = spi_get_device_id(spi)->driver_data;
> +	int config;
>  	int ret;
>  
> +	if (ACPI_COMPANION(&spi->dev)) {
> +		const struct acpi_device_id *ad_id;
> +
> +		ad_id = acpi_match_device(spi->dev.driver->acpi_match_table,
> +					  &spi->dev);
> +		if (!ad_id)
> +			return -ENODEV;
> +
> +		config = ad_id->driver_data;
> +	} else {
> +		config = spi_get_device_id(spi)->driver_data;
> +	}
> +
>  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
>  	if (!indio_dev)
>  		return -ENOMEM;
> @@ -199,10 +213,19 @@ static const struct spi_device_id adc128_id[] = {
>  };
>  MODULE_DEVICE_TABLE(spi, adc128_id);
>  
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id adc128_acpi_match[] = {
> +	{ "AANT1280", 2 }, /* ADC124S021 compatible ACPI ID */
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, adc128_acpi_match);
> +#endif
> +
>  static struct spi_driver adc128_driver = {
>  	.driver = {
>  		.name = "adc128s052",
>  		.of_match_table = adc128_of_match,
> +		.acpi_match_table = ACPI_PTR(adc128_acpi_match),
>  	},
>  	.probe = adc128_probe,
>  	.remove = adc128_remove,

Re: [PATCH 2/2] iio: adc128s052: add ACPI _HID AANT1280

[Javier Arteaga]

Hi Jonathan,

On Sat, Apr 21, 2018 at 05:05:05PM +0100, Jonathan Cameron wrote:
> This is in principle fine, but I'd like to see it done somewhat more
> explicitly.
> 
> 1) Add the adc124s101 and similar families to the driver first. I think
>    this is just a case of IDs for all of
>    adc124s051 and adc124s101.
> 
> 2) Add the ACPI ID as you have done here.
> 
> That way we at least reflect clearly that the part is supported rather than
> just in a comment on the ACPI ID.

Will do for v2.

Thank you!