linux-kernel - Re: [PATCH] drm/panel: lvds: Handle the optional regulator case properly

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] [day] [month] [year] [list]

Message-ID: <20180115091821.wwg73p6q67k4ey4p@flea.lan>
Date:   Mon, 15 Jan 2018 10:18:21 +0100
From:   Maxime Ripard <maxime.ripard@...e-electrons.com>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:     dri-devel@...ts.freedesktop.org,
        Daniel Vetter <daniel.vetter@...el.com>,
        Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/panel: lvds: Handle the optional regulator case
 properly

On Fri, Jan 12, 2018 at 12:09:39AM +0200, Laurent Pinchart wrote:
> Hi Maxime,
> 
> On Friday, 12 January 2018 00:06:06 EET Laurent Pinchart wrote:
> > On Thursday, 11 January 2018 15:12:56 EET Maxime Ripard wrote:
> > > On Thu, Jan 11, 2018 at 03:05:01PM +0200, Laurent Pinchart wrote:
> > >> On Wednesday, 10 January 2018 17:59:41 EET Maxime Ripard wrote:
> > >>> The devm_regulator_get_optional function, unlike it was assumed in the
> > >>> commit a1c55bccf600 ("drm/panel: lvds: Add support for the power-supply
> > >>> property"), is actually returning an error pointer with -ENODEV instead
> > >>> of NULL when there's no regulator to find.
> > >>> 
> > >>> Make sure we handle that case properly.
> > >>> 
> > >>> Fixes: a1c55bccf600 ("drm/panel: lvds: Add support for the power-supply
> > >>> property") Signed-off-by: Maxime Ripard
> > >>> <maxime.ripard@...e-electrons.com>
> > >>> ---
> > >>> 
> > >>>  drivers/gpu/drm/panel/panel-lvds.c | 9 +++++++--
> > >>>  1 file changed, 7 insertions(+), 2 deletions(-)
> > >>> 
> > >>> diff --git a/drivers/gpu/drm/panel/panel-lvds.c
> > >>> b/drivers/gpu/drm/panel/panel-lvds.c index 57e38a9e7ab4..9f46e7095c0e
> > >>> 100644
> > >>> --- a/drivers/gpu/drm/panel/panel-lvds.c
> > >>> +++ b/drivers/gpu/drm/panel/panel-lvds.c
> > >>> @@ -215,8 +215,13 @@ static int panel_lvds_probe(struct platform_device
> > >>> *pdev)
> > >>> 	lvds->supply = devm_regulator_get_optional(lvds->dev, "power");
> > >>>  	if (IS_ERR(lvds->supply)) {
> > >>>  		ret = PTR_ERR(lvds->supply);
> > >>> -		dev_err(lvds->dev, "failed to request regulator: %d\n", ret);
> > >>> -		return ret;
> > >>> +
> > >>> +		if (ret != -ENODEV) {
> > >>> +			dev_err(lvds->dev, "failed to request regulator: %d\n", ret);
> > >>> +			return ret;
> > >> 
> > >> I wouldn't print an error message if ret == -EPROBE_DEFER.
> > >> 
> > >>> +		} else {
> > >>> +			lvds->supply = NULL;
> > >>> +		}
> > >>>  	}
> > >> 
> > >> How about
> > >> 
> > >> 	lvds->supply = devm_regulator_get_optional(lvds->dev, "power");
> > >> 	if (IS_ERR(lvds->supply)) {
> > >> 		ret = PTR_ERR(lvds->supply);
> > >> 		if (ret != -ENODEV) {
> > >> 			if (ret == -EPROBE_DEFER)
> > > 
> > > I guess that would be != -EPROBE_DEFER
> > 
> > Of course, my bad.
> > 
> > >> 				dev_err(lvds->dev, "failed to request regulator: %d\n", ret);
> > >> 			return ret;
> > >> 		}
> > >> 		
> > >> 		lvds->supply = NULL;
> > >> 	}
> > > 
> > > Otherwise, it works for me.
> 
> With the above change,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> Tested-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>

Applied to drm-misc-next-fixes.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)