netdev - Re: RE: RE: [PATCH net-next 07/13] net: fec: fec_probe(): update quirk: bring IRQs in correct order

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZxEZR2lmIbX6+xX2@lizhi-Precision-Tower-5810>
Date: Thu, 17 Oct 2024 10:03:51 -0400
From: Frank Li <Frank.li@....com>
To: Marc Kleine-Budde <mkl@...gutronix.de>
Cc: Wei Fang <wei.fang@....com>, Shenwei Wang <shenwei.wang@....com>,
	Clark Wang <xiaoning.wang@....com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Richard Cochran <richardcochran@...il.com>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"kernel@...gutronix.de" <kernel@...gutronix.de>
Subject: Re: RE: RE: [PATCH net-next 07/13] net: fec: fec_probe(): update
 quirk: bring IRQs in correct order

On Thu, Oct 17, 2024 at 12:21:10PM +0200, Marc Kleine-Budde wrote:
> On 17.10.2024 07:43:55, Wei Fang wrote:
> > > > > Subject: [PATCH net-next 07/13] net: fec: fec_probe(): update quirk: bring
> > > IRQs
> > > > > in correct order
> > > > >
> > > > > With i.MX8MQ and compatible SoCs, the order of the IRQs in the device
> > > > > tree is not optimal. The driver expects the first three IRQs to match
> > > > > their corresponding queue, while the last (fourth) IRQ is used for the
> > > > > PPS:
> > > > >
> > > > > - 1st IRQ: "int0": queue0 + other IRQs
> > > > > - 2nd IRQ: "int1": queue1
> > > > > - 3rd IRQ: "int2": queue2
> > > > > - 4th IRQ: "pps": pps
> > > > >
> > > > > However, the i.MX8MQ and compatible SoCs do not use the
> > > > > "interrupt-names" property and specify the IRQs in the wrong order:
> > > > >
> > > > > - 1st IRQ: queue1
> > > > > - 2nd IRQ: queue2
> > > > > - 3rd IRQ: queue0 + other IRQs
> > > > > - 4th IRQ: pps
> > > > >
> > > > > First rename the quirk from FEC_QUIRK_WAKEUP_FROM_INT2 to
> > > > > FEC_QUIRK_INT2_IS_MAIN_IRQ, to better reflect it's functionality.
> > > > >
> > > > > If the FEC_QUIRK_INT2_IS_MAIN_IRQ quirk is active, put the IRQs back
> > > > > in the correct order, this is done in fec_probe().
> > > > >
> > > >
> > > > I think FEC_QUIRK_INT2_IS_MAIN_IRQ or FEC_QUIRK_WAKEUP_FROM_INT2
> > > > is *NO* needed anymore. Actually, INT2 is also the main IRQ for i.MX8QM
> > > and
> > > > its compatible SoCs, but i.MX8QM uses a different solution. I don't know
> > > why
> > > > there are two different ways of doing it, as I don't know the history. But you
> > > can
> > > > refer to the solution of i.MX8QM, which I think is more suitable.
> > > >
> > > > See arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi, the IRQ 258 is
> > > > placed first.
> > >
> > > Yes, that is IMHO the correct description of the IP core, but the
> > > i.MX8M/N/Q DTS have the wrong order of IRQs. And for compatibility
> > > reasons (fixed DTS with old driver) it's IMHO not possible to change the
> > > DTS.
> > >
> >
> > I don't think it is a correct behavior for old drivers to use new DTBs or new
> > drivers to use old DTBs. Maybe you are correct, Frank also asked the same
> > question, let's see how Frank responded.
>
> DTBs should be considered stable ABI.
>

ABI defined at binding doc.
  interrupt-names:
    oneOf:
      - items:
          - const: int0
      - items:
          - const: int0
          - const: pps
      - items:
          - const: int0
          - const: int1
          - const: int2
      - items:
          - const: int0
          - const: int1
          - const: int2
          - const: pps

DTB should align binding doc. There are not 'descriptions' at 'interrupt',
which should match 'interrupt-names'. So IMX8MP dts have not match ABI,
which defined by binding doc. So it is DTS implement wrong.

Frank

> regards,
> Marc
>
> --
> Pengutronix e.K.                 | Marc Kleine-Budde          |
> Embedded Linux                   | https://www.pengutronix.de |
> Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |