pinctrl: sun4i: add spdif to pin description.

PATCH Superseded codekipper at gmail.com 3 participants 5 messages Aug 11, 2015 – Aug 12, 2015 2 messages not in the archive

Subsystems (2)

Arm/Allwinner sunXi SoC support Pin control

lore

Share & tools

lore

codekipper at gmail.com

author

2015-08-11 16:19 UTC #0 lore raw

PATCH 1/1 pinctrl: sun4i: add spdif to pin description.

From: Marcus Cooper <[email protected]>

Signed-off-by Marcus Cooper <[email protected]>

---
 drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c +6 -3

index 7376a97..daf7dec 100644
@@ -135,7 +135,8 @@ static const struct sunxi_desc_pin sun4i_a10_pins[] = {
 	SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 3),
 		  SUNXI_FUNCTION(0x0, "gpio_in"),
 		  SUNXI_FUNCTION(0x1, "gpio_out"),
-		  SUNXI_FUNCTION(0x2, "ir0")),		/* TX */
+		  SUNXI_FUNCTION(0x2, "ir0"),		/* TX */
+		  SUNXI_FUNCTION(0x4, "spdif")),        /* MCLK */
 	SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 4),
 		  SUNXI_FUNCTION(0x0, "gpio_in"),
 		  SUNXI_FUNCTION(0x1, "gpio_out"),

@@ -176,11 +177,13 @@ static const struct sunxi_desc_pin sun4i_a10_pins[] = {
 		  SUNXI_FUNCTION(0x0, "gpio_in"),
 		  SUNXI_FUNCTION(0x1, "gpio_out"),
 		  SUNXI_FUNCTION(0x2, "i2s"),		/* DI */
-		  SUNXI_FUNCTION(0x3, "ac97")),		/* DI */
+		  SUNXI_FUNCTION(0x3, "ac97"),		/* DI */
+		  SUNXI_FUNCTION(0x4, "spdif")),        /* DI */
 	SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 13),
 		  SUNXI_FUNCTION(0x0, "gpio_in"),
 		  SUNXI_FUNCTION(0x1, "gpio_out"),
-		  SUNXI_FUNCTION(0x2, "spi2")),		/* CS1 */
+		  SUNXI_FUNCTION(0x2, "spi2"),		/* CS1 */
+		  SUNXI_FUNCTION(0x4, "spdif")),        /* DO */
 	SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 14),
 		  SUNXI_FUNCTION(0x0, "gpio_in"),
 		  SUNXI_FUNCTION(0x1, "gpio_out"),

Signature · 1 line

2.5.0

Chen-Yu Tsai

2015-08-11 16:48 UTC #1 lore raw

PATCH 1/1 pinctrl: sun4i: add spdif to pin description.

Quoted 24 lines · On Wed, Aug 12, 2015 at 12:19 AM, <[email protected]> wrote:

From: Marcus Cooper <[email protected]>

Signed-off-by Marcus Cooper <[email protected]>

---
 drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c +2 -1

index 7376a97..daf7dec 100644
@@ -135,7 +135,8 @@ static const struct sunxi_desc_pin sun4i_a10_pins[] = {
        SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 3),
                  SUNXI_FUNCTION(0x0, "gpio_in"),
                  SUNXI_FUNCTION(0x1, "gpio_out"),
-                 SUNXI_FUNCTION(0x2, "ir0")),          /* TX */
+                 SUNXI_FUNCTION(0x2, "ir0"),           /* TX */
+                 SUNXI_FUNCTION(0x4, "spdif")),        /* MCLK */
        SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 4),
                  SUNXI_FUNCTION(0x0, "gpio_in"),
                  SUNXI_FUNCTION(0x1, "gpio_out"),

@@ -176,11 +177,13 @@ static const struct sunxi_desc_pin sun4i_a10_pins[] = {
                  SUNXI_FUNCTION(0x0, "gpio_in"),
                  SUNXI_FUNCTION(0x1, "gpio_out"),

drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c static const struct sunxi_desc_pin sun4i_a10_pins[] = { · line 186

Quoted 10 lines

drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c +4 -2

                  SUNXI_FUNCTION(0x2, "i2s"),           /* DI */
-                 SUNXI_FUNCTION(0x3, "ac97")),         /* DI */
+                 SUNXI_FUNCTION(0x3, "ac97"),          /* DI */
+                 SUNXI_FUNCTION(0x4, "spdif")),        /* DI */
        SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 13),
                  SUNXI_FUNCTION(0x0, "gpio_in"),
                  SUNXI_FUNCTION(0x1, "gpio_out"),
