diff mbox series

[V2,6/9] spi: bcm2835aux: warn in dmesg that native cs is not really supported

Message ID 20190324175002.28969-7-kernel@martin.sperl.org (mailing list archive)
State Superseded
Headers show
Series spi: bcm2835aux: bug fixes and improvements | expand

Commit Message

Martin Sperl March 24, 2019, 5:49 p.m. UTC
From: Martin Sperl <kernel@martin.sperl.org>

From personal bad experience (even as the author of the original driver)
it shows that native-cs is "somewhat" supported by the spi bus driver
when using a buggy device tree.

So make sure that the driver is warning in dmesg about this fact
that we are running in a not supported mode that may have surprizing
limitations.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Fixes: 1ea29b39f4c812ece2f936065a0a3d6fe44a263e
---
 drivers/spi/spi-bcm2835aux.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

--
2.11.0

Comments

Stefan Wahren March 25, 2019, 10:28 a.m. UTC | #1
Hi Martin,

Am 24.03.19 um 18:49 schrieb kernel@martin.sperl.org:
> From: Martin Sperl <kernel@martin.sperl.org>
>
> From personal bad experience (even as the author of the original driver)
> it shows that native-cs is "somewhat" supported by the spi bus driver
> when using a buggy device tree.
>
> So make sure that the driver is warning in dmesg about this fact
> that we are running in a not supported mode that may have surprizing
> limitations.
>
> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> Fixes: 1ea29b39f4c812ece2f936065a0a3d6fe44a263e
please use a proper fixes tag.
> ---
>  drivers/spi/spi-bcm2835aux.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
> index c8acde017b6a..6af07dec5fdc 100644
> --- a/drivers/spi/spi-bcm2835aux.c
> +++ b/drivers/spi/spi-bcm2835aux.c
> @@ -396,6 +396,38 @@ static void bcm2835aux_spi_handle_err(struct spi_master *master,
>  	bcm2835aux_spi_reset_hw(bs);
>  }
>
> +static int bcm2835aux_spi_setup(struct spi_device *spi)
> +{
> +	int ret;
> +
> +	if (spi->mode & SPI_NO_CS)
> +		return 0;
> +	/* sanity check for native cs */
> +	if (!gpio_is_valid(spi->cs_gpio)) {
> +		/* for dt-backwards compatibility: only support native on CS0
> +		 * known things not supported with broken native CS:
> +		 * * multiple chip-selects: cs0-cs2 are all
> +		 *     simultaniously asserted whenever there is a transfer
> +		 *     this even includes SPI_NO_CS
> +		 * * SPI_CS_HIGH: cs are always asserted low
> +		 * * cs_change: cs is deasserted after each spi_transfer
> +		 * * cs_delay_usec: cs is always deasserted one SCK cycle
> +		 *     after the last transfer
> +		 * probably more...
> +		 */
> +		if (spi->chip_select == 0) {
> +			dev_err(&spi->dev,
> +				"native CS is not supported but may work for some use-cases for cs = 0 -  please define a valid value cs-gpios in DT for complete feature set\n");
> +			return 0;

These messages are rather too long, please keep it in the ~ 80 char
limit (in source code and dmesg).
Or make it two lines ...

Since we return with 0, i prefer to use dev_warn instead of dev_err.

Thanks

> +		}
> +		dev_err(&spi->dev,
> +			"native CS is not supported for cs > 0 -  please define a valid value cs-gpios in DT to enable multiple cs\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
diff mbox series

Patch

diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
index c8acde017b6a..6af07dec5fdc 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c
@@ -396,6 +396,38 @@  static void bcm2835aux_spi_handle_err(struct spi_master *master,
 	bcm2835aux_spi_reset_hw(bs);
 }

+static int bcm2835aux_spi_setup(struct spi_device *spi)
+{
+	int ret;
+
+	if (spi->mode & SPI_NO_CS)
+		return 0;
+	/* sanity check for native cs */
+	if (!gpio_is_valid(spi->cs_gpio)) {
+		/* for dt-backwards compatibility: only support native on CS0
+		 * known things not supported with broken native CS:
+		 * * multiple chip-selects: cs0-cs2 are all
+		 *     simultaniously asserted whenever there is a transfer
+		 *     this even includes SPI_NO_CS
+		 * * SPI_CS_HIGH: cs are always asserted low
+		 * * cs_change: cs is deasserted after each spi_transfer
+		 * * cs_delay_usec: cs is always deasserted one SCK cycle
+		 *     after the last transfer
+		 * probably more...
+		 */
+		if (spi->chip_select == 0) {
+			dev_err(&spi->dev,
+				"native CS is not supported but may work for some use-cases for cs = 0 -  please define a valid value cs-gpios in DT for complete feature set\n");
+			return 0;
+		}
+		dev_err(&spi->dev,
+			"native CS is not supported for cs > 0 -  please define a valid value cs-gpios in DT to enable multiple cs\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int bcm2835aux_spi_probe(struct platform_device *pdev)
 {
 	struct spi_master *master;
@@ -425,6 +457,7 @@  static int bcm2835aux_spi_probe(struct platform_device *pdev)
 	 *     a spi_transfer
 	 */
 	master->num_chipselect = 1;
+	master->setup = bcm2835aux_spi_setup;
 	master->transfer_one = bcm2835aux_spi_transfer_one;
 	master->handle_err = bcm2835aux_spi_handle_err;
 	master->prepare_message = bcm2835aux_spi_prepare_message;