[U-Boot] [PATCH 07/26] pinctrl: add support for MediaTek MT7628

[Weijie Gao]

On Wed, 2019-08-28 at 14:37 +0200, Stefan Roese wrote:
> On 28.08.19 14:26, Stefan Roese wrote:
> > On 28.08.19 08:37, Weijie Gao wrote:
> >> This patch adds pinctrl support for mt7628, with a file for common pinmux
> >> functions and a file for mt7628 which has additional support for pinconf.
> >>
> >> Signed-off-by: Weijie Gao <weijie.gao at mediatek.com>
> >> ---
> >>    drivers/pinctrl/Kconfig                       |   1 +
> >>    drivers/pinctrl/Makefile                      |   1 +
> >>    drivers/pinctrl/mtmips/Kconfig                |  13 +
> >>    drivers/pinctrl/mtmips/Makefile               |   7 +
> >>    drivers/pinctrl/mtmips/pinctrl-mt7628.c       | 585 ++++++++++++++++++
> >>    .../pinctrl/mtmips/pinctrl-mtmips-common.c    |  87 +++
> >>    .../pinctrl/mtmips/pinctrl-mtmips-common.h    |  53 ++
> >>    7 files changed, 747 insertions(+)
> >>    create mode 100644 drivers/pinctrl/mtmips/Kconfig
> >>    create mode 100644 drivers/pinctrl/mtmips/Makefile
> >>    create mode 100644 drivers/pinctrl/mtmips/pinctrl-mt7628.c
> >>    create mode 100644 drivers/pinctrl/mtmips/pinctrl-mtmips-common.c
> >>    create mode 100644 drivers/pinctrl/mtmips/pinctrl-mtmips-common.h
> > 
> > Nice patch. I do have 2 questions though:
> > 
> > a) Why are you introducing a new "mtmips" directory and don't re-use
> > the already available "mediatek" directory? Is there nothing in
> > common with these "mediatek" drivers?
> > 
> > b) Somewhat related: You introduce a mtmips-common file. For which
> > platforms is this targeted (non-mt7628)? Is there nothing in common
> > with the "mediatek" files already available?
> > 
> > Other than that I've tested this on my MT7688 board and it works
> > just fine. Thanks a lot!
> 
> I do have another comment though:
> 
> I've used the common "pinctrl-single" driver in Linux a few weeks ago as
> there is no need for a separate MT7628 specific pin-mux driver [1][2] etc.
> Frankly, I don't know that status of the "pinctrl-single" U-Boot driver
> in depth. If its compatible with the Linux one (which I really hope), then
> we don't need a MT7628 specific pinctrl driver but can use the "pinctrl-single"
> driver as I've done in the Linux [1][2].
> 
> It would be great if you could check this and change this pinctrl support
> to the common "single" driver is possible.
> 
> Thanks,
> Stefan
> 
> [1] https://github.com/torvalds/linux/commit/380f072c57a590d7593050b8533d88e18b6a7daa
> [2] https://github.com/torvalds/linux/commit/6394de396ed36f3e8043734676eaa9c26f84bb1b

Although I am trying to write the pinctrl driver to be similar with the
"mediatek" one, they cannot share the same codebase because the register
map and definition are totally different. The "mtmips" chips have
separate pinmux registers, pinconf registers and gpio registers, while
the "mediatek" chips have uniformed registers for all these three
functionalities.

I introduced the common file to reserve space for mt7620.

If I use pinctrl-single, I still have to write a new driver for the
pinconf function and I'd rather just write them into one driver.

Besides the pinmux function takes advantages from the OpenWrt source.
Since some pinmux groups have 4 different function, I think is more
readable using the string configuration. And the users don't need to
read the programming guide to set the registers/shift/mask for a single
pinmux function.

Please see
https://github.com/torvalds/linux/blob/master/arch/mips/ralink/mt7620.c

Best Regards,

Weijie