On Mon, Sep 2, 2013 at 5:51 PM, Tomasz Figa <t.figa@xxxxxxxxxxx> wrote:
> On Monday 02 of September 2013 16:45:41 Yuvaraj Kumar wrote:
>> On Fri, Aug 30, 2013 at 12:29 PM, Yuvaraj Kumar <yuvaraj.cd@xxxxxxxxx>
> wrote:
>> > On Wed, Aug 28, 2013 at 5:52 PM, Tomasz Figa <t.figa@xxxxxxxxxxx>
> wrote:
>> >> Hi Yuvaraj,
>> >>
>> >> On Wednesday 28 of August 2013 17:33:06 Yuvaraj Kumar C D wrote:
>> >>> This patch adds the device tree node entries for exynos5420 SOC.
>> >>> Exynos5420 has a different version of DWMMC controller,so a new
>> >>> compatible string is used to distinguish it from the prior SOC's.
>> >>>
>> >>> This patch depends on
>> >>>
>> >>> mmc: dw_mmc: exynos: Add a new compatible string for exynos5420
>> >>>
>> >>> changes since v4:
>> >>> 1.Droppped the bypass-smu binding property.
>> >>> 2.Used compatible string "samsung,exynos5250-dw-mshc"
>> >>>
>> >>> for controller instance which does not have SMU.
>> >>>
>> >>> changes since V3:
>> >>> 1.change fifo-depth size from 0x80 to 0x40
>> >>> 2.Move the below properties
>> >>>
>> >>> a.card-detect-delay
>> >>> b.samsung,dw-mshc-ciu-div
>> >>> c.samsung,dw-mshc-sdr-timing
>> >>> d.samsung,dw-mshc-ddr-timing
>> >>>
>> >>> from SOC dts to board dts file as suggested by Doug Anderson
>> >>>
>> >>> changes since V2:
>> >>> 1.dropped num-slots property from node as its not required
>> >>>
>> >>> if number of card slots available is 1.
>> >>>
>> >>> 2.Move the below properties
>> >>>
>> >>> a.fifo-depth
>> >>> b.card-detect-delay
>> >>> c.samsung,dw-mshc-ciu-div
>> >>> d.samsung,dw-mshc-sdr-timing
>> >>> e.samsung,dw-mshc-ddr-timing
>> >>>
>> >>> from board dts to SOC dts,as these are not board specific
>> >>> properties.
>> >>>
>> >>> 3.Updated the binding document exynos-dw-mshc.txt.
>> >>>
>> >>> changes since V1:
>> >>> 1.disable node by status = disabled in SOC file
>> >>> 2.enable node by status = okay in board specific file
>> >>>
>> >>> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@xxxxxxxxxxx>
>> >>> ---
>> >>>
>> >>> .../devicetree/bindings/mmc/exynos-dw-mshc.txt | 2 +
>> >>> arch/arm/boot/dts/exynos5420-smdk5420.dts | 33
>> >>>
>> >>> +++++++++++++++++ arch/arm/boot/dts/exynos5420.dtsi
>> >>> |
>> >>> 39 ++++++++++++++++++++ 3 files changed, 74 insertions(+)
>> >>
>> >> This patch looks good to me now, except some minor comments below.
>> >>
>> >>> diff --git a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>> >>> b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt index
>> >>> 6d1c098..84cd56f 100644
>> >>> --- a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>> >>> +++ b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>> >>>
>> >>> @@ -16,6 +16,8 @@ Required Properties:
>> >>> specific extensions.
>> >>>
>> >>> - "samsung,exynos5250-dw-mshc": for controllers with Samsung
>> >>> Exynos5250
>> >>>
>> >>> specific extensions.
>> >>> + - "samsung,exynos5420-dw-mshc": for controllers with Samsung
>> >>> Exynos5420 + specific extensions.
>> >>>
>> >>> * samsung,dw-mshc-ciu-div: Specifies the divider value for the card
>> >>>
>> >>> interface unit (ciu) clock. This property is applicable only for
>> >>> Exynos5
>> >>> SoC's and diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts
>> >>> b/arch/arm/boot/dts/exynos5420-smdk5420.dts index bafba25..3ce5c97
>> >>> 100644
>> >>> --- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
>> >>> +++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
>> >>> @@ -31,6 +31,39 @@
>> >>>
>> >>> };
>> >>>
>> >>> };
>> >>>
>> >>> + dwmmc0@12200000 {
>> >>
>> >> Node name should not contain instance index. Instead, according to
>> >> ePAPR
>> >> recommendation, I would opt for a generic name such as mmc, mshc or
>> >> sdio- host. mshc is already used on Exynos 4 so it might be a good
>> >> idea to use it for consistency.
>> >
>> > If we change node name's to mshc, debug statements are look like below:
>> > dwmmc_exynos 12200000.mshc: num-slots property not found, assuming 1
>> > slot is available
>> > dwmmc_exynos 12200000.mshc: no vmmc regulator found: -19
>> > dwmmc_exynos 12200000.mshc: Using internal DMA controller.
>> > dwmmc_exynos 12200000.mshc: Version ID is 250a
>> > dwmmc_exynos 12200000.mshc: DW MMC controller at irq 107, 64 bit host
>> > data width, 64 deep fifo
>> >
>> > "dwmmc_exynos" is platform driver name and "12200000.mshc" from dt
>> > node.Is'nt it look like bit confusing?
>
> Well, device tree should be written in an OS-agnostic way, so it's not a
> valid reason to use node names not complying to the recommendations.
>
> Also, I don't see anything wrong in this name. There is a lot of devices
> named this way, like 13920000.spi, 139d0000.pwm or 13800000.serial.
>
> Looking at other dts(i) files around the kernel, mmc is the most commonly
> used node name for MMC controllers (after sdhci, but it's used for the
> whole class of controllers compatible with standard SDHCI).
>
> So, for me, just go with "mmc" and it should be fine.
I have renamed the node from "mshc" to "mmc" in next version and posted
[PATCH V7] ARM: dts: Add dwmmc DT nodes for exynos5420 SOC
>
> Best regards,
> Tomasz
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
