RE: [PATCH 0/7] towards QE support on ARM
From: Qiang Zhao
Date: Mon Oct 21 2019 - 22:24:29 EST
On Mon, Oct 22, 2019 at 6:11 AM Leo Li wrote
> -----Original Message-----
> From: Li Yang <leoyang.li@xxxxxxx>
> Sent: 2019å10æ22æ 6:11
> To: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>
> Cc: Timur Tabi <timur@xxxxxxxxxx>; Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx;
> linux-serial@xxxxxxxxxxxxxxx; Jiri Slaby <jslaby@xxxxxxxx>;
> linuxppc-dev@xxxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; Qiang
> Zhao <qiang.zhao@xxxxxxx>
> Subject: Re: [PATCH 0/7] towards QE support on ARM
>
> On Mon, Oct 21, 2019 at 3:46 AM Rasmus Villemoes
> <linux@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > On 18/10/2019 23.52, Li Yang wrote:
> > > On Fri, Oct 18, 2019 at 3:54 PM Rasmus Villemoes
> > > <linux@xxxxxxxxxxxxxxxxxx> wrote:
> > >>
> > >> On 18/10/2019 22.16, Leo Li wrote:
> > >>>
> > >>>>
> > >>>> There have been several attempts in the past few years to allow
> > >>>> building the QUICC engine drivers for platforms other than PPC.
> > >>>> This is (the beginning of) yet another attempt. I hope I can get
> > >>>> someone to pick up these relatively trivial patches (I _think_
> > >>>> they shouldn't change functionality at all), and then I'll
> > >>>> continue slowly working towards removing the PPC32 dependency for
> CONFIG_QUICC_ENGINE.
> > >>>
> > >>> Hi Rasmus,
> > >>>
> > >>> I don't fully understand the motivation of this work. As far as I know
> the QUICC ENGINE is only used on PowerPC based SoCs.
> > >>
> > >> Hm, you're not the Leo Li that participated in this thread
> > >>
> <https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ke
> rnel.org%2Flkml%2FAM3PR04MB11857AE8D2B0BE56121B97D391C90%40A
> M3PR04MB1185.eurprd04.prod.outlook.com%2FT%2F%23u&data=02%7
> C01%7Cqiang.zhao%40nxp.com%7C1ba8c50c2db14b22bef608d756739d82%
> 7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6370729268771788
> 75&sdata=k4zM75OczXwZF%2Br9ec4RxiVR2a%2F8GhSZmK70JYddIck%3
> D&reserved=0>?
> > >
> > > Oops, I totally forgot about this discussion which is just three
> > > years ago. :) The QE-HDLC on LS1021a is kind of a special case.
> > >
> > >>
> > >>
> > >> Can you give an example on how is it used on ARM system?
> > >>
> > >> LS1021A, for example, which is the one I'm aiming for getting fully
> > >> supported in mainline.
> > >> <https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > >>
> www.nxp.com%2Fproducts%2Fprocessors-and-microcontrollers%2Farm-proc
> > >> essors%2Flayerscape-communication-process%2Fqoriq-layerscape-1021a-
> > >>
> dual-core-communications-processor-with-lcd-controller%3ALS1021A&am
> > >>
> p;data=02%7C01%7Cqiang.zhao%40nxp.com%7C1ba8c50c2db14b22bef608d
> 7567
> > >>
> 39d82%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6370729268
> 771788
> > >>
> 75&sdata=vqPYSZqEv6vCEIxJshLuA4gngh9J4IsFAQrTwJKMjm4%3D&r
> es
> > >> erved=0>
> > >>
> > >> The forks at
> > >>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
> com%2Fqoriq-open-source%2Flinux.git&data=02%7C01%7Cqiang.zhao%
> 40nxp.com%7C1ba8c50c2db14b22bef608d756739d82%7C686ea1d3bc2b4c6
> fa92cd99c5c301635%7C0%7C0%7C637072926877178875&sdata=v4eG
> 4KqGTWQkQHp%2FYg2OHCKITLWaOgH64JYpY%2B1LilA%3D&reserved=0
> have various degrees of support (grep for commits saying stuff like "remove
> PPCisms"
> > >> - some versions can be found on
> > >> <https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > >>
> lore.kernel.org%2Flkml%2F%3Fq%3Dremove%2Bppcisms&data=02%7C0
> 1%7
> > >>
> Cqiang.zhao%40nxp.com%7C1ba8c50c2db14b22bef608d756739d82%7C686e
> a1d3
> > >>
> bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637072926877178875&sdat
> a=i2WdKNHLV68a1mTOMQ%2FoMr0y5ee8edS07xq61M8%2BvPU%3D&r
> eserved=0>). Our current kernel is based on commits from the now-vanished
> 4.1 branch, and unfortunately at least the 4.14 branch (LSDK-18.06-V4.14)
> trivially doesn't build on ARM, despite the PPC32 dependency having been
> removed from CONFIG_QUICC_ENGINE.
> > >
> > > Can you try the 4.14 branch from a newer LSDK release? LS1021a
> > > should be supported platform on LSDK. If it is broken, something is
> wrong.
> >
> > What newer release? LSDK-18.06-V4.14 is the latest -V4.14 tag at
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> >
> ub.com%2Fqoriq-open-source%2Flinux.git&data=02%7C01%7Cqiang.zha
> o%4
> >
> 0nxp.com%7C1ba8c50c2db14b22bef608d756739d82%7C686ea1d3bc2b4c6f
> a92cd99c
> >
> 5c301635%7C0%7C0%7C637072926877188868&sdata=vdm4qPoTzfIpXL
> mRrv17EW
> > noxG3n91qELMGqvRh9we4%3D&reserved=0, and identical to the
>
> That tree has been abandoned for a while, we probably should state that in the
> github. The latest tree can be found at
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.
> codeaurora.org%2Fexternal%2Fqoriq%2Fqoriq-components%2Flinux%2F&
> ;data=02%7C01%7Cqiang.zhao%40nxp.com%7C1ba8c50c2db14b22bef608d7
> 56739d82%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6370729
> 26877188868&sdata=NooBFUWnceTG2OF24pCuP0AYgKfr0Df%2BtrcCY6
> X6Dog%3D&reserved=0
>
> > linux-4.14 branch. And despite commit 4c33e2d0576b removing the PPC32
> > dependency from QUICC_ENGINE, it clearly hasn't been built on arm,
> > since back around v4.12, mainline's qe.c grew a call to pvr_version_is
> > which is ppc-only. So from that I sort of assumed that NXP had dropped
> > trying to support the LS1021A even in their own kernels.
> >
> > In any case, we have zero interest in running an NXP kernel. Maybe I
> > should clarify what I meant by "based on commits from" above: We're
> > currently running a mainline 4.14 kernel on LS1021A, with a few
> > patches inspired from the NXP 4.1 branch applied on top - but also
> > with some manual fixes for e.g. the pvr_version_is() issue. Now we
> > want to move that to a 4.19-based kernel (so that it aligns with our
> MPC8309 platform).
>
> We also provide 4.19 based kernel in the codeaurora repo. I think it will be
> helpful to reuse patches there if you want to make your own tree.
>
> >
> > >> This is just some first few steps, and I'm not claiming that this
> > >> is sufficient to make the QE drivers build on ARM yet. But I have a
> > >> customer with both mpc8309-based and ls1021a-based platforms, and
> > >> they want to run the same, as-close-to-mainline-as-possible, kernel
> > >> on both. So I will take a piecemeal approach, and try to make sure
> > >> I don't break the ppc boards in the process (just building and
> > >> booting one board is of course not sufficient, but better than
> > >> nothing). Once I get to actually build some of the QE drivers for
> > >> ARM, I'll of course also test them.
> > >
> > > Understood. Zhao Qiang also maintains some patches similar to your
> > > patchset and I think they are tested on ARM. But the review of
> > > these patches from last submission didn't finish. It looks like
> > > your patches are better divided but not really verified on ARM.
> > > Zhao Qiang's patches are tested but maybe need some final touch for
> > > cleaning up. I will let you guys decide what is the best approach
> > > to make this upstreamed.
> >
> > Yes, as I said, I wanted to try a fresh approach since Zhao Qiang's
> > patches seemed to be getting nowhere. Splitting the patches into
> > smaller pieces is definitely part of that - for example, the
> > completely trivial whitespace fix in patch 1 is to make sure the later
> > coccinelle generated patch is precisely that (i.e., a later respin can
> > just rerun the coccinelle script, with zero manual fixups). I also
> > want to avoid mixing the ppcism cleanups with other things (e.g.
> > replacing some
> > of_get_property() by of_property_read_u32()). And the "testing on ARM"
> > part comes once I get to actually building on ARM. But there's not
> > much point doing all that unless there's some indication that this can
> > be applied to some tree that actually feeds into Linus', which is why
> > I started with a few trivial patches and precisely to start this discussion.
>
> Right. I'm really interested in getting this applied to my tree and make it
> upstream. Zhao Qiang, can you help to review Rasmus's patches and
> comment?
As you know, I maintained a similar patchset removing PPC, and someone told me qe_ic should moved into drivers/irqchip/.
I also thought qe_ic is a interrupt control driver, should be moved into dir irqchip.
Regards,
Qiang