RE: [PATCH 1/3] gpio: sch: Consolidate similar algorithms

From: Chang, Rebecca Swee Fun
Date: Thu Sep 25 2014 - 21:36:58 EST




> -----Original Message-----
> From: 'Mika Westerberg' [mailto:mika.westerberg@xxxxxxxxxxxxxxx]
> Sent: 24 September, 2014 5:51 PM
> To: Chang, Rebecca Swee Fun
> Cc: Linus Walleij; linux-gpio@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/3] gpio: sch: Consolidate similar algorithms
>
> On Wed, Sep 24, 2014 at 12:55:07AM +0000, Chang, Rebecca Swee Fun wrote:
> > > > The register values are required when it comes to IRQ handling. By
> > > > passing in the registers values, we can make full use of the
> > > > algorithms without introducing extra/similar algorithms to compute
> > > > other register offset values.
> > > > For example, we have other offset values to handle such as:-
> > > > GTPE 0x0C
> > > > GTNE 0x10
> > > > GGPE 0x14
> > > > GSMI 0x18
> > > > GTS 0x1C
> > > > CGNMIEN 0x40
> > > > RGNMIEN 0x44
> > >
> > > Well, can we at least call it something else than sch_gpio_enable()?
> > > Perhaps sch_gpio_set_value() or so?
> >
> > sch_gpio_set_value() sounds good. After think twice, I intend to merge
> > sch_gpio_enable() and sch_gpio_disable() into one functions. Using
> > variable "enable" as an indicator, I can control whether to enable or
> > disable when calling the function. Here is my draft:
>
> Actually sch_gpio_set_value() is too close to sch_gpio_set() which sets the GPIO
> to 1 or 0. How about sch_gpio_register_set() or something along those lines?
>
> And I don't think it is good idea to add yet another functionality, like enable
> there. Please leave sch_gpio_enable()/sch_gpio_disable() as is.

Alright, I will change the sch_gpio_enable()/sch_gpio_disable() into sch_gpio_register_set()/sch_gpio_register_clear().
Thanks.

Rebecca
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/