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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <154724735183.169631.5181161977573120432@swboyd.mtv.corp.google.com>
Date:   Fri, 11 Jan 2019 14:55:51 -0800
From:   Stephen Boyd <sboyd@...nel.org>
To:     Lina Iyer <ilina@...eaurora.org>
Cc:     evgreen@...omium.org, marc.zyngier@....com,
        linux-kernel@...r.kernel.org, rplsssn@...eaurora.org,
        linux-arm-msm@...r.kernel.org, thierry.reding@...il.com,
        bjorn.andersson@...aro.org
Subject: Re: [PATCH 3/7] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs

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.