On Tue, 2018-05-29 at 14:27 +0300, Jarkko Nikula wrote:
> 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@xxxxxxxxxxxxxxx>
with a caveat to get rid of use of so-o built-in macro either here or in
separate patch (for the separate patch you may inherit above tag I
give).
> Signed-off-by: Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx>
> ---
> 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(-)
>
> 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)) {
Perhaps somewhere in this series to change this so built-in macro which
this driver the only user of, to __swab32() which is used widely.
> + /* 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;
--
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy
