Linux-i2c Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] i2c: revert "i2c: core: Allow drivers to disable i2c-core irq mapping"
@ 2020-06-30 16:24 Wolfram Sang
  2020-07-03 12:45 ` Hans de Goede
  2020-07-24 19:52 ` Wolfram Sang
  0 siblings, 2 replies; 4+ messages in thread
From: Wolfram Sang @ 2020-06-30 16:24 UTC (permalink / raw)
  To: linux-i2c; +Cc: Hans de Goede, Wolfram Sang

This manually reverts commit d1d84bb95364ed604015c2b788caaf3dbca0262f.
The only user has gone two years ago with commit 589edb56b424 ("ACPI /
scan: Create platform device for INT33FE ACPI nodes") and no new user
has showed up. Remove and hope we will never need it again.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Build tested only. Looking for Hans' opinion here.

 drivers/i2c/i2c-core-base.c | 6 +++---
 include/linux/i2c.h         | 3 ---
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 26f03a14a478..dc43242a85ba 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -319,11 +319,9 @@ static int i2c_device_probe(struct device *dev)
 	if (!client)
 		return 0;
 
-	driver = to_i2c_driver(dev->driver);
-
 	client->irq = client->init_irq;
 
-	if (!client->irq && !driver->disable_i2c_core_irq_mapping) {
+	if (!client->irq) {
 		int irq = -ENOENT;
 
 		if (client->flags & I2C_CLIENT_HOST_NOTIFY) {
@@ -349,6 +347,8 @@ static int i2c_device_probe(struct device *dev)
 		client->irq = irq;
 	}
 
+	driver = to_i2c_driver(dev->driver);
+
 	/*
 	 * An I2C ID table is not mandatory, if and only if, a suitable OF
 	 * or ACPI ID table is supplied for the probing device.
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index b8b8963f8bb9..098405df431f 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -231,7 +231,6 @@ enum i2c_alert_protocol {
  * @detect: Callback for device detection
  * @address_list: The I2C addresses to probe (for detect)
  * @clients: List of detected clients we created (for i2c-core use only)
- * @disable_i2c_core_irq_mapping: Tell the i2c-core to not do irq-mapping
  *
  * The driver.owner field should be set to the module owner of this driver.
  * The driver.name field should be set to the name of this driver.
@@ -290,8 +289,6 @@ struct i2c_driver {
 	int (*detect)(struct i2c_client *client, struct i2c_board_info *info);
 	const unsigned short *address_list;
 	struct list_head clients;
-
-	bool disable_i2c_core_irq_mapping;
 };
 #define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
 
-- 
2.27.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] i2c: revert "i2c: core: Allow drivers to disable i2c-core irq mapping"
  2020-06-30 16:24 [PATCH] i2c: revert "i2c: core: Allow drivers to disable i2c-core irq mapping" Wolfram Sang
@ 2020-07-03 12:45 ` Hans de Goede
  2020-07-24 19:56   ` Wolfram Sang
  2020-07-24 19:52 ` Wolfram Sang
  1 sibling, 1 reply; 4+ messages in thread
From: Hans de Goede @ 2020-07-03 12:45 UTC (permalink / raw)
  To: Wolfram Sang, linux-i2c

Hi,

On 6/30/20 6:24 PM, Wolfram Sang wrote:
> This manually reverts commit d1d84bb95364ed604015c2b788caaf3dbca0262f.
> The only user has gone two years ago with commit 589edb56b424 ("ACPI /
> scan: Create platform device for INT33FE ACPI nodes") and no new user
> has showed up. Remove and hope we will never need it again.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> Build tested only. Looking for Hans' opinion here.

I'm fine with removing this.

Semi off-topic:

Recently I did have another special case, see:

https://fedoraproject.org/wiki/Changes/RemoveDeviceMapperMultipathFromWorkstationLiveCD

During one of the iterations trying to deal with this
it would have been useful if the code instantiating
the client (rather then the driver for it) could have disabled
the i2c-core code for searching for an irq, without actually
specifying one. This would allow passing through the ACPI fwnode
as fwnode in the board_info, without triggering the ACPI IRQ
lookup code in the core.

So basically allow board_info to say:

"There is no IRQ and do not try to find one"

This could be as simple as having the instantiating code do:

	board_info.irq = -ENOENT;

Combined with the a tiny i2c-core change,
to deal with drivers checking for:

	if (i2c_client->irq)

Rather then for:

	if (i2c_client->irq > 0)

This tiny i2c-core change would look like this:

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 26f03a14a478..a7b05ef31f5f 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -343,11 +343,10 @@ static int i2c_device_probe(struct device *dev)
  			goto put_sync_adapter;
  		}

-		if (irq < 0)
-			irq = 0;
-
  		client->irq = irq;
  	}
+	if (client->irq < 0)
+		client->irq = 0;

  	/*
  	 * An I2C ID table is not mandatory, if and only if, a suitable OF

ATM I do not have a use-case for this, still I think this would be
useful to have. Would you be willing to take a patch with the above
change for this?

Regards,

Hans




> 
>   drivers/i2c/i2c-core-base.c | 6 +++---
>   include/linux/i2c.h         | 3 ---
>   2 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 26f03a14a478..dc43242a85ba 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -319,11 +319,9 @@ static int i2c_device_probe(struct device *dev)
>   	if (!client)
>   		return 0;
>   
> -	driver = to_i2c_driver(dev->driver);
> -
>   	client->irq = client->init_irq;
>   
> -	if (!client->irq && !driver->disable_i2c_core_irq_mapping) {
> +	if (!client->irq) {
>   		int irq = -ENOENT;
>   
>   		if (client->flags & I2C_CLIENT_HOST_NOTIFY) {
> @@ -349,6 +347,8 @@ static int i2c_device_probe(struct device *dev)
>   		client->irq = irq;
>   	}
>   
> +	driver = to_i2c_driver(dev->driver);
> +
>   	/*
>   	 * An I2C ID table is not mandatory, if and only if, a suitable OF
>   	 * or ACPI ID table is supplied for the probing device.
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index b8b8963f8bb9..098405df431f 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -231,7 +231,6 @@ enum i2c_alert_protocol {
>    * @detect: Callback for device detection
>    * @address_list: The I2C addresses to probe (for detect)
>    * @clients: List of detected clients we created (for i2c-core use only)
> - * @disable_i2c_core_irq_mapping: Tell the i2c-core to not do irq-mapping
>    *
>    * The driver.owner field should be set to the module owner of this driver.
>    * The driver.name field should be set to the name of this driver.
> @@ -290,8 +289,6 @@ struct i2c_driver {
>   	int (*detect)(struct i2c_client *client, struct i2c_board_info *info);
>   	const unsigned short *address_list;
>   	struct list_head clients;
> -
> -	bool disable_i2c_core_irq_mapping;
>   };
>   #define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
>   
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] i2c: revert "i2c: core: Allow drivers to disable i2c-core irq mapping"
  2020-06-30 16:24 [PATCH] i2c: revert "i2c: core: Allow drivers to disable i2c-core irq mapping" Wolfram Sang
  2020-07-03 12:45 ` Hans de Goede
@ 2020-07-24 19:52 ` Wolfram Sang
  1 sibling, 0 replies; 4+ messages in thread
From: Wolfram Sang @ 2020-07-24 19:52 UTC (permalink / raw)
  To: linux-i2c; +Cc: Hans de Goede


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

On Tue, Jun 30, 2020 at 06:24:40PM +0200, Wolfram Sang wrote:
> This manually reverts commit d1d84bb95364ed604015c2b788caaf3dbca0262f.
> The only user has gone two years ago with commit 589edb56b424 ("ACPI /
> scan: Create platform device for INT33FE ACPI nodes") and no new user
> has showed up. Remove and hope we will never need it again.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied to for-next, thanks!


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] i2c: revert "i2c: core: Allow drivers to disable i2c-core irq mapping"
  2020-07-03 12:45 ` Hans de Goede
@ 2020-07-24 19:56   ` Wolfram Sang
  0 siblings, 0 replies; 4+ messages in thread
From: Wolfram Sang @ 2020-07-24 19:56 UTC (permalink / raw)
  To: Hans de Goede; +Cc: linux-i2c


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

Hi Hans,

> > Build tested only. Looking for Hans' opinion here.
> 
> I'm fine with removing this.

Thanks!

[semi off-topic case]

> So basically allow board_info to say:
> 
> "There is no IRQ and do not try to find one"
> 
> This could be as simple as having the instantiating code do:
> 
> 	board_info.irq = -ENOENT;

...

> ATM I do not have a use-case for this, still I think this would be
> useful to have. Would you be willing to take a patch with the above
> change for this?

I haven't checked your code change in detail to check for side-effects.
In general, I think we could have something like this. However, I am a
bit conservative when it comes to changing something without a use case.

All the best,

   Wolfram


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-30 16:24 [PATCH] i2c: revert "i2c: core: Allow drivers to disable i2c-core irq mapping" Wolfram Sang
2020-07-03 12:45 ` Hans de Goede
2020-07-24 19:56   ` Wolfram Sang
2020-07-24 19:52 ` Wolfram Sang

Linux-i2c Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-i2c/0 linux-i2c/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-i2c linux-i2c/ https://lore.kernel.org/linux-i2c \
		linux-i2c@vger.kernel.org
	public-inbox-index linux-i2c

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-i2c


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git