linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] dt-bindings: fpga: xilinx-slave-serial: valid for the 7 Series too
@ 2020-06-11 21:11 Luca Ceresoli
  2020-06-11 21:11 ` [PATCH 2/5] fpga manager: xilinx-spi: " Luca Ceresoli
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Luca Ceresoli @ 2020-06-11 21:11 UTC (permalink / raw)
  To: linux-fpga
  Cc: Luca Ceresoli, Moritz Fischer, Rob Herring, Michal Simek,
	devicetree, linux-arm-kernel, linux-kernel, Anatolij Gustschin

The Xilinx 7-series uses the same protocol, mention that.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 .../devicetree/bindings/fpga/xilinx-slave-serial.txt     | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt b/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
index cfa4ed42b62f..9f103f3872e8 100644
--- a/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
+++ b/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
@@ -1,11 +1,14 @@
 Xilinx Slave Serial SPI FPGA Manager
 
-Xilinx Spartan-6 FPGAs support a method of loading the bitstream over
-what is referred to as "slave serial" interface.
+Xilinx Spartan-6 and 7 Series FPGAs support a method of loading the
+bitstream over what is referred to as "slave serial" interface.
 The slave serial link is not technically SPI, and might require extra
 circuits in order to play nicely with other SPI slaves on the same bus.
 
-See https://www.xilinx.com/support/documentation/user_guides/ug380.pdf
+See:
+- https://www.xilinx.com/support/documentation/user_guides/ug380.pdf
+- https://www.xilinx.com/support/documentation/user_guides/ug470_7Series_Config.pdf
+- https://www.xilinx.com/support/documentation/application_notes/xapp583-fpga-configuration.pdf
 
 Required properties:
 - compatible: should contain "xlnx,fpga-slave-serial"
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/5] fpga manager: xilinx-spi: valid for the 7 Series too
  2020-06-11 21:11 [PATCH 1/5] dt-bindings: fpga: xilinx-slave-serial: valid for the 7 Series too Luca Ceresoli
@ 2020-06-11 21:11 ` Luca Ceresoli
  2020-06-19  1:38   ` Moritz Fischer
  2020-06-11 21:11 ` [PATCH 3/5] fpga manager: xilinx-spi: remove unneeded, mistyped variables Luca Ceresoli
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Luca Ceresoli @ 2020-06-11 21:11 UTC (permalink / raw)
  To: linux-fpga
  Cc: Luca Ceresoli, Moritz Fischer, Rob Herring, Michal Simek,
	devicetree, linux-arm-kernel, linux-kernel, Anatolij Gustschin

The Xilinx 7-series uses the same protocol, mention that.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 drivers/fpga/xilinx-spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
index 272ee0c22822..79106626c3f8 100644
--- a/drivers/fpga/xilinx-spi.c
+++ b/drivers/fpga/xilinx-spi.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Xilinx Spartan6 Slave Serial SPI Driver
+ * Xilinx Spartan6 and 7 Series Slave Serial SPI Driver
  *
  * Copyright (C) 2017 DENX Software Engineering
  *
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/5] fpga manager: xilinx-spi: remove unneeded, mistyped variables
  2020-06-11 21:11 [PATCH 1/5] dt-bindings: fpga: xilinx-slave-serial: valid for the 7 Series too Luca Ceresoli
  2020-06-11 21:11 ` [PATCH 2/5] fpga manager: xilinx-spi: " Luca Ceresoli
@ 2020-06-11 21:11 ` Luca Ceresoli
  2020-06-19  1:38   ` Moritz Fischer
  2020-06-11 21:11 ` [PATCH 4/5] dt-bindings: fpga: xilinx-slave-serial: add optional INIT_B GPIO Luca Ceresoli
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Luca Ceresoli @ 2020-06-11 21:11 UTC (permalink / raw)
  To: linux-fpga
  Cc: Luca Ceresoli, Moritz Fischer, Rob Herring, Michal Simek,
	devicetree, linux-arm-kernel, linux-kernel, Anatolij Gustschin

Using variables does not add readability here: parameters passed
to udelay*() are obviously in microseconds and their meaning is clear
from the context.

