[PATCH 1/3] i2c: davinci: kill platform data

[PATCH 1/3] i2c: davinci: kill platform data

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

There are no more board file users of this driver. The platform data
structure is only used internally. Two of the four fields it stores are
not used at all anymore. Pull the remainder into the driver data struct
and shrink code by removing parts that are now dead code.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/i2c/busses/i2c-davinci.c          | 80 +++++------------------
 include/linux/platform_data/i2c-davinci.h | 26 --------
 2 files changed, 17 insertions(+), 89 deletions(-)
 delete mode 100644 include/linux/platform_data/i2c-davinci.h

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 71dc0a6688b7..4b499931fdfd 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -26,7 +26,6 @@
 #include <linux/cpufreq.h>
 #include <linux/gpio/consumer.h>
 #include <linux/of.h>
-#include <linux/platform_data/i2c-davinci.h>
 #include <linux/pm_runtime.h>
 
 /* ----- global defines ----------------------------------------------- */
@@ -117,6 +116,8 @@
 /* timeout for pm runtime autosuspend */
 #define DAVINCI_I2C_PM_TIMEOUT	1000	/* ms */
 
+#define DAVINCI_I2C_DEFAULT_BUS_FREQ	100
+
 struct davinci_i2c_dev {
 	struct device           *dev;
 	void __iomem		*base;
@@ -132,13 +133,10 @@ struct davinci_i2c_dev {
 #ifdef CONFIG_CPU_FREQ
 	struct notifier_block	freq_transition;
 #endif
-	struct davinci_i2c_platform_data *pdata;
-};
-
-/* default platform data to use if not supplied in the platform_device */
-static struct davinci_i2c_platform_data davinci_i2c_platform_data_default = {
-	.bus_freq	= 100,
-	.bus_delay	= 0,
+	/* standard bus frequency (kHz) */
+	unsigned int		bus_freq;
+	/* Chip has a ICPFUNC register */
+	bool			has_pfunc;
 };
 
 static inline void davinci_i2c_write_reg(struct davinci_i2c_dev *i2c_dev,
@@ -168,7 +166,6 @@ static inline void davinci_i2c_reset_ctrl(struct davinci_i2c_dev *i2c_dev,
 
 static void i2c_davinci_calc_clk_dividers(struct davinci_i2c_dev *dev)
 {
-	struct davinci_i2c_platform_data *pdata = dev->pdata;
 	u16 psc;
 	u32 clk;
 	u32 d;
@@ -212,16 +209,16 @@ static void i2c_davinci_calc_clk_dividers(struct davinci_i2c_dev *dev)
 	if (of_node && of_device_is_compatible(of_node, "ti,keystone-i2c"))
 		d = 6;
 
-	clk = ((input_clock / (psc + 1)) / (pdata->bus_freq * 1000));
+	clk = ((input_clock / (psc + 1)) / (dev->bus_freq * 1000));
 	/* Avoid driving the bus too fast because of rounding errors above */
-	if (input_clock / (psc + 1) / clk > pdata->bus_freq * 1000)
+	if (input_clock / (psc + 1) / clk > dev->bus_freq * 1000)
 		clk++;
 	/*
 	 * According to I2C-BUS Spec 2.1, in FAST-MODE LOW period should be at
 	 * least 1.3uS, which is not the case with 50% duty cycle. Driving HIGH
 	 * to LOW ratio as 1 to 2 is more safe.
 	 */
-	if (pdata->bus_freq > 100)
+	if (dev->bus_freq > 100)
 		clkl = (clk << 1) / 3;
 	else
 		clkl = (clk >> 1);
@@ -255,8 +252,6 @@ static void i2c_davinci_calc_clk_dividers(struct davinci_i2c_dev *dev)
  */
 static int i2c_davinci_init(struct davinci_i2c_dev *dev)
 {
-	struct davinci_i2c_platform_data *pdata = dev->pdata;
-
 	/* put I2C into reset */
 	davinci_i2c_reset_ctrl(dev, 0);
 
@@ -274,8 +269,7 @@ static int i2c_davinci_init(struct davinci_i2c_dev *dev)
 		davinci_i2c_read_reg(dev, DAVINCI_I2C_CLKL_REG));
 	dev_dbg(dev->dev, "CLKH = %d\n",
 		davinci_i2c_read_reg(dev, DAVINCI_I2C_CLKH_REG));
-	dev_dbg(dev->dev, "bus_freq = %dkHz, bus_delay = %d\n",
-		pdata->bus_freq, pdata->bus_delay);
+	dev_dbg(dev->dev, "bus_freq = %dkHz\n", dev->bus_freq);
 
 
 	/* Take the I2C module out of reset: */
@@ -309,12 +303,6 @@ static void davinci_i2c_unprepare_recovery(struct i2c_adapter *adap)
 	i2c_davinci_init(dev);
 }
 
-static struct i2c_bus_recovery_info davinci_i2c_gpio_recovery_info = {
-	.recover_bus = i2c_generic_scl_recovery,
-	.prepare_recovery = davinci_i2c_prepare_recovery,
-	.unprepare_recovery = davinci_i2c_unprepare_recovery,
-};
-
 static void davinci_i2c_set_scl(struct i2c_adapter *adap, int val)
 {
 	struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
@@ -414,7 +402,6 @@ static int
 i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
 {
 	struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
-	struct davinci_i2c_platform_data *pdata = dev->pdata;
 	u32 flag;
 	u16 w;
 	unsigned long time_left;
@@ -424,10 +411,6 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
 		return -EADDRNOTAVAIL;
 	}
 
-	/* Introduce a delay, required for some boards (e.g Davinci EVM) */
-	if (pdata->bus_delay)
-		udelay(pdata->bus_delay);
-
 	/* set the target address */
 	davinci_i2c_write_reg(dev, DAVINCI_I2C_SAR_REG, msg->addr);
 
@@ -758,8 +741,8 @@ static int davinci_i2c_probe(struct platform_device *pdev)
 {
 	struct davinci_i2c_dev *dev;
 	struct i2c_adapter *adap;
-	struct i2c_bus_recovery_info *rinfo;
 	int r, irq;
+	u32 prop;
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
@@ -773,29 +756,15 @@ static int davinci_i2c_probe(struct platform_device *pdev)
 
 	dev->dev = &pdev->dev;
 	dev->irq = irq;
-	dev->pdata = dev_get_platdata(&pdev->dev);
 	platform_set_drvdata(pdev, dev);
 
-	if (!dev->pdata && pdev->dev.of_node) {
-		u32 prop;
+	r = device_property_read_u32(&pdev->dev, "clock-frequency", &prop);
+	if (r)
+		prop = DAVINCI_I2C_DEFAULT_BUS_FREQ;
 
-		dev->pdata = devm_kzalloc(&pdev->dev,
-			sizeof(struct davinci_i2c_platform_data), GFP_KERNEL);
-		if (!dev->pdata)
-			return -ENOMEM;
+	dev->bus_freq = prop / 1000;
 
-		memcpy(dev->pdata, &davinci_i2c_platform_data_default,
-			sizeof(struct davinci_i2c_platform_data));
-		if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency",
-			&prop))
-			dev->pdata->bus_freq = prop / 1000;
-
-		dev->pdata->has_pfunc =
-			of_property_read_bool(pdev->dev.of_node,
-					      "ti,has-pfunc");
-	} else if (!dev->pdata) {
-		dev->pdata = &davinci_i2c_platform_data_default;
-	}
+	dev->has_pfunc = device_property_present(&pdev->dev, "ti,has-pfunc");
 
 	dev->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(dev->clk))
@@ -843,23 +812,8 @@ static int davinci_i2c_probe(struct platform_device *pdev)
 	adap->timeout = DAVINCI_I2C_TIMEOUT;
 	adap->dev.of_node = pdev->dev.of_node;
 
-	if (dev->pdata->has_pfunc)
+	if (dev->has_pfunc)
 		adap->bus_recovery_info = &davinci_i2c_scl_recovery_info;
-	else if (dev->pdata->gpio_recovery) {
-		rinfo =  &davinci_i2c_gpio_recovery_info;
-		adap->bus_recovery_info = rinfo;
-		rinfo->scl_gpiod = devm_gpiod_get(&pdev->dev, "scl",
-						  GPIOD_OUT_HIGH_OPEN_DRAIN);
-		if (IS_ERR(rinfo->scl_gpiod)) {
-			r = PTR_ERR(rinfo->scl_gpiod);
-			goto err_unuse_clocks;
-		}
-		rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN);
-		if (IS_ERR(rinfo->sda_gpiod)) {
-			r = PTR_ERR(rinfo->sda_gpiod);
-			goto err_unuse_clocks;
-		}
-	}
 
 	adap->nr = pdev->id;
 	r = i2c_add_numbered_adapter(adap);
diff --git a/include/linux/platform_data/i2c-davinci.h b/include/linux/platform_data/i2c-davinci.h
deleted file mode 100644
index 98967df07468..000000000000
--- a/include/linux/platform_data/i2c-davinci.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/*
- * DaVinci I2C controller platform_device info
- *
- * Author: Vladimir Barinov, MontaVista Software, Inc. <source@mvista.com>
- *
- * 2007 (c) MontaVista Software, Inc. This file is licensed under
- * the terms of the GNU General Public License version 2. This program
- * is licensed "as is" without any warranty of any kind, whether express
- * or implied.
-*/
-
-#ifndef __ASM_ARCH_I2C_H
-#define __ASM_ARCH_I2C_H
-
-/* All frequencies are expressed in kHz */
-struct davinci_i2c_platform_data {
-	unsigned int	bus_freq;	/* standard bus frequency (kHz) */
-	unsigned int	bus_delay;	/* post-transaction delay (usec) */
-	bool		gpio_recovery;	/* Use GPIO recovery method */
-	bool		has_pfunc;	/* Chip has a ICPFUNC register */
-};
-
-/* for board setup code */
-void davinci_init_i2c(struct davinci_i2c_platform_data *);
-
-#endif /* __ASM_ARCH_I2C_H */
-- 
2.45.2

[PATCH 2/3] i2c: davinci: order includes alphabetically

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

For better readability order included headers alphabetically.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/i2c/busses/i2c-davinci.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 4b499931fdfd..bab9f785eeec 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -11,22 +11,23 @@
  *
  * ----------------------------------------------------------------------------
  */
+
+#include <linux/clk.h>
+#include <linux/cpufreq.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/delay.h>
-#include <linux/i2c.h>
-#include <linux/clk.h>
-#include <linux/errno.h>
-#include <linux/sched.h>
-#include <linux/err.h>
-#include <linux/interrupt.h>
-#include <linux/platform_device.h>
-#include <linux/io.h>
-#include <linux/slab.h>
-#include <linux/cpufreq.h>
-#include <linux/gpio/consumer.h>
 #include <linux/of.h>
+#include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
 
 /* ----- global defines ----------------------------------------------- */
 
-- 
2.45.2

[PATCH 3/3] i2c: davinci: use generic device property accessors

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Don't use generic OF APIs if the generic device-level ones will do.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/i2c/busses/i2c-davinci.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index bab9f785eeec..6a909d339681 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -23,9 +23,9 @@
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
 
@@ -173,7 +173,6 @@ static void i2c_davinci_calc_clk_dividers(struct davinci_i2c_dev *dev)
 	u32 clkh;
 	u32 clkl;
 	u32 input_clock = clk_get_rate(dev->clk);
-	struct device_node *of_node = dev->dev->of_node;
 
 	/* NOTE: I2C Clock divider programming info
 	 * As per I2C specs the following formulas provide prescaler
@@ -207,7 +206,7 @@ static void i2c_davinci_calc_clk_dividers(struct davinci_i2c_dev *dev)
 		psc++;	/* better to run under spec than over */
 	d = (psc >= 2) ? 5 : 7 - psc;
 
-	if (of_node && of_device_is_compatible(of_node, "ti,keystone-i2c"))
+	if (device_is_compatible(dev->dev, "ti,keystone-i2c"))
 		d = 6;
 
 	clk = ((input_clock / (psc + 1)) / (dev->bus_freq * 1000));
@@ -811,7 +810,7 @@ static int davinci_i2c_probe(struct platform_device *pdev)
 	adap->algo = &i2c_davinci_algo;
 	adap->dev.parent = &pdev->dev;
 	adap->timeout = DAVINCI_I2C_TIMEOUT;
-	adap->dev.of_node = pdev->dev.of_node;
+	adap->dev.of_node = dev_of_node(&pdev->dev);
 
 	if (dev->has_pfunc)
 		adap->bus_recovery_info = &davinci_i2c_scl_recovery_info;
-- 
2.45.2

Re: [PATCH 1/3] i2c: davinci: kill platform data

[Andi Shyti]

Hi Bartosz,

...

>  struct davinci_i2c_dev {
>  	struct device           *dev;
>  	void __iomem		*base;
> @@ -132,13 +133,10 @@ struct davinci_i2c_dev {
>  #ifdef CONFIG_CPU_FREQ
>  	struct notifier_block	freq_transition;
>  #endif
> -	struct davinci_i2c_platform_data *pdata;
> -};
> -
> -/* default platform data to use if not supplied in the platform_device */
> -static struct davinci_i2c_platform_data davinci_i2c_platform_data_default = {
> -	.bus_freq	= 100,
> -	.bus_delay	= 0,

what happened to bus_delay?

...

> +	/* standard bus frequency (kHz) */
> +	unsigned int		bus_freq;
> +	/* Chip has a ICPFUNC register */
> +	bool			has_pfunc;
>  };

> -static struct i2c_bus_recovery_info davinci_i2c_gpio_recovery_info = {
> -	.recover_bus = i2c_generic_scl_recovery,
> -	.prepare_recovery = davinci_i2c_prepare_recovery,
> -	.unprepare_recovery = davinci_i2c_unprepare_recovery,
> -};
> -

what happened to the gpio_recovery_info?

Andi

Re: [PATCH 2/3] i2c: davinci: order includes alphabetically

[Andi Shyti]

Hi Bartosz,

On Wed, Dec 11, 2024 at 11:23:36AM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> For better readability order included headers alphabetically.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Reviewed-by: Andi Shyti <andi.shyti@kernel.org>

Thanks,
Andi

Re: [PATCH 3/3] i2c: davinci: use generic device property accessors

[Andi Shyti]

Hi Bartosz,

On Wed, Dec 11, 2024 at 11:23:37AM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Don't use generic OF APIs if the generic device-level ones will do.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Reviewed-by: Andi Shyti <andi.shyti@kernel.org>

Thanks,
Andi

Re: [PATCH 1/3] i2c: davinci: kill platform data

[Bartosz Golaszewski]

On Fri, Dec 27, 2024 at 12:41 AM Andi Shyti <andi.shyti@kernel.org> wrote:
>
> Hi Bartosz,
>
> ...
>
> >  struct davinci_i2c_dev {
> >       struct device           *dev;
> >       void __iomem            *base;
> > @@ -132,13 +133,10 @@ struct davinci_i2c_dev {
> >  #ifdef CONFIG_CPU_FREQ
> >       struct notifier_block   freq_transition;
> >  #endif
> > -     struct davinci_i2c_platform_data *pdata;
> > -};
> > -
> > -/* default platform data to use if not supplied in the platform_device */
> > -static struct davinci_i2c_platform_data davinci_i2c_platform_data_default = {
> > -     .bus_freq       = 100,
> > -     .bus_delay      = 0,
>
> what happened to bus_delay?
>

bus_delay is not set by means other than platform data and it defaults
to 0 so it's safe to just remove it.

> ...
>
> > +     /* standard bus frequency (kHz) */
> > +     unsigned int            bus_freq;
> > +     /* Chip has a ICPFUNC register */
> > +     bool                    has_pfunc;
> >  };
>
> > -static struct i2c_bus_recovery_info davinci_i2c_gpio_recovery_info = {
> > -     .recover_bus = i2c_generic_scl_recovery,
> > -     .prepare_recovery = davinci_i2c_prepare_recovery,
> > -     .unprepare_recovery = davinci_i2c_unprepare_recovery,
> > -};
> > -
>
> what happened to the gpio_recovery_info?
>

Similar story: it's only ever used if there's a pdata-provided
recovery GPIO line set but nobody does it anymore.

Bartosz

Re: [PATCH 1/3] i2c: davinci: kill platform data

[Andi Shyti]

Hi Bartosz,

On Thu, Jan 02, 2025 at 01:44:10PM +0100, Bartosz Golaszewski wrote:
> On Fri, Dec 27, 2024 at 12:41 AM Andi Shyti <andi.shyti@kernel.org> wrote:
> >
> > Hi Bartosz,
> >
> > ...
> >
> > >  struct davinci_i2c_dev {
> > >       struct device           *dev;
> > >       void __iomem            *base;
> > > @@ -132,13 +133,10 @@ struct davinci_i2c_dev {
> > >  #ifdef CONFIG_CPU_FREQ
> > >       struct notifier_block   freq_transition;
> > >  #endif
> > > -     struct davinci_i2c_platform_data *pdata;
> > > -};
> > > -
> > > -/* default platform data to use if not supplied in the platform_device */
> > > -static struct davinci_i2c_platform_data davinci_i2c_platform_data_default = {
> > > -     .bus_freq       = 100,
> > > -     .bus_delay      = 0,
> >
> > what happened to bus_delay?
> >
> 
> bus_delay is not set by means other than platform data and it defaults
> to 0 so it's safe to just remove it.

yes, but how is it related to this patch? Can we put it on a
different patch?

Thanks,
Andi

> > ...
> >
> > > +     /* standard bus frequency (kHz) */
> > > +     unsigned int            bus_freq;
> > > +     /* Chip has a ICPFUNC register */
> > > +     bool                    has_pfunc;
> > >  };
> >
> > > -static struct i2c_bus_recovery_info davinci_i2c_gpio_recovery_info = {
> > > -     .recover_bus = i2c_generic_scl_recovery,
> > > -     .prepare_recovery = davinci_i2c_prepare_recovery,
> > > -     .unprepare_recovery = davinci_i2c_unprepare_recovery,
> > > -};
> > > -
> >
> > what happened to the gpio_recovery_info?
> >
> 
> Similar story: it's only ever used if there's a pdata-provided
> recovery GPIO line set but nobody does it anymore.
> 
> Bartosz

Re: [PATCH 1/3] i2c: davinci: kill platform data

[Bartosz Golaszewski]

On Sat, Jan 4, 2025 at 1:02 AM Andi Shyti <andi.shyti@kernel.org> wrote:
>
> > > > -
> > > > -/* default platform data to use if not supplied in the platform_device */
> > > > -static struct davinci_i2c_platform_data davinci_i2c_platform_data_default = {
> > > > -     .bus_freq       = 100,
> > > > -     .bus_delay      = 0,
> > >
> > > what happened to bus_delay?
> > >
> >
> > bus_delay is not set by means other than platform data and it defaults
> > to 0 so it's safe to just remove it.
>
> yes, but how is it related to this patch? Can we put it on a
> different patch?
>

No, why? This patch removes platform data and all bits and pieces
associated with it. Splitting it into two just to first remove
bus_delay and then the rest doesn't make much sense IMO. The argument
that it's already not used would be incorrect as it IS used by
platform data (even though it itself is no longer defined anywhere).
I'd keep it as is.

Bart

Re: [PATCH 1/3] i2c: davinci: kill platform data

[Andi Shyti]

Hi Bartosz,

On Mon, Jan 06, 2025 at 11:31:23AM +0100, Bartosz Golaszewski wrote:
> On Sat, Jan 4, 2025 at 1:02 AM Andi Shyti <andi.shyti@kernel.org> wrote:
> > > > > -/* default platform data to use if not supplied in the platform_device */
> > > > > -static struct davinci_i2c_platform_data davinci_i2c_platform_data_default = {
> > > > > -     .bus_freq       = 100,
> > > > > -     .bus_delay      = 0,
> > > >
> > > > what happened to bus_delay?
> > > >
> > >
> > > bus_delay is not set by means other than platform data and it defaults
> > > to 0 so it's safe to just remove it.
> >
> > yes, but how is it related to this patch? Can we put it on a
> > different patch?
> >
> 
> No, why? This patch removes platform data and all bits and pieces
> associated with it. Splitting it into two just to first remove
> bus_delay and then the rest doesn't make much sense IMO. The argument
> that it's already not used would be incorrect as it IS used by
> platform data (even though it itself is no longer defined anywhere).
> I'd keep it as is.

yeah... merged to i2c/i2c-host.

Thanks,
Andi