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: Mon, 7 Jan 2019 11:48:36 -0700 Message-ID: <20190107184836.GG14960@codeaurora.org> References: <20181219221105.3004-1-ilina@codeaurora.org> <20181219221105.3004-4-ilina@codeaurora.org> <154533717951.79149.1309452983166815703@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: <154533717951.79149.1309452983166815703@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 Thu, Dec 20 2018 at 13:19 -0700, Stephen Boyd wrote: >Quoting Lina Iyer (2018-12-19 14:11:01) >> diff --git a/drivers/irqchip/qcom-pdc-data.c b/drivers/irqchip/qcom-pdc-data.c >> new file mode 100644 >> index 000000000000..99b4be0af5db >> --- /dev/null >> +++ b/drivers/irqchip/qcom-pdc-data.c >> @@ -0,0 +1,94 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2018, The Linux Foundation. All rights reserved. >> + */ >> + >> +#include "qcom-pdc.h" >> + [...] >> +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. >> + { }, >> +}; > >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. >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. Thanks, Lina