Re: [PATCH v3 1/4] clk: add a clk_hw helpers to get the clock device or device_node

[Stephen Boyd <sboyd@kernel.org>]

Quoting Jerome Brunet (2025-02-27 02:07:39)
> On Wed 26 Feb 2025 at 17:01, Stephen Boyd <sboyd@kernel.org> wrote:
> 
> > Quoting Jerome Brunet (2025-01-20 09:15:30)
> >> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> >> index 9b45fa005030f56e1478b9742715ebcde898133f..9818f87c1c56ab9a3782c2fd55d3f602041769c3 100644
> >> --- a/drivers/clk/clk.c
> >> +++ b/drivers/clk/clk.c
> >> @@ -365,6 +365,39 @@ const char *clk_hw_get_name(const struct clk_hw *hw)
[...]
> >> + * Return: the struct device associated with the clock, or NULL if there
> >> + * is none.
> >> + */
> >> +struct device *clk_hw_get_dev(const struct clk_hw *hw)
> >> +{
> >> +       return hw->core->dev;
> >
> > Maybe we should increment the device refcount and require callers to
> > put_device(). Now's our chance to make the change!
> 
> I'm afraid this would lead to a lot of boilerplate code and mis-management,
> especially in the clock ops.

Don't we have __release() helpers? Not sure what boilerplate you're
talking about.

> 
> Would it be better if clock core took care of that, at least for the ops
> part ? I mean incrementing and decrementing the ref count based on the
> clk_hw registration. That would make things a lot easier for clock
> users.

Meh, I don't know. We've been assuming that the device is present
because the driver will be unbound and the clks unregistered before the
device can disappear. Nothing enforces that though so things could go
wrong if we have a bug somewhere vs. knowing for sure that the refcount
is incremented here. What you're suggesting is a bigger change, pushing
down the reference counting so that as long as the clk is registered the
device is known to be valid.

Looking into the crystal ball of the future shows me that this will get
wrapped in rust and at that point we'll be sharing the reference (likely
not mutable) with the caller. If we do proper reference counting at the
start that will make it easier to convert code, but it probably doesn't
matter much because any rust clk provider would use the rust wrapper
where we could handle the refcount logic.

> 
> If the consumer of the API uses it for something that may outlive the
> clk_hw, then it is up to it to properly increment the ref count since it
> is clearly not clock stuff.

Sure. I'm fine to not change anything. Mostly thinking out loud.