Re: [PATCH] i2c-ocores: add common clock support

[Wolfram Sang]

On Thu, Jan 22, 2015 at 07:28:21PM +0100, Peter Korsgaard wrote:
> >>>>> "Wolfram" == Wolfram Sang <wsa@xxxxxxxxxxxxx> writes:
> 
>  >> The clock here is not the i2c bus clock, but the clock input of the
> 
>  > Yes, what I would expect from a clk-property :)
> 
>  >> controller. The function ocores_init initializes the prescaler register of
>  >> the controller so that the bus clock equals 100kHz (internal clock
>  >> runs at 500kHz):
> 
>  > 'clock-frequency' usually describes the I2C bus speed. So, for ocores,
>  > it describes speed of the clock for the controller? That would be
>  > ouch...
> 
> Indeed :/

Oh well, then let's fix this...

> Looking back in the history, the device tree patch originally used a
> custom "clock_khz" property until some guy told him to use
> clock-frequency ;)

I have sympathy for that guy. Very uncommon to specify IP core clocks
specifically in a dts ;)

My suggestion is:

1) if there is a clk node:
	- we get the clock rate via clock framework
	- "clock-frequency" is describing the bus speed as usual (Note
	  that parsing here can be as simple as checking for 100kHz only.
	  Although a seperate patch could probably easily add support for
	  other bus speeds to)

2?) a new binding is present to specify the IP clock speed:
	- is this needed? is somebody using the driver without CCF?
	- if so, the new binding is parsed and evaluated
	- I couldn't find an existing binding to specify a clock speed.
	  Please have a look, too. Otherwise we need to introduce sth
	  like "opencores,ip-clock-khz" probably.
	- "clock-frequency" is describing the bus speed as usual

3) only "clock-frequency" is present:
	- we keep the current behaviour to be backwards compatible.
	- driver should emit a warning to convert to new style
	- must be marked deprecated everywhere

The documentation should be updated accordingly.

Thoughts?

Attachment: signature.asc
Description: Digital signature