* [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()"
@ 2021-12-02 16:24 Stefan Binding
2021-12-02 16:24 ` [PATCH 2/3] spi: Make spi_alloc_device and spi_add_device public again Stefan Binding
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Stefan Binding @ 2021-12-02 16:24 UTC (permalink / raw)
To: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross
Cc: linux-kernel, linux-spi, linux-acpi, platform-driver-x86,
patches, Lucas Tanure, Stefan Binding
From: Lucas Tanure <tanureal@opensource.cirrus.com>
Revert commit bdc7ca008e1f ("spi: Remove unused function
spi_busnum_to_master()")
This function is needed for the spi version of i2c multi
instantiate driver.
Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
Documentation/spi/spi-summary.rst | 8 +++++++
drivers/spi/spi.c | 35 +++++++++++++++++++++++++++++++
include/linux/spi/spi.h | 2 ++
3 files changed, 45 insertions(+)
diff --git a/Documentation/spi/spi-summary.rst b/Documentation/spi/spi-summary.rst
index aab5d07cb3d7..d4239025461d 100644
--- a/Documentation/spi/spi-summary.rst
+++ b/Documentation/spi/spi-summary.rst
@@ -336,6 +336,14 @@ certainly includes SPI devices hooked up through the card connectors!
Non-static Configurations
^^^^^^^^^^^^^^^^^^^^^^^^^
+Developer boards often play by different rules than product boards, and one
+example is the potential need to hotplug SPI devices and/or controllers.
+
+For those cases you might need to use spi_busnum_to_master() to look
+up the spi bus master, and will likely need spi_new_device() to provide the
+board info based on the board that was hotplugged. Of course, you'd later
+call at least spi_unregister_device() when that board is removed.
+
When Linux includes support for MMC/SD/SDIO/DataFlash cards through SPI, those
configurations will also be dynamic. Fortunately, such devices all support
basic device identification probes, so they should hotplug normally.
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 8726309b3eaf..7c81173edb0c 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3145,6 +3145,41 @@ int spi_controller_resume(struct spi_controller *ctlr)
}
EXPORT_SYMBOL_GPL(spi_controller_resume);
+static int __spi_controller_match(struct device *dev, const void *data)
+{
+ struct spi_controller *ctlr;
+ const u16 *bus_num = data;
+
+ ctlr = container_of(dev, struct spi_controller, dev);
+ return ctlr->bus_num == *bus_num;
+}
+
+/**
+ * spi_busnum_to_master - look up master associated with bus_num
+ * @bus_num: the master's bus number
+ * Context: can sleep
+ *
+ * This call may be used with devices that are registered after
+ * arch init time. It returns a refcounted pointer to the relevant
+ * spi_controller (which the caller must release), or NULL if there is
+ * no such master registered.
+ *
+ * Return: the SPI master structure on success, else NULL.
+ */
+struct spi_controller *spi_busnum_to_master(u16 bus_num)
+{
+ struct device *dev;
+ struct spi_controller *ctlr = NULL;
+
+ dev = class_find_device(&spi_master_class, NULL, &bus_num,
+ __spi_controller_match);
+ if (dev)
+ ctlr = container_of(dev, struct spi_controller, dev);
+ /* reference got in class_find_device */
+ return ctlr;
+}
+EXPORT_SYMBOL_GPL(spi_busnum_to_master);
+
/*-------------------------------------------------------------------------*/
/* Core methods for spi_message alterations */
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index eb7ac8a1e03c..5f2781cb750f 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -759,6 +759,8 @@ extern int devm_spi_register_controller(struct device *dev,
struct spi_controller *ctlr);
extern void spi_unregister_controller(struct spi_controller *ctlr);
+extern struct spi_controller *spi_busnum_to_master(u16 busnum);
+
/*
* SPI resource management while processing a SPI message
*/
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/3] spi: Make spi_alloc_device and spi_add_device public again 2021-12-02 16:24 [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()" Stefan Binding @ 2021-12-02 16:24 ` Stefan Binding 2021-12-02 16:24 ` [PATCH 3/3] platform/x86: Support Spi in i2c-multi-instantiate driver Stefan Binding ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Stefan Binding @ 2021-12-02 16:24 UTC (permalink / raw) To: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross Cc: 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 7c81173edb0c..ebce296662c4 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -531,7 +531,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; @@ -556,6 +556,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) { @@ -651,7 +652,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; @@ -672,6 +673,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 5f2781cb750f..eb348cee1c57 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -1454,7 +1454,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] 10+ messages in thread
* [PATCH 3/3] platform/x86: Support Spi in i2c-multi-instantiate driver 2021-12-02 16:24 [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()" Stefan Binding 2021-12-02 16:24 ` [PATCH 2/3] spi: Make spi_alloc_device and spi_add_device public again Stefan Binding @ 2021-12-02 16:24 ` Stefan Binding 2021-12-03 11:22 ` Hans de Goede 2021-12-03 11:35 ` Andy Shevchenko 2021-12-02 16:53 ` [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()" Mark Brown 2021-12-03 11:06 ` Andy Shevchenko 3 siblings, 2 replies; 10+ messages in thread From: Stefan Binding @ 2021-12-02 16:24 UTC (permalink / raw) To: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross Cc: linux-kernel, linux-spi, linux-acpi, platform-driver-x86, patches, Stefan Binding Add support for spi bus in i2c-multi-instantiate driver and rename it for a multiple purpose driver name By adding spi support into this driver enables devices to use the same _HID string for i2c and spi uses and minimize the support for two drivers doing the same thing for different busses Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com> --- MAINTAINERS | 4 +- drivers/acpi/scan.c | 19 +- drivers/platform/x86/Kconfig | 14 +- drivers/platform/x86/Makefile | 2 +- drivers/platform/x86/bus-multi-instantiate.c | 432 +++++++++++++++++++ drivers/platform/x86/i2c-multi-instantiate.c | 174 -------- 6 files changed, 452 insertions(+), 193 deletions(-) create mode 100644 drivers/platform/x86/bus-multi-instantiate.c delete mode 100644 drivers/platform/x86/i2c-multi-instantiate.c diff --git a/MAINTAINERS b/MAINTAINERS index 5b1e79f8e3d8..f75600d917bc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -387,11 +387,11 @@ L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) S: Maintained F: drivers/acpi/arm64 -ACPI I2C MULTI INSTANTIATE DRIVER +ACPI BUS 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/bus-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 5991dddbc9ce..2f7da1a08112 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1696,14 +1696,15 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) struct list_head resource_list; bool is_serial_bus_slave = false; /* - * 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 I2cSerialBus/SpiSerialBusV2 resources + * and an (i2c/spi)-client must be instantiated for each, each with + * its own i2c_device_id/spi_device_id. + * Normally we only instantiate an (i2c/spi)-client for the first + * resource, using the ACPI HID as id. These special cases are handled + * by the drivers/platform/x86/bus-multi-instantiate.c driver, which + * knows which i2c_device_id or spi_device_id to use for each resource. */ - static const struct acpi_device_id i2c_multi_instantiate_ids[] = { + static const struct acpi_device_id bus_multi_instantiate_ids[] = { {"BSG1160", }, {"BSG2150", }, {"INT33FE", }, @@ -1721,8 +1722,8 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) fwnode_property_present(&device->fwnode, "baud"))) return true; - /* Instantiate a pdev for the i2c-multi-instantiate drv to bind to */ - if (!acpi_match_device_ids(device, i2c_multi_instantiate_ids)) + /* Instantiate a pdev for the bus-multi-instantiate drv to bind to */ + if (!acpi_match_device_ids(device, bus_multi_instantiate_ids)) return false; INIT_LIST_HEAD(&resource_list); diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 97e87628eb35..5a413b123c01 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -958,16 +958,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" - depends on I2C && ACPI +config BUS_MULTI_INSTANTIATE + tristate "Bus multi instantiate pseudo device driver" + depends on I2C && SPI && ACPI help - Some ACPI-based systems list multiple i2c-devices in a single ACPI - firmware-node. This driver will instantiate separate i2c-clients - for each device in the firmware-node. + Some ACPI-based systems list multiple i2c/spi devices in a + single ACPI firmware-node. This driver will instantiate separate + i2c-clients or spi-devices 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 bus-multi-instantiate. config MLX_PLATFORM tristate "Mellanox Technologies platform support" diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 219478061683..639a50af0bec 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -108,7 +108,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_BUS_MULTI_INSTANTIATE) += bus-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/bus-multi-instantiate.c b/drivers/platform/x86/bus-multi-instantiate.c new file mode 100644 index 000000000000..1b55380a2057 --- /dev/null +++ b/drivers/platform/x86/bus-multi-instantiate.c @@ -0,0 +1,432 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Bus multi-instantiate driver, pseudo driver to instantiate multiple + * i2c-clients or spi-devices from a single fwnode. + * + * Copyright 2018 Hans de Goede <hdegoede@redhat.com> + */ + +#include <linux/acpi.h> +#include <linux/bits.h> +#include <linux/i2c.h> +#include <linux/interrupt.h> +#include <linux/kernel.h> +#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) +#define IRQ_RESOURCE_NONE 0 +#define IRQ_RESOURCE_GPIO 1 +#define IRQ_RESOURCE_APIC 2 +#define MAX_RESOURCE_SOURCE_CHAR 30 + +struct inst_data { + const char *type; + unsigned int flags; + int irq_idx; +}; + +struct multi_inst_data { + int i2c_num; + int spi_num; + struct spi_device **spi_devs; + struct i2c_client **i2c_devs; +}; + +struct spi_acpi_data { + char resource_source[MAX_RESOURCE_SOURCE_CHAR]; + struct acpi_resource_spi_serialbus sb; +}; + +struct spi_serialbus_acpi_data { + int count; + struct spi_acpi_data acpi_data[]; +}; + +static int 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; +} + +static int spi_count_resources(struct acpi_device *adev) +{ + LIST_HEAD(r); + int count = 0; + int ret; + + ret = acpi_dev_get_resources(adev, &r, spi_count, &count); + if (ret < 0) + return ret; + + acpi_dev_free_resource_list(&r); + return count; +} + +static int spi_save_res(struct acpi_resource *ares, void *data) +{ + struct acpi_resource_spi_serialbus *sb; + struct spi_serialbus_acpi_data *resources = 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; + + memcpy(&resources->acpi_data[resources->count].sb, sb, sizeof(*sb)); + strscpy(resources->acpi_data[resources->count].resource_source, + sb->resource_source.string_ptr, + sizeof(resources->acpi_data[resources->count].resource_source)); + resources->count++; + + return 1; +} + +static struct spi_serialbus_acpi_data *spi_get_resources(struct device *dev, + struct acpi_device *adev, int count) +{ + struct spi_serialbus_acpi_data *resources; + LIST_HEAD(r); + int ret; + + resources = kmalloc(struct_size(resources, acpi_data, count), GFP_KERNEL); + if (!resources) + return NULL; + + ret = acpi_dev_get_resources(adev, &r, spi_save_res, resources); + if (ret < 0) + goto error; + + acpi_dev_free_resource_list(&r); + + return resources; + +error: + kfree(resources); + return NULL; +} + +static struct spi_controller *find_spi_controller(char *path) +{ + struct spi_controller *ctlr; + acpi_handle parent_handle; + acpi_status status; + int i; + + status = acpi_get_handle(NULL, path, &parent_handle); + if (ACPI_FAILURE(status)) + return NULL; + + /* There will be not more than 10 spi controller for a device */ + for (i = 0 ; i < 10 ; i++) { + ctlr = spi_busnum_to_master(i); + if (ctlr && ACPI_HANDLE(ctlr->dev.parent) == parent_handle) + return ctlr; + } + + return NULL; +} + +static int spi_multi_inst_probe(struct platform_device *pdev, struct acpi_device *adev, + const struct inst_data *inst_data, int count) +{ + struct spi_serialbus_acpi_data *acpi_data; + struct device *dev = &pdev->dev; + struct multi_inst_data *multi; + struct spi_controller *ctlr; + struct spi_device *spi_dev; + char name[50]; + int i, ret; + + multi = devm_kzalloc(dev, sizeof(*multi), GFP_KERNEL); + if (!multi) + return -ENOMEM; + + multi->spi_devs = devm_kcalloc(dev, count, sizeof(*multi->spi_devs), GFP_KERNEL); + if (!multi->spi_devs) + return -ENOMEM; + + acpi_data = spi_get_resources(dev, adev, count); + if (!acpi_data) + return -ENOMEM; + + for (i = 0; i < count && inst_data[i].type; i++) { + ctlr = find_spi_controller(acpi_data->acpi_data[i].resource_source); + if (!ctlr) { + ret = -EPROBE_DEFER; + goto probe_error; + } + + spi_dev = spi_alloc_device(ctlr); + if (!spi_dev) { + dev_err(&ctlr->dev, "failed to allocate SPI device for %s\n", + dev_name(&adev->dev)); + ret = -ENOMEM; + goto probe_error; + } + + strscpy(spi_dev->modalias, inst_data[i].type, sizeof(spi_dev->modalias)); + + if (ctlr->fw_translate_cs) { + int cs = ctlr->fw_translate_cs(ctlr, + acpi_data->acpi_data[i].sb.device_selection); + if (cs < 0) { + ret = cs; + goto probe_error; + } + spi_dev->chip_select = cs; + } else { + spi_dev->chip_select = acpi_data->acpi_data[i].sb.device_selection; + } + + spi_dev->max_speed_hz = acpi_data->acpi_data[i].sb.connection_speed; + spi_dev->bits_per_word = acpi_data->acpi_data[i].sb.data_bit_length; + + if (acpi_data->acpi_data[i].sb.clock_phase == ACPI_SPI_SECOND_PHASE) + spi_dev->mode |= SPI_CPHA; + if (acpi_data->acpi_data[i].sb.clock_polarity == ACPI_SPI_START_HIGH) + spi_dev->mode |= SPI_CPOL; + if (acpi_data->acpi_data[i].sb.device_polarity == ACPI_SPI_ACTIVE_HIGH) + spi_dev->mode |= SPI_CS_HIGH; + + switch (inst_data[i].flags & IRQ_RESOURCE_TYPE) { + case IRQ_RESOURCE_GPIO: + ret = acpi_dev_gpio_irq_get(adev, inst_data[i].irq_idx); + if (ret < 0) { + if (ret != -EPROBE_DEFER) + dev_err(dev, "Error requesting irq at index %d: %d\n", + inst_data[i].irq_idx, ret); + goto probe_error; + } + spi_dev->irq = ret; + break; + case IRQ_RESOURCE_APIC: + ret = platform_get_irq(pdev, inst_data[i].irq_idx); + if (ret < 0) { + dev_dbg(dev, "Error requesting irq at index %d: %d\n", + inst_data[i].irq_idx, ret); + goto probe_error; + } + spi_dev->irq = ret; + break; + default: + spi_dev->irq = 0; + break; + } + + snprintf(name, sizeof(name), "%s.%u-%s-%s.%d", dev_name(&ctlr->dev), + spi_dev->chip_select, dev_name(dev), inst_data[i].type, i); + spi_dev->dev.init_name = name; + + if (spi_add_device(spi_dev)) { + dev_err(&ctlr->dev, "failed to add SPI device %s from ACPI\n", + dev_name(&adev->dev)); + spi_dev_put(spi_dev); + goto probe_error; + } + + multi->spi_devs[i] = spi_dev; + multi->spi_num++; + } + + if (multi->spi_num < count) { + dev_err(dev, "Error finding driver, idx %d\n", i); + ret = -ENODEV; + goto probe_error; + } + + dev_info(dev, "Instantiate %d devices.\n", multi->spi_num); + platform_set_drvdata(pdev, multi); + kfree(acpi_data); + + return 0; + +probe_error: + while (--i >= 0) + spi_unregister_device(multi->spi_devs[i]); + + kfree(acpi_data); + return ret; +} + +static int i2c_multi_inst_probe(struct platform_device *pdev, struct acpi_device *adev, + const struct inst_data *inst_data, int count) +{ + struct i2c_board_info board_info = {}; + struct device *dev = &pdev->dev; + struct multi_inst_data *multi; + char name[32]; + int i, ret; + + multi = devm_kzalloc(dev, sizeof(*multi), GFP_KERNEL); + if (!multi) + return -ENOMEM; + + multi->i2c_devs = devm_kcalloc(dev, count, sizeof(*multi->i2c_devs), GFP_KERNEL); + if (!multi->i2c_devs) + return -ENOMEM; + + for (i = 0; i < count && inst_data[i].type; i++) { + memset(&board_info, 0, sizeof(board_info)); + strscpy(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); + board_info.dev_name = name; + switch (inst_data[i].flags & IRQ_RESOURCE_TYPE) { + case IRQ_RESOURCE_GPIO: + ret = acpi_dev_gpio_irq_get(adev, inst_data[i].irq_idx); + if (ret < 0) { + if (ret != -EPROBE_DEFER) + dev_err(dev, "Error requesting irq at index %d: %d\n", + inst_data[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); + if (ret < 0) { + dev_dbg(dev, "Error requesting irq at index %d: %d\n", + inst_data[i].irq_idx, ret); + goto error; + } + board_info.irq = ret; + break; + default: + board_info.irq = 0; + break; + } + multi->i2c_devs[i] = i2c_acpi_new_device(dev, i, &board_info); + if (IS_ERR(multi->i2c_devs[i])) { + ret = dev_err_probe(dev, PTR_ERR(multi->i2c_devs[i]), + "Error creating i2c-client, idx %d\n", i); + goto error; + } + multi->i2c_num++; + } + if (multi->i2c_num < count) { + dev_err(dev, "Error finding driver, idx %d\n", i); + ret = -ENODEV; + goto error; + } + + dev_info(dev, "Instantiate %d devices.\n", multi->i2c_num); + platform_set_drvdata(pdev, multi); + + return 0; + +error: + while (--i >= 0) + i2c_unregister_device(multi->i2c_devs[i]); + + return ret; +} + +static int bus_multi_inst_probe(struct platform_device *pdev) +{ + const struct inst_data *inst_data; + struct device *dev = &pdev->dev; + struct acpi_device *adev; + int count; + + inst_data = device_get_match_data(dev); + if (!inst_data) { + dev_err(dev, "Error ACPI match data is missing\n"); + return -ENODEV; + } + + adev = ACPI_COMPANION(dev); + + /* Count number of i2c clients to instantiate */ + count = i2c_acpi_client_count(adev); + if (count > 0) + return i2c_multi_inst_probe(pdev, adev, inst_data, count); + else if (count < 0) + dev_warn(dev, "I2C multi instantiate error %d\n", count); + + /* Count number of spi devices to instantiate */ + count = spi_count_resources(adev); + if (count > 0) + return spi_multi_inst_probe(pdev, adev, inst_data, count); + else if (count < 0) + dev_warn(dev, "SPI multi instantiate error %d\n", count); + + return -ENODEV; +} + +static int bus_multi_inst_remove(struct platform_device *pdev) +{ + struct multi_inst_data *multi = platform_get_drvdata(pdev); + int i; + + for (i = 0; i < multi->i2c_num; i++) + i2c_unregister_device(multi->i2c_devs[i]); + + for (i = 0; i < multi->spi_num; i++) + spi_unregister_device(multi->spi_devs[i]); + + return 0; +} + +static const struct inst_data bsg1160_data[] = { + { "bmc150_accel", IRQ_RESOURCE_GPIO, 0 }, + { "bmc150_magn" }, + { "bmg160" }, + {} +}; + +static const struct inst_data 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 inst_data int3515_data[] = { + { "tps6598x", IRQ_RESOURCE_APIC, 0 }, + { "tps6598x", IRQ_RESOURCE_APIC, 1 }, + { "tps6598x", IRQ_RESOURCE_APIC, 2 }, + { "tps6598x", IRQ_RESOURCE_APIC, 3 }, + {} +}; + +/* + * Note new device-ids must also be added to bus_multi_instantiate_ids in + * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). + */ +static const struct acpi_device_id bus_multi_inst_acpi_ids[] = { + { "BSG1160", (unsigned long)bsg1160_data }, + { "BSG2150", (unsigned long)bsg2150_data }, + { "INT3515", (unsigned long)int3515_data }, + { } +}; +MODULE_DEVICE_TABLE(acpi, bus_multi_inst_acpi_ids); + +static struct platform_driver bus_multi_inst_driver = { + .driver = { + .name = "Bus multi instantiate pseudo device driver", + .acpi_match_table = bus_multi_inst_acpi_ids, + }, + .probe = bus_multi_inst_probe, + .remove = bus_multi_inst_remove, +}; +module_platform_driver(bus_multi_inst_driver); + +MODULE_DESCRIPTION("Bus multi instantiate pseudo device driver"); +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>"); +MODULE_LICENSE("GPL"); diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c deleted file mode 100644 index 4956a1df5b90..000000000000 --- a/drivers/platform/x86/i2c-multi-instantiate.c +++ /dev/null @@ -1,174 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/* - * I2C multi-instantiate driver, pseudo driver to instantiate multiple - * i2c-clients from a single fwnode. - * - * Copyright 2018 Hans de Goede <hdegoede@redhat.com> - */ - -#include <linux/acpi.h> -#include <linux/bits.h> -#include <linux/i2c.h> -#include <linux/interrupt.h> -#include <linux/kernel.h> -#include <linux/module.h> -#include <linux/platform_device.h> -#include <linux/property.h> -#include <linux/types.h> - -#define IRQ_RESOURCE_TYPE GENMASK(1, 0) -#define IRQ_RESOURCE_NONE 0 -#define IRQ_RESOURCE_GPIO 1 -#define IRQ_RESOURCE_APIC 2 - -struct i2c_inst_data { - const char *type; - unsigned int flags; - int irq_idx; -}; - -struct i2c_multi_inst_data { - int num_clients; - struct i2c_client *clients[]; -}; - -static int i2c_multi_inst_probe(struct platform_device *pdev) -{ - struct i2c_multi_inst_data *multi; - const struct i2c_inst_data *inst_data; - struct i2c_board_info board_info = {}; - struct device *dev = &pdev->dev; - struct acpi_device *adev; - char name[32]; - int i, ret; - - inst_data = device_get_match_data(dev); - if (!inst_data) { - 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; - - multi = devm_kmalloc(dev, struct_size(multi, clients, ret), GFP_KERNEL); - if (!multi) - return -ENOMEM; - - multi->num_clients = ret; - - for (i = 0; i < multi->num_clients && inst_data[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); - board_info.dev_name = name; - switch (inst_data[i].flags & IRQ_RESOURCE_TYPE) { - case IRQ_RESOURCE_GPIO: - ret = acpi_dev_gpio_irq_get(adev, inst_data[i].irq_idx); - if (ret < 0) { - dev_err(dev, "Error requesting irq at index %d: %d\n", - inst_data[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); - if (ret < 0) { - dev_dbg(dev, "Error requesting irq at index %d: %d\n", - inst_data[i].irq_idx, ret); - goto error; - } - board_info.irq = ret; - break; - default: - 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]), - "Error creating i2c-client, idx %d\n", i); - goto error; - } - } - if (i < multi->num_clients) { - dev_err(dev, "Error finding driver, idx %d\n", i); - ret = -ENODEV; - goto error; - } - - platform_set_drvdata(pdev, multi); - return 0; - -error: - while (--i >= 0) - i2c_unregister_device(multi->clients[i]); - - return ret; -} - -static int i2c_multi_inst_remove(struct platform_device *pdev) -{ - struct i2c_multi_inst_data *multi = platform_get_drvdata(pdev); - int i; - - for (i = 0; i < multi->num_clients; i++) - i2c_unregister_device(multi->clients[i]); - - return 0; -} - -static const struct i2c_inst_data bsg1160_data[] = { - { "bmc150_accel", IRQ_RESOURCE_GPIO, 0 }, - { "bmc150_magn" }, - { "bmg160" }, - {} -}; - -static const struct i2c_inst_data 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 i2c_inst_data int3515_data[] = { - { "tps6598x", IRQ_RESOURCE_APIC, 0 }, - { "tps6598x", IRQ_RESOURCE_APIC, 1 }, - { "tps6598x", IRQ_RESOURCE_APIC, 2 }, - { "tps6598x", IRQ_RESOURCE_APIC, 3 }, - {} -}; - -/* - * Note new device-ids must also be added to i2c_multi_instantiate_ids in - * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). - */ -static const struct acpi_device_id i2c_multi_inst_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); - -static struct platform_driver i2c_multi_inst_driver = { - .driver = { - .name = "I2C multi instantiate pseudo device driver", - .acpi_match_table = i2c_multi_inst_acpi_ids, - }, - .probe = i2c_multi_inst_probe, - .remove = i2c_multi_inst_remove, -}; -module_platform_driver(i2c_multi_inst_driver); - -MODULE_DESCRIPTION("I2C 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] 10+ messages in thread
* Re: [PATCH 3/3] platform/x86: Support Spi in i2c-multi-instantiate driver 2021-12-02 16:24 ` [PATCH 3/3] platform/x86: Support Spi in i2c-multi-instantiate driver Stefan Binding @ 2021-12-03 11:22 ` Hans de Goede 2021-12-03 11:35 ` Andy Shevchenko 1 sibling, 0 replies; 10+ messages in thread From: Hans de Goede @ 2021-12-03 11:22 UTC (permalink / raw) To: Stefan Binding, Mark Brown, Rafael J . Wysocki, Len Brown, Mark Gross Cc: linux-kernel, linux-spi, linux-acpi, platform-driver-x86, patches Hi Stefan, On 12/2/21 17:24, Stefan Binding wrote: > Add support for spi bus in i2c-multi-instantiate driver > and rename it for a multiple purpose driver name > By adding spi support into this driver enables devices > to use the same _HID string for i2c and spi uses and > minimize the support for two drivers doing the same thing > for different busses > > Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com> > --- > MAINTAINERS | 4 +- > drivers/acpi/scan.c | 19 +- > drivers/platform/x86/Kconfig | 14 +- > drivers/platform/x86/Makefile | 2 +- > drivers/platform/x86/bus-multi-instantiate.c | 432 +++++++++++++++++++ > drivers/platform/x86/i2c-multi-instantiate.c | 174 -------- git's rename detection is failing here. Please split this in 2 patches: 1. Just the rename without any code changes (outside of Kconfig + Makefile) 2. The actual code changes This will make it much easier to review what has actually changed and will also make git log --follow work properly. I have one more remark below. > 6 files changed, 452 insertions(+), 193 deletions(-) > create mode 100644 drivers/platform/x86/bus-multi-instantiate.c > delete mode 100644 drivers/platform/x86/i2c-multi-instantiate.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 5b1e79f8e3d8..f75600d917bc 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -387,11 +387,11 @@ L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) > S: Maintained > F: drivers/acpi/arm64 > > -ACPI I2C MULTI INSTANTIATE DRIVER > +ACPI BUS 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/bus-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 5991dddbc9ce..2f7da1a08112 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -1696,14 +1696,15 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) > struct list_head resource_list; > bool is_serial_bus_slave = false; > /* > - * 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 I2cSerialBus/SpiSerialBusV2 resources > + * and an (i2c/spi)-client must be instantiated for each, each with > + * its own i2c_device_id/spi_device_id. > + * Normally we only instantiate an (i2c/spi)-client for the first > + * resource, using the ACPI HID as id. These special cases are handled > + * by the drivers/platform/x86/bus-multi-instantiate.c driver, which > + * knows which i2c_device_id or spi_device_id to use for each resource. > */ > - static const struct acpi_device_id i2c_multi_instantiate_ids[] = { > + static const struct acpi_device_id bus_multi_instantiate_ids[] = { > {"BSG1160", }, > {"BSG2150", }, > {"INT33FE", }, > @@ -1721,8 +1722,8 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) > fwnode_property_present(&device->fwnode, "baud"))) > return true; > > - /* Instantiate a pdev for the i2c-multi-instantiate drv to bind to */ > - if (!acpi_match_device_ids(device, i2c_multi_instantiate_ids)) > + /* Instantiate a pdev for the bus-multi-instantiate drv to bind to */ > + if (!acpi_match_device_ids(device, bus_multi_instantiate_ids)) > return false; > > INIT_LIST_HEAD(&resource_list); > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 97e87628eb35..5a413b123c01 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -958,16 +958,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" > - depends on I2C && ACPI > +config BUS_MULTI_INSTANTIATE > + tristate "Bus multi instantiate pseudo device driver" > + depends on I2C && SPI && ACPI > help > - Some ACPI-based systems list multiple i2c-devices in a single ACPI > - firmware-node. This driver will instantiate separate i2c-clients > - for each device in the firmware-node. > + Some ACPI-based systems list multiple i2c/spi devices in a > + single ACPI firmware-node. This driver will instantiate separate > + i2c-clients or spi-devices 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 bus-multi-instantiate. > > config MLX_PLATFORM > tristate "Mellanox Technologies platform support" > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index 219478061683..639a50af0bec 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -108,7 +108,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_BUS_MULTI_INSTANTIATE) += bus-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/bus-multi-instantiate.c b/drivers/platform/x86/bus-multi-instantiate.c > new file mode 100644 > index 000000000000..1b55380a2057 > --- /dev/null > +++ b/drivers/platform/x86/bus-multi-instantiate.c > @@ -0,0 +1,432 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Bus multi-instantiate driver, pseudo driver to instantiate multiple > + * i2c-clients or spi-devices from a single fwnode. > + * > + * Copyright 2018 Hans de Goede <hdegoede@redhat.com> > + */ > + > +#include <linux/acpi.h> > +#include <linux/bits.h> > +#include <linux/i2c.h> > +#include <linux/interrupt.h> > +#include <linux/kernel.h> > +#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) > +#define IRQ_RESOURCE_NONE 0 > +#define IRQ_RESOURCE_GPIO 1 > +#define IRQ_RESOURCE_APIC 2 > +#define MAX_RESOURCE_SOURCE_CHAR 30 > + > +struct inst_data { > + const char *type; > + unsigned int flags; > + int irq_idx; > +}; > + > +struct multi_inst_data { > + int i2c_num; > + int spi_num; > + struct spi_device **spi_devs; > + struct i2c_client **i2c_devs; > +}; > + > +struct spi_acpi_data { > + char resource_source[MAX_RESOURCE_SOURCE_CHAR]; > + struct acpi_resource_spi_serialbus sb; > +}; > + > +struct spi_serialbus_acpi_data { > + int count; > + struct spi_acpi_data acpi_data[]; > +}; > + > +static int 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; > +} > + > +static int spi_count_resources(struct acpi_device *adev) > +{ > + LIST_HEAD(r); > + int count = 0; > + int ret; > + > + ret = acpi_dev_get_resources(adev, &r, spi_count, &count); > + if (ret < 0) > + return ret; > + > + acpi_dev_free_resource_list(&r); > + return count; > +} > + > +static int spi_save_res(struct acpi_resource *ares, void *data) > +{ > + struct acpi_resource_spi_serialbus *sb; > + struct spi_serialbus_acpi_data *resources = 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; > + > + memcpy(&resources->acpi_data[resources->count].sb, sb, sizeof(*sb)); > + strscpy(resources->acpi_data[resources->count].resource_source, > + sb->resource_source.string_ptr, > + sizeof(resources->acpi_data[resources->count].resource_source)); > + resources->count++; > + > + return 1; > +} > + > +static struct spi_serialbus_acpi_data *spi_get_resources(struct device *dev, > + struct acpi_device *adev, int count) > +{ > + struct spi_serialbus_acpi_data *resources; > + LIST_HEAD(r); > + int ret; > + > + resources = kmalloc(struct_size(resources, acpi_data, count), GFP_KERNEL); > + if (!resources) > + return NULL; > + > + ret = acpi_dev_get_resources(adev, &r, spi_save_res, resources); > + if (ret < 0) > + goto error; > + > + acpi_dev_free_resource_list(&r); > + > + return resources; > + > +error: > + kfree(resources); > + return NULL; > +} > + > +static struct spi_controller *find_spi_controller(char *path) > +{ > + struct spi_controller *ctlr; > + acpi_handle parent_handle; > + acpi_status status; > + int i; > + > + status = acpi_get_handle(NULL, path, &parent_handle); > + if (ACPI_FAILURE(status)) > + return NULL; > + > + /* There will be not more than 10 spi controller for a device */ > + for (i = 0 ; i < 10 ; i++) { > + ctlr = spi_busnum_to_master(i); > + if (ctlr && ACPI_HANDLE(ctlr->dev.parent) == parent_handle) > + return ctlr; > + } > + > + return NULL; > +} > + > +static int spi_multi_inst_probe(struct platform_device *pdev, struct acpi_device *adev, > + const struct inst_data *inst_data, int count) > +{ > + struct spi_serialbus_acpi_data *acpi_data; > + struct device *dev = &pdev->dev; > + struct multi_inst_data *multi; > + struct spi_controller *ctlr; > + struct spi_device *spi_dev; > + char name[50]; > + int i, ret; > + > + multi = devm_kzalloc(dev, sizeof(*multi), GFP_KERNEL); > + if (!multi) > + return -ENOMEM; > + > + multi->spi_devs = devm_kcalloc(dev, count, sizeof(*multi->spi_devs), GFP_KERNEL); > + if (!multi->spi_devs) > + return -ENOMEM; > + > + acpi_data = spi_get_resources(dev, adev, count); > + if (!acpi_data) > + return -ENOMEM; > + > + for (i = 0; i < count && inst_data[i].type; i++) { > + ctlr = find_spi_controller(acpi_data->acpi_data[i].resource_source); > + if (!ctlr) { > + ret = -EPROBE_DEFER; > + goto probe_error; > + } > + > + spi_dev = spi_alloc_device(ctlr); > + if (!spi_dev) { > + dev_err(&ctlr->dev, "failed to allocate SPI device for %s\n", > + dev_name(&adev->dev)); > + ret = -ENOMEM; > + goto probe_error; > + } > + > + strscpy(spi_dev->modalias, inst_data[i].type, sizeof(spi_dev->modalias)); > + > + if (ctlr->fw_translate_cs) { > + int cs = ctlr->fw_translate_cs(ctlr, > + acpi_data->acpi_data[i].sb.device_selection); > + if (cs < 0) { > + ret = cs; > + goto probe_error; > + } > + spi_dev->chip_select = cs; > + } else { > + spi_dev->chip_select = acpi_data->acpi_data[i].sb.device_selection; > + } > + > + spi_dev->max_speed_hz = acpi_data->acpi_data[i].sb.connection_speed; > + spi_dev->bits_per_word = acpi_data->acpi_data[i].sb.data_bit_length; > + > + if (acpi_data->acpi_data[i].sb.clock_phase == ACPI_SPI_SECOND_PHASE) > + spi_dev->mode |= SPI_CPHA; > + if (acpi_data->acpi_data[i].sb.clock_polarity == ACPI_SPI_START_HIGH) > + spi_dev->mode |= SPI_CPOL; > + if (acpi_data->acpi_data[i].sb.device_polarity == ACPI_SPI_ACTIVE_HIGH) > + spi_dev->mode |= SPI_CS_HIGH; > + > + switch (inst_data[i].flags & IRQ_RESOURCE_TYPE) { > + case IRQ_RESOURCE_GPIO: > + ret = acpi_dev_gpio_irq_get(adev, inst_data[i].irq_idx); > + if (ret < 0) { > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "Error requesting irq at index %d: %d\n", > + inst_data[i].irq_idx, ret); > + goto probe_error; > + } > + spi_dev->irq = ret; > + break; > + case IRQ_RESOURCE_APIC: > + ret = platform_get_irq(pdev, inst_data[i].irq_idx); > + if (ret < 0) { > + dev_dbg(dev, "Error requesting irq at index %d: %d\n", > + inst_data[i].irq_idx, ret); > + goto probe_error; > + } > + spi_dev->irq = ret; > + break; > + default: > + spi_dev->irq = 0; > + break; > + } This switch (inst_data[i].flags & IRQ_RESOURCE_TYPE) { } block is duplicated with the i2c code, please put this in some get_irq helper, so that you end up writing: spi_dev->irq = multi_inst_get_irq(pdev, &inst_data[i]); Here. > + > + snprintf(name, sizeof(name), "%s.%u-%s-%s.%d", dev_name(&ctlr->dev), > + spi_dev->chip_select, dev_name(dev), inst_data[i].type, i); > + spi_dev->dev.init_name = name; > + > + if (spi_add_device(spi_dev)) { > + dev_err(&ctlr->dev, "failed to add SPI device %s from ACPI\n", > + dev_name(&adev->dev)); > + spi_dev_put(spi_dev); > + goto probe_error; > + } > + > + multi->spi_devs[i] = spi_dev; > + multi->spi_num++; > + } > + > + if (multi->spi_num < count) { > + dev_err(dev, "Error finding driver, idx %d\n", i); > + ret = -ENODEV; > + goto probe_error; > + } > + > + dev_info(dev, "Instantiate %d devices.\n", multi->spi_num); > + platform_set_drvdata(pdev, multi); > + kfree(acpi_data); > + > + return 0; > + > +probe_error: > + while (--i >= 0) > + spi_unregister_device(multi->spi_devs[i]); > + > + kfree(acpi_data); > + return ret; > +} > + > +static int i2c_multi_inst_probe(struct platform_device *pdev, struct acpi_device *adev, > + const struct inst_data *inst_data, int count) > +{ > + struct i2c_board_info board_info = {}; > + struct device *dev = &pdev->dev; > + struct multi_inst_data *multi; > + char name[32]; > + int i, ret; > + > + multi = devm_kzalloc(dev, sizeof(*multi), GFP_KERNEL); > + if (!multi) > + return -ENOMEM; > + > + multi->i2c_devs = devm_kcalloc(dev, count, sizeof(*multi->i2c_devs), GFP_KERNEL); > + if (!multi->i2c_devs) > + return -ENOMEM; > + > + for (i = 0; i < count && inst_data[i].type; i++) { > + memset(&board_info, 0, sizeof(board_info)); > + strscpy(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); > + board_info.dev_name = name; > + switch (inst_data[i].flags & IRQ_RESOURCE_TYPE) { > + case IRQ_RESOURCE_GPIO: > + ret = acpi_dev_gpio_irq_get(adev, inst_data[i].irq_idx); > + if (ret < 0) { > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "Error requesting irq at index %d: %d\n", > + inst_data[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); > + if (ret < 0) { > + dev_dbg(dev, "Error requesting irq at index %d: %d\n", > + inst_data[i].irq_idx, ret); > + goto error; > + } > + board_info.irq = ret; > + break; > + default: > + board_info.irq = 0; > + break; > + } > + multi->i2c_devs[i] = i2c_acpi_new_device(dev, i, &board_info); > + if (IS_ERR(multi->i2c_devs[i])) { > + ret = dev_err_probe(dev, PTR_ERR(multi->i2c_devs[i]), > + "Error creating i2c-client, idx %d\n", i); > + goto error; > + } > + multi->i2c_num++; > + } > + if (multi->i2c_num < count) { > + dev_err(dev, "Error finding driver, idx %d\n", i); > + ret = -ENODEV; > + goto error; > + } > + > + dev_info(dev, "Instantiate %d devices.\n", multi->i2c_num); > + platform_set_drvdata(pdev, multi); > + > + return 0; > + > +error: > + while (--i >= 0) > + i2c_unregister_device(multi->i2c_devs[i]); > + > + return ret; > +} > + > +static int bus_multi_inst_probe(struct platform_device *pdev) > +{ > + const struct inst_data *inst_data; > + struct device *dev = &pdev->dev; > + struct acpi_device *adev; > + int count; > + > + inst_data = device_get_match_data(dev); > + if (!inst_data) { > + dev_err(dev, "Error ACPI match data is missing\n"); > + return -ENODEV; > + } > + > + adev = ACPI_COMPANION(dev); You should check that adev != NULL here, the old code relied on i2c_acpi_client_count() returning an error for adev == NULL, but since you are now just warning when that fails an explicit check for this would be good. > + > + /* Count number of i2c clients to instantiate */ > + count = i2c_acpi_client_count(adev); > + if (count > 0) > + return i2c_multi_inst_probe(pdev, adev, inst_data, count); What if an ACPI device has both i2c and spi resources ? Regards, Hans > + else if (count < 0) > + dev_warn(dev, "I2C multi instantiate error %d\n", count); > + > + /* Count number of spi devices to instantiate */ > + count = spi_count_resources(adev); > + if (count > 0) > + return spi_multi_inst_probe(pdev, adev, inst_data, count); > + else if (count < 0) > + dev_warn(dev, "SPI multi instantiate error %d\n", count); > + > + return -ENODEV; > +} > + > +static int bus_multi_inst_remove(struct platform_device *pdev) > +{ > + struct multi_inst_data *multi = platform_get_drvdata(pdev); > + int i; > + > + for (i = 0; i < multi->i2c_num; i++) > + i2c_unregister_device(multi->i2c_devs[i]); > + > + for (i = 0; i < multi->spi_num; i++) > + spi_unregister_device(multi->spi_devs[i]); > + > + return 0; > +} > + > +static const struct inst_data bsg1160_data[] = { > + { "bmc150_accel", IRQ_RESOURCE_GPIO, 0 }, > + { "bmc150_magn" }, > + { "bmg160" }, > + {} > +}; > + > +static const struct inst_data 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 inst_data int3515_data[] = { > + { "tps6598x", IRQ_RESOURCE_APIC, 0 }, > + { "tps6598x", IRQ_RESOURCE_APIC, 1 }, > + { "tps6598x", IRQ_RESOURCE_APIC, 2 }, > + { "tps6598x", IRQ_RESOURCE_APIC, 3 }, > + {} > +}; > + > +/* > + * Note new device-ids must also be added to bus_multi_instantiate_ids in > + * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). > + */ > +static const struct acpi_device_id bus_multi_inst_acpi_ids[] = { > + { "BSG1160", (unsigned long)bsg1160_data }, > + { "BSG2150", (unsigned long)bsg2150_data }, > + { "INT3515", (unsigned long)int3515_data }, > + { } > +}; > +MODULE_DEVICE_TABLE(acpi, bus_multi_inst_acpi_ids); > + > +static struct platform_driver bus_multi_inst_driver = { > + .driver = { > + .name = "Bus multi instantiate pseudo device driver", > + .acpi_match_table = bus_multi_inst_acpi_ids, > + }, > + .probe = bus_multi_inst_probe, > + .remove = bus_multi_inst_remove, > +}; > +module_platform_driver(bus_multi_inst_driver); > + > +MODULE_DESCRIPTION("Bus multi instantiate pseudo device driver"); > +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c > deleted file mode 100644 > index 4956a1df5b90..000000000000 > --- a/drivers/platform/x86/i2c-multi-instantiate.c > +++ /dev/null > @@ -1,174 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0+ > -/* > - * I2C multi-instantiate driver, pseudo driver to instantiate multiple > - * i2c-clients from a single fwnode. > - * > - * Copyright 2018 Hans de Goede <hdegoede@redhat.com> > - */ > - > -#include <linux/acpi.h> > -#include <linux/bits.h> > -#include <linux/i2c.h> > -#include <linux/interrupt.h> > -#include <linux/kernel.h> > -#include <linux/module.h> > -#include <linux/platform_device.h> > -#include <linux/property.h> > -#include <linux/types.h> > - > -#define IRQ_RESOURCE_TYPE GENMASK(1, 0) > -#define IRQ_RESOURCE_NONE 0 > -#define IRQ_RESOURCE_GPIO 1 > -#define IRQ_RESOURCE_APIC 2 > - > -struct i2c_inst_data { > - const char *type; > - unsigned int flags; > - int irq_idx; > -}; > - > -struct i2c_multi_inst_data { > - int num_clients; > - struct i2c_client *clients[]; > -}; > - > -static int i2c_multi_inst_probe(struct platform_device *pdev) > -{ > - struct i2c_multi_inst_data *multi; > - const struct i2c_inst_data *inst_data; > - struct i2c_board_info board_info = {}; > - struct device *dev = &pdev->dev; > - struct acpi_device *adev; > - char name[32]; > - int i, ret; > - > - inst_data = device_get_match_data(dev); > - if (!inst_data) { > - 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; > - > - multi = devm_kmalloc(dev, struct_size(multi, clients, ret), GFP_KERNEL); > - if (!multi) > - return -ENOMEM; > - > - multi->num_clients = ret; > - > - for (i = 0; i < multi->num_clients && inst_data[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); > - board_info.dev_name = name; > - switch (inst_data[i].flags & IRQ_RESOURCE_TYPE) { > - case IRQ_RESOURCE_GPIO: > - ret = acpi_dev_gpio_irq_get(adev, inst_data[i].irq_idx); > - if (ret < 0) { > - dev_err(dev, "Error requesting irq at index %d: %d\n", > - inst_data[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); > - if (ret < 0) { > - dev_dbg(dev, "Error requesting irq at index %d: %d\n", > - inst_data[i].irq_idx, ret); > - goto error; > - } > - board_info.irq = ret; > - break; > - default: > - 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]), > - "Error creating i2c-client, idx %d\n", i); > - goto error; > - } > - } > - if (i < multi->num_clients) { > - dev_err(dev, "Error finding driver, idx %d\n", i); > - ret = -ENODEV; > - goto error; > - } > - > - platform_set_drvdata(pdev, multi); > - return 0; > - > -error: > - while (--i >= 0) > - i2c_unregister_device(multi->clients[i]); > - > - return ret; > -} > - > -static int i2c_multi_inst_remove(struct platform_device *pdev) > -{ > - struct i2c_multi_inst_data *multi = platform_get_drvdata(pdev); > - int i; > - > - for (i = 0; i < multi->num_clients; i++) > - i2c_unregister_device(multi->clients[i]); > - > - return 0; > -} > - > -static const struct i2c_inst_data bsg1160_data[] = { > - { "bmc150_accel", IRQ_RESOURCE_GPIO, 0 }, > - { "bmc150_magn" }, > - { "bmg160" }, > - {} > -}; > - > -static const struct i2c_inst_data 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 i2c_inst_data int3515_data[] = { > - { "tps6598x", IRQ_RESOURCE_APIC, 0 }, > - { "tps6598x", IRQ_RESOURCE_APIC, 1 }, > - { "tps6598x", IRQ_RESOURCE_APIC, 2 }, > - { "tps6598x", IRQ_RESOURCE_APIC, 3 }, > - {} > -}; > - > -/* > - * Note new device-ids must also be added to i2c_multi_instantiate_ids in > - * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). > - */ > -static const struct acpi_device_id i2c_multi_inst_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); > - > -static struct platform_driver i2c_multi_inst_driver = { > - .driver = { > - .name = "I2C multi instantiate pseudo device driver", > - .acpi_match_table = i2c_multi_inst_acpi_ids, > - }, > - .probe = i2c_multi_inst_probe, > - .remove = i2c_multi_inst_remove, > -}; > -module_platform_driver(i2c_multi_inst_driver); > - > -MODULE_DESCRIPTION("I2C multi instantiate pseudo device driver"); > -MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>"); > -MODULE_LICENSE("GPL"); > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] platform/x86: Support Spi in i2c-multi-instantiate driver 2021-12-02 16:24 ` [PATCH 3/3] platform/x86: Support Spi in i2c-multi-instantiate driver Stefan Binding 2021-12-03 11:22 ` Hans de Goede @ 2021-12-03 11:35 ` Andy Shevchenko 1 sibling, 0 replies; 10+ messages in thread From: Andy Shevchenko @ 2021-12-03 11:35 UTC (permalink / raw) To: Stefan Binding Cc: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, linux-kernel, linux-spi, linux-acpi, platform-driver-x86, patches On Thu, Dec 02, 2021 at 04:24:21PM +0000, Stefan Binding wrote: > Add support for spi bus in i2c-multi-instantiate driver > and rename it for a multiple purpose driver name > By adding spi support into this driver enables devices > to use the same _HID string for i2c and spi uses and > minimize the support for two drivers doing the same thing > for different busses Please take care about periods at the end of sentences. But this is minor in comparison to the following issues: - you enable this for existing I²C multi-instantiate devices, are you sure it is fine? - continuing above, how can you guarantee that the same ID would be used I²C and SPI versions of the same chip and not, let's say, for UART? - or other way around, how do we know that the same component will have the same ID for different bus types? (Yes, I understand that this is logically appropriate assumption, but you never know OEMs and others in their ways to (ab)use ACPI specifications and common sense) - if we even go this way, it should be under drivers/acpi -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()" 2021-12-02 16:24 [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()" Stefan Binding 2021-12-02 16:24 ` [PATCH 2/3] spi: Make spi_alloc_device and spi_add_device public again Stefan Binding 2021-12-02 16:24 ` [PATCH 3/3] platform/x86: Support Spi in i2c-multi-instantiate driver Stefan Binding @ 2021-12-02 16:53 ` Mark Brown 2021-12-03 11:06 ` Andy Shevchenko 3 siblings, 0 replies; 10+ messages in thread From: Mark Brown @ 2021-12-02 16:53 UTC (permalink / raw) To: Stefan Binding Cc: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, linux-kernel, linux-spi, linux-acpi, platform-driver-x86, patches, Lucas Tanure [-- Attachment #1: Type: text/plain, Size: 390 bytes --] On Thu, Dec 02, 2021 at 04:24:19PM +0000, Stefan Binding wrote: > From: Lucas Tanure <tanureal@opensource.cirrus.com> > > Revert commit bdc7ca008e1f ("spi: Remove unused function > spi_busnum_to_master()") > This function is needed for the spi version of i2c multi > instantiate driver. If we're going to restore this API we should rename it to _controller() while we're at it. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()" 2021-12-02 16:24 [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()" Stefan Binding ` (2 preceding siblings ...) 2021-12-02 16:53 ` [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()" Mark Brown @ 2021-12-03 11:06 ` Andy Shevchenko 2021-12-03 11:14 ` Hans de Goede 3 siblings, 1 reply; 10+ messages in thread From: Andy Shevchenko @ 2021-12-03 11:06 UTC (permalink / raw) To: Stefan Binding Cc: Mark Brown, Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, linux-kernel, linux-spi, linux-acpi, platform-driver-x86, patches, Lucas Tanure On Thu, Dec 02, 2021 at 04:24:19PM +0000, Stefan Binding wrote: > From: Lucas Tanure <tanureal@opensource.cirrus.com> > > Revert commit bdc7ca008e1f ("spi: Remove unused function > spi_busnum_to_master()") > This function is needed for the spi version of i2c multi > instantiate driver. I understand the intention, but I have no clue from this series (it lacks of proper cover letter, it lacks of much better and justified commit message in the patch 3) what is the actual issue. Without these to be provided it's no go for the series. Please, provide much better description what is the actual issue you are trying to solve (from patch 3 my guts telling me that this can be achieved differently without this code being involved). -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()" 2021-12-03 11:06 ` Andy Shevchenko @ 2021-12-03 11:14 ` Hans de Goede 2021-12-10 18:10 ` Lucas tanure 0 siblings, 1 reply; 10+ messages in thread From: Hans de Goede @ 2021-12-03 11:14 UTC (permalink / raw) To: Andy Shevchenko, Stefan Binding Cc: Mark Brown, Rafael J . Wysocki, Len Brown, Mark Gross, linux-kernel, linux-spi, linux-acpi, platform-driver-x86, patches, Lucas Tanure Hi, On 12/3/21 12:06, Andy Shevchenko wrote: > On Thu, Dec 02, 2021 at 04:24:19PM +0000, Stefan Binding wrote: >> From: Lucas Tanure <tanureal@opensource.cirrus.com> >> >> Revert commit bdc7ca008e1f ("spi: Remove unused function >> spi_busnum_to_master()") >> This function is needed for the spi version of i2c multi >> instantiate driver. > > I understand the intention, but I have no clue from this series (it lacks of > proper cover letter, it lacks of much better and justified commit message in > the patch 3) what is the actual issue. Without these to be provided it's no go > for the series. Please, provide much better description what is the actual > issue you are trying to solve (from patch 3 my guts telling me that this can > be achieved differently without this code being involved). Yes I assume that eventually there will be a follow-up which will actually add some new ACPI HIDs to the new bus-multi-instantiate.c file ? Can we please have (some of) those patches as part of the next version, so that we can see how you will actually use this? Also I'm wondering is this actually about ACPI device's having multiple SpiSerialBusV2 resources in a single _CRS resource list ? Or do you plan to use this for devices with only a single I2cSerialBusV2 or SpiSerialBusV2 resource to e.g. share IRQ lookup code between the 2 cases ? If you plan to use this for devices with only a single I2cSerialBusV2 or SpiSerialBusV2 resource, then I'm going to have to nack this. Each ACPI HID which needs to be handled in this code also needs an entry in the i2c_multi_instantiate_ids[] list in drivers/acpi/scan.c which is code which ends up loaded on every single ACPI system, so we really only want to add HIDs there for the special case of having multiple I2cSerialBusV2 or SpiSerialBusV2 resources in a single ACPI Device / ACPI fwnode. If you are looking to use this as a way to share code for other reasons (which is a good goal to strive for!) please find another way, such as e.g. adding some helper functions to drivers/gpio/gpiolib-acpi.c (note there already are a couple of helpers there which you may use). Regards, Hans ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()" 2021-12-03 11:14 ` Hans de Goede @ 2021-12-10 18:10 ` Lucas tanure 2021-12-10 18:22 ` Hans de Goede 0 siblings, 1 reply; 10+ messages in thread From: Lucas tanure @ 2021-12-10 18:10 UTC (permalink / raw) To: Hans de Goede, Andy Shevchenko, Stefan Binding Cc: Mark Brown, Rafael J . Wysocki, Len Brown, Mark Gross, linux-kernel, linux-spi, linux-acpi, platform-driver-x86, patches On 12/3/21 11:14, Hans de Goede wrote: > Hi, > > On 12/3/21 12:06, Andy Shevchenko wrote: >> On Thu, Dec 02, 2021 at 04:24:19PM +0000, Stefan Binding wrote: >>> From: Lucas Tanure <tanureal@opensource.cirrus.com> >>> >>> Revert commit bdc7ca008e1f ("spi: Remove unused function >>> spi_busnum_to_master()") >>> This function is needed for the spi version of i2c multi >>> instantiate driver. >> >> I understand the intention, but I have no clue from this series (it lacks of >> proper cover letter, it lacks of much better and justified commit message in >> the patch 3) what is the actual issue. Without these to be provided it's no go >> for the series. Please, provide much better description what is the actual >> issue you are trying to solve (from patch 3 my guts telling me that this can >> be achieved differently without this code being involved). > > Yes I assume that eventually there will be a follow-up which will > actually add some new ACPI HIDs to the new bus-multi-instantiate.c file ? > Yes, we are developing an HDA sound driver for the HID CSC3551, which is used for laptops that use SPI or I2C. And in that series is where we plan to put a patch to add that HID. > Can we please have (some of) those patches as part of the next > version, so that we can see how you will actually use this? The series is this one https://lkml.org/lkml/2021/11/23/723, but the SPI HID was not ready to be sent in that version, but will be part of the next submission. > > Also I'm wondering is this actually about ACPI device's having multiple > SpiSerialBusV2 resources in a single _CRS resource list ? yes, a single _CRS with 2 or 4 SpiSerialBusV2 inside. > > Or do you plan to use this for devices with only a single > I2cSerialBusV2 or SpiSerialBusV2 resource to e.g. share IRQ lookup > code between the 2 cases ? No, the minimum number SpiSerialBusV2 or I2cSerialBusV2 inside the _CRS is two. > > If you plan to use this for devices with only a single > I2cSerialBusV2 or SpiSerialBusV2 resource, then I'm going to have to > nack this. > > Each ACPI HID which needs to be handled in this code also needs an > entry in the i2c_multi_instantiate_ids[] list in drivers/acpi/scan.c > which is code which ends up loaded on every single ACPI system, so > we really only want to add HIDs there for the special case of having > multiple I2cSerialBusV2 or SpiSerialBusV2 resources in a single > ACPI Device / ACPI fwnode. > > If you are looking to use this as a way to share code for other reasons > (which is a good goal to strive for!) please find another way, such > as e.g. adding some helper functions to drivers/gpio/gpiolib-acpi.c > (note there already are a couple of helpers there which you may use). No, we only want to multi instantiate SPI or I2C devices from a single _CRS. > > Regards, > > Hans > We sent a request to the laptop vendor about releasing the SPI DSDT, and after that gets cleared, we will send it to you for review. That will likely be next. Thanks Lucas Tanure ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()" 2021-12-10 18:10 ` Lucas tanure @ 2021-12-10 18:22 ` Hans de Goede 0 siblings, 0 replies; 10+ messages in thread From: Hans de Goede @ 2021-12-10 18:22 UTC (permalink / raw) To: Lucas tanure, Andy Shevchenko, Stefan Binding Cc: Mark Brown, Rafael J . Wysocki, Len Brown, Mark Gross, linux-kernel, linux-spi, linux-acpi, platform-driver-x86, patches Hi, On 12/10/21 19:10, Lucas tanure wrote: > On 12/3/21 11:14, Hans de Goede wrote: >> Hi, >> >> On 12/3/21 12:06, Andy Shevchenko wrote: >>> On Thu, Dec 02, 2021 at 04:24:19PM +0000, Stefan Binding wrote: >>>> From: Lucas Tanure <tanureal@opensource.cirrus.com> >>>> >>>> Revert commit bdc7ca008e1f ("spi: Remove unused function >>>> spi_busnum_to_master()") >>>> This function is needed for the spi version of i2c multi >>>> instantiate driver. >>> >>> I understand the intention, but I have no clue from this series (it lacks of >>> proper cover letter, it lacks of much better and justified commit message in >>> the patch 3) what is the actual issue. Without these to be provided it's no go >>> for the series. Please, provide much better description what is the actual >>> issue you are trying to solve (from patch 3 my guts telling me that this can >>> be achieved differently without this code being involved). >> >> Yes I assume that eventually there will be a follow-up which will >> actually add some new ACPI HIDs to the new bus-multi-instantiate.c file ? >> > Yes, we are developing an HDA sound driver for the HID CSC3551, > which is used for laptops that use SPI or I2C. > And in that series is where we plan to put a patch to add that HID. > >> Can we please have (some of) those patches as part of the next >> version, so that we can see how you will actually use this? > The series is this one https://lkml.org/lkml/2021/11/23/723, but > the SPI HID was not ready to be sent in that version, but will be > part of the next submission. > >> >> Also I'm wondering is this actually about ACPI device's having multiple >> SpiSerialBusV2 resources in a single _CRS resource list ? > yes, a single _CRS with 2 or 4 SpiSerialBusV2 inside. > >> >> Or do you plan to use this for devices with only a single >> I2cSerialBusV2 or SpiSerialBusV2 resource to e.g. share IRQ lookup >> code between the 2 cases ? > No, the minimum number SpiSerialBusV2 or I2cSerialBusV2 inside the > _CRS is two. > >> >> If you plan to use this for devices with only a single >> I2cSerialBusV2 or SpiSerialBusV2 resource, then I'm going to have to >> nack this. >> >> Each ACPI HID which needs to be handled in this code also needs an >> entry in the i2c_multi_instantiate_ids[] list in drivers/acpi/scan.c >> which is code which ends up loaded on every single ACPI system, so >> we really only want to add HIDs there for the special case of having >> multiple I2cSerialBusV2 or SpiSerialBusV2 resources in a single >> ACPI Device / ACPI fwnode. >> >> If you are looking to use this as a way to share code for other reasons >> (which is a good goal to strive for!) please find another way, such >> as e.g. adding some helper functions to drivers/gpio/gpiolib-acpi.c >> (note there already are a couple of helpers there which you may use). > No, we only want to multi instantiate SPI or I2C devices from a single _CRS. Ok, that is fine, thank you for clarifying things. Regards, Hans ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-12-10 18:22 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-02 16:24 [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()" Stefan Binding 2021-12-02 16:24 ` [PATCH 2/3] spi: Make spi_alloc_device and spi_add_device public again Stefan Binding 2021-12-02 16:24 ` [PATCH 3/3] platform/x86: Support Spi in i2c-multi-instantiate driver Stefan Binding 2021-12-03 11:22 ` Hans de Goede 2021-12-03 11:35 ` Andy Shevchenko 2021-12-02 16:53 ` [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()" Mark Brown 2021-12-03 11:06 ` Andy Shevchenko 2021-12-03 11:14 ` Hans de Goede 2021-12-10 18:10 ` Lucas tanure 2021-12-10 18:22 ` Hans de Goede
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.