[08/19] sh: sh3: sh770x: Fix platform data for the IRDA serial port
diff mbox

Message ID 20170103230639.19660-9-laurent.pinchart+renesas@ideasonboard.com
State Under Review
Delegated to: Geert Uytterhoeven
Headers show

Commit Message

Laurent Pinchart Jan. 3, 2017, 11:06 p.m. UTC
Even though most of its registers are 8-bit wide, the IRDA has two
16-bit registers that make it a 16-bit peripheral and not a 8-bit
peripheral with addresses shifted by one. Fix the memory resource size
and the platform data regshift value.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/sh/kernel/cpu/sh3/setup-sh770x.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Geert Uytterhoeven Jan. 6, 2017, 10:18 a.m. UTC | #1
Hi Laurent,

On Wed, Jan 4, 2017 at 12:06 AM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> Even though most of its registers are 8-bit wide, the IRDA has two
> 16-bit registers that make it a 16-bit peripheral and not a 8-bit
> peripheral with addresses shifted by one. Fix the memory resource size
> and the platform data regshift value.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  arch/sh/kernel/cpu/sh3/setup-sh770x.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/sh/kernel/cpu/sh3/setup-sh770x.c b/arch/sh/kernel/cpu/sh3/setup-sh770x.c
> index e1e54258b822..084a91e6027e 100644
> --- a/arch/sh/kernel/cpu/sh3/setup-sh770x.c
> +++ b/arch/sh/kernel/cpu/sh3/setup-sh770x.c
> @@ -157,11 +157,10 @@ static struct platform_device scif1_device = {
>  static struct plat_sci_port scif2_platform_data = {
>         .type           = PORT_IRDA,
>         .ops            = &sh770x_sci_port_ops,
> -       .regshift       = 1,

According to my sh7707 and sh7709 datasheets. regshift = 1 is correct.

SCIx_IRDA_REGTYPE uses:

                [SCSMR]         = { 0x00,  8 },
                [SCBRR]         = { 0x01,  8 },
                [SCSCR]         = { 0x02,  8 },
                [SCxTDR]        = { 0x03,  8 },
                [SCxSR]         = { 0x04,  8 },
                [SCxRDR]        = { 0x05,  8 },
                [SCFCR]         = { 0x06,  8 },
                [SCFDR]         = { 0x07, 16 },

While the datasheet says:

        SCSMR1  H'A4000140      8 bits
        SCBRR1  H'A4000142      8 bits
        SCSCR1  H'A4000144      8 bits
        SCFTDR1 H'A4000146      8 bits
        SCSSR1  H'A4000148      16 bits
        SCFRDR1 H'A400014A      8 bits
        SCFCR1  H'A400014C      8 bits
        SCFDR1  H'A400014E      16 bits

So you do need regshift = 1 to handle the gaps.

Note that SCSSR1 is a 16-bit registers, while SCIx_IRDA_REGTYPE
declares it as 8-bit, so it may not work at all...

Ah, you're fixing all this in "[PATCH 19/19] serial: sh-sci: Compute the
regshift value for SCI ports".
I think that part should be moved to this patch, to not break bisection.

>  static struct resource scif2_resources[] = {
> -       DEFINE_RES_MEM(0xa4000140, 0x10),
> +       DEFINE_RES_MEM(0xa4000140, 0x20),
>         DEFINE_RES_IRQ(evt2irq(0x880)),
>  };

According to the register list above, resource size 0x10 is correct.

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
Laurent Pinchart Jan. 6, 2017, 10:25 a.m. UTC | #2
Hi Geert,

On Friday 06 Jan 2017 11:18:32 Geert Uytterhoeven wrote:
> On Wed, Jan 4, 2017 at 12:06 AM, Laurent Pinchart wrote:
> > Even though most of its registers are 8-bit wide, the IRDA has two
> > 16-bit registers that make it a 16-bit peripheral and not a 8-bit
> > peripheral with addresses shifted by one. Fix the memory resource size
> > and the platform data regshift value.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  arch/sh/kernel/cpu/sh3/setup-sh770x.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/arch/sh/kernel/cpu/sh3/setup-sh770x.c
> > b/arch/sh/kernel/cpu/sh3/setup-sh770x.c index e1e54258b822..084a91e6027e
> > 100644
> > --- a/arch/sh/kernel/cpu/sh3/setup-sh770x.c
> > +++ b/arch/sh/kernel/cpu/sh3/setup-sh770x.c
> > @@ -157,11 +157,10 @@ static struct platform_device scif1_device = {
> >  static struct plat_sci_port scif2_platform_data = {
> >         .type           = PORT_IRDA,
> >         .ops            = &sh770x_sci_port_ops,
> > -       .regshift       = 1,
> 
> According to my sh7707 and sh7709 datasheets. regshift = 1 is correct.
> 
> SCIx_IRDA_REGTYPE uses:
> 
>                 [SCSMR]         = { 0x00,  8 },
>                 [SCBRR]         = { 0x01,  8 },
>                 [SCSCR]         = { 0x02,  8 },
>                 [SCxTDR]        = { 0x03,  8 },
>                 [SCxSR]         = { 0x04,  8 },
>                 [SCxRDR]        = { 0x05,  8 },
>                 [SCFCR]         = { 0x06,  8 },
>                 [SCFDR]         = { 0x07, 16 },
> 
> While the datasheet says:
> 
>         SCSMR1  H'A4000140      8 bits
>         SCBRR1  H'A4000142      8 bits
>         SCSCR1  H'A4000144      8 bits
>         SCFTDR1 H'A4000146      8 bits
>         SCSSR1  H'A4000148      16 bits
>         SCFRDR1 H'A400014A      8 bits
>         SCFCR1  H'A400014C      8 bits
>         SCFDR1  H'A400014E      16 bits
> 
> So you do need regshift = 1 to handle the gaps.
> 
> Note that SCSSR1 is a 16-bit registers, while SCIx_IRDA_REGTYPE
> declares it as 8-bit, so it may not work at all...
> 
> Ah, you're fixing all this in "[PATCH 19/19] serial: sh-sci: Compute the
> regshift value for SCI ports".
> I think that part should be moved to this patch, to not break bisection.

Yes, I realized that after sending the series. If you think we need to care 
about bisection here (given that it seems that sh-sci hasn't been tested on SH 
since at least v3.1...) we can fix that.

> >  static struct resource scif2_resources[] = {
> > -       DEFINE_RES_MEM(0xa4000140, 0x10),
> > +       DEFINE_RES_MEM(0xa4000140, 0x20),
> >         DEFINE_RES_IRQ(evt2irq(0x880)),
> >  };
> 
> According to the register list above, resource size 0x10 is correct.

I'll fix this.

Patch
diff mbox

diff --git a/arch/sh/kernel/cpu/sh3/setup-sh770x.c b/arch/sh/kernel/cpu/sh3/setup-sh770x.c
index e1e54258b822..084a91e6027e 100644
--- a/arch/sh/kernel/cpu/sh3/setup-sh770x.c
+++ b/arch/sh/kernel/cpu/sh3/setup-sh770x.c
@@ -157,11 +157,10 @@  static struct platform_device scif1_device = {
 static struct plat_sci_port scif2_platform_data = {
 	.type		= PORT_IRDA,
 	.ops		= &sh770x_sci_port_ops,
-	.regshift	= 1,
 };
 
 static struct resource scif2_resources[] = {
-	DEFINE_RES_MEM(0xa4000140, 0x10),
+	DEFINE_RES_MEM(0xa4000140, 0x20),
 	DEFINE_RES_IRQ(evt2irq(0x880)),
 };