Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-02-01 Thread Andy Shevchenko 
On Thu, Feb 1, 2018 at 2:22 PM, Marcus Folkesson
 wrote:
> On Thu, Feb 01, 2018 at 11:08:46AM +0800, Baolin Wang wrote:

> Then you could replace

> +subsys_initcall(sprd_gpio_init);

> module_platform_driver(sprd_gpio_driver);

It's not an equivalent when built-in


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-02-01 Thread Andy Shevchenko 
On Thu, Feb 1, 2018 at 2:22 PM, Marcus Folkesson
 wrote:
> On Thu, Feb 01, 2018 at 11:08:46AM +0800, Baolin Wang wrote:

> Then you could replace

> +subsys_initcall(sprd_gpio_init);

> module_platform_driver(sprd_gpio_driver);

It's not an equivalent when built-in


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-02-01 Thread Baolin Wang 
On 1 February 2018 at 20:22, Marcus Folkesson
 wrote:
> On Thu, Feb 01, 2018 at 11:08:46AM +0800, Baolin Wang wrote:
>> On 31 January 2018 at 22:23, Andy Shevchenko  
>> wrote:
>> > On Wed, Jan 31, 2018 at 4:01 AM, Baolin Wang  
>> > wrote:
>> >> On 31 January 2018 at 00:48, Andy Shevchenko  
>> >> wrote:
>> >>> On Tue, Jan 30, 2018 at 2:07 PM, Baolin Wang  
>> >>> wrote:
>>  The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
>>  each group contains 16 GPIOs. Each GPIO can set input/output and has
>>  the interrupt capability.
>> >>>
>>  +config GPIO_SPRD
>> >>>
>>  +   bool "Spreadtrum GPIO support"
>> >>>
>> >>> Either you have to put tristate here, or remove all redundant
>> >>> module_*() and MODULE_*() macros.
>> >>
>> >> I will remove module_*() and MODULE_*() macros in next version. Thanks
>> >> for your comments.
>> >
>> > In that case you need to explain why driver can't be module. (And
>> > don't forget to replace module.h with init.h).
>>
>> After more investigation, I found most GPIO dependencies can be
>> deferred to probe. So I will change the GPIO driver to module level
>> and change bool to tristate in next version.
>
> Then you could replace
>
> +static int __init sprd_gpio_init(void)
> +{
> +   return platform_driver_register(_gpio_driver);
> +}
> +subsys_initcall(sprd_gpio_init);
> +
> +static void __exit sprd_gpio_exit(void)
> +{
> +   platform_driver_unregister(_gpio_driver);
> +}
> +module_exit(sprd_gpio_exit);
>
> with
>
> module_platform_driver(sprd_gpio_driver);

Yes, I already did this in V2 patch set, you can refer to the V2 patch
set. Thanks.

-- 
Baolin.wang
Best Regards

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-02-01 Thread Baolin Wang 
On 1 February 2018 at 20:22, Marcus Folkesson
 wrote:
> On Thu, Feb 01, 2018 at 11:08:46AM +0800, Baolin Wang wrote:
>> On 31 January 2018 at 22:23, Andy Shevchenko  
>> wrote:
>> > On Wed, Jan 31, 2018 at 4:01 AM, Baolin Wang  
>> > wrote:
>> >> On 31 January 2018 at 00:48, Andy Shevchenko  
>> >> wrote:
>> >>> On Tue, Jan 30, 2018 at 2:07 PM, Baolin Wang  
>> >>> wrote:
>>  The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
>>  each group contains 16 GPIOs. Each GPIO can set input/output and has
>>  the interrupt capability.
>> >>>
>>  +config GPIO_SPRD
>> >>>
>>  +   bool "Spreadtrum GPIO support"
>> >>>
>> >>> Either you have to put tristate here, or remove all redundant
>> >>> module_*() and MODULE_*() macros.
>> >>
>> >> I will remove module_*() and MODULE_*() macros in next version. Thanks
>> >> for your comments.
>> >
>> > In that case you need to explain why driver can't be module. (And
>> > don't forget to replace module.h with init.h).
>>
>> After more investigation, I found most GPIO dependencies can be
>> deferred to probe. So I will change the GPIO driver to module level
>> and change bool to tristate in next version.
>
> Then you could replace
>
> +static int __init sprd_gpio_init(void)
> +{
> +   return platform_driver_register(_gpio_driver);
> +}
> +subsys_initcall(sprd_gpio_init);
> +
> +static void __exit sprd_gpio_exit(void)
> +{
> +   platform_driver_unregister(_gpio_driver);
> +}
> +module_exit(sprd_gpio_exit);
>
> with
>
> module_platform_driver(sprd_gpio_driver);

Yes, I already did this in V2 patch set, you can refer to the V2 patch
set. Thanks.

-- 
Baolin.wang
Best Regards

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-02-01 Thread Baolin Wang 
Hi Markus,

On 1 February 2018 at 20:17, Marcus Folkesson
 wrote:
> Hi Baolin,
>
> On Tue, Jan 30, 2018 at 08:07:43PM +0800, Baolin Wang wrote:
>> The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
>> each group contains 16 GPIOs. Each GPIO can set input/output and has
>> the interrupt capability.
>>
>> Signed-off-by: Baolin Wang 
>> ---
>> diff --git a/drivers/gpio/gpio-sprd.c b/drivers/gpio/gpio-sprd.c
>> new file mode 100644
>> index 000..af59b9f
>> --- /dev/null
>> +++ b/drivers/gpio/gpio-sprd.c
>> @@ -0,0 +1,301 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Spreadtrum Communications Inc.
>> + * Copyright (c) 2018 Linaro Ltd.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/* GPIO registers definition */
>> +#define SPRD_GPIO_DATA   0x0
>> +#define SPRD_GPIO_DMSK   0x4
>> +#define SPRD_GPIO_DIR0x8
>> +#define SPRD_GPIO_IS 0xc
>> +#define SPRD_GPIO_IBE0x10
>> +#define SPRD_GPIO_IEV0x14
>> +#define SPRD_GPIO_IE 0x18
>> +#define SPRD_GPIO_RIS0x1c
>> +#define SPRD_GPIO_MIS0x20
>> +#define SPRD_GPIO_IC 0x24
>> +#define SPRD_GPIO_INEN   0x28
>> +
>> +/* We have 16 groups GPIOs and each group contain 16 GPIOs */
>> +#define SPRD_GPIO_GROUP_NR   16
>> +#define SPRD_GPIO_NR 256
>> +#define SPRD_GPIO_GROUP_SIZE 0x80
>> +#define SPRD_GPIO_GROUP_MASK GENMASK(15, 0)
>> +#define SPRD_GPIO_BIT(x) ((x) & (SPRD_GPIO_GROUP_NR - 1))
>> +
>> +struct sprd_gpio {
>> + struct gpio_chip chip;
>> + void __iomem *base;
>> + spinlock_t lock;
>> + int irq;
>> +};
>> +
>> +static inline void __iomem *sprd_gpio_group_base(struct sprd_gpio 
>> *sprd_gpio,
>> +  unsigned int group)
>> +{
>> + return sprd_gpio->base + SPRD_GPIO_GROUP_SIZE * group;
>> +}
>> +
>> +static void sprd_gpio_update(struct gpio_chip *chip, unsigned int offset,
>> +  unsigned int reg, unsigned int val)
>> +{
>> + struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip);
>> + void __iomem *base = sprd_gpio_group_base(sprd_gpio,
>> +   offset / SPRD_GPIO_GROUP_NR);
>> + u32 shift = SPRD_GPIO_BIT(offset);
>> + unsigned long flags;
>> + u32 orig, tmp;
>> +
>> + spin_lock_irqsave(_gpio->lock, flags);
>> + orig = readl_relaxed(base + reg);
>> +
>> + tmp = (orig & ~BIT(shift)) | (val << shift);
>> + writel_relaxed(tmp, base + reg);
>> + spin_unlock_irqrestore(_gpio->lock, flags);
>> +}
>> +
>> +static int sprd_gpio_read(struct gpio_chip *chip, unsigned int offset,
>> +   unsigned int reg)
>> +{
>> + struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip);
>> + void __iomem *base = sprd_gpio_group_base(sprd_gpio,
>> +   offset / SPRD_GPIO_GROUP_NR);
>> + u32 value = readl_relaxed(base + reg) & SPRD_GPIO_GROUP_MASK;
>> + u32 shift = SPRD_GPIO_BIT(offset);
>> +
>> + return !!(value & BIT(shift));
>> +}
>> +
>> +static int sprd_gpio_request(struct gpio_chip *chip, unsigned int offset)
>> +{
>> + sprd_gpio_update(chip, offset, SPRD_GPIO_DMSK, 1);
>> + return 0;
>> +}
>
> Better to change the function to void since the return value is not
> valueable.

The function prototype of gpio_chip structure need one return value,
so we need give one returned value though it is no meaningless.

>
>> +
>> +static void sprd_gpio_free(struct gpio_chip *chip, unsigned int offset)
>> +{
>> + sprd_gpio_update(chip, offset, SPRD_GPIO_DMSK, 0);
>> +}
>> +
>> +static int sprd_gpio_direction_input(struct gpio_chip *chip,
>> +  unsigned int offset)
>> +{
>> + sprd_gpio_update(chip, offset, SPRD_GPIO_DIR, 0);
>> + sprd_gpio_update(chip, offset, SPRD_GPIO_INEN, 1);
>> + return 0;
>> +}
>
> Same here
>
>
>> +
>> +static int sprd_gpio_direction_output(struct gpio_chip *chip,
>> +   unsigned int offset, int value)
>> +{
>> + sprd_gpio_update(chip, offset, SPRD_GPIO_DIR, 1);
>> + sprd_gpio_update(chip, offset, SPRD_GPIO_INEN, 0);
>> + sprd_gpio_update(chip, offset, SPRD_GPIO_DATA, value);
>> + return 0;
>> +}
>
> and here.
>
>
> Thanks,
> Marcus Folkesson
>
>
>



-- 
Baolin.wang
Best Regards

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-02-01 Thread Baolin Wang 
Hi Markus,

On 1 February 2018 at 20:17, Marcus Folkesson
 wrote:
> Hi Baolin,
>
> On Tue, Jan 30, 2018 at 08:07:43PM +0800, Baolin Wang wrote:
>> The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
>> each group contains 16 GPIOs. Each GPIO can set input/output and has
>> the interrupt capability.
>>
>> Signed-off-by: Baolin Wang 
>> ---
>> diff --git a/drivers/gpio/gpio-sprd.c b/drivers/gpio/gpio-sprd.c
>> new file mode 100644
>> index 000..af59b9f
>> --- /dev/null
>> +++ b/drivers/gpio/gpio-sprd.c
>> @@ -0,0 +1,301 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Spreadtrum Communications Inc.
>> + * Copyright (c) 2018 Linaro Ltd.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/* GPIO registers definition */
>> +#define SPRD_GPIO_DATA   0x0
>> +#define SPRD_GPIO_DMSK   0x4
>> +#define SPRD_GPIO_DIR0x8
>> +#define SPRD_GPIO_IS 0xc
>> +#define SPRD_GPIO_IBE0x10
>> +#define SPRD_GPIO_IEV0x14
>> +#define SPRD_GPIO_IE 0x18
>> +#define SPRD_GPIO_RIS0x1c
>> +#define SPRD_GPIO_MIS0x20
>> +#define SPRD_GPIO_IC 0x24
>> +#define SPRD_GPIO_INEN   0x28
>> +
>> +/* We have 16 groups GPIOs and each group contain 16 GPIOs */
>> +#define SPRD_GPIO_GROUP_NR   16
>> +#define SPRD_GPIO_NR 256
>> +#define SPRD_GPIO_GROUP_SIZE 0x80
>> +#define SPRD_GPIO_GROUP_MASK GENMASK(15, 0)
>> +#define SPRD_GPIO_BIT(x) ((x) & (SPRD_GPIO_GROUP_NR - 1))
>> +
>> +struct sprd_gpio {
>> + struct gpio_chip chip;
>> + void __iomem *base;
>> + spinlock_t lock;
>> + int irq;
>> +};
>> +
>> +static inline void __iomem *sprd_gpio_group_base(struct sprd_gpio 
>> *sprd_gpio,
>> +  unsigned int group)
>> +{
>> + return sprd_gpio->base + SPRD_GPIO_GROUP_SIZE * group;
>> +}
>> +
>> +static void sprd_gpio_update(struct gpio_chip *chip, unsigned int offset,
>> +  unsigned int reg, unsigned int val)
>> +{
>> + struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip);
>> + void __iomem *base = sprd_gpio_group_base(sprd_gpio,
>> +   offset / SPRD_GPIO_GROUP_NR);
>> + u32 shift = SPRD_GPIO_BIT(offset);
>> + unsigned long flags;
>> + u32 orig, tmp;
>> +
>> + spin_lock_irqsave(_gpio->lock, flags);
>> + orig = readl_relaxed(base + reg);
>> +
>> + tmp = (orig & ~BIT(shift)) | (val << shift);
>> + writel_relaxed(tmp, base + reg);
>> + spin_unlock_irqrestore(_gpio->lock, flags);
>> +}
>> +
>> +static int sprd_gpio_read(struct gpio_chip *chip, unsigned int offset,
>> +   unsigned int reg)
>> +{
>> + struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip);
>> + void __iomem *base = sprd_gpio_group_base(sprd_gpio,
>> +   offset / SPRD_GPIO_GROUP_NR);
>> + u32 value = readl_relaxed(base + reg) & SPRD_GPIO_GROUP_MASK;
>> + u32 shift = SPRD_GPIO_BIT(offset);
>> +
>> + return !!(value & BIT(shift));
>> +}
>> +
>> +static int sprd_gpio_request(struct gpio_chip *chip, unsigned int offset)
>> +{
>> + sprd_gpio_update(chip, offset, SPRD_GPIO_DMSK, 1);
>> + return 0;
>> +}
>
> Better to change the function to void since the return value is not
> valueable.

The function prototype of gpio_chip structure need one return value,
so we need give one returned value though it is no meaningless.

>
>> +
>> +static void sprd_gpio_free(struct gpio_chip *chip, unsigned int offset)
>> +{
>> + sprd_gpio_update(chip, offset, SPRD_GPIO_DMSK, 0);
>> +}
>> +
>> +static int sprd_gpio_direction_input(struct gpio_chip *chip,
>> +  unsigned int offset)
>> +{
>> + sprd_gpio_update(chip, offset, SPRD_GPIO_DIR, 0);
>> + sprd_gpio_update(chip, offset, SPRD_GPIO_INEN, 1);
>> + return 0;
>> +}
>
> Same here
>
>
>> +
>> +static int sprd_gpio_direction_output(struct gpio_chip *chip,
>> +   unsigned int offset, int value)
>> +{
>> + sprd_gpio_update(chip, offset, SPRD_GPIO_DIR, 1);
>> + sprd_gpio_update(chip, offset, SPRD_GPIO_INEN, 0);
>> + sprd_gpio_update(chip, offset, SPRD_GPIO_DATA, value);
>> + return 0;
>> +}
>
> and here.
>
>
> Thanks,
> Marcus Folkesson
>
>
>



-- 
Baolin.wang
Best Regards

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-02-01 Thread Marcus Folkesson 
On Thu, Feb 01, 2018 at 11:08:46AM +0800, Baolin Wang wrote:
> On 31 January 2018 at 22:23, Andy Shevchenko  
> wrote:
> > On Wed, Jan 31, 2018 at 4:01 AM, Baolin Wang  wrote:
> >> On 31 January 2018 at 00:48, Andy Shevchenko  
> >> wrote:
> >>> On Tue, Jan 30, 2018 at 2:07 PM, Baolin Wang  
> >>> wrote:
>  The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
>  each group contains 16 GPIOs. Each GPIO can set input/output and has
>  the interrupt capability.
> >>>
>  +config GPIO_SPRD
> >>>
>  +   bool "Spreadtrum GPIO support"
> >>>
> >>> Either you have to put tristate here, or remove all redundant
> >>> module_*() and MODULE_*() macros.
> >>
> >> I will remove module_*() and MODULE_*() macros in next version. Thanks
> >> for your comments.
> >
> > In that case you need to explain why driver can't be module. (And
> > don't forget to replace module.h with init.h).
> 
> After more investigation, I found most GPIO dependencies can be
> deferred to probe. So I will change the GPIO driver to module level
> and change bool to tristate in next version.

Then you could replace

+static int __init sprd_gpio_init(void)
+{
+   return platform_driver_register(_gpio_driver);
+}
+subsys_initcall(sprd_gpio_init);
+
+static void __exit sprd_gpio_exit(void)
+{
+   platform_driver_unregister(_gpio_driver);
+}
+module_exit(sprd_gpio_exit);

with

module_platform_driver(sprd_gpio_driver);

> 
> -- 
> Baolin.wang
> Best Regards


Best regards
Marcus Folkesson


signature.asc
Description: PGP signature

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-02-01 Thread Marcus Folkesson 
On Thu, Feb 01, 2018 at 11:08:46AM +0800, Baolin Wang wrote:
> On 31 January 2018 at 22:23, Andy Shevchenko  
> wrote:
> > On Wed, Jan 31, 2018 at 4:01 AM, Baolin Wang  wrote:
> >> On 31 January 2018 at 00:48, Andy Shevchenko  
> >> wrote:
> >>> On Tue, Jan 30, 2018 at 2:07 PM, Baolin Wang  
> >>> wrote:
>  The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
>  each group contains 16 GPIOs. Each GPIO can set input/output and has
>  the interrupt capability.
> >>>
>  +config GPIO_SPRD
> >>>
>  +   bool "Spreadtrum GPIO support"
> >>>
> >>> Either you have to put tristate here, or remove all redundant
> >>> module_*() and MODULE_*() macros.
> >>
> >> I will remove module_*() and MODULE_*() macros in next version. Thanks
> >> for your comments.
> >
> > In that case you need to explain why driver can't be module. (And
> > don't forget to replace module.h with init.h).
> 
> After more investigation, I found most GPIO dependencies can be
> deferred to probe. So I will change the GPIO driver to module level
> and change bool to tristate in next version.

Then you could replace

+static int __init sprd_gpio_init(void)
+{
+   return platform_driver_register(_gpio_driver);
+}
+subsys_initcall(sprd_gpio_init);
+
+static void __exit sprd_gpio_exit(void)
+{
+   platform_driver_unregister(_gpio_driver);
+}
+module_exit(sprd_gpio_exit);

with

module_platform_driver(sprd_gpio_driver);

> 
> -- 
> Baolin.wang
> Best Regards


Best regards
Marcus Folkesson


signature.asc
Description: PGP signature

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-02-01 Thread Marcus Folkesson 
Hi Baolin,

On Tue, Jan 30, 2018 at 08:07:43PM +0800, Baolin Wang wrote:
> The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
> each group contains 16 GPIOs. Each GPIO can set input/output and has
> the interrupt capability.
> 
> Signed-off-by: Baolin Wang 
> ---
> diff --git a/drivers/gpio/gpio-sprd.c b/drivers/gpio/gpio-sprd.c
> new file mode 100644
> index 000..af59b9f
> --- /dev/null
> +++ b/drivers/gpio/gpio-sprd.c
> @@ -0,0 +1,301 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Spreadtrum Communications Inc.
> + * Copyright (c) 2018 Linaro Ltd.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* GPIO registers definition */
> +#define SPRD_GPIO_DATA   0x0
> +#define SPRD_GPIO_DMSK   0x4
> +#define SPRD_GPIO_DIR0x8
> +#define SPRD_GPIO_IS 0xc
> +#define SPRD_GPIO_IBE0x10
> +#define SPRD_GPIO_IEV0x14
> +#define SPRD_GPIO_IE 0x18
> +#define SPRD_GPIO_RIS0x1c
> +#define SPRD_GPIO_MIS0x20
> +#define SPRD_GPIO_IC 0x24
> +#define SPRD_GPIO_INEN   0x28
> +
> +/* We have 16 groups GPIOs and each group contain 16 GPIOs */
> +#define SPRD_GPIO_GROUP_NR   16
> +#define SPRD_GPIO_NR 256
> +#define SPRD_GPIO_GROUP_SIZE 0x80
> +#define SPRD_GPIO_GROUP_MASK GENMASK(15, 0)
> +#define SPRD_GPIO_BIT(x) ((x) & (SPRD_GPIO_GROUP_NR - 1))
> +
> +struct sprd_gpio {
> + struct gpio_chip chip;
> + void __iomem *base;
> + spinlock_t lock;
> + int irq;
> +};
> +
> +static inline void __iomem *sprd_gpio_group_base(struct sprd_gpio *sprd_gpio,
> +  unsigned int group)
> +{
> + return sprd_gpio->base + SPRD_GPIO_GROUP_SIZE * group;
> +}
> +
> +static void sprd_gpio_update(struct gpio_chip *chip, unsigned int offset,
> +  unsigned int reg, unsigned int val)
> +{
> + struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip);
> + void __iomem *base = sprd_gpio_group_base(sprd_gpio,
> +   offset / SPRD_GPIO_GROUP_NR);
> + u32 shift = SPRD_GPIO_BIT(offset);
> + unsigned long flags;
> + u32 orig, tmp;
> +
> + spin_lock_irqsave(_gpio->lock, flags);
> + orig = readl_relaxed(base + reg);
> +
> + tmp = (orig & ~BIT(shift)) | (val << shift);
> + writel_relaxed(tmp, base + reg);
> + spin_unlock_irqrestore(_gpio->lock, flags);
> +}
> +
> +static int sprd_gpio_read(struct gpio_chip *chip, unsigned int offset,
> +   unsigned int reg)
> +{
> + struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip);
> + void __iomem *base = sprd_gpio_group_base(sprd_gpio,
> +   offset / SPRD_GPIO_GROUP_NR);
> + u32 value = readl_relaxed(base + reg) & SPRD_GPIO_GROUP_MASK;
> + u32 shift = SPRD_GPIO_BIT(offset);
> +
> + return !!(value & BIT(shift));
> +}
> +
> +static int sprd_gpio_request(struct gpio_chip *chip, unsigned int offset)
> +{
> + sprd_gpio_update(chip, offset, SPRD_GPIO_DMSK, 1);
> + return 0;
> +}

Better to change the function to void since the return value is not
valueable.

> +
> +static void sprd_gpio_free(struct gpio_chip *chip, unsigned int offset)
> +{
> + sprd_gpio_update(chip, offset, SPRD_GPIO_DMSK, 0);
> +}
> +
> +static int sprd_gpio_direction_input(struct gpio_chip *chip,
> +  unsigned int offset)
> +{
> + sprd_gpio_update(chip, offset, SPRD_GPIO_DIR, 0);
> + sprd_gpio_update(chip, offset, SPRD_GPIO_INEN, 1);
> + return 0;
> +}

Same here


> +
> +static int sprd_gpio_direction_output(struct gpio_chip *chip,
> +   unsigned int offset, int value)
> +{
> + sprd_gpio_update(chip, offset, SPRD_GPIO_DIR, 1);
> + sprd_gpio_update(chip, offset, SPRD_GPIO_INEN, 0);
> + sprd_gpio_update(chip, offset, SPRD_GPIO_DATA, value);
> + return 0;
> +}

and here.


Thanks,
Marcus Folkesson





signature.asc
Description: PGP signature

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-02-01 Thread Marcus Folkesson 
Hi Baolin,

On Tue, Jan 30, 2018 at 08:07:43PM +0800, Baolin Wang wrote:
> The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
> each group contains 16 GPIOs. Each GPIO can set input/output and has
> the interrupt capability.
> 
> Signed-off-by: Baolin Wang 
> ---
> diff --git a/drivers/gpio/gpio-sprd.c b/drivers/gpio/gpio-sprd.c
> new file mode 100644
> index 000..af59b9f
> --- /dev/null
> +++ b/drivers/gpio/gpio-sprd.c
> @@ -0,0 +1,301 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Spreadtrum Communications Inc.
> + * Copyright (c) 2018 Linaro Ltd.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* GPIO registers definition */
> +#define SPRD_GPIO_DATA   0x0
> +#define SPRD_GPIO_DMSK   0x4
> +#define SPRD_GPIO_DIR0x8
> +#define SPRD_GPIO_IS 0xc
> +#define SPRD_GPIO_IBE0x10
> +#define SPRD_GPIO_IEV0x14
> +#define SPRD_GPIO_IE 0x18
> +#define SPRD_GPIO_RIS0x1c
> +#define SPRD_GPIO_MIS0x20
> +#define SPRD_GPIO_IC 0x24
> +#define SPRD_GPIO_INEN   0x28
> +
> +/* We have 16 groups GPIOs and each group contain 16 GPIOs */
> +#define SPRD_GPIO_GROUP_NR   16
> +#define SPRD_GPIO_NR 256
> +#define SPRD_GPIO_GROUP_SIZE 0x80
> +#define SPRD_GPIO_GROUP_MASK GENMASK(15, 0)
> +#define SPRD_GPIO_BIT(x) ((x) & (SPRD_GPIO_GROUP_NR - 1))
> +
> +struct sprd_gpio {
> + struct gpio_chip chip;
> + void __iomem *base;
> + spinlock_t lock;
> + int irq;
> +};
> +
> +static inline void __iomem *sprd_gpio_group_base(struct sprd_gpio *sprd_gpio,
> +  unsigned int group)
> +{
> + return sprd_gpio->base + SPRD_GPIO_GROUP_SIZE * group;
> +}
> +
> +static void sprd_gpio_update(struct gpio_chip *chip, unsigned int offset,
> +  unsigned int reg, unsigned int val)
> +{
> + struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip);
> + void __iomem *base = sprd_gpio_group_base(sprd_gpio,
> +   offset / SPRD_GPIO_GROUP_NR);
> + u32 shift = SPRD_GPIO_BIT(offset);
> + unsigned long flags;
> + u32 orig, tmp;
> +
> + spin_lock_irqsave(_gpio->lock, flags);
> + orig = readl_relaxed(base + reg);
> +
> + tmp = (orig & ~BIT(shift)) | (val << shift);
> + writel_relaxed(tmp, base + reg);
> + spin_unlock_irqrestore(_gpio->lock, flags);
> +}
> +
> +static int sprd_gpio_read(struct gpio_chip *chip, unsigned int offset,
> +   unsigned int reg)
> +{
> + struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip);
> + void __iomem *base = sprd_gpio_group_base(sprd_gpio,
> +   offset / SPRD_GPIO_GROUP_NR);
> + u32 value = readl_relaxed(base + reg) & SPRD_GPIO_GROUP_MASK;
> + u32 shift = SPRD_GPIO_BIT(offset);
> +
> + return !!(value & BIT(shift));
> +}
> +
> +static int sprd_gpio_request(struct gpio_chip *chip, unsigned int offset)
> +{
> + sprd_gpio_update(chip, offset, SPRD_GPIO_DMSK, 1);
> + return 0;
> +}

Better to change the function to void since the return value is not
valueable.

> +
> +static void sprd_gpio_free(struct gpio_chip *chip, unsigned int offset)
> +{
> + sprd_gpio_update(chip, offset, SPRD_GPIO_DMSK, 0);
> +}
> +
> +static int sprd_gpio_direction_input(struct gpio_chip *chip,
> +  unsigned int offset)
> +{
> + sprd_gpio_update(chip, offset, SPRD_GPIO_DIR, 0);
> + sprd_gpio_update(chip, offset, SPRD_GPIO_INEN, 1);
> + return 0;
> +}

Same here


> +
> +static int sprd_gpio_direction_output(struct gpio_chip *chip,
> +   unsigned int offset, int value)
> +{
> + sprd_gpio_update(chip, offset, SPRD_GPIO_DIR, 1);
> + sprd_gpio_update(chip, offset, SPRD_GPIO_INEN, 0);
> + sprd_gpio_update(chip, offset, SPRD_GPIO_DATA, value);
> + return 0;
> +}

and here.


Thanks,
Marcus Folkesson





signature.asc
Description: PGP signature

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-01-31 Thread Baolin Wang 
On 31 January 2018 at 22:23, Andy Shevchenko  wrote:
> On Wed, Jan 31, 2018 at 4:01 AM, Baolin Wang  wrote:
>> On 31 January 2018 at 00:48, Andy Shevchenko  
>> wrote:
>>> On Tue, Jan 30, 2018 at 2:07 PM, Baolin Wang  wrote:
 The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
 each group contains 16 GPIOs. Each GPIO can set input/output and has
 the interrupt capability.
>>>
 +config GPIO_SPRD
>>>
 +   bool "Spreadtrum GPIO support"
