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....@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to