From patchwork Tue Apr 5 11:26:02 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Archit Taneja X-Patchwork-Id: 8749711 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 7EC04C0553 for ; Tue, 5 Apr 2016 11:26:13 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4C799203B1 for ; Tue, 5 Apr 2016 11:26:12 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id DCAEE203A0 for ; Tue, 5 Apr 2016 11:26:10 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A0D7B6E757; Tue, 5 Apr 2016 11:26:09 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from smtp.codeaurora.org (smtp.codeaurora.org [198.145.29.96]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5FDDC88F5A for ; Tue, 5 Apr 2016 11:26:08 +0000 (UTC) Received: from smtp.codeaurora.org (localhost [127.0.0.1]) by smtp.codeaurora.org (Postfix) with ESMTP id 3C1446164B; Tue, 5 Apr 2016 11:26:08 +0000 (UTC) Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 2ED8F6164C; Tue, 5 Apr 2016 11:26:08 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from [10.79.40.55] (unknown [202.46.23.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: architt@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id 8601D615BC; Tue, 5 Apr 2016 11:26:05 +0000 (UTC) Subject: Re: [PATCH] drm/msm/dsi: Fix regulator API abuse To: Mark Brown References: <1459364022-28484-1-git-send-email-broonie@kernel.org> From: Archit Taneja Message-ID: <5703A0CA.8020802@codeaurora.org> Date: Tue, 5 Apr 2016 16:56:02 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <1459364022-28484-1-git-send-email-broonie@kernel.org> X-Virus-Scanned: ClamAV using ClamSMTP Cc: dri-devel@lists.freedesktop.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP Hi, On 03/31/2016 12:23 AM, Mark Brown wrote: > The voltage changing code in this driver is broken and should be > removed. The driver sets a single, exact voltage on probe. Unless > there is a very good reason for this (which should be documented in > comments) constraints like this need to be set via the machine > constraints, voltage setting in a driver is expected to be used in cases > where the voltage varies at runtime. > > In addition client drivers should almost never be calling > regulator_can_set_voltage(), if the device needs to set a voltage it > needs to set the voltage and the regulator core will handle the case > where the regulator is fixed voltage. If the driver simply skips > setting the voltage if it doesn't have permission then it shouild just s/shouild/should Could you also pull in the diff I've added below? It removes another set_voltage call from the hdmi driver and the struct members used to manage the voltage range. > not bother in the first place. > +John, you'll need to update your regulator nodes with the min/max voltage values in your N7 dtsi whenever you plan to rebase next. Thanks, Archit > Signed-off-by: Mark Brown > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 12 ------------ > 1 file changed, 12 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > index 4282ec6bbaaf..a3e47ad83eb3 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -325,18 +325,6 @@ static int dsi_regulator_init(struct msm_dsi_host *msm_host) > return ret; > } > > - for (i = 0; i < num; i++) { > - if (regulator_can_change_voltage(s[i].consumer)) { > - ret = regulator_set_voltage(s[i].consumer, > - regs[i].min_voltage, regs[i].max_voltage); > - if (ret < 0) { > - pr_err("regulator %d set voltage failed, %d\n", > - i, ret); > - return ret; > - } > - } > - } > - > return 0; > } > > ---8<--- --->8--- diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 749fbb2..03f115f 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -41,8 +41,6 @@ enum msm_dsi_phy_type { /* Regulators for DSI devices */ struct dsi_reg_entry { char name[32]; - int min_voltage; - int max_voltage; int enable_load; int disable_load; }; diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c index e58e9b9..cc802bb 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c @@ -22,9 +22,9 @@ static const struct msm_dsi_config apq8064_dsi_cfg = { .reg_cfg = { .num = 3, .regs = { - {"vdda", 1200000, 1200000, 100000, 100}, - {"avdd", 3000000, 3000000, 110000, 100}, - {"vddio", 1800000, 1800000, 100000, 100}, + {"vdda", 100000, 100}, + {"avdd", 10000, 100}, + {"vddio", 100000, 100}, }, }, .bus_clk_names = dsi_v2_bus_clk_names, @@ -40,10 +40,10 @@ static const struct msm_dsi_config msm8974_apq8084_dsi_cfg = { .reg_cfg = { .num = 4, .regs = { - {"gdsc", -1, -1, -1, -1}, - {"vdd", 3000000, 3000000, 150000, 100}, - {"vdda", 1200000, 1200000, 100000, 100}, - {"vddio", 1800000, 1800000, 100000, 100}, + {"gdsc", -1, -1}, + {"vdd", 150000, 100}, + {"vdda", 100000, 100}, + {"vddio", 100000, 100}, }, }, .bus_clk_names = dsi_6g_bus_clk_names, @@ -59,9 +59,9 @@ static const struct msm_dsi_config msm8916_dsi_cfg = { .reg_cfg = { .num = 3, .regs = { - {"gdsc", -1, -1, -1, -1}, - {"vdda", 1200000, 1200000, 100000, 100}, - {"vddio", 1800000, 1800000, 100000, 100}, + {"gdsc", -1, -1}, + {"vdda", 100000, 100}, + {"vddio", 100000, 100}, }, }, .bus_clk_names = dsi_8916_bus_clk_names, @@ -73,13 +73,13 @@ static const struct msm_dsi_config msm8994_dsi_cfg = { .reg_cfg = { .num = 7, .regs = { - {"gdsc", -1, -1, -1, -1}, - {"vdda", 1250000, 1250000, 100000, 100}, - {"vddio", 1800000, 1800000, 100000, 100}, - {"vcca", 1000000, 1000000, 10000, 100}, - {"vdd", 1800000, 1800000, 100000, 100}, - {"lab_reg", -1, -1, -1, -1}, - {"ibb_reg", -1, -1, -1, -1}, + {"gdsc", -1, -1}, + {"vdda", 100000, 100}, + {"vddio", 100000, 100}, + {"vcca", 10000, 100}, + {"vdd", 100000, 100}, + {"lab_reg", -1, -1}, + {"ibb_reg", -1, -1}, }, }, .bus_clk_names = dsi_6g_bus_clk_names, diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c index 91a95fb..e2f42d8 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c @@ -177,19 +177,6 @@ static int dsi_phy_regulator_init(struct msm_dsi_phy *phy) return ret; } - for (i = 0; i < num; i++) { - if (regulator_can_change_voltage(s[i].consumer)) { - ret = regulator_set_voltage(s[i].consumer, - regs[i].min_voltage, regs[i].max_voltage); - if (ret < 0) { - dev_err(dev, - "regulator %d set voltage failed, %d\n", - i, ret); - return ret; - } - } - } - return 0; } diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_20nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_20nm.c index 2e9ba11..3c93cfc 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_20nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_20nm.c @@ -138,8 +138,8 @@ const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs = { .reg_cfg = { .num = 2, .regs = { - {"vddio", 1800000, 1800000, 100000, 100}, - {"vcca", 1000000, 1000000, 10000, 100}, + {"vddio", 100000, 100}, + {"vcca", 10000, 100}, }, }, .ops = { diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c index edf7411..397f09a 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c @@ -138,7 +138,7 @@ const struct msm_dsi_phy_cfg dsi_phy_28nm_hpm_cfgs = { .reg_cfg = { .num = 1, .regs = { - {"vddio", 1800000, 1800000, 100000, 100}, + {"vddio", 100000, 100}, }, }, .ops = { @@ -153,7 +153,7 @@ const struct msm_dsi_phy_cfg dsi_phy_28nm_lp_cfgs = { .reg_cfg = { .num = 1, .regs = { - {"vddio", 1800000, 1800000, 100000, 100}, + {"vddio", 100000, 100}, }, }, .ops = { diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c index 197b039..4ed7b57 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c @@ -185,7 +185,7 @@ const struct msm_dsi_phy_cfg dsi_phy_28nm_8960_cfgs = { .reg_cfg = { .num = 1, .regs = { - {"vddio", 1800000, 1800000, 100000, 100}, + {"vddio", 100000, 100}, }, }, .ops = {