On 28-Dec-16 15:12, Andy Shevchenko wrote:
> On Wed, 2016-12-28 at 14:43 +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 mode.
>> - I have to take off DW_IC_INTR_TX_EMPTY from DW_IC_INTR_DEFAULT_MASK
>> because it is master specific.
>>
>> The purpose of this is to prepare the controller to have is I2C MASTER
>> flow in a separate driver. To do this first all the
>> functions/definitions related to the MASTER flow were identified.
>
> Thanks for an update.
> Some style related comments below (For the code related is up to you, my
> tag still stands).
>
>>
>> Signed-off-by: Luis Oliveira <loli...@synopsys.com>
>> ---
>> Changes V4->V5: (ACK by Andy)
>
> When you get an Ack, or other tag (Reviewed-by, Tested-by, etc), and you
> send new version, include this tag to your commit message (it applies to
> all affected patches in your series).
>
Thank you. I didn't knew.
> It would be also good to have some high level changelog in the cover
> letter, from this series I don't see, for example, which base you did
> use (i2c-next? linux-next? v4.9? v4.10-rc1?).
>
>> + dev_dbg(dev->dev,
>> + "%s: enabled=%#x stat=%#x\n", __func__, enabled,
> stat);
>
> I hope you can fit format string on the first line. __func__ is
> redundant when you are using debug printing (Dynamic Debug would include
> it if asked for).
I will check that.
>
>> +static void i2c_dw_configure_master(struct platform_device *pdev)
>> +{
>> + struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
>
> By the way, does it make sense to pass struct dw_i2c_dev * as a
> parameter of the function?
>
Yes, by looking at it now I think I can pass just the struct dw_i2c_dev
to this function. And probably the same with the i2c_dw_configure_slave.
>> +
>> + dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE
>> |
>> + DW_IC_CON_RESTART_EN;
>> +
>> + dev_dbg(&pdev->dev, "I am registed as a I2C Master!\n");
>> +
>> + switch (dev->clk_freq) {
>> + case 100000:
>> + dev->master_cfg |= DW_IC_CON_SPEED_STD;
>> + break;
>> + case 3400000:
>> + dev->master_cfg |= DW_IC_CON_SPEED_HIGH;
>> + break;
>> + default:
>> + dev->master_cfg |= DW_IC_CON_SPEED_FAST;
>> + }
>> +}
>> +
>>
>
>
