diff mbox series

[1/3] pinctrl: armada-37xx: Correct mpp definitions

Message ID 20181221173259.8372-2-gregory.clement@bootlin.com
State New
Headers show
Series Few fix for pins configuration on Armada 37xx | expand

Commit Message

Gregory CLEMENT Dec. 21, 2018, 5:32 p.m. UTC 
From: Marek Behún <marek.behun@nic.cz>

This is a cleanup and fix of the patch by Ken Ma <make@marvell.com>.

Fix the mpp definitions according to newest revision of the
specification:
  - northbridge:
    fix pmic1 gpio number to 7
    fix pmic0 gpio number to 6
  - southbridge
    split pcie1 group bit mask to BIT(5) and  BIT(9)
    fix ptp group bit mask to BIT(11) | BIT(12) | BIT(13)
    add smi group with bit mask BIT(4)

[gregory: split the pcie group in 2, as at hardware level they can be
configured separately]
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
 .../pinctrl/marvell,armada-37xx-pinctrl.txt    | 18 +++++++++++++-----
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c    | 10 ++++++----
 2 files changed, 19 insertions(+), 9 deletions(-)

Comments

Marek Behún Dec. 22, 2018, 2:32 a.m. UTC | #1 
On Fri, 21 Dec 2018 18:32:57 +0100
Gregory CLEMENT <gregory.clement@bootlin.com> wrote:

> +	PIN_GRP_GPIO("pcie1", 3, 1, BIT(5), "pcie"),
> +	PIN_GRP_GPIO("pcie1_clkreq", 4, 1, BIT(9), "pcie"),

If the pair is split to clkreq and reset, shouldn't the first be called
pcie1_reset?
Marek
Gregory CLEMENT Dec. 24, 2018, 5:05 p.m. UTC | #2 
Hi Marek,
 
 On sam., déc. 22 2018, Marek Behun <marek.behun@nic.cz> wrote:

> On Fri, 21 Dec 2018 18:32:57 +0100
> Gregory CLEMENT <gregory.clement@bootlin.com> wrote:
>
>> +	PIN_GRP_GPIO("pcie1", 3, 1, BIT(5), "pcie"),
>> +	PIN_GRP_GPIO("pcie1_clkreq", 4, 1, BIT(9), "pcie"),
>
> If the pair is split to clkreq and reset, shouldn't the first be called
> pcie1_reset?

I considered this but chose to keep pcie1 in order to preserve backward
compatibility.

I agree that it is debatable, because without the fix the old device
tree can't work. However I find it better preserving the initial intent
of an existing device tree.

By talking about it, I think about an other option, keeping pcie1 name
to setup the pins 39 and 40 how it was documented. And introducing
pcie1_reset and pcie1_clkreq for new binding. however I don't know how
it could be handle by the pinctrl framework.

Gregory

> Marek
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
index c7c088d2dd50..f69f82741cae 100644
--- a/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
@@ -58,11 +58,11 @@  group pwm3
  - functions pwm, gpio
 
 group pmic1
- - pin 17
+ - pin 7
  - functions pmic, gpio
 
 group pmic0
- - pin 16
+ - pin 6
  - functions pmic, gpio
 
 group i2c2
@@ -112,17 +112,25 @@  group usb2_drvvbus1
  - functions drvbus, gpio
 
 group sdio_sb
- - pins 60-64
+ - pins 60-65
  - functions sdio, gpio
 
 group rgmii
- - pins 42-55
+ - pins 42-53
  - functions mii, gpio
 
 group pcie1
- - pins 39-40
+ - pins 39
+ - functions pcie, gpio
+
+group pcie1_clkreq
+ - pins 40
  - functions pcie, gpio
 
+group smi
+ - pins 54-55
+ - functions smi, gpio
+
 group ptp
  - pins 56-58
  - functions ptp, gpio
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index aa48b3f23c7f..267dfc530985 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -170,8 +170,8 @@  static struct armada_37xx_pin_group armada_37xx_nb_groups[] = {
 	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("pmic1", 17, 1, BIT(7), "pmic"),
-	PIN_GRP_GPIO("pmic0", 16, 1, BIT(8), "pmic"),
+	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"),
 	PIN_GRP_GPIO("i2c1", 0, 2, BIT(10), "i2c"),
 	PIN_GRP_GPIO("spi_cs1", 17, 1, BIT(12), "spi"),
@@ -195,8 +195,10 @@  static struct armada_37xx_pin_group armada_37xx_sb_groups[] = {
 	PIN_GRP_GPIO("usb2_drvvbus1", 1, 1, BIT(1), "drvbus"),
 	PIN_GRP_GPIO("sdio_sb", 24, 6, BIT(2), "sdio"),
 	PIN_GRP_GPIO("rgmii", 6, 12, BIT(3), "mii"),
-	PIN_GRP_GPIO("pcie1", 3, 2, BIT(4), "pcie"),
-	PIN_GRP_GPIO("ptp", 20, 3, BIT(5), "ptp"),
+	PIN_GRP_GPIO("smi", 18, 2, BIT(4), "smi"),
+	PIN_GRP_GPIO("pcie1", 3, 1, BIT(5), "pcie"),
+	PIN_GRP_GPIO("pcie1_clkreq", 4, 1, BIT(9), "pcie"),
+	PIN_GRP_GPIO("ptp", 20, 3, BIT(11) | BIT(12) | BIT(13), "ptp"),
 	PIN_GRP("ptp_clk", 21, 1, BIT(6), "ptp", "mii"),
 	PIN_GRP("ptp_trig", 22, 1, BIT(7), "ptp", "mii"),
 	PIN_GRP_GPIO_3("mii_col", 23, 1, BIT(8) | BIT(14), 0, BIT(8), BIT(14),