Hey Geert,
On Tue, Nov 24, 2015 at 04:08:13PM +0100, Geert Uytterhoeven wrote:
> If the Renesas External IRQ Pin driver cannot find a functional clock,
> it prints a warning, .e.g.
>
> renesas_intc_irqpin fe78001c.interrupt-controller: unable to get clock
>
> and continues, as the clock is optional, depending on the SoC type.
> This warning may confuse users.
>
> To fix this, add a flag to indicate that the clock is mandatory or
> optional, and add a few more compatible entries:
> - If the clock is mandatory (on R-Mobile A1 or SH-Mobile AG5), a
> missing clock is now treated as a fatal error,
> - If the clock is optional (on R-Car Gen1, or using the generic
> "renesas,intc-irqpin" compatible value), the warning is no longer
> printed.
>
> This requires making struct intc_irqpin_irlm_config more generic by
> renaming it to intc_irqpin_config, and adding a flag to indicate if IRLM
> is needed.
> The new clock flag is merged with the existing shared_irqs boolean into
> a bitfield to save space.
>
> Suggested-by: Magnus Damm <magnus.d...@gmail.com>
> Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>
> ---
> Tested on
> - r8a7779/marzen (no functional clock),
> - r8a7740/armadillo (with and without mandatory functional clock),
> - sh73a0/kzm9g (with and without mandatory functional clock).
>
> drivers/irqchip/irq-renesas-intc-irqpin.c | 45
> ++++++++++++++++++++++---------
> 1 file changed, 33 insertions(+), 12 deletions(-)
I'll pick up this a the previous series you just sent (fyi: tglx).
> diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c
> b/drivers/irqchip/irq-renesas-intc-irqpin.c
> index 7f6cf19aa6ac00d7..713177d97c7aa0b6 100644
> --- a/drivers/irqchip/irq-renesas-intc-irqpin.c
> +++ b/drivers/irqchip/irq-renesas-intc-irqpin.c
> @@ -79,12 +79,15 @@ struct intc_irqpin_priv {
> struct irq_chip irq_chip;
> struct irq_domain *irq_domain;
> struct clk *clk;
> - bool shared_irqs;
> + unsigned shared_irqs:1;
> + unsigned needs_clk:1;
> u8 shared_irq_mask;
> };
>
> -struct intc_irqpin_irlm_config {
> +struct intc_irqpin_config {
> unsigned int irlm_bit;
> + unsigned needs_irlm:1;
> + unsigned needs_clk:1;
> };
Why not just stick with bool? Changing shared_irqs -> has_shared_irqs?
> static unsigned long intc_irqpin_read32(void __iomem *iomem)
> @@ -359,8 +362,15 @@ static const struct irq_domain_ops
> intc_irqpin_irq_domain_ops = {
> .xlate = irq_domain_xlate_twocell,
> };
>
> -static const struct intc_irqpin_irlm_config intc_irqpin_irlm_r8a777x = {
> +static const struct intc_irqpin_config intc_irqpin_irlm_r8a777x = {
> .irlm_bit = 23, /* ICR0.IRLM0 */
> + .needs_irlm = 1,
> + .needs_clk = 0,
> +};
> +
> +static const struct intc_irqpin_config intc_irqpin_rmobile = {
> + .needs_irlm = 0,
> + .needs_clk = 1,
> };
>
> static const struct of_device_id intc_irqpin_dt_ids[] = {
> @@ -369,12 +379,17 @@ static const struct of_device_id intc_irqpin_dt_ids[] =
> {
> .data = &intc_irqpin_irlm_r8a777x },
> { .compatible = "renesas,intc-irqpin-r8a7779",
> .data = &intc_irqpin_irlm_r8a777x },
> + { .compatible = "renesas,intc-irqpin-r8a7740",
> + .data = &intc_irqpin_rmobile },
> + { .compatible = "renesas,intc-irqpin-sh73a0",
> + .data = &intc_irqpin_rmobile },
> {},
> };
> MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
Add these compatibles and the clock requirement info to the devicetree
binding?
> @@ -500,10 +521,10 @@ static int intc_irqpin_probe(struct platform_device
> *pdev)
>
> /* scan for shared interrupt lines */
> ref_irq = p->irq[0].requested_irq;
> - p->shared_irqs = true;
> + p->shared_irqs = 1;
> for (k = 1; k < nirqs; k++) {
> if (ref_irq != p->irq[k].requested_irq) {
> - p->shared_irqs = false;
> + p->shared_irqs = 0;
> break;
> }
> }
The bool suggestion, above, would make this easier to read imo.
thx,
Jason.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
