Re: [PATCH] clk: renesas: rcar-gen4: implement SDSRC properly

[Wolfram Sang]

Hi Geert,

> > Tested on my Spider board (r8a779f0). Only build tested for r8a779g0 but
> > the docs for the registers are the same.
> 
> While the SDSRCSEL bits are the same, the register at offset 0x8a4 is
> called SD0CKCR1 on R-Car S4-8, and CKSRCSELCR on R-Car V4H.
> I guess that is why you removed the definition of SD0CKCR1, and stored
> the register offset in DEF_GEN4_SDSRC(), despite both being the same?

TBH, no :) I did that to be future proof in case the register gets moved
somewhere else. Also, this is consistent how we did it with DEF_GEN3_SD.

> >         case CLK_TYPE_GEN4_SDSRC:
> > -               div = ((readl(base + SD0CKCR1) >> 29) & 0x03) + 4;
> > +               value = (readl(base + core->offset) >> 29) & 3;
> > +               if (value) {
> > +                       div = value + 4;
> > +               } else {
> > +                       parent = clks[core->parent >> 16];
> > +                       if (IS_ERR(parent))
> > +                               return ERR_CAST(parent);
> > +                       div = 2;
> > +               }
> 
> So this gives the exact same divider of PLL5 before.
> 
> The clock diagram indeed shows different paths for value 0
> (PLL5 -> 1/2 -> 1/2) and values 1 and 2 (PLL5 -> {1/5 or 1/6}).
> But the textual description for SDSRC says "The SDSRC divider divides
> PLL5 output clock", matching the original code.
> 
> Do we have to complicate the code? ;-)
> I guess the clock diagram was based on the diagram for R-Car H3
> (which has two daisy-chained fixed 1/2 dividers), with the new 1/5
> and 1/6 dividers added.

We don't have to complicate the code unnecessarily. If you think the
diagram is flawed, then we can keep the current code. I changed the code
because I was confused when checking 'clk_summary' with the diagram and
wanted to make it proper to reduce my confusion.

My patches to enable eMMC on Spider have a significantly lower
throughput than the BSP, so this was the first step of trying to verify
things and get the clocks in shape.

If you call it superfluous, then we can drop it. No hard feelings here.

All the best,

   Wolfram

Attachment: signature.asc
Description: PGP signature