Re: [PATCH v4] PCI: rcar: Fix missing MACCTLR register setting in rcar_pcie_hw_init()

[Eugeniu Rosca <erosca@de.adit-jv.com>]

Dear Shimoda-san, dear reviewers,

On Fri, Oct 11, 2019 at 01:50:32PM +0900, Yoshihiro Shimoda wrote:
> According to the R-Car Gen2/3 manual, the bit 0 of MACCTLR register
> should be written to 0 before enabling PCIETCTLR.CFINIT because
> the bit 0 is set to 1 on reset. To avoid unexpected behaviors from
> this incorrect setting, this patch fixes it.

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 or is there another more
precise statement referring to resetting LSB only (w/o touching the
rest of the MACCTLR bits)?

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.

> 
> Fixes: c25da4778803 ("PCI: rcar: Add Renesas R-Car PCIe driver")
> Fixes: be20bbcb0a8c ("PCI: rcar: Add the initialization of PCIe link in resume_noirq()")
> Cc: <stable@vger.kernel.org> # v5.2+
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

-- 
Best Regards,
Eugeniu