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

Groups > linux.kernel > #1442370 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/3] Init device ids from ACPI of_compatible Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-07-13 14:00 +0200
    [PATCH v2 2/3] acpi i2c: Initialize info.type from of_compatible Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-07-13 14:00 +0200
      Re: [PATCH v2 2/3] acpi i2c: Initialize info.type from of_compatible "Rafael J. Wysocki" <rafael@kernel.org> - 2016-07-13 14:30 +0200
        Re: [PATCH v2 2/3] acpi i2c: Initialize info.type from of_compatible Mika Westerberg <mika.westerberg@linux.intel.com> - 2016-07-19 09:10 +0200
    [PATCH v2 3/3] acpi spi: Initialize modalias from of_compatible Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-07-13 14:00 +0200
      Re: [PATCH v2 3/3] acpi spi: Initialize modalias from of_compatible "Rafael J. Wysocki" <rafael@kernel.org> - 2016-07-13 14:30 +0200
        Re: [PATCH v2 3/3] acpi spi: Initialize modalias from of_compatible Mika Westerberg <mika.westerberg@linux.intel.com> - 2016-07-19 09:10 +0200
      Re: [PATCH v2 3/3] acpi spi: Initialize modalias from of_compatible Mark Brown <broonie@kernel.org> - 2016-07-19 12:30 +0200
        Re: [PATCH v2 3/3] acpi spi: Initialize modalias from of_compatible Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-07-20 13:30 +0200
          Re: [PATCH v2 3/3] acpi spi: Initialize modalias from of_compatible Mark Brown <broonie@kernel.org> - 2016-07-20 13:40 +0200
    [PATCH v2 1/3] acpi: Export acpi_of_modalias equiv of of_modalias_node Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-07-13 14:00 +0200
      Re: [PATCH v2 1/3] acpi: Export acpi_of_modalias equiv of of_modalias_node "Rafael J. Wysocki" <rafael@kernel.org> - 2016-07-13 14:30 +0200
      Re: [PATCH v2 1/3] acpi: Export acpi_of_modalias equiv of  of_modalias_node Mika Westerberg <mika.westerberg@linux.intel.com> - 2016-07-19 09:20 +0200
    Re: [PATCH v2 0/3] Init device ids from ACPI of_compatible "Rafael J. Wysocki" <rafael@kernel.org> - 2016-07-13 14:20 +0200
    Re: [PATCH v2 0/3] Init device ids from ACPI of_compatible Dan O'Donovan <dan@emutex.com> - 2016-07-13 16:20 +0200

#1442370 — [PATCH v2 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 in order to differentiate between
hardware variants try to replicate it when using ACPI with DT ids.

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

These patches are on top of linux-pm/linux-next. I delayed v2 until ACPI
overlays got in in order to avoid conflicts. I tested using ACPI overlays but
there is no actual dependency. This series just extends the PRP0001 feature to
be more useful for I2C/SPI.

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

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"},
	}
    })
}

Link to v1: https://www.spinics.net/lists/linux-acpi/msg66469.html
Changes:
 * Rebase on after acpi overlays got it.
 * Change acpi_of_modalias outlen param to size_t
 * Use {} after else

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       | 11 ++++++++++-
 include/acpi/acpi_bus.h |  1 +
 4 files changed, 54 insertions(+), 2 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1442372 — [PATCH v2 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 74e5aea..62a1339 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -179,7 +179,13 @@ static int acpi_i2c_get_info(struct acpi_device *adev,
 
 	acpi_dev_free_resource_list(&resource_list);
 
-	strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
+	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));
+	}
 
 	return 0;
 }
-- 
2.7.4

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

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

On Wed, Jul 13, 2016 at 1:53 PM, Crestez Dan Leonard
<leonard.crestez@intel.com> 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.
>
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>

Looks good to me, but Mika needs to have a look at it too.

> ---
>  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 74e5aea..62a1339 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -179,7 +179,13 @@ static int acpi_i2c_get_info(struct acpi_device *adev,
>
>         acpi_dev_free_resource_list(&resource_list);
>
> -       strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
> +       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));
> +       }
>
>         return 0;
>  }
> --


Thanks,
Rafael

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

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

On Wed, Jul 13, 2016 at 02:20:57PM +0200, Rafael J. Wysocki wrote:
> On Wed, Jul 13, 2016 at 1:53 PM, Crestez Dan Leonard
> <leonard.crestez@intel.com> 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.
> >
> > Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
> 
> Looks good to me, but Mika needs to have a look at it too.

Looks good to me,

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

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

#1442373 — [PATCH v2 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 | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 7589c8a..49fdf6e 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1679,13 +1679,22 @@ static acpi_status acpi_register_spi_device(struct spi_master *master,
 		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);
 
 	acpi_device_set_enumerated(adev);
 
 	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.7.4

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

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

On Wed, Jul 13, 2016 at 1:53 PM, Crestez Dan Leonard
<leonard.crestez@intel.com> 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>

Same as for the i2c patch: it looks good to me, but I'd prefer to get
a word from Mika on it too.

> ---
>  drivers/spi/spi.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 7589c8a..49fdf6e 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1679,13 +1679,22 @@ static acpi_status acpi_register_spi_device(struct spi_master *master,
>                 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);
>
>         acpi_device_set_enumerated(adev);
>
>         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",
> --

Thanks,
Rafael

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

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

On Wed, Jul 13, 2016 at 02:22:02PM +0200, Rafael J. Wysocki wrote:
> On Wed, Jul 13, 2016 at 1:53 PM, Crestez Dan Leonard
> <leonard.crestez@intel.com> 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>
> 
> Same as for the i2c patch: it looks good to me, but I'd prefer to get
> a word from Mika on it too.

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

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

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

[Multipart message — attachments visible in raw view] — view raw

On Wed, Jul 13, 2016 at 02:53:42PM +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".

Please submit patches using subject lines reflecting the style for the
subsystem.  This makes it easier for people to identify relevant
patches.  Look at what existing commits in the area you're changing are
doing and make sure your subject lines visually resemble what they're
doing.

> 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.

This is basically fine but...

> +	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;
> +		}

The only reason this could fail currently is that there wasn't a
compatible in the first place so why don't we just handle it like the no
compatible case?  It's probably not realistic but it seems like there's
a small chance this could regress some platform if we do add more error
detection in acpi_of_modalias().

I'm still not sure why if we want to translate ACPI into DT why we don't
actually translate and avoid all this special casing and churn.

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

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

On 07/19/2016 01:22 PM, Mark Brown wrote:
> On Wed, Jul 13, 2016 at 02:53:42PM +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".
> 
> Please submit patches using subject lines reflecting the style for the
> subsystem.  This makes it easier for people to identify relevant
> patches.  Look at what existing commits in the area you're changing are
> doing and make sure your subject lines visually resemble what they're
> doing.

So the prefix should be something like "spi: acpi: "?

>> 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.
> 
> This is basically fine but...
> 
>> +	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;
>> +		}
> 
> The only reason this could fail currently is that there wasn't a
> compatible in the first place so why don't we just handle it like the no
> compatible case?  It's probably not realistic but it seems like there's
> a small chance this could regress some platform if we do add more error
> detection in acpi_of_modalias().

If acpi_of_modalias fails for some new reason wouldn't it be better to
fail explicitly rather than ignore it?

-- 
Regards,
Leonard

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

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

[Multipart message — attachments visible in raw view] — view raw

On Wed, Jul 20, 2016 at 02:21:55PM +0300, Crestez Dan Leonard wrote:
> On 07/19/2016 01:22 PM, Mark Brown wrote:

> > Please submit patches using subject lines reflecting the style for the
> > subsystem.  This makes it easier for people to identify relevant
> > patches.  Look at what existing commits in the area you're changing are
> > doing and make sure your subject lines visually resemble what they're
> > doing.

> So the prefix should be something like "spi: acpi: "?

Yes.

> >> +	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;
> >> +		}

> > The only reason this could fail currently is that there wasn't a
> > compatible in the first place so why don't we just handle it like the no
> > compatible case?  It's probably not realistic but it seems like there's
> > a small chance this could regress some platform if we do add more error
> > detection in acpi_of_modalias().

> If acpi_of_modalias fails for some new reason wouldn't it be better to
> fail explicitly rather than ignore it?

The current code will happily proceed to create a device without doing
this parsing so clearly we can do that.  It's not clear to me that it's
better to refuse to create the device at all than to soldier on and
create a device with only the native ACPI information, perhaps it just
needs a comment explaining why we do that.

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

#1442376 — [PATCH v2 1/3] acpi: Export acpi_of_modalias equiv of of_modalias_node

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.

Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
---
 drivers/acpi/bus.c      | 36 ++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |  1 +
 2 files changed, 37 insertions(+)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index cb9558e..77a1a80 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -654,6 +654,42 @@ static bool acpi_of_match_device(struct acpi_device *adev,
 	return false;
 }
 
+/**
+ * acpi_of_modalias - Like of_modalias_node for ACPI with DT ids
+ * @adev:	ACPI device object to match.
+ * @outstr:	Pointer to buffer for result
+ * @outlen:	Length of outstr value
+ *
+ * If we have a DT id set outstr to the first compatible string with the vendor
+ * prefix stripped, just like of_modalias_node does for devicetree.
+ *
+ * Returns 0 on success or negative errno on failure.
+ */
+int acpi_of_modalias(struct acpi_device *adev, char *outstr, size_t outlen)
+{
+	const union acpi_object *of_compatible;
+	const union acpi_object *obj;
+	const char *str, *chr;
+
+	of_compatible = adev->data.of_compatible;
+	if (!of_compatible)
+		return -ENODEV;
+
+	if (of_compatible->type == ACPI_TYPE_PACKAGE)
+		obj = of_compatible->package.elements;
+	else /* Must be ACPI_TYPE_STRING. */
+		obj = of_compatible;
+
+	str = obj->string.pointer;
+	chr = strchr(str, ',');
+	if (chr)
+		str = chr + 1;
+	strlcpy(outstr, str, outlen);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(acpi_of_modalias);
+
 static bool __acpi_match_device_cls(const struct acpi_device_id *id,
 				    struct acpi_hardware_id *hwid)
 {
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 788c6c35..1d8ff0d 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -515,6 +515,7 @@ void acpi_bus_trim(struct acpi_device *start);
 acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
 int acpi_match_device_ids(struct acpi_device *device,
 			  const struct acpi_device_id *ids);
+int acpi_of_modalias(struct acpi_device *adev, char *outstr, size_t outlen);
 int acpi_create_dir(struct acpi_device *);
 void acpi_remove_dir(struct acpi_device *);
 
-- 
2.7.4

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

#1442392 — Re: [PATCH v2 1/3] acpi: Export acpi_of_modalias equiv of of_modalias_node

On Wed, Jul 13, 2016 at 1:53 PM, Crestez Dan Leonard
<leonard.crestez@intel.com> wrote:
> 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.
>
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
> ---
>  drivers/acpi/bus.c      | 36 ++++++++++++++++++++++++++++++++++++
>  include/acpi/acpi_bus.h |  1 +
>  2 files changed, 37 insertions(+)
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index cb9558e..77a1a80 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -654,6 +654,42 @@ static bool acpi_of_match_device(struct acpi_device *adev,
>         return false;
>  }
>
> +/**
> + * acpi_of_modalias - Like of_modalias_node for ACPI with DT ids
> + * @adev:      ACPI device object to match.
> + * @outstr:    Pointer to buffer for result
> + * @outlen:    Length of outstr value
> + *
> + * If we have a DT id set outstr to the first compatible string with the vendor
> + * prefix stripped, just like of_modalias_node does for devicetree.

I'd say

"This is a counterpart of of_modalias_node() for struct acpi_device
objects.  If there is a compatible string for @adev, copy it to the
@outstr location with the vendor prefix stripped."

> + *
> + * Returns 0 on success or negative errno on failure.
> + */
> +int acpi_of_modalias(struct acpi_device *adev, char *outstr, size_t outlen)
> +{
> +       const union acpi_object *of_compatible;
> +       const union acpi_object *obj;
> +       const char *str, *chr;
> +
> +       of_compatible = adev->data.of_compatible;
> +       if (!of_compatible)
> +               return -ENODEV;
> +
> +       if (of_compatible->type == ACPI_TYPE_PACKAGE)
> +               obj = of_compatible->package.elements;
> +       else /* Must be ACPI_TYPE_STRING. */
> +               obj = of_compatible;
> +
> +       str = obj->string.pointer;
> +       chr = strchr(str, ',');
> +       if (chr)
> +               str = chr + 1;
> +       strlcpy(outstr, str, outlen);

I would prefer

strlcpy(outstr, chr ? chr + 1 : str, outlen);

ie. without the extra assignment.

> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(acpi_of_modalias);
> +
>  static bool __acpi_match_device_cls(const struct acpi_device_id *id,
>                                     struct acpi_hardware_id *hwid)
>  {
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 788c6c35..1d8ff0d 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -515,6 +515,7 @@ void acpi_bus_trim(struct acpi_device *start);
>  acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
>  int acpi_match_device_ids(struct acpi_device *device,
>                           const struct acpi_device_id *ids);
> +int acpi_of_modalias(struct acpi_device *adev, char *outstr, size_t outlen);
>  int acpi_create_dir(struct acpi_device *);
>  void acpi_remove_dir(struct acpi_device *);
>
> --

Thanks,
Rafael

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

#1446128 — Re: [PATCH v2 1/3] acpi: Export acpi_of_modalias equiv of of_modalias_node

On Wed, Jul 13, 2016 at 02:53:40PM +0300, Crestez Dan Leonard wrote:
> 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.
> 
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>

Looks good to me. Once you have addressed comments from Rafael you can
add my,

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

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

#1442386

On Wed, Jul 13, 2016 at 1:53 PM, Crestez Dan Leonard
<leonard.crestez@intel.com> wrote:
> 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 in order to differentiate between
> hardware variants try to replicate it when using ACPI with DT ids.
>
> This also makes it so that the i2c_device_id parameter passed to probe is
> non-NULL when matching with ACPI and DT ids.
>
> These patches are on top of linux-pm/linux-next. I delayed v2 until ACPI
> overlays got in in order to avoid conflicts. I tested using ACPI overlays but
> there is no actual dependency. This series just extends the PRP0001 feature to
> be more useful for I2C/SPI.
>
> The patches only touches the ACPI-specific parts of the i2c and spi core.
>
> Here is an example .dsl for an SPI accelerometer connected to minnowboard max:
>
> Device (ACCL)
> {
>     Name (_ADR, Zero)
>     Name (_HID, "PRP0001")
>     Name (_CID, "PRP0001")

This is bad ASL.

Generally, it is a bug to use the same device ID in both _HID and
_CID.  It will work, but it is incorrect.

Thanks,
Rafael

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

#1442497

On 07/13/2016 12:53 PM, Crestez Dan Leonard wrote:
> 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 in order to differentiate between
> hardware variants try to replicate it when using ACPI with DT ids.
>
> This also makes it so that the i2c_device_id parameter passed to probe is
> non-NULL when matching with ACPI and DT ids.
This solves a problem for me, thanks Leonard!  For the series (in case
it helps):

Tested-by: Dan O'Donovan <dan@emutex.com>

>
> These patches are on top of linux-pm/linux-next. I delayed v2 until ACPI
> overlays got in in order to avoid conflicts. I tested using ACPI overlays but
> there is no actual dependency. This series just extends the PRP0001 feature to
> be more useful for I2C/SPI.
>
> The patches only touches the ACPI-specific parts of the i2c and spi core.
>
> 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"},
> 	}
>     })
> }
>
> Link to v1: https://www.spinics.net/lists/linux-acpi/msg66469.html
> Changes:
>  * Rebase on after acpi overlays got it.
>  * Change acpi_of_modalias outlen param to size_t
>  * Use {} after else
>
> 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       | 11 ++++++++++-
>  include/acpi/acpi_bus.h |  1 +
>  4 files changed, 54 insertions(+), 2 deletions(-)
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web