[PATCH 1/1] spi: lpspi: Add support for 'num-cs' property

[PATCH 1/1] spi: lpspi: Add support for 'num-cs' property

[Alexander Stein]

This adds support for multiple hardware chip selects. They are controlled
in register TCR bits 24, and possibly more depending on hardware platform.
The driver already supports multiple chip selects in fsl_lpspi_set_cmd(),
so allow setting more than the default 1 for supported chip selects in DT.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
This has been verified using a logic analyzer on a custom board on a
i.MX8XQP with 2 chip selects connected.

 drivers/spi/spi-fsl-lpspi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
index 4c601294f8fa..532bdfb523ea 100644
--- a/drivers/spi/spi-fsl-lpspi.c
+++ b/drivers/spi/spi-fsl-lpspi.c
@@ -819,6 +819,7 @@ static int fsl_lpspi_probe(struct platform_device *pdev)
 	struct spi_controller *controller;
 	struct resource *res;
 	int ret, irq;
+	u32 num_cs;
 	u32 temp;
 	bool is_slave;
 
@@ -835,6 +836,11 @@ static int fsl_lpspi_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, controller);
 
+	if (!device_property_read_u32(&pdev->dev, "num-cs", &num_cs))
+		controller->num_chipselect = num_cs;
+	else
+		controller->num_chipselect = 1;
+
 	fsl_lpspi = spi_controller_get_devdata(controller);
 	fsl_lpspi->dev = &pdev->dev;
 	fsl_lpspi->is_slave = is_slave;
-- 
2.25.1

Re: [PATCH 1/1] spi: lpspi: Add support for 'num-cs' property

[Mark Brown]

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

On Tue, Dec 07, 2021 at 11:41:14AM +0100, Alexander Stein wrote:

> +	if (!device_property_read_u32(&pdev->dev, "num-cs", &num_cs))
> +		controller->num_chipselect = num_cs;
> +	else
> +		controller->num_chipselect = 1;

Do we need to use the num_cs property or can we just set num_chipselect
to whatever the maximum value that can be set is?  I've never been 100%
clear on why num-cs is in the binding.

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

Re: (EXT) Re: [PATCH 1/1] spi: lpspi: Add support for 'num-cs' property

[Alexander Stein]

Am Dienstag, dem 07.12.2021 um 12:57 +0000 schrieb Mark Brown:
> * PGP Signed by an unknown key
> 
> On Tue, Dec 07, 2021 at 11:41:14AM +0100, Alexander Stein wrote:
> 
> > +	if (!device_property_read_u32(&pdev->dev, "num-cs", &num_cs))
> > +		controller->num_chipselect = num_cs;
> > +	else
> > +		controller->num_chipselect = 1;
> 
> Do we need to use the num_cs property or can we just set num_chipselect
> to whatever the maximum value that can be set is?  I've never been 100%
> clear on why num-cs is in the binding.

I see two things which needs to be considered when setting
num_chipselect:

1.
The hardware limitation in the uC. The i.MX8XQP RM says:
>  The entire CS field is not fully supported in every LPSPI module
> instance. Refer to the chip-specific information for LPSPI.

This indicates there are some i.MX, not necessarily i.MX8 only, which
have more than 2 hardware chip selects. This might be set depending on
the compatible.

2.
The hardware limitation on the SOM and/or mainboard. E.g. even if the
uC supports 2 chip selects, only one may be available on the board
connector. This is something which can only be set in the DT.
This case is what this patch is about: Providing 2 hardware chip
selects, as the default (if unset) is just one.

Best regards,
Alexander

Re: (EXT) Re: [PATCH 1/1] spi: lpspi: Add support for 'num-cs' property

[Mark Brown]

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

On Wed, Dec 08, 2021 at 07:59:00AM +0100, Alexander Stein wrote:
> Am Dienstag, dem 07.12.2021 um 12:57 +0000 schrieb Mark Brown:

> > Do we need to use the num_cs property or can we just set num_chipselect
> > to whatever the maximum value that can be set is?  I've never been 100%
> > clear on why num-cs is in the binding.

> I see two things which needs to be considered when setting
> num_chipselect:

> 1.
> The hardware limitation in the uC. The i.MX8XQP RM says:
> >  The entire CS field is not fully supported in every LPSPI module
> > instance. Refer to the chip-specific information for LPSPI.

> This indicates there are some i.MX, not necessarily i.MX8 only, which
> have more than 2 hardware chip selects. This might be set depending on
> the compatible.

Yes, this sounds like it should be figured out from the compatible and
not need a separate property - that's usually better in general if
there's quirks in the IP configuration in different SoCs.

> 2.
> The hardware limitation on the SOM and/or mainboard. E.g. even if the
> uC supports 2 chip selects, only one may be available on the board
> connector. This is something which can only be set in the DT.
> This case is what this patch is about: Providing 2 hardware chip
> selects, as the default (if unset) is just one.

Given that we won't try to enable a chip select unless there's something
connected to it this shouldn't be an issue - with a lot of designs you
can't avoid doing that anyway since the chip selects are all in a single
register.  If the pin isn't connected (or the signal not pinmuxed out)
that doesn't seem like a problem?

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