From: Vinod Koul <vkoul@kernel.org> To: Bard Liao <yung-chuan.liao@linux.intel.com> Cc: alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, tiwai@suse.de, broonie@kernel.org, gregkh@linuxfoundation.org, jank@cadence.com, srinivas.kandagatla@linaro.org, rander.wang@linux.intel.com, ranjani.sridharan@linux.intel.com, hui.wang@canonical.com, pierre-louis.bossart@linux.intel.com, sanyog.r.kale@intel.com, bard.liao@intel.com Subject: Re: [PATCH 4/7] soundwire/regmap: use _no_pm functions in regmap_read/write Date: Sat, 5 Dec 2020 13:15:54 +0530 [thread overview] Message-ID: <20201205074554.GR8403@vkoul-mobl> (raw) In-Reply-To: <20201202204645.23891-5-yung-chuan.liao@linux.intel.com> On 03-12-20, 04:46, Bard Liao wrote: > sdw_update_slave_status will be invoked when a codec is attached, > and the codec driver will initialize the codec with regmap functions > while the codec device is pm_runtime suspended. > > regmap routines currently rely on regular SoundWire IO functions, > which will call pm_runtime_get_sync()/put_autosuspend. > > This causes a deadlock where the resume routine waits for an > initialization complete signal that while the initialization complete > can only be reached when the resume completes. > > The only solution if we allow regmap functions to be used in resume > operations as well as during codec initialization is to use _no_pm > routines. The duty of making sure the bus is operational needs to be > handled above the regmap level. > > Fixes: 7c22ce6e21840 ('regmap: Add SoundWire bus support') > Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> > Reviewed-by: Rander Wang <rander.wang@linux.intel.com> > --- > drivers/base/regmap/regmap-sdw.c | 4 ++-- > drivers/soundwire/bus.c | 6 ++++-- > include/linux/soundwire/sdw.h | 2 ++ > 3 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/base/regmap/regmap-sdw.c b/drivers/base/regmap/regmap-sdw.c > index c92d614b4943..4b8d2d010cab 100644 > --- a/drivers/base/regmap/regmap-sdw.c > +++ b/drivers/base/regmap/regmap-sdw.c > @@ -11,7 +11,7 @@ static int regmap_sdw_write(void *context, unsigned int reg, unsigned int val) > struct device *dev = context; > struct sdw_slave *slave = dev_to_sdw_dev(dev); > > - return sdw_write(slave, reg, val); > + return sdw_write_no_pm(slave, reg, val); > } > > static int regmap_sdw_read(void *context, unsigned int reg, unsigned int *val) > @@ -20,7 +20,7 @@ static int regmap_sdw_read(void *context, unsigned int reg, unsigned int *val) > struct sdw_slave *slave = dev_to_sdw_dev(dev); > int read; > > - read = sdw_read(slave, reg); > + read = sdw_read_no_pm(slave, reg); > if (read < 0) > return read; > > diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c > index 86c339d77a39..c5ea59673dee 100644 > --- a/drivers/soundwire/bus.c > +++ b/drivers/soundwire/bus.c > @@ -405,10 +405,11 @@ sdw_nwrite_no_pm(struct sdw_slave *slave, u32 addr, size_t count, u8 *val) > return sdw_transfer(slave->bus, &msg); > } > > -static int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value) > +int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value) > { > return sdw_nwrite_no_pm(slave, addr, 1, &value); > } > +EXPORT_SYMBOL(sdw_write_no_pm); Why not export this is patch 1..? > > static int > sdw_bread_no_pm(struct sdw_bus *bus, u16 dev_num, u32 addr) > @@ -476,7 +477,7 @@ int sdw_bwrite_no_pm_unlocked(struct sdw_bus *bus, u16 dev_num, u32 addr, u8 val > } > EXPORT_SYMBOL(sdw_bwrite_no_pm_unlocked); > > -static int > +int > sdw_read_no_pm(struct sdw_slave *slave, u32 addr) > { > u8 buf; > @@ -488,6 +489,7 @@ sdw_read_no_pm(struct sdw_slave *slave, u32 addr) > else > return buf; > } > +EXPORT_SYMBOL(sdw_read_no_pm); > > static int sdw_update_no_pm(struct sdw_slave *slave, u32 addr, u8 mask, u8 val) > { > diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h > index f0b01b728640..d08039d65825 100644 > --- a/include/linux/soundwire/sdw.h > +++ b/include/linux/soundwire/sdw.h > @@ -1005,6 +1005,8 @@ int sdw_bus_exit_clk_stop(struct sdw_bus *bus); > > int sdw_read(struct sdw_slave *slave, u32 addr); > int sdw_write(struct sdw_slave *slave, u32 addr, u8 value); > +int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value); > +int sdw_read_no_pm(struct sdw_slave *slave, u32 addr); > int sdw_nread(struct sdw_slave *slave, u32 addr, size_t count, u8 *val); > int sdw_nwrite(struct sdw_slave *slave, u32 addr, size_t count, u8 *val); > > -- > 2.17.1 -- ~Vinod
WARNING: multiple messages have this Message-ID (diff)
From: Vinod Koul <vkoul@kernel.org> To: Bard Liao <yung-chuan.liao@linux.intel.com> Cc: pierre-louis.bossart@linux.intel.com, alsa-devel@alsa-project.org, tiwai@suse.de, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, ranjani.sridharan@linux.intel.com, hui.wang@canonical.com, broonie@kernel.org, srinivas.kandagatla@linaro.org, jank@cadence.com, sanyog.r.kale@intel.com, rander.wang@linux.intel.com, bard.liao@intel.com Subject: Re: [PATCH 4/7] soundwire/regmap: use _no_pm functions in regmap_read/write Date: Sat, 5 Dec 2020 13:15:54 +0530 [thread overview] Message-ID: <20201205074554.GR8403@vkoul-mobl> (raw) In-Reply-To: <20201202204645.23891-5-yung-chuan.liao@linux.intel.com> On 03-12-20, 04:46, Bard Liao wrote: > sdw_update_slave_status will be invoked when a codec is attached, > and the codec driver will initialize the codec with regmap functions > while the codec device is pm_runtime suspended. > > regmap routines currently rely on regular SoundWire IO functions, > which will call pm_runtime_get_sync()/put_autosuspend. > > This causes a deadlock where the resume routine waits for an > initialization complete signal that while the initialization complete > can only be reached when the resume completes. > > The only solution if we allow regmap functions to be used in resume > operations as well as during codec initialization is to use _no_pm > routines. The duty of making sure the bus is operational needs to be > handled above the regmap level. > > Fixes: 7c22ce6e21840 ('regmap: Add SoundWire bus support') > Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> > Reviewed-by: Rander Wang <rander.wang@linux.intel.com> > --- > drivers/base/regmap/regmap-sdw.c | 4 ++-- > drivers/soundwire/bus.c | 6 ++++-- > include/linux/soundwire/sdw.h | 2 ++ > 3 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/base/regmap/regmap-sdw.c b/drivers/base/regmap/regmap-sdw.c > index c92d614b4943..4b8d2d010cab 100644 > --- a/drivers/base/regmap/regmap-sdw.c > +++ b/drivers/base/regmap/regmap-sdw.c > @@ -11,7 +11,7 @@ static int regmap_sdw_write(void *context, unsigned int reg, unsigned int val) > struct device *dev = context; > struct sdw_slave *slave = dev_to_sdw_dev(dev); > > - return sdw_write(slave, reg, val); > + return sdw_write_no_pm(slave, reg, val); > } > > static int regmap_sdw_read(void *context, unsigned int reg, unsigned int *val) > @@ -20,7 +20,7 @@ static int regmap_sdw_read(void *context, unsigned int reg, unsigned int *val) > struct sdw_slave *slave = dev_to_sdw_dev(dev); > int read; > > - read = sdw_read(slave, reg); > + read = sdw_read_no_pm(slave, reg); > if (read < 0) > return read; > > diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c > index 86c339d77a39..c5ea59673dee 100644 > --- a/drivers/soundwire/bus.c > +++ b/drivers/soundwire/bus.c > @@ -405,10 +405,11 @@ sdw_nwrite_no_pm(struct sdw_slave *slave, u32 addr, size_t count, u8 *val) > return sdw_transfer(slave->bus, &msg); > } > > -static int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value) > +int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value) > { > return sdw_nwrite_no_pm(slave, addr, 1, &value); > } > +EXPORT_SYMBOL(sdw_write_no_pm); Why not export this is patch 1..? > > static int > sdw_bread_no_pm(struct sdw_bus *bus, u16 dev_num, u32 addr) > @@ -476,7 +477,7 @@ int sdw_bwrite_no_pm_unlocked(struct sdw_bus *bus, u16 dev_num, u32 addr, u8 val > } > EXPORT_SYMBOL(sdw_bwrite_no_pm_unlocked); > > -static int > +int > sdw_read_no_pm(struct sdw_slave *slave, u32 addr) > { > u8 buf; > @@ -488,6 +489,7 @@ sdw_read_no_pm(struct sdw_slave *slave, u32 addr) > else > return buf; > } > +EXPORT_SYMBOL(sdw_read_no_pm); > > static int sdw_update_no_pm(struct sdw_slave *slave, u32 addr, u8 mask, u8 val) > { > diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h > index f0b01b728640..d08039d65825 100644 > --- a/include/linux/soundwire/sdw.h > +++ b/include/linux/soundwire/sdw.h > @@ -1005,6 +1005,8 @@ int sdw_bus_exit_clk_stop(struct sdw_bus *bus); > > int sdw_read(struct sdw_slave *slave, u32 addr); > int sdw_write(struct sdw_slave *slave, u32 addr, u8 value); > +int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value); > +int sdw_read_no_pm(struct sdw_slave *slave, u32 addr); > int sdw_nread(struct sdw_slave *slave, u32 addr, size_t count, u8 *val); > int sdw_nwrite(struct sdw_slave *slave, u32 addr, size_t count, u8 *val); > > -- > 2.17.1 -- ~Vinod
next prev parent reply other threads:[~2020-12-05 7:46 UTC|newest] Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-02 20:46 [PATCH 0/7] soundwire/regmap: use _no_pm routines Bard Liao 2020-12-02 20:46 ` Bard Liao 2020-12-02 20:46 ` [PATCH 1/7] soundwire: bus: use sdw_update_no_pm when initializing a device Bard Liao 2020-12-02 20:46 ` Bard Liao 2020-12-05 7:45 ` Vinod Koul 2020-12-05 7:45 ` Vinod Koul 2020-12-05 14:59 ` Pierre-Louis Bossart 2020-12-07 4:43 ` Vinod Koul 2020-12-07 4:43 ` Vinod Koul 2020-12-07 15:31 ` Pierre-Louis Bossart 2020-12-07 15:31 ` Pierre-Louis Bossart 2020-12-08 4:56 ` Vinod Koul 2020-12-08 4:56 ` Vinod Koul 2020-12-02 20:46 ` [PATCH 2/7] soundwire: bus: use sdw_write_no_pm when setting the bus scale registers Bard Liao 2020-12-02 20:46 ` Bard Liao 2020-12-02 20:46 ` [PATCH 3/7] soundwire: bus: use no_pm IO routines for all interrupt handling Bard Liao 2020-12-02 20:46 ` Bard Liao 2020-12-02 20:46 ` [PATCH 4/7] soundwire/regmap: use _no_pm functions in regmap_read/write Bard Liao 2020-12-02 20:46 ` Bard Liao 2020-12-05 7:45 ` Vinod Koul [this message] 2020-12-05 7:45 ` Vinod Koul 2020-12-05 14:43 ` Pierre-Louis Bossart 2020-12-02 20:46 ` [PATCH 5/7] regmap: sdw: use no_pm routines for SoundWire 1.2 MBQ Bard Liao 2020-12-02 20:46 ` Bard Liao 2020-12-05 7:46 ` Vinod Koul 2020-12-05 7:46 ` Vinod Koul 2020-12-05 14:52 ` Pierre-Louis Bossart 2020-12-05 16:31 ` Greg KH 2020-12-05 16:31 ` Greg KH 2020-12-07 4:47 ` Vinod Koul 2020-12-07 4:47 ` Vinod Koul 2020-12-02 20:46 ` [PATCH 6/7] soundwire: bus: fix confusion on device used by pm_runtime Bard Liao 2020-12-02 20:46 ` Bard Liao 2020-12-02 20:46 ` [PATCH 7/7] soundwire: bus: clarify dev_err/dbg device references Bard Liao 2020-12-02 20:46 ` Bard Liao
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=20201205074554.GR8403@vkoul-mobl \ --to=vkoul@kernel.org \ --cc=alsa-devel@alsa-project.org \ --cc=bard.liao@intel.com \ --cc=broonie@kernel.org \ --cc=gregkh@linuxfoundation.org \ --cc=hui.wang@canonical.com \ --cc=jank@cadence.com \ --cc=linux-kernel@vger.kernel.org \ --cc=pierre-louis.bossart@linux.intel.com \ --cc=rander.wang@linux.intel.com \ --cc=ranjani.sridharan@linux.intel.com \ --cc=sanyog.r.kale@intel.com \ --cc=srinivas.kandagatla@linaro.org \ --cc=tiwai@suse.de \ --cc=yung-chuan.liao@linux.intel.com \ /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: linkBe 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.