Re: [PATCH v2 08/12] i2c: designware: Introduce platform drivers glue layer interface
On Mon, May 25, 2020 at 04:16:05PM +0300, Jarkko Nikula wrote: > Hi > > On 5/21/20 5:37 AM, Serge Semin wrote: > > On Wed, May 20, 2020 at 03:46:11PM +0300, Jarkko Nikula wrote: > > > Hi > > > > > > On 5/10/20 12:50 PM, Serge Semin wrote: > > > > Seeing the DW I2C platform driver is getting overcomplicated with a lot > > > > of > > > > vendor-specific configs let's introduce a glue-layer interface so new > > > > platforms which equipped with Synopsys Designware APB I2C IP-core would > > > > be able to handle their peculiarities in the dedicated objects. > > > > > > > Comment to this patch and patches 9/12 and 12/12: > > > > > > Currently i2c-designware-platdrv.c is about 500 lines of code so I don't > > > think it's too overcomplicated. But I feel we have already too many > > > Kconfig > > > options and source modules for i2c-designware and obviously would like to > > > push back a little from adding more. > > > > > > I don't think i2c-designware-platdrv.c becomes yet too complicated if > > > Baikal > > > related code is added there, perhaps under #ifdef CONFIG_OF like MSCC > > > Ocelot > > > code is currently. > > > > Well, it's up to you to decide, what solution is more suitable for you to > > maintain. My idea of detaching the MSCC and Baikal-T1 code to the dedicated > > source files was to eventually move the whole i2c-designware-* set of files > > into a dedicated directory drivers/i2c/buses/dw as it's done for some others > > Synopsys DesignWare controllers: drivers/pci/controller/dwc/, > > drivers/usb/dwc2, > > drivers/usb/dwc3, drivers/net/ethernet/synopsys/ . If you think, that it's > > too > > early for Dw I2C code to live in a dedicated directory, fine with me. I can > > merge the MSCC and Baikal-T1 code back into the i2c-designware-platdrv.c . > > So what's your final word in this matter? > > > I think sub directory decision under each subsystem is more subsystem rather > than vendor/driver specific. Good point anyway. > > For this patchset I'd like more if changes are done to > i2c-designware-platdrv.c since it's not too complicated yet :-) > > If it starts to look too messy in the future then it's time split I think. Ok. I'll merge the MSCC back and add Baikal-T1 System I2C support into the DW I2C platform driver. -Sergey > > Jarkko
Re: [PATCH v2 08/12] i2c: designware: Introduce platform drivers glue layer interface
On Mon, May 25, 2020 at 04:16:05PM +0300, Jarkko Nikula wrote: > On 5/21/20 5:37 AM, Serge Semin wrote: > For this patchset I'd like more if changes are done to > i2c-designware-platdrv.c since it's not too complicated yet :-) And after moving ACPI stuff to common code, the one has even been shrunk significantly. > If it starts to look too messy in the future then it's time split I think. -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 08/12] i2c: designware: Introduce platform drivers glue layer interface
Hi On 5/21/20 5:37 AM, Serge Semin wrote: On Wed, May 20, 2020 at 03:46:11PM +0300, Jarkko Nikula wrote: Hi On 5/10/20 12:50 PM, Serge Semin wrote: Seeing the DW I2C platform driver is getting overcomplicated with a lot of vendor-specific configs let's introduce a glue-layer interface so new platforms which equipped with Synopsys Designware APB I2C IP-core would be able to handle their peculiarities in the dedicated objects. Comment to this patch and patches 9/12 and 12/12: Currently i2c-designware-platdrv.c is about 500 lines of code so I don't think it's too overcomplicated. But I feel we have already too many Kconfig options and source modules for i2c-designware and obviously would like to push back a little from adding more. I don't think i2c-designware-platdrv.c becomes yet too complicated if Baikal related code is added there, perhaps under #ifdef CONFIG_OF like MSCC Ocelot code is currently. Well, it's up to you to decide, what solution is more suitable for you to maintain. My idea of detaching the MSCC and Baikal-T1 code to the dedicated source files was to eventually move the whole i2c-designware-* set of files into a dedicated directory drivers/i2c/buses/dw as it's done for some others Synopsys DesignWare controllers: drivers/pci/controller/dwc/, drivers/usb/dwc2, drivers/usb/dwc3, drivers/net/ethernet/synopsys/ . If you think, that it's too early for Dw I2C code to live in a dedicated directory, fine with me. I can merge the MSCC and Baikal-T1 code back into the i2c-designware-platdrv.c . So what's your final word in this matter? I think sub directory decision under each subsystem is more subsystem rather than vendor/driver specific. Good point anyway. For this patchset I'd like more if changes are done to i2c-designware-platdrv.c since it's not too complicated yet :-) If it starts to look too messy in the future then it's time split I think. Jarkko
Re: [PATCH v2 08/12] i2c: designware: Introduce platform drivers glue layer interface
On Wed, May 20, 2020 at 03:46:11PM +0300, Jarkko Nikula wrote: > Hi > > On 5/10/20 12:50 PM, Serge Semin wrote: > > Seeing the DW I2C platform driver is getting overcomplicated with a lot of > > vendor-specific configs let's introduce a glue-layer interface so new > > platforms which equipped with Synopsys Designware APB I2C IP-core would > > be able to handle their peculiarities in the dedicated objects. > > > Comment to this patch and patches 9/12 and 12/12: > > Currently i2c-designware-platdrv.c is about 500 lines of code so I don't > think it's too overcomplicated. But I feel we have already too many Kconfig > options and source modules for i2c-designware and obviously would like to > push back a little from adding more. > > I don't think i2c-designware-platdrv.c becomes yet too complicated if Baikal > related code is added there, perhaps under #ifdef CONFIG_OF like MSCC Ocelot > code is currently. Well, it's up to you to decide, what solution is more suitable for you to maintain. My idea of detaching the MSCC and Baikal-T1 code to the dedicated source files was to eventually move the whole i2c-designware-* set of files into a dedicated directory drivers/i2c/buses/dw as it's done for some others Synopsys DesignWare controllers: drivers/pci/controller/dwc/, drivers/usb/dwc2, drivers/usb/dwc3, drivers/net/ethernet/synopsys/ . If you think, that it's too early for Dw I2C code to live in a dedicated directory, fine with me. I can merge the MSCC and Baikal-T1 code back into the i2c-designware-platdrv.c . So what's your final word in this matter? -Sergey > > -- > Jarkko
Re: [PATCH v2 08/12] i2c: designware: Introduce platform drivers glue layer interface
Hi On 5/10/20 12:50 PM, Serge Semin wrote: Seeing the DW I2C platform driver is getting overcomplicated with a lot of vendor-specific configs let's introduce a glue-layer interface so new platforms which equipped with Synopsys Designware APB I2C IP-core would be able to handle their peculiarities in the dedicated objects. Comment to this patch and patches 9/12 and 12/12: Currently i2c-designware-platdrv.c is about 500 lines of code so I don't think it's too overcomplicated. But I feel we have already too many Kconfig options and source modules for i2c-designware and obviously would like to push back a little from adding more. I don't think i2c-designware-platdrv.c becomes yet too complicated if Baikal related code is added there, perhaps under #ifdef CONFIG_OF like MSCC Ocelot code is currently. -- Jarkko
[PATCH v2 08/12] i2c: designware: Introduce platform drivers glue layer interface
Seeing the DW I2C platform driver is getting overcomplicated with a lot of
vendor-specific configs let's introduce a glue-layer interface so new
platforms which equipped with Synopsys Designware APB I2C IP-core would
be able to handle their peculiarities in the dedicated objects.
The generic platform setups and cleanups can now be performed by means of
two new functions exported from the Dw I2C platform driver:
int i2c_dw_plat_setup(struct dw_i2c_dev *dev);
int i2c_dw_plat_clear(struct dw_i2c_dev *dev);
They also install and remove the I2C controller respectively. In addition
if device supports the PM interface a glue driver can use the generic
platform PM callbacks collected into the PM dev ops structure:
const struct dev_pm_ops i2c_dw_plat_dev_pm_ops;
Before setting the platform DW I2C device up the glue probe code is
supposed to create an instance of `struct dw_i2c_dev` and pre-initialize
its `struct device` pointer together with optional platform-specific
flags. Currently the ACCESS_NO_IRQ_SUSPEND and ACCESS_INTR_MASK flags are
supported.
Note we moved the platform driver private data setup to the generic
platform probe method. By doing so the driver data pointer will be free
to be used by the glue-layer driver.
Suggested-by: Andy Shevchenko
Signed-off-by: Serge Semin
Cc: Alexey Malahov
Cc: Thomas Bogendoerfer
Cc: Paul Burton
Cc: Ralf Baechle
Cc: Rob Herring
Cc: Frank Rowand
Cc: linux-m...@vger.kernel.org
Cc: devicet...@vger.kernel.org
---
drivers/i2c/busses/i2c-designware-core.h| 4 +
drivers/i2c/busses/i2c-designware-platdrv.c | 84 +
drivers/i2c/busses/i2c-designware-platdrv.h | 16
3 files changed, 71 insertions(+), 33 deletions(-)
create mode 100644 drivers/i2c/busses/i2c-designware-platdrv.h
diff --git a/drivers/i2c/busses/i2c-designware-core.h
b/drivers/i2c/busses/i2c-designware-core.h
index b5b981c1bb0d..10606266b30c 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -8,6 +8,8 @@
* Copyright (C) 2007 MontaVista Software Inc.
* Copyright (C) 2009 Provigent Ltd.
*/
+#ifndef __I2C_DESIGNWARE_CORE_H__
+#define __I2C_DESIGNWARE_CORE_H__
#include
#include
@@ -324,3 +326,5 @@ extern int i2c_dw_probe_lock_support(struct dw_i2c_dev
*dev);
#else
static inline int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev) { return
0; }
#endif
+
+#endif /* __I2C_DESIGNWARE_CORE_H__ */
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 5536673060cc..274953155569 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -32,6 +32,7 @@
#include
#include "i2c-designware-core.h"
+#include "i2c-designware-platdrv.h"
static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
{
@@ -80,9 +81,9 @@ static void dw_i2c_acpi_params(struct platform_device *pdev,
char method[],
kfree(buf.pointer);
}
-static int dw_i2c_acpi_configure(struct platform_device *pdev)
+static int dw_i2c_acpi_configure(struct dw_i2c_dev *dev)
{
- struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
+ struct platform_device *pdev = to_platform_device(dev->dev);
struct i2c_timings *t = &dev->timings;
u32 ss_ht = 0, fp_ht = 0, hs_ht = 0, fs_ht = 0;
@@ -135,7 +136,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
};
MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
#else
-static inline int dw_i2c_acpi_configure(struct platform_device *pdev)
+static inline int dw_i2c_acpi_configure(struct dw_i2c_dev *dev)
{
return -ENODEV;
}
@@ -154,9 +155,9 @@ static int mscc_twi_set_sda_hold_time(struct dw_i2c_dev
*dev)
return 0;
}
-static int dw_i2c_of_configure(struct platform_device *pdev)
+static int dw_i2c_of_configure(struct dw_i2c_dev *dev)
{
- struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
+ struct platform_device *pdev = to_platform_device(dev->dev);
struct resource *mem;
switch (dev->flags & MODEL_MASK) {
@@ -180,7 +181,7 @@ static const struct of_device_id dw_i2c_of_match[] = {
};
MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
#else
-static inline int dw_i2c_of_configure(struct platform_device *pdev)
+static inline int dw_i2c_of_configure(struct dw_i2c_dev *dev)
{
return -ENODEV;
}
@@ -234,33 +235,25 @@ static const u32 supported_speeds[] = {
I2C_MAX_STANDARD_MODE_FREQ,
};
-static int dw_i2c_plat_probe(struct platform_device *pdev)
+int i2c_dw_plat_setup(struct dw_i2c_dev *dev)
{
+ struct platform_device *pdev = to_platform_device(dev->dev);
struct dw_i2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
struct i2c_adapter *adap;
- struct dw_i2c_dev *dev;
struct i2c_timings *t;
u32 acpi_speed;
struct resource *mem;
- int i, irq, ret;
+ int i, ret;
- irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- retu
