On Wed, Mar 09, 2016 at 04:41:12PM +0200, Heikki Krogerus wrote:
> Hi Rafael,
>
> > > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > > index a163f2c..a9df21a9 100644
> > > --- a/drivers/base/property.c
> > > +++ b/drivers/base/property.c
> > > @@ -820,11 +820,13 @@ void device_remove_property_set(struct device *dev)
> > > * the pset. If there is no real firmware node (ACPI/DT) primary
> > > * will hold the pset.
> > > */
> > > - if (!is_pset_node(fwnode))
> > > + if (is_pset_node(fwnode))
> > > + dev->fwnode = NULL;
> >
> > I don't really like the way you clear dev->fwnode directly here.
> > set_primary_fwnode(dev, NULL) would be more appropriate IMO.
> >
> > Also set_secondary_fwnode(dev, NULL) need not be done in that case, because it
> > doesn't change anything.
> >
> > Moreover, if the primary node is not pset, the secondary one should only be
> > cleared if it is pset.
> >
> > So that would mean
> >
> > if (is_pset_node(fwnode)) {
> > set_primary_fwnode(dev, NULL);
So this does create an other problem. We may now end up having ERR_PTR
in the primary fwnode which of course is not considered in the
property handling code, and would need to be fixed similarly as I
fixed the secondary fwnode checking in 1/2 of this series.
But instead of doing that, the whole logic of handing the
primary/secondary fwnodes is probable a little bit broken and should
really be replaced with something better. We can fix these issues one
by one with the code we have now, but it really looks like one fix
will always result into two new problems. Andy promised to look at it.
In the meantime, would it make sense to go ahead with my proposal to
fix the original problem? As a temporary hack?
Thanks,
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
