[U-Boot] [PATCH v3 2/2] gpio: Add GPIO driver for Marvell SoC Armada100

[U-Boot] [PATCH v3 2/2] gpio: Add GPIO driver for Marvell SoC Armada100

[Ajay Bhargav]

This patch adds support for generic GPIO driver framework for Marvell
SoC Armada100.

Signed-off-by: Ajay Bhargav <ajay.bhargav@einfochips.com>
---
 arch/arm/include/asm/arch-armada100/armada100.h |    4 ++
 arch/arm/include/asm/arch-armada100/gpio.h      |   54 +++++++++++++++++++++++
 2 files changed, 58 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-armada100/gpio.h

diff --git a/arch/arm/include/asm/arch-armada100/armada100.h b/arch/arm/include/asm/arch-armada100/armada100.h
index d5d125a..aad3ed1 100644
--- a/arch/arm/include/asm/arch-armada100/armada100.h
+++ b/arch/arm/include/asm/arch-armada100/armada100.h
@@ -59,6 +59,10 @@
 #define ARMD1_MPMU_BASE		0xD4050000
 #define ARMD1_APMU_BASE		0xD4282800
 #define ARMD1_CPU_BASE		0xD4282C00
+#define ARMD1_GPIO0_BASE	0xD4019000
+#define ARMD1_GPIO1_BASE	0xD4019004
+#define ARMD1_GPIO2_BASE	0xD4019008
+#define ARMD1_GPIO3_BASE	0xD4019100
 
 /*
  * Main Power Management (MPMU) Registers
diff --git a/arch/arm/include/asm/arch-armada100/gpio.h b/arch/arm/include/asm/arch-armada100/gpio.h
new file mode 100644
index 0000000..bd7d21a
--- /dev/null
+++ b/arch/arm/include/asm/arch-armada100/gpio.h
@@ -0,0 +1,54 @@
+/*
+ * (C) Copyright 2011
+ * eInfochips Ltd. <www.einfochips.com>
+ * Written-by: Ajay Bhargav <ajay.bhargav@einfochips.com>
+ *
+ * (C) Copyright 2010
+ * Marvell Semiconductor <www.marvell.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ */
+
+#ifndef _ASM_ARCH_GPIO_H
+#define _ASM_ARCH_GPIO_H
+
+#include <asm/types.h>
+#include <asm/arch/armada100.h>
+#include <mvgpio.h>
+
+#define GPIO_TO_REG(gp)		(gp >> 5)
+#define GPIO_TO_BIT(gp)		(1 << (gp & 0x1F))
+#define GPIO_VAL(gp, val)	((val >> (gp & 0x1F)) & 0x01)
+
+static inline void *get_gpio_base(int bank)
+{
+	switch (bank) {
+	case 0:
+		return (struct gpio_reg *)ARMD1_GPIO0_BASE;
+	case 1:
+		return (struct gpio_reg *)ARMD1_GPIO1_BASE;
+	case 2:
+		return (struct gpio_reg *)ARMD1_GPIO2_BASE;
+	case 3:
+		return (struct gpio_reg *)ARMD1_GPIO3_BASE;
+	}
+	return 0;
+}
+
+#endif /* _ASM_ARCH_GPIO_H */
-- 
1.7.0.4

[U-Boot] [PATCH v3 2/2] gpio: Add GPIO driver for Marvell SoC Armada100

[Ajay Bhargav]

----- "Prafulla Wadaskar" <prafulla@marvell.com> wrote:

> 
> I suggest below code for this function.
> { 
>   Const unsigned int offset[4] = {0, 4, 8, 0x100}; /* gpio register
> bank offsets */
>   return (struct gpio_reg *)(ARMD1_GPIO_BASE + offset[bank]);
> }
> 
> Again content in this file are SoC core specific and will duplicate
> for other SoC supports like pantheon.
> 
> Can you please move them to mvgpio.h within #ifdef
> CONFIG_SHEEVA_88SV331xV5?
> I think this should be the final modification for this driver
> support.
> 
> Sorry for the rework.
> 
> Regards..
> Prafulla . .
> 

Hi Prafulla,

Can you please tell me what part of code should be moved to mvgpio.h?
I have no idea about number of banks in other SOCs with same core.

I will do the changes as per suggestion.

Thanks & Regards,
Ajay Bhargav

[U-Boot] [PATCH v3 2/2] gpio: Add GPIO driver for Marvell SoC Armada100

[Prafulla Wadaskar]

> -----Original Message-----
> From: Ajay Bhargav [mailto:ajay.bhargav at einfochips.com]
> Sent: Monday, August 08, 2011 11:40 AM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ajay Bhargav
> Subject: [PATCH v3 2/2] gpio: Add GPIO driver for Marvell SoC Armada100
> 
> This patch adds support for generic GPIO driver framework for Marvell
> SoC Armada100.
> 
> Signed-off-by: Ajay Bhargav <ajay.bhargav@einfochips.com>
> ---
>  arch/arm/include/asm/arch-armada100/armada100.h |    4 ++
>  arch/arm/include/asm/arch-armada100/gpio.h      |   54
> +++++++++++++++++++++++
>  2 files changed, 58 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/include/asm/arch-armada100/gpio.h
> 
> diff --git a/arch/arm/include/asm/arch-armada100/armada100.h
> b/arch/arm/include/asm/arch-armada100/armada100.h
> index d5d125a..aad3ed1 100644
> --- a/arch/arm/include/asm/arch-armada100/armada100.h
> +++ b/arch/arm/include/asm/arch-armada100/armada100.h
> @@ -59,6 +59,10 @@
>  #define ARMD1_MPMU_BASE		0xD4050000
>  #define ARMD1_APMU_BASE		0xD4282800
>  #define ARMD1_CPU_BASE		0xD4282C00
> +#define ARMD1_GPIO0_BASE	0xD4019000

ARMD1_GPIO_BASE is already there in this file.
Having just one definition of GPIO base address here sounds good.
So we don't need to change this file. (see comments below)

> +#define ARMD1_GPIO1_BASE	0xD4019004
> +#define ARMD1_GPIO2_BASE	0xD4019008
> +#define ARMD1_GPIO3_BASE	0xD4019100
> 
>  /*
>   * Main Power Management (MPMU) Registers
> diff --git a/arch/arm/include/asm/arch-armada100/gpio.h
> b/arch/arm/include/asm/arch-armada100/gpio.h
> new file mode 100644
> index 0000000..bd7d21a
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-armada100/gpio.h
> @@ -0,0 +1,54 @@
> +/*
> + * (C) Copyright 2011
> + * eInfochips Ltd. <www.einfochips.com>
> + * Written-by: Ajay Bhargav <ajay.bhargav@einfochips.com>
> + *
> + * (C) Copyright 2010
> + * Marvell Semiconductor <www.marvell.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + */
> +
> +#ifndef _ASM_ARCH_GPIO_H
> +#define _ASM_ARCH_GPIO_H
> +
> +#include <asm/types.h>
> +#include <asm/arch/armada100.h>
> +#include <mvgpio.h>
> +
> +#define GPIO_TO_REG(gp)		(gp >> 5)
> +#define GPIO_TO_BIT(gp)		(1 << (gp & 0x1F))
> +#define GPIO_VAL(gp, val)	((val >> (gp & 0x1F)) & 0x01)
> +
> +static inline void *get_gpio_base(int bank)
> +{
> +	switch (bank) {
> +	case 0:
> +		return (struct gpio_reg *)ARMD1_GPIO0_BASE;
> +	case 1:
> +		return (struct gpio_reg *)ARMD1_GPIO1_BASE;
> +	case 2:
> +		return (struct gpio_reg *)ARMD1_GPIO2_BASE;
> +	case 3:
> +		return (struct gpio_reg *)ARMD1_GPIO3_BASE;
> +	}
> +	return 0;
> +}


I suggest below code for this function.
{ 
  Const unsigned int offset[4] = {0, 4, 8, 0x100}; /* gpio register bank offsets */
  return (struct gpio_reg *)(ARMD1_GPIO_BASE + offset[bank]);
}

Again content in this file are SoC core specific and will duplicate for other SoC supports like pantheon.

Can you please move them to mvgpio.h within #ifdef CONFIG_SHEEVA_88SV331xV5?
I think this should be the final modification for this driver support.

Sorry for the rework.

Regards..
Prafulla . .

[U-Boot] [PATCH v3 2/2] gpio: Add GPIO driver for Marvell SoC Armada100

[Prafulla Wadaskar]

> -----Original Message-----
> From: Ajay Bhargav [mailto:ajay.bhargav at einfochips.com]
> Sent: Wednesday, August 10, 2011 1:37 PM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik
> Subject: Re: [PATCH v3 2/2] gpio: Add GPIO driver for Marvell SoC
> Armada100
> 
> 
> ----- "Prafulla Wadaskar" <prafulla@marvell.com> wrote:
> 
> >
> > I suggest below code for this function.
> > {
> >   Const unsigned int offset[4] = {0, 4, 8, 0x100}; /* gpio register
> > bank offsets */
> >   return (struct gpio_reg *)(ARMD1_GPIO_BASE + offset[bank]);
> > }
> >
> > Again content in this file are SoC core specific and will duplicate
> > for other SoC supports like pantheon.
> >
> > Can you please move them to mvgpio.h within #ifdef
> > CONFIG_SHEEVA_88SV331xV5?
> > I think this should be the final modification for this driver
> > support.
> >
> > Sorry for the rework.
> >
> > Regards..
> > Prafulla . .
> >
> 
> Hi Prafulla,
> 
> Can you please tell me what part of code should be moved to mvgpio.h?

You should move entire contents of gpio.h in mvgpio.h within #ifdef CONFIG_SHEEVA_88SV331xV5, so just mvgpio.c,mvgpio.h,Makefile will add armada100 gpio driver support in more generic way.

> I have no idea about number of banks in other SOCs with same core.

No need to worry, at this moment this driver will be supporting 88SV331xv5 core only.

Regards..
Prafulla . .

[U-Boot] [PATCH v3 2/2] gpio: Add GPIO driver for Marvell SoC Armada100

[Ajay Bhargav]

----- "Prafulla Wadaskar" <prafulla@marvell.com> wrote:

> You should move entire contents of gpio.h in mvgpio.h within #ifdef
> CONFIG_SHEEVA_88SV331xV5, so just mvgpio.c,mvgpio.h,Makefile will add
> armada100 gpio driver support in more generic way.
> 
> > I have no idea about number of banks in other SOCs with same core.
> 
> No need to worry, at this moment this driver will be supporting
> 88SV331xv5 core only.
> 
> Regards..
> Prafulla . .
> 

I think its better to just keep Armada100 related stuff in gpio.h and
I will do the following suggested changes.

> > {
> >   Const unsigned int offset[4] = {0, 4, 8, 0x100}; /* gpio register
> > bank offsets */
> >   return (struct gpio_reg *)(ARMD1_GPIO_BASE + offset[bank]);
> > }

Regards,
Ajay Bhargav

[U-Boot] [PATCH v3 2/2] gpio: Add GPIO driver for Marvell SoC Armada100

[Prafulla Wadaskar]

> -----Original Message-----
> From: Ajay Bhargav [mailto:ajay.bhargav at einfochips.com]
> Sent: Wednesday, August 10, 2011 2:03 PM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik
> Subject: Re: [PATCH v3 2/2] gpio: Add GPIO driver for Marvell SoC
> Armada100
> 
> 
> ----- "Prafulla Wadaskar" <prafulla@marvell.com> wrote:
> 
> > You should move entire contents of gpio.h in mvgpio.h within #ifdef
> > CONFIG_SHEEVA_88SV331xV5, so just mvgpio.c,mvgpio.h,Makefile will add
> > armada100 gpio driver support in more generic way.
> >
> > > I have no idea about number of banks in other SOCs with same core.
> >
> > No need to worry, at this moment this driver will be supporting
> > 88SV331xv5 core only.
> >
> > Regards..
> > Prafulla . .
> >
> 
> I think its better to just keep Armada100 related stuff in gpio.h and
> I will do the following suggested changes.

Yes you are right, removing gpio.h will lead to compilation error.
So in your earlier patch do not modify armada100.h and use suggested function body and repost.

> > > {
> > >   Const unsigned int offset[4] = {0, 4, 8, 0x100}; /* gpio register
> > > bank offsets */
> > >   return (struct gpio_reg *)(ARMD1_GPIO_BASE + offset[bank]);
> > > }

Regards..
Prafulla . .

[U-Boot] [PATCH v3 2/2] gpio: Add GPIO driver for Marvell SoC Armada100

[Ajay Bhargav]

----- "Prafulla Wadaskar" <prafulla@marvell.com> wrote:

> > -----Original Message-----
> > From: Ajay Bhargav [mailto:ajay.bhargav at einfochips.com]
> > Sent: Wednesday, August 10, 2011 1:37 PM
> > To: Prafulla Wadaskar
> > Cc: u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik
> > Subject: Re: [PATCH v3 2/2] gpio: Add GPIO driver for Marvell SoC
> > Armada100
> > 
> > 
> > ----- "Prafulla Wadaskar" <prafulla@marvell.com> wrote:
> > 
> > >
> > > I suggest below code for this function.
> > > {
> > >   Const unsigned int offset[4] = {0, 4, 8, 0x100}; /* gpio
> register
> > > bank offsets */
> > >   return (struct gpio_reg *)(ARMD1_GPIO_BASE + offset[bank]);
> > > }
> > >
> > > Again content in this file are SoC core specific and will
> duplicate
> > > for other SoC supports like pantheon.
> > >
> > > Can you please move them to mvgpio.h within #ifdef
> > > CONFIG_SHEEVA_88SV331xV5?
> > > I think this should be the final modification for this driver
> > > support.
> > >
> > > Sorry for the rework.
> > >
> > > Regards..
> > > Prafulla . .
> > >
> > 
> > Hi Prafulla,
> > 
> > Can you please tell me what part of code should be moved to
> mvgpio.h?
> 
> You should move entire contents of gpio.h in mvgpio.h within #ifdef
> CONFIG_SHEEVA_88SV331xV5, so just mvgpio.c,mvgpio.h,Makefile will add
> armada100 gpio driver support in more generic way.
> 
> > I have no idea about number of banks in other SOCs with same core.
> 
> No need to worry, at this moment this driver will be supporting
> 88SV331xv5 core only.
> 
> Regards..
> Prafulla . .
> 
I feel it is important to keep gpio.h in arch folder, 'coz if someone enables
GPIO command support, generic library header (asm/gpio.h) will look for
asm/arch/gpio.h file. we can simply include mvgpio.h in gpio.h incase of armada100.

Regards,
Ajay Bhargav