All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd@kernel.org>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: linux-pwm@vger.kernel.org, Sean Anderson <sean.anderson@seco.com>,
	Michael Turquette <mturquette@baylibre.com>,
	Russell King <linux@armlinux.org.uk>,
	Maxime Ripard <mripard@kernel.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	kernel@pengutronix.de, Michal Simek <michal.simek@amd.com>,
	linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] clk: Add a devm variant of clk_rate_exclusive_get()
Date: Thu, 04 Jan 2024 13:38:27 -0800	[thread overview]
Message-ID: <c1effbda6f323aa58935e1990ba3aed8.sboyd@kernel.org> (raw)
In-Reply-To: <g5ahts576gcub7iwn3xsaky3yu7cqdh3szu67ovixmrrci7zq6@t5fjhj6as5vk>

Quoting Uwe Kleine-König (2024-01-04 10:06:29)
> Hello Stephen,
> 
> On Mon, Dec 18, 2023 at 02:01:41PM +0100, Uwe Kleine-K�nig wrote:
> > [Cc += Maxime]
> > 
> > Hello Stephen,
> > 
> > On Sun, Dec 17, 2023 at 04:17:41PM -0800, Stephen Boyd wrote:
> > > Quoting Uwe Kleine-K�nig (2023-12-12 10:09:42)
> > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > > > index af2011c2a93b..78249ca2341c 100644
> > > > --- a/drivers/clk/clk.c
> > > > +++ b/drivers/clk/clk.c
> > > > @@ -937,6 +937,21 @@ void clk_rate_exclusive_get(struct clk *clk)
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(clk_rate_exclusive_get);
> > > >  
> > > > +static void devm_clk_rate_exclusive_put(void *data)
> > > > +{
> > > > +       struct clk *clk = data;
> > > > +
> > > > +       clk_rate_exclusive_put(clk);
> > > > +}
> > > > +
> > > > +int devm_clk_rate_exclusive_get(struct device *dev, struct clk *clk)
> > > > +{
> > > > +       clk_rate_exclusive_get(clk);
> > > 
> > > It seems the other thread wants this to return an error value.
> > 
> > The status quo is that clk_rate_exclusive_get() always returns zero.
> > Some users do error handling (which is dead code until Maxime reworks
> > the call that it might return something non-zero), others just call it
> > without checking.
> > 
> > If you don't require to add something like:
> > 
> >       ret = clk_rate_exclusive_get(clk);
> >       if (ret)
> >               return ret;
> > 
> > where we currently have just
> > 
> >       clk_rate_exclusive_get(clk);
> > 
> > the patch can just be applied (using git am -3) not even hitting a merge
> > conflict without that other series.
> 
> I wonder what you think about this. This devm_clk_rate_exclusive_get()
> would be very useful and simplify a few more drivers.
> 
> Do you intend to take the patch as is, or should I rework it to check
> for the zero it returns?
> 

Please check the return value even if it is always zero. The discussion
about handling the return value can continue in parallel.

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Boyd <sboyd@kernel.org>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: linux-pwm@vger.kernel.org, Sean Anderson <sean.anderson@seco.com>,
	Michael Turquette <mturquette@baylibre.com>,
	Russell King <linux@armlinux.org.uk>,
	Maxime Ripard <mripard@kernel.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	kernel@pengutronix.de, Michal Simek <michal.simek@amd.com>,
	linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] clk: Add a devm variant of clk_rate_exclusive_get()
Date: Thu, 04 Jan 2024 13:38:27 -0800	[thread overview]
Message-ID: <c1effbda6f323aa58935e1990ba3aed8.sboyd@kernel.org> (raw)
In-Reply-To: <g5ahts576gcub7iwn3xsaky3yu7cqdh3szu67ovixmrrci7zq6@t5fjhj6as5vk>

Quoting Uwe Kleine-König (2024-01-04 10:06:29)
> Hello Stephen,
> 
> On Mon, Dec 18, 2023 at 02:01:41PM +0100, Uwe Kleine-K�nig wrote:
> > [Cc += Maxime]
> > 
> > Hello Stephen,
> > 
> > On Sun, Dec 17, 2023 at 04:17:41PM -0800, Stephen Boyd wrote:
> > > Quoting Uwe Kleine-K�nig (2023-12-12 10:09:42)
> > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > > > index af2011c2a93b..78249ca2341c 100644
> > > > --- a/drivers/clk/clk.c
> > > > +++ b/drivers/clk/clk.c
> > > > @@ -937,6 +937,21 @@ void clk_rate_exclusive_get(struct clk *clk)
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(clk_rate_exclusive_get);
> > > >  
> > > > +static void devm_clk_rate_exclusive_put(void *data)
> > > > +{
> > > > +       struct clk *clk = data;
> > > > +
> > > > +       clk_rate_exclusive_put(clk);
> > > > +}
> > > > +
> > > > +int devm_clk_rate_exclusive_get(struct device *dev, struct clk *clk)
> > > > +{
> > > > +       clk_rate_exclusive_get(clk);
> > > 
> > > It seems the other thread wants this to return an error value.
> > 
> > The status quo is that clk_rate_exclusive_get() always returns zero.
> > Some users do error handling (which is dead code until Maxime reworks
> > the call that it might return something non-zero), others just call it
> > without checking.
> > 
> > If you don't require to add something like:
> > 
> >       ret = clk_rate_exclusive_get(clk);
> >       if (ret)
> >               return ret;
> > 
> > where we currently have just
> > 
> >       clk_rate_exclusive_get(clk);
> > 
> > the patch can just be applied (using git am -3) not even hitting a merge
> > conflict without that other series.
> 
> I wonder what you think about this. This devm_clk_rate_exclusive_get()
> would be very useful and simplify a few more drivers.
> 
> Do you intend to take the patch as is, or should I rework it to check
> for the zero it returns?
> 

Please check the return value even if it is always zero. The discussion
about handling the return value can continue in parallel.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2024-01-04 21:38 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-12 18:09 [PATCH 0/2] clk: Add a devm variant of clk_rate_exclusive_get() Uwe Kleine-König
2023-12-12 18:09 ` Uwe Kleine-König
2023-12-12 18:09 ` [PATCH 1/2] " Uwe Kleine-König
2023-12-12 18:09   ` Uwe Kleine-König
2023-12-18  0:17   ` Stephen Boyd
2023-12-18  0:17     ` Stephen Boyd
2023-12-18 13:01     ` Uwe Kleine-König
2023-12-18 13:01       ` Uwe Kleine-König
2024-01-04 18:06       ` Uwe Kleine-König
2024-01-04 18:06         ` Uwe Kleine-König
2024-01-04 21:38         ` Stephen Boyd [this message]
2024-01-04 21:38           ` Stephen Boyd
2024-01-04 23:01           ` Uwe Kleine-König
2024-01-04 23:01             ` Uwe Kleine-König
2023-12-12 18:09 ` [PATCH 2/2] pwm: xilinx: Simplify using devm functions Uwe Kleine-König
2023-12-12 18:09   ` Uwe Kleine-König

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=c1effbda6f323aa58935e1990ba3aed8.sboyd@kernel.org \
    --to=sboyd@kernel.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=michal.simek@amd.com \
    --cc=mripard@kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=sean.anderson@seco.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.