Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 10/15] spi: use new `spi_transfer_delay` helper where straightforward
From: Alexandru Ardelean @ 2019-09-13 11:45 UTC (permalink / raw)
  To: linux-spi, linux-iio, linux-arm-kernel, linux-tegra, linux-kernel,
	bcm-kernel-feedback-list
  Cc: f.fainelli, baolin.wang, zhang.lyra, linus.walleij, broonie,
	orsonzhai, Alexandru Ardelean, jic23
In-Reply-To: <20190913114550.956-1-alexandru.ardelean@analog.com>

For many places in the spi drivers, using the new `spi_transfer_delay`
helper is straightforward.
It's just replacing:
```
  if (t->delay_usecs)
     udelay(t->delay_usecs);
```
with `spi_transfer_delay(t)` which handles both `delay_usecs` and the new
`delay` field.

This change replaces in all places (in the spi drivers)  where this change
is simple.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/spi/spi-atmel.c          |  3 +--
 drivers/spi/spi-bcm63xx-hsspi.c  |  3 +--
 drivers/spi/spi-cavium.c         |  3 +--
 drivers/spi/spi-fsl-dspi.c       |  3 +--
 drivers/spi/spi-fsl-espi.c       |  3 +--
 drivers/spi/spi-fsl-spi.c        |  3 +--
 drivers/spi/spi-mpc512x-psc.c    |  3 +--
 drivers/spi/spi-mpc52xx-psc.c    |  3 +--
 drivers/spi/spi-omap-100k.c      |  3 +--
 drivers/spi/spi-pl022.c          | 25 +++++++++++--------------
 drivers/spi/spi-sc18is602.c      |  3 +--
 drivers/spi/spi-sh-hspi.c        |  3 +--
 drivers/spi/spi-tegra20-sflash.c |  2 +-
 drivers/spi/spi-topcliff-pch.c   |  7 +------
 drivers/spi/spi-txx9.c           |  3 +--
 drivers/spi/spi-xcomm.c          |  3 +--
 16 files changed, 26 insertions(+), 47 deletions(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 009bb4fb5e5f..f51af930c7a4 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -1389,8 +1389,7 @@ static int atmel_spi_one_transfer(struct spi_master *master,
 		&& as->use_pdc)
 		atmel_spi_dma_unmap_xfer(master, xfer);
 
-	if (xfer->delay_usecs)
-		udelay(xfer->delay_usecs);
+	spi_transfer_delay(xfer);
 
 	if (xfer->cs_change) {
 		if (list_is_last(&xfer->transfer_list,
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index 9a06ffdb73b8..d9f5c241e2aa 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -291,8 +291,7 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
 
 		msg->actual_length += t->len;
 
-		if (t->delay_usecs)
-			udelay(t->delay_usecs);
+		spi_transfer_delay(t);
 
 		if (t->cs_change)
 			bcm63xx_hsspi_set_cs(bs, spi->chip_select, false);
diff --git a/drivers/spi/spi-cavium.c b/drivers/spi/spi-cavium.c
index 5aaf21582cb5..db7f5f94c525 100644
--- a/drivers/spi/spi-cavium.c
+++ b/drivers/spi/spi-cavium.c
@@ -119,8 +119,7 @@ static int octeon_spi_do_transfer(struct octeon_spi *p,
 			*rx_buf++ = (u8)v;
 		}
 
-	if (xfer->delay_usecs)
-		udelay(xfer->delay_usecs);
+	spi_transfer_delay(xfer);
 
 	return xfer->len;
 }
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 53335ccc98f6..0b761352c16a 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -752,8 +752,7 @@ static int dspi_transfer_one_message(struct spi_master *master,
 			dspi->waitflags = 0;
 		}
 
-		if (transfer->delay_usecs)
-			udelay(transfer->delay_usecs);
+		spi_transfer_delay(transfer);
 	}
 
 out:
diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index f20326714b9d..a3af12127c40 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -427,8 +427,7 @@ static int fsl_espi_trans(struct spi_message *m, struct spi_transfer *trans)
 
 	ret = fsl_espi_bufs(spi, trans);
 
-	if (trans->delay_usecs)
-		udelay(trans->delay_usecs);
+	spi_transfer_delay(trans);
 
 	return ret;
 }
diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c
index 1d9b33aa1a3b..9352b127d5b8 100644
--- a/drivers/spi/spi-fsl-spi.c
+++ b/drivers/spi/spi-fsl-spi.c
@@ -417,8 +417,7 @@ static int fsl_spi_do_one_msg(struct spi_master *master,
 		}
 		m->actual_length += t->len;
 
-		if (t->delay_usecs)
-			udelay(t->delay_usecs);
+		spi_transfer_delay(t);
 
 		if (cs_change) {
 			ndelay(nsecs);
diff --git a/drivers/spi/spi-mpc512x-psc.c b/drivers/spi/spi-mpc512x-psc.c
index a337b842ae8c..329bbdd1f15f 100644
--- a/drivers/spi/spi-mpc512x-psc.c
+++ b/drivers/spi/spi-mpc512x-psc.c
@@ -311,8 +311,7 @@ static int mpc512x_psc_spi_msg_xfer(struct spi_master *master,
 			break;
 		m->actual_length += t->len;
 
-		if (t->delay_usecs)
-			udelay(t->delay_usecs);
+		spi_transfer_delay(t);
 
 		if (cs_change)
 			mpc512x_psc_spi_deactivate_cs(spi);
diff --git a/drivers/spi/spi-mpc52xx-psc.c b/drivers/spi/spi-mpc52xx-psc.c
index c7e478b9b586..dd79d7fd3687 100644
--- a/drivers/spi/spi-mpc52xx-psc.c
+++ b/drivers/spi/spi-mpc52xx-psc.c
@@ -234,8 +234,7 @@ static void mpc52xx_psc_spi_work(struct work_struct *work)
 				break;
 			m->actual_length += t->len;
 
-			if (t->delay_usecs)
-				udelay(t->delay_usecs);
+			spi_transfer_delay(t);
 
 			if (cs_change)
 				mpc52xx_psc_spi_deactivate_cs(spi);
diff --git a/drivers/spi/spi-omap-100k.c b/drivers/spi/spi-omap-100k.c
index b955ca8796d2..eb5b51bd5126 100644
--- a/drivers/spi/spi-omap-100k.c
+++ b/drivers/spi/spi-omap-100k.c
@@ -321,8 +321,7 @@ static int omap1_spi100k_transfer_one_message(struct spi_master *master,
 			}
 		}
 
-		if (t->delay_usecs)
-			udelay(t->delay_usecs);
+		spi_transfer_delay(t);
 
 		/* ignore the "leave it on after last xfer" hint */
 
diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index 7fedea67159c..42cf800073e4 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -485,12 +485,11 @@ static void giveback(struct pl022 *pl022)
 					struct spi_transfer, transfer_list);
 
 	/* Delay if requested before any change in chip select */
-	if (last_transfer->delay_usecs)
-		/*
-		 * FIXME: This runs in interrupt context.
-		 * Is this really smart?
-		 */
-		udelay(last_transfer->delay_usecs);
+	/*
+	 * FIXME: This runs in interrupt context.
+	 * Is this really smart?
+	 */
+	spi_transfer_delay(last_transfer);
 
 	if (!last_transfer->cs_change) {
 		struct spi_message *next_msg;
@@ -1401,12 +1400,11 @@ static void pump_transfers(unsigned long data)
 		previous = list_entry(transfer->transfer_list.prev,
 					struct spi_transfer,
 					transfer_list);
-		if (previous->delay_usecs)
-			/*
-			 * FIXME: This runs in interrupt context.
-			 * Is this really smart?
-			 */
-			udelay(previous->delay_usecs);
+		/*
+		 * FIXME: This runs in interrupt context.
+		 * Is this really smart?
+		 */
+		spi_transfer_delay(previous);
 
 		/* Reselect chip select only if cs_change was requested */
 		if (previous->cs_change)
@@ -1520,8 +1518,7 @@ static void do_polling_transfer(struct pl022 *pl022)
 			previous =
 			    list_entry(transfer->transfer_list.prev,
 				       struct spi_transfer, transfer_list);
-			if (previous->delay_usecs)
-				udelay(previous->delay_usecs);
+			spi_transfer_delay(previous);
 			if (previous->cs_change)
 				pl022_cs_control(pl022, SSP_CHIP_SELECT);
 		} else {
diff --git a/drivers/spi/spi-sc18is602.c b/drivers/spi/spi-sc18is602.c
index 11acddc83304..13cb992df712 100644
--- a/drivers/spi/spi-sc18is602.c
+++ b/drivers/spi/spi-sc18is602.c
@@ -211,8 +211,7 @@ static int sc18is602_transfer_one(struct spi_master *master,
 		}
 		status = 0;
 
-		if (t->delay_usecs)
-			udelay(t->delay_usecs);
+		spi_transfer_delay(t);
 	}
 	m->status = status;
 	spi_finalize_current_message(master);
diff --git a/drivers/spi/spi-sh-hspi.c b/drivers/spi/spi-sh-hspi.c
index 7f73f91d412a..4edad22c222e 100644
--- a/drivers/spi/spi-sh-hspi.c
+++ b/drivers/spi/spi-sh-hspi.c
@@ -190,8 +190,7 @@ static int hspi_transfer_one_message(struct spi_controller *ctlr,
 
 		msg->actual_length += t->len;
 
-		if (t->delay_usecs)
-			udelay(t->delay_usecs);
+		spi_transfer_delay(t);
 
 		if (cs_change) {
 			ndelay(nsecs);
diff --git a/drivers/spi/spi-tegra20-sflash.c b/drivers/spi/spi-tegra20-sflash.c
index cd714a4f52c6..00593096cfed 100644
--- a/drivers/spi/spi-tegra20-sflash.c
+++ b/drivers/spi/spi-tegra20-sflash.c
@@ -344,7 +344,7 @@ static int tegra_sflash_transfer_one_message(struct spi_master *master,
 		if (xfer->cs_change && xfer->delay_usecs) {
 			tegra_sflash_writel(tsd, tsd->def_command_reg,
 					SPI_COMMAND);
-			udelay(xfer->delay_usecs);
+			spi_transfer_delay(xfer);
 		}
 	}
 	ret = 0;
diff --git a/drivers/spi/spi-topcliff-pch.c b/drivers/spi/spi-topcliff-pch.c
index f88cbb94ce12..645b086eece8 100644
--- a/drivers/spi/spi-topcliff-pch.c
+++ b/drivers/spi/spi-topcliff-pch.c
@@ -1229,12 +1229,7 @@ static void pch_spi_process_messages(struct work_struct *pwork)
 			"%s:data->current_msg->actual_length=%d\n",
 			__func__, data->current_msg->actual_length);
 
-		/* check for delay */
-		if (data->cur_trans->delay_usecs) {
-			dev_dbg(&data->master->dev, "%s:delay in usec=%d\n",
-				__func__, data->cur_trans->delay_usecs);
-			udelay(data->cur_trans->delay_usecs);
-		}
+		spi_transfer_delay(data->cur_trans);
 
 		spin_lock(&data->lock);
 
diff --git a/drivers/spi/spi-txx9.c b/drivers/spi/spi-txx9.c
index 51759d3fd45f..65aa736783ae 100644
--- a/drivers/spi/spi-txx9.c
+++ b/drivers/spi/spi-txx9.c
@@ -248,8 +248,7 @@ static void txx9spi_work_one(struct txx9spi *c, struct spi_message *m)
 			len -= count * wsize;
 		}
 		m->actual_length += t->len;
-		if (t->delay_usecs)
-			udelay(t->delay_usecs);
+		spi_transfer_delay(t);
 
 		if (!cs_change)
 			continue;
diff --git a/drivers/spi/spi-xcomm.c b/drivers/spi/spi-xcomm.c
index a3496c46cc1b..3aaae32b0384 100644
--- a/drivers/spi/spi-xcomm.c
+++ b/drivers/spi/spi-xcomm.c
@@ -188,8 +188,7 @@ static int spi_xcomm_transfer_one(struct spi_master *master,
 		}
 		status = 0;
 
-		if (t->delay_usecs)
-			udelay(t->delay_usecs);
+		spi_transfer_delay(t);
 
 		is_first = false;
 	}
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [RFC PATCH 09/15] spi: introduce `delay` field for `spi_transfer` + spi_transfer_exec()
From: Alexandru Ardelean @ 2019-09-13 11:45 UTC (permalink / raw)
  To: linux-spi, linux-iio, linux-arm-kernel, linux-tegra, linux-kernel,
	bcm-kernel-feedback-list
  Cc: f.fainelli, baolin.wang, zhang.lyra, linus.walleij, broonie,
	orsonzhai, Alexandru Ardelean, jic23
In-Reply-To: <20190913114550.956-1-alexandru.ardelean@analog.com>

The change introduces the `delay` field to the `spi_transfer` struct as an
`struct spi_delay` type.
This intends to eventually replace `delay_usecs`.

But, since there are many users of `delay_usecs`, this needs some
intermediate work.
A helper called `spi_transfer_delay()` is also added, which maintains
backwards compatibility with `delay_usecs`, by assigning the value to
`delay` if non-zero.
This should maintain backwards compatibility with current users of
`udelay_usecs`.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/spi/spi.c       |  6 +++---
 include/linux/spi/spi.h | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index ba8731b77753..c55d55acbbb0 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1238,8 +1238,7 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,
 		if (msg->status != -EINPROGRESS)
 			goto out;
 
-		if (xfer->delay_usecs)
-			_spi_transfer_delay_ns(xfer->delay_usecs * 1000);
+		spi_transfer_delay(xfer);
 
 		if (xfer->cs_change) {
 			if (list_is_last(&xfer->transfer_list,
@@ -2904,10 +2903,11 @@ struct spi_replaced_transfers *spi_replace_transfers(
 		/* add to list */
 		list_add(&xfer->transfer_list, rxfer->replaced_after);
 
-		/* clear cs_change and delay_usecs for all but the last */
+		/* clear cs_change and delay for all but the last */
 		if (i) {
 			xfer->cs_change = false;
 			xfer->delay_usecs = 0;
+			xfer->delay.value = 0;
 		}
 	}
 
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index e7ec0be16a82..c7f90a0129be 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -754,6 +754,9 @@ extern void spi_res_release(struct spi_controller *ctlr,
  * @cs_change: affects chipselect after this transfer completes
  * @cs_change_delay: delay between cs deassert and assert when
  *      @cs_change is set and @spi_transfer is not the last in @spi_message
+ * @delay: delay to be introduced after this transfer before
+ *	(optionally) changing the chipselect status, then starting
+ *	the next transfer or completing this @spi_message.
  * @delay_usecs: microseconds to delay after this transfer before
  *	(optionally) changing the chipselect status, then starting
  *	the next transfer or completing this @spi_message.
@@ -843,6 +846,7 @@ struct spi_transfer {
 #define	SPI_NBITS_QUAD		0x04 /* 4bits transfer */
 	u8		bits_per_word;
 	u16		delay_usecs;
+	struct spi_delay	delay;
 	struct spi_delay	cs_change_delay;
 	struct spi_delay	word_delay;
 	u32		speed_hz;
@@ -942,6 +946,20 @@ spi_transfer_del(struct spi_transfer *t)
 	list_del(&t->transfer_list);
 }
 
+static inline int
+spi_transfer_delay(struct spi_transfer *t)
+{
+	struct spi_delay d;
+
+	if (t->delay_usecs) {
+		d.value = t->delay_usecs;
+		d.unit = SPI_DELAY_UNIT_USECS;
+		return spi_delay_exec(&d, NULL);
+	}
+
+	return spi_delay_exec(&t->delay, t);
+}
+
 /**
  * spi_message_init_with_transfers - Initialize spi_message and append transfers
  * @m: spi_message to be initialized
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [RFC PATCH 08/15] spi: core, atmel: convert `word_delay_usecs` -> `word_delay` for spi_device
From: Alexandru Ardelean @ 2019-09-13 11:45 UTC (permalink / raw)
  To: linux-spi, linux-iio, linux-arm-kernel, linux-tegra, linux-kernel,
	bcm-kernel-feedback-list
  Cc: f.fainelli, baolin.wang, zhang.lyra, linus.walleij, broonie,
	orsonzhai, Alexandru Ardelean, jic23
In-Reply-To: <20190913114550.956-1-alexandru.ardelean@analog.com>

This change does a conversion from the `word_delay_usecs` -> `word_delay`
for the `spi_device` struct.

This allows users to specify inter-word delays in other unit types
(nano-seconds or clock cycles), depending on how users want.

The Atmel SPI driver is the only current user of the `word_delay_usecs`
field (from the `spi_device` struct).
So, it needed a slight conversion to use the `word_delay` as an `spi_delay`
struct.

In SPI core, the only required mechanism is to update the `word_delay`
information per `spi_transfer`. This requires a bit more logic than before,
because it needs that both delays be converted to a common unit
(nano-seconds) for comparison.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/spi/spi-atmel.c | 26 ++++++++++++++++++++++++--
 drivers/spi/spi.c       | 24 ++++++++++++++++++++++--
 include/linux/spi/spi.h |  7 ++-----
 3 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index f00b367523cd..009bb4fb5e5f 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -1172,12 +1172,31 @@ atmel_spi_pdc_interrupt(int irq, void *dev_id)
 	return ret;
 }
 
+static int atmel_word_delay_csr(struct spi_device *spi, struct atmel_spi *as)
+{
+	struct spi_delay *delay = &spi->word_delay;
+	u32 value = delay->value;
+
+	switch (delay->unit) {
+	case SPI_DELAY_UNIT_NSECS:
+		value /= 1000;
+		break;
+	case SPI_DELAY_UNIT_USECS:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return (as->spi_clk / 1000000 * value) >> 5;
+}
+
 static int atmel_spi_setup(struct spi_device *spi)
 {
 	struct atmel_spi	*as;
 	struct atmel_spi_device	*asd;
 	u32			csr;
 	unsigned int		bits = spi->bits_per_word;
+	int			word_delay_csr;
 
 	as = spi_master_get_devdata(spi->master);
 
@@ -1201,11 +1220,14 @@ static int atmel_spi_setup(struct spi_device *spi)
 	 */
 	csr |= SPI_BF(DLYBS, 0);
 
+	word_delay_csr = atmel_word_delay_csr(as, spi);
+	if (word_delay_csr < 0)
+		return word_delay_csr;
+
 	/* DLYBCT adds delays between words.  This is useful for slow devices
 	 * that need a bit of time to setup the next transfer.
 	 */
-	csr |= SPI_BF(DLYBCT,
-			(as->spi_clk / 1000000 * spi->word_delay_usecs) >> 5);
+	csr |= SPI_BF(DLYBCT, word_delay_csr);
 
 	asd = spi->controller_state;
 	if (!asd) {
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index d0bf0ffca042..ba8731b77753 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3159,6 +3159,26 @@ void spi_set_cs_timing(struct spi_device *spi, u8 setup, u8 hold,
 }
 EXPORT_SYMBOL_GPL(spi_set_cs_timing);
 
+static int _spi_xfer_word_delay_update(struct spi_transfer *xfer,
+				       struct spi_device *spi)
+{
+	int delay1, delay2;
+
+	delay1 = _spi_delay_to_ns(&xfer->word_delay, xfer);
+	if (delay1 < 0)
+		return delay1;
+
+	delay2 = _spi_delay_to_ns(&spi->word_delay, xfer);
+	if (delay2 < 0)
+		return delay2;
+
+	if (delay1 < delay2)
+		memcpy(&xfer->word_delay, &spi->word_delay,
+		       sizeof(xfer->word_delay));
+
+	return 0;
+}
+
 static int __spi_validate(struct spi_device *spi, struct spi_message *message)
 {
 	struct spi_controller *ctlr = spi->controller;
@@ -3294,8 +3314,8 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message)
 				return -EINVAL;
 		}
 
-		if (xfer->word_delay_usecs < spi->word_delay_usecs)
-			xfer->word_delay_usecs = spi->word_delay_usecs;
+		if (_spi_xfer_word_delay_update(xfer, spi))
+			return -EINVAL;
 	}
 
 	message->status = -EINPROGRESS;
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 16397b210e0c..e7ec0be16a82 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -138,7 +138,7 @@ extern int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer);
  *	the spi_master.
  * @cs_gpiod: gpio descriptor of the chipselect line (optional, NULL when
  *	not using a GPIO line)
- * @word_delay_usecs: microsecond delay to be inserted between consecutive
+ * @word_delay: delay to be inserted between consecutive
  *	words of a transfer
  *
  * @statistics: statistics for the spi_device
@@ -188,7 +188,7 @@ struct spi_device {
 	const char		*driver_override;
 	int			cs_gpio;	/* LEGACY: chip select gpio */
 	struct gpio_desc	*cs_gpiod;	/* chip select gpio desc */
-	uint8_t			word_delay_usecs; /* inter-word delay */
+	struct spi_delay	word_delay; /* inter-word delay */
 
 	/* the statistics */
 	struct spi_statistics	statistics;
@@ -757,8 +757,6 @@ extern void spi_res_release(struct spi_controller *ctlr,
  * @delay_usecs: microseconds to delay after this transfer before
  *	(optionally) changing the chipselect status, then starting
  *	the next transfer or completing this @spi_message.
- * @word_delay_usecs: microseconds to inter word delay after each word size
- *	(set by bits_per_word) transmission.
  * @word_delay: inter word delay to be introduced after each word size
  *	(set by bits_per_word) transmission.
  * @effective_speed_hz: the effective SCK-speed that was used to
@@ -844,7 +842,6 @@ struct spi_transfer {
 #define	SPI_NBITS_DUAL		0x02 /* 2bits transfer */
 #define	SPI_NBITS_QUAD		0x04 /* 4bits transfer */
 	u8		bits_per_word;
-	u8		word_delay_usecs;
 	u16		delay_usecs;
 	struct spi_delay	cs_change_delay;
 	struct spi_delay	word_delay;
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [RFC PATCH 07/15] spi: spidev: use new `word_delay` field for spi transfers
From: Alexandru Ardelean @ 2019-09-13 11:45 UTC (permalink / raw)
  To: linux-spi, linux-iio, linux-arm-kernel, linux-tegra, linux-kernel,
	bcm-kernel-feedback-list
  Cc: f.fainelli, baolin.wang, zhang.lyra, linus.walleij, broonie,
	orsonzhai, Alexandru Ardelean, jic23
In-Reply-To: <20190913114550.956-1-alexandru.ardelean@analog.com>

The `word_delay` field had it's type changed to `struct spi_delay`.
This allows users to specify nano-second or clock-cycle delays (if needed).

Converting to use `word_delay` is straightforward: it's just assigning the
value to `word_delay.value` and hard-coding the `word_delay.unit` to
`SPI_DELAY_UNIT_USECS`

This keeps the uapi for spidev un-changed. Changing it can be part of
another changeset and discussion.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/spi/spidev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index 255786f2e844..495319c2fa11 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -267,7 +267,8 @@ static int spidev_message(struct spidev_data *spidev,
 		k_tmp->bits_per_word = u_tmp->bits_per_word;
 		k_tmp->delay_usecs = u_tmp->delay_usecs;
 		k_tmp->speed_hz = u_tmp->speed_hz;
-		k_tmp->word_delay_usecs = u_tmp->word_delay_usecs;
+		k_tmp->word_delay.value = u_tmp->word_delay_usecs;
+		k_tmp->word_delay.unit = SPI_DELAY_UNIT_USECS;
 		if (!k_tmp->speed_hz)
 			k_tmp->speed_hz = spidev->speed_hz;
 #ifdef VERBOSE
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [RFC PATCH 06/15] spi: orion: use new `word_delay` field for SPI transfers
From: Alexandru Ardelean @ 2019-09-13 11:45 UTC (permalink / raw)
  To: linux-spi, linux-iio, linux-arm-kernel, linux-tegra, linux-kernel,
	bcm-kernel-feedback-list
  Cc: f.fainelli, baolin.wang, zhang.lyra, linus.walleij, broonie,
	orsonzhai, Alexandru Ardelean, jic23
In-Reply-To: <20190913114550.956-1-alexandru.ardelean@analog.com>

The `word_delay` field had it's type changed to `struct spi_delay`.
This allows users to specify nano-second or clock-cycle delays (if needed).

Converting to use `word_delay` is straightforward: it just uses the new
`spi_delay_exec()` routine, that handles the `unit` part.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/spi/spi-orion.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-orion.c b/drivers/spi/spi-orion.c
index 6643ccdc2508..756c37e32dfe 100644
--- a/drivers/spi/spi-orion.c
+++ b/drivers/spi/spi-orion.c
@@ -467,8 +467,7 @@ orion_spi_write_read(struct spi_device *spi, struct spi_transfer *xfer)
 			if (orion_spi_write_read_8bit(spi, &tx, &rx) < 0)
 				goto out;
 			count--;
-			if (xfer->word_delay_usecs)
-				udelay(xfer->word_delay_usecs);
+			spi_delay_exec(&xfer->word_delay, xfer);
 		} while (count);
 	} else if (word_len == 16) {
 		const u16 *tx = xfer->tx_buf;
@@ -478,8 +477,7 @@ orion_spi_write_read(struct spi_device *spi, struct spi_transfer *xfer)
 			if (orion_spi_write_read_16bit(spi, &tx, &rx) < 0)
 				goto out;
 			count -= 2;
-			if (xfer->word_delay_usecs)
-				udelay(xfer->word_delay_usecs);
+			spi_delay_exec(&xfer->word_delay, xfer);
 		} while (count);
 	}
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [RFC PATCH 05/15] spi: sprd: convert transfer word delay to spi_delay struct
From: Alexandru Ardelean @ 2019-09-13 11:45 UTC (permalink / raw)
  To: linux-spi, linux-iio, linux-arm-kernel, linux-tegra, linux-kernel,
	bcm-kernel-feedback-list
  Cc: f.fainelli, baolin.wang, zhang.lyra, linus.walleij, broonie,
	orsonzhai, Alexandru Ardelean, jic23
In-Reply-To: <20190913114550.956-1-alexandru.ardelean@analog.com>

The Spreadtrum SPI driver is the only user of the `word_delay` field in
the `spi_transfer` struct.

This change converts the field to use the `spi_delay` struct. This also
enforces the users to specify the delay unit to be `SPI_DELAY_UNIT_SCK`.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/spi/spi-sprd.c  | 11 +++++++++--
 include/linux/spi/spi.h |  4 ++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-sprd.c b/drivers/spi/spi-sprd.c
index 1b7eebb72c07..5634cd3e4a8d 100644
--- a/drivers/spi/spi-sprd.c
+++ b/drivers/spi/spi-sprd.c
@@ -671,9 +671,13 @@ static void sprd_spi_set_speed(struct sprd_spi *ss, u32 speed_hz)
 
 static void sprd_spi_init_hw(struct sprd_spi *ss, struct spi_transfer *t)
 {
+	struct spi_delay *d = &t->word_delay;
 	u16 word_delay, interval;
 	u32 val;
 
+	if (t->unit != SPI_DELAY_UNIT_SCK)
+		return -EINVAL;
+
 	val = readl_relaxed(ss->base + SPRD_SPI_CTL7);
 	val &= ~(SPRD_SPI_SCK_REV | SPRD_SPI_NG_TX | SPRD_SPI_NG_RX);
 	/* Set default chip selection, clock phase and clock polarity */
@@ -686,7 +690,7 @@ static void sprd_spi_init_hw(struct sprd_spi *ss, struct spi_transfer *t)
 	 * formula as below per datasheet:
 	 * interval time (source clock cycles) = interval * 4 + 10.
 	 */
-	word_delay = clamp_t(u16, t->word_delay, SPRD_SPI_MIN_DELAY_CYCLE,
+	word_delay = clamp_t(u16, d->value, SPRD_SPI_MIN_DELAY_CYCLE,
 			     SPRD_SPI_MAX_DELAY_CYCLE);
 	interval = DIV_ROUND_UP(word_delay - 10, 4);
 	ss->word_delay = interval * 4 + 10;
@@ -719,13 +723,16 @@ static int sprd_spi_setup_transfer(struct spi_device *sdev,
 	struct sprd_spi *ss = spi_controller_get_devdata(sdev->controller);
 	u8 bits_per_word = t->bits_per_word;
 	u32 val, mode = 0;
+	int ret;
 
 	ss->len = t->len;
 	ss->tx_buf = t->tx_buf;
 	ss->rx_buf = t->rx_buf;
 
 	ss->hw_mode = sdev->mode;
-	sprd_spi_init_hw(ss, t);
+	ret = sprd_spi_init_hw(ss, t);
+	if (ret)
+		return ret;
 
 	/* Set tansfer speed and valid bits */
 	sprd_spi_set_speed(ss, t->speed_hz);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 9ded3f44d58e..16397b210e0c 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -759,7 +759,7 @@ extern void spi_res_release(struct spi_controller *ctlr,
  *	the next transfer or completing this @spi_message.
  * @word_delay_usecs: microseconds to inter word delay after each word size
  *	(set by bits_per_word) transmission.
- * @word_delay: clock cycles to inter word delay after each word size
+ * @word_delay: inter word delay to be introduced after each word size
  *	(set by bits_per_word) transmission.
  * @effective_speed_hz: the effective SCK-speed that was used to
  *      transfer this transfer. Set to 0 if the spi bus driver does
@@ -847,8 +847,8 @@ struct spi_transfer {
 	u8		word_delay_usecs;
 	u16		delay_usecs;
 	struct spi_delay	cs_change_delay;
+	struct spi_delay	word_delay;
 	u32		speed_hz;
-	u16		word_delay;
 
 	u32		effective_speed_hz;
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [RFC PATCH 04/15] iio: imu: adis: convert cs_change_delay to spi_delay struct
From: Alexandru Ardelean @ 2019-09-13 11:45 UTC (permalink / raw)
  To: linux-spi, linux-iio, linux-arm-kernel, linux-tegra, linux-kernel,
	bcm-kernel-feedback-list
  Cc: f.fainelli, baolin.wang, zhang.lyra, linus.walleij, broonie,
	orsonzhai, Alexandru Ardelean, jic23
In-Reply-To: <20190913114550.956-1-alexandru.ardelean@analog.com>

The ADIS library is one of the few users of the new `cs_change_delay`
parameter for an spi_transfer.

The introduction of the `spi_delay` struct, requires that the users of of
`cs_change_delay` get an update. This change updates the ADIS library.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/imu/adis.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 1631c255deab..2cd2cc2316c6 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -39,24 +39,24 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
-			.cs_change_delay = adis->data->cs_change_delay,
-			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+			.cs_change_delay.value = adis->data->cs_change_delay,
+			.cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
 		}, {
 			.tx_buf = adis->tx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
-			.cs_change_delay = adis->data->cs_change_delay,
-			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+			.cs_change_delay.value = adis->data->cs_change_delay,
+			.cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
 		}, {
 			.tx_buf = adis->tx + 4,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
-			.cs_change_delay = adis->data->cs_change_delay,
-			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+			.cs_change_delay.value = adis->data->cs_change_delay,
+			.cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
 		}, {
 			.tx_buf = adis->tx + 6,
 			.bits_per_word = 8,
@@ -139,16 +139,16 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
-			.cs_change_delay = adis->data->cs_change_delay,
-			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+			.cs_change_delay.value = adis->data->cs_change_delay,
+			.cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
 		}, {
 			.tx_buf = adis->tx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->read_delay,
-			.cs_change_delay = adis->data->cs_change_delay,
-			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+			.cs_change_delay.value = adis->data->cs_change_delay,
+			.cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
 		}, {
 			.tx_buf = adis->tx + 4,
 			.rx_buf = adis->rx,
@@ -156,8 +156,8 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->read_delay,
-			.cs_change_delay = adis->data->cs_change_delay,
-			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+			.cs_change_delay.value = adis->data->cs_change_delay,
+			.cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
 		}, {
 			.rx_buf = adis->rx + 2,
 			.bits_per_word = 8,
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [RFC PATCH 03/15] spi: make `cs_change_delay` the first user of the `spi_delay` logic
From: Alexandru Ardelean @ 2019-09-13 11:45 UTC (permalink / raw)
  To: linux-spi, linux-iio, linux-arm-kernel, linux-tegra, linux-kernel,
	bcm-kernel-feedback-list
  Cc: f.fainelli, baolin.wang, zhang.lyra, linus.walleij, broonie,
	orsonzhai, Alexandru Ardelean, jic23
In-Reply-To: <20190913114550.956-1-alexandru.ardelean@analog.com>

Since the logic for `spi_delay` struct + `spi_delay_exec()` has been copied
from the `cs_change_delay` logic, it's natural to make this delay, the
first user.

The `cs_change_delay` logic requires that the default remain 10 uS, in case
it is unspecified/unconfigured. So, there is some special handling needed
to do that.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/spi/spi.c       | 28 +++++++---------------------
 include/linux/spi/spi.h |  4 +---
 2 files changed, 8 insertions(+), 24 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 1883de8ffa82..d0bf0ffca042 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1160,9 +1160,9 @@ EXPORT_SYMBOL_GPL(spi_delay_exec);
 static void _spi_transfer_cs_change_delay(struct spi_message *msg,
 					  struct spi_transfer *xfer)
 {
-	u32 delay = xfer->cs_change_delay;
-	u32 unit = xfer->cs_change_delay_unit;
-	u32 hz;
+	u32 delay = xfer->cs_change_delay.value;
+	u32 unit = xfer->cs_change_delay.unit;
+	int ret;
 
 	/* return early on "fast" mode - for everything but USECS */
 	if (!delay) {
@@ -1171,27 +1171,13 @@ static void _spi_transfer_cs_change_delay(struct spi_message *msg,
 		return;
 	}
 
-	switch (unit) {
-	case SPI_DELAY_UNIT_USECS:
-		delay *= 1000;
-		break;
-	case SPI_DELAY_UNIT_NSECS: /* nothing to do here */
-		break;
-	case SPI_DELAY_UNIT_SCK:
-		/* if there is no effective speed know, then approximate
-		 * by underestimating with half the requested hz
-		 */
-		hz = xfer->effective_speed_hz ?: xfer->speed_hz / 2;
-		delay *= DIV_ROUND_UP(1000000000, hz);
-		break;
-	default:
+	ret = spi_delay_exec(&xfer->cs_change_delay, xfer);
+	if (ret) {
 		dev_err_once(&msg->spi->dev,
 			     "Use of unsupported delay unit %i, using default of 10us\n",
-			     xfer->cs_change_delay_unit);
-		delay = 10000;
+			     unit);
+		_spi_transfer_delay_ns(10000);
 	}
-	/* now sleep for the requested amount of time */
-	_spi_transfer_delay_ns(delay);
 }
 
 /*
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index c18cfa7cda35..9ded3f44d58e 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -754,7 +754,6 @@ extern void spi_res_release(struct spi_controller *ctlr,
  * @cs_change: affects chipselect after this transfer completes
  * @cs_change_delay: delay between cs deassert and assert when
  *      @cs_change is set and @spi_transfer is not the last in @spi_message
- * @cs_change_delay_unit: unit of cs_change_delay
  * @delay_usecs: microseconds to delay after this transfer before
  *	(optionally) changing the chipselect status, then starting
  *	the next transfer or completing this @spi_message.
@@ -847,8 +846,7 @@ struct spi_transfer {
 	u8		bits_per_word;
 	u8		word_delay_usecs;
 	u16		delay_usecs;
-	u16		cs_change_delay;
-	u8		cs_change_delay_unit;
+	struct spi_delay	cs_change_delay;
 	u32		speed_hz;
 	u16		word_delay;
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [RFC PATCH 02/15] spi: introduce spi_delay struct as "value + unit" & spi_delay_exec()
From: Alexandru Ardelean @ 2019-09-13 11:45 UTC (permalink / raw)
  To: linux-spi, linux-iio, linux-arm-kernel, linux-tegra, linux-kernel,
	bcm-kernel-feedback-list
  Cc: f.fainelli, baolin.wang, zhang.lyra, linus.walleij, broonie,
	orsonzhai, Alexandru Ardelean, jic23
In-Reply-To: <20190913114550.956-1-alexandru.ardelean@analog.com>

There are plenty of delays that have been introduced in SPI core. Most of
them are in micro-seconds, some need to be in nano-seconds, and some in
clock-cycles.

For some of these delays (related to transfers & CS timing) it may make
sense to have a `spi_delay` struct that abstracts these a bit.

The important element of these delays [for unification] seems to be the
`unit` of the delay.
It looks like micro-seconds is good enough for most people, but every-once
in a while, some delays seem to require other units of measurement.

This change adds the `spi_delay` struct & a `spi_delay_exec()` function
that processes a `spi_delay` object/struct to execute the delay.
It's a copy of the `cs_change_delay` mechanism, but without the default
for 10 uS.

The clock-cycle delay unit is a bit special, as it needs to be bound to an
`spi_transfer` object to execute.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/spi/spi.c       | 51 +++++++++++++++++++++++++++++++++++++++++
 include/linux/spi/spi.h | 18 ++++++++++++---
 2 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index c90e02e6d62f..1883de8ffa82 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1106,6 +1106,57 @@ static void _spi_transfer_delay_ns(u32 ns)
 	}
 }
 
+static int _spi_delay_to_ns(struct spi_delay *_delay, struct spi_transfer *xfer)
+{
+	u32 delay = _delay->value;
+	u32 unit = _delay->unit;
+	u32 hz;
+
+	if (!delay)
+		return 0;
+
+	switch (unit) {
+	case SPI_DELAY_UNIT_USECS:
+		delay *= 1000;
+		break;
+	case SPI_DELAY_UNIT_NSECS: /* nothing to do here */
+		break;
+	case SPI_DELAY_UNIT_SCK:
+		/* clock cycles need to be obtained from spi_transfer */
+		if (!xfer)
+			return -EINVAL;
+		/* if there is no effective speed know, then approximate
+		 * by underestimating with half the requested hz
+		 */
+		hz = xfer->effective_speed_hz ?: xfer->speed_hz / 2;
+		if (!hz)
+			return -EINVAL;
+		delay *= DIV_ROUND_UP(1000000000, hz);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return delay;
+}
+
+int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer)
+{
+	int delay;
+
+	if (!_delay)
+		return -EINVAL;
+
+	delay = _spi_delay_to_ns(_delay, xfer);
+	if (delay < 0)
+		return delay;
+
+	_spi_transfer_delay_ns(delay);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spi_delay_exec);
+
 static void _spi_transfer_cs_change_delay(struct spi_message *msg,
 					  struct spi_transfer *xfer)
 {
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index af4f265d0f67..c18cfa7cda35 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -89,6 +89,21 @@ void spi_statistics_add_transfer_stats(struct spi_statistics *stats,
 #define SPI_STATISTICS_INCREMENT_FIELD(stats, field)	\
 	SPI_STATISTICS_ADD_TO_FIELD(stats, field, 1)
 
+/**
+ * struct spi_delay - SPI delay information
+ * @value: Value for the delay
+ * @unit: Unit for the delay
+ */
+struct spi_delay {
+#define SPI_DELAY_UNIT_USECS	0
+#define SPI_DELAY_UNIT_NSECS	1
+#define SPI_DELAY_UNIT_SCK	2
+	u16	value;
+	u8	unit;
+};
+
+extern int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer);
+
 /**
  * struct spi_device - Controller side proxy for an SPI slave device
  * @dev: Driver model representation of the device.
@@ -834,9 +849,6 @@ struct spi_transfer {
 	u16		delay_usecs;
 	u16		cs_change_delay;
 	u8		cs_change_delay_unit;
-#define SPI_DELAY_UNIT_USECS	0
-#define SPI_DELAY_UNIT_NSECS	1
-#define SPI_DELAY_UNIT_SCK	2
 	u32		speed_hz;
 	u16		word_delay;
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [RFC PATCH 00/15] Unify SPI delays into an `struct spi_delay`
From: Alexandru Ardelean @ 2019-09-13 11:45 UTC (permalink / raw)
  To: linux-spi, linux-iio, linux-arm-kernel, linux-tegra, linux-kernel,
	bcm-kernel-feedback-list
  Cc: f.fainelli, baolin.wang, zhang.lyra, linus.walleij, broonie,
	orsonzhai, Alexandru Ardelean, jic23

Initially, I started this patchset thinking: "we need a new delay for
something-something" (in case someone is curios, we need a CS-hold-time for
the first transfer, because the CS wakes a chip from sleep-mode).

Then I added the delay, and felt a bit dirty-inside about adding a new one
(just like that), and decided to look at maybe cleaning things up a bit,
and a few days later, I got here.

Full disclaimer: this patchset is not complete. It's an RFC.
It's based on top of Jonathan's `iio/togreg` branch which also includes the
ADIS driver library changes and also includes `cs_change_delay`.

I'll send a V2 patchset, which just the first 4 patches, since I feel that
those are a bit more complete.

I thought about just sending the first 4 patches on-their-own, but I
figured that the whole series (even if not complete) serves as a better
explanation about the whole "why?".

Hopefully, this can sort-of-explain things.
I'll reference this RFC on the next series.

Thanks

Alexandru Ardelean (15):
  spi: move `cs_change_delay` backwards compat logic outside switch
  spi: introduce spi_delay struct as "value + unit" &  spi_delay_exec()
  spi: make `cs_change_delay` the first user of the `spi_delay` logic
  iio: imu: adis: convert cs_change_delay to spi_delay struct
  spi: sprd: convert transfer word delay to spi_delay struct
  spi: orion: use new `word_delay` field for SPI transfers
  spi: spidev: use new `word_delay` field for spi transfers
  spi: core,atmel: convert `word_delay_usecs` -> `word_delay` for
    spi_device
  spi: introduce `delay` field for `spi_transfer` + spi_transfer_exec()
  spi: use new `spi_transfer_delay` helper where straightforward
  spi: tegra114: use `spi_transfer_delay` helper
  spi: spi-loopback-test: use new `delay` field
  spi: spidev: use new `delay` field for spi transfers
  spi: tegra114: change format for `spi_set_cs_timing()` function
  spi: implement SW control for CS times

 drivers/iio/imu/adis.c           |  24 ++---
 drivers/spi/spi-atmel.c          |  29 +++++-
 drivers/spi/spi-bcm63xx-hsspi.c  |   3 +-
 drivers/spi/spi-cavium.c         |   3 +-
 drivers/spi/spi-fsl-dspi.c       |   3 +-
 drivers/spi/spi-fsl-espi.c       |   3 +-
 drivers/spi/spi-fsl-spi.c        |   3 +-
 drivers/spi/spi-loopback-test.c  |  12 ++-
 drivers/spi/spi-mpc512x-psc.c    |   3 +-
 drivers/spi/spi-mpc52xx-psc.c    |   3 +-
 drivers/spi/spi-omap-100k.c      |   3 +-
 drivers/spi/spi-orion.c          |   6 +-
 drivers/spi/spi-pl022.c          |  25 +++--
 drivers/spi/spi-sc18is602.c      |   3 +-
 drivers/spi/spi-sh-hspi.c        |   3 +-
 drivers/spi/spi-sprd.c           |  11 ++-
 drivers/spi/spi-tegra114.c       |  39 +++++---
 drivers/spi/spi-tegra20-sflash.c |   2 +-
 drivers/spi/spi-topcliff-pch.c   |   7 +-
 drivers/spi/spi-txx9.c           |   3 +-
 drivers/spi/spi-xcomm.c          |   3 +-
 drivers/spi/spi.c                | 162 +++++++++++++++++++++++++------
 drivers/spi/spidev.c             |   6 +-
 include/linux/spi/spi.h          |  65 ++++++++++---
 24 files changed, 293 insertions(+), 131 deletions(-)

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [RFC PATCH 01/15] spi: move `cs_change_delay` backwards compat logic outside switch
From: Alexandru Ardelean @ 2019-09-13 11:45 UTC (permalink / raw)
  To: linux-spi, linux-iio, linux-arm-kernel, linux-tegra, linux-kernel,
	bcm-kernel-feedback-list
  Cc: f.fainelli, baolin.wang, zhang.lyra, linus.walleij, broonie,
	orsonzhai, Alexandru Ardelean, jic23
In-Reply-To: <20190913114550.956-1-alexandru.ardelean@analog.com>

The `cs_change_delay` backwards compatibility value could be moved outside
of the switch statement.
The only reason to do it, is to make the next patches easier to diff.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/spi/spi.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 75ac046cae52..c90e02e6d62f 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1114,16 +1114,15 @@ static void _spi_transfer_cs_change_delay(struct spi_message *msg,
 	u32 hz;
 
 	/* return early on "fast" mode - for everything but USECS */
-	if (!delay && unit != SPI_DELAY_UNIT_USECS)
+	if (!delay) {
+		if (unit == SPI_DELAY_UNIT_USECS)
+			_spi_transfer_delay_ns(10000);
 		return;
+	}
 
 	switch (unit) {
 	case SPI_DELAY_UNIT_USECS:
-		/* for compatibility use default of 10us */
-		if (!delay)
-			delay = 10000;
-		else
-			delay *= 1000;
+		delay *= 1000;
 		break;
 	case SPI_DELAY_UNIT_NSECS: /* nothing to do here */
 		break;
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v3] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Neil Armstrong @ 2019-09-13  8:46 UTC (permalink / raw)
  To: Jonas Karlman, Cheng-yi Chiang
  Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	Doug Anderson, kuninori.morimoto.gx@renesas.com, David Airlie,
	dri-devel@lists.freedesktop.org, cain.cai@rock-chips.com,
	Andrzej Hajda, Laurent Pinchart, Yakir Yang, sam@ravnborg.org,
	Jerome Brunet, Xing Zheng, linux-rockchip@lists.infradead.org,
	Dylan Reid, tzungbi@chromium.org, Jeffy Chen, 蔡枫,
	linux-arm-kernel@lists.infradead.org, Jernej Škrabec,
	linux-kernel, Daniel Vetter, Enric Balletbo i Serra,
	kuankuan.y@gmail.com
In-Reply-To: <HE1PR06MB4011478532D8E127697565EDACB30@HE1PR06MB4011.eurprd06.prod.outlook.com>

On 13/09/2019 08:37, Jonas Karlman wrote:
> On 2019-09-11 19:02, Cheng-yi Chiang wrote:
>> On Thu, Sep 12, 2019 at 12:54 AM Jernej Škrabec <jernej.skrabec@siol.net> wrote:
>>> Dne sreda, 11. september 2019 ob 18:23:59 CEST je Neil Armstrong napisal(a):
>>>> On 11/09/2019 10:26, Cheng-Yi Chiang wrote:
>>>>> From: Yakir Yang <ykk@rock-chips.com>
>>>>>
>>>>> When transmitting IEC60985 linear PCM audio, we configure the
>>>>> Aduio Sample Channel Status information in the IEC60958 frame.
>>>>> The status bit is already available in iec.status of hdmi_codec_params.
>>>>>
>>>>> This fix the issue that audio does not come out on some monitors
>>>>> (e.g. LG 22CV241)
>>>>>
>>>>> Note that these registers are only for interfaces:
>>>>> I2S audio interface, General Purpose Audio (GPA), or AHB audio DMA
>>>>> (AHBAUDDMA).
>>>>> For S/PDIF interface this information comes from the stream.
>>>>>
>>>>> Currently this function dw_hdmi_set_channel_status is only called
>>>>> from dw-hdmi-i2s-audio in I2S setup.
>>>>>
>>>>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>>>>> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
>>>>> ---
>>>>>
>>>>> Change from v2 to v3:
>>>>> 1. Reuse what is already set in iec.status in hw_param.
>>>>> 2. Remove all useless definition of registers and values.
>>>>> 3. Note that the original sampling frequency is not written to
>>>>>
>>>>>    the channel status as we reuse create_iec958_consumer in pcm_iec958.c.
>>>>>    Without that it can still play audio fine.
>>>>>
>>>>>  .../drm/bridge/synopsys/dw-hdmi-i2s-audio.c   |  1 +
>>>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c     | 20 +++++++++++++++++++
>>>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.h     |  2 ++
>>>>>  include/drm/bridge/dw_hdmi.h                  |  1 +
>>>>>  4 files changed, 24 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
>>>>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c index
>>>>> 34d8e837555f..20f4f92dd866 100644
>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
>>>>> @@ -102,6 +102,7 @@ static int dw_hdmi_i2s_hw_params(struct device *dev,
>>>>> void *data,>
>>>>>     }
>>>>>
>>>>>     dw_hdmi_set_sample_rate(hdmi, hparms->sample_rate);
>>>>>
>>>>> +   dw_hdmi_set_channel_status(hdmi, hparms->iec.status);
>>>>>
>>>>>     dw_hdmi_set_channel_count(hdmi, hparms->channels);
>>>>>     dw_hdmi_set_channel_allocation(hdmi, hparms-
>>>> cea.channel_allocation);
>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
>>>>> bd65d0479683..aa7efd4da1c8 100644
>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> @@ -582,6 +582,26 @@ static unsigned int hdmi_compute_n(unsigned int freq,
>>>>> unsigned long pixel_clk)>
>>>>>     return n;
>>>>>
>>>>>  }
>>>>>
>>>>> +/*
>>>>> + * When transmitting IEC60958 linear PCM audio, these registers allow to
>>>>> + * configure the channel status information of all the channel status
>>>>> + * bits in the IEC60958 frame. For the moment this configuration is only
>>>>> + * used when the I2S audio interface, General Purpose Audio (GPA),
>>>>> + * or AHB audio DMA (AHBAUDDMA) interface is active
>>>>> + * (for S/PDIF interface this information comes from the stream).
>>>>> + */
>>>>> +void dw_hdmi_set_channel_status(struct dw_hdmi *hdmi,
>>>>> +                           u8 *channel_status)
>>>>> +{
>>>>> +   /*
>>>>> +    * Set channel status register for frequency and word length.
>>>>> +    * Use default values for other registers.
>>>>> +    */
>>>>> +   hdmi_writeb(hdmi, channel_status[3], HDMI_FC_AUDSCHNLS7);
>>>>> +   hdmi_writeb(hdmi, channel_status[4], HDMI_FC_AUDSCHNLS8);
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(dw_hdmi_set_channel_status);
>>>>> +
>>>>>
>>>>>  static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
>>>>>
>>>>>     unsigned long pixel_clk, unsigned int sample_rate)
>>>>>
>>>>>  {
>>>>>
>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
>>>>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h index
>>>>> 6988f12d89d9..fcff5059db24 100644
>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
>>>>> @@ -158,6 +158,8 @@
>>>>>
>>>>>  #define HDMI_FC_SPDDEVICEINF                    0x1062
>>>>>  #define HDMI_FC_AUDSCONF                        0x1063
>>>>>  #define HDMI_FC_AUDSSTAT                        0x1064
>>>>>
>>>>> +#define HDMI_FC_AUDSCHNLS7                      0x106e
>>>>> +#define HDMI_FC_AUDSCHNLS8                      0x106f
>>>>>
>>>>>  #define HDMI_FC_DATACH0FILL                     0x1070
>>>>>  #define HDMI_FC_DATACH1FILL                     0x1071
>>>>>  #define HDMI_FC_DATACH2FILL                     0x1072
>>>>>
>>>>> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
>>>>> index cf528c289857..4b3e863c4f8a 100644
>>>>> --- a/include/drm/bridge/dw_hdmi.h
>>>>> +++ b/include/drm/bridge/dw_hdmi.h
>>>>> @@ -156,6 +156,7 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool
>>>>> hpd, bool rx_sense);>
>>>>>  void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
>>>>>  void dw_hdmi_set_channel_count(struct dw_hdmi *hdmi, unsigned int cnt);
>>>>>
>>>>> +void dw_hdmi_set_channel_status(struct dw_hdmi *hdmi, u8
>>>>> *channel_status);
>>>>>
>>>>>  void dw_hdmi_set_channel_allocation(struct dw_hdmi *hdmi, unsigned int
>>>>>  ca);
>>>>>  void dw_hdmi_audio_enable(struct dw_hdmi *hdmi);
>>>>>  void dw_hdmi_audio_disable(struct dw_hdmi *hdmi);
>>>> Looks fine for me:
>>>> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
>>>>
>>>> Jonas ? Jernej ? Russell ?
>>> Patch itself is fine, I'm just wondering if more information should be copied
>>> from status array to registers. But I think they are not 1:1 mapping so some
>>> more work would be needed. Anyway, patch is:
>> Hi Jernej,
>> Yes you are right. I was thinking about the same thing.
>> But there are also some fields in the IEC60958 spec not mapped to the
>> registers on dw-hdmi.
>> So I ended up just writing the two registers in the original ykk's
>> patch, and ignoring "original sampling frequency" like pcm_iec958.
>> It turns out that audio plays fine on my LG monitor. So I suggest we
>> can keep this patch as simple as it is, and add more register setting
>> if we find issue.
>> Thanks!
> 
> In my old multi-channel lpcm patch [1] I only wrote sample rate to FC_AUDSCHNLS7.
> This is much cleaner and simpler, and setting FC_AUDSCHNLS8 does not cause any
> problems when I tested on ASUS Tinker Board S (RK3288).
> 
> Reviewed-by: Jonas Karlman <jonas@kwiboo.se>
> 
> 
> [1] https://github.com/Kwiboo/linux-rockchip/commit/4af9ebc567ccf0a0851fa260097021c27aebbb6b

Thanks Jonas, Jernej,

Applying now.

Neil

> 
> Regards,
> Jonas
> 
>>
>>>
>>> Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>
>>>
>>> Best regards,
>>> Jernej
>>>
>>>> If it's ok for you I'll apply it.
>>>>
>>>> Neil
>>>>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] mm/pgtable/debug: Fix test validating architecture page table helpers
From: Anshuman Khandual @ 2019-09-13  8:42 UTC (permalink / raw)
  To: Christophe Leroy, linux-mm
  Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
	Heiko Carstens, Michal Hocko, Dave Hansen, Paul Mackerras,
	sparclinux, Thomas Gleixner, linux-s390, Jason Gunthorpe, x86,
	Russell King - ARM Linux, Matthew Wilcox, Steven Price,
	Tetsuo Handa, Gerald Schaefer, linux-snps-arc, Kees Cook,
	Mark Brown, Kirill A . Shutemov, Dan Williams, Vlastimil Babka,
	linux-arm-kernel, Sri Krishna chowdary, Masahiro Yamada,
	Greg Kroah-Hartman, Ard Biesheuvel, linux-mips, Ralf Baechle,
	linux-kernel, Paul Burton, Mike Rapoport, Vineet Gupta,
	Martin Schwidefsky, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <cb226b56-ff20-3136-7ffb-890657e56870@c-s.fr>



On 09/13/2019 12:41 PM, Christophe Leroy wrote:
> 
> 
> Le 13/09/2019 à 09:03, Christophe Leroy a écrit :
>>
>>
>> Le 13/09/2019 à 08:58, Anshuman Khandual a écrit :
>>> On 09/13/2019 11:53 AM, Christophe Leroy wrote:
>>>> Fix build failure on powerpc.
>>>>
>>>> Fix preemption imbalance.
>>>>
>>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>>> ---
>>>>   mm/arch_pgtable_test.c | 3 +++
>>>>   1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/mm/arch_pgtable_test.c b/mm/arch_pgtable_test.c
>>>> index 8b4a92756ad8..f2b3c9ec35fa 100644
>>>> --- a/mm/arch_pgtable_test.c
>>>> +++ b/mm/arch_pgtable_test.c
>>>> @@ -24,6 +24,7 @@
>>>>   #include <linux/swap.h>
>>>>   #include <linux/swapops.h>
>>>>   #include <linux/sched/mm.h>
>>>> +#include <linux/highmem.h>
>>>
>>> This is okay.
>>>
>>>>   #include <asm/pgalloc.h>
>>>>   #include <asm/pgtable.h>
>>>> @@ -400,6 +401,8 @@ static int __init arch_pgtable_tests_init(void)
>>>>       p4d_clear_tests(p4dp);
>>>>       pgd_clear_tests(mm, pgdp);
>>>> +    pte_unmap(ptep);
>>>> +
>>>
>>> Now the preemption imbalance via pte_alloc_map() path i.e
>>>
>>> pte_alloc_map() -> pte_offset_map() -> kmap_atomic()
>>>
>>> Is not this very much powerpc 32 specific or this will be applicable
>>> for all platform which uses kmap_XXX() to map high memory ?
>>>
>>
>> See https://elixir.bootlin.com/linux/v5.3-rc8/source/include/linux/highmem.h#L91
>>
>> I think it applies at least to all arches using the generic implementation.
>>
>> Applies also to arm:
>> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/arm/mm/highmem.c#L52
>>
>> Applies also to mips:
>> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/mips/mm/highmem.c#L47
>>
>> Same on sparc:
>> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/sparc/mm/highmem.c#L52
>>
>> Same on x86:
>> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/x86/mm/highmem_32.c#L34
>>
>> I have not checked others, but I guess it is like that for all.
>>
> 
> 
> Seems like I answered too quickly. All kmap_atomic() do preempt_disable(), but not all pte_alloc_map() call kmap_atomic().
> 
> However, for instance ARM does:
> 
> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/arm/include/asm/pgtable.h#L200
> 
> And X86 as well:
> 
> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/x86/include/asm/pgtable_32.h#L51
> 
> Microblaze also:
> 
> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/microblaze/include/asm/pgtable.h#L495

All the above platforms checks out to be using k[un]map_atomic(). I am wondering whether
any of the intermediate levels will have similar problems on any these 32 bit platforms
or any other platforms which might be using generic k[un]map_atomic(). There can be many
permutations here.

	p4dp = p4d_alloc(mm, pgdp, vaddr);
	pudp = pud_alloc(mm, p4dp, vaddr);
	pmdp = pmd_alloc(mm, pudp, vaddr);

Otherwise pte_alloc_map()/pte_unmap() looks good enough which will atleast take care of
a known failure.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2] arm: dts: imx6qdl: add gpio expander pca9535
From: Gilles Doffe @ 2019-09-13  8:38 UTC (permalink / raw)
  To: Marco Felsch
  Cc: mark rutland, devicetree, shawnguo, s hauer, rennes, linux-kernel,
	robh+dt, linux-imx, kernel, Jérome Oufella, festevam,
	linux-arm-kernel
In-Reply-To: <20190912101240.ml5jmdei5rvzesap@pengutronix.de>

Hi Marco,

Ack for all, v3 incoming.

Thank you,
Gilles

----- Le 12 Sep 19, à 12:12, Marco Felsch m.felsch@pengutronix.de a écrit :

> Hi Gilles,
> 
> On 19-09-12 06:01, Gilles Doffe wrote:
>> Hi Marco,
>> 
>> Thanks for your reply and sorry about the delay.
> 
> No worries ;)
> 
>> ----- Le 22 Juil 19, à 9:53, Marco Felsch m.felsch@pengutronix.de a écrit :
>> 
>> > Hi Gilles,
>> > 
>> > can you adapt the patch title, I assumed that the base dtsi is adding a
>> > gpio-expander which makes no sense.
>> 
>> My first intent was to add the gpio-expander pca9535 into the imx6q-rex-pro.dts
>> and in a future imx6qp-rex-ultra.dts
>> However I noticed that the sgtl5000 was already in the dtsi.
>> It is maybe due to the fact that like the pca9535, the sgtl5000 is present on
>> the baseboard not on the SOM.
>> Thus I guess that baseboard stuff common to all rex SOM should be in
>> imx6qdl-rex.dtsi and not in the dts.
>> Does-it seem correct to you ?
> 
> Yes this is correct what Shawn and I mean is that you should adapt the
> commit title. Shawn already give you an example.
> 
>> > 
>> > On 19-07-19 12:46, Gilles DOFFE wrote:
>> >> The pca9535 gpio expander is present on the Rex baseboard, but missing
>> >> from the dtsi.
>> >> 
>> >> Add the new gpio controller and the associated interrupt line
>> >> MX6QDL_PAD_NANDF_CS3__GPIO6_IO16.
>> >> 
>> >> Signed-off-by: Gilles DOFFE <gilles.doffe@savoirfairelinux.com>
>> >> ---
>> > 
>> > Having a changelog would be nice too.
>> > 
>> >>  arch/arm/boot/dts/imx6qdl-rex.dtsi | 19 +++++++++++++++++++
>> >>  1 file changed, 19 insertions(+)
>> >> 
>> >> diff --git a/arch/arm/boot/dts/imx6qdl-rex.dtsi
>> >> b/arch/arm/boot/dts/imx6qdl-rex.dtsi
>> >> index 97f1659144ea..b517efb22fcb 100644
>> >> --- a/arch/arm/boot/dts/imx6qdl-rex.dtsi
>> >> +++ b/arch/arm/boot/dts/imx6qdl-rex.dtsi
>> >> @@ -136,6 +136,19 @@
>> >>  		compatible = "atmel,24c02";
>> >>  		reg = <0x57>;
>> >>  	};
>> >> +
>> >> +	pca9535: gpio8@27 {
>> >> +		compatible = "nxp,pca9535";
>> >> +		reg = <0x27>;
>> > 
>> > The i2c devices are orderd by their i2c-addresses starting from the
>> > lowest.
>> >
>> 
>> Ack.
>> 
>> >> +		gpio-controller;
>> >> +		#gpio-cells = <2>;
>> >> +		pinctrl-names = "default";
>> >> +		pinctrl-0 = <&pinctrl_pca9535>;
>> >> +		interrupt-parent = <&gpio6>;
>> >> +		interrupts = <16 IRQ_TYPE_LEVEL_LOW>;
>> >> +		interrupt-controller;
>> >> +		#interrupt-cells = <2>;
> 
> As you pointed out above this device isn't available on the
> imx6dl-rex-basic? You should add: 'status = "disabled";' if this is the
> case.
> 
> Regards,
>  Marco
> 
>> >> +	};
>> >>  };
>> >>  
>> >>  &i2c3 {
>> >> @@ -237,6 +250,12 @@
>> >>  			>;
>> >>  		};
>> >>  
>> >> +		pinctrl_pca9535: pca9535 {
>> >> +			fsl,pins = <
>> >> +				MX6QDL_PAD_NANDF_CS3__GPIO6_IO16	0x00017059
>> > 
>> > The pinmux below don't use the leading zero's if you are the first I
>> > would drop that.
>> > 
>> > Regards,
>> >  Marco
>> >
>> 
>> Ack.
>> 
>> Regards,
>> Gilles
>> 
> 
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

-- 
Gilles DOFFE 
Senior Product Engineering Consultant | Rennes, Fr 
Bureau 
[ tel:+33972468980 | (+33) 9 72 46 89 80 ] p. : 601 
Cellulaire 
[ tel:+33660025866 | (+33) 6 60 02 58 66 ]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 9/9] sunxi_defconfig: add new crypto options
From: Corentin Labbe @ 2019-09-13  8:15 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: mark.rutland, devicetree, herbert, linux-sunxi, linux,
	linux-kernel, wens, robh+dt, linux-crypto, davem,
	linux-arm-kernel
In-Reply-To: <20190907040353.hrz7gmqgzpfpo4xj@flea>

On Sat, Sep 07, 2019 at 07:03:53AM +0300, Maxime Ripard wrote:
> On Fri, Sep 06, 2019 at 08:45:51PM +0200, Corentin Labbe wrote:
> > This patch adds the new allwinner crypto configs to sunxi_defconfig
> >
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > ---
> >  arch/arm/configs/sunxi_defconfig | 2 ++
> >  1 file changed, 2 insertions(+)
> 
> Can you also enable it in arm64's defconfig as a module?
> 

Does you prefer adding a Kconfig "DEFAULT m if ARCH_SUNXI" which permit to not touch any defconfig ?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v5 0/4] Raspberry Pi 4 DMA addressing support
From: Matthias Brugger @ 2019-09-13  8:09 UTC (permalink / raw)
  To: Stefan Wahren, catalin.marinas, marc.zyngier, Matthias Brugger,
	robh+dt, linux-mm, linux-arm-kernel, linux-riscv, hch,
	Nicolas Saenz Julienne
  Cc: f.fainelli, robin.murphy, linux-kernel, linux-rpi-kernel, phill,
	will, m.szyprowski
In-Reply-To: <2c3e1ef3-0dba-9f79-52e2-314b6b500e14@gmx.net>



On 12/09/2019 21:32, Stefan Wahren wrote:
> 
> Am 12.09.19 um 19:18 schrieb Matthias Brugger:
>>
>> On 10/09/2019 11:27, Matthias Brugger wrote:
>>>
>>> On 09/09/2019 21:33, Stefan Wahren wrote:
>>>> Hi Nicolas,
>>>>
>>>> Am 09.09.19 um 11:58 schrieb Nicolas Saenz Julienne:
>>>>> Hi all,
>>>>> this series attempts to address some issues we found while bringing up
>>>>> the new Raspberry Pi 4 in arm64 and it's intended to serve as a follow
>>>>> up of these discussions:
>>>>> v4: https://lkml.org/lkml/2019/9/6/352
>>>>> v3: https://lkml.org/lkml/2019/9/2/589
>>>>> v2: https://lkml.org/lkml/2019/8/20/767
>>>>> v1: https://lkml.org/lkml/2019/7/31/922
>>>>> RFC: https://lkml.org/lkml/2019/7/17/476
>>>>>
>>>>> The new Raspberry Pi 4 has up to 4GB of memory but most peripherals can
>>>>> only address the first GB: their DMA address range is
>>>>> 0xc0000000-0xfc000000 which is aliased to the first GB of physical
>>>>> memory 0x00000000-0x3c000000. Note that only some peripherals have these
>>>>> limitations: the PCIe, V3D, GENET, and 40-bit DMA channels have a wider
>>>>> view of the address space by virtue of being hooked up trough a second
>>>>> interconnect.
>>>>>
>>>>> Part of this is solved on arm32 by setting up the machine specific
>>>>> '.dma_zone_size = SZ_1G', which takes care of reserving the coherent
>>>>> memory area at the right spot. That said no buffer bouncing (needed for
>>>>> dma streaming) is available at the moment, but that's a story for
>>>>> another series.
>>>>>
>>>>> Unfortunately there is no such thing as 'dma_zone_size' in arm64. Only
>>>>> ZONE_DMA32 is created which is interpreted by dma-direct and the arm64
>>>>> arch code as if all peripherals where be able to address the first 4GB
>>>>> of memory.
>>>>>
>>>>> In the light of this, the series implements the following changes:
>>>>>
>>>>> - Create both DMA zones in arm64, ZONE_DMA will contain the first 1G
>>>>>   area and ZONE_DMA32 the rest of the 32 bit addressable memory. So far
>>>>>   the RPi4 is the only arm64 device with such DMA addressing limitations
>>>>>   so this hardcoded solution was deemed preferable.
>>>>>
>>>>> - Properly set ARCH_ZONE_DMA_BITS.
>>>>>
>>>>> - Reserve the CMA area in a place suitable for all peripherals.
>>>>>
>>>>> This series has been tested on multiple devices both by checking the
>>>>> zones setup matches the expectations and by double-checking physical
>>>>> addresses on pages allocated on the three relevant areas GFP_DMA,
>>>>> GFP_DMA32, GFP_KERNEL:
>>>>>
>>>>> - On an RPi4 with variations on the ram memory size. But also forcing
>>>>>   the situation where all three memory zones are nonempty by setting a 3G
>>>>>   ZONE_DMA32 ceiling on a 4G setup. Both with and without NUMA support.
>>>>>
>>>> i like to test this series on Raspberry Pi 4 and i have some questions
>>>> to get arm64 running:
>>>>
>>>> Do you use U-Boot? Which tree?
>>> If you want to use U-Boot, try v2019.10-rc4, it should have everything you need
>>> to boot your kernel.
>>>
>> Ok, here is a thing. In the linux kernel we now use bcm2711 as SoC name, but the
>> RPi4 devicetree provided by the FW uses mostly bcm2838.
> 
> Do you mean the DTB provided at runtime?
> 
> You mean the merged U-Boot changes, doesn't work with my Raspberry Pi
> series?
> 
>>  U-Boot in its default
>> config uses the devicetree provided by the FW, mostly because this way you don't
>> have to do anything to find out how many RAM you really have. Secondly because
>> this will allow us, in the near future, to have one U-boot binary for both RPi3
>> and RPi4 (and as a side effect one binary for RPi1 and RPi2).
>>
>> Anyway, I found at least, that the following compatibles need to be added:
>>
>> "brcm,bcm2838-cprman"
>> "brcm,bcm2838-gpio"
>>
>> Without at least the cprman driver update, you won't see anything.
>>
>> "brcm,bcm2838-rng200" is also a candidate.
>>
>> I also suppose we will need to add "brcm,bcm2838" to
>> arch/arm/mach-bcm/bcm2711.c, but I haven't verified this.
> How about changing this in the downstream kernel? Which is much easier.

I'm not sure I understand what you want to say. My goal is to use the upstream
kernel with the device tree blob provided by the FW. If you talk about the
downstream kernel, I suppose you mean we should change this in the FW DT blob
and in the downstream kernel. That would work for me.

Did I understand you correctly?

>>
>> Regards,
>> Matthias
>>
>>> Regards,
>>> Matthias
>>>
>>>> Are there any config.txt tweaks necessary?
>>>>
>>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 3/6] ARM: dts: sunxi: h3/h5: Add MBUS controller node
From: Maxime Ripard @ 2019-09-13  7:57 UTC (permalink / raw)
  To: Jernej Škrabec
  Cc: mark.rutland, devicetree, linux-sunxi, linux-kernel, hverkuil,
	wens, robh+dt, mchehab, linux-arm-kernel, linux-media
