[PATCH 1/2] clk: bcm2835: Add AUX interrupt controller

[PATCH 1/2] clk: bcm2835: Add AUX interrupt controller

[Phil Elwell]

Devices in the AUX block share a common interrupt line, with a register
indicating which devices have active IRQs. Expose this as a nested
interrupt controller to avoid IRQ sharing problems (easily observed if
UART1 and SPI1/2 are enabled simultaneously).

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
---
 drivers/clk/bcm/clk-bcm2835-aux.c | 120 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/drivers/clk/bcm/clk-bcm2835-aux.c b/drivers/clk/bcm/clk-bcm2835-aux.c
index bd750cf..41e0702 100644
--- a/drivers/clk/bcm/clk-bcm2835-aux.c
+++ b/drivers/clk/bcm/clk-bcm2835-aux.c
@@ -17,17 +17,107 @@
 #include <linux/clk/bcm2835.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/irqdomain.h>
+#include <linux/of_irq.h>
 #include <dt-bindings/clock/bcm2835-aux.h>
 
 #define BCM2835_AUXIRQ		0x00
 #define BCM2835_AUXENB		0x04
 
+#define BCM2835_AUXIRQ_NUM_IRQS  3
+
+#define BCM2835_AUXIRQ_UART_IRQ  0
+#define BCM2835_AUXIRQ_SPI1_IRQ  1
+#define BCM2835_AUXIRQ_SPI2_IRQ  2
+
+#define BCM2835_AUXIRQ_UART_MASK 0x01
+#define BCM2835_AUXIRQ_SPI1_MASK 0x02
+#define BCM2835_AUXIRQ_SPI2_MASK 0x04
+
+#define BCM2835_AUXIRQ_ALL_MASK \
+	(BCM2835_AUXIRQ_UART_MASK | \
+	 BCM2835_AUXIRQ_SPI1_MASK | \
+	 BCM2835_AUXIRQ_SPI2_MASK)
+
+struct auxirq_state {
+	void __iomem      *status;
+	u32                enables;
+	struct irq_domain *domain;
+	struct regmap     *local_regmap;
+};
+
+static struct auxirq_state auxirq __read_mostly;
+
+static irqreturn_t bcm2835_auxirq_handler(int irq, void *dev_id)
+{
+	u32 stat = readl_relaxed(auxirq.status);
+	u32 masked = stat & auxirq.enables;
+
+	if (masked & BCM2835_AUXIRQ_UART_MASK)
+		generic_handle_irq(irq_linear_revmap(auxirq.domain,
+						     BCM2835_AUXIRQ_UART_IRQ));
+
+	if (masked & BCM2835_AUXIRQ_SPI1_MASK)
+		generic_handle_irq(irq_linear_revmap(auxirq.domain,
+						     BCM2835_AUXIRQ_SPI1_IRQ));
+
+	if (masked & BCM2835_AUXIRQ_SPI2_MASK)
+		generic_handle_irq(irq_linear_revmap(auxirq.domain,
+						     BCM2835_AUXIRQ_SPI2_IRQ));
+
+	return (masked & BCM2835_AUXIRQ_ALL_MASK) ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static int bcm2835_auxirq_xlate(struct irq_domain *d,
+				 struct device_node *ctrlr,
+				 const u32 *intspec, unsigned int intsize,
+				 unsigned long *out_hwirq,
+				 unsigned int *out_type)
+{
+	if (WARN_ON(intsize != 1))
+		return -EINVAL;
+
+	if (WARN_ON(intspec[0] >= BCM2835_AUXIRQ_NUM_IRQS))
+		return -EINVAL;
+
+	*out_hwirq = intspec[0];
+	*out_type = IRQ_TYPE_NONE;
+	return 0;
+}
+
+static void bcm2835_auxirq_mask(struct irq_data *data)
+{
+	irq_hw_number_t hwirq = irqd_to_hwirq(data);
+
+	auxirq.enables &= ~(1 << hwirq);
+}
+
+static void bcm2835_auxirq_unmask(struct irq_data *data)
+{
+	irq_hw_number_t hwirq = irqd_to_hwirq(data);
+
+	auxirq.enables |= (1 << hwirq);
+}
+
+static struct irq_chip bcm2835_auxirq_chip = {
+	.name = "bcm2835-auxirq",
+	.irq_mask = bcm2835_auxirq_mask,
+	.irq_unmask = bcm2835_auxirq_unmask,
+};
+
+static const struct irq_domain_ops bcm2835_auxirq_ops = {
+	.xlate = bcm2835_auxirq_xlate//irq_domain_xlate_onecell
+};
+
 static int bcm2835_aux_clk_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
 	struct clk_hw_onecell_data *onecell;
 	const char *parent;
 	struct clk *parent_clk;
+	int parent_irq;
 	struct resource *res;
 	void __iomem *reg, *gate;
 
@@ -41,6 +131,36 @@ static int bcm2835_aux_clk_probe(struct platform_device *pdev)
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
+	parent_irq = irq_of_parse_and_map(node, 0);
+	if (parent_irq) {
+		int ret;
+		int i;
+
+		/* Manage the AUX irq as well */
+		auxirq.status = reg + BCM2835_AUXIRQ;
+		auxirq.domain = irq_domain_add_linear(node,
+						      BCM2835_AUXIRQ_NUM_IRQS,
+						      &bcm2835_auxirq_ops,
+						      NULL);
+		if (!auxirq.domain)
+			return -ENXIO;
+
+		for (i = 0; i < BCM2835_AUXIRQ_NUM_IRQS; i++) {
+			unsigned int irq = irq_create_mapping(auxirq.domain, i);
+
+			if (irq == 0)
+				return -ENXIO;
+
+			irq_set_chip_and_handler(irq, &bcm2835_auxirq_chip,
+						 handle_level_irq);
+		}
+
+		ret = devm_request_irq(dev, parent_irq, bcm2835_auxirq_handler,
+				       0, "bcm2835-auxirq", NULL);
+		if (ret)
+			return ret;
+	}
+
 	onecell = devm_kmalloc(dev, sizeof(*onecell) + sizeof(*onecell->hws) *
 			       BCM2835_AUX_CLOCK_COUNT, GFP_KERNEL);
 	if (!onecell)
-- 
1.9.1

Re: [PATCH 1/2] clk: bcm2835: Add AUX interrupt controller

[Alexander Stein]

On Wednesday 07 June 2017 12:11:45, Phil Elwell wrote:
> Devices in the AUX block share a common interrupt line, with a register
> indicating which devices have active IRQs. Expose this as a nested
> interrupt controller to avoid IRQ sharing problems (easily observed if
> UART1 and SPI1/2 are enabled simultaneously).
> 
> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
> ---
>  drivers/clk/bcm/clk-bcm2835-aux.c | 120
> ++++++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+)
> 
> diff --git a/drivers/clk/bcm/clk-bcm2835-aux.c
> b/drivers/clk/bcm/clk-bcm2835-aux.c index bd750cf..41e0702 100644
> --- a/drivers/clk/bcm/clk-bcm2835-aux.c
> +++ b/drivers/clk/bcm/clk-bcm2835-aux.c
> [...]
> +struct auxirq_state {
> +	void __iomem      *status;
> +	u32                enables;
> +	struct irq_domain *domain;
> +	struct regmap     *local_regmap;
> +};
> +
> +static struct auxirq_state auxirq __read_mostly;
> +
> +static irqreturn_t bcm2835_auxirq_handler(int irq, void *dev_id)
> +{
> +	u32 stat = readl_relaxed(auxirq.status);
> +	u32 masked = stat & auxirq.enables;

Doesn't this hide any spurious interrupts? Is this acceptable? I mean getting 
informed about spurious interrupts seems nice to me, as it indicates a 
hardware/configuration problem.

> +	if (masked & BCM2835_AUXIRQ_UART_MASK)
> +		generic_handle_irq(irq_linear_revmap(auxirq.domain,
> +						     BCM2835_AUXIRQ_UART_IRQ));
> +
> +	if (masked & BCM2835_AUXIRQ_SPI1_MASK)
> +		generic_handle_irq(irq_linear_revmap(auxirq.domain,
> +						     BCM2835_AUXIRQ_SPI1_IRQ));
> +
> +	if (masked & BCM2835_AUXIRQ_SPI2_MASK)
> +		generic_handle_irq(irq_linear_revmap(auxirq.domain,
> +						     BCM2835_AUXIRQ_SPI2_IRQ));
> +
> +	return (masked & BCM2835_AUXIRQ_ALL_MASK) ? IRQ_HANDLED : IRQ_NONE;
> +}

How does interrupt acknowledgement work in these 3 interrupts work?

Best regards,
Alexander

Re: [PATCH 1/2] clk: bcm2835: Add AUX interrupt controller

[Phil Elwell]

On 07/06/2017 13:07, Alexander Stein wrote:
> On Wednesday 07 June 2017 12:11:45, Phil Elwell wrote:
>> Devices in the AUX block share a common interrupt line, with a register
>> indicating which devices have active IRQs. Expose this as a nested
>> interrupt controller to avoid IRQ sharing problems (easily observed if
>> UART1 and SPI1/2 are enabled simultaneously).
>>
>> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
>> ---
>>  drivers/clk/bcm/clk-bcm2835-aux.c | 120
>> ++++++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+)
>>
>> diff --git a/drivers/clk/bcm/clk-bcm2835-aux.c
>> b/drivers/clk/bcm/clk-bcm2835-aux.c index bd750cf..41e0702 100644
>> --- a/drivers/clk/bcm/clk-bcm2835-aux.c
>> +++ b/drivers/clk/bcm/clk-bcm2835-aux.c
>> [...]
>> +struct auxirq_state {
>> +	void __iomem      *status;
>> +	u32                enables;
>> +	struct irq_domain *domain;
>> +	struct regmap     *local_regmap;
>> +};
>> +
>> +static struct auxirq_state auxirq __read_mostly;
>> +
>> +static irqreturn_t bcm2835_auxirq_handler(int irq, void *dev_id)
>> +{
>> +	u32 stat = readl_relaxed(auxirq.status);
>> +	u32 masked = stat & auxirq.enables;
> 
> Doesn't this hide any spurious interrupts? Is this acceptable? I mean getting 
> informed about spurious interrupts seems nice to me, as it indicates a 
> hardware/configuration problem.

Thanks for the reply. This interrupt handler is capable of dispatching multiple
interrupts but must return a single value - IRQ_HANDLED or IRQ_NONE. I've assumed
that returning IRQ_NONE repeatedly will trigger the spurious interrupt detection.

This implementation returns IRQ_HANDLED if any unmasked interrupts are raised,
otherwise it returns IRQ_NONE. Therefore provided any spurious interrupt isn't
always coincident with a real interrupt then it ought eventually to be identified
as spurious. The alternative - returning IRQ_NONE if there are any spurious
interrupts - seems prone to causing collateral damage.

What did you have in mind?

>> +	if (masked & BCM2835_AUXIRQ_UART_MASK)
>> +		generic_handle_irq(irq_linear_revmap(auxirq.domain,
>> +						     BCM2835_AUXIRQ_UART_IRQ));
>> +
>> +	if (masked & BCM2835_AUXIRQ_SPI1_MASK)
>> +		generic_handle_irq(irq_linear_revmap(auxirq.domain,
>> +						     BCM2835_AUXIRQ_SPI1_IRQ));
>> +
>> +	if (masked & BCM2835_AUXIRQ_SPI2_MASK)
>> +		generic_handle_irq(irq_linear_revmap(auxirq.domain,
>> +						     BCM2835_AUXIRQ_SPI2_IRQ));
>> +
>> +	return (masked & BCM2835_AUXIRQ_ALL_MASK) ? IRQ_HANDLED : IRQ_NONE;
>> +}
> 
> How does interrupt acknowledgement work in these 3 interrupts work?

The interrupt "controller" is just combinatorial logic on the three level-sensitive
interrupt lines from the devices. Interrupts must be acknowledged and cleared at
source.

Phil

Re: [PATCH 1/2] clk: bcm2835: Add AUX interrupt controller

[Alexander Stein]

 On Wednesday, June 7, 2017, 2:37:41 PM CEST Phil Elwell wrote:
> On 07/06/2017 13:07, Alexander Stein wrote:
> > On Wednesday 07 June 2017 12:11:45, Phil Elwell wrote:
> >> Devices in the AUX block share a common interrupt line, with a register
> >> indicating which devices have active IRQs. Expose this as a nested
> >> interrupt controller to avoid IRQ sharing problems (easily observed if
> >> UART1 and SPI1/2 are enabled simultaneously).
> >> 
> >> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
> >> ---
> >> 
> >>  drivers/clk/bcm/clk-bcm2835-aux.c | 120
> >> 
> >> ++++++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+)
> >> 
> >> diff --git a/drivers/clk/bcm/clk-bcm2835-aux.c
> >> b/drivers/clk/bcm/clk-bcm2835-aux.c index bd750cf..41e0702 100644
> >> --- a/drivers/clk/bcm/clk-bcm2835-aux.c
> >> +++ b/drivers/clk/bcm/clk-bcm2835-aux.c
> >> [...]
> >> +struct auxirq_state {
> >> +	void __iomem      *status;
> >> +	u32                enables;
> >> +	struct irq_domain *domain;
> >> +	struct regmap     *local_regmap;
> >> +};
> >> +
> >> +static struct auxirq_state auxirq __read_mostly;
> >> +
> >> +static irqreturn_t bcm2835_auxirq_handler(int irq, void *dev_id)
> >> +{
> >> +	u32 stat = readl_relaxed(auxirq.status);
> >> +	u32 masked = stat & auxirq.enables;
> > 
> > Doesn't this hide any spurious interrupts? Is this acceptable? I mean
> > getting informed about spurious interrupts seems nice to me, as it
> > indicates a hardware/configuration problem.
> 
> Thanks for the reply. This interrupt handler is capable of dispatching
> multiple interrupts but must return a single value - IRQ_HANDLED or
> IRQ_NONE. I've assumed that returning IRQ_NONE repeatedly will trigger the
> spurious interrupt detection.
> 
> This implementation returns IRQ_HANDLED if any unmasked interrupts are
> raised, otherwise it returns IRQ_NONE. Therefore provided any spurious
> interrupt isn't always coincident with a real interrupt then it ought
> eventually to be identified as spurious. The alternative - returning
> IRQ_NONE if there are any spurious interrupts - seems prone to causing
> collateral damage.
> 
> What did you have in mind?

I was wondering about "stat & auxirq.enables". With that you wouldn't forward 
any spurious interrupts to e.g. SPI1. I don't know which way is better, 
returning IRQ_NONE is a masked interrupt happens, or pass it further down. I 
guess this also raises a warning if the SPI driver also returns IRQ_NONE.
BTW: Is it even allowed to call generic_handle_irq on a masked irq?

> >> +	if (masked & BCM2835_AUXIRQ_UART_MASK)
> >> +		generic_handle_irq(irq_linear_revmap(auxirq.domain,
> >> +						     BCM2835_AUXIRQ_UART_IRQ));
> >> +
> >> +	if (masked & BCM2835_AUXIRQ_SPI1_MASK)
> >> +		generic_handle_irq(irq_linear_revmap(auxirq.domain,
> >> +						     BCM2835_AUXIRQ_SPI1_IRQ));
> >> +
> >> +	if (masked & BCM2835_AUXIRQ_SPI2_MASK)
> >> +		generic_handle_irq(irq_linear_revmap(auxirq.domain,
> >> +						     BCM2835_AUXIRQ_SPI2_IRQ));
> >> +
> >> +	return (masked & BCM2835_AUXIRQ_ALL_MASK) ? IRQ_HANDLED : IRQ_NONE;
> >> +}
> > 
> > How does interrupt acknowledgement work in these 3 interrupts work?
> 
> The interrupt "controller" is just combinatorial logic on the three
> level-sensitive interrupt lines from the devices. Interrupts must be
> acknowledged and cleared at source.

Thanks for the info.

Best regards,
Alexander

Re: [PATCH 1/2] clk: bcm2835: Add AUX interrupt controller

[Phil Elwell]

On 07/06/2017 18:25, Alexander Stein wrote:
>  On Wednesday, June 7, 2017, 2:37:41 PM CEST Phil Elwell wrote:
>> On 07/06/2017 13:07, Alexander Stein wrote:
>>> On Wednesday 07 June 2017 12:11:45, Phil Elwell wrote:
>>>> Devices in the AUX block share a common interrupt line, with a register
>>>> indicating which devices have active IRQs. Expose this as a nested
>>>> interrupt controller to avoid IRQ sharing problems (easily observed if
>>>> UART1 and SPI1/2 are enabled simultaneously).
>>>>
>>>> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
>>>> ---
>>>>
>>>>  drivers/clk/bcm/clk-bcm2835-aux.c | 120
>>>>
>>>> ++++++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+)
>>>>
>>>> diff --git a/drivers/clk/bcm/clk-bcm2835-aux.c
>>>> b/drivers/clk/bcm/clk-bcm2835-aux.c index bd750cf..41e0702 100644
>>>> --- a/drivers/clk/bcm/clk-bcm2835-aux.c
>>>> +++ b/drivers/clk/bcm/clk-bcm2835-aux.c
>>>> [...]
>>>> +struct auxirq_state {
>>>> +	void __iomem      *status;
>>>> +	u32                enables;
>>>> +	struct irq_domain *domain;
>>>> +	struct regmap     *local_regmap;
>>>> +};
>>>> +
>>>> +static struct auxirq_state auxirq __read_mostly;
>>>> +
>>>> +static irqreturn_t bcm2835_auxirq_handler(int irq, void *dev_id)
>>>> +{
>>>> +	u32 stat = readl_relaxed(auxirq.status);
>>>> +	u32 masked = stat & auxirq.enables;
>>>
>>> Doesn't this hide any spurious interrupts? Is this acceptable? I mean
>>> getting informed about spurious interrupts seems nice to me, as it
>>> indicates a hardware/configuration problem.
>>
>> Thanks for the reply. This interrupt handler is capable of dispatching
>> multiple interrupts but must return a single value - IRQ_HANDLED or
>> IRQ_NONE. I've assumed that returning IRQ_NONE repeatedly will trigger the
>> spurious interrupt detection.
>>
>> This implementation returns IRQ_HANDLED if any unmasked interrupts are
>> raised, otherwise it returns IRQ_NONE. Therefore provided any spurious
>> interrupt isn't always coincident with a real interrupt then it ought
>> eventually to be identified as spurious. The alternative - returning
>> IRQ_NONE if there are any spurious interrupts - seems prone to causing
>> collateral damage.
>>
>> What did you have in mind?
>
> I was wondering about "stat & auxirq.enables". With that you wouldn't forward
> any spurious interrupts to e.g. SPI1. I don't know which way is better,
> returning IRQ_NONE is a masked interrupt happens, or pass it further down. I
> guess this also raises a warning if the SPI driver also returns IRQ_NONE.

That makes sense. Although my instinct was to ignore the spurious interrupt
as early as possible, it is better to let Linux handle in it. Without that
use of the enables field there is no need for the mask and unmask methods,
so the driver becomes even simpler.

I'll incorporate your suggestions into v2.

> BTW: Is it even allowed to call generic_handle_irq on a masked irq?

Ah, I think that's just a naming confusion - the variable contains bits for
interrupts that are active and enabled, i.e. masked in, not masked out.

Phil

Re: [PATCH 1/2] clk: bcm2835: Add AUX interrupt controller

[Stefan Wahren]

> Phil Elwell <phil@raspberrypi.org> hat am 7. Juni 2017 um 13:11 geschrieben:
> 
> 
> Devices in the AUX block share a common interrupt line, with a register
> indicating which devices have active IRQs. Expose this as a nested
> interrupt controller to avoid IRQ sharing problems (easily observed if
> UART1 and SPI1/2 are enabled simultaneously).
> 
> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
> ---
>  drivers/clk/bcm/clk-bcm2835-aux.c | 120 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 120 insertions(+)
> 
> diff --git a/drivers/clk/bcm/clk-bcm2835-aux.c b/drivers/clk/bcm/clk-bcm2835-aux.c
> index bd750cf..41e0702 100644
> --- a/drivers/clk/bcm/clk-bcm2835-aux.c
> +++ b/drivers/clk/bcm/clk-bcm2835-aux.c
> @@ -17,17 +17,107 @@
>  #include <linux/clk/bcm2835.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqdomain.h>
> +#include <linux/of_irq.h>

Please try to keep alphabetical order.

>  #include <dt-bindings/clock/bcm2835-aux.h>
>  
>  #define BCM2835_AUXIRQ		0x00
>  #define BCM2835_AUXENB		0x04
>  
> +#define BCM2835_AUXIRQ_NUM_IRQS  3
> +
> +#define BCM2835_AUXIRQ_UART_IRQ  0
> +#define BCM2835_AUXIRQ_SPI1_IRQ  1
> +#define BCM2835_AUXIRQ_SPI2_IRQ  2
> +
> +#define BCM2835_AUXIRQ_UART_MASK 0x01
> +#define BCM2835_AUXIRQ_SPI1_MASK 0x02
> +#define BCM2835_AUXIRQ_SPI2_MASK 0x04

I think these are candidates for the BIT macro.

> +
> +#define BCM2835_AUXIRQ_ALL_MASK \
> +	(BCM2835_AUXIRQ_UART_MASK | \
> +	 BCM2835_AUXIRQ_SPI1_MASK | \
> +	 BCM2835_AUXIRQ_SPI2_MASK)
> +
> +struct auxirq_state {
> +	void __iomem      *status;
> +	u32                enables;
> +	struct irq_domain *domain;
> +	struct regmap     *local_regmap;

This member isn't used, can it be dropped?

> +};
> +
> +static struct auxirq_state auxirq __read_mostly;
> +
> +static irqreturn_t bcm2835_auxirq_handler(int irq, void *dev_id)
> +{
> +	u32 stat = readl_relaxed(auxirq.status);
> +	u32 masked = stat & auxirq.enables;
> +
> +	if (masked & BCM2835_AUXIRQ_UART_MASK)
> +		generic_handle_irq(irq_linear_revmap(auxirq.domain,
> +						     BCM2835_AUXIRQ_UART_IRQ));
> +
> +	if (masked & BCM2835_AUXIRQ_SPI1_MASK)
> +		generic_handle_irq(irq_linear_revmap(auxirq.domain,
> +						     BCM2835_AUXIRQ_SPI1_IRQ));
> +
> +	if (masked & BCM2835_AUXIRQ_SPI2_MASK)
> +		generic_handle_irq(irq_linear_revmap(auxirq.domain,
> +						     BCM2835_AUXIRQ_SPI2_IRQ));
> +
> +	return (masked & BCM2835_AUXIRQ_ALL_MASK) ? IRQ_HANDLED : IRQ_NONE;
> +}
> +
> +static int bcm2835_auxirq_xlate(struct irq_domain *d,
> +				 struct device_node *ctrlr,
> +				 const u32 *intspec, unsigned int intsize,
> +				 unsigned long *out_hwirq,
> +				 unsigned int *out_type)
> +{
> +	if (WARN_ON(intsize != 1))
> +		return -EINVAL;
> +
> +	if (WARN_ON(intspec[0] >= BCM2835_AUXIRQ_NUM_IRQS))
> +		return -EINVAL;
> +
> +	*out_hwirq = intspec[0];
> +	*out_type = IRQ_TYPE_NONE;
> +	return 0;
> +}
> +
> +static void bcm2835_auxirq_mask(struct irq_data *data)
> +{
> +	irq_hw_number_t hwirq = irqd_to_hwirq(data);
> +
> +	auxirq.enables &= ~(1 << hwirq);
> +}
> +
> +static void bcm2835_auxirq_unmask(struct irq_data *data)
> +{
> +	irq_hw_number_t hwirq = irqd_to_hwirq(data);
> +
> +	auxirq.enables |= (1 << hwirq);
> +}
> +
> +static struct irq_chip bcm2835_auxirq_chip = {
> +	.name = "bcm2835-auxirq",
> +	.irq_mask = bcm2835_auxirq_mask,
> +	.irq_unmask = bcm2835_auxirq_unmask,
> +};
> +
> +static const struct irq_domain_ops bcm2835_auxirq_ops = {
> +	.xlate = bcm2835_auxirq_xlate//irq_domain_xlate_onecell

The C++ comment looks like an artifact

> +};
> +
>  static int bcm2835_aux_clk_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
>  	struct clk_hw_onecell_data *onecell;
>  	const char *parent;
>  	struct clk *parent_clk;
> +	int parent_irq;
>  	struct resource *res;
>  	void __iomem *reg, *gate;
>  
> @@ -41,6 +131,36 @@ static int bcm2835_aux_clk_probe(struct platform_device *pdev)
>  	if (IS_ERR(reg))
>  		return PTR_ERR(reg);
>  
> +	parent_irq = irq_of_parse_and_map(node, 0);
> +	if (parent_irq) {
> +		int ret;
> +		int i;
> +
> +		/* Manage the AUX irq as well */
> +		auxirq.status = reg + BCM2835_AUXIRQ;
> +		auxirq.domain = irq_domain_add_linear(node,
> +						      BCM2835_AUXIRQ_NUM_IRQS,
> +						      &bcm2835_auxirq_ops,
> +						      NULL);
> +		if (!auxirq.domain)
> +			return -ENXIO;
> +
> +		for (i = 0; i < BCM2835_AUXIRQ_NUM_IRQS; i++) {
> +			unsigned int irq = irq_create_mapping(auxirq.domain, i);
> +
> +			if (irq == 0)
> +				return -ENXIO;
> +
> +			irq_set_chip_and_handler(irq, &bcm2835_auxirq_chip,
> +						 handle_level_irq);
> +		}
> +
> +		ret = devm_request_irq(dev, parent_irq, bcm2835_auxirq_handler,
> +				       0, "bcm2835-auxirq", NULL);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	onecell = devm_kmalloc(dev, sizeof(*onecell) + sizeof(*onecell->hws) *
>  			       BCM2835_AUX_CLOCK_COUNT, GFP_KERNEL);
>  	if (!onecell)
> -- 
> 1.9.1
>