[v3,2/8] i2c: designware: Move register access detection to common code

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

Commit Message

Jarkko Nikula June 11, 2018, 2:22 p.m.
Move register access detection out from master and slave HW
initialization code to common code. Motivation for this is to have
register access configured before HW initialization and remove
duplicated code.

This allows to do further separation between probe time initialization
and runtime reinitialization code.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-common.c | 33 ++++++++++++++++++++++
 drivers/i2c/busses/i2c-designware-core.h   |  1 +
 drivers/i2c/busses/i2c-designware-master.c | 18 +++---------
 drivers/i2c/busses/i2c-designware-slave.c  | 18 +++---------
 4 files changed, 42 insertions(+), 28 deletions(-)

Comments

Andy Shevchenko June 11, 2018, 3:13 p.m. | #1
On Mon, 2018-06-11 at 17:22 +0300, Jarkko Nikula wrote:

> +	int ret = 0;

Redundant assignment.

> +
> +	ret = i2c_dw_acquire_lock(dev);
> +	if (ret)
> +		return ret;

+ blank line?

> +	reg = dw_readl(dev, DW_IC_COMP_TYPE);
> +	i2c_dw_release_lock(dev);
> +
> +	if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
> +		/* Configure register endianess access */
> +		dev->flags |= ACCESS_SWAP;
> +	} else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
> +		/* Configure register access mode 16bit */
> +		dev->flags |= ACCESS_16BIT;
> +	} else if (reg != DW_IC_COMP_TYPE_VALUE) {

> +		dev_err(dev->dev,
> +			"Unknown Synopsys component type: 0x%08x\n",
> reg);
> +		ret = -ENODEV;
> +	}
> +
> +	return ret;

Dunno if it makes sense

 ... {
   ...
   return -ENOVEV;
 }

return 0;

> +}
Jarkko Nikula June 12, 2018, 6:14 a.m. | #2
On 06/11/2018 06:13 PM, Andy Shevchenko wrote:
> On Mon, 2018-06-11 at 17:22 +0300, Jarkko Nikula wrote:
> 
>> +	int ret = 0;
> 
> Redundant assignment.
> 
Grr. I'm obviously blind to these.

>> +
>> +	ret = i2c_dw_acquire_lock(dev);
>> +	if (ret)
>> +		return ret;
> 
> + blank line?
> 
Will do.

>> +	reg = dw_readl(dev, DW_IC_COMP_TYPE);
>> +	i2c_dw_release_lock(dev);
>> +
>> +	if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
>> +		/* Configure register endianess access */
>> +		dev->flags |= ACCESS_SWAP;
>> +	} else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
>> +		/* Configure register access mode 16bit */
>> +		dev->flags |= ACCESS_16BIT;
>> +	} else if (reg != DW_IC_COMP_TYPE_VALUE) {
> 
>> +		dev_err(dev->dev,
>> +			"Unknown Synopsys component type: 0x%08x\n",
>> reg);
>> +		ret = -ENODEV;
>> +	}
>> +
>> +	return ret;
> 
> Dunno if it makes sense
> 
>   ... {
>     ...
>     return -ENOVEV;
>   }
> 
> return 0;
> 
Can change.

Patch

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 48914dfc8ce8..a9e75b809f38 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -94,6 +94,39 @@  void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
 	}
 }
 
+/**
+ * i2c_dw_set_reg_access() - Set register access flags
+ * @dev: device private data
+ *
+ * Autodetects needed register access mode and sets access flags accordingly.
+ * This must be called before doing any other register access.
+ */
+int i2c_dw_set_reg_access(struct dw_i2c_dev *dev)
+{
+	u32 reg;
+	int ret = 0;
+
+	ret = i2c_dw_acquire_lock(dev);
+	if (ret)
+		return ret;
+	reg = dw_readl(dev, DW_IC_COMP_TYPE);
+	i2c_dw_release_lock(dev);
+
+	if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
+		/* Configure register endianess access */
+		dev->flags |= ACCESS_SWAP;
+	} else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
+		/* Configure register access mode 16bit */
+		dev->flags |= ACCESS_16BIT;
+	} else if (reg != DW_IC_COMP_TYPE_VALUE) {
+		dev_err(dev->dev,
+			"Unknown Synopsys component type: 0x%08x\n", reg);
+		ret = -ENODEV;
+	}
+
+	return ret;
+}
+
 u32 i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset)
 {
 	/*
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index d690e648bc01..5d54f70710ad 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -295,6 +295,7 @@  struct dw_i2c_dev {
 
 u32 dw_readl(struct dw_i2c_dev *dev, int offset);
 void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset);
+int i2c_dw_set_reg_access(struct dw_i2c_dev *dev);
 u32 i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset);
 u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset);
 unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 27436a937492..3a7c184f24c8 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -64,20 +64,6 @@  static int i2c_dw_init_master(struct dw_i2c_dev *dev)
 	if (ret)
 		return ret;
 
-	reg = dw_readl(dev, DW_IC_COMP_TYPE);
-	if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
-		/* Configure register endianess access */
-		dev->flags |= ACCESS_SWAP;
-	} else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
-		/* Configure register access mode 16bit */
-		dev->flags |= ACCESS_16BIT;
-	} else if (reg != DW_IC_COMP_TYPE_VALUE) {
-		dev_err(dev->dev,
-			"Unknown Synopsys component type: 0x%08x\n", reg);
-		i2c_dw_release_lock(dev);
-		return -ENODEV;
-	}
-
 	comp_param1 = dw_readl(dev, DW_IC_COMP_PARAM_1);
 
 	/* Disable the adapter */
@@ -681,6 +667,10 @@  int i2c_dw_probe(struct dw_i2c_dev *dev)
 	dev->disable = i2c_dw_disable;
 	dev->disable_int = i2c_dw_disable_int;
 
+	ret = i2c_dw_set_reg_access(dev);
+	if (ret)
+		return ret;
+
 	ret = dev->init(dev);
 	if (ret)
 		return ret;
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index 2036a579b5df..a1f802001e1f 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -58,20 +58,6 @@  static int i2c_dw_init_slave(struct dw_i2c_dev *dev)
 	if (ret)
 		return ret;
 
-	reg = dw_readl(dev, DW_IC_COMP_TYPE);
-	if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
-		/* Configure register endianness access. */
-		dev->flags |= ACCESS_SWAP;
-	} else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
-		/* Configure register access mode 16bit. */
-		dev->flags |= ACCESS_16BIT;
-	} else if (reg != DW_IC_COMP_TYPE_VALUE) {
-		dev_err(dev->dev,
-			"Unknown Synopsys component type: 0x%08x\n", reg);
-		i2c_dw_release_lock(dev);
-		return -ENODEV;
-	}
-
 	/* Disable the adapter. */
 	__i2c_dw_disable(dev);
 
@@ -297,6 +283,10 @@  int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
 	dev->disable = i2c_dw_disable;
 	dev->disable_int = i2c_dw_disable_int;
 
+	ret = i2c_dw_set_reg_access(dev);
+	if (ret)
+		return ret;
+
 	ret = dev->init(dev);
 	if (ret)
 		return ret;