Re: [PATCH v2 0/2] PCI: xgene: Restore working PCIe functionnality

[Marc Zyngier]

On Mon, 21 Mar 2022 18:03:27 +0000,
Rob Herring <robh@xxxxxxxxxx> wrote:
> 
> On Mon, Mar 21, 2022 at 11:36 AM Marc Zyngier <maz@xxxxxxxxxx> wrote:
> >
> > On Mon, 21 Mar 2022 15:17:34 +0000,
> > Rob Herring <robh@xxxxxxxxxx> wrote:
> > >
> > > On Mon, Mar 21, 2022 at 5:49 AM Marc Zyngier <maz@xxxxxxxxxx> wrote:
> > > >
> > > For XGene-1, I'd still like to understand what the issue is. Reverting
> > > the first fix and fixing 'dma-ranges' should have fixed it. I need a
> > > dump of how the IB registers are initialized in both cases. I'm not
> > > saying changing 'dma-ranges' in the firmware is going to be required
> > > here. There's a couple of other ways we could fix that without a
> > > firmware change, but first I need to understand why it broke.
> >
> > Reverting 6dce5aa59e0b was enough for me, without changing anything
> > else.
> 
> Meaning c7a75d07827a didn't matter for you. I'm not sure that it would.
> 
> Can you tell me what 'dma-ranges' contains on your system?

Each pcie node (all 5 of them) has:

dma-ranges = <0x42000000 0x80 0x00 0x80 0x00 0x00 0x80000000
              0x42000000 0x00 0x00 0x00 0x00 0x80 0x00>;

> 
> > m400 probably uses an even older firmware (AFAIR, it was stuck
> > with an ancient version of u-boot that HP never updated, while Mustang
> > had a few updates). In any case, that DT cannot be changed.
> 
> How is Dann changing it then? I assume he's not changing the firmware,
> but overriding it. That could be a possible solution.

I don't know about you, but changing DT is not an acceptable solution
for me. If I'm bisecting something and have to pick the right DT based
on the kernel revision I'm using, that's a huge regression. And I'm
not even mentioning the poor sod who simply updates their distro, only
to find that the box doesn't boot anymore thanks to a kernel upgrade.

We're not talking about a closed embedded device here, but a fully
functional desktop/server box that still run rings around your average
RPi.

> Do the DT's in the kernel tree correspond to anything anyone is using?
> I ask because at some point someone will need to address all the
> warnings they have or we should drop the dts files if they aren't
> close to reality. The same thing applies to Seattle BTW.

I'd be perfectly happy to see both of these go. The last time I used
the kernel DT on my Seattle was some time in 2015, at which point I
got a firmware correctly describing the SMMUs. Oh, and ACPI works just
fine on Seattle.

> > > P.S. We're carrying ACPI and DT support for these platforms. It seems
> > > the few users are using DT, so can we drop the ACPI support? Or do I
> > > need to break it first and wait a year? ;)
> >
> > I'm not sure people on the list are representative of all the users,
> > and I didn't realise the plan was "let's break everything we don't
> > like and see if someone wakes up" either. That definitely puts things
> > in a different perspective.
> 
> I wasn't really suggesting breaking things on purpose. However, there
> is a cost to keeping code and it would be nice to know what's being
> used or not. The cost isn't *that* big, but it is not zero for what's
> not many users.
> 
> At least for Stéphane, using ACPI didn't even work. I'm assuming there
> is some version of h/w and f/w out there that did work with the ACPI
> support in the kernel? That may have never been seen by anyone but APM
> and Jon Masters (his Tested-by is on the patch from APM adding ACPI
> support). It's not hard to imagine there was a version of firmware
> just to shut Jon up.

Well, you can ask Jon. ACPI doesn't work on my box as it doesn't
(properly) describe a console, but my FW is rather old, as explained
in the release notes:

    Version 3.06.25 (10/17/2016)
    Mustang Tianocore (UEFI BIOS) Firmware Features:
     - TianoCore tag linaro-edk2-2014.07 with custom changes

Yes, 2014. Good vintage. But is that the latest and greatest? I have
no idea, and the APM website is long gone.

	M.

-- 
Without deviation from the norm, progress is not possible.