* [PATCH 0/4] spi: s3c64xx: fix DT binding breakage
@ 2014-07-16 15:19 Javier Martinez Canillas
2014-07-16 15:19 ` [PATCH 1/4] Revert "spi: s3c64xx: Added provision for dedicated cs pin" Javier Martinez Canillas
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Javier Martinez Canillas @ 2014-07-16 15:19 UTC (permalink / raw)
To: linux-arm-kernel
Hello Mark,
The s3c64xx SPI driver DT binding is currently broken. Commit
3146bee ("spi: s3c64xx: Added provision for dedicated cs pin")
added a new "cs-gpio" property and made it a requirement in
order to make the driver behave in the same way that it used to.
The motivation of the offending commit was to allow boards that
want to use the native chip select (instead of a GPIO) to work
with the s3c64xx SPI driver. Something that was not possible
before since the driver made it mandatory to specify a GPIO.
Unfortunately the commit also changed the driver defaults since
now besides specifying a GPIO, it makes mandatory to also specify
that a GPIO is used while the default used to be the opposite.
That mean that old FDT are not working anymore after 3146bee
so DT backward compatibility was not ensured by that commit.
This is a follow-up series from a previous one posted by Naveen
Krishna [0] which attempts to solve the issue.
The feedback from Naveen's series was that the patches did not
provide a clear explanation about the rationale and goal of both
the series as a whole and the individual changes [1].
So I tried to take Navee's series and rework them to provide
an easier to understand patch series.
The series is composed of the following patches:
Javier Martinez Canillas (1):
Revert "spi: s3c64xx: Added provision for dedicated cs pin"
Naveen Krishna Chatradhi (3):
spi: s3c64xx: use the generic SPI "cs-gpios" property
spi: samsung: Update binding documentation
ARM: DTS: fix the chip select gpios definition in the SPI nodes
.../devicetree/bindings/spi/spi-samsung.txt | 8 ++--
arch/arm/boot/dts/exynos4210-smdkv310.dts | 2 +-
arch/arm/boot/dts/exynos4412-trats2.dts | 2 +-
arch/arm/boot/dts/exynos5250-smdk5250.dts | 2 +-
drivers/spi/spi-s3c64xx.c | 54 ++++++++++------------
5 files changed, 30 insertions(+), 38 deletions(-)
Patch 01/04 reverts the offending commit since it not only broke
the DT binding but also introduced a confusing "cs-gpio" property
while the driver already used a property with the same name.
Patch 02/04 fixes the DT binding for good by using the SPI core
"cs-gpios" property to specify the chip select GPIO instead of
using a custom property only used by this driver. This change
breaks backward compatibility but this has been broken for more
than a year and nobody complained so I think it's safe to change
it again in favor of using standard DT binding properties.
A benefit of Patch 02/04 is that it allows DT and legacy boards
that need to use the built-in CS instead of a GPIO to work with
the driver which was the original motivation of the broken commit.
Patch 03/04 updates the driver binding documentation accordingly
and patch 04/04 updates all the DTS board files in mainline.
Best regards,
Javier
[0]: http://www.spinics.net/lists/linux-samsung-soc/msg34163.html
[1]: http://www.spinics.net/lists/linux-samsung-soc/msg34309.html
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/4] Revert "spi: s3c64xx: Added provision for dedicated cs pin"
2014-07-16 15:19 [PATCH 0/4] spi: s3c64xx: fix DT binding breakage Javier Martinez Canillas
@ 2014-07-16 15:19 ` Javier Martinez Canillas
2014-07-17 18:46 ` Mark Brown
2014-07-16 15:19 ` [PATCH 2/4] spi: s3c64xx: use the generic SPI "cs-gpios" property Javier Martinez Canillas
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Javier Martinez Canillas @ 2014-07-16 15:19 UTC (permalink / raw)
To: linux-arm-kernel
This reverts commit 3146beec21b64f4551fcf0ac148381d54dc41b1b.
This commit resulted in a DT backward compatibility breakage.
Some devices use the native chip select (CS) instead of a GPIO
pin to drive the CS line. But the SPI driver made it mandatory
to specify a GPIO pin in the SPI device node controller-data.
So, using the built-in CS was not possible with the driver.
Commit 3146bee tried to fix that by adding a "cs-gpio" property
which could be defined in the SPI device node to make the driver
request the GPIO from the controller-data node.
Unfortunately that changed the old DT binding semantics since
now it's mandatory to have the "cs-gpio" property defined in
the SPI device node in order to use a GPIO pin to drive the CS.
As an example, a SPI device was defined before the commit with:
spi at 12d20000 {
slave-node at 0 {
controller-data {
cs-gpio = <&gpb1 2 0>;
}
}
}
and after the commit, the following DTS snippet must be used:
spi at 12d20000 {
cs-gpio;
slave-node at 0 {
controller-data {
cs-gpio = <&gpb1 2 0>;
}
}
}
So, after commit 3146bee the driver does not look for the GPIO
by default and it only looks for it if the top level "cs-gpio"
property is defined while the default used to be the opposite.
To always request the GPIO defined in the controller-data node.
This means that old FDT that of course didn't have this added
"cs-gpio" DT property in the SPI node broke after this change.
The offending commit can't be reverted cleanly since more than
a year have passed and other changes were made in the meantime
but this patch partially reverts the driver to it's original
state so old FDT can work again.
This patch will break Device Trees that were relying on the new
behavior of course but the patch should be reverted because:
a) There aren't DTS in mainline that use this new property.
b) They were relying on a behavior that broke DT compatibility.
c) The new binding is awkard, needing two properties with the
same name (cs-gpio) on different nodes is confusing at least.
d) The new property was not added to the DT binding doc:
Documentation/devicetree/bindings/spi/spi-samsung.txt
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
---
drivers/spi/spi-s3c64xx.c | 37 +++++++++++--------------------------
1 file changed, 11 insertions(+), 26 deletions(-)
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index a771a3a..e48c6fa 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -197,7 +197,6 @@ struct s3c64xx_spi_driver_data {
struct s3c64xx_spi_dma_data tx_dma;
struct s3c64xx_spi_port_config *port_conf;
unsigned int port_id;
- bool cs_gpio;
};
static void flush_fifo(struct s3c64xx_spi_driver_data *sdd)
@@ -754,10 +753,8 @@ static struct s3c64xx_spi_csinfo *s3c64xx_get_slave_ctrldata(
{
struct s3c64xx_spi_csinfo *cs;
struct device_node *slave_np, *data_np = NULL;
- struct s3c64xx_spi_driver_data *sdd;
u32 fb_delay = 0;
- sdd = spi_master_get_devdata(spi->master);
slave_np = spi->dev.of_node;
if (!slave_np) {
dev_err(&spi->dev, "device node not found\n");
@@ -776,10 +773,7 @@ static struct s3c64xx_spi_csinfo *s3c64xx_get_slave_ctrldata(
return ERR_PTR(-ENOMEM);
}
- /* The CS line is asserted/deasserted by the gpio pin */
- if (sdd->cs_gpio)
- cs->line = of_get_named_gpio(data_np, "cs-gpio", 0);
-
+ cs->line = of_get_named_gpio(data_np, "cs-gpio", 0);
if (!gpio_is_valid(cs->line)) {
dev_err(&spi->dev, "chip select gpio is not specified or invalid\n");
kfree(cs);
@@ -818,20 +812,17 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
}
if (!spi_get_ctldata(spi)) {
- /* Request gpio only if cs line is asserted by gpio pins */
- if (sdd->cs_gpio) {
- err = gpio_request_one(cs->line, GPIOF_OUT_INIT_HIGH,
- dev_name(&spi->dev));
- if (err) {
- dev_err(&spi->dev,
- "Failed to get /CS gpio [%d]: %d\n",
- cs->line, err);
- goto err_gpio_req;
- }
-
- spi->cs_gpio = cs->line;
+ err = gpio_request_one(cs->line, GPIOF_OUT_INIT_HIGH,
+ dev_name(&spi->dev));
+ if (err) {
+ dev_err(&spi->dev,
+ "Failed to get /CS gpio [%d]: %d\n",
+ cs->line, err);
+ goto err_gpio_req;
}
+ spi->cs_gpio = cs->line;
+
spi_set_ctldata(spi, cs);
}
@@ -897,10 +888,8 @@ err_gpio_req:
static void s3c64xx_spi_cleanup(struct spi_device *spi)
{
struct s3c64xx_spi_csinfo *cs = spi_get_ctldata(spi);
- struct s3c64xx_spi_driver_data *sdd;
- sdd = spi_master_get_devdata(spi->master);
- if (spi->cs_gpio) {
+ if (cs) {
gpio_free(spi->cs_gpio);
if (spi->dev.of_node)
kfree(cs);
@@ -1075,11 +1064,7 @@ static int s3c64xx_spi_probe(struct platform_device *pdev)
sdd->cntrlr_info = sci;
sdd->pdev = pdev;
sdd->sfr_start = mem_res->start;
- sdd->cs_gpio = true;
if (pdev->dev.of_node) {
- if (!of_find_property(pdev->dev.of_node, "cs-gpio", NULL))
- sdd->cs_gpio = false;
-
ret = of_alias_get_id(pdev->dev.of_node, "spi");
if (ret < 0) {
dev_err(&pdev->dev, "failed to get alias id, errno %d\n",
--
2.0.0.rc2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/4] spi: s3c64xx: use the generic SPI "cs-gpios" property
2014-07-16 15:19 [PATCH 0/4] spi: s3c64xx: fix DT binding breakage Javier Martinez Canillas
2014-07-16 15:19 ` [PATCH 1/4] Revert "spi: s3c64xx: Added provision for dedicated cs pin" Javier Martinez Canillas
@ 2014-07-16 15:19 ` Javier Martinez Canillas
2014-07-17 18:48 ` Mark Brown
2014-07-16 15:19 ` [PATCH 3/4] spi: samsung: Update binding documentation Javier Martinez Canillas
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Javier Martinez Canillas @ 2014-07-16 15:19 UTC (permalink / raw)
To: linux-arm-kernel
From: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
The s3c64xx SPI driver uses a custom DT binding to specify
the GPIO used to drive the chip select (CS) line instead of
using the generic "cs-gpios" property already defined in:
Documentation/devicetree/bindings/spi/spi-bus.txt.
It's unfortunate that drivers are not using standard bindings
and creating custom ones instead but in most cases this can't
be changed without breaking Device Tree backward compatibility.
But in the case of this driver, its DT binding has been broken
for more than a year. Since after commit (dated June, 21 2013):
3146bee ("spi: s3c64xx: Added provision for dedicated cs pin")
DT backward compatibility was broken and nobody noticed until
now when the commit was reverted. So it seems to be safe to
change the binding to use the standard SPI "cs-gpios" property
instead of using a custom one just for this driver.
This patch also allows boards that don't use a GPIO pin for the
CS to work with the driver since the SPI core will take care of
setting spi->cs_gpio to -ENOENT if a board wants to use the built
in CS instead of a GPIO as explained in the SPI bus DT binding:
Documentation/devicetree/bindings/spi/spi-bus.txt.
For non-DT platforms, spi->cs_gpio will be set to -ENOENT as well
unless they specify a GPIO pin in their platform data. So both
native and GPIO chip select is also supported for legacy boards.
The above use case was what motivated commit 3146bee which broke
the DT binding backward compatibility in the first place.
Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
[javier.martinez at collabora.co.uk: split changes and improve commit message]
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
---
drivers/spi/spi-s3c64xx.c | 49 ++++++++++++++++++++++++++++-------------------
1 file changed, 29 insertions(+), 20 deletions(-)
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index e48c6fa..480133e 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -773,14 +773,6 @@ static struct s3c64xx_spi_csinfo *s3c64xx_get_slave_ctrldata(
return ERR_PTR(-ENOMEM);
}
- cs->line = of_get_named_gpio(data_np, "cs-gpio", 0);
- if (!gpio_is_valid(cs->line)) {
- dev_err(&spi->dev, "chip select gpio is not specified or invalid\n");
- kfree(cs);
- of_node_put(data_np);
- return ERR_PTR(-EINVAL);
- }
-
of_property_read_u32(data_np, "samsung,spi-feedback-delay", &fb_delay);
cs->fb_delay = fb_delay;
of_node_put(data_np);
@@ -801,9 +793,16 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
int err;
sdd = spi_master_get_devdata(spi->master);
- if (!cs && spi->dev.of_node) {
+ if (spi->dev.of_node) {
cs = s3c64xx_get_slave_ctrldata(spi);
spi->controller_data = cs;
+ } else if (cs) {
+ /* On non-DT platforms the SPI core will set spi->cs_gpio
+ * to -ENOENT. The GPIO pin used to drive the chip select
+ * is defined by using platform data so spi->cs_gpio value
+ * has to be override to have the proper GPIO pin number.
+ */
+ spi->cs_gpio = cs->line;
}
if (IS_ERR_OR_NULL(cs)) {
@@ -812,17 +811,17 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
}
if (!spi_get_ctldata(spi)) {
- err = gpio_request_one(cs->line, GPIOF_OUT_INIT_HIGH,
- dev_name(&spi->dev));
- if (err) {
- dev_err(&spi->dev,
- "Failed to get /CS gpio [%d]: %d\n",
- cs->line, err);
- goto err_gpio_req;
+ if (gpio_is_valid(spi->cs_gpio)) {
+ err = gpio_request_one(spi->cs_gpio, GPIOF_OUT_INIT_HIGH,
+ dev_name(&spi->dev));
+ if (err) {
+ dev_err(&spi->dev,
+ "Failed to get /CS gpio [%d]: %d\n",
+ spi->cs_gpio, err);
+ goto err_gpio_req;
+ }
}
- spi->cs_gpio = cs->line;
-
spi_set_ctldata(spi, cs);
}
@@ -875,7 +874,8 @@ setup_exit:
/* setup() returns with device de-selected */
writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
- gpio_free(cs->line);
+ if (gpio_is_valid(spi->cs_gpio))
+ gpio_free(spi->cs_gpio);
spi_set_ctldata(spi, NULL);
err_gpio_req:
@@ -889,11 +889,20 @@ static void s3c64xx_spi_cleanup(struct spi_device *spi)
{
struct s3c64xx_spi_csinfo *cs = spi_get_ctldata(spi);
- if (cs) {
+ if (gpio_is_valid(spi->cs_gpio)) {
gpio_free(spi->cs_gpio);
if (spi->dev.of_node)
kfree(cs);
+ else {
+ /* On non-DT platforms, the SPI core sets
+ * spi->cs_gpio to -ENOENT and .setup()
+ * overrides it with the GPIO pin value
+ * passed using platform data.
+ */
+ spi->cs_gpio = -ENOENT;
+ }
}
+
spi_set_ctldata(spi, NULL);
}
--
2.0.0.rc2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/4] spi: samsung: Update binding documentation
2014-07-16 15:19 [PATCH 0/4] spi: s3c64xx: fix DT binding breakage Javier Martinez Canillas
2014-07-16 15:19 ` [PATCH 1/4] Revert "spi: s3c64xx: Added provision for dedicated cs pin" Javier Martinez Canillas
2014-07-16 15:19 ` [PATCH 2/4] spi: s3c64xx: use the generic SPI "cs-gpios" property Javier Martinez Canillas
@ 2014-07-16 15:19 ` Javier Martinez Canillas
2014-07-17 18:48 ` Mark Brown
2014-07-16 15:19 ` [PATCH 4/4] ARM: DTS: fix the chip select gpios definition in the SPI nodes Javier Martinez Canillas
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Javier Martinez Canillas @ 2014-07-16 15:19 UTC (permalink / raw)
To: linux-arm-kernel
From: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
Samsung SPI driver now uses the generic SPI "cs-gpios"
binding so update the documentation accordingly.
Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
[javier.martinez at collabora.co.uk: split changes and improve commit message]
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
---
Documentation/devicetree/bindings/spi/spi-samsung.txt | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/spi/spi-samsung.txt b/Documentation/devicetree/bindings/spi/spi-samsung.txt
index 7f2d88d..1e8a857 100644
--- a/Documentation/devicetree/bindings/spi/spi-samsung.txt
+++ b/Documentation/devicetree/bindings/spi/spi-samsung.txt
@@ -38,15 +38,13 @@ Optional Board Specific Properties:
- num-cs: Specifies the number of chip select lines supported. If
not specified, the default number of chip select lines is set to 1.
+- cs-gpios: should specify GPIOs used for chipselects (see spi-bus.txt)
+
SPI Controller specific data in SPI slave nodes:
- The spi slave nodes should provide the following information which is required
by the spi controller.
- - cs-gpio: A gpio specifier that specifies the gpio line used as
- the slave select line by the spi controller. The format of the gpio
- specifier depends on the gpio controller.
-
- samsung,spi-feedback-delay: The sampling phase shift to be applied on the
miso line (to account for any lag in the miso line). The following are the
valid values.
@@ -84,6 +82,7 @@ Example:
#size-cells = <0>;
pinctrl-names = "default";
pinctrl-0 = <&spi0_bus>;
+ cs-gpios = <&gpa2 5 0>;
w25q80bw at 0 {
#address-cells = <1>;
@@ -93,7 +92,6 @@ Example:
spi-max-frequency = <10000>;
controller-data {
- cs-gpio = <&gpa2 5 1 0 3>;
samsung,spi-feedback-delay = <0>;
};
--
2.0.0.rc2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/4] ARM: DTS: fix the chip select gpios definition in the SPI nodes
2014-07-16 15:19 [PATCH 0/4] spi: s3c64xx: fix DT binding breakage Javier Martinez Canillas
` (2 preceding siblings ...)
2014-07-16 15:19 ` [PATCH 3/4] spi: samsung: Update binding documentation Javier Martinez Canillas
@ 2014-07-16 15:19 ` Javier Martinez Canillas
2014-07-17 18:49 ` Mark Brown
2014-07-16 15:50 ` [PATCH 0/4] spi: s3c64xx: fix DT binding breakage Naveen Krishna Ch
2014-07-16 16:02 ` Mark Brown
5 siblings, 1 reply; 15+ messages in thread
From: Javier Martinez Canillas @ 2014-07-16 15:19 UTC (permalink / raw)
To: linux-arm-kernel
From: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
This patch replaces the "cs-gpio" from "controller-data" node
as was specified in the old binding and uses the standard
"cs-gpios" property expected by the SPI core as is defined now
in the spi-s3c64xx driver DT binding.
Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
---
arch/arm/boot/dts/exynos4210-smdkv310.dts | 2 +-
arch/arm/boot/dts/exynos4412-trats2.dts | 2 +-
arch/arm/boot/dts/exynos5250-smdk5250.dts | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/exynos4210-smdkv310.dts b/arch/arm/boot/dts/exynos4210-smdkv310.dts
index 636d166..676e6e0 100644
--- a/arch/arm/boot/dts/exynos4210-smdkv310.dts
+++ b/arch/arm/boot/dts/exynos4210-smdkv310.dts
@@ -168,6 +168,7 @@
};
spi_2: spi at 13940000 {
+ cs-gpios = <&gpc1 2 0>;
status = "okay";
w25x80 at 0 {
@@ -178,7 +179,6 @@
spi-max-frequency = <1000000>;
controller-data {
- cs-gpio = <&gpc1 2 0>;
samsung,spi-feedback-delay = <0>;
};
diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts
index 7787844..11967f4 100644
--- a/arch/arm/boot/dts/exynos4412-trats2.dts
+++ b/arch/arm/boot/dts/exynos4412-trats2.dts
@@ -589,6 +589,7 @@
spi_1: spi at 13930000 {
pinctrl-names = "default";
pinctrl-0 = <&spi1_bus>;
+ cs-gpios = <&gpb 5 0>;
status = "okay";
s5c73m3_spi: s5c73m3 {
@@ -596,7 +597,6 @@
spi-max-frequency = <50000000>;
reg = <0>;
controller-data {
- cs-gpio = <&gpb 5 0>;
samsung,spi-feedback-delay = <2>;
};
};
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index a794a70..0c6433a 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -316,6 +316,7 @@
};
spi_1: spi at 12d30000 {
+ cs-gpios = <&gpa2 5 0>;
status = "okay";
w25q80bw at 0 {
@@ -326,7 +327,6 @@
spi-max-frequency = <1000000>;
controller-data {
- cs-gpio = <&gpa2 5 0>;
samsung,spi-feedback-delay = <0>;
};
--
2.0.0.rc2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 0/4] spi: s3c64xx: fix DT binding breakage
2014-07-16 15:19 [PATCH 0/4] spi: s3c64xx: fix DT binding breakage Javier Martinez Canillas
` (3 preceding siblings ...)
2014-07-16 15:19 ` [PATCH 4/4] ARM: DTS: fix the chip select gpios definition in the SPI nodes Javier Martinez Canillas
@ 2014-07-16 15:50 ` Naveen Krishna Ch
2014-07-16 16:02 ` Mark Brown
5 siblings, 0 replies; 15+ messages in thread
From: Naveen Krishna Ch @ 2014-07-16 15:50 UTC (permalink / raw)
To: linux-arm-kernel
Hello Javier,
On 16 July 2014 20:49, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> Hello Mark,
>
> The s3c64xx SPI driver DT binding is currently broken. Commit
> 3146bee ("spi: s3c64xx: Added provision for dedicated cs pin")
> added a new "cs-gpio" property and made it a requirement in
> order to make the driver behave in the same way that it used to.
>
> The motivation of the offending commit was to allow boards that
> want to use the native chip select (instead of a GPIO) to work
> with the s3c64xx SPI driver. Something that was not possible
> before since the driver made it mandatory to specify a GPIO.
>
> Unfortunately the commit also changed the driver defaults since
> now besides specifying a GPIO, it makes mandatory to also specify
> that a GPIO is used while the default used to be the opposite.
>
> That mean that old FDT are not working anymore after 3146bee
> so DT backward compatibility was not ensured by that commit.
>
> This is a follow-up series from a previous one posted by Naveen
> Krishna [0] which attempts to solve the issue.
>
> The feedback from Naveen's series was that the patches did not
> provide a clear explanation about the rationale and goal of both
> the series as a whole and the individual changes [1].
>
> So I tried to take Navee's series and rework them to provide
> an easier to understand patch series.
>
> The series is composed of the following patches:
>
> Javier Martinez Canillas (1):
> Revert "spi: s3c64xx: Added provision for dedicated cs pin"
>
> Naveen Krishna Chatradhi (3):
> spi: s3c64xx: use the generic SPI "cs-gpios" property
> spi: samsung: Update binding documentation
> ARM: DTS: fix the chip select gpios definition in the SPI nodes
I should have mentioned what was the issue being fixed. Your
explanation is very clear. I'm working on getting the terminology correct.
Thank you.
>
> .../devicetree/bindings/spi/spi-samsung.txt | 8 ++--
> arch/arm/boot/dts/exynos4210-smdkv310.dts | 2 +-
> arch/arm/boot/dts/exynos4412-trats2.dts | 2 +-
> arch/arm/boot/dts/exynos5250-smdk5250.dts | 2 +-
> drivers/spi/spi-s3c64xx.c | 54 ++++++++++------------
> 5 files changed, 30 insertions(+), 38 deletions(-)
>
> Patch 01/04 reverts the offending commit since it not only broke
> the DT binding but also introduced a confusing "cs-gpio" property
> while the driver already used a property with the same name.
>
> Patch 02/04 fixes the DT binding for good by using the SPI core
> "cs-gpios" property to specify the chip select GPIO instead of
> using a custom property only used by this driver. This change
> breaks backward compatibility but this has been broken for more
> than a year and nobody complained so I think it's safe to change
> it again in favor of using standard DT binding properties.
>
> A benefit of Patch 02/04 is that it allows DT and legacy boards
> that need to use the built-in CS instead of a GPIO to work with
> the driver which was the original motivation of the broken commit.
>
> Patch 03/04 updates the driver binding documentation accordingly
> and patch 04/04 updates all the DTS board files in mainline.
>
> Best regards,
> Javier
>
> [0]: http://www.spinics.net/lists/linux-samsung-soc/msg34163.html
> [1]: http://www.spinics.net/lists/linux-samsung-soc/msg34309.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Shine bright,
(: Nav :)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/4] spi: s3c64xx: fix DT binding breakage
2014-07-16 15:19 [PATCH 0/4] spi: s3c64xx: fix DT binding breakage Javier Martinez Canillas
` (4 preceding siblings ...)
2014-07-16 15:50 ` [PATCH 0/4] spi: s3c64xx: fix DT binding breakage Naveen Krishna Ch
@ 2014-07-16 16:02 ` Mark Brown
2014-07-16 16:33 ` Javier Martinez Canillas
2014-07-16 16:47 ` Naveen Krishna Ch
5 siblings, 2 replies; 15+ messages in thread
From: Mark Brown @ 2014-07-16 16:02 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 16, 2014 at 05:19:06PM +0200, Javier Martinez Canillas wrote:
> The feedback from Naveen's series was that the patches did not
> provide a clear explanation about the rationale and goal of both
> the series as a whole and the individual changes [1].
>
> So I tried to take Navee's series and rework them to provide
> an easier to understand patch series.
*sigh* Yesterday Naveen submitted another, different, patch series also
attempting to improve things which I'm not seeing any reference to here.
Please coordinate with Naveen about what you are doing.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140716/41cca3c9/attachment.sig>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/4] spi: s3c64xx: fix DT binding breakage
2014-07-16 16:02 ` Mark Brown
@ 2014-07-16 16:33 ` Javier Martinez Canillas
2014-07-16 16:47 ` Naveen Krishna Ch
1 sibling, 0 replies; 15+ messages in thread
From: Javier Martinez Canillas @ 2014-07-16 16:33 UTC (permalink / raw)
To: linux-arm-kernel
On 07/16/2014 06:02 PM, Mark Brown wrote:
> On Wed, Jul 16, 2014 at 05:19:06PM +0200, Javier Martinez Canillas wrote:
>
> *sigh* Yesterday Naveen submitted another, different, patch series also
> attempting to improve things which I'm not seeing any reference to here.
> Please coordinate with Naveen about what you are doing.
>
Yes, I answered Naveen on his latest series and told him that I thought his v6
was better and also that the commit messages where still not clearly explaining
the motivation for the changes [0].
So after that I asked him in private if he wouldn't mind if I try to rework his
series to make it easier to understand and he agreed on that. I should had
mentioned it the cover later, sorry about that.
Please take a look to this series instead of the latest that was posted by Naveen.
Thanks a lot and best regards,
Javier
[0]: https://www.mail-archive.com/devicetree at vger.kernel.org/msg34860.html
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/4] spi: s3c64xx: fix DT binding breakage
2014-07-16 16:02 ` Mark Brown
2014-07-16 16:33 ` Javier Martinez Canillas
@ 2014-07-16 16:47 ` Naveen Krishna Ch
1 sibling, 0 replies; 15+ messages in thread
From: Naveen Krishna Ch @ 2014-07-16 16:47 UTC (permalink / raw)
To: linux-arm-kernel
Hello Mark,
On 16 July 2014 21:32, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Jul 16, 2014 at 05:19:06PM +0200, Javier Martinez Canillas wrote:
>
>> The feedback from Naveen's series was that the patches did not
>> provide a clear explanation about the rationale and goal of both
>> the series as a whole and the individual changes [1].
>>
>> So I tried to take Navee's series and rework them to provide
>> an easier to understand patch series.
>
> *sigh* Yesterday Naveen submitted another, different, patch series also
> attempting to improve things which I'm not seeing any reference to here.
> Please coordinate with Naveen about what you are doing.
Tomasz and Javier felt the v6 version is better than the new version i
submitted yesterday. Javier offered me to help in preparing a cleaner
version.
--
Regards,
(: Naveen :)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/4] Revert "spi: s3c64xx: Added provision for dedicated cs pin"
2014-07-16 15:19 ` [PATCH 1/4] Revert "spi: s3c64xx: Added provision for dedicated cs pin" Javier Martinez Canillas
@ 2014-07-17 18:46 ` Mark Brown
2014-07-17 19:33 ` Javier Martinez Canillas
0 siblings, 1 reply; 15+ messages in thread
From: Mark Brown @ 2014-07-17 18:46 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 16, 2014 at 05:19:07PM +0200, Javier Martinez Canillas wrote:
> This reverts commit 3146beec21b64f4551fcf0ac148381d54dc41b1b.
For the benefit of those who haven't memorized the SHA1s of every commit
that's "spi: s3c64xx: Added provision for dedicated cs pin" - please
include the human readable format whenever you reference a SHA1.
I've applied this.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140717/901bfe82/attachment.sig>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/4] spi: s3c64xx: use the generic SPI "cs-gpios" property
2014-07-16 15:19 ` [PATCH 2/4] spi: s3c64xx: use the generic SPI "cs-gpios" property Javier Martinez Canillas
@ 2014-07-17 18:48 ` Mark Brown
0 siblings, 0 replies; 15+ messages in thread
From: Mark Brown @ 2014-07-17 18:48 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 16, 2014 at 05:19:08PM +0200, Javier Martinez Canillas wrote:
> From: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
>
> The s3c64xx SPI driver uses a custom DT binding to specify
> the GPIO used to drive the chip select (CS) line instead of
> using the generic "cs-gpios" property already defined in:
> Documentation/devicetree/bindings/spi/spi-bus.txt.
Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140717/417cc2a7/attachment-0001.sig>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/4] spi: samsung: Update binding documentation
2014-07-16 15:19 ` [PATCH 3/4] spi: samsung: Update binding documentation Javier Martinez Canillas
@ 2014-07-17 18:48 ` Mark Brown
0 siblings, 0 replies; 15+ messages in thread
From: Mark Brown @ 2014-07-17 18:48 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 16, 2014 at 05:19:09PM +0200, Javier Martinez Canillas wrote:
> From: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
>
> Samsung SPI driver now uses the generic SPI "cs-gpios"
> binding so update the documentation accordingly.
Applied, thanks. Please do try to use changelogs that are consistent
with the general style, or at least consistent within a patch series.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140717/73ce064e/attachment.sig>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/4] ARM: DTS: fix the chip select gpios definition in the SPI nodes
2014-07-16 15:19 ` [PATCH 4/4] ARM: DTS: fix the chip select gpios definition in the SPI nodes Javier Martinez Canillas
@ 2014-07-17 18:49 ` Mark Brown
2014-07-18 19:16 ` Kukjin Kim
0 siblings, 1 reply; 15+ messages in thread
From: Mark Brown @ 2014-07-17 18:49 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 16, 2014 at 05:19:10PM +0200, Javier Martinez Canillas wrote:
> From: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
>
> This patch replaces the "cs-gpio" from "controller-data" node
> as was specified in the old binding and uses the standard
> "cs-gpios" property expected by the SPI core as is defined now
> in the spi-s3c64xx driver DT binding.
I've applied this one too since everything here really ought to go in
together and we should probably try to get this into v3.16 - Kukjin,
please say if this is an issue and I can revert.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140717/b479e79f/attachment.sig>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/4] Revert "spi: s3c64xx: Added provision for dedicated cs pin"
2014-07-17 18:46 ` Mark Brown
@ 2014-07-17 19:33 ` Javier Martinez Canillas
0 siblings, 0 replies; 15+ messages in thread
From: Javier Martinez Canillas @ 2014-07-17 19:33 UTC (permalink / raw)
To: linux-arm-kernel
Hello Mark,
On 07/17/2014 08:46 PM, Mark Brown wrote:
> On Wed, Jul 16, 2014 at 05:19:07PM +0200, Javier Martinez Canillas wrote:
>> This reverts commit 3146beec21b64f4551fcf0ac148381d54dc41b1b.
>
> For the benefit of those who haven't memorized the SHA1s of every commit
> that's "spi: s3c64xx: Added provision for dedicated cs pin" - please
> include the human readable format whenever you reference a SHA1.
>
Ok, I'll take into account for the next time.
> I've applied this.
>
Thanks a lot for your help and sorry for all the inconveniences that this series
caused to you.
Best regards,
Javier
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/4] ARM: DTS: fix the chip select gpios definition in the SPI nodes
2014-07-17 18:49 ` Mark Brown
@ 2014-07-18 19:16 ` Kukjin Kim
0 siblings, 0 replies; 15+ messages in thread
From: Kukjin Kim @ 2014-07-18 19:16 UTC (permalink / raw)
To: linux-arm-kernel
On 07/18/14 03:49, Mark Brown wrote:
> On Wed, Jul 16, 2014 at 05:19:10PM +0200, Javier Martinez Canillas wrote:
>> From: Naveen Krishna Chatradhi<ch.naveen@samsung.com>
>>
>> This patch replaces the "cs-gpio" from "controller-data" node
>> as was specified in the old binding and uses the standard
>> "cs-gpios" property expected by the SPI core as is defined now
>> in the spi-s3c64xx driver DT binding.
>
> I've applied this one too since everything here really ought to go in
> together and we should probably try to get this into v3.16 - Kukjin,
> please say if this is an issue and I can revert.
>
It should be fine to me :)
Thanks for your asking.
- Kukjin
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2014-07-18 19:16 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-16 15:19 [PATCH 0/4] spi: s3c64xx: fix DT binding breakage Javier Martinez Canillas
2014-07-16 15:19 ` [PATCH 1/4] Revert "spi: s3c64xx: Added provision for dedicated cs pin" Javier Martinez Canillas
2014-07-17 18:46 ` Mark Brown
2014-07-17 19:33 ` Javier Martinez Canillas
2014-07-16 15:19 ` [PATCH 2/4] spi: s3c64xx: use the generic SPI "cs-gpios" property Javier Martinez Canillas
2014-07-17 18:48 ` Mark Brown
2014-07-16 15:19 ` [PATCH 3/4] spi: samsung: Update binding documentation Javier Martinez Canillas
2014-07-17 18:48 ` Mark Brown
2014-07-16 15:19 ` [PATCH 4/4] ARM: DTS: fix the chip select gpios definition in the SPI nodes Javier Martinez Canillas
2014-07-17 18:49 ` Mark Brown
2014-07-18 19:16 ` Kukjin Kim
2014-07-16 15:50 ` [PATCH 0/4] spi: s3c64xx: fix DT binding breakage Naveen Krishna Ch
2014-07-16 16:02 ` Mark Brown
2014-07-16 16:33 ` Javier Martinez Canillas
2014-07-16 16:47 ` Naveen Krishna Ch
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).