* Re: [v1 PATCH] ucc_geth: fix ethtool set ring param bug
From: Ben Hutchings @ 2010-09-02 16:32 UTC (permalink / raw)
To: Liang Li; +Cc: netdev, avorontsov, davem, linuxppc-dev
In-Reply-To: <20100902154818.GA30293@localhost>
On Thu, 2010-09-02 at 23:48 +0800, Liang Li wrote:
> On Thu, Sep 02, 2010 at 12:11:47PM +0100, Ben Hutchings wrote:
> > On Thu, 2010-09-02 at 08:50 +0800, Liang Li wrote:
> > > On Wed, Sep 01, 2010 at 02:42:30PM +0100, Ben Hutchings wrote:
> > > > On Wed, 2010-09-01 at 09:43 +0800, Liang Li wrote:
> > > > > It's common sense that when we should do change to driver ring
> > > > > desc/buffer etc only after 'stop/shutdown' the device. When we
> > > > > do change while devices/driver is running, kernel oops occur:
> > > > [...]
> > > > > - ug_info->bdRingLenRx[queue] = ring->rx_pending;
> > > > > - ug_info->bdRingLenTx[queue] = ring->tx_pending;
> > > > > -
> > > > > if (netif_running(netdev)) {
> > > > > - /* FIXME: restart automatically */
> > > > > - printk(KERN_INFO
> > > > > - "Please re-open the interface.\n");
> > > > > + u16 rx_t;
> > > > > + u16 tx_t;
> > > > > + printk(KERN_INFO "Stopping interface %s.\n", netdev->name);
> > > > > + ucc_geth_close(netdev);
> > > > > +
> > > > > + rx_t = ug_info->bdRingLenRx[queue];
> > > > > + tx_t = ug_info->bdRingLenTx[queue];
> > > > > +
> > > > > + ug_info->bdRingLenRx[queue] = ring->rx_pending;
> > > > > + ug_info->bdRingLenTx[queue] = ring->tx_pending;
> > > > > +
> > > > > + printk(KERN_INFO "Reactivating interface %s.\n", netdev->name);
> > > > > + ret = ucc_geth_open(netdev);
> > > > > + if (ret) {
> > > > > + printk(KERN_WARNING "uec_set_ringparam: set ring param for running"
> > > > > + " interface %s failed. Please try to make the interface "
> > > > > + " down, then try again.\n", netdev->name);
> > > > > + ug_info->bdRingLenRx[queue] = rx_t;
> > > > > + ug_info->bdRingLenTx[queue] = tx_t;
> > > > > + }
> > > > [...]
> > > >
> > > > Bringing the interface down will call ucc_geth_close(), which will try
> > > > to free resources that have not been allocated!
> > >
> > > Sorry, I did not understand you on this point. There is no
> > > ucc_geth_close when 'open fail'. What you mean here exactly?
> > [...]
> >
> > Read your own warning.
>
> So the warning is mis-leading... We may not need it... Still, let's talk
> about the rare case that 'fail of open', may you please show me what is
> the right thing to do 'clean up work' when ever the open fail. Maybe
> dev_close(net_dev)?
I give up.
Just s/ucc_geth_open/dev_open/ and s/ucc_geth_close/dev_close/.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [v2 PATCH] ucc_geth: fix ethtool set ring param bug
From: Liang Li @ 2010-09-02 16:02 UTC (permalink / raw)
To: leoli, davem, avorontsov, netdev, linuxppc-dev
In-Reply-To: <1283305429-8426-1-git-send-email-liang.li@windriver.com>
It's common sense that when we should do change to driver ring
desc/buffer etc only after 'stop/shutdown' the device. When we
do change while devices/driver is running, kernel oops occur:
[
root@fsl_8569mds:/root> ethtool -G eth0 tx 256
root@fsl_8569mds:/root> Oops: Kernel access of bad area, sig: 11 [#1]
MPC8569 MDS
last sysfs file: /sys/kernel/uevent_seqnum
Modules linked in:
NIP: 00000000 LR: c0072fbc CTR: 00000000
REGS: effefef0 TRAP: 0400 Not tainted (2.6.36-rc3-00002-g6c3b118-dirty)
MSR: 00021000 <ME,CE> CR: 24442048 XER: 00000000
TASK = c0518350[0] 'swapper' THREAD: c0544000
GPR00: 00000000 effeffa0 c0518350 00000020 ef0be000 ef005000 80000000 00000200
GPR08: c03b5d00 00000000 f1010080 ef08d458 000dda96 00000000 3ffb2900 00000000
GPR16: 00000000 3ffa8948 3fff1314 3ffac3f8 00000000 00000000 00000000 00000000
GPR24: 00000000 00000000 00000000 c0530000 00000000 00000000 00000020 ef3709c0
NIP [00000000] (null)
LR [c0072fbc] handle_IRQ_event+0x4c/0x12c
Call Trace:
[effeffa0] [c0019414] qe_ic_mask_irq+0x1c/0x90 (unreliable)
[effeffc0] [c0075b74] handle_level_irq+0x88/0x128
[effeffd0] [c001ca44] qe_ic_cascade_muxed_mpic+0x50/0x88
[effefff0] [c000d5fc] call_handle_irq+0x18/0x28
[c0545ea0] [c0004f3c] do_IRQ+0xa8/0x140
[c0545ed0] [c000e2bc] ret_from_except+0x0/0x18
-- Exception: 501 at cpu_idle+0x9c/0xdc
LR = cpu_idle+0x9c/0xdc
[c0545f90] [c0008318] cpu_idle+0x54/0xdc (unreliable)
[c0545fb0] [c000231c] rest_init+0x68/0x7c
[c0545fc0] [c04e686c] start_kernel+0x230/0x2b0
[c0545ff0] [c000039c] skpinv+0x2b4/0x2f0
Instruction dump:
XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
Kernel panic - not syncing: Fatal exception in interrupt
Rebooting in 180 seconds..
]
Then the natural solution would be 'stop driver/device then adjust
ring buffer parameter then reactivate driver/device'.
v1: add roll back state for 'auto reopen fail'
v2: just do dev_close when 'reopen fail'
Signed-off-by: Liang Li <liang.li@windriver.com>
---
drivers/net/ucc_geth.c | 4 ++--
drivers/net/ucc_geth.h | 2 ++
drivers/net/ucc_geth_ethtool.c | 27 +++++++++++++++++++++------
3 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 7d2e33a..5eadfa3 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3485,7 +3485,7 @@ err:
/* Called when something needs to use the ethernet device */
/* Returns 0 for success. */
-static int ucc_geth_open(struct net_device *dev)
+int ucc_geth_open(struct net_device *dev)
{
struct ucc_geth_private *ugeth = netdev_priv(dev);
int err;
@@ -3542,7 +3542,7 @@ err:
}
/* Stops the kernel queue, and halts the controller */
-static int ucc_geth_close(struct net_device *dev)
+int ucc_geth_close(struct net_device *dev)
{
struct ucc_geth_private *ugeth = netdev_priv(dev);
diff --git a/drivers/net/ucc_geth.h b/drivers/net/ucc_geth.h
index 05a9558..1cfb400 100644
--- a/drivers/net/ucc_geth.h
+++ b/drivers/net/ucc_geth.h
@@ -1235,5 +1235,7 @@ int init_flow_control_params(u32 automatic_flow_control_mode,
u32 __iomem *upsmr_register, u32 __iomem *uempr_register,
u32 __iomem *maccfg1_register);
+extern int ucc_geth_open(struct net_device *);
+extern int ucc_geth_close(struct net_device *);
#endif /* __UCC_GETH_H__ */
diff --git a/drivers/net/ucc_geth_ethtool.c b/drivers/net/ucc_geth_ethtool.c
index 6f92e48..2a615ee 100644
--- a/drivers/net/ucc_geth_ethtool.c
+++ b/drivers/net/ucc_geth_ethtool.c
@@ -255,13 +255,28 @@ uec_set_ringparam(struct net_device *netdev,
return -EINVAL;
}
- ug_info->bdRingLenRx[queue] = ring->rx_pending;
- ug_info->bdRingLenTx[queue] = ring->tx_pending;
-
if (netif_running(netdev)) {
- /* FIXME: restart automatically */
- printk(KERN_INFO
- "Please re-open the interface.\n");
+ u16 rx_t;
+ u16 tx_t;
+ printk(KERN_INFO "Stopping interface %s.\n", netdev->name);
+ ucc_geth_close(netdev);
+
+ rx_t = ug_info->bdRingLenRx[queue];
+ tx_t = ug_info->bdRingLenTx[queue];
+
+ ug_info->bdRingLenRx[queue] = ring->rx_pending;
+ ug_info->bdRingLenTx[queue] = ring->tx_pending;
+
+ printk(KERN_INFO "Reactivating interface %s.\n", netdev->name);
+ ret = ucc_geth_open(netdev);
+ if (ret) {
+ printk(KERN_WARNING "uec_set_ringparam: set ring param for running"
+ " interface %s failed. Please try again.\n", netdev->name);
+ dev_close(netdev);
+ }
+ } else {
+ ug_info->bdRingLenRx[queue] = ring->rx_pending;
+ ug_info->bdRingLenTx[queue] = ring->tx_pending;
}
return ret;
--
1.7.2
^ permalink raw reply related
* RE: Memory allocation modifications in ibm_newemac driver AND sil24 driver
From: Jonathan Haws @ 2010-09-02 15:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-net@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1283401130.16240.9.camel@pasglop>
Pj4gQ2FuIGFueW9uZSBleHBsYWluIHRvIG1lIHdoeSBJIHdvdWxkIGJlIGdldHRpbmcgdGhpcyBl
cnJvciBpbiB0aGUNCj4+IGZpcnN0IHBsYWNlPyAgV2h5IGlzIGl0IGZhaWxpbmcgdG8gYWxsb2Nh
dGUgYSBwYWdlIHdoZW4gdGhlcmUgYXJlDQo+PiBwYWdlcyBhdmFpbGFibGU/ICBUaGF0IGRvZXMg
bm90IG1ha2UgYW55IHNlbnNlIHRvIG1lLg0KDQo+IG9yZGVyOjENCg0KPiBJdCdzIGZhaWxpbmcg
dG8gYWxsb2NhdGUgLXR3by0gcGFnZXMuDQoNCkdvb2QgcG9pbnQuICBIb3dldmVyLCB3aHkgaXMg
aXQgZmFpbGluZz8gIEFjY29yZGluZyB0byB0aGUgZHVtcCwgdGhlcmUgYXJlIDI4IDhrIHBhZ2Vz
IGF2YWlsYWJsZS4NCg0KRnJvbSBkb2luZyBtb3JlIHRlc3RpbmcgeWVzdGVyZGF5LCBJIGZvdW5k
IHRoYXQgdGhpcyBlcnJvciBpcyBhbHNvIGNvbWluZyB1cCBmcm9tIHRoZSBTQVRBIGRyaXZlciBh
cyB3ZWxsIChzaWwyNCkuICBBZ2FpbiB3aXRoIHRob3NlIGVycm9ycywgaXQgZmFpbHMgYWxsb2Nh
dGluZyBhIHBhZ2Ugb2Ygb3JkZXIgMCwgd2hlbiB0aGVyZSBhcmUgYSBmZXcgaHVuZHJlZCBvZiB0
aG9zZSBhdmFpbGFibGUuICBXaGF0IGlzIHVwIHdpdGggdGhlIFZNTT8gIFdoeSBhcmUgcGFnZXMg
ZmFpbGluZyB0byBhbGxvY2F0ZSB3aGVuIHRoZXkgYXJlIGF2YWlsYWJsZT8NCg0KVGhhbmtzLA0K
DQpKb25hdGhhbg0KDQoNCg==
^ permalink raw reply
* Re: [v1 PATCH] ucc_geth: fix ethtool set ring param bug
From: Liang Li @ 2010-09-02 15:48 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, avorontsov, davem, linuxppc-dev
In-Reply-To: <1283425907.2272.0.camel@achroite.uk.solarflarecom.com>
On Thu, Sep 02, 2010 at 12:11:47PM +0100, Ben Hutchings wrote:
> On Thu, 2010-09-02 at 08:50 +0800, Liang Li wrote:
> > On Wed, Sep 01, 2010 at 02:42:30PM +0100, Ben Hutchings wrote:
> > > On Wed, 2010-09-01 at 09:43 +0800, Liang Li wrote:
> > > > It's common sense that when we should do change to driver ring
> > > > desc/buffer etc only after 'stop/shutdown' the device. When we
> > > > do change while devices/driver is running, kernel oops occur:
> > > [...]
> > > > - ug_info->bdRingLenRx[queue] = ring->rx_pending;
> > > > - ug_info->bdRingLenTx[queue] = ring->tx_pending;
> > > > -
> > > > if (netif_running(netdev)) {
> > > > - /* FIXME: restart automatically */
> > > > - printk(KERN_INFO
> > > > - "Please re-open the interface.\n");
> > > > + u16 rx_t;
> > > > + u16 tx_t;
> > > > + printk(KERN_INFO "Stopping interface %s.\n", netdev->name);
> > > > + ucc_geth_close(netdev);
> > > > +
> > > > + rx_t = ug_info->bdRingLenRx[queue];
> > > > + tx_t = ug_info->bdRingLenTx[queue];
> > > > +
> > > > + ug_info->bdRingLenRx[queue] = ring->rx_pending;
> > > > + ug_info->bdRingLenTx[queue] = ring->tx_pending;
> > > > +
> > > > + printk(KERN_INFO "Reactivating interface %s.\n", netdev->name);
> > > > + ret = ucc_geth_open(netdev);
> > > > + if (ret) {
> > > > + printk(KERN_WARNING "uec_set_ringparam: set ring param for running"
> > > > + " interface %s failed. Please try to make the interface "
> > > > + " down, then try again.\n", netdev->name);
> > > > + ug_info->bdRingLenRx[queue] = rx_t;
> > > > + ug_info->bdRingLenTx[queue] = tx_t;
> > > > + }
> > > [...]
> > >
> > > Bringing the interface down will call ucc_geth_close(), which will try
> > > to free resources that have not been allocated!
> >
> > Sorry, I did not understand you on this point. There is no
> > ucc_geth_close when 'open fail'. What you mean here exactly?
> [...]
>
> Read your own warning.
So the warning is mis-leading... We may not need it... Still, let's talk
about the rare case that 'fail of open', may you please show me what is
the right thing to do 'clean up work' when ever the open fail. Maybe
dev_close(net_dev)?
Thanks,
-Liang Li
>
> Ben.
>
> --
> Ben Hutchings, Senior Software Engineer, Solarflare Communications
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: ERR_PTR pattern in phylib
From: Maciej W. Rozycki @ 2010-09-02 15:20 UTC (permalink / raw)
To: Grant Likely; +Cc: netdev, Andy Fleming, linuxppc-dev
In-Reply-To: <AANLkTimjVZTNeLdm4qptU1MqREODLASdGGanUyxqxfub@mail.gmail.com>
On Wed, 1 Sep 2010, Grant Likely wrote:
> I've been looking at the phylib code, and specifically at the use of
> the ERR_PTR() pattern. I'm personally not a big fan of ERR_PTR()
> because the compiler cannot type check the result and it runs
> completely counter to the pattern "if (!ptr)" than is common for
> testing a pointer return value.
Arguably using a union here would make things cleaner and any reasonable
ABI will place small unions used as arguments or return values in
registers, but I'm not sure if the cost of the rewrite is worth the
benefit. Probably not, but feel free to propose a change.
Maciej
^ permalink raw reply
* RE:
From: Rupjyoti Sarmah @ 2010-09-02 14:51 UTC (permalink / raw)
To: Wolfgang Denk; +Cc: linuxppc-dev, Prodyut Hazarika, Mark Miesfeld
In-Reply-To: <20100902061423.19D66153A79@gemini.denx.de>
Hi Wolfgang,
Sorry that I did not have a chance to check it recently.
Regards,
Rup
-----Original Message-----
From: Wolfgang Denk [mailto:wd@denx.de]
Sent: Thursday, September 02, 2010 11:44 AM
To: Rupjyoti Sarmah; Prodyut Hazarika; Mark Miesfeld
Cc: linuxppc-dev@ozlabs.org
Subject: Re:
Dear Rupjyoti, Prodyut, Mark,
two weeks ago I wrote:
In message <20100818205646.57783157D71@gemini.denx.de> you wrote:
>
> drivers/ata/sata_dwc_460ex.c fails to build in current mainline:
...
> Do you have any hints how to fix that?
Any comments or ideas how to fix this?
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Time is an illusion perpetrated by the manufacturers of space.
^ permalink raw reply
* Re: [PATCH 1/2] [PPC] Motion-PRO: Added LED support for the Promess Motion-Pro board. The driver is based on the original version(http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg06694.html), adapted for the current kernel structures.
From: Sergei Poselenov @ 2010-09-02 13:34 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: linuxppc-dev, wd, dzu
In-Reply-To: <20100902124617.GA27957@oksana.dev.rtsoft.ru>
Hi Anton,
On Thu, 2 Sep 2010 16:46:17 +0400
Anton Vorontsov <cbouatmailru@gmail.com> wrote:
> On Thu, Sep 02, 2010 at 12:20:31PM +0200, sposelenov@emcraft.com
> wrote: [...]
> > +config LEDS_MOTIONPRO
> > + tristate "Motionpro LEDs Support"
> > + depends on LEDS_CLASS
> > + help
> > + This option enables support for status and ready LEDs
> > connected
> > + to GPIO lines on Motionpro board.
>
> Why not expose these GPIOs via GPIOLIB[1] and use generic GPIO
> LEDs[2] driver, along with the timer LED trigger[3] for blinking?
>
> Thanks,
>
> [1] Documentation/gpio.txt
> Documentation/powerpc/dts-bindings/gpio/gpio.txt
> [2] drivers/leds/leds-gpio.c
> Documentation/powerpc/dts-bindings/gpio/led.txt
> [3] drivers/leds/ledtrig-timer.c
>
Yes, this seem possible to implement (and thanks for pointing into
this), however, the driver is already exists (actually, since 2007),
so why to not add it to save efforts?
Regards,
Sergei
^ permalink raw reply
* Re: [PATCH 1/2] [PPC] Motion-PRO: Added LED support for the Promess Motion-Pro board. The driver is based on the original version(http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg06694.html), adapted for the current kernel structures.
From: Anton Vorontsov @ 2010-09-02 14:01 UTC (permalink / raw)
To: Sergei Poselenov; +Cc: linuxppc-dev, wd, dzu
In-Reply-To: <20100902173456.3fdef6b8@emcraft.com>
On Thu, Sep 02, 2010 at 05:34:56PM +0400, Sergei Poselenov wrote:
[...]
> > > + tristate "Motionpro LEDs Support"
> > > + depends on LEDS_CLASS
> > > + help
> > > + This option enables support for status and ready LEDs
> > > connected
> > > + to GPIO lines on Motionpro board.
> >
> > Why not expose these GPIOs via GPIOLIB[1] and use generic GPIO
> > LEDs[2] driver, along with the timer LED trigger[3] for blinking?
> >
> > Thanks,
> >
> > [1] Documentation/gpio.txt
> > Documentation/powerpc/dts-bindings/gpio/gpio.txt
> > [2] drivers/leds/leds-gpio.c
> > Documentation/powerpc/dts-bindings/gpio/led.txt
> > [3] drivers/leds/ledtrig-timer.c
> >
>
> Yes, this seem possible to implement (and thanks for pointing into
> this), however, the driver is already exists (actually, since 2007),
> so why to not add it to save efforts?
- Faking PWM in the LEDs driver is just wrong thing to do.
I don't see any other drivers doing this, and even if they
were, they would need to be fixed;
- This duplicates timer trigger functionality;
- By writing (if there isn't any already) a generic GPIOLIB
driver for the GPIO controller that you have, you could use
these GPIOs not only for LEDs, but also for SPI, MDIO, I2C,
MMC, and even raw NAND chips.
I.e., by choosing the right methodology you save much more
efforts in the long run.
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* RE: P1021MDS QE Ethernet Ports
From: Haiying Wang @ 2010-09-02 13:32 UTC (permalink / raw)
To: Ioannis Kokkoris; +Cc: linuxppc-dev
In-Reply-To: <COL120-W10FFCD057D88A833B4B46EA78C0@phx.gbl>
On Thu, 2010-02-09 at 11:26 +0300, Ioannis Kokkoris wrote:
> > From: johnkokko@hotmail.com
> > To: linuxppc-dev@lists.ozlabs.org
> > Subject: P1021MDS QE Ethernet Ports
> > Date: Wed, 1 Sep 2010 15:11:56 +0300
> >
> >
> > Hello,
> >
> > we are seeing a strange behavior when trying to use the QE Ethernet interfaces.
> > ENET5 (UCC5 - RMII) interface on P1021MDS boards does not come up if there is no physical link on the ENET1 (UCC1 - MII) Port.
> > It seems that interrupts from ENET5 are normally received but the link comes up and works properly only if we have physical connection on ENET1.
> >
> So far I found the following:
>
> After adding traces, it seems that genphy_update_link() polls the correct device, with the correct address (0x03), but although a physical link is present in ENET5, the polling is not successful until there is a link in ENET1 (address 0x02)!
>
> genphy_update_link: Dev: Micrel KS8041 ADD: 3 Status read 0x7849 (without ENET1 Link)
> genphy_update_link: Dev: Micrel KS8041 ADD: 3 Status read 0x786D (with ENET1 Link)
>
> How does the MDIO of ENET1 affect the management of the physical interface in a different HW address?
Which board version are you using? this problem is now fixed in the new board version but not available right now. You can connect two UECs for your current development.
>
> > Can anyone think of a possible reason for this behavior, is there a way to trace this problem?
> > I can provide any further information needed.
> >
> > Any help would be appreciated,
> > Regards,
> > John
> >
> >
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/linuxppc-dev
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH 1/2] [PPC] Motion-PRO: Added LED support for the Promess Motion-Pro board. The driver is based on the original version(http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg06694.html), adapted for the current kernel structures.
From: Anton Vorontsov @ 2010-09-02 12:46 UTC (permalink / raw)
To: sposelenov; +Cc: linuxppc-dev, wd, dzu
In-Reply-To: <1283422832-9620-1-git-send-email-sposelenov@emcraft.com>
On Thu, Sep 02, 2010 at 12:20:31PM +0200, sposelenov@emcraft.com wrote:
[...]
> +config LEDS_MOTIONPRO
> + tristate "Motionpro LEDs Support"
> + depends on LEDS_CLASS
> + help
> + This option enables support for status and ready LEDs connected
> + to GPIO lines on Motionpro board.
Why not expose these GPIOs via GPIOLIB[1] and use generic GPIO
LEDs[2] driver, along with the timer LED trigger[3] for blinking?
Thanks,
[1] Documentation/gpio.txt
Documentation/powerpc/dts-bindings/gpio/gpio.txt
[2] drivers/leds/leds-gpio.c
Documentation/powerpc/dts-bindings/gpio/led.txt
[3] drivers/leds/ledtrig-timer.c
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [v1 PATCH] ucc_geth: fix ethtool set ring param bug
From: Ben Hutchings @ 2010-09-02 11:11 UTC (permalink / raw)
To: Liang Li; +Cc: netdev, avorontsov, davem, linuxppc-dev
In-Reply-To: <20100902005034.GA9901@localhost>
On Thu, 2010-09-02 at 08:50 +0800, Liang Li wrote:
> On Wed, Sep 01, 2010 at 02:42:30PM +0100, Ben Hutchings wrote:
> > On Wed, 2010-09-01 at 09:43 +0800, Liang Li wrote:
> > > It's common sense that when we should do change to driver ring
> > > desc/buffer etc only after 'stop/shutdown' the device. When we
> > > do change while devices/driver is running, kernel oops occur:
> > [...]
> > > - ug_info->bdRingLenRx[queue] = ring->rx_pending;
> > > - ug_info->bdRingLenTx[queue] = ring->tx_pending;
> > > -
> > > if (netif_running(netdev)) {
> > > - /* FIXME: restart automatically */
> > > - printk(KERN_INFO
> > > - "Please re-open the interface.\n");
> > > + u16 rx_t;
> > > + u16 tx_t;
> > > + printk(KERN_INFO "Stopping interface %s.\n", netdev->name);
> > > + ucc_geth_close(netdev);
> > > +
> > > + rx_t = ug_info->bdRingLenRx[queue];
> > > + tx_t = ug_info->bdRingLenTx[queue];
> > > +
> > > + ug_info->bdRingLenRx[queue] = ring->rx_pending;
> > > + ug_info->bdRingLenTx[queue] = ring->tx_pending;
> > > +
> > > + printk(KERN_INFO "Reactivating interface %s.\n", netdev->name);
> > > + ret = ucc_geth_open(netdev);
> > > + if (ret) {
> > > + printk(KERN_WARNING "uec_set_ringparam: set ring param for running"
> > > + " interface %s failed. Please try to make the interface "
> > > + " down, then try again.\n", netdev->name);
> > > + ug_info->bdRingLenRx[queue] = rx_t;
> > > + ug_info->bdRingLenTx[queue] = tx_t;
> > > + }
> > [...]
> >
> > Bringing the interface down will call ucc_geth_close(), which will try
> > to free resources that have not been allocated!
>
> Sorry, I did not understand you on this point. There is no
> ucc_geth_close when 'open fail'. What you mean here exactly?
[...]
Read your own warning.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH 1/2] [PPC] Motion-PRO: Added LED support for the Promess Motion-Pro board. The driver is based on the original version(http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg06694.html), adapted for the current kernel structures.
From: sposelenov @ 2010-09-02 10:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Sergei Poselenov, wd, dzu
From: Sergei Poselenov <sposelenov@emcraft.com>
Signed-off-by: Sergei Poselenov <sposelenov@emcraft.com>
---
arch/powerpc/configs/52xx/motionpro_defconfig | 1 +
arch/powerpc/include/asm/mpc52xx.h | 5 +
drivers/leds/Kconfig | 7 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-motionpro.c | 255 +++++++++++++++++++++++++
5 files changed, 269 insertions(+), 0 deletions(-)
create mode 100644 drivers/leds/leds-motionpro.c
diff --git a/arch/powerpc/configs/52xx/motionpro_defconfig b/arch/powerpc/configs/52xx/motionpro_defconfig
index 20d53a1..cad1f44 100644
--- a/arch/powerpc/configs/52xx/motionpro_defconfig
+++ b/arch/powerpc/configs/52xx/motionpro_defconfig
@@ -77,6 +77,7 @@ CONFIG_I2C_CHARDEV=y
CONFIG_I2C_MPC=y
CONFIG_WATCHDOG=y
# CONFIG_USB_SUPPORT is not set
+CONFIG_LEDS_MOTIONPRO=y
CONFIG_NEW_LEDS=y
CONFIG_LEDS_CLASS=y
CONFIG_LEDS_TRIGGERS=y
diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/asm/mpc52xx.h
index 1f41382..b206e47 100644
--- a/arch/powerpc/include/asm/mpc52xx.h
+++ b/arch/powerpc/include/asm/mpc52xx.h
@@ -148,6 +148,11 @@ struct mpc52xx_gpio {
#define MPC52xx_GPIO_PSC_CONFIG_UART_WITH_CD 5
#define MPC52xx_GPIO_PCI_DIS (1<<15)
+/* Enables GPT register to operate as simple GPIO output register */
+#define MPC52xx_GPT_ENABLE_OUTPUT 0x00000024
+/* Puts 1 on GPT output pin */
+#define MPC52xx_GPT_OUTPUT_1 0x00000010
+
/* GPIO with WakeUp*/
struct mpc52xx_gpio_wkup {
u8 wkup_gpioe; /* GPIO_WKUP + 0x00 */
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index e411262..f5c3e6b 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -311,6 +311,13 @@ config LEDS_NS2
Network Space v2 board (and parents). This include Internet Space v2,
Network Space (Max) v2 and d2 Network v2 boards.
+config LEDS_MOTIONPRO
+ tristate "Motionpro LEDs Support"
+ depends on LEDS_CLASS
+ help
+ This option enables support for status and ready LEDs connected
+ to GPIO lines on Motionpro board.
+
config LEDS_TRIGGERS
bool "LED Trigger support"
help
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 7d6b958..738e227 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_LEDS_ADP5520) += leds-adp5520.o
obj-$(CONFIG_LEDS_DELL_NETBOOKS) += dell-led.o
obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o
obj-$(CONFIG_LEDS_NS2) += leds-ns2.o
+obj-$(CONFIG_LEDS_MOTIONPRO) += leds-motionpro.o
# LED SPI Drivers
obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
diff --git a/drivers/leds/leds-motionpro.c b/drivers/leds/leds-motionpro.c
new file mode 100644
index 0000000..94f6cf8
--- /dev/null
+++ b/drivers/leds/leds-motionpro.c
@@ -0,0 +1,255 @@
+/*
+ * LEDs driver for the Motion-PRO board.
+ *
+ * Copyright (C) 2007 Semihalf
+ * Jan Wrobel <wrr@semihalf.com>
+ * Marian Balakowicz <m8@semihalf.com>
+ *
+ * Porting to the mainline Linux tree.
+ * Copyright (C) 2010 Emcraft Systems
+ * Sergei Poselenov <sposelenov@emcraft.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc., 51
+ * Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ *
+ * Decription:
+ * This driver enables control over Motion-PRO status and ready LEDs through
+ * sysfs. LEDs can be controlled by writing to sysfs files:
+ * class/leds/<led-name>/(brightness|delay_off|delay_on).
+ * See Documentation/leds-class.txt for more details.
+ * <led-name> is the set to the value of 'label' property of the
+ * corresponding GPT node.
+ *
+ * Before user issues first control command via sysfs, LED blinking is
+ * controlled by the kernel ('blink-delay' property of the GPT node
+ * in the device tree blob).
+ *
+ */
+
+#define DEBUG
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/leds.h>
+#include <linux/of_platform.h>
+#include <asm/mpc52xx.h>
+
+/* LED control bits */
+#define LED_ON MPC52xx_GPT_OUTPUT_1
+
+/* LED mode */
+#define LED_MODE_KERNEL 1
+#define LED_MODE_USER 2
+
+struct motionpro_led {
+ spinlock_t led_lock; /* Protects the LED data */
+ struct mpc52xx_gpt __iomem *gpt;/* LED registers */
+ struct timer_list blink_timer; /* Used if blink_delay is nonzero */
+ unsigned int blink_delay; /* [ms], if set to 0 blinking is off */
+ unsigned int mode; /* kernel/user */
+ struct led_classdev mpled_cdev; /* LED class */
+};
+
+/*
+ * Timer event - blinks LED before user takes control over it
+ * with the first access via sysfs.
+ */
+static void mpled_timer_toggle(unsigned long data)
+{
+ struct motionpro_led *mpled = (struct motionpro_led *)data;
+
+ spin_lock_bh(&mpled->led_lock);
+ if (mpled->mode == LED_MODE_KERNEL) {
+ u32 val = in_be32(&mpled->gpt->mode);
+ val ^= LED_ON;
+ out_be32(&mpled->gpt->mode, val);
+
+ mod_timer(&mpled->blink_timer,
+ jiffies + msecs_to_jiffies(mpled->blink_delay));
+ }
+ spin_unlock_bh(&mpled->led_lock);
+}
+
+/*
+ * Turn on/off led according to user settings in sysfs.
+ * First call to this function disables kernel blinking.
+ */
+static void mpled_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ struct motionpro_led *mpled;
+ int old_mode;
+ u32 val;
+
+ mpled = container_of(led_cdev, struct motionpro_led, mpled_cdev);
+
+ spin_lock_bh(&mpled->led_lock);
+ /* disable kernel controll */
+ old_mode = mpled->mode;
+ if (old_mode == LED_MODE_KERNEL)
+ mpled->mode = LED_MODE_USER;
+
+ val = in_be32(&mpled->gpt->mode);
+ if (brightness)
+ val |= LED_ON;
+ else
+ val &= ~LED_ON;
+ out_be32(&mpled->gpt->mode, val);
+ spin_unlock_bh(&mpled->led_lock);
+
+ /* delete kernel mode blink timer, not needed anymore */
+ if ((old_mode == LED_MODE_KERNEL) && mpled->blink_delay)
+ del_timer(&mpled->blink_timer);
+}
+
+static void mpled_init_led(void __iomem *gpt_mode)
+{
+ u32 val = in_be32(gpt_mode);
+ val |= MPC52xx_GPT_ENABLE_OUTPUT;
+ val &= ~LED_ON;
+ out_be32(gpt_mode, val);
+}
+
+static int __devinit mpled_probe(struct platform_device *op,
+ const struct of_device_id *match)
+{
+ struct motionpro_led *mpled;
+ const unsigned int *of_blink_delay;
+ const char *label;
+ int err;
+
+ dev_dbg(&op->dev, "mpled_probe: node=%s (op=%p, match=%p)\n",
+ op->name, op, match);
+
+ mpled = kzalloc(sizeof(*mpled), GFP_KERNEL);
+ if (!mpled)
+ return -ENOMEM;
+
+ mpled->gpt = of_iomap(op->dev.of_node, 0);
+ if (!mpled->gpt) {
+ printk(KERN_ERR __FILE__ ": "
+ "Error mapping GPT registers for LED %s\n",
+ op->dev.of_node->full_name);
+ err = -EIO;
+ goto err_free;
+ }
+
+ /* initialize GPT for LED use */
+ mpled_init_led(&mpled->gpt->mode);
+
+ spin_lock_init(&mpled->led_lock);
+ mpled->mode = LED_MODE_KERNEL;
+
+ /* get LED label, used to register led classdev */
+ label = of_get_property(op->dev.of_node, "label", NULL);
+ if (label == NULL) {
+ printk(KERN_ERR __FILE__ ": "
+ "No label property provided for LED %s\n",
+ op->dev.of_node->full_name);
+ err = -EINVAL;
+ goto err;
+ }
+ dev_dbg(&op->dev, "mpled_probe: label = '%s'\n", label);
+
+ /* get 'blink-delay' property if present */
+ of_blink_delay = of_get_property(op->dev.of_node, "blink-delay", NULL);
+ mpled->blink_delay = of_blink_delay ? *of_blink_delay : 0;
+ dev_dbg(&op->dev, "mpled_probe: blink_delay = %d msec\n",
+ mpled->blink_delay);
+
+ /* initialize kernel blink_timer if blink_delay was provided */
+ if (mpled->blink_delay) {
+ init_timer(&mpled->blink_timer);
+ mpled->blink_timer.function = mpled_timer_toggle;
+ mpled->blink_timer.data = (unsigned long)mpled;
+
+ mod_timer(&mpled->blink_timer,
+ jiffies + msecs_to_jiffies(mpled->blink_delay));
+ }
+
+ /* register LED classdev */
+ mpled->mpled_cdev.name = label;
+ mpled->mpled_cdev.brightness_set = mpled_set;
+ mpled->mpled_cdev.default_trigger = "timer";
+
+ err = led_classdev_register(NULL, &mpled->mpled_cdev);
+ if (err) {
+ printk(KERN_ERR __FILE__ ": "
+ "Error registering class device for LED %s\n",
+ op->dev.of_node->full_name);
+ goto err;
+ }
+
+ dev_set_drvdata(&op->dev, mpled);
+ return 0;
+
+err:
+ if (mpled->blink_delay)
+ del_timer(&mpled->blink_timer);
+ iounmap(mpled->gpt);
+err_free:
+ kfree(mpled);
+
+ return err;
+}
+
+static int mpled_remove(struct platform_device *op)
+{
+ struct motionpro_led *mpled = dev_get_drvdata(&op->dev);
+
+ dev_dbg(&op->dev, "mpled_remove: (%p)\n", op);
+
+ if (mpled->blink_delay && (mpled->mode == LED_MODE_KERNEL))
+ del_timer(&mpled->blink_timer);
+
+ led_classdev_unregister(&mpled->mpled_cdev);
+
+ iounmap(mpled->gpt);
+ kfree(mpled);
+
+ return 0;
+}
+
+static const struct of_device_id mpled_match[] = {
+ { .compatible = "promess,motionpro-led", },
+ {},
+};
+
+static struct of_platform_driver mpled_driver = {
+ .probe = mpled_probe,
+ .remove = mpled_remove,
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = "leds-motionpro",
+ .of_match_table = mpled_match,
+ },
+};
+
+static int __init mpled_init(void)
+{
+ return of_register_platform_driver(&mpled_driver);
+}
+
+static void __exit mpled_exit(void)
+{
+ of_unregister_platform_driver(&mpled_driver);
+}
+
+module_init(mpled_init);
+module_exit(mpled_exit);
+
+MODULE_LICENSE("GPL")
+MODULE_DESCRIPTION("Motion-PRO LED driver");
+MODULE_AUTHOR("Jan Wrobel <wrr@semihalf.com>");
+MODULE_AUTHOR("Marian Balakowicz <m8@semihalf.com>");
--
1.6.2.5
^ permalink raw reply related
* [PATCH 2/2] [PPC] Motion-PRO: Changed the default blinking rate for the status LED to 200 ms, as per the customer's request.
From: sposelenov @ 2010-09-02 10:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Sergei Poselenov, wd, dzu
In-Reply-To: <1283422832-9620-1-git-send-email-sposelenov@emcraft.com>
From: Sergei Poselenov <sposelenov@emcraft.com>
Signed-off-by: Sergei Poselenov <sposelenov@emcraft.com>
---
arch/powerpc/boot/dts/motionpro.dts | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/boot/dts/motionpro.dts b/arch/powerpc/boot/dts/motionpro.dts
index 6ca4fc1..b469fc6 100644
--- a/arch/powerpc/boot/dts/motionpro.dts
+++ b/arch/powerpc/boot/dts/motionpro.dts
@@ -105,7 +105,7 @@
label = "motionpro-statusled";
reg = <0x660 0x10>;
interrupts = <1 15 0>;
- blink-delay = <100>; // 100 msec
+ blink-delay = <200>; // 200 msec
};
motionpro-led@670 { // Motion-PRO ready LED
--
1.6.2.5
^ permalink raw reply related
* RE: [PATCH v2 6/6] DTS: add SPI flash(s25fl128p01) support on p4080ds and mpc8536ds board
From: Hu Mingkai-B21284 @ 2010-09-02 8:30 UTC (permalink / raw)
To: Grant Likely
Cc: linuxppc-dev, Gala Kumar-B11780, Zang Roy-R61911,
spi-devel-general
In-Reply-To: <AANLkTinzea7-k3Wd6EsAy+ipThHMhkpMF3pCP7g_6O6J@mail.gmail.com>
> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On Behalf Of =
Grant
> Likely
> Sent: Tuesday, August 10, 2010 3:11 PM
> To: Hu Mingkai-B21284
> Cc: linuxppc-dev@ozlabs.org; spi-devel-general@lists.sourceforge.net; =
Gala
> Kumar-B11780; Zang Roy-R61911
> Subject: Re: [PATCH v2 6/6] DTS: add SPI flash(s25fl128p01) support on =
p4080ds
> and mpc8536ds board
>=20
> Hi Mingkai,
>=20
> one comment below. Otherwise this patch looks good, and so does patch =
5.
>=20
> g.
>=20
> On Mon, Aug 2, 2010 at 1:52 AM, Mingkai Hu <Mingkai.hu@freescale.com> =
wrote:
> > Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
> > ---
> >
> > v2:
> > =A0- Remove the whitespace inconsitencies
> >
> > =A0arch/powerpc/boot/dts/mpc8536ds.dts | =A0 52
> > +++++++++++++++++++++++++++++++++++
> > =A0arch/powerpc/boot/dts/p4080ds.dts =A0 | =A0 11 +++-----
> > =A02 files changed, 56 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/dts/mpc8536ds.dts
> > b/arch/powerpc/boot/dts/mpc8536ds.dts
> > index 815cebb..a75c10e 100644
> > --- a/arch/powerpc/boot/dts/mpc8536ds.dts
> > +++ b/arch/powerpc/boot/dts/mpc8536ds.dts
> > @@ -108,6 +108,58 @@
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0};
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0};
> >
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spi@7000 {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 #address-cells =3D =
<1>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 #size-cells =3D <0>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D =
"fsl,mpc8536-espi";
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D <0x7000 =
0x1000>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 interrupts =3D <59 =
0x2>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 interrupt-parent =3D =
<&mpic>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
fsl,espi-num-chipselects =3D <4>;
> > +
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 flash@0 {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
#address-cells =3D <1>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
#size-cells =3D <1>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
compatible =3D "spansion,s25sl12801";
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =
=3D <0>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
spi-max-frequency =3D <40000000>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
partition@u-boot {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 label =3D "u-boot";
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 reg =3D <0x00000000 0x00100000>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 read-only;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 };
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
partition@kernel {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 label =3D "kernel";
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 reg =3D <0x00100000 0x00500000>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 read-only;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 };
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
partition@dtb {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 label =3D "dtb";
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 reg =3D <0x00600000 0x00100000>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 read-only;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 };
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
partition@fs {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 label =3D "file system";
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 reg =3D <0x00700000 0x00900000>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 };
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 };
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 flash@1 {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
compatible =3D "spansion,s25sl12801";
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =
=3D <1>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
spi-max-frequency =3D <40000000>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 };
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 flash@2 {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
compatible =3D "spansion,s25sl12801";
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =
=3D <2>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
spi-max-frequency =3D <40000000>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 };
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 flash@3 {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
compatible =3D "spansion,s25sl12801";
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =
=3D <3>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
spi-max-frequency =3D <40000000>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 };
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 };
> > +
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dma@21300 {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0#address-cells =3D =
<1>;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0#size-cells =3D <1>; =
diff --git
> > a/arch/powerpc/boot/dts/p4080ds.dts
> > b/arch/powerpc/boot/dts/p4080ds.dts
> > index 6b29eab..48437ad 100644
> > --- a/arch/powerpc/boot/dts/p4080ds.dts
> > +++ b/arch/powerpc/boot/dts/p4080ds.dts
> > @@ -236,22 +236,19 @@
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0};
> >
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0spi@110000 {
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cell-index =3D <0>;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0#address-cells =3D =
<1>;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0#size-cells =3D <0>;
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D =
"fsl,espi";
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D =
"fsl,mpc8536-espi";
>=20
> Should be more specific here by specifying the exact device; plus a =
list of what
> it is compatible with. For example:
>=20
> compatible =3D "fsl,p4080-espi", "fsl,mpc5836-espi";
>=20
> the reason for this is that the driver for the existing part is still =
able to
> bind against the node, but if it ever needs it, then information about =
the
> specific device is available which can be used to (for example) figure =
out when
> to enable silicon bug workarounds.
>=20
Make sense, I'll add it.
Thanks,
Mingkai
^ permalink raw reply
* RE: [PATCH v2 2/6] eSPI: add eSPI controller support
From: Hu Mingkai-B21284 @ 2010-09-02 8:28 UTC (permalink / raw)
To: Grant Likely
Cc: linuxppc-dev, Gala Kumar-B11780, Zang Roy-R61911,
spi-devel-general
In-Reply-To: <AANLkTin3W4++cAFcksVBX3QxeV8QF3d5T2tGy_KbEkRs@mail.gmail.com>
> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On Behalf Of =
Grant
> Likely
> Sent: Tuesday, August 10, 2010 2:45 PM
> To: Hu Mingkai-B21284
> Cc: linuxppc-dev@ozlabs.org; spi-devel-general@lists.sourceforge.net; =
Gala
> Kumar-B11780; Zang Roy-R61911
> Subject: Re: [PATCH v2 2/6] eSPI: add eSPI controller support
>=20
> On Mon, Aug 2, 2010 at 1:52 AM, Mingkai Hu <Mingkai.hu@freescale.com> =
wrote:
> > Add eSPI controller support based on the library code spi_fsl_lib.c.
> >
> > The eSPI controller is newer controller 85xx/Pxxx devices supported.
> > There're some differences comparing to the SPI controller:
> >
> > 1. Has different register map and different bit definition
> > =A0 So leave the code operated the register to the driver code, not
> > =A0 the common code.
> >
> > 2. Support 4 dedicated chip selects
> > =A0 The software can't controll the chip selects directly, The =
SPCOM[CS]
> > =A0 field is used to select which chip selects is used, and the
> > =A0 SPCOM[TRANLEN] field is set to tell the controller how long the =
CS
> > =A0 signal need to be asserted. So the driver doesn't need the
> > chipselect
> > =A0 related function when transfering data, just set corresponding
> > register
> > =A0 fields to controll the chipseclect.
> >
> > 3. Different Transmit/Receive FIFO access register behavior
> > =A0 For SPI controller, the Tx/Rx FIFO access register can hold only
> > =A0 one character regardless of the character length, but for eSPI
> > =A0 controller, the register can hold 4 or 2 characters according to
> > =A0 the character lengths. Access the Tx/Rx FIFO access register of =
the
> > =A0 eSPI controller will shift out/in 4/2 characters one time. For =
SPI
> > =A0 subsystem, the command and data are put into different =
transfers, so
> > =A0 we need to combine all the transfers to one transfer in order to
> > pass
> > =A0 the transfer to eSPI controller.
> >
> > Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
>=20
> I've not dug deep into this patch, but it seems pretty good. I did =
notice one
> thing below...
> [...]
>=20
> > diff --git a/drivers/spi/spi_fsl_lib.h b/drivers/spi/spi_fsl_lib.h
> > index 774e1c8..0772c98 100644
> > --- a/drivers/spi/spi_fsl_lib.h
> > +++ b/drivers/spi/spi_fsl_lib.h
> > @@ -22,10 +22,12 @@
> > =A0struct mpc8xxx_spi {
> > =A0 =A0 =A0 =A0struct device *dev;
> > =A0 =A0 =A0 =A0struct fsl_spi_reg __iomem *base;
> > + =A0 =A0 =A0 struct fsl_espi_reg __iomem *espi_base;
>=20
> There's got to be a cleaner way to do this. Rather than putting both =
pointers
> into mpc8xxx_spi, I suggest each driver use its own fsl_spi_priv and
> fsl_espi_priv wrapper structures that contain the controller specific =
properties.
>=20
Make sense, I'll change it.
Thanks,
Mingkai
^ permalink raw reply
* RE: [PATCH v2 1/6] spi/mpc8xxx: refactor the common code for SPI/eSPI controller
From: Hu Mingkai-B21284 @ 2010-09-02 8:27 UTC (permalink / raw)
To: Grant Likely
Cc: linuxppc-dev, Gala Kumar-B11780, Zang Roy-R61911,
spi-devel-general
In-Reply-To: <AANLkTim=Qc-zA=9E0SfOB+8V48Y957MXkefD324xoq0w@mail.gmail.com>
Hi Grant and all,
Sorry for the delay, I'm on business trip some weeks ago.
> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On Behalf Of =
Grant
> Likely
> Sent: Tuesday, August 10, 2010 2:40 PM
> To: Hu Mingkai-B21284
> Cc: linuxppc-dev@ozlabs.org; spi-devel-general@lists.sourceforge.net; =
Gala
> Kumar-B11780; Zang Roy-R61911
> Subject: Re: [PATCH v2 1/6] spi/mpc8xxx: refactor the common code for =
SPI/eSPI
> controller
>=20
> On Mon, Aug 2, 2010 at 1:51 AM, Mingkai Hu <Mingkai.hu@freescale.com> =
wrote:
> > Refactor the common code in file spi_mpc8xxx.c to spi_fsl_lib.c used
> > by SPI/eSPI controller driver as a library, move the SPI controller
> > driver to a new file spi_fsl_spi.c, and leave the QE/CPM SPI
> > controller code in this file.
> >
> > Because the register map of the SPI controller and eSPI controller =
is
> > so different, also leave the code operated the register to the =
driver
> > code, not the common code.
> >
> > Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
> > ---
> > v2:
> > =A0- Rename spi_mpc8xxx.c to spi_fsl_lib.c, also the config name
> > =A0- Rename fsl_spi.c to spi_fsl_spi.c, also the config name
> > =A0- Move register map definiton from spi_fsl_lib.c to spi_fsl_spi.c
> > =A0- Break some funcions line in the arguments instead of the
> > declaration
> > =A0- Init bits_per_word to 0 to eliminate the else clause
> > =A0- Add brace for the else clause to match if clause
> > =A0- Drop the last entry's comma in the match table
> > =A0- move module_init() immediately after the init fsl_spi_init()
> > function
> >
> > =A0drivers/spi/Kconfig =A0 =A0 =A0 | =A0 20 +-
> > =A0drivers/spi/Makefile =A0 =A0 =A0| =A0 =A03 +-
> > =A0drivers/spi/spi_fsl_lib.c | =A0237 ++++++++
> > =A0drivers/spi/spi_fsl_lib.h | =A0119 ++++
> > =A0drivers/spi/spi_fsl_spi.c | 1173
> > +++++++++++++++++++++++++++++++++++++
> > =A0drivers/spi/spi_mpc8xxx.c | 1421
> > ---------------------------------------------
>=20
> This patch seems pretty good now. However, the rename from =
spi_mpc8xxx.c to
> spi_fsl_lib.c should be done in a separate patch. It is too difficult =
to track
> what has changed, when the file gets moved at the same time. Move the =
file in
> one patch verbatim (with the required Makefile change), and then move =
out the
> fsl_spi specific bits in a second patch.
>=20
Ok, I'll spilt this patch.
Thanks,
Mingkai
^ permalink raw reply
* RE: [PATCH v2 4/6] mtd: m25p80: add a read function to read page by page
From: Hu Mingkai-B21284 @ 2010-09-02 9:04 UTC (permalink / raw)
To: Grant Likely, David Woodhouse
Cc: linuxppc-dev, Gala Kumar-B11780, linux-mtd, Zang Roy-R61911,
spi-devel-general
In-Reply-To: <AANLkTikM5je7Ckt=7bP6fj7OKCaroSn_AqgcS4T+Bv5d@mail.gmail.com>
[adding mtd list]
Hi guys,
Could you please take a look at this patch and give some suggestion?
Thanks,
Mingkai
> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On Behalf Of =
Grant
> Likely
> Sent: Tuesday, August 10, 2010 3:00 PM
> To: Hu Mingkai-B21284; David Woodhouse
> Cc: linuxppc-dev@ozlabs.org; spi-devel-general@lists.sourceforge.net; =
Gala
> Kumar-B11780; Zang Roy-R61911
> Subject: Re: [PATCH v2 4/6] mtd: m25p80: add a read function to read =
page by
> page
>=20
> [adding David Woodhouse]
>=20
> (Btw, you should cc: David Woodhouse and the mtd list on the MTD =
patches).
>=20
> On Mon, Aug 2, 2010 at 1:52 AM, Mingkai Hu <Mingkai.hu@freescale.com> =
wrote:
> > For Freescale's eSPI controller, the max transaction length one time
> > is limitted by the SPCOM[TRANSLEN] field which is 0xFFFF. When used
> > mkfs.ext2 command to create ext2 filesystem on the flash, the read
> > length will exceed the max value of the SPCOM[TRANSLEN] field, so
> > change the read function to read page by page.
> >
> > For other SPI flash driver, also needed to supply the read function =
if
> > used the eSPI controller.
> >
> > Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
>=20
> This one bothers me, but I can't put my finger on it. The flag feels =
like a
> controller specific hack. David, can you take a look at this patch?
>=20
> g.
>=20
> > ---
> >
> > v2:
> > =A0- Add SPI_MASTER_TRANS_LIMIT flag to indicate the master's trans
> > length
> > =A0 limitation, so the MTD driver can select the correct transfer
> > behaviour
> > =A0 at driver probe time
> >
> > =A0drivers/mtd/devices/m25p80.c | =A0 78
> > ++++++++++++++++++++++++++++++++++++++++++
> > =A0drivers/spi/spi_fsl_espi.c =A0 | =A0 =A01 +
> > =A0include/linux/spi/spi.h =A0 =A0 =A0| =A0 =A01 +
> > =A03 files changed, 80 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mtd/devices/m25p80.c
> > b/drivers/mtd/devices/m25p80.c index 5f00075..30e4568 100644
> > --- a/drivers/mtd/devices/m25p80.c
> > +++ b/drivers/mtd/devices/m25p80.c
> > @@ -376,6 +376,81 @@ static int m25p80_read(struct mtd_info *mtd,
> > loff_t from, size_t len,
> > =A0}
> >
> > =A0/*
> > + * Read an address range from the flash chip page by page.
> > + * Some controller has transaction length limitation such as the
> > + * Freescale's eSPI controller can only trasmit 0xFFFF bytes one
> > + * time, so we have to read page by page if the len is more than
> > + * the limitation.
> > + */
> > +static int m25p80_page_read(struct mtd_info *mtd, loff_t from, =
size_t
> > +len,
> > + =A0 =A0 =A0 size_t *retlen, u_char *buf)
> > +{
> > + =A0 =A0 =A0 struct m25p *flash =3D mtd_to_m25p(mtd);
> > + =A0 =A0 =A0 struct spi_transfer t[2];
> > + =A0 =A0 =A0 struct spi_message m;
> > + =A0 =A0 =A0 u32 i, page_size =3D 0;
> > +
> > + =A0 =A0 =A0 DEBUG(MTD_DEBUG_LEVEL2, "%s: %s %s 0x%08x, len %zd\n",
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
dev_name(&flash->spi->dev), __func__, "from",
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (u32)from, len);
> > +
> > + =A0 =A0 =A0 /* sanity checks */
> > + =A0 =A0 =A0 if (!len)
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0;
> > +
> > + =A0 =A0 =A0 if (from + len > flash->mtd.size)
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EINVAL;
> > +
> > + =A0 =A0 =A0 spi_message_init(&m);
> > + =A0 =A0 =A0 memset(t, 0, (sizeof t));
> > +
> > + =A0 =A0 =A0 /* NOTE:
> > + =A0 =A0 =A0 =A0* OPCODE_FAST_READ (if available) is faster.
> > + =A0 =A0 =A0 =A0* Should add 1 byte DUMMY_BYTE.
> > + =A0 =A0 =A0 =A0*/
> > + =A0 =A0 =A0 t[0].tx_buf =3D flash->command;
> > + =A0 =A0 =A0 t[0].len =3D m25p_cmdsz(flash) + FAST_READ_DUMMY_BYTE;
> > + =A0 =A0 =A0 spi_message_add_tail(&t[0], &m);
> > +
> > + =A0 =A0 =A0 t[1].rx_buf =3D buf;
> > + =A0 =A0 =A0 spi_message_add_tail(&t[1], &m);
> > +
> > + =A0 =A0 =A0 /* Byte count starts at zero. */
> > + =A0 =A0 =A0 if (retlen)
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 *retlen =3D 0;
> > +
> > + =A0 =A0 =A0 mutex_lock(&flash->lock);
> > +
> > + =A0 =A0 =A0 /* Wait till previous write/erase is done. */
> > + =A0 =A0 =A0 if (wait_till_ready(flash)) {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* REVISIT status return?? */
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mutex_unlock(&flash->lock);
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 1;
> > + =A0 =A0 =A0 }
> > +
> > + =A0 =A0 =A0 /* Set up the write data buffer. */
> > + =A0 =A0 =A0 flash->command[0] =3D OPCODE_READ;
> > +
> > + =A0 =A0 =A0 for (i =3D page_size; i < len; i +=3D page_size) {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 page_size =3D len - i;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (page_size > flash->page_size)
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 page_size =3D =
flash->page_size;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 m25p_addr2cmd(flash, from + i, =
flash->command);
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 t[1].len =3D page_size;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 t[1].rx_buf =3D buf + i;
> > +
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spi_sync(flash->spi, &m);
> > +
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 *retlen +=3D m.actual_length - =
m25p_cmdsz(flash)
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 - =
FAST_READ_DUMMY_BYTE;
> > + =A0 =A0 =A0 }
> > +
> > + =A0 =A0 =A0 mutex_unlock(&flash->lock);
> > +
> > + =A0 =A0 =A0 return 0;
> > +}
> > +
> > +/*
> > =A0* Write an address range to the flash chip. =A0Data must be =
written in
> > =A0* FLASH_PAGESIZE chunks. =A0The address range may be any size =
provided
> > =A0* it is within the physical boundaries.
> > @@ -877,6 +952,9 @@ static int __devinit m25p_probe(struct =
spi_device
> > *spi)
> > =A0 =A0 =A0 =A0flash->mtd.erase =3D m25p80_erase;
> > =A0 =A0 =A0 =A0flash->mtd.read =3D m25p80_read;
> >
> > + =A0 =A0 =A0 if (spi->master->flags & SPI_MASTER_TRANS_LIMIT)
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 flash->mtd.read =3D m25p80_page_read;
> > +
> > =A0 =A0 =A0 =A0/* sst flash chips use AAI word program */
> > =A0 =A0 =A0 =A0if (info->jedec_id >> 16 =3D=3D 0xbf)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0flash->mtd.write =3D sst_write; diff =
--git
> > a/drivers/spi/spi_fsl_espi.c b/drivers/spi/spi_fsl_espi.c index
> > 61987cf..e15b7dc 100644
> > --- a/drivers/spi/spi_fsl_espi.c
> > +++ b/drivers/spi/spi_fsl_espi.c
> > @@ -470,6 +470,7 @@ static struct spi_master * __devinit
> > fsl_espi_probe(struct device *dev,
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto err_probe;
> >
> > =A0 =A0 =A0 =A0master->setup =3D fsl_espi_setup;
> > + =A0 =A0 =A0 master->flags =3D SPI_MASTER_TRANS_LIMIT;
> >
> > =A0 =A0 =A0 =A0mpc8xxx_spi =3D spi_master_get_devdata(master);
> > =A0 =A0 =A0 =A0mpc8xxx_spi->spi_do_one_msg =3D fsl_espi_do_one_msg; =
diff --git
> > a/include/linux/spi/spi.h b/include/linux/spi/spi.h index
> > af56071..0729cbd 100644
> > --- a/include/linux/spi/spi.h
> > +++ b/include/linux/spi/spi.h
> > @@ -261,6 +261,7 @@ struct spi_master {
> > =A0#define SPI_MASTER_HALF_DUPLEX BIT(0) =A0 =A0 =A0 =A0 =A0/* can't =
do full
> > duplex */
> > =A0#define SPI_MASTER_NO_RX =A0 =A0 =A0 BIT(1) =A0 =A0 =A0 =A0 =A0/* =
can't do buffer
> > read */
> > =A0#define SPI_MASTER_NO_TX =A0 =A0 =A0 BIT(2) =A0 =A0 =A0 =A0 =A0/* =
can't do buffer
> > write */
> > +#define SPI_MASTER_TRANS_LIMIT BIT(3) =A0 =A0 =A0 =A0 =A0/* have =
trans length
> > +limit */
> >
> > =A0 =A0 =A0 =A0/* Setup mode and clock, etc (spi driver may call =
many times).
> > =A0 =A0 =A0 =A0 *
> > --
> > 1.6.4
> >
> >
> >
>=20
>=20
>=20
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
^ permalink raw reply
* RE: P1021MDS QE Ethernet Ports
From: Ioannis Kokkoris @ 2010-09-02 8:26 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <COL120-W4274596E983E02BCBF4AD0A78B0@phx.gbl>
> From: johnkokko@hotmail.com
> To: linuxppc-dev@lists.ozlabs.org
> Subject: P1021MDS QE Ethernet Ports
> Date: Wed=2C 1 Sep 2010 15:11:56 +0300
>
>
> Hello=2C
>
> we are seeing a strange behavior when trying to use the QE Ethernet inter=
faces.
> ENET5 (UCC5 - RMII) interface on P1021MDS boards does not come up if ther=
e is no physical link on the ENET1 (UCC1 - MII) Port.
> It seems that interrupts from ENET5 are normally received but the link co=
mes up and works properly only if we have physical connection on ENET1.
>
So far I found the following:
After adding traces=2C it seems that genphy_update_link() polls the correct=
device=2C with the correct address (0x03)=2C but although a physical link =
is present in ENET5=2C the polling is not successful until there is a link =
in ENET1 (address 0x02)!
genphy_update_link: Dev: Micrel KS8041 ADD: 3 Status read 0x7849=A0 (withou=
t ENET1 Link)
genphy_update_link: Dev: Micrel KS8041 ADD: 3 Status read 0x786D=A0 (with=
=A0=A0=A0 ENET1 Link)
How does the MDIO of ENET1 affect the management of the physical interface =
in a different HW address?
> Can anyone think of a possible reason for this behavior=2C is there a way=
to trace this problem?
> I can provide any further information needed.
>
> Any help would be appreciated=2C
> Regards=2C
> John
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
=
^ permalink raw reply
* Re:
From: Wolfgang Denk @ 2010-09-02 6:14 UTC (permalink / raw)
To: Rupjyoti Sarmah, Prodyut Hazarika, Mark Miesfeld; +Cc: linuxppc-dev
In-Reply-To: <20100818205646.57783157D71@gemini.denx.de>
Dear Rupjyoti, Prodyut, Mark,
two weeks ago I wrote:
In message <20100818205646.57783157D71@gemini.denx.de> you wrote:
>
> drivers/ata/sata_dwc_460ex.c fails to build in current mainline:
...
> Do you have any hints how to fix that?
Any comments or ideas how to fix this?
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Time is an illusion perpetrated by the manufacturers of space.
^ permalink raw reply
* Re: [PATCH][RFC] preempt_count corruption across H_CEDE call with CONFIG_PREEMPT on pseries
From: Darren Hart @ 2010-09-02 6:04 UTC (permalink / raw)
To: Steven Rostedt
Cc: Stephen Rothwell, Michael Neuling, Gautham R Shenoy,
Josh Triplett, linuxppc-dev, Will Schmidt, Paul Mackerras,
Ankita Garg, Thomas Gleixner
In-Reply-To: <1283400367.2356.69.camel@gandalf.stny.rr.com>
On 09/01/2010 09:06 PM, Steven Rostedt wrote:
> On Thu, 2010-09-02 at 11:02 +1000, Michael Neuling wrote:
>
>> We need to call smp_startup_cpu on boot when we the cpus are still in
>> FW. smp_startup_cpu does this for us on boot.
>>
>> I'm wondering if we just need to move the test down a bit to make sure
>> the preempt_count is set. I've not been following this thread, but
>> maybe this might work?
>
> Egad no! Setting the preempt_count to zero _is_ the bug. I think Darren
> even said that adding the exit prevented the bug (although now he's
> hitting a hard lockup someplace else). The original code he was using
> did not have the condition to return for kexec. It was just a
> coincidence that this code helped in bringing a CPU back online.
>
>>
>> Untested patch below...
>>
>> Mikey
>>
>> diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
>> index 0317cce..3afaba4 100644
>> --- a/arch/powerpc/platforms/pseries/smp.c
>> +++ b/arch/powerpc/platforms/pseries/smp.c
>> @@ -104,18 +104,18 @@ static inline int __devinit smp_startup_cpu(unsigned int lcpu)
>>
>> pcpu = get_hard_smp_processor_id(lcpu);
>>
>> - /* Check to see if the CPU out of FW already for kexec */
>> - if (smp_query_cpu_stopped(pcpu) == QCSS_NOT_STOPPED){
>> - cpumask_set_cpu(lcpu, of_spin_mask);
>> - return 1;
>> - }
>> -
>> /* Fixup atomic count: it exited inside IRQ handler. */
>> task_thread_info(paca[lcpu].__current)->preempt_count = 0;
>
> We DON'T want to do the above. It's nasty! This is one CPU's task
> touching an intimate part of another CPU's task. It's equivalent of me
> putting my hand down you wife's blouse. It's offensive, and rude.
>
> OK, if the CPU was never online, then you can do what you want. But what
> we see is that this fails on CPU hotplug. You stop a CPU, and it goes
> into this cede_processor() call. When you wake it up, suddenly the task
> on that woken CPU has its preempt count fscked up. This was really
> really hard to debug. We thought it was stack corruption or something.
> But it ended up being that this code has one CPU touching the breasts of
> another CPU. This code is a pervert!
>
> What the trace clearly showed, was that we take down a CPU, and in doing
> so, the code on that CPU set the preempt count to 1, and it expected to
> have it as 1 when it returned. But the code that kicked started this CPU
> back to life (bring the CPU back online), set the preempt count on the
> task of that CPU to 0, and screwed everything up.
Right, what Steve said.
This patch resolved the problem, but it did so inadvertently while
trying to fix kexec. I'm wondering if the offline/online code needs to
be updated to never call smp_startup_cpu(). Or perhaps this is the right
fix, and the comment needs to be cleaned up so that it isn't only for
kexec.
With this in place, we no longer see the preempt_count dropping below
zero. However, if I offline/online a CPU about 246 times I hit the
opposite problem, a preempt_count() overflow. There appears to be a
missing preempt_enable() somewhere in the offline/online paths.
On 2.6.33.7 with CONFIG_PREEMPT=y I run the following test:
# cat offon_loop.sh
#!/bin/sh
for i in `seq 100`; do
echo "Iteration $i"
echo 0 > /sys/devices/system/cpu/cpu1/online
echo 1 > /sys/devices/system/cpu/cpu1/online
done
And see the overflow:
Iteration 238
Iteration 239
Iteration 240
Message from syslogd@igoort1 at Sep 1 17:36:45 ...
kernel:------------[ cut here ]------------
Iteration 241
Iteration 242
Iteration 243
Iteration 244
Iteration 245
With the following on the console. This is the:
/*
* Spinlock count overflowing soon?
*/
DEBUG_LOCKS_WARN_ON((preempt_count() & PREEMPT_MASK) >=
PREEMPT_MASK - 10);
test. Max preempt count is 256.
Somewhere we are now MISSING a sub_preempt_count() or preempt_enable().
Badness at kernel/sched.c:5372
NIP: c000000000694f14 LR: c000000000694ef8 CTR: c00000000005b0a0
REGS: c00000008e776ae0 TRAP: 0700 Not tainted (2.6.33.7-dirty)
MSR: 8000000000021032 <ME,CE,IR,DR> CR: 28000082 XER: 00000000
TASK = c00000010e6fc040[0] 'swapper' THREAD: c00000008e774000 CPU: 1
GPR00: 0000000000000000 c00000008e776d60 c000000000af2ab8 0000000000000001
GPR04: c00000008e776fb8 0000000000000004 0000000000000001 ffffffffff676980
GPR08: 0000000000000400 c000000000bcd930 0000000000000001 c000000000b2d360
GPR12: 0000000000000002 c000000000b0f680 0000000000000000 000000000f394acc
GPR16: 0000000000000000 0000000000000000 0000000000000000 c00000008e777880
GPR20: c000000000e05160 c00000008e777870 7fffffffffffffff c000000000b0f480
GPR24: c0000000009da400 0000000000000002 0000000000000000 0000000000000001
GPR28: c00000008e776fb8 0000000000000001 c000000000a75bf0 c00000008e776d60
NIP [c000000000694f14] .add_preempt_count+0xc0/0xe0
LR [c000000000694ef8] .add_preempt_count+0xa4/0xe0
Call Trace:
[c00000008e776d60] [c00000008e776e00] 0xc00000008e776e00 (unreliable)
[c00000008e776df0] [c00000000005b0d8] .probe_hcall_entry+0x38/0x94
[c00000008e776e80] [c00000000004ef70] .__trace_hcall_entry+0x80/0xd4
[c00000008e776f10] [c00000000004f96c] .plpar_hcall_norets+0x50/0xd0
[c00000008e776f80] [c000000000055044] .smp_xics_message_pass+0x110/0x244
[c00000008e777030] [c000000000034824] .smp_send_reschedule+0x5c/0x78
[c00000008e7770c0] [c00000000006a6bc] .resched_task+0xb4/0xd8
[c00000008e777150] [c00000000006a840] .check_preempt_curr_idle+0x2c/0x44
[c00000008e7771e0] [c00000000007a868] .try_to_wake_up+0x460/0x548
[c00000008e7772b0] [c00000000007a98c] .default_wake_function+0x3c/0x54
[c00000008e777350] [c0000000000ab3b0] .autoremove_wake_function+0x44/0x84
[c00000008e777400] [c00000000006449c] .__wake_up_common+0x7c/0xf4
[c00000008e7774c0] [c00000000006a5d8] .__wake_up+0x60/0x90
[c00000008e777570] [c0000000000810dc] .printk_tick+0x84/0xa8
[c00000008e777600] [c000000000095c90] .update_process_times+0x64/0xa4
[c00000008e7776a0] [c0000000000bdcec] .tick_sched_timer+0xd0/0x120
[c00000008e777750] [c0000000000afe7c] .__run_hrtimer+0x1a0/0x29c
[c00000008e777800] [c0000000000b02a4] .hrtimer_interrupt+0x124/0x278
[c00000008e777900] [c00000000002ea90] .timer_interrupt+0x1dc/0x2e4
[c00000008e7779a0] [c000000000003700] decrementer_common+0x100/0x180
--- Exception: 901 at .raw_local_irq_restore+0x48/0x54
LR = .cpu_idle+0x12c/0x208
[c00000008e777c90] [c000000000b21130] ppc_md+0x0/0x240 (unreliable)
[c00000008e777cd0] [c000000000016a60] .cpu_idle+0x120/0x208
[c00000008e777d70] [c00000000069dbec] .start_secondary+0x394/0x3d4
[c00000008e777e30] [c000000000008278] .start_secondary_resume+0x10/0x14
Instruction dump:
409e0034 78630620 2ba300f4 40fd0028 4bd0b711 60000000 2fa30000 419e0018
e93e8880 80090000 2f800000 409e0008 <0fe00000> 383f0090 e8010010 7c0803a6
BUG: scheduling while atomic: swapper/0/0x000000f0
Modules linked in: autofs4 binfmt_misc dm_mirror dm_region_hash dm_log [last unloaded: scsi_wait_scan]
Call Trace:
[c00000008e7779a0] [c000000000014280] .show_stack+0xd8/0x218 (unreliable)
[c00000008e777a80] [c0000000006980b0] .dump_stack+0x28/0x3c
[c00000008e777b00] [c000000000071954] .__schedule_bug+0x94/0xb8
[c00000008e777b90] [c00000000068db40] .schedule+0xf8/0xbf4
[c00000008e777cd0] [c000000000016b34] .cpu_idle+0x1f4/0x208
[c00000008e777d70] [c00000000069dbec] .start_secondary+0x394/0x3d4
[c00000008e777e30] [c000000000008278] .start_secondary_resume+0x10/0x14
I'll spend tomorrow collecting traces and trying to see who's groping who this time...
--
Darren
>
> -- Steve
>
>
>>
>> if (get_cpu_current_state(lcpu) == CPU_STATE_INACTIVE)
>> goto out;
>>
>> + /* Check to see if the CPU out of FW already for kexec */
>> + if (smp_query_cpu_stopped(pcpu) == QCSS_NOT_STOPPED){
>> + cpumask_set_cpu(lcpu, of_spin_mask);
>> + return 1;
>> + }
>> +
>> /*
>> * If the RTAS start-cpu token does not exist then presume the
>> * cpu is already spinning.
>>
>
>
--
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team
^ permalink raw reply
* RE: Memory allocation modifications in ibm_newemac driver
From: Benjamin Herrenschmidt @ 2010-09-02 4:18 UTC (permalink / raw)
To: Jonathan Haws
Cc: linux-net@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org
In-Reply-To: <5ABE8A5E96C0364CAF2B4DC1DEB7CEAC127A305B@Mercury.usurf.usu.edu>
On Wed, 2010-09-01 at 22:46 +0000, Jonathan Haws wrote:
>
> Can anyone explain to me why I would be getting this error in the
> first place? Why is it failing to allocate a page when there are
> pages available? That does not make any sense to me.
order:1
It's failing to allocate -two- pages.
Cheers,
Ben.
^ permalink raw reply
* RE: Memory allocation modifications in ibm_newemac driver
From: Benjamin Herrenschmidt @ 2010-09-02 4:18 UTC (permalink / raw)
To: Jonathan Haws
Cc: linux-net@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org
In-Reply-To: <5ABE8A5E96C0364CAF2B4DC1DEB7CEAC127A301B@Mercury.usurf.usu.edu>
On Wed, 2010-09-01 at 22:04 +0000, Jonathan Haws wrote:
> Okay, I think I have all the issues worked out and can now send and
> receive any size packet without a hiccup. I have tested this in our
> system setup as well with data being sent out to disk and did not see
> any problems there either (since it only ever allocates a single page,
> never more).
>
> Is this something that may be wanted in the mainline? I have not run
> full benchmarks, but I anticipate that my modified driver is slightly
> slower than the mainline driver because we keep track of an SKB ring,
> as well as a ring of pages and allocate both on each packet received.
Well, it won't hurt to post the patch to review :-)
Make sure to CC me as I more/less maintain that driver.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH][RFC] preempt_count corruption across H_CEDE call with CONFIG_PREEMPT on pseries
From: Steven Rostedt @ 2010-09-02 4:06 UTC (permalink / raw)
To: Michael Neuling
Cc: Stephen Rothwell, Darren Hart, Gautham R Shenoy, Josh Triplett,
linuxppc-dev, Will Schmidt, Paul Mackerras, Ankita Garg,
Thomas Gleixner
In-Reply-To: <11579.1283389322@neuling.org>
On Thu, 2010-09-02 at 11:02 +1000, Michael Neuling wrote:
> We need to call smp_startup_cpu on boot when we the cpus are still in
> FW. smp_startup_cpu does this for us on boot.
>
> I'm wondering if we just need to move the test down a bit to make sure
> the preempt_count is set. I've not been following this thread, but
> maybe this might work?
Egad no! Setting the preempt_count to zero _is_ the bug. I think Darren
even said that adding the exit prevented the bug (although now he's
hitting a hard lockup someplace else). The original code he was using
did not have the condition to return for kexec. It was just a
coincidence that this code helped in bringing a CPU back online.
>
> Untested patch below...
>
> Mikey
>
> diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
> index 0317cce..3afaba4 100644
> --- a/arch/powerpc/platforms/pseries/smp.c
> +++ b/arch/powerpc/platforms/pseries/smp.c
> @@ -104,18 +104,18 @@ static inline int __devinit smp_startup_cpu(unsigned int lcpu)
>
> pcpu = get_hard_smp_processor_id(lcpu);
>
> - /* Check to see if the CPU out of FW already for kexec */
> - if (smp_query_cpu_stopped(pcpu) == QCSS_NOT_STOPPED){
> - cpumask_set_cpu(lcpu, of_spin_mask);
> - return 1;
> - }
> -
> /* Fixup atomic count: it exited inside IRQ handler. */
> task_thread_info(paca[lcpu].__current)->preempt_count = 0;
We DON'T want to do the above. It's nasty! This is one CPU's task
touching an intimate part of another CPU's task. It's equivalent of me
putting my hand down you wife's blouse. It's offensive, and rude.
OK, if the CPU was never online, then you can do what you want. But what
we see is that this fails on CPU hotplug. You stop a CPU, and it goes
into this cede_processor() call. When you wake it up, suddenly the task
on that woken CPU has its preempt count fscked up. This was really
really hard to debug. We thought it was stack corruption or something.
But it ended up being that this code has one CPU touching the breasts of
another CPU. This code is a pervert!
What the trace clearly showed, was that we take down a CPU, and in doing
so, the code on that CPU set the preempt count to 1, and it expected to
have it as 1 when it returned. But the code that kicked started this CPU
back to life (bring the CPU back online), set the preempt count on the
task of that CPU to 0, and screwed everything up.
-- Steve
>
> if (get_cpu_current_state(lcpu) == CPU_STATE_INACTIVE)
> goto out;
>
> + /* Check to see if the CPU out of FW already for kexec */
> + if (smp_query_cpu_stopped(pcpu) == QCSS_NOT_STOPPED){
> + cpumask_set_cpu(lcpu, of_spin_mask);
> + return 1;
> + }
> +
> /*
> * If the RTAS start-cpu token does not exist then presume the
> * cpu is already spinning.
>
^ permalink raw reply
* Re: [PATCH][RFC] preempt_count corruption across H_CEDE call with CONFIG_PREEMPT on pseries
From: Michael Neuling @ 2010-09-02 3:46 UTC (permalink / raw)
To: Darren Hart
Cc: Stephen Rothwell, Gautham R Shenoy, Josh Triplett, Steven Rostedt,
linuxppc-dev, Will Schmidt, Paul Mackerras, Ankita Garg,
Thomas Gleixner
In-Reply-To: <4C7E9FC1.60004@us.ibm.com>
> + /* Check to see if the CPU out of FW already for kexec */
Wow, that comment is shit. The checkin comment in
aef40e87d866355ffd279ab21021de733242d0d5 is much better.
> This comment is really confusing to me. I _think_ it is saying that this test
> determines if the CPU is done executing firmware code and has begun executing
> OS code.... Is that right?
Yeah.
It means for a normal boot, the CPU will not have started yet (still in
firmware (FW)) so we have to call FW to bring it out. In the kexec case
though, the CPU will have started already (it's spinning in the kernel)
so we don't have to call FW to bring it back out again. To distinguish
between these two cases, we ask FW if the CPU has started or not (via
smp_query_cpu_stopped()) and if it's already start, don't restart it.
Originally, we could call FW to start a cpu that was already started,
but FW changed recently and stopped allowing us to do this. Hence this
patch.
Mikey
^ permalink raw reply
* Re: [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2010-09-02 1:55 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linuxppc-dev list, Andrew Morton, Linux Kernel list
In-Reply-To: <1283234198.2151.15.camel@pasglop>
On Tue, 2010-08-31 at 15:56 +1000, Benjamin Herrenschmidt wrote:
> Hi Linus !
>
> Here's a few small fixes, one is an important fix for a nasty regression
> breaking pseries machine running under hypervisor... oops !
I updated this with some embedded fixed from Kumar and a fix for a new
deadlock in the pseries dlpar code.
New log below.
Cheers,
Ben.
The following changes since commit 2bfc96a127bc1cc94d26bfaa40159966064f9c8c:
Linus Torvalds (1):
Linux 2.6.36-rc3
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge
Alexander Graf (1):
powerpc/85xx: Fix compilation of mpc85xx_mds.c
Anton Vorontsov (1):
powerpc/85xx: Add P1021 PCI IDs and quirks
Julia Lawall (2):
arch/powerpc/platforms/83xx/mpc837x_mds.c: Add missing iounmap
arch/powerpc/sysdev/qe_lib/qe.c: Add of_node_put to avoid memory leak
Kumar Gala (1):
powerpc/85xx: Fix compile issue with p1022_ds due to lmb rename to memblock
Li Yang (1):
fsl_rio: fix compile errors
Matthew McClintock (1):
powerpc/kexec: Adds correct calling convention for kexec purgatory
Michael Neuling (1):
powerpc: Don't use kernel stack with translation off
Nathan Fontenot (1):
powerpc/pseries: Correct rtas_data_buf locking in dlpar code
Paul Mackerras (1):
powerpc/perf_event: Reduce latency of calling perf_event_do_pending
arch/powerpc/kernel/head_64.S | 12 ++++++--
arch/powerpc/kernel/misc_32.S | 3 ++
arch/powerpc/kernel/time.c | 23 +++++++--------
arch/powerpc/platforms/83xx/mpc837x_mds.c | 9 ++++--
arch/powerpc/platforms/85xx/mpc85xx_mds.c | 1 +
arch/powerpc/platforms/85xx/p1022_ds.c | 4 +-
arch/powerpc/platforms/pseries/dlpar.c | 42 ++++++++++++++++++++---------
arch/powerpc/sysdev/fsl_pci.c | 2 +
arch/powerpc/sysdev/fsl_rio.c | 6 +++-
arch/powerpc/sysdev/qe_lib/qe.c | 1 +
include/linux/pci_ids.h | 2 +
11 files changed, 71 insertions(+), 34 deletions(-)
^ 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