On Thu, Mar 10, 2016 at 10:44:45AM +0200, Heikki Krogerus wrote:
> 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?
Scratch that last question. I'll just prepare a patch with your
proposal and in the same patch change the checking of the primary
fwnode into !IS_ERR_OR_NULL(fwnode). I think it should work for now.
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
