All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: jarkko.nikula@linux.intel.com, mika.westerberg@linux.intel.com,
	jsd@semihalf.com, wsa@kernel.org, hdegoede@redhat.com,
	linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
	patches@opensource.cirrus.com
Subject: Re: [PATCH v3] i2c: designware: Fix unbalanced suspended flag
Date: Mon, 19 Dec 2022 14:47:44 +0200	[thread overview]
Message-ID: <Y6BdcEHydaa1gLBI@smile.fi.intel.com> (raw)
In-Reply-To: <20221219112019.882092-1-rf@opensource.cirrus.com>

On Mon, Dec 19, 2022 at 11:20:19AM +0000, Richard Fitzgerald wrote:
> Ensure that i2c_mark_adapter_suspended() is always balanced by a call to
> i2c_mark_adapter_resumed().
> 
> dw_i2c_plat_resume() must always be called, so that
> i2c_mark_adapter_resumed() is called. This is not compatible with
> DPM_FLAG_MAY_SKIP_RESUME, so remove the flag.
> 
> Since the controller is always resumed on system resume the
> dw_i2c_plat_complete() callback is redundant and has been removed.

> The unbalanced suspended flag was introduced by
> commit c57813b8b288 ("i2c: designware: Lock the adapter while setting the
> suspended flag")

Seems like indentation got broken - the commit and hash can be on previous
line, but it's minor thingy here.

> Before that commit, the system and runtime PM used the same functions. The
> DPM_FLAG_MAY_SKIP_RESUME was used to skip the system resume if the driver
> had been in runtime-suspend. If system resume was skipped, the suspended
> flag would be cleared by the next runtime resume. The check of the
> suspended flag was _after_ the call to pm_runtime_get_sync() in
> i2c_dw_xfer(). So either a system resume or a runtime resume would clear
> the flag before it was checked.
> 
> Having introduced the unbalanced suspended flag with that commit, a further
> commit 80704a84a9f8 ("i2c: designware: Use the
> i2c_mark_adapter_suspended/resumed() helpers")
> 
> changed from using a local suspended flag to using the
> i2c_mark_adapter_suspended/resumed() functions. These use a flag that is
> checked by I2C core code before issuing the transfer to the bus driver, so
> there was no opportunity for the bus driver to runtime resume itself before
> the flag check.

> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> Fixes: c57813b8b288 ("i2c: designware: Lock the adapter while setting the suspended flag")

Usually we put other tags, that submitter brought themselves, before SoB.
But again, it's minor thingy.

-- 
With Best Regards,
Andy Shevchenko



      parent reply	other threads:[~2022-12-19 12:47 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-19 11:20 [PATCH v3] i2c: designware: Fix unbalanced suspended flag Richard Fitzgerald
2022-12-19 11:35 ` Hans de Goede
2022-12-19 12:47 ` Andy Shevchenko [this message]

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=Y6BdcEHydaa1gLBI@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=hdegoede@redhat.com \
    --cc=jarkko.nikula@linux.intel.com \
    --cc=jsd@semihalf.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=patches@opensource.cirrus.com \
    --cc=rf@opensource.cirrus.com \
    --cc=wsa@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 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.