On 2020/12/01 3:22, Rob Herring wrote:
> On Wed, Nov 25, 2020 at 2:06 PM Serge Semin <fancer.lancer@xxxxxxxxx> wrote:
>>
>> On Thu, Nov 19, 2020 at 03:09:20PM +0900, Damien Le Moal wrote:
>>> The DesignWare gpio-dwapb GPIO driver ("snps,dw-apb-gpio" or
>>> "apm,xgene-gpio-v2" compatible string) defines the now deprecated
>>> property "snps,nr-gpios" to specify the number of GPIOs available
>>> on a port. However, if this property is used in a device tree, its
>>> "-gpios" suffix causes the generic property supplier parsing code to
>>> interpret it as a cell reference when properties are parsed in
>>> of_link_to_suppliers(), leading to an error messages such as:
>>>
>>> OF: /soc/bus@50200000/gpio-controller@50200000/gpio-port@0: could not
>>> find phandle
>>>
>>> Fix this by manually defining a parse_gpios() function which ignores
>>> this deprecated property that is still present in many device trees,
>>> skipping the search for the supplier and thus avoiding the device tree
>>> parsing error.
>>>
>>> Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxx>
>>> ---
>>> drivers/of/property.c | 16 +++++++++++++++-
>>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/of/property.c b/drivers/of/property.c
>>> index 408a7b5f06a9..4eefe8efc2fe 100644
>>> --- a/drivers/of/property.c
>>> +++ b/drivers/of/property.c
>>> @@ -1308,7 +1308,6 @@ DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL)
>>> DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL)
>>> DEFINE_SUFFIX_PROP(regulators, "-supply", NULL)
>>> DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells")
>>> -DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells")
>>>
>>> static struct device_node *parse_iommu_maps(struct device_node *np,
>>> const char *prop_name, int index)
>>> @@ -1319,6 +1318,21 @@ static struct device_node *parse_iommu_maps(struct device_node *np,
>>> return of_parse_phandle(np, prop_name, (index * 4) + 1);
>>> }
>>>
>>> +static struct device_node *parse_gpios(struct device_node *np,
>>> + const char *prop_name, int index)
>>> +{
>>> + /*
>>> + * Quirk for the deprecated "snps,nr-gpios" property of the
>>> + * DesignWare gpio-dwapb GPIO driver: as this property parsing
>>> + * conflicts with the "xx-gpios" phandle reference property, ignore it.
>>> + */
>>
>>> + if (strcmp(prop_name, "snps,nr-gpios") == 0)
>>> + return NULL;
>>
>> What about printing the warning from instead of doing that from the driver?
>> Like this:
>>
>> + if (strcmp(prop_name, "snps,nr-gpios") == 0) {
>> + pr_warn("%pOF: %s is deprecated in favor of ngpios\n");
>> + return NULL;
>> + }
>>
>> So when the property is removed from all dts'es we wouldn't
>> forget to discard the quirk?
>
> Why do we need this change at all? We've already got a message printed
> and devlink is still default off. If someone cares about devlink and
> the error message, then they should fix their dts file.
>
> Now if there's a stable/mature platform using "snps,nr-gpios" and we
> enable devlink by default (which needs to happen at some point), then
> I'd feel differently and we'll need to handle this. But until then, I
> don't want to carry this quirk.
I have the device tree fixed for my use case, so there is no problem anymore.
The improvement this patch brings is a clearer error message. The one that
shows up without the patch is fairly obscure and it took me a while to figure
out what was wrong. But again, no problems for me since the DT is fixed. So
sure, we can drop this patch.
>
> Rob
>
--
Damien Le Moal
Western Digital Research
