LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: NAND BBT corruption on MPC83xx
From: Scott Wood @ 2011-06-17 21:34 UTC (permalink / raw)
  To: Matthew L. Creech; +Cc: linux-mtd, linuxppc-dev
In-Reply-To: <BANLkTikyKObukzVw+c10HyDz+Q=PH4jozA@mail.gmail.com>

On Fri, 17 Jun 2011 16:54:27 -0400
"Matthew L. Creech" <mlcreech@gmail.com> wrote:

> Hi, I posted this on the Linux-MTD list but haven't gotten any hits.
> Since it looks like it could be MPC83xx-specific, I'm reposting here.
> Rick Johnson noted a problem in fsl_elbc_nand.c back in May which
> might be related:
> 
> http://lists.infradead.org/pipermail/linux-mtd/2011-May/035372.html

It seems that the generic code always passes -1 with PAGEPROG, and only
provides the actual page address on SEQIN.

I don't think the ECC readback is needed, and the fact that it looks like
it has always been broken would seem to confirm that.  It's broken in
other ways, too -- it assumes a particular ECC layout.  Let's get rid of it.

As for the corruption, could it be degradation from repeated reads of that
one page?

> More info on this board:
> - MPC 8313 SoC
> - 1GB Samsung NAND flash (K9K8G08U0B)
> - Linux 2.6.31
> - U-Boot 2009.06

Hmm, 2.6.31... it's probably not related to this problem, but you
should cherry pick b3a70f0bc32d1b70584bcaa6019fa4260b0da92e and
476459a6cf46d20ec73d9b211f3894ced5f9871e.

-Scott

^ permalink raw reply

* NAND BBT corruption on MPC83xx
From: Matthew L. Creech @ 2011-06-17 20:54 UTC (permalink / raw)
  To: linuxppc-dev

Hi, I posted this on the Linux-MTD list but haven't gotten any hits.
Since it looks like it could be MPC83xx-specific, I'm reposting here.
Rick Johnson noted a problem in fsl_elbc_nand.c back in May which
might be related:

http://lists.infradead.org/pipermail/linux-mtd/2011-May/035372.html


We've gotten some devices back from the field which all suffer from
this same problem on bootup when attaching UBI (these messages are
from U-Boot):

...
Bad block table found at page 524224, version 0x01
Bad block table found at page 524160, version 0x01
nand_bbt: ECC error while reading bad block table
...(long stream of bogus bad blocks)...
UBI: attaching mtd1 to ubi0
UBI: physical eraseblock size:   131072 bytes (128 KiB)
UBI: logical eraseblock size:    129024 bytes
UBI: smallest flash I/O unit:    2048
UBI: sub-page size:              512
UBI: VID header offset:          512 (aligned 512)
UBI: data offset:                2048
UBI error: vtbl_check: volume table check failed: record 0, error 9
UBI error: ubi_init: cannot attach mtd1
UBI error: ubi_init: UBI error: cannot initialize UBI, error -22
UBI init error -22

Full console dumps from 2 devices are here:

http://mcreech.com/work/bbt-ecc-error.txt
http://mcreech.com/work/bbt-ecc-error2.txt

Another device encountered a slightly different error, but which I
assume is due to the same underlying problem:

UBI error: init_volumes: not enough PEBs, required 8061, available 8059
UBI error: ubi_wl_init_scan: no enough physical eraseblocks (-2, need 1)

A full dump from that one is here:

http://mcreech.com/work/bbt-ecc-error3.txt

Are there any known issues that could cause the BBT to
become corrupt like this?

I noticed that the reported bad blocks were all aligned at multiples
of 0x80000 (with one exception).  Dump #1 shows:
 - one BBT with lots of bytes that have their lower 1 or 2 bits
un-set (e.g. 0xfe instead of 0xff): this explains all the
each-4th-block alignment.
 - the other BBT shows only one factory-marked bad block at
0x062e0000, which is presumably correct.  This is preserved in the
bogus BBT, and is the only non-0x80000-aligned bad block in the table.
 - Only the first 1024 bytes of the BBT contain bogus info - the
latter half of the BBT is all correct

It seems like the original BBT somehow had 0-2 bits corrupted at the
low end of each of its bytes, either while in memory or when the BBT
was written to NAND.  Any ideas on what I can do to isolate the
problem?  Thanks in advance!

More info on this board:
- MPC 8313 SoC
- 1GB Samsung NAND flash (K9K8G08U0B)
- Linux 2.6.31
- U-Boot 2009.06

-- 
Matthew L. Creech

^ permalink raw reply

* Re: [PATCH 1/3] powerpc: POWER7 optimised copy_page using VMX
From: Segher Boessenkool @ 2011-06-17 20:26 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: paulus, mikey, linuxppc-dev, Anton Blanchard
In-Reply-To: <1308290028.32158.45.camel@pasglop>

>> +	addi	r1,r1,STACKFRAMESIZE
>> +
>> +	.align	5
>
> Do we know that the blank will be filled with something harmless ?

Yes.  See ppc_handle_align() in gas/config/tc-ppc.c : it fills with nops
(ori 0,0,0), and a branch if there are more than four nops, and for 
POWER6
and POWER7 it puts a group-terminating insn (ori 1,1,0 resp. ori 2,2,0) 
at
the end.

It has done this (well, nops at least) since 2001.


Segher

^ permalink raw reply

* Re: [PATCH] Add cpufreq driver for Momentum Maple boards
From: Dave Jones @ 2011-06-17 20:07 UTC (permalink / raw)
  To: Dmitry Eremin-Solenikov; +Cc: paulus, linuxppc-dev, cpufreq, Arnd Bergmann
In-Reply-To: <BANLkTimo6k=Zz+QnP0VaWYf3jDjpFXNL4w@mail.gmail.com>

On Fri, Jun 17, 2011 at 03:12:56PM +0400, Dmitry Eremin-Solenikov wrote:
 > What about drivers/cpufreq/powerpc, or it's an unnecessary?

We haven't done it so far for x86 & arm, so for now at least, just keeping
them in drivers/cpufreq/ should be sufficient.

 > Should I resumbit it, or there will be massive arch/powerpc ->
 > drivers/cpufreq move?

Good question. I haven't heard anything from any of the PPC maintainers
on this subject, but I'll be happy to help out where I can if they
decide to follow suit with x86/arm.

	Dave

^ permalink raw reply

* Re: [PATCH] fs_enet: fix freescale FCC ethernet dp buffer alignment
From: David Miller @ 2011-06-17 19:21 UTC (permalink / raw)
  To: holger.brunck; +Cc: clive.stubbings, netdev, linuxppc-dev, vbordug
In-Reply-To: <1308299439-5975-1-git-send-email-holger.brunck@keymile.com>

From: Holger Brunck <holger.brunck@keymile.com>
Date: Fri, 17 Jun 2011 10:30:39 +0200

> From: Clive Stubbings <clive.stubbings@xentech.co.uk>
> 
> The RIPTR and TIPTR  (receive/transmit internal temporary data pointer),
> used by microcode as a temporary buffer for data, must be 32-byte aligned
> according to the RM for MPC8247.
> 
> Tested on mgcoge.
> 
> Signed-off-by: Clive Stubbings <clive.stubbings@xentech.co.uk>
> Signed-off-by: Holger Brunck <holger.brunck@keymile.com>
> cc: Pantelis Antoniou <pantelis.antoniou@gmail.com>
> cc: Vitaly Bordug <vbordug@ru.mvista.com>
> cc: netdev@vger.kernel.org

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] powerpc/85xx: fix NAND_CMD_READID read bytes number
From: Bryan Hundven @ 2011-06-17 17:58 UTC (permalink / raw)
  To: Shaohui Xie; +Cc: scottwood, linux-mtd, linuxppc-dev
