Re: [PATCH v5 05/12] dt: bindings: Add bindings for Marvell Xenon SD Host Controller

Ulf Hansson
2017-01-26T10:49:52Z

On 11 January 2017 at 18:19, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> From: Hu Ziji <huziji@marvell.com>
>
> Marvell Xenon SDHC can support eMMC/SD/SDIO.
> Add Xenon-specific properties.
> Also add properties for Xenon PHY setting.
>
> Signed-off-by: Hu Ziji <huziji@marvell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

An overall comment. Please run a spell-checking as I noticed some
simple errors that needs to be fixed.

Optional Properties:
> +- mmccard:
> + mmccard child node must be provided when current SDHC is for eMMC.
> + Xenon SDHC often can support both SD and eMMC. This child node indicates that
> + current SDHC is for eMMC card. Thus Xenon eMMC specific configuration and
> + operations can be enabled prior to eMMC init sequence.
> + Please refer to Documentation/devicetree/bindings/mmc/mmc-card.txt.
> + This child node should not be set if current Xenon SDHC is for SD/SDIO.
> +
> +- bus-width:
> + When 8-bit data bus width is in use for eMMC, this property should be
> + explicitly provided and set as 8.
> + It is optional when data bus width is 4-bit or 1-bit.
> +
> +- mmc-ddr-1_8v:
> + Select this property when eMMC HS DDR is supported on SDHC side.
> +
> +- mmc-hs400-1_8v:
> + Select this property when eMMC HS400 is supported on SDHC side.

All the above properties is already specified as common mmc bindings.
Let's not do that again, but instead just refer to mmc.txt as how
other documentations looks like.

> +
> +- no-1-8-v:
> + Select this property when 1.8V signaling voltage supply is unavailable.
> + When this property is enabled, both mmc-ddr-1_8v and mmc-hs400-1_8v should be
> + cleared.

Please don't use this sdhci property, unless really needed.

Perhaps you can elaborate exactly why it makes sense for your case. Or
perhaps we already discussed this, in either case, please re-fresh my
mind.

[...]

Kind regards
Uffe