[PATCH/RFC 6/6] spi: bcm2835: set gpio-cs to high state prior to any

[PATCH/RFC 6/6] spi: bcm2835: set gpio-cs to high state prior to any

[Martin Sperl]

 device initialization

The situation has been observed that on the RPI after a reboot the
GPIOs used for chip-selects are set to low when pinctl finishes setting
up the direction of the pins.

This can negatively impact the spi-bus and inhibit propper device
detection.

This (incomplete) code pulls the CS high to avoid the situation in the
common case of "normal" CS-priority - it still needs a revisit to get
the reverse polarity cases right as well.

A variation of this probably should get moved into spi_register_master
to fix this for other spi-controller as well.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
Tested-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

---
 drivers/spi/spi-bcm2835.c |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Applies against spi - topic/bcm2835.

diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index 1d731b8..b6825a4 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -349,6 +349,26 @@ static int bcm2835_spi_setup(struct spi_device *spi)
 	return -EINVAL;
 }
 
+static void bcm2835_spi_initialsetup_gpio(struct spi_master *master)
+{
+	int i;
+	bool level;
+
+	if (!master->cs_gpios)
+		return;
+
+	for (i = 0; i < master->num_chipselect; i++) {
+		if (gpio_is_valid(master->cs_gpios[i])) {
+			/* actually we need to figure out the
+			 * configured CSPOL of that device here,
+			 * not sure how to do that...
+			 */
+			level = true;
+			gpio_set_value(master->cs_gpios[i], level);
+		}
+	}
+}
+
 static int bcm2835_spi_probe(struct platform_device *pdev)
 {
 	struct spi_master *master;
@@ -414,6 +434,9 @@ static int bcm2835_spi_probe(struct platform_device *pdev)
 		goto out_clk_disable;
 	}
 
+	/* set GPIO-CS to disabled */
+	bcm2835_spi_initialsetup_gpio(master);
+
 	return 0;
 
 out_clk_disable:
-- 
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/RFC 6/6] spi: bcm2835: set gpio-cs to high state prior to any

[Mark Brown]

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

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

> A variation of this probably should get moved into spi_register_master
> to fix this for other spi-controller as well.

Yes, this is the wrong place to solve this.

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

Re: [PATCH/RFC 6/6] spi: bcm2835: set gpio-cs to high state prior to any

[Martin Sperl]

> On 30.03.2015, at 06:28, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> 
> On Sun, Mar 29, 2015 at 04:03:28PM +0200, Martin Sperl wrote:
> 
>> A variation of this probably should get moved into spi_register_master
>> to fix this for other spi-controller as well.
> 
> Yes, this is the wrong place to solve this.

So should we move that into spi_register_master?
My question for completeness remains: how do we get the devices
from dt or whatever while we are still registering the master?

Also I am still missing a bit of the big picture and vision how
to implement it taking all different HW/setup cases into account.

That is why I have put it as an RFC - mostly to get the issue some
exposure and some ideas.



--
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/RFC 6/6] spi: bcm2835: set gpio-cs to high state prior to any

[Stephen Warren]

On 03/30/2015 01:25 PM, Martin Sperl wrote:
> 
>> On 30.03.2015, at 06:28, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>
>> On Sun, Mar 29, 2015 at 04:03:28PM +0200, Martin Sperl wrote:
>>
>>> A variation of this probably should get moved into spi_register_master
>>> to fix this for other spi-controller as well.
>>
>> Yes, this is the wrong place to solve this.
> 
> So should we move that into spi_register_master?
> My question for completeness remains: how do we get the devices
> from dt or whatever while we are still registering the master?
> 
> Also I am still missing a bit of the big picture and vision how
> to implement it taking all different HW/setup cases into account.
> 
> That is why I have put it as an RFC - mostly to get the issue some
> exposure and some ideas.

I suspect the solution is simply not to have the pinctrl node set the CS
pins to GPIO_OUT, but rather rely on gpio_request() ->
bcm2835_gpio_request() -> bcm2835_pmx_gpio_set_direction() setting up
the pinmux function for any GPIOs that are requested. Presumably, the CS
GPIO for each chip is only requested when the DT node for the SPI device
(not SPI controller device) has been parsed, so the correct GPIO CS
value can be passed to gpio_request()/gpio_direction_output()?
--
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/RFC 6/6] spi: bcm2835: set gpio-cs to high state prior to any

[Martin Sperl]

> On 31.03.2015, at 05:35, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> 
> I suspect the solution is simply not to have the pinctrl node set the CS
> pins to GPIO_OUT, but rather rely on gpio_request() ->
> bcm2835_gpio_request() -> bcm2835_pmx_gpio_set_direction() setting up
> the pinmux function for any GPIOs that are requested. Presumably, the CS
> GPIO for each chip is only requested when the DT node for the SPI device
> (not SPI controller device) has been parsed, so the correct GPIO CS
> value can be passed to gpio_request()/gpio_direction_output()?

I will investigate that and create a patch to spi_register_master that
does that for any GPIO-chipselect - this still leaves the question of
how to iterate over all the configured devices early to detect if they
hace cspol set and then setting it low instead of high.

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