Hi Florian,
On Wed, Dec 15 2021, Florian Fainelli wrote:
> On 12/6/21 11:27 PM, Baruch Siach wrote:
>> There must be three parameters in gpio-ranges property. Fixes this not
>> very helpful error message:
>>
>> OF: /soc/pinctrl@1000000: (null) = 3 found 3
>>
>> Fixes: 1e8277854b49 ("arm64: dts: Add ipq6018 SoC and CP01 board support")
>> Cc: Sricharan R <sricharan@xxxxxxxxxxxxxx>
>> Signed-off-by: Baruch Siach <baruch@xxxxxxxxxx>
>> ---
>>
>> The error message was improved in commit af3be70a321 ("of: Improve
>> of_phandle_iterator_next() error message"), but there is evidently some
>> more room for improvement. As I don't really understand the code, I
>> added the commit author and the DT list to Cc.
>
> In the commit message you find what the error message was and then
> became which tells you which property was used to determine the cell
> size, and what count was obtained instead.
>
> In your particular case here however, since the proper was with the
> 'gpio-ranges' property and there is no property that indicates the cell
> size AFAICT, it might been more helpful to revert back to the old
> message maybe?
Thanks for taking a look. A few comment below.
> So maybe something like which is hopefully the best of
> both worlds:
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 372a177f3b80..63bff37bf035 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1378,9 +1378,13 @@ int of_phandle_iterator_next(struct
> of_phandle_iterator *it)
> * property data length
> */
> if (it->cur + count > it->list_end) {
> - pr_err("%pOF: %s = %d found %d\n",
> - it->parent, it->cells_name,
> - count, it->cell_count);
> + if (it->cells_name)
> + pr_err("%pOF: %s = %d found %d\n",
> + it->parent, it->cells_name,
> + count, it->cell_count);
Thinking about it, shouldn't we have 'it->list_end - it->cur' instead of
it->cell_count here? This change produces:
OF: /soc/pinctrl@1000000: (null) = 3 found 2
which looks more correct. This is not related to my original issue,
though it might have helped me back then.
> + else
> + pr_err("%pOF: arguments longer than
> property\n",
> + it->parent);
> goto err;
> }
> }
I think that the best aid would have been the property name. But as far
as I can see struct of_phandle_iterator does not preserve the list_name
parameter of of_phandle_iterator_init().
Second best would probably be the phandle name, that is
of_node_full_name(it->node). It is a little confusing in this case
because it->node is the same as it->parent.
What do you think?
baruch
>> arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
>> index 6a22bb5f42f4..a717fc17523d 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
>> @@ -220,7 +220,7 @@ tlmm: pinctrl@1000000 {
>> interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
>> gpio-controller;
>> #gpio-cells = <2>;
>> - gpio-ranges = <&tlmm 0 80>;
>> + gpio-ranges = <&tlmm 0 0 80>;
>> interrupt-controller;
>> #interrupt-cells = <2>;
>>
>>
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@xxxxxxxxxx - tel: +972.52.368.4656, http://www.tkos.co.il -
