Skip to contentSkip to conversation

Init device ids from ACPI of_compatible

PATCH In review Crestez Dan Leonard 9 participants 48 messages Jul 13, 2016 – Feb 9, 2017

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

v2 cover 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(-)
Signature · 1 line
2.7.4
#1 lore raw

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.

---
 drivers/acpi/bus.c      | 36 ++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |  1 +
 2 files changed, 37 insertions(+)
drivers/acpi/bus.c +36 -0
index cb9558e..77a1a80 100644
@@ -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)
 {
include/acpi/acpi_bus.h +1 -0
index 788c6c35..1d8ff0d 100644
@@ -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 *);
 
Signature · 6 lines
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#2 lore raw

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

Quoted 17 lines · Crestez Dan Leonard

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/acpi/bus.c      | 36 ++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |  1 +
 2 files changed, 37 insertions(+)
drivers/acpi/bus.c +0 -0
index cb9558e..77a1a80 100644
@@ -654,6 +654,42 @@ static bool acpi_of_match_device(struct acpi_device *adev,
        return false;

drivers/acpi/bus.c static bool acpi_of_match_device(struct acpi_device *adev, · line 664

Quoted 10 lines
drivers/acpi/bus.c +8 -0
 }
 
+/**
+ * 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."

Quoted 13 lines
drivers/acpi/bus.c +13 -0
+ *
+ * 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;
+

drivers/acpi/bus.c static bool acpi_of_match_device(struct acpi_device *adev, · line 687

Quoted 10 lines
drivers/acpi/bus.c +10 -0
+       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.

Quoted 13 lines
drivers/acpi/bus.c +5 -0
+
+       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)
 {
include/acpi/acpi_bus.h +0 -0
index 788c6c35..1d8ff0d 100644

include/acpi/acpi_bus.h void acpi_bus_trim(struct acpi_device *start);

Quoted 8 lines
include/acpi/acpi_bus.h +1 -1
@@ -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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Reviewed-by on v2 1/3
#3 lore raw

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

In reply to Crestez Dan Leonard
Quoted 6 lines · Crestez Dan Leonard

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.

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

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#4 lore raw

PATCH v2 2/3 acpi i2c: Initialize info.type from of_compatible

In reply to Crestez Dan Leonard

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.

---
 drivers/i2c/i2c-core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/i2c/i2c-core.c +7 -1
index 74e5aea..62a1339 100644
@@ -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;
 }
Signature · 6 lines
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#5 lore raw

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

Quoted 9 lines · Crestez Dan Leonard

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.

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

Quoted 14 lines
---
 drivers/i2c/i2c-core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/i2c/i2c-core.c +1 -1
index 74e5aea..62a1339 100644
@@ -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) {

drivers/i2c/i2c-core.c static int acpi_i2c_get_info(struct acpi_device *adev,

Quoted 10 lines
drivers/i2c/i2c-core.c +6 -1
+               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

Reviewed-by
#6 lore raw

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

Quoted 7 lines · Rafael J. Wysocki
Quoted 5 lines · Crestez Dan Leonard

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

Quoted 6 lines
Quoted 4 lines

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.

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

Looks good to me,

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#7 lore raw

PATCH v2 3/3 acpi spi: Initialize modalias from of_compatible

In reply to Crestez Dan Leonard

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.

---
 drivers/spi/spi.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/spi/spi.c +10 -1
index 7589c8a..49fdf6e 100644
@@ -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",
Signature · 1 line
2.7.4
#8 lore raw

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

Quoted 10 lines · Crestez Dan Leonard

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.

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

Quoted 23 lines
---
 drivers/spi/spi.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/spi/spi.c +10 -0
index 7589c8a..49fdf6e 100644
@@ -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)

drivers/spi/spi.c static acpi_status acpi_register_spi_device(struct spi_master *master,

Quoted 10 lines
drivers/spi/spi.c +0 -2
                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

Reviewed-by
#9 lore raw

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

Quoted 7 lines · Rafael J. Wysocki
Quoted 5 lines · Crestez Dan Leonard

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

Quoted 8 lines
Quoted 5 lines

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.

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

maintainer
#10 lore raw

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

In reply to Crestez Dan Leonard
Quoted 4 lines · Crestez Dan Leonard

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.

Quoted 3 lines

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

drivers/spi/spi.c static acpi_status acpi_register_spi_device(struct spi_master *master, · line 1687

Quoted 6 lines
drivers/spi/spi.c +6 -0
+	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.

#11 lore raw

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

Quoted 11 lines · Mark Brown
Quoted 4 lines · Crestez Dan Leonard

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: "?

Quoted 6 lines
Quoted 3 lines

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

Quoted 12 lines

drivers/spi/spi.c static acpi_status acpi_register_spi_device(struct spi_master *master, · line 1687

Quoted 6 lines
drivers/spi/spi.c +6 -0
+	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?

Signature · 2 lines
Regards,
Leonard
maintainer
#12 lore raw

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

Quoted 9 lines · Crestez Dan Leonard
Quoted 5 lines · Mark Brown

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.

Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
drivers/spi/spi.c +4 -0
+	if (adev->data.of_compatible) {
+		ret = acpi_of_modalias(adev, spi->modalias, sizeof(spi->modalias));
+		if (ret) {
+			spi_dev_put(spi);
Quoted 8 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+			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().

Quoted 2 lines

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.

#13 lore raw
In reply to Crestez Dan Leonard
Quoted 15 lines · Crestez Dan Leonard

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.

Quoted 7 lines

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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Tested-by on the v2 series
#14 lore raw
In reply to Crestez Dan Leonard
Quoted 7 lines · Crestez Dan Leonard

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):

Quoted 59 lines

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(-)
#15 lore raw

v3 cover Init device ids from ACPI of_compatible

In reply to Crestez Dan Leonard

(Re-submitting this patch set originally created by Leonard Crestez, possibly abandoned by Leonard due to a change in employment)

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.

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 (_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 v2: https://lkml.org/lkml/2016/7/13/392
Changes:
* Use appropriate subject prefix for each subsystem (Mark Brown)
* Use ACPI info as before if getting OF info fails (Mark Brown)
* Minor cosmetic/readability improvements (Rafael J. Wysocki)

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 / bus: Export acpi_of_modalias equiv of of_modalias_node
i2c: acpi: Initialize info.type from of_compatible
spi: acpi: Initialize modalias from of_compatible

 drivers/acpi/bus.c      | 35 +++++++++++++++++++++++++++++++++++
 drivers/i2c/i2c-core.c  |  8 +++++++-
 drivers/spi/spi.c       | 10 +++++++++-
 include/acpi/acpi_bus.h |  1 +
 4 files changed, 52 insertions(+), 2 deletions(-)
Signature · 1 line
2.7.4
#16 lore raw

PATCH v3 1/3 ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node

From: Crestez Dan Leonard <[email protected]>

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/acpi/bus.c      | 35 +++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |  1 +
 2 files changed, 36 insertions(+)
drivers/acpi/bus.c +35 -0
index 95855cb..8b9657f 100644
@@ -677,6 +677,41 @@ 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
+ *
+ * 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, ',');
+	strlcpy(outstr, chr ? chr + 1 : 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)
 {
include/acpi/acpi_bus.h +1 -0
index 4242c31..351b4a4 100644
@@ -522,6 +522,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 *);
 
Signature · 6 lines
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#17 lore raw

PATCH v3 2/3 i2c: acpi: Initialize info.type from of_compatible

In reply to Dan O'Donovan

From: Crestez Dan Leonard <[email protected]>

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.

---
 drivers/i2c/i2c-core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/i2c/i2c-core.c +7 -1
index 583e950..a83dbc7 100644
@@ -221,7 +221,13 @@ static int i2c_acpi_get_info(struct acpi_device *adev,
 
 	acpi_dev_free_resource_list(&resource_list);
 
-	strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
+	/*
+	 * Populate modalias from compatible property if available,
+	 * otherwise use native ACPI information
+	 */
+	if ((!adev->data.of_compatible) ||
+	    acpi_of_modalias(adev, info->type, sizeof(info->type)))
+		strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
 
 	return 0;
 }
