[PATCHv4,1/3] gpio: sch: Consolidate similar algorithms

Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.

Signed-off-by: Chang Rebecca Swee Fun <rebecca.swee.fun.chang@intel.com>
---
 drivers/gpio/gpio-sch.c |   81 ++++++++++++++++-------------------------------
 1 file changed, 28 insertions(+), 53 deletions(-)

On Wed, Nov 26, 2014 at 12:47:58PM +0800, Chang Rebecca Swee Fun wrote:
> Consolidating similar algorithms into common functions to make
> GPIO SCH simpler and manageable.
>
> Signed-off-by: Chang Rebecca Swee Fun <rebecca.swee.fun.chang@intel.com>
> ---
>  drivers/gpio/gpio-sch.c |   81 ++++++++++++++++-------------------------------
>  1 file changed, 28 insertions(+), 53 deletions(-)

Looks good to me,

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

2014-11-26 5:47 GMT+01:00 Chang Rebecca Swee Fun
<rebecca.swee.fun.chang@intel.com>:

> Consolidating similar algorithms into common functions to make
> GPIO SCH simpler and manageable.
>
> Signed-off-by: Chang Rebecca Swee Fun <rebecca.swee.fun.chang@intel.com>

Patch already applied. If you changed anything, send incremental
patches on top of my devel branch or linux-next or something...

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Fri, Nov 28, 2014 at 2:54 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> 2014-11-26 5:47 GMT+01:00 Chang Rebecca Swee Fun
> <rebecca.swee.fun.chang@intel.com>:
>
>> Consolidating similar algorithms into common functions to make
>> GPIO SCH simpler and manageable.
>>
>> Signed-off-by: Chang Rebecca Swee Fun <rebecca.swee.fun.chang@intel.com>
>
> Patch already applied. If you changed anything, send incremental
> patches on top of my devel branch or linux-next or something...

Bah it was *NOT* already applied. But it doesn't apply so please rebase
the patch set to my "devel" branch in the GPIO tree and resend.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

2014-11-26 13:47 GMT+09:00 Chang Rebecca Swee Fun
<rebecca.swee.fun.chang@intel.com>:
> Consolidating similar algorithms into common functions to make
> GPIO SCH simpler and manageable.
>
> Signed-off-by: Chang Rebecca Swee Fun <rebecca.swee.fun.chang@intel.com>
> ---
>  drivers/gpio/gpio-sch.c |   81 ++++++++++++++++-------------------------------
>  1 file changed, 28 insertions(+), 53 deletions(-)

Your v3 had this:

 drivers/gpio/gpio-sch.c |   95 ++++++++++++++++++++++++++---------------------
 1 file changed, 53 insertions(+), 42 deletions(-)

Just looking at these two lines we can already tell this v4 is going
to be *much* better. :)

>
> diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
> index 99720c8..7967f14 100644
> --- a/drivers/gpio/gpio-sch.c
> +++ b/drivers/gpio/gpio-sch.c
> @@ -63,75 +63,59 @@ static unsigned sch_gpio_bit(struct sch_gpio *sch, unsigned gpio)
>         return gpio % 8;
>  }
>
> -static void sch_gpio_enable(struct sch_gpio *sch, unsigned gpio)
> +static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
>  {
> +       struct sch_gpio *sch = to_sch_gpio(gc);
>         unsigned short offset, bit;
> -       u8 enable;
> -
> -       spin_lock(&sch->lock);
> +       u8 curr_dirs;

This name sounds weird in this function. It has been copied from the
direction setting functions, but this function is much more generic.
So maybe come with a more generic name as well?

>
> -       offset = sch_gpio_offset(sch, gpio, GEN);
> +       offset = sch_gpio_offset(sch, gpio, reg);
>         bit = sch_gpio_bit(sch, gpio);
>
> -       enable = inb(sch->iobase + offset);
> -       if (!(enable & (1 << bit)))
> -               outb(enable | (1 << bit), sch->iobase + offset);
> +       curr_dirs = !!(inb(sch->iobase + offset) & BIT(bit));
>
> -       spin_unlock(&sch->lock);
> +       return curr_dirs;
>  }
>
> -static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned  gpio_num)
> +static void sch_gpio_reg_set(struct gpio_chip *gc, unsigned gpio, unsigned reg,
> +                            int val)
>  {
>         struct sch_gpio *sch = to_sch_gpio(gc);
> -       u8 curr_dirs;
>         unsigned short offset, bit;
> +       u8 curr_dirs;

Same here.

Meh, I guess that's my only complain... This really makes the driver
easier to understand, so after fixing this minor issue and rebasing on
Linus' devel branch, feel free to add my

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html