[5/5] i2c: designware: Add debug print for bus speed

Message ID 20180529112754.13477-6-jarkko.nikula@linux.intel.com
State Superseded
Headers show
Series
  • i2c: designware: Improve debug prints
Related show

Commit Message

Jarkko Nikula May 29, 2018, 11:27 a.m.
Trivial added debug print for dev->clk_freq doesn't necessarily tell the
actual bus speed or mode the controller is operating. For instance it
may indicate 1 MHz Fast Mode Plus or 3.4 MHz High Speed but driver ends up
using 400 kHz Fast Mode due missing timing parameters or missing support
from HW.

Add a debug print that prints the bus speed based on the validated speed
that gets programmed into a HW.

Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-master.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Andy Shevchenko May 29, 2018, 1:54 p.m. | #1
On Tue, 2018-05-29 at 14:27 +0300, Jarkko Nikula wrote:
> Trivial added debug print for dev->clk_freq doesn't necessarily tell
> the
> actual bus speed or mode the controller is operating. For instance it
> may indicate 1 MHz Fast Mode Plus or 3.4 MHz High Speed but driver
> ends up
> using 400 kHz Fast Mode due missing timing parameters or missing
> support
> from HW.
> 
> Add a debug print that prints the bus speed based on the validated
> speed
> that gets programmed into a HW.
> 
> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> ---
>  drivers/i2c/busses/i2c-designware-master.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-master.c
> b/drivers/i2c/busses/i2c-designware-master.c
> index b47aa919b377..af8dd4017a2d 100644
> --- a/drivers/i2c/busses/i2c-designware-master.c
> +++ b/drivers/i2c/busses/i2c-designware-master.c
> @@ -49,6 +49,7 @@ static void i2c_dw_set_timings_master(struct
> dw_i2c_dev *dev)
>  {
>  	u32 comp_param1;
>  	u32 sda_falling_time, scl_falling_time;
> +	const char *mode_str;
>  	int ret = 0;
>  
>  	ret = i2c_dw_acquire_lock(dev);
> @@ -133,6 +134,21 @@ static void i2c_dw_set_timings_master(struct
> dw_i2c_dev *dev)
>  	}
>  
>  	i2c_dw_set_sda_hold(dev);
> +
> +	switch (dev->master_cfg & DW_IC_CON_SPEED_MASK) {
> +	case DW_IC_CON_SPEED_STD:
> +		mode_str = "Standard Mode";
> +		break;
> +	case DW_IC_CON_SPEED_HIGH:
> +		mode_str = "High Speed Mode";
> +		break;
> +	default:

> +		if (dev->clk_freq == 1000000)
> +			mode_str = "Fast Mode Plus";
> +		else
> +			mode_str = "Fast Mode";

I don't remember the style used for these modes elsewhere in the driver,
though here it feels like switch-case would be more suitable.

> +	}
> +	dev_dbg(dev->dev, "Bus speed: %s\n", mode_str);
>  }
>  
>  /**
Jarkko Nikula May 30, 2018, 6:43 a.m. | #2
On 05/29/2018 04:54 PM, Andy Shevchenko wrote:
> On Tue, 2018-05-29 at 14:27 +0300, Jarkko Nikula wrote:
>> Trivial added debug print for dev->clk_freq doesn't necessarily tell
>> the
>> actual bus speed or mode the controller is operating. For instance it
>> may indicate 1 MHz Fast Mode Plus or 3.4 MHz High Speed but driver
>> ends up
>> using 400 kHz Fast Mode due missing timing parameters or missing
>> support
>> from HW.
>>
>> Add a debug print that prints the bus speed based on the validated
>> speed
>> that gets programmed into a HW.
>>
>> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
>> ---
>>   drivers/i2c/busses/i2c-designware-master.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-designware-master.c
>> b/drivers/i2c/busses/i2c-designware-master.c
>> index b47aa919b377..af8dd4017a2d 100644
>> --- a/drivers/i2c/busses/i2c-designware-master.c
>> +++ b/drivers/i2c/busses/i2c-designware-master.c
>> @@ -49,6 +49,7 @@ static void i2c_dw_set_timings_master(struct
>> dw_i2c_dev *dev)
>>   {
>>   	u32 comp_param1;
>>   	u32 sda_falling_time, scl_falling_time;
>> +	const char *mode_str;
>>   	int ret = 0;
>>   
>>   	ret = i2c_dw_acquire_lock(dev);
>> @@ -133,6 +134,21 @@ static void i2c_dw_set_timings_master(struct
>> dw_i2c_dev *dev)
>>   	}
>>   
>>   	i2c_dw_set_sda_hold(dev);
>> +
>> +	switch (dev->master_cfg & DW_IC_CON_SPEED_MASK) {
>> +	case DW_IC_CON_SPEED_STD:
>> +		mode_str = "Standard Mode";
>> +		break;
>> +	case DW_IC_CON_SPEED_HIGH:
>> +		mode_str = "High Speed Mode";
>> +		break;
>> +	default:
> 
>> +		if (dev->clk_freq == 1000000)
>> +			mode_str = "Fast Mode Plus";
>> +		else
>> +			mode_str = "Fast Mode";
> 
> I don't remember the style used for these modes elsewhere in the driver,
> though here it feels like switch-case would be more suitable.
> 
HW has only three modes (standard, fast/fm+ and high speed) and only 
difference between fast mode and fast mode plus are the timing parameter 
values. That's the reason for if statement inside the default case here.

Patch

diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index b47aa919b377..af8dd4017a2d 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -49,6 +49,7 @@  static void i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
 {
 	u32 comp_param1;
 	u32 sda_falling_time, scl_falling_time;
+	const char *mode_str;
 	int ret = 0;
 
 	ret = i2c_dw_acquire_lock(dev);
@@ -133,6 +134,21 @@  static void i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
 	}
 
 	i2c_dw_set_sda_hold(dev);
+
+	switch (dev->master_cfg & DW_IC_CON_SPEED_MASK) {
+	case DW_IC_CON_SPEED_STD:
+		mode_str = "Standard Mode";
+		break;
+	case DW_IC_CON_SPEED_HIGH:
+		mode_str = "High Speed Mode";
+		break;
+	default:
+		if (dev->clk_freq == 1000000)
+			mode_str = "Fast Mode Plus";
+		else
+			mode_str = "Fast Mode";
+	}
+	dev_dbg(dev->dev, "Bus speed: %s\n", mode_str);
 }
 
 /**