From: Serge Semin <[email protected]>
The problem with detecting the FIFO depth in the platform driver
is that in order to implement this we have to access the controller
IC_COMP_PARAM_1 register. Currently it's done before the
i2c_dw_set_reg_access() method execution, which is errors prone since
the method determines the registers endianness and access mode and we
can't use dw_readl/dw_writel accessors before this information is
retrieved. We also can't move the i2c_dw_set_reg_access() function
invocation to after the master/slave probe functions call (when endianness
and access mode are determined), since the FIFO depth information is used
by them for initializations. So in order to fix the problem we have no
choice but to move the FIFO size detection methods to the common code and
call it at the probe stage.
Signed-off-by: Serge Semin <[email protected]>
Signed-off-by: Alexey Malahov <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/i2c/busses/i2c-designware-common.c | 22 +++++++++++++++++++
drivers/i2c/busses/i2c-designware-core.h | 1 +
drivers/i2c/busses/i2c-designware-master.c | 2 ++
drivers/i2c/busses/i2c-designware-platdrv.c | 24 ---------------------
drivers/i2c/busses/i2c-designware-slave.c | 2 ++
5 files changed, 27 insertions(+), 24 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 2de7452fcd6d..4291ff6246d8 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -344,6 +344,28 @@ int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
return -EIO;
}
+void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
+{
+ u32 param, tx_fifo_depth, rx_fifo_depth;
+
+ /*
+ * Try to detect the FIFO depth if not set by interface driver,
+ * the depth could be from 2 to 256 from HW spec.
+ */
+ param = dw_readl(dev, DW_IC_COMP_PARAM_1);
+ tx_fifo_depth = ((param >> 16) & 0xff) + 1;
+ rx_fifo_depth = ((param >> 8) & 0xff) + 1;
+ if (!dev->tx_fifo_depth) {
+ dev->tx_fifo_depth = tx_fifo_depth;
+ dev->rx_fifo_depth = rx_fifo_depth;
+ } else if (tx_fifo_depth >= 2) {
+ dev->tx_fifo_depth = min_t(u32, dev->tx_fifo_depth,
+ tx_fifo_depth);
+ dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,
+ rx_fifo_depth);
+ }
+}
+
u32 i2c_dw_func(struct i2c_adapter *adap)
{
struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 67edbbde1070..3fbc9f22fcf1 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -297,6 +297,7 @@ int i2c_dw_acquire_lock(struct dw_i2c_dev *dev);
void i2c_dw_release_lock(struct dw_i2c_dev *dev);
int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev);
int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev);
+void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev);
u32 i2c_dw_func(struct i2c_adapter *adap);
void i2c_dw_disable(struct dw_i2c_dev *dev);
void i2c_dw_disable_int(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index e8b328242256..05da900cf375 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -698,6 +698,8 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
if (ret)
return ret;
+ i2c_dw_set_fifo_size(dev);
+
ret = dev->init(dev);
if (ret)
return ret;
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 3b7d58c2fe85..cb494273bb60 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -219,28 +219,6 @@ static void i2c_dw_configure_slave(struct dw_i2c_dev *dev)
dev->mode = DW_IC_SLAVE;
}
-static void dw_i2c_set_fifo_size(struct dw_i2c_dev *dev)
-{
- u32 param, tx_fifo_depth, rx_fifo_depth;
-
- /*
- * Try to detect the FIFO depth if not set by interface driver,
- * the depth could be from 2 to 256 from HW spec.
- */
- param = i2c_dw_read_comp_param(dev);
- tx_fifo_depth = ((param >> 16) & 0xff) + 1;
- rx_fifo_depth = ((param >> 8) & 0xff) + 1;
- if (!dev->tx_fifo_depth) {
- dev->tx_fifo_depth = tx_fifo_depth;
- dev->rx_fifo_depth = rx_fifo_depth;
- } else if (tx_fifo_depth >= 2) {
- dev->tx_fifo_depth = min_t(u32, dev->tx_fifo_depth,
- tx_fifo_depth);
- dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,
- rx_fifo_depth);
- }
-}
-
static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
{
pm_runtime_disable(dev->dev);
@@ -362,8 +340,6 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
div_u64(clk_khz * t->sda_hold_ns + 500000, 1000000);
}
- dw_i2c_set_fifo_size(dev);
-
adap = &dev->adapter;
adap->owner = THIS_MODULE;
adap->class = I2C_CLASS_DEPRECATED;
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index f5f001738df5..0fc3aa31d46a 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -260,6 +260,8 @@ int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
if (ret)
return ret;
+ i2c_dw_set_fifo_size(dev);
+
ret = dev->init(dev);
if (ret)
return ret;
--
2.25.1
On Fri, Mar 06, 2020 at 04:19:54PM +0300, [email protected] wrote:
> From: Serge Semin <[email protected]>
>
> The problem with detecting the FIFO depth in the platform driver
> is that in order to implement this we have to access the controller
> IC_COMP_PARAM_1 register. Currently it's done before the
> i2c_dw_set_reg_access() method execution, which is errors prone since
> the method determines the registers endianness and access mode and we
> can't use dw_readl/dw_writel accessors before this information is
> retrieved. We also can't move the i2c_dw_set_reg_access() function
> invocation to after the master/slave probe functions call (when endianness
> and access mode are determined), since the FIFO depth information is used
> by them for initializations. So in order to fix the problem we have no
> choice but to move the FIFO size detection methods to the common code and
> call it at the probe stage.
Sounds reasonable.
Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Serge Semin <[email protected]>
> Signed-off-by: Alexey Malahov <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Paul Burton <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/i2c/busses/i2c-designware-common.c | 22 +++++++++++++++++++
> drivers/i2c/busses/i2c-designware-core.h | 1 +
> drivers/i2c/busses/i2c-designware-master.c | 2 ++
> drivers/i2c/busses/i2c-designware-platdrv.c | 24 ---------------------
> drivers/i2c/busses/i2c-designware-slave.c | 2 ++
> 5 files changed, 27 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
> index 2de7452fcd6d..4291ff6246d8 100644
> --- a/drivers/i2c/busses/i2c-designware-common.c
> +++ b/drivers/i2c/busses/i2c-designware-common.c
> @@ -344,6 +344,28 @@ int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
> return -EIO;
> }
>
> +void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
> +{
> + u32 param, tx_fifo_depth, rx_fifo_depth;
> +
> + /*
> + * Try to detect the FIFO depth if not set by interface driver,
> + * the depth could be from 2 to 256 from HW spec.
> + */
> + param = dw_readl(dev, DW_IC_COMP_PARAM_1);
> + tx_fifo_depth = ((param >> 16) & 0xff) + 1;
> + rx_fifo_depth = ((param >> 8) & 0xff) + 1;
> + if (!dev->tx_fifo_depth) {
> + dev->tx_fifo_depth = tx_fifo_depth;
> + dev->rx_fifo_depth = rx_fifo_depth;
> + } else if (tx_fifo_depth >= 2) {
> + dev->tx_fifo_depth = min_t(u32, dev->tx_fifo_depth,
> + tx_fifo_depth);
> + dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,
> + rx_fifo_depth);
> + }
> +}
> +
> u32 i2c_dw_func(struct i2c_adapter *adap)
> {
> struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
> index 67edbbde1070..3fbc9f22fcf1 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -297,6 +297,7 @@ int i2c_dw_acquire_lock(struct dw_i2c_dev *dev);
> void i2c_dw_release_lock(struct dw_i2c_dev *dev);
> int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev);
> int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev);
> +void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev);
> u32 i2c_dw_func(struct i2c_adapter *adap);
> void i2c_dw_disable(struct dw_i2c_dev *dev);
> void i2c_dw_disable_int(struct dw_i2c_dev *dev);
> diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
> index e8b328242256..05da900cf375 100644
> --- a/drivers/i2c/busses/i2c-designware-master.c
> +++ b/drivers/i2c/busses/i2c-designware-master.c
> @@ -698,6 +698,8 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
> if (ret)
> return ret;
>
> + i2c_dw_set_fifo_size(dev);
> +
> ret = dev->init(dev);
> if (ret)
> return ret;
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 3b7d58c2fe85..cb494273bb60 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -219,28 +219,6 @@ static void i2c_dw_configure_slave(struct dw_i2c_dev *dev)
> dev->mode = DW_IC_SLAVE;
> }
>
> -static void dw_i2c_set_fifo_size(struct dw_i2c_dev *dev)
> -{
> - u32 param, tx_fifo_depth, rx_fifo_depth;
> -
> - /*
> - * Try to detect the FIFO depth if not set by interface driver,
> - * the depth could be from 2 to 256 from HW spec.
> - */
> - param = i2c_dw_read_comp_param(dev);
> - tx_fifo_depth = ((param >> 16) & 0xff) + 1;
> - rx_fifo_depth = ((param >> 8) & 0xff) + 1;
> - if (!dev->tx_fifo_depth) {
> - dev->tx_fifo_depth = tx_fifo_depth;
> - dev->rx_fifo_depth = rx_fifo_depth;
> - } else if (tx_fifo_depth >= 2) {
> - dev->tx_fifo_depth = min_t(u32, dev->tx_fifo_depth,
> - tx_fifo_depth);
> - dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,
> - rx_fifo_depth);
> - }
> -}
> -
> static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
> {
> pm_runtime_disable(dev->dev);
> @@ -362,8 +340,6 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
> div_u64(clk_khz * t->sda_hold_ns + 500000, 1000000);
> }
>
> - dw_i2c_set_fifo_size(dev);
> -
> adap = &dev->adapter;
> adap->owner = THIS_MODULE;
> adap->class = I2C_CLASS_DEPRECATED;
> diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
> index f5f001738df5..0fc3aa31d46a 100644
> --- a/drivers/i2c/busses/i2c-designware-slave.c
> +++ b/drivers/i2c/busses/i2c-designware-slave.c
> @@ -260,6 +260,8 @@ int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
> if (ret)
> return ret;
>
> + i2c_dw_set_fifo_size(dev);
> +
> ret = dev->init(dev);
> if (ret)
> return ret;
> --
> 2.25.1
>
--
With Best Regards,
Andy Shevchenko
On 3/6/20 5:01 PM, Andy Shevchenko wrote:
> On Fri, Mar 06, 2020 at 04:19:54PM +0300, [email protected] wrote:
>> From: Serge Semin <[email protected]>
>>
>> The problem with detecting the FIFO depth in the platform driver
>> is that in order to implement this we have to access the controller
>> IC_COMP_PARAM_1 register. Currently it's done before the
>> i2c_dw_set_reg_access() method execution, which is errors prone since
>> the method determines the registers endianness and access mode and we
>> can't use dw_readl/dw_writel accessors before this information is
>> retrieved. We also can't move the i2c_dw_set_reg_access() function
>> invocation to after the master/slave probe functions call (when endianness
>> and access mode are determined), since the FIFO depth information is used
>> by them for initializations. So in order to fix the problem we have no
>> choice but to move the FIFO size detection methods to the common code and
>> call it at the probe stage.
>
> Sounds reasonable.
>
> Reviewed-by: Andy Shevchenko <[email protected]>
>
Acked-by: Jarkko Nikula <[email protected]>
On Fri, Mar 06, 2020 at 04:19:54PM +0300, [email protected] wrote:
> From: Serge Semin <[email protected]>
>
> The problem with detecting the FIFO depth in the platform driver
> is that in order to implement this we have to access the controller
> IC_COMP_PARAM_1 register. Currently it's done before the
> i2c_dw_set_reg_access() method execution, which is errors prone since
> the method determines the registers endianness and access mode and we
> can't use dw_readl/dw_writel accessors before this information is
> retrieved. We also can't move the i2c_dw_set_reg_access() function
> invocation to after the master/slave probe functions call (when endianness
> and access mode are determined), since the FIFO depth information is used
> by them for initializations. So in order to fix the problem we have no
> choice but to move the FIFO size detection methods to the common code and
> call it at the probe stage.
>
> Signed-off-by: Serge Semin <[email protected]>
> Signed-off-by: Alexey Malahov <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Paul Burton <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
Applied to for-next, thanks!