RE: [PATCH] ARM: dts: r7s72100: add clock bit definitions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux