Hi Wolfram,
On Tue, Jun 28, 2022 at 1:08 PM Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
> > > 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.
In case of DEF_GEN3_SD(), we have a register offset parameter because
most affected SoCs have multiple SDHI instances, and multiple registers
to control their clocks.
> > > 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.
OK, so let's drop this.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
