On Fri, Jan 11 2019 at 15:55 -0700, Stephen Boyd wrote:
Quoting Lina Iyer (2019-01-07 10:48:36)
On Thu, Dec 20 2018 at 13:19 -0700, Stephen Boyd wrote:
>Quoting Lina Iyer (2018-12-19 14:11:01)
>> +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.
Are you going to add sdm?
Realized this is a bug. Will fix.
>> + { },
>> +};
>
>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.
Ok. The benefit to my approach is that we don't do the string match
twice. We do it once and sacrifice a little code space to jump to the
real init function with the data we want. We can then put those chip
tables inside some #ifdef to save space and allow configurations to turn
off everything in EXPERT mode but leave everything default enabled
otherwise.
Sure. I will use this idea.
>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.
Uh oh. That's sad.
