| Message ID | 20170917093906.16325-1-linus.walleij@linaro.org |
|---|---|
| Headers | show |
| Series | I2C GPIO to use gpiolibs open drain | expand |
Hi Linus, On Sun, Sep 17, 2017 at 11:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > By creating local variables for *dev and *np, the code become > much easier to read, in my opinion. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > I put this at the end of the series because compared to the > rest of the patches it is completely unimportant. > --- > drivers/i2c/busses/i2c-gpio.c | 26 ++++++++++++++------------ > 1 file changed, 14 insertions(+), 12 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c > index 97b9c29e9429..beb5ce523684 100644 > --- a/drivers/i2c/busses/i2c-gpio.c > +++ b/drivers/i2c/busses/i2c-gpio.c > @@ -99,15 +101,15 @@ static int i2c_gpio_probe(struct platform_device *pdev) > bit_data = &priv->bit_data; > pdata = &priv->pdata; > > - if (pdev->dev.of_node) { > - of_i2c_gpio_get_props(pdev->dev.of_node, pdata); > + if (np) { > + of_i2c_gpio_get_props(np, pdata); > } else { > /* > * If all platform data settings are zero it is OK > * to not provide any platform data from the board. > */ > - if (dev_get_platdata(&pdev->dev)) > - memcpy(pdata, dev_get_platdata(&pdev->dev), > + if (dev_get_platdata(dev)) > + memcpy(pdata, dev_get_platdata(dev), > sizeof(*pdata)); This fits on one line again (you have to do something to offset the LoC increase 14 insertions(+), 12 deletions(-) ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Sun, Sep 17, 2017 at 11:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > This adds support for using the "sda" and "scl" GPIOs in > device tree instead of anonymously using index 0 and 1 of > the "gpios" property. > > We add a helper function to retrieve the GPIO descriptors > and some explicit error handling since the probe may have > to be deferred. At least this happened to me when moving > to using named "sda" and "scl" lines (all of a sudden this > started to probe before the GPIO driver) so we need to > gracefully defer probe when we ge -ENOENT in the error get > pointer. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Linus, On Sun, Sep 17, 2017 at 11:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > This adds support for using the "sda" and "scl" GPIOs in > device tree instead of anonymously using index 0 and 1 of > the "gpios" property. > > We add a helper function to retrieve the GPIO descriptors > and some explicit error handling since the probe may have > to be deferred. At least this happened to me when moving > to using named "sda" and "scl" lines (all of a sudden this > started to probe before the GPIO driver) so we need to > gracefully defer probe when we ge -ENOENT in the error > pointer. > > Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > This is pretty much a rewrite of Geerts patch on top of > my own changes to support descriptors. > --- > drivers/i2c/busses/i2c-gpio.c | 59 +++++++++++++++++++++++++++++++------------ > 1 file changed, 43 insertions(+), 16 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c > index beb5ce523684..2738b851f470 100644 > --- a/drivers/i2c/busses/i2c-gpio.c > +++ b/drivers/i2c/busses/i2c-gpio.c > @@ -82,6 +82,42 @@ static void of_i2c_gpio_get_props(struct device_node *np, > of_property_read_bool(np, "i2c-gpio,scl-output-only"); > } > > +static struct gpio_desc *i2c_gpio_get_desc(struct device *dev, > + const char *con_id, > + unsigned int index, > + enum gpiod_flags gflags) > +{ > + struct gpio_desc *retdesc; > + int ret; [...] > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "error trying to get descriptor: %ld\n", ret); warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'int' [-Wformat=] %d (0day busy?) > + > + return retdesc; > +} Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Linus, On Sun, Sep 17, 2017 at 11:38 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > This augments the I2C GPIO driver to use open drain emulation > or hardware support for open drain from the GPIO driver. > > This version layers Geert Uytterhoeven's idea to use explicit > sda-gpios and scl-gpios for the GPIO lines, and strongly > encourage the (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN) flags to be > used in all device trees. > > We have collected ACKs from the ARM SoC maintainers and the > MFD maintainer and are looking for testers to try this out. > > Geert Uytterhoeven (1): > dt-bindings: i2c: i2c-gpio: Add support for named gpios > > Linus Walleij (6): > i2c: gpio: Convert to use descriptors > gpio: Make it possible for consumers to enforce open drain > i2c: gpio: Enforce open drain through gpiolib > i2c: gpio: Augment all boardfiles to use open drain > i2c: gpio: Local vars in probe > i2c: gpio: Add support for named gpios in DT Thanks for doing this, and picking up my patch. I gave this a try on r8a7740/armadillo800eva. Without DT changes, the GPIO i2c bus still works fine, but a warning is printed, as expected: gpio-208 (sda): enforced open drain please flag it properly in DT/ACPI DSDT/board file gpio-91 (scl): enforced open drain please flag it properly in DT/ACPI DSDT/board file After - sda-gpios = <&pfc 208 GPIO_ACTIVE_HIGH>; - scl-gpios = <&pfc 91 GPIO_ACTIVE_HIGH>; + sda-gpios = <&pfc 208 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; + scl-gpios = <&pfc 91 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; the warning is gone, and the GPIO i2c bus still works. Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Mon, Sep 18, 2017 at 11:58 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: >> + if (ret != -EPROBE_DEFER) >> + dev_err(dev, "error trying to get descriptor: %ld\n", ret); > > warning: format '%ld' expects argument of type 'long int', but > argument 3 has type 'int' [-Wformat=] > > %d (0day busy?) Bah haven't pushed it to the 0day builders yet. I'll do that tonight so I can drown in the build errors. :-D Fixing it. Yours, Linus Walleij
On Sun, Sep 17, 2017 at 11:39:05AM +0200, Linus Walleij wrote: > From: Geert Uytterhoeven <geert+renesas () glider ! be> > > The current i2c-gpio DT bindings use a single unnamed "gpios" property > to refer to the SDA and SCL signal lines by index. This is error-prone > for the casual DT writer and reviewer, as one has to look up the order > in the DT bindings. > > Fix this by amending the DT bindings to use two separate named gpios > properties, and deprecate the old unnamed variant. > > Take this opportunity to clearly deprecate the "i2c-gpio,sda-open-drain" > and "i2c-gpio,scl-open-drain" flags as well. The commit describes > in detail what these flags actually mean, and why they should not be > used in new device trees. > > Cc: devicetree@vger.kernel.org > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > [Augmented to what I and Rob would like] > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > ChangeLog v1->v2: > - Create a special section for the deprecated bindings > - Also deprecate the open drain bool properties > - Update the example to use the new style of bindings > --- > Documentation/devicetree/bindings/i2c/i2c-gpio.txt | 32 ++++++++++++++++------ > 1 file changed, 23 insertions(+), 9 deletions(-) Acked-by: Rob Herring <robh@kernel.org>
Linus Walleij <linus.walleij@linaro.org> writes: > We now handle the open drain mode internally in the I2C GPIO > driver, but we will get warnings from the gpiolib that we > override the default mode of the line so it becomes open > drain. > > We can fix all in-kernel users by simply passing the right > flag along in the descriptor table, and we already touched > all of these files in the series so let's just tidy it up. > > Cc: Steven Miao <realmz6@gmail.com> > Cc: Ralf Baechle <ralf@linux-mips.org> > Acked-by: Olof Johansson <olof@lixom.net> > Acked-by: Lee Jones <lee.jones@linaro.org> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > ChangeLog v1->v2: > - Collected ACKs. > > Steven (Blackfin): requesting ACK for Wolfram to take this patch. > Ralf (MIPS): requesting ACK for Wolfram to take this patch. For mach-pxa: Acked-by: Robert Jarzmik <robert.jarzmik@free.fr> Cheers. -- Robert
On Sun, Sep 17, 2017 at 11:39:03AM +0200, Linus Walleij wrote: > Steven (Blackfin): requesting ACK for Wolfram to take this patch. > Ralf (MIPS): requesting ACK for Wolfram to take this patch. Acked-by: Ralf Baechle <ralf@linux-mips.org> Ralf
Hi all, Steven is no longer working on this for ADI. Acked by me if this works. Thanks. Best regards, Aaron Wu Analog Devices Inc. -----Original Message----- From: Ralf Baechle [mailto:ralf@linux-mips.org] Sent: Monday, October 09, 2017 10:28 PM To: Linus Walleij <linus.walleij@linaro.org> Cc: linux-mips@linux-mips.org; Steven Miao <realmz6@gmail.com>; adi-buildroot-devel@lists.sourceforge.net; Geert Uytterhoeven <geert@linux-m68k.org>; linux-i2c@vger.kernel.org; linux-arm-kernel@lists.infradead.org Subject: Re: [Adi-buildroot-devel] [PATCH 4/7] i2c: gpio: Augment all boardfiles to use open drain On Sun, Sep 17, 2017 at 11:39:03AM +0200, Linus Walleij wrote: > Steven (Blackfin): requesting ACK for Wolfram to take this patch. > Ralf (MIPS): requesting ACK for Wolfram to take this patch. Acked-by: Ralf Baechle <ralf@linux-mips.org> Ralf ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Adi-buildroot-devel mailing list Adi-buildroot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/adi-buildroot-devel