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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Eugeniu-san,

Thank you for your comments!

> From: Eugeniu Rosca, Sent: Tuesday, October 29, 2019 11:38 PM
> 
> 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 <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?

Best regards,
Yoshihiro Shimoda

> >
> > 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@xxxxxxxxxxxxxxx> # v5.2+
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> > Reviewed-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> 
> --
> Best Regards,
> Eugeniu




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux