spi: bcm2835: Remove unnecessary workaround to call gpio_set_value

PATCH In review Axel Lin 3 participants 5 messages Feb 13, 2016 – Feb 15, 2016 3 messages not in the archive

Subsystems (3)

SPI Broadcom BCM2711/BCM2835 Arm architecture Broadcom BCM281XX/BCM11XXX/BCM216XX Arm architecture

lore

Share & tools

lore

Axel Lin

author

2016-02-13 16:19 UTC #0 lore raw

PATCH 1/1 spi: bcm2835: Remove unnecessary workaround to call gpio_set_value

This should be fixed by commit 4c02cba18cc9 ("pinctrl: bcm2835: Fix initial value for direction_output")

Signed-off-by Axel Lin <[email protected]>

---
 drivers/spi/spi-bcm2835.c | 5 -----
 1 file changed, 5 deletions(-)

drivers/spi/spi-bcm2835.c +0 -5

index cf04960..f35cc10 100644
@@ -727,11 +727,6 @@ static int bcm2835_spi_setup(struct spi_device *spi)
 			spi->chip_select, spi->cs_gpio, err);
 		return err;
 	}
-	/* the implementation of pinctrl-bcm2835 currently does not
-	 * set the GPIO value when using gpio_direction_output
-	 * so we are setting it here explicitly
-	 */
-	gpio_set_value(spi->cs_gpio, (spi->mode & SPI_CS_HIGH) ? 0 : 1);
 
 	return 0;
 }

Signature · 8 lines

2.1.4



--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

Martin Sperl

2016-02-13 17:08 UTC #2 lore raw

Quoted 7 lines · Axel Lin

This should be fixed by commit 4c02cba18cc9 ("pinctrl: bcm2835: Fix initial value for direction_output")
Signed-off-by Axel Lin <[email protected]>

Actually this also solves the situation when the DT is configured badly - and it is likely that old DTs are configured badly, because it did not matter before (it was always wrong anyway).

This change would break those old DTs in this respect and can result in unexpected behavior and undetected devices.

In this case it would be better to add a warning that the GPIO is not configured in the DT correctly, so that it is clear that the DT needs to change.

But then maybe a more generalized approach implemented in the framework would be better...

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 67B70D1B-7FA5-48A6-8719-490FA484BFF6-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org

Axel Lin

author

2016-02-14 03:41 UTC #4 lore raw

2016-02-14 1:08 GMT+08:00 Martin Sperl <[email protected]>:

Quoted 6 lines

Quoted 6 lines · Axel Lin
This should be fixed by commit 4c02cba18cc9 ("pinctrl: bcm2835: Fix initial value for direction_output")

Quoted 8 lines

Quoted 1 line
Signed-off-by Axel Lin <[email protected]>
Actually this also solves the situation when the DT is configured badly - and it is likely that old DTs are configured badly, because it did not matter before (it was always wrong anyway).
This change would break those old DTs in this respect and can result in unexpected behavior and undetected devices.

Hi Martin,
I don't understand why this patch breaks old DTs.

Before 4c02cba18cc9:
gpio_direction_output(spi->cs_gpio, (spi->mode & SPI_CS_HIGH) ? 0 : 1); gpio_set_value(spi->cs_gpio, (spi->mode & SPI_CS_HIGH) ? 0 : 1);

is equivalent to

After 4c02cba18cc9: (Already in Linus' tree) gpio_direction_output(spi->cs_gpio, (spi->mode & SPI_CS_HIGH) ? 0 : 1);

Axel
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): CAFRkauCaq+4Rs_MaApZmqdZ=SiBG2GViZkm1VnjY9j-ftFsXZg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

Martin Sperl

Acked-by

2016-02-14 11:28 UTC #6 lore raw

Quoted 22 lines · Axel Lin

2016-02-14 1:08 GMT+08:00 Martin Sperl <[email protected]>:
Quoted 6 lines
Quoted 6 lines · Axel Lin
This should be fixed by commit 4c02cba18cc9 ("pinctrl: bcm2835: Fix initial value for direction_output")
Quoted 8 lines
Quoted 1 line
Signed-off-by Axel Lin <[email protected]>
Actually this also solves the situation when the DT is configured badly - and it is likely that old DTs are configured badly, because it did not matter before (it was always wrong anyway).
This change would break those old DTs in this respect and can result in unexpected behavior and undetected devices.
Hi Martin,
I don't understand why this patch breaks old DTs.

Quoted 8 lines

Before 4c02cba18cc9:
gpio_direction_output(spi->cs_gpio, (spi->mode & SPI_CS_HIGH) ? 0 : 1); gpio_set_value(spi->cs_gpio, (spi->mode & SPI_CS_HIGH) ? 0 : 1);
is equivalent to
After 4c02cba18cc9: (Already in Linus' tree) gpio_direction_output(spi->cs_gpio, (spi->mode & SPI_CS_HIGH) ? 0 : 1);

Looking thru the bigger context now.

I thought it also impacted the GPIO-cs case, where
cs-gpios = <&gpio x 1>, <&gpio y 1>;
would just define the GPIOs - as the 3rd value was always ignored and resulted in a “low” level on the GPIO via pinctrl initially - at least there was a time when this has happened.
A change here would have had an impact on “old” DT.

Now I see that this only impacts the native-cs case and that should be fine.

Acked-by Martin Sperl <[email protected]>

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Mark Brown

2016-02-15 20:59 UTC #7 lore raw

Applied "spi: bcm2835: Remove unnecessary workaround to call gpio_set_value" to the spi tree

In reply to a message not in the archive

The patch

spi: bcm2835: Remove unnecessary workaround to call gpio_set_value

has been applied to the spi tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 763dedfcda5f6c9294c4a5d048f5261f442cc1e8 Mon Sep 17 00:00:00 2001

From: Axel Lin <[email protected]>
Date: Sun, 14 Feb 2016 00:19:07 +0800
Subject: [PATCH] spi: bcm2835: Remove unnecessary workaround to call
gpio_set_value

This should be fixed by commit 4c02cba18cc9 ("pinctrl: bcm2835: Fix initial value for direction_output")

Signed-off-by Axel Lin <[email protected]>

Acked-by Martin Sperl <[email protected]>

Signed-off-by Mark Brown <[email protected]>

---
 drivers/spi/spi-bcm2835.c | 5 -----
 1 file changed, 5 deletions(-)

drivers/spi/spi-bcm2835.c +0 -5

index cf04960..f35cc10 100644
@@ -727,11 +727,6 @@ static int bcm2835_spi_setup(struct spi_device *spi)
 			spi->chip_select, spi->cs_gpio, err);
 		return err;
 	}
-	/* the implementation of pinctrl-bcm2835 currently does not
-	 * set the GPIO value when using gpio_direction_output
-	 * so we are setting it here explicitly
-	 */
-	gpio_set_value(spi->cs_gpio, (spi->mode & SPI_CS_HIGH) ? 0 : 1);
 
 	return 0;
 }

Signature · 6 lines

2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html