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? > > >> + { }, > >> +}; > > > >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. > > >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.