* [RFC PATCH 2/5] dmaengine: allow sun6i-dma for more SoCs
From: Andre Przywara @ 2016-11-24 11:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124105516.tv2ybfgka5teiv2h@lukather>
On 24/11/16 10:55, Maxime Ripard wrote:
> On Thu, Nov 24, 2016 at 05:30:45PM +0800, Chen-Yu Tsai wrote:
>> On Thu, Nov 24, 2016 at 5:16 PM, Andre Przywara <andre.przywara@arm.com> wrote:
>>> Hi,
>>>
>>> On 24/11/16 04:16, Chen-Yu Tsai wrote:
>>>> Hi,
>>>>
>>>> On Thu, Nov 24, 2016 at 9:17 AM, Andre Przywara <andre.przywara@arm.com> wrote:
>>>>> The sun6i DMA driver is used in the Allwinner A64 and H5 SoC, which
>>>>> have arm64 capable cores. Add the generic sunxi config symbol to allow
>>>>> the driver to be selected by arm64 Kconfigs, which don't feature
>>>>> SoC specific MACH_xxxx configs.
>>>>>
>>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>>> ---
>>>>> drivers/dma/Kconfig | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>>>>> index af63a6b..003c284 100644
>>>>> --- a/drivers/dma/Kconfig
>>>>> +++ b/drivers/dma/Kconfig
>>>>> @@ -157,7 +157,7 @@ config DMA_SUN4I
>>>>>
>>>>> config DMA_SUN6I
>>>>> tristate "Allwinner A31 SoCs DMA support"
>>>>> - depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST
>>>>> + depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST || ARCH_SUNXI
>>>>
>>>> AFAIK ARCH_SUNXI encompasses/supersedes MACH_SUN*I.
>>>> (And I don't have to add MACH_SUN9I later :) )
>>>
>>> Sure, admittedly it was just a quick hack to get things going.
>>> Actually I don't know why we had a *depend* on those MACH_s before. I
>>> think technically it does not depend on a certain SoC (having the
>>> COMPILE_TEST in there hints on that). So what about:
>>
>> It was really because this DMA engine only comes with the later
>> SoCs. We have dma-sun4i for the older one.
>
> Indeed.
>
>> But yes, there's no reason why you can't build it for the earlier
>> SoC. It just doesn't get used.
>
> I'm still in favor of keeping the depends on. There's no point of
> compiling something we know have zero chance of running.
>
> (But that would be (ARCH_SUNXI && ARM64))
I am OK with that, just wondering if there is a definition of what
"depends" really means. My impression what that it's a about code
dependencies (requires a certain subsystem, for instance), not really if
it's useful in a particular configuration.
Cheers,
Andre.
^ permalink raw reply
* [PATCH RFC] drm/sun4i: rgb: Add 5% tolerance to dot clock frequency check
From: Chen-Yu Tsai @ 2016-11-24 11:22 UTC (permalink / raw)
To: linux-arm-kernel
The panels shipped with Allwinner devices are very "generic", i.e.
they do not have model numbers or reliable sources of information
for the timings (that we know of) other than the fex files shipped
on them. The dot clock frequency provided in the fex files have all
been rounded to the nearest MHz, as that is the unit used in them.
We were using the simple panel "urt,umsh-8596md-t" as a substitute
for the A13 Q8 tablets in the absence of a specific model for what
may be many different but otherwise timing compatible panels. This
was usable without any visual artifacts or side effects, until the
dot clock rate check was added in commit bb43d40d7c83 ("drm/sun4i:
rgb: Validate the clock rate").
The reason this check fails is because the dotclock frequency for
this model is 33.26 MHz, which is not achievable with our dot clock
hardware, and the rate returned by clk_round_rate deviates slightly,
causing the driver to reject the display mode.
The LCD panels have some tolerance on the dot clock frequency, even
if it's not specified in their datasheets.
This patch adds a 5% tolerence to the dot clock check.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
The few LCD panel datasheets I found did not list minimums or maximums
for the dot clock rate. The 5% tolerance is just something I made up.
The point is to be able to use our dot clock, which doesn't have the
resolution needed to generate the exact clock rate requested. AFAIK
the sun4i driver is one of the strictest ones with regards to the dot
clock frequency. Some drivers don't even check it.
The clock rate given in vendor fex files are already rounded down to
MHz resolution. I doubt not using the exact rate as specified in simple
panels would cause any issues. But my experience is limited here.
Feedback on this is requested.
---
drivers/gpu/drm/sun4i/sun4i_rgb.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
index d198ad7e5323..66ad86afa561 100644
--- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
+++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
@@ -93,11 +93,12 @@ static int sun4i_rgb_mode_valid(struct drm_connector *connector,
DRM_DEBUG_DRIVER("Vertical parameters OK\n");
+ /* Check against a 5% tolerance for the dot clock */
rounded_rate = clk_round_rate(tcon->dclk, rate);
- if (rounded_rate < rate)
+ if (rounded_rate < rate * 19 / 20)
return MODE_CLOCK_LOW;
- if (rounded_rate > rate)
+ if (rounded_rate > rate * 21 / 20)
return MODE_CLOCK_HIGH;
DRM_DEBUG_DRIVER("Clock rate OK\n");
--
2.10.2
^ permalink raw reply related
* [PATCH 0/5] spi: atmel: enhance the DMA handling
From: Nicolas Ferre @ 2016-11-24 11:24 UTC (permalink / raw)
To: linux-arm-kernel
Use the dummy buffer handling and DMA mapping core infrastructure to simplify
spi-atmel driver.
By using these frameworks we also fix the use of any kind of memory with
dmaengine. This work was triggered by issues encountered while using UBI/UBIFS
on a serial flash.
Some good ideas of this series have been inspired by spi-rspi.c and commits by
Geert Uytterhoeven.
I have a side question still: is the is_dma_mapped tag still used and considered into
the core spi routines or is it always the driver itself that have to deal with
it (so how to do with the can_dma() routine)?
Cyrille Pitchen (1):
spi: atmel: Use SPI core DMA mapping framework
Nicolas Ferre (4):
spi: atmel: trivial: move info banner to latest probe action
spi: atmel: Use core SPI_MASTER_MUST_[RT]X handling
spi: atmel: trivial: remove unused fields in DMA structure
spi: atmel: remove the use of private channel fields
drivers/spi/spi-atmel.c | 268 ++++++++++++++++++------------------------------
1 file changed, 98 insertions(+), 170 deletions(-)
--
2.9.0
^ permalink raw reply
* [PATCH 1/5] spi: atmel: trivial: move info banner to latest probe action
From: Nicolas Ferre @ 2016-11-24 11:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1479985886.git.nicolas.ferre@atmel.com>
The info banner is here to tell that everything went well, so place
it at the very end of the probe function.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
drivers/spi/spi-atmel.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index a038ffe90766..68e22bf94647 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -1658,10 +1658,6 @@ static int atmel_spi_probe(struct platform_device *pdev)
spi_writel(as, CR, SPI_BIT(FIFOEN));
}
- /* go! */
- dev_info(&pdev->dev, "Atmel SPI Controller at 0x%08lx (irq %d)\n",
- (unsigned long)regs->start, irq);
-
pm_runtime_set_autosuspend_delay(&pdev->dev, AUTOSUSPEND_TIMEOUT);
pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_set_active(&pdev->dev);
@@ -1671,6 +1667,10 @@ static int atmel_spi_probe(struct platform_device *pdev)
if (ret)
goto out_free_dma;
+ /* go! */
+ dev_info(&pdev->dev, "Atmel SPI Controller at 0x%08lx (irq %d)\n",
+ (unsigned long)regs->start, irq);
+
return 0;
out_free_dma:
--
2.9.0
^ permalink raw reply related
* [PATCH 2/5] spi: atmel: Use core SPI_MASTER_MUST_[RT]X handling
From: Nicolas Ferre @ 2016-11-24 11:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1479985886.git.nicolas.ferre@atmel.com>
We need both RX and TX data for each transfer in any case (PIO, PDC, DMA).
So convert the driver to the core dummy buffer handling with the
SPI_MASTER_MUST_RX/SPI_MASTER_MUST_TX infrastructure.
This move changes the maximum PDC/DMA buffer handling to 65535 bytes
instead of a single page and sets master->max_dma_len to this value.
All dummy buffer management is removed from the driver.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
drivers/spi/spi-atmel.c | 131 +++++++++++++-----------------------------------
1 file changed, 35 insertions(+), 96 deletions(-)
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 68e22bf94647..d0a56e11e148 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -304,10 +304,6 @@ struct atmel_spi {
struct completion xfer_completion;
- /* scratch buffer */
- void *buffer;
- dma_addr_t buffer_dma;
-
struct atmel_spi_caps caps;
bool use_dma;
@@ -328,7 +324,7 @@ struct atmel_spi_device {
u32 csr;
};
-#define BUFFER_SIZE PAGE_SIZE
+#define SPI_MAX_DMA_XFER 65535 /* true for both PDC and DMA */
#define INVALID_DMA_ADDRESS 0xffffffff
/*
@@ -613,14 +609,10 @@ static void atmel_spi_next_xfer_single(struct spi_master *master,
cpu_relax();
}
- if (xfer->tx_buf) {
- if (xfer->bits_per_word > 8)
- spi_writel(as, TDR, *(u16 *)(xfer->tx_buf + xfer_pos));
- else
- spi_writel(as, TDR, *(u8 *)(xfer->tx_buf + xfer_pos));
- } else {
- spi_writel(as, TDR, 0);
- }
+ if (xfer->bits_per_word > 8)
+ spi_writel(as, TDR, *(u16 *)(xfer->tx_buf + xfer_pos));
+ else
+ spi_writel(as, TDR, *(u8 *)(xfer->tx_buf + xfer_pos));
dev_dbg(master->dev.parent,
" start pio xfer %p: len %u tx %p rx %p bitpw %d\n",
@@ -667,17 +659,12 @@ static void atmel_spi_next_xfer_fifo(struct spi_master *master,
/* Fill TX FIFO */
while (num_data >= 2) {
- if (xfer->tx_buf) {
- if (xfer->bits_per_word > 8) {
- td0 = *words++;
- td1 = *words++;
- } else {
- td0 = *bytes++;
- td1 = *bytes++;
- }
+ if (xfer->bits_per_word > 8) {
+ td0 = *words++;
+ td1 = *words++;
} else {
- td0 = 0;
- td1 = 0;
+ td0 = *bytes++;
+ td1 = *bytes++;
}
spi_writel(as, TDR, (td1 << 16) | td0);
@@ -685,14 +672,10 @@ static void atmel_spi_next_xfer_fifo(struct spi_master *master,
}
if (num_data) {
- if (xfer->tx_buf) {
- if (xfer->bits_per_word > 8)
- td0 = *words++;
- else
- td0 = *bytes++;
- } else {
- td0 = 0;
- }
+ if (xfer->bits_per_word > 8)
+ td0 = *words++;
+ else
+ td0 = *bytes++;
spi_writew(as, TDR, td0);
num_data--;
@@ -751,24 +734,14 @@ static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
/* prepare the RX dma transfer */
sg_init_table(&as->dma.sgrx, 1);
- if (xfer->rx_buf) {
- as->dma.sgrx.dma_address = xfer->rx_dma + xfer->len - *plen;
- } else {
- as->dma.sgrx.dma_address = as->buffer_dma;
- if (len > BUFFER_SIZE)
- len = BUFFER_SIZE;
- }
+ as->dma.sgrx.dma_address = xfer->rx_dma + xfer->len - *plen;
/* prepare the TX dma transfer */
sg_init_table(&as->dma.sgtx, 1);
- if (xfer->tx_buf) {
- as->dma.sgtx.dma_address = xfer->tx_dma + xfer->len - *plen;
- } else {
- as->dma.sgtx.dma_address = as->buffer_dma;
- if (len > BUFFER_SIZE)
- len = BUFFER_SIZE;
- memset(as->buffer, 0, len);
- }
+ as->dma.sgtx.dma_address = xfer->tx_dma + xfer->len - *plen;
+
+ if (len > master->max_dma_len)
+ len = master->max_dma_len;
sg_dma_len(&as->dma.sgtx) = len;
sg_dma_len(&as->dma.sgrx) = len;
@@ -835,25 +808,10 @@ static void atmel_spi_next_xfer_data(struct spi_master *master,
struct atmel_spi *as = spi_master_get_devdata(master);
u32 len = *plen;
- /* use scratch buffer only when rx or tx data is unspecified */
- if (xfer->rx_buf)
- *rx_dma = xfer->rx_dma + xfer->len - *plen;
- else {
- *rx_dma = as->buffer_dma;
- if (len > BUFFER_SIZE)
- len = BUFFER_SIZE;
- }
-
- if (xfer->tx_buf)
- *tx_dma = xfer->tx_dma + xfer->len - *plen;
- else {
- *tx_dma = as->buffer_dma;
- if (len > BUFFER_SIZE)
- len = BUFFER_SIZE;
- memset(as->buffer, 0, len);
- dma_sync_single_for_device(&as->pdev->dev,
- as->buffer_dma, len, DMA_TO_DEVICE);
- }
+ *rx_dma = xfer->rx_dma + xfer->len - *plen;
+ *tx_dma = xfer->tx_dma + xfer->len - *plen;
+ if (len > master->max_dma_len)
+ len = master->max_dma_len;
*plen = len;
}
@@ -1027,16 +985,12 @@ atmel_spi_pump_single_data(struct atmel_spi *as, struct spi_transfer *xfer)
u16 *rxp16;
unsigned long xfer_pos = xfer->len - as->current_remaining_bytes;
- if (xfer->rx_buf) {
- if (xfer->bits_per_word > 8) {
- rxp16 = (u16 *)(((u8 *)xfer->rx_buf) + xfer_pos);
- *rxp16 = spi_readl(as, RDR);
- } else {
- rxp = ((u8 *)xfer->rx_buf) + xfer_pos;
- *rxp = spi_readl(as, RDR);
- }
+ if (xfer->bits_per_word > 8) {
+ rxp16 = (u16 *)(((u8 *)xfer->rx_buf) + xfer_pos);
+ *rxp16 = spi_readl(as, RDR);
} else {
- spi_readl(as, RDR);
+ rxp = ((u8 *)xfer->rx_buf) + xfer_pos;
+ *rxp = spi_readl(as, RDR);
}
if (xfer->bits_per_word > 8) {
if (as->current_remaining_bytes > 2)
@@ -1075,12 +1029,10 @@ atmel_spi_pump_fifo_data(struct atmel_spi *as, struct spi_transfer *xfer)
/* Read data */
while (num_data) {
rd = spi_readl(as, RDR);
- if (xfer->rx_buf) {
- if (xfer->bits_per_word > 8)
- *words++ = rd;
- else
- *bytes++ = rd;
- }
+ if (xfer->bits_per_word > 8)
+ *words++ = rd;
+ else
+ *bytes++ = rd;
num_data--;
}
}
@@ -1562,29 +1514,22 @@ static int atmel_spi_probe(struct platform_device *pdev)
master->bus_num = pdev->id;
master->num_chipselect = master->dev.of_node ? 0 : 4;
master->setup = atmel_spi_setup;
+ master->flags = (SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX);
master->transfer_one_message = atmel_spi_transfer_one_message;
master->cleanup = atmel_spi_cleanup;
master->auto_runtime_pm = true;
+ master->max_dma_len = SPI_MAX_DMA_XFER;
platform_set_drvdata(pdev, master);
as = spi_master_get_devdata(master);
- /*
- * Scratch buffer is used for throwaway rx and tx data.
- * It's coherent to minimize dcache pollution.
- */
- as->buffer = dma_alloc_coherent(&pdev->dev, BUFFER_SIZE,
- &as->buffer_dma, GFP_KERNEL);
- if (!as->buffer)
- goto out_free;
-
spin_lock_init(&as->lock);
as->pdev = pdev;
as->regs = devm_ioremap_resource(&pdev->dev, regs);
if (IS_ERR(as->regs)) {
ret = PTR_ERR(as->regs);
- goto out_free_buffer;
+ goto out_unmap_regs;
}
as->phybase = regs->start;
as->irq = irq;
@@ -1685,9 +1630,6 @@ static int atmel_spi_probe(struct platform_device *pdev)
clk_disable_unprepare(clk);
out_free_irq:
out_unmap_regs:
-out_free_buffer:
- dma_free_coherent(&pdev->dev, BUFFER_SIZE, as->buffer,
- as->buffer_dma);
out_free:
spi_master_put(master);
return ret;
@@ -1712,9 +1654,6 @@ static int atmel_spi_remove(struct platform_device *pdev)
spi_readl(as, SR);
spin_unlock_irq(&as->lock);
- dma_free_coherent(&pdev->dev, BUFFER_SIZE, as->buffer,
- as->buffer_dma);
-
clk_disable_unprepare(as->clk);
pm_runtime_put_noidle(&pdev->dev);
--
2.9.0
^ permalink raw reply related
* [PATCH 3/5] spi: atmel: Use SPI core DMA mapping framework
From: Nicolas Ferre @ 2016-11-24 11:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1479985886.git.nicolas.ferre@atmel.com>
From: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Use the SPI core DMA mapping framework instead of our own
in case of DMA support. PDC support is not converted to this
framework.
The driver is now able to transfer a complete sg list through DMA.
This eventually fix an issue with vmalloc'ed DMA memory that is
provided for example by UBI/UBIFS layers.
Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
[nicolas.ferre at atmel.com: restrict the use to non-PDC DMA]
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
drivers/spi/spi-atmel.c | 57 ++++++++++++++++++++++---------------------------
1 file changed, 25 insertions(+), 32 deletions(-)
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index d0a56e11e148..f62bc2d27c9e 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -268,8 +268,6 @@
struct atmel_spi_dma {
struct dma_chan *chan_rx;
struct dma_chan *chan_tx;
- struct scatterlist sgrx;
- struct scatterlist sgtx;
struct dma_async_tx_descriptor *data_desc_rx;
struct dma_async_tx_descriptor *data_desc_tx;
@@ -454,6 +452,15 @@ static inline bool atmel_spi_use_dma(struct atmel_spi *as,
return as->use_dma && xfer->len >= DMA_MIN_BYTES;
}
+static bool atmel_spi_can_dma(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ struct atmel_spi *as = spi_master_get_devdata(master);
+
+ return atmel_spi_use_dma(as, xfer);
+}
+
static int atmel_spi_dma_slave_config(struct atmel_spi *as,
struct dma_slave_config *slave_config,
u8 bits_per_word)
@@ -721,7 +728,6 @@ static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
struct dma_async_tx_descriptor *txdesc;
struct dma_slave_config slave_config;
dma_cookie_t cookie;
- u32 len = *plen;
dev_vdbg(master->dev.parent, "atmel_spi_next_xfer_dma_submit\n");
@@ -732,34 +738,22 @@ static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
/* release lock for DMA operations */
atmel_spi_unlock(as);
- /* prepare the RX dma transfer */
- sg_init_table(&as->dma.sgrx, 1);
- as->dma.sgrx.dma_address = xfer->rx_dma + xfer->len - *plen;
-
- /* prepare the TX dma transfer */
- sg_init_table(&as->dma.sgtx, 1);
- as->dma.sgtx.dma_address = xfer->tx_dma + xfer->len - *plen;
-
- if (len > master->max_dma_len)
- len = master->max_dma_len;
-
- sg_dma_len(&as->dma.sgtx) = len;
- sg_dma_len(&as->dma.sgrx) = len;
-
- *plen = len;
+ *plen = xfer->len;
if (atmel_spi_dma_slave_config(as, &slave_config,
xfer->bits_per_word))
goto err_exit;
/* Send both scatterlists */
- rxdesc = dmaengine_prep_slave_sg(rxchan, &as->dma.sgrx, 1,
+ rxdesc = dmaengine_prep_slave_sg(rxchan,
+ xfer->rx_sg.sgl, xfer->rx_sg.nents,
DMA_FROM_DEVICE,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!rxdesc)
goto err_dma;
- txdesc = dmaengine_prep_slave_sg(txchan, &as->dma.sgtx, 1,
+ txdesc = dmaengine_prep_slave_sg(txchan,
+ xfer->tx_sg.sgl, xfer->tx_sg.nents,
DMA_TO_DEVICE,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!txdesc)
@@ -805,15 +799,10 @@ static void atmel_spi_next_xfer_data(struct spi_master *master,
dma_addr_t *rx_dma,
u32 *plen)
{
- struct atmel_spi *as = spi_master_get_devdata(master);
- u32 len = *plen;
-
*rx_dma = xfer->rx_dma + xfer->len - *plen;
*tx_dma = xfer->tx_dma + xfer->len - *plen;
- if (len > master->max_dma_len)
- len = master->max_dma_len;
-
- *plen = len;
+ if (*plen > master->max_dma_len)
+ *plen = master->max_dma_len;
}
static int atmel_spi_set_xfer_speed(struct atmel_spi *as,
@@ -1253,7 +1242,7 @@ static int atmel_spi_one_transfer(struct spi_master *master,
* better fault reporting.
*/
if ((!msg->is_dma_mapped)
- && (atmel_spi_use_dma(as, xfer) || as->use_pdc)) {
+ && as->use_pdc) {
if (atmel_spi_dma_map_xfer(as, xfer) < 0)
return -ENOMEM;
}
@@ -1330,7 +1319,7 @@ static int atmel_spi_one_transfer(struct spi_master *master,
}
if (!msg->is_dma_mapped
- && (atmel_spi_use_dma(as, xfer) || as->use_pdc))
+ && as->use_pdc)
atmel_spi_dma_unmap_xfer(master, xfer);
return 0;
@@ -1341,7 +1330,7 @@ static int atmel_spi_one_transfer(struct spi_master *master,
}
if (!msg->is_dma_mapped
- && (atmel_spi_use_dma(as, xfer) || as->use_pdc))
+ && as->use_pdc)
atmel_spi_dma_unmap_xfer(master, xfer);
if (xfer->delay_usecs)
@@ -1519,6 +1508,7 @@ static int atmel_spi_probe(struct platform_device *pdev)
master->cleanup = atmel_spi_cleanup;
master->auto_runtime_pm = true;
master->max_dma_len = SPI_MAX_DMA_XFER;
+ master->can_dma = atmel_spi_can_dma;
platform_set_drvdata(pdev, master);
as = spi_master_get_devdata(master);
@@ -1555,10 +1545,13 @@ static int atmel_spi_probe(struct platform_device *pdev)
as->use_pdc = false;
if (as->caps.has_dma_support) {
ret = atmel_spi_configure_dma(as);
- if (ret == 0)
+ if (ret == 0) {
+ master->dma_tx = as->dma.chan_tx;
+ master->dma_rx = as->dma.chan_rx;
as->use_dma = true;
- else if (ret == -EPROBE_DEFER)
+ } else if (ret == -EPROBE_DEFER) {
return ret;
+ }
} else {
as->use_pdc = true;
}
--
2.9.0
^ permalink raw reply related
* [PATCH 4/5] spi: atmel: trivial: remove unused fields in DMA structure
From: Nicolas Ferre @ 2016-11-24 11:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1479985886.git.nicolas.ferre@atmel.com>
The atmel_spi_dma structure was cluttered with unused fields relative
to older DMA channel selection API. Remove them.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
drivers/spi/spi-atmel.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index f62bc2d27c9e..f3a225585575 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -268,10 +268,6 @@
struct atmel_spi_dma {
struct dma_chan *chan_rx;
struct dma_chan *chan_tx;
- struct dma_async_tx_descriptor *data_desc_rx;
- struct dma_async_tx_descriptor *data_desc_tx;
-
- struct at_dma_slave dma_slave;
};
struct atmel_spi_caps {
--
2.9.0
^ permalink raw reply related
* [PATCH 5/5] spi: atmel: remove the use of private channel fields
From: Nicolas Ferre @ 2016-11-24 11:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1479985886.git.nicolas.ferre@atmel.com>
For DMA transfers, we now use the core DMA framework which provides
channel fields in the spi_master structure. Remove the private channels
from atmel_spi stucture which were located in a sub-structure. This
last one (atmel_spi_dma) which is now empty is also removed.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
drivers/spi/spi-atmel.c | 86 ++++++++++++++++++++++++-------------------------
1 file changed, 43 insertions(+), 43 deletions(-)
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index f3a225585575..0e7712bac3b6 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -265,11 +265,6 @@
#define AUTOSUSPEND_TIMEOUT 2000
-struct atmel_spi_dma {
- struct dma_chan *chan_rx;
- struct dma_chan *chan_tx;
-};
-
struct atmel_spi_caps {
bool is_spi2;
bool has_wdrbt;
@@ -303,8 +298,6 @@ struct atmel_spi {
bool use_dma;
bool use_pdc;
bool use_cs_gpios;
- /* dmaengine data */
- struct atmel_spi_dma dma;
bool keep_cs;
bool cs_active;
@@ -461,6 +454,7 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,
struct dma_slave_config *slave_config,
u8 bits_per_word)
{
+ struct spi_master *master = platform_get_drvdata(as->pdev);
int err = 0;
if (bits_per_word > 8) {
@@ -492,7 +486,7 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,
* path works the same whether FIFOs are available (and enabled) or not.
*/
slave_config->direction = DMA_MEM_TO_DEV;
- if (dmaengine_slave_config(as->dma.chan_tx, slave_config)) {
+ if (dmaengine_slave_config(master->dma_tx, slave_config)) {
dev_err(&as->pdev->dev,
"failed to configure tx dma channel\n");
err = -EINVAL;
@@ -507,7 +501,7 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,
* enabled) or not.
*/
slave_config->direction = DMA_DEV_TO_MEM;
- if (dmaengine_slave_config(as->dma.chan_rx, slave_config)) {
+ if (dmaengine_slave_config(master->dma_rx, slave_config)) {
dev_err(&as->pdev->dev,
"failed to configure rx dma channel\n");
err = -EINVAL;
@@ -516,7 +510,8 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,
return err;
}
-static int atmel_spi_configure_dma(struct atmel_spi *as)
+static int atmel_spi_configure_dma(struct spi_master *master,
+ struct atmel_spi *as)
{
struct dma_slave_config slave_config;
struct device *dev = &as->pdev->dev;
@@ -526,26 +521,26 @@ static int atmel_spi_configure_dma(struct atmel_spi *as)
dma_cap_zero(mask);
dma_cap_set(DMA_SLAVE, mask);
- as->dma.chan_tx = dma_request_slave_channel_reason(dev, "tx");
- if (IS_ERR(as->dma.chan_tx)) {
- err = PTR_ERR(as->dma.chan_tx);
+ master->dma_tx = dma_request_slave_channel_reason(dev, "tx");
+ if (IS_ERR(master->dma_tx)) {
+ err = PTR_ERR(master->dma_tx);
if (err == -EPROBE_DEFER) {
dev_warn(dev, "no DMA channel available at the moment\n");
- return err;
+ goto error_clear;
}
dev_err(dev,
"DMA TX channel not available, SPI unable to use DMA\n");
err = -EBUSY;
- goto error;
+ goto error_clear;
}
/*
* No reason to check EPROBE_DEFER here since we have already requested
* tx channel. If it fails here, it's for another reason.
*/
- as->dma.chan_rx = dma_request_slave_channel(dev, "rx");
+ master->dma_rx = dma_request_slave_channel(dev, "rx");
- if (!as->dma.chan_rx) {
+ if (!master->dma_rx) {
dev_err(dev,
"DMA RX channel not available, SPI unable to use DMA\n");
err = -EBUSY;
@@ -558,31 +553,38 @@ static int atmel_spi_configure_dma(struct atmel_spi *as)
dev_info(&as->pdev->dev,
"Using %s (tx) and %s (rx) for DMA transfers\n",
- dma_chan_name(as->dma.chan_tx),
- dma_chan_name(as->dma.chan_rx));
+ dma_chan_name(master->dma_tx),
+ dma_chan_name(master->dma_rx));
+
return 0;
error:
- if (as->dma.chan_rx)
- dma_release_channel(as->dma.chan_rx);
- if (!IS_ERR(as->dma.chan_tx))
- dma_release_channel(as->dma.chan_tx);
+ if (master->dma_rx)
+ dma_release_channel(master->dma_rx);
+ if (!IS_ERR(master->dma_tx))
+ dma_release_channel(master->dma_tx);
+error_clear:
+ master->dma_tx = master->dma_rx = NULL;
return err;
}
-static void atmel_spi_stop_dma(struct atmel_spi *as)
+static void atmel_spi_stop_dma(struct spi_master *master)
{
- if (as->dma.chan_rx)
- dmaengine_terminate_all(as->dma.chan_rx);
- if (as->dma.chan_tx)
- dmaengine_terminate_all(as->dma.chan_tx);
+ if (master->dma_rx)
+ dmaengine_terminate_all(master->dma_rx);
+ if (master->dma_tx)
+ dmaengine_terminate_all(master->dma_tx);
}
-static void atmel_spi_release_dma(struct atmel_spi *as)
+static void atmel_spi_release_dma(struct spi_master *master)
{
- if (as->dma.chan_rx)
- dma_release_channel(as->dma.chan_rx);
- if (as->dma.chan_tx)
- dma_release_channel(as->dma.chan_tx);
+ if (master->dma_rx) {
+ dma_release_channel(master->dma_rx);
+ master->dma_rx = NULL;
+ }
+ if (master->dma_tx) {
+ dma_release_channel(master->dma_tx);
+ master->dma_tx = NULL;
+ }
}
/* This function is called by the DMA driver from tasklet context */
@@ -718,8 +720,8 @@ static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
u32 *plen)
{
struct atmel_spi *as = spi_master_get_devdata(master);
- struct dma_chan *rxchan = as->dma.chan_rx;
- struct dma_chan *txchan = as->dma.chan_tx;
+ struct dma_chan *rxchan = master->dma_rx;
+ struct dma_chan *txchan = master->dma_tx;
struct dma_async_tx_descriptor *rxdesc;
struct dma_async_tx_descriptor *txdesc;
struct dma_slave_config slave_config;
@@ -783,7 +785,7 @@ static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
err_dma:
spi_writel(as, IDR, SPI_BIT(OVRES));
- atmel_spi_stop_dma(as);
+ atmel_spi_stop_dma(master);
err_exit:
atmel_spi_lock(as);
return -ENOMEM;
@@ -1311,7 +1313,7 @@ static int atmel_spi_one_transfer(struct spi_master *master,
spi_readl(as, SR);
} else if (atmel_spi_use_dma(as, xfer)) {
- atmel_spi_stop_dma(as);
+ atmel_spi_stop_dma(master);
}
if (!msg->is_dma_mapped
@@ -1540,10 +1542,8 @@ static int atmel_spi_probe(struct platform_device *pdev)
as->use_dma = false;
as->use_pdc = false;
if (as->caps.has_dma_support) {
- ret = atmel_spi_configure_dma(as);
+ ret = atmel_spi_configure_dma(master, as);
if (ret == 0) {
- master->dma_tx = as->dma.chan_tx;
- master->dma_rx = as->dma.chan_rx;
as->use_dma = true;
} else if (ret == -EPROBE_DEFER) {
return ret;
@@ -1612,7 +1612,7 @@ static int atmel_spi_probe(struct platform_device *pdev)
pm_runtime_set_suspended(&pdev->dev);
if (as->use_dma)
- atmel_spi_release_dma(as);
+ atmel_spi_release_dma(master);
spi_writel(as, CR, SPI_BIT(SWRST));
spi_writel(as, CR, SPI_BIT(SWRST)); /* AT91SAM9263 Rev B workaround */
@@ -1634,8 +1634,8 @@ static int atmel_spi_remove(struct platform_device *pdev)
/* reset the hardware and block queue progress */
spin_lock_irq(&as->lock);
if (as->use_dma) {
- atmel_spi_stop_dma(as);
- atmel_spi_release_dma(as);
+ atmel_spi_stop_dma(master);
+ atmel_spi_release_dma(master);
}
spi_writel(as, CR, SPI_BIT(SWRST));
--
2.9.0
^ permalink raw reply related
* [PATCH] ARM: dts: sun6i: hummingbird: Enable USB OTG
From: Chen-Yu Tsai @ 2016-11-24 11:29 UTC (permalink / raw)
To: linux-arm-kernel
The A31 Hummingbird has a mini USB OTG port, and uses GPIO pins from the
SoC for ID pin and VBUS detection and VBUS control. The PMIC can also do
VBUS detection and control.
Here we prefer to use the PMIC's DRIVEVBUS function to control VBUS for
USB OTG, as that is the hardware default.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
index 83643bbd51dc..f094eeb6c499 100644
--- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
+++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
@@ -248,6 +248,7 @@
reg = <0x68>;
interrupt-parent = <&nmi_intc>;
interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+ x-powers,drive-vbus-en;
};
};
@@ -306,6 +307,11 @@
regulator-name = "vcc-dram";
};
+®_drivevbus {
+ regulator-name = "usb0-vbus";
+ status = "okay";
+};
+
®_usb1_vbus {
gpio = <&pio 7 24 GPIO_ACTIVE_HIGH>; /* PH24 */
status = "okay";
@@ -330,12 +336,25 @@
status = "okay";
};
+&usb_otg {
+ dr_mode = "otg";
+ status = "okay";
+};
+
+&usb_power_supply {
+ status = "okay";
+};
+
&usb1_vbus_pin_a {
/* different pin from sunxi-common-regulators */
pins = "PH24";
};
&usbphy {
+ usb0_id_det-gpio = <&pio 0 15 GPIO_ACTIVE_HIGH>; /* PA15 */
+ usb0_vbus_det-gpio = <&pio 0 16 GPIO_ACTIVE_HIGH>; /* PA16 */
+ usb0_vbus_power-supply = <&usb_power_supply>;
+ usb0_vbus-supply = <®_drivevbus>;
usb1_vbus-supply = <®_usb1_vbus>;
status = "okay";
};
--
2.10.2
^ permalink raw reply related
* [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ulf Hansson @ 2016-11-24 11:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a05ffd140f4edc02fc3128db8445b2264cf38723.1477911954.git-series.gregory.clement@free-electrons.com>
On 31 October 2016 at 12:09, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> From: Ziji Hu <huziji@marvell.com>
>
> Marvell Xenon eMMC/SD/SDIO Host Controller contains PHY.
> Three types of PHYs are supported.
>
> Add support to multiple types of PHYs init and configuration.
> Add register definitions of PHYs.
>
> Signed-off-by: Hu Ziji <huziji@marvell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> MAINTAINERS | 2 +-
> drivers/mmc/host/Makefile | 2 +-
> drivers/mmc/host/sdhci-xenon-phy.c | 1181 +++++++++++++++++++++++++++++-
> drivers/mmc/host/sdhci-xenon-phy.h | 157 ++++-
> drivers/mmc/host/sdhci-xenon.c | 4 +-
> drivers/mmc/host/sdhci-xenon.h | 17 +-
> 6 files changed, 1361 insertions(+), 2 deletions(-)
> create mode 100644 drivers/mmc/host/sdhci-xenon-phy.c
> create mode 100644 drivers/mmc/host/sdhci-xenon-phy.h
Can you please consider to split this up somehow!? It would make it
easier to review...
Anyway, allow me to provide some initial feedback, particularly around
those things that Adrian and you requested for my input.
[...]
>
> +
> +static int __xenon_emmc_delay_adj_test(struct mmc_card *card)
> +{
> + int err;
> + u8 *ext_csd = NULL;
> +
> + err = mmc_get_ext_csd(card, &ext_csd);
> + kfree(ext_csd);
Why do you read the ext csd here?
> +
> + return err;
> +}
> +
> +static int __xenon_sdio_delay_adj_test(struct mmc_card *card)
> +{
> + struct mmc_command cmd = {0};
> + int err;
> +
> + cmd.opcode = SD_IO_RW_DIRECT;
> + cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
> +
> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
> + if (err)
> + return err;
> +
> + if (cmd.resp[0] & R5_ERROR)
> + return -EIO;
> + if (cmd.resp[0] & R5_FUNCTION_NUMBER)
> + return -EINVAL;
> + if (cmd.resp[0] & R5_OUT_OF_RANGE)
> + return -ERANGE;
> + return 0;
No thanks! MMC/SD/SDIO protocol code belongs in the core.
> +}
> +
> +static int __xenon_sd_delay_adj_test(struct mmc_card *card)
> +{
> + struct mmc_command cmd = {0};
> + int err;
> +
> + cmd.opcode = MMC_SEND_STATUS;
> + cmd.arg = card->rca << 16;
> + cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
> +
> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
> + return err;
No thanks! MMC/SD/SDIO protocol code belongs in the core.
> +}
> +
[...]
> +int xenon_phy_adj(struct sdhci_host *host, struct mmc_ios *ios)
> +{
> + struct mmc_host *mmc = host->mmc;
> + struct mmc_card *card;
> + int ret = 0;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> + if (!host->clock) {
> + priv->clock = 0;
> + return 0;
> + }
> +
> + /*
> + * The timing, frequency or bus width is changed,
> + * better to set eMMC PHY based on current setting
> + * and adjust Xenon SDHC delay.
> + */
> + if ((host->clock == priv->clock) &&
> + (ios->bus_width == priv->bus_width) &&
> + (ios->timing == priv->timing))
> + return 0;
> +
> + xenon_phy_set(host, ios->timing);
> +
> + /* Update the record */
> + priv->bus_width = ios->bus_width;
> + /* Temp stage from HS200 to HS400 */
> + if (((priv->timing == MMC_TIMING_MMC_HS200) &&
> + (ios->timing == MMC_TIMING_MMC_HS)) ||
> + ((ios->timing == MMC_TIMING_MMC_HS) &&
> + (priv->clock > host->clock))) {
> + priv->timing = ios->timing;
> + priv->clock = host->clock;
> + return 0;
> + }
> + /*
> + * Skip temp stages from HS400 t0 HS200:
> + * from 200MHz to 52MHz in HS400
> + * from HS400 to HS DDR in 52MHz
> + * from HS DDR to HS in 52MHz
> + * from HS to HS200 in 52MHz
> + */
> + if (((priv->timing == MMC_TIMING_MMC_HS400) &&
> + ((host->clock == MMC_HIGH_52_MAX_DTR) ||
> + (ios->timing == MMC_TIMING_MMC_DDR52))) ||
> + ((priv->timing == MMC_TIMING_MMC_DDR52) &&
> + (ios->timing == MMC_TIMING_MMC_HS)) ||
> + ((ios->timing == MMC_TIMING_MMC_HS200) &&
> + (ios->clock == MMC_HIGH_52_MAX_DTR))) {
> + priv->timing = ios->timing;
> + priv->clock = host->clock;
> + return 0;
> + }
> + priv->timing = ios->timing;
> + priv->clock = host->clock;
> +
> + /* Legacy mode is a special case */
> + if (ios->timing == MMC_TIMING_LEGACY)
> + return 0;
> +
> + if (mmc->card)
> + card = mmc->card;
> + else
> + /*
> + * Only valid during initialization
> + * before mmc->card is set
> + */
> + card = priv->card_candidate;
> + if (unlikely(!card)) {
> + dev_warn(mmc_dev(mmc), "card is not present\n");
> + return -EINVAL;
> + }
That your host need to hold a copy of the card pointer, tells me that
something is not really correct.
I might be wrong, if this turns out to be a special case, but I doubt
it. Although, if it *is* a special such case, we shall most likely try
to extend the the mmc core layer instead of adding all these hacks in
your host driver.
[...]
Another suggestion of a general improvement; could you perhaps try to
add some brief information about what goes on in function headers.
Perhaps that could help to more easily understand things.
Kind regards
Uffe
^ permalink raw reply
* [PATCH resend v3] ASoC: sun4i-codec: Add "Right Mixer" to "Line Out Mono Diff." route
From: Chen-Yu Tsai @ 2016-11-24 11:46 UTC (permalink / raw)
To: linux-arm-kernel
The mono differential output for "Line Out" downmixes the stereo audio
from the mixer, instead of just taking the left channel.
Add a route from the "Right Mixer" to "Line Out Source Playback Route"
through the "Mono Differential" path, so DAPM doesn't shut down
everything if the left channel is muted.
Fixes: 0f909f98d7cb ("ASoC: sun4i-codec: Add support for A31 Line Out
playback")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
This patch seems to have fallen through the cracks.
---
sound/soc/sunxi/sun4i-codec.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 092fdcf6de95..b11b0ad4dfbe 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -1047,6 +1047,7 @@ static const struct snd_soc_dapm_route sun6i_codec_codec_dapm_routes[] = {
{ "Line Out Source Playback Route", "Stereo", "Left Mixer" },
{ "Line Out Source Playback Route", "Stereo", "Right Mixer" },
{ "Line Out Source Playback Route", "Mono Differential", "Left Mixer" },
+ { "Line Out Source Playback Route", "Mono Differential", "Right Mixer" },
{ "LINEOUT", NULL, "Line Out Source Playback Route" },
/* ADC Routes */
--
2.10.2
^ permalink raw reply related
* [PATCH resend v3] ASoC: sun4i-codec: Add "Right Mixer" to "Line Out Mono Diff." route
From: Mark Brown @ 2016-11-24 11:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124114649.5188-1-wens@csie.org>
On Thu, Nov 24, 2016 at 07:46:49PM +0800, Chen-Yu Tsai wrote:
> This patch seems to have fallen through the cracks.
No, nobody has reviewed it and normally there's some review for sunxi
patches.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161124/e22b4256/attachment.sig>
^ permalink raw reply
* [PATCH 1/9] clocksource/drivers/rockchip_timer: split bc_timer into rk_timer and rk_clock_event_device
From: Heiko Stübner @ 2016-11-24 12:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479922177-20136-1-git-send-email-al.kochet@gmail.com>
Hi Alexander,
I haven't looked to deep into your patches yet, but what is missing is the
general goal of your whole series.
git format-patch has this nice "--cover-letter" option that creates obviously
a cover-letter where you can describe what your series wants to achieve.
For those reading along, I guess what you want to achieve should be what I
describe below, so of course no need to resend just for this :-)
-----
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.
-----
Right?
Heiko
Am Mittwoch, 23. November 2016, 20:29:29 schrieb Alexander Kochetkov:
> Move ce field out of struct bc_timer into struct rk_clock_event_device,
> rename struct bc_timer to struct rk_timer.
>
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
^ permalink raw reply
* [PATCH 1/9] clocksource/drivers/rockchip_timer: split bc_timer into rk_timer and rk_clock_event_device
From: Alexander Kochetkov @ 2016-11-24 12:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <47746094.VbGlCfGZW0@diego>
Hello Heiko,
> I haven't looked to deep into your patches yet, but what is missing is the
> general goal of your whole series.
I will be very grateful to receive feedback
> 24 ????. 2016 ?., ? 15:01, Heiko St?bner <heiko@sntech.de> ???????(?):
>
> git format-patch has this nice "--cover-letter" option that creates obviously
> a cover-letter where you can describe what your series wants to achieve.
>
I?ve used --cover-letter option but forget to add message text. Just subject.
So all patches was sent without cover letter. I?ll be careful next time.
> For those reading along, I guess what you want to achieve should be what I
> describe below, so of course no need to resend just for this :-)
>
> -----
> 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.
> -----
>
> Right?
Yes, exactly as you wrote.
Regards,
Alexander.
^ permalink raw reply
* [PATCH v4 1/6] arm64: arch_timer: Add device tree binding for hisilicon-161601 erratum
From: John Garry @ 2016-11-24 12:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c3020c0d-bc45-2bb1-f53a-c5e76ab45499@huawei.com>
On 21/11/2016 12:49, Ding Tianhong wrote:
> Ping....
Hi,
was there a cover letter for 0/6? I never saw it.
Thanks,
John
>
> On 2016/11/15 20:16, Ding Tianhong wrote:
>> This erratum describes a bug in logic outside the core, so MIDR can't be
>> used to identify its presence, and reading an SoC-specific revision
>> register from common arch timer code would be awkward. So, describe it
>> in the device tree.
>>
>> v2: Use the new erratum name and update the description.
>>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> Acked-by: Rob Herring <robh@kernel.org>
>> ---
>> Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
>> index ef5fbe9..c27b2c4 100644
>> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
>> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
>> @@ -31,6 +31,14 @@ to deliver its interrupts via SPIs.
>> This also affects writes to the tval register, due to the implicit
>> counter read.
>>
>> +- hisilicon,erratum-161601 : A boolean property. Indicates the presence of
>> + erratum 161601, which says that reading the counter is unreliable unless
>> + reading twice on the register and the value of the second read is larger
>> + than the first by less than 32. If the verification is unsuccessful, then
>> + discard the value of this read and repeat this procedure until the verification
>> + is successful. This also affects writes to the tval register, due to the
>> + implicit counter read.
>> +
>> ** Optional properties:
>>
>> - arm,cpu-registers-not-fw-configured : Firmware does not initialize
>>
>
> _______________________________________________
> linuxarm mailing list
> linuxarm at huawei.com
> http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm
>
> .
>
^ permalink raw reply
* [PATCH 7/9] clocksource/drivers/rockchip_timer: implement clocksource timer
From: Heiko Stübner @ 2016-11-24 12:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <FFD6E644-F68B-4880-BC76-88F6063D070E@gmail.com>
Am Donnerstag, 24. November 2016, 12:36:20 schrieb Alexander Kochetkov:
> > In order to use the patch you have to setup the timer using
> > 'rockchip,clocksource' device tree property
>
> Just came in mind, that it is better to replace 'rockchip,clocksource'
> device tree property with KConfig option in order to enable clocksource on
> dedicated timer?
>
> Someting like:
> [ ] enable clocksource
> clocksource timer name:
That would mean recompiling the kernel for a maybe board-specific setting and
is definitly not how things are handled these days :-) .
I.e. the overall goal is to have one kernel image that can actually run on
multiple arm architectures (rockchip, imx, etc) and only gets configured by the
devicetree.
In your dts-patch you reuse the rk3288-timer compatible value, which is also
non-ideal.
What you may want to do is introduce a rockchip,rk3188-timer compatible and
then make the timer-driver act accordingly, as you then know you are on a
rk3188-board ... see drivers attaching specific structs to the of_device_id
entries. From the documentation it also shouldn't really matter which timer
you use as clocksource, as on the rk3188 it seems all of them act the same way
(except timer3 being always on).
When touching devicetree-properties, please also adapt the binding document
Documentation/devicetree/bindings/timer,rockchip,rk-timer.txt
in this case and also include the devicetree maintainers.
Heiko
^ permalink raw reply
* ath9k ARMv7 OOPS in v4.8.6, v4.2.8
From: Jason Cooper @ 2016-11-24 12:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7aba95bdf0d562f3c9640d533c33e1ca@codeaurora.org>
On Thu, Nov 24, 2016 at 02:06:57PM +0800, miaoqing at codeaurora.org wrote:
>
> >>Okay, so i was 0, so running UP probably isn't going to help. r7 is
> >>also spec_priv->rfs_chan_spec_scan.
> >>
> >>So, I think the question is... how is this NULL - and has it always
> >>been NULL...
> >
> >The problem appears to be that ath_cmn_process_fft() isn't called that
> >often. When it is, it crashes in ath_cmn_is_fft_buf_full() because
> >spec_priv->rfs_chan_spec_scan is NULL when ATH9K_DEBUGFS=n. :-(
> >
> >I'm running with ATH9K_DEBUGFS=y now. If it goes a couple of days
> >without crashing, I'll gin up a patch.
> >
>
> A similar patch was applied to ath-next branch:
> https://patchwork.kernel.org/patch/9431163/.
Hmm. Ok, I'm giving it a spin on my board with SMP=y, ATH9K_DEBUGFS=n
(so the only change from known crashing is the patch) and we'll see how
it goes.
Honestly, though, I think the real problem is when kernels are built
without ATH9K_DEBUGFS. Did the reporter of the crash say if that was
enabled on his system or not?
I'm concerned that there may be other code lurking that secretly depends
on ATH9K_DEBUGFS being enabled.
thx,
Jason.
^ permalink raw reply
* ath9k ARMv7 OOPS in v4.8.6, v4.2.8
From: Jason Cooper @ 2016-11-24 12:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161123214053.GJ2799@io.lakedaemon.net>
All,
On Wed, Nov 23, 2016 at 09:40:53PM +0000, Jason Cooper wrote:
> I'm running with ATH9K_DEBUGFS=y now. If it goes a couple of days
> without crashing, I'll gin up a patch.
Well, it survived overnight, which it's never done before. :-) I'm
testing the relay_open() NULL patch now.
thx,
Jason.
^ permalink raw reply
* [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Ziji Hu @ 2016-11-24 12:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFpqPSqiEi=0UW5LoZmy+y-KHm9nbcGKBSy3RzchdLU9cA@mail.gmail.com>
Hi Ulf,
On 2016/11/24 18:43, Ulf Hansson wrote:
> On 31 October 2016 at 12:09, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
>> From: Ziji Hu <huziji@marvell.com>
>>
<snip>
>> +static int xenon_emmc_signal_voltage_switch(struct mmc_host *mmc,
>> + struct mmc_ios *ios)
>> +{
>> + unsigned char voltage = ios->signal_voltage;
>> +
>> + if ((voltage == MMC_SIGNAL_VOLTAGE_330) ||
>> + (voltage == MMC_SIGNAL_VOLTAGE_180))
>> + return __emmc_signal_voltage_switch(mmc, voltage);
>> +
>> + dev_err(mmc_dev(mmc), "Unsupported signal voltage: %d\n",
>> + voltage);
>> + return -EINVAL;
>
> This wrapper function seems unnessarry. It only adds a dev_err(), so
> then might as well do that in __emmc_signal_voltage_switch().
>
Sure. Will merge it back to __emmc_signal_voltage_switch().
>> +}
>> +
>> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
>> + struct mmc_ios *ios)
>> +{
>> + struct sdhci_host *host = mmc_priv(mmc);
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> +
>> + /*
>> + * Before SD/SDIO set signal voltage, SD bus clock should be
>> + * disabled. However, sdhci_set_clock will also disable the Internal
>> + * clock in mmc_set_signal_voltage().
>
> If that's the case then that is wrong in the generic sdhci code.
> What's the reason why it can't be fixed there instead of having this
> workaround?
>
In my very own opinion, SD Spec doesn't specify whether SDCLK should be
enabled or not during power setting.
Enabling SDCLK might be a special condition only required by our SDHC.
I try to avoid breaking other vendors' SDHC functionality
if their SDHCs require SDCLK disabled.
Thus I prefer to keep it inside our SDHC driver.
>> + * If Internal clock is disabled, the 3.3V/1.8V bit can not be updated.
>> + * Thus here manually enable internal clock.
>> + *
>> + * After switch completes, it is unnecessary to disable internal clock,
>> + * since keeping internal clock active obeys SD spec.
>> + */
>> + enable_xenon_internal_clk(host);
>> +
>> + if (priv->emmc_slot)
>> + return xenon_emmc_signal_voltage_switch(mmc, ios);
>> +
>> + return sdhci_start_signal_voltage_switch(mmc, ios);
>> +}
>> +
>> +/*
>> + * After determining which slot is used for SDIO,
>> + * some additional task is required.
>> + */
>> +static void xenon_init_card(struct mmc_host *mmc, struct mmc_card *card)
>> +{
>> + struct sdhci_host *host = mmc_priv(mmc);
>> + u32 reg;
>> + u8 slot_idx;
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> +
>> + /* Link the card for delay adjustment */
>> + priv->card_candidate = card;
>> + /* Set tuning functionality of this slot */
>> + xenon_slot_tuning_setup(host);
>
> This looks weird. I assume this can be done as a part of the regular
> tuning seqeunce!?
>
It is our SDHC specific preparation prior to tuning, rather than a
standard step in spec.
Thus I leave it inside our driver.
>> +
>> + slot_idx = priv->slot_idx;
>> + if (!mmc_card_sdio(card)) {
>> + /* Clear SDIO Card Inserted indication */
>
> Why do you need this?
>
> If you need to reset this, I think it's better to do it from
> ->set_ios() at MMC_POWER_OFF.
>
This field indicates SDIO card and controls async interrupt feature
of SDIO in our SDHC.
This async interrupt feature is enabled when SDIO card is inserted.
It should be disabled if SD card is inserted instead.
>> + reg = sdhci_readl(host, SDHC_SYS_CFG_INFO);
>> + reg &= ~(1 << (slot_idx + SLOT_TYPE_SDIO_SHIFT));
>> + sdhci_writel(host, reg, SDHC_SYS_CFG_INFO);
>> +
>> + if (mmc_card_mmc(card)) {
>> + mmc->caps |= MMC_CAP_NONREMOVABLE;
>> + if (!(host->quirks2 & SDHCI_QUIRK2_NO_1_8_V))
>> + mmc->caps |= MMC_CAP_1_8V_DDR;
>> + /*
>> + * Force to clear BUS_TEST to
>> + * skip bus_test_pre and bus_test_post
>> + */
>> + mmc->caps &= ~MMC_CAP_BUS_WIDTH_TEST;
>> + mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ |
>> + MMC_CAP2_PACKED_CMD;
>> + if (mmc->caps & MMC_CAP_8_BIT_DATA)
>> + mmc->caps2 |= MMC_CAP2_HS400_1_8V;
>
> Most of this can be specified as DT configurations. Please use that instead.
>
> More importantly, please don't use the ->init_card() ops to assign
> host caps. If not DT, please do it from ->probe().
>
Sure. Will try to use DT instead.
>> + }
>> + } else {
>> + /*
>> + * Set SDIO Card Inserted indication
>> + * to inform that the current slot is for SDIO
>> + */
>> + reg = sdhci_readl(host, SDHC_SYS_CFG_INFO);
>> + reg |= (1 << (slot_idx + SLOT_TYPE_SDIO_SHIFT));
>> + sdhci_writel(host, reg, SDHC_SYS_CFG_INFO);
>
> So this makes sence to have in the ->init_card() ops. The rest above, not.
>
>> + }
>> +}
>> +
>> +static int xenon_execute_tuning(struct mmc_host *mmc, u32 opcode)
>> +{
>> + struct sdhci_host *host = mmc_priv(mmc);
>> +
>> + if (host->timing == MMC_TIMING_UHS_DDR50)
>> + return 0;
>> +
>> + return sdhci_execute_tuning(mmc, opcode);
>> +}
>> +
>> +static void xenon_replace_mmc_host_ops(struct sdhci_host *host)
>> +{
>> + host->mmc_host_ops.set_ios = xenon_set_ios;
>> + host->mmc_host_ops.start_signal_voltage_switch =
>> + xenon_start_signal_voltage_switch;
>> + host->mmc_host_ops.init_card = xenon_init_card;
>> + host->mmc_host_ops.execute_tuning = xenon_execute_tuning;
>> +}
>> +
>> +static int xenon_probe_dt(struct platform_device *pdev)
>> +{
>> + struct device_node *np = pdev->dev.of_node;
>> + struct sdhci_host *host = platform_get_drvdata(pdev);
>> + struct mmc_host *mmc = host->mmc;
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> + int err;
>> + u32 slot_idx, nr_slot;
>> + u32 tuning_count;
>> + u32 reg;
>> +
>> + /* Standard MMC property */
>> + err = mmc_of_parse(mmc);
>> + if (err)
>> + return err;
>> +
>> + /* Standard SDHCI property */
>> + sdhci_get_of_property(pdev);
>> +
>> + /*
>> + * Xenon Specific property:
>> + * emmc: explicitly indicate whether this slot is for eMMC
>> + * slotno: the index of slot. Refer to SDHC_SYS_CFG_INFO register
>> + * tun-count: the interval between re-tuning
>> + * PHY type: "sdhc phy", "emmc phy 5.0" or "emmc phy 5.1"
>> + */
>> + if (of_property_read_bool(np, "marvell,xenon-emmc"))
>> + priv->emmc_slot = true;
>
> So, you need this because of the eMMC voltage switch behaviour, right?
>
> Then I would rather like to describe this a generic DT bindings for
> the eMMC voltage level support. There have acutally been some earlier
> discussions for this, but we haven't yet made some changes.
>
> I think what is missing is a mmc-ddr-3_3v DT binding, which when set,
> allows the host driver to accept I/O voltage switches to 3.3V. If not
> supported the ->start_signal_voltage_switch() ops may return -EINVAL.
> This would inform the mmc core to move on to the next supported
> voltage level. There might be some minor additional changes to the mmc
> card initialization sequence, but those should be simple.
>
> I can help out to look into this, unless you want to do it yourself of course!?
>
Yes. One of the reasons is to provide eMMC specific voltage setting.
But in my very own opinion, it should be irrelevant to voltage level.
The eMMC voltage setting on our SDHC is different from SD/SDIO voltage switch.
It will become more complex with different SOC implementation details.
Unfortunately, MMC driver cannot determine the card type yet when eMMC voltage
setting should be executed.
Thus an flag is required here to tell driver to execute eMMC voltage setting.
Besides, additional eMMC specific settings might be implemented in future, besides
voltage setting. Most of them should be completed before MMC driver recognizes the
card type. Thus I have to keep this flag to indicate current SDHC is for eMMC.
>> + else
>> + priv->emmc_slot = false;
>> +
>> + if (!of_property_read_u32(np, "marvell,xenon-slotno", &slot_idx)) {
>> + nr_slot = sdhci_readl(host, SDHC_SYS_CFG_INFO);
>> + nr_slot &= NR_SUPPORTED_SLOT_MASK;
>> + if (unlikely(slot_idx > nr_slot)) {
>> + dev_err(mmc_dev(mmc), "Slot Index %d exceeds Number of slots %d\n",
>> + slot_idx, nr_slot);
>> + return -EINVAL;
>> + }
>> + } else {
>> + priv->slot_idx = 0x0;
>> + }
>> +
>> + if (!of_property_read_u32(np, "marvell,xenon-tun-count",
>> + &tuning_count)) {
>> + if (unlikely(tuning_count >= TMR_RETUN_NO_PRESENT)) {
>> + dev_err(mmc_dev(mmc), "Wrong Re-tuning Count. Set default value %d\n",
>> + DEF_TUNING_COUNT);
>> + tuning_count = DEF_TUNING_COUNT;
>> + }
>> + } else {
>> + priv->tuning_count = DEF_TUNING_COUNT;
>> + }
>
> To make the code a bit easier...
>
> Maybe set "priv->tuning_count = DEF_TUNING_COUNT" before the "if", and
> instead have the of_property_read_u32() to update the value when set.
>
Yes. You are correct.
>> +
>> + if (of_property_read_bool(np, "marvell,xenon-mask-conflict-err")) {
>> + reg = sdhci_readl(host, SDHC_SYS_EXT_OP_CTRL);
>> + reg |= MASK_CMD_CONFLICT_ERROR;
>> + sdhci_writel(host, reg, SDHC_SYS_EXT_OP_CTRL);
>> + }
>> +
>> + return err;
>> +}
>> +
>> +static int xenon_slot_probe(struct sdhci_host *host)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> + u8 slot_idx = priv->slot_idx;
>> +
>> + /* Enable slot */
>> + xenon_enable_slot(host, slot_idx);
>> +
>> + /* Enable ACG */
>> + xenon_set_acg(host, true);
>> +
>> + /* Enable Parallel Transfer Mode */
>> + xenon_enable_slot_parallel_tran(host, slot_idx);
>> +
>> + priv->timing = MMC_TIMING_FAKE;
>> + priv->clock = 0;
>
> What are these used for?
>
During card initialization, our SDHC PHY setting depends on current
timing and SDCLK frequency.
priv->timing and priv->clock will be used in PHY setting later.
It can be considered as a clean-up.
Anyway, it does look ugly. I will improve them after our PHY setting
passes your review.
>> +
>> + return 0;
>> +}
>> +
>> +static void xenon_slot_remove(struct sdhci_host *host)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> + u8 slot_idx = priv->slot_idx;
>> +
>> + /* disable slot */
>> + xenon_disable_slot(host, slot_idx);
>> +}
>> +
>> +static int sdhci_xenon_probe(struct platform_device *pdev)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host;
>> + struct sdhci_host *host;
>> + struct clk *clk, *axi_clk;
>> + struct sdhci_xenon_priv *priv;
>> + int err;
>> +
>> + host = sdhci_pltfm_init(pdev, &sdhci_xenon_pdata,
>> + sizeof(struct sdhci_xenon_priv));
>> + if (IS_ERR(host))
>> + return PTR_ERR(host);
>> +
>> + pltfm_host = sdhci_priv(host);
>> + priv = sdhci_pltfm_priv(pltfm_host);
>> +
>> + xenon_set_acg(host, false);
>> +
>> + /*
>> + * Link Xenon specific mmc_host_ops function,
>> + * to replace standard ones in sdhci_ops.
>> + */
>> + xenon_replace_mmc_host_ops(host);
>> +
>> + clk = devm_clk_get(&pdev->dev, "core");
>> + if (IS_ERR(clk)) {
>> + dev_err(&pdev->dev, "Failed to setup input clk.\n");
>> + err = PTR_ERR(clk);
>> + goto free_pltfm;
>> + }
>> + clk_prepare_enable(clk);
>
> Check error code.
>
>> + pltfm_host->clk = clk;
>
> Why not assign pltfm_host->clk immedately when doing devm_clk_get(),
> that would make this a bit cleaner, right?
>
Yes, of course.
>> +
>> + /*
>> + * Some SOCs require additional clock to
>> + * manage AXI bus clock.
>> + * It is optional.
>> + */
>> + axi_clk = devm_clk_get(&pdev->dev, "axi");
>> + if (!IS_ERR(axi_clk)) {
>> + clk_prepare_enable(axi_clk);
>> + priv->axi_clk = axi_clk;
>> + }
>
> Same comments as for the above core clock.
>
OK.
>> +
>> + err = xenon_probe_dt(pdev);
>> + if (err)
>> + goto err_clk;
>> +
>> + err = xenon_slot_probe(host);
>> + if (err)
>> + goto err_clk;
>> +
>> + err = sdhci_add_host(host);
>> + if (err)
>> + goto remove_slot;
>> +
>> + return 0;
>> +
>> +remove_slot:
>> + xenon_slot_remove(host);
>> +err_clk:
>> + clk_disable_unprepare(pltfm_host->clk);
>> + if (!IS_ERR(axi_clk))
>> + clk_disable_unprepare(axi_clk);
>> +free_pltfm:
>> + sdhci_pltfm_free(pdev);
>> + return err;
>> +}
>> +
>> +static int sdhci_xenon_remove(struct platform_device *pdev)
>> +{
>> + struct sdhci_host *host = platform_get_drvdata(pdev);
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> + int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xFFFFFFFF);
>> +
>> + xenon_slot_remove(host);
>> +
>> + sdhci_remove_host(host, dead);
>> +
>> + clk_disable_unprepare(pltfm_host->clk);
>> + clk_disable_unprepare(priv->axi_clk);
>> +
>> + sdhci_pltfm_free(pdev);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id sdhci_xenon_dt_ids[] = {
>> + { .compatible = "marvell,xenon-sdhci",},
>> + { .compatible = "marvell,armada-3700-sdhci",},
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids);
>> +
>> +static struct platform_driver sdhci_xenon_driver = {
>> + .driver = {
>> + .name = "xenon-sdhci",
>> + .of_match_table = sdhci_xenon_dt_ids,
>> + .pm = &sdhci_pltfm_pmops,
>> + },
>> + .probe = sdhci_xenon_probe,
>> + .remove = sdhci_xenon_remove,
>> +};
>> +
>> +module_platform_driver(sdhci_xenon_driver);
>> +
>> +MODULE_DESCRIPTION("SDHCI platform driver for Marvell Xenon SDHC");
>> +MODULE_AUTHOR("Hu Ziji <huziji@marvell.com>");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/mmc/host/sdhci-xenon.h b/drivers/mmc/host/sdhci-xenon.h
>> new file mode 100644
>> index 000000000000..4601d0a4b22f
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-xenon.h
>
> I don't think you need a specific header for this, let's instead just
> put everthing in the c-file.
>
Some definitions inside this file will also be referred in PHY setting in
sdhci-xenon-phy.c.
Thus I put all the definitions together into a header file.
>> @@ -0,0 +1,142 @@
>> +/*
>> + * Copyright (C) 2016 Marvell, All Rights Reserved.
>> + *
>> + * Author: Hu Ziji <huziji@marvell.com>
>> + * Date: 2016-8-24
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + */
>> +#ifndef SDHCI_XENON_H_
>> +#define SDHCI_XENON_H_
>> +
>> +#include <linux/clk.h>
>> +#include <linux/mmc/card.h>
>> +#include <linux/of.h>
>> +#include "sdhci.h"
>> +
>> +/* Register Offset of SD Host Controller SOCP self-defined register */
>> +#define SDHC_SYS_CFG_INFO 0x0104
>> +#define SLOT_TYPE_SDIO_SHIFT 24
>> +#define SLOT_TYPE_EMMC_MASK 0xFF
>> +#define SLOT_TYPE_EMMC_SHIFT 16
>> +#define SLOT_TYPE_SD_SDIO_MMC_MASK 0xFF
>> +#define SLOT_TYPE_SD_SDIO_MMC_SHIFT 8
>> +#define NR_SUPPORTED_SLOT_MASK 0x7
>> +
>> +#define SDHC_SYS_OP_CTRL 0x0108
>> +#define AUTO_CLKGATE_DISABLE_MASK BIT(20)
>> +#define SDCLK_IDLEOFF_ENABLE_SHIFT 8
>> +#define SLOT_ENABLE_SHIFT 0
>> +
>> +#define SDHC_SYS_EXT_OP_CTRL 0x010C
>> +#define MASK_CMD_CONFLICT_ERROR BIT(8)
>> +
>> +#define SDHC_SLOT_OP_STATUS_CTRL 0x0128
>> +#define DELAY_90_DEGREE_MASK_EMMC5 BIT(7)
>> +#define DELAY_90_DEGREE_SHIFT_EMMC5 7
>> +#define EMMC_5_0_PHY_FIXED_DELAY_MASK 0x7F
>> +#define EMMC_PHY_FIXED_DELAY_MASK 0xFF
>> +#define EMMC_PHY_FIXED_DELAY_WINDOW_MIN (EMMC_PHY_FIXED_DELAY_MASK >> 3)
>> +#define SDH_PHY_FIXED_DELAY_MASK 0x1FF
>> +#define SDH_PHY_FIXED_DELAY_WINDOW_MIN (SDH_PHY_FIXED_DELAY_MASK >> 4)
>> +
>> +#define TUN_CONSECUTIVE_TIMES_SHIFT 16
>> +#define TUN_CONSECUTIVE_TIMES_MASK 0x7
>> +#define TUN_CONSECUTIVE_TIMES 0x4
>> +#define TUNING_STEP_SHIFT 12
>> +#define TUNING_STEP_MASK 0xF
>> +#define TUNING_STEP_DIVIDER BIT(6)
>> +
>> +#define FORCE_SEL_INVERSE_CLK_SHIFT 11
>> +
>> +#define SDHC_SLOT_EMMC_CTRL 0x0130
>> +#define ENABLE_DATA_STROBE BIT(24)
>> +#define SET_EMMC_RSTN BIT(16)
>> +#define DISABLE_RD_DATA_CRC BIT(14)
>> +#define DISABLE_CRC_STAT_TOKEN BIT(13)
>> +#define EMMC_VCCQ_MASK 0x3
>> +#define EMMC_VCCQ_1_8V 0x1
>> +#define EMMC_VCCQ_3_3V 0x3
>> +
>> +#define SDHC_SLOT_RETUNING_REQ_CTRL 0x0144
>> +/* retuning compatible */
>> +#define RETUNING_COMPATIBLE 0x1
>> +
>> +#define SDHC_SLOT_EXT_PRESENT_STATE 0x014C
>> +#define LOCK_STATE 0x1
>> +
>> +#define SDHC_SLOT_DLL_CUR_DLY_VAL 0x0150
>> +
>> +/* Tuning Parameter */
>> +#define TMR_RETUN_NO_PRESENT 0xF
>> +#define DEF_TUNING_COUNT 0x9
>> +
>> +#define MMC_TIMING_FAKE 0xFF
>> +
>> +#define DEFAULT_SDCLK_FREQ (400000)
>> +
>> +/* Xenon specific Mode Select value */
>> +#define XENON_SDHCI_CTRL_HS200 0x5
>> +#define XENON_SDHCI_CTRL_HS400 0x6
>
> For all defines above:
>
> All these defines needs some *SDHCI* prefix. Can you please update that.
Sure. Will add prefix for all of them.
>
>> +
>> +struct sdhci_xenon_priv {
>> + /*
>> + * The bus_width, timing, and clock fields in below
>> + * record the current setting of Xenon SDHC.
>> + * Driver will call a Sampling Fixed Delay Adjustment
>> + * if any setting is changed.
>> + */
>> + unsigned char bus_width;
>> + unsigned char timing;
>
> These two are not used. Please remove.
>
The above two variables will be used in PHY setting
in sdhci-xenon-phy.c.
Could you please help review them in next patch?
>> + unsigned char tuning_count;
>> + unsigned int clock;
>
> "clock" isn't used, please remove.
>
It will be accessed in PHY setting in sdhci-xenon-phy.c.
Could you please help review it in next patch?
>> + struct clk *axi_clk;
>> +
>> + /* Slot idx */
>> + u8 slot_idx;
>> + /* Whether this slot is for eMMC */
>> + bool emmc_slot;
>> +
>> + /*
>> + * When initializing card, Xenon has to determine card type and
>> + * adjust Sampling Fixed delay for the speed mode in which
>> + * DLL tuning is not support.
>> + * However, at that time, card structure is not linked to mmc_host.
>> + * Thus a card pointer is added here to provide
>> + * the delay adjustment function with the card structure
>> + * of the card during initialization.
>> + *
>> + * It is only valid during initialization after it is updated in
>> + * xenon_init_card().
>> + * Do not access this variable in normal transfers after
>> + * initialization completes.
>> + */
>> + struct mmc_card *card_candidate;
>
> Not activley used in this change, please remove and let's discuss it
> in the next step.
>
This varible will be accessed in PHY setting in sdhci-xenon-phy.c.
I would like to discuss about it in PHY file. Could you please help
review it in next patch?
Thank you.
Best regards,
Hu Ziji
>> +};
>> +
>> +static inline int enable_xenon_internal_clk(struct sdhci_host *host)
>> +{
>> + u32 reg;
>> + u8 timeout;
>> +
>> + reg = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
>> + reg |= SDHCI_CLOCK_INT_EN;
>> + sdhci_writel(host, reg, SDHCI_CLOCK_CONTROL);
>> + /* Wait max 20 ms */
>> + timeout = 20;
>> + while (!((reg = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
>> + & SDHCI_CLOCK_INT_STABLE)) {
>> + if (timeout == 0) {
>> + pr_err("%s: Internal clock never stabilised.\n",
>> + mmc_hostname(host->mmc));
>> + return -ETIMEDOUT;
>> + }
>> + timeout--;
>> + mdelay(1);
>> + }
>> +
>> + return 0;
>> +}
>> +#endif
>> --
>> git-series 0.8.10
>
> Kind regards
> Uffe
>
^ permalink raw reply
* [PATCH 0/2] OF phandle nexus support + GPIO nexus
From: Pantelis Antoniou @ 2016-11-24 12:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124102529.20212-1-stephen.boyd@linaro.org>
Hi Stephen,
> On Nov 24, 2016, at 12:25 , Stephen Boyd <stephen.boyd@linaro.org> wrote:
>
> This is one small chunk of work related to DT overlays for expansion
> boards. It would be good to have a way to expose #<list>-cells types of
> providers through a connector in a standard way. So we introduce a way
> to make "nexus" nodes for these types of properties to remap the consumer
> number space to the other side of the connector's number space. It's
> basically a copy of the interrupt nexus implementation, but without
> the address space matching design and interrupt-parent walking.
>
> The first patch implements a generic method to do this, and the second patch
> adds a unit test for it. The third patch is more of an example than anything
> else. It shows how we would modify frameworks to use the new API.
>
Excellent. It was about time this happened.
> Stephen Boyd (3):
> of: Support parsing phandle argument lists through a nexus node
> of: unittest: Add phandle remapping test
> gpio: Support gpio nexus dt bindings
>
> drivers/gpio/gpiolib-of.c | 5 +-
> drivers/of/base.c | 146 ++++++++++++++++++++++++++++
> drivers/of/unittest-data/testcases.dts | 11 +++
> drivers/of/unittest-data/tests-phandle.dtsi | 24 +++++
> drivers/of/unittest.c | 124 +++++++++++++++++++++++
> include/linux/of.h | 14 +++
> 6 files changed, 322 insertions(+), 2 deletions(-)
>
> --
> 2.10.0.297.gf6727b0
>
Comments inline?
Regards
? Pantelis
^ permalink raw reply
* [PATCH 7/9] clocksource/drivers/rockchip_timer: implement clocksource timer
From: Alexander Kochetkov @ 2016-11-24 13:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1800621.EhepfMxccR@diego>
Hello Heiko!
> 24 ????. 2016 ?., ? 15:17, Heiko St?bner <heiko@sntech.de> ???????(?):
>
> In your dts-patch you reuse the rk3288-timer compatible value, which is also
> non-ideal.
rockchip,rk-timer.txt states what I should use rockchip,rk3288-timer for rk3188 timers:
Required properties:
- compatible: shall be one of:
"rockchip,rk3288-timer" - for rk3066, rk3036, rk3188, rk322x, rk3288, rk3368
Should I add "rockchip,rk3188-timer? (or better "rockchip,rk3036-timer?) ? Or may be second
approach should be used?
> What you may want to do is introduce a rockchip,rk3188-timer compatible and
> then make the timer-driver act accordingly, as you then know you are on a
> rk3188-board ... see drivers attaching specific structs to the of_device_id
> entries. From the documentation
May be it is better to prepend compatible string with "rockchip,rk3188-timer? in the dts
file only? elinux[1] recommend build compatible string from "exact device? string and
?other devices? that the device is compatible with.
As ?other devices? string defined already, I use it.
[1] http://elinux.org/Device_Tree_Usage#Understanding_the_compatible_Property
timer0: timer at 20038000 {
compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer?;
?
};
Just found what rk3036 already declared such way:
rk3036.dtsi: timer: timer at 20044000 {
rk3036.dtsi: compatible = "rockchip,rk3036-timer", "rockchip,rk3288-timer?;
>
> it also shouldn't really matter which timer
> you use as clocksource, as on the rk3188 it seems all of them act the same way
> (except timer3 being always on).
You are right.
I sent dts file with general timer settings, without clockevent enabled, so one could pick
up timer and setup it in the board dts (radxarock, for example) like:
&timer0 {
rockchip,clocksource;
status = ?okay?;
};
> When touching devicetree-properties, please also adapt the binding document
> Documentation/devicetree/bindings/timer,rockchip,rk-timer.txt
> in this case and also include the devicetree maintainers.
If the patch[2] ok, I'll resend it and include devicetree maintainers.
[2] http://lists.infradead.org/pipermail/linux-rockchip/2016-November/013148.html
Regards,
Alexander.
^ permalink raw reply
* TDA998x crash on HDLCD probe failure
From: Robin Murphy @ 2016-11-24 13:18 UTC (permalink / raw)
To: linux-arm-kernel
Hi Liviu, Russell,
I'd been meaning to try digging into this if it hadn't gone away since I
first noticed it, but I don't really have the time and it still happens
with 4.9-rc and today's -next. Representative splat below, but in
summary what happens is that if the HDLCD fails to probe, the TDA998x
connector seems to get cleaned up twice, resulting in a NULL dereference
the second time. I got as far as sketching out the following flow from a
debug session (on the same 4.8-rc2 kernel), but I don't know nearly
enough to tell which driver is at fault:
hdlcd_drm_bind
-> drm_fbdev_cma_init (fails)
...
-> drm_mode_config_cleanup
...
-> drm_connector_cleanup
-> component_unbind_all
...
-> tda998x_unbind
-> drm_connector_cleanup (NULL connector)
It's easily reproduced on Juno by booting arm64 defconfig with
CONFIG_CMA_SIZE_MBYTES=1 and a sufficiently large monitor connected to
warrant a >1MB framebuffer.
Robin.
[ 4.107349] hdlcd 7ff60000.hdlcd: failed to allocate buffer with size
7680000
[ 4.114459] hdlcd 7ff60000.hdlcd: Failed to set initial hw configuration.
[ 4.121888] Unable to handle kernel NULL pointer dereference at
virtual address 00000000
[ 4.129951] pgd = ffffff80091e0000
[ 4.133345] [00000000] *pgd=00000009ffffe003, *pud=00000009ffffe003,
*pmd=0000000000000000
[ 4.141613] Internal error: Oops: 96000005 [#1] PREEMPT SMP
[ 4.147144] Modules linked in:
[ 4.150188] CPU: 0 PID: 122 Comm: kworker/u12:2 Not tainted
4.8.0-rc2+ #989
[ 4.157097] Hardware name: ARM Juno development board (r1) (DT)
[ 4.162981] Workqueue: deferwq deferred_probe_work_func
[ 4.168173] task: ffffffc975d93200 task.stack: ffffffc975dac000
[ 4.174055] PC is at drm_connector_cleanup+0x58/0x1c0
[ 4.179074] LR is at tda998x_unbind+0x24/0x40
[ 4.183401] pc : [<ffffff80084c46f0>] lr : [<ffffff800850414c>]
pstate: 00000045
[ 4.190750] sp : ffffffc975dafa10
[ 4.194041] x29: ffffffc975dafa10 x28: ffffffc9768152a8
[ 4.199325] x27: ffffffc97ff46450 x26: ffffff8008d99000
[ 4.204608] x25: dead000000000100 x24: dead000000000200
[ 4.209891] x23: ffffffc976bf91e8 x22: 0000000000000000
[ 4.215172] x21: ffffffc976bf9170 x20: ffffffc976bf9170
[ 4.220454] x19: ffffffc976bf9018 x18: 0000000000000000
[ 4.225737] x17: 0000000074ce71ee x16: 000000008ff5d35f
[ 4.231019] x15: ffffffc97681e91c x14: ffffffffffffffff
[ 4.236301] x13: ffffffc97681e185 x12: 0000000000000038
[ 4.241583] x11: 0101010101010101 x10: 0000000000000000
[ 4.246866] x9 : 0000000040000000 x8 : 0000000000210d00
[ 4.252148] x7 : ffffffc97fea8c00 x6 : 000000000000001b
[ 4.257430] x5 : ffffff80084b7b8c x4 : 0000000000000080
[ 4.262712] x3 : ffffff8008504128 x2 : ffffffc975df3800
[ 4.267993] x1 : 0000000000000000 x0 : 0000000000000000
[ 4.273274]
[ 4.274759] Process kworker/u12:2 (pid: 122, stack limit =
0xffffffc975dac020)
[ 4.281938] Stack: (0xffffffc975dafa10 to 0xffffffc975db0000)
[ 4.285576] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 0)
[ 4.293602] fa00: ffffffc975dafa60
ffffff800850414c
[ 4.301386] fa20: ffffffc976bf9018 ffffffc976bf9170 0000000000000000
ffffffc975df3800
[ 4.309170] fa40: ffffffc975c1e1a0 ffffffc976b14400 ffffffc976b14410
ffffff8008518784
[ 4.316954] fa60: ffffffc975dafa80 ffffff8008507918 ffffffc976bdf080
ffffffffffffffd8
[ 4.324737] fa80: ffffffc975dafaa0 ffffff8008507a0c ffffffc975c1e180
ffffffc976b14400
[ 4.332521] faa0: ffffffc975dafae0 ffffff80084d5adc ffffffc975df3800
fffffffffffffff4
[ 4.340305] fac0: ffffffc976b14410 ffffffc975df3018 ffffffc975df3018
ffffff80084d5ab4
[ 4.348089] fae0: ffffffc975dafb30 ffffff8008507b58 ffffffc976bdf080
0000000000000028
[ 4.355873] fb00: ffffff8008d3d600 0000000000000001 ffffffc975c1e180
ffffffc976bdf080
[ 4.363657] fb20: ffffffc975c1e098 ffffff80084d5428 ffffffc975dafb90
ffffff8008507c50
[ 4.371441] fb40: ffffffc975c1e180 ffffff8008d3d5f0 ffffffc976bdf080
ffffffc976b57000
[ 4.379225] fb60: ffffff8008d3d000 ffffffc976815000 ffffff8008d926c6
ffffffc976815020
[ 4.387009] fb80: ffffffc976815078 ffffffc9768152a8 ffffffc975dafbd0
ffffff8008504b90
[ 4.394793] fba0: ffffff8008d3d4e8 ffffffc976b57020 ffffffc976b57004
ffffffc976b57000
[ 4.402577] fbc0: ffffff8008504b78 ffffff80086bf8cc ffffffc975dafbe0
ffffff80086bf914
[ 4.410360] fbe0: ffffffc975dafc20 ffffff800850d094 ffffffc976b57020
ffffff8008dc4000
[ 4.418144] fc00: 0000000000000000 ffffff8008d3d448 0000000000000004
ffffff8008dc4000
[ 4.425928] fc20: ffffffc975dafc60 ffffff800850d28c ffffff8008d3d448
ffffffc975dafd00
[ 4.433711] fc40: ffffffc976b57020 0000000000000001 ffffffc975e3b400
ffffff800850b114
[ 4.441495] fc60: ffffffc975dafc90 ffffff800850b108 0000000000000000
ffffffc975dafd00
[ 4.449279] fc80: ffffff800850d1f0 0000000000000001 ffffffc975dafcd0
ffffff800850cd64
[ 4.457062] fca0: ffffffc976b57020 ffffffc976b57080 ffffff8008d584f0
ffffffc976b57080
[ 4.464846] fcc0: ffffffc976af38d0 ffffffc975c20168 ffffffc975dafd10
ffffff800850d338
[ 4.472630] fce0: ffffffc976b57020 ffffffc976b57020 ffffff8008d584f0
ffffff8008d3d000
[ 4.480414] fd00: ffffffc976b57020 0000000000000001 ffffffc975dafd20
ffffff800850c124
[ 4.488198] fd20: ffffffc975dafd50 ffffff800850c5b0 ffffffc976b57020
ffffff8008d3d848
[ 4.495982] fd40: ffffff8008d3d820 ffffff800850c5a8 ffffffc975dafd80
ffffff80080d2998
[ 4.503765] fd60: 0000000000000000 ffffffc976bd4500 ffffff8008d3d880
0000000000000000
[ 4.511549] fd80: ffffffc975dafdc0 ffffff80080d2c40 ffffffc976bd4500
ffffffc976815000
[ 4.519333] fda0: ffffffc976bd4530 ffffffc976815020 ffffff8008ce5000
ffffffc975dac000
[ 4.527117] fdc0: ffffffc975dafe20 ffffff80080d8918 ffffffc976bd3880
ffffff8008d9cb08
[ 4.534900] fde0: ffffff8008ace5f0 ffffffc976bd4500 ffffff80080d2bf8
0000000000000000
[ 4.542683] fe00: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.550467] fe20: 0000000000000000 ffffff8008082e90 ffffff80080d8848
ffffffc976bd3880
[ 4.558250] fe40: 0000000000000000 0000000000000000 0000000000000000
0000d00420000400
[ 4.566034] fe60: ffffffc975dafea0 0000000000000000 ffffff80080d8848
ffffffc976bd4500
[ 4.573817] fe80: 0000000000000000 0000000000000000 ffffffc975dafe90
ffffffc975dafe90
[ 4.581602] fea0: 0000000000000000 ffffff8000000000 ffffffc975dafeb0
ffffffc975dafeb0
[ 4.589384] fec0: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.597168] fee0: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.604951] ff00: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.612734] ff20: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.620517] ff40: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.628299] ff60: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.636082] ff80: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.643865] ffa0: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[ 4.651648] ffc0: 0000000000000000 0000000000000005 0000000000000000
0000000000000000
[ 4.659431] ffe0: 0000000000000000 0000000000000000 100a0c0062800800
42200c0000000c00
[ 4.667210] Call trace:
[ 4.669643] Exception stack(0xffffffc975daf840 to 0xffffffc975daf970)
[ 4.676040] f840: ffffffc976bf9018 0000008000000000 ffffffc975dafa10
ffffff80084c46f0
[ 4.683823] f860: 0000000000000000 0000000000000000 00000001800f000e
0000000000000001
[ 4.691607] f880: ffffffc9769ef2a0 00000001000f000f ffffffbf25da7a00
ffffffc976803100
[ 4.699390] f8a0: ffffffc975daf9b0 ffffff80081a899c ffffffc976803100
ffffff80083446b4
[ 4.707174] f8c0: ffffffbf25da7a00 ffffffc975dac000 ffffffc9769ef2a0
000000000003c380
[ 4.714958] f8e0: 0000000000000000 0000000000000000 ffffffc975df3800
ffffff8008504128
[ 4.722741] f900: 0000000000000080 ffffff80084b7b8c 000000000000001b
ffffffc97fea8c00
[ 4.730524] f920: 0000000000210d00 0000000040000000 0000000000000000
0101010101010101
[ 4.738308] f940: 0000000000000038 ffffffc97681e185 ffffffffffffffff
ffffffc97681e91c
[ 4.746090] f960: 000000008ff5d35f 0000000074ce71ee
[ 4.750937] [<ffffff80084c46f0>] drm_connector_cleanup+0x58/0x1c0
[ 4.756990] [<ffffff800850414c>] tda998x_unbind+0x24/0x40
[ 4.762354] [<ffffff8008507918>] component_unbind.isra.4+0x28/0x50
[ 4.768492] [<ffffff8008507a0c>] component_unbind_all+0xcc/0xd8
[ 4.774373] [<ffffff80084d5adc>] hdlcd_drm_bind+0x234/0x418
[ 4.779909] [<ffffff8008507b58>] try_to_bring_up_master+0x140/0x1a0
[ 4.786133] [<ffffff8008507c50>] component_add+0x98/0x170
[ 4.791496] [<ffffff8008504b90>] tda998x_probe+0x18/0x20
[ 4.796774] [<ffffff80086bf914>] i2c_device_probe+0x164/0x258
[ 4.802481] [<ffffff800850d094>] driver_probe_device+0x204/0x2b0
[ 4.808447] [<ffffff800850d28c>] __device_attach_driver+0x9c/0xf8
[ 4.814498] [<ffffff800850b108>] bus_for_each_drv+0x58/0x98
[ 4.820033] [<ffffff800850cd64>] __device_attach+0xc4/0x138
[ 4.825567] [<ffffff800850d338>] device_initial_probe+0x10/0x18
[ 4.831446] [<ffffff800850c124>] bus_probe_device+0x94/0xa0
[ 4.836981] [<ffffff800850c5b0>] deferred_probe_work_func+0x78/0xb0
[ 4.843207] [<ffffff80080d2998>] process_one_work+0x118/0x378
[ 4.848914] [<ffffff80080d2c40>] worker_thread+0x48/0x498
[ 4.854276] [<ffffff80080d8918>] kthread+0xd0/0xe8
[ 4.859036] [<ffffff8008082e90>] ret_from_fork+0x10/0x40
[ 4.864314] Code: f2fbd5b9 f2fbd5b8 f8478ee0 eb17001f (f9400013)
[ 4.870472] ---[ end trace a643cfe4ce1d838b ]---
^ permalink raw reply
* [PATCH 7/9] clocksource/drivers/rockchip_timer: implement clocksource timer
From: Heiko Stübner @ 2016-11-24 13:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0E972AC9-1132-4D0B-BF25-918BC1AA1A49@gmail.com>
Hi Alexander,
Am Donnerstag, 24. November 2016, 16:05:55 schrieb Alexander Kochetkov:
> Hello Heiko!
>
> > 24 ????. 2016 ?., ? 15:17, Heiko St?bner <heiko@sntech.de> ???????(?):
> >
> > In your dts-patch you reuse the rk3288-timer compatible value, which is
> > also non-ideal.
>
> rockchip,rk-timer.txt states what I should use rockchip,rk3288-timer for
> rk3188 timers:
>
> Required properties:
> - compatible: shall be one of:
> "rockchip,rk3288-timer" - for rk3066, rk3036, rk3188, rk322x, rk3288,
> rk3368
>
> Should I add "rockchip,rk3188-timer? (or better "rockchip,rk3036-timer?) ?
> Or may be second approach should be used?
>
> > What you may want to do is introduce a rockchip,rk3188-timer compatible
> > and
> > then make the timer-driver act accordingly, as you then know you are on a
> > rk3188-board ... see drivers attaching specific structs to the
> > of_device_id
> > entries. From the documentation
>
> May be it is better to prepend compatible string with
> "rockchip,rk3188-timer? in the dts file only? elinux[1] recommend build
> compatible string from "exact device? string and ?other devices? that the
> device is compatible with.
>
> As ?other devices? string defined already, I use it.
>
> [1]
> http://elinux.org/Device_Tree_Usage#Understanding_the_compatible_Property
>
> timer0: timer at 20038000 {
> compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer?;
> ?
> };
>
> Just found what rk3036 already declared such way:
>
> rk3036.dtsi: timer: timer at 20044000 {
> rk3036.dtsi: compatible = "rockchip,rk3036-timer", "rockchip,rk3288-
timer?;
correct, use both but also update the binding, like
mmc/rockchip-dw-mshc.txt does.
> > it also shouldn't really matter which timer
> > you use as clocksource, as on the rk3188 it seems all of them act the same
> > way (except timer3 being always on).
>
> You are right.
>
> I sent dts file with general timer settings, without clockevent enabled, so
> one could pick up timer and setup it in the board dts (radxarock, for
> example) like:
>
> &timer0 {
> rockchip,clocksource;
> status = ?okay?;
> };
what I actually meant was that the driver could also recognize the rk3188-
timer compatible as "we need a clocksource" and it shouldn't matter which
timer actually gets used for this.
> > When touching devicetree-properties, please also adapt the binding
> > document
> >
> > Documentation/devicetree/bindings/timer,rockchip,rk-timer.txt
> >
> > in this case and also include the devicetree maintainers.
>
> If the patch[2] ok, I'll resend it and include devicetree maintainers.
>
> [2]
> http://lists.infradead.org/pipermail/linux-rockchip/2016-November/013148.ht
> ml
Only devicetree people can really tell you if that is ok :-) .
The devicetree is supposed to be a hardware-description and implementation-
independent, so rockchip,clocksource sounds pretty much like linux-specific
configuration things leaking into the devicetree.
Heiko
^ permalink raw reply
* TDA998x crash on HDLCD probe failure
From: Russell King - ARM Linux @ 2016-11-24 13:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8281efe7-bac7-8e35-5784-e05cf0dc7eab@arm.com>
On Thu, Nov 24, 2016 at 01:18:39PM +0000, Robin Murphy wrote:
> Hi Liviu, Russell,
>
> I'd been meaning to try digging into this if it hadn't gone away since I
> first noticed it, but I don't really have the time and it still happens
> with 4.9-rc and today's -next. Representative splat below, but in
> summary what happens is that if the HDLCD fails to probe, the TDA998x
> connector seems to get cleaned up twice, resulting in a NULL dereference
> the second time. I got as far as sketching out the following flow from a
> debug session (on the same 4.8-rc2 kernel), but I don't know nearly
> enough to tell which driver is at fault:
>
> hdlcd_drm_bind
> -> drm_fbdev_cma_init (fails)
> ...
> -> drm_mode_config_cleanup
> ...
> -> drm_connector_cleanup
> -> component_unbind_all
> ...
> -> tda998x_unbind
> -> drm_connector_cleanup (NULL connector)
>
> It's easily reproduced on Juno by booting arm64 defconfig with
> CONFIG_CMA_SIZE_MBYTES=1 and a sufficiently large monitor connected to
> warrant a >1MB framebuffer.
It looks to me like a hdlcd bug.
The probe path operates in this order:
- allocates hdlcd - 1
- allocates drm device - 2
- drm_mode_config_init - 3
- hdlcd_load - 4
- binds all components - 5
- enables runtime PM - 6
- drm_vblank_init - 7
- drm_mode_config_reset - 8
- drm_kms_helper_poll_init - 9
- drm_fbdev_cma_init - 10
- drm_dev_register - 11
However, the cleanup operates in this order:
- drm_fbdev_cma_fini - undoes 10
- drm_kms_helper_poll_fini - undoes 9
- drm_mode_config_cleanup - undoes 3
- drm_vblank_cleanup - undoes 7
- pm_runtime_disable - undoes 6
- component_unbind_all - undoes 5
- drm_irq_uninstall - undoes 4
- of_reserved_mem_device_release - undoes other half of 4
- drm_dev_unref - undoes 2
Spot the step which is out of the correct order - drm_mode_config_cleanup()
is misplaced - it's reversing the actions of drm_mode_config_init(), not
drm_mode_config_reset().
So, drm_mode_config_cleanup() should be much later, after step 4 has
been undone, otherwise there are paths that leave various DRM objects
(created by drm_mode_create_standard_properties()) referenced, and
will cause problems exactly like you're seeing here.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Ulf Hansson @ 2016-11-24 13:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <dd230463-04f6-df31-7056-1a185eb6cfc7@marvell.com>
On 24 November 2016 at 13:41, Ziji Hu <huziji@marvell.com> wrote:
> Hi Ulf,
>
> On 2016/11/24 18:43, Ulf Hansson wrote:
>> On 31 October 2016 at 12:09, Gregory CLEMENT
>> <gregory.clement@free-electrons.com> wrote:
>>> From: Ziji Hu <huziji@marvell.com>
>>>
> <snip>
>>> +static int xenon_emmc_signal_voltage_switch(struct mmc_host *mmc,
>>> + struct mmc_ios *ios)
>>> +{
>>> + unsigned char voltage = ios->signal_voltage;
>>> +
>>> + if ((voltage == MMC_SIGNAL_VOLTAGE_330) ||
>>> + (voltage == MMC_SIGNAL_VOLTAGE_180))
>>> + return __emmc_signal_voltage_switch(mmc, voltage);
>>> +
>>> + dev_err(mmc_dev(mmc), "Unsupported signal voltage: %d\n",
>>> + voltage);
>>> + return -EINVAL;
>>
>> This wrapper function seems unnessarry. It only adds a dev_err(), so
>> then might as well do that in __emmc_signal_voltage_switch().
>>
> Sure. Will merge it back to __emmc_signal_voltage_switch().
>
>>> +}
>>> +
>>> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
>>> + struct mmc_ios *ios)
>>> +{
>>> + struct sdhci_host *host = mmc_priv(mmc);
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>>> +
>>> + /*
>>> + * Before SD/SDIO set signal voltage, SD bus clock should be
>>> + * disabled. However, sdhci_set_clock will also disable the Internal
>>> + * clock in mmc_set_signal_voltage().
>>
>> If that's the case then that is wrong in the generic sdhci code.
>> What's the reason why it can't be fixed there instead of having this
>> workaround?
>>
> In my very own opinion, SD Spec doesn't specify whether SDCLK should be
> enabled or not during power setting.
> Enabling SDCLK might be a special condition only required by our SDHC.
> I try to avoid breaking other vendors' SDHC functionality
> if their SDHCs require SDCLK disabled.
> Thus I prefer to keep it inside our SDHC driver.
I let Adrian comment on this.
For sure we should avoid breaking other sdhci variant, but on the
other hand *if* the generic code is wrong we should fix it!
>
>>> + * If Internal clock is disabled, the 3.3V/1.8V bit can not be updated.
>>> + * Thus here manually enable internal clock.
>>> + *
>>> + * After switch completes, it is unnecessary to disable internal clock,
>>> + * since keeping internal clock active obeys SD spec.
>>> + */
>>> + enable_xenon_internal_clk(host);
>>> +
>>> + if (priv->emmc_slot)
>>> + return xenon_emmc_signal_voltage_switch(mmc, ios);
>>> +
>>> + return sdhci_start_signal_voltage_switch(mmc, ios);
>>> +}
>>> +
>>> +/*
>>> + * After determining which slot is used for SDIO,
>>> + * some additional task is required.
>>> + */
>>> +static void xenon_init_card(struct mmc_host *mmc, struct mmc_card *card)
>>> +{
>>> + struct sdhci_host *host = mmc_priv(mmc);
>>> + u32 reg;
>>> + u8 slot_idx;
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>>> +
>>> + /* Link the card for delay adjustment */
>>> + priv->card_candidate = card;
>>> + /* Set tuning functionality of this slot */
>>> + xenon_slot_tuning_setup(host);
>>
>> This looks weird. I assume this can be done as a part of the regular
>> tuning seqeunce!?
>>
> It is our SDHC specific preparation prior to tuning, rather than a
> standard step in spec.
> Thus I leave it inside our driver.
My point is that this isn't the purpose of ->init_card(). thus you are
abusing it.
Try to make it work in another way, please. I think you can.
>
>>> +
>>> + slot_idx = priv->slot_idx;
>>> + if (!mmc_card_sdio(card)) {
>>> + /* Clear SDIO Card Inserted indication */
>>
>> Why do you need this?
>>
>> If you need to reset this, I think it's better to do it from
>> ->set_ios() at MMC_POWER_OFF.
>>
> This field indicates SDIO card and controls async interrupt feature
> of SDIO in our SDHC.
> This async interrupt feature is enabled when SDIO card is inserted.
> It should be disabled if SD card is inserted instead.
What do you mean by SDIO async interupts? Are you talking about SDIO
irqs on DAT1 line?
Those is supposed to be enabled when someone explicitly requests them,
not when the card is inserted.
In other words when an SDIO func driver have called sdio_claim_irq().
Moreover, we have ->enable_sdio_irq() ops that deals with this.
[...]
>>> +
>>> + /*
>>> + * Xenon Specific property:
>>> + * emmc: explicitly indicate whether this slot is for eMMC
>>> + * slotno: the index of slot. Refer to SDHC_SYS_CFG_INFO register
>>> + * tun-count: the interval between re-tuning
>>> + * PHY type: "sdhc phy", "emmc phy 5.0" or "emmc phy 5.1"
>>> + */
>>> + if (of_property_read_bool(np, "marvell,xenon-emmc"))
>>> + priv->emmc_slot = true;
>>
>> So, you need this because of the eMMC voltage switch behaviour, right?
>>
>> Then I would rather like to describe this a generic DT bindings for
>> the eMMC voltage level support. There have acutally been some earlier
>> discussions for this, but we haven't yet made some changes.
>>
>> I think what is missing is a mmc-ddr-3_3v DT binding, which when set,
>> allows the host driver to accept I/O voltage switches to 3.3V. If not
>> supported the ->start_signal_voltage_switch() ops may return -EINVAL.
>> This would inform the mmc core to move on to the next supported
>> voltage level. There might be some minor additional changes to the mmc
>> card initialization sequence, but those should be simple.
>>
>> I can help out to look into this, unless you want to do it yourself of course!?
>>
> Yes. One of the reasons is to provide eMMC specific voltage setting.
> But in my very own opinion, it should be irrelevant to voltage level.
> The eMMC voltage setting on our SDHC is different from SD/SDIO voltage switch.
> It will become more complex with different SOC implementation details.
Got it. Although I think we can cope with that fine just by using the
different SD/eMMC speed modes settings defined in DT (or from the
SDHCI caps register)
> Unfortunately, MMC driver cannot determine the card type yet when eMMC voltage
> setting should be executed.
> Thus an flag is required here to tell driver to execute eMMC voltage setting.
>
> Besides, additional eMMC specific settings might be implemented in future, besides
> voltage setting. Most of them should be completed before MMC driver recognizes the
> card type. Thus I have to keep this flag to indicate current SDHC is for eMMC.
I doubt you will need a generic "eMMC" flag, but let's see when we go forward.
Currently it's clear you don't need such a flag, so I will submit a
change adding a DT binding for "mmc-ddr-3_3v" then we can take it from
there, to see if it suits your needs.
[...]
Kind regards
Uffe
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox