LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: linux-next: build failure after merge of the final tree (powerpc related)
From: Grant Likely @ 2012-01-02  8:39 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Stephen Rothwell, Benoit Cousson, linux-kernel, linux-next,
	Paul Mackerras, Thomas Gleixner, linuxppc-dev, Samuel Ortiz
In-Reply-To: <20120102082514.GG18381@ponder.secretlab.ca>

On Mon, Jan 2, 2012 at 1:25 AM, Grant Likely <grant.likely@secretlab.ca> wr=
ote:
> On Wed, Dec 28, 2011 at 09:32:14PM +1100, Benjamin Herrenschmidt wrote:
>> On Wed, 2011-12-28 at 19:49 +1100, Stephen Rothwell wrote:
>> > Hi ,
>> >
>> > After merging the final tree, today's linux-next build (powerpc
>> > allyesconfig) failed like this:
>> >
>> > kernel/built-in.o: In function `irq_dispose_mapping':
>> > (.opd+0x159f0): multiple definition of `irq_dispose_mapping'
>> > arch/powerpc/kernel/built-in.o:(.opd+0x960): first defined here
>> > kernel/built-in.o: In function `irq_create_of_mapping':
>> > (.opd+0x15a20): multiple definition of `irq_create_of_mapping'
>> > arch/powerpc/kernel/built-in.o:(.opd+0x9a8): first defined here
>> > kernel/built-in.o: In function `.irq_create_of_mapping':
>> > (.text+0x147030): multiple definition of `.irq_create_of_mapping'
>> > arch/powerpc/kernel/built-in.o:(.text+0x9de0): first defined here
>> > kernel/built-in.o: In function `.irq_dispose_mapping':
>> > (.text+0x146f4c): multiple definition of `.irq_dispose_mapping'
>> > arch/powerpc/kernel/built-in.o:(.text+0x9684): first defined here
>> >
>> > I am not sure what caused this. And have just left it broken.
>>
>> Grant, is your irq remapper misbehaving ?
>
> Hmmmm, that's odd. =A0I've not touched it. =A0I'll investigate.

It looks like CONFIG_IRQ_DOMAIN is getting selected by TWL4030_CORE.
Drivers must not select that config symbol.  It looks like commit
da28adbd (mfd: twl-core: Add initial DT support for twl4030/twl6030)
is the culprit.

The following patch should solve the problem:

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 13c468e..e43a570 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -200,8 +200,7 @@ config MENELAUS

 config TWL4030_CORE
 	bool "Texas Instruments TWL4030/TWL5030/TWL6030/TPS659x0 Support"
-	depends on I2C=3Dy && GENERIC_HARDIRQS
-	select IRQ_DOMAIN
+	depends on I2C=3Dy && GENERIC_HARDIRQS && IRQ_DOMAIN
 	help
 	  Say yes here if you have TWL4030 / TWL6030 family chip on your board.
 	  This core driver provides register access and IRQ handling

^ permalink raw reply related

* Mac address change in linux
From: smitha.vanga @ 2012-01-02 11:24 UTC (permalink / raw)
  To: scottwood; +Cc: linuxppc-dev

Hi Scott,

When I execute below statements in linux to change the mac address I get the=
 error Host name look up error.
Below are the commands I execute to change the mac address and the console o=
utput.


1. ifconfig eth0.246 down
2. ifconfig eth0.246 hw ether 00:11:22:33:44:55
   ifconfig: hw: Host name lookup failure

sh-3.1# ifconfig
eth0      Link encap:Ethernet  HWaddr 00:E0:EE:00:05:2E
          inet addr:172.16.52.20  Bcast:172.16.53.255  Mask:255.255.254.0
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:999 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000
          RX bytes:93661 (91.4 KiB)  TX bytes:0 (0.0 B)

eth0.246  Link encap:Ethernet  HWaddr 00:E0:EE:00:05:2E
          inet addr:172.16.52.20  Bcast:172.16.53.255  Mask:255.255.254.0
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0
          RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)

lo        Link encap:Local Loopback
          inet addr:127.0.0.1  Mask:255.0.0.0
          UP LOOPBACK RUNNING  MTU:16436  Metric:1
          RX packets:4 errors:0 dropped:0 overruns:0 frame:0
          TX packets:4 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0
          RX bytes:248 (248.0 B)  TX bytes:248 (248.0 B)

sh-3.1# ifconfig eth0.246 down
sh-3.1# ifconfig
eth0      Link encap:Ethernet  HWaddr 00:E0:EE:00:05:2E
          inet addr:172.16.52.20  Bcast:172.16.53.255  Mask:255.255.254.0
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:1005 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000
          RX bytes:94205 (91.9 KiB)  TX bytes:0 (0.0 B)

lo        Link encap:Local Loopback
          inet addr:127.0.0.1  Mask:255.0.0.0
          UP LOOPBACK RUNNING  MTU:16436  Metric:1
          RX packets:4 errors:0 dropped:0 overruns:0 frame:0
          TX packets:4 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0
          RX bytes:248 (248.0 B)  TX bytes:248 (248.0 B)

sh-3.1# ifconfig eth0.246 hw ether 00:11:22:33:44:55 ifconfig eth0.246 up
ifconfig: hw: Host name lookup failure
sh-3.1# 


Regards,
Smitha
Please do not print this email unless it is absolutely necessary. =0A=
=0A=
The information contained in this electronic message and any attachments to=
 this message are intended for the exclusive use of the addressee(s) and may=
 contain proprietary, confidential or privileged information. If you are not=
 the intended recipient, you should not disseminate, distribute or copy this=
 e-mail. Please notify the sender immediately and destroy all copies of this=
 message and any attachments. =0A=
=0A=
WARNING: Computer viruses can be transmitted via email. The recipient should=
 check this email and any attachments for the presence of viruses. The compa=
ny accepts no liability for any damage caused by any virus transmitted by th=
is email. =0A=
=0A=
www.wipro.com

^ permalink raw reply

* Re: p1020 unstable with 3.2
From: Alexander Graf @ 2012-01-02 15:20 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev, Fleming Andy-AFLEMING
In-Reply-To: <1325048474.6632.33.camel@pasglop>


On 28.12.2011, at 06:01, Benjamin Herrenschmidt wrote:

> On Sun, 2011-12-25 at 11:48 +0100, Alexander Graf wrote:
>> On 24.12.2011, at 07:53, Benjamin Herrenschmidt wrote:
>> 
>>> On Fri, 2011-12-23 at 17:54 +0100, Alexander Graf wrote:
>>>> Hi guys,
>>>> 
>>>> While trying to test my latest patch queue for ppc kvm, I realized
>>>> that even though the device trees got updated, the p1020 box still is
>>>> unstable. The trace below is the one I've seen the most. It only
>>>> occurs during network I/O which happens a lot on that box, since I'm
>>>> running it using NFS root.
>>>> 
>>>> As for configuration, I use kumar's "merge" branch from today and the
>>>> p1020rdb.dts device tree provided in that tree.
>>>> 
>>>> The last known good configuration I'm aware of is 3.0.
>>>> 
>>>> Any ideas what's going wrong here?
>>> 
>>> Try SLAB instead of SLUB and let me know. It -could- be a bogon in SLUB
>>> that should be fixed upstream now but I think did hit 3.2
>> 
>> Yup, things seem a lot more stable with SLAB now :).
> 
> BTW. Fix for slub should be upstream:
> 
> 42d623a8cd08eb93ab221d22cee5a62618895bbf

Yup, works like a charm now again. Thanks a lot!


Alex

^ permalink raw reply

* Re: [PATCH] KVM: Move gfn_to_memslot() to kvm_host.h
From: Alexander Graf @ 2012-01-02 15:23 UTC (permalink / raw)
  To: Avi Kivity; +Cc: linuxppc-dev, Paul Mackerras, kvm, kvm-ppc
In-Reply-To: <4EF87528.8000305@redhat.com>


On 26.12.2011, at 14:22, Avi Kivity wrote:

> On 12/20/2011 11:21 AM, Paul Mackerras wrote:
>> This moves gfn_to_memslot(), and the functions it calls, that is,
>> search_memslots() and __gfn_to_memslot(), from kvm_main.c to kvm_host.h
>> so that gfn_to_memslot() can be called from non-modular code even
>> when KVM is a module.  On powerpc, the Book3S HV style of KVM has
>> code that is called from real mode which needs to call gfn_to_memslot()
>> and thus needs this.  (Module code is allocated in the vmalloc region,
>> which can't be accessed in real mode.)
>> 
>> With this, we can remove builtin_gfn_to_memslot() from book3s_hv_rm_mmu.c
>> and thus eliminate a little bit of duplication.
> 
> Those functions are too big to be inlined IMO.  How about moving them to
> another C file, and making it builtin for ppc?
> 
> The only issue is what to call it. virt/kvm/builtin-for-ppc seems silly.

Yeah - and it makes it pretty confusing to find the functions then.

> Or we could move the implementation into a header file, with an extra __
> prefix, and have the C stubs call those inlines, so we have exactly on
> instantiation.  Your real mode code can then call the inlines.

I like this version. That way everyone should be happy :)


Alex

^ permalink raw reply

* Re: Mac address change in linux
From: Lorenz Kolb @ 2012-01-02 15:42 UTC (permalink / raw)
  To: smitha.vanga; +Cc: scottwood, linuxppc-dev
In-Reply-To: <40631E9A2581F14BA60888C87A76A1FE01C96A@HYD-MKD-MBX4.wipro.com>

Hi Smitha,

first of: please try changing the MAC of the adapter, not of the virtual 
interface (btw: are you sure, that you really intend to have  a physical 
and a virtual interface that have the same configurations and the same 
metric?).
second: guess your copy+pasted log shows: that you concatenated 
"ifconfig eth0.246 up" to the actual MAC address change command by 
accident. Please try again, without these additional (surely non 
working) parameters.

Regards,

lorenz

On 1/2/2012 12:24 PM, smitha.vanga@wipro.com wrote:
> Hi Scott,
>
> When I execute below statements in linux to change the mac address I get the error Host name look up error.
> Below are the commands I execute to change the mac address and the console output.
>
>
> 1. ifconfig eth0.246 down
> 2. ifconfig eth0.246 hw ether 00:11:22:33:44:55
>     ifconfig: hw: Host name lookup failure
>
> sh-3.1# ifconfig
> eth0      Link encap:Ethernet  HWaddr 00:E0:EE:00:05:2E
>            inet addr:172.16.52.20  Bcast:172.16.53.255  Mask:255.255.254.0
>            UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
>            RX packets:999 errors:0 dropped:0 overruns:0 frame:0
>            TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
>            collisions:0 txqueuelen:1000
>            RX bytes:93661 (91.4 KiB)  TX bytes:0 (0.0 B)
>
> eth0.246  Link encap:Ethernet  HWaddr 00:E0:EE:00:05:2E
>            inet addr:172.16.52.20  Bcast:172.16.53.255  Mask:255.255.254.0
>            UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
>            RX packets:0 errors:0 dropped:0 overruns:0 frame:0
>            TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
>            collisions:0 txqueuelen:0
>            RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)
>
> lo        Link encap:Local Loopback
>            inet addr:127.0.0.1  Mask:255.0.0.0
>            UP LOOPBACK RUNNING  MTU:16436  Metric:1
>            RX packets:4 errors:0 dropped:0 overruns:0 frame:0
>            TX packets:4 errors:0 dropped:0 overruns:0 carrier:0
>            collisions:0 txqueuelen:0
>            RX bytes:248 (248.0 B)  TX bytes:248 (248.0 B)
>
> sh-3.1# ifconfig eth0.246 down
> sh-3.1# ifconfig
> eth0      Link encap:Ethernet  HWaddr 00:E0:EE:00:05:2E
>            inet addr:172.16.52.20  Bcast:172.16.53.255  Mask:255.255.254.0
>            UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
>            RX packets:1005 errors:0 dropped:0 overruns:0 frame:0
>            TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
>            collisions:0 txqueuelen:1000
>            RX bytes:94205 (91.9 KiB)  TX bytes:0 (0.0 B)
>
> lo        Link encap:Local Loopback
>            inet addr:127.0.0.1  Mask:255.0.0.0
>            UP LOOPBACK RUNNING  MTU:16436  Metric:1
>            RX packets:4 errors:0 dropped:0 overruns:0 frame:0
>            TX packets:4 errors:0 dropped:0 overruns:0 carrier:0
>            collisions:0 txqueuelen:0
>            RX bytes:248 (248.0 B)  TX bytes:248 (248.0 B)
>
> sh-3.1# ifconfig eth0.246 hw ether 00:11:22:33:44:55 ifconfig eth0.246 up
> ifconfig: hw: Host name lookup failure
> sh-3.1#
>
>
> Regards,
> Smitha
> Please do not print this email unless it is absolutely necessary.
>
> The information contained in this electronic message and any attachments to this message are intended for the exclusive use of the addressee(s) and may contain proprietary, confidential or privileged information. If you are not the intended recipient, you should not disseminate, distribute or copy this e-mail. Please notify the sender immediately and destroy all copies of this message and any attachments.
>
> WARNING: Computer viruses can be transmitted via email. The recipient should check this email and any attachments for the presence of viruses. The company accepts no liability for any damage caused by any virus transmitted by this email.
>
> www.wipro.com
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH] KVM: Move gfn_to_memslot() to kvm_host.h
From: Avi Kivity @ 2012-01-02 16:13 UTC (permalink / raw)
  To: Alexander Graf; +Cc: linuxppc-dev, Paul Mackerras, kvm, kvm-ppc
In-Reply-To: <6918CBBE-19AF-4323-BFE7-0F8739E32008@suse.de>

On 01/02/2012 05:23 PM, Alexander Graf wrote:
> > Or we could move the implementation into a header file, with an extra __
> > prefix, and have the C stubs call those inlines, so we have exactly on
> > instantiation.  Your real mode code can then call the inlines.
>
> I like this version. That way everyone should be happy :)

Pretty much how everything is solved.  Pile up another layer of
indirection (compile-time here), everyone's happy, and the code bloats.

