[PATCH v4 0/3] Enable named interrupt smbus-alert for ACPI

[PATCH v4 0/3] Enable named interrupt smbus-alert for ACPI

[Akhil R]

I2C - SMBus core drivers use named interrupts to support smbus_alert.
As named interrupts are not available for ACPI based systems, it was
required to change the i2c bus controller driver if to use smbus alert.
These patches provide option for named interrupts in ACPI and  make the
implementation similar to DT. This will enable use of interrupt named
'smbus-alert' in ACPI as well which will be taken during i2c adapter
register.

v3->v4:
  * Removed device_irq_get_byname()
  * Updates in comments and document.
v2->v3:
  * Grammar/spelling corrections.
  * Added description in function comments.
  * Removed 'unlikely' from NULL check on 'name'
v1->v2:
  * Added firmware guide documentation for ACPI named interrupts
  * Updates in function description comments

Akhil R (3):
  device property: Add fwnode_irq_get_byname
  docs: firmware-guide: ACPI: Add named interrupt doc
  i2c: smbus: Use device_*() functions instead of of_*()

 Documentation/firmware-guide/acpi/enumeration.rst | 39 +++++++++++++++++++++++
 drivers/base/property.c                           | 29 +++++++++++++++++
 drivers/i2c/i2c-core-base.c                       |  2 +-
 drivers/i2c/i2c-core-smbus.c                      | 10 +++---
 drivers/i2c/i2c-smbus.c                           |  3 +-
 include/linux/i2c-smbus.h                         |  6 ++--
 include/linux/property.h                          |  1 +
 7 files changed, 80 insertions(+), 10 deletions(-)

-- 
2.7.4

[PATCH v4 1/3] device property: Add fwnode_irq_get_byname

[Akhil R]

Add fwnode_irq_get_byname() to get an interrupt by name from either
ACPI table or Device Tree, whichever is used for enumeration.

In the ACPI case, this allow us to use 'interrupt-names' in
_DSD which can be mapped to Interrupt() resource by index.
The implementation is similar to 'interrupt-names' in the
Device Tree.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/base/property.c  | 29 +++++++++++++++++++++++++++++
 include/linux/property.h |  1 +
 2 files changed, 30 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index e6497f6..fc59e0f 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -936,6 +936,35 @@ void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index)
 EXPORT_SYMBOL(fwnode_iomap);
 
 /**
+ * fwnode_irq_get_byname - Get IRQ from a fwnode using its name
+ * @fwnode:	Pointer to the firmware node
+ * @name:	IRQ name
+ *
+ * Description:
+ * Find a match to the string @name in the 'interrupt-names' string array
+ * in _DSD for ACPI, or of_node for Device Tree. Then get the Linux IRQ
+ * number of the IRQ resource corresponding to the index of the matched
+ * string.
+ *
+ * Return:
+ * Linux IRQ number on success, or negative errno otherwise.
+ */
+int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name)
+{
+	int index;
+
+	if (!name)
+		return -EINVAL;
+
+	index = fwnode_property_match_string(fwnode, "interrupt-names",  name);
+	if (index < 0)
+		return index;
+
+	return fwnode_irq_get(fwnode, index);
+}
+EXPORT_SYMBOL(fwnode_irq_get_byname);
+
+/**
  * fwnode_graph_get_next_endpoint - Get next endpoint firmware node
  * @fwnode: Pointer to the parent firmware node
  * @prev: Previous endpoint node or %NULL to get the first
diff --git a/include/linux/property.h b/include/linux/property.h
index 7399a0b..95d56a5 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -121,6 +121,7 @@ struct fwnode_handle *fwnode_handle_get(struct fwnode_handle *fwnode);
 void fwnode_handle_put(struct fwnode_handle *fwnode);
 
 int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index);
+int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name);
 
 void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index);
 
-- 
2.7.4

Re: [PATCH v4 1/3] device property: Add fwnode_irq_get_byname

[Rafael J. Wysocki]

On Sat, Jan 22, 2022 at 12:33 PM Akhil R <akhilrajeev@nvidia.com> wrote:
>
> Add fwnode_irq_get_byname() to get an interrupt by name from either
> ACPI table or Device Tree, whichever is used for enumeration.
>
> In the ACPI case, this allow us to use 'interrupt-names' in
> _DSD which can be mapped to Interrupt() resource by index.
> The implementation is similar to 'interrupt-names' in the
> Device Tree.
>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  drivers/base/property.c  | 29 +++++++++++++++++++++++++++++
>  include/linux/property.h |  1 +
>  2 files changed, 30 insertions(+)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index e6497f6..fc59e0f 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -936,6 +936,35 @@ void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index)
>  EXPORT_SYMBOL(fwnode_iomap);
>
>  /**
> + * fwnode_irq_get_byname - Get IRQ from a fwnode using its name
> + * @fwnode:    Pointer to the firmware node
> + * @name:      IRQ name
> + *
> + * Description:
> + * Find a match to the string @name in the 'interrupt-names' string array
> + * in _DSD for ACPI, or of_node for Device Tree. Then get the Linux IRQ
> + * number of the IRQ resource corresponding to the index of the matched
> + * string.
> + *
> + * Return:
> + * Linux IRQ number on success, or negative errno otherwise.
> + */
> +int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name)
> +{
> +       int index;
> +
> +       if (!name)
> +               return -EINVAL;
> +
> +       index = fwnode_property_match_string(fwnode, "interrupt-names",  name);
> +       if (index < 0)
> +               return index;
> +
> +       return fwnode_irq_get(fwnode, index);
> +}
> +EXPORT_SYMBOL(fwnode_irq_get_byname);
> +
> +/**
>   * fwnode_graph_get_next_endpoint - Get next endpoint firmware node
>   * @fwnode: Pointer to the parent firmware node
>   * @prev: Previous endpoint node or %NULL to get the first
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 7399a0b..95d56a5 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -121,6 +121,7 @@ struct fwnode_handle *fwnode_handle_get(struct fwnode_handle *fwnode);
>  void fwnode_handle_put(struct fwnode_handle *fwnode);
>
>  int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index);
> +int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name);
>
>  void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index);
>
> --
> 2.7.4
>

[PATCH v4 2/3] docs: firmware-guide: ACPI: Add named interrupt doc

[Akhil R]

Add a detailed example of the named interrupts in the ACPI table.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 Documentation/firmware-guide/acpi/enumeration.rst | 39 +++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/Documentation/firmware-guide/acpi/enumeration.rst b/Documentation/firmware-guide/acpi/enumeration.rst
index 74b830b2..d002256 100644
--- a/Documentation/firmware-guide/acpi/enumeration.rst
+++ b/Documentation/firmware-guide/acpi/enumeration.rst
@@ -143,6 +143,45 @@ In robust cases the client unfortunately needs to call
 acpi_dma_request_slave_chan_by_index() directly and therefore choose the
 specific FixedDMA resource by its index.
 
+Named Interrupts
+================
+
+Drivers enumerated via ACPI can have names to interrupts in the ACPI table
+which can be used to get the IRQ number in the driver.
+
+The interrupt name can be listed in _DSD as 'interrupt-names'. The names
+should be listed as an array of strings which will map to the Interrupt()
+resource in the ACPI table corresponding to its index.
+
+The table below shows an example of its usage::
+
+    Device (DEV0) {
+        ...
+        Name (_CRS, ResourceTemplate() {
+            ...
+            Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
+                0x20,
+                0x24
+            }
+        })
+
+        Name (_DSD, Package () {
+            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+            Package () {
+                Package () {"interrupt-names",
+                Package (2) {"default", "alert"}},
+            }
+        ...
+        })
+    }
+
+The interrupt name 'default' will correspond to 0x20 in Interrupt()
+resource and 'alert' to 0x24. Note that only the Interrupt() resource
+is mapped and not GpioInt() or similar.
+
+The driver can call the function - fwnode_irq_get_byname() with the fwnode
+and interrupt name as arguments to get the corresponding IRQ number.
+
 SPI serial bus support
 ======================
 
-- 
2.7.4

Re: [PATCH v4 2/3] docs: firmware-guide: ACPI: Add named interrupt doc

[Rafael J. Wysocki]

On Sat, Jan 22, 2022 at 12:33 PM Akhil R <akhilrajeev@nvidia.com> wrote:
>
> Add a detailed example of the named interrupts in the ACPI table.
>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  Documentation/firmware-guide/acpi/enumeration.rst | 39 +++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/Documentation/firmware-guide/acpi/enumeration.rst b/Documentation/firmware-guide/acpi/enumeration.rst
> index 74b830b2..d002256 100644
> --- a/Documentation/firmware-guide/acpi/enumeration.rst
> +++ b/Documentation/firmware-guide/acpi/enumeration.rst
> @@ -143,6 +143,45 @@ In robust cases the client unfortunately needs to call
>  acpi_dma_request_slave_chan_by_index() directly and therefore choose the
>  specific FixedDMA resource by its index.
>
> +Named Interrupts
> +================
> +
> +Drivers enumerated via ACPI can have names to interrupts in the ACPI table
> +which can be used to get the IRQ number in the driver.
> +
> +The interrupt name can be listed in _DSD as 'interrupt-names'. The names
> +should be listed as an array of strings which will map to the Interrupt()
> +resource in the ACPI table corresponding to its index.
> +
> +The table below shows an example of its usage::
> +
> +    Device (DEV0) {
> +        ...
> +        Name (_CRS, ResourceTemplate() {
> +            ...
> +            Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
> +                0x20,
> +                0x24
> +            }
> +        })
> +
> +        Name (_DSD, Package () {
> +            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +            Package () {
> +                Package () {"interrupt-names",
> +                Package (2) {"default", "alert"}},
> +            }
> +        ...
> +        })
> +    }
> +
> +The interrupt name 'default' will correspond to 0x20 in Interrupt()
> +resource and 'alert' to 0x24. Note that only the Interrupt() resource
> +is mapped and not GpioInt() or similar.
> +
> +The driver can call the function - fwnode_irq_get_byname() with the fwnode
> +and interrupt name as arguments to get the corresponding IRQ number.
> +
>  SPI serial bus support
>  ======================
>
> --
> 2.7.4
>

[PATCH v4 3/3] i2c: smbus: Use device_*() functions instead of of_*()

[Akhil R]

Change of_*() functions to device_*() for firmware agnostic usage.
This allows to have the smbus_alert interrupt without any changes
in the controller drivers using the ACPI table.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/i2c/i2c-core-base.c  |  2 +-
 drivers/i2c/i2c-core-smbus.c | 10 +++++-----
 drivers/i2c/i2c-smbus.c      |  3 ++-
 include/linux/i2c-smbus.h    |  6 +++---
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 2c59dd7..32a4526 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1479,7 +1479,7 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 		goto out_list;
 	}
 
-	res = of_i2c_setup_smbus_alert(adap);
+	res = i2c_setup_smbus_alert(adap);
 	if (res)
 		goto out_reg;
 
diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
index e5b2d14..4c24c84 100644
--- a/drivers/i2c/i2c-core-smbus.c
+++ b/drivers/i2c/i2c-core-smbus.c
@@ -701,13 +701,13 @@ struct i2c_client *i2c_new_smbus_alert_device(struct i2c_adapter *adapter,
 }
 EXPORT_SYMBOL_GPL(i2c_new_smbus_alert_device);
 
-#if IS_ENABLED(CONFIG_I2C_SMBUS) && IS_ENABLED(CONFIG_OF)
-int of_i2c_setup_smbus_alert(struct i2c_adapter *adapter)
+#if IS_ENABLED(CONFIG_I2C_SMBUS)
+int i2c_setup_smbus_alert(struct i2c_adapter *adapter)
 {
 	int irq;
 
-	irq = of_property_match_string(adapter->dev.of_node, "interrupt-names",
-				       "smbus_alert");
+	irq = device_property_match_string(adapter->dev.parent, "interrupt-names",
+					   "smbus_alert");
 	if (irq == -EINVAL || irq == -ENODATA)
 		return 0;
 	else if (irq < 0)
@@ -715,5 +715,5 @@ int of_i2c_setup_smbus_alert(struct i2c_adapter *adapter)
 
 	return PTR_ERR_OR_ZERO(i2c_new_smbus_alert_device(adapter, NULL));
 }
-EXPORT_SYMBOL_GPL(of_i2c_setup_smbus_alert);
+EXPORT_SYMBOL_GPL(i2c_setup_smbus_alert);
 #endif
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index d3d06e3..026e2355 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -128,7 +128,8 @@ static int smbalert_probe(struct i2c_client *ara,
 	if (setup) {
 		irq = setup->irq;
 	} else {
-		irq = of_irq_get_byname(adapter->dev.of_node, "smbus_alert");
+		irq = fwnode_irq_get_byname(dev_fwnode(adapter->dev.parent),
+					    "smbus_alert");
 		if (irq <= 0)
 			return irq;
 	}
diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
index 1ef4218..95cf902 100644
--- a/include/linux/i2c-smbus.h
+++ b/include/linux/i2c-smbus.h
@@ -30,10 +30,10 @@ struct i2c_client *i2c_new_smbus_alert_device(struct i2c_adapter *adapter,
 					      struct i2c_smbus_alert_setup *setup);
 int i2c_handle_smbus_alert(struct i2c_client *ara);
 
-#if IS_ENABLED(CONFIG_I2C_SMBUS) && IS_ENABLED(CONFIG_OF)
-int of_i2c_setup_smbus_alert(struct i2c_adapter *adap);
+#if IS_ENABLED(CONFIG_I2C_SMBUS)
+int i2c_setup_smbus_alert(struct i2c_adapter *adap);
 #else
-static inline int of_i2c_setup_smbus_alert(struct i2c_adapter *adap)
+static inline int i2c_setup_smbus_alert(struct i2c_adapter *adap)
 {
 	return 0;
 }
-- 
2.7.4

Re: [PATCH v4 3/3] i2c: smbus: Use device_*() functions instead of of_*()

[Wolfram Sang]

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

Hi,

> +	irq = device_property_match_string(adapter->dev.parent, "interrupt-names",
> +					   "smbus_alert");

I think we should include "linux/property.h" for it. Interesting, the of
header was never included.

> -		irq = of_irq_get_byname(adapter->dev.of_node, "smbus_alert");
> +		irq = fwnode_irq_get_byname(dev_fwnode(adapter->dev.parent),
> +					    "smbus_alert");

Here we should replace of_irq.h with property.h.

Rest looks good!

Happy hacking,

   Wolfram


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

Re: [PATCH v4 0/3] Enable named interrupt smbus-alert for ACPI

[Andy Shevchenko]

On Sat, Jan 22, 2022 at 1:33 PM Akhil R <akhilrajeev@nvidia.com> wrote:
>
> I2C - SMBus core drivers use named interrupts to support smbus_alert.
> As named interrupts are not available for ACPI based systems, it was
> required to change the i2c bus controller driver if to use smbus alert.
> These patches provide option for named interrupts in ACPI and  make the
> implementation similar to DT. This will enable use of interrupt named
> 'smbus-alert' in ACPI as well which will be taken during i2c adapter
> register.

I'm fine with the series,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Maintainers can ask something else, though.

> v3->v4:
>   * Removed device_irq_get_byname()
>   * Updates in comments and document.
> v2->v3:
>   * Grammar/spelling corrections.
>   * Added description in function comments.
>   * Removed 'unlikely' from NULL check on 'name'
> v1->v2:
>   * Added firmware guide documentation for ACPI named interrupts
>   * Updates in function description comments
>
> Akhil R (3):
>   device property: Add fwnode_irq_get_byname
>   docs: firmware-guide: ACPI: Add named interrupt doc
>   i2c: smbus: Use device_*() functions instead of of_*()
>
>  Documentation/firmware-guide/acpi/enumeration.rst | 39 +++++++++++++++++++++++
>  drivers/base/property.c                           | 29 +++++++++++++++++
>  drivers/i2c/i2c-core-base.c                       |  2 +-
>  drivers/i2c/i2c-core-smbus.c                      | 10 +++---
>  drivers/i2c/i2c-smbus.c                           |  3 +-
>  include/linux/i2c-smbus.h                         |  6 ++--
>  include/linux/property.h                          |  1 +
>  7 files changed, 80 insertions(+), 10 deletions(-)
>
> --
> 2.7.4
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 0/3] Enable named interrupt smbus-alert for ACPI

[Rafael J. Wysocki]

On Sat, Jan 22, 2022 at 1:06 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Sat, Jan 22, 2022 at 1:33 PM Akhil R <akhilrajeev@nvidia.com> wrote:
> >
> > I2C - SMBus core drivers use named interrupts to support smbus_alert.
> > As named interrupts are not available for ACPI based systems, it was
> > required to change the i2c bus controller driver if to use smbus alert.
> > These patches provide option for named interrupts in ACPI and  make the
> > implementation similar to DT. This will enable use of interrupt named
> > 'smbus-alert' in ACPI as well which will be taken during i2c adapter
> > register.
>
> I'm fine with the series,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Maintainers can ask something else, though.

It looks good to me.

If no one else has concerns regarding it, I'll queue it up for 5.18.

> > v3->v4:
> >   * Removed device_irq_get_byname()
> >   * Updates in comments and document.
> > v2->v3:
> >   * Grammar/spelling corrections.
> >   * Added description in function comments.
> >   * Removed 'unlikely' from NULL check on 'name'
> > v1->v2:
> >   * Added firmware guide documentation for ACPI named interrupts
> >   * Updates in function description comments
> >
> > Akhil R (3):
> >   device property: Add fwnode_irq_get_byname
> >   docs: firmware-guide: ACPI: Add named interrupt doc
> >   i2c: smbus: Use device_*() functions instead of of_*()
> >
> >  Documentation/firmware-guide/acpi/enumeration.rst | 39 +++++++++++++++++++++++
> >  drivers/base/property.c                           | 29 +++++++++++++++++
> >  drivers/i2c/i2c-core-base.c                       |  2 +-
> >  drivers/i2c/i2c-core-smbus.c                      | 10 +++---
> >  drivers/i2c/i2c-smbus.c                           |  3 +-
> >  include/linux/i2c-smbus.h                         |  6 ++--
> >  include/linux/property.h                          |  1 +
> >  7 files changed, 80 insertions(+), 10 deletions(-)
> >
> > --
> > 2.7.4
> >
>
>
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v4 0/3] Enable named interrupt smbus-alert for ACPI

[Wolfram Sang]

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


> It looks good to me.
> 
> If no one else has concerns regarding it, I'll queue it up for 5.18.

I'd prefer this to go via I2C because it touches the I2C core. And SMBus
alert is I2C material anyway :)


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

Re: [PATCH v4 0/3] Enable named interrupt smbus-alert for ACPI

[Rafael J. Wysocki]

On Mon, Jan 24, 2022 at 10:04 PM Wolfram Sang <wsa@kernel.org> wrote:
>
>
> > It looks good to me.
> >
> > If no one else has concerns regarding it, I'll queue it up for 5.18.
>
> I'd prefer this to go via I2C because it touches the I2C core. And SMBus
> alert is I2C material anyway :)

OK, so I've just sent an ACK for the first patch and you can go ahead.

Thanks!