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

Message-ID: <d0fdf2c5-402d-49c1-a524-76943a24ecda@linaro.org>
Date: Tue, 14 Jan 2025 15:43:46 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
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

On 14/01/2025 15:23, Tomi Valkeinen wrote:
> 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")) {

I see now that I combined two changes I wanted to split. This should be
a separate commit.

>> +		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>
Let me send a v2 with your tag.

Best regards,
Krzysztof