LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: qe: add ability to upload QE firmware
From: Timur Tabi @ 2007-10-18 22:53 UTC (permalink / raw)
  To: Jerry Van Baren; +Cc: linuxppc-dev
In-Reply-To: <4717DEE8.9010508@gmail.com>

Jerry Van Baren wrote:

> We seem to be a little conflicted over whether it is an upload or a 
> download.  ;-)

Not me.

To the host == download
 From the host == upload

^ permalink raw reply

* Re: qe: add ability to upload QE firmware
From: Jerry Van Baren @ 2007-10-18 22:32 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev
In-Reply-To: <11927201051427-git-send-email-timur@freescale.com>

Timur Tabi wrote:
> Define the layout of a binary blob that contains a QE firmware and instructions
> on how to upload it.  Add function qe_upload_microcode() to parse the blob
> and perform the actual upload.  Fully define 'struct rsp' in immap_qe.h to
> include the actual RISC Special Registers.
> 
> Signed-off-by: Timur Tabi <timur@freescale.com>

[snip]

> +/*
> + * Download a microcode to the I-RAM at a specific address.
       ^^^^^^^^
> + *
> + * @firmware: pointer to qe_firmware structure
> + */
> +int qe_upload_microcode(const struct qe_firmware *firmware)
           ^^^^^^
We seem to be a little conflicted over whether it is an upload or a 
download.  ;-)

gvb

^ permalink raw reply

* Re: driver using device trees, irqs, powerpc
From: Grant Likely @ 2007-10-18 22:23 UTC (permalink / raw)
  To: Alan Bennett; +Cc: linuxppc-dev
In-Reply-To: <bfa0697f0710181519y261b41b1y46df293c61be344c@mail.gmail.com>

On 10/18/07, Alan Bennett <alan@akb.net> wrote:
> Need help adding new driver to supports a custom FPGA.  Is there a
> good driver that anyone would recommend I can look at that is fully
> functional, works with 2.6.23 (powerpc style) and is described within
> a device tree?
>
> Bonus points:
>   uses freescale interrupts (timers/irq's from mpc 82xx/etc...)
>   isn't enet / cpm_uart (I've already looked at these)

Take a look at the xilinx device drivers.

drivers/block/xsysace.c
drivers/video/xilinxfb.c
driver/serial/uartlite.c

Each of these drivers support both platform bus and of_platform bus
(device tree).  All three are merged into Linus' mainline tree.

Cheers,
g.

>
> -Alan
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* driver using device trees, irqs, powerpc
From: Alan Bennett @ 2007-10-18 22:19 UTC (permalink / raw)
  To: linuxppc-dev

Need help adding new driver to supports a custom FPGA.  Is there a
good driver that anyone would recommend I can look at that is fully
functional, works with 2.6.23 (powerpc style) and is described within
a device tree?

Bonus points:
  uses freescale interrupts (timers/irq's from mpc 82xx/etc...)
  isn't enet / cpm_uart (I've already looked at these)

-Alan

^ permalink raw reply

* Re: [PATCH] synchronize_irq needs a barrier
From: Benjamin Herrenschmidt @ 2007-10-18 22:05 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linuxppc-dev, akpm, torvalds, linux-kernel
In-Reply-To: <E1IiWnJ-0000ne-00@gondolin.me.apana.org.au>


On Thu, 2007-10-18 at 22:56 +0800, Herbert Xu wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> >
> > Take a real life example:
> > 
> > drivers/message/fusion/mptbase.c
> > 
> >        /* Disable interrupts! */
> >        CHIPREG_WRITE32(&ioc->chip->IntMask, 0xFFFFFFFF);
> > 
> >        ioc->active = 0;
> >        synchronize_irq(pdev->irq);
> > 
> > And we aren't in a spinlock here. 
> > 
> > That's just a random example grepped.... I think I see a few more. Then,
> > some drivers like tg3 actually do an smp_mb() before calling
> > synchronize_irq(). But then, some don't.
> 
> I really don't see what the point of the barrier would be here.
> Barriers are generally useless unless you have a counter-part
> on the other side.

The barrier would guarantee that ioc->active (and in fact the write to
the chip too above) are globally visible before the read of the irq
status. There are two different issues not to mixup here. Any previous
store vs. a read (general case) and MMIO store of IRQ mask which has
issues of it's own.

> The counterpart here is presumably the interrupt handler, which
> should be terminated by the IO write above regardless of the
> memory barrier.
>
> Of course I might have missed your point.  If so please give
> a description like this for the race that you see:
> 
> CPU1			CPU2
> disable IRQ
> 			whatever the race is
> synchronize_irq

Let's ignore for a moment the generic problem of loads crossing previous
stores and focus on the pure issue of using MMIO writes to mask
interrupts.

Writing to a chip to disable an IRQ is generally never going to provide
any synchronisation guarantee. The MMIO write itself is asynchronous and
not ordered vs. a subsequent read from main storage (ie memory). So
unless you do an MMIO read to flush it, you basically haven't yet
disabled your IRQ don't know when it will be. That's one thing.

Another one is that even if you do an MMIO read to flush, your IRQ may
already have been latched by your PIC, or may be an MSI already issued
and still travelling along busses, and thus might well occur in a few
instructions. In that later case, note that ignoring it is perfectly
fine since the IRQ line will eventually go down (for a level IRQ that
is, for an edge IRQ, ignoring is always fine). That's what we cause
short interrupts, they can be common, though linux has historical issues
with them (unrelated to this discussion). But your interrupt handler
_will_ be called, and thus should be aware that your intend is to ignore
it.

So for both of the reasons above, the MMIO write doesn't mean you IRQ
won't happen and in fact, synchronize_irq() here is totally useless
since it won't prevent the IRQ from actually still happening just after
the test_bit of IRQ_INPROGRESS.

Now, the way to actually do that properly is to _also_ have a flag to
indicate your handler you don't want to deal with that interrupt. That
could be something along the lines of:

	writel(irq mask);
	wmb();
	chip->masked = 1;
 	smp_mb();
	synchronize_irq();

Now, the IRQ handler can just do

	if (chip->masked == 1)
		return <whatever>;

Another way is to use spinlocks of course, but we are talking about high
perf stuff that tries to avoid spinlocks on every IRQs, which is fair
enough, thus putting the synchronization burden on the slow path.

In the above example, you need that smp_mb() to make sure that the
effect of chip->masked = 1 is globally visible before the read in
synchronize_irq() is performed. Without that, that read could cross the
previous write, in fact, it may even travel all the way to before the
MMIO in some cases, and thus cause synchronize_irq() to operate on a
completely stale version of irq_desc->status, thus possibly bailing out
early while the IRQ is still happening and chip->masked not yet visible
to the other CPUs. 

In general, synchronize_irq() alone is always bogus, because that read
can travel up. That's why I believe it would simply make things simpler
and avoid problems to put the smp_mb(); inside synchronize_irq() (and same
goes with napi_synchronize() for the exact same reasons).

> While in general I'd agree with you about give latitude to
> drivers, memory barriers I think is something that we can't
> afford to.


> The reason is that memory barries tend to come in pairs, e.g.,
> 
> CPU1			CPU2
> write A
> wmb
> write B
> 			read B
> 			rmb
> 			read A
> 
> Taking away either barrier would render the other useless.
> 
> So whenever we add only one barrier for the benefit of driver
> writers who don't bother to think about barriers we may not
> be helping them at all.

In the case of synchronize_irq and my above example, yes, indeed, there
should be a pending barrier between setting IRQ_INPROGRESS and the
reading of chip->masked by the driver. This is a very good point because
I incorrectly assumed that the spin_unlock inside handle_*_irq before
calling the handler would do it, but it will not indeed. The spin_unlock
is only a write barrier, not a read barrier, it won't prevent a
following read from travelling back into the spinlock, potentially
before the setting of IRQ_INPROGRESS.

Thus, indeed, either an smp_rmb() should be used in the IRQ handler
before testing thus chip->masked, or we should stick one in
handle_IRQ_event for safety.

I'm pretty sure quite a few drivers are broken in that regard :-)

So maybe it's fair enough to say that barriers in those case should be
left as a responsibility to the callers. However, I still think that
synchronize_irq() without a barrier will generally not make any sense,
even if you should also generally have some kind of matching barrier
within whatever you are synchronizing with.

Ben.
 

^ permalink raw reply

* [PATCH] [POWERPC] powerpc: Add -mno-spe for ARCH=powerpc builds
From: Kumar Gala @ 2007-10-18 21:55 UTC (permalink / raw)
  To: linuxppc-dev

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/Makefile |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 4e16534..bd87626 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -107,6 +107,9 @@ endif
 # No AltiVec instruction when building kernel
 KBUILD_CFLAGS += $(call cc-option,-mno-altivec)

+# No SPE instruction when building kernel
+KBUILD_CFLAGS += $(call cc-option,-mno-spe)
+
 # Enable unit-at-a-time mode when possible. It shrinks the
 # kernel considerably.
 KBUILD_CFLAGS += $(call cc-option,-funit-at-a-time)
-- 
1.5.2.4

^ permalink raw reply related

* Re: [PATCH] synchronize_irq needs a barrier
From: Benjamin Herrenschmidt @ 2007-10-18 21:35 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linuxppc-dev, akpm, torvalds, linux-kernel
In-Reply-To: <E1IiWTY-0000jx-00@gondolin.me.apana.org.au>


On Thu, 2007-10-18 at 22:35 +0800, Herbert Xu wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> > 
> > Note that some kind of read barrier or compiler barrier should be needed
> > regardless, or we are just not sync'ing with anything at all (we may
> > have loaded the value ages ago and thus operate on a totally stale
> > value). I prefer a full barrier to also ensure all previous stores are
> > pushed out.
> 
> We already have a compiler barrier there in the form of cpu_relax.

Isn't it too late ? The barrier should be before the test_bit, to
prevent it from moving up.

Ben.
 

^ permalink raw reply

* Re: [PATCH 2/2] bootwrapper: Bail from script if any command fails
From: Grant Likely @ 2007-10-18 20:19 UTC (permalink / raw)
  To: Grant Likely, linuxppc-dev, paulus, Scott Wood
In-Reply-To: <20071017003331.GA28260@localhost.localdomain>

On 10/16/07, David Gibson <david@gibson.dropbear.id.au> wrote:
> On Tue, Oct 16, 2007 at 03:31:07PM -0600, Grant Likely wrote:
> > From: Grant Likely <grant.likely@secretlab.ca>
> >
> > Add the 'set -e' command to the wrapper script so that if any command
> > fails then the script will automatically exit
>
> Ah.. this will conflict with my patch to merge dtc (because that
> touches the line invoking dtc to change the path to it).

>From comments I've seen on the list, it looks like if the dtc patch
goes in there still needs to be a couple of review cycles.  Are you
okay with this patch getting merged in the mean time?

Thanks,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* [PATCH] phy/bitbang: missing MODULE_LICENSE
From: Randy Dunlap @ 2007-10-18 19:20 UTC (permalink / raw)
  To: Scott Wood; +Cc: netdev, jgarzik, linuxppc-dev
In-Reply-To: <20070920220123.GG28784@loki.buserror.net>

From: Randy Dunlap <randy.dunlap@oracle.com>

Missing MODULE_LICENSE(), loading this module taints the kernel.

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 drivers/net/phy/mdio-bitbang.c |    2 ++
 1 file changed, 2 insertions(+)

--- linux-2.6.23-git7.orig/drivers/net/phy/mdio-bitbang.c
+++ linux-2.6.23-git7/drivers/net/phy/mdio-bitbang.c
@@ -185,3 +185,5 @@ void free_mdio_bitbang(struct mii_bus *b
 	module_put(ctrl->ops->owner);
 	kfree(bus);
 }
+
+MODULE_LICENSE("GPL");

^ permalink raw reply

* Re: Merge dtc
From: Sam Ravnborg @ 2007-10-18 19:59 UTC (permalink / raw)
  To: Milton Miller, David Gibson; +Cc: ppcdev, Paul Mackerras, David Gibson
In-Reply-To: <9a3f3582990ad94feb76770e82c50bdd@bga.com>

On Thu, Oct 18, 2007 at 12:49:54PM -0500, Milton Miller wrote:
> On Tue Oct 16 15:02:17 EST 2007, David Gibson wrote:
> 
> >This very large patch incorporates a copy of dtc into the kernel
> >source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> >longer an external dependency to build kernels with configurations
> >which need a dtb file.
> >
> >Signed-off-by: David Gibson <david at gibson.dropbear.id.au>
> >
> >Too big for the list, full patch at
> >http://ozlabs.org/~dgibson/home/merge-dtc.patch+
> 
> So split it up.   The obvious one is "here is the unique content, then 
> copy these files from dtc git" would have been better.
One obvious split is a patch solely containing the _shipped files.
And next patch the rest.

As Milton already pointed out you should build dtc in the
dtc directory (why the -src prefix??).
And the dtc specific Makefile looks like something from
the late 80'. Please drop all these ALLUPPERCASE variables
and accept a little bit of redundancy.
Then mere humans may be able to read the Makefile.

When you have done the above I would be happy to review the
kbuild bits.

	Sam

^ permalink raw reply

* Re: [PATCH v3 1/2] [POWERPC] MPC8568E-MDS: create localbus node
From: Kumar Gala @ 2007-10-18 19:59 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: linuxppc-dev
In-Reply-To: <20071018180053.GA16748@localhost.localdomain>


On Oct 18, 2007, at 1:00 PM, Anton Vorontsov wrote:

> This patch creates localbus node, moves bcsr into it, and adds
> localbus to the probe path.
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>

why two patches for lbus and flash to lbus?

- k

^ permalink raw reply

* Re: [PATCH v3 2/2] [POWERPC] MPC8568E-MDS: add support for flash
From: Kumar Gala @ 2007-10-18 19:58 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: linuxppc-dev
In-Reply-To: <20071018180104.GB16748@localhost.localdomain>


On Oct 18, 2007, at 1:01 PM, Anton Vorontsov wrote:

> MPC8568E-MDS have 1 32MB Spansion x16 CFI flash chip. Let's use it.
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
>  arch/powerpc/boot/dts/mpc8568mds.dts |   34 +++++++++++++++++++++++ 
> ++++++++++-
>  1 files changed, 33 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/mpc8568mds.dts b/arch/powerpc/ 
> boot/dts/mpc8568mds.dts
> index be8d512..758ada4 100644
> --- a/arch/powerpc/boot/dts/mpc8568mds.dts
> +++ b/arch/powerpc/boot/dts/mpc8568mds.dts
> @@ -47,12 +47,44 @@
>  		#size-cells = <1>;
>  		compatible = "fsl,mpc8568-localbus";
>  		reg = <e0005000 d8>;
> -		ranges = <1 0 f8000000 0008000>;
> +		ranges = <1 0 f8000000 0008000
> +			  0 0 fe000000 2000000>;
>
>  		bcsr@1,0 {
>  			device_type = "board-control";
>  			reg = <1 0 8000>;
>  		};
> +
> +		flash@0,0 {
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			compatible = "Spansion,S29GL256N11TFIV2O", "cfi-flash";
> +			reg = <0 0 2000000>;
> +			bank-width = <2>;
> +			device-width = <1>;
> +

Are you basing the partition map on something or making it up?   
Clearly hrcw & u-boot are at fixed offsets, wondering about kernel &  
rootfs?

> +			hrcw@0 {
> +				label = "hrcw";
> +				reg = <0 20000>;
> +				read-only;
> +			};
> +
> +			kernel@20000 {
> +				label = "kernel";
> +				reg = <20000 200000>;
> +			};
> +
> +			rootfs@220000 {
> +				label = "rootfs";
> +				reg = <220000 1d60000>;
> +			};
> +
> +			uboot@1f80000 {
> +				label = "u-boot";
> +				reg = <1f80000 80000>;
> +				read-only;
> +			};
> +		};
>  	};
>
>  	soc8568@e0000000 {
> -- 
> 1.5.0.6
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

^ permalink raw reply

* Re: qe: add ability to upload QE firmware
From: Anton Vorontsov @ 2007-10-18 19:43 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev
In-Reply-To: <4717ADCE.9070807@freescale.com>

On Thu, Oct 18, 2007 at 02:02:38PM -0500, Timur Tabi wrote:
> Anton Vorontsov wrote:
>>> +	if (firmware->length == (length + sizeof(u32))) {
>>> +		/* Length is valid, and there's a CRC */
>>> +		crc = be32_to_cpu(*((__be32 *) ((void *) firmware + length)));
>> Spaces are not needed after "(__be32 *)" and "(void *)". The whole
>> construction isn't easy to parse, thus spaces are only distracting.
>
> I have to disagree.  I added the spaces to make it easier to read.
>
>>> +	/* If there's only one microcode, then we assume it's common for all
>>> +	   RISCs, so we set the CERCR.CIR bit to share the IRAM with all RISCs.
>>> +	   This should be safe even on SOCs with only one RISC.
>>> +
>>> +	   If there are multiple 'microcode' structures, but each one points
>>> +	   to the same microcode binary (ignoring offsets), then we also assume
>>> +	   that we want share RAM.
>>> +	 */
>> Comment style is unorthodox.
>
> Should I prefix each line with a "*"?

Yup.

Heh... well, ideal multiline comment is:

/*
 * Multiline comment here.
 */

But this doesn't matter much, the salt is in "*"s. ;-)

>>> +		code = (void *) firmware + be32_to_cpu(ucode->code_offset);
>> space after (void *).
>
> Really?  This:
>
> 	code = (void *)firmware + be32_to_cpu(ucode->code_offset);
>
> is harder to read.  Without the space, it looks like *)firmware is one 
> word.

:-) I'm not about to argue, this is each own preference.

But to complete my point: I don't care what style exactly is used, what
I care about is consistency across the code I'm looking in. 100%
consistency is not affordable, of course. But we're all trying, aren't we?
(type *)variable is orthodox style, and when I see unusual style, my eyes
are itching, and this code is hard to read (to me). So, if everybody will
switch to "(type *) variable" style -- I'll just follow.


And again: this is all of minor importance, just my $0.02 for the code
consistency.

Thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

^ permalink raw reply

* [PATCH] trivial - QUICC UCC driver - remove buggy and unused vdbg macro
From: Joe Perches @ 2007-10-18 19:24 UTC (permalink / raw)
  To: Li Yang; +Cc: netdev, linuxppc-dev

remove buggy and unused vdbg macro

Signed-off-by: Joe Perches <joe@perches.com>

diff --git a/drivers/net/ucc_geth_mii.c b/drivers/net/ucc_geth_mii.c
index df884f0..a4b481c 100644
--- a/drivers/net/ucc_geth_mii.c
+++ b/drivers/net/ucc_geth_mii.c
@@ -46,13 +46,6 @@
 #include "ucc_geth_mii.h"
 #include "ucc_geth.h"
 
-#define DEBUG
-#ifdef DEBUG
-#define vdbg(format, arg...) printk(KERN_DEBUG , format "\n" , ## arg)
-#else
-#define vdbg(format, arg...) do {} while(0)
-#endif
-
 #define MII_DRV_DESC "QE UCC Ethernet Controller MII Bus"
 #define MII_DRV_NAME "fsl-uec_mdio"
 

^ permalink raw reply related

* Re: [PATCH v3 0/4] FEC - fast ethernet controller for mpc52xx
From: Jeff Garzik @ 2007-10-18 19:14 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, netdev, Domen Puncer
In-Reply-To: <fa686aa40710180715u23d81adfy83ff65837725fb80@mail.gmail.com>

Grant Likely wrote:
> On 10/15/07, Jeff Garzik <jgarzik@pobox.com> wrote:
>> Domen Puncer wrote:
>>> Hello!
>>>
>>> If there are no objections, I would like to get this merged
>>> when bestcomm goes in (any time now?).
>>>
>>> It's split into four parts:
>>> 1 - device tree
>>> 2 - small bestcomm change
>>> 3 - the actual driver
>>> 4 - phy part of the driver
>> patches #3 and #4 need to be combined together.
>>
>> Are the arch people OK with patches #1 and #2?
> 
> Jeff,
> 
> The bestcomm patches and patch 1 & 2 from this series are now in
> Linus' tree.  That clears the way for the FEC driver when Domen
> reposts it.  (In other words; there is nothing left in arch land
> blocking this driver)
> 

except a resend combining patches 3 and 4 as requested :)

^ permalink raw reply

* Re: [PATCH 2/2] Use of_get_pci_dev_node() in axon_msi.c
From: Linas Vepstas @ 2007-10-18 19:09 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: sparclinux, Stephen Rothwell, Paul Mackerras, David S. Miller,
	linuxppc-dev
In-Reply-To: <1192670843.6681.10.camel@concordia>

On Thu, Oct 18, 2007 at 11:27:23AM +1000, Michael Ellerman wrote:
> 
> It does what pci_device_to_OF_node() does, but in the right way. 
> 
> The plan is to remove pci_device_to_OF_node() once all the callers have
> been converted to properly handle the refcounting. 

Oh. Yes. well, of course, then. Excellent reason. I didn't get 
that from the patch commit comments. So, FWIW:

Ack'ed-by: Linas Vepstas <linas@austin.ibm.com>

--linas

^ permalink raw reply

* RE: [PATCH v3] Device tree bindings for Xilinx devices
From: Stephen Neuendorffer @ 2007-10-18 19:04 UTC (permalink / raw)
  To: Grant Likely
  Cc: Leonid, Arnd Bergmann, microblaze-uclinux, linuxppc-dev,
	Wolfgang Reissnegger
In-Reply-To: <fa686aa40710181112pbe37a18xb21b7c5bcbe973b6@mail.gmail.com>

=20

> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On=20
> Behalf Of Grant Likely
> Sent: Thursday, October 18, 2007 11:13 AM
> To: Stephen Neuendorffer
> Cc: linuxppc-dev@ozlabs.org; Wolfgang Reissnegger; Leonid;=20
> microblaze-uclinux@itee.uq.edu.au; Josh Boyer; Arnd Bergmann
> Subject: Re: [PATCH v3] Device tree bindings for Xilinx devices
>=20
> On 10/18/07, Stephen Neuendorffer=20
> <stephen.neuendorffer@xilinx.com> wrote:
> >
> > > -----Original Message-----
> > > From: Grant Likely [mailto:grant.likely@secretlab.ca]
> > > Sent: Thursday, October 18, 2007 10:23 AM
> > > To: linuxppc-dev@ozlabs.org; Stephen Neuendorffer; Wolfgang
> > > Reissnegger; Leonid; microblaze-uclinux@itee.uq.edu.au; Josh
> > > Boyer; Arnd Bergmann
> > > Subject: [PATCH v3] Device tree bindings for Xilinx devices
> > >
> > > From: Grant Likely <grant.likely@secretlab.ca>
> > >
> > > Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> >
> > Acked-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
> >
> > This is good enough for the moment, and it reflects the=20
> best we can do
> > right now...  But:
> >
> > > +     (parameter#):   C_* parameters from system.mhs.  The C_
> > > prefix is
> > > +                     dropped from the parameter name, the
> > > name is converted
> > > +                     to lowercase and all underscore '_'
> > > characters are
> > > +                     converted to dashes '-'.
> >
> > Unfortunately, xparameters don't follow this exactly,=20
> particularly for
> > the 'multiple logical devices' case...  Maybe it's worth adding
> > something like: "For simple devices, the parameter name can=20
> be formed
> > by:"
>=20
> What specific parts of this are you concerned about? Examples?

Imagine if the ps2 adapter had a parameter "FOO" for each logical
device, In the core, one would be named C_PORT_0_FOO and another
C_PORT_1_FOO.  But, of course, you would want the FOO parameters to be
unmangled in the correct way for each port (as your hierarchical example
shows, but without any parameters).  In the xparameters, this unmangling
has been done, and the xparameters would look something like
XPAR_PS2_0_FOO and XPAR_PS2_1_FOO.

In other cases, xparameters for one core might be extracted from another
core that it is connected to, and in fact might reflect how the two are
connected.  This is how some cores under development will work (in EDK
9.2).

In short, I think its better to think about this all relative to
xparameters, rather than relative to the .mhs file.  In all current
cases, this is (I think) the same, but in the (very near future) it
won't be.

> > In any event, this should essentially document what the=20
> mechanism for
> > generating the device trees actually does...  Have you updated
> > gen_mhs_devtree.py to reflect all this?
>=20
> No; I'm looking at this as a line in the sand that we can use as a
> starting point and discuss the issues.

OK...  Well, as far as I'm concerned it looks good enough that we should
try it out... :)   I'll hack up gen_mhs_devtree and see how far we get.

