public inbox for linux-i2c@vger.kernel.org
 help / color / mirror / Atom feed
From: "Sverdlin, Alexander" <alexander.sverdlin@siemens.com>
To: "alexander.stein@ew.tq-group.com"
	<alexander.stein@ew.tq-group.com>,
	"linux-imx@nxp.com" <linux-imx@nxp.com>
Cc: "kernel@pengutronix.de" <kernel@pengutronix.de>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"festevam@gmail.com" <festevam@gmail.com>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>,
	"aisheng.dong@nxp.com" <aisheng.dong@nxp.com>
Subject: Re: [PATCH v3] i2c: lpi2c: cache peripheral clock rate
Date: Mon, 15 May 2023 13:11:33 +0000	[thread overview]
Message-ID: <40a2af076fb86301dd1ffac60488ae550a083be7.camel@siemens.com> (raw)
In-Reply-To: <9272339.EvYhyI6sBW@steina-w>

Hello Alexander,

On Fri, 2023-04-21 at 15:48 +0200, Alexander Stein wrote:
> > +       lpi2c_imx->clk_change_nb.notifier_call =
> > lpi2c_imx_clk_change_cb;
> > +       ret = devm_clk_notifier_register(&pdev->dev, lpi2c_imx-
> > >clks[0].clk,
> > +                                        &lpi2c_imx-
> > >clk_change_nb);
> > +       if (ret)
> > +               return dev_err_probe(&pdev->dev, ret,
> > +                                    "can't register peripheral
> > clock 
> notifier\n");
> > +       lpi2c_imx->rate_per = clk_get_rate(lpi2c_imx->clks[0].clk);
> > +       if (!lpi2c_imx->rate_per) {
> > +               dev_err(&pdev->dev, "can't get I2C peripheral clock
> rate\n");
> > +               return -EINVAL;
> > +       }
> > +
> 
> I would switch the order of the calls to devm_clk_notifier_register()
> and 
> clk_get_rate(). AFAICS this looks like a possible lost update. The
> notifier 
> might change rate_per before the (old) value from clk_get_rate is
> actually 
> assigned.

just swapping would not be enough I believe, in the case update event
happens between clk_get_rate() and devm_clk_notifier_register(). I'll
think about it...

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

  parent reply	other threads:[~2023-05-15 13:13 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-10 13:08 [PATCH v3] i2c: lpi2c: cache peripheral clock rate A. Sverdlin
2023-04-21 13:48 ` Alexander Stein
2023-04-21 13:59   ` Marc Kleine-Budde
2023-04-24  7:03     ` Alexander Stein
2023-04-30  7:05       ` Wolfram Sang
2023-05-02  6:50         ` Alexander Stein
2023-05-02  7:03           ` Marc Kleine-Budde
2023-04-21 14:10   ` Sverdlin, Alexander
2023-05-15  9:11   ` Alexander Stein
2023-05-15 12:04     ` Sverdlin, Alexander
2023-05-15 12:32       ` Alexander Stein
2023-05-15 12:37         ` Sverdlin, Alexander
2023-05-15 13:11   ` Sverdlin, Alexander [this message]
2023-04-24  8:08 ` Alexander Stein

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=40a2af076fb86301dd1ffac60488ae550a083be7.camel@siemens.com \
    --to=alexander.sverdlin@siemens.com \
    --cc=aisheng.dong@nxp.com \
    --cc=alexander.stein@ew.tq-group.com \
    --cc=festevam@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    /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