From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C4BDEC433FE for ; Wed, 4 May 2022 12:53:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1350036AbiEDM4h (ORCPT ); Wed, 4 May 2022 08:56:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41596 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236375AbiEDM4f (ORCPT ); Wed, 4 May 2022 08:56:35 -0400 Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2D9DF2AE12; Wed, 4 May 2022 05:52:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1651668779; x=1683204779; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=TAR0OSTmgPalp1GT6kqlq85T8vW4wKavBhWP3MJ2K2A=; b=dHgtXRnpPjjVlbRuYsjwUesITQQlcyEAnpWwSqK8E3QnR3mvzJAk3x94 Hu9ol/3/ksX4ORH3dx2tVizgJRR0+gz8k9VyzFP6adFTdT1874uzhCCgL 53vkHKHmzs182tA6TySkrYpLdZubHRjf2xAKlqVuHhKUfiznQLBI0p3bL JDOPsS/V9HQqtff/dd6WG2GAYw7LUiTRlIsMTZkesDf5ELsNhunkvacAQ ECQ32eEAIs892VDOTYhsDJ6WQ3GEzNERyp1SuQgTXHuaBZveF0/YDEiil IJWEZ0ENBVozZ3mkQphKUS0r8IIOwTRbrwRo64fW2SnagoJs9alYZNg/a A==; X-IronPort-AV: E=McAfee;i="6400,9594,10336"; a="292946346" X-IronPort-AV: E=Sophos;i="5.91,198,1647327600"; d="scan'208";a="292946346" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 May 2022 05:52:58 -0700 X-IronPort-AV: E=Sophos;i="5.91,198,1647327600"; d="scan'208";a="734387769" Received: from smile.fi.intel.com ([10.237.72.54]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 May 2022 05:52:52 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.95) (envelope-from ) id 1nmEUq-00Bssm-FU; Wed, 04 May 2022 15:52:48 +0300 Date: Wed, 4 May 2022 15:52:48 +0300 From: Andy Shevchenko To: Lee Jones Cc: Wolfram Sang , Jean Delvare , Heiner Kallweit , Hans de Goede , Linus Walleij , Tan Jui Nee , Kate Hsuan , Jonathan Yong , 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 , Mauro Carvalho Chehab , Tony Luck , James Morse , Robert Richter , Jean Delvare , Peter Tyser , Mika Westerberg , Andy Shevchenko , Mark Gross , Henning Schild Subject: Re: [PATCH v4 5/8] mfd: lpc_ich: Add support for pinctrl in non-ACPI system Message-ID: References: <20220131151346.45792-1-andriy.shevchenko@linux.intel.com> <20220131151346.45792-6-andriy.shevchenko@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo Precedence: bulk List-ID: X-Mailing-List: linux-edac@vger.kernel.org On Tue, Feb 15, 2022 at 05:29:51PM +0000, Lee Jones wrote: > On Tue, 15 Feb 2022, Andy Shevchenko wrote: > > 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. > > Don't you over-write this entry entirely? > > for (i = 0; i < ARRAY_SIZE(apl_gpio_devices); i++) { > struct resource *mem = &apl_gpio_resources[i][0]; > > /* Fill MEM resource */ > mem->start += base.start; > mem->end += base.start; > mem->flags = base.flags; > } > > Oh wait, you're just adding the base value to the offsets. > > In which case that comment is also confusing! I have realised that in current form it has a bug (*), so I re-do a bit the way that comment stays and the actual actions will be to *fill* the resource. *) unbinding and binding back will bring us to the completely wrong resources. -- With Best Regards, Andy Shevchenko