Re: [PATCH v1 1/2] gpio: intel-mid: Remove potentially harmful code

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Fri, 2016-07-08 at 11:20 +0300, Mika Westerberg wrote:
> On Wed, Jul 06, 2016 at 12:50:12PM +0300, Andy Shevchenko wrote:
> > The commit d56d6b3d7d69 ("gpio: langwell: add Intel Merrifield
> > support")
> > doesn't look at all as a proper support for Intel Merrifield and I
> > dare to say
> > that it distorts the behaviour of the hardware.
> > 
> > The register map is different on Intel Merrifield, i.e. only 6 out
> > of 8
> > register have the same purpose but none of them has same location in
> > the
> > address space. The current case potentially harmful to existing
> > hardware since
> > it's poking registers on wrong offsets and may set some pin to be
> > GPIO output
> > when connected hardware doesn't expect such.
> > 
> > Besides the above GPIO and pinctrl on Intel Merrifield have been
> > located in
> > different IP blocks. The functionality has been extended as well,
> > i.e. added
> > support of level interrupts, special registers for wake capable
> > sources and
> > thus, in my opinion, requires a completele separate driver.
> > 
> > 

> If it is truely poking wrong registers, this is certainly right thing
> to
> do.

Yes, you might check it yourself, GPIO registers starts with offset 4,
instead of 0, and there are couple of w/o registers in the area to set
pin state. It means potentially you may find the case where suddenly one
pin not that one user asked for) might change direction followed by
voltage.

> 
> Reviewed-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> 
> I take it we are getting a driver which is using the correct registers
> soon ;-)

It should be in your mailbox already.


-- 

Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html