[PATCH 0/7] towards QE support on ARM

[PATCH 0/7] towards QE support on ARM

[Rasmus Villemoes]

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.

Tested on an MPC8309-derived board.

Rasmus Villemoes (7):
  soc: fsl: qe: remove space-before-tab
  soc: fsl: qe: drop volatile qualifier of struct qe_ic::regs
  soc: fsl: qe: avoid ppc-specific io accessors
  soc: fsl: qe: replace spin_event_timeout by readx_poll_timeout_atomic
  serial: make SERIAL_QE depend on PPC32
  serial: ucc_uart.c: explicitly include asm/cpm.h
  soc/fsl/qe/qe.h: remove include of asm/cpm.h

 drivers/soc/fsl/qe/gpio.c     | 30 ++++++++--------
 drivers/soc/fsl/qe/qe.c       | 44 +++++++++++------------
 drivers/soc/fsl/qe/qe_ic.c    |  8 ++---
 drivers/soc/fsl/qe/qe_ic.h    |  2 +-
 drivers/soc/fsl/qe/qe_io.c    | 40 ++++++++++-----------
 drivers/soc/fsl/qe/qe_tdm.c   |  8 ++---
 drivers/soc/fsl/qe/ucc.c      | 12 +++----
 drivers/soc/fsl/qe/ucc_fast.c | 66 ++++++++++++++++++-----------------
 drivers/soc/fsl/qe/ucc_slow.c | 38 ++++++++++----------
 drivers/soc/fsl/qe/usb.c      |  2 +-
 drivers/tty/serial/Kconfig    |  1 +
 drivers/tty/serial/ucc_uart.c |  1 +
 include/soc/fsl/qe/qe.h       |  1 -
 13 files changed, 126 insertions(+), 127 deletions(-)

-- 
2.20.1

[PATCH 5/7] serial: make SERIAL_QE depend on PPC32

[Rasmus Villemoes]

Currently SERIAL_QE depends on QUICC_ENGINE, which in turn depends on
PPC32, so this doesn't add any extra dependency. However, the QUICC
Engine IP block also exists on some arm boards, so this serves as
preparation for removing the PPC32 dependency from QUICC_ENGINE and
build the QE support in drivers/soc/fsl/qe, while preventing
allmodconfig/randconfig failures due to SERIAL_QE not being supported
yet.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/tty/serial/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 67a9eb3f94ce..78246f535809 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1056,6 +1056,7 @@ config SERIAL_LANTIQ
 config SERIAL_QE
 	tristate "Freescale QUICC Engine serial port support"
 	depends on QUICC_ENGINE
+	depends on PPC32
 	select SERIAL_CORE
 	select FW_LOADER
 	help
-- 
2.20.1

[PATCH 6/7] serial: ucc_uart.c: explicitly include asm/cpm.h

[Rasmus Villemoes]

This driver uses #defines from asm/cpm.h, so instead of relying on
some other header pulling that in, do that explicitly.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/tty/serial/ucc_uart.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index a0555ae2b1ef..e2c998badf81 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -33,6 +33,7 @@
 
 #include <linux/firmware.h>
 #include <asm/reg.h>
+#include <asm/cpm.h>
 
 /*
  * The GUMR flag for Soft UART.  This would normally be defined in qe.h,
-- 
2.20.1

RE: [PATCH 0/7] towards QE support on ARM

[Leo Li]

> -----Original Message-----
> From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Sent: Friday, October 18, 2019 7:52 AM
> To: Qiang Zhao <qiang.zhao@nxp.com>; Leo Li <leoyang.li@nxp.com>; Greg
> Kroah-Hartman <gregkh@linuxfoundation.org>; Jiri Slaby
> <jslaby@suse.com>; Timur Tabi <timur@kernel.org>; linuxppc-
> dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-serial@vger.kernel.org
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Subject: [PATCH 0/7] towards QE support on ARM
> 
> 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.  Can you give an example on how is it used on ARM system?

> 
> Tested on an MPC8309-derived board.

MPC8309 is also PPC based.

> 
> Rasmus Villemoes (7):
>   soc: fsl: qe: remove space-before-tab
>   soc: fsl: qe: drop volatile qualifier of struct qe_ic::regs
>   soc: fsl: qe: avoid ppc-specific io accessors
>   soc: fsl: qe: replace spin_event_timeout by readx_poll_timeout_atomic
>   serial: make SERIAL_QE depend on PPC32
>   serial: ucc_uart.c: explicitly include asm/cpm.h
>   soc/fsl/qe/qe.h: remove include of asm/cpm.h
> 
>  drivers/soc/fsl/qe/gpio.c     | 30 ++++++++--------
>  drivers/soc/fsl/qe/qe.c       | 44 +++++++++++------------
>  drivers/soc/fsl/qe/qe_ic.c    |  8 ++---
>  drivers/soc/fsl/qe/qe_ic.h    |  2 +-
>  drivers/soc/fsl/qe/qe_io.c    | 40 ++++++++++-----------
>  drivers/soc/fsl/qe/qe_tdm.c   |  8 ++---
>  drivers/soc/fsl/qe/ucc.c      | 12 +++----
>  drivers/soc/fsl/qe/ucc_fast.c | 66 ++++++++++++++++++-----------------
>  drivers/soc/fsl/qe/ucc_slow.c | 38 ++++++++++----------
>  drivers/soc/fsl/qe/usb.c      |  2 +-
>  drivers/tty/serial/Kconfig    |  1 +
>  drivers/tty/serial/ucc_uart.c |  1 +
>  include/soc/fsl/qe/qe.h       |  1 -
>  13 files changed, 126 insertions(+), 127 deletions(-)
> 
> --
> 2.20.1

Re: [PATCH 0/7] towards QE support on ARM

[Rasmus Villemoes]

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://lore.kernel.org/lkml/AM3PR04MB11857AE8D2B0BE56121B97D391C90@AM3PR04MB1185.eurprd04.prod.outlook.com/T/#u>?


 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://www.nxp.com/products/processors-and-microcontrollers/arm-processors/layerscape-communication-process/qoriq-layerscape-1021a-dual-core-communications-processor-with-lcd-controller:LS1021A>

The forks at https://github.com/qoriq-open-source/linux.git have various
degrees of support (grep for commits saying stuff like "remove PPCisms"
- some versions can be found on
<https://lore.kernel.org/lkml/?q=remove+ppcisms>). 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.

>>
>> Tested on an MPC8309-derived board.
> 
> MPC8309 is also PPC based.

True, of course. 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.

Rasmus

Re: [PATCH 0/7] towards QE support on ARM

[Li Yang]

On Fri, Oct 18, 2019 at 3:54 PM Rasmus Villemoes
<linux@rasmusvillemoes.dk> 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://lore.kernel.org/lkml/AM3PR04MB11857AE8D2B0BE56121B97D391C90@AM3PR04MB1185.eurprd04.prod.outlook.com/T/#u>?

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://www.nxp.com/products/processors-and-microcontrollers/arm-processors/layerscape-communication-process/qoriq-layerscape-1021a-dual-core-communications-processor-with-lcd-controller:LS1021A>
>
> The forks at https://github.com/qoriq-open-source/linux.git have various
> degrees of support (grep for commits saying stuff like "remove PPCisms"
> - some versions can be found on
> <https://lore.kernel.org/lkml/?q=remove+ppcisms>). 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.

>
> >>
> >> Tested on an MPC8309-derived board.
> >
> > MPC8309 is also PPC based.
>
> True, of course. 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.

Regards,
Leo

Re: [PATCH 0/7] towards QE support on ARM

[Rasmus Villemoes]

On 18/10/2019 23.52, Li Yang wrote:
> On Fri, Oct 18, 2019 at 3:54 PM Rasmus Villemoes
> <linux@rasmusvillemoes.dk> 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://lore.kernel.org/lkml/AM3PR04MB11857AE8D2B0BE56121B97D391C90@AM3PR04MB1185.eurprd04.prod.outlook.com/T/#u>?
> 
> 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://www.nxp.com/products/processors-and-microcontrollers/arm-processors/layerscape-communication-process/qoriq-layerscape-1021a-dual-core-communications-processor-with-lcd-controller:LS1021A>
>>
>> The forks at https://github.com/qoriq-open-source/linux.git have various
>> degrees of support (grep for commits saying stuff like "remove PPCisms"
>> - some versions can be found on
>> <https://lore.kernel.org/lkml/?q=remove+ppcisms>). 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://github.com/qoriq-open-source/linux.git, and identical to the
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).

>> 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.

Rasmus

Re: [PATCH 0/7] towards QE support on ARM

[Li Yang]

On Mon, Oct 21, 2019 at 3:46 AM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> On 18/10/2019 23.52, Li Yang wrote:
> > On Fri, Oct 18, 2019 at 3:54 PM Rasmus Villemoes
> > <linux@rasmusvillemoes.dk> 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://lore.kernel.org/lkml/AM3PR04MB11857AE8D2B0BE56121B97D391C90@AM3PR04MB1185.eurprd04.prod.outlook.com/T/#u>?
> >
> > 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://www.nxp.com/products/processors-and-microcontrollers/arm-processors/layerscape-communication-process/qoriq-layerscape-1021a-dual-core-communications-processor-with-lcd-controller:LS1021A>
> >>
> >> The forks at https://github.com/qoriq-open-source/linux.git have various
> >> degrees of support (grep for commits saying stuff like "remove PPCisms"
> >> - some versions can be found on
> >> <https://lore.kernel.org/lkml/?q=remove+ppcisms>). 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://github.com/qoriq-open-source/linux.git, 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://source.codeaurora.org/external/qoriq/qoriq-components/linux/

> 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?

Regards,
Leo

RE: [PATCH 0/7] towards QE support on ARM

[Qiang Zhao]

On Mon, Oct 22, 2019 at 6:11 AM Leo Li wrote
> -----Original Message-----
> From: Li Yang <leoyang.li@nxp.com>
> Sent: 2019年10月22日 6:11
> To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Cc: Timur Tabi <timur@kernel.org>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org;
> linux-serial@vger.kernel.org; Jiri Slaby <jslaby@suse.com>;
> linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org; Qiang
> Zhao <qiang.zhao@nxp.com>
> Subject: Re: [PATCH 0/7] towards QE support on ARM
> 
> On Mon, Oct 21, 2019 at 3:46 AM Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:
> >
> > On 18/10/2019 23.52, Li Yang wrote:
> > > On Fri, Oct 18, 2019 at 3:54 PM Rasmus Villemoes
> > > <linux@rasmusvillemoes.dk> 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&amp;data=02%7
> C01%7Cqiang.zhao%40nxp.com%7C1ba8c50c2db14b22bef608d756739d82%
> 7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6370729268771788
> 75&amp;sdata=k4zM75OczXwZF%2Br9ec4RxiVR2a%2F8GhSZmK70JYddIck%3
> D&amp;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&amp;sdata=vqPYSZqEv6vCEIxJshLuA4gngh9J4IsFAQrTwJKMjm4%3D&amp;r
> es
> > >> erved=0>
> > >>
> > >> The forks at
> > >>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
> com%2Fqoriq-open-source%2Flinux.git&amp;data=02%7C01%7Cqiang.zhao%
> 40nxp.com%7C1ba8c50c2db14b22bef608d756739d82%7C686ea1d3bc2b4c6
> fa92cd99c5c301635%7C0%7C0%7C637072926877178875&amp;sdata=v4eG
> 4KqGTWQkQHp%2FYg2OHCKITLWaOgH64JYpY%2B1LilA%3D&amp;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&amp;data=02%7C0
> 1%7
> > >>
> Cqiang.zhao%40nxp.com%7C1ba8c50c2db14b22bef608d756739d82%7C686e
> a1d3
> > >>
> bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637072926877178875&amp;sdat
> a=i2WdKNHLV68a1mTOMQ%2FoMr0y5ee8edS07xq61M8%2BvPU%3D&amp;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&amp;data=02%7C01%7Cqiang.zha
> o%4
> >
> 0nxp.com%7C1ba8c50c2db14b22bef608d756739d82%7C686ea1d3bc2b4c6f
> a92cd99c
> >
> 5c301635%7C0%7C0%7C637072926877188868&amp;sdata=vdm4qPoTzfIpXL
> mRrv17EW
> > noxG3n91qELMGqvRh9we4%3D&amp;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&amp
> ;data=02%7C01%7Cqiang.zhao%40nxp.com%7C1ba8c50c2db14b22bef608d7
> 56739d82%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6370729
> 26877188868&amp;sdata=NooBFUWnceTG2OF24pCuP0AYgKfr0Df%2BtrcCY6
> X6Dog%3D&amp;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

Re: [PATCH 0/7] towards QE support on ARM

[Rasmus Villemoes]

On 22/10/2019 04.24, Qiang Zhao wrote:
> On Mon, Oct 22, 2019 at 6:11 AM Leo Li wrote

>> 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.

Yes, and I also plan to do that at some point. However, that's
orthogonal to making the driver build on ARM, so I don't want to mix the
two. Making it usable on ARM is my/our priority currently.

I'd appreciate your input on my patches.

Rasmus

RE: [PATCH 0/7] towards QE support on ARM

[Qiang Zhao]

On 22/10/2019 18:18, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
> -----Original Message-----
> From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Sent: 2019年10月22日 18:18
> To: Qiang Zhao <qiang.zhao@nxp.com>; Leo Li <leoyang.li@nxp.com>
> Cc: Timur Tabi <timur@kernel.org>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org;
> linux-serial@vger.kernel.org; Jiri Slaby <jslaby@suse.com>;
> linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH 0/7] towards QE support on ARM
> 
> On 22/10/2019 04.24, Qiang Zhao wrote:
> > On Mon, Oct 22, 2019 at 6:11 AM Leo Li wrote
> 
> >> 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.
> 
> Yes, and I also plan to do that at some point. However, that's orthogonal to
> making the driver build on ARM, so I don't want to mix the two. Making it
> usable on ARM is my/our priority currently.
> 
> I'd appreciate your input on my patches.

Yes, we can put this patchset in first place, ensure it can build and work on ARM, then push another patchset to move qe_ic.

Best Regards,
Qiang

Re: [PATCH 0/7] towards QE support on ARM

[Li Yang]

On Tue, Oct 22, 2019 at 9:54 PM Qiang Zhao <qiang.zhao@nxp.com> wrote:
>
> On 22/10/2019 18:18, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
> > -----Original Message-----
> > From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > Sent: 2019年10月22日 18:18
> > To: Qiang Zhao <qiang.zhao@nxp.com>; Leo Li <leoyang.li@nxp.com>
> > Cc: Timur Tabi <timur@kernel.org>; Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org;
> > linux-serial@vger.kernel.org; Jiri Slaby <jslaby@suse.com>;
> > linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org
> > Subject: Re: [PATCH 0/7] towards QE support on ARM
> >
> > On 22/10/2019 04.24, Qiang Zhao wrote:
> > > On Mon, Oct 22, 2019 at 6:11 AM Leo Li wrote
> >
> > >> 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.
> >
> > Yes, and I also plan to do that at some point. However, that's orthogonal to
> > making the driver build on ARM, so I don't want to mix the two. Making it
> > usable on ARM is my/our priority currently.
> >
> > I'd appreciate your input on my patches.
>
> Yes, we can put this patchset in first place, ensure it can build and work on ARM, then push another patchset to move qe_ic.

Right.  I would only accept a patch series that can really build and
work on ARM.  At least the current out-of-tree patches can make it
work on ARM.  If we accept partial changes, there is no way to make it
work on the latest kernel on ARM then.

Regards,
Leo

Re: [PATCH 0/7] towards QE support on ARM

[Rasmus Villemoes]

On 22/10/2019 00.11, Li Yang wrote:
> On Mon, Oct 21, 2019 at 3:46 AM Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:
>>

>>> 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://github.com/qoriq-open-source/linux.git, 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://source.codeaurora.org/external/qoriq/qoriq-components/linux/

Ah. FYI, googling "LSDK" gives https://lsdk.github.io as one of the
first hits, and (apart from itself being a github url) that says on the
front page "Disaggregated components of LSDK are available in github.".
But yes, navigating to the Components tab and from there to lsdk linux
one does get directed at codeaurora.

>> 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.

Again, we don't want to run off an NXP kernel, we want to get the
necessary pieces upstream. For now, we have to live with a patched 4.19
kernel, but hopefully by the time we switch to 5.x (for some x >= 5) we
don't need to supply anything other than our own .dts and defconfig.

>> 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?

Thanks, this is exactly what I was hoping for. Even just getting these
first rather trivial patches (in that they don't attempt to build for
ARM, or change functionality at all for PPC) merged for 5.5 would reduce
the amount of out-of-tree patches that we (and NXP for that matter)
would have to carry. I'll take the above as a go-ahead for me to try to
post more patches working towards enabling some of the QE drivers for ARM.

Rasmus

Re: [PATCH 0/7] towards QE support on ARM

[Christophe Leroy]

On 10/18/2019 12:52 PM, Rasmus Villemoes 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.
> 
> Tested on an MPC8309-derived board.
> 
> Rasmus Villemoes (7):
>    soc: fsl: qe: remove space-before-tab
>    soc: fsl: qe: drop volatile qualifier of struct qe_ic::regs
>    soc: fsl: qe: avoid ppc-specific io accessors
>    soc: fsl: qe: replace spin_event_timeout by readx_poll_timeout_atomic
>    serial: make SERIAL_QE depend on PPC32
>    serial: ucc_uart.c: explicitly include asm/cpm.h
>    soc/fsl/qe/qe.h: remove include of asm/cpm.h

Please copy the entire series to linuxppc-dev list. We are missing 5/7 
and 7/7 (see 
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=137048)

Christophe

> 
>   drivers/soc/fsl/qe/gpio.c     | 30 ++++++++--------
>   drivers/soc/fsl/qe/qe.c       | 44 +++++++++++------------
>   drivers/soc/fsl/qe/qe_ic.c    |  8 ++---
>   drivers/soc/fsl/qe/qe_ic.h    |  2 +-
>   drivers/soc/fsl/qe/qe_io.c    | 40 ++++++++++-----------
>   drivers/soc/fsl/qe/qe_tdm.c   |  8 ++---
>   drivers/soc/fsl/qe/ucc.c      | 12 +++----
>   drivers/soc/fsl/qe/ucc_fast.c | 66 ++++++++++++++++++-----------------
>   drivers/soc/fsl/qe/ucc_slow.c | 38 ++++++++++----------
>   drivers/soc/fsl/qe/usb.c      |  2 +-
>   drivers/tty/serial/Kconfig    |  1 +
>   drivers/tty/serial/ucc_uart.c |  1 +
>   include/soc/fsl/qe/qe.h       |  1 -
>   13 files changed, 126 insertions(+), 127 deletions(-)
> 

[PATCH v2 20/23] serial: make SERIAL_QE depend on PPC32

[Rasmus Villemoes]

Currently SERIAL_QE depends on QUICC_ENGINE, which in turn depends on
PPC32, so this doesn't add any extra dependency. However, the QUICC
Engine IP block also exists on some arm boards, so this serves as
preparation for removing the PPC32 dependency from QUICC_ENGINE and
build the QE support in drivers/soc/fsl/qe, while preventing
allmodconfig/randconfig failures due to SERIAL_QE not being supported
yet.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/tty/serial/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 67a9eb3f94ce..78246f535809 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1056,6 +1056,7 @@ config SERIAL_LANTIQ
 config SERIAL_QE
 	tristate "Freescale QUICC Engine serial port support"
 	depends on QUICC_ENGINE
+	depends on PPC32
 	select SERIAL_CORE
 	select FW_LOADER
 	help
-- 
2.23.0

RE: [PATCH v2 20/23] serial: make SERIAL_QE depend on PPC32

[Leo Li]

> -----Original Message-----
> From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Sent: Friday, October 25, 2019 7:41 AM
> To: Qiang Zhao <qiang.zhao@nxp.com>; Leo Li <leoyang.li@nxp.com>;
> Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; Scott Wood <oss@buserror.net>; Valentin
> Longchamp <valentin.longchamp@keymile.com>; Rasmus Villemoes
> <linux@rasmusvillemoes.dk>; linux-serial@vger.kernel.org
> Subject: [PATCH v2 20/23] serial: make SERIAL_QE depend on PPC32
> 
> Currently SERIAL_QE depends on QUICC_ENGINE, which in turn depends on
> PPC32, so this doesn't add any extra dependency. However, the QUICC
> Engine IP block also exists on some arm boards, so this serves as preparation
> for removing the PPC32 dependency from QUICC_ENGINE and build the QE
> support in drivers/soc/fsl/qe, while preventing allmodconfig/randconfig
> failures due to SERIAL_QE not being supported yet.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

I think your purpose of this series is to make the QE UART not depending on PPC32.  If it does accomplish that then we don't need this change.

> ---
>  drivers/tty/serial/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig index
> 67a9eb3f94ce..78246f535809 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1056,6 +1056,7 @@ config SERIAL_LANTIQ  config SERIAL_QE
>  	tristate "Freescale QUICC Engine serial port support"
>  	depends on QUICC_ENGINE
> +	depends on PPC32
>  	select SERIAL_CORE
>  	select FW_LOADER
>  	help
> --
> 2.23.0

Re: [PATCH v2 20/23] serial: make SERIAL_QE depend on PPC32

[Rasmus Villemoes]

On 29/10/2019 23.44, Leo Li wrote:
> 
> 
>> -----Original Message-----
>> From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>> Sent: Friday, October 25, 2019 7:41 AM
>> To: Qiang Zhao <qiang.zhao@nxp.com>; Leo Li <leoyang.li@nxp.com>;
>> Christophe Leroy <christophe.leroy@c-s.fr>
>> Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
>> linux-kernel@vger.kernel.org; Scott Wood <oss@buserror.net>; Valentin
>> Longchamp <valentin.longchamp@keymile.com>; Rasmus Villemoes
>> <linux@rasmusvillemoes.dk>; linux-serial@vger.kernel.org
>> Subject: [PATCH v2 20/23] serial: make SERIAL_QE depend on PPC32
>>
>> Currently SERIAL_QE depends on QUICC_ENGINE, which in turn depends on
>> PPC32, so this doesn't add any extra dependency. However, the QUICC
>> Engine IP block also exists on some arm boards, so this serves as preparation
>> for removing the PPC32 dependency from QUICC_ENGINE and build the QE
>> support in drivers/soc/fsl/qe, while preventing allmodconfig/randconfig
>> failures due to SERIAL_QE not being supported yet.
>>
>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> 
> I think your purpose of this series is to make the QE UART not depending on PPC32.  If it does accomplish that then we don't need this change.

Yeah, as I've said in private, I now have the patches to make this work,
so this patch (and the later one removing it again) are both gone from
my current dev branch. I'll still wait a day or two to allow the ppc
people to respond to the inline/OOL iowrite32be issue, but after that
I'll resend the whole series.

Rasmus

Re: [PATCH v2 20/23] serial: make SERIAL_QE depend on PPC32

[Christophe Leroy]

Le 25/10/2019 à 14:40, Rasmus Villemoes a écrit :
> Currently SERIAL_QE depends on QUICC_ENGINE, which in turn depends on
> PPC32, so this doesn't add any extra dependency. However, the QUICC
> Engine IP block also exists on some arm boards, so this serves as
> preparation for removing the PPC32 dependency from QUICC_ENGINE and
> build the QE support in drivers/soc/fsl/qe, while preventing
> allmodconfig/randconfig failures due to SERIAL_QE not being supported
> yet.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>   drivers/tty/serial/Kconfig | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 67a9eb3f94ce..78246f535809 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1056,6 +1056,7 @@ config SERIAL_LANTIQ
>   config SERIAL_QE
>   	tristate "Freescale QUICC Engine serial port support"
>   	depends on QUICC_ENGINE
> +	depends on PPC32

Same, would be more obvious as
	depends on QUICC_ENGINE && PPC32

Christophe

>   	select SERIAL_CORE
>   	select FW_LOADER
>   	help
> 

[PATCH v2 21/23] serial: ucc_uart.c: explicitly include asm/cpm.h

[Rasmus Villemoes]

This driver uses #defines from asm/cpm.h, so instead of relying on
some other header pulling that in, do that explicitly.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/tty/serial/ucc_uart.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index a0555ae2b1ef..e2c998badf81 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -33,6 +33,7 @@
 
 #include <linux/firmware.h>
 #include <asm/reg.h>
+#include <asm/cpm.h>
 
 /*
  * The GUMR flag for Soft UART.  This would normally be defined in qe.h,
-- 
2.23.0

[PATCH v3 28/36] serial: ucc_uart: explicitly include soc/fsl/cpm.h

[Rasmus Villemoes]

This driver uses #defines from soc/fsl/cpm.h, so instead of relying on
some other header pulling that in, do that explicitly. This is
preparation for allowing this driver to build on ARM.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/tty/serial/ucc_uart.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index a0555ae2b1ef..7e802616cba8 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -32,6 +32,7 @@
 #include <soc/fsl/qe/ucc_slow.h>
 
 #include <linux/firmware.h>
+#include <soc/fsl/cpm.h>
 #include <asm/reg.h>
 
 /*
-- 
2.23.0

Re: [PATCH v3 28/36] serial: ucc_uart: explicitly include soc/fsl/cpm.h

[Christophe Leroy]

Le 01/11/2019 à 13:42, Rasmus Villemoes a écrit :
> This driver uses #defines from soc/fsl/cpm.h, so instead of relying on
> some other header pulling that in, do that explicitly. This is
> preparation for allowing this driver to build on ARM.
> 

UCC are only on QE.
CPM has SCCs. instead.
So this driver shouldn't need cpm.h

Christophe

> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>   drivers/tty/serial/ucc_uart.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
> index a0555ae2b1ef..7e802616cba8 100644
> --- a/drivers/tty/serial/ucc_uart.c
> +++ b/drivers/tty/serial/ucc_uart.c
> @@ -32,6 +32,7 @@
>   #include <soc/fsl/qe/ucc_slow.h>
>   
>   #include <linux/firmware.h>
> +#include <soc/fsl/cpm.h>
>   #include <asm/reg.h>
>   
>   /*
> 

Re: [PATCH v3 28/36] serial: ucc_uart: explicitly include soc/fsl/cpm.h

[Rasmus Villemoes]

On 01/11/2019 17.19, Christophe Leroy wrote:
> Le 01/11/2019 à 13:42, Rasmus Villemoes a écrit :
>> This driver uses #defines from soc/fsl/cpm.h, so instead of relying on
>> some other header pulling that in, do that explicitly. This is
>> preparation for allowing this driver to build on ARM.
>>
> 
> UCC are only on QE.
> CPM has SCCs. instead.
> So this driver shouldn't need cpm.h

But it does. At the very least for the BD_SC_* defines, possibly others
things. It's possible one could split off the common part to a separate
header, but that sort of cleanup/refactoring is beyond what I'd be
comfortable including in this series.

Rasmus

[PATCH v3 29/36] serial: ucc_uart: replace ppc-specific IO accessors

[Rasmus Villemoes]

Some ARM-based SOCs (e.g. LS1021A) also have a QUICC engine. As
preparation for allowing this driver to build on ARM, replace the
ppc-specific in_be16() etc. by the qe_io* helpers. Done via
coccinelle.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/tty/serial/ucc_uart.c | 210 +++++++++++++++++-----------------
 1 file changed, 102 insertions(+), 108 deletions(-)

diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index 7e802616cba8..8a378ee5d34f 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -258,11 +258,11 @@ static unsigned int qe_uart_tx_empty(struct uart_port *port)
 	struct qe_bd *bdp = qe_port->tx_bd_base;
 
 	while (1) {
-		if (in_be16(&bdp->status) & BD_SC_READY)
+		if (qe_ioread16be(&bdp->status) & BD_SC_READY)
 			/* This BD is not done, so return "not done" */
 			return 0;
 
-		if (in_be16(&bdp->status) & BD_SC_WRAP)
+		if (qe_ioread16be(&bdp->status) & BD_SC_WRAP)
 			/*
 			 * This BD is done and it's the last one, so return
 			 * "done"
@@ -308,7 +308,7 @@ static void qe_uart_stop_tx(struct uart_port *port)
 	struct uart_qe_port *qe_port =
 		container_of(port, struct uart_qe_port, port);
 
-	clrbits16(&qe_port->uccp->uccm, UCC_UART_UCCE_TX);
+	qe_clrbits_be16(&qe_port->uccp->uccm, UCC_UART_UCCE_TX);
 }
 
 /*
@@ -343,10 +343,10 @@ static int qe_uart_tx_pump(struct uart_qe_port *qe_port)
 		p = qe2cpu_addr(bdp->buf, qe_port);
 
 		*p++ = port->x_char;
-		out_be16(&bdp->length, 1);
-		setbits16(&bdp->status, BD_SC_READY);
+		qe_iowrite16be(1, &bdp->length);
+		qe_setbits_be16(&bdp->status, BD_SC_READY);
 		/* Get next BD. */
-		if (in_be16(&bdp->status) & BD_SC_WRAP)
+		if (qe_ioread16be(&bdp->status) & BD_SC_WRAP)
 			bdp = qe_port->tx_bd_base;
 		else
 			bdp++;
@@ -365,7 +365,7 @@ static int qe_uart_tx_pump(struct uart_qe_port *qe_port)
 	/* Pick next descriptor and fill from buffer */
 	bdp = qe_port->tx_cur;
 
-	while (!(in_be16(&bdp->status) & BD_SC_READY) &&
+	while (!(qe_ioread16be(&bdp->status) & BD_SC_READY) &&
 	       (xmit->tail != xmit->head)) {
 		count = 0;
 		p = qe2cpu_addr(bdp->buf, qe_port);
@@ -378,11 +378,11 @@ static int qe_uart_tx_pump(struct uart_qe_port *qe_port)
 				break;
 		}
 
-		out_be16(&bdp->length, count);
-		setbits16(&bdp->status, BD_SC_READY);
+		qe_iowrite16be(count, &bdp->length);
+		qe_setbits_be16(&bdp->status, BD_SC_READY);
 
 		/* Get next BD. */
-		if (in_be16(&bdp->status) & BD_SC_WRAP)
+		if (qe_ioread16be(&bdp->status) & BD_SC_WRAP)
 			bdp = qe_port->tx_bd_base;
 		else
 			bdp++;
@@ -415,12 +415,12 @@ static void qe_uart_start_tx(struct uart_port *port)
 		container_of(port, struct uart_qe_port, port);
 
 	/* If we currently are transmitting, then just return */
-	if (in_be16(&qe_port->uccp->uccm) & UCC_UART_UCCE_TX)
+	if (qe_ioread16be(&qe_port->uccp->uccm) & UCC_UART_UCCE_TX)
 		return;
 
 	/* Otherwise, pump the port and start transmission */
 	if (qe_uart_tx_pump(qe_port))
-		setbits16(&qe_port->uccp->uccm, UCC_UART_UCCE_TX);
+		qe_setbits_be16(&qe_port->uccp->uccm, UCC_UART_UCCE_TX);
 }
 
 /*
@@ -431,7 +431,7 @@ static void qe_uart_stop_rx(struct uart_port *port)
 	struct uart_qe_port *qe_port =
 		container_of(port, struct uart_qe_port, port);
 
-	clrbits16(&qe_port->uccp->uccm, UCC_UART_UCCE_RX);
+	qe_clrbits_be16(&qe_port->uccp->uccm, UCC_UART_UCCE_RX);
 }
 
 /* Start or stop sending  break signal
@@ -470,14 +470,14 @@ static void qe_uart_int_rx(struct uart_qe_port *qe_port)
 	 */
 	bdp = qe_port->rx_cur;
 	while (1) {
-		status = in_be16(&bdp->status);
+		status = qe_ioread16be(&bdp->status);
 
 		/* If this one is empty, then we assume we've read them all */
 		if (status & BD_SC_EMPTY)
 			break;
 
 		/* get number of characters, and check space in RX buffer */
-		i = in_be16(&bdp->length);
+		i = qe_ioread16be(&bdp->length);
 
 		/* If we don't have enough room in RX buffer for the entire BD,
 		 * then we try later, which will be the next RX interrupt.
@@ -508,9 +508,10 @@ static void qe_uart_int_rx(struct uart_qe_port *qe_port)
 		}
 
 		/* This BD is ready to be used again. Clear status. get next */
-		clrsetbits_be16(&bdp->status, BD_SC_BR | BD_SC_FR | BD_SC_PR |
-			BD_SC_OV | BD_SC_ID, BD_SC_EMPTY);
-		if (in_be16(&bdp->status) & BD_SC_WRAP)
+		qe_clrsetbits_be16(&bdp->status,
+				   BD_SC_BR | BD_SC_FR | BD_SC_PR | BD_SC_OV | BD_SC_ID,
+				   BD_SC_EMPTY);
+		if (qe_ioread16be(&bdp->status) & BD_SC_WRAP)
 			bdp = qe_port->rx_bd_base;
 		else
 			bdp++;
@@ -569,8 +570,8 @@ static irqreturn_t qe_uart_int(int irq, void *data)
 	u16 events;
 
 	/* Clear the interrupts */
-	events = in_be16(&uccp->ucce);
-	out_be16(&uccp->ucce, events);
+	events = qe_ioread16be(&uccp->ucce);
+	qe_iowrite16be(events, &uccp->ucce);
 
 	if (events & UCC_UART_UCCE_BRKE)
 		uart_handle_break(&qe_port->port);
@@ -601,17 +602,17 @@ static void qe_uart_initbd(struct uart_qe_port *qe_port)
 	bdp = qe_port->rx_bd_base;
 	qe_port->rx_cur = qe_port->rx_bd_base;
 	for (i = 0; i < (qe_port->rx_nrfifos - 1); i++) {
-		out_be16(&bdp->status, BD_SC_EMPTY | BD_SC_INTRPT);
-		out_be32(&bdp->buf, cpu2qe_addr(bd_virt, qe_port));
-		out_be16(&bdp->length, 0);
+		qe_iowrite16be(BD_SC_EMPTY | BD_SC_INTRPT, &bdp->status);
+		qe_iowrite32be(cpu2qe_addr(bd_virt, qe_port), &bdp->buf);
+		qe_iowrite16be(0, &bdp->length);
 		bd_virt += qe_port->rx_fifosize;
 		bdp++;
 	}
 
 	/* */
-	out_be16(&bdp->status, BD_SC_WRAP | BD_SC_EMPTY | BD_SC_INTRPT);
-	out_be32(&bdp->buf, cpu2qe_addr(bd_virt, qe_port));
-	out_be16(&bdp->length, 0);
+	qe_iowrite16be(BD_SC_WRAP | BD_SC_EMPTY | BD_SC_INTRPT, &bdp->status);
+	qe_iowrite32be(cpu2qe_addr(bd_virt, qe_port), &bdp->buf);
+	qe_iowrite16be(0, &bdp->length);
 
 	/* Set the physical address of the host memory
 	 * buffers in the buffer descriptors, and the
@@ -622,21 +623,21 @@ static void qe_uart_initbd(struct uart_qe_port *qe_port)
 	qe_port->tx_cur = qe_port->tx_bd_base;
 	bdp = qe_port->tx_bd_base;
 	for (i = 0; i < (qe_port->tx_nrfifos - 1); i++) {
-		out_be16(&bdp->status, BD_SC_INTRPT);
-		out_be32(&bdp->buf, cpu2qe_addr(bd_virt, qe_port));
-		out_be16(&bdp->length, 0);
+		qe_iowrite16be(BD_SC_INTRPT, &bdp->status);
+		qe_iowrite32be(cpu2qe_addr(bd_virt, qe_port), &bdp->buf);
+		qe_iowrite16be(0, &bdp->length);
 		bd_virt += qe_port->tx_fifosize;
 		bdp++;
 	}
 
 	/* Loopback requires the preamble bit to be set on the first TX BD */
 #ifdef LOOPBACK
-	setbits16(&qe_port->tx_cur->status, BD_SC_P);
+	qe_setbits_be16(&qe_port->tx_cur->status, BD_SC_P);
 #endif
 
-	out_be16(&bdp->status, BD_SC_WRAP | BD_SC_INTRPT);
-	out_be32(&bdp->buf, cpu2qe_addr(bd_virt, qe_port));
-	out_be16(&bdp->length, 0);
+	qe_iowrite16be(BD_SC_WRAP | BD_SC_INTRPT, &bdp->status);
+	qe_iowrite32be(cpu2qe_addr(bd_virt, qe_port), &bdp->buf);
+	qe_iowrite16be(0, &bdp->length);
 }
 
 /*
@@ -658,78 +659,74 @@ static void qe_uart_init_ucc(struct uart_qe_port *qe_port)
 	ucc_slow_disable(qe_port->us_private, COMM_DIR_RX_AND_TX);
 
 	/* Program the UCC UART parameter RAM */
-	out_8(&uccup->common.rbmr, UCC_BMR_GBL | UCC_BMR_BO_BE);
-	out_8(&uccup->common.tbmr, UCC_BMR_GBL | UCC_BMR_BO_BE);
-	out_be16(&uccup->common.mrblr, qe_port->rx_fifosize);
-	out_be16(&uccup->maxidl, 0x10);
-	out_be16(&uccup->brkcr, 1);
-	out_be16(&uccup->parec, 0);
-	out_be16(&uccup->frmec, 0);
-	out_be16(&uccup->nosec, 0);
-	out_be16(&uccup->brkec, 0);
-	out_be16(&uccup->uaddr[0], 0);
-	out_be16(&uccup->uaddr[1], 0);
-	out_be16(&uccup->toseq, 0);
+	qe_iowrite8(UCC_BMR_GBL | UCC_BMR_BO_BE, &uccup->common.rbmr);
+	qe_iowrite8(UCC_BMR_GBL | UCC_BMR_BO_BE, &uccup->common.tbmr);
+	qe_iowrite16be(qe_port->rx_fifosize, &uccup->common.mrblr);
+	qe_iowrite16be(0x10, &uccup->maxidl);
+	qe_iowrite16be(1, &uccup->brkcr);
+	qe_iowrite16be(0, &uccup->parec);
+	qe_iowrite16be(0, &uccup->frmec);
+	qe_iowrite16be(0, &uccup->nosec);
+	qe_iowrite16be(0, &uccup->brkec);
+	qe_iowrite16be(0, &uccup->uaddr[0]);
+	qe_iowrite16be(0, &uccup->uaddr[1]);
+	qe_iowrite16be(0, &uccup->toseq);
 	for (i = 0; i < 8; i++)
-		out_be16(&uccup->cchars[i], 0xC000);
-	out_be16(&uccup->rccm, 0xc0ff);
+		qe_iowrite16be(0xC000, &uccup->cchars[i]);
+	qe_iowrite16be(0xc0ff, &uccup->rccm);
 
 	/* Configure the GUMR registers for UART */
 	if (soft_uart) {
 		/* Soft-UART requires a 1X multiplier for TX */
-		clrsetbits_be32(&uccp->gumr_l,
-			UCC_SLOW_GUMR_L_MODE_MASK | UCC_SLOW_GUMR_L_TDCR_MASK |
-			UCC_SLOW_GUMR_L_RDCR_MASK,
-			UCC_SLOW_GUMR_L_MODE_UART | UCC_SLOW_GUMR_L_TDCR_1 |
-			UCC_SLOW_GUMR_L_RDCR_16);
-
-		clrsetbits_be32(&uccp->gumr_h, UCC_SLOW_GUMR_H_RFW,
-			UCC_SLOW_GUMR_H_TRX | UCC_SLOW_GUMR_H_TTX);
+		qe_clrsetbits_be32(&uccp->gumr_l,
+				   UCC_SLOW_GUMR_L_MODE_MASK | UCC_SLOW_GUMR_L_TDCR_MASK | UCC_SLOW_GUMR_L_RDCR_MASK,
+				   UCC_SLOW_GUMR_L_MODE_UART | UCC_SLOW_GUMR_L_TDCR_1 | UCC_SLOW_GUMR_L_RDCR_16);
+
+		qe_clrsetbits_be32(&uccp->gumr_h, UCC_SLOW_GUMR_H_RFW,
+				   UCC_SLOW_GUMR_H_TRX | UCC_SLOW_GUMR_H_TTX);
 	} else {
-		clrsetbits_be32(&uccp->gumr_l,
-			UCC_SLOW_GUMR_L_MODE_MASK | UCC_SLOW_GUMR_L_TDCR_MASK |
-			UCC_SLOW_GUMR_L_RDCR_MASK,
-			UCC_SLOW_GUMR_L_MODE_UART | UCC_SLOW_GUMR_L_TDCR_16 |
-			UCC_SLOW_GUMR_L_RDCR_16);
-
-		clrsetbits_be32(&uccp->gumr_h,
-			UCC_SLOW_GUMR_H_TRX | UCC_SLOW_GUMR_H_TTX,
-			UCC_SLOW_GUMR_H_RFW);
+		qe_clrsetbits_be32(&uccp->gumr_l,
+				   UCC_SLOW_GUMR_L_MODE_MASK | UCC_SLOW_GUMR_L_TDCR_MASK | UCC_SLOW_GUMR_L_RDCR_MASK,
+				   UCC_SLOW_GUMR_L_MODE_UART | UCC_SLOW_GUMR_L_TDCR_16 | UCC_SLOW_GUMR_L_RDCR_16);
+
+		qe_clrsetbits_be32(&uccp->gumr_h,
+				   UCC_SLOW_GUMR_H_TRX | UCC_SLOW_GUMR_H_TTX,
+				   UCC_SLOW_GUMR_H_RFW);
 	}
 
 #ifdef LOOPBACK
-	clrsetbits_be32(&uccp->gumr_l, UCC_SLOW_GUMR_L_DIAG_MASK,
-		UCC_SLOW_GUMR_L_DIAG_LOOP);
-	clrsetbits_be32(&uccp->gumr_h,
-		UCC_SLOW_GUMR_H_CTSP | UCC_SLOW_GUMR_H_RSYN,
-		UCC_SLOW_GUMR_H_CDS);
+	qe_clrsetbits_be32(&uccp->gumr_l, UCC_SLOW_GUMR_L_DIAG_MASK,
+			   UCC_SLOW_GUMR_L_DIAG_LOOP);
+	qe_clrsetbits_be32(&uccp->gumr_h,
+			   UCC_SLOW_GUMR_H_CTSP | UCC_SLOW_GUMR_H_RSYN,
+			   UCC_SLOW_GUMR_H_CDS);
 #endif
 
 	/* Disable rx interrupts  and clear all pending events.  */
-	out_be16(&uccp->uccm, 0);
-	out_be16(&uccp->ucce, 0xffff);
-	out_be16(&uccp->udsr, 0x7e7e);
+	qe_iowrite16be(0, &uccp->uccm);
+	qe_iowrite16be(0xffff, &uccp->ucce);
+	qe_iowrite16be(0x7e7e, &uccp->udsr);
 
 	/* Initialize UPSMR */
-	out_be16(&uccp->upsmr, 0);
+	qe_iowrite16be(0, &uccp->upsmr);
 
 	if (soft_uart) {
-		out_be16(&uccup->supsmr, 0x30);
-		out_be16(&uccup->res92, 0);
-		out_be32(&uccup->rx_state, 0);
-		out_be32(&uccup->rx_cnt, 0);
-		out_8(&uccup->rx_bitmark, 0);
-		out_8(&uccup->rx_length, 10);
-		out_be32(&uccup->dump_ptr, 0x4000);
-		out_8(&uccup->rx_temp_dlst_qe, 0);
-		out_be32(&uccup->rx_frame_rem, 0);
-		out_8(&uccup->rx_frame_rem_size, 0);
+		qe_iowrite16be(0x30, &uccup->supsmr);
+		qe_iowrite16be(0, &uccup->res92);
+		qe_iowrite32be(0, &uccup->rx_state);
+		qe_iowrite32be(0, &uccup->rx_cnt);
+		qe_iowrite8(0, &uccup->rx_bitmark);
+		qe_iowrite8(10, &uccup->rx_length);
+		qe_iowrite32be(0x4000, &uccup->dump_ptr);
+		qe_iowrite8(0, &uccup->rx_temp_dlst_qe);
+		qe_iowrite32be(0, &uccup->rx_frame_rem);
+		qe_iowrite8(0, &uccup->rx_frame_rem_size);
 		/* Soft-UART requires TX to be 1X */
-		out_8(&uccup->tx_mode,
-			UCC_UART_TX_STATE_UART | UCC_UART_TX_STATE_X1);
-		out_be16(&uccup->tx_state, 0);
-		out_8(&uccup->resD4, 0);
-		out_be16(&uccup->resD5, 0);
+		qe_iowrite8(UCC_UART_TX_STATE_UART | UCC_UART_TX_STATE_X1,
+			    &uccup->tx_mode);
+		qe_iowrite16be(0, &uccup->tx_state);
+		qe_iowrite8(0, &uccup->resD4);
+		qe_iowrite16be(0, &uccup->resD5);
 
 		/* Set UART mode.
 		 * Enable receive and transmit.
@@ -743,22 +740,19 @@ static void qe_uart_init_ucc(struct uart_qe_port *qe_port)
 		 * ...
 		 * 6.Receiver must use 16x over sampling
 		 */
-		clrsetbits_be32(&uccp->gumr_l,
-			UCC_SLOW_GUMR_L_MODE_MASK | UCC_SLOW_GUMR_L_TDCR_MASK |
-			UCC_SLOW_GUMR_L_RDCR_MASK,
-			UCC_SLOW_GUMR_L_MODE_QMC | UCC_SLOW_GUMR_L_TDCR_16 |
-			UCC_SLOW_GUMR_L_RDCR_16);
+		qe_clrsetbits_be32(&uccp->gumr_l,
+				   UCC_SLOW_GUMR_L_MODE_MASK | UCC_SLOW_GUMR_L_TDCR_MASK | UCC_SLOW_GUMR_L_RDCR_MASK,
+				   UCC_SLOW_GUMR_L_MODE_QMC | UCC_SLOW_GUMR_L_TDCR_16 | UCC_SLOW_GUMR_L_RDCR_16);
 
-		clrsetbits_be32(&uccp->gumr_h,
-			UCC_SLOW_GUMR_H_RFW | UCC_SLOW_GUMR_H_RSYN,
-			UCC_SLOW_GUMR_H_SUART | UCC_SLOW_GUMR_H_TRX |
-			UCC_SLOW_GUMR_H_TTX | UCC_SLOW_GUMR_H_TFL);
+		qe_clrsetbits_be32(&uccp->gumr_h,
+				   UCC_SLOW_GUMR_H_RFW | UCC_SLOW_GUMR_H_RSYN,
+				   UCC_SLOW_GUMR_H_SUART | UCC_SLOW_GUMR_H_TRX | UCC_SLOW_GUMR_H_TTX | UCC_SLOW_GUMR_H_TFL);
 
 #ifdef LOOPBACK
-		clrsetbits_be32(&uccp->gumr_l, UCC_SLOW_GUMR_L_DIAG_MASK,
-				UCC_SLOW_GUMR_L_DIAG_LOOP);
-		clrbits32(&uccp->gumr_h, UCC_SLOW_GUMR_H_CTSP |
-			  UCC_SLOW_GUMR_H_CDS);
+		qe_clrsetbits_be32(&uccp->gumr_l, UCC_SLOW_GUMR_L_DIAG_MASK,
+				   UCC_SLOW_GUMR_L_DIAG_LOOP);
+		qe_clrbits_be32(&uccp->gumr_h,
+				UCC_SLOW_GUMR_H_CTSP | UCC_SLOW_GUMR_H_CDS);
 #endif
 
 		cecr_subblock = ucc_slow_get_qe_cr_subblock(qe_port->ucc_num);
@@ -801,7 +795,7 @@ static int qe_uart_startup(struct uart_port *port)
 	}
 
 	/* Startup rx-int */
-	setbits16(&qe_port->uccp->uccm, UCC_UART_UCCE_RX);
+	qe_setbits_be16(&qe_port->uccp->uccm, UCC_UART_UCCE_RX);
 	ucc_slow_enable(qe_port->us_private, COMM_DIR_RX_AND_TX);
 
 	return 0;
@@ -837,7 +831,7 @@ static void qe_uart_shutdown(struct uart_port *port)
 
 	/* Stop uarts */
 	ucc_slow_disable(qe_port->us_private, COMM_DIR_RX_AND_TX);
-	clrbits16(&uccp->uccm, UCC_UART_UCCE_TX | UCC_UART_UCCE_RX);
+	qe_clrbits_be16(&uccp->uccm, UCC_UART_UCCE_TX | UCC_UART_UCCE_RX);
 
 	/* Shut them really down and reinit buffer descriptors */
 	ucc_slow_graceful_stop_tx(qe_port->us_private);
@@ -857,9 +851,9 @@ static void qe_uart_set_termios(struct uart_port *port,
 	struct ucc_slow __iomem *uccp = qe_port->uccp;
 	unsigned int baud;
 	unsigned long flags;
-	u16 upsmr = in_be16(&uccp->upsmr);
+	u16 upsmr = qe_ioread16be(&uccp->upsmr);
 	struct ucc_uart_pram __iomem *uccup = qe_port->uccup;
-	u16 supsmr = in_be16(&uccup->supsmr);
+	u16 supsmr = qe_ioread16be(&uccup->supsmr);
 	u8 char_length = 2; /* 1 + CL + PEN + 1 + SL */
 
 	/* Character length programmed into the mode register is the
@@ -957,10 +951,10 @@ static void qe_uart_set_termios(struct uart_port *port,
 	/* Update the per-port timeout. */
 	uart_update_timeout(port, termios->c_cflag, baud);
 
-	out_be16(&uccp->upsmr, upsmr);
+	qe_iowrite16be(upsmr, &uccp->upsmr);
 	if (soft_uart) {
-		out_be16(&uccup->supsmr, supsmr);
-		out_8(&uccup->rx_length, char_length);
+		qe_iowrite16be(supsmr, &uccup->supsmr);
+		qe_iowrite8(char_length, &uccup->rx_length);
 
 		/* Soft-UART requires a 1X multiplier for TX */
 		qe_setbrg(qe_port->us_info.rx_clock, baud, 16);
-- 
2.23.0

[PATCH v3 30/36] serial: ucc_uart: factor out soft_uart initialization

[Rasmus Villemoes]

The "soft uart" mechanism is a workaround for a silicon bug which (as
far as I know) only affects some PPC-based SOCs.

The code that determines which microcode blob to request relies on
some powerpc-specific bits (e.g. the mfspr(SPRN_SVR) and hence also
the asm/reg.h header). This makes it a little awkward to allow this
driver to be built for non-PPC based SOCs with a QE, even if they are
not affected by that silicon bug and thus don't need any of the Soft
UART logic.

There's no way around guarding those bits with some ifdeffery, so to
keep that isolated, factor out the
do-we-need-soft-uart-and-if-so-handle-the-firmware to a separate
function, which we can then easily stub out for non-PPC.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/tty/serial/ucc_uart.c | 110 ++++++++++++++++++----------------
 1 file changed, 58 insertions(+), 52 deletions(-)

diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index 8a378ee5d34f..f286e91714cb 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -1183,70 +1183,76 @@ static void uart_firmware_cont(const struct firmware *fw, void *context)
 	release_firmware(fw);
 }
 
-static int ucc_uart_probe(struct platform_device *ofdev)
+static int soft_uart_init(struct platform_device *ofdev)
 {
 	struct device_node *np = ofdev->dev.of_node;
-	const unsigned int *iprop;      /* Integer OF properties */
-	const char *sprop;      /* String OF properties */
-	struct uart_qe_port *qe_port = NULL;
-	struct resource res;
+	struct qe_firmware_info *qe_fw_info;
 	int ret;
 
-	/*
-	 * Determine if we need Soft-UART mode
-	 */
 	if (of_find_property(np, "soft-uart", NULL)) {
 		dev_dbg(&ofdev->dev, "using Soft-UART mode\n");
 		soft_uart = 1;
+	} else {
+		return 0;
 	}
 
-	/*
-	 * If we are using Soft-UART, determine if we need to upload the
-	 * firmware, too.
-	 */
-	if (soft_uart) {
-		struct qe_firmware_info *qe_fw_info;
-
-		qe_fw_info = qe_get_firmware_info();
-
-		/* Check if the firmware has been uploaded. */
-		if (qe_fw_info && strstr(qe_fw_info->id, "Soft-UART")) {
-			firmware_loaded = 1;
-		} else {
-			char filename[32];
-			unsigned int soc;
-			unsigned int rev_h;
-			unsigned int rev_l;
-
-			soc = soc_info(&rev_h, &rev_l);
-			if (!soc) {
-				dev_err(&ofdev->dev, "unknown CPU model\n");
-				return -ENXIO;
-			}
-			sprintf(filename, "fsl_qe_ucode_uart_%u_%u%u.bin",
-				soc, rev_h, rev_l);
-
-			dev_info(&ofdev->dev, "waiting for firmware %s\n",
-				filename);
+	qe_fw_info = qe_get_firmware_info();
 
-			/*
-			 * We call request_firmware_nowait instead of
-			 * request_firmware so that the driver can load and
-			 * initialize the ports without holding up the rest of
-			 * the kernel.  If hotplug support is enabled in the
-			 * kernel, then we use it.
-			 */
-			ret = request_firmware_nowait(THIS_MODULE,
-				FW_ACTION_HOTPLUG, filename, &ofdev->dev,
-				GFP_KERNEL, &ofdev->dev, uart_firmware_cont);
-			if (ret) {
-				dev_err(&ofdev->dev,
-					"could not load firmware %s\n",
-					filename);
-				return ret;
-			}
+	/* Check if the firmware has been uploaded. */
+	if (qe_fw_info && strstr(qe_fw_info->id, "Soft-UART")) {
+		firmware_loaded = 1;
+	} else {
+		char filename[32];
+		unsigned int soc;
+		unsigned int rev_h;
+		unsigned int rev_l;
+
+		soc = soc_info(&rev_h, &rev_l);
+		if (!soc) {
+			dev_err(&ofdev->dev, "unknown CPU model\n");
+			return -ENXIO;
+		}
+		sprintf(filename, "fsl_qe_ucode_uart_%u_%u%u.bin",
+			soc, rev_h, rev_l);
+
+		dev_info(&ofdev->dev, "waiting for firmware %s\n",
+			 filename);
+
+		/*
+		 * We call request_firmware_nowait instead of
+		 * request_firmware so that the driver can load and
+		 * initialize the ports without holding up the rest of
+		 * the kernel.  If hotplug support is enabled in the
+		 * kernel, then we use it.
+		 */
+		ret = request_firmware_nowait(THIS_MODULE,
+					      FW_ACTION_HOTPLUG, filename, &ofdev->dev,
+					      GFP_KERNEL, &ofdev->dev, uart_firmware_cont);
+		if (ret) {
+			dev_err(&ofdev->dev,
+				"could not load firmware %s\n",
+				filename);
+			return ret;
 		}
 	}
+	return 0;
+}
+
+static int ucc_uart_probe(struct platform_device *ofdev)
+{
+	struct device_node *np = ofdev->dev.of_node;
+	const unsigned int *iprop;      /* Integer OF properties */
+	const char *sprop;      /* String OF properties */
+	struct uart_qe_port *qe_port = NULL;
+	struct resource res;
+	int ret;
+
+	/*
+	 * Determine if we need Soft-UART mode
+	 */
+	ret = soft_uart_init(ofdev);
+	if (ret)
+		return ret;
 
 	qe_port = kzalloc(sizeof(struct uart_qe_port), GFP_KERNEL);
 	if (!qe_port) {
-- 
2.23.0

[PATCH v3 31/36] serial: ucc_uart: stub out soft_uart_init for !CONFIG_PPC32

[Rasmus Villemoes]

The Soft UART hack is only needed for some PPC-based SOCs. To allow
building this driver for non-PPC, guard soft_uart_init() and its
helpers by CONFIG_PPC32, and use a no-op soft_uart_init() otherwise.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/tty/serial/ucc_uart.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index f286e91714cb..313697842e24 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -33,7 +33,10 @@
 
 #include <linux/firmware.h>
 #include <soc/fsl/cpm.h>
-#include <asm/reg.h>
+
+#ifdef CONFIG_PPC32
+#include <asm/reg.h> /* mfspr, SPRN_SVR */
+#endif
 
 /*
  * The GUMR flag for Soft UART.  This would normally be defined in qe.h,
@@ -1096,6 +1099,8 @@ static const struct uart_ops qe_uart_pops = {
 	.verify_port    = qe_uart_verify_port,
 };
 
+
+#ifdef CONFIG_PPC32
 /*
  * Obtain the SOC model number and revision level
  *
@@ -1238,6 +1243,16 @@ static int soft_uart_init(struct platform_device *ofdev)
 	return 0;
 }
 
+#else /* !CONFIG_PPC32 */
+
+static int soft_uart_init(struct platform_device *ofdev)
+{
+	return 0;
+}
+
+#endif
+
+
 static int ucc_uart_probe(struct platform_device *ofdev)
 {
 	struct device_node *np = ofdev->dev.of_node;
-- 
2.23.0

Re: [PATCH v3 31/36] serial: ucc_uart: stub out soft_uart_init for !CONFIG_PPC32

[Christophe Leroy]

Le 01/11/2019 à 13:42, Rasmus Villemoes a écrit :
> The Soft UART hack is only needed for some PPC-based SOCs. To allow
> building this driver for non-PPC, guard soft_uart_init() and its
> helpers by CONFIG_PPC32, and use a no-op soft_uart_init() otherwise.

I don't like too much ifdefs in C files, especially arch specific ifdefs 
in generic drivers.

How do we get the QE version on ARM ?

Wouldn't it be cleaner to create a helper for getting the QE version, 
which would be defined in an arch specific header file, calling 
mfspr(SPRN_SVR) on powerpc and whatever needed on arm ?

Christophe

> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>   drivers/tty/serial/ucc_uart.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
> index f286e91714cb..313697842e24 100644
> --- a/drivers/tty/serial/ucc_uart.c
> +++ b/drivers/tty/serial/ucc_uart.c
> @@ -33,7 +33,10 @@
>   
>   #include <linux/firmware.h>
>   #include <soc/fsl/cpm.h>
> -#include <asm/reg.h>
> +
> +#ifdef CONFIG_PPC32
> +#include <asm/reg.h> /* mfspr, SPRN_SVR */
> +#endif
>   
>   /*
>    * The GUMR flag for Soft UART.  This would normally be defined in qe.h,
> @@ -1096,6 +1099,8 @@ static const struct uart_ops qe_uart_pops = {
>   	.verify_port    = qe_uart_verify_port,
>   };
>   
> +
> +#ifdef CONFIG_PPC32
>   /*
>    * Obtain the SOC model number and revision level
>    *
> @@ -1238,6 +1243,16 @@ static int soft_uart_init(struct platform_device *ofdev)
>   	return 0;
>   }
>   
> +#else /* !CONFIG_PPC32 */
> +
> +static int soft_uart_init(struct platform_device *ofdev)
> +{
> +	return 0;
> +}
> +
> +#endif
> +
> +
>   static int ucc_uart_probe(struct platform_device *ofdev)
>   {
>   	struct device_node *np = ofdev->dev.of_node;
> 

Re: [PATCH v3 31/36] serial: ucc_uart: stub out soft_uart_init for !CONFIG_PPC32

[Rasmus Villemoes]

On 01/11/2019 17.27, Christophe Leroy wrote:
> 
> 
> Le 01/11/2019 à 13:42, Rasmus Villemoes a écrit :
>> The Soft UART hack is only needed for some PPC-based SOCs. To allow
>> building this driver for non-PPC, guard soft_uart_init() and its
>> helpers by CONFIG_PPC32, and use a no-op soft_uart_init() otherwise.
> 
> I don't like too much ifdefs in C files, especially arch specific ifdefs
> in generic drivers.

Me neither. However,

> How do we get the QE version on ARM ?
> 
> Wouldn't it be cleaner to create a helper for getting the QE version,
> which would be defined in an arch specific header file, calling
> mfspr(SPRN_SVR) on powerpc and whatever needed on arm ?

First, that would mean we'd have to introduce "depends on PPC32 || ARM"
rather than drop "depends on PPC32". Second, the way the SOC version is
being used to construct the name of the microcode blob seems very
ppc-specific, so I don't see how one can reasonably define an interface
that would work for all architectures. But it's also kind of moot since
the ARM SOCs don't need this workaround.

IMO, having to add qe-specific code to arch/ is a cure worse than the
disease. I'd much rather add arch-specific code to qe.

Rasmus

[PATCH v3 32/36] serial: ucc_uart: use of_property_read_u32() in ucc_uart_probe()

[Rasmus Villemoes]

For this to work correctly on little-endian hosts, don't access the
device-tree properties directly in native endianness, but use the
of_property_read_u32() helper.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/tty/serial/ucc_uart.c | 41 +++++++++++++++--------------------
 1 file changed, 17 insertions(+), 24 deletions(-)

diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index 313697842e24..f5ea84928a3b 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -1256,10 +1256,10 @@ static int soft_uart_init(struct platform_device *ofdev)
 static int ucc_uart_probe(struct platform_device *ofdev)
 {
 	struct device_node *np = ofdev->dev.of_node;
-	const unsigned int *iprop;      /* Integer OF properties */
 	const char *sprop;      /* String OF properties */
 	struct uart_qe_port *qe_port = NULL;
 	struct resource res;
+	u32 val;
 	int ret;
 
 	/*
@@ -1290,23 +1290,19 @@ static int ucc_uart_probe(struct platform_device *ofdev)
 
 	/* Get the UCC number (device ID) */
 	/* UCCs are numbered 1-7 */
-	iprop = of_get_property(np, "cell-index", NULL);
-	if (!iprop) {
-		iprop = of_get_property(np, "device-id", NULL);
-		if (!iprop) {
-			dev_err(&ofdev->dev, "UCC is unspecified in "
-				"device tree\n");
-			ret = -EINVAL;
-			goto out_free;
-		}
+	if (of_property_read_u32(np, "cell-index", &val) &&
+	    of_property_read_u32(np, "device-id", &val)) {
+		dev_err(&ofdev->dev, "UCC is unspecified in device tree\n");
+		ret = -EINVAL;
+		goto out_free;
 	}
 
-	if ((*iprop < 1) || (*iprop > UCC_MAX_NUM)) {
-		dev_err(&ofdev->dev, "no support for UCC%u\n", *iprop);
+	if (val < 1 || val > UCC_MAX_NUM) {
+		dev_err(&ofdev->dev, "no support for UCC%u\n", val);
 		ret = -ENODEV;
 		goto out_free;
 	}
-	qe_port->ucc_num = *iprop - 1;
+	qe_port->ucc_num = val - 1;
 
 	/*
 	 * In the future, we should not require the BRG to be specified in the
@@ -1350,13 +1346,12 @@ static int ucc_uart_probe(struct platform_device *ofdev)
 	}
 
 	/* Get the port number, numbered 0-3 */
-	iprop = of_get_property(np, "port-number", NULL);
-	if (!iprop) {
+	if (of_property_read_u32(np, "port-number", &val)) {
 		dev_err(&ofdev->dev, "missing port-number in device tree\n");
 		ret = -EINVAL;
 		goto out_free;
 	}
-	qe_port->port.line = *iprop;
+	qe_port->port.line = val;
 	if (qe_port->port.line >= UCC_MAX_UART) {
 		dev_err(&ofdev->dev, "port-number must be 0-%u\n",
 			UCC_MAX_UART - 1);
@@ -1386,31 +1381,29 @@ static int ucc_uart_probe(struct platform_device *ofdev)
 		}
 	}
 
-	iprop = of_get_property(np, "brg-frequency", NULL);
-	if (!iprop) {
+	if (of_property_read_u32(np, "brg-frequency", &val)) {
 		dev_err(&ofdev->dev,
 		       "missing brg-frequency in device tree\n");
 		ret = -EINVAL;
 		goto out_np;
 	}
 
-	if (*iprop)
-		qe_port->port.uartclk = *iprop;
+	if (val)
+		qe_port->port.uartclk = val;
 	else {
 		/*
 		 * Older versions of U-Boot do not initialize the brg-frequency
 		 * property, so in this case we assume the BRG frequency is
 		 * half the QE bus frequency.
 		 */
-		iprop = of_get_property(np, "bus-frequency", NULL);
-		if (!iprop) {
+		if (of_property_read_u32(np, "bus-frequency", &val)) {
 			dev_err(&ofdev->dev,
 				"missing QE bus-frequency in device tree\n");
 			ret = -EINVAL;
 			goto out_np;
 		}
-		if (*iprop)
-			qe_port->port.uartclk = *iprop / 2;
+		if (val)
+			qe_port->port.uartclk = val / 2;
 		else {
 			dev_err(&ofdev->dev,
 				"invalid QE bus-frequency in device tree\n");
-- 
2.23.0

[PATCH v3 33/36] serial: ucc_uart: access __be32 field using be32_to_cpu

[Rasmus Villemoes]

The buf member of struct qe_bd is a __be32, so to make this work on
little-endian hosts, use be32_to_cpu when reading it.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/tty/serial/ucc_uart.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index f5ea84928a3b..a5330582b610 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -343,7 +343,7 @@ static int qe_uart_tx_pump(struct uart_qe_port *qe_port)
 		/* Pick next descriptor and fill from buffer */
 		bdp = qe_port->tx_cur;
 
-		p = qe2cpu_addr(bdp->buf, qe_port);
+		p = qe2cpu_addr(be32_to_cpu(bdp->buf), qe_port);
 
 		*p++ = port->x_char;
 		qe_iowrite16be(1, &bdp->length);
@@ -371,7 +371,7 @@ static int qe_uart_tx_pump(struct uart_qe_port *qe_port)
 	while (!(qe_ioread16be(&bdp->status) & BD_SC_READY) &&
 	       (xmit->tail != xmit->head)) {
 		count = 0;
-		p = qe2cpu_addr(bdp->buf, qe_port);
+		p = qe2cpu_addr(be32_to_cpu(bdp->buf), qe_port);
 		while (count < qe_port->tx_fifosize) {
 			*p++ = xmit->buf[xmit->tail];
 			xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
@@ -491,7 +491,7 @@ static void qe_uart_int_rx(struct uart_qe_port *qe_port)
 		}
 
 		/* get pointer */
-		cp = qe2cpu_addr(bdp->buf, qe_port);
+		cp = qe2cpu_addr(be32_to_cpu(bdp->buf), qe_port);
 
 		/* loop through the buffer */
 		while (i-- > 0) {
-- 
2.23.0