The type is also wrong, udelay expects an unsigned long.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 drivers/fpga/xilinx-spi.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
index 79106626c3f8..799ae04301be 100644
--- a/drivers/fpga/xilinx-spi.c
+++ b/drivers/fpga/xilinx-spi.c
@@ -41,8 +41,6 @@ static int xilinx_spi_write_init(struct fpga_manager *mgr,
 				 const char *buf, size_t count)
 {
 	struct xilinx_spi_conf *conf = mgr->priv;
-	const size_t prog_latency_7500us = 7500;
-	const size_t prog_pulse_1us = 1;
 
 	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
 		dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
@@ -51,7 +49,7 @@ static int xilinx_spi_write_init(struct fpga_manager *mgr,
 
 	gpiod_set_value(conf->prog_b, 1);
 
-	udelay(prog_pulse_1us); /* min is 500 ns */
+	udelay(1); /* min is 500 ns */
 
 	gpiod_set_value(conf->prog_b, 0);
 
@@ -61,7 +59,7 @@ static int xilinx_spi_write_init(struct fpga_manager *mgr,
 	}
 
 	/* program latency */
-	usleep_range(prog_latency_7500us, prog_latency_7500us + 100);
+	usleep_range(7500, 7600);
 	return 0;
 }
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/5] dt-bindings: fpga: xilinx-slave-serial: add optional INIT_B GPIO
  2020-06-11 21:11 [PATCH 1/5] dt-bindings: fpga: xilinx-slave-serial: valid for the 7 Series too Luca Ceresoli
  2020-06-11 21:11 ` [PATCH 2/5] fpga manager: xilinx-spi: " Luca Ceresoli
  2020-06-11 21:11 ` [PATCH 3/5] fpga manager: xilinx-spi: remove unneeded, mistyped variables Luca Ceresoli
@ 2020-06-11 21:11 ` Luca Ceresoli
  2020-06-17 22:39   ` Rob Herring
  2020-06-11 21:11 ` [PATCH 5/5] fpga manager: xilinx-spi: check INIT_B pin during write_init Luca Ceresoli
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Luca Ceresoli @ 2020-06-11 21:11 UTC (permalink / raw)
  To: linux-fpga
  Cc: Luca Ceresoli, Moritz Fischer, Rob Herring, Michal Simek,
	devicetree, linux-arm-kernel, linux-kernel, Anatolij Gustschin

The INIT_B is used by the 6 and 7 series to report the programming status,
providing more control and information about programming errors.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 .../devicetree/bindings/fpga/xilinx-slave-serial.txt       | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt b/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
index 9f103f3872e8..a049082e1513 100644
--- a/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
+++ b/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
@@ -16,6 +16,10 @@ Required properties:
 - prog_b-gpios: config pin (referred to as PROGRAM_B in the manual)
 - done-gpios: config status pin (referred to as DONE in the manual)
 
+Optional properties:
+- init_b-gpios: initialization status and configuration error pin
+                (referred to as INIT_B in the manual)
+
 Example for full FPGA configuration:
 
 	fpga-region0 {
@@ -40,7 +44,8 @@ Example for full FPGA configuration:
 			spi-max-frequency = <60000000>;
 			spi-cpha;
 			reg = <0>;
-			done-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
 			prog_b-gpios = <&gpio0 29 GPIO_ACTIVE_LOW>;
+			init_b-gpios = <&gpio0 28 GPIO_ACTIVE_LOW>;
+			done-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
 		};
 	};
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/5] fpga manager: xilinx-spi: check INIT_B pin during write_init
  2020-06-11 21:11 [PATCH 1/5] dt-bindings: fpga: xilinx-slave-serial: valid for the 7 Series too Luca Ceresoli
                   ` (2 preceding siblings ...)
  2020-06-11 21:11 ` [PATCH 4/5] dt-bindings: fpga: xilinx-slave-serial: add optional INIT_B GPIO Luca Ceresoli
@ 2020-06-11 21:11 ` Luca Ceresoli
  2020-06-16  4:43   ` Moritz Fischer
  2020-06-16  4:42 ` [PATCH 1/5] dt-bindings: fpga: xilinx-slave-serial: valid for the 7 Series too Moritz Fischer
  2020-06-17 22:38 ` Rob Herring
  5 siblings, 1 reply; 14+ messages in thread
From: Luca Ceresoli @ 2020-06-11 21:11 UTC (permalink / raw)
  To: linux-fpga
  Cc: Luca Ceresoli, Moritz Fischer, Rob Herring, Michal Simek,
	devicetree, linux-arm-kernel, linux-kernel, Anatolij Gustschin

The INIT_B reports the status during startup and after the end of the
programming process. However the current driver completely ignores it.

Check the pin status during startup to make sure programming is never
started too early and also to detect any hardware issues in the FPGA
connection.

This is optional for backward compatibility. If INIT_B is not passed by
device tree, just fallback to the old udelays.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 drivers/fpga/xilinx-spi.c | 54 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
index 799ae04301be..2710a15ed16b 100644
--- a/drivers/fpga/xilinx-spi.c
+++ b/drivers/fpga/xilinx-spi.c
@@ -23,6 +23,7 @@
 struct xilinx_spi_conf {
 	struct spi_device *spi;
 	struct gpio_desc *prog_b;
+	struct gpio_desc *init_b;
 	struct gpio_desc *done;
 };
 
@@ -36,11 +37,44 @@ static enum fpga_mgr_states xilinx_spi_state(struct fpga_manager *mgr)
 	return FPGA_MGR_STATE_UNKNOWN;
 }
 
+/**
+ * wait_for_init_b - wait for the INIT_B pin to have a given state, or wait
+ * a given delay if the pin is unavailable
+ *
+ * @mgr        The FPGA manager object
+ * @value      Value INIT_B to wait for (1 = asserted = low)
+ * @act_udelay Delay to wait if the INIT_B pin is not available
+ *
+ * Returns 0 when the pin reached the given state or -ETIMEDOUT if too much
+ * time passed waiting for that. If there is no INIT_B, always return 0.
+ */
+static int wait_for_init_b(struct fpga_manager *mgr, int value,
+			   unsigned long backup_udelay)
+{
+	struct xilinx_spi_conf *conf = mgr->priv;
+	unsigned long timeout = jiffies + msecs_to_jiffies(1000);
+
+	if (conf->init_b) {
+		while (time_before(jiffies, timeout)) {
+			/* dump_state(conf, "wait for init_d .."); */
+			if (gpiod_get_value(conf->init_b) == value)
+				return 0;
+			usleep_range(100, 400);
+		}
+		return -ETIMEDOUT;
+	}
+
+	udelay(backup_udelay);
+
+	return 0;
+}
+
 static int xilinx_spi_write_init(struct fpga_manager *mgr,
 				 struct fpga_image_info *info,
 				 const char *buf, size_t count)
 {
 	struct xilinx_spi_conf *conf = mgr->priv;
+	int err;
 
 	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
 		dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
@@ -49,10 +83,21 @@ static int xilinx_spi_write_init(struct fpga_manager *mgr,
 
 	gpiod_set_value(conf->prog_b, 1);
 
-	udelay(1); /* min is 500 ns */
+	err = wait_for_init_b(mgr, 1, 1); /* min is 500 ns */
+	if (err) {
+		dev_err(&mgr->dev, "INIT_B pin did not go low\n");
+		gpiod_set_value(conf->prog_b, 0);
+		return err;
+	}
 
 	gpiod_set_value(conf->prog_b, 0);
 
+	err = wait_for_init_b(mgr, 0, 0);
+	if (err) {
+		dev_err(&mgr->dev, "INIT_B pin did not go high\n");
+		return err;
+	}
+
 	if (gpiod_get_value(conf->done)) {
 		dev_err(&mgr->dev, "Unexpected DONE pin state...\n");
 		return -EIO;
@@ -154,6 +199,13 @@ static int xilinx_spi_probe(struct spi_device *spi)
 		return PTR_ERR(conf->prog_b);
 	}
 
+	conf->init_b = devm_gpiod_get_optional(&spi->dev, "init_b", GPIOD_IN);
+	if (IS_ERR(conf->init_b)) {
+		dev_err(&spi->dev, "Failed to get INIT_B gpio: %ld\n",
+			PTR_ERR(conf->init_b));
+		return PTR_ERR(conf->init_b);
+	}
+
 	conf->done = devm_gpiod_get(&spi->dev, "done", GPIOD_IN);
 	if (IS_ERR(conf->done)) {
 		dev_err(&spi->dev, "Failed to get DONE gpio: %ld\n",
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/5] dt-bindings: fpga: xilinx-slave-serial: valid for the 7 Series too
  2020-06-11 21:11 [PATCH 1/5] dt-bindings: fpga: xilinx-slave-serial: valid for the 7 Series too Luca Ceresoli
                   ` (3 preceding siblings ...)
  2020-06-11 21:11 ` [PATCH 5/5] fpga manager: xilinx-spi: check INIT_B pin during write_init Luca Ceresoli
@ 2020-06-16  4:42 ` Moritz Fischer
  2020-06-17 22:38 ` Rob Herring
  5 siblings, 0 replies; 14+ messages in thread
From: Moritz Fischer @ 2020-06-16  4:42 UTC (permalink / raw)
  To: Luca Ceresoli
  Cc: linux-fpga, Moritz Fischer, Rob Herring, Michal Simek,
	devicetree, linux-arm-kernel, linux-kernel, Anatolij Gustschin

On Thu, Jun 11, 2020 at 11:11:40PM +0200, Luca Ceresoli wrote:
> The Xilinx 7-series uses the same protocol, mention that.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Acked-by: Moritz Fischer <mdf@kernel.org>
> ---
>  .../devicetree/bindings/fpga/xilinx-slave-serial.txt     | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt b/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
> index cfa4ed42b62f..9f103f3872e8 100644
> --- a/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
> +++ b/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
> @@ -1,11 +1,14 @@
>  Xilinx Slave Serial SPI FPGA Manager
>  
> -Xilinx Spartan-6 FPGAs support a method of loading the bitstream over
> -what is referred to as "slave serial" interface.
> +Xilinx Spartan-6 and 7 Series FPGAs support a method of loading the
> +bitstream over what is referred to as "slave serial" interface.
>  The slave serial link is not technically SPI, and might require extra
>  circuits in order to play nicely with other SPI slaves on the same bus.
>  
> -See https://www.xilinx.com/support/documentation/user_guides/ug380.pdf
> +See:
> +- https://www.xilinx.com/support/documentation/user_guides/ug380.pdf
> +- https://www.xilinx.com/support/documentation/user_guides/ug470_7Series_Config.pdf
> +- https://www.xilinx.com/support/documentation/application_notes/xapp583-fpga-configuration.pdf
>  
>  Required properties:
>  - compatible: should contain "xlnx,fpga-slave-serial"
> -- 
> 2.27.0
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 5/5] fpga manager: xilinx-spi: check INIT_B pin during write_init
  2020-06-11 21:11 ` [PATCH 5/5] fpga manager: xilinx-spi: check INIT_B pin during write_init Luca Ceresoli
@ 2020-06-16  4:43   ` Moritz Fischer
  0 siblings, 0 replies; 14+ messages in thread
From: Moritz Fischer @ 2020-06-16  4:43 UTC (permalink / raw)
  To: Luca Ceresoli
  Cc: linux-fpga, Moritz Fischer, Rob Herring, Michal Simek,
	devicetree, linux-arm-kernel, linux-kernel, Anatolij Gustschin

Hi Luca,

On Thu, Jun 11, 2020 at 11:11:44PM +0200, Luca Ceresoli wrote:
> The INIT_B reports the status during startup and after the end of the
> programming process. However the current driver completely ignores it.
> 
> Check the pin status during startup to make sure programming is never
> started too early and also to detect any hardware issues in the FPGA
> connection.
> 
> This is optional for backward compatibility. If INIT_B is not passed by
> device tree, just fallback to the old udelays.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>  drivers/fpga/xilinx-spi.c | 54 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
> index 799ae04301be..2710a15ed16b 100644
> --- a/drivers/fpga/xilinx-spi.c
> +++ b/drivers/fpga/xilinx-spi.c
> @@ -23,6 +23,7 @@
>  struct xilinx_spi_conf {
>  	struct spi_device *spi;
>  	struct gpio_desc *prog_b;
> +	struct gpio_desc *init_b;
>  	struct gpio_desc *done;
>  };
>  
> @@ -36,11 +37,44 @@ static enum fpga_mgr_states xilinx_spi_state(struct fpga_manager *mgr)
>  	return FPGA_MGR_STATE_UNKNOWN;
>  }
>  
> +/**
> + * wait_for_init_b - wait for the INIT_B pin to have a given state, or wait
> + * a given delay if the pin is unavailable
> + *
> + * @mgr        The FPGA manager object
> + * @value      Value INIT_B to wait for (1 = asserted = low)
> + * @act_udelay Delay to wait if the INIT_B pin is not available
> + *
> + * Returns 0 when the pin reached the given state or -ETIMEDOUT if too much
> + * time passed waiting for that. If there is no INIT_B, always return 0.
> + */
> +static int wait_for_init_b(struct fpga_manager *mgr, int value,
> +			   unsigned long backup_udelay)
> +{
> +	struct xilinx_spi_conf *conf = mgr->priv;
> +	unsigned long timeout = jiffies + msecs_to_jiffies(1000);
> +
> +	if (conf->init_b) {
> +		while (time_before(jiffies, timeout)) {
> +			/* dump_state(conf, "wait for init_d .."); */
> +			if (gpiod_get_value(conf->init_b) == value)
> +				return 0;
> +			usleep_range(100, 400);
> +		}
> +		return -ETIMEDOUT;
> +	}
> +
> +	udelay(backup_udelay);
> +
> +	return 0;
> +}
> +
>  static int xilinx_spi_write_init(struct fpga_manager *mgr,
>  				 struct fpga_image_info *info,
>  				 const char *buf, size_t count)
>  {
>  	struct xilinx_spi_conf *conf = mgr->priv;
> +	int err;
>  
>  	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
>  		dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
> @@ -49,10 +83,21 @@ static int xilinx_spi_write_init(struct fpga_manager *mgr,
>  
>  	gpiod_set_value(conf->prog_b, 1);
>  
> -	udelay(1); /* min is 500 ns */
> +	err = wait_for_init_b(mgr, 1, 1); /* min is 500 ns */
> +	if (err) {
> +		dev_err(&mgr->dev, "INIT_B pin did not go low\n");
> +		gpiod_set_value(conf->prog_b, 0);
> +		return err;
> +	}
>  
>  	gpiod_set_value(conf->prog_b, 0);
>  
> +	err = wait_for_init_b(mgr, 0, 0);
> +	if (err) {
> +		dev_err(&mgr->dev, "INIT_B pin did not go high\n");
> +		return err;
> +	}
> +
>  	if (gpiod_get_value(conf->done)) {
>  		dev_err(&mgr->dev, "Unexpected DONE pin state...\n");
>  		return -EIO;
> @@ -154,6 +199,13 @@ static int xilinx_spi_probe(struct spi_device *spi)
>  		return PTR_ERR(conf->prog_b);
>  	}
>  
> +	conf->init_b = devm_gpiod_get_optional(&spi->dev, "init_b", GPIOD_IN);
> +	if (IS_ERR(conf->init_b)) {
> +		dev_err(&spi->dev, "Failed to get INIT_B gpio: %ld\n",
> +			PTR_ERR(conf->init_b));
> +		return PTR_ERR(conf->init_b);
> +	}
> +
>  	conf->done = devm_gpiod_get(&spi->dev, "done", GPIOD_IN);
>  	if (IS_ERR(conf->done)) {
>  		dev_err(&spi->dev, "Failed to get DONE gpio: %ld\n",
> -- 
> 2.27.0
> 

Series looks good, will apply to for-next.

Thanks,
Moritz

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/5] dt-bindings: fpga: xilinx-slave-serial: valid for the 7 Series too
  2020-06-11 21:11 [PATCH 1/5] dt-bindings: fpga: xilinx-slave-serial: valid for the 7 Series too Luca Ceresoli
                   ` (4 preceding siblings ...)
  2020-06-16  4:42 ` [PATCH 1/5] dt-bindings: fpga: xilinx-slave-serial: valid for the 7 Series too Moritz Fischer
@ 2020-06-17 22:38 ` Rob Herring
  2020-06-19  1:37   ` Moritz Fischer
  5 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2020-06-17 22:38 UTC (permalink / raw)
  To: Luca Ceresoli
  Cc: linux-fpga, Rob Herring, Anatolij Gustschin, linux-kernel,
	Michal Simek, linux-arm-kernel, Moritz Fischer, devicetree

On Thu, 11 Jun 2020 23:11:40 +0200, Luca Ceresoli wrote:
> The Xilinx 7-series uses the same protocol, mention that.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>  .../devicetree/bindings/fpga/xilinx-slave-serial.txt     | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/5] dt-bindings: fpga: xilinx-slave-serial: add optional INIT_B GPIO
  2020-06-11 21:11 ` [PATCH 4/5] dt-bindings: fpga: xilinx-slave-serial: add optional INIT_B GPIO Luca Ceresoli
@ 2020-06-17 22:39   ` Rob Herring
  2020-06-18  5:47     ` Luca Ceresoli
  0 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2020-06-17 22:39 UTC (permalink / raw)
  To: Luca Ceresoli
  Cc: linux-fpga, Moritz Fischer, Michal Simek, devicetree,
	linux-arm-kernel, linux-kernel, Anatolij Gustschin

On Thu, Jun 11, 2020 at 11:11:43PM +0200, Luca Ceresoli wrote:
> The INIT_B is used by the 6 and 7 series to report the programming status,
> providing more control and information about programming errors.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>  .../devicetree/bindings/fpga/xilinx-slave-serial.txt       | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt b/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
> index 9f103f3872e8..a049082e1513 100644
> --- a/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
> +++ b/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
> @@ -16,6 +16,10 @@ Required properties:
>  - prog_b-gpios: config pin (referred to as PROGRAM_B in the manual)
>  - done-gpios: config status pin (referred to as DONE in the manual)
>  
> +Optional properties:
> +- init_b-gpios: initialization status and configuration error pin
> +                (referred to as INIT_B in the manual)

Don't use '_' in property names:

init-b-gpios

> +
>  Example for full FPGA configuration:
>  
>  	fpga-region0 {
> @@ -40,7 +44,8 @@ Example for full FPGA configuration:
>  			spi-max-frequency = <60000000>;
>  			spi-cpha;
>  			reg = <0>;
> -			done-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
>  			prog_b-gpios = <&gpio0 29 GPIO_ACTIVE_LOW>;
> +			init_b-gpios = <&gpio0 28 GPIO_ACTIVE_LOW>;
> +			done-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
>  		};
>  	};
> -- 
> 2.27.0
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/5] dt-bindings: fpga: xilinx-slave-serial: add optional INIT_B GPIO
  2020-06-17 22:39   ` Rob Herring
@ 2020-06-18  5:47     ` Luca Ceresoli
  2020-06-18 18:07       ` Rob Herring
  0 siblings, 1 reply; 14+ messages in thread
From: Luca Ceresoli @ 2020-06-18  5:47 UTC (permalink / raw)
  To: Rob Herring, Moritz Fischer
  Cc: linux-fpga, Michal Simek, devicetree, linux-arm-kernel,
	linux-kernel, Anatolij Gustschin

Hi Rob, Moritz,

On 18/06/20 00:39, Rob Herring wrote:
> On Thu, Jun 11, 2020 at 11:11:43PM +0200, Luca Ceresoli wrote:
>> The INIT_B is used by the 6 and 7 series to report the programming status,
>> providing more control and information about programming errors.
>>
>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>> ---
>>  .../devicetree/bindings/fpga/xilinx-slave-serial.txt       | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt b/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
>> index 9f103f3872e8..a049082e1513 100644
>> --- a/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
>> +++ b/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
>> @@ -16,6 +16,10 @@ Required properties:
>>  - prog_b-gpios: config pin (referred to as PROGRAM_B in the manual)
>>  - done-gpios: config status pin (referred to as DONE in the manual)
>>  
>> +Optional properties:
>> +- init_b-gpios: initialization status and configuration error pin
>> +                (referred to as INIT_B in the manual)
> 
> Don't use '_' in property names:
> 
> init-b-gpios

OK, will fix.

Moritz, please don't apply this version of patches 4 and 5 if you still
haven't done so.

Now what about the existing prog_b-gpios property? Should we just leave
it as is for backward compatibility, or is there a migration path to fix
it as well?

Thanks.
-- 
Luca

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/5] dt-bindings: fpga: xilinx-slave-serial: add optional INIT_B GPIO
  2020-06-18  5:47     ` Luca Ceresoli
@ 2020-06-18 18:07       ` Rob Herring
  0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2020-06-18 18:07 UTC (permalink / raw)
  To: Luca Ceresoli
  Cc: Moritz Fischer, linux-fpga, Michal Simek, devicetree,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel, Anatolij Gustschin

On Wed, Jun 17, 2020 at 11:47 PM Luca Ceresoli <luca@lucaceresoli.net> wrote:
>
> Hi Rob, Moritz,
>
> On 18/06/20 00:39, Rob Herring wrote:
> > On Thu, Jun 11, 2020 at 11:11:43PM +0200, Luca Ceresoli wrote:
> >> The INIT_B is used by the 6 and 7 series to report the programming status,
> >> providing more control and information about programming errors.
> >>
> >> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> >> ---
> >>  .../devicetree/bindings/fpga/xilinx-slave-serial.txt       | 7 ++++++-
> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt b/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
> >> index 9f103f3872e8..a049082e1513 100644
> >> --- a/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
> >> +++ b/Documentation/devicetree/bindings/fpga/xilinx-slave-serial.txt
> >> @@ -16,6 +16,10 @@ Required properties:
> >>  - prog_b-gpios: config pin (referred to as PROGRAM_B in the manual)
> >>  - done-gpios: config status pin (referred to as DONE in the manual)
> >>
> >> +Optional properties:
> >> +- init_b-gpios: initialization status and configuration error pin
> >> +                (referred to as INIT_B in the manual)
> >
> > Don't use '_' in property names:
> >
> > init-b-gpios
>
> OK, will fix.
>
> Moritz, please don't apply this version of patches 4 and 5 if you still
> haven't done so.
>
> Now what about the existing prog_b-gpios property? Should we just leave
> it as is for backward compatibility, or is there a migration path to fix
> it as well?

Just leave it.

Rob

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/5] dt-bindings: fpga: xilinx-slave-serial: valid for the 7 Series too
  2020-06-17 22:38 ` Rob Herring
@ 2020-06-19  1:37   ` Moritz Fischer
  0 siblings, 0 replies; 14+ messages in thread
From: Moritz Fischer @ 2020-06-19  1:37 UTC (permalink / raw)
  To: Rob Herring
  Cc: Luca Ceresoli, linux-fpga, Rob Herring, Anatolij Gustschin,
	linux-kernel, Michal Simek, linux-arm-kernel, Moritz Fischer,
	devicetree

On Wed, Jun 17, 2020 at 04:38:41PM -0600, Rob Herring wrote:
> On Thu, 11 Jun 2020 23:11:40 +0200, Luca Ceresoli wrote:
> > The Xilinx 7-series uses the same protocol, mention that.
> > 
> > Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> > ---
> >  .../devicetree/bindings/fpga/xilinx-slave-serial.txt     | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> 
> Acked-by: Rob Herring <robh@kernel.org>
Applied to for-next,

Thanks

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/5] fpga manager: xilinx-spi: valid for the 7 Series too
  2020-06-11 21:11 ` [PATCH 2/5] fpga manager: xilinx-spi: " Luca Ceresoli
@ 2020-06-19  1:38   ` Moritz Fischer
  0 siblings, 0 replies; 14+ messages in thread
From: Moritz Fischer @ 2020-06-19  1:38 UTC (permalink / raw)
  To: Luca Ceresoli
  Cc: linux-fpga, Moritz Fischer, Rob Herring, Michal Simek,
	devicetree, linux-arm-kernel, linux-kernel, Anatolij Gustschin

On Thu, Jun 11, 2020 at 11:11:41PM +0200, Luca Ceresoli wrote:
> The Xilinx 7-series uses the same protocol, mention that.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>  drivers/fpga/xilinx-spi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
> index 272ee0c22822..79106626c3f8 100644
> --- a/drivers/fpga/xilinx-spi.c
> +++ b/drivers/fpga/xilinx-spi.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
> - * Xilinx Spartan6 Slave Serial SPI Driver
> + * Xilinx Spartan6 and 7 Series Slave Serial SPI Driver
>   *
>   * Copyright (C) 2017 DENX Software Engineering
>   *
> -- 
> 2.27.0
> 

Applied to for-next,

Thanks

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/5] fpga manager: xilinx-spi: remove unneeded, mistyped variables
  2020-06-11 21:11 ` [PATCH 3/5] fpga manager: xilinx-spi: remove unneeded, mistyped variables Luca Ceresoli
@ 2020-06-19  1:38   ` Moritz Fischer
  0 siblings, 0 replies; 14+ messages in thread
From: Moritz Fischer @ 2020-06-19  1:38 UTC (permalink / raw)
  To: Luca Ceresoli
  Cc: linux-fpga, Moritz Fischer, Rob Herring, Michal Simek,
	devicetree, linux-arm-kernel, linux-kernel, Anatolij Gustschin

On Thu, Jun 11, 2020 at 11:11:42PM +0200, Luca Ceresoli wrote:
> Using variables does not add readability here: parameters passed
> to udelay*() are obviously in microseconds and their meaning is clear
> from the context.
> 
> The type is also wrong, udelay expects an unsigned long.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>  drivers/fpga/xilinx-spi.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
> index 79106626c3f8..799ae04301be 100644
> --- a/drivers/fpga/xilinx-spi.c
> +++ b/drivers/fpga/xilinx-spi.c
> @@ -41,8 +41,6 @@ static int xilinx_spi_write_init(struct fpga_manager *mgr,
>  				 const char *buf, size_t count)
>  {
>  	struct xilinx_spi_conf *conf = mgr->priv;
> -	const size_t prog_latency_7500us = 7500;
> -	const size_t prog_pulse_1us = 1;
>  
>  	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
>  		dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
> @@ -51,7 +49,7 @@ static int xilinx_spi_write_init(struct fpga_manager *mgr,
>  
>  	gpiod_set_value(conf->prog_b, 1);
>  
> -	udelay(prog_pulse_1us); /* min is 500 ns */
> +	udelay(1); /* min is 500 ns */
>  
>  	gpiod_set_value(conf->prog_b, 0);
>  
> @@ -61,7 +59,7 @@ static int xilinx_spi_write_init(struct fpga_manager *mgr,
>  	}
>  
>  	/* program latency */
> -	usleep_range(prog_latency_7500us, prog_latency_7500us + 100);
> +	usleep_range(7500, 7600);
>  	return 0;
>  }
>  
> -- 
> 2.27.0
> 
Applied to for-next,

Thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-06-19  1:38 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-11 21:11 [PATCH 1/5] dt-bindings: fpga: xilinx-slave-serial: valid for the 7 Series too Luca Ceresoli
2020-06-11 21:11 ` [PATCH 2/5] fpga manager: xilinx-spi: " Luca Ceresoli
2020-06-19  1:38   ` Moritz Fischer
2020-06-11 21:11 ` [PATCH 3/5] fpga manager: xilinx-spi: remove unneeded, mistyped variables Luca Ceresoli
2020-06-19  1:38   ` Moritz Fischer
2020-06-11 21:11 ` [PATCH 4/5] dt-bindings: fpga: xilinx-slave-serial: add optional INIT_B GPIO Luca Ceresoli
2020-06-17 22:39   ` Rob Herring
2020-06-18  5:47     ` Luca Ceresoli
2020-06-18 18:07       ` Rob Herring
2020-06-11 21:11 ` [PATCH 5/5] fpga manager: xilinx-spi: check INIT_B pin during write_init Luca Ceresoli
2020-06-16  4:43   ` Moritz Fischer
2020-06-16  4:42 ` [PATCH 1/5] dt-bindings: fpga: xilinx-slave-serial: valid for the 7 Series too Moritz Fischer
2020-06-17 22:38 ` Rob Herring
2020-06-19  1:37   ` Moritz Fischer

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).