2024-05-14 11:07:41

by Prajna Rajendra Kumar

[permalink] [raw]
Subject: [PATCH v2 0/3] Add support for GPIO based CS

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


2024-05-14 11:08:06

by Prajna Rajendra Kumar

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

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 <[email protected]>
---
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


2024-05-14 11:10:09

by Prajna Rajendra Kumar

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

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 <[email protected]>
---
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


2024-05-14 11:12:22

by Prajna Rajendra Kumar

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

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 <[email protected]>
---
.../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 <[email protected]>

-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


2024-05-14 17:52:54

by Conor Dooley

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

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 <[email protected]>
> ---
> .../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 <[email protected]>

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 <[email protected]>

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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv


Attachments:
(No filename) (2.47 kB)
signature.asc (235.00 B)
Download all attachments

2024-05-14 17:56:19

by Conor Dooley

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

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 <[email protected]>

I gave you a reviewed-by on v1, here it is again:
Reviewed-by: Conor Dooley <[email protected]>

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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv


Attachments:
(No filename) (1.47 kB)
signature.asc (235.00 B)
Download all attachments

2024-05-14 17:57:17

by Conor Dooley

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

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 <[email protected]>

I provided an ack on v1, so here it is again:
Acked-by: Conor Dooley <[email protected]>

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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv


Attachments:
(No filename) (1.93 kB)
signature.asc (235.00 B)
Download all attachments