-                 SUNXI_FUNCTION(0x2, "spi2")),         /* CS1 */
+                 SUNXI_FUNCTION(0x2, "spi2"),          /* CS1 */
+                 SUNXI_FUNCTION(0x4, "spdif")),        /* DO */

The datasheet and manual list them as NC or Reserved. Maybe mention how you knew they were available?

Given that the S/PDIF block is not documented either, you should do the same for the
actual driver, like mention Allwinner kernel sources.

ChenYu

Quoted 10 lines

drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c +0 -1
        SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 14),
                  SUNXI_FUNCTION(0x0, "gpio_in"),
                  SUNXI_FUNCTION(0x1, "gpio_out"),
--
2.5.0
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com. For more options, visit https://groups.google.com/d/optout.

A message that is not in the archive (sent off-list or not yet received): CAEKpxBmS6YJMjREc3gmeSQOzBXFKw=Sb-xPK57oUn6BtmASErg@mail.gmail.com

Hans de Goede

2015-08-12 08:34 UTC #3 lore raw

PATCH 1/1 pinctrl: sun4i: add spdif to pin description.

Hi,

Quoted 41 lines · Code Kipper

Quoted 39 lines · Chen-Yu Tsai

Review

Quoted 8 lines · On Wed, Aug 12, 2015 at 12:19 AM, <[email protected]> wrote:

From: Marcus Cooper <[email protected]>
Signed-off-by Marcus Cooper <[email protected]>
---
  drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c | 9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)
(quoted diff) +0 -0

b/drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c

drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c static const struct sunxi_desc_pin sun4i_a10_pins[]

Quoted 4 lines

index 7376a97..daf7dec 100644
drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c +0 -0
@@ -135,7 +135,8 @@ static const struct sunxi_desc_pin sun4i_a10_pins[]

= {

drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c static const struct sunxi_desc_pin

Quoted 10 lines

drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c +2 -1

         SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 3),
                   SUNXI_FUNCTION(0x0, "gpio_in"),
                   SUNXI_FUNCTION(0x1, "gpio_out"),
-                 SUNXI_FUNCTION(0x2, "ir0")),          /* TX */
+                 SUNXI_FUNCTION(0x2, "ir0"),           /* TX */
+                 SUNXI_FUNCTION(0x4, "spdif")),        /* MCLK */
         SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 4),
                   SUNXI_FUNCTION(0x0, "gpio_in"),
                   SUNXI_FUNCTION(0x1, "gpio_out"),

@@ -176,11 +177,13 @@ static const struct sunxi_desc_pin

sun4i_a10_pins[] = {

Quoted 12 lines

drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c +4 -2

                   SUNXI_FUNCTION(0x0, "gpio_in"),
                   SUNXI_FUNCTION(0x1, "gpio_out"),
                   SUNXI_FUNCTION(0x2, "i2s"),           /* DI */
-                 SUNXI_FUNCTION(0x3, "ac97")),         /* DI */
+                 SUNXI_FUNCTION(0x3, "ac97"),          /* DI */
+                 SUNXI_FUNCTION(0x4, "spdif")),        /* DI */
         SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 13),
                   SUNXI_FUNCTION(0x0, "gpio_in"),
                   SUNXI_FUNCTION(0x1, "gpio_out"),
-                 SUNXI_FUNCTION(0x2, "spi2")),         /* CS1 */
+                 SUNXI_FUNCTION(0x2, "spi2"),          /* CS1 */
+                 SUNXI_FUNCTION(0x4, "spdif")),        /* DO */

Quoted 6 lines

Quoted 4 lines
The datasheet and manual list them as NC or Reserved. Maybe mention how you knew
they were available?
Not sure if this deserves to be in the commit message but I can definitely put together a covering letter with links etc.

I dear to say it belongs right there in the source code, use a multi-line comment above the SUNXI_FUNCTION(0x4, "spdif")) to explain where the mux info comes from. someday sooner or later someone is going to compare the kernel mux table to the datasheet and think "huh, that is not right", that person will be saved an immense amount of time by simply having a comment there.

And do NOT say that this will not happen, because I've already done such a comparison once in the past.

Regards,

Hans

A message that is not in the archive (sent off-list or not yet received): CAEKpxB=UQYEnwXYAzMZA4noR9Xg8wnSjD+H0bNO5GV3dtZQtTg@mail.gmail.com

Chen-Yu Tsai

2015-08-12 09:18 UTC #5 lore raw

PATCH 1/1 pinctrl: sun4i: add spdif to pin description.

Quoted 72 lines · Code Kipper

Quoted 64 lines · Hans de Goede
Hi,
Quoted 50 lines · Code Kipper
Quoted 47 lines · Chen-Yu Tsai
Review
Quoted 9 lines · On Wed, Aug 12, 2015 at 12:19 AM, <[email protected]> wrote:
From: Marcus Cooper <[email protected]>
Signed-off-by Marcus Cooper <[email protected]>
---
  drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c | 9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)
(quoted diff) +0 -0
b/drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c
drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c static const struct sunxi_desc_pin sun4i_a10_pins[]
Quoted 5 lines
index 7376a97..daf7dec 100644
drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c +0 -0
@@ -135,7 +135,8 @@ static const struct sunxi_desc_pin sun4i_a10_pins[]
= {
drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c static const struct sunxi_desc_pin
Quoted 11 lines
drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c +2 -1
         SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 3),
                   SUNXI_FUNCTION(0x0, "gpio_in"),
                   SUNXI_FUNCTION(0x1, "gpio_out"),
-                 SUNXI_FUNCTION(0x2, "ir0")),          /* TX */
+                 SUNXI_FUNCTION(0x2, "ir0"),           /* TX */
+                 SUNXI_FUNCTION(0x4, "spdif")),        /* MCLK */
         SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 4),
                   SUNXI_FUNCTION(0x0, "gpio_in"),
                   SUNXI_FUNCTION(0x1, "gpio_out"),
@@ -176,11 +177,13 @@ static const struct sunxi_desc_pin
sun4i_a10_pins[] = {
Quoted 13 lines
drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c +4 -2
                   SUNXI_FUNCTION(0x0, "gpio_in"),
                   SUNXI_FUNCTION(0x1, "gpio_out"),
                   SUNXI_FUNCTION(0x2, "i2s"),           /* DI */
-                 SUNXI_FUNCTION(0x3, "ac97")),         /* DI */
+                 SUNXI_FUNCTION(0x3, "ac97"),          /* DI */
+                 SUNXI_FUNCTION(0x4, "spdif")),        /* DI */
         SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 13),
                   SUNXI_FUNCTION(0x0, "gpio_in"),
                   SUNXI_FUNCTION(0x1, "gpio_out"),
-                 SUNXI_FUNCTION(0x2, "spi2")),         /* CS1 */
+                 SUNXI_FUNCTION(0x2, "spi2"),          /* CS1 */
+                 SUNXI_FUNCTION(0x4, "spdif")),        /* DO */
Quoted 7 lines
Quoted 4 lines
The datasheet and manual list them as NC or Reserved. Maybe mention how you knew
they were available?
Not sure if this deserves to be in the commit message but I can definitely
put together a covering letter with links etc.
Quoted 6 lines
I dear to say it belongs right there in the source code, use a multi-line comment above the SUNXI_FUNCTION(0x4, "spdif")) to explain where the mux info comes from. someday sooner or later someone is going to compare the kernel mux table to the datasheet and think "huh, that is not right", that person will be saved an immense amount of time by simply having a comment there.

Quoted 9 lines

Good point I was thinking something along the lines of this
/*
* The SPDIF block is not referenced at all in the A10 user
* manual. However it is described in the code leaked and the
* pin descriptions are declared in the A20 user manual which
* is pin compatible with this device.
*/
Thing is do I add this above every spdif pin, the first change(MCLK) or the pin most likely to be used(SPDIF DO)?

I'd say put it before the first pin, and then reference it (like "undocumented, see the comment for pin P??? above", for the rest.

ChenYu

Quoted 12 lines

Thanks,
CK
Quoted 8 lines
And do NOT say that this will not happen, because I've already done such a comparison once in the past.
Regards,
Hans

Hans de Goede

2015-08-12 10:18 UTC #6 lore raw

PATCH 1/1 pinctrl: sun4i: add spdif to pin description.

Hi,

Quoted 83 lines · Chen-Yu Tsai

Quoted 72 lines · Code Kipper
Quoted 64 lines · Hans de Goede
Hi,
Quoted 50 lines · Code Kipper
Quoted 47 lines · Chen-Yu Tsai
Review
Quoted 9 lines · On Wed, Aug 12, 2015 at 12:19 AM, <[email protected]> wrote:
From: Marcus Cooper <[email protected]>
Signed-off-by Marcus Cooper <[email protected]>
---
   drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c | 9 ++++++---
   1 file changed, 6 insertions(+), 3 deletions(-)
(quoted diff) +0 -0
b/drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c
drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c static const struct sunxi_desc_pin sun4i_a10_pins[]
Quoted 5 lines
index 7376a97..daf7dec 100644
drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c +0 -0
@@ -135,7 +135,8 @@ static const struct sunxi_desc_pin sun4i_a10_pins[]
= {
drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c static const struct sunxi_desc_pin
Quoted 11 lines
drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c +2 -1
          SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 3),
                    SUNXI_FUNCTION(0x0, "gpio_in"),
                    SUNXI_FUNCTION(0x1, "gpio_out"),
-                 SUNXI_FUNCTION(0x2, "ir0")),          /* TX */
+                 SUNXI_FUNCTION(0x2, "ir0"),           /* TX */
+                 SUNXI_FUNCTION(0x4, "spdif")),        /* MCLK */
          SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 4),
                    SUNXI_FUNCTION(0x0, "gpio_in"),
                    SUNXI_FUNCTION(0x1, "gpio_out"),
@@ -176,11 +177,13 @@ static const struct sunxi_desc_pin
sun4i_a10_pins[] = {
Quoted 13 lines
drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c +4 -2
                    SUNXI_FUNCTION(0x0, "gpio_in"),
                    SUNXI_FUNCTION(0x1, "gpio_out"),
                    SUNXI_FUNCTION(0x2, "i2s"),           /* DI */
-                 SUNXI_FUNCTION(0x3, "ac97")),         /* DI */
+                 SUNXI_FUNCTION(0x3, "ac97"),          /* DI */
+                 SUNXI_FUNCTION(0x4, "spdif")),        /* DI */
          SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 13),
                    SUNXI_FUNCTION(0x0, "gpio_in"),
                    SUNXI_FUNCTION(0x1, "gpio_out"),
-                 SUNXI_FUNCTION(0x2, "spi2")),         /* CS1 */
+                 SUNXI_FUNCTION(0x2, "spi2"),          /* CS1 */
+                 SUNXI_FUNCTION(0x4, "spdif")),        /* DO */
Quoted 7 lines
Quoted 4 lines
The datasheet and manual list them as NC or Reserved. Maybe mention how you knew
they were available?
Not sure if this deserves to be in the commit message but I can definitely
put together a covering letter with links etc.
Quoted 6 lines
I dear to say it belongs right there in the source code, use a multi-line comment above the SUNXI_FUNCTION(0x4, "spdif")) to explain where the mux info comes from. someday sooner or later someone is going to compare the kernel mux table to the datasheet and think "huh, that is not right", that person will be saved an immense amount of time by simply having a comment there.
Quoted 9 lines
Good point I was thinking something along the lines of this
/*
* The SPDIF block is not referenced at all in the A10 user
* manual. However it is described in the code leaked and the
* pin descriptions are declared in the A20 user manual which
* is pin compatible with this device.
*/
Thing is do I add this above every spdif pin, the first change(MCLK) or the pin most likely to be used(SPDIF DO)?

Quoted 2 lines

I'd say put it before the first pin, and then reference it (like "undocumented, see the comment for pin P??? above", for the rest.

Ack / +1 exactly what I was thinking :)

Regards,

Hans

Quoted 15 lines

ChenYu
Quoted 12 lines
Thanks,
CK
Quoted 8 lines
And do NOT say that this will not happen, because I've already done such a comparison once in the past.
Regards,
Hans