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.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
