RE: [PATCH v2] PCI: rcar: Fix writing the MACCTLR register value

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

 



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
> <yoshihiro.shimoda.uh@xxxxxxxxxxx> 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: <stable@xxxxxxxxxxxxxxx> # v4.9+
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> > Reviewed-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>
> 
> Thanks for your patch!
> 
> This patch fixes the issue where the register is written, so
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

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




[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