* 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 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 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
* [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: 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
* 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: [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: 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
* [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: [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
* 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
* [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 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
* 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: 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
* 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: 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: [PATCH] synchronize_irq needs a barrier
From: Linus Torvalds @ 2007-10-18 22:52 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, akpm, Herbert Xu, linux-kernel
In-Reply-To: <1192745137.7367.40.camel@pasglop>
On Fri, 19 Oct 2007, Benjamin Herrenschmidt wrote:
>
> The barrier would guarantee that ioc->active (and in fact the write to
> the chip too above) are globally visible
No, it doesn't really guarantee that.
The thing is, there is no such thing as "globally visible".
There is a "ordering of visibility wrt CPU's", but it's not global, it's
quite potentially per-CPU. So a barrier on one CPU doesn't guarantee
anything at all without a barrier on the *other* CPU.
That said, the interrupt handling itself contains various barriers on the
CPU's that receive interrupts, thanks to the spinlocking. But I do agree
with Herbert that adding a "smb_mb()" is certainly in no way "obviously
correct", because it doesn't talk about what the other side does wrt
barriers and that word in memory.
Linus
^ permalink raw reply
* Re: [PATCH] synchronize_irq needs a barrier
From: Benjamin Herrenschmidt @ 2007-10-18 23:17 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linuxppc-dev, akpm, Herbert Xu, linux-kernel
In-Reply-To: <alpine.LFD.0.999.0710181549330.26902@woody.linux-foundation.org>
On Thu, 2007-10-18 at 15:52 -0700, Linus Torvalds wrote:
>
> On Fri, 19 Oct 2007, Benjamin Herrenschmidt wrote:
> >
> > The barrier would guarantee that ioc->active (and in fact the write to
> > the chip too above) are globally visible
>
> No, it doesn't really guarantee that.
>
> The thing is, there is no such thing as "globally visible".
>
> There is a "ordering of visibility wrt CPU's", but it's not global, it's
> quite potentially per-CPU. So a barrier on one CPU doesn't guarantee
> anything at all without a barrier on the *other* CPU.
>
> That said, the interrupt handling itself contains various barriers on the
> CPU's that receive interrupts, thanks to the spinlocking. But I do agree
> with Herbert that adding a "smb_mb()" is certainly in no way "obviously
> correct", because it doesn't talk about what the other side does wrt
> barriers and that word in memory.
I agree and you can see that in fact, we don't have enough barrier on
the other side since spin_unlock doesn't prevent subsequent loads from
crossing a previous store...
I wonder if that's worth trying to address, adding a barrier in
handle_IRQ_event for example, or we can continue ignoring the barrier
and let some drivers do their own fixes in fancy ways.
Ben.
^ permalink raw reply
* Re: [PATCH] [POWERPC] powerpc: Add -mno-spe for ARCH=powerpc builds
From: Paul Mackerras @ 2007-10-18 23:19 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <Pine.LNX.4.64.0710181655270.28684@blarg.am.freescale.net>
Kumar Gala writes:
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Your commit message doesn't give any reason why you are doing this, or
any explanation of what goes wrong without it. In fact, the commit
message is completely empty. :) Please resubmit with a decent commit
message.
Paul.
^ permalink raw reply
* Re: [PATCH] ppc44x: support for 256K PAGE_SIZE
From: Paul Mackerras @ 2007-10-18 23:21 UTC (permalink / raw)
To: Yuri Tikhonov; +Cc: linuxppc-dev
In-Reply-To: <200710181108.19413.yur@emcraft.com>
Yuri Tikhonov writes:
> The following patch adds support for 256KB PAGE_SIZE on ppc44x-based boards.
> The applications to be run on the kernel with 256KB PAGE_SIZE have to be
> built using the modified version of binutils, where the MAXPAGESIZE
> definition is set to 0x40000 (as opposite to standard 0x10000).
Have you measured the performance using a 64kB page size? If so, how
does it compare with the 256kB page size?
The 64kB page size has the attraction that no binutils changes are
required.
Paul.
^ permalink raw reply
* 转发: Re: Linux booting problem on Xilinx ppc
From: keng_629 @ 2007-10-18 23:27 UTC (permalink / raw)
To: linuxppc-embedded
In-Reply-To: <fa686aa40710171123uefac227q4fecd64516794129@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2270 bytes --]
On 10/17/07, aauer1 <aauer1@gmx.at > wrote:
>
>
> Grant Likely-2 wrote:
> >
> > On 10/17/07, aauer1 <aauer1@gmx.at > wrote:
> > > Now, I know that the kernel boots but I don't get an output with my
> > > Xilinx
> > > UartLite module. Are there some kernel modules which must be activated??
> > > Btw. I have used the Linux-2.6-virtex kernel from
> > > http://git.secretlab.ca/git/ and another Kernel (2.6.23 from kernel.org)
> > > -
> > > both with the same result.
> >
> > Post the output of __log_buf here please.
> >
> >
>
> Here is my dump of the __log_buf:
> ======================
> <5 >[ 0.000000] Linux version 2.6.23-rc2
> (aauer@servitus.student.iaik.tugraz.at) (gcc version 4.0.0 (DENX ELDK 4.1
> 4.0
> .0)) #8 Wed Oct 17 20:05:28 CEST 2007
> <6 >[ 0.000000] Xilinx ML403 Reference System (Virtex-4 FX)
> <7 >[ 0.000000] Entering add_active_range(0, 0, 16384) 0 entries of 256
> used
> <4 >[ 0.000000] Zone PFN ranges:
> <4 >[ 0.000000] DMA 0 - > 16384
> <4 >[ 0.000000] Normal 16384 - > 16384
> <4 >[ 0.000000] Movable zone start PFN for each node
> <4 >[ 0.000000] early_node_map[1] active PFN ranges
> <4 >[ 0.000000] 0: 0 - > 16384
> <7 >[ 0.000000] On node 0 totalpages: 16384
> <7 >[ 0.000000] DMA zone: 128 pages used for memmap
> <7 >[ 0.000000] DMA zone: 0 pages reserved
> <7 >[ 0.000000] DMA zone: 16256 pages, LIFO batch:3
> <7 >[ 0.000000] Normal zone: 0 pages used for memmap
> <7 >[ 0.000000] Movable zone: 0 pages used for memmap
> <4 >[ 0.000000] Built 1 zonelists in Zone order. Total pages: 16256
> <5 >[ 0.000000] Kernel command line: console=ttUL0 root=/dev/xsysace2 rw
try the command line bootargs console=ttyUL0,boardrate root=/dev/xsysace2 rw
you must make sure that you have the xsysace2 device, or you can take a test using ramdisk.i think it will run well
[-- Attachment #2: Type: text/html, Size: 6254 bytes --]
^ permalink raw reply
* Building zImage
From: Siva Prasad @ 2007-10-18 23:22 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <mailman.1363.1192734606.3099.linuxppc-dev@ozlabs.org>
Hi,
When I tried to build zImage (make zImage V=3D1) based on the 2.6.19
kernel for 8641HPCN board, I got the following error...
make -f scripts/Makefile.build obj=3Dlib
make -f
/export/beavis/work/sprasad/2.6.19/linux-2.6.19/scripts/Makefile.modpost
vmlinux
scripts/mod/modpost -m -o
/export/beavis/work/sprasad/2.6.19/linux-2.6.19/Module.symvers
vmlinux=20
rm -f .old_version
make ARCH=3Dppc64 -f scripts/Makefile.build obj=3Darch/powerpc/boot
arch/powerpc/boot/zImage
ln: accessing `arch/powerpc/boot/zImage': No such file or directory
make[1]: *** [arch/powerpc/boot/zImage] Error 1
make: *** [zImage] Error 2
When I looked into the Makefile, "image-y" was having no value. So, "ln"
was failing. Do I have to select one of the below options of zImage. I
am not really sure which one to use for 8641 supported by FreeScale.
image-$(CONFIG_PPC_PSERIES) +=3D zImage.pseries
image-$(CONFIG_PPC_MAPLE) +=3D zImage.pseries
image-$(CONFIG_PPC_IBM_CELL_BLADE) +=3D zImage.pseries
image-$(CONFIG_PPC_CHRP) +=3D zImage.chrp
image-$(CONFIG_PPC_PMAC) +=3D zImage.pmac
image-$(CONFIG_DEFAULT_UIMAGE) +=3D uImage
Well!... we are not using U-Boot, so uImage is not an option, and that
builds fine.
Any way, I tried to make uImage, but I am not sure if it is including
the dts file in the final uImage built, as the wrapper was passed the
value of platform as "uboot". I am confused.
Thanks for your help in advance.
- siva
^ permalink raw reply
* Re: [PATCH] synchronize_irq needs a barrier
From: Linus Torvalds @ 2007-10-18 23:39 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Herbert Xu, Linux Kernel Mailing List, linuxppc-dev,
Thomas Gleixner, akpm, Ingo Molnar
In-Reply-To: <1192749449.7367.51.camel@pasglop>
On Fri, 19 Oct 2007, Benjamin Herrenschmidt wrote:
>
> I agree and you can see that in fact, we don't have enough barrier on
> the other side since spin_unlock doesn't prevent subsequent loads from
> crossing a previous store...
>
> I wonder if that's worth trying to address, adding a barrier in
> handle_IRQ_event for example, or we can continue ignoring the barrier
> and let some drivers do their own fixes in fancy ways.
So how about something like this (untested! not necessarily very well
thought through either!)
Basic notion: the only thing that serializes the IRQ_INPROGRESS flag is
the descriptor lock. And we don't have to (or even want to!) hold it while
waiting for the thing, but we want to *have*held*it* in between whatever
we're synchronizing with.
The internal irq handler functions already held the lock when they did
whatever they need to serialize - and they are possibly performance
critical too - so they use the "internal" function that doesn't get the
lock unnecessarily again.
Hmm?
Linus
---
kernel/irq/manage.c | 22 ++++++++++++++++++----
1 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 80eab7a..f3e9575 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -14,6 +14,18 @@
#include "internals.h"
+/*
+ * Internally wait for IRQ_INPROGRESS to go away on other CPU's,
+ * after having serialized with the descriptor lock.
+ */
+static inline void do_synchronize_irq(struct irq_desc *desc)
+{
+#ifdef CONFIG_SMP
+ while (desc->status & IRQ_INPROGRESS)
+ cpu_relax();
+#endif
+}
+
#ifdef CONFIG_SMP
/**
@@ -28,13 +40,15 @@
*/
void synchronize_irq(unsigned int irq)
{
+ unsigned long flags;
struct irq_desc *desc = irq_desc + irq;
if (irq >= NR_IRQS)
return;
- while (desc->status & IRQ_INPROGRESS)
- cpu_relax();
+ spin_lock_irqsave(&desc->lock, flags);
+ spin_unlock_irqrestore(&desc->lock, flags);
+ do_synchronize_irq(desc);
}
EXPORT_SYMBOL(synchronize_irq);
@@ -129,7 +143,7 @@ void disable_irq(unsigned int irq)
disable_irq_nosync(irq);
if (desc->action)
- synchronize_irq(irq);
+ do_synchronize_irq(desc);
}
EXPORT_SYMBOL(disable_irq);
@@ -443,7 +457,7 @@ void free_irq(unsigned int irq, void *dev_id)
unregister_handler_proc(irq, action);
/* Make sure it's not being used on another CPU */
- synchronize_irq(irq);
+ do_synchronize_irq(desc);
#ifdef CONFIG_DEBUG_SHIRQ
/*
* It's a shared IRQ -- the driver ought to be
^ permalink raw reply related
* Re: [PATCH] synchronize_irq needs a barrier
From: Benjamin Herrenschmidt @ 2007-10-18 23:52 UTC (permalink / raw)
To: Linus Torvalds
Cc: Herbert Xu, Linux Kernel Mailing List, linuxppc-dev,
Thomas Gleixner, akpm, Ingo Molnar
In-Reply-To: <alpine.LFD.0.999.0710181627091.26902@woody.linux-foundation.org>
> So how about something like this (untested! not necessarily very well
> thought through either!)
>
> Basic notion: the only thing that serializes the IRQ_INPROGRESS flag is
> the descriptor lock. And we don't have to (or even want to!) hold it while
> waiting for the thing, but we want to *have*held*it* in between whatever
> we're synchronizing with.
>
> The internal irq handler functions already held the lock when they did
> whatever they need to serialize - and they are possibly performance
> critical too - so they use the "internal" function that doesn't get the
> lock unnecessarily again.
That may do the trick as the read can't cross the spin_lock (it can
cross spin_unlock but not lock). Advantage over adding a barrier to
handle_IRQ_event() is that it keeps the overhead to the slow path
(synchronize_irq).
Note that I didn't actually experience a problem here. I just came upon
that by accident while thinking about a similar issue I have with
napi_synchronize().
Looks good to me on a first glance (unfortunately a bit ugly but heh).
Ben.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox