Hi,
On 3-Oct-24 7:32 PM, Andy Shevchenko wrote:
> While design wise the idea of converting the driver to use
> the hierarchy of the IRQ chips is correct, the implementation
> has (inherited) flaws. This was unvelead when platform_get_irq()
> had started WARN() on IRQ 0 that is supposed to be a Linux
> IRQ number (also known as vIRQ).
>
> Rework the driver to respect IRQ domain when creating each MFD
> device separately, as the domain is not the same for all of them.
>
> Fixes: 957ae5098185 ("platform/x86: Add Whiskey Cove PMIC TMU support")
> Fixes: 57129044f504 ("mfd: intel_soc_pmic_bxtwc: Use chained IRQs for second level IRQ chips")
> Reported-by: Zhang Ning <zhangn1985@xxxxxxxxxxx>
> Closes: https://lore.kernel.org/r/TY2PR01MB3322FEDCDC048B7D3794F922CDBA2@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Thanks, patch looks good to me:
Acked-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Please feel free to merge this through the MFD tree as suggested in
the cover-letter.
Regards,
Hans
> ---
> drivers/mfd/intel_soc_pmic_bxtwc.c | 31 ++++++++++++++------------
> drivers/platform/x86/intel/bxtwc_tmu.c | 22 +++++-------------
> 2 files changed, 23 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
> index d72995a9e820..628108dcf545 100644
> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
> @@ -245,12 +245,6 @@ static struct mfd_cell bxt_wc_dev[] = {
> .num_resources = ARRAY_SIZE(bcu_resources),
> .resources = bcu_resources,
> },
> - {
> - .name = "bxt_wcove_tmu",
> - .num_resources = ARRAY_SIZE(tmu_resources),
> - .resources = tmu_resources,
> - },
> -
> {
> .name = "bxt_wcove_gpio",
> .num_resources = ARRAY_SIZE(gpio_resources),
> @@ -261,6 +255,14 @@ static struct mfd_cell bxt_wc_dev[] = {
> },
> };
>
> +static const struct mfd_cell bxt_wc_tmu_dev[] = {
> + {
> + .name = "bxt_wcove_tmu",
> + .num_resources = ARRAY_SIZE(tmu_resources),
> + .resources = tmu_resources,
> + },
> +};
> +
> static struct mfd_cell bxt_wc_chgr_dev[] = {
> {
> .name = "bxt_wcove_usbc",
> @@ -489,6 +491,15 @@ static int bxtwc_probe(struct platform_device *pdev)
> if (ret)
> return dev_err_probe(dev, ret, "Failed to add IRQ chip\n");
>
> + ret = bxtwc_add_chained_devices(pmic, bxt_wc_tmu_dev, ARRAY_SIZE(bxt_wc_tmu_dev),
> + pmic->irq_chip_data,
> + BXTWC_TMU_LVL1_IRQ,
> + IRQF_ONESHOT,
> + &bxtwc_regmap_irq_chip_tmu,
> + &pmic->irq_chip_data_tmu);
> + if (ret)
> + return ret;
> +
> ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
> BXTWC_PWRBTN_LVL1_IRQ,
> IRQF_ONESHOT,
> @@ -497,14 +508,6 @@ static int bxtwc_probe(struct platform_device *pdev)
> if (ret)
> return dev_err_probe(dev, ret, "Failed to add PWRBTN IRQ chip\n");
>
> - ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
> - BXTWC_TMU_LVL1_IRQ,
> - IRQF_ONESHOT,
> - &bxtwc_regmap_irq_chip_tmu,
> - &pmic->irq_chip_data_tmu);
> - if (ret)
> - return dev_err_probe(dev, ret, "Failed to add TMU IRQ chip\n");
> -
> /* Add chained IRQ handler for BCU IRQs */
> ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
> BXTWC_BCU_LVL1_IRQ,
> diff --git a/drivers/platform/x86/intel/bxtwc_tmu.c b/drivers/platform/x86/intel/bxtwc_tmu.c
> index d0e2a3c293b0..9ac801b929b9 100644
> --- a/drivers/platform/x86/intel/bxtwc_tmu.c
> +++ b/drivers/platform/x86/intel/bxtwc_tmu.c
> @@ -48,9 +48,8 @@ static irqreturn_t bxt_wcove_tmu_irq_handler(int irq, void *data)
> static int bxt_wcove_tmu_probe(struct platform_device *pdev)
> {
> struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent);
> - struct regmap_irq_chip_data *regmap_irq_chip;
> struct wcove_tmu *wctmu;
> - int ret, virq, irq;
> + int ret;
>
> wctmu = devm_kzalloc(&pdev->dev, sizeof(*wctmu), GFP_KERNEL);
> if (!wctmu)
> @@ -59,27 +58,18 @@ static int bxt_wcove_tmu_probe(struct platform_device *pdev)
> wctmu->dev = &pdev->dev;
> wctmu->regmap = pmic->regmap;
>
> - irq = platform_get_irq(pdev, 0);
> - if (irq < 0)
> - return irq;
> + wctmu->irq = platform_get_irq(pdev, 0);
> + if (wctmu->irq < 0)
> + return wctmu->irq;
>
> - regmap_irq_chip = pmic->irq_chip_data_tmu;
> - virq = regmap_irq_get_virq(regmap_irq_chip, irq);
> - if (virq < 0) {
> - dev_err(&pdev->dev,
> - "failed to get virtual interrupt=%d\n", irq);
> - return virq;
> - }
> -
> - ret = devm_request_threaded_irq(&pdev->dev, virq,
> + ret = devm_request_threaded_irq(&pdev->dev, wctmu->irq,
> NULL, bxt_wcove_tmu_irq_handler,
> IRQF_ONESHOT, "bxt_wcove_tmu", wctmu);
> if (ret) {
> dev_err(&pdev->dev, "request irq failed: %d,virq: %d\n",
> - ret, virq);
> + ret, wctmu->irq);
> return ret;
> }
> - wctmu->irq = virq;
>
> /* Unmask TMU second level Wake & System alarm */
> regmap_update_bits(wctmu->regmap, BXTWC_MTMUIRQ_REG,
