+Carlo On Wed, Dec 2, 2015 at 10:32 AM, Jonas Gorski <jogo@xxxxxxxxxxx> wrote: > Hi, > > On Wed, Dec 2, 2015 at 5:14 PM, Qais Yousef <qais.yousef@xxxxxxxxxx> wrote: >> This reverts commit 52493d446141b07c8ba28dd6a529513f8b2342bd. >> >> Signed-off-by: Qais Yousef <qais.yousef@xxxxxxxxxx> >> >> Conflicts: >> include/linux/of_irq.h >> --- >> I have a patch series that is under review that makes use of of_irq_find_parent() >> >> The affected patch is this: >> >> https://lkml.org/lkml/2015/11/25/291 >> >> Is it wrong to use this function? If yes, what's the alternative? >> If no, OK to revert? > > Make the revert part of the series that needs it, so it won't get > moved again because of no external users. There's 2 cases[1] wanting it, so I'm planning to apply for 4.4 since we removed it in 4.4. Reverting is perhaps the wrong thing as it conflicts, also needs an EXPORT_SYMBOL, and needs to be moved for !OF_IRQ builds as pointed out below. >> drivers/of/irq.c | 2 +- >> include/linux/of_irq.h | 6 ++++++ >> 2 files changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/of/irq.c b/drivers/of/irq.c >> index 902b89be7217..45735d56e435 100644 >> --- a/drivers/of/irq.c >> +++ b/drivers/of/irq.c >> @@ -53,7 +53,7 @@ EXPORT_SYMBOL_GPL(irq_of_parse_and_map); >> * Returns a pointer to the interrupt parent node, or NULL if the interrupt >> * parent could not be determined. >> */ >> -static struct device_node *of_irq_find_parent(struct device_node *child) >> +struct device_node *of_irq_find_parent(struct device_node *child) >> { >> struct device_node *p; >> const __be32 *parp; >> diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h >> index 039f2eec49ce..0c9ea9fb5b63 100644 >> --- a/include/linux/of_irq.h >> +++ b/include/linux/of_irq.h >> @@ -93,6 +93,7 @@ static inline void of_msi_configure(struct device *dev, struct device_node *np) >> * so declare it here regardless of the CONFIG_OF_IRQ setting. >> */ >> extern unsigned int irq_of_parse_and_map(struct device_node *node, int index); >> +extern struct device_node *of_irq_find_parent(struct device_node *child); > > This is the wrong place to add the prototype, and > >> u32 of_msi_map_rid(struct device *dev, struct device_node *msi_np, u32 rid_in); >> >> #else /* !CONFIG_OF && !CONFIG_SPARC */ >> @@ -102,6 +103,11 @@ static inline unsigned int irq_of_parse_and_map(struct device_node *dev, >> return 0; >> } >> >> +static inline void *of_irq_find_parent(struct device_node *child) >> +{ >> + return NULL; >> +} >> + > > This is the wrong place to add the inline version. Both need to be > within the #ifdef CONFIG_OF_IRQ ... #else ... #endif block, as the > function is not available just with CONFIG_OF. > > >> static inline u32 of_msi_map_rid(struct device *dev, >> struct device_node *msi_np, u32 rid_in) > > @Daney: > > And this one is at the wrong place as well. Please fix it. > > This will break the build if CONFIG_OF is enabled but CONFIG_OF_IRQ > isn't, and something tries to call these functions. [1] http://www.spinics.net/lists/arm-kernel/msg464847.html -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
