Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1423311 > unrolled thread

[PATCH 0/3] Init device ids from ACPI of_compatible

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] Init device ids from ACPI of_compatible Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-06-15 20:40 +0200
    [PATCH 2/3] acpi i2c: Initialize info.type from of_compatible Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-06-15 20:40 +0200
      Re: [PATCH 2/3] acpi i2c: Initialize info.type from of_compatible Mika Westerberg <mika.westerberg@linux.intel.com> - 2016-06-16 10:20 +0200
        Re: [PATCH 2/3] acpi i2c: Initialize info.type from of_compatible Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-06-17 14:10 +0200
          Re: [PATCH 2/3] acpi i2c: Initialize info.type from of_compatible Mika Westerberg <mika.westerberg@linux.intel.com> - 2016-06-17 14:20 +0200
    [PATCH 3/3] acpi spi: Initialize modalias from of_compatible Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-06-15 20:40 +0200
      Re: [PATCH 3/3] acpi spi: Initialize modalias from of_compatible Mika Westerberg <mika.westerberg@linux.intel.com> - 2016-06-16 10:20 +0200

#1423311 — [PATCH 0/3] Init device ids from ACPI of_compatible

When using devicetree stuff like i2c_client.name or spi_device.modalias
is initialized to the first DT compatible id with the vendor prefix
stripped. Since some drivers rely on this try to replicate it when using
ACPI with DT ids.

Drivers rely on these ids in order to differentiate between hardware variants
supported by the same driver. As far as I can tell the alternative would be for
drivers to dig into ACPI_COMPANION manually somehow?

This also makes it so that the i2c_device_id parameter passed to probe is
non-NULL when matching with ACPI and DT ids.

The patches only touch the ACPI-specific parts of the i2c and spi core.

These patches are on top of v4.7-rc3. I tested this using Octavin Purdila's
patches for "acpi overlays" but this series does not depend on that. Patch 2
has a conflict with acpi overlays because acpi_i2c_add_device was split into
multiple functions.

Here is an example .dsl for an SPI accelerometer connected to minnowboard max:

Device (ACCL)
{
    Name (_ADR, Zero)
    Name (_HID, "PRP0001")
    Name (_CID, "PRP0001")
    Name (_UID, One)

    Method (_CRS, 0, Serialized)
    {
	Name (RBUF, ResourceTemplate ()
	{
	    SPISerialBus(1, PolarityLow, FourWireMode, 16,
		    ControllerInitiated, 1000000, ClockPolarityLow,
		    ClockPhaseFirst, "\\_SB.SPI1",)
	    GpioInt (Edge, ActiveHigh, Exclusive, PullDown, 0x0000,
		     "\\_SB.GPO2", 0x00, ResourceConsumer, , )
	    { // Pin list
		    1
	    }
	})
	Return (RBUF)
    }
    Name (_DSD, Package ()
    {
	ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
	Package ()
	{
	    Package () {"compatible", "st,lis3dh"},
	}
    })
}

The driver for that device supports tens of models is not aware of ACPI.

Like to prev thread: https://www.spinics.net/lists/linux-acpi/msg66469.html
Like to acpi overlays: https://www.spinics.net/lists/linux-acpi/msg66638.html

Crestez Dan Leonard (3):
  acpi: Export acpi_of_modalias equiv of of_modalias_node
  acpi i2c: Initialize info.type from of_compatible
  acpi spi: Initialize modalias from of_compatible

 drivers/acpi/bus.c      | 36 ++++++++++++++++++++++++++++++++++++
 drivers/i2c/i2c-core.c  |  8 +++++++-
 drivers/spi/spi.c       | 10 +++++++++-
 include/acpi/acpi_bus.h |  1 +
 4 files changed, 53 insertions(+), 2 deletions(-)

-- 
2.5.5

[toc] | [next] | [standalone]

#1423313 — [PATCH 2/3] acpi i2c: Initialize info.type from of_compatible

When using devicetree i2c_board_info.type is set to the compatible
string with the vendor prefix removed. For I2C devices described via
ACPI the i2c_board_info.type string is set to the ACPI device name. When
using ACPI and DT ids this string ends up something like "PRP0001:00".

If the of_compatible property is present try to use that instead. This
makes it easier to instantiate i2c drivers through ACPI with DT ids.

Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
---
 drivers/i2c/i2c-core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index af11b65..5ab1fb9 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -188,8 +188,14 @@ static acpi_status acpi_i2c_add_device(acpi_handle handle, u32 level,
 
 	acpi_dev_free_resource_list(&resource_list);
 
+	if (adev->data.of_compatible) {
+		ret = acpi_of_modalias(adev, info.type, sizeof(info.type));
+		if (ret)
+			return -EINVAL;
+	} else
+		strlcpy(info.type, dev_name(&adev->dev), sizeof(info.type));
+
 	adev->power.flags.ignore_parent = true;
-	strlcpy(info.type, dev_name(&adev->dev), sizeof(info.type));
 	if (!i2c_new_device(adapter, &info)) {
 		adev->power.flags.ignore_parent = false;
 		dev_err(&adapter->dev,
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1423777 — Re: [PATCH 2/3] acpi i2c: Initialize info.type from of_compatible

On Wed, Jun 15, 2016 at 09:30:28PM +0300, Crestez Dan Leonard wrote:
> When using devicetree i2c_board_info.type is set to the compatible
> string with the vendor prefix removed. For I2C devices described via
> ACPI the i2c_board_info.type string is set to the ACPI device name. When
> using ACPI and DT ids this string ends up something like "PRP0001:00".
> 
> If the of_compatible property is present try to use that instead. This
> makes it easier to instantiate i2c drivers through ACPI with DT ids.

Adding Jarkko -- he was the original author of I2C/SPI device naming in
ACPI. Since this only affects devices with PRP0001 in their _HID/_CID,
it should not break anything.

> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
> ---
>  drivers/i2c/i2c-core.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index af11b65..5ab1fb9 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -188,8 +188,14 @@ static acpi_status acpi_i2c_add_device(acpi_handle handle, u32 level,
>  
>  	acpi_dev_free_resource_list(&resource_list);
>  
> +	if (adev->data.of_compatible) {

We may want to add acpi_is_of_compatible(adev) or similar at some point.

> +		ret = acpi_of_modalias(adev, info.type, sizeof(info.type));
> +		if (ret)
> +			return -EINVAL;
> +	} else
> +		strlcpy(info.type, dev_name(&adev->dev), sizeof(info.type));

Please use {} in the else branch as well. See Documentation/CodingStyle.

> +
>  	adev->power.flags.ignore_parent = true;
> -	strlcpy(info.type, dev_name(&adev->dev), sizeof(info.type));
>  	if (!i2c_new_device(adapter, &info)) {
>  		adev->power.flags.ignore_parent = false;
>  		dev_err(&adapter->dev,
> -- 
> 2.5.5

[toc] | [prev] | [next] | [standalone]

#1424991 — Re: [PATCH 2/3] acpi i2c: Initialize info.type from of_compatible

On 06/16/2016 11:11 AM, Mika Westerberg wrote:
> On Wed, Jun 15, 2016 at 09:30:28PM +0300, Crestez Dan Leonard wrote:
>> When using devicetree i2c_board_info.type is set to the compatible
>> string with the vendor prefix removed. For I2C devices described via
>> ACPI the i2c_board_info.type string is set to the ACPI device name. When
>> using ACPI and DT ids this string ends up something like "PRP0001:00".
>>
>> If the of_compatible property is present try to use that instead. This
>> makes it easier to instantiate i2c drivers through ACPI with DT ids.
> 
> Adding Jarkko -- he was the original author of I2C/SPI device naming in
> ACPI. Since this only affects devices with PRP0001 in their _HID/_CID,
> it should not break anything.

It's possible to have something like this:

            Name (_HID, "XXX1234")
            Name (_CID, "PRP0001")
            Name (_DSD, Package () {
                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
                Package () {
                    Package () {"compatible", "vendor,x1234"},
                }
            })

With these patches the i2c device name changes from "XXX1234:00" to
"x1234". This would happen even if a driver matches the XXX1234 acpi ID.
It's not clear if anyone actually uses that or if this change would be
harmful.

It theory it would be possible to use of_compatible for naming only if
there are no other real IDs. I don't think it's worthwhile.

[toc] | [prev] | [next] | [standalone]

#1425000 — Re: [PATCH 2/3] acpi i2c: Initialize info.type from of_compatible

On Fri, Jun 17, 2016 at 03:07:39PM +0300, Crestez Dan Leonard wrote:
> On 06/16/2016 11:11 AM, Mika Westerberg wrote:
> > On Wed, Jun 15, 2016 at 09:30:28PM +0300, Crestez Dan Leonard wrote:
> >> When using devicetree i2c_board_info.type is set to the compatible
> >> string with the vendor prefix removed. For I2C devices described via
> >> ACPI the i2c_board_info.type string is set to the ACPI device name. When
> >> using ACPI and DT ids this string ends up something like "PRP0001:00".
> >>
> >> If the of_compatible property is present try to use that instead. This
> >> makes it easier to instantiate i2c drivers through ACPI with DT ids.
> > 
> > Adding Jarkko -- he was the original author of I2C/SPI device naming in
> > ACPI. Since this only affects devices with PRP0001 in their _HID/_CID,
> > it should not break anything.
> 
> It's possible to have something like this:
> 
>             Name (_HID, "XXX1234")
>             Name (_CID, "PRP0001")
>             Name (_DSD, Package () {
>                 ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>                 Package () {
>                     Package () {"compatible", "vendor,x1234"},
>                 }
>             })
> 
> With these patches the i2c device name changes from "XXX1234:00" to
> "x1234". This would happen even if a driver matches the XXX1234 acpi ID.
> It's not clear if anyone actually uses that or if this change would be
> harmful.

I have not seen a single machine using _DSD with PRP0001 so I don't
think it is a problem.

> It theory it would be possible to use of_compatible for naming only if
> there are no other real IDs. I don't think it's worthwhile.

I agree.

[toc] | [prev] | [next] | [standalone]

#1423315 — [PATCH 3/3] acpi spi: Initialize modalias from of_compatible

When using devicetree spi_device.modalias is set to the compatible
string with the vendor prefix removed. For SPI devices described via
ACPI the i2c_board_info.type string is initialized by acpi_device_hid.
When using ACPI and DT ids this string ends up something like "PRP0001".

Change acpi_register_spi_device to use the of_compatible property if
present. This makes it easier to instantiate spi drivers through ACPI
with DT ids.

Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
---
 drivers/spi/spi.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 77e6e45..eef5ac2 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1680,11 +1680,19 @@ static acpi_status acpi_spi_add_device(acpi_handle handle, u32 level,
 		return AE_OK;
 	}
 
+	if (adev->data.of_compatible) {
+		ret = acpi_of_modalias(adev, spi->modalias, sizeof(spi->modalias));
+		if (ret) {
+			spi_dev_put(spi);
+			return AE_NOT_FOUND;
+		}
+	} else
+		strlcpy(spi->modalias, acpi_device_hid(adev), sizeof(spi->modalias));
+
 	if (spi->irq < 0)
 		spi->irq = acpi_dev_gpio_irq_get(adev, 0);
 
 	adev->power.flags.ignore_parent = true;
-	strlcpy(spi->modalias, acpi_device_hid(adev), sizeof(spi->modalias));
 	if (spi_add_device(spi)) {
 		adev->power.flags.ignore_parent = false;
 		dev_err(&master->dev, "failed to add SPI device %s from ACPI\n",
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1423776 — Re: [PATCH 3/3] acpi spi: Initialize modalias from of_compatible

On Wed, Jun 15, 2016 at 09:30:29PM +0300, Crestez Dan Leonard wrote:
> When using devicetree spi_device.modalias is set to the compatible
> string with the vendor prefix removed. For SPI devices described via
> ACPI the i2c_board_info.type string is initialized by acpi_device_hid.
> When using ACPI and DT ids this string ends up something like "PRP0001".
> 
> Change acpi_register_spi_device to use the of_compatible property if
> present. This makes it easier to instantiate spi drivers through ACPI
> with DT ids.
> 
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
> ---
>  drivers/spi/spi.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 77e6e45..eef5ac2 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1680,11 +1680,19 @@ static acpi_status acpi_spi_add_device(acpi_handle handle, u32 level,
>  		return AE_OK;
>  	}
>  
> +	if (adev->data.of_compatible) {
> +		ret = acpi_of_modalias(adev, spi->modalias, sizeof(spi->modalias));
> +		if (ret) {
> +			spi_dev_put(spi);
> +			return AE_NOT_FOUND;
> +		}
> +	} else
> +		strlcpy(spi->modalias, acpi_device_hid(adev), sizeof(spi->modalias));

Same here, use {} in the else branch.

> +
>  	if (spi->irq < 0)
>  		spi->irq = acpi_dev_gpio_irq_get(adev, 0);
>  
>  	adev->power.flags.ignore_parent = true;
> -	strlcpy(spi->modalias, acpi_device_hid(adev), sizeof(spi->modalias));
>  	if (spi_add_device(spi)) {
>  		adev->power.flags.ignore_parent = false;
>  		dev_err(&master->dev, "failed to add SPI device %s from ACPI\n",
> -- 
> 2.5.5

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web