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

From: Wolfram Sang
Date: Thu Jan 22 2015 - 13:57:23 EST


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