> > > +   Typically, the compatible list will include the exact IP
> > > core version
> > > +   followed by an older IP core version which implements the same
> > > +   interface or any other device with the same interface.
> >
> > This seems to be awkward, since it means that the tree=20
> *and* the driver
> > will likely have long lists of compatible types.  In the=20
> driver, this is
> > hard to maintain, and in the device tree we don't have an=20
> easy way of
> > knowing exactly what is compatible and what isn't anyway...=20
>  It seems
> > better to me to have the
> > compatible list include the exact type (e.g.=20
> xilinx,opb-uartlite-1.00.b)
> > and
> > a generic compatibility class (e.g. xilinx,uartlite), and for the
> > drivers
> > to essentially define the gross compatibility classes. =20
> Then the driver
> > only has an of_match with:
> >
> >  .compatible =3D "xilinx,uartlite"
>=20
> The problem with this is how do you define what "xilinx,uartlite" is?
> For example; there have been 3 major versions of the plb_temac, and
> IIRC v2 and v3 are *not* compatible.  Which one is xilinx,temac?
>=20
> However, this does not need to result in a long list.  Basically, the
> list should include the exact version of the device plus the first
> version that implemented that interface.  (hence it is "compatible"
> with the earlier version of the device).  If there are no prior
> devices implementing the same device, then the compatible list has
> exactly one instance.  This document should probably include a list of
> the well-known compatible values for each of the xilinx devices.
>=20
> I used to try and come up with generic values for compatible, but it
> just ends up causing problems in the long run.  The concept of what is
> generic changes over time, but a specific version is a known quantity.

OK... I'll buy it... :)  I missed the emphasis on the 'sparseness'...
And I agree about the versions being really canonical: we just need to
make sure that the actual versions that are critical get back in at some
point.

Steve

^ permalink raw reply

* Re: [PATCH v2 3/4] [POWERPC] Add restart support for mpc52xx based platforms
From: Marian Balakowicz @ 2007-10-18 19:03 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev
In-Reply-To: <fa686aa40710181153w7bb794ceve38e7802d603ad42@mail.gmail.com>

Grant Likely wrote:
> On 10/18/07, Marian Balakowicz <m8@semihalf.com> wrote:
>> Add common helper routines: mpc52xx_map_wdt() and mpc52xx_restart().
>>
>> This patch relies on Sascha Hauer's patch published in:
>> http://patchwork.ozlabs.org/linuxppc/patch?id=8910.
> 
> By 'relies', do you mean "depends on" or "was derived from"?

Initial version was derived from, and since then it slightly evolved.

m.

^ permalink raw reply

* Re: qe: add ability to upload QE firmware
From: Timur Tabi @ 2007-10-18 19:02 UTC (permalink / raw)
  To: avorontsov; +Cc: linuxppc-dev
In-Reply-To: <20071018185738.GA16782@localhost.localdomain>

Anton Vorontsov wrote:
>> +	if (firmware->length == (length + sizeof(u32))) {
>> +		/* Length is valid, and there's a CRC */
>> +		crc = be32_to_cpu(*((__be32 *) ((void *) firmware + length)));
> 
> Spaces are not needed after "(__be32 *)" and "(void *)". The whole
> construction isn't easy to parse, thus spaces are only distracting.

I have to disagree.  I added the spaces to make it easier to read.

>> +	/* If there's only one microcode, then we assume it's common for all
>> +	   RISCs, so we set the CERCR.CIR bit to share the IRAM with all RISCs.
>> +	   This should be safe even on SOCs with only one RISC.
>> +
>> +	   If there are multiple 'microcode' structures, but each one points
>> +	   to the same microcode binary (ignoring offsets), then we also assume
>> +	   that we want share RAM.
>> +	 */
> 
> Comment style is unorthodox.

Should I prefix each line with a "*"?

>> +		code = (void *) firmware + be32_to_cpu(ucode->code_offset);
> 
> space after (void *).

Really?  This:

	code = (void *)firmware + be32_to_cpu(ucode->code_offset);

is harder to read.  Without the space, it looks like *)firmware is one word.

I shall apply the other changes.  Thanks.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

^ permalink raw reply

* Re: qe: add ability to upload QE firmware
From: Anton Vorontsov @ 2007-10-18 18:57 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev
In-Reply-To: <11927201051427-git-send-email-timur@freescale.com>

On Thu, Oct 18, 2007 at 10:08:25AM -0500, Timur Tabi wrote:
> Define the layout of a binary blob that contains a QE firmware and instructions
> on how to upload it.  Add function qe_upload_microcode() to parse the blob
> and perform the actual upload.  Fully define 'struct rsp' in immap_qe.h to
> include the actual RISC Special Registers.
[...]
> I need to add a few fields to the structure, but I would still appreciate
> comments on the structure and the corresponding code.

It seems I'm too lazy to go deep inside the code, but it's okay... because
I believe it is works fine. ;-)

Thus sorry, only fluffy cosmetic notes.

> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
[...]
> +	if (firmware->length == (length + sizeof(u32))) {
> +		/* Length is valid, and there's a CRC */
> +		crc = be32_to_cpu(*((__be32 *) ((void *) firmware + length)));

Spaces are not needed after "(__be32 *)" and "(void *)". The whole
construction isn't easy to parse, thus spaces are only distracting.

> +		if (crc != crc32(0, firmware, length)) {
> +			printk(KERN_ERR "QE: firmware CRC is invalid\n");
> +			return -EIO;
> +		}
> +	} else if (firmware->length != length) {
> +		printk(KERN_ERR "QE: invalid length(s) in firware structure\n");
> +		return -EPERM;
> +	}
> +
> +	/* If there's only one microcode, then we assume it's common for all
> +	   RISCs, so we set the CERCR.CIR bit to share the IRAM with all RISCs.
> +	   This should be safe even on SOCs with only one RISC.
> +
> +	   If there are multiple 'microcode' structures, but each one points
> +	   to the same microcode binary (ignoring offsets), then we also assume
> +	   that we want share RAM.
> +	 */

Comment style is unorthodox.

> +	if (firmware->count == 1)
> +	      setbits16(&qe_immr->cp.cercr, cpu_to_be16(0x800));
> +	else {

I'd place braces for the first 'if'. No lines added, more good looking.
The CodingStyle also suggest this.

> +		for (i = 1; i < firmware->count; i++)

I'd place braces here too. No rationale though, just better looking.

> +			if (firmware->microcode[i].code_offset !=
> +			    firmware->microcode[0].code_offset)
> +				break;
> +
> +		if (i == firmware->count)
> +			setbits16(&qe_immr->cp.cercr, cpu_to_be16(0x800));

0x800? Desires proper #define.

> +	}
> +
> +	if (firmware->soc.model)
> +		printk(KERN_INFO "QE: uploading microcode '%s' for %u V%u.%u\n",
> +			firmware->id, be16_to_cpu(firmware->soc.model),
> +			firmware->soc.rev_h, firmware->soc.rev_l);
> +	else
> +		printk(KERN_INFO "QE: uploading microcode '%s'\n",
> +			firmware->id);
> +
> +	for (i = 0; i < firmware->count; i++) {
> +		const struct qe_microcode *ucode = &firmware->microcode[i];
> +
> +		code = (void *) firmware + be32_to_cpu(ucode->code_offset);

space after (void *).

> +		/* Use auto-increment */
> +		out_be32(&qe_immr->iram.iadd, cpu_to_be32(0x80080000 |

magic number.

> +			be32_to_cpu(ucode->iram_offset)));
> +
> +		for (j = 0; j < ucode->count; j++)
> +			out_be32(&qe_immr->iram.idata, be32_to_cpu(code[j]));
> +
> +		/* Program the traps.  We program both RISCs, even on platforms
> +		   that only have one.  This *should* be safe. */
> +		for (j = 0; j < 16; j++)
> +			if (ucode->traps[j]) {
> +				u32 trap = be32_to_cpu(ucode->traps[j]);
> +				out_be32(&qe_immr->rsp[0].tibcr[j], trap);

Empty line needed after variable declaration.

> +				out_be32(&qe_immr->rsp[1].tibcr[j], trap);
> +			}
> +	}
> +
> +	/* Enable the traps for both RISCs.  Again, on a single-RISC system,
> +	   this should be safe. */

