* Re: [PATCH v2 2/4] xilinx_spi: Switch to iomem functions and support little endian.
From: Grant Likely @ 2009-11-12 14:59 UTC (permalink / raw)
To: Richard Röjfors
Cc: spi-devel-general, Andrew Morton, dbrownell, John Linn,
linuxppc-dev
In-Reply-To: <4AFC1B2D.2070809@mocean-labs.com>
On Thu, Nov 12, 2009 at 7:26 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 32=
bits ioread/iowrite.
>
> The read and write function are attached to the internal struct as callba=
cks, callback
> is selected depending on endianess.
>
> This will also build on platforms not supporting the in/out calls for ins=
tance x86.
>
> Signed-off-by: Richard R=F6jfors <richard.rojfors@mocean-labs.com>
On brief review looks good to me.
Acked-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> 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
>
> =A0config SPI_XILINX
> =A0 =A0 =A0 =A0tristate "Xilinx SPI controller"
> - =A0 =A0 =A0 depends on EXPERIMENTAL
> + =A0 =A0 =A0 depends on HAS_IOMEM && EXPERIMENTAL
> =A0 =A0 =A0 =A0select SPI_BITBANG
> =A0 =A0 =A0 =A0help
> =A0 =A0 =A0 =A0 =A0This exposes the SPI controller IP from the Xilinx EDK=
.
> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
> index 5761a4c..d0ca13a 100644
> --- a/drivers/spi/xilinx_spi.c
> +++ b/drivers/spi/xilinx_spi.c
> @@ -27,7 +27,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 0x62 =A0 =A0/* 16-bit Control Reg=
ister */
> +#define XSPI_CR_OFFSET =A0 =A0 =A0 =A0 0x60 =A0 =A0/* 16-bit Control Reg=
ister */
>
> =A0#define XSPI_CR_ENABLE =A0 =A0 =A0 =A0 0x02
> =A0#define XSPI_CR_MASTER_MODE =A0 =A00x04
> @@ -39,7 +39,7 @@
> =A0#define XSPI_CR_MANUAL_SSELECT 0x80
> =A0#define XSPI_CR_TRANS_INHIBIT =A00x100
>
> -#define XSPI_SR_OFFSET =A0 =A0 =A0 =A0 0x67 =A0 =A0/* 8-bit Status Regis=
ter */
> +#define XSPI_SR_OFFSET =A0 =A0 =A0 =A0 0x64 =A0 =A0/* 8-bit Status Regis=
ter */
>
> =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 *=
/
> @@ -47,8 +47,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 =A00x6b =A0 =A0/* 8-=
bit Data Transmit Register */
> -#define XSPI_RXD_OFFSET =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x6f =A0 =A0/* 8-=
bit Data Receive Register */
> +#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 */
>
> =A0#define XSPI_SSR_OFFSET =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x70 =A0 =A0/* =
32-bit Slave Select Register */
>
> @@ -86,25 +86,29 @@ struct xilinx_spi {
> =A0 =A0 =A0 =A0u8 *rx_ptr; =A0 =A0 =A0 =A0 =A0 =A0 /* pointer in the Tx b=
uffer */
> =A0 =A0 =A0 =A0const u8 *tx_ptr; =A0 =A0 =A0 /* pointer in the Rx buffer =
*/
> =A0 =A0 =A0 =A0int remaining_bytes; =A0 =A0/* the number of bytes left to=
transfer */
> + =A0 =A0 =A0 unsigned int (*read_fn) (void __iomem *);
> + =A0 =A0 =A0 void (*write_fn) (u32, void __iomem *);
> =A0};
>
> -static void xspi_init_hw(void __iomem *regs_base)
> +static void xspi_init_hw(struct xilinx_spi *xspi)
> =A0{
> + =A0 =A0 =A0 void __iomem *regs_base =3D xspi->regs;
> +
> =A0 =A0 =A0 =A0/* Reset the SPI device */
> - =A0 =A0 =A0 out_be32(regs_base + XIPIF_V123B_RESETR_OFFSET,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0XIPIF_V123B_RESET_MASK);
> + =A0 =A0 =A0 xspi->write_fn(XIPIF_V123B_RESET_MASK,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 regs_base + XIPIF_V123B_RESETR_OFFSET);
> =A0 =A0 =A0 =A0/* Disable all the interrupts just in case */
> - =A0 =A0 =A0 out_be32(regs_base + XIPIF_V123B_IIER_OFFSET, 0);
> + =A0 =A0 =A0 xspi->write_fn(0, regs_base + XIPIF_V123B_IIER_OFFSET);
> =A0 =A0 =A0 =A0/* Enable the global IPIF interrupt */
> - =A0 =A0 =A0 out_be32(regs_base + XIPIF_V123B_DGIER_OFFSET,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0XIPIF_V123B_GINTR_ENABLE);
> + =A0 =A0 =A0 xspi->write_fn(XIPIF_V123B_GINTR_ENABLE,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 regs_base + XIPIF_V123B_DGIER_OFFSET);
> =A0 =A0 =A0 =A0/* Deselect the slave on the SPI bus */
> - =A0 =A0 =A0 out_be32(regs_base + XSPI_SSR_OFFSET, 0xffff);
> + =A0 =A0 =A0 xspi->write_fn(0xffff, regs_base + XSPI_SSR_OFFSET);
> =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 out_be16(regs_base + 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 xspi->write_fn(XSPI_CR_TRANS_INHIBIT | XSPI_CR_MANUAL_SSELE=
CT |
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 XSPI_CR_MASTER_MODE | XSPI_CR_ENABLE,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 regs_base + XSPI_CR_OFFSET);
> =A0}
>
> =A0static void xilinx_spi_chipselect(struct spi_device *spi, int is_on)
> @@ -113,16 +117,16 @@ static void xilinx_spi_chipselect(struct spi_device=
*spi, int is_on)
>
> =A0 =A0 =A0 =A0if (is_on =3D=3D BITBANG_CS_INACTIVE) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Deselect the slave on the SPI bus */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(xspi->regs + XSPI_SSR_OFFSET, 0xff=
ff);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->write_fn(0xffff, xspi->regs + XSPI_SS=
R_OFFSET);
> =A0 =A0 =A0 =A0} else if (is_on =3D=3D BITBANG_CS_ACTIVE) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Set the SPI clock phase and polarity */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 u16 cr =3D in_be16(xspi->regs + XSPI_CR_OFF=
SET)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 u16 cr =3D xspi->read_fn(xspi->regs + XSPI_=
CR_OFFSET)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 & ~XSPI_CR_MODE_MASK;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (spi->mode & SPI_CPHA)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0cr |=3D XSPI_CR_CPHA;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (spi->mode & SPI_CPOL)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0cr |=3D XSPI_CR_CPOL;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be16(xspi->regs + XSPI_CR_OFFSET, cr);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->write_fn(cr, xspi->regs + XSPI_CR_OFF=
SET);
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* We do not check spi->max_speed_hz here =
as the SPI clock
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * frequency is not software programmable =
(the IP block design
> @@ -130,8 +134,8 @@ static void xilinx_spi_chipselect(struct spi_device *=
spi, int is_on)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Activate the chip select */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(xspi->regs + XSPI_SSR_OFFSET,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0~(0x0001 << spi->chip_se=
lect));
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->write_fn(~(0x0001 << spi->chip_select=
),
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->regs + XSPI_SSR_OFFSE=
T);
> =A0 =A0 =A0 =A0}
> =A0}
>
> @@ -177,15 +181,15 @@ static void xilinx_spi_fill_tx_fifo(struct xilinx_s=
pi *xspi)
> =A0 =A0 =A0 =A0u8 sr;
>
> =A0 =A0 =A0 =A0/* Fill the Tx FIFO with as many bytes as possible */
> - =A0 =A0 =A0 sr =3D in_8(xspi->regs + XSPI_SR_OFFSET);
> + =A0 =A0 =A0 sr =3D xspi->read_fn(xspi->regs + XSPI_SR_OFFSET);
> =A0 =A0 =A0 =A0while ((sr & XSPI_SR_TX_FULL_MASK) =3D=3D 0 && xspi->remai=
ning_bytes > 0) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (xspi->tx_ptr) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_8(xspi->regs + 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 out_8(xspi->regs + XSPI_TXD=
_OFFSET, 0);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (xspi->tx_ptr)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->write_fn(*xspi->tx_pt=
r++,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->regs =
+ XSPI_TXD_OFFSET);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->write_fn(0, xspi->reg=
s + XSPI_TXD_OFFSET);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0xspi->remaining_bytes--;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 sr =3D in_8(xspi->regs + XSPI_SR_OFFSET);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 sr =3D xspi->read_fn(xspi->regs + XSPI_SR_O=
FFSET);
> =A0 =A0 =A0 =A0}
> =A0}
>
> @@ -207,18 +211,19 @@ static int xilinx_spi_txrx_bufs(struct spi_device *=
spi, struct spi_transfer *t)
> =A0 =A0 =A0 =A0/* Enable the transmit empty interrupt, which we use to de=
termine
> =A0 =A0 =A0 =A0 * progress on the transmission.
> =A0 =A0 =A0 =A0 */
> - =A0 =A0 =A0 ipif_ier =3D in_be32(xspi->regs + XIPIF_V123B_IIER_OFFSET);
> - =A0 =A0 =A0 out_be32(xspi->regs + XIPIF_V123B_IIER_OFFSET,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ipif_ier | XSPI_INTR_TX_EMPTY);
> + =A0 =A0 =A0 ipif_ier =3D xspi->read_fn(xspi->regs + XIPIF_V123B_IIER_OF=
FSET);
> + =A0 =A0 =A0 xspi->write_fn(ipif_ier | XSPI_INTR_TX_EMPTY,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->regs + XIPIF_V123B_IIER_OFFSET);
>
> =A0 =A0 =A0 =A0/* Start the transfer by not inhibiting the transmitter an=
y longer */
> - =A0 =A0 =A0 cr =3D in_be16(xspi->regs + XSPI_CR_OFFSET) & ~XSPI_CR_TRAN=
S_INHIBIT;
> - =A0 =A0 =A0 out_be16(xspi->regs + XSPI_CR_OFFSET, cr);
> + =A0 =A0 =A0 cr =3D xspi->read_fn(xspi->regs + XSPI_CR_OFFSET) &
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 ~XSPI_CR_TRANS_INHIBIT;
> + =A0 =A0 =A0 xspi->write_fn(cr, xspi->regs + XSPI_CR_OFFSET);
>
> =A0 =A0 =A0 =A0wait_for_completion(&xspi->done);
>
> =A0 =A0 =A0 =A0/* Disable the transmit empty interrupt */
> - =A0 =A0 =A0 out_be32(xspi->regs + XIPIF_V123B_IIER_OFFSET, ipif_ier);
> + =A0 =A0 =A0 xspi->write_fn(ipif_ier, xspi->regs + XIPIF_V123B_IIER_OFFS=
ET);
>
> =A0 =A0 =A0 =A0return t->len - xspi->remaining_bytes;
> =A0}
> @@ -235,8 +240,8 @@ static irqreturn_t xilinx_spi_irq(int irq, void *dev_=
id)
> =A0 =A0 =A0 =A0u32 ipif_isr;
>
> =A0 =A0 =A0 =A0/* Get the IPIF interrupts, and clear them immediately */
> - =A0 =A0 =A0 ipif_isr =3D in_be32(xspi->regs + XIPIF_V123B_IISR_OFFSET);
> - =A0 =A0 =A0 out_be32(xspi->regs + XIPIF_V123B_IISR_OFFSET, ipif_isr);
> + =A0 =A0 =A0 ipif_isr =3D xspi->read_fn(xspi->regs + XIPIF_V123B_IISR_OF=
FSET);
> + =A0 =A0 =A0 xspi->write_fn(ipif_isr, xspi->regs + XIPIF_V123B_IISR_OFFS=
ET);
>
> =A0 =A0 =A0 =A0if (ipif_isr & XSPI_INTR_TX_EMPTY) { =A0 =A0/* Transmissio=
n completed */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0u16 cr;
> @@ -247,20 +252,20 @@ static irqreturn_t xilinx_spi_irq(int irq, void *de=
v_id)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * transmitter while the Isr refills the t=
ransmit register/FIFO,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * or make sure it is stopped if we're don=
e.
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 cr =3D in_be16(xspi->regs + XSPI_CR_OFFSET)=
;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be16(xspi->regs + XSPI_CR_OFFSET,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0cr | XSPI_CR_TRANS_INHIB=
IT);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 cr =3D xspi->read_fn(xspi->regs + XSPI_CR_O=
FFSET);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->write_fn(cr | XSPI_CR_TRANS_INHIBIT,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->regs + XSPI_CR_OFFSET=
);
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Read out all the data from the Rx FIFO =
*/
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 sr =3D in_8(xspi->regs + XSPI_SR_OFFSET);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 sr =3D xspi->read_fn(xspi->regs + XSPI_SR_O=
FFSET);
> =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 =A0u8 data;
>
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 data =3D in_8(xspi->regs + =
XSPI_RXD_OFFSET);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 data =3D xspi->read_fn(xspi=
->regs + XSPI_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 =A0*xspi->rx_=
ptr++ =3D data;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 sr =3D in_8(xspi->regs + XS=
PI_SR_OFFSET);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 sr =3D xspi->read_fn(xspi->=
regs + XSPI_SR_OFFSET);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* See if there is more data to send */
> @@ -269,7 +274,7 @@ static irqreturn_t xilinx_spi_irq(int irq, void *dev_=
id)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Start the transfer by n=
ot inhibiting the
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * transmitter any longer
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be16(xspi->regs + XSPI_=
CR_OFFSET, cr);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->write_fn(cr, xspi->re=
gs + XSPI_CR_OFFSET);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* No more data to send.
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * Indicate the transfer i=
s completed.
> @@ -324,9 +329,16 @@ struct spi_master *xilinx_spi_init(struct device *de=
v, struct resource *mem,
>
> =A0 =A0 =A0 =A0xspi->mem =3D *mem;
> =A0 =A0 =A0 =A0xspi->irq =3D irq;
> + =A0 =A0 =A0 if (pdata->little_endian) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->read_fn =3D ioread32;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->write_fn =3D iowrite32;
> + =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->read_fn =3D ioread32be;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 xspi->write_fn =3D iowrite32be;
> + =A0 =A0 =A0 }
>
> =A0 =A0 =A0 =A0/* SPI controller initializations */
> - =A0 =A0 =A0 xspi_init_hw(xspi->regs);
> + =A0 =A0 =A0 xspi_init_hw(xspi);
>
> =A0 =A0 =A0 =A0/* Register for SPI Interrupt */
> =A0 =A0 =A0 =A0ret =3D request_irq(xspi->irq, xilinx_spi_irq, 0, XILINX_S=
PI_NAME, xspi);
> diff --git a/include/linux/spi/xilinx_spi.h b/include/linux/spi/xilinx_sp=
i.h
> index 06df0ab..a705ad8 100644
> --- a/include/linux/spi/xilinx_spi.h
> +++ b/include/linux/spi/xilinx_spi.h
> @@ -4,11 +4,13 @@
> =A0/**
> =A0* struct xspi_platform_data - Platform data of the Xilinx SPI driver
> =A0* @num_chipselect: =A0 =A0Number of chip select by the IP
> + * @little_endian =A0 =A0 =A0If registers should be accessed little endi=
an or not
> =A0* @devices: =A0 =A0 =A0 =A0 =A0 Devices to add when the driver is prob=
ed.
> =A0* @num_devices: =A0 =A0 =A0 Number of devices in the devices array.
> =A0*/
> =A0struct xspi_platform_data {
> =A0 =A0 =A0 =A0u16 num_chipselect;
> + =A0 =A0 =A0 bool little_endian;
> =A0 =A0 =A0 =A0struct spi_board_info *devices;
> =A0 =A0 =A0 =A0u8 num_devices;
> =A0};
>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH v2 4/4] xilinx_spi: add a platform driver using the xilinx_spi common module.
From: Grant Likely @ 2009-11-12 15:03 UTC (permalink / raw)
To: Richard Röjfors
Cc: spi-devel-general, Andrew Morton, dbrownell, John Linn,
linuxppc-dev
In-Reply-To: <4AFC1B99.1090004@mocean-labs.com>
On Thu, Nov 12, 2009 at 7:28 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>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* RE: Disable Caching for mmap() address
From: Jonathan Haws @ 2009-11-12 15:26 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1258018363.2140.333.camel@pasglop>
> On Mon, 2009-11-09 at 16:21 -0700, Jonathan Haws wrote:
> > All,
> >
> > I would like to disable caching for an address that was returned
> from a call to mmap(). I am using this address for DMA operations
> in user space and want to make sure that the data cache is turned
> off for that buffer.
> >
> > The way this works is the driver simply takes an address I provide
> and begins a DMA operation to that location in RAM (I have ensured
> that this is a physical address I am passing already). When the DMA
> is complete, an interrupt fires and the ISR gives a semaphore that
> the user space application is pending on (RT_SEM from Xenomai). I
> have tried simply calling a cache invalidate routine in the ISR
> before I give the semaphore, but the kernel crashes when I try to
> call that routine - my guess it because the kernel does not have
> direct access to that location in memory (only my application does,
> according to the MMU).
> >
> > Anyway, all I want to do is make sure that the buffer is never
> stored in the cache and that I always fetch it from RAM. How can I
> specify that using mmap() on the /dev/mem device, or is there a
> better way to accomplish this?
>=20
> There is no "proper" way to do this, in large part because it's not
> always legal to map memory non-cached for various reasons I don't
> have time to explain right now...
>=20
> You may be able to get it working though but using a specific driver
> with an mmap function that tweaks the attributes or using mmap
> of /dev/mem after opening it with O_SYNC (off the top of my mind)
>=20
> But it's a bit fishy as the kernel has a cacheable mapping of most
> of memory and so you may end up with cache aliases...
Thanks for the response, Ben. I am hoping that by passing a mem=3D argumen=
t to the kernel at boot time, the memory that I am setting aside for my DMA=
will be kept hidden from the kernel and the MMU. I am then mapping that m=
emory in user space with mmap() on /dev/mem and that descriptor is being op=
ened with the O_SYNC flag. I just wanted to make sure I was covering all m=
y bases.
Thanks,
Jonathan
^ permalink raw reply
* Re: [PATCH v2 1/4] xilinx_spi: Split into of driver and generic part.
From: Richard Röjfors @ 2009-11-12 15:36 UTC (permalink / raw)
To: Grant Likely
Cc: spi-devel-general, Andrew Morton, dbrownell, John Linn,
linuxppc-dev
In-Reply-To: <fa686aa40911120656x3b94b9c7tfabd6e7956ed41fb@mail.gmail.com>
Hi,
There are some proposed updates from you which are inherited from the old code
I think it's better that you (xilinx?) take responsibility for that part, I
really don't want to maintain others' code which I even can't compile.
There was a type error I will fix.
Comments below...
//Richard
Grant Likely wrote:
> On Thu, Nov 12, 2009 at 7:26 AM, Richard Röjfors
> <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.
>>
>> Signed-off-by: Richard Röjfors <richard.rojfors@mocean-labs.com>
>> ---
>> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
>> index 4b6f7cb..e60b264 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 (XILINX_VIRTEX || MICROBLAZE) && EXPERIMENTAL
>> + depends on EXPERIMENTAL
>> select SPI_BITBANG
>> help
>> This exposes the SPI controller IP from the Xilinx EDK.
>> @@ -244,6 +244,12 @@ config SPI_XILINX
>> See the "OPB Serial Peripheral Interface (SPI) (v1.00e)"
>> Product Specification document (DS464) for hardware details.
>>
>> +config SPI_XILINX_OF
>> + tristate "Xilinx SPI controller OF device"
>> + depends on SPI_XILINX && (XILINX_VIRTEX || MICROBLAZE)
>> + help
>> + This is the OF 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 21a1182..97dee8f 100644
>> --- a/drivers/spi/Makefile
>> +++ b/drivers/spi/Makefile
>> @@ -31,6 +31,7 @@ obj-$(CONFIG_SPI_S3C24XX_GPIO) += spi_s3c24xx_gpio.o
>> 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_SH_SCI) += spi_sh_sci.o
>> obj-$(CONFIG_SPI_STMP3XXX) += spi_stmp.o
>> # ... add above this line ...
>> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
>> index 46b8c5c..5761a4c 100644
>> --- a/drivers/spi/xilinx_spi.c
>> +++ b/drivers/spi/xilinx_spi.c
>> @@ -14,16 +14,14 @@
>> #include <linux/module.h>
>> #include <linux/init.h>
>> #include <linux/interrupt.h>
>> -#include <linux/platform_device.h>
>> -
>> -#include <linux/of_platform.h>
>> -#include <linux/of_device.h>
>> -#include <linux/of_spi.h>
>>
>> #include <linux/spi/spi.h>
>> #include <linux/spi/spi_bitbang.h>
>> #include <linux/io.h>
>>
>> +#include "xilinx_spi.h"
>> +#include <linux/spi/xilinx_spi.h>
>> +
>> #define XILINX_SPI_NAME "xilinx_spi"
>>
>> /* Register definitions as per "OPB Serial Peripheral Interface (SPI) (v1.00e)
>> @@ -78,7 +76,7 @@ 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;
>> @@ -283,40 +281,22 @@ static irqreturn_t xilinx_spi_irq(int irq, void *dev_id)
>> return IRQ_HANDLED;
>> }
>>
>> -static int __init xilinx_spi_of_probe(struct of_device *ofdev,
>> - const struct of_device_id *match)
>> +struct spi_master *xilinx_spi_init(struct device *dev, struct resource *mem,
>> + u32 irq, s16 bus_num)
>> {
>> struct spi_master *master;
>> struct xilinx_spi *xspi;
>> - struct resource r_irq_struct;
>> - struct resource r_mem_struct;
>> -
>> - struct resource *r_irq = &r_irq_struct;
>> - struct resource *r_mem = &r_mem_struct;
>> - int rc = 0;
>> - const u32 *prop;
>> - int len;
>> -
>> - /* Get resources(memory, IRQ) associated with the device */
>> - master = spi_alloc_master(&ofdev->dev, sizeof(struct xilinx_spi));
>> + struct xspi_platform_data *pdata = dev->platform_data;
>> + int ret;
>>
>> - if (master == NULL) {
>> - return -ENOMEM;
>> - }
>> -
>> - dev_set_drvdata(&ofdev->dev, master);
>> -
>> - rc = of_address_to_resource(ofdev->node, 0, r_mem);
>> - if (rc) {
>> - dev_warn(&ofdev->dev, "invalid address\n");
>> - goto put_master;
>> + if (!pdata) {
>> + dev_err(dev, "No platform data attached\n");
>> + return NULL;
>> }
>>
>> - rc = of_irq_to_resource(ofdev->node, 0, r_irq);
>> - if (rc == NO_IRQ) {
>> - dev_warn(&ofdev->dev, "no IRQ found\n");
>> - goto put_master;
>> - }
>> + master = spi_alloc_master(dev, sizeof(struct xilinx_spi));
>> + if (!master)
>> + return NULL;
>>
>> /* the spi->mode bits understood by this driver: */
>> master->mode_bits = SPI_CPOL | SPI_CPHA;
>> @@ -329,128 +309,67 @@ static int __init xilinx_spi_of_probe(struct of_device *ofdev,
>> xspi->bitbang.master->setup = xilinx_spi_setup;
>> init_completion(&xspi->done);
>>
>> - xspi->irq = r_irq->start;
>> -
>> - if (!request_mem_region(r_mem->start,
>> - r_mem->end - r_mem->start + 1, XILINX_SPI_NAME)) {
>> - rc = -ENXIO;
>> - dev_warn(&ofdev->dev, "memory request failure\n");
>> + if (!request_mem_region(mem->start, resource_size(mem),
>> + XILINX_SPI_NAME))
>> goto put_master;
>> - }
>>
>> - xspi->regs = ioremap(r_mem->start, r_mem->end - r_mem->start + 1);
>> + xspi->regs = ioremap(mem->start, resource_size(mem));
>> if (xspi->regs == NULL) {
>> - rc = -ENOMEM;
>> - dev_warn(&ofdev->dev, "ioremap failure\n");
>> - goto release_mem;
>> + dev_warn(dev, "ioremap failure\n");
>> + goto map_failed;
>> }
>> - xspi->irq = r_irq->start;
>>
>> - /* dynamic bus assignment */
>> - master->bus_num = -1;
>> + master->bus_num = bus_num;
>> + master->num_chipselect = pdata->num_chipselect;
>>
>> - /* number of slave select bits is required */
>> - prop = of_get_property(ofdev->node, "xlnx,num-ss-bits", &len);
>> - if (!prop || len < sizeof(*prop)) {
>> - dev_warn(&ofdev->dev, "no 'xlnx,num-ss-bits' property\n");
>> - goto unmap_io;
>> - }
>> - master->num_chipselect = *prop;
>> + xspi->mem = *mem;
>> + xspi->irq = irq;
>>
>> /* SPI controller initializations */
>> xspi_init_hw(xspi->regs);
>>
>> /* Register for SPI Interrupt */
>> - rc = request_irq(xspi->irq, xilinx_spi_irq, 0, XILINX_SPI_NAME, xspi);
>> - if (rc != 0) {
>> - dev_warn(&ofdev->dev, "irq request failure: %d\n", xspi->irq);
>> + ret = request_irq(xspi->irq, xilinx_spi_irq, 0, XILINX_SPI_NAME, xspi);
>> + if (ret)
>> goto unmap_io;
>> - }
>>
>> - rc = spi_bitbang_start(&xspi->bitbang);
>> - if (rc != 0) {
>> - dev_err(&ofdev->dev, "spi_bitbang_start FAILED\n");
>> + ret = spi_bitbang_start(&xspi->bitbang);
>> + if (ret) {
>> + dev_err(dev, "spi_bitbang_start FAILED\n");
>> goto free_irq;
>> }
>>
>> - dev_info(&ofdev->dev, "at 0x%08X mapped to 0x%08X, irq=%d\n",
>> - (unsigned int)r_mem->start, (u32)xspi->regs, xspi->irq);
>> -
>> - /* Add any subnodes on the SPI bus */
>> - of_register_spi_devices(master, ofdev->node);
>> -
>> - return rc;
>> + dev_info(dev, "at 0x%08X mapped to 0x%08X, irq=%d\n",
>> + (u32)mem->start, (u32)xspi->regs, xspi->irq);
>> + return master;
>>
>> free_irq:
>> free_irq(xspi->irq, xspi);
>> unmap_io:
>> iounmap(xspi->regs);
>> -release_mem:
>> - release_mem_region(r_mem->start, resource_size(r_mem));
>> +map_failed:
>> + release_mem_region(mem->start, resource_size(mem));
>> put_master:
>> spi_master_put(master);
>> - return rc;
>> + return NULL;
>> }
>> +EXPORT_SYMBOL(xilinx_spi_init);
>>
>> -static int __devexit xilinx_spi_remove(struct of_device *ofdev)
>> +void xilinx_spi_deinit(struct spi_master *master)
>> {
>> struct xilinx_spi *xspi;
>> - struct spi_master *master;
>> - struct resource r_mem;
>>
>> - master = platform_get_drvdata(ofdev);
>> xspi = spi_master_get_devdata(master);
>>
>> spi_bitbang_stop(&xspi->bitbang);
>> free_irq(xspi->irq, xspi);
>> iounmap(xspi->regs);
>> - if (!of_address_to_resource(ofdev->node, 0, &r_mem))
>> - release_mem_region(r_mem.start, resource_size(&r_mem));
>> - dev_set_drvdata(&ofdev->dev, 0);
>> - spi_master_put(xspi->bitbang.master);
>> -
>> - return 0;
>> -}
>> -
>> -/* work with hotplug and coldplug */
>> -MODULE_ALIAS("platform:" XILINX_SPI_NAME);
>> -
>> -static int __exit xilinx_spi_of_remove(struct of_device *op)
>> -{
>> - return xilinx_spi_remove(op);
>> -}
>>
>> -static struct of_device_id xilinx_spi_of_match[] = {
>> - { .compatible = "xlnx,xps-spi-2.00.a", },
>> - { .compatible = "xlnx,xps-spi-2.00.b", },
>> - {}
>> -};
>> -
>> -MODULE_DEVICE_TABLE(of, xilinx_spi_of_match);
>> -
>> -static struct of_platform_driver xilinx_spi_of_driver = {
>> - .owner = THIS_MODULE,
>> - .name = "xilinx-xps-spi",
>> - .match_table = xilinx_spi_of_match,
>> - .probe = xilinx_spi_of_probe,
>> - .remove = __exit_p(xilinx_spi_of_remove),
>> - .driver = {
>> - .name = "xilinx-xps-spi",
>> - .owner = THIS_MODULE,
>> - },
>> -};
>> -
>> -static int __init xilinx_spi_init(void)
>> -{
>> - return of_register_platform_driver(&xilinx_spi_of_driver);
>> + release_mem_region(xspi->mem.start, resource_size(&xspi->mem));
>> + spi_master_put(xspi->bitbang.master);
>> }
>> -module_init(xilinx_spi_init);
>> +EXPORT_SYMBOL(xilinx_spi_deinit);
>>
>> -static void __exit xilinx_spi_exit(void)
>> -{
>> - of_unregister_platform_driver(&xilinx_spi_of_driver);
>> -}
>> -module_exit(xilinx_spi_exit);
>> MODULE_AUTHOR("MontaVista Software, Inc. <source@mvista.com>");
>> MODULE_DESCRIPTION("Xilinx SPI driver");
>> MODULE_LICENSE("GPL");
>> diff --git a/drivers/spi/xilinx_spi.h b/drivers/spi/xilinx_spi.h
>> new file mode 100644
>> index 0000000..d211acc
>> --- /dev/null
>> +++ b/drivers/spi/xilinx_spi.h
>> @@ -0,0 +1,32 @@
>> +/*
>> + * 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. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>> + */
>> +
>> +#ifndef _XILINX_SPI_H_
>> +#define _XILINX_SPI_H_
>> +
>> +#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,
>> + u32 irq, s16 bus_num);
>> +
>> +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..8c3fb54
>> --- /dev/null
>> +++ b/drivers/spi/xilinx_spi_of.c
>> @@ -0,0 +1,136 @@
>> +/*
>> + * Xilinx SPI OF device driver
>> + *
>> + * 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. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>> + */
>> +
>> +/* Supports:
>> + * Xilinx SPI devices as OF devices
>> + *
>> + * Inspired by xilinx_spi.c, 2002-2007 (c) MontaVista Software, Inc.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +
>> +#include <linux/of_platform.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_spi.h>
>> +
>> +#include <linux/spi/xilinx_spi.h>
>> +#include "xilinx_spi.h"
>> +
>> +
>> +static int __init xilinx_spi_of_probe(struct of_device *ofdev,
>> + const struct of_device_id *match)
>> +{
>> + struct resource r_irq_struct;
>> + struct resource r_mem_struct;
>> + struct spi_master *master;
>> +
>> + struct resource *r_irq = &r_irq_struct;
>> + struct resource *r_mem = &r_mem_struct;
>
> This r_{irq,mem}_struct/*r_{irq,mem} construct is really weird (I do
> understand this is just copied from the old code). r_*_struct can
> just be dropped and reference the structures with &r_irq and &_mem.
This is just the old code from montavista, I can't update their code,
I don't have the hardware or even a cross compiler for power pc.
And even less a proper kernel config for ppc.
>
>> + int rc = 0;
>> + const u32 *prop;
>> + int len;
>> +
>> + rc = of_address_to_resource(ofdev->node, 0, r_mem);
>> + if (rc) {
>> + dev_warn(&ofdev->dev, "invalid address\n");
>> + return rc;
>> + }
>> +
>> + rc = of_irq_to_resource(ofdev->node, 0, r_irq);
>> + if (rc == NO_IRQ) {
>> + dev_warn(&ofdev->dev, "no IRQ found\n");
>> + return -ENODEV;
>> + }
>> +
>> + if (!ofdev->dev.platform_data) {
>> + ofdev->dev.platform_data =
>> + kzalloc(sizeof(struct xspi_platform_data), GFP_KERNEL);
>> + if (!ofdev->dev.platform_data)
>> + return -ENOMEM;
>> + }
>
> Minor memory leak. Anything alloced in the probe path should also be
> freed in the remove path. It's not going to spiral out of control or
> anything, but it is important to be strict about such things. Drop
> the outer if{} block here and kfree platform_data on remove.
Yeah I know I though about it, the problem is if a platform_data is already
attached, then we brutally free it. That's why I was lazy. It will only "leak"
once...
>
>> +
>> + /* number of slave select bits is required */
>> + prop = of_get_property(ofdev->node, "xlnx,num-ss-bits", &len);
>> + if (!prop || len < sizeof(*prop)) {
>> + dev_warn(&ofdev->dev, "no 'xlnx,num-ss-bits' property\n");
>> + return -EINVAL;
>> + }
>> + ofdev->dev.platform_data->num_chipselect = *prop;
>
> Have you compile tested this? platform_data is a void*, the
> dereference will not work. I know you don't have hardware; but
> getting the needed cross compiler is easy.
Damn this is an error. No I don't have a cross compiler or proper kernel config.
I will have to update.
>
>> + master = xilinx_spi_init(&ofdev->dev, r_mem, r_irq->start, -1);
>> + if (!master)
>> + return -ENODEV;
>> +
>> + dev_set_drvdata(&ofdev->dev, master);
>> +
>> + /* Add any subnodes on the SPI bus */
>> + of_register_spi_devices(master, ofdev->node);
>> +
>> + return 0;
>> +}
>> +
>> +static int __devexit xilinx_spi_remove(struct of_device *ofdev)
>> +{
>> + xilinx_spi_deinit(dev_get_drvdata(&ofdev->dev));
>> + dev_set_drvdata(&ofdev->dev, 0);
>> + return 0;
>> +}
>> +
>> +static int __exit xilinx_spi_of_remove(struct of_device *op)
>> +{
>> + return xilinx_spi_remove(op);
>> +}
>> +
>> +static struct of_device_id xilinx_spi_of_match[] = {
>> + { .compatible = "xlnx,xps-spi-2.00.a", },
>> + { .compatible = "xlnx,xps-spi-2.00.b", },
>> + {}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, xilinx_spi_of_match);
>> +
>> +static struct of_platform_driver xilinx_spi_of_driver = {
>> + .owner = THIS_MODULE,
>> + .name = "xilinx-xps-spi",
>
> You can actually drop the above two lines. They aren't needed.
Ok, this is old monta vista code. I really don't want to maintain
power pc code.
>
>> + .match_table = xilinx_spi_of_match,
>> + .probe = xilinx_spi_of_probe,
>> + .remove = __exit_p(xilinx_spi_of_remove),
>> + .driver = {
>> + .name = "xilinx-xps-spi",
>> + .owner = THIS_MODULE,
>> + },
>> +};
>
^ permalink raw reply
* problem with POSIX timers
From: Logan.Ratner @ 2009-11-12 16:44 UTC (permalink / raw)
To: linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]
I'm on a project where I recently switched from BSD style timers
(setitimer/getitimer) to POSIX style timers
(timer_settime/timer_gettime) so I can use the timer_getoverrun
function. This works fine, except that now on my embedded PPC platform
changing the system time with either settimeofday or clock_settime has
an effect on relative timers, which according to POSIX shouldn't happen.
The same code works fine on the x86 platforms I've tested on; I only see
the problem on my PPC machine.
I've also tried using CLOCK_MONOTONIC instead of CLOCK_REALTIME, but
that does not seem to be supported on my platform either.
I'm using:
An MPC8270 processor
A 2.6.17.6 kernel
librt ver. 2.3.5
libc ver. 2.3.5
g++ 4.0 targeting powerpc-linux
Is this a known problem? Is there a patch or a later kernel that would
correct this problem?
Also, is this the right place to be asking this question, and if not
could anyone suggest a better forum?
Any help or advice would be greatly appreciated.
Thanks in advance,
Logan Ratner | Software Engineer | Rosemount Analytical
Emerson Process Management | 5650 Brittmoore Rd | Houston | TX | 77041 |
USA
T +1 713 839 9656 | F +1 713 827 3807
Logan.Ratner@Emerson.com
[-- Attachment #2: Type: text/html, Size: 6575 bytes --]
^ permalink raw reply
* RE: [PATCH v2 1/4] xilinx_spi: Split into of driver and generic part.
From: John Linn @ 2009-11-12 17:17 UTC (permalink / raw)
To: Grant Likely, Richard Röjfors
Cc: spi-devel-general, Andrew Morton, dbrownell, linuxppc-dev
In-Reply-To: <fa686aa40911120656x3b94b9c7tfabd6e7956ed41fb@mail.gmail.com>
> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On Behalf Of Gra=
nt Likely
> Sent: Thursday, November 12, 2009 7:56 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: [PATCH v2 1/4] xilinx_spi: Split into of driver and generic =
part.
> =
> On Thu, Nov 12, 2009 at 7:26 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.
> >
> > Signed-off-by: Richard R=F6jfors <richard.rojfors@mocean-labs.com>
> > ---
<snip>
> > +
> > + =A0 =A0 =A0 if (!ofdev->dev.platform_data) {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 ofdev->dev.platform_data =3D
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 kzalloc(sizeof(struct xsp=
i_platform_data), GFP_KERNEL);
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!ofdev->dev.platform_data)
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENOMEM;
> > + =A0 =A0 =A0 }
> =
> Minor memory leak. Anything alloced in the probe path should also be
> freed in the remove path. It's not going to spiral out of control or
> anything, but it is important to be strict about such things. Drop
> the outer if{} block here and kfree platform_data on remove.
> =
> > +
> > + =A0 =A0 =A0 /* number of slave select bits is required */
> > + =A0 =A0 =A0 prop =3D of_get_property(ofdev->node, "xlnx,num-ss-bits",=
&len);
> > + =A0 =A0 =A0 if (!prop || len < sizeof(*prop)) {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_warn(&ofdev->dev, "no 'xlnx,num-ss-bi=
ts' property\n");
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EINVAL;
> > + =A0 =A0 =A0 }
> > + =A0 =A0 =A0 ofdev->dev.platform_data->num_chipselect =3D *prop;
> =
> Have you compile tested this? platform_data is a void*, the
> dereference will not work. I know you don't have hardware; but
> getting the needed cross compiler is easy.
Here's the fixes I did to make it compile. On to testing :)
Thanks,
John
diff --git a/drivers/spi/xilinx_spi_of.c b/drivers/spi/xilinx_spi_of.c
index 13e1591..2dea9f3 100644
--- a/drivers/spi/xilinx_spi_of.c
+++ b/drivers/spi/xilinx_spi_of.c
@@ -39,6 +39,7 @@
static int __init xilinx_spi_of_probe(struct of_device *ofdev,
const struct of_device_id *match)
{
+ struct xspi_platform_data *pdata =3D ofdev->dev.platform_data;
struct resource r_irq_struct;
struct resource r_mem_struct;
struct spi_master *master;
@@ -74,8 +75,8 @@ static int __init xilinx_spi_of_probe(struct of_device *o=
fdev,
dev_warn(&ofdev->dev, "no 'xlnx,num-ss-bits' property\n");
return -EINVAL;
}
- ofdev->dev.platform_data->num_chipselect =3D *prop;
- ofdev->dev.platform_data->bits_per_word =3D 8;
+ pdata->num_chipselect =3D *prop;
+ pdata->bits_per_word =3D 8;
master =3D xilinx_spi_init(&ofdev->dev, r_mem, r_irq->start, -1);
if (!master)
return -ENODEV;
> =
> > + =A0 =A0 =A0 master =3D xilinx_spi_init(&ofdev->dev, r_mem, r_irq->sta=
rt, -1);
> > + =A0 =A0 =A0 if (!master)
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
> > +
> > + =A0 =A0 =A0 dev_set_drvdata(&ofdev->dev, master);
> > +
> > + =A0 =A0 =A0 /* Add any subnodes on the SPI bus */
> > + =A0 =A0 =A0 of_register_spi_devices(master, ofdev->node);
> > +
> > + =A0 =A0 =A0 return 0;
> > +}
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 related
* Re: [PATCH v2 1/4] xilinx_spi: Split into of driver and generic part.
From: Richard Röjfors @ 2009-11-12 17:22 UTC (permalink / raw)
To: John Linn; +Cc: spi-devel-general, Andrew Morton, dbrownell, linuxppc-dev
In-Reply-To: <38eacaec-f0d3-436c-b461-40b5d795cdd9@VA3EHSMHS009.ehs.local>
On 11/12/09 6:17 PM, John Linn wrote:
>> Have you compile tested this? platform_data is a void*, the
>> dereference will not work. I know you don't have hardware; but
>> getting the needed cross compiler is easy.
>
> Here's the fixes I did to make it compile. On to testing :)
Great! I tested this against a max7301 GPIO expander.
--Richard
^ permalink raw reply
* [PATCHv2 0/3] mpc52xx/wdt: re-enable the MPC5200 WDT
From: Albrecht Dreß @ 2009-11-12 18:39 UTC (permalink / raw)
To: Likely, Grant, Linux PPC Development, Devicetree Discussions,
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.
Compared to the first version of the patch, the basic change is a full merg=
e
of the WDT code into the GPT implementation in arch/powerpc/platforms/52xx.
It makes the old WDT implementation file obsolete. As a side effect, it is
not possible any more to build the 52xx WDT support as module which shouldn=
't
be a big issue, though.
Cheers, Albrecht.
---
Documentation/powerpc/dts-bindings/fsl/mpc5200.txt | 17 +-
arch/powerpc/include/asm/mpc52xx.h | 5 +-
arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 328 ++++++++++++++++=
++--
drivers/watchdog/Kconfig | 4 +-
drivers/watchdog/Makefile | 1 -
5 files changed, 331 insertions(+), 24 deletions(-)
^ permalink raw reply
* [PATCHv2 1/3] mpc52xx/wdt: OF property to enable the WDT on boot
From: Albrecht Dreß @ 2009-11-12 18:43 UTC (permalink / raw)
To: Likely, Grant, Linux PPC Development, Devicetree Discussions,
Wim Van Sebroeck
Add the "fsl,wdt-on-boot" OF property as to reserve a GPT as WDT which may
be a requirement in safety-related (e.g. ISO/EN 61508) applications.
Signed-off-by: Albrecht Dre=DF <albrecht.dress@arcor.de>
---
Change against v1: rename the new property.
Documentation/powerpc/dts-bindings/fsl/mpc5200.txt | 17 ++++++++++++++++=
-
1 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt b/Documenta=
tion/powerpc/dts-bindings/fsl/mpc5200.txt
index 8447fd7..ddd5ee3 100644
--- a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
+++ b/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
@@ -103,7 +103,22 @@ 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
+ fsl,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.
+
+The watchdog will respect the CONFIG_WATCHDOG_NOWAYOUT option.
=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
* [PATCHv2 2/3] mpc52xx/wdt: merge WDT code into the GPT driver
From: Albrecht Dreß @ 2009-11-12 18:44 UTC (permalink / raw)
To: Likely, Grant, Linux PPC Development, Devicetree Discussions,
Wim Van Sebroeck
Merge the WDT code into the GPT interface.
Signed-off-by: Albrecht Dre=DF <albrecht.dress@arcor.de>
---
Change against v1: fully merge the wdt api into this file.
Note: The patch does also include the tiny GPT api changes from
<http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-November/077647.html>.
arch/powerpc/include/asm/mpc52xx.h | 5 +-
arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 328 +++++++++++++++++++++++++=
++--
2 files changed, 312 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/asm/=
mpc52xx.h
index 707ab75..b664ce7 100644
--- a/arch/powerpc/include/asm/mpc52xx.h
+++ b/arch/powerpc/include/asm/mpc52xx.h
@@ -279,9 +279,10 @@ 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,
+extern int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *gpt, u64 perio=
d,
int continuous);
-extern void mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt);
+extern u64 mpc52xx_gpt_timer_period(struct mpc52xx_gpt_priv *gpt);
+extern int mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt);
/* 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..42caecf 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
@@ -16,8 +16,14 @@
* output signals or measure input signals.
*
* This driver supports the GPIO and IRQ controller functions of the GPT
- * device. Timer functions are not yet supported, nor is the watchdog
- * timer.
+ * device. Timer functions are not yet supported.
+ *
+ * The timer gpt0 can be used as watchdog (wdt). If the wdt mode is used,
+ * this prevents the use of any gpt0 gpt function (i.e. they will fail wit=
h
+ * -EBUSY). Thus, the safety wdt function always has precedence over the =
gpt
+ * function. If the kernel has been compiled with CONFIG_WATCHDOG_NOWAYOU=
T,
+ * this means that gpt0 is locked in wdt mode until the next reboot - this
+ * may be a requirement in safety applications.
*
* To use the GPIO function, the following two properties must be added
* to the device tree node for the gpt device (typically in the .dts file
@@ -56,11 +62,14 @@
#include <linux/of_platform.h>
#include <linux/of_gpio.h>
#include <linux/kernel.h>
+#include <linux/watchdog.h>
+#include <linux/miscdevice.h>
+#include <linux/uaccess.h>
#include <asm/div64.h>
#include <asm/mpc52xx.h>
MODULE_DESCRIPTION("Freescale MPC52xx gpt driver");
-MODULE_AUTHOR("Sascha Hauer, Grant Likely");
+MODULE_AUTHOR("Sascha Hauer, Grant Likely, Albrecht Dre=DF");
MODULE_LICENSE("GPL");
/**
@@ -70,6 +79,9 @@ 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 relevant for gpt0: bit 0 (MPC52xx_GPT_CAN_WDT) indicate=
s
+ * if the gpt may be used as wdt, bit 1 (MPC52xx_GPT_IS_WDT) indicates
+ * if the timer is actively used as wdt which blocks gpt functions
*/
struct mpc52xx_gpt_priv {
struct list_head list; /* List of all GPT devices */
@@ -78,6 +90,7 @@ struct mpc52xx_gpt_priv {
spinlock_t lock;
struct irq_host *irqhost;
u32 ipb_freq;
+ u8 wdt_mode;
#if defined(CONFIG_GPIOLIB)
struct of_gpio_chip of_gc;
@@ -101,14 +114,21 @@ 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)
#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)
+#define MPC52xx_GPT_MODE_WDT_PING (0xa5)
+
#define MPC52xx_GPT_STATUS_IRQMASK (0x000f)
+#define MPC52xx_GPT_CAN_WDT (1 << 0)
+#define MPC52xx_GPT_IS_WDT (1 << 1)
+
+
/* ---------------------------------------------------------------------
* Cascaded interrupt controller hooks
*/
@@ -375,16 +395,8 @@ struct mpc52xx_gpt_priv *mpc52xx_gpt_from_irq(int irq)
}
EXPORT_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,
- int continuous)
+static int mpc52xx_gpt_do_start(struct mpc52xx_gpt_priv *gpt, u64 period,
+ int continuous, int as_wdt)
{
u32 clear, set;
u64 clocks;
@@ -393,15 +405,19 @@ int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *=
gpt, int period,
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)
+ if (as_wdt) {
+ clear |=3D MPC52xx_GPT_MODE_IRQ_EN;
+ set |=3D MPC52xx_GPT_MODE_WDT_EN;
+ } else if (continuous)
set |=3D MPC52xx_GPT_MODE_CONTINUOUS;
/* 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 */
+ * frequency is in Hz. The maximum timeout @33MHz IPB clock is ~130
+ * seconds*/
+ clocks =3D period * (u64)gpt->ipb_freq;
+ do_div(clocks, 1000000000ULL); /* Scale it down to ns range */
/* This device cannot handle a clock count greater than 32 bits */
if (clocks > 0xffffffff)
@@ -427,22 +443,279 @@ int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv =
*gpt, int period,
return -EINVAL;
}
- /* Set and enable the timer */
+ /* Set and enable the timer, reject an attempt to use a wdt as gpt */
spin_lock_irqsave(&gpt->lock, flags);
+ if (as_wdt)
+ gpt->wdt_mode |=3D MPC52xx_GPT_IS_WDT;
+ else if ((gpt->wdt_mode & MPC52xx_GPT_IS_WDT) !=3D 0) {
+ spin_unlock_irqrestore(&gpt->lock, flags);
+ return -EBUSY;
+ }
out_be32(&gpt->regs->count, prescale << 16 | clocks);
clrsetbits_be32(&gpt->regs->mode, clear, set);
spin_unlock_irqrestore(&gpt->lock, flags);
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)
+{
+ return mpc52xx_gpt_do_start(gpt, period, continuous, 0);
+}
EXPORT_SYMBOL(mpc52xx_gpt_start_timer);
-void mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt)
+/**
+ * mpc52xx_gpt_stop_timer - Stop a gpt
+ * @gpt: Pointer to gpt private data structure
+ *
+ * Returns an error if attempting to stop a wdt
+ */
+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 ((gpt->wdt_mode & MPC52xx_GPT_IS_WDT) !=3D 0) {
+ spin_unlock_irqrestore(&gpt->lock, flags);
+ return -EBUSY;
+ }
+
clrbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_COUNTER_ENABLE);
+ spin_unlock_irqrestore(&gpt->lock, flags);
+ return 0;
}
EXPORT_SYMBOL(mpc52xx_gpt_stop_timer);
+/**
+ * mpc52xx_gpt_timer_period - Read the timer period
+ * @gpt: Pointer to gpt private data structure
+ *
+ * Returns the timer period in ns
+ */
+u64 mpc52xx_gpt_timer_period(struct mpc52xx_gpt_priv *gpt)
+{
+ u64 period;
+ u64 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 * prescale * 1000000000ULL;
+ do_div(period, (u64)gpt->ipb_freq);
+ return period;
+}
+EXPORT_SYMBOL(mpc52xx_gpt_timer_period);
+
+#if defined(CONFIG_MPC5200_WDT)
+/***********************************************************************
+ * Watchdog API for gpt0
+ */
+
+#define WDT_IDENTITY "mpc52xx watchdog on GPT0"
+
+/* wdt_is_active stores wether or not the /dev/watchdog device is opened *=
/
+static unsigned long wdt_is_active;
+
+/* wdt-capable gpt */
+static struct mpc52xx_gpt_priv *mpc52xx_gpt_wdt;
+
+/* low-level wdt functions */
+static inline void 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);
+}
+
+/* wdt misc device api */
+static ssize_t mpc52xx_wdt_write(struct file *file, const char __user *dat=
a,
+ size_t len, loff_t *ppos)
+{
+ struct mpc52xx_gpt_priv *gpt_wdt =3D file->private_data;
+ mpc52xx_gpt_wdt_ping(gpt_wdt);
+ return 0;
+}
+
+static struct watchdog_info mpc5200_wdt_info =3D {
+ .options =3D WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
+ .identity =3D WDT_IDENTITY,
+};
+
+static long mpc52xx_wdt_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
+{
+ struct mpc52xx_gpt_priv *gpt_wdt =3D file->private_data;
+ int __user *data =3D (int __user *)arg;
+ int timeout;
+ u64 real_timeout;
+ int ret =3D 0;
+
+ switch (cmd) {
+ case WDIOC_GETSUPPORT:
+ ret =3D copy_to_user(data, &mpc5200_wdt_info,
+ sizeof(mpc5200_wdt_info));
+ if (ret)
+ ret =3D -EFAULT;
+ break;
+
+ case WDIOC_GETSTATUS:
+ case WDIOC_GETBOOTSTATUS:
+ ret =3D put_user(0, data);
+ break;
+
+ case WDIOC_KEEPALIVE:
+ mpc52xx_gpt_wdt_ping(gpt_wdt);
+ break;
+
+ case WDIOC_SETTIMEOUT:
+ ret =3D get_user(timeout, data);
+ if (ret)
+ break;
+ real_timeout =3D (u64) timeout * 1000000000ULL;
+ ret =3D mpc52xx_gpt_do_start(gpt_wdt, real_timeout, 0, 1);
+ if (ret)
+ break;
+ /* fall through and return the timeout */
+
+ case WDIOC_GETTIMEOUT:
+ /* we need to round here as to avoid e.g. the following
+ * situation:
+ * - timeout requested is 1 second;
+ * - real timeout @33MHz is 999997090ns
+ * - the int divide by 10^9 will return 0.
+ */
+ real_timeout =3D
+ mpc52xx_gpt_timer_period(gpt_wdt) + 500000000ULL;
+ do_div(real_timeout, 1000000000ULL);
+ timeout =3D (int) real_timeout;
+ ret =3D put_user(timeout, data);
+ break;
+
+ default:
+ ret =3D -ENOTTY;
+ }
+ return ret;
+}
+
+static int mpc52xx_wdt_open(struct inode *inode, struct file *file)
+{
+ int ret;
+
+ /* sanity check */
+ if (!mpc52xx_gpt_wdt)
+ return -ENODEV;
+
+ /* /dev/watchdog can only be opened once */
+ if (test_and_set_bit(0, &wdt_is_active))
+ return -EBUSY;
+
+ /* Set and activate the watchdog with 30 seconds timeout */
+ ret =3D mpc52xx_gpt_do_start(mpc52xx_gpt_wdt, 30ULL * 1000000000ULL,
+ 0, 1);
+ if (ret) {
+ clear_bit(0, &wdt_is_active);
+ return ret;
+ }
+
+ file->private_data =3D mpc52xx_gpt_wdt;
+ return nonseekable_open(inode, file);
+}
+
+static int mpc52xx_wdt_release(struct inode *inode, struct file *file)
+{
+ /* note: releasing the wdt in NOWAYOUT-mode does not stop it */
+#if !defined(CONFIG_WATCHDOG_NOWAYOUT)
+ struct mpc52xx_gpt_priv *gpt_wdt =3D file->private_data;
+ 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
+ clear_bit(0, &wdt_is_active);
+ return 0;
+}
+
+
+static const struct file_operations mpc52xx_wdt_fops =3D {
+ .owner =3D THIS_MODULE,
+ .llseek =3D no_llseek,
+ .write =3D mpc52xx_wdt_write,
+ .unlocked_ioctl =3D mpc52xx_wdt_ioctl,
+ .open =3D mpc52xx_wdt_open,
+ .release =3D mpc52xx_wdt_release,
+};
+
+static struct miscdevice mpc52xx_wdt_miscdev =3D {
+ .minor =3D WATCHDOG_MINOR,
+ .name =3D "watchdog",
+ .fops =3D &mpc52xx_wdt_fops,
+};
+
+static int __devinit mpc52xx_gpt_wdt_init(void)
+{
+ int err;
+
+ /* try to register the watchdog misc device */
+ err =3D misc_register(&mpc52xx_wdt_miscdev);
+ if (err)
+ pr_err("%s: cannot register watchdog device\n", WDT_IDENTITY);
+ else
+ pr_info("%s: watchdog device registered\n", WDT_IDENTITY);
+ return err;
+}
+
+static int mpc52xx_gpt_wdt_setup(struct mpc52xx_gpt_priv *gpt,
+ const u32 *period)
+{
+ u64 real_timeout;
+
+ /* remember the gpt for the wdt operation */
+ mpc52xx_gpt_wdt =3D gpt;
+
+ /* configure the wdt if the device tree contained a timeout */
+ if (!period || *period =3D=3D 0)
+ return 0;
+
+ real_timeout =3D (u64) *period * 1000000000ULL;
+ if (mpc52xx_gpt_do_start(gpt, real_timeout, 0, 1))
+ dev_warn(gpt->dev, "starting as wdt failed\n");
+ else
+ dev_info(gpt->dev, "watchdog set to %us timeout\n", *period);
+ return 0;
+}
+
+#else
+
+static int __devinit mpc52xx_gpt_wdt_init(void)
+{
+ return 0;
+}
+
+#define mpc52xx_gpt_wdt_setup(x, y) (0)
+
+#endif /* CONFIG_MPC5200_WDT */
+
/* ---------------------------------------------------------------------
* of_platform bus binding code
*/
@@ -473,6 +746,22 @@ static int __devinit mpc52xx_gpt_probe(struct of_devic=
e *ofdev,
list_add(&gpt->list, &mpc52xx_gpt_list);
mutex_unlock(&mpc52xx_gpt_list_mutex);
+ /* 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;
+ on_boot_wdt =3D of_get_property(ofdev->node, "fsl,wdt-on-boot",
+ NULL);
+ if (on_boot_wdt) {
+ dev_info(gpt->dev, "used as watchdog\n");
+ gpt->wdt_mode |=3D MPC52xx_GPT_IS_WDT;
+ } else
+ dev_info(gpt->dev, "can function as watchdog\n");
+ mpc52xx_gpt_wdt_setup(gpt, on_boot_wdt);
+ }
+
return 0;
}
@@ -507,3 +796,4 @@ static int __init mpc52xx_gpt_init(void)
/* Make sure GPIOs and IRQs get set up before anyone tries to use them */
subsys_initcall(mpc52xx_gpt_init);
+device_initcall(mpc52xx_gpt_wdt_init);
^ permalink raw reply related
* [PATCHv2 3/3] mpc52xx/wdt: remove obsolete old WDT implementation
From: Albrecht Dreß @ 2009-11-12 18:45 UTC (permalink / raw)
To: Likely, Grant, Linux PPC Development, Devicetree Discussions,
Wim Van Sebroeck
Remove the old WDT implementation.
Signed-off-by: Albrecht Dre=DF <albrecht.dress@arcor.de>
---
Change against v1: WDT stuff now fully merged into the file
arch/powerpc/platforms/52xx/mpc52xx_gpt.c.
Note: The file drivers/watchdog/mpc5200_wdt.c is now obsolete and should be=
removed.
drivers/watchdog/Kconfig | 4 +++-
drivers/watchdog/Makefile | 1 -
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 3711b88..d958b76 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -861,8 +861,10 @@ config GEF_WDT
Watchdog timer found in a number of GE Fanuc single board computers.
=20
config MPC5200_WDT
- tristate "MPC5200 Watchdog Timer"
+ bool "MPC52xx Watchdog Timer"
depends on PPC_MPC52xx
+ help
+ Use General Purpose Timer (GPT) 0 on the MPC5200 as Watchdog.
=20
config 8xxx_WDT
tristate "MPC8xxx Platform Watchdog Timer"
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 699199b..89c045d 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -118,7 +118,6 @@ obj-$(CONFIG_TXX9_WDT) +=3D txx9wdt.o
=20
# POWERPC Architecture
obj-$(CONFIG_GEF_WDT) +=3D gef_wdt.o
-obj-$(CONFIG_MPC5200_WDT) +=3D mpc5200_wdt.o
obj-$(CONFIG_8xxx_WDT) +=3D mpc8xxx_wdt.o
obj-$(CONFIG_MV64X60_WDT) +=3D mv64x60_wdt.o
obj-$(CONFIG_PIKA_WDT) +=3D pika_wdt.o
^ permalink raw reply related
* Re: [PATCHv2 1/3] mpc52xx/wdt: OF property to enable the WDT on boot
From: Grant Likely @ 2009-11-12 19:06 UTC (permalink / raw)
To: Albrecht Dreß
Cc: Linux PPC Development, Devicetree Discussions, Wim Van Sebroeck
In-Reply-To: <1258051427.2280.1@antares>
On Thu, Nov 12, 2009 at 11:43 AM, Albrecht Dre=DF <albrecht.dress@arcor.de>=
wrote:
> Add the "fsl,wdt-on-boot" OF property as to reserve a GPT as WDT which ma=
y
> be a requirement in safety-related (e.g. ISO/EN 61508) applications.
>
> Signed-off-by: Albrecht Dre=DF <albrecht.dress@arcor.de>
>
> ---
>
> Change against v1: rename the new property.
>
> =A0Documentation/powerpc/dts-bindings/fsl/mpc5200.txt | =A0 17 ++++++++++=
++++++-
> =A01 files changed, 16 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt b/Documen=
tation/powerpc/dts-bindings/fsl/mpc5200.txt
> index 8447fd7..ddd5ee3 100644
> --- a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
> +++ b/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
> @@ -103,7 +103,22 @@ 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 fsl,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.
> +
> +The watchdog will respect the CONFIG_WATCHDOG_NOWAYOUT option.
I think it would be better to use a device tree property to enable up
NOWAYOUT. The static config isn't multiplatform-friendly. Thoughts?
Otherwise, this looks good to me.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCHv2 2/3] mpc52xx/wdt: merge WDT code into the GPT driver
From: Grant Likely @ 2009-11-12 19:12 UTC (permalink / raw)
To: Albrecht Dreß
Cc: Linux PPC Development, Devicetree Discussions, Wim Van Sebroeck
In-Reply-To: <1258051487.2280.2@antares>
On Thu, Nov 12, 2009 at 11:44 AM, 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>
Looks good to me.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCHv2 3/3] mpc52xx/wdt: remove obsolete old WDT implementation
From: Grant Likely @ 2009-11-12 19:14 UTC (permalink / raw)
To: Albrecht Dreß
Cc: Linux PPC Development, Devicetree Discussions, Wim Van Sebroeck
In-Reply-To: <1258051520.2280.3@antares>
On Thu, Nov 12, 2009 at 11:45 AM, Albrecht Dre=DF <albrecht.dress@arcor.de>=
wrote:
> Remove the old WDT implementation.
>
> Signed-off-by: Albrecht Dre=DF <albrecht.dress@arcor.de>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
Wim, I'm picking up the other 2 patches into my tree. Do you want me
to push this patch through my tree, or do you want to pick it up?
g.
> ---
>
> Change against v1: WDT stuff now fully merged into the file
> arch/powerpc/platforms/52xx/mpc52xx_gpt.c.
>
> Note: The file drivers/watchdog/mpc5200_wdt.c is now obsolete and should =
be removed.
>
> =A0drivers/watchdog/Kconfig =A0| =A0 =A04 +++-
> =A0drivers/watchdog/Makefile | =A0 =A01 -
> =A02 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 3711b88..d958b76 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -861,8 +861,10 @@ config GEF_WDT
> =A0 =A0 =A0 =A0 =A0Watchdog timer found in a number of GE Fanuc single bo=
ard computers.
>
> =A0config MPC5200_WDT
> - =A0 =A0 =A0 tristate "MPC5200 Watchdog Timer"
> + =A0 =A0 =A0 bool "MPC52xx Watchdog Timer"
> =A0 =A0 =A0 =A0depends on PPC_MPC52xx
> + =A0 =A0 =A0 help
> + =A0 =A0 =A0 =A0 Use General Purpose Timer (GPT) 0 on the MPC5200 as Wat=
chdog.
>
> =A0config 8xxx_WDT
> =A0 =A0 =A0 =A0tristate "MPC8xxx Platform Watchdog Timer"
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 699199b..89c045d 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -118,7 +118,6 @@ obj-$(CONFIG_TXX9_WDT) +=3D txx9wdt.o
>
> =A0# POWERPC Architecture
> =A0obj-$(CONFIG_GEF_WDT) +=3D gef_wdt.o
> -obj-$(CONFIG_MPC5200_WDT) +=3D mpc5200_wdt.o
> =A0obj-$(CONFIG_8xxx_WDT) +=3D mpc8xxx_wdt.o
> =A0obj-$(CONFIG_MV64X60_WDT) +=3D mv64x60_wdt.o
> =A0obj-$(CONFIG_PIKA_WDT) +=3D pika_wdt.o
>
>
--=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: Scott Wood @ 2009-11-12 19:45 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <OFE47A9BB7.59140781-ONC125766C.00322B87-C125766C.003266AD@transmode.se>
Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 11/11/2009 16:26:53:
>> 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.
>
> Yeah, I misread the CPU15 errata so my ideas will not work.
>
> Anyhow, will you send a patch that make TLB pinning mandatory?
> After that my series can go in.
One other concern with pinning on 8xx -- could it cause problems with
uncached DMA mappings? What happens if a speculative load pulls in a
cache line in an area that's supposed to be uncached?
-Scott
^ permalink raw reply
* Re: [PATCHv2 3/3] mpc52xx/wdt: remove obsolete old WDT implementation
From: Wolfram Sang @ 2009-11-12 19:56 UTC (permalink / raw)
To: Albrecht Dreß
Cc: Linux PPC Development, Devicetree Discussions, Wim Van Sebroeck
In-Reply-To: <1258051520.2280.3@antares>
[-- Attachment #1: Type: text/plain, Size: 1966 bytes --]
On Thu, Nov 12, 2009 at 07:45:20PM +0100, Albrecht Dreß wrote:
> Remove the old WDT implementation.
>
> Signed-off-by: Albrecht Dreß <albrecht.dress@arcor.de>
> ---
>
> Change against v1: WDT stuff now fully merged into the file
> arch/powerpc/platforms/52xx/mpc52xx_gpt.c.
>
> Note: The file drivers/watchdog/mpc5200_wdt.c is now obsolete and should be removed.
Ehrm, this is possible within a patch :)
>
> drivers/watchdog/Kconfig | 4 +++-
> drivers/watchdog/Makefile | 1 -
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 3711b88..d958b76 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -861,8 +861,10 @@ config GEF_WDT
> Watchdog timer found in a number of GE Fanuc single board computers.
>
> config MPC5200_WDT
> - tristate "MPC5200 Watchdog Timer"
> + bool "MPC52xx Watchdog Timer"
> depends on PPC_MPC52xx
> + help
> + Use General Purpose Timer (GPT) 0 on the MPC5200 as Watchdog.
s/5200/52xx/ here too.
>
> config 8xxx_WDT
> tristate "MPC8xxx Platform Watchdog Timer"
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 699199b..89c045d 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -118,7 +118,6 @@ obj-$(CONFIG_TXX9_WDT) += txx9wdt.o
>
> # POWERPC Architecture
> obj-$(CONFIG_GEF_WDT) += gef_wdt.o
> -obj-$(CONFIG_MPC5200_WDT) += mpc5200_wdt.o
> obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
> obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
> obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCHv2 1/3] mpc52xx/wdt: OF property to enable the WDT on boot
From: Albrecht Dreß @ 2009-11-12 20:33 UTC (permalink / raw)
To: Grant Likely
Cc: Linux PPC Development, Devicetree Discussions, Wim Van Sebroeck
In-Reply-To: <fa686aa40911121106j1d609868vb16d6867955aa607@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]
Hi Grant!
Am 12.11.09 20:06 schrieb(en) Grant Likely:
> > +The watchdog will respect the CONFIG_WATCHDOG_NOWAYOUT option.
>
> I think it would be better to use a device tree property to enable up
> NOWAYOUT. The static config isn't multiplatform-friendly. Thoughts?
I fully agree with you that this property would fit perfectly in the device tree. However, if we now add it *only* for the 52xx, but not for other device tree-aware platforms, this might be somewhat confusing. The good thing is that it wouldn't break
anything for the 5200 as the old wdt driver didn't work anyway.
One *real* advantage of the compile-time option is that it actually removes the code which stops the wdt. I'm not a real expert in that, but if we argue that the system is "safe" as required by IEC/EN 61508 part 3, it's probably beneficial if we can show
that there *is* no code to stop the wdt, not a snipplet only disabled by a flag.
Just my €0.01, though - maybe more insight from the WDT gurus?
Cheers, Albrecht.
[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]
^ permalink raw reply
* Re: [PATCHv2 3/3] mpc52xx/wdt: remove obsolete old WDT implementation
From: Albrecht Dreß @ 2009-11-12 20:36 UTC (permalink / raw)
To: Wolfram Sang
Cc: Linux PPC Development, Devicetree Discussions, Wim Van Sebroeck
In-Reply-To: <20091112195646.GB13350@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 880 bytes --]
Hi Wolfram:
Am 12.11.09 20:56 schrieb(en) Wolfram Sang:
> > Note: The file drivers/watchdog/mpc5200_wdt.c is now obsolete and should be removed.
>
> Ehrm, this is possible within a patch :)
Ummm, I tried that (removed the file from git locally), but apparently I'm too dumb to find the proper options to get this into the diff (to my defence, I'm still using cvs/svn for my projects). Can you give me a hint?
> > config MPC5200_WDT
> > - tristate "MPC5200 Watchdog Timer"
> > + bool "MPC52xx Watchdog Timer"
> > depends on PPC_MPC52xx
> > + help
> > + Use General Purpose Timer (GPT) 0 on the MPC5200 as Watchdog.
>
> s/5200/52xx/ here too.
Yes, you're right. The config option should still be 'MPC5200_WDT', though, as otherwise all the default configs had to be touched - IMO, on the low level, this does no harm. Thoughts?
Cheers, Albrecht.
[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]
^ permalink raw reply
* Re: [PATCHv2 3/3] mpc52xx/wdt: remove obsolete old WDT implementation
From: Grant Likely @ 2009-11-12 20:43 UTC (permalink / raw)
To: Albrecht Dreß
Cc: Linux PPC Development, Devicetree Discussions, Wim Van Sebroeck
In-Reply-To: <1258058202.2192.1@antares>
On Thu, Nov 12, 2009 at 1:36 PM, Albrecht Dre=DF <albrecht.dress@arcor.de> =
wrote:
> Hi Wolfram:
>
> Am 12.11.09 20:56 schrieb(en) Wolfram Sang:
>>
>> > Note: The file drivers/watchdog/mpc5200_wdt.c is now obsolete and shou=
ld
>> > be removed.
>>
>> Ehrm, this is possible within a patch :)
>
> Ummm, I tried that (removed the file from git locally), but apparently I'=
m
> too dumb to find the proper options to get this into the diff (to my
> defence, I'm still using cvs/svn for my projects). =A0Can you give me a h=
int?
>
>> > =A0config MPC5200_WDT
>> > - =A0 =A0 tristate "MPC5200 Watchdog Timer"
>> > + =A0 =A0 bool "MPC52xx Watchdog Timer"
>> > =A0 =A0 =A0 depends on PPC_MPC52xx
>> > + =A0 =A0 help
>> > + =A0 =A0 =A0 Use General Purpose Timer (GPT) 0 on the MPC5200 as Watc=
hdog.
>>
>> s/5200/52xx/ here too.
>
> Yes, you're right. =A0The config option should still be 'MPC5200_WDT', th=
ough,
> as otherwise all the default configs had to be touched - IMO, on the low
> level, this does no harm. =A0Thoughts?
I don't care. Don't bother respinning just for this.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCHv2 3/3] mpc52xx/wdt: remove obsolete old WDT implementation
From: Wolfram Sang @ 2009-11-12 20:47 UTC (permalink / raw)
To: Albrecht Dreß
Cc: Linux PPC Development, Devicetree Discussions, Wim Van Sebroeck
In-Reply-To: <1258058202.2192.1@antares>
[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]
On Thu, Nov 12, 2009 at 09:36:42PM +0100, Albrecht Dreß wrote:
> Hi Wolfram:
>
> Am 12.11.09 20:56 schrieb(en) Wolfram Sang:
>> > Note: The file drivers/watchdog/mpc5200_wdt.c is now obsolete and should
>> > be removed.
>>
>> Ehrm, this is possible within a patch :)
>
> Ummm, I tried that (removed the file from git locally), but apparently I'm
> too dumb to find the proper options to get this into the diff (to my defence,
> I'm still using cvs/svn for my projects). Can you give me a hint?
git rm file
You can even do this after you removed the file using rm already. 'git status'
gives you an overview about the current changes and how to add them to the
commit.
> Yes, you're right. The config option should still be 'MPC5200_WDT', though,
> as otherwise all the default configs had to be touched - IMO, on the low
> level, this does no harm. Thoughts?
I'd also leave the config name. But the help text should match the prompt :)
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] mpc5200/gpt: tiny fix for gpt period limitation
From: Grant Likely @ 2009-11-12 21:15 UTC (permalink / raw)
To: Albrecht Dreß; +Cc: linuxppc-dev
In-Reply-To: <18304285.1257929369240.JavaMail.ngmail@webmail19.ha2.local>
On Wed, Nov 11, 2009 at 1:49 AM, Albrecht Dre=DF <albrecht.dress@arcor.de> =
wrote:
> This patch fixes a limitation of the 5200's period.
>
> Signed-off-by: Albrecht Dre=DF <albrecht.dress@arcor.de>
Okay, I've applied this and patches 1 & 2 from your series. I'm
waiting for a reply from Wim to know whether or not I should also pick
up patch 3. BUT, you're on my shit list. Patch 2 in your series
conflicts with this patch. I had to fix it up by hand. This patch
should have been part of the series, or at least base the series on
this patch. Take a look in my -test branch and make sure I fixed it
right.
g.
> ---
>
> =A0arch/powerpc/include/asm/mpc52xx.h =A0 =A0 =A0 =A0| =A0 =A02 +-
> =A0arch/powerpc/platforms/52xx/mpc52xx_gpt.c | =A0 =A06 +++---
> =A02 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/as=
m/mpc52xx.h
> index 707ab75..933fb8f 100644
> --- a/arch/powerpc/include/asm/mpc52xx.h
> +++ b/arch/powerpc/include/asm/mpc52xx.h
> @@ -279,7 +279,7 @@ extern void mpc52xx_restart(char *cmd);
> =A0/* mpc52xx_gpt.c */
> =A0struct mpc52xx_gpt_priv;
> =A0extern struct mpc52xx_gpt_priv *mpc52xx_gpt_from_irq(int irq);
> -extern int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *gpt, int per=
iod,
> +extern int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *gpt, u64 per=
iod,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int continuous);
> =A0extern void mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt);
>
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c b/arch/powerpc/pla=
tforms/52xx/mpc52xx_gpt.c
> index 2c3fa13..77572ab 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> @@ -378,12 +378,12 @@ EXPORT_SYMBOL(mpc52xx_gpt_from_irq);
> =A0/**
> =A0* mpc52xx_gpt_start_timer - Set and enable the GPT timer
> =A0* @gpt: Pointer to gpt private data structure
> - * @period: period of timer
> + * @period: period of timer in ns; max. ~130s @ 33MHz IPB clock
> =A0* @continuous: set to 1 to make timer continuous free running
> =A0*
> =A0* An interrupt will be generated every time the timer fires
> =A0*/
> -int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *gpt, int period,
> +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 =A0 int continuous)
> =A0{
> =A0 =A0 =A0 =A0u32 clear, set;
> @@ -400,7 +400,7 @@ int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *=
gpt, int period,
> =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 in=
'ns', bus
> =A0 =A0 =A0 =A0 * frequency is in Hz. */
> - =A0 =A0 =A0 clocks =3D (u64)period * (u64)gpt->ipb_freq;
> + =A0 =A0 =A0 clocks =3D period * (u64)gpt->ipb_freq;
> =A0 =A0 =A0 =A0do_div(clocks, 1000000000); /* Scale it down to ns range *=
/
>
> =A0 =A0 =A0 =A0/* This device cannot handle a clock count greater than 32=
bits */
>
>
> Jetzt NEU: Do it youself E-Cards bei Arcor.de!
> Stellen Sie Ihr eigenes Unikat zusammen und machen Sie dem Empf=E4nger ei=
ne ganz pers=F6nliche Freude!
> E-Card Marke Eigenbau: HIER KLICKEN: http://www.arcor.de/rd/footer.ecard
>
--=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-12 21:14 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <4AFC65F7.4050600@freescale.com>
Scott Wood <scottwood@freescale.com> wrote on 12/11/2009 20:45:59:
>
> Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 11/11/2009 16:26:53:
> >> 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.
> >
> > Yeah, I misread the CPU15 errata so my ideas will not work.
> >
> > Anyhow, will you send a patch that make TLB pinning mandatory?
> > After that my series can go in.
>
> One other concern with pinning on 8xx -- could it cause problems with
> uncached DMA mappings? What happens if a speculative load pulls in a
> cache line in an area that's supposed to be uncached?
hmm, why should this be a problem? Pinning has been around as a config option
for a long time so any problems should have surfaced by now.
Secondly, I was thinking that we could just make the ITLB pinning
mandatory and let the DTLB pinning be as is, configurable.
Jocke
^ permalink raw reply
* Re: [PATCH 3/3] mpc52xx/wdt: WDT uses GPT api
From: Wim Van Sebroeck @ 2009-11-12 21:36 UTC (permalink / raw)
To: Albrecht Dreß
Cc: Linux PPC Development, devicetree-discuss, Wim Van Sebroeck
In-Reply-To: <1257884778.14374.5@antares>
Hi All,
>> 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!
My opinion: it is harder to maintain the watchdog code if it is being moved away from drivers/watchdog.
I need to check the code before I comment any further on this, but my first thought is: why don't you do it with platform resources like other devices are doing? This way you can keep the platform code under arch and the watchdog itself under drivers/watchdog/. You can look at the following drivers as an example: adx_wdt.c ar7_wdt.c at32ap700x_wdt.c coh901327_wdt.c davinci_wdt.c mpcore_wdt.c mv64x60_wdt.c nuc900_wdt.c omap_wdt.c pnx4008_wdt.c rc32434_wdt.c s3c2410_wdt.c txx9wdt.c .
Kind regards,
Wim.
^ permalink raw reply
* Re: [PATCH] mpc5200/gpt: tiny fix for gpt period limitation
From: Wolfram Sang @ 2009-11-12 21:44 UTC (permalink / raw)
To: Grant Likely; +Cc: Albrecht Dreß, linuxppc-dev
In-Reply-To: <fa686aa40911121315h4374711ftf07314cc4d9e7aff@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 453 bytes --]
> up patch 3. BUT, you're on my shit list. Patch 2 in your series
Huh, are you in a bad mood today? IMHO he addressed some valid issues; and we
all had to start somewhere regarding the workflow. I know that manually
adapting patches is annoying, but this wording sounds quite strong...
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
From: Scott Wood @ 2009-11-12 21:57 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <OFF2FA2BCA.A6C0CAA9-ONC125766C.0074494F-C125766C.0074AF53@transmode.se>
Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 12/11/2009 20:45:59:
>> One other concern with pinning on 8xx -- could it cause problems with
>> uncached DMA mappings? What happens if a speculative load pulls in a
>> cache line in an area that's supposed to be uncached?
>
> hmm, why should this be a problem?
Because then you would be accessing potentially stale DMA data -- and
more generally, the architecture prohibits such mixing.
> Pinning has been around as a config option
> for a long time so any problems should have surfaced by now.
It has existed as an option, which is going to get less test coverage
than something that is always on. Plus, it would not be a particularly
common failure -- easy to blame one-off weirdness on something else.
> Secondly, I was thinking that we could just make the ITLB pinning
> mandatory and let the DTLB pinning be as is, configurable.
That could work. We could also limit the pool of memory
dma_alloc_coherent() uses to not overlap with anything pinned.
-Scott
^ 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