Re: [PATCH 3/7] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs

[Lina Iyer]

On Thu, Dec 20 2018 at 13:19 -0700, Stephen Boyd wrote:
> Quoting Lina Iyer (2018-12-19 14:11:01)
> > diff --git a/drivers/irqchip/qcom-pdc-data.c b/drivers/irqchip/qcom-pdc-data.c
> > new file mode 100644
> > index 000000000000..99b4be0af5db
> > --- /dev/null
> > +++ b/drivers/irqchip/qcom-pdc-data.c
> > @@ -0,0 +1,94 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> > + */
> > +
> > +#include "qcom-pdc.h"
> > +
[...]

> > +static const struct pdc_gpio_pin_data sdm845_gpio_data = {
> > +       .size = ARRAY_SIZE(sdm845_gpio_pdc_map),
> > +       .map = sdm845_gpio_pdc_map,
> > +};
> > +
> > +const struct of_device_id pdc_gpio_match_table[] = {
> > +       { .compatible = "qcom,845-pdc", .data = &sdm845_gpio_data },
>
> Why not qcom,sdm845-pdc?
The compatible matches the compatible specified in the PDC driver. Not
sure why the 'sdm' was left out at that time.

> > +       { },
> > +};
>
> I wonder why we wouldn't just put this all into the qcom-pdc.c file at
> the bottom and then have that IRQCHIP_DECLARE() macros call small
> functions that pass the pdc to gpio mapping table to qcom_pdc_init()
> that takes a third argument?
We could. I feel we would be adding tables like this and it just
clutters up the driver file. May be in the future we could move to
target specific data file like the gpios, but that could be excessive
too. Thought this might be a compromise. I am open to change.

> I really hope that in the future all the gpios can be wakeup capable so
> that we don't need to have the table at all!
I doubt there are plans to support that in hardware. We should plan for
supporting tables like this for other chipsets based on the PDC
architecture.

Thanks,
Lina