[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9cc5b24d-7a4e-073e-f7a4-124f76d44d77@redhat.com>
Date: Tue, 1 Feb 2022 15:29:32 +0100
From: Hans de Goede <hdegoede@...hat.com>
To: Stefan Binding <sbinding@...nsource.cirrus.com>,
Mark Brown <broonie@...nel.org>,
"Rafael J . Wysocki" <rafael@...nel.org>,
Len Brown <lenb@...nel.org>, Mark Gross <markgross@...nel.org>,
Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>
Cc: alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
linux-spi@...r.kernel.org, linux-acpi@...r.kernel.org,
platform-driver-x86@...r.kernel.org, patches@...nsource.cirrus.com
Subject: Re: [PATCH v6 3/9] spi: Support selection of the index of the ACPI
Spi Resource before alloc
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@...nsource.cirrus.com>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@...hat.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
>
> /*
>
Powered by blists - more mailing lists