On Wed, 2016-12-07 at 17:55 +0000, Luis Oliveira wrote:
> - Factor out all _master() part of code from i2c-designware-core
>   and i2c-designware-platdrv to separate functions.
> - Standardize all code related with MASTER modes.
> 

Couple of comments, after addressing them
Acked-by: Andy Shevchenko <[email protected]>


> +     if ((dev->master_cfg & DW_IC_CON_MASTER) &&
> +              (dev->master_cfg & DW_IC_CON_SLAVE_DISABLE))
> +             i2c_dw_configure_fifo_master(dev);

So, logically it's a part of slave patch.
For now it would be just 
 i2c_dw_configure_fifo_master(dev);

> +static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
> +{
> +     struct dw_i2c_dev *dev = dev_id;
> +     u32 stat, enabled, mode;

mode is unused here, this is a part of slave patch either.

> +static void i2c_dw_configure_master(struct platform_device *pdev)
> +{
> +     struct dw_i2c_dev *dev = platform_get_drvdata(pdev);

> +     dev_info(&pdev->dev, "I am registed as a I2C Master!\n");

I don't want bikeshedding here, but the question just comes:
"Do we need to have this available via sysfs as a part of ABI?" So. user
space can check for / set a mode.

In any case this one is a separate story and another patch, here just to
make the message less annoying, it looks like dev_dbg() to me.

-- 
Andy Shevchenko <[email protected]>
Intel Finland Oy

Reply via email to