Re: [PATCH] sdhci-fujitsu: add support for setting the CMD_DAT_DELAY attribute

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 2 November 2017 at 10:48, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 2 November 2017 at 01:12, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 2 November 2017 at 00:08, Rob Herring <robh@kernel.org> wrote:
>>> On Wed, Nov 01, 2017 at 12:21:32AM +0000, Ard Biesheuvel wrote:
>>>> The Socionext SynQuacer SoC inherits this IP from Fujitsu, but
>>>> requires the F_SDH30_CMD_DAT_DELAY bit to be set in the
>>>> F_SDH30_ESD_CONTROL control register.
>>>>
>>>> So let's add an optional property to this device's binding, and
>>>> set the attribute if it is present in the DT node.
>>>>
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> ---
>>>>  Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt |  2 ++
>>>>  drivers/mmc/host/sdhci_f_sdh30.c                        | 18 +++++++++++++++++-
>>>>  2 files changed, 19 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
>>>> index de2c53cff4f1..9ad02f743ad0 100644
>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
>>>> @@ -15,6 +15,8 @@ Required properties:
>>>>  Optional properties:
>>>>  - vqmmc-supply: phandle to the regulator device tree node, mentioned
>>>>    as the VCCQ/VDD_IO supply in the eMMC/SD specs.
>>>> +- cmd-dat-delay-select: boolean property indicating that this host requires
>>>> +  the CMD_DAT_DELAY control to be enabled.
>>>
>>> Needs a vendor prefix unless this is a standard SDHCI bit (in which
>>> case it should be documented in a common spot).
>>>
>>
>> I suspect this is a non-standard thing, so I'll add the fujitsu prefix. Ulf?
>
> This is specific to the Fujitsu variant.
>
> However, what puzzles me is why this bit is needed for the Socionext
> SynQuacer SoC, but not for other SoCs that uses this IP. That seems
> weird (probably wrong).
>
> Moreover, the F_SDH30_CMD_DAT_DELAY register define is already there,
> so I would rather suspect that it's something that did got fully
> implemented. In other words, I don't think you need a new DT binding,
> but rather try to implement it generically for the sdhci-f_sdh30
> driver. Do you think that will work?
>

Do you mean set it unconditionally? That works for me, but I have no
way of testing whether it still works on other SoCs that use this IP.