| Message ID | 20211109200840.135019-2-puranjay12@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | device property: Adding fwnode_irq_get_byname() | expand |
On Wed, Nov 10, 2021 at 01:38:39AM +0530, Puranjay Mohan wrote: > The fwnode framework did not have means to obtain the IRQ number from > the name of a node. > Add that now, in form of the fwnode_irq_get_byname() function. ... > +int fwnode_irq_get_byname(struct fwnode_handle *fwnode, const char *name) > +{ > + int index; > + > + if (unlikely(!name)) > + return -EINVAL; > + index = fwnode_property_match_string(fwnode, "interrupt-names", name); > + if (index < 0) > + return index; It won't work like this. The ACPI table quite likely won't have this in them. Also it doesn't cover the GPIO interrupts in ACPI case. > + return fwnode_irq_get(fwnode, index); Neither this covers GPIO IRQs. > +} > +EXPORT_SYMBOL(fwnode_irq_get_byname); So, first you need to provide a design for this how ACPI cases can be handled. Imagine these cases (at least) for _CRS method in ACPI: 1/ Single GSI Interrupt() 2/ Single GPIO IRQ GpioInt() 3/ Both in different orders a) Interrupt() GpioInt() b) GpioInt() Interrupt() 4/ Mixed (complicated cases) Interrupt() Interrupt() GpioInt() Interrupt() GpioInt() Obvious question, what does the index mean in all these cases? Next one is, how can we quirk out the platforms with the old ACPI tables where no properties are provided? For GPIO there is struct acpi_gpio_params which goes deep into ACPI glue layer. Luckily, the GPIO IRQ case has already available APIs for indexing and naming match: acpi_dev_gpio_irq_get_by(). Hence, the main task is to define index in cases like 4 and see what can be done for the GSI cases.
On Wed, Nov 10, 2021 at 2:23 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Wed, Nov 10, 2021 at 01:38:39AM +0530, Puranjay Mohan wrote: > > The fwnode framework did not have means to obtain the IRQ number from > > the name of a node. > > Add that now, in form of the fwnode_irq_get_byname() function. > > ... > > > +int fwnode_irq_get_byname(struct fwnode_handle *fwnode, const char *name) > > +{ > > + int index; > > + > > + if (unlikely(!name)) > > + return -EINVAL; > > > + index = fwnode_property_match_string(fwnode, "interrupt-names", name); > > + if (index < 0) > > + return index; > > It won't work like this. The ACPI table quite likely won't have this in them. > Also it doesn't cover the GPIO interrupts in ACPI case. > > > + return fwnode_irq_get(fwnode, index); > > Neither this covers GPIO IRQs. > > > +} > > +EXPORT_SYMBOL(fwnode_irq_get_byname); > > So, first you need to provide a design for this how ACPI cases can be handled. > > Imagine these cases (at least) for _CRS method in ACPI: > 1/ Single GSI > > Interrupt() > > 2/ Single GPIO IRQ > > GpioInt() > > 3/ Both in different orders > a) > Interrupt() > GpioInt() > > b) > GpioInt() > Interrupt() > > 4/ Mixed (complicated cases) > > Interrupt() > Interrupt() > GpioInt() > Interrupt() > GpioInt() > > Obvious question, what does the index mean in all these cases? > > Next one is, how can we quirk out the platforms with the old ACPI tables > where no properties are provided? For GPIO there is struct acpi_gpio_params > which goes deep into ACPI glue layer. > > Luckily, the GPIO IRQ case has already available APIs for indexing and naming > match: acpi_dev_gpio_irq_get_by(). > > Hence, the main task is to define index in cases like 4 and see what can be > done for the GSI cases. > > -- > With Best Regards, > Andy Shevchenko > > Hi Andy, I wrote this function keeping the device tree in mind. I will have to look into ACPI and see how the cases you mentioned can be implemented. Let's see how far I can get with understanding the ACPI.
On Wed, Nov 10, 2021 at 10:34:43PM +0530, Puranjay Mohan wrote: > On Wed, Nov 10, 2021 at 2:23 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Wed, Nov 10, 2021 at 01:38:39AM +0530, Puranjay Mohan wrote: > I wrote this function keeping the device tree in mind. I will have to > look into ACPI and see how the cases you mentioned can be implemented. > Let's see how far I can get with understanding the ACPI. Yeah. What you need to have is 1) expand fwnode_irq_get() to support ACPI GPIO IRQ resources. 2) provide a simple version of the fwnode_irq_get_by_name() like if (is_of_node()) return of_...(); return acpi_dev_gpio_irq_get_by(); 3) establish understanding about naming for ACPI and 4) extend fwnode_irq_get_by_name() by it if (is_of_node()) return of_...(); ret = acpi_irq_get_by_name(); if (ret > 0) return ret; return acpi_dev_gpio_irq_get_by(); As I mentioned, items 1 and 2 are easy to achieve with currently existing APIs.
diff --git a/drivers/base/property.c b/drivers/base/property.c index f1f35b48ab8b..0d685c79b0e8 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -958,6 +958,29 @@ int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index) } EXPORT_SYMBOL(fwnode_irq_get); +/** + * fwnode_irq_get_byname - Get IRQ directly from its name. + * @fwnode: Pointer to the firmware node + * @name: IRQ Name + * + * Returns Linux IRQ number on success. Other values are determined + * accordingly to acpi_/of_ irq_get() operation. + */ +int fwnode_irq_get_byname(struct fwnode_handle *fwnode, const char *name) +{ + int index; + + if (unlikely(!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 diff --git a/include/linux/property.h b/include/linux/property.h index 88fa726a76df..9c6177597ba8 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -122,6 +122,8 @@ 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(struct fwnode_handle *fwnode, const char *name); + unsigned int device_get_child_node_count(struct device *dev); static inline bool device_property_read_bool(struct device *dev,
The fwnode framework did not have means to obtain the IRQ number from the name of a node. Add that now, in form of the fwnode_irq_get_byname() function. Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> --- drivers/base/property.c | 23 +++++++++++++++++++++++ include/linux/property.h | 2 ++ 2 files changed, 25 insertions(+)