Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: setup: report non-optional CPU features
From: Alexander Van Brunt @ 2014-01-28 17:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140128172253.GD24617@arm.com>

There are a large number of Android applications that read "/proc/cpuinfo" to determine what CPU features are available. These applications are delivered as binaries and cannot be easily recompiled. Another example of code is the Android NDK framework. It will separately read "/proc/cpuinfo" and parse the "Features" list to get the list of CPU features.

Are there other architectures that report different information in "/proc/cpuinfo" depending on the personality? x86 doesn't. The problem I have with making this conditional on the personality is that CPU's "Features" list should be listing the features of the CPU not the features of the kernel mode. For example, the CPU does have the "thumb" feature even if the personality doesn't use it.

-----Original Message-----
From: Catalin Marinas [mailto:catalin.marinas at arm.com] 
Sent: Tuesday, January 28, 2014 9:23 AM
To: Alexander Van Brunt
Cc: Will Deacon; Linux ARM Kernel
Subject: Re: [PATCH] arm64: setup: report non-optional CPU features

On Tue, Jan 28, 2014 at 01:23:41AM +0000, Alex Van Brunt wrote:
> There are a large number of popular applications compiled for ARMv7-A 
> that read /proc/cpuinfo to find out what features the CPU has.

ELF_HWCAP should work fine. Which popular applications are these?

> But, when they are run
> on an arm64 kernel, they fail to run. This is because features that 
> were optional on ARMv7 or earlier but are not optional on ARMv8-A like 
> Thumb are not listed as a CPU feature using the arm64 kernel. To make 
> those applications run, the kernel still needs to print the features in the list.
> 
> This patch changes "cat /proc/cpuinfo" from printing:
> 
> Features        : fp asimd
> 
> To printing:
> 
> Features        : fp asimd wp half thumb fastmult vfp edsp neon vfpv3d16 tlsi vfpv4 idiva idivt

That's not correct, the features reported are for the AArch64 mode, it doesn't make sense to overlap the AArch32 features here. There is COMPAT_ELF_HWCAP and the bits should be passed correctly to AArch32 binaries.

A solution would be to add a check for personality(current->personality) == PER_LINUX32 and report cpuinfo in an AArch32 compatible way and based on COMPAT_ELF_HWCAP.

--
Catalin

^ permalink raw reply

* [Xen-devel] [PATCH] arm/xen: Initialize event channels earlier
From: Stefano Stabellini @ 2014-01-28 17:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52E7EA78.5020305@linaro.org>

On Tue, 28 Jan 2014, Julien Grall wrote:
> >> +static int xen_cpu_notification(struct notifier_block *self,
> >> +				unsigned long action,
> >> +				void *hcpu)
> >> +{
> >> +	int cpu = (long)hcpu;
> >> +
> >> +	switch (action) {
> >> +	case CPU_UP_PREPARE:
> >> +		xen_percpu_init(cpu);
> >> +		break;
> >> +	case CPU_STARTING:
> >> +		xen_interrupt_init();
> >> +		break;
> > 
> > Is CPU_STARTING guaranteed to be called on the new cpu only?
> 
> Yes.
> 
> > If so, why not call both xen_percpu_init and xen_interrupt_init on
> > CPU_STARTING?
> 
> Just in case that xen_vcpu is used somewhere else by a cpu notifier
> callback CPU_STARTING. We don't know which callback is called first.

Could you please elaborate a bit more on the problem you are trying to
describe?


> > As it stands I think you introduced a subtle change (that might be OK
> > but I think is unintentional): xen_percpu_init might not be called from
> > the same cpu as its target anymore.
> 
> No, xen_percpu_init and xen_interrupt_init are called on the boot cpu at
> the end of xen_guest_init.
 
Is CPU_UP_PREPARE guaranteed to be called on the target cpu? I think
not, therefore you would be executing xen_percpu_init for cpu1 on cpu0.

^ permalink raw reply

* PCIe trouble on imx6q
From: Bjorn Helgaas @ 2014-01-28 17:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAM9uW_75p7rt4V82TysSKmVbsYYgWqYwgaecyHMa+q4kSRwzUw@mail.gmail.com>

[+cc Richard, Shawn, linux-arm-kernel (all from MAINTAINERS)]

On Tue, Jan 28, 2014 at 1:02 AM, Kamel BOUHARA <k.bouhara@gmail.com> wrote:
> Hello,
>
> Im getting trouble with kernel 3.13 at boot time, the pcie link failed
> to get up with the following log:
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at drivers/gpio/gpiolib.c:159 gpio_to_desc+0x34/0x48()
> invalid GPIO -2
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.13.0+ #4
> Backtrace:
> [<8001217c>] (dump_backtrace) from [<80012460>] (show_stack+0x18/0x1c)
>  r6:802b9548 r5:00000000 r4:808d3060 r3:00000000
> [<80012448>] (show_stack) from [<806414fc>] (dump_stack+0x84/0x9c)
> [<80641478>] (dump_stack) from [<800289f8>] (warn_slowpath_common+0x70/0x94)
>  r5:00000009 r4:bf05bcb0
> [<80028988>] (warn_slowpath_common) from [<80028a54>]
> (warn_slowpath_fmt+0x38/0x40)
>  r8:01f00000 r7:00000000 r6:0011cc11 r5:808b68c0 r4:bf24fa30
> [<80028a20>] (warn_slowpath_fmt) from [<802b9548>] (gpio_to_desc+0x34/0x48)
>  r3:fffffffe r2:807d23fc
> [<802b9514>] (gpio_to_desc) from [<802d9de0>] (imx6_pcie_host_init+0x174/0x434)
> [<802d9c6c>] (imx6_pcie_host_init) from [<80886dbc>]
> (dw_pcie_host_init+0x348/0x41c)
>  r6:00000000 r5:808d52cc r4:00000020 r3:802d9c6c
> [<80886a74>] (dw_pcie_host_init) from [<808871d4>] (imx6_pcie_probe+0x320/0x3dc)
>  r10:00000000 r9:000000c4 r8:808d539c r7:bf7e3384 r6:bf24fa30 r5:bf135810
>  r4:bf24fa10
> [<80886eb4>] (imx6_pcie_probe) from [<8034b670>] (platform_drv_probe+0x20/0x50)
>  r8:808d539c r7:00000000 r6:00000000 r5:808d539c r4:bf135810
> [<8034b650>] (platform_drv_probe) from [<80349c74>]
> (driver_probe_device+0x118/0x234)
>  r5:bf135810 r4:80e526b8
> [<80349b5c>] (driver_probe_device) from [<80349e78>] (__driver_attach+0x9c/0xa0)
>  r8:80886e90 r7:00000000 r6:bf135844 r5:808d539c r4:bf135810 r3:00000000
> [<80349ddc>] (__driver_attach) from [<8034806c>] (bus_for_each_dev+0x68/0x9c)
>  r6:80349ddc r5:808d539c r4:00000000 r3:00000000
> [<80348004>] (bus_for_each_dev) from [<8034972c>] (driver_attach+0x20/0x28)
>  r6:808df6a8 r5:bf1f5e00 r4:808d539c
> [<8034970c>] (driver_attach) from [<803493b0>] (bus_add_driver+0x148/0x1f4)
> [<80349268>] (bus_add_driver) from [<8034a4c8>] (driver_register+0x80/0x100)
>  r7:8090e640 r6:8090e640 r5:00000005 r4:808d539c
> [<8034a448>] (driver_register) from [<8034b63c>]
> (__platform_driver_register+0x50/0x64)
>  r5:00000005 r4:808d5388
> [<8034b5ec>] (__platform_driver_register) from [<8034b6e0>]
> (platform_driver_probe+0x28/0xac)
> [<8034b6b8>] (platform_driver_probe) from [<80886ea8>]
> (imx6_pcie_init+0x18/0x24)
>  r5:00000005 r4:808aa104
> [<80886e90>] (imx6_pcie_init) from [<80008978>] (do_one_initcall+0x100/0x164)
> [<80008878>] (do_one_initcall) from [<8085ecc0>]
> (kernel_init_freeable+0x10c/0x1d0)
>  r10:8089e060 r9:000000c4 r8:8089e050 r7:8090e640 r6:8090e640 r5:00000005
>  r4:808aa104
> [<8085ebb4>] (kernel_init_freeable) from [<8063b67c>] (kernel_init+0x10/0x120)
>  r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:8063b66c
>  r4:00000000
> [<8063b66c>] (kernel_init) from [<8000e9c8>] (ret_from_fork+0x14/0x2c)
>  r4:00000000 r3:ffffffff
> ---[ end trace b5e746dfc2398cd6 ]---
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at drivers/gpio/gpiolib.c:159 gpio_to_desc+0x34/0x48()
> invalid GPIO -2
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W    3.13.0+ #4
> Backtrace:
> [<8001217c>] (dump_backtrace) from [<80012460>] (show_stack+0x18/0x1c)
>  r6:802b9548 r5:00000000 r4:808d3060 r3:00000000
> [<80012448>] (show_stack) from [<806414fc>] (dump_stack+0x84/0x9c)
> [<80641478>] (dump_stack) from [<800289f8>] (warn_slowpath_common+0x70/0x94)
>  r5:00000009 r4:bf05bcb0
> [<80028988>] (warn_slowpath_common) from [<80028a54>]
> (warn_slowpath_fmt+0x38/0x40)
>  r8:01f00000 r7:00000000 r6:0011cc11 r5:808b68c0 r4:bf24fa30
> [<80028a20>] (warn_slowpath_fmt) from [<802b9548>] (gpio_to_desc+0x34/0x48)
>  r3:fffffffe r2:807d23fc
> [<802b9514>] (gpio_to_desc) from [<802d9df8>] (imx6_pcie_host_init+0x18c/0x434)
> [<802d9c6c>] (imx6_pcie_host_init) from [<80886dbc>]
> (dw_pcie_host_init+0x348/0x41c)
>  r6:00000000 r5:808d52cc r4:00000020 r3:802d9c6c
> [<80886a74>] (dw_pcie_host_init) from [<808871d4>] (imx6_pcie_probe+0x320/0x3dc)
>  r10:00000000 r9:000000c4 r8:808d539c r7:bf7e3384 r6:bf24fa30 r5:bf135810
>  r4:bf24fa10
> [<80886eb4>] (imx6_pcie_probe) from [<8034b670>] (platform_drv_probe+0x20/0x50)
>  r8:808d539c r7:00000000 r6:00000000 r5:808d539c r4:bf135810
> [<8034b650>] (platform_drv_probe) from [<80349c74>]
> (driver_probe_device+0x118/0x234)
>  r5:bf135810 r4:80e526b8
> [<80349b5c>] (driver_probe_device) from [<80349e78>] (__driver_attach+0x9c/0xa0)
>  r8:80886e90 r7:00000000 r6:bf135844 r5:808d539c r4:bf135810 r3:00000000
> [<80349ddc>] (__driver_attach) from [<8034806c>] (bus_for_each_dev+0x68/0x9c)
>  r6:80349ddc r5:808d539c r4:00000000 r3:00000000
> [<80348004>] (bus_for_each_dev) from [<8034972c>] (driver_attach+0x20/0x28)
>  r6:808df6a8 r5:bf1f5e00 r4:808d539c
> [<8034970c>] (driver_attach) from [<803493b0>] (bus_add_driver+0x148/0x1f4)
> [<80349268>] (bus_add_driver) from [<8034a4c8>] (driver_register+0x80/0x100)
>  r7:8090e640 r6:8090e640 r5:00000005 r4:808d539c
> [<8034a448>] (driver_register) from [<8034b63c>]
> (__platform_driver_register+0x50/0x64)
>  r5:00000005 r4:808d5388
> [<8034b5ec>] (__platform_driver_register) from [<8034b6e0>]
> (platform_driver_probe+0x28/0xac)
> [<8034b6b8>] (platform_driver_probe) from [<80886ea8>]
> (imx6_pcie_init+0x18/0x24)
>  r5:00000005 r4:808aa104
> [<80886e90>] (imx6_pcie_init) from [<80008978>] (do_one_initcall+0x100/0x164)
> [<80008878>] (do_one_initcall) from [<8085ecc0>]
> (kernel_init_freeable+0x10c/0x1d0)
>  r10:8089e060 r9:000000c4 r8:8089e050 r7:8090e640 r6:8090e640 r5:00000005
>  r4:808aa104
> [<8085ebb4>] (kernel_init_freeable) from [<8063b67c>] (kernel_init+0x10/0x120)
>  r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:8063b66c
>  r4:00000000
> [<8063b66c>] (kernel_init) from [<8000e9c8>] (ret_from_fork+0x14/0x2c)
>  r4:00000000 r3:ffffffff
> ---[ end trace b5e746dfc2398cd7 ]---
> imx6q-pcie 1ffc000.pcie: phy link never came up
> PCI host bridge to bus 0000:00
> pci_bus 0000:00: root bus resource [io  0x1000-0x10000]
> pci_bus 0000:00: root bus resource [mem 0x01000000-0x01efffff]
> pci_bus 0000:00: No busn resource found for root bus, will use [bus 00-ff]

Not related to the GPIO/link problem, but something's wrong here --
the host bridge driver should be telling us what bus numbers are
behind the host bridge.  Since it didn't, the PCI core had to guess.

> PCI: bus0: Fast back to back transfers disabled
> PCI: bus1: Fast back to back transfers enabled
> pci 0000:00:00.0: BAR 0: assigned [mem 0x01000000-0x010fffff]
> pci 0000:00:00.0: BAR 6: assigned [mem 0x01100000-0x0110ffff pref]
> pci 0000:00:00.0: PCI bridge to [bus 01]
> pci 0000:00:00.0: PCI bridge to [bus 01]
>
> Please, any help is welcome.
> Regards,
> Kamel.B
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [Xen-devel] [PATCH] arm/xen: Initialize event channels earlier
From: Julien Grall @ 2014-01-28 17:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.DEB.2.02.1401281651400.4373@kaball.uk.xensource.com>

On 01/28/2014 05:13 PM, Stefano Stabellini wrote:
> On Tue, 28 Jan 2014, Julien Grall wrote:
>> Event channels driver needs to be initialized very early. Until now, Xen
>> initialization was done after all CPUs was bring up.
>>
>> We can safely move the initialization to an early initcall.
>>
>> Also use a cpu notifier to:
>>     - Register the VCPU when the CPU is prepared
>>     - Enable event channel IRQ when the CPU is running
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> 
> Did you test this patch in Dom0 as well as in DomUs?
> 

Only try dom0. I will try domU.

> 
>>  arch/arm/xen/enlighten.c |   84 ++++++++++++++++++++++++++++++----------------
>>  1 file changed, 55 insertions(+), 29 deletions(-)
>>
>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>> index 293eeea..39b668e 100644
>> --- a/arch/arm/xen/enlighten.c
>> +++ b/arch/arm/xen/enlighten.c
>> @@ -23,6 +23,7 @@
>>  #include <linux/of_address.h>
>>  #include <linux/cpuidle.h>
>>  #include <linux/cpufreq.h>
>> +#include <linux/cpu.h>
>>  
>>  #include <linux/mm.h>
>>  
>> @@ -154,12 +155,11 @@ int xen_unmap_domain_mfn_range(struct vm_area_struct *vma,
>>  }
>>  EXPORT_SYMBOL_GPL(xen_unmap_domain_mfn_range);
>>  
>> -static void __init xen_percpu_init(void *unused)
>> +static void xen_percpu_init(int cpu)
>>  {
>>  	struct vcpu_register_vcpu_info info;
>>  	struct vcpu_info *vcpup;
>>  	int err;
>> -	int cpu = get_cpu();
>>  
>>  	pr_info("Xen: initializing cpu%d\n", cpu);
>>  	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>> @@ -170,9 +170,11 @@ static void __init xen_percpu_init(void *unused)
>>  	err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info, cpu, &info);
>>  	BUG_ON(err);
>>  	per_cpu(xen_vcpu, cpu) = vcpup;
>> +}
>>  
>> +static void xen_interrupt_init(void)
>> +{
>>  	enable_percpu_irq(xen_events_irq, 0);
>> -	put_cpu();
>>  }
>>  
>>  static void xen_restart(enum reboot_mode reboot_mode, const char *cmd)
>> @@ -193,6 +195,36 @@ static void xen_power_off(void)
>>  		BUG();
>>  }
>>  
>> +static irqreturn_t xen_arm_callback(int irq, void *arg)
>> +{
>> +	xen_hvm_evtchn_do_upcall();
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int xen_cpu_notification(struct notifier_block *self,
>> +				unsigned long action,
>> +				void *hcpu)
>> +{
>> +	int cpu = (long)hcpu;
>> +
>> +	switch (action) {
>> +	case CPU_UP_PREPARE:
>> +		xen_percpu_init(cpu);
>> +		break;
>> +	case CPU_STARTING:
>> +		xen_interrupt_init();
>> +		break;
> 
> Is CPU_STARTING guaranteed to be called on the new cpu only?

Yes.

> If so, why not call both xen_percpu_init and xen_interrupt_init on
> CPU_STARTING?

Just in case that xen_vcpu is used somewhere else by a cpu notifier
callback CPU_STARTING. We don't know which callback is called first.

> As it stands I think you introduced a subtle change (that might be OK
> but I think is unintentional): xen_percpu_init might not be called from
> the same cpu as its target anymore.

No, xen_percpu_init and xen_interrupt_init are called on the boot cpu at
the end of xen_guest_init.

> 
> 
>> +	default:
>> +		break;
>> +	}
>> +
>> +	return NOTIFY_OK;
>> +}
>> +
>> +static struct notifier_block xen_cpu_notifier = {
>> +	.notifier_call = xen_cpu_notification,
>> +};
>> +
>>  /*
>>   * see Documentation/devicetree/bindings/arm/xen.txt for the
>>   * documentation of the Xen Device Tree format.
>> @@ -209,6 +241,7 @@ static int __init xen_guest_init(void)
>>  	const char *xen_prefix = "xen,xen-";
>>  	struct resource res;
>>  	phys_addr_t grant_frames;
>> +	int cpu;
>>  
>>  	node = of_find_compatible_node(NULL, NULL, "xen,xen");
>>  	if (!node) {
>> @@ -281,9 +314,27 @@ static int __init xen_guest_init(void)
>>  	disable_cpuidle();
>>  	disable_cpufreq();
>>  
>> +	xen_init_IRQ();
>> +
>> +	if (xen_events_irq < 0)
>> +		return -ENODEV;
> 
> Since you are moving this code to xen_guest_init, you can check for
> xen_events_irq earlier on, where we parse the irq from device tree.

Will do.


-- 
Julien Grall

^ permalink raw reply

* [GIT PULL] irqchip: dove: drivers for v3.14
From: Jason Cooper @ 2014-01-28 17:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140110183430.GO19878@titan.lakedaemon.net>

Thomas,

On Fri, Jan 10, 2014 at 01:34:30PM -0500, Jason Cooper wrote:
> On Wed, Dec 11, 2013 at 12:50:37PM -0500, Jason Cooper wrote:
> > On Sun, Dec 01, 2013 at 12:29:14PM -0500, Jason Cooper wrote:
> > > Here's an mvebu driver we've had on the ML for a while, and it's been in
> > > -next for 5 days.
> > 
> > Just checking to make sure this hasn't slipped off of your plate.
> > 
> 
> I'm not able to see where this one might have been pulled, could you
> please pull or let me know if it's not acceptable?

I see you pulled in mvebu/irqchip-fixes.  Thanks for that.  It's getting
near to the end of the merge window and there's been no activity on this
pull request.

Please let us know if there's anything we can do to assist.

thx,

Jason.

> > > The following changes since commit 6ce4eac1f600b34f2f7f58f9cd8f0503d79e42ae:
> > > 
> > >   Linux 3.13-rc1 (2013-11-22 11:30:55 -0800)
> > > 
> > > are available in the git repository at:
> > > 
> > >   git://git.infradead.org/linux-mvebu.git tags/mvebu-irqchip-3.14
> > > 
> > > for you to fetch changes up to 40b367d95fb3d60fc1edb9ba8f6ef52272e48936:
> > > 
> > >   irqchip: irq-dove: Add PMU interrupt controller. (2013-11-26 15:04:53 +0000)
> > > 
> > > ----------------------------------------------------------------
> > > mvebu irqchip changes for v3.14
> > > 
> > >  - add Dove PMU interrupt controller
> > > 
> > > ----------------------------------------------------------------
> > > Andrew Lunn (1):
> > >       irqchip: irq-dove: Add PMU interrupt controller.
> > > 
> > >  .../interrupt-controller/marvell,dove-pmu-intc.txt |  17 +++
> > >  drivers/irqchip/Makefile                           |   1 +
> > >  drivers/irqchip/irq-dove.c                         | 126 +++++++++++++++++++++
> > >  3 files changed, 144 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,dove-pmu-intc.txt
> > >  create mode 100644 drivers/irqchip/irq-dove.c
> > > 
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > linux-arm-kernel at lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] arm64: setup: report non-optional CPU features
From: Catalin Marinas @ 2014-01-28 17:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.DEB.2.02.1401271712340.10177@avanbrunt-dt>

On Tue, Jan 28, 2014 at 01:23:41AM +0000, Alex Van Brunt wrote:
> There are a large number of popular applications compiled for ARMv7-A that read
> /proc/cpuinfo to find out what features the CPU has.

ELF_HWCAP should work fine. Which popular applications are these?

> But, when they are run
> on an arm64 kernel, they fail to run. This is because features that were
> optional on ARMv7 or earlier but are not optional on ARMv8-A like Thumb are not
> listed as a CPU feature using the arm64 kernel. To make those applications run,
> the kernel still needs to print the features in the list.
> 
> This patch changes "cat /proc/cpuinfo" from printing:
> 
> Features        : fp asimd
> 
> To printing:
> 
> Features        : fp asimd wp half thumb fastmult vfp edsp neon vfpv3d16 tlsi vfpv4 idiva idivt

That's not correct, the features reported are for the AArch64 mode, it
doesn't make sense to overlap the AArch32 features here. There is
COMPAT_ELF_HWCAP and the bits should be passed correctly to AArch32
binaries.

A solution would be to add a check for personality(current->personality)
== PER_LINUX32 and report cpuinfo in an AArch32 compatible way and based
on COMPAT_ELF_HWCAP.

-- 
Catalin

^ permalink raw reply

* [PATCH] arm/xen: Initialize event channels earlier
From: Stefano Stabellini @ 2014-01-28 17:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390920842-21886-1-git-send-email-julien.grall@linaro.org>

On Tue, 28 Jan 2014, Julien Grall wrote:
> Event channels driver needs to be initialized very early. Until now, Xen
> initialization was done after all CPUs was bring up.
> 
> We can safely move the initialization to an early initcall.
> 
> Also use a cpu notifier to:
>     - Register the VCPU when the CPU is prepared
>     - Enable event channel IRQ when the CPU is running
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

Did you test this patch in Dom0 as well as in DomUs?


>  arch/arm/xen/enlighten.c |   84 ++++++++++++++++++++++++++++++----------------
>  1 file changed, 55 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 293eeea..39b668e 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -23,6 +23,7 @@
>  #include <linux/of_address.h>
>  #include <linux/cpuidle.h>
>  #include <linux/cpufreq.h>
> +#include <linux/cpu.h>
>  
>  #include <linux/mm.h>
>  
> @@ -154,12 +155,11 @@ int xen_unmap_domain_mfn_range(struct vm_area_struct *vma,
>  }
>  EXPORT_SYMBOL_GPL(xen_unmap_domain_mfn_range);
>  
> -static void __init xen_percpu_init(void *unused)
> +static void xen_percpu_init(int cpu)
>  {
>  	struct vcpu_register_vcpu_info info;
>  	struct vcpu_info *vcpup;
>  	int err;
> -	int cpu = get_cpu();
>  
>  	pr_info("Xen: initializing cpu%d\n", cpu);
>  	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
> @@ -170,9 +170,11 @@ static void __init xen_percpu_init(void *unused)
>  	err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info, cpu, &info);
>  	BUG_ON(err);
>  	per_cpu(xen_vcpu, cpu) = vcpup;
> +}
>  
> +static void xen_interrupt_init(void)
> +{
>  	enable_percpu_irq(xen_events_irq, 0);
> -	put_cpu();
>  }
>  
>  static void xen_restart(enum reboot_mode reboot_mode, const char *cmd)
> @@ -193,6 +195,36 @@ static void xen_power_off(void)
>  		BUG();
>  }
>  
> +static irqreturn_t xen_arm_callback(int irq, void *arg)
> +{
> +	xen_hvm_evtchn_do_upcall();
> +	return IRQ_HANDLED;
> +}
> +
> +static int xen_cpu_notification(struct notifier_block *self,
> +				unsigned long action,
> +				void *hcpu)
> +{
> +	int cpu = (long)hcpu;
> +
> +	switch (action) {
> +	case CPU_UP_PREPARE:
> +		xen_percpu_init(cpu);
> +		break;
> +	case CPU_STARTING:
> +		xen_interrupt_init();
> +		break;

Is CPU_STARTING guaranteed to be called on the new cpu only?
If so, why not call both xen_percpu_init and xen_interrupt_init on
CPU_STARTING?
As it stands I think you introduced a subtle change (that might be OK
but I think is unintentional): xen_percpu_init might not be called from
the same cpu as its target anymore.


> +	default:
> +		break;
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block xen_cpu_notifier = {
> +	.notifier_call = xen_cpu_notification,
> +};
> +
>  /*
>   * see Documentation/devicetree/bindings/arm/xen.txt for the
>   * documentation of the Xen Device Tree format.
> @@ -209,6 +241,7 @@ static int __init xen_guest_init(void)
>  	const char *xen_prefix = "xen,xen-";
>  	struct resource res;
>  	phys_addr_t grant_frames;
> +	int cpu;
>  
>  	node = of_find_compatible_node(NULL, NULL, "xen,xen");
>  	if (!node) {
> @@ -281,9 +314,27 @@ static int __init xen_guest_init(void)
>  	disable_cpuidle();
>  	disable_cpufreq();
>  
> +	xen_init_IRQ();
> +
> +	if (xen_events_irq < 0)
> +		return -ENODEV;

Since you are moving this code to xen_guest_init, you can check for
xen_events_irq earlier on, where we parse the irq from device tree.


> +	if (request_percpu_irq(xen_events_irq, xen_arm_callback,
> +			       "events", &xen_vcpu)) {
> +		pr_err("Error request IRQ %d\n", xen_events_irq);
> +		return -EINVAL;
> +	}
> +
> +	cpu = get_cpu();
> +	xen_percpu_init(cpu);
> +	xen_interrupt_init();
> +	put_cpu();
> +
> +	register_cpu_notifier(&xen_cpu_notifier);
> +
>  	return 0;
>  }
> -core_initcall(xen_guest_init);
> +early_initcall(xen_guest_init);
>  
>  static int __init xen_pm_init(void)
>  {
> @@ -297,31 +348,6 @@ static int __init xen_pm_init(void)
>  }
>  late_initcall(xen_pm_init);
>  
> -static irqreturn_t xen_arm_callback(int irq, void *arg)
> -{
> -	xen_hvm_evtchn_do_upcall();
> -	return IRQ_HANDLED;
> -}
> -
> -static int __init xen_init_events(void)
> -{
> -	if (!xen_domain() || xen_events_irq < 0)
> -		return -ENODEV;
> -
> -	xen_init_IRQ();
> -
> -	if (request_percpu_irq(xen_events_irq, xen_arm_callback,
> -			"events", &xen_vcpu)) {
> -		pr_err("Error requesting IRQ %d\n", xen_events_irq);
> -		return -EINVAL;
> -	}
> -
> -	on_each_cpu(xen_percpu_init, NULL, 0);
> -
> -	return 0;
> -}
> -postcore_initcall(xen_init_events);
> -
>  /* In the hypervisor.S file. */
>  EXPORT_SYMBOL_GPL(HYPERVISOR_event_channel_op);
>  EXPORT_SYMBOL_GPL(HYPERVISOR_grant_table_op);
> -- 
> 1.7.10.4
> 

^ permalink raw reply

* [PATCH v3 07/24] drm/i2c: tda998x: set the video mode from the adjusted value
From: Jean-Francois Moine @ 2014-01-28 17:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140123232907.GA25988@ti.com>

On Thu, 23 Jan 2014 17:29:07 -0600
Darren Etheridge <detheridge@ti.com> wrote:

> > @@ -896,9 +897,9 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
> >  	 * TDA19988 requires high-active sync at input stage,
> >  	 * so invert low-active sync provided by master encoder here
> >  	 */
> > -	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> > +	if (adj_mode->flags & DRM_MODE_FLAG_NHSYNC)
> >  		reg_set(priv, REG_VIP_CNTRL_3, VIP_CNTRL_3_H_TGL);
> > -	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> > +	if (adj_mode->flags & DRM_MODE_FLAG_NVSYNC)
> >  		reg_set(priv, REG_VIP_CNTRL_3, VIP_CNTRL_3_V_TGL);
> >    
> 
> Using the adj_mode->flags breaks a workaround I had done on BeagleBone Black
> (tilcdc + tda998x) to resolve an issue with out of spec syncs from the
> tlcdc.  I invert the HSYNC in adj_mode->flags but don't want the tda998x to
> really know that I am doing that so I use adj_mode in the tilcdc driver, and
> mode here in the tda998x driver.  The theory being adj_mode contains whatever
> workarounds I need to do for the driving device and mode has the pristine
> values that I want to send to the monitor.  I would need to look if there is a
> different way to solve this as I am guessing you are actually using adj_mode in
> the manner it was intended.

No. In fact, I just wanted the function to use only one mode.

Looking at the other drivers, it seems that they don't touch the
adjusted_mode, so, for the Cubox, mode and adjusted_mode have same
values.

I will do an other patch so that you will not have to touch the tilcdc
driver.

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

^ permalink raw reply

* [RFC] arm: vdso: Convert sigpage to vdso implementation
From: Russell King - ARM Linux @ 2014-01-28 17:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390926308-15581-1-git-send-email-steve.capper@linaro.org>

On Tue, Jan 28, 2014 at 04:25:08PM +0000, Steve Capper wrote:
> ARM has a special sigpage that is used for signal return trampolines.
> Its implementation is very similar to a VDSO conceptually in that it
> occupies a special mapping in user address space.
> 
> One could actually host the trampoline code in a VDSO instead with the
> added advantage that one could also host specialised routines there.
> One such routine could be gettimeofday where on ARM we have architected
> (and some vendor supplied) timers that can be queried entirely in
> userspace, obviating the need for an expensive syscall.
> 
> This patch converts the sigpage implementation to a VDSO. It is mostly
> a direct port from Will Deacon's arm64 implementation with the ARM
> signal trampoline plumbed in.
> 
> Signed-off-by: Steve Capper <steve.capper@linaro.org>
> ---
> As can be inferred from this RFC, I am interested ultimately in
> implementing a syscall-less gettimeofday for ARM. Whilst researching
> possible vectors page or VDSO implementations, I came across the
> sigpage mechanism which is very similar to a VDSO.
> 
> The very simple function, __kernel_vdso_doubler, resolved in a test
> program automatically on my Arndale board (running Fedora 20) without
> any additional prodding.
> 
> IPC stress tests from LTP were executed to test the signal trampoline.
> 
> I would appreciate any comments on this approach of converting the
> sigpage to a VDSO. If this looks sane to people, I will work on the
> gettimeofday logic in a later patch.

I'm not happy with this removing much of the work I pushed into the
kernel to work around the security issues which were identified with
the fixed-address placement of stuff in the vectors page.  Particularly
the random placement of the signal return stubs within the new signal
page is gone with the VDSO approach, which means if someone can discover
the VDSO page, they can issue any system call they please by knowing
the appropriate offset into the page to call.

While the VDSO page will be placed randomly, I'd also like to have the
signal handlers placed randomly within that page as well - there's no
need for them to be at a fixed offset.  The only thing which needs to
know where they are after all is the kernel.

I'm not sure about putting gettimeofday() into this - gettimeofday()
would need to have various kernel variables exported into userspace
for the VDSO page to then compute the current time of day from the
timer value(s), and that's certainly not going to be at a fixed
address.

I believe x86 eventually ended up going down the path of trapping and
emulating calls to the VDSO page because VDSO became too much of a
problem (though I think it does provide the option for having it back
but not by default.)

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply

* [PATCH v3 1/3] ARM: sun7i/sun6i: irqchip: Add irqchip driver for NMI controller
From: Maxime Ripard @ 2014-01-28 16:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52E78E3F.6080902@redhat.com>

Hi,

On Tue, Jan 28, 2014 at 12:02:23PM +0100, Hans de Goede wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hi,
> 
> On 01/28/2014 11:40 AM, Maxime Ripard wrote:
> 
> Jumping in here to try and clarify things, or so I hope at least :)

Sure :)

> No, the IRQ from the PMIC is a level sensitive IRQ, so it would look
> like this:

Hmm, your mailer seems to have mangled your drawing :(

> The PMIC irq line won't go low until an i2c write to its irq status
> registers write-clears all status bits for which the corresponding
> bit in the irq-mask register is set.

Which makes sense too

> And the only reason the NMI -> GIC also goes low is because the unmask
> operation writes a second ack to the NMI controller in the unmask
> callback of the NMI controller driver.

Yes, and this is exactly what I don't understand. You shouldn't need
that ack in first place, since it's been done already right after the
unmask.

> Note that we cannot use edge triggered interrupts here because the PMIC
> has the typical setup with multiple irq status bits driving a single
> irq line, so the irq handler does read irq-status, handle stuff,
> write-clear irq-status. And if any other irq-status bits get set
> between the read and write-clear the PMIC -> NMI line will stay
> high, as it should since there are more interrupts to handle.

Yep, the edge-thing was just the only case I could think of where it
could lead to troubles.

In what you're saying, which makes total sense, if we don't do the
ack, as soon as the irq will be unmasked, since the level is high, the
handler will be called again, treat the new interrupts, and so on. I
don't see how this is an issue actually.

> > But in this case, you would have two events coming from your
> > device (the two rising edges), so you'd expect two interrupts. And
> > in the case of a level triggered interrupt, the device would keep
> > the interrupt line active until it's unmasked, so we wouldn't end
> > up with this either.
> > 
> >> sunxi_sc_nmi_ack_and_unmask is therefore called (by
> >> irq_finalize_oneshot) after the IRQ thread has been
> >> executed. After the IRQ thread has ACKed the IRQs on the
> >> originating device we can finally ACK and unmask again the NMI.
> > 
> > And what happens if you get a new interrupt right between the end
> > of the handler and the unmask?
> 
> The implicit ack done by the unmask will be ignored if the NMI line
> is still high, just like the initial ack is ignored (which is why we
> need the mask), and when the unmask completes the irq will
> immediately retrigger, as it should.

Yeah, but why do we need the ack in the first place? I can't think of
a case where the ACK would be doing something useful. Either:
  - there is no new interrupts between the mask/ack and the unmask, so
    there's no interrupt to ack.
  - There's a new interrupt between the mask/ack and the
    unmask. There's two more cases here:
    * The interrupt came before the device handler kicked in, and the
      handler will treat it/ack it: No issue
    * The interrupt comes right after the handler has been acking its
      interrupt, the level stays high, your handler is called once
      again, you can treat it: No issue

> >>> I really wonder whether it makes sense to have a generic chip
> >>> here. It seems to be much more complicated than it should. It's
> >>> only about a single interrupt interrupt chip here.
> >> 
> >> I agree but is there any other way to manage the NMI without the
> >> driver of the device connected to NMI having to worry about
> >> acking/masking/unmasking/ etc..?
> > 
> > Yes, just declare a "raw" irqchip. Pretty much like we do on
> > irq-sun4i for example.
> 
> Hmm, I'm not going to say that using a raw irqchip here is a bad
> idea, it sounds like it would be better.

There's none. It was a separate comment :)

> But I don't think this will solve the need the mask / umask. The
> problem is we need to do an i2c write to clear the interrupt source,
> which needs to be done from a thread / workqueue. So when the
> interrupt handler finishes the source won't be cleared yet, and
> AFAIK then only way to deal with this is to mask the interrupt until
> we've cleared the interrupt source.

Yes, but the interrupt is still masked during the time between the
"hard" handler and the thread, so there's no way you can get an
interrupt flood during that time.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140128/18a6c784/attachment.sig>

^ permalink raw reply

* [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
From: Felipe Balbi @ 2014-01-28 16:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140128153230.GA20991@xps8300>

Hi,

On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote:
> On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote:
> > > Why would you need to know if the PHY drivers are needed or not
> > > explicitly in your controller driver?
> > 
> > because, one way or another, they all do need it. Except for quirky ones
> > like AM437x where a USB3 IP was hardwired into USB2-only mode, so it
> > really lacks a USB3 PHY.
> 
> The Baytrail board I deal with has completely autonomous PHYs. But my
> question is, why would you need to care about the PHYs in your
> controller driver? Why can't you leave the responsibility of them to
> the upper layers and trust what they tell you?
> 
> If there is no USB3 PHY for dwc3 then, the driver gets something like
> -ENODEV and just continues. There is no need to know about the
> details.

it's a little more complicated than that. We could get -EPROBE_DEFER
meaning we should try probing at a later time because the PHY isn't
available yet.

But sure, if we can very easily differentiate between those two
conditions in a way that error report from PHY layer (whichever it is),
then we can certainly do that.

> For the controller drivers the PHYs are just a resource like any
> other. The controller drivers can't have any responsibility of
> them. They should not care if PHY drivers are available for them or
> not, or even if the PHY framework is available or not.

huh? If memory isn't available you don't continue probing, right ? If
your IORESOURCE_MEM is missing, you also don't continue probing, if your
IRQ line is missing, you bail too. Those are also nothing but resources
to the driver, what you're asking here is to treat PHY as a _different_
resource; which might be fine, but we need to make sure we don't
continue probing when a PHY is missing in a platform that certainly
needs a PHY.

> > > > But I really want to see the argument against using no-op. As far as I
> > > > could see, everybody needs a PHY driver one way or another, some
> > > > platforms just haven't sent any PHY driver upstream and have their own
> > > > hacked up solution to avoid using the PHY layer.
> > > 
> > > Not true in our case. Platforms using Intel's SoCs and chip sets may
> > > or may not have controllable USB PHY. Quite often they don't. The
> > > Baytrails have usually ULPI PHY for USB2, but that does not mean they
> > > provide any vendor specific functions or any need for a driver in any
> > > case.
> > 
> > that's different from what I heard.
> 
> I don't know where you got that impression, but it's not true. The
> Baytrail SoCs for example don't have internal USB PHYs, which means
> the manufacturers using it can select what they want. So we have
> boards where PHY driver(s) is needed and boards where it isn't.

alright, that explains it ;-) So you have external USB2 and USB3 PHYs ?
You have an external PIPE3 interface ? That's quite an achievement,
kudos to your HW designers. Getting timing closure on PIPE3 is a
difficult task.

> The problem is that we just don't always know all the details about
> the platform. If the PHY is ULPI PHY, we can do runtime detection, but
> we can't even rely on always having ULPI.

right, we've had that before on n900 ;-)

> > > Are we talking about the old USB PHY library or the new PHY framework
> > > with the no-op PHY driver?
> > > 
> > > Well, in any case, I don't understand what is the purpose of the no-op
> > > PHY driver. What are you drying to achieve with that?
> > 
> > I'm trying to avoid supporting 500 different combinations for a single
> > driver. We already support old USB PHY layer and generic PHY layer, now
> > they both need to be made optional.
> 
> This is really good to get. We have some projects where we are dealing
> with more embedded environments, like IVI, where the kernel should be
> stripped of everything useless. Since the PHYs are autonomous, we
> should be able to disable the PHY libraries/frameworks.

hmmm, in that case it's a lot easier to treat. We can use
ERR_PTR(-ENXIO) as an indication that the framework is disabled, or
something like that.

The difficult is really reliably supporting e.g. OMAP5 (which won't work
without a PHY) and your BayTrail with autonomous PHYs. What can we use
as an indication ?

I mean, I need to know that a particular platform depends on a PHY
driver before I decide to return -EPROBE_DEFER or just assume the PHY
isn't needed ;-)

> But I still don't understand what is the benefit in the no-op? You
> really need to explain this. What you have now in dwc3 is expectations
> regarding the PHY, which put a lot of pressure on upper layers to
> satisfy the driver. The no-op is just an extra thing that you have to
> provide when you fail to determine if the system requires a PHY driver
> or not, or if you know that it doesn't, plus an additional check for
> the case where the PHY lib/framework is not enabled. I don't see the
> value in it.

it "solves" the difficulty above. If everybody has to provide a PHY
driver, the problem is a lot simpler don't you think ? ;-)

> > The old USB PHY layer also provides
> > a no-op, now called phy-generic, which is actually pretty useful.
> > 
> > On top of all that, I'm sure you'll subscribe to the idea of preventing
> > dwc3 from becoming another MUSB which supports several different
> > configurations and none work correctly. I much prefer to take baby steps
> > which are well thought-out and very well exercised, so I'll be very
> > pedantic about proof of testing.
> 
> I think our goals are the same. I just want to also minimize the need
> for any platform specific extra work from the upper layers regarding
> the PHYs.

I'll agree to that, but let's not apply patches until we're 100% sure
there will be no regressions. Looking at $subject, I don't think it
satisfies that condition ;-)

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140128/40c2bd34/attachment.sig>

^ permalink raw reply

* [PATCH 15/18] regulator: max14577: Add support for max77836 regulators
From: Mark Brown @ 2014-01-28 16:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390911522-28209-16-git-send-email-k.kozlowski@samsung.com>

On Tue, Jan 28, 2014 at 01:18:39PM +0100, Krzysztof Kozlowski wrote:
> Add support for MAX77836 chipset and its additional two LDO regulators.
> These LDO regulators are controlled by the PMIC block with additional
> regmap (different I2C slave address).

Reviewed-by: Mark Brown <broonie@linaro.org>

This should go in with the rest of the changes I guess.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140128/84ec92c7/attachment.sig>

^ permalink raw reply

* [RFC] arm: vdso: Convert sigpage to vdso implementation
From: Steve Capper @ 2014-01-28 16:25 UTC (permalink / raw)
  To: linux-arm-kernel

ARM has a special sigpage that is used for signal return trampolines.
Its implementation is very similar to a VDSO conceptually in that it
occupies a special mapping in user address space.

One could actually host the trampoline code in a VDSO instead with the
added advantage that one could also host specialised routines there.
One such routine could be gettimeofday where on ARM we have architected
(and some vendor supplied) timers that can be queried entirely in
userspace, obviating the need for an expensive syscall.

This patch converts the sigpage implementation to a VDSO. It is mostly
a direct port from Will Deacon's arm64 implementation with the ARM
signal trampoline plumbed in.

Signed-off-by: Steve Capper <steve.capper@linaro.org>
---
As can be inferred from this RFC, I am interested ultimately in
implementing a syscall-less gettimeofday for ARM. Whilst researching
possible vectors page or VDSO implementations, I came across the
sigpage mechanism which is very similar to a VDSO.

The very simple function, __kernel_vdso_doubler, resolved in a test
program automatically on my Arndale board (running Fedora 20) without
any additional prodding.

IPC stress tests from LTP were executed to test the signal trampoline.

I would appreciate any comments on this approach of converting the
sigpage to a VDSO. If this looks sane to people, I will work on the
gettimeofday logic in a later patch.

Cheers,
-- 
Steve
---
 arch/arm/include/asm/elf.h               |  11 +++-
 arch/arm/include/asm/mmu.h               |   2 +-
 arch/arm/include/asm/vdso.h              |  44 +++++++++++++
 arch/arm/kernel/Makefile                 |   9 +++
 arch/arm/kernel/process.c                |  48 +++++---------
 arch/arm/kernel/signal.c                 |  38 ++---------
 arch/arm/kernel/vdso.c                   | 105 +++++++++++++++++++++++++++++++
 arch/arm/kernel/vdso/.gitignore          |   2 +
 arch/arm/kernel/vdso/Makefile            |  72 +++++++++++++++++++++
 arch/arm/kernel/vdso/gen_vdso_offsets.sh |  15 +++++
 arch/arm/kernel/vdso/simple.S            |  31 +++++++++
 arch/arm/kernel/vdso/vdso.S              |  35 +++++++++++
 arch/arm/kernel/vdso/vdso.lds.S          |  99 +++++++++++++++++++++++++++++
 13 files changed, 442 insertions(+), 69 deletions(-)
 create mode 100644 arch/arm/include/asm/vdso.h
 create mode 100644 arch/arm/kernel/vdso.c
 create mode 100644 arch/arm/kernel/vdso/.gitignore
 create mode 100644 arch/arm/kernel/vdso/Makefile
 create mode 100755 arch/arm/kernel/vdso/gen_vdso_offsets.sh
 create mode 100644 arch/arm/kernel/vdso/simple.S
 create mode 100644 arch/arm/kernel/vdso/vdso.S
 create mode 100644 arch/arm/kernel/vdso/vdso.lds.S

diff --git a/arch/arm/include/asm/elf.h b/arch/arm/include/asm/elf.h
index f4b46d3..ee45b67 100644
--- a/arch/arm/include/asm/elf.h
+++ b/arch/arm/include/asm/elf.h
@@ -132,6 +132,15 @@ extern unsigned long arch_randomize_brk(struct mm_struct *mm);
 #define ARCH_HAS_SETUP_ADDITIONAL_PAGES 1
 struct linux_binprm;
 int arch_setup_additional_pages(struct linux_binprm *, int);
-#endif
+
+#define AT_SYSINFO_EHDR			33
+#define __HAVE_ARCH_GATE_AREA		1
+
+#define ARCH_DLINFO							\
+do {									\
+	NEW_AUX_ENT(AT_SYSINFO_EHDR,					\
+		    (elf_addr_t)current->mm->context.vdso);		\
+} while (0)
+#endif /* CONFIG_MMU */
 
 #endif
diff --git a/arch/arm/include/asm/mmu.h b/arch/arm/include/asm/mmu.h
index 64fd151..11bcbf3 100644
--- a/arch/arm/include/asm/mmu.h
+++ b/arch/arm/include/asm/mmu.h
@@ -10,7 +10,7 @@ typedef struct {
 	int		switch_pending;
 #endif
 	unsigned int	vmalloc_seq;
-	unsigned long	sigpage;
+	unsigned long	vdso;
 } mm_context_t;
 
 #ifdef CONFIG_CPU_HAS_ASID
diff --git a/arch/arm/include/asm/vdso.h b/arch/arm/include/asm/vdso.h
new file mode 100644
index 0000000..024b9726
--- /dev/null
+++ b/arch/arm/include/asm/vdso.h
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2014 Linaro Ltd.
+ *
+ * Based on Will Deacon's implementation in arch/arm64
+ * Copyright (C) 2012 ARM 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.
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef __ASM_VDSO_H
+#define __ASM_VDSO_H
+
+#ifdef __KERNEL__
+
+/*
+ * Default link address for the vDSO.
+ * Since we randomise the VDSO mapping, there's little point in trying
+ * to prelink this.
+ */
+#define VDSO_LBASE	0x0
+
+#ifndef __ASSEMBLY__
+
+#include <generated/vdso-offsets.h>
+
+#define VDSO_SYMBOL(base, name)						   \
+({									   \
+	(void *)(vdso_offset_##name - VDSO_LBASE + (unsigned long)(base)); \
+})
+
+#endif /* !__ASSEMBLY__ */
+
+#endif /* __KERNEL__ */
+
+#endif /* __ASM_VDSO_H */
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index a30fc9b..87983ef 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -99,3 +99,12 @@ obj-$(CONFIG_SMP)		+= psci_smp.o
 endif
 
 extra-y := $(head-y) vmlinux.lds
+
+ifdef CONFIG_MMU
+obj-y				+= vdso.o
+obj-y				+= vdso/
+
+# vDSO - this must be built first to generate the symbol offsets
+$(call objectify,$(obj-y)): $(obj)/vdso/vdso-offsets.h
+$(obj)/vdso/vdso-offsets.h: $(obj)/vdso
+endif
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 92f7b15..1aa1cc2 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -467,46 +467,28 @@ int in_gate_area_no_mm(unsigned long addr)
 }
 #define is_gate_vma(vma)	((vma) == &gate_vma)
 #else
-#define is_gate_vma(vma)	0
-#endif
+#define is_gate_vma(vma)	(0)
 
-const char *arch_vma_name(struct vm_area_struct *vma)
+struct vm_area_struct * get_gate_vma(struct mm_struct *mm)
 {
-	return is_gate_vma(vma) ? "[vectors]" :
-		(vma->vm_mm && vma->vm_start == vma->vm_mm->context.sigpage) ?
-		 "[sigpage]" : NULL;
+	return NULL;
 }
 
-static struct page *signal_page;
-extern struct page *get_signal_page(void);
-
-int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
+int in_gate_area_no_mm(unsigned long addr)
 {
-	struct mm_struct *mm = current->mm;
-	unsigned long addr;
-	int ret;
-
-	if (!signal_page)
-		signal_page = get_signal_page();
-	if (!signal_page)
-		return -ENOMEM;
-
-	down_write(&mm->mmap_sem);
-	addr = get_unmapped_area(NULL, 0, PAGE_SIZE, 0, 0);
-	if (IS_ERR_VALUE(addr)) {
-		ret = addr;
-		goto up_fail;
-	}
+	return 0;
+}
+#endif
 
-	ret = install_special_mapping(mm, addr, PAGE_SIZE,
-		VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC,
-		&signal_page);
+const char *arch_vma_name(struct vm_area_struct *vma)
+{
+	if (is_gate_vma(vma))
+		return "[vectors]";
 
-	if (ret == 0)
-		mm->context.sigpage = addr;
+	if (vma->vm_mm && vma->vm_start == vma->vm_mm->context.vdso)
+		return "[vdso]";
 
- up_fail:
-	up_write(&mm->mmap_sem);
-	return ret;
+	return NULL;
 }
+
 #endif
diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index 04d6388..b510077 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -20,11 +20,10 @@
 #include <asm/ucontext.h>
 #include <asm/unistd.h>
 #include <asm/vfp.h>
+#include <asm/vdso.h>
 
 extern const unsigned long sigreturn_codes[7];
 
-static unsigned long signal_return_offset;
-
 #ifdef CONFIG_CRUNCH
 static int preserve_crunch_context(struct crunch_sigframe __user *frame)
 {
@@ -395,8 +394,9 @@ setup_return(struct pt_regs *regs, struct ksignal *ksig,
 			 * except when the MPU has protected the vectors
 			 * page from PL0
 			 */
-			retcode = mm->context.sigpage + signal_return_offset +
-				  (idx << 2) + thumb;
+			retcode = (unsigned long) VDSO_SYMBOL(mm->context.vdso,
+								sigtramp);
+			retcode += (idx << 2) + thumb;
 		} else
 #endif
 		{
@@ -600,33 +600,3 @@ do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall)
 	} while (thread_flags & _TIF_WORK_MASK);
 	return 0;
 }
-
-struct page *get_signal_page(void)
-{
-	unsigned long ptr;
-	unsigned offset;
-	struct page *page;
-	void *addr;
-
-	page = alloc_pages(GFP_KERNEL, 0);
-
-	if (!page)
-		return NULL;
-
-	addr = page_address(page);
-
-	/* Give the signal return code some randomness */
-	offset = 0x200 + (get_random_int() & 0x7fc);
-	signal_return_offset = offset;
-
-	/*
-	 * Copy signal return handlers into the vector page, and
-	 * set sigreturn to be a pointer to these.
-	 */
-	memcpy(addr + offset, sigreturn_codes, sizeof(sigreturn_codes));
-
-	ptr = (unsigned long)addr + offset;
-	flush_icache_range(ptr, ptr + sizeof(sigreturn_codes));
-
-	return page;
-}
diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
new file mode 100644
index 0000000..fd69184
--- /dev/null
+++ b/arch/arm/kernel/vdso.c
@@ -0,0 +1,105 @@
+/*
+ * VDSO implementation for ARM
+ *
+ * Copyright (C) 2014 Linaro Ltd.
+ *
+ * Code based on Will Deacon's arm64 VDSO implementation.
+ * Copyright (C) 2012 ARM 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.
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/mm.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+#include <linux/binfmts.h>
+#include <asm/vdso.h>
+
+extern char vdso_start, vdso_end;
+static unsigned long vdso_pages;
+static struct page **vdso_pagelist;
+
+static int __init vdso_init(void)
+{
+	struct page *pg;
+	char *vbase;
+	int i, ret = 0;
+
+	vdso_pages = (&vdso_end - &vdso_start) >> PAGE_SHIFT;
+	pr_info("vdso: %ld pages (%ld code, %ld data) at base %p\n",
+		vdso_pages, vdso_pages, 0L, &vdso_start);
+
+	vdso_pagelist = kzalloc(sizeof(struct page *) * vdso_pages,
+				GFP_KERNEL);
+	if (vdso_pagelist == NULL) {
+		pr_err("Failed to allocate vDSO pagelist!\n");
+		return -ENOMEM;
+	}
+
+	/* Grab the vDSO code pages. */
+	for (i = 0; i < vdso_pages; i++) {
+		pg = virt_to_page(&vdso_start + i*PAGE_SIZE);
+		ClearPageReserved(pg);
+		get_page(pg);
+		vdso_pagelist[i] = pg;
+	}
+
+	/* Sanity check the shared object header. */
+	vbase = vmap(vdso_pagelist, 1, 0, PAGE_KERNEL);
+	if (vbase == NULL) {
+		pr_err("Failed to map vDSO pagelist!\n");
+		return -ENOMEM;
+	} else if (memcmp(vbase, "\177ELF", 4)) {
+		pr_err("vDSO is not a valid ELF object!\n");
+		ret = -EINVAL;
+		goto unmap;
+	}
+
+unmap:
+	vunmap(vbase);
+	return ret;
+}
+arch_initcall(vdso_init);
+
+int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
+{
+	struct mm_struct *mm = current->mm;
+	unsigned long vdso_base, vdso_mapping_len;
+	int ret;
+
+	vdso_mapping_len = vdso_pages << PAGE_SHIFT;
+
+	down_write(&mm->mmap_sem);
+	vdso_base = get_unmapped_area(NULL, 0, vdso_mapping_len, 0, 0);
+	if (IS_ERR_VALUE(vdso_base)) {
+		ret = vdso_base;
+		goto up_fail;
+	}
+	mm->context.vdso = vdso_base;
+
+	ret = install_special_mapping(mm, vdso_base, vdso_mapping_len,
+		VM_READ | VM_EXEC |
+		VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC,
+		vdso_pagelist);
+
+	if (ret) {
+		mm->context.vdso = 0;
+		goto up_fail;
+	}
+
+ up_fail:
+	up_write(&mm->mmap_sem);
+	return ret;
+}
diff --git a/arch/arm/kernel/vdso/.gitignore b/arch/arm/kernel/vdso/.gitignore
new file mode 100644
index 0000000..b8cc94e
--- /dev/null
+++ b/arch/arm/kernel/vdso/.gitignore
@@ -0,0 +1,2 @@
+vdso.lds
+vdso-offsets.h
diff --git a/arch/arm/kernel/vdso/Makefile b/arch/arm/kernel/vdso/Makefile
new file mode 100644
index 0000000..13d3531
--- /dev/null
+++ b/arch/arm/kernel/vdso/Makefile
@@ -0,0 +1,72 @@
+#
+# Building a vDSO image for ARM.
+#
+# Based heavily on arm64 implementation by:
+# Author: Will Deacon <will.deacon@arm.com>
+# Heavily based on the vDSO Makefiles for other archs.
+#
+
+obj-vdso := simple.o
+obj-sig  := sigreturn_codes.o
+
+# Build rules
+targets := $(obj-vdso) $(obj-sig) vdso.so vdso.so.dbg
+obj-vdso := $(addprefix $(obj)/, $(obj-vdso))
+obj-sig := $(addprefix $(obj)/, $(obj-sig))
+
+ccflags-y := -shared -fno-common -fno-builtin
+ccflags-y += -nostdlib -Wl,-soname=linux-vdso.so.1 \
+		$(call cc-ldoption, -Wl$(comma)--hash-style=sysv)
+
+obj-y += vdso.o
+extra-y += vdso.lds vdso-offsets.h
+CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
+
+# Force dependency (incbin is bad)
+$(obj)/vdso.o : $(obj)/vdso.so
+
+# Link rule for the .so file, .lds has to be first
+$(obj)/vdso.so.dbg: $(src)/vdso.lds $(obj-vdso) $(obj-sig)
+	$(call if_changed,vdsold)
+
+# Strip rule for the .so file
+$(obj)/%.so: OBJCOPYFLAGS := -S
+$(obj)/%.so: $(obj)/%.so.dbg FORCE
+	$(call if_changed,objcopy)
+
+# Generate VDSO offsets using helper script
+gen-vdsosym := $(srctree)/$(src)/gen_vdso_offsets.sh
+quiet_cmd_vdsosym = VDSOSYM $@
+define cmd_vdsosym
+	$(NM) $< | $(gen-vdsosym) | LC_ALL=C sort > $@ && \
+	cp $@ include/generated/
+endef
+
+$(obj)/vdso-offsets.h: $(obj)/vdso.so.dbg FORCE
+	$(call if_changed,vdsosym)
+
+# We can't move sigreturn_codes.S into our vdso as it contains code
+# which can also be used if we have no mmu. So we re-compile the
+# source from the parent directory, to prevent code duplication.
+$(obj)/sigreturn_codes.o:	$(obj)/../sigreturn_codes.S
+				$(call if_changed_dep,vdsoas)
+
+# Assembly rules for the .S files
+$(obj-vdso): %.o: %.S
+	$(call if_changed_dep,vdsoas)
+
+# Actual build commands
+quiet_cmd_vdsold = VDSOL $@
+      cmd_vdsold = $(CC) $(c_flags) -Wl,-T $^ -o $@
+quiet_cmd_vdsoas = VDSOA $@
+      cmd_vdsoas = $(CC) $(a_flags) -c -o $@ $<
+
+# Install commands for the unstripped file
+quiet_cmd_vdso_install = INSTALL $@
+      cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
+
+vdso.so: $(obj)/vdso.so.dbg
+	@mkdir -p $(MODLIB)/vdso
+	$(call cmd,vdso_install)
+
+vdso_install: vdso.so
diff --git a/arch/arm/kernel/vdso/gen_vdso_offsets.sh b/arch/arm/kernel/vdso/gen_vdso_offsets.sh
new file mode 100755
index 0000000..5b329ae
--- /dev/null
+++ b/arch/arm/kernel/vdso/gen_vdso_offsets.sh
@@ -0,0 +1,15 @@
+#!/bin/sh
+
+#
+# Match symbols in the DSO that look like VDSO_*; produce a header file
+# of constant offsets into the shared object.
+#
+# Doing this inside the Makefile will break the $(filter-out) function,
+# causing Kbuild to rebuild the vdso-offsets header file every time.
+#
+# Author: Will Deacon <will.deacon@arm.com>
+#
+
+LC_ALL=C
+sed -n -e 's/^00*/0/' -e \
+'s/^\([0-9a-fA-F]*\) . VDSO_\([a-zA-Z0-9_]*\)$/\#define vdso_offset_\2\t0x\1/p'
diff --git a/arch/arm/kernel/vdso/simple.S b/arch/arm/kernel/vdso/simple.S
new file mode 100644
index 0000000..6f21324
--- /dev/null
+++ b/arch/arm/kernel/vdso/simple.S
@@ -0,0 +1,31 @@
+/*
+ * Simple test function for VDSO implementation for ARM
+ *
+ * Copyright (C) 2014 Linaro Ltd.
+ *
+ * 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.
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+
+
+#include <linux/linkage.h>
+#include <asm/unistd.h>
+
+/*
+ * An extremely simple test function:
+ * unsigned int __kernel_vdso_doubler(unsigned int arg);
+ */
+	.text
+ENTRY(__kernel_vdso_doubler)
+	lsl r0, r0, #1
+	mov pc, lr
+ENDPROC(__kernel_vdso_doubler)
diff --git a/arch/arm/kernel/vdso/vdso.S b/arch/arm/kernel/vdso/vdso.S
new file mode 100644
index 0000000..a459d42
--- /dev/null
+++ b/arch/arm/kernel/vdso/vdso.S
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2014 Linaro Ltd.
+ *
+ * Based on arm64 implementation by Will Deacon.
+ * Copyright (C) 2012 ARM 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.
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/linkage.h>
+#include <linux/const.h>
+#include <asm/page.h>
+
+	__PAGE_ALIGNED_DATA
+
+	.globl vdso_start, vdso_end
+	.balign PAGE_SIZE
+vdso_start:
+	.incbin "arch/arm/kernel/vdso/vdso.so"
+	.balign PAGE_SIZE
+vdso_end:
+
+	.previous
diff --git a/arch/arm/kernel/vdso/vdso.lds.S b/arch/arm/kernel/vdso/vdso.lds.S
new file mode 100644
index 0000000..1bacbe8
--- /dev/null
+++ b/arch/arm/kernel/vdso/vdso.lds.S
@@ -0,0 +1,99 @@
+/*
+ * GNU linker script for the VDSO library.
+ *
+ * Copyright (C) 2014 Linaro ltd.
+ * Based heavily on work by:
+ * Will Deacon <will.deacon@arm.com>
+ * Copyright (C) 2012 ARM Limited
+ * Heavily based on the vDSO linker scripts for other archs.
+ *
+ * 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.
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <linux/const.h>
+#include <asm/page.h>
+#include <asm/vdso.h>
+
+OUTPUT_FORMAT("elf32-littlearm", "elf32-bigarm", "elf32-littlearm")
+OUTPUT_ARCH(arm)
+
+SECTIONS
+{
+	. = VDSO_LBASE + SIZEOF_HEADERS;
+
+	.hash		: { *(.hash) }			:text
+	.gnu.hash	: { *(.gnu.hash) }
+	.dynsym		: { *(.dynsym) }
+	.dynstr		: { *(.dynstr) }
+	.gnu.version	: { *(.gnu.version) }
+	.gnu.version_d	: { *(.gnu.version_d) }
+	.gnu.version_r	: { *(.gnu.version_r) }
+
+	.note		: { *(.note.*) }		:text	:note
+
+	. = ALIGN(16);
+
+	.text		: { *(.text*) }			:text
+	PROVIDE (__etext = .);
+	PROVIDE (_etext = .);
+	PROVIDE (etext = .);
+
+	.eh_frame_hdr	: { *(.eh_frame_hdr) }		:text	:eh_frame_hdr
+	.eh_frame	: { KEEP (*(.eh_frame)) }	:text
+
+	.dynamic	: { *(.dynamic) }		:text	:dynamic
+
+	.rodata		: { *(.rodata*) }		:text
+
+	_end = .;
+	PROVIDE(end = .);
+
+	. = ALIGN(PAGE_SIZE);
+	PROVIDE(_vdso_data = .);
+
+	/DISCARD/	: {
+		*(.note.GNU-stack)
+		*(.data .data.* .gnu.linkonce.d.* .sdata*)
+		*(.bss .sbss .dynbss .dynsbss)
+	}
+}
+
+/*
+ * We must supply the ELF program headers explicitly to get just one
+ * PT_LOAD segment, and set the flags explicitly to make segments read-only.
+ */
+PHDRS
+{
+	text		PT_LOAD		FLAGS(5) FILEHDR PHDRS; /* PF_R|PF_X */
+	dynamic		PT_DYNAMIC	FLAGS(4);		/* PF_R */
+	note		PT_NOTE		FLAGS(4);		/* PF_R */
+	eh_frame_hdr	PT_GNU_EH_FRAME;
+}
+
+/*
+ * This controls what symbols we export from the DSO.
+ */
+VERSION
+{
+	LINUX_2.6.39 {
+	global:
+		__kernel_vdso_doubler;
+	local: *;
+	};
+}
+
+/*
+ * Make the sigreturn code visible to the kernel.
+ */
+VDSO_sigtramp		= sigreturn_codes;
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH RFC 00/73] tree-wide: clean up some no longer required #include <linux/init.h>
From: Paul Gortmaker @ 2014-01-28 16:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390878783.3872.63.camel@pasglop>

On 14-01-27 10:13 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2014-01-22 at 19:38 -0500, Paul Gortmaker wrote:
> 
>> Thanks, it was a great help as it uncovered a few issues in fringe arch
>> that I didn't have toolchains for, and I've fixed all of those up.
>>
>> I've noticed that powerpc has been un-buildable for a while now; I have
>> used this hack patch locally so I could run the ppc defconfigs to check
>> that I didn't break anything.  Maybe useful for linux-next in the
>> interim?  It is a hack patch -- Not-Signed-off-by: Paul Gortmaker.  :)
> 
> Can you and/or Aneesh submit that as a proper patch (with S-O-B
> etc...) ?

I'd updated toolchains and didn't realize it was still broken.  Patch sent.

http://patchwork.ozlabs.org/patch/314749/

Paul.
--

> 
> Thanks !
> 
> Cheers,
> Ben.
> 
>> Paul.
>> --
>>
>> diff --git a/arch/powerpc/include/asm/pgtable-ppc64.h b/arch/powerpc/include/asm/pgtable-ppc64.h
>> index d27960c89a71..d0f070a2b395 100644
>> --- a/arch/powerpc/include/asm/pgtable-ppc64.h
>> +++ b/arch/powerpc/include/asm/pgtable-ppc64.h
>> @@ -560,9 +560,9 @@ extern void pmdp_invalidate(struct vm_area_struct *vma, unsigned long address,
>>  			    pmd_t *pmdp);
>>  
>>  #define pmd_move_must_withdraw pmd_move_must_withdraw
>> -typedef struct spinlock spinlock_t;
>> -static inline int pmd_move_must_withdraw(spinlock_t *new_pmd_ptl,
>> -					 spinlock_t *old_pmd_ptl)
>> +struct spinlock;
>> +static inline int pmd_move_must_withdraw(struct spinlock *new_pmd_ptl,
>> +					 struct spinlock *old_pmd_ptl)
>>  {
>>  	/*
>>  	 * Archs like ppc64 use pgtable to store per pmd
>>
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 
> 

^ permalink raw reply

* [PATCH 01/18] regulator: max14577: Remove unused state container definition
From: Mark Brown @ 2014-01-28 16:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390911522-28209-2-git-send-email-k.kozlowski@samsung.com>

On Tue, Jan 28, 2014 at 01:18:25PM +0100, Krzysztof Kozlowski wrote:
> Remove the "struct max14577_regulator" because this is not used
> anywhere. It should be removed earlier along with changing the driver
> after review on the mailing lists.

Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140128/f46a547b/attachment.sig>

^ permalink raw reply

* [PATCH V2] arm64: add DSB after icache flush in __flush_icache_all()
From: Will Deacon @ 2014-01-28 16:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390892813-30407-1-git-send-email-vkale@apm.com>

On Tue, Jan 28, 2014 at 07:06:53AM +0000, Vinayak Kale wrote:
> Add DSB after icache flush. It's needed to complete the cache maintenance
> operation. The function __flush_icache_all() is used only for user space
> mappings and an ISB is not required because of an exception return before
> executing user instructions. An exception return would behave like an ISB.
> 
> This patch also uses 'memory' clobber for flush operation instruction to 
> prevent instruction re-ordering by compiler. 
> 
> Signed-off-by: Vinayak Kale <vkale@apm.com>
> ---
> 
> V2: - Add more desciption in the commit message as suggested by Catalin & Will
>     - Use 'memory' clobber for flush instruction as suggested by Will

Please can you check and fix other occurrences of this bug too, as I asked
in v1? For example, a 2 second grep shows problems with data-cache
maintenance in kvm. I can also see the same problem for system register
writes followed up with isb.

I also don't buy you not being able to test AArch32 kernels. Does KVM not
work for you?

Will

> 
>  arch/arm64/include/asm/cacheflush.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h
> index fea9ee3..bd30f42 100644
> --- a/arch/arm64/include/asm/cacheflush.h
> +++ b/arch/arm64/include/asm/cacheflush.h
> @@ -115,7 +115,8 @@ extern void flush_dcache_page(struct page *);
>  
>  static inline void __flush_icache_all(void)
>  {
> -	asm("ic	ialluis");
> +	asm volatile("ic ialluis" : : : "memory");
> +	dsb();
>  }
>  
>  #define flush_dcache_mmap_lock(mapping) \
> -- 
> 1.8.2.1
> 
> 

^ permalink raw reply

* [PATCH] ARM: multi_v7: add mvebu and drivers to defconfig
From: Kevin Hilman @ 2014-01-28 15:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52E7CF49.8040703@free-electrons.com>

On Tue, Jan 28, 2014 at 7:39 AM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> On 28/01/2014 16:17, Kevin Hilman wrote:
>> Jason Cooper <jason@lakedaemon.net> writes:
>>
>>> boot farms testing is highlighting some errors in mvebu.  Let's get some
>>> more coverage for multi_v7_defconfig kernels.
>>>
>>> Signed-off-by: Jason Cooper <jason@lakedaemon.net>
>>> ---
>>> Kevin,
>>>
>>> This is against tonight's master.  Feel free to take it directly.
>>>
>>
>> OK, but this one doesn't currently fix any known errors, right?  (the
>> pull request you sent yesterday fixes the boot regressions, IIUC.)  The
>> goal here is just to get more mvebu coverage in multi_v7_defconfig,
>> correct?
>>
>> That being said, have a closer look at the Kconfig entries that are
>> removed by the patch below, many of which will have an impact on other
>> platforms.
>
> Hi Kevin,
>
> I may be wrong but I suspect this config file was generated with make savedefconfig.
> this rule only generate the CONFIG that are not implied by other CONFIG,
> so I wonder if all this changes are just due to of modificatino in the Kconfig
> themselves.

Yes, I suspect that as well, which means the changelog needs to have a
bit more detail justifying those removals that look suspicious when
just looking through the diff.

Kevin

^ permalink raw reply

* [PATCH 2/2] arm64: kernel: use seq_puts() instead of seq_printf()
From: Catalin Marinas @ 2014-01-28 15:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <001f01cf1bc9$57767070$06635150$%han@samsung.com>

On Tue, Jan 28, 2014 at 01:36:18AM +0000, Jingoo Han wrote:
> For a constant format without additional arguments, use seq_puts()
> instead of seq_printf(). Also, it fixes the following checkpatch
> warning.
> 
>   WARNING: Prefer seq_puts to seq_printf
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>  arch/arm64/kernel/setup.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index c8e9eff..4507691 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -416,7 +416,7 @@ static int c_show(struct seq_file *m, void *v)
>  			seq_printf(m, "%s ", hwcap_str[i]);
>  
>  	seq_printf(m, "\nCPU implementer\t: 0x%02x\n", read_cpuid_id() >> 24);
> -	seq_printf(m, "CPU architecture: AArch64\n");
> +	seq_puts(m, "CPU architecture: AArch64\n");
>  	seq_printf(m, "CPU variant\t: 0x%x\n", (read_cpuid_id() >> 20) & 15);
>  	seq_printf(m, "CPU part\t: 0x%03x\n", (read_cpuid_id() >> 4) & 0xfff);
>  	seq_printf(m, "CPU revision\t: %d\n", read_cpuid_id() & 15);

Just ignore the checkpatch warning. I prefer the consistency of
seq_printf() in this function.

-- 
Catalin

^ permalink raw reply

* [PATCH] ARM: multi_v7: add mvebu and drivers to defconfig
From: Gregory CLEMENT @ 2014-01-28 15:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <8761p4w2yu.fsf@paris.lan>

On 28/01/2014 16:17, Kevin Hilman wrote:
> Jason Cooper <jason@lakedaemon.net> writes:
> 
>> boot farms testing is highlighting some errors in mvebu.  Let's get some
>> more coverage for multi_v7_defconfig kernels.
>>
>> Signed-off-by: Jason Cooper <jason@lakedaemon.net>
>> ---
>> Kevin,
>>
>> This is against tonight's master.  Feel free to take it directly.
>>
> 
> OK, but this one doesn't currently fix any known errors, right?  (the
> pull request you sent yesterday fixes the boot regressions, IIUC.)  The
> goal here is just to get more mvebu coverage in multi_v7_defconfig,
> correct?
> 
> That being said, have a closer look at the Kconfig entries that are
> removed by the patch below, many of which will have an impact on other
> platforms.

Hi Kevin,

I may be wrong but I suspect this config file was generated with make savedefconfig.
this rule only generate the CONFIG that are not implied by other CONFIG,
so I wonder if all this changes are just due to of modificatino in the Kconfig
themselves.

For example the config symbol TEGRA_PCI have been removed since v3.12.

Gregory






> 
> Kevin
> 
>>  arch/arm/configs/multi_v7_defconfig | 28 +++++++++++++++++-----------
>>  1 file changed, 17 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
>> index 687e4e811b2a..13e7d649230b 100644
>> --- a/arch/arm/configs/multi_v7_defconfig
>> +++ b/arch/arm/configs/multi_v7_defconfig
>> @@ -38,7 +38,6 @@ CONFIG_ARCH_TEGRA=y
>>  CONFIG_ARCH_TEGRA_2x_SOC=y
>>  CONFIG_ARCH_TEGRA_3x_SOC=y
>>  CONFIG_ARCH_TEGRA_114_SOC=y
>> -CONFIG_TEGRA_PCI=y
> 
> huh?
> 
>>  CONFIG_TEGRA_EMC_SCALING_ENABLE=y
>>  CONFIG_ARCH_U8500=y
>>  CONFIG_MACH_HREFV60=y
>> @@ -49,7 +48,9 @@ CONFIG_ARCH_VEXPRESS_CA9X4=y
>>  CONFIG_ARCH_VIRT=y
>>  CONFIG_ARCH_WM8850=y
>>  CONFIG_ARCH_ZYNQ=y
>> -CONFIG_SMP=y
> 
> huh?
> 
>> +CONFIG_PCI=y
>> +CONFIG_PCI_MSI=y
>> +CONFIG_PCI_MVEBU=y
>>  CONFIG_HIGHPTE=y
>>  CONFIG_ARM_APPENDED_DTB=y
>>  CONFIG_ARM_ATAG_DTB_COMPAT=y
>> @@ -69,12 +70,14 @@ CONFIG_SATA_MV=y
>>  CONFIG_NETDEVICES=y
>>  CONFIG_SUN4I_EMAC=y
>>  CONFIG_NET_CALXEDA_XGMAC=y
>> +CONFIG_MVNETA=y
>>  CONFIG_KS8851=y
>>  CONFIG_SMSC911X=y
>>  CONFIG_STMMAC_ETH=y
>> -CONFIG_ICPLUS_PHY=y
>> -CONFIG_MDIO_SUN4I=y
>>  CONFIG_TI_CPSW=y
>> +CONFIG_MARVELL_PHY=y
>> +CONFIG_ICPLUS_PHY=y
>> +CONFIG_KEYBOARD_GPIO=y
>>  CONFIG_KEYBOARD_SPEAR=y
>>  CONFIG_SERIO_AMBAKMI=y
>>  CONFIG_SERIAL_8250=y
>> @@ -99,25 +102,27 @@ CONFIG_SERIAL_FSL_LPUART_CONSOLE=y
>>  CONFIG_SERIAL_ST_ASC=y
>>  CONFIG_SERIAL_ST_ASC_CONSOLE=y
>>  CONFIG_I2C_DESIGNWARE_PLATFORM=y
>> +CONFIG_I2C_MV64XXX=y
>>  CONFIG_I2C_SIRF=y
>>  CONFIG_I2C_TEGRA=y
>>  CONFIG_SPI=y
>>  CONFIG_SPI_OMAP24XX=y
>> +CONFIG_SPI_ORION=y
>>  CONFIG_SPI_PL022=y
>>  CONFIG_SPI_SIRF=y
>>  CONFIG_SPI_TEGRA114=y
>>  CONFIG_SPI_TEGRA20_SLINK=y
>> -CONFIG_PINCTRL_SINGLE=y
>>  CONFIG_GPIO_GENERIC_PLATFORM=y
>>  CONFIG_GPIO_TWL4030=y
>> -CONFIG_REGULATOR_GPIO=y
>> +CONFIG_THERMAL=y
>> +CONFIG_ARMADA_THERMAL=y
>>  CONFIG_REGULATOR_AB8500=y
>> +CONFIG_REGULATOR_GPIO=y
>>  CONFIG_REGULATOR_TPS51632=y
>>  CONFIG_REGULATOR_TPS62360=y
>>  CONFIG_REGULATOR_TWL4030=y
>>  CONFIG_REGULATOR_VEXPRESS=y
>>  CONFIG_DRM=y
>> -CONFIG_TEGRA_HOST1X=y
> 
> huh?
> 
>>  CONFIG_DRM_TEGRA=y
>>  CONFIG_FB_ARMCLCD=y
>>  CONFIG_FB_WM8505=y
>> @@ -132,8 +137,6 @@ CONFIG_USB_STORAGE=y
>>  CONFIG_USB_CHIPIDEA=y
>>  CONFIG_USB_CHIPIDEA_HOST=y
>>  CONFIG_AB8500_USB=y
>> -CONFIG_NOP_USB_XCEIV=y
>> -CONFIG_OMAP_USB2=y
> 
> huh?
> 
>>  CONFIG_OMAP_USB3=y
>>  CONFIG_SAMSUNG_USB2PHY=y
>>  CONFIG_SAMSUNG_USB3PHY=y
>> @@ -144,13 +147,13 @@ CONFIG_MMC=y
>>  CONFIG_MMC_BLOCK_MINORS=16
>>  CONFIG_MMC_ARMMMCI=y
>>  CONFIG_MMC_SDHCI=y
>> -CONFIG_MMC_SDHCI_PLTFM=y
>>  CONFIG_MMC_SDHCI_ESDHC_IMX=y
>>  CONFIG_MMC_SDHCI_TEGRA=y
>>  CONFIG_MMC_SDHCI_SPEAR=y
>>  CONFIG_MMC_SDHCI_BCM_KONA=y
>>  CONFIG_MMC_OMAP=y
>>  CONFIG_MMC_OMAP_HS=y
>> +CONFIG_MMC_MVSDIO=y
>>  CONFIG_EDAC=y
>>  CONFIG_EDAC_MM_EDAC=y
>>  CONFIG_EDAC_HIGHBANK_MC=y
>> @@ -159,20 +162,23 @@ CONFIG_RTC_CLASS=y
>>  CONFIG_RTC_DRV_TWL4030=y
>>  CONFIG_RTC_DRV_PL031=y
>>  CONFIG_RTC_DRV_VT8500=y
>> +CONFIG_RTC_DRV_MV=y
>>  CONFIG_RTC_DRV_TEGRA=y
>>  CONFIG_DMADEVICES=y
>>  CONFIG_DW_DMAC=y
>> +CONFIG_MV_XOR=y
>>  CONFIG_TEGRA20_APB_DMA=y
>>  CONFIG_STE_DMA40=y
>>  CONFIG_SIRF_DMA=y
>> -CONFIG_TI_EDMA=y
>>  CONFIG_PL330_DMA=y
>>  CONFIG_IMX_SDMA=y
>>  CONFIG_IMX_DMA=y
>>  CONFIG_MXS_DMA=y
>>  CONFIG_DMA_OMAP=y
>> +CONFIG_MEMORY=y
>>  CONFIG_PWM=y
>>  CONFIG_PWM_VT8500=y
>> +CONFIG_OMAP_USB2=y
>>  CONFIG_EXT4_FS=y
>>  CONFIG_TMPFS=y
>>  CONFIG_NFS_FS=y


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
From: Heikki Krogerus @ 2014-01-28 15:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140127160458.GF13268@saruman.home>

Hi,

On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote:
> > Why would you need to know if the PHY drivers are needed or not
> > explicitly in your controller driver?
> 
> because, one way or another, they all do need it. Except for quirky ones
> like AM437x where a USB3 IP was hardwired into USB2-only mode, so it
> really lacks a USB3 PHY.

The Baytrail board I deal with has completely autonomous PHYs. But my
question is, why would you need to care about the PHYs in your
controller driver? Why can't you leave the responsibility of them to
the upper layers and trust what they tell you?

If there is no USB3 PHY for dwc3 then, the driver gets something like
-ENODEV and just continues. There is no need to know about the
details.

For the controller drivers the PHYs are just a resource like any
other. The controller drivers can't have any responsibility of
them. They should not care if PHY drivers are available for them or
not, or even if the PHY framework is available or not.

> > > But I really want to see the argument against using no-op. As far as I
> > > could see, everybody needs a PHY driver one way or another, some
> > > platforms just haven't sent any PHY driver upstream and have their own
> > > hacked up solution to avoid using the PHY layer.
> > 
> > Not true in our case. Platforms using Intel's SoCs and chip sets may
> > or may not have controllable USB PHY. Quite often they don't. The
> > Baytrails have usually ULPI PHY for USB2, but that does not mean they
> > provide any vendor specific functions or any need for a driver in any
> > case.
> 
> that's different from what I heard.

I don't know where you got that impression, but it's not true. The
Baytrail SoCs for example don't have internal USB PHYs, which means
the manufacturers using it can select what they want. So we have
boards where PHY driver(s) is needed and boards where it isn't.

The problem is that we just don't always know all the details about
the platform. If the PHY is ULPI PHY, we can do runtime detection, but
we can't even rely on always having ULPI.

> > Are we talking about the old USB PHY library or the new PHY framework
> > with the no-op PHY driver?
> > 
> > Well, in any case, I don't understand what is the purpose of the no-op
> > PHY driver. What are you drying to achieve with that?
> 
> I'm trying to avoid supporting 500 different combinations for a single
> driver. We already support old USB PHY layer and generic PHY layer, now
> they both need to be made optional.

This is really good to get. We have some projects where we are dealing
with more embedded environments, like IVI, where the kernel should be
stripped of everything useless. Since the PHYs are autonomous, we
should be able to disable the PHY libraries/frameworks.

But I still don't understand what is the benefit in the no-op? You
really need to explain this. What you have now in dwc3 is expectations
regarding the PHY, which put a lot of pressure on upper layers to
satisfy the driver. The no-op is just an extra thing that you have to
provide when you fail to determine if the system requires a PHY driver
or not, or if you know that it doesn't, plus an additional check for
the case where the PHY lib/framework is not enabled. I don't see the
value in it.

> The old USB PHY layer also provides
> a no-op, now called phy-generic, which is actually pretty useful.
> 
> On top of all that, I'm sure you'll subscribe to the idea of preventing
> dwc3 from becoming another MUSB which supports several different
> configurations and none work correctly. I much prefer to take baby steps
> which are well thought-out and very well exercised, so I'll be very
> pedantic about proof of testing.

I think our goals are the same. I just want to also minimize the need
for any platform specific extra work from the upper layers regarding
the PHYs.


Thanks,

-- 
heikki

^ permalink raw reply

* [RFC] drivers: irq-chip: Enable SGI's for Secure-to-NonSecure communication use-cases
From: Bill Pringlemeir @ 2014-01-28 15:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <79CD15C6BA57404B839C016229A409A83EDC1E71@DBDE04.ent.ti.com>


>>> You have any other alternative?

>> From: Bill Pringlemeir [mailto:bpringlemeir at nbsps.com]

>> I think you need to put Bhupesh Sharma's comment with this.  The
>> typical sane mode for GIC with TZ is to have the monitor mode toggle
>> the IRQ/FIQ routing bits in the SCR (cp15) bits 1,2.  That is, the
>> IRQ goes direct to core and FIQ goes to monitor from the 'Normal'
>> world.  In the 'Secure' world, the FIQ goes direct to core and IRQ
>> traps to monitor.  The monitor mode vector table has a gateway from
>> secure to normal for IRQ and gateway from normal to secure for FIQ.
>>
>> Now, consider the 'SMC' instruction and what is present in stuff like
>> this,
>>
>> http://lwn.net/Articles/513756/

>> Instead of messing around with the GIC, why not use something even
>> more generic like the 'SMC' instruction.  It has the same sort of
>> 'end game' which is a trap to monitor mode.  The monitor has to be a
>> little smarter to determine which world called but this should always
>> be the case; really you want to check this.
>>
>> Btw, the situation is the same no matter which world Linux is in.  I
>> don't think Linux can be the recipient of an 'SMC' call.  But I think
>> most use cases would put it in the 'normal world' and the SMC is
>> fine.

On 28 Jan 2014, hvaibhav at ti.com wrote:

> May be I am missing something here,

> I find your above two statements contradictory,

> If we want to use SMC as you mentioned, and assuming Secure Monitor
> mode is intelligent enough to determine the calling world (whether
> secure or non-secure), then without Linux being recipient (in any
> world) of an 'SMC' call how can realtime switch possible from secure
> world to non-secure world??

I don't think that using an SMC in either world is any different than
writing to the GIC distributors SGIR?  So, the generation is the same.
The monitor switching should be the same.  The only difference is where
does the world switch ends up.

> Just to clarify,

> The need here is, to switch from secure world to non-secure world on
> any realtime (multiple) hardware events, which in turn gets
> processed/handled in non-secure world.  In certain cases even we do
> not want non-secure world know about the hardware event. In this case,
> the Processing of hardware event completely happens in secure world,
> and different event/trigger/info/message goes to non-secure world.

Ok.  Your needs are backwards to my understanding.  You want the
'secure/non-linux' to send an SGI to the Linux.  Ie, you want the
recipient case for the normal world.  The normal world can always issue
an SMC during boot to register a recipient mechanism.  You can fake
another interrupt chip or use some other mechanism.

> So just manipulating IRQ/FIQ routing will not solve the need here. :)

I didn't mean this would solve your problem.  I meant this keeps the
latency to a minimum when using TrustZone.  If an 'interrupt' (FIQ or
IRQ) source occurs in the destination world, then the path is as per
usual.  It is only when an 'interrupt' occurs in the opposite world when
a world switch is needed; this is a fairly expensive operation.

In your case, it sounds like you want the secure world to handle all
interrupts or at least the majority.  This has a pre-defined destination
in a jump to the vector table of the opposite world.  This is the
'recipient' mechanism.  At the very least a 'multi-chip' IRQ could be
used and you can ldrex/strex with world shareable memory to communicate
interrupt sources to the other world; the world shareable memory acts as
a 'IRQ controller register set'.  With the SMC, you have the opportunity
to transfer some information in registers.  It has a bigger question of
how the schedulers would inter-act.  Do you have a scheduler in the
secure world?

I think these are 'alternatives' as you asked?

Regards,
Bill Pringlemeir.

^ permalink raw reply

* [PATCH] ARM: multi_v7: add mvebu and drivers to defconfig
From: Kevin Hilman @ 2014-01-28 15:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390871129-4838-1-git-send-email-jason@lakedaemon.net>

Jason Cooper <jason@lakedaemon.net> writes:

> boot farms testing is highlighting some errors in mvebu.  Let's get some
> more coverage for multi_v7_defconfig kernels.
>
> Signed-off-by: Jason Cooper <jason@lakedaemon.net>
> ---
> Kevin,
>
> This is against tonight's master.  Feel free to take it directly.
>

OK, but this one doesn't currently fix any known errors, right?  (the
pull request you sent yesterday fixes the boot regressions, IIUC.)  The
goal here is just to get more mvebu coverage in multi_v7_defconfig,
correct?

That being said, have a closer look at the Kconfig entries that are
removed by the patch below, many of which will have an impact on other
platforms.

Kevin

>  arch/arm/configs/multi_v7_defconfig | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> index 687e4e811b2a..13e7d649230b 100644
> --- a/arch/arm/configs/multi_v7_defconfig
> +++ b/arch/arm/configs/multi_v7_defconfig
> @@ -38,7 +38,6 @@ CONFIG_ARCH_TEGRA=y
>  CONFIG_ARCH_TEGRA_2x_SOC=y
>  CONFIG_ARCH_TEGRA_3x_SOC=y
>  CONFIG_ARCH_TEGRA_114_SOC=y
> -CONFIG_TEGRA_PCI=y

huh?

>  CONFIG_TEGRA_EMC_SCALING_ENABLE=y
>  CONFIG_ARCH_U8500=y
>  CONFIG_MACH_HREFV60=y
> @@ -49,7 +48,9 @@ CONFIG_ARCH_VEXPRESS_CA9X4=y
>  CONFIG_ARCH_VIRT=y
>  CONFIG_ARCH_WM8850=y
>  CONFIG_ARCH_ZYNQ=y
> -CONFIG_SMP=y

huh?

> +CONFIG_PCI=y
> +CONFIG_PCI_MSI=y
> +CONFIG_PCI_MVEBU=y
>  CONFIG_HIGHPTE=y
>  CONFIG_ARM_APPENDED_DTB=y
>  CONFIG_ARM_ATAG_DTB_COMPAT=y
> @@ -69,12 +70,14 @@ CONFIG_SATA_MV=y
>  CONFIG_NETDEVICES=y
>  CONFIG_SUN4I_EMAC=y
>  CONFIG_NET_CALXEDA_XGMAC=y
> +CONFIG_MVNETA=y
>  CONFIG_KS8851=y
>  CONFIG_SMSC911X=y
>  CONFIG_STMMAC_ETH=y
> -CONFIG_ICPLUS_PHY=y
> -CONFIG_MDIO_SUN4I=y
>  CONFIG_TI_CPSW=y
> +CONFIG_MARVELL_PHY=y
> +CONFIG_ICPLUS_PHY=y
> +CONFIG_KEYBOARD_GPIO=y
>  CONFIG_KEYBOARD_SPEAR=y
>  CONFIG_SERIO_AMBAKMI=y
>  CONFIG_SERIAL_8250=y
> @@ -99,25 +102,27 @@ CONFIG_SERIAL_FSL_LPUART_CONSOLE=y
>  CONFIG_SERIAL_ST_ASC=y
>  CONFIG_SERIAL_ST_ASC_CONSOLE=y
>  CONFIG_I2C_DESIGNWARE_PLATFORM=y
> +CONFIG_I2C_MV64XXX=y
>  CONFIG_I2C_SIRF=y
>  CONFIG_I2C_TEGRA=y
>  CONFIG_SPI=y
>  CONFIG_SPI_OMAP24XX=y
> +CONFIG_SPI_ORION=y
>  CONFIG_SPI_PL022=y
>  CONFIG_SPI_SIRF=y
>  CONFIG_SPI_TEGRA114=y
>  CONFIG_SPI_TEGRA20_SLINK=y
> -CONFIG_PINCTRL_SINGLE=y
>  CONFIG_GPIO_GENERIC_PLATFORM=y
>  CONFIG_GPIO_TWL4030=y
> -CONFIG_REGULATOR_GPIO=y
> +CONFIG_THERMAL=y
> +CONFIG_ARMADA_THERMAL=y
>  CONFIG_REGULATOR_AB8500=y
> +CONFIG_REGULATOR_GPIO=y
>  CONFIG_REGULATOR_TPS51632=y
>  CONFIG_REGULATOR_TPS62360=y
>  CONFIG_REGULATOR_TWL4030=y
>  CONFIG_REGULATOR_VEXPRESS=y
>  CONFIG_DRM=y
> -CONFIG_TEGRA_HOST1X=y

huh?

>  CONFIG_DRM_TEGRA=y
>  CONFIG_FB_ARMCLCD=y
>  CONFIG_FB_WM8505=y
> @@ -132,8 +137,6 @@ CONFIG_USB_STORAGE=y
>  CONFIG_USB_CHIPIDEA=y
>  CONFIG_USB_CHIPIDEA_HOST=y
>  CONFIG_AB8500_USB=y
> -CONFIG_NOP_USB_XCEIV=y
> -CONFIG_OMAP_USB2=y

huh?

>  CONFIG_OMAP_USB3=y
>  CONFIG_SAMSUNG_USB2PHY=y
>  CONFIG_SAMSUNG_USB3PHY=y
> @@ -144,13 +147,13 @@ CONFIG_MMC=y
>  CONFIG_MMC_BLOCK_MINORS=16
>  CONFIG_MMC_ARMMMCI=y
>  CONFIG_MMC_SDHCI=y
> -CONFIG_MMC_SDHCI_PLTFM=y
>  CONFIG_MMC_SDHCI_ESDHC_IMX=y
>  CONFIG_MMC_SDHCI_TEGRA=y
>  CONFIG_MMC_SDHCI_SPEAR=y
>  CONFIG_MMC_SDHCI_BCM_KONA=y
>  CONFIG_MMC_OMAP=y
>  CONFIG_MMC_OMAP_HS=y
> +CONFIG_MMC_MVSDIO=y
>  CONFIG_EDAC=y
>  CONFIG_EDAC_MM_EDAC=y
>  CONFIG_EDAC_HIGHBANK_MC=y
> @@ -159,20 +162,23 @@ CONFIG_RTC_CLASS=y
>  CONFIG_RTC_DRV_TWL4030=y
>  CONFIG_RTC_DRV_PL031=y
>  CONFIG_RTC_DRV_VT8500=y
> +CONFIG_RTC_DRV_MV=y
>  CONFIG_RTC_DRV_TEGRA=y
>  CONFIG_DMADEVICES=y
>  CONFIG_DW_DMAC=y
> +CONFIG_MV_XOR=y
>  CONFIG_TEGRA20_APB_DMA=y
>  CONFIG_STE_DMA40=y
>  CONFIG_SIRF_DMA=y
> -CONFIG_TI_EDMA=y
>  CONFIG_PL330_DMA=y
>  CONFIG_IMX_SDMA=y
>  CONFIG_IMX_DMA=y
>  CONFIG_MXS_DMA=y
>  CONFIG_DMA_OMAP=y
> +CONFIG_MEMORY=y
>  CONFIG_PWM=y
>  CONFIG_PWM_VT8500=y
> +CONFIG_OMAP_USB2=y
>  CONFIG_EXT4_FS=y
>  CONFIG_TMPFS=y
>  CONFIG_NFS_FS=y

^ permalink raw reply

* [PATCH v2 1/7] cpufreq: cpufreq-cpu0: allow optional safe voltage during frequency transitions
From: Thomas Abraham @ 2014-01-28 15:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140128160633.470202ec@amdc2363>

Hi Lukasz,

On Tue, Jan 28, 2014 at 8:36 PM, Lukasz Majewski <l.majewski@samsung.com> wrote:
> Hi Thomas,
>
>> Hi Lukasz,
>>
>> On Tue, Jan 28, 2014 at 1:47 PM, Lukasz Majewski
>> <l.majewski@samsung.com> wrote:
>> > Hi Thomas, Mike
>> >
>> >> Hi Mike,
>> >>
>> >> On Tue, Jan 28, 2014 at 1:55 AM, Mike Turquette
>> >> <mturquette@linaro.org> wrote:
>> >> > Quoting Thomas Abraham (2014-01-18 04:10:51)
>> >> >> From: Thomas Abraham <thomas.ab@samsung.com>
>> >> >>
>> >> >> On some platforms such as the Samsung Exynos, changing the
>> >> >> frequency of the CPU clock requires changing the frequency of
>> >> >> the PLL that is supplying the CPU clock. To change the
>> >> >> frequency of the PLL, the CPU clock is temporarily reparented
>> >> >> to another parent clock.
>> >> >>
>> >> >> The clock frequency of this temporary parent clock could be much
>> >> >> higher than the clock frequency of the PLL at the time of
>> >> >> reparenting. Due to the temporary increase in the CPU clock
>> >> >> speed, the CPU (and any other components in the CPU clock
>> >> >> domain such as dividers, mux, etc.) have to to be operated at a
>> >> >> higher voltage level, called the safe voltage level. This patch
>> >> >> adds optional support to temporarily switch to a safe voltage
>> >> >> level during CPU frequency transitions.
>> >> >>
>> >> >> Cc: Shawn Guo <shawn.guo@linaro.org>
>> >> >> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
>> >> >
>> >> > I'm not a fan of this change. This corner case should be
>> >> > abstracted away somehow. I had talked to Chander Kayshap
>> >> > previously about handling voltage changes in clock notifier
>> >> > callbacks, which then renders any voltage change as a trivial
>> >> > part of the clock rate transition. That means that this "safe
>> >> > voltage" thing could be handled automagically without any
>> >> > additional code in the CPUfreq driver.
>> >> >
>> >> > There are two nice ways to do this with the clock framework.
>> >> > First is explicit re-parenting with voltage scaling done in the
>> >> > clock rate-change notifiers:
>> >> >
>> >> > clk_set_parent(cpu_clk, temp_parent);
>> >> > /* implicit voltage scaling to "safe voltage" happens above */
>> >> > clk_set_rate(pll, some_rate);
>> >> > clk_set_parent(cpu_clk, pll);
>> >> > /* implicit voltage scaling to nominal OPP voltage happens above
>> >> > */
>> >> >
>> >
>> > I must agree with Mike here. In my opinion the above approach is
>> > more compliant with CCF (as I've pointed it out in my other comment
>> > - the cpu_clk has more than one parent and we could switch between
>> > them when needed).
>>
>> The mux which is used to re-parent is part of the larger opaque cpu
>> clock type (more like the composite clock). So I am not sure how this
>> isn't compliant with ccf.
>
> My point here is to use the clk_set_parent() explicitly instead of
> changing the mux with writing values directly to registers.
>
> However, I'm also aware, that we must reparent quickly. so I'm OK
> with your approach.

Okay.

>
>>
>> >
>> >> > The above sequence would require a separate exnyos CPUfreq
>> >> > driver, due to the added clk_set_parent logic.
>> >> >
>> >> > The second way to do this is to abstract the clk re-muxing logic
>> >> > out into the clk driver, which would allow cpufreq-cpu0 to be
>> >> > used for the exynos chips.
>> >>
>> >> This is the approach this patch series takes (patch 2/7). The clock
>> >> re-muxing logic is handled by a clock driver code. The difference
>> >> from what you suggested is that the safe voltage (that may be
>> >> optionally) required before doing the re-muxing is handled here in
>> >> cpufreq-cpu0 driver.
>> >>
>> >> The safe voltage setup can be done in the notifier as you
>> >> suggested.
>> >
>> > If the clk_set_parent() approach is not suitable, then cannot we
>> > consider using the one from highbank-cpufreq.c?
>> >
>> > Here we have cpufreq-cpu0.c which sets voltage of the cpu_clk.
>> > In the highbank-cpufreq.c there are clock notifiers to change the
>> > voltage.
>> >
>> > Cannot Exynos reuse such approach? Why shall we pollute
>> > cpufreq-cpu0.c with another solution?
>>
>> The highbank-cpufreq.c file was introduced because platforms using
>> this driver did not have the usual regulator to control the voltage.
>> The first commit of this driver explains this (copied below).
>>
>> "Highbank processors depend on the external ECME to perform voltage
>> management based on a requested frequency. Communication between the
>> A9 cores and the ECME happens over the pl320 IPC channel."
>>
>> So those platforms had no choice but to use an alternative approach to
>> control the voltage (and reuse cpufreq-cpu0 as much as possible).
>> The
>> case with exynos is a different one.
>
> Highbank needs to set voltage via IPC, Exynos needs to adjust voltage
> when reparenting.
>
> Both can be recognized as unusual cases. That is why I asked if we
> could reuse the same approach for Exynos.

Okay.

>
>> cpufreq-cpu0 is fully re-usable
>> for exynos with the additional support for "safe voltage". If we agree
>> that there might be existing or future platforms with single
>> clock/voltage rail that require the "safe voltage" feature, then
>> adding "safe voltage" support in cpufreq-cpu0 driver seems to be the
>> right approach.
>
> I think that Shawn's opinion will be final here.
>
>>
>> >
>> >> But, doing that in cpufreq-cpu0 driver will help other platforms
>> >> reuse this feature if required. Also, if done here, the regulator
>> >> handling is localized in this driver which otherwise would need to
>> >> be handled in two places, cpufreq-cpu0 driver and the clock
>> >> notifier.
>> >
>> > I think that there is a logical distinction between setting voltage
>> > for cpufreq-cpu0 related clock and increasing voltage of reparented
>> > clock.
>> >
>> > The former fits naturally to cpufreq-cpu0, when the latter seems
>> > like some corner case (as Mike pointed out) for Exynos.
>>
>> Agreed, it is a corner case. But for this corner case, we are
>> performing some additional actions on the same regulator which is used
>> in the normal functioning of the driver.
>>
>> >
>> >>
>> >> So I tend to prefer the approach in this patch but I am willing to
>> >> consider any suggestions.
>> >
>> > Thomas, what do you think about highbank-cpufreq.c approach (with
>> > using clock notifiers)?
>>
>> I have made a related comment on this above.
>>
>> >
>> > Do you think, that it is feasible to reuse it with Exynos?
>>
>> highbank cpufreq driver is intended for a different purpose so I don't
>> think it can be reused for exynos. Yes, we can make exynos specific
>> hacks into highbank driver but how would that be better over the
>> approach in this patch?
>
> I think, that I was misunderstood here. I wanted to ask if we could
> reuse the clk notifier approach.

Okay, I misunderstood your comment. We could do something similar to
highbank cpufreq driver for exynos as well. Anyways, Shawn prefers not
to add "safe voltage" support into cpufreq-cpu0 driver. So we need to
look for other options.

Thanks,
Thomas.

>
>>
>> >
>> >> Shawn, it would be helpful if you could let
>> >> us know your thoughts on this. I am almost done with testing the
>> >> v3 of this series and want to post it so if there are any
>> >> objections to the changes in this patch, please let me know.
>> >>
>> >> Thanks,
>> >> Thomas.
>> >>
>> >> >
>> >> > I'm more a fan of explicitly listing the Exact Steps for the cpu
>> >> > opp transition in a separate exynos-specific CPUfreq driver, but
>> >> > that's probably an unpopular view.
>> >> >
>> >> > Regards,
>> >> > Mike
>> >> >
>> >> >> ---
>> >> >>  .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   |    7 ++++
>> >> >>  drivers/cpufreq/cpufreq-cpu0.c                     |   37
>> >> >> +++++++++++++++++-- 2 files changed, 40 insertions(+), 4
>> >> >> deletions(-)
>> >> >>
>> >> >> diff --git
>> >> >> a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>> >> >> b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>> >> >> index f055515..37453ab 100644 ---
>> >> >> a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt +++
>> >> >> b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt @@
>> >> >> -19,6 +19,12 @@ Optional properties:
>> >> >>  - cooling-min-level:
>> >> >>  - cooling-max-level:
>> >> >>       Please refer to
>> >> >> Documentation/devicetree/bindings/thermal/thermal.txt. +-
>> >> >> safe-opp: Certain platforms require that during a opp
>> >> >> transition,
>> >> >> +  a system should not go below a particular opp level. For such
>> >> >> systems,
>> >> >> +  this property specifies the minimum opp to be maintained
>> >> >> during the
>> >> >> +  opp transitions. The safe-opp value is a tuple with first
>> >> >> element
>> >> >> +  representing the safe frequency and the second element
>> >> >> representing the
>> >> >> +  safe voltage.
>> >> >>
>> >> >>  Examples:
>> >> >>
>> >> >> @@ -36,6 +42,7 @@ cpus {
>> >> >>                         396000  950000
>> >> >>                         198000  850000
>> >> >>                 >;
>> >> >> +               safe-opp = <396000 950000>
>> >> >>                 clock-latency = <61036>; /* two CLK32 periods */
>> >> >>                 #cooling-cells = <2>;
>> >> >>                 cooling-min-level = <0>;
>> >> >> diff --git a/drivers/cpufreq/cpufreq-cpu0.c
>> >> >> b/drivers/cpufreq/cpufreq-cpu0.c index 0c12ffc..075d3d1 100644
>> >> >> --- a/drivers/cpufreq/cpufreq-cpu0.c
>> >> >> +++ b/drivers/cpufreq/cpufreq-cpu0.c
>> >> >> @@ -27,6 +27,8 @@
>> >> >>
>> >> >>  static unsigned int transition_latency;
>> >> >>  static unsigned int voltage_tolerance; /* in percentage */
>> >> >> +static unsigned long safe_frequency;
>> >> >> +static unsigned long safe_voltage;
>> >> >>
>> >> >>  static struct device *cpu_dev;
>> >> >>  static struct clk *cpu_clk;
>> >> >> @@ -64,17 +66,30 @@ static int cpu0_set_target(struct
>> >> >> cpufreq_policy *policy, unsigned int index) volt_old =
>> >> >> regulator_get_voltage(cpu_reg); }
>> >> >>
>> >> >> -       pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
>> >> >> +       pr_debug("\n\n%u MHz, %ld mV --> %u MHz, %ld mV\n",
>> >> >>                  old_freq / 1000, volt_old ? volt_old / 1000 :
>> >> >> -1, new_freq / 1000, volt ? volt / 1000 : -1);
>> >> >>
>> >> >>         /* scaling up?  scale voltage before frequency */
>> >> >> -       if (!IS_ERR(cpu_reg) && new_freq > old_freq) {
>> >> >> +       if (!IS_ERR(cpu_reg) && new_freq > old_freq &&
>> >> >> +                               new_freq >= safe_frequency) {
>> >> >>                 ret = regulator_set_voltage_tol(cpu_reg, volt,
>> >> >> tol); if (ret) {
>> >> >>                         pr_err("failed to scale voltage up:
>> >> >> %d\n", ret); return ret;
>> >> >>                 }
>> >> >> +       } else if (!IS_ERR(cpu_reg) && old_freq <
>> >> >> safe_frequency) {
>> >> >> +               /*
>> >> >> +                * the scaled up voltage level for the new_freq
>> >> >> is lower
>> >> >> +                * than the safe voltage level. so set
>> >> >> safe_voltage
>> >> >> +                * as the intermediate voltage level and revert
>> >> >> it
>> >> >> +                * back after the frequency has been changed.
>> >> >> +                */
>> >> >> +               ret = regulator_set_voltage_tol(cpu_reg,
>> >> >> safe_voltage, tol);
>> >> >> +               if (ret) {
>> >> >> +                       pr_err("failed to set safe voltage:
>> >> >> %d\n", ret);
>> >> >> +                       return ret;
>> >> >> +               }
>> >> >>         }
>> >> >>
>> >> >>         ret = clk_set_rate(cpu_clk, freq_exact);
>> >> >> @@ -86,7 +101,8 @@ static int cpu0_set_target(struct
>> >> >> cpufreq_policy *policy, unsigned int index) }
>> >> >>
>> >> >>         /* scaling down?  scale voltage after frequency */
>> >> >> -       if (!IS_ERR(cpu_reg) && new_freq < old_freq) {
>> >> >> +       if (!IS_ERR(cpu_reg) &&
>> >> >> +                       (new_freq < old_freq || new_freq <
>> >> >> safe_frequency)) { ret = regulator_set_voltage_tol(cpu_reg,
>> >> >> volt, tol); if (ret) {
>> >> >>                         pr_err("failed to scale voltage down:
>> >> >> %d\n", ret); @@ -116,6 +132,8 @@ static struct cpufreq_driver
>> >> >> cpu0_cpufreq_driver = {
>> >> >>
>> >> >>  static int cpu0_cpufreq_probe(struct platform_device *pdev)
>> >> >>  {
>> >> >> +       const struct property *prop;
>> >> >> +       struct dev_pm_opp *opp;
>> >> >>         struct device_node *np;
>> >> >>         int ret;
>> >> >>
>> >> >> @@ -165,13 +183,24 @@ static int cpu0_cpufreq_probe(struct
>> >> >> platform_device *pdev) goto out_put_node;
>> >> >>         }
>> >> >>
>> >> >> +       prop = of_find_property(np, "safe-opp", NULL);
>> >> >> +       if (prop) {
>> >> >> +               if (prop->value && (prop->length / sizeof(u32))
>> >> >> == 2) {
>> >> >> +                       const __be32 *val;
>> >> >> +                       val = prop->value;
>> >> >> +                       safe_frequency = be32_to_cpup(val++);
>> >> >> +                       safe_voltage = be32_to_cpup(val);
>> >> >> +               } else {
>> >> >> +                       pr_err("invalid safe-opp level
>> >> >> specified\n");
>> >> >> +               }
>> >> >> +       }
>> >> >> +
>> >> >>         of_property_read_u32(np, "voltage-tolerance",
>> >> >> &voltage_tolerance);
>> >> >>
>> >> >>         if (of_property_read_u32(np, "clock-latency",
>> >> >> &transition_latency)) transition_latency = CPUFREQ_ETERNAL;
>> >> >>
>> >> >>         if (!IS_ERR(cpu_reg)) {
>> >> >> -               struct dev_pm_opp *opp;
>> >> >>                 unsigned long min_uV, max_uV;
>> >> >>                 int i;
>> >> >>
>> >> >> --
>> >> >> 1.6.6.rc2
>> >> >>
>> >
>> >
>> >
>> > --
>> > Best regards,
>> >
>> > Lukasz Majewski
>> >
>> > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
>>
>> Thanks for your comments Lukasz.
>>
>> Regards,
>> Thomas.
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
>
> --
> Best regards,
>
> Lukasz Majewski
>
> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply

* [PATCH v2 1/7] cpufreq: cpufreq-cpu0: allow optional safe voltage during frequency transitions
From: Lukasz Majewski @ 2014-01-28 15:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAJuA9ag_izbSmQ_9-NH4NSGeCnzNBdwPUKzBTg5azuFKVqD28g@mail.gmail.com>

Hi Thomas,

> Hi Lukasz,
> 
> On Tue, Jan 28, 2014 at 1:47 PM, Lukasz Majewski
> <l.majewski@samsung.com> wrote:
> > Hi Thomas, Mike
> >
> >> Hi Mike,
> >>
> >> On Tue, Jan 28, 2014 at 1:55 AM, Mike Turquette
> >> <mturquette@linaro.org> wrote:
> >> > Quoting Thomas Abraham (2014-01-18 04:10:51)
> >> >> From: Thomas Abraham <thomas.ab@samsung.com>
> >> >>
> >> >> On some platforms such as the Samsung Exynos, changing the
> >> >> frequency of the CPU clock requires changing the frequency of
> >> >> the PLL that is supplying the CPU clock. To change the
> >> >> frequency of the PLL, the CPU clock is temporarily reparented
> >> >> to another parent clock.
> >> >>
> >> >> The clock frequency of this temporary parent clock could be much
> >> >> higher than the clock frequency of the PLL at the time of
> >> >> reparenting. Due to the temporary increase in the CPU clock
> >> >> speed, the CPU (and any other components in the CPU clock
> >> >> domain such as dividers, mux, etc.) have to to be operated at a
> >> >> higher voltage level, called the safe voltage level. This patch
> >> >> adds optional support to temporarily switch to a safe voltage
> >> >> level during CPU frequency transitions.
> >> >>
> >> >> Cc: Shawn Guo <shawn.guo@linaro.org>
> >> >> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> >> >
> >> > I'm not a fan of this change. This corner case should be
> >> > abstracted away somehow. I had talked to Chander Kayshap
> >> > previously about handling voltage changes in clock notifier
> >> > callbacks, which then renders any voltage change as a trivial
> >> > part of the clock rate transition. That means that this "safe
> >> > voltage" thing could be handled automagically without any
> >> > additional code in the CPUfreq driver.
> >> >
> >> > There are two nice ways to do this with the clock framework.
> >> > First is explicit re-parenting with voltage scaling done in the
> >> > clock rate-change notifiers:
> >> >
> >> > clk_set_parent(cpu_clk, temp_parent);
> >> > /* implicit voltage scaling to "safe voltage" happens above */
> >> > clk_set_rate(pll, some_rate);
> >> > clk_set_parent(cpu_clk, pll);
> >> > /* implicit voltage scaling to nominal OPP voltage happens above
> >> > */
> >> >
> >
> > I must agree with Mike here. In my opinion the above approach is
> > more compliant with CCF (as I've pointed it out in my other comment
> > - the cpu_clk has more than one parent and we could switch between
> > them when needed).
> 
> The mux which is used to re-parent is part of the larger opaque cpu
> clock type (more like the composite clock). So I am not sure how this
> isn't compliant with ccf.

My point here is to use the clk_set_parent() explicitly instead of
changing the mux with writing values directly to registers.

However, I'm also aware, that we must reparent quickly. so I'm OK
with your approach.

> 
> >
> >> > The above sequence would require a separate exnyos CPUfreq
> >> > driver, due to the added clk_set_parent logic.
> >> >
> >> > The second way to do this is to abstract the clk re-muxing logic
> >> > out into the clk driver, which would allow cpufreq-cpu0 to be
> >> > used for the exynos chips.
> >>
> >> This is the approach this patch series takes (patch 2/7). The clock
> >> re-muxing logic is handled by a clock driver code. The difference
> >> from what you suggested is that the safe voltage (that may be
> >> optionally) required before doing the re-muxing is handled here in
> >> cpufreq-cpu0 driver.
> >>
> >> The safe voltage setup can be done in the notifier as you
> >> suggested.
> >
> > If the clk_set_parent() approach is not suitable, then cannot we
> > consider using the one from highbank-cpufreq.c?
> >
> > Here we have cpufreq-cpu0.c which sets voltage of the cpu_clk.
> > In the highbank-cpufreq.c there are clock notifiers to change the
> > voltage.
> >
> > Cannot Exynos reuse such approach? Why shall we pollute
> > cpufreq-cpu0.c with another solution?
> 
> The highbank-cpufreq.c file was introduced because platforms using
> this driver did not have the usual regulator to control the voltage.
> The first commit of this driver explains this (copied below).
> 
> "Highbank processors depend on the external ECME to perform voltage
> management based on a requested frequency. Communication between the
> A9 cores and the ECME happens over the pl320 IPC channel."
> 
> So those platforms had no choice but to use an alternative approach to
> control the voltage (and reuse cpufreq-cpu0 as much as possible). 
> The
> case with exynos is a different one.

Highbank needs to set voltage via IPC, Exynos needs to adjust voltage
when reparenting.

Both can be recognized as unusual cases. That is why I asked if we
could reuse the same approach for Exynos.

> cpufreq-cpu0 is fully re-usable
> for exynos with the additional support for "safe voltage". If we agree
> that there might be existing or future platforms with single
> clock/voltage rail that require the "safe voltage" feature, then
> adding "safe voltage" support in cpufreq-cpu0 driver seems to be the
> right approach.

I think that Shawn's opinion will be final here.

> 
> >
> >> But, doing that in cpufreq-cpu0 driver will help other platforms
> >> reuse this feature if required. Also, if done here, the regulator
> >> handling is localized in this driver which otherwise would need to
> >> be handled in two places, cpufreq-cpu0 driver and the clock
> >> notifier.
> >
> > I think that there is a logical distinction between setting voltage
> > for cpufreq-cpu0 related clock and increasing voltage of reparented
> > clock.
> >
> > The former fits naturally to cpufreq-cpu0, when the latter seems
> > like some corner case (as Mike pointed out) for Exynos.
> 
> Agreed, it is a corner case. But for this corner case, we are
> performing some additional actions on the same regulator which is used
> in the normal functioning of the driver.
> 
> >
> >>
> >> So I tend to prefer the approach in this patch but I am willing to
> >> consider any suggestions.
> >
> > Thomas, what do you think about highbank-cpufreq.c approach (with
> > using clock notifiers)?
> 
> I have made a related comment on this above.
> 
> >
> > Do you think, that it is feasible to reuse it with Exynos?
> 
> highbank cpufreq driver is intended for a different purpose so I don't
> think it can be reused for exynos. Yes, we can make exynos specific
> hacks into highbank driver but how would that be better over the
> approach in this patch?

I think, that I was misunderstood here. I wanted to ask if we could
reuse the clk notifier approach.

> 
> >
> >> Shawn, it would be helpful if you could let
> >> us know your thoughts on this. I am almost done with testing the
> >> v3 of this series and want to post it so if there are any
> >> objections to the changes in this patch, please let me know.
> >>
> >> Thanks,
> >> Thomas.
> >>
> >> >
> >> > I'm more a fan of explicitly listing the Exact Steps for the cpu
> >> > opp transition in a separate exynos-specific CPUfreq driver, but
> >> > that's probably an unpopular view.
> >> >
> >> > Regards,
> >> > Mike
> >> >
> >> >> ---
> >> >>  .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   |    7 ++++
> >> >>  drivers/cpufreq/cpufreq-cpu0.c                     |   37
> >> >> +++++++++++++++++-- 2 files changed, 40 insertions(+), 4
> >> >> deletions(-)
> >> >>
> >> >> diff --git
> >> >> a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> >> >> b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> >> >> index f055515..37453ab 100644 ---
> >> >> a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt +++
> >> >> b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt @@
> >> >> -19,6 +19,12 @@ Optional properties:
> >> >>  - cooling-min-level:
> >> >>  - cooling-max-level:
> >> >>       Please refer to
> >> >> Documentation/devicetree/bindings/thermal/thermal.txt. +-
> >> >> safe-opp: Certain platforms require that during a opp
> >> >> transition,
> >> >> +  a system should not go below a particular opp level. For such
> >> >> systems,
> >> >> +  this property specifies the minimum opp to be maintained
> >> >> during the
> >> >> +  opp transitions. The safe-opp value is a tuple with first
> >> >> element
> >> >> +  representing the safe frequency and the second element
> >> >> representing the
> >> >> +  safe voltage.
> >> >>
> >> >>  Examples:
> >> >>
> >> >> @@ -36,6 +42,7 @@ cpus {
> >> >>                         396000  950000
> >> >>                         198000  850000
> >> >>                 >;
> >> >> +               safe-opp = <396000 950000>
> >> >>                 clock-latency = <61036>; /* two CLK32 periods */
> >> >>                 #cooling-cells = <2>;
> >> >>                 cooling-min-level = <0>;
> >> >> diff --git a/drivers/cpufreq/cpufreq-cpu0.c
> >> >> b/drivers/cpufreq/cpufreq-cpu0.c index 0c12ffc..075d3d1 100644
> >> >> --- a/drivers/cpufreq/cpufreq-cpu0.c
> >> >> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> >> >> @@ -27,6 +27,8 @@
> >> >>
> >> >>  static unsigned int transition_latency;
> >> >>  static unsigned int voltage_tolerance; /* in percentage */
> >> >> +static unsigned long safe_frequency;
> >> >> +static unsigned long safe_voltage;
> >> >>
> >> >>  static struct device *cpu_dev;
> >> >>  static struct clk *cpu_clk;
> >> >> @@ -64,17 +66,30 @@ static int cpu0_set_target(struct
> >> >> cpufreq_policy *policy, unsigned int index) volt_old =
> >> >> regulator_get_voltage(cpu_reg); }
> >> >>
> >> >> -       pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
> >> >> +       pr_debug("\n\n%u MHz, %ld mV --> %u MHz, %ld mV\n",
> >> >>                  old_freq / 1000, volt_old ? volt_old / 1000 :
> >> >> -1, new_freq / 1000, volt ? volt / 1000 : -1);
> >> >>
> >> >>         /* scaling up?  scale voltage before frequency */
> >> >> -       if (!IS_ERR(cpu_reg) && new_freq > old_freq) {
> >> >> +       if (!IS_ERR(cpu_reg) && new_freq > old_freq &&
> >> >> +                               new_freq >= safe_frequency) {
> >> >>                 ret = regulator_set_voltage_tol(cpu_reg, volt,
> >> >> tol); if (ret) {
> >> >>                         pr_err("failed to scale voltage up:
> >> >> %d\n", ret); return ret;
> >> >>                 }
> >> >> +       } else if (!IS_ERR(cpu_reg) && old_freq <
> >> >> safe_frequency) {
> >> >> +               /*
> >> >> +                * the scaled up voltage level for the new_freq
> >> >> is lower
> >> >> +                * than the safe voltage level. so set
> >> >> safe_voltage
> >> >> +                * as the intermediate voltage level and revert
> >> >> it
> >> >> +                * back after the frequency has been changed.
> >> >> +                */
> >> >> +               ret = regulator_set_voltage_tol(cpu_reg,
> >> >> safe_voltage, tol);
> >> >> +               if (ret) {
> >> >> +                       pr_err("failed to set safe voltage:
> >> >> %d\n", ret);
> >> >> +                       return ret;
> >> >> +               }
> >> >>         }
> >> >>
> >> >>         ret = clk_set_rate(cpu_clk, freq_exact);
> >> >> @@ -86,7 +101,8 @@ static int cpu0_set_target(struct
> >> >> cpufreq_policy *policy, unsigned int index) }
> >> >>
> >> >>         /* scaling down?  scale voltage after frequency */
> >> >> -       if (!IS_ERR(cpu_reg) && new_freq < old_freq) {
> >> >> +       if (!IS_ERR(cpu_reg) &&
> >> >> +                       (new_freq < old_freq || new_freq <
> >> >> safe_frequency)) { ret = regulator_set_voltage_tol(cpu_reg,
> >> >> volt, tol); if (ret) {
> >> >>                         pr_err("failed to scale voltage down:
> >> >> %d\n", ret); @@ -116,6 +132,8 @@ static struct cpufreq_driver
> >> >> cpu0_cpufreq_driver = {
> >> >>
> >> >>  static int cpu0_cpufreq_probe(struct platform_device *pdev)
> >> >>  {
> >> >> +       const struct property *prop;
> >> >> +       struct dev_pm_opp *opp;
> >> >>         struct device_node *np;
> >> >>         int ret;
> >> >>
> >> >> @@ -165,13 +183,24 @@ static int cpu0_cpufreq_probe(struct
> >> >> platform_device *pdev) goto out_put_node;
> >> >>         }
> >> >>
> >> >> +       prop = of_find_property(np, "safe-opp", NULL);
> >> >> +       if (prop) {
> >> >> +               if (prop->value && (prop->length / sizeof(u32))
> >> >> == 2) {
> >> >> +                       const __be32 *val;
> >> >> +                       val = prop->value;
> >> >> +                       safe_frequency = be32_to_cpup(val++);
> >> >> +                       safe_voltage = be32_to_cpup(val);
> >> >> +               } else {
> >> >> +                       pr_err("invalid safe-opp level
> >> >> specified\n");
> >> >> +               }
> >> >> +       }
> >> >> +
> >> >>         of_property_read_u32(np, "voltage-tolerance",
> >> >> &voltage_tolerance);
> >> >>
> >> >>         if (of_property_read_u32(np, "clock-latency",
> >> >> &transition_latency)) transition_latency = CPUFREQ_ETERNAL;
> >> >>
> >> >>         if (!IS_ERR(cpu_reg)) {
> >> >> -               struct dev_pm_opp *opp;
> >> >>                 unsigned long min_uV, max_uV;
> >> >>                 int i;
> >> >>
> >> >> --
> >> >> 1.6.6.rc2
> >> >>
> >
> >
> >
> > --
> > Best regards,
> >
> > Lukasz Majewski
> >
> > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
> 
> Thanks for your comments Lukasz.
> 
> Regards,
> Thomas.
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply

* Building with gcc 4.6.4
From: Russell King - ARM Linux @ 2014-01-28 15:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <21223.49366.479579.145000@gargle.gargle.HOWL>

On Tue, Jan 28, 2014 at 03:38:14PM +0100, Mikael Pettersson wrote:
> Russell King - ARM Linux writes:
>  > So, yesterday I built gcc 4.6.4 (mainline) for the autobuilder, and the
>  > result is that every build failed with the same error:
>  > 
>  > scripts/mod/empty.c:1:0: error: FPA is unsupported in the AAPCS
>  > 
>  > This seems to be because linux-elf targets default to fpe3 in mainline
>  > gcc, but specifying -mabi=aapcs-linux switches us into EABI mode where
>  > the compiler errors out with the default FPU.
>  > 
>  > Hence, I believe we need this to ensure that a compatible VFP is
>  > selected.  One can argue that building EABI ARMv4 with VFP is silly,
>  > but it seems that's what the gcc folk have decided (rightly or
>  > wrongly.)
>  > 
>  > Maybe this is a bug in mainline GCC - which begs the question why
>  > (presumably, since no one has picked this up) Linaro's toolchain
>  > has fixes but mainline GCC doesn't.
>  > 
>  > Comments?
> 
> Perhaps because most ARM EABI toolchains default to soft-float,
> and the hardfloat ones usually select v6 or v7 + vfp-d16 or neon
> as their defaults, so the archaic FPA is never the default.

soft-float has nothing to do with it, because the kernel always passes
-msoft-float.

> Or are you using an OABI toolchain to compile an EABI kernel?

... which should make no difference what so ever since the kernel should
be passing the appropriate options.  That's why we pass -mabi=aapcs-linux
to the kernel.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox