[v5,2/7] i2c: designware: refactoring of the i2c-designware

- 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.

Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
---
Changes V4->V5: (ACK by Andy)
Fixed the bellow issues:
- Removed a part belonging to slave patch  
- Removed unused variable "mode"
- Changed dev_info() to dev_dbg()

 drivers/i2c/busses/i2c-designware-core.c    | 56 ++++++++++++++++++-----------
 drivers/i2c/busses/i2c-designware-platdrv.c | 35 +++++++++++-------
 2 files changed, 58 insertions(+), 33 deletions(-)

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 <lolivei@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).

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).

> +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?

> +
> +	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;
> +	}
> +}
> +
>

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 <lolivei@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;
>> +	}
>> +}
>> +
>>
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html