[PATCH v2 0/3] Add support for GPIO based CS

[PATCH v2 0/3] Add support for GPIO based CS

[Prajna Rajendra Kumar]

The Microchip PolarFire SoC SPI "hard" controller supports eight 
chip selects. However, only one chip select is physically wired. 
Therefore, use GPIO descriptors to configure additional chip select 
lines.

v1-> v2: 
 - Modified all commit messages for better understanding
 - driver - added spi_is_csgpiod() API to address review comment
 - bindings - fixed bindings to set the default value of num-cs

Prajna Rajendra Kumar (3):
  spi: dt-bindings: Add num-cs property for mpfs-spi
  spi: spi-microchip-core: Fix the number of chip selects supported
  spi: spi-microchip-core: Add support for GPIO based CS

 .../bindings/spi/microchip,mpfs-spi.yaml      | 29 +++++++++++++++++--
 drivers/spi/spi-microchip-core.c              |  6 +++-
 2 files changed, 31 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH v2 1/3] spi: dt-bindings: Add num-cs property for mpfs-spi

[Prajna Rajendra Kumar]

The PolarFire SoC SPI "hard" controller supports eight CS lines, out of
which only one CS line is physically wired. The default value of
'num-cs' was never set and it did not didn't impose a maximum value.

To reflect this hardware limitation in the device tree, the binding
enforces that the 'num-cs' property cannot exceed 1 unless additional
CS lines are explicitly defined using GPIO descriptors.

Fixes: 2da187304e55 ("spi: add bindings for microchip mpfs spi")
Signed-off-by: Prajna Rajendra Kumar <prajna.rajendrakumar@microchip.com>
---
 .../bindings/spi/microchip,mpfs-spi.yaml      | 29 +++++++++++++++++--
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
index 74a817cc7d94..ffa8d1b48f8b 100644
--- a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
+++ b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
@@ -13,9 +13,6 @@ description:
 maintainers:
   - Conor Dooley <conor.dooley@microchip.com>
 
-allOf:
-  - $ref: spi-controller.yaml#
-
 properties:
   compatible:
     oneOf:
@@ -43,6 +40,32 @@ required:
   - interrupts
   - clocks
 
+allOf:
+  - $ref: spi-controller.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: microchip,mpfs-spi
+    then:
+      properties:
+        num-cs:
+          default: 1
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: microchip,mpfs-spi
+      not:
+        required:
+          - cs-gpios
+    then:
+      properties:
+        num-cs:
+          maximum: 1
+
 unevaluatedProperties: false
 
 examples:
-- 
2.25.1

Re: [PATCH v2 1/3] spi: dt-bindings: Add num-cs property for mpfs-spi

[Conor Dooley]

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

On Tue, May 14, 2024 at 11:45:06AM +0100, Prajna Rajendra Kumar wrote:
> The PolarFire SoC SPI "hard" controller supports eight CS lines, out of
> which only one CS line is physically wired. The default value of
> 'num-cs' was never set and it did not didn't impose a maximum value.
> 
> To reflect this hardware limitation in the device tree, the binding
> enforces that the 'num-cs' property cannot exceed 1 unless additional
> CS lines are explicitly defined using GPIO descriptors.
> 
> Fixes: 2da187304e55 ("spi: add bindings for microchip mpfs spi")
> Signed-off-by: Prajna Rajendra Kumar <prajna.rajendrakumar@microchip.com>
> ---
>  .../bindings/spi/microchip,mpfs-spi.yaml      | 29 +++++++++++++++++--
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
> index 74a817cc7d94..ffa8d1b48f8b 100644
> --- a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
> @@ -13,9 +13,6 @@ description:
>  maintainers:
>    - Conor Dooley <conor.dooley@microchip.com>

I provided the conditions below, so it's maybe a little disingenuous for
me to provide a review from a dt-bindings correctness point of view, but
then again I am the one listed as a maintainer for this particular
binding and what's being done here does match the hardware, so:

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.


>  
> -allOf:
> -  - $ref: spi-controller.yaml#
> -
>  properties:
>    compatible:
>      oneOf:
> @@ -43,6 +40,32 @@ required:
>    - interrupts
>    - clocks
>  
> +allOf:
> +  - $ref: spi-controller.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: microchip,mpfs-spi
> +    then:
> +      properties:
> +        num-cs:
> +          default: 1
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: microchip,mpfs-spi
> +      not:
> +        required:
> +          - cs-gpios
> +    then:
> +      properties:
> +        num-cs:
> +          maximum: 1
> +
>  unevaluatedProperties: false
>  
>  examples:
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

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

Re: [PATCH v2 1/3] spi: dt-bindings: Add num-cs property for mpfs-spi

[Conor Dooley]

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

On Tue, May 14, 2024 at 11:45:06AM +0100, Prajna Rajendra Kumar wrote:
> The PolarFire SoC SPI "hard" controller supports eight CS lines, out of
> which only one CS line is physically wired. The default value of
> 'num-cs' was never set and it did not didn't impose a maximum value.
> 
> To reflect this hardware limitation in the device tree, the binding
> enforces that the 'num-cs' property cannot exceed 1 unless additional
> CS lines are explicitly defined using GPIO descriptors.
> 
> Fixes: 2da187304e55 ("spi: add bindings for microchip mpfs spi")
> Signed-off-by: Prajna Rajendra Kumar <prajna.rajendrakumar@microchip.com>
> ---
>  .../bindings/spi/microchip,mpfs-spi.yaml      | 29 +++++++++++++++++--
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
> index 74a817cc7d94..ffa8d1b48f8b 100644
> --- a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
> @@ -13,9 +13,6 @@ description:
>  maintainers:
>    - Conor Dooley <conor.dooley@microchip.com>
>  
> -allOf:
> -  - $ref: spi-controller.yaml#
> -
>  properties:
>    compatible:
>      oneOf:
> @@ -43,6 +40,32 @@ required:
>    - interrupts
>    - clocks
>  
> +allOf:
> +  - $ref: spi-controller.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: microchip,mpfs-spi
> +    then:
> +      properties:
> +        num-cs:
> +          default: 1
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: microchip,mpfs-spi
> +      not:
> +        required:
> +          - cs-gpios
> +    then:
> +      properties:
> +        num-cs:
> +          maximum: 1

So, it turns out that "maximum" here should actually be 2, as there's a
CS routed via the FPGA fabric as well. I think default should become 2
also. I can show you what we missed in the configurator on Monday, or
Cyril can if you see him before then.

Cheers,
Conor.

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

[PATCH v2 2/3] spi: spi-microchip-core: Fix the number of chip selects supported

[Prajna Rajendra Kumar]

The SPI "hard" controller in PolarFire SoC has eight CS lines, but only
one CS line is wired. When the 'num-cs' property is not specified in
the device tree, the driver defaults to the MAX_CS value, which has
been fixed to 1 to match the hardware configuration; however, when the
'num-cs' property is explicitly defined in the device tree, it
overrides the default value.

Fixes: 9ac8d17694b6 ("spi: add support for microchip fpga spi controllers")
Signed-off-by: Prajna Rajendra Kumar <prajna.rajendrakumar@microchip.com>
---
 drivers/spi/spi-microchip-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-microchip-core.c b/drivers/spi/spi-microchip-core.c
index 634364c7cfe6..c10de45aa472 100644
--- a/drivers/spi/spi-microchip-core.c
+++ b/drivers/spi/spi-microchip-core.c
@@ -21,7 +21,7 @@
 #include <linux/spi/spi.h>
 
 #define MAX_LEN				(0xffff)
-#define MAX_CS				(8)
+#define MAX_CS				(1)
 #define DEFAULT_FRAMESIZE		(8)
 #define FIFO_DEPTH			(32)
 #define CLK_GEN_MODE1_MAX		(255)
-- 
2.25.1

Re: [PATCH v2 2/3] spi: spi-microchip-core: Fix the number of chip selects supported

[Conor Dooley]

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

On Tue, May 14, 2024 at 11:45:07AM +0100, Prajna Rajendra Kumar wrote:
> The SPI "hard" controller in PolarFire SoC has eight CS lines, but only
> one CS line is wired. When the 'num-cs' property is not specified in
> the device tree, the driver defaults to the MAX_CS value, which has
> been fixed to 1 to match the hardware configuration; however, when the
> 'num-cs' property is explicitly defined in the device tree, it
> overrides the default value.
> 
> Fixes: 9ac8d17694b6 ("spi: add support for microchip fpga spi controllers")
> Signed-off-by: Prajna Rajendra Kumar <prajna.rajendrakumar@microchip.com>

I gave you a reviewed-by on v1, here it is again:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

> ---
>  drivers/spi/spi-microchip-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-microchip-core.c b/drivers/spi/spi-microchip-core.c
> index 634364c7cfe6..c10de45aa472 100644
> --- a/drivers/spi/spi-microchip-core.c
> +++ b/drivers/spi/spi-microchip-core.c
> @@ -21,7 +21,7 @@
>  #include <linux/spi/spi.h>
>  
>  #define MAX_LEN				(0xffff)
> -#define MAX_CS				(8)
> +#define MAX_CS				(1)
>  #define DEFAULT_FRAMESIZE		(8)
>  #define FIFO_DEPTH			(32)
>  #define CLK_GEN_MODE1_MAX		(255)
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[PATCH v2 3/3] spi: spi-microchip-core: Add support for GPIO based CS

[Prajna Rajendra Kumar]

The SPI "hard" controller within the PolarFire SoC is capable of
handling eight CS lines, but only one CS line is wired. Therefore, use
GPIO descriptors to configure additional CS lines.

Signed-off-by: Prajna Rajendra Kumar <prajna.rajendrakumar@microchip.com>
---
 drivers/spi/spi-microchip-core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/spi/spi-microchip-core.c b/drivers/spi/spi-microchip-core.c
index c10de45aa472..6246254e1dff 100644
--- a/drivers/spi/spi-microchip-core.c
+++ b/drivers/spi/spi-microchip-core.c
@@ -258,6 +258,9 @@ static int mchp_corespi_setup(struct spi_device *spi)
 	struct mchp_corespi *corespi = spi_controller_get_devdata(spi->controller);
 	u32 reg;
 
+	if (spi_is_csgpiod(spi))
+		return 0;
+
 	/*
 	 * Active high targets need to be specifically set to their inactive
 	 * states during probe by adding them to the "control group" & thus
@@ -516,6 +519,7 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 
 	host->num_chipselect = num_cs;
 	host->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
+	host->use_gpio_descriptors = true;
 	host->setup = mchp_corespi_setup;
 	host->bits_per_word_mask = SPI_BPW_MASK(8);
 	host->transfer_one = mchp_corespi_transfer_one;
-- 
2.25.1

Re: [PATCH v2 3/3] spi: spi-microchip-core: Add support for GPIO based CS

[Conor Dooley]

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

Prajna, Mark,

On Tue, May 14, 2024 at 11:45:08AM +0100, Prajna Rajendra Kumar wrote:
> The SPI "hard" controller within the PolarFire SoC is capable of
> handling eight CS lines, but only one CS line is wired. Therefore, use
> GPIO descriptors to configure additional CS lines.
> 
> Signed-off-by: Prajna Rajendra Kumar <prajna.rajendrakumar@microchip.com>

I provided an ack on v1, so here it is again:
Acked-by: Conor Dooley <conor.dooley@microchip.com>

In general you can keep tags between versions, if you intentionally drop
tags you should mention why you dropped them.

> ---
>  drivers/spi/spi-microchip-core.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/spi/spi-microchip-core.c b/drivers/spi/spi-microchip-core.c
> index c10de45aa472..6246254e1dff 100644
> --- a/drivers/spi/spi-microchip-core.c
> +++ b/drivers/spi/spi-microchip-core.c
> @@ -258,6 +258,9 @@ static int mchp_corespi_setup(struct spi_device *spi)
>  	struct mchp_corespi *corespi = spi_controller_get_devdata(spi->controller);
>  	u32 reg;
>  
> +	if (spi_is_csgpiod(spi))
> +		return 0;

Mark,

This has no users outside of core code, but is < 6 months old. Is using
it in a driver like this okay?

Cheers,
Conor.

> +
>  	/*
>  	 * Active high targets need to be specifically set to their inactive
>  	 * states during probe by adding them to the "control group" & thus
> @@ -516,6 +519,7 @@ static int mchp_corespi_probe(struct platform_device *pdev)
>  
>  	host->num_chipselect = num_cs;
>  	host->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
> +	host->use_gpio_descriptors = true;
>  	host->setup = mchp_corespi_setup;
>  	host->bits_per_word_mask = SPI_BPW_MASK(8);
>  	host->transfer_one = mchp_corespi_transfer_one;
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

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