* [PATCH 1/7] ARM: tegra: cpuidle: separate cpuidle driver for different chips
From: Joseph Lo @ 2012-10-11 6:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5074A3C3.1080006@wwwdotorg.org>
On Wed, 2012-10-10 at 06:22 +0800, Stephen Warren wrote:
> On 10/08/2012 04:26 AM, Joseph Lo wrote:
> > The different Tegra chips may have different CPU idle states and data.
> > Individual CPU idle driver make it more easy to maintain.
>
> > diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
> > similarity index 72%
> > copy from arch/arm/mach-tegra/cpuidle.c
> > copy to arch/arm/mach-tegra/cpuidle-tegra20.c
> > index 4e0b07c..e2fc26a 100644
> > --- a/arch/arm/mach-tegra/cpuidle.c
> > +++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
> > @@ -1,24 +1,22 @@
> > /*
> > - * arch/arm/mach-tegra/cpuidle.c
> > + * CPU idle driver for Tegra20 CPUs
> > *
> > - * CPU idle driver for Tegra CPUs
> > - *
> > - * Copyright (c) 2010-2012, NVIDIA Corporation.
> > + * Copyright (c) 2010-2012, NVIDIA Corporation. All rights reserved.
> > * Copyright (c) 2011 Google, Inc.
> > * Author: Colin Cross <ccross@android.com>
> > * Gary King <gking@nvidia.com>
> > *
> > - * Rework for 3.3 by Peter De Schrijver <pdeschrijver@nvidia.com>
> > - *
> > - * This program is free software; you can redistribute it and/or modify
> > - * it under the terms of the GNU General Public License as published by
> > - * the Free Software Foundation; either version 2 of the License, or
> > - * (at your option) any later version.
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions 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/>.
> > */
>
> While that is the correct (c) header for new work submitted upstream,
> given this file is existing, just renamed and tweaked a bit, I don't
> believe you want to be changing the (c) header at all. Same for
> cpuidle-tegra30.c.
OK. I would just remove this line.
- * arch/arm/mach-tegra/cpuidle.c
Thanks,
Joseph
^ permalink raw reply
* [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support
From: Joseph Lo @ 2012-10-11 6:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5074A47B.3050906@wwwdotorg.org>
Hi Stephen,
Thanks for review and sorry for late response due to national holiday
yesterday.
On Wed, 2012-10-10 at 06:26 +0800, Stephen Warren wrote:
> On 10/08/2012 04:26 AM, Joseph Lo wrote:
> > The CPU idle LP2 is a power gating idle mode for Tegra30. It supports the
> > secondary CPUs (i.e., CPU1-CPU3) to go into LP2 dynamically. When any of
> > the secondary CPUs go into LP2, it can be power gated alone. There is a
> > limitation on CPU0. The CPU0 can go into LP2 only when all secondary CPUs
> > are already in LP2. After CPU0 is in LP2, the CPU rail can be turned off.
> >
> > Verified on Seaboard(Tegra20) and Cardhu(Tegra30).
>
> What's the most comprehensive way to verify this? I booted Cardhu with
> these patches applied and saw that all CPU cores did enter both idle
> states. However, I'm unsure what the best way to stress the system is,
> i.e. how would I stress and test for correct handling of all the L2
> caching/SMP coherency issues, etc.
Yes, we need a pattern to verify this. The idea is try to make data
traffic busy and the CPU can still fall into LP2. So I used a software
video decoding process to verify this. You can image how the data
traffic it is. The raw video data from file system to memory. Memory to
memory access during video decoding process. The L1/L2 cache and TLB
maintenance procedure can be verified.
To verify this, you need to get a mplayer and "lower resolution" video
clips. Using DVD resolution would be good choice. Then creating two
decoding processes in the background. Please do check the system can
fall into LP2. If not, please try to lower the fps value. Finally, using
loop to keep the decoding process always in the background.
The criteria to pass the verification. The system need to keep alive
over a weekend at least.
Thanks,
Joseph
^ permalink raw reply
* [PATCH 3.6.0- 3/6] ARM/pxa: use module_platform_driver macro
From: Igor Grinberg @ 2012-10-11 6:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1349893837-7441-1-git-send-email-srinivas.kandagatla@st.com>
On 10/10/12 20:30, Srinivas KANDAGATLA wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>
> This patch removes some code duplication by using
> module_platform_driver.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Cc'ed Haojian,
I agree on Paul's comment about the description.
Otherwise looks fine to me:
Acked-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
> arch/arm/mach-pxa/pxa3xx-ulpi.c | 13 +------------
> 1 files changed, 1 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/pxa3xx-ulpi.c b/arch/arm/mach-pxa/pxa3xx-ulpi.c
> index 7dbe3cc..e329cce 100644
> --- a/arch/arm/mach-pxa/pxa3xx-ulpi.c
> +++ b/arch/arm/mach-pxa/pxa3xx-ulpi.c
> @@ -384,18 +384,7 @@ static struct platform_driver pxa3xx_u2d_ulpi_driver = {
> .probe = pxa3xx_u2d_probe,
> .remove = pxa3xx_u2d_remove,
> };
> -
> -static int pxa3xx_u2d_ulpi_init(void)
> -{
> - return platform_driver_register(&pxa3xx_u2d_ulpi_driver);
> -}
> -module_init(pxa3xx_u2d_ulpi_init);
> -
> -static void __exit pxa3xx_u2d_ulpi_exit(void)
> -{
> - platform_driver_unregister(&pxa3xx_u2d_ulpi_driver);
> -}
> -module_exit(pxa3xx_u2d_ulpi_exit);
> +module_platform_driver(pxa3xx_u2d_ulpi_driver);
>
> MODULE_DESCRIPTION("PXA3xx U2D ULPI driver");
> MODULE_AUTHOR("Igor Grinberg");
--
Regards,
Igor.
^ permalink raw reply
* [PATCH] usb: remove CONFIG_USB_MUSB_HOST etc
From: Heiko Schocher @ 2012-10-11 6:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <A73F36158E33644199EB82C5EC81C7BC3E9CA495@DBDE01.ent.ti.com>
Hello Manjunathappa
On 11.10.2012 07:42, Manjunathappa, Prakash wrote:
> Hi,
> On Mon, Oct 08, 2012 at 18:47:07, Constantine Shulyupin wrote:
>> From: Constantine Shulyupin<const@MakeLinux.com>
>>
>> Remove USB configuration in arch/arm/mach-davinci/usb.c accordingly
>> CONFIG_USB_MUSB_OTG CONFIG_USB_MUSB_PERIPHERAL CONFIG_USB_MUSB_HOST
>> and set MUSB_OTG configuration by default
>> because this configuration options are removed from Kconfig.
>>
>> Signed-off-by: Constantine Shulyupin<const@MakeLinux.com>
>>
>> ---
>> arch/arm/mach-davinci/usb.c | 6 ------
>> 1 file changed, 6 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
>> index f77b953..34509ff 100644
>> --- a/arch/arm/mach-davinci/usb.c
>> +++ b/arch/arm/mach-davinci/usb.c
>> @@ -42,14 +42,8 @@ static struct musb_hdrc_config musb_config = {
>> };
>>
>> static struct musb_hdrc_platform_data usb_data = {
>> -#if defined(CONFIG_USB_MUSB_OTG)
>> /* OTG requires a Mini-AB connector */
>> .mode = MUSB_OTG,
>> -#elif defined(CONFIG_USB_MUSB_PERIPHERAL)
>> - .mode = MUSB_PERIPHERAL,
>> -#elif defined(CONFIG_USB_MUSB_HOST)
>> - .mode = MUSB_HOST,
>> -#endif
>> .clock = "usb",
>> .config =&musb_config,
>> };
>
> Tested it on DM6446-EVM for host mode with MSC thumb drive and gadget
> mode with g-ether. It works.
>
> Acked-by: Manjunathappa, Prakash<prakash.pm@ti.com>
I sent a similiar patch here:
http://comments.gmane.org/gmane.linux.usb.general/54512
If the issues, mentioned from Sergei for my patch, nullified I add my:
Acked-by: Heiko Schocher <hs@denx.de>
for this patch.
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply
* [PATCH 1/3] ARM: dts: Renaming elpida_ecb240abacn.dtsi as lpddr2_data.dtsi
From: Lokesh Vutla @ 2012-10-11 6:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50758AA0.5080307@ti.com>
+ devicetree-discuss
Hi Benoit,
On Wednesday 10 October 2012 08:18 PM, Benoit Cousson wrote:
> Hi Lokesh,
>
> On 10/10/2012 02:05 PM, Lokesh Vutla wrote:
>> Renaming elpida_ecb240abacn.dtsi file generic, so that the
>> same file can be reused for other parts from different vendors.
>
> Could you elaborate a little bit?
> Are these settings purely reflecting lpddr2 spec timings?
The basic idea is to group data for all the lpddr2 devices in a single
file instead of creating separate file for each lpddr2 device.
Right now the file elpida_ecb240abacn.dtsi contains only data for
lpddr2-s4 2G parts from Elpida.
I wanted to add data for lpddr2-s4 4G parts from Samsung.
So I renamed the file elpida_ecb240abacn.dtsi as lpddr2_data.dtsi.
Please let me know if more clarification is needed.
Thanks,
Lokesh
>
> Regards,
> Benoit
>
>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>> arch/arm/boot/dts/{elpida_ecb240abacn.dtsi => lpddr2_data.dtsi} | 0
>> arch/arm/boot/dts/omap4-panda.dts | 2 +-
>> arch/arm/boot/dts/omap4-sdp.dts | 2 +-
>> 3 files changed, 2 insertions(+), 2 deletions(-)
>> rename arch/arm/boot/dts/{elpida_ecb240abacn.dtsi => lpddr2_data.dtsi} (100%)
>>
>> diff --git a/arch/arm/boot/dts/elpida_ecb240abacn.dtsi b/arch/arm/boot/dts/lpddr2_data.dtsi
>> similarity index 100%
>> rename from arch/arm/boot/dts/elpida_ecb240abacn.dtsi
>> rename to arch/arm/boot/dts/lpddr2_data.dtsi
>> diff --git a/arch/arm/boot/dts/omap4-panda.dts b/arch/arm/boot/dts/omap4-panda.dts
>> index 20b966e..09d3a32 100644
>> --- a/arch/arm/boot/dts/omap4-panda.dts
>> +++ b/arch/arm/boot/dts/omap4-panda.dts
>> @@ -8,7 +8,7 @@
>> /dts-v1/;
>>
>> /include/ "omap4.dtsi"
>> -/include/ "elpida_ecb240abacn.dtsi"
>> +/include/ "lpddr2_data.dtsi"
>>
>> / {
>> model = "TI OMAP4 PandaBoard";
>> diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
>> index 94a23b3..dd749fc 100644
>> --- a/arch/arm/boot/dts/omap4-sdp.dts
>> +++ b/arch/arm/boot/dts/omap4-sdp.dts
>> @@ -8,7 +8,7 @@
>> /dts-v1/;
>>
>> /include/ "omap4.dtsi"
>> -/include/ "elpida_ecb240abacn.dtsi"
>> +/include/ "lpddr2_data.dtsi"
>>
>> / {
>> model = "TI OMAP4 SDP board";
>>
>
^ permalink raw reply
* [PATCH 3/3] ARM: dts: EMIF and LPDDR2 device tree data for OMAP5 boards
From: Lokesh Vutla @ 2012-10-11 6:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50758DC7.3070408@ti.com>
+ devicetree-discuss
Hi Benoit,
On Wednesday 10 October 2012 08:31 PM, Benoit Cousson wrote:
> On 10/10/2012 02:05 PM, Lokesh Vutla wrote:
>> Device tree data for the EMIF sdram controllers in OMAP5
>> and LPDDR2 memory devices attached to OMAP5 boards.
>
> Nit: Could you make a sentence with a verb to explain what you are doing
> in this patch.
I am really sorry about this.
I ll make sure that all patch descriptions will be clear in V2 of this
patch series.
In this patch I am adding device tree data for LPDDR2 memory devices
attached to omap5-sevm and also adding device tree data for EMIF sdram
controllers in OMAP5.
>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>> arch/arm/boot/dts/lpddr2_data.dtsi | 64 +++++++++++++++++++++++++++++++++++-
>> arch/arm/boot/dts/omap5-evm.dts | 11 +++++++
>> arch/arm/boot/dts/omap5.dtsi | 18 ++++++++++
>> 3 files changed, 92 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/lpddr2_data.dtsi b/arch/arm/boot/dts/lpddr2_data.dtsi
>> index f97f70f..8e8c1bc 100644
>> --- a/arch/arm/boot/dts/lpddr2_data.dtsi
>> +++ b/arch/arm/boot/dts/lpddr2_data.dtsi
>> @@ -3,7 +3,7 @@
>> */
>>
>> / {
>> - elpida_ECB240ABACN: lpddr2 {
>> + elpida_ECB240ABACN: lpddr2 at 0 {
>> compatible = "Elpida,ECB240ABACN","jedec,lpddr2-s4";
>> density = <2048>;
>> io-width = <32>;
>> @@ -64,4 +64,66 @@
>> tDQSCK-max-derated = <6000>;
>> };
>> };
>> +
>> + samsung_K3PE0E000B: lpddr2 at 1 {
>
> I'm confused now, why are you reusing the same lpddr2_data.dtsi file?
> You should create a file per memory. That will make the reuse much easier.
>
> If the goal of your first patch was to do that, it is then the wrong
> approach.
Yes, I wanted to group data for all lppdr2 devices in a single file than
creating separate file for each device.
May be a dumb question, Why can't we group data for all the lpddr2
devices in a single file?
>
>> + compatible = "Samsung,K3PE0E000B","jedec,lpddr2-s4";
>> + density = <4096>;
>> + io-width = <32>;
>> +
>> + tRPab-min-tck = <3>;
>> + tRCD-min-tck = <3>;
>> + tWR-min-tck = <3>;
>> + tRASmin-min-tck = <3>;
>> + tRRD-min-tck = <2>;
>> + tWTR-min-tck = <2>;
>> + tXP-min-tck = <2>;
>> + tRTP-min-tck = <2>;
>> + tCKE-min-tck = <3>;
>> + tCKESR-min-tck = <3>;
>> + tFAW-min-tck = <8>;
>> +
>> + timings_samsung_K3PE0E000B_533mhz: lpddr2-timings at 0 {
>> + compatible = "jedec,lpddr2-timings";
>> + min-freq = <10000000>;
>> + max-freq = <533333333>;
>> + tRPab = <21000>;
>> + tRCD = <18000>;
>> + tWR = <15000>;
>> + tRAS-min = <42000>;
>> + tRRD = <10000>;
>> + tWTR = <7500>;
>> + tXP = <7500>;
>> + tRTP = <7500>;
>> + tCKESR = <15000>;
>> + tDQSCK-max = <5500>;
>> + tFAW = <50000>;
>> + tZQCS = <90000>;
>> + tZQCL = <360000>;
>> + tZQinit = <1000000>;
>> + tRAS-max-ns = <70000>;
>> + tDQSCK-max-derated = <5620>;
>> + };
>> +
>> + timings_samsung_K3PE0E000B_266mhz: lpddr2-timings at 1 {
>> + compatible = "jedec,lpddr2-timings";
>> + min-freq = <10000000>;
>> + max-freq = <266666666>;
>> + tRPab = <21000>;
>> + tRCD = <18000>;
>> + tWR = <15000>;
>> + tRAS-min = <42000>;
>> + tRRD = <10000>;
>> + tWTR = <7500>;
>> + tXP = <7500>;
>> + tRTP = <7500>;
>> + tCKESR = <15000>;
>> + tDQSCK-max = <5500>;
>> + tFAW = <50000>;
>> + tZQCS = <90000>;
>> + tZQCL = <360000>;
>> + tZQinit = <1000000>;
>> + tRAS-max-ns = <70000>;
>> + tDQSCK-max-derated = <6000>;
>> + };
>> + };
>> };
>> diff --git a/arch/arm/boot/dts/omap5-evm.dts b/arch/arm/boot/dts/omap5-evm.dts
>> index 6f87e1a..8a952f8 100644
>> --- a/arch/arm/boot/dts/omap5-evm.dts
>> +++ b/arch/arm/boot/dts/omap5-evm.dts
>> @@ -8,6 +8,7 @@
>> /dts-v1/;
>>
>> /include/ "omap5.dtsi"
>> +/include/ "lpddr2_data.dtsi"
>>
>> / {
>> model = "TI OMAP5 EVM board";
>> @@ -82,3 +83,13 @@
>> 0x020700d9>; /* SEARCH */
>> linux,input-no-autorepeat;
>> };
>> +
>> +&emif1 {
>> + cs1-used;
>> + device-handle = <&samsung_K3PE0E000B>;
>> +};
>> +
>> +&emif2 {
>> + cs1-used;
>> + device-handle = <&samsung_K3PE0E000B>;
>> +};
>> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
>> index 5db33f4..40b41c2 100644
>> --- a/arch/arm/boot/dts/omap5.dtsi
>> +++ b/arch/arm/boot/dts/omap5.dtsi
>> @@ -319,5 +319,23 @@
>> ti,buffer-size = <128>;
>> ti,hwmods = "mcbsp3";
>> };
>> +
>> + emif1: emif at 0x4c000000 {
>> + compatible = "ti,emif-4d5";
>> + ti,hwmods = "emif1";
>> + phy-type = <2>;
>> + hw-caps-read-idle-ctrl;
>> + hw-caps-ll-interface;
>> + hw-caps-temp-alert;
>
> You should now add the reg, and interrupt attributes as well.
OK I ll add these fields.
Thanks,
Lokesh
>
> Regards,
> Benoit
>
^ permalink raw reply
* bug with 3.4.6, 3.5.3, 3.6.1
From: Gilles Chanteperdrix @ 2012-10-11 5:46 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
when booting Linux v3.4.6, v3.5.3, or v3.6.1 on a pandaboard with an
OMAP4430 ES2.1, compiled with the following configuration:
http://xenomai.org/~gch/config-panda
I get the bug below after mounting the root filesystem.
CONFIG_VMSPLIT_2G and CONFIG_THUMB2_KERNEL disabled seems to be the
combination which triggers the bug.
With this configuration, it seems the init_mm.mm_count incrementation
done at the beginning of secondary_start_kernel() is "lost" after the
calls to cpu_switch_mm() and local_flush_tlb().
Modifying the secondary_startup() function in head.S to pass the
swapper pgdir instead of the idmap pgdir in r4 also avoids the issue.
Regards.
init (301): undefined instruction: pc=80027fe0
Code: e59f3058 e1a04000 e1500003 1a000000 (e7f001f2)
------------[ cut here ]------------
kernel BUG at kernel/fork.c:558!
Internal error: Oops - BUG: 0 [#1] SMP ARM
CPU: 1 Tainted: G W (3.4.6+ #61)
PC is at __mmdrop+0x1c/0x78
LR is at finish_task_switch+0xa4/0xec
pc : [<80027fe0>] lr : [<80049a78>] psr: 60000113
sp : bfa79f60 ip : bfa79f78 fp : bfa79f74
r10: 00000000 r9 : 00000000 r8 : 00000000
r7 : 00000000 r6 : 00000000 r5 : bf83a080 r4 : 803655c8
r3 : 803655c8 r2 : 00000000 r1 : 00000000 r0 : 803655c8
Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
Control: 10c53c7d Table: bfa7c04a DAC: 00000015
Process init (pid: 301, stack limit = 0xbfa782f0)
Stack: (0xbfa79f60 to 0xbfa7a000)
9f60: 803655c8 bf83a080 bfa79f94 bfa79f78 80049a78 80027fd0 80be2100 bf83a080
9f80: 00000000 00000000 bfa79fac bfa79f98 8004b2e0 800499e0 00000000 00000000
9fa0: 00000000 bfa79fb0 8000de44 8004b2ac 00000000 7ee4aba0 76f734c0 00000000
9fc0: 000e21e0 7ee4ac7c 00000001 000000be 00000305 00000000 0001196c 00000000
9fe0: 00000075 7ee4ac30 000a2494 76e07384 60000010 80000000 ffefffff 00000000
Backtrace:
[<80027fc4>] (__mmdrop+0x0/0x78) from [<80049a78>] (finish_task_switch+0xa4/0xec)
r5:bf83a080 r4:803655c8
[<800499d4>] (finish_task_switch+0x0/0xec) from [<8004b2e0>] (schedule_tail+0x40/0xd0)
r7:00000000 r6:00000000 r5:bf83a080 r4:80be2100
[<8004b2a0>] (schedule_tail+0x0/0xd0) from [<8000de44>] (ret_from_fork+0x8/0x44)
r5:00000000 r4:00000000
Code: e59f3058 e1a04000 e1500003 1a000000 (e7f001f2)
mappings:
0x00008000-0x000d7000 r-xp 0x00000000 /bin/busybox
0x000df000-0x000e0000 rw-p 0x000cf000 /bin/busybox
0x000e0000-0x00103000 rw-p 0x000e0000 [heap]
0x76d66000-0x76ea1000 r-xp 0x00000000 /lib/libc.so.6
0x76ea1000-0x76ea9000 ---p 0x0013b000 /lib/libc.so.6
0x76ea9000-0x76eab000 r--p 0x0013b000 /lib/libc.so.6
0x76eab000-0x76eac000 rw-p 0x0013d000 /lib/libc.so.6
0x76eac000-0x76eaf000 rw-p 0x76eac000
0x76eaf000-0x76f47000 r-xp 0x00000000 /lib/libm.so.6
0x76f47000-0x76f4e000 ---p 0x00098000 /lib/libm.so.6
0x76f4e000-0x76f4f000 r--p 0x00097000 /lib/libm.so.6
0x76f4f000-0x76f50000 rw-p 0x00098000 /lib/libm.so.6
0x76f50000-0x76f6f000 r-xp 0x00000000 /lib/ld-linux.so.3
0x76f73000-0x76f74000 rw-p 0x76f73000
0x76f75000-0x76f76000 rw-p 0x76f75000
0x76f76000-0x76f77000 r--p 0x0001e000 /lib/ld-linux.so.3
0x76f77000-0x76f78000 rw-p 0x0001f000 /lib/ld-linux.so.3
0x7ee29000-0x7ee4b000 rw-p 0x7efde000 [stack]
---[ end trace 1b75b31a2719ed1e ]---
--
Gilles.
^ permalink raw reply
* [PATCH] usb: remove CONFIG_USB_MUSB_HOST etc
From: Manjunathappa, Prakash @ 2012-10-11 5:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1349702227-24875-1-git-send-email-const@MakeLinux.com>
Hi,
On Mon, Oct 08, 2012 at 18:47:07, Constantine Shulyupin wrote:
> From: Constantine Shulyupin <const@MakeLinux.com>
>
> Remove USB configuration in arch/arm/mach-davinci/usb.c accordingly
> CONFIG_USB_MUSB_OTG CONFIG_USB_MUSB_PERIPHERAL CONFIG_USB_MUSB_HOST
> and set MUSB_OTG configuration by default
> because this configuration options are removed from Kconfig.
>
> Signed-off-by: Constantine Shulyupin <const@MakeLinux.com>
>
> ---
> arch/arm/mach-davinci/usb.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
> index f77b953..34509ff 100644
> --- a/arch/arm/mach-davinci/usb.c
> +++ b/arch/arm/mach-davinci/usb.c
> @@ -42,14 +42,8 @@ static struct musb_hdrc_config musb_config = {
> };
>
> static struct musb_hdrc_platform_data usb_data = {
> -#if defined(CONFIG_USB_MUSB_OTG)
> /* OTG requires a Mini-AB connector */
> .mode = MUSB_OTG,
> -#elif defined(CONFIG_USB_MUSB_PERIPHERAL)
> - .mode = MUSB_PERIPHERAL,
> -#elif defined(CONFIG_USB_MUSB_HOST)
> - .mode = MUSB_HOST,
> -#endif
> .clock = "usb",
> .config = &musb_config,
> };
Tested it on DM6446-EVM for host mode with MSC thumb drive and gadget
mode with g-ether. It works.
Acked-by: Manjunathappa, Prakash <prakash.pm@ti.com>
Thanks,
Prakash
> --
> 1.7.9.5
>
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source at linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
>
^ permalink raw reply
* [PATCH 4/4] mtd: nand: omap2: Add data correction support
From: Philip, Avinash @ 2012-10-11 5:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121010170806.GB13585@parrot.com>
On Wed, Oct 10, 2012 at 22:38:06, Ivan Djelic wrote:
> On Tue, Oct 09, 2012 at 01:36:50PM +0100, Philip, Avinash wrote:
> (...)
> > > There are at least 2 potential problems when reading an erased page with bitflips:
> > >
> > > 1. bitflip in data area and no bitflip in spare area (all 0xff)
> > > Your code will not perform any ECC correction.
> > > UBIFS does not like finding bitflips in empty pages, see for instance
> > > http://lists.infradead.org/pipermail/linux-mtd/2012-March/040328.html.
> >
> > In case of error correction using ELM, syndrome vector calculated after reading
> > Data area & OOB area. So handling of erased page requires a software workaround.
> > I am planning something as follows.
> >
> > I will first check calculated ecc, which would be zero for non error pages.
> > Then I would check 0xFF in OOB area (for erased page) by checking number of
> > bit zeros in OOB area. If it is 0xFF (number of bit zero count is zero),
> > set entire page as 0xFF if number of bit zeros is less than max bit flips
> > (8 or 4) by counting the number of bit zero's in data area.
> >
> > This logic is implemented in fsmc_nand.c
> >
> > See commit
> > mtd: fsmc: Newly erased page read algorithm implemented
> >
> > >
> > > 2. bitflip in ECC bytes in spare area
> > > Your code will report an uncorrectable error upon reading; if this happens while reading a partially programmed UBI block,
> > > I guess you will lose data.
> >
> > In case of uncorrectable errors due to bit flips in spare area,
> > I can go on checking number of bit zero's in data area + OOB area
> > are less than max bit flips (8 or 4), I can go on setting the entire
> > page as 0xFF.
> >
>
> OK, sounds reasonable.
> Another simple strategy could use the fact that you add a 14th zero byte to
> the 13 BCH bytes for RBL compatibility:
RBL compatibility (14th byte) is applicable only for BCH8 ecc scheme.
So I am planning adding an extra byte (0) for BCH4 ecc scheme. So with this
we can go for same approaches in BCH4 & BCH8 ecc scheme.
If I understood correctly, software BCH ecc scheme is modifying calculated
ecc data to handle bit flips in erased pages.
If that is the only reason, whether same logic can go for same ECC calculation
(remove modification of calculated ecc in case of software ecc correction)
by adding an extra byte (0) in spare area to handle erased pages.
So can you share if I am missing something?
>
> Upon reading:
> - if this 14th byte is zero (*) => page was programmed: perform ECC
> correction as usual
> - else, page was not programmed: do not perform ECC, read entire data+spare
> area, and set it to 0xff if less than 8 or 4 (max bitflips) zero bits
> were found
>
> (*) for robustness to bitflip in 14th byte, replace condition
> "14th byte is zero" by e.g. "14th byte has less than 4 bits set to 1".
>
> What do you think ?
This seems logically good.
Thanks
Avinash
>
> BR,
> --
> Ivan
>
^ permalink raw reply
* [PATCH v2 00/14] OMAP-GPMC related cleanup for common zImage
From: Mohammed, Afzal @ 2012-10-11 5:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121010163840.GA13585@parrot.com>
On Wed, Oct 10, 2012 at 22:08:41, Ivan Djelic wrote:
> On Mon, Oct 08, 2012 at 07:08:08AM +0100, Mohammed, Afzal wrote:
> > Please verify that BCH[48] works as earlier with this series.
> I ran several mtd regression tests on a Beagle Board on your gpmc-czimage-v2 tag.
> All BCH error correcting tests passed successfully.
>
> I occasionally had weird read errors though, especially when reading blank pages:
> the omap driver returned 512-byte sectors containing something like:
> I was able to reproduce the problem also on l2-mtd tip, albeit less often.
> The problem seems to occur quite randomly, it may be a hardware issue on
> my board...
>
> Anyway, the ECC handling part looks OK to me.
Thanks Ivan
Regards
Afzal
^ permalink raw reply
* Booting vanilla kernel on the beaglebone
From: Richard Cochran @ 2012-10-11 5:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <79CD15C6BA57404B839C016229A409A83EB3A700@DBDE01.ent.ti.com>
On Tue, Oct 09, 2012 at 06:34:18PM +0000, Hiremath, Vaibhav wrote:
>
> As I mentioned they are expected to get merged for rc1 and result of
> integration during merge window.
There is a DT binding document
Documentation/devicetree/bindings/net/cpsw.txt
but cpsw does not appear in either am335x-bone.dts, am335x-evm.dts, or
am33xx.dtsi, or for that matter, in *any* dts file anywhere. I have
looked in mainline master, linux-next, and in the master branch of
git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git
Is there an issue keeping the cpsw/DT combination from working?
If not, is there a file dts for the bealgebone somewhere?
Thanks,
Richard
^ permalink raw reply
* Multiplatform Kconfig options
From: Tony Prisk @ 2012-10-11 5:03 UTC (permalink / raw)
To: linux-arm-kernel
What is the preference for multiplatform-enabled arch/Kconfig's?
Should we be specifying options that are already enabled by
multiplatform or can we assume they will always be enabled?
It 'feels' safer to explicitly enable them to catch problems later when
things are changed and people forget :)
Examples: MULTI_IRQ_HANDLER, SPARSE_IRQ, USE_OF, COMMON_CLK...
Regards
Tony P
^ permalink raw reply
* [PATCH v3] regulator: Versatile Express regulator driver
From: Mark Brown @ 2012-10-11 4:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1348509414-21261-2-git-send-email-pawel.moll@arm.com>
On Mon, Sep 24, 2012 at 06:56:54PM +0100, Pawel Moll wrote:
> Implementation of the regulator framework driver for the
> Versatile Express voltage control. Devices without
> voltage constraints (ie. "regulator-[min|max]-microvolt"
> properties in the DT node) are treated as fixed (or rather
> read-only) regulators.
Applied, thanks.
^ permalink raw reply
* [RFC] regulator: core: Support for continuous voltage range
From: Mark Brown @ 2012-10-11 4:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1348509414-21261-1-git-send-email-pawel.moll@arm.com>
On Mon, Sep 24, 2012 at 06:56:53PM +0100, Pawel Moll wrote:
> Some regulators can set any voltage within the constraints range,
> not being limited to specified operating points.
>
> This patch makes it possible to describe such regulator and makes
> the regulator_is_supported_voltage() function behave correctly.
Applied, thanks.
^ permalink raw reply
* [PATCH 05/16] vfs: bogus warnings in fs/namei.c
From: Al Viro @ 2012-10-11 4:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201210091307.19225.arnd@arndb.de>
On Tue, Oct 09, 2012 at 01:07:19PM +0000, Arnd Bergmann wrote:
> Update: I could actually reproduce the problem now, but it only happens when
> building with 'gcc -s' (i.e. CONFIG_CC_OPTIMIZE_FOR_SIZE). It does happen
> with both gcc-4.6 and with gcc-4.8, and on both x86-64 and ARM. An alternative
> patch that would also make it go away is the variant below, but I think that's
> even worse than the first version I suggested because it makes the binary
> output slightly worse by adding an unnecessary initialization when building with
> 'make -s'.
I can live with that, provided that you give it sane commit message and
your s-o-b.
^ permalink raw reply
* [PATCH 04/11] fsmc/nand: Accept nand timing parameters via DT
From: Vipin Kumar @ 2012-10-11 4:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121009115705.GN12801@game.jcrosoft.org>
On 10/9/2012 5:27 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 16:14 Tue 09 Oct , Vipin Kumar wrote:
>> Add support to accept nand timing parameters via device tree
>>
>> Signed-off-by: Vipin Kumar<vipin.kumar@st.com>
>> ---
>> .../devicetree/bindings/mtd/fsmc-nand.txt | 20 ++++++++
>> drivers/mtd/nand/fsmc_nand.c | 59 ++++++++++++++--------
>> include/linux/mtd/fsmc.h | 2 +-
>> 3 files changed, 59 insertions(+), 22 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/fsmc-nand.txt b/Documentation/devicetree/bindings/mtd/fsmc-nand.txt
>> index 29d1a2f..6a7fc43 100644
>> --- a/Documentation/devicetree/bindings/mtd/fsmc-nand.txt
>> +++ b/Documentation/devicetree/bindings/mtd/fsmc-nand.txt
>> @@ -13,6 +13,18 @@ Optional properties:
>> defaults to 1 byte
>> - nand-skip-bbtscan: Indicates the the BBT scanning should be skipped
>>
>> +- nand-timings-enabled: Indicates if the timing parameters are passed
>> + via DT
>> +- nand-timings,tclr:
>> +- nand-timings,tar:
>> +- nand-timings,thiz:
>> +- nand-timings,thold:
>> +- nand-timings,twait:
>> +- nand-timings,tset: All these timing parameters come from the actual
>> + nand device specification. Each of this represents a number of time
>> + period of hclk ie a number 4 in thold with hclk = 166MHz means that
>> + thold = (1000 / 166) * 4 ns = 24.09ns
>> +
>> Example:
>>
>> fsmc: flash at d1800000 {
>> @@ -26,6 +38,14 @@ Example:
>> st,cle-off =<0x10000>;
>> maxbanks =<1>;
>>
>> + nand-timings-enabled;
>> + nand-timings,tclr =<1>;
>> + nand-timings,tar =<1>;
>> + nand-timings,thiz =<1>;
>> + nand-timings,thold =<4>;
>> + nand-timings,twait =<6>;
>> + nand-timings,tset =<0>;
>> +
>> bank-width =<1>;
>> nand-skip-bbtscan;
>>
>> diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
>> index fc6a044..f3d69b3 100644
>> --- a/drivers/mtd/nand/fsmc_nand.c
>> +++ b/drivers/mtd/nand/fsmc_nand.c
>> @@ -415,27 +415,13 @@ static void fsmc_nand_setup(void __iomem *regs, uint32_t bank,
>> {
>> uint32_t value = FSMC_DEVTYPE_NAND | FSMC_ENABLE | FSMC_WAITON;
>> uint32_t tclr, tar, thiz, thold, twait, tset;
>> - struct fsmc_nand_timings *tims;
>> - struct fsmc_nand_timings default_timings = {
>> - .tclr = FSMC_TCLR_1,
>> - .tar = FSMC_TAR_1,
>> - .thiz = FSMC_THIZ_1,
>> - .thold = FSMC_THOLD_4,
>> - .twait = FSMC_TWAIT_6,
>> - .tset = FSMC_TSET_0,
>> - };
>> -
>> - if (timings)
>> - tims = timings;
>> - else
>> - tims =&default_timings;
>>
>> - tclr = (tims->tclr& FSMC_TCLR_MASK)<< FSMC_TCLR_SHIFT;
>> - tar = (tims->tar& FSMC_TAR_MASK)<< FSMC_TAR_SHIFT;
>> - thiz = (tims->thiz& FSMC_THIZ_MASK)<< FSMC_THIZ_SHIFT;
>> - thold = (tims->thold& FSMC_THOLD_MASK)<< FSMC_THOLD_SHIFT;
>> - twait = (tims->twait& FSMC_TWAIT_MASK)<< FSMC_TWAIT_SHIFT;
>> - tset = (tims->tset& FSMC_TSET_MASK)<< FSMC_TSET_SHIFT;
>> + tclr = (timings->tclr& FSMC_TCLR_MASK)<< FSMC_TCLR_SHIFT;
>> + tar = (timings->tar& FSMC_TAR_MASK)<< FSMC_TAR_SHIFT;
>> + thiz = (timings->thiz& FSMC_THIZ_MASK)<< FSMC_THIZ_SHIFT;
>> + thold = (timings->thold& FSMC_THOLD_MASK)<< FSMC_THOLD_SHIFT;
>> + twait = (timings->twait& FSMC_TWAIT_MASK)<< FSMC_TWAIT_SHIFT;
>> + tset = (timings->tset& FSMC_TSET_MASK)<< FSMC_TSET_SHIFT;
>>
>> if (busw)
>> writel(value | FSMC_DEVWID_16, FSMC_NAND_REG(regs, bank, PC));
>> @@ -876,6 +862,14 @@ static int __devinit fsmc_nand_probe_config_dt(struct platform_device *pdev,
>> struct device_node *np)
>> {
>> struct fsmc_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
>> + struct fsmc_nand_timings default_timings = {
>> + .tclr = FSMC_TCLR_1,
>> + .tar = FSMC_TAR_1,
>> + .thiz = FSMC_THIZ_1,
>> + .thold = FSMC_THOLD_4,
>> + .twait = FSMC_TWAIT_6,
>> + .tset = FSMC_TSET_0,
>> + };
>> u32 val;
>>
>> /* Set default NAND width to 8 bits */
>> @@ -894,6 +888,29 @@ static int __devinit fsmc_nand_probe_config_dt(struct platform_device *pdev,
>> pdata->options = NAND_SKIP_BBTSCAN;
>> of_property_read_u32(np, "maxbanks",&pdata->max_banks);
>>
>> + if (of_property_read_bool(np, "nand-timings-enabled")) {
>> + of_property_read_u32(np, "nand-timings,tclr",&val);
>> + pdata->nand_timings.tclr = (uint8_t)val;
> use a mask will be better
OK, will add a mask in v2
>> +
>> + of_property_read_u32(np, "nand-timings,tar",&val);
>> + pdata->nand_timings.tar = (uint8_t)val;
>> +
>> + of_property_read_u32(np, "nand-timings,thiz",&val);
>> + pdata->nand_timings.thiz = (uint8_t)val;
>> +
>> + of_property_read_u32(np, "nand-timings,thold",&val);
>> + pdata->nand_timings.thold = (uint8_t)val;
>> +
>> + of_property_read_u32(np, "nand-timings,twait",&val);
>> + pdata->nand_timings.twait = (uint8_t)val;
>> +
>> + of_property_read_u32(np, "nand-timings,tset",&val);
>> + pdata->nand_timings.tset = (uint8_t)val;
>> + } else {
>> + memcpy(&pdata->nand_timings,&default_timings,
>> + sizeof(default_timings));
>> + }
>> +
>> return 0;
>> }
>> #else
>> @@ -1031,7 +1048,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
>> host->partitions = pdata->partitions;
>> host->nr_partitions = pdata->nr_partitions;
>> host->dev =&pdev->dev;
>> - host->dev_timings = pdata->nand_timings;
>> + host->dev_timings =&pdata->nand_timings;
>> host->mode = pdata->mode;
>> host->max_banks = pdata->max_banks;
>>
>> diff --git a/include/linux/mtd/fsmc.h b/include/linux/mtd/fsmc.h
>> index f0ab734..4fbdce4 100644
>> --- a/include/linux/mtd/fsmc.h
>> +++ b/include/linux/mtd/fsmc.h
>> @@ -148,7 +148,7 @@ enum access_mode {
>> * this may be set to NULL
>> */
>> struct fsmc_nand_platform_data {
>> - struct fsmc_nand_timings *nand_timings;
>> + struct fsmc_nand_timings nand_timings;
>> struct mtd_partition *partitions;
>> unsigned int nr_partitions;
>> unsigned int options;
>> --
>> 1.7.11.4
>>
> .
>
^ permalink raw reply
* [PATCH 02/11] fsmc/nand: Rearrange the fsmc_nand_data structure and update comments
From: Vipin Kumar @ 2012-10-11 4:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121009115245.GL12801@game.jcrosoft.org>
On 10/9/2012 5:22 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 16:14 Tue 09 Oct , Vipin Kumar wrote:
>> Signed-off-by: Vipin Kumar<vipin.kumar@st.com>
>> ---
>> drivers/mtd/nand/fsmc_nand.c | 51 ++++++++++++++++++++++++++------------------
>> 1 file changed, 30 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
>> index e96d7d1..bd89580 100644
>> --- a/drivers/mtd/nand/fsmc_nand.c
>> +++ b/drivers/mtd/nand/fsmc_nand.c
>> @@ -278,52 +278,61 @@ static struct fsmc_eccplace fsmc_ecc4_sp_place = {
>> * struct fsmc_nand_data - structure for FSMC NAND device state
>> *
>> * @pid: Part ID on the AMBA PrimeCell format
>> - * @mtd: MTD info for a NAND flash.
>> - * @nand: Chip related info for a NAND flash.
>> - * @partitions: Partition info for a NAND Flash.
>> - * @nr_partitions: Total number of partition of a NAND flash.
>> - *
>> - * @ecc_place: ECC placing locations in oobfree type format.
>> - * @bank: Bank number for probed device.
>> - * @clk: Clock structure for FSMC.
>> + * @mtd: MTD info for a NAND flash
>> + * @nand: Chip related info for a NAND flash
>> + * @dev: Device structure pointer
>> + * @clk: Clock structure for FSMC
>> + * @ecc_place: ECC placing locations in oobfree type format
>> + * @bank: Bank number for probed device
>> *
>> * @read_dma_chan: DMA channel for read access
>> * @write_dma_chan: DMA channel for write access to NAND
>> * @dma_access_complete: Completion structure
>> *
>> - * @data_pa: NAND Physical port for Data.
>> - * @data_va: NAND port for Data.
>> - * @cmd_va: NAND port for Command.
>> - * @addr_va: NAND port for Address.
>> - * @regs_va: FSMC regs base address.
>> + * @dev_timings: Timings to be programmed in controller
>> + * @partitions: Partition info for a NAND Flash
>> + * @nr_partitions: Total number of partition of a NAND flash
>> + * @mode: Defines the NAND device access mode
>> + * Can be one of:
>> + * - DMA access
>> + * - Word access (CPU)
>> + * - None (Use driver default ie bus width specific
>> + * CPU access)
>> + * @select_chip: Select a particular bank
>> + *
>> + * @data_pa: NAND Physical port for Data
>> + * @data_va: NAND port for Data
>> + * @cmd_va: NAND port for Command
>> + * @addr_va: NAND port for Address
>> + * @regs_va: FSMC regs base address
>> */
>> struct fsmc_nand_data {
>> u32 pid;
>> struct mtd_info mtd;
>> struct nand_chip nand;
>> - struct mtd_partition *partitions;
>> - unsigned int nr_partitions;
>> -
>> - struct fsmc_eccplace *ecc_place;
>> - unsigned int bank;
>> struct device *dev;
>> - enum access_mode mode;
>> struct clk *clk;
>> + struct fsmc_eccplace *ecc_place;
>> + unsigned int bank;
>>
>> /* DMA related objects */
>> struct dma_chan *read_dma_chan;
>> struct dma_chan *write_dma_chan;
>> struct completion dma_access_complete;
>>
>> + /* Recieved from plat data */
>> struct fsmc_nand_timings *dev_timings;
>> + struct mtd_partition *partitions;
>> + unsigned int nr_partitions;
> where do you use those execpt at probe time
Yes, I can remove it completely. Thanks for that
^ permalink raw reply
* [PATCH 09/11] fsmc/nand:FIX: replace change_bit routine
From: Vipin Kumar @ 2012-10-11 4:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121010204546.GK4625@n2100.arm.linux.org.uk>
On 10/11/2012 2:15 AM, Russell King - ARM Linux wrote:
> On Wed, Oct 10, 2012 at 07:22:04PM +0200, Linus Walleij wrote:
>> On Tue, Oct 9, 2012 at 12:44 PM, Vipin Kumar<vipin.kumar@st.com> wrote:
>>
>>> change_bit routine accepts only ulong pointers as buffer, so an unaligned char
>>> pointer passed to change_bit may lead to a crash.
>>>
>>> Fix this bug by accessing the buffer as char pointer.
>>
>> Why not see if we can fix change_bit() instead?
>> Since I suspect this is on ARM I bet Russell and Nico
>> want to hear about this if there is a problem.
>
> Not particularly. There's a reason the argument to change_bit() is typed
> 'unsigned long' and that's not because it can take a void, char, or a
> short. It's because it _expects_ the buffer to be aligned to an
> "unsigned long" quantity.
>
> That's because on many architectures, misaligned loads and stores are
> not atomic operations - and in this case, load/store exclusive will
> fail when they're misalighed.
>
> So...
>
> - change_bit(err_idx[i], (unsigned long *)dat);
>
> is highly invalid code.
>
Thanks. Got your point
>> Can the ARM change_bit() not be fixed, so that
>> long arguments are the only option?
>
> Spot the intentional cast:
>
>>> - change_bit(err_idx[i], (unsigned long *)dat);
>
> which tries to work around this. Remember my attitude towards casts:
> if you're having to use a cast, you are _probably_ doing something
> wrong. In this case, it's hiding a warning which was saying that
> the code is doing something wrong, and then the result blows up.
> By adding that cast, the wrong wire was cut... you get to keep the
> remains. ;)
> .
I agree with you. Although I was a bit relaxed this time, I would really
think before adding a cast explicitly just to avoid a warning
-Vipin
^ permalink raw reply
* [PATCH 09/11] fsmc/nand:FIX: replace change_bit routine
From: Vipin Kumar @ 2012-10-11 4:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.02.1210101559250.16518@xanadu.home>
On 10/11/2012 1:51 AM, Nicolas Pitre wrote:
> On Wed, 10 Oct 2012, Linus Walleij wrote:
>
>> On Tue, Oct 9, 2012 at 12:44 PM, Vipin Kumar<vipin.kumar@st.com> wrote:
>>
>>> change_bit routine accepts only ulong pointers as buffer, so an unaligned char
>>> pointer passed to change_bit may lead to a crash.
>>>
>>> Fix this bug by accessing the buffer as char pointer.
>>
>> Why not see if we can fix change_bit() instead?
>> Since I suspect this is on ARM I bet Russell and Nico
>> want to hear about this if there is a problem.
>>
>> Can the ARM change_bit() not be fixed, so that
>> long arguments are the only option?
>
Hello Nicolas
> No. It is this code which is totally broken.
>
Yes, I understand and accept the probelm. That's why the fix
> The change_bit() is defined to operate on long values, in the _native_
> endian. Now imagine what this is going to do to your data buffer if
> instead you are running on a big endian device.
>
> And I doubt there is anything requiring atomic bit manipulation here
> either.
>
No, there is no requirement for an atomic change_bit
>>> if (err_idx[i]< chip->ecc.size * 8) {
>>> - change_bit(err_idx[i], (unsigned long *)dat);
>>> + uint8_t *p = dat + err_idx[i] / 8;
>>> + *p = *p ^ (1<< (err_idx[i] % 8));
>>
>> I'm one of these people who would write>>3 and
>> &7 rather than /8 or %8 but I guess we are all
>> different. Atleast consider it if you stick with this...
>
> Better yet:
>
> dat[err_idx[i] / 8] ^= (1<< (err_idx[i] % 8));
>
> The /8 and %8 will be changed into>>3 and&7 by the compiler anyway,
> while the /8 and %8 form is possibly a bit less obscure.
>
Yes, that's exactly why I kept /8 and %8. I would change the code as
suggested by you
> Of course, this needs to be done over _all_ this driver, not only a few
> cases.
>
There is only one place which needs a change_bit. I would send a v2 with
the suggested change
Vipin
>
> Nicolas
> .
>
^ permalink raw reply
* [PATCH 07/11] fsmc/nand: Provide contiguous buffers to dma
From: viresh kumar @ 2012-10-11 4:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdb6ArMUQ6MAsue+Gh15UfOUrWNAc9wrxd6awc0BTwOGiw@mail.gmail.com>
On Wed, Oct 10, 2012 at 10:37 PM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Tue, Oct 9, 2012 at 12:44 PM, Vipin Kumar <vipin.kumar@st.com> wrote:
> The real problem is likely the DMA driver. The stuf that get
> fed into dma.device_prep_dma_memcpy() needs to be
> converted to a scatterlist and then set up in the LLI list
> for the controller.
>
> IIRC SPEAr is using drivers/dma/dw_dmac.c so
> check this driver's dwc_prep_dma_memcpy().
> It does seem like it is checking whether src or
> dest is scattered in this for() loop:
>
> for (offset = 0; offset < len; offset += xfer_count << src_width) {}
>
> 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.
dwc_prep_dma_memcpy() doesn't expect a virtual address, look at type
of src & dest bufs: dma_addr_t. It is responsibility of user drivers to pass
physically contiguous address to it.
--
viresh
^ permalink raw reply
* [PATCH 07/11] fsmc/nand: Provide contiguous buffers to dma
From: Vipin Kumar @ 2012-10-11 4:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdb6ArMUQ6MAsue+Gh15UfOUrWNAc9wrxd6awc0BTwOGiw@mail.gmail.com>
On 10/10/2012 10:37 PM, Linus Walleij wrote:
> On Tue, Oct 9, 2012 at 12:44 PM, Vipin Kumar<vipin.kumar@st.com> wrote:
>
>> read_buf/write_buf callbacks should be able to accept a user space memory
>> address (virtually contiguous memory) as buffer pointer.
>>
>> This patch allocates a logically contiguous memory area which is use for dma
>
> You mean PHYSICALLY contigous, don't you?
>
Yes Sorry for that :)
>> xfers during read and write accesses.
>>
>> Signed-off-by: Vipin Kumar<vipin.kumar@st.com>
>
> If you really want a physically contigous buffer you need to use
> CMA, but I don't think that is the real problem here...
>
> We're already using userspace buffers in e.g. the MMCI driver
> (drivers/mmc/host/mmci.c).
>
> The real problem is likely the DMA driver. The stuf that get
> fed into dma.device_prep_dma_memcpy() needs to be
> converted to a scatterlist and then set up in the LLI list
> for the controller.
>
> IIRC SPEAr is using drivers/dma/dw_dmac.c so
> check this driver's dwc_prep_dma_memcpy().
> It does seem like it is checking whether src or
> dest is scattered in this for() loop:
>
> for (offset = 0; offset< len; offset += xfer_count<< src_width) {}
>
> dma_sync_single_for_device() is translating the virtual
> address to physical for every chunk BTW.
>
> So instead of doing this copying, debug the problem, and
> see if there is a bug in that for()-loop or similar, if it needs
> to be rewritten or so.
>
I would debug again and reply to this mail soon
> Yours,
> Linus Walleij
> .
>
^ permalink raw reply
* [PATCH 07/11] fsmc/nand: Provide contiguous buffers to dma
From: Vipin Kumar @ 2012-10-11 4:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOh2x==Up=mOc+YA6-yKD7xpoxFyzfb-XtoQ6DENTXayTa=vtA@mail.gmail.com>
On 10/11/2012 8:46 AM, viresh kumar wrote:
> On Wed, Oct 10, 2012 at 10:37 PM, Linus Walleij
> <linus.walleij@linaro.org> wrote:
>> The real problem is likely the DMA driver. The stuf that get
>> fed into dma.device_prep_dma_memcpy() needs to be
>> converted to a scatterlist and then set up in the LLI list
>> for the controller.
>>
>> IIRC SPEAr is using drivers/dma/dw_dmac.c so
>
> Both dw_dmac and pl080 for different SPEAr SoC's.
>
>> check this driver's dwc_prep_dma_memcpy().
>> It does seem like it is checking whether src or
>> dest is scattered in this for() loop:
>>
>> for (offset = 0; offset< len; offset += xfer_count<< src_width) {}
>>
>> dma_sync_single_for_device() is translating the virtual
>> address to physical for every chunk BTW.
>>
>> So instead of doing this copying, debug the problem, and
>> see if there is a bug in that for()-loop or similar, if it needs
>> to be rewritten or so.
>
> But what's the problem you saw vipin, for which you generated this patch?
> .
>
The nand tests were failing and that was because of a user pointer being
passed to them but it was a long time back and this patch just carried
on in the local repo. It is only now that I am sending it
It seems may be the dma drivers also got updated so it needs another
test cycle it. I will do the needful and re-reply back on this mail
Vipin
^ permalink raw reply
* [PATCH 07/11] fsmc/nand: Provide contiguous buffers to dma
From: viresh kumar @ 2012-10-11 3:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdb6ArMUQ6MAsue+Gh15UfOUrWNAc9wrxd6awc0BTwOGiw@mail.gmail.com>
On Wed, Oct 10, 2012 at 10:37 PM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> The real problem is likely the DMA driver. The stuf that get
> fed into dma.device_prep_dma_memcpy() needs to be
> converted to a scatterlist and then set up in the LLI list
> for the controller.
>
> IIRC SPEAr is using drivers/dma/dw_dmac.c so
Both dw_dmac and pl080 for different SPEAr SoC's.
> check this driver's dwc_prep_dma_memcpy().
> It does seem like it is checking whether src or
> dest is scattered in this for() loop:
>
> for (offset = 0; offset < len; offset += xfer_count << src_width) {}
>
> dma_sync_single_for_device() is translating the virtual
> address to physical for every chunk BTW.
>
> So instead of doing this copying, debug the problem, and
> see if there is a bug in that for()-loop or similar, if it needs
> to be rewritten or so.
But what's the problem you saw vipin, for which you generated this patch?
^ permalink raw reply
* alignment faults in 3.6
From: Jon Masters @ 2012-10-11 2:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <yw1xtxu1pwtv.fsf@unicorn.mansr.com>
On 10/10/2012 10:27 PM, M?ns Rullg?rd wrote:
> There are exactly two possible solutions:
>
> 1. Change the networking code so those structs are always aligned. This
> might not be (easily) possible.
> 2. Mark the structs __packed and fix any typecasts like the ones seen in
> this thread. This will have an adverse effect in cases where the
> structs are in fact aligned.
>
> Both solutions lie squarely in the networking code. It's time to
> involve that list, or we'll never get anywhere.
Sure, please do let's figure out the plan. But my question is tangential
- I am after input from rmk on whether that patch he posted to fix the
atomicity of missaligned faults is going to be something we should plan
to be pulling into 3.6 for Fedora (after there's an official version) to
correct the might_fault warnings, etc.
Meanwhile, a separate fix of some kind is still likely to be needed
because we don't want to take a large number of alignment traps.
Jon.
^ permalink raw reply
* alignment faults in 3.6
From: Måns Rullgård @ 2012-10-11 2:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <507619FA.6080001@jonmasters.org>
Jon Masters <jonathan@jonmasters.org> writes:
> Hi everyone,
>
> On 10/05/2012 10:33 AM, Rob Herring wrote:
>> On 10/05/2012 09:05 AM, Russell King - ARM Linux wrote:
>>> On Fri, Oct 05, 2012 at 07:24:44AM -0500, Rob Herring wrote:
>>>> On 10/05/2012 03:24 AM, Russell King - ARM Linux wrote:
>>>>> Does it matter? I'm just relaying the argument against adding __packed
>>>>> which was used before we were forced (by the networking folk) to implement
>>>>> the alignment fault handler.
>>>>
>>>> It doesn't really matter what will be accepted or not as adding __packed
>>>> to struct iphdr doesn't fix the problem anyway. gcc still emits a ldm.
>>>> The only way I've found to eliminate the alignment fault is adding a
>>>> barrier between the 2 loads. That seems like a compiler issue to me if
>>>> there is not a better fix.
This turns out to be caused by pointers being typecast to normal
(aligned) types.
>>> Even so, please test the patch I've sent you in the sub-thread - that
>>> needs testing whether or not GCC is at fault. Will's patch to add the
>>> warnings _has_ uncovered a potential issue with the use of __get_user()
>>> in some parts of the ARM specific kernel, and I really need you to test
>>> that while you're experiencing this problem.
>>
>> I've tested your patch and it appears to fix things. Thanks!
[...]
>> Now on to getting rid of faults on practically every single received IP
>> packet:
>>
>> Multi: 9871002
>>
>> RX packets:9872010 errors:0 dropped:0 overruns:0 frame:0
>
> This will still be a problem, indeed. At least we can be aware we're
> taking a large number of faults and hope for a netdev solution.
There are exactly two possible solutions:
1. Change the networking code so those structs are always aligned. This
might not be (easily) possible.
2. Mark the structs __packed and fix any typecasts like the ones seen in
this thread. This will have an adverse effect in cases where the
structs are in fact aligned.
Both solutions lie squarely in the networking code. It's time to
involve that list, or we'll never get anywhere.
--
M?ns Rullg?rd
mans at mansr.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox