| Message ID | 1421240530-7971-3-git-send-email-ulf.hansson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Ulf, On Wed, Jan 14, 2015 at 01:02:08PM +0000, Ulf Hansson wrote: > The simple MMC power sequence provider, intends to supports a set of > common properties between SOC designs. It thus enables us to re-use the > same provider for several SOCs. > > In this initial step, let's add the top level description of the MMC > power sequence and describe the compatible string for the simple MMC > power sequence provider. > > Following patches will step by step add support for new properties to > the simple MMC power sequence provider. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > --- > > Changes in v2: > - None. > > --- > .../devicetree/bindings/mmc/mmc,pwrseq-simple.txt | 18 ++++++++++++++++++ > Documentation/devicetree/bindings/mmc/mmc.txt | 5 +++++ > 2 files changed, 23 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt > > diff --git a/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt b/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt > new file mode 100644 > index 0000000..e1b7f9c > --- /dev/null > +++ b/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt > @@ -0,0 +1,18 @@ > +* The simple MMC power sequence provider > + > +System on chip designs may specify a specific MMC power sequence. To > +successfully detect an (e)MMC/SD/SDIO card, that power sequence must be > +maintained while initializing the card. > + > +The simple MMC power sequence provider, intends to supports a set of common > +properties between SOC designs. It thus enables us to re-use the same provider > +for several SOC designs. > + > +Required properties: > +- compatible : contains "mmc,pwrseq-simple". Nit: "mmc" is not a vendor prefix. > + > +Example: > + > + sdhci0_pwrseq { > + compatible = "mmc,pwrseq-simple"; > + } I'm a little confused here. What specific sequence is described by this node? We don't appear to have referred to any resources used as part of that sequence, and the description above only mentions that there could be a specific sequence, not what that sequence is. So I don't think this makes sense on its own, and should probably be folded with patches adding the initial support for the resources used as part of the sequence (e.g. the GPIOs added in a later patch). Thanks, Mark. > diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt > index bac1311..b12de1e 100644 > --- a/Documentation/devicetree/bindings/mmc/mmc.txt > +++ b/Documentation/devicetree/bindings/mmc/mmc.txt > @@ -64,6 +64,10 @@ Optional SDIO properties: > - keep-power-in-suspend: Preserves card power during a suspend/resume cycle > - enable-sdio-wakeup: Enables wake up of host system on SDIO IRQ assertion > > +Optional MMC power sequence: > +- mmc-pwrseq: phandle to the MMC power sequence node. See "mmc,pwrseq-*" > + for documentation of MMC power sequence bindings. > + > > Use of Function subnodes > ------------------------ > @@ -101,6 +105,7 @@ sdhci@ab000000 { > max-frequency = <50000000>; > keep-power-in-suspend; > enable-sdio-wakeup; > + mmc-pwrseq = <&sdhci0_pwrseq> > } > > Example with sdio function subnode: > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 15 Jan 2015 16:58:26 +0000 Mark Rutland <mark.rutland@arm.com> wrote: > Hi Ulf, > > On Wed, Jan 14, 2015 at 01:02:08PM +0000, Ulf Hansson wrote: > > The simple MMC power sequence provider, intends to supports a set of > > common properties between SOC designs. It thus enables us to re-use the > > same provider for several SOCs. > > > > In this initial step, let's add the top level description of the MMC > > power sequence and describe the compatible string for the simple MMC > > power sequence provider. > > > > Following patches will step by step add support for new properties to > > the simple MMC power sequence provider. > > > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > --- > > > > Changes in v2: > > - None. > > > > --- > > .../devicetree/bindings/mmc/mmc,pwrseq-simple.txt | 18 ++++++++++++++++++ > > Documentation/devicetree/bindings/mmc/mmc.txt | 5 +++++ > > 2 files changed, 23 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt > > > > diff --git a/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt b/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt > > new file mode 100644 > > index 0000000..e1b7f9c > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt > > @@ -0,0 +1,18 @@ > > +* The simple MMC power sequence provider > > + > > +System on chip designs may specify a specific MMC power sequence. To > > +successfully detect an (e)MMC/SD/SDIO card, that power sequence must be > > +maintained while initializing the card. > > + > > +The simple MMC power sequence provider, intends to supports a set of common > > +properties between SOC designs. It thus enables us to re-use the same provider > > +for several SOC designs. > > + > > +Required properties: > > +- compatible : contains "mmc,pwrseq-simple". > > Nit: "mmc" is not a vendor prefix. > > > + > > +Example: > > + > > + sdhci0_pwrseq { > > + compatible = "mmc,pwrseq-simple"; > > + } > > I'm a little confused here. What specific sequence is described by this > node? We don't appear to have referred to any resources used as part of > that sequence, and the description above only mentions that there could > be a specific sequence, not what that sequence is. > > So I don't think this makes sense on its own, and should probably be > folded with patches adding the initial support for the resources used as > part of the sequence (e.g. the GPIOs added in a later patch). > I guess I assumed that this "simple" referred to the current behaviour, which includes some of vmmc-supply, vqmmc-supply, vmmc_aux-supply and pbias-supply being switched at "appropriate" times. Would it make sense to bring all of these explicitly under the "pwrseq" umbrella, leaving the current behaviour only when no pwrseq node is provided? Also, it isn't clear to me whether the need for power/reset is a function of the mmc-host, or a function of the device attached to the host. I suspect some are needed by one, some by the other. Any by both? Should the two needs be kept separate? NeilBrown
On 15 January 2015 at 17:58, Mark Rutland <mark.rutland@arm.com> wrote: > Hi Ulf, > > On Wed, Jan 14, 2015 at 01:02:08PM +0000, Ulf Hansson wrote: >> The simple MMC power sequence provider, intends to supports a set of >> common properties between SOC designs. It thus enables us to re-use the >> same provider for several SOCs. >> >> In this initial step, let's add the top level description of the MMC >> power sequence and describe the compatible string for the simple MMC >> power sequence provider. >> >> Following patches will step by step add support for new properties to >> the simple MMC power sequence provider. >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >> --- >> >> Changes in v2: >> - None. >> >> --- >> .../devicetree/bindings/mmc/mmc,pwrseq-simple.txt | 18 ++++++++++++++++++ >> Documentation/devicetree/bindings/mmc/mmc.txt | 5 +++++ >> 2 files changed, 23 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt >> >> diff --git a/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt b/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt >> new file mode 100644 >> index 0000000..e1b7f9c >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt >> @@ -0,0 +1,18 @@ >> +* The simple MMC power sequence provider >> + >> +System on chip designs may specify a specific MMC power sequence. To >> +successfully detect an (e)MMC/SD/SDIO card, that power sequence must be >> +maintained while initializing the card. >> + >> +The simple MMC power sequence provider, intends to supports a set of common >> +properties between SOC designs. It thus enables us to re-use the same provider >> +for several SOC designs. >> + >> +Required properties: >> +- compatible : contains "mmc,pwrseq-simple". > > Nit: "mmc" is not a vendor prefix. > >> + >> +Example: >> + >> + sdhci0_pwrseq { >> + compatible = "mmc,pwrseq-simple"; >> + } > > I'm a little confused here. What specific sequence is described by this > node? We don't appear to have referred to any resources used as part of > that sequence, and the description above only mentions that there could > be a specific sequence, not what that sequence is. > > So I don't think this makes sense on its own, and should probably be > folded with patches adding the initial support for the resources used as > part of the sequence (e.g. the GPIOs added in a later patch). You are absolutely right. I fix it. Again, thanks for reviewing. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15 January 2015 at 19:53, NeilBrown <neilb@suse.de> wrote: > On Thu, 15 Jan 2015 16:58:26 +0000 Mark Rutland <mark.rutland@arm.com> wrote: > >> Hi Ulf, >> >> On Wed, Jan 14, 2015 at 01:02:08PM +0000, Ulf Hansson wrote: >> > The simple MMC power sequence provider, intends to supports a set of >> > common properties between SOC designs. It thus enables us to re-use the >> > same provider for several SOCs. >> > >> > In this initial step, let's add the top level description of the MMC >> > power sequence and describe the compatible string for the simple MMC >> > power sequence provider. >> > >> > Following patches will step by step add support for new properties to >> > the simple MMC power sequence provider. >> > >> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >> > --- >> > >> > Changes in v2: >> > - None. >> > >> > --- >> > .../devicetree/bindings/mmc/mmc,pwrseq-simple.txt | 18 ++++++++++++++++++ >> > Documentation/devicetree/bindings/mmc/mmc.txt | 5 +++++ >> > 2 files changed, 23 insertions(+) >> > create mode 100644 Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt >> > >> > diff --git a/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt b/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt >> > new file mode 100644 >> > index 0000000..e1b7f9c >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt >> > @@ -0,0 +1,18 @@ >> > +* The simple MMC power sequence provider >> > + >> > +System on chip designs may specify a specific MMC power sequence. To >> > +successfully detect an (e)MMC/SD/SDIO card, that power sequence must be >> > +maintained while initializing the card. >> > + >> > +The simple MMC power sequence provider, intends to supports a set of common >> > +properties between SOC designs. It thus enables us to re-use the same provider >> > +for several SOC designs. >> > + >> > +Required properties: >> > +- compatible : contains "mmc,pwrseq-simple". >> >> Nit: "mmc" is not a vendor prefix. >> >> > + >> > +Example: >> > + >> > + sdhci0_pwrseq { >> > + compatible = "mmc,pwrseq-simple"; >> > + } >> >> I'm a little confused here. What specific sequence is described by this >> node? We don't appear to have referred to any resources used as part of >> that sequence, and the description above only mentions that there could >> be a specific sequence, not what that sequence is. >> >> So I don't think this makes sense on its own, and should probably be >> folded with patches adding the initial support for the resources used as >> part of the sequence (e.g. the GPIOs added in a later patch). >> > > I guess I assumed that this "simple" referred to the current behaviour, which > includes some of vmmc-supply, vqmmc-supply, vmmc_aux-supply and pbias-supply > being switched at "appropriate" times. > > Would it make sense to bring all of these explicitly under the "pwrseq" > umbrella, leaving the current behaviour only when no pwrseq node is provided? In principle there is nothing that prevents us from following your suggestion. But why should we? Moreover, I don't think it's required at this point, let's first get the basic mmc-pwrseq support in place. Then we can evolve it. :-) > > Also, it isn't clear to me whether the need for power/reset is a function of > the mmc-host, or a function of the device attached to the host. I suspect > some are needed by one, some by the other. Any by both? To me the the power is coupled with the host and thus controlled by the mmc layer. But I guess we can discuss this in a forever long loop. :-) They may be other resources which is coupled with an SDIO function / eMMC,SDIO card, such as irqs. Those shall be described in the child node of the host. I have queued the below patchset from Hans, which I believe should address this: https://www.mail-archive.com/linux-mmc@vger.kernel.org/msg27241.html > > Should the two needs be kept separate? Yes, I think so. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 16, 2015 at 07:53:08AM +1300, NeilBrown wrote: > Also, it isn't clear to me whether the need for power/reset is a function of > the mmc-host, or a function of the device attached to the host. I suspect > some are needed by one, some by the other. Any by both? Neil, There's a horrid issue there. The standard model of MMC/SD is that the device will be powered up by the host, and will then be probed to find out what the device is. This normally works fine as the host is responsible for controlling the power to the socket which the card is plugged into. Where things fall down is when you have a MMC/SD device embedded onto a board, and they decide that it'll be given separate controls for power and reset which are not under the control of the host. If the MMC/SD device is not powered up before we probe the bus, then the device is not discoverable, and this breaks the standard model: - If we run the standard MMC/SD device discovery code with the device powered down, it will find no device attached, so no device will be created. - If we create a device, then the device driver will be probed immediately. While the device driver can then bind, discover the power and reset controls, and activate them, it can't then talk to its device because the MMC layer hasn't gone through the required standard device discovery and probe sequence. If we could do that, we would then need some way to stop that mechanism creating another device. If we instead take the view that the host is responsible for powering up the device, then we are merely keeping with the standard MMC/SD model, and we don't have to hack around his. Keeping to the standard model of a host responsible for power control of its child devices is just a whole lot nicer.
diff --git a/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt b/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt new file mode 100644 index 0000000..e1b7f9c --- /dev/null +++ b/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt @@ -0,0 +1,18 @@ +* The simple MMC power sequence provider + +System on chip designs may specify a specific MMC power sequence. To +successfully detect an (e)MMC/SD/SDIO card, that power sequence must be +maintained while initializing the card. + +The simple MMC power sequence provider, intends to supports a set of common +properties between SOC designs. It thus enables us to re-use the same provider +for several SOC designs. + +Required properties: +- compatible : contains "mmc,pwrseq-simple". + +Example: + + sdhci0_pwrseq { + compatible = "mmc,pwrseq-simple"; + } diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt index bac1311..b12de1e 100644 --- a/Documentation/devicetree/bindings/mmc/mmc.txt +++ b/Documentation/devicetree/bindings/mmc/mmc.txt @@ -64,6 +64,10 @@ Optional SDIO properties: - keep-power-in-suspend: Preserves card power during a suspend/resume cycle - enable-sdio-wakeup: Enables wake up of host system on SDIO IRQ assertion +Optional MMC power sequence: +- mmc-pwrseq: phandle to the MMC power sequence node. See "mmc,pwrseq-*" + for documentation of MMC power sequence bindings. + Use of Function subnodes ------------------------ @@ -101,6 +105,7 @@ sdhci@ab000000 { max-frequency = <50000000>; keep-power-in-suspend; enable-sdio-wakeup; + mmc-pwrseq = <&sdhci0_pwrseq> } Example with sdio function subnode:
The simple MMC power sequence provider, intends to supports a set of common properties between SOC designs. It thus enables us to re-use the same provider for several SOCs. In this initial step, let's add the top level description of the MMC power sequence and describe the compatible string for the simple MMC power sequence provider. Following patches will step by step add support for new properties to the simple MMC power sequence provider. Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- Changes in v2: - None. --- .../devicetree/bindings/mmc/mmc,pwrseq-simple.txt | 18 ++++++++++++++++++ Documentation/devicetree/bindings/mmc/mmc.txt | 5 +++++ 2 files changed, 23 insertions(+) create mode 100644 Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt