diff mbox series

[v7,1/6] i2c: wmt: create wmt_i2c_init for general init

Message ID eb2249f78697bd295d720c14501554a37ab65132.1704440251.git.hanshu-oc@zhaoxin.com
State Superseded
Headers show
Series i2c: add zhaoxin i2c controller driver | expand

Commit Message

Hans Hu Jan. 5, 2024, 7:51 a.m. UTC 
v5->v7:
	nothing changed.
v4->v5:
	add previous prototype 'static' for wmt_i2c_init().
	Link: https://lore.kernel.org/all/ZYx0VPVmyQhtG+B9@shikoro/1-a.txt

Some common initialization actions are put in the function
wmt_i2c_init(), which is convenient to share with zhaoxin.

Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
Signed-off-by: Hans Hu <hanshu-oc@zhaoxin.com>
---
 drivers/i2c/busses/i2c-wmt.c | 67 +++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 31 deletions(-)

Comments

Krzysztof Kozlowski Jan. 15, 2024, 3:17 p.m. UTC | #1 
On 05/01/2024 08:51, Hans Hu wrote:
> v5->v7:
> 	nothing changed.
> v4->v5:
> 	add previous prototype 'static' for wmt_i2c_init().
> 	Link: https://lore.kernel.org/all/ZYx0VPVmyQhtG+B9@shikoro/1-a.txt

Why there is only changelog and no commit msg? Changelog goes usually
under ---, especially if it is quite non-informative...

Best regards,
Krzysztof
Andi Shyti Jan. 21, 2024, 11:56 p.m. UTC | #2 
Hi Hans,

> > On 05/01/2024 08:51, Hans Hu wrote:
> > > v5->v7:
> > >        nothing changed.
> > > v4->v5:
> > >        add previous prototype 'static' for wmt_i2c_init().
> > >        Link: https://lore.kernel.org/all/ZYx0VPVmyQhtG+B9@shikoro/1-a.txt
> > Why there is only changelog and no commit msg? Changelog goes usually
> > under ---, especially if it is quite non-informative...
> 
> Commit msg comes after changelog. Yes, I should have put
> commit msg at the beginning. Other patches also have this
> problem. Adjustments will be made at the next submission.

While it can be convenient to include the changelog in each
patch from the reviewers perspective, having it all in patch 0 is
sufficient.

Personally, I'm not in favour of adding the changelog to the
commit log, even though it's a practice common in many
communities. However, I'm open to whatever approach you select, 
provided there is consistency.

If you decide to keep the changelog, consider placing it at the
end of the commit message, right before the tag section.

Andi
Wolfram Sang Feb. 21, 2024, 12:10 p.m. UTC | #3 
On Fri, Jan 05, 2024 at 03:51:44PM +0800, Hans Hu wrote:
> v5->v7:
> 	nothing changed.
> v4->v5:
> 	add previous prototype 'static' for wmt_i2c_init().
> 	Link: https://lore.kernel.org/all/ZYx0VPVmyQhtG+B9@shikoro/1-a.txt
> 
> Some common initialization actions are put in the function
> wmt_i2c_init(), which is convenient to share with zhaoxin.
> 
> Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
> Signed-off-by: Hans Hu <hanshu-oc@zhaoxin.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-wmt.c b/drivers/i2c/busses/i2c-wmt.c
index ec2a8da134e5..f1888f100d83 100644
--- a/drivers/i2c/busses/i2c-wmt.c
+++ b/drivers/i2c/busses/i2c-wmt.c
@@ -286,6 +286,38 @@  static irqreturn_t wmt_i2c_isr(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static int wmt_i2c_init(struct platform_device *pdev, struct wmt_i2c_dev **pi2c_dev)
+{
+	int err;
+	struct wmt_i2c_dev *i2c_dev;
+	struct device_node *np = pdev->dev.of_node;
+
+	i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
+	if (!i2c_dev)
+		return -ENOMEM;
+
+	i2c_dev->base = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
+	if (IS_ERR(i2c_dev->base))
+		return PTR_ERR(i2c_dev->base);
+
+	i2c_dev->irq = irq_of_parse_and_map(np, 0);
+	if (!i2c_dev->irq)
+		return -EINVAL;
+
+	err = devm_request_irq(&pdev->dev, i2c_dev->irq, wmt_i2c_isr,
+					0, pdev->name, i2c_dev);
+	if (err)
+		return dev_err_probe(&pdev->dev, err,
+				"failed to request irq %i\n", i2c_dev->irq);
+
+	i2c_dev->dev = &pdev->dev;
+	init_completion(&i2c_dev->complete);
+	platform_set_drvdata(pdev, i2c_dev);
+
+	*pi2c_dev = i2c_dev;
+	return 0;
+}
+
 static int wmt_i2c_reset_hardware(struct wmt_i2c_dev *i2c_dev)
 {
 	int err;
@@ -327,19 +359,9 @@  static int wmt_i2c_probe(struct platform_device *pdev)
 	int err;
 	u32 clk_rate;
 
-	i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
-	if (!i2c_dev)
-		return -ENOMEM;
-
-	i2c_dev->base = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
-	if (IS_ERR(i2c_dev->base))
-		return PTR_ERR(i2c_dev->base);
-
-	i2c_dev->irq = irq_of_parse_and_map(np, 0);
-	if (!i2c_dev->irq) {
-		dev_err(&pdev->dev, "irq missing or invalid\n");
-		return -EINVAL;
-	}
+	err = wmt_i2c_init(pdev, &i2c_dev);
+	if (err)
+		return err;
 
 	i2c_dev->clk = of_clk_get(np, 0);
 	if (IS_ERR(i2c_dev->clk)) {
@@ -351,15 +373,6 @@  static int wmt_i2c_probe(struct platform_device *pdev)
 	if (!err && (clk_rate == I2C_MAX_FAST_MODE_FREQ))
 		i2c_dev->tcr = TCR_FAST_MODE;
 
-	i2c_dev->dev = &pdev->dev;
-
-	err = devm_request_irq(&pdev->dev, i2c_dev->irq, wmt_i2c_isr, 0,
-							"i2c", i2c_dev);
-	if (err) {
-		dev_err(&pdev->dev, "failed to request irq %i\n", i2c_dev->irq);
-		return err;
-	}
-
 	adap = &i2c_dev->adapter;
 	i2c_set_adapdata(adap, i2c_dev);
 	strscpy(adap->name, "WMT I2C adapter", sizeof(adap->name));
@@ -368,21 +381,13 @@  static int wmt_i2c_probe(struct platform_device *pdev)
 	adap->dev.parent = &pdev->dev;
 	adap->dev.of_node = pdev->dev.of_node;
 
-	init_completion(&i2c_dev->complete);
-
 	err = wmt_i2c_reset_hardware(i2c_dev);
 	if (err) {
 		dev_err(&pdev->dev, "error initializing hardware\n");
 		return err;
 	}
 
-	err = i2c_add_adapter(adap);
-	if (err)
-		return err;
-
-	platform_set_drvdata(pdev, i2c_dev);
-
-	return 0;
+	return i2c_add_adapter(adap);
 }
 
 static void wmt_i2c_remove(struct platform_device *pdev)