Received-SPF: pass (google.com: domain of linux-kernel+bounces-14791-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20231226192149.1830592-1-markhas@chromium.org>
 <20231226122113.v3.2.Ifd0903f1c351e84376d71dbdadbd43931197f5ea@changeid> <ZYxbBkimS1VIuU0a@smile.fi.intel.com>
In-Reply-To: <ZYxbBkimS1VIuU0a@smile.fi.intel.com>
From: Mark Hasemeyer <markhas@chromium.org>
Date: Tue, 2 Jan 2024 13:03:42 -0700
Message-ID: <CANg-bXA12ZAd9vZFV9L2ffMmcG33ByJ-=+03aB+Rz3OMZQGCgw@mail.gmail.com>
Subject: Re: [PATCH v3 02/24] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by()
 to use resource
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: LKML <linux-kernel@vger.kernel.org>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, 
	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, Rob Herring <robh@kernel.org>, 
	Konrad Dybcio <konrad.dybcio@linaro.org>, Sudeep Holla <sudeep.holla@arm.com>, 
	Raul Rangel <rrangel@chromium.org>, Tzung-Bi Shih <tzungbi@kernel.org>, 
	Bartosz Golaszewski <brgl@bgdev.pl>, Len Brown <lenb@kernel.org>, 
	Linus Walleij <linus.walleij@linaro.org>, 
	Mika Westerberg <mika.westerberg@linux.intel.com>, "Rafael J. Wysocki" <rafael@kernel.org>, 
	Wolfram Sang <wsa@kernel.org>, linux-acpi@vger.kernel.org, linux-gpio@vger.kernel.org, 
	linux-i2c@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"

>
> > +                     *r = DEFINE_RES_NAMED(irq, 1, NULL, res_flags);
>
> So? The whole exercise with the first patch is to have here:
>
>                         *r = DEFINE_RES_IRQ_NAMED_FLAGS(irq, NULL, res_flags);

Thanks. I was staring at the macro changes in ioport.h for too long...

>
> > +     ret = acpi_dev_get_gpio_irq_resource(adev, name, index, &r);
> > +     return ret ?: r.start;
>
> Btw, this function requires header to include ioport.h. I'm not sure
> if it's good for ACPI. I would prefer safest approach, i.e. exporting
> this from a C code, i.e. gpiolib-acpi.c.

It actually just needs 'struct resource' declared. I removed the
dependency on ioport.h, but it may make sense to leave the definition
of 'acpi_dev_gpio_irq_get_by()' here because non ACPI based systems
need a default implementation anyway.