Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] ARM: vt8500: Convert arch-vt8500 to multiplatform
From: Tony Prisk @ 2012-10-11 18:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201210110747.46682.arnd@arndb.de>

On Thu, 2012-10-11 at 07:47 +0000, Arnd Bergmann wrote:
> On Thursday 11 October 2012, Alexey Charkov wrote:
> > Does this imply that early printk won't be supported any longer, or am
> > I missing something?
> 
> You are correct. However, there is work under way to bring it back.
> If you want to keep it around in the meantime, you could let the user
> enable VT8500 either in MULTIPLATFORM or standalone using some Kconfig
> logic like:
> 
> choice "Platform selection"
> 
> ...
> 
> config VT8500_SINGLE
> 	bool "Via/Wondermedia VT8500 / WM8505 / WM8650"
> 
> ...
> 
> endchoice
> 
> ...
> 
> config VT8500
> 	bool "Via/Wondermedia VT8500 / WM8505 / WM8650" if ARCH_MULTIPLATFORM
> 	default VT8500_SINGLE
> 
> 	Arnd

Alexey (or anyone else),

Do you want it kept in the meantime?

I think the easiest way to add/keep it would be to create:

ARCH_VT8500_SINGLE (arm/Kconfig) and ARCH_VT8500_MULTI
(arch-vt8500/Kconfig) and have them both select ARCH_VT8500 as an option
to make drivers available.

Regards
Tony P

^ permalink raw reply

* [PATCH v2 1/4] net: mvneta: driver for Marvell Armada 370/XP network unit
From: Baruch Siach @ 2012-10-11 18:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1349969282-12676-2-git-send-email-thomas.petazzoni@free-electrons.com>

Hi Thomas,

On Thu, Oct 11, 2012 at 05:27:59PM +0200, Thomas Petazzoni wrote:
> +module_param(mvneta_rxq_number, int, S_IRUGO);
> +module_param(mvneta_txq_number, int, S_IRUGO);
> +
> +module_param(mvneta_rxq_def, int, S_IRUGO);
> +module_param(mvneta_txq_def, int, S_IRUGO);

Since these are module params they need the driver name prefix when referenced 
in the kernel command line, so there should be no need for another "mvneta" 
prefix.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

^ permalink raw reply

* [PATCH 3/6] ARM: ux500: Add support for ST-Ericsson's u9540 SoC
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-10-11 18:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121011092039.GV9707@gmail.com>

On 10:20 Thu 11 Oct     , Lee Jones wrote:
> On Thu, 11 Oct 2012, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> > On 08:38 Thu 11 Oct     , Lee Jones wrote:
> > > On Wed, 10 Oct 2012, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > 
> > > > On 16:18 Wed 10 Oct     , Lee Jones wrote:
> > > > > First level board support for the u9540.
> > > > > 
> > > > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > > > ---
> > > > >  arch/arm/mach-ux500/cpu-db8500.c |   14 ++++++++++++++
> > > > >  1 file changed, 14 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
> > > > > index 3d8e321..6b7f970 100644
> > > > > --- a/arch/arm/mach-ux500/cpu-db8500.c
> > > > > +++ b/arch/arm/mach-ux500/cpu-db8500.c
> > > > > @@ -17,6 +17,7 @@
> > > > >  #include <linux/platform_device.h>
> > > > >  #include <linux/io.h>
> > > > >  #include <linux/mfd/abx500/ab8500.h>
> > > > > +#include <linux/mfd/dbx500-prcmu.h>
> > > > >  #include <linux/of.h>
> > > > >  #include <linux/of_platform.h>
> > > > >  #include <linux/regulator/machine.h>
> > > > > @@ -319,6 +320,8 @@ static void __init u8500_init_machine(void)
> > > > >  		snowball_pinmaps_init();
> > > > >  	else if (of_machine_is_compatible("st-ericsson,hrefv60+"))
> > > > >  		hrefv60_pinmaps_init();
> > > > > +	else if (of_machine_is_compatible("st-ericsson,u9540")) {}
> > > > > +		/* TODO: Add u9540 pinmaps. */
> > > > do you via DT no here
> > > 
> > > We don't support pinctrl yet. It will come, it's just not available yet.
> > so stop to add more and update the drviers
> 
> We'll get round to it. 
> 
> We have a set priority list. Neither you nor I have control over that.
yes but code mainline is not driver be politics but by code review

so no do the effort to add the dt to the pinctrl it's 12 hours work so you have
no excuse to delay it again
> 
> > and u9540 is a SoC name not a baord name
> 
> It's both. The u9540 board is affectionately called the u9540 board.
> 
> > > > >  	/* TODO: Export SoC, USB, cpu-freq and DMA40 */
> > > > >  	parent = u8500_of_init_devices();
> > > > > @@ -331,6 +334,7 @@ static const char * u8500_dt_board_compat[] = {
> > > > >  	"calaosystems,snowball-a9500",
> > > > >  	"st-ericsson,hrefv60+",
> > > > >  	"st-ericsson,mop500",
> > > > > +	"st-ericsson,u9540",
> > > > stop to add more compatible sue a more generic one
> > > 
> > > Not entirely sure what you mean here.
> > > 
> > > These are the only four boards we officially support.
> > > 
> > > There are other uXXXX boards that we do not.
> > you do not need it just mach a global compatbile
> 
> Why? What difference does it make? I could understand if we
> had a long list of supported boards, but I think this is move
> verbose and descriptive.

this is already the case simplify it and give a good example in the mainline
code as people just do copy & paste usualy
> 
> > > > >  	NULL,
> > > > >  };
> > > > >  
> > > > > @@ -345,4 +349,14 @@ DT_MACHINE_START(U8500_DT, "ST-Ericsson U8500 platform (Device Tree Support)")
> > > > >  	.dt_compat      = u8500_dt_board_compat,
> > > > >  MACHINE_END
> > > > >  
> > > > > +DT_MACHINE_START(U9540_DT, "ST-Ericsson 9540 platform (Device Tree Support)")
> > > > > +	.map_io		= u8500_map_io,
> > > > > +	.init_irq	= ux500_init_irq,
> > > > > +	.timer		= &ux500_timer,
> > > > > +	.handle_irq	= gic_handle_irq,
> > > > > +	.init_machine	= u8500_init_machine,
> > > > > +	.init_late	= NULL,
> > > > > +	.dt_compat      = u8500_dt_board_compat,
> > > > > +MACHINE_END
> > > > really?
> > > > no need drop it for a more generic on
> > > 
> > > This may look the same as the previous MACHINE_START now, but
> > > it will have differences in the future as more u9540 support is
> > > upstreamed. I think it should stick around as a separate entity
> > > for the time being. If we can consolidate the init functions at
> > > a later date, believe me I will make sure it happens.
no sorry add it when it's need not now

Best Regards,
J.

^ permalink raw reply

* [PATCH 4/6] ARM: ux500: Add UART support to the u9540 Device Tree
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-10-11 18:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121011092222.GW9707@gmail.com>

On 10:22 Thu 11 Oct     , Lee Jones wrote:
> On Thu, 11 Oct 2012, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> > On 08:29 Thu 11 Oct     , Lee Jones wrote:
> > > On Wed, 10 Oct 2012, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > 
> > > > On 16:18 Wed 10 Oct     , Lee Jones wrote:
> > > > > Add the 3 UART nodes required to enable serial ports on the u9540.
> > > > > 
> > > > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > > > ---
> > > > >  arch/arm/boot/dts/u9540.dts |   13 ++++++++++++-
> > > > dts? it's dtsi
> > > 
> > > No it's not.
> > so the name of the file is wrong s u9540 is a SoC name
> 
> The board could actually be called either ccu9540, or just 
> u9540. I picked one and went with it. The full name is the
> u9540 development board.
so use ccu9540 as u9540 is the soc name so if we use u9540 as compatible we
expect to mach the soc and not a board

Best Regards,
J.

^ permalink raw reply

* [PATCH 1/2] ARM: nomadik: switch over to using the FSMC driver
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-10-11 18:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1349953632.12014.8.camel@sauron.fi.intel.com>

On 14:07 Thu 11 Oct     , Artem Bityutskiy wrote:
> On Thu, 2012-10-04 at 09:28 +0200, Linus Walleij wrote:
> > The Nomadik NAND driver is really just a subset of the existing
> > FSMC driver, so let's switch over to using that driver instead,
> > since it handles more variants of this chip. The callbacks for
> > setting up the chip is doing stuff now handled by the FSMC
> > driver.
> > 
> > Cc: Alessandro Rubini <rubini@unipv.it>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Pushed both to l2-mtd.git, thanks!
wait on this one Linus said he will send an other version based on my patch
for fmsc

Best Regards,
J.
> 
> -- 
> Best Regards,
> Artem Bityutskiy



> _______________________________________________
> 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 07/11] fsmc/nand: Provide contiguous buffers to dma
From: viresh kumar @ 2012-10-11 17:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACRpkdYjA8QUh4HqvE1rsnXcGA4WafTkMq_wLNJ__gXEnEYrow@mail.gmail.com>

On Thu, Oct 11, 2012 at 9:36 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Oct 11, 2012 at 6:15 AM, viresh kumar <viresh.kumar@linaro.org> wrote:
>> I pray that i am wrong here, otherwise i would be thrown out from

s/am/am not/ :(

>> the maintainers list for this driver :)
>>
>> dma_sync_single_for_device() is not doing anything on the buffer, but
>> on the LLI item. Actually it is flushing LLI struct so that DMA h/w can get
>> the correct values.
>
> Sorry no, I'm the one who's wrong...

Glad to hear that. :)
It happens very few times in one's lifetime, that a beginner like me is correct
and an expert like you is not. Just kidding :)

> And the block layer of the subsystem should take care
> of only handing the driver buffers that are contiguous

But why should that be a constraint on block layer? It is working in virtual
space and is concerned about that onlly.

--
viresh

^ permalink raw reply

* alignment faults in 3.6
From: Catalin Marinas @ 2012-10-11 16:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121011103257.GO4625@n2100.arm.linux.org.uk>

On 11 October 2012 11:32, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Oct 11, 2012 at 12:22:06PM +0200, Eric Dumazet wrote:
>> I took a look, and I dont see why/how gcc could use a ldm instruction
>>
>> Doing so assumed the alignment of the structure was 8 bytes, but its
>> not.
>>
>> Networking stack mandates that IP headers are aligned on 4 bytes
>> boundaries, not 8 bytes.
>
> Err, no.  ldm is "load multiple" not "load double".  It loads multiple
> 32-bit registers, and its requirement for non-faulting behaviour is for
> the pointer to be 4 byte aligned.  However, "load double" requires 8
> byte alignment.

It got better with ARMv6 where LDRD/STRD only require 4 byte alignment
(the only 8 byte alignment is required by LDREXD/STREXD). But on ARMv5
LDRD/STRD 8 byte alignment is indeed required (otherwise
unpredictable).

-- 
Catalin

^ permalink raw reply

* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
From: Colin Cross @ 2012-10-11 16:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5076F5CB.4020200@wwwdotorg.org>

On Thu, Oct 11, 2012 at 9:37 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/11/2012 05:24 AM, Joseph Lo wrote:
>> On Wed, 2012-10-10 at 06:49 +0800, Stephen Warren wrote:
>>> On 10/08/2012 04:26 AM, Joseph Lo wrote:
>>>> The cpuidle LP2 is a power gating idle mode. It support power gating
>>>> vdd_cpu rail after all cpu cores in LP2. For Tegra30, the CPU0 must
>>>> be last one to go into LP2. We need to take care and make sure whole
>>>> secondary CPUs were in LP2 by checking CPU and power gate status.
>>>> After that, the CPU0 can go into LP2 safely. Then power gating the
>>>> CPU rail.

<snip>

>>>> @@ -85,16 +108,22 @@ static int __cpuinit tegra30_idle_lp2(struct cpuidle_device *dev,
>>>>                                   int index)
>>>>  {
>>>>     bool entered_lp2 = false;
>>>> +   bool last_cpu;
>>>>
>>>>     local_fiq_disable();
>>>>
>>>> +   last_cpu = tegra_set_cpu_in_lp2(dev->cpu);
>>>> +   if (dev->cpu == 0) {
>>>> +           if (last_cpu)
>>>> +                   entered_lp2 = tegra30_idle_enter_lp2_cpu_0(dev, drv,
>>>> +                                                              index);
>>>> +           else
>>>> +                   cpu_do_idle();
>>>> +   } else {
>>>>             entered_lp2 = tegra30_idle_enter_lp2_cpu_n(dev, drv, index);
>>>> +   }
>>>
>>> Hmm. That means that if the last CPU to enter LP2 is e.g. CPU1, then
>>> even though all CPUs are now in LP2, the complex as a whole doesn't
>>> enter LP2. Is there a way to make the cluster as a whole enter LP2 in
>>> this case? Isn't that what coupled cpuidle is for?
>>
>> It may look like the coupled cpuidle can satisfy the usage here. But it
>> didn't. Please check the criteria of coupled cpuidle.
>
> What about the first part of the question. What happens if:
>
> CPU3 enters LP2
> CPU2 enters LP2
> CPU0 enters LP2
> CPU1 enters LP2
>
> Since CPU1 is not CPU0, tegra30_idle_enter_lp2_cpu_n() is called, and
> hence I think the whole CPU complex is never rail-gated (just each CPU
> is power-gated) even though all CPUs are in LP2 and the complex could be
> rail-gated. Isn't this missing out on power-savings?
>
> So, we either need to:
>
> a) Make tegra30_idle_enter_lp2_cpu_n() rail-gate if the last CPU is
> entering LP2, and then I'm not sure the implementation would be any
> different to tegra30_idle_enter_lp2_cpu_0, would it?
>
> b) If CPUn can't trigger rail-gating, then when CPUn is the last to
> enter LP2 of the whole complex, it needs to IPI to CPU0 to tell it to
> rail-gate, and simply power-gate itself. I believe this IPI interaction
> is exactly what coupled cpuidle is about, isn't it?
>
>> /*
>>  * To use coupled cpuidle states, a cpuidle driver must:
>>  *
>>  *    Set struct cpuidle_device.coupled_cpus to the mask of all
>>  *    coupled cpus, usually the same as cpu_possible_mask if all cpus
>>  *    are part of the same cluster.  The coupled_cpus mask must be
>>  *    set in the struct cpuidle_device for each cpu.
>>  *
>>  *    Set struct cpuidle_device.safe_state to a state that is not a
>>  *    coupled state.  This is usually WFI.
>>  *
>>  *    Set CPUIDLE_FLAG_COUPLED in struct cpuidle_state.flags for each
>>  *    state that affects multiple cpus.
>>  *
>>  *    Provide a struct cpuidle_state.enter function for each state
>>  *    that affects multiple cpus.  This function is guaranteed to be
>>  *    called on all cpus at approximately the same time.  The driver
>>  *    should ensure that the cpus all abort together if any cpu tries
>>  *    to abort once the function is called.  The function should return
>>  *    with interrupts still disabled.
>>  */
>>
>> The Tegra30 can support the secondary CPUs go into LP2 (power-gate)
>> independently.
>
> I think that just means that the safe state for CPUn (i.e. not CPU0) can
> do better than WFI on Tegra30, even though it can't on Tegra20.

