| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMhs-H9CEvN8QXcQgin6NRZYzYFn5T82tLsu4rBbSMMBy=Xoiw@mail.gmail.com>
Date: Tue, 28 May 2024 09:10:18 +0200
From: Sergio Paracuellos <sergio.paracuellos@...il.com>
To: liweihao <cn.liweihao@...il.com>
Cc: arinc.unal@...nc9.com, sean.wang@...nel.org, linus.walleij@...aro.org,
matthias.bgg@...il.com, angelogioacchino.delregno@...labora.com,
linux-mips@...r.kernel.org, linux-gpio@...r.kernel.org,
linux-kernel@...r.kernel.org, Shiji Yang <yangshiji66@...look.com>
Subject: Re: [PATCH] pinctrl: ralink: mt76x8: fix pinmux function
Hi,
On Mon, May 27, 2024 at 4:21 AM liweihao <cn.liweihao@...il.com> wrote:
>
> From: Weihao Li <cn.liweihao@...il.com>
>
> The current version of the pinctrl driver has some issues:
Regarding the dtsi file which is in the kernel tree mt76x8 is using
'pinctrl-single' for pin muxing [0].
>
> 1. Duplicated "gpio" pmx function
>
> The common code will add a "gpio" pmx functon to every pin group, so
> it's not necessary to define a separate "gpio" pmx function in pin
> groups.
Do you mean that pin 0 always has a GPIO function? [1]
> 2. Duplicated pmx function name
>
> There are some same function name in different pin groups, which will
> cause some problems. For example, when we want to use PAD_GPIO0 as
> refclk output function, the common clk framework code will search the
> entire pin function lists, then return the first one matched, in this
> case the matched function list only include the PAD_CO_CLKO pin group
> because there are three "refclk" pin function, which is added by
> refclk_grp, spi_cs1_grp and gpio_grp.
>
> To solve this problem, a simple way is just add a pingrp refix to
> function name like mt7620 pinctrl driver does.
>
> 3. Useless "-" or "rsvd" functon
>
> It's really unnecessary to add a reserved pin mux function to the
> function lists, because we never use it.
>
> Signed-off-by: Weihao Li <cn.liweihao@...il.com>
> ---
> drivers/pinctrl/mediatek/pinctrl-mt76x8.c | 88 +++++++----------------
> 1 file changed, 27 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt76x8.c b/drivers/pinctrl/mediatek/pinctrl-mt76x8.c
> index e7d6ad2f62e4e..2bc8d4409ca27 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mt76x8.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mt76x8.c
> @@ -37,36 +37,30 @@
>
> static struct mtmips_pmx_func pwm1_grp[] = {
> FUNC("sdxc d6", 3, 19, 1),
> - FUNC("utif", 2, 19, 1),
> - FUNC("gpio", 1, 19, 1),
> + FUNC("pwm1 utif", 2, 19, 1),
> FUNC("pwm1", 0, 19, 1),
> };
>
> static struct mtmips_pmx_func pwm0_grp[] = {
> FUNC("sdxc d7", 3, 18, 1),
> - FUNC("utif", 2, 18, 1),
> - FUNC("gpio", 1, 18, 1),
> + FUNC("pwm0 utif", 2, 18, 1),
> FUNC("pwm0", 0, 18, 1),
> };
>
> static struct mtmips_pmx_func uart2_grp[] = {
> FUNC("sdxc d5 d4", 3, 20, 2),
> - FUNC("pwm", 2, 20, 2),
> - FUNC("gpio", 1, 20, 2),
> + FUNC("uart2 pwm", 2, 20, 2),
> FUNC("uart2", 0, 20, 2),
> };
>
> static struct mtmips_pmx_func uart1_grp[] = {
> FUNC("sw_r", 3, 45, 2),
> - FUNC("pwm", 2, 45, 2),
> - FUNC("gpio", 1, 45, 2),
> + FUNC("uart1 pwm", 2, 45, 2),
> FUNC("uart1", 0, 45, 2),
> };
>
> static struct mtmips_pmx_func i2c_grp[] = {
> - FUNC("-", 3, 4, 2),
> FUNC("debug", 2, 4, 2),
> - FUNC("gpio", 1, 4, 2),
> FUNC("i2c", 0, 4, 2),
> };
>
> @@ -76,128 +70,100 @@ static struct mtmips_pmx_func wdt_grp[] = { FUNC("wdt", 0, 38, 1) };
> static struct mtmips_pmx_func spi_grp[] = { FUNC("spi", 0, 7, 4) };
>
> static struct mtmips_pmx_func sd_mode_grp[] = {
> - FUNC("jtag", 3, 22, 8),
> - FUNC("utif", 2, 22, 8),
> - FUNC("gpio", 1, 22, 8),
> + FUNC("sdxc jtag", 3, 22, 8),
> + FUNC("sdxc utif", 2, 22, 8),
> FUNC("sdxc", 0, 22, 8),
> };
>
> static struct mtmips_pmx_func uart0_grp[] = {
> - FUNC("-", 3, 12, 2),
> - FUNC("-", 2, 12, 2),
> - FUNC("gpio", 1, 12, 2),
> FUNC("uart0", 0, 12, 2),
> };
>
> static struct mtmips_pmx_func i2s_grp[] = {
> FUNC("antenna", 3, 0, 4),
> FUNC("pcm", 2, 0, 4),
> - FUNC("gpio", 1, 0, 4),
> FUNC("i2s", 0, 0, 4),
> };
>
> static struct mtmips_pmx_func spi_cs1_grp[] = {
> - FUNC("-", 3, 6, 1),
> - FUNC("refclk", 2, 6, 1),
> - FUNC("gpio", 1, 6, 1),
> + FUNC("spi refclk", 2, 6, 1),
> FUNC("spi cs1", 0, 6, 1),
> };
>
> static struct mtmips_pmx_func spis_grp[] = {
> FUNC("pwm_uart2", 3, 14, 4),
> - FUNC("utif", 2, 14, 4),
> - FUNC("gpio", 1, 14, 4),
> + FUNC("spis utif", 2, 14, 4),
> FUNC("spis", 0, 14, 4),
> };
>
> static struct mtmips_pmx_func gpio_grp[] = {
> FUNC("pcie", 3, 11, 1),
> - FUNC("refclk", 2, 11, 1),
> - FUNC("gpio", 1, 11, 1),
> - FUNC("gpio", 0, 11, 1),
> + FUNC("gpio refclk", 2, 11, 1),
> };
>
> static struct mtmips_pmx_func p4led_kn_grp[] = {
> - FUNC("jtag", 3, 30, 1),
> - FUNC("utif", 2, 30, 1),
> - FUNC("gpio", 1, 30, 1),
> + FUNC("p4led_kn jtag", 3, 30, 1),
> + FUNC("p4led_kn utif", 2, 30, 1),
> FUNC("p4led_kn", 0, 30, 1),
> };
>
> static struct mtmips_pmx_func p3led_kn_grp[] = {
> - FUNC("jtag", 3, 31, 1),
> - FUNC("utif", 2, 31, 1),
> - FUNC("gpio", 1, 31, 1),
> + FUNC("p3led_kn jtag", 3, 31, 1),
> + FUNC("p3led_kn utif", 2, 31, 1),
> FUNC("p3led_kn", 0, 31, 1),
> };
>
> static struct mtmips_pmx_func p2led_kn_grp[] = {
> - FUNC("jtag", 3, 32, 1),
> - FUNC("utif", 2, 32, 1),
> - FUNC("gpio", 1, 32, 1),
> + FUNC("p2led_kn jtag", 3, 32, 1),
> + FUNC("p2led_kn utif", 2, 32, 1),
> FUNC("p2led_kn", 0, 32, 1),
> };
>
> static struct mtmips_pmx_func p1led_kn_grp[] = {
> - FUNC("jtag", 3, 33, 1),
> - FUNC("utif", 2, 33, 1),
> - FUNC("gpio", 1, 33, 1),
> + FUNC("p1led_kn jtag", 3, 33, 1),
> + FUNC("p1led_kn utif", 2, 33, 1),
> FUNC("p1led_kn", 0, 33, 1),
> };
>
> static struct mtmips_pmx_func p0led_kn_grp[] = {
> - FUNC("jtag", 3, 34, 1),
> - FUNC("rsvd", 2, 34, 1),
> - FUNC("gpio", 1, 34, 1),
> + FUNC("p0led_kn jtag", 3, 34, 1),
> FUNC("p0led_kn", 0, 34, 1),
> };
>
> static struct mtmips_pmx_func wled_kn_grp[] = {
> - FUNC("rsvd", 3, 35, 1),
> - FUNC("rsvd", 2, 35, 1),
> - FUNC("gpio", 1, 35, 1),
> FUNC("wled_kn", 0, 35, 1),
> };
>
> static struct mtmips_pmx_func p4led_an_grp[] = {
> - FUNC("jtag", 3, 39, 1),
> - FUNC("utif", 2, 39, 1),
> - FUNC("gpio", 1, 39, 1),
> + FUNC("p4led_an jtag", 3, 39, 1),
> + FUNC("p4led_an utif", 2, 39, 1),
> FUNC("p4led_an", 0, 39, 1),
> };
>
> static struct mtmips_pmx_func p3led_an_grp[] = {
> - FUNC("jtag", 3, 40, 1),
> - FUNC("utif", 2, 40, 1),
> - FUNC("gpio", 1, 40, 1),
> + FUNC("p3led_an jtag", 3, 40, 1),
> + FUNC("p3led_an utif", 2, 40, 1),
> FUNC("p3led_an", 0, 40, 1),
> };
>
> static struct mtmips_pmx_func p2led_an_grp[] = {
> - FUNC("jtag", 3, 41, 1),
> - FUNC("utif", 2, 41, 1),
> - FUNC("gpio", 1, 41, 1),
> + FUNC("p2led_an jtag", 3, 41, 1),
> + FUNC("p2led_an utif", 2, 41, 1),
> FUNC("p2led_an", 0, 41, 1),
> };
>
> static struct mtmips_pmx_func p1led_an_grp[] = {
> - FUNC("jtag", 3, 42, 1),
> - FUNC("utif", 2, 42, 1),
> - FUNC("gpio", 1, 42, 1),
> + FUNC("p1led_an jtag", 3, 42, 1),
> + FUNC("p1led_an utif", 2, 42, 1),
> FUNC("p1led_an", 0, 42, 1),
> };
>
> static struct mtmips_pmx_func p0led_an_grp[] = {
> - FUNC("jtag", 3, 43, 1),
> - FUNC("rsvd", 2, 43, 1),
> - FUNC("gpio", 1, 43, 1),
> + FUNC("p0led_an jtag", 3, 43, 1),
> FUNC("p0led_an", 0, 43, 1),
> };
>
> static struct mtmips_pmx_func wled_an_grp[] = {
> - FUNC("rsvd", 3, 44, 1),
> - FUNC("rsvd", 2, 44, 1),
> - FUNC("gpio", 1, 44, 1),
> FUNC("wled_an", 0, 44, 1),
> };
>
> --
> 2.39.2
>
Changes look good to me. However I cannot test them.
[+cc Shiji Yang] who probably has a related board to test.
Thanks,
Sergio Paracuellos
[0]: https://elixir.bootlin.com/linux/latest/source/arch/mips/boot/dts/ralink/mt7628a.dtsi#L45
[1]: https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/mediatek/pinctrl-mtmips.c#L297
Powered by blists - more mailing lists