On Wed, Dec 05, 2018 at 08:20:58PM +0200, Matti Vaittinen wrote:
> On Wed, Dec 05, 2018 at 09:19:33AM -0800, Stephen Boyd wrote:
> > Quoting Matti Vaittinen (2018-12-04 23:00:46)
> > > But that won't solve the issue if we don't do "dirty hacks" in driver.
> > > The devm interface still only gets the device-pointer, not the DT node
> > > as argument. And if DT node for device is NULL (like in MFD cases) -
> > > then there is no parent node, only parent device with a node. For plain
> > > of_clk_add_provider() the driver can just give the parent's node pointer
> > > in cases where it knows it is the parent who has the provider data in
> > > DT. But our original problem is in devm interfaces.
> > >
> >
> > I was misunderstanding the MFD design. Should still work though, so I
> > squashed this into the patch to clean things up a bit. Does this work
> > for you?
>
> This looks good to me. Especially changing the of_is_clk_provider to
> get_clk_provider_node which allows to remove some repetition. If you
> apply this then I will drop my patch from the series. Just please let me
> know. I will cook version 7 of the series at Friday - tomorrow is the
> independence day in Finland and I'll be offline =)
I see you already applied the follow-up patches to clk-next. Please note
that most of the clean-ups did require this functionality as they used
the parent DT node.
>
> Thanks!
>
> >
> > ------8<-----
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index bb689161f0f5..6ff852bda892 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -3893,9 +3893,23 @@ static void devm_of_clk_release_provider(struct device *dev, void *res)
> > of_clk_del_provider(*(struct device_node **)res);
> > }
> >
> > -static int of_is_clk_provider(struct device_node *np)
> > +/*
> > + * We allow a child device to use its parent device as the clock provider node
> > + * for cases like MFD sub-devices where the child device driver wants to use
> > + * devm_*() APIs but not list the device in DT as a sub-node.
> > + */
> > +static struct device_node *get_clk_provider_node(struct device *dev)
> > {
> > - return !!of_find_property(np, "#clock-cells", NULL);
> > + struct device_node *np, *parent_np;
> > +
> > + np = dev->of_node;
> > + parent_np = dev->parent ? dev->parent->of_node : NULL;
> > +
> > + if (!of_find_property(np, "#clock-cells", NULL))
> > + if (of_find_property(parent_np, "#clock-cells", NULL))
> > + np = parent_np;
> > +
> > + return np;
> > }
> >
> > /**
> > @@ -3920,17 +3934,12 @@ int devm_of_clk_add_hw_provider(struct device *dev,
> > struct device_node **ptr, *np;
> > int ret;
> >
> > - np = dev->of_node;
> > -
> > - if (!of_is_clk_provider(dev->of_node))
> > - if (of_is_clk_provider(dev->parent->of_node))
> > - np = dev->parent->of_node;
> > -
> > ptr = devres_alloc(devm_of_clk_release_provider, sizeof(*ptr),
> > GFP_KERNEL);
> > if (!ptr)
> > return -ENOMEM;
> >
> > + np = get_clk_provider_node(dev);
> > ret = of_clk_add_hw_provider(np, get, data);
> > if (!ret) {
> > *ptr = np;
> > @@ -3981,13 +3990,8 @@ static int devm_clk_provider_match(struct device *dev, void *res, void *data)
> > void devm_of_clk_del_provider(struct device *dev)
> > {
> > int ret;
> > - struct device_node *np;
> > -
> > - np = dev->of_node;
> > + struct device_node *np = get_clk_provider_node(dev);
> >
> > - if (!of_is_clk_provider(dev->of_node))
> > - if (of_is_clk_provider(dev->parent->of_node))
> > - np = dev->parent->of_node;
> > ret = devres_release(dev, devm_of_clk_release_provider,
> > devm_clk_provider_match, np);
> >
> >
>
> --
> Matti Vaittinen
> ROHM Semiconductors
>
> ~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
--
Matti Vaittinen
ROHM Semiconductors
~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
