Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 15 Feb 2022 19:11:38 +0200
From:   Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To:     Lee Jones <lee.jones@linaro.org>
Cc:     Wolfram Sang <wsa@kernel.org>, Jean Delvare <jdelvare@suse.de>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Hans de Goede <hdegoede@redhat.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        Tan Jui Nee <jui.nee.tan@intel.com>,
        Kate Hsuan <hpa@redhat.com>,
        Jonathan Yong <jonathan.yong@intel.com>,
        linux-kernel@vger.kernel.org, linux-edac@vger.kernel.org,
        linux-i2c@vger.kernel.org, linux-gpio@vger.kernel.org,
        platform-driver-x86@vger.kernel.org,
        Borislav Petkov <bp@alien8.de>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Tony Luck <tony.luck@intel.com>,
        James Morse <james.morse@arm.com>,
        Robert Richter <rric@kernel.org>,
        Jean Delvare <jdelvare@suse.com>,
        Peter Tyser <ptyser@xes-inc.com>,
        Mika Westerberg <mika.westerberg@linux.intel.com>,
        Andy Shevchenko <andy@kernel.org>,
        Mark Gross <markgross@kernel.org>,
        Henning Schild <henning.schild@siemens.com>
Subject: Re: [PATCH v4 5/8] mfd: lpc_ich: Add support for pinctrl in non-ACPI
 system
Message-ID: <YgveyspHVXCp2ul+@smile.fi.intel.com>
References: <20220131151346.45792-1-andriy.shevchenko@linux.intel.com>
 <20220131151346.45792-6-andriy.shevchenko@linux.intel.com>
 <YgvaqBB8fNVWp1lN@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YgvaqBB8fNVWp1lN@google.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

On Tue, Feb 15, 2022 at 04:54:00PM +0000, Lee Jones wrote:
> On Mon, 31 Jan 2022, Andy Shevchenko wrote:

Thank you for the review, my answers below.

...

> > +static struct resource apl_gpio_resources[APL_GPIO_NR_DEVICES][2] = {
> > +	[APL_GPIO_NORTH] = {
> > +		DEFINE_RES_MEM(APL_GPIO_NORTH_OFFSET, 0x1000),
> 
> Are these 0x1000's being over-written in lpc_ich_init_pinctrl()?
> 
> If so, why pre-initialise?

You mean to pre-initialize the offsets, but leave the length to be added
in the function? It can be done, but it feels inconsistent, since we would
have offsets and lengths in different places for the same thingy. That said,
I prefer current way for the sake of consistency.

> > +		DEFINE_RES_IRQ(APL_GPIO_IRQ),
> > +	},

...

> > +/* The order must be in sync with apl_pinctrl_soc_data */
> 
> Why does the order matter if you've pre-enumerated them all?

Indeed. I will drop the confusing comment in the next version.

-- 
With Best Regards,
Andy Shevchenko