Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1327511 > unrolled thread

[RFC PATCH] SPI/ACPI: DesignWare: Add ACPI support for Designware SPI driver

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH] SPI/ACPI: DesignWare: Add ACPI support for Designware SPI driver qiujiang <qiujiang@huawei.com> - 2016-02-05 08:10 +0100
    Re: [RFC PATCH] SPI/ACPI: DesignWare: Add ACPI support for  Designware SPI driver Mark Brown <broonie@kernel.org> - 2016-02-05 12:20 +0100
    Re: [RFC PATCH] SPI/ACPI: DesignWare: Add ACPI support for Designware  SPI driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-02-05 17:00 +0100
      Re: [RFC PATCH] SPI/ACPI: DesignWare: Add ACPI support for  Designware SPI driver Mark Brown <broonie@kernel.org> - 2016-02-05 17:20 +0100

#1327511 — [RFC PATCH] SPI/ACPI: DesignWare: Add ACPI support for Designware SPI driver

This patch added ACPI support for DesignWare SPI mmio driver. It
was based the corresponding DT driver and compatible for this two
way. This patch has been tested on Hisilicon D02 board. It relies
on the GPIO patchset.

Signed-off-by: qiujiang <qiujiang@huawei.com>
---
 drivers/spi/spi-dw-mmio.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index a6d7029..de542f0 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -8,6 +8,7 @@
  * version 2, as published by the Free Software Foundation.
  */
 
+#include <linux/acpi.h>
 #include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
@@ -36,7 +37,9 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 	struct dw_spi *dws;
 	struct resource *mem;
 	int ret;
-	int num_cs;
+	int num_cs, i;
+	struct gpio_desc *gpiod;
+	char propname[32];
 
 	dwsmmio = devm_kzalloc(&pdev->dev, sizeof(struct dw_spi_mmio),
 			GFP_KERNEL);
@@ -84,8 +87,6 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 	dws->num_cs = num_cs;
 
 	if (pdev->dev.of_node) {
-		int i;
-
 		for (i = 0; i < dws->num_cs; i++) {
 			int cs_gpio = of_get_named_gpio(pdev->dev.of_node,
 					"cs-gpios", i);
@@ -104,6 +105,18 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (ACPI_COMPANION(&pdev->dev)) {
+		for (i = 0; i < dws->num_cs; i++) {
+			snprintf(propname, sizeof(propname), "cs%d", i);
+			gpiod = devm_gpiod_get(&pdev->dev,
+				propname, GPIOD_ASIS);
+			if (IS_ERR(gpiod)) {
+				dev_err(&pdev->dev, "Get gpio desc failed!\n");
+				return PTR_ERR(gpiod);
+			}
+		}
+	}
+
 	ret = dw_spi_add_host(&pdev->dev, dws);
 	if (ret)
 		goto out;
@@ -132,12 +145,19 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
 
+static const struct acpi_device_id dw_spi_mmio_acpi_match[] = {
+		{"HISI0171", 0},
+		{ }
+};
+MODULE_DEVICE_TABLE(acpi, dw_spi_mmio_acpi_match);
+
 static struct platform_driver dw_spi_mmio_driver = {
 	.probe		= dw_spi_mmio_probe,
 	.remove		= dw_spi_mmio_remove,
 	.driver		= {
 		.name	= DRIVER_NAME,
 		.of_match_table = dw_spi_mmio_of_match,
+		.acpi_match_table = ACPI_PTR(dw_spi_mmio_acpi_match),
 	},
 };
 module_platform_driver(dw_spi_mmio_driver);
-- 
1.9.1

[toc] | [next] | [standalone]

#1327611 — Re: [RFC PATCH] SPI/ACPI: DesignWare: Add ACPI support for Designware SPI driver

[Multipart message — attachments visible in raw view] — view raw

On Fri, Feb 05, 2016 at 03:11:20PM +0800, qiujiang wrote:

> This patch added ACPI support for DesignWare SPI mmio driver. It
> was based the corresponding DT driver and compatible for this two
> way. This patch has been tested on Hisilicon D02 board. It relies
> on the GPIO patchset.

Intel are heavy users of this driver on their systems which also use
ACPI.  Have you discussed this binding with them?  I've copied Andy and
Jarkko who've worked on the driver recently.

Please use subject lines matching the style for the subsystem.  This
makes it easier for people to identify relevant patches.

> +	char propname[32];

That's a magic number, where did it come from and why is it a magic
nummber?

> +	if (ACPI_COMPANION(&pdev->dev)) {
> +		for (i = 0; i < dws->num_cs; i++) {
> +			snprintf(propname, sizeof(propname), "cs%d", i);
> +			gpiod = devm_gpiod_get(&pdev->dev,
> +				propname, GPIOD_ASIS);
> +			if (IS_ERR(gpiod)) {
> +				dev_err(&pdev->dev, "Get gpio desc failed!\n");
> +				return PTR_ERR(gpiod);
> +			}
> +		}
> +	}

I'm not seeing anywhere where we store the GPIO in this loop.  It is
therefore unclear to me how the chip select is going to work?

> +static const struct acpi_device_id dw_spi_mmio_acpi_match[] = {
> +		{"HISI0171", 0},
> +		{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, dw_spi_mmio_acpi_match);

I really do wish ACPI had some more sensible system for allocating
device IDs so the tables were a little more legible. :(

[toc] | [prev] | [next] | [standalone]

#1327859 — Re: [RFC PATCH] SPI/ACPI: DesignWare: Add ACPI support for Designware SPI driver

On Fri, Feb 5, 2016 at 9:11 AM, qiujiang <qiujiang@huawei.com> wrote:
> This patch added ACPI support for DesignWare SPI mmio driver. It
> was based the corresponding DT driver and compatible for this two
> way. This patch has been tested on Hisilicon D02 board. It relies
> on the GPIO patchset.

My comments below.

> @@ -84,8 +87,6 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
>         dws->num_cs = num_cs;
>
>         if (pdev->dev.of_node) {
> -               int i;
> -
>                 for (i = 0; i < dws->num_cs; i++) {
>                         int cs_gpio = of_get_named_gpio(pdev->dev.of_node,
>                                         "cs-gpios", i);

It seems the driver was never validated with more than one chip select.
Perhaps someone has to switch to use of_spi_register_master() here.

> @@ -104,6 +105,18 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
>                 }
>         }
>
> +       if (ACPI_COMPANION(&pdev->dev)) {
> +               for (i = 0; i < dws->num_cs; i++) {
> +                       snprintf(propname, sizeof(propname), "cs%d", i);
> +                       gpiod = devm_gpiod_get(&pdev->dev,
> +                               propname, GPIOD_ASIS);
> +                       if (IS_ERR(gpiod)) {
> +                               dev_err(&pdev->dev, "Get gpio desc failed!\n");
> +                               return PTR_ERR(gpiod);
> +                       }
> +               }
> +       }

Like Mark noticed there is also same issue. Do you indeed check the
configuration with different chip select signals?

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1327877 — Re: [RFC PATCH] SPI/ACPI: DesignWare: Add ACPI support for Designware SPI driver

[Multipart message — attachments visible in raw view] — view raw

On Fri, Feb 05, 2016 at 05:55:42PM +0200, Andy Shevchenko wrote:

> Like Mark noticed there is also same issue. Do you indeed check the
> configuration with different chip select signals?

It's not clear to me that *any* chip select signals work, we get a GPIO
descriptor back but I didn't see any code that ever used the value.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web