(I'm not against this, just grumpy)

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply

* Re: [PATCH 1/2][v2] Integrated Flash Controller support
From: Scott Wood @ 2012-01-02 17:31 UTC (permalink / raw)
  To: dpervushin; +Cc: Liu Shuo, linuxppc-dev, Prabhakar Kushwaha
In-Reply-To: <1325079052.12535.22.camel@mrki-medvjed.diimka.lan>

On 12/28/2011 07:30 AM, dmitry pervushin wrote:
> If you're using memory-mapped structure, shouldn't it be announced with
> __attribute__(packed) ?

Why?  We know this isn't going to be compiled on some strange ABI where
a struct with nothing but u32 gets padding.  __attribute__((packed))
also tells GCC the struct itself could be unaligned which is not true.

-Scott

^ permalink raw reply

* Re: Mac address in the DTS file
From: Scott Wood @ 2012-01-02 17:53 UTC (permalink / raw)
  To: smitha.vanga; +Cc: linuxppc-dev
In-Reply-To: <40631E9A2581F14BA60888C87A76A1FE01C6BB@HYD-MKD-MBX4.wipro.com>

On 12/30/2011 06:51 AM, smitha.vanga@wipro.com wrote:
>  
> Hi Scott,
> 
> In my DTS file for mpc8247 I have defined the ether MAC in the node as below.
> 
> ethernet@24000 {
>                        #address-cells = <1>;
>                        #size-cells = <0>;
>                        device_type = "network";
>                        device-id = <1>;
>                        compatible = "fs_enet";
>                        model = "FCC";
>                        reg = <11300 20 8400 100 11380 30>;
>                        mac-address = [ 00 e0 ee 00 05 2e ];
>                        interrupts = <20 2>;
>                        interrupt-parent = <10c00>;
>                        phy-handle = <2452000>;
>                        rx-clock = <13>;
>                        tx-clock = <12>;
>                };

Others have answered your question (the mac address should not be in the
.dts file -- it should be patched in by boot software), but I'd like to
point out that this is a deprecated device tree binding that was very
short-lived in a very old kernel, from an early attempt at 82xx support
in arch/powerpc.  Why are you still using it?  It will not work with
current kernels (which suggests you're also using a very old kernel).

-Scott

^ permalink raw reply

* Re: [PATCH] DTS: fix the bug and add the chip compatible for eSDHC
From: Scott Wood @ 2012-01-02 18:27 UTC (permalink / raw)
  To: r66093; +Cc: Jerry Huang, linuxppc-dev
In-Reply-To: <1324620659-7135-1-git-send-email-r66093@freescale.com>

On 12/23/2011 12:10 AM, r66093@freescale.com wrote:
> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> 
> Accordint to latest kernel, the auto-cmd12 property should be
> "sdhci,auto-cmd12", and according to the SDHC binding and the workaround for
> the special chip, add the chip compatible for eSDHC: "fsl,p1022-esdhc",
> "fsl,mpc8536-esdhc", "fsl,p2020-esdhc" and "fsl,p1010-esdhc".
> 
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> ---
>  arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi |    4 ++++
>  arch/powerpc/boot/dts/fsl/p1010si-post.dtsi   |    3 ++-
>  arch/powerpc/boot/dts/fsl/p1022si-post.dtsi   |    3 ++-
>  arch/powerpc/boot/dts/fsl/p2020si-post.dtsi   |    4 ++++
>  4 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi b/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
> index 89af626..44e0ed9 100644
> --- a/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
> @@ -236,6 +236,10 @@
>  	};
>  
>  /include/ "pq3-esdhc-0.dtsi"
> +	sdhc@2e000 {
> +		compatible = "fsl,esdhc", "fsl,mpc8536-esdhc";
> +	};

More-specific compatible entries should come first.

-Scott

^ permalink raw reply

* Problem in getting shared memory access on P1022RDK
From: Arshad, Farrukh @ 2012-01-02 19:10 UTC (permalink / raw)
  To: linuxppc-dev@lists.ozlabs.org

[-- Attachment #1: Type: text/plain, Size: 1381 bytes --]

Greetings All,

Please excuse me for a 'not very precise' question but I just need some clue or point to look for the trouble.
I am running linux in AMP configuration on P1022RDK dual core. My memory partitioning is given below

Core Base Address (CONFIG_PHYSICAL_START / bootm_low) Size (bootm_size / mem kernel boot parameter)
Core 0 0x0000,0000 0x1000,0000
Core 1 0x1000,0000 0x0C00,0000
Shared Memory 0x1C00,0000 0x0400,0000

I have set bootm_low / bootm_size and provided mem parameter in both kernel configurations, both
kernels are booting up fine. I have a shared memory driver running in both cores. My shared memory
driver just exposes the physical memory (0x1C00,0000) to user-space.

The problem I am facing is I can not get access to same shared memory in user-space on both kernels.
I have verified my shared memory driver on another platform (p1022ds) and it works perfect. I have also
tried using non-cached memory mapping in shared memory driver but that does not work as well.

To me it seems one of my kernels is going beyond its defined memory partition (which I have set above), I have
set kernel's partitioning as CONFIG_PHYSICAL_START, bootm_size, bootm_low and mem kernel boot param.

Besides the shared memory driver, can anyone suggest or give me clue where should I look for the
trouble in the kernel.

Regards,
Farrukh Arshad.

[-- Attachment #2: Type: text/html, Size: 2532 bytes --]

^ permalink raw reply

* Re: [PATCH] DTS: fix the bug and add the chip compatible for eSDHC
From: Tabi Timur-B04825 @ 2012-01-02 20:30 UTC (permalink / raw)
  To: Huang Changming-R66093
  Cc: Huang Changming-R66093, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1324620659-7135-1-git-send-email-r66093@freescale.com>

On Fri, Dec 23, 2011 at 12:10 AM,  <r66093@freescale.com> wrote:
>
> Accordint to latest kernel, the auto-cmd12 property should be
> "sdhci,auto-cmd12", and according to the SDHC binding and the workaround =
for
> the special chip, add the chip compatible for eSDHC: "fsl,p1022-esdhc",
> "fsl,mpc8536-esdhc", "fsl,p2020-esdhc" and "fsl,p1010-esdhc".

Please do not use the phrase "fix the bug" in patch summaries.

--=20
Timur Tabi
Linux kernel developer at Freescale=

^ permalink raw reply

* Re: [PATCH powerpc] fix unpaired __trace_hcall_entry and __trace_hcall_exit
From: Benjamin Herrenschmidt @ 2012-01-03  0:54 UTC (permalink / raw)
  To: Li Zhong
  Cc: PowerPC email list, Paul E. McKenney, Paul Mackerras, LKML,
	Anton Blanchard
In-Reply-To: <1324260407.3006.17.camel@ThinkPad-T61>

On Mon, 2011-12-19 at 10:06 +0800, Li Zhong wrote:
> Unpaired calling of __trace_hcall_entry and __trace_hcall_exit could
> cause incorrect preempt count. And it might happen as the global
> variable hcall_tracepoint_refcount is checked separately before calling
> them. 
> 
> I don't know much about the powerpc arch. But the idea here is to store
> the hcall_tracepoint_refcount locally, so __trace_hcall_entry and
> __trace_hcall_exit will be called or not called in pair by checking the
> same value. 

Can you re-use an existing spot in the stack frame rather than adding
this entry ? There's plenty of unused spots :-) For example the link
editor doubleword is never going to be used in that function, we could
hijack it safely. It would make the patch (and the code) simpler.

Cheers,
Ben.

> Reported-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
> Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/hvCall.S |   20 +++++++++++---------
>  1 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/hvCall.S b/arch/powerpc/platforms/pseries/hvCall.S
> index fd05fde..1240bd2 100644
> --- a/arch/powerpc/platforms/pseries/hvCall.S
> +++ b/arch/powerpc/platforms/pseries/hvCall.S
> @@ -14,6 +14,7 @@
>  #include <asm/ptrace.h>
>  	
>  #define STK_PARM(i)     (48 + ((i)-3)*8)
> +#define REG_SIZE        (2*8)
>  
>  #ifdef CONFIG_TRACEPOINTS
>  
> @@ -32,11 +33,12 @@ hcall_tracepoint_refcount:
>   * unconditional cpu feature.
>   */
>  #define HCALL_INST_PRECALL(FIRST_REG)				\
> +	std	r31,-8(r1);					\
>  BEGIN_FTR_SECTION;						\
>  	b	1f;						\
>  END_FTR_SECTION(0, 1);						\
> -	ld      r12,hcall_tracepoint_refcount@toc(r2);		\
> -	cmpdi	r12,0;						\
> +	ld      r31,hcall_tracepoint_refcount@toc(r2);		\
> +	cmpdi	r31,0;						\
>  	beq+	1f;						\
>  	mflr	r0;						\
>  	std	r3,STK_PARM(r3)(r1);				\
> @@ -49,9 +51,9 @@ END_FTR_SECTION(0, 1);						\
>  	std	r10,STK_PARM(r10)(r1);				\
>  	std	r0,16(r1);					\
>  	addi	r4,r1,STK_PARM(FIRST_REG);			\
> -	stdu	r1,-STACK_FRAME_OVERHEAD(r1);			\
> +	stdu	r1,-STACK_FRAME_OVERHEAD-REG_SIZE(r1);		\
>  	bl	.__trace_hcall_entry;				\
> -	addi	r1,r1,STACK_FRAME_OVERHEAD;			\
> +	addi	r1,r1,STACK_FRAME_OVERHEAD+REG_SIZE;		\
>  	ld	r0,16(r1);					\
>  	ld	r3,STK_PARM(r3)(r1);				\
>  	ld	r4,STK_PARM(r4)(r1);				\
> @@ -74,8 +76,7 @@ END_FTR_SECTION(0, 1);						\
>  BEGIN_FTR_SECTION;						\
>  	b	1f;						\
>  END_FTR_SECTION(0, 1);						\
> -	ld      r12,hcall_tracepoint_refcount@toc(r2);		\
> -	cmpdi	r12,0;						\
> +	cmpdi	r31,0;						\
>  	beq+	1f;						\
>  	mflr	r0;						\
>  	ld	r6,STK_PARM(r3)(r1);				\
> @@ -83,13 +84,14 @@ END_FTR_SECTION(0, 1);						\
>  	mr	r4,r3;						\
>  	mr	r3,r6;						\
>  	std	r0,16(r1);					\
> -	stdu	r1,-STACK_FRAME_OVERHEAD(r1);			\
> +	stdu	r1,-STACK_FRAME_OVERHEAD-REG_SIZE(r1);		\
>  	bl	.__trace_hcall_exit;				\
> -	addi	r1,r1,STACK_FRAME_OVERHEAD;			\
> +	addi	r1,r1,STACK_FRAME_OVERHEAD+REG_SIZE;		\
>  	ld	r0,16(r1);					\
>  	ld	r3,STK_PARM(r3)(r1);				\
>  	mtlr	r0;						\
> -1:
> +1:								\
> +	ld	r31,-8(r1);
>  
>  #define HCALL_INST_POSTCALL_NORETS				\
>  	li	r5,0;						\

^ permalink raw reply

* RE: Problem in getting shared memory access on P1022RDK
From: Arshad, Farrukh @ 2012-01-03  9:42 UTC (permalink / raw)
  To: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <93CD5F41FDBC6042A6B449764F3B35CC050CCA22@EU-MBX-03.mgc.mentorg.com>

[-- Attachment #1: Type: text/plain, Size: 1807 bytes --]

Adding more to it,

When I write from Core 1 on the shared memory region it is visible at Core 0 and it can read what I have written from Core 1 but when I write from Core 0 on this shared memory it is not visible on Core 1.

Regards,
Farrukh Arshad

From: Arshad, Farrukh
Sent: Tuesday, January 03, 2012 12:11 AM
To: linuxppc-dev@lists.ozlabs.org
Subject: Problem in getting shared memory access on P1022RDK

Greetings All,

Please excuse me for a 'not very precise' question but I just need some clue or point to look for the trouble.
I am running linux in AMP configuration on P1022RDK dual core. My memory partitioning is given below

Core Base Address (CONFIG_PHYSICAL_START / bootm_low)
Size (bootm_size / mem kernel boot parameter)
Core 0 0x0000,0000
0x1000,0000
Core 1 0x1000,0000
0x0C00,0000
Shared Memory 0x1C00,0000
0x0400,0000

I have set bootm_low / bootm_size and provided mem parameter in both kernel configurations, both
kernels are booting up fine. I have a shared memory driver running in both cores. My shared memory
driver just exposes the physical memory (0x1C00,0000) to user-space.

The problem I am facing is I can not get access to same shared memory in user-space on both kernels.
I have verified my shared memory driver on another platform (p1022ds) and it works perfect. I have also
tried using non-cached memory mapping in shared memory driver but that does not work as well.

To me it seems one of my kernels is going beyond its defined memory partition (which I have set above), I have
set kernel's partitioning as CONFIG_PHYSICAL_START, bootm_size, bootm_low and mem kernel boot param.

Besides the shared memory driver, can anyone suggest or give me clue where should I look for the
trouble in the kernel.

Regards,
Farrukh Arshad.

[-- Attachment #2: Type: text/html, Size: 9938 bytes --]

^ permalink raw reply

* [RFC PATCH 0/9] Remove useless on_each_cpu return value
From: Gilad Ben-Yossef @ 2012-01-03 14:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-ia64, David Airlie, Will Deacon, dri-devel, Paul Mackerras,
	H. Peter Anvin, Russell King, x86, Ingo Molnar,
	Arnaldo Carvalho de Melo, Matt Turner, Fenghua Yu, Peter Zijlstra,
	devicetree-discuss, Michal Nazarewicz, Gilad Ben-Yossef,
	Ivan Kokshaysky, Rob Herring, Thomas Gleixner, linux-arm-kernel,
	Richard Henderson, Tony Luck, linux-alpha, linuxppc-dev

on_each_cpu() returns as its own return value the return value of 
smp_call_function(). smp_call_function() in turn returns a hard 
coded value of zero.

Some callers to on_each_cpu() waste cycles and bloat code space
by checking the return value to on_each_cpu(), probably for 
historical reasons.

This patch set refactors callers to not test on_each_cpu()
(fixed) return value and then refactors on_each_cpu to
return void to avoid confusing future users.

In other words, this patch aims to delete 18 source code lines
while not changing any functionality :-)

I tested as best as I could the x86 changes and compiled some
of the others, but I don't have access to all the needed hardware
for testing. Reviewers and testers welcome!

CC: Michal Nazarewicz <mina86@mina86.com>
CC: David Airlie <airlied@linux.ie>
CC: dri-devel@lists.freedesktop.org
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: Grant Likely <grant.likely@secretlab.ca>
CC: Rob Herring <rob.herring@calxeda.com>
CC: linuxppc-dev@lists.ozlabs.org
CC: devicetree-discuss@lists.ozlabs.org
CC: Richard Henderson <rth@twiddle.net>
CC: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
CC: Matt Turner <mattst88@gmail.com>
CC: linux-alpha@vger.kernel.org
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: x86@kernel.org
CC: Tony Luck <tony.luck@intel.com>
CC: Fenghua Yu <fenghua.yu@intel.com>
CC: linux-ia64@vger.kernel.org
CC: Will Deacon <will.deacon@arm.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
CC: Russell King <linux@arm.linux.org.uk>
CC: linux-arm-kernel@lists.infradead.org

Gilad Ben-Yossef (9):
  arm: avoid using on_each_cpu hard coded ret value
  ia64: avoid using on_each_cpu hard coded ret value
  x86: avoid using on_each_cpu hard coded ret value
  alpha: avoid using on_each_cpu hard coded ret value
  ppc: avoid using on_each_cpu hard coded ret value
  agp: avoid using on_each_cpu hard coded ret value
  drm: avoid using on_each_cpu hard coded ret value
  smp: refactor on_each_cpu to void returning func
  x86: refactor wbinvd_on_all_cpus to void function

 arch/alpha/kernel/smp.c      |    7 ++-----
 arch/arm/kernel/perf_event.c |    2 +-
 arch/ia64/kernel/perfmon.c   |   12 ++----------
 arch/powerpc/kernel/rtas.c   |    3 +--
 arch/x86/include/asm/smp.h   |    5 ++---
 arch/x86/lib/cache-smp.c     |    4 ++--
 drivers/char/agp/generic.c   |    3 +--
 drivers/gpu/drm/drm_cache.c  |    3 +--
 include/linux/smp.h          |    7 +++----
 kernel/smp.c                 |    6 ++----
 10 files changed, 17 insertions(+), 35 deletions(-)

^ permalink raw reply

* [RFC PATCH 5/9] ppc: avoid using on_each_cpu hard coded ret value
From: Gilad Ben-Yossef @ 2012-01-03 14:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: devicetree-discuss, Rob Herring, Gilad Ben-Yossef, Paul Mackerras,
	Michal Nazarewicz, linuxppc-dev
In-Reply-To: <1325600353-10895-1-git-send-email-gilad@benyossef.com>

on_each_cpu always returns a hard coded return code of zero.

Removing all tests based on this return value saves run time
cycles for compares and code bloat for branches.

CC: Michal Nazarewicz <mina86@mina86.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: Grant Likely <grant.likely@secretlab.ca>
CC: Rob Herring <rob.herring@calxeda.com>
CC: linuxppc-dev@lists.ozlabs.org
CC: devicetree-discuss@lists.ozlabs.org
---
 arch/powerpc/kernel/rtas.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 517b1d8..829629e 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -850,8 +850,7 @@ int rtas_ibm_suspend_me(struct rtas_args *args)
 	/* Call function on all CPUs.  One of us will make the
 	 * rtas call
 	 */
-	if (on_each_cpu(rtas_percpu_suspend_me, &data, 0))
-		atomic_set(&data.error, -EINVAL);
+	on_each_cpu(rtas_percpu_suspend_me, &data, 0);
 
 	wait_for_completion(&done);
 
-- 
1.7.0.4

^ permalink raw reply related

* [RFC PATCH 8/9] smp: refactor on_each_cpu to void returning func
From: Gilad Ben-Yossef @ 2012-01-03 14:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-ia64, David Airlie, Will Deacon, dri-devel, Paul Mackerras,
	H. Peter Anvin, Russell King, x86, Ingo Molnar,
	Arnaldo Carvalho de Melo, Matt Turner, Fenghua Yu, Peter Zijlstra,
	devicetree-discuss, Michal Nazarewicz, Gilad Ben-Yossef,
	Ivan Kokshaysky, Rob Herring, Thomas Gleixner, linux-arm-kernel,
	Richard Henderson, Tony Luck, linux-alpha, linuxppc-dev
In-Reply-To: <1325600353-10895-1-git-send-email-gilad@benyossef.com>

on_each_cpu returns the retunr value of smp_call_function
which is hard coded to 0.

Refactor on_each_cpu to a void function and the few callers
that check the return value to save compares and branches.

CC: Michal Nazarewicz <mina86@mina86.com>
CC: David Airlie <airlied@linux.ie>
CC: dri-devel@lists.freedesktop.org
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: Grant Likely <grant.likely@secretlab.ca>
CC: Rob Herring <rob.herring@calxeda.com>
CC: linuxppc-dev@lists.ozlabs.org
CC: devicetree-discuss@lists.ozlabs.org
CC: Richard Henderson <rth@twiddle.net>
CC: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
CC: Matt Turner <mattst88@gmail.com>
CC: linux-alpha@vger.kernel.org
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: x86@kernel.org
CC: Tony Luck <tony.luck@intel.com>
CC: Fenghua Yu <fenghua.yu@intel.com>
CC: linux-ia64@vger.kernel.org
CC: Will Deacon <will.deacon@arm.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
CC: Russell King <linux@arm.linux.org.uk>
CC: linux-arm-kernel@lists.infradead.org
---
 include/linux/smp.h |    7 +++----
 kernel/smp.c        |    6 ++----
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/include/linux/smp.h b/include/linux/smp.h
index 8cc38d3..050ddd4 100644
--- a/include/linux/smp.h
+++ b/include/linux/smp.h
@@ -99,7 +99,7 @@ static inline void call_function_init(void) { }
 /*
  * Call a function on all processors
  */
-int on_each_cpu(smp_call_func_t func, void *info, int wait);
+void on_each_cpu(smp_call_func_t func, void *info, int wait);
 
 /*
  * Mark the boot cpu "online" so that it can call console drivers in
@@ -126,12 +126,11 @@ static inline int up_smp_call_function(smp_call_func_t func, void *info)
 #define smp_call_function(func, info, wait) \
 			(up_smp_call_function(func, info))
 #define on_each_cpu(func,info,wait)		\
-	({					\
+	{					\
 		local_irq_disable();		\
 		func(info);			\
 		local_irq_enable();		\
-		0;				\
-	})
+	}
 static inline void smp_send_reschedule(int cpu) { }
 #define num_booting_cpus()			1
 #define smp_prepare_boot_cpu()			do {} while (0)
diff --git a/kernel/smp.c b/kernel/smp.c
index db197d6..f66a1b2 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -687,17 +687,15 @@ void __init smp_init(void)
  * early_boot_irqs_disabled is set.  Use local_irq_save/restore() instead
  * of local_irq_disable/enable().
  */
-int on_each_cpu(void (*func) (void *info), void *info, int wait)
+void on_each_cpu(void (*func) (void *info), void *info, int wait)
 {
 	unsigned long flags;
-	int ret = 0;
 
 	preempt_disable();
-	ret = smp_call_function(func, info, wait);
+	smp_call_function(func, info, wait);
 	local_irq_save(flags);
 	func(info);
 	local_irq_restore(flags);
 	preempt_enable();
-	return ret;
 }
 EXPORT_SYMBOL(on_each_cpu);
-- 
1.7.0.4

^ permalink raw reply related

* Re: [RFC PATCH 0/9] Remove useless on_each_cpu return value
From: Michal Nazarewicz @ 2012-01-03 14:25 UTC (permalink / raw)
  To: linux-kernel, Gilad Ben-Yossef
  Cc: linux-ia64, David Airlie, Will Deacon, dri-devel, Paul Mackerras,
	H. Peter Anvin, Russell King, x86, Ingo Molnar,
	Arnaldo Carvalho de Melo, Matt Turner, Fenghua Yu, Peter Zijlstra,
	devicetree-discuss, Rob Herring, Ivan Kokshaysky, Thomas Gleixner,
	linux-arm-kernel, Richard Henderson, Tony Luck, linux-alpha,
	linuxppc-dev
In-Reply-To: <1325600353-10895-1-git-send-email-gilad@benyossef.com>

On Tue, 03 Jan 2012 15:19:04 +0100, Gilad Ben-Yossef <gilad@benyossef.co=
m> wrote:
> on_each_cpu() returns as its own return value the return value of
> smp_call_function(). smp_call_function() in turn returns a hard
> coded value of zero.
>
> Some callers to on_each_cpu() waste cycles and bloat code space
> by checking the return value to on_each_cpu(), probably for
> historical reasons.
>
> This patch set refactors callers to not test on_each_cpu()
> (fixed) return value and then refactors on_each_cpu to
> return void to avoid confusing future users.
>
> In other words, this patch aims to delete 18 source code lines
> while not changing any functionality :-)
>
> I tested as best as I could the x86 changes and compiled some
> of the others, but I don't have access to all the needed hardware
> for testing. Reviewers and testers welcome!

Other then the lack of Signed-off-by in the patches, looks good to me,
even though personally I'd choose a bottom-up approach, ie. make
smp_call_function() return void and from that conclude that
on_each_cpu() can return void.  With those patches, we have a situation,=

where smp_call_function() has a return value which is then lost for no
immediately apparent reason lost in on_each_cpu().

-- =

Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=3D./ `o
..o | Computer Science,  Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz=
    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

^ permalink raw reply

* Re: [RFC PATCH 0/9] Remove useless on_each_cpu return value
From: Gilad Ben-Yossef @ 2012-01-03 16:08 UTC (permalink / raw)
  To: Michal Nazarewicz
  Cc: linux-ia64, David Airlie, Will Deacon, dri-devel, Paul Mackerras,
	H. Peter Anvin, Russell King, x86, Ingo Molnar,
	Arnaldo Carvalho de Melo, Matt Turner, Fenghua Yu, Peter Zijlstra,
	devicetree-discuss, Rob Herring, Ivan Kokshaysky, Thomas Gleixner,
	linux-arm-kernel, Richard Henderson, Tony Luck, linux-kernel,
	linux-alpha, linuxppc-dev
In-Reply-To: <op.v7iidbwz3l0zgt@mpn-glaptop>

2012/1/3 Michal Nazarewicz <mina86@mina86.com>:
> On Tue, 03 Jan 2012 15:19:04 +0100, Gilad Ben-Yossef <gilad@benyossef.com=
>
> wrote:
>>
>> on_each_cpu() returns as its own return value the return value of
>> smp_call_function(). smp_call_function() in turn returns a hard
>> coded value of zero.
>>
>> Some callers to on_each_cpu() waste cycles and bloat code space
>> by checking the return value to on_each_cpu(), probably for
>> historical reasons.
>>
>> This patch set refactors callers to not test on_each_cpu()
>> (fixed) return value and then refactors on_each_cpu to
>> return void to avoid confusing future users.
>>
>> In other words, this patch aims to delete 18 source code lines
>> while not changing any functionality :-)
>>
>> I tested as best as I could the x86 changes and compiled some
>> of the others, but I don't have access to all the needed hardware
>> for testing. Reviewers and testers welcome!
>
>
> Other then the lack of Signed-off-by in the patches, looks good to me,

Blimey! I'll resend with a proper Signed-off-by after more people have
a chance to
comment. And thanks for the review.

> even though personally I'd choose a bottom-up approach, ie. make
> smp_call_function() return void and from that conclude that
> on_each_cpu() can return void. =A0With those patches, we have a situation=
,
> where smp_call_function() has a return value which is then lost for no
> immediately apparent reason lost in on_each_cpu().

There are so many call site of smp_call_function() that do not check the
return value right now that I think we can tolerate it for just a
little bit longer
until that get fixed as well... :-)

Thanks,
Gilad


--=20
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"Unfortunately, cache misses are an equal opportunity pain provider."
-- Mike Galbraith, LKML

^ permalink raw reply

* Re: Problem in getting shared memory access on P1022RDK
From: Scott Wood @ 2012-01-03 17:10 UTC (permalink / raw)
  To: Arshad, Farrukh; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <93CD5F41FDBC6042A6B449764F3B35CC050CCA80@EU-MBX-03.mgc.mentorg.com>

On 01/03/2012 03:42 AM, Arshad, Farrukh wrote:
> Adding more to it,
> 
>  
> 
> When I write from Core 1 on the shared memory region it is visible at
> Core 0 and it can read what I have written from Core 1 but when I write
> from Core 0 on this shared memory it is not visible on Core 1.

Is the memory mapped coherent on both cores?

-Scott

^ permalink raw reply

* Re: [PATCH 2/2] mtd/nand: Fix IFC driver to support 2K NAND page
From: Scott Wood @ 2012-01-03 19:49 UTC (permalink / raw)
  To: Prabhakar Kushwaha; +Cc: linux-mtd, linuxppc-dev, Poonam Aggrwal
In-Reply-To: <1325134779-3571-3-git-send-email-prabhakar@freescale.com>

On 12/28/2011 10:59 PM, Prabhakar Kushwaha wrote:
> 1) OOB area should be updated irrespective of NAND page size. Earlier it was
> updated only for 512byte NAND page.
> 
> 2) During OOB update fbcr should be equal to OOB size.
> 
> Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com>
> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
> ---
>  git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git (branch next)

The IFC driver hasn't been merged into that tree that I can see.

>  Tested on P1010RDB
> 
>  drivers/mtd/nand/fsl_ifc_nand.c |   20 ++++++++------------
>  1 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
> index 2df7206..2c02168 100644
> --- a/drivers/mtd/nand/fsl_ifc_nand.c
> +++ b/drivers/mtd/nand/fsl_ifc_nand.c
> @@ -439,20 +439,16 @@ static void fsl_ifc_cmdfunc(struct mtd_info *mtd, unsigned int command,
>  			out_be32(&ifc->ifc_nand.nand_fir1,
>  				 (IFC_FIR_OP_CW1 << IFC_NAND_FIR1_OP5_SHIFT));
>  
> -			if (column >= mtd->writesize) {
> -				/* OOB area --> READOOB */
> -				column -= mtd->writesize;
> -				nand_fcr0 |= NAND_CMD_READOOB <<
> -						IFC_NAND_FCR0_CMD0_SHIFT;
> -				ifc_nand_ctrl->oob = 1;
> -			} else if (column < 256)
> +			if (column < 256)
>  				/* First 256 bytes --> READ0 */
>  				nand_fcr0 |=
>  				NAND_CMD_READ0 << IFC_NAND_FCR0_CMD0_SHIFT;
> -			else
> -				/* Second 256 bytes --> READ1 */
> -				nand_fcr0 |=
> -				NAND_CMD_READ1 << IFC_NAND_FCR0_CMD0_SHIFT;
> +		}
> +
> +		if (column >= mtd->writesize) {
> +			/* OOB area --> READOOB */
> +			column -= mtd->writesize;
> +			ifc_nand_ctrl->oob = 1;
>  		}

Where is NAND_CMD_READOOB going to be set in the small-page case?

The small-page code should read something like:

	if (column >= mtd->writesize) {
		nand_fcr0 |=
			NAND_CMD_READOOB << IFC_NAND_FCR0_CMD0_SHIFT;
	} else {
		nand_fcr0 |=
			NAND_CMD_READ0 << IFC_NAND_FCR0_CMD0_SHIFT;
	}

It looks like we can get rid of ctrl->column, BTW.

-Scott

^ permalink raw reply

* Re: [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank()
From: Scott Wood @ 2012-01-03 20:24 UTC (permalink / raw)
  To: Prabhakar Kushwaha; +Cc: linux-mtd, linuxppc-dev, Poonam Aggrwal
In-Reply-To: <1325134779-3571-2-git-send-email-prabhakar@freescale.com>

On 12/28/2011 10:59 PM, Prabhakar Kushwaha wrote:
> IFC NAND Machine calculates ECC on 512byte sector. Same is taken care in
> fsl_ifc_run_command() while ECC status verification. Here buffer number is
> calculated assuming 512byte sector and same is passed to is_blank.
> However in is_blank() buffer address is calculated using mdt->writesize which is
> wrong. It should be calculated on basis of ecc sector size.
> 
> Also, in fsl_ifc_run_command() bufferpage is calculated on the basis of ecc sector
> size instead of hard coded value.
> 
> Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com>
> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
> ---
>  git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git (branch next)
> 
>  Tested on P1010RDB
> 
>  drivers/mtd/nand/fsl_ifc_nand.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
> index 8475b88..2df7206 100644
> --- a/drivers/mtd/nand/fsl_ifc_nand.c
> +++ b/drivers/mtd/nand/fsl_ifc_nand.c
> @@ -191,7 +191,9 @@ static int is_blank(struct mtd_info *mtd, unsigned int bufnum)
>  {
>  	struct nand_chip *chip = mtd->priv;
>  	struct fsl_ifc_mtd *priv = chip->priv;
> -	u8 __iomem *addr = priv->vbase + bufnum * (mtd->writesize * 2);
> +	int bufperpage = mtd->writesize / chip->ecc.size;
> +	u8 __iomem *addr = priv->vbase + bufnum / bufperpage
> +					* (mtd->writesize * 2);
>  	u32 __iomem *mainarea = (u32 *)addr;
>  	u8 __iomem *oob = addr + mtd->writesize;
>  	int i;

This function should only be checking one ECC block, not the entire
page.  The caller is responsible for passing in the appropriate buffer
numbers.

I think what the current code needs is for (mtd->writesize * 2) to be
replaced with chip->ecc.size, and for the calling code to multiply the
starting bufnum by two.

> @@ -273,7 +275,7 @@ static void fsl_ifc_run_command(struct mtd_info *mtd)
>  		dev_err(priv->dev, "NAND Flash Write Protect Error\n");
>  
>  	if (nctrl->eccread) {
> -		int bufperpage = mtd->writesize / 512;
> +		int bufperpage = mtd->writesize / chip->ecc.size;
>  		int bufnum = (nctrl->page & priv->bufnum_mask) * bufperpage;
>  		int bufnum_end = bufnum + bufperpage - 1;
>  

Currently this driver always sets chip->ecc.size to 512.  If we want to
support other ECC block sizes that future versions of IFC may have, can
we calculate bufperpage during chip init (similar to bufnum_mask) to
avoid the runtime division?  It's probably not huge overhead compared to
everything else we do per NAND page transfer, but still...

-Scott

^ permalink raw reply

* Re: [PATCH v3] powerpc/85xx: add support to JOG feature using cpufreq interface
From: Scott Wood @ 2012-01-03 22:14 UTC (permalink / raw)
  To: Zhao Chenhui; +Cc: linuxppc-dev, Jerry Huang
In-Reply-To: <1324985129-26219-1-git-send-email-chenhui.zhao@freescale.com>

On 12/27/2011 05:25 AM, Zhao Chenhui wrote:
>  * The driver doesn't support MPC8536 Rev 1.0 due to a JOG erratum.
>    Subsequent revisions of MPC8536 have corrected the erratum.

Where do you check for this?

> +#define POWMGTCSR_LOSSLESS_MASK	0x00400000
> +#define POWMGTCSR_JOG_MASK	0x00200000

Are these really masks, or just values to use?

> +#define POWMGTCSR_CORE0_IRQ_MSK	0x80000000
> +#define POWMGTCSR_CORE0_CI_MSK	0x40000000
> +#define POWMGTCSR_CORE0_DOZING	0x00000008
> +#define POWMGTCSR_CORE0_NAPPING	0x00000004
> +
> +#define POWMGTCSR_CORE_INT_MSK	0x00000800
> +#define POWMGTCSR_CORE_CINT_MSK	0x00000400
> +#define POWMGTCSR_CORE_UDE_MSK	0x00000200
> +#define POWMGTCSR_CORE_MCP_MSK	0x00000100
> +#define P1022_POWMGTCSR_MSK	(POWMGTCSR_CORE_INT_MSK | \
> +				 POWMGTCSR_CORE_CINT_MSK | \
> +				 POWMGTCSR_CORE_UDE_MSK | \
> +				 POWMGTCSR_CORE_MCP_MSK)
> +
> +static void keep_waking_up(void *dummy)
> +{
> +	unsigned long flags;
> +
> +	local_irq_save(flags);
> +	mb();
> +
> +	in_jog_process = 1;
> +	mb();
> +
> +	while (in_jog_process != 0)
> +		mb();
> +
> +	local_irq_restore(flags);
> +}

Please document this.  Compare in_jog_process == 1, not != 0 -- it's
unlikely, but what if the other cpu sees that in_jog_process has been
set to 1, exits and sets in_jog_process to 0, then re-enters set_pll and
sets in_jog_process to -1 again before this function does another load
of in_jog_process?

Do you really need all these mb()s?  I think this would suffice:

	local_irq_save(flags);

	in_jog_process = 1;

	while (in_jog_process == 1)
		barrier();

	local_irq_restore();

It's not really a performance issue, just simplicity.

> +static int p1022_set_pll(unsigned int cpu, unsigned int pll)
> +{
> +	int index, hw_cpu = get_hard_smp_processor_id(cpu);
> +	int shift;
> +	u32 corefreq, val, mask = 0;
> +	unsigned int cur_pll = get_pll(hw_cpu);
> +	unsigned long flags;
> +	int ret = 0;
> +
> +	if (pll == cur_pll)
> +		return 0;
> +
> +	shift = hw_cpu * CORE_RATIO_BITS + CORE0_RATIO_SHIFT;
> +	val = (pll & CORE_RATIO_MASK) << shift;
> +
> +	corefreq = sysfreq * pll / 2;
> +	/*
> +	 * Set the COREx_SPD bit if the requested core frequency
> +	 * is larger than the threshold frequency.
> +	 */
> +	if (corefreq > FREQ_533MHz)
> +		val |= PMJCR_CORE0_SPD_MASK << hw_cpu;

P1022 manual says the threshold is 500 MHz (but doesn't say how to set
the bit if the frequency is exactly 500 MHz).  Where did 533340000 come
from?

> +
> +	mask = (CORE_RATIO_MASK << shift) | (PMJCR_CORE0_SPD_MASK << hw_cpu);
> +	clrsetbits_be32(guts + PMJCR, mask, val);
> +
> +	/* readback to sync write */
> +	val = in_be32(guts + PMJCR);

You don't use val after this -- just ignore the return value from in_be32().

> +	/*
> +	 * A Jog request can not be asserted when any core is in a low
> +	 * power state on P1022. Before executing a jog request, any
> +	 * core which is in a low power state must be waked by a
> +	 * interrupt, and keep waking up until the sequence is
> +	 * finished.
> +	 */
> +	for_each_present_cpu(index) {
> +		if (!cpu_online(index))
> +			return -EFAULT;
> +	}

EFAULT is not the appropriate error code -- it is for when userspace
passes a bad virtual address.

Better, don't fail here -- bring the other core out of the low power
state in order to do the jog.  cpufreq shouldn't stop working just
because we took a core offline.

What prevents a core from going offline just after you check here?

> +	in_jog_process = -1;
> +	mb();
> +	smp_call_function(keep_waking_up, NULL, 0);

What does "keep waking up" mean?  Something like spin_while_jogging
might be clearer.

> +	local_irq_save(flags);
> +	mb();
> +	/* Wait for the other core to wake. */
> +	while (in_jog_process != 1)
> +		mb();

Timeout?  And more unnecessary mb()s.

Might be nice to support more than two cores, even if this code isn't
currently expected to be used on such hardware (it's just a generic
"hold other cpus" loop; might as well make it reusable).  You could do
this by using an atomic count for other cores to check in and out of the
spin loop.

> +	out_be32(guts + POWMGTCSR, POWMGTCSR_JOG_MASK | P1022_POWMGTCSR_MSK);
> +
> +	if (!spin_event_timeout(((in_be32(guts + POWMGTCSR) &
> +	    POWMGTCSR_JOG_MASK) == 0), 10000, 10)) {
> +		pr_err("%s: Fail to switch the core frequency.\n", __func__);
> +		ret = -EFAULT;
> +	}
> +
> +	clrbits32(guts + POWMGTCSR, P1022_POWMGTCSR_MSK);
> +	in_jog_process = 0;
> +	mb();

This mb() (or better, a readback of POWMGTCSR) should be before you
clear in_jog_process.  For clarity of its purpose, the clearing of
POWMGTCSR should go in the failure branch of spin_event_timeout().

> +	/* the latency of a transition, the unit is ns */
> +	policy->cpuinfo.transition_latency = 2000;

Is this based on observation?

> diff --git a/arch/powerpc/platforms/85xx/sleep.S b/arch/powerpc/platforms/85xx/sleep.S
> index 763d2f2..919781d 100644
> --- a/arch/powerpc/platforms/85xx/sleep.S
> +++ b/arch/powerpc/platforms/85xx/sleep.S
> @@ -59,6 +59,7 @@ powmgtreq:
>  	 * r5 = JOG or deep sleep request
>  	 *      JOG-0x00200000, deep sleep-0x00100000
>  	 */
> +_GLOBAL(mpc85xx_enter_jog)
>  _GLOBAL(mpc85xx_enter_deep_sleep)
>  	lis	r6, ccsrbase_low@ha
>  	stw	r4, ccsrbase_low@l(r6)

Why does this need two entry points rather than a more appropriate name?

> diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
> index 3fe6d92..1d0c4e0 100644
> --- a/arch/powerpc/platforms/Kconfig
> +++ b/arch/powerpc/platforms/Kconfig
> @@ -200,6 +200,14 @@ config CPU_FREQ_PMAC64
>  	  This adds support for frequency switching on Apple iMac G5,
>  	  and some of the more recent desktop G5 machines as well.
>  
> +config MPC85xx_CPUFREQ
> +	bool "Support for Freescale MPC85xx CPU freq"
> +	depends on PPC_85xx && PPC32
> +	select CPU_FREQ_TABLE
> +	help
> +	  This adds support for frequency switching on Freescale MPC85xx,
> +	  currently including P1022 and MPC8536.

default y, given the dependencies?  Or wait for more testing before we
do that?

-Scott

^ permalink raw reply

* RE: [PATCH] DTS: fix the bug and add the chip compatible for eSDHC
From: Huang Changming-R66093 @ 2012-01-04  3:11 UTC (permalink / raw)
  To: Wood Scott-B07421; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4F01F721.7090407@freescale.com>



> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, January 03, 2012 2:28 AM
> To: Huang Changming-R66093
> Cc: linuxppc-dev@lists.ozlabs.org; Huang Changming-R66093
> Subject: Re: [PATCH] DTS: fix the bug and add the chip compatible for
> eSDHC
>=20
> On 12/23/2011 12:10 AM, r66093@freescale.com wrote:
> > From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >
> > Accordint to latest kernel, the auto-cmd12 property should be
> > "sdhci,auto-cmd12", and according to the SDHC binding and the
> > workaround for the special chip, add the chip compatible for eSDHC:
> > "fsl,p1022-esdhc", "fsl,mpc8536-esdhc", "fsl,p2020-esdhc" and
> "fsl,p1010-esdhc".
> >
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > ---
> >  arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi |    4 ++++
> >  arch/powerpc/boot/dts/fsl/p1010si-post.dtsi   |    3 ++-
> >  arch/powerpc/boot/dts/fsl/p1022si-post.dtsi   |    3 ++-
> >  arch/powerpc/boot/dts/fsl/p2020si-post.dtsi   |    4 ++++
> >  4 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
> > b/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
> > index 89af626..44e0ed9 100644
> > --- a/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
> > +++ b/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
> > @@ -236,6 +236,10 @@
> >  	};
> >
> >  /include/ "pq3-esdhc-0.dtsi"
> > +	sdhc@2e000 {
> > +		compatible =3D "fsl,esdhc", "fsl,mpc8536-esdhc";
> > +	};
>=20
> More-specific compatible entries should come first.

I don't understand you, why more-specific compatible entries should come?
The Documentation/devicetree/bindings/mmc/fsl-esdhc.txt has introduced it:
  - compatible : should be
    "fsl,<chip>-esdhc", "fsl,esdhc"
I don't think I should introduce new entries.

^ permalink raw reply

* RE: [PATCH] DTS: fix the bug and add the chip compatible for eSDHC
From: Huang Changming-R66093 @ 2012-01-04  3:14 UTC (permalink / raw)
  To: Tabi Timur-B04825; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <CAOZdJXXXvTuJO_ZpyeRatWxTMbmcEVokA9qx7ajNdzKyiEgBhw@mail.gmail.com>



> -----Original Message-----
> From: Tabi Timur-B04825
> Sent: Tuesday, January 03, 2012 4:30 AM
> To: Huang Changming-R66093
> Cc: linuxppc-dev@lists.ozlabs.org; Huang Changming-R66093
> Subject: Re: [PATCH] DTS: fix the bug and add the chip compatible for
> eSDHC
>=20
> On Fri, Dec 23, 2011 at 12:10 AM,  <r66093@freescale.com> wrote:
> >
> > Accordint to latest kernel, the auto-cmd12 property should be
> > "sdhci,auto-cmd12", and according to the SDHC binding and the
> > workaround for the special chip, add the chip compatible for eSDHC:
> > "fsl,p1022-esdhc", "fsl,mpc8536-esdhc", "fsl,p2020-esdhc" and
> "fsl,p1010-esdhc".
>=20
> Please do not use the phrase "fix the bug" in patch summaries.
>=20
Then, could you tell me how to describe it?

^ permalink raw reply

* Re: [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank()
From: Prabhakar @ 2012-01-04  4:35 UTC (permalink / raw)
  To: Scott Wood; +Cc: linux-mtd, linuxppc-dev, Poonam Aggrwal
In-Reply-To: <4F0363E2.6040702@freescale.com>

On Wednesday 04 January 2012 01:54 AM, Scott Wood wrote:
> On 12/28/2011 10:59 PM, Prabhakar Kushwaha wrote:
>> IFC NAND Machine calculates ECC on 512byte sector. Same is taken care in
>> fsl_ifc_run_command() while ECC status verification. Here buffer number is
>> calculated assuming 512byte sector and same is passed to is_blank.
>> However in is_blank() buffer address is calculated using mdt->writesize which is
>> wrong. It should be calculated on basis of ecc sector size.
>>
>> Also, in fsl_ifc_run_command() bufferpage is calculated on the basis of ecc sector
>> size instead of hard coded value.
>>
>> Signed-off-by: Poonam Aggrwal<poonam.aggrwal@freescale.com>
>> Signed-off-by: Prabhakar Kushwaha<prabhakar@freescale.com>
>> ---
>>   git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git (branch next)
>>
>>   Tested on P1010RDB
>>
>>   drivers/mtd/nand/fsl_ifc_nand.c |    6 ++++--
>>   1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
>> index 8475b88..2df7206 100644
>> --- a/drivers/mtd/nand/fsl_ifc_nand.c
>> +++ b/drivers/mtd/nand/fsl_ifc_nand.c
>> @@ -191,7 +191,9 @@ static int is_blank(struct mtd_info *mtd, unsigned int bufnum)
>>   {
>>   	struct nand_chip *chip = mtd->priv;
>>   	struct fsl_ifc_mtd *priv = chip->priv;
>> -	u8 __iomem *addr = priv->vbase + bufnum * (mtd->writesize * 2);
>> +	int bufperpage = mtd->writesize / chip->ecc.size;
>> +	u8 __iomem *addr = priv->vbase + bufnum / bufperpage
>> +					* (mtd->writesize * 2);
>>   	u32 __iomem *mainarea = (u32 *)addr;
>>   	u8 __iomem *oob = addr + mtd->writesize;
>>   	int i;
> This function should only be checking one ECC block, not the entire
> page.  The caller is responsible for passing in the appropriate buffer
> numbers.
>
> I think what the current code needs is for (mtd->writesize * 2) to be
> replaced with chip->ecc.size, and for the calling code to multiply the
> starting bufnum by two.

     Got your point :). I will take care in next patch version.


>> @@ -273,7 +275,7 @@ static void fsl_ifc_run_command(struct mtd_info *mtd)
>>   		dev_err(priv->dev, "NAND Flash Write Protect Error\n");
>>
>>   	if (nctrl->eccread) {
>> -		int bufperpage = mtd->writesize / 512;
>> +		int bufperpage = mtd->writesize / chip->ecc.size;
>>   		int bufnum = (nctrl->page&  priv->bufnum_mask) * bufperpage;
>>   		int bufnum_end = bufnum + bufperpage - 1;
>>
> Currently this driver always sets chip->ecc.size to 512.  If we want to
> support other ECC block sizes that future versions of IFC may have, can
> we calculate bufperpage during chip init (similar to bufnum_mask) to
> avoid the runtime division?  It's probably not huge overhead compared to
> everything else we do per NAND page transfer, but still...
>

   Yes. I agree.
    We are working on this in order to support new controller version.

--Prabhakar

^ permalink raw reply


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