linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver
@ 2022-01-21 17:24 Stefan Binding
  2022-01-21 17:24 ` [PATCH v6 1/9] spi: Make spi_alloc_device and spi_add_device public again Stefan Binding
                   ` (11 more replies)
  0 siblings, 12 replies; 29+ messages in thread
From: Stefan Binding @ 2022-01-21 17:24 UTC (permalink / raw)
  To: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches, Stefan Binding

Add support for SPI bus in the i2c-multi-instantiate driver as
upcoming laptops will need to multi instantiate SPI devices from
a single device node, which has multiple SpiSerialBus entries at
the ACPI table.

With the new SPI support, i2c-multi-instantiate becomes
bus-multi-instantiate and is moved to the ACPI folder.

The intention is to support the SPI bus by re-using the current
I2C multi instantiate, instead of creating a new SPI multi
instantiate, to make it possible for peripherals that can be
controlled by I2C or SPI to have the same HID at the ACPI table.

The new driver (serial multi instantiate, smi) checks for the
hard-coded bus type and returns -ENODEV in case of zero devices
found for that bus. In the case of automatic bus detection, 
the driver will give preference to I2C.

The expectation is for a device node in the ACPI table to have
multiple I2cSerialBus only or multiple SpiSerialBus only, not
a mix of both; and for the case where there are both entries in
one device node, only the I2C ones would be probed.

This new serial multi instantiate will be used in CS35L41 HDA new
driver.

Changes since V5:
 - comment, commit message and Kconfig description fixes
 - minor fixes in serial-multi-instantiate
 - use lowercase for SSIDs in patch_realtek.c

Lucas Tanure (4):
  platform/x86: i2c-multi-instantiate: Rename it for a generic serial
    driver name
  platform/x86: serial-multi-instantiate: Reorganize I2C functions
  ALSA: hda/realtek: Add support for HP Laptops
  ACPI / scan: Create platform device for CS35L41

