[U-Boot] [PATCH 1/6] usb: dwc2: Add support for v3 snpsid value

[U-Boot] [PATCH 1/6] usb: dwc2: Add support for v3 snpsid value

[Marek Vasut]

On Tuesday, May 12, 2015 at 03:38:27 PM, Peter Griffin wrote:
> This has been tested to the extent that I can enumerate
> a asix usb networking adapter and boot a kernel over usb
> on the 96boards hikey u-boot port I'm currently doing.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  drivers/usb/host/dwc2.c | 3 ++-
>  drivers/usb/host/dwc2.h | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
> index e8142ac..19a6007 100644
> --- a/drivers/usb/host/dwc2.c
> +++ b/drivers/usb/host/dwc2.c
> @@ -1015,7 +1015,8 @@ int usb_lowlevel_init(int index, enum usb_init_type
> init, void **controller) snpsid = readl(&regs->gsnpsid);
>  	printf("Core Release: %x.%03x\n", snpsid >> 12 & 0xf, snpsid & 0xfff);
> 
> -	if ((snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_2xx) {
> +	if ((snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_2xx &&
> +	    (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_3xx) {

Isn't this then a DWC3 controller instead ?

Best regards,
Marek Vasut

[U-Boot] [PATCH 2/6] dm: gpio: hi6220: Add a hi6220 GPIO driver model driver.

[Marek Vasut]

On Tuesday, May 12, 2015 at 03:38:28 PM, Peter Griffin wrote:

Hi!

> This patch adds support for the GPIO perif found on hi6220
> SoC.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  arch/arm/include/asm/arch-armv8/gpio.h | 47 +++++++++++++++++
>  drivers/gpio/Makefile                  |  2 +
>  drivers/gpio/hi6220_gpio.c             | 95
> ++++++++++++++++++++++++++++++++++ 3 files changed, 144 insertions(+)
>  create mode 100644 arch/arm/include/asm/arch-armv8/gpio.h
>  create mode 100644 drivers/gpio/hi6220_gpio.c
> 
> diff --git a/arch/arm/include/asm/arch-armv8/gpio.h
> b/arch/arm/include/asm/arch-armv8/gpio.h new file mode 100644
> index 0000000..162c2d9
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-armv8/gpio.h
> @@ -0,0 +1,47 @@
> +/*
> + * Copyright (C) 2015 Linaro
> + * Peter Griffin <peter.griffin@linaro.org>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef _HI6220_GPIO_H_
> +#define _HI6220_GPIO_H_
> +
> +#define HI6220_GPIO0_BASE	(void *)0xf8011000

You should drop the explicit cast, that's nasty.

Also, why don't you define this as a HI6220_GPIO_BASE(bank) instead?
That'd trim down the number of macros and from what I see, it should
be rather easy to do ...

#define HI6220_GPIO_BASE(bank)
	(((bank < 4) ? 0xf8012000 : (0xf7020000 - 0x4000)) + (0x1000 * bank))

> +#define HI6220_GPIO1_BASE	(void *)0xf8012000
> +#define HI6220_GPIO2_BASE	(void *)0xf8013000
> +#define HI6220_GPIO3_BASE	(void *)0xf8014000
> +#define HI6220_GPIO4_BASE	(void *)0xf7020000
> +#define HI6220_GPIO5_BASE	(void *)0xf7021000
> +#define HI6220_GPIO6_BASE	(void *)0xf7022000
> +#define HI6220_GPIO7_BASE	(void *)0xf7023000
> +#define HI6220_GPIO8_BASE	(void *)0xf7024000
> +#define HI6220_GPIO9_BASE	(void *)0xf7025000
> +#define HI6220_GPIO10_BASE	(void *)0xf7026000
> +#define HI6220_GPIO11_BASE	(void *)0xf7027000
> +#define HI6220_GPIO12_BASE	(void *)0xf7028000
> +#define HI6220_GPIO13_BASE	(void *)0xf7029000
> +#define HI6220_GPIO14_BASE	(void *)0xf702a000
> +#define HI6220_GPIO15_BASE	(void *)0xf702b000
> +#define HI6220_GPIO16_BASE	(void *)0xf702c000
> +#define HI6220_GPIO17_BASE	(void *)0xf702d000
> +#define HI6220_GPIO18_BASE	(void *)0xf702e000
> +#define HI6220_GPIO19_BASE	(void *)0xf702f000

But are these even used in the driver anywhere ?

> +#define BIT(x)			(1 << (x))

This macro should be placed into common header files.

> +#define HI6220_GPIO_PER_BANK	8
> +#define HI6220_GPIO_DIR		0x400
> +
> +struct gpio_bank {
> +	u8 *base;	/* address of registers in physical memory */

Should be void __iomem *, no ?

> +};
> +
> +/* Information about a GPIO bank */
> +struct hikey_gpio_platdata {
> +	int bank_index;
> +	void *base;     /* address of registers in physical memory */
> +};
> +
> +#endif /* _HI6220_GPIO_H_ */

[...]

[U-Boot] [PATCH 5/6] mmc: hi6220_dw_mmc: Add hi6220 glue code for dw_mmc controller.

[Marek Vasut]

On Tuesday, May 12, 2015 at 03:38:31 PM, Peter Griffin wrote:
> This patch adds the glue code for hi6220 SoC which has 2x synopsis
> dw_mmc controllers. This will be used by the hikey board support
> in subsequent patches.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>

[...]

> diff --git a/drivers/mmc/hi6220_dw_mmc.c b/drivers/mmc/hi6220_dw_mmc.c
> new file mode 100644
> index 0000000..a3880a3
> --- /dev/null
> +++ b/drivers/mmc/hi6220_dw_mmc.c
> @@ -0,0 +1,63 @@
> +/*
> + * (C) Copyright 2015 Linaro
> + * peter.griffin <peter.griffin@linaro.org>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dwmmc.h>
> +#include <malloc.h>
> +#include <asm-generic/errno.h>
> +
> +#define	DWMMC_MAX_CH_NUM		4
> +
> +/*
> +#define	DWMMC_MAX_FREQ			52000000
> +#define	DWMMC_MIN_FREQ			400000
> +*/

Please zap these dead macros.

> +/*TODO we should probably use the frequencies above, but ATF uses
> +  the ones below so stick with that for the moment */
> +#define	DWMMC_MAX_FREQ			50000000
> +#define	DWMMC_MIN_FREQ			378000
> +
> +/* Source clock is configured to 100Mhz by ATF bl1*/
> +#define MMC0_DEFAULT_FREQ		100000000

[...]

> +int hi6220_dwmci_add_port(int index, u32 regbase, int bus_width)
> +{
> +	struct dwmci_host *host = NULL;
> +
> +	host = malloc(sizeof(struct dwmci_host));

calloc(1, sizeof(...)) so the data are inited/zero'd out please.
[...]

[U-Boot] [PATCH 2/6] dm: gpio: hi6220: Add a hi6220 GPIO driver model driver.

[Tom Rini]

On Tue, May 12, 2015 at 08:36:30PM +0200, Marek Vasut wrote:
> On Tuesday, May 12, 2015 at 03:38:28 PM, Peter Griffin wrote:
[snip]
> > +#define BIT(x)			(1 << (x))
> 
> This macro should be placed into common header files.

We'll fix that once Jagan's series comes in which gives everyone BIT().

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150512/cc8ecd94/attachment.sig>

[U-Boot] [PATCH 1/6] usb: dwc2: Add support for v3 snpsid value

[Peter Griffin]

Hi Marek,

On Tue, 12 May 2015, Marek Vasut wrote:

> On Tuesday, May 12, 2015 at 03:38:27 PM, Peter Griffin wrote:
> > This has been tested to the extent that I can enumerate
> > a asix usb networking adapter and boot a kernel over usb
> > on the 96boards hikey u-boot port I'm currently doing.
> > 
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  drivers/usb/host/dwc2.c | 3 ++-
> >  drivers/usb/host/dwc2.h | 1 +
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
> > index e8142ac..19a6007 100644
> > --- a/drivers/usb/host/dwc2.c
> > +++ b/drivers/usb/host/dwc2.c
> > @@ -1015,7 +1015,8 @@ int usb_lowlevel_init(int index, enum usb_init_type
> > init, void **controller) snpsid = readl(&regs->gsnpsid);
> >  	printf("Core Release: %x.%03x\n", snpsid >> 12 & 0xf, snpsid & 0xfff);
> > 
> > -	if ((snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_2xx) {
> > +	if ((snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_2xx &&
> > +	    (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_3xx) {
> 
> Isn't this then a DWC3 controller instead ?

No I don't believe so. dwc3 is a usb3 controller with different register set AFAIK.

I believe this piece of code is replicating what the Linux
driver does here: -

http://lxr.free-electrons.com/source/drivers/usb/dwc2/core.c#L2664

Also see here for the various different sub core revisions within 2.xx and 3.xx
which are supported by the dwc2 Linux driver
http://lxr.free-electrons.com/source/drivers/usb/dwc2/core.h#L625.

This also matches up with the available datasheet here
https://github.com/96boards/documentation/blob/master/hikey/
Hi6220V100_Multi-Mode_Application_Processor_Function_Description.pdf on page 219
which states it is a USB2 OTG controller, but near the bottom of the page also
tates: -

"The USB core used in Hi6220 is build from Synopsys IP(3.00a).".

I just checked through the DWC_otg_databook.odf, and these revision numbers also match up
with the "Revision History" changes in the document. So it all ties together.

It is worth highlighting at this point that not bailing here (along with setting up
the appropriate clocks to the IP) was enough for me to get a DWC2 core rev 3.00a 
enumerating a ASIX networking adapter and tftp a kernel into DDR. However there maybe
some other changes required to "fully" support the 3.00a hardware version. There is of
course the problem I mentioned in the cover letter regarding enumerating mass storage
devices which may or may not be related to this.

Although a quick grep of the Linux driver only gets three hits

git grep DWC2_CORE_REV_3_00a
core.h:#define DWC2_CORE_REV_3_00a      0x4f54300a
core_intr.c:            if (hsotg->hw_params.snpsid >= DWC2_CORE_REV_3_00a)
hcd.c:  if (hsotg->hw_params.snpsid >= DWC2_CORE_REV_3_00a) {

Neither of which seem relevant to the mass storage device problem (one adds a delay in
the irq handler, and the other sets a bit when freeing the hcd.

kind regards,

Peter.

[U-Boot] [PATCH 2/6] dm: gpio: hi6220: Add a hi6220 GPIO driver model driver.

[Peter Griffin]

Hi Marek,

Thanks for reviewing.

> 
> > This patch adds support for the GPIO perif found on hi6220
> > SoC.
> > 
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  arch/arm/include/asm/arch-armv8/gpio.h | 47 +++++++++++++++++
> >  drivers/gpio/Makefile                  |  2 +
> >  drivers/gpio/hi6220_gpio.c             | 95
> > ++++++++++++++++++++++++++++++++++ 3 files changed, 144 insertions(+)
> >  create mode 100644 arch/arm/include/asm/arch-armv8/gpio.h
> >  create mode 100644 drivers/gpio/hi6220_gpio.c
> > 
> > diff --git a/arch/arm/include/asm/arch-armv8/gpio.h
> > b/arch/arm/include/asm/arch-armv8/gpio.h new file mode 100644
> > index 0000000..162c2d9
> > --- /dev/null
> > +++ b/arch/arm/include/asm/arch-armv8/gpio.h
> > @@ -0,0 +1,47 @@
> > +/*
> > + * Copyright (C) 2015 Linaro
> > + * Peter Griffin <peter.griffin@linaro.org>
> > + *
> > + * SPDX-License-Identifier:	GPL-2.0+
> > + */
> > +
> > +#ifndef _HI6220_GPIO_H_
> > +#define _HI6220_GPIO_H_
> > +
> > +#define HI6220_GPIO0_BASE	(void *)0xf8011000
> 
> You should drop the explicit cast, that's nasty.

If I drop the cast I get lots of 

include/asm/arch/gpio.h:11:32: warning: initialization makes pointer from integer
without a cast

compiler warnings.


> Also, why don't you define this as a HI6220_GPIO_BASE(bank) instead?
> That'd trim down the number of macros and from what I see, it should
> be rather easy to do ...
> 
> #define HI6220_GPIO_BASE(bank)
> 	(((bank < 4) ? 0xf8012000 : (0xf7020000 - 0x4000)) + (0x1000 * bank))

Yes good idea, I will do it like you suggest in V2. Currently I have

#define HI6220_GPIO_BASE(bank)	(void *)(((bank < 4) ? 0xf8011000 : \
				0xf7020000 - 0x4000) + (0x1000 * bank))

To avoid the warnings mentioned above.

<snip>
> > +#define HI6220_GPIO17_BASE	(void *)0xf702d000
> > +#define HI6220_GPIO18_BASE	(void *)0xf702e000
> > +#define HI6220_GPIO19_BASE	(void *)0xf702f000
> 
> But are these even used in the driver anywhere ?

They are currently used in the hikey.c board file which defines the 
hikey_gpio_platdata structure.

Although thinking about it some more this should be moved from the hikey board
file to the driver as it is SoC specific rather than board specific.
> 
> > +#define BIT(x)			(1 << (x))
> 
> This macro should be placed into common header files.
> 
> > +#define HI6220_GPIO_PER_BANK	8
> > +#define HI6220_GPIO_DIR		0x400
> > +
> > +struct gpio_bank {
> > +	u8 *base;	/* address of registers in physical memory */
> 
> Should be void __iomem *, no ?

Will fix in v2.

regards,

Peter.

[U-Boot] [PATCH 5/6] mmc: hi6220_dw_mmc: Add hi6220 glue code for dw_mmc controller.

[Peter Griffin]

Hi Marek,

On Tue, 12 May 2015, Marek Vasut wrote:

> On Tuesday, May 12, 2015 at 03:38:31 PM, Peter Griffin wrote:
> > This patch adds the glue code for hi6220 SoC which has 2x synopsis
> > dw_mmc controllers. This will be used by the hikey board support
> > in subsequent patches.
> > 
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> 
> [...]
> 
> > diff --git a/drivers/mmc/hi6220_dw_mmc.c b/drivers/mmc/hi6220_dw_mmc.c
> > new file mode 100644
> > index 0000000..a3880a3
> > --- /dev/null
> > +++ b/drivers/mmc/hi6220_dw_mmc.c
> > @@ -0,0 +1,63 @@
> > +/*
> > + * (C) Copyright 2015 Linaro
> > + * peter.griffin <peter.griffin@linaro.org>
> > + *
> > + * SPDX-License-Identifier:	GPL-2.0+
> > + */
> > +
> > +#include <common.h>
> > +#include <dwmmc.h>
> > +#include <malloc.h>
> > +#include <asm-generic/errno.h>
> > +
> > +#define	DWMMC_MAX_CH_NUM		4
> > +
> > +/*
> > +#define	DWMMC_MAX_FREQ			52000000
> > +#define	DWMMC_MIN_FREQ			400000
> > +*/
> 
> Please zap these dead macros.

Ok will fix in v2.
> 
> > +/*TODO we should probably use the frequencies above, but ATF uses
> > +  the ones below so stick with that for the moment */
> > +#define	DWMMC_MAX_FREQ			50000000
> > +#define	DWMMC_MIN_FREQ			378000
> > +
> > +/* Source clock is configured to 100Mhz by ATF bl1*/
> > +#define MMC0_DEFAULT_FREQ		100000000
> 
> [...]
> 
> > +int hi6220_dwmci_add_port(int index, u32 regbase, int bus_width)
> > +{
> > +	struct dwmci_host *host = NULL;
> > +
> > +	host = malloc(sizeof(struct dwmci_host));
> 
> calloc(1, sizeof(...)) so the data are inited/zero'd out please.
> [...]

Ok will fix in v2

regards,

Peter.

[U-Boot] [PATCH 2/6] dm: gpio: hi6220: Add a hi6220 GPIO driver model driver.

[Marek Vasut]

On Wednesday, May 13, 2015 at 02:00:13 PM, Peter Griffin wrote:
> Hi Marek,
> 
> Thanks for reviewing.

Hi!

> > > +#ifndef _HI6220_GPIO_H_
> > > +#define _HI6220_GPIO_H_
> > > +
> > > +#define HI6220_GPIO0_BASE	(void *)0xf8011000
> > 
> > You should drop the explicit cast, that's nasty.
> 
> If I drop the cast I get lots of
> 
> include/asm/arch/gpio.h:11:32: warning: initialization makes pointer from
> integer without a cast
> 
> compiler warnings.

But this is gpio.h header for your architecture, so maybe fix it there too?

> > Also, why don't you define this as a HI6220_GPIO_BASE(bank) instead?
> > That'd trim down the number of macros and from what I see, it should
> > be rather easy to do ...
> > 
> > #define HI6220_GPIO_BASE(bank)
> > 
> > 	(((bank < 4) ? 0xf8012000 : (0xf7020000 - 0x4000)) + (0x1000 * bank))
> 
> Yes good idea, I will do it like you suggest in V2. Currently I have
> 
> #define HI6220_GPIO_BASE(bank)	(void *)(((bank < 4) ? 0xf8011000 : \
> 				0xf7020000 - 0x4000) + (0x1000 * bank))
> 
> To avoid the warnings mentioned above.

Yup, or maybe even make it an inline function so you get proper typechecking?

> <snip>
> 
> > > +#define HI6220_GPIO17_BASE	(void *)0xf702d000
> > > +#define HI6220_GPIO18_BASE	(void *)0xf702e000
> > > +#define HI6220_GPIO19_BASE	(void *)0xf702f000
> > 
> > But are these even used in the driver anywhere ?
> 
> They are currently used in the hikey.c board file which defines the
> hikey_gpio_platdata structure.
> 
> Although thinking about it some more this should be moved from the hikey
> board file to the driver as it is SoC specific rather than board specific.

This is specific to the CPU, so this should be in arch-<something> .

> > > +#define BIT(x)			(1 << (x))
> > 
> > This macro should be placed into common header files.
> > 
> > > +#define HI6220_GPIO_PER_BANK	8
> > > +#define HI6220_GPIO_DIR		0x400
> > > +
> > > +struct gpio_bank {
> > > +	u8 *base;	/* address of registers in physical memory */
> > 
> > Should be void __iomem *, no ?
> 
> Will fix in v2.

Thanks! Also, please wait a bit for other reviews before sending V2 :)

[U-Boot] [PATCH 1/6] usb: dwc2: Add support for v3 snpsid value

[Marek Vasut]

On Wednesday, May 13, 2015 at 01:00:01 PM, Peter Griffin wrote:
> Hi Marek,
> 
> On Tue, 12 May 2015, Marek Vasut wrote:
> > On Tuesday, May 12, 2015 at 03:38:27 PM, Peter Griffin wrote:
> > > This has been tested to the extent that I can enumerate
> > > a asix usb networking adapter and boot a kernel over usb
> > > on the 96boards hikey u-boot port I'm currently doing.
> > > 
> > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > > ---
> > > 
> > >  drivers/usb/host/dwc2.c | 3 ++-
> > >  drivers/usb/host/dwc2.h | 1 +
> > >  2 files changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
> > > index e8142ac..19a6007 100644
> > > --- a/drivers/usb/host/dwc2.c
> > > +++ b/drivers/usb/host/dwc2.c
> > > @@ -1015,7 +1015,8 @@ int usb_lowlevel_init(int index, enum
> > > usb_init_type init, void **controller) snpsid = readl(&regs->gsnpsid);
> > > 
> > >  	printf("Core Release: %x.%03x\n", snpsid >> 12 & 0xf, snpsid &
> > >  	0xfff);
> > > 
> > > -	if ((snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_2xx) {
> > > +	if ((snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_2xx &&
> > > +	    (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_3xx) {
> > 
> > Isn't this then a DWC3 controller instead ?
> 
> No I don't believe so. dwc3 is a usb3 controller with different register
> set AFAIK.
> 
> I believe this piece of code is replicating what the Linux
> driver does here: -
> 
> http://lxr.free-electrons.com/source/drivers/usb/dwc2/core.c#L2664
> 
> Also see here for the various different sub core revisions within 2.xx and
> 3.xx which are supported by the dwc2 Linux driver
> http://lxr.free-electrons.com/source/drivers/usb/dwc2/core.h#L625.
> 
> This also matches up with the available datasheet here
> https://github.com/96boards/documentation/blob/master/hikey/
> Hi6220V100_Multi-Mode_Application_Processor_Function_Description.pdf on
> page 219 which states it is a USB2 OTG controller, but near the bottom of
> the page also tates: -
> 
> "The USB core used in Hi6220 is build from Synopsys IP(3.00a).".
> 
> I just checked through the DWC_otg_databook.odf, and these revision numbers
> also match up with the "Revision History" changes in the document. So it
> all ties together.
> 
> It is worth highlighting at this point that not bailing here (along with
> setting up the appropriate clocks to the IP) was enough for me to get a
> DWC2 core rev 3.00a enumerating a ASIX networking adapter and tftp a
> kernel into DDR. However there maybe some other changes required to
> "fully" support the 3.00a hardware version. There is of course the problem
> I mentioned in the cover letter regarding enumerating mass storage devices
> which may or may not be related to this.
> 
> Although a quick grep of the Linux driver only gets three hits
> 
> git grep DWC2_CORE_REV_3_00a
> core.h:#define DWC2_CORE_REV_3_00a      0x4f54300a
> core_intr.c:            if (hsotg->hw_params.snpsid >= DWC2_CORE_REV_3_00a)
> hcd.c:  if (hsotg->hw_params.snpsid >= DWC2_CORE_REV_3_00a) {
> 
> Neither of which seem relevant to the mass storage device problem (one adds
> a delay in the irq handler, and the other sets a bit when freeing the hcd.

Ooh, I see. Thank you for the extensive explanation!

I picked this patch for u-boot-usb, so feel free to submit any improvements
for the 3.xx core as you cook them. Thanks!

[U-Boot] [PATCH 2/6] dm: gpio: hi6220: Add a hi6220 GPIO driver model driver.

[Simon Glass]

Hi Peter,

On 12 May 2015 at 07:25, Peter Griffin <peter.griffin@linaro.org> wrote:
> This patch adds support for the GPIO perif found on hi6220
> SoC.
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  arch/arm/include/asm/arch-armv8/gpio.h | 47 +++++++++++++++++
>  drivers/gpio/Makefile                  |  2 +
>  drivers/gpio/hi6220_gpio.c             | 95 ++++++++++++++++++++++++++++++++++
>  3 files changed, 144 insertions(+)
>  create mode 100644 arch/arm/include/asm/arch-armv8/gpio.h
>  create mode 100644 drivers/gpio/hi6220_gpio.c
>
> diff --git a/arch/arm/include/asm/arch-armv8/gpio.h b/arch/arm/include/asm/arch-armv8/gpio.h
> new file mode 100644
> index 0000000..162c2d9
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-armv8/gpio.h
> @@ -0,0 +1,47 @@
> +/*
> + * Copyright (C) 2015 Linaro
> + * Peter Griffin <peter.griffin@linaro.org>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef _HI6220_GPIO_H_
> +#define _HI6220_GPIO_H_
> +
> +#define HI6220_GPIO0_BASE      (void *)0xf8011000
> +#define HI6220_GPIO1_BASE      (void *)0xf8012000
> +#define HI6220_GPIO2_BASE      (void *)0xf8013000
> +#define HI6220_GPIO3_BASE      (void *)0xf8014000
> +#define HI6220_GPIO4_BASE      (void *)0xf7020000
> +#define HI6220_GPIO5_BASE      (void *)0xf7021000
> +#define HI6220_GPIO6_BASE      (void *)0xf7022000
> +#define HI6220_GPIO7_BASE      (void *)0xf7023000
> +#define HI6220_GPIO8_BASE      (void *)0xf7024000
> +#define HI6220_GPIO9_BASE      (void *)0xf7025000
> +#define HI6220_GPIO10_BASE     (void *)0xf7026000
> +#define HI6220_GPIO11_BASE     (void *)0xf7027000
> +#define HI6220_GPIO12_BASE     (void *)0xf7028000
> +#define HI6220_GPIO13_BASE     (void *)0xf7029000
> +#define HI6220_GPIO14_BASE     (void *)0xf702a000
> +#define HI6220_GPIO15_BASE     (void *)0xf702b000
> +#define HI6220_GPIO16_BASE     (void *)0xf702c000
> +#define HI6220_GPIO17_BASE     (void *)0xf702d000
> +#define HI6220_GPIO18_BASE     (void *)0xf702e000
> +#define HI6220_GPIO19_BASE     (void *)0xf702f000

I see device tree patches for this SoC  - so is it possible to use
device tree for this driver and avoid including these addresses?



[snip]

Regards,
Simon

[U-Boot] [PATCH 2/6] dm: gpio: hi6220: Add a hi6220 GPIO driver model driver.

[Peter Griffin]

This patch adds support for the GPIO perif found on hi6220
SoC.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm/include/asm/arch-armv8/gpio.h | 47 +++++++++++++++++
 drivers/gpio/Makefile                  |  2 +
 drivers/gpio/hi6220_gpio.c             | 95 ++++++++++++++++++++++++++++++++++
 3 files changed, 144 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-armv8/gpio.h
 create mode 100644 drivers/gpio/hi6220_gpio.c

diff --git a/arch/arm/include/asm/arch-armv8/gpio.h b/arch/arm/include/asm/arch-armv8/gpio.h
new file mode 100644
index 0000000..162c2d9
--- /dev/null
+++ b/arch/arm/include/asm/arch-armv8/gpio.h
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2015 Linaro
+ * Peter Griffin <peter.griffin@linaro.org>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _HI6220_GPIO_H_
+#define _HI6220_GPIO_H_
+
+#define HI6220_GPIO0_BASE	(void *)0xf8011000
+#define HI6220_GPIO1_BASE	(void *)0xf8012000
+#define HI6220_GPIO2_BASE	(void *)0xf8013000
+#define HI6220_GPIO3_BASE	(void *)0xf8014000
+#define HI6220_GPIO4_BASE	(void *)0xf7020000
+#define HI6220_GPIO5_BASE	(void *)0xf7021000
+#define HI6220_GPIO6_BASE	(void *)0xf7022000
+#define HI6220_GPIO7_BASE	(void *)0xf7023000
+#define HI6220_GPIO8_BASE	(void *)0xf7024000
+#define HI6220_GPIO9_BASE	(void *)0xf7025000
+#define HI6220_GPIO10_BASE	(void *)0xf7026000
+#define HI6220_GPIO11_BASE	(void *)0xf7027000
+#define HI6220_GPIO12_BASE	(void *)0xf7028000
+#define HI6220_GPIO13_BASE	(void *)0xf7029000
+#define HI6220_GPIO14_BASE	(void *)0xf702a000
+#define HI6220_GPIO15_BASE	(void *)0xf702b000
+#define HI6220_GPIO16_BASE	(void *)0xf702c000
+#define HI6220_GPIO17_BASE	(void *)0xf702d000
+#define HI6220_GPIO18_BASE	(void *)0xf702e000
+#define HI6220_GPIO19_BASE	(void *)0xf702f000
+
+#define BIT(x)			(1 << (x))
+
+#define HI6220_GPIO_PER_BANK	8
+#define HI6220_GPIO_DIR		0x400
+
+struct gpio_bank {
+	u8 *base;	/* address of registers in physical memory */
+};
+
+/* Information about a GPIO bank */
+struct hikey_gpio_platdata {
+	int bank_index;
+	void *base;     /* address of registers in physical memory */
+};
+
+#endif /* _HI6220_GPIO_H_ */
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 85f71c5..470fabd 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -42,3 +42,5 @@ obj-$(CONFIG_TCA642X)		+= tca642x.o
 oby-$(CONFIG_SX151X)		+= sx151x.o
 obj-$(CONFIG_SUNXI_GPIO)	+= sunxi_gpio.o
 obj-$(CONFIG_LPC32XX_GPIO)	+= lpc32xx_gpio.o
+obj-$(CONFIG_HIKEY_GPIO)	+= hi6220_gpio.o
+
diff --git a/drivers/gpio/hi6220_gpio.c b/drivers/gpio/hi6220_gpio.c
new file mode 100644
index 0000000..3f41bff
--- /dev/null
+++ b/drivers/gpio/hi6220_gpio.c
@@ -0,0 +1,95 @@
+/*
+ * Copyright (C) 2015 Linaro
+ * Peter Griffin <peter.griffin@linaro.org>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <asm/gpio.h>
+#include <asm/io.h>
+#include <errno.h>
+
+static int hi6220_gpio_direction_input(struct udevice *dev, unsigned int gpio)
+{
+	struct gpio_bank *bank = dev_get_priv(dev);
+	u8 data;
+
+	data = readb(bank->base + HI6220_GPIO_DIR);
+	data &= ~(1 << gpio);
+	writeb(data, bank->base + HI6220_GPIO_DIR);
+
+	return 0;
+}
+
+static int hi6220_gpio_set_value(struct udevice *dev, unsigned gpio,
+				  int value)
+{
+	struct gpio_bank *bank = dev_get_priv(dev);
+
+	writeb(!!value << gpio, bank->base + (BIT(gpio + 2)));
+	return 0;
+}
+
+static int hi6220_gpio_direction_output(struct udevice *dev, unsigned gpio,
+					int value)
+{
+	struct gpio_bank *bank = dev_get_priv(dev);
+	u8 data;
+
+	data = readb(bank->base + HI6220_GPIO_DIR);
+	data |= 1 << gpio;
+	writeb(data, bank->base + HI6220_GPIO_DIR);
+
+	hi6220_gpio_set_value(dev, gpio, value);
+
+	return 0;
+}
+
+static int hi6220_gpio_get_value(struct udevice *dev, unsigned gpio)
+{
+	struct gpio_bank *bank = dev_get_priv(dev);
+
+	return !!readb(bank->base + (BIT(gpio + 2)));
+}
+
+
+
+static const struct dm_gpio_ops gpio_hi6220_ops = {
+	.direction_input	= hi6220_gpio_direction_input,
+	.direction_output	= hi6220_gpio_direction_output,
+	.get_value		= hi6220_gpio_get_value,
+	.set_value		= hi6220_gpio_set_value,
+};
+
+static int hi6220_gpio_probe(struct udevice *dev)
+{
+	struct gpio_bank *bank = dev_get_priv(dev);
+	struct hikey_gpio_platdata *plat = dev_get_platdata(dev);
+	struct gpio_dev_priv *uc_priv = dev->uclass_priv;
+	char name[18], *str;
+
+	sprintf(name, "GPIO%d_", plat->bank_index);
+
+	str = strdup(name);
+	if (!str)
+		return -ENOMEM;
+
+	uc_priv->bank_name = str;
+	uc_priv->gpio_count = HI6220_GPIO_PER_BANK;
+
+	bank->base = (u8 *)plat->base;
+
+	return 0;
+}
+
+U_BOOT_DRIVER(gpio_hi6220) = {
+	.name	= "gpio_hi6220",
+	.id	= UCLASS_GPIO,
+	.ops	= &gpio_hi6220_ops,
+	.probe	= hi6220_gpio_probe,
+	.priv_auto_alloc_size = sizeof(struct gpio_bank),
+};
+
+
-- 
1.9.1