| Message ID | 20230725143023.86325-4-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: designware: code consolidation & cleanups | expand |
Hi Andy, On Tue, Jul 25, 2023 at 05:30:17PM +0300, Andy Shevchenko wrote: > For the sake of consistency align dw_i2c_of_configure() with > i2c_dw_acpi_configure() and rename the former to i2c_dw_of_configure(). > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/i2c/busses/i2c-designware-platdrv.c | 19 +++++++++---------- > 1 file changed, 9 insertions(+), 10 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c > index c60e55b8398e..d35a6bbcb6fb 100644 > --- a/drivers/i2c/busses/i2c-designware-platdrv.c > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c > @@ -132,9 +132,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 void i2c_dw_of_do_configure(struct dw_i2c_dev *dev, struct device *device) > { > - struct dw_i2c_dev *dev = platform_get_drvdata(pdev); > + struct platform_device *pdev = to_platform_device(device); > > switch (dev->flags & MODEL_MASK) { > case MODEL_MSCC_OCELOT: > @@ -145,8 +145,12 @@ static int dw_i2c_of_configure(struct platform_device *pdev) > default: > break; > } > +} > > - return 0; > +static void i2c_dw_of_configure(struct dw_i2c_dev *dev) > +{ > + if (dev_of_node(dev->dev)) > + i2c_dw_of_do_configure(dev, dev->dev); You could add this check above and avoid this micro-if-functions. if (!dev_of_node(dev->dev)) return; up to you... Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Andi > } > > static const struct of_device_id dw_i2c_of_match[] = { > @@ -162,10 +166,7 @@ static int bt1_i2c_request_regs(struct dw_i2c_dev *dev) > return -ENODEV; > } > > -static inline int dw_i2c_of_configure(struct platform_device *pdev) > -{ > - return -ENODEV; > -} > +static inline void i2c_dw_of_configure(struct dw_i2c_dev *dev) { } > #endif > > static int txgbe_i2c_request_regs(struct dw_i2c_dev *dev) > @@ -311,9 +312,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev) > > i2c_dw_adjust_bus_speed(dev); > > - if (pdev->dev.of_node) > - dw_i2c_of_configure(pdev); > - > + i2c_dw_of_configure(dev); > i2c_dw_acpi_configure(dev); > > ret = i2c_dw_validate_speed(dev); > -- > 2.40.0.1.gaa8946217a0b >
On Tue, Jul 25, 2023 at 11:48:36PM +0200, Andi Shyti wrote: > On Tue, Jul 25, 2023 at 05:30:17PM +0300, Andy Shevchenko wrote: ... > > +static void i2c_dw_of_configure(struct dw_i2c_dev *dev) > > +{ > > + if (dev_of_node(dev->dev)) > > + i2c_dw_of_do_configure(dev, dev->dev); > > You could add this check above and avoid this micro-if-functions. > > if (!dev_of_node(dev->dev)) > return; > > up to you... Have you had a chance to look into patch 7? Maybe you can come up with some advice or ideas on how to make the series better... > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Thank you! > > }
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c index c60e55b8398e..d35a6bbcb6fb 100644 --- a/drivers/i2c/busses/i2c-designware-platdrv.c +++ b/drivers/i2c/busses/i2c-designware-platdrv.c @@ -132,9 +132,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 void i2c_dw_of_do_configure(struct dw_i2c_dev *dev, struct device *device) { - struct dw_i2c_dev *dev = platform_get_drvdata(pdev); + struct platform_device *pdev = to_platform_device(device); switch (dev->flags & MODEL_MASK) { case MODEL_MSCC_OCELOT: @@ -145,8 +145,12 @@ static int dw_i2c_of_configure(struct platform_device *pdev) default: break; } +} - return 0; +static void i2c_dw_of_configure(struct dw_i2c_dev *dev) +{ + if (dev_of_node(dev->dev)) + i2c_dw_of_do_configure(dev, dev->dev); } static const struct of_device_id dw_i2c_of_match[] = { @@ -162,10 +166,7 @@ static int bt1_i2c_request_regs(struct dw_i2c_dev *dev) return -ENODEV; } -static inline int dw_i2c_of_configure(struct platform_device *pdev) -{ - return -ENODEV; -} +static inline void i2c_dw_of_configure(struct dw_i2c_dev *dev) { } #endif static int txgbe_i2c_request_regs(struct dw_i2c_dev *dev) @@ -311,9 +312,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev) i2c_dw_adjust_bus_speed(dev); - if (pdev->dev.of_node) - dw_i2c_of_configure(pdev); - + i2c_dw_of_configure(dev); i2c_dw_acpi_configure(dev); ret = i2c_dw_validate_speed(dev);
For the sake of consistency align dw_i2c_of_configure() with i2c_dw_acpi_configure() and rename the former to i2c_dw_of_configure(). Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/i2c/busses/i2c-designware-platdrv.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)