<div dir="ltr"><div><div><div><div>Hi John.<br><br></div>i check the code which functions need read/write register.<br></div>3 functions (prepare_message, set_cs, transfer_one) are used<br></div>at same function __spi_pump_messages at spi.c.<br></div><div><br></div><div>the __spi_pump_message is protected by spi_master->queue_lock<br></div><div>this make sure spi device operation is serialized.<br><br></div><div>the spi bus is protected by spi_master->bus_lock_spinlock. you can<br></div><div>found it at spi_async function. it make sure only one spi device work<br></div><div>on the same spi bus.<br></div><div>so the 3 functions don't have lock problem.<br><br></div><div>about the setup function. it maybe be called by user space to setup<br></div><div>a new spi device. but it will not affect other spi device. because it just<br></div><div>disable the new spi device by useing cs pin.<br><br></div><div>so i think it is save to remove the lock protect.<br><br></div><div>the most important is the lock on probe function must be wrong.<br></div><div>because it only call spin_lock_irqsave. but not call spin_unlock_irqrestore.</div></div><div class="gmail_extra"><br><div class="gmail_quote">2015-10-26 22:26 GMT+08:00 John Crispin <span dir="ltr"><<a href="mailto:blogic@openwrt.org" target="_blank">blogic@openwrt.org</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Michael,<br>
<br>
while merging the series i stumbled across this patch. to me this looks<br>
wrong.<br>
<br>
rather than removing the locking i think we should add more to<br>
rt2880_spi_read and rt2880_spi_write.<br>
<br>
i'll merge the other 7 patches just now if they apply without 2/8<br>
<span class="HOEnZb"><font color="#888888"><br>
<br>
        John<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On 08/10/2015 16:16, Michael Lee wrote:<br>
> Signed-off-by: Michael Lee <<a href="mailto:igvtee@gmail.com">igvtee@gmail.com</a>><br>
> ---<br>
>  ...0050-SPI-ralink-add-Ralink-SoC-spi-driver.patch | 23 +++++-----------------<br>
>  1 file changed, 5 insertions(+), 18 deletions(-)<br>
><br>
> 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<br>
> index ca04a17..d6a462c 100644<br>
> --- a/target/linux/ramips/patches-3.18/0050-SPI-ralink-add-Ralink-SoC-spi-driver.patch<br>
> +++ b/target/linux/ramips/patches-3.18/0050-SPI-ralink-add-Ralink-SoC-spi-driver.patch<br>
> @@ -41,7 +41,7 @@ Acked-by: John Crispin <<a href="mailto:blogic@openwrt.org">blogic@openwrt.org</a>><br>
>   spi-s3c24xx-hw-$(CONFIG_SPI_S3C24XX_FIQ) += spi-s3c24xx-fiq.o<br>
>  --- /dev/null<br>
>  +++ b/drivers/spi/spi-rt2880.c<br>
> -@@ -0,0 +1,493 @@<br>
> +@@ -0,0 +1,480 @@<br>
>  +/*<br>
>  + * spi-rt2880.c -- Ralink RT288x/RT305x SPI controller driver<br>
>  + *<br>
> @@ -174,7 +174,6 @@ Acked-by: John Crispin <<a href="mailto:blogic@openwrt.org">blogic@openwrt.org</a>><br>
>  +    unsigned int            sys_freq;<br>
>  +    unsigned int            speed;<br>
>  +    struct clk              *clk;<br>
> -+    spinlock_t              lock;<br>
>  +};<br>
>  +<br>
>  +static inline struct rt2880_spi *spidev_to_rt2880_spi(struct spi_device *spi)<br>
> @@ -187,7 +186,8 @@ Acked-by: John Crispin <<a href="mailto:blogic@openwrt.org">blogic@openwrt.org</a>><br>
>  +    return ioread32(rs->base + reg);<br>
>  +}<br>
>  +<br>
> -+static inline void rt2880_spi_write(struct rt2880_spi *rs, u32 reg, u32 val)<br>
> ++static inline void rt2880_spi_write(struct rt2880_spi *rs, u32 reg,<br>
> ++            const u32 val)<br>
>  +{<br>
>  +    iowrite32(val, rs->base + reg);<br>
>  +}<br>
> @@ -195,27 +195,15 @@ Acked-by: John Crispin <<a href="mailto:blogic@openwrt.org">blogic@openwrt.org</a>><br>
>  +static inline void rt2880_spi_setbits(struct rt2880_spi *rs, u32 reg, u32 mask)<br>
>  +{<br>
>  +    void __iomem *addr = rs->base + reg;<br>
> -+    unsigned long flags;<br>
> -+    u32 val;<br>
>  +<br>
> -+    spin_lock_irqsave(&rs->lock, flags);<br>
> -+    val = ioread32(addr);<br>
> -+    val |= mask;<br>
> -+    iowrite32(val, addr);<br>
> -+    spin_unlock_irqrestore(&rs->lock, flags);<br>
> ++    iowrite32((ioread32(addr) | mask), addr);<br>
>  +}<br>
>  +<br>
>  +static inline void rt2880_spi_clrbits(struct rt2880_spi *rs, u32 reg, u32 mask)<br>
>  +{<br>
>  +    void __iomem *addr = rs->base + reg;<br>
> -+    unsigned long flags;<br>
> -+    u32 val;<br>
>  +<br>
> -+    spin_lock_irqsave(&rs->lock, flags);<br>
> -+    val = ioread32(addr);<br>
> -+    val &= ~mask;<br>
> -+    iowrite32(val, addr);<br>
> -+    spin_unlock_irqrestore(&rs->lock, flags);<br>
> ++    iowrite32((ioread32(addr) & ~mask), addr);<br>
>  +}<br>
>  +<br>
>  +static int rt2880_spi_baudrate_set(struct spi_device *spi, unsigned int speed)<br>
> @@ -488,7 +476,6 @@ Acked-by: John Crispin <<a href="mailto:blogic@openwrt.org">blogic@openwrt.org</a>><br>
>  +    rs->master = master;<br>
>  +    rs->sys_freq = clk_get_rate(rs->clk);<br>
>  +    dev_dbg(&pdev->dev, "sys_freq: %u\n", rs->sys_freq);<br>
> -+    spin_lock_irqsave(&rs->lock, flags);<br>
>  +<br>
>  +    device_reset(&pdev->dev);<br>
>  +<br>
><br>
</div></div></blockquote></div><br></div>