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 =)
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 ~~~
