[PATCH v4 2/9] usb: xhci: Use only 32-bit accesses in xhci_writeq/xhci_readq

[Bin Meng]

Hi Matthias,

On Wed, May 27, 2020 at 8:20 PM Matthias Brugger <mbrugger at suse.com> wrote:
>
>
>
> On 26/05/2020 10:07, Bin Meng wrote:
> > Hi Simon,
> >
> > On Tue, May 26, 2020 at 5:41 AM Simon Glass <sjg at chromium.org> wrote:
> >>
> >> Hi Sylwester,
> >>
> >> On Mon, 25 May 2020 at 11:42, Sylwester Nawrocki <s.nawrocki at samsung.com> wrote:
> >>>
> >>> Hi Simon,
> >>>
> >>> On 25.05.2020 19:04, Simon Glass wrote:
> >>>> On Mon, 25 May 2020 at 10:57, Sylwester Nawrocki <s.nawrocki at samsung.com> wrote:
> >>>>> On 25.05.2020 16:57, Simon Glass wrote:
> >>>>>> On Mon, 25 May 2020 at 05:40, Sylwester Nawrocki <s.nawrocki at samsung.com> wrote:
> >>>>>>>
> >>>>>>> There might be hardware configurations where 64-bit data accesses
> >>>>>>> to XHCI registers are not supported properly.  This patch removes
> >>>>>>> the readq/writeq so always two 32-bit accesses are used to read/write
> >>>>>>> 64-bit XHCI registers, similarly as it is done in Linux kernel.
> >>>>>>>
> >>>>>>> This patch fixes operation of the XHCI controller on RPI4 Broadcom
> >>>>>>> BCM2711 SoC based board, where the VL805 USB XHCI controller is
> >>>>>>> connected to the PCIe Root Complex, which is attached to the system
> >>>>>>> through the SCB bridge.
> >>>>>>>
> >>>>>>> Even though the architecture is 64-bit the PCIe BAR is 32-bit and likely
> >>>>>>> the 64-bit wide register accesses initiated by the CPU are not properly
> >>>>>>> translated to a sequence of 32-bit PCIe accesses.
> >>>>>>> xhci_readq(), for example, always returns same value in upper and lower
> >>>>>>> 32-bits, e.g. 0xabcd1234abcd1234 instead of 0x00000000abcd1234.
> >>>>>
> >>>>>> Then I think this should be done with a quirk flag, enabled for this
> >>>>>> particular device via the compatible string. It should not be an #if,
> >>>>>> but an if().
> >>>>>
> >>>>> Thanks for your comments. I will check and see how this could be done.
> >>>>> It might not be so straightforward since the XHCI controller is a PCI
> >>>>> device matched by the pci_device_id so we would need to be looking
> >>>>> at the compatible string of the PCI controller to set the quirk in
> >>>>> the xhci layer. It's the PCI bridge that introduces the limitation,
> >>>>> not the VL805 XHCI controller chip.
> >>>>
> >>>> OK then it should be modelled as such.
> >>>>
> >>>> How is this done in Linux?
> >>>
> >>> In Linux simply always two 32-bit accesses are used for 64-bit registers
> >>> read/write.
> >
> > This was discussed during review of the previous version of this
> > patch, and I think aligning to what Linux does is fine. Previously we
> > discussed adding an Kconfig option to control this, but I feel that's
> > not good. Having a quirk flag to detect this is a dynamic approach,
> > compared to the static Kconfig option, but overall I don't see a need
> > not to align with Linux xHCI driver.
> >
>
> My understanding is, that we will keep the approach of 32-bit accesses. My plan
> was to take the whole series through my treen.
>
> Bin is that fine with you?

No problem. Thanks!

Regards,
Bin