linux-kernel - Re: [PATCH RFC 06/14] drivers: irqchip: pdc: additionally set type in SPI config registers

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: <5d71a6db.1c69fb81.1bc1c.9225@mx.google.com>
Date:   Thu, 05 Sep 2019 17:22:50 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     Lina Iyer <ilina@...eaurora.org>, evgreen@...omium.org,
        linus.walleij@...aro.org, marc.zyngier@....com
Cc:     linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        bjorn.andersson@...aro.org, mkshah@...eaurora.org,
        linux-gpio@...r.kernel.org, rnayak@...eaurora.org,
        Lina Iyer <ilina@...eaurora.org>
Subject: Re: [PATCH RFC 06/14] drivers: irqchip: pdc: additionally set type in SPI config registers

Quoting Lina Iyer (2019-08-29 11:11:55)
> diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
> index ad1faf634bcf..bf5f98bb4d2b 100644
> --- a/drivers/irqchip/qcom-pdc.c
> +++ b/drivers/irqchip/qcom-pdc.c
> @@ -100,6 +112,57 @@ static void qcom_pdc_gic_unmask(struct irq_data *d)
>         irq_chip_unmask_parent(d);
>  }
>  
> +static u32 __spi_pin_read(unsigned int pin)
> +{
> +       void __iomem *cfg_reg = spi_cfg->base + pin * 4;
> +       u64 scm_cfg_reg = spi_cfg->start + pin * 4;
> +
> +       if (spi_cfg->scm_io) {
> +               unsigned int val;
> +
> +               qcom_scm_io_readl(scm_cfg_reg, &val);
> +               return val;
> +       } else {
> +               return readl(cfg_reg);
> +       }

Please remove the else and just return readl() result instead.

> +}
> +
> +static void __spi_pin_write(unsigned int pin, unsigned int val)
> +{
> +       void __iomem *cfg_reg = spi_cfg->base + pin * 4;
> +       u64 scm_cfg_reg = spi_cfg->start + pin * 4;
> +
> +       if (spi_cfg->scm_io)
> +               qcom_scm_io_writel(scm_cfg_reg, val);
> +       else
> +               writel(val, cfg_reg);
> +}
> +
> +static int spi_configure_type(irq_hw_number_t hwirq, unsigned int type)
> +{
> +       int spi = hwirq - 32;
> +       u32 pin = spi / 32;
> +       u32 mask = BIT(spi % 32);
> +       u32 val;
> +       unsigned long flags;
> +
> +       if (!spi_cfg)
> +               return 0;
> +
> +       if (pin * 4 > spi_cfg->size)
> +               return -EFAULT;
> +
> +       raw_spin_lock_irqsave(&pdc_lock, flags);

Ah I don't think the regmap would use a raw spinlock, so that's another
hurdle to get over here.

> +       val = __spi_pin_read(pin);
> +       val &= ~mask;
> +       if (type & IRQ_TYPE_LEVEL_MASK)
> +               val |= mask;
> +       __spi_pin_write(pin, val);

Does monitoring level triggered interrupts matter? I'm asking if the
whole thing can be configured to monitor for edges regardless of trigger
type and then let the level handling be done by the GIC after the wakeup
or when the device is active.

> +       raw_spin_unlock_irqrestore(&pdc_lock, flags);
> +
> +       return 0;
> +}
> +
>  /*
>   * GIC does not handle falling edge or active low. To allow falling edge and
>   * active low interrupts to be handled at GIC, PDC has an inverter that inverts