* [PATCH 4/6] gianfar: Fix thinko in gfar_set_rx_stash_index()
From: Anton Vorontsov @ 2009-11-11 0:11 UTC (permalink / raw)
To: David Miller
Cc: Jon Loeliger, Kumar Gopalpet-B05799, netdev, linuxppc-dev,
Andy Fleming, Stephen Hemminger, Lennert Buytenhek
In-Reply-To: <20091111001036.GA28576@oksana.dev.rtsoft.ru>
We obviously want to write a modified 'temp' value back to the
register, not the saved IRQ flags.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/gianfar_sysfs.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/gianfar_sysfs.c b/drivers/net/gianfar_sysfs.c
index 3724835..b31c9c8 100644
--- a/drivers/net/gianfar_sysfs.c
+++ b/drivers/net/gianfar_sysfs.c
@@ -186,7 +186,7 @@ static ssize_t gfar_set_rx_stash_index(struct device *dev,
temp = gfar_read(®s->attreli);
temp &= ~ATTRELI_EI_MASK;
temp |= ATTRELI_EI(index);
- gfar_write(®s->attreli, flags);
+ gfar_write(®s->attreli, temp);
out:
unlock_rx_qs(priv);
--
1.6.3.3
^ permalink raw reply related
* [PATCH 3/6] gianfar: Fix build with CONFIG_PM=y
From: Anton Vorontsov @ 2009-11-11 0:11 UTC (permalink / raw)
To: David Miller
Cc: Jon Loeliger, Kumar Gopalpet-B05799, netdev, linuxppc-dev,
Andy Fleming, Stephen Hemminger, Lennert Buytenhek
In-Reply-To: <20091111001036.GA28576@oksana.dev.rtsoft.ru>
commit fba4ed030cfae7efdb6b79a57b0c5a9d72c9 ("gianfar: Add Multiple
Queue Support") introduced the following build failure:
CC gianfar.o
gianfar.c: In function 'gfar_restore':
gianfar.c:1249: error: request for member 'napi' in something not a structure or union
This patch fixes the issue.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/gianfar.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 79c28f5..a5b0038 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -1246,7 +1246,7 @@ static int gfar_restore(struct device *dev)
phy_start(priv->phydev);
netif_device_attach(ndev);
- napi_enable(&priv->gfargrp.napi);
+ enable_napi(priv);
return 0;
}
--
1.6.3.3
^ permalink raw reply related
* [PATCH 2/6] gianfar: Remove 'Interrupt problem!' warning
From: Anton Vorontsov @ 2009-11-11 0:11 UTC (permalink / raw)
To: David Miller
Cc: Jon Loeliger, Kumar Gopalpet-B05799, netdev, linuxppc-dev,
Andy Fleming, Stephen Hemminger, Lennert Buytenhek
In-Reply-To: <20091111001036.GA28576@oksana.dev.rtsoft.ru>
It is OK to poll with disabled IRQs, so remove the warning.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/gianfar.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 197b358..79c28f5 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -2504,8 +2504,6 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
skb_put(skb, pkt_len);
dev->stats.rx_bytes += pkt_len;
- if (in_irq() || irqs_disabled())
- printk("Interrupt problem!\n");
gfar_process_frame(dev, skb, amount_pull);
} else {
--
1.6.3.3
^ permalink raw reply related
* [PATCH 1/6] skbuff: Do not allow skb recycling with disabled IRQs
From: Anton Vorontsov @ 2009-11-11 0:11 UTC (permalink / raw)
To: David Miller
Cc: Jon Loeliger, Kumar Gopalpet-B05799, netdev, linuxppc-dev,
Andy Fleming, Stephen Hemminger, Lennert Buytenhek
In-Reply-To: <20091111001036.GA28576@oksana.dev.rtsoft.ru>
NAPI drivers try to recycle SKBs in their polling routine, but we
generally don't know the context in which the polling will be called,
and the skb recycling itself may require IRQs to be enabled.
This patch adds irqs_disabled() test to the skb_recycle_check()
routine, so that we'll not let the drivers hit the skb recycling
path with IRQs disabled.
As a side effect, this patch actually disables skb recycling for some
[broken] drivers. E.g. gianfar driver grabs an irqsave spinlock during
TX ring processing, and then tries to recycle an skb, and that caused
the following badness:
nf_conntrack version 0.5.0 (1008 buckets, 4032 max)
------------[ cut here ]------------
Badness at kernel/softirq.c:143
NIP: c003e3c4 LR: c423a528 CTR: c003e344
...
NIP [c003e3c4] local_bh_enable+0x80/0xc4
LR [c423a528] destroy_conntrack+0xd4/0x13c [nf_conntrack]
Call Trace:
[c15d1b60] [c003e32c] local_bh_disable+0x1c/0x34 (unreliable)
[c15d1b70] [c423a528] destroy_conntrack+0xd4/0x13c [nf_conntrack]
[c15d1b80] [c02c6370] nf_conntrack_destroy+0x3c/0x70
--- Exception: c428168c at 0xc15d1c50
LR = 0xc15d1c40
[c15d1ba0] [c0286f3c] skb_release_head_state+0x100/0x104 (unreliable)
[c15d1bb0] [c0288340] skb_recycle_check+0x8c/0x10c
[c15d1bc0] [c01e1688] gfar_poll+0x190/0x384
[c15d1c10] [c02935ac] net_rx_action+0xec/0x22c
[c15d1c50] [c003dd8c] __do_softirq+0xe8/0x224
...
Reported-by: Jon Loeliger <jdl@jdl.com>
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
net/core/skbuff.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 80a9616..941bac9 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -493,6 +493,9 @@ int skb_recycle_check(struct sk_buff *skb, int skb_size)
{
struct skb_shared_info *shinfo;
+ if (irqs_disabled())
+ return 0;
+
if (skb_is_nonlinear(skb) || skb->fclone != SKB_FCLONE_UNAVAILABLE)
return 0;
--
1.6.3.3
^ permalink raw reply related
* [PATCH 0/6] gianfar: Some fixes
From: Anton Vorontsov @ 2009-11-11 0:10 UTC (permalink / raw)
To: David Miller
Cc: Jon Loeliger, Kumar Gopalpet-B05799, netdev, linuxppc-dev,
Andy Fleming, Stephen Hemminger, Lennert Buytenhek
Hi all,
Here are some fixes for the gianfar driver, patches on the way.
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
From: Joakim Tjernlund @ 2009-11-11 0:06 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <4AF9F56E.9080104@freescale.com>
Scott Wood <scottwood@freescale.com> wrote on 11/11/2009 00:21:18:
>
> Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 23:02:10:
> >> Joakim Tjernlund wrote:
> >> It wasn't the CPU15 workaround that I was worried about taking down the
> >> pinning -- but rather the CPU15 bug itself causing bad code to be
> >> executed inside the pinned kernel mapping.
> >
> > Oh, but then one is screwed anyway.
>
> Not if there's a workaround...
>
> >> However, the erratum says "MMU page", not "4K region", so I suppose if
> >> we have a pinned 8M page the problem could only occur at the end of the
> >> 8M (by which point the text segment should have ended).
> >
> > The wording makes me wonder why not a dcbi would fix the problem too.
> > Invalidate the problem cache lines and let the branch resolve.
>
> Where would you put the dcbi? How do you regain control after that
> cache line has been refilled, but before code flows back to the bad branch?
The dcbi would replace the current CPU15 tlbie.
I would try mitigating bullet 3 (unresolved long enough ...) by
invalidating n+1 cache line. Hopefully that will abort the prefetch.
If that doesn't work, invalidate n+2(bullet 4) as it says it must be in cache for the problem
to manifest.
But I see now that I probably misread the errata so the above wont work.
^ permalink raw reply
* Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
From: Scott Wood @ 2009-11-10 23:21 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <OF1FCFA636.9185793A-ONC125766A.007F44D4-C125766A.007FCA22@transmode.se>
Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 23:02:10:
>> Joakim Tjernlund wrote:
>> It wasn't the CPU15 workaround that I was worried about taking down the
>> pinning -- but rather the CPU15 bug itself causing bad code to be
>> executed inside the pinned kernel mapping.
>
> Oh, but then one is screwed anyway.
Not if there's a workaround...
>> However, the erratum says "MMU page", not "4K region", so I suppose if
>> we have a pinned 8M page the problem could only occur at the end of the
>> 8M (by which point the text segment should have ended).
>
> The wording makes me wonder why not a dcbi would fix the problem too.
> Invalidate the problem cache lines and let the branch resolve.
Where would you put the dcbi? How do you regain control after that
cache line has been refilled, but before code flows back to the bad branch?
-Scott
^ permalink raw reply
* Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
From: Joakim Tjernlund @ 2009-11-10 23:15 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <4AF9E2E2.7030100@freescale.com>
Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 23:02:10:
>
> Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 22:36:32:
> >> Joakim Tjernlund wrote:
> >>> yes, maybe there is a way around that. Perhaps by using one of the
> >>> pinned entries for loaded modules, i.e avoid ITLB misses for kernel space?
> >> Not sure what you mean... loaded modules won't be pinned, and since
> >> they shouldn't contain rfi, don't need to be.
> >
> > But CPU15 may invalidate a pinned TLB if you take a TLB Miss?
> > If not there should not be a problem, because the rest
> > of the kernel will never take a ITLB Miss.
>
> It wasn't the CPU15 workaround that I was worried about taking down the
> pinning -- but rather the CPU15 bug itself causing bad code to be
> executed inside the pinned kernel mapping.
Oh, but then one is screwed anyway.
>
> However, the erratum says "MMU page", not "4K region", so I suppose if
> we have a pinned 8M page the problem could only occur at the end of the
> 8M (by which point the text segment should have ended).
The wording makes me wonder why not a dcbi would fix the problem too.
Invalidate the problem cache lines and let the branch resolve.
>
> Unless we have any evidence that this is not what the erratum means, I'd
> say make pinning mandatory, and avoid placing modules immediately after
> a pinned entry.
It is worth a try.
>
> > BTW, you could probably cram the DARFix into the DTLBerror with some luck.
> > Especially if you allow it to spill over to the next trap. Then create a
> > branch insn at 0x1500 to 0x1600. Would that make everything aligned again?
>
> Yes, until some other code change breaks it again.
>
> -Scott
>
>
^ permalink raw reply
* Re: [spi-devel-general] [PATCH v4] xilinx_spi: Splitted into generic, of and platform driver, added support for DS570
From: Richard Röjfors @ 2009-11-10 16:19 UTC (permalink / raw)
To: Grant Likely
Cc: spi-devel-general, Andrew Morton, dbrownell, John Linn,
linuxppc-dev
In-Reply-To: <fa686aa40911091321w187fe85fl4e6d9f36f5966f34@mail.gmail.com>
Grant Likely wrote:
> Oops, I replied to the original version, but missed the subsequent
> versions. Looks like some of my comments still apply though.
> Overall, the patch changes too many things all at once. You should
> look at splitting it up. At the very least the io accessor changes
> should be done in a separate patch.
>
> On Mon, Sep 28, 2009 at 7:22 AM, Richard Röjfors
> <richard.rojfors@mocean-labs.com> wrote:
>> @@ -227,6 +227,21 @@ config SPI_XILINX
>> See the "OPB Serial Peripheral Interface (SPI) (v1.00e)"
>> Product Specification document (DS464) for hardware details.
>>
>> + Or for the DS570, see "XPS Serial Peripheral Interface (SPI) (v2.00b)"
>> +
>> +config SPI_XILINX_OF
>> + tristate "Xilinx SPI controller OF device"
>> + depends on SPI_XILINX && XILINX_VIRTEX
>> + help
>> + This is the OF driver for the SPI controller IP from the Xilinx EDK.
>> +
>> +config SPI_XILINX_PLTFM
>> + tristate "Xilinx SPI controller platform device"
>> + depends on SPI_XILINX
>> + help
>> + This is the platform driver for the SPI controller IP
>> + from the Xilinx EDK.
>> +
>
> Personally, I don't think it is necessary to present these options to
> the user. I think they can be auto-selected depending on CONFIG_OF
> and CONFIG_PLATFORM.
Sure that can be changed, I prefer to to that in an incremental patch after this one.
>
>> +struct xilinx_spi {
>> + /* bitbang has to be first */
>> + struct spi_bitbang bitbang;
>> + struct completion done;
>> + struct resource mem; /* phys mem */
>> + void __iomem *regs; /* virt. address of the control registers */
>> + u32 irq;
>> + u8 *rx_ptr; /* pointer in the Tx buffer */
>> + const u8 *tx_ptr; /* pointer in the Rx buffer */
>> + int remaining_bytes; /* the number of bytes left to transfer */
>> + /* offset to the XSPI regs, these might vary... */
>> + u8 bits_per_word;
>> + bool big_endian; /* The device could be accessed big or little
>> + * endian
>> + */
>> +};
>> +
>
> Why is the definition of xilinx_spi moved?
I liked the idea of heaving the struct defined in the top of the file.
>> /* Register definitions as per "OPB Serial Peripheral Interface (SPI) (v1.00e)
>> * Product Specification", DS464
>> */
>> -#define XSPI_CR_OFFSET 0x62 /* 16-bit Control Register */
>> +#define XSPI_CR_OFFSET 0x60 /* Control Register */
>>
>> #define XSPI_CR_ENABLE 0x02
>> #define XSPI_CR_MASTER_MODE 0x04
>> @@ -40,8 +53,9 @@
>> #define XSPI_CR_RXFIFO_RESET 0x40
>> #define XSPI_CR_MANUAL_SSELECT 0x80
>> #define XSPI_CR_TRANS_INHIBIT 0x100
>> +#define XSPI_CR_LSB_FIRST 0x200
>>
>> -#define XSPI_SR_OFFSET 0x67 /* 8-bit Status Register */
>> +#define XSPI_SR_OFFSET 0x64 /* Status Register */
>>
>> #define XSPI_SR_RX_EMPTY_MASK 0x01 /* Receive FIFO is empty */
>> #define XSPI_SR_RX_FULL_MASK 0x02 /* Receive FIFO is full */
>> @@ -49,8 +63,8 @@
>> #define XSPI_SR_TX_FULL_MASK 0x08 /* Transmit FIFO is full */
>> #define XSPI_SR_MODE_FAULT_MASK 0x10 /* Mode fault error */
>>
>> -#define XSPI_TXD_OFFSET 0x6b /* 8-bit Data Transmit Register */
>> -#define XSPI_RXD_OFFSET 0x6f /* 8-bit Data Receive Register */
>> +#define XSPI_TXD_OFFSET 0x68 /* Data Transmit Register */
>> +#define XSPI_RXD_OFFSET 0x6C /* Data Receive Register */
>>
>> #define XSPI_SSR_OFFSET 0x70 /* 32-bit Slave Select Register */
>>
>> @@ -70,43 +84,72 @@
>> #define XSPI_INTR_TX_UNDERRUN 0x08 /* TxFIFO was underrun */
>> #define XSPI_INTR_RX_FULL 0x10 /* RxFIFO is full */
>> #define XSPI_INTR_RX_OVERRUN 0x20 /* RxFIFO was overrun */
>> +#define XSPI_INTR_TX_HALF_EMPTY 0x40 /* TxFIFO is half empty */
>>
>> #define XIPIF_V123B_RESETR_OFFSET 0x40 /* IPIF reset register */
>> #define XIPIF_V123B_RESET_MASK 0x0a /* the value to write */
>>
>> -struct xilinx_spi {
>> - /* bitbang has to be first */
>> - struct spi_bitbang bitbang;
>> - struct completion done;
>> +/* to follow are some functions that does little of big endian read and
>> + * write depending on the config of the device.
>> + */
>> +static inline void xspi_write8(struct xilinx_spi *xspi, u32 offs, u8 val)
>> +{
>> + iowrite8(val, xspi->regs + offs + ((xspi->big_endian) ? 3 : 0));
>> +}
>>
>> - void __iomem *regs; /* virt. address of the control registers */
>> +static inline void xspi_write16(struct xilinx_spi *xspi, u32 offs, u16 val)
>> +{
>> + if (xspi->big_endian)
>> + iowrite16be(val, xspi->regs + offs + 2);
>> + else
>> + iowrite16(val, xspi->regs + offs);
>> +}
>>
>> - u32 irq;
>> +static inline void xspi_write32(struct xilinx_spi *xspi, u32 offs, u32 val)
>> +{
>> + if (xspi->big_endian)
>> + iowrite32be(val, xspi->regs + offs);
>> + else
>> + iowrite32(val, xspi->regs + offs);
>> +}
>>
>> - u32 speed_hz; /* SCK has a fixed frequency of speed_hz Hz */
>> +static inline u8 xspi_read8(struct xilinx_spi *xspi, u32 offs)
>> +{
>> + return ioread8(xspi->regs + offs + ((xspi->big_endian) ? 3 : 0));
>> +}
>>
>> - u8 *rx_ptr; /* pointer in the Tx buffer */
>> - const u8 *tx_ptr; /* pointer in the Rx buffer */
>> - int remaining_bytes; /* the number of bytes left to transfer */
>> -};
>> +static inline u16 xspi_read16(struct xilinx_spi *xspi, u32 offs)
>> +{
>> + if (xspi->big_endian)
>> + return ioread16be(xspi->regs + offs + 2);
>> + else
>> + return ioread16(xspi->regs + offs);
>> +}
>> +
>> +static inline u32 xspi_read32(struct xilinx_spi *xspi, u32 offs)
>> +{
>> + if (xspi->big_endian)
>> + return ioread32be(xspi->regs + offs);
>> + else
>> + return ioread32(xspi->regs + offs);
>> +}
>
> Ah, you changed these to functions instead of macros. I like.
> However, as you suggested elsewhere in this thread, you could change
> these to callbacks and then eliminate the if/else statements. I think
> that is the approach you should use. I don't think you need to worry
> about it being slower. Any extra cycles for jumping to a callback
> will be far dwarfed by the number of cycles it takes to complete an
> SPI transfer.
Sure that can be updated. I prefer to do that in an incremental patch, would be great to get this
big one merged first.
--Richard
^ permalink raw reply
* Re: [PATCH 3/3] mpc52xx/wdt: WDT uses GPT api
From: Grant Likely @ 2009-11-10 19:59 UTC (permalink / raw)
To: Albrecht Dreß
Cc: Linux PPC Development, devicetree-discuss, Wim Van Sebroeck
In-Reply-To: <1257882180.14374.3@antares>
On Tue, Nov 10, 2009 at 12:43 PM, Albrecht Dre=DF <albrecht.dress@arcor.de>=
wrote:
> Use the MPC5200 GPT api for the WDT which drastically simplifies this fil=
e.
>
> Signed-off-by: Albrecht Dre=DF <albrecht.dress@arcor.de>
> ---
>
> =A0drivers/watchdog/mpc5200_wdt.c | =A0246 +++++++++++-------------------=
----------
> =A01 files changed, 65 insertions(+), 181 deletions(-)
Can the WDT functionality just be merged entirely into
arch/powerpc/platforms/52xx/mpc52xx_gpt.c, eliminating the need for
this file entirely? I think I'd rather have all the GPT "built in"
behaviour handled by a single driver.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 3/3] mpc52xx/wdt: WDT uses GPT api
From: Albrecht Dreß @ 2009-11-10 20:26 UTC (permalink / raw)
To: Grant Likely; +Cc: Linux PPC Development, devicetree-discuss, Wim Van Sebroeck
In-Reply-To: <fa686aa40911101159y7bef3f9bte74e9aeb5c6e864b@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1472 bytes --]
Hi Grant:
Am 10.11.09 20:59 schrieb(en) Grant Likely:
> On Tue, Nov 10, 2009 at 12:43 PM, Albrecht Dreß <albrecht.dress@arcor.de> wrote:
> > Use the MPC5200 GPT api for the WDT which drastically simplifies this file.
> >
> > Signed-off-by: Albrecht Dreß <albrecht.dress@arcor.de>
> > ---
> >
> > drivers/watchdog/mpc5200_wdt.c | 246 +++++++++++-----------------------------
> > 1 files changed, 65 insertions(+), 181 deletions(-)
>
>
> Can the WDT functionality just be merged entirely into
> arch/powerpc/platforms/52xx/mpc52xx_gpt.c, eliminating the need for
> this file entirely? I think I'd rather have all the GPT "built in"
> behaviour handled by a single driver.
I also thought about it, as it has IMHO the cleaner code, and it would have the extra benefit that the gpt-wdt api doesn't need to be public.
However, the reasons I hesitated to do so are:
- I don't want to remove a file someone else wrote (even it doesn't work);
- WDT code is shifted from drivers/watchdog to arch/powerpc/platforms/52xx which might not be the "logical" place from the directory layout's pov;
- a file living in arch/powerpc/platforms/52xx depends upon config options set from drivers/watchdog/Kconfig which may be confusing.
You see these are more political/cosmetical questions, so I would prefer to leave the decision to the maintainers (i.e. you and Wim). Preparing a fully merged driver is actually a matter of minutes!
Thanks,
Albrecht.
[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]
^ permalink raw reply
* Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
From: Joakim Tjernlund @ 2009-11-10 19:08 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <4AF99B00.6080504@freescale.com>
Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 17:55:28:
>
> Scott Wood wrote:
> > Joakim Tjernlund wrote:
> >> Why does not pinning interact well with CPU15? If pinned, you never get
> >> a TLB miss for kernel text so that should mitigate the CPU15 problem.
> >
> > The nature of the workaround for CPU15 is that we can't keep it pinned
> > -- we have to take an ITLB miss on every page boundary crossing. If you
> > try to pin, it'll just be invalidated by the workaround.
>
> Except that the invalidation only happens when you take an ITLB miss on
> an adjacent page, which means we'd likely never get CPU15 protection for
> kernel code if pinning is enabled. :-(
So tlbie invalidates pinned TLBs too?
It is likely that you won't get ITLB Misses for normal kernel
space but for modules you will. Also, since the pinned D&I TLBs overlap,
how do you make sure that invalidating a kernel DTLB won't
spill over to the pinned ITLB?
Does pinned TLBs work for you?
Rex, how does it look in your end?
Jocke
^ permalink raw reply
* [PATCH 0/3] mpc52xx/wdt: re-enable the MPC5200 WDT
From: Albrecht Dreß @ 2009-11-10 19:40 UTC (permalink / raw)
To: Linux PPC Development, Grant Likely, devicetree-discuss,
Wim Van Sebroeck
This set of patches merges the MPC5200 WDT into the GPT code, making it
functional again - currently, the MPC5200 GPT code blocks using the WDT.
Additionally, it defines a new OF property as to reserve and/or enable the
WDT during the boot process which may be a requirement for safety-related
(e.g. ISO/EN 61508) applications.
Note that all patches are against Grant Likely's "next" tree.
Cheers, Albrecht.
^ permalink raw reply
* RE: [spi-devel-general] [PATCH v4] xilinx_spi: Splitted into generic, of and platform driver, added support for DS570
From: John Linn @ 2009-11-10 17:23 UTC (permalink / raw)
To: Grant Likely, Richard Röjfors
Cc: spi-devel-general, Andrew Morton, dbrownell, linuxppc-dev
In-Reply-To: <fa686aa40911100844q19789297h8e6713e915185a7@mail.gmail.com>
> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On Behalf Of Gra=
nt Likely
> Sent: Tuesday, November 10, 2009 9:45 AM
> To: Richard R=F6jfors
> Cc: spi-devel-general@lists.sourceforge.net; linuxppc-dev@ozlabs.org; And=
rew Morton;
> dbrownell@users.sourceforge.net; John Linn
> Subject: Re: [spi-devel-general] [PATCH v4] xilinx_spi: Splitted into gen=
eric, of and platform
> driver, added support for DS570
> =
> On Tue, Nov 10, 2009 at 9:19 AM, Richard R=F6jfors
> <richard.rojfors@mocean-labs.com> wrote:
> > Grant Likely wrote:
> >> Oops, I replied to the original version, but missed the subsequent
> >> versions. =A0Looks like some of my comments still apply though.
> >> Overall, the patch changes too many things all at once. =A0You should
> >> look at splitting it up. =A0At the very least the io accessor changes
> >> should be done in a separate patch.
> =
> Hi Richard. Please do another spin of this patch. I don't have any
> particular problem with the changes, but it needs to be in a more
> granular form so I can review it properly.
I agree. =
We have a functioning driver such that I need to make sure it doesn't regre=
ss. More granular changes will also allow us to better isolate any problems=
if they occur.
Thanks,
John
> =
> >>> +struct xilinx_spi {
> >>> + =A0 =A0 =A0 /* bitbang has to be first */
> >>> + =A0 =A0 =A0 struct spi_bitbang bitbang;
> >>> + =A0 =A0 =A0 struct completion done;
> >>> + =A0 =A0 =A0 struct resource mem; /* phys mem */
> >>> + =A0 =A0 =A0 void __iomem =A0 =A0*regs; =A0/* virt. address of the c=
ontrol registers */
> >>> + =A0 =A0 =A0 u32 irq;
> >>> + =A0 =A0 =A0 u8 *rx_ptr; =A0 =A0 =A0 =A0 =A0 =A0 /* pointer in the T=
x buffer */
> >>> + =A0 =A0 =A0 const u8 *tx_ptr; =A0 =A0 =A0 /* pointer in the Rx buff=
er */
> >>> + =A0 =A0 =A0 int remaining_bytes; =A0 =A0/* the number of bytes left=
to transfer */
> >>> + =A0 =A0 =A0 /* offset to the XSPI regs, these might vary... */
> >>> + =A0 =A0 =A0 u8 bits_per_word;
> >>> + =A0 =A0 =A0 bool big_endian; =A0 =A0 =A0 =A0/* The device could be =
accessed big or little
> >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* en=
dian
> >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
> >>> +};
> >>> +
> >>
> >> Why is the definition of xilinx_spi moved?
> >
> > I liked the idea of heaving the struct defined in the top of the file.
> =
> ... which is completely unrelated to the patch purpose, and is not
> mentioned in the patch header. If you really want to move it then put
> it in a completely separate patch and describe the change properly.
> As it is right now it is just noise that makes the stated purpose of
> the patch hard to review.
> =
> >> Ah, you changed these to functions instead of macros. =A0I like.
> >> However, as you suggested elsewhere in this thread, you could change
> >> these to callbacks and then eliminate the if/else statements. =A0I thi=
nk
> >> that is the approach you should use. =A0I don't think you need to worr=
y
> >> about it being slower. =A0Any extra cycles for jumping to a callback
> >> will be far dwarfed by the number of cycles it takes to complete an
> >> SPI transfer.
> >
> > Sure that can be updated. I prefer to do that in an incremental patch, =
would be great to get this
> > big one merged first.
> =
> As already commented on, this patch is too big and does too many
> unrelated things. Please split into discrete changes so it can be
> reviewed properly.
> =
> Thanks,
> g.
> =
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
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 0/8] 8xx: Misc fixes for buggy insn
From: Joakim Tjernlund @ 2009-11-10 19:54 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <20091109230004.GA24671@loki.buserror.net>
Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 00:00:04:
>
> On Mon, Nov 09, 2009 at 03:53:21PM -0600, Scott Wood wrote:
> > On Fri, Nov 06, 2009 at 10:29:44AM +0100, Joakim Tjernlund wrote:
> > > > > With this, the kernel hangs after "Mount-cache hash table entries: 512".
> > > >
> > > > Somewhat surprising result. I didn't expect you would even hit this
> > > > condition now as we haven't enabled the use of dcbX insn yet.
> > > > The only thing I can think of is the you hit the 0x00f0 due to other
> > > > dcbX insn use and the kernel managed to fixup this in the page fault handler
> > > > by pure luck before.
> >
> > It's bizarre -- it still happens even if I revert the branch to FixupDAR.
> > However, if I comment out the final "b 151b", it stops happening. It also
> > stops happening sometimes depending on where I stick printk()s to debug
> > this.
> >
> > So it may be an unrelated issue that just got perturbed somehow.
>
> OK, figured it out. The fixup code pushed things around so that in
> syscall_exit_cont, SRR0/SRR1 were being loaded immediately prior to a page
> boundary, with the rfi after the page boundary. On crossing the boundary,
> we take an ITLB miss (which goes from possibility to certainty with the
> CPU15 workaround), and SRR0/SRR1 get clobbered.
I think I have misunderstood, its is not CPU15 or 8xx problem per se, it
is a general problem that could happen to any ppc CPU, right?
8xx just happen to be the first CPU that hits this case due to my
DAR fixing
>
> I suppose we'll need to find all places where we do rfi with the MMU
> enabled, and ensure acceptable alignment. :-(
That seems the right fix, the asm in entry_32.S needs have some alignment
here and there to make sure you don't cross a page boundary at the wrong time.
^ permalink raw reply
* Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
From: Joakim Tjernlund @ 2009-11-10 21:05 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <4AF9C647.50600@freescale.com>
Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 21:00:07:
>
> Joakim Tjernlund wrote:
> > I think I have misunderstood, its is not CPU15 or 8xx problem per se, it
> > is a general problem that could happen to any ppc CPU, right?
> > 8xx just happen to be the first CPU that hits this case due to my
> > DAR fixing
>
> Is there any chip other than 8xx where the kernel text is not always pinned?
No idea, something for the list to comment on.
But if so 8xx should do the same(at least for ITLB), there is
no other way to guarantee this problem won't happen I think.
Jocke
^ permalink raw reply
* [PATCH 3/3] mpc52xx/wdt: WDT uses GPT api
From: Albrecht Dreß @ 2009-11-10 19:43 UTC (permalink / raw)
To: Linux PPC Development, Grant Likely, devicetree-discuss,
Wim Van Sebroeck
Use the MPC5200 GPT api for the WDT which drastically simplifies this file.
Signed-off-by: Albrecht Dre=DF <albrecht.dress@arcor.de>
---
drivers/watchdog/mpc5200_wdt.c | 246 +++++++++++-------------------------=
----
1 files changed, 65 insertions(+), 181 deletions(-)
diff --git a/drivers/watchdog/mpc5200_wdt.c b/drivers/watchdog/mpc5200_wdt.=
c
index fa9c47c..5bb553c 100644
--- a/drivers/watchdog/mpc5200_wdt.c
+++ b/drivers/watchdog/mpc5200_wdt.c
@@ -2,25 +2,16 @@
#include <linux/module.h>
#include <linux/miscdevice.h>
#include <linux/watchdog.h>
-#include <linux/io.h>
-#include <linux/spinlock.h>
#include <linux/of_platform.h>
#include <linux/uaccess.h>
#include <asm/mpc52xx.h>
=20
=20
-#define GPT_MODE_WDT (1 << 15)
-#define GPT_MODE_CE (1 << 12)
-#define GPT_MODE_MS_TIMER (0x4)
-
+#define WDT_IDENTITY "mpc5200 watchdog on GPT0"
=20
struct mpc5200_wdt {
- unsigned count; /* timer ticks before watchdog kicks in */
- long ipb_freq;
- struct miscdevice miscdev;
- struct resource mem;
- struct mpc52xx_gpt __iomem *regs;
- spinlock_t io_lock;
+ int timeout;
+ struct mpc52xx_gpt_priv *timer;
};
=20
/* is_active stores wether or not the /dev/watchdog device is opened */
@@ -32,80 +23,33 @@ static unsigned long is_active;
static struct mpc5200_wdt *wdt_global;
=20
=20
-/* helper to calculate timeout in timer counts */
-static void mpc5200_wdt_set_timeout(struct mpc5200_wdt *wdt, int timeout)
-{
- /* use biggest prescaler of 64k */
- wdt->count =3D (wdt->ipb_freq + 0xffff) / 0x10000 * timeout;
-
- if (wdt->count > 0xffff)
- wdt->count =3D 0xffff;
-}
-/* return timeout in seconds (calculated from timer count) */
-static int mpc5200_wdt_get_timeout(struct mpc5200_wdt *wdt)
-{
- return wdt->count * 0x10000 / wdt->ipb_freq;
-}
-
-
-/* watchdog operations */
-static int mpc5200_wdt_start(struct mpc5200_wdt *wdt)
-{
- spin_lock(&wdt->io_lock);
- /* disable */
- out_be32(&wdt->regs->mode, 0);
- /* set timeout, with maximum prescaler */
- out_be32(&wdt->regs->count, 0x0 | wdt->count);
- /* enable watchdog */
- out_be32(&wdt->regs->mode, GPT_MODE_CE | GPT_MODE_WDT |
- GPT_MODE_MS_TIMER);
- spin_unlock(&wdt->io_lock);
-
- return 0;
-}
-static int mpc5200_wdt_ping(struct mpc5200_wdt *wdt)
-{
- spin_lock(&wdt->io_lock);
- /* writing A5 to OCPW resets the watchdog */
- out_be32(&wdt->regs->mode, 0xA5000000 |
- (0xffffff & in_be32(&wdt->regs->mode)));
- spin_unlock(&wdt->io_lock);
- return 0;
-}
-static int mpc5200_wdt_stop(struct mpc5200_wdt *wdt)
-{
- spin_lock(&wdt->io_lock);
- /* disable */
- out_be32(&wdt->regs->mode, 0);
- spin_unlock(&wdt->io_lock);
- return 0;
-}
-
-
/* file operations */
static ssize_t mpc5200_wdt_write(struct file *file, const char __user *dat=
a,
- size_t len, loff_t *ppos)
+ size_t len, loff_t *ppos)
{
struct mpc5200_wdt *wdt =3D file->private_data;
- mpc5200_wdt_ping(wdt);
+ mpc52xx_gpt_wdt_ping(wdt->timer);
return 0;
}
+
static struct watchdog_info mpc5200_wdt_info =3D {
.options =3D WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
- .identity =3D "mpc5200 watchdog on GPT0",
+ .identity =3D WDT_IDENTITY,
};
+
static long mpc5200_wdt_ioctl(struct file *file, unsigned int cmd,
- unsigned long arg)
+ unsigned long arg)
{
struct mpc5200_wdt *wdt =3D file->private_data;
int __user *data =3D (int __user *)arg;
int timeout;
+ u64 real_timeout;
int ret =3D 0;
=20
switch (cmd) {
case WDIOC_GETSUPPORT:
ret =3D copy_to_user(data, &mpc5200_wdt_info,
- sizeof(mpc5200_wdt_info));
+ sizeof(mpc5200_wdt_info));
if (ret)
ret =3D -EFAULT;
break;
@@ -116,19 +60,23 @@ static long mpc5200_wdt_ioctl(struct file *file, unsig=
ned int cmd,
break;
=20
case WDIOC_KEEPALIVE:
- mpc5200_wdt_ping(wdt);
+ mpc52xx_gpt_wdt_ping(wdt->timer);
break;
=20
case WDIOC_SETTIMEOUT:
ret =3D get_user(timeout, data);
if (ret)
break;
- mpc5200_wdt_set_timeout(wdt, timeout);
- mpc5200_wdt_start(wdt);
+ ret =3D mpc52xx_gpt_wdt_start(wdt->timer, timeout);
+ if (ret)
+ break;
+ wdt->timeout =3D timeout;
/* fall through and return the timeout */
=20
case WDIOC_GETTIMEOUT:
- timeout =3D mpc5200_wdt_get_timeout(wdt);
+ real_timeout =3D mpc52xx_gpt_timer_period(wdt->timer);
+ do_div(real_timeout, 1000000000);
+ timeout =3D (int) real_timeout;
ret =3D put_user(timeout, data);
break;
=20
@@ -140,154 +88,90 @@ static long mpc5200_wdt_ioctl(struct file *file, unsi=
gned int cmd,
=20
static int mpc5200_wdt_open(struct inode *inode, struct file *file)
{
+ int ret;
+
/* /dev/watchdog can only be opened once */
if (test_and_set_bit(0, &is_active))
return -EBUSY;
=20
/* Set and activate the watchdog */
- mpc5200_wdt_set_timeout(wdt_global, 30);
- mpc5200_wdt_start(wdt_global);
+ ret =3D mpc52xx_gpt_wdt_start(wdt_global->timer, 30);
+ if (ret) {
+ clear_bit(0, &is_active);
+ return ret;
+ }
+ wdt_global->timeout =3D 30;
+
file->private_data =3D wdt_global;
return nonseekable_open(inode, file);
}
+
static int mpc5200_wdt_release(struct inode *inode, struct file *file)
{
-#if WATCHDOG_NOWAYOUT =3D=3D 0
+#if !defined(CONFIG_WATCHDOG_NOWAYOUT)
struct mpc5200_wdt *wdt =3D file->private_data;
- mpc5200_wdt_stop(wdt);
- wdt->count =3D 0; /* =3D=3D disabled */
+ mpc52xx_gpt_wdt_stop(wdt->timer);
+ wdt->timeout =3D 0; /* =3D=3D disabled */
#endif
clear_bit(0, &is_active);
return 0;
}
=20
static const struct file_operations mpc5200_wdt_fops =3D {
- .owner =3D THIS_MODULE,
- .write =3D mpc5200_wdt_write,
+ .owner =3D THIS_MODULE,
+ .llseek =3D no_llseek,
+ .write =3D mpc5200_wdt_write,
.unlocked_ioctl =3D mpc5200_wdt_ioctl,
- .open =3D mpc5200_wdt_open,
- .release =3D mpc5200_wdt_release,
+ .open =3D mpc5200_wdt_open,
+ .release =3D mpc5200_wdt_release,
+};
+
+static struct miscdevice mpc5200_wdt_miscdev =3D {
+ .minor =3D WATCHDOG_MINOR,
+ .name =3D "watchdog",
+ .fops =3D &mpc5200_wdt_fops,
};
=20
/* module operations */
-static int mpc5200_wdt_probe(struct of_device *op,
- const struct of_device_id *match)
+static int __init mpc5200_wdt_init(void)
{
- struct mpc5200_wdt *wdt;
int err;
- const void *has_wdt;
- int size;
+ struct mpc52xx_gpt_priv *timer;
=20
- has_wdt =3D of_get_property(op->node, "has-wdt", NULL);
- if (!has_wdt)
- has_wdt =3D of_get_property(op->node, "fsl,has-wdt", NULL);
- if (!has_wdt)
+ /* grab the watchdog-capable gpt */
+ timer =3D mpc52xx_gpt_wdt_probe();
+ if (!timer) {
+ pr_err(WDT_IDENTITY ": probing failed\n");
return -ENODEV;
+ }
=20
- wdt =3D kzalloc(sizeof(*wdt), GFP_KERNEL);
- if (!wdt)
+ wdt_global =3D kzalloc(sizeof(struct mpc5200_wdt), GFP_KERNEL);
+ if (!wdt_global) {
+ mpc52xx_gpt_wdt_release(timer);
return -ENOMEM;
-
- wdt->ipb_freq =3D mpc5xxx_get_bus_frequency(op->node);
-
- err =3D of_address_to_resource(op->node, 0, &wdt->mem);
- if (err)
- goto out_free;
- size =3D wdt->mem.end - wdt->mem.start + 1;
- if (!request_mem_region(wdt->mem.start, size, "mpc5200_wdt")) {
- err =3D -ENODEV;
- goto out_free;
- }
- wdt->regs =3D ioremap(wdt->mem.start, size);
- if (!wdt->regs) {
- err =3D -ENODEV;
- goto out_release;
}
-
- dev_set_drvdata(&op->dev, wdt);
- spin_lock_init(&wdt->io_lock);
-
- wdt->miscdev =3D (struct miscdevice) {
- .minor =3D WATCHDOG_MINOR,
- .name =3D "watchdog",
- .fops =3D &mpc5200_wdt_fops,
- .parent =3D &op->dev,
- };
- wdt_global =3D wdt;
- err =3D misc_register(&wdt->miscdev);
+ wdt_global->timer =3D timer;
+ err =3D misc_register(&mpc5200_wdt_miscdev);
if (!err)
- return 0;
-
- iounmap(wdt->regs);
-out_release:
- release_mem_region(wdt->mem.start, size);
-out_free:
- kfree(wdt);
+ pr_info(WDT_IDENTITY ": registered\n");
+ else {
+ mpc52xx_gpt_wdt_release(timer);
+ kfree(wdt_global);
+ }
return err;
}
=20
-static int mpc5200_wdt_remove(struct of_device *op)
-{
- struct mpc5200_wdt *wdt =3D dev_get_drvdata(&op->dev);
-
- mpc5200_wdt_stop(wdt);
- misc_deregister(&wdt->miscdev);
- iounmap(wdt->regs);
- release_mem_region(wdt->mem.start, wdt->mem.end - wdt->mem.start + 1);
- kfree(wdt);
-
- return 0;
-}
-static int mpc5200_wdt_suspend(struct of_device *op, pm_message_t state)
-{
- struct mpc5200_wdt *wdt =3D dev_get_drvdata(&op->dev);
- mpc5200_wdt_stop(wdt);
- return 0;
-}
-static int mpc5200_wdt_resume(struct of_device *op)
-{
- struct mpc5200_wdt *wdt =3D dev_get_drvdata(&op->dev);
- if (wdt->count)
- mpc5200_wdt_start(wdt);
- return 0;
-}
-static int mpc5200_wdt_shutdown(struct of_device *op)
-{
- struct mpc5200_wdt *wdt =3D dev_get_drvdata(&op->dev);
- mpc5200_wdt_stop(wdt);
- return 0;
-}
-
-static struct of_device_id mpc5200_wdt_match[] =3D {
- { .compatible =3D "mpc5200-gpt", },
- { .compatible =3D "fsl,mpc5200-gpt", },
- {},
-};
-static struct of_platform_driver mpc5200_wdt_driver =3D {
- .owner =3D THIS_MODULE,
- .name =3D "mpc5200-gpt-wdt",
- .match_table =3D mpc5200_wdt_match,
- .probe =3D mpc5200_wdt_probe,
- .remove =3D mpc5200_wdt_remove,
- .suspend =3D mpc5200_wdt_suspend,
- .resume =3D mpc5200_wdt_resume,
- .shutdown =3D mpc5200_wdt_shutdown,
-};
-
-
-static int __init mpc5200_wdt_init(void)
-{
- return of_register_platform_driver(&mpc5200_wdt_driver);
-}
-
static void __exit mpc5200_wdt_exit(void)
{
- of_unregister_platform_driver(&mpc5200_wdt_driver);
+ mpc52xx_gpt_wdt_release(wdt_global->timer);
+ misc_deregister(&mpc5200_wdt_miscdev);
+ kfree(wdt_global);
}
=20
module_init(mpc5200_wdt_init);
module_exit(mpc5200_wdt_exit);
=20
-MODULE_AUTHOR("Domen Puncer <domen.puncer@telargo.com>");
+MODULE_AUTHOR("Domen Puncer <domen.puncer@telargo.com>, "
+ "Albrecht Dre=DF <albrecht.dress@arcor.de>");
MODULE_LICENSE("Dual BSD/GPL");
MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
^ permalink raw reply related
* [PATCH 2/3] mpc52xx/wdt: merge WDT code into the GPT
From: Albrecht Dreß @ 2009-11-10 19:41 UTC (permalink / raw)
To: Linux PPC Development, Grant Likely, devicetree-discuss,
Wim Van Sebroeck
Merge the WDT code into the GPT interface.
Signed-off-by: Albrecht Dre=DF <albrecht.dress@arcor.de>
---
Notes:
The maximum timeout for a 5200 GPT @ 33 MHz clock is ~130 seconds. As this
exceeds the range of an int, some api's had to be changed to u64.
The WDT api is exported as to keep the WDT driver separated from the GPT
driver.
If GPT0 is used as WDT, this prevents the use of any GPT0 GPT function (i.e=
.
they will fail with -EBUSY). IOW, the safety function always has precedenc=
e
over the GPT function. If the kernel has been compiled with
CONFIG_WATCHDOG_NOWAYOUT, this means that GPT0 is locked in WDT mode until
the next reboot - this may be a requirement in safety applications.
arch/powerpc/include/asm/mpc52xx.h | 18 ++-
arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 281 +++++++++++++++++++++++++=
+---
2 files changed, 270 insertions(+), 29 deletions(-)
diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/asm/=
mpc52xx.h
index 707ab75..0ece07f 100644
--- a/arch/powerpc/include/asm/mpc52xx.h
+++ b/arch/powerpc/include/asm/mpc52xx.h
@@ -279,9 +279,21 @@ extern void mpc52xx_restart(char *cmd);
/* mpc52xx_gpt.c */
struct mpc52xx_gpt_priv;
extern struct mpc52xx_gpt_priv *mpc52xx_gpt_from_irq(int irq);
-extern int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *gpt, int perio=
d,
- int continuous);
-extern void mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt);
+extern int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *gpt, u64 perio=
d,
+ int continuous);
+extern int mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt);
+extern u64 mpc52xx_gpt_timer_period(struct mpc52xx_gpt_priv *gpt);
+#if (defined(CONFIG_MPC5200_WDT)) || (defined(CONFIG_MPC5200_WDT_MODULE))
+extern struct mpc52xx_gpt_priv *mpc52xx_gpt_wdt_probe(void);
+extern int mpc52xx_gpt_wdt_start(struct mpc52xx_gpt_priv *gpt_wdt,
+ int wdt_timeout);
+extern int mpc52xx_gpt_wdt_ping(struct mpc52xx_gpt_priv *gpt_wdt);
+extern int mpc52xx_gpt_wdt_release(struct mpc52xx_gpt_priv *gpt_wdt);
+#if !defined(CONFIG_WATCHDOG_NOWAYOUT)
+extern int mpc52xx_gpt_wdt_stop(struct mpc52xx_gpt_priv *gpt_wdt);
+#endif /* CONFIG_WATCHDOG_NOWAYOUT */
+#endif /* CONFIG_MPC5200_WDT */
+
=20
/* mpc52xx_lpbfifo.c */
#define MPC52XX_LPBFIFO_FLAG_READ (0)
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c b/arch/powerpc/platf=
orms/52xx/mpc52xx_gpt.c
index 2c3fa13..8274ebb 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
@@ -60,9 +60,13 @@
#include <asm/mpc52xx.h>
=20
MODULE_DESCRIPTION("Freescale MPC52xx gpt driver");
-MODULE_AUTHOR("Sascha Hauer, Grant Likely");
+MODULE_AUTHOR("Sascha Hauer, Grant Likely, Albrecht Dre=DF");
MODULE_LICENSE("GPL");
=20
+#if (defined(CONFIG_MPC5200_WDT)) || (defined(CONFIG_MPC5200_WDT_MODULE))
+#define HAVE_MPC5200_WDT
+#endif
+
/**
* struct mpc52xx_gpt - Private data structure for MPC52xx GPT driver
* @dev: pointer to device structure
@@ -70,6 +74,8 @@ MODULE_LICENSE("GPL");
* @lock: spinlock to coordinate between different functions.
* @of_gc: of_gpio_chip instance structure; used when GPIO is enabled
* @irqhost: Pointer to irq_host instance; used when IRQ mode is supported
+ * @wdt_mode: only used for gpt 0: 0 gpt-only timer, 1 can be used as a
+ * wdt, 2 currently used as wdt, cannot be used as gpt
*/
struct mpc52xx_gpt_priv {
struct list_head list; /* List of all GPT devices */
@@ -78,6 +84,9 @@ struct mpc52xx_gpt_priv {
spinlock_t lock;
struct irq_host *irqhost;
u32 ipb_freq;
+#if defined(HAVE_MPC5200_WDT)
+ u8 wdt_mode;
+#endif
=20
#if defined(CONFIG_GPIOLIB)
struct of_gpio_chip of_gc;
@@ -101,14 +110,23 @@ DEFINE_MUTEX(mpc52xx_gpt_list_mutex);
#define MPC52xx_GPT_MODE_CONTINUOUS (0x0400)
#define MPC52xx_GPT_MODE_OPEN_DRAIN (0x0200)
#define MPC52xx_GPT_MODE_IRQ_EN (0x0100)
+#define MPC52xx_GPT_MODE_WDT_EN (0x8000)
=20
#define MPC52xx_GPT_MODE_ICT_MASK (0x030000)
#define MPC52xx_GPT_MODE_ICT_RISING (0x010000)
#define MPC52xx_GPT_MODE_ICT_FALLING (0x020000)
#define MPC52xx_GPT_MODE_ICT_TOGGLE (0x030000)
=20
+#define MPC52xx_GPT_MODE_WDT_PING (0xa5)
+
#define MPC52xx_GPT_STATUS_IRQMASK (0x000f)
=20
+#define NS_PER_SEC 1000000000LL
+
+#define MPC52xx_GPT_CAN_WDT (1 << 0)
+#define MPC52xx_GPT_IS_WDT (1 << 1)
+
+
/* ---------------------------------------------------------------------
* Cascaded interrupt controller hooks
*/
@@ -375,36 +393,22 @@ struct mpc52xx_gpt_priv *mpc52xx_gpt_from_irq(int irq=
)
}
EXPORT_SYMBOL(mpc52xx_gpt_from_irq);
=20
-/**
- * mpc52xx_gpt_start_timer - Set and enable the GPT timer
- * @gpt: Pointer to gpt private data structure
- * @period: period of timer
- * @continuous: set to 1 to make timer continuous free running
- *
- * An interrupt will be generated every time the timer fires
- */
-int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *gpt, int period,
- int continuous)
+/* Calculate the timer counter input register MBAR + 0x6n4 from the
+ * period in ns. The maximum period for 33 MHz IPB clock is ~130s. */
+static int mpc52xx_gpt_calc_counter_input(u64 period, u64 ipb_freq,
+ u32 *reg_val)
{
- u32 clear, set;
u64 clocks;
u32 prescale;
- unsigned long flags;
-
- clear =3D MPC52xx_GPT_MODE_MS_MASK | MPC52xx_GPT_MODE_CONTINUOUS;
- set =3D MPC52xx_GPT_MODE_MS_GPIO | MPC52xx_GPT_MODE_COUNTER_ENABLE;
- if (continuous)
- set |=3D MPC52xx_GPT_MODE_CONTINUOUS;
=20
/* Determine the number of clocks in the requested period. 64 bit
* arithmatic is done here to preserve the precision until the value
- * is scaled back down into the u32 range. Period is in 'ns', bus
- * frequency is in Hz. */
- clocks =3D (u64)period * (u64)gpt->ipb_freq;
- do_div(clocks, 1000000000); /* Scale it down to ns range */
+ * is scaled back down into the u32 range. */
+ clocks =3D period * ipb_freq;
+ do_div(clocks, NS_PER_SEC); /* Scale it down to ns range */
=20
- /* This device cannot handle a clock count greater than 32 bits */
- if (clocks > 0xffffffff)
+ /* the maximum count is 0x10000 pre-scaler * 0xffff count */
+ if (clocks > 0xffff0000)
return -EINVAL;
=20
/* Calculate the prescaler and count values from the clocks value.
@@ -427,9 +431,47 @@ int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *g=
pt, int period,
return -EINVAL;
}
=20
+ *reg_val =3D (prescale & 0xffff) << 16 | clocks;
+ return 0;
+}
+
+/**
+ * mpc52xx_gpt_start_timer - Set and enable the GPT timer
+ * @gpt: Pointer to gpt private data structure
+ * @period: period of timer in ns
+ * @continuous: set to 1 to make timer continuous free running
+ *
+ * An interrupt will be generated every time the timer fires
+ */
+int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *gpt, u64 period,
+ int continuous)
+{
+ u32 clear, set;
+ u32 counter_reg;
+ unsigned long flags;
+
+#if defined(HAVE_MPC5200_WDT)
+ /* reject the operation if the timer is used as watchdog (gpt 0 only) */
+ spin_lock_irqsave(&gpt->lock, flags);
+ if ((gpt->wdt_mode & MPC52xx_GPT_IS_WDT)) {
+ spin_unlock_irqrestore(&gpt->lock, flags);
+ return -EBUSY;
+ }
+ spin_unlock_irqrestore(&gpt->lock, flags);
+#endif
+
+ clear =3D MPC52xx_GPT_MODE_MS_MASK | MPC52xx_GPT_MODE_CONTINUOUS;
+ set =3D MPC52xx_GPT_MODE_MS_GPIO | MPC52xx_GPT_MODE_COUNTER_ENABLE;
+ if (continuous)
+ set |=3D MPC52xx_GPT_MODE_CONTINUOUS;
+
+ if (mpc52xx_gpt_calc_counter_input(period, (u64)gpt->ipb_freq,
+ &counter_reg) !=3D 0)
+ return -EINVAL;
+
/* Set and enable the timer */
spin_lock_irqsave(&gpt->lock, flags);
- out_be32(&gpt->regs->count, prescale << 16 | clocks);
+ out_be32(&gpt->regs->count, counter_reg);
clrsetbits_be32(&gpt->regs->mode, clear, set);
spin_unlock_irqrestore(&gpt->lock, flags);
=20
@@ -437,12 +479,175 @@ int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv =
*gpt, int period,
}
EXPORT_SYMBOL(mpc52xx_gpt_start_timer);
=20
-void mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt)
+/**
+ * mpc52xx_gpt_timer_period - Return the timer period in nanoseconds
+ * Note: reads the timer config directly from the hardware
+ */
+u64 mpc52xx_gpt_timer_period(struct mpc52xx_gpt_priv *gpt)
+{
+ u64 period;
+ u32 prescale;
+ unsigned long flags;
+
+ spin_lock_irqsave(&gpt->lock, flags);
+ period =3D in_be32(&gpt->regs->count);
+ spin_unlock_irqrestore(&gpt->lock, flags);
+
+ prescale =3D period >> 16;
+ period &=3D 0xffff;
+ if (prescale =3D=3D 0)
+ prescale =3D 0x10000;
+ period =3D period * (u64) prescale * NS_PER_SEC;
+ do_div(period, (u64)gpt->ipb_freq);
+ return period;
+}
+EXPORT_SYMBOL(mpc52xx_gpt_timer_period);
+
+int mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt)
{
+ unsigned long flags;
+
+ /* reject the operation if the timer is used as watchdog (gpt 0 only) */
+ spin_lock_irqsave(&gpt->lock, flags);
+#if defined(HAVE_MPC5200_WDT)
+ if ((gpt->wdt_mode & MPC52xx_GPT_IS_WDT)) {
+ spin_unlock_irqrestore(&gpt->lock, flags);
+ return -EBUSY;
+ }
+#endif
+
clrbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_COUNTER_ENABLE);
+ spin_unlock_irqrestore(&gpt->lock, flags);
+ return 0;
}
EXPORT_SYMBOL(mpc52xx_gpt_stop_timer);
=20
+#if defined(HAVE_MPC5200_WDT)
+/**
+ * mpc52xx_gpt_wdt_probe - Find the wdt devide in the gpt list
+ */
+struct mpc52xx_gpt_priv *mpc52xx_gpt_wdt_probe(void)
+{
+ struct list_head *pos;
+ struct mpc52xx_gpt_priv *this_gpt =3D NULL;
+
+ /* find the wdt device in our list */
+ mutex_lock(&mpc52xx_gpt_list_mutex);
+ list_for_each(pos, &mpc52xx_gpt_list) {
+ this_gpt =3D container_of(pos, struct mpc52xx_gpt_priv, list);
+ if ((this_gpt->wdt_mode & MPC52xx_GPT_CAN_WDT)) {
+ mutex_unlock(&mpc52xx_gpt_list_mutex);
+ return this_gpt;
+ }
+ }
+ mutex_unlock(&mpc52xx_gpt_list_mutex);
+ return NULL;
+}
+EXPORT_SYMBOL(mpc52xx_gpt_wdt_probe);
+
+/**
+ * mpc52xx_gpt_wdt_start - Start the passed timer as watchdog
+ * @gpt_wdt: the watchdog gpt
+ * @wdt_timeout: watchdog timeout in seconds
+ *
+ * Note: the function does not protect itself form being called without a
+ * timer or with a timer which cannot function as wdt.
+ */
+int mpc52xx_gpt_wdt_start(struct mpc52xx_gpt_priv *gpt_wdt, int wdt_timeou=
t)
+{
+ u32 clear, set;
+ u32 counter_reg;
+ unsigned long flags;
+
+ /* calculate register settings */
+ if (mpc52xx_gpt_calc_counter_input((u64) wdt_timeout * NS_PER_SEC,
+ (u64)gpt_wdt->ipb_freq,
+ &counter_reg) !=3D 0) {
+ dev_info(gpt_wdt->dev, "bad timeout value %d\n", wdt_timeout);
+ return -EINVAL;
+ }
+
+ clear =3D MPC52xx_GPT_MODE_MS_MASK | MPC52xx_GPT_MODE_CONTINUOUS |
+ MPC52xx_GPT_MODE_IRQ_EN;
+ set =3D MPC52xx_GPT_MODE_MS_GPIO | MPC52xx_GPT_MODE_COUNTER_ENABLE |
+ MPC52xx_GPT_MODE_WDT_EN;
+
+ /* set the time-out and launch as wdt */
+ spin_lock_irqsave(&gpt_wdt->lock, flags);
+ out_be32(&gpt_wdt->regs->count, counter_reg);
+ clrsetbits_be32(&gpt_wdt->regs->mode, clear, set);
+ gpt_wdt->wdt_mode |=3D MPC52xx_GPT_IS_WDT;
+ spin_unlock_irqrestore(&gpt_wdt->lock, flags);
+
+ return 0;
+}
+EXPORT_SYMBOL(mpc52xx_gpt_wdt_start);
+
+/**
+ * mpc52xx_gpt_wdt_ping - Toggle the keep-alive signal of the watchdog
+ * @gpt_wdt: the watchdog gpt
+ *
+ * Note: the function does not protect itself form being called without a
+ * timer or with a timer which cannot function as wdt.
+ */
+int mpc52xx_gpt_wdt_ping(struct mpc52xx_gpt_priv *gpt_wdt)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&gpt_wdt->lock, flags);
+ out_8((u8 *) &gpt_wdt->regs->mode, MPC52xx_GPT_MODE_WDT_PING);
+ spin_unlock_irqrestore(&gpt_wdt->lock, flags);
+
+ return 0;
+}
+EXPORT_SYMBOL(mpc52xx_gpt_wdt_ping);
+
+/**
+ * mpc52xx_gpt_wdt_release - Release the watchdog so it can be used as gpt
+ * @gpt_wdt: the watchdog gpt
+ *
+ * Note: Stops the watchdog if CONFIG_WATCHDOG_NOWAYOUT is not defined.
+ * the function does not protect itself form being called without a
+ * timer or with a timer which cannot function as wdt.
+ */
+int mpc52xx_gpt_wdt_release(struct mpc52xx_gpt_priv *gpt_wdt)
+{
+#if !defined(CONFIG_WATCHDOG_NOWAYOUT)
+ unsigned long flags;
+
+ spin_lock_irqsave(&gpt_wdt->lock, flags);
+ clrbits32(&gpt_wdt->regs->mode,
+ MPC52xx_GPT_MODE_COUNTER_ENABLE | MPC52xx_GPT_MODE_WDT_EN);
+ gpt_wdt->wdt_mode &=3D ~MPC52xx_GPT_IS_WDT;
+ spin_unlock_irqrestore(&gpt_wdt->lock, flags);
+#endif
+ return 0;
+}
+EXPORT_SYMBOL(mpc52xx_gpt_wdt_release);
+
+#if !defined(CONFIG_WATCHDOG_NOWAYOUT)
+/**
+ * mpc52xx_gpt_wdt_stop - Stop the watchdog
+ * @gpt_wdt: the watchdog gpt
+ *
+ * Note: the function does not protect itself form being called without a
+ * timer or with a timer which cannot function as wdt.
+ */
+int mpc52xx_gpt_wdt_stop(struct mpc52xx_gpt_priv *gpt_wdt)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&gpt_wdt->lock, flags);
+ clrbits32(&gpt_wdt->regs->mode,
+ MPC52xx_GPT_MODE_COUNTER_ENABLE | MPC52xx_GPT_MODE_WDT_EN);
+ gpt_wdt->wdt_mode &=3D ~MPC52xx_GPT_IS_WDT;
+ spin_unlock_irqrestore(&gpt_wdt->lock, flags);
+ return 0;
+}
+EXPORT_SYMBOL(mpc52xx_gpt_wdt_stop);
+#endif /* CONFIG_WATCHDOG_NOWAYOUT */
+#endif /* HAVE_MPC5200_WDT */
+
/* ---------------------------------------------------------------------
* of_platform bus binding code
*/
@@ -473,6 +678,30 @@ static int __devinit mpc52xx_gpt_probe(struct of_devic=
e *ofdev,
list_add(&gpt->list, &mpc52xx_gpt_list);
mutex_unlock(&mpc52xx_gpt_list_mutex);
=20
+#if defined(HAVE_MPC5200_WDT)
+ /* check if this device could be a watchdog */
+ if (of_get_property(ofdev->node, "fsl,has-wdt", NULL) ||
+ of_get_property(ofdev->node, "has-wdt", NULL)) {
+ const u32 *on_boot_wdt;
+
+ gpt->wdt_mode =3D MPC52xx_GPT_CAN_WDT;
+
+ /* check if the device shall be used as on-boot watchdog */
+ on_boot_wdt =3D of_get_property(ofdev->node, "wdt,on-boot", NULL);
+ if (on_boot_wdt) {
+ gpt->wdt_mode |=3D MPC52xx_GPT_IS_WDT;
+ if (*on_boot_wdt > 0 &&
+ mpc52xx_gpt_wdt_start(gpt, *on_boot_wdt) =3D=3D 0)
+ dev_info(gpt->dev,
+ "running as wdt, timeout %us\n",
+ *on_boot_wdt);
+ else
+ dev_info(gpt->dev, "reserved as wdt\n");
+ } else
+ dev_info(gpt->dev, "can function as wdt\n");
+ }
+#endif
+
return 0;
}
=20
^ permalink raw reply related
* [PATCH 1/3] mpc52xx/wdt: OF property to enable the WDT on boot
From: Albrecht Dreß @ 2009-11-10 19:40 UTC (permalink / raw)
To: Linux PPC Development, Grant Likely, devicetree-discuss,
Wim Van Sebroeck
Add the "wdt,on-boot" OF property as to reserve a GPT as WDT which may be a
requirement in safety-related (e.g. ISO 61508) applications.
Signed-off-by: Albrecht Dre=DF <albrecht.dress@arcor.de>
---
Documentation/powerpc/dts-bindings/fsl/mpc5200.txt | 15 ++++++++++++++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt b/Documenta=
tion/powerpc/dts-bindings/fsl/mpc5200.txt
index 8447fd7..1eecb06 100644
--- a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
+++ b/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
@@ -103,7 +103,20 @@ fsl,mpc5200-gpt nodes
---------------------
On the mpc5200 and 5200b, GPT0 has a watchdog timer function. If the boar=
d
design supports the internal wdt, then the device node for GPT0 should
-include the empty property 'fsl,has-wdt'.
+include the empty property 'fsl,has-wdt'. Note that this does not activat=
e
+the watchdog. The timer will function as a GPT if the timer api is used, =
and
+it will function as watchdog if the watchdog device is used. The watchdog
+mode has priority over the gpt mode, i.e. if the watchdog is activated, an=
y
+gpt api call to this timer will fail with -EBUSY.
+
+If you add the property
+ wdt,on-boot =3D <n>;
+GPT0 will be marked as in-use watchdog, i.e. blocking every gpt access to =
it.
+If n>0, the watchdog is started with a timeout of n seconds. If n=3D0, th=
e
+configuration of the watchdog is not touched. This is useful in two cases=
:
+- just mark GPT0 as watchdog, blocking gpt accesses, and configure it late=
r;
+- do not touch a configuration assigned by the boot loader which supervise=
s
+ the boot process itself.
=20
An mpc5200-gpt can be used as a single line GPIO controller. To do so,
add the following properties to the gpt node:
^ permalink raw reply related
* Re: [PATCH 1/3] mpc52xx/wdt: OF property to enable the WDT on boot
From: Grant Likely @ 2009-11-10 21:02 UTC (permalink / raw)
To: Albrecht Dreß
Cc: Linux PPC Development, devicetree-discuss, Wim Van Sebroeck
In-Reply-To: <1257882057.14374.1@antares>
On Tue, Nov 10, 2009 at 12:40 PM, Albrecht Dre=DF <albrecht.dress@arcor.de>=
wrote:
> Add the "wdt,on-boot" OF property as to reserve a GPT as WDT which may be=
a
> requirement in safety-related (e.g. ISO 61508) applications.
>
> Signed-off-by: Albrecht Dre=DF <albrecht.dress@arcor.de>
> ---
>
> =A0Documentation/powerpc/dts-bindings/fsl/mpc5200.txt | =A0 15 ++++++++++=
++++-
> =A01 files changed, 14 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt b/Documen=
tation/powerpc/dts-bindings/fsl/mpc5200.txt
> index 8447fd7..1eecb06 100644
> --- a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
> +++ b/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
> @@ -103,7 +103,20 @@ fsl,mpc5200-gpt nodes
> =A0---------------------
> =A0On the mpc5200 and 5200b, GPT0 has a watchdog timer function. =A0If th=
e board
> =A0design supports the internal wdt, then the device node for GPT0 should
> -include the empty property 'fsl,has-wdt'.
> +include the empty property 'fsl,has-wdt'. =A0Note that this does not act=
ivate
> +the watchdog. =A0The timer will function as a GPT if the timer api is us=
ed, and
> +it will function as watchdog if the watchdog device is used. =A0The watc=
hdog
> +mode has priority over the gpt mode, i.e. if the watchdog is activated, =
any
> +gpt api call to this timer will fail with -EBUSY.
> +
> +If you add the property
> + =A0 =A0 =A0 wdt,on-boot =3D <n>;
> +GPT0 will be marked as in-use watchdog, i.e. blocking every gpt access t=
o it.
> +If n>0, the watchdog is started with a timeout of n seconds. =A0If n=3D0=
, the
> +configuration of the watchdog is not touched. =A0This is useful in two c=
ases:
> +- just mark GPT0 as watchdog, blocking gpt accesses, and configure it la=
ter;
> +- do not touch a configuration assigned by the boot loader which supervi=
ses
> + =A0the boot process itself.
I'm not *totally* convinced on the usage model, but I just need some
time to think about it. Give me a day or so and ping me again if you
haven't heard from me. However, until a common WDT binding is
defined, this property needs to be named something like
"fsl,wdt-on-boot".
Cheers,
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 3/3] mpc52xx/wdt: WDT uses GPT api
From: Grant Likely @ 2009-11-10 21:07 UTC (permalink / raw)
To: Albrecht Dreß
Cc: Linux PPC Development, devicetree-discuss, Wim Van Sebroeck
In-Reply-To: <1257884778.14374.5@antares>
On Tue, Nov 10, 2009 at 1:26 PM, Albrecht Dre=DF <albrecht.dress@arcor.de> =
wrote:
> Hi Grant:
>
> Am 10.11.09 20:59 schrieb(en) Grant Likely:
>>
>> On Tue, Nov 10, 2009 at 12:43 PM, Albrecht Dre=DF <albrecht.dress@arcor.=
de>
>> wrote:
>> > Use the MPC5200 GPT api for the WDT which drastically simplifies this
>> > file.
>> >
>> > Signed-off-by: Albrecht Dre=DF <albrecht.dress@arcor.de>
>> > ---
>> >
>> > =A0drivers/watchdog/mpc5200_wdt.c | =A0246
>> > +++++++++++-----------------------------
>> > =A01 files changed, 65 insertions(+), 181 deletions(-)
>>
>>
>> Can the WDT functionality just be merged entirely into
>> arch/powerpc/platforms/52xx/mpc52xx_gpt.c, eliminating the need for
>> this file entirely? =A0I think I'd rather have all the GPT "built in"
>> behaviour handled by a single driver.
>
> I also thought about it, as it has IMHO the cleaner code, and it would ha=
ve
> the extra benefit that the gpt-wdt api doesn't need to be public.
>
> However, the reasons I hesitated to do so are:
> - I don't want to remove a file someone else wrote (even it doesn't work)=
;
Shouldn't be a problem, and I'll handle the fallout if it is.
> - WDT code is shifted from drivers/watchdog to arch/powerpc/platforms/52x=
x
> which might not be the "logical" place from the directory layout's pov;
There is precedence of this in the past, particularly on arch or
platform specific hardware drivers and multifunction devices. (Heck,
that's almost entirely what arch/powerpc/sysdev is). I'm not
concerned.
> - a file living in arch/powerpc/platforms/52xx depends upon config option=
s
> set from drivers/watchdog/Kconfig which may be confusing.
I'm not concerned about this either.
> You see these are more political/cosmetical questions, so I would prefer =
to
> leave the decision to the maintainers (i.e. you and Wim). =A0Preparing a =
fully
> merged driver is actually a matter of minutes!
Do it. I'll champion getting it in. Wim, do you have any issues with this=
?
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
From: Joakim Tjernlund @ 2009-11-10 21:25 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <4AF9CC99.1030500@freescale.com>
Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 21:27:05:
>
> Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 17:55:28:
> >> Except that the invalidation only happens when you take an ITLB miss on
> >> an adjacent page, which means we'd likely never get CPU15 protection for
> >> kernel code if pinning is enabled. :-(
> >
> > So tlbie invalidates pinned TLBs too?
>
> Yes.
OK, and this is in no way unique for 8xx?
>
> > It is likely that you won't get ITLB Misses for normal kernel
> > space but for modules you will. Also, since the pinned D&I TLBs overlap,
> > how do you make sure that invalidating a kernel DTLB won't
> > spill over to the pinned ITLB?
>
> I don't see when you'd ever invalidate the pinned entry, whether for
> instruction or data purposes, unless you take an ITLB miss for the page
> immediately following the pinned mapping.
tlbie is used by the kernel in other places too, so I assumed it could
be on kernel space too. However, it was just a guess, and by the looks of things,
a bad one.
>
> > Does pinned TLBs work for you?
>
> Yes -- I turned on CONFIG_PIN_TLB, padded things so that the rfi is
> still on the beginning of a new page, and it boots fine. If I keep
> everything the same but replace MD_RSV4I with zero, it fails again.
Ah, good.
>
> But who knows when CPU15 will strike...
yes, maybe there is a way around that. Perhaps by using one of the
pinned entries for loaded modules, i.e avoid ITLB misses for kernel space?
In any case one should consider fixing
entry_32.S and friends to be properly aligned. Then we are no worse off
than when we started, right?
Jocke
^ permalink raw reply
* Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
From: Scott Wood @ 2009-11-10 22:02 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <OF15D21C1C.B7A920F0-ONC125766A.0076C36E-C125766A.0077B321@transmode.se>
Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 22:36:32:
>> Joakim Tjernlund wrote:
>>> yes, maybe there is a way around that. Perhaps by using one of the
>>> pinned entries for loaded modules, i.e avoid ITLB misses for kernel space?
>> Not sure what you mean... loaded modules won't be pinned, and since
>> they shouldn't contain rfi, don't need to be.
>
> But CPU15 may invalidate a pinned TLB if you take a TLB Miss?
> If not there should not be a problem, because the rest
> of the kernel will never take a ITLB Miss.
It wasn't the CPU15 workaround that I was worried about taking down the
pinning -- but rather the CPU15 bug itself causing bad code to be
executed inside the pinned kernel mapping.
However, the erratum says "MMU page", not "4K region", so I suppose if
we have a pinned 8M page the problem could only occur at the end of the
8M (by which point the text segment should have ended).
Unless we have any evidence that this is not what the erratum means, I'd
say make pinning mandatory, and avoid placing modules immediately after
a pinned entry.
> BTW, you could probably cram the DARFix into the DTLBerror with some luck.
> Especially if you allow it to spill over to the next trap. Then create a
> branch insn at 0x1500 to 0x1600. Would that make everything aligned again?
Yes, until some other code change breaks it again.
-Scott
^ permalink raw reply
* Re: [PATCH 2/3] mpc52xx/wdt: merge WDT code into the GPT
From: Grant Likely @ 2009-11-10 20:59 UTC (permalink / raw)
To: Albrecht Dreß
Cc: Linux PPC Development, devicetree-discuss, Wim Van Sebroeck
In-Reply-To: <1257882118.14374.2@antares>
On Tue, Nov 10, 2009 at 12:41 PM, Albrecht Dre=DF <albrecht.dress@arcor.de>=
wrote:
> Merge the WDT code into the GPT interface.
>
> Signed-off-by: Albrecht Dre=DF <albrecht.dress@arcor.de>
> ---
Hi Albrecht,
Thanks for this work. Comments below.
>
> Notes:
>
> The maximum timeout for a 5200 GPT @ 33 MHz clock is ~130 seconds. =A0As =
this
> exceeds the range of an int, some api's had to be changed to u64.
>
> The WDT api is exported as to keep the WDT driver separated from the GPT
> driver.
>
> If GPT0 is used as WDT, this prevents the use of any GPT0 GPT function (i=
.e.
> they will fail with -EBUSY). =A0IOW, the safety function always has prece=
dence
> over the GPT function. =A0If the kernel has been compiled with
> CONFIG_WATCHDOG_NOWAYOUT, this means that GPT0 is locked in WDT mode unti=
l
> the next reboot - this may be a requirement in safety applications.
This description would look great in the header comment block of the
.c file.
Also, as I commented in patch 3; I'd rather see all the WDT
functionality rolled into this driver. As long as you keep the
functional code blocks logically arranged, I think the driver will be
easier to maintain and understand that way.
> =A0arch/powerpc/include/asm/mpc52xx.h =A0 =A0 =A0 =A0| =A0 18 ++-
> =A0arch/powerpc/platforms/52xx/mpc52xx_gpt.c | =A0281 +++++++++++++++++++=
+++++++---
> =A02 files changed, 270 insertions(+), 29 deletions(-)
>
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c b/arch/powerpc/pla=
tforms/52xx/mpc52xx_gpt.c
> index 2c3fa13..8274ebb 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> @@ -60,9 +60,13 @@
> =A0#include <asm/mpc52xx.h>
>
> =A0MODULE_DESCRIPTION("Freescale MPC52xx gpt driver");
> -MODULE_AUTHOR("Sascha Hauer, Grant Likely");
> +MODULE_AUTHOR("Sascha Hauer, Grant Likely, Albrecht Dre=DF");
> =A0MODULE_LICENSE("GPL");
>
> +#if (defined(CONFIG_MPC5200_WDT)) || (defined(CONFIG_MPC5200_WDT_MODULE)=
)
> +#define HAVE_MPC5200_WDT
> +#endif
> +
> =A0/**
> =A0* struct mpc52xx_gpt - Private data structure for MPC52xx GPT driver
> =A0* @dev: pointer to device structure
> @@ -70,6 +74,8 @@ MODULE_LICENSE("GPL");
> =A0* @lock: spinlock to coordinate between different functions.
> =A0* @of_gc: of_gpio_chip instance structure; used when GPIO is enabled
> =A0* @irqhost: Pointer to irq_host instance; used when IRQ mode is suppor=
ted
> + * @wdt_mode: only used for gpt 0: 0 gpt-only timer, 1 can be used as a
> + * =A0 =A0 =A0 =A0 =A0 =A0wdt, 2 currently used as wdt, cannot be used a=
s gpt
> =A0*/
> =A0struct mpc52xx_gpt_priv {
> =A0 =A0 =A0 =A0struct list_head list; =A0 =A0 =A0 =A0 =A0/* List of all G=
PT devices */
> @@ -78,6 +84,9 @@ struct mpc52xx_gpt_priv {
> =A0 =A0 =A0 =A0spinlock_t lock;
> =A0 =A0 =A0 =A0struct irq_host *irqhost;
> =A0 =A0 =A0 =A0u32 ipb_freq;
> +#if defined(HAVE_MPC5200_WDT)
> + =A0 =A0 =A0 u8 wdt_mode;
> +#endif
I wouldn't bother with the #if/#endif. I'm willing to sacrifice
32bits for the sake of readability of the code.
> +#define NS_PER_SEC =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 1000000000LL
> +
> +#define MPC52xx_GPT_CAN_WDT =A0 =A0 =A0 =A0 =A0 =A0(1 << 0)
> +#define MPC52xx_GPT_IS_WDT =A0 =A0 =A0 =A0 =A0 =A0 (1 << 1)
> +
> +
> =A0/* -------------------------------------------------------------------=
--
> =A0* Cascaded interrupt controller hooks
> =A0*/
> @@ -375,36 +393,22 @@ struct mpc52xx_gpt_priv *mpc52xx_gpt_from_irq(int i=
rq)
> =A0}
> =A0EXPORT_SYMBOL(mpc52xx_gpt_from_irq);
>
> -/**
> - * mpc52xx_gpt_start_timer - Set and enable the GPT timer
> - * @gpt: Pointer to gpt private data structure
> - * @period: period of timer
> - * @continuous: set to 1 to make timer continuous free running
> - *
> - * An interrupt will be generated every time the timer fires
> - */
> -int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *gpt, int period,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int continuous)
> +/* Calculate the timer counter input register MBAR + 0x6n4 from the
> + * period in ns. =A0The maximum period for 33 MHz IPB clock is ~130s. */
> +static int mpc52xx_gpt_calc_counter_input(u64 period, u64 ipb_freq,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0 u32 *reg_val)
> =A0{
> - =A0 =A0 =A0 u32 clear, set;
> =A0 =A0 =A0 =A0u64 clocks;
> =A0 =A0 =A0 =A0u32 prescale;
> - =A0 =A0 =A0 unsigned long flags;
> -
> - =A0 =A0 =A0 clear =3D MPC52xx_GPT_MODE_MS_MASK | MPC52xx_GPT_MODE_CONTI=
NUOUS;
> - =A0 =A0 =A0 set =3D MPC52xx_GPT_MODE_MS_GPIO | MPC52xx_GPT_MODE_COUNTER=
_ENABLE;
> - =A0 =A0 =A0 if (continuous)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 set |=3D MPC52xx_GPT_MODE_CONTINUOUS;
>
> =A0 =A0 =A0 =A0/* Determine the number of clocks in the requested period.=
=A064 bit
> =A0 =A0 =A0 =A0 * arithmatic is done here to preserve the precision until=
the value
> - =A0 =A0 =A0 =A0* is scaled back down into the u32 range. =A0Period is i=
n 'ns', bus
> - =A0 =A0 =A0 =A0* frequency is in Hz. */
> - =A0 =A0 =A0 clocks =3D (u64)period * (u64)gpt->ipb_freq;
> - =A0 =A0 =A0 do_div(clocks, 1000000000); /* Scale it down to ns range */
> + =A0 =A0 =A0 =A0* is scaled back down into the u32 range. */
> + =A0 =A0 =A0 clocks =3D period * ipb_freq;
> + =A0 =A0 =A0 do_div(clocks, NS_PER_SEC); =A0 =A0 =A0 =A0 /* Scale it dow=
n to ns range */
Nit: I wouldn't bother with the NS_PER_SEC macro personally. ns per s
is such a fundamental property that I'd rather see the actual number.
>
> - =A0 =A0 =A0 /* This device cannot handle a clock count greater than 32 =
bits */
> - =A0 =A0 =A0 if (clocks > 0xffffffff)
> + =A0 =A0 =A0 /* the maximum count is 0x10000 pre-scaler * 0xffff count *=
/
> + =A0 =A0 =A0 if (clocks > 0xffff0000)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -EINVAL;
This a bug fix? Separate patch please.
> =A0 =A0 =A0 =A0/* Calculate the prescaler and count values from the clock=
s value.
> @@ -427,9 +431,47 @@ int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv =
*gpt, int period,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -EINVAL;
> =A0 =A0 =A0 =A0}
>
> + =A0 =A0 =A0 *reg_val =3D (prescale & 0xffff) << 16 | clocks;
> + =A0 =A0 =A0 return 0;
> +}
> +
> +/**
> + * mpc52xx_gpt_start_timer - Set and enable the GPT timer
> + * @gpt: Pointer to gpt private data structure
> + * @period: period of timer in ns
> + * @continuous: set to 1 to make timer continuous free running
> + *
> + * An interrupt will be generated every time the timer fires
> + */
> +int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *gpt, u64 period,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int continuous)
> +{
> + =A0 =A0 =A0 u32 clear, set;
> + =A0 =A0 =A0 u32 counter_reg;
> + =A0 =A0 =A0 unsigned long flags;
> +
> +#if defined(HAVE_MPC5200_WDT)
> + =A0 =A0 =A0 /* reject the operation if the timer is used as watchdog (g=
pt 0 only) */
> + =A0 =A0 =A0 spin_lock_irqsave(&gpt->lock, flags);
> + =A0 =A0 =A0 if ((gpt->wdt_mode & MPC52xx_GPT_IS_WDT)) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&gpt->lock, flags);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EBUSY;
> + =A0 =A0 =A0 }
> + =A0 =A0 =A0 spin_unlock_irqrestore(&gpt->lock, flags);
> +#endif
I think the behaviour needs to be consistent, regardless of
HAVE_MPC5200_WDT state. If the IS_WDT flag is set, then this needs
to bail, even if WDT support is not enabled.
Also, move this code block down into the spin lock/unlock block lower
in the function. it doesn't make much sense to grab the spin lock
twice in the same function, and the worst think that can happen if
this test fails is that a few extra cycles are burned calculating what
the counter_reg value should be.
> +
> + =A0 =A0 =A0 clear =3D MPC52xx_GPT_MODE_MS_MASK | MPC52xx_GPT_MODE_CONTI=
NUOUS;
> + =A0 =A0 =A0 set =3D MPC52xx_GPT_MODE_MS_GPIO | MPC52xx_GPT_MODE_COUNTER=
_ENABLE;
> + =A0 =A0 =A0 if (continuous)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 set |=3D MPC52xx_GPT_MODE_CONTINUOUS;
> +
> + =A0 =A0 =A0 if (mpc52xx_gpt_calc_counter_input(period, (u64)gpt->ipb_fr=
eq,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0 =A0&counter_reg) !=3D 0)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EINVAL;
> +
> =A0 =A0 =A0 =A0/* Set and enable the timer */
> =A0 =A0 =A0 =A0spin_lock_irqsave(&gpt->lock, flags);
> - =A0 =A0 =A0 out_be32(&gpt->regs->count, prescale << 16 | clocks);
> + =A0 =A0 =A0 out_be32(&gpt->regs->count, counter_reg);
> =A0 =A0 =A0 =A0clrsetbits_be32(&gpt->regs->mode, clear, set);
> =A0 =A0 =A0 =A0spin_unlock_irqrestore(&gpt->lock, flags);
>
> @@ -437,12 +479,175 @@ int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_pri=
v *gpt, int period,
> =A0}
> =A0EXPORT_SYMBOL(mpc52xx_gpt_start_timer);
>
> -void mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt)
> +/**
> + * mpc52xx_gpt_timer_period - Return the timer period in nanoseconds
> + * Note: reads the timer config directly from the hardware
> + */
> +u64 mpc52xx_gpt_timer_period(struct mpc52xx_gpt_priv *gpt)
> +{
> + =A0 =A0 =A0 u64 period;
> + =A0 =A0 =A0 u32 prescale;
> + =A0 =A0 =A0 unsigned long flags;
> +
> + =A0 =A0 =A0 spin_lock_irqsave(&gpt->lock, flags);
> + =A0 =A0 =A0 period =3D in_be32(&gpt->regs->count);
> + =A0 =A0 =A0 spin_unlock_irqrestore(&gpt->lock, flags);
> +
> + =A0 =A0 =A0 prescale =3D period >> 16;
> + =A0 =A0 =A0 period &=3D 0xffff;
> + =A0 =A0 =A0 if (prescale =3D=3D 0)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 prescale =3D 0x10000;
> + =A0 =A0 =A0 period =3D period * (u64) prescale * NS_PER_SEC;
> + =A0 =A0 =A0 do_div(period, (u64)gpt->ipb_freq);
Are the casts necessary? Maybe prescale should just be a u64 value?
> + =A0 =A0 =A0 return period;
> +}
> +EXPORT_SYMBOL(mpc52xx_gpt_timer_period);
> +
> +int mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt)
> =A0{
> + =A0 =A0 =A0 unsigned long flags;
> +
> + =A0 =A0 =A0 /* reject the operation if the timer is used as watchdog (g=
pt 0 only) */
> + =A0 =A0 =A0 spin_lock_irqsave(&gpt->lock, flags);
> +#if defined(HAVE_MPC5200_WDT)
> + =A0 =A0 =A0 if ((gpt->wdt_mode & MPC52xx_GPT_IS_WDT)) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&gpt->lock, flags);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EBUSY;
> + =A0 =A0 =A0 }
> +#endif
Again, drop the #if/endif wrapper.
> +/**
> + * mpc52xx_gpt_wdt_release - Release the watchdog so it can be used as g=
pt
> + * @gpt_wdt: the watchdog gpt
> + *
> + * Note: Stops the watchdog if CONFIG_WATCHDOG_NOWAYOUT is not defined.
> + * the function does not protect itself form being called without a
> + * timer or with a timer which cannot function as wdt.
> + */
> +int mpc52xx_gpt_wdt_release(struct mpc52xx_gpt_priv *gpt_wdt)
> +{
> +#if !defined(CONFIG_WATCHDOG_NOWAYOUT)
> + =A0 =A0 =A0 unsigned long flags;
> +
> + =A0 =A0 =A0 spin_lock_irqsave(&gpt_wdt->lock, flags);
> + =A0 =A0 =A0 clrbits32(&gpt_wdt->regs->mode,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 MPC52xx_GPT_MODE_COUNTER_ENABLE | MPC52=
xx_GPT_MODE_WDT_EN);
> + =A0 =A0 =A0 gpt_wdt->wdt_mode &=3D ~MPC52xx_GPT_IS_WDT;
> + =A0 =A0 =A0 spin_unlock_irqrestore(&gpt_wdt->lock, flags);
> +#endif
> + =A0 =A0 =A0 return 0;
> +}
> +EXPORT_SYMBOL(mpc52xx_gpt_wdt_release);
> +
> +#if !defined(CONFIG_WATCHDOG_NOWAYOUT)
> +/**
> + * mpc52xx_gpt_wdt_stop - Stop the watchdog
> + * @gpt_wdt: the watchdog gpt
> + *
> + * Note: the function does not protect itself form being called without =
a
> + * timer or with a timer which cannot function as wdt.
> + */
> +int mpc52xx_gpt_wdt_stop(struct mpc52xx_gpt_priv *gpt_wdt)
> +{
> + =A0 =A0 =A0 unsigned long flags;
> +
> + =A0 =A0 =A0 spin_lock_irqsave(&gpt_wdt->lock, flags);
> + =A0 =A0 =A0 clrbits32(&gpt_wdt->regs->mode,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 MPC52xx_GPT_MODE_COUNTER_ENABLE | MPC52=
xx_GPT_MODE_WDT_EN);
> + =A0 =A0 =A0 gpt_wdt->wdt_mode &=3D ~MPC52xx_GPT_IS_WDT;
> + =A0 =A0 =A0 spin_unlock_irqrestore(&gpt_wdt->lock, flags);
> + =A0 =A0 =A0 return 0;
> +}
> +EXPORT_SYMBOL(mpc52xx_gpt_wdt_stop);
> +#endif =A0/* =A0CONFIG_WATCHDOG_NOWAYOUT =A0*/
> +#endif =A0/* =A0HAVE_MPC5200_WDT =A0*/
> +
> =A0/* -------------------------------------------------------------------=
--
> =A0* of_platform bus binding code
> =A0*/
> @@ -473,6 +678,30 @@ static int __devinit mpc52xx_gpt_probe(struct of_dev=
ice *ofdev,
> =A0 =A0 =A0 =A0list_add(&gpt->list, &mpc52xx_gpt_list);
> =A0 =A0 =A0 =A0mutex_unlock(&mpc52xx_gpt_list_mutex);
>
> +#if defined(HAVE_MPC5200_WDT)
> + =A0 =A0 =A0 /* check if this device could be a watchdog */
> + =A0 =A0 =A0 if (of_get_property(ofdev->node, "fsl,has-wdt", NULL) ||
> + =A0 =A0 =A0 =A0 =A0 of_get_property(ofdev->node, "has-wdt", NULL)) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 const u32 *on_boot_wdt;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 gpt->wdt_mode =3D MPC52xx_GPT_CAN_WDT;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* check if the device shall be used as on-=
boot watchdog */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 on_boot_wdt =3D of_get_property(ofdev->node=
, "wdt,on-boot", NULL);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (on_boot_wdt) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 gpt->wdt_mode |=3D MPC52xx_=
GPT_IS_WDT;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (*on_boot_wdt > 0 &&
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc52xx_gpt_wdt_sta=
rt(gpt, *on_boot_wdt) =3D=3D 0)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_info(gp=
t->dev,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0"running as wdt, timeout %us\n",
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0*on_boot_wdt);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_info(gp=
t->dev, "reserved as wdt\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_info(gpt->dev, "can fun=
ction as wdt\n");
> + =A0 =A0 =A0 }
> +#endif
Ditto on the #if/endif
Cheers,
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
From: Joakim Tjernlund @ 2009-11-10 21:47 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <4AF9DCE0.4030805@freescale.com>
Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 22:36:32:
>
> Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 21:27:05:
> >> Joakim Tjernlund wrote:
> >>> Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 17:55:28:
> >>>> Except that the invalidation only happens when you take an ITLB miss on
> >>>> an adjacent page, which means we'd likely never get CPU15 protection for
> >>>> kernel code if pinning is enabled. :-(
> >>> So tlbie invalidates pinned TLBs too?
> >> Yes.
> >
> > OK, and this is in no way unique for 8xx?
>
> Not sure about others, but 8xx manual explicitly says that it
> invalidates reserved entries.
>
> >> But who knows when CPU15 will strike...
> > yes, maybe there is a way around that. Perhaps by using one of the
> > pinned entries for loaded modules, i.e avoid ITLB misses for kernel space?
>
> Not sure what you mean... loaded modules won't be pinned, and since
> they shouldn't contain rfi, don't need to be.
But CPU15 may invalidate a pinned TLB if you take a TLB Miss?
If not there should not be a problem, because the rest
of the kernel will never take a ITLB Miss.
>
> I don't see how to pin any part of the kernel without introducing some
> possibility of CPU15, unless we go scanning the last word of every
> instruction page, creating trampolines, and hoping there's no data
> embedded in the text segment. :-P
Yes, it is not going to be easy.
So aligning the srr0/srr1/rfi properly is needed.
BTW, you could probably cram the DARFix into the DTLBerror with some luck.
Especially if you allow it to spill over to the next trap. Then create a
branch insn at 0x1500 to 0x1600. Would that make everything aligned again?
Jocke
^ 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