All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
To: Adrian Hunter
	<adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Thierry Reding
	<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH V2 1/2] mmc: sdhci: Add support for setting parent clock
Date: Fri, 17 Mar 2017 09:25:31 +0000	[thread overview]
Message-ID: <1489742732-7722-1-git-send-email-jonathanh@nvidia.com> (raw)

It is common for SD/MMC host controllers to set the parent clock that
drives the SD/MMC interface in order to support various operating
speeds. Typically, this is performed by calling common clock framework
APIs such as clk_set_rate(). The problem is that these APIs may sleep
and must not be called from within atomic sections and therefore, these
functions cannot be called within the existing 'set_clock' SDHCI
operator because they are called from within the context of a spinlock.
Add a new 'set_parent_clock' operator for the SDHCI driver that is
called early during the SDHCI 'set_ios' before the spinlock is aquired
to give the platform driver the opportunity to set the parent clock
rate.

Please note that the Tegra and MSM SDHCI drivers currently appear to
mis-use the 'set_clock' operator by calling clk_set_rate(). In the case
of Tegra, occasionally but not always, 'scheduling while atomic' errors
are reported (so most of the time we are getting lucky). In the of the
MSM SDHCI driver, it is releasing and re-acquiring the spinlock which is
bad.

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---

Changes since V1:
- Fixed idiotic copy-paste error and testing thoroughly!

 drivers/mmc/host/sdhci.c | 3 +++
 drivers/mmc/host/sdhci.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6fdd7a70f229..8effc28ece15 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1579,6 +1579,9 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	if (ios->power_mode == MMC_POWER_UNDEFINED)
 		return;
 
+	if (host->ops->set_parent_clock)
+		host->ops->set_parent_clock(host, ios->clock);
+
 	spin_lock_irqsave(&host->lock, flags);
 
 	if (host->flags & SDHCI_DEVICE_DEAD) {
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index edf3adfbc213..585fbcdab70c 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -541,6 +541,8 @@ struct sdhci_ops {
 #endif
 
 	void	(*set_clock)(struct sdhci_host *host, unsigned int clock);
+	void	(*set_parent_clock)(struct sdhci_host *host,
+				    unsigned int clock);
 	void	(*set_power)(struct sdhci_host *host, unsigned char mode,
 			     unsigned short vdd);
 
-- 
2.7.4

WARNING: multiple messages have this Message-ID (diff)
From: Jon Hunter <jonathanh@nvidia.com>
To: Adrian Hunter <adrian.hunter@intel.com>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	Ritesh Harjani <riteshh@codeaurora.org>
Cc: <linux-mmc@vger.kernel.org>, <linux-tegra@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, Jon Hunter <jonathanh@nvidia.com>
Subject: [PATCH V2 1/2] mmc: sdhci: Add support for setting parent clock
Date: Fri, 17 Mar 2017 09:25:31 +0000	[thread overview]
Message-ID: <1489742732-7722-1-git-send-email-jonathanh@nvidia.com> (raw)

It is common for SD/MMC host controllers to set the parent clock that
drives the SD/MMC interface in order to support various operating
speeds. Typically, this is performed by calling common clock framework
APIs such as clk_set_rate(). The problem is that these APIs may sleep
and must not be called from within atomic sections and therefore, these
functions cannot be called within the existing 'set_clock' SDHCI
operator because they are called from within the context of a spinlock.
Add a new 'set_parent_clock' operator for the SDHCI driver that is
called early during the SDHCI 'set_ios' before the spinlock is aquired
to give the platform driver the opportunity to set the parent clock
rate.

Please note that the Tegra and MSM SDHCI drivers currently appear to
mis-use the 'set_clock' operator by calling clk_set_rate(). In the case
of Tegra, occasionally but not always, 'scheduling while atomic' errors
are reported (so most of the time we are getting lucky). In the of the
MSM SDHCI driver, it is releasing and re-acquiring the spinlock which is
bad.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---

Changes since V1:
- Fixed idiotic copy-paste error and testing thoroughly!

 drivers/mmc/host/sdhci.c | 3 +++
 drivers/mmc/host/sdhci.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6fdd7a70f229..8effc28ece15 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1579,6 +1579,9 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	if (ios->power_mode == MMC_POWER_UNDEFINED)
 		return;
 
+	if (host->ops->set_parent_clock)
+		host->ops->set_parent_clock(host, ios->clock);
+
 	spin_lock_irqsave(&host->lock, flags);
 
 	if (host->flags & SDHCI_DEVICE_DEAD) {
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index edf3adfbc213..585fbcdab70c 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -541,6 +541,8 @@ struct sdhci_ops {
 #endif
 
 	void	(*set_clock)(struct sdhci_host *host, unsigned int clock);
+	void	(*set_parent_clock)(struct sdhci_host *host,
+				    unsigned int clock);
 	void	(*set_power)(struct sdhci_host *host, unsigned char mode,
 			     unsigned short vdd);
 
-- 
2.7.4

             reply	other threads:[~2017-03-17  9:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-17  9:25 Jon Hunter [this message]
2017-03-17  9:25 ` [PATCH V2 1/2] mmc: sdhci: Add support for setting parent clock Jon Hunter
     [not found] ` <1489742732-7722-1-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2017-03-17  9:25   ` [PATCH V2 2/2] mmc: tegra: Fix setting of Tegra SDHCI module clock Jon Hunter
2017-03-17  9:25     ` Jon Hunter
2017-03-20 16:52     ` Thierry Reding
2017-03-20 16:52 ` [PATCH V2 1/2] mmc: sdhci: Add support for setting parent clock 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=1489742732-7722-1-git-send-email-jonathanh@nvidia.com \
    --to=jonathanh-ddmlm1+adcrqt0dzr+alfa@public.gmane.org \
    --cc=adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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.