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@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