Hi Geert-san,
> From: Geert Uytterhoeven, Sent: Wednesday, October 30, 2019 5:30 PM
<snip>
> > > Your development and reviewing effort to reach v4 is very appreciated.
> > >
> > > However, in the context of some internal reviews of this patch, we are
> > > having hard times reconciling the change with our (possibly incomplete
> > > or inaccurate) interpretation of the R-Car3 HW User’s Manual (Rev.2.00
> > > Jul 2019). The latter says in
> > > Chapter "54. PCIE Controller" / "(2) Initial Setting of PCI Express":
> > >
> > > ----snip----
> > > Be sure to write the initial value (= H'80FF 0000) to MACCTLR before
> > > enabling PCIETCTLR.CFINIT.
> > > ----snip----
> > >
> > > Is my assumption correct that the description of this patch is a
> > > rewording of the above quote from the manual <snip>
> >
> > You are correct. Since the reset value of MACCTLR is H'80FF 0001, I thought
> > clearing the LSB bit was enough.
> > However, as your situation, I think I should have described the above quote
> > from the manual and have such a code (writing the value instead of clearing
> > the LSB only).
> >
> > > If it is only the LSB which "should be written to 0 before enabling
> > > PCIETCTLR.CFINIT", would you agree that the statement quoted from the
> > > manual would better be rephrased appropriately? TIA.
> >
> > As I mentioned above, I think the above quote from the manual is better
> > than rephrased.
> >
> > Sergei, Geert-san, I think we should revert this patch and fix code/commit
> > log to follow the manual. What do you think?
>
> The initial value mentioned in the manual makes sense to me.
> Of course when using that, #defines should be added for bits used, to
> avoid writing the magical value "0x80ff0001".
Thank you for your reply! So, I'll submit two patches (revert it at first
and then fix again) later.
> Initially, the "ff" part worried me. Fortunately some archaeology learned
> me that these bits where called "NFTS" in the SH7786 Hardware User's
> Manual, and used to specify the number of Fast Training Sequences to
> be transferred when the MAC returns from L0 to L0s (6--255).
>
> arch/sh/drivers/pci/pcie-sh7786.c seems to be aware of this:
>
> /*
> * Set fast training sequences to the maximum 255,
> * and enable MAC data scrambling.
> */
> data = pci_read_reg(chan, SH4A_PCIEMACCTLR);
> data &= ~PCIEMACCTLR_SCR_DIS;
> data |= (0xff << 16);
> pci_write_reg(chan, data, SH4A_PCIEMACCTLR);
I didn't know that..
> No idea why this was deemed not-to-be-modified by the user later
> (as of R-Car H1).
Same here...
Best regards,
Yoshihiro Shimoda
> 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
