Received-SPF: pass (google.com: domain of linux-kernel+bounces-20006-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240102210820.2604667-1-markhas@chromium.org>
 <20240102140734.v4.2.Ifd0903f1c351e84376d71dbdadbd43931197f5ea@changeid> <ZZlnNR1-yKLSIWeF@smile.fi.intel.com>
In-Reply-To: <ZZlnNR1-yKLSIWeF@smile.fi.intel.com>
From: Mark Hasemeyer <markhas@chromium.org>
Date: Mon, 8 Jan 2024 12:08:54 -0700
Message-ID: <CANg-bXA+zuPAYHKaYihkPN1W3+78gBx_Edvhb-a6DqD_adJeaA@mail.gmail.com>
Subject: Re: [PATCH v4 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>, Sudeep Holla <sudeep.holla@arm.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, Rob Herring <robh@kernel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, Konrad Dybcio <konrad.dybcio@linaro.org>, 
	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"

> Missing blank line.
> We put a commit message as
>
> $SUMARY
> ...blank line...
> $DESCRIPTION (can contain blank lines)
> ...blank line...
> $TAG block (may not contain blank lines)
>
> > Signed-off-by: Mark Hasemeyer <markhas@chromium.org>

Looks like a nuance of patman I need to iron out.

>
> ...
>
> > +                     unsigned long res_flags;
>
> Why not calling it irq_flags?

irq_flags is already used within the same scope, although it's
declared at the top of the function. I'll move the declaration to the
scope where it's used and rename irq_flags -> irq_type, and irq_res ->
irq_flags.

> > +struct resource;
>
> This...
>
> > +     struct resource r = {};
>
> > +     return ret ?: r.start;
>
> ...does _not_ cover these cases.
>
> Hence ioport.h must be included. Did I miss it?

You're right. It didn't break the build, which means ioport.h must be
included indirectly. I'll add it back.