Hi Linus,
sorry for the noise..
On 19-10-09 11:56, Marco Felsch wrote:
> Hi Linus,
>
> On 19-10-07 10:51, Marco Felsch wrote:
> > Hi Linus,
> >
> > thanks for you feedback.
> >
> > On 19-10-04 21:27, Linus Walleij wrote:
> > > On Tue, Sep 17, 2019 at 12:59 PM Marco Felsch <m.felsch@xxxxxxxxxxxxxx> wrote:
> > >
> > > > +static int da9062_gpio_direction_input(struct gpio_chip *gc,
> > > > + unsigned int offset)
> > > > +{
> > > > + struct da9062_gpio *gpio = gpiochip_get_data(gc);
> > > > + struct regmap *regmap = gpio->da9062->regmap;
> > > > + struct gpio_desc *desc = gpiochip_get_desc(gc, offset);
>
> This won't work anymore since I moved the driver to pinctrl and can't
> include the drivers/gpio/gpiolib.h anymore. What is the right way to
> get the same result within the pinctrl space? There are three possible
> ways:
> 1) Revert commit 1bd6b601fe196b6fbce2c93536ce0f3f53577cec which isn't
> the best due to safeness.
> 2) Set the gpio as active low hard as the other da90*-gpio drivers did
> 3) Introduce a dt-binding (seems wrong because the information is
> already there).
> 4) "Re-implement" the gpiochip_get_desc() functionality driver
> internally.
4) won't work didn't recognize that 'struct gpio_device' is an internal
struct.
Regards,
Marco
> Thanks for your advice.
>
> Regards,
> Marco
>
> > > > + unsigned int gpi_type;
> > > > + int ret;
> > > > +
> > > > + ret = da9062_gpio_set_pin_mode(regmap, offset, DA9062_PIN_GPI);
> > > > + if (ret)
> > > > + return ret;
> > >
> > > Fair enough.
> > >
> > > > + /*
> > > > + * If the gpio is active low we should set it in hw too. No worries
> > > > + * about gpio_get() because we read and return the gpio-level. So the
> > > > + * gpiolob active_low handling is still correct.
> > >
> > > gpiolib?
> >
> > Thanks for covering that.
> >
> > > > + *
> > > > + * 0 - active low, 1 - active high
> > > > + */
> > > > + gpi_type = !gpiod_is_active_low(desc);
> > > > + return regmap_update_bits(regmap, DA9062AA_GPIO_0_1 + (offset >> 1),
> > > > + DA9062AA_GPIO0_TYPE_MASK << DA9062_TYPE(offset),
> > > > + gpi_type << DA9062_TYPE(offset));
> > > > +}
> > >
> > > So this does not affect the value out set by da9062_gpio_set()?
> >
> > Please check [1] table 54, the datasheet says it is only gpi
> > (gpio-input). So I assume it doesn't affect out values.
> >
> > [1] https://www.dialog-semiconductor.com/sites/default/files/da9062-a_datasheet_2v3.pdf
> >
> > Unfortunately the other gpio-da90* drivers sets this as active low hard
> > within the driver. I wanted to avoid this here since it isn't always
> > true.
> >
> > > What is the electrical effect of this then, really? To me that seems like
> > > something that is mostly going to be related to how interrupts
> > > trigger (like whether to trig on rising or falling edge) and then it
> > > should really be in the .set_type() callback, should it not?
> >
> > Not only interrupts.. The dialog pmics has a lot of options to use this
> > pins e.g. you can set it as voltage-selection input. You saw the patches
> > I made for the regulator :)
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
