All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: dts: r7s72100: add clock bit definitions
@ 2017-05-25 16:05 Chris Brandt
       [not found] ` <20170525160548.103182-1-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Chris Brandt @ 2017-05-25 16:05 UTC (permalink / raw)
  To: Simon Horman, Geert Uytterhoeven, Rob Herring, Mark Rutland
  Cc: devicetree, linux-renesas-soc, Chris Brandt

Add the remaining bit locations for the module stop clock registers.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 include/dt-bindings/clock/r7s72100-clock.h | 40 ++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/include/dt-bindings/clock/r7s72100-clock.h b/include/dt-bindings/clock/r7s72100-clock.h
index dcd2072151fc..ff5023ca97ed 100644
--- a/include/dt-bindings/clock/r7s72100-clock.h
+++ b/include/dt-bindings/clock/r7s72100-clock.h
@@ -13,7 +13,14 @@
 #define R7S72100_CLK_PLL	0
 
 /* MSTP3 */
+#define R7S72100_CLK_IEBUS	7
+#define R7S72100_CLK_IRDA	6
+#define R7S72100_CLK_LIN0	5
+#define R7S72100_CLK_LIN1	4
 #define R7S72100_CLK_MTU2	3
+#define R7S72100_CLK_CAN	2
+#define R7S72100_CLK_ADCPWR	1
+#define R7S72100_CLK_MCPWM	0
 
 /* MSTP4 */
 #define R7S72100_CLK_SCIF0	7
@@ -26,25 +33,51 @@
 #define R7S72100_CLK_SCIF7	0
 
 /* MSTP5 */
+#define R7S72100_CLK_SCI0	7
+#define R7S72100_CLK_SCI1	6
+#define R7S72100_CLK_SNDGEN0	5
+#define R7S72100_CLK_SNDGEN1	4
+#define R7S72100_CLK_SNDGEN2	3
+#define R7S72100_CLK_SNDGEN3	2
 #define R7S72100_CLK_OSTM0	1
 #define R7S72100_CLK_OSTM1	0
 
 /* MSTP6 */
+#define R7S72100_CLK_ADC	7
+#define R7S72100_CLK_CEU	6
+#define R7S72100_CLK_DOC0	5
+#define R7S72100_CLK_DOC1	4
+#define R7S72100_CLK_DRC0	3
+#define R7S72100_CLK_DRC1	2
+#define R7S72100_CLK_JCU	1
 #define R7S72100_CLK_RTC	0
 
 /* MSTP7 */
+#define R7S72100_CLK_VDEC0	7
+#define R7S72100_CLK_VDEC1	6
 #define R7S72100_CLK_ETHER	4
+#define R7S72100_CLK_NAND	3
 #define R7S72100_CLK_USB0	1
 #define R7S72100_CLK_USB1	0
 
 /* MSTP8 */
+#define R7S72100_CLK_IMR0	7
+#define R7S72100_CLK_IMR1	6
+#define R7S72100_CLK_IMRDISP	5
 #define R7S72100_CLK_MMCIF	4
+#define R7S72100_CLK_MLB	3
+#define R7S72100_CLK_ETHABV	2
+#define R7S72100_CLK_SCUX	1
 
 /* MSTP9 */
 #define R7S72100_CLK_I2C0	7
 #define R7S72100_CLK_I2C1	6
 #define R7S72100_CLK_I2C2	5
 #define R7S72100_CLK_I2C3	4
+#define R7S72100_CLK_SPIMBC0	3
+#define R7S72100_CLK_SPIMBC1	2
+#define R7S72100_CLK_VDC50	1	/* and LVDS */
+#define R7S72100_CLK_VDC51	0
 
 /* MSTP10 */
 #define R7S72100_CLK_SPI0	7
@@ -52,6 +85,9 @@
 #define R7S72100_CLK_SPI2	5
 #define R7S72100_CLK_SPI3	4
 #define R7S72100_CLK_SPI4	3
+#define R7S72100_CLK_CDROM	2
+#define R7S72100_CLK_SPDIF	1
+#define R7S72100_CLK_RGPVG2	0
 
 /* MSTP12 */
 #define R7S72100_CLK_SDHI00	3
@@ -59,4 +95,8 @@
 #define R7S72100_CLK_SDHI10	1
 #define R7S72100_CLK_SDHI11	0
 
+/* MSTP13 */
+#define R7S72100_CLK_PIX1	2
+#define R7S72100_CLK_PIX0	1
+
 #endif /* __DT_BINDINGS_CLOCK_R7S72100_H__ */
-- 
2.13.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] ARM: dts: r7s72100: add clock bit definitions
  2017-05-25 16:05 [PATCH] ARM: dts: r7s72100: add clock bit definitions Chris Brandt
@ 2017-05-29  9:31     ` Geert Uytterhoeven
  0 siblings, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2017-05-29  9:31 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Simon Horman, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux-Renesas

Hi Chris,

On Thu, May 25, 2017 at 6:05 PM, Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> Add the remaining bit locations for the module stop clock registers.
>
> Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

> --- a/include/dt-bindings/clock/r7s72100-clock.h
> +++ b/include/dt-bindings/clock/r7s72100-clock.h
> @@ -13,7 +13,14 @@
>  #define R7S72100_CLK_PLL       0

No CoreSight (MSTP20)?

>  /* MSTP3 */
> +#define R7S72100_CLK_IEBUS     7
> +#define R7S72100_CLK_IRDA      6
> +#define R7S72100_CLK_LIN0      5
> +#define R7S72100_CLK_LIN1      4
>  #define R7S72100_CLK_MTU2      3
> +#define R7S72100_CLK_CAN       2
> +#define R7S72100_CLK_ADCPWR    1
> +#define R7S72100_CLK_MCPWM     0

Perhaps just R7S72100_CLK_PWM?

>  /* MSTP4 */
>  #define R7S72100_CLK_SCIF0     7
> @@ -26,25 +33,51 @@
>  #define R7S72100_CLK_SCIF7     0
>
>  /* MSTP5 */
> +#define R7S72100_CLK_SCI0      7
> +#define R7S72100_CLK_SCI1      6
> +#define R7S72100_CLK_SNDGEN0   5
> +#define R7S72100_CLK_SNDGEN1   4
> +#define R7S72100_CLK_SNDGEN2   3
> +#define R7S72100_CLK_SNDGEN3   2

R7S72100_CLK_SG[0-3]?

>  #define R7S72100_CLK_OSTM0     1
>  #define R7S72100_CLK_OSTM1     0
>
>  /* MSTP6 */
> +#define R7S72100_CLK_ADC       7
> +#define R7S72100_CLK_CEU       6
> +#define R7S72100_CLK_DOC0      5
> +#define R7S72100_CLK_DOC1      4
> +#define R7S72100_CLK_DRC0      3
> +#define R7S72100_CLK_DRC1      2
> +#define R7S72100_CLK_JCU       1
>  #define R7S72100_CLK_RTC       0
>
>  /* MSTP7 */
> +#define R7S72100_CLK_VDEC0     7
> +#define R7S72100_CLK_VDEC1     6

R7S72100_CLK_VIN[01]?

>  #define R7S72100_CLK_ETHER     4
> +#define R7S72100_CLK_NAND      3
>  #define R7S72100_CLK_USB0      1
>  #define R7S72100_CLK_USB1      0
>
>  /* MSTP8 */
> +#define R7S72100_CLK_IMR0      7
> +#define R7S72100_CLK_IMR1      6
> +#define R7S72100_CLK_IMRDISP   5
>  #define R7S72100_CLK_MMCIF     4
> +#define R7S72100_CLK_MLB       3
> +#define R7S72100_CLK_ETHABV    2

R7S72100_CLK_ETHAVB

> +#define R7S72100_CLK_SCUX      1
>
>  /* MSTP9 */
>  #define R7S72100_CLK_I2C0      7
>  #define R7S72100_CLK_I2C1      6
>  #define R7S72100_CLK_I2C2      5
>  #define R7S72100_CLK_I2C3      4
> +#define R7S72100_CLK_SPIMBC0   3
> +#define R7S72100_CLK_SPIMBC1   2

R7S72100_CLK_SPB[0-1]?
All related registers and clocks are called SPB<something>.

> +#define R7S72100_CLK_VDC50     1       /* and LVDS */
> +#define R7S72100_CLK_VDC51     0
>
>  /* MSTP10 */
>  #define R7S72100_CLK_SPI0      7
> @@ -52,6 +85,9 @@
>  #define R7S72100_CLK_SPI2      5
>  #define R7S72100_CLK_SPI3      4
>  #define R7S72100_CLK_SPI4      3
> +#define R7S72100_CLK_CDROM     2
> +#define R7S72100_CLK_SPDIF     1
> +#define R7S72100_CLK_RGPVG2    0

No SSI (MSTP11[0-5])?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ARM: dts: r7s72100: add clock bit definitions
@ 2017-05-29  9:31     ` Geert Uytterhoeven
  0 siblings, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2017-05-29  9:31 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Simon Horman, Rob Herring, Mark Rutland, devicetree, Linux-Renesas

Hi Chris,

On Thu, May 25, 2017 at 6:05 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> Add the remaining bit locations for the module stop clock registers.
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/include/dt-bindings/clock/r7s72100-clock.h
> +++ b/include/dt-bindings/clock/r7s72100-clock.h
> @@ -13,7 +13,14 @@
>  #define R7S72100_CLK_PLL       0

No CoreSight (MSTP20)?

>  /* MSTP3 */
> +#define R7S72100_CLK_IEBUS     7
> +#define R7S72100_CLK_IRDA      6
> +#define R7S72100_CLK_LIN0      5
> +#define R7S72100_CLK_LIN1      4
>  #define R7S72100_CLK_MTU2      3
> +#define R7S72100_CLK_CAN       2
> +#define R7S72100_CLK_ADCPWR    1
> +#define R7S72100_CLK_MCPWM     0

Perhaps just R7S72100_CLK_PWM?

>  /* MSTP4 */
>  #define R7S72100_CLK_SCIF0     7
> @@ -26,25 +33,51 @@
>  #define R7S72100_CLK_SCIF7     0
>
>  /* MSTP5 */
> +#define R7S72100_CLK_SCI0      7
> +#define R7S72100_CLK_SCI1      6
> +#define R7S72100_CLK_SNDGEN0   5
> +#define R7S72100_CLK_SNDGEN1   4
> +#define R7S72100_CLK_SNDGEN2   3
> +#define R7S72100_CLK_SNDGEN3   2

R7S72100_CLK_SG[0-3]?

>  #define R7S72100_CLK_OSTM0     1
>  #define R7S72100_CLK_OSTM1     0
>
>  /* MSTP6 */
> +#define R7S72100_CLK_ADC       7
> +#define R7S72100_CLK_CEU       6
> +#define R7S72100_CLK_DOC0      5
> +#define R7S72100_CLK_DOC1      4
> +#define R7S72100_CLK_DRC0      3
> +#define R7S72100_CLK_DRC1      2
> +#define R7S72100_CLK_JCU       1
>  #define R7S72100_CLK_RTC       0
>
>  /* MSTP7 */
> +#define R7S72100_CLK_VDEC0     7
> +#define R7S72100_CLK_VDEC1     6

R7S72100_CLK_VIN[01]?

>  #define R7S72100_CLK_ETHER     4
> +#define R7S72100_CLK_NAND      3
>  #define R7S72100_CLK_USB0      1
>  #define R7S72100_CLK_USB1      0
>
>  /* MSTP8 */
> +#define R7S72100_CLK_IMR0      7
> +#define R7S72100_CLK_IMR1      6
> +#define R7S72100_CLK_IMRDISP   5
>  #define R7S72100_CLK_MMCIF     4
> +#define R7S72100_CLK_MLB       3
> +#define R7S72100_CLK_ETHABV    2

R7S72100_CLK_ETHAVB

> +#define R7S72100_CLK_SCUX      1
>
>  /* MSTP9 */
>  #define R7S72100_CLK_I2C0      7
>  #define R7S72100_CLK_I2C1      6
>  #define R7S72100_CLK_I2C2      5
>  #define R7S72100_CLK_I2C3      4
> +#define R7S72100_CLK_SPIMBC0   3
> +#define R7S72100_CLK_SPIMBC1   2

R7S72100_CLK_SPB[0-1]?
All related registers and clocks are called SPB<something>.

> +#define R7S72100_CLK_VDC50     1       /* and LVDS */
> +#define R7S72100_CLK_VDC51     0
>
>  /* MSTP10 */
>  #define R7S72100_CLK_SPI0      7
> @@ -52,6 +85,9 @@
>  #define R7S72100_CLK_SPI2      5
>  #define R7S72100_CLK_SPI3      4
>  #define R7S72100_CLK_SPI4      3
> +#define R7S72100_CLK_CDROM     2
> +#define R7S72100_CLK_SPDIF     1
> +#define R7S72100_CLK_RGPVG2    0

No SSI (MSTP11[0-5])?

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] 7+ messages in thread

* Re: [PATCH] ARM: dts: r7s72100: add clock bit definitions
  2017-05-29  9:31     ` Geert Uytterhoeven
  (?)
@ 2017-05-30  8:42     ` Simon Horman
  -1 siblings, 0 replies; 7+ messages in thread
From: Simon Horman @ 2017-05-30  8:42 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Chris Brandt, Rob Herring, Mark Rutland, devicetree, Linux-Renesas

On Mon, May 29, 2017 at 11:31:37AM +0200, Geert Uytterhoeven wrote:
> Hi Chris,
> 
> On Thu, May 25, 2017 at 6:05 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> > Add the remaining bit locations for the module stop clock registers.
> >
> > Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Chris, please address Geert's review and repost as appropriate.

> 
> > --- a/include/dt-bindings/clock/r7s72100-clock.h
> > +++ b/include/dt-bindings/clock/r7s72100-clock.h
> > @@ -13,7 +13,14 @@
> >  #define R7S72100_CLK_PLL       0
> 
> No CoreSight (MSTP20)?
> 
> >  /* MSTP3 */
> > +#define R7S72100_CLK_IEBUS     7
> > +#define R7S72100_CLK_IRDA      6
> > +#define R7S72100_CLK_LIN0      5
> > +#define R7S72100_CLK_LIN1      4
> >  #define R7S72100_CLK_MTU2      3
> > +#define R7S72100_CLK_CAN       2
> > +#define R7S72100_CLK_ADCPWR    1
> > +#define R7S72100_CLK_MCPWM     0
> 
> Perhaps just R7S72100_CLK_PWM?
> 
> >  /* MSTP4 */
> >  #define R7S72100_CLK_SCIF0     7
> > @@ -26,25 +33,51 @@
> >  #define R7S72100_CLK_SCIF7     0
> >
> >  /* MSTP5 */
> > +#define R7S72100_CLK_SCI0      7
> > +#define R7S72100_CLK_SCI1      6
> > +#define R7S72100_CLK_SNDGEN0   5
> > +#define R7S72100_CLK_SNDGEN1   4
> > +#define R7S72100_CLK_SNDGEN2   3
> > +#define R7S72100_CLK_SNDGEN3   2
> 
> R7S72100_CLK_SG[0-3]?
> 
> >  #define R7S72100_CLK_OSTM0     1
> >  #define R7S72100_CLK_OSTM1     0
> >
> >  /* MSTP6 */
> > +#define R7S72100_CLK_ADC       7
> > +#define R7S72100_CLK_CEU       6
> > +#define R7S72100_CLK_DOC0      5
> > +#define R7S72100_CLK_DOC1      4
> > +#define R7S72100_CLK_DRC0      3
> > +#define R7S72100_CLK_DRC1      2
> > +#define R7S72100_CLK_JCU       1
> >  #define R7S72100_CLK_RTC       0
> >
> >  /* MSTP7 */
> > +#define R7S72100_CLK_VDEC0     7
> > +#define R7S72100_CLK_VDEC1     6
> 
> R7S72100_CLK_VIN[01]?
> 
> >  #define R7S72100_CLK_ETHER     4
> > +#define R7S72100_CLK_NAND      3
> >  #define R7S72100_CLK_USB0      1
> >  #define R7S72100_CLK_USB1      0
> >
> >  /* MSTP8 */
> > +#define R7S72100_CLK_IMR0      7
> > +#define R7S72100_CLK_IMR1      6
> > +#define R7S72100_CLK_IMRDISP   5
> >  #define R7S72100_CLK_MMCIF     4
> > +#define R7S72100_CLK_MLB       3
> > +#define R7S72100_CLK_ETHABV    2
> 
> R7S72100_CLK_ETHAVB
> 
> > +#define R7S72100_CLK_SCUX      1
> >
> >  /* MSTP9 */
> >  #define R7S72100_CLK_I2C0      7
> >  #define R7S72100_CLK_I2C1      6
> >  #define R7S72100_CLK_I2C2      5
> >  #define R7S72100_CLK_I2C3      4
> > +#define R7S72100_CLK_SPIMBC0   3
> > +#define R7S72100_CLK_SPIMBC1   2
> 
> R7S72100_CLK_SPB[0-1]?
> All related registers and clocks are called SPB<something>.
> 
> > +#define R7S72100_CLK_VDC50     1       /* and LVDS */
> > +#define R7S72100_CLK_VDC51     0
> >
> >  /* MSTP10 */
> >  #define R7S72100_CLK_SPI0      7
> > @@ -52,6 +85,9 @@
> >  #define R7S72100_CLK_SPI2      5
> >  #define R7S72100_CLK_SPI3      4
> >  #define R7S72100_CLK_SPI4      3
> > +#define R7S72100_CLK_CDROM     2
> > +#define R7S72100_CLK_SPDIF     1
> > +#define R7S72100_CLK_RGPVG2    0
> 
> No SSI (MSTP11[0-5])?
> 
> 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] 7+ messages in thread

* RE: [PATCH] ARM: dts: r7s72100: add clock bit definitions
  2017-05-29  9:31     ` Geert Uytterhoeven
@ 2017-05-30 16:23         ` Chris Brandt
  -1 siblings, 0 replies; 7+ messages in thread
From: Chris Brandt @ 2017-05-30 16:23 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Simon Horman, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux-Renesas

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3224 bytes --]

Hi Geert,

As always Thank you for your review!


On Monday, May 29, 2017, Geert Uytterhoeven wrote:
> >  #define R7S72100_CLK_PLL       0
> 
> No CoreSight (MSTP20)?

OK, I'll add that in.


> > +#define R7S72100_CLK_MCPWM     0
> 
> Perhaps just R7S72100_CLK_PWM?

OK.
People don't seem to be using it for motor control anyway ;)


> > +#define R7S72100_CLK_SNDGEN0   5
> > +#define R7S72100_CLK_SNDGEN1   4
> > +#define R7S72100_CLK_SNDGEN2   3
> > +#define R7S72100_CLK_SNDGEN3   2
> 
> R7S72100_CLK_SG[0-3]?

I debated against 'SG' vs 'SNDGEN'.
Since you also suggested SG, I'll change it.

> >  /* MSTP7 */
> > +#define R7S72100_CLK_VDEC0     7
> > +#define R7S72100_CLK_VDEC1     6
> 
> R7S72100_CLK_VIN[01]?

These are analog video inputs (NTSC/PAL).
For R-Car, "VIN" seem to refer to parallel digital inputs (ie, rcar-vin 
driver)

So for the RZ/A series, I was going to keep with the convention that we 
use for the non-Linux sample code and app notes such that these inputs 
are 'analog video decoders' and not just video input/capture 
pins....hence vdec.


> > +#define R7S72100_CLK_ETHABV    2
> 
> R7S72100_CLK_ETHAVB

Opps. Thank you!


> > +#define R7S72100_CLK_SPIMBC0   3
> > +#define R7S72100_CLK_SPIMBC1   2
> 
> R7S72100_CLK_SPB[0-1]?

Here's the one that I'm struggling with what to call.

Internally, the IP block is referred to as the "SPIBSC". As in, SPI Bus 
State Controller (because basically anything that has a parallel 
interface to the internal bus the design guys call it a BSC: LBSC, DBSC, 
etc...).

However, for every device this IP is used in it is called the "SPI 
Multi I/O Bus Controller" in the Hardware Manual (SH7769, RZ/A1, R-Car Gen3, 
etc...).

So, that might be confusing to users.

Originally, it was called "SPI BSC" because they were only connecting a 
"SPI" bus interface as a "Bus State Controller". However, now they've 
added onto the IP and it does more than just SPI.


> All related registers and clocks are called SPB<something>.
Only the pins are labeled SPB<something>, not the registers. The 
registers doesn't really have a common prefix.


So in general, what's your opinion on what to call this thing (since 
I'd like to name the driver in a similar manner)?

(1) "SPIBSC": Because that's what all the non-Linux sample code and app 
     note refer to it.

(2) "SPIMBC": Taken from "SPI Multi I/O Bus Controller" (using all 1st 
    letters except the "I/O" part)

(3) "SPB": Because that is how they named the pins for RZ/1 (NOTE that
    is just for RZ/A1 and SH7269, for RZ/A2++ and R-Car they are labeled
    QSPI_xxx...so you will probably never see SPB again in future hardware
    manuals)

(4) "SBC": For Serial Bus Controller


I am leaning to just staying with "SPIBSC" which is what I use now for 
our current Linux BSP and non-Linux sample code, or "SBC" just to 
shorten it.

Any opinions???



> No SSI (MSTP11[0-5])?

Opps, I missed that register somehow.
I'll add them in.


Thank you,
Chris

N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·zøœzÚÞz)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢j.ïÛ°\½½MŽúgjÌæa×\x02››–' ™©Þ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾\a«‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] ARM: dts: r7s72100: add clock bit definitions
@ 2017-05-30 16:23         ` Chris Brandt
  0 siblings, 0 replies; 7+ messages in thread
From: Chris Brandt @ 2017-05-30 16:23 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Simon Horman, Rob Herring, Mark Rutland, devicetree, Linux-Renesas

Hi Geert,

As always Thank you for your review!


On Monday, May 29, 2017, Geert Uytterhoeven wrote:
> >  #define R7S72100_CLK_PLL       0
> 
> No CoreSight (MSTP20)?

OK, I'll add that in.


> > +#define R7S72100_CLK_MCPWM     0
> 
> Perhaps just R7S72100_CLK_PWM?

OK.
People don't seem to be using it for motor control anyway ;)


> > +#define R7S72100_CLK_SNDGEN0   5
> > +#define R7S72100_CLK_SNDGEN1   4
> > +#define R7S72100_CLK_SNDGEN2   3
> > +#define R7S72100_CLK_SNDGEN3   2
> 
> R7S72100_CLK_SG[0-3]?

I debated against 'SG' vs 'SNDGEN'.
Since you also suggested SG, I'll change it.

> >  /* MSTP7 */
> > +#define R7S72100_CLK_VDEC0     7
> > +#define R7S72100_CLK_VDEC1     6
> 
> R7S72100_CLK_VIN[01]?

These are analog video inputs (NTSC/PAL).
For R-Car, "VIN" seem to refer to parallel digital inputs (ie, rcar-vin 
driver)

So for the RZ/A series, I was going to keep with the convention that we 
use for the non-Linux sample code and app notes such that these inputs 
are 'analog video decoders' and not just video input/capture 
pins....hence vdec.


> > +#define R7S72100_CLK_ETHABV    2
> 
> R7S72100_CLK_ETHAVB

Opps. Thank you!


> > +#define R7S72100_CLK_SPIMBC0   3
> > +#define R7S72100_CLK_SPIMBC1   2
> 
> R7S72100_CLK_SPB[0-1]?

Here's the one that I'm struggling with what to call.

Internally, the IP block is referred to as the "SPIBSC". As in, SPI Bus 
State Controller (because basically anything that has a parallel 
interface to the internal bus the design guys call it a BSC: LBSC, DBSC, 
etc...).

However, for every device this IP is used in it is called the "SPI 
Multi I/O Bus Controller" in the Hardware Manual (SH7769, RZ/A1, R-Car Gen3, 
etc...).

So, that might be confusing to users.

Originally, it was called "SPI BSC" because they were only connecting a 
"SPI" bus interface as a "Bus State Controller". However, now they've 
added onto the IP and it does more than just SPI.


> All related registers and clocks are called SPB<something>.
Only the pins are labeled SPB<something>, not the registers. The 
registers doesn't really have a common prefix.


So in general, what's your opinion on what to call this thing (since 
I'd like to name the driver in a similar manner)?

(1) "SPIBSC": Because that's what all the non-Linux sample code and app 
     note refer to it.

(2) "SPIMBC": Taken from "SPI Multi I/O Bus Controller" (using all 1st 
    letters except the "I/O" part)

(3) "SPB": Because that is how they named the pins for RZ/1 (NOTE that
    is just for RZ/A1 and SH7269, for RZ/A2++ and R-Car they are labeled
    QSPI_xxx...so you will probably never see SPB again in future hardware
    manuals)

(4) "SBC": For Serial Bus Controller


I am leaning to just staying with "SPIBSC" which is what I use now for 
our current Linux BSP and non-Linux sample code, or "SBC" just to 
shorten it.

Any opinions???



> No SSI (MSTP11[0-5])?

Opps, I missed that register somehow.
I'll add them in.


Thank you,
Chris


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ARM: dts: r7s72100: add clock bit definitions
  2017-05-30 16:23         ` Chris Brandt
  (?)
@ 2017-06-01  8:22         ` Geert Uytterhoeven
  -1 siblings, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2017-06-01  8:22 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Simon Horman, Rob Herring, Mark Rutland, devicetree, Linux-Renesas

Hi Chris,

On Tue, May 30, 2017 at 6:23 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote:
>> > +#define R7S72100_CLK_SPIMBC0   3
>> > +#define R7S72100_CLK_SPIMBC1   2
>>
>> R7S72100_CLK_SPB[0-1]?
>
> Here's the one that I'm struggling with what to call.
>
> Internally, the IP block is referred to as the "SPIBSC". As in, SPI Bus
> State Controller (because basically anything that has a parallel
> interface to the internal bus the design guys call it a BSC: LBSC, DBSC,
> etc...).
>
> However, for every device this IP is used in it is called the "SPI
> Multi I/O Bus Controller" in the Hardware Manual (SH7769, RZ/A1, R-Car Gen3,
> etc...).
>
> So, that might be confusing to users.
>
> Originally, it was called "SPI BSC" because they were only connecting a
> "SPI" bus interface as a "Bus State Controller". However, now they've
> added onto the IP and it does more than just SPI.
>
>
>> All related registers and clocks are called SPB<something>.
> Only the pins are labeled SPB<something>, not the registers. The
> registers doesn't really have a common prefix.
>
>
> So in general, what's your opinion on what to call this thing (since
> I'd like to name the driver in a similar manner)?
>
> (1) "SPIBSC": Because that's what all the non-Linux sample code and app
>      note refer to it.
>
> (2) "SPIMBC": Taken from "SPI Multi I/O Bus Controller" (using all 1st
>     letters except the "I/O" part)
>
> (3) "SPB": Because that is how they named the pins for RZ/1 (NOTE that
>     is just for RZ/A1 and SH7269, for RZ/A2++ and R-Car they are labeled
>     QSPI_xxx...so you will probably never see SPB again in future hardware
>     manuals)
>
> (4) "SBC": For Serial Bus Controller
>
>
> I am leaning to just staying with "SPIBSC" which is what I use now for
> our current Linux BSP and non-Linux sample code, or "SBC" just to
> shorten it.
>
> Any opinions???

I'm fine with SPIBSC.

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] 7+ messages in thread

end of thread, other threads:[~2017-06-01  8:22 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-25 16:05 [PATCH] ARM: dts: r7s72100: add clock bit definitions Chris Brandt
     [not found] ` <20170525160548.103182-1-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
2017-05-29  9:31   ` Geert Uytterhoeven
2017-05-29  9:31     ` Geert Uytterhoeven
2017-05-30  8:42     ` Simon Horman
     [not found]     ` <CAMuHMdW21gwJZxCMsepX9c3hqpfw_+zc1x_f-6D0oKFVvqY2Kg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-05-30 16:23       ` Chris Brandt
2017-05-30 16:23         ` Chris Brandt
2017-06-01  8:22         ` Geert Uytterhoeven

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.