Re: [PATCH] mmc: dt: Add 'broken-cd' DT binding

[Mitch Bradley <wmb-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org>]

Hi Thomas,

On 8/22/2012 2:04 AM, Thomas Abraham wrote:
> On 22 August 2012 16:38, Chris Ball <cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org> wrote:
>> Hi Thomas,
>>
>> On Wed, Aug 22 2012, Thomas Abraham wrote:
>>>> This matches Mitch's last suggestion exactly -- I think we're all agreed
>>>> on these properties now.  The only remaining question is how to handle
>>>> the pinctrl for CD in Thomas's case.
>>>
>>> Hi Chris,
>>>
>>> For sdhci-s3c driver, the 'broken-cd' and 'cd-gpios' bindings are
>>> sufficient. But, are drivers free to use implementation specific
>>> behavior when using these bindings. Or do we strictly adhere to the
>>> table which Shawn has listed. For sdhci-s3c driver, I would like to
>>> deviate from the "implication" column listed above.
>>
>> I'd rather you use the implications described above for the names
>> described above; there's not much point in defining named behaviors
>> across the subsystem if they're actually different in each driver.
> 
> Ok.
> 
>>
>> But I think you can still use driver-internal properties that change the
>> interpretation of those names in a documented way.  If the meaning of
>> cd-gpios is modified when coupled with your "samsung,sdhci-cd-external",
>> that's okay with me.  That should cover all the cases you need until you
>> migrate to using pinctrl, right?  So, you could immediately support:
>>
>> none          -> currently "samsung,sdhci-cd-internal"
>> broken-cd     -> currently "samsung,sdhci-cd-none"
>> cd-gpios      -> currently "samsung,sdhci-cd-gpios"
>> non-removable -> currently "samsung,sdhci-cd-permanent"
>> cd-gpios + samsung,sdhci-cd-external -> currently "samsung,sdhci-cd-external"
>>
>> How does that sound?
> 
> Not quite there yet. It is not possible to leave out cd-gpios for
> "samsung,sdhci-cd-internal" since the current Samsung pinmux
> configuration piggybacks on gpio dt binding.


Sorry to interject on a topic that seems to have already been decided,
but I'm confused by one thing and would like clarification.  I
understand that you need to use a GPIO-style specifier as a surrogate
for a pinmux specification - that much is clear.  What is not clear is
why it's necessary to (ab)use the name "cd-gpios" for it.

Why not use a different property name, e.g. "samsung,cd-pinmux-gpio =
<gpio-specifier>" for the "cd-gpios + samsung,sdhci-cd-internal" case?
Then both "samsung,sdhci-cdi-internal" and "samsung,sdhci-cd-external"
could go away.  There would only be one system-dependent property
"samsung,cd-pinmux-gpio" whose name would make it clear that it's
conflating pinmuxing and gpios.

I think the scheme I propose would be clearer, less likely to confuse
other people who try to use the driver as a model, require less
hand-waving in the documentation, and easier to change to a proper
pinmuxing scheme should that become available later.


 Can we use the following
> instead.
> 
> none               -> currently "samsung,sdhci-cd-none"
> broken-cd        -> currently "samsung,sdhci-cd-none"
> cd-gpios          -> currently "samsung,sdhci-cd-gpios"
> non-removable -> currently "samsung,sdhci-cd-permanent"
> cd-gpios + samsung,sdhci-cd-internal -> currently "samsung,sdhci-cd-internal"
> 
> Thanks Chris.
> 
> Regards,
> Thomas.
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>