[PATCH 4/6] spi: bcm2835: warn about native-chip-selects being used

[PATCH 4/6] spi: bcm2835: warn about native-chip-selects being used

[Martin Sperl]

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
Tested-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 drivers/spi/spi-bcm2835.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

Applies against spi - topic/bcm2835

Could get replaced by something similar to this.
	if (!gpio_is_valid(spi->gpio_cs)) {
		if (spi->chip_select == 0) {
			spi->gpio_cs = 8;
			gpio_set_output(spi->gpio_cs);
		}
		if (spi->chip_select == 1) {
			spi->gpio_cs = 7;
			gpio_set_output(spi->gpio_cs);
		}
	}
But I do not know what is the correct call to use.

diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index adf157b..601fc5e 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -82,6 +82,7 @@ struct bcm2835_spi {
 	u8 *rx_buf;
 	int tx_len;
 	int rx_len;
+	bool native_cs_use_warning_done;
 };
 
 static inline u32 bcm2835_rd(struct bcm2835_spi *bs, unsigned reg)
@@ -287,6 +288,9 @@ static void bcm2835_spi_set_cs(struct spi_device *spi, bool gpio_level)
 
 static int bcm2835_spi_setup(struct spi_device *spi)
 {
+	struct spi_master *master = spi->master;
+	struct bcm2835_spi *bs = spi_master_get_devdata(master);
+
 	/*
 	 * sanity checking the native-chipselects
 	 */
@@ -294,6 +298,14 @@ static int bcm2835_spi_setup(struct spi_device *spi)
 		return 0;
 	if (gpio_is_valid(spi->cs_gpio))
 		return 0;
+	/* we are in the native chipselect case now,
+	 * so warn about the fact that some things may not work as well
+	 */
+	if (!bs->native_cs_use_warning_done) {
+		dev_warn(&spi->dev,
+			 "setup: native chipselect is used - some driver functions/optimizations are not applied\n");
+		bs->native_cs_use_warning_done = 1;
+	}
 	if (spi->chip_select < 3)
 		return 0;
 
-- 
1.7.10.4


--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] spi: bcm2835: warn about native-chip-selects being used

[Mark Brown]

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

On Sun, Mar 29, 2015 at 04:03:26PM +0200, Martin Sperl wrote:

> Could get replaced by something similar to this.
> 	if (!gpio_is_valid(spi->gpio_cs)) {
> 		if (spi->chip_select == 0) {
> 			spi->gpio_cs = 8;
> 			gpio_set_output(spi->gpio_cs);
> 		}
> 		if (spi->chip_select == 1) {
> 			spi->gpio_cs = 7;
> 			gpio_set_output(spi->gpio_cs);
> 		}
> 	}
> But I do not know what is the correct call to use.

I don't know what you mean by "correct call" here, sorry.  If you're
talking about working out which GPIO maps to the /CS pin then can the
pinctrl API help?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 4/6] spi: bcm2835: warn about native-chip-selects being used

[Martin Sperl]

> On 30.03.2015, at 06:22, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> 
>> But I do not know what is the correct call to use.
> 
> I don't know what you mean by "correct call" here, sorry.  If you're
> talking about working out which GPIO maps to the /CS pin then can the
> pinctrl API help?

Well - how do I change the mode of a GPIO to output (which is typically ALT0)?
something like this:
>> 			gpio_set_output(spi->gpio_cs);

I was hoping someone would have an explicit pointer...

Martin


--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] spi: bcm2835: warn about native-chip-selects being used

[Stephen Warren]

On 03/30/2015 01:20 PM, Martin Sperl wrote:
> 
>> On 30.03.2015, at 06:22, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>>
>>> But I do not know what is the correct call to use.
>>
>> I don't know what you mean by "correct call" here, sorry.  If you're
>> talking about working out which GPIO maps to the /CS pin then can the
>> pinctrl API help?
> 
> Well - how do I change the mode of a GPIO to output (which is typically ALT0)?
> something like this:
>
>>> 			gpio_set_output(spi->gpio_cs);
> 
> I was hoping someone would have an explicit pointer...

Presumably the following DT bindings would work:

Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] spi: bcm2835: warn about native-chip-selects being used

[Stephen Warren]

On 03/29/2015 08:03 AM, Martin Sperl wrote:
> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> Tested-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

It would be useful to have a commit description saying why such a
warning was useful.

> diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c

> @@ -82,6 +82,7 @@ struct bcm2835_spi {
>  	u8 *rx_buf;
>  	int tx_len;
>  	int rx_len;
> +	bool native_cs_use_warning_done;
>  };
...
> @@ -294,6 +298,14 @@ static int bcm2835_spi_setup(struct spi_device *spi)
...
> +	/* we are in the native chipselect case now,
> +	 * so warn about the fact that some things may not work as well

What things and why? (I'm asking mainly because that should be in the
commit description and perhaps this comment).

> +	 */
> +	if (!bs->native_cs_use_warning_done) {
> +		dev_warn(&spi->dev,
> +			 "setup: native chipselect is used - some driver functions/optimizations are not applied\n");
> +		bs->native_cs_use_warning_done = 1;
> +	}

Isn't there a warn_once() that would remove the need to check/set
native_cs_use_warning_done?
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html