[PATCH v2 1/4] i2c: mux: pca954x: Refactor pca954x_irq_handler()

[PATCH v2 1/4] i2c: mux: pca954x: Refactor pca954x_irq_handler()

[Andy Shevchenko]

Refactor pca954x_irq_handler() to:
  - use for_each_set_bit() macro
  - use IRQ_RETVAL() macro

Above change makes code easy to read and understand.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: masked pending to prevent handling of spurious IRQs (Peter)
 drivers/i2c/muxes/i2c-mux-pca954x.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index a0d926ae3f86..b764c7c746e9 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -327,21 +327,18 @@ static DEVICE_ATTR_RW(idle_state);
 static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
 {
 	struct pca954x *data = dev_id;
-	unsigned int child_irq;
-	int ret, i, handled = 0;
+	unsigned long pending;
+	int ret, i;
 
 	ret = i2c_smbus_read_byte(data->client);
 	if (ret < 0)
 		return IRQ_NONE;
 
-	for (i = 0; i < data->chip->nchans; i++) {
-		if (ret & BIT(PCA954X_IRQ_OFFSET + i)) {
-			child_irq = irq_linear_revmap(data->irq, i);
-			handle_nested_irq(child_irq);
-			handled++;
-		}
-	}
-	return handled ? IRQ_HANDLED : IRQ_NONE;
+	pending = (ret >> PCA954X_IRQ_OFFSET) & (BIT(data->chip->nchans) - 1);
+	for_each_set_bit(i, &pending, data->chip->nchans)
+		handle_nested_irq(irq_linear_revmap(data->irq, i));
+
+	return IRQ_RETVAL(pending);
 }
 
 static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
-- 
2.25.1

[PATCH v2 2/4] i2c: mux: pca954x: Make use of device properties

[Andy Shevchenko]

Device property API allows to gather device resources from different sources,
such as ACPI. Convert the drivers to unleash the power of device property API.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: dropped I2C table ID changes (Peter)
 drivers/i2c/muxes/i2c-mux-pca954x.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index b764c7c746e9..928c6f5ceac7 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -43,10 +43,8 @@
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_device.h>
-#include <linux/of_irq.h>
 #include <linux/pm.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <dt-bindings/mux/mux.h>
@@ -198,7 +196,6 @@ static const struct i2c_device_id pca954x_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, pca954x_id);
 
-#ifdef CONFIG_OF
 static const struct of_device_id pca954x_of_match[] = {
 	{ .compatible = "nxp,pca9540", .data = &chips[pca_9540] },
 	{ .compatible = "nxp,pca9542", .data = &chips[pca_9542] },
@@ -215,7 +212,6 @@ static const struct of_device_id pca954x_of_match[] = {
 	{}
 };
 MODULE_DEVICE_TABLE(of, pca954x_of_match);
-#endif
 
 /* Write to mux register. Don't use i2c_transfer()/i2c_smbus_xfer()
    for this as they will try to lock adapter a second time */
@@ -426,7 +422,6 @@ static int pca954x_probe(struct i2c_client *client,
 {
 	struct i2c_adapter *adap = client->adapter;
 	struct device *dev = &client->dev;
-	struct device_node *np = dev->of_node;
 	struct gpio_desc *gpio;
 	struct i2c_mux_core *muxc;
 	struct pca954x *data;
@@ -456,7 +451,7 @@ static int pca954x_probe(struct i2c_client *client,
 		udelay(1);
 	}
 
-	data->chip = of_device_get_match_data(dev);
+	data->chip = device_get_match_data(dev);
 	if (!data->chip)
 		data->chip = &chips[id->driver_data];
 
@@ -478,8 +473,8 @@ static int pca954x_probe(struct i2c_client *client,
 	}
 
 	data->idle_state = MUX_IDLE_AS_IS;
-	if (of_property_read_u32(np, "idle-state", &data->idle_state)) {
-		if (np && of_property_read_bool(np, "i2c-mux-idle-disconnect"))
+	if (device_property_read_u32(dev, "idle-state", &data->idle_state)) {
+		if (device_property_read_bool(dev, "i2c-mux-idle-disconnect"))
 			data->idle_state = MUX_IDLE_DISCONNECT;
 	}
 
@@ -562,7 +557,7 @@ static struct i2c_driver pca954x_driver = {
 	.driver		= {
 		.name	= "pca954x",
 		.pm	= &pca954x_pm,
-		.of_match_table = of_match_ptr(pca954x_of_match),
+		.of_match_table = pca954x_of_match,
 	},
 	.probe		= pca954x_probe,
 	.remove		= pca954x_remove,
-- 
2.25.1

[PATCH v2 3/4] i2c: mux: pca954x: Move device_remove_file() out of pca954x_cleanup()

[Andy Shevchenko]

device_create_file() is called the last in ->probe() but pca954x_cleanup(),
which is called earlier in error path, tries to remove never created file.
Move device_remove_file() call outside of pca954x_cleanup() to make it
slightly closer to what pca954x_init() is doing.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: no change
 drivers/i2c/muxes/i2c-mux-pca954x.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 928c6f5ceac7..0908a0655cb7 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -383,11 +383,8 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
 static void pca954x_cleanup(struct i2c_mux_core *muxc)
 {
 	struct pca954x *data = i2c_mux_priv(muxc);
-	struct i2c_client *client = data->client;
 	int c, irq;
 
-	device_remove_file(&client->dev, &dev_attr_idle_state);
-
 	if (data->irq) {
 		for (c = 0; c < data->chip->nchans; c++) {
 			irq = irq_find_mapping(data->irq, c);
@@ -531,6 +528,8 @@ static int pca954x_remove(struct i2c_client *client)
 {
 	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
 
+	device_remove_file(&client->dev, &dev_attr_idle_state);
+
 	pca954x_cleanup(muxc);
 	return 0;
 }
-- 
2.25.1

[PATCH v2 4/4] i2c: mux: pca954x: Convert license to SPDX identifier

[Andy Shevchenko]

Use a shorter SPDX identifier instead of pasting the whole license.

While here, replace duplicating PCA954x with PCA984x.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: no change
 drivers/i2c/muxes/i2c-mux-pca954x.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 0908a0655cb7..4ad665757dd8 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -1,10 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * I2C multiplexer
  *
  * Copyright (c) 2008-2009 Rodolfo Giometti <giometti@linux.it>
  * Copyright (c) 2008-2009 Eurotech S.p.A. <info@eurotech.it>
  *
- * This module supports the PCA954x and PCA954x series of I2C multiplexer/switch
+ * This module supports the PCA954x and PCA984x series of I2C multiplexer/switch
  * chips made by NXP Semiconductors.
  * This includes the:
  *	 PCA9540, PCA9542, PCA9543, PCA9544, PCA9545, PCA9546, PCA9547,
@@ -29,10 +30,6 @@
  *	i2c-virtual_cb.c from Brian Kuschak <bkuschak@yahoo.com>
  * and
  *	pca9540.c from Jean Delvare <jdelvare@suse.de>.
- *
- * 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.
  */
 
 #include <linux/device.h>
-- 
2.25.1

Re: [PATCH v2 1/4] i2c: mux: pca954x: Refactor pca954x_irq_handler()

[Andy Shevchenko]

On Mon, Mar 16, 2020 at 06:07:21PM +0200, Andy Shevchenko wrote:
> Refactor pca954x_irq_handler() to:
>   - use for_each_set_bit() macro
>   - use IRQ_RETVAL() macro
> 
> Above change makes code easy to read and understand.

Peter, does this series good in your opinion?

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: masked pending to prevent handling of spurious IRQs (Peter)
>  drivers/i2c/muxes/i2c-mux-pca954x.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index a0d926ae3f86..b764c7c746e9 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -327,21 +327,18 @@ static DEVICE_ATTR_RW(idle_state);
>  static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
>  {
>  	struct pca954x *data = dev_id;
> -	unsigned int child_irq;
> -	int ret, i, handled = 0;
> +	unsigned long pending;
> +	int ret, i;
>  
>  	ret = i2c_smbus_read_byte(data->client);
>  	if (ret < 0)
>  		return IRQ_NONE;
>  
> -	for (i = 0; i < data->chip->nchans; i++) {
> -		if (ret & BIT(PCA954X_IRQ_OFFSET + i)) {
> -			child_irq = irq_linear_revmap(data->irq, i);
> -			handle_nested_irq(child_irq);
> -			handled++;
> -		}
> -	}
> -	return handled ? IRQ_HANDLED : IRQ_NONE;
> +	pending = (ret >> PCA954X_IRQ_OFFSET) & (BIT(data->chip->nchans) - 1);
> +	for_each_set_bit(i, &pending, data->chip->nchans)
> +		handle_nested_irq(irq_linear_revmap(data->irq, i));
> +
> +	return IRQ_RETVAL(pending);
>  }
>  
>  static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
> -- 
> 2.25.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/4] i2c: mux: pca954x: Refactor pca954x_irq_handler()

[Andy Shevchenko]

On Wed, Mar 25, 2020 at 11:43:16PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 16, 2020 at 06:07:21PM +0200, Andy Shevchenko wrote:
> > Refactor pca954x_irq_handler() to:
> >   - use for_each_set_bit() macro
> >   - use IRQ_RETVAL() macro
> > 
> > Above change makes code easy to read and understand.
> 
> Peter, does this series good in your opinion?

Peter, Wolfram, this missed previous cycle, any work needs to be done?

> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > v2: masked pending to prevent handling of spurious IRQs (Peter)
> >  drivers/i2c/muxes/i2c-mux-pca954x.c | 17 +++++++----------
> >  1 file changed, 7 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> > index a0d926ae3f86..b764c7c746e9 100644
> > --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> > +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> > @@ -327,21 +327,18 @@ static DEVICE_ATTR_RW(idle_state);
> >  static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
> >  {
> >  	struct pca954x *data = dev_id;
> > -	unsigned int child_irq;
> > -	int ret, i, handled = 0;
> > +	unsigned long pending;
> > +	int ret, i;
> >  
> >  	ret = i2c_smbus_read_byte(data->client);
> >  	if (ret < 0)
> >  		return IRQ_NONE;
> >  
> > -	for (i = 0; i < data->chip->nchans; i++) {
> > -		if (ret & BIT(PCA954X_IRQ_OFFSET + i)) {
> > -			child_irq = irq_linear_revmap(data->irq, i);
> > -			handle_nested_irq(child_irq);
> > -			handled++;
> > -		}
> > -	}
> > -	return handled ? IRQ_HANDLED : IRQ_NONE;
> > +	pending = (ret >> PCA954X_IRQ_OFFSET) & (BIT(data->chip->nchans) - 1);
> > +	for_each_set_bit(i, &pending, data->chip->nchans)
> > +		handle_nested_irq(irq_linear_revmap(data->irq, i));
> > +
> > +	return IRQ_RETVAL(pending);
> >  }
> >  
> >  static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
> > -- 
> > 2.25.1
> > 
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

-- 
With Best Regards,
Andy Shevchenko