This reverts commit 52493d446141b07c8ba28dd6a529513f8b2342bd.
Signed-off-by: Qais Yousef <[email protected]>
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
Hi,
On Wed, Dec 2, 2015 at 5:14 PM, Qais Yousef <[email protected]> wrote:
> This reverts commit 52493d446141b07c8ba28dd6a529513f8b2342bd.
>
> Signed-off-by: Qais Yousef <[email protected]>
>
> 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
+Carlo
On Wed, Dec 2, 2015 at 10:32 AM, Jonas Gorski <[email protected]> wrote:
> Hi,
>
> On Wed, Dec 2, 2015 at 5:14 PM, Qais Yousef <[email protected]> wrote:
>> This reverts commit 52493d446141b07c8ba28dd6a529513f8b2342bd.
>>
>> Signed-off-by: Qais Yousef <[email protected]>
>>
>> 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
On 12/02/2015 05:10 PM, Rob Herring wrote:
> +Carlo
>
> On Wed, Dec 2, 2015 at 10:32 AM, Jonas Gorski <[email protected]> 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