Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org E352360397
Date:   Fri, 11 Jan 2019 16:34:18 -0700
From:   Lina Iyer <ilina@codeaurora.org>
To:     Stephen Boyd <sboyd@kernel.org>
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
Subject: Re: [PATCH 3/7] drivers: irqchip: add PDC irqdomain for wakeup
 capable GPIOs
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
Content-Disposition: inline
In-Reply-To: <154724735183.169631.5181161977573120432@swboyd.mtv.corp.google.com>
User-Agent: Mutt/1.11.1 (2018-12-01)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.
>