| Message ID | 1381936707-10336-2-git-send-email-balajitk@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Oct 16, 2013 at 04:18:22PM +0100, Balaji T K wrote: > From: Sekhar Nori <nsekhar@ti.com> > > add generic "non-removable" binding support for omap_hsmmc > > Signed-off-by: Sekhar Nori <nsekhar@ti.com> > Signed-off-by: Balaji T K <balajitk@ti.com> > --- > .../devicetree/bindings/mmc/ti-omap-hsmmc.txt | 2 +- > drivers/mmc/host/omap_hsmmc.c | 3 +++ > 2 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt > index 8c8908a..3b95719 100644 > --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt > +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt > @@ -17,7 +17,7 @@ Optional properties: > ti,dual-volt: boolean, supports dual voltage cards > <supply-name>-supply: phandle to the regulator device tree node > "supply-name" examples are "vmmc", "vmmc_aux" etc > -ti,non-removable: non-removable slot (like eMMC) > +ti,non-removable: non-removable eMMC with always on vccq and configurable vcc Why this change? What do "vccq" and "vcc" correspond to? The regulators are called "vmmc" and "vmmc_aux"... Why is no mention of "non-removable" added, given that it's added to the code? Is one preferred over the other? That should be noted. > ti,needs-special-reset: Requires a special softreset sequence > ti,needs-special-hs-handling: HSMMC IP needs special setting for handling High Speed > dmas: List of DMA specifiers with the controller specific format > diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c > index 6ac63df..5992048 100644 > --- a/drivers/mmc/host/omap_hsmmc.c > +++ b/drivers/mmc/host/omap_hsmmc.c > @@ -1738,6 +1738,9 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev) > pdata->slots[0].switch_pin = cd_gpio; > pdata->slots[0].gpio_wp = wp_gpio; > > + if (of_find_property(np, "non-removable", NULL)) { > + pdata->slots[0].nonremovable = true; > + } This wasn't mentioned in the binding, and it seems to have different semantics to "ti,non-removable". Why is it different? > if (of_find_property(np, "ti,non-removable", NULL)) { > pdata->slots[0].nonremovable = true; > pdata->slots[0].no_regulator_off_init = true; Cheers, Mark. -- 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 Thursday 17 October 2013 02:08 PM, Mark Rutland wrote: > On Wed, Oct 16, 2013 at 04:18:22PM +0100, Balaji T K wrote: >> From: Sekhar Nori <nsekhar@ti.com> >> >> add generic "non-removable" binding support for omap_hsmmc >> >> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >> Signed-off-by: Balaji T K <balajitk@ti.com> >> --- >> .../devicetree/bindings/mmc/ti-omap-hsmmc.txt | 2 +- >> drivers/mmc/host/omap_hsmmc.c | 3 +++ >> 2 files changed, 4 insertions(+), 1 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt >> index 8c8908a..3b95719 100644 >> --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt >> +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt >> @@ -17,7 +17,7 @@ Optional properties: >> ti,dual-volt: boolean, supports dual voltage cards >> <supply-name>-supply: phandle to the regulator device tree node >> "supply-name" examples are "vmmc", "vmmc_aux" etc >> -ti,non-removable: non-removable slot (like eMMC) >> +ti,non-removable: non-removable eMMC with always on vccq and configurable vcc > > Why this change? > Hi, earlier ti,non-removable was used for all eMMC and SDIO card, now it will be used only for eMMC with always on vccq and configurable vcc. > What do "vccq" and "vcc" correspond to? The regulators are called "vmmc" > and "vmmc_aux"... > vccq and vcc are supply names of eMMC part > Why is no mention of "non-removable" added, given that it's added to the > code? Because this file makes a reference to mmc.txt and the core properties described by mmc.txt are not added in ti-omap-hsmmc.txt > > Is one preferred over the other? That should be noted. > >> ti,needs-special-reset: Requires a special softreset sequence >> ti,needs-special-hs-handling: HSMMC IP needs special setting for handling High Speed >> dmas: List of DMA specifiers with the controller specific format >> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c >> index 6ac63df..5992048 100644 >> --- a/drivers/mmc/host/omap_hsmmc.c >> +++ b/drivers/mmc/host/omap_hsmmc.c >> @@ -1738,6 +1738,9 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev) >> pdata->slots[0].switch_pin = cd_gpio; >> pdata->slots[0].gpio_wp = wp_gpio; >> >> + if (of_find_property(np, "non-removable", NULL)) { >> + pdata->slots[0].nonremovable = true; >> + } > > This wasn't mentioned in the binding, and it seems to have different > semantics to "ti,non-removable". Why is it different? > When ti,non-removable was added, Only OMAP platform that had eMMC was that on OMAP4 where power to eMMC cannot be switched off without sending CMD5 sleep command, so no_regulator_off_init was needed to get it detected during boot. Now start using generic non-removable for all removable cards which do not have such limitation. >> if (of_find_property(np, "ti,non-removable", NULL)) { >> pdata->slots[0].nonremovable = true; >> pdata->slots[0].no_regulator_off_init = true; > > Cheers, > Mark. > -- 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, Oct 17, 2013 at 11:53:48AM +0100, Balaji T K wrote: > On Thursday 17 October 2013 02:08 PM, Mark Rutland wrote: > > On Wed, Oct 16, 2013 at 04:18:22PM +0100, Balaji T K wrote: > >> From: Sekhar Nori <nsekhar@ti.com> > >> > >> add generic "non-removable" binding support for omap_hsmmc > >> > >> Signed-off-by: Sekhar Nori <nsekhar@ti.com> > >> Signed-off-by: Balaji T K <balajitk@ti.com> > >> --- > >> .../devicetree/bindings/mmc/ti-omap-hsmmc.txt | 2 +- > >> drivers/mmc/host/omap_hsmmc.c | 3 +++ > >> 2 files changed, 4 insertions(+), 1 deletions(-) > >> > >> diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt > >> index 8c8908a..3b95719 100644 > >> --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt > >> +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt > >> @@ -17,7 +17,7 @@ Optional properties: > >> ti,dual-volt: boolean, supports dual voltage cards > >> <supply-name>-supply: phandle to the regulator device tree node > >> "supply-name" examples are "vmmc", "vmmc_aux" etc > >> -ti,non-removable: non-removable slot (like eMMC) > >> +ti,non-removable: non-removable eMMC with always on vccq and configurable vcc > > > > Why this change? > > > Hi, > > earlier ti,non-removable was used for all eMMC and SDIO card, now it will > be used only for eMMC with always on vccq and configurable vcc. Please expand the commit message to mention this. It wasn't clear why adding support for a property meant modifying the description of another. > > > What do "vccq" and "vcc" correspond to? The regulators are called "vmmc" > > and "vmmc_aux"... > > > > vccq and vcc are supply names of eMMC part The binding has vmmc-supply and vmmc_aux-supply. How do {vmmc,vmmc_aux} and {vcc,vccq} relate? That should be clarified in the binding document, something like: - vmmc-supply: phandle of the regulator for the VCC input - vmmc_aux-supply: phandle of the regulator for the VCCQ input > > > Why is no mention of "non-removable" added, given that it's added to the > > code? > > Because this file makes a reference to mmc.txt and the core properties described > by mmc.txt are not added in ti-omap-hsmmc.txt There is room for confusion here. While "non-removable" is a generic property, it would be good to contrast "non-removable" and "ti,non-removable" in the binding as they imply different things. > > > > > Is one preferred over the other? That should be noted. > > > >> ti,needs-special-reset: Requires a special softreset sequence > >> ti,needs-special-hs-handling: HSMMC IP needs special setting for handling High Speed > >> dmas: List of DMA specifiers with the controller specific format > >> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c > >> index 6ac63df..5992048 100644 > >> --- a/drivers/mmc/host/omap_hsmmc.c > >> +++ b/drivers/mmc/host/omap_hsmmc.c > >> @@ -1738,6 +1738,9 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev) > >> pdata->slots[0].switch_pin = cd_gpio; > >> pdata->slots[0].gpio_wp = wp_gpio; > >> > >> + if (of_find_property(np, "non-removable", NULL)) { > >> + pdata->slots[0].nonremovable = true; > >> + } > > > > This wasn't mentioned in the binding, and it seems to have different > > semantics to "ti,non-removable". Why is it different? > > > > When ti,non-removable was added, Only OMAP platform that had eMMC was that on OMAP4 > where power to eMMC cannot be switched off without sending CMD5 sleep command, > so no_regulator_off_init was needed to get it detected during boot. > > Now start using generic non-removable for all removable cards which do not > have such limitation. OK. I think this would be much clearer with something in the binding contrasting the two properties. Thanks, Mark. -- 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 Thursday 17 October 2013 08:55 PM, Mark Rutland wrote: > On Thu, Oct 17, 2013 at 11:53:48AM +0100, Balaji T K wrote: >> On Thursday 17 October 2013 02:08 PM, Mark Rutland wrote: >>> On Wed, Oct 16, 2013 at 04:18:22PM +0100, Balaji T K wrote: >>>> From: Sekhar Nori <nsekhar@ti.com> >>>> >>>> add generic "non-removable" binding support for omap_hsmmc >>>> >>>> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >>>> Signed-off-by: Balaji T K <balajitk@ti.com> >>>> --- >>>> .../devicetree/bindings/mmc/ti-omap-hsmmc.txt | 2 +- >>>> drivers/mmc/host/omap_hsmmc.c | 3 +++ >>>> 2 files changed, 4 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt >>>> index 8c8908a..3b95719 100644 >>>> --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt >>>> +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt >>>> @@ -17,7 +17,7 @@ Optional properties: >>>> ti,dual-volt: boolean, supports dual voltage cards >>>> <supply-name>-supply: phandle to the regulator device tree node >>>> "supply-name" examples are "vmmc", "vmmc_aux" etc >>>> -ti,non-removable: non-removable slot (like eMMC) >>>> +ti,non-removable: non-removable eMMC with always on vccq and configurable vcc >>> >>> Why this change? >>> >> Hi, >> >> earlier ti,non-removable was used for all eMMC and SDIO card, now it will >> be used only for eMMC with always on vccq and configurable vcc. > > Please expand the commit message to mention this. It wasn't clear why > adding support for a property meant modifying the description of > another. > Hi, Ok >> >>> What do "vccq" and "vcc" correspond to? The regulators are called "vmmc" >>> and "vmmc_aux"... >>> >> >> vccq and vcc are supply names of eMMC part > > The binding has vmmc-supply and vmmc_aux-supply. How do {vmmc,vmmc_aux} > and {vcc,vccq} relate? That should be clarified in the binding document, > something like: > > - vmmc-supply: phandle of the regulator for the VCC input > - vmmc_aux-supply: phandle of the regulator for the VCCQ input > It can be different for SD card, so will add vcc to vmmc mapping to ti,non-removable description. >> >>> Why is no mention of "non-removable" added, given that it's added to the >>> code? >> >> Because this file makes a reference to mmc.txt and the core properties described >> by mmc.txt are not added in ti-omap-hsmmc.txt > > There is room for confusion here. While "non-removable" is a generic > property, it would be good to contrast "non-removable" and > "ti,non-removable" in the binding as they imply different things. > >> >>> >>> Is one preferred over the other? That should be noted. >>> >>>> ti,needs-special-reset: Requires a special softreset sequence >>>> ti,needs-special-hs-handling: HSMMC IP needs special setting for handling High Speed >>>> dmas: List of DMA specifiers with the controller specific format >>>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c >>>> index 6ac63df..5992048 100644 >>>> --- a/drivers/mmc/host/omap_hsmmc.c >>>> +++ b/drivers/mmc/host/omap_hsmmc.c >>>> @@ -1738,6 +1738,9 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev) >>>> pdata->slots[0].switch_pin = cd_gpio; >>>> pdata->slots[0].gpio_wp = wp_gpio; >>>> >>>> + if (of_find_property(np, "non-removable", NULL)) { >>>> + pdata->slots[0].nonremovable = true; >>>> + } >>> >>> This wasn't mentioned in the binding, and it seems to have different >>> semantics to "ti,non-removable". Why is it different? >>> >> >> When ti,non-removable was added, Only OMAP platform that had eMMC was that on OMAP4 >> where power to eMMC cannot be switched off without sending CMD5 sleep command, >> so no_regulator_off_init was needed to get it detected during boot. >> >> Now start using generic non-removable for all removable cards which do not >> have such limitation. > > OK. I think this would be much clearer with something in the binding > contrasting the two properties. Thanks for comments, will add those info. > > Thanks, > Mark. > -- 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
diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt index 8c8908a..3b95719 100644 --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt @@ -17,7 +17,7 @@ Optional properties: ti,dual-volt: boolean, supports dual voltage cards <supply-name>-supply: phandle to the regulator device tree node "supply-name" examples are "vmmc", "vmmc_aux" etc -ti,non-removable: non-removable slot (like eMMC) +ti,non-removable: non-removable eMMC with always on vccq and configurable vcc ti,needs-special-reset: Requires a special softreset sequence ti,needs-special-hs-handling: HSMMC IP needs special setting for handling High Speed dmas: List of DMA specifiers with the controller specific format diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 6ac63df..5992048 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1738,6 +1738,9 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev) pdata->slots[0].switch_pin = cd_gpio; pdata->slots[0].gpio_wp = wp_gpio; + if (of_find_property(np, "non-removable", NULL)) { + pdata->slots[0].nonremovable = true; + } if (of_find_property(np, "ti,non-removable", NULL)) { pdata->slots[0].nonremovable = true; pdata->slots[0].no_regulator_off_init = true;