* [PATCH 0/3] gpio: brcmstb: Misc changes
@ 2016-01-06 18:55 Florian Fainelli
2016-01-06 18:55 ` [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall() Florian Fainelli
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Florian Fainelli @ 2016-01-06 18:55 UTC (permalink / raw)
To: linux-gpio
Cc: linux-mips, gregory.0xf0, jaedon.shin, linus.walleij, gnurou,
bcm-kernel-feedback-list, 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall()
2016-01-06 18:55 [PATCH 0/3] gpio: brcmstb: Misc changes Florian Fainelli
@ 2016-01-06 18:55 ` Florian Fainelli
2016-01-07 6:05 ` Gregory Fong
2016-01-07 15:28 ` Linus Walleij
2016-01-06 18:55 ` [PATCH 2/3] gpio: brcmstb: Set endian flags for big-endian MIPS Florian Fainelli
2016-01-06 18:55 ` [PATCH 3/3] gpio: brcmstb: Allow building driver for BMIPS_GENERIC Florian Fainelli
2 siblings, 2 replies; 15+ messages in thread
From: Florian Fainelli @ 2016-01-06 18:55 UTC (permalink / raw)
To: linux-gpio
Cc: linux-mips, gregory.0xf0, jaedon.shin, linus.walleij, gnurou,
bcm-kernel-feedback-list, Jim Quinlan, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] gpio: brcmstb: Set endian flags for big-endian MIPS
2016-01-06 18:55 [PATCH 0/3] gpio: brcmstb: Misc changes Florian Fainelli
2016-01-06 18:55 ` [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall() Florian Fainelli
@ 2016-01-06 18:55 ` Florian Fainelli
2016-01-07 8:08 ` Gregory Fong
2016-01-07 15:26 ` Linus Walleij
2016-01-06 18:55 ` [PATCH 3/3] gpio: brcmstb: Allow building driver for BMIPS_GENERIC Florian Fainelli
2 siblings, 2 replies; 15+ messages in thread
From: Florian Fainelli @ 2016-01-06 18:55 UTC (permalink / raw)
To: linux-gpio
Cc: linux-mips, gregory.0xf0, jaedon.shin, linus.walleij, gnurou,
bcm-kernel-feedback-list, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] gpio: brcmstb: Allow building driver for BMIPS_GENERIC
2016-01-06 18:55 [PATCH 0/3] gpio: brcmstb: Misc changes Florian Fainelli
2016-01-06 18:55 ` [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall() Florian Fainelli
2016-01-06 18:55 ` [PATCH 2/3] gpio: brcmstb: Set endian flags for big-endian MIPS Florian Fainelli
@ 2016-01-06 18:55 ` Florian Fainelli
2016-01-07 8:12 ` Gregory Fong
2016-01-07 15:27 ` Linus Walleij
2 siblings, 2 replies; 15+ messages in thread
From: Florian Fainelli @ 2016-01-06 18:55 UTC (permalink / raw)
To: linux-gpio
Cc: linux-mips, gregory.0xf0, jaedon.shin, linus.walleij, gnurou,
bcm-kernel-feedback-list, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall()
2016-01-06 18:55 ` [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall() Florian Fainelli
@ 2016-01-07 6:05 ` Gregory Fong
2016-01-07 18:12 ` Florian Fainelli
2016-01-07 15:28 ` Linus Walleij
1 sibling, 1 reply; 15+ messages in thread
From: Gregory Fong @ 2016-01-07 6:05 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-gpio, open list:MIPS, jaedon.shin, Linus Walleij,
Alexandre Courbot, bcm-kernel-feedback-list, Jim Quinlan
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] gpio: brcmstb: Set endian flags for big-endian MIPS
2016-01-06 18:55 ` [PATCH 2/3] gpio: brcmstb: Set endian flags for big-endian MIPS Florian Fainelli
@ 2016-01-07 8:08 ` Gregory Fong
2016-01-07 15:26 ` Linus Walleij
1 sibling, 0 replies; 15+ messages in thread
From: Gregory Fong @ 2016-01-07 8:08 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-gpio, open list:MIPS, jaedon.shin, Linus Walleij,
Alexandre Courbot, bcm-kernel-feedback-list
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>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] gpio: brcmstb: Allow building driver for BMIPS_GENERIC
2016-01-06 18:55 ` [PATCH 3/3] gpio: brcmstb: Allow building driver for BMIPS_GENERIC Florian Fainelli
@ 2016-01-07 8:12 ` Gregory Fong
2016-01-07 15:27 ` Linus Walleij
1 sibling, 0 replies; 15+ messages in thread
From: Gregory Fong @ 2016-01-07 8:12 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-gpio, open list:MIPS, jaedon.shin, Linus Walleij,
Alexandre Courbot, bcm-kernel-feedback-list
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] gpio: brcmstb: Set endian flags for big-endian MIPS
2016-01-06 18:55 ` [PATCH 2/3] gpio: brcmstb: Set endian flags for big-endian MIPS Florian Fainelli
2016-01-07 8:08 ` Gregory Fong
@ 2016-01-07 15:26 ` Linus Walleij
1 sibling, 0 replies; 15+ messages in thread
From: Linus Walleij @ 2016-01-07 15:26 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-gpio, Linux MIPS, Gregory Fong, jaedon.shin,
Alexandre Courbot, bcm-kernel-feedback-list
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] gpio: brcmstb: Allow building driver for BMIPS_GENERIC
2016-01-06 18:55 ` [PATCH 3/3] gpio: brcmstb: Allow building driver for BMIPS_GENERIC Florian Fainelli
2016-01-07 8:12 ` Gregory Fong
@ 2016-01-07 15:27 ` Linus Walleij
1 sibling, 0 replies; 15+ messages in thread
From: Linus Walleij @ 2016-01-07 15:27 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-gpio, Linux MIPS, Gregory Fong, jaedon.shin,
Alexandre Courbot, bcm-kernel-feedback-list
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall()
2016-01-06 18:55 ` [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall() Florian Fainelli
2016-01-07 6:05 ` Gregory Fong
@ 2016-01-07 15:28 ` Linus Walleij
1 sibling, 0 replies; 15+ messages in thread
From: Linus Walleij @ 2016-01-07 15:28 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-gpio, Linux MIPS, Gregory Fong, jaedon.shin,
Alexandre Courbot, bcm-kernel-feedback-list, Jim Quinlan
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall()
2016-01-07 6:05 ` Gregory Fong
@ 2016-01-07 18:12 ` Florian Fainelli
0 siblings, 0 replies; 15+ messages in thread
From: Florian Fainelli @ 2016-01-07 18:12 UTC (permalink / raw)
To: Gregory Fong, Florian Fainelli
Cc: linux-gpio, MIPS, jaedon.shin, Linus Walleij, Alexandre Courbot,
bcm-kernel-feedback-list, Jim Quinlan
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall()
@ 2016-01-07 18:12 ` Florian Fainelli
0 siblings, 0 replies; 15+ messages in thread
From: Florian Fainelli @ 2016-01-07 18:12 UTC (permalink / raw)
To: Gregory Fong, Florian Fainelli
Cc: linux-gpio, MIPS, jaedon.shin, Linus Walleij, Alexandre Courbot,
bcm-kernel-feedback-list, Jim Quinlan
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall()
2016-01-07 18:12 ` Florian Fainelli
(?)
@ 2016-01-19 21:18 ` Jim Quinlan
2016-01-20 9:40 ` Gregory Fong
-1 siblings, 1 reply; 15+ messages in thread
From: Jim Quinlan @ 2016-01-19 21:18 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Gregory Fong, linux-gpio
Hi, sorry I am late to this thead. Our brcmstb PCIe code is using
platform_driver_probe() which does not tolerate EPROBE_DEFER. Also, I
do not see any "cost" associated with having gpio-brcmstb.c using
subsys_initcall(); in fact, I see 33 gpio-*.c files that use
subsys_initcall(). Thanks, Jim
On Thu, Jan 7, 2016 at 1:12 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall()
2016-01-19 21:18 ` Jim Quinlan
@ 2016-01-20 9:40 ` Gregory Fong
2016-01-20 15:30 ` Jim Quinlan
0 siblings, 1 reply; 15+ messages in thread
From: Gregory Fong @ 2016-01-20 9:40 UTC (permalink / raw)
To: Jim Quinlan; +Cc: Florian Fainelli, linux-gpio
Hi Jim,
On Tue, Jan 19, 2016 at 1:18 PM, Jim Quinlan <jim2101024@gmail.com> wrote:
> Hi, sorry I am late to this thead. Our brcmstb PCIe code is using
> platform_driver_probe() which does not tolerate EPROBE_DEFER.
Can't that be changed? For instance, see this changeset which was
merged, particularly patches 3 and 4:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-August/191612.html
> Also, I
> do not see any "cost" associated with having gpio-brcmstb.c using
> subsys_initcall(); in fact, I see 33 gpio-*.c files that use
> subsys_initcall().
I've been seeing some recent patches merged that remove uses of
subsys_initcall() in favor of deferred probe. But I acknowledge it
may have some problems:
- may slow boot time because it just hacks around the dependency
problem, not really taking the required order into account
- may waste memory for non-hotpluggable devices (like PCIe root
complex) by requiring that the initialization functions be kept in
memory for the case where probe must be deferred
This isn't advising against using subsys_initcall() in the brcmstb
downstream tree as a stopgap. But I'd rather we don't contribute to
the manual initcall mess without a very good reason, and since
1. it sounds like there's a way to write the pcie root complex driver
to use deferred probe,
2. I'm not fully convinced that the cost of writing in a way to handle
deferred probe is that high vs platform_driver_probe(), and
3. the one driver that would benefit from this isn't being submitted
upstream at this time,
I don't think this patch should be applied right now.
Aside: It looks like there were some good ideas thrown around after
the 2015 Kernel Summit last November for better handling of device
dependencies: https://lwn.net/Articles/662820/ . Not sure what's
developed since then.
Best regards,
Gregory
>
> On Thu, Jan 7, 2016 at 1:12 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall()
2016-01-20 9:40 ` Gregory Fong
@ 2016-01-20 15:30 ` Jim Quinlan
0 siblings, 0 replies; 15+ messages in thread
From: Jim Quinlan @ 2016-01-20 15:30 UTC (permalink / raw)
To: Gregory Fong; +Cc: Florian Fainelli, linux-gpio
Hi Gregory,
I don't want to use deferred probes as I think I can move the PCIe RC
driver to use module_platform_driver(). It originally used
arch_initcall(), but that was for legacy reasons; in it's previous
life it was a MIPs PCIe driver, and all of the MIPs PCIe drivers use
arch/postcore_initcall() because of the way the Linux MIPs PCIe
infrastructure works. So, assuming I can do this, we do not need this
patch.
Thanks, Jim
On Wed, Jan 20, 2016 at 4:40 AM, Gregory Fong <gregory.0xf0@gmail.com> wrote:
> Hi Jim,
>
> On Tue, Jan 19, 2016 at 1:18 PM, Jim Quinlan <jim2101024@gmail.com> wrote:
>> Hi, sorry I am late to this thead. Our brcmstb PCIe code is using
>> platform_driver_probe() which does not tolerate EPROBE_DEFER.
>
> Can't that be changed? For instance, see this changeset which was
> merged, particularly patches 3 and 4:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-August/191612.html
>
>> Also, I
>> do not see any "cost" associated with having gpio-brcmstb.c using
>> subsys_initcall(); in fact, I see 33 gpio-*.c files that use
>> subsys_initcall().
>
> I've been seeing some recent patches merged that remove uses of
> subsys_initcall() in favor of deferred probe. But I acknowledge it
> may have some problems:
> - may slow boot time because it just hacks around the dependency
> problem, not really taking the required order into account
> - may waste memory for non-hotpluggable devices (like PCIe root
> complex) by requiring that the initialization functions be kept in
> memory for the case where probe must be deferred
>
> This isn't advising against using subsys_initcall() in the brcmstb
> downstream tree as a stopgap. But I'd rather we don't contribute to
> the manual initcall mess without a very good reason, and since
> 1. it sounds like there's a way to write the pcie root complex driver
> to use deferred probe,
> 2. I'm not fully convinced that the cost of writing in a way to handle
> deferred probe is that high vs platform_driver_probe(), and
> 3. the one driver that would benefit from this isn't being submitted
> upstream at this time,
> I don't think this patch should be applied right now.
>
> Aside: It looks like there were some good ideas thrown around after
> the 2015 Kernel Summit last November for better handling of device
> dependencies: https://lwn.net/Articles/662820/ . Not sure what's
> developed since then.
>
> Best regards,
> Gregory
>
>>
>> On Thu, Jan 7, 2016 at 1:12 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-01-20 15:30 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-06 18:55 [PATCH 0/3] gpio: brcmstb: Misc changes Florian Fainelli
2016-01-06 18:55 ` [PATCH 1/3] gpio: brcmstb: have driver register during subsys_initcall() Florian Fainelli
2016-01-07 6:05 ` Gregory Fong
2016-01-07 18:12 ` Florian Fainelli
2016-01-07 18:12 ` Florian Fainelli
2016-01-19 21:18 ` Jim Quinlan
2016-01-20 9:40 ` Gregory Fong
2016-01-20 15:30 ` Jim Quinlan
2016-01-07 15:28 ` Linus Walleij
2016-01-06 18:55 ` [PATCH 2/3] gpio: brcmstb: Set endian flags for big-endian MIPS Florian Fainelli
2016-01-07 8:08 ` Gregory Fong
2016-01-07 15:26 ` Linus Walleij
2016-01-06 18:55 ` [PATCH 3/3] gpio: brcmstb: Allow building driver for BMIPS_GENERIC Florian Fainelli
2016-01-07 8:12 ` Gregory Fong
2016-01-07 15:27 ` Linus Walleij
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.