In-Reply-To: <1307931792-30386-1-git-send-email-Shaohui.Xie@freescale.com>

On Sun, Jun 12, 2011 at 7:23 PM, Shaohui Xie <Shaohui.Xie@freescale.com> wr=
ote:
> when nand_get_flash_type() is called, it will read 8 bytes of ID instead =
of 5,
> but the driver only read 5 bytes, so kernel will dump error messages like=
:
>
> fsl-lbc ffe124000.localbus: read_byte beyond end of buffer
> fsl-lbc ffe124000.localbus: read_byte beyond end of buffer
> fsl-lbc ffe124000.localbus: read_byte beyond end of buffer
>
> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> Acked-by: Scott Wood <scottwood@freescale.com>
> ---
> =C2=A0drivers/mtd/nand/fsl_elbc_nand.c | =C2=A0 =C2=A06 +++---
> =C2=A01 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc=
_nand.c
> index 0bb254c..33d8aad 100644
> --- a/drivers/mtd/nand/fsl_elbc_nand.c
> +++ b/drivers/mtd/nand/fsl_elbc_nand.c
> @@ -339,9 +339,9 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, un=
signed int command,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(FIR_OP_UA =C2=A0<< FIR=
_OP1_SHIFT) |
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(FIR_OP_RBW << FIR_OP2_=
SHIFT));
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0out_be32(&lbc->fcr=
, NAND_CMD_READID << FCR_CMD0_SHIFT);
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* 5 bytes for manuf, =
device and exts */
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 out_be32(&lbc->fbcr, 5=
);
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 elbc_fcm_ctrl->read_by=
tes =3D 5;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* nand_get_flash_type=
() reads 8 bytes of entire ID string */
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 out_be32(&lbc->fbcr, 8=
);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 elbc_fcm_ctrl->read_by=
tes =3D 8;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0elbc_fcm_ctrl->use=
_mdr =3D 1;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0elbc_fcm_ctrl->mdr=
 =3D 0;
>
> --
> 1.6.4
>
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>

Shaohui,

I have tested this on p1011rdb, p1020rdb, and p2020rdb on 2.6.35.13
and 2.6.39.1.
Thank you for fixing this :-)

-Bryan

^ permalink raw reply

* Re: [PATCH v3 2/2] powerpc: add support for MPIC message register API
From: Scott Wood @ 2011-06-17 16:58 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Meador Inge, openmcapi-dev, devicetree-discuss, Hollis Blanchard,
	linuxppc-dev
In-Reply-To: <1308288784.32158.30.camel@pasglop>

On Fri, 17 Jun 2011 15:33:04 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Tue, 2011-05-31 at 14:19 -0500, Meador Inge wrote:
> > +void mpic_msgr_enable(struct mpic_msgr *msgr)
> > +{
> > +	out_be32(msgr->mer, in_be32(msgr->mer) | (1 << msgr->num));
> > +}
> > +EXPORT_SYMBOL(mpic_msgr_enable);
>=20
> Why are all those exported non-GPL ? We have a policy of making new
> in-kernel APIs generally GPL only.

=46rom Documentation/DocBook/kernel-hacking.tmpl:

  <sect1 id=3D"sym-exportsymbols-gpl">
   <title><function>EXPORT_SYMBOL_GPL()</function>  =20
    <filename class=3D"headerfile">include/linux/module.h</filename></title>
                                                    =20
   <para>                                           =20
    Similar to <function>EXPORT_SYMBOL()</function> except that the
    symbols exported by <function>EXPORT_SYMBOL_GPL()</function> can
    only be seen by modules with a                  =20
    <function>MODULE_LICENSE()</function> that specifies a GPL
    compatible license.  It implies that the function is considered
    an internal implementation issue, and not really an interface.
   </para>                                          =20
  </sect1>

When did this change from "considered an internal implementation issue, and
not really an interface" to "all new interfaces"?

-Scott

^ permalink raw reply

* Re: [PATCH] powerpc/book3e-64: use a separate TLB handler when linear map is bolted
From: Scott Wood @ 2011-06-17 16:32 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1308276050.2516.129.camel@pasglop>

On Fri, 17 Jun 2011 12:00:50 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> Does this completely replace your previous series of 7 patches ? (IE.
> Should I ditch them in patchwork ?) Or does it apply on top of them ?

It replaces them.

> >  #define SET_IVOR(vector_number, vector_offset)	\
> > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> > index a73668a..9d9e444 100644
> > --- a/arch/powerpc/include/asm/mmu_context.h
> > +++ b/arch/powerpc/include/asm/mmu_context.h
> > @@ -54,6 +54,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
> >  	/* 64-bit Book3E keeps track of current PGD in the PACA */
> >  #ifdef CONFIG_PPC_BOOK3E_64
> >  	get_paca()->pgd = next->pgd;
> > +	get_paca()->extlb[0][EX_TLB_PGD / 8] = (unsigned long)next->pgd;
> >  #endif
> >  	/* Nothing else to do if we aren't actually switching */
> >  	if (prev == next)
> > @@ -110,6 +111,7 @@ static inline void enter_lazy_tlb(struct mm_struct *mm,
> >  	/* 64-bit Book3E keeps track of current PGD in the PACA */
> >  #ifdef CONFIG_PPC_BOOK3E_64
> >  	get_paca()->pgd = NULL;
> > +	get_paca()->extlb[0][EX_TLB_PGD / 8] = 0;
> >  #endif
> >  }
> 
> Why do you keep a copy of the pgd there since it's in the PACA already
> and you have r13 setup in your handlers ?

This cache line is already being used by the TLB miss handler.  The cache
line containing paca->pgd isn't otherwise used there.

> > +.macro tlb_prolog_bolted addr
> > +	mtspr	SPRN_SPRG_TLB_SCRATCH,r13
> > +	mfspr	r13,SPRN_SPRG_PACA
> > +	std	r10,PACA_EXTLB+EX_TLB_R10(r13)
> > +	mfcr	r10
> > +	std	r11,PACA_EXTLB+EX_TLB_R11(r13)
> > +	mfspr	r11,SPRN_SPRG_TLB_SCRATCH
> 
> Do you need that ? Can't you leave r13 in scratch the whole way and
> just pop it out in the error case when branching to DSI/ISI ? The only
> thing is that TLB_SCRATCH needs to be saved/restored by
> crit/debug/mcheck but thats worth saving cycles in the TLB miss handler
> no ?

Sounds right... I'll try it.

> > +	mfspr	r11,SPRN_ESR
> > +
> > +	srdi	r15,r16,60		/* get region */
> > +	rldicl.	r10,r16,64-PGTABLE_EADDR_SIZE,PGTABLE_EADDR_SIZE+4
> > +	bne-	dtlb_miss_fault_bolted
> 
> Ok so I'm not familiar with your pipeline here, but wouldn't it be
> better to move the srdi to after the bne above and make it srdi., thus
> avoiding the compare below ?
> 
> > +	rlwinm	r10,r11,32-19,27,27
> > +	rlwimi	r10,r11,32-16,19,19
> > +	cmpwi	r15,0
> > +	ori	r10,r10,_PAGE_PRESENT
> > +	oris	r11,r10,_PAGE_ACCESSED@h

The cmpwi will run in parallel with the rlwimi above on e5500, but
changing it shouldn't hurt, and it's one less instruction to sit in the
icache...

-Scott

^ permalink raw reply

* Re: [PATCH 2/2] powerpc/book3e-64: reraise doorbell when masked by soft-irq-disable
From: Scott Wood @ 2011-06-17 16:02 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1308287520.32158.14.camel@pasglop>

On Fri, 17 Jun 2011 15:12:00 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Tue, 2011-05-24 at 06:51 +1000, Benjamin Herrenschmidt wrote:
> > On Mon, 2011-05-23 at 15:26 -0500, Scott Wood wrote:
> > > On Sat, 21 May 2011 08:32:58 +1000
> > > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> > > 
> > > > On Fri, 2011-05-20 at 14:00 -0500, Scott Wood wrote:
> > > > > Signed-off-by: Scott Wood <scottwood@freescale.com>
> > > > > ---
> > > > >  arch/powerpc/kernel/exceptions-64e.S |   22 +++++++++++++++++++++-
> > > > >  1 files changed, 21 insertions(+), 1 deletions(-)
> > > > 
> > > > You can probably remove the doorbell re-check when enabling interrupts
> > > > now, can't you ?
> > > 
> > > Ah, so that's how it currently gets away without re-raising when the
> > > interrupt happens. :-)
> > > 
> > > I'll remove it.
> > 
> > Yup, I was too lazy to make a special case in the exception handlers :-)
> 
> Are you going to send a re-spin ?

Ah, yes, forgot about this one. :-)

-Scott

^ permalink raw reply

* RE: Changes to of_device ?
From: David Laight @ 2011-06-17 14:23 UTC (permalink / raw)
  To: Grant Likely, Guillaume Dargaud; +Cc: linuxppc-dev
In-Reply-To: <BANLkTi=uDe3WevxtnW8ns3ixMET7tbHzeQ@mail.gmail.com>

=20
> For finding structures, the following pattern tends to work well:
>=20
> git grep 'struct device_node {'

In general 'typedef.*\<type_name\>' will find types.
eg: grep -r -n --include '*.h' 'typedef.*\<type_name\>'
base_of_source_tree

Finding functions (in .c files) is easy if names start in
column 1 '^function_name\>' will work.
But I ended up with: '^[^=3D"]*[^ =3D][^=3D"]*function_name\>' to find
some where the type was on the same line.

If you get really desparate, the definitions of functions can
be found by grepping through the namelists of the .o files.

	David

^ permalink raw reply

* Re: Changes to of_device ?
From: Grant Likely @ 2011-06-17 14:08 UTC (permalink / raw)
  To: Guillaume Dargaud; +Cc: linuxppc-dev
In-Reply-To: <201106171553.06443.dargaud@lpsc.in2p3.fr>

On Fri, Jun 17, 2011 at 7:53 AM, Guillaume Dargaud
<dargaud@lpsc.in2p3.fr> wrote:
>> I'm sorry, I should have mentioned that struct of_device was replaced by
>> struct platform_device. So in order to get your code compiled, you have
>> to change the probe function to
>>
>> static int xad_driver_probe(struct platform_device* dev, const struct
>> of_device_id *match) {
>
> Thanks, that worked...
>
> Side question: how do you guy find your way inside the kernel ? I spent hours trying to find the declarations of various
> basic structs like device, of_node, etc even with ack-grep and ctags (which I probably don't know how to use very well).

For finding structures, the following pattern tends to work well:

git grep 'struct device_node {'

I used to use cscope (make cscope in the kernel tree), but stopped
because the tags go out of date every time I change branches.

g.

^ permalink raw reply

* Re: Changes to of_device ?
From: Guillaume Dargaud @ 2011-06-17 13:53 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <4DFB516C.4060602@gate-nine.de>

> I'm sorry, I should have mentioned that struct of_device was replaced by
> struct platform_device. So in order to get your code compiled, you have
> to change the probe function to
> 
> static int xad_driver_probe(struct platform_device* dev, const struct
> of_device_id *match) {

Thanks, that worked...

Side question: how do you guy find your way inside the kernel ? I spent hours trying to find the declarations of various 
basic structs like device, of_node, etc even with ack-grep and ctags (which I probably don't know how to use very well).
-- 
Guillaume Dargaud
http://www.gdargaud.net/

^ permalink raw reply

* Re: [openmcapi-dev] Re: [PATCH v3 2/2] powerpc: add support for MPIC message register API
From: Meador Inge @ 2011-06-17 13:49 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: openmcapi-dev, Hollis Blanchard, devicetree-discuss, linuxppc-dev
In-Reply-To: <1308288784.32158.30.camel@pasglop>

On 06/17/2011 12:33 AM, Benjamin Herrenschmidt wrote:

> On Tue, 2011-05-31 at 14:19 -0500, Meador Inge wrote:
>> Some MPIC implementations contain one or more blocks of message registers
>> that are used to send messages between cores via IPIs.  A simple API has
>> been added to access (get/put, read, write, etc ...) these message registers.
>> The available message registers are initially discovered via nodes in the
>> device tree.  A separate commit contains a binding for the message register
>> nodes.
> 
> Ok, so I finally got to look at that in a bit more details...
>> +#ifndef _ASM_MPIC_MSGR_H
>> +#define _ASM_MPIC_MSGR_H
>> +
>> +#include <linux/types.h>
>> +
>> +struct mpic_msgr {
>> +	u32 __iomem *addr;
>> +	u32 __iomem *mer;
>> +	u32 __iomem	*msr;
>> +	int irq;
>> +	atomic_t in_use;
>> +	int num;
>> +};
> 
> General comment... I'm really not fan of "msgr", I'd rather see
> "mpic_message_*", it's a tad more verbose but looks a lot better, no ?

It reads a little better.  I got 'msgr' from the HW manual.  Also, 'msgr'
is used in the device tree and file names too.  So I would like to use
'msgr' or 'message' everywhere.  I am OK with 'msgr'.

> Also do you need those 3 iomem pointers ? Not just one with fixed
> offsets ? Or do they come from vastly different sources ?

I can drop the 'msr' field as it is not used anymore.  I wouldn't say the
others are vastly different, but they are not right next to each other.  On the
MPC8572DS, for example, there is one MER for every message register block.  The
first block starts at offset 0x1400 (MSGR1), 0x1410 (MSGR2), 0x1420 (MSGR2),
0x1430 (MSGR3).  So you could do the math to get the MER from the MSGR.

> atomic_t in_use looks fishy, but let's see how you use it...
> 
>> +extern struct mpic_msgr* mpic_msgr_get(unsigned int reg_num);
>> +extern void mpic_msgr_put(struct mpic_msgr* msgr);
>> +extern void mpic_msgr_enable(struct mpic_msgr *msgr);
>> +extern void mpic_msgr_disable(struct mpic_msgr *msgr);
>> +extern void mpic_msgr_write(struct mpic_msgr *msgr, u32 message);
>> +extern u32 mpic_msgr_read(struct mpic_msgr *msgr);
>> +extern void mpic_msgr_clear(struct mpic_msgr *msgr);
>> +extern void mpic_msgr_set_destination(struct mpic_msgr *msgr, u32 cpu_num);
>> +extern int mpic_msgr_get_irq(struct mpic_msgr *msgr);
> 
> Documentation of the API please.

Will do.

>> +#endif
>> diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
>> index f7b0772..4d65593 100644
>> --- a/arch/powerpc/platforms/Kconfig
>> +++ b/arch/powerpc/platforms/Kconfig
>> @@ -78,6 +78,14 @@ config MPIC_WEIRD
>>  	bool
>>  	default n
>>  
>> +config MPIC_MSGR
>> +	bool "MPIC message register support"
>> +	depends on MPIC
>> +	default n
>> +	help
>> +	  Enables support for the MPIC message registers.  These
>> +	  registers are used for inter-processor communication.
>> +
>>  config PPC_I8259
>>  	bool
>>  	default n
>> diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
>> index 1e0c933..6d40185 100644
>> --- a/arch/powerpc/sysdev/Makefile
>> +++ b/arch/powerpc/sysdev/Makefile
>> @@ -3,7 +3,8 @@ subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
>>  ccflags-$(CONFIG_PPC64)		:= -mno-minimal-toc
>>  
>>  mpic-msi-obj-$(CONFIG_PCI_MSI)	+= mpic_msi.o mpic_u3msi.o mpic_pasemi_msi.o
>> -obj-$(CONFIG_MPIC)		+= mpic.o $(mpic-msi-obj-y)
>> +mpic-msgr-obj-$(CONFIG_MPIC_MSGR)	+= mpic_msgr.o
>> +obj-$(CONFIG_MPIC)		+= mpic.o $(mpic-msi-obj-y) $(mpic-msgr-obj-y)
>>  fsl-msi-obj-$(CONFIG_PCI_MSI)	+= fsl_msi.o
>>  obj-$(CONFIG_PPC_MSI_BITMAP)	+= msi_bitmap.o
>>  
>> diff --git a/arch/powerpc/sysdev/mpic_msgr.c b/arch/powerpc/sysdev/mpic_msgr.c
>> new file mode 100644
>> index 0000000..bfa0612
>> --- /dev/null
>> +++ b/arch/powerpc/sysdev/mpic_msgr.c
>> @@ -0,0 +1,279 @@
>> +/*
>> + * Copyright 2011-2012, Meador Inge, Mentor Graphics Corporation.
>> + *
>> + * Some ideas based on un-pushed work done by Vivek Mahajan, Jason Jin, and
>> + * Mingkai Hu from Freescale Semiconductor, Inc.
>> + *
>> + * 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; version 2 of the
>> + * License.
>> + *
>> + */
>> +
>> +#include <linux/list.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/errno.h>
>> +#include <asm/prom.h>
>> +#include <asm/hw_irq.h>
>> +#include <asm/ppc-pci.h>
>> +#include <asm/mpic_msgr.h>
>> +
>> +#define MPIC_MSGR_REGISTERS_PER_BLOCK 4
>> +#define MSGR_INUSE 0
>> +#define MSGR_FREE 1
>> +
>> +/* Internal structure used *only* for IO mapping register blocks. */
>> +struct mpic_msgr_block {
>> +	struct msgr {
>> +		u32 msgr;
>> +		u8 res[12];
>> +	} msgrs[MPIC_MSGR_REGISTERS_PER_BLOCK];
>> +	u8 res0[192];
>> +	u32 mer;
>> +	u8 res1[12];
>> +	u32 msr;
>> +};
> 
> So this represent HW registers ? Please make it clear in the comment.
> I'm not a terrible fan of using structures to map HW especially with so
> few registers.

Yes, HW registers.  I will change the comment.

>> +	if (atomic_cmpxchg(&msgr->in_use, MSGR_FREE, MSGR_INUSE) == MSGR_FREE)
>> +		return msgr;
>> +
>> +	return ERR_PTR(-EBUSY);
>> +}
>> +EXPORT_SYMBOL(mpic_msgr_get);
> 
> So how are those things intended to be used ? Clients get a fixed
> "register" number to use ? It looks like this stuff would have been
> better off using some kind of allocator no ? A bitmap would have done
> the job... or do you really need to have some kind of fixed numbering
> associated with clients ?

A fixed numbering scheme really is needed.  We are using these registers
to communicate b/w different kernels in an AMP system.  In order
to do this each kernel has to be using the same numbering so that they
can communicate.

>> +void mpic_msgr_put(struct mpic_msgr* msgr)
>> +{
>> +	atomic_set(&msgr->in_use, MSGR_FREE);
>> +}
>> +EXPORT_SYMBOL(mpic_msgr_put);
> 
> Shouldn't it be disabled too while at it ?

Yes.

>> +void mpic_msgr_enable(struct mpic_msgr *msgr)
>> +{
>> +	out_be32(msgr->mer, in_be32(msgr->mer) | (1 << msgr->num));
>> +}
>> +EXPORT_SYMBOL(mpic_msgr_enable);
> 
> Why are all those exported non-GPL ? We have a policy of making new
> in-kernel APIs generally GPL only.

Oversight on my part, I will fix it.

>> +void mpic_msgr_disable(struct mpic_msgr *msgr)
>> +{
>> +	out_be32(msgr->mer, in_be32(msgr->mer) & ~(1 << msgr->num));
>> +}
>> +EXPORT_SYMBOL(mpic_msgr_disable);
> 
> I see read-modify-write cycles here with no synchronization/locking
> whatsoever... Might be ok as long as you document it in the doc of the
> API, ie, the caller is required to synchronize.

I will add locking.

>> +void mpic_msgr_write(struct mpic_msgr *msgr, u32 message)
>> +{
>> +	out_be32(msgr->addr, message);
>> +}
>> +EXPORT_SYMBOL(mpic_msgr_write);
>> +
>> +u32 mpic_msgr_read(struct mpic_msgr *msgr)
>> +{
>> +	return in_be32(msgr->addr);
>> +}
>> +EXPORT_SYMBOL(mpic_msgr_read);
> 
> Those look like significant bloat/overhead for what is just an MMIO
> wrapper on a register access, why not make them static inline's
> instead ?

I will make them 'static inline'.

>> +int mpic_msgr_get_irq(struct mpic_msgr *msgr)
>> +{
>> +	return msgr->irq;
>> +}
>> +EXPORT_SYMBOL(mpic_msgr_get_irq);
>> +
>> +/* The following three functions are used to compute the order and number of
>> + * the message register blocks.  They are clearly very inefficent.  However,
>> + * they are called *only* a few times during device initialization.
>> + */
>> +static unsigned int mpic_msgr_number_of_blocks(void)
>> +{
>> +	unsigned int count;
>> +	struct device_node *aliases;
>> +
>> +	count = 0;
>> +	aliases = of_find_node_by_name(NULL, "aliases");
>> +
>> +	if (aliases) {
>> +		char buf[32];
>> +
>> +		for (;;) {
>> +			snprintf(buf, sizeof(buf), "mpic-msgr-block%d", count);
>> +			if (!of_find_property(aliases, buf, NULL))
>> +				break;
>> +
>> +			count += 1;
>> +		}
>> +	}
>> +
>> +	return count;
>> +}
> 
> I don't like relying on aliases as a way to figure out what HW is
> present, that's not right. You should find the actual devices themselves
> based on their compatible properties.

In this case, the device is a block of message registers.  These are found by
probing the compatible properties.  However, once a _block_ is found the
register within it are numbered using the found aliases.  This is
due to the need for a fixed number scheme as mentioned previously.

Why a fixed numbering scheme is needed is always this first question about this
patch.  So, I should add a comment explaining all of this in the implementation.

> Ok, so we have another scheme of:
> 
>  - Count all devices in the system of a given type
>  - Assign them numbers
>  - API uses number
> 
> That sucks... unless you have an allocator. And even then.
> 
> I'd rather clients use something like struct mpic_msgr (or msg_reg or
> message_reg) as the "handle" to one of these things.

Most of the APIs *do* use a 'mpic_msgr' handle.  The only one that doesn't
is 'mpic_msgr_get', which takes the register number to retrieve.  We discussed
in the beginning having an allocator, but it didn't fit well with the AMP use
case we are working on.  There was a recent discussion on how a dynamic
allocator could be added in addition to the static one in PATCH v0:
http://patchwork.ozlabs.org/patch/92037/.

Thanks a lot for the review Ben.  Much appreciated.

-- 
Meador Inge
CodeSourcery / Mentor Embedded
http://www.mentor.com/embedded-software

^ permalink raw reply

* [PATCH 1/2] ppc: enable scom access functions on Maple
From: Dmitry Eremin-Solenikov @ 2011-06-17 13:10 UTC (permalink / raw)
  To: linuxppc-dev, cpufreq; +Cc: Paul Mackerras, Dave Jones

Enable functions used to access SCOM if PPC_MAPLE is defined: they are
used by cpufreq driver to control hardware.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 arch/powerpc/kernel/misc_64.S |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
index e89df59..616921e 100644
--- a/arch/powerpc/kernel/misc_64.S
+++ b/arch/powerpc/kernel/misc_64.S
@@ -339,7 +339,7 @@ _GLOBAL(real_205_writeb)
 #endif /* CONFIG_PPC_PASEMI */
 
 
-#ifdef CONFIG_CPU_FREQ_PMAC64
+#if defined(CONFIG_CPU_FREQ_PMAC64) || defined(CONFIG_CPU_FREQ_MAPLE)
 /*
  * SCOM access functions for 970 (FX only for now)
  *
@@ -408,7 +408,7 @@ _GLOBAL(scom970_write)
 	/* restore interrupts */
 	mtmsrd	r5,1
 	blr
-#endif /* CONFIG_CPU_FREQ_PMAC64 */
+#endif /* CONFIG_CPU_FREQ_PMAC64 || CONFIG_CPU_FREQ_MAPLE */
 
 
 /*
-- 
1.7.5.3

^ permalink raw reply related

* [PATCH 2/2] Add cpufreq driver for Momentum Maple boards
From: Dmitry Eremin-Solenikov @ 2011-06-17 13:10 UTC (permalink / raw)
  To: linuxppc-dev, cpufreq; +Cc: Paul Mackerras, Dave Jones
In-Reply-To: <1308316207-9075-1-git-send-email-dbaryshkov@gmail.com>

Add simple cpufreq driver for Maple-based boards (ppc970fx evaluation
kit and others). Driver is based on a cpufreq driver for 64-bit powermac
boxes with all pmac-dependant features removed and simple cleanup
applied.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/cpufreq/Kconfig         |    5 +
 drivers/cpufreq/Kconfig.powerpc |    7 +
 drivers/cpufreq/Makefile        |    5 +
 drivers/cpufreq/maple-cpufreq.c |  314 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 331 insertions(+), 0 deletions(-)
 create mode 100644 drivers/cpufreq/Kconfig.powerpc
 create mode 100644 drivers/cpufreq/maple-cpufreq.c

diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index 9fb8485..61ae639 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -184,5 +184,10 @@ depends on X86
 source "drivers/cpufreq/Kconfig.x86"
 endmenu
 
+menu "PowerPC CPU frequency scaling drivers"
+depends on PPC32 || PPC64
+source "drivers/cpufreq/Kconfig.powerpc"
+endmenu
+
 endif
 endmenu
diff --git a/drivers/cpufreq/Kconfig.powerpc b/drivers/cpufreq/Kconfig.powerpc
new file mode 100644
index 0000000..e76992f
--- /dev/null
+++ b/drivers/cpufreq/Kconfig.powerpc
@@ -0,0 +1,7 @@
+config CPU_FREQ_MAPLE
+	bool "Support for Maple 970FX Evaluation Board"
+	depends on PPC_MAPLE
+	select CPU_FREQ_TABLE
+	help
+	  This adds support for frequency switching on Maple 970FX
+	  Evaluation Board and compatible boards (IBM JS2x blades).
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index e2fc2d2..ca3796d 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -41,3 +41,8 @@ obj-$(CONFIG_X86_CPUFREQ_NFORCE2)	+= cpufreq-nforce2.o
 
 # ARM SoC drivers
 obj-$(CONFIG_UX500_SOC_DB8500)		+= db8500-cpufreq.o
+
+
+##################################################################################d
+# PowerPC platform drivers
+obj-$(CONFIG_CPU_FREQ_MAPLE)		+= maple-cpufreq.o
diff --git a/drivers/cpufreq/maple-cpufreq.c b/drivers/cpufreq/maple-cpufreq.c
new file mode 100644
index 0000000..a61f6b9
--- /dev/null
+++ b/drivers/cpufreq/maple-cpufreq.c
@@ -0,0 +1,314 @@
+/*
+ *  Copyright (C) 2011 Dmitry Eremin-Solenikov
+ *  Copyright (C) 2002 - 2005 Benjamin Herrenschmidt <benh@kernel.crashing.org>
+ *  and                       Markus Demleitner <msdemlei@cl.uni-heidelberg.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This driver adds basic cpufreq support for SMU & 970FX based G5 Macs,
+ * that is iMac G5 and latest single CPU desktop.
+ */
+
+#undef DEBUG
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/delay.h>
+#include <linux/sched.h>
+#include <linux/cpufreq.h>
+#include <linux/init.h>
+#include <linux/completion.h>
+#include <linux/mutex.h>
+#include <linux/time.h>
+#include <linux/of.h>
+
+#define DBG(fmt...) pr_debug(fmt)
+
+/* see 970FX user manual */
+
+#define SCOM_PCR 0x0aa001			/* PCR scom addr */
+
+#define PCR_HILO_SELECT		0x80000000U	/* 1 = PCR, 0 = PCRH */
+#define PCR_SPEED_FULL		0x00000000U	/* 1:1 speed value */
+#define PCR_SPEED_HALF		0x00020000U	/* 1:2 speed value */
+#define PCR_SPEED_QUARTER	0x00040000U	/* 1:4 speed value */
+#define PCR_SPEED_MASK		0x000e0000U	/* speed mask */
+#define PCR_SPEED_SHIFT		17
+#define PCR_FREQ_REQ_VALID	0x00010000U	/* freq request valid */
+#define PCR_VOLT_REQ_VALID	0x00008000U	/* volt request valid */
+#define PCR_TARGET_TIME_MASK	0x00006000U	/* target time */
+#define PCR_STATLAT_MASK	0x00001f00U	/* STATLAT value */
+#define PCR_SNOOPLAT_MASK	0x000000f0U	/* SNOOPLAT value */
+#define PCR_SNOOPACC_MASK	0x0000000fU	/* SNOOPACC value */
+
+#define SCOM_PSR 0x408001			/* PSR scom addr */
+/* warning: PSR is a 64 bits register */
+#define PSR_CMD_RECEIVED	0x2000000000000000U   /* command received */
+#define PSR_CMD_COMPLETED	0x1000000000000000U   /* command completed */
+#define PSR_CUR_SPEED_MASK	0x0300000000000000U   /* current speed */
+#define PSR_CUR_SPEED_SHIFT	(56)
+
+/*
+ * The G5 only supports two frequencies (Quarter speed is not supported)
+ */
+#define CPUFREQ_HIGH                  0
+#define CPUFREQ_LOW                   1
+
+static struct cpufreq_frequency_table maple_cpu_freqs[] = {
+	{CPUFREQ_HIGH,		0},
+	{CPUFREQ_LOW,		0},
+	{0,			CPUFREQ_TABLE_END},
+};
+
+static struct freq_attr *maple_cpu_freqs_attr[] = {
+	&cpufreq_freq_attr_scaling_available_freqs,
+	NULL,
+};
+
+/* Power mode data is an array of the 32 bits PCR values to use for
+ * the various frequencies, retrieved from the device-tree
+ */
+static int maple_pmode_cur;
+
+static DEFINE_MUTEX(maple_switch_mutex);
+
+static const u32 *maple_pmode_data;
+static int maple_pmode_max;
+
+/*
+ * Fake voltage switching for platforms with missing support
+ */
+
+static void maple_dummy_switch_volt(int speed_mode)
+{
+}
+
+/*
+ * SCOM based frequency switching for 970FX rev3
+ */
+static int maple_scom_switch_freq(int speed_mode)
+{
+	unsigned long flags;
+	int to;
+
+	/* If frequency is going up, first ramp up the voltage */
+	if (speed_mode < maple_pmode_cur)
+		maple_dummy_switch_volt(speed_mode);
+
+	local_irq_save(flags);
+
+	/* Clear PCR high */
+	scom970_write(SCOM_PCR, 0);
+	/* Clear PCR low */
+	scom970_write(SCOM_PCR, PCR_HILO_SELECT | 0);
+	/* Set PCR low */
+	scom970_write(SCOM_PCR, PCR_HILO_SELECT |
+		      maple_pmode_data[speed_mode]);
+
+	/* Wait for completion */
+	for (to = 0; to < 10; to++) {
+		unsigned long psr = scom970_read(SCOM_PSR);
+
+		if ((psr & PSR_CMD_RECEIVED) == 0 &&
+		    (((psr >> PSR_CUR_SPEED_SHIFT) ^
+		      (maple_pmode_data[speed_mode] >> PCR_SPEED_SHIFT)) & 0x3)
+		    == 0)
+			break;
+		if (psr & PSR_CMD_COMPLETED)
+			break;
+		udelay(100);
+	}
+
+	local_irq_restore(flags);
+
+	/* If frequency is going down, last ramp the voltage */
+	if (speed_mode > maple_pmode_cur)
+		maple_dummy_switch_volt(speed_mode);
+
+	maple_pmode_cur = speed_mode;
+	ppc_proc_freq = maple_cpu_freqs[speed_mode].frequency * 1000ul;
+
+	return 0;
+}
+
+static int maple_scom_query_freq(void)
+{
+	unsigned long psr = scom970_read(SCOM_PSR);
+	int i;
+
+	for (i = 0; i <= maple_pmode_max; i++)
+		if ((((psr >> PSR_CUR_SPEED_SHIFT) ^
+		      (maple_pmode_data[i] >> PCR_SPEED_SHIFT)) & 0x3) == 0)
+			break;
+	return i;
+}
+
+/*
+ * Common interface to the cpufreq core
+ */
+
+static int maple_cpufreq_verify(struct cpufreq_policy *policy)
+{
+	return cpufreq_frequency_table_verify(policy, maple_cpu_freqs);
+}
+
+static int maple_cpufreq_target(struct cpufreq_policy *policy,
+	unsigned int target_freq, unsigned int relation)
+{
+	unsigned int newstate = 0;
+	struct cpufreq_freqs freqs;
+	int rc;
+
+	if (cpufreq_frequency_table_target(policy, maple_cpu_freqs,
+			target_freq, relation, &newstate))
+		return -EINVAL;
+
+	if (maple_pmode_cur == newstate)
+		return 0;
+
+	mutex_lock(&maple_switch_mutex);
+
+	freqs.old = maple_cpu_freqs[maple_pmode_cur].frequency;
+	freqs.new = maple_cpu_freqs[newstate].frequency;
+	freqs.cpu = 0;
+
+	cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+	rc = maple_scom_switch_freq(newstate);
+	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+
+	mutex_unlock(&maple_switch_mutex);
+
+	return rc;
+}
+
+static unsigned int maple_cpufreq_get_speed(unsigned int cpu)
+{
+	return maple_cpu_freqs[maple_pmode_cur].frequency;
+}
+
+static int maple_cpufreq_cpu_init(struct cpufreq_policy *policy)
+{
+	policy->cpuinfo.transition_latency = 12000;
+	policy->cur = maple_cpu_freqs[maple_scom_query_freq()].frequency;
+	/* secondary CPUs are tied to the primary one by the
+	 * cpufreq core if in the secondary policy we tell it that
+	 * it actually must be one policy together with all others. */
+	cpumask_copy(policy->cpus, cpu_online_mask);
+	cpufreq_frequency_table_get_attr(maple_cpu_freqs, policy->cpu);
+
+	return cpufreq_frequency_table_cpuinfo(policy,
+		maple_cpu_freqs);
+}
+
+
+static struct cpufreq_driver maple_cpufreq_driver = {
+	.name		= "maple",
+	.owner		= THIS_MODULE,
+	.flags		= CPUFREQ_CONST_LOOPS,
+	.init		= maple_cpufreq_cpu_init,
+	.verify		= maple_cpufreq_verify,
+	.target		= maple_cpufreq_target,
+	.get		= maple_cpufreq_get_speed,
+	.attr		= maple_cpu_freqs_attr,
+};
+
+static int __init maple_cpufreq_init(void)
+{
+	struct device_node *cpus;
+	struct device_node *cpunode;
+	unsigned int psize;
+	unsigned long max_freq;
+	const u32 *valp;
+	u32 pvr_hi;
+	int rc = -ENODEV;
+
+	cpus = of_find_node_by_path("/cpus");
+	if (cpus == NULL) {
+		DBG("No /cpus node !\n");
+		return -ENODEV;
+	}
+
+	/* Get first CPU node */
+	for (cpunode = NULL;
+	     (cpunode = of_get_next_child(cpus, cpunode)) != NULL;) {
+		const u32 *reg = of_get_property(cpunode, "reg", NULL);
+		if (reg == NULL || (*reg) != 0)
+			continue;
+		if (!strcmp(cpunode->type, "cpu"))
+			break;
+	}
+	if (cpunode == NULL) {
+		printk(KERN_ERR "cpufreq: Can't find any CPU 0 node\n");
+		goto bail_cpus;
+	}
+
+	/* Check 970FX for now */
+	/* we actually don't care on which CPU to access PVR */
+	pvr_hi = PVR_VER(mfspr(SPRN_PVR));
+	if (pvr_hi != 0x3c && pvr_hi != 0x44) {
+		printk(KERN_ERR "cpufreq: Unsupported CPU version (%x)\n",
+				pvr_hi);
+		goto bail_noprops;
+	}
+
+	/* Look for the powertune data in the device-tree */
+	maple_pmode_data = of_get_property(cpunode, "power-mode-data", &psize);
+	if (!maple_pmode_data) {
+		DBG("No power-mode-data !\n");
+		goto bail_noprops;
+	}
+	maple_pmode_max = psize / sizeof(u32) - 1;
+
+	/*
+	 * From what I see, clock-frequency is always the maximal frequency.
+	 * The current driver can not slew sysclk yet, so we really only deal
+	 * with powertune steps for now. We also only implement full freq and
+	 * half freq in this version. So far, I haven't yet seen a machine
+	 * supporting anything else.
+	 */
+	valp = of_get_property(cpunode, "clock-frequency", NULL);
+	if (!valp)
+		return -ENODEV;
+	max_freq = (*valp)/1000;
+	maple_cpu_freqs[0].frequency = max_freq;
+	maple_cpu_freqs[1].frequency = max_freq/2;
+
+	/* Force apply current frequency to make sure everything is in
+	 * sync (voltage is right for example). Firmware may leave us with
+	 * a strange setting ...
+	 */
+	maple_dummy_switch_volt(CPUFREQ_HIGH);
+	msleep(10);
+	maple_pmode_cur = -1;
+	maple_scom_switch_freq(maple_scom_query_freq());
+
+	printk(KERN_INFO "Registering G5 CPU frequency driver\n");
+	printk(KERN_INFO "Frequency method: SCOM, Voltage method: none\n");
+	printk(KERN_INFO "Low: %d Mhz, High: %d Mhz, Cur: %d MHz\n",
+		maple_cpu_freqs[1].frequency/1000,
+		maple_cpu_freqs[0].frequency/1000,
+		maple_cpu_freqs[maple_pmode_cur].frequency/1000);
+
+	rc = cpufreq_register_driver(&maple_cpufreq_driver);
+
+	of_node_put(cpunode);
+	of_node_put(cpus);
+
+	return rc;
+
+bail_noprops:
+	of_node_put(cpunode);
+bail_cpus:
+	of_node_put(cpus);
+
+	return rc;
+}
+
+module_init(maple_cpufreq_init);
+
+
+MODULE_LICENSE("GPL");
-- 
1.7.5.3

^ permalink raw reply related

* Re: Changes to of_device ?
From: Philipp Ittershagen @ 2011-06-17 13:06 UTC (permalink / raw)
  To: Guillaume Dargaud; +Cc: linuxppc-dev
In-Reply-To: <4DFB4F51.1090509@gate-nine.de>

On 06/17/2011 02:57 PM, Philipp Ittershagen wrote:
> The struct device_node was moved to struct device in order to make the
> CONFIG_OF more generic. See
> 
> commit d706c1b050274b3bf97d7cb0542c0d070c9ccb8b
> Author: Grant Likely <grant.likely@secretlab.ca>
> Date:   Tue Apr 13 16:12:28 2010 -0700
> 
>     driver-core: Add device node pointer to struct device
> 

I'm sorry, I should have mentioned that struct of_device was replaced by
struct platform_device. So in order to get your code compiled, you have
to change the probe function to

static int xad_driver_probe(struct platform_device* dev, const struct
of_device_id *match) {

The change was introduced by

commit 2dc11581376829303b98eadb2de253bee065a56a
Author: Grant Likely <grant.likely@secretlab.ca>
Date:   Fri Aug 6 09:25:50 2010 -0600

    of/device: Replace struct of_device with struct platform_device


Greetings,

  Philipp

^ permalink raw reply

* Re: Changes to of_device ?
From: Philipp Ittershagen @ 2011-06-17 12:57 UTC (permalink / raw)
  To: Guillaume Dargaud; +Cc: linuxppc-dev
In-Reply-To: <201106171322.19067.dargaud@lpsc.in2p3.fr>

On 06/17/2011 01:22 PM, Guillaume Dargaud wrote:
>> Not answering your primary question here, but in order to print the most
>> recent tag (along with a uniqe suffix, see man page) that is reachable
>> from your current branch, you can use "git describe". It will print out
>> the kernel version on your latest Xilinx tree.
> 
> $ git describe
> v2.6.37-719-gecf08a4
> 
> $ cat ./include/linux/version.h
> #define LINUX_VERSION_CODE 132645
> #define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))
> 
> $ printf "%x\n" 132645
> 20625
> Something doesn't quite match here... 

It is hexadecimal. 0x25 = 37.

> But my original question about how do you declare a probe function in
> the latest kernels still stand...

The struct device_node was moved to struct device in order to make the
CONFIG_OF more generic. See

commit d706c1b050274b3bf97d7cb0542c0d070c9ccb8b
Author: Grant Likely <grant.likely@secretlab.ca>
Date:   Tue Apr 13 16:12:28 2010 -0700

    driver-core: Add device node pointer to struct device


Hope this helps,

  Philipp

^ permalink raw reply

* [PATCH V3 2/2] cpc925_edac: support single-processor configurations
From: Dmitry Eremin-Solenikov @ 2011-06-17 12:51 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Harry Ciao, Paul Mackerras, Doug Thompson
In-Reply-To: <1308315107-29182-1-git-send-email-dbaryshkov@gmail.com>

If second CPU is not enabled, CPC925 EDAC driver will spill out warnings
about errors on second Processor Interface. Support masking that out,
by detecting at runtime which CPUs are present in device tree.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Cc: Harry Ciao <qingtao.cao@windriver.com>
Cc: Doug Thompson <dougthompson@xmission.com>
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/edac/cpc925_edac.c |   67 ++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/drivers/edac/cpc925_edac.c b/drivers/edac/cpc925_edac.c
index a687a0d..a774c0d 100644
--- a/drivers/edac/cpc925_edac.c
+++ b/drivers/edac/cpc925_edac.c
@@ -90,6 +90,7 @@ enum apimask_bits {
 	ECC_MASK_ENABLE = (APIMASK_ECC_UE_H | APIMASK_ECC_CE_H |
 			   APIMASK_ECC_UE_L | APIMASK_ECC_CE_L),
 };
+#define APIMASK_ADI(n)		CPC925_BIT(((n)+1))
 
 /************************************************************
  *	Processor Interface Exception Register (APIEXCP)
@@ -581,16 +582,73 @@ static void cpc925_mc_check(struct mem_ctl_info *mci)
 }
 
 /******************** CPU err device********************************/
+static u32 cpc925_cpu_mask_disabled(void)
+{
+	struct device_node *cpus;
+	struct device_node *cpunode = NULL;
+	static u32 mask = 0;
+
+	/* use cached value if available */
+	if (mask != 0)
+		return mask;
+
+	mask = APIMASK_ADI0 | APIMASK_ADI1;
+
+	cpus = of_find_node_by_path("/cpus");
+	if (cpus == NULL) {
+		cpc925_printk(KERN_DEBUG, "No /cpus node !\n");
+		return 0;
+	}
+
+	while ((cpunode = of_get_next_child(cpus, cpunode)) != NULL) {
+		const u32 *reg = of_get_property(cpunode, "reg", NULL);
+
+		if (strcmp(cpunode->type, "cpu")) {
+			cpc925_printk(KERN_ERR, "Not a cpu node in /cpus: %s\n", cpunode->name);
+			continue;
+		}
+
+		if (reg == NULL || *reg > 2) {
+			cpc925_printk(KERN_ERR, "Bad reg value at %s\n", cpunode->full_name);
+			continue;
+		}
+
+		mask &= ~APIMASK_ADI(*reg);
+	}
+
+	if (mask != (APIMASK_ADI0 | APIMASK_ADI1)) {
+		/* We assume that each CPU sits on it's own PI and that
+		 * for present CPUs the reg property equals to the PI
+		 * interface id */
+		cpc925_printk(KERN_WARNING,
+				"Assuming PI id is equal to CPU MPIC id!\n");
+	}
+
+	of_node_put(cpunode);
+	of_node_put(cpus);
+
+	return mask;
+}
+
 /* Enable CPU Errors detection */
 static void cpc925_cpu_init(struct cpc925_dev_info *dev_info)
 {
 	u32 apimask;
+	u32 cpumask;
 
 	apimask = __raw_readl(dev_info->vbase + REG_APIMASK_OFFSET);
-	if ((apimask & CPU_MASK_ENABLE) == 0) {
-		apimask |= CPU_MASK_ENABLE;
-		__raw_writel(apimask, dev_info->vbase + REG_APIMASK_OFFSET);
+
+	cpumask = cpc925_cpu_mask_disabled();
+	if (apimask & cpumask) {
+		cpc925_printk(KERN_WARNING, "CPU(s) not present, "
+				"but enabled in APIMASK, disabling\n");
+		apimask &= ~cpumask;
 	}
+
+	if ((apimask & CPU_MASK_ENABLE) == 0)
+		apimask |= CPU_MASK_ENABLE;
+
+	__raw_writel(apimask, dev_info->vbase + REG_APIMASK_OFFSET);
 }
 
 /* Disable CPU Errors detection */
@@ -622,6 +680,9 @@ static void cpc925_cpu_check(struct edac_device_ctl_info *edac_dev)
 	if ((apiexcp & CPU_EXCP_DETECTED) == 0)
 		return;
 
+	if ((apiexcp & ~cpc925_cpu_mask_disabled()) == 0)
+		return;
+
 	apimask = __raw_readl(dev_info->vbase + REG_APIMASK_OFFSET);
 	cpc925_printk(KERN_INFO, "Processor Interface Fault\n"
 				 "Processor Interface register dump:\n");
-- 
1.7.5.3

^ permalink raw reply related

* [PATCH V3 1/2] Maple: register CPC925 EDAC device on all boards with CPC925
From: Dmitry Eremin-Solenikov @ 2011-06-17 12:51 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras
In-Reply-To: <1308315107-29182-1-git-send-email-dbaryshkov@gmail.com>

Currently Maple setup code creates cpc925_edac device only on
Motorola ATCA-6101 blade. Make setup code check bridge revision
and enable EDAC on all U3H bridges.

Verified on Momentum MapleD (ppc970fx kit) board.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 arch/powerpc/platforms/maple/setup.c |   41 +++++++++++++++------------------
 1 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/platforms/maple/setup.c b/arch/powerpc/platforms/maple/setup.c
index fe34c3d..5b3388b 100644
--- a/arch/powerpc/platforms/maple/setup.c
+++ b/arch/powerpc/platforms/maple/setup.c
@@ -338,35 +338,16 @@ define_machine(maple) {
 #ifdef CONFIG_EDAC
 /*
  * Register a platform device for CPC925 memory controller on
- * Motorola ATCA-6101 blade.
+ * all boards with U3H (CPC925) bridge.
  */
-#define MAPLE_CPC925_MODEL	"Motorola,ATCA-6101"
 static int __init maple_cpc925_edac_setup(void)
 {
 	struct platform_device *pdev;
 	struct device_node *np = NULL;
 	struct resource r;
-	const unsigned char *model;
 	int ret;
-
-	np = of_find_node_by_path("/");
-	if (!np) {
-		printk(KERN_ERR "%s: Unable to get root node\n", __func__);
-		return -ENODEV;
-	}
-
-	model = (const unsigned char *)of_get_property(np, "model", NULL);
-	if (!model) {
-		printk(KERN_ERR "%s: Unabel to get model info\n", __func__);
-		of_node_put(np);
-		return -ENODEV;
-	}
-
-	ret = strcmp(model, MAPLE_CPC925_MODEL);
-	of_node_put(np);
-
-	if (ret != 0)
-		return 0;
+	volatile void __iomem *mem;
+	u32 rev;
 
 	np = of_find_node_by_type(NULL, "memory-controller");
 	if (!np) {
@@ -384,6 +365,22 @@ static int __init maple_cpc925_edac_setup(void)
 		return -ENODEV;
 	}
 
+	mem = ioremap(r.start, resource_size(&r));
+	if (!mem) {
+		printk(KERN_ERR "%s: Unable to map memory-controller memory\n",
+				__func__);
+		return -ENOMEM;
+	}
+
+	rev = __raw_readl(mem);
+	iounmap(mem);
+
+	if (rev < 0x34 || rev > 0x3f) { /* U3H */
+		printk(KERN_ERR "%s: Non-CPC925(U3H) bridge revision: %02x\n",
+			__func__, rev);
+		return 0;
+	}
+
 	pdev = platform_device_register_simple("cpc925_edac", 0, &r, 1);
 	if (IS_ERR(pdev))
 		return PTR_ERR(pdev);
-- 
1.7.5.3

^ permalink raw reply related

* [PATCH V3 0/2] Improve CPC925 EDAC handling code on Maple
From: Dmitry Eremin-Solenikov @ 2011-06-17 12:51 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras

Hello, colleagues,

Another respin of this patchset.

Changes since previous version:
* Fix comments in cpc925_edac.c
* Rename cpc925_cpu_getmask() to cpc925_cpu_mask_disabled()
* Don't return an error from init code if it's a U3 and not U3H

 arch/powerpc/platforms/maple/setup.c |   41 +++++++++------------
 drivers/edac/cpc925_edac.c           |   67 +++++++++++++++++++++++++++++++++--
 2 files changed, 83 insertions(+), 25 deletions(-)

-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: Changes to of_device ?
From: Guillaume Dargaud @ 2011-06-17 11:22 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <4DFB329A.7010104@gate-nine.de>

> Not answering your primary question here, but in order to print the most
> recent tag (along with a uniqe suffix, see man page) that is reachable
> from your current branch, you can use "git describe". It will print out
> the kernel version on your latest Xilinx tree.

$ git describe
v2.6.37-719-gecf08a4

$ cat ./include/linux/version.h
#define LINUX_VERSION_CODE 132645
#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))

$ printf "%x\n" 132645
20625

Something doesn't quite match here... But my original question about how do you declare a probe function in the latest 
kernels still stand...
-- 
Guillaume Dargaud
http://www.gdargaud.net/

^ permalink raw reply

* Re: [PATCH] Add cpufreq driver for Momentum Maple boards
From: Arnd Bergmann @ 2011-06-17 11:52 UTC (permalink / raw)
  To: Dmitry Eremin-Solenikov; +Cc: davej, linuxppc-dev, cpufreq, paulus
In-Reply-To: <BANLkTimo6k=Zz+QnP0VaWYf3jDjpFXNL4w@mail.gmail.com>

On Friday 17 June 2011 13:12:56 Dmitry Eremin-Solenikov wrote:
> > I think new cpufreq drivers should live in drivers/cpufreq, not in arch/.
> > We've
> > started moving other drivers away from arch/x86 and arch/arm already.
> 
> What about drivers/cpufreq/powerpc, or it's an unnecessary?

drivers/cpufreq is fine by itself, unless Dave has some preference.

> Should I resumbit it, or there will be massive arch/powerpc ->
> drivers/cpufreq move?

My recommendation would be to submit the new driver for drivers/cpufreq
directly and let Ben take care of the others at a convenient time.

	Arnd

^ permalink raw reply

* Re: [PATCH] Add cpufreq driver for Momentum Maple boards
From: Dmitry Eremin-Solenikov @ 2011-06-17 11:12 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: davej, linuxppc-dev, cpufreq, paulus
In-Reply-To: <201106171117.14794.arnd@arndb.de>

Hello,

On 6/17/11, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 17 June 2011 10:49:16 Dmitry Eremin-Solenikov wrote:
>> Add simple cpufreq driver for Maple-based boards (ppc970fx evaluation
>> kit and others). Driver is based on a cpufreq driver for 64-bit powermac
>> boxes with all pmac-dependant features removed and simple cleanup
>> applied.
>>
>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>> ---
>>  arch/powerpc/kernel/misc_64.S          |    4 +-
>>  arch/powerpc/platforms/Kconfig         |    8 +
>>  arch/powerpc/platforms/maple/Makefile  |    1 +
>>  arch/powerpc/platforms/maple/cpufreq.c |  317
>> ++++++++++++++++++++++++++++++++
>>  4 files changed, 328 insertions(+), 2 deletions(-)
>>  create mode 100644 arch/powerpc/platforms/maple/cpufreq.c
>>
>
>
> I think new cpufreq drivers should live in drivers/cpufreq, not in arch/.
> We've
> started moving other drivers away from arch/x86 and arch/arm already.

What about drivers/cpufreq/powerpc, or it's an unnecessary?

Should I resumbit it, or there will be massive arch/powerpc ->
drivers/cpufreq move?

-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: Changes to of_device ?
From: Philipp Ittershagen @ 2011-06-17 10:55 UTC (permalink / raw)
  To: Guillaume Dargaud; +Cc: linuxppc-dev
In-Reply-To: <201106171119.48178.dargaud@lpsc.in2p3.fr>

On 06/17/2011 11:19 AM, Guillaume Dargaud wrote:
> [I'm on the latest Xilinx tree, FWIW, so that's 2.6.25 still ?]

Not answering your primary question here, but in order to print the most
recent tag (along with a uniqe suffix, see man page) that is reachable
from your current branch, you can use "git describe". It will print out
the kernel version on your latest Xilinx tree.


Greetings,

  Philipp

^ permalink raw reply

* Changes to of_device ?
From: Guillaume Dargaud @ 2011-06-17  9:19 UTC (permalink / raw)
  To: linuxppc-dev

A couple linux versions ago, I had the following working code in my driver:

static int xad_driver_probe(struct of_device* dev, const struct of_device_id *match) {
	struct device_node *dn = dev->dev.of_node;

But now it won't compile and I can't figure out how to get the device_node structure from the of_device one ?
dev->dev is a struct device and there's no device_node info in it...
What has changed ?

[I'm on the latest Xilinx tree, FWIW, so that's 2.6.25 still ?]
-- 
Guillaume Dargaud
http://www.gdargaud.net/

^ 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