mbox series

[v3,0/8] i2c: designware: Improve debug prints

Message ID 20180611142248.10798-1-jarkko.nikula@linux.intel.com
Headers show
Series i2c: designware: Improve debug prints | expand

Message

Jarkko Nikula June 11, 2018, 2:22 p.m. UTC
Motivation here is to improve debug prints and while doing so to remove
some duplication and separate timing parameter validation from actual
register writes as this parameter validation and/or calculation is
needed to do only during probe.

Currently debug code prints SCL timing parameters whenever HW is
reinitialized but doesn't print importand SDA hold time nor actual bus
speed the controller is going to operate.

v3:
- Fail always if i2c_dw_acquire_lock() returns an error.
- Keep include statement order
- Remove redundant "ret = 0; " assignment from i2c_dw_set_sda_hold()

v2:
- SDA hold time configuration moved from "Separate timing parameter
  setting from HW initalization" patch to a new patch as well as
  i2c_dw_clk_rate() cleanup.
- New patch replacing ___constant_swab32() with swab32().
- Added Andys review by tags to patches 1-2.

Jarkko Nikula (8):
  i2c: designware: Remove needless variable from i2c_dw_init_slave()
  i2c: designware: Move register access detection to common code
  i2c: designware: Don't use internal ___constant_swab32
  i2c: designware: Call i2c_dw_clk_rate() only once in
    i2c_dw_init_master()
  i2c: designware: Move SDA hold time configuration to common code
  i2c: designware: Separate timing parameter setting from HW
    initalization
  i2c: designware: Add debug print for SDA hold time value
  i2c: designware: Add debug print for bus speed

 drivers/i2c/busses/i2c-designware-common.c |  75 ++++++++
 drivers/i2c/busses/i2c-designware-core.h   |   2 +
 drivers/i2c/busses/i2c-designware-master.c | 190 ++++++++++++---------
 drivers/i2c/busses/i2c-designware-slave.c  |  46 +----
 4 files changed, 195 insertions(+), 118 deletions(-)

Comments

Andy Shevchenko June 11, 2018, 3:11 p.m. UTC | #1
On Mon, 2018-06-11 at 17:22 +0300, Jarkko Nikula wrote:
> Motivation here is to improve debug prints and while doing so to
> remove
> some duplication and separate timing parameter validation from actual
> register writes as this parameter validation and/or calculation is
> needed to do only during probe.
> 
> Currently debug code prints SCL timing parameters whenever HW is
> reinitialized but doesn't print importand SDA hold time nor actual bus
> speed the controller is going to operate.
> 
> v3:
> - Fail always if i2c_dw_acquire_lock() returns an error.
> - Keep include statement order

> - Remove redundant "ret = 0; " assignment from i2c_dw_set_sda_hold()

Still there...

> 
> v2:
> - SDA hold time configuration moved from "Separate timing parameter
>   setting from HW initalization" patch to a new patch as well as
>   i2c_dw_clk_rate() cleanup.
> - New patch replacing ___constant_swab32() with swab32().
> - Added Andys review by tags to patches 1-2.
> 
> Jarkko Nikula (8):
>   i2c: designware: Remove needless variable from i2c_dw_init_slave()
>   i2c: designware: Move register access detection to common code
>   i2c: designware: Don't use internal ___constant_swab32
>   i2c: designware: Call i2c_dw_clk_rate() only once in
>     i2c_dw_init_master()
>   i2c: designware: Move SDA hold time configuration to common code
>   i2c: designware: Separate timing parameter setting from HW
>     initalization
>   i2c: designware: Add debug print for SDA hold time value
>   i2c: designware: Add debug print for bus speed
> 
>  drivers/i2c/busses/i2c-designware-common.c |  75 ++++++++
>  drivers/i2c/busses/i2c-designware-core.h   |   2 +
>  drivers/i2c/busses/i2c-designware-master.c | 190 ++++++++++++------
> ---
>  drivers/i2c/busses/i2c-designware-slave.c  |  46 +----
>  4 files changed, 195 insertions(+), 118 deletions(-)
>
Andy Shevchenko June 11, 2018, 3:15 p.m. UTC | #2
On Mon, 2018-06-11 at 18:11 +0300, Andy Shevchenko wrote:
> On Mon, 2018-06-11 at 17:22 +0300, Jarkko Nikula wrote:
> > Motivation here is to improve debug prints and while doing so to
> > remove
> > some duplication and separate timing parameter validation from
> > actual
> > register writes as this parameter validation and/or calculation is
> > needed to do only during probe.
> > 
> > Currently debug code prints SCL timing parameters whenever HW is
> > reinitialized but doesn't print importand SDA hold time nor actual
> > bus
> > speed the controller is going to operate.
> > 
> > v3:
> > - Fail always if i2c_dw_acquire_lock() returns an error.
> > - Keep include statement order
> > - Remove redundant "ret = 0; " assignment from i2c_dw_set_sda_hold()
> 
> Still there...
> 

Ah, it is another one. Sorry, didn't notice before.
Andy Shevchenko June 11, 2018, 3:22 p.m. UTC | #3
On Mon, 2018-06-11 at 17:22 +0300, Jarkko Nikula wrote:
> Motivation here is to improve debug prints and while doing so to
> remove
> some duplication and separate timing parameter validation from actual
> register writes as this parameter validation and/or calculation is
> needed to do only during probe.
> 
> Currently debug code prints SCL timing parameters whenever HW is
> reinitialized but doesn't print importand SDA hold time nor actual bus
> speed the controller is going to operate.
> 

So, after addressing the recent comments

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

for the entire series.

> v3:
> - Fail always if i2c_dw_acquire_lock() returns an error.
> - Keep include statement order
> - Remove redundant "ret = 0; " assignment from i2c_dw_set_sda_hold()
> 
> v2:
> - SDA hold time configuration moved from "Separate timing parameter
>   setting from HW initalization" patch to a new patch as well as
>   i2c_dw_clk_rate() cleanup.
> - New patch replacing ___constant_swab32() with swab32().
> - Added Andys review by tags to patches 1-2.
> 
> Jarkko Nikula (8):
>   i2c: designware: Remove needless variable from i2c_dw_init_slave()
>   i2c: designware: Move register access detection to common code
>   i2c: designware: Don't use internal ___constant_swab32
>   i2c: designware: Call i2c_dw_clk_rate() only once in
>     i2c_dw_init_master()
>   i2c: designware: Move SDA hold time configuration to common code
>   i2c: designware: Separate timing parameter setting from HW
>     initalization
>   i2c: designware: Add debug print for SDA hold time value
>   i2c: designware: Add debug print for bus speed
> 
>  drivers/i2c/busses/i2c-designware-common.c |  75 ++++++++
>  drivers/i2c/busses/i2c-designware-core.h   |   2 +
>  drivers/i2c/busses/i2c-designware-master.c | 190 ++++++++++++------
> ---
>  drivers/i2c/busses/i2c-designware-slave.c  |  46 +----
>  4 files changed, 195 insertions(+), 118 deletions(-)
>