| Message ID | 20180529112754.13477-6-jarkko.nikula@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | i2c: designware: Improve debug prints | expand |
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); > } > > /**
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.
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); } /**
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(+)