From patchwork Wed Feb 27 10:41:45 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marc Gonzalez X-Patchwork-Id: 10831519 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3620715AC for ; Wed, 27 Feb 2019 10:42:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1CD292CD96 for ; Wed, 27 Feb 2019 10:42:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0C6822D0DE; Wed, 27 Feb 2019 10:42:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,FREEMAIL_FROM, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 293E52CD96 for ; Wed, 27 Feb 2019 10:42:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726950AbfB0KmW (ORCPT ); Wed, 27 Feb 2019 05:42:22 -0500 Received: from smtp3-g21.free.fr ([212.27.42.3]:16693 "EHLO smtp3-g21.free.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725993AbfB0KmW (ORCPT ); Wed, 27 Feb 2019 05:42:22 -0500 Received: from [192.168.108.68] (unknown [213.36.7.13]) (Authenticated sender: marc.w.gonzalez) by smtp3-g21.free.fr (Postfix) with ESMTPSA id 442E013F81D; Wed, 27 Feb 2019 11:41:45 +0100 (CET) Subject: [PATCH v6 1/2] Revert "scsi: ufs: disable vccq if it's not needed by UFS device" From: Marc Gonzalez To: SCSI , LKML Cc: Jeffrey Hugo , Bjorn Andersson , Evan Green , Douglas Anderson , Alim Akhtar , Avri Altman , Pedro Sousa , Bart Van Assche , Stanislav Nijnikov , Alex Lemberg , Ohad Sharabi , Hannes Reinecke , Kyuho Choi , Martin Petersen References: Message-ID: Date: Wed, 27 Feb 2019 11:41:45 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This reverts commit 60f0187031c05e04cbadffb62f557d0ff3564490. There was one conflict in drivers/scsi/ufs/ufshcd.c <<<<<<< HEAD /* Init check for device descriptor sizes */ ufshcd_init_desc_sizes(hba); ret = ufs_get_device_desc(hba, &card); if (ret) { dev_err(hba->dev, "%s: Failed getting device info. err = %d\n", __func__, ret); goto out; } ufs_fixup_device_setup(hba, &card); ufshcd_tune_unipro_params(hba); ret = ufshcd_set_vccq_rail_unused(hba, (hba->dev_quirks & UFS_DEVICE_NO_VCCQ) ? true : false); if (ret) goto out; ======= ufs_advertise_fixup_device(hba); >>>>>>> parent of 60f0187031c0... scsi: ufs: disable vccq if it's not needed by UFS device Resolution: keep HEAD, and delete the ufshcd_set_vccq_rail_unused() call and corresponding error-handling code. Clean up loose ends in a follow-up patch. 60f0187031c0 introduced a small power optimization: ignore the vccq load specified in the UFSHC DT node when said host controller is connected to specific Flash chips (currently, Samsung and Hynix). 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. Acked-by: Avri Altman Acked-by: Alim Akhtar Signed-off-by: Marc Gonzalez --- drivers/scsi/ufs/ufs.h | 1 - drivers/scsi/ufs/ufshcd.c | 59 +++------------------------------------ 2 files changed, 4 insertions(+), 56 deletions(-) diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h index 6d176815e6ce..21e4ccb5ba6e 100644 --- a/drivers/scsi/ufs/ufs.h +++ b/drivers/scsi/ufs/ufs.h @@ -514,7 +514,6 @@ struct ufs_vreg { struct regulator *reg; const char *name; bool enabled; - bool unused; int min_uV; int max_uV; int min_uA; diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 5ca4581e60d5..a5bfcf04fdba 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -251,7 +251,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba); static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on, bool skip_ref_clk); static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on); -static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused); static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba); static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba); static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba); @@ -6830,11 +6829,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba) ufs_fixup_device_setup(hba, &card); ufshcd_tune_unipro_params(hba); - ret = ufshcd_set_vccq_rail_unused(hba, - (hba->dev_quirks & UFS_DEVICE_NO_VCCQ) ? true : false); - if (ret) - goto out; - /* UFS device is also active now */ ufshcd_set_ufs_dev_active(hba); ufshcd_force_reset_auto_bkops(hba); @@ -7018,24 +7012,13 @@ static int ufshcd_config_vreg_load(struct device *dev, struct ufs_vreg *vreg, static inline int ufshcd_config_vreg_lpm(struct ufs_hba *hba, struct ufs_vreg *vreg) { - if (!vreg) - return 0; - else if (vreg->unused) - return 0; - else - return ufshcd_config_vreg_load(hba->dev, vreg, - UFS_VREG_LPM_LOAD_UA); + return ufshcd_config_vreg_load(hba->dev, vreg, UFS_VREG_LPM_LOAD_UA); } static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba, struct ufs_vreg *vreg) { - if (!vreg) - return 0; - else if (vreg->unused) - return 0; - else - return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA); + return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA); } static int ufshcd_config_vreg(struct device *dev, @@ -7073,9 +7056,7 @@ static int ufshcd_enable_vreg(struct device *dev, struct ufs_vreg *vreg) { int ret = 0; - if (!vreg) - goto out; - else if (vreg->enabled || vreg->unused) + if (!vreg || vreg->enabled) goto out; ret = ufshcd_config_vreg(dev, vreg, true); @@ -7095,9 +7076,7 @@ static int ufshcd_disable_vreg(struct device *dev, struct ufs_vreg *vreg) { int ret = 0; - if (!vreg) - goto out; - else if (!vreg->enabled || vreg->unused) + if (!vreg || !vreg->enabled) goto out; ret = regulator_disable(vreg->reg); @@ -7203,36 +7182,6 @@ static int ufshcd_init_hba_vreg(struct ufs_hba *hba) return 0; } -static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused) -{ - int ret = 0; - struct ufs_vreg_info *info = &hba->vreg_info; - - if (!info) - goto out; - else if (!info->vccq) - goto out; - - if (unused) { - /* shut off the rail here */ - ret = ufshcd_toggle_vreg(hba->dev, info->vccq, false); - /* - * Mark this rail as no longer used, so it doesn't get enabled - * later by mistake - */ - if (!ret) - info->vccq->unused = true; - } else { - /* - * rail should have been already enabled hence just make sure - * that unused flag is cleared. - */ - info->vccq->unused = false; - } -out: - return ret; -} - static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on, bool skip_ref_clk) {