Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1341712 > unrolled thread

[PATCH] spi/rockchip: Make sure spi clk is on in rockchip_spi_set_cs

Back to article view | Back to linux.kernel

Contents

  [PATCH] spi/rockchip: Make sure spi clk is on in rockchip_spi_set_cs Huibin Hong <huibin.hong@rock-chips.com> - 2016-02-24 11:10 +0100
    Re: [PATCH] spi/rockchip: Make sure spi clk is on in  rockchip_spi_set_cs "Huang, Tao" <huangtao@rock-chips.com> - 2016-02-24 12:20 +0100
      Re: [PATCH] spi/rockchip: Make sure spi clk is on in  rockchip_spi_set_cs Mark Brown <broonie@kernel.org> - 2016-02-26 03:00 +0100
        Re: [PATCH] spi/rockchip: Make sure spi clk is on in  rockchip_spi_set_cs "Huang, Tao" <huangtao@rock-chips.com> - 2016-02-26 03:40 +0100

#1341712 — [PATCH] spi/rockchip: Make sure spi clk is on in rockchip_spi_set_cs

Rockchip_spi_set_cs could be called by spi_setup, but
spi_setup may be called by device driver after runtime suspend.
Then the spi clock is closed, rockchip_spi_set_cs may access the
spi registers, which causes cpu block in some socs.

Fixes: 64e36824b32 ("spi/rockchip: add driver for Rockchip RK3xxx")
Cc: Addy Ke <addy.ke@rockchip.com>
Cc: shawn.lin <shawn.lin@kernel-upstream.org>
Signed-off-by: Huibin Hong <huibin.hong@rock-chips.com>
---

 drivers/spi/spi-rockchip.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 79a8bc4..035767c 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -265,7 +265,10 @@ static inline u32 rx_max(struct rockchip_spi *rs)
 static void rockchip_spi_set_cs(struct spi_device *spi, bool enable)
 {
 	u32 ser;
-	struct rockchip_spi *rs = spi_master_get_devdata(spi->master);
+	struct spi_master *master = spi->master;
+	struct rockchip_spi *rs = spi_master_get_devdata(master);
+
+	pm_runtime_get_sync(rs->dev);

 	ser = readl_relaxed(rs->regs + ROCKCHIP_SPI_SER) & SER_MASK;

@@ -290,6 +293,8 @@ static void rockchip_spi_set_cs(struct spi_device *spi, bool enable)
 		ser &= ~(1 << spi->chip_select);

 	writel_relaxed(ser, rs->regs + ROCKCHIP_SPI_SER);
+
+	pm_runtime_put_sync(rs->dev);
 }

 static int rockchip_spi_prepare_message(struct spi_master *master,
--
1.9.1

[toc] | [next] | [standalone]

#1341892 — Re: [PATCH] spi/rockchip: Make sure spi clk is on in rockchip_spi_set_cs

Hi, Mark:

Another way to solve this bug is add runtime PM support while spi setup.
Some other chips may have some problem, for example mt65xx and orion,
which access hardware register too.

On 2016年02月24日 18:00, Huibin Hong wrote:
> Rockchip_spi_set_cs could be called by spi_setup, but
> spi_setup may be called by device driver after runtime suspend.
> Then the spi clock is closed, rockchip_spi_set_cs may access the
> spi registers, which causes cpu block in some socs.
> 
> Fixes: 64e36824b32 ("spi/rockchip: add driver for Rockchip RK3xxx")
> Cc: Addy Ke <addy.ke@rockchip.com>
> Cc: shawn.lin <shawn.lin@kernel-upstream.org>
> Signed-off-by: Huibin Hong <huibin.hong@rock-chips.com>
> ---
> 
>  drivers/spi/spi-rockchip.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
> index 79a8bc4..035767c 100644
> --- a/drivers/spi/spi-rockchip.c
> +++ b/drivers/spi/spi-rockchip.c
> @@ -265,7 +265,10 @@ static inline u32 rx_max(struct rockchip_spi *rs)
>  static void rockchip_spi_set_cs(struct spi_device *spi, bool enable)
>  {
>  	u32 ser;
> -	struct rockchip_spi *rs = spi_master_get_devdata(spi->master);
> +	struct spi_master *master = spi->master;
> +	struct rockchip_spi *rs = spi_master_get_devdata(master);
> +
> +	pm_runtime_get_sync(rs->dev);
> 
>  	ser = readl_relaxed(rs->regs + ROCKCHIP_SPI_SER) & SER_MASK;
> 
> @@ -290,6 +293,8 @@ static void rockchip_spi_set_cs(struct spi_device *spi, bool enable)
>  		ser &= ~(1 << spi->chip_select);
> 
>  	writel_relaxed(ser, rs->regs + ROCKCHIP_SPI_SER);
> +
> +	pm_runtime_put_sync(rs->dev);
>  }
> 
>  static int rockchip_spi_prepare_message(struct spi_master *master,
> --
> 1.9.1
> 
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 
> 
> 

[toc] | [prev] | [next] | [standalone]

#1343724 — Re: [PATCH] spi/rockchip: Make sure spi clk is on in rockchip_spi_set_cs

[Multipart message — attachments visible in raw view] — view raw

On Wed, Feb 24, 2016 at 07:09:57PM +0800, Huang, Tao wrote:

Please don't top post, reply in line with needed context.  This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.

> Another way to solve this bug is add runtime PM support while spi setup.
> Some other chips may have some problem, for example mt65xx and orion,
> which access hardware register too.

No, this is telling you you're doing something wrong - setup() might be
called while another transfer is in progress so you shouldn't be
changing the hardware setup (see Documentation/spi/spi-summary).  This
means that you normally shouldn't be writing to registers there.  Moving
this to prepare_message() instead is probably what you want.

[toc] | [prev] | [next] | [standalone]

#1343740 — Re: [PATCH] spi/rockchip: Make sure spi clk is on in rockchip_spi_set_cs

On 2016年02月26日 09:58, Mark Brown wrote:
> 
>> Another way to solve this bug is add runtime PM support while spi setup.
>> Some other chips may have some problem, for example mt65xx and orion,
>> which access hardware register too.
> 
> No, this is telling you you're doing something wrong - setup() might be
> called while another transfer is in progress so you shouldn't be
> changing the hardware setup (see Documentation/spi/spi-summary).  This
> means that you normally shouldn't be writing to registers there.  Moving
> this to prepare_message() instead is probably what you want.
> 
You misunderstand me. I talk about spi_setup, as
Documentation/spi/spi-summary, which would normally be called from
probe() before the first I/O is done to the device.
spi_setup will call spi_set_cs(spi, false), which introduced with commit
1a7b7ee72c21 ("spi: Ensure that CS line is in non-active state after
spi_setup()"). And spi_set_cs will call spi->master->set_cs, and set_cs
callback will access register. Without clk enable, I believe some
drivers will failed to run.

Best Regards,

Huang, Tao

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web