RE: [PATCH v4 5/5] arm64: dts: Add node for ufs exynos7

["Alim Akhtar" <alim.akhtar@samsung.com>]

Hi Pawel,

> -----Original Message-----
> From: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> Sent: 05 April 2020 01:56
> To: Alim Akhtar <alim.akhtar@samsung.com>; robh+dt@kernel.org;
> devicetree@vger.kernel.org; linux-scsi@vger.kernel.org
> Cc: krzk@kernel.org; avri.altman@wdc.com; martin.petersen@oracle.com;
> kwmad.kim@samsung.com; stanley.chu@mediatek.com;
> cang@codeaurora.org; linux-samsung-soc@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v4 5/5] arm64: dts: Add node for ufs exynos7
> 
> On Sat, 2020-04-04 at 21:33 +0200, Paweł Chmiel wrote:
> > On Sat, 2020-04-04 at 23:45 +0530, Alim Akhtar wrote:
> > Hi Alim,
> > > Hi Pawel,
> > >
> > > > -----Original Message-----
> > > > From: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> > > > Sent: 03 April 2020 22:22
> > > > To: Alim Akhtar <alim.akhtar@samsung.com>; robh+dt@kernel.org;
> > > > devicetree@vger.kernel.org; linux-scsi@vger.kernel.org
> > > > Cc: krzk@kernel.org; avri.altman@wdc.com;
> > > > martin.petersen@oracle.com; kwmad.kim@samsung.com;
> > > > stanley.chu@mediatek.com; cang@codeaurora.org;
> > > > linux-samsung-soc@vger.kernel.org; linux-arm-
> > > > kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> > > > Subject: Re: [PATCH v4 5/5] arm64: dts: Add node for ufs exynos7
> > > >
> > > > Hi Alim
> > > >
> > > > Looking at vendor sources, my device is using the same gpios for
> > > > urfs_rst_n and ufs_refclk_out like Espresso (with one difference -
> > > > ufs_rst_n shouldn't be pulled up).
> > > >
> > > > About regulators (it would be easier if dts would have all regulators).
> > > > It's also using s2mps15 as Espresso, but it vendor dts had only 8
> > > > (of
> > > > 10 possible bucks, one missing was for UFS) and 14 ldos (of 27
> > > > possible), where almost all rails are connected to something.
> > > >
> > > > I'm wondering how it's working on Espresso, because when adding
> > > > correct regulators for ufs (vccq = buck10 from s2mps15, always
> > > > enabled for testing plus vccq2 and vccq = two regulators enabled
> > > > by one gpio, enabled at boot by firmware), ufs wasn't still
> > > > working because it was then failing at defer probe (s2mps15 was
> > > > probed after ufs)
> > > >
> > > > [    0.962482] exynos-ufshc 15570000.ufs: ufshcd_get_vreg: vccq get
> > > > failed, err=-517
> > > >
> > > As I said, this is very specific to the board, on Espresso we have LDO12
> connected to UFS_RESETn.
> > > Either make all of them as always-on, or just disabled s2mps15
> > > (default voltage supply should be ok, unless bootloader on your
> > > board does have messed too much with PMIC)
> > >
> > > > After that boot would just stop/hang.
> > > >
> > > > After making a "dirty fix" by making s2mps15 regulator driver use
> > > > subsys_initcall (like in vendor sources) and ufs late_initcall (to
> > > > give it more time to setup and get it working and solve it later),
> > > > i had to mark following clocks as CLK_IGNORE_UNUSED to be able to
> > > > bring link up (it replicates setting done by vendor kernel, which
> > > > enables them on boot):
> > > > - "phyclk_ufs20_rx1_symbol_user"
> > > > - "phyclk_ufs20_rx0_symbol_user"
> > > > - "phyclk_ufs20_tx0_symbol_user"
> > > >
> > > Coming to these clocks, all these are supplied by default, my best
> > > guess is since you are using an actual product (S6 edge), they might have
> optimized for power saving And most likely all clock might be  gated initially. In
> my case all are set to default.
> > > I have attached a small change in the exynos7 dts and phy driver clock
> handling, please try this attached patch and let me know if this helps in removing
> some of your hacks.
> > > In the later SoCs these clocks are not in this form, so I didn't included in my
> current patch set, If this works for your, will add as an optional for
> exynos7/7420.
> > > I also assume you are using clk-exynos7.c and my posted ufs driver.
> > Yes, i'm using clk-exynos7 (and other exynos7 drivers/dts/etc).
> > It would be great if someone could say how exynos7 and exynos7420 are
> > similar. For now it looks like that only difference is that exynos7
> > has only 4 cores (a57) where 7420 has 4xa53 + 4xa57.
> > It would be very valuable information for me so i could know how much
> > i could reuse my device.
> > > > Now it's able to bring both device and link, but it fails at
> > > > ufshcd_uic_change_pwr_mode.
> > > >
> > > Can you please use the exact ufs and ufs-phy device node as in my patch?
> > With Your patch + removed my changes to clocks (removed fix for wrong
> > clock order in dts + removed CLK_IGNORE_UNUSED from symbol clocks in
> > clk-exynos7) it's finally able to detect my UFS device!!
> >

Wow, great to know that UFS device started working for you on S6.

> > (but of fails later...with constant error spam in kernel log).
> >
> > [    1.383481] exynos-ufshc 15570000.ufs: ufshcd_populate_vreg: Unable
> > to find vdd-hba-supply regulator, assuming enabled
> > [    1.390060] exynos-ufshc 15570000.ufs: ufshcd_populate_vreg: unable
> > to find vcc-max-microamp
> > [    1.398465] exynos-ufshc 15570000.ufs: ufshcd_populate_vreg: unable
> > to find vccq-max-microamp
> > [    1.406968] exynos-ufshc 15570000.ufs: ufshcd_populate_vreg: unable
> > to find vccq2-max-microamp
> > [    1.415569] exynos-ufshc 15570000.ufs: ufshcd_init_clocks: clk:
> > core_clk, rate: 100000000
> > [    1.423715] exynos-ufshc 15570000.ufs: ufshcd_init_clocks: clk:
> > sclk_unipro_main, rate: 167000000
> > [    1.432569] exynos-ufshc 15570000.ufs: __ufshcd_setup_clocks: clk:
> > core_clk enabled
> > [    1.440205] exynos-ufshc 15570000.ufs: __ufshcd_setup_clocks: clk:
> > sclk_unipro_main enabled
> > [    1.449613] scsi host0: ufshcd
> > [    1.452179] samsung-ufs-phy 15571800.ufs-phy: MPHY ref_clk_rate =
> > 26000000
> > [    1.458448] samsung-ufs-phy 15571800.ufs-phy: MPHY
> > ref_parent_clk_rate = 26000000
> > [    1.487288] exynos-ufshc 15570000.ufs: ufshcd_print_pwr_info:[RX,
> > TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE],
> rate
> > =
> > 0
> > [    2.025569] exynos-ufshc 15570000.ufs: dme-set: attr-id 0xd041 val
> > 0x1fff error code 1
> > [    2.025715] exynos-ufshc 15570000.ufs: dme-set: attr-id 0xd041 val
> > 0x1fff failed 0 retries
> > [    2.025880] exynos-ufshc 15570000.ufs: dme-set: attr-id 0xd042 val
> > 0xffff error code 1
> > [    2.027354] exynos-ufshc 15570000.ufs: dme-set: attr-id 0xd042 val
> > 0xffff failed 0 retries
> > [    2.035583] exynos-ufshc 15570000.ufs: dme-set: attr-id 0xd043 val
> > 0x7fff error code 1
> > [    2.043465] exynos-ufshc 15570000.ufs: dme-set: attr-id 0xd043 val
> > 0x7fff failed 0 retries
> > [    2.054049] exynos-ufshc 15570000.ufs: Power mode change 0 : Fast
> > series_B G_2 L_2
> > [    2.059261] exynos-ufshc 15570000.ufs: ufshcd_print_pwr_info:[RX,
> > TX]: gear=[2, 2], lane[2, 2], pwr[FAST MODE, FAST MODE], rate = 2
> > [    2.071307] exynos-ufshc 15570000.ufs: ufshcd_init_icc_levels:
> > setting icc_level 0x0
> > [    2.081624] exynos-ufshc 15570000.ufs: ufshcd_set_queue_depth:
> > activate tcq with queue depth 1
> > [    2.087576] scsi 0:0:0:49488: scsi_add_lun: correcting incorrect
> > peripheral device type 0x0 for W-LUN 0x            c150hN
> > [    2.098400] scsi 0:0:0:49488: Well-known LUN    SAMSUNG  KLUBG4G1BD-
> > E0B1  0200 PQ: 0 ANSI: 6
> > [    2.107585] exynos-ufshc 15570000.ufs: ufshcd_set_queue_depth:
> > activate tcq with queue depth 16
> > [    2.115588] scsi 0:0:0:49476: scsi_add_lun: correcting incorrect
> > peripheral device type 0x0 for W-LUN 0x            c144hN
> > [    2.126519] scsi 0:0:0:49476: Well-known LUN    SAMSUNG  KLUBG4G1BD-
> > E0B1  0200 PQ: 0 ANSI: 6
> > [    2.135534] exynos-ufshc 15570000.ufs: ufshcd_set_queue_depth:
> > activate tcq with queue depth 1
> > [    2.143612] scsi 0:0:0:49456: scsi_add_lun: correcting incorrect
> > peripheral device type 0x0 for W-LUN 0x            c130hN
> > [    2.154543] scsi 0:0:0:49456: Well-known LUN    SAMSUNG  KLUBG4G1BD-
> > E0B1  0200 PQ: 0 ANSI: 6
> > [    2.163597] exynos-ufshc 15570000.ufs: ufshcd_set_queue_depth:
> > activate tcq with queue depth 16
> > [    2.171721] scsi 0:0:0:0: Direct-Access     SAMSUNG  KLUBG4G1BD-
> > E0B1  0200 PQ: 0 ANSI: 6
> > [    2.180352] exynos-ufshc 15570000.ufs: OCS error from controller = 7
> > for tag 0
> > [    2.186921] host_regs: 00000000: 0383ff0f 00000000 00000200 00000000
> > [    2.193230] host_regs: 00000010: 00000101 00007fce 00000c96 00000000
> > [    2.199565] host_regs: 00000020: 00000000 00030e75 00000000 00000000
> > [    2.205899] host_regs: 00000030: 0000010f 00000000 80000010 00000000
> > [    2.212234] host_regs: 00000040: 00000000 00000000 00000000 00000000
> > [    2.218568] host_regs: 00000050: f8d64000 00000000 00000000 00000000
> > [    2.224903] host_regs: 00000060: 00000001 00000000 00000000 00000000
> > [    2.231237] host_regs: 00000070: f8da2000 00000000 00000000 00000000
> > [    2.237572] host_regs: 00000080: 00000001 00000000 00000000 00000000
> > [    2.243907] host_regs: 00000090: 00000002 95190000 00000000 00000000
> > [    2.250242] exynos-ufshc 15570000.ufs: hba->ufs_version = 0x200,
> > hba->capabilities = 0x383ff0f
> >
> > Full bootlog
> > https://protect2.fireeye.com/url?k=edbae146-b069b8f8-edbb6a09-0cc47a31
> > ba82-
> 8b13b1e4caed34d7&q=1&u=https%3A%2F%2Fgist.github.com%2FPabloPL%2F
> > 0bcb24492f4ab6e9703c2a4ea20ceb18 kernel source:
> > https://protect2.fireeye.com/url?k=75038dec-28d0d452-750206a3-0cc47a31
> > ba82-
> 4c366bec6fc01e64&q=1&u=https%3A%2F%2Fgithub.com%2FPabloPL%2Flinux
> > %2Ftree%2Fufs-mainline dts file: exynos7-zeroflt.dts (it should be
> > zerolt, but will be fixed/changed later).
> 
> Actually, after waiting enough time (about 15 or even more sec of that error
> "spam"), was able to mount partitions and manipulate files there.
> 
You need below patch and  a change in the ufs driver:
https://www.spinics.net/lists/linux-scsi/msg138501.html

And

diff --git a/drivers/scsi/ufs/ufs-exynos.c b/drivers/scsi/ufs/ufs-exynos.c
index ce2c3d674e4b..c6332deff03a 100644
--- a/drivers/scsi/ufs/ufs-exynos.c
+++ b/drivers/scsi/ufs/ufs-exynos.c
@@ -1359,7 +1359,8 @@ struct exynos_ufs_drv_data exynos_ufs_drvs = {
        .quirks                 = UFSHCD_QUIRK_PRDT_BYTE_GRAN |
                                  UFSHCI_QUIRK_BROKEN_REQ_LIST_CLR |
                                  UFSHCI_QUIRK_BROKEN_HCE |
-                                 UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR,
+                                 UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR |
+                                 UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR,
        .opts                   = EXYNOS_UFS_OPT_HAS_APB_CLK_CTRL |
                                  EXYNOS_UFS_OPT_BROKEN_AUTO_CLK_CTRL |
                                  EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX,

> So for me the only issue to solve are defered probe when regulators are not yet
> found (for example when pmic is probed after ufs) and not sure what about that
> errors (despite working ufs).
> 
The error will go away with the above changes, about regulators, you need to figure it out, as I am not aware of Galaxy S6 PMIC schemes.
I also seek your Tested-by tag on these patches .

> Thanks for all

Thanks for helping in testing.
> >
> > Thanks