* [PATCH v4 07/16] spi: dw: Use DMA max burst to set the request thresholds
From: Serge Semin @ 2020-05-22 0:07 UTC (permalink / raw)
To: Mark Brown
Cc: Serge Semin, Serge Semin, Andy Shevchenko, Alexey Malahov,
Thomas Bogendoerfer, Paul Burton, Ralf Baechle, Arnd Bergmann,
Rob Herring, linux-mips, devicetree, Georgy Vlasov, Ramil Zaripov,
Wan Ahmad Zainie, Thomas Gleixner, Jarkko Nikula, Clement Leger,
Linus Walleij, linux-spi, linux-kernel
In-Reply-To: <20200522000806.7381-1-Sergey.Semin@baikalelectronics.ru>
Each channel of DMA controller may have a limited length of burst
transaction (number of IO operations performed at ones in a single
DMA client request). This parameter can be used to setup the most
optimal DMA Tx/Rx data level values. In order to avoid the Tx buffer
overrun we can set the DMA Tx level to be of FIFO depth minus the
maximum burst transactions length. To prevent the Rx buffer underflow
the DMA Rx level should be set to the maximum burst transactions length.
This commit setups the DMA channels and the DW SPI DMA Tx/Rx levels
in accordance with these rules.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v3:
- Use min() method to calculate the optimal burst values.
---
drivers/spi/spi-dw-mid.c | 37 +++++++++++++++++++++++++++++++++----
drivers/spi/spi-dw.h | 2 ++
2 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-dw-mid.c b/drivers/spi/spi-dw-mid.c
index 1598c36c905f..ac96b99ef226 100644
--- a/drivers/spi/spi-dw-mid.c
+++ b/drivers/spi/spi-dw-mid.c
@@ -35,6 +35,31 @@ static bool mid_spi_dma_chan_filter(struct dma_chan *chan, void *param)
return true;
}
+static void mid_spi_maxburst_init(struct dw_spi *dws)
+{
+ struct dma_slave_caps caps;
+ u32 max_burst, def_burst;
+ int ret;
+
+ def_burst = dws->fifo_len / 2;
+
+ ret = dma_get_slave_caps(dws->rxchan, &caps);
+ if (!ret && caps.max_burst)
+ max_burst = caps.max_burst;
+ else
+ max_burst = RX_BURST_LEVEL;
+
+ dws->rxburst = min(max_burst, def_burst);
+
+ ret = dma_get_slave_caps(dws->txchan, &caps);
+ if (!ret && caps.max_burst)
+ max_burst = caps.max_burst;
+ else
+ max_burst = TX_BURST_LEVEL;
+
+ dws->txburst = min(max_burst, def_burst);
+}
+
static int mid_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
{
struct dw_dma_slave slave = {
@@ -70,6 +95,8 @@ static int mid_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
dws->master->dma_rx = dws->rxchan;
dws->master->dma_tx = dws->txchan;
+ mid_spi_maxburst_init(dws);
+
return 0;
free_rxchan:
@@ -95,6 +122,8 @@ static int mid_spi_dma_init_generic(struct device *dev, struct dw_spi *dws)
dws->master->dma_rx = dws->rxchan;
dws->master->dma_tx = dws->txchan;
+ mid_spi_maxburst_init(dws);
+
return 0;
}
@@ -200,7 +229,7 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_tx(struct dw_spi *dws,
memset(&txconf, 0, sizeof(txconf));
txconf.direction = DMA_MEM_TO_DEV;
txconf.dst_addr = dws->dma_addr;
- txconf.dst_maxburst = TX_BURST_LEVEL;
+ txconf.dst_maxburst = dws->txburst;
txconf.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
txconf.dst_addr_width = convert_dma_width(dws->n_bytes);
txconf.device_fc = false;
@@ -275,7 +304,7 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_rx(struct dw_spi *dws,
memset(&rxconf, 0, sizeof(rxconf));
rxconf.direction = DMA_DEV_TO_MEM;
rxconf.src_addr = dws->dma_addr;
- rxconf.src_maxburst = RX_BURST_LEVEL;
+ rxconf.src_maxburst = dws->rxburst;
rxconf.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
rxconf.src_addr_width = convert_dma_width(dws->n_bytes);
rxconf.device_fc = false;
@@ -300,8 +329,8 @@ static int mid_spi_dma_setup(struct dw_spi *dws, struct spi_transfer *xfer)
{
u16 imr = 0, dma_ctrl = 0;
- dw_writel(dws, DW_SPI_DMARDLR, RX_BURST_LEVEL - 1);
- dw_writel(dws, DW_SPI_DMATDLR, TX_BURST_LEVEL);
+ dw_writel(dws, DW_SPI_DMARDLR, dws->rxburst - 1);
+ dw_writel(dws, DW_SPI_DMATDLR, dws->fifo_len - dws->txburst);
if (xfer->tx_buf) {
dma_ctrl |= SPI_DMA_TDMAE;
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 4902f937c3d7..d0c8b7d3a5d2 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -141,7 +141,9 @@ struct dw_spi {
/* DMA info */
struct dma_chan *txchan;
+ u32 txburst;
struct dma_chan *rxchan;
+ u32 rxburst;
unsigned long dma_chan_busy;
dma_addr_t dma_addr; /* phy address of the Data register */
const struct dw_spi_dma_ops *dma_ops;
--
2.25.1
^ permalink raw reply related
* [PATCH v4 04/16] spi: dw: Discard unused void priv pointer
From: Serge Semin @ 2020-05-22 0:07 UTC (permalink / raw)
To: Mark Brown
Cc: Serge Semin, Serge Semin, Andy Shevchenko, Georgy Vlasov,
Ramil Zaripov, Alexey Malahov, Thomas Bogendoerfer, Paul Burton,
Ralf Baechle, Arnd Bergmann, Rob Herring, linux-mips, devicetree,
Wan Ahmad Zainie, Linus Walleij, Clement Leger, linux-spi,
linux-kernel
In-Reply-To: <20200522000806.7381-1-Sergey.Semin@baikalelectronics.ru>
Seeing the "void *priv" member of the dw_spi data structure is unused
let's remove it. The glue-layers can embed the DW APB SSI controller
descriptor into their private data object. MMIO driver for instance
already utilizes that design pattern.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v2:
- It's a new patch created as a result of more thorough driver study.
---
drivers/spi/spi-dw.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 60e9e430ce7b..b6ab81e0c747 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -147,8 +147,6 @@ struct dw_spi {
dma_addr_t dma_addr; /* phy address of the Data register */
const struct dw_spi_dma_ops *dma_ops;
- /* Bus interface info */
- void *priv;
#ifdef CONFIG_DEBUG_FS
struct dentry *debugfs;
#endif
--
2.25.1
^ permalink raw reply related
* [PATCH v4 02/16] spi: dw: Enable interrupts in accordance with DMA xfer mode
From: Serge Semin @ 2020-05-22 0:07 UTC (permalink / raw)
To: Mark Brown
Cc: Serge Semin, Serge Semin, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Andy Shevchenko, Rob Herring, linux-mips,
devicetree, Wan Ahmad Zainie, Jarkko Nikula, Thomas Gleixner,
linux-spi, linux-kernel
In-Reply-To: <20200522000806.7381-1-Sergey.Semin@baikalelectronics.ru>
It's pointless to track the Tx overrun interrupts if Rx-only SPI
transfer is issued. Similarly there is no need in handling the Rx
overrun/underrun interrupts if Tx-only SPI transfer is executed.
So lets unmask the interrupts only if corresponding SPI
transactions are implied.
Co-developed-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Signed-off-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
drivers/spi/spi-dw-mid.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-dw-mid.c b/drivers/spi/spi-dw-mid.c
index 2152c0cb3953..f24fcd925b48 100644
--- a/drivers/spi/spi-dw-mid.c
+++ b/drivers/spi/spi-dw-mid.c
@@ -297,19 +297,23 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_rx(struct dw_spi *dws,
static int mid_spi_dma_setup(struct dw_spi *dws, struct spi_transfer *xfer)
{
- u16 dma_ctrl = 0;
+ u16 imr = 0, dma_ctrl = 0;
dw_writel(dws, DW_SPI_DMARDLR, 0xf);
dw_writel(dws, DW_SPI_DMATDLR, 0x10);
- if (xfer->tx_buf)
+ if (xfer->tx_buf) {
dma_ctrl |= SPI_DMA_TDMAE;
- if (xfer->rx_buf)
+ imr |= SPI_INT_TXOI;
+ }
+ if (xfer->rx_buf) {
dma_ctrl |= SPI_DMA_RDMAE;
+ imr |= SPI_INT_RXUI | SPI_INT_RXOI;
+ }
dw_writel(dws, DW_SPI_DMACR, dma_ctrl);
/* Set the interrupt mask */
- spi_umask_intr(dws, SPI_INT_TXOI | SPI_INT_RXUI | SPI_INT_RXOI);
+ spi_umask_intr(dws, imr);
dws->transfer_handler = dma_transfer;
--
2.25.1
^ permalink raw reply related
* [PATCH v4 00/16] spi: dw: Add generic DW DMA controller support
From: Serge Semin @ 2020-05-22 0:07 UTC (permalink / raw)
To: Mark Brown
Cc: Serge Semin, Serge Semin, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Maxim Kaurkin, Pavel Parkhomenko,
Ekaterina Skachko, Vadim Vlasov, Alexey Kolotnikov,
Thomas Bogendoerfer, Paul Burton, Ralf Baechle, Arnd Bergmann,
Andy Shevchenko, Rob Herring, linux-mips, linux-spi, devicetree,
linux-kernel
Baikal-T1 SoC provides a DW DMA controller to perform low-speed peripherals
Mem-to-Dev and Dev-to-Mem transaction. This is also applicable to the DW
APB SSI devices embedded into the SoC. Currently the DMA-based transfers
are supported by the DW APB SPI driver only as a middle layer code for
Intel MID/Elkhart PCI devices. Seeing the same code can be used for normal
platform DMAC device we introduced a set of patches to fix it within this
series.
First of all we need to add the Tx and Rx DMA channels support into the DW
APB SSI binding. Then there are several fixes and cleanups provided as a
initial preparation for the Generic DMA support integration: add Tx/Rx
finish wait methods, clear DMAC register when done or stopped, Fix native
CS being unset, enable interrupts in accordance with DMA xfer mode,
discard static DW DMA slave structures, discard unused void priv pointer
and dma_width member of the dw_spi structure, provide the DMA Tx/Rx burst
length parametrisation and make sure it's optionally set in accordance
with the DMA max-burst capability.
In order to have the DW APB SSI MMIO driver working with DMA we need to
initialize the paddr field with the physical base address of the DW APB SSI
registers space. Then we unpin the Intel MID specific code from the
generic DMA one and placed it into the spi-dw-pci.c driver, which is a
better place for it anyway. After that the naming cleanups are performed
since the code is going to be used for a generic DMAC device. Finally the
Generic DMA initialization can be added to the generic version of the
DW APB SSI IP.
Last but not least we traditionally convert the legacy plain text-based
dt-binding file with yaml-based one and as a cherry on a cake replace
the manually written DebugFS registers read method with a ready-to-use
for the same purpose regset32 DebugFS interface usage.
This patchset is rebased and tested on the spi/for-next (5.7-rc5):
base-commit: fe9fce6b2cf3 ("Merge remote-tracking branch 'spi/for-5.8' into spi-next")
Link: https://lore.kernel.org/linux-spi/20200508132943.9826-1-Sergey.Semin@baikalelectronics.ru/
Changelog v2:
- Rebase on top of the spi repository for-next branch.
- Move bindings conversion patch to the tail of the series.
- Move fixes to the head of the series.
- Apply as many changes as possible to be applied the Generic DMA
functionality support is added and the spi-dw-mid is moved to the
spi-dw-dma driver.
- Discard patch "spi: dw: Fix dma_slave_config used partly uninitialized"
since the problem has already been fixed.
- Add new patch "spi: dw: Discard unused void priv pointer".
- Add new patch "spi: dw: Discard dma_width member of the dw_spi structure".
n_bytes member of the DW SPI data can be used instead.
- Build the DMA functionality into the DW APB SSI core if required instead
of creating a separate kernel module.
- Use conditional statement instead of the ternary operator in the ref
clock getter.
Link: https://lore.kernel.org/linux-spi/20200515104758.6934-1-Sergey.Semin@baikalelectronics.ru/
Changelog v3:
- Use spi_delay_exec() method to wait for the DMA operation completion.
- Explicitly initialize the dw_dma_slave members on stack.
- Discard the dws->fifo_len utilization in the Tx FIFO DMA threshold
setting from the patch where we just add the default burst length
constants.
- Use min() method to calculate the optimal burst values.
- Add new patch which moves the spi-dw.c source file to spi-dw-core.c in
order to preserve the DW APB SSI core driver name.
- Add commas in the debugfs_reg32 structure initializer and after the last
entry of the dw_spi_dbgfs_regs array.
Link: https://lore.kernel.org/linux-spi/20200521012206.14472-1-Sergey.Semin@baikalelectronics.ru
Changelog v4:
- Get back ndelay() method to wait for an SPI transfer completion.
spi_delay_exec() isn't suitable for the atomic context.
Co-developed-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Signed-off-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Co-developed-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Signed-off-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-spi@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Serge Semin (16):
spi: dw: Add Tx/Rx finish wait methods to the MID DMA
spi: dw: Enable interrupts in accordance with DMA xfer mode
spi: dw: Discard static DW DMA slave structures
spi: dw: Discard unused void priv pointer
spi: dw: Discard dma_width member of the dw_spi structure
spi: dw: Parameterize the DMA Rx/Tx burst length
spi: dw: Use DMA max burst to set the request thresholds
spi: dw: Fix Rx-only DMA transfers
spi: dw: Add core suffix to the DW APB SSI core source file
spi: dw: Move Non-DMA code to the DW PCIe-SPI driver
spi: dw: Remove DW DMA code dependency from DW_DMAC_PCI
spi: dw: Add DW SPI DMA/PCI/MMIO dependency on the DW SPI core
spi: dw: Cleanup generic DW DMA code namings
spi: dw: Add DMA support to the DW SPI MMIO driver
spi: dw: Use regset32 DebugFS method to create regdump file
dt-bindings: spi: Convert DW SPI binding to DT schema
.../bindings/spi/snps,dw-apb-ssi.txt | 44 ---
.../bindings/spi/snps,dw-apb-ssi.yaml | 127 +++++++++
.../devicetree/bindings/spi/spi-dw.txt | 24 --
drivers/spi/Kconfig | 15 +-
drivers/spi/Makefile | 5 +-
drivers/spi/{spi-dw.c => spi-dw-core.c} | 88 ++----
drivers/spi/{spi-dw-mid.c => spi-dw-dma.c} | 261 ++++++++++--------
drivers/spi/spi-dw-mmio.c | 4 +
drivers/spi/spi-dw-pci.c | 50 +++-
drivers/spi/spi-dw.h | 33 ++-
10 files changed, 392 insertions(+), 259 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
create mode 100644 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
delete mode 100644 Documentation/devicetree/bindings/spi/spi-dw.txt
rename drivers/spi/{spi-dw.c => spi-dw-core.c} (82%)
rename drivers/spi/{spi-dw-mid.c => spi-dw-dma.c} (55%)
--
2.25.1
^ permalink raw reply
* Re: [PATCH v2 0/4] TI K3 DSP remoteproc driver for C66x DSPs
From: Bjorn Andersson @ 2020-05-22 0:03 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Suman Anna, Rob Herring, Lokesh Vutla, linux-remoteproc,
devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20200521222334.GA11366@xps15>
On Thu 21 May 15:23 PDT 2020, Mathieu Poirier wrote:
> Gents,
>
> On Thu, May 21, 2020 at 12:01:41PM -0700, Bjorn Andersson wrote:
> > On Thu 21 May 11:59 PDT 2020, Suman Anna wrote:
> >
> > > Hi Bjorn,
> > >
> > > On 5/20/20 7:10 PM, Suman Anna wrote:
> > > > Hi All,
> > > >
> > > > The following is v2 of the K3 DSP remoteproc driver supporting the C66x DSPs
> > > > on the TI K3 J721E SoCs. The patches are based on the latest commit on the
> > > > rproc-next branch, 7dcef3988eed ("remoteproc: Fix an error code in
> > > > devm_rproc_alloc()").
> > >
> > > I realized I also had the R5F patches on my branch, so the third patch won't
> > > apply cleanly (conflict on Makefile). Let me know if you want a new revision
> > > posted for you to pick up the series.
> > >
> >
> > That should be fine, thanks for the heads up!
> >
> > Will give Mathieu a day or two to take a look as well.
>
> I don't see having the time to review this set before the middle/end of next
> week. I also understand we are crunched by time if we want to get this in
> for the upcoming merge window.
>
> If memory serves me well there wasn't anything controversial about this work.
> Under normal circumstances I'd give it a final look but I trust Suman to have
> carried out what we agreed on.
>
> Bjorn - if you are happy with this set then go ahead and queue it.
>
Thanks Mathieu.
I looked through the patches again and saw that we're still waiting for
a ack on the dt binding, so I guess I need to hold off on this a little
bit longer.
Regards,
Bjorn
^ permalink raw reply
* Re: [PATCH v3 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA
From: Serge Semin @ 2020-05-21 23:33 UTC (permalink / raw)
To: Mark Brown, Grant Likely, Vinod Koul, Feng Tang, Alan Cox,
Linus Walleij
Cc: Serge Semin, Georgy Vlasov, Ramil Zaripov, Alexey Malahov,
Thomas Bogendoerfer, Paul Burton, Ralf Baechle, Arnd Bergmann,
Andy Shevchenko, Rob Herring, linux-mips, devicetree,
Jarkko Nikula, Thomas Gleixner, Wan Ahmad Zainie, Linus Walleij,
Clement Leger, linux-spi, linux-kernel
In-Reply-To: <20200521012206.14472-2-Sergey.Semin@baikalelectronics.ru>
Mark, Andy,
On Thu, May 21, 2020 at 04:21:51AM +0300, Serge Semin wrote:
>
[nip]
> +static void dw_spi_dma_calc_delay(struct dw_spi *dws, u32 nents,
> + struct spi_delay *delay)
> +{
> + unsigned long ns, us;
> +
> + ns = (NSEC_PER_SEC / spi_get_clk(dws)) * nents * dws->n_bytes *
> + BITS_PER_BYTE;
> +
> + if (ns <= NSEC_PER_USEC) {
> + delay->unit = SPI_DELAY_UNIT_NSECS;
> + delay->value = ns;
> + } else {
> + us = DIV_ROUND_UP(ns, NSEC_PER_USEC);
> + delay->unit = SPI_DELAY_UNIT_USECS;
> + delay->value = clamp_val(us, 0, USHRT_MAX);
> + }
> +}
> +
> +static inline bool dw_spi_dma_tx_busy(struct dw_spi *dws)
> +{
> + return !(dw_readl(dws, DW_SPI_SR) & SR_TF_EMPT);
> +}
> +
> +static void dw_spi_dma_wait_tx_done(struct dw_spi *dws)
> +{
> + int retry = WAIT_RETRIES;
> + struct spi_delay delay;
> + u32 nents;
> +
> + nents = dw_readl(dws, DW_SPI_TXFLR);
> + dw_spi_dma_calc_delay(dws, nents, &delay);
> +
> + while (dw_spi_dma_tx_busy(dws) && retry--)
> + spi_delay_exec(&delay, NULL);
I've just discovered using spi_delay_exec() wasn't a good idea here. Look at the
call stack:
dw_dma_tasklet() -> dwc_scan_descriptors() -> dwc_descriptor_complete() ->
dw_spi_dma_tx_done() -> spi_delay_exec() -> usleep_range() -> ...
So tasklet is calling a sleeping function.((( I've absolutely forgotten to check
the context the DMA completion function is called with. We'll have to manually
select either ndelay or udelay here and nothing else. Since basically both
functions represent an atomic context and most of the platforms ndelay fallsback to
udelay, I'll get the ndelay back to the wait functions. I'll resend a patchset
shortly.
-Sergey
^ permalink raw reply
* Re: [PATCH 10/12] of/irq: Make of_msi_map_rid() PCI bus agnostic
From: Rob Herring @ 2020-05-21 23:17 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Marc Zyngier, Linux IOMMU, linux-acpi, devicetree, PCI,
Rafael J. Wysocki, Joerg Roedel, Hanjun Guo, Bjorn Helgaas,
Sudeep Holla, Robin Murphy, Catalin Marinas, Will Deacon,
Makarand Pawagi, Diana Craciun, Laurentiu Tudor
In-Reply-To: <20200521130008.8266-11-lorenzo.pieralisi@arm.com>
On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> There is nothing PCI bus specific in the of_msi_map_rid()
> implementation other than the requester ID tag for the input
> ID space. Rename requester ID to a more generic ID so that
> the translation code can be used by all busses that require
> input/output ID translations.
>
> Leave a wrapper function of_msi_map_rid() in place to keep
> existing PCI code mapping requester ID syntactically unchanged.
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Marc Zyngier <maz@kernel.org>
> ---
> drivers/of/irq.c | 28 ++++++++++++++--------------
> include/linux/of_irq.h | 14 ++++++++++++--
> 2 files changed, 26 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 48a40326984f..25d17b8a1a1a 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -576,43 +576,43 @@ void __init of_irq_init(const struct of_device_id *matches)
> }
> }
>
> -static u32 __of_msi_map_rid(struct device *dev, struct device_node **np,
> - u32 rid_in)
> +static u32 __of_msi_map_id(struct device *dev, struct device_node **np,
> + u32 id_in)
> {
> struct device *parent_dev;
> - u32 rid_out = rid_in;
> + u32 id_out = id_in;
>
> /*
> * Walk up the device parent links looking for one with a
> * "msi-map" property.
> */
> for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent)
> - if (!of_map_rid(parent_dev->of_node, rid_in, "msi-map",
> - "msi-map-mask", np, &rid_out))
> + if (!of_map_id(parent_dev->of_node, id_in, "msi-map",
> + "msi-map-mask", np, &id_out))
> break;
> - return rid_out;
> + return id_out;
> }
>
> /**
> - * of_msi_map_rid - Map a MSI requester ID for a device.
> + * of_msi_map_id - Map a MSI ID for a device.
> * @dev: device for which the mapping is to be done.
> * @msi_np: device node of the expected msi controller.
> - * @rid_in: unmapped MSI requester ID for the device.
> + * @id_in: unmapped MSI ID for the device.
> *
> * Walk up the device hierarchy looking for devices with a "msi-map"
> - * property. If found, apply the mapping to @rid_in.
> + * property. If found, apply the mapping to @id_in.
> *
> - * Returns the mapped MSI requester ID.
> + * Returns the mapped MSI ID.
> */
> -u32 of_msi_map_rid(struct device *dev, struct device_node *msi_np, u32 rid_in)
> +u32 of_msi_map_id(struct device *dev, struct device_node *msi_np, u32 id_in)
> {
> - return __of_msi_map_rid(dev, &msi_np, rid_in);
> + return __of_msi_map_id(dev, &msi_np, id_in);
> }
>
> /**
> * of_msi_map_get_device_domain - Use msi-map to find the relevant MSI domain
> * @dev: device for which the mapping is to be done.
> - * @rid: Requester ID for the device.
> + * @id: Device ID.
> * @bus_token: Bus token
> *
> * Walk up the device hierarchy looking for devices with a "msi-map"
> @@ -625,7 +625,7 @@ struct irq_domain *of_msi_map_get_device_domain(struct device *dev, u32 id,
> {
> struct device_node *np = NULL;
>
> - __of_msi_map_rid(dev, &np, id);
> + __of_msi_map_id(dev, &np, id);
> return irq_find_matching_host(np, bus_token);
> }
>
> diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
> index 7142a3722758..cf9cb1e545ce 100644
> --- a/include/linux/of_irq.h
> +++ b/include/linux/of_irq.h
> @@ -55,7 +55,12 @@ extern struct irq_domain *of_msi_map_get_device_domain(struct device *dev,
> u32 id,
> u32 bus_token);
> extern void of_msi_configure(struct device *dev, struct device_node *np);
> -u32 of_msi_map_rid(struct device *dev, struct device_node *msi_np, u32 rid_in);
> +u32 of_msi_map_id(struct device *dev, struct device_node *msi_np, u32 id_in);
> +static inline u32 of_msi_map_rid(struct device *dev,
> + struct device_node *msi_np, u32 rid_in)
> +{
> + return of_msi_map_id(dev, msi_np, rid_in);
> +}
> #else
> static inline int of_irq_count(struct device_node *dev)
> {
> @@ -93,10 +98,15 @@ static inline struct irq_domain *of_msi_map_get_device_domain(struct device *dev
> static inline void of_msi_configure(struct device *dev, struct device_node *np)
> {
> }
> +static inline u32 of_msi_map_id(struct device *dev,
> + struct device_node *msi_np, u32 id_in)
> +{
> + return id_in;
> +}
> static inline u32 of_msi_map_rid(struct device *dev,
> struct device_node *msi_np, u32 rid_in)
Move this out of the ifdef and you only need it declared once.
But again, I think I'd just kill of_msi_map_rid.
> {
> - return rid_in;
> + return of_msi_map_id(dev, msi_np, rid_in);
> }
> #endif
>
> --
> 2.26.1
>
^ permalink raw reply
* Re: [PATCH 09/12] dt-bindings: arm: fsl: Add msi-map device-tree binding for fsl-mc bus
From: Rob Herring @ 2020-05-21 23:10 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Laurentiu Tudor, Linux IOMMU, linux-acpi, devicetree, PCI,
Rafael J. Wysocki, Joerg Roedel, Hanjun Guo, Bjorn Helgaas,
Sudeep Holla, Robin Murphy, Catalin Marinas, Will Deacon,
Marc Zyngier, Makarand Pawagi, Diana Craciun
In-Reply-To: <20200521130008.8266-10-lorenzo.pieralisi@arm.com>
On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>
> The existing bindings cannot be used to specify the relationship
> between fsl-mc devices and GIC ITSes.
>
> Add a generic binding for mapping fsl-mc devices to GIC ITSes, using
> msi-map property.
>
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> ---
> .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 30 +++++++++++++++++--
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> index 9134e9bcca56..b0813b2d0493 100644
> --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> @@ -18,9 +18,9 @@ same hardware "isolation context" and a 10-bit value called an ICID
> the requester.
>
> The generic 'iommus' property is insufficient to describe the relationship
> -between ICIDs and IOMMUs, so an iommu-map property is used to define
> -the set of possible ICIDs under a root DPRC and how they map to
> -an IOMMU.
> +between ICIDs and IOMMUs, so the iommu-map and msi-map properties are used
> +to define the set of possible ICIDs under a root DPRC and how they map to
> +an IOMMU and a GIC ITS respectively.
>
> For generic IOMMU bindings, see
> Documentation/devicetree/bindings/iommu/iommu.txt.
> @@ -28,6 +28,9 @@ Documentation/devicetree/bindings/iommu/iommu.txt.
> For arm-smmu binding, see:
> Documentation/devicetree/bindings/iommu/arm,smmu.yaml.
>
> +For GICv3 and GIC ITS bindings, see:
> +Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml.
> +
> Required properties:
>
> - compatible
> @@ -119,6 +122,15 @@ Optional properties:
> associated with the listed IOMMU, with the iommu-specifier
> (i - icid-base + iommu-base).
>
> +- msi-map: Maps an ICID to a GIC ITS and associated iommu-specifier
> + data.
> +
> + The property is an arbitrary number of tuples of
> + (icid-base,iommu,iommu-base,length).
I'm confused because the example has GIC ITS phandle, not an IOMMU.
What is an iommu-base?
> +
> + Any ICID in the interval [icid-base, icid-base + length) is
> + associated with the listed GIC ITS, with the iommu-specifier
> + (i - icid-base + iommu-base).
> Example:
>
> smmu: iommu@5000000 {
> @@ -128,6 +140,16 @@ Example:
> ...
> };
>
> + gic: interrupt-controller@6000000 {
> + compatible = "arm,gic-v3";
> + ...
> + its: gic-its@6020000 {
> + compatible = "arm,gic-v3-its";
> + msi-controller;
> + ...
> + };
> + };
> +
> fsl_mc: fsl-mc@80c000000 {
> compatible = "fsl,qoriq-mc";
> reg = <0x00000008 0x0c000000 0 0x40>, /* MC portal base */
> @@ -135,6 +157,8 @@ Example:
> msi-parent = <&its>;
> /* define map for ICIDs 23-64 */
> iommu-map = <23 &smmu 23 41>;
> + /* define msi map for ICIDs 23-64 */
> + msi-map = <23 &its 23 41>;
Seeing 23 twice is odd. The numbers to the right of 'its' should be an
ITS number space.
> #address-cells = <3>;
> #size-cells = <1>;
>
> --
> 2.26.1
>
^ permalink raw reply
* Re: [PATCH 07/12] of/device: Add input id to of_dma_configure()
From: Rob Herring @ 2020-05-21 23:02 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Robin Murphy, Joerg Roedel, Laurentiu Tudor, Linux IOMMU,
linux-acpi, devicetree, PCI, Rafael J. Wysocki, Hanjun Guo,
Bjorn Helgaas, Sudeep Holla, Catalin Marinas, Will Deacon,
Marc Zyngier, Makarand Pawagi, Diana Craciun
In-Reply-To: <20200521130008.8266-8-lorenzo.pieralisi@arm.com>
On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> Devices sitting on proprietary busses have a device ID space that
> is owned by the respective bus and related firmware bindings. In order
> to let the generic OF layer handle the input translations to
> an IOMMU id, for such busses the current of_dma_configure() interface
> should be extended in order to allow the bus layer to provide the
> device input id parameter - that is retrieved/assigned in bus
> specific code and firmware.
>
> Augment of_dma_configure() to add an optional input_id parameter,
> leaving current functionality unchanged.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> ---
> drivers/bus/fsl-mc/fsl-mc-bus.c | 4 ++-
> drivers/iommu/of_iommu.c | 53 +++++++++++++++++++++------------
> drivers/of/device.c | 8 +++--
> include/linux/of_device.h | 16 ++++++++--
> include/linux/of_iommu.h | 6 ++--
> 5 files changed, 60 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index 40526da5c6a6..8ead3f0238f2 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -118,11 +118,13 @@ static int fsl_mc_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
> static int fsl_mc_dma_configure(struct device *dev)
> {
> struct device *dma_dev = dev;
> + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> + u32 input_id = mc_dev->icid;
>
> while (dev_is_fsl_mc(dma_dev))
> dma_dev = dma_dev->parent;
>
> - return of_dma_configure(dev, dma_dev->of_node, 0);
> + return of_dma_configure_id(dev, dma_dev->of_node, 0, &input_id);
> }
>
> static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index ad96b87137d6..4516d5bf6cc9 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -139,25 +139,53 @@ static int of_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
> return err;
> }
>
> -static int of_fsl_mc_iommu_init(struct fsl_mc_device *mc_dev,
> - struct device_node *master_np)
> +static int of_iommu_configure_dev_id(struct device_node *master_np,
> + struct device *dev,
> + const u32 *id)
Should have read this patch before #6. I guess you could still make
of_pci_iommu_init() call
of_iommu_configure_dev_id.
Rob
^ permalink raw reply
* Re: [PATCH 06/12] of/iommu: Make of_map_rid() PCI agnostic
From: Rob Herring @ 2020-05-21 22:47 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Joerg Roedel, Robin Murphy, Marc Zyngier, Linux IOMMU, linux-acpi,
devicetree, PCI, Rafael J. Wysocki, Hanjun Guo, Bjorn Helgaas,
Sudeep Holla, Catalin Marinas, Will Deacon, Makarand Pawagi,
Diana Craciun, Laurentiu Tudor
In-Reply-To: <20200521130008.8266-7-lorenzo.pieralisi@arm.com>
On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> There is nothing PCI specific (other than the RID - requester ID)
> in the of_map_rid() implementation, so the same function can be
> reused for input/output IDs mapping for other busses just as well.
>
> Rename the RID instances/names to a generic "id" tag and provide
> an of_map_rid() wrapper function so that we can leave the existing
> (and legitimate) callers unchanged.
It's not all that clear to a casual observer that RID is a PCI thing,
so I don't know that keeping it buys much. And there's only 3 callers.
> No functionality change intended.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> ---
> drivers/iommu/of_iommu.c | 2 +-
> drivers/of/base.c | 42 ++++++++++++++++++++--------------------
> include/linux/of.h | 17 +++++++++++++++-
> 3 files changed, 38 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 20738aacac89..ad96b87137d6 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -145,7 +145,7 @@ static int of_fsl_mc_iommu_init(struct fsl_mc_device *mc_dev,
> struct of_phandle_args iommu_spec = { .args_count = 1 };
> int err;
>
> - err = of_map_rid(master_np, mc_dev->icid, "iommu-map",
> + err = of_map_id(master_np, mc_dev->icid, "iommu-map",
I'm not sure this is an improvement because I'd refactor this function
and of_pci_iommu_init() into a single function:
of_bus_iommu_init(struct device *dev, struct device_node *np, u32 id)
Then of_pci_iommu_init() becomes:
of_pci_iommu_init()
{
return of_bus_iommu_init(info->dev, info->np, alias);
}
And replace of_fsl_mc_iommu_init call with:
err = of_bus_iommu_init(dev, master_np, to_fsl_mc_device(dev)->icid);
> "iommu-map-mask", &iommu_spec.np,
> iommu_spec.args);
> if (err)
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index ae03b1218b06..e000e17bd602 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2201,15 +2201,15 @@ int of_find_last_cache_level(unsigned int cpu)
> }
>
> /**
> - * of_map_rid - Translate a requester ID through a downstream mapping.
> + * of_map_id - Translate a requester ID through a downstream mapping.
Still a requester ID?
> * @np: root complex device node.
> - * @rid: device requester ID to map.
> + * @id: device ID to map.
> * @map_name: property name of the map to use.
> * @map_mask_name: optional property name of the mask to use.
> * @target: optional pointer to a target device node.
> * @id_out: optional pointer to receive the translated ID.
> *
> - * Given a device requester ID, look up the appropriate implementation-defined
> + * Given a device ID, look up the appropriate implementation-defined
> * platform ID and/or the target device which receives transactions on that
> * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or
> * @id_out may be NULL if only the other is required. If @target points to
> @@ -2219,11 +2219,11 @@ int of_find_last_cache_level(unsigned int cpu)
> *
> * Return: 0 on success or a standard error code on failure.
> */
> -int of_map_rid(struct device_node *np, u32 rid,
> +int of_map_id(struct device_node *np, u32 id,
> const char *map_name, const char *map_mask_name,
> struct device_node **target, u32 *id_out)
> {
> - u32 map_mask, masked_rid;
> + u32 map_mask, masked_id;
> int map_len;
> const __be32 *map = NULL;
>
> @@ -2235,7 +2235,7 @@ int of_map_rid(struct device_node *np, u32 rid,
> if (target)
> return -ENODEV;
> /* Otherwise, no map implies no translation */
> - *id_out = rid;
> + *id_out = id;
> return 0;
> }
>
> @@ -2255,22 +2255,22 @@ int of_map_rid(struct device_node *np, u32 rid,
> if (map_mask_name)
> of_property_read_u32(np, map_mask_name, &map_mask);
>
> - masked_rid = map_mask & rid;
> + masked_id = map_mask & id;
> for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
> struct device_node *phandle_node;
> - u32 rid_base = be32_to_cpup(map + 0);
> + u32 id_base = be32_to_cpup(map + 0);
> u32 phandle = be32_to_cpup(map + 1);
> u32 out_base = be32_to_cpup(map + 2);
> - u32 rid_len = be32_to_cpup(map + 3);
> + u32 id_len = be32_to_cpup(map + 3);
>
> - if (rid_base & ~map_mask) {
> - pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores rid-base (0x%x)\n",
> + if (id_base & ~map_mask) {
> + pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores id-base (0x%x)\n",
> np, map_name, map_name,
> - map_mask, rid_base);
> + map_mask, id_base);
> return -EFAULT;
> }
>
> - if (masked_rid < rid_base || masked_rid >= rid_base + rid_len)
> + if (masked_id < id_base || masked_id >= id_base + id_len)
> continue;
>
> phandle_node = of_find_node_by_phandle(phandle);
> @@ -2288,20 +2288,20 @@ int of_map_rid(struct device_node *np, u32 rid,
> }
>
> if (id_out)
> - *id_out = masked_rid - rid_base + out_base;
> + *id_out = masked_id - id_base + out_base;
>
> - pr_debug("%pOF: %s, using mask %08x, rid-base: %08x, out-base: %08x, length: %08x, rid: %08x -> %08x\n",
> - np, map_name, map_mask, rid_base, out_base,
> - rid_len, rid, masked_rid - rid_base + out_base);
> + pr_debug("%pOF: %s, using mask %08x, id-base: %08x, out-base: %08x, length: %08x, id: %08x -> %08x\n",
> + np, map_name, map_mask, id_base, out_base,
> + id_len, id, masked_id - id_base + out_base);
> return 0;
> }
>
> - pr_info("%pOF: no %s translation for rid 0x%x on %pOF\n", np, map_name,
> - rid, target && *target ? *target : NULL);
> + pr_info("%pOF: no %s translation for id 0x%x on %pOF\n", np, map_name,
> + id, target && *target ? *target : NULL);
>
> /* Bypasses translation */
> if (id_out)
> - *id_out = rid;
> + *id_out = id;
> return 0;
> }
> -EXPORT_SYMBOL_GPL(of_map_rid);
> +EXPORT_SYMBOL_GPL(of_map_id);
> diff --git a/include/linux/of.h b/include/linux/of.h
> index c669c0a4732f..b7934566a1aa 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -554,10 +554,18 @@ bool of_console_check(struct device_node *dn, char *name, int index);
>
> extern int of_cpu_node_to_id(struct device_node *np);
>
> -int of_map_rid(struct device_node *np, u32 rid,
> +int of_map_id(struct device_node *np, u32 id,
> const char *map_name, const char *map_mask_name,
> struct device_node **target, u32 *id_out);
>
> +static inline int of_map_rid(struct device_node *np, u32 rid,
> + const char *map_name,
> + const char *map_mask_name,
> + struct device_node **target, u32 *id_out)
> +{
> + return of_map_id(np, rid, map_name, map_mask_name, target, id_out);
> +}
> +
> #else /* CONFIG_OF */
>
> static inline void of_core_init(void)
> @@ -978,6 +986,13 @@ static inline int of_cpu_node_to_id(struct device_node *np)
> return -ENODEV;
> }
>
> +static inline int of_map_id(struct device_node *np, u32 id,
> + const char *map_name, const char *map_mask_name,
> + struct device_node **target, u32 *id_out)
> +{
> + return -EINVAL;
> +}
> +
> static inline int of_map_rid(struct device_node *np, u32 rid,
> const char *map_name, const char *map_mask_name,
> struct device_node **target, u32 *id_out)
> --
> 2.26.1
>
^ permalink raw reply
* Re: [PATCH v2 0/4] TI K3 DSP remoteproc driver for C66x DSPs
From: Mathieu Poirier @ 2020-05-21 22:23 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Suman Anna, Rob Herring, Lokesh Vutla, linux-remoteproc,
devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20200521190141.GN408178@builder.lan>
Gents,
On Thu, May 21, 2020 at 12:01:41PM -0700, Bjorn Andersson wrote:
> On Thu 21 May 11:59 PDT 2020, Suman Anna wrote:
>
> > Hi Bjorn,
> >
> > On 5/20/20 7:10 PM, Suman Anna wrote:
> > > Hi All,
> > >
> > > The following is v2 of the K3 DSP remoteproc driver supporting the C66x DSPs
> > > on the TI K3 J721E SoCs. The patches are based on the latest commit on the
> > > rproc-next branch, 7dcef3988eed ("remoteproc: Fix an error code in
> > > devm_rproc_alloc()").
> >
> > I realized I also had the R5F patches on my branch, so the third patch won't
> > apply cleanly (conflict on Makefile). Let me know if you want a new revision
> > posted for you to pick up the series.
> >
>
> That should be fine, thanks for the heads up!
>
> Will give Mathieu a day or two to take a look as well.
I don't see having the time to review this set before the middle/end of next
week. I also understand we are crunched by time if we want to get this in
for the upcoming merge window.
If memory serves me well there wasn't anything controversial about this work.
Under normal circumstances I'd give it a final look but I trust Suman to have
carried out what we agreed on.
Bjorn - if you are happy with this set then go ahead and queue it.
Thanks,
Mathieu
>
> Regards,
> Bjorn
>
> > regards
> > Suman
> >
> > >
> > > v2 includes a new remoteproc core patch (patch 1) that adds an OF helper
> > > for parsing the firmware-name property. This is refactored out to avoid
> > > replicating the code in various remoteproc drivers. Please see the
> > > individual patches for detailed changes.
> > >
> > > The main dependent patches from the previous series are now staged in
> > > rproc-next branch. The only dependency for this series is the common
> > > ti-sci-proc helper between R5 and DSP drivers [1]. Please see the initial
> > > cover-letter [2] for v1 details.
> > >
> > > regards
> > > Suman
> > >
> > > [1] https://patchwork.kernel.org/patch/11456379/
> > > [2] https://patchwork.kernel.org/cover/11458573/
> > >
> > > Suman Anna (4):
> > > remoteproc: Introduce rproc_of_parse_firmware() helper
> > > dt-bindings: remoteproc: Add bindings for C66x DSPs on TI K3 SoCs
> > > remoteproc/k3-dsp: Add a remoteproc driver of K3 C66x DSPs
> > > remoteproc/k3-dsp: Add support for L2RAM loading on C66x DSPs
> > >
> > > .../bindings/remoteproc/ti,k3-dsp-rproc.yaml | 190 +++++
> > > drivers/remoteproc/Kconfig | 13 +
> > > drivers/remoteproc/Makefile | 1 +
> > > drivers/remoteproc/remoteproc_core.c | 23 +
> > > drivers/remoteproc/remoteproc_internal.h | 2 +
> > > drivers/remoteproc/ti_k3_dsp_remoteproc.c | 773 ++++++++++++++++++
> > > 6 files changed, 1002 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> > > create mode 100644 drivers/remoteproc/ti_k3_dsp_remoteproc.c
> > >
> >
^ permalink raw reply
* Re: [PATCH v4 03/14] PCI: cadence: Add support to use custom read and write accessors
From: Rob Herring @ 2020-05-21 22:17 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Arnd Bergmann, Tom Joseph,
Greg Kroah-Hartman, PCI, devicetree, linux-kernel@vger.kernel.org,
linux-omap,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <37d2d6c3-232d-adb8-4e0b-e0c699ec435a@ti.com>
On Thu, May 21, 2020 at 7:33 AM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
> Hi Rob,
>
> On 5/21/2020 3:37 AM, Rob Herring wrote:
> > On Wed, May 06, 2020 at 08:44:18PM +0530, Kishon Vijay Abraham I wrote:
> >> Add support to use custom read and write accessors. Platforms that
> >> don't support half word or byte access or any other constraint
> >> while accessing registers can use this feature to populate custom
> >> read and write accessors. These custom accessors are used for both
> >> standard register access and configuration space register access of
> >> the PCIe host bridge.
> >>
> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >> ---
> >> drivers/pci/controller/cadence/pcie-cadence.h | 107 +++++++++++++++---
> >> 1 file changed, 94 insertions(+), 13 deletions(-)
> >
> > Actually, take back my R-by...
> >
> >>
> >> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
> >> index df14ad002fe9..70b6b25153e8 100644
> >> --- a/drivers/pci/controller/cadence/pcie-cadence.h
> >> +++ b/drivers/pci/controller/cadence/pcie-cadence.h
> >> @@ -223,6 +223,11 @@ enum cdns_pcie_msg_routing {
> >> MSG_ROUTING_GATHER,
> >> };
> >>
> >> +struct cdns_pcie_ops {
> >> + u32 (*read)(void __iomem *addr, int size);
> >> + void (*write)(void __iomem *addr, int size, u32 value);
> >> +};
> >> +
> >> /**
> >> * struct cdns_pcie - private data for Cadence PCIe controller drivers
> >> * @reg_base: IO mapped register base
> >> @@ -239,7 +244,7 @@ struct cdns_pcie {
> >> int phy_count;
> >> struct phy **phy;
> >> struct device_link **link;
> >> - const struct cdns_pcie_common_ops *ops;
> >> + const struct cdns_pcie_ops *ops;
> >> };
> >>
> >> /**
> >> @@ -299,69 +304,145 @@ struct cdns_pcie_ep {
> >> /* Register access */
> >> static inline void cdns_pcie_writeb(struct cdns_pcie *pcie, u32 reg, u8 value)
> >> {
> >> - writeb(value, pcie->reg_base + reg);
> >> + void __iomem *addr = pcie->reg_base + reg;
> >> +
> >> + if (pcie->ops && pcie->ops->write) {
> >> + pcie->ops->write(addr, 0x1, value);
> >> + return;
> >> + }
> >> +
> >> + writeb(value, addr);
> >> }
> >>
> >> static inline void cdns_pcie_writew(struct cdns_pcie *pcie, u32 reg, u16 value)
> >> {
> >> - writew(value, pcie->reg_base + reg);
> >> + void __iomem *addr = pcie->reg_base + reg;
> >> +
> >> + if (pcie->ops && pcie->ops->write) {
> >> + pcie->ops->write(addr, 0x2, value);
> >> + return;
> >> + }
> >> +
> >> + writew(value, addr);
> >> }
> >
> > cdns_pcie_writeb and cdns_pcie_writew are used, so remove them.
> >
> >>
> >> static inline void cdns_pcie_writel(struct cdns_pcie *pcie, u32 reg, u32 value)
> >> {
> >> - writel(value, pcie->reg_base + reg);
> >> + void __iomem *addr = pcie->reg_base + reg;
> >> +
> >> + if (pcie->ops && pcie->ops->write) {
> >> + pcie->ops->write(addr, 0x4, value);
> >> + return;
> >> + }
> >> +
> >> + writel(value, addr);
> >
> > writel isn't broken for you, so you don't need this either.
> >
> >> }
> >>
> >> static inline u32 cdns_pcie_readl(struct cdns_pcie *pcie, u32 reg)
> >> {
> >> - return readl(pcie->reg_base + reg);
> >> + void __iomem *addr = pcie->reg_base + reg;
> >> +
> >> + if (pcie->ops && pcie->ops->read)
> >> + return pcie->ops->read(addr, 0x4);
> >> +
> >> + return readl(addr);
> >
> > And neither is readl.
>
> Sure, I'll remove all the unused functions and avoid using ops for readl and
> writel.
> >
> >> }
> >>
> >> /* Root Port register access */
> >> static inline void cdns_pcie_rp_writeb(struct cdns_pcie *pcie,
> >> u32 reg, u8 value)
> >> {
> >> - writeb(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg);
> >> + void __iomem *addr = pcie->reg_base + CDNS_PCIE_RP_BASE + reg;
> >> +
> >> + if (pcie->ops && pcie->ops->write) {
> >> + pcie->ops->write(addr, 0x1, value);
> >> + return;
> >> + }
> >> +
> >> + writeb(value, addr);
> >> }
> >>
> >> static inline void cdns_pcie_rp_writew(struct cdns_pcie *pcie,
> >> u32 reg, u16 value)
> >> {
> >> - writew(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg);
> >> + void __iomem *addr = pcie->reg_base + CDNS_PCIE_RP_BASE + reg;
> >> +
> >> + if (pcie->ops && pcie->ops->write) {
> >> + pcie->ops->write(addr, 0x2, value);
> >> + return;
> >> + }
> >> +
> >> + writew(value, addr);
> >
> > You removed 2 out of 3 calls to this. I think I'd just make the root
> > port writes always be 32-bit. It is all just one time init stuff
> > anyways.
> >
> > Either rework the calls to assemble the data into 32-bits or keep these
> > functions and do the RMW here.
>
> The problem with assembling data into 32-bits is we have to read/write with
> different offsets. We'll give PCI_VENDOR_ID offset for modifying deviceID,
> PCI_INTERRUPT_LINE for modifying INTERRUPT_PIN which might get non-intuitive.
> Similarly in endpoint we read and write to MSI_FLAGS (which is at offset 2) we
> have to directly use MSI capability offset.
>
> And doing RMW in the accessors would mean the same RMW op is repeated. So if we
> just have cdns_pcie_rp_writeb() and cdns_pcie_rp_writew(), the same code will
> be repeated here twice.
Why repeated? Just copy what the config accessors do:
static inline void cdns_pcie_write_sz(u32 val, void __iomem *addr, int size)
{
u32 tmp, mask, where = (unsigned long)addr & 0x3;
addr -= where;
mask = ~(((1 << (size * 8)) - 1) << (where * 8));
tmp = readl(addr) & mask;
tmp |= val << (where * 8);
writel(tmp, addr);
}
/* Root Port register access */
static inline void cdns_pcie_rp_writeb(struct cdns_pcie *pcie,
u32 reg, u8 value)
{
cdns_pcie_write_sz(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg, 1);
}
static inline void cdns_pcie_rp_writew(struct cdns_pcie *pcie,
u32 reg, u16 value)
{
cdns_pcie_write_sz(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg, 2);
}
>
> IMHO using ops is a lot cleaner for these cases. IMHO except for removing
> unused functions and not changing readl/writel, others should use ops.
Trying to read the DW PCI driver I don't agree...
Again, unless doing RMW is fundamentally broken (like it is for config
space at runtime), then you only want to do it on broken h/w and ops
would be appropriate. It is all mostly one time setup, so doing a few
extra reads isn't a big deal. If you really care about speed on that,
probably should use the _relaxed accessor variants.
I'm being hopeful the Cadence IP doesn't become the mess that DW is.
Rob
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: spi: Add Baikal-T1 System Boot SPI Controller binding
From: Rob Herring @ 2020-05-21 21:35 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Mark Brown, Ramil Zaripov, Alexey Malahov,
Thomas Bogendoerfer, Paul Burton, Ralf Baechle, John Garry,
Chuanhong Guo, Tomer Maimon, Lee Jones, Miquel Raynal,
Arnd Bergmann, open list:MIPS, linux-spi, devicetree,
linux-kernel@vger.kernel.org
In-Reply-To: <20200521151158.f3izg2svbn5dh6hy@mobilestation>
On Thu, May 21, 2020 at 9:12 AM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> On Thu, May 21, 2020 at 08:57:10AM -0600, Rob Herring wrote:
> > On Mon, May 18, 2020 at 3:27 PM Serge Semin
> > <Sergey.Semin@baikalelectronics.ru> wrote:
> > >
> > > On Mon, May 18, 2020 at 09:26:59AM -0600, Rob Herring wrote:
> > > > On Fri, May 08, 2020 at 12:36:20PM +0300, Serge Semin wrote:
> > > > > Baikal-T1 Boot SPI is a part of the SoC System Controller and is
> > > > > responsible for the system bootup from an external SPI flash. It's a DW
> > > > > APB SSI-based SPI-controller with no interrupts, no DMA, with just one
> > > > > native chip-select available and a single reference clock. Since Baikal-T1
> > > > > SoC is normally booted up from an external SPI flash this SPI controller
> > > > > in most of the cases is supposed to be connected to a single SPI-nor
> > > > > flash. Additionally in order to provide a transparent from CPU point of
> > > > > view initial code execution procedure the system designers created an IP
> > > > > block which physically maps the SPI flash found at CS0 to a memory region.
> > > > >
> > > > > Co-developed-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> > > > > Signed-off-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > > > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > > > > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > > > > Cc: Paul Burton <paulburton@kernel.org>
> > > > > Cc: Ralf Baechle <ralf@linux-mips.org>
> > > > > Cc: John Garry <john.garry@huawei.com>
> > > > > Cc: Chuanhong Guo <gch981213@gmail.com>
> > > > > Cc: Tomer Maimon <tmaimon77@gmail.com>
> > > > > Cc: Lee Jones <lee.jones@linaro.org>
> > > > > Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > > Cc: Arnd Bergmann <arnd@arndb.de>
> > > > > Cc: linux-mips@vger.kernel.org
> > > > > Cc: linux-spi@vger.kernel.org
> > > > > ---
> > > > > .../bindings/spi/baikal,bt1-sys-ssi.yaml | 100 ++++++++++++++++++
> > > > > 1 file changed, 100 insertions(+)
> > > > > create mode 100644 Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..d9d3257d78f4
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > > > @@ -0,0 +1,100 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +# Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/spi/baikal,bt1-sys-ssi.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Baikal-T1 System Boot SSI Controller
> > > > > +
> > > > > +description: |
> > > > > + Baikal-T1 System Controller includes a Boot SPI Controller, which is
> > > > > + responsible for loading chip bootup code from an external SPI flash. In order
> > > > > + to do this transparently from CPU point of view there is a dedicated IP block
> > > > > + mapping the 16MB flash to a dedicated MMIO range. The controller is based on
> > > > > + the DW APB SSI IP-core but equipped with very limited resources: no IRQ,
> > > > > + no DMA, a single native CS being necessarily connected to a 16MB SPI flash
> > > > > + (otherwise the system won't bootup from the flash), internal Tx/Rx FIFO of
> > > > > + just 8 bytes depth. Access to DW APB SSI controller registers is mutually
> > > > > + exclusive from normal MMIO interface and from physically mapped SPI Flash
> > > > > + memory. So either one or another way of using the controller functionality
> > > > > + can be enabled at a time.
> > > > > +
> > > > > +maintainers:
> > > > > + - Serge Semin <fancer.lancer@gmail.com>
> > > > > +
> > > > > +allOf:
> > > > > + - $ref: spi-controller.yaml#
> > > > > +
> > > > > +properties:
> > > > > + compatible:
> > > > > + const: baikal,bt1-sys-ssi
> > > > > +
> > > > > + reg:
> > > > > + items:
> > > > > + - description: Baikal-T1 Boot Controller configuration registers
> > > > > + - description: Physically mapped SPI flash ROM found at CS0
> > > > > +
> > > > > + reg-names:
> > > > > + items:
> > > > > + - const: config
> > > > > + - const: map
> > > > > +
> > > > > + clocks:
> > > > > + description: SPI Controller reference clock source
> > > >
> > > > Can drop this.
> > >
> > > Ok.
> > >
> > > >
> > > > > + maxItems: 1
> > > > > +
> > > > > + clock-names:
> > > > > + items:
> > > > > + - const: ssi_clk
> > > > > +
> > > > > + num-cs:
> > > > > + const: 1
> > > > > +
> > > > > +patternProperties:
> > > > > + "^.*@[0-9a-f]+":
> > > > > + type: object
> > > > > + properties:
> > > > > + reg:
> > > > > + minimum: 0
> > > > > + maximum: 0
> > > > > +
> > > > > + spi-rx-bus-width:
> > > > > + const: 1
> > > > > +
> > > > > + spi-tx-bus-width:
> > > > > + const: 1
> > > >
> > > > What's the point of these 2 properties if they aren't required?
> > >
> > > Yes, they are optional, but this is a constraint on the bus-width parameters.
> > > DW APB SSI provides a single laned Tx and Rx.
> >
> > Are you just trying to keep someone from saying 'spi-tx-bus-width: 2'
> > for example?
>
> Right.
>
> >
> > You could also say 'spi-tx-bus-width: false' here to disallow the
> > property. I guess the above is fine.
>
> Ok. If it's fine I'll leave them as is then. Right?
Yes.
> > > > +unevaluatedProperties: false
> > > > +
> > > > +required:
> > > > + - compatible
> > > > + - reg
> > > > + - reg-names
> > >
> > > > + - "#address-cells"
> > > > + - "#size-cells"
> > >
> > > These 2 are required by spi-controller.yaml, so you can drop here.
> >
> > Yes, "#address-cells" is required, but "#size-cells" isn't. Is this supposed to
> > be like that?
>
> As far as I can see in spi-controller.yaml, "#address-cells" is required, but
> "#size-cells" isn't. Is this intentional?
Humm, I guess we didn't have either one because you can have no child
node defined, but then added #address-cells for the spi-slave case.
In any case, it's all pretty well covered already because the core
schema checks that both are present and dtc has some checking too.
Rob
^ permalink raw reply
* Re: [RFC v1 2/3] drivers: nvmem: Add driver for QTI qfprom-efuse support
From: Doug Anderson @ 2020-05-21 21:28 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Ravi Kumar Bokka (Temp), Rob Herring, LKML,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Rajendra Nayak, Sai Prakash Ranjan, dhavalp, mturney, sparate,
c_rbokka, mkurumel
In-Reply-To: <9ecb5790-47fe-583b-6fc3-8f4f3ce7860e@linaro.org>
Hi,
On Thu, May 21, 2020 at 8:56 AM Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
>
> On 21/05/2020 16:10, Doug Anderson wrote:
> >> On 20/05/2020 23:48, Doug Anderson wrote:
> >>>> Is this only applicable for corrected address space?
> >>> I guess I was proposing a two dts-node / two drive approach here.
> >>>
> >>> dts node #1:just covers the memory range for accessing the FEC-corrected data
> >>> driver #1: read-only and reads the FEC-corrected data
> >>>
> >>> dts node #2: covers the memory range that's_not_ the FEC-corrected
> >>> memory range.
> >>> driver #2: read-write. reading reads uncorrected data
> >>>
> >>> Does that seem sane?
> >> I see your point but it does not make sense to have two node for same thing.
> > OK, so that sounds as if we want to go with the proposal where we
> > "deprecate the old driver and/or bindings and say that there really
> > should just be one node and one driver".
> >
> > Would this be acceptable to you?
> >
> > 1. Officially mark the old bindings as deprecated.
>
> Possibly Yes for some reasons below!
>
> >
> > 2. Leave the old driver there to support the old deprecated bindings,
> > at least until everyone can be transferred over. There seem to be
> > quite a few existing users of "qcom,qfprom" and we're supposed to make
> > an attempt at keeping the old device trees working, at least for a
> > little while. Once everyone is transferred over we could decide to
> > delete the old driver.
> we could consider "qcom,qfrom" to be only passing corrected address
> space. Till we transition users to new bindings!
>
> >
> Yes.
>
> > 3. We will have a totally new driver here.
> No, we should still be using the same driver. But the exiting driver
> seems to incorrect and is need of fixing.
>
> Having a look at the memory map for old SoCs like msm8996 and msm8916
> shows that memory map that was passed to qfprom driver is corrected
> address space. Writes will not obviously work!
>
> This should also be true with sdm845 or sc7180
>
> That needs to be fixed first!
OK, so to summarize:
1. We will have one driver: "drivers/nvmem/qfprom.c"
2. If the driver detects that its reg is pointing to the corrected
address space then it should operate in read-only mode. Maybe it can
do this based on the compatible string being just "qcom,qfprom" or
maybe it can do this based on the size of the "reg".
3. If that driver sees a newer compatible string (one that includes
the SoC name in it) it will assume that its "reg" points to the start
of qfprom space.
4. We should post patches to transition all old dts files away from
the deprecated bindings.
> > 4. A given device tree will_not_ be allowed to have both
> > "qcom,qfprom" specified and "qcom,SOC-qfprom" specified. ...and by
> > "qcom,SOC-qfprom" I mean that SOC should be replaced by the SoC name,
> > so "qcom,sc7180-qfprom" or "qcom,sdm845-qfprom". So once you switch
> > to the new node it replaces the old node.
>
> Secondly, this IP is clearly an integral part of Secure Control Block,
> which clearly has versioning information.
>
> Versioning information should be part of compatible string in msm8996 it
> should be "qcom,qfprom-5.1.0"
> for msm8916 it should be "qcom,qfprom-4.0.0" this translates to
> "qcom,qfprom-<MAJOR-NUMBER>-<MINOR-NUMBER>-<STEP>"
I don't know much about this versioning info, but I'm curious: can we
read it from the chip? If so then it actually _doesn't_ need to be in
the compatible string, I think. Device tree shouldn't include things
that can be probed. So if this can be probed then maybe we could have
the compatible as:
compatible = "qcom,msm8996-qfprom", "qcom,qfprom"
...where the SoC is there just in case we need it but we'd expect to
just match on "qcom,qfprom" and then probe.
If this can't be probed then having the version info is nice, so then
I guess you'd have the compatible string:
compatible = "qcom,msm8996-qfprom", "qcom,qfprom-5.1.0"
...where (again) you'd have the SoC specific string there just in case
but you'd expect that you could just use the generic string.
Does that sound right?
> Thirdly we should be able to have common read for all these as they tend
> to just read from corrected address space.
>
> Offsets to corrected address space seems to always constant across SoCs too.
>
> platform specific device tree nodes should also be able to specify
> "read-only" property to not allow writes on to this raw area.
Yeah, I was thinking we probably wanted a read-only property. That
sounds sane to me.
-Doug
^ permalink raw reply
* Re: [PATCH v12 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
From: Wolfram Sang @ 2020-05-21 21:21 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Tali Perry, ofery, brendanhiggins, avifishman70, tmaimon77,
kfting, venture, yuenn, benjaminfair, robh+dt, linux-arm-kernel,
linux-i2c, openbmc, devicetree, linux-kernel
In-Reply-To: <20200521143100.GA16812@ninjato>
[-- Attachment #1: Type: text/plain, Size: 192 bytes --]
> From a glimpse, this looks good to go. I will have a close look later
> today.
Phew, this driver is huge. I won't finish my review today, but I am
working on it and am maybe 2/3 through.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v5 6/8] clocksource: dw_apb_timer_of: Fix missing clockevent timers
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano, Rob Herring,
Heiko Stuebner, Dinh Nguyen, Jamie Iles, Arnd Bergmann
Cc: Serge Semin, Serge Semin, Alexey Malahov, Paul Burton,
Ralf Baechle, Alessandro Zummo, Alexandre Belloni, Rob Herring,
linux-mips, linux-rtc, devicetree, Enrico Weigelt,
Greg Kroah-Hartman, Allison Randal, linux-kernel
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
Commit 100214889973 ("clocksource: dw_apb_timer_of: use
clocksource_of_init") replaced a publicly available driver
initialization method with one called by the timer_probe() method
available after CLKSRC_OF. In current implementation it traverses
all the timers available in the system and calls their initialization
methods if corresponding devices were either in dtb or in acpi. But
if before the commit any number of available timers would be installed
as clockevent and clocksource devices, after that there would be at most
two. The rest are just ignored since default case branch doesn't do
anything. I don't see a reason of such behaviour, neither the commit
message explains it. Moreover this might be wrong if on some platforms
these timers might be used for different purpose, as virtually CPU-local
clockevent timers and as an independent broadcast timer. So in order
to keep the compatibility with the platforms where the order of the
timers detection has some meaning, lets add the secondly discovered
timer to be of clocksource/sched_clock type, while the very first and
the others would provide the clockevents service.
Fixes: 100214889973 ("clocksource: dw_apb_timer_of: use clocksource_of_init")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
drivers/clocksource/dw_apb_timer_of.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
index 2db490f35c20..ab3ddebe8344 100644
--- a/drivers/clocksource/dw_apb_timer_of.c
+++ b/drivers/clocksource/dw_apb_timer_of.c
@@ -147,10 +147,6 @@ static int num_called;
static int __init dw_apb_timer_init(struct device_node *timer)
{
switch (num_called) {
- case 0:
- pr_debug("%s: found clockevent timer\n", __func__);
- add_clockevent(timer);
- break;
case 1:
pr_debug("%s: found clocksource timer\n", __func__);
add_clocksource(timer);
@@ -161,6 +157,8 @@ static int __init dw_apb_timer_init(struct device_node *timer)
#endif
break;
default:
+ pr_debug("%s: found clockevent timer\n", __func__);
+ add_clockevent(timer);
break;
}
--
2.25.1
^ permalink raw reply related
* [PATCH v5 7/8] clocksource: mips-gic-timer: Register as sched_clock
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
Cc: Serge Semin, Serge Semin, Paul Burton, Alexey Malahov,
Ralf Baechle, Alessandro Zummo, Alexandre Belloni, Arnd Bergmann,
Rob Herring, linux-mips, linux-rtc, devicetree, Vincenzo Frascino,
linux-kernel
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
From: Paul Burton <paulburton@kernel.org>
The MIPS GIC timer is well suited for use as sched_clock, so register it
as such.
Whilst the existing gic_read_count() function matches the prototype
needed by sched_clock_register() already, we split it into 2 functions
in order to remove the need to evaluate the mips_cm_is64 condition
within each call since sched_clock should be as fast as possible.
Note the sched clock framework needs the clock source being stable in
order to rely on it. So we register the MIPS GIC timer as schedule clocks
only if it's, if either the system doesn't have CPU-frequency enabled or
the CPU frequency is changed by means of the CPC core clock divider
available on the platforms with CM3 or newer.
Signed-off-by: Paul Burton <paulburton@kernel.org>
Co-developed-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
[Sergey.Semin@baikalelectronics.ru: Register sched-clock if CM3 or !CPU-freq]
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v3:
- Register sched clocks only if MIPS GIC belongs to CM3 or if CPU-freq
isn't supported.
---
drivers/clocksource/mips-gic-timer.c | 31 ++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index 8b5f8ae723cb..ef12c12c2432 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -16,6 +16,7 @@
#include <linux/notifier.h>
#include <linux/of_irq.h>
#include <linux/percpu.h>
+#include <linux/sched_clock.h>
#include <linux/smp.h>
#include <linux/time.h>
#include <asm/mips-cps.h>
@@ -24,13 +25,10 @@ static DEFINE_PER_CPU(struct clock_event_device, gic_clockevent_device);
static int gic_timer_irq;
static unsigned int gic_frequency;
-static u64 notrace gic_read_count(void)
+static u64 notrace gic_read_count_2x32(void)
{
unsigned int hi, hi2, lo;
- if (mips_cm_is64)
- return read_gic_counter();
-
do {
hi = read_gic_counter_32h();
lo = read_gic_counter_32l();
@@ -40,6 +38,19 @@ static u64 notrace gic_read_count(void)
return (((u64) hi) << 32) + lo;
}
+static u64 notrace gic_read_count_64(void)
+{
+ return read_gic_counter();
+}
+
+static u64 notrace gic_read_count(void)
+{
+ if (mips_cm_is64)
+ return gic_read_count_64();
+
+ return gic_read_count_2x32();
+}
+
static int gic_next_event(unsigned long delta, struct clock_event_device *evt)
{
int cpu = cpumask_first(evt->cpumask);
@@ -228,6 +239,18 @@ static int __init gic_clocksource_of_init(struct device_node *node)
/* And finally start the counter */
clear_gic_config(GIC_CONFIG_COUNTSTOP);
+ /*
+ * It's safe to use the MIPS GIC timer as a sched clock source only if
+ * its ticks are stable, which is true on either the platforms with
+ * stable CPU frequency or on the platforms with CM3 and CPU frequency
+ * change performed by the CPC core clocks divider.
+ */
+ if (mips_cm_revision() >= CM_REV_CM3 || !IS_ENABLED(CONFIG_CPU_FREQ)) {
+ sched_clock_register(mips_cm_is64 ?
+ gic_read_count_64 : gic_read_count_2x32,
+ 64, gic_frequency);
+ }
+
return 0;
}
TIMER_OF_DECLARE(mips_gic_timer, "mti,gic-timer",
--
2.25.1
^ permalink raw reply related
* [PATCH v5 8/8] clocksource: mips-gic-timer: Mark GIC timer as unstable if ref clock changes
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
Cc: Serge Semin, Serge Semin, Alexey Malahov, Paul Burton,
Ralf Baechle, Alessandro Zummo, Alexandre Belloni, Arnd Bergmann,
Rob Herring, linux-mips, linux-rtc, devicetree, Paul Cercueil,
Claudiu Beznea, Krzysztof Kozlowski, Maarten ter Huurne,
Bartosz Golaszewski, Randy Dunlap, Vincenzo Frascino,
linux-kernel
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
Currently clocksource framework doesn't support the clocks with variable
frequency. Since MIPS GIC timer ticks rate might be unstable on some
platforms, we must make sure that it justifies the clocksource
requirements. MIPS GIC timer is incremented with the CPU cluster reference
clocks rate. So in case if CPU frequency changes, the MIPS GIC tick rate
changes synchronously. Due to this the clocksource subsystem can't rely on
the timer to measure system clocks anymore. This commit marks the MIPS GIC
based clocksource as unstable if reference clock (normally it's a CPU
reference clocks) rate changes. The clocksource will execute a watchdog
thread, which lowers the MIPS GIC timer rating to zero and fallbacks to a
new stable one.
Note we don't need to set the CLOCK_SOURCE_MUST_VERIFY flag to the MIPS
GIC clocksource since normally the timer is stable. The only reason why
it gets unstable is due to the ref clock rate change, which event we
detect here in the driver by means of the clocks event notifier.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v4:
- Mark clocksource as unstable instead of lowering its rating.
Changelog v5:
- Fix mistakenly added "git_" prefix.
---
drivers/clocksource/Kconfig | 1 +
drivers/clocksource/mips-gic-timer.c | 19 ++++++++++++++++++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index f2142e6bbea3..37a745f3ca91 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -572,6 +572,7 @@ config CLKSRC_VERSATILE
config CLKSRC_MIPS_GIC
bool
depends on MIPS_GIC
+ select CLOCKSOURCE_WATCHDOG
select TIMER_OF
config CLKSRC_TANGO_XTAL
diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index ef12c12c2432..be4175f415ba 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -24,6 +24,9 @@
static DEFINE_PER_CPU(struct clock_event_device, gic_clockevent_device);
static int gic_timer_irq;
static unsigned int gic_frequency;
+static bool __read_mostly gic_clock_unstable;
+
+static void gic_clocksource_unstable(char *reason);
static u64 notrace gic_read_count_2x32(void)
{
@@ -125,8 +128,10 @@ static int gic_clk_notifier(struct notifier_block *nb, unsigned long action,
{
struct clk_notifier_data *cnd = data;
- if (action == POST_RATE_CHANGE)
+ if (action == POST_RATE_CHANGE) {
+ gic_clocksource_unstable("ref clock rate change");
on_each_cpu(gic_update_frequency, (void *)cnd->new_rate, 1);
+ }
return NOTIFY_OK;
}
@@ -172,6 +177,18 @@ static struct clocksource gic_clocksource = {
.vdso_clock_mode = VDSO_CLOCKMODE_GIC,
};
+static void gic_clocksource_unstable(char *reason)
+{
+ if (gic_clock_unstable)
+ return;
+
+ gic_clock_unstable = true;
+
+ pr_info("GIC timer is unstable due to %s\n", reason);
+
+ clocksource_mark_unstable(&gic_clocksource);
+}
+
static int __init __gic_clocksource_init(void)
{
unsigned int count_width;
--
2.25.1
^ permalink raw reply related
* [PATCH v5 5/8] clocksource: dw_apb_timer: Affiliate of-based timer with any CPU
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
Cc: Serge Semin, Serge Semin, Alexey Malahov, Paul Burton,
Ralf Baechle, Alessandro Zummo, Alexandre Belloni, Arnd Bergmann,
Rob Herring, linux-mips, linux-rtc, devicetree, Allison Randal,
Greg Kroah-Hartman, Alexios Zavras, linux-kernel
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
Currently any DW APB Timer device detected in OF is bound to CPU #0.
Doing so is redundant since DW APB Timer isn't CPU-local timer, but as
having APB interface is normally accessible from any CPU in the system. By
artificially affiliating the DW timer to the very first CPU we may and in
our case will make the clockevent subsystem to decline the more performant
real CPU-local timers selection in favor of in fact non-local and
accessible over a slow bus - DW APB Timers.
Let's not affiliate the of-detected DW APB Timers to any CPU. By doing so
the clockevent framework would prefer to select the real CPU-local timer
instead of DW APB one. Otherwise if there is no other than DW APB device
for clockevents tracking then it will be selected.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v5:
- This is a new patch created after reconsidering the solution of the DW
APB Timer CPU-affiliation problem.
---
drivers/clocksource/dw_apb_timer_of.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
index 8c28b127759f..2db490f35c20 100644
--- a/drivers/clocksource/dw_apb_timer_of.c
+++ b/drivers/clocksource/dw_apb_timer_of.c
@@ -73,7 +73,7 @@ static void __init add_clockevent(struct device_node *event_timer)
timer_get_base_and_rate(event_timer, &iobase, &rate);
- ced = dw_apb_clockevent_init(0, event_timer->name, 300, iobase, irq,
+ ced = dw_apb_clockevent_init(-1, event_timer->name, 300, iobase, irq,
rate);
if (!ced)
panic("Unable to initialise clockevent device");
--
2.25.1
^ permalink raw reply related
* [PATCH v5 4/8] clocksource: dw_apb_timer: Make CPU-affiliation being optional
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
Cc: Serge Semin, Serge Semin, Alexey Malahov, Paul Burton,
Ralf Baechle, Alessandro Zummo, Alexandre Belloni, Arnd Bergmann,
Rob Herring, linux-mips, linux-rtc, devicetree, afzal mohammed,
Allison Randal, Greg Kroah-Hartman, linux-kernel
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
Currently the DW APB Timer driver binds each clockevent timers to a
particular CPU. This isn't good for multiple reasons. First of all seeing
the device is placed on APB bus (which makes it accessible from any CPU
core), accessible over MMIO and having the DYNIRQ flag set we can be sure
that manually binding the timer to any CPU just isn't correct. By doing
so we just set an extra limitation on device usage. This also doesn't
reflect the device actual capability, since by setting the IRQ affinity
we can make it virtually local to any CPU. Secondly imagine if you had a
real CPU-local timer with the same rating and the same CPU-affinity.
In this case if DW APB timer was registered first, then due to the
clockevent framework tick-timer selection procedure we'll end up with the
real CPU-local timer being left unselected for clock-events tracking. But
on most of the platforms (MIPS/ARM/etc) such timers are normally embedded
into the CPU core and are accessible with much better performance then
devices placed on APB. For instance in MIPS architectures there is
r4k-timer, which is CPU-local, assigned with the same rating, and normally
its clockevent device is registered after the platform-specific one.
So in order to fix all of these issues let's make the DW APB Timer CPU
affinity being optional and deactivated by passing a negative CPU id,
which will effectively set the DW APB clockevent timer cpumask to
'cpu_possible_mask'.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v5:
- Don't discard CPU affiliation functionality, but allow it being optional.
If CPU id passed to the clockevent registration method is negative, then
don't assign the timer events to any particular CPU.
- Discard the OF-based DW APB Timer CPU-affiliation setting change. It'll
be moved to a dedicated patch.
---
drivers/clocksource/dw_apb_timer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/clocksource/dw_apb_timer.c b/drivers/clocksource/dw_apb_timer.c
index b207a77b0831..f5f24a95ee82 100644
--- a/drivers/clocksource/dw_apb_timer.c
+++ b/drivers/clocksource/dw_apb_timer.c
@@ -222,7 +222,8 @@ static int apbt_next_event(unsigned long delta,
/**
* dw_apb_clockevent_init() - use an APB timer as a clock_event_device
*
- * @cpu: The CPU the events will be targeted at.
+ * @cpu: The CPU the events will be targeted at or -1 if CPU affiliation
+ * isn't required.
* @name: The name used for the timer and the IRQ for it.
* @rating: The rating to give the timer.
* @base: I/O base for the timer registers.
@@ -257,7 +258,7 @@ dw_apb_clockevent_init(int cpu, const char *name, unsigned rating,
dw_ced->ced.max_delta_ticks = 0x7fffffff;
dw_ced->ced.min_delta_ns = clockevent_delta2ns(5000, &dw_ced->ced);
dw_ced->ced.min_delta_ticks = 5000;
- dw_ced->ced.cpumask = cpumask_of(cpu);
+ dw_ced->ced.cpumask = cpu < 0 ? cpu_possible_mask : cpumask_of(cpu);
dw_ced->ced.features = CLOCK_EVT_FEAT_PERIODIC |
CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ;
dw_ced->ced.set_state_shutdown = apbt_shutdown;
--
2.25.1
^ permalink raw reply related
* [PATCH v5 3/8] dt-bindings: interrupt-controller: Convert mti,gic to DT schema
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano,
Jason Cooper, Marc Zyngier, Rob Herring
Cc: Serge Semin, Serge Semin, Rob Herring, Alexey Malahov,
Paul Burton, Ralf Baechle, Alessandro Zummo, Alexandre Belloni,
Arnd Bergmann, linux-mips, linux-rtc, linux-kernel, devicetree
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
Modern device tree bindings are supposed to be created as YAML-files
in accordance with DT schema. This commit replaces MIPS GIC legacy bare
text binding with YAML file. As before the binding file states that the
corresponding dts node is supposed to be compatible with MIPS Global
Interrupt Controller indicated by the "mti,gic" compatible string and
to provide a mandatory interrupt-controller and '#interrupt-cells'
properties. There might be optional registers memory range,
"mti,reserved-cpu-vectors" and "mti,reserved-ipi-vectors" properties
specified.
MIPS GIC also includes a free-running global timer, per-CPU count/compare
timers, and a watchdog. Since currently the GIC Timer is only supported the
DT schema expects an IRQ and clock-phandler charged timer sub-node with
"mti,mips-gic-timer" compatible string.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
---
I don't really know who is the corresponding driver maintainer, so I
added Paul to the maintainers property since he used to be looking for the
MIPS arch and Thomas looking after it now. Any idea what email should be
specified there instead?
Changelog v3:
- Since timer sub-node has no unit-address, the node shouldn't be named
with one. So alter the MIPS GIC bindings to have a pure "timer"
sub-node.
- Discard allOf: [ $ref: /schemas/interrupt-controller.yaml# ].
- Since it's a conversion patch use GPL-2.0-only SPDX header.
---
.../interrupt-controller/mips-gic.txt | 67 --------
.../interrupt-controller/mti,gic.yaml | 148 ++++++++++++++++++
2 files changed, 148 insertions(+), 67 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml
diff --git a/Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt b/Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt
deleted file mode 100644
index 173595305e26..000000000000
--- a/Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt
+++ /dev/null
@@ -1,67 +0,0 @@
-MIPS Global Interrupt Controller (GIC)
-
-The MIPS GIC routes external interrupts to individual VPEs and IRQ pins.
-It also supports local (per-processor) interrupts and software-generated
-interrupts which can be used as IPIs. The GIC also includes a free-running
-global timer, per-CPU count/compare timers, and a watchdog.
-
-Required properties:
-- compatible : Should be "mti,gic".
-- interrupt-controller : Identifies the node as an interrupt controller
-- #interrupt-cells : Specifies the number of cells needed to encode an
- interrupt specifier. Should be 3.
- - The first cell is the type of interrupt, local or shared.
- See <include/dt-bindings/interrupt-controller/mips-gic.h>.
- - The second cell is the GIC interrupt number.
- - The third cell encodes the interrupt flags.
- See <include/dt-bindings/interrupt-controller/irq.h> for a list of valid
- flags.
-
-Optional properties:
-- reg : Base address and length of the GIC registers. If not present,
- the base address reported by the hardware GCR_GIC_BASE will be used.
-- mti,reserved-cpu-vectors : Specifies the list of CPU interrupt vectors
- to which the GIC may not route interrupts. Valid values are 2 - 7.
- This property is ignored if the CPU is started in EIC mode.
-- mti,reserved-ipi-vectors : Specifies the range of GIC interrupts that are
- reserved for IPIs.
- It accepts 2 values, the 1st is the starting interrupt and the 2nd is the size
- of the reserved range.
- If not specified, the driver will allocate the last 2 * number of VPEs in the
- system.
-
-Required properties for timer sub-node:
-- compatible : Should be "mti,gic-timer".
-- interrupts : Interrupt for the GIC local timer.
-
-Optional properties for timer sub-node:
-- clocks : GIC timer operating clock.
-- clock-frequency : Clock frequency at which the GIC timers operate.
-
-Note that one of clocks or clock-frequency must be specified.
-
-Example:
-
- gic: interrupt-controller@1bdc0000 {
- compatible = "mti,gic";
- reg = <0x1bdc0000 0x20000>;
-
- interrupt-controller;
- #interrupt-cells = <3>;
-
- mti,reserved-cpu-vectors = <7>;
- mti,reserved-ipi-vectors = <40 8>;
-
- timer {
- compatible = "mti,gic-timer";
- interrupts = <GIC_LOCAL 1 IRQ_TYPE_NONE>;
- clock-frequency = <50000000>;
- };
- };
-
- uart@18101400 {
- ...
- interrupt-parent = <&gic>;
- interrupts = <GIC_SHARED 24 IRQ_TYPE_LEVEL_HIGH>;
- ...
- };
diff --git a/Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml b/Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml
new file mode 100644
index 000000000000..9f0eb3addac4
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml
@@ -0,0 +1,148 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/mti,gic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MIPS Global Interrupt Controller
+
+maintainers:
+ - Paul Burton <paulburton@kernel.org>
+ - Thomas Bogendoerfer <tsbogend@alpha.franken.de>
+
+description: |
+ The MIPS GIC routes external interrupts to individual VPEs and IRQ pins.
+ It also supports local (per-processor) interrupts and software-generated
+ interrupts which can be used as IPIs. The GIC also includes a free-running
+ global timer, per-CPU count/compare timers, and a watchdog.
+
+properties:
+ compatible:
+ const: mti,gic
+
+ "#interrupt-cells":
+ const: 3
+ description: |
+ The 1st cell is the type of interrupt: local or shared defined in the
+ file 'dt-bindings/interrupt-controller/mips-gic.h'. The 2nd cell is the
+ GIC interrupt number. The 3d cell encodes the interrupt flags setting up
+ the IRQ trigger modes, which are defined in the file
+ 'dt-bindings/interrupt-controller/irq.h'.
+
+ reg:
+ description: |
+ Base address and length of the GIC registers space. If not present,
+ the base address reported by the hardware GCR_GIC_BASE will be used.
+ maxItems: 1
+
+ interrupt-controller: true
+
+ mti,reserved-cpu-vectors:
+ description: |
+ Specifies the list of CPU interrupt vectors to which the GIC may not
+ route interrupts. This property is ignored if the CPU is started in EIC
+ mode.
+ allOf:
+ - $ref: /schemas/types.yaml#definitions/uint32-array
+ - minItems: 1
+ maxItems: 6
+ uniqueItems: true
+ items:
+ minimum: 2
+ maximum: 7
+
+ mti,reserved-ipi-vectors:
+ description: |
+ Specifies the range of GIC interrupts that are reserved for IPIs.
+ It accepts two values: the 1st is the starting interrupt and the 2nd is
+ the size of the reserved range. If not specified, the driver will
+ allocate the last (2 * number of VPEs in the system).
+ allOf:
+ - $ref: /schemas/types.yaml#definitions/uint32-array
+ - items:
+ - minimum: 0
+ maximum: 254
+ - minimum: 2
+ maximum: 254
+
+ timer:
+ type: object
+ description: |
+ MIPS GIC includes a free-running global timer, per-CPU count/compare
+ timers, and a watchdog. Currently only the GIC Timer is supported.
+ properties:
+ compatible:
+ const: mti,gic-timer
+
+ interrupts:
+ description: |
+ Interrupt for the GIC local timer, so normally it's suppose to be of
+ <GIC_LOCAL X IRQ_TYPE_NONE> format.
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-frequency: true
+
+ required:
+ - compatible
+ - interrupts
+
+ oneOf:
+ - required:
+ - clocks
+ - required:
+ - clock-frequency
+
+ additionalProperties: false
+
+unevaluatedProperties: false
+
+required:
+ - compatible
+ - "#interrupt-cells"
+ - interrupt-controller
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/mips-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ interrupt-controller@1bdc0000 {
+ compatible = "mti,gic";
+ reg = <0x1bdc0000 0x20000>;
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ mti,reserved-cpu-vectors = <7>;
+ mti,reserved-ipi-vectors = <40 8>;
+
+ timer {
+ compatible = "mti,gic-timer";
+ interrupts = <GIC_LOCAL 1 IRQ_TYPE_NONE>;
+ clock-frequency = <50000000>;
+ };
+ };
+ - |
+ #include <dt-bindings/interrupt-controller/mips-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ interrupt-controller@1bdc0000 {
+ compatible = "mti,gic";
+ reg = <0x1bdc0000 0x20000>;
+ interrupt-controller;
+ #interrupt-cells = <3>;
+
+ timer {
+ compatible = "mti,gic-timer";
+ interrupts = <GIC_LOCAL 1 IRQ_TYPE_NONE>;
+ clocks = <&cpu_pll>;
+ };
+ };
+ - |
+ interrupt-controller {
+ compatible = "mti,gic";
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ };
+...
--
2.25.1
^ permalink raw reply related
* [PATCH v5 2/8] dt-bindings: timer: Move snps,dw-apb-timer DT schema from rtc
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano,
Alessandro Zummo, Alexandre Belloni, Rob Herring
Cc: Serge Semin, Serge Semin, Rob Herring, Alexey Malahov,
Paul Burton, Ralf Baechle, Arnd Bergmann, linux-mips, linux-rtc,
devicetree, linux-kernel
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
This binding file doesn't belong to the rtc seeing it's a pure timer
with no rtc facilities like days/months/years counting and alarms.
So move the YAML-file to the Documentation/devicetree/bindings/timer/
directory.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Acked-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org
---
.../devicetree/bindings/{rtc => timer}/snps,dw-apb-timer.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
rename Documentation/devicetree/bindings/{rtc => timer}/snps,dw-apb-timer.yaml (96%)
diff --git a/Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml b/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
similarity index 96%
rename from Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml
rename to Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
index 002fe1ee709b..5d300efdf0ca 100644
--- a/Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml
+++ b/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
%YAML 1.2
---
-$id: http://devicetree.org/schemas/rtc/snps,dw-apb-timer.yaml#
+$id: http://devicetree.org/schemas/timer/snps,dw-apb-timer.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
title: Synopsys DesignWare APB Timer
--
2.25.1
^ permalink raw reply related
* [PATCH v5 1/8] dt-bindings: rtc: Convert snps,dw-apb-timer to DT schema
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano,
Alessandro Zummo, Alexandre Belloni, Rob Herring
Cc: Serge Semin, Serge Semin, Rob Herring, Alexey Malahov,
Paul Burton, Ralf Baechle, Arnd Bergmann, linux-mips, linux-rtc,
devicetree, linux-kernel
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
Modern device tree bindings are supposed to be created as YAML-files
in accordance with DT schema. This commit replaces Synopsys DW Timer
legacy bare text binding with YAML file. As before the binding file
states that the corresponding dts node is supposed to be compatible
with generic DW APB Timer indicated by the "snps,dw-apb-timer"
compatible string and to provide a mandatory registers memory range,
one timer interrupt, either reference clock source or a fixed clock
rate value. It may also have an optional APB bus reference clock
phandle specified.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org
---
This binding file doesn't belong to the bindings/rtc seeing it's a pure
timer with no rtc facilities like days/months/years counting and alarms.
The binding file will be moved to the
"Documentation/devicetree/bindings/timer/" directory in the next patch.
I also don't know who is the corresponding driver maintainer, so I added
Daniel Lezcano to the maintainers schema. Any idea what email should be
specified there instead?
Changelog v3:
- Since it's a conversion patch use GPL-2.0-only SPDX header.
- Replace "additionalProperties: false" property with
"unevaluatedProperties: false".
---
.../devicetree/bindings/rtc/dw-apb.txt | 32 -------
.../bindings/rtc/snps,dw-apb-timer.yaml | 88 +++++++++++++++++++
2 files changed, 88 insertions(+), 32 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/rtc/dw-apb.txt
create mode 100644 Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml
diff --git a/Documentation/devicetree/bindings/rtc/dw-apb.txt b/Documentation/devicetree/bindings/rtc/dw-apb.txt
deleted file mode 100644
index c703d51abb6c..000000000000
--- a/Documentation/devicetree/bindings/rtc/dw-apb.txt
+++ /dev/null
@@ -1,32 +0,0 @@
-* Designware APB timer
-
-Required properties:
-- compatible: One of:
- "snps,dw-apb-timer"
- "snps,dw-apb-timer-sp" <DEPRECATED>
- "snps,dw-apb-timer-osc" <DEPRECATED>
-- reg: physical base address of the controller and length of memory mapped
- region.
-- interrupts: IRQ line for the timer.
-- either clocks+clock-names or clock-frequency properties
-
-Optional properties:
-- clocks : list of clock specifiers, corresponding to entries in
- the clock-names property;
-- clock-names : should contain "timer" and "pclk" entries, matching entries
- in the clocks property.
-- clock-frequency: The frequency in HZ of the timer.
-- clock-freq: For backwards compatibility with picoxcell
-
-If using the clock specifiers, the pclk clock is optional, as not all
-systems may use one.
-
-
-Example:
- timer@ffe00000 {
- compatible = "snps,dw-apb-timer";
- interrupts = <0 170 4>;
- reg = <0xffe00000 0x1000>;
- clocks = <&timer_clk>, <&timer_pclk>;
- clock-names = "timer", "pclk";
- };
diff --git a/Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml b/Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml
new file mode 100644
index 000000000000..002fe1ee709b
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/snps,dw-apb-timer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare APB Timer
+
+maintainers:
+ - Daniel Lezcano <daniel.lezcano@linaro.org>
+
+properties:
+ compatible:
+ oneOf:
+ - const: snps,dw-apb-timer
+ - enum:
+ - snps,dw-apb-timer-sp
+ - snps,dw-apb-timer-osc
+ deprecated: true
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ minItems: 1
+ items:
+ - description: Timer ticks reference clock source
+ - description: APB interface clock source
+
+ clock-names:
+ minItems: 1
+ items:
+ - const: timer
+ - const: pclk
+
+ clock-frequency: true
+
+ clock-freq:
+ $ref: "/schemas/types.yaml#/definitions/uint32"
+ description: |
+ Has the same meaning as the 'clock-frequency' property - timer clock
+ frequency in HZ, but is defined only for the backwards compatibility
+ with the picoxcell platform.
+
+unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+oneOf:
+ - required:
+ - clocks
+ - clock-names
+ - required:
+ - clock-frequency
+ - required:
+ - clock-freq
+
+examples:
+ - |
+ timer@ffe00000 {
+ compatible = "snps,dw-apb-timer";
+ interrupts = <0 170 4>;
+ reg = <0xffe00000 0x1000>;
+ clocks = <&timer_clk>, <&timer_pclk>;
+ clock-names = "timer", "pclk";
+ };
+ - |
+ timer@ffe00000 {
+ compatible = "snps,dw-apb-timer";
+ interrupts = <0 170 4>;
+ reg = <0xffe00000 0x1000>;
+ clocks = <&timer_clk>;
+ clock-names = "timer";
+ };
+ - |
+ timer@ffe00000 {
+ compatible = "snps,dw-apb-timer";
+ interrupts = <0 170 4>;
+ reg = <0xffe00000 0x1000>;
+ clock-frequency = <25000000>;
+ };
+...
--
2.25.1
^ permalink raw reply related
* [PATCH v5 0/8] clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
Cc: Serge Semin, Serge Semin, Alexey Malahov, Maxim Kaurkin,
Pavel Parkhomenko, Ramil Zaripov, Ekaterina Skachko, Vadim Vlasov,
Alexey Kolotnikov, Paul Burton, Ralf Baechle, Arnd Bergmann,
Alessandro Zummo, Alexandre Belloni, Rob Herring, linux-mips,
linux-rtc, devicetree, linux-kernel
As for all Baikal-T1 SoC related patchsets, which need this, we replaced
the DW APB Timer legacy plain text-based dt-binding file with DT schema.
Similarly the MIPS GIC bindings file is also converted to DT schema seeing
it also defines the MIPS GIC Timer binding.
Aside from MIPS-specific r4k timer Baikal-T1 chip also provides a
functionality of two another timers: embedded into the MIPS GIC timer and
three external DW timers available over APB bus. But we can't use them
before the corresponding drivers are properly fixed. First of all DW APB
Timer shouldn't be bound to a single CPU, since as being accessible over
APB they are external with respect to all possible CPUs. Secondly there
might be more than just two DW APB Timers in the system (Baikal-T1 has
three of them), so permit the driver to use one of them as a clocksource
and the rest - for clockevents. Thirdly it's possible to use MIPS GIC
timer as a clocksource so register it in the corresponding subsystem
(the patch has been found in the Paul Burton MIPS repo so I left the
original Signed-off-by attribute). Finally in the same way as r4k timer
the MIPS GIC timer should be used with care when CPUFREQ config is enabled
since in case of CM2 the timer counting depends on the CPU reference clock
frequency while the clocksource subsystem currently doesn't support the
timers with non-stable clock.
This patchset is rebased and tested on the mainline Linux kernel 5.7-rc4:
base-commit: 0e698dfa2822 ("Linux 5.7-rc4")
tag: v5.7-rc4
Changelog v2:
- Fix the SoB tags.
- Our corporate email server doesn't change Message-Id anymore, so the
patchset is resubmitted being in the cover-letter-threaded format.
- Convert the "snps,dw-apb-timer" binding to DT schema in a dedicated
patch.
- Convert the "mti,gic" binding to DT schema in a dedicated patch.
Link: https://lore.kernel.org/linux-rtc/20200324174325.14213-1-Sergey.Semin@baikalelectronics.ru
Changelog v3:
- Make the MIPS GIC timer sub-node name not having a unit-address number.
- Discard allOf: [ $ref: /schemas/interrupt-controller.yaml# ] from MIPS
GIC bindings.
- Add patch moving the "snps,dw-apb-timer" binding file to the directory
with timers binding files.
Link: https://lore.kernel.org/linux-rtc/20200506214107.25956-1-Sergey.Semin@baikalelectronics.ru
Changelog v4:
- Mark clocksource as unstable instead of lowering its rating.
- Move conditional sched clocks registration to the Paul' patch.
- Add Thomas Gleixner to the patchset To-list to draw his attention to the
patch "dt-bindings: interrupt-controller: Convert mti,gic to DT schema".
Link: https://lore.kernel.org/linux-rtc/20200521005321.12129-1-Sergey.Semin@baikalelectronics.ru/
Changelog v5:
- Fix mistakenly added "git_" prefix. Obviously it was supposed to be gic_.
- Add new patch "clocksource: dw_apb_timer: Affiliate of-based timer with
any CPU"
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Paul Burton (1):
clocksource: mips-gic-timer: Register as sched_clock
Serge Semin (7):
dt-bindings: rtc: Convert snps,dw-apb-timer to DT schema
dt-bindings: timer: Move snps,dw-apb-timer DT schema from rtc
dt-bindings: interrupt-controller: Convert mti,gic to DT schema
clocksource: dw_apb_timer: Make CPU-affiliation being optional
clocksource: dw_apb_timer: Affiliate of-based timer with any CPU
clocksource: dw_apb_timer_of: Fix missing clockevent timers
clocksource: mips-gic-timer: Mark GIC timer as unstable if ref clock
changes
.../interrupt-controller/mips-gic.txt | 67 --------
.../interrupt-controller/mti,gic.yaml | 148 ++++++++++++++++++
.../devicetree/bindings/rtc/dw-apb.txt | 32 ----
.../bindings/timer/snps,dw-apb-timer.yaml | 88 +++++++++++
drivers/clocksource/Kconfig | 1 +
drivers/clocksource/dw_apb_timer.c | 5 +-
drivers/clocksource/dw_apb_timer_of.c | 8 +-
drivers/clocksource/mips-gic-timer.c | 50 +++++-
8 files changed, 288 insertions(+), 111 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml
delete mode 100644 Documentation/devicetree/bindings/rtc/dw-apb.txt
create mode 100644 Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
--
2.25.1
^ permalink raw reply
* Re: [PATCH v12 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
From: Tali Perry @ 2020-05-21 20:47 UTC (permalink / raw)
To: Wolfram Sang
Cc: Andy Shevchenko, Ofer Yehielli, Brendan Higgins, avifishman70,
Tomer Maimon, kfting, Patrick Venture, Nancy Yuen, Benjamin Fair,
Rob Herring, linux-arm-kernel, linux-i2c, OpenBMC Maillist,
devicetree, Linux Kernel Mailing List
In-Reply-To: <20200521203758.GA20150@ninjato>
On Thu, May 21, 2020 at 11:37 PM Wolfram Sang <wsa@the-dreams.de> wrote:
>
>
> > > > I wondered also about DEBUG_FS entries. I can see their value when
> > > > developing the driver. But since this is done now, do they really help a
> > > > user to debug a difficult case? I am not sure, and then I wonder if we
> > > > should have that code in upstream. I am open for discussion, though.
> > >
> > > The user wanted to have health monitor implemented on top of the driver.
> > > The user has 16 channels connected the multiple devices. All are operated
> > > using various daemons in the system. Sometimes the slave devices are power down.
> > > Therefor the user wanted to track the health status of the devices.
> >
> > Ah, then there are these options I have in mind (Wolfram, FYI as well!):
> > 1) push with debugfs as a temporary solution and convert to devlink health protocol [1];
> > 2) drop it and develop devlink_health solution;
> > 3) push debugfs and wait if I²C will gain devlink health support
>
> No need for 2). We can push it now and convert it later. That being
> said, I wonder if [1] is suitable for this driver? Things like NACKs and
> timeouts happen regularly on an I2C bus and are not a state of bad
> health.
>
Agree, having a timeout every now and then is not an issue. The user
is interested
in cases when the number of timeouts\BER\nack\recovery are high.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox