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

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

[Ajay Bhargav]

Hi Lei,
Thank you for confirming.

> What do you mean by "changed back to Armada100 gpio"?
> 

Well just incase GPIO register set is different for different archs
this current driver need to be renamed as armada100 gpio.

So if other Marvell SoCs are having similar register set then we can
have a common gpio header file like earlier version of this patch "mvgpio.h"

Regards,
Ajay Bhargav

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

[Prafulla Wadaskar]

> -----Original Message-----
> From: Ajay Bhargav [mailto:ajay.bhargav at einfochips.com]
> Sent: Friday, July 22, 2011 3:34 PM
> To: Lei Wen
> Cc: Prafulla Wadaskar; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH v2 2/2] gpio: Add GPIO driver for Marvell
> SoC Armada100
> 
> Hi Lei,
> Thank you for confirming.
> 
> > What do you mean by "changed back to Armada100 gpio"?
> >
> 
> Well just incase GPIO register set is different for different archs
> this current driver need to be renamed as armada100 gpio.
> 
> So if other Marvell SoCs are having similar register set then we can
> have a common gpio header file like earlier version of this patch
> "mvgpio.h"

The main objective of having mvgpio.c is to share/reuse a driver code across several SoCs.

If most of the SoCs (what we know) have similar register definition, it makes sense to keep them in mvgpio.c

What may differ for each SOC is that- base addresses, number of GPIOs, number of GPIO Banks, that should go in asm/arch/gpio.h

If any future SoC follows totally different GPIO architecture, we can always have <soc_name>_gpio.c.

Regards..
Prafulla . .

> 
> Regards,
> Ajay Bhargav

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

[Ajay Bhargav]

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

> The main objective of having mvgpio.c is to share/reuse a driver code
> across several SoCs.
> 
> If most of the SoCs (what we know) have similar register definition,
> it makes sense to keep them in mvgpio.c
> 
> What may differ for each SOC is that- base addresses, number of GPIOs,
> number of GPIO Banks, that should go in asm/arch/gpio.h
> 
> If any future SoC follows totally different GPIO architecture, we can
> always have _gpio.c.
> 
> Regards..
> Prafulla . .
> 

So what about v2 patches i sent? I did things accordingly you said.
can you please review them and let me know if any changes are required.

Regards,
Ajay Bhargav

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

[Ajay Bhargav]

Hi Prafulla,

Any comments on the GPIO patches? Is there any modifications you suggest?

Regards,
Ajay Bhargav

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

[Lei Wen]

Hi Ajay,

On Fri, Jul 22, 2011 at 4:57 PM, Ajay Bhargav
<ajay.bhargav@einfochips.com> wrote:
> Hi Lei,
>
>> I think if have no such same structure, the gpio driver may need to be
>> rewrite,
>> since the meaning of the register may also change.
>>
> so should this be changed back to Armada100 gpio? How bout mmp3, is it having
> same register set?
>
For mmp3, its register arragement is exact same as Armada100(aka pxa168).
What do you mean by "changed back to Armada100 gpio"?

Best regards,
Lei

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

[Ajay Bhargav]

Hi Lei,

> I think if have no such same structure, the gpio driver may need to be
> rewrite,
> since the meaning of the register may also change.
> 
so should this be changed back to Armada100 gpio? How bout mmp3, is it having
same register set?

Regards,
Ajay Bhargav

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

[Lei Wen]

On Fri, Jul 22, 2011 at 3:27 PM, Ajay Bhargav
<ajay.bhargav@einfochips.com> wrote:
> Hi Lei,
>
>> Why not put this structure into driver file?
>> I think this register is not easy to change, and put this into arch
>> .h
>> would make the work duplicated,
>> as each arch need to redefine the same structure again.
>>
> Other architectures may/may not have the same structure. One can redefine
> structure based on this header file.

I think if have no such same structure, the gpio driver may need to be rewrite,
since the meaning of the register may also change.

Best regards,
Lei

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

[Lei Wen]

Hi Ajay,

On Fri, Jul 22, 2011 at 3:16 PM, Ajay Bhargav
<ajay.bhargav@einfochips.com> wrote:
> 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 ? ? ?| ? 95 +++++++++++++++++++++++
> ?2 files changed, 99 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..0593b13
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-armada100/gpio.h
> @@ -0,0 +1,95 @@
> +/*
> + * (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>
> +
> +#define GPIO_TO_REG(gp) ? ? ? ? ? ? ? ?(gp >> 5)
> +#define GPIO_TO_BIT(gp) ? ? ? ? ? ? ? ?(1 << (gp & 0x1F))
> +#define GPIO_VAL(gp, val) ? ? ?((val >> (gp & 0x1F)) & 0x01)
> +
> +#define GPIO_SET ? ? ? ? ? ? ? 1
> +#define GPIO_CLR ? ? ? ? ? ? ? 0
> +
> +/*
> + * GPIO register map
> + * Refer Datasheet Appendix A.36
> + */
> +struct gpio_reg {

Why not put this structure into driver file?
I think this register is not easy to change, and put this into arch .h
would make the work duplicated,
as each arch need to redefine the same structure again.


> +static inline struct gpio_reg *get_gpio_base(int bank)

After put the structure into the driver file, you could simply return
void* instead of gpio_reg*.

Best regards,
Lei

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

[Ajay Bhargav]

Hi Lei,

> Why not put this structure into driver file?
> I think this register is not easy to change, and put this into arch
> .h
> would make the work duplicated,
> as each arch need to redefine the same structure again.
> 
Other architectures may/may not have the same structure. One can redefine
structure based on this header file.

Thanks & Regards,
Ajay Bhargav

[U-Boot] [PATCH v2 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      |   95 +++++++++++++++++++++++
 2 files changed, 99 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..0593b13
--- /dev/null
+++ b/arch/arm/include/asm/arch-armada100/gpio.h
@@ -0,0 +1,95 @@
+/*
+ * (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>
+
+#define GPIO_TO_REG(gp)		(gp >> 5)
+#define GPIO_TO_BIT(gp)		(1 << (gp & 0x1F))
+#define GPIO_VAL(gp, val)	((val >> (gp & 0x1F)) & 0x01)
+
+#define GPIO_SET		1
+#define GPIO_CLR		0
+
+/*
+ * GPIO register map
+ * Refer Datasheet Appendix A.36
+ */
+struct gpio_reg {
+	u32 gplr;	/* Pin Level Register - 0x0000 */
+	u32 pad0[2];
+	u32 gpdr;	/* Pin Direction Register - 0x000C */
+	u32 pad1[2];
+	u32 gpsr;	/* Pin Output Set Register - 0x0018 */
+	u32 pad2[2];
+	u32 gpcr;	/* Pin Output Clear Register - 0x0024 */
+	u32 pad3[2];
+	u32 grer;	/* Rising-Edge Detect Enable Register - 0x0030 */
+	u32 pad4[2];
+	u32 gfer;	/* Falling-Edge Detect Enable Register - 0x003C */
+	u32 pad5[2];
+	u32 gedr;	/* Edge Detect Status Register - 0x0048 */
+	u32 pad6[2];
+	u32 gsdr;	/* Bitwise Set of GPIO Direction Register - 0x0054 */
+	u32 pad7[2];
+	u32 gcdr;	/* Bitwise Clear of GPIO Direction Register - 0x0060 */
+	u32 pad8[2];
+	u32 gsrer;	/* Bitwise Set of Rising-Edge Detect Enable
+			   Register - 0x006C */
+	u32 pad9[2];
+	u32 gcrer;	/* Bitwise Clear of Rising-Edge Detect Enable
+			   Register - 0x0078 */
+	u32 pad10[2];
+	u32 gsfer;	/* Bitwise Set of Falling-Edge Detect Enable
+			   Register - 0x0084 */
+	u32 pad11[2];
+	u32 gcfer;	/* Bitwise Clear of Falling-Edge Detect Enable
+			   Register - 0x0090 */
+	u32 pad12[2];
+	u32 apmask;	/* Bitwise Mask of Edge Detect Register - 0x009C */
+};
+
+
+static inline struct gpio_reg *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