Message ID | 20210723175711.5469-1-marek.behun@nic.cz |
---|---|
State | Accepted |
Commit | 5534fb4f4833eda4e1a2df1c89a75db72e5e2008 |
Delegated to: | Stefan Roese |
Headers | show |
Series | [u-boot-mvebu] arm64: a37xx: pinctrl: Correct PWM pins definitions | expand |
On 23.07.21 19:57, Marek Behún wrote: > The PWM pins on North Bridge on Armada 37xx can be configured into PWM > or GPIO functions. When in PWM function, each pin can also be configured > to drive low on 0 and tri-state on 1 (LED mode). > > The current definitions handle this by declaring two pin groups for each > pin: > - group "pwmN" with functions "pwm" and "gpio" > - group "ledN_od" ("od" for open drain) with functions "led" and "gpio" > > This is semantically incorrect. The correct definition for each pin > should be one group with three functions: "pwm", "led" and "gpio". > > Change the "pwmN" groups to support "led" function. > > Remove "ledN_od" groups. This cannot break backwards compatibility with > older device trees: no device tree uses it since there is no PWM driver > for this SOC yet. Also "ledN_od" groups are not even documented. > > Signed-off-by: Marek Behún <marek.behun@nic.cz> Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan > --- > This was recently applied also in Linux, see > https://lore.kernel.org/linux-gpio/CACRpkdbkWxw8wEOt2nsiYKLw+=TjFrJX=WYALupCtWWc9xGw7A@mail.gmail.com/T/#t > --- > .../pinctrl/marvell,armada-37xx-pinctrl.txt | 8 ++++---- > drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 17 ++++++++--------- > 2 files changed, 12 insertions(+), 13 deletions(-) > > diff --git a/doc/device-tree-bindings/pinctrl/marvell,armada-37xx-pinctrl.txt b/doc/device-tree-bindings/pinctrl/marvell,armada-37xx-pinctrl.txt > index 86ec11361c..3139a99fa9 100644 > --- a/doc/device-tree-bindings/pinctrl/marvell,armada-37xx-pinctrl.txt > +++ b/doc/device-tree-bindings/pinctrl/marvell,armada-37xx-pinctrl.txt > @@ -38,19 +38,19 @@ group emmc_nb > > group pwm0 > - pin 11 (GPIO1-11) > - - functions pwm, gpio > + - functions pwm, led, gpio > > group pwm1 > - pin 12 > - - functions pwm, gpio > + - functions pwm, led, gpio > > group pwm2 > - pin 13 > - - functions pwm, gpio > + - functions pwm, led, gpio > > group pwm3 > - pin 14 > - - functions pwm, gpio > + - functions pwm, led, gpio > > group pmic1 > - pin 7 > diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > index b9d389e70f..1cf1f06f10 100644 > --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > @@ -160,10 +160,14 @@ static struct armada_37xx_pin_group armada_37xx_nb_groups[] = { > PIN_GRP_GPIO("jtag", 20, 5, BIT(0), "jtag"), > PIN_GRP_GPIO("sdio0", 8, 3, BIT(1), "sdio"), > PIN_GRP_GPIO("emmc_nb", 27, 9, BIT(2), "emmc"), > - PIN_GRP_GPIO("pwm0", 11, 1, BIT(3), "pwm"), > - PIN_GRP_GPIO("pwm1", 12, 1, BIT(4), "pwm"), > - PIN_GRP_GPIO("pwm2", 13, 1, BIT(5), "pwm"), > - PIN_GRP_GPIO("pwm3", 14, 1, BIT(6), "pwm"), > + PIN_GRP_GPIO_3("pwm0", 11, 1, BIT(3) | BIT(20), 0, BIT(20), BIT(3), > + "pwm", "led"), > + PIN_GRP_GPIO_3("pwm1", 11, 1, BIT(4) | BIT(21), 0, BIT(21), BIT(4), > + "pwm", "led"), > + PIN_GRP_GPIO_3("pwm2", 11, 1, BIT(5) | BIT(22), 0, BIT(22), BIT(5), > + "pwm", "led"), > + PIN_GRP_GPIO_3("pwm3", 11, 1, BIT(6) | BIT(23), 0, BIT(23), BIT(6), > + "pwm", "led"), > PIN_GRP_GPIO("pmic1", 7, 1, BIT(7), "pmic"), > PIN_GRP_GPIO("pmic0", 6, 1, BIT(8), "pmic"), > PIN_GRP_GPIO("i2c2", 2, 2, BIT(9), "i2c"), > @@ -177,11 +181,6 @@ static struct armada_37xx_pin_group armada_37xx_nb_groups[] = { > PIN_GRP_EXTRA("uart2", 9, 2, BIT(1) | BIT(13) | BIT(14) | BIT(19), > BIT(1) | BIT(13) | BIT(14), BIT(1) | BIT(19), > 18, 2, "gpio", "uart"), > - PIN_GRP_GPIO("led0_od", 11, 1, BIT(20), "led"), > - PIN_GRP_GPIO("led1_od", 12, 1, BIT(21), "led"), > - PIN_GRP_GPIO("led2_od", 13, 1, BIT(22), "led"), > - PIN_GRP_GPIO("led3_od", 14, 1, BIT(23), "led"), > - > }; > > static struct armada_37xx_pin_group armada_37xx_sb_groups[] = { > Viele Grüße, Stefan
On 23.07.21 19:57, Marek Behún wrote: > The PWM pins on North Bridge on Armada 37xx can be configured into PWM > or GPIO functions. When in PWM function, each pin can also be configured > to drive low on 0 and tri-state on 1 (LED mode). > > The current definitions handle this by declaring two pin groups for each > pin: > - group "pwmN" with functions "pwm" and "gpio" > - group "ledN_od" ("od" for open drain) with functions "led" and "gpio" > > This is semantically incorrect. The correct definition for each pin > should be one group with three functions: "pwm", "led" and "gpio". > > Change the "pwmN" groups to support "led" function. > > Remove "ledN_od" groups. This cannot break backwards compatibility with > older device trees: no device tree uses it since there is no PWM driver > for this SOC yet. Also "ledN_od" groups are not even documented. > > Signed-off-by: Marek Behún <marek.behun@nic.cz> Applied to u-boot-marvell/master Thanks, Stefan > --- > This was recently applied also in Linux, see > https://lore.kernel.org/linux-gpio/CACRpkdbkWxw8wEOt2nsiYKLw+=TjFrJX=WYALupCtWWc9xGw7A@mail.gmail.com/T/#t > --- > .../pinctrl/marvell,armada-37xx-pinctrl.txt | 8 ++++---- > drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 17 ++++++++--------- > 2 files changed, 12 insertions(+), 13 deletions(-) > > diff --git a/doc/device-tree-bindings/pinctrl/marvell,armada-37xx-pinctrl.txt b/doc/device-tree-bindings/pinctrl/marvell,armada-37xx-pinctrl.txt > index 86ec11361c..3139a99fa9 100644 > --- a/doc/device-tree-bindings/pinctrl/marvell,armada-37xx-pinctrl.txt > +++ b/doc/device-tree-bindings/pinctrl/marvell,armada-37xx-pinctrl.txt > @@ -38,19 +38,19 @@ group emmc_nb > > group pwm0 > - pin 11 (GPIO1-11) > - - functions pwm, gpio > + - functions pwm, led, gpio > > group pwm1 > - pin 12 > - - functions pwm, gpio > + - functions pwm, led, gpio > > group pwm2 > - pin 13 > - - functions pwm, gpio > + - functions pwm, led, gpio > > group pwm3 > - pin 14 > - - functions pwm, gpio > + - functions pwm, led, gpio > > group pmic1 > - pin 7 > diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > index b9d389e70f..1cf1f06f10 100644 > --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > @@ -160,10 +160,14 @@ static struct armada_37xx_pin_group armada_37xx_nb_groups[] = { > PIN_GRP_GPIO("jtag", 20, 5, BIT(0), "jtag"), > PIN_GRP_GPIO("sdio0", 8, 3, BIT(1), "sdio"), > PIN_GRP_GPIO("emmc_nb", 27, 9, BIT(2), "emmc"), > - PIN_GRP_GPIO("pwm0", 11, 1, BIT(3), "pwm"), > - PIN_GRP_GPIO("pwm1", 12, 1, BIT(4), "pwm"), > - PIN_GRP_GPIO("pwm2", 13, 1, BIT(5), "pwm"), > - PIN_GRP_GPIO("pwm3", 14, 1, BIT(6), "pwm"), > + PIN_GRP_GPIO_3("pwm0", 11, 1, BIT(3) | BIT(20), 0, BIT(20), BIT(3), > + "pwm", "led"), > + PIN_GRP_GPIO_3("pwm1", 11, 1, BIT(4) | BIT(21), 0, BIT(21), BIT(4), > + "pwm", "led"), > + PIN_GRP_GPIO_3("pwm2", 11, 1, BIT(5) | BIT(22), 0, BIT(22), BIT(5), > + "pwm", "led"), > + PIN_GRP_GPIO_3("pwm3", 11, 1, BIT(6) | BIT(23), 0, BIT(23), BIT(6), > + "pwm", "led"), > PIN_GRP_GPIO("pmic1", 7, 1, BIT(7), "pmic"), > PIN_GRP_GPIO("pmic0", 6, 1, BIT(8), "pmic"), > PIN_GRP_GPIO("i2c2", 2, 2, BIT(9), "i2c"), > @@ -177,11 +181,6 @@ static struct armada_37xx_pin_group armada_37xx_nb_groups[] = { > PIN_GRP_EXTRA("uart2", 9, 2, BIT(1) | BIT(13) | BIT(14) | BIT(19), > BIT(1) | BIT(13) | BIT(14), BIT(1) | BIT(19), > 18, 2, "gpio", "uart"), > - PIN_GRP_GPIO("led0_od", 11, 1, BIT(20), "led"), > - PIN_GRP_GPIO("led1_od", 12, 1, BIT(21), "led"), > - PIN_GRP_GPIO("led2_od", 13, 1, BIT(22), "led"), > - PIN_GRP_GPIO("led3_od", 14, 1, BIT(23), "led"), > - > }; > > static struct armada_37xx_pin_group armada_37xx_sb_groups[] = { > Viele Grüße, Stefan
diff --git a/doc/device-tree-bindings/pinctrl/marvell,armada-37xx-pinctrl.txt b/doc/device-tree-bindings/pinctrl/marvell,armada-37xx-pinctrl.txt index 86ec11361c..3139a99fa9 100644 --- a/doc/device-tree-bindings/pinctrl/marvell,armada-37xx-pinctrl.txt +++ b/doc/device-tree-bindings/pinctrl/marvell,armada-37xx-pinctrl.txt @@ -38,19 +38,19 @@ group emmc_nb group pwm0 - pin 11 (GPIO1-11) - - functions pwm, gpio + - functions pwm, led, gpio group pwm1 - pin 12 - - functions pwm, gpio + - functions pwm, led, gpio group pwm2 - pin 13 - - functions pwm, gpio + - functions pwm, led, gpio group pwm3 - pin 14 - - functions pwm, gpio + - functions pwm, led, gpio group pmic1 - pin 7 diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c index b9d389e70f..1cf1f06f10 100644 --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c @@ -160,10 +160,14 @@ static struct armada_37xx_pin_group armada_37xx_nb_groups[] = { PIN_GRP_GPIO("jtag", 20, 5, BIT(0), "jtag"), PIN_GRP_GPIO("sdio0", 8, 3, BIT(1), "sdio"), PIN_GRP_GPIO("emmc_nb", 27, 9, BIT(2), "emmc"), - PIN_GRP_GPIO("pwm0", 11, 1, BIT(3), "pwm"), - PIN_GRP_GPIO("pwm1", 12, 1, BIT(4), "pwm"), - PIN_GRP_GPIO("pwm2", 13, 1, BIT(5), "pwm"), - PIN_GRP_GPIO("pwm3", 14, 1, BIT(6), "pwm"), + PIN_GRP_GPIO_3("pwm0", 11, 1, BIT(3) | BIT(20), 0, BIT(20), BIT(3), + "pwm", "led"), + PIN_GRP_GPIO_3("pwm1", 11, 1, BIT(4) | BIT(21), 0, BIT(21), BIT(4), + "pwm", "led"), + PIN_GRP_GPIO_3("pwm2", 11, 1, BIT(5) | BIT(22), 0, BIT(22), BIT(5), + "pwm", "led"), + PIN_GRP_GPIO_3("pwm3", 11, 1, BIT(6) | BIT(23), 0, BIT(23), BIT(6), + "pwm", "led"), PIN_GRP_GPIO("pmic1", 7, 1, BIT(7), "pmic"), PIN_GRP_GPIO("pmic0", 6, 1, BIT(8), "pmic"), PIN_GRP_GPIO("i2c2", 2, 2, BIT(9), "i2c"), @@ -177,11 +181,6 @@ static struct armada_37xx_pin_group armada_37xx_nb_groups[] = { PIN_GRP_EXTRA("uart2", 9, 2, BIT(1) | BIT(13) | BIT(14) | BIT(19), BIT(1) | BIT(13) | BIT(14), BIT(1) | BIT(19), 18, 2, "gpio", "uart"), - PIN_GRP_GPIO("led0_od", 11, 1, BIT(20), "led"), - PIN_GRP_GPIO("led1_od", 12, 1, BIT(21), "led"), - PIN_GRP_GPIO("led2_od", 13, 1, BIT(22), "led"), - PIN_GRP_GPIO("led3_od", 14, 1, BIT(23), "led"), - }; static struct armada_37xx_pin_group armada_37xx_sb_groups[] = {
The PWM pins on North Bridge on Armada 37xx can be configured into PWM or GPIO functions. When in PWM function, each pin can also be configured to drive low on 0 and tri-state on 1 (LED mode). The current definitions handle this by declaring two pin groups for each pin: - group "pwmN" with functions "pwm" and "gpio" - group "ledN_od" ("od" for open drain) with functions "led" and "gpio" This is semantically incorrect. The correct definition for each pin should be one group with three functions: "pwm", "led" and "gpio". Change the "pwmN" groups to support "led" function. Remove "ledN_od" groups. This cannot break backwards compatibility with older device trees: no device tree uses it since there is no PWM driver for this SOC yet. Also "ledN_od" groups are not even documented. Signed-off-by: Marek Behún <marek.behun@nic.cz> --- This was recently applied also in Linux, see https://lore.kernel.org/linux-gpio/CACRpkdbkWxw8wEOt2nsiYKLw+=TjFrJX=WYALupCtWWc9xGw7A@mail.gmail.com/T/#t --- .../pinctrl/marvell,armada-37xx-pinctrl.txt | 8 ++++---- drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 17 ++++++++--------- 2 files changed, 12 insertions(+), 13 deletions(-)