| Message ID | 20211207104114.2720764-1-alexander.stein@ew.tq-group.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] spi: lpspi: Add support for 'num-cs' property | expand |
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.
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
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?
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;
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(+)