You can subscribe to this list here.
| 2013 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
|
Jul
(12) |
Aug
(75) |
Sep
(29) |
Oct
(9) |
Nov
(5) |
Dec
(2) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 2014 |
Jan
(29) |
Feb
(39) |
Mar
(15) |
Apr
(44) |
May
(12) |
Jun
(17) |
Jul
(90) |
Aug
(41) |
Sep
(21) |
Oct
(111) |
Nov
(4) |
Dec
(31) |
| 2015 |
Jan
(89) |
Feb
(53) |
Mar
(59) |
Apr
(69) |
May
(19) |
Jun
(10) |
Jul
(6) |
Aug
(14) |
Sep
(3) |
Oct
(1) |
Nov
|
Dec
(154) |
| 2016 |
Jan
(60) |
Feb
|
Mar
(1) |
Apr
(2) |
May
(4) |
Jun
|
Jul
(2) |
Aug
(1) |
Sep
(1) |
Oct
|
Nov
|
Dec
|
| 2017 |
Jan
(1) |
Feb
(2) |
Mar
(2) |
Apr
|
May
(5) |
Jun
(3) |
Jul
(2) |
Aug
(9) |
Sep
(18) |
Oct
(2) |
Nov
|
Dec
(2) |
| 2018 |
Jan
(1) |
Feb
(1) |
Mar
|
Apr
|
May
(1) |
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
|
From: Igor Eslava <igor.eslava@zi...> - 2018-05-25 10:32:05
|
Hello, I found a problem when trying to initialize and probe the ds1339 rtc that works via I2C, because the first I2C transfer wasn't working properly due to a uninitialized variable in i2c-bfin-twi. I attach a patch that initializes it. This message and its attachment, if any, are confidential and may contain legally privileged information. If you are not the intended recipient, please contact the sender immediately and delete this message and its attachment, if any, from your system. You should not copy this message or disclose its contents to any other person or use it for any purpose. Statements and opinions expressed in this e-mail are those of the sender, and do not necessarily reflect those of ZIV Automation company, ZIV Automation company accepts no liability for damage caused by any virus transmitted by this email. |
|
From: Geert Uytterhoeven <geert@li...> - 2018-02-12 10:28:39
|
On Mon, Feb 12, 2018 at 11:17 AM, Geert Uytterhoeven <geert@...> wrote: > Below is the list of build error/warning regressions/improvements in > v4.16-rc1[1] compared to v4.15[2]. > > Summarized: > - build errors: +13/-5 > - build warnings: +1653/-1537 > > Note that there may be false regressions, as some logs are incomplete. > Still, they're build errors/warnings. > > Happy fixing! ;-) > > Thanks to the linux-next team for providing the build service. > > [1] http://kisskb.ellerman.id.au/kisskb/head/7928b2cbe55b2a410a0f5c1f154610059c57b1b2/ (all 273 configs) > [2] http://kisskb.ellerman.id.au/kisskb/head/d8a5b80568a9cb66810e75b182018e9edb68e8ff/ (271 out of 273 configs) > > > *** ERRORS *** > > 13 error regressions: > + /home/kisskb/slave/src/arch/powerpc/kvm/powerpc.c: error: 'emulated' may be used uninitialized in this function [-Werror=uninitialized]: => 1361:2 Lots of powerpc configs > + /home/kisskb/slave/src/drivers/net/ethernet/intel/i40e/i40e_ethtool.c: error: implicit declaration of function 'cmpxchg64' [-Werror=implicit-function-declaration]: => 4443:6, 4443:2 mips{,el}-allmodconfig > + /home/kisskb/slave/src/fs/signalfd.c: error: 'BUS_MCEERR_AR' undeclared (first use in this function): => 126:26 Lots of blackfin configs > + error: "mdesc_get_property" [drivers/sbus/char/oradax.ko] undefined!: => N/A > + error: "mdesc_grab" [drivers/sbus/char/oradax.ko] undefined!: => N/A > + error: "mdesc_node_by_name" [drivers/sbus/char/oradax.ko] undefined!: => N/A > + error: "mdesc_release" [drivers/sbus/char/oradax.ko] undefined!: => N/A > + error: "sun4v_ccb_info" [drivers/sbus/char/oradax.ko] undefined!: => N/A > + error: "sun4v_ccb_kill" [drivers/sbus/char/oradax.ko] undefined!: => N/A > + error: "sun4v_ccb_submit" [drivers/sbus/char/oradax.ko] undefined!: => N/A > + error: "sun4v_hvapi_register" [drivers/sbus/char/oradax.ko] undefined!: => N/A sparc-allmodconfig (i.e. sparc32) > + error: No rule to make target arch/ia64/kernel/pci-swiotlb.o: => N/A ia64-defconfig (patch availavle) > + error: hotplug-cpu.c: undefined reference to `find_and_online_cpu_nid': => .text+0x13c) ppc64le/pseries_le_defconfig Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@... 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 |
|
From: Wu, Aaron <Aaron.Wu@an...> - 2018-01-11 17:23:27
|
My apologize for this ... At this point ADI are not able to commit to maintaining the Blackfin port in the Linux Kernel. As such we propose that we remove the port from the active mainline so that it does not interfere with other kernel development. Moving forward we are still in effort to get closely involved with the community and in trying to support Linux for newer multi-core processors. I want to show my thanks sincerely to the community for supporting on us, and appreciation your understanding on this. Best regards, Aaron Wu Analog Devices > -----Original Message----- > From: Linus Walleij [mailto:linus.walleij@...] > Sent: 2017年12月28日 20:40 > To: Adam Borowski <kilobyte@...>; Arnd Bergmann <arnd@...> > Cc: Linus Torvalds <torvalds@...>; linux- > kernel@...; Steven Miao <realmz6@...>; Wu, Aaron > <Aaron.Wu@...>; Scott Jiang <scott.jiang.linux@...>; adi- > buildroot-devel@... > Subject: Re: [PATCH] MAINTAINERS: mark arch/blackfin/ and its gubbins as > orphaned > > Looping in Arnd as arch maintainer. > > On Mon, Dec 25, 2017 at 4:38 PM, Adam Borowski <kilobyte@...> > wrote: > > > The blackfin architecture has seen no maintainer action of any kind > > since April 2015. No new code, no pull requests, no acks to patches, > > no response to mails, nothing. > > > > The web site has an expired certificate (expiration Sep 2017, issued > > in 2013), the mailing list sees no answers either, with one exception: > > > > https://sourceforge.net/p/adi-buildroot/mailman/adi-buildroot-devel/ > >> 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. > > > > But, Aaron doesn't seem to respond to queries either. > > > > Signed-off-by: Adam Borowski <kilobyte@...> > > Acked-by: Linus Walleij <linus.walleij@...> > > Yours, > Linus Walleij |
|
From: Linus Walleij <linus.walleij@li...> - 2017-12-28 12:39:40
|
Looping in Arnd as arch maintainer. On Mon, Dec 25, 2017 at 4:38 PM, Adam Borowski <kilobyte@...> wrote: > The blackfin architecture has seen no maintainer action of any kind since > April 2015. No new code, no pull requests, no acks to patches, no response > to mails, nothing. > > The web site has an expired certificate (expiration Sep 2017, issued in > 2013), the mailing list sees no answers either, with one exception: > > https://sourceforge.net/p/adi-buildroot/mailman/adi-buildroot-devel/ >> 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. > > But, Aaron doesn't seem to respond to queries either. > > Signed-off-by: Adam Borowski <kilobyte@...> Acked-by: Linus Walleij <linus.walleij@...> Yours, Linus Walleij |
|
From: Linus Walleij <linus.walleij@li...> - 2017-12-27 11:57:15
|
On Tue, Dec 26, 2017 at 2:30 PM, Krzysztof Kozlowski <krzk@...> wrote:
> Remove old, dead Kconfig options (in order appearing in this commit):
> - EXPERIMENTAL is gone since v3.9;
> - INET_LRO: commit 7bbf3cae65b6 ("ipv4: Remove inet_lro library");
> - USB_DEVICE_CLASS: commit 007bab91324e ("USB: remove
> CONFIG_USB_DEVICE_CLASS");
> - HID_SUPPORT: commit 1f41a6a99476 ("HID: Fix the generic Kconfig
> options");
> - NETDEV_1000 and NETDEV_10000: commit f860b0522f65 ("drivers/net:
> Kconfig and Makefile cleanup"); NET_ETHERNET should be replaced with
> just ETHERNET but that is separate change;
> - RCU_CPU_STALL_DETECTOR: commit a00e0d714fbd ("rcu: Remove conditional
> compilation for RCU CPU stall warnings");
> - MISC_DEVICES: commit 7c5763b8453a ("drivers: misc: Remove
> MISC_DEVICES config option");
>
> Signed-off-by: Krzysztof Kozlowski <krzk@...>
Acked-by: Linus Walleij <linus.walleij@...>
This architecture is becoming a burden :/
Last blackfin pull request was in June 2014 by Steven Miao.
Steven, what's up with this? I don't mind the arch/blackfin as much
as the plethora of boardfiles that need to be maintained as soon as
we change some platform data or so, and it's just a mystery whether
things really get tested and any of the changes made here since 2014
are screwing up the blackfin. No ACKs or Tested-by's ever appear.
Is there active testing of mainline with blackfin?
Yours,
Linus Walleij
|
|
From: Wu, Aaron <Aaron.Wu@an...> - 2017-10-11 11:10:27
|
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@...] Sent: Monday, October 09, 2017 10:28 PM To: Linus Walleij <linus.walleij@...> Cc: linux-mips@...; Steven Miao <realmz6@...>; adi-buildroot-devel@...; Geert Uytterhoeven <geert@...>; linux-i2c@...; linux-arm-kernel@... 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@...> 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@... https://lists.sourceforge.net/lists/listinfo/adi-buildroot-devel |
|
From: Ralf Baechle <ralf@li...> - 2017-10-09 14:28:37
|
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@...> Ralf |
|
From: David Miller <davem@da...> - 2017-09-20 22:57:50
|
From: Florian Fainelli <f.fainelli@...> Date: Tue, 19 Sep 2017 18:03:44 -0700 > I sent those many months ago in the hope that the bfin-linux people > would pick those patches but nobody seems to be responding, can you > queue those via net-next since this affects DSA? Ok, if they aren't responding what can we do. Probably should update the MAINTAINERS file to reflect this... Series applied to net-next, thanks. |
|
From: Linus Walleij <linus.walleij@li...> - 2017-09-18 19:09:51
|
On Mon, Sep 18, 2017 at 11:58 AM, Geert Uytterhoeven <geert@...> 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 |
|
From: Geert Uytterhoeven <geert@li...> - 2017-09-18 11:16:03
|
Hi Linus,
On Sun, Sep 17, 2017 at 11:38 AM, Linus Walleij
<linus.walleij@...> 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@...>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...
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
|
|
From: Geert Uytterhoeven <geert@li...> - 2017-09-18 09:58:47
|
Hi Linus,
On Sun, Sep 17, 2017 at 11:39 AM, Linus Walleij
<linus.walleij@...> 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@...>
> Signed-off-by: Linus Walleij <linus.walleij@...>
> ---
> 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@...
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
|
|
From: Geert Uytterhoeven <geert@li...> - 2017-09-18 09:26:16
|
On Sun, Sep 17, 2017 at 11:39 AM, Linus Walleij
<linus.walleij@...> 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@...
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
|
|
From: Geert Uytterhoeven <geert@li...> - 2017-09-18 09:11:39
|
Hi Linus,
On Sun, Sep 17, 2017 at 11:39 AM, Linus Walleij
<linus.walleij@...> 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@...>
> ---
> 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@...
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
|
|
From: Linus Walleij <linus.walleij@li...> - 2017-09-17 09:40:24
|
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@...>
Signed-off-by: Linus Walleij <linus.walleij@...>
---
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;
+
+ retdesc = devm_gpiod_get(dev, con_id, gflags);
+ if (!IS_ERR(retdesc)) {
+ dev_dbg(dev, "got GPIO from name %s\n", con_id);
+ return retdesc;
+ }
+
+ retdesc = devm_gpiod_get_index(dev, NULL, index, gflags);
+ if (!IS_ERR(retdesc)) {
+ dev_dbg(dev, "got GPIO from index %u\n", index);
+ return retdesc;
+ }
+
+ ret = PTR_ERR(retdesc);
+
+ /* FIXME: hack in the old code, is this really necessary? */
+ if (ret == -EINVAL)
+ retdesc = ERR_PTR(-EPROBE_DEFER);
+
+ /* This happens if the GPIO driver is not yet probed, let's defer */
+ if (ret == -ENOENT)
+ retdesc = ERR_PTR(-EPROBE_DEFER);
+
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "error trying to get descriptor: %ld\n", ret);
+
+ return retdesc;
+}
+
static int i2c_gpio_probe(struct platform_device *pdev)
{
struct i2c_gpio_private_data *priv;
@@ -125,14 +161,10 @@ static int i2c_gpio_probe(struct platform_device *pdev)
gflags = GPIOD_OUT_HIGH;
else
gflags = GPIOD_OUT_HIGH_OPEN_DRAIN;
- priv->sda = devm_gpiod_get_index(dev, NULL, 0, gflags);
- if (IS_ERR(priv->sda)) {
- ret = PTR_ERR(priv->sda);
- /* FIXME: hack in the old code, is this really necessary? */
- if (ret == -EINVAL)
- ret = -EPROBE_DEFER;
- return ret;
- }
+ priv->sda = i2c_gpio_get_desc(dev, "sda", 0, gflags);
+ if (IS_ERR(priv->sda))
+ return PTR_ERR(priv->sda);
+
/*
* If the SCL line is marked from platform data or device tree as
* "open drain" it means something outside of our control is making
@@ -144,14 +176,9 @@ static int i2c_gpio_probe(struct platform_device *pdev)
gflags = GPIOD_OUT_LOW;
else
gflags = GPIOD_OUT_LOW_OPEN_DRAIN;
- priv->scl = devm_gpiod_get_index(dev, NULL, 1, gflags);
- if (IS_ERR(priv->scl)) {
- ret = PTR_ERR(priv->scl);
- /* FIXME: hack in the old code, is this really necessary? */
- if (ret == -EINVAL)
- ret = -EPROBE_DEFER;
- return ret;
- }
+ priv->scl = i2c_gpio_get_desc(dev, "scl", 1, gflags);
+ if (IS_ERR(priv->scl))
+ return PTR_ERR(priv->scl);
bit_data->setsda = i2c_gpio_setsda_val;
bit_data->setscl = i2c_gpio_setscl_val;
--
2.13.5
|
|
From: Linus Walleij <linus.walleij@li...> - 2017-09-17 09:39:39
|
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@...>
---
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
@@ -88,10 +88,12 @@ static int i2c_gpio_probe(struct platform_device *pdev)
struct i2c_gpio_platform_data *pdata;
struct i2c_algo_bit_data *bit_data;
struct i2c_adapter *adap;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
enum gpiod_flags gflags;
int ret;
- priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
@@ -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));
}
@@ -123,7 +125,7 @@ static int i2c_gpio_probe(struct platform_device *pdev)
gflags = GPIOD_OUT_HIGH;
else
gflags = GPIOD_OUT_HIGH_OPEN_DRAIN;
- priv->sda = devm_gpiod_get_index(&pdev->dev, NULL, 0, gflags);
+ priv->sda = devm_gpiod_get_index(dev, NULL, 0, gflags);
if (IS_ERR(priv->sda)) {
ret = PTR_ERR(priv->sda);
/* FIXME: hack in the old code, is this really necessary? */
@@ -142,7 +144,7 @@ static int i2c_gpio_probe(struct platform_device *pdev)
gflags = GPIOD_OUT_LOW;
else
gflags = GPIOD_OUT_LOW_OPEN_DRAIN;
- priv->scl = devm_gpiod_get_index(&pdev->dev, NULL, 1, gflags);
+ priv->scl = devm_gpiod_get_index(dev, NULL, 1, gflags);
if (IS_ERR(priv->scl)) {
ret = PTR_ERR(priv->scl);
/* FIXME: hack in the old code, is this really necessary? */
@@ -173,15 +175,15 @@ static int i2c_gpio_probe(struct platform_device *pdev)
bit_data->data = priv;
adap->owner = THIS_MODULE;
- if (pdev->dev.of_node)
- strlcpy(adap->name, dev_name(&pdev->dev), sizeof(adap->name));
+ if (np)
+ strlcpy(adap->name, dev_name(dev), sizeof(adap->name));
else
snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);
adap->algo_data = bit_data;
adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
- adap->dev.parent = &pdev->dev;
- adap->dev.of_node = pdev->dev.of_node;
+ adap->dev.parent = dev;
+ adap->dev.of_node = np;
adap->nr = pdev->id;
ret = i2c_bit_add_numbered_bus(adap);
@@ -195,7 +197,7 @@ static int i2c_gpio_probe(struct platform_device *pdev)
* get accessors to get the actual name of the GPIO line,
* from the descriptor, then provide that instead.
*/
- dev_info(&pdev->dev, "using lines %u (SDA) and %u (SCL%s)\n",
+ dev_info(dev, "using lines %u (SDA) and %u (SCL%s)\n",
desc_to_gpio(priv->sda), desc_to_gpio(priv->scl),
pdata->scl_is_output_only
? ", no clock stretching" : "");
--
2.13.5
|
|
From: Linus Walleij <linus.walleij@li...> - 2017-09-17 09:39:37
|
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@...>
Cc: Ralf Baechle <ralf@...>
Acked-by: Olof Johansson <olof@...>
Acked-by: Lee Jones <lee.jones@...>
Signed-off-by: Linus Walleij <linus.walleij@...>
---
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.
---
arch/arm/mach-ep93xx/core.c | 6 ++++--
arch/arm/mach-ixp4xx/avila-setup.c | 4 ++--
arch/arm/mach-ixp4xx/dsmg600-setup.c | 4 ++--
arch/arm/mach-ixp4xx/fsg-setup.c | 4 ++--
arch/arm/mach-ixp4xx/ixdp425-setup.c | 4 ++--
arch/arm/mach-ixp4xx/nas100d-setup.c | 4 ++--
arch/arm/mach-ixp4xx/nslu2-setup.c | 4 ++--
arch/arm/mach-ks8695/board-acs5k.c | 6 ++++--
arch/arm/mach-pxa/palmz72.c | 6 ++++--
arch/arm/mach-pxa/viper.c | 8 ++++----
arch/arm/mach-sa1100/simpad.c | 6 ++++--
arch/blackfin/mach-bf533/boards/blackstamp.c | 4 ++--
arch/blackfin/mach-bf533/boards/ezkit.c | 4 ++--
arch/blackfin/mach-bf533/boards/stamp.c | 4 ++--
arch/blackfin/mach-bf561/boards/ezkit.c | 4 ++--
arch/mips/alchemy/board-gpr.c | 4 ++++
arch/mips/ath79/mach-pb44.c | 4 ++--
drivers/mfd/sm501.c | 4 ++--
18 files changed, 48 insertions(+), 36 deletions(-)
diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c
index 6b4754ecaffc..efddea867cbe 100644
--- a/arch/arm/mach-ep93xx/core.c
+++ b/arch/arm/mach-ep93xx/core.c
@@ -326,8 +326,10 @@ static struct gpiod_lookup_table ep93xx_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
/* Use local offsets on gpiochip/port "G" */
- GPIO_LOOKUP_IDX("G", 1, NULL, 0, GPIO_ACTIVE_HIGH),
- GPIO_LOOKUP_IDX("G", 0, NULL, 1, GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX("G", 1, NULL, 0,
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
+ GPIO_LOOKUP_IDX("G", 0, NULL, 1,
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/arch/arm/mach-ixp4xx/avila-setup.c b/arch/arm/mach-ixp4xx/avila-setup.c
index 72122b5e7f28..bb6fbfc9b11a 100644
--- a/arch/arm/mach-ixp4xx/avila-setup.c
+++ b/arch/arm/mach-ixp4xx/avila-setup.c
@@ -53,9 +53,9 @@ static struct gpiod_lookup_table avila_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
GPIO_LOOKUP_IDX("IXP4XX_GPIO_CHIP", AVILA_SDA_PIN,
- NULL, 0, GPIO_ACTIVE_HIGH),
+ NULL, 0, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
GPIO_LOOKUP_IDX("IXP4XX_GPIO_CHIP", AVILA_SCL_PIN,
- NULL, 1, GPIO_ACTIVE_HIGH),
+ NULL, 1, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/arch/arm/mach-ixp4xx/dsmg600-setup.c b/arch/arm/mach-ixp4xx/dsmg600-setup.c
index 68ccd669051b..af543dd3da5d 100644
--- a/arch/arm/mach-ixp4xx/dsmg600-setup.c
+++ b/arch/arm/mach-ixp4xx/dsmg600-setup.c
@@ -72,9 +72,9 @@ static struct gpiod_lookup_table dsmg600_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
GPIO_LOOKUP_IDX("IXP4XX_GPIO_CHIP", DSMG600_SDA_PIN,
- NULL, 0, GPIO_ACTIVE_HIGH),
+ NULL, 0, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
GPIO_LOOKUP_IDX("IXP4XX_GPIO_CHIP", DSMG600_SCL_PIN,
- NULL, 1, GPIO_ACTIVE_HIGH),
+ NULL, 1, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/arch/arm/mach-ixp4xx/fsg-setup.c b/arch/arm/mach-ixp4xx/fsg-setup.c
index a0350ad15175..8afb3f4db376 100644
--- a/arch/arm/mach-ixp4xx/fsg-setup.c
+++ b/arch/arm/mach-ixp4xx/fsg-setup.c
@@ -58,9 +58,9 @@ static struct gpiod_lookup_table fsg_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
GPIO_LOOKUP_IDX("IXP4XX_GPIO_CHIP", FSG_SDA_PIN,
- NULL, 0, GPIO_ACTIVE_HIGH),
+ NULL, 0, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
GPIO_LOOKUP_IDX("IXP4XX_GPIO_CHIP", FSG_SCL_PIN,
- NULL, 1, GPIO_ACTIVE_HIGH),
+ NULL, 1, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/arch/arm/mach-ixp4xx/ixdp425-setup.c b/arch/arm/mach-ixp4xx/ixdp425-setup.c
index fd32ccde20c5..483d48bf252e 100644
--- a/arch/arm/mach-ixp4xx/ixdp425-setup.c
+++ b/arch/arm/mach-ixp4xx/ixdp425-setup.c
@@ -126,9 +126,9 @@ static struct gpiod_lookup_table ixdp425_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
GPIO_LOOKUP_IDX("IXP4XX_GPIO_CHIP", IXDP425_SDA_PIN,
- NULL, 0, GPIO_ACTIVE_HIGH),
+ NULL, 0, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
GPIO_LOOKUP_IDX("IXP4XX_GPIO_CHIP", IXDP425_SCL_PIN,
- NULL, 1, GPIO_ACTIVE_HIGH),
+ NULL, 1, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/arch/arm/mach-ixp4xx/nas100d-setup.c b/arch/arm/mach-ixp4xx/nas100d-setup.c
index 612ec8c63456..7e59c59c96a3 100644
--- a/arch/arm/mach-ixp4xx/nas100d-setup.c
+++ b/arch/arm/mach-ixp4xx/nas100d-setup.c
@@ -104,9 +104,9 @@ static struct gpiod_lookup_table nas100d_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
GPIO_LOOKUP_IDX("IXP4XX_GPIO_CHIP", NAS100D_SDA_PIN,
- NULL, 0, GPIO_ACTIVE_HIGH),
+ NULL, 0, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
GPIO_LOOKUP_IDX("IXP4XX_GPIO_CHIP", NAS100D_SCL_PIN,
- NULL, 1, GPIO_ACTIVE_HIGH),
+ NULL, 1, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/arch/arm/mach-ixp4xx/nslu2-setup.c b/arch/arm/mach-ixp4xx/nslu2-setup.c
index 13afb03b50fa..224717eb8ac2 100644
--- a/arch/arm/mach-ixp4xx/nslu2-setup.c
+++ b/arch/arm/mach-ixp4xx/nslu2-setup.c
@@ -72,9 +72,9 @@ static struct gpiod_lookup_table nslu2_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
GPIO_LOOKUP_IDX("IXP4XX_GPIO_CHIP", NSLU2_SDA_PIN,
- NULL, 0, GPIO_ACTIVE_HIGH),
+ NULL, 0, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
GPIO_LOOKUP_IDX("IXP4XX_GPIO_CHIP", NSLU2_SCL_PIN,
- NULL, 1, GPIO_ACTIVE_HIGH),
+ NULL, 1, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/arch/arm/mach-ks8695/board-acs5k.c b/arch/arm/mach-ks8695/board-acs5k.c
index f034724e01e1..937eb1d47e7b 100644
--- a/arch/arm/mach-ks8695/board-acs5k.c
+++ b/arch/arm/mach-ks8695/board-acs5k.c
@@ -41,8 +41,10 @@
static struct gpiod_lookup_table acs5k_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
- GPIO_LOOKUP_IDX("KS8695", 4, NULL, 0, GPIO_ACTIVE_HIGH),
- GPIO_LOOKUP_IDX("KS8695", 5, NULL, 1, GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX("KS8695", 4, NULL, 0,
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
+ GPIO_LOOKUP_IDX("KS8695", 5, NULL, 1,
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/arch/arm/mach-pxa/palmz72.c b/arch/arm/mach-pxa/palmz72.c
index 94f75632c007..5877e547cecd 100644
--- a/arch/arm/mach-pxa/palmz72.c
+++ b/arch/arm/mach-pxa/palmz72.c
@@ -324,8 +324,10 @@ static struct soc_camera_link palmz72_iclink = {
static struct gpiod_lookup_table palmz72_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
- GPIO_LOOKUP_IDX("gpio-pxa", 118, NULL, 0, GPIO_ACTIVE_HIGH),
- GPIO_LOOKUP_IDX("gpio-pxa", 117, NULL, 1, GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX("gpio-pxa", 118, NULL, 0,
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
+ GPIO_LOOKUP_IDX("gpio-pxa", 117, NULL, 1,
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/arch/arm/mach-pxa/viper.c b/arch/arm/mach-pxa/viper.c
index a680742bee2b..4185e7ff073f 100644
--- a/arch/arm/mach-pxa/viper.c
+++ b/arch/arm/mach-pxa/viper.c
@@ -463,9 +463,9 @@ static struct gpiod_lookup_table viper_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
GPIO_LOOKUP_IDX("gpio-pxa", VIPER_RTC_I2C_SDA_GPIO,
- NULL, 0, GPIO_ACTIVE_HIGH),
+ NULL, 0, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
GPIO_LOOKUP_IDX("gpio-pxa", VIPER_RTC_I2C_SCL_GPIO,
- NULL, 1, GPIO_ACTIVE_HIGH),
+ NULL, 1, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
@@ -792,9 +792,9 @@ struct gpiod_lookup_table viper_tpm_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
GPIO_LOOKUP_IDX("gpio-pxa", VIPER_TPM_I2C_SDA_GPIO,
- NULL, 0, GPIO_ACTIVE_HIGH),
+ NULL, 0, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
GPIO_LOOKUP_IDX("gpio-pxa", VIPER_TPM_I2C_SCL_GPIO,
- NULL, 1, GPIO_ACTIVE_HIGH),
+ NULL, 1, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/arch/arm/mach-sa1100/simpad.c b/arch/arm/mach-sa1100/simpad.c
index c6e7e6d8733a..91526024964b 100644
--- a/arch/arm/mach-sa1100/simpad.c
+++ b/arch/arm/mach-sa1100/simpad.c
@@ -327,8 +327,10 @@ static struct platform_device simpad_gpio_leds = {
static struct gpiod_lookup_table simpad_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
- GPIO_LOOKUP_IDX("gpio", GPIO_GPIO21, NULL, 0, GPIO_ACTIVE_HIGH),
- GPIO_LOOKUP_IDX("gpio", GPIO_GPIO25, NULL, 1, GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX("gpio", GPIO_GPIO21, NULL, 0,
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
+ GPIO_LOOKUP_IDX("gpio", GPIO_GPIO25, NULL, 1,
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/arch/blackfin/mach-bf533/boards/blackstamp.c b/arch/blackfin/mach-bf533/boards/blackstamp.c
index d801ca5ca6c4..fab69c736515 100644
--- a/arch/blackfin/mach-bf533/boards/blackstamp.c
+++ b/arch/blackfin/mach-bf533/boards/blackstamp.c
@@ -367,9 +367,9 @@ static struct gpiod_lookup_table bfin_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
GPIO_LOOKUP_IDX("BFIN-GPIO", GPIO_PF8, NULL, 0,
- GPIO_ACTIVE_HIGH),
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
GPIO_LOOKUP_IDX("BFIN-GPIO", GPIO_PF9, NULL, 1,
- GPIO_ACTIVE_HIGH),
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/arch/blackfin/mach-bf533/boards/ezkit.c b/arch/blackfin/mach-bf533/boards/ezkit.c
index 463a72358b0e..d64d270e9e62 100644
--- a/arch/blackfin/mach-bf533/boards/ezkit.c
+++ b/arch/blackfin/mach-bf533/boards/ezkit.c
@@ -395,9 +395,9 @@ static struct gpiod_lookup_table bfin_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
GPIO_LOOKUP_IDX("BFIN-GPIO", GPIO_PF1, NULL, 0,
- GPIO_ACTIVE_HIGH),
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
GPIO_LOOKUP_IDX("BFIN-GPIO", GPIO_PF0, NULL, 1,
- GPIO_ACTIVE_HIGH),
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/arch/blackfin/mach-bf533/boards/stamp.c b/arch/blackfin/mach-bf533/boards/stamp.c
index d2479359adb7..27cbf2fa2c62 100644
--- a/arch/blackfin/mach-bf533/boards/stamp.c
+++ b/arch/blackfin/mach-bf533/boards/stamp.c
@@ -517,9 +517,9 @@ static struct gpiod_lookup_table bfin_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
GPIO_LOOKUP_IDX("BFIN-GPIO", GPIO_PF2, NULL, 0,
- GPIO_ACTIVE_HIGH),
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
GPIO_LOOKUP_IDX("BFIN-GPIO", GPIO_PF3, NULL, 1,
- GPIO_ACTIVE_HIGH),
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/arch/blackfin/mach-bf561/boards/ezkit.c b/arch/blackfin/mach-bf561/boards/ezkit.c
index 72f757ebaa84..acc5363f60c6 100644
--- a/arch/blackfin/mach-bf561/boards/ezkit.c
+++ b/arch/blackfin/mach-bf561/boards/ezkit.c
@@ -384,9 +384,9 @@ static struct gpiod_lookup_table bfin_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
GPIO_LOOKUP_IDX("BFIN-GPIO", GPIO_PF1, NULL, 0,
- GPIO_ACTIVE_HIGH),
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
GPIO_LOOKUP_IDX("BFIN-GPIO", GPIO_PF0, NULL, 1,
- GPIO_ACTIVE_HIGH),
+ GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/arch/mips/alchemy/board-gpr.c b/arch/mips/alchemy/board-gpr.c
index daebc36e5ecb..328d697e72b4 100644
--- a/arch/mips/alchemy/board-gpr.c
+++ b/arch/mips/alchemy/board-gpr.c
@@ -235,6 +235,10 @@ static struct gpiod_lookup_table gpr_i2c_gpiod_table = {
};
static struct i2c_gpio_platform_data gpr_i2c_data = {
+ /*
+ * The open drain mode is hardwired somewhere or an electrical
+ * property of the alchemy GPIO controller.
+ */
.sda_is_open_drain = 1,
.scl_is_open_drain = 1,
.udelay = 2, /* ~100 kHz */
diff --git a/arch/mips/ath79/mach-pb44.c b/arch/mips/ath79/mach-pb44.c
index a95409063847..6b2c6f3baefa 100644
--- a/arch/mips/ath79/mach-pb44.c
+++ b/arch/mips/ath79/mach-pb44.c
@@ -37,9 +37,9 @@ static struct gpiod_lookup_table pb44_i2c_gpiod_table = {
.dev_id = "i2c-gpio",
.table = {
GPIO_LOOKUP_IDX("ath79-gpio", PB44_GPIO_I2C_SDA,
- NULL, 0, GPIO_ACTIVE_HIGH),
+ NULL, 0, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
GPIO_LOOKUP_IDX("ath79-gpio", PB44_GPIO_I2C_SCL,
- NULL, 1, GPIO_ACTIVE_HIGH),
+ NULL, 1, GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
},
};
diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c
index 4d40d013a412..ad774161a22d 100644
--- a/drivers/mfd/sm501.c
+++ b/drivers/mfd/sm501.c
@@ -1152,7 +1152,7 @@ static int sm501_register_gpio_i2c_instance(struct sm501_devdata *sm,
lookup->table[0].chip_hwnum = iic->pin_sda % 32;
lookup->table[0].con_id = NULL;
lookup->table[0].idx = 0;
- lookup->table[0].flags = GPIO_ACTIVE_HIGH;
+ lookup->table[0].flags = GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN;
if (iic->pin_scl < 32)
lookup->table[1].chip_label = "SM501-LOW";
else
@@ -1160,7 +1160,7 @@ static int sm501_register_gpio_i2c_instance(struct sm501_devdata *sm,
lookup->table[1].chip_hwnum = iic->pin_scl % 32;
lookup->table[1].con_id = NULL;
lookup->table[1].idx = 1;
- lookup->table[1].flags = GPIO_ACTIVE_HIGH;
+ lookup->table[1].flags = GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN;
gpiod_add_lookup_table(lookup);
icd = dev_get_platdata(&pdev->dev);
--
2.13.5
|
|
From: Linus Walleij <linus.walleij@li...> - 2017-09-17 09:39:34
|
The I2C GPIO bitbang driver currently emulates open drain
behaviour by implementing what the gpiolib already does:
not actively driving the line high, instead setting it to
input.
This makes no sense. Use the new facility in gpiolib to
request the lines enforced into open drain mode, and let
the open drain emulation already present in the gpiolib
kick in and handle this.
As a bonus: if the GPIO driver in the back-end actually
supports open drain in hardware using the .set_config()
callback, it will be utilized. That's correct: we never
used that hardware feature before, instead relying on
emulating open drain even if the GPIO controller could
actually handle this for us.
Users will sometimes get messages like this:
gpio-485 (?): enforced open drain please flag it properly
in DT/ACPI DSDT/board file
gpio-486 (?): enforced open drain please flag it properly
in DT/ACPI DSDT/board file
i2c-gpio gpio-i2c: using lines 485 (SDA) and 486 (SCL)
Which is completely proper: since the line is used as
open drain, it should actually be flagged properly with
e.g.
gpios = <&gpio0 5 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>,
<&gpio0 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
Or similar facilities in board file descriptor tables
or ACPI DSDT.
Signed-off-by: Linus Walleij <linus.walleij@...>
---
ChangeLog v1->v2:
- Fix a minor typo in error path (copy-paste scl was sda)
---
drivers/i2c/busses/i2c-gpio.c | 102 ++++++++++++++++--------------------------
1 file changed, 39 insertions(+), 63 deletions(-)
diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index b4664037eded..97b9c29e9429 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -25,23 +25,6 @@ struct i2c_gpio_private_data {
struct i2c_gpio_platform_data pdata;
};
-/* Toggle SDA by changing the direction of the pin */
-static void i2c_gpio_setsda_dir(void *data, int state)
-{
- struct i2c_gpio_private_data *priv = data;
-
- /*
- * This is a way of saying "do not drive
- * me actively high" which means emulating open drain.
- * The right way to do this is for gpiolib to
- * handle this, by the function below.
- */
- if (state)
- gpiod_direction_input(priv->sda);
- else
- gpiod_direction_output(priv->sda, 0);
-}
-
/*
* Toggle SDA by changing the output value of the pin. This is only
* valid for pins configured as open drain (i.e. setting the value
@@ -54,17 +37,6 @@ static void i2c_gpio_setsda_val(void *data, int state)
gpiod_set_value(priv->sda, state);
}
-/* Toggle SCL by changing the direction of the pin. */
-static void i2c_gpio_setscl_dir(void *data, int state)
-{
- struct i2c_gpio_private_data *priv = data;
-
- if (state)
- gpiod_direction_input(priv->scl);
- else
- gpiod_direction_output(priv->scl, 0);
-}
-
/*
* Toggle SCL by changing the output value of the pin. This is used
* for pins that are configured as open drain and for output-only
@@ -116,30 +88,13 @@ static int i2c_gpio_probe(struct platform_device *pdev)
struct i2c_gpio_platform_data *pdata;
struct i2c_algo_bit_data *bit_data;
struct i2c_adapter *adap;
+ enum gpiod_flags gflags;
int ret;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
- /* First get the GPIO pins; if it fails, we'll defer the probe. */
- priv->sda = devm_gpiod_get_index(&pdev->dev, NULL, 0, GPIOD_OUT_HIGH);
- if (IS_ERR(priv->sda)) {
- ret = PTR_ERR(priv->sda);
- /* FIXME: hack in the old code, is this really necessary? */
- if (ret == -EINVAL)
- ret = -EPROBE_DEFER;
- return ret;
- }
- priv->scl = devm_gpiod_get_index(&pdev->dev, NULL, 1, GPIOD_OUT_LOW);
- if (IS_ERR(priv->scl)) {
- ret = PTR_ERR(priv->scl);
- /* FIXME: hack in the old code, is this really necessary? */
- if (ret == -EINVAL)
- ret = -EPROBE_DEFER;
- return ret;
- }
-
adap = &priv->adap;
bit_data = &priv->bit_data;
pdata = &priv->pdata;
@@ -157,27 +112,48 @@ static int i2c_gpio_probe(struct platform_device *pdev)
}
/*
- * FIXME: this is a hack emulating the open drain emulation
- * that gpiolib can already do for us. Make all clients properly
- * flag their lines as open drain and get rid of this property
- * and the special callback.
+ * First get the GPIO pins; if it fails, we'll defer the probe.
+ * If the SDA line is marked from platform data or device tree as
+ * "open drain" it means something outside of our control is making
+ * this line being handled as open drain, and we should just handle
+ * it as any other output. Else we enforce open drain as this is
+ * required for an I2C bus.
*/
- if (pdata->sda_is_open_drain) {
- gpiod_direction_output(priv->sda, 1);
- bit_data->setsda = i2c_gpio_setsda_val;
- } else {
- gpiod_direction_input(priv->sda);
- bit_data->setsda = i2c_gpio_setsda_dir;
+ if (pdata->sda_is_open_drain)
+ gflags = GPIOD_OUT_HIGH;
+ else
+ gflags = GPIOD_OUT_HIGH_OPEN_DRAIN;
+ priv->sda = devm_gpiod_get_index(&pdev->dev, NULL, 0, gflags);
+ if (IS_ERR(priv->sda)) {
+ ret = PTR_ERR(priv->sda);
+ /* FIXME: hack in the old code, is this really necessary? */
+ if (ret == -EINVAL)
+ ret = -EPROBE_DEFER;
+ return ret;
}
-
- if (pdata->scl_is_open_drain || pdata->scl_is_output_only) {
- gpiod_direction_output(priv->scl, 1);
- bit_data->setscl = i2c_gpio_setscl_val;
- } else {
- gpiod_direction_input(priv->scl);
- bit_data->setscl = i2c_gpio_setscl_dir;
+ /*
+ * If the SCL line is marked from platform data or device tree as
+ * "open drain" it means something outside of our control is making
+ * this line being handled as open drain, and we should just handle
+ * it as any other output. Else we enforce open drain as this is
+ * required for an I2C bus.
+ */
+ if (pdata->scl_is_open_drain)
+ gflags = GPIOD_OUT_LOW;
+ else
+ gflags = GPIOD_OUT_LOW_OPEN_DRAIN;
+ priv->scl = devm_gpiod_get_index(&pdev->dev, NULL, 1, gflags);
+ if (IS_ERR(priv->scl)) {
+ ret = PTR_ERR(priv->scl);
+ /* FIXME: hack in the old code, is this really necessary? */
+ if (ret == -EINVAL)
+ ret = -EPROBE_DEFER;
+ return ret;
}
+ bit_data->setsda = i2c_gpio_setsda_val;
+ bit_data->setscl = i2c_gpio_setscl_val;
+
if (!pdata->scl_is_output_only)
bit_data->getscl = i2c_gpio_getscl;
bit_data->getsda = i2c_gpio_getsda;
--
2.13.5
|
|
From: Linus Walleij <linus.walleij@li...> - 2017-09-17 09:39:31
|
Some busses, like I2C, strictly need to have the line handled
as open drain, i.e. not actively driven high. For this reason
the i2c-gpio.c bit-banged I2C driver is reimplementing open
drain handling outside of gpiolib.
This is not very optimal. Instead make it possible for a
consumer to explcitly express that the line must be handled
as open drain instead of allowing local hacks papering over
this issue.
The descriptor tables, whether DT, ACPI or board files, should
of course have flagged these lines as open drain. E.g.:
enum gpio_lookup_flags GPIO_OPEN_DRAIN for a board file, or
gpios = <&foo 42 GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN>; in a
device tree using <dt-bindings/gpio/gpio.h>
But more often than not, these descriptors are wrong. So
we need to make it possible for consumers to enforce this
open drain behaviour.
We now have two new enumerated GPIO descriptor config flags:
GPIOD_OUT_LOW_OPEN_DRAIN and GPIOD_OUT_HIGH_OPEN_DRAIN
that will set up the lined enforced as open drain as output
low or high, using open drain (if the driver supports it)
or using open drain emulation (setting the line as input
to drive it high) from the gpiolib core.
Cc: linux-gpio@...
Signed-off-by: Linus Walleij <linus.walleij@...>
---
Obviously I authorize this patch to be applied directly
to the I2C tree as part of the refactorings.
---
drivers/gpio/gpiolib.c | 13 +++++++++++++
include/linux/gpio/consumer.h | 6 ++++++
2 files changed, 19 insertions(+)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index cd003b74512f..cb73a50a5d5d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3228,8 +3228,21 @@ int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
if (lflags & GPIO_ACTIVE_LOW)
set_bit(FLAG_ACTIVE_LOW, &desc->flags);
+
if (lflags & GPIO_OPEN_DRAIN)
set_bit(FLAG_OPEN_DRAIN, &desc->flags);
+ else if (dflags & GPIOD_FLAGS_BIT_OPEN_DRAIN) {
+ /*
+ * This enforces open drain mode from the consumer side.
+ * This is necessary for some busses like I2C, but the lookup
+ * should *REALLY* have specified them as open drain in the
+ * first place, so print a little warning here.
+ */
+ set_bit(FLAG_OPEN_DRAIN, &desc->flags);
+ gpiod_warn(desc,
+ "enforced open drain please flag it properly in DT/ACPI DSDT/board file\n");
+ }
+
if (lflags & GPIO_OPEN_SOURCE)
set_bit(FLAG_OPEN_SOURCE, &desc->flags);
if (lflags & GPIO_SLEEP_MAY_LOOSE_VALUE)
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 8f702fcbe485..5f72a49d1aa3 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -28,6 +28,7 @@ struct gpio_descs {
#define GPIOD_FLAGS_BIT_DIR_SET BIT(0)
#define GPIOD_FLAGS_BIT_DIR_OUT BIT(1)
#define GPIOD_FLAGS_BIT_DIR_VAL BIT(2)
+#define GPIOD_FLAGS_BIT_OPEN_DRAIN BIT(3)
/**
* Optional flags that can be passed to one of gpiod_* to configure direction
@@ -39,6 +40,11 @@ enum gpiod_flags {
GPIOD_OUT_LOW = GPIOD_FLAGS_BIT_DIR_SET | GPIOD_FLAGS_BIT_DIR_OUT,
GPIOD_OUT_HIGH = GPIOD_FLAGS_BIT_DIR_SET | GPIOD_FLAGS_BIT_DIR_OUT |
GPIOD_FLAGS_BIT_DIR_VAL,
+ GPIOD_OUT_LOW_OPEN_DRAIN = GPIOD_FLAGS_BIT_DIR_SET |
+ GPIOD_FLAGS_BIT_DIR_OUT | GPIOD_FLAGS_BIT_OPEN_DRAIN,
+ GPIOD_OUT_HIGH_OPEN_DRAIN = GPIOD_FLAGS_BIT_DIR_SET |
+ GPIOD_FLAGS_BIT_DIR_OUT | GPIOD_FLAGS_BIT_DIR_VAL |
+ GPIOD_FLAGS_BIT_OPEN_DRAIN,
};
#ifdef CONFIG_GPIOLIB
--
2.13.5
|
|
From: Linus Walleij <linus.walleij@li...> - 2017-09-17 09:39:24
|
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 Documentation/devicetree/bindings/i2c/i2c-gpio.txt | 32 +++- arch/arm/mach-ep93xx/core.c | 41 ++-- arch/arm/mach-ep93xx/edb93xx.c | 15 +- arch/arm/mach-ep93xx/include/mach/platform.h | 4 +- arch/arm/mach-ep93xx/simone.c | 12 +- arch/arm/mach-ep93xx/snappercl15.c | 12 +- arch/arm/mach-ep93xx/vision_ep9307.c | 7 +- arch/arm/mach-ixp4xx/avila-setup.c | 17 +- arch/arm/mach-ixp4xx/dsmg600-setup.c | 16 +- arch/arm/mach-ixp4xx/fsg-setup.c | 16 +- arch/arm/mach-ixp4xx/goramo_mlr.c | 24 +-- arch/arm/mach-ixp4xx/ixdp425-setup.c | 16 +- arch/arm/mach-ixp4xx/nas100d-setup.c | 16 +- arch/arm/mach-ixp4xx/nslu2-setup.c | 16 +- arch/arm/mach-ks8695/board-acs5k.c | 15 +- arch/arm/mach-pxa/palmz72.c | 14 +- arch/arm/mach-pxa/viper.c | 27 ++- arch/arm/mach-sa1100/simpad.c | 14 +- arch/blackfin/mach-bf533/boards/blackstamp.c | 19 +- arch/blackfin/mach-bf533/boards/ezkit.c | 18 +- arch/blackfin/mach-bf533/boards/stamp.c | 18 +- arch/blackfin/mach-bf561/boards/ezkit.c | 18 +- arch/mips/alchemy/board-gpr.c | 23 ++- arch/mips/ath79/mach-pb44.c | 16 +- drivers/gpio/gpiolib.c | 13 ++ drivers/i2c/busses/i2c-gpio.c | 213 ++++++++++----------- drivers/mfd/sm501.c | 49 ++--- include/linux/gpio/consumer.h | 6 + include/linux/i2c-gpio.h | 4 - 29 files changed, 423 insertions(+), 288 deletions(-) -- 2.13.5 |
|
From: Lee Jones <lee.jones@li...> - 2017-09-14 10:04:38
|
On Sun, 10 Sep 2017, Linus Walleij wrote: > This converts the GPIO-based I2C-driver to using GPIO > descriptors instead of the old global numberspace-based > GPIO interface. We: > > - Convert the driver to unconditionally grab two GPIOs > from the device by index 0 (SDA) and 1 (SCL) which > will work fine with device tree and descriptor tables. > The existing device trees will continue to work just > like before, but without any roundtrip through the > global numberspace. > > - Brutally convert all boardfiles still passing global > GPIOs by registering descriptor tables associated with > the devices instead so this driver does not need to keep > supporting passing any GPIO numbers as platform data. > > There is no stepwise approach as elegant as this, I > strongly prefer this big hammer over any antsteps for this > conversion. This way the old GPIO numbers go away and > NEVER COME BACK. > > Special conversion for the different boards utilizing > I2C-GPIO: > > - EP93xx (arch/arm/mach-ep93xx): pretty straight forward as > all boards were using the same two GPIO lines, just define > these two in a lookup table for "i2c-gpio" and register > these along with the device. None of them define any > other platform data so just pass NULL as platform data. > This platform selects GPIOLIB so all should be smooth. > The pins appear on a gpiochip for bank "G" as pins 1 (SDA) > and 0 (SCL). > > - IXP4 (arch/arm/mach-ixp4): descriptor tables have to > be registered for each board separately. They all use > "IXP4XX_GPIO_CHIP" so it is pretty straight forward. > Most board define no other platform data than SCL/SDA > so they can drop the #include of <linux/i2c-gpio.h> and > assign NULL to platform data. > > The "goramo_mlr" (Goramo Multilink Router) board is a bit > worrisome: it implements its own I2C bit-banging in the > board file, and optionally registers an I2C serial port, > but claims the same GPIO lines for itself in the board file. > This is not going to work: there will be competition for the > GPIO lines, so delete the optional extra I2C bus instead, no > I2C devices are registered on it anyway, there are just hints > that it may contain an EEPROM that may be accessed from > userspace. This needs to be fixed up properly by the serial > clock using I2C emulation so drop a note in the code. > > - KS8695 board acs5k (arch/arm/mach-ks8695/board-acs5.c) > has some platform data in addition to the pins so it needs to > be kept around sans GPIO lines. Its GPIO chip is named > "KS8695" and the arch selects GPIOLIB. > > - PXA boards (arch/arm/mach-pxa/*) use some of the platform > data so it needs to be preserved here. The viper board even > registers two GPIO I2Cs. The gpiochip is named "gpio-pxa" and > the arch selects GPIOLIB. > > - SA1100 Simpad (arch/arm/mach-sa1100/simpad.c) defines a GPIO > I2C bus, and the arch selects GPIOLIB. > > - Blackfin boards (arch/blackfin/bf533 etc) for these I assume > their I2C GPIOs refer to the local gpiochip defined in > arch/blackfin/kernel/bfin_gpio.c names "BFIN-GPIO". > The arch selects GPIOLIB. The boards get spiked with > IF_ENABLED(I2C_GPIO) but that is a side effect of it > being like that already (I would just have Kconfig select > I2C_GPIO and get rid of them all.) I also delete any > platform data set to 0 as it will get that value anyway > from static declartions of platform data. > > - The MIPS selects GPIOLIB and the Alchemy machine is using > two local GPIO chips, one of them has a GPIO I2C. We need > to adjust the local offset from the global number space here. > The ATH79 has a proper GPIO driver in drivers/gpio/gpio-ath79.c > and AFAICT the chip is named "ath79-gpio" and the PB44 > PCF857x expander spawns from this on GPIO 1 and 0. The latter > board only use the platform data to specify pins so it can be > cut altogether after this. > > - The MFD Silicon Motion SM501 is a special case. It dynamically > spawns an I2C bus off the MFD using sm501_create_subdev(). > We use an approach to dynamically create a machine descriptor > table and attach this to the "SM501-LOW" or "SM501-HIGH" > gpiochip. We use chip-local offsets to grab the right lines. > We can get rid of two local static inline helpers as part > of this refactoring. > > Cc: arm@... > Cc: Steven Miao <realmz6@...> > Cc: Ralf Baechle <ralf@...> > Cc: Lee Jones <lee.jones@...> > Signed-off-by: Linus Walleij <linus.walleij@...> > --- > ARM SoC folks: requesting ACK for Wolfram to take this patch. > Steven (Blackfin): requesting ACK for Wolfram to take this patch. > Ralf (MIPS): requesting ACK for Wolfram to take this patch. > Lee: requesting ACK for Wolfram to take this patch. > --- > arch/arm/mach-ep93xx/core.c | 39 ++++---- > arch/arm/mach-ep93xx/edb93xx.c | 15 +-- > arch/arm/mach-ep93xx/include/mach/platform.h | 4 +- > arch/arm/mach-ep93xx/simone.c | 12 +-- > arch/arm/mach-ep93xx/snappercl15.c | 12 +-- > arch/arm/mach-ep93xx/vision_ep9307.c | 7 +- > arch/arm/mach-ixp4xx/avila-setup.c | 17 +++- > arch/arm/mach-ixp4xx/dsmg600-setup.c | 16 +++- > arch/arm/mach-ixp4xx/fsg-setup.c | 16 +++- > arch/arm/mach-ixp4xx/goramo_mlr.c | 24 ++--- > arch/arm/mach-ixp4xx/ixdp425-setup.c | 16 +++- > arch/arm/mach-ixp4xx/nas100d-setup.c | 16 +++- > arch/arm/mach-ixp4xx/nslu2-setup.c | 16 +++- > arch/arm/mach-ks8695/board-acs5k.c | 13 ++- > arch/arm/mach-pxa/palmz72.c | 12 ++- > arch/arm/mach-pxa/viper.c | 27 +++++- > arch/arm/mach-sa1100/simpad.c | 12 ++- > arch/blackfin/mach-bf533/boards/blackstamp.c | 19 +++- > arch/blackfin/mach-bf533/boards/ezkit.c | 18 +++- > arch/blackfin/mach-bf533/boards/stamp.c | 18 +++- > arch/blackfin/mach-bf561/boards/ezkit.c | 18 +++- > arch/mips/alchemy/board-gpr.c | 19 +++- > arch/mips/ath79/mach-pb44.c | 16 +++- > drivers/i2c/busses/i2c-gpio.c | 134 +++++++++++++-------------- > drivers/mfd/sm501.c | 49 +++++----- I'd prefer for this to be applied with a Tested-by. I appreciate that this is an old driver, but can you attempt to contact one of the authors or someone else who might have hardware please? > include/linux/i2c-gpio.h | 4 - > 26 files changed, 327 insertions(+), 242 deletions(-) -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog |
|
From: Lee Jones <lee.jones@li...> - 2017-09-14 10:03:23
|
On Sun, 10 Sep 2017, Linus Walleij wrote: > 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: arm@... > Cc: Steven Miao <realmz6@...> > Cc: Ralf Baechle <ralf@...> > Cc: Lee Jones <lee.jones@...> > Signed-off-by: Linus Walleij <linus.walleij@...> > --- > ARM SoC folks: requesting ACK for Wolfram to take this patch. > Steven (Blackfin): requesting ACK for Wolfram to take this patch. > Ralf (MIPS): requesting ACK for Wolfram to take this patch. > Lee: requesting ACK for Wolfram to take this patch. > --- > arch/arm/mach-ep93xx/core.c | 6 ++++-- > arch/arm/mach-ixp4xx/avila-setup.c | 4 ++-- > arch/arm/mach-ixp4xx/dsmg600-setup.c | 4 ++-- > arch/arm/mach-ixp4xx/fsg-setup.c | 4 ++-- > arch/arm/mach-ixp4xx/ixdp425-setup.c | 4 ++-- > arch/arm/mach-ixp4xx/nas100d-setup.c | 4 ++-- > arch/arm/mach-ixp4xx/nslu2-setup.c | 4 ++-- > arch/arm/mach-ks8695/board-acs5k.c | 6 ++++-- > arch/arm/mach-pxa/palmz72.c | 6 ++++-- > arch/arm/mach-pxa/viper.c | 8 ++++---- > arch/arm/mach-sa1100/simpad.c | 6 ++++-- > arch/blackfin/mach-bf533/boards/blackstamp.c | 4 ++-- > arch/blackfin/mach-bf533/boards/ezkit.c | 4 ++-- > arch/blackfin/mach-bf533/boards/stamp.c | 4 ++-- > arch/blackfin/mach-bf561/boards/ezkit.c | 4 ++-- > arch/mips/alchemy/board-gpr.c | 4 ++++ > arch/mips/ath79/mach-pb44.c | 4 ++-- > drivers/mfd/sm501.c | 4 ++-- Acked-by: Lee Jones <lee.jones@...> -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog |
|
From: Linus Walleij <linus.walleij@li...> - 2017-09-10 22:17:39
|
The I2C GPIO bitbang driver currently emulates open drain
behaviour by implementing what the gpiolib already does:
not actively driving the line high, instead setting it to
input.
This makes no sense. Use the new facility in gpiolib to
request the lines enforced into open drain mode, and let
the open drain emulation already present in the gpiolib
kick in and handle this.
As a bonus: if the GPIO driver in the back-end actually
supports open drain in hardware using the .set_config()
callback, it will be utilized. That's correct: we never
used that hardware feature before, instead relying on
emulating open drain even if the GPIO controller could
actually handle this for us.
Users will sometimes get messages like this:
gpio-485 (?): enforced open drain please flag it properly
in DT/ACPI DSDT/board file
gpio-486 (?): enforced open drain please flag it properly
in DT/ACPI DSDT/board file
i2c-gpio gpio-i2c: using lines 485 (SDA) and 486 (SCL)
Which is completely proper: since the line is used as
open drain, it should actually be flagged properly with
e.g.
gpios = <&gpio0 5 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>,
<&gpio0 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
Or similar facilities in board file descriptor tables
or ACPI DSDT.
Signed-off-by: Linus Walleij <linus.walleij@...>
---
drivers/i2c/busses/i2c-gpio.c | 102 ++++++++++++++++--------------------------
1 file changed, 39 insertions(+), 63 deletions(-)
diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index 49e8b3ab30be..18ac4cf2ee75 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -25,23 +25,6 @@ struct i2c_gpio_private_data {
struct i2c_gpio_platform_data pdata;
};
-/* Toggle SDA by changing the direction of the pin */
-static void i2c_gpio_setsda_dir(void *data, int state)
-{
- struct i2c_gpio_private_data *priv = data;
-
- /*
- * This is a way of saying "do not drive
- * me actively high" which means emulating open drain.
- * The right way to do this is for gpiolib to
- * handle this, by the function below.
- */
- if (state)
- gpiod_direction_input(priv->sda);
- else
- gpiod_direction_output(priv->sda, 0);
-}
-
/*
* Toggle SDA by changing the output value of the pin. This is only
* valid for pins configured as open drain (i.e. setting the value
@@ -54,17 +37,6 @@ static void i2c_gpio_setsda_val(void *data, int state)
gpiod_set_value(priv->sda, state);
}
-/* Toggle SCL by changing the direction of the pin. */
-static void i2c_gpio_setscl_dir(void *data, int state)
-{
- struct i2c_gpio_private_data *priv = data;
-
- if (state)
- gpiod_direction_input(priv->scl);
- else
- gpiod_direction_output(priv->scl, 0);
-}
-
/*
* Toggle SCL by changing the output value of the pin. This is used
* for pins that are configured as open drain and for output-only
@@ -116,30 +88,13 @@ static int i2c_gpio_probe(struct platform_device *pdev)
struct i2c_gpio_platform_data *pdata;
struct i2c_algo_bit_data *bit_data;
struct i2c_adapter *adap;
+ enum gpiod_flags gflags;
int ret;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
- /* First get the GPIO pins; if it fails, we'll defer the probe. */
- priv->sda = devm_gpiod_get_index(&pdev->dev, NULL, 0, GPIOD_OUT_HIGH);
- if (IS_ERR(priv->sda)) {
- ret = PTR_ERR(priv->sda);
- /* FIXME: hack in the old code, is this really necessary? */
- if (ret == -EINVAL)
- ret = -EPROBE_DEFER;
- return ret;
- }
- priv->scl = devm_gpiod_get_index(&pdev->dev, NULL, 1, GPIOD_OUT_LOW);
- if (IS_ERR(priv->scl)) {
- ret = PTR_ERR(priv->sda);
- /* FIXME: hack in the old code, is this really necessary? */
- if (ret == -EINVAL)
- ret = -EPROBE_DEFER;
- return ret;
- }
-
adap = &priv->adap;
bit_data = &priv->bit_data;
pdata = &priv->pdata;
@@ -157,27 +112,48 @@ static int i2c_gpio_probe(struct platform_device *pdev)
}
/*
- * FIXME: this is a hack emulating the open drain emulation
- * that gpiolib can already do for us. Make all clients properly
- * flag their lines as open drain and get rid of this property
- * and the special callback.
+ * First get the GPIO pins; if it fails, we'll defer the probe.
+ * If the SDA line is marked from platform data or device tree as
+ * "open drain" it means something outside of our control is making
+ * this line being handled as open drain, and we should just handle
+ * it as any other output. Else we enforce open drain as this is
+ * required for an I2C bus.
*/
- if (pdata->sda_is_open_drain) {
- gpiod_direction_output(priv->sda, 1);
- bit_data->setsda = i2c_gpio_setsda_val;
- } else {
- gpiod_direction_input(priv->sda);
- bit_data->setsda = i2c_gpio_setsda_dir;
+ if (pdata->sda_is_open_drain)
+ gflags = GPIOD_OUT_HIGH;
+ else
+ gflags = GPIOD_OUT_HIGH_OPEN_DRAIN;
+ priv->sda = devm_gpiod_get_index(&pdev->dev, NULL, 0, gflags);
+ if (IS_ERR(priv->sda)) {
+ ret = PTR_ERR(priv->sda);
+ /* FIXME: hack in the old code, is this really necessary? */
+ if (ret == -EINVAL)
+ ret = -EPROBE_DEFER;
+ return ret;
}
-
- if (pdata->scl_is_open_drain || pdata->scl_is_output_only) {
- gpiod_direction_output(priv->scl, 1);
- bit_data->setscl = i2c_gpio_setscl_val;
- } else {
- gpiod_direction_input(priv->scl);
- bit_data->setscl = i2c_gpio_setscl_dir;
+ /*
+ * If the SCL line is marked from platform data or device tree as
+ * "open drain" it means something outside of our control is making
+ * this line being handled as open drain, and we should just handle
+ * it as any other output. Else we enforce open drain as this is
+ * required for an I2C bus.
+ */
+ if (pdata->scl_is_open_drain)
+ gflags = GPIOD_OUT_LOW;
+ else
+ gflags = GPIOD_OUT_LOW_OPEN_DRAIN;
+ priv->scl = devm_gpiod_get_index(&pdev->dev, NULL, 1, gflags);
+ if (IS_ERR(priv->scl)) {
+ ret = PTR_ERR(priv->sda);
+ /* FIXME: hack in the old code, is this really necessary? */
+ if (ret == -EINVAL)
+ ret = -EPROBE_DEFER;
+ return ret;
}
+ bit_data->setsda = i2c_gpio_setsda_val;
+ bit_data->setscl = i2c_gpio_setscl_val;
+
if (!pdata->scl_is_output_only)
bit_data->getscl = i2c_gpio_getscl;
bit_data->getsda = i2c_gpio_getsda;
--
2.13.5
|
|
From: Linus Walleij <linus.walleij@li...> - 2017-09-10 22:12:02
|
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@...>
---
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 18ac4cf2ee75..7fe599f4fdb0 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -88,10 +88,12 @@ static int i2c_gpio_probe(struct platform_device *pdev)
struct i2c_gpio_platform_data *pdata;
struct i2c_algo_bit_data *bit_data;
struct i2c_adapter *adap;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
enum gpiod_flags gflags;
int ret;
- priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
@@ -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));
}
@@ -123,7 +125,7 @@ static int i2c_gpio_probe(struct platform_device *pdev)
gflags = GPIOD_OUT_HIGH;
else
gflags = GPIOD_OUT_HIGH_OPEN_DRAIN;
- priv->sda = devm_gpiod_get_index(&pdev->dev, NULL, 0, gflags);
+ priv->sda = devm_gpiod_get_index(dev, NULL, 0, gflags);
if (IS_ERR(priv->sda)) {
ret = PTR_ERR(priv->sda);
/* FIXME: hack in the old code, is this really necessary? */
@@ -142,7 +144,7 @@ static int i2c_gpio_probe(struct platform_device *pdev)
gflags = GPIOD_OUT_LOW;
else
gflags = GPIOD_OUT_LOW_OPEN_DRAIN;
- priv->scl = devm_gpiod_get_index(&pdev->dev, NULL, 1, gflags);
+ priv->scl = devm_gpiod_get_index(dev, NULL, 1, gflags);
if (IS_ERR(priv->scl)) {
ret = PTR_ERR(priv->sda);
/* FIXME: hack in the old code, is this really necessary? */
@@ -173,15 +175,15 @@ static int i2c_gpio_probe(struct platform_device *pdev)
bit_data->data = priv;
adap->owner = THIS_MODULE;
- if (pdev->dev.of_node)
- strlcpy(adap->name, dev_name(&pdev->dev), sizeof(adap->name));
+ if (np)
+ strlcpy(adap->name, dev_name(dev), sizeof(adap->name));
else
snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);
adap->algo_data = bit_data;
adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
- adap->dev.parent = &pdev->dev;
- adap->dev.of_node = pdev->dev.of_node;
+ adap->dev.parent = dev;
+ adap->dev.of_node = np;
adap->nr = pdev->id;
ret = i2c_bit_add_numbered_bus(adap);
@@ -195,7 +197,7 @@ static int i2c_gpio_probe(struct platform_device *pdev)
* get accessors to get the actual name of the GPIO line,
* from the descriptor, then provide that instead.
*/
- dev_info(&pdev->dev, "using lines %u (SDA) and %u (SCL%s)\n",
+ dev_info(dev, "using lines %u (SDA) and %u (SCL%s)\n",
desc_to_gpio(priv->sda), desc_to_gpio(priv->scl),
pdata->scl_is_output_only
? ", no clock stretching" : "");
--
2.13.5
|
|
From: Linus Walleij <linus.walleij@li...> - 2017-09-10 22:11:28
|
I recently looked at this driver when testing the I2C GPIO on the Gemini platform. It's one of the archectypical places in the kernel where we have open coded open drain emulation, also without much explanation. So I went in and fixed it. The fix is pretty brutal changing all boards and one MFD device using this, but I like the end result, making the code much more readable and skipping the intermediate step of looping through the old GPIO API. It would be nice to get some testing and ACKs on this if people agree. I imagine Wolfram would use the whole thing into the I2C tree but I could also carry a branch in GPIO to be merged through the GPIO tree if it is preferred (like e.g. Wolfram want me to get the heat for any regressions, hehe). Linus Walleij (5): 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 arch/arm/mach-ep93xx/core.c | 41 +++--- arch/arm/mach-ep93xx/edb93xx.c | 15 +-- arch/arm/mach-ep93xx/include/mach/platform.h | 4 +- arch/arm/mach-ep93xx/simone.c | 12 +- arch/arm/mach-ep93xx/snappercl15.c | 12 +- arch/arm/mach-ep93xx/vision_ep9307.c | 7 +- arch/arm/mach-ixp4xx/avila-setup.c | 17 ++- arch/arm/mach-ixp4xx/dsmg600-setup.c | 16 ++- arch/arm/mach-ixp4xx/fsg-setup.c | 16 ++- arch/arm/mach-ixp4xx/goramo_mlr.c | 24 +--- arch/arm/mach-ixp4xx/ixdp425-setup.c | 16 ++- arch/arm/mach-ixp4xx/nas100d-setup.c | 16 ++- arch/arm/mach-ixp4xx/nslu2-setup.c | 16 ++- arch/arm/mach-ks8695/board-acs5k.c | 15 ++- arch/arm/mach-pxa/palmz72.c | 14 +- arch/arm/mach-pxa/viper.c | 27 +++- arch/arm/mach-sa1100/simpad.c | 14 +- arch/blackfin/mach-bf533/boards/blackstamp.c | 19 ++- arch/blackfin/mach-bf533/boards/ezkit.c | 18 ++- arch/blackfin/mach-bf533/boards/stamp.c | 18 ++- arch/blackfin/mach-bf561/boards/ezkit.c | 18 ++- arch/mips/alchemy/board-gpr.c | 23 +++- arch/mips/ath79/mach-pb44.c | 16 ++- drivers/gpio/gpiolib.c | 13 ++ drivers/i2c/busses/i2c-gpio.c | 184 +++++++++++---------------- drivers/mfd/sm501.c | 49 +++---- include/linux/gpio/consumer.h | 6 + include/linux/i2c-gpio.h | 4 - 28 files changed, 372 insertions(+), 278 deletions(-) -- 2.13.5 |
|
From: Linus Walleij <linus.walleij@li...> - 2017-09-10 22:07:20
|
Some busses, like I2C, strictly need to have the line handled
as open drain, i.e. not actively driven high. For this reason
the i2c-gpio.c bit-banged I2C driver is reimplementing open
drain handling outside of gpiolib.
This is not very optimal. Instead make it possible for a
consumer to explcitly express that the line must be handled
as open drain instead of allowing local hacks papering over
this issue.
The descriptor tables, whether DT, ACPI or board files, should
of course have flagged these lines as open drain. E.g.:
enum gpio_lookup_flags GPIO_OPEN_DRAIN for a board file, or
gpios = <&foo 42 GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN>; in a
device tree using <dt-bindings/gpio/gpio.h>
But more often than not, these descriptors are wrong. So
we need to make it possible for consumers to enforce this
open drain behaviour.
We now have two new enumerated GPIO descriptor config flags:
GPIOD_OUT_LOW_OPEN_DRAIN and GPIOD_OUT_HIGH_OPEN_DRAIN
that will set up the lined enforced as open drain as output
low or high, using open drain (if the driver supports it)
or using open drain emulation (setting the line as input
to drive it high) from the gpiolib core.
Cc: linux-gpio@...
Signed-off-by: Linus Walleij <linus.walleij@...>
---
Obviously I authorize this patch to be applied directly
to the I2C tree as part of the refactorings.
---
drivers/gpio/gpiolib.c | 13 +++++++++++++
include/linux/gpio/consumer.h | 6 ++++++
2 files changed, 19 insertions(+)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index cd003b74512f..cb73a50a5d5d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3228,8 +3228,21 @@ int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
if (lflags & GPIO_ACTIVE_LOW)
set_bit(FLAG_ACTIVE_LOW, &desc->flags);
+
if (lflags & GPIO_OPEN_DRAIN)
set_bit(FLAG_OPEN_DRAIN, &desc->flags);
+ else if (dflags & GPIOD_FLAGS_BIT_OPEN_DRAIN) {
+ /*
+ * This enforces open drain mode from the consumer side.
+ * This is necessary for some busses like I2C, but the lookup
+ * should *REALLY* have specified them as open drain in the
+ * first place, so print a little warning here.
+ */
+ set_bit(FLAG_OPEN_DRAIN, &desc->flags);
+ gpiod_warn(desc,
+ "enforced open drain please flag it properly in DT/ACPI DSDT/board file\n");
+ }
+
if (lflags & GPIO_OPEN_SOURCE)
set_bit(FLAG_OPEN_SOURCE, &desc->flags);
if (lflags & GPIO_SLEEP_MAY_LOOSE_VALUE)
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 8f702fcbe485..5f72a49d1aa3 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -28,6 +28,7 @@ struct gpio_descs {
#define GPIOD_FLAGS_BIT_DIR_SET BIT(0)
#define GPIOD_FLAGS_BIT_DIR_OUT BIT(1)
#define GPIOD_FLAGS_BIT_DIR_VAL BIT(2)
+#define GPIOD_FLAGS_BIT_OPEN_DRAIN BIT(3)
/**
* Optional flags that can be passed to one of gpiod_* to configure direction
@@ -39,6 +40,11 @@ enum gpiod_flags {
GPIOD_OUT_LOW = GPIOD_FLAGS_BIT_DIR_SET | GPIOD_FLAGS_BIT_DIR_OUT,
GPIOD_OUT_HIGH = GPIOD_FLAGS_BIT_DIR_SET | GPIOD_FLAGS_BIT_DIR_OUT |
GPIOD_FLAGS_BIT_DIR_VAL,
+ GPIOD_OUT_LOW_OPEN_DRAIN = GPIOD_FLAGS_BIT_DIR_SET |
+ GPIOD_FLAGS_BIT_DIR_OUT | GPIOD_FLAGS_BIT_OPEN_DRAIN,
+ GPIOD_OUT_HIGH_OPEN_DRAIN = GPIOD_FLAGS_BIT_DIR_SET |
+ GPIOD_FLAGS_BIT_DIR_OUT | GPIOD_FLAGS_BIT_DIR_VAL |
+ GPIOD_FLAGS_BIT_OPEN_DRAIN,
};
#ifdef CONFIG_GPIOLIB
--
2.13.5
|