Signature · 1 line
2.7.4
#18 lore raw

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

Quoted 10 lines · Dan O'Donovan

From: Crestez Dan Leonard <[email protected]>

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.

drivers/i2c/i2c-core.c static int i2c_acpi_get_info(struct acpi_device *adev, · line 229

Quoted 7 lines
drivers/i2c/i2c-core.c +6 -1
-       strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
+       /*
+        * Populate modalias from compatible property if available,
+        * otherwise use native ACPI information
+        */
+       if ((!adev->data.of_compatible) ||
+           acpi_of_modalias(adev, info->type, sizeof(info->type)))

Redundant parens in lvalue. After removing them is it possible to use one line for it?

Quoted 1 line

+ strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#19 lore raw

PATCH v3 3/3 spi: acpi: Initialize modalias from of_compatible

In reply to Dan O'Donovan

From: Crestez Dan Leonard <[email protected]>

When using devicetree spi_device.modalias is set to the compatible string with the vendor prefix removed. For SPI devices described via ACPI the spi_device.modalias 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.

---
 drivers/spi/spi.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/spi/spi.c +9 -1
index 656dd3e..4b562e8 100644
@@ -1722,13 +1722,21 @@ static acpi_status acpi_register_spi_device(struct spi_master *master,
 		return AE_OK;
 	}
 
+	/*
+	 * Populate modalias from compatible property if available,
+	 * otherwise use native ACPI information
+	 */
+	if ((!adev->data.of_compatible) ||
+	    acpi_of_modalias(adev, spi->modalias, sizeof(spi->modalias)))
+		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",
Signature · 6 lines
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#20 lore raw

Re: [PATCH v3 3/3] spi: acpi: Initialize modalias from of_compatible

Quoted 11 lines · Dan O'Donovan

From: Crestez Dan Leonard <[email protected]>

When using devicetree spi_device.modalias is set to the compatible string with the vendor prefix removed. For SPI devices described via ACPI the spi_device.modalias 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.

drivers/spi/spi.c static acpi_status acpi_register_spi_device(struct spi_master *master, · line 1730

Quoted 6 lines
drivers/spi/spi.c +6 -0
+       /*
+        * Populate modalias from compatible property if available,
+        * otherwise use native ACPI information
+        */
+       if ((!adev->data.of_compatible) ||
+           acpi_of_modalias(adev, spi->modalias, sizeof(spi->modalias)))

Same comment as in patch 2.

drivers/spi/spi.c static acpi_status acpi_register_spi_device(struct spi_master *master, · line 1732

Quoted 2 lines
drivers/spi/spi.c +2 -0
+               strlcpy(spi->modalias, acpi_device_hid(adev),
+                       sizeof(spi->modalias));

Could this be one line?

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#21 lore raw

Re: [PATCH v3 3/3] spi: acpi: Initialize modalias from of_compatible

Quoted 17 lines · Andy Shevchenko
Quoted 10 lines · Dan O'Donovan

From: Crestez Dan Leonard <[email protected]>

When using devicetree spi_device.modalias is set to the compatible string with the vendor prefix removed. For SPI devices described via ACPI the spi_device.modalias 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.

drivers/spi/spi.c static acpi_status acpi_register_spi_device(struct spi_master *master, · line 1730

Quoted 6 lines
drivers/spi/spi.c +6 -0
+       /*
+        * Populate modalias from compatible property if available,
+        * otherwise use native ACPI information
+        */
+       if ((!adev->data.of_compatible) ||
+           acpi_of_modalias(adev, spi->modalias, sizeof(spi->modalias)))
Quoted 1 line

Same comment as in patch 2.

Thanks for the feedback, Andy. The check on of_compatible is redundant, because its repeated in acpi_of_modalias(), so I'll remove it here (and in patch 2) to reduce this to one line. v4 on the way.

Quoted 3 lines

drivers/spi/spi.c static acpi_status acpi_register_spi_device(struct spi_master *master, · line 1732

Quoted 2 lines
drivers/spi/spi.c +2 -0
+               strlcpy(spi->modalias, acpi_device_hid(adev),
+                       sizeof(spi->modalias));

Could this be one line?

I couldn't see a way to reduce this to one line without exceeding 80 chars or adding another line somewhere else, so I'll leave this one as it is if that's ok.

#22 lore raw

Re: [PATCH v3 3/3] spi: acpi: Initialize modalias from of_compatible

Quoted 9 lines · Dan O'Donovan
Quoted 5 lines · Andy Shevchenko

drivers/spi/spi.c static acpi_status acpi_register_spi_device(struct spi_master *master, · line 1732

Quoted 2 lines · Dan O'Donovan
drivers/spi/spi.c +2 -0
+               strlcpy(spi->modalias, acpi_device_hid(adev),
+                       sizeof(spi->modalias));

Could this be one line?

I couldn't see a way to reduce this to one line without exceeding 80 chars or adding another line somewhere else, so I'll leave this one as it is if that's ok.

80 characters per line is not carved in stone. How many do you have over? 3 like "s);"?

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#23 lore raw

Re: [PATCH v3 3/3] spi: acpi: Initialize modalias from of_compatible

In reply to Dan O'Donovan
Quoted 35 lines · Dan O'Donovan

From: Crestez Dan Leonard <[email protected]>

When using devicetree spi_device.modalias is set to the compatible string with the vendor prefix removed. For SPI devices described via ACPI the spi_device.modalias 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.

---
 drivers/spi/spi.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/spi/spi.c +9 -0
index 656dd3e..4b562e8 100644
@@ -1722,13 +1722,21 @@ static acpi_status acpi_register_spi_device(struct spi_master *master,
                return AE_OK;
        }
 
+       /*
+        * Populate modalias from compatible property if available,
+        * otherwise use native ACPI information
+        */
+       if ((!adev->data.of_compatible) ||
+           acpi_of_modalias(adev, spi->modalias, sizeof(spi->modalias)))
+               strlcpy(spi->modalias, acpi_device_hid(adev),
+                       sizeof(spi->modalias));
+
        if (spi->irq < 0)

drivers/spi/spi.c static acpi_status acpi_register_spi_device(struct spi_master *master,

Quoted 10 lines
drivers/spi/spi.c +0 -2
                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",
--

Mark, any objections here?

Thanks,
Rafael

maintainer
#24 lore raw

Re: [PATCH v3 3/3] spi: acpi: Initialize modalias from of_compatible

Quoted 3 lines · Rafael J. Wysocki

On Mon, Jan 23, 2017 at 5:08 PM, Dan O'Donovan <[email protected]> wrote:

Mark, any objections here?

I've not looked yet, please allow a reasonable time for review (there still seemed to be ongoing discussion at the ACPI level). I continue to be concerned about the amount of duplication and special casing that the various firmware paths seem to entail.

#25 lore raw

Re: [PATCH v3 3/3] spi: acpi: Initialize modalias from of_compatible

Quoted 6 lines · Mark Brown
Quoted 3 lines · Rafael J. Wysocki

On Mon, Jan 23, 2017 at 5:08 PM, Dan O'Donovan <[email protected]> wrote:

Mark, any objections here?

I've not looked yet, please allow a reasonable time for review

Sure, no problem at all.

Quoted 1 line

(there still seemed to be ongoing discussion at the ACPI level).

Honestly, I'm not sure what you mean.

The first patch in the series (which is the ACPI level here) does not seem to be controversial at all and the i2c one has been acked already.

Quoted 2 lines

I continue to be concerned about the amount of duplication and special casing that the various firmware paths seem to entail.

Well, this particular change is relatively straightforward and one can argue that it fixes a bug. It does address a problem for sure.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Reviewed-by on the v3 series
#26 lore raw
In reply to Dan O'Donovan
Quoted 8 lines · Dan O'Donovan

(Re-submitting this patch set originally created by Leonard Crestez, possibly abandoned by Leonard due to a change in employment)

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.

Quoted 7 lines

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

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.

The series looks good. Few nitpicks in the comments of corresponding patches though.
FWIW:

Quoted 60 lines

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

Device (ACCL)

{
    Name (_ADR, Zero)
    Name (_HID, "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 v2: https://lkml.org/lkml/2016/7/13/392
Changes:
* Use appropriate subject prefix for each subsystem (Mark Brown)
* Use ACPI info as before if getting OF info fails (Mark Brown)
* Minor cosmetic/readability improvements (Rafael J. Wysocki)

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 / bus: Export acpi_of_modalias equiv of of_modalias_node
i2c: acpi: Initialize info.type from of_compatible
spi: acpi: Initialize modalias from of_compatible

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

--
2.7.4

Signature · 2 lines
With Best Regards,
Andy Shevchenko
Reviewed-by Tested-by on the v3 series
#27 lore raw
In reply to Dan O'Donovan
Quoted 66 lines · Dan O'Donovan

(Re-submitting this patch set originally created by Leonard Crestez, possibly abandoned by Leonard due to a change in employment)

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.

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 (_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 v2: https://lkml.org/lkml/2016/7/13/392
Changes:
* Use appropriate subject prefix for each subsystem (Mark Brown)
* Use ACPI info as before if getting OF info fails (Mark Brown)
* Minor cosmetic/readability improvements (Rafael J. Wysocki)

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 / bus: Export acpi_of_modalias equiv of of_modalias_node
i2c: acpi: Initialize info.type from of_compatible
spi: acpi: Initialize modalias from of_compatible

Quoted 6 lines
 drivers/acpi/bus.c      | 35 +++++++++++++++++++++++++++++++++++
 drivers/i2c/i2c-core.c  |  8 +++++++-
 drivers/spi/spi.c       | 10 +++++++++-
 include/acpi/acpi_bus.h |  1 +
 4 files changed, 52 insertions(+), 2 deletions(-)

After addressing comments from Andy

---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.

#28 lore raw
Quoted 4 lines · Jarkko Nikula

After addressing comments from Andy

Quoted 8 lines

---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.

Ai-ai-ai ^^^

Signature · 6 lines
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#29 lore raw

v4 cover Init device ids from ACPI of_compatible

In reply to Dan O'Donovan

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.

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 (_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 v3: http://www.spinics.net/lists/linux-acpi/msg71531.html
Changes:
* Minor cosmetic/readability improvements (Andy Shevchenko)

Link to v2: https://lkml.org/lkml/2016/7/13/392
Changes:
* Use appropriate subject prefix for each subsystem (Mark Brown)
* Use ACPI info as before if getting OF info fails (Mark Brown)
* Minor cosmetic/readability improvements (Rafael J. Wysocki)

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 / bus: Export acpi_of_modalias equiv of of_modalias_node
i2c: acpi: Initialize info.type from of_compatible
spi: acpi: Initialize modalias from of_compatible

 drivers/acpi/bus.c      | 35 +++++++++++++++++++++++++++++++++++
 drivers/i2c/i2c-core.c  |  7 ++++++-
 drivers/spi/spi.c       |  9 ++++++++-
 include/acpi/acpi_bus.h |  1 +
 4 files changed, 50 insertions(+), 2 deletions(-)
Signature · 1 line
2.7.4
#30 lore raw

PATCH v4 1/3 ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node

From: Crestez Dan Leonard <[email protected]>

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/acpi/bus.c      | 35 +++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |  1 +
 2 files changed, 36 insertions(+)
drivers/acpi/bus.c +35 -0
index 95855cb..8b9657f 100644
@@ -677,6 +677,41 @@ 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
+ *
+ * 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, ',');
+	strlcpy(outstr, chr ? chr + 1 : 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)
 {
include/acpi/acpi_bus.h +1 -0
index 4242c31..351b4a4 100644
@@ -522,6 +522,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 *);
 
Signature · 1 line
2.7.4
#31 lore raw

Re: [PATCH v4 1/3] ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node

Quoted 7 lines · Dan O'Donovan

From: Crestez Dan Leonard <[email protected]>

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.

Quoted 6 lines

First off, the original author of the patch doesn't seem to be reachable under the Intel address any more, so his S-o-b is meaningless and it shouldn't be there. In such cases you can send the patch as From: you and give a credit to the original author in the changelog.

Second, as Mark said ->

Quoted 16 lines
---
 drivers/acpi/bus.c      | 35 +++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |  1 +
 2 files changed, 36 insertions(+)
drivers/acpi/bus.c +3 -0
index 95855cb..8b9657f 100644
@@ -677,6 +677,41 @@ 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.

drivers/acpi/bus.c static bool acpi_of_match_device(struct acpi_device *adev, · line 692

Quoted 10 lines
drivers/acpi/bus.c +10 -0
+ * @outstr:	Pointer to buffer for result
+ * @outlen:	Length of outstr value
+ *
+ * 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)

-> this could be a bool function and you could provide something like acpi_set_modalias() doing the entire

if (!acpi_of_modalias(adev, outstr, outlen))

               strlcpy(outstr, dev_name(&adev->dev), outlen);

thing, so that its users don't have to duplicate this conditional.

And then acpi_of_modalias() could be static and the only thing to export would be acpi_set_modalias().

Quoted 30 lines
drivers/acpi/bus.c +22 -0
+{
+	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, ',');
+	strlcpy(outstr, chr ? chr + 1 : 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)
 {
include/acpi/acpi_bus.h +0 -0
index 4242c31..351b4a4 100644

include/acpi/acpi_bus.h void acpi_bus_trim(struct acpi_device *start); · line 527

Quoted 6 lines
include/acpi/acpi_bus.h +1 -0
@@ -522,6 +522,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

#32 lore raw

Re: [PATCH v4 1/3] ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node

Quoted 50 lines · Rafael J. Wysocki
Quoted 7 lines · Dan O'Donovan

From: Crestez Dan Leonard <[email protected]>

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.

Quoted 6 lines

First off, the original author of the patch doesn't seem to be reachable under the Intel address any more, so his S-o-b is meaningless and it shouldn't be there. In such cases you can send the patch as From: you and give a credit to the original author in the changelog.

Second, as Mark said ->

Quoted 16 lines
---
 drivers/acpi/bus.c      | 35 +++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |  1 +
 2 files changed, 36 insertions(+)
drivers/acpi/bus.c +3 -0
index 95855cb..8b9657f 100644
@@ -677,6 +677,41 @@ 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.

drivers/acpi/bus.c static bool acpi_of_match_device(struct acpi_device *adev, · line 692

Quoted 10 lines
drivers/acpi/bus.c +10 -0
+ * @outstr:	Pointer to buffer for result
+ * @outlen:	Length of outstr value
+ *
+ * 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)

-> this could be a bool function and you could provide something like acpi_set_modalias() doing the entire

Quoted 7 lines

if (!acpi_of_modalias(adev, outstr, outlen))

               strlcpy(outstr, dev_name(&adev->dev), outlen);

thing, so that its users don't have to duplicate this conditional.

And then acpi_of_modalias() could be static and the only thing to export would be acpi_set_modalias().

Thanks Mark and Rafael for the latest feedback. Those suggestions make sense. I'll send an updated patch-set soon.

Quoted 41 lines
Quoted 31 lines
drivers/acpi/bus.c +22 -0
+{
+	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, ',');
+	strlcpy(outstr, chr ? chr + 1 : 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)
 {
include/acpi/acpi_bus.h +0 -0
index 4242c31..351b4a4 100644

include/acpi/acpi_bus.h void acpi_bus_trim(struct acpi_device *start); · line 527

Quoted 6 lines
include/acpi/acpi_bus.h +1 -0
@@ -522,6 +522,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

#33 lore raw

PATCH v4 2/3 i2c: acpi: Initialize info.type from of_compatible

In reply to Dan O'Donovan

From: Crestez Dan Leonard <[email protected]>

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.

---
 drivers/i2c/i2c-core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/i2c/i2c-core.c +6 -1
index 583e950..6e99431 100644
@@ -221,7 +221,12 @@ static int i2c_acpi_get_info(struct acpi_device *adev,
 
 	acpi_dev_free_resource_list(&resource_list);
 
-	strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
+	/*
+	 * Populate modalias from compatible property if available,
+	 * otherwise use native ACPI information
+	 */
+	if (acpi_of_modalias(adev, info->type, sizeof(info->type)))
+		strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
 
 	return 0;
 }
Signature · 1 line
2.7.4
Acked-by on v4 2/3
#34 lore raw

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

Quoted 7 lines · Dan O'Donovan

From: Crestez Dan Leonard <[email protected]>

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

Quoted 8 lines

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.

I assume this shall go in as one series via ACPI tree, so:

#35 lore raw

PATCH v4 3/3 spi: acpi: Initialize modalias from of_compatible

In reply to Dan O'Donovan

From: Crestez Dan Leonard <[email protected]>

When using devicetree spi_device.modalias is set to the compatible string with the vendor prefix removed. For SPI devices described via ACPI the spi_device.modalias 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.

---
 drivers/spi/spi.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/spi/spi.c +8 -1
index 656dd3e..9a8c880 100644
@@ -1722,13 +1722,20 @@ static acpi_status acpi_register_spi_device(struct spi_master *master,
 		return AE_OK;
 	}
 
+	/*
+	 * Populate modalias from compatible property if available,
+	 * otherwise use native ACPI information
+	 */
+	if (acpi_of_modalias(adev, spi->modalias, sizeof(spi->modalias)))
+		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",
Signature · 1 line
2.7.4
maintainer
#36 lore raw

Re: [PATCH v4 3/3] spi: acpi: Initialize modalias from of_compatible

drivers/spi/spi.c static acpi_status acpi_register_spi_device(struct spi_master *master, · line 1731

Quoted 3 lines · Dan O'Donovan
drivers/spi/spi.c +3 -0
+	if (acpi_of_modalias(adev, spi->modalias, sizeof(spi->modalias)))
+		strlcpy(spi->modalias, acpi_device_hid(adev),
+			sizeof(spi->modalias));

I can't help but think that this would be simpler to use if the interface were something like acpi_set_modalias() and it did the fallback to the ACPI HID internally. It'd both avoid a small amount of code duplication and make the code read a bit more naturally - I have to think to realise that this means "if there isn't an OF name use the HID" since it looks like a boolean function. Then again just putting a != 0 in the test would have the same effect probably.

#37 lore raw

v5 cover Init device ids from ACPI of_compatible

In reply to Dan O'Donovan

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.

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

Credit to Leonard Crestez for the original version of this patch set.

Link to v4: http://www.spinics.net/lists/linux-acpi/msg71657.html
Changes:
* add acpi_set_modalias wrapper to handle fallback to ACPI ID (Mark Brown)

Link to v3: http://www.spinics.net/lists/linux-acpi/msg71531.html
Changes:
* Minor cosmetic/readability improvements (Andy Shevchenko)

Link to v2: https://lkml.org/lkml/2016/7/13/392
Changes:
* Use appropriate subject prefix for each subsystem (Mark Brown)
* Use ACPI info as before if getting OF info fails (Mark Brown)
* Minor cosmetic/readability improvements (Rafael J. Wysocki)

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

Dan O'Donovan (3):
ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node
i2c: acpi: Initialize info.type from of_compatible
spi: acpi: Initialize modalias from of_compatible

 drivers/acpi/bus.c      | 42 ++++++++++++++++++++++++++++++++++++++++++
 drivers/i2c/i2c-core.c  |  3 ++-
 drivers/spi/spi.c       |  4 +++-
 include/acpi/acpi_bus.h |  2 ++
 4 files changed, 49 insertions(+), 2 deletions(-)
Signature · 1 line
2.7.4
#38 lore raw

PATCH v5 1/3 ACPI / bus: 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.

---
 drivers/acpi/bus.c      | 42 ++++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |  2 ++
 2 files changed, 44 insertions(+)
drivers/acpi/bus.c +42 -0
index 95855cb..80cb5eb 100644
@@ -677,6 +677,48 @@ static bool acpi_of_match_device(struct acpi_device *adev,
 	return false;
 }
 
+static bool acpi_of_modalias(struct acpi_device *adev,
+			     char *modalias, size_t len)
+{
+	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 false;
+
+	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, ',');
+	strlcpy(modalias, chr ? chr + 1 : str, len);
+
+	return true;
+}
+
+/**
+ * acpi_set_modalias - Set modalias using "compatible" property or supplied ID
+ * @adev:	ACPI device object to match
+ * @default_id:	ID string to use as default if no compatible string found
+ * @modalias:   Pointer to buffer that modalias value will be copied into
+ * @len:	Length of modalias buffer
+ *
+ * This is a counterpart of of_modalias_node() for struct acpi_device objects.
+ * If there is a compatible string for @adev, it will be copied to @modalias
+ * with the vendor prefix stripped; otherwise, @default_id will be used.
+ */
+void acpi_set_modalias(struct acpi_device *adev, const char *default_id,
+		       char *modalias, size_t len)
+{
+	if (!acpi_of_modalias(adev, modalias, len))
+		strlcpy(modalias, default_id, len);
+}
+EXPORT_SYMBOL_GPL(acpi_set_modalias);
+
 static bool __acpi_match_device_cls(const struct acpi_device_id *id,
 				    struct acpi_hardware_id *hwid)
 {
include/acpi/acpi_bus.h +2 -0
index 4242c31..ef0ae8a 100644
@@ -522,6 +522,8 @@ 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);
+void acpi_set_modalias(struct acpi_device *adev, const char *default_id,
+		       char *modalias, size_t len);
 int acpi_create_dir(struct acpi_device *);
 void acpi_remove_dir(struct acpi_device *);
 
Signature · 1 line
2.7.4
#39 lore raw

PATCH v5 2/3 i2c: acpi: Initialize info.type from of_compatible

In reply to Dan O'Donovan

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.

---
 drivers/i2c/i2c-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/i2c/i2c-core.c +2 -1
index 583e950..28d2181 100644
@@ -221,7 +221,8 @@ static int i2c_acpi_get_info(struct acpi_device *adev,
 
 	acpi_dev_free_resource_list(&resource_list);
 
-	strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
+	acpi_set_modalias(adev, dev_name(&adev->dev),
+			  info->type, sizeof(info->type));
 
 	return 0;
 }
Signature · 6 lines
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#40 lore raw

PATCH v5 3/3 spi: acpi: Initialize modalias from of_compatible

In reply to Dan O'Donovan

When using devicetree spi_device.modalias is set to the compatible string with the vendor prefix removed. For SPI devices described via ACPI the spi_device.modalias 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.

---
 drivers/spi/spi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/spi/spi.c +3 -1
index 656dd3e..d9afc32 100644
@@ -1722,13 +1722,15 @@ static acpi_status acpi_register_spi_device(struct spi_master *master,
 		return AE_OK;
 	}
 
+	acpi_set_modalias(adev, acpi_device_hid(adev),
+			  spi->modalias, 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",
Signature · 1 line
2.7.4
maintainer Acked-by on v5 3/3
#41 lore raw

Re: [PATCH v5 3/3] spi: acpi: Initialize modalias from of_compatible

drivers/spi/spi.c static acpi_status acpi_register_spi_device(struct spi_master *master, · line 1727

Quoted 3 lines · Dan O'Donovan
drivers/spi/spi.c +3 -0
+	acpi_set_modalias(adev, acpi_device_hid(adev),
+			  spi->modalias, sizeof(spi->modalias));
+

The formatting here is really weird (why isn't the third argument on the line above?) but otherwise

#42 lore raw

v6 cover Init device ids from ACPI of_compatible

In reply to Dan O'Donovan

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.

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

Credit to Leonard Crestez for the original version of this patch set.

Link to v5: http://www.spinics.net/lists/linux-acpi/msg71902.html
Changes:
* Change formatting of arguments passed to acpi_set_modalias (Mark Brown)

Link to v4: http://www.spinics.net/lists/linux-acpi/msg71657.html
Changes:
* add acpi_set_modalias wrapper to handle fallback to ACPI ID (Mark Brown)

Link to v3: http://www.spinics.net/lists/linux-acpi/msg71531.html
Changes:
* Minor cosmetic/readability improvements (Andy Shevchenko)

Link to v2: https://lkml.org/lkml/2016/7/13/392
Changes:
* Use appropriate subject prefix for each subsystem (Mark Brown)
* Use ACPI info as before if getting OF info fails (Mark Brown)
* Minor cosmetic/readability improvements (Rafael J. Wysocki)

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

Dan O'Donovan (3):
ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node
i2c: acpi: Initialize info.type from of_compatible
spi: acpi: Initialize modalias from of_compatible

 drivers/acpi/bus.c      | 42 ++++++++++++++++++++++++++++++++++++++++++
 drivers/i2c/i2c-core.c  |  3 ++-
 drivers/spi/spi.c       |  4 +++-
 include/acpi/acpi_bus.h |  2 ++
 4 files changed, 49 insertions(+), 2 deletions(-)
Signature · 1 line
2.7.4
#43 lore raw

PATCH v6 1/3 ACPI / bus: 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.

---
 drivers/acpi/bus.c      | 42 ++++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |  2 ++
 2 files changed, 44 insertions(+)
drivers/acpi/bus.c +42 -0
index 95855cb..80cb5eb 100644
@@ -677,6 +677,48 @@ static bool acpi_of_match_device(struct acpi_device *adev,
 	return false;
 }
 
+static bool acpi_of_modalias(struct acpi_device *adev,
+			     char *modalias, size_t len)
+{
+	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 false;
+
+	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, ',');
+	strlcpy(modalias, chr ? chr + 1 : str, len);
+
+	return true;
+}
+
+/**
+ * acpi_set_modalias - Set modalias using "compatible" property or supplied ID
+ * @adev:	ACPI device object to match
+ * @default_id:	ID string to use as default if no compatible string found
+ * @modalias:   Pointer to buffer that modalias value will be copied into
+ * @len:	Length of modalias buffer
+ *
+ * This is a counterpart of of_modalias_node() for struct acpi_device objects.
+ * If there is a compatible string for @adev, it will be copied to @modalias
+ * with the vendor prefix stripped; otherwise, @default_id will be used.
+ */
+void acpi_set_modalias(struct acpi_device *adev, const char *default_id,
+		       char *modalias, size_t len)
+{
+	if (!acpi_of_modalias(adev, modalias, len))
+		strlcpy(modalias, default_id, len);
+}
+EXPORT_SYMBOL_GPL(acpi_set_modalias);
+
 static bool __acpi_match_device_cls(const struct acpi_device_id *id,
 				    struct acpi_hardware_id *hwid)
 {
include/acpi/acpi_bus.h +2 -0
index 4242c31..ef0ae8a 100644
@@ -522,6 +522,8 @@ 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);
+void acpi_set_modalias(struct acpi_device *adev, const char *default_id,
+		       char *modalias, size_t len);
 int acpi_create_dir(struct acpi_device *);
 void acpi_remove_dir(struct acpi_device *);
 
Signature · 1 line
2.7.4
#44 lore raw

PATCH v6 2/3 i2c: acpi: Initialize info.type from of_compatible

In reply to Dan O'Donovan

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.

---
 drivers/i2c/i2c-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/i2c/i2c-core.c +2 -1
index 583e950..1e52395 100644
@@ -221,7 +221,8 @@ static int i2c_acpi_get_info(struct acpi_device *adev,
 
 	acpi_dev_free_resource_list(&resource_list);
 
-	strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
+	acpi_set_modalias(adev, dev_name(&adev->dev), info->type,
+			  sizeof(info->type));
 
 	return 0;
 }
Signature · 1 line
2.7.4
#45 lore raw

PATCH v6 3/3 spi: acpi: Initialize modalias from of_compatible

In reply to Dan O'Donovan

When using devicetree spi_device.modalias is set to the compatible string with the vendor prefix removed. For SPI devices described via ACPI the spi_device.modalias 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.

---
 drivers/spi/spi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/spi/spi.c +3 -1
index 656dd3e..ad7f638 100644
@@ -1722,13 +1722,15 @@ static acpi_status acpi_register_spi_device(struct spi_master *master,
 		return AE_OK;
 	}
 
+	acpi_set_modalias(adev, acpi_device_hid(adev), spi->modalias,
+			  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",
Signature · 1 line
2.7.4
maintainer Acked-by on v6 3/3
#46 lore raw

Re: [PATCH v6 3/3] spi: acpi: Initialize modalias from of_compatible

Quoted 4 lines · Dan O'Donovan

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

#47 lore raw
In reply to Dan O'Donovan
Quoted 72 lines · Dan O'Donovan

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.

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

Credit to Leonard Crestez for the original version of this patch set.

Link to v5: http://www.spinics.net/lists/linux-acpi/msg71902.html
Changes:
* Change formatting of arguments passed to acpi_set_modalias (Mark Brown)

Link to v4: http://www.spinics.net/lists/linux-acpi/msg71657.html
Changes:
* add acpi_set_modalias wrapper to handle fallback to ACPI ID (Mark Brown)

Link to v3: http://www.spinics.net/lists/linux-acpi/msg71531.html
Changes:
* Minor cosmetic/readability improvements (Andy Shevchenko)

Link to v2: https://lkml.org/lkml/2016/7/13/392
Changes:
* Use appropriate subject prefix for each subsystem (Mark Brown)
* Use ACPI info as before if getting OF info fails (Mark Brown)
* Minor cosmetic/readability improvements (Rafael J. Wysocki)

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

Quoted 4 lines

Dan O'Donovan (3):
ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node
i2c: acpi: Initialize info.type from of_compatible
spi: acpi: Initialize modalias from of_compatible

All [1-3/3] applied.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html