[RFC RFT PATCH v1 0/1] ARM: orion5x: convert D-Link DNS-323 to the Device Tree

[Pali Rohár]

On Monday 09 May 2022 13:48:53 Mauri Sandberg wrote:
> On 8.5.2022 18.41, Pali Rohár wrote:
> > On Sunday 08 May 2022 17:22:37 Pali Rohár wrote:
> >> On Sunday 08 May 2022 17:02:17 Arnd Bergmann wrote:
> >>> On Sun, May 8, 2022 at 4:06 PM Mauri Sandberg <maukka at ext.kapsi.fi> wrote:
> >>>> On 28.4.2022 23.56, Arnd Bergmann wrote:
> >>>>> On Thu, Apr 28, 2022 at 10:01 PM Mauri Sandberg <maukka at ext.kapsi.fi> wrote:
> >>>>>> On 27.4.2022 21.10, Arnd Bergmann wrote:
> >>>>>>> On Wed, Apr 27, 2022 at 6:21 PM Mauri Sandberg <maukka at ext.kapsi.fi> wrote:
> >>>>>>>> - sata_mv fails to initialise with -22 (-EINVAL)
> >>>>>>>
> >>>>>>> No idea, I'd try inserting a printk in every code path that can return -EINVAL
> >>>>>>> from there
> >>>>>>>
> >>>>
> >>>> With debugging the reason for -EINVAL remains a bit mystery.
> >>>>   - sata_mv calls ata_host_activate() [1]
> >>>>   - later on, in request_threaded_irq(), there are sanity checks [2]
> >>>>   - that fail with irq_settings_can_request() returning 0 [3]
> >>>>
> >>>> I cannot really put my finger on why the irq cannot be requested in DT
> >>>> approach.
> >>>
> >>> Are you sure the marvell,orion-intc driver is successfully probed
> >>> at this point? If not, the interrupt won't be there.
> >>>
> >>> I see that the "sata_mv" driver can be used either as a platform
> >>> driver for the orion5x on-chip controller, or as a PCI driver for
> >>> an add-on chip connected to the external bus. It sounds like
> >>> your system has both. Do you know which one fails?
> >>>
> >>> The PCI driver cannot work unless the PCI host works correctly,
> >>> and that in turn requires a correct devicetree description for it.
> >>>
> >>>>>> Is there a way to describe the PCIe bus in the
> >>>>>> device tree? The initalisation of that bus is done for rev A1 only.
> >>>>>
> >>>>> I'm not too familiar with the platform, but my interpretation is that the
> >>>>> DT support here is incomplete:
> >>>>>
> >>>>> The DT based PCI probe using drivers/pci/controller/pci-mvebu.c
> >>>>> is not hooked up in orion5x.dtsi, and the traditional pci code does
> >>>>> not work with DT.
> >>>>
> >>>> Can the existing pci code still be used to init the PCI bus and describe
> >>>> the rest in the DT or is it a futile attempt?
> >>
> >> Hello! Orion uses arch/arm/mach-orion5x/pci.c driver for both PCI and
> >> PCIe buses. This is not device tree driver.
> > 
> > Correction, Orion PCIe driver is arch/arm/plat-orion/pcie.c and it calls
> > common functions from mach-orion5x/pci.c driver.
> > 
> >>>>> I see that orion5x has two separate blocks --  a PCIe host that is
> >>>>> similar to the kirkwood one, and a legacy PCI host that needs
> >>>>> a completely separate driver.
> >>>>>
> >>>>> Which of the two do you actually need here?
> >>>>>
> >>>>
> >>>> I really cannot say which one is it. How can I tell? The functions given
> >>>> in struct hw_pci find their way to drivers/pci/probe.c eventually and
> >>>> use pci_scan_root_bus_bridge(). Nothing seems to utilising mvebu or
> >>>> kirkwood explicitly at least.
> >>>>
> >>>> Here's the output from lspci if the ids reveal anything.
> >>>>
> >>>> # lspci -v -k
> >>>> 00:00.0 Class 0580: 11ab:5181
> >>>> 01:00.0 Class 0580: 11ab:5181
> >>>> 00:01.0 Class 0100: 11ab:7042 sata_mv
> >>>
> >>> The first two seem to be the host bridges, but unfortunately they
> >>>  seem both have the same device ID, despite being very different
> >>> devices.  The first one (00:00.0) should be the PCIe driver, the
> >>> second one (01.00.0) the legacy PCI one. In this case, the 11ab:7042
> >>> device is a PCIe device, and it's on the bus (00) of the first host
> >>> bridge. I think this should work with drivers/pci/controller/pci-mvebu.c
> >>> if you add the bits for probing.
> >>
> >> Last time when I looked on Orion PCIe controller registers, I though
> >> that they are same as in Kirkwood PCIe controller registers. And
> >> Kirkwood is already supported by pci-mvebu.c driver.
> >>
> >> About PCI host bridge, I do not know.
> >>
> >> Beware that PCI Class Id and all PCI registers which are different for
> >> Type 0 and Type 1 are _broken_ on all PCIe Root Ports form all 32-bit
> >> Marvell SoCs. Those registers on Marvell SoCs have different meaning as
> >> what is defined in PCI and PCIe specs. So it means that lspci _may_
> >> display bogus information about PCIe Root Port. pci-mvebu.c uses Root
> >> Port emulator which fills correct data to make kernel and lspci happy.
> >>
> >> If you are going to extend pci-mvebu.c to support also Orion PCIe
> >> controller, I could try to help with it. But I do not have any Orion
> >> hardware, so just basic help...
> >>
> >> Links to Orion documentations, including PCIe errata is available in
> >> kernel documentation. So this could help to understand some details:
> >> https://www.kernel.org/doc/html/latest/arm/marvell.html
> >>
> >> Anyway, could you please provide 'lspci -nn -vv' and 'lspci -nn -t -v'
> >> outputs from Orion?
> >>
> >>> Thomas Petazzoni originally wrote the new driver, and I think he was
> >>> planning at one point to use it for orion5x. I don't know if there were
> >>> any major problems preventing this at the time, or if it just needs to
> >>> get hooked up in the dtsi file.
> >>>
> >>>          Arnd
> > 
> > There is Orion-specific errata that config space via CF8/CFC registers
> > is broken. Workaround documented in errata documented (linked from above
> > documentation) does not work when DMA is used and instead other
> > undocumented workaround is needed (implemented in arch/arm) which maps
> > config space to memory (and therefore avoids usage of broken CF8/CFC
> > memory mapped registers).
> 
> So basically I should look at arch/arm/plat-orion/pcie.c for the
> configuration part, add new compatible to pci-mvebu.c for orion5x
> and alter the probing function accoringly for the same. Did I get
> it correctly?

You would need to replace mvebu_pcie_child_rd_conf() and
mvebu_pcie_child_wr_conf() implementation in pci-mvebu.c with the
correct orion implementation. Orion implementation is in function
pcie_rd_conf_wa() or pcie_rd_conf() (file arch/arm/mach-orion5x/pci.c)
based on the fact if workaround has to be applied or not. Same for *wr*
functions.

Workaround uses wa_base address, which needs to be mapped via mbus
driver. This is something new which is not supported by pci-mvebu.c. And
wa_base address should be correctly specified in ranges= property of
type "configuration space" (ss = 00). See:
https://elinux.org/Device_Tree_Usage#PCI_Address_Translation

> If so, sounds simple when said out lout but I might need some more
> pointers to get started. Like with configuration people generally
> mean setting BARs and WINs? Or is there more to it? :) If you
> could lay out the basic steps that are needed I would really
> appreciate it.

BARs and WINs are configured by pci-mvebu.c. You just have specify
correct ids in DTS. See usage of MBUS_ID() macro and Port X IO/MEM
comments e.g. in arch/arm/boot/dts/armada-385.dtsi file. Also it is
required to set pcie-mem-aperture and pcie-io-aperture properties, see
e.g. in arch/arm/boot/dts/armada-38x.dtsi file.

I'm not sure if this is clear for you. If not, please ask additional
questions :-)