On 08/02/19 8:29 PM, Jeffrey Hugo wrote: > On 2/8/2019 2:09 AM, Alim Akhtar wrote: >> Hi Jeffrey, >> >> On 07/02/19 8:22 PM, Jeffrey Hugo wrote: >>> On 2/7/2019 1:50 AM, Alim Akhtar wrote: >>>> Hi Marc, >>>> >>>> On 06/02/19 9:22 PM, Marc Gonzalez wrote: >>>>> On 06/02/2019 16:27, Alim Akhtar wrote: >>>>> >>>>>> On 06/02/19 8:29 PM, Marc Gonzalez wrote: >>>>>> >>>>>>> [ 2.405734] regulator_disable: ENTER vdd_l26 >>>>>>> [ 2.405958] regulator_disable: EXIT vdd_l26 >>>>>>> [ 2.406032] regulator_set_load: vdd_l26 = 0 uA >>>>>>> [ 3.930447] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode >>>>>>> 0x04 for idn 13 failed, index 0, err = -11 >>>>>>> [ 5.434358] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode >>>>>>> 0x04 for idn 13 failed, index 0, err = -11 >>>>>>> [ 6.938318] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode >>>>>>> 0x04 for idn 13 failed, index 0, err = -11 >>>>>>> [ 6.938414] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr_retry: >>>>>>> query attribute, idn 13, failed with error -11 after 3 retires >>>>>>> [ 6.946959] ufshcd-qcom 1da4000.ufshc: >>>>>>> ufshcd_disable_auto_bkops: failed to enable exception event -11 >>>>>>> [ 6.958523] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id >>>>>>> 0x1587 failed 3 retries >>>>>>> [ 6.967730] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id >>>>>>> 0x1586 failed 3 retries >>>>>>> [ 6.975576] ufshcd-qcom 1da4000.ufshc: ufshcd_get_max_pwr_mode: >>>>>>> invalid max pwm tx gear read = 0 >>>>>>> [ 6.983306] ufshcd-qcom 1da4000.ufshc: ufshcd_probe_hba: Failed >>>>>>> getting max supported power mode >>>>>>> [ 8.506314] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: >>>>>>> Sending flag query for idn 3 failed, err = -11 >>>>>>> [ 10.010352] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: >>>>>>> Sending flag query for idn 3 failed, err = -11 >>>>>>> [ 11.514313] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: >>>>>>> Sending flag query for idn 3 failed, err = -11 >>>>>>> [ 11.514412] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: >>>>>>> query attribute, opcode 5, idn 3, failed with error -11 after 3 >>>>>>> retires >>>>>>> [ 13.050354] ufshcd-qcom 1da4000.ufshc: >>>>>>> __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, >>>>>>> err = -11 >>>>>>> [ 14.554313] ufshcd-qcom 1da4000.ufshc: >>>>>>> __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, >>>>>>> err = -11 >>>>>>> [ 16.058313] ufshcd-qcom 1da4000.ufshc: >>>>>>> __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, >>>>>>> err = -11 >>>>>>> [ 16.058421] ufshcd-qcom 1da4000.ufshc: ufshcd_read_desc_param: >>>>>>> Failed reading descriptor. desc_id 8, desc_index 0, param_offset 0, >>>>>>> ret -11 >>>>>>> [ 16.067654] ufshcd-qcom 1da4000.ufshc: ufshcd_init_icc_levels: >>>>>>> Failed reading power descriptor.len = 98 ret = -11 >>>>>>> [ 37.074334] ufshcd-qcom 1da4000.ufshc: link startup failed 1 >>>>>> >>>>>> Can you check if your UFS device RESET_N is asserted correctly. It >>>>>> might >>>>>> be connected to some regulator and may be you can try keeping that >>>>>> regulator as "regulator-always-on" from your DT node. >>>>> >>>>> How do I check RESET_N? In software or hardware? >>>>> >>>> RST_N is the reset logic for UFS device core logic and it is input to >>>> the device from UFS host controller.So, in your platform please >>>> check if >>>> this line somehow connected to (pulled up) a PMIC supply. If that is >>>> the >>>> case, please keep that regulator ON and see if this issue is resolved. >>> >>> The reset line is routed though the global clock controller (GCC), and >>> must be explicitly asserted within the GCC to trigger a reset. As far >>> as I am aware, Linux is not touching this. >>> >>> Additionally, I fail to see how if this was a reset issue, reverting >>> 60f0187031c0 would have any impact (which doing so addresses our issue) >>> >> OK, that's again implementation dependent and your platform used that >> way. My point was to make sure that reset part is ok, if reset/power is >> not proper to the UFS device core logic this kind of issues comes. > > We are following the Hardware Programming Guide written by the platform > designers with regard to UFS, including the reset logic. I really don't > think the reset logic is an issue here. > >> >>>>> Do you think it is not a good idea to revert >>>>> 60f0187031c05e04cbadffb62f557d0ff3564490 ? >>>>> >>>> Please hold on till we understand the real cause of this issue. Or we >>>> have a consensuses for reverting the said commit. >>>> Thanks! >>> >>> Did you see https://lkml.org/lkml/2019/2/5/659 where I indicated VCCQ >>> powers components within the host controller, and by not setting load on >>> the regulator properly, we are likely undervolting those components due >>> to the current draw? >>> >> In theory may be true. But looks like we dont have a solid evidence yet >> (correct me if I am wrong or misunderstood anything here > The evidence seems simple. We have properly described in DT all the > regulators that are consumed by the UFS host controller, and by > extension, the UFS storage chip as well. > > By default, with no kernel changes, UFS does not work. > > Marc and I debugged the issue, and found that the VCCQ regulator was not > being handled properly, and reverting the change we are discussing fixes > the the VCCQ regulator issue, and as a result UFS works. > Ok, fair, before we revert this patch, Marc can you try below patch, or let me know if you have already tried this and share your result/observation: diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi index 50e9033aa7f6..b08e8d1ea0f3 100644 --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi @@ -212,6 +212,7 @@ vreg_l26a_1p2: l26 { regulator-min-microvolt = <1200000>; regulator-max-microvolt = <1200000>; + regulator-always-on; }; vreg_l28_3p0: l28 { regulator-min-microvolt = <3008000>; --- I believe "vreg_l26a_1p2" is supply for ufs's vccq. check the result of this patch /wo reverting 60f0187031c05e04cbadffb62f557d0ff3564490 > Again, despite the fact that we may have a Samsung UFS storage chip, > which triggers the quirk, the UFS host controller which talks to that > chip requires VCCQ, therefore this quirk breaks us. > >> So that means its some short of hardware/board quirk, right? > > No > >> Can you please recheck the schematic and see what Bjorn is telling >> (about having right entries in the DT for regulator) resolve your issue? > > Already done. The schematic defines vcc, vccq, and vccq2. All of those > are listed in DT, and have been since Marc and I have been trying to > utilize UFS. > >> >> Marc, Can you disabled pmic on that board (hope your board boots with >> default PMIC supply) and see if this issue still occurs? > > The PMIC is required the boot the board. I doubt the board will be > functional with the PMIC disabled. > >> I am just trying to understand and see what is the real cause. > > Our analysis is that VCCQ is required and > 60f0187031c05e04cbadffb62f557d0ff3564490 prevents the proper > configuration of VCCQ, thus a required resource (VCCQ) is not in the > proper state. > Not in proper state or vccq regulator is disabled? >> >> @Yaniv Gardi, will you be able to comment on reason for adding >> 60f0187031c05e04cbadffb62f557d0ff3564490 (any issue faced)? >> > >
