linux-kernel - Re: [PATCH] Revert "of/irq: make of_irq_find

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL_JsqLfFZJ6P6j3ZrjsNNYTqYxyPWMnRTh95NzR1vOm+XtAaA@mail.gmail.com>
Date:	Wed, 2 Dec 2015 11:10:28 -0600
From:	Rob Herring <robh+dt@...nel.org>
To:	Jonas Gorski <jogo@...nwrt.org>,
	Qais Yousef <qais.yousef@...tec.com>,
	Carlo Caione <carlo@...one.org>
Cc:	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Frank Rowand <frowand.list@...il.com>,
	Grant Likely <grant.likely@...aro.org>,
	David Daney <david.daney@...ium.com>
Subject: Re: [PATCH] Revert "of/irq: make of_irq_find_parent static"

+Carlo

On Wed, Dec 2, 2015 at 10:32 AM, Jonas Gorski <jogo@...nwrt.org> wrote:
> Hi,
>
> On Wed, Dec 2, 2015 at 5:14 PM, Qais Yousef <qais.yousef@...tec.com> wrote:
>> This reverts commit 52493d446141b07c8ba28dd6a529513f8b2342bd.
>>
>> Signed-off-by: Qais Yousef <qais.yousef@...tec.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/