[OpenWrt-Devel] [PATCH 2/8] ramips: remove rt2880 spi lock and clean bit operation

[Mingyu Li]

Hi John.

i check the code which functions need read/write register.
3 functions (prepare_message, set_cs, transfer_one) are used
at same function __spi_pump_messages at spi.c.

the __spi_pump_message is protected by spi_master->queue_lock
this make sure spi device operation is serialized.

the spi bus is protected by spi_master->bus_lock_spinlock. you can
found it at spi_async function. it make sure only one spi device work
on the same spi bus.
so the 3 functions don't have lock problem.

about the setup function. it maybe be called by user space to setup
a new spi device. but it will not affect other spi device. because it just
disable the new spi device by useing cs pin.

so i think it is save to remove the lock protect.

the most important is the lock on probe function must be wrong.
because it only call spin_lock_irqsave. but not call spin_unlock_irqrestore.

2015-10-26 22:26 GMT+08:00 John Crispin <blogic at openwrt.org>:

> Hi Michael,
>
> while merging the series i stumbled across this patch. to me this looks
> wrong.
>
> rather than removing the locking i think we should add more to
> rt2880_spi_read and rt2880_spi_write.
>
> i'll merge the other 7 patches just now if they apply without 2/8
>
>
>         John
>
> On 08/10/2015 16:16, Michael Lee wrote:
> > Signed-off-by: Michael Lee <igvtee at gmail.com>
> > ---
> >  ...0050-SPI-ralink-add-Ralink-SoC-spi-driver.patch | 23
> +++++-----------------
> >  1 file changed, 5 insertions(+), 18 deletions(-)
> >
> > diff --git
> a/target/linux/ramips/patches-3.18/0050-SPI-ralink-add-Ralink-SoC-spi-driver.patch
> b/target/linux/ramips/patches-3.18/0050-SPI-ralink-add-Ralink-SoC-spi-driver.patch
> > index ca04a17..d6a462c 100644
> > ---
> a/target/linux/ramips/patches-3.18/0050-SPI-ralink-add-Ralink-SoC-spi-driver.patch
> > +++
> b/target/linux/ramips/patches-3.18/0050-SPI-ralink-add-Ralink-SoC-spi-driver.patch
> > @@ -41,7 +41,7 @@ Acked-by: John Crispin <blogic at openwrt.org>
> >   spi-s3c24xx-hw-$(CONFIG_SPI_S3C24XX_FIQ) += spi-s3c24xx-fiq.o
> >  --- /dev/null
> >  +++ b/drivers/spi/spi-rt2880.c
> > -@@ -0,0 +1,493 @@
> > +@@ -0,0 +1,480 @@
> >  +/*
> >  + * spi-rt2880.c -- Ralink RT288x/RT305x SPI controller driver
> >  + *
> > @@ -174,7 +174,6 @@ Acked-by: John Crispin <blogic at openwrt.org>
> >  +    unsigned int            sys_freq;
> >  +    unsigned int            speed;
> >  +    struct clk              *clk;
> > -+    spinlock_t              lock;
> >  +};
> >  +
> >  +static inline struct rt2880_spi *spidev_to_rt2880_spi(struct
> spi_device *spi)
> > @@ -187,7 +186,8 @@ Acked-by: John Crispin <blogic at openwrt.org>
> >  +    return ioread32(rs->base + reg);
> >  +}
> >  +
> > -+static inline void rt2880_spi_write(struct rt2880_spi *rs, u32 reg,
> u32 val)
> > ++static inline void rt2880_spi_write(struct rt2880_spi *rs, u32 reg,
> > ++            const u32 val)
> >  +{
> >  +    iowrite32(val, rs->base + reg);
> >  +}
> > @@ -195,27 +195,15 @@ Acked-by: John Crispin <blogic at openwrt.org>
> >  +static inline void rt2880_spi_setbits(struct rt2880_spi *rs, u32 reg,
> u32 mask)
> >  +{
> >  +    void __iomem *addr = rs->base + reg;
> > -+    unsigned long flags;
> > -+    u32 val;
> >  +
> > -+    spin_lock_irqsave(&rs->lock, flags);
> > -+    val = ioread32(addr);
> > -+    val |= mask;
> > -+    iowrite32(val, addr);
> > -+    spin_unlock_irqrestore(&rs->lock, flags);
> > ++    iowrite32((ioread32(addr) | mask), addr);
> >  +}
> >  +
> >  +static inline void rt2880_spi_clrbits(struct rt2880_spi *rs, u32 reg,
> u32 mask)
> >  +{
> >  +    void __iomem *addr = rs->base + reg;
> > -+    unsigned long flags;
> > -+    u32 val;
> >  +
> > -+    spin_lock_irqsave(&rs->lock, flags);
> > -+    val = ioread32(addr);
> > -+    val &= ~mask;
> > -+    iowrite32(val, addr);
> > -+    spin_unlock_irqrestore(&rs->lock, flags);
> > ++    iowrite32((ioread32(addr) & ~mask), addr);
> >  +}
> >  +
> >  +static int rt2880_spi_baudrate_set(struct spi_device *spi, unsigned
> int speed)
> > @@ -488,7 +476,6 @@ Acked-by: John Crispin <blogic at openwrt.org>
> >  +    rs->master = master;
> >  +    rs->sys_freq = clk_get_rate(rs->clk);
> >  +    dev_dbg(&pdev->dev, "sys_freq: %u\n", rs->sys_freq);
> > -+    spin_lock_irqsave(&rs->lock, flags);
> >  +
> >  +    device_reset(&pdev->dev);
> >  +
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20151027/8733b33b/attachment.htm>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel