[RFC PATCH] powerpc, qe: add DTS support for parallel I/O ports

[Heiko Schocher]

Hello Qiang Zhao,

Am 15.04.2020 um 10:10 schrieb Qiang Zhao:
> On 2020/4/15 12:46 Heiko Schocher <hs at denx.de>
> 
>> -----Original Message-----
>> From: Heiko Schocher <hs at denx.de>
>> Sent: 2020年4月15日 12:46
>> To: Qiang Zhao <qiang.zhao at nxp.com>
>> Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Holger Brunck
>> <holger.brunck at ch.abb.com>; Mario Six <mario.six at gdsys.cc>; Wolfgang Denk
>> <wd at denx.de>; Holger Brunck <holger.brunck at ch.abb.com>
>> Subject: Re: [RFC PATCH] powerpc, qe: add DTS support for parallel I/O ports
>>
>> Hello Qiang Zhao,
>>
>> Am 09.04.2020 um 08:58 schrieb Qiang Zhao:
>>> On 2020/2/18 17:05, Heiko Schocher <hs at denx.de <mailto:hs at denx.de>>
>> wrote:
>>>
>>>> -----Original Message-----
>>>
>>>> From: Heiko Schocher <hs at denx.de>
>>>
>>>> Sent: 2020年2月18日17:05
>>>
>>>> To: U-Boot Mailing List <u-boot at lists.denx.de>
>>>
>>>> Cc: Heiko Schocher <hs at denx.de>; Holger Brunck
>>>
>>>> <holger.brunck at ch.abb.com>; Mario Six <mario.six at gdsys.cc>; Qiang
>>>> Zhao
>>>
>>>> <qiang.zhao at nxp.com>; Wolfgang Denk <wd at denx.de>
>>>
>>>> Subject: [RFC PATCH] powerpc, qe: add DTS support for parallel I/O
>>>> ports
>>>
>>>>
>>>
>>>> add DM support for parallel I/O ports on QUICC Engine Block
>>>
>>>>
>>>
>>>> Signed-off-by: Heiko Schocher <hs at denx.de <mailto:hs at denx.de>>
>>>
>>>> ---
>>>
>>>>
>>>
>>>>
>>>
>>>>   arch/powerpc/cpu/mpc83xx/cpu_init.c |   8 ++
>>>
>>>>   arch/powerpc/cpu/mpc83xx/qe_io.c    | 193
>>>
>>>> +++++++++++++++++++++++++++-
>>>
>>>>   include/fsl_qe.h                    |   3 +
>>>
>>>>   3 files changed, 201 insertions(+), 3 deletions(-)
>>>
>>>>
>>>
>>>> diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c
>>>
>>>> b/arch/powerpc/cpu/mpc83xx/cpu_init.c
>>>
>>>> index af8facad53..cfcc16607c 100644
>>>
>>>> --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c
>>>
>>>> +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c
>>>
>>> *//*
>>>
>>> How about mpc85xx?
>>
>> Yes, this is a Todo (one reason why this patch is RFC). I currently have reworked
>> this for the mpc83xx based keymile (now abb) boards (which are locally
>> running with complete DM/DTS support now).
> 
> You mean you have tested RFC patch on your side? That's great!

Yes, on the following boards:

"kmcoge5ne tuxx1 kmopti2 kmtegr1 tuge1 kmsupx5"

Not all in mainline (yet)

@Holger: should/could we mainline the missing ones?

This boards work with full DM/DTS support now, and have no compiletime
warnign like:

     ===================== WARNING ======================
     This board does not use CONFIG_DM_ETH (Driver Model

anymore...

> Would you like to push the dts patch too?

Yes, but give me some time, until I can test the changed patchset again.

>> I have one mpx85xx based board I can try, but may others are working on this
>> support, so I wanted to post my patches very early, so we can work together on
>> it...
>>
>> As my first question in patch comment is:
>>
>> may we should move this part to drivers/pinctrl ?
> 
> Yes, it is more better to move to drivers/pinctrl.

Ok, I must look into it.

> 
>>
>>>> @@ -11,6 +11,9 @@
>>>
>>>>   #ifdef CONFIG_USB_EHCI_FSL
>>>
>>>>   #include <usb/ehci-ci.h>
>>>
>>>>   #endif
>>>
>>>> +#ifdef CONFIG_QE
>>>
>>>> +#include <fsl_qe.h>
>>>
>>>> +#endif
>>>
>>> *//*
>>>
>>> If include this headfile, the function declaration for qe_init and
>>> qe_reset in this file should be removed as below:
>>>
>>> extern void qe_init(uint qe_base);
>>>
>>> extern void qe_reset(void);
>>
>> Yes, of course! But this part of code I did not touch... may this needs a cleanup
>> at all.
>   
> Just two lines extern declaration in this file.

I remove them in seperate patch.

> 
>>
>>>
>>>>
>>>
>>>>   #include "lblaw/lblaw.h"
>>>
>>>>   #include "elbc/elbc.h"
>>>
>>>> @@ -27,6 +30,7 @@ extern void qe_config_iopin(u8 port, u8 pin, int
>>>> dir,
>>>
>>>> extern void qe_init(uint qe_base);  extern void qe_reset(void);
>>>
>>>>
>>>
>>>> +/**
>>>
>>>> + * par_io_of_config_node  config
>>>
>>>> + * @dev:     pointer to pinctrl device
>>>
>>>> + * @pio:      ofnode of pinconfig property
>>>
>>>> + */
>>>
>>>> +static int par_io_of_config_node(struct udevice *dev, ofnode pio) {
>>>
>>>> +   struct qe_io_platdata      *plat = dev->platdata;
>>>
>>>> +   volatile qepio83xx_t        *par_io = plat->base;
>>>
>>>> +   const unsigned int *pio_map;
>>>
>>>> +   int pio_map_len;
>>>
>>>> +
>>>
>>>> +   pio_map = ofnode_get_property(pio, "pio-map", &pio_map_len);
>>>
>>>> +   if (pio_map == NULL)
>>>
>>>> +            return -ENOENT;
>>>
>>>> +
>>>
>>>> +   pio_map_len /= sizeof(unsigned int);
>>>
>>>> +   if ((pio_map_len % 6) != 0) {
>>>
>>>> +            printk(KERN_ERR "pio-map format wrong!\n");
>>>
>>>> +            return -EINVAL;
>>>
>>>> +   }
>>>
>>>> +
>>>
>>>> +   while (pio_map_len > 0) {
>>>
>>>> +            /*
>>>
>>>> +            * column pio_map[5] from linux (has_irq) not
>>>
>>>> +            * supported in u-boot yet.
>>>
>>>> +            */
>>>
>>> *//*
>>>
>>> remove or keep pio_map[5] in uboot dts, which is better?
>>
>> I think we keep it as it is in linux dts. We simply ignore it (yet) in U-Boot.
>   
> That's OK.
> 
>>
>>>
>>>> +
>>>
>>>> +const struct pinctrl_ops par_io_pinctrl_ops = {
>>>
>>>> +   .set_state = par_io_pinctrl_set_state, };
>>>
>>>> +
>>>
>>>> +static const struct udevice_id par_io_pinctrl_match[] = {
>>>
>>>> +   { .compatible = "fsl,mpc8360-par_io"},
>>>
>>> *//*
>>>
>>> Why is*//*fsl,mpc8360-par_io, maybe fsl,qe-par-io are more better.*//*
>>
>> Yes, more common, but in Linux is already:
>>
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kerne
>> l.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree
>> %2Farch%2Fpowerpc%2Fboot%2Fdts%2Fkmeter1.dts%23n133&data=02%
>> 7C01%7Cqiang.zhao%40nxp.com%7C783271f84fe043eaf21708d7e0f7df46%7
>> C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637225227517025151
>> &sdata=5VZsJPI2QwZBnS5bgSDC1cTF1lKh%2BGQXHpP%2FF4jGzCU%3D&
>> amp;reserved=0
>>
>> or
>> compatible = "fsl,mpc8323-qe-pario";
>>
>> for board
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kerne
>> l.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree
>> %2Farch%2Fpowerpc%2Fboot%2Fdts%2Fmpc832x_rdb.dts%23n163&data
>> =02%7C01%7Cqiang.zhao%40nxp.com%7C783271f84fe043eaf21708d7e0f7df4
>> 6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637225227517025
>> 151&sdata=AwMW4NtqID9GwbM9RnZcqqDQVZ7RvuaRw5U6LFC3VWA%
>> 3D&reserved=0
>>
>> Unfortunately, it is more or less a mess in linux, as each board scans in board
>> code for the node *name* not compatible entry ! See as an example:
>>
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kerne
>> l.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree
>> %2Farch%2Fpowerpc%2Fplatforms%2F83xx%2Fmpc832x_rdb.c%23n198&amp
>> ;data=02%7C01%7Cqiang.zhao%40nxp.com%7C783271f84fe043eaf21708d7e0
>> f7df46%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6372252275
>> 17025151&sdata=RG2pIeiGMFZwL16CLjRHWrvC12YBEDDi5H7kNoZXz0A
>> %3D&reserved=0
>>
>> So may we are free here, to add here a new compatible entry "fsl,qe-par-io" ?
>> (But I want to be compatible with linux and accept the same compatible strings
>> as linux has.)
> 
> OK!
> 
> Best Regards
> Qiang Zhao
> 

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de