Re: [PATCH v9] i2c-designware: make SDA hold time configurable

[Wolfram Sang]

Hi,

On Fri, Jun 21, 2013 at 11:53:42AM +0200, Christian Ruppert wrote:
> This patch makes the SDA hold time configurable through device tree.
> 
> Signed-off-by: Christian Ruppert <christian.ruppert@xxxxxxxxxx>
> Signed-off-by: Pierrick Hascoet <pierrick.hascoet@xxxxxxxxxx>
> ---
>  .../devicetree/bindings/i2c/i2c-designware.txt     |   14 ++++++++++++++
>  arch/arc/boot/dts/abilis_tb100_dvk.dts             |   10 +++++-----
>  arch/arc/boot/dts/abilis_tb101_dvk.dts             |   10 +++++-----

Vineet, an ack for those would be nice since I think it is better if I
take all this via I2C.

>  drivers/i2c/busses/i2c-designware-core.c           |   13 +++++++++++++
>  drivers/i2c/busses/i2c-designware-core.h           |    1 +
>  drivers/i2c/busses/i2c-designware-platdrv.c        |   10 ++++++++++
>  6 files changed, 48 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> index e42a2ee..3266cc7 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> @@ -10,6 +10,9 @@ Recommended properties :
>  
>   - clock-frequency : desired I2C bus clock frequency in Hz.
>  
> +Optional properties :
> + - i2c-sda-hold-time-ns : should contain the SDA hold time in nanoseconds.

Not on all versions, should be mentioned here.

> +
>  Example :
>  
>  	i2c@f0000 {
> @@ -20,3 +23,14 @@ Example :
>  		interrupts = <11>;
>  		clock-frequency = <400000>;
>  	};
> +
> +	i2c@1120000 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "snps,designware-i2c";
> +		reg = <0x1120000 0x1000>;
> +		interrupt-parent = <&ictl>;
> +		interrupts = <12 1>;
> +		clock-frequency = <400000>;
> +		i2c-sda-hold-time-ns = <300>;
> +	};
> diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
> index c41ca63..66119e2 100644
> --- a/drivers/i2c/busses/i2c-designware-core.c
> +++ b/drivers/i2c/busses/i2c-designware-core.c
> @@ -67,9 +67,12 @@
>  #define DW_IC_STATUS		0x70
>  #define DW_IC_TXFLR		0x74
>  #define DW_IC_RXFLR		0x78
> +#define DW_IC_SDA_HOLD		0x7c
>  #define DW_IC_TX_ABRT_SOURCE	0x80
>  #define DW_IC_ENABLE_STATUS	0x9c
>  #define DW_IC_COMP_PARAM_1	0xf4
> +#define DW_IC_COMP_VERSION	0xf8
> +#define DW_IC_SDA_HOLD_MIN_VERS	0x3131312A
>  #define DW_IC_COMP_TYPE		0xfc
>  #define DW_IC_COMP_TYPE_VALUE	0x44570140
>  
> @@ -332,6 +335,16 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
>  	dw_writel(dev, lcnt, DW_IC_FS_SCL_LCNT);
>  	dev_dbg(dev->dev, "Fast-mode HCNT:LCNT = %d:%d\n", hcnt, lcnt);
>  
> +	/* Configure SDA Hold Time if required */
> +	reg = dw_readl(dev, DW_IC_COMP_VERSION);

Why not reading reg inside the if-block?

Thanks,

   Wolfram

Attachment: signature.asc
Description: Digital signature