* [PATCH 1/3] coresight: tmc: Cleanup operation mode handling
From: Suzuki K Poulose @ 2016-09-27 13:44 UTC (permalink / raw)
To: linux-arm-kernel
The mode of operation of the TMC tracked in drvdata->mode is defined
as a local_t type. This is always checked and modified under the
drvdata->spinlock and hence we don't need local_t for it and the
unnecessary synchronisation instructions that comes with it. This
change makes the code a bit more cleaner.
Also fixes the order in which we update the drvdata->mode to
CS_MODE_DISABLED. i.e, in tmc_disable_etX_sink we change the
mode to CS_MODE_DISABLED before invoking tmc_disable_etX_hw()
which in turn depends on the mode to decide whether to dump the
trace to a buffer.
Applies on mathieu's coresight/next tree [1]
https://git.linaro.org/kernel/coresight.git next
Reported-by: Venkatesh Vivekanandan <venkatesh.vivekanandan@broadcom.com>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
drivers/hwtracing/coresight/coresight-tmc-etf.c | 32 +++++++++++--------------
drivers/hwtracing/coresight/coresight-tmc-etr.c | 26 +++++++++-----------
drivers/hwtracing/coresight/coresight-tmc.h | 2 +-
3 files changed, 26 insertions(+), 34 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index d6941ea..e80a8f4 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -70,7 +70,7 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
* When operating in sysFS mode the content of the buffer needs to be
* read before the TMC is disabled.
*/
- if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
+ if (drvdata->mode == CS_MODE_SYSFS)
tmc_etb_dump_hw(drvdata);
tmc_disable_hw(drvdata);
@@ -108,7 +108,6 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev, u32 mode)
int ret = 0;
bool used = false;
char *buf = NULL;
- long val;
unsigned long flags;
struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
@@ -138,13 +137,12 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev, u32 mode)
goto out;
}
- val = local_xchg(&drvdata->mode, mode);
/*
* In sysFS mode we can have multiple writers per sink. Since this
* sink is already enabled no memory is needed and the HW need not be
* touched.
*/
- if (val == CS_MODE_SYSFS)
+ if (drvdata->mode == CS_MODE_SYSFS)
goto out;
/*
@@ -163,6 +161,7 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev, u32 mode)
drvdata->buf = buf;
}
+ drvdata->mode = CS_MODE_SYSFS;
tmc_etb_enable_hw(drvdata);
out:
spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -180,7 +179,6 @@ out:
static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, u32 mode)
{
int ret = 0;
- long val;
unsigned long flags;
struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
@@ -194,17 +192,17 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, u32 mode)
goto out;
}
- val = local_xchg(&drvdata->mode, mode);
/*
* In Perf mode there can be only one writer per sink. There
* is also no need to continue if the ETB/ETR is already operated
* from sysFS.
*/
- if (val != CS_MODE_DISABLED) {
+ if (drvdata->mode != CS_MODE_DISABLED) {
ret = -EINVAL;
goto out;
}
+ drvdata->mode = mode;
tmc_etb_enable_hw(drvdata);
out:
spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -227,7 +225,6 @@ static int tmc_enable_etf_sink(struct coresight_device *csdev, u32 mode)
static void tmc_disable_etf_sink(struct coresight_device *csdev)
{
- long val;
unsigned long flags;
struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
@@ -237,10 +234,11 @@ static void tmc_disable_etf_sink(struct coresight_device *csdev)
return;
}
- val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
/* Disable the TMC only if it needs to */
- if (val != CS_MODE_DISABLED)
+ if (drvdata->mode != CS_MODE_DISABLED) {
tmc_etb_disable_hw(drvdata);
+ drvdata->mode = CS_MODE_DISABLED;
+ }
spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -260,7 +258,7 @@ static int tmc_enable_etf_link(struct coresight_device *csdev,
}
tmc_etf_enable_hw(drvdata);
- local_set(&drvdata->mode, CS_MODE_SYSFS);
+ drvdata->mode = CS_MODE_SYSFS;
spin_unlock_irqrestore(&drvdata->spinlock, flags);
dev_info(drvdata->dev, "TMC-ETF enabled\n");
@@ -280,7 +278,7 @@ static void tmc_disable_etf_link(struct coresight_device *csdev,
}
tmc_etf_disable_hw(drvdata);
- local_set(&drvdata->mode, CS_MODE_DISABLED);
+ drvdata->mode = CS_MODE_DISABLED;
spin_unlock_irqrestore(&drvdata->spinlock, flags);
dev_info(drvdata->dev, "TMC disabled\n");
@@ -383,7 +381,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
return;
/* This shouldn't happen */
- if (WARN_ON_ONCE(local_read(&drvdata->mode) != CS_MODE_PERF))
+ if (WARN_ON_ONCE(drvdata->mode != CS_MODE_PERF))
return;
CS_UNLOCK(drvdata->base);
@@ -504,7 +502,6 @@ const struct coresight_ops tmc_etf_cs_ops = {
int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
{
- long val;
enum tmc_mode mode;
int ret = 0;
unsigned long flags;
@@ -528,9 +525,8 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
goto out;
}
- val = local_read(&drvdata->mode);
/* Don't interfere if operated from Perf */
- if (val == CS_MODE_PERF) {
+ if (drvdata->mode == CS_MODE_PERF) {
ret = -EINVAL;
goto out;
}
@@ -542,7 +538,7 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
}
/* Disable the TMC if need be */
- if (val == CS_MODE_SYSFS)
+ if (drvdata->mode == CS_MODE_SYSFS)
tmc_etb_disable_hw(drvdata);
drvdata->reading = true;
@@ -573,7 +569,7 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata)
}
/* Re-enable the TMC if need be */
- if (local_read(&drvdata->mode) == CS_MODE_SYSFS) {
+ if (drvdata->mode == CS_MODE_SYSFS) {
/*
* The trace run will continue with the same allocated trace
* buffer. As such zero-out the buffer so that we don't end
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 886ea83..f23ef0c 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -86,7 +86,7 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
* When operating in sysFS mode the content of the buffer needs to be
* read before the TMC is disabled.
*/
- if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
+ if (drvdata->mode == CS_MODE_SYSFS)
tmc_etr_dump_hw(drvdata);
tmc_disable_hw(drvdata);
@@ -97,7 +97,6 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev, u32 mode)
{
int ret = 0;
bool used = false;
- long val;
unsigned long flags;
void __iomem *vaddr = NULL;
dma_addr_t paddr;
@@ -134,13 +133,12 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev, u32 mode)
goto out;
}
- val = local_xchg(&drvdata->mode, mode);
/*
* In sysFS mode we can have multiple writers per sink. Since this
* sink is already enabled no memory is needed and the HW need not be
* touched.
*/
- if (val == CS_MODE_SYSFS)
+ if (drvdata->mode == CS_MODE_SYSFS)
goto out;
/*
@@ -157,6 +155,7 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev, u32 mode)
memset(drvdata->vaddr, 0, drvdata->size);
+ drvdata->mode = CS_MODE_SYSFS;
tmc_etr_enable_hw(drvdata);
out:
spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -174,7 +173,6 @@ out:
static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, u32 mode)
{
int ret = 0;
- long val;
unsigned long flags;
struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
@@ -188,17 +186,17 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, u32 mode)
goto out;
}
- val = local_xchg(&drvdata->mode, mode);
/*
* In Perf mode there can be only one writer per sink. There
* is also no need to continue if the ETR is already operated
* from sysFS.
*/
- if (val != CS_MODE_DISABLED) {
+ if (drvdata->mode != CS_MODE_DISABLED) {
ret = -EINVAL;
goto out;
}
+ drvdata->mode = CS_MODE_PERF;
tmc_etr_enable_hw(drvdata);
out:
spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -221,7 +219,6 @@ static int tmc_enable_etr_sink(struct coresight_device *csdev, u32 mode)
static void tmc_disable_etr_sink(struct coresight_device *csdev)
{
- long val;
unsigned long flags;
struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
@@ -231,10 +228,11 @@ static void tmc_disable_etr_sink(struct coresight_device *csdev)
return;
}
- val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
/* Disable the TMC only if it needs to */
- if (val != CS_MODE_DISABLED)
+ if (drvdata->mode != CS_MODE_DISABLED) {
tmc_etr_disable_hw(drvdata);
+ drvdata->mode = CS_MODE_DISABLED;
+ }
spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -253,7 +251,6 @@ const struct coresight_ops tmc_etr_cs_ops = {
int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
{
int ret = 0;
- long val;
unsigned long flags;
/* config types are set a boot time and never change */
@@ -266,9 +263,8 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
goto out;
}
- val = local_read(&drvdata->mode);
/* Don't interfere if operated from Perf */
- if (val == CS_MODE_PERF) {
+ if (drvdata->mode == CS_MODE_PERF) {
ret = -EINVAL;
goto out;
}
@@ -280,7 +276,7 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
}
/* Disable the TMC if need be */
- if (val == CS_MODE_SYSFS)
+ if (drvdata->mode == CS_MODE_SYSFS)
tmc_etr_disable_hw(drvdata);
drvdata->reading = true;
@@ -303,7 +299,7 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)
spin_lock_irqsave(&drvdata->spinlock, flags);
/* RE-enable the TMC if need be */
- if (local_read(&drvdata->mode) == CS_MODE_SYSFS) {
+ if (drvdata->mode == CS_MODE_SYSFS) {
/*
* The trace run will continue with the same allocated trace
* buffer. The trace buffer is cleared in tmc_etr_enable_hw(),
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 44b3ae3..51c0185 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -117,7 +117,7 @@ struct tmc_drvdata {
void __iomem *vaddr;
u32 size;
u32 len;
- local_t mode;
+ u32 mode;
enum tmc_config_type config_type;
enum tmc_mem_intf_width memwidth;
u32 trigger_cntr;
--
2.7.4
^ permalink raw reply related
* [PATCH 2/2] drm/rockchip: analogix_dp: Refuse to enable PSR if panel doesn't support it
From: Sean Paul @ 2016-09-27 13:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474639600-30090-2-git-send-email-tomeu.vizoso@collabora.com>
On Fri, Sep 23, 2016 at 10:06 AM, Tomeu Vizoso
<tomeu.vizoso@collabora.com> wrote:
> There's no point in enabling PSR when the panel doesn't support it.
>
> This also avoids a problem when PSR gets enabled when a CRTC is being
> disabled, because sometimes in that situation the DSP_HOLD_VALID_INTR
> interrupt on which we wait will never arrive. This was observed on
> RK3288 with a panel without PSR (veyron-jaq Chromebook).
>
> It's very easy to reproduce by running the kms_rmfb test in IGT a few
> times.
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Thanks for digging into this.
Reviewed-by: Sean Paul <seanpaul@chromium.org>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Yakir Yang <ykk@rock-chips.com>
> Cc: Archit Taneja <architt@codeaurora.org>
> ---
> drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index e83be157cc2a..8548e8271639 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -85,6 +85,9 @@ static void analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled)
> struct rockchip_dp_device *dp = to_dp(encoder);
> unsigned long flags;
>
> + if (!analogix_dp_psr_supported(dp->dev))
> + return;
> +
> dev_dbg(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit");
>
> spin_lock_irqsave(&dp->psr_lock, flags);
> --
> 2.7.4
>
^ permalink raw reply
* [RFC] irqchip/gic-v3: Implement suspend and resume callbacks
From: Sudeep Holla @ 2016-09-27 13:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57EA6774.2060807@codeaurora.org>
On 27/09/16 13:35, Chandra Sekhar Lingutla wrote:
> On 09/21/2016 03:40 PM, Marc Zyngier wrote:
>> +Sudeep, Lorenzo,
[...]
>>
>> But here's my fundamental objection: None of that should be required and
>> setting (IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND) as part of the
>> irqchip flags should be enough.
>>
>> Can you explain why this doesn't work for you?
>>
> These flags work for me, I will remove suspend/resume functions, but i
> think it is very useful to know the wakeup source for debugging purposes.
> Please let me know, if you have any better way to achieve this.
>
I don't understand what's missing ? As the user you can set the wakeup
source if it's wakeup capable. You will find the sysfs entries which can
say if it's enabled or not. You can use the same to enable it.
I am not sure what you mean by "it is very useful to know the wakeup
source for debugging purposes".
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH 5/5] tty: amba-pl011: Add earlycon support for SBSA UART
From: Kefeng Wang @ 2016-09-27 13:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160927105748.GA3847@kroah.com>
On 2016/9/27 18:57, Greg Kroah-Hartman wrote:
> On Sat, Sep 24, 2016 at 05:14:25PM +0800, Kefeng Wang wrote:
>> Declare an OF early console for SBSA UART so that the early console device
>> can be specified via the "stdout-path" property in device-tree.
>>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>> drivers/tty/serial/amba-pl011.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
>> index 7d9b291..3688d3b 100644
>> --- a/drivers/tty/serial/amba-pl011.c
>> +++ b/drivers/tty/serial/amba-pl011.c
>> @@ -2330,6 +2330,7 @@ static int __init pl011_early_console_setup(struct earlycon_device *device,
>> return 0;
>> }
>> OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup);
>> +OF_EARLYCON_DECLARE(pl011, "arm,sbsa-uart", pl011_early_console_setup);
>
> Why do you need another option for the same thing?
It is used to support earlycon(without option) for sbsa-uart in bootargs.
chosen {
stdout-path = "serial0:115200n8";
bootargs = "earlycon"
};
uart0: uart at 602b0000 {
compatible = "arm,sbsa-uart";
reg = <0x0 0x602b0000 0x0 0x1000>;
...
};
We setup a unique struct with compatible name by OF_EARLYCON_DECLARE,
#define OF_EARLYCON_DECLARE(_name, compat, fn) \
static const struct earlycon_id __UNIQUE_ID(__earlycon_##_name) \
__used __section(__earlycon_table) \
= { .name = __stringify(_name), \
.compatible = compat, \
.setup = fn }
if without this patch(see drivers/of/fdt.c),
early_init_dt_scan_chosen_serial()
- for (match = __earlycon_table; match < __earlycon_table_end; match++)
-- if (fdt_node_check_compatible(fdt, offset, match->compatible))
countinue;
-- of_setup_earlycon(match, offset, options); // will never touch here.
Thanks,
Kefeng
>
> confused,
>
> greg k-h
>
> .
>
^ permalink raw reply
* [PATCH v2 3/8] i2c: bcm2835: Use ratelimited logging on transfer errors
From: Martin Sperl @ 2016-09-27 13:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474977426-3272-4-git-send-email-noralf@tronnes.org>
> On 27 Sep 2016, at 13:57, Noralf Tr?nnes <noralf@tronnes.org> wrote:
>
> Writing to an AT24C32 generates on average 2x i2c transfer errors per
> 32-byte page write. Which amounts to a lot for a 4k write. This is due
> to the fact that the chip doesn't respond during it's internal write
> cycle when the at24 driver tries and retries the next write.
> Reduce this flooding of the log by using dev_err_ratelimited().
>
> Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
> Reviewed-by: Eric Anholt <eric@anholt.net>
> ---
> drivers/i2c/busses/i2c-bcm2835.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
> index df036ed..370a322 100644
> --- a/drivers/i2c/busses/i2c-bcm2835.c
> +++ b/drivers/i2c/busses/i2c-bcm2835.c
> @@ -207,7 +207,8 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
> (msg->flags & I2C_M_IGNORE_NAK))
> return 0;
>
> - dev_err(i2c_dev->dev, "i2c transfer failed: %x\n", i2c_dev->msg_err);
> + dev_err_ratelimited(i2c_dev->dev, "i2c transfer failed: %x\n",
> + i2c_dev->msg_err);
Do we really need this error message at all?
Maybe just remove it instead, because error messages during
"normal"/successfull operations of at24 seems strange.
Or make it a debug message instead.
Martin
^ permalink raw reply
* [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
From: Robin Murphy @ 2016-09-27 12:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57EA632C.3000208@codeaurora.org>
On 27/09/16 13:16, Srinivas Ramana wrote:
> Hi Robin,
Sorry! This one had slipped my mind already...
> On 09/13/2016 08:22 PM, Srinivas Ramana wrote:
>> On 09/12/2016 11:21 PM, Robin Murphy wrote:
>>> On 12/09/16 07:57, Srinivas Ramana wrote:
>>>> If the bootloader uses the long descriptor format and jumps to
>>>> kernel decompressor code, TTBCR may not be in a right state.
>>>> Before enabling the MMU, it is required to clear the TTBCR.PD0
>>>> field to use TTBR0 for translation table walks.
>>>>
>>>> The 'commit dbece45894d3a ("ARM: 7501/1: decompressor:
>>>> reset ttbcr for VMSA ARMv7 cores")' does the reset of TTBCR.N, but
>>>> doesn't consider all the bits for the size of TTBCR.N.
>>>>
>>>> Clear TTBCR.PD0 field and reset all the three bits of TTBCR.N to
>>>> indicate the use of TTBR0 and the correct base address width.
>>>>
>>>> Signed-off-by: Srinivas Ramana <sramana@codeaurora.org>
>>>> ---
>>>> arch/arm/boot/compressed/head.S | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/boot/compressed/head.S
>>>> b/arch/arm/boot/compressed/head.S
>>>> index af11c2f8f3b7..fc6d541549a2 100644
>>>> --- a/arch/arm/boot/compressed/head.S
>>>> +++ b/arch/arm/boot/compressed/head.S
>>>> @@ -779,7 +779,7 @@ __armv7_mmu_cache_on:
>>>> orrne r0, r0, #1 @ MMU enabled
>>>> movne r1, #0xfffffffd @ domain 0 = client
>>>> bic r6, r6, #1 << 31 @ 32-bit translation system
>>>
>>> Hmm, if TTBCR.EAE _was_ actually set...
>>>
>>>> - bic r6, r6, #3 << 0 @ use only ttbr0
>>>> + bic r6, r6, #(7 << 0) | (1 << 4) @ use only ttbr0
>>>> mcrne p15, 0, r3, c2, c0, 0 @ load page table pointer
>>>> mcrne p15, 0, r1, c3, c0, 0 @ load domain access
>>>> control
>>>> mcrne p15, 0, r6, c2, c0, 2 @ load ttb control
>>>
>>> ...then strictly the TLBIALL needs to happen after the ISB following
>>> this update. Otherwise per B3.10.2 of DDI406C.c I think we might be into
>>> unpredictable territory - i.e. if the TLB happens to treat long- and
>>> short-descriptor entries differently then the TLBI beforehand (with EAE
>>> set) may be at liberty to only discard long-descriptor entries and leave
>>> bogus short-descriptor entries sitting around.
>> Yes, it seems this has to be taken care of, along with resetting
>> TTBCR.PD0 and TTBCR.N. Do you say that this needs to be done in the same
>> patch or a different one?
>>>
>>> In other words, something like (completely untested):
>>>
>>> ---8<---
>>> diff --git a/arch/arm/boot/compressed/head.S
>>> b/arch/arm/boot/compressed/head.S
>>> index af11c2f8f3b7..536b7781024a 100644
>>> --- a/arch/arm/boot/compressed/head.S
>>> +++ b/arch/arm/boot/compressed/head.S
>>> @@ -764,7 +764,6 @@ __armv7_mmu_cache_on:
>>> mov r0, #0
>>> mcr p15, 0, r0, c7, c10, 4 @ drain write buffer
>>> tst r11, #0xf @ VMSA
>>> - mcrne p15, 0, r0, c8, c7, 0 @ flush I,D TLBs
>>
>> Shouldn't this be still there for the same reason you explained above? I
>> mean to discard the long descriptor entries when EAE was 1 (before we
>> reset it).
>>> #endif
>>> mrc p15, 0, r0, c1, c0, 0 @ read control reg
>>> bic r0, r0, #1 << 28 @ clear SCTLR.TRE
>>> @@ -783,8 +782,11 @@ __armv7_mmu_cache_on:
>>> mcrne p15, 0, r3, c2, c0, 0 @ load page table
>>> pointer
>>> mcrne p15, 0, r1, c3, c0, 0 @ load domain access
>>> control
>>> mcrne p15, 0, r6, c2, c0, 2 @ load ttb control
>>> -#endif
>>> mcr p15, 0, r0, c7, c5, 4 @ ISB
>>> + mcrne p15, 0, r0, c8, c7, 0 @ flush I,D TLBs
>>> +#else
>>> + mcr p15, 0, r0, c7, c5, 4 @ ISB
>>> +#endif
>>> mcr p15, 0, r0, c1, c0, 0 @ load control register
>>> mrc p15, 0, r0, c1, c0, 0 @ and read it back
>>> ---8<---
>>>
>>> Robin.
>>>
>> i have tested this change (flush I, D, TLBs after TTB control is
>> written) and don't see any issue. But on my setup decompression is
>> successful even without this (probably not hitting the case in
>> discussion).
>>
>>
>> Thanks,
>> -- Srinivas R
>>
>
> Would like your feedback on the above. Can we get the TTBCR fix merged
> first?(will send final patch with Russell Kings comments fixed)
>
> For testing the TLB flush change we may have to check if we can create a
> failure case.
Yeah, the TLBI being in the wrong place is a separate, pre-existing
problem; as far as this patch goes, it does what it claims to do, and
matches what the ARMv7 (and ARMv6) docs say, so:
Acked-by: Robin Murphy <robin.murphy@arm.com>
>
> Thanks,
> -- Srinivas R
>
^ permalink raw reply
* [PATCH/RFT 0/4] ARM: shmobile: R-Car Gen2: Allow booting secondary CPU cores in debug mode
From: Geert Uytterhoeven @ 2016-09-27 12:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1471877069-4157-1-git-send-email-geert+renesas@glider.be>
Hi Simon, Magnus,
On Mon, Aug 22, 2016 at 4:44 PM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> This patch series is an attempt to allow booting secondary CPU cores on
> R-Car Gen2 when hardware debug mode is enabled. In this mode, reset
> requests derived from power-shutoff to the AP-system CPU cores must be
> enabled before the AP-system cores first resume from power-shutoff. Else
> resume may fail, causing the system to hang during boot. Currently we
> avoid the hang by prohibiting booting secondary CPU cores when hardware
> debug mode is enabled.
>
> On all R-Car Gen2 SoCs, hardware debug mode is enabled by setting
> MD21=1. On both Koelsch and Lager, this is done by setting mode switch
> SW8-4 to OFF.
>
> Unfortunately the hang is not easy to reproduce: I only saw it (on
> Koelsch) during real cold boot (power off during the night), and even
> then it's not guaranteed to trigger. Pressing the reset button
> afterwards recovers the system, and a subsequent boot will succeed
> (incl. secondary CPU core boot).
>
> This series configures the reset requests as documented in the R-Car
> Gen2 datasheet, and removes the check for MD21 during secondary CPU
> bringup. It was inspired by CPU-specific patches in the BSP by
> Nakamura-san.
>
> This series has been boot-tested on r8a7791/koelsch (both debug mode and
> normal mode), on r8a7790/lager and r8a7793/gose (normal mode only), and
> on r8a7794/alt (normal mode UP only).
Any comments?
Any objection to applying this series?
I've been running my Koelsch with MD21=1 since I posted this series,
and it has been included in renesas-drivers since the beginning of September.
My main motivation to push this is that it removes two more users of
rcar_gen2_read_mode_pins(). After this, the only remaining user is the
clock driver, invoked from rcar_gen2_timer_init().
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [RFC] irqchip/gic-v3: Implement suspend and resume callbacks
From: Chandra Sekhar Lingutla @ 2016-09-27 12:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57E25C8E.7080502@arm.com>
On 09/21/2016 03:40 PM, Marc Zyngier wrote:
> +Sudeep, Lorenzo,
>
> On 21/09/16 09:42, Lingutla Chandrasekhar wrote:
>> Implement suspend and resume syscore_ops to disable and
>> enable non wake up capable interrupts.
>>
>> When system enters suspend, enable only wakeup capable
>> interrupts. While resuming, enable previously enabled interrupts
>> and show triggered/pending interrupts.
>
> The fundamental problem (which you're not mentioning at all) is that the
> GICv3 architecture doesn't mention wake-up interrupts at all, so this
> has to be entirely handled in firmware. Is that what is happening?
>
>>
>> Signed-off-by: Lingutla Chandrasekhar <clingutla@codeaurora.org>
>>
>> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
>> index ede5672..511a5a1 100644
>> --- a/drivers/irqchip/irq-gic-v3.c
>> +++ b/drivers/irqchip/irq-gic-v3.c
>> @@ -33,6 +33,7 @@
>> #include <linux/irqchip/arm-gic-common.h>
>> #include <linux/irqchip/arm-gic-v3.h>
>> #include <linux/irqchip/irq-partition-percpu.h>
>> +#include <linux/syscore_ops.h>
>>
>> #include <asm/cputype.h>
>> #include <asm/exception.h>
>> @@ -57,6 +58,10 @@ struct gic_chip_data {
>> u32 nr_redist_regions;
>> unsigned int irq_nr;
>> struct partition_desc *ppi_descs[16];
>> +#ifdef CONFIG_PM
>> + unsigned int wakeup_irqs[32];
>> + unsigned int enabled_irqs[32];
>
> Do not use ambiguous types for something that comes from the HW. Where
> does this '32' comes from?
Expecting wakeup capable irq can be any of maximum 1024 interrupt lines, so
used array of size 32 (32 * 32 bits).
>
>> +#endif
>> };
>>
>> static struct gic_chip_data gic_data __read_mostly;
>> @@ -330,6 +335,81 @@ static int gic_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu)
>> return 0;
>> }
>>
>> +#ifdef CONFIG_PM
>> +static int gic_suspend(void)
>> +{
>> + unsigned int i;
>> + void __iomem *base = gic_data.dist_base;
>> +
>> + for (i = 0; i * 32 < gic->irq_nr; i++) {
>> + gic->enabled_irqs[i]
>> + = readl_relaxed(base + GICD_ISENABLER + i * 4);
>
> Do you realize that GICD_ISENABLER0 is always zero? What do you do for
> PPIs? Please keep the assignment on a single line.
Agreed, will update.
>
>> + /* disable all of them */
>> + writel_relaxed(0xffffffff, base + GICD_ICENABLER + i * 4);
>> + /* enable the wakeup set */
>> + writel_relaxed(gic->wakeup_irqs[i],
>> + base + GICD_ISENABLER + i * 4);
>
> On a single line as well.
>
>> + }
>> + return 0;
>> +}
>> +
>> +static void gic_show_pending(void)
>> +{
>> + unsigned int i;
>> + u32 enabled;
>> + u32 pending[32];
>> + void __iomem *base = gic_data.dist_base;
>> +
>> + for (i = 0; i * 32 < gic->irq_nr; i++) {
>> + enabled = readl_relaxed(base + GICD_ICENABLER + i * 4);
>> + pending[i] = readl_relaxed(base + GICD_ISPENDR + i * 4);
>> + pending[i] &= enabled;
>> + }
>> +
>> + for_each_set_bit(i, (unsigned long *)pending, gic->irq_nr) {
>> + unsigned int irq = irq_find_mapping(gic->domain, i);
>> + struct irq_desc *desc = irq_to_desc(irq);
>> + const char *name = "null";
>> +
>> + if (desc == NULL)
>> + name = "stray irq";
>> + else if (desc->action && desc->action->name)
>> + name = desc->action->name;
>> +
>> + pr_debug("Pending IRQ: %d [%s]\n", __func__, irq, name);
>> + }
>> +}
>
> Please drop this function from this patch, it doesn't serve any purpose
> other than your own debugging.
>
I think, this function is useful for debugging to know wakeup reason.
Can we move this function under PM_DEBUG flag or debugfs entry ?
>> +
>> +static void gic_resume(void)
>> +{
>> + unsigned int i;
>> + void __iomem *base = gic_data.dist_base;
>> +
>> + gic_show_pending();
>> +
>> + for (i = 0; i * 32 < gic->irq_nr; i++) {
>> + /* disable all of them */
>> + writel_relaxed(0xffffffff, base + GICD_ICENABLER + i * 4);
>> + /* enable the enabled set */
>> + writel_relaxed(gic->enabled_irqs[i],
>> + base + GICD_ISENABLER + i * 4);
>
> Same remarks as the suspend side.
>
>> + }
>> +}
>> +
>> +static struct syscore_ops gic_syscore_ops = {
>> + .suspend = gic_suspend,
>> + .resume = gic_resume,
>> +};
>> +
>> +static int __init gic_init_sys(void)
>> +{
>> + register_syscore_ops(&gic_syscore_ops);
>> + return 0;
>> +}
>> +device_initcall(gic_init_sys);
>> +
>> +#endif
>> +
>> static u64 gic_mpidr_to_affinity(unsigned long mpidr)
>> {
>> u64 aff;
>> @@ -666,6 +746,32 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
>> #define gic_smp_init() do { } while(0)
>> #endif
>>
>> +#ifdef CONFIG_PM
>> +int gic_set_wake(struct irq_data *d, unsigned int on)
>> +{
>> + int ret = -ENXIO;
>> + unsigned int reg_offset, bit_offset;
>> + unsigned int gicirq = gic_irq(d);
>> + struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
>> +
>> + /* per-cpu interrupts cannot be wakeup interrupts */
>> + WARN_ON(gicirq < 32);
>
> How did you decide that? There is no such specification anywhere.
>
I am basically looking at system suspend, where cores would be in power collapse.
>> +
>> + reg_offset = gicirq / 32;
>> + bit_offset = gicirq % 32;
>> +
>> + if (on)
>> + gic_data->wakeup_irqs[reg_offset] |= 1 << bit_offset;
>> + else
>> + gic_data->wakeup_irqs[reg_offset] &= ~(1 << bit_offset);
>> +
>> + return ret;
>> +}
>> +
>> +#else
>> +#define gic_set_wake NULL
>> +#endif
>> +
>> #ifdef CONFIG_CPU_PM
>> /* Check whether it's single security state view */
>> static bool gic_dist_security_disabled(void)
>> @@ -707,6 +813,7 @@ static struct irq_chip gic_chip = {
>> .irq_eoi = gic_eoi_irq,
>> .irq_set_type = gic_set_type,
>> .irq_set_affinity = gic_set_affinity,
>> + .irq_set_wake = gic_set_wake,
>> .irq_get_irqchip_state = gic_irq_get_irqchip_state,
>> .irq_set_irqchip_state = gic_irq_set_irqchip_state,
>> .flags = IRQCHIP_SET_TYPE_MASKED,
>> @@ -723,6 +830,7 @@ static struct irq_chip gic_eoimode1_chip = {
>> .irq_set_irqchip_state = gic_irq_set_irqchip_state,
>> .irq_set_vcpu_affinity = gic_irq_set_vcpu_affinity,
>> .flags = IRQCHIP_SET_TYPE_MASKED,
>> + .irq_set_wake = gic_set_wake,
>
> Keep the fields in the same order.
>
>> };
>>
>> #define GIC_ID_NR (1U << gic_data.rdists.id_bits)
>>
>
> But here's my fundamental objection: None of that should be required and
> setting (IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND) as part of the
> irqchip flags should be enough.
>
> Can you explain why this doesn't work for you?
>
These flags work for me, I will remove suspend/resume functions, but i think
it is very useful to know the wakeup source for debugging purposes.
Please let me know, if you have any better way to achieve this.
> Thanks,
>
> M.
>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH] ARM: dts: lpc32xx: add pwm-cells to base dts file
From: Sylvain Lemieux (gmail) @ 2016-09-27 12:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <aaf0fc9c-ea2f-0081-cf58-b5024e450030@mleia.com>
Hi Vladimir,
On Tue, 2016-09-27 at 01:07 +0300, Vladimir Zapolskiy wrote:
> Hi Sylvain,
>
> On 26.09.2016 21:47, Sylvain Lemieux wrote:
> > From: Sylvain Lemieux <slemieux@tycoint.com>
> >
> > There is no need to define the "pwm-cells" in the board
> > specific dts file; move the entry to the base dts file.
> >
> > Signed-off-by: Sylvain Lemieux <slemieux@tycoint.com>
> > ---
> > Note:
> > * This patch should be apply after
> > "ARM: dts: lpc32xx: set default parent clock for pwm1 & pwm2"
> > http://www.spinics.net/lists/arm-kernel/msg530277.html
> > - There is no dependency between the patches.
> >
> > arch/arm/boot/dts/lpc32xx.dtsi | 2 ++
> > 1 file changed, 2 insertions(+)
> >
[...]
>
> that's something I have done locally and in a different manner, but I haven't
> published it yet, please find below a draft.
>
> First of all from multiple places in the User's Manual you can find that there
> are "two single output PWM blocks" or "the LPC32x0 provides two 8-bit PWMs" etc.
>
> In this case it does not make sense to set PWM cells to 2 (there is only one
> channel), and 1 cell for frequency is good enough, and that's the proposed
> change to support it:
>
> diff --git a/drivers/pwm/pwm-lpc32xx.c b/drivers/pwm/pwm-lpc32xx.c
> index a9b3cff..447ae44 100644
> --- a/drivers/pwm/pwm-lpc32xx.c
> +++ b/drivers/pwm/pwm-lpc32xx.c
> @@ -99,6 +99,22 @@ static const struct pwm_ops lpc32xx_pwm_ops = {
> .owner = THIS_MODULE,
> };
>
> +static struct pwm_device *lpc32xx_pwm_of_xlate(struct pwm_chip *pc,
> + const struct of_phandle_args *args)
> +{
> + struct pwm_device *pwm;
> +
> + pwm = pwm_request_from_chip(pc, 0, NULL);
> + if (IS_ERR(pwm))
> + return pwm;
> +
> + pwm->args.period = args->args[0];
> +
> + return pwm;
> +}
> +
> static int lpc32xx_pwm_probe(struct platform_device *pdev)
> {
> struct lpc32xx_pwm_chip *lpc32xx;
> @@ -123,6 +139,8 @@ static int lpc32xx_pwm_probe(struct platform_device *pdev)
> lpc32xx->chip.ops = &lpc32xx_pwm_ops;
> lpc32xx->chip.npwm = 1;
> lpc32xx->chip.base = -1;
> + lpc32xx->chip.of_xlate = lpc32xx_pwm_of_xlate;
> + lpc32xx->chip.of_pwm_n_cells = 1;
>
> ret = pwmchip_add(&lpc32xx->chip);
> if (ret < 0) {
>
>
> What is your opinion about this proposal?
>
I agree with you, this clean-up make sense; the PWM cell should be 1.
> If this change is applied, then lpc32xx.dtsi should contain #pwm-cells = <1>.
>
Can you submit your change on the mailing list?
I will send a version 2 of this patch after.
If it is helping, I can take care of submitting a patch
to update the documentation (lpc32xx-pwm.txt).
Did you have a change to look at the 2 others PWM related change:
* clk: lpc32xx: fix pwm clock divider computation
http://www.spinics.net/lists/arm-kernel/msg534048.html
* ARM: dts: lpc32xx: set pwm1 & pwm2 default clock rate
http://www.spinics.net/lists/arm-kernel/msg534051.html
> --
> With best wishes,
> Vladimir
Sylvain
^ permalink raw reply
* [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
From: Srinivas Ramana @ 2016-09-27 12:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D812A7.6040905@codeaurora.org>
Hi Robin,
On 09/13/2016 08:22 PM, Srinivas Ramana wrote:
> On 09/12/2016 11:21 PM, Robin Murphy wrote:
>> On 12/09/16 07:57, Srinivas Ramana wrote:
>>> If the bootloader uses the long descriptor format and jumps to
>>> kernel decompressor code, TTBCR may not be in a right state.
>>> Before enabling the MMU, it is required to clear the TTBCR.PD0
>>> field to use TTBR0 for translation table walks.
>>>
>>> The 'commit dbece45894d3a ("ARM: 7501/1: decompressor:
>>> reset ttbcr for VMSA ARMv7 cores")' does the reset of TTBCR.N, but
>>> doesn't consider all the bits for the size of TTBCR.N.
>>>
>>> Clear TTBCR.PD0 field and reset all the three bits of TTBCR.N to
>>> indicate the use of TTBR0 and the correct base address width.
>>>
>>> Signed-off-by: Srinivas Ramana <sramana@codeaurora.org>
>>> ---
>>> arch/arm/boot/compressed/head.S | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/compressed/head.S
>>> b/arch/arm/boot/compressed/head.S
>>> index af11c2f8f3b7..fc6d541549a2 100644
>>> --- a/arch/arm/boot/compressed/head.S
>>> +++ b/arch/arm/boot/compressed/head.S
>>> @@ -779,7 +779,7 @@ __armv7_mmu_cache_on:
>>> orrne r0, r0, #1 @ MMU enabled
>>> movne r1, #0xfffffffd @ domain 0 = client
>>> bic r6, r6, #1 << 31 @ 32-bit translation system
>>
>> Hmm, if TTBCR.EAE _was_ actually set...
>>
>>> - bic r6, r6, #3 << 0 @ use only ttbr0
>>> + bic r6, r6, #(7 << 0) | (1 << 4) @ use only ttbr0
>>> mcrne p15, 0, r3, c2, c0, 0 @ load page table pointer
>>> mcrne p15, 0, r1, c3, c0, 0 @ load domain access control
>>> mcrne p15, 0, r6, c2, c0, 2 @ load ttb control
>>
>> ...then strictly the TLBIALL needs to happen after the ISB following
>> this update. Otherwise per B3.10.2 of DDI406C.c I think we might be into
>> unpredictable territory - i.e. if the TLB happens to treat long- and
>> short-descriptor entries differently then the TLBI beforehand (with EAE
>> set) may be at liberty to only discard long-descriptor entries and leave
>> bogus short-descriptor entries sitting around.
> Yes, it seems this has to be taken care of, along with resetting
> TTBCR.PD0 and TTBCR.N. Do you say that this needs to be done in the same
> patch or a different one?
>>
>> In other words, something like (completely untested):
>>
>> ---8<---
>> diff --git a/arch/arm/boot/compressed/head.S
>> b/arch/arm/boot/compressed/head.S
>> index af11c2f8f3b7..536b7781024a 100644
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -764,7 +764,6 @@ __armv7_mmu_cache_on:
>> mov r0, #0
>> mcr p15, 0, r0, c7, c10, 4 @ drain write buffer
>> tst r11, #0xf @ VMSA
>> - mcrne p15, 0, r0, c8, c7, 0 @ flush I,D TLBs
>
> Shouldn't this be still there for the same reason you explained above? I
> mean to discard the long descriptor entries when EAE was 1 (before we
> reset it).
>> #endif
>> mrc p15, 0, r0, c1, c0, 0 @ read control reg
>> bic r0, r0, #1 << 28 @ clear SCTLR.TRE
>> @@ -783,8 +782,11 @@ __armv7_mmu_cache_on:
>> mcrne p15, 0, r3, c2, c0, 0 @ load page table
>> pointer
>> mcrne p15, 0, r1, c3, c0, 0 @ load domain access
>> control
>> mcrne p15, 0, r6, c2, c0, 2 @ load ttb control
>> -#endif
>> mcr p15, 0, r0, c7, c5, 4 @ ISB
>> + mcrne p15, 0, r0, c8, c7, 0 @ flush I,D TLBs
>> +#else
>> + mcr p15, 0, r0, c7, c5, 4 @ ISB
>> +#endif
>> mcr p15, 0, r0, c1, c0, 0 @ load control register
>> mrc p15, 0, r0, c1, c0, 0 @ and read it back
>> ---8<---
>>
>> Robin.
>>
> i have tested this change (flush I, D, TLBs after TTB control is
> written) and don't see any issue. But on my setup decompression is
> successful even without this (probably not hitting the case in discussion).
>
>
> Thanks,
> -- Srinivas R
>
Would like your feedback on the above. Can we get the TTBCR fix merged
first?(will send final patch with Russell Kings comments fixed)
For testing the TLB flush change we may have to check if we can create a
failure case.
Thanks,
-- Srinivas R
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative
Project.
^ permalink raw reply
* [PATCH] ARM: at91: pm: remove useless extern definition
From: Nicolas Ferre @ 2016-09-27 11:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160927103715.26785-1-alexandre.belloni@free-electrons.com>
Le 27/09/2016 ? 12:37, Alexandre Belloni a ?crit :
> at91_ramc_base is local to pm.c, remove its definition in pm.h
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> arch/arm/mach-at91/pm.c | 2 +-
> arch/arm/mach-at91/pm.h | 2 --
> 2 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index b4332b727e9c..3d89b7905bd9 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -55,7 +55,7 @@ static struct {
> int memctrl;
> } at91_pm_data;
>
> -void __iomem *at91_ramc_base[2];
> +static void __iomem *at91_ramc_base[2];
>
> static int at91_pm_valid_state(suspend_state_t state)
> {
> diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
> index 3fcf8810f14e..bf980c6ef294 100644
> --- a/arch/arm/mach-at91/pm.h
> +++ b/arch/arm/mach-at91/pm.h
> @@ -18,8 +18,6 @@
> #include <soc/at91/at91sam9_sdramc.h>
>
> #ifndef __ASSEMBLY__
> -extern void __iomem *at91_ramc_base[];
> -
> #define at91_ramc_read(id, field) \
> __raw_readl(at91_ramc_base[id] + field)
>
>
--
Nicolas Ferre
^ permalink raw reply
* [PATCH v2 8/8] ARM: bcm2835: Disable i2c2 in the Device Tree
From: Noralf Trønnes @ 2016-09-27 11:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474977426-3272-1-git-send-email-noralf@tronnes.org>
i2c2 is connected to the HDMI connector and is controlled by the
firmware. Disable it to stay out of harms way.
>From the downstream commit:
i2c-bcm2708/BCM270X_DT: Add support for I2C2
The third I2C bus (I2C2) is normally reserved for HDMI use. Careless
use of this bus can break an attached display - use with caution.
It is recommended to disable accesses by VideoCore by setting
hdmi_ignore_edid=1 or hdmi_edid_file=1 in config.txt.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
---
arch/arm/boot/dts/bcm2835-rpi.dtsi | 4 ----
1 file changed, 4 deletions(-)
diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index e9b47b2..8bffbee 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -59,10 +59,6 @@
clock-frequency = <100000>;
};
-&i2c2 {
- status = "okay";
-};
-
&sdhci {
status = "okay";
bus-width = <4>;
--
2.8.2
^ permalink raw reply related
* [PATCH v2 7/8] i2c: bcm2835: Add support for dynamic clock
From: Noralf Trønnes @ 2016-09-27 11:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474977426-3272-1-git-send-email-noralf@tronnes.org>
Support a dynamic clock by reading the frequency and setting the
divisor in the transfer function instead of during probe.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
---
drivers/i2c/busses/i2c-bcm2835.c | 51 +++++++++++++++++++++++++---------------
1 file changed, 32 insertions(+), 19 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index 79dd15f..5aed514 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -58,6 +58,7 @@ struct bcm2835_i2c_dev {
void __iomem *regs;
struct clk *clk;
int irq;
+ u32 bus_clk_rate;
struct i2c_adapter adapter;
struct completion completion;
struct i2c_msg *curr_msg;
@@ -78,6 +79,30 @@ static inline u32 bcm2835_i2c_readl(struct bcm2835_i2c_dev *i2c_dev, u32 reg)
return readl(i2c_dev->regs + reg);
}
+static int bcm2835_i2c_set_divider(struct bcm2835_i2c_dev *i2c_dev)
+{
+ u32 divider;
+
+ divider = DIV_ROUND_UP(clk_get_rate(i2c_dev->clk),
+ i2c_dev->bus_clk_rate);
+ /*
+ * Per the datasheet, the register is always interpreted as an even
+ * number, by rounding down. In other words, the LSB is ignored. So,
+ * if the LSB is set, increment the divider to avoid any issue.
+ */
+ if (divider & 1)
+ divider++;
+ if ((divider < BCM2835_I2C_CDIV_MIN) ||
+ (divider > BCM2835_I2C_CDIV_MAX)) {
+ dev_err_ratelimited(i2c_dev->dev, "Invalid clock-frequency\n");
+ return -EINVAL;
+ }
+
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_DIV, divider);
+
+ return 0;
+}
+
static void bcm2835_fill_txfifo(struct bcm2835_i2c_dev *i2c_dev)
{
u32 val;
@@ -215,7 +240,7 @@ static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
{
struct bcm2835_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
unsigned long time_left;
- int i;
+ int i, ret;
for (i = 0; i < (num - 1); i++)
if (msgs[i].flags & I2C_M_RD) {
@@ -224,6 +249,10 @@ static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
return -EOPNOTSUPP;
}
+ ret = bcm2835_i2c_set_divider(i2c_dev);
+ if (ret)
+ return ret;
+
i2c_dev->curr_msg = msgs;
i2c_dev->num_msgs = num;
reinit_completion(&i2c_dev->completion);
@@ -274,7 +303,6 @@ static int bcm2835_i2c_probe(struct platform_device *pdev)
{
struct bcm2835_i2c_dev *i2c_dev;
struct resource *mem, *irq;
- u32 bus_clk_rate, divider;
int ret;
struct i2c_adapter *adap;
@@ -298,27 +326,12 @@ static int bcm2835_i2c_probe(struct platform_device *pdev)
}
ret = of_property_read_u32(pdev->dev.of_node, "clock-frequency",
- &bus_clk_rate);
+ &i2c_dev->bus_clk_rate);
if (ret < 0) {
dev_warn(&pdev->dev,
"Could not read clock-frequency property\n");
- bus_clk_rate = 100000;
- }
-
- divider = DIV_ROUND_UP(clk_get_rate(i2c_dev->clk), bus_clk_rate);
- /*
- * Per the datasheet, the register is always interpreted as an even
- * number, by rounding down. In other words, the LSB is ignored. So,
- * if the LSB is set, increment the divider to avoid any issue.
- */
- if (divider & 1)
- divider++;
- if ((divider < BCM2835_I2C_CDIV_MIN) ||
- (divider > BCM2835_I2C_CDIV_MAX)) {
- dev_err(&pdev->dev, "Invalid clock-frequency\n");
- return -ENODEV;
+ i2c_dev->bus_clk_rate = 100000;
}
- bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_DIV, divider);
irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (!irq) {
--
2.8.2
^ permalink raw reply related
* [PATCH v2 6/8] i2c: bcm2835: Support i2c-dev ioctl I2C_TIMEOUT
From: Noralf Trønnes @ 2016-09-27 11:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474977426-3272-1-git-send-email-noralf@tronnes.org>
Use i2c_adapter->timeout for the completion timeout value. The core
default is 1 second.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
---
drivers/i2c/busses/i2c-bcm2835.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index f3a1472..79dd15f 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -53,8 +53,6 @@
#define BCM2835_I2C_CDIV_MIN 0x0002
#define BCM2835_I2C_CDIV_MAX 0xFFFE
-#define BCM2835_I2C_TIMEOUT (msecs_to_jiffies(1000))
-
struct bcm2835_i2c_dev {
struct device *dev;
void __iomem *regs;
@@ -233,7 +231,7 @@ static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
bcm2835_i2c_start_transfer(i2c_dev);
time_left = wait_for_completion_timeout(&i2c_dev->completion,
- BCM2835_I2C_TIMEOUT);
+ adap->timeout);
if (!time_left) {
bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C,
BCM2835_I2C_C_CLEAR);
--
2.8.2
^ permalink raw reply related
* [PATCH v2 5/8] i2c: bcm2835: Add support for Repeated Start Condition
From: Noralf Trønnes @ 2016-09-27 11:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474977426-3272-1-git-send-email-noralf@tronnes.org>
Documentation/i2c/i2c-protocol states that Combined transactions should
separate messages with a Start bit and end the whole transaction with a
Stop bit. This patch adds support for issuing only a Start between
messages instead of a Stop followed by a Start.
This implementation differs from downstream i2c-bcm2708 in 2 respects:
- it uses an interrupt to detect that the transfer is active instead
of using polling. There is no interrupt for Transfer Active, but by
not prefilling the FIFO it's possible to use the TXW interrupt.
- when resetting/disabling the controller between transfers it writes
CLEAR to the control register instead of just zero.
Using just zero gave many errors. This might be the reason why
downstream had to disable this feature and make it available with a
module parameter.
I have run thousands of transfers to a DS1307 (rtc), MMA8451 (accel)
and AT24C32 (eeprom) in parallel without problems.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
---
drivers/i2c/busses/i2c-bcm2835.c | 101 ++++++++++++++++++++++++---------------
1 file changed, 63 insertions(+), 38 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index 4e08add..f3a1472 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -63,6 +63,7 @@ struct bcm2835_i2c_dev {
struct i2c_adapter adapter;
struct completion completion;
struct i2c_msg *curr_msg;
+ int num_msgs;
u32 msg_err;
u8 *msg_buf;
size_t msg_buf_remaining;
@@ -109,6 +110,45 @@ static void bcm2835_drain_rxfifo(struct bcm2835_i2c_dev *i2c_dev)
}
}
+/*
+ * Repeated Start Condition (Sr)
+ * The BCM2835 ARM Peripherals datasheet mentions a way to trigger a Sr when it
+ * talks about reading from a slave with 10 bit address. This is achieved by
+ * issuing a write, poll the I2CS.TA flag and wait for it to be set, and then
+ * issue a read.
+ * A comment in https://github.com/raspberrypi/linux/issues/254 shows how the
+ * firmware actually does it using polling and says that it's a workaround for
+ * a problem in the state machine.
+ * It turns out that it is possible to use the TXW interrupt to know when the
+ * transfer is active, provided the FIFO has not been prefilled.
+ */
+
+static void bcm2835_i2c_start_transfer(struct bcm2835_i2c_dev *i2c_dev)
+{
+ u32 c = BCM2835_I2C_C_ST | BCM2835_I2C_C_I2CEN;
+ struct i2c_msg *msg = i2c_dev->curr_msg;
+ bool last_msg = (i2c_dev->num_msgs == 1);
+
+ if (!i2c_dev->num_msgs)
+ return;
+
+ i2c_dev->num_msgs--;
+ i2c_dev->msg_buf = msg->buf;
+ i2c_dev->msg_buf_remaining = msg->len;
+
+ if (msg->flags & I2C_M_RD)
+ c |= BCM2835_I2C_C_READ | BCM2835_I2C_C_INTR;
+ else
+ c |= BCM2835_I2C_C_INTT;
+
+ if (last_msg)
+ c |= BCM2835_I2C_C_INTD;
+
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_A, msg->addr);
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_DLEN, msg->len);
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, c);
+}
+
static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
{
struct bcm2835_i2c_dev *i2c_dev = data;
@@ -142,6 +182,12 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
}
bcm2835_fill_txfifo(i2c_dev);
+
+ if (i2c_dev->num_msgs && !i2c_dev->msg_buf_remaining) {
+ i2c_dev->curr_msg++;
+ bcm2835_i2c_start_transfer(i2c_dev);
+ }
+
return IRQ_HANDLED;
}
@@ -166,30 +212,25 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
return IRQ_HANDLED;
}
-static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
- struct i2c_msg *msg)
+static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
+ int num)
{
- u32 c;
+ struct bcm2835_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
unsigned long time_left;
+ int i;
- i2c_dev->curr_msg = msg;
- i2c_dev->msg_buf = msg->buf;
- i2c_dev->msg_buf_remaining = msg->len;
- reinit_completion(&i2c_dev->completion);
-
- bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, BCM2835_I2C_C_CLEAR);
+ for (i = 0; i < (num - 1); i++)
+ if (msgs[i].flags & I2C_M_RD) {
+ dev_warn_once(i2c_dev->dev,
+ "only one read message supported, has to be last\n");
+ return -EOPNOTSUPP;
+ }
- if (msg->flags & I2C_M_RD) {
- c = BCM2835_I2C_C_READ | BCM2835_I2C_C_INTR;
- } else {
- c = BCM2835_I2C_C_INTT;
- bcm2835_fill_txfifo(i2c_dev);
- }
- c |= BCM2835_I2C_C_ST | BCM2835_I2C_C_INTD | BCM2835_I2C_C_I2CEN;
+ i2c_dev->curr_msg = msgs;
+ i2c_dev->num_msgs = num;
+ reinit_completion(&i2c_dev->completion);
- bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_A, msg->addr);
- bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_DLEN, msg->len);
- bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, c);
+ bcm2835_i2c_start_transfer(i2c_dev);
time_left = wait_for_completion_timeout(&i2c_dev->completion,
BCM2835_I2C_TIMEOUT);
@@ -200,32 +241,16 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
return -ETIMEDOUT;
}
- if (likely(!i2c_dev->msg_err))
- return 0;
+ if (!i2c_dev->msg_err)
+ return num;
dev_err_ratelimited(i2c_dev->dev, "i2c transfer failed: %x\n",
i2c_dev->msg_err);
if (i2c_dev->msg_err & BCM2835_I2C_S_ERR)
return -EREMOTEIO;
- else
- return -EIO;
-}
-
-static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
- int num)
-{
- struct bcm2835_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
- int i;
- int ret = 0;
-
- for (i = 0; i < num; i++) {
- ret = bcm2835_i2c_xfer_msg(i2c_dev, &msgs[i]);
- if (ret)
- break;
- }
- return ret ?: i;
+ return -EIO;
}
static u32 bcm2835_i2c_func(struct i2c_adapter *adap)
--
2.8.2
^ permalink raw reply related
* [PATCH v2 4/8] i2c: bcm2835: Can't support I2C_M_IGNORE_NAK
From: Noralf Trønnes @ 2016-09-27 11:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474977426-3272-1-git-send-email-noralf@tronnes.org>
The controller can't support this flag, so remove it.
Documentation/i2c/i2c-protocol states that all of the message is sent:
I2C_M_IGNORE_NAK:
Normally message is interrupted immediately if there is [NA] from the
client. Setting this flag treats any [NA] as [A], and all of
message is sent.
>From the BCM2835 ARM Peripherals datasheet:
The ERR field is set when the slave fails to acknowledge either
its address or a data byte written to it.
So when the controller doesn't receive an ack, it sets ERR and raises
an interrupt. In other words, the whole message is not sent.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
---
drivers/i2c/busses/i2c-bcm2835.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index 370a322..4e08add 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -203,10 +203,6 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
if (likely(!i2c_dev->msg_err))
return 0;
- if ((i2c_dev->msg_err & BCM2835_I2C_S_ERR) &&
- (msg->flags & I2C_M_IGNORE_NAK))
- return 0;
-
dev_err_ratelimited(i2c_dev->dev, "i2c transfer failed: %x\n",
i2c_dev->msg_err);
--
2.8.2
^ permalink raw reply related
* [PATCH v2 3/8] i2c: bcm2835: Use ratelimited logging on transfer errors
From: Noralf Trønnes @ 2016-09-27 11:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474977426-3272-1-git-send-email-noralf@tronnes.org>
Writing to an AT24C32 generates on average 2x i2c transfer errors per
32-byte page write. Which amounts to a lot for a 4k write. This is due
to the fact that the chip doesn't respond during it's internal write
cycle when the at24 driver tries and retries the next write.
Reduce this flooding of the log by using dev_err_ratelimited().
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
---
drivers/i2c/busses/i2c-bcm2835.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index df036ed..370a322 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -207,7 +207,8 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
(msg->flags & I2C_M_IGNORE_NAK))
return 0;
- dev_err(i2c_dev->dev, "i2c transfer failed: %x\n", i2c_dev->msg_err);
+ dev_err_ratelimited(i2c_dev->dev, "i2c transfer failed: %x\n",
+ i2c_dev->msg_err);
if (i2c_dev->msg_err & BCM2835_I2C_S_ERR)
return -EREMOTEIO;
--
2.8.2
^ permalink raw reply related
* [PATCH v2 2/8] i2c: bcm2835: Protect against unexpected TXW/RXR interrupts
From: Noralf Trønnes @ 2016-09-27 11:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474977426-3272-1-git-send-email-noralf@tronnes.org>
If an unexpected TXW or RXR interrupt occurs (msg_buf_remaining == 0),
the driver has no way to fill/drain the FIFO to stop the interrupts.
In this case the controller has to be disabled and the transfer
completed to avoid hang.
(CLKT | ERR) and DONE interrupts are completed in their own paths, and
the controller is disabled in the transfer function after completion.
Unite the code paths and do disabling inside the interrupt routine.
Clear interrupt status bits in the united completion path instead of
trying to do it on every interrupt which isn't necessary.
Only CLKT, ERR and DONE can be cleared that way.
Add the status value to the error value in case of TXW/RXR errors to
distinguish them from the other S_LEN error.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
---
drivers/i2c/busses/i2c-bcm2835.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index f283b71..df036ed 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -50,8 +50,6 @@
#define BCM2835_I2C_S_CLKT BIT(9)
#define BCM2835_I2C_S_LEN BIT(10) /* Fake bit for SW error reporting */
-#define BCM2835_I2C_BITMSK_S 0x03FF
-
#define BCM2835_I2C_CDIV_MIN 0x0002
#define BCM2835_I2C_CDIV_MAX 0xFFFE
@@ -117,14 +115,11 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
u32 val, err;
val = bcm2835_i2c_readl(i2c_dev, BCM2835_I2C_S);
- val &= BCM2835_I2C_BITMSK_S;
- bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_S, val);
err = val & (BCM2835_I2C_S_CLKT | BCM2835_I2C_S_ERR);
if (err) {
i2c_dev->msg_err = err;
- complete(&i2c_dev->completion);
- return IRQ_HANDLED;
+ goto complete;
}
if (val & BCM2835_I2C_S_DONE) {
@@ -137,21 +132,38 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
i2c_dev->msg_err = BCM2835_I2C_S_LEN;
else
i2c_dev->msg_err = 0;
- complete(&i2c_dev->completion);
- return IRQ_HANDLED;
+ goto complete;
}
if (val & BCM2835_I2C_S_TXW) {
+ if (!i2c_dev->msg_buf_remaining) {
+ i2c_dev->msg_err = val | BCM2835_I2C_S_LEN;
+ goto complete;
+ }
+
bcm2835_fill_txfifo(i2c_dev);
return IRQ_HANDLED;
}
if (val & BCM2835_I2C_S_RXR) {
+ if (!i2c_dev->msg_buf_remaining) {
+ i2c_dev->msg_err = val | BCM2835_I2C_S_LEN;
+ goto complete;
+ }
+
bcm2835_drain_rxfifo(i2c_dev);
return IRQ_HANDLED;
}
return IRQ_NONE;
+
+complete:
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, BCM2835_I2C_C_CLEAR);
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_S, BCM2835_I2C_S_CLKT |
+ BCM2835_I2C_S_ERR | BCM2835_I2C_S_DONE);
+ complete(&i2c_dev->completion);
+
+ return IRQ_HANDLED;
}
static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
@@ -181,8 +193,9 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
time_left = wait_for_completion_timeout(&i2c_dev->completion,
BCM2835_I2C_TIMEOUT);
- bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, BCM2835_I2C_C_CLEAR);
if (!time_left) {
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C,
+ BCM2835_I2C_C_CLEAR);
dev_err(i2c_dev->dev, "i2c transfer timed out\n");
return -ETIMEDOUT;
}
--
2.8.2
^ permalink raw reply related
* [PATCH v2 1/8] i2c: bcm2835: Fix hang for writing messages larger than 16 bytes
From: Noralf Trønnes @ 2016-09-27 11:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474977426-3272-1-git-send-email-noralf@tronnes.org>
Writing messages larger than the FIFO size results in a hang, rendering
the machine unusable. This is because the RXD status flag is set on the
first interrupt which results in bcm2835_drain_rxfifo() stealing bytes
from the buffer. The controller continues to trigger interrupts waiting
for the missing bytes, but bcm2835_fill_txfifo() has none to give.
In this situation wait_for_completion_timeout() apparently is unable to
stop the madness.
The BCM2835 ARM Peripherals datasheet has this to say about the flags:
TXD: is set when the FIFO has space for at least one byte of data.
RXD: is set when the FIFO contains at least one byte of data.
TXW: is set during a write transfer and the FIFO is less than full.
RXR: is set during a read transfer and the FIFO is or more full.
Implementing the logic from the downstream i2c-bcm2708 driver solved
the hang problem.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/i2c/busses/i2c-bcm2835.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index d4f3239..f283b71 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -64,6 +64,7 @@ struct bcm2835_i2c_dev {
int irq;
struct i2c_adapter adapter;
struct completion completion;
+ struct i2c_msg *curr_msg;
u32 msg_err;
u8 *msg_buf;
size_t msg_buf_remaining;
@@ -126,14 +127,13 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
return IRQ_HANDLED;
}
- if (val & BCM2835_I2C_S_RXD) {
- bcm2835_drain_rxfifo(i2c_dev);
- if (!(val & BCM2835_I2C_S_DONE))
- return IRQ_HANDLED;
- }
-
if (val & BCM2835_I2C_S_DONE) {
- if (i2c_dev->msg_buf_remaining)
+ if (i2c_dev->curr_msg->flags & I2C_M_RD) {
+ bcm2835_drain_rxfifo(i2c_dev);
+ val = bcm2835_i2c_readl(i2c_dev, BCM2835_I2C_S);
+ }
+
+ if ((val & BCM2835_I2C_S_RXD) || i2c_dev->msg_buf_remaining)
i2c_dev->msg_err = BCM2835_I2C_S_LEN;
else
i2c_dev->msg_err = 0;
@@ -141,11 +141,16 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
return IRQ_HANDLED;
}
- if (val & BCM2835_I2C_S_TXD) {
+ if (val & BCM2835_I2C_S_TXW) {
bcm2835_fill_txfifo(i2c_dev);
return IRQ_HANDLED;
}
+ if (val & BCM2835_I2C_S_RXR) {
+ bcm2835_drain_rxfifo(i2c_dev);
+ return IRQ_HANDLED;
+ }
+
return IRQ_NONE;
}
@@ -155,6 +160,7 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
u32 c;
unsigned long time_left;
+ i2c_dev->curr_msg = msg;
i2c_dev->msg_buf = msg->buf;
i2c_dev->msg_buf_remaining = msg->len;
reinit_completion(&i2c_dev->completion);
--
2.8.2
^ permalink raw reply related
* [PATCH v2 0/8] i2c: bcm2835: Bring in changes from downstream
From: Noralf Trønnes @ 2016-09-27 11:56 UTC (permalink / raw)
To: linux-arm-kernel
This patchset tries to bring in the lessons learned in the downstream
driver i2c-bcm2708. The downstream clock stretcing timeout patch has
been left out since clock stretching is broken/unreliable on this
controller, so no point in setting it.
This second version of the patchset has been expanded to try and
encompass all the differences between mainline and downstream.
Hopefully downstream can now start using this driver.
Thanks to Martin for jogging my memory with his comments which led to
the discovery that interrupt could be used instead of polling for the
Transfer Active state.
Noralf.
Noralf Tr?nnes (8):
i2c: bcm2835: Fix hang for writing messages larger than 16 bytes
i2c: bcm2835: Protect against unexpected TXW/RXR interrupts
i2c: bcm2835: Use ratelimited logging on transfer errors
i2c: bcm2835: Can't support I2C_M_IGNORE_NAK
i2c: bcm2835: Add support for Repeated Start Condition
i2c: bcm2835: Support i2c-dev ioctl I2C_TIMEOUT
i2c: bcm2835: Add support for dynamic clock
ARM: bcm2835: Disable i2c2 in the Device Tree
arch/arm/boot/dts/bcm2835-rpi.dtsi | 4 -
drivers/i2c/busses/i2c-bcm2835.c | 210 +++++++++++++++++++++++--------------
2 files changed, 131 insertions(+), 83 deletions(-)
--
2.8.2
^ permalink raw reply
* [RFC PATCH 08/11] pci: controller: dra7xx: Add EP mode support
From: Kishon Vijay Abraham I @ 2016-09-27 11:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160923145259.GA28285@rob-hp-laptop>
Hi,
On Friday 23 September 2016 08:22 PM, Rob Herring wrote:
> On Wed, Sep 14, 2016 at 10:42:04AM +0530, Kishon Vijay Abraham I wrote:
>> The PCIe controller integrated in dra7xx SoCs is capable of operating
>> in endpoint mode. Add support for dra7xx SoCs to operate in endpoint
>> mode.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>> Documentation/devicetree/bindings/pci/ti-pci.txt | 30 ++-
>> drivers/pci/controller/Kconfig | 21 +++
>> drivers/pci/controller/pci-dra7xx.c | 211 +++++++++++++++++++---
>> 3 files changed, 225 insertions(+), 37 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
>> index 60e2516..b0e76f6 100644
>> --- a/Documentation/devicetree/bindings/pci/ti-pci.txt
>> +++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
>> @@ -1,17 +1,22 @@
>> TI PCI Controllers
>>
>> PCIe Designware Controller
>> - - compatible: Should be "ti,dra7-pcie""
>> - - reg : Two register ranges as listed in the reg-names property
>> - - reg-names : The first entry must be "ti-conf" for the TI specific registers
>> - The second entry must be "rc-dbics" for the designware pcie
>> - registers
>> - The third entry must be "config" for the PCIe configuration space
>> + - compatible: Should be "ti,dra7-pcie" for RC
>> + Should be "ti,dra7-pcie-ep" for EP
>> - phys : list of PHY specifiers (used by generic PHY framework)
>> - phy-names : must be "pcie-phy0", "pcie-phy1", "pcie-phyN".. based on the
>> number of PHYs as specified in *phys* property.
>> - ti,hwmods : Name of the hwmod associated to the pcie, "pcie<X>",
>> where <X> is the instance number of the pcie from the HW spec.
>> + - num-lanes as specified in ../designware-pcie.txt
>> +
>> +HOST MODE
>> +=========
>> + - reg : Two register ranges as listed in the reg-names property
>> + - reg-names : The first entry must be "ti-conf" for the TI specific registers
>> + The second entry must be "rc-dbics" for the designware pcie
>> + registers
>> + The third entry must be "config" for the PCIe configuration space
>> - interrupts : Two interrupt entries must be specified. The first one is for
>> main interrupt line and the second for MSI interrupt line.
>> - #address-cells,
>> @@ -19,13 +24,24 @@ PCIe Designware Controller
>> #interrupt-cells,
>> device_type,
>> ranges,
>> - num-lanes,
>> interrupt-map-mask,
>> interrupt-map : as specified in ../designware-pcie.txt
>>
>> Optional Property:
>> - gpios : Should be added if a gpio line is required to drive PERST# line
>
> Don't you need gpios as the input side of GPIO outputs in RC mode? Or
> for EP mode they are all handled by h/w?
I couldn't find any mention of the gpios being used in EP mode. I'll check this
again.
Thanks
Kishon
^ permalink raw reply
* [RFC PATCH 07/11] pci: controller: designware: Add EP mode support
From: Kishon Vijay Abraham I @ 2016-09-27 11:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160923144141.GA24842@rob-hp-laptop>
Hi,
On Friday 23 September 2016 08:11 PM, Rob Herring wrote:
> On Wed, Sep 14, 2016 at 10:42:03AM +0530, Kishon Vijay Abraham I wrote:
>> Add endpoint mode support to designware driver. This uses the
>> EP Core layer introduced recently to add endpoint mode support.
>> *Any* function driver can now use this designware device
>> to achieve the EP functionality.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>> .../devicetree/bindings/pci/designware-pcie.txt | 26 ++-
>> drivers/pci/controller/Kconfig | 5 +
>> drivers/pci/controller/Makefile | 1 +
>> drivers/pci/controller/pcie-designware-ep.c | 228 ++++++++++++++++++++
>> drivers/pci/controller/pcie-designware.c | 30 +++
>> drivers/pci/controller/pcie-designware.h | 45 ++++
>> 6 files changed, 324 insertions(+), 11 deletions(-)
>> create mode 100644 drivers/pci/controller/pcie-designware-ep.c
>>
>> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> index 6c5322c..bb0b789 100644
>> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> @@ -6,23 +6,27 @@ Required properties:
>> - reg-names: Must be "config" for the PCIe configuration space.
>> (The old way of getting the configuration address space from "ranges"
>> is deprecated and should be avoided.)
>> -- #address-cells: set to <3>
>> -- #size-cells: set to <2>
>> -- device_type: set to "pci"
>> -- ranges: ranges for the PCI memory and I/O regions
>> -- #interrupt-cells: set to <1>
>> -- interrupt-map-mask and interrupt-map: standard PCI properties
>> - to define the mapping of the PCIe interface to interrupt
>> +- #address-cells (only for host mode): set to <3>
>> +- #size-cells (only for host mode): set to <2>
>> +- device_type (only for host mode): set to "pci"
>> +- ranges (only for host mode): ranges for the PCI memory and I/O regions
>> +- num-ib-windows (only for EP mode): number of inbound address translation
>> + windows
>> +- num-ob-windows (only for EP mode): number of outbound address translation
>> + windows
>> +- #interrupt-cells (only for host mode): set to <1>
>> +- interrupt-map-mask and interrupt-map (only for host mode): standard PCI
>> + properties to define the mapping of the PCIe interface to interrupt
>
> It may be clearer to just document EP mode in a separate section even if
> there's some duplication of properties. Other standard PCI binding
> properties probably also don't apply.
right, will change this accordingly.
Thanks
Kishon
^ permalink raw reply
* [PATCH 5/5] tty: amba-pl011: Add earlycon support for SBSA UART
From: Greg Kroah-Hartman @ 2016-09-27 10:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474708465-38958-6-git-send-email-wangkefeng.wang@huawei.com>
On Sat, Sep 24, 2016 at 05:14:25PM +0800, Kefeng Wang wrote:
> Declare an OF early console for SBSA UART so that the early console device
> can be specified via the "stdout-path" property in device-tree.
>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
> drivers/tty/serial/amba-pl011.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 7d9b291..3688d3b 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2330,6 +2330,7 @@ static int __init pl011_early_console_setup(struct earlycon_device *device,
> return 0;
> }
> OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup);
> +OF_EARLYCON_DECLARE(pl011, "arm,sbsa-uart", pl011_early_console_setup);
Why do you need another option for the same thing?
confused,
greg k-h
^ permalink raw reply
* [PATCH] ARM: at91: pm: remove useless extern definition
From: Alexandre Belloni @ 2016-09-27 10:37 UTC (permalink / raw)
To: linux-arm-kernel
at91_ramc_base is local to pm.c, remove its definition in pm.h
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
arch/arm/mach-at91/pm.c | 2 +-
arch/arm/mach-at91/pm.h | 2 --
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index b4332b727e9c..3d89b7905bd9 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -55,7 +55,7 @@ static struct {
int memctrl;
} at91_pm_data;
-void __iomem *at91_ramc_base[2];
+static void __iomem *at91_ramc_base[2];
static int at91_pm_valid_state(suspend_state_t state)
{
diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
index 3fcf8810f14e..bf980c6ef294 100644
--- a/arch/arm/mach-at91/pm.h
+++ b/arch/arm/mach-at91/pm.h
@@ -18,8 +18,6 @@
#include <soc/at91/at91sam9_sdramc.h>
#ifndef __ASSEMBLY__
-extern void __iomem *at91_ramc_base[];
-
#define at91_ramc_read(id, field) \
__raw_readl(at91_ramc_base[id] + field)
--
2.9.3
^ permalink raw reply related
* ARM juno R2 board USB Issue (EHCI probe failed)
From: Sudeep Holla @ 2016-09-27 9:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AT5PR84MB00524ACD87B9FF61AAB55223D9CC0@AT5PR84MB0052.NAMPRD84.PROD.OUTLOOK.COM>
On 27/09/16 09:55, Sajjan, Vikas C wrote:
> Hi Sudeep,
>
> -----Original Message-----
> From: Sudeep Holla [mailto:sudeep.holla at arm.com]
> Sent: Tuesday, September 27, 2016 2:21 PM
> To: Vikas Sajjan <sajjan.linux@gmail.com>; linux-usb at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-acpi at vger.kernel.org
> Cc: Sudeep Holla <sudeep.holla@arm.com>; mark.rutland at arm.com; lorenzo.pieralisi at arm.com; Sajjan, Vikas C <vikas.cha.sajjan@hpe.com>
> Subject: Re: ARM juno R2 board USB Issue (EHCI probe failed)
>
> Hi Vikas,
>
> On 27/09/16 09:14, Vikas Sajjan wrote:
>> Adding USB mailing list.
>>
>>
>> On Tue, Sep 27, 2016 at 12:33 PM, Sajjan, Vikas C
>> <vikas.cha.sajjan@hpe.com> wrote:
>>> Hi All,
>>>
>>> I working on ARM juno R2 board, with latest kernel 4.8.rc7 and I get
>>> below USB EHCI probe error while booting with acpi=force.
>>>
>
> Are you using the latest UEFI EDK2 ?
> No, I am still using the UEFI binary which came as part of the Juno board.
>
>
>>> [ 1.223662] VFIO - User Level meta-driver version: 0.3
>>> [ 1.229335] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
>>> [ 1.235882] ehci-pci: EHCI PCI platform driver
>>> [ 1.240359] ehci-platform: EHCI generic platform driver
>>> [ 1.245619] ehci-platform ARMH0D20:00: Error: DMA mask configuration failed
>>> [ 1.272491] ehci-platform: probe of ARMH0D20:00 failed with error -5
>>> [ 1.278876] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
>>> [ 1.285071] ohci-pci: OHCI PCI platform driver
>>> [ 1.289548] ohci-platform: OHCI generic platform driver
>>> [ 1.294884] usbcore: registered new interface driver usb-storage
>>> [ 1.301231] mousedev: PS/2 mouse device common for all mice
>>> [ 1.307197] rtc-efi rtc-efi: rtc core: registered rtc-efi as rtc0
>>>
>>> But this error goes off, if I don't force ACPI booting, i.e., if I
>>> remove acpi=force from kernel command line , USB is detected and my
>>> RFS which is in the usb drive, gets mounted successfully.
>>>
>
> As I mentioned in private, I do get the same error if I drop _CCA in
> USB object of ACPI DSDT. Can you give it a spin with latest UEFI ?
>
> Sure, will try with latest UEFI.
>
I bet that's 8-12 months old. It puts the banner during boot with the
build date. You can try to follow [1] or access it from [2]
--
Regards,
Sudeep
[1] https://community.arm.com/docs/DOC-11395
[2]
http://snapshots.linaro.org/member-builds/armlt-platforms-release/32/juno-uefi.zip
^ 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