From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lina Iyer Subject: Re: [PATCH 3/7] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs Date: Fri, 11 Jan 2019 16:34:18 -0700 Message-ID: <20190111233418.GA29875@codeaurora.org> References: <20181219221105.3004-1-ilina@codeaurora.org> <20181219221105.3004-4-ilina@codeaurora.org> <154533717951.79149.1309452983166815703@swboyd.mtv.corp.google.com> <20190107184836.GG14960@codeaurora.org> <154724735183.169631.5181161977573120432@swboyd.mtv.corp.google.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Return-path: Content-Disposition: inline In-Reply-To: <154724735183.169631.5181161977573120432@swboyd.mtv.corp.google.com> Sender: linux-kernel-owner@vger.kernel.org To: Stephen Boyd Cc: evgreen@chromium.org, marc.zyngier@arm.com, linux-kernel@vger.kernel.org, rplsssn@codeaurora.org, linux-arm-msm@vger.kernel.org, thierry.reding@gmail.com, bjorn.andersson@linaro.org List-Id: linux-arm-msm@vger.kernel.org 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. >