Re: [PATCH v2 1/2] of: Fix property supplier parsing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Rob



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux