Re: [PATCH v6 2/3] gpio: add a reusable generic gpio_chip using regmap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 2020-05-28 17:55, schrieb Pierre-Louis Bossart:
+    /* if we have a direction register we need both input and
output */
+    if ((config->reg_dir_out_base || config->reg_dir_in_base) &&
+        (!config->reg_dat_base || !config->reg_set_base))
+        return ERR_PTR(-EINVAL);
 This failed for me since I didn't have the 'dat' base assigned. I
still can't figure out what 'dat' stands for...I just assigned it to
the same offset as the 'set' base but really don't understand what
this is supposed to do.

DAT is the data register, aka input register, if the GPIO is in input
mode.

If I read the datasheet correctly you should use the following:

PCM512x_GPIO_EN should be reg_dir_out_base
PCM512x_GPIO_CONTROL_1 should be reg_set_base
PCM512x_GPIN should be reg_dat_base

no custom xlate necessary. GPIN looks a bit fishy in that datasheet:
 http://www.ti.com/lit/ds/symlink/pcm5121.pdf?ts=1590684141147

PCM512x_GPIO_OUTPUT_1..6 is pinmux control and shouldn't be part of
the gpio-regmap. Your driver needs to take care of that.

+
+    /* we don't support having both registers simultaneously for
now */
+    if (config->reg_dir_out_base && config->reg_dir_in_base)
+        return ERR_PTR(-EINVAL);

and this second test seems to contradict the notion of 'both input and
output' above?

dir_out_base is used if the register is high active to select an output.
dir_in_base is used for a low active register. Thus both bases are used
to switch a GPIO between input and output.

re-adding comment from previous series:
 >> I still have a series of odd warnings I didn't have before: >> >>
[  101.400263] WARNING: CPU: 3 PID: 1129 at >>
drivers/gpio/gpiolib.c:4084 gpiod_set_value+0x3f/0x50 >> >> This seems
to come from >>     /* Should be using gpiod_set_value_cansleep() */
    WARN_ON(desc->gdev->chip->can_sleep); > > Right now,
gpio-regmap hardcodes can_sleep to true. But the only regmap > which
don't sleep is regmap-mmio. The PCM512x seems to be either I2C or >
SPI, which can both sleep. So this warning is actually correct and >
wherever this gpio is set should do it by calling the _cansleep() >
version.

I still have the warnings with this version, not sure if you wanted to
fix it in the v6 or is this needs to be fixed in another piece of
code/patch. How would we go about removing this warning?

There is no fix in gpio-regmap. wherever this GPIO is connected to must
not call gpiod_set_value() but have to use gpiod_set_value_cansleep().

-michael



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux