On Thu, Sep 27, 2012 at 11:44:25PM -0700, Kuninori Morimoto wrote:
> R-Car I2C is similar with SH7760 I2C.
> But the SH7760 I2C driver had many workaround operations, since H/W had bugs.
> Thus, it was pointless to keep compatible between SH7760 and R-Car I2C drivers.
> This patch creates new Renesas R-Car I2C driver.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
Only minor issues which might be fixed later. Applied to -next.
> +static void rcar_i2c_write(struct rcar_i2c_priv *priv, int reg, u32 val)
> +{
> + writel(val, priv->io + reg);
> +}
> +
> +static u32 rcar_i2c_read(struct rcar_i2c_priv *priv, int reg)
> +{
> + return readl(priv->io + reg);
> +}
Not sure if those needed encapsulation.
...
> +static int rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
> +{
> + struct i2c_msg *msg = priv->msg;
> +
> + /*
> + * FIXME
> + * sometimes, unknown interrupt happened.
> + * Do nothing
> + */
> + if (!(msr & MDE))
> + return 0;
Uh, is this a known hardware flaw?
...
> +static int __devinit rcar_i2c_probe(struct platform_device *pdev)
> +{
> + struct i2c_rcar_platform_data *pdata = pdev->dev.platform_data;
> + struct rcar_i2c_priv *priv;
> + struct i2c_adapter *adap;
> + struct resource *res;
> + struct device *dev = &pdev->dev;
> + u32 bus_speed;
> + int ret;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(dev, "no mmio resources\n");
> + return -ENODEV;
> + }
> +
> + priv = devm_kzalloc(dev, sizeof(struct rcar_i2c_priv), GFP_KERNEL);
> + if (!priv) {
> + dev_err(dev, "no mem for private data\n");
> + return -ENOMEM;
> + }
> +
> + bus_speed = 100000; /* default 100 kHz */
> + if (pdata && pdata->bus_speed)
> + bus_speed = pdata->bus_speed;
> + ret = rcar_i2c_clock_calculate(priv, bus_speed, dev);
> + if (ret < 0)
> + return ret;
> +
> + priv->io = devm_ioremap(dev, res->start, resource_size(res));
devm_request_mem_region is missing. Or use the helper
devm_request_and_ioremap.
> + if (!priv->io) {
> + dev_err(dev, "cannot ioremap\n");
> + return -ENODEV;
> + }
> +
> + priv->irq = platform_get_irq(pdev, 0);
> + init_waitqueue_head(&priv->wait);
> + spin_lock_init(&priv->lock);
> +
> + adap = &priv->adap;
> + adap->nr = pdev->id;
> + adap->algo = &rcar_i2c_algo;
> + adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
> + adap->retries = 3;
Do you really need the class?
Rest looks good.
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Attachment:
signature.asc
Description: Digital signature
