From: Olof Johansson <hidden> Date: 2014-01-20 03:56:52
This is a small series enhancing the MMC core code to power on modules
before the host in cases where needed, and the corresponding DT bindings
changes.
I've got some other issues to debug on the Chromebook, i.e. the interface
doens't actually work. So far it seems unrelated to this patch set so
it's worth posting this and get things going since others need the same
functionality (i.e Cubox-i).
As mentioned in the patch in the series, I haven't implemented power-down
yet, I wanted to make sure that the power-on side will be adequate for
those who are looking to use it right away.
Comments/test reports/etc welcome.
-Olof
From: Olof Johansson <hidden> Date: 2014-01-20 03:56:53
This patch enables support for power-on sequencing of SDIO peripherals through DT.
In general, it's quite common that wifi modules and other similar
peripherals have several signals in addition to the SDIO interface that
needs wiggling before the module will power on. It's common to have a
reference clock, one or several power rails and one or several lines
for reset/enable type functions.
The binding as written today introduces a number of reset gpios,
a regulator and a clock specifier. The code will handle up to 2 gpio
reset lines, but it's trivial to increase to more than that if needed
at some point.
Implementation-wise, the MMC core has been changed to handle this during
host power up, before the host interface is powered on. I have not yet
implemented the power-down side, I wanted people to have a chance for
reporting back w.r.t. issues (or comments on the bindings) first.
I have not tested the regulator portion, since the system and module
I'm working on doesn't need one (Samsung Chromebook with Marvell
8797-based wifi). Testing of those portions (and reporting back) would
be appreciated.
Signed-off-by: Olof Johansson <redacted>
---
Documentation/devicetree/bindings/mmc/mmc.txt | 11 +++++++
drivers/mmc/core/core.c | 42 +++++++++++++++++++++++++
drivers/mmc/core/host.c | 30 +++++++++++++++++-
include/linux/mmc/host.h | 5 +++
4 files changed, 87 insertions(+), 1 deletion(-)
@@ -5,6 +5,8 @@ these definitions. Interpreted by the OF core: - reg: Registers location and length. - interrupts: Interrupts used by the MMC controller.+- clocks: Clocks needed for the host controller, if any.+- clock-names: Goes with clocks above. Card detection: If no property below is supplied, host native card detect is used.
@@ -30,6 +32,15 @@ Optional properties: - cap-sdio-irq: enable SDIO IRQ signalling on this interface - full-pwr-cycle: full power cycle of the card is supported+Card power and reset control:+The following properties can be specified for cases where the MMC+peripheral needs additional reset, regulator and clock lines. It is for+example common for WiFi/BT adapters to have these separate from the main+MMC bus:+ - card-reset-gpios: Specify GPIOs for card reset (reset active low)+ - card-external-vcc-supply: Regulator to drive (independent) card VCC+ - clock with name "card_ext_clock": External clock provided to the card+ *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line polarity properties, we have to fix the meaning of the "normal" and "inverted" line levels. We choose to follow the SDHCI standard, which specifies both those
@@ -1519,6 +1521,43 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type)mmc_host_clk_release(host);}+staticvoidmmc_card_power_up(structmmc_host*host)+{+inti;+structgpio_desc**gds=host->card_reset_gpios;++for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){+if(gds[i]){+dev_dbg(host->parent,"Asserting reset line %d",i);+gpiod_set_value(gds[i],1);+}+}++if(host->card_regulator){+dev_dbg(host->parent,"Enabling external regulator");+if(regulator_enable(host->card_regulator))+dev_err(host->parent,"Failed to enable external regulator");+}++if(host->card_clk){+dev_dbg(host->parent,"Enabling external clock");+clk_prepare_enable(host->card_clk);+}++/* 2ms delay to let clocks and power settle */+mmc_delay(20);++for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){+if(gds[i]){+dev_dbg(host->parent,"Deasserting reset line %d",i);+gpiod_set_value(gds[i],0);+}+}++/* 2ms delay to after reset release */+mmc_delay(20);+}+/**ApplypowertotheMMCstack.Thisisatwo-stageprocess.*First,weenablepowertothecardwithouttheclockrunning.
@@ -1535,6 +1574,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)if(host->ios.power_mode==MMC_POWER_ON)return;+/* Power up the card/module first, if needed */+mmc_card_power_up(host);+mmc_host_clk_hold(host);host->ios.vdd=fls(ocr)-1;
@@ -312,7 +316,7 @@ int mmc_of_parse(struct mmc_host *host)u32bus_width;boolexplicit_inv_wp,gpio_inv_wp=false;enumof_gpio_flagsflags;-intlen,ret,gpio;+inti,len,ret,gpio;if(!host->parent||!host->parent->of_node)return0;
@@ -415,6 +419,30 @@ int mmc_of_parse(struct mmc_host *host)if(explicit_inv_wp^gpio_inv_wp)host->caps2|=MMC_CAP2_RO_ACTIVE_HIGH;+/* Parse card power/reset/clock control */+if(of_find_property(np,"card-reset-gpios",NULL)){+structgpio_desc*gpd;+for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){+gpd=devm_gpiod_get_index(host->parent,"card-reset",i);+if(IS_ERR(gpd))+break;+gpiod_direction_output(gpd,0);+host->card_reset_gpios[i]=gpd;+}++gpd=devm_gpiod_get_index(host->parent,"card-reset",ARRAY_SIZE(host->card_reset_gpios));+if(!IS_ERR(gpd)){+dev_warn(host->parent,"More reset gpios than we can handle");+gpiod_put(gpd);+}+}++host->card_clk=of_clk_get_by_name(np,"card_ext_clock");+if(IS_ERR(host->card_clk))+host->card_clk=NULL;++host->card_regulator=regulator_get(host->parent,"card-external-vcc");+if(of_find_property(np,"cap-sd-highspeed",&len))host->caps|=MMC_CAP_SD_HIGHSPEED;if(of_find_property(np,"cap-mmc-highspeed",&len))
@@ -297,6 +297,11 @@ struct mmc_host {unsignedlongclkgate_delay;#endif+/* card specific properties to deal with power and reset */+structregulator*card_regulator;/* External VCC needed by the card */+structgpio_desc*card_reset_gpios[2];/* External resets, active low */+structclk*card_clk;/* External clock needed by the card */+/* host specific block data */unsignedintmax_seg_size;/* see blk_queue_max_segment_size */unsignedshortmax_segs;/* see blk_queue_max_segments */
From: Olof Johansson <hidden> Date: 2014-01-20 03:56:54
The shared of parse function fills in common options for capabilities,
etc, but it needs to be called from each driver that wants to make use
of it. dw_mmc was missing the call.
Signed-off-by: Olof Johansson <redacted>
---
drivers/mmc/host/dw_mmc.c | 2 ++
1 file changed, 2 insertions(+)
From: Olof Johansson <hidden> Date: 2014-01-20 03:56:55
This adds the required specifications of reset-gpios on snow to enable wifi.
The wifi module is always powered, but it has three more signals needed:
- Enable line (active high)
- Reset line (active low)
- 32kHz reference clock
The enable and reset lines are in practice quite similar (i.e. both
need to be 1 for the module to be enabled), so treat them both as
"reset gpios". The clock is specified using the new clock property on
MMC, which due to lack of append options right now has to duplicate the
first two clocks that are SoC generic.
Also, fix the pinctrl setting (that has moved from slot node to host
controller node), and add the two gpio lines to the pinctrl setup.
Signed-off-by: Olof Johansson <redacted>
---
arch/arm/boot/dts/exynos5250-cros-common.dtsi | 17 ++++++++++++++++-
arch/arm/boot/dts/exynos5250-snow.dts | 11 +++++++++--
2 files changed, 25 insertions(+), 3 deletions(-)
Hi, Olof.
It looks good to me. When use the mmc_of_parse, we can also remove the duplicated property.
I have also worked the patch for using mmc_of_parse().
1. caps2-mmc-hs200-1_8v/1_2v capability can be moved into mmc_of_parse.
then we can remove parsing of dt into dw_mmc.c
2. keep-power-in-suspend/enable-sdio-wakeup can be removed.
If i missed other patch, let me know, plz.
how about?
if you're ok, i will send the patch-set on this week.
Best Regards,
Jaehoon Chung
On 01/20/2014 12:56 PM, Olof Johansson wrote:
quoted hunk
The shared of parse function fills in common options for capabilities,
etc, but it needs to be called from each driver that wants to make use
of it. dw_mmc was missing the call.
Signed-off-by: Olof Johansson <redacted>
---
drivers/mmc/host/dw_mmc.c | 2 ++
1 file changed, 2 insertions(+)
On 20 January 2014 04:56, Olof Johansson [off-list ref] wrote:
quoted hunk
This patch enables support for power-on sequencing of SDIO peripherals through DT.
In general, it's quite common that wifi modules and other similar
peripherals have several signals in addition to the SDIO interface that
needs wiggling before the module will power on. It's common to have a
reference clock, one or several power rails and one or several lines
for reset/enable type functions.
The binding as written today introduces a number of reset gpios,
a regulator and a clock specifier. The code will handle up to 2 gpio
reset lines, but it's trivial to increase to more than that if needed
at some point.
Implementation-wise, the MMC core has been changed to handle this during
host power up, before the host interface is powered on. I have not yet
implemented the power-down side, I wanted people to have a chance for
reporting back w.r.t. issues (or comments on the bindings) first.
I have not tested the regulator portion, since the system and module
I'm working on doesn't need one (Samsung Chromebook with Marvell
8797-based wifi). Testing of those portions (and reporting back) would
be appreciated.
Signed-off-by: Olof Johansson <redacted>
---
Documentation/devicetree/bindings/mmc/mmc.txt | 11 +++++++
drivers/mmc/core/core.c | 42 +++++++++++++++++++++++++
drivers/mmc/core/host.c | 30 +++++++++++++++++-
include/linux/mmc/host.h | 5 +++
4 files changed, 87 insertions(+), 1 deletion(-)
@@ -5,6 +5,8 @@ these definitions. Interpreted by the OF core: - reg: Registers location and length. - interrupts: Interrupts used by the MMC controller.+- clocks: Clocks needed for the host controller, if any.+- clock-names: Goes with clocks above. Card detection: If no property below is supplied, host native card detect is used.
@@ -30,6 +32,15 @@ Optional properties: - cap-sdio-irq: enable SDIO IRQ signalling on this interface - full-pwr-cycle: full power cycle of the card is supported+Card power and reset control:+The following properties can be specified for cases where the MMC+peripheral needs additional reset, regulator and clock lines. It is for+example common for WiFi/BT adapters to have these separate from the main+MMC bus:+ - card-reset-gpios: Specify GPIOs for card reset (reset active low)+ - card-external-vcc-supply: Regulator to drive (independent) card VCC+ - clock with name "card_ext_clock": External clock provided to the card+ *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line polarity properties, we have to fix the meaning of the "normal" and "inverted" line levels. We choose to follow the SDHCI standard, which specifies both those
@@ -1519,6 +1521,43 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type)mmc_host_clk_release(host);}+staticvoidmmc_card_power_up(structmmc_host*host)+{+inti;+structgpio_desc**gds=host->card_reset_gpios;++for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){+if(gds[i]){+dev_dbg(host->parent,"Asserting reset line %d",i);+gpiod_set_value(gds[i],1);+}+}++if(host->card_regulator){+dev_dbg(host->parent,"Enabling external regulator");+if(regulator_enable(host->card_regulator))+dev_err(host->parent,"Failed to enable external regulator");+}++if(host->card_clk){+dev_dbg(host->parent,"Enabling external clock");+clk_prepare_enable(host->card_clk);+}++/* 2ms delay to let clocks and power settle */+mmc_delay(20);++for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){+if(gds[i]){+dev_dbg(host->parent,"Deasserting reset line %d",i);+gpiod_set_value(gds[i],0);+}+}++/* 2ms delay to after reset release */+mmc_delay(20);+}+/**ApplypowertotheMMCstack.Thisisatwo-stageprocess.*First,weenablepowertothecardwithouttheclockrunning.
@@ -1535,6 +1574,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)if(host->ios.power_mode==MMC_POWER_ON)return;+/* Power up the card/module first, if needed */+mmc_card_power_up(host);+mmc_host_clk_hold(host);host->ios.vdd=fls(ocr)-1;
@@ -312,7 +316,7 @@ int mmc_of_parse(struct mmc_host *host)u32bus_width;boolexplicit_inv_wp,gpio_inv_wp=false;enumof_gpio_flagsflags;-intlen,ret,gpio;+inti,len,ret,gpio;if(!host->parent||!host->parent->of_node)return0;
@@ -415,6 +419,30 @@ int mmc_of_parse(struct mmc_host *host)if(explicit_inv_wp^gpio_inv_wp)host->caps2|=MMC_CAP2_RO_ACTIVE_HIGH;+/* Parse card power/reset/clock control */
I would like us to prevent to open up for confusion with the "eMMC hw
reset" when adding this. Unless we are able to combine them in some
way?
Could we maybe add some more comments about in what scenarios this DT
property would be useful?
+ if (of_find_property(np, "card-reset-gpios", NULL)) {
+ struct gpio_desc *gpd;
+ for (i = 0; i < ARRAY_SIZE(host->card_reset_gpios); i++) {
+ gpd = devm_gpiod_get_index(host->parent, "card-reset", i);
+ if (IS_ERR(gpd))
+ break;
+ gpiod_direction_output(gpd, 0);
+ host->card_reset_gpios[i] = gpd;
+ }
+
+ gpd = devm_gpiod_get_index(host->parent, "card-reset", ARRAY_SIZE(host->card_reset_gpios));
+ if (!IS_ERR(gpd)) {
+ dev_warn(host->parent, "More reset gpios than we can handle");
+ gpiod_put(gpd);
+ }
+ }
+
+ host->card_clk = of_clk_get_by_name(np, "card_ext_clock");
of_clk_get_by_name relies on COMMON_CLK, is that really what you want here?
Is the above regulator related to host->ocr_avail mask? Could the
above regulator be replaced by vmmc?
At the moment host drivers uses mmc_regulator_get_supply(), which
fetches regulators called "vmmc" and "vqmmc". It is also common to
have these defined in DT like "vmmc-supply". This has not been
properly documented for most host cases, and we should fix that. I
also think it would make sense to include these in the documentation
for the common mmc bindings, instead of host specific bindings.
Kind regards
Ulf Hansson
quoted hunk
+
if (of_find_property(np, "cap-sd-highspeed", &len))
host->caps |= MMC_CAP_SD_HIGHSPEED;
if (of_find_property(np, "cap-mmc-highspeed", &len))
@@ -297,6 +297,11 @@ struct mmc_host {unsignedlongclkgate_delay;#endif+/* card specific properties to deal with power and reset */+structregulator*card_regulator;/* External VCC needed by the card */+structgpio_desc*card_reset_gpios[2];/* External resets, active low */+structclk*card_clk;/* External clock needed by the card */+/* host specific block data */unsignedintmax_seg_size;/* see blk_queue_max_segment_size */unsignedshortmax_segs;/* see blk_queue_max_segments */--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
...this won't handle probe deferral or other errors. Given what this is
for it should probably be using regulator_get_optional(), the driver is
happy to carry on if a supply isn't available. On the other hand it
just enables and disables so it's probably easier to just ignore that
and let the stub regulator get used anyway.
I have to say I do worry what happens if the device has multiple
supplies that need to be managed (which is common enough, for example
analogue and digital supplies tend to be decoupled) or if the device can
do useful things with the supplies. In the case of SDIO it's *probably*
less relevant though I have seen applications where it might be.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140120/39e9521a/attachment.sig>
@@ -5,6 +5,8 @@ these definitions. Interpreted by the OF core: - reg: Registers location and length. - interrupts: Interrupts used by the MMC controller.+- clocks: Clocks needed for the host controller, if any.+- clock-names: Goes with clocks above. Card detection: If no property below is supplied, host native card detect is used.
@@ -30,6 +32,15 @@ Optional properties: - cap-sdio-irq: enable SDIO IRQ signalling on this interface - full-pwr-cycle: full power cycle of the card is supported+Card power and reset control:+The following properties can be specified for cases where the MMC+peripheral needs additional reset, regulator and clock lines. It is for+example common for WiFi/BT adapters to have these separate from the main+MMC bus:+ - card-reset-gpios: Specify GPIOs for card reset (reset active low)+ - card-external-vcc-supply: Regulator to drive (independent) card VCC+ - clock with name "card_ext_clock": External clock provided to the card+
This looks good. I can connect the wifi/bt power control to a regulator,
and give that as the card-external-vcc-supply property. I can specify
the WIFI/BT resets for card-reset-gpios.
So far so good. Now, what about this external oscillator which has its
own separate power control. My immediate thought is that this can be
specified via card_ext_clock - I would simply need to declare a fixed-rate
clock with either a regulator (power switch) controlled via a gpio (which
would probably be closer to the hardware) or a gpio as an enable... ah,
that requires me to write a common clock driver for that bit since this
is currently not modelled by CCF...
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
On Mon, Jan 20, 2014 at 2:48 PM, Russell King - ARM Linux
[off-list ref] wrote:
So far so good. Now, what about this external oscillator which has its
own separate power control. My immediate thought is that this can be
specified via card_ext_clock - I would simply need to declare a fixed-rate
clock with either a regulator (power switch) controlled via a gpio (which
would probably be closer to the hardware) or a gpio as an enable... ah,
that requires me to write a common clock driver for that bit since this
is currently not modelled by CCF...
Isn't this covered by the gpios property of fixed-clock?
We do the following to enable the 26MHz codec clock in
imx51-babbage.dts via GPIO4_26:
clk_26M: codec_clock {
compatible = "fixed-clock";
reg=<0>;
#clock-cells = <0>;
clock-frequency = <26000000>;
gpios = <&gpio4 26 GPIO_ACTIVE_LOW>;
};
Regards,
Fabio Estevam
From: Russell King - ARM Linux <hidden> Date: 2014-01-20 17:16:21
On Mon, Jan 20, 2014 at 03:03:50PM -0200, Fabio Estevam wrote:
On Mon, Jan 20, 2014 at 2:48 PM, Russell King - ARM Linux
[off-list ref] wrote:
quoted
So far so good. Now, what about this external oscillator which has its
own separate power control. My immediate thought is that this can be
specified via card_ext_clock - I would simply need to declare a fixed-rate
clock with either a regulator (power switch) controlled via a gpio (which
would probably be closer to the hardware) or a gpio as an enable... ah,
that requires me to write a common clock driver for that bit since this
is currently not modelled by CCF...
Isn't this covered by the gpios property of fixed-clock?
We do the following to enable the 26MHz codec clock in
imx51-babbage.dts via GPIO4_26:
clk_26M: codec_clock {
compatible = "fixed-clock";
reg=<0>;
#clock-cells = <0>;
clock-frequency = <26000000>;
gpios = <&gpio4 26 GPIO_ACTIVE_LOW>;
};
Not as far as I can see. fixed-clock appears to have two properies:
clock-frequency
clock-output-names
and nothing else. See of_fixed_clk_setup in drivers/clk/clk-fixed-rate.c.
You'll also find that the documentation in this file says this about it:
* DOC: basic fixed-rate clock that cannot gate
*
* Traits of this clock:
* prepare - clk_(un)prepare only ensures parents are prepared
* enable - clk_enable only ensures parents are enabled
* rate - rate is always a fixed value. No clk_set_rate support
* parent - fixed parent. No clk_set_parent support
So, I think the bit which you quote from imx51-babbage.dts is wishful
thinking on the part of the author of the DT file, rather than actually
being implemented in any way by the kernel DT support.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
On Mon, Jan 20, 2014 at 3:16 PM, Russell King - ARM Linux
[off-list ref] wrote:
Not as far as I can see. fixed-clock appears to have two properies:
clock-frequency
clock-output-names
and nothing else. See of_fixed_clk_setup in drivers/clk/clk-fixed-rate.c.
You'll also find that the documentation in this file says this about it:
Looks like Documentation/devicetree/bindings/clock/fixed-clock.txt is
misleading then:
"Optional properties:
- gpios : From common gpio binding; gpio connection to clock enable pin."
Regards,
Fabio Estevam
+Card power and reset control:
+The following properties can be specified for cases where the MMC
+peripheral needs additional reset, regulator and clock lines. It is for
+example common for WiFi/BT adapters to have these separate from the main
+MMC bus:
+ - card-reset-gpios: Specify GPIOs for card reset (reset active low)
+ - card-external-vcc-supply: Regulator to drive (independent) card VCC
+ - clock with name "card_ext_clock": External clock provided to the card
I wonder whether the reset line should use the generic reset controller binding
rather than the gpio binding. There has been a recent discussion about a
gpio-reset driver, which should generalize this at the reset level, and
it would make it possible to use reset controllers that are not gpio driven.
In general, any gpio can be used as a reset, but not every reset line can
be a gpio. I don't know whether anyone would use an internal reset-controller
for an external SDIO chip though, so maybe your approach is sufficient.
Arnd
From: Olof Johansson <hidden> Date: 2014-01-20 19:13:02
On Mon, Jan 20, 2014 at 09:44:23AM +0100, Ulf Hansson wrote:
On 20 January 2014 04:56, Olof Johansson [off-list ref] wrote:
quoted
This patch enables support for power-on sequencing of SDIO peripherals through DT.
In general, it's quite common that wifi modules and other similar
peripherals have several signals in addition to the SDIO interface that
needs wiggling before the module will power on. It's common to have a
reference clock, one or several power rails and one or several lines
for reset/enable type functions.
The binding as written today introduces a number of reset gpios,
a regulator and a clock specifier. The code will handle up to 2 gpio
reset lines, but it's trivial to increase to more than that if needed
at some point.
Implementation-wise, the MMC core has been changed to handle this during
host power up, before the host interface is powered on. I have not yet
implemented the power-down side, I wanted people to have a chance for
reporting back w.r.t. issues (or comments on the bindings) first.
I have not tested the regulator portion, since the system and module
I'm working on doesn't need one (Samsung Chromebook with Marvell
8797-based wifi). Testing of those portions (and reporting back) would
be appreciated.
Signed-off-by: Olof Johansson <redacted>
---
Documentation/devicetree/bindings/mmc/mmc.txt | 11 +++++++
drivers/mmc/core/core.c | 42 +++++++++++++++++++++++++
drivers/mmc/core/host.c | 30 +++++++++++++++++-
include/linux/mmc/host.h | 5 +++
4 files changed, 87 insertions(+), 1 deletion(-)
@@ -5,6 +5,8 @@ these definitions. Interpreted by the OF core: - reg: Registers location and length. - interrupts: Interrupts used by the MMC controller.+- clocks: Clocks needed for the host controller, if any.+- clock-names: Goes with clocks above. Card detection: If no property below is supplied, host native card detect is used.
@@ -30,6 +32,15 @@ Optional properties: - cap-sdio-irq: enable SDIO IRQ signalling on this interface - full-pwr-cycle: full power cycle of the card is supported+Card power and reset control:+The following properties can be specified for cases where the MMC+peripheral needs additional reset, regulator and clock lines. It is for+example common for WiFi/BT adapters to have these separate from the main+MMC bus:+ - card-reset-gpios: Specify GPIOs for card reset (reset active low)+ - card-external-vcc-supply: Regulator to drive (independent) card VCC+ - clock with name "card_ext_clock": External clock provided to the card+ *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line polarity properties, we have to fix the meaning of the "normal" and "inverted" line levels. We choose to follow the SDHCI standard, which specifies both those
@@ -1519,6 +1521,43 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type)mmc_host_clk_release(host);}+staticvoidmmc_card_power_up(structmmc_host*host)+{+inti;+structgpio_desc**gds=host->card_reset_gpios;++for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){+if(gds[i]){+dev_dbg(host->parent,"Asserting reset line %d",i);+gpiod_set_value(gds[i],1);+}+}++if(host->card_regulator){+dev_dbg(host->parent,"Enabling external regulator");+if(regulator_enable(host->card_regulator))+dev_err(host->parent,"Failed to enable external regulator");+}++if(host->card_clk){+dev_dbg(host->parent,"Enabling external clock");+clk_prepare_enable(host->card_clk);+}++/* 2ms delay to let clocks and power settle */+mmc_delay(20);++for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){+if(gds[i]){+dev_dbg(host->parent,"Deasserting reset line %d",i);+gpiod_set_value(gds[i],0);+}+}++/* 2ms delay to after reset release */+mmc_delay(20);+}+/**ApplypowertotheMMCstack.Thisisatwo-stageprocess.*First,weenablepowertothecardwithouttheclockrunning.
@@ -1535,6 +1574,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)if(host->ios.power_mode==MMC_POWER_ON)return;+/* Power up the card/module first, if needed */+mmc_card_power_up(host);+mmc_host_clk_hold(host);host->ios.vdd=fls(ocr)-1;
@@ -312,7 +316,7 @@ int mmc_of_parse(struct mmc_host *host)u32bus_width;boolexplicit_inv_wp,gpio_inv_wp=false;enumof_gpio_flagsflags;-intlen,ret,gpio;+inti,len,ret,gpio;if(!host->parent||!host->parent->of_node)return0;
@@ -415,6 +419,30 @@ int mmc_of_parse(struct mmc_host *host)if(explicit_inv_wp^gpio_inv_wp)host->caps2|=MMC_CAP2_RO_ACTIVE_HIGH;+/* Parse card power/reset/clock control */
I would like us to prevent to open up for confusion with the "eMMC hw
reset" when adding this. Unless we are able to combine them in some
way?
Could we maybe add some more comments about in what scenarios this DT
property would be useful?
Ok, can do. How about something like:
/*
* Some cards need separate power/reset/clock control from the main
* MMC/SDIO bus. Parse the description of those controls so we can
* power on the card before the host controller.
*/
quoted
+ if (of_find_property(np, "card-reset-gpios", NULL)) {
+ struct gpio_desc *gpd;
+ for (i = 0; i < ARRAY_SIZE(host->card_reset_gpios); i++) {
+ gpd = devm_gpiod_get_index(host->parent, "card-reset", i);
+ if (IS_ERR(gpd))
+ break;
+ gpiod_direction_output(gpd, 0);
+ host->card_reset_gpios[i] = gpd;
+ }
+
+ gpd = devm_gpiod_get_index(host->parent, "card-reset", ARRAY_SIZE(host->card_reset_gpios));
+ if (!IS_ERR(gpd)) {
+ dev_warn(host->parent, "More reset gpios than we can handle");
+ gpiod_put(gpd);
+ }
+ }
+
+ host->card_clk = of_clk_get_by_name(np, "card_ext_clock");
of_clk_get_by_name relies on COMMON_CLK, is that really what you want here?
Is the above regulator related to host->ocr_avail mask? Could the
above regulator be replaced by vmmc?
I have to admit that I don't know MMC as well as I could, but OCR seems to be
something that's between the driver/controller/device, not related to external
power control in this case?
At the moment host drivers uses mmc_regulator_get_supply(), which
fetches regulators called "vmmc" and "vqmmc". It is also common to
have these defined in DT like "vmmc-supply". This has not been
properly documented for most host cases, and we should fix that. I
also think it would make sense to include these in the documentation
for the common mmc bindings, instead of host specific bindings.
Hm, I had been of the impression that the vmmc stuff is to control
power/voltage on the signal lines, not for external card power. Still, even in
that case there's need for the reset line handling and clock control.
I'll take a look and see if there's a way to handle that in a properly
sequenced way and still use the same regulator.
-Olof
On Mon, Jan 20, 2014 at 1:56 AM, Olof Johansson [off-list ref] wrote:
+Card power and reset control:
+The following properties can be specified for cases where the MMC
+peripheral needs additional reset, regulator and clock lines. It is for
+example common for WiFi/BT adapters to have these separate from the main
+MMC bus:
+ - card-reset-gpios: Specify GPIOs for card reset (reset active low)
Wouldn't it be better not to assume that the card reset is always
active low and read the GPIO_ACTIVE_xxx flags instead?
Regards,
Fabio Estevam
From: Olof Johansson <hidden> Date: 2014-01-20 19:14:35
On Mon, Jan 20, 2014 at 11:14 AM, Fabio Estevam [off-list ref] wrote:
On Mon, Jan 20, 2014 at 1:56 AM, Olof Johansson [off-list ref] wrote:
quoted
+Card power and reset control:
+The following properties can be specified for cases where the MMC
+peripheral needs additional reset, regulator and clock lines. It is for
+example common for WiFi/BT adapters to have these separate from the main
+MMC bus:
+ - card-reset-gpios: Specify GPIOs for card reset (reset active low)
Wouldn't it be better not to assume that the card reset is always
active low and read the GPIO_ACTIVE_xxx flags instead?
That's actually what the code does, I need to update the text. Thanks
for spotting it.
-Olof
On Sun, Jan 19, 2014 at 07:56:53PM -0800, Olof Johansson wrote:
quoted hunk
This patch enables support for power-on sequencing of SDIO peripherals through DT.
In general, it's quite common that wifi modules and other similar
peripherals have several signals in addition to the SDIO interface that
needs wiggling before the module will power on. It's common to have a
reference clock, one or several power rails and one or several lines
for reset/enable type functions.
The binding as written today introduces a number of reset gpios,
a regulator and a clock specifier. The code will handle up to 2 gpio
reset lines, but it's trivial to increase to more than that if needed
at some point.
Implementation-wise, the MMC core has been changed to handle this during
host power up, before the host interface is powered on. I have not yet
implemented the power-down side, I wanted people to have a chance for
reporting back w.r.t. issues (or comments on the bindings) first.
I have not tested the regulator portion, since the system and module
I'm working on doesn't need one (Samsung Chromebook with Marvell
8797-based wifi). Testing of those portions (and reporting back) would
be appreciated.
Signed-off-by: Olof Johansson <redacted>
---
Documentation/devicetree/bindings/mmc/mmc.txt | 11 +++++++
drivers/mmc/core/core.c | 42 +++++++++++++++++++++++++
drivers/mmc/core/host.c | 30 +++++++++++++++++-
include/linux/mmc/host.h | 5 +++
4 files changed, 87 insertions(+), 1 deletion(-)
@@ -5,6 +5,8 @@ these definitions. Interpreted by the OF core: - reg: Registers location and length. - interrupts: Interrupts used by the MMC controller.+- clocks: Clocks needed for the host controller, if any.+- clock-names: Goes with clocks above. Card detection: If no property below is supplied, host native card detect is used.
@@ -30,6 +32,15 @@ Optional properties: - cap-sdio-irq: enable SDIO IRQ signalling on this interface - full-pwr-cycle: full power cycle of the card is supported+Card power and reset control:+The following properties can be specified for cases where the MMC+peripheral needs additional reset, regulator and clock lines. It is for+example common for WiFi/BT adapters to have these separate from the main+MMC bus:+ - card-reset-gpios: Specify GPIOs for card reset (reset active low)
We have GPIO_ACTIVE_LOW/HIGH. No need to hardcode this.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Tue, Jan 21, 2014 at 08:24:47AM +0100, Sascha Hauer wrote:
On Sun, Jan 19, 2014 at 07:56:53PM -0800, Olof Johansson wrote:
quoted
+Card power and reset control:
+The following properties can be specified for cases where the MMC
+peripheral needs additional reset, regulator and clock lines. It is for
+example common for WiFi/BT adapters to have these separate from the main
+MMC bus:
+ - card-reset-gpios: Specify GPIOs for card reset (reset active low)
We have GPIO_ACTIVE_LOW/HIGH. No need to hardcode this.
Fabio already spotted that. Sorry for the noise.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 20 January 2014 20:13, Olof Johansson [off-list ref] wrote:
On Mon, Jan 20, 2014 at 09:44:23AM +0100, Ulf Hansson wrote:
quoted
On 20 January 2014 04:56, Olof Johansson [off-list ref] wrote:
quoted
This patch enables support for power-on sequencing of SDIO peripherals through DT.
In general, it's quite common that wifi modules and other similar
peripherals have several signals in addition to the SDIO interface that
needs wiggling before the module will power on. It's common to have a
reference clock, one or several power rails and one or several lines
for reset/enable type functions.
The binding as written today introduces a number of reset gpios,
a regulator and a clock specifier. The code will handle up to 2 gpio
reset lines, but it's trivial to increase to more than that if needed
at some point.
Implementation-wise, the MMC core has been changed to handle this during
host power up, before the host interface is powered on. I have not yet
implemented the power-down side, I wanted people to have a chance for
reporting back w.r.t. issues (or comments on the bindings) first.
I have not tested the regulator portion, since the system and module
I'm working on doesn't need one (Samsung Chromebook with Marvell
8797-based wifi). Testing of those portions (and reporting back) would
be appreciated.
Signed-off-by: Olof Johansson <redacted>
---
Documentation/devicetree/bindings/mmc/mmc.txt | 11 +++++++
drivers/mmc/core/core.c | 42 +++++++++++++++++++++++++
drivers/mmc/core/host.c | 30 +++++++++++++++++-
include/linux/mmc/host.h | 5 +++
4 files changed, 87 insertions(+), 1 deletion(-)
@@ -5,6 +5,8 @@ these definitions. Interpreted by the OF core: - reg: Registers location and length. - interrupts: Interrupts used by the MMC controller.+- clocks: Clocks needed for the host controller, if any.+- clock-names: Goes with clocks above. Card detection: If no property below is supplied, host native card detect is used.
@@ -30,6 +32,15 @@ Optional properties: - cap-sdio-irq: enable SDIO IRQ signalling on this interface - full-pwr-cycle: full power cycle of the card is supported+Card power and reset control:+The following properties can be specified for cases where the MMC+peripheral needs additional reset, regulator and clock lines. It is for+example common for WiFi/BT adapters to have these separate from the main+MMC bus:+ - card-reset-gpios: Specify GPIOs for card reset (reset active low)+ - card-external-vcc-supply: Regulator to drive (independent) card VCC+ - clock with name "card_ext_clock": External clock provided to the card+ *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line polarity properties, we have to fix the meaning of the "normal" and "inverted" line levels. We choose to follow the SDHCI standard, which specifies both those
@@ -1519,6 +1521,43 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type)mmc_host_clk_release(host);}+staticvoidmmc_card_power_up(structmmc_host*host)+{+inti;+structgpio_desc**gds=host->card_reset_gpios;++for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){+if(gds[i]){+dev_dbg(host->parent,"Asserting reset line %d",i);+gpiod_set_value(gds[i],1);+}+}++if(host->card_regulator){+dev_dbg(host->parent,"Enabling external regulator");+if(regulator_enable(host->card_regulator))+dev_err(host->parent,"Failed to enable external regulator");+}++if(host->card_clk){+dev_dbg(host->parent,"Enabling external clock");+clk_prepare_enable(host->card_clk);+}++/* 2ms delay to let clocks and power settle */+mmc_delay(20);++for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){+if(gds[i]){+dev_dbg(host->parent,"Deasserting reset line %d",i);+gpiod_set_value(gds[i],0);+}+}++/* 2ms delay to after reset release */+mmc_delay(20);+}+/**ApplypowertotheMMCstack.Thisisatwo-stageprocess.*First,weenablepowertothecardwithouttheclockrunning.
@@ -1535,6 +1574,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)if(host->ios.power_mode==MMC_POWER_ON)return;+/* Power up the card/module first, if needed */+mmc_card_power_up(host);+mmc_host_clk_hold(host);host->ios.vdd=fls(ocr)-1;
@@ -312,7 +316,7 @@ int mmc_of_parse(struct mmc_host *host)u32bus_width;boolexplicit_inv_wp,gpio_inv_wp=false;enumof_gpio_flagsflags;-intlen,ret,gpio;+inti,len,ret,gpio;if(!host->parent||!host->parent->of_node)return0;
@@ -415,6 +419,30 @@ int mmc_of_parse(struct mmc_host *host)if(explicit_inv_wp^gpio_inv_wp)host->caps2|=MMC_CAP2_RO_ACTIVE_HIGH;+/* Parse card power/reset/clock control */
I would like us to prevent to open up for confusion with the "eMMC hw
reset" when adding this. Unless we are able to combine them in some
way?
Could we maybe add some more comments about in what scenarios this DT
property would be useful?
Ok, can do. How about something like:
/*
* Some cards need separate power/reset/clock control from the main
* MMC/SDIO bus. Parse the description of those controls so we can
* power on the card before the host controller.
*/
quoted
quoted
+ if (of_find_property(np, "card-reset-gpios", NULL)) {
+ struct gpio_desc *gpd;
+ for (i = 0; i < ARRAY_SIZE(host->card_reset_gpios); i++) {
+ gpd = devm_gpiod_get_index(host->parent, "card-reset", i);
+ if (IS_ERR(gpd))
+ break;
+ gpiod_direction_output(gpd, 0);
+ host->card_reset_gpios[i] = gpd;
+ }
+
+ gpd = devm_gpiod_get_index(host->parent, "card-reset", ARRAY_SIZE(host->card_reset_gpios));
+ if (!IS_ERR(gpd)) {
+ dev_warn(host->parent, "More reset gpios than we can handle");
+ gpiod_put(gpd);
+ }
+ }
+
+ host->card_clk = of_clk_get_by_name(np, "card_ext_clock");
of_clk_get_by_name relies on COMMON_CLK, is that really what you want here?
Is the above regulator related to host->ocr_avail mask? Could the
above regulator be replaced by vmmc?
I have to admit that I don't know MMC as well as I could, but OCR seems to be
something that's between the driver/controller/device, not related to external
power control in this case?
This is related to the power of the card, typically external
regulators controlled by the host driver.
Both the card and the host supports a voltage range. This range is
exactly what the OCR mask describes. At initialization of the card,
the host ocr is validated against the card ocr.
quoted
At the moment host drivers uses mmc_regulator_get_supply(), which
fetches regulators called "vmmc" and "vqmmc". It is also common to
have these defined in DT like "vmmc-supply". This has not been
properly documented for most host cases, and we should fix that. I
also think it would make sense to include these in the documentation
for the common mmc bindings, instead of host specific bindings.
Hm, I had been of the impression that the vmmc stuff is to control
power/voltage on the signal lines, not for external card power. Still, even in
that case there's need for the reset line handling and clock control.
vmmc: the power to the card.
vqmmc: the I/O voltage levels (for the signal lines).
Regarding reset, I agree, those seems to be needed.
Regarding clock control. I suppose you are referring to separate
external clocks, not affecting the SDIO/SD/MMC interface speed!?
That could make sense, but still I wonder how those shall be handled
in a fine grained power management setup. In other words, when shall
those be gated/ungated? Is the mmc core able to take the correct
decision about these?
Kind regards
Uffe
I'll take a look and see if there's a way to handle that in a properly
sequenced way and still use the same regulator.
From: Olof Johansson <hidden> Date: 2014-01-21 18:14:49
On Tue, Jan 21, 2014 at 12:55 AM, Ulf Hansson [off-list ref] wrote:
[pruning quotes a bit]
quoted
quoted
Is the above regulator related to host->ocr_avail mask? Could the
above regulator be replaced by vmmc?
I have to admit that I don't know MMC as well as I could, but OCR seems to be
something that's between the driver/controller/device, not related to external
power control in this case?
This is related to the power of the card, typically external
regulators controlled by the host driver.
Both the card and the host supports a voltage range. This range is
exactly what the OCR mask describes. At initialization of the card,
the host ocr is validated against the card ocr.
Ok, so they specify the voltage needed, but there's no way to
determine what regulator is wired up to control. So that information
is still needed (possibly through vmmc though).
quoted
quoted
At the moment host drivers uses mmc_regulator_get_supply(), which
fetches regulators called "vmmc" and "vqmmc". It is also common to
have these defined in DT like "vmmc-supply". This has not been
properly documented for most host cases, and we should fix that. I
also think it would make sense to include these in the documentation
for the common mmc bindings, instead of host specific bindings.
Hm, I had been of the impression that the vmmc stuff is to control
power/voltage on the signal lines, not for external card power. Still, even in
that case there's need for the reset line handling and clock control.
vmmc: the power to the card.
vqmmc: the I/O voltage levels (for the signal lines).
Ah, ok. So vmmc seems like it's the same then. I'll try to get some
cycles to look at it today.
Regarding reset, I agree, those seems to be needed.
Regarding clock control. I suppose you are referring to separate
external clocks, not affecting the SDIO/SD/MMC interface speed!?
That could make sense, but still I wonder how those shall be handled
in a fine grained power management setup. In other words, when shall
those be gated/ungated? Is the mmc core able to take the correct
decision about these?
The reference clock is in most cases I've seen 32kHz, and not
something that's under fine-grained power management. So it's not used
to regulate interface speed, etc.
-Olof
From: Tomasz Figa <hidden> Date: 2014-01-21 18:34:15
Hi,
On 20.01.2014 04:56, Olof Johansson wrote:
This patch enables support for power-on sequencing of SDIO peripherals through DT.
In general, it's quite common that wifi modules and other similar
peripherals have several signals in addition to the SDIO interface that
needs wiggling before the module will power on. It's common to have a
reference clock, one or several power rails and one or several lines
for reset/enable type functions.
The binding as written today introduces a number of reset gpios,
a regulator and a clock specifier. The code will handle up to 2 gpio
reset lines, but it's trivial to increase to more than that if needed
at some point.
Implementation-wise, the MMC core has been changed to handle this during
host power up, before the host interface is powered on. I have not yet
implemented the power-down side, I wanted people to have a chance for
reporting back w.r.t. issues (or comments on the bindings) first.
I have not tested the regulator portion, since the system and module
I'm working on doesn't need one (Samsung Chromebook with Marvell
8797-based wifi). Testing of those portions (and reporting back) would
be appreciated.
While I fully agree that this is an important problem that needs to be
solved, I really don't think this is the right way, because:
a) power-up sequence is really specific to the MMC device and often it's
not simply a matter of switching on one regulator or one clock, e.g.
specific time constraints need to be met.
b) you can have WLAN chips in which SDIO is just one of the options to
use as host interface, which may be also HSIC, I2C or UART. Really. See [1].
c) this is leaking device specific details to generic host code, which
isn't really elegant.
Now, to make this a bit more constructive, [2] is a solution that I came
up with (not perfect either), which simply adds a separate platform
device for the low level part of the chip. I believe this is a better
solution because:
a) you can often see such WLAN/BT combo chip as a set of separate
devices, e.g. SDIO WLAN, UART BT and a simple PMIC or management IC,
which provides power/reset control, out of band signalling and etc. for
the first two, so it isn't that bad to have a separate device node for
the last one,
b) you have full freedom of defining your DT binding with whatever data
you need, any number of clocks, regulators, GPIOs and even out of band
interrupts (IMHO the most important one).
c) you can implement power-on, power-off sequences as needed for your
particular device,
d) you have full separation of device-specific data from MMC core (or
any other subsystem simply used as a way to perform I/O to the chip).
Now what's missing there is a way to signal the MMC core or any other
transport that a device showed up and the controller should be woken up
out of standby and scan of the bus initialized. This could be done by
explicitly specifying the device as a subnode of the
MMC/UART/USB(HSIC)/I2C or whatever with a link (phandle) to the power
controller of the chip or the other way around - a link to the
MMC/UART/... controller from the power controller node.
What do you think?
Best regards,
Tomasz
References:
[1] - http://www.marvell.com/wireless/assets/marvell_avastar_88w8797.pdf
[2] -
https://review.tizen.org/git/?p=platform/kernel/linux-3.10.git;a=commitdiff;h=978bc9523622248271e4007330ae1a0eee6e0254
@@ -5,6 +5,8 @@ these definitions. Interpreted by the OF core: - reg: Registers location and length. - interrupts: Interrupts used by the MMC controller.+- clocks: Clocks needed for the host controller, if any.+- clock-names: Goes with clocks above. Card detection: If no property below is supplied, host native card detect is used.
@@ -30,6 +32,15 @@ Optional properties: - cap-sdio-irq: enable SDIO IRQ signalling on this interface - full-pwr-cycle: full power cycle of the card is supported+Card power and reset control:+The following properties can be specified for cases where the MMC+peripheral needs additional reset, regulator and clock lines. It is for+example common for WiFi/BT adapters to have these separate from the main+MMC bus:+ - card-reset-gpios: Specify GPIOs for card reset (reset active low)+ - card-external-vcc-supply: Regulator to drive (independent) card VCC+ - clock with name "card_ext_clock": External clock provided to the card+ *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line polarity properties, we have to fix the meaning of the "normal" and "inverted" line levels. We choose to follow the SDHCI standard, which specifies both those
@@ -1519,6 +1521,43 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type)mmc_host_clk_release(host);}+staticvoidmmc_card_power_up(structmmc_host*host)+{+inti;+structgpio_desc**gds=host->card_reset_gpios;++for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){+if(gds[i]){+dev_dbg(host->parent,"Asserting reset line %d",i);+gpiod_set_value(gds[i],1);+}+}++if(host->card_regulator){+dev_dbg(host->parent,"Enabling external regulator");+if(regulator_enable(host->card_regulator))+dev_err(host->parent,"Failed to enable external regulator");+}++if(host->card_clk){+dev_dbg(host->parent,"Enabling external clock");+clk_prepare_enable(host->card_clk);+}++/* 2ms delay to let clocks and power settle */+mmc_delay(20);++for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){+if(gds[i]){+dev_dbg(host->parent,"Deasserting reset line %d",i);+gpiod_set_value(gds[i],0);+}+}++/* 2ms delay to after reset release */+mmc_delay(20);+}+/**ApplypowertotheMMCstack.Thisisatwo-stageprocess.*First,weenablepowertothecardwithouttheclockrunning.
@@ -1535,6 +1574,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)if(host->ios.power_mode==MMC_POWER_ON)return;+/* Power up the card/module first, if needed */+mmc_card_power_up(host);+mmc_host_clk_hold(host);host->ios.vdd=fls(ocr)-1;
@@ -312,7 +316,7 @@ int mmc_of_parse(struct mmc_host *host)u32bus_width;boolexplicit_inv_wp,gpio_inv_wp=false;enumof_gpio_flagsflags;-intlen,ret,gpio;+inti,len,ret,gpio;if(!host->parent||!host->parent->of_node)return0;
@@ -415,6 +419,30 @@ int mmc_of_parse(struct mmc_host *host)if(explicit_inv_wp^gpio_inv_wp)host->caps2|=MMC_CAP2_RO_ACTIVE_HIGH;+/* Parse card power/reset/clock control */+if(of_find_property(np,"card-reset-gpios",NULL)){+structgpio_desc*gpd;+for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){+gpd=devm_gpiod_get_index(host->parent,"card-reset",i);+if(IS_ERR(gpd))+break;+gpiod_direction_output(gpd,0);+host->card_reset_gpios[i]=gpd;+}++gpd=devm_gpiod_get_index(host->parent,"card-reset",ARRAY_SIZE(host->card_reset_gpios));+if(!IS_ERR(gpd)){+dev_warn(host->parent,"More reset gpios than we can handle");+gpiod_put(gpd);+}+}++host->card_clk=of_clk_get_by_name(np,"card_ext_clock");+if(IS_ERR(host->card_clk))+host->card_clk=NULL;++host->card_regulator=regulator_get(host->parent,"card-external-vcc");+if(of_find_property(np,"cap-sd-highspeed",&len))host->caps|=MMC_CAP_SD_HIGHSPEED;if(of_find_property(np,"cap-mmc-highspeed",&len))
@@ -297,6 +297,11 @@ struct mmc_host {unsignedlongclkgate_delay;#endif+/* card specific properties to deal with power and reset */+structregulator*card_regulator;/* External VCC needed by the card */+structgpio_desc*card_reset_gpios[2];/* External resets, active low */+structclk*card_clk;/* External clock needed by the card */+/* host specific block data */unsignedintmax_seg_size;/* see blk_queue_max_segment_size */unsignedshortmax_segs;/* see blk_queue_max_segments */
From: Russell King - ARM Linux <hidden> Date: 2014-01-21 19:19:49
On Mon, Jan 20, 2014 at 04:47:34PM -0200, Fabio Estevam wrote:
On Mon, Jan 20, 2014 at 3:16 PM, Russell King - ARM Linux
[off-list ref] wrote:
quoted
Not as far as I can see. fixed-clock appears to have two properies:
clock-frequency
clock-output-names
and nothing else. See of_fixed_clk_setup in drivers/clk/clk-fixed-rate.c.
You'll also find that the documentation in this file says this about it:
Looks like Documentation/devicetree/bindings/clock/fixed-clock.txt is
misleading then:
"Optional properties:
- gpios : From common gpio binding; gpio connection to clock enable pin."
It seems so. It appears that the binding was added to clk-fixed-rate.c
here:
commit 015ba40246497ae02a5f644d4c8adfec76d9b75c
Date: Sat Apr 7 21:39:39 2012 -0500
and the binding documentation separately:
commit 766e6a4ec602d0c107553b91b3434fe9c03474f4
Date: Mon Apr 9 14:50:06 2012 -0500
From what I can see, this "gpios" property has never been supported.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
From: Mark Brown <broonie@kernel.org> Date: 2014-01-22 11:30:49
On Tue, Jan 21, 2014 at 10:14:49AM -0800, Olof Johansson wrote:
On Tue, Jan 21, 2014 at 12:55 AM, Ulf Hansson [off-list ref] wrote:
quoted
That could make sense, but still I wonder how those shall be handled
in a fine grained power management setup. In other words, when shall
those be gated/ungated? Is the mmc core able to take the correct
decision about these?
The reference clock is in most cases I've seen 32kHz, and not
something that's under fine-grained power management. So it's not used
to regulate interface speed, etc.
I have seen devices connected using SDIO which did use fine grained
power management here with faster clocks. They're definitely not the
common case but they're there. :/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140122/de54e4eb/attachment.sig>
On Mon, Jan 20, 2014 at 2:48 PM, Russell King - ARM Linux
[off-list ref] wrote:
So far so good. Now, what about this external oscillator which has its
own separate power control. My immediate thought is that this can be
specified via card_ext_clock - I would simply need to declare a fixed-rate
clock with either a regulator (power switch) controlled via a gpio (which
would probably be closer to the hardware) or a gpio as an enable... ah,
that requires me to write a common clock driver for that bit since this
is currently not modelled by CCF...
From: Tomasz Figa <hidden> Date: 2014-01-26 17:26:24
On 21.01.2014 19:34, Tomasz Figa wrote:
Hi,
On 20.01.2014 04:56, Olof Johansson wrote:
quoted
This patch enables support for power-on sequencing of SDIO peripherals
through DT.
In general, it's quite common that wifi modules and other similar
peripherals have several signals in addition to the SDIO interface that
needs wiggling before the module will power on. It's common to have a
reference clock, one or several power rails and one or several lines
for reset/enable type functions.
The binding as written today introduces a number of reset gpios,
a regulator and a clock specifier. The code will handle up to 2 gpio
reset lines, but it's trivial to increase to more than that if needed
at some point.
Implementation-wise, the MMC core has been changed to handle this during
host power up, before the host interface is powered on. I have not yet
implemented the power-down side, I wanted people to have a chance for
reporting back w.r.t. issues (or comments on the bindings) first.
I have not tested the regulator portion, since the system and module
I'm working on doesn't need one (Samsung Chromebook with Marvell
8797-based wifi). Testing of those portions (and reporting back) would
be appreciated.
While I fully agree that this is an important problem that needs to be
solved, I really don't think this is the right way, because:
a) power-up sequence is really specific to the MMC device and often it's
not simply a matter of switching on one regulator or one clock, e.g.
specific time constraints need to be met.
b) you can have WLAN chips in which SDIO is just one of the options to
use as host interface, which may be also HSIC, I2C or UART. Really. See
[1].
c) this is leaking device specific details to generic host code, which
isn't really elegant.
Now, to make this a bit more constructive, [2] is a solution that I came
up with (not perfect either), which simply adds a separate platform
device for the low level part of the chip. I believe this is a better
solution because:
a) you can often see such WLAN/BT combo chip as a set of separate
devices, e.g. SDIO WLAN, UART BT and a simple PMIC or management IC,
which provides power/reset control, out of band signalling and etc. for
the first two, so it isn't that bad to have a separate device node for
the last one,
b) you have full freedom of defining your DT binding with whatever data
you need, any number of clocks, regulators, GPIOs and even out of band
interrupts (IMHO the most important one).
c) you can implement power-on, power-off sequences as needed for your
particular device,
d) you have full separation of device-specific data from MMC core (or
any other subsystem simply used as a way to perform I/O to the chip).
Now what's missing there is a way to signal the MMC core or any other
transport that a device showed up and the controller should be woken up
out of standby and scan of the bus initialized. This could be done by
explicitly specifying the device as a subnode of the
MMC/UART/USB(HSIC)/I2C or whatever with a link (phandle) to the power
controller of the chip or the other way around - a link to the
MMC/UART/... controller from the power controller node.
I've looked a bit around MMC core code and got some basic idea how
things look. I will definitely need some guidance, or at least some
opinions, from MMC guys, as some MMC core changes are unavoidable.
Now, the device-specific code is not really an issue, existing drivers
usually already have their ways of powering the chips on and off, based
on platform data. Everything needed here is to retrieve needed resources
(GPIOs, clocks, regulators) using DT, which should be trivial.
The worse part is the interaction between MMC and power controller
driver (the platform driver part of WLAN driver, if you look at brcmfmac
as an example). I believe that we need following things:
a) A way to tell the MMC controller that there is no card detection
mechanism available on given slot and it also should not be polling the
slot to check card presence. Something like a "manual card detect" that
would be triggered by another kernel entity that controls whether the
MMC device is present (i.e. WLAN driver). We already have "broken-cd"
property, but it only implies the former, wasting time on needless polling.
b) A mechanism to bind the power controller to used MMC slot. Something
like "mmc-bus = <&mmc2>;" property in device node of the power
controller and a function like of_find_mmc_controller_by_node(), which
would be an MMC counterpart of I2C's of_find_i2c_adapter_by_node(). To
avoid races, it should probably take a reference on MMC host that would
have to be dropped explicitly whenever it is not needed anymore.
c) A method to notify the MMC subsystem that card presence has changed.
We already have something like this in drivers/mmc/core/slot-gpio.c, but
used for a simple GPIO-based card detection. If the main part of
mmc_gpio_cd_irqt() could be turned into an exported helper, e.g.
mmc_force_card_detect(host) then basically we would have everything needed.
Unfortunately, I don't have more time left for today to create patches
and test them, so for now, I'd like to hear opinion of MMC maintainers
about this approach. Do you find this acceptable?
By the way, it seems like slot-gpio.c could replace a lot of custom GPIO
card detection code used in MMC host drivers, e.g. sdhci-s3c. Is there
any reason why it couldn't?
Best regards,
Tomasz
On Mon, Jan 20, 2014 at 2:48 PM, Russell King - ARM Linux
[off-list ref] wrote:
quoted
So far so good. Now, what about this external oscillator which has its
own separate power control. My immediate thought is that this can be
specified via card_ext_clock - I would simply need to declare a fixed-rate
clock with either a regulator (power switch) controlled via a gpio (which
would probably be closer to the hardware) or a gpio as an enable... ah,
that requires me to write a common clock driver for that bit since this
is currently not modelled by CCF...
I have not received too much feedback to my patch yet. CCF is a bit new
territory to me, but I think having a separate stackable clk-gpio would
be more flexible than having the gpio property implemented in
clk-fixed-rate.
Anyway, I am happy do it either way as long as I can get a gpio
-controlled clock implementation into the main line.
Cheers,
Jyri
Hi,
On Mon, Jan 27, 2014 at 4:43 PM, Jyri Sarha [off-list ref] wrote:
On 01/24/2014 07:35 PM, Fabio Estevam wrote:
quoted
On Mon, Jan 20, 2014 at 2:48 PM, Russell King - ARM Linux
[off-list ref] wrote:
quoted
So far so good. Now, what about this external oscillator which has its
own separate power control. My immediate thought is that this can be
specified via card_ext_clock - I would simply need to declare a
fixed-rate
clock with either a regulator (power switch) controlled via a gpio (which
would probably be closer to the hardware) or a gpio as an enable... ah,
that requires me to write a common clock driver for that bit since this
is currently not modelled by CCF...
I have not received too much feedback to my patch yet. CCF is a bit new
territory to me, but I think having a separate stackable clk-gpio would be
more flexible than having the gpio property implemented in clk-fixed-rate.
Not sure where stacking external clocks would be used, but sounds like a
fixed factor clock? Maybe you should add DT support to that if that's what
you need or have.
Anyway, I am happy do it either way as long as I can get a gpio -controlled
clock implementation into the main line.
Hi,
On Mon, Jan 27, 2014 at 4:43 PM, Jyri Sarha [off-list ref] wrote:
quoted
On 01/24/2014 07:35 PM, Fabio Estevam wrote:
...
quoted
I have not received too much feedback to my patch yet. CCF is a bit new
territory to me, but I think having a separate stackable clk-gpio would be
more flexible than having the gpio property implemented in clk-fixed-rate.
Not sure where stacking external clocks would be used, but sounds like a
fixed factor clock? Maybe you should add DT support to that if that's what
you need or have.
Ah, but then there is also the problem of having to change
clk_register_fixed_rate() in the CCF api, which is called from all over
the place.
Of course I could add a new clk_register_fixed_rate_gpio(), but still it
does not look to me like the way to go.
Cheers,
Jyri
On 26 January 2014 18:26, Tomasz Figa [off-list ref] wrote:
On 21.01.2014 19:34, Tomasz Figa wrote:
quoted
Hi,
On 20.01.2014 04:56, Olof Johansson wrote:
quoted
This patch enables support for power-on sequencing of SDIO peripherals
through DT.
In general, it's quite common that wifi modules and other similar
peripherals have several signals in addition to the SDIO interface that
needs wiggling before the module will power on. It's common to have a
reference clock, one or several power rails and one or several lines
for reset/enable type functions.
The binding as written today introduces a number of reset gpios,
a regulator and a clock specifier. The code will handle up to 2 gpio
reset lines, but it's trivial to increase to more than that if needed
at some point.
Implementation-wise, the MMC core has been changed to handle this during
host power up, before the host interface is powered on. I have not yet
implemented the power-down side, I wanted people to have a chance for
reporting back w.r.t. issues (or comments on the bindings) first.
I have not tested the regulator portion, since the system and module
I'm working on doesn't need one (Samsung Chromebook with Marvell
8797-based wifi). Testing of those portions (and reporting back) would
be appreciated.
While I fully agree that this is an important problem that needs to be
solved, I really don't think this is the right way, because:
a) power-up sequence is really specific to the MMC device and often it's
not simply a matter of switching on one regulator or one clock, e.g.
specific time constraints need to be met.
b) you can have WLAN chips in which SDIO is just one of the options to
use as host interface, which may be also HSIC, I2C or UART. Really. See
[1].
c) this is leaking device specific details to generic host code, which
isn't really elegant.
Now, to make this a bit more constructive, [2] is a solution that I came
up with (not perfect either), which simply adds a separate platform
device for the low level part of the chip. I believe this is a better
solution because:
a) you can often see such WLAN/BT combo chip as a set of separate
devices, e.g. SDIO WLAN, UART BT and a simple PMIC or management IC,
which provides power/reset control, out of band signalling and etc. for
the first two, so it isn't that bad to have a separate device node for
the last one,
b) you have full freedom of defining your DT binding with whatever data
you need, any number of clocks, regulators, GPIOs and even out of band
interrupts (IMHO the most important one).
c) you can implement power-on, power-off sequences as needed for your
particular device,
d) you have full separation of device-specific data from MMC core (or
any other subsystem simply used as a way to perform I/O to the chip).
Now what's missing there is a way to signal the MMC core or any other
transport that a device showed up and the controller should be woken up
out of standby and scan of the bus initialized. This could be done by
explicitly specifying the device as a subnode of the
MMC/UART/USB(HSIC)/I2C or whatever with a link (phandle) to the power
controller of the chip or the other way around - a link to the
MMC/UART/... controller from the power controller node.
I've looked a bit around MMC core code and got some basic idea how things
look. I will definitely need some guidance, or at least some opinions, from
MMC guys, as some MMC core changes are unavoidable.
Now, the device-specific code is not really an issue, existing drivers
usually already have their ways of powering the chips on and off, based on
platform data. Everything needed here is to retrieve needed resources
(GPIOs, clocks, regulators) using DT, which should be trivial.
The worse part is the interaction between MMC and power controller driver
(the platform driver part of WLAN driver, if you look at brcmfmac as an
example). I believe that we need following things:
a) A way to tell the MMC controller that there is no card detection
mechanism available on given slot and it also should not be polling the slot
to check card presence. Something like a "manual card detect" that would be
triggered by another kernel entity that controls whether the MMC device is
present (i.e. WLAN driver). We already have "broken-cd" property, but it
only implies the former, wasting time on needless polling.
There is already a host capability that I think we could use to handle
this. MMC_CAP_NONREMOVABLE, the corresponding DT binding string is
"non-removable", and it may be set per host device.
Using this cap means the mmc_rescan process that runs to detect new
cards, will only be executed once and during boot. So, we need to make
sure all resources and powers are provided to the card at this point.
Otherwise the card will not be detected.
In the SDIO case, to save power, the SDIO func driver may use runtime
PM to tell the mmc core power about whether the card needs to be
powered. Typically from the WLAN driver's probe() and "interface
up/down" the runtime PM reference for the SDIO func device, should be
adjusted with pm_runtime_get|put.
b) A mechanism to bind the power controller to used MMC slot. Something like
"mmc-bus = <&mmc2>;" property in device node of the power controller and a
function like of_find_mmc_controller_by_node(), which would be an MMC
counterpart of I2C's of_find_i2c_adapter_by_node(). To avoid races, it
should probably take a reference on MMC host that would have to be dropped
explicitly whenever it is not needed anymore.
I suppose an "MMC slot" can be translated to "MMC host"?
What I am trying to understand is how the mmc core (or if we push it
to be handled from the mmc host's .set_ios callback) shall be able to
tell the power controller driver to enable/disable it's resources.
Somehow we need the struct device available to handle that. Then I
guess operating on it using runtime PM would be a solution that would
be quite nice!?
c) A method to notify the MMC subsystem that card presence has changed. We
already have something like this in drivers/mmc/core/slot-gpio.c, but used
for a simple GPIO-based card detection. If the main part of
mmc_gpio_cd_irqt() could be turned into an exported helper, e.g.
mmc_force_card_detect(host) then basically we would have everything needed.
I am not sure I understand why this is needed. I think it would be
more convenient to use MMC_CAP_NONREMOVABLE instead as stated earlier.
But please elaborate, I might have missed something.
Unfortunately, I don't have more time left for today to create patches and
test them, so for now, I'd like to hear opinion of MMC maintainers about
this approach. Do you find this acceptable?
By the way, it seems like slot-gpio.c could replace a lot of custom GPIO
card detection code used in MMC host drivers, e.g. sdhci-s3c. Is there any
reason why it couldn't?
I suppose most host driver's should convert to the slot-gpio API, it's
is just a matter of someone to send the patches. :-)
Kind regards
Ulf Hansson
From: Tomasz Figa <hidden> Date: 2014-01-28 00:59:28
On 27.01.2014 11:19, Ulf Hansson wrote:
On 26 January 2014 18:26, Tomasz Figa [off-list ref] wrote:
quoted
On 21.01.2014 19:34, Tomasz Figa wrote:
quoted
Hi,
On 20.01.2014 04:56, Olof Johansson wrote:
quoted
This patch enables support for power-on sequencing of SDIO peripherals
through DT.
In general, it's quite common that wifi modules and other similar
peripherals have several signals in addition to the SDIO interface that
needs wiggling before the module will power on. It's common to have a
reference clock, one or several power rails and one or several lines
for reset/enable type functions.
The binding as written today introduces a number of reset gpios,
a regulator and a clock specifier. The code will handle up to 2 gpio
reset lines, but it's trivial to increase to more than that if needed
at some point.
Implementation-wise, the MMC core has been changed to handle this during
host power up, before the host interface is powered on. I have not yet
implemented the power-down side, I wanted people to have a chance for
reporting back w.r.t. issues (or comments on the bindings) first.
I have not tested the regulator portion, since the system and module
I'm working on doesn't need one (Samsung Chromebook with Marvell
8797-based wifi). Testing of those portions (and reporting back) would
be appreciated.
While I fully agree that this is an important problem that needs to be
solved, I really don't think this is the right way, because:
a) power-up sequence is really specific to the MMC device and often it's
not simply a matter of switching on one regulator or one clock, e.g.
specific time constraints need to be met.
b) you can have WLAN chips in which SDIO is just one of the options to
use as host interface, which may be also HSIC, I2C or UART. Really. See
[1].
c) this is leaking device specific details to generic host code, which
isn't really elegant.
Now, to make this a bit more constructive, [2] is a solution that I came
up with (not perfect either), which simply adds a separate platform
device for the low level part of the chip. I believe this is a better
solution because:
a) you can often see such WLAN/BT combo chip as a set of separate
devices, e.g. SDIO WLAN, UART BT and a simple PMIC or management IC,
which provides power/reset control, out of band signalling and etc. for
the first two, so it isn't that bad to have a separate device node for
the last one,
b) you have full freedom of defining your DT binding with whatever data
you need, any number of clocks, regulators, GPIOs and even out of band
interrupts (IMHO the most important one).
c) you can implement power-on, power-off sequences as needed for your
particular device,
d) you have full separation of device-specific data from MMC core (or
any other subsystem simply used as a way to perform I/O to the chip).
Now what's missing there is a way to signal the MMC core or any other
transport that a device showed up and the controller should be woken up
out of standby and scan of the bus initialized. This could be done by
explicitly specifying the device as a subnode of the
MMC/UART/USB(HSIC)/I2C or whatever with a link (phandle) to the power
controller of the chip or the other way around - a link to the
MMC/UART/... controller from the power controller node.
I've looked a bit around MMC core code and got some basic idea how things
look. I will definitely need some guidance, or at least some opinions, from
MMC guys, as some MMC core changes are unavoidable.
Now, the device-specific code is not really an issue, existing drivers
usually already have their ways of powering the chips on and off, based on
platform data. Everything needed here is to retrieve needed resources
(GPIOs, clocks, regulators) using DT, which should be trivial.
The worse part is the interaction between MMC and power controller driver
(the platform driver part of WLAN driver, if you look at brcmfmac as an
example). I believe that we need following things:
a) A way to tell the MMC controller that there is no card detection
mechanism available on given slot and it also should not be polling the slot
to check card presence. Something like a "manual card detect" that would be
triggered by another kernel entity that controls whether the MMC device is
present (i.e. WLAN driver). We already have "broken-cd" property, but it
only implies the former, wasting time on needless polling.
There is already a host capability that I think we could use to handle
this. MMC_CAP_NONREMOVABLE, the corresponding DT binding string is
"non-removable", and it may be set per host device.
Using this cap means the mmc_rescan process that runs to detect new
cards, will only be executed once and during boot. So, we need to make
sure all resources and powers are provided to the card at this point.
Otherwise the card will not be detected.
I don't quite like this requirement, especially if you consider
multi-platform kernels where a lot of drivers is going to be provided as
modules. WLAN drivers are especially good candidates. This means that
even if the card is powered off at boot-up, if user (or init system)
loads appropriate module, which powers the chip on, MMC core must be
able to notice this.
In the SDIO case, to save power, the SDIO func driver may use runtime
PM to tell the mmc core power about whether the card needs to be
powered. Typically from the WLAN driver's probe() and "interface
up/down" the runtime PM reference for the SDIO func device, should be
adjusted with pm_runtime_get|put.
I need to think a bit more about the power management control flow here.
In case of such chips I'd tend to look at MMC merely as a host
interface, which as I said, might be only one of available options. I'm
not sure if it should be the host interface core that decides whether
the whole device should be powered off. However there might be a
solution that leverages SDIO func runtime PM, which wouldn't imply such
control flow. Let me reconsider this.
quoted
b) A mechanism to bind the power controller to used MMC slot. Something like
"mmc-bus = <&mmc2>;" property in device node of the power controller and a
function like of_find_mmc_controller_by_node(), which would be an MMC
counterpart of I2C's of_find_i2c_adapter_by_node(). To avoid races, it
should probably take a reference on MMC host that would have to be dropped
explicitly whenever it is not needed anymore.
I suppose an "MMC slot" can be translated to "MMC host"?
Right.
What I am trying to understand is how the mmc core (or if we push it
to be handled from the mmc host's .set_ios callback) shall be able to
tell the power controller driver to enable/disable it's resources.
Somehow we need the struct device available to handle that. Then I
guess operating on it using runtime PM would be a solution that would
be quite nice!?
As I wrote above, I'm not quite sure about this yet.
quoted
c) A method to notify the MMC subsystem that card presence has changed. We
already have something like this in drivers/mmc/core/slot-gpio.c, but used
for a simple GPIO-based card detection. If the main part of
mmc_gpio_cd_irqt() could be turned into an exported helper, e.g.
mmc_force_card_detect(host) then basically we would have everything needed.
I am not sure I understand why this is needed. I think it would be
more convenient to use MMC_CAP_NONREMOVABLE instead as stated earlier.
But please elaborate, I might have missed something.
See above. I'm not quite convinced that state of MMC interface should
determine power state of the chip. I can easily imagine a situation
where the MMC link is powered down (link power management) but the WLAN
chip keeps operation. Keep in mind that those are usually complete SoCs
that can keep processing network traffic autonomously and wake-up the
application processor whenever anything interesting happens using extra
out of bounds signalling, which might trigger re-enabling the MMC link.
quoted
Unfortunately, I don't have more time left for today to create patches and
test them, so for now, I'd like to hear opinion of MMC maintainers about
this approach. Do you find this acceptable?
By the way, it seems like slot-gpio.c could replace a lot of custom GPIO
card detection code used in MMC host drivers, e.g. sdhci-s3c. Is there any
reason why it couldn't?
I suppose most host driver's should convert to the slot-gpio API, it's
is just a matter of someone to send the patches. :-)
OK, great. I'll add conversion of sdhci-s3c to my queue then.
Best regards,
Tomasz
On 28 January 2014 01:59, Tomasz Figa [off-list ref] wrote:
On 27.01.2014 11:19, Ulf Hansson wrote:
quoted
On 26 January 2014 18:26, Tomasz Figa [off-list ref] wrote:
quoted
On 21.01.2014 19:34, Tomasz Figa wrote:
quoted
Hi,
On 20.01.2014 04:56, Olof Johansson wrote:
quoted
This patch enables support for power-on sequencing of SDIO peripherals
through DT.
In general, it's quite common that wifi modules and other similar
peripherals have several signals in addition to the SDIO interface that
needs wiggling before the module will power on. It's common to have a
reference clock, one or several power rails and one or several lines
for reset/enable type functions.
The binding as written today introduces a number of reset gpios,
a regulator and a clock specifier. The code will handle up to 2 gpio
reset lines, but it's trivial to increase to more than that if needed
at some point.
Implementation-wise, the MMC core has been changed to handle this
during
host power up, before the host interface is powered on. I have not yet
implemented the power-down side, I wanted people to have a chance for
reporting back w.r.t. issues (or comments on the bindings) first.
I have not tested the regulator portion, since the system and module
I'm working on doesn't need one (Samsung Chromebook with Marvell
8797-based wifi). Testing of those portions (and reporting back) would
be appreciated.
While I fully agree that this is an important problem that needs to be
solved, I really don't think this is the right way, because:
a) power-up sequence is really specific to the MMC device and often it's
not simply a matter of switching on one regulator or one clock, e.g.
specific time constraints need to be met.
b) you can have WLAN chips in which SDIO is just one of the options to
use as host interface, which may be also HSIC, I2C or UART. Really. See
[1].
c) this is leaking device specific details to generic host code, which
isn't really elegant.
Now, to make this a bit more constructive, [2] is a solution that I came
up with (not perfect either), which simply adds a separate platform
device for the low level part of the chip. I believe this is a better
solution because:
a) you can often see such WLAN/BT combo chip as a set of separate
devices, e.g. SDIO WLAN, UART BT and a simple PMIC or management IC,
which provides power/reset control, out of band signalling and etc. for
the first two, so it isn't that bad to have a separate device node for
the last one,
b) you have full freedom of defining your DT binding with whatever data
you need, any number of clocks, regulators, GPIOs and even out of band
interrupts (IMHO the most important one).
c) you can implement power-on, power-off sequences as needed for your
particular device,
d) you have full separation of device-specific data from MMC core (or
any other subsystem simply used as a way to perform I/O to the chip).
Now what's missing there is a way to signal the MMC core or any other
transport that a device showed up and the controller should be woken up
out of standby and scan of the bus initialized. This could be done by
explicitly specifying the device as a subnode of the
MMC/UART/USB(HSIC)/I2C or whatever with a link (phandle) to the power
controller of the chip or the other way around - a link to the
MMC/UART/... controller from the power controller node.
I've looked a bit around MMC core code and got some basic idea how things
look. I will definitely need some guidance, or at least some opinions,
from
MMC guys, as some MMC core changes are unavoidable.
Now, the device-specific code is not really an issue, existing drivers
usually already have their ways of powering the chips on and off, based
on
platform data. Everything needed here is to retrieve needed resources
(GPIOs, clocks, regulators) using DT, which should be trivial.
The worse part is the interaction between MMC and power controller driver
(the platform driver part of WLAN driver, if you look at brcmfmac as an
example). I believe that we need following things:
a) A way to tell the MMC controller that there is no card detection
mechanism available on given slot and it also should not be polling the
slot
to check card presence. Something like a "manual card detect" that would
be
triggered by another kernel entity that controls whether the MMC device
is
present (i.e. WLAN driver). We already have "broken-cd" property, but it
only implies the former, wasting time on needless polling.
There is already a host capability that I think we could use to handle
this. MMC_CAP_NONREMOVABLE, the corresponding DT binding string is
"non-removable", and it may be set per host device.
Using this cap means the mmc_rescan process that runs to detect new
cards, will only be executed once and during boot. So, we need to make
sure all resources and powers are provided to the card at this point.
Otherwise the card will not be detected.
I don't quite like this requirement, especially if you consider
multi-platform kernels where a lot of drivers is going to be provided as
modules. WLAN drivers are especially good candidates. This means that even
if the card is powered off at boot-up, if user (or init system) loads
appropriate module, which powers the chip on, MMC core must be able to
notice this.
To be able to detect the card, the WLAN driver doesn't have to be
probed, only the "power controller" driver. I suppose this is were it
becomes a bit tricky.
Somehow the mmc core needs to be involved in the probe process of the
power controller driver. Could perhaps the power controller bus be
located in the mmc core and thus the power controller driver needs to
register itself by using a new API from the mmc core? Similar how SDIO
func driver's register themselves.
I have one concern here though. Unless the SDIO func driver gets
probed, the SDIO card will be kept powered, which is not optimal from
a power management perspective.
To solve this, we need to change the policy about how to handle SDIO
cards after the initialization sequence (mmc_rescan) has been
completed. This will affect SDIO func driver's as well, since at the
moment those expects the card to be fully powered once they are being
probed.
quoted
In the SDIO case, to save power, the SDIO func driver may use runtime
PM to tell the mmc core power about whether the card needs to be
powered. Typically from the WLAN driver's probe() and "interface
up/down" the runtime PM reference for the SDIO func device, should be
adjusted with pm_runtime_get|put.
I need to think a bit more about the power management control flow here. In
case of such chips I'd tend to look at MMC merely as a host interface, which
as I said, might be only one of available options. I'm not sure if it should
be the host interface core that decides whether the whole device should be
powered off. However there might be a solution that leverages SDIO func
runtime PM, which wouldn't imply such control flow. Let me reconsider this.
Just to clarify things; it is not the "host interface" that decides
whether the whole device should be powered off. This is decided from
the SDIO func driver, by using runtime PM.
The "host interface" still needs to be in control of the power on/off
sequence, since the knowledge about the SDIO spec is required to
handle this.
quoted
quoted
b) A mechanism to bind the power controller to used MMC slot. Something
like
"mmc-bus = <&mmc2>;" property in device node of the power controller and
a
function like of_find_mmc_controller_by_node(), which would be an MMC
counterpart of I2C's of_find_i2c_adapter_by_node(). To avoid races, it
should probably take a reference on MMC host that would have to be
dropped
explicitly whenever it is not needed anymore.
I suppose an "MMC slot" can be translated to "MMC host"?
Right.
quoted
What I am trying to understand is how the mmc core (or if we push it
to be handled from the mmc host's .set_ios callback) shall be able to
tell the power controller driver to enable/disable it's resources.
Somehow we need the struct device available to handle that. Then I
guess operating on it using runtime PM would be a solution that would
be quite nice!?
As I wrote above, I'm not quite sure about this yet.
quoted
quoted
c) A method to notify the MMC subsystem that card presence has changed.
We
already have something like this in drivers/mmc/core/slot-gpio.c, but
used
for a simple GPIO-based card detection. If the main part of
mmc_gpio_cd_irqt() could be turned into an exported helper, e.g.
mmc_force_card_detect(host) then basically we would have everything
needed.
I am not sure I understand why this is needed. I think it would be
more convenient to use MMC_CAP_NONREMOVABLE instead as stated earlier.
But please elaborate, I might have missed something.
See above. I'm not quite convinced that state of MMC interface should
determine power state of the chip. I can easily imagine a situation where
the MMC link is powered down (link power management) but the WLAN chip keeps
operation. Keep in mind that those are usually complete SoCs that can keep
processing network traffic autonomously and wake-up the application
processor whenever anything interesting happens using extra out of bounds
signalling, which might trigger re-enabling the MMC link.
Am not sure I understand what you mean with MMC link here.
We have the VCC regulator that the mmc host driver handles and the
resources by "power controller" driver. Do you want these to be
remained enabled during system suspend or are you saying we might need
even more fine grained power management?
Additionally, as Chris also pointed out in his reply; SDIO func
drivers can prevent the mmc core from powering off the card during
system suspend. Check for the flag, MMC_PM_KEEP_POWER in the code.
Kind regards
Uffe
quoted
quoted
Unfortunately, I don't have more time left for today to create patches
and
test them, so for now, I'd like to hear opinion of MMC maintainers about
this approach. Do you find this acceptable?
By the way, it seems like slot-gpio.c could replace a lot of custom GPIO
card detection code used in MMC host drivers, e.g. sdhci-s3c. Is there
any
reason why it couldn't?
I suppose most host driver's should convert to the slot-gpio API, it's
is just a matter of someone to send the patches. :-)
OK, great. I'll add conversion of sdhci-s3c to my queue then.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 28 January 2014 01:59, Tomasz Figa [off-list ref] wrote:
quoted
On 27.01.2014 11:19, Ulf Hansson wrote:
quoted
There is already a host capability that I think we could use to handle
this. MMC_CAP_NONREMOVABLE, the corresponding DT binding string is
"non-removable", and it may be set per host device.
Using this cap means the mmc_rescan process that runs to detect new
cards, will only be executed once and during boot. So, we need to make
sure all resources and powers are provided to the card at this point.
Otherwise the card will not be detected.
I don't quite like this requirement, especially if you consider
multi-platform kernels where a lot of drivers is going to be provided as
modules. WLAN drivers are especially good candidates. This means that even
if the card is powered off at boot-up, if user (or init system) loads
appropriate module, which powers the chip on, MMC core must be able to
notice this.
To be able to detect the card, the WLAN driver doesn't have to be
probed, only the "power controller" driver. I suppose this is were it
becomes a bit tricky.
Somehow the mmc core needs to be involved in the probe process of the
power controller driver. Could perhaps the power controller bus be
located in the mmc core and thus the power controller driver needs to
register itself by using a new API from the mmc core? Similar how SDIO
func driver's register themselves.
I think there is another option, which does have its own pros and cons:
We could move all the power handling back into the sdio function driver
if we allow a secondary detection path using DT rather than the probing
of the SDIO bus. Essentially you'd have to list the class/vendor/device
ID for each function that cannot be autodetected in DT, and have the
SDIO core pretend that it found the device just by looking at the
device nodes, and register the struct sdio_func so it can be bound to
the driver. The driver then does all the power handling in a device
specific way. At some point the hardware gets registered at the
mmc host, and the sdio core connects the bus state to the already present
sdio_func, possibly notifying the function driver that it has become
usable.
Obviously, this can only work for CAP_NONREMOVABLE devices, but those
are exactly the ones we are worried about here. The advantage is that
the power sequencing for a particular device can then be in device
specific code and can have an arbitrarily complex in the driver without
needing the mmc code to handle all possible corner cases.
Arnd
From: Russell King - ARM Linux <hidden> Date: 2014-01-30 21:49:17
On Sun, Jan 19, 2014 at 07:56:52PM -0800, Olof Johansson wrote:
This is a small series enhancing the MMC core code to power on modules
before the host in cases where needed, and the corresponding DT bindings
changes.
I've got some other issues to debug on the Chromebook, i.e. the interface
doens't actually work. So far it seems unrelated to this patch set so
it's worth posting this and get things going since others need the same
functionality (i.e Cubox-i).
As mentioned in the patch in the series, I haven't implemented power-down
yet, I wanted to make sure that the power-on side will be adequate for
those who are looking to use it right away.
Comments/test reports/etc welcome.
So, I thought I'd give this a go on the Cubox-i4, and... it doesn't work
there. It's not your patches, it's down to sdhci-esdhc-imx.c not using
mmc_of_parse() at all, so those new properties have no way to be used
there.
It doesn't look like it could in its current form use mmc_of_parse(),
as the imx code manually parses some of the generic properties to hand
them into the sdhci layer. This looks icky, and it looks like something
that should be fixed - why should drivers be parsing the core attributes
themselves?
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
From: Russell King - ARM Linux <hidden> Date: 2014-02-01 16:14:20
On Thu, Jan 30, 2014 at 09:49:17PM +0000, Russell King - ARM Linux wrote:
On Sun, Jan 19, 2014 at 07:56:52PM -0800, Olof Johansson wrote:
quoted
This is a small series enhancing the MMC core code to power on modules
before the host in cases where needed, and the corresponding DT bindings
changes.
I've got some other issues to debug on the Chromebook, i.e. the interface
doens't actually work. So far it seems unrelated to this patch set so
it's worth posting this and get things going since others need the same
functionality (i.e Cubox-i).
As mentioned in the patch in the series, I haven't implemented power-down
yet, I wanted to make sure that the power-on side will be adequate for
those who are looking to use it right away.
Comments/test reports/etc welcome.
So, I thought I'd give this a go on the Cubox-i4, and... it doesn't work
there. It's not your patches, it's down to sdhci-esdhc-imx.c not using
mmc_of_parse() at all, so those new properties have no way to be used
there.
It doesn't look like it could in its current form use mmc_of_parse(),
as the imx code manually parses some of the generic properties to hand
them into the sdhci layer. This looks icky, and it looks like something
that should be fixed - why should drivers be parsing the core attributes
themselves?
Here's an illustration of why it's icky.
If we call mmc_of_parse() in the sdhci-esdhc-imx driver (which we'd need to
do in order to get information on how to configure the card detection etc)
then this fills in mmc->f_max.
However, the subsequent call to sdhci_add_host() computes the maximum clock
from the sdhci capabilities, and then does this:
host->max_clk *= 1000000;
if (host->max_clk == 0 || host->quirks &
SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN) {
if (!host->ops->get_max_clock) {
pr_err("%s: Hardware doesn't specify base clock "
"frequency.\n", mmc_hostname(mmc));
return -ENODEV;
}
host->max_clk = host->ops->get_max_clock(host);
}
...
/*
* Set host parameters.
*/
mmc->ops = &sdhci_ops;
mmc->f_max = host->max_clk;
which would have the effect of overwriting a previously set f_max from
the OF data.
There's also the whole "cd-gpios" thing which would need sorting out -
the imx sdhci driver already parses this property itself, and sets its
own internal data (so it knows whether it has to use the controller
based card detect or the gpio card detect) and simply adding a call to
mmc_of_parse() would result in the gpio slot stuff being setup twice.
The obvious solution here is to rewrite the sdhci initialisation such
that it uses the generic infrastructure, but I don't have the motivation
to do that (I've already plenty of patches to deal with that I don't
need any more at the moment.)
A simpler solution would be to split mmc_of_parse() so that the new bits
are a separate function, which the generic MMC core always calls for
every host - taking the decision over whether this is supported completely
away from hosts. I think that makes a lot of sense, especially as this
has nothing to do with the facilities found on any particular host.
There's another issue here about resets. Let's take the case where the
external card is powered off, but has active high resets. At the moment,
the sequence is this:
power: _____/~~~~~~~~~~~~
reset: __/~~~~\__________
That's not particularly nice, as the reset signal will tend do drive power
into the device before it's powered up via the clamping diodes in the case.
Generally, devices are not designed to be powered in this way. However,
this is a relatively minor issue though compared to this one, which is what
happens if the card uses active low reset:
power: _____/~~~~~~~~~~~~
reset: ~~\_____/~~~~~~~~~
This is definitely not good, because it means that the reset is higher for
longer, which may result in unacceptable dissapation in the package from
those clamping diodes. What we need instead is for active low reset is:
power: _____/~~~~~~~~~~~~
reset: ________/~~~~~~~~~
So, we need the GPIO layer to tell us whether the output is active high or
active low and adjust the initial setting accordingly. Basically, whenever
the attached device is powered down, GPIOs to it should be held at low level
or high impedance (with a pull-down to reduce the risks of ESD damage.)
I've seen designs get this wrong in the past - Intel Assabet is a good one
where the UDA1341 audio codec ends up illuminating a LED by being powered
not via it's supply pin, but by a CPLD output driving one of the I2S pins
high. The result is that the CPLD output sources quite a bit of current
into the UDA1341, which then holds other pins on the SA1110 around mid-rail,
which is the /worst/ thing you can do with CMOS. Powering chips via their
inputs is basically a big no-no.
So, I think something like the below is needed on top of your patches.
Note that I added -EPROBE_DEFER handling too (which fixes a bug, because
regulator_get() returns pointer-errors):
drivers/mmc/core/host.c | 90 +++++++++++++++++++++++++++-----------
1 files changed, 65 insertions(+), 25 deletions(-)
@@ -316,7 +316,7 @@ int mmc_of_parse(struct mmc_host *host)u32bus_width;boolexplicit_inv_wp,gpio_inv_wp=false;enumof_gpio_flagsflags;-inti,len,ret,gpio;+intlen,ret,gpio;if(!host->parent||!host->parent->of_node)return0;
@@ -419,30 +419,6 @@ int mmc_of_parse(struct mmc_host *host)if(explicit_inv_wp^gpio_inv_wp)host->caps2|=MMC_CAP2_RO_ACTIVE_HIGH;-/* Parse card power/reset/clock control */-if(of_find_property(np,"card-reset-gpios",NULL)){-structgpio_desc*gpd;-for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){-gpd=devm_gpiod_get_index(host->parent,"card-reset",i);-if(IS_ERR(gpd))-break;-gpiod_direction_output(gpd,0);-host->card_reset_gpios[i]=gpd;-}--gpd=devm_gpiod_get_index(host->parent,"card-reset",ARRAY_SIZE(host->card_reset_gpios));-if(!IS_ERR(gpd)){-dev_warn(host->parent,"More reset gpios than we can handle");-gpiod_put(gpd);-}-}--host->card_clk=of_clk_get_by_name(np,"card_ext_clock");-if(IS_ERR(host->card_clk))-host->card_clk=NULL;--host->card_regulator=regulator_get(host->parent,"card-external-vcc");-if(of_find_property(np,"cap-sd-highspeed",&len))host->caps|=MMC_CAP_SD_HIGHSPEED;if(of_find_property(np,"cap-mmc-highspeed",&len))
@@ -467,6 +443,66 @@ int mmc_of_parse(struct mmc_host *host)EXPORT_SYMBOL(mmc_of_parse);+staticintmmc_of_parse_child(structmmc_host*host)+{+structdevice_node*np;+structclk*clk;+inti;++if(!host->parent||!host->parent->of_node)+return0;++np=host->parent->of_node;++host->card_regulator=regulator_get(host->parent,"card-external-vcc");+if(IS_ERR(host->card_regulator)){+if(PTR_ERR(host->card_regulator)==-EPROBE_DEFER)+returnPTR_ERR(host->card_regulator);+host->card_regulator=NULL;+}++/* Parse card power/reset/clock control */+if(of_find_property(np,"card-reset-gpios",NULL)){+structgpio_desc*gpd;+intlevel=0;++/*+*Iftheregulatorisenabled,thenwecanholdthe+*cardinresetwithanactivehighresets.Otherwise,+*holdtheresetslow.+*/+if(host->card_regulator&®ulator_is_enabled(host->card_regulator))+level=1;++for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){+gpd=devm_gpiod_get_index(host->parent,"card-reset",i);+if(IS_ERR(gpd)){+if(PTR_ERR(gpd)==-EPROBE_DEFER)+returnPTR_ERR(gpd);+break;+}+gpiod_direction_output(gpd,gpiod_is_active_low(gpd)|level);+host->card_reset_gpios[i]=gpd;+}++gpd=devm_gpiod_get_index(host->parent,"card-reset",ARRAY_SIZE(host->card_reset_gpios));+if(!IS_ERR(gpd)){+dev_warn(host->parent,"More reset gpios than we can handle");+gpiod_put(gpd);+}+}++clk=of_clk_get_by_name(np,"card_ext_clock");+if(IS_ERR(clk)){+if(PTR_ERR(clk)==-EPROBE_DEFER)+returnPTR_ERR(clk);+clk=NULL;+}+host->card_clk=clk;++return0;+}+/***mmc_alloc_host-initialisetheper-hoststructure.*@extra:sizeofprivatedatastructure
@@ -546,6 +582,10 @@ int mmc_add_host(struct mmc_host *host){interr;+err=mmc_of_parse_child(host);+if(err)+returnerr;+WARN_ON((host->caps&MMC_CAP_SDIO_IRQ)&&!host->ops->enable_sdio_irq);
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
From: Mark Brown <broonie@kernel.org> Date: 2014-02-12 18:33:01
On Tue, Jan 28, 2014 at 11:48:10AM +0100, Arnd Bergmann wrote:
I think there is another option, which does have its own pros and cons:
We could move all the power handling back into the sdio function driver
if we allow a secondary detection path using DT rather than the probing
of the SDIO bus. Essentially you'd have to list the class/vendor/device
ID for each function that cannot be autodetected in DT, and have the
SDIO core pretend that it found the device just by looking at the
device nodes, and register the struct sdio_func so it can be bound to
the driver. The driver then does all the power handling in a device
specific way. At some point the hardware gets registered at the
mmc host, and the sdio core connects the bus state to the already present
sdio_func, possibly notifying the function driver that it has become
usable.
Obviously, this can only work for CAP_NONREMOVABLE devices, but those
are exactly the ones we are worried about here. The advantage is that
the power sequencing for a particular device can then be in device
specific code and can have an arbitrarily complex in the driver without
needing the mmc code to handle all possible corner cases.
I think this is going to be the most generic solution overall, it's
going to work with other buses and it's going to allow the device to
fully power itself off while running (I don't know if it's useful for SD
but that's definitely common with some other buses).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140212/aba45cd9/attachment.sig>
On 28 January 2014 11:48, Arnd Bergmann [off-list ref] wrote:
On Tuesday 28 January 2014, Ulf Hansson wrote:
quoted
On 28 January 2014 01:59, Tomasz Figa [off-list ref] wrote:
quoted
On 27.01.2014 11:19, Ulf Hansson wrote:
quoted
There is already a host capability that I think we could use to handle
this. MMC_CAP_NONREMOVABLE, the corresponding DT binding string is
"non-removable", and it may be set per host device.
Using this cap means the mmc_rescan process that runs to detect new
cards, will only be executed once and during boot. So, we need to make
sure all resources and powers are provided to the card at this point.
Otherwise the card will not be detected.
I don't quite like this requirement, especially if you consider
multi-platform kernels where a lot of drivers is going to be provided as
modules. WLAN drivers are especially good candidates. This means that even
if the card is powered off at boot-up, if user (or init system) loads
appropriate module, which powers the chip on, MMC core must be able to
notice this.
To be able to detect the card, the WLAN driver doesn't have to be
probed, only the "power controller" driver. I suppose this is were it
becomes a bit tricky.
Somehow the mmc core needs to be involved in the probe process of the
power controller driver. Could perhaps the power controller bus be
located in the mmc core and thus the power controller driver needs to
register itself by using a new API from the mmc core? Similar how SDIO
func driver's register themselves.
I think there is another option, which does have its own pros and cons:
We could move all the power handling back into the sdio function driver
if we allow a secondary detection path using DT rather than the probing
of the SDIO bus. Essentially you'd have to list the class/vendor/device
ID for each function that cannot be autodetected in DT, and have the
SDIO core pretend that it found the device just by looking at the
device nodes, and register the struct sdio_func so it can be bound to
the driver. The driver then does all the power handling in a device
specific way. At some point the hardware gets registered at the
mmc host, and the sdio core connects the bus state to the already present
sdio_func, possibly notifying the function driver that it has become
usable.
It seems like an option we could try.
There are some tricky parts that needs to be taken care of by the
mmc/sdio core, regarding the probe/removal and the suspend/resume
sequence, but I suppose it should be possible.
A minor concern is how do we handle devices that are fully powered at
boot? Unless the sdio func driver will be loaded we can't power off
them, right? Do we need to cover this case, do you think?
Obviously, this can only work for CAP_NONREMOVABLE devices, but those
are exactly the ones we are worried about here. The advantage is that
the power sequencing for a particular device can then be in device
specific code and can have an arbitrarily complex in the driver without
needing the mmc code to handle all possible corner cases.
Agree!
I think a removable SDIO card won't l need this additional power
controller mechanism.
Kind regards
Uffe
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Tomasz Figa <hidden> Date: 2014-02-13 09:01:20
On 13.02.2014 09:56, Ulf Hansson wrote:
On 28 January 2014 11:48, Arnd Bergmann [off-list ref] wrote:
quoted
On Tuesday 28 January 2014, Ulf Hansson wrote:
quoted
On 28 January 2014 01:59, Tomasz Figa [off-list ref] wrote:
quoted
On 27.01.2014 11:19, Ulf Hansson wrote:
quoted
There is already a host capability that I think we could use to handle
this. MMC_CAP_NONREMOVABLE, the corresponding DT binding string is
"non-removable", and it may be set per host device.
Using this cap means the mmc_rescan process that runs to detect new
cards, will only be executed once and during boot. So, we need to make
sure all resources and powers are provided to the card at this point.
Otherwise the card will not be detected.
I don't quite like this requirement, especially if you consider
multi-platform kernels where a lot of drivers is going to be provided as
modules. WLAN drivers are especially good candidates. This means that even
if the card is powered off at boot-up, if user (or init system) loads
appropriate module, which powers the chip on, MMC core must be able to
notice this.
To be able to detect the card, the WLAN driver doesn't have to be
probed, only the "power controller" driver. I suppose this is were it
becomes a bit tricky.
Somehow the mmc core needs to be involved in the probe process of the
power controller driver. Could perhaps the power controller bus be
located in the mmc core and thus the power controller driver needs to
register itself by using a new API from the mmc core? Similar how SDIO
func driver's register themselves.
I think there is another option, which does have its own pros and cons:
We could move all the power handling back into the sdio function driver
if we allow a secondary detection path using DT rather than the probing
of the SDIO bus. Essentially you'd have to list the class/vendor/device
ID for each function that cannot be autodetected in DT, and have the
SDIO core pretend that it found the device just by looking at the
device nodes, and register the struct sdio_func so it can be bound to
the driver. The driver then does all the power handling in a device
specific way. At some point the hardware gets registered at the
mmc host, and the sdio core connects the bus state to the already present
sdio_func, possibly notifying the function driver that it has become
usable.
It seems like an option we could try.
There are some tricky parts that needs to be taken care of by the
mmc/sdio core, regarding the probe/removal and the suspend/resume
sequence, but I suppose it should be possible.
A minor concern is how do we handle devices that are fully powered at
boot? Unless the sdio func driver will be loaded we can't power off
them, right? Do we need to cover this case, do you think?
quoted
Obviously, this can only work for CAP_NONREMOVABLE devices, but those
are exactly the ones we are worried about here. The advantage is that
the power sequencing for a particular device can then be in device
specific code and can have an arbitrarily complex in the driver without
needing the mmc code to handle all possible corner cases.
Agree!
I think a removable SDIO card won't l need this additional power
controller mechanism.
Yes, sounds good to me too. It will be more tricky to implement than the
solution I initially proposed, but should end up being much cleaner and
possibly cover more cases.
Best regards,
Tomasz
From: Russell King - ARM Linux <hidden> Date: 2014-02-13 10:36:40
Any comments on this?
On Sat, Feb 01, 2014 at 04:14:20PM +0000, Russell King - ARM Linux wrote:
quoted hunk
On Thu, Jan 30, 2014 at 09:49:17PM +0000, Russell King - ARM Linux wrote:
quoted
On Sun, Jan 19, 2014 at 07:56:52PM -0800, Olof Johansson wrote:
quoted
This is a small series enhancing the MMC core code to power on modules
before the host in cases where needed, and the corresponding DT bindings
changes.
I've got some other issues to debug on the Chromebook, i.e. the interface
doens't actually work. So far it seems unrelated to this patch set so
it's worth posting this and get things going since others need the same
functionality (i.e Cubox-i).
As mentioned in the patch in the series, I haven't implemented power-down
yet, I wanted to make sure that the power-on side will be adequate for
those who are looking to use it right away.
Comments/test reports/etc welcome.
So, I thought I'd give this a go on the Cubox-i4, and... it doesn't work
there. It's not your patches, it's down to sdhci-esdhc-imx.c not using
mmc_of_parse() at all, so those new properties have no way to be used
there.
It doesn't look like it could in its current form use mmc_of_parse(),
as the imx code manually parses some of the generic properties to hand
them into the sdhci layer. This looks icky, and it looks like something
that should be fixed - why should drivers be parsing the core attributes
themselves?
Here's an illustration of why it's icky.
If we call mmc_of_parse() in the sdhci-esdhc-imx driver (which we'd need to
do in order to get information on how to configure the card detection etc)
then this fills in mmc->f_max.
However, the subsequent call to sdhci_add_host() computes the maximum clock
from the sdhci capabilities, and then does this:
host->max_clk *= 1000000;
if (host->max_clk == 0 || host->quirks &
SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN) {
if (!host->ops->get_max_clock) {
pr_err("%s: Hardware doesn't specify base clock "
"frequency.\n", mmc_hostname(mmc));
return -ENODEV;
}
host->max_clk = host->ops->get_max_clock(host);
}
...
/*
* Set host parameters.
*/
mmc->ops = &sdhci_ops;
mmc->f_max = host->max_clk;
which would have the effect of overwriting a previously set f_max from
the OF data.
There's also the whole "cd-gpios" thing which would need sorting out -
the imx sdhci driver already parses this property itself, and sets its
own internal data (so it knows whether it has to use the controller
based card detect or the gpio card detect) and simply adding a call to
mmc_of_parse() would result in the gpio slot stuff being setup twice.
The obvious solution here is to rewrite the sdhci initialisation such
that it uses the generic infrastructure, but I don't have the motivation
to do that (I've already plenty of patches to deal with that I don't
need any more at the moment.)
A simpler solution would be to split mmc_of_parse() so that the new bits
are a separate function, which the generic MMC core always calls for
every host - taking the decision over whether this is supported completely
away from hosts. I think that makes a lot of sense, especially as this
has nothing to do with the facilities found on any particular host.
There's another issue here about resets. Let's take the case where the
external card is powered off, but has active high resets. At the moment,
the sequence is this:
power: _____/~~~~~~~~~~~~
reset: __/~~~~\__________
That's not particularly nice, as the reset signal will tend do drive power
into the device before it's powered up via the clamping diodes in the case.
Generally, devices are not designed to be powered in this way. However,
this is a relatively minor issue though compared to this one, which is what
happens if the card uses active low reset:
power: _____/~~~~~~~~~~~~
reset: ~~\_____/~~~~~~~~~
This is definitely not good, because it means that the reset is higher for
longer, which may result in unacceptable dissapation in the package from
those clamping diodes. What we need instead is for active low reset is:
power: _____/~~~~~~~~~~~~
reset: ________/~~~~~~~~~
So, we need the GPIO layer to tell us whether the output is active high or
active low and adjust the initial setting accordingly. Basically, whenever
the attached device is powered down, GPIOs to it should be held at low level
or high impedance (with a pull-down to reduce the risks of ESD damage.)
I've seen designs get this wrong in the past - Intel Assabet is a good one
where the UDA1341 audio codec ends up illuminating a LED by being powered
not via it's supply pin, but by a CPLD output driving one of the I2S pins
high. The result is that the CPLD output sources quite a bit of current
into the UDA1341, which then holds other pins on the SA1110 around mid-rail,
which is the /worst/ thing you can do with CMOS. Powering chips via their
inputs is basically a big no-no.
So, I think something like the below is needed on top of your patches.
Note that I added -EPROBE_DEFER handling too (which fixes a bug, because
regulator_get() returns pointer-errors):
drivers/mmc/core/host.c | 90 +++++++++++++++++++++++++++-----------
1 files changed, 65 insertions(+), 25 deletions(-)
@@ -316,7 +316,7 @@ int mmc_of_parse(struct mmc_host *host)u32bus_width;boolexplicit_inv_wp,gpio_inv_wp=false;enumof_gpio_flagsflags;-inti,len,ret,gpio;+intlen,ret,gpio;if(!host->parent||!host->parent->of_node)return0;
@@ -419,30 +419,6 @@ int mmc_of_parse(struct mmc_host *host)if(explicit_inv_wp^gpio_inv_wp)host->caps2|=MMC_CAP2_RO_ACTIVE_HIGH;-/* Parse card power/reset/clock control */-if(of_find_property(np,"card-reset-gpios",NULL)){-structgpio_desc*gpd;-for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){-gpd=devm_gpiod_get_index(host->parent,"card-reset",i);-if(IS_ERR(gpd))-break;-gpiod_direction_output(gpd,0);-host->card_reset_gpios[i]=gpd;-}--gpd=devm_gpiod_get_index(host->parent,"card-reset",ARRAY_SIZE(host->card_reset_gpios));-if(!IS_ERR(gpd)){-dev_warn(host->parent,"More reset gpios than we can handle");-gpiod_put(gpd);-}-}--host->card_clk=of_clk_get_by_name(np,"card_ext_clock");-if(IS_ERR(host->card_clk))-host->card_clk=NULL;--host->card_regulator=regulator_get(host->parent,"card-external-vcc");-if(of_find_property(np,"cap-sd-highspeed",&len))host->caps|=MMC_CAP_SD_HIGHSPEED;if(of_find_property(np,"cap-mmc-highspeed",&len))
@@ -467,6 +443,66 @@ int mmc_of_parse(struct mmc_host *host)EXPORT_SYMBOL(mmc_of_parse);+staticintmmc_of_parse_child(structmmc_host*host)+{+structdevice_node*np;+structclk*clk;+inti;++if(!host->parent||!host->parent->of_node)+return0;++np=host->parent->of_node;++host->card_regulator=regulator_get(host->parent,"card-external-vcc");+if(IS_ERR(host->card_regulator)){+if(PTR_ERR(host->card_regulator)==-EPROBE_DEFER)+returnPTR_ERR(host->card_regulator);+host->card_regulator=NULL;+}++/* Parse card power/reset/clock control */+if(of_find_property(np,"card-reset-gpios",NULL)){+structgpio_desc*gpd;+intlevel=0;++/*+*Iftheregulatorisenabled,thenwecanholdthe+*cardinresetwithanactivehighresets.Otherwise,+*holdtheresetslow.+*/+if(host->card_regulator&®ulator_is_enabled(host->card_regulator))+level=1;++for(i=0;i<ARRAY_SIZE(host->card_reset_gpios);i++){+gpd=devm_gpiod_get_index(host->parent,"card-reset",i);+if(IS_ERR(gpd)){+if(PTR_ERR(gpd)==-EPROBE_DEFER)+returnPTR_ERR(gpd);+break;+}+gpiod_direction_output(gpd,gpiod_is_active_low(gpd)|level);+host->card_reset_gpios[i]=gpd;+}++gpd=devm_gpiod_get_index(host->parent,"card-reset",ARRAY_SIZE(host->card_reset_gpios));+if(!IS_ERR(gpd)){+dev_warn(host->parent,"More reset gpios than we can handle");+gpiod_put(gpd);+}+}++clk=of_clk_get_by_name(np,"card_ext_clock");+if(IS_ERR(clk)){+if(PTR_ERR(clk)==-EPROBE_DEFER)+returnPTR_ERR(clk);+clk=NULL;+}+host->card_clk=clk;++return0;+}+/***mmc_alloc_host-initialisetheper-hoststructure.*@extra:sizeofprivatedatastructure
@@ -546,6 +582,10 @@ int mmc_add_host(struct mmc_host *host){interr;+err=mmc_of_parse_child(host);+if(err)+returnerr;+WARN_ON((host->caps&MMC_CAP_SDIO_IRQ)&&!host->ops->enable_sdio_irq);
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
From: Russell King - ARM Linux <hidden> Date: 2014-02-13 10:42:48
On Tue, Jan 28, 2014 at 11:48:10AM +0100, Arnd Bergmann wrote:
I think there is another option, which does have its own pros and cons:
We could move all the power handling back into the sdio function driver
if we allow a secondary detection path using DT rather than the probing
of the SDIO bus.
No thanks.
What if we have a platform where things subtly change, like for instance,
the wiring on the SD slot to fix a problem with UHS-1 cards, which means
you don't have UHS-1 support for some platforms but do for others.
What if you have a platform which uses a brcm4329 chip for Wifi, but then
later in the production run switch to using a different Wifi chipset?
With this information encoded into DT, the number of DT files quickly
increases, and then this presents its own problem - how do users get to
know which DT file should be used for their platform when all they see
externally is "a product of type A"?
Let's say that the board folk were kind enough to set some kind of
identifing feature for the first but not the second (why would they,
it's probe-able, damn it).
The "we can do it in DT" approach just makes things unnecessarily more
difficult from the _user_ and _hardware_ point of view.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
On Thursday 13 February 2014 10:42:48 Russell King - ARM Linux wrote:
What if we have a platform where things subtly change, like for instance,
the wiring on the SD slot to fix a problem with UHS-1 cards, which means
you don't have UHS-1 support for some platforms but do for others.
What if you have a platform which uses a brcm4329 chip for Wifi, but then
later in the production run switch to using a different Wifi chipset?
As far as I can tell, the power sequencing is normally really
dependent on the device. If someone has an on-board brcm4329
that requires a specific set of clocks, resets, voltages etc
to be routed to the chip and enabled in the correct order to
allow probing, it seems unlikely that changing the wifi chipset
to something else would keep the exact same requirements.
With this information encoded into DT, the number of DT files quickly
increases, and then this presents its own problem - how do users get to
know which DT file should be used for their platform when all they see
externally is "a product of type A"?
I haven't seen a suggestion that would not encode this in DT. The
difference between Olof's original suggestion and mine is that he
proposed to put the data into the existing node of the host controller,
while my approach would be to add a node for each function with
these requirements and modify the sdio code so that we can deal
with it in the function driver. The data that would get passed
however is almost the same.
Arnd
From: Russell King - ARM Linux <hidden> Date: 2014-02-13 14:41:06
On Thu, Feb 13, 2014 at 01:48:55PM +0100, Arnd Bergmann wrote:
On Thursday 13 February 2014 10:42:48 Russell King - ARM Linux wrote:
quoted
What if we have a platform where things subtly change, like for instance,
the wiring on the SD slot to fix a problem with UHS-1 cards, which means
you don't have UHS-1 support for some platforms but do for others.
What if you have a platform which uses a brcm4329 chip for Wifi, but then
later in the production run switch to using a different Wifi chipset?
As far as I can tell, the power sequencing is normally really
dependent on the device. If someone has an on-board brcm4329
that requires a specific set of clocks, resets, voltages etc
to be routed to the chip and enabled in the correct order to
allow probing, it seems unlikely that changing the wifi chipset
to something else would keep the exact same requirements.
That's your assertion - however, do we /know/ whether there's a situation
where Olof's solution doesn't work because the sequencing is wrong?
I see nothing unreasonable about the sequence:
1. hold reset at low level
2. apply power
3. turn clock on
4. apply reset
5. release reset
The points being:
* never set a signal to a high level before power is applied, otherwise
we can end up supplying power through that signal (which may cause
damage.) That goes for the reset and clock.
* devices normally want clocks running to complete their reset sequencing.
This is actually the sequence which MMC/SD cards do (except without the
reset) anyway - it's spec'd that on the MMC/SD bus, power will be applied
and will be stable before the clock signal is applied, and then the clock
signal runs for a certain number of cycles before you even start talking
to the card.
That all said, we do have the problem that once we decide, we need to
support it because it becomes part of DT - this is one of the things I
hate about DT, it requires over-design. Your point is "Olof's solution
may break if we have a device which requires a different sequence" which
is a valid point which has to be considered from the DT perspective and
addressed whether or not we actually have a device which meets that
criteria. I don't see an easy solution to this.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
On Thursday 13 February 2014 14:41:06 Russell King - ARM Linux wrote:
On Thu, Feb 13, 2014 at 01:48:55PM +0100, Arnd Bergmann wrote:
quoted
On Thursday 13 February 2014 10:42:48 Russell King - ARM Linux wrote:
quoted
What if we have a platform where things subtly change, like for instance,
the wiring on the SD slot to fix a problem with UHS-1 cards, which means
you don't have UHS-1 support for some platforms but do for others.
What if you have a platform which uses a brcm4329 chip for Wifi, but then
later in the production run switch to using a different Wifi chipset?
As far as I can tell, the power sequencing is normally really
dependent on the device. If someone has an on-board brcm4329
that requires a specific set of clocks, resets, voltages etc
to be routed to the chip and enabled in the correct order to
allow probing, it seems unlikely that changing the wifi chipset
to something else would keep the exact same requirements.
That's your assertion - however, do we /know/ whether there's a situation
where Olof's solution doesn't work because the sequencing is wrong?
I see nothing unreasonable about the sequence:
1. hold reset at low level
2. apply power
3. turn clock on
4. apply reset
5. release reset
I was thinking of cases where you may need a more complex sequence:
- wait for a device specific time between some of the steps
(the cw1200 driver seems to need that, but we could probably
get away with waiting long enough for everyone)
- have more than one of each, and turn them on in the right order.
cw1200 seems to need two voltages, two gpio resets ("reset"
and "powerup").
Again, we could specify a larger number of clocks that can be
provided to the host, but if we make it a device specific
property, we already know how many we need.
I can't think of anything that would require significant changes
to the procedure though, just refinements as we run into problems.
The points being:
* never set a signal to a high level before power is applied, otherwise
we can end up supplying power through that signal (which may cause
damage.) That goes for the reset and clock.
* devices normally want clocks running to complete their reset sequencing.
This is actually the sequence which MMC/SD cards do (except without the
reset) anyway - it's spec'd that on the MMC/SD bus, power will be applied
and will be stable before the clock signal is applied, and then the clock
signal runs for a certain number of cycles before you even start talking
to the card.
It may be dangerous to refer to the spec, since we are talking
specifically about devices that require something beyond what the
spec says ;-) For instance in SD/MMC cards I'd assume the device clock
to be derived from the bus clock. However we can expect that clock
to work already (any working mmc host driver would provide that),
but we may need to drive the device clock. It still sounds reasonable
to assume that the sequencing is the same as for the bus clock.
That all said, we do have the problem that once we decide, we need to
support it because it becomes part of DT - this is one of the things I
hate about DT, it requires over-design.
Yes, I agree. It is a problem that we have to face all the time.
We have in the past defined bindings of both types, overdesigned
and not thought through enough.
Your point is "Olof's solution
may break if we have a device which requires a different sequence" which
is a valid point which has to be considered from the DT perspective and
addressed whether or not we actually have a device which meets that
criteria. I don't see an easy solution to this.
I think either one will work. With Olof's suggestion that may mean we
have to keep adding support for increasingly complex cases when we
run into them, or it may all be easy. With my suggestion, we give
more room for function drivers to mess things up, but at least we
can keep the complexity in the places that need them and only need
to change the core once.
Aside from the power-on problem, my suggestion would at the same
time solve the second problem of having a place to stick arbitrary
DT properties for the sdio function. Again looking at the cw1200
example, they may require passing an IRQ descriptor, a MAC address,
the device clock rate, and two flags for things that are not
detectable by looking at the device ID (whether a 5GHz antenna is
connected and something about odd block size transfers).
This is probably the main difference between the two approaches.
Arnd
From: Olof Johansson <hidden> Date: 2014-02-13 17:31:35
On Thu, Feb 13, 2014 at 05:13:14PM +0100, Arnd Bergmann wrote:
On Thursday 13 February 2014 14:41:06 Russell King - ARM Linux wrote:
quoted
On Thu, Feb 13, 2014 at 01:48:55PM +0100, Arnd Bergmann wrote:
quoted
On Thursday 13 February 2014 10:42:48 Russell King - ARM Linux wrote:
quoted
What if we have a platform where things subtly change, like for instance,
the wiring on the SD slot to fix a problem with UHS-1 cards, which means
you don't have UHS-1 support for some platforms but do for others.
What if you have a platform which uses a brcm4329 chip for Wifi, but then
later in the production run switch to using a different Wifi chipset?
As far as I can tell, the power sequencing is normally really
dependent on the device. If someone has an on-board brcm4329
that requires a specific set of clocks, resets, voltages etc
to be routed to the chip and enabled in the correct order to
allow probing, it seems unlikely that changing the wifi chipset
to something else would keep the exact same requirements.
That's your assertion - however, do we /know/ whether there's a situation
where Olof's solution doesn't work because the sequencing is wrong?
I see nothing unreasonable about the sequence:
1. hold reset at low level
2. apply power
3. turn clock on
4. apply reset
5. release reset
I was thinking of cases where you may need a more complex sequence:
- wait for a device specific time between some of the steps
(the cw1200 driver seems to need that, but we could probably
get away with waiting long enough for everyone)
- have more than one of each, and turn them on in the right order.
cw1200 seems to need two voltages, two gpio resets ("reset"
and "powerup").
Those two gpio resets are extremely common, but on snow (the chromebook) the
powerup gpio is hard-wired. So it's not all that unusual. As I mention in the
patch, a positive-sense powerup and a negative-sense reset aren't all that
different in practice.
Again, we could specify a larger number of clocks that can be
provided to the host, but if we make it a device specific
property, we already know how many we need.
I can't think of anything that would require significant changes
to the procedure though, just refinements as we run into problems.
The main pain will be if there's a requirement to do
gpio-requlator-gpio-regulator. We could mandate that regulators are turned on
in order. (Also, see below).
quoted
The points being:
* never set a signal to a high level before power is applied, otherwise
we can end up supplying power through that signal (which may cause
damage.) That goes for the reset and clock.
* devices normally want clocks running to complete their reset sequencing.
This is actually the sequence which MMC/SD cards do (except without the
reset) anyway - it's spec'd that on the MMC/SD bus, power will be applied
and will be stable before the clock signal is applied, and then the clock
signal runs for a certain number of cycles before you even start talking
to the card.
It may be dangerous to refer to the spec, since we are talking
specifically about devices that require something beyond what the
spec says ;-) For instance in SD/MMC cards I'd assume the device clock
to be derived from the bus clock. However we can expect that clock
to work already (any working mmc host driver would provide that),
but we may need to drive the device clock. It still sounds reasonable
to assume that the sequencing is the same as for the bus clock.
quoted
That all said, we do have the problem that once we decide, we need to
support it because it becomes part of DT - this is one of the things I
hate about DT, it requires over-design.
Yes, I agree. It is a problem that we have to face all the time.
We have in the past defined bindings of both types, overdesigned
and not thought through enough.
quoted
Your point is "Olof's solution
may break if we have a device which requires a different sequence" which
is a valid point which has to be considered from the DT perspective and
addressed whether or not we actually have a device which meets that
criteria. I don't see an easy solution to this.
I think either one will work. With Olof's suggestion that may mean we
have to keep adding support for increasingly complex cases when we
run into them, or it may all be easy. With my suggestion, we give
more room for function drivers to mess things up, but at least we
can keep the complexity in the places that need them and only need
to change the core once.
I always anticipated the binding needing amendment over time -- for example if
a device needs longer delays between clock enable and reset release. But most
of those can be handled through bindings amendments as needed (with default
behavior for non-amended bindings the same as today).
Chances are that if we do a per-device binding, we'll likely end up having
shared helpers to parse the settings anyway, so in the end we end up with
similar code, and similar bind maybe subtly different bindings. I suspect
sharing one common binding and common code will be easier long-term but it's
not a black and white choice.
Aside from the power-on problem, my suggestion would at the same
time solve the second problem of having a place to stick arbitrary
DT properties for the sdio function. Again looking at the cw1200
example, they may require passing an IRQ descriptor, a MAC address,
the device clock rate, and two flags for things that are not
detectable by looking at the device ID (whether a 5GHz antenna is
connected and something about odd block size transfers).
This is probably the main difference between the two approaches.
So, we do have the option of making the mmc binding take a device subnode
that gets passed in as the of_node when binding the device, and we can
move the power/reset/clock data there even if we don't leave it up to
the card driver to handle and act upon it. It would give us a place to
locate per-device properties like these, but it wouldn't greatly affect
how the rest of the solution looks.
-Olof
On Thursday 13 February 2014 17:13:14 Arnd Bergmann wrote:
Aside from the power-on problem, my suggestion would at the same
time solve the second problem of having a place to stick arbitrary
DT properties for the sdio function. Again looking at the cw1200
example, they may require passing an IRQ descriptor, a MAC address,
the device clock rate, and two flags for things that are not
detectable by looking at the device ID (whether a 5GHz antenna is
connected and something about odd block size transfers).
This is probably the main difference between the two approaches.
I just realized one more thing: Some devices have multiple
alternative bus interfaces and it would be nice to use just
one binding for the device, independent of the interface.
If a wlan adapter has both SPI and SDIO front-ends, the external
dependencies (reset, clock, voltage, ...) will be the same, and
from the kernel perspective the main difference is that SPI cannot
be probed at all, while SDIO can be probed as long as the device
is powered on already. If we do what I suggested and add a way to
the SDIO core to probe a device just from DT information, both
the binding and much of the device driver code can be shared
between the bus front-end glues, which would not be possible
if we rely on the SDIO bus probe and add the power-on sequencing
to the SDIO core.
Arnd
From: Russell King - ARM Linux <hidden> Date: 2014-02-15 12:27:33
On Sat, Feb 15, 2014 at 01:18:02PM +0100, Arnd Bergmann wrote:
If a wlan adapter has both SPI and SDIO front-ends, the external
dependencies (reset, clock, voltage, ...) will be the same, and
from the kernel perspective the main difference is that SPI cannot
be probed at all, while SDIO can be probed as long as the device
is powered on already.
Remember that MMC/SD/SDIO cards can be driven by either a MMC host
interface, or a SPI interface. Both are probe-able.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
On Saturday 15 February 2014 12:27:33 Russell King - ARM Linux wrote:
On Sat, Feb 15, 2014 at 01:18:02PM +0100, Arnd Bergmann wrote:
quoted
If a wlan adapter has both SPI and SDIO front-ends, the external
dependencies (reset, clock, voltage, ...) will be the same, and
from the kernel perspective the main difference is that SPI cannot
be probed at all, while SDIO can be probed as long as the device
is powered on already.
Remember that MMC/SD/SDIO cards can be driven by either a MMC host
interface, or a SPI interface. Both are probe-able.
I knew about MMC/SD cards being required to understand simple SPI,
I wasn't sure about SDIO. My understanding however is that you
have to use the mmc_spi host driver to actually use MMC/SD devices
as a block device, and that requires having either a DT description
for the host or an spi_board_info, which I would not consider
discoverable.
For spi-mode SDIO devices I'm assuming it's similar, except that
you'd describe the actual SDIO device in the board info rather than
create a fake SDIO controller. Still not discoverable unless I'm
missing your point.
Arnd
From: Tomasz Figa <hidden> Date: 2014-02-15 13:22:30
On 15.02.2014 14:09, Arnd Bergmann wrote:
On Saturday 15 February 2014 12:27:33 Russell King - ARM Linux wrote:
quoted
On Sat, Feb 15, 2014 at 01:18:02PM +0100, Arnd Bergmann wrote:
quoted
If a wlan adapter has both SPI and SDIO front-ends, the external
dependencies (reset, clock, voltage, ...) will be the same, and
from the kernel perspective the main difference is that SPI cannot
be probed at all, while SDIO can be probed as long as the device
is powered on already.
Remember that MMC/SD/SDIO cards can be driven by either a MMC host
interface, or a SPI interface. Both are probe-able.
I knew about MMC/SD cards being required to understand simple SPI,
I wasn't sure about SDIO. My understanding however is that you
have to use the mmc_spi host driver to actually use MMC/SD devices
as a block device, and that requires having either a DT description
for the host or an spi_board_info, which I would not consider
discoverable.
For spi-mode SDIO devices I'm assuming it's similar, except that
you'd describe the actual SDIO device in the board info rather than
create a fake SDIO controller. Still not discoverable unless I'm
missing your point.
I'm not sure if we should assume that SPI = MMC over SPI. I believe
there might be a custom protocol involved as well.
Stepping aside from SPI, I already gave an example of a WLAN chip that
supports multiple control busses [1]. In addition to the commonly used
SDIO, it supports USB and HSIC as well:
[1] http://www.marvell.com/wireless/assets/marvell_avastar_88w8797.pdf
Moreover, some of Samsung boards use HSIC to communicate with modem
chips, which have exactly the same problem as we're trying to solve here
- they need to be powered on to be discovered.
So I really don't think we should be limiting this to MMC alone by any
means.
Now I don't really know why we want that badly to represent low level
control parts of such devices as children of control buses of their
enumerable parts. Could you tell me what benefits it has to justify the
added complexity of having to instantiate fake devices in respective
devices, even though they can be fully detected later?
Best regards,
Tomasz
On Saturday 15 February 2014 14:22:30 Tomasz Figa wrote:
On 15.02.2014 14:09, Arnd Bergmann wrote:
quoted
For spi-mode SDIO devices I'm assuming it's similar, except that
you'd describe the actual SDIO device in the board info rather than
create a fake SDIO controller. Still not discoverable unless I'm
missing your point.
I'm not sure if we should assume that SPI = MMC over SPI. I believe
there might be a custom protocol involved as well.
In case of SD/MMC, you essentially have three separate command sets:
SPI, MMC and SD, and each of them has multiple versions. MMC and SD
compatible devices generally also support the SPI command set (IIRC
it is required, but I'm not completely sure), and on the bus
the main difference seems to be that you have only 1-bit serial
signalling, while MMC also supports 4-bit and 8-bit parallel
transmission.
If a device supports both SDIO and SPI, I think a straightforward
implementation would be to use the exact same command set, but
you are right that this isn't the only possibility, and the SD/MMC
shows how they can be slightly different already.
Stepping aside from SPI, I already gave an example of a WLAN chip that
supports multiple control busses [1]. In addition to the commonly used
SDIO, it supports USB and HSIC as well:
[1] http://www.marvell.com/wireless/assets/marvell_avastar_88w8797.pdf
Moreover, some of Samsung boards use HSIC to communicate with modem
chips, which have exactly the same problem as we're trying to solve here
- they need to be powered on to be discovered.
Thanks, this definitely makes a good example. I see that it also
supports SPI mode for SDIO as mentioned in your link.
So I really don't think we should be limiting this to MMC alone by any
means.
Agreed. Putting the same chip on USB or HSIC has the exact same
requirements, since we also have a discoverable bus, but actually
finding the device likely involves some power-on sequencing before
the bus controller can find it.
Now I don't really know why we want that badly to represent low level
control parts of such devices as children of control buses of their
enumerable parts. Could you tell me what benefits it has to justify the
added complexity of having to instantiate fake devices in respective
devices, even though they can be fully detected later?
It's not that I "badly want" to do it one way or another, I'm just
trying to find arguments either way, since as Russell points out
whatever we decide to do in DT is what we're stuck with for the
long term future.
Let me try to summarize what we found so far:
* Common aspects:
* we need a way to attach properties for run-time configuration
to devices on discoverable buses when the configuration
is not discoverable. In your 88w8797 example, this includes
the use of the GPIO pins, runtime power management (clocks,
regulators) and the attached codecs.
* We may want to connect the same device to either a discoverable
or a nondiscoverable bus, ideally using the same binding and
sharing code whereever possible.
* Olof's proposal (add properties or a child node to the host
controller node with just power-on sequencing information):
+ We only need one implementation for each bus, possibly shared
across buses to some degree, and can handle lots of devices
without having to touch their individual drivers.
+ A logical extension of things we already do on SD cards
(CD/WP GPIOs, external clocks and voltages supplied to
standard compliant devices as part of the normal probing)
- The shared code may get rather complex to deal with all
possible corner cases we run into over the years.
- Somewhat harder to do if you have to attach the power
information to a device node for a USB hub port, rather
than an SDIO controller that only has one slave device.
* Arnd's proposal (change bus code to probe nonstandard devices
from DT if we can't easily detect them):
+ Matches what we already do for PCI (at least on powerpc)
and AMBA/Primecell devices: If a device can't be probed
using the standard method, we treat it as nondiscoverable
and describe it using DT.
+ Devices can have arbitrary complex requirements without
impacting the core, since all code is contained in the
driver for the nonstandard device.
+ Properties that are required for probing and runtime
configuration only have to be set once (e.g. you may
need clk_get() for probing and clk_set_rate() for
runtime-pm).
+ Devices that have alternative bus interfaces like 88w8797
can implement the power-on code in a central place per
driver, and can reuse the code they have for nondiscoverable
buses on the buses that are normally discoverable but
broken here.
- Still need to modify each subsystem to have alternate
ways of probing, and match up devices later.
- Has to be implemented in each driver that needs it, making
it harder to share code for drivers with the same need
(e.g. every device that just needs an external reset
trigger).
Arnd
From: Russell King - ARM Linux <hidden> Date: 2014-02-15 20:52:00
On Sat, Feb 15, 2014 at 05:21:11PM +0100, Arnd Bergmann wrote:
On Saturday 15 February 2014 14:22:30 Tomasz Figa wrote:
quoted
On 15.02.2014 14:09, Arnd Bergmann wrote:
quoted
For spi-mode SDIO devices I'm assuming it's similar, except that
you'd describe the actual SDIO device in the board info rather than
create a fake SDIO controller. Still not discoverable unless I'm
missing your point.
I'm not sure if we should assume that SPI = MMC over SPI. I believe
there might be a custom protocol involved as well.
In case of SD/MMC, you essentially have three separate command sets:
SPI, MMC and SD, and each of them has multiple versions. MMC and SD
compatible devices generally also support the SPI command set (IIRC
it is required,
SPI support is mandatory for SDIO as well.
SDIO has CIS (remember card information structures... like PCMCIA)
which identifies the various different logical functions of the device,
giving class information, vendor information etc.
So... certainly the type of the attached device is discoverable even
on SPI.
If a device supports both SDIO and SPI, I think a straightforward
implementation would be to use the exact same command set, but
you are right that this isn't the only possibility, and the SD/MMC
shows how they can be slightly different already.
Given that the SPI mode is mandatory for SDIO cards, why would you
also implement another SPI mode with different commands?
quoted
Stepping aside from SPI, I already gave an example of a WLAN chip that
supports multiple control busses [1]. In addition to the commonly used
SDIO, it supports USB and HSIC as well:
[1] http://www.marvell.com/wireless/assets/marvell_avastar_88w8797.pdf
Moreover, some of Samsung boards use HSIC to communicate with modem
chips, which have exactly the same problem as we're trying to solve here
- they need to be powered on to be discovered.
Thanks, this definitely makes a good example. I see that it also
supports SPI mode for SDIO as mentioned in your link.
Well, USB is another discoverable bus. As HSIC is a derivative of USB,
I'd be surprised if it weren't discoverable there too.
So, out of everything identified so far, we have no undiscoverable buses.
Agreed. Putting the same chip on USB or HSIC has the exact same
requirements, since we also have a discoverable bus, but actually
finding the device likely involves some power-on sequencing before
the bus controller can find it.
That's partly the nature of integrating something onto a board where you
want maximal power savings. It's basically that dreaded word which
software people seem to hate: "embedded".
* Arnd's proposal (change bus code to probe nonstandard devices
from DT if we can't easily detect them):
+ Matches what we already do for PCI (at least on powerpc)
and AMBA/Primecell devices: If a device can't be probed
using the standard method, we treat it as nondiscoverable
and describe it using DT.
+ Devices can have arbitrary complex requirements without
impacting the core, since all code is contained in the
driver for the nonstandard device.
+ Properties that are required for probing and runtime
configuration only have to be set once (e.g. you may
need clk_get() for probing and clk_set_rate() for
runtime-pm).
+ Devices that have alternative bus interfaces like 88w8797
can implement the power-on code in a central place per
driver, and can reuse the code they have for nondiscoverable
buses on the buses that are normally discoverable but
broken here.
- Still need to modify each subsystem to have alternate
ways of probing, and match up devices later.
- Has to be implemented in each driver that needs it, making
it harder to share code for drivers with the same need
(e.g. every device that just needs an external reset
trigger).
- requires different DT if the chip is changed, which causes problems
for users to identify which out of zillions of DT files they should
use for their exact platform.
- have to work out how to match up the fake device with a probed device
when it becomes available: existing SDIO drivers all assume that the
card has been through a fairly complex initialisation sequence already.
- multi-function SDIO is much harder to deal with since you have mutliple
drivers involved, and the SDIO device as a whole needs initialisation
before anyone can drive it.
- adds complexity to the SDIO drivers; they would have to know whether
they're embedded or on a plug-in card.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
From: Tomasz Figa <hidden> Date: 2014-02-15 21:35:34
On 15.02.2014 21:52, Russell King - ARM Linux wrote:
On Sat, Feb 15, 2014 at 05:21:11PM +0100, Arnd Bergmann wrote:
quoted
On Saturday 15 February 2014 14:22:30 Tomasz Figa wrote:
quoted
On 15.02.2014 14:09, Arnd Bergmann wrote:
quoted
For spi-mode SDIO devices I'm assuming it's similar, except that
you'd describe the actual SDIO device in the board info rather than
create a fake SDIO controller. Still not discoverable unless I'm
missing your point.
I'm not sure if we should assume that SPI = MMC over SPI. I believe
there might be a custom protocol involved as well.
In case of SD/MMC, you essentially have three separate command sets:
SPI, MMC and SD, and each of them has multiple versions. MMC and SD
compatible devices generally also support the SPI command set (IIRC
it is required,
SPI support is mandatory for SDIO as well.
SDIO has CIS (remember card information structures... like PCMCIA)
which identifies the various different logical functions of the device,
giving class information, vendor information etc.
So... certainly the type of the attached device is discoverable even
on SPI.
It certainly is, assuming that you properly configure the
non-discoverable part.
quoted
If a device supports both SDIO and SPI, I think a straightforward
implementation would be to use the exact same command set, but
you are right that this isn't the only possibility, and the SD/MMC
shows how they can be slightly different already.
Given that the SPI mode is mandatory for SDIO cards, why would you
also implement another SPI mode with different commands?
Well, isn't this embedded world we're living in a constant race between
software and hardware engineers, where the former invent more and more
generic solutions to cover wider ranges of hardware with nice
abstractions, while the latter invent more quirky hardware designs to
destroy the effort of the former? Cute embedded nonsense hacks and so...
quoted
quoted
Stepping aside from SPI, I already gave an example of a WLAN chip that
supports multiple control busses [1]. In addition to the commonly used
SDIO, it supports USB and HSIC as well:
[1] http://www.marvell.com/wireless/assets/marvell_avastar_88w8797.pdf
Moreover, some of Samsung boards use HSIC to communicate with modem
chips, which have exactly the same problem as we're trying to solve here
- they need to be powered on to be discovered.
Thanks, this definitely makes a good example. I see that it also
supports SPI mode for SDIO as mentioned in your link.
Well, USB is another discoverable bus. As HSIC is a derivative of USB,
I'd be surprised if it weren't discoverable there too.
So, out of everything identified so far, we have no undiscoverable buses.
quoted
Agreed. Putting the same chip on USB or HSIC has the exact same
requirements, since we also have a discoverable bus, but actually
finding the device likely involves some power-on sequencing before
the bus controller can find it.
That's partly the nature of integrating something onto a board where you
want maximal power savings. It's basically that dreaded word which
software people seem to hate: "embedded".
This isn't something we can ignore, though.
quoted
* Arnd's proposal (change bus code to probe nonstandard devices
from DT if we can't easily detect them):
+ Matches what we already do for PCI (at least on powerpc)
and AMBA/Primecell devices: If a device can't be probed
using the standard method, we treat it as nondiscoverable
and describe it using DT.
+ Devices can have arbitrary complex requirements without
impacting the core, since all code is contained in the
driver for the nonstandard device.
+ Properties that are required for probing and runtime
configuration only have to be set once (e.g. you may
need clk_get() for probing and clk_set_rate() for
runtime-pm).
+ Devices that have alternative bus interfaces like 88w8797
can implement the power-on code in a central place per
driver, and can reuse the code they have for nondiscoverable
buses on the buses that are normally discoverable but
broken here.
- Still need to modify each subsystem to have alternate
ways of probing, and match up devices later.
- Has to be implemented in each driver that needs it, making
it harder to share code for drivers with the same need
(e.g. every device that just needs an external reset
trigger).
- requires different DT if the chip is changed, which causes problems
for users to identify which out of zillions of DT files they should
use for their exact platform.
That's how things work in embedded world, unfortunately. As Arnd stated
in one of his previous replies, different chips use to have different
power-up sequences and set of data the driver needs to know.
If you change your hardware in an incompatible way and such change can't
be detected automatically (i.e. the chip is non-discoverable) then I
don't know how you could let the OS know about this change in any other
way than providing it with the information it needs.
- have to work out how to match up the fake device with a probed device
when it becomes available: existing SDIO drivers all assume that the
card has been through a fairly complex initialisation sequence already.
- multi-function SDIO is much harder to deal with since you have mutliple
drivers involved, and the SDIO device as a whole needs initialisation
before anyone can drive it.
- adds complexity to the SDIO drivers; they would have to know whether
they're embedded or on a plug-in card.
The three points above could be eliminated by adopting the solution I
proposed [1]. Moreover it could be analogically implemented for other
bus types and use almost identical DT bindings.
[1] http://thread.gmane.org/gmane.linux.kernel.mmc/24728/focus=24864
Best regards,
Tomasz
From: Russell King - ARM Linux <hidden> Date: 2014-02-15 22:03:19
On Sat, Feb 15, 2014 at 10:35:34PM +0100, Tomasz Figa wrote:
If you change your hardware in an incompatible way and such change can't
be detected automatically (i.e. the chip is non-discoverable) then I
don't know how you could let the OS know about this change in any other
way than providing it with the information it needs.
I'm not talking about such major changes. I'm talking about minor
changes where the SDIO device may change, but everything else stays
compatible.
Why should such a situation be forced down the path of having a different
DT file to specify the different SDIO device, when the SDIO device itself
is discoverable? For example, BRCM4329 to a BRCM4330.
Look, let's think seriously about this. Let's say we have a platform
where there's already two different DT files - one for models with a
single CPU, and a different DT file for a quad CPU. This is fine,
users generally know which model they have, and can choose between
the two without much difficulty - because this information is part of
their decision making process when buying the product.
How, let's say that the SDIO chip becomes obsolete, and has to be
replaced later in the production run. The external form, fit and
function remain the same, it's just the internals have now changed.
Now, if we go down Arnd's route, then we need to have not two DT files,
but now four. One for single CPU with BRCM4329, quad CPU with the
same, and then those two with a BRCM4330.
Now, how does the user choose which DT file is right for their device?
Do they:
(a) ask the manufacturer, leading to hundreds of support requests.
(b) open the device up, invalidating the warranty to find out what chips
are inside.
(c) something else (please specify)
What I'm trying to get here is the perspective from the other side - the
/user/ perspective, and the problems that making the wrong decision here
brings. It has huge implications, and can make the difference between
"shall we use a mainline kernel, or shall we keep our own kernel/use an
ancient vendor's kernel with our own private hacks so we can reduce our
support costs".
I'm already seeing exactly this kind of problem on the horizon, because
I know of a platform where there was a bug in part of the hardware -
and enabling support for some interface features screws up on those
with the hardware bug. So, there's already _four_ DT files to think
about for this hardware platform. If Arnd's suggestion goes forward,
then we end up with _six_. It doesn't take much to see where this
leads.
Meanwhile, these kinds of differences could be dealt with using
/board specific code/ in the old days to read on-SoC configuration and
adjust things appropriately - and transparently to the user.
DT is _our_ convenience as kernel hackers. It is a big _inconvenience_
to the user when a particular product goes through a revision.
So, never ever forget that "oh, we can just deal with it with a different
DT file" makes our lives easier at the expense of our users, and without
users, we've lost.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
On Saturday 15 February 2014, Russell King - ARM Linux wrote:
On Sat, Feb 15, 2014 at 05:21:11PM +0100, Arnd Bergmann wrote:
quoted
On Saturday 15 February 2014 14:22:30 Tomasz Figa wrote:
quoted
On 15.02.2014 14:09, Arnd Bergmann wrote:
quoted
For spi-mode SDIO devices I'm assuming it's similar, except that
you'd describe the actual SDIO device in the board info rather than
create a fake SDIO controller. Still not discoverable unless I'm
missing your point.
I'm not sure if we should assume that SPI = MMC over SPI. I believe
there might be a custom protocol involved as well.
In case of SD/MMC, you essentially have three separate command sets:
SPI, MMC and SD, and each of them has multiple versions. MMC and SD
compatible devices generally also support the SPI command set (IIRC
it is required,
SPI support is mandatory for SDIO as well.
SDIO has CIS (remember card information structures... like PCMCIA)
which identifies the various different logical functions of the device,
giving class information, vendor information etc.
So... certainly the type of the attached device is discoverable even
on SPI
But how are SPI SDIO devices used in Linux and described in DT?
If we always use the MMC_SPI host controller driver and only
describe the controller in DT, then you are right, and there is
no difference to the normal SDIO case.
If we just describe it as an SPI device OTOH, the fact that we can
identify it is useless, because we first have to know what it
is before we can ask for its identity.
quoted
If a device supports both SDIO and SPI, I think a straightforward
implementation would be to use the exact same command set, but
you are right that this isn't the only possibility, and the SD/MMC
shows how they can be slightly different already.
Given that the SPI mode is mandatory for SDIO cards, why would you
also implement another SPI mode with different commands?
I don't know why, I was just saying that it's what storage SD cards do.
quoted
quoted
Stepping aside from SPI, I already gave an example of a WLAN chip that
supports multiple control busses [1]. In addition to the commonly used
SDIO, it supports USB and HSIC as well:
[1] http://www.marvell.com/wireless/assets/marvell_avastar_88w8797.pdf
Moreover, some of Samsung boards use HSIC to communicate with modem
chips, which have exactly the same problem as we're trying to solve here
- they need to be powered on to be discovered.
Thanks, this definitely makes a good example. I see that it also
supports SPI mode for SDIO as mentioned in your link.
Well, USB is another discoverable bus. As HSIC is a derivative of USB,
I'd be surprised if it weren't discoverable there too.
So, out of everything identified so far, we have no undiscoverable buses.
The device also lists a high-speed uart interface. While in Linux a
tty line discipline is not exactly the same as a driver, I would
still count it as a nondiscoverable interface.
- requires different DT if the chip is changed, which causes problems
for users to identify which out of zillions of DT files they should
use for their exact platform.
I'm still not buying this argument. Making any changes to the board
design that are visible to software can have this effect.
- have to work out how to match up the fake device with a probed device
when it becomes available: existing SDIO drivers all assume that the
card has been through a fairly complex initialisation sequence already.
This one is an implementation detail. Yes, we have to ensure it works,
but it's not inherently harder that getting the other approach to work.
- multi-function SDIO is much harder to deal with since you have mutliple
drivers involved, and the SDIO device as a whole needs initialisation
before anyone can drive it.
Agreed, this point is a big one that I hadn't considered before.
I agree that my approach isn't ideal for multifunction cards, especially
when you want to be able to load function drivers individually or have
only one of them in use. It can probably work fine for things like calling
clk_get() where you can simply describe the clock in each function device
node separately, but I don't see how I'd easily use a 'reset' line
that has to be triggered for the first function driver that gets loaded
but must not be triggered again for the second one :(
- adds complexity to the SDIO drivers; they would have to know whether
they're embedded or on a plug-in card.
How so? If a device can be a plug-in card, I would assume that it is
actually discoverable without a special power-on sequence, while a device
that needs this can't be used standalone.
Arnd
From: Mark Brown <hidden> Date: 2014-02-17 23:25:11
On Sat, Feb 15, 2014 at 05:21:11PM +0100, Arnd Bergmann wrote:
On Saturday 15 February 2014 14:22:30 Tomasz Figa wrote:
quoted
I'm not sure if we should assume that SPI = MMC over SPI. I believe
there might be a custom protocol involved as well.
In case of SD/MMC, you essentially have three separate command sets:
SPI, MMC and SD, and each of them has multiple versions. MMC and SD
compatible devices generally also support the SPI command set (IIRC
...
If a device supports both SDIO and SPI, I think a straightforward
implementation would be to use the exact same command set, but
you are right that this isn't the only possibility, and the SD/MMC
shows how they can be slightly different already.
I'm aware of existing devices that do in fact break this assumption.
quoted
Stepping aside from SPI, I already gave an example of a WLAN chip that
supports multiple control busses [1]. In addition to the commonly used
SDIO, it supports USB and HSIC as well:
Moreover, some of Samsung boards use HSIC to communicate with modem
chips, which have exactly the same problem as we're trying to solve here
- they need to be powered on to be discovered.
Thanks, this definitely makes a good example. I see that it also
supports SPI mode for SDIO as mentioned in your link.
Slimbus has all these issues too with the added fun that for normal
operation some devices want to be in a low power mode where they're
disconnected from the Slimbus a lot of the time.
* Olof's proposal (add properties or a child node to the host
controller node with just power-on sequencing information):
+ We only need one implementation for each bus, possibly shared
across buses to some degree, and can handle lots of devices
without having to touch their individual drivers.
+ A logical extension of things we already do on SD cards
(CD/WP GPIOs, external clocks and voltages supplied to
standard compliant devices as part of the normal probing)
- The shared code may get rather complex to deal with all
possible corner cases we run into over the years.
- Somewhat harder to do if you have to attach the power
information to a device node for a USB hub port, rather
than an SDIO controller that only has one slave device.
We would also need mechanisms to allow devices to take over the running
of their own resources for cases like the Slimbus ones I mentioned, and
some mechanism to cope with devices that hotplug themselves in normal
operation.
* Arnd's proposal (change bus code to probe nonstandard devices
from DT if we can't easily detect them):
I've also proposed this in the past FWIW but never got far enough
through my list of things I want to do with my subsystems to actually
start coding.
- Has to be implemented in each driver that needs it, making
it harder to share code for drivers with the same need
(e.g. every device that just needs an external reset
trigger).
I think we can probably come up with some standard helpers that work
well for the common case here (see also some of the discussions about
power domains ensuring that core IP clocks are provided for IPs, it's
kind of circling back to the same issues).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140218/ff86593e/attachment.sig>