From: Jiang Qiu <qiujiang@huawei.com> To: Andy Shevchenko <andy.shevchenko@gmail.com> Cc: Linus Walleij <linus.walleij@linaro.org>, Alexandre Courbot <gnurou@gmail.com>, Mika Westerberg <mika.westerberg@linux.intel.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>, "linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>, linuxarm@huawei.com, haifeng.wei@huawei.com, charles.chenxin@huawei.com Subject: Re: [PATCH v4 1/2] gpio: designware: switch device node to fwnode and add acpi binding Date: Tue, 1 Mar 2016 09:09:32 +0800 [thread overview] Message-ID: <56D4EBCC.3070406@huawei.com> (raw) In-Reply-To: <CAHp75Ves-+6pj_LZccxkKugXxJ9NJUwSotSCRf6=aOveBOx=Jg@mail.gmail.com> 在 2016/2/29 21:51, Andy Shevchenko 写道: > On Mon, Feb 29, 2016 at 3:13 PM, qiujiang <qiujiang@huawei.com> wrote: >> This patch switches device node to fwnode and adds acpi >> binding. As a result, DT and acpi bingdings are compatible >> for this driver. > > I'm not sure it makes sense to add ACPI binding here. It more logical > to me to add them in patch 2/2. > And I see that it touches different subsystems. > >> >> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > Due to above and Mika's Ack was only to ACPI parts, I think > you may be split this also to two and we will have clearly logical set: > 1. Convert to fwnode > 2. Add ACPI bindings > 3. ACPI event support. > > Does it sound okay to you? > Sure, I will do that ASAP, thank you. > If you do that I give my > Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com> > to intel_quark_i2c_gpio.c part and > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > to the rest. > >> Signed-off-by: qiujiang <qiujiang@huawei.com> >> --- >> drivers/gpio/gpio-dwapb.c | 62 +++++++++++++++++++------------- >> drivers/mfd/intel_quark_i2c_gpio.c | 2 +- >> include/linux/platform_data/gpio-dwapb.h | 2 +- >> 3 files changed, 39 insertions(+), 27 deletions(-) >> >> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c >> index 597de1e..7a37c65 100644 >> --- a/drivers/gpio/gpio-dwapb.c >> +++ b/drivers/gpio/gpio-dwapb.c >> @@ -7,6 +7,7 @@ >> * >> * All enquiries to support@picochip.com >> */ >> +#include <linux/acpi.h> > > +#include <linux/property.h> instead (see above). > >> #include <linux/gpio/driver.h> >> /* FIXME: for gpio_get_value(), replace this with direct register read */ >> #include <linux/gpio.h> > >> @@ -496,18 +492,27 @@ dwapb_gpio_get_pdata_of(struct device *dev) >> * Only port A can provide interrupts in all configurations of >> * the IP. >> */ >> - if (pp->idx == 0 && >> - of_property_read_bool(port_np, "interrupt-controller")) { >> - pp->irq = irq_of_parse_and_map(port_np, 0); >> + if (dev->of_node && pp->idx == 0 && >> + of_property_read_bool(to_of_node(fwnode), >> + "interrupt-controller")) { > >> + if (has_acpi_companion(dev) && pp->idx == 0) >> + pp->irq = platform_get_irq(to_platform_device(dev), 0); >> + > > To patch 2. > >> pp->irq_shared = false; >> pp->gpio_base = -1; > >> - pp->name = port_np->full_name; >> + >> + if (dev->of_node) >> + pp->name = to_of_node(fwnode)->full_name; >> + > > And here you have to provide something in case of default / built-in > device properties. > > >> + if (has_acpi_companion(dev)) >> + pp->name = acpi_dev_name(to_acpi_device_node(fwnode)); > > To patch 2. > >> } >> >> return pdata; > >> @@ -580,6 +585,12 @@ static const struct of_device_id dwapb_of_match[] = { >> }; >> MODULE_DEVICE_TABLE(of, dwapb_of_match); >> >> +static const struct acpi_device_id dwapb_acpi_match[] = { >> + {"HISI0181", 0}, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(acpi, dwapb_acpi_match); >> + >> #ifdef CONFIG_PM_SLEEP >> static int dwapb_gpio_suspend(struct device *dev) >> { >> @@ -674,6 +685,7 @@ static struct platform_driver dwapb_gpio_driver = { >> .name = "gpio-dwapb", >> .pm = &dwapb_gpio_pm_ops, >> .of_match_table = of_match_ptr(dwapb_of_match), >> + .acpi_match_table = ACPI_PTR(dwapb_acpi_match), >> }, >> .probe = dwapb_gpio_probe, >> .remove = dwapb_gpio_remove, > > Ditto to the above hunks. > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Jiang Qiu <qiujiang@huawei.com> To: Andy Shevchenko <andy.shevchenko@gmail.com> Cc: Linus Walleij <linus.walleij@linaro.org>, Alexandre Courbot <gnurou@gmail.com>, Mika Westerberg <mika.westerberg@linux.intel.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>, "linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>, <linuxarm@huawei.com>, <haifeng.wei@huawei.com>, <charles.chenxin@huawei.com> Subject: Re: [PATCH v4 1/2] gpio: designware: switch device node to fwnode and add acpi binding Date: Tue, 1 Mar 2016 09:09:32 +0800 [thread overview] Message-ID: <56D4EBCC.3070406@huawei.com> (raw) In-Reply-To: <CAHp75Ves-+6pj_LZccxkKugXxJ9NJUwSotSCRf6=aOveBOx=Jg@mail.gmail.com> 在 2016/2/29 21:51, Andy Shevchenko 写道: > On Mon, Feb 29, 2016 at 3:13 PM, qiujiang <qiujiang@huawei.com> wrote: >> This patch switches device node to fwnode and adds acpi >> binding. As a result, DT and acpi bingdings are compatible >> for this driver. > > I'm not sure it makes sense to add ACPI binding here. It more logical > to me to add them in patch 2/2. > And I see that it touches different subsystems. > >> >> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > Due to above and Mika's Ack was only to ACPI parts, I think > you may be split this also to two and we will have clearly logical set: > 1. Convert to fwnode > 2. Add ACPI bindings > 3. ACPI event support. > > Does it sound okay to you? > Sure, I will do that ASAP, thank you. > If you do that I give my > Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com> > to intel_quark_i2c_gpio.c part and > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > to the rest. > >> Signed-off-by: qiujiang <qiujiang@huawei.com> >> --- >> drivers/gpio/gpio-dwapb.c | 62 +++++++++++++++++++------------- >> drivers/mfd/intel_quark_i2c_gpio.c | 2 +- >> include/linux/platform_data/gpio-dwapb.h | 2 +- >> 3 files changed, 39 insertions(+), 27 deletions(-) >> >> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c >> index 597de1e..7a37c65 100644 >> --- a/drivers/gpio/gpio-dwapb.c >> +++ b/drivers/gpio/gpio-dwapb.c >> @@ -7,6 +7,7 @@ >> * >> * All enquiries to support@picochip.com >> */ >> +#include <linux/acpi.h> > > +#include <linux/property.h> instead (see above). > >> #include <linux/gpio/driver.h> >> /* FIXME: for gpio_get_value(), replace this with direct register read */ >> #include <linux/gpio.h> > >> @@ -496,18 +492,27 @@ dwapb_gpio_get_pdata_of(struct device *dev) >> * Only port A can provide interrupts in all configurations of >> * the IP. >> */ >> - if (pp->idx == 0 && >> - of_property_read_bool(port_np, "interrupt-controller")) { >> - pp->irq = irq_of_parse_and_map(port_np, 0); >> + if (dev->of_node && pp->idx == 0 && >> + of_property_read_bool(to_of_node(fwnode), >> + "interrupt-controller")) { > >> + if (has_acpi_companion(dev) && pp->idx == 0) >> + pp->irq = platform_get_irq(to_platform_device(dev), 0); >> + > > To patch 2. > >> pp->irq_shared = false; >> pp->gpio_base = -1; > >> - pp->name = port_np->full_name; >> + >> + if (dev->of_node) >> + pp->name = to_of_node(fwnode)->full_name; >> + > > And here you have to provide something in case of default / built-in > device properties. > > >> + if (has_acpi_companion(dev)) >> + pp->name = acpi_dev_name(to_acpi_device_node(fwnode)); > > To patch 2. > >> } >> >> return pdata; > >> @@ -580,6 +585,12 @@ static const struct of_device_id dwapb_of_match[] = { >> }; >> MODULE_DEVICE_TABLE(of, dwapb_of_match); >> >> +static const struct acpi_device_id dwapb_acpi_match[] = { >> + {"HISI0181", 0}, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(acpi, dwapb_acpi_match); >> + >> #ifdef CONFIG_PM_SLEEP >> static int dwapb_gpio_suspend(struct device *dev) >> { >> @@ -674,6 +685,7 @@ static struct platform_driver dwapb_gpio_driver = { >> .name = "gpio-dwapb", >> .pm = &dwapb_gpio_pm_ops, >> .of_match_table = of_match_ptr(dwapb_of_match), >> + .acpi_match_table = ACPI_PTR(dwapb_acpi_match), >> }, >> .probe = dwapb_gpio_probe, >> .remove = dwapb_gpio_remove, > > Ditto to the above hunks. >
next prev parent reply other threads:[~2016-03-01 1:10 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-02-29 13:13 [PATCH v4 0/2] gpio: designware: add gpio-signaled acpi event support for power button qiujiang 2016-02-29 13:13 ` qiujiang 2016-02-29 13:13 ` [PATCH v4 1/2] gpio: designware: switch device node to fwnode and add acpi binding qiujiang 2016-02-29 13:13 ` qiujiang 2016-02-29 13:51 ` Andy Shevchenko 2016-03-01 1:09 ` Jiang Qiu [this message] 2016-03-01 1:09 ` Jiang Qiu 2016-03-01 6:41 ` Jiang Qiu 2016-03-01 6:41 ` Jiang Qiu 2016-02-29 13:13 ` [PATCH v4 2/2] gpio: designware: add gpio-signaled acpi event support qiujiang 2016-02-29 13:13 ` qiujiang 2016-02-29 13:54 ` Andy Shevchenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=56D4EBCC.3070406@huawei.com \ --to=qiujiang@huawei.com \ --cc=andy.shevchenko@gmail.com \ --cc=charles.chenxin@huawei.com \ --cc=gnurou@gmail.com \ --cc=haifeng.wei@huawei.com \ --cc=linus.walleij@linaro.org \ --cc=linux-acpi@vger.kernel.org \ --cc=linux-gpio@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxarm@huawei.com \ --cc=mika.westerberg@linux.intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.