lkml.org 
[lkml]   [2019]   [Dec]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 5/5] gpio: syscon: Add support for the Xylon LogiCVC GPIOs
Hi Linus,

On Fri 29 Nov 19, 10:24, Linus Walleij wrote:
> Hi Paul,
>
> thanks for your patch!
>
> On Thu, Nov 28, 2019 at 4:54 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
>
> > The LogiCVC display hardware block comes with GPIO capabilities
> > that must be exposed separately from the main driver (as GPIOs) for
> > use with regulators and panels. A syscon is used to share the same
> > regmap across the two drivers.
> >
> > Since the GPIO capabilities are pretty simple, add them to the syscon
> > GPIO driver.
> >
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> (...)
> > +#define LOGICVC_CTRL_REG 0x40
> > +#define LOGICVC_CTRL_GPIO_SHIFT 11
> > +#define LOGICVC_CTRL_GPIO_BITS 5
> > +
> > +#define LOGICVC_POWER_CTRL_REG 0x78
> > +#define LOGICVC_POWER_CTRL_GPIO_SHIFT 0
> > +#define LOGICVC_POWER_CTRL_GPIO_BITS 4
> > +
> > +static void logicvc_gpio_offset(struct syscon_gpio_priv *priv,
> > + unsigned offset, unsigned int *reg,
> > + unsigned int *bit)
> > +{
> > + if (offset >= LOGICVC_CTRL_GPIO_BITS) {
> > + *reg = LOGICVC_POWER_CTRL_REG;
> > +
> > + /* To the (virtual) power ctrl offset. */
> > + offset -= LOGICVC_CTRL_GPIO_BITS;
> > + /* To the actual bit offset in reg. */
> > + offset += LOGICVC_POWER_CTRL_GPIO_SHIFT;
> > + } else {
> > + *reg = LOGICVC_CTRL_REG;
> > +
> > + /* To the actual bit offset in reg. */
> > + offset += LOGICVC_CTRL_GPIO_SHIFT;
> > + }
> > +
> > + *bit = BIT(offset);
> > +}
>
> The gpio-syscon.c is for simple syscons where the lines
> you want to affect are nicely ordered in the registers.
> It is intended to be generic.
>
> This is kind of shoehorning a special case into the generic
> code.
>
> Isn't it more appropriate to create a specific driver for this
> hardware?

Yes I'm fine with that too. Indeed the driver has custom set/get operations
that don't really fit well into generic code.

> Special get/set quirks for any possible quirky offset is
> certainly not the way to go, if this should be supported
> we need generic properties in struct syscon_gpio_data
> to indicate the valid bits and offsets.

I guess the rationale would be to define multiple possible bit offsets for
different ranges of GPIO offsets, but I don't think it would be very useful
outside of this case.

I'll probably craft a new version with a dedicated driver then.

Cheers,

Paul

--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2019-12-03 09:59    [W:0.041 / U:1.968 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site