[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150520071152.GP24769@pengutronix.de>
Date: Wed, 20 May 2015 09:11:52 +0200
From: Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>
To: Eddie Huang <eddie.huang@...iatek.com>
Cc: Mark Rutland <mark.rutland@....com>,
Xudong Chen <xudong.chen@...iatek.com>,
srv_heupstream@...iatek.com, Pawel Moll <pawel.moll@....com>,
Ian Campbell <ijc+devicetree@...lion.org.uk>,
Wolfram Sang <wsa@...-dreams.de>,
Liguo Zhang <liguo.zhang@...iatek.com>,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
Rob Herring <robh+dt@...nel.org>,
linux-mediatek@...ts.infradead.org, linux-i2c@...r.kernel.org,
Sascha Hauer <kernel@...gutronix.de>,
Kumar Gala <galak@...eaurora.org>,
Matthias Brugger <matthias.bgg@...il.com>,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v8 2/3] I2C: mediatek: Add driver for MediaTek I2C
controller
Hello Eddie,
On Wed, May 20, 2015 at 10:40:11AM +0800, Eddie Huang wrote:
> On Mon, 2015-05-18 at 20:43 +0200, Uwe Kleine-König wrote:
> > On Tue, May 19, 2015 at 12:40:08AM +0800, Eddie Huang wrote:
> > > +/* calculate i2c port speed */
> > It would be nice to summarize the clock frequency settings here.
> > Something like:
> >
> > /*
> > * The input clock is divided by the value specified in the
> > * device tree as clock-div. The actual bus speed is then
> > * derived from this frequency by the following formula:
> > * ....
> >
> > This would make it possible to verify your calculations below.
>
> The comment will be:
> /*
> * khz: I2C bus clock
> * hclk: The input clock is divided by the value specified in the
> * device tree as clock-div
and which one of the two clocks you're writing about is hclk now? I
assume the divided one.
> * div = (sample_cnt + 1) * (step_cnt + 1)
> * khz = (hclk / 2) / div
khz for the 2nd time.
> *
> * The calculation is to get div value that let result of
> * ((hclk / 2) / div) most approach and less than khz
> */
I imagined something more hardware related. A list of register (or
register bit fields) that influence the frequency and a formula
i2c_freq = parent_clk / clock-div * (...)
(It seems to be a bit more complicated here as there are two registers
involved that are set differently depending on the target frequency.)
> > > +static int mtk_i2c_set_speed(struct mtk_i2c *i2c, unsigned int clk_src_in_hz)
clk_src_in_hz is the module's input rate already divided by clock-div.
This clock-div value is fixed in hardware and unchangeable, right?
Maybe give that divided clock a nice name?
The target frequency is i2c->speed_hz, so among the possible frequencies
we want to pick the highest one that is still less than or equal
i2c->speed_hz, right?
> > > + /* Set the hign speed mode register */
I just notice s/hign/high/ here.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists