Re: [PATCH] gpio: driver for Xtensa GPIO32

[Baruch Siach <baruch@tkos.co.il>]

Hi Linus,

On Wed, Dec 11, 2013 at 01:32:47PM +0100, Linus Walleij wrote:
> On Thu, Dec 5, 2013 at 6:43 AM, Baruch Siach <baruch@tkos.co.il> wrote:
> > We need to preserve the irq flags. Let me know if the following is OK:
> >
> > static inline unsigned long enable_cp(void)
> > {
> >         unsigned long flags, cpenable;
> >
> >         local_irq_save(flags);
> >         RSR_CPENABLE(cpenable);
> >         WSR_CPENABLE(cpenable | XCHAL_CP_PORT_MASK);
> >
> >         return flags;
> > }
> >
> > static inline void disable_cp(unsigned long flags)
> > {
> >         WSR_CPENABLE(cpenable);
> >         local_irq_restore(flags);
> > }
> 
> Looks OK.

Done in v2.

> >> > +static void xtensa_impwire_set_value(struct gpio_chip *gc, unsigned offset,
> >> > +                                   int value)
> >> > +{
> >> > +}
> >>
> >> This is a rather nasty implementation for something that will always
> >> fail. What about putting an dev_err(gc->dev, "..."); in that set_value?
> >
> > This is an input only device, so this callback should never run. Ideally it
> > would be nice to just have it NULL, but the core gpiolib doesn't support that.
> > Are you sure bloating the kernel with an error message that never displays is
> > worth it?
> 
> Yes I am sure. If you think a few lines of error message is too much,
> you can add a BUG(); here instead.

Will do in v3.

> >> Why not device_initcall()? (i.e. same as module_init())
> >> Do you need it very early?
> >
> > The main use case for this driver is for intra-core signaling, so it might be
> > needed quite early I guess. I don't need any specific need for it to be
> > available early at the moment, so I can change it device_initcall() if you
> > prefer that.
> 
> Please do that for now, thanks. We shouldn't stick strange stuff
> in because "may need later", as this can lead to weird stuff that people
> don't dare to change. (Cargo-cult phenomenon.)

Done in v2.

Thanks,
baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -