linux-kernel - Re: [PATCH] drm/omap/dss: Use syscon_regmap_lookup_by_phandle

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: <9b54bb98-0d96-4c48-9500-4e754889d795@ideasonboard.com>
Date: Tue, 14 Jan 2025 16:23:11 +0200
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/omap/dss: Use syscon_regmap_lookup_by_phandle_args

Hi,

On 12/01/2025 15:46, Krzysztof Kozlowski wrote:
> Use syscon_regmap_lookup_by_phandle_args() which is a wrapper over
> syscon_regmap_lookup_by_phandle() combined with getting the syscon
> argument.  Except simpler code this annotates within one line that given
> phandle has arguments, so grepping for code would be easier.
> 
> There is also no real benefit in printing errors on missing syscon
> argument, because this is done just too late: runtime check on
> static/build-time data.  Dtschema and Devicetree bindings offer the
> static/build-time check for this already.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> ---
>   drivers/gpu/drm/omapdrm/dss/dss.c | 14 ++++----------
>   1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c b/drivers/gpu/drm/omapdrm/dss/dss.c
> index 7b2df3185de4..692df747e2ae 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dss.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dss.c
> @@ -1236,21 +1236,15 @@ static int dss_video_pll_probe(struct dss_device *dss)
>   	if (!np)
>   		return 0;
>   
> -	if (of_property_read_bool(np, "syscon-pll-ctrl")) {
> -		dss->syscon_pll_ctrl = syscon_regmap_lookup_by_phandle(np,
> -			"syscon-pll-ctrl");
> +	if (of_property_present(np, "syscon-pll-ctrl")) {
> +		dss->syscon_pll_ctrl =
> +			syscon_regmap_lookup_by_phandle_args(np, "syscon-pll-ctrl",
> +							     1, &dss->syscon_pll_ctrl_offset);
>   		if (IS_ERR(dss->syscon_pll_ctrl)) {
>   			dev_err(&pdev->dev,
>   				"failed to get syscon-pll-ctrl regmap\n");
>   			return PTR_ERR(dss->syscon_pll_ctrl);
>   		}
> -
> -		if (of_property_read_u32_index(np, "syscon-pll-ctrl", 1,
> -				&dss->syscon_pll_ctrl_offset)) {
> -			dev_err(&pdev->dev,
> -				"failed to get syscon-pll-ctrl offset\n");
> -			return -EINVAL;
> -		}
>   	}
>   
>   	pll_regulator = devm_regulator_get(&pdev->dev, "vdda_video");

Looks reasonable to me. Tested on dra76 evm.

I can push this to drm-misc-next later if I see no other comments.

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>

  Tomi