Messages in this thread | | | Date | Thu, 22 Mar 2012 11:15:31 +0000 | From | Mark Brown <> | Subject | Re: [PATCH 4/4] clk: wm831x: Add initial WM831x clock driver |
| |
On Wed, Mar 21, 2012 at 11:26:22PM +0100, Sascha Hauer wrote: > On Wed, Mar 21, 2012 at 08:01:22PM +0000, Mark Brown wrote:
> > + if (!clk_register(&pdev->dev, "xtal", &wm831x_xtal_ops, > > + &clkdata->xtal_hw, NULL, 0, CLK_IS_ROOT)) > > + return -EINVAL;
> The clock names are unique identifiers for the clock, so clocks in > drivers should probably have dev_name encoded into them.
No, that's not sensible. We shouldn't be open coding this into each individual driver that provides clocks, and we shouldn't have clock users having to guess at what scheme the driver author used to dedupe the clocks. As a driver author you would assume that the reason we're providing the struct device to the registration function in the first place is so that the core has the information it needs to do that.
I did provide patches to do what you suggest in the core for one of the earlier versions of the API, I have to say I didn't check to see if they got dropped during the general lulls.
> You could also use the fixed rate generic clock here.
It doesn't really do what I want with reporting the enables - for diagnostic purposes I wanted to always have the clock present but non-enablable so we could actually look at the clock tree and see what's going on directly when there's no clock coming out. [unhandled content-type:application/pgp-signature] | |