RE: [RFC V3 4/4] mmc: dw_mmc: exynos: configure SMU in exynos5420.

From: Seungwon Jeon <tgih.jun@samsung.com>
To: 'Tomasz Figa' <t.figa@samsung.com>
Cc: 'Yuvaraj Kumar' <yuvaraj.cd@gmail.com>,
	'Alim Akhtar' <alim.akhtar@gmail.com>,
	'linux-mmc' <linux-mmc@vger.kernel.org>,
	linux-samsung-soc@vger.kernel.org, 'Chris Ball' <cjb@laptop.org>,
	'Jaehoon Chung' <jh80.chung@samsung.com>,
	'Girish K S' <ks.giri@samsung.com>,
	'Alim Akhtar' <alim.akhtar@samsung.com>,
	'Yuvaraj Kumar C D' <yuvaraj.cd@samsung.com>,
	swarren@wwwdotorg.org, mark.rutland@arm.com,
	ian.campbell@citrix.com, rob.herring@calxeda.com,
	pawel.moll@arm.com, galak@codeaurora.org,
	devicetree@vger.kernel.org
Subject: RE: [RFC V3 4/4] mmc: dw_mmc: exynos: configure SMU in exynos5420.
Date: Fri, 30 Aug 2013 12:44:43 +0900	[thread overview]
Message-ID: <000a01cea533$43cfa580$cb6ef080$%jun@samsung.com> (raw)
In-Reply-To: <49209892.KLA1Q6dtil@amdc1227>

On Thu, August 29, 2013, Tomasz Figa wrote:
> Hi Seungwon,
> 
> On Thursday 29 of August 2013 19:36:28 Seungwon Jeon wrote:
> > Yuvaraj Kumar wrote:
> > > On Thu, Aug 29, 2013 at 3:14 PM, Seungwon Jeon <tgih.jun@samsung.com>
> wrote:
> > > > On Thu, August 29, 2013, Alim Akhtar wrote:
> > > >> Hi Seungwon,
> > > >>
> > > >> On Thu, Aug 29, 2013 at 1:53 PM, Seungwon Jeon
> <tgih.jun@samsung.com> wrote:
> > > >> > On Wed, August 28, 2013, Yuvaraj Kumar C D wrote:
> > > >> >> Exynos5420 Mobile Storage Host controller has Security Management
> > > >> >> Unit
> > > >> >> (SMU) for channel 0 and channel 1 (mainly for eMMC).This patch
> > > >> >> configures SMU for exynos5420.
> > > >> >>
> > > >> >> This patch is on top of the below patch by Doug Anderson.
> > > >> >> mmc: dw_mmc: Add exynos resume_noirq callback to clear WAKEUP_INT
> > > >> >>
> > > >> >> changes since V2:
> > > >> >>       1.Droppped the bypass-smu quirk.
> > > >> >>       2.Changed the subject line for this patch
> > > >> >>
> > > >> >>         add a quirk for SMU -> configure SMU in exynos5420
> > > >> >>
> > > >> >> changes since V1:
> > > >> >>       1.avoid code duplication by calling dw_mci_exynos_priv_init
> > > >> >>       in
> > > >> >>
> > > >> >>         resume path.
> > > >> >>
> > > >> >> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> > > >> >> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> > > >> >> ---
> > > >> >>
> > > >> >>  drivers/mmc/host/dw_mmc-exynos.c |   29
> > > >> >>  +++++++++++++++++++++++++++++
> > > >> >>  1 file changed, 29 insertions(+)
> > > >> >>
> > > >> >> diff --git a/drivers/mmc/host/dw_mmc-exynos.c
> > > >> >> b/drivers/mmc/host/dw_mmc-exynos.c index 19c845b..db28f10 100644
> > > >> >> --- a/drivers/mmc/host/dw_mmc-exynos.c
> > > >> >> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> > > >> >> @@ -35,6 +35,25 @@
> > > >> >>
> > > >> >>  #define EXYNOS4210_FIXED_CIU_CLK_DIV 2
> > > >> >>  #define EXYNOS4412_FIXED_CIU_CLK_DIV 4
> > > >> >>
> > > >> >> +/* Block number in eMMC */
> > > >> >> +#define DWMCI_BLOCK_NUM                      0xFFFFFFFF
> > > >> >> +
> > > >> >> +#define SDMMC_EMMCP_BASE             0x1000
> > > >> >> +#define SDMMC_MPSECURITY             (SDMMC_EMMCP_BASE + 0x0010)
> > > >> >> +#define SDMMC_MPSBEGIN0                      (SDMMC_EMMCP_BASE +
> > > >> >> 0x0200) +#define SDMMC_MPSEND0
> > > >> >> (SDMMC_EMMCP_BASE + 0x0204) +#define SDMMC_MPSCTRL0
> > > >> >>          (SDMMC_EMMCP_BASE + 0x020C) +
> > > >> >> +/* SMU control bits */
> > > >> >> +#define DWMCI_MPSCTRL_SECURE_READ_BIT                BIT(7)
> > > >> >> +#define DWMCI_MPSCTRL_SECURE_WRITE_BIT               BIT(6)
> > > >> >> +#define DWMCI_MPSCTRL_NON_SECURE_READ_BIT    BIT(5)
> > > >> >> +#define DWMCI_MPSCTRL_NON_SECURE_WRITE_BIT   BIT(4)
> > > >> >> +#define DWMCI_MPSCTRL_USE_FUSE_KEY           BIT(3)
> > > >> >> +#define DWMCI_MPSCTRL_ECB_MODE                       BIT(2)
> > > >> >> +#define DWMCI_MPSCTRL_ENCRYPTION             BIT(1)
> > > >> >> +#define DWMCI_MPSCTRL_VALID                  BIT(0)
> > > >> >> +
> > > >> >>
> > > >> >>  /* Variations in Exynos specific dw-mshc controller */
> > > >> >>  enum dw_mci_exynos_type {
> > > >> >>
> > > >> >>       DW_MCI_TYPE_EXYNOS4210,
> > > >> >>
> > > >> >> @@ -74,6 +93,15 @@ static int dw_mci_exynos_priv_init(struct
> > > >> >> dw_mci *host)> >> >>
> > > >> >>  {
> > > >> >>
> > > >> >>       struct dw_mci_exynos_priv_data *priv = host->priv;
> > > >> >>
> > > >> >> +     if (priv->ctrl_type == DW_MCI_TYPE_EXYNOS5420) {
> > > >> >> +             mci_writel(host, MPSBEGIN0, 0);
> > > >> >> +             mci_writel(host, MPSEND0, DWMCI_BLOCK_NUM);
> > > >> >> +             mci_writel(host, MPSCTRL0,
> > > >> >> DWMCI_MPSCTRL_SECURE_WRITE_BIT | +
> > > >> >> DWMCI_MPSCTRL_NON_SECURE_READ_BIT |
> > > >> >> +                     DWMCI_MPSCTRL_VALID |
> > > >> >> +                     DWMCI_MPSCTRL_NON_SECURE_WRITE_BIT);
> > > >> >
> > > >> > Yuvaraj,
> > > >> >
> > > >> > Just one thing to check.
> > > >> > ch#0 and #1 of three hosts are only valid for SMU control.
> > > >> > Did you consider #2 host?
> > > >> > It seems not.
> > > >>
> > > >> Only host#0 and host#1 has SMU (On exynos5420).
> > > >> Host #2 does not contain SMU.
> > > >
> > > > Let me clear it.
> > > > I mean that current change allows for ch2 to access registers related
> > > > to SMU, even though ch2 doesn't actually has SMU. It's not valid IO
> > > > area.>
> > > No,host#0 and host#1 are compatible with "samsung,exynos5420-dw-mshc"
> > > but host#2 is compatible with the "samsung,exynos5250-dw-mshc".
> > > Below is the DT patch posted in another thread.
> > > [1] [PATCH V4] ARM: dts: Add dwmmc DT nodes for exynos5420 SOC
> > > http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg21985.
> > > html I will resubmit the DT patch[1] with reg = <0x12220000 0x1000> for
> > > host#2.
> > Hmm. If we notice exynos5420's other newly added registers apart from
> > SMU, it is difficult to say exynos5420 is compatible with exynos5250
> > fully for #2. But because there is no use case for new registers, it
> > could be acceptable. Ok, we should be careful to configure DT file
> > though.
> 
> Could you elaborate a bit more on the differences in DW MMC IP between
> Exynos5250 and Exynos5420? If the differences are significant, then it
> might be necessary to use different compatible value, as it should not
> depend on any use case (or lack of).
Actually the reason is  that extended register of ch2 doesn't work rather than none of use case.
Ch2 is targeted for removable card type.

> 
> If the DW MMC version used in Exynos 5420 provides the whole set of
> functionality of the version used in Exynos 5250 and some extra extensions,
> then it can be called "compatible with Exynos 5250", but if you ever need
> to support those extra extensions, you will have to add a separate
> compatible string for it.
> 
> With regard to SMU that is present only on selected instances, you can as
> well create two compatible values for the DW MMC IP on Exynos 5420, e.g.
> "samsung,exynos5420-dw-mshc" and "samsung,exynos5420-dw-mshc-smu".
I think your suggestion is a nice thing. It makes sense.
And I missed one thing.
Apart from added register, Exynos5420 ch2 is designed for I/O of 1.2V voltage unlike Exynos5250.
So it is worthy to be distinguished.

Yuvaraj,
Could you apply Tomasz's suggestion ("samsung,exynos5420-dw-mshc" and "samsung,exynos5420-dw-mshc-smu") and resend it?

Thanks,
Seungwon Jeon

> 
> [Adding devicetree ML and DT maintainers on CC, as this is something they
> should be able to comment on.]
> 
> Best regards,
> Tomasz
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html