[PATCH 1/2] spi: clps711x: Driver refactor

[PATCH 1/2] spi: clps711x: Driver refactor

[Arnd Bergmann]

From: Alexander Shiyan <shc_work@mail.ru>

This is a complex patch for refactoring CLPS711X SPI driver.
This change adds devicetree support and removes board support.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/spi/spi-clps711x.c | 69 +++++++++++++++++-----------------------------
 1 file changed, 26 insertions(+), 43 deletions(-)

diff --git a/drivers/spi/spi-clps711x.c b/drivers/spi/spi-clps711x.c
index 8c30de0315e7..18193df2eba8 100644
--- a/drivers/spi/spi-clps711x.c
+++ b/drivers/spi/spi-clps711x.c
@@ -1,7 +1,7 @@
 /*
  *  CLPS711X SPI bus driver
  *
- *  Copyright (C) 2012-2014 Alexander Shiyan <shc_work@mail.ru>
+ *  Copyright (C) 2012-2016 Alexander Shiyan <shc_work@mail.ru>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -12,7 +12,6 @@
 #include <linux/io.h>
 #include <linux/clk.h>
 #include <linux/gpio.h>
-#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
@@ -20,9 +19,8 @@
 #include <linux/mfd/syscon.h>
 #include <linux/mfd/syscon/clps711x.h>
 #include <linux/spi/spi.h>
-#include <linux/platform_data/spi-clps711x.h>
 
-#define DRIVER_NAME	"spi-clps711x"
+#define DRIVER_NAME		"clps711x-spi"
 
 #define SYNCIO_FRMLEN(x)	((x) << 8)
 #define SYNCIO_TXFRMEN		(1 << 14)
@@ -40,6 +38,17 @@ struct spi_clps711x_data {
 
 static int spi_clps711x_setup(struct spi_device *spi)
 {
+	if (!spi->controller_state) {
+		int ret;
+
+		ret = devm_gpio_request(&spi->master->dev, spi->cs_gpio,
+					dev_name(&spi->master->dev));
+		if (ret)
+			return ret;
+
+		spi->controller_state = spi;
+	}
+
 	/* We are expect that SPI-device is not selected */
 	gpio_direction_output(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
 
@@ -104,20 +113,9 @@ static irqreturn_t spi_clps711x_isr(int irq, void *dev_id)
 static int spi_clps711x_probe(struct platform_device *pdev)
 {
 	struct spi_clps711x_data *hw;
-	struct spi_clps711x_pdata *pdata = dev_get_platdata(&pdev->dev);
 	struct spi_master *master;
 	struct resource *res;
-	int i, irq, ret;
-
-	if (!pdata) {
-		dev_err(&pdev->dev, "No platform data supplied\n");
-		return -EINVAL;
-	}
-
-	if (pdata->num_chipselect < 1) {
-		dev_err(&pdev->dev, "At least one CS must be defined\n");
-		return -EINVAL;
-	}
+	int irq, ret;
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
@@ -127,40 +125,24 @@ static int spi_clps711x_probe(struct platform_device *pdev)
 	if (!master)
 		return -ENOMEM;
 
-	master->cs_gpios = devm_kzalloc(&pdev->dev, sizeof(int) *
-					pdata->num_chipselect, GFP_KERNEL);
-	if (!master->cs_gpios) {
-		ret = -ENOMEM;
-		goto err_out;
-	}
-
-	master->bus_num = pdev->id;
+	master->bus_num = -1;
 	master->mode_bits = SPI_CPHA | SPI_CS_HIGH;
 	master->bits_per_word_mask =  SPI_BPW_RANGE_MASK(1, 8);
-	master->num_chipselect = pdata->num_chipselect;
+	master->dev.of_node = pdev->dev.of_node;
 	master->setup = spi_clps711x_setup;
 	master->prepare_message = spi_clps711x_prepare_message;
 	master->transfer_one = spi_clps711x_transfer_one;
 
 	hw = spi_master_get_devdata(master);
 
-	for (i = 0; i < master->num_chipselect; i++) {
-		master->cs_gpios[i] = pdata->chipselect[i];
-		ret = devm_gpio_request(&pdev->dev, master->cs_gpios[i],
-					DRIVER_NAME);
-		if (ret) {
-			dev_err(&pdev->dev, "Can't get CS GPIO %i\n", i);
-			goto err_out;
-		}
-	}
-
 	hw->spi_clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(hw->spi_clk)) {
 		ret = PTR_ERR(hw->spi_clk);
 		goto err_out;
 	}
 
-	hw->syscon = syscon_regmap_lookup_by_pdevname("syscon.3");
+	hw->syscon =
+		syscon_regmap_lookup_by_compatible("cirrus,ep7209-syscon3");
 	if (IS_ERR(hw->syscon)) {
 		ret = PTR_ERR(hw->syscon);
 		goto err_out;
@@ -185,14 +167,8 @@ static int spi_clps711x_probe(struct platform_device *pdev)
 		goto err_out;
 
 	ret = devm_spi_register_master(&pdev->dev, master);
-	if (!ret) {
-		dev_info(&pdev->dev,
-			 "SPI bus driver initialized. Master clock %u Hz\n",
-			 master->max_speed_hz);
+	if (!ret)
 		return 0;
-	}
-
-	dev_err(&pdev->dev, "Failed to register master\n");
 
 err_out:
 	spi_master_put(master);
@@ -200,9 +176,16 @@ err_out:
 	return ret;
 }
 
+static const struct of_device_id clps711x_spi_dt_ids[] = {
+	{ .compatible = "cirrus,ep7209-spi", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, clps711x_spi_dt_ids);
+
 static struct platform_driver clps711x_spi_driver = {
 	.driver	= {
 		.name	= DRIVER_NAME,
+		.of_match_table = clps711x_spi_dt_ids,
 	},
 	.probe	= spi_clps711x_probe,
 };
-- 
2.9.0

[PATCH 2/2] dt-bindings: spi: add binding for clps711x SPI

[Arnd Bergmann]

This documents the binding used by Alexander Shiyan's DT support for
the clps711x SPI controller.

I've left the file name to match the ARM platform port name "clps711x"
for consistency with the other bindings, even though the compatible
string refers to the later ep7309 chip.

Linux no longer supports the old clps711x and ep72xx product lines,
but we still use the name. The entire family is now discontinued
by the manufacturer.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Alexander Shiyan <shc_work@mail.ru>
---
 .../devicetree/bindings/spi/spi-clps711x.txt       | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-clps711x.txt

diff --git a/Documentation/devicetree/bindings/spi/spi-clps711x.txt b/Documentation/devicetree/bindings/spi/spi-clps711x.txt
new file mode 100644
index 000000000000..4c3ec13f423f
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-clps711x.txt
@@ -0,0 +1,33 @@
+Serial Peripheral Interface on Cirrus Logic CL-PS71xx, EP72xx, EP73xx
+
+Required properties
+- #address-cells: must be <1>
+- #size-cells: must be <0>
+- compatible: should include "cirrus,ep7209-spi"
+- reg: Address and length of one register range
+- interrupts: one interrupt line
+- clocks: One entry, refers to the SPI bus clock
+- cs-gpios: Specifies the gpio pins to be used for chipselects.
+	    See: Documentation/devicetree/bindings/spi/spi-bus.txt
+
+An additional register is present in the system controller,
+which is assumed to be in the same device tree, with and marked
+as compatible with "cirrus,ep7209-syscon3".
+
+Example:
+
+spi@80000500 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	compatible = "cirrus,ep7209-spi";
+	reg = <0x80000500 0x4>;
+	interrupts = <15>;
+	clocks = <&clks CLPS711X_CLK_SPI>;
+	status = "disabled";
+};
+
+syscon3: syscon@80002200 {
+	compatible = "cirrus,ep7209-syscon3", "syscon";
+	reg = <0x80002200 0x40>;
+};
+
-- 
2.9.0

Re: [PATCH 2/2] dt-bindings: spi: add binding for clps711x SPI

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 292 bytes --]

On Wed, Jul 06, 2016 at 04:53:13PM +0200, Arnd Bergmann wrote:
> This documents the binding used by Alexander Shiyan's DT support for
> the clps711x SPI controller.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 2/2] dt-bindings: spi: add binding for clps711x SPI

[Arnd Bergmann]

On Thursday, July 7, 2016 11:25:46 AM CEST Mark Brown wrote:
> On Wed, Jul 06, 2016 at 04:53:13PM +0200, Arnd Bergmann wrote:
> > This documents the binding used by Alexander Shiyan's DT support for
> > the clps711x SPI controller.
> 
> Please use subject lines matching the style for the subsystem.  This
> makes it easier for people to identify relevant patches.

I tried, though it's a bit tricky for dt-bindings. When I created the
patch, using 'dt-bindings: $SUBSYSTEM' appeared to be the most common
one, ahead of '$SUBSYSTEM' (mainly used for combined patches)
and other random prefixes.

I'll resend it with just 'spi:', but I don't think there is a broad
consensus on this in the existing patches.

	Arnd

Applied "spi: add binding for clps711x SPI" to the spi tree

[Mark Brown]

The patch

   spi: add binding for clps711x SPI

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

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 add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From ea2ff61ba3f8239483f5cd823ac0113d194b469f Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 6 Jul 2016 16:53:13 +0200
Subject: [PATCH] spi: add binding for clps711x SPI

This documents the binding used by Alexander Shiyan's DT support for
the clps711x SPI controller.

I've left the file name to match the ARM platform port name "clps711x"
for consistency with the other bindings, even though the compatible
string refers to the later ep7309 chip.

Linux no longer supports the old clps711x and ep72xx product lines,
but we still use the name. The entire family is now discontinued
by the manufacturer.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../devicetree/bindings/spi/spi-clps711x.txt       | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-clps711x.txt

diff --git a/Documentation/devicetree/bindings/spi/spi-clps711x.txt b/Documentation/devicetree/bindings/spi/spi-clps711x.txt
new file mode 100644
index 000000000000..4c3ec13f423f
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-clps711x.txt
@@ -0,0 +1,33 @@
+Serial Peripheral Interface on Cirrus Logic CL-PS71xx, EP72xx, EP73xx
+
+Required properties
+- #address-cells: must be <1>
+- #size-cells: must be <0>
+- compatible: should include "cirrus,ep7209-spi"
+- reg: Address and length of one register range
+- interrupts: one interrupt line
+- clocks: One entry, refers to the SPI bus clock
+- cs-gpios: Specifies the gpio pins to be used for chipselects.
+	    See: Documentation/devicetree/bindings/spi/spi-bus.txt
+
+An additional register is present in the system controller,
+which is assumed to be in the same device tree, with and marked
+as compatible with "cirrus,ep7209-syscon3".
+
+Example:
+
+spi@80000500 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	compatible = "cirrus,ep7209-spi";
+	reg = <0x80000500 0x4>;
+	interrupts = <15>;
+	clocks = <&clks CLPS711X_CLK_SPI>;
+	status = "disabled";
+};
+
+syscon3: syscon@80002200 {
+	compatible = "cirrus,ep7209-syscon3", "syscon";
+	reg = <0x80002200 0x40>;
+};
+
-- 
2.8.1

Applied "spi: clps711x: Driver refactor" to the spi tree

[Mark Brown]

The patch

   spi: clps711x: Driver refactor

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

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 add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 6acaadc852f10b9f13b665a5798d4f2783317f80 Mon Sep 17 00:00:00 2001
From: Alexander Shiyan <shc_work@mail.ru>
Date: Wed, 6 Jul 2016 16:53:12 +0200
Subject: [PATCH] spi: clps711x: Driver refactor

This is a complex patch for refactoring CLPS711X SPI driver.
This change adds devicetree support and removes board support.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-clps711x.c | 69 +++++++++++++++++-----------------------------
 1 file changed, 26 insertions(+), 43 deletions(-)

diff --git a/drivers/spi/spi-clps711x.c b/drivers/spi/spi-clps711x.c
index 8c30de0315e7..18193df2eba8 100644
--- a/drivers/spi/spi-clps711x.c
+++ b/drivers/spi/spi-clps711x.c
@@ -1,7 +1,7 @@
 /*
  *  CLPS711X SPI bus driver
  *
- *  Copyright (C) 2012-2014 Alexander Shiyan <shc_work@mail.ru>
+ *  Copyright (C) 2012-2016 Alexander Shiyan <shc_work@mail.ru>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -12,7 +12,6 @@
 #include <linux/io.h>
 #include <linux/clk.h>
 #include <linux/gpio.h>
-#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
@@ -20,9 +19,8 @@
 #include <linux/mfd/syscon.h>
 #include <linux/mfd/syscon/clps711x.h>
 #include <linux/spi/spi.h>
-#include <linux/platform_data/spi-clps711x.h>
 
-#define DRIVER_NAME	"spi-clps711x"
+#define DRIVER_NAME		"clps711x-spi"
 
 #define SYNCIO_FRMLEN(x)	((x) << 8)
 #define SYNCIO_TXFRMEN		(1 << 14)
@@ -40,6 +38,17 @@ struct spi_clps711x_data {
 
 static int spi_clps711x_setup(struct spi_device *spi)
 {
+	if (!spi->controller_state) {
+		int ret;
+
+		ret = devm_gpio_request(&spi->master->dev, spi->cs_gpio,
+					dev_name(&spi->master->dev));
+		if (ret)
+			return ret;
+
+		spi->controller_state = spi;
+	}
+
 	/* We are expect that SPI-device is not selected */
 	gpio_direction_output(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
 
@@ -104,20 +113,9 @@ static irqreturn_t spi_clps711x_isr(int irq, void *dev_id)
 static int spi_clps711x_probe(struct platform_device *pdev)
 {
 	struct spi_clps711x_data *hw;
-	struct spi_clps711x_pdata *pdata = dev_get_platdata(&pdev->dev);
 	struct spi_master *master;
 	struct resource *res;
-	int i, irq, ret;
-
-	if (!pdata) {
-		dev_err(&pdev->dev, "No platform data supplied\n");
-		return -EINVAL;
-	}
-
-	if (pdata->num_chipselect < 1) {
-		dev_err(&pdev->dev, "At least one CS must be defined\n");
-		return -EINVAL;
-	}
+	int irq, ret;
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
@@ -127,40 +125,24 @@ static int spi_clps711x_probe(struct platform_device *pdev)
 	if (!master)
 		return -ENOMEM;
 
-	master->cs_gpios = devm_kzalloc(&pdev->dev, sizeof(int) *
-					pdata->num_chipselect, GFP_KERNEL);
-	if (!master->cs_gpios) {
-		ret = -ENOMEM;
-		goto err_out;
-	}
-
-	master->bus_num = pdev->id;
+	master->bus_num = -1;
 	master->mode_bits = SPI_CPHA | SPI_CS_HIGH;
 	master->bits_per_word_mask =  SPI_BPW_RANGE_MASK(1, 8);
-	master->num_chipselect = pdata->num_chipselect;
+	master->dev.of_node = pdev->dev.of_node;
 	master->setup = spi_clps711x_setup;
 	master->prepare_message = spi_clps711x_prepare_message;
 	master->transfer_one = spi_clps711x_transfer_one;
 
 	hw = spi_master_get_devdata(master);
 
-	for (i = 0; i < master->num_chipselect; i++) {
-		master->cs_gpios[i] = pdata->chipselect[i];
-		ret = devm_gpio_request(&pdev->dev, master->cs_gpios[i],
-					DRIVER_NAME);
-		if (ret) {
-			dev_err(&pdev->dev, "Can't get CS GPIO %i\n", i);
-			goto err_out;
-		}
-	}
-
 	hw->spi_clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(hw->spi_clk)) {
 		ret = PTR_ERR(hw->spi_clk);
 		goto err_out;
 	}
 
-	hw->syscon = syscon_regmap_lookup_by_pdevname("syscon.3");
+	hw->syscon =
+		syscon_regmap_lookup_by_compatible("cirrus,ep7209-syscon3");
 	if (IS_ERR(hw->syscon)) {
 		ret = PTR_ERR(hw->syscon);
 		goto err_out;
@@ -185,14 +167,8 @@ static int spi_clps711x_probe(struct platform_device *pdev)
 		goto err_out;
 
 	ret = devm_spi_register_master(&pdev->dev, master);
-	if (!ret) {
-		dev_info(&pdev->dev,
-			 "SPI bus driver initialized. Master clock %u Hz\n",
-			 master->max_speed_hz);
+	if (!ret)
 		return 0;
-	}
-
-	dev_err(&pdev->dev, "Failed to register master\n");
 
 err_out:
 	spi_master_put(master);
@@ -200,9 +176,16 @@ err_out:
 	return ret;
 }
 
+static const struct of_device_id clps711x_spi_dt_ids[] = {
+	{ .compatible = "cirrus,ep7209-spi", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, clps711x_spi_dt_ids);
+
 static struct platform_driver clps711x_spi_driver = {
 	.driver	= {
 		.name	= DRIVER_NAME,
+		.of_match_table = clps711x_spi_dt_ids,
 	},
 	.probe	= spi_clps711x_probe,
 };
-- 
2.8.1

Re: [PATCH 2/2] dt-bindings: spi: add binding for clps711x SPI

[Rob Herring]

On Thu, Jul 07, 2016 at 11:48:25AM +0200, Arnd Bergmann wrote:
> On Thursday, July 7, 2016 11:25:46 AM CEST Mark Brown wrote:
> > On Wed, Jul 06, 2016 at 04:53:13PM +0200, Arnd Bergmann wrote:
> > > This documents the binding used by Alexander Shiyan's DT support for
> > > the clps711x SPI controller.
> > 
> > Please use subject lines matching the style for the subsystem.  This
> > makes it easier for people to identify relevant patches.
> 
> I tried, though it's a bit tricky for dt-bindings. When I created the
> patch, using 'dt-bindings: $SUBSYSTEM' appeared to be the most common
> one, ahead of '$SUBSYSTEM' (mainly used for combined patches)
> and other random prefixes.

There isn't. Mark and I don't agree here as the former is my 
preference, but I care less than he does. :) 

Rob