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

[Wolfram Sang]

On Thu, Jan 22, 2015 at 11:53:51PM +0300, Max Filippov wrote:
> On Thu, Jan 22, 2015 at 10:26 PM, Wolfram Sang <wsa@xxxxxxxxxxxxx> wrote:
> > On Thu, Jan 22, 2015 at 10:15:40PM +0300, Max Filippov wrote:
> >> On Thu, Jan 22, 2015 at 9:57 PM, Wolfram Sang <wsa@xxxxxxxxxxxxx> wrote:
> >> > 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?
> >>
> >> I can update my patch to do (1) and (3), leaving (2) to whoever may
> >> need that.
> >
> > Please implement (2) as well. Otherwise we would have documented
> > ambiguity of "clock-frequency" which is bad. It shouldn't be much code.
> 
> There will be ambiguity if we want to maintain backwards compatibility,
> so we're documenting it anyway. And since we're going to maintain it,
> those who don't use CCF will still have option (3). Is there a point in
> introducing (2) which currently does not exist and thus has no users?

Yes, because 3) is marked as deprecated "DON'T USE". If you don't
implement 2), then we force people to actually use the ambiguity!

Attachment: signature.asc
Description: Digital signature