RE: [PATCH v5 1/2] scsi: ufs: Do not disable vccq in UFSHC driver

[Avri Altman]

> 
> Commit 60f0187031c0 ("disable vccq if it's not needed by UFS device")
> introduced a small power optimization as a driver quirk: ignore the
> vccq load specified in the UFSHC DT node when said host controller
> is connected to specific Flash chips (Samsung and Hynix currently).
> 
> Unfortunately, this optimization breaks UFS on systems where vccq
> powers not only the Flash chip, but the host controller as well,
> such as APQ8098 MEDIABOX or MTP8998:
> 
> [    3.929877] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04
> for idn 13 failed, index 0, err = -11
> [    5.433815] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04
> for idn 13 failed, index 0, err = -11
> [    6.937771] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04
> for idn 13 failed, index 0, err = -11
> [    6.937866] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr_retry: query
> attribute, idn 13, failed with error -11 after 3 retires
> [    6.946412] ufshcd-qcom 1da4000.ufshc: ufshcd_disable_auto_bkops:
> failed to enable exception event -11
> [    6.957972] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1587
> failed 3 retries
> [    6.967181] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1586
> failed 3 retries
> [    6.975025] ufshcd-qcom 1da4000.ufshc: ufshcd_get_max_pwr_mode:
> invalid max pwm tx gear read = 0
> [    6.982755] ufshcd-qcom 1da4000.ufshc: ufshcd_probe_hba: Failed getting
> max supported power mode
> [    8.505770] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag
> query for idn 3 failed, err = -11
> [   10.009807] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag
> query for idn 3 failed, err = -11
> [   11.513766] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag
> query for idn 3 failed, err = -11
> [   11.513861] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: query
> attribute, opcode 5, idn 3, failed with error -11 after 3 retires
> [   13.049807] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor:
> opcode 0x01 for idn 8 failed, index 0, err = -11
> [   14.553768] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor:
> opcode 0x01 for idn 8 failed, index 0, err = -11
> [   16.057767] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor:
> opcode 0x01 for idn 8 failed, index 0, err = -11
> [   16.057872] ufshcd-qcom 1da4000.ufshc: ufshcd_read_desc_param: Failed
> reading descriptor. desc_id 8, desc_index 0, param_offset 0, ret -11
> [   16.067109] ufshcd-qcom 1da4000.ufshc: ufshcd_init_icc_levels: Failed
> reading power descriptor.len = 98 ret = -11
> [   37.073787] ufshcd-qcom 1da4000.ufshc: link startup failed 1
> 
> In my opinion, the rationale for the original patch is questionable.
> If neither the UFSHC, nor the Flash chip, require any load from vccq,
> then that power rail should simply not be specified at all in the DT.
> 
> Working around that fact in the driver is detrimental, as evidenced
> by the failure to initialize the host controller on MSM8998.
> 
> Revert the original patch, and clean up loose ends in the next patch.
> 
> Relevant patches:
> 
> 60f0187031c05e04cbadffb62f557d0ff3564490
> c58ab7aab71e2c783087115f0ce1623c2fdcf0b2
> 46c1cf706076500cdcde3445be97233793eec7f1
> 
> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@xxxxxxx>
Acked-by: Avri Altman <avri.altman@xxxxxxx>