* RE: [PATCH 2/4] xilinx_spi: Switch to iomem functions and support little endian.
From: John Linn @ 2009-11-11 22:19 UTC (permalink / raw)
To: Richard Röjfors, spi-devel-general
Cc: linuxppc-dev, Andrew Morton, dbrownell
In-Reply-To: <4AFACC6A.304@mocean-labs.com>
> -----Original Message-----
> From: Richard R=F6jfors [mailto:richard.rojfors@mocean-labs.com]
> Sent: Wednesday, November 11, 2009 7:39 AM
> To: spi-devel-general@lists.sourceforge.net
> Cc: linuxppc-dev@ozlabs.org; Andrew Morton; dbrownell@users.sourceforge.n=
et; John Linn;
> grant.likely@secretlab.ca
> Subject: [PATCH 2/4] xilinx_spi: Switch to iomem functions and support li=
ttle endian.
> =
> This patch changes the out_(be)(8|16|32) and in_(be)(8|16|32) calls to io=
write(8|16|32)
> and ioread(8|16|32). This to be able to build on platforms not supporting=
the in/out calls
> for instance x86.
> =
> Support is also added for little endian writes. In some systems the regis=
ters should be
> accessed little endian rather than big endian.
> =
> Signed-off-by: Richard R=F6jfors <richard.rojfors@mocean-labs.com>
> ---
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index e60b264..9667650 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -236,7 +236,7 @@ config SPI_TXX9
> =
> config SPI_XILINX
> tristate "Xilinx SPI controller"
> - depends on EXPERIMENTAL
> + depends on HAS_IOMEM && EXPERIMENTAL
> select SPI_BITBANG
> help
> This exposes the SPI controller IP from the Xilinx EDK.
> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
> index 1562e9b..b00dabc 100644
> --- a/drivers/spi/xilinx_spi.c
> +++ b/drivers/spi/xilinx_spi.c
> @@ -19,12 +19,12 @@
> #include <linux/spi/spi_bitbang.h>
> #include <linux/io.h>
> =
> -#define XILINX_SPI_NAME "xilinx_spi"
> +#include "xilinx_spi.h"
> =
> /* Register definitions as per "OPB Serial Peripheral Interface (SPI) (v=
1.00e)
> * Product Specification", DS464
> */
> -#define XSPI_CR_OFFSET 0x62 /* 16-bit Control Register */
> +#define XSPI_CR_OFFSET 0x60 /* 16-bit Control Register */
> =
> #define XSPI_CR_ENABLE 0x02
> #define XSPI_CR_MASTER_MODE 0x04
> @@ -36,7 +36,7 @@
> #define XSPI_CR_MANUAL_SSELECT 0x80
> #define XSPI_CR_TRANS_INHIBIT 0x100
> =
> -#define XSPI_SR_OFFSET 0x67 /* 8-bit Status Register */
> +#define XSPI_SR_OFFSET 0x64 /* 8-bit Status Register */
> =
> #define XSPI_SR_RX_EMPTY_MASK 0x01 /* Receive FIFO is empty */
> #define XSPI_SR_RX_FULL_MASK 0x02 /* Receive FIFO is full */
> @@ -44,8 +44,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 /* 8-bit Data Transmit Register */
> +#define XSPI_RXD_OFFSET 0x6c /* 8-bit Data Receive Register */
> =
> #define XSPI_SSR_OFFSET 0x70 /* 32-bit Slave Select Register */
> =
> @@ -83,23 +83,69 @@ struct xilinx_spi {
> 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 */
> + bool big_endian; /* The device could be accessed big or little
> + * endian
> + */
> };
> =
> -static void xspi_init_hw(void __iomem *regs_base)
> +/* 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));
> +}
> +
> +static inline void xspi_write16(struct xilinx_spi *xspi, u32 offs, u16 v=
al)
> +{
> + if (xspi->big_endian)
> + iowrite16be(val, xspi->regs + offs + 2);
> + else
> + iowrite16(val, xspi->regs + offs);
> +}
> +
> +static inline void xspi_write32(struct xilinx_spi *xspi, u32 offs, u32 v=
al)
> +{
> + if (xspi->big_endian)
> + iowrite32be(val, xspi->regs + offs);
> + else
> + iowrite32(val, xspi->regs + offs);
> +}
> +
> +static inline u8 xspi_read8(struct xilinx_spi *xspi, u32 offs)
> +{
> + return ioread8(xspi->regs + offs + ((xspi->big_endian) ? 3 : 0));
> +}
> +
> +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);
> +}
> +
Hi Richard,
The registers of the device should all be accessible as 32 bit operations.
It seems like it would be simpler to do that.
Thanks,
John
> +static void xspi_init_hw(struct xilinx_spi *xspi)
> {
> /* Reset the SPI device */
> - out_be32(regs_base + XIPIF_V123B_RESETR_OFFSET,
> - XIPIF_V123B_RESET_MASK);
> + xspi_write32(xspi, XIPIF_V123B_RESETR_OFFSET, XIPIF_V123B_RESET_MASK);
> /* Disable all the interrupts just in case */
> - out_be32(regs_base + XIPIF_V123B_IIER_OFFSET, 0);
> + xspi_write32(xspi, XIPIF_V123B_IIER_OFFSET, 0);
> /* Enable the global IPIF interrupt */
> - out_be32(regs_base + XIPIF_V123B_DGIER_OFFSET,
> - XIPIF_V123B_GINTR_ENABLE);
> + xspi_write32(xspi, XIPIF_V123B_DGIER_OFFSET, XIPIF_V123B_GINTR_ENABLE);=
> /* Deselect the slave on the SPI bus */
> - out_be32(regs_base + XSPI_SSR_OFFSET, 0xffff);
> + xspi_write32(xspi, XSPI_SSR_OFFSET, 0xffff);
> /* Disable the transmitter, enable Manual Slave Select Assertion,
> * put SPI controller into master mode, and enable it */
> - out_be16(regs_base + XSPI_CR_OFFSET,
> + xspi_write16(xspi, XSPI_CR_OFFSET,
> XSPI_CR_TRANS_INHIBIT | XSPI_CR_MANUAL_SSELECT
> | XSPI_CR_MASTER_MODE | XSPI_CR_ENABLE);
> }
> @@ -110,16 +156,15 @@ static void xilinx_spi_chipselect(struct spi_device=
*spi, int is_on)
> =
> if (is_on =3D=3D BITBANG_CS_INACTIVE) {
> /* Deselect the slave on the SPI bus */
> - out_be32(xspi->regs + XSPI_SSR_OFFSET, 0xffff);
> + xspi_write32(xspi, XSPI_SSR_OFFSET, 0xffff);
> } else if (is_on =3D=3D BITBANG_CS_ACTIVE) {
> /* Set the SPI clock phase and polarity */
> - u16 cr =3D in_be16(xspi->regs + XSPI_CR_OFFSET)
> - & ~XSPI_CR_MODE_MASK;
> + u16 cr =3D xspi_read16(xspi, XSPI_CR_OFFSET) & ~XSPI_CR_MODE_MASK;
> if (spi->mode & SPI_CPHA)
> cr |=3D XSPI_CR_CPHA;
> if (spi->mode & SPI_CPOL)
> cr |=3D XSPI_CR_CPOL;
> - out_be16(xspi->regs + XSPI_CR_OFFSET, cr);
> + xspi_write16(xspi, XSPI_CR_OFFSET, cr);
> =
> /* We do not check spi->max_speed_hz here as the SPI clock
> * frequency is not software programmable (the IP block design
> @@ -127,7 +172,7 @@ static void xilinx_spi_chipselect(struct spi_device *=
spi, int is_on)
> */
> =
> /* Activate the chip select */
> - out_be32(xspi->regs + XSPI_SSR_OFFSET,
> + xspi_write32(xspi, XSPI_SSR_OFFSET,
> ~(0x0001 << spi->chip_select));
> }
> }
> @@ -174,15 +219,15 @@ static void xilinx_spi_fill_tx_fifo(struct xilinx_s=
pi *xspi)
> u8 sr;
> =
> /* Fill the Tx FIFO with as many bytes as possible */
> - sr =3D in_8(xspi->regs + XSPI_SR_OFFSET);
> + sr =3D xspi_read8(xspi, XSPI_SR_OFFSET);
> while ((sr & XSPI_SR_TX_FULL_MASK) =3D=3D 0 && xspi->remaining_bytes > =
0) {
> if (xspi->tx_ptr) {
> - out_8(xspi->regs + XSPI_TXD_OFFSET, *xspi->tx_ptr++);
> + xspi_write8(xspi, XSPI_TXD_OFFSET, *xspi->tx_ptr++);
> } else {
> - out_8(xspi->regs + XSPI_TXD_OFFSET, 0);
> + xspi_write8(xspi, XSPI_TXD_OFFSET, 0);
> }
> xspi->remaining_bytes--;
> - sr =3D in_8(xspi->regs + XSPI_SR_OFFSET);
> + sr =3D xspi_read8(xspi, XSPI_SR_OFFSET);
> }
> }
> =
> @@ -204,18 +249,18 @@ static int xilinx_spi_txrx_bufs(struct spi_device *=
spi, struct spi_transfer *t)
> /* Enable the transmit empty interrupt, which we use to determine
> * progress on the transmission.
> */
> - ipif_ier =3D in_be32(xspi->regs + XIPIF_V123B_IIER_OFFSET);
> - out_be32(xspi->regs + XIPIF_V123B_IIER_OFFSET,
> + ipif_ier =3D xspi_read32(xspi, XIPIF_V123B_IIER_OFFSET);
> + xspi_write32(xspi, XIPIF_V123B_IIER_OFFSET,
> ipif_ier | XSPI_INTR_TX_EMPTY);
> =
> /* Start the transfer by not inhibiting the transmitter any longer */
> - cr =3D in_be16(xspi->regs + XSPI_CR_OFFSET) & ~XSPI_CR_TRANS_INHIBIT;
> - out_be16(xspi->regs + XSPI_CR_OFFSET, cr);
> + cr =3D xspi_read16(xspi, XSPI_CR_OFFSET) & ~XSPI_CR_TRANS_INHIBIT;
> + xspi_write16(xspi, XSPI_CR_OFFSET, cr);
> =
> wait_for_completion(&xspi->done);
> =
> /* Disable the transmit empty interrupt */
> - out_be32(xspi->regs + XIPIF_V123B_IIER_OFFSET, ipif_ier);
> + xspi_write32(xspi, XIPIF_V123B_IIER_OFFSET, ipif_ier);
> =
> return t->len - xspi->remaining_bytes;
> }
> @@ -232,8 +277,8 @@ static irqreturn_t xilinx_spi_irq(int irq, void *dev_=
id)
> u32 ipif_isr;
> =
> /* Get the IPIF interrupts, and clear them immediately */
> - ipif_isr =3D in_be32(xspi->regs + XIPIF_V123B_IISR_OFFSET);
> - out_be32(xspi->regs + XIPIF_V123B_IISR_OFFSET, ipif_isr);
> + ipif_isr =3D xspi_read32(xspi, XIPIF_V123B_IISR_OFFSET);
> + xspi_write32(xspi, XIPIF_V123B_IISR_OFFSET, ipif_isr);
> =
> if (ipif_isr & XSPI_INTR_TX_EMPTY) { /* Transmission completed */
> u16 cr;
> @@ -244,20 +289,19 @@ static irqreturn_t xilinx_spi_irq(int irq, void *de=
v_id)
> * transmitter while the Isr refills the transmit register/FIFO,
> * or make sure it is stopped if we're done.
> */
> - cr =3D in_be16(xspi->regs + XSPI_CR_OFFSET);
> - out_be16(xspi->regs + XSPI_CR_OFFSET,
> - cr | XSPI_CR_TRANS_INHIBIT);
> + cr =3D xspi_read16(xspi, XSPI_CR_OFFSET);
> + xspi_write16(xspi, XSPI_CR_OFFSET, cr | XSPI_CR_TRANS_INHIBIT);
> =
> /* Read out all the data from the Rx FIFO */
> - sr =3D in_8(xspi->regs + XSPI_SR_OFFSET);
> + sr =3D xspi_read8(xspi, XSPI_SR_OFFSET);
> while ((sr & XSPI_SR_RX_EMPTY_MASK) =3D=3D 0) {
> u8 data;
> =
> - data =3D in_8(xspi->regs + XSPI_RXD_OFFSET);
> + data =3D xspi_read8(xspi, XSPI_RXD_OFFSET);
> if (xspi->rx_ptr) {
> *xspi->rx_ptr++ =3D data;
> }
> - sr =3D in_8(xspi->regs + XSPI_SR_OFFSET);
> + sr =3D xspi_read8(xspi, XSPI_SR_OFFSET);
> }
> =
> /* See if there is more data to send */
> @@ -266,7 +310,7 @@ static irqreturn_t xilinx_spi_irq(int irq, void *dev_=
id)
> /* Start the transfer by not inhibiting the
> * transmitter any longer
> */
> - out_be16(xspi->regs + XSPI_CR_OFFSET, cr);
> + xspi_write16(xspi, XSPI_CR_OFFSET, cr);
> } else {
> /* No more data to send.
> * Indicate the transfer is completed.
> @@ -279,7 +323,7 @@ static irqreturn_t xilinx_spi_irq(int irq, void *dev_=
id)
> }
> =
> struct spi_master *xilinx_spi_init(struct device *dev, struct resource *=
mem,
> - u32 irq, s16 bus_num, u16 num_chipselect)
> + u32 irq, s16 bus_num, u16 num_chipselect, bool big_endian)
> {
> struct spi_master *master;
> struct xilinx_spi *xspi;
> @@ -319,9 +363,10 @@ struct spi_master *xilinx_spi_init(struct device *de=
v, struct resource *mem,
> =
> xspi->mem =3D *mem;
> xspi->irq =3D irq;
> + xspi->big_endian =3D big_endian;
> =
> /* SPI controller initializations */
> - xspi_init_hw(xspi->regs);
> + xspi_init_hw(xspi);
> =
> /* Register for SPI Interrupt */
> ret =3D request_irq(xspi->irq, xilinx_spi_irq, 0, XILINX_SPI_NAME, xspi=
);
> diff --git a/drivers/spi/xilinx_spi.h b/drivers/spi/xilinx_spi.h
> index 84c98ee..c381c4a 100644
> --- a/drivers/spi/xilinx_spi.h
> +++ b/drivers/spi/xilinx_spi.h
> @@ -25,7 +25,7 @@
> #define XILINX_SPI_NAME "xilinx_spi"
> =
> struct spi_master *xilinx_spi_init(struct device *dev, struct resource *=
mem,
> - u32 irq, s16 bus_num, u16 num_chipselect);
> + u32 irq, s16 bus_num, u16 num_chipselect, bool big_endian);
> =
> void xilinx_spi_deinit(struct spi_master *master);
> #endif
> diff --git a/drivers/spi/xilinx_spi_of.c b/drivers/spi/xilinx_spi_of.c
> index 5440253..83f23be 100644
> --- a/drivers/spi/xilinx_spi_of.c
> +++ b/drivers/spi/xilinx_spi_of.c
> @@ -65,7 +65,8 @@ static int __init xilinx_spi_of_probe(struct of_device =
*ofdev,
> dev_warn(&ofdev->dev, "no 'xlnx,num-ss-bits' property\n");
> return -EINVAL;
> }
> - master =3D xilinx_spi_init(&ofdev->dev, r_mem, r_irq->start, -1, *prop)=
;
> + master =3D xilinx_spi_init(&ofdev->dev, r_mem, r_irq->start, -1, *prop,=
> + true);
> if (IS_ERR(master))
> return PTR_ERR(master);
> =
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 2/4] xilinx_spi: Switch to iomem functions and support little endian.
From: Grant Likely @ 2009-11-11 22:25 UTC (permalink / raw)
To: John Linn
Cc: spi-devel-general, Richard Röjfors, dbrownell, Andrew Morton,
linuxppc-dev
In-Reply-To: <1b8be32b-f557-4013-8443-4cff999e3b80@VA3EHSMHS013.ehs.local>
On Wed, Nov 11, 2009 at 3:19 PM, John Linn <John.Linn@xilinx.com> wrote:
>> -----Original Message-----
>> From: Richard R=F6jfors [mailto:richard.rojfors@mocean-labs.com]
>> Sent: Wednesday, November 11, 2009 7:39 AM
[...]
>> -static void xspi_init_hw(void __iomem *regs_base)
>> +/* 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 va=
l)
>> +{
>> + =A0 =A0 iowrite8(val, xspi->regs + offs + ((xspi->big_endian) ? 3 : 0)=
);
>> +}
>> +
>> +static inline void xspi_write16(struct xilinx_spi *xspi, u32 offs, u16 =
val)
>> +{
>> + =A0 =A0 if (xspi->big_endian)
>> + =A0 =A0 =A0 =A0 =A0 =A0 iowrite16be(val, xspi->regs + offs + 2);
>> + =A0 =A0 else
>> + =A0 =A0 =A0 =A0 =A0 =A0 iowrite16(val, xspi->regs + offs);
>> +}
>> +
>> +static inline void xspi_write32(struct xilinx_spi *xspi, u32 offs, u32 =
val)
>> +{
>> + =A0 =A0 if (xspi->big_endian)
>> + =A0 =A0 =A0 =A0 =A0 =A0 iowrite32be(val, xspi->regs + offs);
>> + =A0 =A0 else
>> + =A0 =A0 =A0 =A0 =A0 =A0 iowrite32(val, xspi->regs + offs);
>> +}
>> +
>> +static inline u8 xspi_read8(struct xilinx_spi *xspi, u32 offs)
>> +{
>> + =A0 =A0 return ioread8(xspi->regs + offs + ((xspi->big_endian) ? 3 : 0=
));
>> +}
>> +
>> +static inline u16 xspi_read16(struct xilinx_spi *xspi, u32 offs)
>> +{
>> + =A0 =A0 if (xspi->big_endian)
>> + =A0 =A0 =A0 =A0 =A0 =A0 return ioread16be(xspi->regs + offs + 2);
>> + =A0 =A0 else
>> + =A0 =A0 =A0 =A0 =A0 =A0 return ioread16(xspi->regs + offs);
>> +}
>> +
>> +static inline u32 xspi_read32(struct xilinx_spi *xspi, u32 offs)
>> +{
>> + =A0 =A0 if (xspi->big_endian)
>> + =A0 =A0 =A0 =A0 =A0 =A0 return ioread32be(xspi->regs + offs);
>> + =A0 =A0 else
>> + =A0 =A0 =A0 =A0 =A0 =A0 return ioread32(xspi->regs + offs);
>> +}
>> +
>
> Hi Richard,
>
> The registers of the device should all be accessible as 32 bit operations=
.
>
> It seems like it would be simpler to do that.
If all register accesses can be converted to 32bits, then definitely
you should just do that and make your patch simpler.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [alsa-devel] [PATCH 2/6] ASoC/mpc5200: get rid of the appl_ptr tracking nonsense
From: Grant Likely @ 2009-11-11 21:57 UTC (permalink / raw)
To: Jon Smirl; +Cc: John Bonesio, alsa-devel, Mark Brown, linuxppc-dev, lrg
In-Reply-To: <9e4733910911111334o24f3114as6baaef82d7b17a05@mail.gmail.com>
On Wed, Nov 11, 2009 at 2:34 PM, Jon Smirl <jonsmirl@gmail.com> wrote:
> On Wed, Nov 11, 2009 at 2:24 PM, Grant Likely <grant.likely@secretlab.ca>=
wrote:
>> On Wed, Nov 11, 2009 at 11:37 AM, Mark Brown
>> <broonie@opensource.wolfsonmicro.com> wrote:
>>> On Wed, Nov 11, 2009 at 11:38:06AM -0500, Jon Smirl wrote:
>>>> > Providing a final valid data point to the driver would possibly even
>>>> > make things worse since if it were used then you'd have the equivale=
nt
>>>> > race where the application has initialized some data but not yet man=
aged
>>>> > to update the driver to tell it it's being handed over; if the drive=
r
>>>
>>>> That's an under run condition.
>>>
>>> Yes, of course - the issue is that this approach encourages them, makin=
g
>>> the system less robust if things are on the edge. =A0The mpc5200 seems =
to
>>> be not just on the edge but comfortably beyond it for some reason.
>>
>> I can't reproduce the issue at all as long at the dev_dbg() statement
>> in the trigger stop path is disabled. =A0With it enabled, I hear the
>> problem every time. =A0The 5200 may not be a speedy beast, but it is
>> plenty fast enough to shut down the audio stream before stale data
>> starts getting played out.
>
> "fast enough" - you just said it is a race.
> I've been saying it is a race too.
Yes, it is a race; but not the kind that is dangerous. Audio playout
is always a real-time problem; whether in the middle of a stream or at
the end. If the CPU gets nailed with an unbounded latency, then there
will be audible artifacts - Regardless of whether the driver knows
where the end of data is or not. If it does know, then audio will
stutter. If it doesn't know, then there will be repeated samples.
Both are nasty to the human ear. So, making the driver do extra work
to keep the extra data in sync will probably force larger minimum
latencies for playout (trouble for VoIP apps) so the CPU can keep up,
and won't help one iota for making audio better.
The real solution is to fix the worst case latencies.
> There are two options:
> 1) Eliminate the race by developing a system to deterministically flag
> the end of valid data.
> 2) Fudge everything around making it almost impossible to lose the
> race, but the race is still there.
3) eliminate the unbounded latencies (fix the PSC driver and/or use a
real time kernel)
4) make sure userspace fills all the periods with silence before
triggering stop. Gstreamer seems to already do this. I suspect
pulseaudio does the same.
> The dev_dbg() aggravates the race until it is obviously visible every
> time. A deterministic solution would not be impacted by the dev_dbg().
But it still wouldn't help a bit when the same latency occurs in the
middle of playback.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH v5 0/4] pseries: Add cede support for cpu-offline
From: Peter Zijlstra @ 2009-11-11 21:45 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Gautham R Shenoy, linux-kernel, Arun R Bharadwaj, Andrew Morton,
linuxppc-dev, Ingo Molnar
In-Reply-To: <1257975350.2140.49.camel@pasglop>
On Thu, 2009-11-12 at 08:35 +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2009-11-11 at 14:25 +0100, Peter Zijlstra wrote:
> > On Fri, 2009-10-30 at 10:52 +0530, Gautham R Shenoy wrote:
> >
> > > Gautham R Shenoy (4):
> > > pseries: Serialize cpu hotplug operations during deactivate Vs deallocate
> > > pseries: Add code to online/offline CPUs of a DLPAR node.
> > > pSeries: Add hooks to put the CPU into an appropriate offline state
> > > pSeries: extended_cede_processor() helper function.
> > >
> > >
> > > Documentation/cpu-hotplug.txt | 6 +
> > > arch/powerpc/include/asm/lppaca.h | 9 +
> > > arch/powerpc/platforms/pseries/dlpar.c | 129 ++++++++++++++++
> > > arch/powerpc/platforms/pseries/hotplug-cpu.c | 182 ++++++++++++++++++++++-
> > > arch/powerpc/platforms/pseries/offline_states.h | 18 ++
> > > arch/powerpc/platforms/pseries/plpar_wrappers.h | 22 +++
> > > arch/powerpc/platforms/pseries/smp.c | 19 ++
> > > arch/powerpc/xmon/xmon.c | 3
> > > drivers/base/cpu.c | 2
> > > include/linux/cpu.h | 13 ++
> > > 10 files changed, 387 insertions(+), 16 deletions(-)
> > > create mode 100644 arch/powerpc/platforms/pseries/offline_states.h
> >
> > Not quite sure how you solved the DLPAR communication but since pretty
> > much everything is under arch/powerpc/ I really don't have much to say.
>
> Allright. I was hoping to have your ack for the drivers/base/cpu.c
> change :-)
Sure.
Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
^ permalink raw reply
* Re: [PATCH v2] xilinx_spi: Add the platform driver to the Kconfig
From: Grant Likely @ 2009-11-11 21:36 UTC (permalink / raw)
To: Richard Röjfors
Cc: spi-devel-general, Andrew Morton, dbrownell, John Linn,
linuxppc-dev
In-Reply-To: <4AFAE870.7080308@mocean-labs.com>
On Wed, Nov 11, 2009 at 9:38 AM, Richard R=F6jfors
<richard.rojfors@mocean-labs.com> wrote:
> This patch adds the xilinx_spi_pltfm to the SPI Kconfig and Makefile.
>
> The xilinx_spi_pltfm was added in a previous patchset.
I assume this will just get rolled into your series when you respin
for the next version.
g.
>
> Signed-off-by: Richard R=F6jfors <richard.rojfors@mocean-labs.com>
> ---
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index b956284..d1f8ee3 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -252,6 +252,13 @@ config SPI_XILINX_OF
> =A0 =A0 =A0 =A0help
> =A0 =A0 =A0 =A0 =A0This is the OF driver for the SPI controller IP from t=
he Xilinx EDK.
>
> +config SPI_XILINX_PLTFM
> + =A0 =A0 =A0 tristate "Xilinx SPI controller platform device"
> + =A0 =A0 =A0 depends on SPI_XILINX
> + =A0 =A0 =A0 help
> + =A0 =A0 =A0 =A0 This is the platform driver for the SPI controller IP
> + =A0 =A0 =A0 =A0 from the Xilinx EDK.
> +
> =A0#
> =A0# Add new SPI master controllers in alphabetical order above this line
> =A0#
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 97dee8f..d8b0e4c 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_SPI_S3C24XX) =A0 =A0 =A0 =A0 =A0 =A0 +=3D =
spi_s3c24xx.o
> =A0obj-$(CONFIG_SPI_TXX9) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 +=3D spi_txx9.o
> =A0obj-$(CONFIG_SPI_XILINX) =A0 =A0 =A0 =A0 =A0 =A0 =A0 +=3D xilinx_spi.o
> =A0obj-$(CONFIG_SPI_XILINX_OF) =A0 =A0 =A0 =A0 =A0 =A0+=3D xilinx_spi_of.=
o
> +obj-$(CONFIG_SPI_XILINX_PLTFM) =A0 =A0 =A0 =A0 +=3D xilinx_spi_pltfm.o
> =A0obj-$(CONFIG_SPI_SH_SCI) =A0 =A0 =A0 =A0 =A0 =A0 =A0 +=3D spi_sh_sci.o
> =A0obj-$(CONFIG_SPI_STMP3XXX) =A0 =A0 =A0 =A0 =A0 =A0 +=3D spi_stmp.o
> =A0# =A0 =A0 =A0... add above this line ...
>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH v5 0/4] pseries: Add cede support for cpu-offline
From: Benjamin Herrenschmidt @ 2009-11-11 21:35 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Gautham R Shenoy, linux-kernel, Arun R Bharadwaj, Andrew Morton,
linuxppc-dev, Ingo Molnar
In-Reply-To: <1257945935.4372.36.camel@twins>
On Wed, 2009-11-11 at 14:25 +0100, Peter Zijlstra wrote:
> On Fri, 2009-10-30 at 10:52 +0530, Gautham R Shenoy wrote:
>
> > Gautham R Shenoy (4):
> > pseries: Serialize cpu hotplug operations during deactivate Vs deallocate
> > pseries: Add code to online/offline CPUs of a DLPAR node.
> > pSeries: Add hooks to put the CPU into an appropriate offline state
> > pSeries: extended_cede_processor() helper function.
> >
> >
> > Documentation/cpu-hotplug.txt | 6 +
> > arch/powerpc/include/asm/lppaca.h | 9 +
> > arch/powerpc/platforms/pseries/dlpar.c | 129 ++++++++++++++++
> > arch/powerpc/platforms/pseries/hotplug-cpu.c | 182 ++++++++++++++++++++++-
> > arch/powerpc/platforms/pseries/offline_states.h | 18 ++
> > arch/powerpc/platforms/pseries/plpar_wrappers.h | 22 +++
> > arch/powerpc/platforms/pseries/smp.c | 19 ++
> > arch/powerpc/xmon/xmon.c | 3
> > drivers/base/cpu.c | 2
> > include/linux/cpu.h | 13 ++
> > 10 files changed, 387 insertions(+), 16 deletions(-)
> > create mode 100644 arch/powerpc/platforms/pseries/offline_states.h
>
> Not quite sure how you solved the DLPAR communication but since pretty
> much everything is under arch/powerpc/ I really don't have much to say.
Allright. I was hoping to have your ack for the drivers/base/cpu.c
change :-)
Cheers,
Ben.
^ permalink raw reply
* Re: [alsa-devel] [PATCH 2/6] ASoC/mpc5200: get rid of the appl_ptr tracking nonsense
From: Jon Smirl @ 2009-11-11 21:34 UTC (permalink / raw)
To: Grant Likely; +Cc: John Bonesio, alsa-devel, Mark Brown, linuxppc-dev, lrg
In-Reply-To: <fa686aa40911111124i12b72196xdba7000b13b3bdc@mail.gmail.com>
On Wed, Nov 11, 2009 at 2:24 PM, Grant Likely <grant.likely@secretlab.ca> w=
rote:
> On Wed, Nov 11, 2009 at 11:37 AM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
>> On Wed, Nov 11, 2009 at 11:38:06AM -0500, Jon Smirl wrote:
>>> > Providing a final valid data point to the driver would possibly even
>>> > make things worse since if it were used then you'd have the equivalen=
t
>>> > race where the application has initialized some data but not yet mana=
ged
>>> > to update the driver to tell it it's being handed over; if the driver
>>
>>> That's an under run condition.
>>
>> Yes, of course - the issue is that this approach encourages them, making
>> the system less robust if things are on the edge. =A0The mpc5200 seems t=
o
>> be not just on the edge but comfortably beyond it for some reason.
>
> I can't reproduce the issue at all as long at the dev_dbg() statement
> in the trigger stop path is disabled. =A0With it enabled, I hear the
> problem every time. =A0The 5200 may not be a speedy beast, but it is
> plenty fast enough to shut down the audio stream before stale data
> starts getting played out.
"fast enough" - you just said it is a race.
I've been saying it is a race too.
There are two options:
1) Eliminate the race by developing a system to deterministically flag
the end of valid data.
2) Fudge everything around making it almost impossible to lose the
race, but the race is still there.
The dev_dbg() aggravates the race until it is obviously visible every
time. A deterministic solution would not be impacted by the dev_dbg().
Implementing a deterministic solution requires support from ALSA core.
--=20
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [alsa-devel] [PATCH 2/6] ASoC/mpc5200: get rid of the appl_ptr tracking nonsense
From: Jon Smirl @ 2009-11-11 21:26 UTC (permalink / raw)
To: Mark Brown; +Cc: John Bonesio, alsa-devel, linuxppc-dev, lrg
In-Reply-To: <20091111183753.GA31815@rakim.wolfsonmicro.main>
On Wed, Nov 11, 2009 at 1:37 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Nov 11, 2009 at 11:38:06AM -0500, Jon Smirl wrote:
>
>> There are two solutions:
>> 1) tell me where the end of the valid data is. That allows me to
>> program the hardware to not enqueue the invalid data.
>> 2) For batched hardware, pad an extra period with silence after the
>> end of the stream. (that what zeroing the buffer before handing it
>> back to ALSA
>
> You've also got the option of lying about where the hardware is in some
> form in order to give you more headroom.
>
> I'm not sure what you mean by "batched hardware" here.
SNDRV_PCM_INFO_BATCH
Hardware that can't give you the DMA position except at the end of DMA
transfers.
>
>> I believe this race is present in all ALSA drivers. =A0It's just a lot
>> harder to hit on different hardware. For example to hit it on Intel
>> HDA which is non-batched hardware, the song would need to end right at
>> the end of a period. Then the interrupt latency would need to be bad
>> enough that some invalid data got played. But x86 CPUs are very fast
>> so it is rare for the interrupt latency to be bad enough that the
>> stream doesn't get stopped in time.
>
> The potential is there for this to happen on any hardware, yes. =A0 On th=
e
> other hand, it's not been a pressing issue elswhere - including on
> things like older ARM systems which aren't exactly noted for their
> snappy performance. =A0It really does sound like there's something specia=
l
> going on with these systems that's at least somewhat unique to them.
>
>> > Providing a final valid data point to the driver would possibly even
>> > make things worse since if it were used then you'd have the equivalent
>> > race where the application has initialized some data but not yet manag=
ed
>> > to update the driver to tell it it's being handed over; if the driver
>
>> That's an under run condition.
>
> Yes, of course - the issue is that this approach encourages them, making
> the system less robust if things are on the edge. =A0The mpc5200 seems to
> be not just on the edge but comfortably beyond it for some reason.
>
--=20
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* suspend to ram on pmac ?
From: Giuliano Pochini @ 2009-11-11 21:14 UTC (permalink / raw)
To: linuxppc-dev
I have a pmac G4 MDD. Is suspend to ram supported ? I enabled all pm
related kernel options I could find, but:
# ls -la /sys/power/
total 0
drwxr-xr-x 2 root root 0 Nov 9 19:33 .
drwxr-xr-x 12 root root 0 Nov 9 19:32 ..
-rw-r--r-- 1 root root 4096 Nov 9 19:33 pm_test
-rw-r--r-- 1 root root 4096 Nov 9 19:33 state
# cat pm_test
[none] core processors platform devices freezer
# cat state
#
I do not actually need STR. I have to test suspend/resume support for a
driver, so I need at least a working pm_test at devices level.
--
Giuliano.
^ permalink raw reply
* Re: [PATCH 4/4] xilinx_spi: add a platform driver using the xilinx_spi common module.
From: Grant Likely @ 2009-11-11 21:17 UTC (permalink / raw)
To: Richard Röjfors
Cc: spi-devel-general, Andrew Morton, dbrownell, John Linn,
linuxppc-dev
In-Reply-To: <4AFACCDE.7050802@mocean-labs.com>
On Wed, Nov 11, 2009 at 7:40 AM, Richard R=F6jfors
<richard.rojfors@mocean-labs.com> wrote:
> This patch adds in a platform device driver using the xilinx_spi common m=
odule.
>
> Signed-off-by: Richard R=F6jfors <richard.rojfors@mocean-labs.com>
Pretty straight forward stuff. Looks right to me. (Except of course
for adding the pdata structure earlier in the series so that the
of_driver can use it too.
Thanks for all this work.
Cheers,
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 3/4] xilinx_spi: add support for the DS570 IP.
From: Grant Likely @ 2009-11-11 21:15 UTC (permalink / raw)
To: Richard Röjfors
Cc: spi-devel-general, Andrew Morton, dbrownell, John Linn,
linuxppc-dev
In-Reply-To: <4AFACCAC.10304@mocean-labs.com>
On Wed, Nov 11, 2009 at 7:39 AM, Richard R=F6jfors
<richard.rojfors@mocean-labs.com> wrote:
> This patch adds in support for the DS570 IP.
>
> It's register compatible with the DS464, but adds support for 8/16/32 SPI=
.
>
> Signed-off-by: Richard R=F6jfors <richard.rojfors@mocean-labs.com>
Needs some changes. Comments below....
> ---
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 9667650..b956284 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -235,7 +235,7 @@ config SPI_TXX9
> =A0 =A0 =A0 =A0 =A0SPI driver for Toshiba TXx9 MIPS SoCs
>
> =A0config SPI_XILINX
> - =A0 =A0 =A0 tristate "Xilinx SPI controller"
> + =A0 =A0 =A0 tristate "Xilinx SPI controller common module"
> =A0 =A0 =A0 =A0depends on HAS_IOMEM && EXPERIMENTAL
> =A0 =A0 =A0 =A0select SPI_BITBANG
> =A0 =A0 =A0 =A0help
> @@ -244,6 +244,8 @@ config SPI_XILINX
> =A0 =A0 =A0 =A0 =A0See the "OPB Serial Peripheral Interface (SPI) (v1.00e=
)"
> =A0 =A0 =A0 =A0 =A0Product Specification document (DS464) for hardware de=
tails.
>
> + =A0 =A0 =A0 =A0 Or for the DS570, see "XPS Serial Peripheral Interface =
(SPI) (v2.00b)"
> +
> =A0config SPI_XILINX_OF
> =A0 =A0 =A0 =A0tristate "Xilinx SPI controller OF device"
> =A0 =A0 =A0 =A0depends on SPI_XILINX && (XILINX_VIRTEX || MICROBLAZE)
> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
> index b00dabc..ae744ba 100644
> --- a/drivers/spi/xilinx_spi.c
> +++ b/drivers/spi/xilinx_spi.c
> @@ -24,7 +24,7 @@
> =A0/* Register definitions as per "OPB Serial Peripheral Interface (SPI) =
(v1.00e)
> =A0* Product Specification", DS464
> =A0*/
> -#define XSPI_CR_OFFSET =A0 =A0 =A0 =A0 0x60 =A0 =A0/* 16-bit Control Reg=
ister */
> +#define XSPI_CR_OFFSET =A0 =A0 =A0 =A0 0x60 =A0 =A0/* Control Register *=
/
>
> =A0#define XSPI_CR_ENABLE =A0 =A0 =A0 =A0 0x02
> =A0#define XSPI_CR_MASTER_MODE =A0 =A00x04
> @@ -35,8 +35,9 @@
> =A0#define XSPI_CR_RXFIFO_RESET =A0 0x40
> =A0#define XSPI_CR_MANUAL_SSELECT 0x80
> =A0#define XSPI_CR_TRANS_INHIBIT =A00x100
> +#define XSPI_CR_LSB_FIRST =A0 =A0 =A00x200
>
> -#define XSPI_SR_OFFSET =A0 =A0 =A0 =A0 0x64 =A0 =A0/* 8-bit Status Regis=
ter */
> +#define XSPI_SR_OFFSET =A0 =A0 =A0 =A0 0x64 =A0 =A0/* Status Register */
>
> =A0#define XSPI_SR_RX_EMPTY_MASK =A00x01 =A0 =A0/* Receive FIFO is empty =
*/
> =A0#define XSPI_SR_RX_FULL_MASK =A0 0x02 =A0 =A0/* Receive FIFO is full *=
/
> @@ -44,8 +45,8 @@
> =A0#define XSPI_SR_TX_FULL_MASK =A0 0x08 =A0 =A0/* Transmit FIFO is full =
*/
> =A0#define XSPI_SR_MODE_FAULT_MASK =A0 =A0 =A0 =A00x10 =A0 =A0/* Mode fau=
lt error */
>
> -#define XSPI_TXD_OFFSET =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x68 =A0 =A0/* 8-=
bit Data Transmit Register */
> -#define XSPI_RXD_OFFSET =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x6c =A0 =A0/* 8-=
bit Data Receive Register */
> +#define XSPI_TXD_OFFSET =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x68 =A0 =A0/* Da=
ta Transmit Register */
> +#define XSPI_RXD_OFFSET =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x6c =A0 =A0/* Da=
ta Receive Register */
>
> =A0#define XSPI_SSR_OFFSET =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x70 =A0 =A0/* =
32-bit Slave Select Register */
>
> @@ -65,6 +66,7 @@
> =A0#define XSPI_INTR_TX_UNDERRUN =A0 =A0 =A0 =A0 =A00x08 =A0 =A0/* TxFIFO=
was underrun */
> =A0#define XSPI_INTR_RX_FULL =A0 =A0 =A0 =A0 =A0 =A0 =A00x10 =A0 =A0/* Rx=
FIFO is full */
> =A0#define XSPI_INTR_RX_OVERRUN =A0 =A0 =A0 =A0 =A0 0x20 =A0 =A0/* RxFIFO=
was overrun */
> +#define XSPI_INTR_TX_HALF_EMPTY =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x40 =A0 =
=A0/* TxFIFO is half empty */
>
> =A0#define XIPIF_V123B_RESETR_OFFSET =A0 =A0 =A00x40 =A0 =A0/* IPIF reset=
register */
> =A0#define XIPIF_V123B_RESET_MASK =A0 =A0 =A0 =A0 0x0a =A0 =A0/* the valu=
e to write */
> @@ -86,6 +88,7 @@ struct xilinx_spi {
> =A0 =A0 =A0 =A0bool big_endian; =A0 =A0 =A0 =A0/* The device could be acc=
essed big or little
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * endian
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */
> + =A0 =A0 =A0 u8 bits_per_word;
> =A0};
>
> =A0/* to follow are some functions that does little of big endian read an=
d
> @@ -146,8 +149,9 @@ static void xspi_init_hw(struct xilinx_spi *xspi)
> =A0 =A0 =A0 =A0/* Disable the transmitter, enable Manual Slave Select Ass=
ertion,
> =A0 =A0 =A0 =A0 * put SPI controller into master mode, and enable it */
> =A0 =A0 =A0 =A0xspi_write16(xspi, XSPI_CR_OFFSET,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0XSPI_CR_TRANS_INHIBIT | XSPI_CR_MANUAL_S=
SELECT
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0| XSPI_CR_MASTER_MODE | XSPI_CR_ENABLE);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 XSPI_CR_TRANS_INHIBIT | XSPI_CR_MANUAL_SSEL=
ECT |
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 XSPI_CR_MASTER_MODE | XSPI_CR_ENABLE | XSPI=
_CR_TXFIFO_RESET |
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 XSPI_CR_RXFIFO_RESET);
> =A0}
>
> =A0static void xilinx_spi_chipselect(struct spi_device *spi, int is_on)
> @@ -179,17 +183,20 @@ static void xilinx_spi_chipselect(struct spi_device=
*spi, int is_on)
>
> =A0/* spi_bitbang requires custom setup_transfer() to be defined if there=
is a
> =A0* custom txrx_bufs(). We have nothing to setup here as the SPI IP bloc=
k
> - * supports just 8 bits per word, and SPI clock can't be changed in soft=
ware.
> - * Check for 8 bits per word. Chip select delay calculations could be
> + * supports 8 or 16 bits per word, which can not be changed in software.
> + * SPI clock can't be changed in software.
> + * Check for correct bits per word. Chip select delay calculations could=
be
> =A0* added here as soon as bitbang_work() can be made aware of the delay =
value.
> =A0*/
> =A0static int xilinx_spi_setup_transfer(struct spi_device *spi,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct spi_transfer *t)
> + =A0 =A0 =A0 struct spi_transfer *t)
> =A0{
> =A0 =A0 =A0 =A0u8 bits_per_word;
> + =A0 =A0 =A0 struct xilinx_spi *xspi =3D spi_master_get_devdata(spi->mas=
ter);
>
> - =A0 =A0 =A0 bits_per_word =3D (t) ? t->bits_per_word : spi->bits_per_wo=
rd;
> - =A0 =A0 =A0 if (bits_per_word !=3D 8) {
> + =A0 =A0 =A0 bits_per_word =3D (t->bits_per_word) ? t->bits_per_word :
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spi->bits_per_word;
> + =A0 =A0 =A0 if (bits_per_word !=3D xspi->bits_per_word) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dev_err(&spi->dev, "%s, unsupported bits_p=
er_word=3D%d\n",
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0__func__, bits_per_word);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -EINVAL;
> @@ -200,33 +207,49 @@ static int xilinx_spi_setup_transfer(struct spi_dev=
ice *spi,
>
> =A0static int xilinx_spi_setup(struct spi_device *spi)
> =A0{
> - =A0 =A0 =A0 struct spi_bitbang *bitbang;
> - =A0 =A0 =A0 struct xilinx_spi *xspi;
> - =A0 =A0 =A0 int retval;
> -
> - =A0 =A0 =A0 xspi =3D spi_master_get_devdata(spi->master);
> - =A0 =A0 =A0 bitbang =3D &xspi->bitbang;
> -
> - =A0 =A0 =A0 retval =3D xilinx_spi_setup_transfer(spi, NULL);
> - =A0 =A0 =A0 if (retval < 0)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 return retval;
> -
> + =A0 =A0 =A0 /* always return 0, we can not check the number of bits.
> + =A0 =A0 =A0 =A0* There are cases when SPI setup is called before any dr=
iver is
> + =A0 =A0 =A0 =A0* there, in that case the SPI core defaults to 8 bits, w=
hich we
> + =A0 =A0 =A0 =A0* do not support in some cases. But if we return an erro=
r, the
> + =A0 =A0 =A0 =A0* SPI device would not be registered and no driver can g=
et hold of it
> + =A0 =A0 =A0 =A0* When the driver is there, it will call SPI setup again=
with the
> + =A0 =A0 =A0 =A0* correct number of bits per transfer.
> + =A0 =A0 =A0 =A0* If a driver setups with the wrong bit number, it will =
fail when
> + =A0 =A0 =A0 =A0* it tries to do a transfer
> + =A0 =A0 =A0 =A0*/
> =A0 =A0 =A0 =A0return 0;
> =A0}
>
> =A0static void xilinx_spi_fill_tx_fifo(struct xilinx_spi *xspi)
> =A0{
> =A0 =A0 =A0 =A0u8 sr;
> + =A0 =A0 =A0 u8 wsize;
> + =A0 =A0 =A0 if (xspi->bits_per_word =3D=3D 8)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 wsize =3D 1;
> + =A0 =A0 =A0 else if (xspi->bits_per_word =3D=3D 16)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 wsize =3D 2;
> + =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 wsize =3D 4;
wsize =3D xspi->bits_per_word / 8 perhaps?
>
> =A0 =A0 =A0 =A0/* Fill the Tx FIFO with as many bytes as possible */
> =A0 =A0 =A0 =A0sr =3D xspi_read8(xspi, XSPI_SR_OFFSET);
> - =A0 =A0 =A0 while ((sr & XSPI_SR_TX_FULL_MASK) =3D=3D 0 && xspi->remain=
ing_bytes > 0) {
> + =A0 =A0 =A0 while ((sr & XSPI_SR_TX_FULL_MASK) =3D=3D 0 &&
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->remaining_bytes > 0) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (xspi->tx_ptr) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi_write8(xspi, XSPI_TXD_=
OFFSET, *xspi->tx_ptr++);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (wsize =3D=3D 1)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi_write8=
(xspi, XSPI_TXD_OFFSET,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 *xspi->tx_ptr);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else if (wsize =3D=3D 2)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi_write1=
6(xspi, XSPI_TXD_OFFSET,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 *(u16 *)(xspi->tx_ptr));
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else if (wsize =3D=3D 4)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi_write3=
2(xspi, XSPI_TXD_OFFSET,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 *(u32 *)(xspi->tx_ptr));
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->tx_ptr +=3D wsize;
This would go better as a callback instead of performing the same test
*every time* through the loop. Make for simpler code too.
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } else
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0xspi_write8(xspi, XSPI_TXD=
_OFFSET, 0);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->remaining_bytes--;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->remaining_bytes -=3D wsize;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sr =3D xspi_read8(xspi, XSPI_SR_OFFSET);
> =A0 =A0 =A0 =A0}
> =A0}
> @@ -283,6 +306,13 @@ static irqreturn_t xilinx_spi_irq(int irq, void *dev=
_id)
> =A0 =A0 =A0 =A0if (ipif_isr & XSPI_INTR_TX_EMPTY) { =A0 =A0/* Transmissio=
n completed */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0u16 cr;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0u8 sr;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 u8 rsize;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (xspi->bits_per_word =3D=3D 8)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rsize =3D 1;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else if (xspi->bits_per_word =3D=3D 16)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rsize =3D 2;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rsize =3D 4;
Ditto.
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* A transmit has just completed. Process =
received data and
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * check for more data to transmit. Always=
inhibit the
> @@ -295,11 +325,22 @@ static irqreturn_t xilinx_spi_irq(int irq, void *de=
v_id)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Read out all the data from the Rx FIFO =
*/
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sr =3D xspi_read8(xspi, XSPI_SR_OFFSET);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0while ((sr & XSPI_SR_RX_EMPTY_MASK) =3D=3D=
0) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 u8 data;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 u32 data;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (rsize =3D=3D 1)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 data =3D xs=
pi_read8(xspi, XSPI_RXD_OFFSET);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else if (rsize =3D=3D 2)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 data =3D xs=
pi_read16(xspi, XSPI_RXD_OFFSET);
> + =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 data =3D xs=
pi_read32(xspi, XSPI_RXD_OFFSET);
>
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 data =3D xspi_read8(xspi, X=
SPI_RXD_OFFSET);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (xspi->rx_ptr) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 *xspi->rx_p=
tr++ =3D data;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (rsize =
=3D=3D 1)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 *xspi->rx_ptr =3D data & 0xff;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else if (rs=
ize =3D=3D 2)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 *(u16 *)(xspi->rx_ptr) =3D data & 0xffff;
> + =A0 =A0 =A0 =A0 =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 =A0 =A0 =A0=
=A0 *((u32 *)(xspi->rx_ptr)) =3D data;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->rx_pt=
r +=3D rsize;
ditto
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sr =3D xspi_read8(xspi, XS=
PI_SR_OFFSET);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> @@ -323,7 +364,8 @@ static irqreturn_t xilinx_spi_irq(int irq, void *dev_=
id)
> =A0}
>
> =A0struct spi_master *xilinx_spi_init(struct device *dev, struct resource=
*mem,
> - =A0 =A0 =A0 u32 irq, s16 bus_num, u16 num_chipselect, bool big_endian)
> + =A0 =A0 =A0 u32 irq, s16 bus_num, u16 num_chipselect, bool big_endian,
> + =A0 =A0 =A0 u8 bits_per_word)
As mentioned in my comments in patch 1/4; the increase to the
parameters to _init() would not be needed if the of_driver stowed
everything into a pdata structure.
Cheers,
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 2/4] xilinx_spi: Switch to iomem functions and support little endian.
From: Grant Likely @ 2009-11-11 21:09 UTC (permalink / raw)
To: Richard Röjfors
Cc: spi-devel-general, Andrew Morton, dbrownell, John Linn,
linuxppc-dev
In-Reply-To: <4AFACC6A.304@mocean-labs.com>
On Wed, Nov 11, 2009 at 7:38 AM, Richard R=F6jfors
<richard.rojfors@mocean-labs.com> wrote:
> This patch changes the out_(be)(8|16|32) and in_(be)(8|16|32) calls to io=
write(8|16|32)
> and ioread(8|16|32). This to be able to build on platforms not supporting=
the in/out calls
> for instance x86.
As discussed previously, I'd rather see as an ops table instead of as
if/else in each accessor, but I won't push it unless I hear the same
opinion from others. John what do you think? (But if it should be an
ops table, then I want it done now so that we don't end up with
another just as invasive patch to change the form yet again in 6
months time).
Otherwise looks good to me on brief read-through.
Cheers,
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 1/4] xilinx_spi: Split into of driver and generic part.
From: Grant Likely @ 2009-11-11 21:03 UTC (permalink / raw)
To: Richard Röjfors
Cc: spi-devel-general, Andrew Morton, dbrownell, John Linn,
linuxppc-dev
In-Reply-To: <4AFACC55.3010802@mocean-labs.com>
On Wed, Nov 11, 2009 at 7:38 AM, Richard R=F6jfors
<richard.rojfors@mocean-labs.com> wrote:
> This patch splits the xilinx_spi driver into a generic part and a
> OF driver part.
>
> The reason for this is to later add in a platform driver as well.
Hey Richard,
Thanks for the quick response. A couple of important comments, and a
bunch of nitpicks. Comments below.
> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
> index 46b8c5c..1562e9b 100644
> --- a/drivers/spi/xilinx_spi.c
> +++ b/drivers/spi/xilinx_spi.c
> @@ -14,11 +14,6 @@
> =A0#include <linux/module.h>
> =A0#include <linux/init.h>
> =A0#include <linux/interrupt.h>
> -#include <linux/platform_device.h>
> -
> -#include <linux/of_platform.h>
> -#include <linux/of_device.h>
> -#include <linux/of_spi.h>
>
> =A0#include <linux/spi/spi.h>
> =A0#include <linux/spi/spi_bitbang.h>
> @@ -78,7 +73,7 @@ struct xilinx_spi {
> =A0 =A0 =A0 =A0/* bitbang has to be first */
> =A0 =A0 =A0 =A0struct spi_bitbang bitbang;
> =A0 =A0 =A0 =A0struct completion done;
> -
> + =A0 =A0 =A0 struct resource mem; /* phys mem */
> =A0 =A0 =A0 =A0void __iomem =A0 =A0*regs; =A0/* virt. address of the cont=
rol registers */
>
> =A0 =A0 =A0 =A0u32 =A0 =A0 =A0 =A0 =A0 =A0 irq;
> @@ -283,40 +278,17 @@ static irqreturn_t xilinx_spi_irq(int irq, void *de=
v_id)
> =A0 =A0 =A0 =A0return IRQ_HANDLED;
> =A0}
>
> -static int __init xilinx_spi_of_probe(struct of_device *ofdev,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 const struct of_device_id *match)
> +struct spi_master *xilinx_spi_init(struct device *dev, struct resource *=
mem,
> + =A0 =A0 =A0 u32 irq, s16 bus_num, u16 num_chipselect)
Nit: I personally prefer like _setup and _teardown instead of _init
and _deinit; but that's just me. (and bike sheds should be blue)
Also, in patch 4/4, a new platform_data structure is defined. The
platform probe routine extracts the pdata and passes each item
individually to _init. The of_driver probe does the same, except it
extracts the data from the device tree. That is also the approach
that I used to take when writing drivers with multiple bindings.
However, it becomes a problem as a driver matures and more and more
data needs to be passed.
Instead, how about getting the of_driver probe to allocate and
populate the pdata structure and stow it in of_dev->dev.platform_data.
That way the _init function signature stays sane, the platform driver
code becomes simpler, and the driver is better prepared to handle the
eventual deprecation of the of_platform bus.
> =A0{
> =A0 =A0 =A0 =A0struct spi_master *master;
> =A0 =A0 =A0 =A0struct xilinx_spi *xspi;
> - =A0 =A0 =A0 struct resource r_irq_struct;
> - =A0 =A0 =A0 struct resource r_mem_struct;
> -
> - =A0 =A0 =A0 struct resource *r_irq =3D &r_irq_struct;
> - =A0 =A0 =A0 struct resource *r_mem =3D &r_mem_struct;
> - =A0 =A0 =A0 int rc =3D 0;
> - =A0 =A0 =A0 const u32 *prop;
> - =A0 =A0 =A0 int len;
> + =A0 =A0 =A0 int ret =3D 0;
>
> - =A0 =A0 =A0 /* Get resources(memory, IRQ) associated with the device */
> - =A0 =A0 =A0 master =3D spi_alloc_master(&ofdev->dev, sizeof(struct xili=
nx_spi));
> -
> - =A0 =A0 =A0 if (master =3D=3D NULL) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENOMEM;
> - =A0 =A0 =A0 }
> + =A0 =A0 =A0 master =3D spi_alloc_master(dev, sizeof(struct xilinx_spi))=
;
>
> - =A0 =A0 =A0 dev_set_drvdata(&ofdev->dev, master);
> -
> - =A0 =A0 =A0 rc =3D of_address_to_resource(ofdev->node, 0, r_mem);
> - =A0 =A0 =A0 if (rc) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_warn(&ofdev->dev, "invalid address\n");
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto put_master;
> - =A0 =A0 =A0 }
> -
> - =A0 =A0 =A0 rc =3D of_irq_to_resource(ofdev->node, 0, r_irq);
> - =A0 =A0 =A0 if (rc =3D=3D NO_IRQ) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_warn(&ofdev->dev, "no IRQ found\n");
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto put_master;
> - =A0 =A0 =A0 }
> + =A0 =A0 =A0 if (master =3D=3D NULL)
Nit: 'if (!master)' is a pretty well accepted idiom.
> @@ -329,128 +301,70 @@ static int __init xilinx_spi_of_probe(struct of_de=
vice *ofdev,
[...]
> =A0free_irq:
> =A0 =A0 =A0 =A0free_irq(xspi->irq, xspi);
> =A0unmap_io:
> =A0 =A0 =A0 =A0iounmap(xspi->regs);
> -release_mem:
> - =A0 =A0 =A0 release_mem_region(r_mem->start, resource_size(r_mem));
> +map_failed:
> + =A0 =A0 =A0 release_mem_region(mem->start, resource_size(mem));
> =A0put_master:
> =A0 =A0 =A0 =A0spi_master_put(master);
> - =A0 =A0 =A0 return rc;
> + =A0 =A0 =A0 return ERR_PTR(ret);
The SPI subsystem does not use the ERR_PTR() pattern, and errors are
already printed to the console when a failure occurs. Please return
NULL on failure so that the driver isn't mixing idioms.
> diff --git a/drivers/spi/xilinx_spi.h b/drivers/spi/xilinx_spi.h
> new file mode 100644
> index 0000000..84c98ee
> --- /dev/null
> +++ b/drivers/spi/xilinx_spi.h
> @@ -0,0 +1,31 @@
> +/*
> + * xilinx_spi.h
Nit: filename is kind of meaningless. Say what that file /is/ instead.
ie: * Xilinx SPI device driver API and platform data header file
> + * Copyright (c) 2009 Intel Corporation
> + *
> + * 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. =A0See 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., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#ifndef _XILINX_SPI_H_
> +#define _XILINX_SPI_H_ 1
Nit: The '1' is unnecessary
> +
> +#include <linux/spi/spi.h>
> +#include <linux/spi/spi_bitbang.h>
> +
> +#define XILINX_SPI_NAME "xilinx_spi"
> +
> +struct spi_master *xilinx_spi_init(struct device *dev, struct resource *=
mem,
> + =A0 =A0 =A0 u32 irq, s16 bus_num, u16 num_chipselect);
> +
> +void xilinx_spi_deinit(struct spi_master *master);
> +#endif
> diff --git a/drivers/spi/xilinx_spi_of.c b/drivers/spi/xilinx_spi_of.c
> new file mode 100644
> index 0000000..5440253
> --- /dev/null
> +++ b/drivers/spi/xilinx_spi_of.c
> @@ -0,0 +1,126 @@
> +/*
> + * xilinx_spi_of.c Support for Xilinx SPI OF devices
Ditto nit here.
Looking good. Thanks for this work.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: Re: [PATCH 3/3] mpc52xx/wdt: WDT uses GPT api
From: Grant Likely @ 2009-11-11 20:18 UTC (permalink / raw)
To: Albrecht Dreß; +Cc: linuxppc-dev, devicetree-discuss, wim
In-Reply-To: <16246194.1257928342802.JavaMail.ngmail@webmail19.ha2.local>
On Wed, Nov 11, 2009 at 1:32 AM, Albrecht Dre=DF <albrecht.dress@arcor.de> =
wrote:
> Hi Grant:
>
> O.k., thanks for your comments. =A0If Wim doesn't have any objections to =
it, I will provide a merged patch. =A0One consequence I forgot to mention i=
s that we loose the ability to build the wdt support as module, but I don't=
think it's a real problem.
I'm not too worried about this. It is also potentially possible to
rework the entire GPT driver as a module. But there are non-trivial
architecture fiddly bits needed to make it safe to load IRQ
controllers in a module.
> I think we still should keep the kernel config option enable/disable the =
wdt support, which would mask out the wdt code if disabled. =A0Is that ok f=
or you?
Yup.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 2/3] mpc52xx/wdt: merge WDT code into the GPT
From: Grant Likely @ 2009-11-11 20:11 UTC (permalink / raw)
To: Albrecht Dreß; +Cc: linuxppc-dev, devicetree-discuss, wim
In-Reply-To: <1980629.1257928058732.JavaMail.ngmail@webmail19.ha2.local>
On Wed, Nov 11, 2009 at 1:27 AM, Albrecht Dre=DF <albrecht.dress@arcor.de> =
wrote:
> Hi Grant:
>
> Thanks a lot for your comments!
>
>> On Tue, Nov 10, 2009 at 12:41 PM, Albrecht Dre=DF <albrecht.dress@arcor.=
de>
>> > +#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->n=
ode, "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 MPC52=
xx_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_=
start(gpt, *on_boot_wdt) =3D=3D
>> 0)
>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_info=
(gpt->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=
(gpt->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 =
function as wdt\n");
>> > + =A0 =A0 =A0 }
>> > +#endif
>>
>> Ditto on the #if/endif
>
> I don't think it can be removed here. =A0Think about the following:
> - user has a dtb with fsl,has-wdt and fsl,wdt-on-boot properties and
> - disables 5200 WDT in the kernel config.
>
> Now the system refuses to use GPT0 as GPT, although the WDT functionality=
has been disabled. =A0Of course, the dts doesn't fit the kernel config now=
, but I think we should catch this case.
I think the behaviour should be consistent regardless of what support
is compiled into the kernel. If the wdt-on-boot property is set, the
it does make sense for the kernel to never use it as a GPT. Otherwise
the kernel behaviour becomes subtly different in ways non-obvious to
the user.
> Actually I tried to implement your requirements from <http://lists.ozlabs=
.org/pipermail/linuxppc-dev/2009-August/074595.html>:
> - fsl,has-wdt indicates that the GPT can serve as WDT;
> - any access through the wdt api to a wdt-capable gpt actually reserves i=
t as wdt;
> - the new of property forces reserving the wdt before any gpt api functio=
n has chance to grab it as gpt.
>
> Or did I get something wrong here?
No, you got it right... Well, I can't even remember what I said
yesterday, but this list of requirements looks sane. :-)
Cheers,
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [alsa-devel] [PATCH 2/6] ASoC/mpc5200: get rid of the appl_ptr tracking nonsense
From: Mark Brown @ 2009-11-11 20:03 UTC (permalink / raw)
To: Grant Likely
Cc: John Bonesio, alsa-devel@alsa-project.org,
linuxppc-dev@lists.ozlabs.org, lrg@slimlogic.co.uk
In-Reply-To: <fa686aa40911111124i12b72196xdba7000b13b3bdc@mail.gmail.com>
On 11 Nov 2009, at 19:24, Grant Likely <grant.likely@secretlab.ca>
wrote:
> On Wed, Nov 11, 2009 at 11:37 AM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
>> On Wed, Nov 11, 2009 at 11:38:06AM -0500, Jon Smirl wrote:
>>>> Providing a final valid data point to the driver would possibly
>>>> even
>>>> make things worse since if it were used then you'd have the
>>>> equivalent
>>>> race where the application has initialized some data but not yet
>>>> managed
>>>> to update the driver to tell it it's being handed over; if the
>>>> driver
>>
>>> That's an under run condition.
>>
>> Yes, of course - the issue is that this approach encourages them,
>> making
>> the system less robust if things are on the edge. The mpc5200
>> seems to
>> be not just on the edge but comfortably beyond it for some reason.
>
> I can't reproduce the issue at all as long at the dev_dbg() statement
> in the trigger stop path is disabled. With it enabled, I hear the
> problem every time. The 5200 may not be a speedy beast, but it is
> plenty fast enough to shut down the audio stream before stale data
> starts getting played out.
Yes, that does sound entirely consistent with the problem being a
latency issue if you're sending the dev_dbg() output to the serial
port. I'd be surprised if it were anything else to be honest.
^ permalink raw reply
* Re: [alsa-devel] [PATCH 2/6] ASoC/mpc5200: get rid of the appl_ptr tracking nonsense
From: Grant Likely @ 2009-11-11 19:24 UTC (permalink / raw)
To: Mark Brown; +Cc: John Bonesio, alsa-devel, linuxppc-dev, lrg
In-Reply-To: <20091111183753.GA31815@rakim.wolfsonmicro.main>
On Wed, Nov 11, 2009 at 11:37 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Nov 11, 2009 at 11:38:06AM -0500, Jon Smirl wrote:
>> > Providing a final valid data point to the driver would possibly even
>> > make things worse since if it were used then you'd have the equivalent
>> > race where the application has initialized some data but not yet manag=
ed
>> > to update the driver to tell it it's being handed over; if the driver
>
>> That's an under run condition.
>
> Yes, of course - the issue is that this approach encourages them, making
> the system less robust if things are on the edge. =A0The mpc5200 seems to
> be not just on the edge but comfortably beyond it for some reason.
I can't reproduce the issue at all as long at the dev_dbg() statement
in the trigger stop path is disabled. With it enabled, I hear the
problem every time. The 5200 may not be a speedy beast, but it is
plenty fast enough to shut down the audio stream before stale data
starts getting played out.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [alsa-devel] [PATCH 2/6] ASoC/mpc5200: get rid of the appl_ptr tracking nonsense
From: Mark Brown @ 2009-11-11 18:37 UTC (permalink / raw)
To: Jon Smirl; +Cc: John Bonesio, alsa-devel, linuxppc-dev, lrg
In-Reply-To: <9e4733910911110838w6010cf3atcfcd30c85f0764a1@mail.gmail.com>
On Wed, Nov 11, 2009 at 11:38:06AM -0500, Jon Smirl wrote:
> There are two solutions:
> 1) tell me where the end of the valid data is. That allows me to
> program the hardware to not enqueue the invalid data.
> 2) For batched hardware, pad an extra period with silence after the
> end of the stream. (that what zeroing the buffer before handing it
> back to ALSA
You've also got the option of lying about where the hardware is in some
form in order to give you more headroom.
I'm not sure what you mean by "batched hardware" here.
> I believe this race is present in all ALSA drivers. It's just a lot
> harder to hit on different hardware. For example to hit it on Intel
> HDA which is non-batched hardware, the song would need to end right at
> the end of a period. Then the interrupt latency would need to be bad
> enough that some invalid data got played. But x86 CPUs are very fast
> so it is rare for the interrupt latency to be bad enough that the
> stream doesn't get stopped in time.
The potential is there for this to happen on any hardware, yes. On the
other hand, it's not been a pressing issue elswhere - including on
things like older ARM systems which aren't exactly noted for their
snappy performance. It really does sound like there's something special
going on with these systems that's at least somewhat unique to them.
> > Providing a final valid data point to the driver would possibly even
> > make things worse since if it were used then you'd have the equivalent
> > race where the application has initialized some data but not yet managed
> > to update the driver to tell it it's being handed over; if the driver
> That's an under run condition.
Yes, of course - the issue is that this approach encourages them, making
the system less robust if things are on the edge. The mpc5200 seems to
be not just on the edge but comfortably beyond it for some reason.
^ permalink raw reply
* [PATCH v2] xilinx_spi: Add the platform driver to the Kconfig
From: Richard Röjfors @ 2009-11-11 16:38 UTC (permalink / raw)
To: spi-devel-general; +Cc: linuxppc-dev, Andrew Morton, dbrownell, John Linn
This patch adds the xilinx_spi_pltfm to the SPI Kconfig and Makefile.
The xilinx_spi_pltfm was added in a previous patchset.
Signed-off-by: Richard Röjfors <richard.rojfors@mocean-labs.com>
---
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index b956284..d1f8ee3 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -252,6 +252,13 @@ config SPI_XILINX_OF
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.
+
#
# Add new SPI master controllers in alphabetical order above this line
#
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 97dee8f..d8b0e4c 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_SPI_S3C24XX) += spi_s3c24xx.o
obj-$(CONFIG_SPI_TXX9) += spi_txx9.o
obj-$(CONFIG_SPI_XILINX) += xilinx_spi.o
obj-$(CONFIG_SPI_XILINX_OF) += xilinx_spi_of.o
+obj-$(CONFIG_SPI_XILINX_PLTFM) += xilinx_spi_pltfm.o
obj-$(CONFIG_SPI_SH_SCI) += spi_sh_sci.o
obj-$(CONFIG_SPI_STMP3XXX) += spi_stmp.o
# ... add above this line ...
^ permalink raw reply related
* Re: [alsa-devel] [PATCH 2/6] ASoC/mpc5200: get rid of the appl_ptr tracking nonsense
From: Jon Smirl @ 2009-11-11 16:38 UTC (permalink / raw)
To: Mark Brown; +Cc: John Bonesio, alsa-devel, linuxppc-dev, lrg
In-Reply-To: <20091107201233.GC31789@sirena.org.uk>
On Sat, Nov 7, 2009 at 3:12 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Sat, Nov 07, 2009 at 11:51:16AM -0700, Grant Likely wrote:
>> On Sat, Nov 7, 2009 at 5:51 AM, Jon Smirl <jonsmirl@gmail.com> wrote:
>
>> current period. =A0My understanding of ALSA is that the application is
>> supposed to make sure there is enough silence in the buffer to handle
>> the lag between notification that the last period with valid data has
>> been played out and the stop trigger.
>
> This is certainly the most robust approach for applications. =A0For a
> large proportion of hardware it won't matter too much since they're able
> to shut down the audio very quickly but that can't be entirely relied
> upon, especially at higher rates on slower machines.
I can comment but no access to test equipment...
Playing invalid data always happens in the current ALSA model. The
only question is does enough of it play to be audible.
on the mpc5200 batched driver...
At the end of song ALSA only pads out the last period with silence.
When this buffer is fully enqueued the DMA hardware generates an interrupt.
The DMA hardware also begins enqueuing the next period.
Now we start a race. Can ALSA come back from that interrupt and stop
the playing of the enqueued data before it makes it out of the FIFO?
An mpc5200 is slow enough that the CPU never makes it back in time.
This isn't a latency problem, it never makes it back in time. Latency
issues just make it play more invalid data.
There are two solutions:
1) tell me where the end of the valid data is. That allows me to
program the hardware to not enqueue the invalid data.
2) For batched hardware, pad an extra period with silence after the
end of the stream. (that what zeroing the buffer before handing it
back to ALSA
I believe this race is present in all ALSA drivers. It's just a lot
harder to hit on different hardware. For example to hit it on Intel
HDA which is non-batched hardware, the song would need to end right at
the end of a period. Then the interrupt latency would need to be bad
enough that some invalid data got played. But x86 CPUs are very fast
so it is rare for the interrupt latency to be bad enough that the
stream doesn't get stopped in time.
>
>> occur. =A0That says to me that the real problem is an unbounded latency
>> caused by another part of the kernel (the tty console in this case).
>
> That's certainly not going to help anything here - if a delay is
> introduced in telling the hardware to shut down the DMA then that
> increases the chance for the DMA controller to start pushing valid audio
> data from the buffer to the audio interface.
>
>> > A much cleaner solution would be for ALSA to provide a field that
>> > indicates the last valid address in the ring buffer system. Then in
>> > the driver's buffer complete callback I could get that value and
>> > reprogram the DMA engine not to run off the end of valid data. As each
>> > buffer completes I would reread the value and update the DMA stop
>> > address. You also need the last valid address field when DMA is first
>> > started.
>
>> ... assuming that audio needs to stop exactly at the end of valid
>> data. =A0But if the last few periods are silence, then this assumption
>> isn't true.
>
> Indeed, it makes the whole thing much more reliable.
>
> Providing a final valid data point to the driver would possibly even
> make things worse since if it were used then you'd have the equivalent
> race where the application has initialized some data but not yet managed
> to update the driver to tell it it's being handed over; if the driver
> just carries on running through the data there's a reasonable chance
> nobody will notice that case.
That's an under run condition.
ALSA would need to track how many periods the driver has queue. If
ALSA has received enough interrupts to know that the driver is playing
the last period, it would not be safe to just tack the data onto the
end. You would also need to call into the driver with a 'append' call.
That's because it isn't possible for ALSA to deterministically know if
the last period has finished playing or not. In the 'append' call
implementation the driver would determine if the DMA hardware was
still running and restart it if needed.
>
--=20
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* [PATCH] xilinx_spi: Add the platform driver to the Kconfig
From: Richard Röjfors @ 2009-11-11 16:33 UTC (permalink / raw)
To: spi-devel-general; +Cc: linuxppc-dev, Andrew Morton, dbrownell, John Linn
This patch adds the xilinx_spi_pltfm to the SPI Kconfig.
The xilinx_spi_pltfm was added in a previous patchset.
Signed-off-by: Richard Röjfors <richard.rojfors@mocean-labs.com>
---
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index b956284..d1f8ee3 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -252,6 +252,13 @@ config SPI_XILINX_OF
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.
+
#
# Add new SPI master controllers in alphabetical order above this line
#
^ permalink raw reply related
* Re: [PATCH 2/4] xilinx_spi: Switch to iomem functions and support little endian.
From: Richard Röjfors @ 2009-11-11 14:54 UTC (permalink / raw)
To: Josh Boyer
Cc: spi-devel-general, Andrew Morton, dbrownell, John Linn,
linuxppc-dev
In-Reply-To: <20091111144924.GG30489@zod.rchland.ibm.com>
Josh Boyer wrote:
> On Wed, Nov 11, 2009 at 03:38:34PM +0100, Richard Röjfors wrote:
>> This patch changes the out_(be)(8|16|32) and in_(be)(8|16|32) calls to iowrite(8|16|32)
>> and ioread(8|16|32). This to be able to build on platforms not supporting the in/out calls
>> for instance x86.
>>
>> Support is also added for little endian writes. In some systems the registers should be
>> accessed little endian rather than big endian.
>
> I wonder if you should make the endianness a config option. Right now you
> have a conditional check for every read and write. Does that impact
> performance at all?
It won't affect the performance noticeable.
In our case we need the possibility to run both endians using the same xilinx_spi module.
--Richard
^ permalink raw reply
* Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
From: Scott Wood @ 2009-11-11 15:26 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <OF2843D474.207958C1-ONC125766A.00828F87-C125766B.00009091@transmode.se>
On Wed, Nov 11, 2009 at 01:06:10AM +0100, Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 11/11/2009 00:21:18:
> > 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.
But that only works if you take an ITLB miss at the right time.
-Scott
^ permalink raw reply
* Re: [PATCH 0/6] gianfar: Some fixes
From: Kumar Gala @ 2009-11-11 15:16 UTC (permalink / raw)
To: avorontsov
Cc: Jon Loeliger, Kumar Gopalpet-B05799, netdev, linuxppc-dev,
Andy Fleming, Stephen Hemminger, David Miller, Lennert Buytenhek
In-Reply-To: <20091111001036.GA28576@oksana.dev.rtsoft.ru>
On Nov 10, 2009, at 6:10 PM, Anton Vorontsov wrote:
> Hi all,
>
> Here are some fixes for the gianfar driver, patches on the way.
>
> Thanks,
Acked-by: Kumar Gala <galak@kernel.crashing.org>
- k
^ permalink raw reply
* Re: [PATCH 2/4] xilinx_spi: Switch to iomem functions and support little endian.
From: Josh Boyer @ 2009-11-11 14:49 UTC (permalink / raw)
To: Richard Röjfors
Cc: spi-devel-general, Andrew Morton, dbrownell, John Linn,
linuxppc-dev
In-Reply-To: <4AFACC6A.304@mocean-labs.com>
On Wed, Nov 11, 2009 at 03:38:34PM +0100, Richard Röjfors wrote:
>This patch changes the out_(be)(8|16|32) and in_(be)(8|16|32) calls to iowrite(8|16|32)
>and ioread(8|16|32). This to be able to build on platforms not supporting the in/out calls
>for instance x86.
>
>Support is also added for little endian writes. In some systems the registers should be
>accessed little endian rather than big endian.
I wonder if you should make the endianness a config option. Right now you
have a conditional check for every read and write. Does that impact
performance at all?
josh
^ 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