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

On 2020/11/10 2:45, Serge Semin wrote:

Hello Andy,

On Mon, Nov 09, 2020 at 05:14:21PM +0200, Andy Shevchenko wrote:

quoted

On Sat, Nov 7, 2020 at 10:14 AM Damien Le Moal [off-list ref] wrote:

quoted

@@ -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

quoted

-- 
With Best Regards,
Andy Shevchenko

-- 
Damien Le Moal
Western Digital Research