mpc85xx gpio related header files changes to compile common cmd_gpio.c

PATCH New Oleksandr G Zhadan 3 participants 8 messages Mar 26, 2015 – Mar 27, 2015 PowerPC

lore

Share & tools

lore

Oleksandr G Zhadan

author

2015-03-26 21:16 UTC #0 lore raw

PATCH 1/2 mpc85xx gpio related header files changes to compile common cmd_gpio.c

From: Oleksandr Zhadan <[email protected]>

Signed-off-by Oleksandr Zhadan <[email protected]>

Signed-off-by Michael Durrant <[email protected]>

---

  arch/powerpc/include/asm/arch-mpc85xx/gpio.h |    2 ++
  arch/powerpc/include/asm/mpc85xx_gpio.h      |    6 ++++--
  2 files changed, 6 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/arch/powerpc/include/asm/arch-mpc85xx/gpio.h
index 8beed30..71794a8 100644

arch/powerpc/include/asm/arch-mpc85xx/gpio.h +2 -0

@@ -12,4 +12,6 @@
  #ifndef __ASM_ARCH_MX85XX_GPIO_H
  #define __ASM_ARCH_MX85XX_GPIO_H
 
+#include <asm/mpc85xx_gpio.h>
+
  #endif

(quoted diff) +0 -0

b/arch/powerpc/include/asm/mpc85xx_gpio.h
index 87bb4a0..1d0dad4 100644

arch/powerpc/include/asm/mpc85xx_gpio.h +0 -0

@@ -72,9 +72,10 @@ static inline int gpio_request(unsigned gpio, const

char *label)

      return 0;

}

-static inline void gpio_free(unsigned gpio)
+static inline int gpio_free(unsigned gpio)

  {
      /* Compatibility shim */

+ return 0;
}

static inline int gpio_direction_input(unsigned gpio)
@@ -97,12 +98,13 @@ static inline int gpio_get_value(unsigned gpio)

      return !!mpc85xx_gpio_get(1U << gpio);

}

-static inline void gpio_set_value(unsigned gpio, int value) +static inline int gpio_set_value(unsigned gpio, int value)

  {
      if (value)
          mpc85xx_gpio_set_high(1U << gpio);
      else
          mpc85xx_gpio_set_low(1U << gpio);

+ return 0;
}

static inline int gpio_is_valid(int gpio)

York Sun

2015-03-26 21:20 UTC #1 lore raw

PATCH 1/2 mpc85xx gpio related header files changes to compile common cmd_gpio.c

Quoted 1 line · Oleksandr G Zhadan

From: Oleksandr Zhadan <[email protected]>

Please add commit message to explain why and what you are changing.

Quoted 4 lines

Signed-off-by Oleksandr Zhadan <[email protected]>
Signed-off-by Michael Durrant <[email protected]>
---

If you resend the same patch, please add a comment under the --- line. If you make a new version, please add change log.

York

Joakim Tjernlund

2015-03-27 00:26 UTC #4 lore raw

PATCH 1/2 mpc85xx gpio related header files changes to compile common cmd_gpio.c

In reply to Oleksandr G Zhadan

Quoted 43 lines · Oleksandr G Zhadan

From: Oleksandr Zhadan <[email protected]>

Signed-off-by Oleksandr Zhadan <[email protected]>

Signed-off-by Michael Durrant <[email protected]>

---

  arch/powerpc/include/asm/arch-mpc85xx/gpio.h |    2 ++
  arch/powerpc/include/asm/mpc85xx_gpio.h      |    6 ++++--
  2 files changed, 6 insertions(+), 2 deletions(-)

arch/powerpc/include/asm/arch-mpc85xx/gpio.h +2 -0

index 8beed30..71794a8 100644
@@ -12,4 +12,6 @@
  #ifndef __ASM_ARCH_MX85XX_GPIO_H
  #define __ASM_ARCH_MX85XX_GPIO_H
 
+#include <asm/mpc85xx_gpio.h>
+
  #endif

arch/powerpc/include/asm/mpc85xx_gpio.h +2 -1

index 87bb4a0..1d0dad4 100644
@@ -72,9 +72,10 @@ static inline int gpio_request(unsigned gpio, const 
 char *label)
      return 0;
  }
 
-static inline void gpio_free(unsigned gpio)
+static inline int gpio_free(unsigned gpio)
  {
      /* Compatibility shim */
+    return 0;
  }

static inline int gpio_direction_input(unsigned gpio)

arch/powerpc/include/asm/mpc85xx_gpio.h +0 -0

@@ -97,12 +98,13 @@ static inline int gpio_get_value(unsigned gpio)
      return !!mpc85xx_gpio_get(1U << gpio);
  }

arch/powerpc/include/asm/mpc85xx_gpio.h static inline int gpio_get_value(unsigned gpio) · line 108

Quoted 10 lines

arch/powerpc/include/asm/mpc85xx_gpio.h +2 -1

 
-static inline void gpio_set_value(unsigned gpio, int value)
+static inline int gpio_set_value(unsigned gpio, int value)
  {
      if (value)
          mpc85xx_gpio_set_high(1U << gpio);
      else
          mpc85xx_gpio_set_low(1U << gpio);
+    return 0;
  }

mpc85xx_gpio_xxx really needs an extra bank argument such as:
static inline void my_mpc85xx_gpio_set(unsigned int bank, unsigned int mask,
unsigned int dir, unsigned int val)

{
        ccsr_gpio_t *gpio = (void *)(CONFIG_SYS_MPC85xx_GPIO_ADDR +
                                     bank * 0x1000);

....

so one can choose which GPIO bank to use

Jocke

Oleksandr G Zhadan

author

2015-03-27 14:51 UTC #5 lore raw

PATCH 1/2 mpc85xx gpio related header files changes to compile common cmd_gpio.c

Extra gpio bank argument at low level may be discussable, but IMPO we should not brake standard int gpio_set_value(unsigned gpio, int value) definition.

And the reason of this patch is to fix:

1. Incompatibility in functions gpio_free() and gpio_set_value() definitions between <asm/gpio.h> and <asm-generic/gpio.h> (void vs int)

2. Compilation error when build common/cmd_gpio.c for mpc85xx (p1020)

Quoted 68 lines · Joakim Tjernlund

Quoted 43 lines · Oleksandr G Zhadan
From: Oleksandr Zhadan <[email protected]>
Signed-off-by Oleksandr Zhadan <[email protected]>
Signed-off-by Michael Durrant <[email protected]>
---
   arch/powerpc/include/asm/arch-mpc85xx/gpio.h |    2 ++
   arch/powerpc/include/asm/mpc85xx_gpio.h      |    6 ++++--
   2 files changed, 6 insertions(+), 2 deletions(-)
arch/powerpc/include/asm/arch-mpc85xx/gpio.h +2 -0
index 8beed30..71794a8 100644
@@ -12,4 +12,6 @@
   #ifndef __ASM_ARCH_MX85XX_GPIO_H
   #define __ASM_ARCH_MX85XX_GPIO_H
 
+#include <asm/mpc85xx_gpio.h>
+
   #endif
arch/powerpc/include/asm/mpc85xx_gpio.h +2 -1
index 87bb4a0..1d0dad4 100644
@@ -72,9 +72,10 @@ static inline int gpio_request(unsigned gpio, const
 char *label)
       return 0;
   }
 
-static inline void gpio_free(unsigned gpio)
+static inline int gpio_free(unsigned gpio)
   {
       /* Compatibility shim */
+    return 0;
   }
 
static inline int gpio_direction_input(unsigned gpio)
arch/powerpc/include/asm/mpc85xx_gpio.h +0 -0
@@ -97,12 +98,13 @@ static inline int gpio_get_value(unsigned gpio)
       return !!mpc85xx_gpio_get(1U << gpio);
   }
arch/powerpc/include/asm/mpc85xx_gpio.h static inline int gpio_get_value(unsigned gpio) · line 108
Quoted 10 lines
arch/powerpc/include/asm/mpc85xx_gpio.h +2 -1
 
-static inline void gpio_set_value(unsigned gpio, int value)
+static inline int gpio_set_value(unsigned gpio, int value)
   {
       if (value)
           mpc85xx_gpio_set_high(1U << gpio);
       else
           mpc85xx_gpio_set_low(1U << gpio);
+    return 0;
   }
mpc85xx_gpio_xxx really needs an extra bank argument such as:
static inline void my_mpc85xx_gpio_set(unsigned int bank, unsigned int mask,
unsigned int dir, unsigned int val)
{
         ccsr_gpio_t *gpio = (void *)(CONFIG_SYS_MPC85xx_GPIO_ADDR +
                                      bank * 0x1000);
....
so one can choose which GPIO bank to use
Jocke

Signature · 4 lines

Oleksandr Zhadan
Arcturus Networks
T 416 621-0125 x235
F 416 621-0190

Joakim Tjernlund

2015-03-27 15:46 UTC #6 lore raw

PATCH 1/2 mpc85xx gpio related header files changes to compile common cmd_gpio.c

Quoted 10 lines · Oleksandr G Zhadan

Extra gpio bank argument at low level may be discussable, but IMPO we should not brake standard int gpio_set_value(unsigned gpio, int value) definition.
And the reason of this patch is to fix:
1. Incompatibility in functions gpio_free() and gpio_set_value() definitions between <asm/gpio.h> and <asm-generic/gpio.h> (void vs int)
2. Compilation error when build common/cmd_gpio.c for mpc85xx (p1020)

Sure, when I saw "mpc85xx gpio .." I remembered seeing this flaw so I figured I point it out before I forget again.

This also makes me remember, drivers/spi/fsl_espi.c is broken for anything but reading an EEPROM.

Jocke

Oleksandr G Zhadan

author

2015-03-27 16:09 UTC #7 lore raw

PATCH 1/2 mpc85xx gpio related header files changes to compile common cmd_gpio.c

Yes, here locally we have some additional changes/fixes in some places as well. And if talk about mpc85xx gpio support it's better to have driver in drivers/gpio vs static definitions in header file and I have it. But currently I need this minimal patch to be accepted to send our board support patch based on this one.

Oleks

Quoted 20 lines · Joakim Tjernlund

Quoted 10 lines · Oleksandr G Zhadan
Extra gpio bank argument at low level may be discussable, but IMPO we should not brake standard int gpio_set_value(unsigned gpio, int value) definition.
And the reason of this patch is to fix:
1. Incompatibility in functions gpio_free() and gpio_set_value() definitions between <asm/gpio.h> and <asm-generic/gpio.h> (void vs int)
2. Compilation error when build common/cmd_gpio.c for mpc85xx (p1020)
Sure, when I saw "mpc85xx gpio .." I remembered seeing this flaw so I figured I point it out before I forget again.
This also makes me remember, drivers/spi/fsl_espi.c is broken for anything but reading an EEPROM.
Jocke

Oleksandr G Zhadan

author

2015-03-27 18:27 UTC #2 lore raw

PATCH 1/2 mpc85xx gpio related header files changes to compile common cmd_gpio.c

In reply to York Sun

Hi York,

I add explanation and re-send the same patch.

Quoted 8 lines · York Sun

Quoted 1 line · Oleksandr G Zhadan
From: Oleksandr Zhadan <[email protected]>
Please add commit message to explain why and what you are changing.
Quoted 1 line

Quoted 10 lines

Quoted 3 lines
Signed-off-by Oleksandr Zhadan <[email protected]>
Signed-off-by Michael Durrant <[email protected]>
---
If you resend the same patch, please add a comment under the --- line. If you make a new version, please add change log.
York

From: Oleksandr Zhadan <[email protected]>

1. Fix Incompatibility in functions gpio_free() and gpio_set_value() definitions between <asm/gpio.h> and <asm-generic/gpio.h> (void vs int)

2. Fix Compilation error when build common/cmd_gpio.c for mpc85xx (p1020)

Signed-off-by Oleksandr Zhadan <[email protected]>

Signed-off-by Michael Durrant <[email protected]>

---

  arch/powerpc/include/asm/arch-mpc85xx/gpio.h |    2 ++
  arch/powerpc/include/asm/mpc85xx_gpio.h      |    6 ++++--
  2 files changed, 6 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/arch/powerpc/include/asm/arch-mpc85xx/gpio.h
index 8beed30..71794a8 100644

arch/powerpc/include/asm/arch-mpc85xx/gpio.h +2 -0

@@ -12,4 +12,6 @@
  #ifndef __ASM_ARCH_MX85XX_GPIO_H
  #define __ASM_ARCH_MX85XX_GPIO_H
 
+#include <asm/mpc85xx_gpio.h>
+
  #endif

(quoted diff) +0 -0

b/arch/powerpc/include/asm/mpc85xx_gpio.h
index 87bb4a0..1d0dad4 100644

arch/powerpc/include/asm/mpc85xx_gpio.h +0 -0

@@ -72,9 +72,10 @@ static inline int gpio_request(unsigned gpio, const

char *label)

      return 0;

}

-static inline void gpio_free(unsigned gpio)
+static inline int gpio_free(unsigned gpio)

  {
      /* Compatibility shim */

+ return 0;
}

static inline int gpio_direction_input(unsigned gpio)
@@ -97,12 +98,13 @@ static inline int gpio_get_value(unsigned gpio)

      return !!mpc85xx_gpio_get(1U << gpio);

}

-static inline void gpio_set_value(unsigned gpio, int value) +static inline int gpio_set_value(unsigned gpio, int value)

  {
      if (value)
          mpc85xx_gpio_set_high(1U << gpio);
      else
          mpc85xx_gpio_set_low(1U << gpio);

+ return 0;
}

static inline int gpio_is_valid(int gpio)

York Sun

2015-03-27 18:44 UTC #3 lore raw

PATCH 1/2 mpc85xx gpio related header files changes to compile common cmd_gpio.c

Quoted 3 lines · Oleksandr G Zhadan

Hi York,
I add explanation and re-send the same patch.

Whenever you make changes to the patch, please add a change log under the --- line. Please also see my comments on previous patch sent 5 minutes before this.

York