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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150122185711.GA14880@katana>
Date:	Thu, 22 Jan 2015 19:57:12 +0100
From:	Wolfram Sang <wsa@...-dreams.de>
To:	Peter Korsgaard <peter@...sgaard.com>
Cc:	Max Filippov <jcmvbkbc@...il.com>,
	Peter Korsgaard <jacmet@...site.dk>, linux-i2c@...r.kernel.org,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] i2c-ocores: add common clock support

On Thu, Jan 22, 2015 at 07:28:21PM +0100, Peter Korsgaard wrote:
> >>>>> "Wolfram" == Wolfram Sang <wsa@...-dreams.de> 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?


Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)