Linux-PWM Archive mirror
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: "Uwe Kleine-König" <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
Date: Thu, 15 Feb 2024 21:25:56 +0200	[thread overview]
Message-ID: <Zc5lRMQwROtVJdhV@smile.fi.intel.com> (raw)
In-Reply-To: <likebxfhlcg6equjhxnf7cimsgac4qvoge3bf65qyir6apwq4n@iotwg6zjjr6c>

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



  reply	other threads:[~2024-02-15 19:26 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-08  7:05 [PATCH v2 0/5] DesignWare PWM improvements Raag Jadav
2024-02-08  7:05 ` [PATCH v2 1/5] pwm: dwc: drop redundant error check Raag Jadav
2024-02-08  7:46   ` Uwe Kleine-König
2024-02-08 17:04     ` Andy Shevchenko
2024-02-08 17:06       ` Andy Shevchenko
2024-02-14 17:45       ` Uwe Kleine-König
2024-02-14 17:54         ` Andy Shevchenko
2024-02-15  9:22           ` Uwe Kleine-König
2024-02-15 13:36             ` Andy Shevchenko
2024-02-15 17:20               ` Uwe Kleine-König
2024-02-15 19:25                 ` Andy Shevchenko [this message]
2024-02-08  7:05 ` [PATCH v2 2/5] pwm: dwc: Add 16 channel support for Intel Elkhart Lake Raag Jadav
2024-02-08 17:20   ` Andy Shevchenko
2024-02-09 19:18     ` Raag Jadav
2024-02-09 19:41       ` Andy Shevchenko
2024-02-10 17:19         ` Uwe Kleine-König
2024-02-12 11:53           ` Andy Shevchenko
2024-02-08  7:05 ` [PATCH v2 3/5] pwm: dwc: simplify error handling Raag Jadav
2024-02-08 17:22   ` Andy Shevchenko
2024-02-09 20:33     ` Raag Jadav
2024-02-12 11:52       ` Andy Shevchenko
2024-02-08  7:05 ` [PATCH v2 4/5] pwm: dwc: access driver_data using dev_get_drvdata() Raag Jadav
2024-02-08  7:05 ` [PATCH v2 5/5] pwm: dwc: use pm_sleep_ptr() macro Raag Jadav
2024-02-08 17:22   ` Andy Shevchenko
2024-02-15 11:22     ` Uwe Kleine-König
2024-02-08 17:23 ` [PATCH v2 0/5] DesignWare PWM improvements Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zc5lRMQwROtVJdhV@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=jarkko.nikula@linux.intel.com \
    --cc=lakshmi.sowjanya.d@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=raag.jadav@intel.com \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).