Hi Geert-san.

> From: Geert Uytterhoeven, Sent: Wednesday, October 9, 2019 5:58 PM
> 
> Hi Shimoda-san,
> 
> On Wed, Oct 9, 2019 at 6:03 AM Yoshihiro Shimoda
> <[email protected]> wrote:
> > According to the R-Car Gen2/3 manual, the bit 0 of MACCTLR register
> > should be written to 0 because the register is set to 1 on reset.
> > To avoid unexpected behaviors from this incorrect setting, this
> > patch fixes it.
> >
> > Fixes: b3327f7fae66 ("PCI: rcar: Try increasing PCIe link speed to 5 GT/s 
> > at boot")
> > Cc: <[email protected]> # v4.9+
> > Signed-off-by: Yoshihiro Shimoda <[email protected]>
> > Reviewed-by: Sergei Shtylyov <[email protected]>
> 
> Thanks for your patch!
> 
> This patch fixes the issue where the register is written, so
> Reviewed-by: Geert Uytterhoeven <[email protected]>

Thank you for your review!

> However, according to the R-Car H1, Gen2, and Gen3 Hardware User's
> Manuals, this reserved bit should be cleared on initialization.
> Are we sure that is guaranteed to happen? If the checks at the top of
> rcar_pcie_force_speedup() trigger, the register is never written to,
> and the bit may still be set?

Thank you for the pointed it out! As you said, this driver should set
the bit 0 of register on rcar_pcie_hw_init(), not rcar_pcie_force_speedup().
I checked that the bit 0t of register keep to be 0 after the driver
cleared the bit. So, clearing the bit 0 on rcar_pcie_hw_init() only
is enough, I think. So, I'll send v3 patch.

Best regards,
Yoshihiro Shimoda


> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> [email protected]
> 
> 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

Reply via email to