[PATCH] Revert "of/irq: make of_irq_find_parent static"

[PATCH] Revert "of/irq: make of_irq_find_parent static"

[Qais Yousef]

This reverts commit 52493d446141b07c8ba28dd6a529513f8b2342bd.

Signed-off-by: Qais Yousef <qais.yousef-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>

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?

Thanks,
Qais


 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);
 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;
+}
+
 static inline u32 of_msi_map_rid(struct device *dev,
 				 struct device_node *msi_np, u32 rid_in)
 {
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Revert "of/irq: make of_irq_find_parent static"

[Jonas Gorski]

Hi,

On Wed, Dec 2, 2015 at 5:14 PM, Qais Yousef <qais.yousef@imgtec.com> wrote:
> This reverts commit 52493d446141b07c8ba28dd6a529513f8b2342bd.
>
> Signed-off-by: Qais Yousef <qais.yousef@imgtec.com>
>
> 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.

also ...

>
> Thanks,
> Qais
>
>
>  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.


Regards
Jonas

Re: [PATCH] Revert "of/irq: make of_irq_find_parent static"

[Rob Herring]

+Carlo

On Wed, Dec 2, 2015 at 10:32 AM, Jonas Gorski <jogo@openwrt.org> wrote:
> Hi,
>
> On Wed, Dec 2, 2015 at 5:14 PM, Qais Yousef <qais.yousef@imgtec.com> wrote:
>> This reverts commit 52493d446141b07c8ba28dd6a529513f8b2342bd.
>>
>> Signed-off-by: Qais Yousef <qais.yousef@imgtec.com>
>>
>> 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

Re: [PATCH] Revert "of/irq: make of_irq_find_parent static"

[Qais Yousef]

On 12/02/2015 05:10 PM, Rob Herring wrote:
> +Carlo
>
> On Wed, Dec 2, 2015 at 10:32 AM, Jonas Gorski <jogo@openwrt.org> wrote:
>> 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.

I don't need the EXPORT_SYMBOL() in my case since I'm using it from ARCH 
code.

> [1] http://www.spinics.net/lists/arm-kernel/msg464847.html

Looks like it's already sorted then. I'll pull in this patch and test it.

Thanks,
Qais