Stefan Binding (5):
  spi: Make spi_alloc_device and spi_add_device public again
  spi: Create helper API to lookup ACPI info for spi device
  spi: Support selection of the index of the ACPI Spi Resource before
    alloc
  spi: Add API to count spi acpi resources
  platform/x86: serial-multi-instantiate: Add SPI support

 MAINTAINERS                                   |   4 +-
 drivers/acpi/scan.c                           |  16 +-
 drivers/platform/x86/Kconfig                  |  12 +-
 drivers/platform/x86/Makefile                 |   2 +-
 drivers/platform/x86/i2c-multi-instantiate.c  | 174 ---------
 .../platform/x86/serial-multi-instantiate.c   | 349 ++++++++++++++++++
 drivers/spi/spi.c                             | 137 ++++++-
 include/linux/spi/spi.h                       |  20 +
 sound/pci/hda/patch_realtek.c                 |  43 ++-
 9 files changed, 551 insertions(+), 206 deletions(-)
 delete mode 100644 drivers/platform/x86/i2c-multi-instantiate.c
 create mode 100644 drivers/platform/x86/serial-multi-instantiate.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 1/9] spi: Make spi_alloc_device and spi_add_device public again
  2022-01-21 17:24 [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Stefan Binding
@ 2022-01-21 17:24 ` Stefan Binding
  2022-02-01 14:26   ` Hans de Goede
  2022-01-21 17:24 ` [PATCH v6 2/9] spi: Create helper API to lookup ACPI info for spi device Stefan Binding
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 29+ messages in thread
From: Stefan Binding @ 2022-01-21 17:24 UTC (permalink / raw)
  To: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches, Stefan Binding

This functions were previously made private since they
were not used. However, these functions will be needed
again.

Partial revert of commit da21fde0fdb3
("spi: Make several public functions private to spi.c")

Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 drivers/spi/spi.c       |  6 ++++--
 include/linux/spi/spi.h | 12 ++++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 4599b121d744..1eb84101c4ad 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -532,7 +532,7 @@ static DEFINE_MUTEX(board_lock);
  *
  * Return: a pointer to the new device, or NULL.
  */
-static struct spi_device *spi_alloc_device(struct spi_controller *ctlr)
+struct spi_device *spi_alloc_device(struct spi_controller *ctlr)
 {
 	struct spi_device	*spi;
 
@@ -557,6 +557,7 @@ static struct spi_device *spi_alloc_device(struct spi_controller *ctlr)
 	device_initialize(&spi->dev);
 	return spi;
 }
+EXPORT_SYMBOL_GPL(spi_alloc_device);
 
 static void spi_dev_set_name(struct spi_device *spi)
 {
@@ -652,7 +653,7 @@ static int __spi_add_device(struct spi_device *spi)
  *
  * Return: 0 on success; negative errno on failure
  */
-static int spi_add_device(struct spi_device *spi)
+int spi_add_device(struct spi_device *spi)
 {
 	struct spi_controller *ctlr = spi->controller;
 	struct device *dev = ctlr->dev.parent;
@@ -673,6 +674,7 @@ static int spi_add_device(struct spi_device *spi)
 	mutex_unlock(&ctlr->add_lock);
 	return status;
 }
+EXPORT_SYMBOL_GPL(spi_add_device);
 
 static int spi_add_device_locked(struct spi_device *spi)
 {
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 7ab3fed7b804..0346a3ff27fd 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -1452,7 +1452,19 @@ spi_register_board_info(struct spi_board_info const *info, unsigned n)
  * use spi_new_device() to describe each device.  You can also call
  * spi_unregister_device() to start making that device vanish, but
  * normally that would be handled by spi_unregister_controller().
+ *
+ * You can also use spi_alloc_device() and spi_add_device() to use a two
+ * stage registration sequence for each spi_device. This gives the caller
+ * some more control over the spi_device structure before it is registered,
+ * but requires that caller to initialize fields that would otherwise
+ * be defined using the board info.
  */
+extern struct spi_device *
+spi_alloc_device(struct spi_controller *ctlr);
+
+extern int
+spi_add_device(struct spi_device *spi);
+
 extern struct spi_device *
 spi_new_device(struct spi_controller *, struct spi_board_info *);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 2/9] spi: Create helper API to lookup ACPI info for spi device
  2022-01-21 17:24 [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Stefan Binding
  2022-01-21 17:24 ` [PATCH v6 1/9] spi: Make spi_alloc_device and spi_add_device public again Stefan Binding
@ 2022-01-21 17:24 ` Stefan Binding
  2022-02-01 14:28   ` Hans de Goede
  2022-01-21 17:24 ` [PATCH v6 3/9] spi: Support selection of the index of the ACPI Spi Resource before alloc Stefan Binding
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 29+ messages in thread
From: Stefan Binding @ 2022-01-21 17:24 UTC (permalink / raw)
  To: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches, Stefan Binding

This can then be used to find a spi resource inside an
ACPI node, and allocate a spi device.

Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 drivers/spi/spi.c       | 46 ++++++++++++++++++++++++++++++++---------
 include/linux/spi/spi.h |  6 ++++++
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 1eb84101c4ad..13f4701f0694 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2410,8 +2410,18 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)
 	return 1;
 }
 
-static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
-					    struct acpi_device *adev)
+/**
+ * acpi_spi_device_alloc - Allocate a spi device, and fill it in with ACPI information
+ * @ctlr: controller to which the spi device belongs
+ * @adev: ACPI Device for the spi device
+ *
+ * This should be used to allocate a new spi device from and ACPI Node.
+ * The caller is responsible for calling spi_add_device to register the spi device.
+ *
+ * Return: a pointer to the new device, or ERR_PTR on error.
+ */
+struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
+					 struct acpi_device *adev)
 {
 	acpi_handle parent_handle = NULL;
 	struct list_head resource_list;
@@ -2419,10 +2429,6 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
 	struct spi_device *spi;
 	int ret;
 
-	if (acpi_bus_get_status(adev) || !adev->status.present ||
-	    acpi_device_enumerated(adev))
-		return AE_OK;
-
 	lookup.ctlr		= ctlr;
 	lookup.irq		= -1;
 
@@ -2433,7 +2439,7 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
 
 	if (ret < 0)
 		/* found SPI in _CRS but it points to another controller */
-		return AE_OK;
+		return ERR_PTR(-ENODEV);
 
 	if (!lookup.max_speed_hz &&
 	    ACPI_SUCCESS(acpi_get_parent(adev->handle, &parent_handle)) &&
@@ -2443,16 +2449,15 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
 	}
 
 	if (!lookup.max_speed_hz)
-		return AE_OK;
+		return ERR_PTR(-ENODEV);
 
 	spi = spi_alloc_device(ctlr);
 	if (!spi) {
 		dev_err(&ctlr->dev, "failed to allocate SPI device for %s\n",
 			dev_name(&adev->dev));
-		return AE_NO_MEMORY;
+		return ERR_PTR(-ENOMEM);
 	}
 
-
 	ACPI_COMPANION_SET(&spi->dev, adev);
 	spi->max_speed_hz	= lookup.max_speed_hz;
 	spi->mode		|= lookup.mode;
@@ -2460,6 +2465,27 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
 	spi->bits_per_word	= lookup.bits_per_word;
 	spi->chip_select	= lookup.chip_select;
 
+	return spi;
+}
+EXPORT_SYMBOL_GPL(acpi_spi_device_alloc);
+
+static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
+					    struct acpi_device *adev)
+{
+	struct spi_device *spi;
+
+	if (acpi_bus_get_status(adev) || !adev->status.present ||
+	    acpi_device_enumerated(adev))
+		return AE_OK;
+
+	spi = acpi_spi_device_alloc(ctlr, adev);
+	if (IS_ERR(spi)) {
+		if (PTR_ERR(spi) == -ENOMEM)
+			return AE_NO_MEMORY;
+		else
+			return AE_OK;
+	}
+
 	acpi_set_modalias(adev, acpi_device_hid(adev), spi->modalias,
 			  sizeof(spi->modalias));
 
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 0346a3ff27fd..d159cef12f1a 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -16,6 +16,7 @@
 #include <linux/gpio/consumer.h>
 
 #include <uapi/linux/spi/spi.h>
+#include <linux/acpi.h>
 
 struct dma_chan;
 struct software_node;
@@ -759,6 +760,11 @@ extern int devm_spi_register_controller(struct device *dev,
 					struct spi_controller *ctlr);
 extern void spi_unregister_controller(struct spi_controller *ctlr);
 
+#if IS_ENABLED(CONFIG_ACPI)
+extern struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
+						struct acpi_device *adev);
+#endif
+
 /*
  * SPI resource management while processing a SPI message
  */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 3/9] spi: Support selection of the index of the ACPI Spi Resource before alloc
  2022-01-21 17:24 [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Stefan Binding
  2022-01-21 17:24 ` [PATCH v6 1/9] spi: Make spi_alloc_device and spi_add_device public again Stefan Binding
  2022-01-21 17:24 ` [PATCH v6 2/9] spi: Create helper API to lookup ACPI info for spi device Stefan Binding
@ 2022-01-21 17:24 ` Stefan Binding
  2022-02-01 14:29   ` Hans de Goede
  2022-01-21 17:24 ` [PATCH v6 4/9] spi: Add API to count spi acpi resources Stefan Binding
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 29+ messages in thread
From: Stefan Binding @ 2022-01-21 17:24 UTC (permalink / raw)
  To: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches, Stefan Binding

If a node contains more than one Spi Resources, it may be necessary to
use an index to select which one you want to allocate a spi device for.

Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 drivers/spi/spi.c       | 51 +++++++++++++++++++++++++++++++++++------
 include/linux/spi/spi.h |  3 ++-
 2 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 13f4701f0694..06c0a308b38b 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2320,6 +2320,8 @@ struct acpi_spi_lookup {
 	int			irq;
 	u8			bits_per_word;
 	u8			chip_select;
+	int			n;
+	int			index;
 };
 
 static void acpi_spi_parse_apple_properties(struct acpi_device *dev,
@@ -2351,6 +2353,8 @@ static void acpi_spi_parse_apple_properties(struct acpi_device *dev,
 		lookup->mode |= SPI_CPHA;
 }
 
+static struct spi_controller *acpi_spi_find_controller_by_adev(struct acpi_device *adev);
+
 static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)
 {
 	struct acpi_spi_lookup *lookup = data;
@@ -2364,14 +2368,35 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)
 		sb = &ares->data.spi_serial_bus;
 		if (sb->type == ACPI_RESOURCE_SERIAL_TYPE_SPI) {
 
+			if (lookup->index != -1 && lookup->n++ != lookup->index)
+				return 1;
+
+			if (lookup->index == -1 && !ctlr)
+				return -ENODEV;
+
 			status = acpi_get_handle(NULL,
 						 sb->resource_source.string_ptr,
 						 &parent_handle);
 
-			if (ACPI_FAILURE(status) ||
-			    ACPI_HANDLE(ctlr->dev.parent) != parent_handle)
+			if (ACPI_FAILURE(status))
 				return -ENODEV;
 
+			if (ctlr) {
+				if (ACPI_HANDLE(ctlr->dev.parent) != parent_handle)
+					return -ENODEV;
+			} else {
+				struct acpi_device *adev;
+
+				if (acpi_bus_get_device(parent_handle, &adev))
+					return -ENODEV;
+
+				ctlr = acpi_spi_find_controller_by_adev(adev);
+				if (!ctlr)
+					return -ENODEV;
+
+				lookup->ctlr = ctlr;
+			}
+
 			/*
 			 * ACPI DeviceSelection numbering is handled by the
 			 * host controller driver in Windows and can vary
@@ -2414,14 +2439,21 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)
  * acpi_spi_device_alloc - Allocate a spi device, and fill it in with ACPI information
  * @ctlr: controller to which the spi device belongs
  * @adev: ACPI Device for the spi device
+ * @index: Index of the spi resource inside the ACPI Node
  *
  * This should be used to allocate a new spi device from and ACPI Node.
  * The caller is responsible for calling spi_add_device to register the spi device.
  *
+ * If ctlr is set to NULL, the Controller for the spi device will be looked up
+ * using the resource.
+ * If index is set to -1, index is not used.
+ * Note: If index is -1, ctlr must be set.
+ *
  * Return: a pointer to the new device, or ERR_PTR on error.
  */
 struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
-					 struct acpi_device *adev)
+					 struct acpi_device *adev,
+					 int index)
 {
 	acpi_handle parent_handle = NULL;
 	struct list_head resource_list;
@@ -2429,8 +2461,13 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
 	struct spi_device *spi;
 	int ret;
 
+	if (!ctlr && index == -1)
+		return ERR_PTR(-EINVAL);
+
 	lookup.ctlr		= ctlr;
 	lookup.irq		= -1;
+	lookup.index		= index;
+	lookup.n		= 0;
 
 	INIT_LIST_HEAD(&resource_list);
 	ret = acpi_dev_get_resources(adev, &resource_list,
@@ -2443,7 +2480,7 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
 
 	if (!lookup.max_speed_hz &&
 	    ACPI_SUCCESS(acpi_get_parent(adev->handle, &parent_handle)) &&
-	    ACPI_HANDLE(ctlr->dev.parent) == parent_handle) {
+	    ACPI_HANDLE(lookup.ctlr->dev.parent) == parent_handle) {
 		/* Apple does not use _CRS but nested devices for SPI slaves */
 		acpi_spi_parse_apple_properties(adev, &lookup);
 	}
@@ -2451,9 +2488,9 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
 	if (!lookup.max_speed_hz)
 		return ERR_PTR(-ENODEV);
 
-	spi = spi_alloc_device(ctlr);
+	spi = spi_alloc_device(lookup.ctlr);
 	if (!spi) {
-		dev_err(&ctlr->dev, "failed to allocate SPI device for %s\n",
+		dev_err(&lookup.ctlr->dev, "failed to allocate SPI device for %s\n",
 			dev_name(&adev->dev));
 		return ERR_PTR(-ENOMEM);
 	}
@@ -2478,7 +2515,7 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
 	    acpi_device_enumerated(adev))
 		return AE_OK;
 
-	spi = acpi_spi_device_alloc(ctlr, adev);
+	spi = acpi_spi_device_alloc(ctlr, adev, -1);
 	if (IS_ERR(spi)) {
 		if (PTR_ERR(spi) == -ENOMEM)
 			return AE_NO_MEMORY;
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index d159cef12f1a..e5bbb9cbd3d7 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -762,7 +762,8 @@ extern void spi_unregister_controller(struct spi_controller *ctlr);
 
 #if IS_ENABLED(CONFIG_ACPI)
 extern struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
-						struct acpi_device *adev);
+						struct acpi_device *adev,
+						int index);
 #endif
 
 /*
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 4/9] spi: Add API to count spi acpi resources
  2022-01-21 17:24 [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Stefan Binding
                   ` (2 preceding siblings ...)
  2022-01-21 17:24 ` [PATCH v6 3/9] spi: Support selection of the index of the ACPI Spi Resource before alloc Stefan Binding
@ 2022-01-21 17:24 ` Stefan Binding
  2022-02-01 14:42   ` Hans de Goede
  2022-01-21 17:24 ` [PATCH v6 5/9] platform/x86: i2c-multi-instantiate: Rename it for a generic serial driver name Stefan Binding
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 29+ messages in thread
From: Stefan Binding @ 2022-01-21 17:24 UTC (permalink / raw)
  To: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches, Stefan Binding

Some ACPI nodes may have more than one Spi Resource.
To be able to handle these case, its necessary to have
a way of counting these resources.

Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 drivers/spi/spi.c       | 40 ++++++++++++++++++++++++++++++++++++++++
 include/linux/spi/spi.h |  1 +
 2 files changed, 41 insertions(+)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 06c0a308b38b..ec9f2ed579e3 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2324,6 +2324,46 @@ struct acpi_spi_lookup {
 	int			index;
 };
 
+static int acpi_spi_count(struct acpi_resource *ares, void *data)
+{
+	struct acpi_resource_spi_serialbus *sb;
+	int *count = data;
+
+	if (ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS)
+		return 1;
+
+	sb = &ares->data.spi_serial_bus;
+	if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_SPI)
+		return 1;
+
+	*count = *count + 1;
+
+	return 1;
+}
+
+/**
+ * acpi_spi_count_resources - Count the number of SpiSerialBus resources
+ * @adev:	ACPI device
+ *
+ * Returns the number of SpiSerialBus resources in the ACPI-device's
+ * resource-list; or a negative error code.
+ */
+int acpi_spi_count_resources(struct acpi_device *adev)
+{
+	LIST_HEAD(r);
+	int count = 0;
+	int ret;
+
+	ret = acpi_dev_get_resources(adev, &r, acpi_spi_count, &count);
+	if (ret < 0)
+		return ret;
+
+	acpi_dev_free_resource_list(&r);
+
+	return count;
+}
+EXPORT_SYMBOL_GPL(acpi_spi_count_resources);
+
 static void acpi_spi_parse_apple_properties(struct acpi_device *dev,
 					    struct acpi_spi_lookup *lookup)
 {
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index e5bbb9cbd3d7..394b4241d989 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -764,6 +764,7 @@ extern void spi_unregister_controller(struct spi_controller *ctlr);
 extern struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
 						struct acpi_device *adev,
 						int index);
+int acpi_spi_count_resources(struct acpi_device *adev);
 #endif
 
 /*
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 5/9] platform/x86: i2c-multi-instantiate: Rename it for a generic serial driver name
  2022-01-21 17:24 [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Stefan Binding
                   ` (3 preceding siblings ...)
  2022-01-21 17:24 ` [PATCH v6 4/9] spi: Add API to count spi acpi resources Stefan Binding
@ 2022-01-21 17:24 ` Stefan Binding
  2022-02-01 14:55   ` Hans de Goede
  2022-01-21 17:24 ` [PATCH v6 6/9] platform/x86: serial-multi-instantiate: Reorganize I2C functions Stefan Binding
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 29+ messages in thread
From: Stefan Binding @ 2022-01-21 17:24 UTC (permalink / raw)
  To: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches, Lucas Tanure, Stefan Binding

From: Lucas Tanure <tanureal@opensource.cirrus.com>

Rename I2C multi instantiate driver to serial-multi-instantiate for
upcoming addition of SPI support

Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 MAINTAINERS                                   |  4 +-
 drivers/acpi/scan.c                           | 13 +--
 drivers/platform/x86/Kconfig                  | 10 +-
 drivers/platform/x86/Makefile                 |  2 +-
 ...stantiate.c => serial-multi-instantiate.c} | 91 +++++++++----------
 5 files changed, 60 insertions(+), 60 deletions(-)
 rename drivers/platform/x86/{i2c-multi-instantiate.c => serial-multi-instantiate.c} (51%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4e828542b089..be50537ea6bd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -388,11 +388,11 @@ L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
 F:	drivers/acpi/arm64
 
-ACPI I2C MULTI INSTANTIATE DRIVER
+ACPI SERIAL MULTI INSTANTIATE DRIVER
 M:	Hans de Goede <hdegoede@redhat.com>
 L:	platform-driver-x86@vger.kernel.org
 S:	Maintained
-F:	drivers/platform/x86/i2c-multi-instantiate.c
+F:	drivers/platform/x86/serial-multi-instantiate.c
 
 ACPI PCC(Platform Communication Channel) MAILBOX DRIVER
 M:	Sudeep Holla <sudeep.holla@arm.com>
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 1331756d4cfc..48db5e80c2dc 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1734,12 +1734,13 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)
 	bool is_serial_bus_slave = false;
 	static const struct acpi_device_id ignore_serial_bus_ids[] = {
 	/*
-	 * These devices have multiple I2cSerialBus resources and an i2c-client
-	 * must be instantiated for each, each with its own i2c_device_id.
-	 * Normally we only instantiate an i2c-client for the first resource,
-	 * using the ACPI HID as id. These special cases are handled by the
-	 * drivers/platform/x86/i2c-multi-instantiate.c driver, which knows
-	 * which i2c_device_id to use for each resource.
+	 * These devices have multiple SerialBus resources and a client
+	 * device must be instantiated for each of them, each with
+	 * its own device id.
+	 * Normally we only instantiate one client device for the first
+	 * resource, using the ACPI HID as id. These special cases are handled
+	 * by the drivers/platform/x86/serial-multi-instantiate.c driver, which
+	 * knows which client device id to use for each resource.
 	 */
 		{"BSG1160", },
 		{"BSG2150", },
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 24deeeb29af2..2e656909a866 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -990,16 +990,16 @@ config TOPSTAR_LAPTOP
 
 	  If you have a Topstar laptop, say Y or M here.
 
-config I2C_MULTI_INSTANTIATE
-	tristate "I2C multi instantiate pseudo device driver"
+config SERIAL_MULTI_INSTANTIATE
+	tristate "Serial bus multi instantiate pseudo device driver"
 	depends on I2C && ACPI
 	help
-	  Some ACPI-based systems list multiple i2c-devices in a single ACPI
-	  firmware-node. This driver will instantiate separate i2c-clients
+	  Some ACPI-based systems list multiple devices in a single ACPI
+	  firmware-node. This driver will instantiate separate clients
 	  for each device in the firmware-node.
 
 	  To compile this driver as a module, choose M here: the module
-	  will be called i2c-multi-instantiate.
+	  will be called serial-multi-instantiate.
 
 config MLX_PLATFORM
 	tristate "Mellanox Technologies platform support"
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index c12a9b044fd8..9527088bba7f 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -110,7 +110,7 @@ obj-$(CONFIG_TOPSTAR_LAPTOP)	+= topstar-laptop.o
 
 # Platform drivers
 obj-$(CONFIG_FW_ATTR_CLASS)		+= firmware_attributes_class.o
-obj-$(CONFIG_I2C_MULTI_INSTANTIATE)	+= i2c-multi-instantiate.o
+obj-$(CONFIG_SERIAL_MULTI_INSTANTIATE)	+= serial-multi-instantiate.o
 obj-$(CONFIG_MLX_PLATFORM)		+= mlx-platform.o
 obj-$(CONFIG_TOUCHSCREEN_DMI)		+= touchscreen_dmi.o
 obj-$(CONFIG_WIRELESS_HOTKEY)		+= wireless-hotkey.o
diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
similarity index 51%
rename from drivers/platform/x86/i2c-multi-instantiate.c
rename to drivers/platform/x86/serial-multi-instantiate.c
index 4956a1df5b90..33cbb0caed33 100644
--- a/drivers/platform/x86/i2c-multi-instantiate.c
+++ b/drivers/platform/x86/serial-multi-instantiate.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * I2C multi-instantiate driver, pseudo driver to instantiate multiple
- * i2c-clients from a single fwnode.
+ * Serial multi-instantiate driver, pseudo driver to instantiate multiple
+ * client devices from a single fwnode.
  *
  * Copyright 2018 Hans de Goede <hdegoede@redhat.com>
  */
@@ -21,29 +21,29 @@
 #define IRQ_RESOURCE_GPIO	1
 #define IRQ_RESOURCE_APIC	2
 
-struct i2c_inst_data {
+struct smi_instance {
 	const char *type;
 	unsigned int flags;
 	int irq_idx;
 };
 
-struct i2c_multi_inst_data {
-	int num_clients;
-	struct i2c_client *clients[];
+struct smi {
+	int i2c_num;
+	struct i2c_client *i2c_devs[];
 };
 
-static int i2c_multi_inst_probe(struct platform_device *pdev)
+static int smi_probe(struct platform_device *pdev)
 {
-	struct i2c_multi_inst_data *multi;
-	const struct i2c_inst_data *inst_data;
 	struct i2c_board_info board_info = {};
+	const struct smi_instance *inst;
 	struct device *dev = &pdev->dev;
 	struct acpi_device *adev;
+	struct smi *smi;
 	char name[32];
 	int i, ret;
 
-	inst_data = device_get_match_data(dev);
-	if (!inst_data) {
+	inst = device_get_match_data(dev);
+	if (!inst) {
 		dev_err(dev, "Error ACPI match data is missing\n");
 		return -ENODEV;
 	}
@@ -55,33 +55,32 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	multi = devm_kmalloc(dev, struct_size(multi, clients, ret), GFP_KERNEL);
-	if (!multi)
+	smi = devm_kmalloc(dev, struct_size(smi, i2c_devs, ret), GFP_KERNEL);
+	if (!smi)
 		return -ENOMEM;
 
-	multi->num_clients = ret;
+	smi->i2c_num = ret;
 
-	for (i = 0; i < multi->num_clients && inst_data[i].type; i++) {
+	for (i = 0; i < smi->i2c_num && inst[i].type; i++) {
 		memset(&board_info, 0, sizeof(board_info));
-		strlcpy(board_info.type, inst_data[i].type, I2C_NAME_SIZE);
-		snprintf(name, sizeof(name), "%s-%s.%d", dev_name(dev),
-			 inst_data[i].type, i);
+		strlcpy(board_info.type, inst[i].type, I2C_NAME_SIZE);
+		snprintf(name, sizeof(name), "%s-%s.%d", dev_name(dev), inst[i].type, i);
 		board_info.dev_name = name;
-		switch (inst_data[i].flags & IRQ_RESOURCE_TYPE) {
+		switch (inst[i].flags & IRQ_RESOURCE_TYPE) {
 		case IRQ_RESOURCE_GPIO:
-			ret = acpi_dev_gpio_irq_get(adev, inst_data[i].irq_idx);
+			ret = acpi_dev_gpio_irq_get(adev, inst[i].irq_idx);
 			if (ret < 0) {
 				dev_err(dev, "Error requesting irq at index %d: %d\n",
-					inst_data[i].irq_idx, ret);
+						inst[i].irq_idx, ret);
 				goto error;
 			}
 			board_info.irq = ret;
 			break;
 		case IRQ_RESOURCE_APIC:
-			ret = platform_get_irq(pdev, inst_data[i].irq_idx);
+			ret = platform_get_irq(pdev, inst[i].irq_idx);
 			if (ret < 0) {
 				dev_dbg(dev, "Error requesting irq at index %d: %d\n",
-					inst_data[i].irq_idx, ret);
+					inst[i].irq_idx, ret);
 				goto error;
 			}
 			board_info.irq = ret;
@@ -90,48 +89,48 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
 			board_info.irq = 0;
 			break;
 		}
-		multi->clients[i] = i2c_acpi_new_device(dev, i, &board_info);
-		if (IS_ERR(multi->clients[i])) {
-			ret = dev_err_probe(dev, PTR_ERR(multi->clients[i]),
+		smi->i2c_devs[i] = i2c_acpi_new_device(dev, i, &board_info);
+		if (IS_ERR(smi->i2c_devs[i])) {
+			ret = dev_err_probe(dev, PTR_ERR(smi->i2c_devs[i]),
 					    "Error creating i2c-client, idx %d\n", i);
 			goto error;
 		}
 	}
-	if (i < multi->num_clients) {
+	if (i < smi->i2c_num) {
 		dev_err(dev, "Error finding driver, idx %d\n", i);
 		ret = -ENODEV;
 		goto error;
 	}
 
-	platform_set_drvdata(pdev, multi);
+	platform_set_drvdata(pdev, smi);
 	return 0;
 
 error:
 	while (--i >= 0)
-		i2c_unregister_device(multi->clients[i]);
+		i2c_unregister_device(smi->i2c_devs[i]);
 
 	return ret;
 }
 
-static int i2c_multi_inst_remove(struct platform_device *pdev)
+static int smi_remove(struct platform_device *pdev)
 {
-	struct i2c_multi_inst_data *multi = platform_get_drvdata(pdev);
+	struct smi *smi = platform_get_drvdata(pdev);
 	int i;
 
-	for (i = 0; i < multi->num_clients; i++)
-		i2c_unregister_device(multi->clients[i]);
+	for (i = 0; i < smi->i2c_num; i++)
+		i2c_unregister_device(smi->i2c_devs[i]);
 
 	return 0;
 }
 
-static const struct i2c_inst_data bsg1160_data[]  = {
+static const struct smi_instance bsg1160_data[]  = {
 	{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
 	{ "bmc150_magn" },
 	{ "bmg160" },
 	{}
 };
 
-static const struct i2c_inst_data bsg2150_data[]  = {
+static const struct smi_instance bsg2150_data[]  = {
 	{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
 	{ "bmc150_magn" },
 	/* The resources describe a 3th client, but it is not really there. */
@@ -139,7 +138,7 @@ static const struct i2c_inst_data bsg2150_data[]  = {
 	{}
 };
 
-static const struct i2c_inst_data int3515_data[]  = {
+static const struct smi_instance int3515_data[]  = {
 	{ "tps6598x", IRQ_RESOURCE_APIC, 0 },
 	{ "tps6598x", IRQ_RESOURCE_APIC, 1 },
 	{ "tps6598x", IRQ_RESOURCE_APIC, 2 },
@@ -148,27 +147,27 @@ static const struct i2c_inst_data int3515_data[]  = {
 };
 
 /*
- * Note new device-ids must also be added to i2c_multi_instantiate_ids in
+ * Note new device-ids must also be added to serial_multi_instantiate_ids in
  * drivers/acpi/scan.c: acpi_device_enumeration_by_parent().
  */
-static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = {
+static const struct acpi_device_id smi_acpi_ids[] = {
 	{ "BSG1160", (unsigned long)bsg1160_data },
 	{ "BSG2150", (unsigned long)bsg2150_data },
 	{ "INT3515", (unsigned long)int3515_data },
 	{ }
 };
-MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids);
+MODULE_DEVICE_TABLE(acpi, smi_acpi_ids);
 
-static struct platform_driver i2c_multi_inst_driver = {
+static struct platform_driver smi_driver = {
 	.driver	= {
-		.name = "I2C multi instantiate pseudo device driver",
-		.acpi_match_table = i2c_multi_inst_acpi_ids,
+		.name = "Serial bus multi instantiate pseudo device driver",
+		.acpi_match_table = smi_acpi_ids,
 	},
-	.probe = i2c_multi_inst_probe,
-	.remove = i2c_multi_inst_remove,
+	.probe = smi_probe,
+	.remove = smi_remove,
 };
-module_platform_driver(i2c_multi_inst_driver);
+module_platform_driver(smi_driver);
 
-MODULE_DESCRIPTION("I2C multi instantiate pseudo device driver");
+MODULE_DESCRIPTION("Serial multi instantiate pseudo device driver");
 MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
 MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 6/9] platform/x86: serial-multi-instantiate: Reorganize I2C functions
  2022-01-21 17:24 [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Stefan Binding
                   ` (4 preceding siblings ...)
  2022-01-21 17:24 ` [PATCH v6 5/9] platform/x86: i2c-multi-instantiate: Rename it for a generic serial driver name Stefan Binding
@ 2022-01-21 17:24 ` Stefan Binding
  2022-02-01 14:59   ` Hans de Goede
  2022-01-21 17:24 ` [PATCH v6 7/9] platform/x86: serial-multi-instantiate: Add SPI support Stefan Binding
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 29+ messages in thread
From: Stefan Binding @ 2022-01-21 17:24 UTC (permalink / raw)
  To: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches, Lucas Tanure, Stefan Binding

From: Lucas Tanure <tanureal@opensource.cirrus.com>

Reorganize I2C functions to accommodate SPI support
Split the probe and factor out parts of the code
that will be used in the SPI support
Also switched from strlcpy() to strscpy()

Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 .../platform/x86/serial-multi-instantiate.c   | 144 +++++++++++-------
 1 file changed, 90 insertions(+), 54 deletions(-)

diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
index 33cbb0caed33..4cd6d72a0741 100644
--- a/drivers/platform/x86/serial-multi-instantiate.c
+++ b/drivers/platform/x86/serial-multi-instantiate.c
@@ -29,96 +29,132 @@ struct smi_instance {
 
 struct smi {
 	int i2c_num;
-	struct i2c_client *i2c_devs[];
+	struct i2c_client **i2c_devs;
 };
 
-static int smi_probe(struct platform_device *pdev)
+static int smi_get_irq(struct platform_device *pdev, struct acpi_device *adev,
+		       const struct smi_instance *inst)
+{
+	int ret;
+
+	switch (inst->flags & IRQ_RESOURCE_TYPE) {
+	case IRQ_RESOURCE_GPIO:
+		ret = acpi_dev_gpio_irq_get(adev, inst->irq_idx);
+		break;
+	case IRQ_RESOURCE_APIC:
+		ret = platform_get_irq(pdev, inst->irq_idx);
+		break;
+	default:
+		return 0;
+	}
+
+	if (ret < 0)
+		dev_err_probe(&pdev->dev, ret, "Error requesting irq at index %d: %d\n",
+			      inst->irq_idx, ret);
+
+	return ret;
+}
+
+static void smi_devs_unregister(struct smi *smi)
+{
+	while (smi->i2c_num > 0)
+		i2c_unregister_device(smi->i2c_devs[--smi->i2c_num]);
+}
+
+/**
+ * smi_i2c_probe - Instantiate multiple I2C devices from inst array
+ * @pdev:	Platform device
+ * @adev:	ACPI device
+ * @smi:	Internal struct for Serial multi instantiate driver
+ * @inst_array:	Array of instances to probe
+ *
+ * Returns the number of I2C devices instantiate, Zero if none is found or a negative error code.
+ */
+static int smi_i2c_probe(struct platform_device *pdev, struct acpi_device *adev, struct smi *smi,
+			 const struct smi_instance *inst_array)
 {
 	struct i2c_board_info board_info = {};
-	const struct smi_instance *inst;
 	struct device *dev = &pdev->dev;
-	struct acpi_device *adev;
-	struct smi *smi;
 	char name[32];
-	int i, ret;
+	int i, ret, count;
 
-	inst = device_get_match_data(dev);
-	if (!inst) {
-		dev_err(dev, "Error ACPI match data is missing\n");
-		return -ENODEV;
-	}
-
-	adev = ACPI_COMPANION(dev);
-
-	/* Count number of clients to instantiate */
 	ret = i2c_acpi_client_count(adev);
 	if (ret < 0)
 		return ret;
+	else if (!ret)
+		return -ENODEV;
 
-	smi = devm_kmalloc(dev, struct_size(smi, i2c_devs, ret), GFP_KERNEL);
-	if (!smi)
-		return -ENOMEM;
+	count = ret;
 
-	smi->i2c_num = ret;
+	smi->i2c_devs = devm_kcalloc(dev, count, sizeof(*smi->i2c_devs), GFP_KERNEL);
+	if (!smi->i2c_devs)
+		return -ENOMEM;
 
-	for (i = 0; i < smi->i2c_num && inst[i].type; i++) {
+	for (i = 0; i < count && inst_array[i].type; i++) {
 		memset(&board_info, 0, sizeof(board_info));
-		strlcpy(board_info.type, inst[i].type, I2C_NAME_SIZE);
-		snprintf(name, sizeof(name), "%s-%s.%d", dev_name(dev), inst[i].type, i);
+		strscpy(board_info.type, inst_array[i].type, I2C_NAME_SIZE);
+		snprintf(name, sizeof(name), "%s-%s.%d", dev_name(dev), inst_array[i].type, i);
 		board_info.dev_name = name;
-		switch (inst[i].flags & IRQ_RESOURCE_TYPE) {
-		case IRQ_RESOURCE_GPIO:
-			ret = acpi_dev_gpio_irq_get(adev, inst[i].irq_idx);
-			if (ret < 0) {
-				dev_err(dev, "Error requesting irq at index %d: %d\n",
-						inst[i].irq_idx, ret);
-				goto error;
-			}
-			board_info.irq = ret;
-			break;
-		case IRQ_RESOURCE_APIC:
-			ret = platform_get_irq(pdev, inst[i].irq_idx);
-			if (ret < 0) {
-				dev_dbg(dev, "Error requesting irq at index %d: %d\n",
-					inst[i].irq_idx, ret);
-				goto error;
-			}
-			board_info.irq = ret;
-			break;
-		default:
-			board_info.irq = 0;
-			break;
-		}
+
+		ret = smi_get_irq(pdev, adev, &inst_array[i]);
+		if (ret < 0)
+			goto error;
+		board_info.irq = ret;
+
 		smi->i2c_devs[i] = i2c_acpi_new_device(dev, i, &board_info);
 		if (IS_ERR(smi->i2c_devs[i])) {
 			ret = dev_err_probe(dev, PTR_ERR(smi->i2c_devs[i]),
 					    "Error creating i2c-client, idx %d\n", i);
 			goto error;
 		}
+		smi->i2c_num++;
 	}
-	if (i < smi->i2c_num) {
-		dev_err(dev, "Error finding driver, idx %d\n", i);
+	if (smi->i2c_num < count) {
+		dev_dbg(dev, "Error finding driver, idx %d\n", i);
 		ret = -ENODEV;
 		goto error;
 	}
 
-	platform_set_drvdata(pdev, smi);
-	return 0;
+	dev_info(dev, "Instantiated %d I2C devices.\n", smi->i2c_num);
 
+	return 0;
 error:
-	while (--i >= 0)
-		i2c_unregister_device(smi->i2c_devs[i]);
+	smi_devs_unregister(smi);
 
 	return ret;
 }
 
+static int smi_probe(struct platform_device *pdev)
+{
+	const struct smi_instance *inst_array;
+	struct device *dev = &pdev->dev;
+	struct acpi_device *adev;
+	struct smi *smi;
+
+	adev = ACPI_COMPANION(dev);
+	if (!adev)
+		return -ENODEV;
+
+	inst_array = device_get_match_data(dev);
+	if (!inst_array) {
+		dev_dbg(dev, "Error ACPI match data is missing\n");
+		return -ENODEV;
+	}
+
+	smi = devm_kzalloc(dev, sizeof(*smi), GFP_KERNEL);
+	if (!smi)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, smi);
+
+	return smi_i2c_probe(pdev, adev, smi, inst_array);
+}
+
 static int smi_remove(struct platform_device *pdev)
 {
 	struct smi *smi = platform_get_drvdata(pdev);
-	int i;
 
-	for (i = 0; i < smi->i2c_num; i++)
-		i2c_unregister_device(smi->i2c_devs[i]);
+	smi_devs_unregister(smi);
 
 	return 0;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 7/9] platform/x86: serial-multi-instantiate: Add SPI support
  2022-01-21 17:24 [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Stefan Binding
                   ` (5 preceding siblings ...)
  2022-01-21 17:24 ` [PATCH v6 6/9] platform/x86: serial-multi-instantiate: Reorganize I2C functions Stefan Binding
@ 2022-01-21 17:24 ` Stefan Binding
  2022-02-01 15:02   ` Hans de Goede
  2022-01-21 17:24 ` [PATCH v6 8/9] ALSA: hda/realtek: Add support for HP Laptops Stefan Binding
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 29+ messages in thread
From: Stefan Binding @ 2022-01-21 17:24 UTC (permalink / raw)
  To: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches, Stefan Binding

Add support for spi bus in serial-multi-instantiate driver

Some peripherals can have either a I2C or a SPI connection
to the host (but not both) but use the same HID for both
types. So it is not possible to use the HID to determine
whether it is I2C or SPI. The driver must check the node
to see if it contains I2cSerialBus or SpiSerialBus entries.

For backwards-compatibility with the existing nodes I2C is
checked first and if such entries are found ONLY I2C devices
are created. Since some existing nodes that were already
handled by this driver could also contain unrelated
SpiSerialBus nodes that were previously ignored, and this
preserves that behavior. If there is ever a need to handle
a node where both I2C and SPI devices must be instantiated
this can be added in future.

Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 drivers/platform/x86/Kconfig                  |   2 +-
 .../platform/x86/serial-multi-instantiate.c   | 174 +++++++++++++++---
 2 files changed, 151 insertions(+), 25 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 2e656909a866..8d1eec208854 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -992,7 +992,7 @@ config TOPSTAR_LAPTOP
 
 config SERIAL_MULTI_INSTANTIATE
 	tristate "Serial bus multi instantiate pseudo device driver"
-	depends on I2C && ACPI
+	depends on I2C && SPI && ACPI
 	help
 	  Some ACPI-based systems list multiple devices in a single ACPI
 	  firmware-node. This driver will instantiate separate clients
diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
index 4cd6d72a0741..3f05385ca2cf 100644
--- a/drivers/platform/x86/serial-multi-instantiate.c
+++ b/drivers/platform/x86/serial-multi-instantiate.c
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
+#include <linux/spi/spi.h>
 #include <linux/types.h>
 
 #define IRQ_RESOURCE_TYPE	GENMASK(1, 0)
@@ -21,15 +22,28 @@
 #define IRQ_RESOURCE_GPIO	1
 #define IRQ_RESOURCE_APIC	2
 
+enum smi_bus_type {
+	SMI_I2C,
+	SMI_SPI,
+	SMI_AUTO_DETECT,
+};
+
 struct smi_instance {
 	const char *type;
 	unsigned int flags;
 	int irq_idx;
 };
 
+struct smi_node {
+	enum smi_bus_type bus_type;
+	struct smi_instance instances[];
+};
+
 struct smi {
 	int i2c_num;
+	int spi_num;
 	struct i2c_client **i2c_devs;
+	struct spi_device **spi_devs;
 };
 
 static int smi_get_irq(struct platform_device *pdev, struct acpi_device *adev,
@@ -59,6 +73,95 @@ static void smi_devs_unregister(struct smi *smi)
 {
 	while (smi->i2c_num > 0)
 		i2c_unregister_device(smi->i2c_devs[--smi->i2c_num]);
+
+	while (smi->spi_num > 0)
+		spi_unregister_device(smi->spi_devs[--smi->spi_num]);
+}
+
+/**
+ * smi_spi_probe - Instantiate multiple SPI devices from inst array
+ * @pdev:	Platform device
+ * @adev:	ACPI device
+ * @smi:	Internal struct for Serial multi instantiate driver
+ * @inst_array:	Array of instances to probe
+ *
+ * Returns the number of SPI devices instantiate, Zero if none is found or a negative error code.
+ */
+static int smi_spi_probe(struct platform_device *pdev, struct acpi_device *adev, struct smi *smi,
+			 const struct smi_instance *inst_array)
+{
+	struct device *dev = &pdev->dev;
+	struct spi_controller *ctlr;
+	struct spi_device *spi_dev;
+	char name[50];
+	int i, ret, count;
+
+	ret = acpi_spi_count_resources(adev);
+	if (ret < 0)
+		return ret;
+	else if (!ret)
+		return -ENODEV;
+
+	count = ret;
+
+	smi->spi_devs = devm_kcalloc(dev, count, sizeof(*smi->spi_devs), GFP_KERNEL);
+	if (!smi->spi_devs)
+		return -ENOMEM;
+
+	for (i = 0; i < count && inst_array[i].type; i++) {
+
+		spi_dev = acpi_spi_device_alloc(NULL, adev, i);
+		if (IS_ERR(spi_dev)) {
+			ret = PTR_ERR(spi_dev);
+			dev_err_probe(dev, ret, "failed to allocate SPI device %s from ACPI: %d\n",
+				      dev_name(&adev->dev), ret);
+			goto error;
+		}
+
+		ctlr = spi_dev->controller;
+
+		strscpy(spi_dev->modalias, inst_array[i].type, sizeof(spi_dev->modalias));
+
+		ret = smi_get_irq(pdev, adev, &inst_array[i]);
+		if (ret < 0) {
+			spi_dev_put(spi_dev);
+			goto error;
+		}
+		spi_dev->irq = ret;
+
+		snprintf(name, sizeof(name), "%s-%s-%s.%d", dev_name(&ctlr->dev), dev_name(dev),
+			 inst_array[i].type, i);
+		spi_dev->dev.init_name = name;
+
+		ret = spi_add_device(spi_dev);
+		if (ret) {
+			dev_err_probe(&ctlr->dev, ret,
+				      "failed to add SPI device %s from ACPI: %d\n",
+				      dev_name(&adev->dev), ret);
+			spi_dev_put(spi_dev);
+			goto error;
+		}
+
+		dev_dbg(dev, "SPI device %s using chip select %u", name, spi_dev->chip_select);
+
+		smi->spi_devs[i] = spi_dev;
+		smi->spi_num++;
+	}
+
+	if (smi->spi_num < count) {
+		dev_dbg(dev, "Error finding driver, idx %d\n", i);
+		ret = -ENODEV;
+		goto error;
+	}
+
+	dev_info(dev, "Instantiated %d SPI devices.\n", smi->spi_num);
+
+	return 0;
+error:
+	smi_devs_unregister(smi);
+
+	return ret;
+
 }
 
 /**
@@ -126,8 +229,8 @@ static int smi_i2c_probe(struct platform_device *pdev, struct acpi_device *adev,
 
 static int smi_probe(struct platform_device *pdev)
 {
-	const struct smi_instance *inst_array;
 	struct device *dev = &pdev->dev;
+	const struct smi_node *node;
 	struct acpi_device *adev;
 	struct smi *smi;
 
@@ -135,8 +238,8 @@ static int smi_probe(struct platform_device *pdev)
 	if (!adev)
 		return -ENODEV;
 
-	inst_array = device_get_match_data(dev);
-	if (!inst_array) {
+	node = device_get_match_data(dev);
+	if (!node) {
 		dev_dbg(dev, "Error ACPI match data is missing\n");
 		return -ENODEV;
 	}
@@ -147,7 +250,21 @@ static int smi_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, smi);
 
-	return smi_i2c_probe(pdev, adev, smi, inst_array);
+	switch (node->bus_type) {
+	case SMI_I2C:
+		return smi_i2c_probe(pdev, adev, smi, node->instances);
+	case SMI_SPI:
+		return smi_spi_probe(pdev, adev, smi, node->instances);
+	case SMI_AUTO_DETECT:
+		if (i2c_acpi_client_count(adev) > 0)
+			return smi_i2c_probe(pdev, adev, smi, node->instances);
+		else
+			return smi_spi_probe(pdev, adev, smi, node->instances);
+	default:
+		break;
+	}
+
+	return 0; /* never reached */
 }
 
 static int smi_remove(struct platform_device *pdev)
@@ -159,27 +276,36 @@ static int smi_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct smi_instance bsg1160_data[]  = {
-	{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
-	{ "bmc150_magn" },
-	{ "bmg160" },
-	{}
+static const struct smi_node bsg1160_data = {
+	.instances = {
+		{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
+		{ "bmc150_magn" },
+		{ "bmg160" },
+		{}
+	},
+	.bus_type = SMI_I2C,
 };
 
-static const struct smi_instance bsg2150_data[]  = {
-	{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
-	{ "bmc150_magn" },
-	/* The resources describe a 3th client, but it is not really there. */
-	{ "bsg2150_dummy_dev" },
-	{}
+static const struct smi_node bsg2150_data = {
+	.instances = {
+		{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
+		{ "bmc150_magn" },
+		/* The resources describe a 3th client, but it is not really there. */
+		{ "bsg2150_dummy_dev" },
+		{}
+	},
+	.bus_type = SMI_I2C,
 };
 
-static const struct smi_instance int3515_data[]  = {
-	{ "tps6598x", IRQ_RESOURCE_APIC, 0 },
-	{ "tps6598x", IRQ_RESOURCE_APIC, 1 },
-	{ "tps6598x", IRQ_RESOURCE_APIC, 2 },
-	{ "tps6598x", IRQ_RESOURCE_APIC, 3 },
-	{}
+static const struct smi_node int3515_data = {
+	.instances = {
+		{ "tps6598x", IRQ_RESOURCE_APIC, 0 },
+		{ "tps6598x", IRQ_RESOURCE_APIC, 1 },
+		{ "tps6598x", IRQ_RESOURCE_APIC, 2 },
+		{ "tps6598x", IRQ_RESOURCE_APIC, 3 },
+		{}
+	},
+	.bus_type = SMI_I2C,
 };
 
 /*
@@ -187,9 +313,9 @@ static const struct smi_instance int3515_data[]  = {
  * drivers/acpi/scan.c: acpi_device_enumeration_by_parent().
  */
 static const struct acpi_device_id smi_acpi_ids[] = {
-	{ "BSG1160", (unsigned long)bsg1160_data },
-	{ "BSG2150", (unsigned long)bsg2150_data },
-	{ "INT3515", (unsigned long)int3515_data },
+	{ "BSG1160", (unsigned long)&bsg1160_data },
+	{ "BSG2150", (unsigned long)&bsg2150_data },
+	{ "INT3515", (unsigned long)&int3515_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, smi_acpi_ids);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 8/9] ALSA: hda/realtek: Add support for HP Laptops
  2022-01-21 17:24 [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Stefan Binding
                   ` (6 preceding siblings ...)
  2022-01-21 17:24 ` [PATCH v6 7/9] platform/x86: serial-multi-instantiate: Add SPI support Stefan Binding
@ 2022-01-21 17:24 ` Stefan Binding
  2022-02-02  9:05   ` Takashi Iwai
  2022-01-21 17:24 ` [PATCH v6 9/9] ACPI / scan: Create platform device for CS35L41 Stefan Binding
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 29+ messages in thread
From: Stefan Binding @ 2022-01-21 17:24 UTC (permalink / raw)
  To: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches, Lucas Tanure, Stefan Binding

From: Lucas Tanure <tanureal@opensource.cirrus.com>

Add support for two and four CS35L41 using the component
binding method

Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 sound/pci/hda/patch_realtek.c | 43 ++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 668274e52674..db0fce42887c 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -6611,6 +6611,16 @@ static void cs35l41_fixup_i2c_two(struct hda_codec *cdc, const struct hda_fixup
 	cs35l41_generic_fixup(cdc, action, "i2c", "CSC3551", 2);
 }
 
+static void cs35l41_fixup_spi_two(struct hda_codec *codec, const struct hda_fixup *fix, int action)
+{
+	cs35l41_generic_fixup(codec, action, "spi0", "CSC3551", 2);
+}
+
+static void cs35l41_fixup_spi_four(struct hda_codec *codec, const struct hda_fixup *fix, int action)
+{
+	cs35l41_generic_fixup(codec, action, "spi0", "CSC3551", 4);
+}
+
 static void alc287_legion_16achg6_playback_hook(struct hda_pcm_stream *hinfo, struct hda_codec *cdc,
 						struct snd_pcm_substream *sub, int action)
 {
@@ -6948,6 +6958,9 @@ enum {
 	ALC285_FIXUP_LEGION_Y9000X_AUTOMUTE,
 	ALC287_FIXUP_LEGION_16ACHG6,
 	ALC287_FIXUP_CS35L41_I2C_2,
+	ALC245_FIXUP_CS35L41_SPI_2,
+	ALC245_FIXUP_CS35L41_SPI_4,
+	ALC245_FIXUP_CS35L41_SPI_4_HP_GPIO_LED,
 	ALC285_FIXUP_HP_SPEAKERS_MICMUTE_LED,
 };
 
@@ -8699,6 +8712,20 @@ static const struct hda_fixup alc269_fixups[] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = cs35l41_fixup_i2c_two,
 	},
+	[ALC245_FIXUP_CS35L41_SPI_2] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = cs35l41_fixup_spi_two,
+	},
+	[ALC245_FIXUP_CS35L41_SPI_4] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = cs35l41_fixup_spi_four,
+	},
+	[ALC245_FIXUP_CS35L41_SPI_4_HP_GPIO_LED] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc245_fixup_hp_gpio_led,
+		.chained = true,
+		.chain_id = ALC245_FIXUP_CS35L41_SPI_4,
+	},
 	[ALC285_FIXUP_HP_SPEAKERS_MICMUTE_LED] = {
 		.type = HDA_FIXUP_VERBS,
 		.v.verbs = (const struct hda_verb[]) {
@@ -8926,7 +8953,21 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x103c, 0x8896, "HP EliteBook 855 G8 Notebook PC", ALC285_FIXUP_HP_MUTE_LED),
 	SND_PCI_QUIRK(0x103c, 0x8898, "HP EliteBook 845 G8 Notebook PC", ALC285_FIXUP_HP_LIMIT_INT_MIC_BOOST),
 	SND_PCI_QUIRK(0x103c, 0x88d0, "HP Pavilion 15-eh1xxx (mainboard 88D0)", ALC287_FIXUP_HP_GPIO_LED),
-	SND_PCI_QUIRK(0x103c, 0x89c3, "HP", ALC285_FIXUP_HP_GPIO_LED),
+	SND_PCI_QUIRK(0x103c, 0x896e, "HP EliteBook x360 830 G9", ALC245_FIXUP_CS35L41_SPI_2),
+	SND_PCI_QUIRK(0x103c, 0x8971, "HP EliteBook 830 G9", ALC245_FIXUP_CS35L41_SPI_2),
+	SND_PCI_QUIRK(0x103c, 0x8972, "HP EliteBook 840 G9", ALC245_FIXUP_CS35L41_SPI_2),
+	SND_PCI_QUIRK(0x103c, 0x8973, "HP EliteBook 860 G9", ALC245_FIXUP_CS35L41_SPI_2),
+	SND_PCI_QUIRK(0x103c, 0x8974, "HP EliteBook 840 Aero G9", ALC245_FIXUP_CS35L41_SPI_2),
+	SND_PCI_QUIRK(0x103c, 0x8975, "HP EliteBook x360 840 Aero G9", ALC245_FIXUP_CS35L41_SPI_2),
+	SND_PCI_QUIRK(0x103c, 0x8981, "HP Elite Dragonfly G3", ALC245_FIXUP_CS35L41_SPI_4),
+	SND_PCI_QUIRK(0x103c, 0x898e, "HP EliteBook 835 G9", ALC287_FIXUP_CS35L41_I2C_2),
+	SND_PCI_QUIRK(0x103c, 0x898f, "HP EliteBook 835 G9", ALC287_FIXUP_CS35L41_I2C_2),
+	SND_PCI_QUIRK(0x103c, 0x8991, "HP EliteBook 845 G9", ALC287_FIXUP_CS35L41_I2C_2),
+	SND_PCI_QUIRK(0x103c, 0x8992, "HP EliteBook 845 G9", ALC287_FIXUP_CS35L41_I2C_2),
+	SND_PCI_QUIRK(0x103c, 0x8994, "HP EliteBook 855 G9", ALC287_FIXUP_CS35L41_I2C_2),
+	SND_PCI_QUIRK(0x103c, 0x8995, "HP EliteBook 855 G9", ALC287_FIXUP_CS35L41_I2C_2),
+	SND_PCI_QUIRK(0x103c, 0x89c3, "Zbook Studio G9", ALC245_FIXUP_CS35L41_SPI_4_HP_GPIO_LED),
+	SND_PCI_QUIRK(0x103c, 0x89c6, "Zbook Fury 17 G9", ALC245_FIXUP_CS35L41_SPI_2),
 	SND_PCI_QUIRK(0x103c, 0x89ca, "HP", ALC236_FIXUP_HP_MUTE_LED_MICMUTE_VREF),
 	SND_PCI_QUIRK(0x1043, 0x103e, "ASUS X540SA", ALC256_FIXUP_ASUS_MIC),
 	SND_PCI_QUIRK(0x1043, 0x103f, "ASUS TX300", ALC282_FIXUP_ASUS_TX300),
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 9/9] ACPI / scan: Create platform device for CS35L41
  2022-01-21 17:24 [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Stefan Binding
                   ` (7 preceding siblings ...)
  2022-01-21 17:24 ` [PATCH v6 8/9] ALSA: hda/realtek: Add support for HP Laptops Stefan Binding
@ 2022-01-21 17:24 ` Stefan Binding
  2022-02-01 15:03   ` Hans de Goede
  2022-02-01 14:25 ` [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Hans de Goede
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 29+ messages in thread
From: Stefan Binding @ 2022-01-21 17:24 UTC (permalink / raw)
  To: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches, Lucas Tanure, Stefan Binding

From: Lucas Tanure <tanureal@opensource.cirrus.com>

The ACPI device with CSC3551 or CLSA0100 are sound cards
with multiple instances of CS35L41 connected by I2C or SPI
to the main CPU.

We add an ID to the ignore_serial_bus_ids list to enumerate
all I2C or SPI devices correctly.

The same IDs are also added into serial-multi-instantiate
so that the driver can correctly enumerate the ACPI.

Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 drivers/acpi/scan.c                             |  3 +++
 drivers/platform/x86/serial-multi-instantiate.c | 14 ++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 48db5e80c2dc..ebd10af3ff7f 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1744,8 +1744,11 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)
 	 */
 		{"BSG1160", },
 		{"BSG2150", },
+		{"CSC3551", },
 		{"INT33FE", },
 		{"INT3515", },
+	/* Non-conforming _HID for Cirrus Logic already released */
+		{"CLSA0100", },
 	/*
 	 * HIDs of device with an UartSerialBusV2 resource for which userspace
 	 * expects a regular tty cdev to be created (instead of the in kernel
diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
index 3f05385ca2cf..d09f11eac4f8 100644
--- a/drivers/platform/x86/serial-multi-instantiate.c
+++ b/drivers/platform/x86/serial-multi-instantiate.c
@@ -308,6 +308,17 @@ static const struct smi_node int3515_data = {
 	.bus_type = SMI_I2C,
 };
 
+static const struct smi_node cs35l41_hda = {
+	.instances = {
+		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
+		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
+		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
+		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
+		{}
+	},
+	.bus_type = SMI_AUTO_DETECT,
+};
+
 /*
  * Note new device-ids must also be added to serial_multi_instantiate_ids in
  * drivers/acpi/scan.c: acpi_device_enumeration_by_parent().
@@ -316,6 +327,9 @@ static const struct acpi_device_id smi_acpi_ids[] = {
 	{ "BSG1160", (unsigned long)&bsg1160_data },
 	{ "BSG2150", (unsigned long)&bsg2150_data },
 	{ "INT3515", (unsigned long)&int3515_data },
+	{ "CSC3551", (unsigned long)&cs35l41_hda },
+	/* Non-conforming _HID for Cirrus Logic already released */
+	{ "CLSA0100", (unsigned long)&cs35l41_hda },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, smi_acpi_ids);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver
  2022-01-21 17:24 [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Stefan Binding
                   ` (8 preceding siblings ...)
  2022-01-21 17:24 ` [PATCH v6 9/9] ACPI / scan: Create platform device for CS35L41 Stefan Binding
@ 2022-02-01 14:25 ` Hans de Goede
  2022-02-01 17:18 ` (subset) " Mark Brown
  2022-02-01 17:41 ` Mark Brown
  11 siblings, 0 replies; 29+ messages in thread
From: Hans de Goede @ 2022-02-01 14:25 UTC (permalink / raw)
  To: Stefan Binding, Mark Brown, Rafael J . Wysocki, Len Brown,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches

Hi Stefan,

On 1/21/22 18:24, Stefan Binding wrote:
> Add support for SPI bus in the i2c-multi-instantiate driver as
> upcoming laptops will need to multi instantiate SPI devices from
> a single device node, which has multiple SpiSerialBus entries at
> the ACPI table.
> 
> With the new SPI support, i2c-multi-instantiate becomes
> bus-multi-instantiate and is moved to the ACPI folder.
> 
> The intention is to support the SPI bus by re-using the current
> I2C multi instantiate, instead of creating a new SPI multi
> instantiate, to make it possible for peripherals that can be
> controlled by I2C or SPI to have the same HID at the ACPI table.
> 
> The new driver (serial multi instantiate, smi) checks for the
> hard-coded bus type and returns -ENODEV in case of zero devices
> found for that bus. In the case of automatic bus detection, 
> the driver will give preference to I2C.
> 
> The expectation is for a device node in the ACPI table to have
> multiple I2cSerialBus only or multiple SpiSerialBus only, not
> a mix of both; and for the case where there are both entries in
> one device node, only the I2C ones would be probed.
> 
> This new serial multi instantiate will be used in CS35L41 HDA new
> driver.
> 
> Changes since V5:
>  - comment, commit message and Kconfig description fixes
>  - minor fixes in serial-multi-instantiate
>  - use lowercase for SSIDs in patch_realtek.c

Thank you for the new versions.

I gave a bunch of:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

tags for v4, unless you make big changes, you are supposed to
collect this tag and add it to newer versions of the
patch-set (just above your own S-o-b).

I'll resend the Reviewed-by-s for this version, please
collect them for the next version (I noticed there are
still some small issues, sorry).

Regards,

Hans



> 
> Lucas Tanure (4):
>   platform/x86: i2c-multi-instantiate: Rename it for a generic serial
>     driver name
>   platform/x86: serial-multi-instantiate: Reorganize I2C functions
>   ALSA: hda/realtek: Add support for HP Laptops
>   ACPI / scan: Create platform device for CS35L41
> 
> Stefan Binding (5):
>   spi: Make spi_alloc_device and spi_add_device public again
>   spi: Create helper API to lookup ACPI info for spi device
>   spi: Support selection of the index of the ACPI Spi Resource before
>     alloc
>   spi: Add API to count spi acpi resources
>   platform/x86: serial-multi-instantiate: Add SPI support
> 
>  MAINTAINERS                                   |   4 +-
>  drivers/acpi/scan.c                           |  16 +-
>  drivers/platform/x86/Kconfig                  |  12 +-
>  drivers/platform/x86/Makefile                 |   2 +-
>  drivers/platform/x86/i2c-multi-instantiate.c  | 174 ---------
>  .../platform/x86/serial-multi-instantiate.c   | 349 ++++++++++++++++++
>  drivers/spi/spi.c                             | 137 ++++++-
>  include/linux/spi/spi.h                       |  20 +
>  sound/pci/hda/patch_realtek.c                 |  43 ++-
>  9 files changed, 551 insertions(+), 206 deletions(-)
>  delete mode 100644 drivers/platform/x86/i2c-multi-instantiate.c
>  create mode 100644 drivers/platform/x86/serial-multi-instantiate.c
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 1/9] spi: Make spi_alloc_device and spi_add_device public again
  2022-01-21 17:24 ` [PATCH v6 1/9] spi: Make spi_alloc_device and spi_add_device public again Stefan Binding
@ 2022-02-01 14:26   ` Hans de Goede
  2022-02-01 14:52     ` Rafael J. Wysocki
  0 siblings, 1 reply; 29+ messages in thread
From: Hans de Goede @ 2022-02-01 14:26 UTC (permalink / raw)
  To: Stefan Binding, Mark Brown, Rafael J . Wysocki, Len Brown,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches

Hi,

On 1/21/22 18:24, Stefan Binding wrote:
> This functions were previously made private since they
> were not used. However, these functions will be needed
> again.
> 
> Partial revert of commit da21fde0fdb3
> ("spi: Make several public functions private to spi.c")
> 
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  drivers/spi/spi.c       |  6 ++++--
>  include/linux/spi/spi.h | 12 ++++++++++++
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 4599b121d744..1eb84101c4ad 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -532,7 +532,7 @@ static DEFINE_MUTEX(board_lock);
>   *
>   * Return: a pointer to the new device, or NULL.
>   */
> -static struct spi_device *spi_alloc_device(struct spi_controller *ctlr)
> +struct spi_device *spi_alloc_device(struct spi_controller *ctlr)
>  {
>  	struct spi_device	*spi;
>  
> @@ -557,6 +557,7 @@ static struct spi_device *spi_alloc_device(struct spi_controller *ctlr)
>  	device_initialize(&spi->dev);
>  	return spi;
>  }
> +EXPORT_SYMBOL_GPL(spi_alloc_device);
>  
>  static void spi_dev_set_name(struct spi_device *spi)
>  {
> @@ -652,7 +653,7 @@ static int __spi_add_device(struct spi_device *spi)
>   *
>   * Return: 0 on success; negative errno on failure
>   */
> -static int spi_add_device(struct spi_device *spi)
> +int spi_add_device(struct spi_device *spi)
>  {
>  	struct spi_controller *ctlr = spi->controller;
>  	struct device *dev = ctlr->dev.parent;
> @@ -673,6 +674,7 @@ static int spi_add_device(struct spi_device *spi)
>  	mutex_unlock(&ctlr->add_lock);
>  	return status;
>  }
> +EXPORT_SYMBOL_GPL(spi_add_device);
>  
>  static int spi_add_device_locked(struct spi_device *spi)
>  {
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index 7ab3fed7b804..0346a3ff27fd 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -1452,7 +1452,19 @@ spi_register_board_info(struct spi_board_info const *info, unsigned n)
>   * use spi_new_device() to describe each device.  You can also call
>   * spi_unregister_device() to start making that device vanish, but
>   * normally that would be handled by spi_unregister_controller().
> + *
> + * You can also use spi_alloc_device() and spi_add_device() to use a two
> + * stage registration sequence for each spi_device. This gives the caller
> + * some more control over the spi_device structure before it is registered,
> + * but requires that caller to initialize fields that would otherwise
> + * be defined using the board info.
>   */
> +extern struct spi_device *
> +spi_alloc_device(struct spi_controller *ctlr);
> +
> +extern int
> +spi_add_device(struct spi_device *spi);
> +
>  extern struct spi_device *
>  spi_new_device(struct spi_controller *, struct spi_board_info *);
>  
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 2/9] spi: Create helper API to lookup ACPI info for spi device
  2022-01-21 17:24 ` [PATCH v6 2/9] spi: Create helper API to lookup ACPI info for spi device Stefan Binding
@ 2022-02-01 14:28   ` Hans de Goede
  2022-02-01 17:28     ` Stefan Binding
  0 siblings, 1 reply; 29+ messages in thread
From: Hans de Goede @ 2022-02-01 14:28 UTC (permalink / raw)
  To: Stefan Binding, Mark Brown, Rafael J . Wysocki, Len Brown,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches

Hi,

On 1/21/22 18:24, Stefan Binding wrote:
> This can then be used to find a spi resource inside an
> ACPI node, and allocate a spi device.
> 
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
> ---
>  drivers/spi/spi.c       | 46 ++++++++++++++++++++++++++++++++---------
>  include/linux/spi/spi.h |  6 ++++++
>  2 files changed, 42 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 1eb84101c4ad..13f4701f0694 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2410,8 +2410,18 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)
>  	return 1;
>  }
>  
> -static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
> -					    struct acpi_device *adev)
> +/**
> + * acpi_spi_device_alloc - Allocate a spi device, and fill it in with ACPI information
> + * @ctlr: controller to which the spi device belongs
> + * @adev: ACPI Device for the spi device
> + *
> + * This should be used to allocate a new spi device from and ACPI Node.
> + * The caller is responsible for calling spi_add_device to register the spi device.
> + *
> + * Return: a pointer to the new device, or ERR_PTR on error.
> + */
> +struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
> +					 struct acpi_device *adev)
>  {
>  	acpi_handle parent_handle = NULL;
>  	struct list_head resource_list;
> @@ -2419,10 +2429,6 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
>  	struct spi_device *spi;
>  	int ret;
>  
> -	if (acpi_bus_get_status(adev) || !adev->status.present ||
> -	    acpi_device_enumerated(adev))
> -		return AE_OK;
> -
>  	lookup.ctlr		= ctlr;
>  	lookup.irq		= -1;
>  
> @@ -2433,7 +2439,7 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
>  
>  	if (ret < 0)
>  		/* found SPI in _CRS but it points to another controller */
> -		return AE_OK;
> +		return ERR_PTR(-ENODEV);
>  
>  	if (!lookup.max_speed_hz &&
>  	    ACPI_SUCCESS(acpi_get_parent(adev->handle, &parent_handle)) &&
> @@ -2443,16 +2449,15 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
>  	}
>  
>  	if (!lookup.max_speed_hz)
> -		return AE_OK;
> +		return ERR_PTR(-ENODEV);
>  
>  	spi = spi_alloc_device(ctlr);
>  	if (!spi) {
>  		dev_err(&ctlr->dev, "failed to allocate SPI device for %s\n",
>  			dev_name(&adev->dev));
> -		return AE_NO_MEMORY;
> +		return ERR_PTR(-ENOMEM);
>  	}
>  
> -
>  	ACPI_COMPANION_SET(&spi->dev, adev);
>  	spi->max_speed_hz	= lookup.max_speed_hz;
>  	spi->mode		|= lookup.mode;
> @@ -2460,6 +2465,27 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
>  	spi->bits_per_word	= lookup.bits_per_word;
>  	spi->chip_select	= lookup.chip_select;
>  
> +	return spi;
> +}
> +EXPORT_SYMBOL_GPL(acpi_spi_device_alloc);
> +
> +static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
> +					    struct acpi_device *adev)
> +{
> +	struct spi_device *spi;
> +
> +	if (acpi_bus_get_status(adev) || !adev->status.present ||
> +	    acpi_device_enumerated(adev))
> +		return AE_OK;
> +
> +	spi = acpi_spi_device_alloc(ctlr, adev);
> +	if (IS_ERR(spi)) {
> +		if (PTR_ERR(spi) == -ENOMEM)
> +			return AE_NO_MEMORY;
> +		else
> +			return AE_OK;
> +	}
> +
>  	acpi_set_modalias(adev, acpi_device_hid(adev), spi->modalias,
>  			  sizeof(spi->modalias));
>  
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index 0346a3ff27fd..d159cef12f1a 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -16,6 +16,7 @@
>  #include <linux/gpio/consumer.h>
>  
>  #include <uapi/linux/spi/spi.h>
> +#include <linux/acpi.h>
>  
>  struct dma_chan;
>  struct software_node;
> @@ -759,6 +760,11 @@ extern int devm_spi_register_controller(struct device *dev,
>  					struct spi_controller *ctlr);
>  extern void spi_unregister_controller(struct spi_controller *ctlr);
>  
> +#if IS_ENABLED(CONFIG_ACPI)
> +extern struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
> +						struct acpi_device *adev);
> +#endif
> +

There is no need to add a #ifdef about something which is just a
function prototype. Having this declared when CONFIG_ACPI is not set is
harmless, please drop the #ifdef.

With that fixed, please add my R-b to the next version:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



>  /*
>   * SPI resource management while processing a SPI message
>   */
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 3/9] spi: Support selection of the index of the ACPI Spi Resource before alloc
  2022-01-21 17:24 ` [PATCH v6 3/9] spi: Support selection of the index of the ACPI Spi Resource before alloc Stefan Binding
@ 2022-02-01 14:29   ` Hans de Goede
  0 siblings, 0 replies; 29+ messages in thread
From: Hans de Goede @ 2022-02-01 14:29 UTC (permalink / raw)
  To: Stefan Binding, Mark Brown, Rafael J . Wysocki, Len Brown,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches

Hi,

On 1/21/22 18:24, Stefan Binding wrote:
> If a node contains more than one Spi Resources, it may be necessary to
> use an index to select which one you want to allocate a spi device for.
> 
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  drivers/spi/spi.c       | 51 +++++++++++++++++++++++++++++++++++------
>  include/linux/spi/spi.h |  3 ++-
>  2 files changed, 46 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 13f4701f0694..06c0a308b38b 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2320,6 +2320,8 @@ struct acpi_spi_lookup {
>  	int			irq;
>  	u8			bits_per_word;
>  	u8			chip_select;
> +	int			n;
> +	int			index;
>  };
>  
>  static void acpi_spi_parse_apple_properties(struct acpi_device *dev,
> @@ -2351,6 +2353,8 @@ static void acpi_spi_parse_apple_properties(struct acpi_device *dev,
>  		lookup->mode |= SPI_CPHA;
>  }
>  
> +static struct spi_controller *acpi_spi_find_controller_by_adev(struct acpi_device *adev);
> +
>  static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)
>  {
>  	struct acpi_spi_lookup *lookup = data;
> @@ -2364,14 +2368,35 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)
>  		sb = &ares->data.spi_serial_bus;
>  		if (sb->type == ACPI_RESOURCE_SERIAL_TYPE_SPI) {
>  
> +			if (lookup->index != -1 && lookup->n++ != lookup->index)
> +				return 1;
> +
> +			if (lookup->index == -1 && !ctlr)
> +				return -ENODEV;
> +
>  			status = acpi_get_handle(NULL,
>  						 sb->resource_source.string_ptr,
>  						 &parent_handle);
>  
> -			if (ACPI_FAILURE(status) ||
> -			    ACPI_HANDLE(ctlr->dev.parent) != parent_handle)
> +			if (ACPI_FAILURE(status))
>  				return -ENODEV;
>  
> +			if (ctlr) {
> +				if (ACPI_HANDLE(ctlr->dev.parent) != parent_handle)
> +					return -ENODEV;
> +			} else {
> +				struct acpi_device *adev;
> +
> +				if (acpi_bus_get_device(parent_handle, &adev))
> +					return -ENODEV;
> +
> +				ctlr = acpi_spi_find_controller_by_adev(adev);
> +				if (!ctlr)
> +					return -ENODEV;
> +
> +				lookup->ctlr = ctlr;
> +			}
> +
>  			/*
>  			 * ACPI DeviceSelection numbering is handled by the
>  			 * host controller driver in Windows and can vary
> @@ -2414,14 +2439,21 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)
>   * acpi_spi_device_alloc - Allocate a spi device, and fill it in with ACPI information
>   * @ctlr: controller to which the spi device belongs
>   * @adev: ACPI Device for the spi device
> + * @index: Index of the spi resource inside the ACPI Node
>   *
>   * This should be used to allocate a new spi device from and ACPI Node.
>   * The caller is responsible for calling spi_add_device to register the spi device.
>   *
> + * If ctlr is set to NULL, the Controller for the spi device will be looked up
> + * using the resource.
> + * If index is set to -1, index is not used.
> + * Note: If index is -1, ctlr must be set.
> + *
>   * Return: a pointer to the new device, or ERR_PTR on error.
>   */
>  struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
> -					 struct acpi_device *adev)
> +					 struct acpi_device *adev,
> +					 int index)
>  {
>  	acpi_handle parent_handle = NULL;
>  	struct list_head resource_list;
> @@ -2429,8 +2461,13 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
>  	struct spi_device *spi;
>  	int ret;
>  
> +	if (!ctlr && index == -1)
> +		return ERR_PTR(-EINVAL);
> +
>  	lookup.ctlr		= ctlr;
>  	lookup.irq		= -1;
> +	lookup.index		= index;
> +	lookup.n		= 0;
>  
>  	INIT_LIST_HEAD(&resource_list);
>  	ret = acpi_dev_get_resources(adev, &resource_list,
> @@ -2443,7 +2480,7 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
>  
>  	if (!lookup.max_speed_hz &&
>  	    ACPI_SUCCESS(acpi_get_parent(adev->handle, &parent_handle)) &&
> -	    ACPI_HANDLE(ctlr->dev.parent) == parent_handle) {
> +	    ACPI_HANDLE(lookup.ctlr->dev.parent) == parent_handle) {
>  		/* Apple does not use _CRS but nested devices for SPI slaves */
>  		acpi_spi_parse_apple_properties(adev, &lookup);
>  	}
> @@ -2451,9 +2488,9 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
>  	if (!lookup.max_speed_hz)
>  		return ERR_PTR(-ENODEV);
>  
> -	spi = spi_alloc_device(ctlr);
> +	spi = spi_alloc_device(lookup.ctlr);
>  	if (!spi) {
> -		dev_err(&ctlr->dev, "failed to allocate SPI device for %s\n",
> +		dev_err(&lookup.ctlr->dev, "failed to allocate SPI device for %s\n",
>  			dev_name(&adev->dev));
>  		return ERR_PTR(-ENOMEM);
>  	}
> @@ -2478,7 +2515,7 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
>  	    acpi_device_enumerated(adev))
>  		return AE_OK;
>  
> -	spi = acpi_spi_device_alloc(ctlr, adev);
> +	spi = acpi_spi_device_alloc(ctlr, adev, -1);
>  	if (IS_ERR(spi)) {
>  		if (PTR_ERR(spi) == -ENOMEM)
>  			return AE_NO_MEMORY;
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index d159cef12f1a..e5bbb9cbd3d7 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -762,7 +762,8 @@ extern void spi_unregister_controller(struct spi_controller *ctlr);
>  
>  #if IS_ENABLED(CONFIG_ACPI)
>  extern struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
> -						struct acpi_device *adev);
> +						struct acpi_device *adev,
> +						int index);
>  #endif
>  
>  /*
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 4/9] spi: Add API to count spi acpi resources
  2022-01-21 17:24 ` [PATCH v6 4/9] spi: Add API to count spi acpi resources Stefan Binding
@ 2022-02-01 14:42   ` Hans de Goede
  0 siblings, 0 replies; 29+ messages in thread
From: Hans de Goede @ 2022-02-01 14:42 UTC (permalink / raw)
  To: Stefan Binding, Mark Brown, Rafael J . Wysocki, Len Brown,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches

Hi,

On 1/21/22 18:24, Stefan Binding wrote:
> Some ACPI nodes may have more than one Spi Resource.
> To be able to handle these case, its necessary to have
> a way of counting these resources.
> 
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  drivers/spi/spi.c       | 40 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/spi/spi.h |  1 +
>  2 files changed, 41 insertions(+)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 06c0a308b38b..ec9f2ed579e3 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2324,6 +2324,46 @@ struct acpi_spi_lookup {
>  	int			index;
>  };
>  
> +static int acpi_spi_count(struct acpi_resource *ares, void *data)
> +{
> +	struct acpi_resource_spi_serialbus *sb;
> +	int *count = data;
> +
> +	if (ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS)
> +		return 1;
> +
> +	sb = &ares->data.spi_serial_bus;
> +	if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_SPI)
> +		return 1;
> +
> +	*count = *count + 1;
> +
> +	return 1;
> +}
> +
> +/**
> + * acpi_spi_count_resources - Count the number of SpiSerialBus resources
> + * @adev:	ACPI device
> + *
> + * Returns the number of SpiSerialBus resources in the ACPI-device's
> + * resource-list; or a negative error code.
> + */
> +int acpi_spi_count_resources(struct acpi_device *adev)
> +{
> +	LIST_HEAD(r);
> +	int count = 0;
> +	int ret;
> +
> +	ret = acpi_dev_get_resources(adev, &r, acpi_spi_count, &count);
> +	if (ret < 0)
> +		return ret;
> +
> +	acpi_dev_free_resource_list(&r);
> +
> +	return count;
> +}
> +EXPORT_SYMBOL_GPL(acpi_spi_count_resources);
> +
>  static void acpi_spi_parse_apple_properties(struct acpi_device *dev,
>  					    struct acpi_spi_lookup *lookup)
>  {
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index e5bbb9cbd3d7..394b4241d989 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -764,6 +764,7 @@ extern void spi_unregister_controller(struct spi_controller *ctlr);
>  extern struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
>  						struct acpi_device *adev,
>  						int index);
> +int acpi_spi_count_resources(struct acpi_device *adev);
>  #endif
>  
>  /*
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 1/9] spi: Make spi_alloc_device and spi_add_device public again
  2022-02-01 14:26   ` Hans de Goede
@ 2022-02-01 14:52     ` Rafael J. Wysocki
  2022-02-01 14:54       ` Hans de Goede
  0 siblings, 1 reply; 29+ messages in thread
From: Rafael J. Wysocki @ 2022-02-01 14:52 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Stefan Binding, Mark Brown, Rafael J . Wysocki, Len Brown,
	Mark Gross, Jaroslav Kysela, Takashi Iwai,
	moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	Linux Kernel Mailing List, linux-spi, ACPI Devel Maling List,
	Platform Driver, patches

On Tue, Feb 1, 2022 at 3:26 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 1/21/22 18:24, Stefan Binding wrote:
> > This functions were previously made private since they
> > were not used. However, these functions will be needed
> > again.
> >
> > Partial revert of commit da21fde0fdb3
> > ("spi: Make several public functions private to spi.c")
> >
> > Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
>
> Thanks, patch looks good to me:
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

The series also looks good to me from the ACPI side, so what tree
should it go into?

> > ---
> >  drivers/spi/spi.c       |  6 ++++--
> >  include/linux/spi/spi.h | 12 ++++++++++++
> >  2 files changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> > index 4599b121d744..1eb84101c4ad 100644
> > --- a/drivers/spi/spi.c
> > +++ b/drivers/spi/spi.c
> > @@ -532,7 +532,7 @@ static DEFINE_MUTEX(board_lock);
> >   *
> >   * Return: a pointer to the new device, or NULL.
> >   */
> > -static struct spi_device *spi_alloc_device(struct spi_controller *ctlr)
> > +struct spi_device *spi_alloc_device(struct spi_controller *ctlr)
> >  {
> >       struct spi_device       *spi;
> >
> > @@ -557,6 +557,7 @@ static struct spi_device *spi_alloc_device(struct spi_controller *ctlr)
> >       device_initialize(&spi->dev);
> >       return spi;
> >  }
> > +EXPORT_SYMBOL_GPL(spi_alloc_device);
> >
> >  static void spi_dev_set_name(struct spi_device *spi)
> >  {
> > @@ -652,7 +653,7 @@ static int __spi_add_device(struct spi_device *spi)
> >   *
> >   * Return: 0 on success; negative errno on failure
> >   */
> > -static int spi_add_device(struct spi_device *spi)
> > +int spi_add_device(struct spi_device *spi)
> >  {
> >       struct spi_controller *ctlr = spi->controller;
> >       struct device *dev = ctlr->dev.parent;
> > @@ -673,6 +674,7 @@ static int spi_add_device(struct spi_device *spi)
> >       mutex_unlock(&ctlr->add_lock);
> >       return status;
> >  }
> > +EXPORT_SYMBOL_GPL(spi_add_device);
> >
> >  static int spi_add_device_locked(struct spi_device *spi)
> >  {
> > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> > index 7ab3fed7b804..0346a3ff27fd 100644
> > --- a/include/linux/spi/spi.h
> > +++ b/include/linux/spi/spi.h
> > @@ -1452,7 +1452,19 @@ spi_register_board_info(struct spi_board_info const *info, unsigned n)
> >   * use spi_new_device() to describe each device.  You can also call
> >   * spi_unregister_device() to start making that device vanish, but
> >   * normally that would be handled by spi_unregister_controller().
> > + *
> > + * You can also use spi_alloc_device() and spi_add_device() to use a two
> > + * stage registration sequence for each spi_device. This gives the caller
> > + * some more control over the spi_device structure before it is registered,
> > + * but requires that caller to initialize fields that would otherwise
> > + * be defined using the board info.
> >   */
> > +extern struct spi_device *
> > +spi_alloc_device(struct spi_controller *ctlr);
> > +
> > +extern int
> > +spi_add_device(struct spi_device *spi);
> > +
> >  extern struct spi_device *
> >  spi_new_device(struct spi_controller *, struct spi_board_info *);
> >
> >
>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 1/9] spi: Make spi_alloc_device and spi_add_device public again
  2022-02-01 14:52     ` Rafael J. Wysocki
@ 2022-02-01 14:54       ` Hans de Goede
  0 siblings, 0 replies; 29+ messages in thread
From: Hans de Goede @ 2022-02-01 14:54 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Stefan Binding, Mark Brown, Len Brown, Mark Gross,
	Jaroslav Kysela, Takashi Iwai,
	moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	Linux Kernel Mailing List, linux-spi, ACPI Devel Maling List,
	Platform Driver, patches

Hi,

On 2/1/22 15:52, Rafael J. Wysocki wrote:
> On Tue, Feb 1, 2022 at 3:26 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 1/21/22 18:24, Stefan Binding wrote:
>>> This functions were previously made private since they
>>> were not used. However, these functions will be needed
>>> again.
>>>
>>> Partial revert of commit da21fde0fdb3
>>> ("spi: Make several public functions private to spi.c")
>>>
>>> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
>>
>> Thanks, patch looks good to me:
>>
>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> The series also looks good to me from the ACPI side, so what tree
> should it go into?

Minus the ALSA patch from 8/9 which should go through the sound
tree AFAIK, I would be happy to pick up the entire series in the
pdx86 tree.

This requires an ack from Mark though for me merging the spi bits,
Mark ?

Regards,

Hans


> 
>>> ---
>>>  drivers/spi/spi.c       |  6 ++++--
>>>  include/linux/spi/spi.h | 12 ++++++++++++
>>>  2 files changed, 16 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>>> index 4599b121d744..1eb84101c4ad 100644
>>> --- a/drivers/spi/spi.c
>>> +++ b/drivers/spi/spi.c
>>> @@ -532,7 +532,7 @@ static DEFINE_MUTEX(board_lock);
>>>   *
>>>   * Return: a pointer to the new device, or NULL.
>>>   */
>>> -static struct spi_device *spi_alloc_device(struct spi_controller *ctlr)
>>> +struct spi_device *spi_alloc_device(struct spi_controller *ctlr)
>>>  {
>>>       struct spi_device       *spi;
>>>
>>> @@ -557,6 +557,7 @@ static struct spi_device *spi_alloc_device(struct spi_controller *ctlr)
>>>       device_initialize(&spi->dev);
>>>       return spi;
>>>  }
>>> +EXPORT_SYMBOL_GPL(spi_alloc_device);
>>>
>>>  static void spi_dev_set_name(struct spi_device *spi)
>>>  {
>>> @@ -652,7 +653,7 @@ static int __spi_add_device(struct spi_device *spi)
>>>   *
>>>   * Return: 0 on success; negative errno on failure
>>>   */
>>> -static int spi_add_device(struct spi_device *spi)
>>> +int spi_add_device(struct spi_device *spi)
>>>  {
>>>       struct spi_controller *ctlr = spi->controller;
>>>       struct device *dev = ctlr->dev.parent;
>>> @@ -673,6 +674,7 @@ static int spi_add_device(struct spi_device *spi)
>>>       mutex_unlock(&ctlr->add_lock);
>>>       return status;
>>>  }
>>> +EXPORT_SYMBOL_GPL(spi_add_device);
>>>
>>>  static int spi_add_device_locked(struct spi_device *spi)
>>>  {
>>> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
>>> index 7ab3fed7b804..0346a3ff27fd 100644
>>> --- a/include/linux/spi/spi.h
>>> +++ b/include/linux/spi/spi.h
>>> @@ -1452,7 +1452,19 @@ spi_register_board_info(struct spi_board_info const *info, unsigned n)
>>>   * use spi_new_device() to describe each device.  You can also call
>>>   * spi_unregister_device() to start making that device vanish, but
>>>   * normally that would be handled by spi_unregister_controller().
>>> + *
>>> + * You can also use spi_alloc_device() and spi_add_device() to use a two
>>> + * stage registration sequence for each spi_device. This gives the caller
>>> + * some more control over the spi_device structure before it is registered,
>>> + * but requires that caller to initialize fields that would otherwise
>>> + * be defined using the board info.
>>>   */
>>> +extern struct spi_device *
>>> +spi_alloc_device(struct spi_controller *ctlr);
>>> +
>>> +extern int
>>> +spi_add_device(struct spi_device *spi);
>>> +
>>>  extern struct spi_device *
>>>  spi_new_device(struct spi_controller *, struct spi_board_info *);
>>>
>>>
>>
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 5/9] platform/x86: i2c-multi-instantiate: Rename it for a generic serial driver name
  2022-01-21 17:24 ` [PATCH v6 5/9] platform/x86: i2c-multi-instantiate: Rename it for a generic serial driver name Stefan Binding
@ 2022-02-01 14:55   ` Hans de Goede
  0 siblings, 0 replies; 29+ messages in thread
From: Hans de Goede @ 2022-02-01 14:55 UTC (permalink / raw)
  To: Stefan Binding, Mark Brown, Rafael J . Wysocki, Len Brown,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches, Lucas Tanure

Hi,

On 1/21/22 18:24, Stefan Binding wrote:
> From: Lucas Tanure <tanureal@opensource.cirrus.com>
> 
> Rename I2C multi instantiate driver to serial-multi-instantiate for
> upcoming addition of SPI support
> 
> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  MAINTAINERS                                   |  4 +-
>  drivers/acpi/scan.c                           | 13 +--
>  drivers/platform/x86/Kconfig                  | 10 +-
>  drivers/platform/x86/Makefile                 |  2 +-
>  ...stantiate.c => serial-multi-instantiate.c} | 91 +++++++++----------
>  5 files changed, 60 insertions(+), 60 deletions(-)
>  rename drivers/platform/x86/{i2c-multi-instantiate.c => serial-multi-instantiate.c} (51%)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4e828542b089..be50537ea6bd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -388,11 +388,11 @@ L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>  S:	Maintained
>  F:	drivers/acpi/arm64
>  
> -ACPI I2C MULTI INSTANTIATE DRIVER
> +ACPI SERIAL MULTI INSTANTIATE DRIVER
>  M:	Hans de Goede <hdegoede@redhat.com>
>  L:	platform-driver-x86@vger.kernel.org
>  S:	Maintained
> -F:	drivers/platform/x86/i2c-multi-instantiate.c
> +F:	drivers/platform/x86/serial-multi-instantiate.c
>  
>  ACPI PCC(Platform Communication Channel) MAILBOX DRIVER
>  M:	Sudeep Holla <sudeep.holla@arm.com>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 1331756d4cfc..48db5e80c2dc 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1734,12 +1734,13 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)
>  	bool is_serial_bus_slave = false;
>  	static const struct acpi_device_id ignore_serial_bus_ids[] = {
>  	/*
> -	 * These devices have multiple I2cSerialBus resources and an i2c-client
> -	 * must be instantiated for each, each with its own i2c_device_id.
> -	 * Normally we only instantiate an i2c-client for the first resource,
> -	 * using the ACPI HID as id. These special cases are handled by the
> -	 * drivers/platform/x86/i2c-multi-instantiate.c driver, which knows
> -	 * which i2c_device_id to use for each resource.
> +	 * These devices have multiple SerialBus resources and a client
> +	 * device must be instantiated for each of them, each with
> +	 * its own device id.
> +	 * Normally we only instantiate one client device for the first
> +	 * resource, using the ACPI HID as id. These special cases are handled
> +	 * by the drivers/platform/x86/serial-multi-instantiate.c driver, which
> +	 * knows which client device id to use for each resource.
>  	 */
>  		{"BSG1160", },
>  		{"BSG2150", },
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 24deeeb29af2..2e656909a866 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -990,16 +990,16 @@ config TOPSTAR_LAPTOP
>  
>  	  If you have a Topstar laptop, say Y or M here.
>  
> -config I2C_MULTI_INSTANTIATE
> -	tristate "I2C multi instantiate pseudo device driver"
> +config SERIAL_MULTI_INSTANTIATE
> +	tristate "Serial bus multi instantiate pseudo device driver"
>  	depends on I2C && ACPI
>  	help
> -	  Some ACPI-based systems list multiple i2c-devices in a single ACPI
> -	  firmware-node. This driver will instantiate separate i2c-clients
> +	  Some ACPI-based systems list multiple devices in a single ACPI
> +	  firmware-node. This driver will instantiate separate clients
>  	  for each device in the firmware-node.
>  
>  	  To compile this driver as a module, choose M here: the module
> -	  will be called i2c-multi-instantiate.
> +	  will be called serial-multi-instantiate.
>  
>  config MLX_PLATFORM
>  	tristate "Mellanox Technologies platform support"
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index c12a9b044fd8..9527088bba7f 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -110,7 +110,7 @@ obj-$(CONFIG_TOPSTAR_LAPTOP)	+= topstar-laptop.o
>  
>  # Platform drivers
>  obj-$(CONFIG_FW_ATTR_CLASS)		+= firmware_attributes_class.o
> -obj-$(CONFIG_I2C_MULTI_INSTANTIATE)	+= i2c-multi-instantiate.o
> +obj-$(CONFIG_SERIAL_MULTI_INSTANTIATE)	+= serial-multi-instantiate.o
>  obj-$(CONFIG_MLX_PLATFORM)		+= mlx-platform.o
>  obj-$(CONFIG_TOUCHSCREEN_DMI)		+= touchscreen_dmi.o
>  obj-$(CONFIG_WIRELESS_HOTKEY)		+= wireless-hotkey.o
> diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
> similarity index 51%
> rename from drivers/platform/x86/i2c-multi-instantiate.c
> rename to drivers/platform/x86/serial-multi-instantiate.c
> index 4956a1df5b90..33cbb0caed33 100644
> --- a/drivers/platform/x86/i2c-multi-instantiate.c
> +++ b/drivers/platform/x86/serial-multi-instantiate.c
> @@ -1,7 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
> - * I2C multi-instantiate driver, pseudo driver to instantiate multiple
> - * i2c-clients from a single fwnode.
> + * Serial multi-instantiate driver, pseudo driver to instantiate multiple
> + * client devices from a single fwnode.
>   *
>   * Copyright 2018 Hans de Goede <hdegoede@redhat.com>
>   */
> @@ -21,29 +21,29 @@
>  #define IRQ_RESOURCE_GPIO	1
>  #define IRQ_RESOURCE_APIC	2
>  
> -struct i2c_inst_data {
> +struct smi_instance {
>  	const char *type;
>  	unsigned int flags;
>  	int irq_idx;
>  };
>  
> -struct i2c_multi_inst_data {
> -	int num_clients;
> -	struct i2c_client *clients[];
> +struct smi {
> +	int i2c_num;
> +	struct i2c_client *i2c_devs[];
>  };
>  
> -static int i2c_multi_inst_probe(struct platform_device *pdev)
> +static int smi_probe(struct platform_device *pdev)
>  {
> -	struct i2c_multi_inst_data *multi;
> -	const struct i2c_inst_data *inst_data;
>  	struct i2c_board_info board_info = {};
> +	const struct smi_instance *inst;
>  	struct device *dev = &pdev->dev;
>  	struct acpi_device *adev;
> +	struct smi *smi;
>  	char name[32];
>  	int i, ret;
>  
> -	inst_data = device_get_match_data(dev);
> -	if (!inst_data) {
> +	inst = device_get_match_data(dev);
> +	if (!inst) {
>  		dev_err(dev, "Error ACPI match data is missing\n");
>  		return -ENODEV;
>  	}
> @@ -55,33 +55,32 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> -	multi = devm_kmalloc(dev, struct_size(multi, clients, ret), GFP_KERNEL);
> -	if (!multi)
> +	smi = devm_kmalloc(dev, struct_size(smi, i2c_devs, ret), GFP_KERNEL);
> +	if (!smi)
>  		return -ENOMEM;
>  
> -	multi->num_clients = ret;
> +	smi->i2c_num = ret;
>  
> -	for (i = 0; i < multi->num_clients && inst_data[i].type; i++) {
> +	for (i = 0; i < smi->i2c_num && inst[i].type; i++) {
>  		memset(&board_info, 0, sizeof(board_info));
> -		strlcpy(board_info.type, inst_data[i].type, I2C_NAME_SIZE);
> -		snprintf(name, sizeof(name), "%s-%s.%d", dev_name(dev),
> -			 inst_data[i].type, i);
> +		strlcpy(board_info.type, inst[i].type, I2C_NAME_SIZE);
> +		snprintf(name, sizeof(name), "%s-%s.%d", dev_name(dev), inst[i].type, i);
>  		board_info.dev_name = name;
> -		switch (inst_data[i].flags & IRQ_RESOURCE_TYPE) {
> +		switch (inst[i].flags & IRQ_RESOURCE_TYPE) {
>  		case IRQ_RESOURCE_GPIO:
> -			ret = acpi_dev_gpio_irq_get(adev, inst_data[i].irq_idx);
> +			ret = acpi_dev_gpio_irq_get(adev, inst[i].irq_idx);
>  			if (ret < 0) {
>  				dev_err(dev, "Error requesting irq at index %d: %d\n",
> -					inst_data[i].irq_idx, ret);
> +						inst[i].irq_idx, ret);
>  				goto error;
>  			}
>  			board_info.irq = ret;
>  			break;
>  		case IRQ_RESOURCE_APIC:
> -			ret = platform_get_irq(pdev, inst_data[i].irq_idx);
> +			ret = platform_get_irq(pdev, inst[i].irq_idx);
>  			if (ret < 0) {
>  				dev_dbg(dev, "Error requesting irq at index %d: %d\n",
> -					inst_data[i].irq_idx, ret);
> +					inst[i].irq_idx, ret);
>  				goto error;
>  			}
>  			board_info.irq = ret;
> @@ -90,48 +89,48 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
>  			board_info.irq = 0;
>  			break;
>  		}
> -		multi->clients[i] = i2c_acpi_new_device(dev, i, &board_info);
> -		if (IS_ERR(multi->clients[i])) {
> -			ret = dev_err_probe(dev, PTR_ERR(multi->clients[i]),
> +		smi->i2c_devs[i] = i2c_acpi_new_device(dev, i, &board_info);
> +		if (IS_ERR(smi->i2c_devs[i])) {
> +			ret = dev_err_probe(dev, PTR_ERR(smi->i2c_devs[i]),
>  					    "Error creating i2c-client, idx %d\n", i);
>  			goto error;
>  		}
>  	}
> -	if (i < multi->num_clients) {
> +	if (i < smi->i2c_num) {
>  		dev_err(dev, "Error finding driver, idx %d\n", i);
>  		ret = -ENODEV;
>  		goto error;
>  	}
>  
> -	platform_set_drvdata(pdev, multi);
> +	platform_set_drvdata(pdev, smi);
>  	return 0;
>  
>  error:
>  	while (--i >= 0)
> -		i2c_unregister_device(multi->clients[i]);
> +		i2c_unregister_device(smi->i2c_devs[i]);
>  
>  	return ret;
>  }
>  
> -static int i2c_multi_inst_remove(struct platform_device *pdev)
> +static int smi_remove(struct platform_device *pdev)
>  {
> -	struct i2c_multi_inst_data *multi = platform_get_drvdata(pdev);
> +	struct smi *smi = platform_get_drvdata(pdev);
>  	int i;
>  
> -	for (i = 0; i < multi->num_clients; i++)
> -		i2c_unregister_device(multi->clients[i]);
> +	for (i = 0; i < smi->i2c_num; i++)
> +		i2c_unregister_device(smi->i2c_devs[i]);
>  
>  	return 0;
>  }
>  
> -static const struct i2c_inst_data bsg1160_data[]  = {
> +static const struct smi_instance bsg1160_data[]  = {
>  	{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
>  	{ "bmc150_magn" },
>  	{ "bmg160" },
>  	{}
>  };
>  
> -static const struct i2c_inst_data bsg2150_data[]  = {
> +static const struct smi_instance bsg2150_data[]  = {
>  	{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
>  	{ "bmc150_magn" },
>  	/* The resources describe a 3th client, but it is not really there. */
> @@ -139,7 +138,7 @@ static const struct i2c_inst_data bsg2150_data[]  = {
>  	{}
>  };
>  
> -static const struct i2c_inst_data int3515_data[]  = {
> +static const struct smi_instance int3515_data[]  = {
>  	{ "tps6598x", IRQ_RESOURCE_APIC, 0 },
>  	{ "tps6598x", IRQ_RESOURCE_APIC, 1 },
>  	{ "tps6598x", IRQ_RESOURCE_APIC, 2 },
> @@ -148,27 +147,27 @@ static const struct i2c_inst_data int3515_data[]  = {
>  };
>  
>  /*
> - * Note new device-ids must also be added to i2c_multi_instantiate_ids in
> + * Note new device-ids must also be added to serial_multi_instantiate_ids in
>   * drivers/acpi/scan.c: acpi_device_enumeration_by_parent().
>   */
> -static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = {
> +static const struct acpi_device_id smi_acpi_ids[] = {
>  	{ "BSG1160", (unsigned long)bsg1160_data },
>  	{ "BSG2150", (unsigned long)bsg2150_data },
>  	{ "INT3515", (unsigned long)int3515_data },
>  	{ }
>  };
> -MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids);
> +MODULE_DEVICE_TABLE(acpi, smi_acpi_ids);
>  
> -static struct platform_driver i2c_multi_inst_driver = {
> +static struct platform_driver smi_driver = {
>  	.driver	= {
> -		.name = "I2C multi instantiate pseudo device driver",
> -		.acpi_match_table = i2c_multi_inst_acpi_ids,
> +		.name = "Serial bus multi instantiate pseudo device driver",
> +		.acpi_match_table = smi_acpi_ids,
>  	},
> -	.probe = i2c_multi_inst_probe,
> -	.remove = i2c_multi_inst_remove,
> +	.probe = smi_probe,
> +	.remove = smi_remove,
>  };
> -module_platform_driver(i2c_multi_inst_driver);
> +module_platform_driver(smi_driver);
>  
> -MODULE_DESCRIPTION("I2C multi instantiate pseudo device driver");
> +MODULE_DESCRIPTION("Serial multi instantiate pseudo device driver");
>  MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
>  MODULE_LICENSE("GPL");
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 6/9] platform/x86: serial-multi-instantiate: Reorganize I2C functions
  2022-01-21 17:24 ` [PATCH v6 6/9] platform/x86: serial-multi-instantiate: Reorganize I2C functions Stefan Binding
@ 2022-02-01 14:59   ` Hans de Goede
  0 siblings, 0 replies; 29+ messages in thread
From: Hans de Goede @ 2022-02-01 14:59 UTC (permalink / raw)
  To: Stefan Binding, Mark Brown, Rafael J . Wysocki, Len Brown,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches, Lucas Tanure

Hi,

On 1/21/22 18:24, Stefan Binding wrote:
> From: Lucas Tanure <tanureal@opensource.cirrus.com>
> 
> Reorganize I2C functions to accommodate SPI support
> Split the probe and factor out parts of the code
> that will be used in the SPI support
> Also switched from strlcpy() to strscpy()
> 
> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  .../platform/x86/serial-multi-instantiate.c   | 144 +++++++++++-------
>  1 file changed, 90 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
> index 33cbb0caed33..4cd6d72a0741 100644
> --- a/drivers/platform/x86/serial-multi-instantiate.c
> +++ b/drivers/platform/x86/serial-multi-instantiate.c
> @@ -29,96 +29,132 @@ struct smi_instance {
>  
>  struct smi {
>  	int i2c_num;
> -	struct i2c_client *i2c_devs[];
> +	struct i2c_client **i2c_devs;
>  };
>  
> -static int smi_probe(struct platform_device *pdev)
> +static int smi_get_irq(struct platform_device *pdev, struct acpi_device *adev,
> +		       const struct smi_instance *inst)
> +{
> +	int ret;
> +
> +	switch (inst->flags & IRQ_RESOURCE_TYPE) {
> +	case IRQ_RESOURCE_GPIO:
> +		ret = acpi_dev_gpio_irq_get(adev, inst->irq_idx);
> +		break;
> +	case IRQ_RESOURCE_APIC:
> +		ret = platform_get_irq(pdev, inst->irq_idx);
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	if (ret < 0)
> +		dev_err_probe(&pdev->dev, ret, "Error requesting irq at index %d: %d\n",
> +			      inst->irq_idx, ret);
> +
> +	return ret;
> +}
> +
> +static void smi_devs_unregister(struct smi *smi)
> +{
> +	while (smi->i2c_num > 0)
> +		i2c_unregister_device(smi->i2c_devs[--smi->i2c_num]);
> +}
> +
> +/**
> + * smi_i2c_probe - Instantiate multiple I2C devices from inst array
> + * @pdev:	Platform device
> + * @adev:	ACPI device
> + * @smi:	Internal struct for Serial multi instantiate driver
> + * @inst_array:	Array of instances to probe
> + *
> + * Returns the number of I2C devices instantiate, Zero if none is found or a negative error code.
> + */
> +static int smi_i2c_probe(struct platform_device *pdev, struct acpi_device *adev, struct smi *smi,
> +			 const struct smi_instance *inst_array)
>  {
>  	struct i2c_board_info board_info = {};
> -	const struct smi_instance *inst;
>  	struct device *dev = &pdev->dev;
> -	struct acpi_device *adev;
> -	struct smi *smi;
>  	char name[32];
> -	int i, ret;
> +	int i, ret, count;
>  
> -	inst = device_get_match_data(dev);
> -	if (!inst) {
> -		dev_err(dev, "Error ACPI match data is missing\n");
> -		return -ENODEV;
> -	}
> -
> -	adev = ACPI_COMPANION(dev);
> -
> -	/* Count number of clients to instantiate */
>  	ret = i2c_acpi_client_count(adev);
>  	if (ret < 0)
>  		return ret;
> +	else if (!ret)
> +		return -ENODEV;
>  
> -	smi = devm_kmalloc(dev, struct_size(smi, i2c_devs, ret), GFP_KERNEL);
> -	if (!smi)
> -		return -ENOMEM;
> +	count = ret;
>  
> -	smi->i2c_num = ret;
> +	smi->i2c_devs = devm_kcalloc(dev, count, sizeof(*smi->i2c_devs), GFP_KERNEL);
> +	if (!smi->i2c_devs)
> +		return -ENOMEM;
>  
> -	for (i = 0; i < smi->i2c_num && inst[i].type; i++) {
> +	for (i = 0; i < count && inst_array[i].type; i++) {
>  		memset(&board_info, 0, sizeof(board_info));
> -		strlcpy(board_info.type, inst[i].type, I2C_NAME_SIZE);
> -		snprintf(name, sizeof(name), "%s-%s.%d", dev_name(dev), inst[i].type, i);
> +		strscpy(board_info.type, inst_array[i].type, I2C_NAME_SIZE);
> +		snprintf(name, sizeof(name), "%s-%s.%d", dev_name(dev), inst_array[i].type, i);
>  		board_info.dev_name = name;
> -		switch (inst[i].flags & IRQ_RESOURCE_TYPE) {
> -		case IRQ_RESOURCE_GPIO:
> -			ret = acpi_dev_gpio_irq_get(adev, inst[i].irq_idx);
> -			if (ret < 0) {
> -				dev_err(dev, "Error requesting irq at index %d: %d\n",
> -						inst[i].irq_idx, ret);
> -				goto error;
> -			}
> -			board_info.irq = ret;
> -			break;
> -		case IRQ_RESOURCE_APIC:
> -			ret = platform_get_irq(pdev, inst[i].irq_idx);
> -			if (ret < 0) {
> -				dev_dbg(dev, "Error requesting irq at index %d: %d\n",
> -					inst[i].irq_idx, ret);
> -				goto error;
> -			}
> -			board_info.irq = ret;
> -			break;
> -		default:
> -			board_info.irq = 0;
> -			break;
> -		}
> +
> +		ret = smi_get_irq(pdev, adev, &inst_array[i]);
> +		if (ret < 0)
> +			goto error;
> +		board_info.irq = ret;
> +
>  		smi->i2c_devs[i] = i2c_acpi_new_device(dev, i, &board_info);
>  		if (IS_ERR(smi->i2c_devs[i])) {
>  			ret = dev_err_probe(dev, PTR_ERR(smi->i2c_devs[i]),
>  					    "Error creating i2c-client, idx %d\n", i);
>  			goto error;
>  		}
> +		smi->i2c_num++;
>  	}
> -	if (i < smi->i2c_num) {
> -		dev_err(dev, "Error finding driver, idx %d\n", i);
> +	if (smi->i2c_num < count) {
> +		dev_dbg(dev, "Error finding driver, idx %d\n", i);
>  		ret = -ENODEV;
>  		goto error;
>  	}
>  
> -	platform_set_drvdata(pdev, smi);
> -	return 0;
> +	dev_info(dev, "Instantiated %d I2C devices.\n", smi->i2c_num);
>  
> +	return 0;
>  error:
> -	while (--i >= 0)
> -		i2c_unregister_device(smi->i2c_devs[i]);
> +	smi_devs_unregister(smi);
>  
>  	return ret;
>  }
>  
> +static int smi_probe(struct platform_device *pdev)
> +{
> +	const struct smi_instance *inst_array;
> +	struct device *dev = &pdev->dev;
> +	struct acpi_device *adev;
> +	struct smi *smi;
> +
> +	adev = ACPI_COMPANION(dev);
> +	if (!adev)
> +		return -ENODEV;
> +
> +	inst_array = device_get_match_data(dev);
> +	if (!inst_array) {
> +		dev_dbg(dev, "Error ACPI match data is missing\n");
> +		return -ENODEV;
> +	}
> +
> +	smi = devm_kzalloc(dev, sizeof(*smi), GFP_KERNEL);
> +	if (!smi)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, smi);
> +
> +	return smi_i2c_probe(pdev, adev, smi, inst_array);
> +}
> +
>  static int smi_remove(struct platform_device *pdev)
>  {
>  	struct smi *smi = platform_get_drvdata(pdev);
> -	int i;
>  
> -	for (i = 0; i < smi->i2c_num; i++)
> -		i2c_unregister_device(smi->i2c_devs[i]);
> +	smi_devs_unregister(smi);
>  
>  	return 0;
>  }
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 7/9] platform/x86: serial-multi-instantiate: Add SPI support
  2022-01-21 17:24 ` [PATCH v6 7/9] platform/x86: serial-multi-instantiate: Add SPI support Stefan Binding
@ 2022-02-01 15:02   ` Hans de Goede
  2022-02-02  8:55     ` Hans de Goede
  0 siblings, 1 reply; 29+ messages in thread
From: Hans de Goede @ 2022-02-01 15:02 UTC (permalink / raw)
  To: Stefan Binding, Mark Brown, Rafael J . Wysocki, Len Brown,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches

Hi,

On 1/21/22 18:24, Stefan Binding wrote:
> Add support for spi bus in serial-multi-instantiate driver
> 
> Some peripherals can have either a I2C or a SPI connection
> to the host (but not both) but use the same HID for both
> types. So it is not possible to use the HID to determine
> whether it is I2C or SPI. The driver must check the node
> to see if it contains I2cSerialBus or SpiSerialBus entries.
> 
> For backwards-compatibility with the existing nodes I2C is
> checked first and if such entries are found ONLY I2C devices
> are created. Since some existing nodes that were already
> handled by this driver could also contain unrelated
> SpiSerialBus nodes that were previously ignored, and this
> preserves that behavior. If there is ever a need to handle
> a node where both I2C and SPI devices must be instantiated
> this can be added in future.
> 
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
> ---
>  drivers/platform/x86/Kconfig                  |   2 +-
>  .../platform/x86/serial-multi-instantiate.c   | 174 +++++++++++++++---
>  2 files changed, 151 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 2e656909a866..8d1eec208854 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -992,7 +992,7 @@ config TOPSTAR_LAPTOP
>  
>  config SERIAL_MULTI_INSTANTIATE
>  	tristate "Serial bus multi instantiate pseudo device driver"
> -	depends on I2C && ACPI
> +	depends on I2C && SPI && ACPI
>  	help
>  	  Some ACPI-based systems list multiple devices in a single ACPI
>  	  firmware-node. This driver will instantiate separate clients
> diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
> index 4cd6d72a0741..3f05385ca2cf 100644
> --- a/drivers/platform/x86/serial-multi-instantiate.c
> +++ b/drivers/platform/x86/serial-multi-instantiate.c
> @@ -14,6 +14,7 @@
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/property.h>
> +#include <linux/spi/spi.h>
>  #include <linux/types.h>
>  
>  #define IRQ_RESOURCE_TYPE	GENMASK(1, 0)
> @@ -21,15 +22,28 @@
>  #define IRQ_RESOURCE_GPIO	1
>  #define IRQ_RESOURCE_APIC	2
>  
> +enum smi_bus_type {
> +	SMI_I2C,
> +	SMI_SPI,
> +	SMI_AUTO_DETECT,
> +};
> +
>  struct smi_instance {
>  	const char *type;
>  	unsigned int flags;
>  	int irq_idx;
>  };
>  
> +struct smi_node {
> +	enum smi_bus_type bus_type;
> +	struct smi_instance instances[];
> +};
> +
>  struct smi {
>  	int i2c_num;
> +	int spi_num;
>  	struct i2c_client **i2c_devs;
> +	struct spi_device **spi_devs;
>  };
>  
>  static int smi_get_irq(struct platform_device *pdev, struct acpi_device *adev,
> @@ -59,6 +73,95 @@ static void smi_devs_unregister(struct smi *smi)
>  {
>  	while (smi->i2c_num > 0)
>  		i2c_unregister_device(smi->i2c_devs[--smi->i2c_num]);
> +
> +	while (smi->spi_num > 0)
> +		spi_unregister_device(smi->spi_devs[--smi->spi_num]);
> +}
> +
> +/**
> + * smi_spi_probe - Instantiate multiple SPI devices from inst array
> + * @pdev:	Platform device
> + * @adev:	ACPI device
> + * @smi:	Internal struct for Serial multi instantiate driver
> + * @inst_array:	Array of instances to probe
> + *
> + * Returns the number of SPI devices instantiate, Zero if none is found or a negative error code.
> + */
> +static int smi_spi_probe(struct platform_device *pdev, struct acpi_device *adev, struct smi *smi,
> +			 const struct smi_instance *inst_array)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct spi_controller *ctlr;
> +	struct spi_device *spi_dev;
> +	char name[50];
> +	int i, ret, count;
> +
> +	ret = acpi_spi_count_resources(adev);
> +	if (ret < 0)
> +		return ret;
> +	else if (!ret)
> +		return -ENODEV;
> +
> +	count = ret;
> +
> +	smi->spi_devs = devm_kcalloc(dev, count, sizeof(*smi->spi_devs), GFP_KERNEL);
> +	if (!smi->spi_devs)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < count && inst_array[i].type; i++) {
> +
> +		spi_dev = acpi_spi_device_alloc(NULL, adev, i);
> +		if (IS_ERR(spi_dev)) {
> +			ret = PTR_ERR(spi_dev);
> +			dev_err_probe(dev, ret, "failed to allocate SPI device %s from ACPI: %d\n",
> +				      dev_name(&adev->dev), ret);
> +			goto error;
> +		}
> +
> +		ctlr = spi_dev->controller;
> +
> +		strscpy(spi_dev->modalias, inst_array[i].type, sizeof(spi_dev->modalias));
> +
> +		ret = smi_get_irq(pdev, adev, &inst_array[i]);
> +		if (ret < 0) {
> +			spi_dev_put(spi_dev);
> +			goto error;
> +		}
> +		spi_dev->irq = ret;
> +
> +		snprintf(name, sizeof(name), "%s-%s-%s.%d", dev_name(&ctlr->dev), dev_name(dev),
> +			 inst_array[i].type, i);
> +		spi_dev->dev.init_name = name;
> +
> +		ret = spi_add_device(spi_dev);
> +		if (ret) {
> +			dev_err_probe(&ctlr->dev, ret,
> +				      "failed to add SPI device %s from ACPI: %d\n",
> +				      dev_name(&adev->dev), ret);
> +			spi_dev_put(spi_dev);
> +			goto error;
> +		}
> +
> +		dev_dbg(dev, "SPI device %s using chip select %u", name, spi_dev->chip_select);
> +
> +		smi->spi_devs[i] = spi_dev;
> +		smi->spi_num++;
> +	}
> +
> +	if (smi->spi_num < count) {
> +		dev_dbg(dev, "Error finding driver, idx %d\n", i);
> +		ret = -ENODEV;
> +		goto error;
> +	}
> +
> +	dev_info(dev, "Instantiated %d SPI devices.\n", smi->spi_num);
> +
> +	return 0;
> +error:
> +	smi_devs_unregister(smi);
> +
> +	return ret;
> +
>  }
>  
>  /**
> @@ -126,8 +229,8 @@ static int smi_i2c_probe(struct platform_device *pdev, struct acpi_device *adev,
>  
>  static int smi_probe(struct platform_device *pdev)
>  {
> -	const struct smi_instance *inst_array;
>  	struct device *dev = &pdev->dev;
> +	const struct smi_node *node;
>  	struct acpi_device *adev;
>  	struct smi *smi;
>  
> @@ -135,8 +238,8 @@ static int smi_probe(struct platform_device *pdev)
>  	if (!adev)
>  		return -ENODEV;
>  
> -	inst_array = device_get_match_data(dev);
> -	if (!inst_array) {
> +	node = device_get_match_data(dev);
> +	if (!node) {
>  		dev_dbg(dev, "Error ACPI match data is missing\n");
>  		return -ENODEV;
>  	}
> @@ -147,7 +250,21 @@ static int smi_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, smi);
>  
> -	return smi_i2c_probe(pdev, adev, smi, inst_array);
> +	switch (node->bus_type) {
> +	case SMI_I2C:
> +		return smi_i2c_probe(pdev, adev, smi, node->instances);
> +	case SMI_SPI:
> +		return smi_spi_probe(pdev, adev, smi, node->instances);
> +	case SMI_AUTO_DETECT:
> +		if (i2c_acpi_client_count(adev) > 0)
> +			return smi_i2c_probe(pdev, adev, smi, node->instances);
> +		else
> +			return smi_spi_probe(pdev, adev, smi, node->instances);
> +	default:
> +		break;

Please replace this break with : "return -EINVAL" (since we really
should never hit this default case).

With that fixed, please add my R-b to the next version:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> +	}
> +
> +	return 0; /* never reached */
>  }
>  
>  static int smi_remove(struct platform_device *pdev)
> @@ -159,27 +276,36 @@ static int smi_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static const struct smi_instance bsg1160_data[]  = {
> -	{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
> -	{ "bmc150_magn" },
> -	{ "bmg160" },
> -	{}
> +static const struct smi_node bsg1160_data = {
> +	.instances = {
> +		{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
> +		{ "bmc150_magn" },
> +		{ "bmg160" },
> +		{}
> +	},
> +	.bus_type = SMI_I2C,
>  };
>  
> -static const struct smi_instance bsg2150_data[]  = {
> -	{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
> -	{ "bmc150_magn" },
> -	/* The resources describe a 3th client, but it is not really there. */
> -	{ "bsg2150_dummy_dev" },
> -	{}
> +static const struct smi_node bsg2150_data = {
> +	.instances = {
> +		{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
> +		{ "bmc150_magn" },
> +		/* The resources describe a 3th client, but it is not really there. */
> +		{ "bsg2150_dummy_dev" },
> +		{}
> +	},
> +	.bus_type = SMI_I2C,
>  };
>  
> -static const struct smi_instance int3515_data[]  = {
> -	{ "tps6598x", IRQ_RESOURCE_APIC, 0 },
> -	{ "tps6598x", IRQ_RESOURCE_APIC, 1 },
> -	{ "tps6598x", IRQ_RESOURCE_APIC, 2 },
> -	{ "tps6598x", IRQ_RESOURCE_APIC, 3 },
> -	{}
> +static const struct smi_node int3515_data = {
> +	.instances = {
> +		{ "tps6598x", IRQ_RESOURCE_APIC, 0 },
> +		{ "tps6598x", IRQ_RESOURCE_APIC, 1 },
> +		{ "tps6598x", IRQ_RESOURCE_APIC, 2 },
> +		{ "tps6598x", IRQ_RESOURCE_APIC, 3 },
> +		{}
> +	},
> +	.bus_type = SMI_I2C,
>  };
>  
>  /*
> @@ -187,9 +313,9 @@ static const struct smi_instance int3515_data[]  = {
>   * drivers/acpi/scan.c: acpi_device_enumeration_by_parent().
>   */
>  static const struct acpi_device_id smi_acpi_ids[] = {
> -	{ "BSG1160", (unsigned long)bsg1160_data },
> -	{ "BSG2150", (unsigned long)bsg2150_data },
> -	{ "INT3515", (unsigned long)int3515_data },
> +	{ "BSG1160", (unsigned long)&bsg1160_data },
> +	{ "BSG2150", (unsigned long)&bsg2150_data },
> +	{ "INT3515", (unsigned long)&int3515_data },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(acpi, smi_acpi_ids);
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 9/9] ACPI / scan: Create platform device for CS35L41
  2022-01-21 17:24 ` [PATCH v6 9/9] ACPI / scan: Create platform device for CS35L41 Stefan Binding
@ 2022-02-01 15:03   ` Hans de Goede
  0 siblings, 0 replies; 29+ messages in thread
From: Hans de Goede @ 2022-02-01 15:03 UTC (permalink / raw)
  To: Stefan Binding, Mark Brown, Rafael J . Wysocki, Len Brown,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches, Lucas Tanure

Hi,

On 1/21/22 18:24, Stefan Binding wrote:
> From: Lucas Tanure <tanureal@opensource.cirrus.com>
> 
> The ACPI device with CSC3551 or CLSA0100 are sound cards
> with multiple instances of CS35L41 connected by I2C or SPI
> to the main CPU.
> 
> We add an ID to the ignore_serial_bus_ids list to enumerate
> all I2C or SPI devices correctly.
> 
> The same IDs are also added into serial-multi-instantiate
> so that the driver can correctly enumerate the ACPI.
> 
> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  drivers/acpi/scan.c                             |  3 +++
>  drivers/platform/x86/serial-multi-instantiate.c | 14 ++++++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 48db5e80c2dc..ebd10af3ff7f 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1744,8 +1744,11 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)
>  	 */
>  		{"BSG1160", },
>  		{"BSG2150", },
> +		{"CSC3551", },
>  		{"INT33FE", },
>  		{"INT3515", },
> +	/* Non-conforming _HID for Cirrus Logic already released */
> +		{"CLSA0100", },
>  	/*
>  	 * HIDs of device with an UartSerialBusV2 resource for which userspace
>  	 * expects a regular tty cdev to be created (instead of the in kernel
> diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
> index 3f05385ca2cf..d09f11eac4f8 100644
> --- a/drivers/platform/x86/serial-multi-instantiate.c
> +++ b/drivers/platform/x86/serial-multi-instantiate.c
> @@ -308,6 +308,17 @@ static const struct smi_node int3515_data = {
>  	.bus_type = SMI_I2C,
>  };
>  
> +static const struct smi_node cs35l41_hda = {
> +	.instances = {
> +		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
> +		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
> +		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
> +		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
> +		{}
> +	},
> +	.bus_type = SMI_AUTO_DETECT,
> +};
> +
>  /*
>   * Note new device-ids must also be added to serial_multi_instantiate_ids in
>   * drivers/acpi/scan.c: acpi_device_enumeration_by_parent().
> @@ -316,6 +327,9 @@ static const struct acpi_device_id smi_acpi_ids[] = {
>  	{ "BSG1160", (unsigned long)&bsg1160_data },
>  	{ "BSG2150", (unsigned long)&bsg2150_data },
>  	{ "INT3515", (unsigned long)&int3515_data },
> +	{ "CSC3551", (unsigned long)&cs35l41_hda },
> +	/* Non-conforming _HID for Cirrus Logic already released */
> +	{ "CLSA0100", (unsigned long)&cs35l41_hda },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(acpi, smi_acpi_ids);
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: (subset) [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver
  2022-01-21 17:24 [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Stefan Binding
                   ` (9 preceding siblings ...)
  2022-02-01 14:25 ` [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Hans de Goede
@ 2022-02-01 17:18 ` Mark Brown
  2022-02-01 17:41 ` Mark Brown
  11 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2022-02-01 17:18 UTC (permalink / raw)
  To: Rafael J . Wysocki, Len Brown, Mark Gross, Stefan Binding,
	Takashi Iwai, Jaroslav Kysela, Hans de Goede
  Cc: alsa-devel, patches, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86

On Fri, 21 Jan 2022 17:24:22 +0000, Stefan Binding wrote:
> Add support for SPI bus in the i2c-multi-instantiate driver as
> upcoming laptops will need to multi instantiate SPI devices from
> a single device node, which has multiple SpiSerialBus entries at
> the ACPI table.
> 
> With the new SPI support, i2c-multi-instantiate becomes
> bus-multi-instantiate and is moved to the ACPI folder.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/9] spi: Make spi_alloc_device and spi_add_device public again
      commit: 941bffd7d7f5d6030a54184c5d81b0eb9116ca9a
[2/9] spi: Create helper API to lookup ACPI info for spi device
      commit: 70dd264bc07aee4f89e65138db11e908701388dd
[3/9] spi: Support selection of the index of the ACPI Spi Resource before alloc
      commit: 92640f98a78c6a3ea1ca32143144241eceb129bd
[4/9] spi: Add API to count spi acpi resources
      commit: 113962301d2d9a5c11381d9c25ddea7af71be2ff

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 29+ messages in thread

* RE: [PATCH v6 2/9] spi: Create helper API to lookup ACPI info for spi device
  2022-02-01 14:28   ` Hans de Goede
@ 2022-02-01 17:28     ` Stefan Binding
  2022-02-01 17:37       ` Mark Brown
  0 siblings, 1 reply; 29+ messages in thread
From: Stefan Binding @ 2022-02-01 17:28 UTC (permalink / raw)
  To: 'Hans de Goede', 'Mark Brown',
	'Rafael J . Wysocki', 'Len Brown',
	'Mark Gross', 'Jaroslav Kysela',
	'Takashi Iwai'
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches

Hi,

> -----Original Message-----
> From: Hans de Goede <hdegoede@redhat.com>
> Sent: 01 February 2022 14:28
> To: Stefan Binding <sbinding@opensource.cirrus.com>; Mark Brown
> <broonie@kernel.org>; Rafael J . Wysocki <rafael@kernel.org>; Len Brown
> <lenb@kernel.org>; Mark Gross <markgross@kernel.org>; Jaroslav Kysela
> <perex@perex.cz>; Takashi Iwai <tiwai@suse.com>
> Cc: alsa-devel@alsa-project.org; linux-kernel@vger.kernel.org; linux-
> spi@vger.kernel.org; linux-acpi@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; patches@opensource.cirrus.com
> Subject: Re: [PATCH v6 2/9] spi: Create helper API to lookup ACPI info for spi
> device
> 
> Hi,
> 
> On 1/21/22 18:24, Stefan Binding wrote:
> > This can then be used to find a spi resource inside an
> > ACPI node, and allocate a spi device.
> >
> > Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
> > ---
> >  drivers/spi/spi.c       | 46 ++++++++++++++++++++++++++++++++---------
> >  include/linux/spi/spi.h |  6 ++++++
> >  2 files changed, 42 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> > index 1eb84101c4ad..13f4701f0694 100644
> > --- a/drivers/spi/spi.c
> > +++ b/drivers/spi/spi.c
> > @@ -2410,8 +2410,18 @@ static int acpi_spi_add_resource(struct
> acpi_resource *ares, void *data)
> >  	return 1;
> >  }
> >
> > -static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
> > -					    struct acpi_device *adev)
> > +/**
> > + * acpi_spi_device_alloc - Allocate a spi device, and fill it in with ACPI
> information
> > + * @ctlr: controller to which the spi device belongs
> > + * @adev: ACPI Device for the spi device
> > + *
> > + * This should be used to allocate a new spi device from and ACPI Node.
> > + * The caller is responsible for calling spi_add_device to register the spi
> device.
> > + *
> > + * Return: a pointer to the new device, or ERR_PTR on error.
> > + */
> > +struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
> > +					 struct acpi_device *adev)
> >  {
> >  	acpi_handle parent_handle = NULL;
> >  	struct list_head resource_list;
> > @@ -2419,10 +2429,6 @@ static acpi_status
> acpi_register_spi_device(struct spi_controller *ctlr,
> >  	struct spi_device *spi;
> >  	int ret;
> >
> > -	if (acpi_bus_get_status(adev) || !adev->status.present ||
> > -	    acpi_device_enumerated(adev))
> > -		return AE_OK;
> > -
> >  	lookup.ctlr		= ctlr;
> >  	lookup.irq		= -1;
> >
> > @@ -2433,7 +2439,7 @@ static acpi_status acpi_register_spi_device(struct
> spi_controller *ctlr,
> >
> >  	if (ret < 0)
> >  		/* found SPI in _CRS but it points to another controller */
> > -		return AE_OK;
> > +		return ERR_PTR(-ENODEV);
> >
> >  	if (!lookup.max_speed_hz &&
> >  	    ACPI_SUCCESS(acpi_get_parent(adev->handle, &parent_handle))
> &&
> > @@ -2443,16 +2449,15 @@ static acpi_status
> acpi_register_spi_device(struct spi_controller *ctlr,
> >  	}
> >
> >  	if (!lookup.max_speed_hz)
> > -		return AE_OK;
> > +		return ERR_PTR(-ENODEV);
> >
> >  	spi = spi_alloc_device(ctlr);
> >  	if (!spi) {
> >  		dev_err(&ctlr->dev, "failed to allocate SPI device for %s\n",
> >  			dev_name(&adev->dev));
> > -		return AE_NO_MEMORY;
> > +		return ERR_PTR(-ENOMEM);
> >  	}
> >
> > -
> >  	ACPI_COMPANION_SET(&spi->dev, adev);
> >  	spi->max_speed_hz	= lookup.max_speed_hz;
> >  	spi->mode		|= lookup.mode;
> > @@ -2460,6 +2465,27 @@ static acpi_status
> acpi_register_spi_device(struct spi_controller *ctlr,
> >  	spi->bits_per_word	= lookup.bits_per_word;
> >  	spi->chip_select	= lookup.chip_select;
> >
> > +	return spi;
> > +}
> > +EXPORT_SYMBOL_GPL(acpi_spi_device_alloc);
> > +
> > +static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
> > +					    struct acpi_device *adev)
> > +{
> > +	struct spi_device *spi;
> > +
> > +	if (acpi_bus_get_status(adev) || !adev->status.present ||
> > +	    acpi_device_enumerated(adev))
> > +		return AE_OK;
> > +
> > +	spi = acpi_spi_device_alloc(ctlr, adev);
> > +	if (IS_ERR(spi)) {
> > +		if (PTR_ERR(spi) == -ENOMEM)
> > +			return AE_NO_MEMORY;
> > +		else
> > +			return AE_OK;
> > +	}
> > +
> >  	acpi_set_modalias(adev, acpi_device_hid(adev), spi->modalias,
> >  			  sizeof(spi->modalias));
> >
> > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> > index 0346a3ff27fd..d159cef12f1a 100644
> > --- a/include/linux/spi/spi.h
> > +++ b/include/linux/spi/spi.h
> > @@ -16,6 +16,7 @@
> >  #include <linux/gpio/consumer.h>
> >
> >  #include <uapi/linux/spi/spi.h>
> > +#include <linux/acpi.h>
> >
> >  struct dma_chan;
> >  struct software_node;
> > @@ -759,6 +760,11 @@ extern int devm_spi_register_controller(struct
> device *dev,
> >  					struct spi_controller *ctlr);
> >  extern void spi_unregister_controller(struct spi_controller *ctlr);
> >
> > +#if IS_ENABLED(CONFIG_ACPI)
> > +extern struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
> > +						struct acpi_device *adev);
> > +#endif
> > +
> 
> There is no need to add a #ifdef about something which is just a
> function prototype. Having this declared when CONFIG_ACPI is not set is
> harmless, please drop the #ifdef.
> 
> With that fixed, please add my R-b to the next version:
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> Regards,
> 
> Hans

I was just fixing this, however, I just noticed that a subset of this chain - including
this patch - just got applied. Do you want me to fix this in a separate patch?

Thanks,
Stefan

> 
> 
> 
> >  /*
> >   * SPI resource management while processing a SPI message
> >   */
> >



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 2/9] spi: Create helper API to lookup ACPI info for spi device
  2022-02-01 17:28     ` Stefan Binding
@ 2022-02-01 17:37       ` Mark Brown
  0 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2022-02-01 17:37 UTC (permalink / raw)
  To: Stefan Binding
  Cc: 'Hans de Goede', 'Rafael J . Wysocki',
	'Len Brown', 'Mark Gross',
	'Jaroslav Kysela', 'Takashi Iwai',
	alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches

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

On Tue, Feb 01, 2022 at 05:28:34PM +0000, Stefan Binding wrote:

> I was just fixing this, however, I just noticed that a subset of this chain - including
> this patch - just got applied. Do you want me to fix this in a separate patch?

Quoting from the mail you got saying that the series was applied:

| If any updates are required or you are submitting further changes they
| should be sent as incremental updates against current git, existing
| patches will not be replaced.

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver
  2022-01-21 17:24 [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Stefan Binding
                   ` (10 preceding siblings ...)
  2022-02-01 17:18 ` (subset) " Mark Brown
@ 2022-02-01 17:41 ` Mark Brown
  11 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2022-02-01 17:41 UTC (permalink / raw)
  To: Stefan Binding
  Cc: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross,
	Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-kernel,
	linux-spi, linux-acpi, platform-driver-x86, patches

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

On Fri, Jan 21, 2022 at 05:24:22PM +0000, Stefan Binding wrote:
> Add support for SPI bus in the i2c-multi-instantiate driver as
> upcoming laptops will need to multi instantiate SPI devices from
> a single device node, which has multiple SpiSerialBus entries at
> the ACPI table.

The following changes since commit e783362eb54cd99b2cac8b3a9aeac942e6f6ac07:

  Linux 5.17-rc1 (2022-01-23 10:12:53 +0200)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git tags/spi-acpi-helpers

for you to fetch changes up to e612af7acef2459f1afd885f4107748995a05963:

  spi: Add API to count spi acpi resources (2022-02-01 17:38:48 +0000)

----------------------------------------------------------------
spi: ACPI helpers

This patch series enhances the ACPI helpers to cope with multiple
resources being available and exports them for use.

----------------------------------------------------------------
Stefan Binding (4):
      spi: Make spi_alloc_device and spi_add_device public again
      spi: Create helper API to lookup ACPI info for spi device
      spi: Support selection of the index of the ACPI Spi Resource before alloc
      spi: Add API to count spi acpi resources

 drivers/spi/spi.c       | 137 ++++++++++++++++++++++++++++++++++++++++++------
 include/linux/spi/spi.h |  20 +++++++
 2 files changed, 141 insertions(+), 16 deletions(-)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 7/9] platform/x86: serial-multi-instantiate: Add SPI support
  2022-02-01 15:02   ` Hans de Goede
@ 2022-02-02  8:55     ` Hans de Goede
  0 siblings, 0 replies; 29+ messages in thread
From: Hans de Goede @ 2022-02-02  8:55 UTC (permalink / raw)
  To: Stefan Binding, Mark Brown, Rafael J . Wysocki, Len Brown,
	Mark Gross, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-spi, linux-acpi,
	platform-driver-x86, patches

Hi,

On 2/1/22 16:02, Hans de Goede wrote:
> Hi,
> 
> On 1/21/22 18:24, Stefan Binding wrote:
>> Add support for spi bus in serial-multi-instantiate driver
>>
>> Some peripherals can have either a I2C or a SPI connection
>> to the host (but not both) but use the same HID for both
>> types. So it is not possible to use the HID to determine
>> whether it is I2C or SPI. The driver must check the node
>> to see if it contains I2cSerialBus or SpiSerialBus entries.
>>
>> For backwards-compatibility with the existing nodes I2C is
>> checked first and if such entries are found ONLY I2C devices
>> are created. Since some existing nodes that were already
>> handled by this driver could also contain unrelated
>> SpiSerialBus nodes that were previously ignored, and this
>> preserves that behavior. If there is ever a need to handle
>> a node where both I2C and SPI devices must be instantiated
>> this can be added in future.
>>
>> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
>> ---
>>  drivers/platform/x86/Kconfig                  |   2 +-
>>  .../platform/x86/serial-multi-instantiate.c   | 174 +++++++++++++++---
>>  2 files changed, 151 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index 2e656909a866..8d1eec208854 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -992,7 +992,7 @@ config TOPSTAR_LAPTOP
>>  
>>  config SERIAL_MULTI_INSTANTIATE
>>  	tristate "Serial bus multi instantiate pseudo device driver"
>> -	depends on I2C && ACPI
>> +	depends on I2C && SPI && ACPI
>>  	help
>>  	  Some ACPI-based systems list multiple devices in a single ACPI
>>  	  firmware-node. This driver will instantiate separate clients
>> diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
>> index 4cd6d72a0741..3f05385ca2cf 100644
>> --- a/drivers/platform/x86/serial-multi-instantiate.c
>> +++ b/drivers/platform/x86/serial-multi-instantiate.c
>> @@ -14,6 +14,7 @@
>>  #include <linux/module.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/property.h>
>> +#include <linux/spi/spi.h>
>>  #include <linux/types.h>
>>  
>>  #define IRQ_RESOURCE_TYPE	GENMASK(1, 0)
>> @@ -21,15 +22,28 @@
>>  #define IRQ_RESOURCE_GPIO	1
>>  #define IRQ_RESOURCE_APIC	2
>>  
>> +enum smi_bus_type {
>> +	SMI_I2C,
>> +	SMI_SPI,
>> +	SMI_AUTO_DETECT,
>> +};
>> +
>>  struct smi_instance {
>>  	const char *type;
>>  	unsigned int flags;
>>  	int irq_idx;
>>  };
>>  
>> +struct smi_node {
>> +	enum smi_bus_type bus_type;
>> +	struct smi_instance instances[];
>> +};
>> +
>>  struct smi {
>>  	int i2c_num;
>> +	int spi_num;
>>  	struct i2c_client **i2c_devs;
>> +	struct spi_device **spi_devs;
>>  };
>>  
>>  static int smi_get_irq(struct platform_device *pdev, struct acpi_device *adev,
>> @@ -59,6 +73,95 @@ static void smi_devs_unregister(struct smi *smi)
>>  {
>>  	while (smi->i2c_num > 0)
>>  		i2c_unregister_device(smi->i2c_devs[--smi->i2c_num]);
>> +
>> +	while (smi->spi_num > 0)
>> +		spi_unregister_device(smi->spi_devs[--smi->spi_num]);
>> +}
>> +
>> +/**
>> + * smi_spi_probe - Instantiate multiple SPI devices from inst array
>> + * @pdev:	Platform device
>> + * @adev:	ACPI device
>> + * @smi:	Internal struct for Serial multi instantiate driver
>> + * @inst_array:	Array of instances to probe
>> + *
>> + * Returns the number of SPI devices instantiate, Zero if none is found or a negative error code.
>> + */
>> +static int smi_spi_probe(struct platform_device *pdev, struct acpi_device *adev, struct smi *smi,
>> +			 const struct smi_instance *inst_array)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct spi_controller *ctlr;
>> +	struct spi_device *spi_dev;
>> +	char name[50];
>> +	int i, ret, count;
>> +
>> +	ret = acpi_spi_count_resources(adev);
>> +	if (ret < 0)
>> +		return ret;
>> +	else if (!ret)
>> +		return -ENODEV;
>> +
>> +	count = ret;
>> +
>> +	smi->spi_devs = devm_kcalloc(dev, count, sizeof(*smi->spi_devs), GFP_KERNEL);
>> +	if (!smi->spi_devs)
>> +		return -ENOMEM;
>> +
>> +	for (i = 0; i < count && inst_array[i].type; i++) {
>> +
>> +		spi_dev = acpi_spi_device_alloc(NULL, adev, i);
>> +		if (IS_ERR(spi_dev)) {
>> +			ret = PTR_ERR(spi_dev);
>> +			dev_err_probe(dev, ret, "failed to allocate SPI device %s from ACPI: %d\n",
>> +				      dev_name(&adev->dev), ret);
>> +			goto error;
>> +		}
>> +
>> +		ctlr = spi_dev->controller;
>> +
>> +		strscpy(spi_dev->modalias, inst_array[i].type, sizeof(spi_dev->modalias));
>> +
>> +		ret = smi_get_irq(pdev, adev, &inst_array[i]);
>> +		if (ret < 0) {
>> +			spi_dev_put(spi_dev);
>> +			goto error;
>> +		}
>> +		spi_dev->irq = ret;
>> +
>> +		snprintf(name, sizeof(name), "%s-%s-%s.%d", dev_name(&ctlr->dev), dev_name(dev),
>> +			 inst_array[i].type, i);
>> +		spi_dev->dev.init_name = name;
>> +
>> +		ret = spi_add_device(spi_dev);
>> +		if (ret) {
>> +			dev_err_probe(&ctlr->dev, ret,
>> +				      "failed to add SPI device %s from ACPI: %d\n",
>> +				      dev_name(&adev->dev), ret);
>> +			spi_dev_put(spi_dev);
>> +			goto error;
>> +		}
>> +
>> +		dev_dbg(dev, "SPI device %s using chip select %u", name, spi_dev->chip_select);
>> +
>> +		smi->spi_devs[i] = spi_dev;
>> +		smi->spi_num++;
>> +	}
>> +
>> +	if (smi->spi_num < count) {
>> +		dev_dbg(dev, "Error finding driver, idx %d\n", i);
>> +		ret = -ENODEV;
>> +		goto error;
>> +	}
>> +
>> +	dev_info(dev, "Instantiated %d SPI devices.\n", smi->spi_num);
>> +
>> +	return 0;
>> +error:
>> +	smi_devs_unregister(smi);
>> +
>> +	return ret;
>> +
>>  }
>>  
>>  /**
>> @@ -126,8 +229,8 @@ static int smi_i2c_probe(struct platform_device *pdev, struct acpi_device *adev,
>>  
>>  static int smi_probe(struct platform_device *pdev)
>>  {
>> -	const struct smi_instance *inst_array;
>>  	struct device *dev = &pdev->dev;
>> +	const struct smi_node *node;
>>  	struct acpi_device *adev;
>>  	struct smi *smi;
>>  
>> @@ -135,8 +238,8 @@ static int smi_probe(struct platform_device *pdev)
>>  	if (!adev)
>>  		return -ENODEV;
>>  
>> -	inst_array = device_get_match_data(dev);
>> -	if (!inst_array) {
>> +	node = device_get_match_data(dev);
>> +	if (!node) {
>>  		dev_dbg(dev, "Error ACPI match data is missing\n");
>>  		return -ENODEV;
>>  	}
>> @@ -147,7 +250,21 @@ static int smi_probe(struct platform_device *pdev)
>>  
>>  	platform_set_drvdata(pdev, smi);
>>  
>> -	return smi_i2c_probe(pdev, adev, smi, inst_array);
>> +	switch (node->bus_type) {
>> +	case SMI_I2C:
>> +		return smi_i2c_probe(pdev, adev, smi, node->instances);
>> +	case SMI_SPI:
>> +		return smi_spi_probe(pdev, adev, smi, node->instances);
>> +	case SMI_AUTO_DETECT:
>> +		if (i2c_acpi_client_count(adev) > 0)
>> +			return smi_i2c_probe(pdev, adev, smi, node->instances);
>> +		else
>> +			return smi_spi_probe(pdev, adev, smi, node->instances);
>> +	default:
>> +		break;
> 
> Please replace this break with : "return -EINVAL" (since we really
> should never hit this default case).
> 
> With that fixed, please add my R-b to the next version:

Note since Mark has merged 1-4 and provided a tag + pull-req for those
changes (thank you Mark). I plan to merge the rest of this series
into pdx86/for-next later today (except for 8/9).

I'll add the "return -EINVAL" myself when merging, so there is no
need to send out a new version.

Regards,

Hans




> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> Regards,
> 
> Hans
> 
> 
>> +	}
>> +
>> +	return 0; /* never reached */
>>  }
>>  
>>  static int smi_remove(struct platform_device *pdev)
>> @@ -159,27 +276,36 @@ static int smi_remove(struct platform_device *pdev)
>>  	return 0;
>>  }
>>  
>> -static const struct smi_instance bsg1160_data[]  = {
>> -	{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
>> -	{ "bmc150_magn" },
>> -	{ "bmg160" },
>> -	{}
>> +static const struct smi_node bsg1160_data = {
>> +	.instances = {
>> +		{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
>> +		{ "bmc150_magn" },
>> +		{ "bmg160" },
>> +		{}
>> +	},
>> +	.bus_type = SMI_I2C,
>>  };
>>  
>> -static const struct smi_instance bsg2150_data[]  = {
>> -	{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
>> -	{ "bmc150_magn" },
>> -	/* The resources describe a 3th client, but it is not really there. */
>> -	{ "bsg2150_dummy_dev" },
>> -	{}
>> +static const struct smi_node bsg2150_data = {
>> +	.instances = {
>> +		{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
>> +		{ "bmc150_magn" },
>> +		/* The resources describe a 3th client, but it is not really there. */
>> +		{ "bsg2150_dummy_dev" },
>> +		{}
>> +	},
>> +	.bus_type = SMI_I2C,
>>  };
>>  
>> -static const struct smi_instance int3515_data[]  = {
>> -	{ "tps6598x", IRQ_RESOURCE_APIC, 0 },
>> -	{ "tps6598x", IRQ_RESOURCE_APIC, 1 },
>> -	{ "tps6598x", IRQ_RESOURCE_APIC, 2 },
>> -	{ "tps6598x", IRQ_RESOURCE_APIC, 3 },
>> -	{}
>> +static const struct smi_node int3515_data = {
>> +	.instances = {
>> +		{ "tps6598x", IRQ_RESOURCE_APIC, 0 },
>> +		{ "tps6598x", IRQ_RESOURCE_APIC, 1 },
>> +		{ "tps6598x", IRQ_RESOURCE_APIC, 2 },
>> +		{ "tps6598x", IRQ_RESOURCE_APIC, 3 },
>> +		{}
>> +	},
>> +	.bus_type = SMI_I2C,
>>  };
>>  
>>  /*
>> @@ -187,9 +313,9 @@ static const struct smi_instance int3515_data[]  = {
>>   * drivers/acpi/scan.c: acpi_device_enumeration_by_parent().
>>   */
>>  static const struct acpi_device_id smi_acpi_ids[] = {
>> -	{ "BSG1160", (unsigned long)bsg1160_data },
>> -	{ "BSG2150", (unsigned long)bsg2150_data },
>> -	{ "INT3515", (unsigned long)int3515_data },
>> +	{ "BSG1160", (unsigned long)&bsg1160_data },
>> +	{ "BSG2150", (unsigned long)&bsg2150_data },
>> +	{ "INT3515", (unsigned long)&int3515_data },
>>  	{ }
>>  };
>>  MODULE_DEVICE_TABLE(acpi, smi_acpi_ids);
>>


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 8/9] ALSA: hda/realtek: Add support for HP Laptops
  2022-01-21 17:24 ` [PATCH v6 8/9] ALSA: hda/realtek: Add support for HP Laptops Stefan Binding
@ 2022-02-02  9:05   ` Takashi Iwai
  2022-02-02 16:58     ` Hans de Goede
  0 siblings, 1 reply; 29+ messages in thread
From: Takashi Iwai @ 2022-02-02  9:05 UTC (permalink / raw)
  To: Stefan Binding
  Cc: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede,
	Mark Gross, Jaroslav Kysela, Takashi Iwai, alsa-devel,
	linux-kernel, linux-spi, linux-acpi, platform-driver-x86,
	patches, Lucas Tanure

On Fri, 21 Jan 2022 18:24:30 +0100,
Stefan Binding wrote:
> 
> From: Lucas Tanure <tanureal@opensource.cirrus.com>
> 
> Add support for two and four CS35L41 using the component
> binding method
> 
> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>

Reviewed-by: Takashi Iwai <tiwai@suse.de>

Hans, feel free to include this one into your tree.
Or if you prefer this going through my tree, let me know.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 8/9] ALSA: hda/realtek: Add support for HP Laptops
  2022-02-02  9:05   ` Takashi Iwai
@ 2022-02-02 16:58     ` Hans de Goede
  2022-02-02 17:19       ` Takashi Iwai
  0 siblings, 1 reply; 29+ messages in thread
From: Hans de Goede @ 2022-02-02 16:58 UTC (permalink / raw)
  To: Takashi Iwai, Stefan Binding
  Cc: Mark Brown, Rafael J . Wysocki, Len Brown, Mark Gross,
	Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-kernel,
	linux-spi, linux-acpi, platform-driver-x86, patches,
	Lucas Tanure

Hi,

On 2/2/22 10:05, Takashi Iwai wrote:
> On Fri, 21 Jan 2022 18:24:30 +0100,
> Stefan Binding wrote:
>>
>> From: Lucas Tanure <tanureal@opensource.cirrus.com>
>>
>> Add support for two and four CS35L41 using the component
>> binding method
>>
>> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
>> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
> 
> Reviewed-by: Takashi Iwai <tiwai@suse.de>
> 
> Hans, feel free to include this one into your tree.

Ok, I'm working on merging the entire series (on top of Mark's
spi-acpi-helpers tag) now. I'll send you a pull-req for
a signed tag when I'm done, in case you want to merge 
this to avoid conflicts.

Regards,

Hans


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 8/9] ALSA: hda/realtek: Add support for HP Laptops
  2022-02-02 16:58     ` Hans de Goede
@ 2022-02-02 17:19       ` Takashi Iwai
  0 siblings, 0 replies; 29+ messages in thread
From: Takashi Iwai @ 2022-02-02 17:19 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Stefan Binding, Mark Brown, Rafael J . Wysocki, Len Brown,
	Mark Gross, Jaroslav Kysela, Takashi Iwai, alsa-devel,
	linux-kernel, linux-spi, linux-acpi, platform-driver-x86,
	patches, Lucas Tanure

On Wed, 02 Feb 2022 17:58:20 +0100,
Hans de Goede wrote:
> 
> Hi,
> 
> On 2/2/22 10:05, Takashi Iwai wrote:
> > On Fri, 21 Jan 2022 18:24:30 +0100,
> > Stefan Binding wrote:
> >>
> >> From: Lucas Tanure <tanureal@opensource.cirrus.com>
> >>
> >> Add support for two and four CS35L41 using the component
> >> binding method
> >>
> >> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
> >> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
> > 
> > Reviewed-by: Takashi Iwai <tiwai@suse.de>
> > 
> > Hans, feel free to include this one into your tree.
> 
> Ok, I'm working on merging the entire series (on top of Mark's
> spi-acpi-helpers tag) now. I'll send you a pull-req for
> a signed tag when I'm done, in case you want to merge 
> this to avoid conflicts.

Great, thanks.


Takashi

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2022-02-02 17:19 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-21 17:24 [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Stefan Binding
2022-01-21 17:24 ` [PATCH v6 1/9] spi: Make spi_alloc_device and spi_add_device public again Stefan Binding
2022-02-01 14:26   ` Hans de Goede
2022-02-01 14:52     ` Rafael J. Wysocki
2022-02-01 14:54       ` Hans de Goede
2022-01-21 17:24 ` [PATCH v6 2/9] spi: Create helper API to lookup ACPI info for spi device Stefan Binding
2022-02-01 14:28   ` Hans de Goede
2022-02-01 17:28     ` Stefan Binding
2022-02-01 17:37       ` Mark Brown
2022-01-21 17:24 ` [PATCH v6 3/9] spi: Support selection of the index of the ACPI Spi Resource before alloc Stefan Binding
2022-02-01 14:29   ` Hans de Goede
2022-01-21 17:24 ` [PATCH v6 4/9] spi: Add API to count spi acpi resources Stefan Binding
2022-02-01 14:42   ` Hans de Goede
2022-01-21 17:24 ` [PATCH v6 5/9] platform/x86: i2c-multi-instantiate: Rename it for a generic serial driver name Stefan Binding
2022-02-01 14:55   ` Hans de Goede
2022-01-21 17:24 ` [PATCH v6 6/9] platform/x86: serial-multi-instantiate: Reorganize I2C functions Stefan Binding
2022-02-01 14:59   ` Hans de Goede
2022-01-21 17:24 ` [PATCH v6 7/9] platform/x86: serial-multi-instantiate: Add SPI support Stefan Binding
2022-02-01 15:02   ` Hans de Goede
2022-02-02  8:55     ` Hans de Goede
2022-01-21 17:24 ` [PATCH v6 8/9] ALSA: hda/realtek: Add support for HP Laptops Stefan Binding
2022-02-02  9:05   ` Takashi Iwai
2022-02-02 16:58     ` Hans de Goede
2022-02-02 17:19       ` Takashi Iwai
2022-01-21 17:24 ` [PATCH v6 9/9] ACPI / scan: Create platform device for CS35L41 Stefan Binding
2022-02-01 15:03   ` Hans de Goede
2022-02-01 14:25 ` [PATCH v6 0/9] Support Spi in i2c-multi-instantiate driver Hans de Goede
2022-02-01 17:18 ` (subset) " Mark Brown
2022-02-01 17:41 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).