| Message ID | 20220121172431.6876-3-sbinding@opensource.cirrus.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support Spi in i2c-multi-instantiate driver | expand |
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 > */ >
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 > > */ > >
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.
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 */
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(-)