Re: [PATCH] mmc: dt: Add 'broken-cd' DT binding
From: Chris Ball <hidden>
Date: 2012-08-21 14:48:43
Also in:
linux-mmc, linux-samsung-soc
Hi, adding Shawn and Wolfram, On Tue, Aug 21 2012, Arnd Bergmann wrote:
On Tuesday 21 August 2012, Chris Ball wrote:quoted
How about this? broken-cd: No CD available, use polling. cd-gpios: The CD pin on the host is working and brought out to a GPIO. external-cd-gpios: The CD pin on the host is broken, but there's an independent external GPIO available. (Each host should only have one of these properties.)The fsl-imx-esdhc.txt binding already has all three cases, but describes them differently: fsl,cd-internal: when present, the CD pin on the host is working cd-gpios: when present, contains the gpio line that CD is connected to If both are absent, it has to use polling.
Aside: the bindings do not match the code. The bindings document says
to use "fsl,cd-internal", and imx51-babbage.dts does so -- but the code
doesn't check for "fsl,cd-internal", it checks for "fsl,cd-controller":
if (of_get_property(np, "fsl,cd-controller", NULL))
boarddata->cd_type = ESDHC_CD_CONTROLLER;
The same confusion is present for fsl,wp-{controller,internal}.
Ignoring that, these bindings are similar, but not the same -- imx-esdhc
only allows one of the cd_type cases to be true, so you either have
cd-internal or you have cd-gpios:
if (of_get_property(np, "fsl,cd-controller", NULL))
boarddata->cd_type = ESDHC_CD_CONTROLLER;
boarddata->cd_gpio = of_get_named_gpio(np, "cd-gpios", 0);
if (gpio_is_valid(boarddata->cd_gpio))
boarddata->cd_type = ESDHC_CD_GPIO;
This differs from Thomas's binding -- he wants a way to say "the cd-gpio
mentioned in cd-gpios is [internal/external] to the card's CD pin".
Rob Herring said:This makes the most sense to me. However, I prefer broken-cd over cd-internal. The binding should add properties for exceptions, not SDHCI spec compliant implementations.
Agreed, I was going to say the same thing. Putting it all together, it
sounds like we want:
no extra properties: the CD pin on the host just works.
broken-cd: the CD pin on the host is broken; use polling.
cd-gpios: the GPIO listed is the CD pin on the host being
brought out directly to a GPIO.
cd-external: when used with cd-gpios, specifies that the GPIO
in cd-gpios is external to the CD pin on the host.
cd-gpios and cd-external can be present on the same node. if broken-cd
is present, it must be the only one of these nodes used.
Shawn, I guess I'll leave it up to you on whether/when you'd like to
move away from the "fsl," properties to the new common ones?
If this looks okay to everyone, I'll send a patch soon.
Thanks!
- Chris.
--
Chris Ball [off-list ref] <http://printf.net/>
One Laptop Per Child