linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2 1/2] i2c: cadence: Fix the Documentation
@ 2016-03-01  9:21 Shubhrajyoti Datta
  2016-03-01  9:21 ` [PATCHv2 2/2] i2c: cadence: Implement save restore Shubhrajyoti Datta
  2016-03-02  8:22 ` [PATCHv2 1/2] i2c: cadence: Fix the Documentation Michal Simek
  0 siblings, 2 replies; 6+ messages in thread
From: Shubhrajyoti Datta @ 2016-03-01  9:21 UTC (permalink / raw)
  To: linux-i2c; +Cc: soren.brinkmann, michal.simek, wsa, Shubhrajyoti Datta

The runtime adaptation patch missed the documentation update.
This fixes the same.

Signed-off-by: Shubhrajyoti Datta <shubhraj@xilinx.com>
---
v2 changes:
fix the dev param description

 drivers/i2c/busses/i2c-cadence.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 6b08d16..f9f485d 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -124,6 +124,8 @@
 
 /**
  * struct cdns_i2c - I2C device private data structure
+ *
+ * @dev:		Pointer to device structure
  * @membase:		Base address of the I2C device
  * @adap:		I2C adapter instance
  * @p_msg:		Message pointer
@@ -815,8 +817,8 @@ static int cdns_i2c_clk_notifier_cb(struct notifier_block *nb, unsigned long
 }
 
 /**
- * cdns_i2c_suspend - Suspend method for the driver
- * @_dev:	Address of the platform_device structure
+ * cdns_i2c_runtime_suspend -  Runtime suspend method for the driver
+ * @dev:	Address of the platform_device structure
  *
  * Put the driver into low power mode.
  *
@@ -833,10 +835,10 @@ static int __maybe_unused cdns_i2c_runtime_suspend(struct device *dev)
 }
 
 /**
- * cdns_i2c_resume - Resume from suspend
- * @_dev:	Address of the platform_device structure
+ * cdns_i2c_runtime_resume - Runtime resume
+ * @dev:	Address of the platform_device structure
  *
- * Resume operation after suspend.
+ * Runtime resume callback.
  *
  * Return: 0 on success and error value on error
  */
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCHv2 2/2] i2c: cadence: Implement save restore
  2016-03-01  9:21 [PATCHv2 1/2] i2c: cadence: Fix the Documentation Shubhrajyoti Datta
@ 2016-03-01  9:21 ` Shubhrajyoti Datta
  2016-03-02  8:32   ` Michal Simek
  2016-03-02  8:22 ` [PATCHv2 1/2] i2c: cadence: Fix the Documentation Michal Simek
  1 sibling, 1 reply; 6+ messages in thread
From: Shubhrajyoti Datta @ 2016-03-01  9:21 UTC (permalink / raw)
  To: linux-i2c; +Cc: soren.brinkmann, michal.simek, wsa, Shubhrajyoti Datta

Implement save restore for i2c module.
Since we have only a couple of registers
an unconditional restore is done.

Signed-off-by: Shubhrajyoti Datta <shubhraj@xilinx.com>
---
zynq-mp has the capability of going off.
the current kernel does not hit off however some day it will.
since the overhead of having the support is not much may be it is better to have it in the kernel.

v2 changes:
add kernel doc for ctrl_reg and cdns_i2c_init

 drivers/i2c/busses/i2c-cadence.c |   41 +++++++++++++++++++++++++-------------
 1 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index f9f485d..88d2c8b 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -143,6 +143,7 @@
  * @clk:		Pointer to struct clk
  * @clk_rate_change_nb:	Notifier block for clock rate changes
  * @quirks:		flag for broken hold bit usage in r1p10
+ * @ctrl_reg:		Cached value of the control register.
  */
 struct cdns_i2c {
 	struct device		*dev;
@@ -163,6 +164,7 @@ struct cdns_i2c {
 	struct clk *clk;
 	struct notifier_block clk_rate_change_nb;
 	u32 quirks;
+	u32 ctrl_reg;
 };
 
 struct cdns_platform_data {
@@ -745,12 +747,11 @@ static int cdns_i2c_setclk(unsigned long clk_in, struct cdns_i2c *id)
 	if (ret)
 		return ret;
 
-	ctrl_reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET);
+	ctrl_reg = id->ctrl_reg;
 	ctrl_reg &= ~(CDNS_I2C_CR_DIVA_MASK | CDNS_I2C_CR_DIVB_MASK);
 	ctrl_reg |= ((div_a << CDNS_I2C_CR_DIVA_SHIFT) |
 			(div_b << CDNS_I2C_CR_DIVB_SHIFT));
-	cdns_i2c_writereg(ctrl_reg, CDNS_I2C_CR_OFFSET);
-
+	id->ctrl_reg = ctrl_reg;
 	return 0;
 }
 
@@ -835,6 +836,26 @@ static int __maybe_unused cdns_i2c_runtime_suspend(struct device *dev)
 }
 
 /**
+ * cdns_i2c_init -  Controller initialisation
+ * @id:		Device private data structure
+ *
+ * Initialise the i2c controller.
+ *
+ */
+static void cdns_i2c_init(struct cdns_i2c *id)
+{
+	cdns_i2c_writereg(id->ctrl_reg, CDNS_I2C_CR_OFFSET);
+	/*
+	 * Cadence I2C controller has a bug wherein it generates
+	 * invalid read transaction after HW timeout in master receiver mode.
+	 * HW timeout is not used by this driver and the interrupt is disabled.
+	 * But the feature itself cannot be disabled. Hence maximum value
+	 * is written to this register to reduce the chances of error.
+	 */
+	cdns_i2c_writereg(CDNS_I2C_TIMEOUT_MAX, CDNS_I2C_TIME_OUT_OFFSET);
+}
+
+/**
  * cdns_i2c_runtime_resume - Runtime resume
  * @dev:	Address of the platform_device structure
  *
@@ -853,6 +874,7 @@ static int __maybe_unused cdns_i2c_runtime_resume(struct device *dev)
 		dev_err(dev, "Cannot enable clock.\n");
 		return ret;
 	}
+	cdns_i2c_init(xi2c);
 
 	return 0;
 }
@@ -945,8 +967,7 @@ static int cdns_i2c_probe(struct platform_device *pdev)
 	if (ret || (id->i2c_clk > CDNS_I2C_SPEED_MAX))
 		id->i2c_clk = CDNS_I2C_SPEED_DEFAULT;
 
-	cdns_i2c_writereg(CDNS_I2C_CR_ACK_EN | CDNS_I2C_CR_NEA | CDNS_I2C_CR_MS,
-			  CDNS_I2C_CR_OFFSET);
+	id->ctrl_reg = CDNS_I2C_CR_ACK_EN | CDNS_I2C_CR_NEA | CDNS_I2C_CR_MS;
 
 	ret = cdns_i2c_setclk(id->input_clk, id);
 	if (ret) {
@@ -967,15 +988,7 @@ static int cdns_i2c_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "reg adap failed: %d\n", ret);
 		goto err_clk_dis;
 	}
-
-	/*
-	 * Cadence I2C controller has a bug wherein it generates
-	 * invalid read transaction after HW timeout in master receiver mode.
-	 * HW timeout is not used by this driver and the interrupt is disabled.
-	 * But the feature itself cannot be disabled. Hence maximum value
-	 * is written to this register to reduce the chances of error.
-	 */
-	cdns_i2c_writereg(CDNS_I2C_TIMEOUT_MAX, CDNS_I2C_TIME_OUT_OFFSET);
+	cdns_i2c_init(id);
 
 	dev_info(&pdev->dev, "%u kHz mmio %08lx irq %d\n",
 		 id->i2c_clk / 1000, (unsigned long)r_mem->start, id->irq);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 1/2] i2c: cadence: Fix the Documentation
  2016-03-01  9:21 [PATCHv2 1/2] i2c: cadence: Fix the Documentation Shubhrajyoti Datta
  2016-03-01  9:21 ` [PATCHv2 2/2] i2c: cadence: Implement save restore Shubhrajyoti Datta
@ 2016-03-02  8:22 ` Michal Simek
  2016-03-02  9:01   ` Wolfram Sang
  1 sibling, 1 reply; 6+ messages in thread
From: Michal Simek @ 2016-03-02  8:22 UTC (permalink / raw)
  To: Shubhrajyoti Datta, linux-i2c
  Cc: soren.brinkmann, michal.simek, wsa, Shubhrajyoti Datta

On 1.3.2016 10:21, Shubhrajyoti Datta wrote:
> The runtime adaptation patch missed the documentation update.
> This fixes the same.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhraj@xilinx.com>
> ---
> v2 changes:
> fix the dev param description
> 
>  drivers/i2c/busses/i2c-cadence.c |   12 +++++++-----
>  1 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
> index 6b08d16..f9f485d 100644
> --- a/drivers/i2c/busses/i2c-cadence.c
> +++ b/drivers/i2c/busses/i2c-cadence.c
> @@ -124,6 +124,8 @@
>  
>  /**
>   * struct cdns_i2c - I2C device private data structure
> + *
> + * @dev:		Pointer to device structure
>   * @membase:		Base address of the I2C device
>   * @adap:		I2C adapter instance
>   * @p_msg:		Message pointer
> @@ -815,8 +817,8 @@ static int cdns_i2c_clk_notifier_cb(struct notifier_block *nb, unsigned long
>  }
>  
>  /**
> - * cdns_i2c_suspend - Suspend method for the driver
> - * @_dev:	Address of the platform_device structure
> + * cdns_i2c_runtime_suspend -  Runtime suspend method for the driver
> + * @dev:	Address of the platform_device structure
>   *
>   * Put the driver into low power mode.
>   *
> @@ -833,10 +835,10 @@ static int __maybe_unused cdns_i2c_runtime_suspend(struct device *dev)
>  }
>  
>  /**
> - * cdns_i2c_resume - Resume from suspend
> - * @_dev:	Address of the platform_device structure
> + * cdns_i2c_runtime_resume - Runtime resume
> + * @dev:	Address of the platform_device structure
>   *
> - * Resume operation after suspend.
> + * Runtime resume callback.
>   *
>   * Return: 0 on success and error value on error
>   */
> 

Can you also please include this fix there too?

diff --git a/drivers/i2c/busses/i2c-cadence.c
b/drivers/i2c/busses/i2c-cadence.c
index f9f485d2d917..90bbd9f9dd8f 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -173,7 +173,7 @@ struct cdns_platform_data {
                                             clk_rate_change_nb)

 /**
- * cdns_i2c_clear_bus_hold() - Clear bus hold bit
+ * cdns_i2c_clear_bus_hold - Clear bus hold bit
  * @id:        Pointer to driver data struct
  *
  * Helper to clear the controller's bus hold bit.

When it is added feel free to add my
Tested-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 2/2] i2c: cadence: Implement save restore
  2016-03-01  9:21 ` [PATCHv2 2/2] i2c: cadence: Implement save restore Shubhrajyoti Datta
@ 2016-03-02  8:32   ` Michal Simek
  0 siblings, 0 replies; 6+ messages in thread
From: Michal Simek @ 2016-03-02  8:32 UTC (permalink / raw)
  To: Shubhrajyoti Datta, linux-i2c
  Cc: soren.brinkmann, michal.simek, wsa, Shubhrajyoti Datta

On 1.3.2016 10:21, Shubhrajyoti Datta wrote:
> Implement save restore for i2c module.
> Since we have only a couple of registers
> an unconditional restore is done.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhraj@xilinx.com>
> ---
> zynq-mp has the capability of going off.
> the current kernel does not hit off however some day it will.
> since the overhead of having the support is not much may be it is better to have it in the kernel.
> 
> v2 changes:
> add kernel doc for ctrl_reg and cdns_i2c_init
> 
>  drivers/i2c/busses/i2c-cadence.c |   41 +++++++++++++++++++++++++-------------
>  1 files changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
> index f9f485d..88d2c8b 100644
> --- a/drivers/i2c/busses/i2c-cadence.c
> +++ b/drivers/i2c/busses/i2c-cadence.c
> @@ -143,6 +143,7 @@
>   * @clk:		Pointer to struct clk
>   * @clk_rate_change_nb:	Notifier block for clock rate changes
>   * @quirks:		flag for broken hold bit usage in r1p10
> + * @ctrl_reg:		Cached value of the control register.
>   */
>  struct cdns_i2c {
>  	struct device		*dev;
> @@ -163,6 +164,7 @@ struct cdns_i2c {
>  	struct clk *clk;
>  	struct notifier_block clk_rate_change_nb;
>  	u32 quirks;
> +	u32 ctrl_reg;
>  };
>  
>  struct cdns_platform_data {
> @@ -745,12 +747,11 @@ static int cdns_i2c_setclk(unsigned long clk_in, struct cdns_i2c *id)
>  	if (ret)
>  		return ret;
>  
> -	ctrl_reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET);
> +	ctrl_reg = id->ctrl_reg;
>  	ctrl_reg &= ~(CDNS_I2C_CR_DIVA_MASK | CDNS_I2C_CR_DIVB_MASK);
>  	ctrl_reg |= ((div_a << CDNS_I2C_CR_DIVA_SHIFT) |
>  			(div_b << CDNS_I2C_CR_DIVB_SHIFT));
> -	cdns_i2c_writereg(ctrl_reg, CDNS_I2C_CR_OFFSET);
> -
> +	id->ctrl_reg = ctrl_reg;
>  	return 0;
>  }
>  
> @@ -835,6 +836,26 @@ static int __maybe_unused cdns_i2c_runtime_suspend(struct device *dev)
>  }
>  
>  /**
> + * cdns_i2c_init -  Controller initialisation
> + * @id:		Device private data structure
> + *
> + * Initialise the i2c controller.
> + *
> + */
> +static void cdns_i2c_init(struct cdns_i2c *id)
> +{
> +	cdns_i2c_writereg(id->ctrl_reg, CDNS_I2C_CR_OFFSET);
> +	/*
> +	 * Cadence I2C controller has a bug wherein it generates
> +	 * invalid read transaction after HW timeout in master receiver mode.
> +	 * HW timeout is not used by this driver and the interrupt is disabled.
> +	 * But the feature itself cannot be disabled. Hence maximum value
> +	 * is written to this register to reduce the chances of error.
> +	 */
> +	cdns_i2c_writereg(CDNS_I2C_TIMEOUT_MAX, CDNS_I2C_TIME_OUT_OFFSET);
> +}
> +
> +/**
>   * cdns_i2c_runtime_resume - Runtime resume
>   * @dev:	Address of the platform_device structure
>   *
> @@ -853,6 +874,7 @@ static int __maybe_unused cdns_i2c_runtime_resume(struct device *dev)
>  		dev_err(dev, "Cannot enable clock.\n");
>  		return ret;
>  	}
> +	cdns_i2c_init(xi2c);
>  
>  	return 0;
>  }
> @@ -945,8 +967,7 @@ static int cdns_i2c_probe(struct platform_device *pdev)
>  	if (ret || (id->i2c_clk > CDNS_I2C_SPEED_MAX))
>  		id->i2c_clk = CDNS_I2C_SPEED_DEFAULT;
>  
> -	cdns_i2c_writereg(CDNS_I2C_CR_ACK_EN | CDNS_I2C_CR_NEA | CDNS_I2C_CR_MS,
> -			  CDNS_I2C_CR_OFFSET);
> +	id->ctrl_reg = CDNS_I2C_CR_ACK_EN | CDNS_I2C_CR_NEA | CDNS_I2C_CR_MS;
>  
>  	ret = cdns_i2c_setclk(id->input_clk, id);
>  	if (ret) {
> @@ -967,15 +988,7 @@ static int cdns_i2c_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "reg adap failed: %d\n", ret);
>  		goto err_clk_dis;
>  	}
> -
> -	/*
> -	 * Cadence I2C controller has a bug wherein it generates
> -	 * invalid read transaction after HW timeout in master receiver mode.
> -	 * HW timeout is not used by this driver and the interrupt is disabled.
> -	 * But the feature itself cannot be disabled. Hence maximum value
> -	 * is written to this register to reduce the chances of error.
> -	 */
> -	cdns_i2c_writereg(CDNS_I2C_TIMEOUT_MAX, CDNS_I2C_TIME_OUT_OFFSET);
> +	cdns_i2c_init(id);
>  
>  	dev_info(&pdev->dev, "%u kHz mmio %08lx irq %d\n",
>  		 id->i2c_clk / 1000, (unsigned long)r_mem->start, id->irq);
> 

Looks good to me.
Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 1/2] i2c: cadence: Fix the Documentation
  2016-03-02  8:22 ` [PATCHv2 1/2] i2c: cadence: Fix the Documentation Michal Simek
@ 2016-03-02  9:01   ` Wolfram Sang
  2016-03-02  9:05     ` Michal Simek
  0 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2016-03-02  9:01 UTC (permalink / raw)
  To: Michal Simek
  Cc: Shubhrajyoti Datta, linux-i2c, soren.brinkmann,
	Shubhrajyoti Datta

[-- Attachment #1: Type: text/plain, Size: 92 bytes --]


> Tested-by: Michal Simek <michal.simek@xilinx.com>

Tested-by for a documentation patch?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 1/2] i2c: cadence: Fix the Documentation
  2016-03-02  9:01   ` Wolfram Sang
@ 2016-03-02  9:05     ` Michal Simek
  0 siblings, 0 replies; 6+ messages in thread
From: Michal Simek @ 2016-03-02  9:05 UTC (permalink / raw)
  To: Wolfram Sang, Michal Simek
  Cc: Shubhrajyoti Datta, linux-i2c, soren.brinkmann,
	Shubhrajyoti Datta

On 2.3.2016 10:01, Wolfram Sang wrote:
> 
>> Tested-by: Michal Simek <michal.simek@xilinx.com>
> 
> Tested-by for a documentation patch?

yep. I was running kernel-doc which check it fully.
That's why I have added tested-by.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-03-02  9:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-01  9:21 [PATCHv2 1/2] i2c: cadence: Fix the Documentation Shubhrajyoti Datta
2016-03-01  9:21 ` [PATCHv2 2/2] i2c: cadence: Implement save restore Shubhrajyoti Datta
2016-03-02  8:32   ` Michal Simek
2016-03-02  8:22 ` [PATCHv2 1/2] i2c: cadence: Fix the Documentation Michal Simek
2016-03-02  9:01   ` Wolfram Sang
2016-03-02  9:05     ` Michal Simek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).