Re: [PATCH v3 4/4] mmc: sdhci-esdhc-imx: add device tree probe support

On Thu, Jul 14, 2011 at 12:52 AM, Anton Vorontsov
[off-list ref] wrote:

Hi,

On Wed, Jul 06, 2011 at 03:05:18PM -0600, Grant Likely wrote:

quoted

On Thu, Jul 07, 2011 at 12:47:50AM +0800, Shawn Guo wrote:

quoted

The patch adds device tree probe support for sdhci-esdhc-imx driver.

Signed-off-by: Shawn Guo <redacted>
Cc: Wolfram Sang <redacted>
Cc: Chris Ball <redacted>
Cc: Grant Likely <redacted>

Acked-by: Grant Likely <redacted>

[...]

quoted

quoted

+Optional properties:
+- fsl,card-wired : Indicate the card is wired to host permanently
+- fsl,cd-internal : Indicate to use controller internal card detection
+- fsl,wp-internal : Indicate to use controller internal write protection
+- cd-gpios : Specify GPIOs for card detection
+- wp-gpios : Specify GPIOs for write protection

[...]

quoted

quoted

+   cd-gpios = <&gpio0 6 0>; /* GPIO1_6 */
+   wp-gpios = <&gpio0 5 0>; /* GPIO1_5 */

Is there any particular reason why we started using named GPIOs
in the device tree? To me, that's quite drastic change... should
we start using named regs and interrupts as well? Personally, I
don't think that the idea is great, as now you don't know where
to expect memory resources, interrupt resources and, eventually
GPIO resources.

No, there are no plans to move to named irqs or regs.  irq and reg
resources tend to be a lot more regular than gpios.  It's not a
drastic change though because existing bindings remain unaffected, and
new bindings can still use unnamed gpios if that is more appropriate.

Just a few disadvantages off the top of my head:

- You can't statically validate your device tree for correctness.
 Today dtc checks #{address,size}-cells sanity for 'regs' and
 'ranges';

We can. The gpio binding was extended to be in the form
[<name>-]gpios.  Any property with the string "gpios" at the end can
be statically validated.

- You can't automatically convert named resources into 'struct
 resource *', as we do for platform devices now;

Only for irqs and regs.  gpios have never been automatically loaded
into resources.

- Any pros for using named resources in the device tree? I don't
 see any.

Human readability.  To know exactly what a gpio is intended to be used
for.  Particularly for the case where a device might not use all the
gpios that it could use.  Yes, the gpios property can have 'holes' in
it, but the observation was made by several people that it is easy to
get wrong.  I for one thing the concern was well justified.

So, I suggest to at least discuss this stuff a little bit more
before polluting device trees with dubious ideas.

It was discussed on list quite a while ago.

p.s.

As for this particular patch, I really don't see why we should
use named GPIOs. For consistency, I'd suggest to reuse bindings
from Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt.

Plus, fsl,cd-internal and fsl,wp-internal is not needed: either
you specify GPIOs or not. That already signals whether driver
should use internal detection (i.e. 'present' register) or
external (i.e. GPIO).

wp-internal and cd-internal differentiates between using the internal
functionality and not having wp/cd at all.

And also, why {cd,wp}-gpioS (plural)?

For consistency.  Doing it that way means that the plural "gpios" is
always the suffix for both the "gpios" and "<name>-gpios" use cases.

--
Anton Vorontsov
Email: cbouatmailru@gmail.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.