Re: [Resend Patch v6] driver/clk/clk-si5338: Add common clock framework driver for si5338

[York Sun <yorksun@freescale.com>]

On 10/20/2015 10:49 AM, Stephen Boyd wrote:
> On 10/19, York Sun wrote:
>>
>>
>> On 10/19/2015 05:36 PM, Stephen Boyd wrote:
>>> On 10/20, York Sun wrote:
>>>>
>>>> You can see, when the fixed-rate clock is registered, the device id of si5338 is unknown yet. (I am using one 25MHz clock to feed multiple si5338, where maybe I should create multiple 25MHz clocks, but that's another discussion.)
>>>>
>>>> I hope I have made it clear.
>>>
>>> Yes. It would be great if we could modify the i2c framework to
>>> let us create an i2c device but not call device_register() until a later
>>> time. So something like this could be done in the platform
>>> driver:
>>>
>>> 	for (i = 0; i < NUM_SI5338_CHIPS; i++) {
>>> 	     adap = i2c_get_adapter(private->i2c_adp->nr + 1 + i);
>>> 	     private->i2c_client[i] = i2c_new_device_unregistered(adap,
>>> 	     				&si5338_info[i], i2c_si5338_addr);
>>> 	     clkdev_create(clk, NULL, dev_name(private->i2c_client[i]->dev));
>>> 	     device_register(private->i2c_client[i]->dev);
>>> 	}
>>>
>>> Then in the si5338 driver we call devm_clk_get(i2c->dev, NULL)
>>> and we get the xtal clock.
>>>
>>
>> Then what do we do before we have this i2c_new_device_unregistered()? Does my
>> proposal make sense?
> 
> There's nothing to do before we make this new API. Make a patch
> to add the new API, and then use it in the pcie driver. Your
> proposal makes sense, but it isn't necessary or desirable.
> 

Stephen,

I am making progress on this direction, but need your guidance on one matter.

When device tree is used, devm_clk_get() generates a pr_err if the clock doesn't
exist. For si5338, it can have one/two/three/four parent clocks. Missing some
clocks shouldn't be treated as an error. It works but the message is confusing
and annoying. I am thinking to add flags to mark unused clocks when parsing
device tree. Please let me know if it is a bad idea.

York