Re: [PATCH] pinctrl: armada-37xx: Correct mpp definitions

[Gregory CLEMENT <gregory.clement@xxxxxxxxxxx>]

Hi Linus,
 
 On mer., sept. 05 2018, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:

> Sorry for top posting.
>
> You need to include Gregory CLEMENT on reviews for this driver and
> bindings.
>
> Gregory, does this look OK?
>
> Yours,
> Linus Walleij
>
> On Thu, Aug 30, 2018 at 4:05 PM Marek Behún <marek.behun@xxxxxx> wrote:
>>
>> From: Ken Ma <make@xxxxxxxxxxx>
>>
>> This patch icorrects below mpp definitions:
>>  - The sdio_sb group is composed of 6 pins and not 5;
>>  - The rgmii group contains pins mpp2[17:6] and not mpp2[19:6];
>>  - Pin of group "pmic0" is mpp1[6] but not mpp1[16];
>>  - Pin of group "pmic1" is mpp1[7] but not mpp1[17];
>>  - A new group "smi" is added in A0 with 2 pins - mpp2[19:18], its
>>    bitmask is bit4;
>>  - Group "pcie1" has 3 pins in A0 - mpp2[5:3], its bit mask is
>>    bit5 | bit9 | bit10 but not bit4;
>>  - Group "ptp" has 3 pins in A0 as Z1, but its bitmask is changed to
>>    bit11 | bit12 | bit13.
>>
>> Reviewed-on: http://vgitil04.il.marvell.com:8080/41830
>> Reviewed-on: http://vgitil04.il.marvell.com:8080/42774
>> Reviewed-on: http://vgitil04.il.marvell.com:8080/41970
>> Reviewed-on: http://vgitil04.il.marvell.com:8080/42775
>> Reviewed-by: Wilson Ding <dingwei@xxxxxxxxxxx>
>> Reviewed-by: Evan Wang <xswang@xxxxxxxxxxx>
>> Reviewed-by: Victor Gu <xigu@xxxxxxxxxxx>
>> Tested-by: Wilson Ding <dingwei@xxxxxxxxxxx>
>> Tested-by: iSoC Platform CI <ykjenk@xxxxxxxxxxx>
>> Tested-by: Victor Gu <xigu@xxxxxxxxxxx>
>> Verified-Armada37x0: Wilson Ding <dingwei@xxxxxxxxxxx>
>> Signed-off-by: Ken Ma <make@xxxxxxxxxxx>
>> Signed-off-by: Marek Behun <marek.behun@xxxxxx>

This patch comes directly from Marvell LSP and should be clean-up.

Usually I removed all the tag excepting the SoB ones as the Reviewed-by
were done behind closed door.

Moreover the commit log described fix that was already applied!


>> ---
>>  .../bindings/pinctrl/marvell,armada-37xx-pinctrl.txt       | 14 +++++++++-----
>>  drivers/pinctrl/mvebu/pinctrl-armada-37xx.c                |  9 +++++----
>>  2 files changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
>> index c7c088d2dd50..9c1499b2f4c4 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,21 @@ 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-41
>>   - 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..200c9e620dfc 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,9 @@ 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, 3, BIT(5) | BIT(9) | BIT(10),
>> "pcie"),
On the Rev C of the datasheet the bit 10 is marked as reserved
so this line should be:
+       PIN_GRP_GPIO("pcie1", 3, 2, BIT(5) | 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),

The other changes they are OK : I've just download the last revision of
the datasheet which confirm these new values.

Gregory

>> --
>> 2.16.4
>>

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com