* Gianfar driver failing on MPC8641D based board
From: Martyn Welch @ 2010-02-05 14:00 UTC (permalink / raw)
To: linuxppc-dev list, netdev; +Cc: Anton Vorontsov, Sandeep Gopalpet, davem
I have recently attempted to boot an 8641D based board from an NFS root.
The boot process grinds to a halt not long after the first access of the
NFS root and I receive multiple "nfs: server 192.168.0.1 not responding,
still trying" messages. Wireshark suggests that there is no further
traffic from this board at this point on. The NFS server seems to
eventually try sending duplicate packets it's already sent, which
results in "nfs: server 192.168.0.1 OK" messages, but the "not
responding" messages resume with no further traffic from the board.
I am able to boot to a ramdisk fine and the network seems to work -
though I haven't really pushed the interface from it.
I have attempted to git bisect, though I wasn't able to get much further
than discovering the problem was introduced in the 2.6.33 merge window -
at which point the gianfar network driver fails to compile (I have tried
to git bisect skip many, many times to no avail).
NFS booting fails for this board on todays linux-next, the master branch
of Kumar's PPC tree and the head of the main tree. I have also been able
to NFS boot from a random x86 based board that I have, using the head of
the main tree and the linux-next tree.
Copying the gianfar drivers from 2.6.32 into the head of the main tree
restores the correct behaviour and I'm able to NFS boot. I have heard
from others that the latest drivers work on 83xx and 85xx based boards,
but it seems to be broken on at least the 8641D.
I can see there has been a fair amount of work done on the gianfar
driver, I assume that this is a bug introduced by the multiple queue
support, but I'm way out of my depth on this.
I'm also off for the next week - so if I'm quiet, it'll be because of that.
Martyn
--
Martyn Welch (Principal Software Engineer) | Registered in England and
GE Intelligent Platforms | Wales (3828642) at 100
T +44(0)127322748 | Barbirolli Square, Manchester,
E martyn.welch@ge.com | M2 3AB VAT:GB 927559189
^ permalink raw reply
* Re: [PATCH 0/4] Fedora kernel patch review
From: Josh Boyer @ 2010-02-05 13:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20100205135354.GI12001@hansolo.jdub.homelinux.org>
On Fri, Feb 05, 2010 at 08:53:54AM -0500, Josh Boyer wrote:
>Provide a modalias entry for VIO devices in sysfs. I believe
>this was another initrd generation bugfix for anaconda.
Bah, subject on this should have been [PATCH 3/4] Provide VIO modalias
^ permalink raw reply
* Re: [PATCH 1/4] Add ps3_storage module alias
From: Josh Boyer @ 2010-02-05 14:03 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: linuxppc-dev
In-Reply-To: <alpine.LRH.2.00.1002051446280.5981@mink.sonytel.be>
On Fri, Feb 05, 2010 at 02:49:14PM +0100, Geert Uytterhoeven wrote:
>On Fri, 5 Feb 2010, Josh Boyer wrote:
>> This allows the upgrade path from older kernel releases to work
>> with the Fedora mkinitrd tool.
>>
>> ---
>>
>> --- linux-2.6.22.ppc64/drivers/block/ps3disk.c~ 2007-07-25 16:06:16.000000000 +0100
>> +++ linux-2.6.22.ppc64/drivers/block/ps3disk.c 2007-07-26 08:49:44.000000000 +0100
>> @@ -628,3 +628,4 @@ MODULE_LICENSE("GPL");
>> MODULE_DESCRIPTION("PS3 Disk Storage Driver");
>> MODULE_AUTHOR("Sony Corporation");
>> MODULE_ALIAS(PS3_MODULE_ALIAS_STOR_DISK);
>> +MODULE_ALIAS("ps3_storage");
>
>NAKed-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
>
>ps3_storage was replaced by 3 drivers: ps3disk, ps3rom, and ps3flash. These are
>automatically loaded by udev.
>
>How does mkinitrd determine the driver for the root file system on other
>systems? Does it really contain a big list of e.g. all SCSI drivers?
At one time I believe it did have a list of "odd" drivers to explicitly
include. Anaconda didn't (doesn't?) use udev to determine which drivers
to include in the initrd since the install image is most likely not composed
on a machine that includes those drivers. If the install initrd didn't have
the drivers loaded, it wouldn't install on PS3.
This is highlighted as a Fedora local change, probably for the reasons you
state. We might have already moved past the point where this is needed,
so I'm fine with it being NAKed. I'll probably just drop it from the Fedora
kernel builds.
josh
^ permalink raw reply
* Re: [PATCH 0/4] Fedora kernel patch review
From: Josh Boyer @ 2010-02-05 14:04 UTC (permalink / raw)
To: linuxppc-dev; +Cc: dwmw2
In-Reply-To: <20100205133552.GF12001@hansolo.jdub.homelinux.org>
On Fri, Feb 05, 2010 at 08:35:52AM -0500, Josh Boyer wrote:
>Hi All,
>
>Below are the set of ppc/ppc64 specific patches that Fedora has been
>carrying for a very long time now. It would be good to get some eyes
>on them to see if they could just be incorporated upstream.
David, I think you are the actual author on all 4 of these patches.
If you could look them over and reply with your Signed-off-by that
would be appreciated.
josh
^ permalink raw reply
* Re: [PATCH 1/4] Add ps3_storage module alias
From: Geert Uytterhoeven @ 2010-02-05 13:49 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev
In-Reply-To: <20100205134235.GG12001@hansolo.jdub.homelinux.org>
On Fri, 5 Feb 2010, Josh Boyer wrote:
> This allows the upgrade path from older kernel releases to work
> with the Fedora mkinitrd tool.
>=20
> ---
>=20
> --- linux-2.6.22.ppc64/drivers/block/ps3disk.c~=092007-07-25 16:06:16.000=
000000 +0100
> +++ linux-2.6.22.ppc64/drivers/block/ps3disk.c=092007-07-26 08:49:44.0000=
00000 +0100
> @@ -628,3 +628,4 @@ MODULE_LICENSE("GPL");
> MODULE_DESCRIPTION("PS3 Disk Storage Driver");
> MODULE_AUTHOR("Sony Corporation");
> MODULE_ALIAS(PS3_MODULE_ALIAS_STOR_DISK);
> +MODULE_ALIAS("ps3_storage");
NAKed-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
ps3_storage was replaced by 3 drivers: ps3disk, ps3rom, and ps3flash. These=
are
automatically loaded by udev.
How does mkinitrd determine the driver for the root file system on other
systems? Does it really contain a big list of e.g. all SCSI drivers?
With kind regards,
Geert Uytterhoeven
Software Architect
Techsoft Centre
Technology and Software Centre Europe
The Corporate Village =B7 Da Vincilaan 7-D1 =B7 B-1935 Zaventem =B7 Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 =B7 RPR Brussels
Fortis =B7 BIC GEBABEBB =B7 IBAN BE41293037680010
^ permalink raw reply
* Re: [PATCH 1/4] Add ps3_storage module alias
From: Geert Uytterhoeven @ 2010-02-05 14:39 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev
In-Reply-To: <20100205140350.GL12001@hansolo.jdub.homelinux.org>
On Fri, 5 Feb 2010, Josh Boyer wrote:
> On Fri, Feb 05, 2010 at 02:49:14PM +0100, Geert Uytterhoeven wrote:
> >On Fri, 5 Feb 2010, Josh Boyer wrote:
> >> This allows the upgrade path from older kernel releases to work
> >> with the Fedora mkinitrd tool.
> >>=20
> >> ---
> >>=20
> >> --- linux-2.6.22.ppc64/drivers/block/ps3disk.c~=092007-07-25 16:06:16.=
000000000 +0100
> >> +++ linux-2.6.22.ppc64/drivers/block/ps3disk.c=092007-07-26 08:49:44.0=
00000000 +0100
> >> @@ -628,3 +628,4 @@ MODULE_LICENSE("GPL");
> >> MODULE_DESCRIPTION("PS3 Disk Storage Driver");
> >> MODULE_AUTHOR("Sony Corporation");
> >> MODULE_ALIAS(PS3_MODULE_ALIAS_STOR_DISK);
> >> +MODULE_ALIAS("ps3_storage");
> >
> >NAKed-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> >
> >ps3_storage was replaced by 3 drivers: ps3disk, ps3rom, and ps3flash. Th=
ese are
> >automatically loaded by udev.
> >
> >How does mkinitrd determine the driver for the root file system on other
> >systems? Does it really contain a big list of e.g. all SCSI drivers?
>=20
> At one time I believe it did have a list of "odd" drivers to explicitly
> include. Anaconda didn't (doesn't?) use udev to determine which drivers
> to include in the initrd since the install image is most likely not compo=
sed
> on a machine that includes those drivers. If the install initrd didn't h=
ave
> the drivers loaded, it wouldn't install on PS3.
>=20
> This is highlighted as a Fedora local change, probably for the reasons yo=
u
> state. We might have already moved past the point where this is needed,
> so I'm fine with it being NAKed. I'll probably just drop it from the Fed=
ora
> kernel builds.
It's possible it's (still) needed on Fedora, though (Geoff, do you know?).
But it's definitely not something we want in mainline.
With kind regards,
Geert Uytterhoeven
Software Architect
Techsoft Centre
Technology and Software Centre Europe
The Corporate Village =B7 Da Vincilaan 7-D1 =B7 B-1935 Zaventem =B7 Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 =B7 RPR Brussels
Fortis =B7 BIC GEBABEBB =B7 IBAN BE41293037680010
^ permalink raw reply
* Re: PATCH: Add non-Virtex 5 support for LL TEMAC driver
From: John Tyner @ 2010-02-05 17:16 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <fa686aa41002031501q14e8ab54s6713edc58d0fddc1@mail.gmail.com>
Grant Likely wrote:
> On Mon, Feb 1, 2010 at 4:25 PM, John Tyner <jtyner@cs.ucr.edu> wrote:
>> This patch adds support for using the LL TEMAC Ethernet driver on non-Virtex
>> 5 platforms by adding support for accessing the Soft DMA registers as if
>> they were memory mapped instead of solely through the DCR's (available on
>> the Virtex 5).
>>
>> Signed-off-by: John Tyner <jtyner@cs.ucr.edu>
>
> Hi John, thanks for doing this work. A couple of comments below.
>
[snip]
>> * TODO:
>> - * - Fix driver to work on more than just Virtex5. Right now the driver
>> - * assumes that the locallink DMA registers are accessed via DCR
>> - * instructions.
>> * - Factor out locallink DMA code into separate driver
>> * - Fix multicast assignment.
>> * - Fix support for hardware checksumming.
>> @@ -117,12 +114,20 @@
>>
>> static u32 temac_dma_in32(struct temac_local *lp, int reg)
>> {
>> - return dcr_read(lp->sdma_dcrs, reg);
>> + if (lp->sdma_regs) {
>> + return __raw_readl(lp->sdma_regs + reg);
>> + } else {
>> + return dcr_read(lp->sdma_dcrs, reg);
>> + }
>
> Rather than taking the ugliness an if/else block on every register
> access, why not create an ops structure and populate it with the
> correct access routines at runtime?
As you can see from the comments in the TODO section, someone thinks
(and I would tend to agree) that it would be a good idea to separate the
DMA code from the Ethernet driver entirely. I also agree with your
suggestion of avoiding the ugliness of the if/else block, but it seems
like a job better left to be done when that refactoring takes place.
This patch is only about 20 lines worth of changes, and to add a bunch
of complexity doesn't seem worthwhile to me at this time.
>> @@ -895,7 +904,7 @@
>>
>> lp->phy_node = of_parse_phandle(op->node, "phy-handle", 0);
>> if (lp->phy_node)
>> - dev_dbg(lp->dev, "using PHY node %s (%p)\n", np->full_name, np);
>> + dev_dbg(lp->dev, "using PHY node %s (%p)\n", lp->phy_node->full_name, lp->phy_node);
>
> This looks like an unrelated bug fix. Please put into a separate
> patch and post separately.
It is an unrelated bug fix. If it's really a big deal, I can submit a
separate patch, but it seems like it would be easier for someone to
manually make this change than for me to go back and generate a new
patch (since I'm not working on anything related to this anymore).
I apologize for being kind of a dick about this stuff, but as I said,
I'm not doing anything related to this stuff anymore. The change is
really small, but adds support for something that currently doesn't
work, so I'm hoping somebody else is willing to take up the cause in my
stead. :)
John
^ permalink raw reply
* RE: [PATCH] net: emaclite: adding MDIO and phy lib support
From: John Linn @ 2010-02-05 18:50 UTC (permalink / raw)
To: John Linn, netdev, linuxppc-dev, jgarzik, grant.likely, jwboyer
Cc: Sadanand Mutyala, john.williams
In-Reply-To: <1265244560-23593-1-git-send-email-john.linn@xilinx.com>
> -----Original Message-----
> From: Grant Likely
> Sent: Thurs, 4 Feb 2010 11:12:29 AM
> To: John Linn
> Subject: FW: [PATCH] net: emaclite: adding MDIO and phy lib support
> =
> Hi John and Sadanand. Looks like a good patch, but a few issues to
> resolve. Comments below.
> =
> g.
Somehow I didn't get this message but found it out on the web, so this
is a contrived version of the message so I could respond :)
> =
> On Wed, Feb 3, 2010 at 5:49 PM, John Linn <john.linn@xxxxxxxxxx>
wrote:
> > These changes add MDIO and phy lib support to the driver as the
> > IP core now supports the MDIO bus.
> >
> > The MDIO bus and phy are added as a child to the emaclite in the
device
> > tree as illustrated below.
> >
> > mdio {
> > #address-cells =3D <1>;
> > #size-cells =3D <0>;
> > phy0: phy@7 {
> > reg =3D <7>;
> =
> For completeness, phy node need a "compatible" property.
It seems like this is in the device tree only but the driver won't use
it. So no real affect on the driver unless I'm not synced up with you.
> =
> > } ;
> > }
> >
> > Signed-off-by: Sadanand Mutyala <Sadanand.Mutyala@xxxxxxxxxx>
> > Signed-off-by: John Linn <john.linn@xxxxxxxxxx>
> > ---
> > drivers/net/Kconfig | 1 +
> > drivers/net/xilinx_emaclite.c | 362
++++++++++++++++++++++++++++++++++++-----
> > 2 files changed, 319 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> > index 396fd38..2056cd2 100644
> > --- a/drivers/net/Kconfig
> > +++ b/drivers/net/Kconfig
> > @@ -1947,6 +1947,7 @@ config ATL2
> > config XILINX_EMACLITE
> > tristate "Xilinx 10/100 Ethernet Lite support"
> > depends on PPC32 || MICROBLAZE
> > + select PHYLIB
> > help
> > This driver supports the 10/100 Ethernet Lite from Xilinx.
> >
> =
> Patch appears to be whitespace damaged. All tabs have been converted
to spaces.
Ok, will look at this.
<snip>
> > +static int xemaclite_mdio_read(struct mii_bus *bus, int phy_id, int
reg)
> > +{
> > + struct net_local *lp =3D bus->priv;
> > + u32 ctrl_reg;
> > + u32 rc;
> > +
> > + /* Wait till the device is ready */
> > + do {
> > + ctrl_reg =3D in_be32(lp->base_addr +
XEL_MDIOCTRL_OFFSET);
> > + } while (ctrl_reg & XEL_MDIOCTRL_MDIOSTS_MASK);
> =
> This is a busywait loop that just burns cycles while waiting for the
> MDIO bus to become non-busy, and further down...
Yep agreed.
> =
> > +
> > + /* Write the PHY address, register number and set the OP bit
in the
> > + * MDIO Address register. Set the Status bit in the MDIO
Control
> > + * register to start a MDIO read transaction.
> > + */
> > + out_be32(lp->base_addr + XEL_MDIOADDR_OFFSET,
> > + XEL_MDIOADDR_OP_MASK |
> > + ((phy_id << XEL_MDIOADDR_PHYADR_SHIFT) | reg));
> > + out_be32(lp->base_addr + XEL_MDIOCTRL_OFFSET,
> > + ctrl_reg | XEL_MDIOCTRL_MDIOSTS_MASK);
> > +
> > + /* Wait for the device to complete the transaction and read
the value
> > + * from MDIO Read Data register.
> > + */
> > + do {
> > + ctrl_reg =3D in_be32(lp->base_addr +
XEL_MDIOCTRL_OFFSET);
> > + } while (ctrl_reg & XEL_MDIOCTRL_MDIOSTS_MASK);
> =
> ... I see the same thing waiting for the issued transaction to
> complete. Busywaiting on slow events, like MDIO transfers, wastes a
> lot of cycles that could be used for running other threads.
> =
> At the very least, the wait loop should msleep() so that other threads
> get scheduled. Even better is if a completion is used and the MDIO
> irq can be used to wake up the thread.
> =
> Another problem with this busywait is that is has no failure path if
> the MDIO bus hangs up. The thread could get stuck spinning on this
> loop forever with no way to kill it.
> =
Agree, will fix all these, probably not use an interrupt.
<snip>
> > * Return: 0, if the driver is bound to the Emaclite device, or
> > * a negative error if there is failure.
> > @@ -853,7 +1104,6 @@ static int __devinit xemaclite_of_probe(struct
of_device *ofdev,
> > struct net_local *lp =3D NULL;
> > struct device *dev =3D &ofdev->dev;
> > const void *mac_address;
> > -
> =
> Unrelated whitespace change.
Yep.
> =
> > int rc =3D 0;
> >
> > dev_info(dev, "Device Tree Probing\n");
> > @@ -880,6 +1130,7 @@ static int __devinit xemaclite_of_probe(struct
of_device *ofdev,
> > }
> >
> > dev_set_drvdata(dev, ndev);
> > + SET_NETDEV_DEV(ndev, &ofdev->dev);
> >
> > ndev->irq =3D r_irq.start;
> > ndev->mem_start =3D r_mem.start;
> > @@ -923,7 +1174,16 @@ static int __devinit xemaclite_of_probe(struct
of_device *ofdev,
> > out_be32(lp->base_addr + XEL_BUFFER_OFFSET + XEL_TSR_OFFSET,
0);
> >
> > /* Set the MAC address in the EmacLite device */
> > - xemaclite_set_mac_address(lp, ndev->dev_addr);
> > + xemaclite_update_address(lp, ndev->dev_addr);
> > +
> > + /* Check if MDIO is included in the HW */
> > + lp->has_mdio =3D get_bool(ofdev, "xlnx,include-mdio");
> > + if (lp->has_mdio) {
> > + lp->phy_node =3D of_parse_phandle(ofdev->node,
"phy-handle", 0);
> > + rc =3D xemaclite_mdio_setup(lp, &ofdev->dev);
> > + if (rc)
> > + dev_warn(&ofdev->dev, "error registering
MDIO bus\n");
> > + }
> =
> What if the phy is attached to a different MDIO bus (which is
> completely possible)? The fetching of phy_node should be performed
> regardless of whether or not xlnx,include-mdio is set.
> =
So the issue here is that the IP core can be built without any PHY
interface and MDIO bus.
Maybe we can still do that, I'll need to test it. =
Will get a new patch ready soon.
Thanks,
John
This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.
^ permalink raw reply
* Re: [PATCH] net: emaclite: adding MDIO and phy lib support
From: Grant Likely @ 2010-02-05 20:22 UTC (permalink / raw)
To: John Linn; +Cc: linuxppc-dev, netdev, Sadanand Mutyala, jgarzik, john.williams
In-Reply-To: <a3cfa41e-7220-460d-8fce-96db4752ab08@SG2EHSMHS012.ehs.local>
On Fri, Feb 5, 2010 at 10:50 AM, John Linn <John.Linn@xilinx.com> wrote:
>> -----Original Message-----
>> From: Grant Likely
>> Sent: Thurs, 4 Feb 2010 11:12:29 AM
>> To: John Linn
>> Subject: FW: [PATCH] net: emaclite: adding MDIO and phy lib support
>>
>> Hi John and Sadanand. =A0Looks like a good patch, but a few issues to
>> resolve. =A0Comments below.
>>
>> g.
>
> Somehow I didn't get this message but found it out on the web, so this
> is a contrived version of the message so I could respond :)
>
>>
>> On Wed, Feb 3, 2010 at 5:49 PM, John Linn <john.linn@xxxxxxxxxx>
> wrote:
>> > These changes add MDIO and phy lib support to the driver as the
>> > IP core now supports the MDIO bus.
>> >
>> > The MDIO bus and phy are added as a child to the emaclite in the
> device
>> > tree as illustrated below.
>> >
>> > mdio {
>> > =A0 =A0 =A0 =A0#address-cells =3D <1>;
>> > =A0 =A0 =A0 =A0#size-cells =3D <0>;
>> > =A0 =A0 =A0 =A0phy0: phy@7 {
>> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0reg =3D <7>;
>>
>> For completeness, phy node need a "compatible" property.
>
> It seems like this is in the device tree only but the driver won't use
> it. So no real affect on the driver unless I'm not synced up with you.
That's right. That's why I said "for completeness". This is just an
example phy node, but it is good practice to give accurate examples.
:-)
>> > =A0 =A0 =A0 =A0/* Set the MAC address in the EmacLite device */
>> > - =A0 =A0 =A0 xemaclite_set_mac_address(lp, ndev->dev_addr);
>> > + =A0 =A0 =A0 xemaclite_update_address(lp, ndev->dev_addr);
>> > +
>> > + =A0 =A0 =A0 /* Check if MDIO is included in the HW */
>> > + =A0 =A0 =A0 lp->has_mdio =3D get_bool(ofdev, "xlnx,include-mdio");
>> > + =A0 =A0 =A0 if (lp->has_mdio) {
>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 lp->phy_node =3D of_parse_phandle(ofdev-=
>node,
> "phy-handle", 0);
>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 rc =3D xemaclite_mdio_setup(lp, &ofdev->=
dev);
>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (rc)
>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_warn(&ofdev->dev, "e=
rror registering
> MDIO bus\n");
>> > + =A0 =A0 =A0 }
>>
>> What if the phy is attached to a different MDIO bus (which is
>> completely possible)? =A0The fetching of phy_node should be performed
>> regardless of whether or not xlnx,include-mdio is set.
>>
>
> So the issue here is that the IP core can be built without any PHY
> interface and MDIO bus.
Right, but just because the *core* doesn't have a phy interface built
in, doesn't mean that the PHY isn't attached to a different MDIO bus
(ie. on another emaclite, or on a gpio driven MDIO bus). So even if
mdio is configured out, the driver should still handle the case of it
having an valid phy-handle.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: PATCH: Add non-Virtex 5 support for LL TEMAC driver
From: Grant Likely @ 2010-02-05 20:29 UTC (permalink / raw)
To: John Tyner; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <4B6C5271.9080802@cs.ucr.edu>
On Fri, Feb 5, 2010 at 9:16 AM, John Tyner <jtyner@cs.ucr.edu> wrote:
>
>
> Grant Likely wrote:
>>
>> On Mon, Feb 1, 2010 at 4:25 PM, John Tyner <jtyner@cs.ucr.edu> wrote:
>>>
>>> This patch adds support for using the LL TEMAC Ethernet driver on
>>> non-Virtex
>>> 5 platforms by adding support for accessing the Soft DMA registers as i=
f
>>> they were memory mapped instead of solely through the DCR's (available =
on
>>> the Virtex 5).
>>>
>>> Signed-off-by: John Tyner <jtyner@cs.ucr.edu>
>>
>> Hi John, thanks for doing this work. =A0A couple of comments below.
>>
>
> [snip]
>
>>> =A0* TODO:
>>> - * - Fix driver to work on more than just Virtex5. =A0Right now the dr=
iver
>>> - * =A0 assumes that the locallink DMA registers are accessed via DCR
>>> - * =A0 instructions.
>>> =A0* - Factor out locallink DMA code into separate driver
>>> =A0* - Fix multicast assignment.
>>> =A0* - Fix support for hardware checksumming.
>>> @@ -117,12 +114,20 @@
>>>
>>> =A0static u32 temac_dma_in32(struct temac_local *lp, int reg)
>>> =A0{
>>> - =A0 =A0 =A0 return dcr_read(lp->sdma_dcrs, reg);
>>> + =A0 =A0 =A0 if (lp->sdma_regs) {
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return __raw_readl(lp->sdma_regs + reg);
>>> + =A0 =A0 =A0 } else {
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return dcr_read(lp->sdma_dcrs, reg);
>>> + =A0 =A0 =A0 }
>>
>> Rather than taking the ugliness an if/else block on every register
>> access, why not create an ops structure and populate it with the
>> correct access routines at runtime?
>
> As you can see from the comments in the TODO section, someone thinks (and=
I
> would tend to agree) that it would be a good idea to separate the DMA cod=
e
> from the Ethernet driver entirely. I also agree with your suggestion of
> avoiding the ugliness of the if/else block, but it seems like a job bette=
r
> left to be done when that refactoring takes place.
>
> This patch is only about 20 lines worth of changes, and to add a bunch of
> complexity doesn't seem worthwhile to me at this time.
I wrote that comment, so that someone would happen to be me. :-)
Breaking the register access functions out into callbacks or an ops
callbacks structure is unrelated to the DMA refactoring task. Doing
callbacks is pretty trivial to implement and it makes things simpler,
not more complex.
>
>>> @@ -895,7 +904,7 @@
>>>
>>> =A0 =A0 =A0 =A0lp->phy_node =3D of_parse_phandle(op->node, "phy-handle"=
, 0);
>>> =A0 =A0 =A0 =A0if (lp->phy_node)
>>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_dbg(lp->dev, "using PHY node %s (%p)\=
n",
>>> np->full_name, np);
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_dbg(lp->dev, "using PHY node %s (%p)\=
n",
>>> lp->phy_node->full_name, lp->phy_node);
>>
>> This looks like an unrelated bug fix. =A0Please put into a separate
>> patch and post separately.
>
> It is an unrelated bug fix. If it's really a big deal, I can submit a
> separate patch, but it seems like it would be easier for someone to manua=
lly
> make this change than for me to go back and generate a new patch (since I=
'm
> not working on anything related to this anymore).
>
> I apologize for being kind of a dick about this stuff, but as I said, I'm
> not doing anything related to this stuff anymore. The change is really
> small, but adds support for something that currently doesn't work, so I'm
> hoping somebody else is willing to take up the cause in my stead. :)
Fair enough. Thanks for posting what you have.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* [PATCH v2 0/4] OF GPIO integration for I2C/SPI GPIO chips
From: Anton Vorontsov @ 2010-02-05 20:32 UTC (permalink / raw)
To: Grant Likely, David Brownell
Cc: Bill Gatliff, Dmitry Eremin-Solenikov, linux-kernel, linuxppc-dev,
Andrew Morton
Hi all,
Here comes v2. Changes:
- In 'gpiolib: Introduce chip addition/removal notifier', open-coded
gpio_call_chain().
- In 'of/gpio: Add support for two-stage registration for the
of_gpio_chips', improved commit message.
David, I think it would be best if these patches could go via powerpc
or OF trees, so your Ack would be highly appreciated.
Thanks!
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* [PATCH 1/4] gpiolib: Introduce chip addition/removal notifier
From: Anton Vorontsov @ 2010-02-05 20:32 UTC (permalink / raw)
To: Grant Likely, David Brownell
Cc: Bill Gatliff, Dmitry Eremin-Solenikov, linux-kernel, linuxppc-dev,
Andrew Morton
In-Reply-To: <20100205203201.GA32281@oksana.dev.rtsoft.ru>
Some platforms (e.g. OpenFirmware) want to know when a particular chip
added or removed, so that the platforms could add their specifics for
non-platform devices, like I2C or SPI GPIO chips.
This patch implements the notifier for chip addition and removal events.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/gpio/gpiolib.c | 14 ++++++++++++++
include/asm-generic/gpio.h | 8 ++++++++
2 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 350842a..375c03a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -9,6 +9,7 @@
#include <linux/seq_file.h>
#include <linux/gpio.h>
#include <linux/idr.h>
+#include <linux/notifier.h>
/* Optional implementation infrastructure for GPIO interfaces.
@@ -1029,6 +1030,9 @@ static inline void gpiochip_unexport(struct gpio_chip *chip)
#endif /* CONFIG_GPIO_SYSFS */
+BLOCKING_NOTIFIER_HEAD(gpio_notifier);
+EXPORT_SYMBOL_GPL(gpio_notifier);
+
/**
* gpiochip_add() - register a gpio_chip
* @chip: the chip to register, with chip->base initialized
@@ -1103,6 +1107,9 @@ fail:
pr_err("gpiochip_add: gpios %d..%d (%s) not registered\n",
chip->base, chip->base + chip->ngpio - 1,
chip->label ? : "generic");
+ else
+ blocking_notifier_call_chain(&gpio_notifier,
+ GPIO_NOTIFY_CHIP_ADDED, chip);
return status;
}
EXPORT_SYMBOL_GPL(gpiochip_add);
@@ -1119,6 +1126,13 @@ int gpiochip_remove(struct gpio_chip *chip)
int status = 0;
unsigned id;
+ /* Ask external subsystems to release the chip. */
+ status = blocking_notifier_call_chain(&gpio_notifier,
+ GPIO_NOTIFY_CHIP_REMOVE, chip);
+ status = notifier_to_errno(status);
+ if (status)
+ return status;
+
spin_lock_irqsave(&gpio_lock, flags);
for (id = chip->base; id < chip->base + chip->ngpio; id++) {
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 485eeb6..84faae4 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -4,6 +4,7 @@
#include <linux/kernel.h>
#include <linux/types.h>
#include <linux/errno.h>
+#include <linux/notifier.h>
#ifdef CONFIG_GPIOLIB
@@ -208,4 +209,11 @@ static inline void gpio_unexport(unsigned gpio)
}
#endif /* CONFIG_GPIO_SYSFS */
+enum gpio_notify_msg {
+ GPIO_NOTIFY_CHIP_ADDED = 0,
+ GPIO_NOTIFY_CHIP_REMOVE = 1,
+};
+
+extern struct blocking_notifier_head gpio_notifier;
+
#endif /* _ASM_GENERIC_GPIO_H */
--
1.6.5.7
^ permalink raw reply related
* [PATCH 2/4] of/gpio: Add support for two-stage registration for the of_gpio_chips
From: Anton Vorontsov @ 2010-02-05 20:32 UTC (permalink / raw)
To: Grant Likely, David Brownell
Cc: Bill Gatliff, Dmitry Eremin-Solenikov, linux-kernel, linuxppc-dev,
Andrew Morton
In-Reply-To: <20100205203201.GA32281@oksana.dev.rtsoft.ru>
I2C/SPI drivers allocate gpio_chip structure already, so we don't need
'struct gpio_chip gc' in the OF GPIO subsystem, instead we need to store
just a pointer, and then attach the already allocated gpio_chip to the
of_gpio_chip structure.
With this patch there are two ways to register OF GPIO controllers:
1. Allocating the of_gpio_chip structure and passing the
&of_gc->gc pointer to the gpiochip_add. (Can use container_of
to convert the gpio_chip to the of_gpio_chip.)
2. Allocating and registering the gpio_chip structure separately
from the of_gpio_chip. (Since two allocations are separate,
container_of won't work.)
As time goes by we'll kill the first option.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c | 1 +
drivers/of/gpio.c | 23 +++++++++++++++++++++--
include/linux/of_gpio.h | 3 ++-
3 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
index 82a9bcb..73c7e6b 100644
--- a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
+++ b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
@@ -93,6 +93,7 @@ static int mcu_gpiochip_add(struct mcu *mcu)
gc->base = -1;
gc->set = mcu_gpio_set;
gc->direction_output = mcu_gpio_dir_out;
+ of_gc->chip = gc;
of_gc->gpio_cells = 2;
of_gc->xlate = of_gpio_simple_xlate;
diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c
index 6eea601..12c4af0 100644
--- a/drivers/of/gpio.c
+++ b/drivers/of/gpio.c
@@ -70,7 +70,7 @@ int of_get_gpio_flags(struct device_node *np, int index,
if (ret < 0)
goto err1;
- ret += of_gc->gc.base;
+ ret += of_gc->chip->base;
err1:
of_node_put(gc);
err0:
@@ -140,7 +140,7 @@ int of_gpio_simple_xlate(struct of_gpio_chip *of_gc, struct device_node *np,
return -EINVAL;
}
- if (*gpio > of_gc->gc.ngpio)
+ if (*gpio > of_gc->chip->ngpio)
return -EINVAL;
if (flags)
@@ -178,6 +178,25 @@ int of_mm_gpiochip_add(struct device_node *np,
struct of_gpio_chip *of_gc = &mm_gc->of_gc;
struct gpio_chip *gc = &of_gc->gc;
+ /*
+ * Currently there are two ways to register OF GPIO controllers:
+ *
+ * 1. Allocating the of_gpio_chip structure and passing the
+ * &of_gc->gc pointer to the gpiochip_add. (Can use container_of
+ * to convert the gpio_chip to the of_gpio_chip.)
+ *
+ * 2. Allocating and registering the gpio_chip structure separately
+ * from the of_gpio_chip. (Since two allocations are separate,
+ * container_of won't work.)
+ *
+ * As time goes by we'll kill the first option. For now just check
+ * if it's the new-style registration or the old-style.
+ */
+ if (!of_gc->chip)
+ of_gc->chip = gc;
+ else
+ gc = of_gc->chip;
+
gc->label = kstrdup(np->full_name, GFP_KERNEL);
if (!gc->label)
goto err0;
diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
index fc2472c..c74cb37 100644
--- a/include/linux/of_gpio.h
+++ b/include/linux/of_gpio.h
@@ -36,7 +36,8 @@ enum of_gpio_flags {
* Generic OF GPIO chip
*/
struct of_gpio_chip {
- struct gpio_chip gc;
+ struct gpio_chip gc; /* legacy, don't use for a new code */
+ struct gpio_chip *chip;
int gpio_cells;
int (*xlate)(struct of_gpio_chip *of_gc, struct device_node *np,
const void *gpio_spec, enum of_gpio_flags *flags);
--
1.6.5.7
^ permalink raw reply related
* [PATCH 3/4] of/gpio: Implement GPIOLIB notifier hooks
From: Anton Vorontsov @ 2010-02-05 20:32 UTC (permalink / raw)
To: Grant Likely, David Brownell
Cc: Bill Gatliff, Dmitry Eremin-Solenikov, linux-kernel, linuxppc-dev,
Andrew Morton
In-Reply-To: <20100205203201.GA32281@oksana.dev.rtsoft.ru>
This patch implements GPIOLIB notifier hooks, and thus makes device-enabled
GPIO chips (i.e. the ones that have gpio_chip->dev specified) automatically
attached to the OpenFirmware subsystem. Which means that now we can handle
I2C and SPI GPIO chips almost* transparently.
* "Almost" because some chips still require platform data, and for these
chips OF-glue is still needed, though with this support the glue will
be much smaller.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/of/gpio.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 100 insertions(+), 0 deletions(-)
diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c
index 12c4af0..9d8df77 100644
--- a/drivers/of/gpio.c
+++ b/drivers/of/gpio.c
@@ -13,6 +13,7 @@
#include <linux/kernel.h>
#include <linux/errno.h>
+#include <linux/notifier.h>
#include <linux/io.h>
#include <linux/of.h>
#include <linux/of_gpio.h>
@@ -236,3 +237,102 @@ err0:
return ret;
}
EXPORT_SYMBOL(of_mm_gpiochip_add);
+
+/**
+ * of_gpiochip_register_simple - Register a chip with the OF GPIO subsystem
+ * @chip pointer to a GPIO chip
+ * @np: device node to register the GPIO chip with
+ *
+ * This function registers a GPIO chip with the OF infrastructure. It is
+ * assumed that the chip was previsously allocated and added to a generic
+ * GPIOLIB framework (using gpiochip_add() function).
+ *
+ * The `simple' name means that the chip is using simple two-cells scheme for
+ * the gpio-specifier.
+ */
+static int of_gpiochip_register_simple(struct gpio_chip *chip,
+ struct device_node *np)
+{
+ struct of_gpio_chip *of_gc;
+
+ if (np->data) {
+ WARN_ON(1);
+ return -EBUSY;
+ }
+
+ of_gc = kzalloc(sizeof(*of_gc), GFP_KERNEL);
+ if (!of_gc)
+ return -ENOMEM;
+
+ of_gc->gpio_cells = 2;
+ of_gc->xlate = of_gpio_simple_xlate;
+ of_gc->chip = chip;
+ np->data = of_gc;
+ of_node_get(np);
+
+ return 0;
+}
+EXPORT_SYMBOL(of_gpiochip_register_simple);
+
+/**
+ * of_gpiochip_unregister - Unregister a GPIO chip
+ * @chip pointer to a GPIO chip
+ * @np: device node for which the GPIO chip was previously registered
+ *
+ * This function unregisters a GPIO chip that was previsously registered
+ * with of_gpiochip_register*().
+ */
+static int of_gpiochip_unregister(struct gpio_chip *chip,
+ struct device_node *np)
+{
+ struct of_gpio_chip *of_gc = np->data;
+
+ if (!of_gc || of_gc->chip != chip) {
+ WARN_ON(1);
+ return -EINVAL;
+ }
+
+ np->data = NULL;
+ kfree(of_gc);
+ of_node_put(np);
+
+ return 0;
+}
+
+static int of_gpio_notify(struct notifier_block *nb, unsigned long msg,
+ void *chip)
+{
+ struct gpio_chip *gc = chip;
+ struct device_node *np;
+ int ret = 0;
+
+ if (!gc->dev)
+ return NOTIFY_DONE;
+
+ np = dev_archdata_get_node(&gc->dev->archdata);
+ if (!np)
+ return NOTIFY_DONE;
+
+ switch (msg) {
+ case GPIO_NOTIFY_CHIP_ADDED:
+ ret = of_gpiochip_register_simple(gc, np);
+ break;
+ case GPIO_NOTIFY_CHIP_REMOVE:
+ ret = of_gpiochip_unregister(gc, np);
+ break;
+ default:
+ break;
+ }
+
+ return ret ? notifier_from_errno(ret) : NOTIFY_OK;
+}
+
+static struct notifier_block of_gpio_nb = {
+ .notifier_call = of_gpio_notify,
+};
+
+static int __init of_gpio_notifier_init(void)
+{
+ return blocking_notifier_chain_register(&gpio_notifier, &of_gpio_nb);
+}
+arch_initcall(of_gpio_notifier_init);
--
1.6.5.7
^ permalink raw reply related
* [PATCH 4/4] powerpc/mcu_mpc8349emitx: Remove OF GPIO handling stuff
From: Anton Vorontsov @ 2010-02-05 20:32 UTC (permalink / raw)
To: Grant Likely, David Brownell
Cc: Bill Gatliff, Dmitry Eremin-Solenikov, linux-kernel, linuxppc-dev,
Andrew Morton
In-Reply-To: <20100205203201.GA32281@oksana.dev.rtsoft.ru>
With the new OF GPIO infrastructure it's much easier to handle I2C
GPIO controllers, i.e. now drivers don't have to deal with the
OF-specific bits.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c | 68 +++++-------------------
1 files changed, 14 insertions(+), 54 deletions(-)
diff --git a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
index 73c7e6b..5525175 100644
--- a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
+++ b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
@@ -18,8 +18,6 @@
#include <linux/mutex.h>
#include <linux/i2c.h>
#include <linux/gpio.h>
-#include <linux/of.h>
-#include <linux/of_gpio.h>
#include <asm/prom.h>
#include <asm/machdep.h>
@@ -36,7 +34,7 @@ struct mcu {
struct mutex lock;
struct device_node *np;
struct i2c_client *client;
- struct of_gpio_chip of_gc;
+ struct gpio_chip gc;
u8 reg_ctrl;
};
@@ -55,8 +53,7 @@ static void mcu_power_off(void)
static void mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
{
- struct of_gpio_chip *of_gc = to_of_gpio_chip(gc);
- struct mcu *mcu = container_of(of_gc, struct mcu, of_gc);
+ struct mcu *mcu = container_of(gc, struct mcu, gc);
u8 bit = 1 << (4 + gpio);
mutex_lock(&mcu->lock);
@@ -75,53 +72,6 @@ static int mcu_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
return 0;
}
-static int mcu_gpiochip_add(struct mcu *mcu)
-{
- struct device_node *np;
- struct of_gpio_chip *of_gc = &mcu->of_gc;
- struct gpio_chip *gc = &of_gc->gc;
- int ret;
-
- np = of_find_compatible_node(NULL, NULL, "fsl,mcu-mpc8349emitx");
- if (!np)
- return -ENODEV;
-
- gc->owner = THIS_MODULE;
- gc->label = np->full_name;
- gc->can_sleep = 1;
- gc->ngpio = MCU_NUM_GPIO;
- gc->base = -1;
- gc->set = mcu_gpio_set;
- gc->direction_output = mcu_gpio_dir_out;
- of_gc->chip = gc;
- of_gc->gpio_cells = 2;
- of_gc->xlate = of_gpio_simple_xlate;
-
- np->data = of_gc;
- mcu->np = np;
-
- /*
- * We don't want to lose the node, its ->data and ->full_name...
- * So, if succeeded, we don't put the node here.
- */
- ret = gpiochip_add(gc);
- if (ret)
- of_node_put(np);
- return ret;
-}
-
-static int mcu_gpiochip_remove(struct mcu *mcu)
-{
- int ret;
-
- ret = gpiochip_remove(&mcu->of_gc.gc);
- if (ret)
- return ret;
- of_node_put(mcu->np);
-
- return 0;
-}
-
static int __devinit mcu_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -141,7 +91,16 @@ static int __devinit mcu_probe(struct i2c_client *client,
goto err;
mcu->reg_ctrl = ret;
- ret = mcu_gpiochip_add(mcu);
+ mcu->gc.dev = &client->dev;
+ mcu->gc.owner = THIS_MODULE;
+ mcu->gc.label = dev_name(&client->dev);
+ mcu->gc.can_sleep = 1;
+ mcu->gc.ngpio = MCU_NUM_GPIO;
+ mcu->gc.base = -1;
+ mcu->gc.set = mcu_gpio_set;
+ mcu->gc.direction_output = mcu_gpio_dir_out;
+
+ ret = gpiochip_add(&mcu->gc);
if (ret)
goto err;
@@ -168,9 +127,10 @@ static int __devexit mcu_remove(struct i2c_client *client)
glob_mcu = NULL;
}
- ret = mcu_gpiochip_remove(mcu);
+ ret = gpiochip_remove(&mcu->gc);
if (ret)
return ret;
+
i2c_set_clientdata(client, NULL);
kfree(mcu);
return 0;
--
1.6.5.7
^ permalink raw reply related
* [PATCH RFC 0/3] Implement refcounting for OF GPIO chips
From: Anton Vorontsov @ 2010-02-05 20:49 UTC (permalink / raw)
To: Grant Likely, David Brownell
Cc: Michal Simek, devicetree-discuss, linux-kernel, linuxppc-dev,
microblaze-uclinux, David Miller
Hi all,
OF GPIO infrastructure is using dynamic GPIO bases, so it is possible
that of_get_gpio()'s returned GPIO number will be no longer valid, or
worse, it may point to an unexpected GPIO controller.
This scenario is possible:
driver A: driver B: driver C:
--------- --------- ---------
gpiochip_add()
gpio = of_get_gpio()
gpiochip_remove()
gpiochip_add()
gpio_request(gpio);
gpio_set_value(gpio);
That is, driver A assumes that it is working with GPIO from driver B,
but in practice it may disappear and driver C will take its GPIO base
number, so it will provide the same GPIO numbers.
The above situation is hard to trigger, but the issue is there
nonetheless, and so needs fixing.
Thanks,
p.s. The patches are based on top of
'[PATCH v2 0/4] OF GPIO integration for I2C/SPI GPIO chips'
http://lkml.org/lkml/2010/2/5/243
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* [PATCH 1/3] of platforms: Move common static initialization to of_node_init()
From: Anton Vorontsov @ 2010-02-05 20:50 UTC (permalink / raw)
To: Grant Likely, David Brownell
Cc: Michal Simek, devicetree-discuss, linux-kernel, linuxppc-dev,
microblaze-uclinux, David Miller
In-Reply-To: <20100205204949.GA2575@oksana.dev.rtsoft.ru>
So far of_node_init() just initializes a kref, later we'll have to
initialize other fields (for example node->data_lock).
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
arch/microblaze/kernel/prom.c | 4 ++--
arch/powerpc/kernel/prom.c | 4 ++--
arch/powerpc/platforms/iseries/vio.c | 5 +++--
arch/powerpc/platforms/pseries/dlpar.c | 7 ++++---
arch/powerpc/platforms/pseries/reconfig.c | 7 ++++---
arch/powerpc/sysdev/msi_bitmap.c | 4 ++--
arch/sparc/kernel/prom_common.c | 6 +++---
drivers/of/base.c | 10 ++++++++++
include/linux/of.h | 2 ++
9 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
index b817df1..0003453 100644
--- a/arch/microblaze/kernel/prom.c
+++ b/arch/microblaze/kernel/prom.c
@@ -30,6 +30,7 @@
#include <linux/debugfs.h>
#include <linux/irq.h>
#include <linux/lmb.h>
+#include <linux/of.h>
#include <asm/prom.h>
#include <asm/page.h>
@@ -255,7 +256,7 @@ static unsigned long __init unflatten_dt_node(unsigned long mem,
np = unflatten_dt_alloc(&mem, sizeof(struct device_node) + allocl,
__alignof__(struct device_node));
if (allnextpp) {
- memset(np, 0, sizeof(*np));
+ of_node_init(np);
np->full_name = ((char *)np) + sizeof(struct device_node);
if (new_format) {
char *p2 = np->full_name;
@@ -287,7 +288,6 @@ static unsigned long __init unflatten_dt_node(unsigned long mem,
dad->next->sibling = np;
dad->next = np;
}
- kref_init(&np->kref);
}
while (1) {
u32 sz, noff;
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 4ec3008..d8c2528 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -32,6 +32,7 @@
#include <linux/debugfs.h>
#include <linux/irq.h>
#include <linux/lmb.h>
+#include <linux/of.h>
#include <asm/prom.h>
#include <asm/rtas.h>
@@ -290,7 +291,7 @@ static unsigned long __init unflatten_dt_node(unsigned long mem,
np = unflatten_dt_alloc(&mem, sizeof(struct device_node) + allocl,
__alignof__(struct device_node));
if (allnextpp) {
- memset(np, 0, sizeof(*np));
+ of_node_init(np);
np->full_name = ((char*)np) + sizeof(struct device_node);
if (new_format) {
char *p = np->full_name;
@@ -321,7 +322,6 @@ static unsigned long __init unflatten_dt_node(unsigned long mem,
dad->next->sibling = np;
dad->next = np;
}
- kref_init(&np->kref);
}
while(1) {
u32 sz, noff;
diff --git a/arch/powerpc/platforms/iseries/vio.c b/arch/powerpc/platforms/iseries/vio.c
index 2aa8b56..5f91a96 100644
--- a/arch/powerpc/platforms/iseries/vio.c
+++ b/arch/powerpc/platforms/iseries/vio.c
@@ -83,10 +83,12 @@ static void free_property(struct property *np)
static struct device_node *new_node(const char *path,
struct device_node *parent)
{
- struct device_node *np = kzalloc(sizeof(*np), GFP_KERNEL);
+ struct device_node *np = kmalloc(sizeof(*np), GFP_KERNEL);
if (!np)
return NULL;
+
+ of_node_init(np);
np->full_name = kmalloc(strlen(path) + 1, GFP_KERNEL);
if (!np->full_name) {
kfree(np);
@@ -94,7 +96,6 @@ static struct device_node *new_node(const char *path,
}
strcpy(np->full_name, path);
of_node_set_flag(np, OF_DYNAMIC);
- kref_init(&np->kref);
np->parent = of_node_get(parent);
return np;
}
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 37bce52..72043ad 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -11,11 +11,11 @@
*/
#include <linux/kernel.h>
-#include <linux/kref.h>
#include <linux/notifier.h>
#include <linux/proc_fs.h>
#include <linux/spinlock.h>
#include <linux/cpu.h>
+#include <linux/of.h>
#include "offline_states.h"
#include <asm/prom.h>
@@ -69,10 +69,12 @@ static struct device_node *dlpar_parse_cc_node(struct cc_workarea *ccwa)
struct device_node *dn;
char *name;
- dn = kzalloc(sizeof(*dn), GFP_KERNEL);
+ dn = kmalloc(sizeof(*dn), GFP_KERNEL);
if (!dn)
return NULL;
+ of_node_init(dn);
+
/* The configure connector reported name does not contain a
* preceeding '/', so we allocate a buffer large enough to
* prepend this to the full_name.
@@ -242,7 +244,6 @@ int dlpar_attach_node(struct device_node *dn)
int rc;
of_node_set_flag(dn, OF_DYNAMIC);
- kref_init(&dn->kref);
dn->parent = derive_parent(dn->full_name);
if (!dn->parent)
return -ENOMEM;
diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
index a2305d2..a0b65b7 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -12,9 +12,9 @@
*/
#include <linux/kernel.h>
-#include <linux/kref.h>
#include <linux/notifier.h>
#include <linux/proc_fs.h>
+#include <linux/of.h>
#include <asm/prom.h>
#include <asm/machdep.h>
@@ -113,10 +113,12 @@ static int pSeries_reconfig_add_node(const char *path, struct property *proplist
struct device_node *np;
int err = -ENOMEM;
- np = kzalloc(sizeof(*np), GFP_KERNEL);
+ np = kmalloc(sizeof(*np), GFP_KERNEL);
if (!np)
goto out_err;
+ of_node_init(np);
+
np->full_name = kmalloc(strlen(path) + 1, GFP_KERNEL);
if (!np->full_name)
goto out_err;
@@ -125,7 +127,6 @@ static int pSeries_reconfig_add_node(const char *path, struct property *proplist
np->properties = proplist;
of_node_set_flag(np, OF_DYNAMIC);
- kref_init(&np->kref);
np->parent = derive_parent(path);
if (IS_ERR(np->parent)) {
diff --git a/arch/powerpc/sysdev/msi_bitmap.c b/arch/powerpc/sysdev/msi_bitmap.c
index 5a32cbe..0f259ff 100644
--- a/arch/powerpc/sysdev/msi_bitmap.c
+++ b/arch/powerpc/sysdev/msi_bitmap.c
@@ -10,6 +10,7 @@
#include <linux/kernel.h>
#include <linux/bitmap.h>
+#include <linux/of.h>
#include <asm/msi_bitmap.h>
int msi_bitmap_alloc_hwirqs(struct msi_bitmap *bmp, int num)
@@ -199,8 +200,7 @@ void __init test_of_node(void)
DECLARE_BITMAP(expected, size);
/* There should really be a struct device_node allocator */
- memset(&of_node, 0, sizeof(of_node));
- kref_init(&of_node.kref);
+ of_node_init(&of_node);
of_node.full_name = node_name;
check(0 == msi_bitmap_alloc(&bmp, size, &of_node));
diff --git a/arch/sparc/kernel/prom_common.c b/arch/sparc/kernel/prom_common.c
index d80a65d..dbfad05 100644
--- a/arch/sparc/kernel/prom_common.c
+++ b/arch/sparc/kernel/prom_common.c
@@ -229,11 +229,11 @@ static struct device_node * __init prom_create_node(phandle node,
return NULL;
dp = prom_early_alloc(sizeof(*dp));
- dp->unique_id = prom_unique_id++;
- dp->parent = parent;
- kref_init(&dp->kref);
+ of_node_init(dp);
+ dp->unique_id = prom_unique_id++;
+ dp->parent = parent;
dp->name = get_one_property(node, "name");
dp->type = get_one_property(node, "device_type");
dp->node = node;
diff --git a/drivers/of/base.c b/drivers/of/base.c
index e6627b2..716d439 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -28,6 +28,16 @@ struct device_node *allnodes;
*/
DEFINE_RWLOCK(devtree_lock);
+/**
+ * of_node_init - Initialize a device node
+ * @n: Node to initialize
+ */
+void of_node_init(struct device_node *np)
+{
+ memset(np, 0, sizeof(*np));
+ kref_init(&np->kref);
+}
+
int of_n_addr_cells(struct device_node *np)
{
const int *ip;
diff --git a/include/linux/of.h b/include/linux/of.h
index e7facd8..717d690 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -63,6 +63,8 @@ struct device_node {
#endif
};
+extern void of_node_init(struct device_node *np);
+
static inline int of_node_check_flag(struct device_node *n, unsigned long flag)
{
return test_bit(flag, &n->_flags);
--
1.6.5.7
^ permalink raw reply related
* [PATCH 2/3] of: Introduce safe accessors for node->data
From: Anton Vorontsov @ 2010-02-05 20:50 UTC (permalink / raw)
To: Grant Likely, David Brownell
Cc: Michal Simek, devicetree-discuss, linux-kernel, linuxppc-dev,
microblaze-uclinux, David Miller
In-Reply-To: <20100205204949.GA2575@oksana.dev.rtsoft.ru>
Platform code use node->data to store some private information
associated with a node.
Previously there was no need for any locks and accessors since we were
initializing the data mostly at boot time and never modified it later.
Though, nowadays OF GPIO infrastructure supports GPIO chips detaching,
so to handle this correctly we have to introduce locking for the
node->data field.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/of/base.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/of.h | 5 +++++
2 files changed, 50 insertions(+), 0 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 716d439..41ed6ba 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -36,8 +36,53 @@ void of_node_init(struct device_node *np)
{
memset(np, 0, sizeof(*np));
kref_init(&np->kref);
+ spin_lock_init(&np->data_lock);
}
+/**
+ * of_node_set_data - Try to set node->data
+ * @node: Node
+ * @data: Data
+ *
+ * This function tries to safely set node->data. Returns 0 on success,
+ * -EBUSY if node->data was already set.
+ */
+int of_node_set_data(struct device_node *np, void *data)
+{
+ int ret = 0;
+ unsigned long flags;
+
+ spin_lock_irqsave(&np->data_lock, flags);
+ if (np->data)
+ ret = -EBUSY;
+ else
+ np->data = data;
+ spin_unlock_irqrestore(&np->data_lock, flags);
+
+ if (!ret)
+ of_node_get(np);
+
+ return ret;
+}
+EXPORT_SYMBOL(of_node_set_data);
+
+/**
+ * of_node_release_data_unlocked - Release node->data
+ * @node: Node
+ *
+ * This function releases node->data, so that others could set it.
+ *
+ * This function doesn't grab any locks, so that a caller can grab the
+ * lock itself, atomically read the data and decide if it wants to
+ * release it.
+ */
+void of_node_release_data_unlocked(struct device_node *np)
+{
+ np->data = NULL;
+ of_node_put(np);
+}
+EXPORT_SYMBOL(of_node_release_data_unlocked);
+
int of_n_addr_cells(struct device_node *np)
{
const int *ip;
diff --git a/include/linux/of.h b/include/linux/of.h
index 717d690..c573ee2 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -18,6 +18,7 @@
#include <linux/types.h>
#include <linux/bitops.h>
#include <linux/kref.h>
+#include <linux/spinlock.h>
#include <linux/mod_devicetable.h>
typedef u32 phandle;
@@ -56,6 +57,7 @@ struct device_node {
struct kref kref;
unsigned long _flags;
void *data;
+ spinlock_t data_lock;
#if defined(CONFIG_SPARC)
char *path_component_name;
unsigned int unique_id;
@@ -65,6 +67,9 @@ struct device_node {
extern void of_node_init(struct device_node *np);
+extern int of_node_set_data(struct device_node *np, void *data);
+extern void of_node_release_data_unlocked(struct device_node *np);
+
static inline int of_node_check_flag(struct device_node *n, unsigned long flag)
{
return test_bit(flag, &n->_flags);
--
1.6.5.7
^ permalink raw reply related
* [PATCH 3/3] of/gpio: Introduce of_put_gpio(), add ref counting for OF GPIO chips
From: Anton Vorontsov @ 2010-02-05 20:50 UTC (permalink / raw)
To: Grant Likely, David Brownell
Cc: Michal Simek, devicetree-discuss, linux-kernel, linuxppc-dev,
microblaze-uclinux, David Miller
In-Reply-To: <20100205204949.GA2575@oksana.dev.rtsoft.ru>
OF GPIO infrastructure is using dynamic GPIO bases, so it is possible
that of_get_gpio()'s returned GPIO number will be no longer valid, or
worse, it may point to an unexpected GPIO controller.
This scenario is possible:
driver A: driver B: driver C:
--------- --------- ---------
gpiochip_add()
gpio = of_get_gpio()
gpiochip_remove()
gpiochip_add()
gpio_request(gpio);
gpio_set_value(gpio);
That is, driver A assumes that it is working with GPIO from driver B,
but in practice it may disappear and driver C will take its GPIO base
number, so it will provide the same GPIO numbers.
With this patch that situation is no longer possible. Though drivers
will need to learn to put GPIOs back, so that GPIO controllers could
be removed.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/of/gpio.c | 82 ++++++++++++++++++++++++++++++++++++++++-------
include/linux/of_gpio.h | 5 +++
2 files changed, 75 insertions(+), 12 deletions(-)
diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c
index 9d8df77..e94c5c8 100644
--- a/drivers/of/gpio.c
+++ b/drivers/of/gpio.c
@@ -28,6 +28,8 @@
* Returns GPIO number to use with Linux generic GPIO API, or one of the errno
* value on the error condition. If @flags is not NULL the function also fills
* in flags for the GPIO.
+ *
+ * Remeber to put the GPIO back using of_put_gpio() call.
*/
int of_get_gpio_flags(struct device_node *np, int index,
enum of_gpio_flags *flags)
@@ -46,6 +48,8 @@ int of_get_gpio_flags(struct device_node *np, int index,
goto err0;
}
+ spin_lock(&gc->data_lock);
+
of_gc = gc->data;
if (!of_gc) {
pr_debug("%s: gpio controller %s isn't registered\n",
@@ -72,15 +76,62 @@ int of_get_gpio_flags(struct device_node *np, int index,
goto err1;
ret += of_gc->chip->base;
+
+ if (!try_module_get(of_gc->chip->owner)) {
+ ret = -EINVAL;
+ goto err1;
+ }
+
+ of_gc->refcnt++;
err1:
+ spin_unlock(&gc->data_lock);
+
of_node_put(gc);
err0:
pr_debug("%s exited with status %d\n", __func__, ret);
+
return ret;
}
EXPORT_SYMBOL(of_get_gpio_flags);
/**
+ * of_put_gpio - Put a GPIO back to the OF subsystem
+ * @np: device node of the GPIO owner
+ * @index: index of the GPIO
+ */
+static inline void of_put_gpio(struct device_node *np, int index)
+{
+ int ret;
+ struct device_node *gc;
+ struct of_gpio_chip *of_gc;
+
+ ret = of_parse_phandles_with_args(np, "gpios", "#gpio-cells", index,
+ &gc, NULL);
+ if (ret) {
+ pr_debug("%s: can't parse gpios property\n", __func__);
+ return;
+ }
+
+ spin_lock(&gc->data_lock);
+
+ of_gc = gc->data;
+ if (!of_gc) {
+ pr_debug("%s: gpio controller %s isn't registered\n",
+ np->full_name, gc->full_name);
+ goto err;
+ }
+
+ if (of_gc->refcnt)
+ of_gc->refcnt--;
+ else
+ WARN_ON(1);
+
+ module_put(of_gc->chip->owner);
+err:
+ spin_unlock(&gc->data_lock);
+}
+
+/**
* of_gpio_count - Count GPIOs for a device
* @np: device node to count GPIOs for
*
@@ -254,11 +305,7 @@ static int of_gpiochip_register_simple(struct gpio_chip *chip,
struct device_node *np)
{
struct of_gpio_chip *of_gc;
-
- if (np->data) {
- WARN_ON(1);
- return -EBUSY;
- }
+ int ret;
of_gc = kzalloc(sizeof(*of_gc), GFP_KERNEL);
if (!of_gc)
@@ -267,10 +314,12 @@ static int of_gpiochip_register_simple(struct gpio_chip *chip,
of_gc->gpio_cells = 2;
of_gc->xlate = of_gpio_simple_xlate;
of_gc->chip = chip;
- np->data = of_gc;
- of_node_get(np);
- return 0;
+ ret = of_node_set_data(np, of_gc);
+ if (ret)
+ kfree(of_gc);
+
+ return ret;
}
EXPORT_SYMBOL(of_gpiochip_register_simple);
@@ -286,17 +335,26 @@ static int of_gpiochip_unregister(struct gpio_chip *chip,
struct device_node *np)
{
struct of_gpio_chip *of_gc = np->data;
+ int ret = 0;
if (!of_gc || of_gc->chip != chip) {
WARN_ON(1);
return -EINVAL;
}
- np->data = NULL;
- kfree(of_gc);
- of_node_put(np);
+ spin_lock(&np->data_lock);
- return 0;
+ if (of_gc->refcnt)
+ ret = -EBUSY;
+ else
+ of_node_release_data_unlocked(np);
+
+ spin_unlock(&np->data_lock);
+
+ if (!ret)
+ kfree(of_gc);
+
+ return ret;
}
static int of_gpio_notify(struct notifier_block *nb, unsigned long msg,
diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
index c74cb37..aca7ab1 100644
--- a/include/linux/of_gpio.h
+++ b/include/linux/of_gpio.h
@@ -38,6 +38,7 @@ enum of_gpio_flags {
struct of_gpio_chip {
struct gpio_chip gc; /* legacy, don't use for a new code */
struct gpio_chip *chip;
+ unsigned int refcnt;
int gpio_cells;
int (*xlate)(struct of_gpio_chip *of_gc, struct device_node *np,
const void *gpio_spec, enum of_gpio_flags *flags);
@@ -83,6 +84,8 @@ static inline int of_get_gpio_flags(struct device_node *np, int index,
return -ENOSYS;
}
+static inline void of_put_gpio(struct device_node *np, int index) {}
+
static inline unsigned int of_gpio_count(struct device_node *np)
{
return 0;
@@ -97,6 +100,8 @@ static inline unsigned int of_gpio_count(struct device_node *np)
*
* Returns GPIO number to use with Linux generic GPIO API, or one of the errno
* value on the error condition.
+ *
+ * Remeber to put the GPIO back using of_put_gpio() call.
*/
static inline int of_get_gpio(struct device_node *np, int index)
{
--
1.6.5.7
^ permalink raw reply related
* [PATCH] leds-gpio: Fix default state handling on OF platforms
From: Anton Vorontsov @ 2010-02-05 20:54 UTC (permalink / raw)
To: Andrew Morton; +Cc: linuxppc-dev, Trent Piepho, linux-kernel, Richard Purdie
The driver wrongly sets default state for LEDs that don't specify
default-state property.
Currently the driver handles default state this way:
memset(&led, 0, sizeof(led));
for_each_child_of_node(np, child) {
state = of_get_property(child, "default-state", NULL);
if (state) {
if (!strcmp(state, "keep"))
led.default_state = LEDS_GPIO_DEFSTATE_KEEP;
...
}
ret = create_gpio_led(&led, ...);
}
Which means that all LEDs that do not specify default-state will
inherit the last value of the default-state property, which is wrong.
This patch fixes the issue by moving LED's template initialization
into the loop body.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/leds/leds-gpio.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index e5225d2..0823e26 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -211,7 +211,6 @@ static int __devinit of_gpio_leds_probe(struct of_device *ofdev,
const struct of_device_id *match)
{
struct device_node *np = ofdev->node, *child;
- struct gpio_led led;
struct gpio_led_of_platform_data *pdata;
int count = 0, ret;
@@ -226,8 +225,8 @@ static int __devinit of_gpio_leds_probe(struct of_device *ofdev,
if (!pdata)
return -ENOMEM;
- memset(&led, 0, sizeof(led));
for_each_child_of_node(np, child) {
+ struct gpio_led led = {};
enum of_gpio_flags flags;
const char *state;
--
1.6.5.7
^ permalink raw reply related
* Re: [PATCH 4/4] Fix iMac iSight PCI bridge setup
From: Benjamin Herrenschmidt @ 2010-02-05 20:57 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev
In-Reply-To: <20100205135546.GJ12001@hansolo.jdub.homelinux.org>
On Fri, 2010-02-05 at 08:55 -0500, Josh Boyer wrote:
> This works around the PCIe bridge setup on the iMac iSight boxen.
Is this still needed ? I though I had fixed the root cause (incorrect
setup of the bridge) a while back...
Cheers,
Ben.
> ---
>
> --- linux/arch/powerpc/platforms/powermac/pci.c~ 2008-03-22 19:08:07.000000000 +0000
> +++ linux/arch/powerpc/platforms/powermac/pci.c 2008-03-23 09:10:46.000000000 +0000
> @@ -1271,6 +1271,12 @@ void pmac_pci_fixup_pciata(struct pci_de
> }
> }
> DECLARE_PCI_FIXUP_EARLY(PCI_ANY_ID, PCI_ANY_ID, pmac_pci_fixup_pciata);
> +#else /* CONFIG_PPC64 */
> +static void __devinit imac_transparent_bridge(struct pci_dev *dev)
> +{
> + dev->transparent = 1;
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, 0x005b, imac_transparent_bridge);
> #endif /* CONFIG_PPC32 */
>
> /*
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* [PATCHv4 1/2] sched: enable ARCH_POWER
From: Joel Schopp @ 2010-02-05 20:57 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: ego, linux-kernel, Ingo Molnar, linuxppc-dev
In-Reply-To: <1264720833.9660.21.camel@jschopp-laptop>
Enable the scheduler feature that allows use of arch_scale_smt_power. Stub out
the broken x86 implementation.
Signed-off-by: Joel Schopp <jschopp@austin.ibm.com>
---
Index: linux-2.6.git/kernel/sched_features.h
===================================================================
--- linux-2.6.git.orig/kernel/sched_features.h
+++ linux-2.6.git/kernel/sched_features.h
@@ -102,7 +102,7 @@ SCHED_FEAT(CACHE_HOT_BUDDY, 1)
/*
* Use arch dependent cpu power functions
*/
-SCHED_FEAT(ARCH_POWER, 0)
+SCHED_FEAT(ARCH_POWER, 1)
SCHED_FEAT(HRTICK, 0)
SCHED_FEAT(DOUBLE_TICK, 0)
Index: linux-2.6.git/arch/x86/kernel/cpu/sched.c
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/cpu/sched.c
+++ linux-2.6.git/arch/x86/kernel/cpu/sched.c
@@ -44,11 +44,9 @@ unsigned long arch_scale_freq_power(stru
unsigned long arch_scale_smt_power(struct sched_domain *sd, int cpu)
{
/*
- * aperf/mperf already includes the smt gain
+ * aperf/mperf already includes the smt gain, but represents capacity
+ * as 0 when idle. So for now just return default.
*/
- if (boot_cpu_has(X86_FEATURE_APERFMPERF))
- return SCHED_LOAD_SCALE;
-
return default_scale_smt_power(sd, cpu);
}
^ permalink raw reply
* [PATCHv4 0/2] sched: arch_scale_smt_powers
From: Joel Schopp @ 2010-02-05 20:57 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: ego, linux-kernel, Ingo Molnar, linuxppc-dev
In-Reply-To: <1264720808.9660.18.camel@jschopp-laptop>
The new Power7 processor has 4 way SMT. This 4 way SMT benefits from
dynamic power updates that arch_scale_smt_power was designed to provide.
The first patch fixes a generic scheduler bug necessary for arch_scale_smt
to properly function. The second patch implements arch_scale_smt_power
for powerpc, and in particular for Power7 processors.
Version 2 changes:
- Drop Patch 1 from the original series since it's in the -tip tree now
- Move enabling the cpu feature into it's own patch (now patch 1)
- Add stubbing out broken x86 implementation to patch 1
- clean up coding style in patch 2
Version 3 changes:
- Added a comment to Patch 2
Version 4 changes:
- Wrap arch_scale_smt_power() in #ifdef CONFIG_SCHED_SMT in Patch 2
---
Joel Schopp (2):
sched: enable ARCH_POWER
powerpc: implement arch_scale_smt_power for Power7
arch/powerpc/include/asm/cputable.h | 3 +
arch/powerpc/kernel/smp.c | 58
++++++++++++++++++++++++++++++++++++
arch/x86/kernel/cpu/sched.c | 6 +--
kernel/sched_features.h | 2 -
4 files changed, 63 insertions(+), 6 deletions(-)
^ permalink raw reply
* [PATCHv4 2/2] powerpc: implement arch_scale_smt_power for Power7
From: Joel Schopp @ 2010-02-05 20:57 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: ego, linux-kernel, Ingo Molnar, linuxppc-dev
In-Reply-To: <1264721088.10385.1.camel@jschopp-laptop>
On Power7 processors running in SMT4 mode with 2, 3, or 4 idle threads
there is performance benefit to idling the higher numbered threads in
the core.
This patch implements arch_scale_smt_power to dynamically update smt
thread power in these idle cases in order to prefer threads 0,1 over
threads 2,3 within a core.
Signed-off-by: Joel Schopp <jschopp@austin.ibm.com>
---
Version 3 adds the #ifdef to avoid compiling on kernels that don't need it
Index: linux-2.6.git/arch/powerpc/kernel/smp.c
===================================================================
--- linux-2.6.git.orig/arch/powerpc/kernel/smp.c
+++ linux-2.6.git/arch/powerpc/kernel/smp.c
@@ -620,3 +620,61 @@ void __cpu_die(unsigned int cpu)
smp_ops->cpu_die(cpu);
}
#endif
+
+#ifdef CONFIG_SCHED_SMT
+unsigned long arch_scale_smt_power(struct sched_domain *sd, int cpu)
+{
+ int sibling;
+ int idle_count = 0;
+ int thread;
+
+ /* Setup the default weight and smt_gain used by most cpus for SMT
+ * Power. Doing this right away covers the default case and can be
+ * used by cpus that modify it dynamically.
+ */
+ struct cpumask *sibling_map = sched_domain_span(sd);
+ unsigned long weight = cpumask_weight(sibling_map);
+ unsigned long smt_gain = sd->smt_gain;
+
+
+ if (cpu_has_feature(CPU_FTR_ASYNC_SMT4) && weight == 4) {
+ for_each_cpu(sibling, sibling_map) {
+ if (idle_cpu(sibling))
+ idle_count++;
+ }
+
+ /* the following section attempts to tweak cpu power based
+ * on current idleness of the threads dynamically at runtime
+ */
+ if (idle_count > 1) {
+ thread = cpu_thread_in_core(cpu);
+ if (thread < 2) {
+ /* add 75 % to thread power */
+ smt_gain += (smt_gain >> 1) + (smt_gain >> 2);
+ } else {
+ /* subtract 75 % to thread power */
+ smt_gain = smt_gain >> 2;
+ }
+ }
+ }
+
+ /* default smt gain is 1178, weight is # of SMT threads */
+ switch (weight) {
+ case 1:
+ /*divide by 1, do nothing*/
+ break;
+ case 2:
+ smt_gain = smt_gain >> 1;
+ break;
+ case 4:
+ smt_gain = smt_gain >> 2;
+ break;
+ default:
+ smt_gain /= weight;
+ break;
+ }
+
+ return smt_gain;
+
+}
+#endif
Index: linux-2.6.git/arch/powerpc/include/asm/cputable.h
===================================================================
--- linux-2.6.git.orig/arch/powerpc/include/asm/cputable.h
+++ linux-2.6.git/arch/powerpc/include/asm/cputable.h
@@ -195,6 +195,7 @@ extern const char *powerpc_base_platform
#define CPU_FTR_SAO LONG_ASM_CONST(0x0020000000000000)
#define CPU_FTR_CP_USE_DCBTZ LONG_ASM_CONST(0x0040000000000000)
#define CPU_FTR_UNALIGNED_LD_STD LONG_ASM_CONST(0x0080000000000000)
+#define CPU_FTR_ASYNC_SMT4 LONG_ASM_CONST(0x0100000000000000)
#ifndef __ASSEMBLY__
@@ -409,7 +410,7 @@ extern const char *powerpc_base_platform
CPU_FTR_MMCRA | CPU_FTR_SMT | \
CPU_FTR_COHERENT_ICACHE | CPU_FTR_LOCKLESS_TLBIE | \
CPU_FTR_PURR | CPU_FTR_SPURR | CPU_FTR_REAL_LE | \
- CPU_FTR_DSCR | CPU_FTR_SAO)
+ CPU_FTR_DSCR | CPU_FTR_SAO | CPU_FTR_ASYNC_SMT4)
#define CPU_FTRS_CELL (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
^ permalink raw reply
* [PATCH] powerpc/85xx: Add NOR, LEDs and PIB support for MPC8568E-MDS boards
From: Anton Vorontsov @ 2010-02-05 21:06 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
This patch adds NOR Flash, LEDs and PIB support for MPC8568E-MDS
boards. Plus, move bcsr node into localbus node, and add bcsr5
gpio-controller node.
Some platform code modifications were also needed.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
arch/powerpc/boot/dts/mpc8568mds.dts | 65 +++++++++++++++++++++++++++-
arch/powerpc/platforms/85xx/mpc85xx_mds.c | 3 +
2 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/boot/dts/mpc8568mds.dts b/arch/powerpc/boot/dts/mpc8568mds.dts
index 6d892ba..92fb178 100644
--- a/arch/powerpc/boot/dts/mpc8568mds.dts
+++ b/arch/powerpc/boot/dts/mpc8568mds.dts
@@ -54,9 +54,52 @@
reg = <0x0 0x10000000>;
};
- bcsr@f8000000 {
- compatible = "fsl,mpc8568mds-bcsr";
- reg = <0xf8000000 0x8000>;
+ localbus@e0005000 {
+ #address-cells = <2>;
+ #size-cells = <1>;
+ compatible = "fsl,mpc8568-localbus", "fsl,pq3-localbus",
+ "simple-bus";
+ reg = <0xe0005000 0x1000>;
+
+ ranges = <0x0 0x0 0xfe000000 0x02000000
+ 0x1 0x0 0xf8000000 0x00008000
+ 0x2 0x0 0xf0000000 0x04000000
+ 0x4 0x0 0xf8008000 0x00008000
+ 0x5 0x0 0xf8010000 0x00008000>;
+
+ nor@0,0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "cfi-flash";
+ reg = <0x0 0x0 0x02000000>;
+ bank-width = <2>;
+ device-width = <2>;
+ };
+
+ bcsr@1,0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "fsl,mpc8568mds-bcsr";
+ reg = <1 0 0x8000>;
+ ranges = <0 1 0 0x8000>;
+
+ bcsr5: gpio-controller@11 {
+ #gpio-cells = <2>;
+ compatible = "fsl,mpc8568mds-bcsr-gpio";
+ reg = <0x5 0x1>;
+ gpio-controller;
+ };
+ };
+
+ pib@4,0 {
+ compatible = "fsl,mpc8568mds-pib";
+ reg = <4 0 0x8000>;
+ };
+
+ pib@5,0 {
+ compatible = "fsl,mpc8568mds-pib";
+ reg = <5 0 0x8000>;
+ };
};
soc8568@e0000000 {
@@ -610,4 +653,20 @@
sleep = <&pmc 0x00080000 /* controller */
&pmc 0x00040000>; /* message unit */
};
+
+ leds {
+ compatible = "gpio-leds";
+
+ green {
+ gpios = <&bcsr5 1 0>;
+ };
+
+ amber {
+ gpios = <&bcsr5 2 0>;
+ };
+
+ red {
+ gpios = <&bcsr5 3 0>;
+ };
+ };
};
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
index 21f61b8..04af81e 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
@@ -302,11 +302,14 @@ static struct of_device_id mpc85xx_ids[] = {
{ .compatible = "gianfar", },
{ .compatible = "fsl,rapidio-delta", },
{ .compatible = "fsl,mpc8548-guts", },
+ { .compatible = "gpio-leds", },
{},
};
static int __init mpc85xx_publish_devices(void)
{
+ if (machine_is(mpc8568_mds))
+ simple_gpiochip_init("fsl,mpc8568mds-bcsr-gpio");
if (machine_is(mpc8569_mds))
simple_gpiochip_init("fsl,mpc8569mds-bcsr-gpio");
--
1.6.5.7
^ permalink raw reply related
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