* [PATCH 02/16] ARM: b.L: introduce the CPU/cluster power API
From: Santosh Shilimkar @ 2013-01-11 17:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357777251-13541-3-git-send-email-nicolas.pitre@linaro.org>
On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote:
> This is the basic API used to handle the powering up/down of individual
> CPUs in a big.LITTLE system. The platform specific backend implementation
> has the responsibility to also handle the cluster level power as well when
> the first/last CPU in a cluster is brought up/down.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/arm/common/bL_entry.c | 88 +++++++++++++++++++++++++++++++++++++++
> arch/arm/include/asm/bL_entry.h | 92 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 180 insertions(+)
>
> diff --git a/arch/arm/common/bL_entry.c b/arch/arm/common/bL_entry.c
> index 80fff49417..41de0622de 100644
> --- a/arch/arm/common/bL_entry.c
> +++ b/arch/arm/common/bL_entry.c
> @@ -11,11 +11,13 @@
>
> #include <linux/kernel.h>
> #include <linux/init.h>
> +#include <linux/irqflags.h>
>
> #include <asm/bL_entry.h>
> #include <asm/barrier.h>
> #include <asm/proc-fns.h>
> #include <asm/cacheflush.h>
> +#include <asm/idmap.h>
>
> extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];
>
> @@ -28,3 +30,89 @@ void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr)
> outer_clean_range(__pa(&bL_entry_vectors[cluster][cpu]),
> __pa(&bL_entry_vectors[cluster][cpu + 1]));
> }
> +
> +static const struct bL_platform_power_ops *platform_ops;
> +
> +int __init bL_platform_power_register(const struct bL_platform_power_ops *ops)
> +{
> + if (platform_ops)
> + return -EBUSY;
> + platform_ops = ops;
> + return 0;
> +}
> +
> +int bL_cpu_power_up(unsigned int cpu, unsigned int cluster)
> +{
> + if (!platform_ops)
> + return -EUNATCH;
> + might_sleep();
> + return platform_ops->power_up(cpu, cluster);
> +}
> +
> +typedef void (*phys_reset_t)(unsigned long);
> +
> +void bL_cpu_power_down(void)
> +{
> + phys_reset_t phys_reset;
> +
> + BUG_ON(!platform_ops);
> + BUG_ON(!irqs_disabled());
> +
> + /*
> + * Do this before calling into the power_down method,
> + * as it might not always be safe to do afterwards.
> + */
> + setup_mm_for_reboot();
> +
> + platform_ops->power_down();
> +
> + /*
> + * It is possible for a power_up request to happen concurrently
> + * with a power_down request for the same CPU. In this case the
> + * power_down method might not be able to actually enter a
> + * powered down state with the WFI instruction if the power_up
> + * method has removed the required reset condition. The
> + * power_down method is then allowed to return. We must perform
> + * a re-entry in the kernel as if the power_up method just had
> + * deasserted reset on the CPU.
> + *
> + * To simplify race issues, the platform specific implementation
> + * must accommodate for the possibility of unordered calls to
> + * power_down and power_up with a usage count. Therefore, if a
> + * call to power_up is issued for a CPU that is not down, then
> + * the next call to power_down must not attempt a full shutdown
> + * but only do the minimum (normally disabling L1 cache and CPU
> + * coherency) and return just as if a concurrent power_up request
> + * had happened as described above.
> + */
> +
> + phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
> + phys_reset(virt_to_phys(bL_entry_point));
> +
> + /* should never get here */
> + BUG();
> +}
> +
> +void bL_cpu_suspend(u64 expected_residency)
> +{
> + phys_reset_t phys_reset;
> +
> + BUG_ON(!platform_ops);
> + BUG_ON(!irqs_disabled());
> +
> + /* Very similar to bL_cpu_power_down() */
> + setup_mm_for_reboot();
> + platform_ops->suspend(expected_residency);
> + phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
> + phys_reset(virt_to_phys(bL_entry_point));
> + BUG();
>
I might be missing all the rationales behind not having a recovery for
CPUs entering suspend if they actualy come here because of some events.
This is pretty much possible in many scenario's and hence letting CPU
cpu come out of suspend should be possible. May be switcher code don't
have such requirement but it appeared bit off to me.
Regards
Santosh
^ permalink raw reply
* [PATCH v2 2/2] ARM: KVM: Power State Coordination Interface implementation
From: Marc Zyngier @ 2013-01-11 17:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111171232.GH23505@n2100.arm.linux.org.uk>
On 11/01/13 17:12, Russell King - ARM Linux wrote:
> On Thu, Jan 10, 2013 at 04:06:45PM +0000, Marc Zyngier wrote:
>> +int kvm_psci_call(struct kvm_vcpu *vcpu)
>> +{
>> + unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>> + unsigned long val;
>> +
>> + switch (psci_fn) {
>> + case KVM_PSCI_FN_CPU_OFF:
>> + kvm_psci_vcpu_off(vcpu);
>> + val = KVM_PSCI_RET_SUCCESS;
>> + break;
>> + case KVM_PSCI_FN_CPU_ON:
>> + val = kvm_psci_vcpu_on(vcpu);
>> + break;
>> + case KVM_PSCI_FN_CPU_SUSPEND:
>> + case KVM_PSCI_FN_MIGRATE:
>> + val = KVM_PSCI_RET_NI;
>> + break;
>> +
>> + default:
>> + return -1;
>> + }
>> +
>> + *vcpu_reg(vcpu, 0) = val;
>> + return 0;
>> +}
>
> We were discussing recently on #kernel about kernel APIs and the way that
> our integer-returning functions pretty much use 0 for success, and -errno
> for failures, whereas our pointer-returning functions are a mess.
>
> And above we have something returning -1 to some other chunk of code outside
> this compilation unit. That doesn't sound particularly clever to me.
The original code used to return -EINVAL, see:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-January/004509.html
Christoffer (Cc-ed) didn't like this, hence the -1. I'm happy to revert
the code to its original state though.
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH 6/6] ARM: virt: hide CONFIG_ARM_VIRT_EXT from user
From: Christopher Covington @ 2013-01-11 17:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111164330.GA12464@mudshark.cambridge.arm.com>
On 01/11/2013 11:43 AM, Will Deacon wrote:
> On Fri, Jan 11, 2013 at 03:42:40PM +0000, Christopher Covington wrote:
>> Hi Will,
>
> Hello,
>
>> On 01/11/2013 10:34 AM, Will Deacon wrote:
>>> ARM_VIRT_EXT is a property of CPU_V7, but does not adversely affect
>>> other CPUs that can be built into the same kernel image (i.e. ARMv6+).
>>>
>>> This patch defaults ARM_VIRT_EXT to y if CPU_V7, allowing hypervisors
>>> such as KVM to make better use of the option and being able to rely
>>> on hyp-mode boot support.
>>
>> [...]
>>
>>> @@ -640,11 +641,6 @@ config ARM_VIRT_EXT
>>> use of this feature. Refer to Documentation/arm/Booting for
>>> details.
>>>
>>> - It is safe to enable this option even if the kernel may not be
>>> - booted in HYP mode, may not have support for the
>>> - virtualization extensions, or may be booted with a
>>> - non-compliant bootloader.
>>
>> Why take this out?
>
> I just removed that last paragraph because it's not user-selectable anymore,
> so this comment about enabling the option is redundant. I could remove all
> of the help text, but we have it for some other options that are selected
> automatically and it has a pointer to some documentation too.
It's not a big deal, but that paragraph helped me understand the (lack of)
ramifications for v7 CPU's without the virtualization extensions, and I figure
it might still be helpful for others as well.
Christopher
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
the Linux Foundation
^ permalink raw reply
* [PATCH] ARM: kernel: DT cpu map validity check helper function
From: Lorenzo Pieralisi @ 2013-01-11 17:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111163057.GE23505@n2100.arm.linux.org.uk>
On Fri, Jan 11, 2013 at 04:30:57PM +0000, Russell King - ARM Linux wrote:
> On Fri, Jan 11, 2013 at 04:17:38PM +0000, Lorenzo Pieralisi wrote:
> > This patch implements a helper function that platforms can call to check
> > whether DT based cpu map initialization and cores count were completed
> > successfully.
>
> Umm, are you sure this works? Two problems here:
> - the kernel boot marks the booting CPU (in our case, CPU0) as present,
> possible and online before arch code gets called. smp_init_cpus()
> will be called with the maps already initialized per that.
Gah, my bad, you are definitely right.
> - this really needs to be paired with a patch showing how you intend it
> to be used; merely adding the helper without any sign of it being used
> means it's just bloat which may not end up being used.
Ok, that's a fair point, I will ask to get it merged with an implementation
that relies on it, showing its usage.
Thanks for the review,
Lorenzo
^ permalink raw reply
* [PATCH 04/14] usb: phy: nop: Handle power supply regulator for the PHY
From: Russell King - ARM Linux @ 2013-01-11 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357836694-30788-5-git-send-email-rogerq@ti.com>
On Thu, Jan 10, 2013 at 06:51:24PM +0200, Roger Quadros wrote:
> We use "vcc" as the supply name for the PHY's power supply.
> The power supply will be enabled during .init() and disabled
> during .shutdown()
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> drivers/usb/otg/nop-usb-xceiv.c | 18 ++++++++++++++++++
> 1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
> index 163f972..1c6db10 100644
> --- a/drivers/usb/otg/nop-usb-xceiv.c
> +++ b/drivers/usb/otg/nop-usb-xceiv.c
> @@ -33,11 +33,13 @@
> #include <linux/usb/nop-usb-xceiv.h>
> #include <linux/slab.h>
> #include <linux/clk.h>
> +#include <linux/regulator/consumer.h>
>
> struct nop_usb_xceiv {
> struct usb_phy phy;
> struct device *dev;
> struct clk *clk;
> + struct regulator *vcc;
> };
>
> static struct platform_device *pd;
> @@ -70,6 +72,11 @@ static int nop_init(struct usb_phy *phy)
> {
> struct nop_usb_xceiv *nop = dev_get_drvdata(phy->dev);
>
> + if (nop->vcc) {
> + if (regulator_enable(nop->vcc))
> + dev_err(phy->dev, "Failed to enable power\n");
> + }
> +
> if (nop->clk)
> clk_enable(nop->clk);
>
> @@ -82,6 +89,11 @@ static void nop_shutdown(struct usb_phy *phy)
>
> if (nop->clk)
> clk_disable(nop->clk);
> +
> + if (nop->vcc) {
> + if (regulator_disable(nop->vcc))
> + dev_err(phy->dev, "Failed to disable power\n");
> + }
> }
>
> static int nop_set_peripheral(struct usb_otg *otg, struct usb_gadget *gadget)
> @@ -157,6 +169,12 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
> }
> }
>
> + nop->vcc = devm_regulator_get(&pdev->dev, "vcc");
> + if (IS_ERR(nop->vcc)) {
> + dev_dbg(&pdev->dev, "Error getting vcc regulator\n");
> + nop->vcc = NULL;
> + }
Is it really appropriate for drivers to do this kind of thing with
pointer-returning functions (I mean, setting the pointer to NULL on
error, rather than just using a test for IS_ERR() in the above
locations). You are imposing driver-local assumptions on an API.
Practically it probably doesn't make much difference but given the
amount of mistakes that we have with IS_ERR_OR_NULL()...
^ permalink raw reply
* [PATCH 01/16] ARM: b.L: secondary kernel entry code
From: Santosh Shilimkar @ 2013-01-11 17:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357777251-13541-2-git-send-email-nicolas.pitre@linaro.org>
On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote:
> CPUs in a big.LITTLE systems have special needs when entering the kernel
> due to a hotplug event, or when resuming from a deep sleep mode.
>
> This is vectorized so multiple CPUs can enter the kernel in parallel
> without serialization.
>
> Only the basic structure is introduced here. This will be extended
> later.
>
> TODO: MPIDR based indexing should eventually be made runtime adjusted.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
[..]
> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
> index e8a4e58f1b..50880c494f 100644
> --- a/arch/arm/common/Makefile
> +++ b/arch/arm/common/Makefile
> @@ -13,3 +13,6 @@ obj-$(CONFIG_SHARP_PARAM) += sharpsl_param.o
> obj-$(CONFIG_SHARP_SCOOP) += scoop.o
> obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o
> obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o
> +obj-$(CONFIG_FIQ_GLUE) += fiq_glue.o fiq_glue_setup.o
> +obj-$(CONFIG_FIQ_DEBUGGER) += fiq_debugger.o
> +obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o
> diff --git a/arch/arm/common/bL_entry.c b/arch/arm/common/bL_entry.c
> new file mode 100644
> index 0000000000..80fff49417
> --- /dev/null
> +++ b/arch/arm/common/bL_entry.c
> @@ -0,0 +1,30 @@
> +/*
> + * arch/arm/common/bL_entry.c -- big.LITTLE kernel re-entry point
> + *
> + * Created by: Nicolas Pitre, March 2012
> + * Copyright: (C) 2012 Linaro Limited
2013 now :-)
Looks like you need to update rest of the patches as well.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +
> +#include <asm/bL_entry.h>
> +#include <asm/barrier.h>
> +#include <asm/proc-fns.h>
> +#include <asm/cacheflush.h>
> +
> +extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];
> +
> +void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr)
> +{
> + unsigned long val = ptr ? virt_to_phys(ptr) : 0;
> + bL_entry_vectors[cluster][cpu] = val;
> + smp_wmb();
> + __cpuc_flush_dcache_area((void *)&bL_entry_vectors[cluster][cpu], 4);
> + outer_clean_range(__pa(&bL_entry_vectors[cluster][cpu]),
> + __pa(&bL_entry_vectors[cluster][cpu + 1]));
> +}
I had the same question about smp_wmb() as Catalin but after following
rest of the comments, I understand it will be removed so thats good.
> diff --git a/arch/arm/common/bL_head.S b/arch/arm/common/bL_head.S
> new file mode 100644
> index 0000000000..9d351f2b4c
> --- /dev/null
> +++ b/arch/arm/common/bL_head.S
> @@ -0,0 +1,81 @@
> +/*
> + * arch/arm/common/bL_head.S -- big.LITTLE kernel re-entry point
> + *
> + * Created by: Nicolas Pitre, March 2012
> + * Copyright: (C) 2012 Linaro Limited
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/linkage.h>
> +#include <asm/bL_entry.h>
> +
> + .macro pr_dbg cpu, string
> +#if defined(CONFIG_DEBUG_LL) && defined(DEBUG)
> + b 1901f
> +1902: .ascii "CPU 0: \0CPU 1: \0CPU 2: \0CPU 3: \0"
> + .ascii "CPU 4: \0CPU 5: \0CPU 6: \0CPU 7: \0"
> +1903: .asciz "\string"
> + .align
> +1901: adr r0, 1902b
> + add r0, r0, \cpu, lsl #3
> + bl printascii
> + adr r0, 1903b
> + bl printascii
> +#endif
> + .endm
> +
> + .arm
> + .align
> +
> +ENTRY(bL_entry_point)
> +
> + THUMB( adr r12, BSYM(1f) )
> + THUMB( bx r12 )
> + THUMB( .thumb )
> +1:
> + mrc p15, 0, r0, c0, c0, 5
> + ubfx r9, r0, #0, #4 @ r9 = cpu
> + ubfx r10, r0, #8, #4 @ r10 = cluster
> + mov r3, #BL_CPUS_PER_CLUSTER
> + mla r4, r3, r10, r9 @ r4 = canonical CPU index
> + cmp r4, #(BL_CPUS_PER_CLUSTER * BL_NR_CLUSTERS)
> + blo 2f
> +
> + /* We didn't expect this CPU. Try to make it quiet. */
> +1: wfi
> + wfe
Why do you need a wfe followed by wif ?
Just curious.
Regards
Santosh
^ permalink raw reply
* [PATCH v2 2/2] ARM: KVM: Power State Coordination Interface implementation
From: Russell King - ARM Linux @ 2013-01-11 17:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357834005-23843-3-git-send-email-marc.zyngier@arm.com>
On Thu, Jan 10, 2013 at 04:06:45PM +0000, Marc Zyngier wrote:
> +int kvm_psci_call(struct kvm_vcpu *vcpu)
> +{
> + unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
> + unsigned long val;
> +
> + switch (psci_fn) {
> + case KVM_PSCI_FN_CPU_OFF:
> + kvm_psci_vcpu_off(vcpu);
> + val = KVM_PSCI_RET_SUCCESS;
> + break;
> + case KVM_PSCI_FN_CPU_ON:
> + val = kvm_psci_vcpu_on(vcpu);
> + break;
> + case KVM_PSCI_FN_CPU_SUSPEND:
> + case KVM_PSCI_FN_MIGRATE:
> + val = KVM_PSCI_RET_NI;
> + break;
> +
> + default:
> + return -1;
> + }
> +
> + *vcpu_reg(vcpu, 0) = val;
> + return 0;
> +}
We were discussing recently on #kernel about kernel APIs and the way that
our integer-returning functions pretty much use 0 for success, and -errno
for failures, whereas our pointer-returning functions are a mess.
And above we have something returning -1 to some other chunk of code outside
this compilation unit. That doesn't sound particularly clever to me.
^ permalink raw reply
* [PATCH 00/11] switch the Nomadik to Device Tree
From: Grant Likely @ 2013-01-11 17:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357599325-7310-1-git-send-email-linus.walleij@linaro.org>
On Mon, 7 Jan 2013 23:55:25 +0100, Linus Walleij <linus.walleij@linaro.org> wrote:
> This implements Device Tree support for the Nomadik S8815 and
> deletes the old board files.
>
> When another patch from today removing the <mach/irqs.h> file
> dependency from drivers/pinctrl/pinctrl-nomadik.c is merged
> I can also remove <mach/irqs.h> and move the machine over to
> multiplatform, quite easily, as it will then have all required
> things in place: generic clocks, SPARSE_IRQ, DT...
>
> Caveats: some auxdata left, notably for naming some blocks
> to match the clock lookups. There is also an OCR mask thing
> which I think was debated in the past (Lee?)
>
> I would like to get this in first then fix the auxdatas one
> by one.
>
> Linus Walleij (11):
> ARM: nomadik: move last custom calls to pinctrl
> ARM: nomadik: initial devicetree support
> ARM: nomadik: move pin maps to cpu file
> ARM: nomadik: move remaining PrimeCells to device tree
> ARM: nomadik: add FSMC NAND
> ARM: nomadik: move GPIO and pinctrl to device tree
> ARM: nomadik: convert SMSC91x ethernet to device tree
> ARM: nomadik: migrate MMC/SD card support to device tree
> ARM: nomadik: add I2C devices to the device tree
> ARM: nomadik: delete old board files
> ARM: nomadik: get rid of <mach/hardware.h>
>
> .../devicetree/bindings/mtd/fsmc-nand.txt | 2 +-
> arch/arm/Kconfig | 2 +
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/ste-nomadik-s8815.dts | 34 ++
> arch/arm/boot/dts/ste-nomadik-stn8815.dtsi | 256 ++++++++++++++
> arch/arm/mach-nomadik/Kconfig | 10 +-
> arch/arm/mach-nomadik/Makefile | 6 -
> arch/arm/mach-nomadik/board-nhk8815.c | 359 --------------------
> arch/arm/mach-nomadik/cpu-8815.c | 370 +++++++++++++++------
> arch/arm/mach-nomadik/cpu-8815.h | 4 -
> arch/arm/mach-nomadik/i2c-8815nhk.c | 88 -----
> arch/arm/mach-nomadik/include/mach/hardware.h | 90 -----
> arch/arm/mach-nomadik/include/mach/irqs.h | 2 -
> arch/arm/mach-nomadik/include/mach/uncompress.h | 1 -
> drivers/mtd/nand/fsmc_nand.c | 1 +
> drivers/pinctrl/pinctrl-nomadik.c | 4 +
> 16 files changed, 578 insertions(+), 652 deletions(-)
Nice diffstat. :-)
Series looks pretty nice to me.
g.
^ permalink raw reply
* [PATCH 0/3] arch_decomp_wdog cleanup
From: Russell King - ARM Linux @ 2013-01-11 17:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357827165-9320-1-git-send-email-shawn.guo@linaro.org>
On Thu, Jan 10, 2013 at 10:12:42PM +0800, Shawn Guo wrote:
> All the arch_decomp_wdog related codes are there without any users,
> since ARCH_HAS_DECOMP_WDOG is only used by lib/inflate.c which however
> is not used by arch/arm/boot/compressed/decompress.c.
Hmm, this suggests that those samsung platforms haven't been booted for
a while - because the decompressor will enable the watchdog, but won't
pat it at all. So, do we still need them in the kernel?
Other than that, this looks good. Who's going to deal with it though?
^ permalink raw reply
* [PATCH 02/11] ARM: nomadik: initial devicetree support
From: Grant Likely @ 2013-01-11 17:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130108095729.GA2718@e106331-lin.cambridge.arm.com>
On Tue, 8 Jan 2013 09:57:29 +0000, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi,
>
> I have a couple of comments on the dts{,i} portion.
>
> On Mon, Jan 07, 2013 at 10:55:59PM +0000, Linus Walleij wrote:
> > Support basic device tree boot on the Nomadik. Implement the
> > support in the cpu file with the intent of deleting the board
> > files later. At this stage IRQ controllers, system timer,
> > l2x0 cache, UARTs and thus console boot is fully functional.
> > Patch out the code adding devices by initcalls for now so
> > as not to disturb the boot.
> >
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > arch/arm/boot/dts/Makefile | 1 +
> > arch/arm/boot/dts/ste-nomadik-s8815.dts | 18 ++++++
> > arch/arm/boot/dts/ste-nomadik-stn8815.dtsi | 77 +++++++++++++++++++++++++
> > arch/arm/mach-nomadik/board-nhk8815.c | 4 ++
> > arch/arm/mach-nomadik/cpu-8815.c | 90 ++++++++++++++++++++++++++++++
> > arch/arm/mach-nomadik/cpu-8815.h | 1 +
> > arch/arm/mach-nomadik/i2c-8815nhk.c | 5 ++
> > 7 files changed, 196 insertions(+)
> > create mode 100644 arch/arm/boot/dts/ste-nomadik-s8815.dts
> > create mode 100644 arch/arm/boot/dts/ste-nomadik-stn8815.dtsi
> >
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index e44da40..7bc5ba73 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -100,6 +100,7 @@ dtb-$(CONFIG_ARCH_MXS) += imx23-evk.dtb \
> > imx28-m28evk.dtb \
> > imx28-sps1.dtb \
> > imx28-tx28.dtb
> > +dtb-$(CONFIG_ARCH_NOMADIK) += ste-nomadik-s8815.dtb
> > dtb-$(CONFIG_ARCH_OMAP2PLUS) += omap2420-h4.dtb \
> > omap3-beagle.dtb \
> > omap3-beagle-xm.dtb \
> > diff --git a/arch/arm/boot/dts/ste-nomadik-s8815.dts b/arch/arm/boot/dts/ste-nomadik-s8815.dts
> > new file mode 100644
> > index 0000000..b698bc2
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/ste-nomadik-s8815.dts
> > @@ -0,0 +1,18 @@
> > +/*
> > + * Device Tree for the ST-Ericsson Nomadik S8815 board
> > + * Produced by Calao Systems
> > + */
> > +
> > +/dts-v1/;
> > +/include/ "ste-nomadik-stn8815.dtsi"
> > +
> > +/ {
> > + model = "ST-Ericsson Nomadik S8815";
> > + compatible = "stericsson,nomadik";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
>
> Given there are addresses and sizes in the dtsi (but none in the dts), should
> #address-cells and #size-cells not be in the dtsi?
>
> > +
> > + chosen {
> > + bootargs = "root=/dev/ram0 console=ttyAMA1,115200n8 earlyprintk";
> > + };
> > +};
> > diff --git a/arch/arm/boot/dts/ste-nomadik-stn8815.dtsi b/arch/arm/boot/dts/ste-nomadik-stn8815.dtsi
> > new file mode 100644
> > index 0000000..81284c5
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/ste-nomadik-stn8815.dtsi
> > @@ -0,0 +1,77 @@
> > +/*
> > + * Device Tree for the ST-Ericsson Nomadik 8815 STn8815 SoC
> > + */
> > +/include/ "skeleton.dtsi"
> > +
> > +/ {
> > + memory {
> > + reg = <0x00000000 0x04000000>;
> > + reg = <0x08000000 0x04000000>;
>
> I believe that's not valid (duplicate property name).
>
> Judging by other dts files under arch/arm/boot/dts, these should be two nodes,
> with their unit addresses set (and possibly device_type, which I see is set for
> some dts files, but not all).
One node should be just fine if the following form is used:
memory {
reg = <0x00000000 0x04000000>,
<0x08000000 0x04000000>;
};
>
> Maybe I've misunderstood how this is laid out, but I can't see where we get a
> cpus node from in the board's dts. Has this just been forgotten?
A cpus node isn't required if it doesn't provide any non-discoverable
information.
g.
^ permalink raw reply
* [PATCH v4 2/5] ARM: dts: Add disable-wp for sd card slot on smdk5250
From: Doug Anderson @ 2013-01-11 17:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357923834-31641-1-git-send-email-dianders@chromium.org>
The next change will remove the code from the dw_mmc-exynos that added
the DW_MCI_QUIRK_NO_WRITE_PROTECT. Keep existing functionality of
having no write protect pin on smdk5250 by adding the disable-wp
property.
Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Seungwon Jeon <tgih.jun@samsung.com>
---
Changes in v4: None
Changes in v3:
- New for this version of the patch series.
arch/arm/boot/dts/exynos5250-smdk5250.dts | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index 942d576..75b9d83 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -146,6 +146,7 @@
reg = <0>;
bus-width = <4>;
samsung,cd-pinmux-gpio = <&gpc3 2 2 3 3>;
+ disable-wp;
gpios = <&gpc3 0 2 0 3>, <&gpc3 1 2 0 3>,
<&gpc3 3 2 3 3>, <&gpc3 4 2 3 3>,
<&gpc3 5 2 3 3>, <&gpc3 6 2 3 3>,
--
1.7.7.3
^ permalink raw reply related
* [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access
From: Santosh Shilimkar @ 2013-01-11 17:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111150703.GA13717@mudshark.cambridge.arm.com>
On Friday 11 January 2013 08:37 PM, Will Deacon wrote:
> On Fri, Jan 11, 2013 at 02:54:52PM +0000, Mark Rutland wrote:
>> On Fri, Jan 11, 2013 at 01:40:22PM +0000, Santosh Shilimkar wrote:
>>> So how do you expect platform to enabled the user-space access in case
>>> they want to access it for some cases.
>>
>> Unlike AArch64, at the moment we don't have the infrastructure to map this for
>> userspace accesses, so it isn't much of a problem.
>>
>> If in future we wish to map it on 32bit platforms, the arm implementation of
>> arch_counter_set_user_access can be modified to allow userspace access to
>> specific registers, and additional code would be required to actually map it
>> into the user address space, etc.
>
> I'd also add that it's not up to a platform to decide whether to expose
> this to userspace: it needs to be an architecture-wide decision. Otherwise,
> userspace becomes SoC-specific, which is a complete disaster.
>
> So, if userspace people want these available, they need to convince us to
> flip the switch. In the meantime, it should default to off so that if/when
> we do enable it we can do it in a sane manner for ARM (perhaps via the
> vectors page).
>
Thanks Will for rationale behind the change. Good to capture the
reasoning in changelog for future reference.
Regards,
Santosh
^ permalink raw reply
* [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access
From: Santosh Shilimkar @ 2013-01-11 16:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111165053.GB1794@arm.com>
On Friday 11 January 2013 10:20 PM, Catalin Marinas wrote:
> On Fri, Jan 11, 2013 at 01:40:22PM +0000, Santosh Shilimkar wrote:
>> On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
>>> Several bits in CNTKCTL reset to 0, including PL0VTEN. For platforms
>>> using the generic timer which wish to have a fast gettimeofday vDSO
>>> implementation, these bits must be set to 1 by the kernel. On other
>>> platforms, the bootloader might enable userspace access when we don't
>>> want it.
>>>
>>> This patch adds arch_counter_set_user_access, which sets the PL0 access
>>> permissions to that required by the platform. For arm, this currently
>> minor nit.
>> s/arm/ARM
>
> I think Mark meant arch/arm. Or we could call it AArch32 where we don't
> use this feature.
>
OK. AArch32 or even arch/arm is just fine then.
Regards,
Santosh
^ permalink raw reply
* [PATCH 04/16] ARM: b.L: Add baremetal voting mutexes
From: Dave Martin @ 2013-01-11 16:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.02.1301102131010.6300@xanadu.home>
On Thu, Jan 10, 2013 at 10:15:22PM -0500, Nicolas Pitre wrote:
> On Thu, 10 Jan 2013, Will Deacon wrote:
>
> > On Thu, Jan 10, 2013 at 12:20:39AM +0000, Nicolas Pitre wrote:
> > > From: Dave Martin <dave.martin@linaro.org>
> > >
> > > This patch adds a simple low-level voting mutex implementation
> > > to be used to arbitrate during first man selection when no load/store
> > > exclusive instructions are usable.
> > >
> > > For want of a better name, these are called "vlocks". (I was
> > > tempted to call them ballot locks, but "block" is way too confusing
> > > an abbreviation...)
> > >
> > > There is no function to wait for the lock to be released, and no
> > > vlock_lock() function since we don't need these at the moment.
> > > These could straightforwardly be added if vlocks get used for other
> > > purposes.
> >
> > [...]
> >
> > > diff --git a/Documentation/arm/big.LITTLE/vlocks.txt b/Documentation/arm/big.LITTLE/vlocks.txt
> > > new file mode 100644
> > > index 0000000000..90672ddc6a
> > > --- /dev/null
> > > +++ b/Documentation/arm/big.LITTLE/vlocks.txt
> > > @@ -0,0 +1,211 @@
> > > +vlocks for Bare-Metal Mutual Exclusion
> > > +======================================
> >
> > [...]
> >
> > > +ARM implementation
> > > +------------------
> > > +
> > > +The current ARM implementation [2] contains a some optimisations beyond
> >
> > -a
>
> Fixed.
>
> >
> > > +the basic algorithm:
> > > +
> > > + * By packing the members of the currently_voting array close together,
> > > + we can read the whole array in one transaction (providing the number
> > > + of CPUs potentially contending the lock is small enough). This
> > > + reduces the number of round-trips required to external memory.
> > > +
> > > + In the ARM implementation, this means that we can use a single load
> > > + and comparison:
> > > +
> > > + LDR Rt, [Rn]
> > > + CMP Rt, #0
> > > +
> > > + ...in place of code equivalent to:
> > > +
> > > + LDRB Rt, [Rn]
> > > + CMP Rt, #0
> > > + LDRBEQ Rt, [Rn, #1]
> > > + CMPEQ Rt, #0
> > > + LDRBEQ Rt, [Rn, #2]
> > > + CMPEQ Rt, #0
> > > + LDRBEQ Rt, [Rn, #3]
> > > + CMPEQ Rt, #0
> > > +
> > > + This cuts down on the fast-path latency, as well as potentially
> > > + reducing bus contention in contended cases.
> > > +
> > > + The optimisation relies on the fact that the ARM memory system
> > > + guarantees coherency between overlapping memory accesses of
> > > + different sizes, similarly to many other architectures. Note that
> > > + we do not care which element of currently_voting appears in which
> > > + bits of Rt, so there is no need to worry about endianness in this
> > > + optimisation.
> > > +
> > > + If there are too many CPUs to read the currently_voting array in
> > > + one transaction then multiple transations are still required. The
> > > + implementation uses a simple loop of word-sized loads for this
> > > + case. The number of transactions is still fewer than would be
> > > + required if bytes were loaded individually.
> > > +
> > > +
> > > + In principle, we could aggregate further by using LDRD or LDM, but
> > > + to keep the code simple this was not attempted in the initial
> > > + implementation.
> > > +
> > > +
> > > + * vlocks are currently only used to coordinate between CPUs which are
> > > + unable to enable their caches yet. This means that the
> > > + implementation removes many of the barriers which would be required
> > > + when executing the algorithm in cached memory.
> >
> > I think you need to elaborate on this and clearly identify the
> > requirements of the memory behaviour. In reality, these locks are hardly
> > ever usable so we don't want them cropping up in driver code and the
> > like!
>
> Doesn't the following paragraph make that clear enough?
>
> Maybe we should rip out the C interface to avoid such abuses. I think
> that was initially added when we weren't sure if the C code had to be
> involved.
>
> > > + packing of the currently_voting array does not work with cached
> > > + memory unless all CPUs contending the lock are cache-coherent, due
> > > + to cache writebacks from one CPU clobbering values written by other
> > > + CPUs. (Though if all the CPUs are cache-coherent, you should be
> > > + probably be using proper spinlocks instead anyway).
> > > +
> > > +
> > > + * The "no votes yet" value used for the last_vote variable is 0 (not
> > > + -1 as in the pseudocode). This allows statically-allocated vlocks
> > > + to be implicitly initialised to an unlocked state simply by putting
> > > + them in .bss.
> >
> > You could also put them in their own section and initialise them to -1
> > there.
>
> Same argument as for bL_vectors: That is less efficient than using .bss
> which takes no image space. Plus the transformation for CPU 0 to work
> with this is basically free.
>
> > > + An offset is added to each CPU's ID for the purpose of setting this
> > > + variable, so that no CPU uses the value 0 for its ID.
> > > +
> > > +
> > > +Colophon
> > > +--------
> > > +
> > > +Originally created and documented by Dave Martin for Linaro Limited, for
> > > +use in ARM-based big.LITTLE platforms, with review and input gratefully
> > > +received from Nicolas Pitre and Achin Gupta. Thanks to Nicolas for
> > > +grabbing most of this text out of the relevant mail thread and writing
> > > +up the pseudocode.
> > > +
> > > +Copyright (C) 2012 Linaro Limited
> > > +Distributed under the terms of Version 2 of the GNU General Public
> > > +License, as defined in linux/COPYING.
> > > +
> > > +
> > > +References
> > > +----------
> > > +
> > > +[1] Lamport, L. "A New Solution of Dijkstra's Concurrent Programming
> > > + Problem", Communications of the ACM 17, 8 (August 1974), 453-455.
> > > +
> > > + http://en.wikipedia.org/wiki/Lamport%27s_bakery_algorithm
> > > +
> > > +[2] linux/arch/arm/common/vlock.S, www.kernel.org.
> > > diff --git a/arch/arm/common/vlock.S b/arch/arm/common/vlock.S
> > > new file mode 100644
> > > index 0000000000..0a1ee3a7f5
> > > --- /dev/null
> > > +++ b/arch/arm/common/vlock.S
> > > @@ -0,0 +1,108 @@
> > > +/*
> > > + * vlock.S - simple voting lock implementation for ARM
> > > + *
> > > + * Created by: Dave Martin, 2012-08-16
> > > + * Copyright: (C) 2012 Linaro Limited
> > > + *
> > > + * 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
> > > + * (at your option) any later version.
> >
> > Your documentation is strictly GPLv2, so there's a strange discrepancy
> > here.
>
> Indeed.
>
> @Dave: your call.
This can all be strict v2. The discrepancy was unintentional.
>
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > + * GNU General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU General Public License along
> > > + * with this program; if not, write to the Free Software Foundation, Inc.,
> > > + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> > > + *
> > > + *
> > > + * This algorithm is described in more detail in
> > > + * Documentation/arm/big.LITTLE/vlocks.txt.
> > > + */
> > > +
> > > +#include <linux/linkage.h>
> > > +#include "vlock.h"
> > > +
> > > +#if VLOCK_VOTING_SIZE > 4
> >
> > 4? Maybe a CONFIG option or a #define in an arch vlock.h?
>
> The 4 here is actually related to the number of bytes in a word, to
> decide whether or not a loop is needed for voters enumeration. That is
> not configurable.
This is arch-specific assembler, and the 4-bytes-per-word proprty is
a fixed property of the architecture.
We could have a comment maybe:
/*
* Each voting occupies a byte, so if there are 4 or fewer, the whole
* set of voting flags can be accessed with a single word access.
*/
>
> > > +#define FEW(x...)
> > > +#define MANY(x...) x
> > > +#else
> > > +#define FEW(x...) x
> > > +#define MANY(x...)
> > > +#endif
> > > +
> > > +@ voting lock for first-man coordination
> > > +
> > > +.macro voting_begin rbase:req, rcpu:req, rscratch:req
> > > + mov \rscratch, #1
> > > + strb \rscratch, [\rbase, \rcpu]
> > > + dsb
> > > +.endm
> > > +
> > > +.macro voting_end rbase:req, rcpu:req, rscratch:req
> > > + mov \rscratch, #0
> > > + strb \rscratch, [\rbase, \rcpu]
> > > + dsb
> > > + sev
> > > +.endm
> > > +
> > > +/*
> > > + * The vlock structure must reside in Strongly-Ordered or Device memory.
> > > + * This implementation deliberately eliminates most of the barriers which
> > > + * would be required for other memory types, and assumes that independent
> > > + * writes to neighbouring locations within a cacheline do not interfere
> > > + * with one another.
> > > + */
> > > +
> > > +@ r0: lock structure base
> > > +@ r1: CPU ID (0-based index within cluster)
> > > +ENTRY(vlock_trylock)
> > > + add r1, r1, #VLOCK_VOTING_OFFSET
> > > +
> > > + voting_begin r0, r1, r2
> > > +
> > > + ldrb r2, [r0, #VLOCK_OWNER_OFFSET] @ check whether lock is held
> > > + cmp r2, #VLOCK_OWNER_NONE
> > > + bne trylock_fail @ fail if so
> > > +
> > > + strb r1, [r0, #VLOCK_OWNER_OFFSET] @ submit my vote
> > > +
> > > + voting_end r0, r1, r2
> > > +
> > > + @ Wait for the current round of voting to finish:
> > > +
> > > + MANY( mov r3, #VLOCK_VOTING_OFFSET )
> > > +0:
> > > + MANY( ldr r2, [r0, r3] )
> > > + FEW( ldr r2, [r0, #VLOCK_VOTING_OFFSET] )
> > > + cmp r2, #0
> > > + wfene
> >
> > Is there a race here? I wonder if you can end up in a situation where
> > everybody enters wfe and then there is nobody left to signal an event
> > via voting_end (if, for example the last voter sent the sev when
> > everybody else was simultaneously doing the cmp before the wfe)...
> >
> > ... actually, that's ok as long as VLOCK_VOTING_OFFSET isn't speculated,
> > which it shouldn't be from strongly-ordered memory. Fair enough!
Indeed. The order of accesses to the voting flags is guaranteed by
strongly-ordered-ness. The ordering between the strb and sev in voting_end
required a dsb, which we have. The ordering between the external load and
wfe in the waiting code is guaranteed so S-O-ness and a control dependency.
> >
> > > + bne 0b
> > > + MANY( add r3, r3, #4 )
> > > + MANY( cmp r3, #VLOCK_VOTING_OFFSET + VLOCK_VOTING_SIZE )
> > > + MANY( bne 0b )
> > > +
> > > + @ Check who won:
> > > +
> > > + ldrb r2, [r0, #VLOCK_OWNER_OFFSET]
> > > + eor r0, r1, r2 @ zero if I won, else nonzero
> > > + bx lr
> > > +
> > > +trylock_fail:
> > > + voting_end r0, r1, r2
> > > + mov r0, #1 @ nonzero indicates that I lost
> > > + bx lr
> > > +ENDPROC(vlock_trylock)
> > > +
> > > +@ r0: lock structure base
> > > +ENTRY(vlock_unlock)
> > > + mov r1, #VLOCK_OWNER_NONE
> > > + dsb
> > > + strb r1, [r0, #VLOCK_OWNER_OFFSET]
> > > + dsb
> > > + sev
> > > + bx lr
> > > +ENDPROC(vlock_unlock)
> > > diff --git a/arch/arm/common/vlock.h b/arch/arm/common/vlock.h
> > > new file mode 100644
> > > index 0000000000..94c29a6caf
> > > --- /dev/null
> > > +++ b/arch/arm/common/vlock.h
> > > @@ -0,0 +1,43 @@
> > > +/*
> > > + * vlock.h - simple voting lock implementation
> > > + *
> > > + * Created by: Dave Martin, 2012-08-16
> > > + * Copyright: (C) 2012 Linaro Limited
> > > + *
> > > + * 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
> > > + * (at your option) any later version.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > + * GNU General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU General Public License along
> > > + * with this program; if not, write to the Free Software Foundation, Inc.,
> > > + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> > > + */
> > > +
> > > +#ifndef __VLOCK_H
> > > +#define __VLOCK_H
> > > +
> > > +#include <asm/bL_entry.h>
> > > +
> > > +#define VLOCK_OWNER_OFFSET 0
> > > +#define VLOCK_VOTING_OFFSET 4
> >
> > asm-offsets again?
>
> Same answer.
I did start out by adding stuff to asm-offsets, but it just ende up
looking like cruft.
asm-offsets is primarily for synchronising C structures with asm. The
vlock structure is not accessed from C, though.
>
> > > +#define VLOCK_VOTING_SIZE ((BL_CPUS_PER_CLUSTER + 3) / 4 * 4)
> >
> > Huh?
>
> Each ballot is one byte, and we pack them into words. So this is the
> size of the required words to hold all ballots.
Hopefully we don't need a comment? I hoped this was straightforward.
>
> > > +#define VLOCK_SIZE (VLOCK_VOTING_OFFSET + VLOCK_VOTING_SIZE)
> > > +#define VLOCK_OWNER_NONE 0
> > > +
> > > +#ifndef __ASSEMBLY__
> > > +
> > > +struct vlock {
> > > + char data[VLOCK_SIZE];
> > > +};
> >
> > Does this mean the struct is only single byte aligned? You do word
> > accesses to it in your vlock code and rely on atomicity, so I'd feel
> > safer if it was aligned to 4 bytes, especially since this isn't being
> > accessed via a normal mapping.
>
> The structure size is always a multiple of 4 bytes. Its alignment is
> actually much larger than 4 as it needs to span a whole cache line not
> to be overwritten by dirty line writeback.
>
> As I mentioned before, given that this structure is allocated and
> accessed only by assembly code, we could simply remove all those unused
> C definitions to avoid potential confusion and misuse.
Agreed. Originally I anticipated this stuff being usable from C, but
this is so tenuous that providing C declarations may just confuse people.
Cheers
---Dave
^ permalink raw reply
* [PATCHv2 07/11] arm: arch_timer: divorce from local_timer api
From: Santosh Shilimkar @ 2013-01-11 16:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111164635.GA1794@arm.com>
On Friday 11 January 2013 10:16 PM, Catalin Marinas wrote:
> On Fri, Jan 11, 2013 at 01:34:23PM +0000, Santosh Shilimkar wrote:
>> On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
>>> Currently, the arch_timer driver is tied to the arm port, as it relies
>>> on code in arch/arm/smp.c to setup and teardown timers as cores are
>>> hotplugged on and off. The timer is registered through an arm-specific
>>> registration mechanism, preventing sharing the driver with the arm64
>>> port.
>>>
>>> This patch moves the driver to using a cpu notifier instead, making it
>>> easier to port.
>>>
>>> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
>>> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>>> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>>> ---
>> This is really a nit idea. I think we should do the same
>> for ARM gic code.
>
> I plan to do the same once Rob's GIC patches get merged. In my
> soc-armv8-model branch I have a copy of gic.c into drivers/irqchip and
> the CPU interface is done automatically via a notifier. The only trick
> is to set the priority of the GIC notifier higher than the timer one.
>
Great. Thanks for the information.
Regards,
Santosh
^ permalink raw reply
* [PATCH 1/2 V3] iio: mxs: Implement support for touchscreen
From: Dmitry Torokhov @ 2013-01-11 16:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357861431-8978-1-git-send-email-marex@denx.de>
Hi Marek,
On Fri, Jan 11, 2013 at 12:43:50AM +0100, Marek Vasut wrote:
> This patch implements support for sampling of a touchscreen into
> the MXS LRADC driver. The LRADC block allows configuring some of
> it's channels into special mode where they either output the drive
> voltage or sample it, allowing it to operate a 4-wire or 5-wire
> resistive touchscreen.
>
> In case the touchscreen mode is enabled, the LRADC slot #7 is
> reserved for touchscreen only, therefore it is not possible to
> sample 8 LRADC channels at time, but only 7 channels.
>
> The touchscreen controller is configured such that the PENDOWN event
> disables touchscreen interrupts and triggers execution of worker
> thread, which then polls the touchscreen controller for X, Y and
> Pressure values. This reduces the overhead of interrupt-driven
> operation. Upon the PENUP event, the worker thread re-enables the
> PENDOWN detection interrupt and exits.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> ---
> .../bindings/staging/iio/adc/mxs-lradc.txt | 6 +
> drivers/staging/iio/adc/mxs-lradc.c | 473 +++++++++++++++++++-
> 2 files changed, 455 insertions(+), 24 deletions(-)
>
> V2: - Replace the use_touchscreen* with enum mxs_lradc_ts, which now tracks
> touchscreen state (OFF, 4-wire, 5-wire)
> - Add "stop_touchscreen" bit, which indicates the touchscreen is going down
> and makes the driver not re-enable touchscreen interrupts and start any
> new touchscreen works.
> - Make all bitfields "unsigned int" instead of plain "int"
> - Use switch(plate) in mxs_lradc_ts_sample()
> - Cancel touchscreen thread in mxs_lradc_ts_close(), do this only after
> we are sure touchscreen interrupt is off and will never be re-enabled.
> This avoids serious race condition.
> - Call input_free_device() if input_register_device() fails to avoid memory
> leak.
> - Do not call input_free_device() after input_unregister_device() in
> mxs_lradc_ts_unregister() to avoid double-free
>
> V3: - Replace bitfields with bools
> - Make sure touchscreen workQ doesn't cause race condition upon closing
> the input device
> - Make touchscreen allocation failure fatal
> - Remove redundant check for lradc->ts_input in mxs_lradc_ts_unregister()
> - Drop __devinit and __devexit
>
> diff --git a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
> index 801d58c..4688205 100644
> --- a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
> +++ b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
> @@ -5,6 +5,12 @@ Required properties:
> - reg: Address and length of the register set for the device
> - interrupts: Should contain the LRADC interrupts
>
> +Optional properties:
> +- fsl,lradc-touchscreen-wires: Number of wires used to connect the touchscreen
> + to LRADC. Valid value is either 4 or 5. If this
> + property is not present, then the touchscreen is
> + disabled.
> +
> Examples:
>
> lradc at 80050000 {
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index 6249957..2db344b 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -32,6 +32,8 @@
> #include <linux/stmp_device.h>
> #include <linux/bitops.h>
> #include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/input.h>
>
> #include <mach/mxs.h>
> #include <mach/common.h>
> @@ -59,6 +61,21 @@
> #define LRADC_DELAY_TIMER_PER 200
> #define LRADC_DELAY_TIMER_LOOP 5
>
> +/*
> + * Once the pen touches the touchscreen, the touchscreen switches from
> + * IRQ-driven mode to polling mode to prevent interrupt storm. The polling
> + * is realized by worker thread, which is called every 20 or so milliseconds.
> + * This gives the touchscreen enough fluence and does not strain the system
> + * too much.
> + */
> +#define LRADC_TS_SAMPLE_DELAY_MS 5
> +
> +/*
> + * The LRADC reads the following amount of samples from each touchscreen
> + * channel and the driver then computes avarage of these.
> + */
> +#define LRADC_TS_SAMPLE_AMOUNT 4
> +
> static const char * const mxs_lradc_irq_name[] = {
> "mxs-lradc-touchscreen",
> "mxs-lradc-thresh0",
> @@ -75,6 +92,12 @@ static const char * const mxs_lradc_irq_name[] = {
> "mxs-lradc-button1",
> };
>
> +enum mxs_lradc_ts {
> + MXS_LRADC_TOUCHSCREEN_NONE = 0,
> + MXS_LRADC_TOUCHSCREEN_4WIRE,
> + MXS_LRADC_TOUCHSCREEN_5WIRE,
> +};
> +
> struct mxs_lradc {
> struct device *dev;
> void __iomem *base;
> @@ -86,21 +109,69 @@ struct mxs_lradc {
> struct mutex lock;
>
> struct completion completion;
> +
> + /*
> + * Touchscreen LRADC channels receives a private slot in the CTRL4
> + * register, the slot #7. Therefore only 7 slots instead of 8 in the
> + * CTRL4 register can be mapped to LRADC channels when using the
> + * touchscreen.
> + *
> + * Furthermore, certain LRADC channels are shared between touchscreen
> + * and/or touch-buttons and generic LRADC block. Therefore when using
> + * either of these, these channels are not available for the regular
> + * sampling. The shared channels are as follows:
> + *
> + * CH0 -- Touch button #0
> + * CH1 -- Touch button #1
> + * CH2 -- Touch screen XPUL
> + * CH3 -- Touch screen YPLL
> + * CH4 -- Touch screen XNUL
> + * CH5 -- Touch screen YNLR
> + * CH6 -- Touch screen WIPER (5-wire only)
> + *
> + * The bitfields below represents which parts of the LRADC block are
> + * switched into special mode of operation. These channels can not
> + * be sampled as regular LRADC channels. The driver will refuse any
> + * attempt to sample these channels.
> + */
> +#define CHAN_MASK_TOUCHBUTTON (0x3 << 0)
> +#define CHAN_MASK_TOUCHSCREEN_4WIRE (0xf << 2)
> +#define CHAN_MASK_TOUCHSCREEN_5WIRE (0x1f << 2)
> + enum mxs_lradc_ts use_touchscreen;
> + bool stop_touchscreen;
> + bool use_touchbutton;
> +
> + struct input_dev *ts_input;
> + struct work_struct ts_work;
> };
>
> #define LRADC_CTRL0 0x00
> -#define LRADC_CTRL0_TOUCH_DETECT_ENABLE (1 << 23)
> -#define LRADC_CTRL0_TOUCH_SCREEN_TYPE (1 << 22)
> +#define LRADC_CTRL0_TOUCH_DETECT_ENABLE (1 << 23)
> +#define LRADC_CTRL0_TOUCH_SCREEN_TYPE (1 << 22)
> +#define LRADC_CTRL0_YNNSW /* YM */ (1 << 21)
> +#define LRADC_CTRL0_YPNSW /* YP */ (1 << 20)
> +#define LRADC_CTRL0_YPPSW /* YP */ (1 << 19)
> +#define LRADC_CTRL0_XNNSW /* XM */ (1 << 18)
> +#define LRADC_CTRL0_XNPSW /* XM */ (1 << 17)
> +#define LRADC_CTRL0_XPPSW /* XP */ (1 << 16)
> +#define LRADC_CTRL0_PLATE_MASK (0x3f << 16)
>
> #define LRADC_CTRL1 0x10
> -#define LRADC_CTRL1_LRADC_IRQ(n) (1 << (n))
> -#define LRADC_CTRL1_LRADC_IRQ_MASK 0x1fff
> +#define LRADC_CTRL1_TOUCH_DETECT_IRQ_EN (1 << 24)
> #define LRADC_CTRL1_LRADC_IRQ_EN(n) (1 << ((n) + 16))
> #define LRADC_CTRL1_LRADC_IRQ_EN_MASK (0x1fff << 16)
> +#define LRADC_CTRL1_LRADC_IRQ_EN_OFFSET 16
> +#define LRADC_CTRL1_TOUCH_DETECT_IRQ (1 << 8)
> +#define LRADC_CTRL1_LRADC_IRQ(n) (1 << (n))
> +#define LRADC_CTRL1_LRADC_IRQ_MASK 0x1fff
> +#define LRADC_CTRL1_LRADC_IRQ_OFFSET 0
>
> #define LRADC_CTRL2 0x20
> #define LRADC_CTRL2_TEMPSENSE_PWD (1 << 15)
>
> +#define LRADC_STATUS 0x40
> +#define LRADC_STATUS_TOUCH_DETECT_RAW (1 << 0)
> +
> #define LRADC_CH(n) (0x50 + (0x10 * (n)))
> #define LRADC_CH_ACCUMULATE (1 << 29)
> #define LRADC_CH_NUM_SAMPLES_MASK (0x1f << 24)
> @@ -132,6 +203,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
> {
> struct mxs_lradc *lradc = iio_priv(iio_dev);
> int ret;
> + unsigned long mask;
>
> if (m != IIO_CHAN_INFO_RAW)
> return -EINVAL;
> @@ -140,6 +212,12 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
> if (chan->channel > LRADC_MAX_TOTAL_CHANS)
> return -EINVAL;
>
> + /* Validate the channel if it doesn't intersect with reserved chans. */
> + bitmap_set(&mask, chan->channel, 1);
> + ret = iio_validate_scan_mask_onehot(iio_dev, &mask);
> + if (ret)
> + return -EINVAL;
> +
> /*
> * See if there is no buffered operation in progess. If there is, simply
> * bail out. This can be improved to support both buffered and raw IO at
> @@ -161,7 +239,11 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
> lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>
> - writel(chan->channel, lradc->base + LRADC_CTRL4);
> + /* Clean the slot's previous content, then set new one. */
> + writel(LRADC_CTRL4_LRADCSELECT_MASK(0),
> + lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
> + writel(chan->channel, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
> +
> writel(0, lradc->base + LRADC_CH(0));
>
> /* Enable the IRQ and start sampling the channel. */
> @@ -195,6 +277,270 @@ static const struct iio_info mxs_lradc_iio_info = {
> };
>
> /*
> + * Touchscreen handling
> + */
> +enum lradc_ts_plate {
> + LRADC_SAMPLE_X,
> + LRADC_SAMPLE_Y,
> + LRADC_SAMPLE_PRESSURE,
> +};
> +
> +static int mxs_lradc_ts_touched(struct mxs_lradc *lradc)
> +{
> + uint32_t reg;
> +
> + /* Enable touch detection. */
> + writel(LRADC_CTRL0_PLATE_MASK,
> + lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> + writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> + lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +
> + msleep(LRADC_TS_SAMPLE_DELAY_MS);
> +
> + reg = readl(lradc->base + LRADC_STATUS);
> +
> + return reg & LRADC_STATUS_TOUCH_DETECT_RAW;
> +}
> +
> +static int32_t mxs_lradc_ts_sample(struct mxs_lradc *lradc,
> + enum lradc_ts_plate plate, int change)
> +{
> + unsigned long delay, jiff;
> + uint32_t reg, ctrl0 = 0, chan = 0;
> + /* The touchscreen always uses CTRL4 slot #7. */
> + const uint8_t slot = 7;
> + uint32_t val;
> +
> + /*
> + * There are three correct configurations of the controller sampling
> + * the touchscreen, each of these configuration provides different
> + * information from the touchscreen.
> + *
> + * The following table describes the sampling configurations:
> + * +-------------+-------+-------+-------+
> + * | Wire \ Axis | X | Y | Z |
> + * +---------------------+-------+-------+
> + * | X+ (CH2) | HI | TS | TS |
> + * +-------------+-------+-------+-------+
> + * | X- (CH4) | LO | SH | HI |
> + * +-------------+-------+-------+-------+
> + * | Y+ (CH3) | SH | HI | HI |
> + * +-------------+-------+-------+-------+
> + * | Y- (CH5) | TS | LO | SH |
> + * +-------------+-------+-------+-------+
> + *
> + * HI ... strong '1' ; LO ... strong '0'
> + * SH ... sample here ; TS ... tri-state
> + *
> + * There are a few other ways of obtaining the Z coordinate
> + * (aka. pressure), but the one in the table seems to be the
> + * most reliable one.
> + */
> + switch (plate) {
> + case LRADC_SAMPLE_X:
> + ctrl0 = LRADC_CTRL0_XPPSW | LRADC_CTRL0_XNNSW;
> + chan = 3;
> + break;
> + case LRADC_SAMPLE_Y:
> + ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_YNNSW;
> + chan = 4;
> + break;
> + case LRADC_SAMPLE_PRESSURE:
> + ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_XNNSW;
> + chan = 5;
> + break;
> + }
> +
> + if (change) {
> + writel(LRADC_CTRL0_PLATE_MASK,
> + lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> + writel(ctrl0, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +
> + writel(LRADC_CTRL4_LRADCSELECT_MASK(slot),
> + lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
> + writel(chan << LRADC_CTRL4_LRADCSELECT_OFFSET(slot),
> + lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
> + }
> +
> + writel(0xffffffff, lradc->base + LRADC_CH(slot) + STMP_OFFSET_REG_CLR);
> + writel(1 << slot, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +
> + delay = jiffies + msecs_to_jiffies(LRADC_TS_SAMPLE_DELAY_MS);
> + do {
> + jiff = jiffies;
> + reg = readl_relaxed(lradc->base + LRADC_CTRL1);
> + if (reg & LRADC_CTRL1_LRADC_IRQ(slot))
> + break;
> + } while (time_before(jiff, delay));
> +
> + writel(LRADC_CTRL1_LRADC_IRQ(slot),
> + lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> +
> + if (time_after_eq(jiff, delay))
> + return -ETIMEDOUT;
> +
> + val = readl(lradc->base + LRADC_CH(slot));
> + val &= LRADC_CH_VALUE_MASK;
> +
> + return val;
> +}
> +
> +static int32_t mxs_lradc_ts_sample_filter(struct mxs_lradc *lradc,
> + enum lradc_ts_plate plate)
> +{
> + int32_t val, tot = 0;
> + int i;
> +
> + val = mxs_lradc_ts_sample(lradc, plate, 1);
> +
> + /* Delay a bit so the touchscreen is stable. */
> + mdelay(2);
> +
> + for (i = 0; i < LRADC_TS_SAMPLE_AMOUNT; i++) {
> + val = mxs_lradc_ts_sample(lradc, plate, 0);
> + tot += val;
> + }
> +
> + return tot / LRADC_TS_SAMPLE_AMOUNT;
> +}
> +
> +static void mxs_lradc_ts_work(struct work_struct *ts_work)
> +{
> + struct mxs_lradc *lradc = container_of(ts_work,
> + struct mxs_lradc, ts_work);
> + int val_x, val_y, val_p;
> + bool valid = false;
> +
> + while (mxs_lradc_ts_touched(lradc)) {
> + /* Disable touch detector so we can sample the touchscreen. */
> + writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> + lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +
> + if (likely(valid)) {
> + input_report_abs(lradc->ts_input, ABS_X, val_x);
> + input_report_abs(lradc->ts_input, ABS_Y, val_y);
> + input_report_abs(lradc->ts_input, ABS_PRESSURE, val_p);
> + input_report_key(lradc->ts_input, BTN_TOUCH, 1);
> + input_sync(lradc->ts_input);
> + }
> +
> + valid = false;
> +
> + val_x = mxs_lradc_ts_sample_filter(lradc, LRADC_SAMPLE_X);
> + if (val_x < 0)
> + continue;
> + val_y = mxs_lradc_ts_sample_filter(lradc, LRADC_SAMPLE_Y);
> + if (val_y < 0)
> + continue;
> + val_p = mxs_lradc_ts_sample_filter(lradc, LRADC_SAMPLE_PRESSURE);
> + if (val_p < 0)
> + continue;
> +
> + valid = true;
> + }
> +
> + input_report_abs(lradc->ts_input, ABS_PRESSURE, 0);
> + input_report_key(lradc->ts_input, BTN_TOUCH, 0);
> + input_sync(lradc->ts_input);
> +
> + /* Do not restart the TS IRQ if the driver is shutting down. */
> + if (lradc->stop_touchscreen)
> + return;
> +
> + /* Restart the touchscreen interrupts. */
> + writel(LRADC_CTRL1_TOUCH_DETECT_IRQ,
> + lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> + writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> + lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
> +}
> +
> +static int mxs_lradc_ts_open(struct input_dev *dev)
> +{
> + struct mxs_lradc *lradc = input_get_drvdata(dev);
> +
> + /* The touchscreen is starting. */
> + lradc->stop_touchscreen = false;
> +
> + /* Enable the touch-detect circuitry. */
> + writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> + lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +
> + /* Enable the touch-detect IRQ. */
> + writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> + lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
> +
> + return 0;
> +}
> +
> +static void mxs_lradc_ts_close(struct input_dev *dev)
> +{
> + struct mxs_lradc *lradc = input_get_drvdata(dev);
> +
> + /* Indicate the touchscreen is stopping. */
> + lradc->stop_touchscreen = true;
> + mb();
> +
> + /* Wait until touchscreen thread finishes any possible remnants. */
> + cancel_work_sync(&lradc->ts_work);
> +
> + /* Disable touchscreen touch-detect IRQ. */
> + writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> + lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> +
> + /* Power-down touchscreen touch-detect circuitry. */
> + writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> + lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +}
> +
> +static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
> +{
> + struct input_dev *input;
> + struct device *dev = lradc->dev;
> + int ret;
> +
> + if (!lradc->use_touchscreen)
> + return 0;
> +
> + input = input_allocate_device();
> + if (!input) {
> + dev_err(dev, "Failed to allocate TS device!\n");
> + lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_NONE;
> + return -ENOMEM;
Since errors are now fatal you do not need to modify "use_touchscreen".
> + }
> +
> + input->name = DRIVER_NAME;
> + input->id.bustype = BUS_HOST;
> + input->dev.parent = dev;
> + input->open = mxs_lradc_ts_open;
> + input->close = mxs_lradc_ts_close;
> +
> + __set_bit(EV_ABS, input->evbit);
> + __set_bit(EV_KEY, input->evbit);
> + __set_bit(BTN_TOUCH, input->keybit);
> + input_set_abs_params(input, ABS_X, 0, LRADC_CH_VALUE_MASK, 0, 0);
> + input_set_abs_params(input, ABS_Y, 0, LRADC_CH_VALUE_MASK, 0, 0);
> + input_set_abs_params(input, ABS_PRESSURE, 0, LRADC_CH_VALUE_MASK, 0, 0);
> +
> + lradc->ts_input = input;
> + input_set_drvdata(input, lradc);
> + ret = input_register_device(input);
> + if (ret)
> + input_free_device(lradc->ts_input);
> +
> + return ret;
> +}
> +
> +static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> +{
> + if (!lradc->use_touchscreen)
> + return;
> +
> + cancel_work_sync(&lradc->ts_work);
> +
> + input_unregister_device(lradc->ts_input);
> +}
> +
> +/*
> * IRQ Handling
> */
> static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
> @@ -202,14 +548,24 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
> struct iio_dev *iio = data;
> struct mxs_lradc *lradc = iio_priv(iio);
> unsigned long reg = readl(lradc->base + LRADC_CTRL1);
> + const uint32_t ts_irq_mask =
> + LRADC_CTRL1_TOUCH_DETECT_IRQ_EN |
> + LRADC_CTRL1_TOUCH_DETECT_IRQ;
>
> if (!(reg & LRADC_CTRL1_LRADC_IRQ_MASK))
> return IRQ_NONE;
>
> /*
> - * Touchscreen IRQ handling code shall probably have priority
> - * and therefore shall be placed here.
> + * Touchscreen IRQ handling code has priority and therefore
> + * is placed here. In case touchscreen IRQ arrives, disable
> + * it ASAP
> */
> + if (reg & LRADC_CTRL1_TOUCH_DETECT_IRQ) {
> + writel(ts_irq_mask,
> + lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> + if (!lradc->stop_touchscreen)
> + schedule_work(&lradc->ts_work);
> + }
>
> if (iio_buffer_enabled(iio))
> iio_trigger_poll(iio->trig, iio_get_time_ns());
> @@ -306,8 +662,10 @@ static int mxs_lradc_buffer_preenable(struct iio_dev *iio)
> {
> struct mxs_lradc *lradc = iio_priv(iio);
> struct iio_buffer *buffer = iio->buffer;
> - int ret = 0, chan, ofs = 0, enable = 0;
> - uint32_t ctrl4 = 0;
> + int ret = 0, chan, ofs = 0;
> + unsigned long enable = 0;
> + uint32_t ctrl4_set = 0;
> + uint32_t ctrl4_clr = 0;
> uint32_t ctrl1_irq = 0;
> const uint32_t chan_value = LRADC_CH_ACCUMULATE |
> ((LRADC_DELAY_TIMER_LOOP - 1) << LRADC_CH_NUM_SAMPLES_OFFSET);
> @@ -339,17 +697,20 @@ static int mxs_lradc_buffer_preenable(struct iio_dev *iio)
> writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>
> for_each_set_bit(chan, buffer->scan_mask, LRADC_MAX_TOTAL_CHANS) {
> - ctrl4 |= chan << LRADC_CTRL4_LRADCSELECT_OFFSET(ofs);
> + ctrl4_set |= chan << LRADC_CTRL4_LRADCSELECT_OFFSET(ofs);
> + ctrl4_clr |= LRADC_CTRL4_LRADCSELECT_MASK(ofs);
> ctrl1_irq |= LRADC_CTRL1_LRADC_IRQ_EN(ofs);
> writel(chan_value, lradc->base + LRADC_CH(ofs));
> - enable |= 1 << ofs;
> + bitmap_set(&enable, ofs, 1);
> ofs++;
> };
>
> writel(LRADC_DELAY_TRIGGER_LRADCS_MASK | LRADC_DELAY_KICK,
> lradc->base + LRADC_DELAY(0) + STMP_OFFSET_REG_CLR);
>
> - writel(ctrl4, lradc->base + LRADC_CTRL4);
> + writel(ctrl4_clr, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
> + writel(ctrl4_set, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
> +
> writel(ctrl1_irq, lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
>
> writel(enable << LRADC_DELAY_TRIGGER_LRADCS_OFFSET,
> @@ -384,9 +745,33 @@ static int mxs_lradc_buffer_postdisable(struct iio_dev *iio)
> static bool mxs_lradc_validate_scan_mask(struct iio_dev *iio,
> const unsigned long *mask)
> {
> - const int mw = bitmap_weight(mask, iio->masklength);
> -
> - return mw <= LRADC_MAX_MAPPED_CHANS;
> + struct mxs_lradc *lradc = iio_priv(iio);
> + const int len = iio->masklength;
> + const int map_chans = bitmap_weight(mask, len);
> + int rsvd_chans = 0;
> + unsigned long rsvd_mask = 0;
> +
> + if (lradc->use_touchbutton)
> + rsvd_mask |= CHAN_MASK_TOUCHBUTTON;
> + if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_4WIRE)
> + rsvd_mask |= CHAN_MASK_TOUCHSCREEN_4WIRE;
> + if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE)
> + rsvd_mask |= CHAN_MASK_TOUCHSCREEN_5WIRE;
> +
> + if (lradc->use_touchbutton)
> + rsvd_chans++;
> + if (lradc->use_touchscreen)
> + rsvd_chans++;
> +
> + /* Test for attempts to map channels with special mode of operation. */
> + if (bitmap_intersects(mask, &rsvd_mask, len))
> + return 0;
Since the fucntion's return type is bool it is better to do
return false;
> +
> + /* Test for attempts to map more channels then available slots. */
> + if (map_chans + rsvd_chans > LRADC_MAX_MAPPED_CHANS)
> + return 0;
return false;
> +
> + return 1;
return true;
> }
>
> static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = {
> @@ -435,15 +820,29 @@ static const struct iio_chan_spec mxs_lradc_chan_spec[] = {
>
> static void mxs_lradc_hw_init(struct mxs_lradc *lradc)
> {
> - int i;
> - const uint32_t cfg =
> + /* The ADC always uses DELAY CHANNEL 0. */
> + const uint32_t adc_cfg =
> + (1 << (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + 0)) |
> (LRADC_DELAY_TIMER_PER << LRADC_DELAY_DELAY_OFFSET);
>
> stmp_reset_block(lradc->base);
>
> - for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
> - writel(cfg | (1 << (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + i)),
> - lradc->base + LRADC_DELAY(i));
> + /* Configure DELAY CHANNEL 0 for generic ADC sampling. */
> + writel(adc_cfg, lradc->base + LRADC_DELAY(0));
> +
> + /* Disable remaining DELAY CHANNELs */
> + writel(0, lradc->base + LRADC_DELAY(1));
> + writel(0, lradc->base + LRADC_DELAY(2));
> + writel(0, lradc->base + LRADC_DELAY(3));
> +
> + /* Configure the touchscreen type */
> + writel(LRADC_CTRL0_TOUCH_SCREEN_TYPE,
> + lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +
> + if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE) {
> + writel(LRADC_CTRL0_TOUCH_SCREEN_TYPE,
> + lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> + }
>
> /* Start internal temperature sensing. */
> writel(0, lradc->base + LRADC_CTRL2);
> @@ -460,12 +859,14 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)
> writel(0, lradc->base + LRADC_DELAY(i));
> }
>
> -static int __devinit mxs_lradc_probe(struct platform_device *pdev)
> +static int mxs_lradc_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> + struct device_node *node = dev->of_node;
> struct mxs_lradc *lradc;
> struct iio_dev *iio;
> struct resource *iores;
> + uint32_t ts_wires = 0;
> int ret = 0;
> int i;
>
> @@ -487,6 +888,21 @@ static int __devinit mxs_lradc_probe(struct platform_device *pdev)
> goto err_addr;
> }
>
> + INIT_WORK(&lradc->ts_work, mxs_lradc_ts_work);
> +
> + /* Check if touchscreen is enabled in DT. */
> + ret = of_property_read_u32(node, "fsl,lradc-touchscreen-wires",
> + &ts_wires);
> + if (ret)
> + dev_info(dev, "Touchscreen not enabled.\n");
> + else if (ts_wires == 4)
> + lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_4WIRE;
> + else if (ts_wires == 5)
> + lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_5WIRE;
> + else
> + dev_warn(dev, "Unsupported number of touchscreen wires (%d)\n",
> + ts_wires);
> +
> /* Grab all IRQ sources */
> for (i = 0; i < 13; i++) {
> lradc->irq[i] = platform_get_irq(pdev, i);
> @@ -524,11 +940,16 @@ static int __devinit mxs_lradc_probe(struct platform_device *pdev)
> if (ret)
> goto err_trig;
>
> + /* Register the touchscreen input device. */
> + ret = mxs_lradc_ts_register(lradc);
> + if (ret)
> + goto err_dev;
> +
> /* Register IIO device. */
> ret = iio_device_register(iio);
> if (ret) {
> dev_err(dev, "Failed to register IIO device\n");
> - goto err_dev;
> + goto err_ts;
> }
>
> /* Configure the hardware. */
> @@ -536,6 +957,8 @@ static int __devinit mxs_lradc_probe(struct platform_device *pdev)
>
> return 0;
>
> +err_ts:
> + mxs_lradc_ts_unregister(lradc);
> err_dev:
> mxs_lradc_trigger_remove(iio);
> err_trig:
> @@ -545,11 +968,13 @@ err_addr:
> return ret;
> }
>
> -static int __devexit mxs_lradc_remove(struct platform_device *pdev)
> +static int mxs_lradc_remove(struct platform_device *pdev)
> {
> struct iio_dev *iio = platform_get_drvdata(pdev);
> struct mxs_lradc *lradc = iio_priv(iio);
>
> + mxs_lradc_ts_unregister(lradc);
> +
> mxs_lradc_hw_stop(lradc);
>
> iio_device_unregister(iio);
> @@ -573,7 +998,7 @@ static struct platform_driver mxs_lradc_driver = {
> .of_match_table = mxs_lradc_dt_ids,
> },
> .probe = mxs_lradc_probe,
> - .remove = __devexit_p(mxs_lradc_remove),
> + .remove = mxs_lradc_remove,
> };
>
> module_platform_driver(mxs_lradc_driver);
> --
> 1.7.10.4
>
Besides the minor nits above,
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Thanks!
--
Dmitry
^ permalink raw reply
* [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access
From: Catalin Marinas @ 2013-01-11 16:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F01646.4010203@ti.com>
On Fri, Jan 11, 2013 at 01:40:22PM +0000, Santosh Shilimkar wrote:
> On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> > Several bits in CNTKCTL reset to 0, including PL0VTEN. For platforms
> > using the generic timer which wish to have a fast gettimeofday vDSO
> > implementation, these bits must be set to 1 by the kernel. On other
> > platforms, the bootloader might enable userspace access when we don't
> > want it.
> >
> > This patch adds arch_counter_set_user_access, which sets the PL0 access
> > permissions to that required by the platform. For arm, this currently
> minor nit.
> s/arm/ARM
I think Mark meant arch/arm. Or we could call it AArch32 where we don't
use this feature.
--
Catalin
^ permalink raw reply
* [PATCHv2 07/11] arm: arch_timer: divorce from local_timer api
From: Catalin Marinas @ 2013-01-11 16:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F014DF.7090205@ti.com>
On Fri, Jan 11, 2013 at 01:34:23PM +0000, Santosh Shilimkar wrote:
> On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> > Currently, the arch_timer driver is tied to the arm port, as it relies
> > on code in arch/arm/smp.c to setup and teardown timers as cores are
> > hotplugged on and off. The timer is registered through an arm-specific
> > registration mechanism, preventing sharing the driver with the arm64
> > port.
> >
> > This patch moves the driver to using a cpu notifier instead, making it
> > easier to port.
> >
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> This is really a nit idea. I think we should do the same
> for ARM gic code.
I plan to do the same once Rob's GIC patches get merged. In my
soc-armv8-model branch I have a copy of gic.c into drivers/irqchip and
the CPU interface is done automatically via a notifier. The only trick
is to set the priority of the GIC notifier higher than the timer one.
--
Catalin
^ permalink raw reply
* Early kernel hang with big DTB appended
From: Sascha Hauer @ 2013-01-11 16:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7811936.npteFbuYu2@amdc1227>
On Thu, Jan 03, 2013 at 04:55:00PM +0100, Tomasz Figa wrote:
> Hi,
>
> I'm observing strange behavior when booting 3.8-rc1 and -rc2 with appended
> DTB. The kernel hangs very early when the DTB is bigger than some
> threshold somewhere around 24 KiB. With fullest possible low level UART
> debugging (and printk patched to use printascii) I'm receiving following
> output:
>
> Uncompressing Linux... done, booting the kernel.
> Booting Linux on physical CPU 0xa00
> Linux version 3.8.0-rc1-00073-gdf6efca-dirty (t.figa at amdc1227) (gcc
> version 4.5.2 (Gentoo 4.5.2 p1.2, pie-0.4.5) ) #2 SMP PREEMPT Thu Jan 3
> 15:37:35 CET 2013
> CPU: ARMv7 Processor [413fc090] revision 0 (ARMv7), cr=10c53c7d
> CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
>
> I tested on two Exynos-based boards (exynos4210-trats and one internal
> exynos4412-based board) and same happens on both.
>
> Do you have any ideas?
Another thing besides the things already mentioned is that the dtb may
not cross a 1MiB boundary. The Kernel uses a single 1Mib section
(aligned to 1Mib) to initially map the dtb. Once you cross that boundary
parts of the dtb won't be accessible for the Kernel anymore.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCH 6/6] ARM: virt: hide CONFIG_ARM_VIRT_EXT from user
From: Will Deacon @ 2013-01-11 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F032F0.3070906@codeaurora.org>
On Fri, Jan 11, 2013 at 03:42:40PM +0000, Christopher Covington wrote:
> Hi Will,
Hello,
> On 01/11/2013 10:34 AM, Will Deacon wrote:
> > ARM_VIRT_EXT is a property of CPU_V7, but does not adversely affect
> > other CPUs that can be built into the same kernel image (i.e. ARMv6+).
> >
> > This patch defaults ARM_VIRT_EXT to y if CPU_V7, allowing hypervisors
> > such as KVM to make better use of the option and being able to rely
> > on hyp-mode boot support.
>
> [...]
>
> > @@ -640,11 +641,6 @@ config ARM_VIRT_EXT
> > use of this feature. Refer to Documentation/arm/Booting for
> > details.
> >
> > - It is safe to enable this option even if the kernel may not be
> > - booted in HYP mode, may not have support for the
> > - virtualization extensions, or may be booted with a
> > - non-compliant bootloader.
>
> Why take this out?
I just removed that last paragraph because it's not user-selectable anymore,
so this comment about enabling the option is redundant. I could remove all
of the help text, but we have it for some other options that are selected
automatically and it has a pointer to some documentation too.
Will
^ permalink raw reply
* [PATCH] ARM: dts: OMAP3: Add support for OMAP3430 SDP board
From: Jon Hunter @ 2013-01-11 16:39 UTC (permalink / raw)
To: linux-arm-kernel
Adds basic device-tree support for OMAP3430 SDP board which has 256MB
of RAM and uses the TWL4030 power management IC.
Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/omap3430-sdp.dts | 46 ++++++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
create mode 100644 arch/arm/boot/dts/omap3430-sdp.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index e44da40..5d6dff0 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -104,6 +104,7 @@ dtb-$(CONFIG_ARCH_OMAP2PLUS) += omap2420-h4.dtb \
omap3-beagle.dtb \
omap3-beagle-xm.dtb \
omap3-evm.dtb \
+ omap3430-sdp.dtb \
omap3-tobi.dtb \
omap4-panda.dtb \
omap4-panda-a4.dtb \
diff --git a/arch/arm/boot/dts/omap3430-sdp.dts b/arch/arm/boot/dts/omap3430-sdp.dts
new file mode 100644
index 0000000..be0650d
--- /dev/null
+++ b/arch/arm/boot/dts/omap3430-sdp.dts
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+/dts-v1/;
+
+/include/ "omap3.dtsi"
+
+/ {
+ model = "TI OMAP3430 SDP";
+ compatible = "ti,omap3430-sdp", "ti,omap3";
+
+ memory {
+ device_type = "memory";
+ reg = <0x80000000 0x10000000>; /* 256 MB */
+ };
+};
+
+&i2c1 {
+ clock-frequency = <2600000>;
+
+ twl: twl at 48 {
+ reg = <0x48>;
+ interrupts = <7>; /* SYS_NIRQ cascaded to intc */
+ interrupt-parent = <&intc>;
+ };
+};
+
+/include/ "twl4030.dtsi"
+
+&mmc1 {
+ vmmc-supply = <&vmmc1>;
+ vmmc_aux-supply = <&vsim>;
+ bus-width = <8>;
+};
+
+&mmc2 {
+ status = "disabled";
+};
+
+&mmc3 {
+ status = "disabled";
+};
--
1.7.10.4
^ permalink raw reply related
* [PATCH 07/14] usb: ehci-omap: Instantiate PHY devices if required
From: Russell King - ARM Linux @ 2013-01-11 16:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F037C9.40306@ti.com>
On Fri, Jan 11, 2013 at 06:03:21PM +0200, Roger Quadros wrote:
> diff --git a/include/linux/platform_data/usb-omap.h
> b/include/linux/platform_data/usb-omap.h
> index d63eb7d..927b8a1 100644
> --- a/include/linux/platform_data/usb-omap.h
> +++ b/include/linux/platform_data/usb-omap.h
> @@ -38,6 +38,12 @@ enum usbhs_omap_port_mode {
> OMAP_OHCI_PORT_MODE_TLL_2PIN_DPDM
> };
>
> +struct usbhs_phy_config {
> + char *name; /* binds to device driver */
You may wish to consider making this const.
^ permalink raw reply
* [PATCH] ARM: kernel: DT cpu map validity check helper function
From: Russell King - ARM Linux @ 2013-01-11 16:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357921058-11322-1-git-send-email-lorenzo.pieralisi@arm.com>
On Fri, Jan 11, 2013 at 04:17:38PM +0000, Lorenzo Pieralisi wrote:
> This patch implements a helper function that platforms can call to check
> whether DT based cpu map initialization and cores count were completed
> successfully.
Umm, are you sure this works? Two problems here:
- the kernel boot marks the booting CPU (in our case, CPU0) as present,
possible and online before arch code gets called. smp_init_cpus()
will be called with the maps already initialized per that.
- this really needs to be paired with a patch showing how you intend it
to be used; merely adding the helper without any sign of it being used
means it's just bloat which may not end up being used.
^ permalink raw reply
* [PATCH] ARM: let CPUs not being able to run in ARM mode enter in THUMB mode
From: Uwe Kleine-König @ 2013-01-11 16:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111160753.GC23505@n2100.arm.linux.org.uk>
Hi Russell,
On Fri, Jan 11, 2013 at 04:07:53PM +0000, Russell King - ARM Linux wrote:
> On Fri, Jan 11, 2013 at 12:39:57PM +0100, Uwe Kleine-K?nig wrote:
> > +# Select this if your CPU doesn't support the 32 bit ARM instructions.
> > +config THUMBONLY_CPU
> > + bool
> > + select THUMB2_KERNEL
> > + select ARM_THUMB
>
> Hmm, not convinced this is the best solution. Yes, fine for there to be
> a THUMBONLY_CPU option, _but_ not the select statements onto user visible
> symbols. We can get this instead by:
>
> config THUMB2_KERNEL
> bool "Compile the kernel in Thumb-2 mode" if !THUMBONLY_CPU
> depends on (CPU_V7 && !CPU_V6 && !CPU_V6K) || THUMBONLY_CPU
> default y if THUMBONLY_CPU
> select AEABI
> select ARM_ASM_UNIFIED
> select ARM_UNWIND
>
> and:
>
> config ARM_THUMB
> bool "Support Thumb user binaries" if !THUMBONLY_CPU
> depends on CPU_ARM720T || CPU_ARM740T || CPU_ARM920T || \
> CPU_ARM922T || CPU_ARM925T || CPU_ARM926T || \
> CPU_ARM940T || CPU_ARM946E || CPU_ARM1020 || \
> CPU_ARM1020E || CPU_ARM1022 || CPU_ARM1026 || \
> CPU_XSCALE || CPU_XSC3 || CPU_MOHAWK || CPU_V6 || \
> CPU_V6K || CPU_V7 || CPU_FEROCEON || THUMBONLY_CPU
> default y
>
> And... I'm left wondering - should we have this instead:
>
> config CPU_ARM
> bool
>
> config CPU_THUMB
> bool
>
> which indicates whether the CPU supports the ARM instruction set or the
> Thumb instruction set (or both) - that should then allow us to select
> those from the individual CPU_xxx options and eliminate that big long
> list of dependencies against ARM_THUMB.
I like your idea and I will come up with a patch.
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH] ARM: kernel: DT cpu map validity check helper function
From: Lorenzo Pieralisi @ 2013-01-11 16:17 UTC (permalink / raw)
To: linux-arm-kernel
Since the introduction of /cpu nodes bindings for ARM and the
corresponding parse function arm_dt_init_cpu_maps(), the cpu_logical_map
and the number of possible CPUs are set according to the DT /cpu
nodes entries. Currently most of the existing ARM SMP platforms detect the
number of cores through HW probing in their .smp_init_cpus functions and set
the possible CPU mask accordingly.
This method should be upgraded so that the CPU counting mechanism will be
based on DT, keeping legacy HW probing mechanism as a fall back solution.
In order to implement this fall back solution mechanism, the ARM DT code
should provide a helper function to platforms to check if the cpu map
has been properly initialized through DT. If the check fails the
platform will resort to legacy HW based cores counting mechanism.
This patch implements a helper function that platforms can call to check
whether DT based cpu map initialization and cores count were completed
successfully.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
arch/arm/include/asm/prom.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h
index a219227..a913989 100644
--- a/arch/arm/include/asm/prom.h
+++ b/arch/arm/include/asm/prom.h
@@ -13,11 +13,21 @@
#define HAVE_ARCH_DEVTREE_FIXUPS
+#include <linux/cpumask.h>
+
#ifdef CONFIG_OF
extern struct machine_desc *setup_machine_fdt(unsigned int dt_phys);
extern void arm_dt_memblock_reserve(void);
extern void __init arm_dt_init_cpu_maps(void);
+/*
+ * Return true if cpu map initialization has been
+ * carried out correctly from DT
+ */
+static inline bool __init arm_dt_cpu_map_valid(void)
+{
+ return !!(num_possible_cpus());
+}
#else /* CONFIG_OF */
@@ -28,6 +38,7 @@ static inline struct machine_desc *setup_machine_fdt(unsigned int dt_phys)
static inline void arm_dt_memblock_reserve(void) { }
static inline void arm_dt_init_cpu_maps(void) { }
+static inline bool __init arm_dt_cpu_map_valid(void) { return false; }
#endif /* CONFIG_OF */
#endif /* ASMARM_PROM_H */
--
1.7.12
^ permalink raw reply related
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