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;elsereturn 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
*/