All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Rahul Tanwar <rahul.tanwar@linux.intel.com>,
	linux-pwm@vger.kernel.org, lee.jones@linaro.org,
	thierry.reding@gmail.com, p.zabel@pengutronix.de,
	robh+dt@kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, songjun.Wu@intel.com,
	cheol.yong.kim@intel.com, qi-ming.wu@intel.com,
	rahul.tanwar.linux@gmail.com, rtanwar@maxlinear.com
Subject: Re: [PATCH v13 2/2] Add PWM fan controller driver for LGM SoC
Date: Thu, 24 Sep 2020 16:23:34 +0300	[thread overview]
Message-ID: <20200924132334.GT3956970@smile.fi.intel.com> (raw)
In-Reply-To: <20200924065534.e2anwghhtysv63e7@pengutronix.de>

On Thu, Sep 24, 2020 at 08:55:34AM +0200, Uwe Kleine-König wrote:
> On Tue, Sep 15, 2020 at 04:23:37PM +0800, Rahul Tanwar wrote:

...

> > +	ret = lgm_clk_enable(dev, pc);
> > +	if (ret) {
> > +		dev_err(dev, "failed to enable clock\n");
> 
> You used dev_err_probe four times for six error paths. I wonder why you
> didn't use it here (and below for a failing pwmchip_add()).

dev_err_probe() makes sense when we might experience deferred probe. In neither
of mentioned function this can be the case.

> > +		return ret;
> > +	}

...

> > +	ret = lgm_reset_control_deassert(dev, pc);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "cannot deassert reset control\n");
> 
> After lgm_reset_control_deassert is called pc->rst is unused. So there
> is no need to have this member in struct lgm_pwm_chip. The same applies
> to ->clk. (You have to pass rst (or clk) to devm_add_action_or_reset for
> that to work. Looks like a nice idea anyhow.)

True. And above dev_err_probe() is not needed.

-- 
With Best Regards,
Andy Shevchenko



  parent reply	other threads:[~2020-09-24 13:39 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1600158087.git.rahul.tanwar@linux.intel.com>
2020-09-15  8:23 ` [PATCH v13 1/2] Add DT bindings YAML schema for PWM fan controller of LGM SoC Rahul Tanwar
2020-09-15  8:23 ` [PATCH v13 2/2] Add PWM fan controller driver for " Rahul Tanwar
2020-09-24  6:55   ` Uwe Kleine-König
2020-09-24  7:12     ` Thierry Reding
2020-09-24  7:38       ` Tanwar, Rahul
2020-09-24 13:23     ` Andy Shevchenko [this message]
2020-09-24 14:16       ` Uwe Kleine-König
2020-09-25  8:39         ` Andy Shevchenko
2020-09-28  6:45         ` Thierry Reding
2020-09-25  8:49     ` Tanwar, Rahul
2020-09-23 11:49 ` [PATCH v13 0/2] pwm: intel: Add PWM driver for a new SoC Thierry Reding

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=20200924132334.GT3956970@smile.fi.intel.com \
    --to=andriy.shevchenko@intel.com \
    --cc=cheol.yong.kim@intel.com \
    --cc=devicetree@vger.kernel.org \
    --cc=lee.jones@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=qi-ming.wu@intel.com \
    --cc=rahul.tanwar.linux@gmail.com \
    --cc=rahul.tanwar@linux.intel.com \
    --cc=robh+dt@kernel.org \
    --cc=rtanwar@maxlinear.com \
    --cc=songjun.Wu@intel.com \
    --cc=thierry.reding@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.