[PATCH v3] spi: spi-sun6i: implement DMA-based transfer mode
From: Alexander Kochetkov
DMA-based transfer will be enabled if data length is larger than FIFO size
(64 bytes for A64). This greatly reduce number of interrupts for
transferring data.
For smaller data size PIO mode will be used. In PIO mode whole buffer will
be loaded into FIFO.
If driver failed to request DMA channels then it fallback for PIO mode.
Tested on SOPINE (https://www.pine64.org/sopine/)
Signed-off-by: Alexander Kochetkov
---
Changes in v3:
- Remove use_dma from struct sun6i_spi
Changes in v2:
- Fix 'checkpatch.pl --strict' warnings
- Optimezed DMA transfers. Fix burst size and address width for
DMA transfers. The code works for me an I did some tests with different
values and conditions. Empirically found that trigger level used for
PIO mode also can be used for DMA mode (TRM for A64 lacks that
description)
- Handling inside sun6i_spi_handler() leaved as is, it explicity states that
sun6i_spi_drain_fifo() is not used in DMA mode. Yes, if we call
sun6i_spi_drain_fifo() after DMA transfer, it will not drain anything
becase DMA already drain FIFO.
I can remove condition if it's better without it.
drivers/spi/spi-sun6i.c | 194
1 file changed, 176 insertions(+), 18 deletions(-)
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 19238e1b76b4..5336c5e32694 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
@@ -52,10 +53,12 @@
#define SUN6I_FIFO_CTL_REG 0x18
#define SUN6I_FIFO_CTL_RF_RDY_TRIG_LEVEL_MASK 0xff
+#define SUN6I_FIFO_CTL_RF_DRQ_EN BIT(8)
#define SUN6I_FIFO_CTL_RF_RDY_TRIG_LEVEL_BITS 0
#define SUN6I_FIFO_CTL_RF_RST BIT(15)
#define SUN6I_FIFO_CTL_TF_ERQ_TRIG_LEVEL_MASK 0xff
#define SUN6I_FIFO_CTL_TF_ERQ_TRIG_LEVEL_BITS 16
+#define SUN6I_FIFO_CTL_TF_DRQ_EN BIT(24)
#define SUN6I_FIFO_CTL_TF_RST BIT(31)
#define SUN6I_FIFO_STA_REG 0x1c
@@ -83,6 +86,8 @@
struct sun6i_spi {
struct spi_master *master;
void __iomem*base_addr;
+ dma_addr_t dma_addr_rx;
+ dma_addr_t dma_addr_tx;
struct clk *hclk;
struct clk *mclk;
struct reset_control*rstc;
@@ -182,6 +187,68 @@ static size_t sun6i_spi_max_transfer_size(struct
spi_device *spi)
return SUN6I_MAX_XFER_SIZE - 1;
}
+static int sun6i_spi_prepare_dma(struct sun6i_spi *sspi,
+struct spi_transfer *tfr)
+{
+ struct dma_async_tx_descriptor *rxdesc, *txdesc;
+ struct spi_master *master = sspi->master;
+
+ rxdesc = NULL;
+ if (tfr->rx_buf) {
+ struct dma_slave_config rxconf = {
+ .direction = DMA_DEV_TO_MEM,
+ .src_addr = sspi->dma_addr_rx,
+ .src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
+ .src_maxburst = 8,
+ };
+
+ dmaengine_slave_config(master->dma_rx, &rxconf);
+
+ rxdesc = dmaengine_prep_slave_sg(master->dma_rx,
+tfr->rx_sg.sgl,
+tfr->rx_sg.nents,
+DMA_DEV_TO_MEM,
+DMA_PREP_INTERRUPT);
+ if (!rxdesc)
+ return -EINVAL;
+ }
+
+ txdesc = NULL;
+ if (tfr->tx_buf) {
+ struct dma_slave_config txconf = {
+ .direction = DMA_MEM_TO_DEV,
+ .dst_addr = sspi->dma_addr_tx,
+ .dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
+ .dst_maxburst = 8,
+ };
+
+ dmaengine_slave_config(master->dma_tx, &txconf);
+
+ txdesc = dmaengine_prep_slave_sg(master->dma_tx,
+tfr->tx_sg.sgl,
+tfr->tx_sg.nents,
+DMA_MEM_TO_DEV,
+DMA_PREP_INTERRUPT);
+ if (!txdesc) {
+ if (rxdesc)
+ dmaengine_terminate_sync(master->dma_rx);
+ return -EINVAL;
+ }
+ }
+
+ if (tfr->rx_buf) {
+ dmaengine_submit(rxdesc);
+ dma_async_issue_pending(master->dma_rx);
+ }
+
+ if (tfr->tx_buf) {
+ dmaengine_submit(txdesc);
+ dma_async_issue_pending(master->dma_tx);
+ }
+
+ return 0;
+}
+
static int sun6i_spi_transfer_one(struct spi_master *master,
struc
Re: [PATCH] spi: spi-sun6i: implement DMA-based transfer mode
> 19 окт. 2020 г., в 11:21, Maxime Ripard написал(а): > > Hi! > > On Thu, Oct 15, 2020 at 06:47:40PM +0300, Alexander Kochetkov wrote: >> DMA-based transfer will be enabled if data length is larger than FIFO size >> (64 bytes for A64). This greatly reduce number of interrupts for >> transferring data. >> >> For smaller data size PIO mode will be used. In PIO mode whole buffer will >> be loaded into FIFO. >> >> If driver failed to request DMA channels then it fallback for PIO mode. >> >> Tested on SOPINE (https://www.pine64.org/sopine/) >> >> Signed-off-by: Alexander Kochetkov > > Thanks for working on this, it's been a bit overdue Hi, Maxime! We did custom A64 based computation module for our product. Do you mean that A64 is obsolete or EOL product? If so, can you recommend active replacement for A64 from Allwinner same price? Alexander
[PATCH v2] spi: spi-sun6i: enable autosuspend feature
From: Alexander Kochetkov
If SPI is used for periodic polling any sensor, significant delays
sometimes appear. Switching on module clocks during resume lead to delays.
Enabling autosuspend mode causes the controller to not suspend between
SPI transfers and the delays disappear.
The commit also remove unnecessary call to pm_runtime_idle() used
to explicit put device to suspended state. Without pm_runtime_idle() PM
core will put device in the suspended state just after probe() returns.
Signed-off-by: Alexander Kochetkov
---
Changes in v2:
- Extend commit description with explanation about removal
of pm_runtime_idle()
drivers/spi/spi-sun6i.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 29ea1e87ce7e..86f29c3e335a 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -22,6 +22,8 @@
#include
+#define SUN6I_AUTOSUSPEND_TIMEOUT 2000
+
#define SUN6I_FIFO_DEPTH 128
#define SUN8I_FIFO_DEPTH 64
@@ -652,9 +654,10 @@ static int sun6i_spi_probe(struct platform_device *pdev)
goto err_free_dma_rx;
}
+ pm_runtime_set_autosuspend_delay(&pdev->dev, SUN6I_AUTOSUSPEND_TIMEOUT);
+ pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_set_active(&pdev->dev);
pm_runtime_enable(&pdev->dev);
- pm_runtime_idle(&pdev->dev);
ret = devm_spi_register_master(&pdev->dev, master);
if (ret) {
--
2.17.1
[PATCH v2] spi: spi-sun6i: implement DMA-based transfer mode
From: Alexander Kochetkov
DMA-based transfer will be enabled if data length is larger than FIFO size
(64 bytes for A64). This greatly reduce number of interrupts for
transferring data.
For smaller data size PIO mode will be used. In PIO mode whole buffer will
be loaded into FIFO.
If driver failed to request DMA channels then it fallback for PIO mode.
Tested on SOPINE (https://www.pine64.org/sopine/)
Signed-off-by: Alexander Kochetkov
---
Changes in v2:
- Fix 'checkpatch.pl --strict' warnings
- Optimezed DMA transfers. Fix burst size and address width for
DMA transfers. The code works for me an I did some tests with different
values and conditions. Empirically found that trigger level used for
PIO mode also can be used for DMA mode (TRM for A64 lacks that
description)
- Handling inside sun6i_spi_handler() leaved as is, it explicity states that
sun6i_spi_drain_fifo() is not used in DMA mode. Yes, if we call
sun6i_spi_drain_fifo() after DMA transfer, it will not drain anything
becase DMA already drain FIFO.
I can remove condition if it's better without it.
drivers/spi/spi-sun6i.c | 198
1 file changed, 179 insertions(+), 19 deletions(-)
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 19238e1b76b4..29ea1e87ce7e 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
@@ -52,10 +53,12 @@
#define SUN6I_FIFO_CTL_REG 0x18
#define SUN6I_FIFO_CTL_RF_RDY_TRIG_LEVEL_MASK 0xff
+#define SUN6I_FIFO_CTL_RF_DRQ_EN BIT(8)
#define SUN6I_FIFO_CTL_RF_RDY_TRIG_LEVEL_BITS 0
#define SUN6I_FIFO_CTL_RF_RST BIT(15)
#define SUN6I_FIFO_CTL_TF_ERQ_TRIG_LEVEL_MASK 0xff
#define SUN6I_FIFO_CTL_TF_ERQ_TRIG_LEVEL_BITS 16
+#define SUN6I_FIFO_CTL_TF_DRQ_EN BIT(24)
#define SUN6I_FIFO_CTL_TF_RST BIT(31)
#define SUN6I_FIFO_STA_REG 0x1c
@@ -83,6 +86,8 @@
struct sun6i_spi {
struct spi_master *master;
void __iomem*base_addr;
+ dma_addr_t dma_addr_rx;
+ dma_addr_t dma_addr_tx;
struct clk *hclk;
struct clk *mclk;
struct reset_control*rstc;
@@ -92,6 +97,7 @@ struct sun6i_spi {
const u8*tx_buf;
u8 *rx_buf;
int len;
+ booluse_dma;
unsigned long fifo_depth;
};
@@ -182,6 +188,68 @@ static size_t sun6i_spi_max_transfer_size(struct
spi_device *spi)
return SUN6I_MAX_XFER_SIZE - 1;
}
+static int sun6i_spi_prepare_dma(struct sun6i_spi *sspi,
+struct spi_transfer *tfr)
+{
+ struct dma_async_tx_descriptor *rxdesc, *txdesc;
+ struct spi_master *master = sspi->master;
+
+ rxdesc = NULL;
+ if (tfr->rx_buf) {
+ struct dma_slave_config rxconf = {
+ .direction = DMA_DEV_TO_MEM,
+ .src_addr = sspi->dma_addr_rx,
+ .src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
+ .src_maxburst = 8,
+ };
+
+ dmaengine_slave_config(master->dma_rx, &rxconf);
+
+ rxdesc = dmaengine_prep_slave_sg(master->dma_rx,
+tfr->rx_sg.sgl,
+tfr->rx_sg.nents,
+DMA_DEV_TO_MEM,
+DMA_PREP_INTERRUPT);
+ if (!rxdesc)
+ return -EINVAL;
+ }
+
+ txdesc = NULL;
+ if (tfr->tx_buf) {
+ struct dma_slave_config txconf = {
+ .direction = DMA_MEM_TO_DEV,
+ .dst_addr = sspi->dma_addr_tx,
+ .dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
+ .dst_maxburst = 8,
+ };
+
+ dmaengine_slave_config(master->dma_tx, &txconf);
+
+ txdesc = dmaengine_prep_slave_sg(master->dma_tx,
+tfr->tx_sg.sgl,
+tfr->tx_sg.nents,
+DMA_MEM_TO_DEV,
+DMA_PREP_INTERRUPT);
+ if (!txdesc) {
+ if (rxdesc)
+ dmaengine_terminate_sync(master->dma_rx);
+ return -EINVAL;
+ }
+ }
+
+ if (tfr->rx_buf) {
+ dmaengine_submit(rxdesc);
+ dma_async_issue_pending(master->dma_rx);
+ }
+
+ if (tfr->tx_buf) {
+ dmaeng
Re: [PATCH] spi: spi-sun6i: implement DMA-based transfer mode
Hi, Maxime! Thanks for reviewing patches!
>>
>> +static int sun6i_spi_prepare_dma(struct sun6i_spi *sspi,
>> + struct spi_transfer *tfr)
>> +{
>> +struct dma_async_tx_descriptor *rxdesc, *txdesc;
>> +struct spi_master *master = sspi->master;
>> +
>> +rxdesc = NULL;
>> +if (tfr->rx_buf) {
>> +struct dma_slave_config rxconf = {
>> +.direction = DMA_DEV_TO_MEM,
>> +.src_addr = sspi->dma_addr_rx,
>> +.src_addr_width = 1,
>> +.src_maxburst = 1,
>> +};
>
> That doesn't really look optimal, the controller seems to be able to
> read / write 32 bits at a time from its FIFO and we probably can
> increase the burst length too?
I had doubts if it would work. I didn’t know will DMA work for transfers with
lengths not
aligned to 32 bits. For example, if we init DMA with src_addr_width = 1 and
.src_maxburst = 8 will DMA work for transfer with length 11? I expect that DMA
fill FIFO
with 16 bytes and SPI transfer only 11 bytes and 5 bytes will leave in TX fifo.
I did the test
and there is no additional data left in the fifo buffer. Also at reception the
is no memory
overwrites.
I made test with src_addr_width = 4, src_maxburst = 1 and transfer length 3.
Looks
like in that case DMA doesn’t issue 4 bytes transfer.
For test with src_addr_width = 4, src_maxburst = 8 I had to adjust
RF_RDY_TRIG_LEVEL_BITS
and TF_ERQ_TRIG_LEVEL_BITS of FIFO_CTL_REG to half of FIFO (32 bytes). With the
config
DMA will transfer burst of half of FIFO length during transfer and remaining
bytes at the end of
transfer.
>>
>> @@ -343,7 +436,8 @@ static irqreturn_t sun6i_spi_handler(int irq, void
>> *dev_id)
>> /* Transfer complete */
>> if (status & SUN6I_INT_CTL_TC) {
>> sun6i_spi_write(sspi, SUN6I_INT_STA_REG, SUN6I_INT_CTL_TC);
>> -sun6i_spi_drain_fifo(sspi);
>> +if (!sspi->use_dma)
>> +sun6i_spi_drain_fifo(sspi);
>
> Is it causing any issue? The FIFO will be drained only if there's
> something remaining in the FIFO, which shouldn't happen with DMA?
>
No. It’s for make code patch explicit.
Remove the change?
Alexander.
[PATCH] spi: rockchip: enable autosuspend feature
If SPI is used for periodic polling any sensor, significant delays
sometimes appear. Switching on module clocks during resume lead to delays.
Enabling autosuspend mode causes the controller to not suspend between
SPI transfers and the delays disappear.
Signed-off-by: Alexander Kochetkov
---
drivers/spi/spi-rockchip.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 2cc6d9951b52..3e77b1a79bc8 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -157,6 +157,8 @@
#define ROCKCHIP_SPI_MAX_CS_NUM2
+#define ROCKCHIP_AUTOSUSPEND_TIMEOUT 2000
+
struct rockchip_spi {
struct device *dev;
@@ -670,6 +672,8 @@ static int rockchip_spi_probe(struct platform_device *pdev)
goto err_disable_spiclk;
}
+ pm_runtime_set_autosuspend_delay(&pdev->dev,
ROCKCHIP_AUTOSUSPEND_TIMEOUT);
+ pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_set_active(&pdev->dev);
pm_runtime_enable(&pdev->dev);
--
2.17.1
[PATCH] spi: spi-sun6i: enable autosuspend feature
If SPI is used for periodic polling any sensor, significant delays
sometimes appear. Switching on module clocks during resume lead to delays.
Enabling autosuspend mode causes the controller to not suspend between
SPI transfers and the delays disappear.
Signed-off-by: Alexander Kochetkov
---
drivers/spi/spi-sun6i.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 38e5b8af7da6..4cc0280e934c 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -22,6 +22,8 @@
#include
+#define SUN6I_AUTOSUSPEND_TIMEOUT 2000
+
#define SUN6I_FIFO_DEPTH 128
#define SUN8I_FIFO_DEPTH 64
@@ -639,9 +641,10 @@ static int sun6i_spi_probe(struct platform_device *pdev)
goto err_free_dma_rx;
}
+ pm_runtime_set_autosuspend_delay(&pdev->dev, SUN6I_AUTOSUSPEND_TIMEOUT);
+ pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_set_active(&pdev->dev);
pm_runtime_enable(&pdev->dev);
- pm_runtime_idle(&pdev->dev);
ret = devm_spi_register_master(&pdev->dev, master);
if (ret) {
--
2.17.1
[PATCH] spi: spi-sun6i: implement DMA-based transfer mode
DMA-based transfer will be enabled if data length is larger than FIFO size
(64 bytes for A64). This greatly reduce number of interrupts for
transferring data.
For smaller data size PIO mode will be used. In PIO mode whole buffer will
be loaded into FIFO.
If driver failed to request DMA channels then it fallback for PIO mode.
Tested on SOPINE (https://www.pine64.org/sopine/)
Signed-off-by: Alexander Kochetkov
---
drivers/spi/spi-sun6i.c | 171 +---
1 file changed, 159 insertions(+), 12 deletions(-)
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 19238e1b76b4..38e5b8af7da6 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
@@ -52,10 +53,12 @@
#define SUN6I_FIFO_CTL_REG 0x18
#define SUN6I_FIFO_CTL_RF_RDY_TRIG_LEVEL_MASK 0xff
+#define SUN6I_FIFO_CTL_RF_DRQ_EN BIT(8)
#define SUN6I_FIFO_CTL_RF_RDY_TRIG_LEVEL_BITS 0
#define SUN6I_FIFO_CTL_RF_RST BIT(15)
#define SUN6I_FIFO_CTL_TF_ERQ_TRIG_LEVEL_MASK 0xff
#define SUN6I_FIFO_CTL_TF_ERQ_TRIG_LEVEL_BITS 16
+#define SUN6I_FIFO_CTL_TF_DRQ_EN BIT(24)
#define SUN6I_FIFO_CTL_TF_RST BIT(31)
#define SUN6I_FIFO_STA_REG 0x1c
@@ -83,6 +86,8 @@
struct sun6i_spi {
struct spi_master *master;
void __iomem*base_addr;
+ dma_addr_t dma_addr_rx;
+ dma_addr_t dma_addr_tx;
struct clk *hclk;
struct clk *mclk;
struct reset_control*rstc;
@@ -92,6 +97,7 @@ struct sun6i_spi {
const u8*tx_buf;
u8 *rx_buf;
int len;
+ booluse_dma;
unsigned long fifo_depth;
};
@@ -182,6 +188,66 @@ static size_t sun6i_spi_max_transfer_size(struct
spi_device *spi)
return SUN6I_MAX_XFER_SIZE - 1;
}
+static int sun6i_spi_prepare_dma(struct sun6i_spi *sspi,
+struct spi_transfer *tfr)
+{
+ struct dma_async_tx_descriptor *rxdesc, *txdesc;
+ struct spi_master *master = sspi->master;
+
+ rxdesc = NULL;
+ if (tfr->rx_buf) {
+ struct dma_slave_config rxconf = {
+ .direction = DMA_DEV_TO_MEM,
+ .src_addr = sspi->dma_addr_rx,
+ .src_addr_width = 1,
+ .src_maxburst = 1,
+ };
+
+ dmaengine_slave_config(master->dma_rx, &rxconf);
+
+ rxdesc = dmaengine_prep_slave_sg(
+ master->dma_rx,
+ tfr->rx_sg.sgl, tfr->rx_sg.nents,
+ DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT);
+ if (!rxdesc)
+ return -EINVAL;
+ }
+
+ txdesc = NULL;
+ if (tfr->tx_buf) {
+ struct dma_slave_config txconf = {
+ .direction = DMA_MEM_TO_DEV,
+ .dst_addr = sspi->dma_addr_tx,
+ .dst_addr_width = 1,
+ .dst_maxburst = 1,
+ };
+
+ dmaengine_slave_config(master->dma_tx, &txconf);
+
+ txdesc = dmaengine_prep_slave_sg(
+ master->dma_tx,
+ tfr->tx_sg.sgl, tfr->tx_sg.nents,
+ DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT);
+ if (!txdesc) {
+ if (rxdesc)
+ dmaengine_terminate_sync(master->dma_rx);
+ return -EINVAL;
+ }
+ }
+
+ if (tfr->rx_buf) {
+ dmaengine_submit(rxdesc);
+ dma_async_issue_pending(master->dma_rx);
+ }
+
+ if (tfr->tx_buf) {
+ dmaengine_submit(txdesc);
+ dma_async_issue_pending(master->dma_tx);
+ }
+
+ return 0;
+}
+
static int sun6i_spi_transfer_one(struct spi_master *master,
struct spi_device *spi,
struct spi_transfer *tfr)
@@ -201,6 +267,8 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
sspi->tx_buf = tfr->tx_buf;
sspi->rx_buf = tfr->rx_buf;
sspi->len = tfr->len;
+ sspi->use_dma = master->can_dma ?
+ master->can_dma(master, spi, tfr) : false;
/* Clear pending interrupts */
sun6i_spi_write(sspi, SUN6I_INT_STA_REG, ~0);
@@ -216,9 +284,17 @@ static int sun6i_spi_transfer_one(struct spi_master
*master,
* (See spi-sun4i.c)
*/
trig_level = sspi->fifo_depth / 4 * 3;
- sun6i_spi_write(sspi, SUN6
[PATCH] arm64: dts: allwinner: replace numerical constant with CCU_CLKX
From: Alexander Kochetkov Signed-off-by: Alexander Kochetkov --- arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi index c26cc1fcaffd..dfeeb7350808 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi @@ -51,7 +51,7 @@ reg = <0>; enable-method = "psci"; next-level-cache = <&L2>; - clocks = <&ccu 21>; + clocks = <&ccu CLK_CPUX>; clock-names = "cpu"; #cooling-cells = <2>; }; @@ -62,7 +62,7 @@ reg = <1>; enable-method = "psci"; next-level-cache = <&L2>; - clocks = <&ccu 21>; + clocks = <&ccu CLK_CPUX>; clock-names = "cpu"; #cooling-cells = <2>; }; @@ -73,7 +73,7 @@ reg = <2>; enable-method = "psci"; next-level-cache = <&L2>; - clocks = <&ccu 21>; + clocks = <&ccu CLK_CPUX>; clock-names = "cpu"; #cooling-cells = <2>; }; @@ -84,7 +84,7 @@ reg = <3>; enable-method = "psci"; next-level-cache = <&L2>; - clocks = <&ccu 21>; + clocks = <&ccu CLK_CPUX>; clock-names = "cpu"; #cooling-cells = <2>; }; -- 2.17.1
Re: [PATCH 2/2] mmc: dw_mmc-rockchip: fix transfer hangs on rk3188【请注意，邮件由linux-mmc-ow...@vger.kernel.org代发】
Hello!
Forgot to mention transfer hags happen only on mem to dev transfers (dma writes
to
device) and never on dev to mem.
Yea, I know, rk3188 and earlier are quite ancient, but we made custom hardware
based on rk3188 and some of our customers report problems.
For testing I use rk3188 based custom board with eMMC (probably rk3188-radxa
rock
with SD can also be used for testing) with cpufreq enabled.
For testing I made simple script, that do in loop following:
1. Creates 6 new empty partitions using mkfs.ext3 about 1Gb total
2. extract 100MB archive of linux image to 512Mb partition (about 400MB
extracted size).
3. sleep random time from 60 to 120 sec
CPU load looks like that:
cpufreq stats: 312 MHz:32.63%, 504 MHz:0.00%, 600 MHz:0.00%, 816 MHz:0.38%,
1.01 GHz:29.83%, 1.20 GHz:0.38%, 1.42 GHz:0.00%, 1.61 GHz:36.79% (494481)
This test can run for 6 hours and than transfer can hang. I used 5 devices to
test. Some
devices may run test for long time, but some may fail within an hour.
I played with CPU clock settings in u-boot and mmc bus clock settings dts file.
I tried to lower eMMC bus
clock frequency to exclude PCB errors. Found that some combinations of settings
make my test run longer, but test fail anyway.
Also I found, that making following change to dw_mmc, result in high error
count:
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 9c54d60..dcf7d36e 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2905,10 +2905,9 @@ static int dw_mci_init_slot(struct dw_mci *host)
} else if (host->use_dma == TRANS_MODE_EDMAC) {
mmc->max_segs = 64;
mmc->max_blk_size = 65535;
- mmc->max_blk_count = 65535;
- mmc->max_req_size =
- mmc->max_blk_size * mmc->max_blk_count;
- mmc->max_seg_size = mmc->max_req_size;
+ mmc->max_seg_size = 0x1000;
+ mmc->max_req_size = mmc->max_seg_size * mmc->max_segs;
+ mmc->max_blk_count = mmc->max_req_size / 512;
} else {
/* TRANS_MODE_PIO */
mmc->max_segs = 64;
With this settings mmc core split large transfer to multiply item scatterlists
and
increase scatterlists switching rate inside pl330. So I assumed that the root
of problem
is dma goes out of sync with device.
For, example, there is a patch in mainline linux:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/dma/pl330.c?h=v5.0.3&id=1d48745b192a7a45bbdd3557b4c039609569ca41
It fix the problem EDMA can get out of sync with device. But the patch don’t
work for rk3188, because
rk3188 has PL330_QUIRK_BROKEN_NO_FLUSHP quirk.
I’ll try to backport EDMA driver from vendor 4.4 kernel and report test result.
Problem safer to fix patching dw_mmc code, than pl330 code. Because
patch change transfer parameters from known to work values:
mmc->max_segs = 64;
mmc->max_blk_size = 65535;
mmc->max_blk_count = 65535;
mmc->max_req_size =
mmc->max_blk_size * mmc->max_blk_count;
mmc->max_seg_size = mmc->max_req_size;
to
mmc->max_segs = 1;
mmc->max_blk_size = 65535;
mmc->max_blk_count = 64 * 512;
mmc->max_req_size =
mmc->max_blk_size * mmc->max_blk_count;
mmc->max_seg_size = mmc->max_req_size;
> 21 марта 2019 г., в 5:31, Shawn Lin написал(а):
>
> + Caesar Wang
>
> On 2019/3/21 1:48, Alexander Kochetkov wrote:
>> I've found that sometimes dw_mmc in my rk3188 based board stop transfer
>> any data with error:
>> kernel: dwmmc_rockchip 1021c000.dwmmc: Unexpected command timeout, state 3
>> Further digging into problem showed that sometimes one of EDMA-based
>> transfers hangs and abort with HTO error. I've made test, that 100%
>
> I'm not sure what 100% means, but Caesar fired QA test for RK3036 with
> EDMA-based dwmmc in vendor 4.4 kernel, and seems not big deal. The
> vendor 4.4 kernel didn't patch anything else wrt EDMA code, but we did
> enhance PL330 code and fix some bug there, so you may have a try.
>
>> reproduce the error. I found, that setting max_segs parameter to 1 fix
>> the problem.
>> I guess the problem is hardware related and relates to DMA controller
>> implementation for rk3188. Probably it can relates to missed FLUSHP,
>> see commit 271e1b86e691 ("dmaengine: pl330: add quirk for broken no
>> flushp"). It is possible that pl330 and dw_mmc become out of sync then
>> pl330 driver switch from one scatterlist to another. If we limit
>> scatterlist size to 1, we can avoid switching sca
[PATCH 1/2] mmc: dw_mmc: add init_slot() hook to platform function table
The init_slot() hook allow platform driver override slot defaults
provided by generic dw_mmc driver. It's required to fix EDMA based
transfer hangs observed on rockchip rk3188.
Signed-off-by: Alexander Kochetkov
---
drivers/mmc/host/dw_mmc.c |4
drivers/mmc/host/dw_mmc.h |2 ++
2 files changed, 6 insertions(+)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 80dc2fd..d3ecee9 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2819,6 +2819,7 @@ static int dw_mci_init_slot_caps(struct dw_mci_slot *slot)
static int dw_mci_init_slot(struct dw_mci *host)
{
+ const struct dw_mci_drv_data *drv_data = host->drv_data;
struct mmc_host *mmc;
struct dw_mci_slot *slot;
int ret;
@@ -2876,6 +2877,9 @@ static int dw_mci_init_slot(struct dw_mci *host)
mmc->max_seg_size = mmc->max_req_size;
}
+ if (drv_data && drv_data->init_slot)
+ drv_data->init_slot(host);
+
dw_mci_get_cd(mmc);
ret = mmc_add_host(mmc);
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 46e9f8e..de51c59 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -548,6 +548,7 @@ struct dw_mci_slot {
* @caps: mmc subsystem specified capabilities of the controller(s).
* @num_caps: number of capabilities specified by @caps.
* @init: early implementation specific initialization.
+ * @init_slot: platform specific slot initialization.
* @set_ios: handle bus specific extensions.
* @parse_dt: parse implementation specific device tree properties.
* @execute_tuning: implementation specific tuning procedure.
@@ -560,6 +561,7 @@ struct dw_mci_drv_data {
unsigned long *caps;
u32 num_caps;
int (*init)(struct dw_mci *host);
+ void(*init_slot)(struct dw_mci *host);
void(*set_ios)(struct dw_mci *host, struct mmc_ios *ios);
int (*parse_dt)(struct dw_mci *host);
int (*execute_tuning)(struct dw_mci_slot *slot, u32 opcode);
--
1.7.9.5
[PATCH 2/2] mmc: dw_mmc-rockchip: fix transfer hangs on rk3188
I've found that sometimes dw_mmc in my rk3188 based board stop transfer
any data with error:
kernel: dwmmc_rockchip 1021c000.dwmmc: Unexpected command timeout, state 3
Further digging into problem showed that sometimes one of EDMA-based
transfers hangs and abort with HTO error. I've made test, that 100%
reproduce the error. I found, that setting max_segs parameter to 1 fix
the problem.
I guess the problem is hardware related and relates to DMA controller
implementation for rk3188. Probably it can relates to missed FLUSHP,
see commit 271e1b86e691 ("dmaengine: pl330: add quirk for broken no
flushp"). It is possible that pl330 and dw_mmc become out of sync then
pl330 driver switch from one scatterlist to another. If we limit
scatterlist size to 1, we can avoid switching scatterlists and avoid
hardware problem. Setting max_segs to 1 tells mmc core to use maximum
one scatterlist for one transfer.
I guess that all other rk3xxx chips that lacks FLUSHP also affected by
the problem. So I made fix for all rk3xxx chips from rk2928 to rk3188.
Signed-off-by: Alexander Kochetkov
---
drivers/mmc/host/dw_mmc-rockchip.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/mmc/host/dw_mmc-rockchip.c
b/drivers/mmc/host/dw_mmc-rockchip.c
index 8c86a80..2eed922 100644
--- a/drivers/mmc/host/dw_mmc-rockchip.c
+++ b/drivers/mmc/host/dw_mmc-rockchip.c
@@ -292,6 +292,24 @@ static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
return 0;
}
+static void dw_mci_rk2928_init_slot(struct dw_mci *host)
+{
+ struct mmc_host *mmc = host->slot->mmc;
+
+ if (host->use_dma == TRANS_MODE_EDMAC) {
+ /*
+* Using max_segs > 1 leads to rare EDMA transfer hangs
+* resulting in HTO errors.
+*/
+ mmc->max_segs = 1;
+ mmc->max_blk_size = 65535;
+ mmc->max_blk_count = 64 * 512;
+ mmc->max_req_size =
+ mmc->max_blk_size * mmc->max_blk_count;
+ mmc->max_seg_size = mmc->max_req_size;
+ }
+}
+
static int dw_mci_rockchip_init(struct dw_mci *host)
{
/* It is slot 8 on Rockchip SoCs */
@@ -314,6 +332,7 @@ static int dw_mci_rockchip_init(struct dw_mci *host)
static const struct dw_mci_drv_data rk2928_drv_data = {
.init = dw_mci_rockchip_init,
+ .init_slot = dw_mci_rk2928_init_slot,
};
static const struct dw_mci_drv_data rk3288_drv_data = {
--
1.7.9.5
[PATCH 0/2] Fix eMMC hang on rk3188 and earlier
Hello! I found, that sometimes dw_mmc driver stop transfer data to eMMC card on my rk3188 based board. One of tranfers hangs then doing EDMA transfer and controller gives HTO. And here is a fix. Alexander Kochetkov (2): mmc: dw_mmc: add init_slot() hook to platform function table mmc: dw_mmc-rockchip: fix transfer hangs on rk3188 drivers/mmc/host/dw_mmc-rockchip.c | 19 +++ drivers/mmc/host/dw_mmc.c |4 drivers/mmc/host/dw_mmc.h |2 ++ 3 files changed, 25 insertions(+) -- 1.7.9.5
Re: [PATCH AUTOSEL for 3.18 34/63] ARM: dts: rockchip: disable arm-global-timer for rk3188
Hello, Sasha!
Following 2 patches must be applied together with the patch:
5e0a39d0f727b35c8b7ef56ba0724c8ceb006297 clocksource/drivers/rockchip_timer:
Implement clocksource timer
627988a66aee3c845aa2f1f874a3ddba8adb89d9 ARM: dts: rockchip: Add timer entries
to rk3188 SoC
Alexander.
> 4 марта 2018 г., в 1:33, Sasha Levin
> написал(а):
>
> From: Alexander Kochetkov
>
> [ Upstream commit 500d0aa918a2ea6bb918fee8adcf27dc2912bcd1 ]
>
> The clocksource and the sched_clock provided by the arm_global_timer
> are quite unstable because their rates depend on the cpu frequency.
>
> On the other side, the arm_global_timer has a higher rating than the
> rockchip_timer, it will be selected by default by the time framework
> while we want to use the stable rockchip clocksource.
>
> Let's disable the arm_global_timer in order to have the rockchip
> clocksource selected by default.
>
> Signed-off-by: Alexander Kochetkov
> Signed-off-by: Daniel Lezcano
> Reviewed-by: Heiko Stuebner
> Signed-off-by: Sasha Levin
> ---
> arch/arm/boot/dts/rk3188.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
> index ddaada788b45..72b0b4cb538f 100644
> --- a/arch/arm/boot/dts/rk3188.dtsi
> +++ b/arch/arm/boot/dts/rk3188.dtsi
> @@ -404,6 +404,7 @@
>
> &global_timer {
> interrupts = ;
> + status = "disabled";
> };
>
> &local_timer {
> --
> 2.14.1
Re: [PATCH AUTOSEL for 4.4 062/115] ARM: dts: rockchip: disable arm-global-timer for rk3188
Hello, Sasha!
Following 2 patches must be applied together with the patch:
5e0a39d0f727b35c8b7ef56ba0724c8ceb006297 clocksource/drivers/rockchip_timer:
Implement clocksource timer
627988a66aee3c845aa2f1f874a3ddba8adb89d9 ARM: dts: rockchip: Add timer entries
to rk3188 SoC
Alexander.
> 4 марта 2018 г., в 1:31, Sasha Levin
> написал(а):
>
> From: Alexander Kochetkov
>
> [ Upstream commit 500d0aa918a2ea6bb918fee8adcf27dc2912bcd1 ]
>
> The clocksource and the sched_clock provided by the arm_global_timer
> are quite unstable because their rates depend on the cpu frequency.
>
> On the other side, the arm_global_timer has a higher rating than the
> rockchip_timer, it will be selected by default by the time framework
> while we want to use the stable rockchip clocksource.
>
> Let's disable the arm_global_timer in order to have the rockchip
> clocksource selected by default.
>
> Signed-off-by: Alexander Kochetkov
> Signed-off-by: Daniel Lezcano
> Reviewed-by: Heiko Stuebner
> Signed-off-by: Sasha Levin
> ---
> arch/arm/boot/dts/rk3188.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
> index 6399942f1840..0e5142b6e914 100644
> --- a/arch/arm/boot/dts/rk3188.dtsi
> +++ b/arch/arm/boot/dts/rk3188.dtsi
> @@ -513,6 +513,7 @@
>
> &global_timer {
> interrupts = ;
> + status = "disabled";
> };
>
> &local_timer {
> --
> 2.14.1
Re: [PATCH AUTOSEL for 4.9 124/219] ARM: dts: rockchip: disable arm-global-timer for rk3188
Hello, Sasha!
Following 2 patches must be applied together with the patch:
5e0a39d0f727b35c8b7ef56ba0724c8ceb006297 clocksource/drivers/rockchip_timer:
Implement clocksource timer
627988a66aee3c845aa2f1f874a3ddba8adb89d9 ARM: dts: rockchip: Add timer entries
to rk3188 SoC
Alexander.
> 4 марта 2018 г., в 1:29, Sasha Levin
> написал(а):
>
> From: Alexander Kochetkov
>
> [ Upstream commit 500d0aa918a2ea6bb918fee8adcf27dc2912bcd1 ]
>
> The clocksource and the sched_clock provided by the arm_global_timer
> are quite unstable because their rates depend on the cpu frequency.
>
> On the other side, the arm_global_timer has a higher rating than the
> rockchip_timer, it will be selected by default by the time framework
> while we want to use the stable rockchip clocksource.
>
> Let's disable the arm_global_timer in order to have the rockchip
> clocksource selected by default.
>
> Signed-off-by: Alexander Kochetkov
> Signed-off-by: Daniel Lezcano
> Reviewed-by: Heiko Stuebner
> Signed-off-by: Sasha Levin
> ---
> arch/arm/boot/dts/rk3188.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
> index 31f81b265cef..73a200914f0b 100644
> --- a/arch/arm/boot/dts/rk3188.dtsi
> +++ b/arch/arm/boot/dts/rk3188.dtsi
> @@ -530,6 +530,7 @@
>
> &global_timer {
> interrupts = ;
> + status = "disabled";
> };
>
> &local_timer {
> --
> 2.14.1
Re: [PATCH 1/2] clk: rename clk_core_get_boundaries() to clk_hw_get_boundaries() and expose
> 29 дек. 2017 г., в 3:14, Stephen Boyd написал(а): > > I'm asking if the rate is capped on the consumer side with > clk_set_max_rate() or if it's capped on the clk provider side to > express a hardware constraint. I do that using clk_set_max_rate() at provider size inside clk-rk3188.c. > > Sounds like there are some things to be figured out here still. I > can take a closer look next week. Maybe Heiko will respond before > then. I will be very grateful for the ideas. I can continue to work on this next week too. Happy New Year and Merry Christmas! Regards, Alexander.
Re: [PATCH 1/2] clk: rename clk_core_get_boundaries() to clk_hw_get_boundaries() and expose
Initial thread here: https://www.spinics.net/lists/linux-clk/msg21682.html > 27 дек. 2017 г., в 4:06, Stephen Boyd написал(а): > > Are these limits the min/max limits that the parent clk can > output at? Or the min/max limits that software has constrained on > the clk? > Don’t know how to answer. For example, parent can output 768MHz, but some IP work unstable with that parent rate. This issues was observed by me and I didn’t get official confirmation from rockchip. So, I limit such clock to 192MHz using clk_set_max_rate(). May be I have to limit clk rate using another approach. Anybody from rockchip can confirm that? Or may be contact me clarifying details? > I haven't looked in detail at this > rockchip_fractional_approximation() code, but it shouldn't be > doing the work of both the child rate determination and the > parent rate determination in one place. It should work with the > parent to figure out the rate the parent can provide and then > figure out how to achieve the desired rate from there. Here is clock tree: clock rate - xin24m 2400 pll_gpll76800 gpll 76800 i2s_src 76800 i2s0_pre 19200 i2s0_frac 16384000 sclk_i2s0 16384000 I limit i2s0_pre rate to 192MHz in order to I2S IP work properly. rockchip_fractional_approximation() get called for i2s0_frac. if i2s0_pre rate is 20x times less than i2s0_frac, than rockchip_fractional_approximation() tries to set i2s0_pre rate to i2s_src rate. It tries to increase it’s parent rate in order to maximise relation between nominator and denominator. If I convert rockchip_fractional_approximation() to rockchip_determine_rate(), than I get min=0, max=192MHz limits inside rockchip_determine_rate(). May be there should be new logic inside clk framework based on some new clk flags, that will provide max=768MHz for rockchip_determine_rate(). Also found, that rockchip_fractional_approximation() increase parents rate unconditionally without taking into account CLK_SET_RATE_PARENT flag. Stephen, thanks a lot for deep description of determine_rate() background. I’ll taking some time thinking about possible solutions. Regards, Alexander.
Re: [PATCH 1/2] clk: rename clk_core_get_boundaries() to clk_hw_get_boundaries() and expose
> 21 дек. 2017 г., в 23:07, Stephen Boyd написал(а):
>
> Can you convert to the determine_rate op instead of round_rate?
> That function should tell you the min/max limits so that you
> don't need to query that information from the core.
I converted rockchip_fractional_approximation() to rockchip_determine_rate()
(see the patch attached).
If it increase parent’s clock for out of limits value, than clock request will
fail with -EINVAL, like
with round_rate() approach.
The problem is that min/max limits provided to determine_rate() is for clock
for which the determine_rate()
was called. While rockchip_determine_rate()
(rockchip_fractional_approximation()) requires information
about parent clock limits.
How can I know parents clock limits for current clock? Implement
determine_rate() for each parent clocks
the same way I did for this one clock?
Regards,
Alexander.
diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
index 3c1fb0d..1e0c701 100644
--- a/drivers/clk/rockchip/clk.c
+++ b/drivers/clk/rockchip/clk.c
@@ -174,23 +174,9 @@ static void rockchip_fractional_approximation(struct
clk_hw *hw,
unsigned long *m, unsigned long *n)
{
struct clk_fractional_divider *fd = to_clk_fd(hw);
- unsigned long p_rate, p_parent_rate;
- unsigned long min_rate = 0, max_rate = 0;
- struct clk_hw *p_parent;
unsigned long scale;
-
- p_rate = clk_hw_get_rate(clk_hw_get_parent(hw));
- if ((rate * 20 > p_rate) && (p_rate % rate != 0)) {
- p_parent = clk_hw_get_parent(clk_hw_get_parent(hw));
- p_parent_rate = clk_hw_get_rate(p_parent);
- clk_hw_get_boundaries(clk_hw_get_parent(hw),
- &min_rate, &max_rate);
- if (p_parent_rate < min_rate)
- p_parent_rate = min_rate;
- if (p_parent_rate > max_rate)
- p_parent_rate = max_rate;
- *parent_rate = p_parent_rate;
- }
+ unsigned long rate_orig = rate;
+ unsigned long parent_rate_orig = *parent_rate;
/*
* Get rate closer to *parent_rate to guarantee there is no overflow
@@ -204,8 +190,36 @@ static void rockchip_fractional_approximation(struct
clk_hw *hw,
rational_best_approximation(rate, *parent_rate,
GENMASK(fd->mwidth - 1, 0), GENMASK(fd->nwidth - 1, 0),
m, n);
+
+ pr_info("%s: %s: rate:%lu -> %lu parent_rate:%lu -> %lu m:%lu n:%lu\n",
+ __func__, clk_hw_get_name(hw), rate_orig, rate,
+ parent_rate_orig, *parent_rate,
+ *m, *n);
}
+static int rockchip_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ unsigned long p_rate, p_parent_rate;
+ struct clk_hw *p_parent;
+ unsigned long best_parent_rate = req->best_parent_rate;
+
+ p_rate = clk_hw_get_rate(clk_hw_get_parent(hw));
+ if ((req->rate * 20 > p_rate) && (p_rate % req->rate != 0)) {
+ p_parent = clk_hw_get_parent(clk_hw_get_parent(hw));
+ p_parent_rate = clk_hw_get_rate(p_parent);
+ req->best_parent_rate = p_parent_rate;
+ }
+
+ pr_info("%s: %s: rate:%lu min_rate:%lu max_rate:%lu
best_parent_rate:%lu -> %lu best_parent_hw:%s\n",
+ __func__, clk_hw_get_name(hw), req->rate, req->min_rate,
req->max_rate, best_parent_rate, req->best_parent_rate,
+ req->best_parent_hw ? clk_hw_get_name(req->best_parent_hw) :
"");
+
+ return 0;
+}
+
+static struct clk_ops rockchip_clk_fractional_divider_ops;
+
static struct clk *rockchip_clk_register_frac_branch(
struct rockchip_clk_provider *ctx, const char *name,
const char *const *parent_names, u8 num_parents,
@@ -253,7 +267,8 @@ static struct clk *rockchip_clk_register_frac_branch(
div->nmask = GENMASK(div->nwidth - 1, 0) << div->nshift;
div->lock = lock;
div->approximation = rockchip_fractional_approximation;
- div_ops = &clk_fractional_divider_ops;
+ div_ops = &rockchip_clk_fractional_divider_ops;
+
clk = clk_register_composite(NULL, name, parent_names, num_parents,
NULL, NULL,
@@ -392,6 +407,9 @@ struct rockchip_clk_provider * __init
rockchip_clk_init(struct device_node *np,
ctx->grf = syscon_regmap_lookup_by_phandle(ctx->cru_node,
"rockchip,grf");
+ rockchip_clk_fractional_divider_ops = clk_fractional_divider_ops;
+ rockchip_clk_fractional_divider_ops.determine_rate =
rockchip_determine_rate;
+
return ctx;
err_free:
[PATCH 1/2] clk: rename clk_core_get_boundaries() to clk_hw_get_boundaries() and expose
In order to provide a way to know clock limits to clock providers.
The patch is needed for fixing commit 5d890c2df900 ("clk: rockchip:
add special approximation to fix up fractional clk's jitter").
Custom approximation function introduced by the patch, can
select frequency rate larger than one configured using
clk_set_max_rate().
Signed-off-by: Alexander Kochetkov
---
drivers/clk/clk.c| 14 --
include/linux/clk-provider.h |2 ++
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index c8d83ac..8943aac 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -421,10 +421,11 @@ struct clk *__clk_lookup(const char *name)
return !core ? NULL : core->hw->clk;
}
-static void clk_core_get_boundaries(struct clk_core *core,
- unsigned long *min_rate,
- unsigned long *max_rate)
+void clk_hw_get_boundaries(struct clk_hw *hw,
+ unsigned long *min_rate,
+ unsigned long *max_rate)
{
+ struct clk_core *core = hw->core;
struct clk *clk_user;
*min_rate = core->min_rate;
@@ -436,6 +437,7 @@ static void clk_core_get_boundaries(struct clk_core *core,
hlist_for_each_entry(clk_user, &core->clks, clks_node)
*max_rate = min(*max_rate, clk_user->max_rate);
}
+EXPORT_SYMBOL_GPL(clk_hw_get_boundaries);
void clk_hw_set_rate_range(struct clk_hw *hw, unsigned long min_rate,
unsigned long max_rate)
@@ -894,7 +896,7 @@ unsigned long clk_hw_round_rate(struct clk_hw *hw, unsigned
long rate)
int ret;
struct clk_rate_request req;
- clk_core_get_boundaries(hw->core, &req.min_rate, &req.max_rate);
+ clk_hw_get_boundaries(hw, &req.min_rate, &req.max_rate);
req.rate = rate;
ret = clk_core_round_rate_nolock(hw->core, &req);
@@ -924,7 +926,7 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
clk_prepare_lock();
- clk_core_get_boundaries(clk->core, &req.min_rate, &req.max_rate);
+ clk_hw_get_boundaries(clk->core->hw, &req.min_rate, &req.max_rate);
req.rate = rate;
ret = clk_core_round_rate_nolock(clk->core, &req);
@@ -1353,7 +1355,7 @@ static struct clk_core *clk_calc_new_rates(struct
clk_core *core,
if (parent)
best_parent_rate = parent->rate;
- clk_core_get_boundaries(core, &min_rate, &max_rate);
+ clk_hw_get_boundaries(core->hw, &min_rate, &max_rate);
/* find the closest rate and parent clk/rate */
if (core->ops->determine_rate) {
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 5100ec1..2f10999 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -755,6 +755,8 @@ int __clk_mux_determine_rate_closest(struct clk_hw *hw,
void clk_hw_reparent(struct clk_hw *hw, struct clk_hw *new_parent);
void clk_hw_set_rate_range(struct clk_hw *hw, unsigned long min_rate,
unsigned long max_rate);
+void clk_hw_get_boundaries(struct clk_hw *hw, unsigned long *min_rate,
+ unsigned long *max_rate);
static inline void __clk_hw_set_clk(struct clk_hw *dst, struct clk_hw *src)
{
--
1.7.9.5
[PATCH 0/2] Fix clock rate in the rockchip_fractional_approximation()
Hello!
Here are two patches fixing issue in the rockchip_fractional_approximation().
rockchip_fractional_approximation() can select clock rate whose
value will violate clock limit settings (i.e. one configured with
clk_set_max_rate() and clk_set_min_rate()).
rockchip_fractional_approximation() was introduced by commit 5d890c2df900
("clk: rockchip: add special approximation to fix up fractional clk's jitter")
whose description states that setting denominator 20 times larger than
numerator will generate precise clock frequency. It's strange, because
on my custom rk3188-based board and on radxa rock I've observed strange
hardware issues. I2S, for example, sometimes doesn't setup correct
rate on external SCLK_I2S0. UART0, for example, started to receive '\0'
characters instead of valid symbols, signals on UART_RX was good.
So, I use clk_set_max_rate() to limit max value of i2s0_pre, uart0_pre,
uart1_pre, uart2_pre, uart3_pre to value of aclk_cpu_pre. That fixes
strange I2S and UART issues for me. If that make sense, than I can
send the patch. But it will logically conflict with commit 5d890c2df900
("clk: rockchip: add special approximation to fix up fractional clk's jitter").
Alexander Kochetkov (2):
clk: rename clk_core_get_boundaries() to clk_hw_get_boundaries() and
expose
clk: rockchip: limit clock rate in the
rockchip_fractional_approximation()
drivers/clk/clk.c| 14 --
drivers/clk/rockchip/clk.c |7 +++
include/linux/clk-provider.h |2 ++
3 files changed, 17 insertions(+), 6 deletions(-)
--
1.7.9.5
[PATCH 2/2] clk: rockchip: limit clock rate in the rockchip_fractional_approximation()
rockchip_fractional_approximation() can choose clock rate that can
be larger than one configured using clk_set_max_rate(). Request
to setup correct clock rate whose parent rate will be adjusted
to out of range value will fail with -EINVAL.
Fixes: commit 5d890c2df900 ("clk: rockchip: add special approximation
to fix up fractional clk's jitter").
Signed-off-by: Alexander Kochetkov
---
drivers/clk/rockchip/clk.c |7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
index 35dbd63..3c1fb0d 100644
--- a/drivers/clk/rockchip/clk.c
+++ b/drivers/clk/rockchip/clk.c
@@ -175,6 +175,7 @@ static void rockchip_fractional_approximation(struct clk_hw
*hw,
{
struct clk_fractional_divider *fd = to_clk_fd(hw);
unsigned long p_rate, p_parent_rate;
+ unsigned long min_rate = 0, max_rate = 0;
struct clk_hw *p_parent;
unsigned long scale;
@@ -182,6 +183,12 @@ static void rockchip_fractional_approximation(struct
clk_hw *hw,
if ((rate * 20 > p_rate) && (p_rate % rate != 0)) {
p_parent = clk_hw_get_parent(clk_hw_get_parent(hw));
p_parent_rate = clk_hw_get_rate(p_parent);
+ clk_hw_get_boundaries(clk_hw_get_parent(hw),
+ &min_rate, &max_rate);
+ if (p_parent_rate < min_rate)
+ p_parent_rate = min_rate;
+ if (p_parent_rate > max_rate)
+ p_parent_rate = max_rate;
*parent_rate = p_parent_rate;
}
--
1.7.9.5
Re: [PATCH] net: arc_emac: fix arc_emac_rx() error paths
> 19 дек. 2017 г., в 18:22, David Miller написал(а): > > From: Alexander Kochetkov > Date: Fri, 15 Dec 2017 20:20:06 +0300 > >> arc_emac_rx() has some issues found by code review. >> >> In case netdev_alloc_skb_ip_align() or dma_map_single() failure >> rx fifo entry will not be returned to EMAC. >> >> In case dma_map_single() failure previously allocated skb became >> lost to driver. At the same time address of newly allocated skb >> will not be provided to EMAC. >> >> Signed-off-by: Alexander Kochetkov > > This patch adds quite a few bugs, here is one which shows this is not > functionally tested: May be I don’t understand correctly, but I don’t see bug here. Wrong dma mapping usage should immediately manifest itself (kernel instability, koops and so on). The patch was tested on rk3188 and work fine for me. Also I did simulations of netdev_alloc_skb_ip_align() and dma_map_single() faults and can confirm that new error handling work. Could someone else with ARC EMAC test the patch? I would be very grateful for the help. Florian or Eric, can you test it on your hardware? >> + >> +/* unmap previosly mapped skb */ >> +dma_unmap_single(&ndev->dev, dma_unmap_addr(rx_buff, addr), >> + dma_unmap_len(rx_buff, len), DMA_FROM_DEVICE); > > And then you unmap it. "addr" is the new DMA mapping, not the old one. The old mapping should be unmapped here. It refer to old skb what contains already received packet. Because buffer doesn’t belong to EMAC anymore. That old mapping point to old skb buffer. And netif_receive_skb() will be called for old skb after that. The new mapping «addr" will be unmapped then EMAC receive new packet. Later by the code (after netif_receive_skb()) there are lines for saving «addr» value: rx_buff->skb = skb; dma_unmap_addr_set(rx_buff, addr, addr); dma_unmap_len_set(rx_buff, len, EMAC_BUFFER_SIZE); Regards, Alexander.
[PATCH v2] net: arc_emac: fix arc_emac_rx() error paths
arc_emac_rx() has some issues found by code review.
In case netdev_alloc_skb_ip_align() or dma_map_single() failure
rx fifo entry will not be returned to EMAC.
In case dma_map_single() failure previously allocated skb became
lost to driver. At the same time address of newly allocated skb
will not be provided to EMAC.
Signed-off-by: Alexander Kochetkov
---
Changes in v2:
- Rebased against stable linux-4.14.y branch
drivers/net/ethernet/arc/emac_main.c | 53 --
1 file changed, 31 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/arc/emac_main.c
b/drivers/net/ethernet/arc/emac_main.c
index 363d909..bd277b0 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -212,39 +212,48 @@ static int arc_emac_rx(struct net_device *ndev, int
budget)
continue;
}
- pktlen = info & LEN_MASK;
- stats->rx_packets++;
- stats->rx_bytes += pktlen;
- skb = rx_buff->skb;
- skb_put(skb, pktlen);
- skb->dev = ndev;
- skb->protocol = eth_type_trans(skb, ndev);
-
- dma_unmap_single(&ndev->dev, dma_unmap_addr(rx_buff, addr),
-dma_unmap_len(rx_buff, len), DMA_FROM_DEVICE);
-
- /* Prepare the BD for next cycle */
- rx_buff->skb = netdev_alloc_skb_ip_align(ndev,
-EMAC_BUFFER_SIZE);
- if (unlikely(!rx_buff->skb)) {
+ /* Prepare the BD for next cycle. netif_receive_skb()
+* only if new skb was allocated and mapped to avoid holes
+* in the RX fifo.
+*/
+ skb = netdev_alloc_skb_ip_align(ndev, EMAC_BUFFER_SIZE);
+ if (unlikely(!skb)) {
+ if (net_ratelimit())
+ netdev_err(ndev, "cannot allocate skb\n");
+ /* Return ownership to EMAC */
+ rxbd->info = cpu_to_le32(FOR_EMAC | EMAC_BUFFER_SIZE);
stats->rx_errors++;
- /* Because receive_skb is below, increment rx_dropped */
stats->rx_dropped++;
continue;
}
- /* receive_skb only if new skb was allocated to avoid holes */
- netif_receive_skb(skb);
-
- addr = dma_map_single(&ndev->dev, (void *)rx_buff->skb->data,
+ addr = dma_map_single(&ndev->dev, (void *)skb->data,
EMAC_BUFFER_SIZE, DMA_FROM_DEVICE);
if (dma_mapping_error(&ndev->dev, addr)) {
if (net_ratelimit())
- netdev_err(ndev, "cannot dma map\n");
- dev_kfree_skb(rx_buff->skb);
+ netdev_err(ndev, "cannot map dma buffer\n");
+ dev_kfree_skb(skb);
+ /* Return ownership to EMAC */
+ rxbd->info = cpu_to_le32(FOR_EMAC | EMAC_BUFFER_SIZE);
stats->rx_errors++;
+ stats->rx_dropped++;
continue;
}
+
+ /* unmap previosly mapped skb */
+ dma_unmap_single(&ndev->dev, dma_unmap_addr(rx_buff, addr),
+dma_unmap_len(rx_buff, len), DMA_FROM_DEVICE);
+
+ pktlen = info & LEN_MASK;
+ stats->rx_packets++;
+ stats->rx_bytes += pktlen;
+ skb_put(rx_buff->skb, pktlen);
+ rx_buff->skb->dev = ndev;
+ rx_buff->skb->protocol = eth_type_trans(rx_buff->skb, ndev);
+
+ netif_receive_skb(rx_buff->skb);
+
+ rx_buff->skb = skb;
dma_unmap_addr_set(rx_buff, addr, addr);
dma_unmap_len_set(rx_buff, len, EMAC_BUFFER_SIZE);
--
1.7.9.5
[PATCH v2] net: arc_emac: restart stalled EMAC
Under certain conditions EMAC stop reception of incoming packets and
continuously increment R_MISS register instead of saving data into
provided buffer. The commit implement workaround for such situation.
Then the stall detected EMAC will be restarted.
On device the stall looks like the device lost it's dynamic IP address.
ifconfig shows that interface error counter rapidly increments.
At the same time on the DHCP server we can see continues DHCP-requests
from device.
In real network stalls happen really rarely. To make them frequent the
broadcast storm[1] should be simulated. For simulation it is necessary
to make following connections:
1. connect radxarock to 1st port of switch
2. connect some PC to 2nd port of switch
3. connect two other free ports together using standard ethernet cable,
in order to make a switching loop.
After that, is necessary to make a broadcast storm. For example, running on
PC 'ping' to some IP address triggers ARP-request storm. After some
time (~10sec), EMAC on rk3188 will stall.
Observed and tested on rk3188 radxarock.
[1] https://en.wikipedia.org/wiki/Broadcast_radiation
Signed-off-by: Alexander Kochetkov
---
Changes in v2:
- Rebased against stable linux-4.14.y branch
drivers/net/ethernet/arc/emac.h |2 +
drivers/net/ethernet/arc/emac_main.c | 111 ++
2 files changed, 113 insertions(+)
diff --git a/drivers/net/ethernet/arc/emac.h b/drivers/net/ethernet/arc/emac.h
index 3c63b16..d9efbc8 100644
--- a/drivers/net/ethernet/arc/emac.h
+++ b/drivers/net/ethernet/arc/emac.h
@@ -159,6 +159,8 @@ struct arc_emac_priv {
unsigned int link;
unsigned int duplex;
unsigned int speed;
+
+ unsigned int rx_missed_errors;
};
/**
diff --git a/drivers/net/ethernet/arc/emac_main.c
b/drivers/net/ethernet/arc/emac_main.c
index 3241af1..363d909 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -26,6 +26,8 @@
#include "emac.h"
+static void arc_emac_restart(struct net_device *ndev);
+
/**
* arc_emac_tx_avail - Return the number of available slots in the tx ring.
* @priv: Pointer to ARC EMAC private data structure.
@@ -259,6 +261,53 @@ static int arc_emac_rx(struct net_device *ndev, int budget)
}
/**
+ * arc_emac_rx_miss_handle - handle R_MISS register
+ * @ndev: Pointer to the net_device structure.
+ */
+static void arc_emac_rx_miss_handle(struct net_device *ndev)
+{
+ struct arc_emac_priv *priv = netdev_priv(ndev);
+ struct net_device_stats *stats = &ndev->stats;
+ unsigned int miss;
+
+ miss = arc_reg_get(priv, R_MISS);
+ if (miss) {
+ stats->rx_errors += miss;
+ stats->rx_missed_errors += miss;
+ priv->rx_missed_errors += miss;
+ }
+}
+
+/**
+ * arc_emac_rx_stall_check - check RX stall
+ * @ndev: Pointer to the net_device structure.
+ * @budget:How many BDs requested to process on 1 call.
+ * @work_done: How many BDs processed
+ *
+ * Under certain conditions EMAC stop reception of incoming packets and
+ * continuously increment R_MISS register instead of saving data into
+ * provided buffer. This function detect that condition and restart
+ * EMAC.
+ */
+static void arc_emac_rx_stall_check(struct net_device *ndev,
+ int budget, unsigned int work_done)
+{
+ struct arc_emac_priv *priv = netdev_priv(ndev);
+ struct arc_emac_bd *rxbd;
+
+ if (work_done)
+ priv->rx_missed_errors = 0;
+
+ if (priv->rx_missed_errors && budget) {
+ rxbd = &priv->rxbd[priv->last_rx_bd];
+ if (le32_to_cpu(rxbd->info) & FOR_EMAC) {
+ arc_emac_restart(ndev);
+ priv->rx_missed_errors = 0;
+ }
+ }
+}
+
+/**
* arc_emac_poll - NAPI poll handler.
* @napi: Pointer to napi_struct structure.
* @budget:How many BDs to process on 1 call.
@@ -272,6 +321,7 @@ static int arc_emac_poll(struct napi_struct *napi, int
budget)
unsigned int work_done;
arc_emac_tx_clean(ndev);
+ arc_emac_rx_miss_handle(ndev);
work_done = arc_emac_rx(ndev, budget);
if (work_done < budget) {
@@ -279,6 +329,8 @@ static int arc_emac_poll(struct napi_struct *napi, int
budget)
arc_reg_or(priv, R_ENABLE, RXINT_MASK | TXINT_MASK);
}
+ arc_emac_rx_stall_check(ndev, budget, work_done);
+
return work_done;
}
@@ -320,6 +372,8 @@ static irqreturn_t arc_emac_intr(int irq, void
*dev_instance)
if (status & MSER_MASK) {
stats->rx_missed_errors += 0x100;
stats->rx_errors += 0x100;
+ priv->rx_missed_errors += 0x100;
+ napi_schedule(&priv->napi);
}
[PATCH] net: arc_emac: fix arc_emac_rx() error paths
arc_emac_rx() has some issues found by code review.
In case netdev_alloc_skb_ip_align() or dma_map_single() failure
rx fifo entry will not be returned to EMAC.
In case dma_map_single() failure previously allocated skb became
lost to driver. At the same time address of newly allocated skb
will not be provided to EMAC.
Signed-off-by: Alexander Kochetkov
---
drivers/net/ethernet/arc/emac_main.c | 53 --
1 file changed, 31 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/arc/emac_main.c
b/drivers/net/ethernet/arc/emac_main.c
index b2e0051..0ea57fe 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -212,39 +212,48 @@ static int arc_emac_rx(struct net_device *ndev, int
budget)
continue;
}
- pktlen = info & LEN_MASK;
- stats->rx_packets++;
- stats->rx_bytes += pktlen;
- skb = rx_buff->skb;
- skb_put(skb, pktlen);
- skb->dev = ndev;
- skb->protocol = eth_type_trans(skb, ndev);
-
- dma_unmap_single(&ndev->dev, dma_unmap_addr(rx_buff, addr),
-dma_unmap_len(rx_buff, len), DMA_FROM_DEVICE);
-
- /* Prepare the BD for next cycle */
- rx_buff->skb = netdev_alloc_skb_ip_align(ndev,
-EMAC_BUFFER_SIZE);
- if (unlikely(!rx_buff->skb)) {
+ /* Prepare the BD for next cycle. netif_receive_skb()
+* only if new skb was allocated and mapped to avoid holes
+* in the RX fifo.
+*/
+ skb = netdev_alloc_skb_ip_align(ndev, EMAC_BUFFER_SIZE);
+ if (unlikely(!skb)) {
+ if (net_ratelimit())
+ netdev_err(ndev, "cannot allocate skb\n");
+ /* Return ownership to EMAC */
+ rxbd->info = cpu_to_le32(FOR_EMAC | EMAC_BUFFER_SIZE);
stats->rx_errors++;
- /* Because receive_skb is below, increment rx_dropped */
stats->rx_dropped++;
continue;
}
- /* receive_skb only if new skb was allocated to avoid holes */
- netif_receive_skb(skb);
-
- addr = dma_map_single(&ndev->dev, (void *)rx_buff->skb->data,
+ addr = dma_map_single(&ndev->dev, (void *)skb->data,
EMAC_BUFFER_SIZE, DMA_FROM_DEVICE);
if (dma_mapping_error(&ndev->dev, addr)) {
if (net_ratelimit())
- netdev_err(ndev, "cannot dma map\n");
- dev_kfree_skb(rx_buff->skb);
+ netdev_err(ndev, "cannot map dma buffer\n");
+ dev_kfree_skb(skb);
+ /* Return ownership to EMAC */
+ rxbd->info = cpu_to_le32(FOR_EMAC | EMAC_BUFFER_SIZE);
stats->rx_errors++;
+ stats->rx_dropped++;
continue;
}
+
+ /* unmap previosly mapped skb */
+ dma_unmap_single(&ndev->dev, dma_unmap_addr(rx_buff, addr),
+dma_unmap_len(rx_buff, len), DMA_FROM_DEVICE);
+
+ pktlen = info & LEN_MASK;
+ stats->rx_packets++;
+ stats->rx_bytes += pktlen;
+ skb_put(rx_buff->skb, pktlen);
+ rx_buff->skb->dev = ndev;
+ rx_buff->skb->protocol = eth_type_trans(rx_buff->skb, ndev);
+
+ netif_receive_skb(rx_buff->skb);
+
+ rx_buff->skb = skb;
dma_unmap_addr_set(rx_buff, addr, addr);
dma_unmap_len_set(rx_buff, len, EMAC_BUFFER_SIZE);
--
1.7.9.5
[PATCH] net: arc_emac: restart stalled EMAC
Under certain conditions EMAC stop reception of incoming packets and
continuously increment R_MISS register instead of saving data into
provided buffer. The commit implement workaround for such situation.
Then the stall detected EMAC will be restarted.
On device the stall looks like the device lost it's dynamic IP address.
ifconfig shows that interface error counter rapidly increments.
At the same time on the DHCP server we can see continues DHCP-requests
from device.
In real network stalls happen really rarely. To make them frequent the
broadcast storm[1] should be simulated. For simulation it is necessary
to make following connections:
1. connect radxarock to 1st port of switch
2. connect some PC to 2nd port of switch
3. connect two other free ports together using standard ethernet cable,
in order to make a switching loop.
After that, is necessary to make a broadcast storm. For example, running on
PC 'ping' to some IP address triggers ARP-request storm. After some
time (~10sec), EMAC on rk3188 will stall.
Observed and tested on rk3188 radxarock.
[1] https://en.wikipedia.org/wiki/Broadcast_radiation
Signed-off-by: Alexander Kochetkov
---
drivers/net/ethernet/arc/emac.h |2 +
drivers/net/ethernet/arc/emac_main.c | 111 ++
2 files changed, 113 insertions(+)
diff --git a/drivers/net/ethernet/arc/emac.h b/drivers/net/ethernet/arc/emac.h
index e4feb71..1c7afc5 100644
--- a/drivers/net/ethernet/arc/emac.h
+++ b/drivers/net/ethernet/arc/emac.h
@@ -158,6 +158,8 @@ struct arc_emac_priv {
unsigned int link;
unsigned int duplex;
unsigned int speed;
+
+ unsigned int rx_missed_errors;
};
/**
diff --git a/drivers/net/ethernet/arc/emac_main.c
b/drivers/net/ethernet/arc/emac_main.c
index 68de2f2..b2e0051 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -26,6 +26,8 @@
#include "emac.h"
+static void arc_emac_restart(struct net_device *ndev);
+
/**
* arc_emac_tx_avail - Return the number of available slots in the tx ring.
* @priv: Pointer to ARC EMAC private data structure.
@@ -259,6 +261,53 @@ static int arc_emac_rx(struct net_device *ndev, int budget)
}
/**
+ * arc_emac_rx_miss_handle - handle R_MISS register
+ * @ndev: Pointer to the net_device structure.
+ */
+static void arc_emac_rx_miss_handle(struct net_device *ndev)
+{
+ struct arc_emac_priv *priv = netdev_priv(ndev);
+ struct net_device_stats *stats = &ndev->stats;
+ unsigned int miss;
+
+ miss = arc_reg_get(priv, R_MISS);
+ if (miss) {
+ stats->rx_errors += miss;
+ stats->rx_missed_errors += miss;
+ priv->rx_missed_errors += miss;
+ }
+}
+
+/**
+ * arc_emac_rx_stall_check - check RX stall
+ * @ndev: Pointer to the net_device structure.
+ * @budget:How many BDs requested to process on 1 call.
+ * @work_done: How many BDs processed
+ *
+ * Under certain conditions EMAC stop reception of incoming packets and
+ * continuously increment R_MISS register instead of saving data into
+ * provided buffer. This function detect that condition and restart
+ * EMAC.
+ */
+static void arc_emac_rx_stall_check(struct net_device *ndev,
+ int budget, unsigned int work_done)
+{
+ struct arc_emac_priv *priv = netdev_priv(ndev);
+ struct arc_emac_bd *rxbd;
+
+ if (work_done)
+ priv->rx_missed_errors = 0;
+
+ if (priv->rx_missed_errors && budget) {
+ rxbd = &priv->rxbd[priv->last_rx_bd];
+ if (le32_to_cpu(rxbd->info) & FOR_EMAC) {
+ arc_emac_restart(ndev);
+ priv->rx_missed_errors = 0;
+ }
+ }
+}
+
+/**
* arc_emac_poll - NAPI poll handler.
* @napi: Pointer to napi_struct structure.
* @budget:How many BDs to process on 1 call.
@@ -272,6 +321,7 @@ static int arc_emac_poll(struct napi_struct *napi, int
budget)
unsigned int work_done;
arc_emac_tx_clean(ndev);
+ arc_emac_rx_miss_handle(ndev);
work_done = arc_emac_rx(ndev, budget);
if (work_done < budget) {
@@ -279,6 +329,8 @@ static int arc_emac_poll(struct napi_struct *napi, int
budget)
arc_reg_or(priv, R_ENABLE, RXINT_MASK | TXINT_MASK);
}
+ arc_emac_rx_stall_check(ndev, budget, work_done);
+
return work_done;
}
@@ -320,6 +372,8 @@ static irqreturn_t arc_emac_intr(int irq, void
*dev_instance)
if (status & MSER_MASK) {
stats->rx_missed_errors += 0x100;
stats->rx_errors += 0x100;
+ priv->rx_missed_errors += 0x100;
+ napi_schedule(&priv->napi);
}
if (status & RXCR_MASK) {
@@
Re: [PATCH v2 1/2] dmaengine: pl330: fix descriptor allocation fail
Hello Vinod!
It looks like the patch did not get enough attention. If no one except me will
test the patch,
how do we proceed? This patch does not depend on the SOC implementation and the
runtime environment. For me the patch looks fine and will not do harm if it
will be committed.
Regards,
Alexander.
> 4 окт. 2017 г., в 14:37, Alexander Kochetkov написал(а):
>
> If two concurrent threads call pl330_get_desc() when DMAC descriptor
> pool is empty it is possible that allocation for one of threads will fail
> with message:
>
> kernel: dma-pl330 20078000.dma-controller: pl330_get_desc:2469 ALERT!
>
> Here how that can happen. Thread A calls pl330_get_desc() to get
> descriptor. If DMAC descriptor pool is empty pl330_get_desc() allocates
> new descriptor on shared pool using add_desc() and then get newly
> allocated descriptor using pluck_desc(). At the same time thread B calls
> pluck_desc() and take newly allocated descriptor. In that case descriptor
> allocation for thread A will fail.
>
> Using on-stack pool for new descriptor allow avoid the issue described.
> The patch modify pl330_get_desc() to use on-stack pool for allocation
> new descriptors.
>
> Signed-off-by: Alexander Kochetkov
> ---
> drivers/dma/pl330.c | 39 ---
> 1 file changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index b19ee04..deec4a4 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2390,7 +2390,8 @@ static inline void _init_desc(struct dma_pl330_desc
> *desc)
> }
>
> /* Returns the number of descriptors added to the DMAC pool */
> -static int add_desc(struct pl330_dmac *pl330, gfp_t flg, int count)
> +static int add_desc(struct list_head *pool, spinlock_t *lock,
> + gfp_t flg, int count)
> {
> struct dma_pl330_desc *desc;
> unsigned long flags;
> @@ -2400,27 +2401,28 @@ static int add_desc(struct pl330_dmac *pl330, gfp_t
> flg, int count)
> if (!desc)
> return 0;
>
> - spin_lock_irqsave(&pl330->pool_lock, flags);
> + spin_lock_irqsave(lock, flags);
>
> for (i = 0; i < count; i++) {
> _init_desc(&desc[i]);
> - list_add_tail(&desc[i].node, &pl330->desc_pool);
> + list_add_tail(&desc[i].node, pool);
> }
>
> - spin_unlock_irqrestore(&pl330->pool_lock, flags);
> + spin_unlock_irqrestore(lock, flags);
>
> return count;
> }
>
> -static struct dma_pl330_desc *pluck_desc(struct pl330_dmac *pl330)
> +static struct dma_pl330_desc *pluck_desc(struct list_head *pool,
> + spinlock_t *lock)
> {
> struct dma_pl330_desc *desc = NULL;
> unsigned long flags;
>
> - spin_lock_irqsave(&pl330->pool_lock, flags);
> + spin_lock_irqsave(lock, flags);
>
> - if (!list_empty(&pl330->desc_pool)) {
> - desc = list_entry(pl330->desc_pool.next,
> + if (!list_empty(pool)) {
> + desc = list_entry(pool->next,
> struct dma_pl330_desc, node);
>
> list_del_init(&desc->node);
> @@ -2429,7 +2431,7 @@ static struct dma_pl330_desc *pluck_desc(struct
> pl330_dmac *pl330)
> desc->txd.callback = NULL;
> }
>
> - spin_unlock_irqrestore(&pl330->pool_lock, flags);
> + spin_unlock_irqrestore(lock, flags);
>
> return desc;
> }
> @@ -2441,20 +2443,18 @@ static struct dma_pl330_desc *pl330_get_desc(struct
> dma_pl330_chan *pch)
> struct dma_pl330_desc *desc;
>
> /* Pluck one desc from the pool of DMAC */
> - desc = pluck_desc(pl330);
> + desc = pluck_desc(&pl330->desc_pool, &pl330->pool_lock);
>
> /* If the DMAC pool is empty, alloc new */
> if (!desc) {
> - if (!add_desc(pl330, GFP_ATOMIC, 1))
> - return NULL;
> + DEFINE_SPINLOCK(lock);
> + LIST_HEAD(pool);
>
> - /* Try again */
> - desc = pluck_desc(pl330);
> - if (!desc) {
> - dev_err(pch->dmac->ddma.dev,
> - "%s:%d ALERT!\n", __func__, __LINE__);
> + if (!add_desc(&pool, &lock, GFP_ATOMIC, 1))
> return NULL;
> - }
> +
> + desc = pluck_desc(&pool, &lock);
> + WARN_ON(!desc || !list_empty(&pool));
> }
>
> /* Initialize the descriptor */
> @@ -2868,7 +2868,8 @@ static int __maybe_unused pl330_resume(struct device
> *dev)
> spin_lock_init(&pl330->pool_lock);
>
> /* Create a descriptor pool of default size */
> - if (!add_desc(pl330, GFP_KERNEL, NR_DEFAULT_DESC))
> + if (!add_desc(&pl330->desc_pool, &pl330->pool_lock,
> + GFP_KERNEL, NR_DEFAULT_DESC))
> dev_warn(&adev->dev, "unable to allocate desc\n");
>
> INIT_LIST_HEAD(&pd->channels);
> --
> 1.7.9.5
>
[PATCH v2 1/2] dmaengine: pl330: fix descriptor allocation fail
If two concurrent threads call pl330_get_desc() when DMAC descriptor
pool is empty it is possible that allocation for one of threads will fail
with message:
kernel: dma-pl330 20078000.dma-controller: pl330_get_desc:2469 ALERT!
Here how that can happen. Thread A calls pl330_get_desc() to get
descriptor. If DMAC descriptor pool is empty pl330_get_desc() allocates
new descriptor on shared pool using add_desc() and then get newly
allocated descriptor using pluck_desc(). At the same time thread B calls
pluck_desc() and take newly allocated descriptor. In that case descriptor
allocation for thread A will fail.
Using on-stack pool for new descriptor allow avoid the issue described.
The patch modify pl330_get_desc() to use on-stack pool for allocation
new descriptors.
Signed-off-by: Alexander Kochetkov
---
drivers/dma/pl330.c | 39 ---
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index b19ee04..deec4a4 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2390,7 +2390,8 @@ static inline void _init_desc(struct dma_pl330_desc *desc)
}
/* Returns the number of descriptors added to the DMAC pool */
-static int add_desc(struct pl330_dmac *pl330, gfp_t flg, int count)
+static int add_desc(struct list_head *pool, spinlock_t *lock,
+ gfp_t flg, int count)
{
struct dma_pl330_desc *desc;
unsigned long flags;
@@ -2400,27 +2401,28 @@ static int add_desc(struct pl330_dmac *pl330, gfp_t
flg, int count)
if (!desc)
return 0;
- spin_lock_irqsave(&pl330->pool_lock, flags);
+ spin_lock_irqsave(lock, flags);
for (i = 0; i < count; i++) {
_init_desc(&desc[i]);
- list_add_tail(&desc[i].node, &pl330->desc_pool);
+ list_add_tail(&desc[i].node, pool);
}
- spin_unlock_irqrestore(&pl330->pool_lock, flags);
+ spin_unlock_irqrestore(lock, flags);
return count;
}
-static struct dma_pl330_desc *pluck_desc(struct pl330_dmac *pl330)
+static struct dma_pl330_desc *pluck_desc(struct list_head *pool,
+spinlock_t *lock)
{
struct dma_pl330_desc *desc = NULL;
unsigned long flags;
- spin_lock_irqsave(&pl330->pool_lock, flags);
+ spin_lock_irqsave(lock, flags);
- if (!list_empty(&pl330->desc_pool)) {
- desc = list_entry(pl330->desc_pool.next,
+ if (!list_empty(pool)) {
+ desc = list_entry(pool->next,
struct dma_pl330_desc, node);
list_del_init(&desc->node);
@@ -2429,7 +2431,7 @@ static struct dma_pl330_desc *pluck_desc(struct
pl330_dmac *pl330)
desc->txd.callback = NULL;
}
- spin_unlock_irqrestore(&pl330->pool_lock, flags);
+ spin_unlock_irqrestore(lock, flags);
return desc;
}
@@ -2441,20 +2443,18 @@ static struct dma_pl330_desc *pl330_get_desc(struct
dma_pl330_chan *pch)
struct dma_pl330_desc *desc;
/* Pluck one desc from the pool of DMAC */
- desc = pluck_desc(pl330);
+ desc = pluck_desc(&pl330->desc_pool, &pl330->pool_lock);
/* If the DMAC pool is empty, alloc new */
if (!desc) {
- if (!add_desc(pl330, GFP_ATOMIC, 1))
- return NULL;
+ DEFINE_SPINLOCK(lock);
+ LIST_HEAD(pool);
- /* Try again */
- desc = pluck_desc(pl330);
- if (!desc) {
- dev_err(pch->dmac->ddma.dev,
- "%s:%d ALERT!\n", __func__, __LINE__);
+ if (!add_desc(&pool, &lock, GFP_ATOMIC, 1))
return NULL;
- }
+
+ desc = pluck_desc(&pool, &lock);
+ WARN_ON(!desc || !list_empty(&pool));
}
/* Initialize the descriptor */
@@ -2868,7 +2868,8 @@ static int __maybe_unused pl330_resume(struct device *dev)
spin_lock_init(&pl330->pool_lock);
/* Create a descriptor pool of default size */
- if (!add_desc(pl330, GFP_KERNEL, NR_DEFAULT_DESC))
+ if (!add_desc(&pl330->desc_pool, &pl330->pool_lock,
+ GFP_KERNEL, NR_DEFAULT_DESC))
dev_warn(&adev->dev, "unable to allocate desc\n");
INIT_LIST_HEAD(&pd->channels);
--
1.7.9.5
[PATCH v2 2/2] !!! FOR TESTING ONLY !!! dmaengine: pl330: add verbose message and set NR_DEFAULT_DESC to 1
Commit add verbose output to pl330 showing what changes introduced by
commit 1/2 from series work as expected. You should see similar output
running modified kernel:
The patch tested on rk3188 radxdarock. Could someone else test it on
other hardware with pl330 DMA?
root@host:~# dmesg | grep pl330
[0.277520] dma-pl330 20018000.dma-controller: Loaded driver for PL330
DMAC-241330
[0.277538] dma-pl330 20018000.dma-controller: DBUFF-32x8bytes
Num_Chans-6 Num_Peri-12 Num_Events-12
[0.279894] dma-pl330 20078000.dma-controller: Loaded driver for PL330
DMAC-241330
[0.279910] dma-pl330 20078000.dma-controller: DBUFF-64x8bytes
Num_Chans-7 Num_Peri-20 Num_Events-14
[1.344804] dma-pl330 20078000.dma-controller: pl330_get_desc:2458 Allocated
one more descriptor
[1.344832] dma-pl330 20078000.dma-controller: pl330_get_desc:2458 Allocated
one more descriptor
[1.344853] dma-pl330 20078000.dma-controller: pl330_get_desc:2458 Allocated
one more descriptor
[1.344873] dma-pl330 20078000.dma-controller: pl330_get_desc:2458 Allocated
one more descriptor
[1.344893] dma-pl330 20078000.dma-controller: pl330_get_desc:2458 Allocated
one more descriptor
[1.344912] dma-pl330 20078000.dma-controller: pl330_get_desc:2458 Allocated
one more descriptor
--- rest of similar lines omitted ---
Signed-off-by: Alexander Kochetkov
---
drivers/dma/pl330.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index deec4a4..3441c16 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -266,7 +266,7 @@ enum pl330_byteswap {
/* The number of default descriptors */
-#define NR_DEFAULT_DESC16
+#define NR_DEFAULT_DESC1
/* Delay for runtime PM autosuspend, ms */
#define PL330_AUTOSUSPEND_DELAY 20
@@ -2455,6 +2455,9 @@ static struct dma_pl330_desc *pl330_get_desc(struct
dma_pl330_chan *pch)
desc = pluck_desc(&pool, &lock);
WARN_ON(!desc || !list_empty(&pool));
+
+ dev_err(pch->dmac->ddma.dev, "%s:%d Allocated one more
descriptor\n",
+ __func__, __LINE__);
}
/* Initialize the descriptor */
--
1.7.9.5
[PATCH v2 0/2] dmaengine: pl330: fix descriptor allocation fail
Here is the patch fixing descriptor allocation issue. Could someone with pl330 hardware test it and confirm that it doesn't brake current pl330 driver? I will be very grateful to you! Changes in v2: - removed wrappers add_desc(), pluck_desc() - fix code intendation Alexander Kochetkov (2): dmaengine: pl330: fix descriptor allocation fail !!! FOR TESTING ONLY !!! dmaengine: pl330: add verbose message and set NR_DEFAULT_DESC to 1 drivers/dma/pl330.c | 44 1 file changed, 24 insertions(+), 20 deletions(-) -- 1.7.9.5
Re: [PATCH] dmaengine: pl330: fix descriptor allocation fail
Hello Vinod! Thanks for review! > 26 сент. 2017 г., в 20:37, Vinod Koul написал(а): > > Tested-by please... In order to test the patch the driver should be rebuild with NR_DEFAULT_DESC defined to 1 and with some trace code included. Is it OK if I provide second patch I used for testing with trace showing how change work? > one more wrapper why, we dont have any logic here! The idea was to keep rest of driver code intact. Ok, I’ll send v2 with no wrappers. > right justifed please Some functions has two tabs on second line, some has alignment to beginning of argument declaration. How correct? 1) or like this (two tabs) static int add_desc(struct list_head *pool, spinlock_t *lock, gfp_t flg, int count) 2) Like this: static int add_desc(struct list_head *pool, spinlock_t *lock, gfp_t flg, int count) Regards, Alexander.
[PATCH] dmaengine: pl330: fix descriptor allocation fail
Thread A calls pl330_get_desc() to get descriptor. If DMAC descriptor
pool is empty pl330_get_desc() allocates new descriptor using add_desc()
and then get newly allocated descriptor using pluck_desc().
It is possible that another concurrent thread B calls pluck_desc()
and catch newly allocated descriptor. In that case descriptor allocation
for thread A will fail with:
kernel: dma-pl330 20078000.dma-controller: pl330_get_desc:2469 ALERT!
The commit fix the issue by calling _add_desc() to allocate new descriptor
to the local on stack pool and than get it from local pool. So the issue
described will nether happen.
Signed-off-by: Alexander Kochetkov
---
drivers/dma/pl330.c | 44 +++-
1 file changed, 27 insertions(+), 17 deletions(-)
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index f37f497..0e7f6c9 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2417,7 +2417,8 @@ static inline void _init_desc(struct dma_pl330_desc *desc)
}
/* Returns the number of descriptors added to the DMAC pool */
-static int add_desc(struct pl330_dmac *pl330, gfp_t flg, int count)
+static int _add_desc(struct list_head *pool, spinlock_t *lock,
+ gfp_t flg, int count)
{
struct dma_pl330_desc *desc;
unsigned long flags;
@@ -2427,27 +2428,33 @@ static int add_desc(struct pl330_dmac *pl330, gfp_t
flg, int count)
if (!desc)
return 0;
- spin_lock_irqsave(&pl330->pool_lock, flags);
+ spin_lock_irqsave(lock, flags);
for (i = 0; i < count; i++) {
_init_desc(&desc[i]);
- list_add_tail(&desc[i].node, &pl330->desc_pool);
+ list_add_tail(&desc[i].node, pool);
}
- spin_unlock_irqrestore(&pl330->pool_lock, flags);
+ spin_unlock_irqrestore(lock, flags);
return count;
}
-static struct dma_pl330_desc *pluck_desc(struct pl330_dmac *pl330)
+static int add_desc(struct pl330_dmac *pl330, gfp_t flg, int count)
+{
+ return _add_desc(&pl330->desc_pool, &pl330->pool_lock, flg, count);
+}
+
+static struct dma_pl330_desc *_pluck_desc(struct list_head *pool,
+ spinlock_t *lock)
{
struct dma_pl330_desc *desc = NULL;
unsigned long flags;
- spin_lock_irqsave(&pl330->pool_lock, flags);
+ spin_lock_irqsave(lock, flags);
- if (!list_empty(&pl330->desc_pool)) {
- desc = list_entry(pl330->desc_pool.next,
+ if (!list_empty(pool)) {
+ desc = list_entry(pool->next,
struct dma_pl330_desc, node);
list_del_init(&desc->node);
@@ -2456,11 +2463,16 @@ static struct dma_pl330_desc *pluck_desc(struct
pl330_dmac *pl330)
desc->txd.callback = NULL;
}
- spin_unlock_irqrestore(&pl330->pool_lock, flags);
+ spin_unlock_irqrestore(lock, flags);
return desc;
}
+static struct dma_pl330_desc *pluck_desc(struct pl330_dmac *pl330)
+{
+ return _pluck_desc(&pl330->desc_pool, &pl330->pool_lock);
+}
+
static struct dma_pl330_desc *pl330_get_desc(struct dma_pl330_chan *pch)
{
struct pl330_dmac *pl330 = pch->dmac;
@@ -2472,16 +2484,14 @@ static struct dma_pl330_desc *pl330_get_desc(struct
dma_pl330_chan *pch)
/* If the DMAC pool is empty, alloc new */
if (!desc) {
- if (!add_desc(pl330, GFP_ATOMIC, 1))
- return NULL;
+ DEFINE_SPINLOCK(lock);
+ LIST_HEAD(pool);
- /* Try again */
- desc = pluck_desc(pl330);
- if (!desc) {
- dev_err(pch->dmac->ddma.dev,
- "%s:%d ALERT!\n", __func__, __LINE__);
+ if (!_add_desc(&pool, &lock, GFP_ATOMIC, 1))
return NULL;
- }
+
+ desc = _pluck_desc(&pool, &lock);
+ WARN_ON(!desc || !list_empty(&pool));
}
/* Initialize the descriptor */
--
1.7.9.5
Re: [PATCH 0/2] Free running cyclic transfer implementation for pl330
Hello! Just to let you know, that I got following test report from Stephen (thanks a lot!). The patch won’t work due to full cyclic transfer doesn’t fit into mcbufsz (256 bytes long). His application requested driver to do cyclic transfer with large number of cycles. pl330 microcode has restriction on how far PC can change and this limit is 256 bytes, so increasing mcbufsz will not solve problem. Don’t want to make jump bridges or something like on the fly CPU modified microcode. > 14 апр. 2017 г., в 23:24, Stephen Barber написал(а): > > Hi Alexander, > > Thanks for your patches! > > I gave them a try on kevin (Samsung Chromebook Plus) with our > chromeos-4.4 kernel branch, plus some cherry-picks on top to get > things to apply cleanly. Here's what my tree looks like: > > 344daf13fa4e (HEAD -> apply-pl330) dmaengine: pl330: don't emit code > for one iteration loop > 062596b83fec dmaengine: pl330: make cyclic transfer free runnable > 0e75f2647b8e dmaengine: pl330: fix double lock > 7d22b54b79f2 dmaengine: pl330: remove unused ‘regs’ > 8769d7115cec dmaengine: pl330: do not generate unaligned access > 65ad077f685b dmaengine: pl330: convert callback to helper function > 368e7aa6dffd dmaengine: add support to provide error result from a DMA > transation > 8f8afe84472f dmaengine: Add helper function to prep for error reporting > 2acc1e704232 dmaengine: pl330: explicitly freeup irq > ed36cde14cf0 (m/master, cros/chromeos-4.4) UPSTREAM: audit: add tty > field to LOGIN event > > Unfortunately when I start playing audio, things don't seem to work :( > Rolling back HEAD to "dmaengine: pl330: fix double lock" works though. > > The only thing I get from dmesg relevant to pl330 is this: > [ 59.203375] dma-pl330 ff6d.dma-controller: > pl330_submit_req:1498 Try increasing mcbufsz (12810/256) > [ 59.203395] dma-pl330 ff6d.dma-controller: fill_queue:2023 Bad Desc(2) > [ 63.837390] dma-pl330 ff6d.dma-controller: > pl330_submit_req:1498 Try increasing mcbufsz (12810/256) > [ 63.837410] dma-pl330 ff6d.dma-controller: fill_queue:2023 Bad Desc(2) > > Thanks, > Steve > > 14 апр. 2017 г., в 21:04, Krzysztof Kozlowski написал(а): > > Let me know if you need more data. I wonder why you haven't experience > this? I don’t have idea why this might happen on Odroid and due to the fact the patch don’t work for Stephen and I don’t have another idea how to implement that, it is better to leave the problem along. Krzysztof, thanks a lot for help. Really. Regards, Alexander.
Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt
Hello David!
> 25 апр. 2017 г., в 17:36, David Miller написал(а):
>
> So... what are we doing here?
>
> My understanding is that this should fix the same problem that commit
> 99f81afc139c6edd14d77a91ee91685a414a1c66 ("phy: micrel: Disable auto
> negotiation on startup") fixed and that this micrel commit should thus
> be reverted to improve MAC startup times which regressed.
>
> Florian, any guidance?
Yes, this should be done.
I aksed Alexandre to check if 99f81afc139c6edd14d77a91ee91685a414a1c66 ("phy:
micrel: Disable auto
negotiation on startup») can be reverted, and he answered what it may do that
sometime this/next week.
Alexander.
Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt
> 21 апр. 2017 г., в 17:18, Roger Quadros написал(а):
>
> I think the following commit broke functionality with interrupt driven PHYs
> 3c293f4e08b5 ("net: phy: Trigger state machine on state change and not
> polling.")
Probably this one[1] broke, according to Alexandre’s commit[2].
And it was since Nov 16 2015. But it was hidden by some other commits.
For Roger problem became visible after 3c293f4e08b5 ("net: phy:
Trigger state machine on state change and not polling.»),
For my problem became visible after 529ed1275263 ("net: phy: phy drivers
should not set SUPPORTED_[Asym_]Pause»). As commit 529ed1275263
removed SUPPORTED_Pause flag from PHY advertising property and
genphy_config_aneg() began to skip PHY auto-negotiation.
Alexander.
[1] Fixes: 321beec5047a (net: phy: Use interrupts when available in NOLINK
state)
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=321beec5047af83db90c88114b7e664b156f49fe
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99f81afc139c6edd14d77a91ee91685a414a1c66
Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt
Hi, Alexandre!
Just found that you've fixed similar problem for Micrel PHY:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99f81afc139c6edd14d77a91ee91685a414a1c66
Could you please test if my patch[1] fix your problem?
As reverting your patch will speedup MAC startup time for boards with Micrel PHY
on 3~5 sec (auto-negotiation time[2]). Could you check that also?
Regards,
Alexander.
[1] https://lkml.org/lkml/2017/4/20/357
[2] http://www.ieee802.org/3/af/public/jan02/brown_1_0102.pdf
> 20 апр. 2017 г., в 14:00, Alexander Kochetkov
> написал(а):
>
> The Ethernet link on an interrupt driven PHY was not coming up if the Ethernet
> cable was plugged before the Ethernet interface was brought up.
>
> The patch trigger PHY state machine to update link state if PHY was requested
> to
> do auto-negotiation and auto-negotiation complete flag already set.
>
> During power-up cycle the PHY do auto-negotiation, generate interrupt and set
> auto-negotiation complete flag. Interrupt is handled by PHY state machine but
> doesn't update link state because PHY is in PHY_READY state. After some time
> MAC bring up, start and request PHY to do auto-negotiation. If there are no
> new
> settings to advertise genphy_config_aneg() doesn't start PHY auto-negotiation.
> PHY continue to stay in auto-negotiation complete state and doesn't fire
> interrupt. At the same time PHY state machine expect that PHY started
> auto-negotiation and is waiting for interrupt from PHY and it won't get it.
>
> Signed-off-by: Alexander Kochetkov
> Cc: stable # v4.9+
> ---
> drivers/net/phy/phy.c | 40
> include/linux/phy.h |1 +
> 2 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 7cc1b7d..2d9975b 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -591,16 +591,18 @@ int phy_mii_ioctl(struct phy_device *phydev, struct
> ifreq *ifr, int cmd)
> EXPORT_SYMBOL(phy_mii_ioctl);
>
> /**
> - * phy_start_aneg - start auto-negotiation for this PHY device
> + * phy_start_aneg_priv - start auto-negotiation for this PHY device
> * @phydev: the phy_device struct
> + * @sync: indicate whether we should wait for the workqueue cancelation
> *
> * Description: Sanitizes the settings (if we're not autonegotiating
> * them), and then calls the driver's config_aneg function.
> * If the PHYCONTROL Layer is operating, we change the state to
> * reflect the beginning of Auto-negotiation or forcing.
> */
> -int phy_start_aneg(struct phy_device *phydev)
> +static int phy_start_aneg_priv(struct phy_device *phydev, bool sync)
> {
> + bool trigger = 0;
> int err;
>
> mutex_lock(&phydev->lock);
> @@ -625,10 +627,40 @@ int phy_start_aneg(struct phy_device *phydev)
> }
> }
>
> + /* Re-schedule a PHY state machine to check PHY status because
> + * negotiation may already be done and aneg interrupt may not be
> + * generated.
> + */
> + if (phy_interrupt_is_valid(phydev) && (phydev->state == PHY_AN)) {
> + err = phy_aneg_done(phydev);
> + if (err > 0) {
> + trigger = true;
> + err = 0;
> + }
> + }
> +
> out_unlock:
> mutex_unlock(&phydev->lock);
> +
> + if (trigger)
> + phy_trigger_machine(phydev, sync);
> +
> return err;
> }
> +
> +/**
> + * phy_start_aneg - start auto-negotiation for this PHY device
> + * @phydev: the phy_device struct
> + *
> + * Description: Sanitizes the settings (if we're not autonegotiating
> + * them), and then calls the driver's config_aneg function.
> + * If the PHYCONTROL Layer is operating, we change the state to
> + * reflect the beginning of Auto-negotiation or forcing.
> + */
> +int phy_start_aneg(struct phy_device *phydev)
> +{
> + return phy_start_aneg_priv(phydev, true);
> +}
> EXPORT_SYMBOL(phy_start_aneg);
>
> /**
> @@ -656,7 +688,7 @@ void phy_start_machine(struct phy_device *phydev)
> * state machine runs.
> */
>
> -static void phy_trigger_machine(struct phy_device *phydev, bool sync)
> +void phy_trigger_machine(struct phy_device *phydev, bool sync)
> {
> if (sync)
> cancel_delayed_work_sync(&phydev->state_queue);
> @@ -1151,7 +1183,7 @@ void phy_state_machine(struct work_struct *work)
> mutex_unlock(&phydev->lock);
>
> if (needs_aneg)
> - err = phy_start_aneg(phydev);
> + err = phy_start_aneg_priv(ph
Re: [PATCH] mmc: dw_mmc: hide clock message when card is resuming
> 20 апр. 2017 г., в 13:06, Jaehoon Chung написал(а):
>
> I think you are not using the latest kernel. which version do you use?
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ce69e2fea093b7fa3991c87849c4955cd47796c9
>
> Could you check this?
Commit ce69e2fea093 («mmc: dw_mmc: silent verbose log when calling from PM
context») fix
issue I have. So my patch not needed anymore.
I use 4.10.10, probably ce69e2fea093b7fa3991c87849c4955cd47796c9 should be
backported to 4.10 branch.
4.9 branch doesn’t have commit e9748e0364fe ("mmc: dw_mmc: force setup bus if
active slots exist») what
cause the problem.
I’ll send request to stable.
Regards,
Alexander.
[PATCH v2] net: arc_emac: switch to phy_start()/phy_stop()
Currently driver use phy_start_aneg() in arc_emac_open() to bring up PHY. But phy_start() function is more appropriate for this purposes. Besides that it call phy_start_aneg() as part of PHY startup sequence it also can correctly bring up PHY from error and suspended states. So the patch replace phy_start_aneg() to phy_start(). Also the patch add call to phy_stop() to arc_emac_stop() to allow the PHY device to be fully suspended when the interface is unused. Signed-off-by: Alexander Kochetkov --- Changes in v2: - Updated commit message to clarify changes drivers/net/ethernet/arc/emac_main.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/arc/emac_main.c b/drivers/net/ethernet/arc/emac_main.c index abc9f2a..188676d 100644 --- a/drivers/net/ethernet/arc/emac_main.c +++ b/drivers/net/ethernet/arc/emac_main.c @@ -434,7 +434,7 @@ static int arc_emac_open(struct net_device *ndev) /* Enable EMAC */ arc_reg_or(priv, R_CTRL, EN_MASK); - phy_start_aneg(ndev->phydev); + phy_start(ndev->phydev); netif_start_queue(ndev); @@ -556,6 +556,8 @@ static int arc_emac_stop(struct net_device *ndev) napi_disable(&priv->napi); netif_stop_queue(ndev); + phy_stop(ndev->phydev); + /* Disable interrupts */ arc_reg_clr(priv, R_ENABLE, RXINT_MASK | TXINT_MASK | ERR_MASK); -- 1.7.9.5
[PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt
The Ethernet link on an interrupt driven PHY was not coming up if the Ethernet
cable was plugged before the Ethernet interface was brought up.
The patch trigger PHY state machine to update link state if PHY was requested to
do auto-negotiation and auto-negotiation complete flag already set.
During power-up cycle the PHY do auto-negotiation, generate interrupt and set
auto-negotiation complete flag. Interrupt is handled by PHY state machine but
doesn't update link state because PHY is in PHY_READY state. After some time
MAC bring up, start and request PHY to do auto-negotiation. If there are no new
settings to advertise genphy_config_aneg() doesn't start PHY auto-negotiation.
PHY continue to stay in auto-negotiation complete state and doesn't fire
interrupt. At the same time PHY state machine expect that PHY started
auto-negotiation and is waiting for interrupt from PHY and it won't get it.
Signed-off-by: Alexander Kochetkov
Cc: stable # v4.9+
---
drivers/net/phy/phy.c | 40
include/linux/phy.h |1 +
2 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 7cc1b7d..2d9975b 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -591,16 +591,18 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq
*ifr, int cmd)
EXPORT_SYMBOL(phy_mii_ioctl);
/**
- * phy_start_aneg - start auto-negotiation for this PHY device
+ * phy_start_aneg_priv - start auto-negotiation for this PHY device
* @phydev: the phy_device struct
+ * @sync: indicate whether we should wait for the workqueue cancelation
*
* Description: Sanitizes the settings (if we're not autonegotiating
* them), and then calls the driver's config_aneg function.
* If the PHYCONTROL Layer is operating, we change the state to
* reflect the beginning of Auto-negotiation or forcing.
*/
-int phy_start_aneg(struct phy_device *phydev)
+static int phy_start_aneg_priv(struct phy_device *phydev, bool sync)
{
+ bool trigger = 0;
int err;
mutex_lock(&phydev->lock);
@@ -625,10 +627,40 @@ int phy_start_aneg(struct phy_device *phydev)
}
}
+ /* Re-schedule a PHY state machine to check PHY status because
+* negotiation may already be done and aneg interrupt may not be
+* generated.
+*/
+ if (phy_interrupt_is_valid(phydev) && (phydev->state == PHY_AN)) {
+ err = phy_aneg_done(phydev);
+ if (err > 0) {
+ trigger = true;
+ err = 0;
+ }
+ }
+
out_unlock:
mutex_unlock(&phydev->lock);
+
+ if (trigger)
+ phy_trigger_machine(phydev, sync);
+
return err;
}
+
+/**
+ * phy_start_aneg - start auto-negotiation for this PHY device
+ * @phydev: the phy_device struct
+ *
+ * Description: Sanitizes the settings (if we're not autonegotiating
+ * them), and then calls the driver's config_aneg function.
+ * If the PHYCONTROL Layer is operating, we change the state to
+ * reflect the beginning of Auto-negotiation or forcing.
+ */
+int phy_start_aneg(struct phy_device *phydev)
+{
+ return phy_start_aneg_priv(phydev, true);
+}
EXPORT_SYMBOL(phy_start_aneg);
/**
@@ -656,7 +688,7 @@ void phy_start_machine(struct phy_device *phydev)
* state machine runs.
*/
-static void phy_trigger_machine(struct phy_device *phydev, bool sync)
+void phy_trigger_machine(struct phy_device *phydev, bool sync)
{
if (sync)
cancel_delayed_work_sync(&phydev->state_queue);
@@ -1151,7 +1183,7 @@ void phy_state_machine(struct work_struct *work)
mutex_unlock(&phydev->lock);
if (needs_aneg)
- err = phy_start_aneg(phydev);
+ err = phy_start_aneg_priv(phydev, false);
else if (do_suspend)
phy_suspend(phydev);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7fc1105..b19ae66 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -840,6 +840,7 @@ int phy_drivers_register(struct phy_driver *new_driver, int
n,
void phy_mac_interrupt(struct phy_device *phydev, int new_link);
void phy_start_machine(struct phy_device *phydev);
void phy_stop_machine(struct phy_device *phydev);
+void phy_trigger_machine(struct phy_device *phydev, bool sync);
int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd);
int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
int phy_ethtool_ksettings_get(struct phy_device *phydev,
--
1.7.9.5
[PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt
Hello Florian, Roger and Madalin! This is slightly modified version of previous patch[1]. It take into account that phy_start_aneg() may be called from differend places not only from PHY state machine. So the patch use phy_trigger_machine() to schedule link state update correctly. I borrow some text from Roger's commit message. Added 'Cc: stable tag' because the problem exist on 4.9 and 4.10 branches. Don't know what commit brake the code, so I don't add Fixes tag. Roger, could you please test this one patch? Madalin, I found, that you tested Roger’s patch[2] and this one patch will get upstream instead of patch[2]. Could you please test it? Regards, Alexander. [1] http://patchwork.ozlabs.org/patch/743773/ [2] https://lkml.org/lkml/2017/3/30/517 Alexander Kochetkov (1): net: phy: fix auto-negotiation stall due to unavailable interrupt drivers/net/phy/phy.c | 40 include/linux/phy.h |1 + 2 files changed, 37 insertions(+), 4 deletions(-) -- 1.7.9.5
Re: [PATCH] net: arc_emac: switch to phy_start()/phy_stop()
> 20 апр. 2017 г., в 0:08, Florian Fainelli написал(а): > > This looks fine. If you wanted to go further, you could move the > phy_connect(), phy_disconnect() calls down to the arc_emac_open() > respectively arc_emac_stop() as this would also allow the PHY device to > be fully suspended when the interface is unused. I’ve checked patch phy_connect() is called from arc_emac_open() and phy_disconnect() is called from arc_emac_stop(). So, I’ve made mistake in the commit message. Thank you for review. > > 19 апр. 2017 г., в 21:22, Sergei Shtylyov > написал(а): > > On 04/19/2017 05:29 PM, Alexander Kochetkov wrote: > >> The patch replace phy_start_aneg() with phy_start(). phy_start() call > > Replaces. > >> phy_start_aneg() as a part of startup sequence and allow recover from >> error (PHY_HALTED) state. >> >> Also added call phy_stop() to arc_emac_remove() to stop PHY state machine > > To arc_emac_stop() maybe? > Sergei, thanks for spell and gramma checking. Regards, Alexander.
Re: [PATCH] net: phy: fix auto-negotiation stall due to unavailable interrupt
> 19 апр. 2017 г., в 19:32, Florian Fainelli написал(а):
>
> http://patchwork.ozlabs.org/patch/743773/
>
> Roger can you also test Alexander's patch?
If MAC use phy_start_aneg() instead of phy_start() my patch will not work as
expected. Roger, if patch don’t work for you please check what MAC bring up PHY
using
phy_start():
http://patchwork.ozlabs.org/patch/752308/
Is it correct to start PHY inside MAC probe using phy_start_aneg()? Or
phy_start() must be used?
And probably this tags should be added for my patch:
Fixes: 3c293f4e08b5 ("net: phy: Trigger state machine on state change and not
polling.")
Cc: stable # v4.9+
Because I bisected to commit 529ed1275263 ("net: phy: phy drivers
should not set SUPPORTED_[Asym_]Pause») that looks pretty good.
Also, there is another issue I found. link_timeout doesn’t work for interrupt
driven PHY.
It is possible to implement timer to handle this case.
Florian, what do you think? Should this be fixed?
Alexander.
[PATCH] mmc: dw_mmc: hide clock message when card is resuming
Commit e9748e0364fe ("mmc: dw_mmc: force setup bus if active slots exist")
made a change resulted in clock message appears every time the card is
resuming (every 5 second in average). Add condition that previously used to
print the message.
Fixes: e9748e0364fe ("mmc: dw_mmc: force setup bus if active slots exist")
Signed-off-by: Alexander Kochetkov
---
drivers/mmc/host/dw_mmc.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 73db085..faaf2c1 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1178,7 +1178,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot,
bool force_clkinit)
if ((clock != slot->__clk_old &&
!test_bit(DW_MMC_CARD_NEEDS_POLL, &slot->flags)) ||
- force_clkinit) {
+ (force_clkinit &&
+ (slot->mmc->pm_flags & MMC_PM_KEEP_POWER))) {
dev_info(&slot->mmc->class_dev,
"Bus speed (slot %d) = %dHz (slot req %dHz,
actual %dHZ div = %d)\n",
slot->id, host->bus_hz, clock,
--
1.7.9.5
[PATCH] net: arc_emac: switch to phy_start()/phy_stop()
The patch replace phy_start_aneg() with phy_start(). phy_start() call phy_start_aneg() as a part of startup sequence and allow recover from error (PHY_HALTED) state. Also added call phy_stop() to arc_emac_remove() to stop PHY state machine when MAC is down. Signed-off-by: Alexander Kochetkov --- drivers/net/ethernet/arc/emac_main.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/arc/emac_main.c b/drivers/net/ethernet/arc/emac_main.c index abc9f2a..188676d 100644 --- a/drivers/net/ethernet/arc/emac_main.c +++ b/drivers/net/ethernet/arc/emac_main.c @@ -434,7 +434,7 @@ static int arc_emac_open(struct net_device *ndev) /* Enable EMAC */ arc_reg_or(priv, R_CTRL, EN_MASK); - phy_start_aneg(ndev->phydev); + phy_start(ndev->phydev); netif_start_queue(ndev); @@ -556,6 +556,8 @@ static int arc_emac_stop(struct net_device *ndev) napi_disable(&priv->napi); netif_stop_queue(ndev); + phy_stop(ndev->phydev); + /* Disable interrupts */ arc_reg_clr(priv, R_ENABLE, RXINT_MASK | TXINT_MASK | ERR_MASK); -- 1.7.9.5
Re: [PATCH] net: phy: fix auto-negotiation stall due to unavailable interrupt
Just found similar problem fixed in another PHY. See commit 99f81afc139c
("phy: micrel: Disable auto negotiation on startup»)
> 19 апр. 2017 г., в 16:46, Alexander Kochetkov
> написал(а):
>
> The problem I fix related to SMSC LAN8710/LAN8720 PHY handled using
> interrupts. During power-up cycle the PHY do auto-negotiation, generate
> interrupt and set BMSR_ANEGCOMPLETE flag. Interrupt is handled by PHY
> state machine but doesn't update link because PHY is in PHY_READY state.
> After some time MAC bring up and connect with PHY. It start PHY using
> phy_start(). During startup PHY change state to PHY_AN but doesn't
> set BMCR_ANRESTART flag due to genphy_config_aneg() doesn't update MII_BMCR
> because there no new to advertising. As a result, state machine wait for
> interrupt from PHY and nether get "link is up". Because BMSR_ANEGCOMPLETE
> already set the patch schedule check link without waiting interrupt.
> In case genphy_config_aneg() update MII_BMCR and set BMCR_ANRESTART
> flag, BMSR_ANEGCOMPLETE will be cleared and state machine will continue
> on auto-negotiation interrupt.
>
> Signed-off-by: Alexander Kochetkov
> ---
> drivers/net/phy/phy.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 7cc1b7d..da8f03d 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -1169,6 +1169,18 @@ void phy_state_machine(struct work_struct *work)
> if (phydev->irq == PHY_POLL)
> queue_delayed_work(system_power_efficient_wq,
> &phydev->state_queue,
> PHY_STATE_TIME * HZ);
> +
> + /* Re-schedule a PHY state machine to check PHY status because
> + * negotiation already done and aneg interrupt may not be generated.
> + */
> + if (needs_aneg && (phydev->irq > 0) && (phydev->state == PHY_AN)) {
> + err = phy_aneg_done(phydev);
> + if (err > 0)
> + queue_delayed_work(system_power_efficient_wq,
> +&phydev->state_queue, 0);
> + if (err < 0)
> + phy_error(phydev);
> + }
> }
>
> /**
> --
> 1.7.9.5
>
[PATCH] net: phy: fix auto-negotiation stall due to unavailable interrupt
The problem I fix related to SMSC LAN8710/LAN8720 PHY handled using
interrupts. During power-up cycle the PHY do auto-negotiation, generate
interrupt and set BMSR_ANEGCOMPLETE flag. Interrupt is handled by PHY
state machine but doesn't update link because PHY is in PHY_READY state.
After some time MAC bring up and connect with PHY. It start PHY using
phy_start(). During startup PHY change state to PHY_AN but doesn't
set BMCR_ANRESTART flag due to genphy_config_aneg() doesn't update MII_BMCR
because there no new to advertising. As a result, state machine wait for
interrupt from PHY and nether get "link is up". Because BMSR_ANEGCOMPLETE
already set the patch schedule check link without waiting interrupt.
In case genphy_config_aneg() update MII_BMCR and set BMCR_ANRESTART
flag, BMSR_ANEGCOMPLETE will be cleared and state machine will continue
on auto-negotiation interrupt.
Signed-off-by: Alexander Kochetkov
---
drivers/net/phy/phy.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 7cc1b7d..da8f03d 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1169,6 +1169,18 @@ void phy_state_machine(struct work_struct *work)
if (phydev->irq == PHY_POLL)
queue_delayed_work(system_power_efficient_wq,
&phydev->state_queue,
PHY_STATE_TIME * HZ);
+
+ /* Re-schedule a PHY state machine to check PHY status because
+* negotiation already done and aneg interrupt may not be generated.
+*/
+ if (needs_aneg && (phydev->irq > 0) && (phydev->state == PHY_AN)) {
+ err = phy_aneg_done(phydev);
+ if (err > 0)
+ queue_delayed_work(system_power_efficient_wq,
+ &phydev->state_queue, 0);
+ if (err < 0)
+ phy_error(phydev);
+ }
}
/**
--
1.7.9.5
[PATCH 2/2] dmaengine: pl330: don't emit code for one iteration loop
The patch remove one iteration outer loop in the _loop().
Removing loop saves 4 bytes of MicroCode buffer. This
savings make sense for free-running cyclic transfer
implementation.
DMALP_0 0
...
DMALPENDA_0 bjmpto_9
Signed-off-by: Alexander Kochetkov
---
drivers/dma/pl330.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 56a2377..6126dde 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -1268,7 +1268,7 @@ static inline int _loop(struct pl330_dmac *pl330,
unsigned dry_run, u8 buf[],
lpend.bjump = 0;
szlpend = _emit_LPEND(1, buf, &lpend);
- if (lcnt0) {
+ if (lcnt0 > 1) {
szlp *= 2;
szlpend *= 2;
}
@@ -1284,7 +1284,7 @@ static inline int _loop(struct pl330_dmac *pl330,
unsigned dry_run, u8 buf[],
off = 0;
- if (lcnt0) {
+ if (lcnt0 > 1) {
off += _emit_LP(dry_run, &buf[off], 0, lcnt0);
ljmp0 = off;
}
@@ -1300,7 +1300,7 @@ static inline int _loop(struct pl330_dmac *pl330,
unsigned dry_run, u8 buf[],
lpend.bjump = off - ljmp1;
off += _emit_LPEND(dry_run, &buf[off], &lpend);
- if (lcnt0) {
+ if (lcnt0 > 1) {
lpend.cond = ALWAYS;
lpend.forever = false;
lpend.loop = 0;
@@ -1309,7 +1309,7 @@ static inline int _loop(struct pl330_dmac *pl330,
unsigned dry_run, u8 buf[],
}
*bursts = lcnt1 * cyc;
- if (lcnt0)
+ if (lcnt0 > 1)
*bursts *= lcnt0;
return off;
--
1.7.9.5
[PATCH 1/2] dmaengine: pl330: make cyclic transfer free runnable
The patch solve the I2S click problem on rk3188. Actually
all the devices using pl330 should have I2S click problem
due to pl330 cyclic transfer implementation.
Current implementation depends on soft irq. If pl330 unable
to submit next transfer in time some samples could be lost.
The lost samples heard as sound click. In order to check
lost samples, I've installed I2S interrupt handler to signal
overflow/underflow conditions. Sometimes I saw overflow
or underflow events and heard clicks.
The patch setup cyclic transfer such a way, that transfer can
run infinitely without CPU intervention. As a result,
lost samples and clicks gone.
Signed-off-by: Alexander Kochetkov
---
drivers/dma/pl330.c | 192 +--
1 file changed, 94 insertions(+), 98 deletions(-)
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 7539f73..56a2377 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -446,9 +446,6 @@ struct dma_pl330_chan {
int burst_len; /* the number of burst */
dma_addr_t fifo_addr;
- /* for cyclic capability */
- bool cyclic;
-
/* for runtime pm tracking */
bool active;
};
@@ -532,6 +529,10 @@ struct dma_pl330_desc {
unsigned peri:5;
/* Hook to attach to DMAC's list of reqs with due callback */
struct list_head rqd;
+
+ /* For cyclic capability */
+ bool cyclic;
+ size_t num_periods;
};
struct _xfer_spec {
@@ -1333,16 +1334,19 @@ static inline int _setup_loops(struct pl330_dmac *pl330,
}
static inline int _setup_xfer(struct pl330_dmac *pl330,
- unsigned dry_run, u8 buf[],
+ unsigned dry_run, u8 buf[], u32 period,
const struct _xfer_spec *pxs)
{
struct pl330_xfer *x = &pxs->desc->px;
+ struct pl330_reqcfg *rqcfg = &pxs->desc->rqcfg;
int off = 0;
/* DMAMOV SAR, x->src_addr */
- off += _emit_MOV(dry_run, &buf[off], SAR, x->src_addr);
+ off += _emit_MOV(dry_run, &buf[off], SAR,
+ x->src_addr + rqcfg->src_inc * period * x->bytes);
/* DMAMOV DAR, x->dst_addr */
- off += _emit_MOV(dry_run, &buf[off], DAR, x->dst_addr);
+ off += _emit_MOV(dry_run, &buf[off], DAR,
+ x->dst_addr + rqcfg->dst_inc * period * x->bytes);
/* Setup Loop(s) */
off += _setup_loops(pl330, dry_run, &buf[off], pxs);
@@ -1362,23 +1366,41 @@ static int _setup_req(struct pl330_dmac *pl330,
unsigned dry_run,
struct pl330_xfer *x;
u8 *buf = req->mc_cpu;
int off = 0;
+ int period;
+ int again_off;
PL330_DBGMC_START(req->mc_bus);
/* DMAMOV CCR, ccr */
off += _emit_MOV(dry_run, &buf[off], CCR, pxs->ccr);
+ again_off = off;
x = &pxs->desc->px;
/* Error if xfer length is not aligned at burst size */
if (x->bytes % (BRST_SIZE(pxs->ccr) * BRST_LEN(pxs->ccr)))
return -EINVAL;
- off += _setup_xfer(pl330, dry_run, &buf[off], pxs);
+ for (period = 0; period < pxs->desc->num_periods; period++) {
+ off += _setup_xfer(pl330, dry_run, &buf[off], period, pxs);
- /* DMASEV peripheral/event */
- off += _emit_SEV(dry_run, &buf[off], thrd->ev);
- /* DMAEND */
- off += _emit_END(dry_run, &buf[off]);
+ /* DMASEV peripheral/event */
+ off += _emit_SEV(dry_run, &buf[off], thrd->ev);
+ }
+
+ if (!pxs->desc->cyclic) {
+ /* DMAEND */
+ off += _emit_END(dry_run, &buf[off]);
+ } else {
+ struct _arg_LPEND lpend;
+ /* LP */
+ off += _emit_LP(dry_run, &buf[off], 0, 255);
+ /* LPEND */
+ lpend.cond = ALWAYS;
+ lpend.forever = false;
+ lpend.loop = 0;
+ lpend.bjump = off - again_off;
+ off += _emit_LPEND(dry_run, &buf[off], &lpend);
+ }
return off;
}
@@ -1640,12 +1662,13 @@ static int pl330_update(struct pl330_dmac *pl330)
/* Detach the req */
descdone = thrd->req[active].desc;
- thrd->req[active].desc = NULL;
-
- thrd->req_running = -1;
- /* Get going again ASAP */
- _start(thrd);
+ if (!descdone->cyclic) {
+ thrd->req[active].desc = NULL;
+ thrd->req_running = -1;
+ /* Get going again ASAP */
+ _start(thrd);
+ }
/* For now
[PATCH 0/2] Free running cyclic transfer implementation for pl330
Hello! This series contain free running cyclic transfer implementation for pl330. It affect ALL chips using pl330 (not only rockchip) and allow to run cyclic transfers without CPU intervention. As a result it fix sound clicks (observed and not yet observed) because sound clicks must be heard under heavy system load due to the way how cyclic transfers implemented now for pl330. My previous series[1] doesn't get enough attention (no one except me tested it). And it don't get upstream: > 8-03-2016, 6:03, Vinod Koul *: >Overall this series looks okay, but can someone test this. I would not like pl330 to be broken again Now I was asked about the series[1] again by guys from Rockchip, so I send rebased against 4.10.10 version. Hope, someone might test it and confirm that patches work fine. Regards, Alexander. Alexander Kochetkov (2): dmaengine: pl330: make cyclic transfer free runnable dmaengine: pl330: don't emit code for one iteration loop drivers/dma/pl330.c | 200 +-- 1 file changed, 98 insertions(+), 102 deletions(-) -- 1.7.9.5
Re: [PATCH v7 0/7] Implement clocksource for rockchip SoC using rockchip timer
Hello, Daniel.
Due to recent comments from Mark[1], may be is better to apply v6[2] series
instead of v7[3]?
Because my main goal was to fix wall time on rk3188. And I did it the same way
how that was
already done for other timer drivers (one of possible solution).
You can rename CLOCKSOURCE_OF to TIMER_OF later. I can help with that, but I
don’t
think it is good idea to combine my changes and timer framework
cleanups/improvements
into single series.
And I thinks, that probably is is better to drop [3] and [4] and revert
0c8893c9095d ("clockevents: Add a
clkevt-of mechanism like clksrc-of»).
What do you think?
[1] https://lkml.org/lkml/2017/3/29/286
[2] https://lkml.org/lkml/2017/1/31/401
[3] https://lkml.org/lkml/2017/3/22/508
[4] https://lkml.org/lkml/2017/3/22/420
[5] https://lkml.org/lkml/2017/3/22/426
> 22 марта 2017 г., в 18:48, Alexander Kochetkov
> написал(а):
>
> Hello, Daniel, Heiko.
>
> Here is try 7 :) Could you please take a look into it?
>
> rockchip_timer init code implemented using CLOCKEVENT_OF_DECLARE()
> introduced in commit 0c8893c9095d ("clockevents: Add a clkevt-of
> mechanism like clksrc-of")
>
> There is change in the arch/arm/mach-rockchip/rockchip.c.
>
> This series should be applied after the commit:
> https://lkml.org/lkml/2017/3/22/426
>
> As without the commit, you will get linker error ("clkevt-probe.c:63:
> undefined reference to `__clkevt_of_table’")
>
> Regards,
> Alexander.
>
>
> Alexander Kochetkov (6):
> dt-bindings: clarify compatible property for rockchip timers
> ARM: dts: rockchip: update compatible property for rk322x timer
> ARM: dts: rockchip: add clockevent attribute to rockchip timers
> clocksource/drivers/rockchip_timer: implement clocksource timer
> ARM: dts: rockchip: add timer entries to rk3188 SoC
> ARM: dts: rockchip: disable arm-global-timer for rk3188
>
> Daniel Lezcano (1):
> clocksource/drivers/clksrc-evt-probe: Describe with the DT both the
>clocksource and the clockevent
>
> .../bindings/timer/rockchip,rk-timer.txt | 12 +-
> Documentation/devicetree/bindings/timer/timer.txt | 38
> arch/arm/boot/dts/rk3036.dtsi |1 +
> arch/arm/boot/dts/rk3188.dtsi | 19 ++
> arch/arm/boot/dts/rk322x.dtsi |3 +-
> arch/arm/boot/dts/rk3288.dtsi |1 +
> arch/arm/mach-rockchip/rockchip.c |2 +
> arch/arm64/boot/dts/rockchip/rk3368.dtsi |1 +
> drivers/clocksource/Kconfig|2 +
> drivers/clocksource/clkevt-probe.c |7 +
> drivers/clocksource/clksrc-probe.c |7 +
> drivers/clocksource/rockchip_timer.c | 215 ++--
> 12 files changed, 241 insertions(+), 67 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/timer/timer.txt
>
> --
> 1.7.9.5
>
[PATCH] clk: rockchip: limit i2s0_pre max rate on rk3188
Change i2s0_pre from 768MHz to 192MHz and limit it's rate to 192MHz in
order to fix issues described below.
If right after the boot change i2s0_clk to 16.384MHz, real rate on
i2s0_clk pin may differ from 16.384MHz. The issue is random. Sometimes
rate on i2s0_clk pin equal to 16.384MHz, sometimes not.
There is another 100% reproducable issue. First we have to boot and see
the correct frequency on i2s0_clk pin (16.384MHz). Then we change its rate
to 8.192MHz (and it changes), then we change its rate again to 16.384MHz.
Rate leaves unchanged and equal to 8.192MHz.
'clk_summary' shows following clock connection in all the cases, where
rate was set to 16.384MHz (even then real rate differs).
clock rate
-
xin24m 2400
pll_gpll 76800
gpll 76800
i2s_src 76800
i2s0_pre 76800
i2s0_frac 16384000
sclk_i2s0 16384000
Also I found, that if I change i2s0_pre to 192MHz all the issues described
above gone. I supposed that the issues is due to high i2s0_pre rate.
Probably rk3188 has some sort of frequency restrictions imposed. Haven't
found anything in the RK3188 TRM, so this is my assumption.
Anyway, with the patch, all the issues gone.
Signed-off-by: Alexander Kochetkov
---
drivers/clk/rockchip/clk-rk3188.c |9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/clk/rockchip/clk-rk3188.c
b/drivers/clk/rockchip/clk-rk3188.c
index d0e722a..e883f48 100644
--- a/drivers/clk/rockchip/clk-rk3188.c
+++ b/drivers/clk/rockchip/clk-rk3188.c
@@ -849,6 +849,15 @@ static void __init rk3188a_clk_init(struct device_node *np)
__func__);
}
+ /* limit i2s0_pre max rate */
+ clk1 = __clk_lookup("aclk_cpu_pre");
+ clk2 = __clk_lookup("i2s0_pre");
+ if (clk1 && clk2) {
+ rate = clk_get_rate(clk1);
+ clk_set_max_rate(clk2, rate);
+ clk_set_rate(clk2, rate);
+ }
+
rockchip_clk_protect_critical(rk3188_critical_clocks,
ARRAY_SIZE(rk3188_critical_clocks));
rockchip_clk_of_add_provider(np, ctx);
--
1.7.9.5
Re: [PATCH v7 5/7] clocksource/drivers/rockchip_timer: implement clocksource timer
The patch series should be applied after the patches [1] and [2] haven’t merged
yet into the kernel.
That mention in the cover letter.
[1] https://lkml.org/lkml/2017/3/22/420
[2] https://lkml.org/lkml/2017/3/22/426
> 24 марта 2017 г., в 11:29, kbuild test robot написал(а):
>
> Hi Alexander,
>
> [auto build test ERROR on tip/timers/core]
> [also build test ERROR on v4.11-rc3 next-20170323]
> [cannot apply to rockchip/for-next]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Alexander-Kochetkov/Implement-clocksource-for-rockchip-SoC-using-rockchip-timer/20170324-113008
> config: arm64-defconfig (attached as .config)
> compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>wget
> https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O
> ~/bin/make.cross
>chmod +x ~/bin/make.cross
># save the attached .config to linux build tree
>make.cross ARCH=arm64
>
> All errors (new ones prefixed by >>):
>
> In file included from include/linux/tick.h:7:0,
>from drivers//acpi/processor_idle.c:32:
>>> include/linux/clockchips.h:232:2: error: invalid preprocessing directive
>>> #els
>#els
> ^~~
>>> include/linux/clockchips.h:233:19: error: static declaration of
>>> 'clockevent_probe' follows non-static declaration
>static inline int clockevent_probe(void) { return 0; }
> ^~~~
> include/linux/clockchips.h:231:12: note: previous declaration of
> 'clockevent_probe' was here
>extern int clockevent_probe(void);
> ^~~~
> --
>>> drivers//clocksource/clkevt-probe.c:20:29: fatal error: linux/clockchip.h:
>>> No such file or directory
>#include
>^
> compilation terminated.
>
> vim +232 include/linux/clockchips.h
>
> d316c57f Thomas Gleixner 2007-02-16 226
> 376bc271 Daniel Lezcano 2016-04-19 227 #define CLOCKEVENT_OF_DECLARE(name,
> compat, fn) \
> 376bc271 Daniel Lezcano 2016-04-19 228 OF_DECLARE_1_RET(clkevt, name,
> compat, fn)
> 376bc271 Daniel Lezcano 2016-04-19 229
> 376bc271 Daniel Lezcano 2016-04-19 230 #ifdef CONFIG_CLKEVT_PROBE
> 376bc271 Daniel Lezcano 2016-04-19 231 extern int clockevent_probe(void);
> 376bc271 Daniel Lezcano 2016-04-19 @232 #els
> 376bc271 Daniel Lezcano 2016-04-19 @233 static inline int
> clockevent_probe(void) { return 0; }
> 376bc271 Daniel Lezcano 2016-04-19 234 #endif
> 376bc271 Daniel Lezcano 2016-04-19 235
> 9eed56e8 Ingo Molnar 2015-04-02 236 #endif /* _LINUX_CLOCKCHIPS_H */
>
> :: The code at line 232 was first introduced by commit
> :: 376bc27150f180d9f5eddec6a14117780177589d clockevents: Add a clkevt-of
> mechanism like clksrc-of
>
> :: TO: Daniel Lezcano
> :: CC: Daniel Lezcano
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
> <.config.gz>
Re: [PATCH] ARM: dts: rockchip: increase SD clock frequency on Radxa Rock
> 22 марта 2017 г., в 20:04, Alexander Kochetkov > написал(а): > > Ok. Let leave as is for now. > > My radxa worked stable for a year with this settings. Ok, I not sure what my radxa worked stable with this settings. Hieko, thanks for catching that. Sorry for your time.
Re: [PATCH] ARM: dts: rockchip: increase SD clock frequency on Radxa Rock
> 22 марта 2017 г., в 19:47, Heiko Stuebner написал(а): > > sorry, but I will not apply this patch at this time. > When testing on my radxarock with the card I always use upon entering the > rootfs everything explodes with -110 errors, while without that patch the > card > runs stable as far as I can tell. > I also don't have the time to investigate further right now though. Ok. Let leave as is for now. My radxa worked stable for a year with this settings. And my custom rk3188 based board also has this setting. Kernel 4.8. And I saw -110 errors only then I disconnect SD-card from board.
Re: [PATCH] ARM: dts: rockchip: setup DMA-channels for mmc0 and emmc for rk3188
Hello, Heiko! > 22 марта 2017 г., в 18:54, Heiko Stuebner написал(а): > > I've applied a slightly different variant in [0] with your commit message and > moved the dma properties to the mmc/emmc nodes in rk3xxx.dtsi - as the dma > channels are the same on both rk3188 and rk3066. Thank you! I had changes in the rk3xxx.dtsi initially, but them moved them into rk3188. Don’t know why I did that. May be because I don’t have rk3066 based board to test on. > While at it, I've also added the sdio dma as there is no need to leave it out. I’ve tested mmc0 and emm and they work great. I haven’t tested sdio. > > Also when changing the devicetree, please do not append stuff at the bottom > but instead sort new nodes (the &mmc &emmc in this case) and properties > alphabetically. Ok. Thank you. I have working settings for eMMC for radxa rock. What do you think, is it good idea to add them to the kernel DT? As Radxa Rock doesn’t come with eMMC, but eMMC can be soldered manually. Alexander.
[PATCH v7 7/7] ARM: dts: rockchip: disable arm-global-timer for rk3188
The clocksource and the sched_clock provided by the arm_global_timer
are quite unstable because their rates depend on the cpu frequency.
On the other side, the arm_global_timer has a higher rating than the
rockchip_timer, it will be selected by default by the time framework
while we want to use the stable rockchip clocksource.
Let's disable the arm_global_timer in order to have the rockchip
clocksource selected by default.
Signed-off-by: Alexander Kochetkov
Reviewed-by: Heiko Stuebner
---
arch/arm/boot/dts/rk3188.dtsi |1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index a20d501..c316468 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -548,6 +548,7 @@
&global_timer {
interrupts = ;
+ status = "disabled";
};
&local_timer {
--
1.7.9.5
[PATCH v7 5/7] clocksource/drivers/rockchip_timer: implement clocksource timer
The clock supplying the arm-global-timer on the rk3188 is coming from the
the cpu clock itself and thus changes its rate everytime cpufreq adjusts
the cpu frequency making this timer unsuitable as a stable clocksource
and sched clock.
The rk3188, rk3288 and following socs share a separate timer block already
handled by the rockchip-timer driver. Therefore adapt this driver to also
be able to act as clocksource and sched clock on rk3188.
In order to test clocksource you can run following commands and check
how much time it take in real. On rk3188 it take about ~45 seconds.
cpufreq-set -f 1.6GHZ
date; sleep 60; date
In order to use the patch you need to declare two timers in the dts
file. The first timer will be initialized as clockevent provider
and the second one as clocksource. The clockevent must be from
alive subsystem as it used as backup for the local timers at sleep
time.
The patch does not break compatibility with older device tree files.
The older device tree files contain only one timer. The timer
will be initialized as clockevent, as expected. But new warnings like
'Failed to initialize /timer@' will appear with old device tree
files. To resolve them 'clockevent' attribute should be added to the
timer.
rk3288 (and probably anything newer) is irrelevant to this patch,
as it has the arch timer interface. This patch may be useful
for Cortex-A9/A5 based parts.
Signed-off-by: Alexander Kochetkov
---
arch/arm/mach-rockchip/rockchip.c|2 +
drivers/clocksource/Kconfig |2 +
drivers/clocksource/rockchip_timer.c | 215 --
3 files changed, 156 insertions(+), 63 deletions(-)
diff --git a/arch/arm/mach-rockchip/rockchip.c
b/arch/arm/mach-rockchip/rockchip.c
index a7ab9ec..5184f7d 100644
--- a/arch/arm/mach-rockchip/rockchip.c
+++ b/arch/arm/mach-rockchip/rockchip.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -67,6 +68,7 @@ static void __init rockchip_timer_init(void)
}
of_clk_init(NULL);
+ clockevent_probe();
clocksource_probe();
}
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 21f84ea..5e0df76 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -71,6 +71,8 @@ config ROCKCHIP_TIMER
bool "Rockchip timer driver" if COMPILE_TEST
depends on ARM || ARM64
select CLKSRC_OF
+ select CLKEVT_OF
+ select CLKSRC_MMIO
help
Enables the support for the rockchip timer driver.
diff --git a/drivers/clocksource/rockchip_timer.c
b/drivers/clocksource/rockchip_timer.c
index 23e267a..0ac71bb 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -11,6 +11,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -19,6 +21,8 @@
#define TIMER_LOAD_COUNT0 0x00
#define TIMER_LOAD_COUNT1 0x04
+#define TIMER_CURRENT_VALUE0 0x08
+#define TIMER_CURRENT_VALUE1 0x0C
#define TIMER_CONTROL_REG3288 0x10
#define TIMER_CONTROL_REG3399 0x1c
#define TIMER_INT_STATUS 0x18
@@ -29,103 +33,118 @@
#define TIMER_MODE_USER_DEFINED_COUNT (1 << 1)
#define TIMER_INT_UNMASK (1 << 2)
-struct bc_timer {
- struct clock_event_device ce;
+struct rk_timer {
void __iomem *base;
void __iomem *ctrl;
+ struct clk *clk;
+ struct clk *pclk;
u32 freq;
+ int irq;
};
-static struct bc_timer bc_timer;
+struct rk_clkevt {
+ struct clock_event_device ce;
+ struct rk_timer timer;
+};
-static inline struct bc_timer *rk_timer(struct clock_event_device *ce)
-{
- return container_of(ce, struct bc_timer, ce);
-}
+/* global instance for rk_timer_sched_read() */
+static struct rk_timer *rk_clksrc;
-static inline void __iomem *rk_base(struct clock_event_device *ce)
+static inline struct rk_timer *rk_timer(struct clock_event_device *ce)
{
- return rk_timer(ce)->base;
+ return &container_of(ce, struct rk_clkevt, ce)->timer;
}
-static inline void __iomem *rk_ctrl(struct clock_event_device *ce)
+static inline void rk_timer_disable(struct rk_timer *timer)
{
- return rk_timer(ce)->ctrl;
+ writel_relaxed(TIMER_DISABLE, timer->ctrl);
}
-static inline void rk_timer_disable(struct clock_event_device *ce)
+static inline void rk_timer_enable(struct rk_timer *timer, u32 flags)
{
- writel_relaxed(TIMER_DISABLE, rk_ctrl(ce));
-}
-
-static inline void rk_timer_enable(struct clock_event_device *ce, u32 flags)
-{
- writel_relaxed(TIMER_ENABLE | TIMER_INT_UNMASK | flags,
- rk_ctrl(ce));
+ writel_relaxed(TIMER_ENABLE | flags, timer->ctrl);
}
static void rk_timer_update_counter(unsigned long cycles,
- struct clock_event_device *c
[PATCH v7 2/7] dt-bindings: clarify compatible property for rockchip timers
Make all properties description in form '"rockchip,-timer", "rockchip,rk3288-timer"' for all chips found in linux kernel. Suggested-by: Heiko Stübner Signed-off-by: Alexander Kochetkov Acked-by: Rob Herring Reviewed-by: Heiko Stuebner --- .../bindings/timer/rockchip,rk-timer.txt | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt index a41b184..16a5f45 100644 --- a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt +++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt @@ -1,9 +1,15 @@ Rockchip rk timer Required properties: -- compatible: shall be one of: - "rockchip,rk3288-timer" - for rk3066, rk3036, rk3188, rk322x, rk3288, rk3368 - "rockchip,rk3399-timer" - for rk3399 +- compatible: should be: + "rockchip,rk3036-timer", "rockchip,rk3288-timer": for Rockchip RK3036 + "rockchip,rk3066-timer", "rockchip,rk3288-timer": for Rockchip RK3066 + "rockchip,rk3188-timer", "rockchip,rk3288-timer": for Rockchip RK3188 + "rockchip,rk3228-timer", "rockchip,rk3288-timer": for Rockchip RK3228 + "rockchip,rk3229-timer", "rockchip,rk3288-timer": for Rockchip RK3229 + "rockchip,rk3288-timer": for Rockchip RK3288 + "rockchip,rk3368-timer", "rockchip,rk3288-timer": for Rockchip RK3368 + "rockchip,rk3399-timer": for Rockchip RK3399 - reg: base address of the timer register starting with TIMERS CONTROL register - interrupts: should contain the interrupts for Timer0 - clocks : must contain an entry for each entry in clock-names -- 1.7.9.5
[PATCH v7 6/7] ARM: dts: rockchip: add timer entries to rk3188 SoC
The patch add two timers to all rk3188 based boards.
The first timer is from alive subsystem and it act as a backup
for the local timers at sleep time. It act the same as other
SoC rockchip timers already present in kernel.
The second timer is from CPU subsystem and act as replacement
for the arm-global-timer clocksource and sched clock. It run
at stable frequency 24MHz.
Signed-off-by: Alexander Kochetkov
Reviewed-by: Heiko Stuebner
---
arch/arm/boot/dts/rk3188.dtsi | 18 ++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index b3e6a30..a20d501 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -106,6 +106,24 @@
};
};
+ timer3: timer@2000e000 {
+ compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer";
+ reg = <0x2000e000 0x20>;
+ interrupts = ;
+ clocks = <&cru SCLK_TIMER3>, <&cru PCLK_TIMER3>;
+ clock-names = "timer", "pclk";
+ clockevent;
+ };
+
+ timer6: timer@200380a0 {
+ compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer";
+ reg = <0x200380a0 0x20>;
+ interrupts = ;
+ clocks = <&cru SCLK_TIMER6>, <&cru PCLK_TIMER0>;
+ clock-names = "timer", "pclk";
+ clocksource;
+ };
+
i2s0: i2s@1011a000 {
compatible = "rockchip,rk3188-i2s", "rockchip,rk3066-i2s";
reg = <0x1011a000 0x2000>;
--
1.7.9.5
[PATCH v7 3/7] ARM: dts: rockchip: update compatible property for rk322x timer
Property set to '"rockchip,rk3228-timer", "rockchip,rk3288-timer"'
to match devicetree bindings.
Signed-off-by: Alexander Kochetkov
Suggested-by: Heiko Stübner
Reviewed-by: Heiko Stuebner
---
arch/arm/boot/dts/rk322x.dtsi |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/rk322x.dtsi b/arch/arm/boot/dts/rk322x.dtsi
index 9e6bf0e..2a4eee2 100644
--- a/arch/arm/boot/dts/rk322x.dtsi
+++ b/arch/arm/boot/dts/rk322x.dtsi
@@ -323,7 +323,7 @@
};
timer: timer@110c {
- compatible = "rockchip,rk3288-timer";
+ compatible = "rockchip,rk3228-timer", "rockchip,rk3288-timer";
reg = <0x110c 0x20>;
interrupts = ;
clocks = <&xin24m>, <&cru PCLK_TIMER>;
--
1.7.9.5
[PATCH v7 4/7] ARM: dts: rockchip: add clockevent attribute to rockchip timers
All rockchip timers present in the DT act as clockevent.
It is possible to specify timer role using DT attribute.
Mark them accordingly.
Also this patch specify that for timer should be called init
function specified with CLOCKEVENT_OF_DECLARE().
Without the commit boot warnings will appear because clock
framework will try initialize them as clocksource.
Signed-off-by: Alexander Kochetkov
---
arch/arm/boot/dts/rk3036.dtsi|1 +
arch/arm/boot/dts/rk322x.dtsi|1 +
arch/arm/boot/dts/rk3288.dtsi|1 +
arch/arm64/boot/dts/rockchip/rk3368.dtsi |1 +
4 files changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index 843d2be..a04bb5a 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi
@@ -353,6 +353,7 @@
interrupts = ;
clocks = <&xin24m>, <&cru PCLK_TIMER>;
clock-names = "timer", "pclk";
+ clockevent;
};
pwm0: pwm@2005 {
diff --git a/arch/arm/boot/dts/rk322x.dtsi b/arch/arm/boot/dts/rk322x.dtsi
index 2a4eee2..2250640 100644
--- a/arch/arm/boot/dts/rk322x.dtsi
+++ b/arch/arm/boot/dts/rk322x.dtsi
@@ -328,6 +328,7 @@
interrupts = ;
clocks = <&xin24m>, <&cru PCLK_TIMER>;
clock-names = "timer", "pclk";
+ clockevent;
};
cru: clock-controller@110e {
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 91c4b3c..f45b7ad 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -217,6 +217,7 @@
interrupts = ;
clocks = <&xin24m>, <&cru PCLK_TIMER>;
clock-names = "timer", "pclk";
+ clockevent;
};
display-subsystem {
diff --git a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
index 4f44d11..054dadd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
@@ -673,6 +673,7 @@
compatible = "rockchip,rk3368-timer", "rockchip,rk3288-timer";
reg = <0x0 0xff81 0x0 0x20>;
interrupts = ;
+ clockevent;
};
gic: interrupt-controller@ffb71000 {
--
1.7.9.5
[PATCH v7 0/7] Implement clocksource for rockchip SoC using rockchip timer
Hello, Daniel, Heiko.
Here is try 7 :) Could you please take a look into it?
rockchip_timer init code implemented using CLOCKEVENT_OF_DECLARE()
introduced in commit 0c8893c9095d ("clockevents: Add a clkevt-of
mechanism like clksrc-of")
There is change in the arch/arm/mach-rockchip/rockchip.c.
This series should be applied after the commit:
https://lkml.org/lkml/2017/3/22/426
As without the commit, you will get linker error ("clkevt-probe.c:63:
undefined reference to `__clkevt_of_table’")
Regards,
Alexander.
Alexander Kochetkov (6):
dt-bindings: clarify compatible property for rockchip timers
ARM: dts: rockchip: update compatible property for rk322x timer
ARM: dts: rockchip: add clockevent attribute to rockchip timers
clocksource/drivers/rockchip_timer: implement clocksource timer
ARM: dts: rockchip: add timer entries to rk3188 SoC
ARM: dts: rockchip: disable arm-global-timer for rk3188
Daniel Lezcano (1):
clocksource/drivers/clksrc-evt-probe: Describe with the DT both the
clocksource and the clockevent
.../bindings/timer/rockchip,rk-timer.txt | 12 +-
Documentation/devicetree/bindings/timer/timer.txt | 38
arch/arm/boot/dts/rk3036.dtsi |1 +
arch/arm/boot/dts/rk3188.dtsi | 19 ++
arch/arm/boot/dts/rk322x.dtsi |3 +-
arch/arm/boot/dts/rk3288.dtsi |1 +
arch/arm/mach-rockchip/rockchip.c |2 +
arch/arm64/boot/dts/rockchip/rk3368.dtsi |1 +
drivers/clocksource/Kconfig|2 +
drivers/clocksource/clkevt-probe.c |7 +
drivers/clocksource/clksrc-probe.c |7 +
drivers/clocksource/rockchip_timer.c | 215 ++--
12 files changed, 241 insertions(+), 67 deletions(-)
create mode 100644 Documentation/devicetree/bindings/timer/timer.txt
--
1.7.9.5
[PATCH v7 1/7] clocksource/drivers/clksrc-evt-probe: Describe with the DT both the clocksource and the clockevent
From: Daniel Lezcano
The CLOCKSOURCE_OF_DECLARE() was introduced before the
CLOCKEVENT_OF_DECLARE() and that resulted in some abuse where the
clockevent and the clocksource are both initialized in the same init
routine.
With the introduction of the CLOCKEVENT_OF_DECLARE(), the driver can
now split the clocksource and the clockevent init code. However, the
device tree may specify a single node, so the same node will be passed
to the clockevent/clocksource's init function, with the same base
address.
with this patch it is possible to specify an attribute to the timer's node to
specify if it is a clocksource or a clockevent and define two timers node.
For example:
timer: timer@9840 {
compatible = "moxa,moxart-timer";
reg = <0x9840 0x42>;
interrupts = <19 1>;
clocks = <&coreclk>;
clockevent;
};
timer: timer@98400010 {
compatible = "moxa,moxart-timer";
reg = <0x98400010 0x42>;
clocks = <&coreclk>;
clocksource;
};
With this approach, we allow a mechanism to clearly define a clocksource or a
clockevent without aerobatics we can find around in some drivers:
timer-sp804.c, arc-timer.c, dw_apb_timer_of.c, mps2-timer.c,
renesas-ostm.c, time-efm32.c, time-lpc32xx.c.
Signed-off-by: Daniel Lezcano
Signed-off-by: Alexander Kochetkov
---
Documentation/devicetree/bindings/timer/timer.txt | 38 +
drivers/clocksource/clkevt-probe.c|7
drivers/clocksource/clksrc-probe.c|7
3 files changed, 52 insertions(+)
create mode 100644 Documentation/devicetree/bindings/timer/timer.txt
diff --git a/Documentation/devicetree/bindings/timer/timer.txt
b/Documentation/devicetree/bindings/timer/timer.txt
new file mode 100644
index 000..f1ee0cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/timer.txt
@@ -0,0 +1,38 @@
+
+Specifying timer information for devices
+
+
+The timer can be declared via the macro:
+
+CLOCKSOURCE_OF_DECLARE(name, init) if it is a clocksource
+CLOCKEVENT_OF_DECLARE(name, init) if it is a clockevent
+
+The CLOCKSOURCE_OF_DECLARE() was introduced before the
+CLOCKEVENT_OF_DECLARE() and that resulted in some abuse where the
+clockevent and the clocksource are both initialized in the same init
+routine.
+
+With the introduction of the CLOCKEVENT_OF_DECLARE(), the driver can
+now split the clocksource and the clockevent init code. However, the
+device tree may specify a single node, so the same node will be passed
+to the clockevent/clocksource's init function, with the same base
+address. It is possible to specify an attribute to the timer's node to
+specify if it is a clocksource or a clockevent and define two timers
+node.
+
+Example:
+
+ timer: timer@9840 {
+ compatible = "moxa,moxart-timer";
+ reg = <0x9840 0x42>;
+ interrupts = <19 1>;
+ clocks = <&coreclk>;
+ clockevent;
+ };
+
+ timer: timer@98400010 {
+ compatible = "moxa,moxart-timer";
+ reg = <0x98400010 0x42>;
+ clocks = <&coreclk>;
+ clocksource;
+ };
diff --git a/drivers/clocksource/clkevt-probe.c
b/drivers/clocksource/clkevt-probe.c
index eb89b50..fa02ac1 100644
--- a/drivers/clocksource/clkevt-probe.c
+++ b/drivers/clocksource/clkevt-probe.c
@@ -37,6 +37,13 @@ int __init clockevent_probe(void)
init_func = match->data;
+ /*
+* The device node describes a clocksource, ignore it
+* as we are in the clockevent init routine.
+*/
+ if (of_property_read_bool(np, "clocksource"))
+ continue;
+
ret = init_func(np);
if (ret) {
pr_warn("Failed to initialize '%s' (%d)\n",
diff --git a/drivers/clocksource/clksrc-probe.c
b/drivers/clocksource/clksrc-probe.c
index bc62be9..ce50f33 100644
--- a/drivers/clocksource/clksrc-probe.c
+++ b/drivers/clocksource/clksrc-probe.c
@@ -38,6 +38,13 @@ void __init clocksource_probe(void)
init_func_ret = match->data;
+ /*
+* The device node describes a clockevent, ignore it
+* as we are in the clocksource init routine.
+*/
+ if (of_property_read_bool(np, "clockevent"))
+ continue;
+
ret = init_func_ret(np);
if (ret) {
pr_err("Failed to initialize '%s': %d",
--
1.7.9.5
[PATCH] vmlinux.lds: add __clkevt_of_table to kernel
The code introduced by commit 0c8893c9095d ("clockevents: Add a
clkevt-of mechanism like clksrc-of") refer to __clkevt_of_table
what doesn't exist in the vmlinux. As a result kernel build
failed with error: "clkevt-probe.c:63: undefined reference to
`__clkevt_of_table’"
Fixes: 0c8893c9095d ("clockevents: Add a clkevt-of mechanism like clksrc-of")
Signed-off-by: Alexander Kochetkov
---
include/asm-generic/vmlinux.lds.h |2 ++
1 file changed, 2 insertions(+)
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index 2456397..42afee3 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -173,6 +173,7 @@
*(__##name##_of_table_end)
#define CLKSRC_OF_TABLES() OF_TABLE(CONFIG_CLKSRC_OF, clksrc)
+#define CLKEVT_OF_TABLES() OF_TABLE(CONFIG_CLKEVT_OF, clkevt)
#define IRQCHIP_OF_MATCH_TABLE() OF_TABLE(CONFIG_IRQCHIP, irqchip)
#define CLK_OF_TABLES()OF_TABLE(CONFIG_COMMON_CLK, clk)
#define IOMMU_OF_TABLES() OF_TABLE(CONFIG_OF_IOMMU, iommu)
@@ -539,6 +540,7 @@
CLK_OF_TABLES() \
RESERVEDMEM_OF_TABLES() \
CLKSRC_OF_TABLES() \
+ CLKEVT_OF_TABLES() \
IOMMU_OF_TABLES() \
CPU_METHOD_OF_TABLES() \
CPUIDLE_METHOD_OF_TABLES() \
--
1.7.9.5
[PATCH] clockevents: fix syntax errors
The patch fix syntax errors introduced by commit 0c8893c9095d
("clockevents: Add a clkevt-of mechanism like clksrc-of").
Fixes: 0c8893c9095d ("clockevents: Add a clkevt-of mechanism like clksrc-of")
Signed-off-by: Alexander Kochetkov
---
drivers/clocksource/clkevt-probe.c |2 +-
include/linux/clockchips.h |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clocksource/clkevt-probe.c
b/drivers/clocksource/clkevt-probe.c
index 86c096a..fa02ac1 100644
--- a/drivers/clocksource/clkevt-probe.c
+++ b/drivers/clocksource/clkevt-probe.c
@@ -17,7 +17,7 @@
#include
#include
-#include
+#include
extern struct of_device_id __clkevt_of_table[];
diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index 5d3053c..6d7edc3 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -229,7 +229,7 @@ static inline void tick_setup_hrtimer_broadcast(void) { }
#ifdef CONFIG_CLKEVT_PROBE
extern int clockevent_probe(void);
-#els
+#else
static inline int clockevent_probe(void) { return 0; }
#endif
--
1.7.9.5
[PATCH] ARM: dts: rockchip: increase SD clock frequency on Radxa Rock
The patch set SD clock frequency to maximum possible to SD-card (50MHz). This change actual clock frequency from 32MHz to 48MHz (speedup 1.5X). Banner line before patch: 'mmc_host mmc0: Bus speed (slot 0) = 3200Hz (slot req 5000Hz, actual 3200HZ div = 0)'. Banner line after patch: 'mmc_host mmc0: Bus speed (slot 0) = 9600Hz (slot req 5000Hz, actual 4800HZ div = 1)' Signed-off-by: Alexander Kochetkov --- arch/arm/boot/dts/rk3188-radxarock.dts |1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/boot/dts/rk3188-radxarock.dts b/arch/arm/boot/dts/rk3188-radxarock.dts index 1da46d1..5b7daa4 100644 --- a/arch/arm/boot/dts/rk3188-radxarock.dts +++ b/arch/arm/boot/dts/rk3188-radxarock.dts @@ -302,6 +302,7 @@ pinctrl-0 = <&sd0_clk>, <&sd0_cmd>, <&sd0_cd>, <&sd0_bus4>; vmmc-supply = <&vcc_sd0>; + clock-frequency = <5000>; bus-width = <4>; cap-mmc-highspeed; cap-sd-highspeed; -- 1.7.9.5
[PATCH] ARM: dts: rockchip: setup DMA-channels for mmc0 and emmc for rk3188
This commit enable DMA-based transfers for SD/eMMC card adapters
and reduce number of interrupts produced by SD-card/eMMC-card
adapters.
Sometimes interrupts from SD-card/eMMC-card adapters running in
PIO mode blocks execution of hrtimers and I2S DMA callbacks for
a long periods (100 ms or more).
Signed-off-by: Alexander Kochetkov
---
arch/arm/boot/dts/rk3188.dtsi | 10 ++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index 44da3d42..de786f8 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -638,3 +638,13 @@
&wdt {
compatible = "rockchip,rk3188-wdt", "snps,dw-wdt";
};
+
+&mmc0 {
+ dmas = <&dmac2 1>;
+ dma-names = "rx-tx";
+};
+
+&emmc {
+ dmas = <&dmac2 4>;
+ dma-names = "rx-tx";
+};
--
1.7.9.5
Re: [PATCH v6 0/5] Implement clocksource for rockchip SoC using rockchip timer
Hello, Daniel. Sorry for bothering you, but could you please take a look at v6[1] series and tell what do you think about it? I see your commit[2] in the git, but I don’t understand well how to use it in rockchip driver. [1] https://lkml.org/lkml/2017/1/31/401 [2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/clocksource?id=376bc27150f180d9f5eddec6a14117780177589d Regards, Alexander. > 31 янв. 2017 г., в 15:43, Alexander Kochetkov > написал(а): > > From: Alexander Kochetkov > > Hello, Daniel, Heiko. > > Here is try 6 :) Thanks a lot for helping me to bring the code > into kernel! > > This patch series contain: > - devicetree bindings clarification for rockchip timers > - dts files fixes for rk3228-evb, rk3229-evb and rk3188 > - implementation of clocksource and sched clock for rockchip SoC > > The clock supplying the arm-global-timer on the rk3188 is coming from the > the cpu clock itself and thus changes its rate everytime cpufreq adjusts > the cpu frequency making this timer unsuitable as a stable clocksource. > > The rk3188, rk3288 and following socs share a separate timer block already > handled by the rockchip-timer driver. Therefore adapt this driver to also > be able to act as clocksource on rk3188. > > In order to test clocksource you can run following commands and check > how much time it take in real. On rk3188 it take about ~45 seconds. > > cpufreq-set -f 1.6GHZ > date; sleep 60; date > > rk3288 (and probably anything newer) is irrelevant to this patch, > as it has the arch timer interface. This patch may be usefull > for Cortex-A9/A5 based parts. > > Regards, > Alexander. > > Changes in v6: > - Removed Reviewed-by: Heiko Stübner tag > - Merge 5/8, 6/8, 7/8, 8/9 into single file > - split init paths into rk_clkevt_init() and rk_clksrc_init() > so the driver code could be adopted to CLOCKEVENT_OF_DECLARE() > - clockevents implemented using clocksource_mmio_init() > - fixed commit message for 4/8 (thanks a lot Daniel) > > Changes in v5: > - Add Acked-by: Rob Herring to 1/8 > http://lists.infradead.org/pipermail/linux-rockchip/2016-December/013308.html > - Add Reviwed-by: Heiko Stübner to series > - change timer compatible property in the rk322x.dtsi 2/8 > http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013784.html > - updated comment message for 4/8 > http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013786.html > - updated comment message for 5/8 > http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013787.html > - fixed build error for 8/8 > http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013789.html > > Alexander Kochetkov (5): > dt-bindings: clarify compatible property for rockchip timers > ARM: dts: rockchip: update compatible property for rk322x timer > clocksource/drivers/rockchip_timer: implement clocksource timer > ARM: dts: rockchip: add timer entries to rk3188 SoC > ARM: dts: rockchip: disable arm-global-timer for rk3188 > > .../bindings/timer/rockchip,rk-timer.txt | 12 +- > arch/arm/boot/dts/rk3188.dtsi | 17 ++ > arch/arm/boot/dts/rk322x.dtsi |2 +- > drivers/clocksource/Kconfig|1 + > drivers/clocksource/rockchip_timer.c | 218 ++-- > 5 files changed, 185 insertions(+), 65 deletions(-) > > -- > 1.7.9.5 >
[PATCH v6 2/5] ARM: dts: rockchip: update compatible property for rk322x timer
Property set to '"rockchip,rk3228-timer", "rockchip,rk3288-timer"'
to match devicetree bindings.
Signed-off-by: Alexander Kochetkov
Suggested-by: Heiko Stübner
---
arch/arm/boot/dts/rk322x.dtsi |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/rk322x.dtsi b/arch/arm/boot/dts/rk322x.dtsi
index 9e6bf0e..2a4eee2 100644
--- a/arch/arm/boot/dts/rk322x.dtsi
+++ b/arch/arm/boot/dts/rk322x.dtsi
@@ -323,7 +323,7 @@
};
timer: timer@110c {
- compatible = "rockchip,rk3288-timer";
+ compatible = "rockchip,rk3228-timer", "rockchip,rk3288-timer";
reg = <0x110c 0x20>;
interrupts = ;
clocks = <&xin24m>, <&cru PCLK_TIMER>;
--
1.7.9.5
[PATCH v6 4/5] ARM: dts: rockchip: add timer entries to rk3188 SoC
The patch add two timers to all rk3188 based boards.
The first timer is from alive subsystem and it act as a backup
for the local timers at sleep time. It act the same as other
SoC rockchip timers already present in kernel.
The second timer is from CPU subsystem and act as replacement
for the arm-global-timer clocksource and sched clock. It run
at stable frequency 24MHz.
Signed-off-by: Alexander Kochetkov
---
arch/arm/boot/dts/rk3188.dtsi | 16
1 file changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index 31f81b2..0dc52fe 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -106,6 +106,22 @@
};
};
+ timer3: timer@2000e000 {
+ compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer";
+ reg = <0x2000e000 0x20>;
+ interrupts = ;
+ clocks = <&cru SCLK_TIMER3>, <&cru PCLK_TIMER3>;
+ clock-names = "timer", "pclk";
+ };
+
+ timer6: timer@200380a0 {
+ compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer";
+ reg = <0x200380a0 0x20>;
+ interrupts = ;
+ clocks = <&cru SCLK_TIMER6>, <&cru PCLK_TIMER0>;
+ clock-names = "timer", "pclk";
+ };
+
i2s0: i2s@1011a000 {
compatible = "rockchip,rk3188-i2s", "rockchip,rk3066-i2s";
reg = <0x1011a000 0x2000>;
--
1.7.9.5
[PATCH v6 5/5] ARM: dts: rockchip: disable arm-global-timer for rk3188
The clocksource and the sched_clock provided by the arm_global_timer
are quite unstable because their rates depend on the cpu frequency.
On the other side, the arm_global_timer has a higher rating than the
rockchip_timer, it will be selected by default by the time framework
while we want to use the stable rockchip clocksource.
Let's disable the arm_global_timer in order to have the rockchip
clocksource selected by default.
Signed-off-by: Alexander Kochetkov
---
arch/arm/boot/dts/rk3188.dtsi |1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index 0dc52fe..44da3d42 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -546,6 +546,7 @@
&global_timer {
interrupts = ;
+ status = "disabled";
};
&local_timer {
--
1.7.9.5
[PATCH v6 3/5] clocksource/drivers/rockchip_timer: implement clocksource timer
The clock supplying the arm-global-timer on the rk3188 is coming from the
the cpu clock itself and thus changes its rate everytime cpufreq adjusts
the cpu frequency making this timer unsuitable as a stable clocksource
and sched clock.
The rk3188, rk3288 and following socs share a separate timer block already
handled by the rockchip-timer driver. Therefore adapt this driver to also
be able to act as clocksource and sched clock on rk3188.
In order to test clocksource you can run following commands and check
how much time it take in real. On rk3188 it take about ~45 seconds.
cpufreq-set -f 1.6GHZ
date; sleep 60; date
In order to use the patch you need to declare two timers in the dts
file. The first timer will be initialized as clockevent provider
and the second one as clocksource. The clockevent must be from
alive subsystem as it used as backup for the local timers at sleep
time.
The patch does not break compatibility with older device tree files.
The older device tree files contain only one timer. The timer
will be initialized as clockevent, as expected.
rk3288 (and probably anything newer) is irrelevant to this patch,
as it has the arch timer interface. This patch may be useful
for Cortex-A9/A5 based parts.
Signed-off-by: Alexander Kochetkov
---
drivers/clocksource/Kconfig |1 +
drivers/clocksource/rockchip_timer.c | 218 --
2 files changed, 158 insertions(+), 61 deletions(-)
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 5677886..e34d4ac 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -64,6 +64,7 @@ config ROCKCHIP_TIMER
bool "Rockchip timer driver" if COMPILE_TEST
depends on ARM || ARM64
select CLKSRC_OF
+ select CLKSRC_MMIO
help
Enables the support for the rockchip timer driver.
diff --git a/drivers/clocksource/rockchip_timer.c
b/drivers/clocksource/rockchip_timer.c
index 23e267a..49c02be 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -11,6 +11,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -19,6 +21,8 @@
#define TIMER_LOAD_COUNT0 0x00
#define TIMER_LOAD_COUNT1 0x04
+#define TIMER_CURRENT_VALUE0 0x08
+#define TIMER_CURRENT_VALUE1 0x0C
#define TIMER_CONTROL_REG3288 0x10
#define TIMER_CONTROL_REG3399 0x1c
#define TIMER_INT_STATUS 0x18
@@ -29,103 +33,118 @@
#define TIMER_MODE_USER_DEFINED_COUNT (1 << 1)
#define TIMER_INT_UNMASK (1 << 2)
-struct bc_timer {
- struct clock_event_device ce;
+struct rk_timer {
void __iomem *base;
void __iomem *ctrl;
+ struct clk *clk;
+ struct clk *pclk;
u32 freq;
+ int irq;
};
-static struct bc_timer bc_timer;
-
-static inline struct bc_timer *rk_timer(struct clock_event_device *ce)
-{
- return container_of(ce, struct bc_timer, ce);
-}
+struct rk_clkevt {
+ struct clock_event_device ce;
+ struct rk_timer timer;
+};
-static inline void __iomem *rk_base(struct clock_event_device *ce)
-{
- return rk_timer(ce)->base;
-}
+static struct rk_clkevt *rk_clkevt;
+static struct rk_timer *rk_clksrc;
-static inline void __iomem *rk_ctrl(struct clock_event_device *ce)
+static inline struct rk_timer *rk_timer(struct clock_event_device *ce)
{
- return rk_timer(ce)->ctrl;
+ return &container_of(ce, struct rk_clkevt, ce)->timer;
}
-static inline void rk_timer_disable(struct clock_event_device *ce)
+static inline void rk_timer_disable(struct rk_timer *timer)
{
- writel_relaxed(TIMER_DISABLE, rk_ctrl(ce));
+ writel_relaxed(TIMER_DISABLE, timer->ctrl);
}
-static inline void rk_timer_enable(struct clock_event_device *ce, u32 flags)
+static inline void rk_timer_enable(struct rk_timer *timer, u32 flags)
{
- writel_relaxed(TIMER_ENABLE | TIMER_INT_UNMASK | flags,
- rk_ctrl(ce));
+ writel_relaxed(TIMER_ENABLE | flags, timer->ctrl);
}
static void rk_timer_update_counter(unsigned long cycles,
- struct clock_event_device *ce)
+ struct rk_timer *timer)
{
- writel_relaxed(cycles, rk_base(ce) + TIMER_LOAD_COUNT0);
- writel_relaxed(0, rk_base(ce) + TIMER_LOAD_COUNT1);
+ writel_relaxed(cycles, timer->base + TIMER_LOAD_COUNT0);
+ writel_relaxed(0, timer->base + TIMER_LOAD_COUNT1);
}
-static void rk_timer_interrupt_clear(struct clock_event_device *ce)
+static void rk_timer_interrupt_clear(struct rk_timer *timer)
{
- writel_relaxed(1, rk_base(ce) + TIMER_INT_STATUS);
+ writel_relaxed(1, timer->base + TIMER_INT_STATUS);
}
static inline int rk_timer_set_next_event(unsigned long cycles,
struct clock_event_device *ce)
{
- rk_
[PATCH v6 0/5] Implement clocksource for rockchip SoC using rockchip timer
From: Alexander Kochetkov Hello, Daniel, Heiko. Here is try 6 :) Thanks a lot for helping me to bring the code into kernel! This patch series contain: - devicetree bindings clarification for rockchip timers - dts files fixes for rk3228-evb, rk3229-evb and rk3188 - implementation of clocksource and sched clock for rockchip SoC The clock supplying the arm-global-timer on the rk3188 is coming from the the cpu clock itself and thus changes its rate everytime cpufreq adjusts the cpu frequency making this timer unsuitable as a stable clocksource. The rk3188, rk3288 and following socs share a separate timer block already handled by the rockchip-timer driver. Therefore adapt this driver to also be able to act as clocksource on rk3188. In order to test clocksource you can run following commands and check how much time it take in real. On rk3188 it take about ~45 seconds. cpufreq-set -f 1.6GHZ date; sleep 60; date rk3288 (and probably anything newer) is irrelevant to this patch, as it has the arch timer interface. This patch may be usefull for Cortex-A9/A5 based parts. Regards, Alexander. Changes in v6: - Removed Reviewed-by: Heiko Stübner tag - Merge 5/8, 6/8, 7/8, 8/9 into single file - split init paths into rk_clkevt_init() and rk_clksrc_init() so the driver code could be adopted to CLOCKEVENT_OF_DECLARE() - clockevents implemented using clocksource_mmio_init() - fixed commit message for 4/8 (thanks a lot Daniel) Changes in v5: - Add Acked-by: Rob Herring to 1/8 http://lists.infradead.org/pipermail/linux-rockchip/2016-December/013308.html - Add Reviwed-by: Heiko Stübner to series - change timer compatible property in the rk322x.dtsi 2/8 http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013784.html - updated comment message for 4/8 http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013786.html - updated comment message for 5/8 http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013787.html - fixed build error for 8/8 http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013789.html Alexander Kochetkov (5): dt-bindings: clarify compatible property for rockchip timers ARM: dts: rockchip: update compatible property for rk322x timer clocksource/drivers/rockchip_timer: implement clocksource timer ARM: dts: rockchip: add timer entries to rk3188 SoC ARM: dts: rockchip: disable arm-global-timer for rk3188 .../bindings/timer/rockchip,rk-timer.txt | 12 +- arch/arm/boot/dts/rk3188.dtsi | 17 ++ arch/arm/boot/dts/rk322x.dtsi |2 +- drivers/clocksource/Kconfig|1 + drivers/clocksource/rockchip_timer.c | 218 ++-- 5 files changed, 185 insertions(+), 65 deletions(-) -- 1.7.9.5
[PATCH v6 1/5] dt-bindings: clarify compatible property for rockchip timers
Make all properties description in form '"rockchip,-timer", "rockchip,rk3288-timer"' for all chips found in linux kernel. Suggested-by: Heiko Stübner Signed-off-by: Alexander Kochetkov Acked-by: Rob Herring --- .../bindings/timer/rockchip,rk-timer.txt | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt index a41b184..16a5f45 100644 --- a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt +++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt @@ -1,9 +1,15 @@ Rockchip rk timer Required properties: -- compatible: shall be one of: - "rockchip,rk3288-timer" - for rk3066, rk3036, rk3188, rk322x, rk3288, rk3368 - "rockchip,rk3399-timer" - for rk3399 +- compatible: should be: + "rockchip,rk3036-timer", "rockchip,rk3288-timer": for Rockchip RK3036 + "rockchip,rk3066-timer", "rockchip,rk3288-timer": for Rockchip RK3066 + "rockchip,rk3188-timer", "rockchip,rk3288-timer": for Rockchip RK3188 + "rockchip,rk3228-timer", "rockchip,rk3288-timer": for Rockchip RK3228 + "rockchip,rk3229-timer", "rockchip,rk3288-timer": for Rockchip RK3229 + "rockchip,rk3288-timer": for Rockchip RK3288 + "rockchip,rk3368-timer", "rockchip,rk3288-timer": for Rockchip RK3368 + "rockchip,rk3399-timer": for Rockchip RK3399 - reg: base address of the timer register starting with TIMERS CONTROL register - interrupts: should contain the interrupts for Timer0 - clocks : must contain an entry for each entry in clock-names -- 1.7.9.5
Re: [PATCH v5 5/8] clocksource/drivers/rockchip_timer: split bc_timer into rk_timer and rk_clock_event_device
> 30 янв. 2017 г., в 16:12, Daniel Lezcano > написал(а): > > I don't get the point of these changes. The patch does not explain why they > are > needed. I’d like to extract timer API from current implementation. And to make code more readable I’d like to introduce 'struct rk_timer’ what can be reused with current implementation and with my patch (8/8). And in order keep patches simple and readable I split that into three patches: 5/8, 6/8, 7/8. Current implementation named rockchip timer as ‘struct bc_timer’ (broadcast timer). I renamed it to more suitable to it role (may be bad choice). Yes, the patch itself looks strange. You are right. What do you think about that solution: - in the patch 6/8 i will Introduce 'struct rk_timer’ and 'struct rk_time_clkevt’ (renamed ‘struct bc_timer’). - rk_timer_init() changes from 5/8 I will merge with 8/8 - 8/8 introduce 'struct rk_time_clksrc' - 5/8 drop Alexander.
Re: [PATCH v5 3/8] ARM: dts: rockchip: add timer entries to rk3188 SoC
> 30 янв. 2017 г., в 15:04, Daniel Lezcano > написал(а): > > There is no case when the rockchip timer is used for the clockevent. The is already timer entry for rk3228 in the DT. And it act as clockevent. I guess it work as backup, but I cannot test it also. In order to not break DT compatibility I had to provide one timer to be initialized as clockevent. And implemented implicit rule the driver (first DT timer - clockevent, second DT timer - clocksource) already exists for other timers. > If I'm not wrong, you can check /proc/interrupts and see there is no interrupt > on the rockchip timer. Yes, you right here. I’ve temporary disable smp_twd to test rockchip timer interrupts. There is no interrupt on the rockchip timer during normal work. > - when the CPU enters a deep idle state. But such state does not exist on > rk3188 ARM chip/revision specific? > - when the system goes to suspend. But the timers are stopped in any case and > CPU0 is always on. There is timer for rk3228 in the DT. I guess there are situations where it can be used. May be the situations are also acceptable for rk3188? May be something like suspend to RAM or suspend to HDD? I am not expert in that questions. I can do some tests if you give hint/link. So, as I understood you suggest to leave only one timer what can be used as clocksource only. I can implement that, but there should be DT rule what allow to setup timer as clocksource only. I cannot do more without timer framework support. Looks, like I have to wait your patch to implement that. Alexander.
Re: [PATCH v5 3/8] ARM: dts: rockchip: add timer entries to rk3188 SoC
> 30 янв. 2017 г., в 14:04, Daniel Lezcano > написал(а): > > t is up to the RTC to wake up the system from suspend and there is no > idle state stopping the local timers on these SoCs. So, the rockchip timers > won't be ever used on the rk3188, right ? No rockchip timers where used on rk3188 before. Actually, I don’t know how to test backup timer functionality. rk3228 backup timer is from alive subsystem. It is unclear from TRM what this mean, I assume that somehow related to CLK during suspend. So I pick the timer from alive subsystem on rk3188 also. DT timer configuration where tested: interrupts and IO works. > This patch should go after the rockchip timer patches Ok, let me know, when I should send v6. Or may be you change the order of the patches during merge. And thank you for fixing commit message for previous patch. Feel free to tell me than I write something really unclear or incorrect. Alexander.
Re: [PATCH] clockevents: Add a clkevt-of mechanism like clksrc-of
> 25 янв. 2017 г., в 15:21, Daniel Lezcano > написал(а): > > Hopefully, that can help to do some housework in the directory, perhaps > split the drivers in to entities, for example: > - clksrc-rockchip.c > - clkevt-rockchip.c > > Also, it gives the possibility to declare clocksources separately in the > DT and then use a clocksource from IP block while while clockevents are > used from another IP block. Hello, Daniel! There is no way to do declare clocksources and clockevents in the DT now. As I understood, it is incorrect to add properties like ‘clocksource’ or ‘clockevent’ to timer entries because this sounds like linux specific configuration. While device tree must contain only hardware description. Thus many timer drivers has implicit assumption: first DT timer became clocksource, second clockevent. If we can find the way to describe timer role in the DT, than many timer drivers could be rewritten such a way. Also, there is timers what use single IP to provide clocksource and clockevent at the same time. So splitting them into two files result into more complex code. Regards, Alexander.
Re: [PATCH v4 0/9] Implement clocksource for rockchip SoC using rockchip timer
Hello Daniel! Sorry for bothering you. Just let you know that I sent v5 here: http://lists.infradead.org/pipermail/linux-rockchip/2017-January/thread.html#13812 Because previous series was somehow missed in your inbox. Heiko don’t want to add "Reviewed-by» to my patches. Could you please apply the patches without the tag. Let me know is all ok with series now? Or may be I have to do something else? Regards, Alexander. > 23 янв. 2017 г., в 20:57, Daniel Lezcano > написал(а): > > On Mon, Jan 23, 2017 at 08:24:47PM +0300, Alexander Kochetkov wrote: >> Heiko, Daniel, thanks a lot for review! >> I’ll send v5 series this week. >> >> Heiko, Daniel, may I add 'Reviewed-by:’ to patch series? >> >> Reviewed-by: Heiko Stübner ? >> Reviewed-by: Daniel Lezcano ? >> > > I will answer with the corresponding tag myself when you send the v5. > > Thanks. > > -- Daniel > > -- > > <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs > > Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | > <http://twitter.com/#!/linaroorg> Twitter | > <http://www.linaro.org/linaro-blog/> Blog
Re: [PATCH v5 5/8] clocksource/drivers/rockchip_timer: split bc_timer into rk_timer and rk_clock_event_device
> 24 янв. 2017 г., в 18:02, Heiko Stübner написал(а): > > Please don't add Reviewed-by tags without explicit mention of them by > reviewers. (Also it's spelled wrong). > > I haven't looked that deeply into the driver itself and the changes made to > feel comfortable with a "Reviewed-by". Than I posted another linux code I was asked to add such tags (and others) to add credits to people. Sorry for that. v6? or feel free to drop any tags during merge.
[PATCH v5 0/8] Implement clocksource for rockchip SoC using rockchip timer
Hello, This patch series contain: - devicetree bindings clarification for rockchip timers - dts files fixes for rk3228-evb, rk3229-evb and rk3188 - implementation of clocksource and sched clock for rockchip SoC The clock supplying the arm-global-timer on the rk3188 is coming from the the cpu clock itself and thus changes its rate everytime cpufreq adjusts the cpu frequency making this timer unsuitable as a stable clocksource. The rk3188, rk3288 and following socs share a separate timer block already handled by the rockchip-timer driver. Therefore adapt this driver to also be able to act as clocksource on rk3188. In order to test clocksource you can run following commands and check how much time it take in real. On rk3188 it take about ~45 seconds. cpufreq-set -f 1.6GHZ date; sleep 60; date rk3288 (and probably anything newer) is irrelevant to this patch, as it has the arch timer interface. This patch may be usefull for Cortex-A9/A5 based parts. Regards, Alexander. Changes in v5: - Add 'Acked-by: Rob Herring ' to 1/8 http://lists.infradead.org/pipermail/linux-rockchip/2016-December/013308.html - Add 'Reviwed-by: Heiko Stübner ' to series - change timer compatible property in the rk322x.dtsi 2/8 http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013784.html - updated comment message for 4/8 http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013786.html - updated comment message for 5/8 http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013787.html - fixed build error for 8/8 http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013789.html Changes in v4: merged 7 and 8 from series 3 merged 10, 11, 12, 13 from series 3 fixed commit message Changes in v3: added patches: ARM: dts: rockchip: disable arm-global-timer for rk3188 clocksource/drivers/rockchip_timer: Prevent ftrace recursion devicetree v1 patches: https://patchwork.ozlabs.org/patch/699019/ https://patchwork.ozlabs.org/patch/699020/ kernel v1 patches: https://patchwork.kernel.org/patch/9443975/ https://patchwork.kernel.org/patch/9443971/ https://patchwork.kernel.org/patch/9443959/ https://patchwork.kernel.org/patch/9443963/ https://patchwork.kernel.org/patch/9443979/ https://patchwork.kernel.org/patch/9443989/ https://patchwork.kernel.org/patch/9443987/ https://patchwork.kernel.org/patch/9443977/ https://patchwork.kernel.org/patch/9443991/ Alexander Kochetkov (8): dt-bindings: clarify compatible property for rockchip timers ARM: dts: rockchip: update compatible property for rk322x timer ARM: dts: rockchip: add timer entries to rk3188 SoC ARM: dts: rockchip: disable arm-global-timer for rk3188 clocksource/drivers/rockchip_timer: split bc_timer into rk_timer and rk_clock_event_device clocksource/drivers/rockchip_timer: low level routines take rk_timer as parameter clocksource/drivers/rockchip_timer: move TIMER_INT_UNMASK out of rk_timer_enable() clocksource/drivers/rockchip_timer: implement clocksource timer .../bindings/timer/rockchip,rk-timer.txt | 12 +- arch/arm/boot/dts/rk3188.dtsi | 17 ++ arch/arm/boot/dts/rk322x.dtsi |2 +- drivers/clocksource/rockchip_timer.c | 207 +++- 4 files changed, 183 insertions(+), 55 deletions(-) -- 1.7.9.5
[PATCH v5 4/8] ARM: dts: rockchip: disable arm-global-timer for rk3188
clocksource and shed_clock provided by arm-global-timer is quite
unstable, because their rate depends on cpu frequency.
So disable arm-global-timer and use clocksource and sched_clock
from rockchip_timer.
It is impossible get stable clocksource having rockchip_timer and
arm-global-timer enabled at the same time. Because arm-global-timer
looks like a better candidate for the kernel: it has higher
frequency and rating.
Disabling arm-global-timer doesn't leave kernel without
clockevents as there is another clockevent provider (smp-twd).
Signed-off-by: Alexander Kochetkov
Reviwed-by: Heiko Stübner
---
arch/arm/boot/dts/rk3188.dtsi |1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index bcf8e03..f677130 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -546,6 +546,7 @@
&global_timer {
interrupts = ;
+ status = "disabled";
};
&local_timer {
--
1.7.9.5
[PATCH v5 2/8] ARM: dts: rockchip: update compatible property for rk322x timer
Property set to '"rockchip,rk3228-timer", "rockchip,rk3288-timer"'
to match devicetree bindings.
Signed-off-by: Alexander Kochetkov
Suggested-by: Heiko Stübner
---
arch/arm/boot/dts/rk322x.dtsi |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/rk322x.dtsi b/arch/arm/boot/dts/rk322x.dtsi
index 9d3aee5..aaeacf8 100644
--- a/arch/arm/boot/dts/rk322x.dtsi
+++ b/arch/arm/boot/dts/rk322x.dtsi
@@ -325,7 +325,7 @@
};
timer: timer@110c {
- compatible = "rockchip,rk3288-timer";
+ compatible = "rockchip,rk3228-timer", "rockchip,rk3288-timer";
reg = <0x110c 0x20>;
interrupts = ;
clocks = <&xin24m>, <&cru PCLK_TIMER>;
--
1.7.9.5
[PATCH v5 5/8] clocksource/drivers/rockchip_timer: split bc_timer into rk_timer and rk_clock_event_device
The patch move ce field out of struct bc_timer into struct
rk_clock_event_device and rename struct bc_timer to struct rk_timer.
The main idea for the commit is to exctact low level timer
routines from current implementation so they could be
reused in the following clocksource implementation commit.
This is refactoring step without functional changes.
Signed-off-by: Alexander Kochetkov
Reviwed-by: Heiko Stübner
---
drivers/clocksource/rockchip_timer.c | 33 ++---
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c
b/drivers/clocksource/rockchip_timer.c
index 23e267a..6d68d4c 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -29,18 +29,28 @@
#define TIMER_MODE_USER_DEFINED_COUNT (1 << 1)
#define TIMER_INT_UNMASK (1 << 2)
-struct bc_timer {
- struct clock_event_device ce;
+struct rk_timer {
void __iomem *base;
void __iomem *ctrl;
u32 freq;
};
-static struct bc_timer bc_timer;
+struct rk_clock_event_device {
+ struct clock_event_device ce;
+ struct rk_timer timer;
+};
+
+static struct rk_clock_event_device bc_timer;
+
+static inline struct rk_clock_event_device*
+rk_clock_event_device(struct clock_event_device *ce)
+{
+ return container_of(ce, struct rk_clock_event_device, ce);
+}
-static inline struct bc_timer *rk_timer(struct clock_event_device *ce)
+static inline struct rk_timer *rk_timer(struct clock_event_device *ce)
{
- return container_of(ce, struct bc_timer, ce);
+ return &rk_clock_event_device(ce)->timer;
}
static inline void __iomem *rk_base(struct clock_event_device *ce)
@@ -116,16 +126,17 @@ static irqreturn_t rk_timer_interrupt(int irq, void
*dev_id)
static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
{
struct clock_event_device *ce = &bc_timer.ce;
+ struct rk_timer *timer = &bc_timer.timer;
struct clk *timer_clk;
struct clk *pclk;
int ret = -EINVAL, irq;
- bc_timer.base = of_iomap(np, 0);
- if (!bc_timer.base) {
+ timer->base = of_iomap(np, 0);
+ if (!timer->base) {
pr_err("Failed to get base address for '%s'\n", TIMER_NAME);
return -ENXIO;
}
- bc_timer.ctrl = bc_timer.base + ctrl_reg;
+ timer->ctrl = timer->base + ctrl_reg;
pclk = of_clk_get_by_name(np, "pclk");
if (IS_ERR(pclk)) {
@@ -153,7 +164,7 @@ static int __init rk_timer_init(struct device_node *np, u32
ctrl_reg)
goto out_timer_clk;
}
- bc_timer.freq = clk_get_rate(timer_clk);
+ timer->freq = clk_get_rate(timer_clk);
irq = irq_of_parse_and_map(np, 0);
if (!irq) {
@@ -181,7 +192,7 @@ static int __init rk_timer_init(struct device_node *np, u32
ctrl_reg)
goto out_irq;
}
- clockevents_config_and_register(ce, bc_timer.freq, 1, UINT_MAX);
+ clockevents_config_and_register(ce, timer->freq, 1, UINT_MAX);
return 0;
@@ -190,7 +201,7 @@ static int __init rk_timer_init(struct device_node *np, u32
ctrl_reg)
out_timer_clk:
clk_disable_unprepare(pclk);
out_unmap:
- iounmap(bc_timer.base);
+ iounmap(timer->base);
return ret;
}
--
1.7.9.5
[PATCH v5 6/8] clocksource/drivers/rockchip_timer: low level routines take rk_timer as parameter
Pass rk_timer instead of clock_event_device to low lever timer routines.
So that code could be reused by clocksource implementation.
Drop rk_base() and rk_ctrl().
This is refactoring step without functional changes.
Signed-off-by: Alexander Kochetkov
Reviwed-by: Heiko Stübner
---
drivers/clocksource/rockchip_timer.c | 57 --
1 file changed, 27 insertions(+), 30 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c
b/drivers/clocksource/rockchip_timer.c
index 6d68d4c..a17dc61 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -53,70 +53,67 @@ static inline struct rk_timer *rk_timer(struct
clock_event_device *ce)
return &rk_clock_event_device(ce)->timer;
}
-static inline void __iomem *rk_base(struct clock_event_device *ce)
+static inline void rk_timer_disable(struct rk_timer *timer)
{
- return rk_timer(ce)->base;
+ writel_relaxed(TIMER_DISABLE, timer->ctrl);
}
-static inline void __iomem *rk_ctrl(struct clock_event_device *ce)
-{
- return rk_timer(ce)->ctrl;
-}
-
-static inline void rk_timer_disable(struct clock_event_device *ce)
-{
- writel_relaxed(TIMER_DISABLE, rk_ctrl(ce));
-}
-
-static inline void rk_timer_enable(struct clock_event_device *ce, u32 flags)
+static inline void rk_timer_enable(struct rk_timer *timer, u32 flags)
{
writel_relaxed(TIMER_ENABLE | TIMER_INT_UNMASK | flags,
- rk_ctrl(ce));
+ timer->ctrl);
}
static void rk_timer_update_counter(unsigned long cycles,
- struct clock_event_device *ce)
+ struct rk_timer *timer)
{
- writel_relaxed(cycles, rk_base(ce) + TIMER_LOAD_COUNT0);
- writel_relaxed(0, rk_base(ce) + TIMER_LOAD_COUNT1);
+ writel_relaxed(cycles, timer->base + TIMER_LOAD_COUNT0);
+ writel_relaxed(0, timer->base + TIMER_LOAD_COUNT1);
}
-static void rk_timer_interrupt_clear(struct clock_event_device *ce)
+static void rk_timer_interrupt_clear(struct rk_timer *timer)
{
- writel_relaxed(1, rk_base(ce) + TIMER_INT_STATUS);
+ writel_relaxed(1, timer->base + TIMER_INT_STATUS);
}
static inline int rk_timer_set_next_event(unsigned long cycles,
struct clock_event_device *ce)
{
- rk_timer_disable(ce);
- rk_timer_update_counter(cycles, ce);
- rk_timer_enable(ce, TIMER_MODE_USER_DEFINED_COUNT);
+ struct rk_timer *timer = rk_timer(ce);
+
+ rk_timer_disable(timer);
+ rk_timer_update_counter(cycles, timer);
+ rk_timer_enable(timer, TIMER_MODE_USER_DEFINED_COUNT);
return 0;
}
static int rk_timer_shutdown(struct clock_event_device *ce)
{
- rk_timer_disable(ce);
+ struct rk_timer *timer = rk_timer(ce);
+
+ rk_timer_disable(timer);
return 0;
}
static int rk_timer_set_periodic(struct clock_event_device *ce)
{
- rk_timer_disable(ce);
- rk_timer_update_counter(rk_timer(ce)->freq / HZ - 1, ce);
- rk_timer_enable(ce, TIMER_MODE_FREE_RUNNING);
+ struct rk_timer *timer = rk_timer(ce);
+
+ rk_timer_disable(timer);
+ rk_timer_update_counter(timer->freq / HZ - 1, timer);
+ rk_timer_enable(timer, TIMER_MODE_FREE_RUNNING);
return 0;
}
static irqreturn_t rk_timer_interrupt(int irq, void *dev_id)
{
struct clock_event_device *ce = dev_id;
+ struct rk_timer *timer = rk_timer(ce);
- rk_timer_interrupt_clear(ce);
+ rk_timer_interrupt_clear(timer);
if (clockevent_state_oneshot(ce))
- rk_timer_disable(ce);
+ rk_timer_disable(timer);
ce->event_handler(ce);
@@ -183,8 +180,8 @@ static int __init rk_timer_init(struct device_node *np, u32
ctrl_reg)
ce->cpumask = cpu_possible_mask;
ce->rating = 250;
- rk_timer_interrupt_clear(ce);
- rk_timer_disable(ce);
+ rk_timer_interrupt_clear(timer);
+ rk_timer_disable(timer);
ret = request_irq(irq, rk_timer_interrupt, IRQF_TIMER, TIMER_NAME, ce);
if (ret) {
--
1.7.9.5
[PATCH v5 8/8] clocksource/drivers/rockchip_timer: implement clocksource timer
The clock supplying the arm-global-timer on the rk3188 is coming from the
the cpu clock itself and thus changes its rate everytime cpufreq adjusts
the cpu frequency making this timer unsuitable as a stable clocksource
and sched clock.
The rk3188, rk3288 and following socs share a separate timer block already
handled by the rockchip-timer driver. Therefore adapt this driver to also
be able to act as clocksource and sched clock on rk3188.
In order to test clocksource you can run following commands and check
how much time it take in real. On rk3188 it take about ~45 seconds.
cpufreq-set -f 1.6GHZ
date; sleep 60; date
In order to use the patch you need to declare two timers in the dts
file. The first timer will be initialized as clockevent provider
and the second one as clocksource. The clockevent must be from
alive subsystem as it used as backup for the local timers at sleep
time.
The patch does not break compatibility with older device tree files.
The older device tree files contain only one timer. The timer
will be initialized as clockevent, as expected.
rk3288 (and probably anything newer) is irrelevant to this patch,
as it has the arch timer interface. This patch may be usefull
for Cortex-A9/A5 based parts.
Signed-off-by: Alexander Kochetkov
Reviwed-by: Heiko Stübner
---
drivers/clocksource/rockchip_timer.c | 137 +-
1 file changed, 117 insertions(+), 20 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c
b/drivers/clocksource/rockchip_timer.c
index 61c3bb1..3ff533c 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -19,6 +20,8 @@
#define TIMER_LOAD_COUNT0 0x00
#define TIMER_LOAD_COUNT1 0x04
+#define TIMER_CURRENT_VALUE0 0x08
+#define TIMER_CURRENT_VALUE1 0x0C
#define TIMER_CONTROL_REG3288 0x10
#define TIMER_CONTROL_REG3399 0x1c
#define TIMER_INT_STATUS 0x18
@@ -40,7 +43,19 @@ struct rk_clock_event_device {
struct rk_timer timer;
};
+struct rk_clocksource {
+ struct clocksource cs;
+ struct rk_timer timer;
+};
+
+enum {
+ ROCKCHIP_CLKSRC_CLOCKEVENT = 0,
+ ROCKCHIP_CLKSRC_CLOCKSOURCE = 1,
+};
+
static struct rk_clock_event_device bc_timer;
+static struct rk_clocksource cs_timer;
+static int rk_next_clksrc = ROCKCHIP_CLKSRC_CLOCKEVENT;
static inline struct rk_clock_event_device*
rk_clock_event_device(struct clock_event_device *ce)
@@ -63,11 +78,37 @@ static inline void rk_timer_enable(struct rk_timer *timer,
u32 flags)
writel_relaxed(TIMER_ENABLE | flags, timer->ctrl);
}
-static void rk_timer_update_counter(unsigned long cycles,
- struct rk_timer *timer)
+static void rk_timer_update_counter(u64 cycles, struct rk_timer *timer)
+{
+ u32 lower = cycles & 0x;
+ u32 upper = (cycles >> 32) & 0x;
+
+ writel_relaxed(lower, timer->base + TIMER_LOAD_COUNT0);
+ writel_relaxed(upper, timer->base + TIMER_LOAD_COUNT1);
+}
+
+static u64 notrace _rk_timer_counter_read(struct rk_timer *timer)
{
- writel_relaxed(cycles, timer->base + TIMER_LOAD_COUNT0);
- writel_relaxed(0, timer->base + TIMER_LOAD_COUNT1);
+ u64 counter;
+ u32 lower;
+ u32 upper, old_upper;
+
+ upper = readl_relaxed(timer->base + TIMER_CURRENT_VALUE1);
+ do {
+ old_upper = upper;
+ lower = readl_relaxed(timer->base + TIMER_CURRENT_VALUE0);
+ upper = readl_relaxed(timer->base + TIMER_CURRENT_VALUE1);
+ } while (upper != old_upper);
+
+ counter = upper;
+ counter <<= 32;
+ counter |= lower;
+ return counter;
+}
+
+static u64 rk_timer_counter_read(struct rk_timer *timer)
+{
+ return _rk_timer_counter_read(timer);
}
static void rk_timer_interrupt_clear(struct rk_timer *timer)
@@ -120,13 +161,46 @@ static irqreturn_t rk_timer_interrupt(int irq, void
*dev_id)
return IRQ_HANDLED;
}
+static u64 rk_timer_clocksource_read(struct clocksource *cs)
+{
+ struct rk_clocksource *_cs =
+ container_of(cs, struct rk_clocksource, cs);
+
+ return ~rk_timer_counter_read(&_cs->timer);
+}
+
+static u64 notrace rk_timer_sched_clock_read(void)
+{
+ struct rk_clocksource *_cs = &cs_timer;
+
+ return ~_rk_timer_counter_read(&_cs->timer);
+}
+
static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
{
- struct clock_event_device *ce = &bc_timer.ce;
- struct rk_timer *timer = &bc_timer.timer;
+ struct clock_event_device *ce = NULL;
+ struct clocksource *cs = NULL;
+ struct rk_timer *timer = NULL;
struct clk *timer_clk;
struct clk *pclk;
int ret = -EINVAL, irq;
+ int clksrc;
+
+ clksrc = rk_next_clks
[PATCH v5 7/8] clocksource/drivers/rockchip_timer: move TIMER_INT_UNMASK out of rk_timer_enable()
This allow to enable timer without enabling interrupts from it.
As that mode will be used in clocksource implementation.
This is refactoring step without functional changes.
Signed-off-by: Alexander Kochetkov
Reviwed-by: Heiko Stübner
---
drivers/clocksource/rockchip_timer.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c
b/drivers/clocksource/rockchip_timer.c
index a17dc61..61c3bb1 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -60,8 +60,7 @@ static inline void rk_timer_disable(struct rk_timer *timer)
static inline void rk_timer_enable(struct rk_timer *timer, u32 flags)
{
- writel_relaxed(TIMER_ENABLE | TIMER_INT_UNMASK | flags,
- timer->ctrl);
+ writel_relaxed(TIMER_ENABLE | flags, timer->ctrl);
}
static void rk_timer_update_counter(unsigned long cycles,
@@ -83,7 +82,8 @@ static inline int rk_timer_set_next_event(unsigned long
cycles,
rk_timer_disable(timer);
rk_timer_update_counter(cycles, timer);
- rk_timer_enable(timer, TIMER_MODE_USER_DEFINED_COUNT);
+ rk_timer_enable(timer, TIMER_MODE_USER_DEFINED_COUNT |
+ TIMER_INT_UNMASK);
return 0;
}
@@ -101,7 +101,7 @@ static int rk_timer_set_periodic(struct clock_event_device
*ce)
rk_timer_disable(timer);
rk_timer_update_counter(timer->freq / HZ - 1, timer);
- rk_timer_enable(timer, TIMER_MODE_FREE_RUNNING);
+ rk_timer_enable(timer, TIMER_MODE_FREE_RUNNING | TIMER_INT_UNMASK);
return 0;
}
--
1.7.9.5
[PATCH v5 3/8] ARM: dts: rockchip: add timer entries to rk3188 SoC
The patch add two timers to all rk3188 based boards.
The first timer is from alive subsystem and it act as a backup
for the local timers at sleep time. It act the same as other
SoC rockchip timers already present in kernel.
The second timer is from CPU subsystem and act as replacement
for the arm-global-timer clocksource and sched clock. It run
at stable frequency 24MHz.
Signed-off-by: Alexander Kochetkov
Reviwed-by: Heiko Stübner
---
arch/arm/boot/dts/rk3188.dtsi | 16
1 file changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index 869e189..bcf8e03 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -106,6 +106,22 @@
};
};
+ timer3: timer@2000e000 {
+ compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer";
+ reg = <0x2000e000 0x20>;
+ interrupts = ;
+ clocks = <&cru SCLK_TIMER3>, <&cru PCLK_TIMER3>;
+ clock-names = "timer", "pclk";
+ };
+
+ timer6: timer@200380a0 {
+ compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer";
+ reg = <0x200380a0 0x20>;
+ interrupts = ;
+ clocks = <&cru SCLK_TIMER6>, <&cru PCLK_TIMER0>;
+ clock-names = "timer", "pclk";
+ };
+
i2s0: i2s@1011a000 {
compatible = "rockchip,rk3188-i2s", "rockchip,rk3066-i2s";
reg = <0x1011a000 0x2000>;
--
1.7.9.5
[PATCH v5 1/8] dt-bindings: clarify compatible property for rockchip timers
Make all properties description in form '"rockchip,-timer", "rockchip,rk3288-timer"' for all chips found in linux kernel. Suggested-by: Heiko Stübner Signed-off-by: Alexander Kochetkov Acked-by: Rob Herring --- .../bindings/timer/rockchip,rk-timer.txt | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt index a41b184..16a5f45 100644 --- a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt +++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt @@ -1,9 +1,15 @@ Rockchip rk timer Required properties: -- compatible: shall be one of: - "rockchip,rk3288-timer" - for rk3066, rk3036, rk3188, rk322x, rk3288, rk3368 - "rockchip,rk3399-timer" - for rk3399 +- compatible: should be: + "rockchip,rk3036-timer", "rockchip,rk3288-timer": for Rockchip RK3036 + "rockchip,rk3066-timer", "rockchip,rk3288-timer": for Rockchip RK3066 + "rockchip,rk3188-timer", "rockchip,rk3288-timer": for Rockchip RK3188 + "rockchip,rk3228-timer", "rockchip,rk3288-timer": for Rockchip RK3228 + "rockchip,rk3229-timer", "rockchip,rk3288-timer": for Rockchip RK3229 + "rockchip,rk3288-timer": for Rockchip RK3288 + "rockchip,rk3368-timer", "rockchip,rk3288-timer": for Rockchip RK3368 + "rockchip,rk3399-timer": for Rockchip RK3399 - reg: base address of the timer register starting with TIMERS CONTROL register - interrupts: should contain the interrupts for Timer0 - clocks : must contain an entry for each entry in clock-names -- 1.7.9.5
Re: [PATCH v4 0/9] Implement clocksource for rockchip SoC using rockchip timer
Heiko, Daniel, thanks a lot for review! I’ll send v5 series this week. Heiko, Daniel, may I add 'Reviewed-by:’ to patch series? Reviewed-by: Heiko Stübner ? Reviewed-by: Daniel Lezcano ? Regards, Alexander. > 23 янв. 2017 г., в 20:12, Heiko Stübner написал(а): > > Am Montag, 23. Januar 2017, 15:47:44 CET schrieb Daniel Lezcano: >> On Wed, Dec 21, 2016 at 05:21:05PM +0300, Alexander Kochetkov wrote: >>> Hello Heiko, Daniel! >>> >>> Are there any reasons why the patches [1][2] are not applied yet into >>> kernel? How can I help in applying patches? >> >> Hi Alexander, >> >> sorry for the delay. Let me review them before taking the patchset. >> >> Heiko, can you have a look to them also ? > > somehow this series moved down to much in my inbox, sorry. > > Devicetree changes look good, except where I commented. > I guess it would be best (least intrusive) if I queue up the (then fixed) > devicetree changes after you are satisfied with the code parts. > > I've also looked over the code changes again, test-build them and found the > build error mentioned separately. Overall they look good though. > > > Heiko
Re: [PATCH v4 6/9] clocksource/drivers/rockchip_timer: split bc_timer into rk_timer and rk_clock_event_device
Daniel, thanks for reviewing patches! > 23 янв. 2017 г., в 19:26, Daniel Lezcano > написал(а): > > On Tue, Nov 29, 2016 at 07:14:49PM +0300, Alexander Kochetkov wrote: >> The patch move ce field out of struct bc_timer into struct >> rk_clock_event_device and rename struct bc_timer to struct rk_timer. > > Why ? Single rockchip timer can be either broadcast timer/clockevent (current implementation) or clocksource (the one I’ve implemented in series). Both implementations based on low level timer routines (rk_timer_disable, rk_timer_enable, rk_timer_update_counter, rk_timer_counter_read and so on). Both implementations use timers in different modes (interrupts vs free running). In order to distinguish this concepts I tried to split low level timer routines from bc_timer. Otherwise I can place all needed clocksource filelds into 'struct bc_timer’ and rename 'struct bc_timer’ into 'struct rk_timer’. If this is more clear solution I can rewrite patches. Alexander.
Re: [PATCH v4 2/9] ARM: dts: rockchip: update compatible property for rk3228 timer
Hello Daniel!
:
> +
> +&timer {
> + compatible = "rockchip,rk3228-timer", "rockchip,rk3288-timer";
> +};
> 23 янв. 2017 г., в 18:40, Daniel Lezcano
> написал(а):
>
> I'm not sure this is correct, to which timer &timer will refer ?
>
>timer {
>compatible = "arm,armv7-timer";
> ...
> }
>
>timer: timer@110c {
>compatible = "rockchip,rk3288-timer";
> ...
> }
The block '&timer { …}' change compatible string for 'timer: timer@110c’
timer'. It refers to it using ’timer:’ label.
'arm,armv7-timer’ has ’timer’ node name without label and cannot be addressed
in underline dts files.
> Why not change the compatible string in the timer definition in rk322x.dtsi ?
Initially my patch series was for rk3188 only. I declared rk3188 timer using
rk3228 compatible string[1].
Heiko reviewed the series and suggest[2] to use another ‘compatible=‘ binding
for rk3188. And
suggest[3] to update rockchip,rk-timer.txt bindings, like
mmc/rockchip-dw-mshc.txt does.
So I updated rockchip,rk-timer.txt. rk322x family has has two parts rk3228 and
rk3229. So I replaced
rk322x to rk3228 and rk3229 and updated dts file to match bindings.
[1]
http://lists.infradead.org/pipermail/linux-rockchip/2016-November/013152.html
[2]
http://lists.infradead.org/pipermail/linux-rockchip/2016-November/013174.html
[3]
http://lists.infradead.org/pipermail/linux-rockchip/2016-November/013174.html
> Same comment for the other patches doing these changes.
>
> -- Daniel
Re: [PATCH v4 0/9] Implement clocksource for rockchip SoC using rockchip timer
Hello Heiko, Daniel! Are there any reasons why the patches [1][2] are not applied yet into kernel? How can I help in applying patches? Regards, Alexander. [1] http://lists.infradead.org/pipermail/linux-rockchip/2016-November/thread.html#13236 [PATCH v4 0/9] Implement clocksource for rockchip SoC using rockchip timer [2] http://lists.infradead.org/pipermail/linux-rockchip/2016-December/013308.html > 29 нояб. 2016 г., в 19:14, Alexander Kochetkov > написал(а): > > Hello, > > This patch series contain: > - devicetree bindings clarification for rockchip timers > - dts files fixes for rk3228-evb, rk3229-evb and rk3188 > - implementation of clocksource and sched clock for rockchip SoC > > The clock supplying the arm-global-timer on the rk3188 is coming from the > the cpu clock itself and thus changes its rate everytime cpufreq adjusts > the cpu frequency making this timer unsuitable as a stable clocksource. > > The rk3188, rk3288 and following socs share a separate timer block already > handled by the rockchip-timer driver. Therefore adapt this driver to also > be able to act as clocksource on rk3188. > > In order to test clocksource you can run following commands and check > how much time it take in real. On rk3188 it take about ~45 seconds. > >cpufreq-set -f 1.6GHZ >date; sleep 60; date > > rk3288 (and probably anything newer) is irrelevant to this patch, > as it has the arch timer interface. This patch may be usefull > for Cortex-A9/A5 based parts. > > Regards, > Alexander. > > This is try 4. Please discard all v1, v2, v3 patches. > > Changes in v4: > merged 7 and 8 from series 3 > merged 10, 11, 12, 13 from series 3 > fixed commit message > > Changes in v3: > added patches: > ARM: dts: rockchip: disable arm-global-timer for rk3188 > clocksource/drivers/rockchip_timer: Prevent ftrace recursion > > devicetree v1 patches: > https://patchwork.ozlabs.org/patch/699019/ > https://patchwork.ozlabs.org/patch/699020/ > > kernel v1 patches: > https://patchwork.kernel.org/patch/9443975/ > https://patchwork.kernel.org/patch/9443971/ > https://patchwork.kernel.org/patch/9443959/ > https://patchwork.kernel.org/patch/9443963/ > https://patchwork.kernel.org/patch/9443979/ > https://patchwork.kernel.org/patch/9443989/ > https://patchwork.kernel.org/patch/9443987/ > https://patchwork.kernel.org/patch/9443977/ > https://patchwork.kernel.org/patch/9443991/ > > Alexander Kochetkov (9): > dt-bindings: clarify compatible property for rockchip timers > ARM: dts: rockchip: update compatible property for rk3228 timer > ARM: dts: rockchip: update compatible property for rk3229 timer > ARM: dts: rockchip: add timer entries to rk3188 SoC > ARM: dts: rockchip: disable arm-global-timer for rk3188 > clocksource/drivers/rockchip_timer: split bc_timer into rk_timer and >rk_clock_event_device > clocksource/drivers/rockchip_timer: low level routines take rk_timer >as parameter > clocksource/drivers/rockchip_timer: move TIMER_INT_UNMASK out of >rk_timer_enable() > clocksource/drivers/rockchip_timer: implement clocksource timer > > .../bindings/timer/rockchip,rk-timer.txt | 12 +- > arch/arm/boot/dts/rk3188.dtsi | 17 ++ > arch/arm/boot/dts/rk3228-evb.dts |4 + > arch/arm/boot/dts/rk3229-evb.dts |4 + > drivers/clocksource/rockchip_timer.c | 207 +++- > 5 files changed, 190 insertions(+), 54 deletions(-) > > -- > 1.7.9.5 >
Re: [PATCH] dt-bindings: document how to setup rockchip timers as clocksource
> 1 дек. 2016 г., в 0:30, Rob Herring написал(а): > > 1st and 2nd are ambiguous. Plus this is an OS implementation detail that > doesn't belong in the binding. > >> +If you want to bind specific timer as clockevent (i.e. one from alive >> subsystem) >> +and specific timer as clocksource, you can number the timers in "aliases" >> node. > > No. > > Use and/or describe what are the features of a timer to make the > decision. There has to be some reason you care which one. One has an > interrupt and the other doesn't. One is always on. Etc. Thank you, Rob. Eventually I abandoned this decision. I left only one patch, which you confirmed recently. And sorry for making noise with duplicate patches. Alexander.
[PATCH v4 1/9] dt-bindings: clarify compatible property for rockchip timers
Make all properties description in form '"rockchip,-timer", "rockchip,rk3288-timer"' for all chips found in linux kernel. Suggested-by: Heiko Stübner Signed-off-by: Alexander Kochetkov --- .../bindings/timer/rockchip,rk-timer.txt | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt index a41b184..16a5f45 100644 --- a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt +++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt @@ -1,9 +1,15 @@ Rockchip rk timer Required properties: -- compatible: shall be one of: - "rockchip,rk3288-timer" - for rk3066, rk3036, rk3188, rk322x, rk3288, rk3368 - "rockchip,rk3399-timer" - for rk3399 +- compatible: should be: + "rockchip,rk3036-timer", "rockchip,rk3288-timer": for Rockchip RK3036 + "rockchip,rk3066-timer", "rockchip,rk3288-timer": for Rockchip RK3066 + "rockchip,rk3188-timer", "rockchip,rk3288-timer": for Rockchip RK3188 + "rockchip,rk3228-timer", "rockchip,rk3288-timer": for Rockchip RK3228 + "rockchip,rk3229-timer", "rockchip,rk3288-timer": for Rockchip RK3229 + "rockchip,rk3288-timer": for Rockchip RK3288 + "rockchip,rk3368-timer", "rockchip,rk3288-timer": for Rockchip RK3368 + "rockchip,rk3399-timer": for Rockchip RK3399 - reg: base address of the timer register starting with TIMERS CONTROL register - interrupts: should contain the interrupts for Timer0 - clocks : must contain an entry for each entry in clock-names -- 1.7.9.5
[PATCH v4 9/9] clocksource/drivers/rockchip_timer: implement clocksource timer
The clock supplying the arm-global-timer on the rk3188 is coming from the
the cpu clock itself and thus changes its rate everytime cpufreq adjusts
the cpu frequency making this timer unsuitable as a stable clocksource
and sched clock.
The rk3188, rk3288 and following socs share a separate timer block already
handled by the rockchip-timer driver. Therefore adapt this driver to also
be able to act as clocksource and sched clock on rk3188.
In order to test clocksource you can run following commands and check
how much time it take in real. On rk3188 it take about ~45 seconds.
cpufreq-set -f 1.6GHZ
date; sleep 60; date
In order to use the patch you need to declare two timers in the dts
file. The first timer will be initialized as clockevent provider
and the second one as clocksource. The clockevent must be from
alive subsystem as it used as backup for the local timers at sleep
time.
The patch does not break compatibility with older device tree files.
The older device tree files contain only one timer. The timer
will be initialized as clockevent, as expected.
rk3288 (and probably anything newer) is irrelevant to this patch,
as it has the arch timer interface. This patch may be usefull
for Cortex-A9/A5 based parts.
Signed-off-by: Alexander Kochetkov
---
drivers/clocksource/rockchip_timer.c | 137 +-
1 file changed, 117 insertions(+), 20 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c
b/drivers/clocksource/rockchip_timer.c
index 61c3bb1..a127822 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -19,6 +20,8 @@
#define TIMER_LOAD_COUNT0 0x00
#define TIMER_LOAD_COUNT1 0x04
+#define TIMER_CURRENT_VALUE0 0x08
+#define TIMER_CURRENT_VALUE1 0x0C
#define TIMER_CONTROL_REG3288 0x10
#define TIMER_CONTROL_REG3399 0x1c
#define TIMER_INT_STATUS 0x18
@@ -40,7 +43,19 @@ struct rk_clock_event_device {
struct rk_timer timer;
};
+struct rk_clocksource {
+ struct clocksource cs;
+ struct rk_timer timer;
+};
+
+enum {
+ ROCKCHIP_CLKSRC_CLOCKEVENT = 0,
+ ROCKCHIP_CLKSRC_CLOCKSOURCE = 1,
+};
+
static struct rk_clock_event_device bc_timer;
+static struct rk_clocksource cs_timer;
+static int rk_next_clksrc = ROCKCHIP_CLKSRC_CLOCKEVENT;
static inline struct rk_clock_event_device*
rk_clock_event_device(struct clock_event_device *ce)
@@ -63,11 +78,37 @@ static inline void rk_timer_enable(struct rk_timer *timer,
u32 flags)
writel_relaxed(TIMER_ENABLE | flags, timer->ctrl);
}
-static void rk_timer_update_counter(unsigned long cycles,
- struct rk_timer *timer)
+static void rk_timer_update_counter(u64 cycles, struct rk_timer *timer)
+{
+ u32 lower = cycles & 0x;
+ u32 upper = (cycles >> 32) & 0x;
+
+ writel_relaxed(lower, timer->base + TIMER_LOAD_COUNT0);
+ writel_relaxed(upper, timer->base + TIMER_LOAD_COUNT1);
+}
+
+static u64 notrace _rk_timer_counter_read(struct rk_timer *timer)
{
- writel_relaxed(cycles, timer->base + TIMER_LOAD_COUNT0);
- writel_relaxed(0, timer->base + TIMER_LOAD_COUNT1);
+ u64 counter;
+ u32 lower;
+ u32 upper, old_upper;
+
+ upper = readl_relaxed(timer->base + TIMER_CURRENT_VALUE1);
+ do {
+ old_upper = upper;
+ lower = readl_relaxed(timer->base + TIMER_CURRENT_VALUE0);
+ upper = readl_relaxed(timer->base + TIMER_CURRENT_VALUE1);
+ } while (upper != old_upper);
+
+ counter = upper;
+ counter <<= 32;
+ counter |= lower;
+ return counter;
+}
+
+static u64 rk_timer_counter_read(struct rk_timer *timer)
+{
+ return _rk_timer_counter_read(timer);
}
static void rk_timer_interrupt_clear(struct rk_timer *timer)
@@ -120,13 +161,46 @@ static irqreturn_t rk_timer_interrupt(int irq, void
*dev_id)
return IRQ_HANDLED;
}
+static cycle_t rk_timer_clocksource_read(struct clocksource *cs)
+{
+ struct rk_clocksource *_cs =
+ container_of(cs, struct rk_clocksource, cs);
+
+ return ~rk_timer_counter_read(&_cs->timer);
+}
+
+static u64 notrace rk_timer_sched_clock_read(void)
+{
+ struct rk_clocksource *_cs = &cs_timer;
+
+ return ~_rk_timer_counter_read(&_cs->timer);
+}
+
static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
{
- struct clock_event_device *ce = &bc_timer.ce;
- struct rk_timer *timer = &bc_timer.timer;
+ struct clock_event_device *ce = NULL;
+ struct clocksource *cs = NULL;
+ struct rk_timer *timer = NULL;
struct clk *timer_clk;
struct clk *pclk;
int ret = -EINVAL, irq;
+ int clksrc;
+
+ clksrc = rk_next_clksrc;
+ rk_next_clksrc+
[PATCH v4 2/9] ARM: dts: rockchip: update compatible property for rk3228 timer
Property set to '"rockchip,rk3228-timer", "rockchip,rk3288-timer"'
to match devicetree bindings.
Signed-off-by: Alexander Kochetkov
---
arch/arm/boot/dts/rk3228-evb.dts |4
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/rk3228-evb.dts b/arch/arm/boot/dts/rk3228-evb.dts
index 904668e..38eab87 100644
--- a/arch/arm/boot/dts/rk3228-evb.dts
+++ b/arch/arm/boot/dts/rk3228-evb.dts
@@ -70,3 +70,7 @@
&uart2 {
status = "okay";
};
+
+&timer {
+ compatible = "rockchip,rk3228-timer", "rockchip,rk3288-timer";
+};
--
1.7.9.5
[PATCH v4 3/9] ARM: dts: rockchip: update compatible property for rk3229 timer
Property set to '"rockchip,rk3229-timer", "rockchip,rk3288-timer"'
to match devicetree bindings.
Signed-off-by: Alexander Kochetkov
---
arch/arm/boot/dts/rk3229-evb.dts |4
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/rk3229-evb.dts b/arch/arm/boot/dts/rk3229-evb.dts
index b6a1203..6629769 100644
--- a/arch/arm/boot/dts/rk3229-evb.dts
+++ b/arch/arm/boot/dts/rk3229-evb.dts
@@ -88,3 +88,7 @@
&uart2 {
status = "okay";
};
+
+&timer {
+ compatible = "rockchip,rk3229-timer", "rockchip,rk3288-timer";
+};
--
1.7.9.5
[PATCH v4 4/9] ARM: dts: rockchip: add timer entries to rk3188 SoC
The patch add two timers to all rk3188 based boards.
The first timer is from alive subsystem and it act as a backup
for the local timers at sleep time. It act the same as other
SoC rockchip timers already present in kernel.
The second timer is from CPU subsystem and act as replacement
for the arm-global-timer clocksource and sched clock. It run
at stable frequency 24MHz.
Signed-off-by: Alexander Kochetkov
---
arch/arm/boot/dts/rk3188.dtsi | 16
1 file changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index 31f81b2..0dc52fe 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -106,6 +106,22 @@
};
};
+ timer3: timer@2000e000 {
+ compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer";
+ reg = <0x2000e000 0x20>;
+ interrupts = ;
+ clocks = <&cru SCLK_TIMER3>, <&cru PCLK_TIMER3>;
+ clock-names = "timer", "pclk";
+ };
+
+ timer6: timer@200380a0 {
+ compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer";
+ reg = <0x200380a0 0x20>;
+ interrupts = ;
+ clocks = <&cru SCLK_TIMER6>, <&cru PCLK_TIMER0>;
+ clock-names = "timer", "pclk";
+ };
+
i2s0: i2s@1011a000 {
compatible = "rockchip,rk3188-i2s", "rockchip,rk3066-i2s";
reg = <0x1011a000 0x2000>;
--
1.7.9.5