In-Reply-To: <5193854.KWMFve8tAi@jernej-laptop>

On Thu, Sep 12, 2019 at 10:46:58PM +0200, Jernej Škrabec wrote:
> Dne četrtek, 12. september 2019 ob 22:34:27 CEST je Maxime Ripard napisal(a):
> > On Thu, Sep 12, 2019 at 10:28:37PM +0200, Jernej Škrabec wrote:
> > > Dne četrtek, 12. september 2019 ob 22:20:57 CEST je Maxime Ripard 
> napisal(a):
> > > > Hi,
> > > > 
> > > > On Thu, Sep 12, 2019 at 07:51:29PM +0200, Jernej Skrabec wrote:
> > > > > Both, H3 and H5, contain MBUS, which is the bus used by DMA devices to
> > > > > access system memory.
> > > > > 
> > > > > MBUS controller is responsible for arbitration between channels based
> > > > > on set priority and can do some other things as well, like report
> > > > > bandwidth used. It also maps RAM region to different address than CPU.
> > > > > 
> > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > > > ---
> > > > > 
> > > > >  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 9 +++++++++
> > > > >  1 file changed, 9 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> > > > > b/arch/arm/boot/dts/sunxi-h3-h5.dtsi index eba190b3f9de..ef1d03812636
> > > > > 100644
> > > > > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> > > > > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> > > > > @@ -109,6 +109,7 @@
> > > > > 
> > > > >  		compatible = "simple-bus";
> > > > >  		#address-cells = <1>;
> > > > >  		#size-cells = <1>;
> > > > > 
> > > > > +		dma-ranges;
> > > > > 
> > > > >  		ranges;
> > > > >  		
> > > > >  		display_clocks: clock@1000000 {
> > > > > 
> > > > > @@ -538,6 +539,14 @@
> > > > > 
> > > > >  			};
> > > > >  		
> > > > >  		};
> > > > > 
> > > > > +		mbus: dram-controller@1c62000 {
> > > > > +			compatible = "allwinner,sun8i-h3-mbus";
> > > > > +			reg = <0x01c62000 0x1000>;
> > > > > +			clocks = <&ccu 113>;
> > > > > +			dma-ranges = <0x00000000 0x40000000
> > > 
> > > 0xc0000000>;
> > > 
> > > > > +			#interconnect-cells = <1>;
> > > > > +		};
> > > > > +
> > > > 
> > > > If that's easy enough to access, can you also add the references in
> > > > the devices that are already there? (CSI and DE comes to my mind, but
> > > > there might be others).
> > > 
> > > Strangely, DE2 doesn't use this offset. That was tested on OrangePi
> > > Plus2E,
> > > which has 2 GiB of RAM and subtracting this offset causes corrupted image.
> > 
> > Ok, weird. But if it was tested then fine by me :)
> > 
> > > But I can add this properties to CSI too. However, wouldn't that need CSI
> > > DT binding expansion with those properties? othetwise DT check will fail.
> > Oh right, we definitely need to update the binding indeed. The code
> > should be able to cope with both cases already.
> 
> I guess it's better to handle that with another patch series then? Changing 
> CSI bindings doesn't fit here.

Yeah, you can do it in a separate series if you prefer

Maxime

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [V2, 2/2] media: i2c: Add more sensor modes for ov8856 camera sensor
From: Sakari Ailus @ 2019-09-13  7:55 UTC (permalink / raw)
  To: dongchun.zhu
  Cc: mark.rutland, drinkcat, andriy.shevchenko, srv_heupstream,
	devicetree, shengnan.wang, tfiga, louis.kuo, sj.huang, robh+dt,
	linux-mediatek, matthias.bgg, bingbu.cao, mchehab,
	linux-arm-kernel, linux-media
In-Reply-To: <20190910130446.26413-3-dongchun.zhu@mediatek.com>

Hi Dongchun,

On Tue, Sep 10, 2019 at 09:04:46PM +0800, dongchun.zhu@mediatek.com wrote:
> From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> 
> This patch mainly adds two more sensor modes for OV8856 CMOS image sensor.
> That is, the resolution of 1632*1224 and 3264*2448, corresponding to the bayer order of BGGR.
> The sensor revision also differs in some OTP register.
> 
> Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> ---
>  drivers/media/i2c/ov8856.c | 654 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 639 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
> index cd347d6..9ad0b73 100644
> --- a/drivers/media/i2c/ov8856.c
> +++ b/drivers/media/i2c/ov8856.c
> @@ -1,12 +1,15 @@
>  // SPDX-License-Identifier: GPL-2.0
>  // Copyright (c) 2019 Intel Corporation.
>  
> +#include <linux/clk.h>
>  #include <asm/unaligned.h>
>  #include <linux/acpi.h>
>  #include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/i2c.h>
>  #include <linux/module.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-fwnode.h>
> @@ -18,10 +21,15 @@
>  #define OV8856_LINK_FREQ_360MHZ		360000000ULL
>  #define OV8856_LINK_FREQ_180MHZ		180000000ULL
>  #define OV8856_SCLK			144000000ULL
> -#define OV8856_MCLK			19200000
> +#define OV8856_XVCLK			19200000
> +#define OV8856_XVCLK_TYP		24000000

