Re: [PATCH v2 4/8] i2c: designware: Move SDA hold time configuration to common code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/04/2018 03:42 PM, Andy Shevchenko wrote:
On Fri, 2018-06-01 at 16:47 +0300, Jarkko Nikula wrote:
SDA hold time configuration is common to both master and slave code.
It
is also something that can be done once during probe and do only
register write when HW needs to be reinitialized.

Remove duplication and move SDA hold time configuration to common
code.
It will be called from slave probe and for master code from a new
i2c_dw_set_timings_master() to where we will populate more probe time
timing parameter setting.


Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

(Some minor issues / questions below)

...
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -45,6 +45,11 @@ static void i2c_dw_configure_fifo_master(struct
dw_i2c_dev *dev)
  	dw_writel(dev, dev->master_cfg, DW_IC_CON);
  }
+static void i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
+{
+	i2c_dw_set_sda_hold(dev);
+}
+

Looking into patch 6 I'm not sure this split is required here and not
there.

Have you checked which one looks better?

Originally thought keeping this and patch 6/8 in one patch but started liking more this separation after you asked about it :-)

I noticed from your 6/8 comment that I need to return error code also from i2c_dw_set_sda_hold() as we attempt to do locking there too. I'd like to keep this separate from 6/8 for simplicity but move 5/8 before this and 6/8.

--
Jarkko



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux