From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9C6ADC433FE for ; Wed, 16 Nov 2022 12:27:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232155AbiKPM1T (ORCPT ); Wed, 16 Nov 2022 07:27:19 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44264 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235900AbiKPM0o (ORCPT ); Wed, 16 Nov 2022 07:26:44 -0500 Received: from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com [IPv6:2a00:1450:4864:20::32c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F0C704D5D5 for ; Wed, 16 Nov 2022 04:23:03 -0800 (PST) Received: by mail-wm1-x32c.google.com with SMTP id t4so11768776wmj.5 for ; Wed, 16 Nov 2022 04:23:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=content-transfer-encoding:in-reply-to:organization:references:cc:to :content-language:subject:reply-to:from:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=NCcvrWSluZ6xWxoxMrbOrq+h9iD8NIFWt/ldX0zWuhg=; b=y5/L+vS0fpgfHuv4SUDaU3im4zKnqVUZqAewsrm2yL1im0aOMqkrOFCAeyfCFuHKFk i3pG4dsMgGdOYxJzSRV47RSETvRsCIcJMvuVa5Y6RgUUgGS6gi5RiE9sIpSzhLxwZxkj owMv+z+wewK/mGatUMqLf3Z064KooAQ/4W+xYnmJ+Y0IAyu3sCjotY9kaqY3oYeHGDzf pb9XrrAhk+gjDwe+KnWjk2wDITh5g+q8OQVIlxies5+Zt3Nw0xoVTXGF/pDiuCJoNzV0 PvxBapPwuDzv6N3AyIEQA2ktOz/xzYSBKVyzmO6mD2e7dnrtF3nORH8a7YGB91Rq5n9x dTag== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:organization:references:cc:to :content-language:subject:reply-to:from:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=NCcvrWSluZ6xWxoxMrbOrq+h9iD8NIFWt/ldX0zWuhg=; b=Sj4UPBAKdI5fNyZ+dQWhJdF+THejVXwzdlytjhlhP28aTxdRQlj3olFmY+3iY29JV/ 0CMIUaT7/MKjD0i4u9JSEdDPjU6f6tCa6aKZpXFcoAbuxTPfXe0CWTfF9BZnOOg2kDvn av5OZ/i9HgQPKAz5NTSm8Jy4n5FxUxMnNNo1GXZv73GoiTaMqz0Uatr73dFt4+SJINVB JLHfI/IToujESBW+4ok560ZB8eAoJALWJ2lt5UhRCRic+KQZLVTRSgrhwFESxGq0TCHA K3CDhNbIpXgWGxtgs4vqBhP54uPJoEfCSWxWuQV6ZON3POtHVccqeYiHK+k5+xQYseXf bzAw== X-Gm-Message-State: ANoB5plgHzlHT3hOWAo21dYyOuXpqtcWPvE1MIqQofipht0hOiv8sS/v 9OBVWNJVqgB6W4mhWy32oTM0tw== X-Google-Smtp-Source: AA0mqf5IwVD4rl/ORoKYsGl8D8jAfmH8GhkGCPt7qRpoPNnSYVvMMs/Qz7TOI1ONBlHx5rk8KVrHcw== X-Received: by 2002:a05:600c:354d:b0:3cf:6b14:1033 with SMTP id i13-20020a05600c354d00b003cf6b141033mr1995260wmq.103.1668601382220; Wed, 16 Nov 2022 04:23:02 -0800 (PST) Received: from ?IPV6:2a01:e0a:982:cbb0:2a03:71a6:7a9d:9c71? ([2a01:e0a:982:cbb0:2a03:71a6:7a9d:9c71]) by smtp.gmail.com with ESMTPSA id c17-20020a5d63d1000000b002322bff5b3bsm18299131wrw.54.2022.11.16.04.23.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 16 Nov 2022 04:23:01 -0800 (PST) Message-ID: <1fdd6b67-c7c4-b927-8d01-485ce8ee645f@linaro.org> Date: Wed, 16 Nov 2022 13:23:00 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.3 From: "neil.armstrong@linaro.org" Reply-To: neil.armstrong@linaro.org Subject: Re: [PATCH v4 1/4] gpu: drm: meson: Use devm_regulator_*get_enable*() Content-Language: en-US To: "Vaittinen, Matti" , Laurent Pinchart , Matti Vaittinen Cc: Andrzej Hajda , Robert Foss , Jonas Karlman , Jernej Skrabec , David Airlie , Daniel Vetter , Kevin Hilman , Jerome Brunet , Martin Blumenstingl , Michael Hennerich , Jean Delvare , Guenter Roeck , Liam Girdwood , Mark Brown , "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , "linux-amlogic@lists.infradead.org" , "linux-arm-kernel@lists.infradead.org" , "linux-hwmon@vger.kernel.org" References: <00d90039-c38a-ad8a-80a1-5a654a528756@linaro.org> <12f645b5-2820-0618-1271-6c254425099c@fi.rohmeurope.com> Organization: Linaro Developer Services In-Reply-To: <12f645b5-2820-0618-1271-6c254425099c@fi.rohmeurope.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 16/11/2022 13:02, Vaittinen, Matti wrote: > Hi dee Ho Laurent & All, > > On 10/21/22 18:29, Neil Armstrong wrote: >> Hi, >> >> On 21/10/2022 17:02, Laurent Pinchart wrote: >>> Hi Matti, >>> >>> On Fri, Oct 21, 2022 at 04:18:01PM +0300, Matti Vaittinen wrote: >>>> Simplify using the devm_regulator_get_enable_optional(). Also drop the >>>> seemingly unused struct member 'hdmi_supply'. >>>> >>>> Signed-off-by: Matti Vaittinen >>>> >>>> --- >>>> v3 => v4: >>>> - split meson part to own patch >>>> >>>> RFCv1 => v2: >>>> - Change also sii902x to use devm_regulator_bulk_get_enable() >>>> >>>> Please note - this is only compile-tested due to the lack of HW. Careful >>>> review and testing is _highly_ appreciated. >>>> --- >>>>   drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++-------------------- >>>>   1 file changed, 3 insertions(+), 20 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c >>>> b/drivers/gpu/drm/meson/meson_dw_hdmi.c >>>> index 5cd2b2ebbbd3..7642f740272b 100644 >>>> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c >>>> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c >>>> @@ -140,7 +140,6 @@ struct meson_dw_hdmi { >>>>       struct reset_control *hdmitx_apb; >>>>       struct reset_control *hdmitx_ctrl; >>>>       struct reset_control *hdmitx_phy; >>>> -    struct regulator *hdmi_supply; >>>>       u32 irq_stat; >>>>       struct dw_hdmi *hdmi; >>>>       struct drm_bridge *bridge; >>>> @@ -665,11 +664,6 @@ static void meson_dw_hdmi_init(struct >>>> meson_dw_hdmi *meson_dw_hdmi) >>>>   } >>>> -static void meson_disable_regulator(void *data) >>>> -{ >>>> -    regulator_disable(data); >>>> -} >>>> - >>>>   static void meson_disable_clk(void *data) >>>>   { >>>>       clk_disable_unprepare(data); >>>> @@ -723,20 +717,9 @@ static int meson_dw_hdmi_bind(struct device >>>> *dev, struct device *master, >>>>       meson_dw_hdmi->data = match; >>>>       dw_plat_data = &meson_dw_hdmi->dw_plat_data; >>>> -    meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev, >>>> "hdmi"); >>>> -    if (IS_ERR(meson_dw_hdmi->hdmi_supply)) { >>>> -        if (PTR_ERR(meson_dw_hdmi->hdmi_supply) == -EPROBE_DEFER) >>>> -            return -EPROBE_DEFER; >>>> -        meson_dw_hdmi->hdmi_supply = NULL; >>>> -    } else { >>>> -        ret = regulator_enable(meson_dw_hdmi->hdmi_supply); >>>> -        if (ret) >>>> -            return ret; >>>> -        ret = devm_add_action_or_reset(dev, meson_disable_regulator, >>>> -                           meson_dw_hdmi->hdmi_supply); >>>> -        if (ret) >>>> -            return ret; >>>> -    } >>>> +    ret = devm_regulator_get_enable_optional(dev, "hdmi"); >>>> +    if (ret != -ENODEV) >>>> +        return ret; >>> >>> As noted in the review of the series that introduced >>> devm_regulator_get_enable_optional(), the right thing to do is to >>> implement runtime PM in this driver to avoid wasting power. >> >> While I agree, it's not really the same level of effort as this patch >> should be functionally equivalent. > > While we all agree that power savings gained by runtime PM would be > great - I don't think we should stop minor improvements while waiting > for that to magically happen ;) > > I like the saying I first heard from Jonathan Cameron - "Don't let the > perfect be an enemy of good". > > After that being said, should I re-spin this or just drop it? (I am > cleaning up my local git from old stuff, and these are about to vanish > from my books). I'm ok with you, please re-spin it. Neil > > Yours, > -- Matti >