[PATCH 0/3] gpio: brcmstb: Misc changes

[PATCH 0/3] gpio: brcmstb: Misc changes

[Florian Fainelli]

Hi Linus, Alexandre,

This patch series updates the brcmstb GPIO driver to support the following:

- move the initialization to subsys_initcall to allow using this driver with
  GPIO-driven regulators
- properly configure the driver for big-endian MIPS operation
- enable the driver for MIPS-based brcmstb chips

Thank you!

Florian Fainelli (2):
  gpio: brcmstb: Set endian flags for big-endian MIPS
  gpio: brcmstb: Allow building driver for BMIPS_GENERIC

Jim Quinlan (1):
  gpio: brcmstb: have driver register during subsys_initcall()

 drivers/gpio/Kconfig        |  4 ++--
 drivers/gpio/gpio-brcmstb.c | 28 ++++++++++++++++++++++++++--
 2 files changed, 28 insertions(+), 4 deletions(-)

-- 
2.1.0

[PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall()

[Florian Fainelli]

From: Jim Quinlan <jim2101024@gmail.com>

Because regulators are started with subsys_initcall(), and gpio references may
be contained in the regulators, it makes sense to start the brcmstb-gpio's with
a subsys_initcall(). The order within the drivers/Makefile ensures that the
gpio initialization happens prior to the regulator's initialization.

We need to unroll module_platform_driver() now to allow this and have custom
exit and init module functions to control the initialization level.

Signed-off-by: Jim Quinlan <jim2101024@gmail.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/gpio/gpio-brcmstb.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index dc3f0395693b..3618b9fd0cba 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -535,7 +535,18 @@ static struct platform_driver brcmstb_gpio_driver = {
 	.probe = brcmstb_gpio_probe,
 	.remove = brcmstb_gpio_remove,
 };
-module_platform_driver(brcmstb_gpio_driver);
+
+static int __init brcmstb_gpio_init(void)
+{
+	return platform_driver_register(&brcmstb_gpio_driver);
+}
+subsys_initcall(brcmstb_gpio_init);
+
+static void __exit brcmstb_gpio_exit(void)
+{
+	platform_driver_unregister(&brcmstb_gpio_driver);
+}
+module_exit(brcmstb_gpio_exit);
 
 MODULE_AUTHOR("Gregory Fong");
 MODULE_DESCRIPTION("Driver for Broadcom BRCMSTB SoC UPG GPIO");
-- 
2.1.0

Re: [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall()

[Gregory Fong]

Hello Florian and Jim,

On Wed, Jan 6, 2016 at 10:55 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> From: Jim Quinlan <jim2101024@gmail.com>
>
> Because regulators are started with subsys_initcall(), and gpio references may
> be contained in the regulators, it makes sense to start the brcmstb-gpio's with
> a subsys_initcall(). The order within the drivers/Makefile ensures that the
> gpio initialization happens prior to the regulator's initialization.
>
> We need to unroll module_platform_driver() now to allow this and have custom
> exit and init module functions to control the initialization level.

If gpio pins are needed for a regulator to come up, wouldn't it be
better to handle this using deferred probe instead of initcall-based
initialization?  Deferred probe has its problems, but I was under the
impression that it's the encouraged way to resolve these sort of
initialization order issues.

Best regards,
Gregory

Re: [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall()

[Florian Fainelli]

On 06/01/16 22:05, Gregory Fong wrote:
> Hello Florian and Jim,
> 
> On Wed, Jan 6, 2016 at 10:55 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> From: Jim Quinlan <jim2101024@gmail.com>
>>
>> Because regulators are started with subsys_initcall(), and gpio references may
>> be contained in the regulators, it makes sense to start the brcmstb-gpio's with
>> a subsys_initcall(). The order within the drivers/Makefile ensures that the
>> gpio initialization happens prior to the regulator's initialization.
>>
>> We need to unroll module_platform_driver() now to allow this and have custom
>> exit and init module functions to control the initialization level.
> 
> If gpio pins are needed for a regulator to come up, wouldn't it be
> better to handle this using deferred probe instead of initcall-based
> initialization?  Deferred probe has its problems, but I was under the
> impression that it's the encouraged way to resolve these sort of
> initialization order issues.

To give you some more context associated with this change, we now have
some boards which have GPIO-connected regulators to turn on/off PCIe
endpoint devices. In the downstream kernel, and with lack of a better
solution for now, we ended-up having the PCIE Root Complex driver to
claim these regulator, and flip them on shortly before attempting a bus
scan.

If we used deferred probing, I am assuming the sequence of events could
go like this:

- PCIe driver gets initialized, looks for regulators, cannot get a
handle on them, gets EPROBE_DEFER (arch_initcall right now)
- regulator subsystem gets initialized, does not have a valid GPIO
provider driver yet, returns EPROBE_DEFER (subsys_initcall)
- GPIO provider (gpio-brcmstb) finally gets probed and registered,
regulator get registered and available, PCIe RC driver can now use them
and power on the PCIE end point (module_initcall)

I suppose this might be working actually, let me go back to the white
board and look at this with Jim.
-- 
Florian

Re: [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall()

[Florian Fainelli]

On 06/01/16 22:05, Gregory Fong wrote:
> Hello Florian and Jim,
> 
> On Wed, Jan 6, 2016 at 10:55 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> From: Jim Quinlan <jim2101024@gmail.com>
>>
>> Because regulators are started with subsys_initcall(), and gpio references may
>> be contained in the regulators, it makes sense to start the brcmstb-gpio's with
>> a subsys_initcall(). The order within the drivers/Makefile ensures that the
>> gpio initialization happens prior to the regulator's initialization.
>>
>> We need to unroll module_platform_driver() now to allow this and have custom
>> exit and init module functions to control the initialization level.
> 
> If gpio pins are needed for a regulator to come up, wouldn't it be
> better to handle this using deferred probe instead of initcall-based
> initialization?  Deferred probe has its problems, but I was under the
> impression that it's the encouraged way to resolve these sort of
> initialization order issues.

To give you some more context associated with this change, we now have
some boards which have GPIO-connected regulators to turn on/off PCIe
endpoint devices. In the downstream kernel, and with lack of a better
solution for now, we ended-up having the PCIE Root Complex driver to
claim these regulator, and flip them on shortly before attempting a bus
scan.

If we used deferred probing, I am assuming the sequence of events could
go like this:

- PCIe driver gets initialized, looks for regulators, cannot get a
handle on them, gets EPROBE_DEFER (arch_initcall right now)
- regulator subsystem gets initialized, does not have a valid GPIO
provider driver yet, returns EPROBE_DEFER (subsys_initcall)
- GPIO provider (gpio-brcmstb) finally gets probed and registered,
regulator get registered and available, PCIe RC driver can now use them
and power on the PCIE end point (module_initcall)

I suppose this might be working actually, let me go back to the white
board and look at this with Jim.
-- 
Florian

Re: [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall()

[Linus Walleij]

On Wed, Jan 6, 2016 at 7:55 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:

> From: Jim Quinlan <jim2101024@gmail.com>
>
> Because regulators are started with subsys_initcall(), and gpio references may
> be contained in the regulators, it makes sense to start the brcmstb-gpio's with
> a subsys_initcall(). The order within the drivers/Makefile ensures that the
> gpio initialization happens prior to the regulator's initialization.
>
> We need to unroll module_platform_driver() now to allow this and have custom
> exit and init module functions to control the initialization level.
>
> Signed-off-by: Jim Quinlan <jim2101024@gmail.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

I'm holding this back until the initcall ordering discussion is
resolved, the other two patches are applied.

Yours,
Linus Walleij

[PATCH 2/3] gpio: brcmstb: Set endian flags for big-endian MIPS

[Florian Fainelli]

Broadcom MIPS-based STB chips endianness is configured by boot strap,
which also reverses all bus endianness (i.e., big-endian CPU + big
endian bus ==> native endian I/O).

Other architectures (e.g., ARM) either do not support big endian, or
else leave I/O in little endian mode.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/gpio/gpio-brcmstb.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index 3618b9fd0cba..8e8ddc76a56f 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -409,6 +409,7 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
 	int num_banks = 0;
 	int err;
 	static int gpio_base;
+	unsigned long flags = 0;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -438,6 +439,18 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
 	if (brcmstb_gpio_sanity_check_banks(dev, np, res))
 		return -EINVAL;
 
+	/*
+	 * MIPS endianness is configured by boot strap, which also reverses all
+	 * bus endianness (i.e., big-endian CPU + big endian bus ==> native
+	 * endian I/O).
+	 *
+	 * Other architectures (e.g., ARM) either do not support big endian, or
+	 * else leave I/O in little endian mode.
+	 */
+#if defined(CONFIG_MIPS) && defined(__BIG_ENDIAN)
+	flags = BGPIOF_BIG_ENDIAN_BYTE_ORDER;
+#endif
+
 	of_property_for_each_u32(np, "brcm,gpio-bank-widths", prop, p,
 			bank_width) {
 		struct brcmstb_gpio_bank *bank;
@@ -466,7 +479,7 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
 		err = bgpio_init(gc, dev, 4,
 				reg_base + GIO_DATA(bank->id),
 				NULL, NULL, NULL,
-				reg_base + GIO_IODIR(bank->id), 0);
+				reg_base + GIO_IODIR(bank->id), flags);
 		if (err) {
 			dev_err(dev, "bgpio_init() failed\n");
 			goto fail;
-- 
2.1.0

Re: [PATCH 2/3] gpio: brcmstb: Set endian flags for big-endian MIPS

[Gregory Fong]

On Wed, Jan 6, 2016 at 10:55 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> Broadcom MIPS-based STB chips endianness is configured by boot strap,
> which also reverses all bus endianness (i.e., big-endian CPU + big
> endian bus ==> native endian I/O).
>
> Other architectures (e.g., ARM) either do not support big endian, or
> else leave I/O in little endian mode.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Acked-by: Gregory Fong <gregory.0xf0@gmail.com>

Re: [PATCH 2/3] gpio: brcmstb: Set endian flags for big-endian MIPS

[Linus Walleij]

On Wed, Jan 6, 2016 at 7:55 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:

> Broadcom MIPS-based STB chips endianness is configured by boot strap,
> which also reverses all bus endianness (i.e., big-endian CPU + big
> endian bus ==> native endian I/O).
>
> Other architectures (e.g., ARM) either do not support big endian, or
> else leave I/O in little endian mode.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Patch applied with Gregory's ACK.

Yours,
Linus Walleij

[PATCH 3/3] gpio: brcmstb: Allow building driver for BMIPS_GENERIC

[Florian Fainelli]

BMIPS_GENERIC (arch/mips/bmips) is the Kconfig symbol associated with
Broadcom MIPS-based STB chips. Since this driver is perfectly usable on
these platforms as well, allow using it.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/gpio/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index b60f40a423f3..cb212ebb39ff 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -134,8 +134,8 @@ config GPIO_BCM_KONA
 
 config GPIO_BRCMSTB
 	tristate "BRCMSTB GPIO support"
-	default y if ARCH_BRCMSTB
-	depends on OF_GPIO && (ARCH_BRCMSTB || COMPILE_TEST)
+	default y if (ARCH_BRCMSTB || BMIPS_GENERIC)
+	depends on OF_GPIO && (ARCH_BRCMSTB || BMIPS_GENERIC || COMPILE_TEST)
 	select GPIO_GENERIC
 	select GPIOLIB_IRQCHIP
 	help
-- 
2.1.0

Re: [PATCH 3/3] gpio: brcmstb: Allow building driver for BMIPS_GENERIC

[Gregory Fong]

On Wed, Jan 6, 2016 at 10:55 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> BMIPS_GENERIC (arch/mips/bmips) is the Kconfig symbol associated with
> Broadcom MIPS-based STB chips. Since this driver is perfectly usable on
> these platforms as well, allow using it.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Acked-by: Gregory Fong <gregory.0xf0@gmail.com>

I never did get to test this out myself on big-endian bmips.  Glad it
worked after the bus endianness fix in patch 2/3.

Re: [PATCH 3/3] gpio: brcmstb: Allow building driver for BMIPS_GENERIC

[Linus Walleij]

On Wed, Jan 6, 2016 at 7:55 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:

> BMIPS_GENERIC (arch/mips/bmips) is the Kconfig symbol associated with
> Broadcom MIPS-based STB chips. Since this driver is perfectly usable on
> these platforms as well, allow using it.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Patch applied with Gregory's ACK.

Yours,
Linus Walleij