How about calling these e.g. OV8856_XCLK_19_2 and OV8856_CLK_24? Or just
use the plain numbers; there's nothing really special about them.

Ideally you'd have both in a single location in the driver --- an array of
modes supported on each of them as they're different.

>  #define OV8856_DATA_LANES		4
>  #define OV8856_RGB_DEPTH		10
>  
> +#define REG_X_ADDR_START		0x3808
> +#define X_OUTPUT_FULL_SIZE		0x0cc0
> +#define X_OUTPUT_BINNING_SIZE		0x0660
> +
>  #define OV8856_REG_CHIP_ID		0x300a
>  #define OV8856_CHIP_ID			0x00885a
>  
> @@ -29,6 +37,22 @@
>  #define OV8856_MODE_STANDBY		0x00
>  #define OV8856_MODE_STREAMING		0x01
>  
> +/* define 1B module revision */
> +#define OV8856_1B_MODULE		0x02
> +
> +/* the OTP read-out buffer is at 0x7000 and 0xf is the offset
> + * of the byte in the OTP that means the module revision
> + */
> +#define OV8856_MODULE_REVISION		0x700f
> +#define OV8856_OTP_MODE_CTRL		0x3d84
> +#define OV8856_OTP_LOAD_CTRL		0x3d81
> +#define OV8856_OTP_MODE_AUTO		0x00
> +#define OV8856_OTP_LOAD_CTRL_ENABLE	BIT(0)
> +
> +/* Analog control register that decided by module revision */
> +#define OV8856_ANAL_MODE_CTRL		0x3614
> +#define OV8856_ANAL_1B_VAL		0x20
> +
>  /* vertical-timings from sensor */
>  #define OV8856_REG_VTS			0x380e
>  #define OV8856_VTS_MAX			0x7fff
> @@ -64,6 +88,14 @@
>  
>  #define to_ov8856(_sd)			container_of(_sd, struct ov8856, sd)
>  
> +static const char * const ov8856_supply_names[] = {
> +	"dovdd",	/* Digital I/O power */
> +	"avdd",		/* Analog power */
> +	"dvdd",		/* Digital core power */
> +};
> +
> +#define OV8856_NUM_SUPPLIES ARRAY_SIZE(ov8856_supply_names)
> +
>  enum {
>  	OV8856_LINK_FREQ_720MBPS,
>  	OV8856_LINK_FREQ_360MBPS,
> @@ -195,11 +227,11 @@ static const struct ov8856_reg mode_3280x2464_regs[] = {
>  	{0x3800, 0x00},
>  	{0x3801, 0x00},
>  	{0x3802, 0x00},
> -	{0x3803, 0x06},
> +	{0x3803, 0x07},
>  	{0x3804, 0x0c},
>  	{0x3805, 0xdf},
>  	{0x3806, 0x09},
> -	{0x3807, 0xa7},
> +	{0x3807, 0xa6},

This is changing an existing sensor mode. What are the effects of the
change?

Given that this register is not documented in the driver in any way, I'd
expect the commit message to carry some reasoning behind the change, also
suggesting it belongs to a separate patch. The same applies to changes to
the other existing modes.

>  	{0x3808, 0x0c},
>  	{0x3809, 0xd0},
>  	{0x380a, 0x09},
> @@ -211,7 +243,7 @@ static const struct ov8856_reg mode_3280x2464_regs[] = {
>  	{0x3810, 0x00},
>  	{0x3811, 0x00},
>  	{0x3812, 0x00},
> -	{0x3813, 0x01},
> +	{0x3813, 0x00},
>  	{0x3814, 0x01},
>  	{0x3815, 0x01},
>  	{0x3816, 0x00},
> @@ -316,6 +348,209 @@ static const struct ov8856_reg mode_3280x2464_regs[] = {
>  	{0x5e00, 0x00}
>  };
>  
> +static const struct ov8856_reg mode_3264x2448_regs[] = {
> +	{0x0103, 0x01},
> +	{0x0302, 0x3c},
> +	{0x0303, 0x01},
> +	{0x031e, 0x0c},
> +	{0x3000, 0x20},
> +	{0x3003, 0x08},
> +	{0x300e, 0x20},
> +	{0x3010, 0x00},
> +	{0x3015, 0x84},
> +	{0x3018, 0x72},
> +	{0x3021, 0x23},
> +	{0x3033, 0x24},
> +	{0x3500, 0x00},
> +	{0x3501, 0x9a},
> +	{0x3502, 0x20},
> +	{0x3503, 0x08},
> +	{0x3505, 0x83},
> +	{0x3508, 0x01},
> +	{0x3509, 0x80},
> +	{0x350c, 0x00},
> +	{0x350d, 0x80},
> +	{0x350e, 0x04},
> +	{0x350f, 0x00},
> +	{0x3510, 0x00},
> +	{0x3511, 0x02},
> +	{0x3512, 0x00},
> +	{0x3600, 0x72},
> +	{0x3601, 0x40},
> +	{0x3602, 0x30},
> +	{0x3610, 0xc5},
> +	{0x3611, 0x58},
> +	{0x3612, 0x5c},
> +	{0x3613, 0xca},
> +	{0x3614, 0x60},
> +	{0x3628, 0xff},
> +	{0x3629, 0xff},
> +	{0x362a, 0xff},
> +	{0x3633, 0x10},
> +	{0x3634, 0x10},
> +	{0x3635, 0x10},
> +	{0x3636, 0x10},
> +	{0x3663, 0x08},
> +	{0x3669, 0x34},
> +	{0x366d, 0x00},
> +	{0x366e, 0x10},
> +	{0x3706, 0x86},
> +	{0x370b, 0x7e},
> +	{0x3714, 0x23},
> +	{0x3730, 0x12},
> +	{0x3733, 0x10},
> +	{0x3764, 0x00},
> +	{0x3765, 0x00},
> +	{0x3769, 0x62},
> +	{0x376a, 0x2a},
> +	{0x376b, 0x30},
> +	{0x3780, 0x00},
> +	{0x3781, 0x24},
> +	{0x3782, 0x00},
> +	{0x3783, 0x23},
> +	{0x3798, 0x2f},
> +	{0x37a1, 0x60},
> +	{0x37a8, 0x6a},
> +	{0x37ab, 0x3f},
> +	{0x37c2, 0x04},
> +	{0x37c3, 0xf1},
> +	{0x37c9, 0x80},
> +	{0x37cb, 0x16},
> +	{0x37cc, 0x16},
> +	{0x37cd, 0x16},
> +	{0x37ce, 0x16},
> +	{0x3800, 0x00},
> +	{0x3801, 0x00},
> +	{0x3802, 0x00},
> +	{0x3803, 0x0c},
> +	{0x3804, 0x0c},
> +	{0x3805, 0xdf},
> +	{0x3806, 0x09},
> +	{0x3807, 0xa3},
> +	{0x3808, 0x0c},
> +	{0x3809, 0xc0},
> +	{0x380a, 0x09},
> +	{0x380b, 0x90},
> +	{0x380c, 0x07},
> +	{0x380d, 0x8c},
> +	{0x380e, 0x09},
> +	{0x380f, 0xb2},
> +	{0x3810, 0x00},
> +	{0x3811, 0x04},
> +	{0x3812, 0x00},
> +	{0x3813, 0x02},
> +	{0x3814, 0x01},
> +	{0x3815, 0x01},
> +	{0x3816, 0x00},
> +	{0x3817, 0x00},
> +	{0x3818, 0x00},
> +	{0x3819, 0x00},
> +	{0x3820, 0x80},
> +	{0x3821, 0x46},
> +	{0x382a, 0x01},
> +	{0x382b, 0x01},
> +	{0x3830, 0x06},
> +	{0x3836, 0x02},
> +	{0x3862, 0x04},
> +	{0x3863, 0x08},
> +	{0x3cc0, 0x33},
> +	{0x3d85, 0x17},
> +	{0x3d8c, 0x73},
> +	{0x3d8d, 0xde},
> +	{0x4001, 0xe0},
> +	{0x4003, 0x40},
> +	{0x4008, 0x00},
> +	{0x4009, 0x0b},
> +	{0x400a, 0x00},
> +	{0x400b, 0x84},
> +	{0x400f, 0x80},
> +	{0x4010, 0xf0},
> +	{0x4011, 0xff},
> +	{0x4012, 0x02},
> +	{0x4013, 0x01},
> +	{0x4014, 0x01},
> +	{0x4015, 0x01},
> +	{0x4042, 0x00},
> +	{0x4043, 0x80},
> +	{0x4044, 0x00},
> +	{0x4045, 0x80},
> +	{0x4046, 0x00},
> +	{0x4047, 0x80},
> +	{0x4048, 0x00},
> +	{0x4049, 0x80},
> +	{0x4041, 0x03},
> +	{0x404c, 0x20},
> +	{0x404d, 0x00},
> +	{0x404e, 0x20},
> +	{0x4203, 0x80},
> +	{0x4307, 0x30},
> +	{0x4317, 0x00},
> +	{0x4502, 0x50},
> +	{0x4503, 0x08},
> +	{0x4601, 0x80},
> +	{0x4800, 0x44},
> +	{0x4816, 0x53},
> +	{0x481b, 0x50},
> +	{0x481f, 0x27},
> +	{0x4823, 0x3c},
> +	{0x482b, 0x00},
> +	{0x4831, 0x66},
> +	{0x4837, 0x16},
> +	{0x483c, 0x0f},
> +	{0x484b, 0x05},
> +	{0x5000, 0x77},
> +	{0x5001, 0x0a},
> +	{0x5003, 0xc8},
> +	{0x5004, 0x04},
> +	{0x5006, 0x00},
> +	{0x5007, 0x00},
> +	{0x502e, 0x03},
> +	{0x5030, 0x41},
> +	{0x5780, 0x14},
> +	{0x5781, 0x0f},
> +	{0x5782, 0x44},
> +	{0x5783, 0x02},
> +	{0x5784, 0x01},
> +	{0x5785, 0x01},
> +	{0x5786, 0x00},
> +	{0x5787, 0x04},
> +	{0x5788, 0x02},
> +	{0x5789, 0x0f},
> +	{0x578a, 0xfd},
> +	{0x578b, 0xf5},
> +	{0x578c, 0xf5},
> +	{0x578d, 0x03},
> +	{0x578e, 0x08},
> +	{0x578f, 0x0c},
> +	{0x5790, 0x08},
> +	{0x5791, 0x04},
> +	{0x5792, 0x00},
> +	{0x5793, 0x52},
> +	{0x5794, 0xa3},
> +	{0x5795, 0x02},
> +	{0x5796, 0x20},
> +	{0x5797, 0x20},
> +	{0x5798, 0xd5},
> +	{0x5799, 0xd5},
> +	{0x579a, 0x00},
> +	{0x579b, 0x50},
> +	{0x579c, 0x00},
> +	{0x579d, 0x2c},
> +	{0x579e, 0x0c},
> +	{0x579f, 0x40},
> +	{0x57a0, 0x09},
> +	{0x57a1, 0x40},
> +	{0x59f8, 0x3d},
> +	{0x5a08, 0x02},
> +	{0x5b00, 0x02},
> +	{0x5b01, 0x10},
> +	{0x5b02, 0x03},
> +	{0x5b03, 0xcf},
> +	{0x5b05, 0x6c},
> +	{0x5e00, 0x00},
> +	{0x5e10, 0xfc}
> +};
> +
>  static const struct ov8856_reg mode_1640x1232_regs[] = {
>  	{0x3000, 0x20},
>  	{0x3003, 0x08},
> @@ -385,11 +620,11 @@ static const struct ov8856_reg mode_1640x1232_regs[] = {
>  	{0x3800, 0x00},
>  	{0x3801, 0x00},
>  	{0x3802, 0x00},
> -	{0x3803, 0x06},
> +	{0x3803, 0x07},
>  	{0x3804, 0x0c},
>  	{0x3805, 0xdf},
>  	{0x3806, 0x09},
> -	{0x3807, 0xa7},
> +	{0x3807, 0xa6},
>  	{0x3808, 0x06},
>  	{0x3809, 0x68},
>  	{0x380a, 0x04},
> @@ -401,7 +636,7 @@ static const struct ov8856_reg mode_1640x1232_regs[] = {
>  	{0x3810, 0x00},
>  	{0x3811, 0x00},
>  	{0x3812, 0x00},
> -	{0x3813, 0x01},
> +	{0x3813, 0x00},
>  	{0x3814, 0x03},
>  	{0x3815, 0x01},
>  	{0x3816, 0x00},
> @@ -506,6 +741,209 @@ static const struct ov8856_reg mode_1640x1232_regs[] = {
>  	{0x5e00, 0x00}
>  };
>  
> +static const struct ov8856_reg mode_1632x1224_regs[] = {
> +	{0x0103, 0x01},
> +	{0x0302, 0x3c},
> +	{0x0303, 0x01},
> +	{0x031e, 0x0c},
> +	{0x3000, 0x20},
> +	{0x3003, 0x08},
> +	{0x300e, 0x20},
> +	{0x3010, 0x00},
> +	{0x3015, 0x84},
> +	{0x3018, 0x72},
> +	{0x3021, 0x23},
> +	{0x3033, 0x24},
> +	{0x3500, 0x00},
> +	{0x3501, 0x4c},
> +	{0x3502, 0xe0},
> +	{0x3503, 0x08},
> +	{0x3505, 0x83},
> +	{0x3508, 0x01},
> +	{0x3509, 0x80},
> +	{0x350c, 0x00},
> +	{0x350d, 0x80},
> +	{0x350e, 0x04},
> +	{0x350f, 0x00},
> +	{0x3510, 0x00},
> +	{0x3511, 0x02},
> +	{0x3512, 0x00},
> +	{0x3600, 0x72},
> +	{0x3601, 0x40},
> +	{0x3602, 0x30},
> +	{0x3610, 0xc5},
> +	{0x3611, 0x58},
> +	{0x3612, 0x5c},
> +	{0x3613, 0xca},
> +	{0x3614, 0x60},
> +	{0x3628, 0xff},
> +	{0x3629, 0xff},
> +	{0x362a, 0xff},
> +	{0x3633, 0x10},
> +	{0x3634, 0x10},
> +	{0x3635, 0x10},
> +	{0x3636, 0x10},
> +	{0x3663, 0x08},
> +	{0x3669, 0x34},
> +	{0x366d, 0x00},
> +	{0x366e, 0x08},
> +	{0x3706, 0x86},
> +	{0x370b, 0x7e},
> +	{0x3714, 0x27},
> +	{0x3730, 0x12},
> +	{0x3733, 0x10},
> +	{0x3764, 0x00},
> +	{0x3765, 0x00},
> +	{0x3769, 0x62},
> +	{0x376a, 0x2a},
> +	{0x376b, 0x30},
> +	{0x3780, 0x00},
> +	{0x3781, 0x24},
> +	{0x3782, 0x00},
> +	{0x3783, 0x23},
> +	{0x3798, 0x2f},
> +	{0x37a1, 0x60},
> +	{0x37a8, 0x6a},
> +	{0x37ab, 0x3f},
> +	{0x37c2, 0x14},
> +	{0x37c3, 0xf1},
> +	{0x37c9, 0x80},
> +	{0x37cb, 0x16},
> +	{0x37cc, 0x16},
> +	{0x37cd, 0x16},
> +	{0x37ce, 0x16},
> +	{0x3800, 0x00},
> +	{0x3801, 0x00},
> +	{0x3802, 0x00},
> +	{0x3803, 0x0c},
> +	{0x3804, 0x0c},
> +	{0x3805, 0xdf},
> +	{0x3806, 0x09},
> +	{0x3807, 0xa3},
> +	{0x3808, 0x06},
> +	{0x3809, 0x60},
> +	{0x380a, 0x04},
> +	{0x380b, 0xc8},
> +	{0x380c, 0x07},
> +	{0x380d, 0x8c},
> +	{0x380e, 0x09},
> +	{0x380f, 0xb2},
> +	{0x3810, 0x00},
> +	{0x3811, 0x02},
> +	{0x3812, 0x00},
> +	{0x3813, 0x02},
> +	{0x3814, 0x03},
> +	{0x3815, 0x01},
> +	{0x3816, 0x00},
> +	{0x3817, 0x00},
> +	{0x3818, 0x00},
> +	{0x3819, 0x00},
> +	{0x3820, 0x80},
> +	{0x3821, 0x47},
> +	{0x382a, 0x03},
> +	{0x382b, 0x01},
> +	{0x3830, 0x06},
> +	{0x3836, 0x02},
> +	{0x3862, 0x04},
> +	{0x3863, 0x08},
> +	{0x3cc0, 0x33},
> +	{0x3d85, 0x17},
> +	{0x3d8c, 0x73},
> +	{0x3d8d, 0xde},
> +	{0x4001, 0xe0},
> +	{0x4003, 0x40},
> +	{0x4008, 0x00},
> +	{0x4009, 0x05},
> +	{0x400a, 0x00},
> +	{0x400b, 0x84},
> +	{0x400f, 0x80},
> +	{0x4010, 0xf0},
> +	{0x4011, 0xff},
> +	{0x4012, 0x02},
> +	{0x4013, 0x01},
> +	{0x4014, 0x01},
> +	{0x4015, 0x01},
> +	{0x4042, 0x00},
> +	{0x4043, 0x80},
> +	{0x4044, 0x00},
> +	{0x4045, 0x80},
> +	{0x4046, 0x00},
> +	{0x4047, 0x80},
> +	{0x4048, 0x00},
> +	{0x4049, 0x80},
> +	{0x4041, 0x03},
> +	{0x404c, 0x20},
> +	{0x404d, 0x00},
> +	{0x404e, 0x20},
> +	{0x4203, 0x80},
> +	{0x4307, 0x30},
> +	{0x4317, 0x00},
> +	{0x4502, 0x50},
> +	{0x4503, 0x08},
> +	{0x4601, 0x80},
> +	{0x4800, 0x44},
> +	{0x4816, 0x53},
> +	{0x481b, 0x50},
> +	{0x481f, 0x27},
> +	{0x4823, 0x3c},
> +	{0x482b, 0x00},
> +	{0x4831, 0x66},
> +	{0x4837, 0x16},
> +	{0x483c, 0x0f},
> +	{0x484b, 0x05},
> +	{0x5000, 0x77},
> +	{0x5001, 0x0a},
> +	{0x5003, 0xc8},
> +	{0x5004, 0x04},
> +	{0x5006, 0x00},
> +	{0x5007, 0x00},
> +	{0x502e, 0x03},
> +	{0x5030, 0x41},
> +	{0x5795, 0x00},
> +	{0x5796, 0x10},
> +	{0x5797, 0x10},
> +	{0x5798, 0x73},
> +	{0x5799, 0x73},
> +	{0x579a, 0x00},
> +	{0x579b, 0x28},
> +	{0x579c, 0x00},
> +	{0x579d, 0x16},
> +	{0x579e, 0x06},
> +	{0x579f, 0x20},
> +	{0x57a0, 0x04},
> +	{0x57a1, 0xa0},
> +	{0x5780, 0x14},
> +	{0x5781, 0x0f},
> +	{0x5782, 0x44},
> +	{0x5783, 0x02},
> +	{0x5784, 0x01},
> +	{0x5785, 0x01},
> +	{0x5786, 0x00},
> +	{0x5787, 0x04},
> +	{0x5788, 0x02},
> +	{0x5789, 0x0f},
> +	{0x578a, 0xfd},
> +	{0x578b, 0xf5},
> +	{0x578c, 0xf5},
> +	{0x578d, 0x03},
> +	{0x578e, 0x08},
> +	{0x578f, 0x0c},
> +	{0x5790, 0x08},
> +	{0x5791, 0x04},
> +	{0x5792, 0x00},
> +	{0x5793, 0x52},
> +	{0x5794, 0xa3},
> +	{0x59f8, 0x3d},
> +	{0x5a08, 0x02},
> +	{0x5b00, 0x02},
> +	{0x5b01, 0x10},
> +	{0x5b02, 0x03},
> +	{0x5b03, 0xcf},
> +	{0x5b05, 0x6c},
> +	{0x5e00, 0x00},
> +	{0x5e10, 0xfc}
> +};
> +
>  static const char * const ov8856_test_pattern_menu[] = {
>  	"Disabled",
>  	"Standard Color Bar",
> @@ -548,6 +986,18 @@ static const struct ov8856_mode supported_modes[] = {
>  		.link_freq_index = OV8856_LINK_FREQ_720MBPS,
>  	},
>  	{
> +		.width    = 3264,
> +		.height   = 2448,
> +		.hts      = 1932,
> +		.vts_def  = 2482,
> +		.vts_min  = 2482,
> +		.reg_list = {
> +			.num_of_regs = ARRAY_SIZE(mode_3264x2448_regs),
> +			.regs = mode_3264x2448_regs,
> +		},
> +		.link_freq_index = OV8856_LINK_FREQ_720MBPS,
> +	},

How do you guarantee that the modes are only used on the xvclk frequency
they're intended for?

I think it'd be best if you added the new modes in a separete patch.

> +	{
>  		.width = 1640,
>  		.height = 1232,
>  		.hts = 3820,
> @@ -558,6 +1008,18 @@ static const struct ov8856_mode supported_modes[] = {
>  			.regs = mode_1640x1232_regs,
>  		},
>  		.link_freq_index = OV8856_LINK_FREQ_360MBPS,
> +	},
> +	{
> +		.width    = 1632,
> +		.height   = 1224,
> +		.hts      = 1932,
> +		.vts_def  = 2482,
> +		.vts_min  = 2482,
> +		.reg_list = {
> +			.num_of_regs = ARRAY_SIZE(mode_1632x1224_regs),
> +			.regs = mode_1632x1224_regs,
> +		},
> +		.link_freq_index = OV8856_LINK_FREQ_360MBPS,
>  	}
>  };
>  
> @@ -566,16 +1028,28 @@ struct ov8856 {
>  	struct media_pad pad;
>  	struct v4l2_ctrl_handler ctrl_handler;
>  
> +	struct clk		*xvclk;
> +	struct gpio_desc	*n_shutdn_gpio;
> +	struct regulator_bulk_data supplies[OV8856_NUM_SUPPLIES];
> +
>  	/* V4L2 Controls */
>  	struct v4l2_ctrl *link_freq;
>  	struct v4l2_ctrl *pixel_rate;
>  	struct v4l2_ctrl *vblank;
>  	struct v4l2_ctrl *hblank;
>  	struct v4l2_ctrl *exposure;
> +	struct v4l2_mbus_framefmt	fmt;
>  
>  	/* Current mode */
>  	const struct ov8856_mode *cur_mode;
>  
> +	/* module hardware version that can be read out from register 0x700f
> +	 * the register value corresponds to different hardware version
> +	 * 01: 2A module revision
> +	 * 02: 1B module revision
> +	 */
> +	bool is_1B_revision;
> +
>  	/* To serialize asynchronus callbacks */
>  	struct mutex mutex;
>  
> @@ -696,6 +1170,25 @@ static int ov8856_test_pattern(struct ov8856 *ov8856, u32 pattern)
>  				OV8856_REG_VALUE_08BIT, pattern);
>  }
>  
> +static int ov8856_check_revision(struct ov8856 *ov8856)
> +{
> +	int ret;
> +
> +	ret = ov8856_write_reg(ov8856, OV8856_REG_MODE_SELECT,
> +			       OV8856_REG_VALUE_08BIT, OV8856_MODE_STREAMING);
> +	if (ret)
> +		return ret;
> +
> +	ret = ov8856_write_reg(ov8856, OV8856_OTP_MODE_CTRL,
> +			       OV8856_REG_VALUE_08BIT, OV8856_OTP_MODE_AUTO);
> +	if (ret)
> +		return ret;
> +
> +	return ov8856_write_reg(ov8856, OV8856_OTP_LOAD_CTRL,
> +				OV8856_REG_VALUE_08BIT,
> +				OV8856_OTP_LOAD_CTRL_ENABLE);

Is it not necessary to disable streaming after reading EEPROM?

> +}
> +
>  static int ov8856_set_ctrl(struct v4l2_ctrl *ctrl)
>  {
>  	struct ov8856 *ov8856 = container_of(ctrl->handler,
> @@ -825,7 +1318,6 @@ static void ov8856_update_pad_format(const struct ov8856_mode *mode,
>  {
>  	fmt->width = mode->width;
>  	fmt->height = mode->height;
> -	fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
>  	fmt->field = V4L2_FIELD_NONE;
>  }
>  
> @@ -834,6 +1326,7 @@ static int ov8856_start_streaming(struct ov8856 *ov8856)
>  	struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
>  	const struct ov8856_reg_list *reg_list;
>  	int link_freq_index, ret;
> +	u32 h_size;
>  
>  	link_freq_index = ov8856->cur_mode->link_freq_index;
>  	reg_list = &link_freq_configs[link_freq_index].reg_list;
> @@ -850,6 +1343,29 @@ static int ov8856_start_streaming(struct ov8856 *ov8856)
>  		return ret;
>  	}
>  
> +	/* Update R3614 if the revision is 1B module */
> +	if (ov8856->is_1B_revision) {
> +		ret = ov8856_write_reg(ov8856, OV8856_ANAL_MODE_CTRL,
> +				       OV8856_REG_VALUE_08BIT,
> +				       OV8856_ANAL_1B_VAL);
> +		if (ret) {
> +			dev_err(&client->dev, "failed to set R3614");
> +			return ret;
> +		}
> +	}
> +
> +	ret = ov8856_read_reg(ov8856, REG_X_ADDR_START,
> +			      OV8856_REG_VALUE_16BIT, &h_size);
> +	if (ret) {
> +		dev_err(&client->dev, "failed to read out R3614");
> +		return ret;
> +	}
> +
> +	if (h_size == X_OUTPUT_FULL_SIZE || h_size == X_OUTPUT_BINNING_SIZE)
> +		ov8856->fmt.code = MEDIA_BUS_FMT_SBGGR10_1X10;

I wonder if this condition is right. The pixel order (and thus the media
bus code) is determined by cropping (and flipping and mirroring, but
they're not supported right now). See e.g. the smiapp driver for an
example.

The correct media bus code needs to be available to the user through the
regular API at all times, not only after streaming is started.

I'd suggest adding support for other pixel orders in a separate patch.

> +
>  	ret = __v4l2_ctrl_handler_setup(ov8856->sd.ctrl_handler);
>  	if (ret)
>  		return ret;
> @@ -878,6 +1394,7 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
>  	struct ov8856 *ov8856 = to_ov8856(sd);
>  	struct i2c_client *client = v4l2_get_subdevdata(sd);
>  	int ret = 0;
> +	u32 val;
>  
>  	if (ov8856->streaming == enable)
>  		return 0;
> @@ -908,6 +1425,44 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
>  	return ret;
>  }
>  
> +static int __ov8856_power_on(struct ov8856 *ov8856)
> +{
> +	struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
> +	int ret;
> +
> +	ret = clk_prepare_enable(ov8856->xvclk);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed to enable xvclk\n");
> +		return ret;
> +	}
> +
> +	gpiod_set_value_cansleep(ov8856->n_shutdn_gpio, GPIOD_OUT_LOW);
> +
> +	ret = regulator_bulk_enable(OV8856_NUM_SUPPLIES, ov8856->supplies);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed to enable regulators\n");
> +		goto disable_clk;
> +	}
> +
> +	gpiod_set_value_cansleep(ov8856->n_shutdn_gpio, GPIOD_OUT_HIGH);

..., 1);

> +
> +	usleep_range(1400, 1500);
> +
> +	return 0;
> +
> +disable_clk:
> +	clk_disable_unprepare(ov8856->xvclk);

How about the n_shutdn GPIO?

> +
> +	return ret;
> +}
> +
> +static void __ov8856_power_off(struct ov8856 *ov8856)
> +{
> +	gpiod_set_value_cansleep(ov8856->n_shutdn_gpio, 1);

Why to inactivate xshutdn here (it already was)?

> +	regulator_bulk_disable(OV8856_NUM_SUPPLIES, ov8856->supplies);
> +	clk_disable_unprepare(ov8856->xvclk);
> +}
> +
>  static int __maybe_unused ov8856_suspend(struct device *dev)
>  {
>  	struct i2c_client *client = to_i2c_client(dev);
> @@ -951,6 +1506,7 @@ static int ov8856_set_format(struct v4l2_subdev *sd,
>  			     struct v4l2_subdev_format *fmt)
>  {
>  	struct ov8856 *ov8856 = to_ov8856(sd);
> +	struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
>  	const struct ov8856_mode *mode;
>  	s32 vblank_def, h_blank;
>  
> @@ -960,7 +1516,9 @@ static int ov8856_set_format(struct v4l2_subdev *sd,
>  				      fmt->format.height);
>  
>  	mutex_lock(&ov8856->mutex);
> -	ov8856_update_pad_format(mode, &fmt->format);
> +	mbus_fmt->code = ov8856->fmt.code;
> +	ov8856_update_pad_format(mode, mbus_fmt);
> +	ov8856->fmt = fmt->format;
>  	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
>  		*v4l2_subdev_get_try_format(sd, cfg, fmt->pad) = fmt->format;
>  	} else {
> @@ -992,13 +1550,17 @@ static int ov8856_get_format(struct v4l2_subdev *sd,
>  			     struct v4l2_subdev_format *fmt)
>  {
>  	struct ov8856 *ov8856 = to_ov8856(sd);
> +	struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
>  
>  	mutex_lock(&ov8856->mutex);
> -	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> +	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
>  		fmt->format = *v4l2_subdev_get_try_format(&ov8856->sd, cfg,
>  							  fmt->pad);
> -	else
> -		ov8856_update_pad_format(ov8856->cur_mode, &fmt->format);
> +	} else {
> +		fmt->format = ov8856->fmt;
> +		ov8856_update_pad_format(ov8856->cur_mode, mbus_fmt);
> +		mbus_fmt->code = ov8856->fmt.code;
> +	}
>  
>  	mutex_unlock(&ov8856->mutex);
>  
> @@ -1009,11 +1571,12 @@ static int ov8856_enum_mbus_code(struct v4l2_subdev *sd,
>  				 struct v4l2_subdev_pad_config *cfg,
>  				 struct v4l2_subdev_mbus_code_enum *code)
>  {
> -	/* Only one bayer order GRBG is supported */
> +	struct ov8856 *ov8856 = to_ov8856(sd);
> +
>  	if (code->index > 0)
>  		return -EINVAL;
>  
> -	code->code = MEDIA_BUS_FMT_SGRBG10_1X10;
> +	code->code = ov8856->fmt.code;
>  
>  	return 0;
>  }
> @@ -1089,6 +1652,20 @@ static int ov8856_identify_module(struct ov8856 *ov8856)
>  		return -ENXIO;
>  	}
>  
> +	/* check sensor hardware revision */
> +	ret = ov8856_check_revision(ov8856);
> +	if (ret) {
> +		dev_err(&client->dev, "failed to check sensor revision");
> +		return ret;
> +	}
> +
> +	ret = ov8856_read_reg(ov8856, OV8856_MODULE_REVISION,
> +			      OV8856_REG_VALUE_08BIT, &val);
> +	if (ret)
> +		return ret;
> +
> +	ov8856->is_1B_revision = (val == OV8856_1B_MODULE) ? 1 : 0;
> +
>  	return 0;
>  }
>  
> @@ -1107,7 +1684,7 @@ static int ov8856_check_hwcfg(struct device *dev)
>  		return -ENXIO;
>  
>  	fwnode_property_read_u32(fwnode, "clock-frequency", &mclk);
> -	if (mclk != OV8856_MCLK) {
> +	if (mclk != OV8856_XVCLK) {
>  		dev_err(dev, "external clock %d is not supported", mclk);
>  		return -EINVAL;
>  	}
> @@ -1164,6 +1741,9 @@ static int ov8856_remove(struct i2c_client *client)
>  	media_entity_cleanup(&sd->entity);
>  	v4l2_ctrl_handler_free(sd->ctrl_handler);
>  	pm_runtime_disable(&client->dev);
> +	if (!pm_runtime_status_suspended(&client->dev))
> +		__ov8856_power_off(ov8856);
> +	pm_runtime_set_suspended(&client->dev);
>  	mutex_destroy(&ov8856->mutex);
>  
>  	return 0;
> @@ -1172,6 +1752,7 @@ static int ov8856_remove(struct i2c_client *client)
>  static int ov8856_probe(struct i2c_client *client)
>  {
>  	struct ov8856 *ov8856;
> +	unsigned int i;
>  	int ret;
>  
>  	ret = ov8856_check_hwcfg(&client->dev);
> @@ -1186,6 +1767,42 @@ static int ov8856_probe(struct i2c_client *client)
>  		return -ENOMEM;
>  
>  	v4l2_i2c_subdev_init(&ov8856->sd, client, &ov8856_subdev_ops);
> +	ov8856->fmt.code = MEDIA_BUS_FMT_SGRBG10_1X10;
> +
> +	ov8856->xvclk = devm_clk_get(&client->dev, "xvclk");

Does this work on ACPI?

See e.g. the smiapp driver for an example. The same applies to the rest of
the chunk.

> +	if (IS_ERR(ov8856->xvclk)) {
> +		dev_err(&client->dev, "failed to get xvclk\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = clk_set_rate(ov8856->xvclk, OV8856_XVCLK_TYP);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed to set xvclk rate (24MHz)\n");
> +		return ret;
> +	}
> +	if (clk_get_rate(ov8856->xvclk) != OV8856_XVCLK_TYP)
> +		dev_warn(&client->dev,
> +			 "xvclk mismatched, modes are based on 24MHz\n");
> +
> +	ov8856->n_shutdn_gpio = devm_gpiod_get(&client->dev, "reset",
> +					       GPIOD_OUT_LOW);
> +	if (IS_ERR(ov8856->n_shutdn_gpio)) {
> +		dev_err(&client->dev, "failed to get reset-gpios\n");
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < OV8856_NUM_SUPPLIES; i++)
> +		ov8856->supplies[i].supply = ov8856_supply_names[i];
> +
> +	ret = devm_regulator_bulk_get(&client->dev, OV8856_NUM_SUPPLIES,
> +				      ov8856->supplies);
> +	if (ret)
> +		dev_warn(&client->dev, "failed to get regulators\n");
> +
> +	ret = __ov8856_power_on(ov8856);
> +	if (ret)
> +		dev_warn(&client->dev, "failed to power on\n");
> +
>  	ret = ov8856_identify_module(ov8856);
>  	if (ret) {
>  		dev_err(&client->dev, "failed to find sensor: %d", ret);
> @@ -1251,11 +1868,18 @@ static const struct acpi_device_id ov8856_acpi_ids[] = {
>  MODULE_DEVICE_TABLE(acpi, ov8856_acpi_ids);
>  #endif
>  
> +static const struct of_device_id ov8856_of_match[] = {
> +	{ .compatible = "ovti,ov8856" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ov8856_of_match);
> +
>  static struct i2c_driver ov8856_i2c_driver = {
>  	.driver = {
>  		.name = "ov8856",
>  		.pm = &ov8856_pm_ops,
>  		.acpi_match_table = ACPI_PTR(ov8856_acpi_ids),
> +		.of_match_table = ov8856_of_match,
>  	},
>  	.probe_new = ov8856_probe,
>  	.remove = ov8856_remove,

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 4/6] arm64: dts: meson: sei510: Add minimal thermal zone
From: Amit Kucheria @ 2019-09-13  7:47 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux PM list, Daniel Lezcano, LKML, Eduardo Valentin,
	Guillaume La Roque, linux-amlogic, Zhang Rui, lakml
In-Reply-To: <7hsgpu5c7j.fsf@baylibre.com>

On Thu, Aug 22, 2019 at 4:59 AM Kevin Hilman <khilman@baylibre.com> wrote:
>
> Guillaume La Roque <glaroque@baylibre.com> writes:
>
> > Add minimal thermal zone for two temperature sensor
> > One is located close to the DDR and the other one is
> > located close to the PLLs (between the CPU and GPU)
> >
> > Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
> > Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > ---
> >  .../boot/dts/amlogic/meson-g12a-sei510.dts    | 70 +++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
> > index c9fa23a56562..35d2ebbd6d4e 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
> > +++ b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
> > @@ -10,6 +10,7 @@
> >  #include <dt-bindings/input/input.h>
> >  #include <dt-bindings/gpio/meson-g12a-gpio.h>
> >  #include <dt-bindings/sound/meson-g12a-tohdmitx.h>
> > +#include <dt-bindings/thermal/thermal.h>
> >
> >  / {
> >       compatible = "seirobotics,sei510", "amlogic,g12a";
> > @@ -33,6 +34,67 @@
> >               ethernet0 = &ethmac;
> >       };
> >
> > +     thermal-zones {
> > +             cpu-thermal {
> > +                     polling-delay = <1000>;
> > +                     polling-delay-passive = <100>;
> > +                     thermal-sensors = <&cpu_temp>;
> > +
> > +                     trips {
> > +                             cpu_hot: cpu-hot {
> > +                                     temperature = <85000>; /* millicelsius */
> > +                                     hysteresis = <2000>; /* millicelsius */
> > +                                     type = "hot";
> > +                             };

No passive trip point? That is where the cooling-maps are really useful.

> > +
> > +                             cpu_critical: cpu-critical {
> > +                                     temperature = <110000>; /* millicelsius */
> > +                                     hysteresis = <2000>; /* millicelsius */
> > +                                     type = "critical";
> > +                             };
> > +                     };
> > +

I think, what you really want is to change your hot trip point above
to passive. And if you need another trip before that (to send
notification to userspace, for example), just add another hot trip
point at a lower temperature.

> > +                     cooling-maps {
> > +                             map0 {
> > +                                     trip = <&cpu_hot>;
> > +                                     cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > +                             };
> > +
> > +                             map1 {
> > +                                     trip = <&cpu_critical>;
> > +                                     cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > +                             };

The cooling-map associated with a critical trip point is of no use in
my experience because the device is already on its way to shutting
down then.

> > +                     };
> > +             };
> > +
> > +             ddr-thermal {
> > +                     polling-delay = <1000>;
> > +                     polling-delay-passive = <100>;
> > +                     thermal-sensors = <&ddr_temp>;
> > +
> > +                     trips {
> > +                             ddr_critical: ddr-critical {
> > +                                     temperature = <110000>; /* millicelsius */
> > +                                     hysteresis = <2000>; /* millicelsius */
> > +                                     type = "critical";
> > +                             };
> > +                     };
> > +
> > +                     cooling-maps {
> > +                             map {
> > +                                     trip = <&ddr_critical>;
> > +                                     cooling-device = <&mali THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;

Same here. The cooling-map makes more sense against a passive trip type.

> > +                             };
> > +                     };
> > +             };
> > +     };
> > +
> >       mono_dac: audio-codec-0 {
> >               compatible = "maxim,max98357a";
> >               #sound-dai-cells = <0>;
> > @@ -321,6 +383,7 @@
> >       operating-points-v2 = <&cpu_opp_table>;
> >       clocks = <&clkc CLKID_CPU_CLK>;
> >       clock-latency = <50000>;
> > +     #cooling-cells = <2>;
> >  };
> >
> >  &cpu1 {
> > @@ -328,6 +391,7 @@
> >       operating-points-v2 = <&cpu_opp_table>;
> >       clocks = <&clkc CLKID_CPU_CLK>;
> >       clock-latency = <50000>;
> > +     #cooling-cells = <2>;
> >  };
> >
> >  &cpu2 {
> > @@ -335,6 +399,7 @@
> >       operating-points-v2 = <&cpu_opp_table>;
> >       clocks = <&clkc CLKID_CPU_CLK>;
> >       clock-latency = <50000>;
> > +     #cooling-cells = <2>;
> >  };
> >
> >  &cpu3 {
> > @@ -342,6 +407,7 @@
> >       operating-points-v2 = <&cpu_opp_table>;
> >       clocks = <&clkc CLKID_CPU_CLK>;
> >       clock-latency = <50000>;
> > +     #cooling-cells = <2>;
> >  };
> >
> >  &cvbs_vdac_port {
> > @@ -368,6 +434,10 @@
> >       status = "okay";
> >  };
> >
> > +&mali {
> > +     #cooling-cells = <2>;
> > +};
> > +
>
> Is there a reason these #cooling-cells properties belong in the SoC
> .dtsi and not the board .dts.  Seems like you'll have to repeat this in
> every board .dts which doesn't seem necessary.
>
> Same comment for patch 5/6

Agreed. Even the thermal zones belong in the SoC .dtsi. You can always
override the trip-points in a board .dts if required if you have a
board designed in a different form-factor or with active cooling.

/Amit

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3] input: keyboard: snvs_pwrkey: Send key events for i.MX6 S, DL and Q
From: robin @ 2019-09-13  7:39 UTC (permalink / raw)
  To: Marco Felsch
  Cc: Adam Ford, Dmitry Torokhov, linux-kernel @ vger . kernel . org,
	Pengutronix Kernel Team, linux-input @ vger . kernel . org,
	RobinGong, Shawn Guo, linux-arm-kernel @ lists . infradead . org
In-Reply-To: <20190904065248.4i7q2vuxxt2xdnrr@pengutronix.de>

On 2019-09-04 08:52, Marco Felsch wrote:
> Hi Robin,
> 
> thanks for the patch it looks quite good, just two minor nitpicks.
> 
> On 19-09-04 06:23, Robin van der Gracht wrote:
>> The first generation i.MX6 processors does not send an interrupt when 
>> the
>> power key is pressed. It sends a power down request interrupt if the 
>> key is
>> released before a hard shutdown (5 second press). This should allow
>> software to bring down the SoC safely.
>> 
>> For this driver to work as a regular power key with the older SoCs, we 
>> need
>> to send a keypress AND release when we get the power down request irq.
>> 
>> Signed-off-by: Robin van der Gracht <robin@protonic.nl>
>> ---
>> 
>> Changes v2 -> v3:
>>  - Drop alt compatible string for identifying first revision snvs 
>> hardware,
>>    read minor revision from register instead.
>>  - Drop imx6qdl.dtsi modification and device-tree binding 
>> documentation.
>>  - Add an additional input_sync() to create 2 seperate input reports 
>> for press
>>    and release.
>> 
>>  drivers/input/keyboard/Kconfig       |  2 +-
>>  drivers/input/keyboard/snvs_pwrkey.c | 28 
>> ++++++++++++++++++++++++++--
>>  2 files changed, 27 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/input/keyboard/Kconfig 
>> b/drivers/input/keyboard/Kconfig
>> index 7c4f19dab34f..937e58da5ce1 100644
>> --- a/drivers/input/keyboard/Kconfig
>> +++ b/drivers/input/keyboard/Kconfig
>> @@ -436,7 +436,7 @@ config KEYBOARD_SNVS_PWRKEY
>>  	depends on OF
>>  	help
>>  	  This is the snvs powerkey driver for the Freescale i.MX 
>> application
>> -	  processors that are newer than i.MX6 SX.
>> +	  processors.
>> 
>>  	  To compile this driver as a module, choose M here; the
>>  	  module will be called snvs_pwrkey.
>> diff --git a/drivers/input/keyboard/snvs_pwrkey.c 
>> b/drivers/input/keyboard/snvs_pwrkey.c
>> index 5342d8d45f81..828580eee0d2 100644
>> --- a/drivers/input/keyboard/snvs_pwrkey.c
>> +++ b/drivers/input/keyboard/snvs_pwrkey.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/mfd/syscon.h>
>>  #include <linux/regmap.h>
>> 
>> +#define SNVS_HPVIDR1_REG 0xF8
>>  #define SNVS_LPSR_REG	0x4C	/* LP Status Register */
>>  #define SNVS_LPCR_REG	0x38	/* LP Control Register */
>>  #define SNVS_HPSR_REG	0x14
>> @@ -37,6 +38,7 @@ struct pwrkey_drv_data {
>>  	int wakeup;
>>  	struct timer_list check_timer;
>>  	struct input_dev *input;
>> +	u8 minor_rev;
>>  };
>> 
>>  static void imx_imx_snvs_check_for_events(struct timer_list *t)
>> @@ -45,6 +47,20 @@ static void imx_imx_snvs_check_for_events(struct 
>> timer_list *t)
>>  	struct input_dev *input = pdata->input;
>>  	u32 state;
>> 
>> +	if (pdata->minor_rev == 0) {
> 
> Should we use a define here and ..
> 
>> +		/*
>> +		 * The first generation i.MX6 SoCs only sends an interrupt on
>> +		 * button release. To mimic power-key usage, we'll prepend a
>> +		 * press event.
>> +		 */
>> +		input_report_key(input, pdata->keycode, 1);
>> +		input_sync(input);
>> +		input_report_key(input, pdata->keycode, 0);
>> +		input_sync(input);
>> +		pm_relax(input->dev.parent);
>> +		return;
>> +	}
>> +
>>  	regmap_read(pdata->snvs, SNVS_HPSR_REG, &state);
>>  	state = state & SNVS_HPSR_BTN ? 1 : 0;
>> 
>> @@ -67,13 +83,17 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int 
>> irq, void *dev_id)
>>  {
>>  	struct platform_device *pdev = dev_id;
>>  	struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev);
>> +	unsigned long expire = jiffies;
>>  	u32 lp_status;
>> 
>>  	pm_wakeup_event(pdata->input->dev.parent, 0);
>> 
>>  	regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status);
>> -	if (lp_status & SNVS_LPSR_SPO)
>> -		mod_timer(&pdata->check_timer, jiffies + 
>> msecs_to_jiffies(DEBOUNCE_TIME));
>> +	if (lp_status & SNVS_LPSR_SPO) {
>> +		if (pdata->minor_rev > 0)
> 
> here? Just a nitpick, feel free to add/drop it.

Like a Macro?

#define FIRST_HW_REV(pdata)      (pdata->minor_rev == 0)

if (FIRST_HW_REV(pdata) {
         ...
}


or just a define to identify the minor rev used for the first hw 
revision


#define FIRST_HW_MINOR_REV       0

if (pdata->minor_rev == FIRST_HW_MINOR_REV) {
         ...
}

Regards,
Robin van der Gracht

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3] input: keyboard: snvs_pwrkey: Send key events for i.MX6 S, DL and Q
From: robin @ 2019-09-13  7:30 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Adam Ford, Marco Felsch, linux-kernel @ vger . kernel . org,
	Pengutronix Kernel Team, linux-input @ vger . kernel . org,
	RobinGong, Shawn Guo, linux-arm-kernel @ lists . infradead . org
In-Reply-To: <20190912201300.GA636@penguin>

Hi Dmitry,

On 2019-09-12 22:13, Dmitry Torokhov wrote:
> Hi Robin,
> 
> On Wed, Sep 04, 2019 at 06:23:29AM +0000, Robin van der Gracht wrote:
>> The first generation i.MX6 processors does not send an interrupt when 
>> the
>> power key is pressed. It sends a power down request interrupt if the 
>> key is
>> released before a hard shutdown (5 second press). This should allow
>> software to bring down the SoC safely.
>> 
>> For this driver to work as a regular power key with the older SoCs, we 
>> need
>> to send a keypress AND release when we get the power down request irq.
>> 
>> Signed-off-by: Robin van der Gracht <robin@protonic.nl>
>> ---
>> 
>> Changes v2 -> v3:
>>  - Drop alt compatible string for identifying first revision snvs 
>> hardware,
>>    read minor revision from register instead.
>>  - Drop imx6qdl.dtsi modification and device-tree binding 
>> documentation.
>>  - Add an additional input_sync() to create 2 seperate input reports 
>> for press
>>    and release.
>> 
>>  drivers/input/keyboard/Kconfig       |  2 +-
>>  drivers/input/keyboard/snvs_pwrkey.c | 28 
>> ++++++++++++++++++++++++++--
>>  2 files changed, 27 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/input/keyboard/Kconfig 
>> b/drivers/input/keyboard/Kconfig
>> index 7c4f19dab34f..937e58da5ce1 100644
>> --- a/drivers/input/keyboard/Kconfig
>> +++ b/drivers/input/keyboard/Kconfig
>> @@ -436,7 +436,7 @@ config KEYBOARD_SNVS_PWRKEY
>>  	depends on OF
>>  	help
>>  	  This is the snvs powerkey driver for the Freescale i.MX 
>> application
>> -	  processors that are newer than i.MX6 SX.
>> +	  processors.
>> 
>>  	  To compile this driver as a module, choose M here; the
>>  	  module will be called snvs_pwrkey.
>> diff --git a/drivers/input/keyboard/snvs_pwrkey.c 
>> b/drivers/input/keyboard/snvs_pwrkey.c
>> index 5342d8d45f81..828580eee0d2 100644
>> --- a/drivers/input/keyboard/snvs_pwrkey.c
>> +++ b/drivers/input/keyboard/snvs_pwrkey.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/mfd/syscon.h>
>>  #include <linux/regmap.h>
>> 
>> +#define SNVS_HPVIDR1_REG 0xF8
>>  #define SNVS_LPSR_REG	0x4C	/* LP Status Register */
>>  #define SNVS_LPCR_REG	0x38	/* LP Control Register */
>>  #define SNVS_HPSR_REG	0x14
>> @@ -37,6 +38,7 @@ struct pwrkey_drv_data {
>>  	int wakeup;
>>  	struct timer_list check_timer;
>>  	struct input_dev *input;
>> +	u8 minor_rev;
>>  };
>> 
>>  static void imx_imx_snvs_check_for_events(struct timer_list *t)
>> @@ -45,6 +47,20 @@ static void imx_imx_snvs_check_for_events(struct 
>> timer_list *t)
>>  	struct input_dev *input = pdata->input;
>>  	u32 state;
>> 
>> +	if (pdata->minor_rev == 0) {
>> +		/*
>> +		 * The first generation i.MX6 SoCs only sends an interrupt on
>> +		 * button release. To mimic power-key usage, we'll prepend a
>> +		 * press event.
>> +		 */
>> +		input_report_key(input, pdata->keycode, 1);
>> +		input_sync(input);
>> +		input_report_key(input, pdata->keycode, 0);
>> +		input_sync(input);
>> +		pm_relax(input->dev.parent);
>> +		return;
>> +	}
>> +
>>  	regmap_read(pdata->snvs, SNVS_HPSR_REG, &state);
>>  	state = state & SNVS_HPSR_BTN ? 1 : 0;
>> 
>> @@ -67,13 +83,17 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int 
>> irq, void *dev_id)
>>  {
>>  	struct platform_device *pdev = dev_id;
>>  	struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev);
>> +	unsigned long expire = jiffies;
>>  	u32 lp_status;
>> 
>>  	pm_wakeup_event(pdata->input->dev.parent, 0);
>> 
>>  	regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status);
>> -	if (lp_status & SNVS_LPSR_SPO)
>> -		mod_timer(&pdata->check_timer, jiffies + 
>> msecs_to_jiffies(DEBOUNCE_TIME));
>> +	if (lp_status & SNVS_LPSR_SPO) {
>> +		if (pdata->minor_rev > 0)
>> +			expire = jiffies + msecs_to_jiffies(DEBOUNCE_TIME);
>> +		mod_timer(&pdata->check_timer, expire);
> 
> Why do we even need to fire the timer in case of the first generation
> hardware? Just send press and release events directly from the ISR.

Robin Gong proposed to move the code to imx_imx_snvs_check_for_events()
to improve readability and unload the ISR.

But since I, eventually, couldn't use the existing handling in
imx_imx_snvs_check_for_events(), I do see why you're asking.

I'll move the code to the ISR and submit a new patch.

Robin van der Gracht

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 1/2] iommu: pass cell_count = -1 to of_for_each_phandle with cells_name
From: Joerg Roedel @ 2019-09-13  7:37 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: devicetree, Will Deacon, Robin Murphy, linux-kernel, iommu,
	Rob Herring, linux-mediatek, kernel, Matthias Brugger,
	Frank Rowand, linux-arm-kernel
In-Reply-To: <20190912074353.wqohpfydjxueqade@pengutronix.de>

On Thu, Sep 12, 2019 at 09:43:53AM +0200, Uwe Kleine-König wrote:
> On Tue, Sep 03, 2019 at 02:52:10PM +0200, Joerg Roedel wrote:
> > Acked-by: Joerg Roedel <jroedel@suse.de>
> 
> Does this ack mean that Rob is expected to apply this together with
> patch 2?

"Expected" is a strong word. I'd more phrase it like I am fine with this
patch going through his tree.

Regards,

	Joerg

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: KASAN: slab-out-of-bounds Read in handle_vmptrld
From: Paolo Bonzini @ 2019-09-13  7:34 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: mark.rutland, x86, wanpengli, kvm, narmstrong, catalin.marinas,
	will.deacon, hpa, khilman, joro, rkrcmar, mingo, Dmitry Vyukov,
	syzbot, devicetree, syzkaller-bugs, robh+dt, bp, linux-amlogic,
	tglx, linux-arm-kernel, jmattson, USB list, linux-kernel,
	sean.j.christopherson, carlo, Vitaly Kuznetsov
In-Reply-To: <20190913044614.GA120223@kroah.com>

On 13/09/19 06:46, Greg Kroah-Hartman wrote:
> USB drivers expect kmalloc to return DMA-able memory.  I don't know
> about specific alignment issues, that should only an issue for the host
> controller being used here, which you do not say in the above list.

I have no idea, this is just the analysis of a syzkaller report.  From 
the backtrace, it's one that ends up calling kmalloc; all of them should
have the same issue with KASAN.

The specific alignment requirement for this bug comes from this call in
usbdev_mmap:

	if (remap_pfn_range(vma, vma->vm_start,
			virt_to_phys(usbm->mem) >> PAGE_SHIFT,
			size, vma->vm_page_prot) < 0) {

> We have had some reports that usbdev_mmap() does not do the "correct
> thing" for all host controllers, but a lot of the DMA work that is in
> linux-next for 5.4-rc1 should have helped resolve those issues.  What
> tree are you seeing these bug reports happening from?

It's in master, but the relevant code is the same in linux-next; in fact
in this case there is no DMA involved at all.  hcd_buffer_alloc hits
the case "some USB hosts just use PIO".

On those host controllers, it should be reproducible with just this:

diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 7fcb9f782931..cc0460730bce 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -905,9 +905,12 @@ EXPORT_SYMBOL_GPL(__usb_get_extra_descriptor);
 void *usb_alloc_coherent(struct usb_device *dev, size_t size, gfp_t mem_flags,
 			 dma_addr_t *dma)
 {
+	void *buf;
 	if (!dev || !dev->bus)
 		return NULL;
-	return hcd_buffer_alloc(dev->bus, size, mem_flags, dma);
+	buf = hcd_buffer_alloc(dev->bus, size, mem_flags, dma);
+	WARN_ON_ONCE(virt_to_phys(buf) & ~PAGE_MASK);
+	return buf;
 }
 EXPORT_SYMBOL_GPL(usb_alloc_coherent);
 

and CONFIG_KASAN=y or possibly just CONFIG_DEBUG_SLAB=y.  mmap-ing /dev/usb
should warn if my analysis is correct.

If you think the above patch makes sense, I can test it and submit it formally.

Paolo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH] serial: uartps: Fix uartps_major handling
From: Michal Simek @ 2019-09-13  7:28 UTC (permalink / raw)
  To: linux-kernel, monstr, michal.simek, git, Paul Thomas
  Cc: Greg Kroah-Hartman, linux-serial, linux-arm-kernel, Jiri Slaby

There are two parts which should be fixed. The first one is to assigned
uartps_major at the end of probe() to avoid complicated logic when
something fails.
The second part is initialized uartps_major number to 0 when last device is
removed. This will ensure that on next probe driver will ask for new
dynamic major number.

Fixes: c9712e333809 ("serial: uartps: Use the same dynamic major number for all ports")
Reported-by: Paul Thomas <pthomas8589@gmail.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/tty/serial/xilinx_uartps.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index f145946f659b..92df0c4f1c7a 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1550,7 +1550,6 @@ static int cdns_uart_probe(struct platform_device *pdev)
 		goto err_out_id;
 	}
 
-	uartps_major = cdns_uart_uart_driver->tty_driver->major;
 	cdns_uart_data->cdns_uart_driver = cdns_uart_uart_driver;
 
 	/*
@@ -1680,6 +1679,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
 		console_port = NULL;
 #endif
 
+	uartps_major = cdns_uart_uart_driver->tty_driver->major;
 	cdns_uart_data->cts_override = of_property_read_bool(pdev->dev.of_node,
 							     "cts-override");
 	return 0;
@@ -1741,6 +1741,12 @@ static int cdns_uart_remove(struct platform_device *pdev)
 		console_port = NULL;
 #endif
 
+	/* If this is last instance major number should be initialized */
+	mutex_lock(&bitmap_lock);
+	if (bitmap_empty(bitmap, MAX_UART_INSTANCES))
+		uartps_major = 0;
+	mutex_unlock(&bitmap_lock);
+
 	uart_unregister_driver(cdns_uart_data->cdns_uart_driver);
 	return rc;
 }
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v5 0/4] Raspberry Pi 4 DMA addressing support
From: Matthias Brugger @ 2019-09-13  7:15 UTC (permalink / raw)
  To: Stefan Wahren, Matthias Brugger, catalin.marinas, marc.zyngier,
	robh+dt, linux-mm, linux-arm-kernel, linux-riscv, hch,
	Nicolas Saenz Julienne
  Cc: f.fainelli, robin.murphy, linux-kernel, linux-rpi-kernel, phill,
	will, m.szyprowski
In-Reply-To: <2c3e1ef3-0dba-9f79-52e2-314b6b500e14@gmx.net>



On 12/09/2019 21:32, Stefan Wahren wrote:
> 
> Am 12.09.19 um 19:18 schrieb Matthias Brugger:
>>
>> On 10/09/2019 11:27, Matthias Brugger wrote:
>>>
>>> On 09/09/2019 21:33, Stefan Wahren wrote:
>>>> Hi Nicolas,
>>>>
>>>> Am 09.09.19 um 11:58 schrieb Nicolas Saenz Julienne:
>>>>> Hi all,
>>>>> this series attempts to address some issues we found while bringing up
>>>>> the new Raspberry Pi 4 in arm64 and it's intended to serve as a follow
>>>>> up of these discussions:
>>>>> v4: https://lkml.org/lkml/2019/9/6/352
>>>>> v3: https://lkml.org/lkml/2019/9/2/589
>>>>> v2: https://lkml.org/lkml/2019/8/20/767
>>>>> v1: https://lkml.org/lkml/2019/7/31/922
>>>>> RFC: https://lkml.org/lkml/2019/7/17/476
>>>>>
>>>>> The new Raspberry Pi 4 has up to 4GB of memory but most peripherals can
>>>>> only address the first GB: their DMA address range is
>>>>> 0xc0000000-0xfc000000 which is aliased to the first GB of physical
>>>>> memory 0x00000000-0x3c000000. Note that only some peripherals have these
>>>>> limitations: the PCIe, V3D, GENET, and 40-bit DMA channels have a wider
>>>>> view of the address space by virtue of being hooked up trough a second
>>>>> interconnect.
>>>>>
>>>>> Part of this is solved on arm32 by setting up the machine specific
>>>>> '.dma_zone_size = SZ_1G', which takes care of reserving the coherent
>>>>> memory area at the right spot. That said no buffer bouncing (needed for
>>>>> dma streaming) is available at the moment, but that's a story for
>>>>> another series.
>>>>>
>>>>> Unfortunately there is no such thing as 'dma_zone_size' in arm64. Only
>>>>> ZONE_DMA32 is created which is interpreted by dma-direct and the arm64
>>>>> arch code as if all peripherals where be able to address the first 4GB
>>>>> of memory.
>>>>>
>>>>> In the light of this, the series implements the following changes:
>>>>>
>>>>> - Create both DMA zones in arm64, ZONE_DMA will contain the first 1G
>>>>>   area and ZONE_DMA32 the rest of the 32 bit addressable memory. So far
>>>>>   the RPi4 is the only arm64 device with such DMA addressing limitations
>>>>>   so this hardcoded solution was deemed preferable.
>>>>>
>>>>> - Properly set ARCH_ZONE_DMA_BITS.
>>>>>
>>>>> - Reserve the CMA area in a place suitable for all peripherals.
>>>>>
>>>>> This series has been tested on multiple devices both by checking the
>>>>> zones setup matches the expectations and by double-checking physical
>>>>> addresses on pages allocated on the three relevant areas GFP_DMA,
>>>>> GFP_DMA32, GFP_KERNEL:
>>>>>
>>>>> - On an RPi4 with variations on the ram memory size. But also forcing
>>>>>   the situation where all three memory zones are nonempty by setting a 3G
>>>>>   ZONE_DMA32 ceiling on a 4G setup. Both with and without NUMA support.
>>>>>
>>>> i like to test this series on Raspberry Pi 4 and i have some questions
>>>> to get arm64 running:
>>>>
>>>> Do you use U-Boot? Which tree?
>>> If you want to use U-Boot, try v2019.10-rc4, it should have everything you need
>>> to boot your kernel.
>>>
>> Ok, here is a thing. In the linux kernel we now use bcm2711 as SoC name, but the
>> RPi4 devicetree provided by the FW uses mostly bcm2838.
> 
> Do you mean the DTB provided at runtime?
> 

Yes.

> You mean the merged U-Boot changes, doesn't work with my Raspberry Pi
> series?
> 

Unfortunately that is exactly the state right now.

>>  U-Boot in its default
>> config uses the devicetree provided by the FW, mostly because this way you don't
>> have to do anything to find out how many RAM you really have. Secondly because
>> this will allow us, in the near future, to have one U-boot binary for both RPi3
>> and RPi4 (and as a side effect one binary for RPi1 and RPi2).
>>
>> Anyway, I found at least, that the following compatibles need to be added:
>>
>> "brcm,bcm2838-cprman"
>> "brcm,bcm2838-gpio"
>>
>> Without at least the cprman driver update, you won't see anything.
>>
>> "brcm,bcm2838-rng200" is also a candidate.
>>
>> I also suppose we will need to add "brcm,bcm2838" to
>> arch/arm/mach-bcm/bcm2711.c, but I haven't verified this.
> How about changing this in the downstream kernel? Which is much easier.
>>
>> Regards,
>> Matthias
>>
>>> Regards,
>>> Matthias
>>>
>>>> Are there any config.txt tweaks necessary?
>>>>
>>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox