* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-09-06 12:24 ` [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only Linus Walleij
@ 2018-09-10 17:01 ` Janusz Krzysztofik
2018-09-11 16:06 ` Mike Rapoport
` (3 subsequent siblings)
4 siblings, 0 replies; 28+ messages in thread
From: Janusz Krzysztofik @ 2018-09-10 17:01 UTC (permalink / raw)
To: Linus Walleij
Cc: Liam Girdwood, Mark Brown, linux-kernel, Alexander Shiyan,
Haojian Zhuang, Aaro Koskinen, Mike Rapoport, Robert Jarzmik,
Philipp Zabel, Daniel Mack, Marc Zyngier, Jacopo Mondi,
Geert Uytterhoeven, Russell King, Janusz Krzysztofik
Hi Linus,
On Thursday, September 6, 2018 2:24:36 PM CEST Linus Walleij wrote:
> As we augmented the regulator core to accept a GPIO descriptor instead
> of a GPIO number, we can augment the fixed GPIO regulator to look up
> and pass that descriptor directly from device tree or board GPIO
> descriptor look up tables.
>
> Some boards just auto-enumerate their fixed regulator platform devices
> and I have assumed they get names like "fixed-regulator.0" but it's
> pretty hard to guess this. I need some testing from board maintainers to
> be sure. Other boards are straight forward, using just plain
> "fixed-regulator" (ID -1) or "fixed-regulator.1" hammering down the
> device ID.
>
> It seems the da9055 and da9211 has never got around to actually passing
> any enable gpio into its platform data (not the in-tree code anyway) so we
> can just decide to simply pass a descriptor instead.
>
> The fixed GPIO-controlled regulator in mach-pxa/ezx.c was confusingly named
> "*_dummy_supply_device" while it is a very real device backed by a GPIO
> line. There is nothing dummy about it at all, so I renamed it with the
> infix *_regulator_* as part of this patch set.
>
> Intel MID portions tested by Andy.
>
> Cc: Janusz Krzysztofik <jmkrzyszt@gmail.com> # OMAP1
> Cc: Alexander Shiyan <shc_work@mail.ru> # i.MX boards user
> Cc: Haojian Zhuang <haojian.zhuang@gmail.com> # MMP2 maintainer
> Cc: Aaro Koskinen <aaro.koskinen@iki.fi> # OMAP1 maintainer
> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com> # EM-X270 maintainer
> Cc: Robert Jarzmik <robert.jarzmik@free.fr> # EZX maintainer
> Cc: Philipp Zabel <philipp.zabel@gmail.com> # Magician maintainer
> Cc: Daniel Mack <zonque@gmail.com> # Raumfeld maintainer
> Cc: Marc Zyngier <marc.zyngier@arm.com> # Zeus maintainer
> Cc: Jacopo Mondi <jacopo@jmondi.org> # SH Ecovec24
> Cc: Geert Uytterhoeven <geert+renesas@glider.be> # SuperH pinctrl/GPIO maintainer
> Cc: Russell King <rmk+kernel@armlinux.org.uk> # SA1100
> Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> # Check the x86 BCM stuff
> Acked-by: Tony Lindgren <tony@atomide.com> # OMAP1,2,3 maintainer
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v6->v7:
> - As the autobuilder churned along, after 24+ hours it was testing
> SH and found a bug in the ecovec24 boardfile. It does test a
> SH arch byt default, sh7763rdp_defconfig, sadly not this one.
> So it discovers more obscure boards in later testing.
> - Shaked out this bug too and re-pushed and posted.
> ChangeLog v5->v6:
> - New code appeared in the OMAP1 AMS delta board that added a
> new user of the removed .gpio member. The build robot was first
> happy, then came back later and was not happy.
> - Fixed up the offending .gpio, now rebuilt for this OMAP1
> board specifically to make sure it really really work now.
> ChangeLog v4->v5:
> - Rebased on v4.19-rc1
> - Put the OMAP1 AMD delta GPIO table addition in the *TOP*
> of the ams_delta_gpio_tables[] so Janusz can add any
> new addtions on the *BOTTOM*
> - Hopefully we can merge this now.
> ChangeLog v3->v4:
> - Rebase and adapt the OMAP1 changes for the GPIO descriptor
> look-up tables deployed by Janusz.
> - Add two calls to add the GPIO descriptor tables properly on
> the Super-H Ecovec24 board as pointed out by Geert.
> - Go over all patches to board files and make sure we pass
> a NULL descriptor instead of an "enable" descriptor. The code
> is looking for unnamed GPIOs as the device tree also just pass
> gpio[s] = <&foo> so board files also need to use anonymous
> GPIOs.
> - Fold in an EZX fix from Arnd Bergmann.
> - Add Andy's Tested-by tag.
> - Send this patch *ALONE* as I realized I need to take smaller
> steps so things do not blow up left and right.
> ChangeLog v2->v3:
> - Resending.
> ChangeLog v1->v2:
> - Rebase the patch on mainline with Blackfin gone and other changes.
> - Fix up the new users that appeared in sa1100
> - Drop some suplus comments in x86.
> ---
> arch/arm/mach-imx/mach-mx21ads.c | 12 ++++++-
> arch/arm/mach-imx/mach-mx27ads.c | 12 ++++++-
> arch/arm/mach-mmp/brownstone.c | 12 ++++++-
> arch/arm/mach-omap1/board-ams-delta.c | 12 +++++--
> arch/arm/mach-omap2/pdata-quirks.c | 16 ++++++++-
> arch/arm/mach-pxa/em-x270.c | 1 -
> arch/arm/mach-pxa/ezx.c | 33 ++++++++++++-------
> arch/arm/mach-pxa/magician.c | 2 +-
> arch/arm/mach-pxa/raumfeld.c | 12 +++++--
> arch/arm/mach-pxa/zeus.c | 23 +++++++++++--
> arch/arm/mach-s3c64xx/mach-crag6410.c | 1 -
> arch/arm/mach-s3c64xx/mach-smdk6410.c | 1 -
> arch/arm/mach-sa1100/assabet.c | 21 ++++++++----
> arch/arm/mach-sa1100/generic.c | 5 +--
> arch/arm/mach-sa1100/generic.h | 3 +-
> arch/arm/mach-sa1100/shannon.c | 4 +--
> arch/sh/boards/mach-ecovec24/setup.c | 27 +++++++++++++--
> .../intel-mid/device_libs/platform_bcm43xx.c | 17 ++++++++--
> drivers/regulator/fixed-helper.c | 1 -
> drivers/regulator/fixed.c | 33 +++++++++----------
> include/linux/regulator/fixed.h | 3 --
> 21 files changed, 188 insertions(+), 63 deletions(-)
For arch/arm/mach-omap1/board-ams-delta.c:
Reviewed-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Thanks,
Janusz
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-09-06 12:24 ` [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only Linus Walleij
2018-09-10 17:01 ` Janusz Krzysztofik
@ 2018-09-11 16:06 ` Mike Rapoport
2018-09-28 23:32 ` John Stultz
` (2 subsequent siblings)
4 siblings, 0 replies; 28+ messages in thread
From: Mike Rapoport @ 2018-09-11 16:06 UTC (permalink / raw)
To: Linus Walleij
Cc: Liam Girdwood, Mark Brown, linux-kernel, Janusz Krzysztofik,
Alexander Shiyan, Haojian Zhuang, Aaro Koskinen, Robert Jarzmik,
Philipp Zabel, Daniel Mack, Marc Zyngier, Jacopo Mondi,
Geert Uytterhoeven, Russell King
On Thu, Sep 06, 2018 at 02:24:36PM +0200, Linus Walleij wrote:
> As we augmented the regulator core to accept a GPIO descriptor instead
> of a GPIO number, we can augment the fixed GPIO regulator to look up
> and pass that descriptor directly from device tree or board GPIO
> descriptor look up tables.
>
> Some boards just auto-enumerate their fixed regulator platform devices
> and I have assumed they get names like "fixed-regulator.0" but it's
> pretty hard to guess this. I need some testing from board maintainers to
> be sure. Other boards are straight forward, using just plain
> "fixed-regulator" (ID -1) or "fixed-regulator.1" hammering down the
> device ID.
>
> It seems the da9055 and da9211 has never got around to actually passing
> any enable gpio into its platform data (not the in-tree code anyway) so we
> can just decide to simply pass a descriptor instead.
>
> The fixed GPIO-controlled regulator in mach-pxa/ezx.c was confusingly named
> "*_dummy_supply_device" while it is a very real device backed by a GPIO
> line. There is nothing dummy about it at all, so I renamed it with the
> infix *_regulator_* as part of this patch set.
>
> Intel MID portions tested by Andy.
>
> Cc: Janusz Krzysztofik <jmkrzyszt@gmail.com> # OMAP1
> Cc: Alexander Shiyan <shc_work@mail.ru> # i.MX boards user
> Cc: Haojian Zhuang <haojian.zhuang@gmail.com> # MMP2 maintainer
> Cc: Aaro Koskinen <aaro.koskinen@iki.fi> # OMAP1 maintainer
> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com> # EM-X270 maintainer
> Cc: Robert Jarzmik <robert.jarzmik@free.fr> # EZX maintainer
> Cc: Philipp Zabel <philipp.zabel@gmail.com> # Magician maintainer
> Cc: Daniel Mack <zonque@gmail.com> # Raumfeld maintainer
> Cc: Marc Zyngier <marc.zyngier@arm.com> # Zeus maintainer
> Cc: Jacopo Mondi <jacopo@jmondi.org> # SH Ecovec24
> Cc: Geert Uytterhoeven <geert+renesas@glider.be> # SuperH pinctrl/GPIO maintainer
> Cc: Russell King <rmk+kernel@armlinux.org.uk> # SA1100
> Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> # Check the x86 BCM stuff
> Acked-by: Tony Lindgren <tony@atomide.com> # OMAP1,2,3 maintainer
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
For EM-X270 part
Reviewed-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> ---
> ChangeLog v6->v7:
> - As the autobuilder churned along, after 24+ hours it was testing
> SH and found a bug in the ecovec24 boardfile. It does test a
> SH arch byt default, sh7763rdp_defconfig, sadly not this one.
> So it discovers more obscure boards in later testing.
> - Shaked out this bug too and re-pushed and posted.
> ChangeLog v5->v6:
> - New code appeared in the OMAP1 AMS delta board that added a
> new user of the removed .gpio member. The build robot was first
> happy, then came back later and was not happy.
> - Fixed up the offending .gpio, now rebuilt for this OMAP1
> board specifically to make sure it really really work now.
> ChangeLog v4->v5:
> - Rebased on v4.19-rc1
> - Put the OMAP1 AMD delta GPIO table addition in the *TOP*
> of the ams_delta_gpio_tables[] so Janusz can add any
> new addtions on the *BOTTOM*
> - Hopefully we can merge this now.
> ChangeLog v3->v4:
> - Rebase and adapt the OMAP1 changes for the GPIO descriptor
> look-up tables deployed by Janusz.
> - Add two calls to add the GPIO descriptor tables properly on
> the Super-H Ecovec24 board as pointed out by Geert.
> - Go over all patches to board files and make sure we pass
> a NULL descriptor instead of an "enable" descriptor. The code
> is looking for unnamed GPIOs as the device tree also just pass
> gpio[s] = <&foo> so board files also need to use anonymous
> GPIOs.
> - Fold in an EZX fix from Arnd Bergmann.
> - Add Andy's Tested-by tag.
> - Send this patch *ALONE* as I realized I need to take smaller
> steps so things do not blow up left and right.
> ChangeLog v2->v3:
> - Resending.
> ChangeLog v1->v2:
> - Rebase the patch on mainline with Blackfin gone and other changes.
> - Fix up the new users that appeared in sa1100
> - Drop some suplus comments in x86.
> ---
> arch/arm/mach-imx/mach-mx21ads.c | 12 ++++++-
> arch/arm/mach-imx/mach-mx27ads.c | 12 ++++++-
> arch/arm/mach-mmp/brownstone.c | 12 ++++++-
> arch/arm/mach-omap1/board-ams-delta.c | 12 +++++--
> arch/arm/mach-omap2/pdata-quirks.c | 16 ++++++++-
> arch/arm/mach-pxa/em-x270.c | 1 -
> arch/arm/mach-pxa/ezx.c | 33 ++++++++++++-------
> arch/arm/mach-pxa/magician.c | 2 +-
> arch/arm/mach-pxa/raumfeld.c | 12 +++++--
> arch/arm/mach-pxa/zeus.c | 23 +++++++++++--
> arch/arm/mach-s3c64xx/mach-crag6410.c | 1 -
> arch/arm/mach-s3c64xx/mach-smdk6410.c | 1 -
> arch/arm/mach-sa1100/assabet.c | 21 ++++++++----
> arch/arm/mach-sa1100/generic.c | 5 +--
> arch/arm/mach-sa1100/generic.h | 3 +-
> arch/arm/mach-sa1100/shannon.c | 4 +--
> arch/sh/boards/mach-ecovec24/setup.c | 27 +++++++++++++--
> .../intel-mid/device_libs/platform_bcm43xx.c | 17 ++++++++--
> drivers/regulator/fixed-helper.c | 1 -
> drivers/regulator/fixed.c | 33 +++++++++----------
> include/linux/regulator/fixed.h | 3 --
> 21 files changed, 188 insertions(+), 63 deletions(-)
>
> diff --git a/arch/arm/mach-imx/mach-mx21ads.c b/arch/arm/mach-imx/mach-mx21ads.c
> index 5e366824814f..2e1e540f2e5a 100644
> --- a/arch/arm/mach-imx/mach-mx21ads.c
> +++ b/arch/arm/mach-imx/mach-mx21ads.c
> @@ -18,6 +18,7 @@
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/physmap.h>
> #include <linux/gpio/driver.h>
> +#include <linux/gpio/machine.h>
> #include <linux/gpio.h>
> #include <linux/regulator/fixed.h>
> #include <linux/regulator/machine.h>
> @@ -175,6 +176,7 @@ static struct resource mx21ads_mmgpio_resource =
> DEFINE_RES_MEM_NAMED(MX21ADS_IO_REG, SZ_2, "dat");
>
> static struct bgpio_pdata mx21ads_mmgpio_pdata = {
> + .label = "mx21ads-mmgpio",
> .base = MX21ADS_MMGPIO_BASE,
> .ngpio = 16,
> };
> @@ -203,7 +205,6 @@ static struct regulator_init_data mx21ads_lcd_regulator_init_data = {
> static struct fixed_voltage_config mx21ads_lcd_regulator_pdata = {
> .supply_name = "LCD",
> .microvolts = 3300000,
> - .gpio = MX21ADS_IO_LCDON,
> .enable_high = 1,
> .init_data = &mx21ads_lcd_regulator_init_data,
> };
> @@ -216,6 +217,14 @@ static struct platform_device mx21ads_lcd_regulator = {
> },
> };
>
> +static struct gpiod_lookup_table mx21ads_lcd_regulator_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.0", /* Let's hope ID 0 is what we get */
> + .table = {
> + GPIO_LOOKUP("mx21ads-mmgpio", 9, NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> /*
> * Connected is a portrait Sharp-QVGA display
> * of type: LQ035Q7DB02
> @@ -311,6 +320,7 @@ static void __init mx21ads_late_init(void)
> {
> imx21_add_mxc_mmc(0, &mx21ads_sdhc_pdata);
>
> + gpiod_add_lookup_table(&mx21ads_lcd_regulator_gpiod_table);
> platform_add_devices(platform_devices, ARRAY_SIZE(platform_devices));
>
> mx21ads_cs8900_resources[1].start =
> diff --git a/arch/arm/mach-imx/mach-mx27ads.c b/arch/arm/mach-imx/mach-mx27ads.c
> index a04bb094ded1..f5e04047ed13 100644
> --- a/arch/arm/mach-imx/mach-mx27ads.c
> +++ b/arch/arm/mach-imx/mach-mx27ads.c
> @@ -16,6 +16,7 @@
> #include <linux/gpio/driver.h>
> /* Needed for gpio_to_irq() */
> #include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
> #include <linux/platform_device.h>
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/map.h>
> @@ -230,10 +231,17 @@ static struct regulator_init_data mx27ads_lcd_regulator_init_data = {
> static struct fixed_voltage_config mx27ads_lcd_regulator_pdata = {
> .supply_name = "LCD",
> .microvolts = 3300000,
> - .gpio = MX27ADS_LCD_GPIO,
> .init_data = &mx27ads_lcd_regulator_init_data,
> };
>
> +static struct gpiod_lookup_table mx27ads_lcd_regulator_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.0", /* Let's hope ID 0 is what we get */
> + .table = {
> + GPIO_LOOKUP("LCD", 0, NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> static void __init mx27ads_regulator_init(void)
> {
> struct gpio_chip *vchip;
> @@ -247,6 +255,8 @@ static void __init mx27ads_regulator_init(void)
> vchip->set = vgpio_set;
> gpiochip_add_data(vchip, NULL);
>
> + gpiod_add_lookup_table(&mx27ads_lcd_regulator_gpiod_table);
> +
> platform_device_register_data(NULL, "reg-fixed-voltage",
> PLATFORM_DEVID_AUTO,
> &mx27ads_lcd_regulator_pdata,
> diff --git a/arch/arm/mach-mmp/brownstone.c b/arch/arm/mach-mmp/brownstone.c
> index d1613b954926..a04e249c654b 100644
> --- a/arch/arm/mach-mmp/brownstone.c
> +++ b/arch/arm/mach-mmp/brownstone.c
> @@ -15,6 +15,7 @@
> #include <linux/platform_device.h>
> #include <linux/io.h>
> #include <linux/gpio-pxa.h>
> +#include <linux/gpio/machine.h>
> #include <linux/regulator/machine.h>
> #include <linux/regulator/max8649.h>
> #include <linux/regulator/fixed.h>
> @@ -148,7 +149,6 @@ static struct regulator_init_data brownstone_v_5vp_data = {
> static struct fixed_voltage_config brownstone_v_5vp = {
> .supply_name = "v_5vp",
> .microvolts = 5000000,
> - .gpio = GPIO_5V_ENABLE,
> .enable_high = 1,
> .enabled_at_boot = 1,
> .init_data = &brownstone_v_5vp_data,
> @@ -162,6 +162,15 @@ static struct platform_device brownstone_v_5vp_device = {
> },
> };
>
> +static struct gpiod_lookup_table brownstone_v_5vp_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.1", /* .id set to 1 above */
> + .table = {
> + GPIO_LOOKUP("gpio-pxa", GPIO_5V_ENABLE,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> static struct max8925_platform_data brownstone_max8925_info = {
> .irq_base = MMP_NR_IRQS,
> };
> @@ -217,6 +226,7 @@ static void __init brownstone_init(void)
> mmp2_add_isram(&mmp2_isram_platdata);
>
> /* enable 5v regulator */
> + gpiod_add_lookup_table(&brownstone_v_5vp_gpiod_table);
> platform_device_register(&brownstone_v_5vp_device);
> }
>
> diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
> index dd28d2614d7f..f226973f3d8c 100644
> --- a/arch/arm/mach-omap1/board-ams-delta.c
> +++ b/arch/arm/mach-omap1/board-ams-delta.c
> @@ -300,7 +300,6 @@ static struct regulator_init_data modem_nreset_data = {
> static struct fixed_voltage_config modem_nreset_config = {
> .supply_name = "modem_nreset",
> .microvolts = 3300000,
> - .gpio = AMS_DELTA_GPIO_PIN_MODEM_NRESET,
> .startup_delay = 25000,
> .enable_high = 1,
> .enabled_at_boot = 1,
> @@ -315,6 +314,15 @@ static struct platform_device modem_nreset_device = {
> },
> };
>
> +static struct gpiod_lookup_table ams_delta_nreset_gpiod_table = {
> + .dev_id = "reg-fixed-voltage",
> + .table = {
> + GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_MODEM_NRESET,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> struct modem_private_data {
> struct regulator *regulator;
> };
> @@ -568,7 +576,6 @@ static struct regulator_init_data keybrd_pwr_initdata = {
> static struct fixed_voltage_config keybrd_pwr_config = {
> .supply_name = "keybrd_pwr",
> .microvolts = 5000000,
> - .gpio = AMS_DELTA_GPIO_PIN_KEYBRD_PWR,
> .enable_high = 1,
> .init_data = &keybrd_pwr_initdata,
> };
> @@ -602,6 +609,7 @@ static struct platform_device *ams_delta_devices[] __initdata = {
> };
>
> static struct gpiod_lookup_table *ams_delta_gpio_tables[] __initdata = {
> + &ams_delta_nreset_gpiod_table,
> &ams_delta_audio_gpio_table,
> &keybrd_pwr_gpio_table,
> &ams_delta_lcd_gpio_table,
> diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
> index 7f02743edbe4..d0f7a7cc70cb 100644
> --- a/arch/arm/mach-omap2/pdata-quirks.c
> +++ b/arch/arm/mach-omap2/pdata-quirks.c
> @@ -10,6 +10,7 @@
> #include <linux/clk.h>
> #include <linux/davinci_emac.h>
> #include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/of_platform.h>
> @@ -328,7 +329,6 @@ static struct regulator_init_data pandora_vmmc3 = {
> static struct fixed_voltage_config pandora_vwlan = {
> .supply_name = "vwlan",
> .microvolts = 1800000, /* 1.8V */
> - .gpio = PANDORA_WIFI_NRESET_GPIO,
> .startup_delay = 50000, /* 50ms */
> .enable_high = 1,
> .init_data = &pandora_vmmc3,
> @@ -342,6 +342,19 @@ static struct platform_device pandora_vwlan_device = {
> },
> };
>
> +static struct gpiod_lookup_table pandora_vwlan_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.1",
> + .table = {
> + /*
> + * As this is a low GPIO number it should be at the first
> + * GPIO bank.
> + */
> + GPIO_LOOKUP("gpio-0-31", PANDORA_WIFI_NRESET_GPIO,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> static void pandora_wl1251_init_card(struct mmc_card *card)
> {
> /*
> @@ -403,6 +416,7 @@ static void __init pandora_wl1251_init(void)
> static void __init omap3_pandora_legacy_init(void)
> {
> platform_device_register(&pandora_backlight);
> + gpiod_add_lookup_table(&pandora_vwlan_gpiod_table);
> platform_device_register(&pandora_vwlan_device);
> omap_hsmmc_init(pandora_mmc3);
> omap_hsmmc_late_init(pandora_mmc3);
> diff --git a/arch/arm/mach-pxa/em-x270.c b/arch/arm/mach-pxa/em-x270.c
> index 29be04c6cc48..b14c47a6ee6b 100644
> --- a/arch/arm/mach-pxa/em-x270.c
> +++ b/arch/arm/mach-pxa/em-x270.c
> @@ -986,7 +986,6 @@ static struct fixed_voltage_config camera_dummy_config = {
> .supply_name = "camera_vdd",
> .input_supply = "vcc cam",
> .microvolts = 2800000,
> - .gpio = -1,
> .enable_high = 0,
> .init_data = &camera_dummy_initdata,
> };
> diff --git a/arch/arm/mach-pxa/ezx.c b/arch/arm/mach-pxa/ezx.c
> index 2c90b58f347d..565965e9acc7 100644
> --- a/arch/arm/mach-pxa/ezx.c
> +++ b/arch/arm/mach-pxa/ezx.c
> @@ -21,6 +21,7 @@
> #include <linux/regulator/fixed.h>
> #include <linux/input.h>
> #include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
> #include <linux/gpio_keys.h>
> #include <linux/leds-lp3944.h>
> #include <linux/platform_data/i2c-pxa.h>
> @@ -698,31 +699,39 @@ static struct pxa27x_keypad_platform_data e2_keypad_platform_data = {
>
> #if defined(CONFIG_MACH_EZX_A780) || defined(CONFIG_MACH_EZX_A910)
> /* camera */
> -static struct regulator_consumer_supply camera_dummy_supplies[] = {
> +static struct regulator_consumer_supply camera_regulator_supplies[] = {
> REGULATOR_SUPPLY("vdd", "0-005d"),
> };
>
> -static struct regulator_init_data camera_dummy_initdata = {
> - .consumer_supplies = camera_dummy_supplies,
> - .num_consumer_supplies = ARRAY_SIZE(camera_dummy_supplies),
> +static struct regulator_init_data camera_regulator_initdata = {
> + .consumer_supplies = camera_regulator_supplies,
> + .num_consumer_supplies = ARRAY_SIZE(camera_regulator_supplies),
> .constraints = {
> .valid_ops_mask = REGULATOR_CHANGE_STATUS,
> },
> };
>
> -static struct fixed_voltage_config camera_dummy_config = {
> +static struct fixed_voltage_config camera_regulator_config = {
> .supply_name = "camera_vdd",
> .microvolts = 2800000,
> - .gpio = GPIO50_nCAM_EN,
> .enable_high = 0,
> - .init_data = &camera_dummy_initdata,
> + .init_data = &camera_regulator_initdata,
> };
>
> -static struct platform_device camera_supply_dummy_device = {
> +static struct platform_device camera_supply_regulator_device = {
> .name = "reg-fixed-voltage",
> .id = 1,
> .dev = {
> - .platform_data = &camera_dummy_config,
> + .platform_data = &camera_regulator_config,
> + },
> +};
> +
> +static struct gpiod_lookup_table camera_supply_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.1",
> + .table = {
> + GPIO_LOOKUP("gpio-pxa", GPIO50_nCAM_EN,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> },
> };
> #endif
> @@ -800,7 +809,7 @@ static struct i2c_board_info a780_i2c_board_info[] = {
>
> static struct platform_device *a780_devices[] __initdata = {
> &a780_gpio_keys,
> - &camera_supply_dummy_device,
> + &camera_supply_regulator_device,
> };
>
> static void __init a780_init(void)
> @@ -823,6 +832,7 @@ static void __init a780_init(void)
> if (a780_camera_init() == 0)
> pxa_set_camera_info(&a780_pxacamera_platform_data);
>
> + gpiod_add_lookup_table(&camera_supply_gpiod_table);
> pwm_add_table(ezx_pwm_lookup, ARRAY_SIZE(ezx_pwm_lookup));
> platform_add_devices(ARRAY_AND_SIZE(ezx_devices));
> platform_add_devices(ARRAY_AND_SIZE(a780_devices));
> @@ -1098,7 +1108,7 @@ static struct i2c_board_info __initdata a910_i2c_board_info[] = {
>
> static struct platform_device *a910_devices[] __initdata = {
> &a910_gpio_keys,
> - &camera_supply_dummy_device,
> + &camera_supply_regulator_device,
> };
>
> static void __init a910_init(void)
> @@ -1121,6 +1131,7 @@ static void __init a910_init(void)
> if (a910_camera_init() == 0)
> pxa_set_camera_info(&a910_pxacamera_platform_data);
>
> + gpiod_add_lookup_table(&camera_supply_gpiod_table);
> pwm_add_table(ezx_pwm_lookup, ARRAY_SIZE(ezx_pwm_lookup));
> platform_add_devices(ARRAY_AND_SIZE(ezx_devices));
> platform_add_devices(ARRAY_AND_SIZE(a910_devices));
> diff --git a/arch/arm/mach-pxa/magician.c b/arch/arm/mach-pxa/magician.c
> index c5325d1ae77b..14c0f80bc9e7 100644
> --- a/arch/arm/mach-pxa/magician.c
> +++ b/arch/arm/mach-pxa/magician.c
> @@ -18,6 +18,7 @@
> #include <linux/platform_device.h>
> #include <linux/delay.h>
> #include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
> #include <linux/gpio_keys.h>
> #include <linux/input.h>
> #include <linux/mfd/htc-pasic3.h>
> @@ -696,7 +697,6 @@ static struct regulator_init_data vads7846_regulator = {
> static struct fixed_voltage_config vads7846 = {
> .supply_name = "vads7846",
> .microvolts = 3300000, /* probably */
> - .gpio = -EINVAL,
> .startup_delay = 0,
> .init_data = &vads7846_regulator,
> };
> diff --git a/arch/arm/mach-pxa/raumfeld.c b/arch/arm/mach-pxa/raumfeld.c
> index 034345546f84..bd3c23ad6ce6 100644
> --- a/arch/arm/mach-pxa/raumfeld.c
> +++ b/arch/arm/mach-pxa/raumfeld.c
> @@ -886,7 +886,6 @@ static struct regulator_init_data audio_va_initdata = {
> static struct fixed_voltage_config audio_va_config = {
> .supply_name = "audio_va",
> .microvolts = 5000000,
> - .gpio = GPIO_AUDIO_VA_ENABLE,
> .enable_high = 1,
> .enabled_at_boot = 0,
> .init_data = &audio_va_initdata,
> @@ -900,6 +899,15 @@ static struct platform_device audio_va_device = {
> },
> };
>
> +static struct gpiod_lookup_table audio_va_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.0",
> + .table = {
> + GPIO_LOOKUP("gpio-pxa", GPIO_AUDIO_VA_ENABLE,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> /* Dummy supplies for Codec's VD/VLC */
>
> static struct regulator_consumer_supply audio_dummy_supplies[] = {
> @@ -918,7 +926,6 @@ static struct regulator_init_data audio_dummy_initdata = {
> static struct fixed_voltage_config audio_dummy_config = {
> .supply_name = "audio_vd",
> .microvolts = 3300000,
> - .gpio = -1,
> .init_data = &audio_dummy_initdata,
> };
>
> @@ -1033,6 +1040,7 @@ static void __init raumfeld_audio_init(void)
> else
> gpio_direction_output(GPIO_MCLK_RESET, 1);
>
> + gpiod_add_lookup_table(&audio_va_gpiod_table);
> platform_add_devices(ARRAY_AND_SIZE(audio_regulator_devices));
> }
>
> diff --git a/arch/arm/mach-pxa/zeus.c b/arch/arm/mach-pxa/zeus.c
> index e3851795d6d7..d53ea12fc766 100644
> --- a/arch/arm/mach-pxa/zeus.c
> +++ b/arch/arm/mach-pxa/zeus.c
> @@ -17,6 +17,7 @@
> #include <linux/irq.h>
> #include <linux/pm.h>
> #include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
> #include <linux/serial_8250.h>
> #include <linux/dm9000.h>
> #include <linux/mmc/host.h>
> @@ -410,7 +411,6 @@ static struct regulator_init_data can_regulator_init_data = {
> static struct fixed_voltage_config can_regulator_pdata = {
> .supply_name = "CAN_SHDN",
> .microvolts = 3300000,
> - .gpio = ZEUS_CAN_SHDN_GPIO,
> .init_data = &can_regulator_init_data,
> };
>
> @@ -422,6 +422,15 @@ static struct platform_device can_regulator_device = {
> },
> };
>
> +static struct gpiod_lookup_table can_regulator_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.0",
> + .table = {
> + GPIO_LOOKUP("gpio-pxa", ZEUS_CAN_SHDN_GPIO,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> static struct mcp251x_platform_data zeus_mcp2515_pdata = {
> .oscillator_frequency = 16*1000*1000,
> };
> @@ -538,7 +547,6 @@ static struct regulator_init_data zeus_ohci_regulator_data = {
> static struct fixed_voltage_config zeus_ohci_regulator_config = {
> .supply_name = "vbus2",
> .microvolts = 5000000, /* 5.0V */
> - .gpio = ZEUS_USB2_PWREN_GPIO,
> .enable_high = 1,
> .startup_delay = 0,
> .init_data = &zeus_ohci_regulator_data,
> @@ -552,6 +560,15 @@ static struct platform_device zeus_ohci_regulator_device = {
> },
> };
>
> +static struct gpiod_lookup_table zeus_ohci_regulator_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.0",
> + .table = {
> + GPIO_LOOKUP("gpio-pxa", ZEUS_USB2_PWREN_GPIO,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> static struct pxaohci_platform_data zeus_ohci_platform_data = {
> .port_mode = PMM_NPS_MODE,
> /* Clear Power Control Polarity Low and set Power Sense
> @@ -855,6 +872,8 @@ static void __init zeus_init(void)
>
> pxa2xx_mfp_config(ARRAY_AND_SIZE(zeus_pin_config));
>
> + gpiod_add_lookup_table(&can_regulator_gpiod_table);
> + gpiod_add_lookup_table(&zeus_ohci_regulator_gpiod_table);
> platform_add_devices(zeus_devices, ARRAY_SIZE(zeus_devices));
>
> zeus_register_ohci();
> diff --git a/arch/arm/mach-s3c64xx/mach-crag6410.c b/arch/arm/mach-s3c64xx/mach-crag6410.c
> index f04650297487..379424d72ae7 100644
> --- a/arch/arm/mach-s3c64xx/mach-crag6410.c
> +++ b/arch/arm/mach-s3c64xx/mach-crag6410.c
> @@ -352,7 +352,6 @@ static struct fixed_voltage_config wallvdd_pdata = {
> .supply_name = "WALLVDD",
> .microvolts = 5000000,
> .init_data = &wallvdd_data,
> - .gpio = -EINVAL,
> };
>
> static struct platform_device wallvdd_device = {
> diff --git a/arch/arm/mach-s3c64xx/mach-smdk6410.c b/arch/arm/mach-s3c64xx/mach-smdk6410.c
> index c46fa5dfd2e0..908e5aa831c8 100644
> --- a/arch/arm/mach-s3c64xx/mach-smdk6410.c
> +++ b/arch/arm/mach-s3c64xx/mach-smdk6410.c
> @@ -222,7 +222,6 @@ static struct fixed_voltage_config smdk6410_b_pwr_5v_pdata = {
> .supply_name = "B_PWR_5V",
> .microvolts = 5000000,
> .init_data = &smdk6410_b_pwr_5v_data,
> - .gpio = -EINVAL,
> };
>
> static struct platform_device smdk6410_b_pwr_5v = {
> diff --git a/arch/arm/mach-sa1100/assabet.c b/arch/arm/mach-sa1100/assabet.c
> index 575ec085cffa..3e8c0948abcc 100644
> --- a/arch/arm/mach-sa1100/assabet.c
> +++ b/arch/arm/mach-sa1100/assabet.c
> @@ -101,7 +101,7 @@ static int __init assabet_init_gpio(void __iomem *reg, u32 def_val)
>
> assabet_bcr_gc = gc;
>
> - return gc->base;
> + return 0;
> }
>
> /*
> @@ -471,6 +471,14 @@ static struct fixed_voltage_config assabet_cf_vcc_pdata __initdata = {
> .enable_high = 1,
> };
>
> +static struct gpiod_lookup_table assabet_cf_vcc_gpio_table = {
> + .dev_id = "reg-fixed-voltage.0",
> + .table = {
> + GPIO_LOOKUP("assabet", 0, NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> static void __init assabet_init(void)
> {
> /*
> @@ -517,9 +525,11 @@ static void __init assabet_init(void)
> neponset_resources, ARRAY_SIZE(neponset_resources));
> #endif
> } else {
> + gpiod_add_lookup_table(&assabet_cf_vcc_gpio_table);
> sa11x0_register_fixed_regulator(0, &assabet_cf_vcc_pdata,
> - assabet_cf_vcc_consumers,
> - ARRAY_SIZE(assabet_cf_vcc_consumers));
> + assabet_cf_vcc_consumers,
> + ARRAY_SIZE(assabet_cf_vcc_consumers),
> + true);
>
> }
>
> @@ -802,7 +812,6 @@ fs_initcall(assabet_leds_init);
>
> void __init assabet_init_irq(void)
> {
> - unsigned int assabet_gpio_base;
> u32 def_val;
>
> sa1100_init_irq();
> @@ -817,9 +826,7 @@ void __init assabet_init_irq(void)
> *
> * This must precede any driver calls to BCR_set() or BCR_clear().
> */
> - assabet_gpio_base = assabet_init_gpio((void *)&ASSABET_BCR, def_val);
> -
> - assabet_cf_vcc_pdata.gpio = assabet_gpio_base + 0;
> + assabet_init_gpio((void *)&ASSABET_BCR, def_val);
> }
>
> MACHINE_START(ASSABET, "Intel-Assabet")
> diff --git a/arch/arm/mach-sa1100/generic.c b/arch/arm/mach-sa1100/generic.c
> index 7167ddf84a0e..800321c6cbd8 100644
> --- a/arch/arm/mach-sa1100/generic.c
> +++ b/arch/arm/mach-sa1100/generic.c
> @@ -348,7 +348,8 @@ void __init sa11x0_init_late(void)
>
> int __init sa11x0_register_fixed_regulator(int n,
> struct fixed_voltage_config *cfg,
> - struct regulator_consumer_supply *supplies, unsigned num_supplies)
> + struct regulator_consumer_supply *supplies, unsigned num_supplies,
> + bool uses_gpio)
> {
> struct regulator_init_data *id;
>
> @@ -356,7 +357,7 @@ int __init sa11x0_register_fixed_regulator(int n,
> if (!cfg->init_data)
> return -ENOMEM;
>
> - if (cfg->gpio < 0)
> + if (!uses_gpio)
> id->constraints.always_on = 1;
> id->constraints.name = cfg->supply_name;
> id->constraints.min_uV = cfg->microvolts;
> diff --git a/arch/arm/mach-sa1100/generic.h b/arch/arm/mach-sa1100/generic.h
> index 5f3cb52fa6ab..158a4fd5ca24 100644
> --- a/arch/arm/mach-sa1100/generic.h
> +++ b/arch/arm/mach-sa1100/generic.h
> @@ -54,4 +54,5 @@ void sa11x0_register_pcmcia(int socket, struct gpiod_lookup_table *);
> struct fixed_voltage_config;
> struct regulator_consumer_supply;
> int sa11x0_register_fixed_regulator(int n, struct fixed_voltage_config *cfg,
> - struct regulator_consumer_supply *supplies, unsigned num_supplies);
> + struct regulator_consumer_supply *supplies, unsigned num_supplies,
> + bool uses_gpio);
> diff --git a/arch/arm/mach-sa1100/shannon.c b/arch/arm/mach-sa1100/shannon.c
> index 22f7fe0b809f..5bc82e2671c6 100644
> --- a/arch/arm/mach-sa1100/shannon.c
> +++ b/arch/arm/mach-sa1100/shannon.c
> @@ -102,14 +102,14 @@ static struct fixed_voltage_config shannon_cf_vcc_pdata __initdata = {
> .supply_name = "cf-power",
> .microvolts = 3300000,
> .enabled_at_boot = 1,
> - .gpio = -EINVAL,
> };
>
> static void __init shannon_init(void)
> {
> sa11x0_register_fixed_regulator(0, &shannon_cf_vcc_pdata,
> shannon_cf_vcc_consumers,
> - ARRAY_SIZE(shannon_cf_vcc_consumers));
> + ARRAY_SIZE(shannon_cf_vcc_consumers),
> + false);
> sa11x0_register_pcmcia(0, &shannon_pcmcia0_gpio_table);
> sa11x0_register_pcmcia(1, &shannon_pcmcia1_gpio_table);
> sa11x0_ppc_configure_mcp();
> diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
> index adc61d14172c..06a894526a0b 100644
> --- a/arch/sh/boards/mach-ecovec24/setup.c
> +++ b/arch/sh/boards/mach-ecovec24/setup.c
> @@ -633,7 +633,6 @@ static struct regulator_init_data cn12_power_init_data = {
> static struct fixed_voltage_config cn12_power_info = {
> .supply_name = "CN12 SD/MMC Vdd",
> .microvolts = 3300000,
> - .gpio = GPIO_PTB7,
> .enable_high = 1,
> .init_data = &cn12_power_init_data,
> };
> @@ -646,6 +645,16 @@ static struct platform_device cn12_power = {
> },
> };
>
> +static struct gpiod_lookup_table cn12_power_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.0",
> + .table = {
> + /* Offset 7 on port B */
> + GPIO_LOOKUP("sh7724_pfc", GPIO_PTB7,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> #if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
> /* SDHI0 */
> static struct regulator_consumer_supply sdhi0_power_consumers[] =
> @@ -665,7 +674,6 @@ static struct regulator_init_data sdhi0_power_init_data = {
> static struct fixed_voltage_config sdhi0_power_info = {
> .supply_name = "CN11 SD/MMC Vdd",
> .microvolts = 3300000,
> - .gpio = GPIO_PTB6,
> .enable_high = 1,
> .init_data = &sdhi0_power_init_data,
> };
> @@ -678,6 +686,16 @@ static struct platform_device sdhi0_power = {
> },
> };
>
> +static struct gpiod_lookup_table sdhi0_power_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.1",
> + .table = {
> + /* Offset 6 on port B */
> + GPIO_LOOKUP("sh7724_pfc", GPIO_PTB6,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> static struct tmio_mmc_data sdhi0_info = {
> .chan_priv_tx = (void *)SHDMA_SLAVE_SDHI0_TX,
> .chan_priv_rx = (void *)SHDMA_SLAVE_SDHI0_RX,
> @@ -1413,6 +1431,11 @@ static int __init arch_setup(void)
> DMA_MEMORY_EXCLUSIVE);
> platform_device_add(ecovec_ceu_devices[1]);
>
> + gpiod_add_lookup_table(&cn12_power_gpiod_table);
> +#if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
> + gpiod_add_lookup_table(&sdhi0_power_gpiod_table);
> +#endif
> +
> return platform_add_devices(ecovec_devices,
> ARRAY_SIZE(ecovec_devices));
> }
> diff --git a/arch/x86/platform/intel-mid/device_libs/platform_bcm43xx.c b/arch/x86/platform/intel-mid/device_libs/platform_bcm43xx.c
> index 4392c15ed9e0..dbfc5cf2aa93 100644
> --- a/arch/x86/platform/intel-mid/device_libs/platform_bcm43xx.c
> +++ b/arch/x86/platform/intel-mid/device_libs/platform_bcm43xx.c
> @@ -10,7 +10,7 @@
> * of the License.
> */
>
> -#include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
> #include <linux/platform_device.h>
> #include <linux/regulator/machine.h>
> #include <linux/regulator/fixed.h>
> @@ -43,7 +43,6 @@ static struct fixed_voltage_config bcm43xx_vmmc = {
> * real voltage and signaling are still 1.8V.
> */
> .microvolts = 2000000, /* 1.8V */
> - .gpio = -EINVAL,
> .startup_delay = 250 * 1000, /* 250ms */
> .enable_high = 1, /* active high */
> .enabled_at_boot = 0, /* disabled at boot */
> @@ -58,11 +57,23 @@ static struct platform_device bcm43xx_vmmc_regulator = {
> },
> };
>
> +static struct gpiod_lookup_table bcm43xx_vmmc_gpio_table = {
> + .dev_id = "reg-fixed-voltage.0",
> + .table = {
> + GPIO_LOOKUP("0000:00:0c.0", -1, NULL, GPIO_ACTIVE_LOW),
> + {}
> + },
> +};
> +
> static int __init bcm43xx_regulator_register(void)
> {
> + struct gpiod_lookup_table *table = &bcm43xx_vmmc_gpio_table;
> + struct gpiod_lookup *lookup = table->table;
> int ret;
>
> - bcm43xx_vmmc.gpio = get_gpio_by_name(WLAN_SFI_GPIO_ENABLE_NAME);
> + lookup[0].chip_hwnum = get_gpio_by_name(WLAN_SFI_GPIO_ENABLE_NAME);
> + gpiod_add_lookup_table(table);
> +
> ret = platform_device_register(&bcm43xx_vmmc_regulator);
> if (ret) {
> pr_err("%s: vmmc regulator register failed\n", __func__);
> diff --git a/drivers/regulator/fixed-helper.c b/drivers/regulator/fixed-helper.c
> index 777fac6fb4cb..2c6098e6f4bc 100644
> --- a/drivers/regulator/fixed-helper.c
> +++ b/drivers/regulator/fixed-helper.c
> @@ -43,7 +43,6 @@ struct platform_device *regulator_register_always_on(int id, const char *name,
> }
>
> data->cfg.microvolts = uv;
> - data->cfg.gpio = -EINVAL;
> data->cfg.enabled_at_boot = 1;
> data->cfg.init_data = &data->init_data;
>
> diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
> index 988a7472c2ab..1142f195529b 100644
> --- a/drivers/regulator/fixed.c
> +++ b/drivers/regulator/fixed.c
> @@ -24,10 +24,9 @@
> #include <linux/platform_device.h>
> #include <linux/regulator/driver.h>
> #include <linux/regulator/fixed.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/slab.h>
> #include <linux/of.h>
> -#include <linux/of_gpio.h>
> #include <linux/regulator/of_regulator.h>
> #include <linux/regulator/machine.h>
>
> @@ -78,10 +77,6 @@ of_get_fixed_voltage_config(struct device *dev,
> if (init_data->constraints.boot_on)
> config->enabled_at_boot = true;
>
> - config->gpio = of_get_named_gpio(np, "gpio", 0);
> - if ((config->gpio < 0) && (config->gpio != -ENOENT))
> - return ERR_PTR(config->gpio);
> -
> of_property_read_u32(np, "startup-delay-us", &config->startup_delay);
>
> config->enable_high = of_property_read_bool(np, "enable-active-high");
> @@ -102,6 +97,7 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
> struct fixed_voltage_config *config;
> struct fixed_voltage_data *drvdata;
> struct regulator_config cfg = { };
> + enum gpiod_flags gflags;
> int ret;
>
> drvdata = devm_kzalloc(&pdev->dev, sizeof(struct fixed_voltage_data),
> @@ -150,25 +146,28 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
>
> drvdata->desc.fixed_uV = config->microvolts;
>
> - if (gpio_is_valid(config->gpio)) {
> - cfg.ena_gpio = config->gpio;
> - if (pdev->dev.of_node)
> - cfg.ena_gpio_initialized = true;
> - }
> cfg.ena_gpio_invert = !config->enable_high;
> if (config->enabled_at_boot) {
> if (config->enable_high)
> - cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
> + gflags = GPIOD_OUT_HIGH;
> else
> - cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
> + gflags = GPIOD_OUT_LOW;
> } else {
> if (config->enable_high)
> - cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
> + gflags = GPIOD_OUT_LOW;
> else
> - cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
> + gflags = GPIOD_OUT_HIGH;
> }
> - if (config->gpio_is_open_drain)
> - cfg.ena_gpio_flags |= GPIOF_OPEN_DRAIN;
> + if (config->gpio_is_open_drain) {
> + if (gflags == GPIOD_OUT_HIGH)
> + gflags = GPIOD_OUT_HIGH_OPEN_DRAIN;
> + else
> + gflags = GPIOD_OUT_LOW_OPEN_DRAIN;
> + }
> +
> + cfg.ena_gpiod = devm_gpiod_get_optional(&pdev->dev, NULL, gflags);
> + if (IS_ERR(cfg.ena_gpiod))
> + return PTR_ERR(cfg.ena_gpiod);
>
> cfg.dev = &pdev->dev;
> cfg.init_data = config->init_data;
> diff --git a/include/linux/regulator/fixed.h b/include/linux/regulator/fixed.h
> index 48918be649d4..1a4340ed8e2b 100644
> --- a/include/linux/regulator/fixed.h
> +++ b/include/linux/regulator/fixed.h
> @@ -24,8 +24,6 @@ struct regulator_init_data;
> * @supply_name: Name of the regulator supply
> * @input_supply: Name of the input regulator supply
> * @microvolts: Output voltage of regulator
> - * @gpio: GPIO to use for enable control
> - * set to -EINVAL if not used
> * @startup_delay: Start-up time in microseconds
> * @gpio_is_open_drain: Gpio pin is open drain or normal type.
> * If it is open drain type then HIGH will be set
> @@ -49,7 +47,6 @@ struct fixed_voltage_config {
> const char *supply_name;
> const char *input_supply;
> int microvolts;
> - int gpio;
> unsigned startup_delay;
> unsigned gpio_is_open_drain:1;
> unsigned enable_high:1;
> --
> 2.17.1
>
>
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-09-06 12:24 ` [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only Linus Walleij
2018-09-10 17:01 ` Janusz Krzysztofik
2018-09-11 16:06 ` Mike Rapoport
@ 2018-09-28 23:32 ` John Stultz
2018-09-29 17:38 ` Linus Walleij
2018-10-01 18:53 ` Leonard Crestez
2018-10-11 9:01 ` Marek Szyprowski
4 siblings, 1 reply; 28+ messages in thread
From: John Stultz @ 2018-09-28 23:32 UTC (permalink / raw)
To: linus.walleij, Anders Roxell
Cc: lgirdwood, broonie, Linux Kernel Mailing List, jmkrzyszt,
shc_work, haojian.zhuang, aaro.koskinen, rppt, robert.jarzmik,
philipp.zabel, zonque, marc.zyngier, jacopo, geert+renesas,
rmk+kernel, Tom Gall
On Thu, Sep 6, 2018 at 6:01 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> As we augmented the regulator core to accept a GPIO descriptor instead
> of a GPIO number, we can augment the fixed GPIO regulator to look up
> and pass that descriptor directly from device tree or board GPIO
> descriptor look up tables.
>
> Some boards just auto-enumerate their fixed regulator platform devices
> and I have assumed they get names like "fixed-regulator.0" but it's
> pretty hard to guess this. I need some testing from board maintainers to
> be sure. Other boards are straight forward, using just plain
> "fixed-regulator" (ID -1) or "fixed-regulator.1" hammering down the
> device ID.
Hey Linus,
Anders recently noted a regression in -next when running HiKey,
where USB fails to function. I took a look and could reproduce this
as well, and after some unsuccessful muddling about in the usb
changes, I bisected it down to your commit here (specifically
efdfeb079cc3 in -next).
I'm not sure exactly why this would cause trouble, but I suspect it
has something to do w/ the cable-detect OTG to host-hub switch on the
HiKey.
Anyway, I just wanted to raise this with you so it can get sorted out
before that patch hits mainline.
thanks
-john
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-09-28 23:32 ` John Stultz
@ 2018-09-29 17:38 ` Linus Walleij
0 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2018-09-29 17:38 UTC (permalink / raw)
To: John Stultz
Cc: Anders Roxell, Liam Girdwood, Mark Brown,
linux-kernel@vger.kernel.org, Janusz Krzysztofik,
Alexander Shiyan, Haojian Zhuang, Aaro Koskinen, Mike Rapoport,
Robert Jarzmik, Philipp Zabel, Daniel Mack, Marc Zyngier, jacopo,
Geert Uytterhoeven, Russell King, Tom Gall
On Sat, Sep 29, 2018 at 1:32 AM John Stultz <john.stultz@linaro.org> wrote:
> Anders recently noted a regression in -next when running HiKey,
> where USB fails to function. I took a look and could reproduce this
> as well, and after some unsuccessful muddling about in the usb
> changes, I bisected it down to your commit here (specifically
> efdfeb079cc3 in -next).
>
> I'm not sure exactly why this would cause trouble, but I suspect it
> has something to do w/ the cable-detect OTG to host-hub switch on the
> HiKey.
>
> Anyway, I just wanted to raise this with you so it can get sorted out
> before that patch hits mainline.
OK how typical, let's see if we can figure it out. I looked at it but
I can't see what it is right off.
I guess it is this from
arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts:
reg_5v_hub: regulator@2 {
compatible = "regulator-fixed";
regulator-name = "5V_HUB";
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
regulator-boot-on;
gpio = <&gpio0 7 0>;
regulator-always-on;
vin-supply = <®_sys_5v>;
};
It's a regulator with one GPIO.
It used to be fetched here:
+ cfg.ena_gpiod = devm_gpiod_get_optional(&pdev->dev, NULL, gflags);
+ if (IS_ERR(cfg.ena_gpiod))
+ return PTR_ERR(cfg.ena_gpiod);
So first would be to put in a print like this:
if (IS_ERR(cfg.ena_gpiod)) {
dev_info(&pdev->dev, "error fetching enable GPIO for %s, %d\n",
dev_name(&pdev->dev), PTR_ERR(cfg.ena_gpiod));
return PTR_ERR(cfg.ena_gpiod);
}
if (!cfg.ena_gpiod)
dev_info(&pdev->dev, "no enable GPIO for %s\n", dev_name(&pdev->dev));
else
dev_info(&pdev->dev, "fetched valid enable GPIO for %s\n",
dev_name(&pdev->dev));
So we make sure we get the enable GPIO handle.
Else we need to troubleshoot that.
Look if -ENOENT or -EPROBE_DEFER gets returned for example.
If those works we need to check the flags. Since this GPIO is specified
with gpio = <&gpio0 7 0>; it would nominally mean it is active high.
But there is special code in drivers/gpio/gpiolib-of.c to deal with
regulators, since those lines are by default active low and ignore
the flags in the second cell from the device tree.
In of_gpio_flags_quirks(), we force the GPIO to be active low
unless "enable-active-high" is set. So we need to look
in /sys/kernel/debug/gpio or just lsgpio to see if the active edge
is the same before/after the patch.
Sadly I don't have this board myself!
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-09-06 12:24 ` [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only Linus Walleij
` (2 preceding siblings ...)
2018-09-28 23:32 ` John Stultz
@ 2018-10-01 18:53 ` Leonard Crestez
2018-10-01 20:16 ` Linus Walleij
2018-10-11 9:01 ` Marek Szyprowski
4 siblings, 1 reply; 28+ messages in thread
From: Leonard Crestez @ 2018-10-01 18:53 UTC (permalink / raw)
To: broonie@kernel.org, linus.walleij@linaro.org
Cc: linux-kernel@vger.kernel.org, Andy Duan, Fabio Estevam,
linux-gpio@vger.kernel.org, lgirdwood@gmail.com,
shawnguo@kernel.org, Anson Huang, tony@atomide.com,
rppt@linux.vnet.ibm.com, jmkrzyszt@gmail.com,
john.stultz@linaro.org
Hello,
This patch seems to break network booting on imx6sx-sdb in linux-next
because the enet phy regulator is not on. Reverting the patch fixes
boot.
Here is the regulator definition:
reg_enet_3v3: regulator-enet-3v3 {
compatible = "regulator-fixed";
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_enet_3v3>;
regulator-name = "enet_3v3";
regulator-min-microvolt = <3300000>;
regulator-max-microvolt = <3300000>;
gpios = <&gpio2 6 GPIO_ACTIVE_LOW>;
};
Here are some prints with the patch enabled:
[ 0.153150] reg-fixed-voltage regulator-enet-3v3: OUT_HIGH gflags 0x7
[ 0.153171] reg_fixed_voltage_probe(179): regulator-enet-3v3 call gpiod_get_optional gflags=0x7 ena_gpio_invert
[ 0.153218] regulator-enet-3v3 GPIO handle specifies active low - ignored
[ 0.153233] of_gpio_flags_quirks(83): regulator-enet-3v3 set active low because !enable-active-high
[ 0.153258] of_get_named_gpiod_flags: parsed 'gpios' property of node '/regulator-enet-3v3[0]' - status (0)
[ 0.153310] gpio_value: 38 set 0
[ 0.153332] gpio_direction: 38 out (0)
[ 0.153364] enet_3v3: ena_gpiod=(ptrval) ena_gpio=0 init=0 valid=1
[ 0.153377] enet_3v3: request GPIO
[ 0.153393] enet_3v3: already have gpiod
[ 0.153423] enet_3v3: 3300 mV
...
[ 3.867827] enet_3v3: GPIO enable count=0 inv=1
[ 3.872432] enet_3v3: set value 0
[ 3.875779] gpio_value: 38 set 1
That "gpio_value: 30 set 1" tracepoint is wrong, the line is set high.
It seems that gpiod_set_value will check FLAG_ACTIVE_LOW and
automatically invert so maybe ena_gpio_invert should not be used if a
full gpiod is passed to regulator?
--- drivers/regulator/fixed.c
+++ drivers/regulator/fixed.c
@@ -146,7 +146,6 @@ static int reg_fixed_voltage_probe(struct
platform_device *pdev)
drvdata->desc.fixed_uV = config->microvolts;
- cfg.ena_gpio_invert = !config->enable_high;
if (config->enabled_at_boot) {
if (config->enable_high)
gflags = GPIOD_OUT_HIGH;
All these high/low inversions and flags are extremely confusing to me.
Link to original thread: https://lkml.org/lkml/2018/9/6/485
--
Regards,
Leonard
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-01 18:53 ` Leonard Crestez
@ 2018-10-01 20:16 ` Linus Walleij
2018-10-01 20:37 ` Fabio Estevam
0 siblings, 1 reply; 28+ messages in thread
From: Linus Walleij @ 2018-10-01 20:16 UTC (permalink / raw)
To: leonard.crestez, John Stultz
Cc: Mark Brown, linux-kernel@vger.kernel.org, Andy Duan,
Fabio Estevam, open list:GPIO SUBSYSTEM, Liam Girdwood, Shawn Guo,
Anson Huang, ext Tony Lindgren, Mike Rapoport, Janusz Krzysztofik
On Mon, Oct 1, 2018 at 8:53 PM Leonard Crestez <leonard.crestez@nxp.com> wrote:
> This patch seems to break network booting on imx6sx-sdb in linux-next
> because the enet phy regulator is not on. Reverting the patch fixes
> boot.
Thanks for reporting.
John Stultz reported the same problem I'm trying to debug it.
> Here is the regulator definition:
>
> reg_enet_3v3: regulator-enet-3v3 {
> compatible = "regulator-fixed";
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_enet_3v3>;
> regulator-name = "enet_3v3";
> regulator-min-microvolt = <3300000>;
> regulator-max-microvolt = <3300000>;
> gpios = <&gpio2 6 GPIO_ACTIVE_LOW>;
> };
This is a bit odd actually, the GPIO_ACTIVE_LOW flag will
be ignored as you see:
> [ 0.153171] reg_fixed_voltage_probe(179): regulator-enet-3v3 call gpiod_get_optional gflags=0x7 ena_gpio_invert
> [ 0.153218] regulator-enet-3v3 GPIO handle specifies active low - ignored
> [ 0.153233] of_gpio_flags_quirks(83): regulator-enet-3v3 set active low because !enable-active-high
Because regulators don't specify active high/low in the second
cell because of legacy bindings.
So this should not be in the device tree anyway, it should be
GPIO_ACTIVE_HIGH or just 0.
> That "gpio_value: 30 set 1" tracepoint is wrong, the line is set high.
>
> It seems that gpiod_set_value will check FLAG_ACTIVE_LOW and
> automatically invert
(...)
>so maybe ena_gpio_invert should not be used if a
> full gpiod is passed to regulator?
(...)
> - cfg.ena_gpio_invert = !config->enable_high;
Indeed. I will look closer so it's the right fix and provide a patch.
> All these high/low inversions and flags are extremely confusing to me.
Yeah it's what I'm trying to get rid of with these patches,
this is just the first patch in a series that move inversion over
to the GPIO library.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-01 20:16 ` Linus Walleij
@ 2018-10-01 20:37 ` Fabio Estevam
2018-10-01 20:48 ` Linus Walleij
0 siblings, 1 reply; 28+ messages in thread
From: Fabio Estevam @ 2018-10-01 20:37 UTC (permalink / raw)
To: Linus Walleij
Cc: Leonard Crestez, John Stultz, Mark Brown, linux-kernel,
Fugang Duan, Fabio Estevam, open list:GPIO SUBSYSTEM,
Liam Girdwood, Shawn Guo, Yongcai Huang, Tony Lindgren,
Mike Rapoport, jmkrzyszt
Hi Linus,
On Mon, Oct 1, 2018 at 5:17 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > reg_enet_3v3: regulator-enet-3v3 {
> > compatible = "regulator-fixed";
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_enet_3v3>;
> > regulator-name = "enet_3v3";
> > regulator-min-microvolt = <3300000>;
> > regulator-max-microvolt = <3300000>;
> > gpios = <&gpio2 6 GPIO_ACTIVE_LOW>;
> > };
>
> This is a bit odd actually, the GPIO_ACTIVE_LOW flag will
> be ignored as you see:
Yes, the flag will be ignored by the regulator driver, but the dts
description is correct: it is an active low GPIO that turns on the
reg_enet_3v3 regulator.
The 'enable-active-high' flag needs to be passed to indicate an active
high polarity.
> > [ 0.153171] reg_fixed_voltage_probe(179): regulator-enet-3v3 call gpiod_get_optional gflags=0x7 ena_gpio_invert
> > [ 0.153218] regulator-enet-3v3 GPIO handle specifies active low - ignored
> > [ 0.153233] of_gpio_flags_quirks(83): regulator-enet-3v3 set active low because !enable-active-high
>
> Because regulators don't specify active high/low in the second
> cell because of legacy bindings.
>
> So this should not be in the device tree anyway, it should be
> GPIO_ACTIVE_HIGH or just 0.
Then it would provide a wrong description that does not describe the reality.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-01 20:37 ` Fabio Estevam
@ 2018-10-01 20:48 ` Linus Walleij
0 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2018-10-01 20:48 UTC (permalink / raw)
To: Fabio Estevam
Cc: leonard.crestez, John Stultz, Mark Brown,
linux-kernel@vger.kernel.org, Andy Duan, Fabio Estevam,
open list:GPIO SUBSYSTEM, Liam Girdwood, Shawn Guo, Anson Huang,
ext Tony Lindgren, Mike Rapoport, Janusz Krzysztofik
On Mon, Oct 1, 2018 at 10:36 PM Fabio Estevam <festevam@gmail.com> wrote:
> On Mon, Oct 1, 2018 at 5:17 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> > > reg_enet_3v3: regulator-enet-3v3 {
> > > compatible = "regulator-fixed";
> > > pinctrl-names = "default";
> > > pinctrl-0 = <&pinctrl_enet_3v3>;
> > > regulator-name = "enet_3v3";
> > > regulator-min-microvolt = <3300000>;
> > > regulator-max-microvolt = <3300000>;
> > > gpios = <&gpio2 6 GPIO_ACTIVE_LOW>;
> > > };
> >
> > This is a bit odd actually, the GPIO_ACTIVE_LOW flag will
> > be ignored as you see:
>
> Yes, the flag will be ignored by the regulator driver, but the dts
> description is correct: it is an active low GPIO that turns on the
> reg_enet_3v3 regulator.
>
> The 'enable-active-high' flag needs to be passed to indicate an active
> high polarity.
Yes.
> > > [ 0.153171] reg_fixed_voltage_probe(179): regulator-enet-3v3 call gpiod_get_optional gflags=0x7 ena_gpio_invert
> > > [ 0.153218] regulator-enet-3v3 GPIO handle specifies active low - ignored
> > > [ 0.153233] of_gpio_flags_quirks(83): regulator-enet-3v3 set active low because !enable-active-high
> >
> > Because regulators don't specify active high/low in the second
> > cell because of legacy bindings.
> >
> > So this should not be in the device tree anyway, it should be
> > GPIO_ACTIVE_HIGH or just 0.
>
> Then it would provide a wrong description that does not describe the reality.
OK my bad, by all means keep it. :)
The warning message does not say the description is wrong, just that it will
be ignored, and it is there for the users to be aware of that setting
GPIO_ACTIVE_LOW will have no semantic effect.
We introduced it because we were worried that if we don't print that,
users will tend to think that their GPIO_ACTIVE_LOW flags are
respected, so it was the best we could think of.
The real problem, of course, is that the bindings are ambiguous with
the elder binding taking precedence. Sorry about that, we were young
and didn't know how to do it the right way. Lesson learnt.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-09-06 12:24 ` [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only Linus Walleij
` (3 preceding siblings ...)
2018-10-01 18:53 ` Leonard Crestez
@ 2018-10-11 9:01 ` Marek Szyprowski
2018-10-11 9:29 ` Linus Walleij
4 siblings, 1 reply; 28+ messages in thread
From: Marek Szyprowski @ 2018-10-11 9:01 UTC (permalink / raw)
To: Linus Walleij, Liam Girdwood, Mark Brown
Cc: linux-kernel, Janusz Krzysztofik, Alexander Shiyan,
Haojian Zhuang, Aaro Koskinen, Mike Rapoport, Robert Jarzmik,
Philipp Zabel, Daniel Mack, Marc Zyngier, Jacopo Mondi,
Geert Uytterhoeven, Russell King
Hi All,
On 2018-09-06 14:24, Linus Walleij wrote:
> As we augmented the regulator core to accept a GPIO descriptor instead
> of a GPIO number, we can augment the fixed GPIO regulator to look up
> and pass that descriptor directly from device tree or board GPIO
> descriptor look up tables.
>
> Some boards just auto-enumerate their fixed regulator platform devices
> and I have assumed they get names like "fixed-regulator.0" but it's
> pretty hard to guess this. I need some testing from board maintainers to
> be sure. Other boards are straight forward, using just plain
> "fixed-regulator" (ID -1) or "fixed-regulator.1" hammering down the
> device ID.
>
> It seems the da9055 and da9211 has never got around to actually passing
> any enable gpio into its platform data (not the in-tree code anyway) so we
> can just decide to simply pass a descriptor instead.
>
> The fixed GPIO-controlled regulator in mach-pxa/ezx.c was confusingly named
> "*_dummy_supply_device" while it is a very real device backed by a GPIO
> line. There is nothing dummy about it at all, so I renamed it with the
> infix *_regulator_* as part of this patch set.
>
> Intel MID portions tested by Andy.
>
> Cc: Janusz Krzysztofik <jmkrzyszt@gmail.com> # OMAP1
> Cc: Alexander Shiyan <shc_work@mail.ru> # i.MX boards user
> Cc: Haojian Zhuang <haojian.zhuang@gmail.com> # MMP2 maintainer
> Cc: Aaro Koskinen <aaro.koskinen@iki.fi> # OMAP1 maintainer
> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com> # EM-X270 maintainer
> Cc: Robert Jarzmik <robert.jarzmik@free.fr> # EZX maintainer
> Cc: Philipp Zabel <philipp.zabel@gmail.com> # Magician maintainer
> Cc: Daniel Mack <zonque@gmail.com> # Raumfeld maintainer
> Cc: Marc Zyngier <marc.zyngier@arm.com> # Zeus maintainer
> Cc: Jacopo Mondi <jacopo@jmondi.org> # SH Ecovec24
> Cc: Geert Uytterhoeven <geert+renesas@glider.be> # SuperH pinctrl/GPIO maintainer
> Cc: Russell King <rmk+kernel@armlinux.org.uk> # SA1100
> Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> # Check the x86 BCM stuff
> Acked-by: Tony Lindgren <tony@atomide.com> # OMAP1,2,3 maintainer
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> Reviewed-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
I've just noticed that this patch causes regression on Samsung
Exynos4412-based Trats2 board. Conversion to GPIO descriptor breaks
operation when regulators used shared GPIO: sii9234 i2c driver
is not able to get vcc33mhl regulator (it uses shared GPIO enable
line with vsil12 regulator).
This issue has been already pointed in case of commits:
37fa23dbccbd97663acc085bd79246f427e603a1
d1dae72fab2c377ff463742eefd8ac0f9e99b7b9
ab4d11e2c2329cf7cb7be31ff22489aae4dee5dc
Maybe it would be better to first solve the handling of shared enable
GPIO in the descriptor-based interface before converting more regulators
and stepping into this issue again?
> ---
> ChangeLog v6->v7:
> - As the autobuilder churned along, after 24+ hours it was testing
> SH and found a bug in the ecovec24 boardfile. It does test a
> SH arch byt default, sh7763rdp_defconfig, sadly not this one.
> So it discovers more obscure boards in later testing.
> - Shaked out this bug too and re-pushed and posted.
> ChangeLog v5->v6:
> - New code appeared in the OMAP1 AMS delta board that added a
> new user of the removed .gpio member. The build robot was first
> happy, then came back later and was not happy.
> - Fixed up the offending .gpio, now rebuilt for this OMAP1
> board specifically to make sure it really really work now.
> ChangeLog v4->v5:
> - Rebased on v4.19-rc1
> - Put the OMAP1 AMD delta GPIO table addition in the *TOP*
> of the ams_delta_gpio_tables[] so Janusz can add any
> new addtions on the *BOTTOM*
> - Hopefully we can merge this now.
> ChangeLog v3->v4:
> - Rebase and adapt the OMAP1 changes for the GPIO descriptor
> look-up tables deployed by Janusz.
> - Add two calls to add the GPIO descriptor tables properly on
> the Super-H Ecovec24 board as pointed out by Geert.
> - Go over all patches to board files and make sure we pass
> a NULL descriptor instead of an "enable" descriptor. The code
> is looking for unnamed GPIOs as the device tree also just pass
> gpio[s] = <&foo> so board files also need to use anonymous
> GPIOs.
> - Fold in an EZX fix from Arnd Bergmann.
> - Add Andy's Tested-by tag.
> - Send this patch *ALONE* as I realized I need to take smaller
> steps so things do not blow up left and right.
> ChangeLog v2->v3:
> - Resending.
> ChangeLog v1->v2:
> - Rebase the patch on mainline with Blackfin gone and other changes.
> - Fix up the new users that appeared in sa1100
> - Drop some suplus comments in x86.
> ---
> arch/arm/mach-imx/mach-mx21ads.c | 12 ++++++-
> arch/arm/mach-imx/mach-mx27ads.c | 12 ++++++-
> arch/arm/mach-mmp/brownstone.c | 12 ++++++-
> arch/arm/mach-omap1/board-ams-delta.c | 12 +++++--
> arch/arm/mach-omap2/pdata-quirks.c | 16 ++++++++-
> arch/arm/mach-pxa/em-x270.c | 1 -
> arch/arm/mach-pxa/ezx.c | 33 ++++++++++++-------
> arch/arm/mach-pxa/magician.c | 2 +-
> arch/arm/mach-pxa/raumfeld.c | 12 +++++--
> arch/arm/mach-pxa/zeus.c | 23 +++++++++++--
> arch/arm/mach-s3c64xx/mach-crag6410.c | 1 -
> arch/arm/mach-s3c64xx/mach-smdk6410.c | 1 -
> arch/arm/mach-sa1100/assabet.c | 21 ++++++++----
> arch/arm/mach-sa1100/generic.c | 5 +--
> arch/arm/mach-sa1100/generic.h | 3 +-
> arch/arm/mach-sa1100/shannon.c | 4 +--
> arch/sh/boards/mach-ecovec24/setup.c | 27 +++++++++++++--
> .../intel-mid/device_libs/platform_bcm43xx.c | 17 ++++++++--
> drivers/regulator/fixed-helper.c | 1 -
> drivers/regulator/fixed.c | 33 +++++++++----------
> include/linux/regulator/fixed.h | 3 --
> 21 files changed, 188 insertions(+), 63 deletions(-)
>
> diff --git a/arch/arm/mach-imx/mach-mx21ads.c b/arch/arm/mach-imx/mach-mx21ads.c
> index 5e366824814f..2e1e540f2e5a 100644
> --- a/arch/arm/mach-imx/mach-mx21ads.c
> +++ b/arch/arm/mach-imx/mach-mx21ads.c
> @@ -18,6 +18,7 @@
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/physmap.h>
> #include <linux/gpio/driver.h>
> +#include <linux/gpio/machine.h>
> #include <linux/gpio.h>
> #include <linux/regulator/fixed.h>
> #include <linux/regulator/machine.h>
> @@ -175,6 +176,7 @@ static struct resource mx21ads_mmgpio_resource =
> DEFINE_RES_MEM_NAMED(MX21ADS_IO_REG, SZ_2, "dat");
>
> static struct bgpio_pdata mx21ads_mmgpio_pdata = {
> + .label = "mx21ads-mmgpio",
> .base = MX21ADS_MMGPIO_BASE,
> .ngpio = 16,
> };
> @@ -203,7 +205,6 @@ static struct regulator_init_data mx21ads_lcd_regulator_init_data = {
> static struct fixed_voltage_config mx21ads_lcd_regulator_pdata = {
> .supply_name = "LCD",
> .microvolts = 3300000,
> - .gpio = MX21ADS_IO_LCDON,
> .enable_high = 1,
> .init_data = &mx21ads_lcd_regulator_init_data,
> };
> @@ -216,6 +217,14 @@ static struct platform_device mx21ads_lcd_regulator = {
> },
> };
>
> +static struct gpiod_lookup_table mx21ads_lcd_regulator_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.0", /* Let's hope ID 0 is what we get */
> + .table = {
> + GPIO_LOOKUP("mx21ads-mmgpio", 9, NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> /*
> * Connected is a portrait Sharp-QVGA display
> * of type: LQ035Q7DB02
> @@ -311,6 +320,7 @@ static void __init mx21ads_late_init(void)
> {
> imx21_add_mxc_mmc(0, &mx21ads_sdhc_pdata);
>
> + gpiod_add_lookup_table(&mx21ads_lcd_regulator_gpiod_table);
> platform_add_devices(platform_devices, ARRAY_SIZE(platform_devices));
>
> mx21ads_cs8900_resources[1].start =
> diff --git a/arch/arm/mach-imx/mach-mx27ads.c b/arch/arm/mach-imx/mach-mx27ads.c
> index a04bb094ded1..f5e04047ed13 100644
> --- a/arch/arm/mach-imx/mach-mx27ads.c
> +++ b/arch/arm/mach-imx/mach-mx27ads.c
> @@ -16,6 +16,7 @@
> #include <linux/gpio/driver.h>
> /* Needed for gpio_to_irq() */
> #include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
> #include <linux/platform_device.h>
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/map.h>
> @@ -230,10 +231,17 @@ static struct regulator_init_data mx27ads_lcd_regulator_init_data = {
> static struct fixed_voltage_config mx27ads_lcd_regulator_pdata = {
> .supply_name = "LCD",
> .microvolts = 3300000,
> - .gpio = MX27ADS_LCD_GPIO,
> .init_data = &mx27ads_lcd_regulator_init_data,
> };
>
> +static struct gpiod_lookup_table mx27ads_lcd_regulator_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.0", /* Let's hope ID 0 is what we get */
> + .table = {
> + GPIO_LOOKUP("LCD", 0, NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> static void __init mx27ads_regulator_init(void)
> {
> struct gpio_chip *vchip;
> @@ -247,6 +255,8 @@ static void __init mx27ads_regulator_init(void)
> vchip->set = vgpio_set;
> gpiochip_add_data(vchip, NULL);
>
> + gpiod_add_lookup_table(&mx27ads_lcd_regulator_gpiod_table);
> +
> platform_device_register_data(NULL, "reg-fixed-voltage",
> PLATFORM_DEVID_AUTO,
> &mx27ads_lcd_regulator_pdata,
> diff --git a/arch/arm/mach-mmp/brownstone.c b/arch/arm/mach-mmp/brownstone.c
> index d1613b954926..a04e249c654b 100644
> --- a/arch/arm/mach-mmp/brownstone.c
> +++ b/arch/arm/mach-mmp/brownstone.c
> @@ -15,6 +15,7 @@
> #include <linux/platform_device.h>
> #include <linux/io.h>
> #include <linux/gpio-pxa.h>
> +#include <linux/gpio/machine.h>
> #include <linux/regulator/machine.h>
> #include <linux/regulator/max8649.h>
> #include <linux/regulator/fixed.h>
> @@ -148,7 +149,6 @@ static struct regulator_init_data brownstone_v_5vp_data = {
> static struct fixed_voltage_config brownstone_v_5vp = {
> .supply_name = "v_5vp",
> .microvolts = 5000000,
> - .gpio = GPIO_5V_ENABLE,
> .enable_high = 1,
> .enabled_at_boot = 1,
> .init_data = &brownstone_v_5vp_data,
> @@ -162,6 +162,15 @@ static struct platform_device brownstone_v_5vp_device = {
> },
> };
>
> +static struct gpiod_lookup_table brownstone_v_5vp_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.1", /* .id set to 1 above */
> + .table = {
> + GPIO_LOOKUP("gpio-pxa", GPIO_5V_ENABLE,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> static struct max8925_platform_data brownstone_max8925_info = {
> .irq_base = MMP_NR_IRQS,
> };
> @@ -217,6 +226,7 @@ static void __init brownstone_init(void)
> mmp2_add_isram(&mmp2_isram_platdata);
>
> /* enable 5v regulator */
> + gpiod_add_lookup_table(&brownstone_v_5vp_gpiod_table);
> platform_device_register(&brownstone_v_5vp_device);
> }
>
> diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
> index dd28d2614d7f..f226973f3d8c 100644
> --- a/arch/arm/mach-omap1/board-ams-delta.c
> +++ b/arch/arm/mach-omap1/board-ams-delta.c
> @@ -300,7 +300,6 @@ static struct regulator_init_data modem_nreset_data = {
> static struct fixed_voltage_config modem_nreset_config = {
> .supply_name = "modem_nreset",
> .microvolts = 3300000,
> - .gpio = AMS_DELTA_GPIO_PIN_MODEM_NRESET,
> .startup_delay = 25000,
> .enable_high = 1,
> .enabled_at_boot = 1,
> @@ -315,6 +314,15 @@ static struct platform_device modem_nreset_device = {
> },
> };
>
> +static struct gpiod_lookup_table ams_delta_nreset_gpiod_table = {
> + .dev_id = "reg-fixed-voltage",
> + .table = {
> + GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_MODEM_NRESET,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> struct modem_private_data {
> struct regulator *regulator;
> };
> @@ -568,7 +576,6 @@ static struct regulator_init_data keybrd_pwr_initdata = {
> static struct fixed_voltage_config keybrd_pwr_config = {
> .supply_name = "keybrd_pwr",
> .microvolts = 5000000,
> - .gpio = AMS_DELTA_GPIO_PIN_KEYBRD_PWR,
> .enable_high = 1,
> .init_data = &keybrd_pwr_initdata,
> };
> @@ -602,6 +609,7 @@ static struct platform_device *ams_delta_devices[] __initdata = {
> };
>
> static struct gpiod_lookup_table *ams_delta_gpio_tables[] __initdata = {
> + &ams_delta_nreset_gpiod_table,
> &ams_delta_audio_gpio_table,
> &keybrd_pwr_gpio_table,
> &ams_delta_lcd_gpio_table,
> diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
> index 7f02743edbe4..d0f7a7cc70cb 100644
> --- a/arch/arm/mach-omap2/pdata-quirks.c
> +++ b/arch/arm/mach-omap2/pdata-quirks.c
> @@ -10,6 +10,7 @@
> #include <linux/clk.h>
> #include <linux/davinci_emac.h>
> #include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/of_platform.h>
> @@ -328,7 +329,6 @@ static struct regulator_init_data pandora_vmmc3 = {
> static struct fixed_voltage_config pandora_vwlan = {
> .supply_name = "vwlan",
> .microvolts = 1800000, /* 1.8V */
> - .gpio = PANDORA_WIFI_NRESET_GPIO,
> .startup_delay = 50000, /* 50ms */
> .enable_high = 1,
> .init_data = &pandora_vmmc3,
> @@ -342,6 +342,19 @@ static struct platform_device pandora_vwlan_device = {
> },
> };
>
> +static struct gpiod_lookup_table pandora_vwlan_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.1",
> + .table = {
> + /*
> + * As this is a low GPIO number it should be at the first
> + * GPIO bank.
> + */
> + GPIO_LOOKUP("gpio-0-31", PANDORA_WIFI_NRESET_GPIO,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> static void pandora_wl1251_init_card(struct mmc_card *card)
> {
> /*
> @@ -403,6 +416,7 @@ static void __init pandora_wl1251_init(void)
> static void __init omap3_pandora_legacy_init(void)
> {
> platform_device_register(&pandora_backlight);
> + gpiod_add_lookup_table(&pandora_vwlan_gpiod_table);
> platform_device_register(&pandora_vwlan_device);
> omap_hsmmc_init(pandora_mmc3);
> omap_hsmmc_late_init(pandora_mmc3);
> diff --git a/arch/arm/mach-pxa/em-x270.c b/arch/arm/mach-pxa/em-x270.c
> index 29be04c6cc48..b14c47a6ee6b 100644
> --- a/arch/arm/mach-pxa/em-x270.c
> +++ b/arch/arm/mach-pxa/em-x270.c
> @@ -986,7 +986,6 @@ static struct fixed_voltage_config camera_dummy_config = {
> .supply_name = "camera_vdd",
> .input_supply = "vcc cam",
> .microvolts = 2800000,
> - .gpio = -1,
> .enable_high = 0,
> .init_data = &camera_dummy_initdata,
> };
> diff --git a/arch/arm/mach-pxa/ezx.c b/arch/arm/mach-pxa/ezx.c
> index 2c90b58f347d..565965e9acc7 100644
> --- a/arch/arm/mach-pxa/ezx.c
> +++ b/arch/arm/mach-pxa/ezx.c
> @@ -21,6 +21,7 @@
> #include <linux/regulator/fixed.h>
> #include <linux/input.h>
> #include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
> #include <linux/gpio_keys.h>
> #include <linux/leds-lp3944.h>
> #include <linux/platform_data/i2c-pxa.h>
> @@ -698,31 +699,39 @@ static struct pxa27x_keypad_platform_data e2_keypad_platform_data = {
>
> #if defined(CONFIG_MACH_EZX_A780) || defined(CONFIG_MACH_EZX_A910)
> /* camera */
> -static struct regulator_consumer_supply camera_dummy_supplies[] = {
> +static struct regulator_consumer_supply camera_regulator_supplies[] = {
> REGULATOR_SUPPLY("vdd", "0-005d"),
> };
>
> -static struct regulator_init_data camera_dummy_initdata = {
> - .consumer_supplies = camera_dummy_supplies,
> - .num_consumer_supplies = ARRAY_SIZE(camera_dummy_supplies),
> +static struct regulator_init_data camera_regulator_initdata = {
> + .consumer_supplies = camera_regulator_supplies,
> + .num_consumer_supplies = ARRAY_SIZE(camera_regulator_supplies),
> .constraints = {
> .valid_ops_mask = REGULATOR_CHANGE_STATUS,
> },
> };
>
> -static struct fixed_voltage_config camera_dummy_config = {
> +static struct fixed_voltage_config camera_regulator_config = {
> .supply_name = "camera_vdd",
> .microvolts = 2800000,
> - .gpio = GPIO50_nCAM_EN,
> .enable_high = 0,
> - .init_data = &camera_dummy_initdata,
> + .init_data = &camera_regulator_initdata,
> };
>
> -static struct platform_device camera_supply_dummy_device = {
> +static struct platform_device camera_supply_regulator_device = {
> .name = "reg-fixed-voltage",
> .id = 1,
> .dev = {
> - .platform_data = &camera_dummy_config,
> + .platform_data = &camera_regulator_config,
> + },
> +};
> +
> +static struct gpiod_lookup_table camera_supply_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.1",
> + .table = {
> + GPIO_LOOKUP("gpio-pxa", GPIO50_nCAM_EN,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> },
> };
> #endif
> @@ -800,7 +809,7 @@ static struct i2c_board_info a780_i2c_board_info[] = {
>
> static struct platform_device *a780_devices[] __initdata = {
> &a780_gpio_keys,
> - &camera_supply_dummy_device,
> + &camera_supply_regulator_device,
> };
>
> static void __init a780_init(void)
> @@ -823,6 +832,7 @@ static void __init a780_init(void)
> if (a780_camera_init() == 0)
> pxa_set_camera_info(&a780_pxacamera_platform_data);
>
> + gpiod_add_lookup_table(&camera_supply_gpiod_table);
> pwm_add_table(ezx_pwm_lookup, ARRAY_SIZE(ezx_pwm_lookup));
> platform_add_devices(ARRAY_AND_SIZE(ezx_devices));
> platform_add_devices(ARRAY_AND_SIZE(a780_devices));
> @@ -1098,7 +1108,7 @@ static struct i2c_board_info __initdata a910_i2c_board_info[] = {
>
> static struct platform_device *a910_devices[] __initdata = {
> &a910_gpio_keys,
> - &camera_supply_dummy_device,
> + &camera_supply_regulator_device,
> };
>
> static void __init a910_init(void)
> @@ -1121,6 +1131,7 @@ static void __init a910_init(void)
> if (a910_camera_init() == 0)
> pxa_set_camera_info(&a910_pxacamera_platform_data);
>
> + gpiod_add_lookup_table(&camera_supply_gpiod_table);
> pwm_add_table(ezx_pwm_lookup, ARRAY_SIZE(ezx_pwm_lookup));
> platform_add_devices(ARRAY_AND_SIZE(ezx_devices));
> platform_add_devices(ARRAY_AND_SIZE(a910_devices));
> diff --git a/arch/arm/mach-pxa/magician.c b/arch/arm/mach-pxa/magician.c
> index c5325d1ae77b..14c0f80bc9e7 100644
> --- a/arch/arm/mach-pxa/magician.c
> +++ b/arch/arm/mach-pxa/magician.c
> @@ -18,6 +18,7 @@
> #include <linux/platform_device.h>
> #include <linux/delay.h>
> #include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
> #include <linux/gpio_keys.h>
> #include <linux/input.h>
> #include <linux/mfd/htc-pasic3.h>
> @@ -696,7 +697,6 @@ static struct regulator_init_data vads7846_regulator = {
> static struct fixed_voltage_config vads7846 = {
> .supply_name = "vads7846",
> .microvolts = 3300000, /* probably */
> - .gpio = -EINVAL,
> .startup_delay = 0,
> .init_data = &vads7846_regulator,
> };
> diff --git a/arch/arm/mach-pxa/raumfeld.c b/arch/arm/mach-pxa/raumfeld.c
> index 034345546f84..bd3c23ad6ce6 100644
> --- a/arch/arm/mach-pxa/raumfeld.c
> +++ b/arch/arm/mach-pxa/raumfeld.c
> @@ -886,7 +886,6 @@ static struct regulator_init_data audio_va_initdata = {
> static struct fixed_voltage_config audio_va_config = {
> .supply_name = "audio_va",
> .microvolts = 5000000,
> - .gpio = GPIO_AUDIO_VA_ENABLE,
> .enable_high = 1,
> .enabled_at_boot = 0,
> .init_data = &audio_va_initdata,
> @@ -900,6 +899,15 @@ static struct platform_device audio_va_device = {
> },
> };
>
> +static struct gpiod_lookup_table audio_va_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.0",
> + .table = {
> + GPIO_LOOKUP("gpio-pxa", GPIO_AUDIO_VA_ENABLE,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> /* Dummy supplies for Codec's VD/VLC */
>
> static struct regulator_consumer_supply audio_dummy_supplies[] = {
> @@ -918,7 +926,6 @@ static struct regulator_init_data audio_dummy_initdata = {
> static struct fixed_voltage_config audio_dummy_config = {
> .supply_name = "audio_vd",
> .microvolts = 3300000,
> - .gpio = -1,
> .init_data = &audio_dummy_initdata,
> };
>
> @@ -1033,6 +1040,7 @@ static void __init raumfeld_audio_init(void)
> else
> gpio_direction_output(GPIO_MCLK_RESET, 1);
>
> + gpiod_add_lookup_table(&audio_va_gpiod_table);
> platform_add_devices(ARRAY_AND_SIZE(audio_regulator_devices));
> }
>
> diff --git a/arch/arm/mach-pxa/zeus.c b/arch/arm/mach-pxa/zeus.c
> index e3851795d6d7..d53ea12fc766 100644
> --- a/arch/arm/mach-pxa/zeus.c
> +++ b/arch/arm/mach-pxa/zeus.c
> @@ -17,6 +17,7 @@
> #include <linux/irq.h>
> #include <linux/pm.h>
> #include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
> #include <linux/serial_8250.h>
> #include <linux/dm9000.h>
> #include <linux/mmc/host.h>
> @@ -410,7 +411,6 @@ static struct regulator_init_data can_regulator_init_data = {
> static struct fixed_voltage_config can_regulator_pdata = {
> .supply_name = "CAN_SHDN",
> .microvolts = 3300000,
> - .gpio = ZEUS_CAN_SHDN_GPIO,
> .init_data = &can_regulator_init_data,
> };
>
> @@ -422,6 +422,15 @@ static struct platform_device can_regulator_device = {
> },
> };
>
> +static struct gpiod_lookup_table can_regulator_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.0",
> + .table = {
> + GPIO_LOOKUP("gpio-pxa", ZEUS_CAN_SHDN_GPIO,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> static struct mcp251x_platform_data zeus_mcp2515_pdata = {
> .oscillator_frequency = 16*1000*1000,
> };
> @@ -538,7 +547,6 @@ static struct regulator_init_data zeus_ohci_regulator_data = {
> static struct fixed_voltage_config zeus_ohci_regulator_config = {
> .supply_name = "vbus2",
> .microvolts = 5000000, /* 5.0V */
> - .gpio = ZEUS_USB2_PWREN_GPIO,
> .enable_high = 1,
> .startup_delay = 0,
> .init_data = &zeus_ohci_regulator_data,
> @@ -552,6 +560,15 @@ static struct platform_device zeus_ohci_regulator_device = {
> },
> };
>
> +static struct gpiod_lookup_table zeus_ohci_regulator_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.0",
> + .table = {
> + GPIO_LOOKUP("gpio-pxa", ZEUS_USB2_PWREN_GPIO,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> static struct pxaohci_platform_data zeus_ohci_platform_data = {
> .port_mode = PMM_NPS_MODE,
> /* Clear Power Control Polarity Low and set Power Sense
> @@ -855,6 +872,8 @@ static void __init zeus_init(void)
>
> pxa2xx_mfp_config(ARRAY_AND_SIZE(zeus_pin_config));
>
> + gpiod_add_lookup_table(&can_regulator_gpiod_table);
> + gpiod_add_lookup_table(&zeus_ohci_regulator_gpiod_table);
> platform_add_devices(zeus_devices, ARRAY_SIZE(zeus_devices));
>
> zeus_register_ohci();
> diff --git a/arch/arm/mach-s3c64xx/mach-crag6410.c b/arch/arm/mach-s3c64xx/mach-crag6410.c
> index f04650297487..379424d72ae7 100644
> --- a/arch/arm/mach-s3c64xx/mach-crag6410.c
> +++ b/arch/arm/mach-s3c64xx/mach-crag6410.c
> @@ -352,7 +352,6 @@ static struct fixed_voltage_config wallvdd_pdata = {
> .supply_name = "WALLVDD",
> .microvolts = 5000000,
> .init_data = &wallvdd_data,
> - .gpio = -EINVAL,
> };
>
> static struct platform_device wallvdd_device = {
> diff --git a/arch/arm/mach-s3c64xx/mach-smdk6410.c b/arch/arm/mach-s3c64xx/mach-smdk6410.c
> index c46fa5dfd2e0..908e5aa831c8 100644
> --- a/arch/arm/mach-s3c64xx/mach-smdk6410.c
> +++ b/arch/arm/mach-s3c64xx/mach-smdk6410.c
> @@ -222,7 +222,6 @@ static struct fixed_voltage_config smdk6410_b_pwr_5v_pdata = {
> .supply_name = "B_PWR_5V",
> .microvolts = 5000000,
> .init_data = &smdk6410_b_pwr_5v_data,
> - .gpio = -EINVAL,
> };
>
> static struct platform_device smdk6410_b_pwr_5v = {
> diff --git a/arch/arm/mach-sa1100/assabet.c b/arch/arm/mach-sa1100/assabet.c
> index 575ec085cffa..3e8c0948abcc 100644
> --- a/arch/arm/mach-sa1100/assabet.c
> +++ b/arch/arm/mach-sa1100/assabet.c
> @@ -101,7 +101,7 @@ static int __init assabet_init_gpio(void __iomem *reg, u32 def_val)
>
> assabet_bcr_gc = gc;
>
> - return gc->base;
> + return 0;
> }
>
> /*
> @@ -471,6 +471,14 @@ static struct fixed_voltage_config assabet_cf_vcc_pdata __initdata = {
> .enable_high = 1,
> };
>
> +static struct gpiod_lookup_table assabet_cf_vcc_gpio_table = {
> + .dev_id = "reg-fixed-voltage.0",
> + .table = {
> + GPIO_LOOKUP("assabet", 0, NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> static void __init assabet_init(void)
> {
> /*
> @@ -517,9 +525,11 @@ static void __init assabet_init(void)
> neponset_resources, ARRAY_SIZE(neponset_resources));
> #endif
> } else {
> + gpiod_add_lookup_table(&assabet_cf_vcc_gpio_table);
> sa11x0_register_fixed_regulator(0, &assabet_cf_vcc_pdata,
> - assabet_cf_vcc_consumers,
> - ARRAY_SIZE(assabet_cf_vcc_consumers));
> + assabet_cf_vcc_consumers,
> + ARRAY_SIZE(assabet_cf_vcc_consumers),
> + true);
>
> }
>
> @@ -802,7 +812,6 @@ fs_initcall(assabet_leds_init);
>
> void __init assabet_init_irq(void)
> {
> - unsigned int assabet_gpio_base;
> u32 def_val;
>
> sa1100_init_irq();
> @@ -817,9 +826,7 @@ void __init assabet_init_irq(void)
> *
> * This must precede any driver calls to BCR_set() or BCR_clear().
> */
> - assabet_gpio_base = assabet_init_gpio((void *)&ASSABET_BCR, def_val);
> -
> - assabet_cf_vcc_pdata.gpio = assabet_gpio_base + 0;
> + assabet_init_gpio((void *)&ASSABET_BCR, def_val);
> }
>
> MACHINE_START(ASSABET, "Intel-Assabet")
> diff --git a/arch/arm/mach-sa1100/generic.c b/arch/arm/mach-sa1100/generic.c
> index 7167ddf84a0e..800321c6cbd8 100644
> --- a/arch/arm/mach-sa1100/generic.c
> +++ b/arch/arm/mach-sa1100/generic.c
> @@ -348,7 +348,8 @@ void __init sa11x0_init_late(void)
>
> int __init sa11x0_register_fixed_regulator(int n,
> struct fixed_voltage_config *cfg,
> - struct regulator_consumer_supply *supplies, unsigned num_supplies)
> + struct regulator_consumer_supply *supplies, unsigned num_supplies,
> + bool uses_gpio)
> {
> struct regulator_init_data *id;
>
> @@ -356,7 +357,7 @@ int __init sa11x0_register_fixed_regulator(int n,
> if (!cfg->init_data)
> return -ENOMEM;
>
> - if (cfg->gpio < 0)
> + if (!uses_gpio)
> id->constraints.always_on = 1;
> id->constraints.name = cfg->supply_name;
> id->constraints.min_uV = cfg->microvolts;
> diff --git a/arch/arm/mach-sa1100/generic.h b/arch/arm/mach-sa1100/generic.h
> index 5f3cb52fa6ab..158a4fd5ca24 100644
> --- a/arch/arm/mach-sa1100/generic.h
> +++ b/arch/arm/mach-sa1100/generic.h
> @@ -54,4 +54,5 @@ void sa11x0_register_pcmcia(int socket, struct gpiod_lookup_table *);
> struct fixed_voltage_config;
> struct regulator_consumer_supply;
> int sa11x0_register_fixed_regulator(int n, struct fixed_voltage_config *cfg,
> - struct regulator_consumer_supply *supplies, unsigned num_supplies);
> + struct regulator_consumer_supply *supplies, unsigned num_supplies,
> + bool uses_gpio);
> diff --git a/arch/arm/mach-sa1100/shannon.c b/arch/arm/mach-sa1100/shannon.c
> index 22f7fe0b809f..5bc82e2671c6 100644
> --- a/arch/arm/mach-sa1100/shannon.c
> +++ b/arch/arm/mach-sa1100/shannon.c
> @@ -102,14 +102,14 @@ static struct fixed_voltage_config shannon_cf_vcc_pdata __initdata = {
> .supply_name = "cf-power",
> .microvolts = 3300000,
> .enabled_at_boot = 1,
> - .gpio = -EINVAL,
> };
>
> static void __init shannon_init(void)
> {
> sa11x0_register_fixed_regulator(0, &shannon_cf_vcc_pdata,
> shannon_cf_vcc_consumers,
> - ARRAY_SIZE(shannon_cf_vcc_consumers));
> + ARRAY_SIZE(shannon_cf_vcc_consumers),
> + false);
> sa11x0_register_pcmcia(0, &shannon_pcmcia0_gpio_table);
> sa11x0_register_pcmcia(1, &shannon_pcmcia1_gpio_table);
> sa11x0_ppc_configure_mcp();
> diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
> index adc61d14172c..06a894526a0b 100644
> --- a/arch/sh/boards/mach-ecovec24/setup.c
> +++ b/arch/sh/boards/mach-ecovec24/setup.c
> @@ -633,7 +633,6 @@ static struct regulator_init_data cn12_power_init_data = {
> static struct fixed_voltage_config cn12_power_info = {
> .supply_name = "CN12 SD/MMC Vdd",
> .microvolts = 3300000,
> - .gpio = GPIO_PTB7,
> .enable_high = 1,
> .init_data = &cn12_power_init_data,
> };
> @@ -646,6 +645,16 @@ static struct platform_device cn12_power = {
> },
> };
>
> +static struct gpiod_lookup_table cn12_power_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.0",
> + .table = {
> + /* Offset 7 on port B */
> + GPIO_LOOKUP("sh7724_pfc", GPIO_PTB7,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> #if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
> /* SDHI0 */
> static struct regulator_consumer_supply sdhi0_power_consumers[] =
> @@ -665,7 +674,6 @@ static struct regulator_init_data sdhi0_power_init_data = {
> static struct fixed_voltage_config sdhi0_power_info = {
> .supply_name = "CN11 SD/MMC Vdd",
> .microvolts = 3300000,
> - .gpio = GPIO_PTB6,
> .enable_high = 1,
> .init_data = &sdhi0_power_init_data,
> };
> @@ -678,6 +686,16 @@ static struct platform_device sdhi0_power = {
> },
> };
>
> +static struct gpiod_lookup_table sdhi0_power_gpiod_table = {
> + .dev_id = "reg-fixed-voltage.1",
> + .table = {
> + /* Offset 6 on port B */
> + GPIO_LOOKUP("sh7724_pfc", GPIO_PTB6,
> + NULL, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> static struct tmio_mmc_data sdhi0_info = {
> .chan_priv_tx = (void *)SHDMA_SLAVE_SDHI0_TX,
> .chan_priv_rx = (void *)SHDMA_SLAVE_SDHI0_RX,
> @@ -1413,6 +1431,11 @@ static int __init arch_setup(void)
> DMA_MEMORY_EXCLUSIVE);
> platform_device_add(ecovec_ceu_devices[1]);
>
> + gpiod_add_lookup_table(&cn12_power_gpiod_table);
> +#if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
> + gpiod_add_lookup_table(&sdhi0_power_gpiod_table);
> +#endif
> +
> return platform_add_devices(ecovec_devices,
> ARRAY_SIZE(ecovec_devices));
> }
> diff --git a/arch/x86/platform/intel-mid/device_libs/platform_bcm43xx.c b/arch/x86/platform/intel-mid/device_libs/platform_bcm43xx.c
> index 4392c15ed9e0..dbfc5cf2aa93 100644
> --- a/arch/x86/platform/intel-mid/device_libs/platform_bcm43xx.c
> +++ b/arch/x86/platform/intel-mid/device_libs/platform_bcm43xx.c
> @@ -10,7 +10,7 @@
> * of the License.
> */
>
> -#include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
> #include <linux/platform_device.h>
> #include <linux/regulator/machine.h>
> #include <linux/regulator/fixed.h>
> @@ -43,7 +43,6 @@ static struct fixed_voltage_config bcm43xx_vmmc = {
> * real voltage and signaling are still 1.8V.
> */
> .microvolts = 2000000, /* 1.8V */
> - .gpio = -EINVAL,
> .startup_delay = 250 * 1000, /* 250ms */
> .enable_high = 1, /* active high */
> .enabled_at_boot = 0, /* disabled at boot */
> @@ -58,11 +57,23 @@ static struct platform_device bcm43xx_vmmc_regulator = {
> },
> };
>
> +static struct gpiod_lookup_table bcm43xx_vmmc_gpio_table = {
> + .dev_id = "reg-fixed-voltage.0",
> + .table = {
> + GPIO_LOOKUP("0000:00:0c.0", -1, NULL, GPIO_ACTIVE_LOW),
> + {}
> + },
> +};
> +
> static int __init bcm43xx_regulator_register(void)
> {
> + struct gpiod_lookup_table *table = &bcm43xx_vmmc_gpio_table;
> + struct gpiod_lookup *lookup = table->table;
> int ret;
>
> - bcm43xx_vmmc.gpio = get_gpio_by_name(WLAN_SFI_GPIO_ENABLE_NAME);
> + lookup[0].chip_hwnum = get_gpio_by_name(WLAN_SFI_GPIO_ENABLE_NAME);
> + gpiod_add_lookup_table(table);
> +
> ret = platform_device_register(&bcm43xx_vmmc_regulator);
> if (ret) {
> pr_err("%s: vmmc regulator register failed\n", __func__);
> diff --git a/drivers/regulator/fixed-helper.c b/drivers/regulator/fixed-helper.c
> index 777fac6fb4cb..2c6098e6f4bc 100644
> --- a/drivers/regulator/fixed-helper.c
> +++ b/drivers/regulator/fixed-helper.c
> @@ -43,7 +43,6 @@ struct platform_device *regulator_register_always_on(int id, const char *name,
> }
>
> data->cfg.microvolts = uv;
> - data->cfg.gpio = -EINVAL;
> data->cfg.enabled_at_boot = 1;
> data->cfg.init_data = &data->init_data;
>
> diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
> index 988a7472c2ab..1142f195529b 100644
> --- a/drivers/regulator/fixed.c
> +++ b/drivers/regulator/fixed.c
> @@ -24,10 +24,9 @@
> #include <linux/platform_device.h>
> #include <linux/regulator/driver.h>
> #include <linux/regulator/fixed.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/slab.h>
> #include <linux/of.h>
> -#include <linux/of_gpio.h>
> #include <linux/regulator/of_regulator.h>
> #include <linux/regulator/machine.h>
>
> @@ -78,10 +77,6 @@ of_get_fixed_voltage_config(struct device *dev,
> if (init_data->constraints.boot_on)
> config->enabled_at_boot = true;
>
> - config->gpio = of_get_named_gpio(np, "gpio", 0);
> - if ((config->gpio < 0) && (config->gpio != -ENOENT))
> - return ERR_PTR(config->gpio);
> -
> of_property_read_u32(np, "startup-delay-us", &config->startup_delay);
>
> config->enable_high = of_property_read_bool(np, "enable-active-high");
> @@ -102,6 +97,7 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
> struct fixed_voltage_config *config;
> struct fixed_voltage_data *drvdata;
> struct regulator_config cfg = { };
> + enum gpiod_flags gflags;
> int ret;
>
> drvdata = devm_kzalloc(&pdev->dev, sizeof(struct fixed_voltage_data),
> @@ -150,25 +146,28 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
>
> drvdata->desc.fixed_uV = config->microvolts;
>
> - if (gpio_is_valid(config->gpio)) {
> - cfg.ena_gpio = config->gpio;
> - if (pdev->dev.of_node)
> - cfg.ena_gpio_initialized = true;
> - }
> cfg.ena_gpio_invert = !config->enable_high;
> if (config->enabled_at_boot) {
> if (config->enable_high)
> - cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
> + gflags = GPIOD_OUT_HIGH;
> else
> - cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
> + gflags = GPIOD_OUT_LOW;
> } else {
> if (config->enable_high)
> - cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
> + gflags = GPIOD_OUT_LOW;
> else
> - cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
> + gflags = GPIOD_OUT_HIGH;
> }
> - if (config->gpio_is_open_drain)
> - cfg.ena_gpio_flags |= GPIOF_OPEN_DRAIN;
> + if (config->gpio_is_open_drain) {
> + if (gflags == GPIOD_OUT_HIGH)
> + gflags = GPIOD_OUT_HIGH_OPEN_DRAIN;
> + else
> + gflags = GPIOD_OUT_LOW_OPEN_DRAIN;
> + }
> +
> + cfg.ena_gpiod = devm_gpiod_get_optional(&pdev->dev, NULL, gflags);
> + if (IS_ERR(cfg.ena_gpiod))
> + return PTR_ERR(cfg.ena_gpiod);
>
> cfg.dev = &pdev->dev;
> cfg.init_data = config->init_data;
> diff --git a/include/linux/regulator/fixed.h b/include/linux/regulator/fixed.h
> index 48918be649d4..1a4340ed8e2b 100644
> --- a/include/linux/regulator/fixed.h
> +++ b/include/linux/regulator/fixed.h
> @@ -24,8 +24,6 @@ struct regulator_init_data;
> * @supply_name: Name of the regulator supply
> * @input_supply: Name of the input regulator supply
> * @microvolts: Output voltage of regulator
> - * @gpio: GPIO to use for enable control
> - * set to -EINVAL if not used
> * @startup_delay: Start-up time in microseconds
> * @gpio_is_open_drain: Gpio pin is open drain or normal type.
> * If it is open drain type then HIGH will be set
> @@ -49,7 +47,6 @@ struct fixed_voltage_config {
> const char *supply_name;
> const char *input_supply;
> int microvolts;
> - int gpio;
> unsigned startup_delay;
> unsigned gpio_is_open_drain:1;
> unsigned enable_high:1;
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-11 9:01 ` Marek Szyprowski
@ 2018-10-11 9:29 ` Linus Walleij
2018-10-11 9:46 ` Marek Szyprowski
2018-10-11 15:00 ` Jon Hunter
0 siblings, 2 replies; 28+ messages in thread
From: Linus Walleij @ 2018-10-11 9:29 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Liam Girdwood, Mark Brown, linux-kernel@vger.kernel.org,
Janusz Krzysztofik, Alexander Shiyan, Haojian Zhuang,
Aaro Koskinen, Mike Rapoport, Robert Jarzmik, Philipp Zabel,
Daniel Mack, Marc Zyngier, jacopo, Geert Uytterhoeven,
Russell King
On Thu, Oct 11, 2018 at 11:01 AM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> I've just noticed that this patch causes regression on Samsung
> Exynos4412-based Trats2 board. Conversion to GPIO descriptor breaks
> operation when regulators used shared GPIO: sii9234 i2c driver
> is not able to get vcc33mhl regulator (it uses shared GPIO enable
> line with vsil12 regulator).
So I guess this means that this physical GPIO line will enable the
vcc33mhl and the vsil12 regulators at the same time?
> This issue has been already pointed in case of commits:
> 37fa23dbccbd97663acc085bd79246f427e603a1
> d1dae72fab2c377ff463742eefd8ac0f9e99b7b9
> ab4d11e2c2329cf7cb7be31ff22489aae4dee5dc
A big sorry for my ignorance, I guess the information overload
on the mailing list just makes me miss the important points.
I'll try to be better, sadly I constantly fail to keep everything
in mind and constantly break things like this.
> Maybe it would be better to first solve the handling of shared enable
> GPIO in the descriptor-based interface before converting more regulators
> and stepping into this issue again?
I am trying to solve it, but I just don't have systems to reproduce all
kinds of things. It's a bit stressful since this is one of those runtime
things that is hard to test when devising a patch for systems I don't
have.
I am kind of relying on system maintainers to test things.
I was aware of the usecase "several consumers takes the same
GPIO line" (Mark told me several times...) so it was in the back of
my mind, but it's just hard to see when we were gonna run into it.
So it is the fixed regulators, on Samsung boards, I see.
Anyways. Let's try to fix it now then instead of me constantly
hitting this and breaking it. It happens because it is just unintuitive
so I share your frustration.
I don't want to generally make it possible for a gpio line to be
retrieved nonexclusively. We need the semantics to help people
do the right thing.
So I was thinking to introduce
gpiod_get_nonexclusive() to explicitly handle this case for the few
systems that use shared GPIO lines, let me see what I can do.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-11 9:29 ` Linus Walleij
@ 2018-10-11 9:46 ` Marek Szyprowski
2018-10-11 13:15 ` Mark Brown
2018-10-11 15:00 ` Jon Hunter
1 sibling, 1 reply; 28+ messages in thread
From: Marek Szyprowski @ 2018-10-11 9:46 UTC (permalink / raw)
To: Linus Walleij
Cc: Liam Girdwood, Mark Brown, linux-kernel@vger.kernel.org,
Janusz Krzysztofik, Alexander Shiyan, Haojian Zhuang,
Aaro Koskinen, Mike Rapoport, Robert Jarzmik, Philipp Zabel,
Daniel Mack, Marc Zyngier, jacopo, Geert Uytterhoeven,
Russell King
Hi Linus,
On 2018-10-11 11:29, Linus Walleij wrote:
> On Thu, Oct 11, 2018 at 11:01 AM Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>
>> I've just noticed that this patch causes regression on Samsung
>> Exynos4412-based Trats2 board. Conversion to GPIO descriptor breaks
>> operation when regulators used shared GPIO: sii9234 i2c driver
>> is not able to get vcc33mhl regulator (it uses shared GPIO enable
>> line with vsil12 regulator).
> So I guess this means that this physical GPIO line will enable the
> vcc33mhl and the vsil12 regulators at the same time?
Right. It is so common case, that regulator core has special code for
handling shared enable GPIO.
>> This issue has been already pointed in case of commits:
>> 37fa23dbccbd97663acc085bd79246f427e603a1
>> d1dae72fab2c377ff463742eefd8ac0f9e99b7b9
>> ab4d11e2c2329cf7cb7be31ff22489aae4dee5dc
> A big sorry for my ignorance, I guess the information overload
> on the mailing list just makes me miss the important points.
> I'll try to be better, sadly I constantly fail to keep everything
> in mind and constantly break things like this.
>
>> Maybe it would be better to first solve the handling of shared enable
>> GPIO in the descriptor-based interface before converting more regulators
>> and stepping into this issue again?
> I am trying to solve it, but I just don't have systems to reproduce all
> kinds of things. It's a bit stressful since this is one of those runtime
> things that is hard to test when devising a patch for systems I don't
> have.
>
> I am kind of relying on system maintainers to test things.
>
> I was aware of the usecase "several consumers takes the same
> GPIO line" (Mark told me several times...) so it was in the back of
> my mind, but it's just hard to see when we were gonna run into it.
> So it is the fixed regulators, on Samsung boards, I see.
I don't think this is Samsung specific. I saw similar solution on various
other boards too. Sharing enable gpio is rather common thing. The issue
happens if there are separate drivers for each hw block and they need to
enable it from their code.
> Anyways. Let's try to fix it now then instead of me constantly
> hitting this and breaking it. It happens because it is just unintuitive
> so I share your frustration.
I try to test linux-next daily to catch such thing as early as possible
and help others to fix their code. Feel free add me on cc: so I will
test it.
> I don't want to generally make it possible for a gpio line to be
> retrieved nonexclusively. We need the semantics to help people
> do the right thing.
>
> So I was thinking to introduce
> gpiod_get_nonexclusive() to explicitly handle this case for the few
> systems that use shared GPIO lines, let me see what I can do.
The old interface also didn't allow sharing GPIO easily, so regulator
core has special code for shared enable gpio. However I still have no
idea how to do this cleanly using descriptor.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-11 9:46 ` Marek Szyprowski
@ 2018-10-11 13:15 ` Mark Brown
0 siblings, 0 replies; 28+ messages in thread
From: Mark Brown @ 2018-10-11 13:15 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Linus Walleij, Liam Girdwood, linux-kernel@vger.kernel.org,
Janusz Krzysztofik, Alexander Shiyan, Haojian Zhuang,
Aaro Koskinen, Mike Rapoport, Robert Jarzmik, Philipp Zabel,
Daniel Mack, Marc Zyngier, jacopo, Geert Uytterhoeven,
Russell King
[-- Attachment #1: Type: text/plain, Size: 2243 bytes --]
On Thu, Oct 11, 2018 at 11:46:59AM +0200, Marek Szyprowski wrote:
> On 2018-10-11 11:29, Linus Walleij wrote:
> > On Thu, Oct 11, 2018 at 11:01 AM Marek Szyprowski
> >> I've just noticed that this patch causes regression on Samsung
> >> Exynos4412-based Trats2 board. Conversion to GPIO descriptor breaks
> >> operation when regulators used shared GPIO: sii9234 i2c driver
> >> is not able to get vcc33mhl regulator (it uses shared GPIO enable
> >> line with vsil12 regulator).
> > So I guess this means that this physical GPIO line will enable the
> > vcc33mhl and the vsil12 regulators at the same time?
> Right. It is so common case, that regulator core has special code for
> handling shared enable GPIO.
Yes, and we've discussed it several times already.
> > I was aware of the usecase "several consumers takes the same
> > GPIO line" (Mark told me several times...) so it was in the back of
> > my mind, but it's just hard to see when we were gonna run into it.
> > So it is the fixed regulators, on Samsung boards, I see.
> I don't think this is Samsung specific. I saw similar solution on various
> other boards too. Sharing enable gpio is rather common thing. The issue
> happens if there are separate drivers for each hw block and they need to
> enable it from their code.
It's really common, yeah - things like controlling the power for an
entire chip with one GPIO for all the regulators supplying that chip for
example. I think what you're seeing here is that Samsung are among the
most active testers of -next (thanks!) rather than that their hardware
is particularly weird.
> > So I was thinking to introduce
> > gpiod_get_nonexclusive() to explicitly handle this case for the few
> > systems that use shared GPIO lines, let me see what I can do.
> The old interface also didn't allow sharing GPIO easily, so regulator
> core has special code for shared enable gpio. However I still have no
> idea how to do this cleanly using descriptor.
Yes, it's been discussed several times and I thought Linus had some idea
for it - IIRC it was a gpiod_is_equal() or something. You can also do
this by converting descriptors back to numbers and comparing the numbers
but obviously the numbers are supposed to be being removed.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-11 9:29 ` Linus Walleij
2018-10-11 9:46 ` Marek Szyprowski
@ 2018-10-11 15:00 ` Jon Hunter
2018-10-11 15:34 ` Marcel Ziswiler
2018-10-11 17:45 ` Linus Walleij
1 sibling, 2 replies; 28+ messages in thread
From: Jon Hunter @ 2018-10-11 15:00 UTC (permalink / raw)
To: Linus Walleij, Marek Szyprowski
Cc: Liam Girdwood, Mark Brown, linux-kernel@vger.kernel.org,
Janusz Krzysztofik, Alexander Shiyan, Haojian Zhuang,
Aaro Koskinen, Mike Rapoport, Robert Jarzmik, Philipp Zabel,
Daniel Mack, Marc Zyngier, jacopo, Geert Uytterhoeven,
Russell King, linux-tegra
Hi Linus,
On 11/10/18 10:29, Linus Walleij wrote:
> On Thu, Oct 11, 2018 at 11:01 AM Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>
>> I've just noticed that this patch causes regression on Samsung
>> Exynos4412-based Trats2 board. Conversion to GPIO descriptor breaks
>> operation when regulators used shared GPIO: sii9234 i2c driver
>> is not able to get vcc33mhl regulator (it uses shared GPIO enable
>> line with vsil12 regulator).
>
> So I guess this means that this physical GPIO line will enable the
> vcc33mhl and the vsil12 regulators at the same time?
>
>> This issue has been already pointed in case of commits:
>> 37fa23dbccbd97663acc085bd79246f427e603a1
>> d1dae72fab2c377ff463742eefd8ac0f9e99b7b9
>> ab4d11e2c2329cf7cb7be31ff22489aae4dee5dc
>
> A big sorry for my ignorance, I guess the information overload
> on the mailing list just makes me miss the important points.
> I'll try to be better, sadly I constantly fail to keep everything
> in mind and constantly break things like this.
>
>> Maybe it would be better to first solve the handling of shared enable
>> GPIO in the descriptor-based interface before converting more regulators
>> and stepping into this issue again?
>
> I am trying to solve it, but I just don't have systems to reproduce all
> kinds of things. It's a bit stressful since this is one of those runtime
> things that is hard to test when devising a patch for systems I don't
> have.
This also appears to be causing a regression on the Tegra124 Jetson TK1
that also uses a shared GPIO for two regulators. The 2nd regulator that
uses the GPIO now fails to probe [0] ...
[ 0.680021] +5V_SATA: supplied by +5V_SYS
[ 0.683964] reg-fixed-voltage: probe of regulators:regulator@14 failed with error -16
Not sure if you have one of these, but otherwise I can help test.
Cheers
Jon
[0] https://storage.kernelci.org/next/master/next-20181011/arm/tegra_defconfig/lab-baylibre-seattle/boot-tegra124-jetson-tk1.html
--
nvpublic
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-11 15:00 ` Jon Hunter
@ 2018-10-11 15:34 ` Marcel Ziswiler
2018-10-11 17:47 ` Linus Walleij
2018-10-11 17:45 ` Linus Walleij
1 sibling, 1 reply; 28+ messages in thread
From: Marcel Ziswiler @ 2018-10-11 15:34 UTC (permalink / raw)
To: jonathanh@nvidia.com, m.szyprowski@samsung.com,
linus.walleij@linaro.org
Cc: linux-kernel@vger.kernel.org, robert.jarzmik@free.fr,
aaro.koskinen@iki.fi, jacopo@jmondi.org, broonie@kernel.org,
rmk+kernel@armlinux.org.uk, shc_work@mail.ru,
haojian.zhuang@gmail.com, lgirdwood@gmail.com,
rppt@linux.vnet.ibm.com, zonque@gmail.com, marc.zyngier@arm.com,
philipp.zabel@gmail.com, linux-tegra@vger.kernel.org,
jmkrzyszt@gmail.com, geert+renesas@glider.be
Hi Linus
On Thu, 2018-10-11 at 16:00 +0100, Jon Hunter wrote:
> Hi Linus,
>
> On 11/10/18 10:29, Linus Walleij wrote:
> > On Thu, Oct 11, 2018 at 11:01 AM Marek Szyprowski
> > <m.szyprowski@samsung.com> wrote:
> >
> > > I've just noticed that this patch causes regression on Samsung
> > > Exynos4412-based Trats2 board. Conversion to GPIO descriptor
> > > breaks
> > > operation when regulators used shared GPIO: sii9234 i2c driver
> > > is not able to get vcc33mhl regulator (it uses shared GPIO enable
> > > line with vsil12 regulator).
> >
> > So I guess this means that this physical GPIO line will enable the
> > vcc33mhl and the vsil12 regulators at the same time?
> >
> > > This issue has been already pointed in case of commits:
> > > 37fa23dbccbd97663acc085bd79246f427e603a1
> > > d1dae72fab2c377ff463742eefd8ac0f9e99b7b9
> > > ab4d11e2c2329cf7cb7be31ff22489aae4dee5dc
> >
> > A big sorry for my ignorance, I guess the information overload
> > on the mailing list just makes me miss the important points.
> > I'll try to be better, sadly I constantly fail to keep everything
> > in mind and constantly break things like this.
> >
> > > Maybe it would be better to first solve the handling of shared
> > > enable
> > > GPIO in the descriptor-based interface before converting more
> > > regulators
> > > and stepping into this issue again?
> >
> > I am trying to solve it, but I just don't have systems to reproduce
> > all
> > kinds of things. It's a bit stressful since this is one of those
> > runtime
> > things that is hard to test when devising a patch for systems I
> > don't
> > have.
>
> This also appears to be causing a regression on the Tegra124 Jetson
> TK1
> that also uses a shared GPIO for two regulators. The 2nd regulator
> that
> uses the GPIO now fails to probe [0] ...
>
> [ 0.680021] +5V_SATA: supplied by +5V_SYS
> [ 0.683964] reg-fixed-voltage: probe of regulators:regulator@14
> failed with error -16
>
> Not sure if you have one of these, but otherwise I can help test.
I guess that is also what broke HDMI on Apalis/Colibri T30 causing me
to submit a fix [1]. I may also help testing.
BTW: Is it only me or is today's -next completely broken now?
[ 0.691258] Unable to handle kernel NULL pointer dereference at
virtual address 000001f8
[ 0.699704] pgd = (ptrval)
[ 0.702515] [000001f8] *pgd=00000000
[ 0.706236] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[ 0.711749] Modules linked in:
[ 0.714930] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc7-
next-20181011-dirty #3
[ 0.723245] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[ 0.729765] PC is at gpiod_hog+0x2c/0x150
[ 0.733933] LR is at of_gpiochip_add+0x34c/0x510
This has been observed on Apalis TK1.
> Cheers
> Jon
>
> [0] https://storage.kernelci.org/next/master/next-20181011/arm/tegra_
> defconfig/lab-baylibre-seattle/boot-tegra124-jetson-tk1.html
Cheers
Marcel
[1] https://lore.kernel.org/lkml/20181009152523.3771-4-marcel@ziswiler.com
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-11 15:34 ` Marcel Ziswiler
@ 2018-10-11 17:47 ` Linus Walleij
2018-10-12 9:43 ` Marcel Ziswiler
0 siblings, 1 reply; 28+ messages in thread
From: Linus Walleij @ 2018-10-11 17:47 UTC (permalink / raw)
To: Marcel Ziswiler
Cc: Jon Hunter, Marek Szyprowski, linux-kernel@vger.kernel.org,
Robert Jarzmik, Aaro Koskinen, jacopo, Mark Brown, Russell King,
Alexander Shiyan, Haojian Zhuang, Liam Girdwood, Mike Rapoport,
Daniel Mack, Marc Zyngier, Philipp Zabel, linux-tegra,
Janusz Krzysztofik, Geert Uytterhoeven
On Thu, Oct 11, 2018 at 5:34 PM Marcel Ziswiler
<marcel.ziswiler@toradex.com> wrote:
> I guess that is also what broke HDMI on Apalis/Colibri T30 causing me
> to submit a fix [1]. I may also help testing.
I see there are many ways to skin this cat.
Does this patch fix things for you as well?
https://marc.info/?l=linux-kernel&m=153926854327176&w=2
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-11 17:47 ` Linus Walleij
@ 2018-10-12 9:43 ` Marcel Ziswiler
2018-10-12 10:39 ` Jon Hunter
0 siblings, 1 reply; 28+ messages in thread
From: Marcel Ziswiler @ 2018-10-12 9:43 UTC (permalink / raw)
To: linus.walleij@linaro.org
Cc: linux-kernel@vger.kernel.org, jonathanh@nvidia.com,
robert.jarzmik@free.fr, aaro.koskinen@iki.fi, jacopo@jmondi.org,
m.szyprowski@samsung.com, rmk+kernel@armlinux.org.uk,
broonie@kernel.org, shc_work@mail.ru, haojian.zhuang@gmail.com,
lgirdwood@gmail.com, rppt@linux.vnet.ibm.com, zonque@gmail.com,
marc.zyngier@arm.com, philipp.zabel@gmail.com,
linux-tegra@vger.kernel.org, jmkrzyszt@gmail.com,
geert+renesas@glider.be
On Thu, 2018-10-11 at 19:47 +0200, Linus Walleij wrote:
> On Thu, Oct 11, 2018 at 5:34 PM Marcel Ziswiler
> <marcel.ziswiler@toradex.com> wrote:
>
> > I guess that is also what broke HDMI on Apalis/Colibri T30 causing
> > me
> > to submit a fix [1]. I may also help testing.
>
> I see there are many ways to skin this cat.
Yes, as a matter of fact I screened the kernel concerning this multi
gpio stuff but could not quite find many examples and no mentioning
anywhere whether or not this is actually allowed. So I kind of assumed
that this may just not really be allowed and cooked up my patch which
is anyway kind of a cleaner solution. I mean explicitly modelling the
GPIO into some intermediate regulator supplying the others.
> Does this patch fix things for you as well?
> https://marc.info/?l=linux-kernel&m=153926854327176&w=2
Yes, da-da and HDMI works again. Thanks!
I will still try to get my other patch in as like mentioned above I
feel it is a cleaner solution to our regulator setup.
BTW: Have you heard of lore as well? I believe it is the better mailing
list archive as of today:
https://lore.kernel.org/lkml/20181011143531.7195-1-linus.walleij@linaro
.org/
> Yours,
> Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-12 9:43 ` Marcel Ziswiler
@ 2018-10-12 10:39 ` Jon Hunter
2018-10-12 10:43 ` Russell King - ARM Linux
0 siblings, 1 reply; 28+ messages in thread
From: Jon Hunter @ 2018-10-12 10:39 UTC (permalink / raw)
To: Marcel Ziswiler, linus.walleij@linaro.org
Cc: linux-kernel@vger.kernel.org, robert.jarzmik@free.fr,
aaro.koskinen@iki.fi, jacopo@jmondi.org, m.szyprowski@samsung.com,
rmk+kernel@armlinux.org.uk, broonie@kernel.org, shc_work@mail.ru,
haojian.zhuang@gmail.com, lgirdwood@gmail.com,
rppt@linux.vnet.ibm.com, zonque@gmail.com, marc.zyngier@arm.com,
philipp.zabel@gmail.com, linux-tegra@vger.kernel.org,
jmkrzyszt@gmail.com, geert+renesas@glider.be
On 12/10/18 10:43, Marcel Ziswiler wrote:
> On Thu, 2018-10-11 at 19:47 +0200, Linus Walleij wrote:
>> On Thu, Oct 11, 2018 at 5:34 PM Marcel Ziswiler
>> <marcel.ziswiler@toradex.com> wrote:
>>
>>> I guess that is also what broke HDMI on Apalis/Colibri T30 causing
>>> me
>>> to submit a fix [1]. I may also help testing.
>>
>> I see there are many ways to skin this cat.
>
> Yes, as a matter of fact I screened the kernel concerning this multi
> gpio stuff but could not quite find many examples and no mentioning
> anywhere whether or not this is actually allowed. So I kind of assumed
> that this may just not really be allowed and cooked up my patch which
> is anyway kind of a cleaner solution. I mean explicitly modelling the
> GPIO into some intermediate regulator supplying the others.
See the function 'regulator_ena_gpio_request()', it states that the same
GPIO pin can be shared among regulators.
We had the same situation for Tegra124 Jetson TK1 but I don't think that
adding a pseudo intermediate regulator is cleaner. If the GPIO controls
more than one regulator, I don't see why is it necessary to change the
DT. There are several other people reporting the same problem with
various different boards. So this does seem to be a common usage.
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-12 10:39 ` Jon Hunter
@ 2018-10-12 10:43 ` Russell King - ARM Linux
2018-10-12 11:03 ` Linus Walleij
` (2 more replies)
0 siblings, 3 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2018-10-12 10:43 UTC (permalink / raw)
To: Jon Hunter
Cc: Marcel Ziswiler, linus.walleij@linaro.org,
linux-kernel@vger.kernel.org, robert.jarzmik@free.fr,
aaro.koskinen@iki.fi, jacopo@jmondi.org, m.szyprowski@samsung.com,
broonie@kernel.org, shc_work@mail.ru, haojian.zhuang@gmail.com,
lgirdwood@gmail.com, rppt@linux.vnet.ibm.com, zonque@gmail.com,
marc.zyngier@arm.com, philipp.zabel@gmail.com,
linux-tegra@vger.kernel.org, jmkrzyszt@gmail.com,
geert+renesas@glider.be
On Fri, Oct 12, 2018 at 11:39:15AM +0100, Jon Hunter wrote:
> We had the same situation for Tegra124 Jetson TK1 but I don't think that
> adding a pseudo intermediate regulator is cleaner. If the GPIO controls
> more than one regulator, I don't see why is it necessary to change the
> DT. There are several other people reporting the same problem with
> various different boards. So this does seem to be a common usage.
Given that DT describes the hardware, not the software implementation,
it must not change just because we move from GPIO numbers to GPIO
descriptors.
The existing DT description is reasonable, and introducing ficticious
regulators in DT to work around the implementation is not reasonable.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-12 10:43 ` Russell King - ARM Linux
@ 2018-10-12 11:03 ` Linus Walleij
2018-10-12 11:43 ` Marcel Ziswiler
2018-10-12 13:58 ` Andy Shevchenko
2 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2018-10-12 11:03 UTC (permalink / raw)
To: Russell King
Cc: Jon Hunter, Marcel Ziswiler, linux-kernel@vger.kernel.org,
Robert Jarzmik, Aaro Koskinen, jacopo, Marek Szyprowski,
Mark Brown, Alexander Shiyan, Haojian Zhuang, Liam Girdwood,
Mike Rapoport, Daniel Mack, Marc Zyngier, Philipp Zabel,
linux-tegra, Janusz Krzysztofik, Geert Uytterhoeven
On Fri, Oct 12, 2018 at 12:43 PM Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> Given that DT describes the hardware, not the software implementation,
> it must not change just because we move from GPIO numbers to GPIO
> descriptors.
>
> The existing DT description is reasonable, and introducing ficticious
> regulators in DT to work around the implementation is not reasonable.
You're right. In the electronics and the device tree it
makes perfect sense for the same line to enable/disable several
regulators.
The patch I made is a quick hack to allow multiple users of the
same descriptor, I think the long term fix is simply allow multiple
users where applicable and maintain a reference count just like
the regulator core is doing, assert the GPIO when the first
consumer asserts it and de-assert it when the last consumer
de-asserts it.
What the old and current gpiolib does us just
call the callback to enable/disable the line immediately as
response to the callback asserting/deasserting the GPIO,
which is just too simplified.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-12 10:43 ` Russell King - ARM Linux
2018-10-12 11:03 ` Linus Walleij
@ 2018-10-12 11:43 ` Marcel Ziswiler
2018-10-12 12:59 ` Russell King - ARM Linux
2018-10-12 16:57 ` Mark Brown
2018-10-12 13:58 ` Andy Shevchenko
2 siblings, 2 replies; 28+ messages in thread
From: Marcel Ziswiler @ 2018-10-12 11:43 UTC (permalink / raw)
To: linux@armlinux.org.uk, jonathanh@nvidia.com
Cc: linux-kernel@vger.kernel.org, robert.jarzmik@free.fr,
aaro.koskinen@iki.fi, jacopo@jmondi.org, linus.walleij@linaro.org,
m.szyprowski@samsung.com, broonie@kernel.org, shc_work@mail.ru,
haojian.zhuang@gmail.com, lgirdwood@gmail.com,
rppt@linux.vnet.ibm.com, zonque@gmail.com, marc.zyngier@arm.com,
philipp.zabel@gmail.com, linux-tegra@vger.kernel.org,
jmkrzyszt@gmail.com, geert+renesas@glider.be
On Fri, 2018-10-12 at 11:43 +0100, Russell King - ARM Linux wrote:
> On Fri, Oct 12, 2018 at 11:39:15AM +0100, Jon Hunter wrote:
> > We had the same situation for Tegra124 Jetson TK1 but I don't think
> > that
> > adding a pseudo intermediate regulator is cleaner. If the GPIO
> > controls
> > more than one regulator, I don't see why is it necessary to change
> > the
> > DT. There are several other people reporting the same problem with
> > various different boards. So this does seem to be a common usage.
>
> Given that DT describes the hardware, not the software
> implementation,
> it must not change just because we move from GPIO numbers to GPIO
> descriptors.
Yes, that I do agree. However, like mentioned before on quick glance I
really could not find any documentation about this "GPIO sharing" being
allowed or not.
> The existing DT description is reasonable, and introducing ficticious
> regulators in DT to work around the implementation is not reasonable.
I don't think it is that fictitious as it makes it crystal clear that
there is something shared with all its pros and cons. E.g. what happens
if one of them regulators wants to turn off while the other one still
needs power? The regular regulator dependency tree would nicely make
this all clear.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-12 11:43 ` Marcel Ziswiler
@ 2018-10-12 12:59 ` Russell King - ARM Linux
2018-10-12 13:13 ` Marcel Ziswiler
2018-10-12 16:57 ` Mark Brown
1 sibling, 1 reply; 28+ messages in thread
From: Russell King - ARM Linux @ 2018-10-12 12:59 UTC (permalink / raw)
To: Marcel Ziswiler
Cc: jonathanh@nvidia.com, linux-kernel@vger.kernel.org,
robert.jarzmik@free.fr, aaro.koskinen@iki.fi, jacopo@jmondi.org,
linus.walleij@linaro.org, m.szyprowski@samsung.com,
broonie@kernel.org, shc_work@mail.ru, haojian.zhuang@gmail.com,
lgirdwood@gmail.com, rppt@linux.vnet.ibm.com, zonque@gmail.com,
marc.zyngier@arm.com, philipp.zabel@gmail.com,
linux-tegra@vger.kernel.org, jmkrzyszt@gmail.com,
geert+renesas@glider.be
On Fri, Oct 12, 2018 at 11:43:13AM +0000, Marcel Ziswiler wrote:
> I don't think it is that fictitious as it makes it crystal clear that
> there is something shared with all its pros and cons. E.g. what happens
> if one of them regulators wants to turn off while the other one still
> needs power? The regular regulator dependency tree would nicely make
> this all clear.
If you're introducing a regulator that doesn't exist in reality
just to be able to share a GPIO line that is wired to several
real regulators, then it _is_ ficticious. You're not describing
the hardware, you're describing something else to work around the
shortcomings of the implementation that can't cope with how stuff
is wired up in the real world. You're making the DT description
fit the software implementation, rather than the software
implementation fit the real world hardware.
Having a single GPIO that controls multiple separate regulators
which have entirely separate supplies of their own is very common
in electronics.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-12 12:59 ` Russell King - ARM Linux
@ 2018-10-12 13:13 ` Marcel Ziswiler
0 siblings, 0 replies; 28+ messages in thread
From: Marcel Ziswiler @ 2018-10-12 13:13 UTC (permalink / raw)
To: linux@armlinux.org.uk
Cc: linux-kernel@vger.kernel.org, jonathanh@nvidia.com,
robert.jarzmik@free.fr, aaro.koskinen@iki.fi, jacopo@jmondi.org,
linus.walleij@linaro.org, m.szyprowski@samsung.com,
broonie@kernel.org, shc_work@mail.ru, haojian.zhuang@gmail.com,
lgirdwood@gmail.com, rppt@linux.vnet.ibm.com, zonque@gmail.com,
marc.zyngier@arm.com, philipp.zabel@gmail.com,
linux-tegra@vger.kernel.org, jmkrzyszt@gmail.com,
geert+renesas@glider.be
On Fri, 2018-10-12 at 13:59 +0100, Russell King - ARM Linux wrote:
> On Fri, Oct 12, 2018 at 11:43:13AM +0000, Marcel Ziswiler wrote:
> > I don't think it is that fictitious as it makes it crystal clear
> > that
> > there is something shared with all its pros and cons. E.g. what
> > happens
> > if one of them regulators wants to turn off while the other one
> > still
> > needs power? The regular regulator dependency tree would nicely
> > make
> > this all clear.
>
> If you're introducing a regulator that doesn't exist in reality
> just to be able to share a GPIO line that is wired to several
> real regulators, then it _is_ ficticious. You're not describing
> the hardware, you're describing something else to work around the
> shortcomings of the implementation that can't cope with how stuff
> is wired up in the real world. You're making the DT description
> fit the software implementation, rather than the software
> implementation fit the real world hardware.
>
> Having a single GPIO that controls multiple separate regulators
> which have entirely separate supplies of their own is very common
> in electronics.
Sure, fine. I will drop it. Thanks!
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-12 11:43 ` Marcel Ziswiler
2018-10-12 12:59 ` Russell King - ARM Linux
@ 2018-10-12 16:57 ` Mark Brown
1 sibling, 0 replies; 28+ messages in thread
From: Mark Brown @ 2018-10-12 16:57 UTC (permalink / raw)
To: Marcel Ziswiler
Cc: linux@armlinux.org.uk, jonathanh@nvidia.com,
linux-kernel@vger.kernel.org, robert.jarzmik@free.fr,
aaro.koskinen@iki.fi, jacopo@jmondi.org, linus.walleij@linaro.org,
m.szyprowski@samsung.com, shc_work@mail.ru,
haojian.zhuang@gmail.com, lgirdwood@gmail.com,
rppt@linux.vnet.ibm.com, zonque@gmail.com, marc.zyngier@arm.com,
philipp.zabel@gmail.com, linux-tegra@vger.kernel.org,
jmkrzyszt@gmail.com, geert+renesas@glider.be
[-- Attachment #1: Type: text/plain, Size: 993 bytes --]
On Fri, Oct 12, 2018 at 11:43:13AM +0000, Marcel Ziswiler wrote:
> On Fri, 2018-10-12 at 11:43 +0100, Russell King - ARM Linux wrote:
> > The existing DT description is reasonable, and introducing ficticious
> > regulators in DT to work around the implementation is not reasonable.
> I don't think it is that fictitious as it makes it crystal clear that
> there is something shared with all its pros and cons. E.g. what happens
> if one of them regulators wants to turn off while the other one still
> needs power? The regular regulator dependency tree would nicely make
> this all clear.
We already have code to handle that via refcounting on the GPIO once we
identify that it's the same GPIO. If we make a shared virtual parent
regulator that'll break other things where we're tracking what the
actual physical parent for voltage reasons like adjusting parent
voltages up and down to improve efficiency or handling things that are
just dumb power switches rather than actual regulators.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-12 10:43 ` Russell King - ARM Linux
2018-10-12 11:03 ` Linus Walleij
2018-10-12 11:43 ` Marcel Ziswiler
@ 2018-10-12 13:58 ` Andy Shevchenko
2018-10-12 16:17 ` Mark Brown
2 siblings, 1 reply; 28+ messages in thread
From: Andy Shevchenko @ 2018-10-12 13:58 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Jon Hunter, Marcel Ziswiler, Linus Walleij,
Linux Kernel Mailing List, Robert Jarzmik, Aaro Koskinen, jmondi,
Marek Szyprowski, Mark Brown, Alexander Shiyan, Haojian Zhuang,
Liam Girdwood, Mike Rapoport, Daniel Mack, Marc Zyngier,
philipp.zabel, linux-tegra, Janusz Krzysztofik,
Geert Uytterhoeven
On Fri, Oct 12, 2018 at 1:45 PM Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
>
> On Fri, Oct 12, 2018 at 11:39:15AM +0100, Jon Hunter wrote:
> > We had the same situation for Tegra124 Jetson TK1 but I don't think that
> > adding a pseudo intermediate regulator is cleaner. If the GPIO controls
> > more than one regulator, I don't see why is it necessary to change the
> > DT. There are several other people reporting the same problem with
> > various different boards. So this does seem to be a common usage.
>
> Given that DT describes the hardware, not the software implementation,
> it must not change just because we move from GPIO numbers to GPIO
> descriptors.
>
> The existing DT description is reasonable, and introducing ficticious
> regulators in DT to work around the implementation is not reasonable.
If there is no way to detect shared use of GPIO line for regulators
(*) from current DT description, DT description should be updated to
reflect, yes, hardware.
(*) Not familiar with the guts of DT descriptive language, don't know
if there are some ways to do a such without additional flags or so.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-12 13:58 ` Andy Shevchenko
@ 2018-10-12 16:17 ` Mark Brown
0 siblings, 0 replies; 28+ messages in thread
From: Mark Brown @ 2018-10-12 16:17 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Russell King - ARM Linux, Jon Hunter, Marcel Ziswiler,
Linus Walleij, Linux Kernel Mailing List, Robert Jarzmik,
Aaro Koskinen, jmondi, Marek Szyprowski, Alexander Shiyan,
Haojian Zhuang, Liam Girdwood, Mike Rapoport, Daniel Mack,
Marc Zyngier, philipp.zabel, linux-tegra, Janusz Krzysztofik,
Geert Uytterhoeven
[-- Attachment #1: Type: text/plain, Size: 1089 bytes --]
On Fri, Oct 12, 2018 at 04:58:38PM +0300, Andy Shevchenko wrote:
> On Fri, Oct 12, 2018 at 1:45 PM Russell King - ARM Linux
> > Given that DT describes the hardware, not the software implementation,
> > it must not change just because we move from GPIO numbers to GPIO
> > descriptors.
> > The existing DT description is reasonable, and introducing ficticious
> > regulators in DT to work around the implementation is not reasonable.
> If there is no way to detect shared use of GPIO line for regulators
> (*) from current DT description, DT description should be updated to
> reflect, yes, hardware.
> (*) Not familiar with the guts of DT descriptive language, don't know
> if there are some ways to do a such without additional flags or so.
You can detect this via resolving the GPIOs and seeing if it points back
to something that's already in use for an enable. This isn't ideal
especially if you want to do it up front but it is doable. You could
also just assume anything might end up being shared and rather than
doing it up front which is easier and probably about as good.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-11 15:00 ` Jon Hunter
2018-10-11 15:34 ` Marcel Ziswiler
@ 2018-10-11 17:45 ` Linus Walleij
2018-10-12 10:25 ` Jon Hunter
1 sibling, 1 reply; 28+ messages in thread
From: Linus Walleij @ 2018-10-11 17:45 UTC (permalink / raw)
To: Jon Hunter
Cc: Marek Szyprowski, Liam Girdwood, Mark Brown,
linux-kernel@vger.kernel.org, Janusz Krzysztofik,
Alexander Shiyan, Haojian Zhuang, Aaro Koskinen, Mike Rapoport,
Robert Jarzmik, Philipp Zabel, Daniel Mack, Marc Zyngier, jacopo,
Geert Uytterhoeven, Russell King, linux-tegra
Hi Jon,
On Thu, Oct 11, 2018 at 5:00 PM Jon Hunter <jonathanh@nvidia.com> wrote:
> This also appears to be causing a regression on the Tegra124 Jetson TK1
> that also uses a shared GPIO for two regulators. The 2nd regulator that
> uses the GPIO now fails to probe [0] ...
>
> [ 0.680021] +5V_SATA: supplied by +5V_SYS
> [ 0.683964] reg-fixed-voltage: probe of regulators:regulator@14 failed with error -16
>
> Not sure if you have one of these, but otherwise I can help test.
Would be great if you could test the patch I came up with for Marek:
https://marc.info/?l=linux-kernel&m=153926854327176&w=2
FWIW what makes it so confusing for the GPIO maintainer with
multiple consumers is that unless there is some mechanism
(as in the regulator core) to pair them up and avoid them shaking
the GPIO from two ends, it makes little sense.
So I guess in the long run I should pull this into the gpiolib somehow.
Linus
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
2018-10-11 17:45 ` Linus Walleij
@ 2018-10-12 10:25 ` Jon Hunter
0 siblings, 0 replies; 28+ messages in thread
From: Jon Hunter @ 2018-10-12 10:25 UTC (permalink / raw)
To: Linus Walleij
Cc: Marek Szyprowski, Liam Girdwood, Mark Brown,
linux-kernel@vger.kernel.org, Janusz Krzysztofik,
Alexander Shiyan, Haojian Zhuang, Aaro Koskinen, Mike Rapoport,
Robert Jarzmik, Philipp Zabel, Daniel Mack, Marc Zyngier, jacopo,
Geert Uytterhoeven, Russell King, linux-tegra
Hi Linus,
On 11/10/18 18:45, Linus Walleij wrote:
> Hi Jon,
>
> On Thu, Oct 11, 2018 at 5:00 PM Jon Hunter <jonathanh@nvidia.com> wrote:
>
>> This also appears to be causing a regression on the Tegra124 Jetson TK1
>> that also uses a shared GPIO for two regulators. The 2nd regulator that
>> uses the GPIO now fails to probe [0] ...
>>
>> [ 0.680021] +5V_SATA: supplied by +5V_SYS
>> [ 0.683964] reg-fixed-voltage: probe of regulators:regulator@14 failed with error -16
>>
>> Not sure if you have one of these, but otherwise I can help test.
>
> Would be great if you could test the patch I came up with for Marek:
> https://marc.info/?l=linux-kernel&m=153926854327176&w=2
Yes works for me! I will respond to your actual patch as well with
tested-by.
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 28+ messages in thread