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

[PATCH v3 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>
---
v3: rebase on top of latest rc
 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 a0d926ae3f864..b764c7c746e96 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.26.2

[PATCH v3 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>
---
v3: rebase on top of latest rc

 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 b764c7c746e96..928c6f5ceac70 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.26.2

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

[Peter Rosin]

On 2020-04-25 13:51, Andy Shevchenko wrote:
> 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>

Reviewed-by: Peter Rosin <peda@axentia.se>

Cheers,
Peter

[PATCH v3 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>
---
v3: rebase on top of latest rc

 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 928c6f5ceac70..0908a0655cb79 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.26.2

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

[Peter Rosin]

On 2020-04-25 13:51, Andy Shevchenko wrote:
> 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>

Reviewed-by: Peter Rosin <peda@axentia.se>

Cheers,
Peter

[PATCH v3 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>
---
v3: rebase on top of latest rc

 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 0908a0655cb79..4ad665757dd81 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.26.2

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

[Peter Rosin]

On 2020-04-25 13:51, Andy Shevchenko wrote:
> 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>

Reviewed-by: Peter Rosin <peda@axentia.se>

Cheers,
Peter

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

[Andy Shevchenko]

On Sat, Apr 25, 2020 at 02:51:49PM +0300, 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.

If there is no comments, Wolfram, can you apply this?

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v3: rebase on top of latest rc
>  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 a0d926ae3f864..b764c7c746e96 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.26.2
> 

-- 
With Best Regards,
Andy Shevchenko

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

[Peter Rosin]

On 2020-05-06 14:42, Andy Shevchenko wrote:
> On Sat, Apr 25, 2020 at 02:51:49PM +0300, 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.
> 
> If there is no comments, Wolfram, can you apply this?

Perhaps that's best, I have had the intention to get to this for a very long
time and I'm very sorry for delaying. The number of patches for i2c-mux is so
low that I have not bothered to automate the process, but that also means that
I have to sort of rediscover how to do it a few times a year. It's simply not
effective...

Wolfram, I think it would be better if I just quit my i2c-mux tree and I
instead review/ack patches that you then take. Would that be ok with you?

Again, sorry for being a road-block.

Reviewed-by: Peter Rosin <peda@axentia.se>

Cheers,
Peter

>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>> v3: rebase on top of latest rc
>>  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 a0d926ae3f864..b764c7c746e96 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.26.2
>>
> 

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 238 bytes --]


> Wolfram, I think it would be better if I just quit my i2c-mux tree and I
> instead review/ack patches that you then take. Would that be ok with you?

This is totally fine with me. Let's do it like this if it is less
overhead for you.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Andy Shevchenko]

On Wed, May 06, 2020 at 03:19:29PM +0200, Peter Rosin wrote:
> 
> On 2020-05-06 14:42, Andy Shevchenko wrote:
> > On Sat, Apr 25, 2020 at 02:51:49PM +0300, 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.
> > 
> > If there is no comments, Wolfram, can you apply this?
> 
> Perhaps that's best, I have had the intention to get to this for a very long
> time and I'm very sorry for delaying. The number of patches for i2c-mux is so
> low that I have not bothered to automate the process, but that also means that
> I have to sort of rediscover how to do it a few times a year. It's simply not
> effective...
> 
> Wolfram, I think it would be better if I just quit my i2c-mux tree and I
> instead review/ack patches that you then take. Would that be ok with you?
> 
> Again, sorry for being a road-block.
> 
> Reviewed-by: Peter Rosin <peda@axentia.se>

Thank you, Peter!

I hope this applies to the entire series?

-- 
With Best Regards,
Andy Shevchenko

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

[Peter Rosin]

On 2020-05-06 18:06, Andy Shevchenko wrote:
> On Wed, May 06, 2020 at 03:19:29PM +0200, Peter Rosin wrote:
>>
>> On 2020-05-06 14:42, Andy Shevchenko wrote:
>>> On Sat, Apr 25, 2020 at 02:51:49PM +0300, 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.
>>>
>>> If there is no comments, Wolfram, can you apply this?
>>
>> Perhaps that's best, I have had the intention to get to this for a very long
>> time and I'm very sorry for delaying. The number of patches for i2c-mux is so
>> low that I have not bothered to automate the process, but that also means that
>> I have to sort of rediscover how to do it a few times a year. It's simply not
>> effective...
>>
>> Wolfram, I think it would be better if I just quit my i2c-mux tree and I
>> instead review/ack patches that you then take. Would that be ok with you?
>>
>> Again, sorry for being a road-block.
>>
>> Reviewed-by: Peter Rosin <peda@axentia.se>
> 
> Thank you, Peter!
> 
> I hope this applies to the entire series?

I'll reply individually now that there's a plan, this helps Wolfram's automation,
I think.

Cheers,
Peter

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 373 bytes --]


> > Reviewed-by: Peter Rosin <peda@axentia.se>
> 
> Thank you, Peter!
> 
> I hope this applies to the entire series?

I interpreted it this way; anyhow, for me it would be easiest if every
patch gets tagged individually because then 'patchwork' picks them all
up correctly. So, if it is not too much work...

For this series:

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]