Received-SPF: pass (google.com: domain of linux-kernel+bounces-67572-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Thu, 15 Feb 2024 21:25:56 +0200
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= <u.kleine-koenig@pengutronix.de>
Cc: Raag Jadav <raag.jadav@intel.com>, jarkko.nikula@linux.intel.com,
	mika.westerberg@linux.intel.com, lakshmi.sowjanya.d@intel.com,
	linux-pwm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/5] pwm: dwc: drop redundant error check
Message-ID: <Zc5lRMQwROtVJdhV@smile.fi.intel.com>
References: <20240208070529.28562-1-raag.jadav@intel.com>
 <20240208070529.28562-2-raag.jadav@intel.com>
 <qrwcje4t2pbbxilnlfz2q7njodcp6vl54uaypdbvjgvwhgvc5g@4eq5wvumpjjx>
 <ZcUJoSMhgC7lhY-H@smile.fi.intel.com>
 <cv6w4n2ptcdehn5n3mipuyfrtemm4rldhiyppazk4uqdn2xx7e@hxg4kldaacxk>
 <Zcz-csPY5x29DP7v@smile.fi.intel.com>
 <sd2ugzjrmrdvcyxotoyg53qp3i7ta4yko225ln3gk4fmik7iof@a7mab6o2kkvz>
 <Zc4TTLetiGhJlx8d@smile.fi.intel.com>
 <likebxfhlcg6equjhxnf7cimsgac4qvoge3bf65qyir6apwq4n@iotwg6zjjr6c>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <likebxfhlcg6equjhxnf7cimsgac4qvoge3bf65qyir6apwq4n@iotwg6zjjr6c>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo

On Thu, Feb 15, 2024 at 06:20:15PM +0100, Uwe Kleine-K?nig wrote:
> On Thu, Feb 15, 2024 at 03:36:12PM +0200, Andy Shevchenko wrote:
> > On Thu, Feb 15, 2024 at 10:22:57AM +0100, Uwe Kleine-K?nig wrote:
> > > If a driver author knows it while writing the code, it's obvious. But if
> > > the driver author looks again in 2 years or someone else (e.g. me with
> > > the PWM maintainer hat on and with little pci experience) that knowledge
> > > might be faded.
> > 
> > This is widely used pattern. Anybody who works with Git should know how
> > to use `git grep` tool. If in doubts, always can ask in the mailing lists.
> 
> IMHO you're assuming to much. If someone sees this pattern and quickly
> looks at the implementation of pcim_iomap_table() they might (as I did)
> conclude that this call should be error checked. If they send a patch in
> say 2 years I think I won't remember this discussion/patch and happily
> accept this patch. And I probably won't get enough doubts to start
> grepping around.
> 
> > I still consider it redundant.
> > 
> > P.S. That's what you call "bikeshedding" (done by yourself here)?
> 
> I can understand that you consider that bikeshedding given that for you
> it's obvious that the second function cannot fail. For me it's not and I
> take this as a hint that it's not obvious for everyone.

The bottom line that PCI devres code should be refactored. And IIRC somebody
is doing that job, not sure at which stage it is now.

-- 
With Best Regards,
Andy Shevchenko