* [PATCH] pinctrl: mediatek: use builtin_platform_driver
From: Hongzhou Yang @ 2016-11-18 23:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <70fcc58b7e9219a9fbe3695df927041b8234dd08.1479457060.git.geliangtang@gmail.com>
On Fri, 2016-11-18 at 22:12 +0800, Geliang Tang wrote:
> Use builtin_platform_driver() helper to simplify the code.
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
> drivers/pinctrl/mediatek/pinctrl-mt6397.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt6397.c b/drivers/pinctrl/mediatek/pinctrl-mt6397.c
> index 6eccb85..afcede7 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mt6397.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mt6397.c
> @@ -64,8 +64,4 @@ static struct platform_driver mtk_pinctrl_driver = {
> },
> };
>
> -static int __init mtk_pinctrl_init(void)
> -{
> - return platform_driver_register(&mtk_pinctrl_driver);
> -}
> -device_initcall(mtk_pinctrl_init);
> +builtin_platform_driver(mtk_pinctrl_driver);
Acked-by: Hongzhou Yang <hongzhou.yang@mediatek.com>
Thanks,
Hongzhou
^ permalink raw reply
* Low network throughput on i.MX28
From: Jörg Krause @ 2016-11-18 23:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478360733.3405.17.camel@intel.com>
Hi all,
[snip]
I did some time measurements on the wifi, mmc and dma driver to compare
the performance between the vendor and the mainline kernel. For this I
toggled some GPIOs and measured the time difference with an osci. I
started measuring the time before calling sdio_readsb() in the wifi
driver [1] and stopped the time when the call returns. Note that the
time was only measured for a packet length of 1536 bytes.
The vendor kernel took about 250 us to return whereas the mainline
kernel took about 325 us. To investigate where this additional time
comes from I divided the whole procedure into seperate parts and
compared their time consumed.
I noticed that the mainline kernel does took much longer to return
after the DMA request is done, signalled in this case by calling
mxs_mmc_dma_irq_callback() [2] in the mxs-mmc driver. From here it
takes about 150 us to get back to sdio_readsb().
An example for consuming much more time is the mainline mmc driver
where it hangs in?mmc_wait_done() [2] about 50 us just calling
complete(), whereas the vendor mmc driver almost immediately returns
here.
I wonder why this call to complete consumes so much time? Any ideas?
[1] http://lxr.free-electrons.com/source/drivers/net/wireless/broadcom/
brcm80211/brcmfmac/bcmsdh.c#L488
[2] http://lxr.free-electrons.com/source/drivers/mmc/host/mxs-mmc.c#L17
9
[3] http://lxr.free-electrons.com/source/drivers/mmc/core/core.c#L386
Best regards,
J?rg Krause
^ permalink raw reply
* [PATCH 2/4] spi: spi-fsl-dspi: Fix incorrect DMA setup
From: Stefan Agner @ 2016-11-18 23:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118080403.GB1572@Sanchayan-Arch.localdomain>
On 2016-11-18 00:04, maitysanchayan at gmail.com wrote:
> On 16-11-17 17:03:19, Stefan Agner wrote:
>> On 2016-11-17 04:16, Sanchayan Maity wrote:
>> > Currently dmaengine_prep_slave_single was being called with length
>> > set to the complete DMA buffer size. This resulted in unwanted bytes
>> > being transferred to the SPI register leading to clock and MOSI lines
>> > having unwanted data even after chip select got deasserted and the
>> > required bytes having been transferred.
>> >
>> > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>> > ---
>> > drivers/spi/spi-fsl-dspi.c | 10 ++++++++--
>> > 1 file changed, 8 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
>> > index b1ee1f5..aee8c88 100644
>> > --- a/drivers/spi/spi-fsl-dspi.c
>> > +++ b/drivers/spi/spi-fsl-dspi.c
>> > @@ -265,7 +265,10 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
>> >
>> > dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
>> > dma->tx_dma_phys,
>> > - DSPI_DMA_BUFSIZE, DMA_MEM_TO_DEV,
>> > + dma->curr_xfer_len *
>> > + DMA_SLAVE_BUSWIDTH_4_BYTES /
>> > + (tx_word ? 2 : 1),
>> > + DMA_MEM_TO_DEV,
>>
>> Hm, this is getting ridiculous, I think we convert curr_xfer_len from
>> bytes to DMA transfers in almost every use.
>>
>> Can we make it be transfer length in actual 4 byte transfers? We then
>> probably have to convert it to bytes once to subtract from
>> curr_remaining_bytes, but I think it would simplify code overall...
>
> Will the below be acceptable fix?
>
> diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
> index 41422cd..db7f091 100644
> --- a/drivers/spi/spi-fsl-dspi.c
> +++ b/drivers/spi/spi-fsl-dspi.c
> @@ -217,15 +217,13 @@ static void dspi_rx_dma_callback(void *arg)
> struct fsl_dspi *dspi = arg;
> struct fsl_dspi_dma *dma = dspi->dma;
> int rx_word;
> - int i, len;
> + int i;
> u16 d;
>
> rx_word = is_double_byte_mode(dspi);
>
> - len = rx_word ? (dma->curr_xfer_len / 2) : dma->curr_xfer_len;
> -
> if (!(dspi->dataflags & TRAN_STATE_RX_VOID)) {
> - for (i = 0; i < len; i++) {
> + for (i = 0; i < dma->curr_xfer_len; i++) {
> d = dspi->dma->rx_dma_buf[i];
> rx_word ? (*(u16 *)dspi->rx = d) :
> (*(u8 *)dspi->rx = d);
> @@ -242,14 +240,12 @@ static int /(struct
> fsl_dspi *dspi)
> struct device *dev = &dspi->pdev->dev;
> int time_left;
> int tx_word;
> - int i, len;
> + int i;
> u16 val;
>
> tx_word = is_double_byte_mode(dspi);
>
> - len = tx_word ? (dma->curr_xfer_len / 2) : dma->curr_xfer_len;
> -
> - for (i = 0; i < len - 1; i++) {
> + for (i = 0; i < dma->curr_xfer_len - 1; i++) {
> val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
> dspi->dma->tx_dma_buf[i] =
> SPI_PUSHR_TXDATA(val) | SPI_PUSHR_PCS(dspi->cs) |
> @@ -267,7 +263,9 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
>
> dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
> dma->tx_dma_phys,
> - DSPI_DMA_BUFSIZE, DMA_MEM_TO_DEV,
> + dma->curr_xfer_len *
> + DMA_SLAVE_BUSWIDTH_4_BYTES,
> + DMA_MEM_TO_DEV,
> DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> if (!dma->tx_desc) {
> dev_err(dev, "Not able to get desc for DMA xfer\n");
> @@ -283,7 +281,9 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
>
> dma->rx_desc = dmaengine_prep_slave_single(dma->chan_rx,
> dma->rx_dma_phys,
> - DSPI_DMA_BUFSIZE, DMA_DEV_TO_MEM,
> + dma->curr_xfer_len *
> + DMA_SLAVE_BUSWIDTH_4_BYTES,
> + DMA_DEV_TO_MEM,
> DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> if (!dma->rx_desc) {
> dev_err(dev, "Not able to get desc for DMA xfer\n");
> @@ -330,17 +330,17 @@ static int dspi_dma_xfer(struct fsl_dspi *dspi)
> struct device *dev = &dspi->pdev->dev;
> int curr_remaining_bytes;
> int bytes_per_buffer;
> - int tx_word;
> + int tx_word = 1;
> int ret = 0;
>
> - tx_word = is_double_byte_mode(dspi);
> + if (is_double_byte_mode(dspi))
> + tx_word = 2;
I would try to be consistent with tx_word. In most other cases it is
used as boolean, whether this is a 2 byte word or not.
Here you change it to represent the length of a single transfer/frame.
Nothing wrong with that, just if you do such changes, also change the
name of the variable so the reader does not get miss lead from other
uses of "tx_word"...
But otherwise looks good, like this variant much better!
Maybe we should add a comment in struct fsl_dspi_dma:
/* Length of transfer in words of DSPI_FIFO_SIZE */
--
Stefan
> curr_remaining_bytes = dspi->len;
> + bytes_per_buffer = DSPI_DMA_BUFSIZE / DSPI_FIFO_SIZE;
> while (curr_remaining_bytes) {
> /* Check if current transfer fits the DMA buffer */
> - dma->curr_xfer_len = curr_remaining_bytes;
> - bytes_per_buffer = DSPI_DMA_BUFSIZE /
> - (DSPI_FIFO_SIZE / (tx_word ? 2 : 1));
> - if (curr_remaining_bytes > bytes_per_buffer)
> + dma->curr_xfer_len = curr_remaining_bytes / tx_word;
> + if (dma->curr_xfer_len > bytes_per_buffer)
> dma->curr_xfer_len = bytes_per_buffer;
>
> ret = dspi_next_xfer_dma_submit(dspi);
> @@ -349,7 +349,7 @@ static int dspi_dma_xfer(struct fsl_dspi *dspi)
> goto exit;
>
> } else {
> - curr_remaining_bytes -= dma->curr_xfer_len;
> + curr_remaining_bytes -= dma->curr_xfer_len * tx_word;
> if (curr_remaining_bytes < 0)
> curr_remaining_bytes = 0;
> dspi->len = curr_remaining_bytes;
>
>
> Regards,
> Sanchayan.
>>
>> --
>> Stefan
>>
>>
>> > DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>> > if (!dma->tx_desc) {
>> > dev_err(dev, "Not able to get desc for DMA xfer\n");
>> > @@ -281,7 +284,10 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
>> >
>> > dma->rx_desc = dmaengine_prep_slave_single(dma->chan_rx,
>> > dma->rx_dma_phys,
>> > - DSPI_DMA_BUFSIZE, DMA_DEV_TO_MEM,
>> > + dma->curr_xfer_len *
>> > + DMA_SLAVE_BUSWIDTH_4_BYTES /
>> > + (tx_word ? 2 : 1),
>> > + DMA_DEV_TO_MEM,
>> > DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>> > if (!dma->rx_desc) {
>> > dev_err(dev, "Not able to get desc for DMA xfer\n");
^ permalink raw reply
* [BUG] hdlcd gets confused about base address
From: Russell King - ARM Linux @ 2016-11-18 23:37 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
While testing HDMI with Xorg on the Juno board, I find that when Xorg
starts up or shuts down, the display is shifted significantly to the
right and wrapped in the active region. (No sync bars are visible.)
The timings are correct, it behaves as if the start address has been
shifted many pixels _into_ the framebuffer.
This occurs whenever the display mode size is changed - using xrandr
in Xorg shows that changing the resolution triggers the problem
almost every time, but changing the refresh rate does not.
Using devmem2 to disable and re-enable the HDLCD resolves the issue,
and repeated disable/enable cycles do not make the issue re-appear.
So, I patched the HDLCD to do this, and testing it with Xorg after
several repetitions seems to work.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
What I think is going on is that the FIFO or address generator for
reading data from the AXI bus is not properly reset when changing the
resolution, and the enable-disable-enable cycle causes the HDLCD
hardware to sort itself out. It's (eg) significantly out - for example,
to properly align the display, I have to program an address of
0xf4ff0200 into the hardware rather than 0xf5000000 - that's 896 pixels
before the real start of the frame buffer.
With this patch, a patch to TDA998x to avoid the i2c-designware issue,
and xf86-video-armada, I have LXDE running on the Juno.
Something I also noticed is this:
scanout_start = gem->paddr + plane->state->fb->offsets[0] +
plane->state->crtc_y * plane->state->fb->pitches[0] +
plane->state->crtc_x * bpp / 8;
Surely this should be using src_[xy] (which are the position in the
source - iow, memory, and not crtc_[xy] which is the position on the
CRTC displayed window. To put it another way, the src_* define the
region of the source material that is mapped onto a rectangular area
on the display defined by crtc_*.
Another note is that since the CRTC can't place the plane in arbitary
positions and sizes within the active area, should the atomic_check
ensure that crtc_x = crtc_y = 0, and the crtc width/height are the
size of the active area?
drivers/gpu/drm/arm/hdlcd_crtc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 48019ae22ddb..3e97acf6e2a7 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -150,6 +150,8 @@ static void hdlcd_crtc_enable(struct drm_crtc *crtc)
clk_prepare_enable(hdlcd->clk);
hdlcd_crtc_mode_set_nofb(crtc);
hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 1);
+ hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 0);
+ hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 1);
}
static void hdlcd_crtc_disable(struct drm_crtc *crtc)
--
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 related
* [PATCH] PCI: Add information about describing PCI in ACPI
From: Rafael J. Wysocki @ 2016-11-18 23:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161117175938.17465.45820.stgit@bhelgaas-glaptop.roam.corp.google.com>
On Thu, Nov 17, 2016 at 6:59 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> Add a writeup about how PCI host bridges should be described in ACPI
> using PNP0A03/PNP0A08 devices, PNP0C02 devices, and the MCFG table.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Looks great overall, but I have a few comments (below).
> ---
> Documentation/PCI/00-INDEX | 2 +
> Documentation/PCI/acpi-info.txt | 136 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 138 insertions(+)
> create mode 100644 Documentation/PCI/acpi-info.txt
>
> diff --git a/Documentation/PCI/00-INDEX b/Documentation/PCI/00-INDEX
> index 147231f..0780280 100644
> --- a/Documentation/PCI/00-INDEX
> +++ b/Documentation/PCI/00-INDEX
> @@ -1,5 +1,7 @@
> 00-INDEX
> - this file
> +acpi-info.txt
> + - info on how PCI host bridges are represented in ACPI
> MSI-HOWTO.txt
> - the Message Signaled Interrupts (MSI) Driver Guide HOWTO and FAQ.
> PCIEBUS-HOWTO.txt
> diff --git a/Documentation/PCI/acpi-info.txt b/Documentation/PCI/acpi-info.txt
> new file mode 100644
> index 0000000..ccbcfda
> --- /dev/null
> +++ b/Documentation/PCI/acpi-info.txt
> @@ -0,0 +1,136 @@
> + ACPI considerations for PCI host bridges
> +
> +The basic requirement is that the ACPI namespace should describe
> +*everything* that consumes address space unless there's another
> +standard way for the OS to find it [1, 2]. For example, windows that
> +are forwarded to PCI by a PCI host bridge should be described via ACPI
> +devices, since the OS can't locate the host bridge by itself. PCI
> +devices *below* the host bridge do not need to be described via ACPI,
> +because the resources they consume are inside the host bridge windows,
> +and the OS can discover them via the standard PCI enumeration
> +mechanism (using config accesses to read and size the BARs).
> +
> +This ACPI resource description is done via _CRS methods of devices in
To be painfully precise, those need not be methods. They may be
static objects too.
> +the ACPI namespace [2]. _CRS methods are like generalized PCI BARs:
> +the OS can read _CRS and figure out what resource is being consumed
> +even if it doesn't have a driver for the device [3]. That's important
> +because it means an old OS can work correctly even on a system with
> +new devices unknown to the OS. The new devices won't do anything, but
> +the OS can at least make sure no resources conflict with them.
> +
> +Static tables like MCFG, HPET, ECDT, etc., are *not* mechanisms for
> +reserving address space! The static tables are for things the OS
> +needs to know early in boot, before it can parse the ACPI namespace.
> +If a new table is defined, an old OS needs to operate correctly even
> +though it ignores the table. _CRS allows that because it is generic
> +and understood by the old OS; a static table does not.
> +
> +If the OS is expected to manage an ACPI device, that device will have
I'm not very keen on using the term "ACPI device" in documentation as
it is not particularly well defined. As a rule, I prefer to talk
about "non-discoverable devices described via ACPI" or similar.
Accordingly, I'd change the above line to something like "If the OS is
expected to manage a non-discoverable device described via ACPI, that
device will have".
> +a specific _HID/_CID that tells the OS what driver to bind to it, and
> +the _CRS tells the OS and the driver where the device's registers are.
> +
> +PNP0C02 "motherboard" devices are basically a catch-all. There's no
> +programming model for them other than "don't use these resources for
> +anything else." So any address space that is (1) not claimed by some
> +other ACPI device and (2) should not be assigned by the OS to
Here I'd say "any address space that is (1) not claimed by _CRS under
any other device object in the ACPI namespace and (2) ...".
> +something else, should be claimed by a PNP0C02 _CRS method.
Thanks,
Rafael
^ permalink raw reply
* [PATCH 1/3] dt-bindings: Add Macnica Americas vendor prefix
From: Dinh Nguyen @ 2016-11-18 21:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAL_JsqJv7bBmkWUmd44Ln-=PLLKEch+hbjuD8v8ArhOCZ37jdA@mail.gmail.com>
On 11/18/2016 12:03 PM, Rob Herring wrote:
>
> Sorry, didn't send anything out, but the series is already applied if
> you look at -next or PW.
>
Thanks!
Dinh
^ permalink raw reply
* spin_lock behavior with ARM64 big.Little/HMP
From: Vikram Mulukutla @ 2016-11-18 20:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8d9d6333-0ebe-65c4-c6f1-3e3475e3e535@arm.com>
Hi Sudeep,
Thanks for taking a look!
On 2016-11-18 02:30, Sudeep Holla wrote:
> Hi Vikram,
>
> On 18/11/16 02:22, Vikram Mulukutla wrote:
>> Hello,
>>
>> This isn't really a bug report, but just a description of a
>> frequency/IPC
>> dependent behavior that I'm curious if we should worry about. The
>> behavior
>> is exposed by questionable design so I'm leaning towards don't-care.
>>
>> Consider these threads running in parallel on two ARM64 CPUs running
>> mainline
>> Linux:
>>
>
> Are you seeing this behavior with the mainline kernel on any platforms
> as we have a sort of workaround for this ?
>
If I understand that workaround correctly, the ARM timer event stream is
used
to periodically wake up CPUs that are waiting in WFE, is that right? I
think
my scenario below may be different because LittleCPU doesn't actually
wait
on a WFE event in the loop that is trying to increment lock->next, i.e.
it's
stuck in the following loop:
ARM64_LSE_ATOMIC_INSN(
/* LL/SC */
" prfm pstl1strm, %3\n"
"1: ldaxr %w0, %3\n"
" add %w1, %w0, %w5\n"
" stxr %w2, %w1, %3\n"
" cbnz %w2, 1b\n",
I have been testing internal platforms; I'll try to test on something
available publicly that's b.L. In any case, the timer event stream was
enabled
when I tried this out.
>> (Ordering of lines between the two columns does not indicate a
>> sequence of
>> execution. Assume flag=0 initially.)
>>
>> LittleARM64_CPU @ 300MHz (e.g.A53) | BigARM64_CPU @ 1.5GHz (e.g.
>> A57)
>> -------------------------------------+----------------------------------
>> spin_lock_irqsave(s) | local_irq_save()
>> /* critical section */
>> flag = 1 | spin_lock(s)
>> spin_unlock_irqrestore(s) | while (!flag) {
>> | spin_unlock(s)
>> | cpu_relax();
>> | spin_lock(s)
>> | }
>> | spin_unlock(s)
>> | local_irq_restore()
>>
[...]
Thanks,
Vikram
^ permalink raw reply
* [PATCH v16 14/15] clocksource/drivers/arm_arch_timer: Add GTDT support for memory-mapped timer
From: Mark Rutland @ 2016-11-18 20:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479304148-2965-15-git-send-email-fu.wei@linaro.org>
On Wed, Nov 16, 2016 at 09:49:07PM +0800, fu.wei at linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> The patch add memory-mapped timer register support by using the
> information provided by the new GTDT driver of ACPI.
>
> Signed-off-by: Fu Wei <fu.wei@linaro.org>
> ---
> drivers/clocksource/arm_arch_timer.c | 26 +++++++++++++++++++++++++-
> 1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index c494ca8..0aad60a 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -1067,7 +1067,28 @@ CLOCKSOURCE_OF_DECLARE(armv7_arch_timer_mem, "arm,armv7-timer-mem",
> arch_timer_mem_of_init);
>
> #ifdef CONFIG_ACPI_GTDT
> -/* Initialize per-processor generic timer */
> +static int __init arch_timer_mem_acpi_init(void)
> +{
> + struct arch_timer_mem *timer_mem;
> + int ret = 0;
> + int i = 0;
> +
> + timer_mem = kzalloc(sizeof(*timer_mem), GFP_KERNEL);
Why do you need it zeroed? You don't clear it between iterations, so
either you don't need this, or you have a bug in the loop.
> + if (!timer_mem)
> + return -ENOMEM;
> +
> + while (!gtdt_arch_timer_mem_init(timer_mem, i)) {
Huh?
Why doesn't GTDT expose a function to fill in the entire arch_timer_mem
in one go?
There shouldn't be multiple instances, as far as I am aware. Am I
mistaken?
> + ret = arch_timer_mem_init(timer_mem);
> + if (ret)
> + break;
> + i++;
> + }
> +
> + kfree(timer_mem);
> + return ret;
> +}
Regardless, arch_timer_mem is small enough that you don't need dynamic
allocaiton. Just put it on the stack, e.g.
static int __init arch_timer_mem_acpi_init(void)
{
int i = 0;
struct arch_timer_mem timer_mem;
while (!gtdt_arch_timer_mem_init(timer_mem, i)) {
int ret = arch_timer_mem_init();
if (ret)
return ret;
i++
}
return 0;
}
Thanks,
Mark.
^ permalink raw reply
* [PATCH v16 11/15] acpi/arm64: Add GTDT table parse driver
From: Mark Rutland @ 2016-11-18 20:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479304148-2965-12-git-send-email-fu.wei@linaro.org>
On Wed, Nov 16, 2016 at 09:49:04PM +0800, fu.wei at linaro.org wrote:
> +#define for_each_platform_timer(_g) for (; _g; _g = next_platform_timer(_g))
This doesn't fit the usual for_each_* pattern, since _g has to be
manually initialised first. Either come up with a way of maknig this fit
the usual pattern, or get rid of this, and use:
t = however_you_get_the_first_timer();
if (!t)
bailt_out_somehow();
do {
...
} while (t = next_platform_timer(t));
> +/*
> + * Release the memory we have allocated in acpi_gtdt_init.
> + * This should be called, when the driver who called "acpi_gtdt_init" previously
> + * doesn't need the GTDT info anymore.
> + */
> +void __init acpi_gtdt_release(void)
> +{
> + kfree(timer_block);
> + kfree(watchdog);
> + timer_block = NULL;
> + watchdog = NULL;
> +}
Why is this not simply in the error path of acpi_gtdt_init()?
> +
> +/*
> + * Get some basic info from GTDT table, and init the global variables above
> + * for all timers initialization of Generic Timer.
> + * This function does some validation on GTDT table.
> + */
> +int __init acpi_gtdt_init(struct acpi_table_header *table)
> +{
> + timer_block = kcalloc(timer_count,
> + sizeof(struct acpi_gtdt_timer_block *),
> + GFP_KERNEL);
> + if (!timer_block)
> + return -ENOMEM;
> +
> + watchdog = kcalloc(timer_count, sizeof(struct acpi_gtdt_watchdog *),
> + GFP_KERNEL);
> + if (!watchdog) {
> + kfree(timer_block);
> + timer_block = NULL;
> + return -ENOMEM;
> + }
Please have a common error path below, and branch to that when you need
to free these.
> +error:
> + acpi_gtdt_release();
> + return -EINVAL;
> +}
[...]
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 61a3d90..a1611d1 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -577,6 +577,13 @@ enum acpi_reconfig_event {
> int acpi_reconfig_notifier_register(struct notifier_block *nb);
> int acpi_reconfig_notifier_unregister(struct notifier_block *nb);
>
> +#ifdef CONFIG_ACPI_GTDT
> +int acpi_gtdt_init(struct acpi_table_header *table);
> +int acpi_gtdt_map_ppi(int type);
> +bool acpi_gtdt_c3stop(int type);
> +void acpi_gtdt_release(void);
Why do these need to be here?
What possible value is ther in exporting acpi_gtdt_release() !?
Thanks,
Mark.
^ permalink raw reply
* [PATCH] arm64: dts: qcom: msm8996: Fixup smp2p node
From: Bjorn Andersson @ 2016-11-18 20:06 UTC (permalink / raw)
To: linux-arm-kernel
The SMEM state property name changes between the integration branch and
mainline, update to use the correct one.
Fixes: 2f45d9fcd531 ("arm64: dts: msm8996: Add SMP2P and APCS nodes")
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
Seems we're still carrying this discrepancy somewhere and I missed it in my
review, sorry about that.
arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index cde4114bae5a..712d5d043105 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -530,7 +530,7 @@
adsp_smp2p_out: master-kernel {
qcom,entry-name = "master-kernel";
- #qcom,state-cells = <1>;
+ #qcom,smem-state-cells = <1>;
};
adsp_smp2p_in: slave-kernel {
--
2.5.0
^ permalink raw reply related
* [PATCH v16 10/15] clocksource/drivers/arm_arch_timer: Refactor the timer init code to prepare for GTDT
From: Mark Rutland @ 2016-11-18 20:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479304148-2965-11-git-send-email-fu.wei@linaro.org>
On Wed, Nov 16, 2016 at 09:49:03PM +0800, fu.wei at linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> The patch refactor original memory-mapped timer init code:
> (1) Extract a subfunction for detecting a bast time frame:
> is_best_frame.
Please leave this logic in arch_timer_mem_init(). Pulling it out gains
us nothing, but makes the patch harder to review.
> (2) Refactor "arch_timer_mem_init", make it become a common code for
> memory-mapped timer init.
> (3) Add a new function "arch_timer_mem_of_init" for DT init.
These generally look fine.
Thanks,
Mark.
> Signed-off-by: Fu Wei <fu.wei@linaro.org>
> ---
> drivers/clocksource/arm_arch_timer.c | 162 +++++++++++++++++++++++------------
> 1 file changed, 107 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 9ddc091..0836bb9 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -923,17 +923,35 @@ static int __init arch_timer_of_init(struct device_node *np)
> CLOCKSOURCE_OF_DECLARE(armv7_arch_timer, "arm,armv7-timer", arch_timer_of_init);
> CLOCKSOURCE_OF_DECLARE(armv8_arch_timer, "arm,armv8-timer", arch_timer_of_init);
>
> -static int __init arch_timer_mem_init(struct device_node *np)
> +static bool __init is_best_frame(void __iomem *cntctlbase, u32 cnttidr, int n)
> +{
> + u32 cntacr = CNTACR_RFRQ | CNTACR_RWPT | CNTACR_RPCT | CNTACR_RWVT |
> + CNTACR_RVOFF | CNTACR_RVCT;
> +
> + /* Try enabling everything, and see what sticks */
> + writel_relaxed(cntacr, cntctlbase + CNTACR(n));
> + cntacr = readl_relaxed(cntctlbase + CNTACR(n));
> +
> + if ((cnttidr & CNTTIDR_VIRT(n)) &&
> + !(~cntacr & (CNTACR_RWVT | CNTACR_RVCT)))
> + arch_timer_mem_use_virtual = true;
> + else if (~cntacr & (CNTACR_RWPT | CNTACR_RPCT))
> + return false;
> +
> + return true;
> +}
> +
> +static int __init arch_timer_mem_init(struct arch_timer_mem *timer_mem)
> {
> - struct device_node *frame, *best_frame = NULL;
> void __iomem *cntctlbase, *base;
> - unsigned int irq, ret = -EINVAL;
> + struct arch_timer_mem_frame *best_frame = NULL;
> + unsigned int irq;
> u32 cnttidr;
> + int i, ret;
>
> - arch_timers_present |= ARCH_TIMER_TYPE_MEM;
> - cntctlbase = of_iomap(np, 0);
> + cntctlbase = ioremap(timer_mem->cntctlbase, timer_mem->size);
> if (!cntctlbase) {
> - pr_err("Can't find CNTCTLBase\n");
> + pr_err("Can't map CNTCTLBase.\n");
> return -ENXIO;
> }
>
> @@ -943,76 +961,110 @@ static int __init arch_timer_mem_init(struct device_node *np)
> * Try to find a virtual capable frame. Otherwise fall back to a
> * physical capable frame.
> */
> - for_each_available_child_of_node(np, frame) {
> - int n;
> - u32 cntacr;
> -
> - if (of_property_read_u32(frame, "frame-number", &n)) {
> - pr_err("Missing frame-number\n");
> - of_node_put(frame);
> - goto out;
> - }
> -
> - /* Try enabling everything, and see what sticks */
> - cntacr = CNTACR_RFRQ | CNTACR_RWPT | CNTACR_RPCT |
> - CNTACR_RWVT | CNTACR_RVOFF | CNTACR_RVCT;
> - writel_relaxed(cntacr, cntctlbase + CNTACR(n));
> - cntacr = readl_relaxed(cntctlbase + CNTACR(n));
> -
> - if ((cnttidr & CNTTIDR_VIRT(n)) &&
> - !(~cntacr & (CNTACR_RWVT | CNTACR_RVCT))) {
> - of_node_put(best_frame);
> - best_frame = frame;
> - arch_timer_mem_use_virtual = true;
> - break;
> + for (i = 0; i < timer_mem->num_frames; i++) {
> + if (is_best_frame(cntctlbase, cnttidr,
> + timer_mem->frame[i].frame_nr)) {
> + best_frame = &timer_mem->frame[i];
> + if (arch_timer_mem_use_virtual)
> + break;
> }
> -
> - if (~cntacr & (CNTACR_RWPT | CNTACR_RPCT))
> - continue;
> -
> - of_node_put(best_frame);
> - best_frame = of_node_get(frame);
> }
> + iounmap(cntctlbase);
>
> - ret= -ENXIO;
> - base = arch_counter_base = of_iomap(best_frame, 0);
> - if (!base) {
> - pr_err("Can't map frame's registers\n");
> - goto out;
> + if (!best_frame) {
> + pr_err("Can't find frame for register\n");
> + return -EINVAL;
> }
>
> if (arch_timer_mem_use_virtual)
> - irq = irq_of_parse_and_map(best_frame, ARCH_TIMER_VIRT_SPI);
> + irq = best_frame->virt_irq;
> else
> - irq = irq_of_parse_and_map(best_frame, ARCH_TIMER_PHYS_SPI);
> + irq = best_frame->phys_irq;
>
> - ret = -EINVAL;
> if (!irq) {
> pr_err("Frame missing %s irq",
> arch_timer_mem_use_virtual ? "virt" : "phys");
> - goto out;
> + return -EINVAL;
> }
>
> - /*
> - * Try to determine the frequency from the device tree,
> - * if fail, get the frequency from CNTFRQ.
> - */
> - if (!arch_timer_rate &&
> - of_property_read_u32(np, "clock-frequency", &arch_timer_rate))
> - arch_timer_detect_rate(base);
> + base = ioremap(best_frame->cntbase, best_frame->size);
> + if (!base) {
> + pr_err("Can't map frame's registers\n");
> + return -ENXIO;
> + }
> +
> + arch_timer_detect_rate(base);
>
> ret = arch_timer_mem_register(base, irq);
> - if (ret)
> + if (ret) {
> + iounmap(base);
> + return ret;
> + }
> +
> + arch_counter_base = base;
> + arch_timers_present |= ARCH_TIMER_TYPE_MEM;
> +
> + return 0;
> +}
> +
> +static int __init arch_timer_mem_of_init(struct device_node *np)
> +{
> + struct arch_timer_mem *timer_mem;
> + struct device_node *frame_node;
> + struct resource res;
> + int i, ret = -EINVAL;
> +
> + timer_mem = kzalloc(sizeof(*timer_mem), GFP_KERNEL);
> + if (!timer_mem)
> + return -ENOMEM;
> +
> + if (of_address_to_resource(np, 0, &res))
> goto out;
> + timer_mem->cntctlbase = res.start;
> + timer_mem->size = resource_size(&res);
>
> - return arch_timer_common_init();
> + i = 0;
> + for_each_available_child_of_node(np, frame_node) {
> + int n;
> + struct arch_timer_mem_frame *frame = &timer_mem->frame[i];
> +
> + if (of_property_read_u32(frame_node, "frame-number", &n)) {
> + pr_err("Missing frame-number\n");
> + of_node_put(frame_node);
> + goto out;
> + }
> + frame->frame_nr = n;
> +
> + if (of_address_to_resource(frame_node, 0, &res)) {
> + of_node_put(frame_node);
> + goto out;
> + }
> + frame->cntbase = res.start;
> + frame->size = resource_size(&res);
> +
> + frame->virt_irq = irq_of_parse_and_map(frame_node,
> + ARCH_TIMER_VIRT_SPI);
> + frame->phys_irq = irq_of_parse_and_map(frame_node,
> + ARCH_TIMER_PHYS_SPI);
> +
> + if (++i >= ARCH_TIMER_MEM_MAX_FRAMES)
> + break;
> + }
> + timer_mem->num_frames = i;
> +
> + /* Try to determine the frequency from the device tree */
> + if (!arch_timer_rate)
> + of_property_read_u32(np, "clock-frequency", &arch_timer_rate);
> +
> + ret = arch_timer_mem_init(timer_mem);
> + if (!ret)
> + ret = arch_timer_common_init();
> out:
> - iounmap(cntctlbase);
> - of_node_put(best_frame);
> + kfree(timer_mem);
> return ret;
> }
> CLOCKSOURCE_OF_DECLARE(armv7_arch_timer_mem, "arm,armv7-timer-mem",
> - arch_timer_mem_init);
> + arch_timer_mem_of_init);
>
> #ifdef CONFIG_ACPI
> static int __init map_generic_timer_interrupt(u32 interrupt, u32 flags)
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v16 08/15] clocksource/drivers/arm_arch_timer: Refactor arch_timer_needs_probing, and call it only if acpi disabled.
From: Mark Rutland @ 2016-11-18 19:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479304148-2965-9-git-send-email-fu.wei@linaro.org>
On Wed, Nov 16, 2016 at 09:49:01PM +0800, fu.wei at linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> The patch refactor original arch_timer_needs_probing function:
> (1) Separate out arch_timer_needs_probing from arch_timer_common_init,
> and call it only if acpi disabled.
> (2) Rename arch_timer_needs_probing to arch_timer_needs_of_probing.
Please write a real commit message.
> Signed-off-by: Fu Wei <fu.wei@linaro.org>
> ---
> drivers/clocksource/arm_arch_timer.c | 34 +++++++++++++++++++---------------
> 1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index fe4e812..9ddc091 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -792,15 +792,28 @@ static const struct of_device_id arch_timer_mem_of_match[] __initconst = {
> {},
> };
>
> -static bool __init
> -arch_timer_needs_probing(int type, const struct of_device_id *matches)
> +static bool __init arch_timer_needs_of_probing(void)
> {
> struct device_node *dn;
> bool needs_probing = false;
> + unsigned int mask = ARCH_TIMER_TYPE_CP15 | ARCH_TIMER_TYPE_MEM;
> +
> + /* We have two timers, and both device-tree nodes are probed. */
> + if ((arch_timers_present & mask) == mask)
> + return false;
> +
> + /*
> + * Only one type of timer is probed,
> + * check if we have another type of timer node in device-tree.
> + */
> + if (arch_timers_present & ARCH_TIMER_TYPE_CP15)
> + dn = of_find_matching_node(NULL, arch_timer_mem_of_match);
> + else
> + dn = of_find_matching_node(NULL, arch_timer_of_match);
>
> - dn = of_find_matching_node(NULL, matches);
> - if (dn && of_device_is_available(dn) && !(arch_timers_present & type))
> + if (dn && of_device_is_available(dn))
> needs_probing = true;
> +
> of_node_put(dn);
>
> return needs_probing;
> @@ -808,17 +821,8 @@ arch_timer_needs_probing(int type, const struct of_device_id *matches)
>
> static int __init arch_timer_common_init(void)
> {
> - unsigned int mask = ARCH_TIMER_TYPE_CP15 | ARCH_TIMER_TYPE_MEM;
> -
> - /* Wait until both nodes are probed if we have two timers */
> - if ((arch_timers_present & mask) != mask) {
> - if (arch_timer_needs_probing(ARCH_TIMER_TYPE_MEM,
> - arch_timer_mem_of_match))
> - return 0;
> - if (arch_timer_needs_probing(ARCH_TIMER_TYPE_CP15,
> - arch_timer_of_match))
> - return 0;
> - }
Why can't we just move this into the DT-specific caller of
arch_timer_common_init()?
Thanks
Mark.
> + if (acpi_disabled && arch_timer_needs_of_probing())
> + return 0;
>
> arch_timer_banner(arch_timers_present);
> arch_counter_register(arch_timers_present);
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v3 1/3] dt-bindings: firmware: scm: Add MSM8996 DT bindings
From: Bjorn Andersson @ 2016-11-18 19:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479259165-1601-2-git-send-email-spjoshi@codeaurora.org>
On Tue 15 Nov 17:19 PST 2016, Sarangdhar Joshi wrote:
> Add SCM DT bindings for Qualcomm's MSM8996 platform.
>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Sarangdhar Joshi <spjoshi@codeaurora.org>
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
> ---
> Documentation/devicetree/bindings/firmware/qcom,scm.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.txt b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
> index 3b4436e..20f26fb 100644
> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.txt
> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
> @@ -10,8 +10,10 @@ Required properties:
> * "qcom,scm-apq8064" for APQ8064 platforms
> * "qcom,scm-msm8660" for MSM8660 platforms
> * "qcom,scm-msm8690" for MSM8690 platforms
> + * "qcom,scm-msm8996" for MSM8996 platforms
> * "qcom,scm" for later processors (MSM8916, APQ8084, MSM8974, etc)
> - clocks: One to three clocks may be required based on compatible.
> + * No clock required for "qcom,scm-msm8996"
> * Only core clock required for "qcom,scm-apq8064", "qcom,scm-msm8660", and "qcom,scm-msm8960"
> * Core, iface, and bus clocks required for "qcom,scm"
> - clock-names: Must contain "core" for the core clock, "iface" for the interface
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
^ permalink raw reply
* [PATCH v16 07/15] clocksource/drivers/arm_arch_timer: Refactor arch_timer_detect_rate to keep dt code in *_of_init
From: Mark Rutland @ 2016-11-18 19:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479304148-2965-8-git-send-email-fu.wei@linaro.org>
On Wed, Nov 16, 2016 at 09:49:00PM +0800, fu.wei at linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> The patch refactor original arch_timer_detect_rate function:
> (1) Separate out device-tree code, keep them in device-tree init
> function:
> arch_timer_of_init,
> arch_timer_mem_init;
Please write a real commit message.
> (2) Improve original mechanism, if getting from memory-mapped timer
> fail, try arch_timer_get_cntfrq() again.
This is *not* a refactoring. It's completely unrelated to the supposed
refactoring from point (1), and if necessary, should be a separate
patch.
*Why* are you maknig this change? Does some ACPI platform have an MMIO
timer with an ill-configured CNTFRQ register? If so, report that to the
vendor. Don't add yet another needless bodge.
I'd really like to split the MMIO and CP15 timers, and this is yet
another hack that'll make it harder to do so.
> Signed-off-by: Fu Wei <fu.wei@linaro.org>
> ---
> drivers/clocksource/arm_arch_timer.c | 45 +++++++++++++++++++++++-------------
> 1 file changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index af22953..fe4e812 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -487,27 +487,31 @@ static int arch_timer_starting_cpu(unsigned int cpu)
> return 0;
> }
>
> -static void
> -arch_timer_detect_rate(void __iomem *cntbase, struct device_node *np)
> +static void arch_timer_detect_rate(void __iomem *cntbase)
> {
> /* Who has more than one independent system counter? */
> if (arch_timer_rate)
> return;
> -
> /*
> - * Try to determine the frequency from the device tree or CNTFRQ,
> - * if ACPI is enabled, get the frequency from CNTFRQ ONLY.
> + * If we got memory-mapped timer(cntbase != NULL),
> + * try to determine the frequency from CNTFRQ in memory-mapped timer.
> */
*WHY* ?
If we're sharing arch_timer_rate across MMIO and sysreg timers, the
sysreg value is alreayd sufficient.
If we're not, they should be completely independent.
> - if (!acpi_disabled ||
> - of_property_read_u32(np, "clock-frequency", &arch_timer_rate)) {
> - if (cntbase)
> - arch_timer_rate = readl_relaxed(cntbase + CNTFRQ);
> - else
> - arch_timer_rate = arch_timer_get_cntfrq();
> - }
> + if (cntbase)
> + arch_timer_rate = readl_relaxed(cntbase + CNTFRQ);
> + /*
> + * Because in a system that implements both Secure and
> + * Non-secure states, CNTFRQ is only accessible in Secure state.
That's true for the CNTCTLBase frame, but that doesn't matter.
The CNTBase<n> frames should have a readable CNTFRQ.
> + * So the operation above may fail, even if (cntbase != NULL),
> + * especially on ARM64.
> + * In this case, we can try cntfrq_el0(system coprocessor register).
> + */
> + if (!arch_timer_rate)
> + arch_timer_rate = arch_timer_get_cntfrq();
> + else
> + return;
Urrgh.
Please have separate paths to determine the MMIO frequency and the
sysreg frequency, and use the appropriate one for the counter you want
to know the frequency of.
Thanks,
Mark.
^ permalink raw reply
* [PATCH 2/2] i2c: designware: fix rx fifo depth tracking
From: Russell King @ 2016-11-18 19:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118193542.GO1041@n2100.armlinux.org.uk>
When loading the TX fifo to receive bytes on the I2C bus, we incorrectly
count the number of bytes:
rx_limit = dev->rx_fifo_depth - dw_readl(dev, DW_IC_RXFLR);
while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
if (rx_limit - dev->rx_outstanding <= 0)
break;
rx_limit--;
dev->rx_outstanding++;
}
DW_IC_RXFLR indicates how many bytes are available to be read in the
FIFO, dev->rx_fifo_depth is the FIFO size, and dev->rx_outstanding is
the number of bytes that we've requested to be read so far, but which
have not been read.
Firstly, increasing dev->rx_outstanding and decreasing rx_limit and then
comparing them results in each byte consuming "two" bytes in this
tracking, so this is obviously wrong.
Secondly, the number of bytes that _could_ be received into the FIFO at
any time is the number of bytes we have so far requested but not yet
read from the FIFO - in other words dev->rx_outstanding.
So, in order to request enough bytes to fill the RX FIFO, we need to
request dev->rx_fifo_depth - dev->rx_outstanding bytes.
Modifying the code thusly results in us reaching the maximum number of
bytes outstanding each time we queue more "receive" operations, provided
the transfer allows that to happen.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
With this applied, I now get:
[ 1.726388] i2c_designware 7ffa0000.i2c: transfer terminated early - interrupt latency too high?
[ 1.733813] tda998x 0-0070: Error -5 reading from 0x900
[ 1.737708] tda998x 0-0070: failed to read edid block 0: -5
[ 1.743683] tda998x 0-0070: failed to read EDID
which is a more graceful failure than letting DRM detect the bad
transfer by checking the EDID checksum and hoping that the untransferred
bytes don't result in the EDID checksum succeeding.
drivers/i2c/busses/i2c-designware-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 11e866d05368..9703fe392543 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -611,7 +611,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
if (msgs[dev->msg_write_idx].flags & I2C_M_RD) {
/* avoid rx buffer overrun */
- if (rx_limit - dev->rx_outstanding <= 0)
+ if (dev->rx_outstanding >= dev->rx_fifo_depth)
break;
dw_writel(dev, cmd | 0x100, DW_IC_DATA_CMD);
--
2.7.4
^ permalink raw reply related
* [PATCH 1/2] i2c: designware: report short transfers
From: Russell King @ 2016-11-18 19:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118193542.GO1041@n2100.armlinux.org.uk>
Rather than reporting success for a short transfer due to interrupt
latency, report an error both to the caller, as well as to the kernel
log.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/i2c/busses/i2c-designware-core.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 9703fe392543..c53058d6139c 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -758,7 +758,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
}
/* no error */
- if (likely(!dev->cmd_err)) {
+ if (likely(!dev->cmd_err && !dev->status)) {
ret = num;
goto done;
}
@@ -768,6 +768,11 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
ret = i2c_dw_handle_tx_abort(dev);
goto done;
}
+
+ if (dev->status)
+ dev_err(dev->dev,
+ "transfer terminated early - interrupt latency too high?\n");
+
ret = -EIO;
done:
--
2.7.4
^ permalink raw reply related
* [BUG] i2c-designware silently fails on long transfers
From: Russell King - ARM Linux @ 2016-11-18 19:35 UTC (permalink / raw)
To: linux-arm-kernel
With reference to this commit:
commit d39f77b06a712fcba6185a20bb209e357923d980
Author: Andrew Jackson <Andrew.Jackson@arm.com>
Date: Fri Nov 7 12:10:44 2014 +0000
i2c: designware: prevent early stop on TX FIFO empty
If the Designware core is configured with IC_EMPTYFIFO_HOLD_MASTER_EN
set to zero, allowing the TX FIFO to become empty causes a STOP
condition to be generated on the I2C bus. If the transmit FIFO
threshold is set too high, an erroneous STOP condition can be
generated on long transfers - particularly where the interrupt
latency is extended.
Signed-off-by: Andrew Jackson <Andrew.Jackson@arm.com>
Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
The TDA998x driver issues long I2C transfers to read the EDID from the
device - and userspace can also issue large transfers too. However,
if a DW core is configured with IC_EMPTYFIFO_HOLD_MASTER_EN set as
zero, the above commit doesn't seem to solve the problem. During
boot, with the patch below, I see:
[ 1.736549] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x10
[ 1.736564] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x510
[ 1.736608] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x504
[ 1.736799] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x514
[ 1.736819] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x510
...
[ 1.737986] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x504
[ 1.738010] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x514
[ 1.738034] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x504
[ 1.738039] random: fast init done
[ 1.740120] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x714
[ 1.740231] i2c_dw_xfer: ffffffc97657b770:1 -> ffffffc97657b770:1 (0:0) [0 0 3 0] 8 [tx:ffffffc976682380:47] [rx:ffffffc9766823c9:55]
[ 1.740249] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 93
[ 1.746979] Raw EDID:
[ 1.747934] 00 ff ff ff ff ff ff 00 34 a9 96 a2 01 01 01 01
[ 1.752342] 00 17 01 03 80 80 48 78 0a da ff a3 58 4a a2 29
[ 1.756748] 17 49 4b 21 08 00 31 40 45 40 61 40 81 80 01 01
[ 1.761153] 01 01 01 01 01 01 02 3a 80 d0 72 38 2d 40 10 2c
[ 1.765555] 45 80 ba 88 21 00 00 1e 02 00 d0 4e 30 09 12 54
[ 1.769958] 01 08 02 00 23 36 01 40 01 05 00 80 a1 4c 4b 49
[ 1.774361] 22 00 00 40 03 00 28 00 23 01 20 00 01 88 00 01
[ 1.778762] 08 00 00 40 00 02 03 04 0a 00 80 00 02 00 00 40
The significant thing is the "i2c_dw_xfer" line, where I add a print of
the current state. Here, we can see that the transfer is mid-way, but
a stop condition has been generated by the hardware, leaving 55 bytes
to be received.
Unfortunately, the i2c-designware driver ignores this, and believes that
the transfer completed both fully and successfully, but returns bogus
data to userspace or the kernel driver. That's really _bad_ behaviour
by the driver - it should at least return an error.
This problem is _soo_ bad that on my Juno, I can't run Xorg (it hits
this every time we try to read the EDID) nor can I boot with the TV
connected (it hits this every boot as well.)
I'd go as far as to say that the i2c-designware hardware, when
configured with this option set to zero, is fundamentally broken for OS
which do not provide any guarantee for interrupt latency, such as Linux.
The commit above tries to mitigate this by reducing the Tx FIFO
threshold, so the interrupt is raised sooner, but that's clearly not
enough for reliable operation.
Another mitigation would be to lower the I2C bus frequency on Juno from
400kHz to 100kHz, so that there's 4x longer IRQ latency possible.
However, even that isn't going to be reliable - even going to 100kHz
isn't going to allow the above case to be solved - the interrupt is
delayed by around 2ms, and it takes about 1.4ms to send/receive 16 bytes
at 100kHz. (9 * 16 / (100*10^3)).
So, I think all hope is lost for i2c-designware on Juno to cope with
reading the EDID from TDA998x reliably.
I have one patch which solves a problem in the accounting of bytes, and
another to ensure that we return an error for an incomplete transfer,
both will be sent threaded to this mail.
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 11e866d05368..060ae9e5a916 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -752,6 +752,15 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
goto done;
}
+ printk(KERN_DEBUG "%s: %p:%d -> %p:%d (%d:%d) [%x %x %x %x] %d [tx:%p:%d] [rx:%p:%d]\n",
+ __func__, msgs, num,
+ dev->msgs, dev->msgs_num,
+ dev->msg_write_idx, dev->msg_read_idx,
+ dev->cmd_err, dev->msg_err, dev->status, dev->abort_source,
+ dev->rx_outstanding,
+ dev->tx_buf, dev->tx_buf_len,
+ dev->rx_buf, dev->rx_buf_len);
+
if (dev->msg_err) {
ret = dev->msg_err;
goto done;
@@ -857,7 +866,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
enabled = dw_readl(dev, DW_IC_ENABLE);
stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
- dev_dbg(dev->dev, "%s: enabled=%#x stat=%#x\n", __func__, enabled, stat);
+ dev_printk(KERN_DEBUG, dev->dev, "%s: enabled=%#x stat=%#x\n", __func__, enabled, stat);
if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
return IRQ_NONE;
--
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 related
* [PATCH v16 06/15] clocksource/drivers/arm_arch_timer: separate out arch_timer_uses_ppi init code to prepare for GTDT.
From: Mark Rutland @ 2016-11-18 19:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479304148-2965-7-git-send-email-fu.wei@linaro.org>
On Wed, Nov 16, 2016 at 09:48:59PM +0800, fu.wei at linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> The patch refactor original arch_timer_uses_ppi init code:
> (1) Extract a subfunction: arch_timer_uses_ppi_init
> (2) Use the new subfunction in arch_timer_of_init and
> arch_timer_acpi_init
This isn't a strict refactoring, since this now assigns
ARCH_TIMER_PHYS_NONSECURE_PPI to arch_timer_uses_ppi, which we didn't do
previously.
As a general note, please write your commit messages as prose rather
than a list of bullet points. Please also explain the rationale for the
change, rather than enumerating the changes. Call out things which are
important and/or likely to surprise reviewers, for example:
* Can 32-bit ARM still use non-secure interrupts afer this change?
* Does the "arm,cpu-registers-not-fw-configured" proeprty still work?
That will make it vastly easier to have this code reviewed, and it will
be far more helpful for anyone looking at this in future.
For example:
arm_arch_timer: rework PPI determination
Currently, the arch timer driver uses ARCH_TIMER_PHYS_SECURE_PPI to
mean the driver will use the secure PPI *and* potentialy also use the
non-secure PPI. This is somewhat confusing.
For arm64, where it never makes sense to use the secure PPI, this
means we must always request the useless secure PPI, adding to the
confusion. For ACPI, where we may not even have a valid secure PPI
number, this is additionally problematic. We need the driver to be
able to use *only* the non-secure PPI.
The logic to choose which PPI to use is intertwined with other logic
in arch_timer_init(). This patch factors the PPI determination out
into a new function, and then reworks it so that we can handle having
only a non-secure PPI.
[...]
> +/*
> + * If HYP mode is available, we know that the physical timer
> + * has been configured to be accessible from PL1. Use it, so
> + * that a guest can use the virtual timer instead.
> + *
> + * If no interrupt provided for virtual timer, we'll have to
> + * stick to the physical timer. It'd better be accessible...
> + * On ARM64, we we only use ARCH_TIMER_PHYS_NONSECURE_PPI in Linux.
It would be better to say that for arm64 we never use the secure
interrupt.
> + *
> + * On ARMv8.1 with VH extensions, the kernel runs in HYP. VHE
> + * accesses to CNTP_*_EL1 registers are silently redirected to
> + * their CNTHP_*_EL2 counterparts, and use a different PPI
> + * number.
> + */
> +static int __init arch_timer_uses_ppi_init(void)
It would be better to call this something like arch_timer_select_ppi().
As it stands, the name is difficult to read.
> @@ -902,6 +904,10 @@ static int __init arch_timer_of_init(struct device_node *np)
> of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
> arch_timer_uses_ppi = ARCH_TIMER_PHYS_SECURE_PPI;
>
> + ret = arch_timer_uses_ppi_init();
> + if (ret)
> + return ret;
This is clearly broken if you consider what the statement above is
doing.
Thanks,
Mark.
^ permalink raw reply
* [PATCHv3 6/6] arm64: Add support for CONFIG_DEBUG_VIRTUAL
From: Laura Abbott @ 2016-11-18 19:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118190531.GJ1197@leverpostej>
On 11/18/2016 11:05 AM, Mark Rutland wrote:
> On Fri, Nov 18, 2016 at 10:42:56AM -0800, Laura Abbott wrote:
>> On 11/18/2016 09:53 AM, Mark Rutland wrote:
>>> On Thu, Nov 17, 2016 at 05:16:56PM -0800, Laura Abbott wrote:
>
>>>> +#define __virt_to_phys_nodebug(x) ({ \
>>>> phys_addr_t __x = (phys_addr_t)(x); \
>>>> - __x & BIT(VA_BITS - 1) ? (__x & ~PAGE_OFFSET) + PHYS_OFFSET : \
>>>> - (__x - kimage_voffset); })
>>>> + ((__x & ~PAGE_OFFSET) + PHYS_OFFSET); \
>>>> +})
>>>
>>> Given the KASAN failure, and the strong possibility that there's even
>>> more stuff lurking in common code, I think we should retain the logic to
>>> handle kernel image addresses for the timebeing (as x86 does). Once
>>> we've merged DEBUG_VIRTUAL, it will be easier to track those down.
>>
>> Agreed. I might see about adding another option DEBUG_STRICT_VIRTUAL
>> for catching bad __pa vs __pa_symbol usage and keep DEBUG_VIRTUAL for
>> catching addresses that will work in neither case.
>
> I think it makes sense for DEBUG_VIRTUAL to do both, so long as the
> default behaviour (and fallback after a WARN for virt_to_phys()) matches
> what we currently do. We'll get useful diagnostics, but a graceful
> fallback.
>
I was suggesting making the WARN optional for having this be more useful
before all the __pa_symbol stuff gets cleaned up. Maybe the WARN won't
actually be a hindrance.
Thanks,
Laura
> I think the helpers I suggested below do that? Or have I misunderstood,
> and you mean something stricter (e.g. checking whether a lm address is
> is backed by something)?
>
>>> phys_addr_t __virt_to_phys(unsigned long x)
>>> {
>>> WARN(!__is_lm_address(x),
>>> "virt_to_phys() used for non-linear address: %pK\n",
>>> (void*)x);
>>>
>>> return __virt_to_phys_nodebug(x);
>>> }
>>> EXPORT_SYMBOL(__virt_to_phys);
>
>>> phys_addr_t __phys_addr_symbol(unsigned long x)
>>> {
>>> /*
>>> * This is bounds checking against the kernel image only.
>>> * __pa_symbol should only be used on kernel symbol addresses.
>>> */
>>> VIRTUAL_BUG_ON(x < (unsigned long) KERNEL_START ||
>>> x > (unsigned long) KERNEL_END);
>>>
>>> return __pa_symbol_nodebug(x);
>>> }
>>> EXPORT_SYMBOL(__phys_addr_symbol);
>
> Thanks,
> Mark.
>
^ permalink raw reply
* [PATCHv3 6/6] arm64: Add support for CONFIG_DEBUG_VIRTUAL
From: Mark Rutland @ 2016-11-18 19:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <16e4b3da-c552-252d-108a-0681b71b12ef@redhat.com>
On Fri, Nov 18, 2016 at 10:42:56AM -0800, Laura Abbott wrote:
> On 11/18/2016 09:53 AM, Mark Rutland wrote:
> > On Thu, Nov 17, 2016 at 05:16:56PM -0800, Laura Abbott wrote:
> >> +#define __virt_to_phys_nodebug(x) ({ \
> >> phys_addr_t __x = (phys_addr_t)(x); \
> >> - __x & BIT(VA_BITS - 1) ? (__x & ~PAGE_OFFSET) + PHYS_OFFSET : \
> >> - (__x - kimage_voffset); })
> >> + ((__x & ~PAGE_OFFSET) + PHYS_OFFSET); \
> >> +})
> >
> > Given the KASAN failure, and the strong possibility that there's even
> > more stuff lurking in common code, I think we should retain the logic to
> > handle kernel image addresses for the timebeing (as x86 does). Once
> > we've merged DEBUG_VIRTUAL, it will be easier to track those down.
>
> Agreed. I might see about adding another option DEBUG_STRICT_VIRTUAL
> for catching bad __pa vs __pa_symbol usage and keep DEBUG_VIRTUAL for
> catching addresses that will work in neither case.
I think it makes sense for DEBUG_VIRTUAL to do both, so long as the
default behaviour (and fallback after a WARN for virt_to_phys()) matches
what we currently do. We'll get useful diagnostics, but a graceful
fallback.
I think the helpers I suggested below do that? Or have I misunderstood,
and you mean something stricter (e.g. checking whether a lm address is
is backed by something)?
> > phys_addr_t __virt_to_phys(unsigned long x)
> > {
> > WARN(!__is_lm_address(x),
> > "virt_to_phys() used for non-linear address: %pK\n",
> > (void*)x);
> >
> > return __virt_to_phys_nodebug(x);
> > }
> > EXPORT_SYMBOL(__virt_to_phys);
> > phys_addr_t __phys_addr_symbol(unsigned long x)
> > {
> > /*
> > * This is bounds checking against the kernel image only.
> > * __pa_symbol should only be used on kernel symbol addresses.
> > */
> > VIRTUAL_BUG_ON(x < (unsigned long) KERNEL_START ||
> > x > (unsigned long) KERNEL_END);
> >
> > return __pa_symbol_nodebug(x);
> > }
> > EXPORT_SYMBOL(__phys_addr_symbol);
Thanks,
Mark.
^ permalink raw reply
* [PATCH] mfd: twl-core: make driver DT only
From: Lee Jones @ 2016-11-18 19:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478952153-26592-1-git-send-email-Nicolae_Rosia@mentor.com>
On Sat, 12 Nov 2016, Nicolae Rosia wrote:
> All users are DT-only and it makes no sense to keep
> unused code
>
> Signed-off-by: Nicolae Rosia <Nicolae_Rosia@mentor.com>
> ---
> drivers/mfd/Kconfig | 1 +
> drivers/mfd/twl-core.c | 395 ++-----------------------------------------------
> 2 files changed, 10 insertions(+), 386 deletions(-)
I think it would be courteous to add some of the serious contributors
to the review list.
There may still be some out of tree users who might wish for a
heads-up that this change is about to be merged.
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index c6df644..c180f8b 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1333,6 +1333,7 @@ config MFD_TPS80031
> config TWL4030_CORE
> bool "TI TWL4030/TWL5030/TWL6030/TPS659x0 Support"
> depends on I2C=y
> + depends on OF
> select IRQ_DOMAIN
> select REGMAP_I2C
> help
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index c64615d..2025326 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -13,6 +13,9 @@
> * Code cleanup and modifications to IRQ handler.
> * by syed khasim <x0khasim@ti.com>
> *
> + * Code cleanup and modifications:
> + * Copyright (C) 2016 Nicolae Rosia <nicolae.rosia@gmail.com>
> + *
> * 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; either version 2 of the License, or
> @@ -604,376 +607,6 @@ int twl_get_hfclk_rate(void)
> }
> EXPORT_SYMBOL_GPL(twl_get_hfclk_rate);
>
> -static struct device *
> -add_numbered_child(unsigned mod_no, const char *name, int num,
> - void *pdata, unsigned pdata_len,
> - bool can_wakeup, int irq0, int irq1)
> -{
> - struct platform_device *pdev;
> - struct twl_client *twl;
> - int status, sid;
> -
> - if (unlikely(mod_no >= twl_get_last_module())) {
> - pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
> - return ERR_PTR(-EPERM);
> - }
> - sid = twl_priv->twl_map[mod_no].sid;
> - twl = &twl_priv->twl_modules[sid];
> -
> - pdev = platform_device_alloc(name, num);
> - if (!pdev)
> - return ERR_PTR(-ENOMEM);
> -
> - pdev->dev.parent = &twl->client->dev;
> -
> - if (pdata) {
> - status = platform_device_add_data(pdev, pdata, pdata_len);
> - if (status < 0) {
> - dev_dbg(&pdev->dev, "can't add platform_data\n");
> - goto put_device;
> - }
> - }
> -
> - if (irq0) {
> - struct resource r[2] = {
> - { .start = irq0, .flags = IORESOURCE_IRQ, },
> - { .start = irq1, .flags = IORESOURCE_IRQ, },
> - };
> -
> - status = platform_device_add_resources(pdev, r, irq1 ? 2 : 1);
> - if (status < 0) {
> - dev_dbg(&pdev->dev, "can't add irqs\n");
> - goto put_device;
> - }
> - }
> -
> - status = platform_device_add(pdev);
> - if (status)
> - goto put_device;
> -
> - device_init_wakeup(&pdev->dev, can_wakeup);
> -
> - return &pdev->dev;
> -
> -put_device:
> - platform_device_put(pdev);
> - dev_err(&twl->client->dev, "failed to add device %s\n", name);
> - return ERR_PTR(status);
> -}
> -
> -static inline struct device *add_child(unsigned mod_no, const char *name,
> - void *pdata, unsigned pdata_len,
> - bool can_wakeup, int irq0, int irq1)
> -{
> - return add_numbered_child(mod_no, name, -1, pdata, pdata_len,
> - can_wakeup, irq0, irq1);
> -}
> -
> -static struct device *
> -add_regulator_linked(int num, struct regulator_init_data *pdata,
> - struct regulator_consumer_supply *consumers,
> - unsigned num_consumers, unsigned long features)
> -{
> - struct twl_regulator_driver_data drv_data;
> -
> - /* regulator framework demands init_data ... */
> - if (!pdata)
> - return NULL;
> -
> - if (consumers) {
> - pdata->consumer_supplies = consumers;
> - pdata->num_consumer_supplies = num_consumers;
> - }
> -
> - if (pdata->driver_data) {
> - /* If we have existing drv_data, just add the flags */
> - struct twl_regulator_driver_data *tmp;
> - tmp = pdata->driver_data;
> - tmp->features |= features;
> - } else {
> - /* add new driver data struct, used only during init */
> - drv_data.features = features;
> - drv_data.set_voltage = NULL;
> - drv_data.get_voltage = NULL;
> - drv_data.data = NULL;
> - pdata->driver_data = &drv_data;
> - }
> -
> - /* NOTE: we currently ignore regulator IRQs, e.g. for short circuits */
> - return add_numbered_child(TWL_MODULE_PM_MASTER, "twl_reg", num,
> - pdata, sizeof(*pdata), false, 0, 0);
> -}
> -
> -static struct device *
> -add_regulator(int num, struct regulator_init_data *pdata,
> - unsigned long features)
> -{
> - return add_regulator_linked(num, pdata, NULL, 0, features);
> -}
> -
> -/*
> - * NOTE: We know the first 8 IRQs after pdata->base_irq are
> - * for the PIH, and the next are for the PWR_INT SIH, since
> - * that's how twl_init_irq() sets things up.
> - */
> -
> -static int
> -add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
> - unsigned long features)
> -{
> - struct device *child;
> -
> - if (IS_ENABLED(CONFIG_GPIO_TWL4030) && pdata->gpio) {
> - child = add_child(TWL4030_MODULE_GPIO, "twl4030_gpio",
> - pdata->gpio, sizeof(*pdata->gpio),
> - false, irq_base + GPIO_INTR_OFFSET, 0);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> - }
> -
> - if (IS_ENABLED(CONFIG_KEYBOARD_TWL4030) && pdata->keypad) {
> - child = add_child(TWL4030_MODULE_KEYPAD, "twl4030_keypad",
> - pdata->keypad, sizeof(*pdata->keypad),
> - true, irq_base + KEYPAD_INTR_OFFSET, 0);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> - }
> -
> - if (IS_ENABLED(CONFIG_TWL4030_MADC) && pdata->madc &&
> - twl_class_is_4030()) {
> - child = add_child(TWL4030_MODULE_MADC, "twl4030_madc",
> - pdata->madc, sizeof(*pdata->madc),
> - true, irq_base + MADC_INTR_OFFSET, 0);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> - }
> -
> - if (IS_ENABLED(CONFIG_RTC_DRV_TWL4030)) {
> - /*
> - * REVISIT platform_data here currently might expose the
> - * "msecure" line ... but for now we just expect board
> - * setup to tell the chip "it's always ok to SET_TIME".
> - * Eventually, Linux might become more aware of such
> - * HW security concerns, and "least privilege".
> - */
> - child = add_child(TWL_MODULE_RTC, "twl_rtc", NULL, 0,
> - true, irq_base + RTC_INTR_OFFSET, 0);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> - }
> -
> - if (IS_ENABLED(CONFIG_PWM_TWL)) {
> - child = add_child(TWL_MODULE_PWM, "twl-pwm", NULL, 0,
> - false, 0, 0);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> - }
> -
> - if (IS_ENABLED(CONFIG_PWM_TWL_LED)) {
> - child = add_child(TWL_MODULE_LED, "twl-pwmled", NULL, 0,
> - false, 0, 0);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> - }
> -
> - if (IS_ENABLED(CONFIG_TWL4030_USB) && pdata->usb &&
> - twl_class_is_4030()) {
> -
> - static struct regulator_consumer_supply usb1v5 = {
> - .supply = "usb1v5",
> - };
> - static struct regulator_consumer_supply usb1v8 = {
> - .supply = "usb1v8",
> - };
> - static struct regulator_consumer_supply usb3v1 = {
> - .supply = "usb3v1",
> - };
> -
> - /* First add the regulators so that they can be used by transceiver */
> - if (IS_ENABLED(CONFIG_REGULATOR_TWL4030)) {
> - /* this is a template that gets copied */
> - struct regulator_init_data usb_fixed = {
> - .constraints.valid_modes_mask =
> - REGULATOR_MODE_NORMAL
> - | REGULATOR_MODE_STANDBY,
> - .constraints.valid_ops_mask =
> - REGULATOR_CHANGE_MODE
> - | REGULATOR_CHANGE_STATUS,
> - };
> -
> - child = add_regulator_linked(TWL4030_REG_VUSB1V5,
> - &usb_fixed, &usb1v5, 1,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator_linked(TWL4030_REG_VUSB1V8,
> - &usb_fixed, &usb1v8, 1,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator_linked(TWL4030_REG_VUSB3V1,
> - &usb_fixed, &usb3v1, 1,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - }
> -
> - child = add_child(TWL_MODULE_USB, "twl4030_usb",
> - pdata->usb, sizeof(*pdata->usb), true,
> - /* irq0 = USB_PRES, irq1 = USB */
> - irq_base + USB_PRES_INTR_OFFSET,
> - irq_base + USB_INTR_OFFSET);
> -
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - /* we need to connect regulators to this transceiver */
> - if (IS_ENABLED(CONFIG_REGULATOR_TWL4030) && child) {
> - usb1v5.dev_name = dev_name(child);
> - usb1v8.dev_name = dev_name(child);
> - usb3v1.dev_name = dev_name(child);
> - }
> - }
> -
> - if (IS_ENABLED(CONFIG_TWL4030_WATCHDOG) && twl_class_is_4030()) {
> - child = add_child(TWL_MODULE_PM_RECEIVER, "twl4030_wdt", NULL,
> - 0, false, 0, 0);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> - }
> -
> - if (IS_ENABLED(CONFIG_INPUT_TWL4030_PWRBUTTON) && twl_class_is_4030()) {
> - child = add_child(TWL_MODULE_PM_MASTER, "twl4030_pwrbutton",
> - NULL, 0, true, irq_base + 8 + 0, 0);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> - }
> -
> - if (IS_ENABLED(CONFIG_MFD_TWL4030_AUDIO) && pdata->audio &&
> - twl_class_is_4030()) {
> - child = add_child(TWL4030_MODULE_AUDIO_VOICE, "twl4030-audio",
> - pdata->audio, sizeof(*pdata->audio),
> - false, 0, 0);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> - }
> -
> - /* twl4030 regulators */
> - if (IS_ENABLED(CONFIG_REGULATOR_TWL4030) && twl_class_is_4030()) {
> - child = add_regulator(TWL4030_REG_VPLL1, pdata->vpll1,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator(TWL4030_REG_VIO, pdata->vio,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator(TWL4030_REG_VDD1, pdata->vdd1,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator(TWL4030_REG_VDD2, pdata->vdd2,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator(TWL4030_REG_VMMC1, pdata->vmmc1,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator(TWL4030_REG_VDAC, pdata->vdac,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator((features & TWL4030_VAUX2)
> - ? TWL4030_REG_VAUX2_4030
> - : TWL4030_REG_VAUX2,
> - pdata->vaux2, features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator(TWL4030_REG_VINTANA1, pdata->vintana1,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator(TWL4030_REG_VINTANA2, pdata->vintana2,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator(TWL4030_REG_VINTDIG, pdata->vintdig,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> - }
> -
> - /* maybe add LDOs that are omitted on cost-reduced parts */
> - if (IS_ENABLED(CONFIG_REGULATOR_TWL4030) && !(features & TPS_SUBSET)
> - && twl_class_is_4030()) {
> - child = add_regulator(TWL4030_REG_VPLL2, pdata->vpll2,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator(TWL4030_REG_VMMC2, pdata->vmmc2,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator(TWL4030_REG_VSIM, pdata->vsim,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator(TWL4030_REG_VAUX1, pdata->vaux1,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator(TWL4030_REG_VAUX3, pdata->vaux3,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> -
> - child = add_regulator(TWL4030_REG_VAUX4, pdata->vaux4,
> - features);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> - }
> -
> - if (IS_ENABLED(CONFIG_CHARGER_TWL4030) && pdata->bci &&
> - !(features & (TPS_SUBSET | TWL5031))) {
> - child = add_child(TWL_MODULE_MAIN_CHARGE, "twl4030_bci",
> - pdata->bci, sizeof(*pdata->bci), false,
> - /* irq0 = CHG_PRES, irq1 = BCI */
> - irq_base + BCI_PRES_INTR_OFFSET,
> - irq_base + BCI_INTR_OFFSET);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> - }
> -
> - if (IS_ENABLED(CONFIG_TWL4030_POWER) && pdata->power) {
> - child = add_child(TWL_MODULE_PM_MASTER, "twl4030_power",
> - pdata->power, sizeof(*pdata->power), false,
> - 0, 0);
> - if (IS_ERR(child))
> - return PTR_ERR(child);
> - }
> -
> - return 0;
> -}
> -
> -/*----------------------------------------------------------------------*/
> -
> /*
> * These three functions initialize the on-chip clock framework,
> * letting it generate the right frequencies for USB, MADC, and
> @@ -1000,8 +633,7 @@ static inline int __init unprotect_pm_master(void)
> return e;
> }
>
> -static void clocks_init(struct device *dev,
> - struct twl4030_clock_init_data *clock)
> +static void clocks_init(struct device *dev)
> {
> int e = 0;
> struct clk *osc;
> @@ -1031,8 +663,6 @@ static void clocks_init(struct device *dev,
> }
>
> ctrl |= HIGH_PERF_SQ;
> - if (clock && clock->ck32k_lowpwr_enable)
> - ctrl |= CK32K_LOWPWR_EN;
>
> e |= unprotect_pm_master();
> /* effect->MADC+USB ck en */
> @@ -1080,7 +710,6 @@ static struct of_dev_auxdata twl_auxdata_lookup[] = {
> static int
> twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
> {
> - struct twl4030_platform_data *pdata = dev_get_platdata(&client->dev);
> struct device_node *node = client->dev.of_node;
> struct platform_device *pdev;
> const struct regmap_config *twl_regmap_config;
> @@ -1088,8 +717,8 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
> int status;
> unsigned i, num_slaves;
>
> - if (!node && !pdata) {
> - dev_err(&client->dev, "no platform data\n");
> + if (!node) {
> + dev_err(&client->dev, "no DT info\n");
> return -EINVAL;
> }
>
> @@ -1177,7 +806,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
> twl_priv->ready = true;
>
> /* setup clock framework */
> - clocks_init(&pdev->dev, pdata ? pdata->clock : NULL);
> + clocks_init(&pdev->dev);
>
> /* read TWL IDCODE Register */
> if (twl_class_is_4030()) {
> @@ -1225,14 +854,8 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
> TWL4030_DCDC_GLOBAL_CFG);
> }
>
> - if (node) {
> - if (pdata)
> - twl_auxdata_lookup[0].platform_data = pdata->gpio;
> - status = of_platform_populate(node, NULL, twl_auxdata_lookup,
> - &client->dev);
> - } else {
> - status = add_children(pdata, irq_base, id->driver_data);
> - }
> + status = of_platform_populate(node, NULL, twl_auxdata_lookup,
> + &client->dev);
>
> fail:
> if (status < 0)
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [GIT PULL 4/4] bcm2835-defconfig-64-next-2016-11-18
From: Eric Anholt @ 2016-11-18 18:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118185835.14452-1-eric@anholt.net>
Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
are available in the git repository at:
https://github.com/anholt/linux tags/bcm2835-defconfig-64-next-2016-11-18
for you to fetch changes up to ac178e4280e65f4d0d14b13a7bfec3a43ff90e66:
ARM64: bcm2835: add thermal driver to default config (2016-11-11 09:00:00 -0800)
----------------------------------------------------------------
This pull enables the BCM2837 (Pi 3) thermal driver in the defconfig.
----------------------------------------------------------------
Martin Sperl (1):
ARM64: bcm2835: add thermal driver to default config
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
^ permalink raw reply
* [GIT PULL 3/4] bcm2835-defconfig-next-2016-11-18
From: Eric Anholt @ 2016-11-18 18:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118185835.14452-1-eric@anholt.net>
Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
are available in the git repository at:
https://github.com/anholt/linux tags/bcm2835-defconfig-next-2016-11-18
for you to fetch changes up to bab0cb90550467c71f4e1b73da406a2280c4f418:
ARM: bcm2835: add thermal driver to default config (2016-11-11 09:00:37 -0800)
----------------------------------------------------------------
This pull request enables the BCM2835 (Raspberry Pi) thermal driver in
the Pi1 defconfig.
----------------------------------------------------------------
Martin Sperl (1):
ARM: bcm2835: add thermal driver to default config
arch/arm/configs/bcm2835_defconfig | 2 ++
1 file changed, 2 insertions(+)
^ permalink raw reply
* [GIT PULL 2/4] bcm2835-dt-64-next-2016-11-18
From: Eric Anholt @ 2016-11-18 18:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118185835.14452-1-eric@anholt.net>
Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
are available in the git repository at:
https://github.com/anholt/linux tags/bcm2835-dt-64-next-2016-11-18
for you to fetch changes up to a44e87b47148c6ee6b78509f47e6a15c0fae890a:
ARM64: dts: bcm2837-rpi-3-b: remove incorrect pwr LED (2016-11-16 13:49:38 -0800)
----------------------------------------------------------------
This pull request brings thermal support to the BCM2837 DT, and a few
other fixes.
In order to get the thermal node that we're adjusting the compatible
string on, we have to merge in the bcm2835-dt-next branch.
----------------------------------------------------------------
Andrea Merello (1):
ARM64: dts: bcm2837-rpi-3-b: remove incorrect pwr LED
Andreas F?rber (1):
ARM64: dts: bcm2835: Fix bcm2837 compatible string
Eric Anholt (2):
ARM: dts: bcm283x: Define standard pinctrl groups in the gpio node.
Merge branch 'bcm2835-dt-next' into bcm2835-dt-64-next
Gerd Hoffmann (6):
pinctrl: bcm2835: add pull defines to dt bindings
ARM: dts: bcm283x: add pinctrl group to &pwm, drop pins from &gpio
ARM: dts: bcm283x: add pinctrl group to &i2c0, drop pins from &gpio
ARM: dts: bcm283x: add pinctrl group to &i2c1, drop pins from &gpio
ARM: dts: bcm283x: add pinctrl group to &sdhci, drop pins from &gpio
ARM: dts: bcm283x: drop alt3 from &gpio
Linus Walleij (1):
ARM: bcm2835: Add names for the Raspberry Pi GPIO lines
Martin Sperl (3):
dt: bindings: add thermal device driver for bcm2835
ARM: bcm2835: dts: add thermal node to device-tree of bcm283x
ARM64: bcm2835: dts: add thermal node to device-tree of bcm2837
Stefan Wahren (3):
ARM64: dts: bcm283x: Use dtsi for USB host mode
DT: binding: bcm2835-mbox: fix address typo in example
ARM: dts: bcm283x: fix typo in mailbox address
.../bindings/mailbox/brcm,bcm2835-mbox.txt | 2 +-
.../bindings/thermal/brcm,bcm2835-thermal.txt | 17 ++
arch/arm/boot/dts/bcm2835-rpi-a-plus.dts | 67 ++++++-
arch/arm/boot/dts/bcm2835-rpi-a.dts | 69 ++++++-
arch/arm/boot/dts/bcm2835-rpi-b-plus.dts | 68 ++++++-
arch/arm/boot/dts/bcm2835-rpi-b-rev2.dts | 68 ++++++-
arch/arm/boot/dts/bcm2835-rpi-b.dts | 69 ++++++-
arch/arm/boot/dts/bcm2835-rpi-zero.dts | 2 +-
arch/arm/boot/dts/bcm2835-rpi.dtsi | 15 +-
arch/arm/boot/dts/bcm2835.dtsi | 6 +
arch/arm/boot/dts/bcm2836-rpi-2-b.dts | 2 +-
arch/arm/boot/dts/bcm2836.dtsi | 6 +
arch/arm/boot/dts/bcm283x.dtsi | 212 ++++++++++++++++++++-
arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts | 8 +-
arch/arm64/boot/dts/broadcom/bcm2837.dtsi | 8 +-
.../boot/dts/broadcom/bcm283x-rpi-usb-host.dtsi | 1 +
drivers/pinctrl/bcm/pinctrl-bcm2835.c | 6 -
include/dt-bindings/pinctrl/bcm2835.h | 5 +
18 files changed, 602 insertions(+), 29 deletions(-)
create mode 100644 Documentation/devicetree/bindings/thermal/brcm,bcm2835-thermal.txt
create mode 120000 arch/arm64/boot/dts/broadcom/bcm283x-rpi-usb-host.dtsi
^ permalink raw reply
* [GIT PULL 1/4] bcm2835-dt-next-2016-11-18
From: Eric Anholt @ 2016-11-18 18:58 UTC (permalink / raw)
To: linux-arm-kernel
Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
are available in the git repository at:
https://github.com/anholt/linux tags/bcm2835-dt-next-2016-11-18
for you to fetch changes up to 3a1689ea752436917c5ce4487527ed6c444630ee:
ARM: bcm2835: Add names for the RPi Zero GPIO lines (2016-11-16 13:54:36 -0800)
----------------------------------------------------------------
This pull request brings in DT changes for BCM2835: pinctrl setup
cleanups, GPIO line naming, and the node for the new thermal driver.
----------------------------------------------------------------
Eric Anholt (1):
ARM: dts: bcm283x: Define standard pinctrl groups in the gpio node.
Gerd Hoffmann (6):
pinctrl: bcm2835: add pull defines to dt bindings
ARM: dts: bcm283x: add pinctrl group to &pwm, drop pins from &gpio
ARM: dts: bcm283x: add pinctrl group to &i2c0, drop pins from &gpio
ARM: dts: bcm283x: add pinctrl group to &i2c1, drop pins from &gpio
ARM: dts: bcm283x: add pinctrl group to &sdhci, drop pins from &gpio
ARM: dts: bcm283x: drop alt3 from &gpio
Linus Walleij (1):
ARM: bcm2835: Add names for the Raspberry Pi GPIO lines
Martin Sperl (2):
dt: bindings: add thermal device driver for bcm2835
ARM: bcm2835: dts: add thermal node to device-tree of bcm283x
Stefan Wahren (4):
DT: binding: bcm2835-mbox: fix address typo in example
ARM: dts: bcm283x: fix typo in mailbox address
ARM: bcm2835: Fix names for the Raspberry Pi GPIO lines
ARM: bcm2835: Add names for the RPi Zero GPIO lines
.../bindings/mailbox/brcm,bcm2835-mbox.txt | 2 +-
.../bindings/thermal/brcm,bcm2835-thermal.txt | 17 ++
arch/arm/boot/dts/bcm2835-rpi-a-plus.dts | 67 ++++++-
arch/arm/boot/dts/bcm2835-rpi-a.dts | 69 ++++++-
arch/arm/boot/dts/bcm2835-rpi-b-plus.dts | 68 ++++++-
arch/arm/boot/dts/bcm2835-rpi-b-rev2.dts | 68 ++++++-
arch/arm/boot/dts/bcm2835-rpi-b.dts | 69 ++++++-
arch/arm/boot/dts/bcm2835-rpi-zero.dts | 67 ++++++-
arch/arm/boot/dts/bcm2835-rpi.dtsi | 15 +-
arch/arm/boot/dts/bcm2835.dtsi | 6 +
arch/arm/boot/dts/bcm2836-rpi-2-b.dts | 2 +-
arch/arm/boot/dts/bcm2836.dtsi | 6 +
arch/arm/boot/dts/bcm283x.dtsi | 212 ++++++++++++++++++++-
drivers/pinctrl/bcm/pinctrl-bcm2835.c | 6 -
include/dt-bindings/pinctrl/bcm2835.h | 5 +
15 files changed, 658 insertions(+), 21 deletions(-)
create mode 100644 Documentation/devicetree/bindings/thermal/brcm,bcm2835-thermal.txt
^ 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