| Message ID | 1642686255-25951-2-git-send-email-akhilrajeev@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | Enable named interrupt smbus-alert for ACPI | expand |
> Thanks, my comments below. Thanks for the inputs. > > > Add device_irq_get_byname() to get an interrupt by name from both the > > ACPI table and the Device Tree. > > This needs to be clarified (it's not and, but or), what about: > > Add device_irq_get_byname() to get an interrupt by name from either > ACPI table or Device Tree whichever has it. > > > This will allow to use 'interrupt-names' in _DSD which can be mapped > > to > > In the ACPI case this > allow us to > > > Interrupt() resource by index. The implementation is similar to > > 'interrupt-names' in the Device Tree. > > ... > > > /** > > + * 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 > > 'name' --> @name > > > + * in _DSD for ACPI, or of_node for device tree. Then get the Linux > > + IRQ > > Device Tree > > > + * number of the IRQ resource corresponding to the index of the > > + matched > > + * string. > > + * > > + * Return: > > > + * Linux IRQ number on success > > + * Negative errno otherwise. > > * 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); } > > ... > > > +/** > > + * device_irq_get_byname - Get IRQ of a device using interrupt name > > + * @dev: Device to get the interrupt > > + * @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 > > + * Negative errno otherwise. > > + */ > > As per above. > > ... > > > +int device_irq_get_byname(struct device *dev, const char *name); > > Since we don't have device_irq_get() perhaps we don't need this one right now > (just open code it in the caller). This will satisfy Rafael's request. If to code the same in caller, I guess, it would look like this - irq = fwnode_irq_get_byname(dev_fwnode(adapter->dev.parent), "smbus_alert"); Looks okay to me, but if given an option I would go with device_irq_get_byname(). Thanks, Akhil
On Fri, Jan 21, 2022 at 11:18 AM Akhil R <akhilrajeev@nvidia.com> wrote: ... > > > +int device_irq_get_byname(struct device *dev, const char *name); > > > > Since we don't have device_irq_get() perhaps we don't need this one right now > > (just open code it in the caller). This will satisfy Rafael's request. > > If to code the same in caller, I guess, it would look like this - > irq = fwnode_irq_get_byname(dev_fwnode(adapter->dev.parent), > "smbus_alert"); Yep, I meant how you point to it in the documentation, e.g. The user may call fwnode_irq_get_byname() with the firmware node and name of the IRQ it wants to retrieve. > Looks okay to me, but if given an option I would go with device_irq_get_byname(). You see, there was a query from Rafael and I haven't seen an answer yet. On top there is no such function for fwnode_irq_get() (I mean device_irq_get() API). It would be harder to push without good justification why one has the device_ counterpart and the other does not. Easiest way, as I see it, is to drop it for now.
> Thanks, my comments below. > > > Add device_irq_get_byname() to get an interrupt by name from both the > > ACPI table and the Device Tree. > > This needs to be clarified (it's not and, but or), what about: > > Add device_irq_get_byname() to get an interrupt by name from either > ACPI table or Device Tree whichever has it. Does 'whichever has it' mean a bit different here? Would it be good like this -? Add fwnode_irq_get_byname() to get an interrupt by name from either ACPI table or Device Tree, whichever is used for enumeration. Okay with the other comments. Thanks, Akhil
On Fri, Jan 21, 2022 at 2:29 PM Akhil R <akhilrajeev@nvidia.com> wrote: > > > Thanks, my comments below. > > > > > Add device_irq_get_byname() to get an interrupt by name from both the > > > ACPI table and the Device Tree. > > > > This needs to be clarified (it's not and, but or), what about: > > > > Add device_irq_get_byname() to get an interrupt by name from either > > ACPI table or Device Tree whichever has it. > Does 'whichever has it' mean a bit different here? Would it be good like this -? > > Add fwnode_irq_get_byname() to get an interrupt by name from either > ACPI table or Device Tree, whichever is used for enumeration. Yes, your variant is good.
diff --git a/drivers/base/property.c b/drivers/base/property.c index e6497f6..962a645 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -936,6 +936,57 @@ 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 + * 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); + +/** + * device_irq_get_byname - Get IRQ of a device using interrupt name + * @dev: Device to get the interrupt + * @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 + * Negative errno otherwise. + */ +int device_irq_get_byname(struct device *dev, const char *name) +{ + return fwnode_irq_get_byname(dev_fwnode(dev), name); +} +EXPORT_SYMBOL_GPL(device_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..3123b75 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -121,6 +121,9 @@ 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); + +int device_irq_get_byname(struct device *dev, const char *name); void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index);
Add device_irq_get_byname() to get an interrupt by name from both the ACPI table and the Device Tree. This will allow 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 | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/property.h | 3 +++ 2 files changed, 54 insertions(+)