Re: [PATCH 1/6] mmc: omap_hsmmc: start using generic non-removable DT binding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxx>
> >>
> >> add generic "non-removable" binding support for omap_hsmmc
> >>
> >> Signed-off-by: Sekhar Nori <nsekhar@xxxxxx>
> >> Signed-off-by: Balaji T K <balajitk@xxxxxx>
> >> ---
> >>   .../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-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux