Reply-To: <qiujiang@huawei.com>
Subject: Re: [PATCH v5 1/3] gpio: designware: convert device node to fwnode
References: <1457077474-124064-1-git-send-email-qiujiang@huawei.com>
 <1457077474-124064-2-git-send-email-qiujiang@huawei.com>
 <CANk1AXRTvSRZkWMQf03Ty+ouxcJDpD-hJ9tjXSqh5Z0OzOhk+w@mail.gmail.com>
 <CAHp75VfGJZnOWUdZkjCbraupg=KUw2-jhtiRHLxrT2uUwJdvow@mail.gmail.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>,
        Alan Tull <delicious.quinoa@gmail.com>
CC: Linus Walleij <linus.walleij@linaro.org>,
        Alexandre Courbot <gnurou@gmail.com>,
        Mika Westerberg <mika.westerberg@linux.intel.com>,
        linux-kernel <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>
From: Jiang Qiu <qiujiang@huawei.com>
Message-ID: <56E80688.3030407@huawei.com>
Date: Tue, 15 Mar 2016 20:56:40 +0800
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101
 Thunderbird/38.5.1
MIME-Version: 1.0
In-Reply-To: <CAHp75VfGJZnOWUdZkjCbraupg=KUw2-jhtiRHLxrT2uUwJdvow@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3619
Lines: 86

在 2016/3/11 4:27, Andy Shevchenko 写道:
> On Thu, Mar 10, 2016 at 9:09 PM, Alan Tull <delicious.quinoa@gmail.com> wrote:
>> On Fri, Mar 4, 2016 at 1:44 AM, qiujiang <qiujiang@huawei.com> wrote:
>>> This patch converts device node to fwnode in
>>> dwapb_port_property for designware gpio driver,
>>> so as to provide a unified data structure for DT
>>> and ACPI bindings.
>>>
>>> Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>> Signed-off-by: qiujiang <qiujiang@huawei.com>
>>> @@ -496,18 +492,19 @@ 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")) {
>> Hi Qiujiang,
>>
>> Is there a reason to use "of_property_read_bool" here instead of
>> "device_property_read_bool" or similar?
> Yeah, this patch looks unfinished.
>
> This should be
>      if (pp->idx == 0 &&  fwnode_property_read_bool(fwnode,
> "interrupt-controller")) { 
>> Alan
>>
>>> +                       pp->irq = irq_of_parse_and_map(to_of_node(fwnode), 0);
> But here should be common method called which takes fwnode on input.
Hi Alan, Andy,

There is some trouble to replace the interface irq_of_parse_and_map and take
fwnode on input.

As far as I know, there are two common APIs to parse interrupt resource:
1) irq_of_parse_and_map() - It is a DT specific API, and have to use of_node as
input.

2) platform_get_irq() - It could be used to DT as well as ACPI, It needs platform_
device as input. In this driver, the DTs is defined as this:

pc_gpio0: gpio@802e0000 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        compatible = "snps,dw-apb-gpio";
                        reg = <0x0 0x802e0000 0x0 0x10000>;

                        porta: gpio-controller@0 {
                                compatible = "snps,dw-apb-gpio-port";
                                gpio-controller;
                                #gpio-cells = <2>;
                                snps,nr-gpios = <32>;
                                reg = <0>;
                                interrupt-controller;
                                #interrupt-cells = <2>;
                                interrupts = <0 312 4>;
                       };
 };

The compatible string, which is "snps,dw-apb-gpio", is only for the parent node.
That is mean, there is no platform device for the child node, but the interrupt
resource is defined in child node.

Is there any other APIs can be used to parse interrupts resource using fwnode
as input?

Thanks, Jiang
>>>                         if (!pp->irq) {
>>>                                 dev_warn(dev, "no irq for bank %s\n",
>>> -                                        port_np->full_name);
>>> +                                        to_of_node(fwnode)->full_name);
>>>                         }
>>>                 }
>>>
>>>                 pp->irq_shared  = false;
>>>                 pp->gpio_base   = -1;
>>> -               pp->name        = port_np->full_name;
>>> +               pp->name        = to_of_node(fwnode)->full_name;
> Also those two should be device property source agnostic. That's what
> I tried to tell earlier.
>