Re: [PATCH 01/32] of: Fix property supplier parsing

Damien Le Moal
2020-11-16T07:30:34Z

On 2020/11/10 2:45, Serge Semin wrote:
> Hello Andy,
>
> On Mon, Nov 09, 2020 at 05:14:21PM +0200, Andy Shevchenko wrote:
>> On Sat, Nov 7, 2020 at 10:14 AM Damien Le Moal <damien.lemoal@wdc.com> wrote:
>>
>>> @@ -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")
>>
>> Sorry, but the above doesn't sound right to me.
>> It's a generic code and you may imagine how many systems you broke by
>> this change.
>
> Damien replaced the macro above with the code below (your removed it from your
> message):
>
> +static struct device_node *parse_gpios(struct device_node *np,
> + const char *prop_name, int index)
> +{
> + /*
> + * Quirck for the DesignWare gpio-dwapb GPIO driver which defines
> + * the "snps,nr-gpios" property to indicate the total number of GPIOs
> + * available. As this conflict with "xx-gpios" reference properties,
> + * ignore it.
> + */
> + if (strcmp(prop_name, "snps,nr-gpios") == 0)
> + return NULL;
> +
> + return parse_suffix_prop_cells(np, prop_name, index,
> + "-gpios", "#gpio-cells");
> +}
>
> So AFAICS removing the macro shouldn't cause any problem.
>
> My concern was whether the quirk has been really needed. As I said the
> "snps,nr-gpios" property has been marked as deprecated in favor of the standard
> "ngpios" one. Due to the problem noted by Damien any deprecated property
> utilization will cause the DW APB SSI DT-nodes probe malfunction. That
> though implicitly but is supposed to encourage people to provide fixes for
> the dts-files with the deprecated property replaced with "ngpios".
>
> On the other hand an encouragement based on breaking the kernel doesn't seem a
> good solution. So as I see it either we should accept the solution provided by
> Damien, or replace it with a series of fixes for all dts-es with DW APB SSI
> DT-node defined. I suggest to hear the OF-subsystem maintainers out what
> solution would they prefer.

As Rob mentioned, there are still a lot of DTS out there using "snps,nr-gpios",
so I think the fix is needed, albeit with an added warning as Rob suggested so
that board maintainers can notice and update their DT. And I can send a patch
for the DW gpio apb driver to first try the default "ngpios" property, and if it
is not defined, fallback to the legacy "snps,nr-gpios". With that, these new
RISC-V boards will not add another use case of the deprecated "snsps,nr-gpios".
Does that sound like a good plan ?

>
> -Sergey
>
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
>

--
Damien Le Moal
Western Digital Research