* [PATCH] spi: sh-msiof: Add reset of registers before starting transfer
@ 2019-02-28 11:02 Geert Uytterhoeven
2019-03-01 9:24 ` Simon Horman
2019-03-01 11:43 ` Wolfram Sang
0 siblings, 2 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2019-02-28 11:02 UTC (permalink / raw)
To: Mark Brown
Cc: Wolfram Sang, linux-spi, linux-renesas-soc, Hiromitsu Yamasaki,
Geert Uytterhoeven
From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
In accordance with hardware specification Ver 1.0, reset register
transmission / reception setting before transfer.
Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/spi/spi-sh-msiof.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index b20e70a2bdd115d7..6e83368874839d09 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -130,6 +130,8 @@ struct sh_msiof_spi_priv {
#define CTR_TFSE 0x00004000 /* Transmit Frame Sync Signal Output Enable */
#define CTR_TXE 0x00000200 /* Transmit Enable */
#define CTR_RXE 0x00000100 /* Receive Enable */
+#define CTR_TXRST 0x00000002 /* Transmit Reset */
+#define CTR_RXRST 0x00000001 /* Receive Reset */
/* FCTR */
#define FCTR_TFWM_MASK 0xe0000000 /* Transmit FIFO Watermark */
@@ -246,6 +248,25 @@ static irqreturn_t sh_msiof_spi_irq(int irq, void *data)
return IRQ_HANDLED;
}
+static void sh_msiof_spi_reset_regs(struct sh_msiof_spi_priv *p)
+{
+ u32 mask = CTR_TXRST | CTR_RXRST;
+ u32 data;
+ int k;
+
+ data = sh_msiof_read(p, CTR);
+ data |= mask;
+
+ sh_msiof_write(p, CTR, data);
+
+ for (k = 100; k > 0; k--) {
+ if (!(sh_msiof_read(p, CTR) & mask))
+ break;
+
+ udelay(1);
+ }
+}
+
static const u32 sh_msiof_spi_div_array[] = {
SCR_BRDV_DIV_1, SCR_BRDV_DIV_2, SCR_BRDV_DIV_4,
SCR_BRDV_DIV_8, SCR_BRDV_DIV_16, SCR_BRDV_DIV_32,
@@ -925,6 +946,9 @@ static int sh_msiof_transfer_one(struct spi_controller *ctlr,
bool swab;
int ret;
+ /* reset registers */
+ sh_msiof_spi_reset_regs(p);
+
/* setup clocks (clock already enabled in chipselect()) */
if (!spi_controller_is_slave(p->ctlr))
sh_msiof_spi_set_clk_regs(p, clk_get_rate(p->clk), t->speed_hz);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: sh-msiof: Add reset of registers before starting transfer
2019-02-28 11:02 [PATCH] spi: sh-msiof: Add reset of registers before starting transfer Geert Uytterhoeven
@ 2019-03-01 9:24 ` Simon Horman
2019-03-01 9:28 ` Geert Uytterhoeven
2019-03-01 11:43 ` Wolfram Sang
1 sibling, 1 reply; 6+ messages in thread
From: Simon Horman @ 2019-03-01 9:24 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Mark Brown, Wolfram Sang, linux-spi, linux-renesas-soc,
Hiromitsu Yamasaki
On Thu, Feb 28, 2019 at 12:02:15PM +0100, Geert Uytterhoeven wrote:
> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>
> In accordance with hardware specification Ver 1.0, reset register
> transmission / reception setting before transfer.
>
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> drivers/spi/spi-sh-msiof.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index b20e70a2bdd115d7..6e83368874839d09 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -130,6 +130,8 @@ struct sh_msiof_spi_priv {
> #define CTR_TFSE 0x00004000 /* Transmit Frame Sync Signal Output Enable */
> #define CTR_TXE 0x00000200 /* Transmit Enable */
> #define CTR_RXE 0x00000100 /* Receive Enable */
> +#define CTR_TXRST 0x00000002 /* Transmit Reset */
> +#define CTR_RXRST 0x00000001 /* Receive Reset */
nit: can we start using the BIT() macro here?
> /* FCTR */
> #define FCTR_TFWM_MASK 0xe0000000 /* Transmit FIFO Watermark */
> @@ -246,6 +248,25 @@ static irqreturn_t sh_msiof_spi_irq(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +static void sh_msiof_spi_reset_regs(struct sh_msiof_spi_priv *p)
> +{
> + u32 mask = CTR_TXRST | CTR_RXRST;
> + u32 data;
> + int k;
> +
> + data = sh_msiof_read(p, CTR);
> + data |= mask;
> +
> + sh_msiof_write(p, CTR, data);
> +
> + for (k = 100; k > 0; k--) {
> + if (!(sh_msiof_read(p, CTR) & mask))
> + break;
> +
> + udelay(1);
> + }
> +}
> +
> static const u32 sh_msiof_spi_div_array[] = {
> SCR_BRDV_DIV_1, SCR_BRDV_DIV_2, SCR_BRDV_DIV_4,
> SCR_BRDV_DIV_8, SCR_BRDV_DIV_16, SCR_BRDV_DIV_32,
> @@ -925,6 +946,9 @@ static int sh_msiof_transfer_one(struct spi_controller *ctlr,
> bool swab;
> int ret;
>
> + /* reset registers */
> + sh_msiof_spi_reset_regs(p);
> +
> /* setup clocks (clock already enabled in chipselect()) */
> if (!spi_controller_is_slave(p->ctlr))
> sh_msiof_spi_set_clk_regs(p, clk_get_rate(p->clk), t->speed_hz);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: sh-msiof: Add reset of registers before starting transfer
2019-03-01 9:24 ` Simon Horman
@ 2019-03-01 9:28 ` Geert Uytterhoeven
2019-03-01 9:40 ` Simon Horman
0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2019-03-01 9:28 UTC (permalink / raw)
To: Simon Horman
Cc: Geert Uytterhoeven, Mark Brown, Wolfram Sang, linux-spi,
Linux-Renesas, Hiromitsu Yamasaki
Hi Simon,
On Fri, Mar 1, 2019 at 10:25 AM Simon Horman <horms@verge.net.au> wrote:
> On Thu, Feb 28, 2019 at 12:02:15PM +0100, Geert Uytterhoeven wrote:
> > From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> >
> > In accordance with hardware specification Ver 1.0, reset register
> > transmission / reception setting before transfer.
> >
> > Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Thanks!
> > --- a/drivers/spi/spi-sh-msiof.c
> > +++ b/drivers/spi/spi-sh-msiof.c
> > @@ -130,6 +130,8 @@ struct sh_msiof_spi_priv {
> > #define CTR_TFSE 0x00004000 /* Transmit Frame Sync Signal Output Enable */
> > #define CTR_TXE 0x00000200 /* Transmit Enable */
> > #define CTR_RXE 0x00000100 /* Receive Enable */
> > +#define CTR_TXRST 0x00000002 /* Transmit Reset */
> > +#define CTR_RXRST 0x00000001 /* Receive Reset */
>
> nit: can we start using the BIT() macro here?
Sure, if you want to convert the whole driver... ;-)
Note that many of them are multi-bit fields.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: sh-msiof: Add reset of registers before starting transfer
2019-03-01 9:28 ` Geert Uytterhoeven
@ 2019-03-01 9:40 ` Simon Horman
0 siblings, 0 replies; 6+ messages in thread
From: Simon Horman @ 2019-03-01 9:40 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Geert Uytterhoeven, Mark Brown, Wolfram Sang, linux-spi,
Linux-Renesas, Hiromitsu Yamasaki
On Fri, Mar 01, 2019 at 10:28:21AM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
>
> On Fri, Mar 1, 2019 at 10:25 AM Simon Horman <horms@verge.net.au> wrote:
> > On Thu, Feb 28, 2019 at 12:02:15PM +0100, Geert Uytterhoeven wrote:
> > > From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> > >
> > > In accordance with hardware specification Ver 1.0, reset register
> > > transmission / reception setting before transfer.
> > >
> > > Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
>
> Thanks!
>
> > > --- a/drivers/spi/spi-sh-msiof.c
> > > +++ b/drivers/spi/spi-sh-msiof.c
> > > @@ -130,6 +130,8 @@ struct sh_msiof_spi_priv {
> > > #define CTR_TFSE 0x00004000 /* Transmit Frame Sync Signal Output Enable */
> > > #define CTR_TXE 0x00000200 /* Transmit Enable */
> > > #define CTR_RXE 0x00000100 /* Receive Enable */
> > > +#define CTR_TXRST 0x00000002 /* Transmit Reset */
> > > +#define CTR_RXRST 0x00000001 /* Receive Reset */
> >
> > nit: can we start using the BIT() macro here?
>
> Sure, if you want to convert the whole driver... ;-)
> Note that many of them are multi-bit fields.
Yes, my 2c worth is to use BIT() and GENMASK() throughout the driver.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: sh-msiof: Add reset of registers before starting transfer
2019-02-28 11:02 [PATCH] spi: sh-msiof: Add reset of registers before starting transfer Geert Uytterhoeven
2019-03-01 9:24 ` Simon Horman
@ 2019-03-01 11:43 ` Wolfram Sang
2019-03-01 12:03 ` Geert Uytterhoeven
1 sibling, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2019-03-01 11:43 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Mark Brown, Wolfram Sang, linux-spi, linux-renesas-soc,
Hiromitsu Yamasaki
[-- Attachment #1: Type: text/plain, Size: 256 bytes --]
> + for (k = 100; k > 0; k--) {
> + if (!(sh_msiof_read(p, CTR) & mask))
> + break;
> +
> + udelay(1);
> + }
Why no reuse of sh_msiof_modify_ctr_wait? And can't we maybe even use
readl_poll_timeout() here since we know the register is fixed to CTR?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: sh-msiof: Add reset of registers before starting transfer
2019-03-01 11:43 ` Wolfram Sang
@ 2019-03-01 12:03 ` Geert Uytterhoeven
0 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2019-03-01 12:03 UTC (permalink / raw)
To: Wolfram Sang
Cc: Geert Uytterhoeven, Mark Brown, Wolfram Sang, linux-spi,
Linux-Renesas, Hiromitsu Yamasaki
Hi Wolfram,
On Fri, Mar 1, 2019 at 12:43 PM Wolfram Sang <wsa@the-dreams.de> wrote:
> > + for (k = 100; k > 0; k--) {
> > + if (!(sh_msiof_read(p, CTR) & mask))
> > + break;
> > +
> > + udelay(1);
> > + }
>
> Why no reuse of sh_msiof_modify_ctr_wait? And can't we maybe even use
> readl_poll_timeout() here since we know the register is fixed to CTR?
Thanks, that looks like a good improvement. Will do.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-01 12:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-28 11:02 [PATCH] spi: sh-msiof: Add reset of registers before starting transfer Geert Uytterhoeven
2019-03-01 9:24 ` Simon Horman
2019-03-01 9:28 ` Geert Uytterhoeven
2019-03-01 9:40 ` Simon Horman
2019-03-01 11:43 ` Wolfram Sang
2019-03-01 12:03 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).