>>>
>>> Either you have to put tristate here, or remove all redundant
>>> module_*() and MODULE_*() macros.
>>
>> I will remove module_*() and MODULE_*() macros in next version. Thanks
>> for your comments.
>
> In that case you need to explain why driver can't be module. (And
> don't forget to replace module.h with init.h).

After more investigation, I found most GPIO dependencies can be
deferred to probe. So I will change the GPIO driver to module level
and change bool to tristate in next version.

-- 
Baolin.wang
Best Regards

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-01-31 Thread Baolin Wang 
On 31 January 2018 at 22:23, Andy Shevchenko  wrote:
> On Wed, Jan 31, 2018 at 4:01 AM, Baolin Wang  wrote:
>> On 31 January 2018 at 00:48, Andy Shevchenko  
>> wrote:
>>> On Tue, Jan 30, 2018 at 2:07 PM, Baolin Wang  wrote:
 The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
 each group contains 16 GPIOs. Each GPIO can set input/output and has
 the interrupt capability.
>>>
 +config GPIO_SPRD
>>>
 +   bool "Spreadtrum GPIO support"
>>>
>>> Either you have to put tristate here, or remove all redundant
>>> module_*() and MODULE_*() macros.
>>
>> I will remove module_*() and MODULE_*() macros in next version. Thanks
>> for your comments.
>
> In that case you need to explain why driver can't be module. (And
> don't forget to replace module.h with init.h).

After more investigation, I found most GPIO dependencies can be
deferred to probe. So I will change the GPIO driver to module level
and change bool to tristate in next version.

-- 
Baolin.wang
Best Regards

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-01-31 Thread Andy Shevchenko 
On Wed, Jan 31, 2018 at 4:01 AM, Baolin Wang  wrote:
> On 31 January 2018 at 00:48, Andy Shevchenko  
> wrote:
>> On Tue, Jan 30, 2018 at 2:07 PM, Baolin Wang  wrote:
>>> The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
>>> each group contains 16 GPIOs. Each GPIO can set input/output and has
>>> the interrupt capability.
>>
>>> +config GPIO_SPRD
>>
>>> +   bool "Spreadtrum GPIO support"
>>
>> Either you have to put tristate here, or remove all redundant
>> module_*() and MODULE_*() macros.
>
> I will remove module_*() and MODULE_*() macros in next version. Thanks
> for your comments.

In that case you need to explain why driver can't be module. (And
don't forget to replace module.h with init.h).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-01-31 Thread Andy Shevchenko 
On Wed, Jan 31, 2018 at 4:01 AM, Baolin Wang  wrote:
> On 31 January 2018 at 00:48, Andy Shevchenko  
> wrote:
>> On Tue, Jan 30, 2018 at 2:07 PM, Baolin Wang  wrote:
>>> The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
>>> each group contains 16 GPIOs. Each GPIO can set input/output and has
>>> the interrupt capability.
>>
>>> +config GPIO_SPRD
>>
>>> +   bool "Spreadtrum GPIO support"
>>
>> Either you have to put tristate here, or remove all redundant
>> module_*() and MODULE_*() macros.
>
> I will remove module_*() and MODULE_*() macros in next version. Thanks
> for your comments.

In that case you need to explain why driver can't be module. (And
don't forget to replace module.h with init.h).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-01-30 Thread Baolin Wang 
Hi Andy,

On 31 January 2018 at 00:48, Andy Shevchenko  wrote:
> On Tue, Jan 30, 2018 at 2:07 PM, Baolin Wang  wrote:
>> The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
>> each group contains 16 GPIOs. Each GPIO can set input/output and has
>> the interrupt capability.
>
>> +config GPIO_SPRD
>
>> +   bool "Spreadtrum GPIO support"
>
> Either you have to put tristate here, or remove all redundant
> module_*() and MODULE_*() macros.

I will remove module_*() and MODULE_*() macros in next version. Thanks
for your comments.

>
> Other than that looks fine to me, FWIW:
> Reviewed-by: Andy Shevchenko 
>

-- 
Baolin.wang
Best Regards

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-01-30 Thread Baolin Wang 
Hi Andy,

On 31 January 2018 at 00:48, Andy Shevchenko  wrote:
> On Tue, Jan 30, 2018 at 2:07 PM, Baolin Wang  wrote:
>> The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
>> each group contains 16 GPIOs. Each GPIO can set input/output and has
>> the interrupt capability.
>
>> +config GPIO_SPRD
>
>> +   bool "Spreadtrum GPIO support"
>
> Either you have to put tristate here, or remove all redundant
> module_*() and MODULE_*() macros.

I will remove module_*() and MODULE_*() macros in next version. Thanks
for your comments.

>
> Other than that looks fine to me, FWIW:
> Reviewed-by: Andy Shevchenko 
>

-- 
Baolin.wang
Best Regards

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-01-30 Thread Andy Shevchenko 
On Tue, Jan 30, 2018 at 2:07 PM, Baolin Wang  wrote:
> The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
> each group contains 16 GPIOs. Each GPIO can set input/output and has
> the interrupt capability.

> +config GPIO_SPRD

> +   bool "Spreadtrum GPIO support"

Either you have to put tristate here, or remove all redundant
module_*() and MODULE_*() macros.

Other than that looks fine to me, FWIW:
Reviewed-by: Andy Shevchenko 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-01-30 Thread Andy Shevchenko 
On Tue, Jan 30, 2018 at 2:07 PM, Baolin Wang  wrote:
> The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
> each group contains 16 GPIOs. Each GPIO can set input/output and has
> the interrupt capability.

> +config GPIO_SPRD

> +   bool "Spreadtrum GPIO support"

Either you have to put tristate here, or remove all redundant
module_*() and MODULE_*() macros.

Other than that looks fine to me, FWIW:
Reviewed-by: Andy Shevchenko 

-- 
With Best Regards,
Andy Shevchenko

[PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-01-30 Thread Baolin Wang 
The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
each group contains 16 GPIOs. Each GPIO can set input/output and has
the interrupt capability.

Signed-off-by: Baolin Wang 
---
 drivers/gpio/Kconfig |7 ++
 drivers/gpio/Makefile|1 +
 drivers/gpio/gpio-sprd.c |  301 ++
 3 files changed, 309 insertions(+)
 create mode 100644 drivers/gpio/gpio-sprd.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d6a8e85..3bece19 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -404,6 +404,13 @@ config GPIO_SPEAR_SPICS
help
  Say yes here to support ST SPEAr SPI Chip Select as GPIO device
 
+config GPIO_SPRD
+   bool "Spreadtrum GPIO support"
+   depends on ARCH_SPRD || COMPILE_TEST
+   select GPIOLIB_IRQCHIP
+   help
+ Say yes here to support Spreadtrum GPIO device.
+
 config GPIO_STA2X11
bool "STA2x11/ConneXt GPIO support"
depends on MFD_STA2X11
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4bc24fe..5b633a0 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -108,6 +108,7 @@ obj-$(CONFIG_GPIO_SCH)  += gpio-sch.o
 obj-$(CONFIG_GPIO_SCH311X) += gpio-sch311x.o
 obj-$(CONFIG_GPIO_SODAVILLE)   += gpio-sodaville.o
 obj-$(CONFIG_GPIO_SPEAR_SPICS) += gpio-spear-spics.o
+obj-$(CONFIG_GPIO_SPRD)+= gpio-sprd.o
 obj-$(CONFIG_GPIO_STA2X11) += gpio-sta2x11.o
 obj-$(CONFIG_GPIO_STMPE)   += gpio-stmpe.o
 obj-$(CONFIG_GPIO_STP_XWAY)+= gpio-stp-xway.o
diff --git a/drivers/gpio/gpio-sprd.c b/drivers/gpio/gpio-sprd.c
new file mode 100644
index 000..af59b9f
--- /dev/null
+++ b/drivers/gpio/gpio-sprd.c
@@ -0,0 +1,301 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Spreadtrum Communications Inc.
+ * Copyright (c) 2018 Linaro Ltd.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* GPIO registers definition */
+#define SPRD_GPIO_DATA 0x0
+#define SPRD_GPIO_DMSK 0x4
+#define SPRD_GPIO_DIR  0x8
+#define SPRD_GPIO_IS   0xc
+#define SPRD_GPIO_IBE  0x10
+#define SPRD_GPIO_IEV  0x14
+#define SPRD_GPIO_IE   0x18
+#define SPRD_GPIO_RIS  0x1c
+#define SPRD_GPIO_MIS  0x20
+#define SPRD_GPIO_IC   0x24
+#define SPRD_GPIO_INEN 0x28
+
+/* We have 16 groups GPIOs and each group contain 16 GPIOs */
+#define SPRD_GPIO_GROUP_NR 16
+#define SPRD_GPIO_NR   256
+#define SPRD_GPIO_GROUP_SIZE   0x80
+#define SPRD_GPIO_GROUP_MASK   GENMASK(15, 0)
+#define SPRD_GPIO_BIT(x)   ((x) & (SPRD_GPIO_GROUP_NR - 1))
+
+struct sprd_gpio {
+   struct gpio_chip chip;
+   void __iomem *base;
+   spinlock_t lock;
+   int irq;
+};
+
+static inline void __iomem *sprd_gpio_group_base(struct sprd_gpio *sprd_gpio,
+unsigned int group)
+{
+   return sprd_gpio->base + SPRD_GPIO_GROUP_SIZE * group;
+}
+
+static void sprd_gpio_update(struct gpio_chip *chip, unsigned int offset,
+unsigned int reg, unsigned int val)
+{
+   struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip);
+   void __iomem *base = sprd_gpio_group_base(sprd_gpio,
+ offset / SPRD_GPIO_GROUP_NR);
+   u32 shift = SPRD_GPIO_BIT(offset);
+   unsigned long flags;
+   u32 orig, tmp;
+
+   spin_lock_irqsave(_gpio->lock, flags);
+   orig = readl_relaxed(base + reg);
+
+   tmp = (orig & ~BIT(shift)) | (val << shift);
+   writel_relaxed(tmp, base + reg);
+   spin_unlock_irqrestore(_gpio->lock, flags);
+}
+
+static int sprd_gpio_read(struct gpio_chip *chip, unsigned int offset,
+ unsigned int reg)
+{
+   struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip);
+   void __iomem *base = sprd_gpio_group_base(sprd_gpio,
+ offset / SPRD_GPIO_GROUP_NR);
+   u32 value = readl_relaxed(base + reg) & SPRD_GPIO_GROUP_MASK;
+   u32 shift = SPRD_GPIO_BIT(offset);
+
+   return !!(value & BIT(shift));
+}
+
+static int sprd_gpio_request(struct gpio_chip *chip, unsigned int offset)
+{
+   sprd_gpio_update(chip, offset, SPRD_GPIO_DMSK, 1);
+   return 0;
+}
+
+static void sprd_gpio_free(struct gpio_chip *chip, unsigned int offset)
+{
+   sprd_gpio_update(chip, offset, SPRD_GPIO_DMSK, 0);
+}
+
+static int sprd_gpio_direction_input(struct gpio_chip *chip,
+unsigned int offset)
+{
+   sprd_gpio_update(chip, offset, SPRD_GPIO_DIR, 0);
+   sprd_gpio_update(chip, offset, SPRD_GPIO_INEN, 1);
+   return 0;
+}
+
+static int sprd_gpio_direction_output(struct gpio_chip *chip,
+ unsigned int offset, int value)
+{
+   sprd_gpio_update(chip, offset, SPRD_GPIO_DIR, 1);
+

[PATCH 2/2] gpio: Add GPIO driver for Spreadtrum SC9860 platform

2018-01-30 Thread Baolin Wang 
The Spreadtrum SC9860 platform GPIO controller contains 16 groups and
each group contains 16 GPIOs. Each GPIO can set input/output and has
the interrupt capability.

Signed-off-by: Baolin Wang 
---
 drivers/gpio/Kconfig |7 ++
 drivers/gpio/Makefile|1 +
 drivers/gpio/gpio-sprd.c |  301 ++
 3 files changed, 309 insertions(+)
 create mode 100644 drivers/gpio/gpio-sprd.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d6a8e85..3bece19 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -404,6 +404,13 @@ config GPIO_SPEAR_SPICS
help
  Say yes here to support ST SPEAr SPI Chip Select as GPIO device
 
+config GPIO_SPRD
+   bool "Spreadtrum GPIO support"
+   depends on ARCH_SPRD || COMPILE_TEST
+   select GPIOLIB_IRQCHIP
+   help
+ Say yes here to support Spreadtrum GPIO device.
+
 config GPIO_STA2X11
bool "STA2x11/ConneXt GPIO support"
depends on MFD_STA2X11
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4bc24fe..5b633a0 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -108,6 +108,7 @@ obj-$(CONFIG_GPIO_SCH)  += gpio-sch.o
 obj-$(CONFIG_GPIO_SCH311X) += gpio-sch311x.o
 obj-$(CONFIG_GPIO_SODAVILLE)   += gpio-sodaville.o
 obj-$(CONFIG_GPIO_SPEAR_SPICS) += gpio-spear-spics.o
+obj-$(CONFIG_GPIO_SPRD)+= gpio-sprd.o
 obj-$(CONFIG_GPIO_STA2X11) += gpio-sta2x11.o
 obj-$(CONFIG_GPIO_STMPE)   += gpio-stmpe.o
 obj-$(CONFIG_GPIO_STP_XWAY)+= gpio-stp-xway.o
diff --git a/drivers/gpio/gpio-sprd.c b/drivers/gpio/gpio-sprd.c
new file mode 100644
index 000..af59b9f
--- /dev/null
+++ b/drivers/gpio/gpio-sprd.c
@@ -0,0 +1,301 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Spreadtrum Communications Inc.
+ * Copyright (c) 2018 Linaro Ltd.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* GPIO registers definition */
+#define SPRD_GPIO_DATA 0x0
+#define SPRD_GPIO_DMSK 0x4
+#define SPRD_GPIO_DIR  0x8
+#define SPRD_GPIO_IS   0xc
+#define SPRD_GPIO_IBE  0x10
+#define SPRD_GPIO_IEV  0x14
+#define SPRD_GPIO_IE   0x18
+#define SPRD_GPIO_RIS  0x1c
+#define SPRD_GPIO_MIS  0x20
+#define SPRD_GPIO_IC   0x24
+#define SPRD_GPIO_INEN 0x28
+
+/* We have 16 groups GPIOs and each group contain 16 GPIOs */
+#define SPRD_GPIO_GROUP_NR 16
+#define SPRD_GPIO_NR   256
+#define SPRD_GPIO_GROUP_SIZE   0x80
+#define SPRD_GPIO_GROUP_MASK   GENMASK(15, 0)
+#define SPRD_GPIO_BIT(x)   ((x) & (SPRD_GPIO_GROUP_NR - 1))
+
+struct sprd_gpio {
+   struct gpio_chip chip;
+   void __iomem *base;
+   spinlock_t lock;
+   int irq;
+};
+
+static inline void __iomem *sprd_gpio_group_base(struct sprd_gpio *sprd_gpio,
+unsigned int group)
+{
+   return sprd_gpio->base + SPRD_GPIO_GROUP_SIZE * group;
+}
+
+static void sprd_gpio_update(struct gpio_chip *chip, unsigned int offset,
+unsigned int reg, unsigned int val)
+{
+   struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip);
+   void __iomem *base = sprd_gpio_group_base(sprd_gpio,
+ offset / SPRD_GPIO_GROUP_NR);
+   u32 shift = SPRD_GPIO_BIT(offset);
+   unsigned long flags;
+   u32 orig, tmp;
+
+   spin_lock_irqsave(_gpio->lock, flags);
+   orig = readl_relaxed(base + reg);
+
+   tmp = (orig & ~BIT(shift)) | (val << shift);
+   writel_relaxed(tmp, base + reg);
+   spin_unlock_irqrestore(_gpio->lock, flags);
+}
+
+static int sprd_gpio_read(struct gpio_chip *chip, unsigned int offset,
+ unsigned int reg)
+{
+   struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip);
+   void __iomem *base = sprd_gpio_group_base(sprd_gpio,
+ offset / SPRD_GPIO_GROUP_NR);
+   u32 value = readl_relaxed(base + reg) & SPRD_GPIO_GROUP_MASK;
+   u32 shift = SPRD_GPIO_BIT(offset);
+
+   return !!(value & BIT(shift));
+}
+
+static int sprd_gpio_request(struct gpio_chip *chip, unsigned int offset)
+{
+   sprd_gpio_update(chip, offset, SPRD_GPIO_DMSK, 1);
+   return 0;
+}
+
+static void sprd_gpio_free(struct gpio_chip *chip, unsigned int offset)
+{
+   sprd_gpio_update(chip, offset, SPRD_GPIO_DMSK, 0);
+}
+
+static int sprd_gpio_direction_input(struct gpio_chip *chip,
+unsigned int offset)
+{
+   sprd_gpio_update(chip, offset, SPRD_GPIO_DIR, 0);
+   sprd_gpio_update(chip, offset, SPRD_GPIO_INEN, 1);
+   return 0;
+}
+
+static int sprd_gpio_direction_output(struct gpio_chip *chip,
+ unsigned int offset, int value)
+{
+   sprd_gpio_update(chip, offset, SPRD_GPIO_DIR, 1);
+   sprd_gpio_update(chip,