Exactly.

>> The limitation of the CPU0 is the CPU0 must be the last
>> one to go into LP2 to shut off CPU rail.
>>
>> It also no need for every CPU to leave LP2 at the same time. The CPU0
>> would be always the first one that woken up from LP2. But all the other
>> secondary CPUs can still keep in LP2. One of the secondary CPUs can also
>> be woken up alone, if the CPU0 already up.
>
> That seems like an implementation detail. Perhaps coupled cpuidle needs
> to be enhanced to best support Tegra30?

As is, coupled cpuidle will work on Tegra30, but it will unnecessarily
wake up the secondary cpus during the transitions to off and back on
again.  Those cpus will immediately go back to single-cpu LP2, so it
may not be a big deal, but there is a small optimization I've
discussed with a few other people that could avoid waking them up.  I
suggest adding an extra pre-idle hook to the Tegra30 that is called by
coupled cpuidle on the last cpu to go down.  It would return a cpumask
of cpus that have been prepared for idle by guaranteeing that they
will not wake up from an interrupt, and therefore don't need to be
woken up for the transitions.  I haven't worked with a cpu that needs
this optimization yet, so I haven't done it.

^ permalink raw reply

* [PATCH v7] Initialize USB on dm365 EVM
From: Constantine Shulyupin @ 2012-10-11 16:44 UTC (permalink / raw)
  To: linux-arm-kernel

From: Constantine Shulyupin <const@MakeLinux.com>

Call USB initialization davinci_setup_usb from board initialization dm365_evm_init.

Tested with OTG configuration, usb gadget g_zero on DM365 EVM connected to PC.

Note: register USB_PHY_CTRL must have flag USBPHY_CLKFREQ_24MHZ

References:

Original patch by miguel.aguilar at ridgerun.com three years ago:
- http://www.mail-archive.com/davinci-linux-open-source at linux.davincidsp.com/msg14741.html

Signed-off-by: Constantine Shulyupin <const@MakeLinux.com>
---

Changelog

Changes since v6
- patch splitted accordinly request of Sergei
- this split contains call to davinci_setup_usb from dm365_evm_init

Changes since v5 http://www.spinics.net/lists/kernel/msg1413120.html
accordingy feedback of nsekhar at ti.com http://www.spinics.net/lists/kernel/msg1414914.html
- phy configuration moved to drivers/usb/musb/davinci.c
- USB_OTG configuration is submitted in separated patch: http://www.spinics.net/lists/kernel/msg1414964.html
- Setting current limit to 1000 mA. Any way the current is limited to 510 mA in davinci_setup_usb.

Changes since v4 http://www.spinics.net/lists/kernel/msg1412995.html
- removed fix of dev_info in musb_init_controller

Changes since v3 http://www.spinics.net/lists/kernel/msg1412544.html:
- removed optional altering of pr_info

Changes since v1  http://marc.info/?l=linux-kernel&m=130894150803661&w=2:
- removed optional code and reordered
- removed alternation of GPIO33, which is multiplexed with DRVVBUS, because is not need for peripheral USB

This patch is based on code from projects Arago, Angstom and RidgeRun.

---
 arch/arm/mach-davinci/board-dm365-evm.c |    2 ++

diff --git a/arch/arm/mach-davinci/board-dm365-evm.c b/arch/arm/mach-davinci/board-dm365-evm.c
index 688a9c5..ba5ffc1 100644
--- a/arch/arm/mach-davinci/board-dm365-evm.c
+++ b/arch/arm/mach-davinci/board-dm365-evm.c
@@ -38,6 +38,7 @@
 #include <mach/mmc.h>
 #include <mach/nand.h>
 #include <mach/keyscan.h>
+#include <mach/usb.h>
 
 #include <media/tvp514x.h>
 
@@ -610,6 +611,7 @@ static __init void dm365_evm_init(void)
 
 	dm365_init_spi0(BIT(0), dm365_evm_spi_info,
 			ARRAY_SIZE(dm365_evm_spi_info));
+	davinci_setup_usb(1000, 8);
 }
 
 MACHINE_START(DAVINCI_DM365_EVM, "DaVinci DM365 EVM")
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 1/4] net: mvneta: driver for Marvell Armada 370/XP network unit
From: Thomas Petazzoni @ 2012-10-11 16:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5076E9F1.60200@gmail.com>

Rob,

On Thu, 11 Oct 2012 10:46:57 -0500, Rob Herring wrote:

> > +Required properties:
> > +- compatible: should be "marvell,neta".
> 
> This should be more specific such as "marvell,armada-xp-neta".
> 
> Or use 370 instead of xp. It should be which ever chip came first.
> 
> > +- reg: address and length of the register set for the device.
> > +- interrupts: interrupt for the device
> > +- phy-mode: String, operation mode of the PHY interface. Supported
> > +  values are "sgmii" and "rmii".
> > +- phy-addr: Integer, address of the PHY.
> > +- device_type: should be "network".
> 
> Drop this. device_type is not used for FDT.
> 
> > +- clock-frequency: frequency of the peripheral clock of the SoC.
> > +
> > +Example:
> > +
> > +eth at d0070000 {
> 
> Should be ethernet at ...

Thanks for your comments. Will fix in v3.

Best regards,

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

^ permalink raw reply

* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
From: Stephen Warren @ 2012-10-11 16:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1349954685.19413.207.camel@jlo-ubuntu-64.nvidia.com>

On 10/11/2012 05:24 AM, Joseph Lo wrote:
> On Wed, 2012-10-10 at 06:49 +0800, Stephen Warren wrote:
>> On 10/08/2012 04:26 AM, Joseph Lo wrote:
>>> The cpuidle LP2 is a power gating idle mode. It support power gating
>>> vdd_cpu rail after all cpu cores in LP2. For Tegra30, the CPU0 must
>>> be last one to go into LP2. We need to take care and make sure whole
>>> secondary CPUs were in LP2 by checking CPU and power gate status.
>>> After that, the CPU0 can go into LP2 safely. Then power gating the
>>> CPU rail.
>>
>>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
>>
>>> +static bool tegra30_idle_enter_lp2_cpu_0(struct cpuidle_device *dev,
>>> +					 struct cpuidle_driver *drv,
>>> +					 int index)
>>> +{
>>> +	struct cpuidle_state *state = &drv->states[index];
>>> +	u32 cpu_on_time = state->exit_latency;
>>> +	u32 cpu_off_time = state->target_residency - state->exit_latency;
>>> +
>>> +	if (num_online_cpus() > 1 && !tegra_cpu_rail_off_ready()) {
>>
>> Should that be || not &&?
>>
>> Isn't the "num_online_cpus() > 1" condition effectively checked at the
>> call site, i.e. in tegra30_idle_lp2() below via the if (last_cpu) check?
>>
> 
> Should be "&&" here.
> Because we need to check if there are still multi CPUs online, then we
> need to make sure all the secondary CPUs be power gated first. After all
> the secondary CPUs been power gated, the CPU0 could go into LP2 and the
> CPU rail could be shut off.
> If all the secondary CPUs been hot plugged, then the "num_online_cpus()
>> 1" would be always false. Then the CPU0 can go into LP2 directly.
> 
> So it was used to check are there multi cpus online or not? It's
> difference with the last_cpu check below. The last_cpu was used to check
> all the CPUs were in LP2 process or not. If the CPU0 is the last one
> went into LP2 process, then it would be true.
> 
> So the point here is. We can avoid to check the power status of the
> secodarys CPUs if they be unplugged.

OK, so this condition is about ignoring the result of
tegra_cpu_rail_off_ready() if there is only 1 CPU online. That makes
sense, since we know in that case there cannot be any other CPUs to
check if they're in LP2 or not.

But what about the case where 2 CPUs are online and 2 offline. In that
case, num_online_cpus() > 1, so the call to tegra_cpu_rail_off_ready()
is skipped. Yet, with 2 CPUs online, we do need to check whichever other
CPU is online to see if it's in LP2 or not.

I think what we need to do is the following:

cpus_in_lp2_mask = generate_mask_from_pmc_registers();
if (cpus_in_lp2_mask != cpus_online_mask) {
    cpu_do_idle();
    return;
}
enter lp2;

right?

>>> @@ -85,16 +108,22 @@ static int __cpuinit tegra30_idle_lp2(struct cpuidle_device *dev,
>>>  				      int index)
>>>  {
>>>  	bool entered_lp2 = false;
>>> +	bool last_cpu;
>>>  
>>>  	local_fiq_disable();
>>>  
>>> +	last_cpu = tegra_set_cpu_in_lp2(dev->cpu);
>>> +	if (dev->cpu == 0) {
>>> +		if (last_cpu)
>>> +			entered_lp2 = tegra30_idle_enter_lp2_cpu_0(dev, drv,
>>> +								   index);
>>> +		else
>>> +			cpu_do_idle();
>>> +	} else {
>>>  		entered_lp2 = tegra30_idle_enter_lp2_cpu_n(dev, drv, index);
>>> +	}
>>
>> Hmm. That means that if the last CPU to enter LP2 is e.g. CPU1, then
>> even though all CPUs are now in LP2, the complex as a whole doesn't
>> enter LP2. Is there a way to make the cluster as a whole enter LP2 in
>> this case? Isn't that what coupled cpuidle is for?
> 
> It may look like the coupled cpuidle can satisfy the usage here. But it
> didn't. Please check the criteria of coupled cpuidle. 

What about the first part of the question. What happens if:

CPU3 enters LP2
CPU2 enters LP2
CPU0 enters LP2
CPU1 enters LP2

Since CPU1 is not CPU0, tegra30_idle_enter_lp2_cpu_n() is called, and
hence I think the whole CPU complex is never rail-gated (just each CPU
is power-gated) even though all CPUs are in LP2 and the complex could be
rail-gated. Isn't this missing out on power-savings?

So, we either need to:

a) Make tegra30_idle_enter_lp2_cpu_n() rail-gate if the last CPU is
entering LP2, and then I'm not sure the implementation would be any
different to tegra30_idle_enter_lp2_cpu_0, would it?

b) If CPUn can't trigger rail-gating, then when CPUn is the last to
enter LP2 of the whole complex, it needs to IPI to CPU0 to tell it to
rail-gate, and simply power-gate itself. I believe this IPI interaction
is exactly what coupled cpuidle is about, isn't it?

> /*
>  * To use coupled cpuidle states, a cpuidle driver must:
>  *
>  *    Set struct cpuidle_device.coupled_cpus to the mask of all
>  *    coupled cpus, usually the same as cpu_possible_mask if all cpus
>  *    are part of the same cluster.  The coupled_cpus mask must be
>  *    set in the struct cpuidle_device for each cpu.
>  *
>  *    Set struct cpuidle_device.safe_state to a state that is not a
>  *    coupled state.  This is usually WFI.
>  *
>  *    Set CPUIDLE_FLAG_COUPLED in struct cpuidle_state.flags for each
>  *    state that affects multiple cpus.
>  *
>  *    Provide a struct cpuidle_state.enter function for each state
>  *    that affects multiple cpus.  This function is guaranteed to be
>  *    called on all cpus at approximately the same time.  The driver
>  *    should ensure that the cpus all abort together if any cpu tries
>  *    to abort once the function is called.  The function should return
>  *    with interrupts still disabled.
>  */
> 
> The Tegra30 can support the secondary CPUs go into LP2 (power-gate)
> independently.

I think that just means that the safe state for CPUn (i.e. not CPU0) can
do better than WFI on Tegra30, even though it can't on Tegra20.

> The limitation of the CPU0 is the CPU0 must be the last
> one to go into LP2 to shut off CPU rail.
> 
> It also no need for every CPU to leave LP2 at the same time. The CPU0
> would be always the first one that woken up from LP2. But all the other
> secondary CPUs can still keep in LP2. One of the secondary CPUs can also
> be woken up alone, if the CPU0 already up.

That seems like an implementation detail. Perhaps coupled cpuidle needs
to be enhanced to best support Tegra30?

>>> diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
>>
>>> +static void set_power_timers(unsigned long us_on, unsigned long us_off)
>>
>>> +	if (tegra_pclk == NULL) {
>>> +		tegra_pclk = clk_get_sys(NULL, "pclk");
>>> +		if (IS_ERR(tegra_pclk)) {
>>> +			/*
>>> +			 * pclk not been init or not exist.
>>> +			 * Use sclk to take the place of it.
>>> +			 * The default setting was pclk=sclk.
>>> +			 */
>>> +			tegra_pclk = clk_get_sys(NULL, "sclk");
>>> +		}
>>> +	}
>>
>> That's a little odd. Surely the HW has pclk or it doesn't? Why use
>> different clocks at different times for what is apparently the same thing?
> 
> It just because the "pclk" is not available on the Tegra30's clock
> framework but Tegra20 right now.

We should just fix that instead of working around it then. I assume it's
a simple matter of adding the appropriate clock definition?

^ permalink raw reply

* [PATCH v7] Enable USB peripheral mode on dm365 EVM
From: Constantine Shulyupin @ 2012-10-11 16:34 UTC (permalink / raw)
  To: linux-arm-kernel

From: Constantine Shulyupin <const@MakeLinux.com>

Sets USB PHY clock source to 24 MHz clock.

Tested with OTG configuration, usb gadget g_zero on DM365 EVM connected to PC.

To active the patch need to call davinci_setup_usb from dm365_evm_init

References:

Definition of USB_PHY_CTRL and PHYCLKFREQ:
- http://www.makelinux.com/lib/ti/DM36x_ARM/doc-141

Original patch by miguel.aguilar at ridgerun.com three years ago:
- http://www.mail-archive.com/davinci-linux-open-source at linux.davincidsp.com/msg14741.html

Signed-off-by: Constantine Shulyupin <const@MakeLinux.com>
---

Note:

Changelog

Changes since v6
- moved call to davinci_setup_usb from dm365_evm_init to another patch accordinly request of Sergei
 
Changes since v5 http://www.spinics.net/lists/kernel/msg1413120.html
accordingy feedback of nsekhar at ti.com http://www.spinics.net/lists/kernel/msg1414914.html
- phy configuration moved to drivers/usb/musb/davinci.c
- USB_OTG configuration is submitted in separated patch: http://www.spinics.net/lists/kernel/msg1414964.html
- Setting current limit to 1000 mA. Any way the current is limited to 510 mA in davinci_setup_usb.

Changes since v4 http://www.spinics.net/lists/kernel/msg1412995.html
- removed fix of dev_info in musb_init_controller

Changes since v3 http://www.spinics.net/lists/kernel/msg1412544.html:
- removed optional altering of pr_info

Changes since v1  http://marc.info/?l=linux-kernel&m=130894150803661&w=2:
- removed optional code and reordered
- removed alternation of GPIO33, which is multiplexed with DRVVBUS, because is not need for peripheral USB

This patch is based on code from projects Arago, Angstom and RidgeRun.

---
 drivers/usb/musb/davinci.c              |    3 +++
 drivers/usb/musb/davinci.h              |    1 +

diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
index 472c8b4..af09ebf 100644
--- a/drivers/usb/musb/davinci.c
+++ b/drivers/usb/musb/davinci.c
@@ -428,6 +428,9 @@ static int davinci_musb_init(struct musb *musb)
 		__raw_writel(deepsleep, DM355_DEEPSLEEP);
 	}
 
+	if (machine_is_davinci_dm365_evm())
+		writel(readl(USB_PHY_CTRL) | USBPHY_CLKFREQ_24MHZ, USB_PHY_CTRL);
+
 	/* reset the controller */
 	musb_writel(tibase, DAVINCI_USB_CTRL_REG, 0x1);
 
diff --git a/drivers/usb/musb/davinci.h b/drivers/usb/musb/davinci.h
index 371baa0..e737d97 100644
--- a/drivers/usb/musb/davinci.h
+++ b/drivers/usb/musb/davinci.h
@@ -16,6 +16,7 @@
 
 /* Integrated highspeed/otg PHY */
 #define USBPHY_CTL_PADDR	0x01c40034
+#define USBPHY_CLKFREQ_24MHZ	BIT(13)
 #define USBPHY_DATAPOL		BIT(11)	/* (dm355) switch D+/D- */
 #define USBPHY_PHYCLKGD		BIT(8)
 #define USBPHY_SESNDEN		BIT(7)	/* v(sess_end) comparator */
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs
From: Stephen Warren @ 2012-10-11 16:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1349946918.19413.130.camel@jlo-ubuntu-64.nvidia.com>

On 10/11/2012 03:15 AM, Joseph Lo wrote:
> On Wed, 2012-10-10 at 06:38 +0800, Stephen Warren wrote:
>> On 10/08/2012 04:26 AM, Joseph Lo wrote:
>>> This supports power-gated (LP2) idle on secondary CPUs for Tegra30.
>>> The secondary CPUs can go into LP2 state independently. When CPU goes
>>> into LP2 state, it saves it's state and puts itself to flow controlled
>>> WFI state. After that, it will been power gated.
>>
>>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
>>
>>>  static struct cpuidle_driver tegra_idle_driver = {
>>>  	.name = "tegra_idle",
>>>  	.owner = THIS_MODULE,
>>>  	.en_core_tk_irqen = 1,
>>> -	.state_count = 1,
>>> +	.state_count = 2,
>>
>> Doesn't that assignment need to be ifdef'd just like the array entry
>> setup below:
>>
>>>  	.states = {
>>>  		[0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
>>> +#ifdef CONFIG_PM_SLEEP
>>> +		[1] = {
>>> +			.enter			= tegra30_idle_lp2,
>>> +			.exit_latency		= 2000,
>>> +			.target_residency	= 2200,
>>> +			.power_usage		= 0,
>>> +			.flags			= CPUIDLE_FLAG_TIME_VALID,
>>> +			.name			= "LP2",
>>> +			.desc			= "CPU power-gate",
>>> +		},
>>> +#endif
>>>  	},
>>>  };
>>
>>> @@ -41,6 +114,10 @@ int __init tegra30_cpuidle_init(void)
>>>  	struct cpuidle_device *dev;
>>>  	struct cpuidle_driver *drv = &tegra_idle_driver;
>>>  
>>> +#ifndef CONFIG_PM_SLEEP
>>> +	drv->state_count = 1;	/* support clockgating only */
>>> +#endif
>>
>> Oh, I see it's done here. Just fixing the static initialization seems a
>> lot simpler?
>>
> OK. Will do.
> 
>>> diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
>>
>>> +void __cpuinit tegra_clear_cpu_in_lp2(int cpu)
>>> +{
>>> +	spin_lock(&tegra_lp2_lock);
>>> +	BUG_ON(!cpumask_test_cpu(cpu, &tegra_in_lp2));
>>> +	cpumask_clear_cpu(cpu, &tegra_in_lp2);
>>> +
>>> +	/*
>>> +	 * Update the IRAM copy used by the reset handler. The IRAM copy
>>> +	 * can't use used directly by cpumask_clear_cpu() because it uses
>>> +	 * LDREX/STREX which requires the addressed location to be inner
>>> +	 * cacheable and sharable which IRAM isn't.
>>> +	 */
>>> +	writel(tegra_in_lp2.bits[0], tegra_cpu_lp2_mask);
>>> +	dsb();
>>
>> Why not /just/ store the data in IRAM, and read/write directly to it,
>> rather than maintaining an SDRAM-based copy of it?
>>
>> Then, wouldn't the body of this function be simply:
>>
>> spin_lock();
>> BUG_ON(!(tegra_cpu_lp2_mask & BIT(cpu)));
>> tegra_cpu_lp2_mask |= BIT(cpu);
>> spin_unlock();
>>
> 
> It may not simple like this. To maintain it identical to a cpumask. It
> may look likes below. Because I need to compare it with cpu_online_mask.

Oh, the comparison against cpu_online_mask() is what I was missing. I
guess that offline CPUs don't go into LP2, so you can't just check that
tegra_cpu_lp2_mask == (1 << num_cpus()) - 1.

One way to avoid that might be to maintain a cpu_in_lp2_count variable
alongside the mask, and simply compare that against num_online_cpus()
rather than comparing the two masks. At least that would avoid the
following line:

>>> +	writel(tegra_in_lp2.bits[0], tegra_cpu_lp2_mask);

... making use of knowledge of the internal structure of the struct
cpumask type.

However, given the comparison requirement, either way is probably fine.

^ permalink raw reply

* alignment faults in 3.6
From: Måns Rullgård @ 2012-10-11 16:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B703E@saturn3.aculab.com>

"David Laight" <David.Laight@ACULAB.COM> writes:

>> Not exactly. It is asked to to perform 2 32-bit loads which are combined
>> into a single ldm (load multiple) which cannot handle unaligned
>> accesses. Here's a simple example that does the same thing:
>> 
>> void test(char * buf)
>> {
>> 	printf("%d, %d\n", *((unsigned int *)&buf[0]), *((unsigned int *)&buf[4]));
>> }
>
> Have you actually looked at what an ARM processor traditionally did
> with misaligned memory reads?
> While useful, it probably wasn't what was intended.
>
> Actually, and IIRC, some very recent ARM cpus will do the 'expected'
> thing for single-word loads from misaligned addesses.

What various CPUs do with unaligned accesses is not the issue here.  The
casts in the code above act as a promise to the compiler that the
address is in fact properly align for the pointer type.

> However they almost certainly won't for ldm/stm.
>
> The 'ldm' optimisation for adjacent memory loads is also dubious.

There is nothing whatsoever dubious about the compiler using the most
efficient instruction sequence to accomplish what the code asks for.

> On at least some ARMs it is very slow (might only be strongarms).

The compiler will pick instructions suitable for the CPU you specify.

>> So I guess the only ABI legal unaligned access is in a packed struct.
>
> Correct. And you mustn't try casting the address, the compiler is
> allowed to remember where it came from.
> (This causes a lot of grief...)

It is only a problem when you try to outsmart the compiler.

> If you are targeting the ARM cpu that can do misaligned transfers,
> then gcc should generate single instructions for misaligned structure
> members, and never do the 'ldm' optimisations.

That is exactly how gcc works.

> But, the IP header is expected to be aligned.

Everything tells the compiler the struct is perfectly aligned.  When the
buggy driver passes a misaligned pointer, bad things happen.

-- 
M?ns Rullg?rd
mans at mansr.com

^ permalink raw reply

* alignment faults in 3.6
From: Eric Dumazet @ 2012-10-11 16:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5076E472.8030703@gmail.com>

On Thu, 2012-10-11 at 10:23 -0500, Rob Herring wrote:
> On 10/11/2012 08:47 AM, Eric Dumazet wrote:

> > Compiler is asked to perform a 32bit load, it does it.
> 
> Not exactly. It is asked to to perform 2 32-bit loads which are combined
> into a single ldm (load multiple) which cannot handle unaligned
> accesses. Here's a simple example that does the same thing:


Thats simply not true. You are severely mistaken.

ldm does a load of seeral 32bit words.

And the compiler would not use it if the alignment was not matching the
prereq (alignment >= 4)



> 
> void test(char * buf)
> {
> 	printf("%d, %d\n", *((unsigned int *)&buf[0]), *((unsigned int *)&buf[4]));
> }

But you completely miss the fact that network doesnt pass a "char *buf"
but a "be32 *buf". Your example is not relevant at all.

So the compiler is absolutely right, and network stack is _right_ too.

The prereq is that IP header are aligned to 4 bytes boundary.

Denying this fact is not going to help you


> 
> So I guess the only ABI legal unaligned access is in a packed struct.
> 
> > There is no questionable optimization here. Really.
> > Please stop pretending this, this makes no sense.
> 
> I'm not the one calling the networking stack bad code.

Once you understand the issues, you can explain us where is the bad
code. But given you say "Bug is in compiler, and/or network stack, but
my driver is fine", its not very wise.

For the moment, the bug is in your driver.

> 
> I can fix my h/w, so I'll stop caring about this. Others can all get
> bitten by this new behavior in gcc 4.7.

Again compiler is fine. How should we say that so that you stop your
rants ?

Stop trying to find an excuse, dont try to fool us, this is really
embarrassing. Just fix the driver, there is no shame to fix an error.

^ permalink raw reply

* ARM_ATAG_DTB_COMPAT fails on fdt_ro.c compiling
From: Stephen Warren @ 2012-10-11 16:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121011072428.GA26298@S2101-09.ap.freescale.net>

On 10/11/2012 01:24 AM, Shawn Guo wrote:
> On Tue, Oct 09, 2012 at 01:21:02PM -0300, Fabio Estevam wrote:
>> On Tue, Oct 9, 2012 at 1:15 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>
>>>>> I just tried building mainline now.  It is at commit f5a246eab9.
>>>>>
>>>> The build still fails for me at this commit.
>>>
>>> I can't reproduce this.
>>
>> Same here. It builds fine for me.
>>
> Thanks all for confirming it works.
> 
> I just figured it out on my end.  Here is how it gets broken for me.
> 
> - Before commit cd29672 (dtc: import latest upstream dtc) gets in,
>   I did a build without O=<output_dir>.  It results in a copy of those
>   libfdt files in arch/arm/boot/compressed.
> 
> - After the commit gets in, I start a build with O=imx parameter.  When
>   compiling imx/arch/arm/boot/compressed/fdt_ro.c, the #include <libfdt.h>
>   in fdt_ro.c seems to match the one left in arch/arm/boot/compressed/
>   from last build rather than imx/arch/arm/boot/compressed/libfdt.h.
> 
> Cleaning the libfdt copies in arch/arm/boot/compressed fixes the
> problem.

> Not sure why it's not "libfdt.h" but <libfdt.h> is used as
> the inclusion though, since the .h and .c are in the same folder.

I imagine that's because the libfdt code is set up to support being
built as a standalone library. In that case, <fdt.h> and <libfdt.h> are
presumably the public interface to the library, and would hence be
installed into /usr/include or similar. Still, I guess the libfdt source
itself would still work with "libfdt.h" rather than <libfdt.h>.

Note that looking at git history, fdt_ro.c included <libfdt.h> even
before cd29672 (dtc: import latest upstream dtc), and
arch/arm/boot/compressed/Makefile included fdt_ro.c before that commit,
so I'm not sure why the problem only shows up now? Perhaps anything that
caused your objects to be stale, and hence get rebuilt, would have
caused the issue.

^ permalink raw reply

* [PATCH 07/11] fsmc/nand: Provide contiguous buffers to dma
From: Linus Walleij @ 2012-10-11 16:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAOh2x=ksMr7VRCnpF-rK+Mesg1p4Td++SQ-t=ShrguRRVWSSSQ@mail.gmail.com>

On Thu, Oct 11, 2012 at 6:15 AM, viresh kumar <viresh.kumar@linaro.org> wrote:
> On Wed, Oct 10, 2012 at 10:37 PM, Linus Walleij
> <linus.walleij@linaro.org> wrote:

>> dma_sync_single_for_device() is translating the virtual
>> address to physical for every chunk BTW.
>
> I pray that i am wrong here, otherwise i would be thrown out from
> the maintainers list for this driver :)
>
> dma_sync_single_for_device() is not doing anything on the buffer, but
> on the LLI item. Actually it is flushing LLI struct so that DMA h/w can get
> the correct values.

Sorry no, I'm the one who's wrong...

So the DMA engine memcpy() is not mapping virt->phys
but expects physical addresses to be provided.

So dma_map_single() needs to be called on the stuff
passed in to dev->device_prep_dma_memcpy().

And currently there is indeed a dma_map_single() in
dma_xfer() in fsmc_nand.c which should work just fine.

dma_map_single() will only work if the buffer is
physically contiguous.

And the block layer of the subsystem should take care
of only handing the driver buffers that are contiguous
I think? Not that I'm an expert here ... more some
guesswork :-/

Artem will know I hope!

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH] ARM: decompressor: clear SCTLR.A bit for v7 cores
From: Nicolas Pitre @ 2012-10-11 15:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121011135925.GR4625@n2100.arm.linux.org.uk>

On Thu, 11 Oct 2012, Russell King - ARM Linux wrote:

> On Thu, Oct 11, 2012 at 08:31:47AM -0500, Rob Herring wrote:
> > This only affects v7 cores. It should not vary for v7 cores as unaligned
> > access is a required feature. So how is it going to vary on v7 CPUs?
> > We've got bigger problems if there are v7 cores that don't handle
> > unaligned accesses.
> 
> Oh, and this gives me a third reason to NAK this patch.  Why only ensure
> that the A bit is clear for v7 CPUs?  Why not v6, v5, v4 too?  Why does
> ARMv7 get this special treatment?

As I said, gcc knows that ARMv7 can perform word sized accesses even 
with misaligned pointers.  So when it is passed a pointer marked with 
the packed attribute, it will generate a series of byte accesses when 
compiling for anything but ARMv7, and use a single ldr or str when 
compiling for ARMv7.


Nicolas

^ permalink raw reply

* [PATCH 4/8] cpufreq: db8500: Fetch cpufreq table from platform data
From: Lee Jones @ 2012-10-11 15:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121011134140.GA15428@gmail.com>

On Thu, 11 Oct 2012, Lee Jones wrote:

> > -static struct cpufreq_frequency_table freq_table[] = {
> > -	[0] = {
> > -		.index = 0,
> > -		.frequency = 200000,
> > -	},
> > -	[1] = {
> > -		.index = 1,
> > -		.frequency = 400000,
> > -	},
> > -	[2] = {
> > -		.index = 2,
> > -		.frequency = 800000,
> > -	},
> > -	[3] = {
> > -		/* Used for MAX_OPP, if available */
> > -		.index = 3,
> > -		.frequency = CPUFREQ_TABLE_END,
> > -	},
> > -	[4] = {
> > -		.index = 4,
> > -		.frequency = CPUFREQ_TABLE_END,
> > -	},
> > -};
> 
> So where has this table gone? Am I missing a patch?

Ah, I see it now. Nice!

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH v2 1/4] net: mvneta: driver for Marvell Armada 370/XP network unit
From: Rob Herring @ 2012-10-11 15:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1349969282-12676-2-git-send-email-thomas.petazzoni@free-electrons.com>

On 10/11/2012 10:27 AM, Thomas Petazzoni wrote:
> This patch contains a new network driver for the network unit of the
> ARM Marvell Armada 370 and the Armada XP. Both SoCs use the PJ4B
> processor, a Marvell-developed ARM core that implements the ARMv7
> instruction set.


> diff --git a/Documentation/devicetree/bindings/net/marvell-neta.txt b/Documentation/devicetree/bindings/net/marvell-neta.txt
> new file mode 100644
> index 0000000..a031978
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/marvell-neta.txt
> @@ -0,0 +1,24 @@
> +* Marvell Armada 370 / Armada XP Ethernet Controller (NETA)
> +
> +Required properties:
> +- compatible: should be "marvell,neta".

This should be more specific such as "marvell,armada-xp-neta".

Or use 370 instead of xp. It should be which ever chip came first.

> +- reg: address and length of the register set for the device.
> +- interrupts: interrupt for the device
> +- phy-mode: String, operation mode of the PHY interface. Supported
> +  values are "sgmii" and "rmii".
> +- phy-addr: Integer, address of the PHY.
> +- device_type: should be "network".

Drop this. device_type is not used for FDT.

> +- clock-frequency: frequency of the peripheral clock of the SoC.
> +
> +Example:
> +
> +eth at d0070000 {

Should be ethernet at ...

Rob

^ permalink raw reply

* [PATCH] ARM: decompressor: clear SCTLR.A bit for v7 cores
From: Nicolas Pitre @ 2012-10-11 15:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121011134130.GQ4625@n2100.arm.linux.org.uk>

On Thu, 11 Oct 2012, Russell King - ARM Linux wrote:

> On Thu, Oct 11, 2012 at 08:31:47AM -0500, Rob Herring wrote:
> > On 10/11/2012 08:09 AM, Russell King - ARM Linux wrote:
> > > On Thu, Oct 11, 2012 at 07:43:22AM -0500, Rob Herring wrote:
> > >> The contents of this were already reviewed on this thread, so I sent this
> > >> to the patch system and this was Russell's reply:
> > > 
> > > So that's why I couldn't find it - the mailing list thread has a different
> > > subject line to the patch.  Don't do that.  Given the amount of list
> > > traffic we have today, that's as good as not having been posted at all.
> > > 
> > >>> NAK for two reasons.
> > >>>
> > >>> 1. It hasn't been on the list (I can't find a match for "clear SCTLR.A"
> > >>> in my mailbox)
> > >>>
> > >>> 2. The behaviour of unaligned accesses vary depending on CPU.  Some
> > >>> fix-up the access, others load the word and then rotate it.  If we have
> > >>> decompressors which perform unaligned accesses, we need to fix this
> > >>> properly to avoid the CPU specific behaviour, rather than tweaking
> > >>> control bits to hide the problem.
> > >>
> > >> I'm simply matching the behavior of the kernel itself. The A bit is cleared
> > >> for v7 kernels and compilers only generate unaligned accesses for v7.
> > >> Without this the initial state of the A bit is undefined as a bootloader
> > >> could have cleared it already. We should document the required state or set
> > >> it to what we want.
> > > 
> > > Irrespective of this, (2) still stands.  Unaligned accesses in the
> > > decompressor without a fixup (which will be very hard to provide)
> > > will return different data depending on the CPU as I mention in point
> > > 2.
> > 
> > This only affects v7 cores. It should not vary for v7 cores as unaligned
> > access is a required feature. So how is it going to vary on v7 CPUs?
> > We've got bigger problems if there are v7 cores that don't handle
> > unaligned accesses.
> 
> Rob,
> 
> Your patch may only affect v7 cores, but you've raised the issue of the
> decompressor performing unaligned accesses in general.  Shall I re-repeat
> my point over that or is the problem here going to finally sink in?

The decompressor is not performing direct unaligned accesses.  It uses 
the get_unaligned() and put_unaligned() accessors.  That means that 
we're in control of how this is happening.

So let's talk about the how.  On pre ARMv7, those accesses are performed 
with a series of byte accesses.  When compiling for ARMv7, gcc knows and 
that the hardware can do unaligned accesses, and it does optimize its 
output by using ldr/str instructions.  But the A bit has to be cleared 
in that case, and only in that case.  This is why the patch clears the A 
bit only for ARMv7.

So this patch is only setting up the hardware to match gcc's 
expectations when generating code from the use of get_unaligned() and 
put_unaligned() when optimizing for ARMv7.

As always, any code doing unaligned access and _not_ using those 
accessors is broken.


Nicolas

^ permalink raw reply

* alignment faults in 3.6
From: David Laight @ 2012-10-11 15:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5076E472.8030703@gmail.com>

 
> Not exactly. It is asked to to perform 2 32-bit loads which are combined
> into a single ldm (load multiple) which cannot handle unaligned
> accesses. Here's a simple example that does the same thing:
> 
> void test(char * buf)
> {
> 	printf("%d, %d\n", *((unsigned int *)&buf[0]), *((unsigned int *)&buf[4]));
> }

Have you actually looked at what an ARM processor traditionally did
with misaligned memory reads?
While useful, it probably wasn't what was intended.

Actually, and IIRC, some very recent ARM cpus will do the 'expected'
thing for single-word loads from misaligned addesses.
However they almost certainly won't for ldm/stm.

The 'ldm' optimisation for adjacent memory loads is also dubious.
On at least some ARMs it is very slow (might only be strongarms).

> So I guess the only ABI legal unaligned access is in a packed struct.

Correct. And you mustn't try casting the address, the compiler is
allowed to remember where it came from.
(This causes a lot of grief...)

If you are targeting the ARM cpu that can do misaligned transfers,
then gcc should generate single instructions for misaligned structure
members, and never do the 'ldm' optimisations.

But, the IP header is expected to be aligned.

	David

^ permalink raw reply

* [PATCH] Boottime: A tool for automatic measurement of kernel/bootloader boot time
From: Lee Jones @ 2012-10-11 15:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAH9NwWe+s1p73Jwa=s5aY_KjGS2nvP8LR2YH3i5gtpFMUYMiuw@mail.gmail.com>

On Thu, 11 Oct 2012, Christian Gmeiner wrote:

> 2012/10/11 Lee Jones <lee.jones@linaro.org>:
> > From: Jonas Aaberg <jonas.aberg@stericsson.com>
> >
> > The overhead is very low and the results will be found under
> > sysfs/bootime, as well as detailed results in debugfs under
> > boottime/. The bootgraph* files are compatible with
> > scripts/bootgraph.pl. The reason for this patch is to provide
> > data (sysfs/boottime) suitable for automatic testcases as
> > well as help for developers to reduce the boot time (debugfs).
> >
> 
> Nice idea... what about x86?

What about it?

You want to extend the functionality to include it? :)

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH v2 4/4] arm: mvebu: enable Ethernet controllers on Armada 370/XP eval boards
From: Thomas Petazzoni @ 2012-10-11 15:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1349969282-12676-1-git-send-email-thomas.petazzoni@free-electrons.com>

This patch enables the two network interfaces of the Armada 370
official Marvell evaluation platform, and the four network interfaces
of the Armada XP official Marvell evaluation platform.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/boot/dts/armada-370-db.dts |   12 ++++++++++++
 arch/arm/boot/dts/armada-xp-db.dts  |   24 ++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-db.dts b/arch/arm/boot/dts/armada-370-db.dts
index fffd5c2..bb18b0d 100644
--- a/arch/arm/boot/dts/armada-370-db.dts
+++ b/arch/arm/boot/dts/armada-370-db.dts
@@ -38,5 +38,17 @@
 			clock-frequency = <600000000>;
 			status = "okay";
 		};
+                eth at d0070000 {
+                        clock-frequency = <200000000>;
+                        status = "okay";
+                        phy-mode = "rgmii";
+                        phy-addr = <0>;
+                };
+                eth at d0074000 {
+                        clock-frequency = <200000000>;
+                        status = "okay";
+                        phy-mode = "rgmii";
+                        phy-addr = <1>;
+                };
 	};
 };
diff --git a/arch/arm/boot/dts/armada-xp-db.dts b/arch/arm/boot/dts/armada-xp-db.dts
index f97040d..d1a4173 100644
--- a/arch/arm/boot/dts/armada-xp-db.dts
+++ b/arch/arm/boot/dts/armada-xp-db.dts
@@ -46,5 +46,29 @@
 			clock-frequency = <250000000>;
 			status = "okay";
 		};
+                eth at d0070000 {
+                        clock-frequency = <250000000>;
+                        status = "okay";
+                        phy-mode = "rgmii";
+                        phy-addr = <0>;
+                };
+                eth at d0074000 {
+                        clock-frequency = <250000000>;
+                        status = "okay";
+                        phy-mode = "rgmii";
+                        phy-addr = <1>;
+                };
+                eth at d0030000 {
+                        clock-frequency = <250000000>;
+                        status = "okay";
+                        phy-mode = "sgmii";
+                        phy-addr = <25>;
+                };
+                eth at d0034000 {
+                        clock-frequency = <250000000>;
+                        status = "okay";
+                        phy-mode = "sgmii";
+                        phy-addr = <27>;
+                };
 	};
 };
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 3/4] arm: mvebu: add Ethernet controllers using mvneta driver for Armada 370/XP
From: Thomas Petazzoni @ 2012-10-11 15:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1349969282-12676-1-git-send-email-thomas.petazzoni@free-electrons.com>

The Armada 370 SoC has two network units, while the Armada XP has four
network units. The first two network units are common to both the
Armada XP and Armada 370, so they are added to armada-370-xp.dtsi,
while the other two network units are specific to the Armada XP and
therefore added to armada-xp.dtsi.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/boot/dts/armada-370-xp.dtsi |   16 ++++++++++++++++
 arch/arm/boot/dts/armada-xp.dtsi     |   16 ++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi
index 6b6b932..52af710 100644
--- a/arch/arm/boot/dts/armada-370-xp.dtsi
+++ b/arch/arm/boot/dts/armada-370-xp.dtsi
@@ -63,6 +63,22 @@
 			       reg = <0xd0020300 0x30>;
 			       interrupts = <37>, <38>, <39>, <40>;
 		};
+
+                eth at d0070000 {
+                               compatible = "marvell,neta";
+                               reg = <0xd0070000 0x2500>;
+                               interrupts = <8>;
+                               device_type = "network";
+                               status = "disabled";
+                };
+
+                eth at d0074000 {
+                               compatible = "marvell,neta";
+                               reg = <0xd0074000 0x2500>;
+                               interrupts = <10>;
+                               device_type = "network";
+                               status = "disabled";
+                };
 	};
 };
 
diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
index 71d6b5d..59167dc 100644
--- a/arch/arm/boot/dts/armada-xp.dtsi
+++ b/arch/arm/boot/dts/armada-xp.dtsi
@@ -51,5 +51,21 @@
 				compatible = "marvell,armada-370-xp-system-controller";
 				reg = <0xd0018200 0x500>;
 		};
+
+                eth at d0030000 {
+                               compatible = "marvell,neta";
+                               reg = <0xd0030000 0x2500>;
+                               interrupts = <12>;
+                               device_type = "network";
+                               status = "disabled";
+                };
+
+                eth at d0034000 {
+                               compatible = "marvell,neta";
+                               reg = <0xd0034000 0x2500>;
+                               interrupts = <14>;
+                               device_type = "network";
+                               status = "disabled";
+                };
 	};
 };
-- 
1.7.9.5

^ permalink raw reply related


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