* [PATCH v1 1/2] i2c: designware: Make master module optional
@ 2020-03-23 10:04 Andy Shevchenko
2020-03-23 10:04 ` [PATCH v1 2/2] i2c: designware: Move configuration routines to respective modules Andy Shevchenko
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Andy Shevchenko @ 2020-03-23 10:04 UTC (permalink / raw)
To: Jarkko Nikula, Mika Westerberg, linux-i2c, Wolfram Sang; +Cc: Andy Shevchenko
In some cases we know that the controller will be always used in slave mode and
master is just a bulk. In order to drop that, introduce a separate configuration
parameter for master mode. Default it to core to avoid regressions.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/i2c/busses/Kconfig | 10 ++++++++++
drivers/i2c/busses/Makefile | 5 ++++-
drivers/i2c/busses/i2c-designware-core.h | 19 ++++++++++++++++++-
drivers/i2c/busses/i2c-designware-master.c | 4 ++--
drivers/i2c/busses/i2c-designware-pcidrv.c | 2 +-
drivers/i2c/busses/i2c-designware-platdrv.c | 6 +-----
6 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 2ddca08f8a76..1df238ff8dd0 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -527,6 +527,16 @@ config I2C_DAVINCI
config I2C_DESIGNWARE_CORE
tristate
+config I2C_DESIGNWARE_MASTER
+ bool "Synopsys DesignWare Master"
+ default I2C_DESIGNWARE_CORE
+ help
+ If you say yes to this option, support will be included for the
+ Synopsys DesignWare I2C master adapter.
+
+ This is not a standalone module, this module compiles together with
+ i2c-designware-core.
+
config I2C_DESIGNWARE_PLATFORM
tristate "Synopsys DesignWare Platform"
select I2C_DESIGNWARE_CORE
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 25d60889713c..829731d4a1f9 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -49,7 +49,10 @@ obj-$(CONFIG_I2C_CBUS_GPIO) += i2c-cbus-gpio.o
obj-$(CONFIG_I2C_CPM) += i2c-cpm.o
obj-$(CONFIG_I2C_DAVINCI) += i2c-davinci.o
obj-$(CONFIG_I2C_DESIGNWARE_CORE) += i2c-designware-core.o
-i2c-designware-core-objs := i2c-designware-common.o i2c-designware-master.o
+i2c-designware-core-objs := i2c-designware-common.o
+ifeq ($(CONFIG_I2C_DESIGNWARE_MASTER),y)
+i2c-designware-core-objs += i2c-designware-master.o
+endif
ifeq ($(CONFIG_I2C_DESIGNWARE_SLAVE),y)
i2c-designware-core-objs += i2c-designware-slave.o
endif
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index b220ad64c38d..2d34c15dca75 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -314,13 +314,30 @@ static inline void __i2c_dw_disable_nowait(struct dw_i2c_dev *dev)
void __i2c_dw_disable(struct dw_i2c_dev *dev);
-extern int i2c_dw_probe(struct dw_i2c_dev *dev);
+#if IS_ENABLED(CONFIG_I2C_DESIGNWARE_MASTER)
+extern int i2c_dw_probe_master(struct dw_i2c_dev *dev);
+#else
+static inline int i2c_dw_probe_master(struct dw_i2c_dev *dev) { return -EINVAL; }
+#endif
#if IS_ENABLED(CONFIG_I2C_DESIGNWARE_SLAVE)
extern int i2c_dw_probe_slave(struct dw_i2c_dev *dev);
#else
static inline int i2c_dw_probe_slave(struct dw_i2c_dev *dev) { return -EINVAL; }
#endif
+static inline int i2c_dw_probe(struct dw_i2c_dev *dev)
+{
+ switch (dev->mode) {
+ case DW_IC_SLAVE:
+ return i2c_dw_probe_slave(dev);
+ case DW_IC_MASTER:
+ return i2c_dw_probe_master(dev);
+ default:
+ dev_err(dev->dev, "Wrong operation mode: %d\n", dev->mode);
+ return -EINVAL;
+ }
+}
+
#if IS_ENABLED(CONFIG_I2C_DESIGNWARE_BAYTRAIL)
extern int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev);
#else
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 3a58eef20936..9d2af87f45f1 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -678,7 +678,7 @@ static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev)
return 0;
}
-int i2c_dw_probe(struct dw_i2c_dev *dev)
+int i2c_dw_probe_master(struct dw_i2c_dev *dev)
{
struct i2c_adapter *adap = &dev->adapter;
unsigned long irq_flags;
@@ -745,7 +745,7 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
return ret;
}
-EXPORT_SYMBOL_GPL(i2c_dw_probe);
+EXPORT_SYMBOL_GPL(i2c_dw_probe_master);
MODULE_DESCRIPTION("Synopsys DesignWare I2C bus master adapter");
MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 7a0b65b5b5b5..cbab5346e311 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -290,7 +290,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev));
adap->nr = controller->bus_num;
- r = i2c_dw_probe(dev);
+ r = i2c_dw_probe_master(dev);
if (r) {
pci_free_irq_vectors(pdev);
return r;
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index c98befe2a92e..0edcebe2168f 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -371,11 +371,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
pm_runtime_enable(&pdev->dev);
- if (dev->mode == DW_IC_SLAVE)
- ret = i2c_dw_probe_slave(dev);
- else
- ret = i2c_dw_probe(dev);
-
+ ret = i2c_dw_probe(dev);
if (ret)
goto exit_probe;
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v1 2/2] i2c: designware: Move configuration routines to respective modules
2020-03-23 10:04 [PATCH v1 1/2] i2c: designware: Make master module optional Andy Shevchenko
@ 2020-03-23 10:04 ` Andy Shevchenko
2020-03-23 12:26 ` [PATCH v1 1/2] i2c: designware: Make master module optional Andy Shevchenko
2020-03-25 7:47 ` Jarkko Nikula
2 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2020-03-23 10:04 UTC (permalink / raw)
To: Jarkko Nikula, Mika Westerberg, linux-i2c, Wolfram Sang; +Cc: Andy Shevchenko
Move configuration routines to respective modules, i.e. master and slave.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/i2c/busses/i2c-designware-core.h | 12 +++++++
drivers/i2c/busses/i2c-designware-master.c | 24 +++++++++++++
drivers/i2c/busses/i2c-designware-platdrv.c | 38 +--------------------
drivers/i2c/busses/i2c-designware-slave.c | 11 ++++++
4 files changed, 48 insertions(+), 37 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 2d34c15dca75..ba04fa91de0c 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -315,13 +315,17 @@ static inline void __i2c_dw_disable_nowait(struct dw_i2c_dev *dev)
void __i2c_dw_disable(struct dw_i2c_dev *dev);
#if IS_ENABLED(CONFIG_I2C_DESIGNWARE_MASTER)
+extern void i2c_dw_configure_master(struct dw_i2c_dev *dev);
extern int i2c_dw_probe_master(struct dw_i2c_dev *dev);
#else
+static inline void i2c_dw_configure_master(struct dw_i2c_dev *dev) { }
static inline int i2c_dw_probe_master(struct dw_i2c_dev *dev) { return -EINVAL; }
#endif
#if IS_ENABLED(CONFIG_I2C_DESIGNWARE_SLAVE)
+extern void i2c_dw_configure_slave(struct dw_i2c_dev *dev);
extern int i2c_dw_probe_slave(struct dw_i2c_dev *dev);
#else
+static inline void i2c_dw_configure_slave(struct dw_i2c_dev *dev) { }
static inline int i2c_dw_probe_slave(struct dw_i2c_dev *dev) { return -EINVAL; }
#endif
@@ -338,6 +342,14 @@ static inline int i2c_dw_probe(struct dw_i2c_dev *dev)
}
}
+static inline void i2c_dw_configure(struct dw_i2c_dev *dev)
+{
+ if (i2c_detect_slave_mode(dev->dev))
+ i2c_dw_configure_slave(dev);
+ else
+ i2c_dw_configure_master(dev);
+}
+
#if IS_ENABLED(CONFIG_I2C_DESIGNWARE_BAYTRAIL)
extern int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev);
#else
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 9d2af87f45f1..66ada723d740 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -632,6 +632,30 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
return IRQ_HANDLED;
}
+void i2c_dw_configure_master(struct dw_i2c_dev *dev)
+{
+ struct i2c_timings *t = &dev->timings;
+
+ dev->functionality = I2C_FUNC_10BIT_ADDR | DW_IC_DEFAULT_FUNCTIONALITY;
+
+ dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
+ DW_IC_CON_RESTART_EN;
+
+ dev->mode = DW_IC_MASTER;
+
+ switch (t->bus_freq_hz) {
+ case I2C_MAX_STANDARD_MODE_FREQ:
+ dev->master_cfg |= DW_IC_CON_SPEED_STD;
+ break;
+ case I2C_MAX_HIGH_SPEED_MODE_FREQ:
+ dev->master_cfg |= DW_IC_CON_SPEED_HIGH;
+ break;
+ default:
+ dev->master_cfg |= DW_IC_CON_SPEED_FAST;
+ }
+}
+EXPORT_SYMBOL_GPL(i2c_dw_configure_master);
+
static void i2c_dw_prepare_recovery(struct i2c_adapter *adap)
{
struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 0edcebe2168f..6da7f14cba62 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -186,39 +186,6 @@ static inline int dw_i2c_of_configure(struct platform_device *pdev)
}
#endif
-static void i2c_dw_configure_master(struct dw_i2c_dev *dev)
-{
- struct i2c_timings *t = &dev->timings;
-
- dev->functionality = I2C_FUNC_10BIT_ADDR | DW_IC_DEFAULT_FUNCTIONALITY;
-
- dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
- DW_IC_CON_RESTART_EN;
-
- dev->mode = DW_IC_MASTER;
-
- switch (t->bus_freq_hz) {
- case I2C_MAX_STANDARD_MODE_FREQ:
- dev->master_cfg |= DW_IC_CON_SPEED_STD;
- break;
- case I2C_MAX_HIGH_SPEED_MODE_FREQ:
- dev->master_cfg |= DW_IC_CON_SPEED_HIGH;
- break;
- default:
- dev->master_cfg |= DW_IC_CON_SPEED_FAST;
- }
-}
-
-static void i2c_dw_configure_slave(struct dw_i2c_dev *dev)
-{
- dev->functionality = I2C_FUNC_SLAVE | DW_IC_DEFAULT_FUNCTIONALITY;
-
- dev->slave_cfg = DW_IC_CON_RX_FIFO_FULL_HLD_CTRL |
- DW_IC_CON_RESTART_EN | DW_IC_CON_STOP_DET_IFADDRESSED;
-
- dev->mode = DW_IC_SLAVE;
-}
-
static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
{
pm_runtime_disable(dev->dev);
@@ -323,10 +290,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
if (ret)
goto exit_reset;
- if (i2c_detect_slave_mode(&pdev->dev))
- i2c_dw_configure_slave(dev);
- else
- i2c_dw_configure_master(dev);
+ i2c_dw_configure(dev);
/* Optional interface clock */
dev->pclk = devm_clk_get_optional(&pdev->dev, "pclk");
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index f5ecf76c0d02..576e7af4e94b 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -241,6 +241,17 @@ static const struct i2c_algorithm i2c_dw_algo = {
.unreg_slave = i2c_dw_unreg_slave,
};
+void i2c_dw_configure_slave(struct dw_i2c_dev *dev)
+{
+ dev->functionality = I2C_FUNC_SLAVE | DW_IC_DEFAULT_FUNCTIONALITY;
+
+ dev->slave_cfg = DW_IC_CON_RX_FIFO_FULL_HLD_CTRL |
+ DW_IC_CON_RESTART_EN | DW_IC_CON_STOP_DET_IFADDRESSED;
+
+ dev->mode = DW_IC_SLAVE;
+}
+EXPORT_SYMBOL_GPL(i2c_dw_configure_slave);
+
int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
{
struct i2c_adapter *adap = &dev->adapter;
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v1 1/2] i2c: designware: Make master module optional
2020-03-23 10:04 [PATCH v1 1/2] i2c: designware: Make master module optional Andy Shevchenko
2020-03-23 10:04 ` [PATCH v1 2/2] i2c: designware: Move configuration routines to respective modules Andy Shevchenko
@ 2020-03-23 12:26 ` Andy Shevchenko
2020-03-25 7:47 ` Jarkko Nikula
2 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2020-03-23 12:26 UTC (permalink / raw)
To: Jarkko Nikula, Mika Westerberg, linux-i2c, Wolfram Sang
On Mon, Mar 23, 2020 at 12:04:50PM +0200, Andy Shevchenko wrote:
> In some cases we know that the controller will be always used in slave mode and
> master is just a bulk. In order to drop that, introduce a separate configuration
> parameter for master mode. Default it to core to avoid regressions.
>
This series depends to [1] "Provide generic definitions for bus frequencies".
[1]: http://patchwork.ozlabs.org/patch/1259203/
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/i2c/busses/Kconfig | 10 ++++++++++
> drivers/i2c/busses/Makefile | 5 ++++-
> drivers/i2c/busses/i2c-designware-core.h | 19 ++++++++++++++++++-
> drivers/i2c/busses/i2c-designware-master.c | 4 ++--
> drivers/i2c/busses/i2c-designware-pcidrv.c | 2 +-
> drivers/i2c/busses/i2c-designware-platdrv.c | 6 +-----
> 6 files changed, 36 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 2ddca08f8a76..1df238ff8dd0 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -527,6 +527,16 @@ config I2C_DAVINCI
> config I2C_DESIGNWARE_CORE
> tristate
>
> +config I2C_DESIGNWARE_MASTER
> + bool "Synopsys DesignWare Master"
> + default I2C_DESIGNWARE_CORE
> + help
> + If you say yes to this option, support will be included for the
> + Synopsys DesignWare I2C master adapter.
> +
> + This is not a standalone module, this module compiles together with
> + i2c-designware-core.
> +
> config I2C_DESIGNWARE_PLATFORM
> tristate "Synopsys DesignWare Platform"
> select I2C_DESIGNWARE_CORE
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 25d60889713c..829731d4a1f9 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -49,7 +49,10 @@ obj-$(CONFIG_I2C_CBUS_GPIO) += i2c-cbus-gpio.o
> obj-$(CONFIG_I2C_CPM) += i2c-cpm.o
> obj-$(CONFIG_I2C_DAVINCI) += i2c-davinci.o
> obj-$(CONFIG_I2C_DESIGNWARE_CORE) += i2c-designware-core.o
> -i2c-designware-core-objs := i2c-designware-common.o i2c-designware-master.o
> +i2c-designware-core-objs := i2c-designware-common.o
> +ifeq ($(CONFIG_I2C_DESIGNWARE_MASTER),y)
> +i2c-designware-core-objs += i2c-designware-master.o
> +endif
> ifeq ($(CONFIG_I2C_DESIGNWARE_SLAVE),y)
> i2c-designware-core-objs += i2c-designware-slave.o
> endif
> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
> index b220ad64c38d..2d34c15dca75 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -314,13 +314,30 @@ static inline void __i2c_dw_disable_nowait(struct dw_i2c_dev *dev)
>
> void __i2c_dw_disable(struct dw_i2c_dev *dev);
>
> -extern int i2c_dw_probe(struct dw_i2c_dev *dev);
> +#if IS_ENABLED(CONFIG_I2C_DESIGNWARE_MASTER)
> +extern int i2c_dw_probe_master(struct dw_i2c_dev *dev);
> +#else
> +static inline int i2c_dw_probe_master(struct dw_i2c_dev *dev) { return -EINVAL; }
> +#endif
> #if IS_ENABLED(CONFIG_I2C_DESIGNWARE_SLAVE)
> extern int i2c_dw_probe_slave(struct dw_i2c_dev *dev);
> #else
> static inline int i2c_dw_probe_slave(struct dw_i2c_dev *dev) { return -EINVAL; }
> #endif
>
> +static inline int i2c_dw_probe(struct dw_i2c_dev *dev)
> +{
> + switch (dev->mode) {
> + case DW_IC_SLAVE:
> + return i2c_dw_probe_slave(dev);
> + case DW_IC_MASTER:
> + return i2c_dw_probe_master(dev);
> + default:
> + dev_err(dev->dev, "Wrong operation mode: %d\n", dev->mode);
> + return -EINVAL;
> + }
> +}
> +
> #if IS_ENABLED(CONFIG_I2C_DESIGNWARE_BAYTRAIL)
> extern int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev);
> #else
> diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
> index 3a58eef20936..9d2af87f45f1 100644
> --- a/drivers/i2c/busses/i2c-designware-master.c
> +++ b/drivers/i2c/busses/i2c-designware-master.c
> @@ -678,7 +678,7 @@ static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev)
> return 0;
> }
>
> -int i2c_dw_probe(struct dw_i2c_dev *dev)
> +int i2c_dw_probe_master(struct dw_i2c_dev *dev)
> {
> struct i2c_adapter *adap = &dev->adapter;
> unsigned long irq_flags;
> @@ -745,7 +745,7 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
>
> return ret;
> }
> -EXPORT_SYMBOL_GPL(i2c_dw_probe);
> +EXPORT_SYMBOL_GPL(i2c_dw_probe_master);
>
> MODULE_DESCRIPTION("Synopsys DesignWare I2C bus master adapter");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
> index 7a0b65b5b5b5..cbab5346e311 100644
> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> @@ -290,7 +290,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
> ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev));
> adap->nr = controller->bus_num;
>
> - r = i2c_dw_probe(dev);
> + r = i2c_dw_probe_master(dev);
> if (r) {
> pci_free_irq_vectors(pdev);
> return r;
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index c98befe2a92e..0edcebe2168f 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -371,11 +371,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
>
> pm_runtime_enable(&pdev->dev);
>
> - if (dev->mode == DW_IC_SLAVE)
> - ret = i2c_dw_probe_slave(dev);
> - else
> - ret = i2c_dw_probe(dev);
> -
> + ret = i2c_dw_probe(dev);
> if (ret)
> goto exit_probe;
>
> --
> 2.25.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1 1/2] i2c: designware: Make master module optional
2020-03-23 10:04 [PATCH v1 1/2] i2c: designware: Make master module optional Andy Shevchenko
2020-03-23 10:04 ` [PATCH v1 2/2] i2c: designware: Move configuration routines to respective modules Andy Shevchenko
2020-03-23 12:26 ` [PATCH v1 1/2] i2c: designware: Make master module optional Andy Shevchenko
@ 2020-03-25 7:47 ` Jarkko Nikula
2020-03-25 11:45 ` Andy Shevchenko
2 siblings, 1 reply; 6+ messages in thread
From: Jarkko Nikula @ 2020-03-25 7:47 UTC (permalink / raw)
To: Andy Shevchenko, Mika Westerberg, linux-i2c, Wolfram Sang
On 3/23/20 12:04 PM, Andy Shevchenko wrote:
> In some cases we know that the controller will be always used in slave mode and
> master is just a bulk. In order to drop that, introduce a separate configuration
> parameter for master mode. Default it to core to avoid regressions.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/i2c/busses/Kconfig | 10 ++++++++++
> drivers/i2c/busses/Makefile | 5 ++++-
> drivers/i2c/busses/i2c-designware-core.h | 19 ++++++++++++++++++-
> drivers/i2c/busses/i2c-designware-master.c | 4 ++--
> drivers/i2c/busses/i2c-designware-pcidrv.c | 2 +-
> drivers/i2c/busses/i2c-designware-platdrv.c | 6 +-----
> 6 files changed, 36 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 2ddca08f8a76..1df238ff8dd0 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -527,6 +527,16 @@ config I2C_DAVINCI
> config I2C_DESIGNWARE_CORE
> tristate
>
> +config I2C_DESIGNWARE_MASTER
> + bool "Synopsys DesignWare Master"
> + default I2C_DESIGNWARE_CORE
> + help
> + If you say yes to this option, support will be included for the
> + Synopsys DesignWare I2C master adapter.
> +
> + This is not a standalone module, this module compiles together with
> + i2c-designware-core.
> +
I think we should go to a opposite direction - reduce the number of
I2C_DESIGNWARE_ config options rather than add new ones. We already have
5 config options for it.
Size of i2c-designware-core.ko is around 12 kB with all master, slave
and Baytrail semaphore code built in so I don't think it justifies the
added config complexity. I think distributions will have anyway all of
those options set.
Having those code in separate modules and load only when needed might
make sense as that would save a few kB of RAM.
--
Jarkko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/2] i2c: designware: Make master module optional
2020-03-25 7:47 ` Jarkko Nikula
@ 2020-03-25 11:45 ` Andy Shevchenko
2020-04-15 11:37 ` Wolfram Sang
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2020-03-25 11:45 UTC (permalink / raw)
To: Jarkko Nikula; +Cc: Mika Westerberg, linux-i2c, Wolfram Sang
On Wed, Mar 25, 2020 at 09:47:47AM +0200, Jarkko Nikula wrote:
> On 3/23/20 12:04 PM, Andy Shevchenko wrote:
> > In some cases we know that the controller will be always used in slave mode and
> > master is just a bulk. In order to drop that, introduce a separate configuration
> > parameter for master mode. Default it to core to avoid regressions.
> I think we should go to a opposite direction - reduce the number of
> I2C_DESIGNWARE_ config options rather than add new ones. We already have 5
> config options for it.
>
> Size of i2c-designware-core.ko is around 12 kB with all master, slave and
> Baytrail semaphore code built in so I don't think it justifies the added
> config complexity. I think distributions will have anyway all of those
> options set.
I would rather go with conditional based on I²C generic options, like I2C_SLAVE.
Do we have something similar for master?
> Having those code in separate modules and load only when needed might make
> sense as that would save a few kB of RAM.
...which makes sense for embedded systems where exactly the device represents
I²C slave.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/2] i2c: designware: Make master module optional
2020-03-25 11:45 ` Andy Shevchenko
@ 2020-04-15 11:37 ` Wolfram Sang
0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2020-04-15 11:37 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Jarkko Nikula, Mika Westerberg, linux-i2c, Wolfram Sang
[-- Attachment #1: Type: text/plain, Size: 1100 bytes --]
> > Size of i2c-designware-core.ko is around 12 kB with all master, slave and
> > Baytrail semaphore code built in so I don't think it justifies the added
> > config complexity. I think distributions will have anyway all of those
> > options set.
>
> I would rather go with conditional based on I²C generic options, like I2C_SLAVE.
> Do we have something similar for master?
No, we don't have that.
>
> > Having those code in separate modules and load only when needed might make
> > sense as that would save a few kB of RAM.
>
> ...which makes sense for embedded systems where exactly the device represents
> I²C slave.
Frankly: an I2C-slave-only embedded system which runs a modern Linux and
cannot afford those few KB on a core feature it needs? If so, maybe it
should have an out-of-tree patch to achieve this. I don't think it is
worth the added complexity for the upstream version.
Sidenote: There is a lot more overhead in the i2c-core. I think the
complexity to move out stuff there is even more messy.
Disclaimer: you may prove me wrong, of course :)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-15 11:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-23 10:04 [PATCH v1 1/2] i2c: designware: Make master module optional Andy Shevchenko
2020-03-23 10:04 ` [PATCH v1 2/2] i2c: designware: Move configuration routines to respective modules Andy Shevchenko
2020-03-23 12:26 ` [PATCH v1 1/2] i2c: designware: Make master module optional Andy Shevchenko
2020-03-25 7:47 ` Jarkko Nikula
2020-03-25 11:45 ` Andy Shevchenko
2020-04-15 11:37 ` Wolfram Sang
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).