Hello Niklas,
On Thu, Jul 11, 2024 at 04:21:34PM +0200, Niklas Cassel wrote:
> On Wed, Jul 03, 2024 at 12:00:34PM +0200, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@xxxxxxxxxxx>
> >
> > Fix PCIe PERST# signal polarity in TI J721E used on TI K3 machines.
> >
> > PCIe PERST# needs to be de-asserted for PCIe to work, however, the driver is
> > doing the opposite and the device tree files are defining the signal with the
> > wrong polarity to cope with that. Fix both the driver and the affected DT
> > files.
>
> While I understand why you want to fix this,
> I'm not sure if you can actually do so without breaking device tree backwards
> compatibility.
I understand this, and at the same time I know that this was done in the
past for exactly the same reason, see for example commit 87620512681a
("PCI: apple: Fix PERST# polarity").
This patch was send not because the issue was noticed analyzing the
code, but because during a bring-up of a new platform (based on
k3-j784s4) using this PCIe controller driver the PCIe was not working
and this lead to some time consuming debugging on both the
hardware/software before finding this issue. That was worked around just
by describing the HW incorrectly in the DT (the device tree of this
board is not in mainline - yet).
With that said I cannot 100% judge the exact impact, I know most (but
not all) of the boards and I think that making the change is beneficial
despite what you correctly write.
Most of the boards affected are from Texas Instruments (eval boards),
plus one beagle and one board from Siemens. Let's see what these folks
think about this change, these boards are all relatively recent.
> Perhaps you could add a comment in the driver and the DTS files explaining
> that the DTS is actually wrong, but cannot be changed because of DT backwards
> compatibility.
As I wrote my concern is on new boards.
BTW, the RS485 polarity for the UART used on all TI platform (including
the very old ones) have a similar bug [1], however this bug is so old and
deep into the code that we'll have to live with it.
[1] https://lore.kernel.org/all/ZBItlBhzo+YETcJO@xxxxxxxxxxxxxxxxxxxxxxxxxxxx/
Francesco
