Hi Phil
Thank you for your comments
> > +static void rcar_i2c_write(struct rcar_i2c_priv *priv, int reg, u32
> val)
> > +{
> > + __raw_writel(val, priv->io + reg);
> > +}
> > +
> > +static u32 rcar_i2c_read(struct rcar_i2c_priv *priv, int reg)
> > +{
> > + return __raw_readl(priv->io + reg);
> > +}
> I think we should use writel/readl here. We have a number of devices where
> the virtual address map conflicts with register addresses for other
> peripherals.
OK. I will fix it.
> > +static void rcar_i2c_bus_phase(struct rcar_i2c_priv *priv, int phase)
> > +{
> > + switch (phase) {
> > + case RCAR_BUS_PHASE_ADDR:
> > + rcar_i2c_write(priv, ICMCR, MDBS | MIE | ESG);
> > + break;
> > + case RCAR_BUS_PHASE_DATA:
> > + rcar_i2c_write(priv, ICMCR, MDBS | MIE);
> > + break;
> > + case RCAR_BUS_PHASE_STOP:
> > + rcar_i2c_write(priv, ICMCR, MDBS | MIE | FSB);
> > + break;
> > + }
> > +
> > + return;
> > +}
> Don't need the return.
indeed. thanks
> > + /*
> > + * use 95% bus speed for safety.
> > + */
> > + bus_speed = bus_speed * 95 / 100;
> Why do you use 95% of the bus speed? Surely, either the hardware supports
> a specific speed or it doesn't.
(snip)
> > + /*
> > + * it is impossible to calculate large scale
> > + * number on u32
> > + *
> > + * F[(ticf + tr + intd) * ick]
> > + * = F[(35 + 200 + 50)ns * ick]
> > + * = F[285 * ick / 1000000000]
> > + * = F[(ick / 1000000) * 285 / 1000]
> > + */
> > + round = (ick + 500000) / 1000000 * 285;
> > + round = (round + 500) / 1000;
> Now that I see this, I guess that you used 95% bus clock due to rounding
> errors here. If so, maybe it would be better to try to improve this
> calculation.
Indeed, thanks.
> typo: happend => happened
> typo: finised => finished
> typo: happend => happened
Haha :)
Sorry for my English.
> > +static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
> > + struct i2c_msg *msgs,
> > + int num)
> > +{
> > + struct rcar_i2c_priv *priv = i2c_get_adapdata(adap);
> > + struct device *dev = rcar_i2c_priv_to_dev(priv);
> > + unsigned long flags;
> > + int i, ret, timeout;
> > +
> > + /*================== enable i2c device ===================*/
> > + pm_runtime_get_sync(dev);
(snip)
> > + pm_runtime_put_sync(dev);
> > + /*================== disable i2c device ===================*/
> This comment should be above the previous line, unless you the comment is
> meant to say "i2c device disabled"
I see.
Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
