of_clk_get() / devm_clk_get()
Currently we have devm_clk_get() which gives a managed-resource clk (by name), or of_clk_get() which gives an unmanaged resource clk (by id).
I just wanted to sound out everyone as to whether there is a need for a managed version of the of_clk_get.
My personal concern about devm_clk_get is that it requires (if I understand correctly) that the DT node have the clock-names property (which is optional). If the optional name is not supplied, it fails. This basically makes it 'not optional' when a driver uses devm_clk_get. (Please correct me if I'm wrong about this).
Everything below here is irrelevant if the above is wrong :)
If the above IS correct, then couldn't we have a devm_of_clk_get which accepts a *id (as per devm_clk_get) AND an index (as per of_clk_get), with the *id given priority.
Something along the lines of...
struct clk *devm_of_clk_get(struct device *dev,
const char *id,
int index)
{
struct clk **ptr, *clk = NULL;
if (id)
clk = devm_clk_get(dev, id);
if (!clk || IS_ERR(clk)) {
clk = of_clk_get(dev->of_node, index);
if (!IS_ERR(clk)) {
ptr = devres_alloc(devm_clk_release,
sizeof(*ptr),
GFP_KERNEL)
if (!ptr)
return ERR_PTR(-ENOMEM);
*ptr = clk;
devres_add(dev, ptr);
}
}
return clk;
}This would make clock-names optional (as intended) and give a fallback to the index when clock-names isn't specified.
One issue (which I don't know how much of an issue it is) is that this doesn't follow the normal pattern of normal_func -> devm_func parameter layout.
Regards
Tony Prisk
Quoted 11 lines · Tony Prisk
Currently we have
devm_clk_get()which gives a managed-resource clk (by name), orof_clk_get()which gives an unmanaged resource clk (by id).I just wanted to sound out everyone as to whether there is a need for a managed version of the
of_clk_get.My personal concern about
devm_clk_getis that it requires (if I understand correctly) that the DT node have the clock-names property (which is optional). If the optional name is not supplied, it fails. This basically makes it 'not optional' when a driver usesdevm_clk_get. (Please correct me if I'm wrong about this).
I thought supplying NULL for the name/ID simply gave you the first clock in the list (clocks property)? I'm sure I've seen plenty of Tegra drivers that end up doing that.
Quoted 13 lines · Stephen Warren
Quoted 11 lines · Tony Prisk
Currently we have
devm_clk_get()which gives a managed-resource clk (by name), orof_clk_get()which gives an unmanaged resource clk (by id).I just wanted to sound out everyone as to whether there is a need for a managed version of the
of_clk_get.My personal concern about
devm_clk_getis that it requires (if I understand correctly) that the DT node have the clock-names property (which is optional). If the optional name is not supplied, it fails. This basically makes it 'not optional' when a driver usesdevm_clk_get. (Please correct me if I'm wrong about this).
Quoted 4 lines
I thought supplying NULL for the name/ID simply gave you the first clock in the list (clocks property)? I'm sure I've seen plenty of Tegra drivers that end up doing that.
I think someone else told me that once before so it is probably correct
- but it doesn't solve the problem of requesting a second clock without a clock-names property.
That leaves us in the situation of only being able to use managed clocks when there is only one, or forcing dts files to include the 'optional' property.
Regards
Tony Prisk
Quoted 19 lines · Tony Prisk
Quoted 13 lines · Stephen Warren
Quoted 11 lines · Tony Prisk
Currently we have
devm_clk_get()which gives a managed-resource clk (by name), orof_clk_get()which gives an unmanaged resource clk (by id).I just wanted to sound out everyone as to whether there is a need for a managed version of the
of_clk_get.My personal concern about
devm_clk_getis that it requires (if I understand correctly) that the DT node have the clock-names property (which is optional). If the optional name is not supplied, it fails. This basically makes it 'not optional' when a driver usesdevm_clk_get. (Please correct me if I'm wrong about this).Quoted 4 lines
I thought supplying NULL for the name/ID simply gave you the first clock in the list (clocks property)? I'm sure I've seen plenty of Tegra drivers that end up doing that.
Quoted 7 lines
I think someone else told me that once before so it is probably correct
- but it doesn't solve the problem of requesting a second clock without a clock-names property.That leaves us in the situation of only being able to use managed clocks when there is only one, or forcing dts files to include the 'optional' property.
True. Writing a devm_ wrapper around of_clk_get() should be pretty trivial though.