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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <SN6PR04MB4925E5C25FA0E4BDC05D3660FC7B0@SN6PR04MB4925.namprd04.prod.outlook.com>
Date:   Tue, 26 Feb 2019 09:05:22 +0000
From:   Avri Altman <Avri.Altman@....com>
To:     Marc Gonzalez <marc.w.gonzalez@...e.fr>,
        SCSI <linux-scsi@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
CC:     Jeffrey Hugo <jhugo@...eaurora.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Evan Green <evgreen@...omium.org>,
        Douglas Anderson <dianders@...omium.org>,
        Alim Akhtar <alim.akhtar@...sung.com>,
        Pedro Sousa <pedrom.sousa@...opsys.com>,
        Joao Pinto <jpinto@...opsys.com>,
        Mark Brown <broonie@...nel.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Bart Van Assche <bart.vanassche@....com>,
        Stanislav Nijnikov <Stanislav.Nijnikov@....com>,
        Alex Lemberg <Alex.Lemberg@....com>,
        Ohad Sharabi <ohad.sharabi@....com>,
        Venkat Gopalakrishnan <venkatg@...eaurora.org>,
        Subhash Jadavani <subhashj@...eaurora.org>,
        Yaniv Gardi <ygardi@...eaurora.org>,
        Gilad Broner <gbroner@...eaurora.org>,
        Raviv Shvili <rshvili@...eaurora.org>,
        Hannes Reinecke <hare@...e.de>, Kyuho Choi <kyuho.choi@...com>,
        Martin Petersen <martin.petersen@...cle.com>
Subject: RE: [PATCH v5 1/2] scsi: ufs: Do not disable vccq in UFSHC driver

> 
> 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@...e.fr>
Acked-by: Avri Altman <avri.altman@....com>