Comment.

> +	out_be32(&qe_immr->rsp[0].eccr, cpu_to_be32(0x20800000));
> +	out_be32(&qe_immr->rsp[1].eccr, cpu_to_be32(0x20800000));

magic numbers.

Thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

^ permalink raw reply

* Re: [PATCH v2 3/4] [POWERPC] Add restart support for mpc52xx based platforms
From: Grant Likely @ 2007-10-18 18:53 UTC (permalink / raw)
  To: Marian Balakowicz; +Cc: linuxppc-dev
In-Reply-To: <20071018184433.3584.77107.stgit@hekate.izotz.org>

On 10/18/07, Marian Balakowicz <m8@semihalf.com> wrote:
> Add common helper routines: mpc52xx_map_wdt() and mpc52xx_restart().
>
> This patch relies on Sascha Hauer's patch published in:
> http://patchwork.ozlabs.org/linuxppc/patch?id=8910.

By 'relies', do you mean "depends on" or "was derived from"?

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* [PATCH v2 4/4] [POWERPC] Enable restart support for lite5200 board
From: Marian Balakowicz @ 2007-10-18 18:44 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20071018184407.3584.3513.stgit@hekate.izotz.org>

Signed-off-by: Marian Balakowicz <m8@semihalf.com>
---

 arch/powerpc/platforms/52xx/lite5200.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)


diff --git a/arch/powerpc/platforms/52xx/lite5200.c b/arch/powerpc/platforms/52xx/lite5200.c
index 65b7ae4..25d2bfa 100644
--- a/arch/powerpc/platforms/52xx/lite5200.c
+++ b/arch/powerpc/platforms/52xx/lite5200.c
@@ -145,6 +145,9 @@ static void __init lite5200_setup_arch(void)
 	/* Some mpc5200 & mpc5200b related configuration */
 	mpc5200_setup_xlb_arbiter();
 
+	/* Map wdt for mpc52xx_restart() */
+	mpc52xx_map_wdt();
+
 #ifdef CONFIG_PM
 	mpc52xx_suspend.board_suspend_prepare = lite5200_suspend_prepare;
 	mpc52xx_suspend.board_resume_finish = lite5200_resume_finish;
@@ -183,5 +186,6 @@ define_machine(lite5200) {
 	.init		= mpc52xx_declare_of_platform_devices,
 	.init_IRQ 	= mpc52xx_init_irq,
 	.get_irq 	= mpc52xx_get_irq,
+	.restart	= mpc52xx_restart,
 	.calibrate_decr	= generic_calibrate_decr,
 };

^ permalink raw reply related

* [PATCH v2 3/4] [POWERPC] Add restart support for mpc52xx based platforms
From: Marian Balakowicz @ 2007-10-18 18:44 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20071018184407.3584.3513.stgit@hekate.izotz.org>

Add common helper routines: mpc52xx_map_wdt() and mpc52xx_restart().

This patch relies on Sascha Hauer's patch published in:
http://patchwork.ozlabs.org/linuxppc/patch?id=8910.

Signed-off-by: Marian Balakowicz <m8@semihalf.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---

 arch/powerpc/platforms/52xx/mpc52xx_common.c |   50 ++++++++++++++++++++++++++
 include/asm-powerpc/mpc52xx.h                |    3 ++
 2 files changed, 53 insertions(+), 0 deletions(-)


diff --git a/arch/powerpc/platforms/52xx/mpc52xx_common.c b/arch/powerpc/platforms/52xx/mpc52xx_common.c
index 74b4b41..9850685 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_common.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_common.c
@@ -18,6 +18,13 @@
 #include <asm/prom.h>
 #include <asm/mpc52xx.h>
 
+/*
+ * This variable is mapped in mpc52xx_map_wdt() and used in mpc52xx_restart().
+ * Permanent mapping is required because mpc52xx_restart() can be called
+ * from interrupt context while node mapping (which calls ioremap())
+ * cannot be used at such point.
+ */
+static volatile struct mpc52xx_gpt *mpc52xx_wdt = NULL;
 
 static void __iomem *
 mpc52xx_map_node(struct device_node *ofn)
@@ -126,3 +133,46 @@ mpc52xx_declare_of_platform_devices(void)
 			"Error while probing of_platform bus\n");
 }
 
+void __init
+mpc52xx_map_wdt(void)
+{
+	const void *has_wdt;
+	struct device_node *np;
+
+	/* mpc52xx_wdt is mapped here and used in mpc52xx_restart,
+	 * possibly from a interrupt context. wdt is only implement
+	 * on a gpt0, so check has-wdt property before mapping.
+	 */
+	for_each_compatible_node(np, NULL, "fsl,mpc5200-gpt") {
+		has_wdt = of_get_property(np, "fsl,has-wdt", NULL);
+		if (has_wdt) {
+			mpc52xx_wdt = mpc52xx_map_node(np);
+			return;
+		}
+	}
+	for_each_compatible_node(np, NULL, "mpc5200-gpt") {
+		has_wdt = of_get_property(np, "has-wdt", NULL);
+		if (has_wdt) {
+			mpc52xx_wdt = mpc52xx_map_node(np);
+			return;
+		}
+	}
+}
+
+void
+mpc52xx_restart(char *cmd)
+{
+	local_irq_disable();
+
+	/* Turn on the watchdog and wait for it to expire.
+	 * It effectively does a reset. */
+	if (mpc52xx_wdt) {
+		out_be32(&mpc52xx_wdt->mode, 0x00000000);
+		out_be32(&mpc52xx_wdt->count, 0x000000ff);
+		out_be32(&mpc52xx_wdt->mode, 0x00009004);
+	} else
+		printk("mpc52xx_restart: Can't access wdt. "
+			"Restart impossible, system halted.\n");
+
+	while (1);
+}
diff --git a/include/asm-powerpc/mpc52xx.h b/include/asm-powerpc/mpc52xx.h
index 9cf05f9..39f619f 100644
--- a/include/asm-powerpc/mpc52xx.h
+++ b/include/asm-powerpc/mpc52xx.h
@@ -252,6 +252,9 @@ extern unsigned int mpc52xx_get_irq(void);
 
 extern int __init mpc52xx_add_bridge(struct device_node *node);
 
+extern void __init mpc52xx_map_wdt(void);
+extern void mpc52xx_restart(char *cmd);
+
 #endif /* __ASSEMBLY__ */
 
 #ifdef CONFIG_PM

^ permalink raw reply related

* [PATCH v2 2/4] [POWERPC] Update device tree binding for mpc5200 gpt
From: Marian Balakowicz @ 2007-10-18 18:44 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20071018184407.3584.3513.stgit@hekate.izotz.org>

Add 'fsl,' prefix to 'compatible' property for gpt nodes.
Add 'fsl,' prefix to empty, GPT0 specific 'has-wdt' property.

Signed-off-by: Marian Balakowicz <m8@semihalf.com>
---

 .../powerpc/mpc52xx-device-tree-bindings.txt       |    4 ++-
 arch/powerpc/boot/dts/lite5200.dts                 |   26 +++++++-------------
 arch/powerpc/boot/dts/lite5200b.dts                |   26 +++++++-------------
 drivers/char/watchdog/mpc5200_wdt.c                |    3 ++
 4 files changed, 23 insertions(+), 36 deletions(-)


diff --git a/Documentation/powerpc/mpc52xx-device-tree-bindings.txt b/Documentation/powerpc/mpc52xx-device-tree-bindings.txt
index e59fcbb..fedf7ef 100644
--- a/Documentation/powerpc/mpc52xx-device-tree-bindings.txt
+++ b/Documentation/powerpc/mpc52xx-device-tree-bindings.txt
@@ -185,7 +185,7 @@ bestcomm@<addr>	dma-controller		mpc5200-bestcomm 5200 pic also requires
 Recommended soc5200 child nodes; populate as needed for your board
 name		device_type	compatible	  Description
 ----		-----------	----------	  -----------
-gpt@<addr>	gpt		mpc5200-gpt	  General purpose timers
+gpt@<addr>	gpt		fsl,mpc5200-gpt	  General purpose timers
 rtc@<addr>	rtc		mpc5200-rtc	  Real time clock
 mscan@<addr>	mscan		mpc5200-mscan	  CAN bus controller
 pci@<addr>	pci		mpc5200-pci	  PCI bridge
@@ -213,7 +213,7 @@ cell-index	int		When multiple devices are present, is the
 5) General Purpose Timer nodes (child of soc5200 node)
 On the mpc5200 and 5200b, GPT0 has a watchdog timer function.  If the board
 design supports the internal wdt, then the device node for GPT0 should
-include the empty property 'has-wdt'.
+include the empty property 'fsl,has-wdt'.
 
 6) PSC nodes (child of soc5200 node)
 PSC nodes can define the optional 'port-number' property to force assignment
diff --git a/arch/powerpc/boot/dts/lite5200.dts b/arch/powerpc/boot/dts/lite5200.dts
index bc45f5f..6731763 100644
--- a/arch/powerpc/boot/dts/lite5200.dts
+++ b/arch/powerpc/boot/dts/lite5200.dts
@@ -70,18 +70,16 @@
 		};
 
 		gpt@600 {	// General Purpose Timer
-			compatible = "mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200-gpt";
 			cell-index = <0>;
 			reg = <600 10>;
 			interrupts = <1 9 0>;
 			interrupt-parent = <&mpc5200_pic>;
-			has-wdt;
+			fsl,has-wdt;
 		};
 
 		gpt@610 {	// General Purpose Timer
-			compatible = "mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200-gpt";
 			cell-index = <1>;
 			reg = <610 10>;
 			interrupts = <1 a 0>;
@@ -89,8 +87,7 @@
 		};
 
 		gpt@620 {	// General Purpose Timer
-			compatible = "mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200-gpt";
 			cell-index = <2>;
 			reg = <620 10>;
 			interrupts = <1 b 0>;
@@ -98,8 +95,7 @@
 		};
 
 		gpt@630 {	// General Purpose Timer
-			compatible = "mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200-gpt";
 			cell-index = <3>;
 			reg = <630 10>;
 			interrupts = <1 c 0>;
@@ -107,8 +103,7 @@
 		};
 
 		gpt@640 {	// General Purpose Timer
-			compatible = "mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200-gpt";
 			cell-index = <4>;
 			reg = <640 10>;
 			interrupts = <1 d 0>;
@@ -116,8 +111,7 @@
 		};
 
 		gpt@650 {	// General Purpose Timer
-			compatible = "mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200-gpt";
 			cell-index = <5>;
 			reg = <650 10>;
 			interrupts = <1 e 0>;
@@ -125,8 +119,7 @@
 		};
 
 		gpt@660 {	// General Purpose Timer
-			compatible = "mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200-gpt";
 			cell-index = <6>;
 			reg = <660 10>;
 			interrupts = <1 f 0>;
@@ -134,8 +127,7 @@
 		};
 
 		gpt@670 {	// General Purpose Timer
-			compatible = "mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200-gpt";
 			cell-index = <7>;
 			reg = <670 10>;
 			interrupts = <1 10 0>;
diff --git a/arch/powerpc/boot/dts/lite5200b.dts b/arch/powerpc/boot/dts/lite5200b.dts
index 6582c9a..b540388 100644
--- a/arch/powerpc/boot/dts/lite5200b.dts
+++ b/arch/powerpc/boot/dts/lite5200b.dts
@@ -70,18 +70,16 @@
 		};
 
 		gpt@600 {	// General Purpose Timer
-			compatible = "mpc5200b-gpt","mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200b-gpt","fsl,mpc5200-gpt";
 			cell-index = <0>;
 			reg = <600 10>;
 			interrupts = <1 9 0>;
 			interrupt-parent = <&mpc5200_pic>;
-			has-wdt;
+			fsl,has-wdt;
 		};
 
 		gpt@610 {	// General Purpose Timer
-			compatible = "mpc5200b-gpt","mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200b-gpt","fsl,mpc5200-gpt";
 			cell-index = <1>;
 			reg = <610 10>;
 			interrupts = <1 a 0>;
@@ -89,8 +87,7 @@
 		};
 
 		gpt@620 {	// General Purpose Timer
-			compatible = "mpc5200b-gpt","mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200b-gpt","fsl,mpc5200-gpt";
 			cell-index = <2>;
 			reg = <620 10>;
 			interrupts = <1 b 0>;
@@ -98,8 +95,7 @@
 		};
 
 		gpt@630 {	// General Purpose Timer
-			compatible = "mpc5200b-gpt","mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200b-gpt","fsl,mpc5200-gpt";
 			cell-index = <3>;
 			reg = <630 10>;
 			interrupts = <1 c 0>;
@@ -107,8 +103,7 @@
 		};
 
 		gpt@640 {	// General Purpose Timer
-			compatible = "mpc5200b-gpt","mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200b-gpt","fsl,mpc5200-gpt";
 			cell-index = <4>;
 			reg = <640 10>;
 			interrupts = <1 d 0>;
@@ -116,8 +111,7 @@
 		};
 
 		gpt@650 {	// General Purpose Timer
-			compatible = "mpc5200b-gpt","mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200b-gpt","fsl,mpc5200-gpt";
 			cell-index = <5>;
 			reg = <650 10>;
 			interrupts = <1 e 0>;
@@ -125,8 +119,7 @@
 		};
 
 		gpt@660 {	// General Purpose Timer
-			compatible = "mpc5200b-gpt","mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200b-gpt","fsl,mpc5200-gpt";
 			cell-index = <6>;
 			reg = <660 10>;
 			interrupts = <1 f 0>;
@@ -134,8 +127,7 @@
 		};
 
 		gpt@670 {	// General Purpose Timer
-			compatible = "mpc5200b-gpt","mpc5200-gpt";
-			device_type = "gpt";
+			compatible = "fsl,mpc5200b-gpt","fsl,mpc5200-gpt";
 			cell-index = <7>;
 			reg = <670 10>;
 			interrupts = <1 10 0>;
diff --git a/drivers/char/watchdog/mpc5200_wdt.c b/drivers/char/watchdog/mpc5200_wdt.c
index 9cfb975..11f6a11 100644
--- a/drivers/char/watchdog/mpc5200_wdt.c
+++ b/drivers/char/watchdog/mpc5200_wdt.c
@@ -176,6 +176,8 @@ static int mpc5200_wdt_probe(struct of_device *op, const struct of_device_id *ma
 
 	has_wdt = of_get_property(op->node, "has-wdt", NULL);
 	if (!has_wdt)
+		has_wdt = of_get_property(op->node, "fsl,has-wdt", NULL);
+	if (!has_wdt)
 		return -ENODEV;
 
 	wdt = kzalloc(sizeof(*wdt), GFP_KERNEL);
@@ -254,6 +256,7 @@ static int mpc5200_wdt_shutdown(struct of_device *op)
 
 static struct of_device_id mpc5200_wdt_match[] = {
 	{ .compatible = "mpc5200-gpt", },
+	{ .compatible = "fsl,mpc5200-gpt", },
 	{},
 };
 static struct of_platform_driver mpc5200_wdt_driver = {

^ permalink raw reply related

* [PATCH v2 1/4] [POWERPC] Add mpc52xx_find_and_map_path(), refactor utility functions
From: Marian Balakowicz @ 2007-10-18 18:44 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20071018184407.3584.3513.stgit@hekate.izotz.org>

Add helper routine mpc52xx_find_and_map_path(). Extract common code to
mpc52xx_map_node() and refactor mpc52xx_find_and_map().

Signed-off-by: Jan Wrobel <wrr@semihalf.com>
Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
---

 arch/powerpc/platforms/52xx/mpc52xx_common.c |   21 +++++++++++++++++----
 include/asm-powerpc/mpc52xx.h                |    1 +
 2 files changed, 18 insertions(+), 4 deletions(-)


diff --git a/arch/powerpc/platforms/52xx/mpc52xx_common.c b/arch/powerpc/platforms/52xx/mpc52xx_common.c
index 3bc201e..74b4b41 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_common.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_common.c
@@ -19,14 +19,12 @@
 #include <asm/mpc52xx.h>
 
 
-void __iomem *
-mpc52xx_find_and_map(const char *compatible)
+static void __iomem *
+mpc52xx_map_node(struct device_node *ofn)
 {
-	struct device_node *ofn;
 	const u32 *regaddr_p;
 	u64 regaddr64, size64;
 
-	ofn = of_find_compatible_node(NULL, NULL, compatible);
 	if (!ofn)
 		return NULL;
 
@@ -42,8 +40,23 @@ mpc52xx_find_and_map(const char *compatible)
 
 	return ioremap((u32)regaddr64, (u32)size64);
 }
+
+void __iomem *
+mpc52xx_find_and_map(const char *compatible)
+{
+	return mpc52xx_map_node(
+		of_find_compatible_node(NULL, NULL, compatible));
+}
+
 EXPORT_SYMBOL(mpc52xx_find_and_map);
 
+void __iomem *
+mpc52xx_find_and_map_path(const char *path)
+{
+	return mpc52xx_map_node(of_find_node_by_path(path));
+}
+
+EXPORT_SYMBOL(mpc52xx_find_and_map_path);
 
 /**
  * 	mpc52xx_find_ipb_freq - Find the IPB bus frequency for a device
diff --git a/include/asm-powerpc/mpc52xx.h b/include/asm-powerpc/mpc52xx.h
index 24751df..9cf05f9 100644
--- a/include/asm-powerpc/mpc52xx.h
+++ b/include/asm-powerpc/mpc52xx.h
@@ -242,6 +242,7 @@ struct mpc52xx_cdm {
 #ifndef __ASSEMBLY__
 
 extern void __iomem * mpc52xx_find_and_map(const char *);
+extern void __iomem * mpc52xx_find_and_map_path(const char *path);
 extern unsigned int mpc52xx_find_ipb_freq(struct device_node *node);
 extern void mpc5200_setup_xlb_arbiter(void);
 extern void mpc52xx_declare_of_platform_devices(void);

^ permalink raw reply related


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