* [PATCH 2/9] spi: bcm2835aux: warn in dmesg that native cs is not really supported
2019-02-24 12:54 ` kernel
@ 2019-02-24 12:54 ` kernel
-1 siblings, 0 replies; 24+ messages in thread
From: kernel-TqfNSX0MhmxHKSADF0wUEw @ 2019-02-24 12:54 UTC (permalink / raw)
To: Mark Brown, Eric Anholt, Stefan Wahren, Hubert Denkmair,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.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.
Fixes: 1ea29b39f4c812ece2f936065a0a3d6fe44a263e
Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
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 2ece25250d86..4b6fdcb2cf4b 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c
@@ -420,6 +420,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;
@@ -449,6 +481,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;
--
2.11.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/9] spi: bcm2835aux: warn in dmesg that native cs is not really supported
@ 2019-02-24 12:54 ` kernel
0 siblings, 0 replies; 24+ messages in thread
From: kernel @ 2019-02-24 12:54 UTC (permalink / raw)
To: Mark Brown, Eric Anholt, Stefan Wahren, Hubert Denkmair,
linux-spi, linux-rpi-kernel, linux-arm-kernel
Cc: Martin Sperl
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.
Fixes: 1ea29b39f4c812ece2f936065a0a3d6fe44a263e
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
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 2ece25250d86..4b6fdcb2cf4b 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c
@@ -420,6 +420,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;
@@ -449,6 +481,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;
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/9] spi: bcm2835aux: setup gpio-cs to output and correct level during setup
2019-02-24 12:54 ` kernel
@ 2019-02-24 12:54 ` kernel
-1 siblings, 0 replies; 24+ messages in thread
From: kernel-TqfNSX0MhmxHKSADF0wUEw @ 2019-02-24 12:54 UTC (permalink / raw)
To: Mark Brown, Eric Anholt, Stefan Wahren, Hubert Denkmair,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
Setup gpio-cs to the correct levels during setup and also make the
gpio definitely an output GPIO.
This is transparently fixing some badly configured DTs in the process
where cs-gpio is set but the gpios are still configured with native cs.
It also makes 100% sure that the initial CS levels are as expected -
especially on systems with devices on a bus with mixed CS_HIGH/CS_LOW
settings.
Fixes: 1ea29b39f4c812ece2f936065a0a3d6fe44a263e
Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
drivers/spi/spi-bcm2835aux.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
index 4b6fdcb2cf4b..922fd798508e 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c
@@ -449,7 +449,17 @@ static int bcm2835aux_spi_setup(struct spi_device *spi)
return -EINVAL;
}
- return 0;
+ /* with gpio-cs set the GPIO to the correct level
+ * and as output (in case the dt has the gpio not configured
+ * as output but native cs)
+ */
+ ret = gpio_direction_output(spi->cs_gpio,
+ (spi->mode & SPI_CS_HIGH) ? 0 : 1);
+ if (ret)
+ dev_err(&spi->dev, "could not set gpio %i as output: %i\n",
+ spi->cs_gpio, ret);
+
+ return ret;
}
static int bcm2835aux_spi_probe(struct platform_device *pdev)
--
2.11.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/9] spi: bcm2835aux: setup gpio-cs to output and correct level during setup
@ 2019-02-24 12:54 ` kernel
0 siblings, 0 replies; 24+ messages in thread
From: kernel @ 2019-02-24 12:54 UTC (permalink / raw)
To: Mark Brown, Eric Anholt, Stefan Wahren, Hubert Denkmair,
linux-spi, linux-rpi-kernel, linux-arm-kernel
Cc: Martin Sperl
From: Martin Sperl <kernel@martin.sperl.org>
Setup gpio-cs to the correct levels during setup and also make the
gpio definitely an output GPIO.
This is transparently fixing some badly configured DTs in the process
where cs-gpio is set but the gpios are still configured with native cs.
It also makes 100% sure that the initial CS levels are as expected -
especially on systems with devices on a bus with mixed CS_HIGH/CS_LOW
settings.
Fixes: 1ea29b39f4c812ece2f936065a0a3d6fe44a263e
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/spi/spi-bcm2835aux.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
index 4b6fdcb2cf4b..922fd798508e 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c
@@ -449,7 +449,17 @@ static int bcm2835aux_spi_setup(struct spi_device *spi)
return -EINVAL;
}
- return 0;
+ /* with gpio-cs set the GPIO to the correct level
+ * and as output (in case the dt has the gpio not configured
+ * as output but native cs)
+ */
+ ret = gpio_direction_output(spi->cs_gpio,
+ (spi->mode & SPI_CS_HIGH) ? 0 : 1);
+ if (ret)
+ dev_err(&spi->dev, "could not set gpio %i as output: %i\n",
+ spi->cs_gpio, ret);
+
+ return ret;
}
static int bcm2835aux_spi_probe(struct platform_device *pdev)
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/9] spi: bcm2835aux: add driver specific stats to debugfs
2019-02-24 12:54 ` kernel
@ 2019-02-24 12:54 ` kernel
-1 siblings, 0 replies; 24+ messages in thread
From: kernel-TqfNSX0MhmxHKSADF0wUEw @ 2019-02-24 12:54 UTC (permalink / raw)
To: Mark Brown, Eric Anholt, Stefan Wahren, Hubert Denkmair,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
To estimate efficiency add statistics on transfer types
(polling and interrupt) used via debugfs.
Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
drivers/spi/spi-bcm2835aux.c | 61 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
index 922fd798508e..0a5f21eb3b99 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c
@@ -21,6 +21,7 @@
#include <linux/clk.h>
#include <linux/completion.h>
+#include <linux/debugfs.h>
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/interrupt.h>
@@ -102,8 +103,57 @@ struct bcm2835aux_spi {
int tx_len;
int rx_len;
int pending;
+
+#if defined(CONFIG_DEBUG_FS)
+#define BCM2835_AUX_INCR(field) ((field)++)
+ u64 count_transfer_polling;
+ u64 count_transfer_irq;
+ u64 count_transfer_irq_after_poll;
+
+ struct dentry *debugfs_dir;
+#else
+#define BCM2835_AUX_INCR(field)
+#endif /* CONFIG_DEBUG_FS */
};
+#if defined(CONFIG_DEBUG_FS)
+static void bcm2835aux_debugfs_create(struct bcm2835aux_spi *bs,
+ const char *dname)
+{
+ char name[64];
+ struct dentry *dir;
+
+ /* get full name */
+ snprintf(name, sizeof(name), "spi-bcm2835aux-%s", dname);
+
+ /* the base directory */
+ dir = debugfs_create_dir(name, NULL);
+ bs->debugfs_dir = dir;
+
+ /* the counters */
+ debugfs_create_u64("count_transfer_polling", 0444, dir,
+ &bs->count_transfer_polling);
+ debugfs_create_u64("count_transfer_irq", 0444, dir,
+ &bs->count_transfer_irq);
+ debugfs_create_u64("count_transfer_irq_after_poll", 0444, dir,
+ &bs->count_transfer_irq_after_poll);
+}
+
+static void bcm2835aux_debugfs_remove(struct bcm2835aux_spi *bs)
+{
+ debugfs_remove_recursive(bs->debugfs_dir);
+ bs->debugfs_dir = NULL;
+}
+#else
+static void bcm2835aux_debugfs_create(struct bcm2835aux_spi *bs)
+{
+}
+
+static void bcm2835aux_debugfs_remove(struct bcm2835aux_spi *bs)
+{
+}
+#endif /* CONFIG_DEBUG_FS */
+
static inline u32 bcm2835aux_rd(struct bcm2835aux_spi *bs, unsigned reg)
{
return readl(bs->regs + reg);
@@ -251,6 +301,9 @@ static int bcm2835aux_spi_transfer_one_irq(struct spi_master *master,
{
struct bcm2835aux_spi *bs = spi_master_get_devdata(master);
+ /* update statistics */
+ BCM2835_AUX_INCR(bs->count_transfer_irq);
+
/* fill in registers and fifos before enabling interrupts */
bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL1, bs->cntl[1]);
bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL0, bs->cntl[0]);
@@ -275,6 +328,9 @@ static int bcm2835aux_spi_transfer_one_poll(struct spi_master *master,
unsigned long timeout;
u32 stat;
+ /* update statistics */
+ BCM2835_AUX_INCR(bs->count_transfer_polling);
+
/* configure spi */
bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL1, bs->cntl[1]);
bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL0, bs->cntl[0]);
@@ -310,6 +366,7 @@ static int bcm2835aux_spi_transfer_one_poll(struct spi_master *master,
jiffies - timeout,
bs->tx_len, bs->rx_len);
/* forward to interrupt handler */
+ BCM2835_AUX_INCR(bs->count_transfer_irq_after_poll);
return __bcm2835aux_spi_transfer_one_irq(master,
spi, tfr);
}
@@ -555,6 +612,8 @@ static int bcm2835aux_spi_probe(struct platform_device *pdev)
goto out_clk_disable;
}
+ bcm2835aux_debugfs_create(bs, dev_name(&pdev->dev));
+
return 0;
out_clk_disable:
@@ -569,6 +628,8 @@ static int bcm2835aux_spi_remove(struct platform_device *pdev)
struct spi_master *master = platform_get_drvdata(pdev);
struct bcm2835aux_spi *bs = spi_master_get_devdata(master);
+ bcm2835aux_debugfs_remove(bs);
+
bcm2835aux_spi_reset_hw(bs);
/* disable the HW block by releasing the clock */
--
2.11.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/9] spi: bcm2835aux: add driver specific stats to debugfs
@ 2019-02-24 12:54 ` kernel
0 siblings, 0 replies; 24+ messages in thread
From: kernel @ 2019-02-24 12:54 UTC (permalink / raw)
To: Mark Brown, Eric Anholt, Stefan Wahren, Hubert Denkmair,
linux-spi, linux-rpi-kernel, linux-arm-kernel
Cc: Martin Sperl
From: Martin Sperl <kernel@martin.sperl.org>
To estimate efficiency add statistics on transfer types
(polling and interrupt) used via debugfs.
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/spi/spi-bcm2835aux.c | 61 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
index 922fd798508e..0a5f21eb3b99 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c
@@ -21,6 +21,7 @@
#include <linux/clk.h>
#include <linux/completion.h>
+#include <linux/debugfs.h>
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/interrupt.h>
@@ -102,8 +103,57 @@ struct bcm2835aux_spi {
int tx_len;
int rx_len;
int pending;
+
+#if defined(CONFIG_DEBUG_FS)
+#define BCM2835_AUX_INCR(field) ((field)++)
+ u64 count_transfer_polling;
+ u64 count_transfer_irq;
+ u64 count_transfer_irq_after_poll;
+
+ struct dentry *debugfs_dir;
+#else
+#define BCM2835_AUX_INCR(field)
+#endif /* CONFIG_DEBUG_FS */
};
+#if defined(CONFIG_DEBUG_FS)
+static void bcm2835aux_debugfs_create(struct bcm2835aux_spi *bs,
+ const char *dname)
+{
+ char name[64];
+ struct dentry *dir;
+
+ /* get full name */
+ snprintf(name, sizeof(name), "spi-bcm2835aux-%s", dname);
+
+ /* the base directory */
+ dir = debugfs_create_dir(name, NULL);
+ bs->debugfs_dir = dir;
+
+ /* the counters */
+ debugfs_create_u64("count_transfer_polling", 0444, dir,
+ &bs->count_transfer_polling);
+ debugfs_create_u64("count_transfer_irq", 0444, dir,
+ &bs->count_transfer_irq);
+ debugfs_create_u64("count_transfer_irq_after_poll", 0444, dir,
+ &bs->count_transfer_irq_after_poll);
+}
+
+static void bcm2835aux_debugfs_remove(struct bcm2835aux_spi *bs)
+{
+ debugfs_remove_recursive(bs->debugfs_dir);
+ bs->debugfs_dir = NULL;
+}
+#else
+static void bcm2835aux_debugfs_create(struct bcm2835aux_spi *bs)
+{
+}
+
+static void bcm2835aux_debugfs_remove(struct bcm2835aux_spi *bs)
+{
+}
+#endif /* CONFIG_DEBUG_FS */
+
static inline u32 bcm2835aux_rd(struct bcm2835aux_spi *bs, unsigned reg)
{
return readl(bs->regs + reg);
@@ -251,6 +301,9 @@ static int bcm2835aux_spi_transfer_one_irq(struct spi_master *master,
{
struct bcm2835aux_spi *bs = spi_master_get_devdata(master);
+ /* update statistics */
+ BCM2835_AUX_INCR(bs->count_transfer_irq);
+
/* fill in registers and fifos before enabling interrupts */
bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL1, bs->cntl[1]);
bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL0, bs->cntl[0]);
@@ -275,6 +328,9 @@ static int bcm2835aux_spi_transfer_one_poll(struct spi_master *master,
unsigned long timeout;
u32 stat;
+ /* update statistics */
+ BCM2835_AUX_INCR(bs->count_transfer_polling);
+
/* configure spi */
bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL1, bs->cntl[1]);
bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL0, bs->cntl[0]);
@@ -310,6 +366,7 @@ static int bcm2835aux_spi_transfer_one_poll(struct spi_master *master,
jiffies - timeout,
bs->tx_len, bs->rx_len);
/* forward to interrupt handler */
+ BCM2835_AUX_INCR(bs->count_transfer_irq_after_poll);
return __bcm2835aux_spi_transfer_one_irq(master,
spi, tfr);
}
@@ -555,6 +612,8 @@ static int bcm2835aux_spi_probe(struct platform_device *pdev)
goto out_clk_disable;
}
+ bcm2835aux_debugfs_create(bs, dev_name(&pdev->dev));
+
return 0;
out_clk_disable:
@@ -569,6 +628,8 @@ static int bcm2835aux_spi_remove(struct platform_device *pdev)
struct spi_master *master = platform_get_drvdata(pdev);
struct bcm2835aux_spi *bs = spi_master_get_devdata(master);
+ bcm2835aux_debugfs_remove(bs);
+
bcm2835aux_spi_reset_hw(bs);
/* disable the HW block by releasing the clock */
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 5/9] spi: bcm2835aux: make the polling duration limits configurable
2019-02-24 12:54 ` kernel
@ 2019-02-24 12:54 ` kernel
-1 siblings, 0 replies; 24+ messages in thread
From: kernel-TqfNSX0MhmxHKSADF0wUEw @ 2019-02-24 12:54 UTC (permalink / raw)
To: Mark Brown, Eric Anholt, Stefan Wahren, Hubert Denkmair,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
From: Martin Sperl <kernel@martin.sperl.org>
Under some circumstances the default 30 us polling limit is not optimal
and may lead to long delays because we are waiting on the interrupt
handler resulting in poor spi bus utilization.
With this patch we have the possibility to influence this policy
by making this limit configurable via a module parameters
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
Note that this patch relies on the effective_speed_hz patch for the
driver to be applied!
---
drivers/spi/spi-bcm2835aux.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
index 0a5f21eb3b99..af668cb6b38b 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c
@@ -37,6 +37,12 @@
#include <linux/spi/spi.h>
#include <linux/spinlock.h>
+/* define polling limits */
+unsigned int polling_limit_us = 30;
+module_param(polling_limit_us, uint, 0664);
+MODULE_PARM_DESC(polling_limit_us,
+ "time in us to run a transfer in polling mode\n");
+
/*
* spi register defines
*
@@ -89,10 +95,6 @@
#define BCM2835_AUX_SPI_STAT_BUSY 0x00000040
#define BCM2835_AUX_SPI_STAT_BITCOUNT 0x0000003F
-/* timeout values */
-#define BCM2835_AUX_SPI_POLLING_LIMIT_US 30
-#define BCM2835_AUX_SPI_POLLING_JIFFIES 2
-
struct bcm2835aux_spi {
void __iomem *regs;
struct clk *clk;
@@ -335,8 +337,8 @@ static int bcm2835aux_spi_transfer_one_poll(struct spi_master *master,
bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL1, bs->cntl[1]);
bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL0, bs->cntl[0]);
- /* set the timeout */
- timeout = jiffies + BCM2835_AUX_SPI_POLLING_JIFFIES;
+ /* set the timeout to at least 2 jiffies */
+ timeout = jiffies + 2 + HZ * polling_limit_us / 1000000;
/* loop until finished the transfer */
while (bs->rx_len) {
@@ -381,7 +383,7 @@ static int bcm2835aux_spi_transfer_one(struct spi_master *master,
struct spi_transfer *tfr)
{
struct bcm2835aux_spi *bs = spi_master_get_devdata(master);
- unsigned long spi_hz, clk_hz, speed;
+ unsigned long spi_hz, clk_hz, speed, hz_per_byte, byte_limit;
/* calculate the registers to handle
*
@@ -425,14 +427,15 @@ static int bcm2835aux_spi_transfer_one(struct spi_master *master,
* of Hz per byte per polling limit. E.g., we can transfer 1 byte in
* 30 µs per 300,000 Hz of bus clock.
*/
-#define HZ_PER_BYTE ((9 * 1000000) / BCM2835_AUX_SPI_POLLING_LIMIT_US)
+ hz_per_byte = polling_limit_us ? (9 * 1000000) / polling_limit_us : 0;
+ byte_limit = hz_per_byte ? tfr->effective_speed_hz / hz_per_byte : 1;
+
/* run in polling mode for short transfers */
- if (tfr->len < tfr->effective_speed_hz / HZ_PER_BYTE)
+ if (tfr->len < byte_limit)
return bcm2835aux_spi_transfer_one_poll(master, spi, tfr);
/* run in interrupt mode for all others */
return bcm2835aux_spi_transfer_one_irq(master, spi, tfr);
-#undef HZ_PER_BYTE
}
static int bcm2835aux_spi_prepare_message(struct spi_master *master,
--
2.11.0
_______________________________________________
linux-rpi-kernel mailing list
linux-rpi-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 5/9] spi: bcm2835aux: make the polling duration limits configurable
@ 2019-02-24 12:54 ` kernel
0 siblings, 0 replies; 24+ messages in thread
From: kernel @ 2019-02-24 12:54 UTC (permalink / raw)
To: Mark Brown, Eric Anholt, Stefan Wahren, Hubert Denkmair,
linux-spi, linux-rpi-kernel, linux-arm-kernel
Cc: Martin Sperl
From: Martin Sperl <kernel@martin.sperl.org>
Under some circumstances the default 30 us polling limit is not optimal
and may lead to long delays because we are waiting on the interrupt
handler resulting in poor spi bus utilization.
With this patch we have the possibility to influence this policy
by making this limit configurable via a module parameters
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
Note that this patch relies on the effective_speed_hz patch for the
driver to be applied!
---
drivers/spi/spi-bcm2835aux.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
index 0a5f21eb3b99..af668cb6b38b 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c
@@ -37,6 +37,12 @@
#include <linux/spi/spi.h>
#include <linux/spinlock.h>
+/* define polling limits */
+unsigned int polling_limit_us = 30;
+module_param(polling_limit_us, uint, 0664);
+MODULE_PARM_DESC(polling_limit_us,
+ "time in us to run a transfer in polling mode\n");
+
/*
* spi register defines
*
@@ -89,10 +95,6 @@
#define BCM2835_AUX_SPI_STAT_BUSY 0x00000040
#define BCM2835_AUX_SPI_STAT_BITCOUNT 0x0000003F
-/* timeout values */
-#define BCM2835_AUX_SPI_POLLING_LIMIT_US 30
-#define BCM2835_AUX_SPI_POLLING_JIFFIES 2
-
struct bcm2835aux_spi {
void __iomem *regs;
struct clk *clk;
@@ -335,8 +337,8 @@ static int bcm2835aux_spi_transfer_one_poll(struct spi_master *master,
bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL1, bs->cntl[1]);
bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL0, bs->cntl[0]);
- /* set the timeout */
- timeout = jiffies + BCM2835_AUX_SPI_POLLING_JIFFIES;
+ /* set the timeout to at least 2 jiffies */
+ timeout = jiffies + 2 + HZ * polling_limit_us / 1000000;
/* loop until finished the transfer */
while (bs->rx_len) {
@@ -381,7 +383,7 @@ static int bcm2835aux_spi_transfer_one(struct spi_master *master,
struct spi_transfer *tfr)
{
struct bcm2835aux_spi *bs = spi_master_get_devdata(master);
- unsigned long spi_hz, clk_hz, speed;
+ unsigned long spi_hz, clk_hz, speed, hz_per_byte, byte_limit;
/* calculate the registers to handle
*
@@ -425,14 +427,15 @@ static int bcm2835aux_spi_transfer_one(struct spi_master *master,
* of Hz per byte per polling limit. E.g., we can transfer 1 byte in
* 30 µs per 300,000 Hz of bus clock.
*/
-#define HZ_PER_BYTE ((9 * 1000000) / BCM2835_AUX_SPI_POLLING_LIMIT_US)
+ hz_per_byte = polling_limit_us ? (9 * 1000000) / polling_limit_us : 0;
+ byte_limit = hz_per_byte ? tfr->effective_speed_hz / hz_per_byte : 1;
+
/* run in polling mode for short transfers */
- if (tfr->len < tfr->effective_speed_hz / HZ_PER_BYTE)
+ if (tfr->len < byte_limit)
return bcm2835aux_spi_transfer_one_poll(master, spi, tfr);
/* run in interrupt mode for all others */
return bcm2835aux_spi_transfer_one_irq(master, spi, tfr);
-#undef HZ_PER_BYTE
}
static int bcm2835aux_spi_prepare_message(struct spi_master *master,
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 7/9] spi: bcm2835aux: remove dangerous uncontrolled read of fifo
2019-02-24 12:54 ` kernel
@ 2019-02-24 12:54 ` kernel
-1 siblings, 0 replies; 24+ messages in thread
From: kernel-TqfNSX0MhmxHKSADF0wUEw @ 2019-02-24 12:54 UTC (permalink / raw)
To: Mark Brown, Eric Anholt, Stefan Wahren, Hubert Denkmair,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
This read of the fifo is a potential candidate for a race condition
as the spi transfer is not necessarily finished and so can lead to
an early read of the fifo that still misses data.
So it has been removed - an additional loop will read the fifo
correctly.
Fixes: 1ea29b39f4c812ece2f936065a0a3d6fe44a263e
(Note: requires Patch 6)
Suggested-by: Hubert Denkmair <h.denkmair-4pT7WzY1KUiELgA04lAiVw@public.gmane.org>
Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
drivers/spi/spi-bcm2835aux.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
index 9552f5343982..8821fc183ca0 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c
@@ -246,13 +246,6 @@ static void bcm2835aux_spi_transfer_helper(struct bcm2835aux_spi *bs)
BCM2835_AUX_SPI_STAT_TX_FULL))) {
bcm2835aux_wr_fifo(bs);
}
-
- /* and check if we have reached "done" */
- while (bs->rx_len &&
- (!(bcm2835aux_rd(bs, BCM2835_AUX_SPI_STAT) &
- BCM2835_AUX_SPI_STAT_BUSY))) {
- bcm2835aux_rd_fifo(bs);
- }
}
static irqreturn_t bcm2835aux_spi_interrupt(int irq, void *dev_id)
--
2.11.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 7/9] spi: bcm2835aux: remove dangerous uncontrolled read of fifo
@ 2019-02-24 12:54 ` kernel
0 siblings, 0 replies; 24+ messages in thread
From: kernel @ 2019-02-24 12:54 UTC (permalink / raw)
To: Mark Brown, Eric Anholt, Stefan Wahren, Hubert Denkmair,
linux-spi, linux-rpi-kernel, linux-arm-kernel
Cc: Martin Sperl
From: Martin Sperl <kernel@martin.sperl.org>
This read of the fifo is a potential candidate for a race condition
as the spi transfer is not necessarily finished and so can lead to
an early read of the fifo that still misses data.
So it has been removed - an additional loop will read the fifo
correctly.
Fixes: 1ea29b39f4c812ece2f936065a0a3d6fe44a263e
(Note: requires Patch 6)
Suggested-by: Hubert Denkmair <h.denkmair@intence.de>
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/spi/spi-bcm2835aux.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
index 9552f5343982..8821fc183ca0 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c
@@ -246,13 +246,6 @@ static void bcm2835aux_spi_transfer_helper(struct bcm2835aux_spi *bs)
BCM2835_AUX_SPI_STAT_TX_FULL))) {
bcm2835aux_wr_fifo(bs);
}
-
- /* and check if we have reached "done" */
- while (bs->rx_len &&
- (!(bcm2835aux_rd(bs, BCM2835_AUX_SPI_STAT) &
- BCM2835_AUX_SPI_STAT_BUSY))) {
- bcm2835aux_rd_fifo(bs);
- }
}
static irqreturn_t bcm2835aux_spi_interrupt(int irq, void *dev_id)
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 8/9] spi: bcm2835aux: use BCM2835_AUX_SPI_STAT_RX_LVL
2019-02-24 12:54 ` kernel
@ 2019-02-24 12:54 ` kernel
-1 siblings, 0 replies; 24+ messages in thread
From: kernel-TqfNSX0MhmxHKSADF0wUEw @ 2019-02-24 12:54 UTC (permalink / raw)
To: Mark Brown, Eric Anholt, Stefan Wahren, Hubert Denkmair,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
On long running tests with a mcp2517fd can controller it showed that
on rare occations the data read shows corruptions for longer spi transfers.
Example of a 22 byte transfer:
expected (as captured on logic analyzer):
FF FF 78 00 00 00 08 06 00 00 91 20 77 56 84 85 86 87 88 89 8a 8b
read by the driver:
FF FF 78 00 00 00 08 06 00 00 91 20 77 56 84 88 89 8a 00 00 8b 9b
To fix this use BCM2835_AUX_SPI_STAT_RX_LVL to determine when we may
read data from the fifo reliably without any corruption.
Surprisingly the only values ever empirically read in
BCM2835_AUX_SPI_STAT_RX_LVL are 0x00, 0x10, 0x20 and 0x30.
So whenever the mask is not 0 we can read from the fifo in a safe manner.
The patch has now been tested intensively and we are no longer
able to reproduce the "RX" issue any longer.
Fixes: 1ea29b39f4c812ece2f936065a0a3d6fe44a263e
(Note: requires Patch 6)
Reported-by: Hubert Denkmair <h.denkmair-4pT7WzY1KUiELgA04lAiVw@public.gmane.org>
Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
drivers/spi/spi-bcm2835aux.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
index 8821fc183ca0..8cd273d0fef2 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c
@@ -232,12 +232,12 @@ static void bcm2835aux_spi_reset_hw(struct bcm2835aux_spi *bs)
static void bcm2835aux_spi_transfer_helper(struct bcm2835aux_spi *bs)
{
+ u32 stat = bcm2835aux_rd(bs, BCM2835_AUX_SPI_STAT);
+
/* check if we have data to read */
- while (bs->rx_len &&
- (!(bcm2835aux_rd(bs, BCM2835_AUX_SPI_STAT) &
- BCM2835_AUX_SPI_STAT_RX_EMPTY))) {
+ for (; bs->rx_len && (stat & BCM2835_AUX_SPI_STAT_RX_LVL);
+ stat = bcm2835aux_rd(bs, BCM2835_AUX_SPI_STAT))
bcm2835aux_rd_fifo(bs);
- }
/* check if we have data to write */
while (bs->tx_len &&
--
2.11.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 8/9] spi: bcm2835aux: use BCM2835_AUX_SPI_STAT_RX_LVL
@ 2019-02-24 12:54 ` kernel
0 siblings, 0 replies; 24+ messages in thread
From: kernel @ 2019-02-24 12:54 UTC (permalink / raw)
To: Mark Brown, Eric Anholt, Stefan Wahren, Hubert Denkmair,
linux-spi, linux-rpi-kernel, linux-arm-kernel
Cc: Martin Sperl
From: Martin Sperl <kernel@martin.sperl.org>
On long running tests with a mcp2517fd can controller it showed that
on rare occations the data read shows corruptions for longer spi transfers.
Example of a 22 byte transfer:
expected (as captured on logic analyzer):
FF FF 78 00 00 00 08 06 00 00 91 20 77 56 84 85 86 87 88 89 8a 8b
read by the driver:
FF FF 78 00 00 00 08 06 00 00 91 20 77 56 84 88 89 8a 00 00 8b 9b
To fix this use BCM2835_AUX_SPI_STAT_RX_LVL to determine when we may
read data from the fifo reliably without any corruption.
Surprisingly the only values ever empirically read in
BCM2835_AUX_SPI_STAT_RX_LVL are 0x00, 0x10, 0x20 and 0x30.
So whenever the mask is not 0 we can read from the fifo in a safe manner.
The patch has now been tested intensively and we are no longer
able to reproduce the "RX" issue any longer.
Fixes: 1ea29b39f4c812ece2f936065a0a3d6fe44a263e
(Note: requires Patch 6)
Reported-by: Hubert Denkmair <h.denkmair@intence.de>
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/spi/spi-bcm2835aux.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
index 8821fc183ca0..8cd273d0fef2 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c
@@ -232,12 +232,12 @@ static void bcm2835aux_spi_reset_hw(struct bcm2835aux_spi *bs)
static void bcm2835aux_spi_transfer_helper(struct bcm2835aux_spi *bs)
{
+ u32 stat = bcm2835aux_rd(bs, BCM2835_AUX_SPI_STAT);
+
/* check if we have data to read */
- while (bs->rx_len &&
- (!(bcm2835aux_rd(bs, BCM2835_AUX_SPI_STAT) &
- BCM2835_AUX_SPI_STAT_RX_EMPTY))) {
+ for (; bs->rx_len && (stat & BCM2835_AUX_SPI_STAT_RX_LVL);
+ stat = bcm2835aux_rd(bs, BCM2835_AUX_SPI_STAT))
bcm2835aux_rd_fifo(bs);
- }
/* check if we have data to write */
while (bs->tx_len &&
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 9/9] spi: bcm2835aux: remove dead code
2019-02-24 12:54 ` kernel
@ 2019-02-24 12:54 ` kernel
-1 siblings, 0 replies; 24+ messages in thread
From: kernel-TqfNSX0MhmxHKSADF0wUEw @ 2019-02-24 12:54 UTC (permalink / raw)
To: Mark Brown, Eric Anholt, Stefan Wahren, Hubert Denkmair,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
Remove dead code that never can get reached, as we limit count to
a max of 3.
Suggested-by: Hubert Denkmair <h.denkmair-4pT7WzY1KUiELgA04lAiVw@public.gmane.org>
Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
drivers/spi/spi-bcm2835aux.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
index 8cd273d0fef2..109eba6308cc 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c
@@ -175,9 +175,6 @@ static inline void bcm2835aux_rd_fifo(struct bcm2835aux_spi *bs)
data = bcm2835aux_rd(bs, BCM2835_AUX_SPI_IO);
if (bs->rx_buf) {
switch (count) {
- case 4:
- *bs->rx_buf++ = (data >> 24) & 0xff;
- /* fallthrough */
case 3:
*bs->rx_buf++ = (data >> 16) & 0xff;
/* fallthrough */
--
2.11.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 9/9] spi: bcm2835aux: remove dead code
@ 2019-02-24 12:54 ` kernel
0 siblings, 0 replies; 24+ messages in thread
From: kernel @ 2019-02-24 12:54 UTC (permalink / raw)
To: Mark Brown, Eric Anholt, Stefan Wahren, Hubert Denkmair,
linux-spi, linux-rpi-kernel, linux-arm-kernel
Cc: Martin Sperl
From: Martin Sperl <kernel@martin.sperl.org>
Remove dead code that never can get reached, as we limit count to
a max of 3.
Suggested-by: Hubert Denkmair <h.denkmair@intence.de>
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/spi/spi-bcm2835aux.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
index 8cd273d0fef2..109eba6308cc 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c
@@ -175,9 +175,6 @@ static inline void bcm2835aux_rd_fifo(struct bcm2835aux_spi *bs)
data = bcm2835aux_rd(bs, BCM2835_AUX_SPI_IO);
if (bs->rx_buf) {
switch (count) {
- case 4:
- *bs->rx_buf++ = (data >> 24) & 0xff;
- /* fallthrough */
case 3:
*bs->rx_buf++ = (data >> 16) & 0xff;
/* fallthrough */
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 0/9] spi: bcm2835aux: bug fixes and improvements
2019-02-24 12:54 ` kernel
@ 2019-02-24 19:22 ` Stefan Wahren
-1 siblings, 0 replies; 24+ messages in thread
From: Stefan Wahren @ 2019-02-24 19:22 UTC (permalink / raw)
To: kernel-TqfNSX0MhmxHKSADF0wUEw
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, Mark Brown,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Hubert Denkmair,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Hi Martin,
> kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org hat am 24. Februar 2019 um 13:54 geschrieben:
>
>
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>
> Set of patches improving the spi-bcm2835aux driver and fixing
> a data read corruption bug.
>
> The main motivation is a rare data corruption fix that is mostly
> observed in polling mode first reported by Hubert Denkmair.
>
> So this patchset first implements a means to control the parameters
> of when polling mode is used via module parameters and exports
> the corresponding statistics.
>
> As stated in original patch the driver does not support native CS.
> But when cs-gpios is not configured in the dt (so a buggy dt) it is
> still working with a lot of limitations, but the driver does not report
> this fact.
>
> So this patchset adds reporting and allows for a single native CS
> (with limited functionality) to continue working with a buggy DT.
> One question here remains: do we need to legacy support DTs
> that are not following specs in the first place?
>
> Then there is the real fix for the data-corruption which is split
> into 3 parts: some code cleanup with code reuse, removing "dangerous"
> fifo read (possibly introducing fifo data corruption) and safe fifo read
>
> Finally we remove some dead code.
>
> Martin Sperl (9):
> spi: bcm2835aux: fix driver to not allow 65535 (=-1) cs-gpios
> spi: bcm2835aux: warn in dmesg that native cs is not really supported
> spi: bcm2835aux: setup gpio-cs to output and correct level during
> setup
> spi: bcm2835aux: add driver specific stats to debugfs
> spi: bcm2835aux: make the polling duration limits configurable
> spi: bcm2835aux: unifying code between polling and interrupt driven
> code
> spi: bcm2835aux: remove dangerous uncontrolled read of fifo
> spi: bcm2835aux: use BCM2835_AUX_SPI_STAT_RX_LVL
> spi: bcm2835aux: remove dead code
>
i consider patch 7 and 8 as possible stable candidates. How about rearranging this series that these patches comes first and could be backported?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/9] spi: bcm2835aux: bug fixes and improvements
@ 2019-02-24 19:22 ` Stefan Wahren
0 siblings, 0 replies; 24+ messages in thread
From: Stefan Wahren @ 2019-02-24 19:22 UTC (permalink / raw)
To: kernel
Cc: linux-spi, Eric Anholt, Mark Brown, linux-rpi-kernel,
Hubert Denkmair, linux-arm-kernel
Hi Martin,
> kernel@martin.sperl.org hat am 24. Februar 2019 um 13:54 geschrieben:
>
>
> From: Martin Sperl <kernel@martin.sperl.org>
>
> Set of patches improving the spi-bcm2835aux driver and fixing
> a data read corruption bug.
>
> The main motivation is a rare data corruption fix that is mostly
> observed in polling mode first reported by Hubert Denkmair.
>
> So this patchset first implements a means to control the parameters
> of when polling mode is used via module parameters and exports
> the corresponding statistics.
>
> As stated in original patch the driver does not support native CS.
> But when cs-gpios is not configured in the dt (so a buggy dt) it is
> still working with a lot of limitations, but the driver does not report
> this fact.
>
> So this patchset adds reporting and allows for a single native CS
> (with limited functionality) to continue working with a buggy DT.
> One question here remains: do we need to legacy support DTs
> that are not following specs in the first place?
>
> Then there is the real fix for the data-corruption which is split
> into 3 parts: some code cleanup with code reuse, removing "dangerous"
> fifo read (possibly introducing fifo data corruption) and safe fifo read
>
> Finally we remove some dead code.
>
> Martin Sperl (9):
> spi: bcm2835aux: fix driver to not allow 65535 (=-1) cs-gpios
> spi: bcm2835aux: warn in dmesg that native cs is not really supported
> spi: bcm2835aux: setup gpio-cs to output and correct level during
> setup
> spi: bcm2835aux: add driver specific stats to debugfs
> spi: bcm2835aux: make the polling duration limits configurable
> spi: bcm2835aux: unifying code between polling and interrupt driven
> code
> spi: bcm2835aux: remove dangerous uncontrolled read of fifo
> spi: bcm2835aux: use BCM2835_AUX_SPI_STAT_RX_LVL
> spi: bcm2835aux: remove dead code
>
i consider patch 7 and 8 as possible stable candidates. How about rearranging this series that these patches comes first and could be backported?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/9] spi: bcm2835aux: bug fixes and improvements
2019-02-24 19:22 ` Stefan Wahren
@ 2019-02-24 20:11 ` kernel
-1 siblings, 0 replies; 24+ messages in thread
From: kernel @ 2019-02-24 20:11 UTC (permalink / raw)
To: Stefan Wahren
Cc: linux-spi, Eric Anholt, Mark Brown, linux-rpi-kernel,
Hubert Denkmair, linux-arm-kernel
> On 24.02.2019, at 20:22, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>>
>> Finally we remove some dead code.
>>
>> Martin Sperl (9):
>> spi: bcm2835aux: fix driver to not allow 65535 (=-1) cs-gpios
>> spi: bcm2835aux: warn in dmesg that native cs is not really supported
>> spi: bcm2835aux: setup gpio-cs to output and correct level during
>> setup
>> spi: bcm2835aux: add driver specific stats to debugfs
>> spi: bcm2835aux: make the polling duration limits configurable
>> spi: bcm2835aux: unifying code between polling and interrupt driven
>> code
>> spi: bcm2835aux: remove dangerous uncontrolled read of fifo
>> spi: bcm2835aux: use BCM2835_AUX_SPI_STAT_RX_LVL
>> spi: bcm2835aux: remove dead code
>>
>
> i consider patch 7 and 8 as possible stable candidates. How about rearranging this series that these patches comes first and could be backported?
Well - as far as I know 6+7+8 can get applied independently
(and you will need 6 as otherwise you would need to apply
at least 8 in 2 places).
The one observation that I have made is that when applying those
against 4.14 you needed some more patches that have gone in since
4.14 to apply cleanly.
Martin
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/9] spi: bcm2835aux: bug fixes and improvements
@ 2019-02-24 20:11 ` kernel
0 siblings, 0 replies; 24+ messages in thread
From: kernel @ 2019-02-24 20:11 UTC (permalink / raw)
To: Stefan Wahren
Cc: linux-spi, Eric Anholt, Mark Brown, linux-rpi-kernel,
Hubert Denkmair, linux-arm-kernel
> On 24.02.2019, at 20:22, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>>
>> Finally we remove some dead code.
>>
>> Martin Sperl (9):
>> spi: bcm2835aux: fix driver to not allow 65535 (=-1) cs-gpios
>> spi: bcm2835aux: warn in dmesg that native cs is not really supported
>> spi: bcm2835aux: setup gpio-cs to output and correct level during
>> setup
>> spi: bcm2835aux: add driver specific stats to debugfs
>> spi: bcm2835aux: make the polling duration limits configurable
>> spi: bcm2835aux: unifying code between polling and interrupt driven
>> code
>> spi: bcm2835aux: remove dangerous uncontrolled read of fifo
>> spi: bcm2835aux: use BCM2835_AUX_SPI_STAT_RX_LVL
>> spi: bcm2835aux: remove dead code
>>
>
> i consider patch 7 and 8 as possible stable candidates. How about rearranging this series that these patches comes first and could be backported?
Well - as far as I know 6+7+8 can get applied independently
(and you will need 6 as otherwise you would need to apply
at least 8 in 2 places).
The one observation that I have made is that when applying those
against 4.14 you needed some more patches that have gone in since
4.14 to apply cleanly.
Martin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 24+ messages in thread