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.kozlow...@linaro.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.valkei...@ideasonboard.com>
Let me send a v2 with your tag.

Best regards,
Krzysztof

Reply via email to