* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ulf Hansson @ 2016-11-24 11:37 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Adrian Hunter, linux-mmc@vger.kernel.org, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree@vger.kernel.org, Thomas Petazzoni,
linux-arm-kernel@lists.infradead.org, Ziji Hu, Jack(SH) Zhu,
Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao, Doug Jones,
Shiwu Zhang, Victor Gu, Wei(SOCP)
In-Reply-To: <a05ffd140f4edc02fc3128db8445b2264cf38723.1477911954.git-series.gregory.clement@free-electrons.com>
On 31 October 2016 at 12:09, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> From: Ziji Hu <huziji@marvell.com>
>
> Marvell Xenon eMMC/SD/SDIO Host Controller contains PHY.
> Three types of PHYs are supported.
>
> Add support to multiple types of PHYs init and configuration.
> Add register definitions of PHYs.
>
> Signed-off-by: Hu Ziji <huziji@marvell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> MAINTAINERS | 2 +-
> drivers/mmc/host/Makefile | 2 +-
> drivers/mmc/host/sdhci-xenon-phy.c | 1181 +++++++++++++++++++++++++++++-
> drivers/mmc/host/sdhci-xenon-phy.h | 157 ++++-
> drivers/mmc/host/sdhci-xenon.c | 4 +-
> drivers/mmc/host/sdhci-xenon.h | 17 +-
> 6 files changed, 1361 insertions(+), 2 deletions(-)
> create mode 100644 drivers/mmc/host/sdhci-xenon-phy.c
> create mode 100644 drivers/mmc/host/sdhci-xenon-phy.h
Can you please consider to split this up somehow!? It would make it
easier to review...
Anyway, allow me to provide some initial feedback, particularly around
those things that Adrian and you requested for my input.
[...]
>
> +
> +static int __xenon_emmc_delay_adj_test(struct mmc_card *card)
> +{
> + int err;
> + u8 *ext_csd = NULL;
> +
> + err = mmc_get_ext_csd(card, &ext_csd);
> + kfree(ext_csd);
Why do you read the ext csd here?
> +
> + return err;
> +}
> +
> +static int __xenon_sdio_delay_adj_test(struct mmc_card *card)
> +{
> + struct mmc_command cmd = {0};
> + int err;
> +
> + cmd.opcode = SD_IO_RW_DIRECT;
> + cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
> +
> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
> + if (err)
> + return err;
> +
> + if (cmd.resp[0] & R5_ERROR)
> + return -EIO;
> + if (cmd.resp[0] & R5_FUNCTION_NUMBER)
> + return -EINVAL;
> + if (cmd.resp[0] & R5_OUT_OF_RANGE)
> + return -ERANGE;
> + return 0;
No thanks! MMC/SD/SDIO protocol code belongs in the core.
> +}
> +
> +static int __xenon_sd_delay_adj_test(struct mmc_card *card)
> +{
> + struct mmc_command cmd = {0};
> + int err;
> +
> + cmd.opcode = MMC_SEND_STATUS;
> + cmd.arg = card->rca << 16;
> + cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
> +
> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
> + return err;
No thanks! MMC/SD/SDIO protocol code belongs in the core.
> +}
> +
[...]
> +int xenon_phy_adj(struct sdhci_host *host, struct mmc_ios *ios)
> +{
> + struct mmc_host *mmc = host->mmc;
> + struct mmc_card *card;
> + int ret = 0;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> + if (!host->clock) {
> + priv->clock = 0;
> + return 0;
> + }
> +
> + /*
> + * The timing, frequency or bus width is changed,
> + * better to set eMMC PHY based on current setting
> + * and adjust Xenon SDHC delay.
> + */
> + if ((host->clock == priv->clock) &&
> + (ios->bus_width == priv->bus_width) &&
> + (ios->timing == priv->timing))
> + return 0;
> +
> + xenon_phy_set(host, ios->timing);
> +
> + /* Update the record */
> + priv->bus_width = ios->bus_width;
> + /* Temp stage from HS200 to HS400 */
> + if (((priv->timing == MMC_TIMING_MMC_HS200) &&
> + (ios->timing == MMC_TIMING_MMC_HS)) ||
> + ((ios->timing == MMC_TIMING_MMC_HS) &&
> + (priv->clock > host->clock))) {
> + priv->timing = ios->timing;
> + priv->clock = host->clock;
> + return 0;
> + }
> + /*
> + * Skip temp stages from HS400 t0 HS200:
> + * from 200MHz to 52MHz in HS400
> + * from HS400 to HS DDR in 52MHz
> + * from HS DDR to HS in 52MHz
> + * from HS to HS200 in 52MHz
> + */
> + if (((priv->timing == MMC_TIMING_MMC_HS400) &&
> + ((host->clock == MMC_HIGH_52_MAX_DTR) ||
> + (ios->timing == MMC_TIMING_MMC_DDR52))) ||
> + ((priv->timing == MMC_TIMING_MMC_DDR52) &&
> + (ios->timing == MMC_TIMING_MMC_HS)) ||
> + ((ios->timing == MMC_TIMING_MMC_HS200) &&
> + (ios->clock == MMC_HIGH_52_MAX_DTR))) {
> + priv->timing = ios->timing;
> + priv->clock = host->clock;
> + return 0;
> + }
> + priv->timing = ios->timing;
> + priv->clock = host->clock;
> +
> + /* Legacy mode is a special case */
> + if (ios->timing == MMC_TIMING_LEGACY)
> + return 0;
> +
> + if (mmc->card)
> + card = mmc->card;
> + else
> + /*
> + * Only valid during initialization
> + * before mmc->card is set
> + */
> + card = priv->card_candidate;
> + if (unlikely(!card)) {
> + dev_warn(mmc_dev(mmc), "card is not present\n");
> + return -EINVAL;
> + }
That your host need to hold a copy of the card pointer, tells me that
something is not really correct.
I might be wrong, if this turns out to be a special case, but I doubt
it. Although, if it *is* a special such case, we shall most likely try
to extend the the mmc core layer instead of adding all these hacks in
your host driver.
[...]
Another suggestion of a general improvement; could you perhaps try to
add some brief information about what goes on in function headers.
Perhaps that could help to more easily understand things.
Kind regards
Uffe
^ permalink raw reply
* [PATCH V4 10/10] PM / OPP: Don't assume platform doesn't have regulators
From: Viresh Kumar @ 2016-11-24 11:36 UTC (permalink / raw)
To: Rafael Wysocki, nm, sboyd, Viresh Kumar
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1479986491.git.viresh.kumar@linaro.org>
If the regulators aren't set explicitly by the platform, the OPP core
assumes that the platform doesn't have any regulator and uses the
clk-only callback.
If the platform failed to register a regulator with the core, then this
can turn out to be a dangerous assumption as the OPP core will try to
change clk without changing regulators.
Handle that properly by making sure that the DT didn't have any entries
for supply voltages as well.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/base/power/opp/core.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index d3f4c625b809..337057783e73 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -734,7 +734,20 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
/* Only frequency scaling */
if (!regulators) {
+ unsigned long u_volt = opp->supplies[0].u_volt;
+
rcu_read_unlock();
+
+ /*
+ * DT contained supply ratings? Consider platform failed to set
+ * regulators.
+ */
+ if (unlikely(u_volt)) {
+ dev_err(dev, "%s: Regulator not registered with OPP core\n",
+ __func__);
+ return -EINVAL;
+ }
+
return _generic_set_opp_clk_only(dev, clk, old_freq, freq);
}
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V4 09/10] PM / OPP: Don't WARN on multiple calls to dev_pm_opp_set_regulators()
From: Viresh Kumar @ 2016-11-24 11:36 UTC (permalink / raw)
To: Rafael Wysocki, nm, sboyd, Viresh Kumar
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1479986491.git.viresh.kumar@linaro.org>
If a platform specific OPP driver has called this routine first and set
the regulators, then the second call from cpufreq-dt driver will hit the
WARN_ON(). Remove the WARN_ON(), but continue to return error in such
cases.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
---
drivers/base/power/opp/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 7c945d5950bf..d3f4c625b809 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -1469,7 +1469,7 @@ int dev_pm_opp_set_regulators(struct device *dev, const char * const names[],
}
/* Already have regulators set */
- if (WARN_ON(opp_table->regulators)) {
+ if (opp_table->regulators) {
ret = -EBUSY;
goto err;
}
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V4 08/10] PM / OPP: Allow platform specific custom set_opp() callbacks
From: Viresh Kumar @ 2016-11-24 11:36 UTC (permalink / raw)
To: Rafael Wysocki, nm, sboyd, Viresh Kumar
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1479986491.git.viresh.kumar@linaro.org>
The generic set_opp() handler isn't sufficient for platforms with
complex DVFS. For example, some TI platforms have multiple regulators
for a CPU device. The order in which various supplies need to be
programmed is only known to the platform code and its best to leave it
to it.
This patch implements APIs to register platform specific set_opp()
callback.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
---
drivers/base/power/opp/core.c | 116 +++++++++++++++++++++++++++++++++++++++++-
drivers/base/power/opp/opp.h | 2 +
include/linux/pm_opp.h | 10 ++++
3 files changed, 126 insertions(+), 2 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 99491f4099e5..7c945d5950bf 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -673,6 +673,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
{
struct opp_table *opp_table;
unsigned long freq, old_freq;
+ int (*set_opp)(struct device *dev, struct dev_pm_set_opp_data *data);
struct dev_pm_opp *old_opp, *opp;
struct regulator **regulators;
struct dev_pm_set_opp_data *data;
@@ -737,6 +738,11 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
return _generic_set_opp_clk_only(dev, clk, old_freq, freq);
}
+ if (opp_table->set_opp)
+ set_opp = opp_table->set_opp;
+ else
+ set_opp = _generic_set_opp;
+
data = opp_table->set_opp_data;
data->regulators = regulators;
data->regulator_count = opp_table->regulator_count;
@@ -754,7 +760,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
rcu_read_unlock();
- return _generic_set_opp(dev, data);
+ return set_opp(dev, data);
}
EXPORT_SYMBOL_GPL(dev_pm_opp_set_rate);
@@ -888,6 +894,9 @@ static void _remove_opp_table(struct opp_table *opp_table)
if (opp_table->regulators)
return;
+ if (opp_table->set_opp)
+ return;
+
/* Release clk */
if (!IS_ERR(opp_table->clk))
clk_put(opp_table->clk);
@@ -1488,7 +1497,7 @@ int dev_pm_opp_set_regulators(struct device *dev, const char * const names[],
opp_table->regulator_count = count;
- /* Allocate block only once to pass to ->set_rate() */
+ /* Allocate block only once to pass to ->set_opp() */
ret = _allocate_set_opp_data(opp_table);
if (ret)
goto free_regulators;
@@ -1563,6 +1572,109 @@ void dev_pm_opp_put_regulators(struct device *dev)
EXPORT_SYMBOL_GPL(dev_pm_opp_put_regulators);
/**
+ * dev_pm_opp_register_set_opp_helper() - Register custom OPP set rate helper
+ * @dev: Device for which the helper is getting registered.
+ * @set_opp: Custom set OPP helper.
+ *
+ * This is useful to support complex platforms (like platforms with multiple
+ * regulators per device), instead of the generic OPP set rate helper.
+ *
+ * This must be called before any OPPs are initialized for the device.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+int dev_pm_opp_register_set_opp_helper(struct device *dev,
+ int (*set_opp)(struct device *dev, struct dev_pm_set_opp_data *data))
+{
+ struct opp_table *opp_table;
+ int ret;
+
+ if (!set_opp)
+ return -EINVAL;
+
+ mutex_lock(&opp_table_lock);
+
+ opp_table = _add_opp_table(dev);
+ if (!opp_table) {
+ ret = -ENOMEM;
+ goto unlock;
+ }
+
+ /* This should be called before OPPs are initialized */
+ if (WARN_ON(!list_empty(&opp_table->opp_list))) {
+ ret = -EBUSY;
+ goto err;
+ }
+
+ /* Already have custom set_opp helper */
+ if (WARN_ON(opp_table->set_opp)) {
+ ret = -EBUSY;
+ goto err;
+ }
+
+ opp_table->set_opp = set_opp;
+
+ mutex_unlock(&opp_table_lock);
+ return 0;
+
+err:
+ _remove_opp_table(opp_table);
+unlock:
+ mutex_unlock(&opp_table_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_register_set_opp_helper);
+
+/**
+ * dev_pm_opp_register_put_opp_helper() - Releases resources blocked for
+ * set_opp helper
+ * @dev: Device for which custom set_opp helper has to be cleared.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+void dev_pm_opp_register_put_opp_helper(struct device *dev)
+{
+ struct opp_table *opp_table;
+
+ mutex_lock(&opp_table_lock);
+
+ /* Check for existing table for 'dev' first */
+ opp_table = _find_opp_table(dev);
+ if (IS_ERR(opp_table)) {
+ dev_err(dev, "Failed to find opp_table: %ld\n",
+ PTR_ERR(opp_table));
+ goto unlock;
+ }
+
+ if (!opp_table->set_opp) {
+ dev_err(dev, "%s: Doesn't have custom set_opp helper set\n",
+ __func__);
+ goto unlock;
+ }
+
+ /* Make sure there are no concurrent readers while updating opp_table */
+ WARN_ON(!list_empty(&opp_table->opp_list));
+
+ opp_table->set_opp = NULL;
+
+ /* Try freeing opp_table if this was the last blocking resource */
+ _remove_opp_table(opp_table);
+
+unlock:
+ mutex_unlock(&opp_table_lock);
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_register_put_opp_helper);
+
+/**
* dev_pm_opp_add() - Add an OPP table from a table definitions
* @dev: device for which we do this operation
* @freq: Frequency in Hz for this OPP
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index a05e43912c6b..5c038701fe34 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -141,6 +141,7 @@ enum opp_table_access {
* @clk: Device's clock handle
* @regulators: Supply regulators
* @regulator_count: Number of power supply regulators
+ * @set_opp: Platform specific set_opp callback
* @set_opp_data: Data to be passed to set_opp callback
* @dentry: debugfs dentry pointer of the real device directory (not links).
* @dentry_name: Name of the real dentry.
@@ -179,6 +180,7 @@ struct opp_table {
struct regulator **regulators;
unsigned int regulator_count;
+ int (*set_opp)(struct device *dev, struct dev_pm_set_opp_data *data);
struct dev_pm_set_opp_data *set_opp_data;
#ifdef CONFIG_DEBUG_FS
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 2969519bf5f7..cb5bc4747502 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -113,6 +113,8 @@ int dev_pm_opp_set_prop_name(struct device *dev, const char *name);
void dev_pm_opp_put_prop_name(struct device *dev);
int dev_pm_opp_set_regulators(struct device *dev, const char * const names[], unsigned int count);
void dev_pm_opp_put_regulators(struct device *dev);
+int dev_pm_opp_register_set_opp_helper(struct device *dev, int (*set_opp)(struct device *dev, struct dev_pm_set_opp_data *data));
+void dev_pm_opp_register_put_opp_helper(struct device *dev);
int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask);
int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
@@ -212,6 +214,14 @@ static inline int dev_pm_opp_set_supported_hw(struct device *dev,
static inline void dev_pm_opp_put_supported_hw(struct device *dev) {}
+static inline int dev_pm_opp_register_set_opp_helper(struct device *dev,
+ int (*set_opp)(struct device *dev, struct dev_pm_set_opp_data *data))
+{
+ return -ENOTSUPP;
+}
+
+static inline void dev_pm_opp_register_put_opp_helper(struct device *dev) {}
+
static inline int dev_pm_opp_set_prop_name(struct device *dev, const char *name)
{
return -ENOTSUPP;
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V4 07/10] PM / OPP: Separate out _generic_opp_set_rate()
From: Viresh Kumar @ 2016-11-24 11:36 UTC (permalink / raw)
To: Rafael Wysocki, nm, sboyd, Viresh Kumar
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1479986491.git.viresh.kumar@linaro.org>
Later patches would add support for custom opp_set_rate callbacks. This
patch separates out the code for generic opp_set_rate handler in order
to prepare for that.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
---
drivers/base/power/opp/core.c | 180 +++++++++++++++++++++++++++++-------------
drivers/base/power/opp/opp.h | 3 +
include/linux/pm_opp.h | 33 ++++++++
3 files changed, 163 insertions(+), 53 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 2d5c726c920f..99491f4099e5 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -596,6 +596,69 @@ static int _set_opp_voltage(struct device *dev, struct regulator *reg,
return ret;
}
+static inline int
+_generic_set_opp_clk_only(struct device *dev, struct clk *clk,
+ unsigned long old_freq, unsigned long freq)
+{
+ int ret;
+
+ ret = clk_set_rate(clk, freq);
+ if (ret) {
+ dev_err(dev, "%s: failed to set clock rate: %d\n", __func__,
+ ret);
+ }
+
+ return ret;
+}
+
+static int _generic_set_opp(struct device *dev,
+ struct dev_pm_set_opp_data *data)
+{
+ struct dev_pm_opp_supply *old_supply = data->old_opp.supplies;
+ struct dev_pm_opp_supply *new_supply = data->new_opp.supplies;
+ unsigned long old_freq = data->old_opp.rate, freq = data->new_opp.rate;
+ struct regulator *reg = data->regulators[0];
+ int ret;
+
+ /* This function only supports single regulator per device */
+ if (WARN_ON(data->regulator_count > 1)) {
+ dev_err(dev, "multiple regulators are not supported\n");
+ return -EINVAL;
+ }
+
+ /* Scaling up? Scale voltage before frequency */
+ if (freq > old_freq) {
+ ret = _set_opp_voltage(dev, reg, new_supply);
+ if (ret)
+ goto restore_voltage;
+ }
+
+ /* Change frequency */
+ ret = _generic_set_opp_clk_only(dev, data->clk, old_freq, freq);
+ if (ret)
+ goto restore_voltage;
+
+ /* Scaling down? Scale voltage after frequency */
+ if (freq < old_freq) {
+ ret = _set_opp_voltage(dev, reg, new_supply);
+ if (ret)
+ goto restore_freq;
+ }
+
+ return 0;
+
+restore_freq:
+ if (_generic_set_opp_clk_only(dev, data->clk, freq, old_freq))
+ dev_err(dev, "%s: failed to restore old-freq (%lu Hz)\n",
+ __func__, old_freq);
+restore_voltage:
+ /* This shouldn't harm even if the voltages weren't updated earlier */
+ if (old_supply->u_volt)
+ _set_opp_voltage(dev, reg, old_supply);
+
+ return ret;
+}
+
/**
* dev_pm_opp_set_rate() - Configure new OPP based on frequency
* @dev: device for which we do this operation
@@ -609,12 +672,12 @@ static int _set_opp_voltage(struct device *dev, struct regulator *reg,
int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
{
struct opp_table *opp_table;
+ unsigned long freq, old_freq;
struct dev_pm_opp *old_opp, *opp;
- struct regulator *reg = ERR_PTR(-ENXIO);
+ struct regulator **regulators;
+ struct dev_pm_set_opp_data *data;
struct clk *clk;
- unsigned long freq, old_freq;
- struct dev_pm_opp_supply old_supply, new_supply;
- int ret;
+ int ret, size;
if (unlikely(!target_freq)) {
dev_err(dev, "%s: Invalid target frequency %lu\n", __func__,
@@ -663,64 +726,35 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
return ret;
}
- if (opp_table->regulators) {
- /* This function only supports single regulator per device */
- if (WARN_ON(opp_table->regulator_count > 1)) {
- dev_err(dev, "multiple regulators not supported\n");
- rcu_read_unlock();
- return -EINVAL;
- }
+ dev_dbg(dev, "%s: switching OPP: %lu Hz --> %lu Hz\n", __func__,
+ old_freq, freq);
- reg = opp_table->regulators[0];
+ regulators = opp_table->regulators;
+
+ /* Only frequency scaling */
+ if (!regulators) {
+ rcu_read_unlock();
+ return _generic_set_opp_clk_only(dev, clk, old_freq, freq);
}
+ data = opp_table->set_opp_data;
+ data->regulators = regulators;
+ data->regulator_count = opp_table->regulator_count;
+ data->clk = clk;
+
+ data->old_opp.rate = old_freq;
+ size = sizeof(*opp->supplies) * opp_table->regulator_count;
if (IS_ERR(old_opp))
- old_supply.u_volt = 0;
+ memset(data->old_opp.supplies, 0, size);
else
- memcpy(&old_supply, old_opp->supplies, sizeof(old_supply));
+ memcpy(data->old_opp.supplies, old_opp->supplies, size);
- memcpy(&new_supply, opp->supplies, sizeof(new_supply));
+ data->new_opp.rate = freq;
+ memcpy(data->new_opp.supplies, opp->supplies, size);
rcu_read_unlock();
- /* Scaling up? Scale voltage before frequency */
- if (freq > old_freq) {
- ret = _set_opp_voltage(dev, reg, &new_supply);
- if (ret)
- goto restore_voltage;
- }
-
- /* Change frequency */
-
- dev_dbg(dev, "%s: switching OPP: %lu Hz --> %lu Hz\n",
- __func__, old_freq, freq);
-
- ret = clk_set_rate(clk, freq);
- if (ret) {
- dev_err(dev, "%s: failed to set clock rate: %d\n", __func__,
- ret);
- goto restore_voltage;
- }
-
- /* Scaling down? Scale voltage after frequency */
- if (freq < old_freq) {
- ret = _set_opp_voltage(dev, reg, &new_supply);
- if (ret)
- goto restore_freq;
- }
-
- return 0;
-
-restore_freq:
- if (clk_set_rate(clk, old_freq))
- dev_err(dev, "%s: failed to restore old-freq (%lu Hz)\n",
- __func__, old_freq);
-restore_voltage:
- /* This shouldn't harm even if the voltages weren't updated earlier */
- if (old_supply.u_volt)
- _set_opp_voltage(dev, reg, &old_supply);
-
- return ret;
+ return _generic_set_opp(dev, data);
}
EXPORT_SYMBOL_GPL(dev_pm_opp_set_rate);
@@ -1354,6 +1388,38 @@ void dev_pm_opp_put_prop_name(struct device *dev)
}
EXPORT_SYMBOL_GPL(dev_pm_opp_put_prop_name);
+static int _allocate_set_opp_data(struct opp_table *opp_table)
+{
+ struct dev_pm_set_opp_data *data;
+ int len, count = opp_table->regulator_count;
+
+ if (WARN_ON(!count))
+ return -EINVAL;
+
+ /* space for set_opp_data */
+ len = sizeof(*data);
+
+ /* space for old_opp.supplies and new_opp.supplies */
+ len += 2 * sizeof(struct dev_pm_opp_supply) * count;
+
+ data = kzalloc(len, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->old_opp.supplies = (void *)(data + 1);
+ data->new_opp.supplies = data->old_opp.supplies + count;
+
+ opp_table->set_opp_data = data;
+
+ return 0;
+}
+
+static void _free_set_opp_data(struct opp_table *opp_table)
+{
+ kfree(opp_table->set_opp_data);
+ opp_table->set_opp_data = NULL;
+}
+
/**
* dev_pm_opp_set_regulators() - Set regulator names for the device
* @dev: Device for which regulator name is being set.
@@ -1422,6 +1488,11 @@ int dev_pm_opp_set_regulators(struct device *dev, const char * const names[],
opp_table->regulator_count = count;
+ /* Allocate block only once to pass to ->set_rate() */
+ ret = _allocate_set_opp_data(opp_table);
+ if (ret)
+ goto free_regulators;
+
mutex_unlock(&opp_table_lock);
return 0;
@@ -1431,6 +1502,7 @@ int dev_pm_opp_set_regulators(struct device *dev, const char * const names[],
kfree(opp_table->regulators);
opp_table->regulators = NULL;
+ opp_table->regulator_count = 0;
err:
_remove_opp_table(opp_table);
unlock:
@@ -1476,6 +1548,8 @@ void dev_pm_opp_put_regulators(struct device *dev)
for (i = opp_table->regulator_count - 1; i >= 0; i--)
regulator_put(opp_table->regulators[i]);
+ _free_set_opp_data(opp_table);
+
kfree(opp_table->regulators);
opp_table->regulators = NULL;
opp_table->regulator_count = 0;
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 5b0f7e53bede..a05e43912c6b 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -141,6 +141,7 @@ enum opp_table_access {
* @clk: Device's clock handle
* @regulators: Supply regulators
* @regulator_count: Number of power supply regulators
+ * @set_opp_data: Data to be passed to set_opp callback
* @dentry: debugfs dentry pointer of the real device directory (not links).
* @dentry_name: Name of the real dentry.
*
@@ -178,6 +179,8 @@ struct opp_table {
struct regulator **regulators;
unsigned int regulator_count;
+ struct dev_pm_set_opp_data *set_opp_data;
+
#ifdef CONFIG_DEBUG_FS
struct dentry *dentry;
char dentry_name[NAME_MAX];
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 27eea9bfc5ed..2969519bf5f7 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -17,6 +17,8 @@
#include <linux/err.h>
#include <linux/notifier.h>
+struct clk;
+struct regulator;
struct dev_pm_opp;
struct device;
@@ -40,6 +42,37 @@ struct dev_pm_opp_supply {
unsigned long u_amp;
};
+/**
+ * struct dev_pm_opp_info - OPP freq/voltage/current values
+ * @rate: Target clk rate in hz
+ * @supplies: Array of voltage/current values for all power supplies
+ *
+ * This structure stores the freq/voltage/current values for a single OPP.
+ */
+struct dev_pm_opp_info {
+ unsigned long rate;
+ struct dev_pm_opp_supply *supplies;
+};
+
+/**
+ * struct dev_pm_set_opp_data - Set OPP data
+ * @old_opp: Old OPP info
+ * @new_opp: New OPP info
+ * @regulators: Array of regulator pointers
+ * @regulator_count: Number of regulators
+ * @clk: Pointer to clk
+ *
+ * This structure contains all information required for setting an OPP.
+ */
+struct dev_pm_set_opp_data {
+ struct dev_pm_opp_info old_opp;
+ struct dev_pm_opp_info new_opp;
+
+ struct regulator **regulators;
+ unsigned int regulator_count;
+ struct clk *clk;
+};
+
#if defined(CONFIG_PM_OPP)
unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp);
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V4 06/10] PM / OPP: Add infrastructure to manage multiple regulators
From: Viresh Kumar @ 2016-11-24 11:36 UTC (permalink / raw)
To: Rafael Wysocki, nm, sboyd, Viresh Kumar
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1479986491.git.viresh.kumar@linaro.org>
This patch adds infrastructure to manage multiple regulators and updates
the only user (cpufreq-dt) of dev_pm_opp_set{put}_regulator().
This is preparatory work for adding full support for devices with
multiple regulators.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
---
drivers/base/power/opp/core.c | 222 ++++++++++++++++++++++++++-------------
drivers/base/power/opp/debugfs.c | 52 +++++++--
drivers/base/power/opp/of.c | 103 +++++++++++++-----
drivers/base/power/opp/opp.h | 10 +-
drivers/cpufreq/cpufreq-dt.c | 9 +-
include/linux/pm_opp.h | 8 +-
6 files changed, 282 insertions(+), 122 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 37fad2eb0f47..2d5c726c920f 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -93,6 +93,8 @@ struct opp_table *_find_opp_table(struct device *dev)
* Return: voltage in micro volt corresponding to the opp, else
* return 0
*
+ * This is useful only for devices with single power supply.
+ *
* Locking: This function must be called under rcu_read_lock(). opp is a rcu
* protected pointer. This means that opp which could have been fetched by
* opp_find_freq_{exact,ceil,floor} functions is valid as long as we are
@@ -112,7 +114,7 @@ unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
if (IS_ERR_OR_NULL(tmp_opp))
pr_err("%s: Invalid parameters\n", __func__);
else
- v = tmp_opp->supply.u_volt;
+ v = tmp_opp->supplies[0].u_volt;
return v;
}
@@ -222,10 +224,13 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
{
struct opp_table *opp_table;
struct dev_pm_opp *opp;
- struct regulator *reg;
+ struct regulator *reg, **regulators;
unsigned long latency_ns = 0;
- unsigned long min_uV = ~0, max_uV = 0;
- int ret;
+ int ret, size, i, count;
+ struct {
+ unsigned long min;
+ unsigned long max;
+ } *uV;
rcu_read_lock();
@@ -235,21 +240,41 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
return 0;
}
- reg = opp_table->regulator;
- if (IS_ERR(reg)) {
+ count = opp_table->regulator_count;
+
+ if (!count) {
/* Regulator may not be required for device */
rcu_read_unlock();
return 0;
}
- list_for_each_entry_rcu(opp, &opp_table->opp_list, node) {
- if (!opp->available)
- continue;
+ size = count * sizeof(*regulators);
+ regulators = kmemdup(opp_table->regulators, size, GFP_KERNEL);
+ if (!regulators) {
+ rcu_read_unlock();
+ return 0;
+ }
+
+ uV = kmalloc_array(count, sizeof(*uV), GFP_KERNEL);
+ if (!uV) {
+ kfree(regulators);
+ rcu_read_unlock();
+ return 0;
+ }
- if (opp->supply.u_volt_min < min_uV)
- min_uV = opp->supply.u_volt_min;
- if (opp->supply.u_volt_max > max_uV)
- max_uV = opp->supply.u_volt_max;
+ for (i = 0; i < count; i++) {
+ uV[i].min = ~0;
+ uV[i].max = 0;
+
+ list_for_each_entry_rcu(opp, &opp_table->opp_list, node) {
+ if (!opp->available)
+ continue;
+
+ if (opp->supplies[i].u_volt_min < uV[i].min)
+ uV[i].min = opp->supplies[i].u_volt_min;
+ if (opp->supplies[i].u_volt_max > uV[i].max)
+ uV[i].max = opp->supplies[i].u_volt_max;
+ }
}
rcu_read_unlock();
@@ -258,9 +283,14 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
* The caller needs to ensure that opp_table (and hence the regulator)
* isn't freed, while we are executing this routine.
*/
- ret = regulator_set_voltage_time(reg, min_uV, max_uV);
- if (ret > 0)
- latency_ns = ret * 1000;
+ for (i = 0; reg = regulators[i], i < count; i++) {
+ ret = regulator_set_voltage_time(reg, uV[i].min, uV[i].max);
+ if (ret > 0)
+ latency_ns += ret * 1000;
+ }
+
+ kfree(uV);
+ kfree(regulators);
return latency_ns;
}
@@ -580,7 +610,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
{
struct opp_table *opp_table;
struct dev_pm_opp *old_opp, *opp;
- struct regulator *reg;
+ struct regulator *reg = ERR_PTR(-ENXIO);
struct clk *clk;
unsigned long freq, old_freq;
struct dev_pm_opp_supply old_supply, new_supply;
@@ -633,14 +663,23 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
return ret;
}
+ if (opp_table->regulators) {
+ /* This function only supports single regulator per device */
+ if (WARN_ON(opp_table->regulator_count > 1)) {
+ dev_err(dev, "multiple regulators not supported\n");
+ rcu_read_unlock();
+ return -EINVAL;
+ }
+
+ reg = opp_table->regulators[0];
+ }
+
if (IS_ERR(old_opp))
old_supply.u_volt = 0;
else
- memcpy(&old_supply, &old_opp->supply, sizeof(old_supply));
-
- memcpy(&new_supply, &opp->supply, sizeof(new_supply));
+ memcpy(&old_supply, old_opp->supplies, sizeof(old_supply));
- reg = opp_table->regulator;
+ memcpy(&new_supply, opp->supplies, sizeof(new_supply));
rcu_read_unlock();
@@ -764,9 +803,6 @@ static struct opp_table *_add_opp_table(struct device *dev)
_of_init_opp_table(opp_table, dev);
- /* Set regulator to a non-NULL error value */
- opp_table->regulator = ERR_PTR(-ENXIO);
-
/* Find clk for the device */
opp_table->clk = clk_get(dev, NULL);
if (IS_ERR(opp_table->clk)) {
@@ -815,7 +851,7 @@ static void _remove_opp_table(struct opp_table *opp_table)
if (opp_table->prop_name)
return;
- if (!IS_ERR(opp_table->regulator))
+ if (opp_table->regulators)
return;
/* Release clk */
@@ -924,35 +960,50 @@ struct dev_pm_opp *_allocate_opp(struct device *dev,
struct opp_table **opp_table)
{
struct dev_pm_opp *opp;
+ int count, supply_size;
+ struct opp_table *table;
- /* allocate new OPP node */
- opp = kzalloc(sizeof(*opp), GFP_KERNEL);
- if (!opp)
+ table = _add_opp_table(dev);
+ if (!table)
return NULL;
- INIT_LIST_HEAD(&opp->node);
+ /* Allocate space for at least one supply */
+ count = table->regulator_count ? table->regulator_count : 1;
+ supply_size = sizeof(*opp->supplies) * count;
- *opp_table = _add_opp_table(dev);
- if (!*opp_table) {
- kfree(opp);
+ /* allocate new OPP node + and supplies structures */
+ opp = kzalloc(sizeof(*opp) + supply_size, GFP_KERNEL);
+ if (!opp) {
+ kfree(table);
return NULL;
}
+ /* Put the supplies at the end of the OPP structure as an empty array */
+ opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
+ INIT_LIST_HEAD(&opp->node);
+
+ *opp_table = table;
+
return opp;
}
static bool _opp_supported_by_regulators(struct dev_pm_opp *opp,
struct opp_table *opp_table)
{
- struct regulator *reg = opp_table->regulator;
-
- if (!IS_ERR(reg) &&
- !regulator_is_supported_voltage(reg, opp->supply.u_volt_min,
- opp->supply.u_volt_max)) {
- pr_warn("%s: OPP minuV: %lu maxuV: %lu, not supported by regulator\n",
- __func__, opp->supply.u_volt_min,
- opp->supply.u_volt_max);
- return false;
+ struct regulator *reg;
+ int i;
+
+ for (i = 0; i < opp_table->regulator_count; i++) {
+ reg = opp_table->regulators[i];
+
+ if (!regulator_is_supported_voltage(reg,
+ opp->supplies[i].u_volt_min,
+ opp->supplies[i].u_volt_max)) {
+ pr_warn("%s: OPP minuV: %lu maxuV: %lu, not supported by regulator\n",
+ __func__, opp->supplies[i].u_volt_min,
+ opp->supplies[i].u_volt_max);
+ return false;
+ }
}
return true;
@@ -984,12 +1035,13 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
/* Duplicate OPPs */
dev_warn(dev, "%s: duplicate OPPs detected. Existing: freq: %lu, volt: %lu, enabled: %d. New: freq: %lu, volt: %lu, enabled: %d\n",
- __func__, opp->rate, opp->supply.u_volt,
- opp->available, new_opp->rate, new_opp->supply.u_volt,
- new_opp->available);
+ __func__, opp->rate, opp->supplies[0].u_volt,
+ opp->available, new_opp->rate,
+ new_opp->supplies[0].u_volt, new_opp->available);
+ /* Should we compare voltages for all regulators here ? */
return opp->available &&
- new_opp->supply.u_volt == opp->supply.u_volt ? 0 : -EEXIST;
+ new_opp->supplies[0].u_volt == opp->supplies[0].u_volt ? 0 : -EEXIST;
}
new_opp->opp_table = opp_table;
@@ -1056,9 +1108,9 @@ int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
/* populate the opp table */
new_opp->rate = freq;
tol = u_volt * opp_table->voltage_tolerance_v1 / 100;
- new_opp->supply.u_volt = u_volt;
- new_opp->supply.u_volt_min = u_volt - tol;
- new_opp->supply.u_volt_max = u_volt + tol;
+ new_opp->supplies[0].u_volt = u_volt;
+ new_opp->supplies[0].u_volt_min = u_volt - tol;
+ new_opp->supplies[0].u_volt_max = u_volt + tol;
new_opp->available = true;
new_opp->dynamic = dynamic;
@@ -1303,12 +1355,14 @@ void dev_pm_opp_put_prop_name(struct device *dev)
EXPORT_SYMBOL_GPL(dev_pm_opp_put_prop_name);
/**
- * dev_pm_opp_set_regulator() - Set regulator name for the device
+ * dev_pm_opp_set_regulators() - Set regulator names for the device
* @dev: Device for which regulator name is being set.
- * @name: Name of the regulator.
+ * @names: Array of pointers to the names of the regulator.
+ * @count: Number of regulators.
*
* In order to support OPP switching, OPP layer needs to know the name of the
- * device's regulator, as the core would be required to switch voltages as well.
+ * device's regulators, as the core would be required to switch voltages as
+ * well.
*
* This must be called before any OPPs are initialized for the device.
*
@@ -1318,11 +1372,12 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_put_prop_name);
* that this function is *NOT* called under RCU protection or in contexts where
* mutex cannot be locked.
*/
-int dev_pm_opp_set_regulator(struct device *dev, const char *name)
+int dev_pm_opp_set_regulators(struct device *dev, const char * const names[],
+ unsigned int count)
{
struct opp_table *opp_table;
struct regulator *reg;
- int ret;
+ int ret, i;
mutex_lock(&opp_table_lock);
@@ -1338,26 +1393,44 @@ int dev_pm_opp_set_regulator(struct device *dev, const char *name)
goto err;
}
- /* Already have a regulator set */
- if (WARN_ON(!IS_ERR(opp_table->regulator))) {
+ /* Already have regulators set */
+ if (WARN_ON(opp_table->regulators)) {
ret = -EBUSY;
goto err;
}
- /* Allocate the regulator */
- reg = regulator_get_optional(dev, name);
- if (IS_ERR(reg)) {
- ret = PTR_ERR(reg);
- if (ret != -EPROBE_DEFER)
- dev_err(dev, "%s: no regulator (%s) found: %d\n",
- __func__, name, ret);
+
+ opp_table->regulators = kmalloc_array(count,
+ sizeof(*opp_table->regulators),
+ GFP_KERNEL);
+ if (!opp_table->regulators) {
+ ret = -ENOMEM;
goto err;
}
- opp_table->regulator = reg;
+ for (i = 0; i < count; i++) {
+ reg = regulator_get_optional(dev, names[i]);
+ if (IS_ERR(reg)) {
+ ret = PTR_ERR(reg);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "%s: regulator (%s) not found: %d\n",
+ __func__, names[i], ret);
+ goto free_regulators;
+ }
+
+ opp_table->regulators[i] = reg;
+ }
+
+ opp_table->regulator_count = count;
mutex_unlock(&opp_table_lock);
return 0;
+free_regulators:
+ while (i != 0)
+ regulator_put(opp_table->regulators[--i]);
+
+ kfree(opp_table->regulators);
+ opp_table->regulators = NULL;
err:
_remove_opp_table(opp_table);
unlock:
@@ -1365,11 +1438,11 @@ int dev_pm_opp_set_regulator(struct device *dev, const char *name)
return ret;
}
-EXPORT_SYMBOL_GPL(dev_pm_opp_set_regulator);
+EXPORT_SYMBOL_GPL(dev_pm_opp_set_regulators);
/**
- * dev_pm_opp_put_regulator() - Releases resources blocked for regulator
- * @dev: Device for which regulator was set.
+ * dev_pm_opp_put_regulators() - Releases resources blocked for regulators
+ * @dev: Device for which regulators were set.
*
* Locking: The internal opp_table and opp structures are RCU protected.
* Hence this function internally uses RCU updater strategy with mutex locks
@@ -1377,9 +1450,10 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_set_regulator);
* that this function is *NOT* called under RCU protection or in contexts where
* mutex cannot be locked.
*/
-void dev_pm_opp_put_regulator(struct device *dev)
+void dev_pm_opp_put_regulators(struct device *dev)
{
struct opp_table *opp_table;
+ int i;
mutex_lock(&opp_table_lock);
@@ -1391,16 +1465,20 @@ void dev_pm_opp_put_regulator(struct device *dev)
goto unlock;
}
- if (IS_ERR(opp_table->regulator)) {
- dev_err(dev, "%s: Doesn't have regulator set\n", __func__);
+ if (!opp_table->regulators) {
+ dev_err(dev, "%s: Doesn't have regulators set\n", __func__);
goto unlock;
}
/* Make sure there are no concurrent readers while updating opp_table */
WARN_ON(!list_empty(&opp_table->opp_list));
- regulator_put(opp_table->regulator);
- opp_table->regulator = ERR_PTR(-ENXIO);
+ for (i = opp_table->regulator_count - 1; i >= 0; i--)
+ regulator_put(opp_table->regulators[i]);
+
+ kfree(opp_table->regulators);
+ opp_table->regulators = NULL;
+ opp_table->regulator_count = 0;
/* Try freeing opp_table if this was the last blocking resource */
_remove_opp_table(opp_table);
@@ -1408,7 +1486,7 @@ void dev_pm_opp_put_regulator(struct device *dev)
unlock:
mutex_unlock(&opp_table_lock);
}
-EXPORT_SYMBOL_GPL(dev_pm_opp_put_regulator);
+EXPORT_SYMBOL_GPL(dev_pm_opp_put_regulators);
/**
* dev_pm_opp_add() - Add an OPP table from a table definitions
diff --git a/drivers/base/power/opp/debugfs.c b/drivers/base/power/opp/debugfs.c
index c897676ca35f..95f433db4ac7 100644
--- a/drivers/base/power/opp/debugfs.c
+++ b/drivers/base/power/opp/debugfs.c
@@ -15,6 +15,7 @@
#include <linux/err.h>
#include <linux/init.h>
#include <linux/limits.h>
+#include <linux/slab.h>
#include "opp.h"
@@ -34,6 +35,46 @@ void opp_debug_remove_one(struct dev_pm_opp *opp)
debugfs_remove_recursive(opp->dentry);
}
+static bool opp_debug_create_supplies(struct dev_pm_opp *opp,
+ struct opp_table *opp_table,
+ struct dentry *pdentry)
+{
+ struct dentry *d;
+ int i = 0;
+ char *name;
+
+ /* Always create at least supply-0 directory */
+ do {
+ name = kasprintf(GFP_KERNEL, "supply-%d", i);
+
+ /* Create per-opp directory */
+ d = debugfs_create_dir(name, pdentry);
+
+ kfree(name);
+
+ if (!d)
+ return false;
+
+ if (!debugfs_create_ulong("u_volt_target", S_IRUGO, d,
+ &opp->supplies[i].u_volt))
+ return false;
+
+ if (!debugfs_create_ulong("u_volt_min", S_IRUGO, d,
+ &opp->supplies[i].u_volt_min))
+ return false;
+
+ if (!debugfs_create_ulong("u_volt_max", S_IRUGO, d,
+ &opp->supplies[i].u_volt_max))
+ return false;
+
+ if (!debugfs_create_ulong("u_amp", S_IRUGO, d,
+ &opp->supplies[i].u_amp))
+ return false;
+ } while (++i < opp_table->regulator_count);
+
+ return true;
+}
+
int opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)
{
struct dentry *pdentry = opp_table->dentry;
@@ -63,16 +104,7 @@ int opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)
if (!debugfs_create_ulong("rate_hz", S_IRUGO, d, &opp->rate))
return -ENOMEM;
- if (!debugfs_create_ulong("u_volt_target", S_IRUGO, d, &opp->supply.u_volt))
- return -ENOMEM;
-
- if (!debugfs_create_ulong("u_volt_min", S_IRUGO, d, &opp->supply.u_volt_min))
- return -ENOMEM;
-
- if (!debugfs_create_ulong("u_volt_max", S_IRUGO, d, &opp->supply.u_volt_max))
- return -ENOMEM;
-
- if (!debugfs_create_ulong("u_amp", S_IRUGO, d, &opp->supply.u_amp))
+ if (!opp_debug_create_supplies(opp, opp_table, d))
return -ENOMEM;
if (!debugfs_create_ulong("clock_latency_ns", S_IRUGO, d,
diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
index bdf409d42126..3f7d2591b173 100644
--- a/drivers/base/power/opp/of.c
+++ b/drivers/base/power/opp/of.c
@@ -17,6 +17,7 @@
#include <linux/errno.h>
#include <linux/device.h>
#include <linux/of.h>
+#include <linux/slab.h>
#include <linux/export.h>
#include "opp.h"
@@ -101,16 +102,16 @@ static bool _opp_is_supported(struct device *dev, struct opp_table *opp_table,
return true;
}
-/* TODO: Support multiple regulators */
static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
struct opp_table *opp_table)
{
- u32 microvolt[3] = {0};
- u32 val;
- int count, ret;
+ u32 *microvolt, *microamp = NULL;
+ int supplies, vcount, icount, ret, i, j;
struct property *prop = NULL;
char name[NAME_MAX];
+ supplies = opp_table->regulator_count ? opp_table->regulator_count : 1;
+
/* Search for "opp-microvolt-<name>" */
if (opp_table->prop_name) {
snprintf(name, sizeof(name), "opp-microvolt-%s",
@@ -128,34 +129,29 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
return 0;
}
- count = of_property_count_u32_elems(opp->np, name);
- if (count < 0) {
+ vcount = of_property_count_u32_elems(opp->np, name);
+ if (vcount < 0) {
dev_err(dev, "%s: Invalid %s property (%d)\n",
- __func__, name, count);
- return count;
+ __func__, name, vcount);
+ return vcount;
}
- /* There can be one or three elements here */
- if (count != 1 && count != 3) {
- dev_err(dev, "%s: Invalid number of elements in %s property (%d)\n",
- __func__, name, count);
+ /* There can be one or three elements per supply */
+ if (vcount != supplies && vcount != supplies * 3) {
+ dev_err(dev, "%s: Invalid number of elements in %s property (%d) with supplies (%d)\n",
+ __func__, name, vcount, supplies);
return -EINVAL;
}
- ret = of_property_read_u32_array(opp->np, name, microvolt, count);
+ microvolt = kmalloc_array(vcount, sizeof(*microvolt), GFP_KERNEL);
+ if (!microvolt)
+ return -ENOMEM;
+
+ ret = of_property_read_u32_array(opp->np, name, microvolt, vcount);
if (ret) {
dev_err(dev, "%s: error parsing %s: %d\n", __func__, name, ret);
- return -EINVAL;
- }
-
- opp->supply.u_volt = microvolt[0];
-
- if (count == 1) {
- opp->supply.u_volt_min = opp->supply.u_volt;
- opp->supply.u_volt_max = opp->supply.u_volt;
- } else {
- opp->supply.u_volt_min = microvolt[1];
- opp->supply.u_volt_max = microvolt[2];
+ ret = -EINVAL;
+ goto free_microvolt;
}
/* Search for "opp-microamp-<name>" */
@@ -172,10 +168,59 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
prop = of_find_property(opp->np, name, NULL);
}
- if (prop && !of_property_read_u32(opp->np, name, &val))
- opp->supply.u_amp = val;
+ if (prop) {
+ icount = of_property_count_u32_elems(opp->np, name);
+ if (icount < 0) {
+ dev_err(dev, "%s: Invalid %s property (%d)\n", __func__,
+ name, icount);
+ ret = icount;
+ goto free_microvolt;
+ }
- return 0;
+ if (icount != supplies) {
+ dev_err(dev, "%s: Invalid number of elements in %s property (%d) with supplies (%d)\n",
+ __func__, name, icount, supplies);
+ ret = -EINVAL;
+ goto free_microvolt;
+ }
+
+ microamp = kmalloc_array(icount, sizeof(*microamp), GFP_KERNEL);
+ if (!microamp) {
+ ret = -EINVAL;
+ goto free_microvolt;
+ }
+
+ ret = of_property_read_u32_array(opp->np, name, microamp,
+ icount);
+ if (ret) {
+ dev_err(dev, "%s: error parsing %s: %d\n", __func__,
+ name, ret);
+ ret = -EINVAL;
+ goto free_microamp;
+ }
+ }
+
+ for (i = 0, j = 0; i < supplies; i++) {
+ opp->supplies[i].u_volt = microvolt[j++];
+
+ if (vcount == supplies) {
+ opp->supplies[i].u_volt_min = opp->supplies[i].u_volt;
+ opp->supplies[i].u_volt_max = opp->supplies[i].u_volt;
+ } else {
+ opp->supplies[i].u_volt_min = microvolt[j++];
+ opp->supplies[i].u_volt_max = microvolt[j++];
+ }
+
+ if (microamp)
+ opp->supplies[i].u_amp = microamp[i];
+ }
+
+free_microamp:
+ kfree(microamp);
+free_microvolt:
+ kfree(microvolt);
+
+ return ret;
}
/**
@@ -304,8 +349,8 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
pr_debug("%s: turbo:%d rate:%lu uv:%lu uvmin:%lu uvmax:%lu latency:%lu\n",
__func__, new_opp->turbo, new_opp->rate,
- new_opp->supply.u_volt, new_opp->supply.u_volt_min,
- new_opp->supply.u_volt_max, new_opp->clock_latency_ns);
+ new_opp->supplies[0].u_volt, new_opp->supplies[0].u_volt_min,
+ new_opp->supplies[0].u_volt_max, new_opp->clock_latency_ns);
/*
* Notify the changes in the availability of the operable
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 8a02516542c2..5b0f7e53bede 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -61,7 +61,7 @@ extern struct list_head opp_tables;
* @turbo: true if turbo (boost) OPP
* @suspend: true if suspend OPP
* @rate: Frequency in hertz
- * @supply: Power supply voltage/current values
+ * @supplies: Power supplies voltage/current values
* @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's
* frequency from any other OPP's frequency.
* @opp_table: points back to the opp_table struct this opp belongs to
@@ -80,7 +80,7 @@ struct dev_pm_opp {
bool suspend;
unsigned long rate;
- struct dev_pm_opp_supply supply;
+ struct dev_pm_opp_supply *supplies;
unsigned long clock_latency_ns;
@@ -139,7 +139,8 @@ enum opp_table_access {
* @supported_hw_count: Number of elements in supported_hw array.
* @prop_name: A name to postfix to many DT properties, while parsing them.
* @clk: Device's clock handle
- * @regulator: Supply regulator
+ * @regulators: Supply regulators
+ * @regulator_count: Number of power supply regulators
* @dentry: debugfs dentry pointer of the real device directory (not links).
* @dentry_name: Name of the real dentry.
*
@@ -174,7 +175,8 @@ struct opp_table {
unsigned int supported_hw_count;
const char *prop_name;
struct clk *clk;
- struct regulator *regulator;
+ struct regulator **regulators;
+ unsigned int regulator_count;
#ifdef CONFIG_DEBUG_FS
struct dentry *dentry;
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 5c07ae05d69a..15cb26118dc7 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -186,7 +186,10 @@ static int cpufreq_init(struct cpufreq_policy *policy)
*/
name = find_supply_name(cpu_dev);
if (name) {
- ret = dev_pm_opp_set_regulator(cpu_dev, name);
+ const char *names[] = {name};
+
+ ret = dev_pm_opp_set_regulators(cpu_dev, names,
+ ARRAY_SIZE(names));
if (ret) {
dev_err(cpu_dev, "Failed to set regulator for cpu%d: %d\n",
policy->cpu, ret);
@@ -285,7 +288,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
out_free_opp:
dev_pm_opp_of_cpumask_remove_table(policy->cpus);
if (name)
- dev_pm_opp_put_regulator(cpu_dev);
+ dev_pm_opp_put_regulators(cpu_dev);
out_put_clk:
clk_put(cpu_clk);
@@ -300,7 +303,7 @@ static int cpufreq_exit(struct cpufreq_policy *policy)
dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table);
dev_pm_opp_of_cpumask_remove_table(policy->related_cpus);
if (priv->reg_name)
- dev_pm_opp_put_regulator(priv->cpu_dev);
+ dev_pm_opp_put_regulators(priv->cpu_dev);
clk_put(policy->clk);
kfree(priv);
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index f69126e2bb59..27eea9bfc5ed 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -78,8 +78,8 @@ int dev_pm_opp_set_supported_hw(struct device *dev, const u32 *versions,
void dev_pm_opp_put_supported_hw(struct device *dev);
int dev_pm_opp_set_prop_name(struct device *dev, const char *name);
void dev_pm_opp_put_prop_name(struct device *dev);
-int dev_pm_opp_set_regulator(struct device *dev, const char *name);
-void dev_pm_opp_put_regulator(struct device *dev);
+int dev_pm_opp_set_regulators(struct device *dev, const char * const names[], unsigned int count);
+void dev_pm_opp_put_regulators(struct device *dev);
int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask);
int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
@@ -186,12 +186,12 @@ static inline int dev_pm_opp_set_prop_name(struct device *dev, const char *name)
static inline void dev_pm_opp_put_prop_name(struct device *dev) {}
-static inline int dev_pm_opp_set_regulator(struct device *dev, const char *name)
+static inline int dev_pm_opp_set_regulators(struct device *dev, const char *names[], unsigned int count)
{
return -ENOTSUPP;
}
-static inline void dev_pm_opp_put_regulator(struct device *dev) {}
+static inline void dev_pm_opp_put_regulators(struct device *dev) {}
static inline int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
{
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V4 05/10] PM / OPP: Pass struct dev_pm_opp_supply to _set_opp_voltage()
From: Viresh Kumar @ 2016-11-24 11:36 UTC (permalink / raw)
To: Rafael Wysocki, nm, sboyd, Viresh Kumar
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1479986491.git.viresh.kumar@linaro.org>
Pass the entire supply structure instead of all of its fields.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
---
drivers/base/power/opp/core.c | 44 +++++++++++++++++--------------------------
1 file changed, 17 insertions(+), 27 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 8d6006151c9a..37fad2eb0f47 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -542,8 +542,7 @@ static struct clk *_get_opp_clk(struct device *dev)
}
static int _set_opp_voltage(struct device *dev, struct regulator *reg,
- unsigned long u_volt, unsigned long u_volt_min,
- unsigned long u_volt_max)
+ struct dev_pm_opp_supply *supply)
{
int ret;
@@ -554,14 +553,15 @@ static int _set_opp_voltage(struct device *dev, struct regulator *reg,
return 0;
}
- dev_dbg(dev, "%s: voltages (mV): %lu %lu %lu\n", __func__, u_volt_min,
- u_volt, u_volt_max);
+ dev_dbg(dev, "%s: voltages (mV): %lu %lu %lu\n", __func__,
+ supply->u_volt_min, supply->u_volt, supply->u_volt_max);
- ret = regulator_set_voltage_triplet(reg, u_volt_min, u_volt,
- u_volt_max);
+ ret = regulator_set_voltage_triplet(reg, supply->u_volt_min,
+ supply->u_volt, supply->u_volt_max);
if (ret)
dev_err(dev, "%s: failed to set voltage (%lu %lu %lu mV): %d\n",
- __func__, u_volt_min, u_volt, u_volt_max, ret);
+ __func__, supply->u_volt_min, supply->u_volt,
+ supply->u_volt_max, ret);
return ret;
}
@@ -583,8 +583,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
struct regulator *reg;
struct clk *clk;
unsigned long freq, old_freq;
- unsigned long u_volt, u_volt_min, u_volt_max;
- unsigned long old_u_volt, old_u_volt_min, old_u_volt_max;
+ struct dev_pm_opp_supply old_supply, new_supply;
int ret;
if (unlikely(!target_freq)) {
@@ -634,17 +633,12 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
return ret;
}
- if (IS_ERR(old_opp)) {
- old_u_volt = 0;
- } else {
- old_u_volt = old_opp->supply.u_volt;
- old_u_volt_min = old_opp->supply.u_volt_min;
- old_u_volt_max = old_opp->supply.u_volt_max;
- }
+ if (IS_ERR(old_opp))
+ old_supply.u_volt = 0;
+ else
+ memcpy(&old_supply, &old_opp->supply, sizeof(old_supply));
- u_volt = opp->supply.u_volt;
- u_volt_min = opp->supply.u_volt_min;
- u_volt_max = opp->supply.u_volt_max;
+ memcpy(&new_supply, &opp->supply, sizeof(new_supply));
reg = opp_table->regulator;
@@ -652,8 +646,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
/* Scaling up? Scale voltage before frequency */
if (freq > old_freq) {
- ret = _set_opp_voltage(dev, reg, u_volt, u_volt_min,
- u_volt_max);
+ ret = _set_opp_voltage(dev, reg, &new_supply);
if (ret)
goto restore_voltage;
}
@@ -672,8 +665,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
/* Scaling down? Scale voltage after frequency */
if (freq < old_freq) {
- ret = _set_opp_voltage(dev, reg, u_volt, u_volt_min,
- u_volt_max);
+ ret = _set_opp_voltage(dev, reg, &new_supply);
if (ret)
goto restore_freq;
}
@@ -686,10 +678,8 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
__func__, old_freq);
restore_voltage:
/* This shouldn't harm even if the voltages weren't updated earlier */
- if (old_u_volt) {
- _set_opp_voltage(dev, reg, old_u_volt, old_u_volt_min,
- old_u_volt_max);
- }
+ if (old_supply.u_volt)
+ _set_opp_voltage(dev, reg, &old_supply);
return ret;
}
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V4 04/10] PM / OPP: Manage supply's voltage/current in a separate structure
From: Viresh Kumar @ 2016-11-24 11:36 UTC (permalink / raw)
To: Rafael Wysocki, nm, sboyd, Viresh Kumar
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1479986491.git.viresh.kumar@linaro.org>
This is a preparatory step for multiple regulator per device support.
Move the voltage/current variables to a new structure.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
---
drivers/base/power/opp/core.c | 44 +++++++++++++++++++++-------------------
drivers/base/power/opp/debugfs.c | 8 ++++----
drivers/base/power/opp/of.c | 18 ++++++++--------
drivers/base/power/opp/opp.h | 11 +++-------
include/linux/pm_opp.h | 16 +++++++++++++++
5 files changed, 55 insertions(+), 42 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 056527a3fb4e..8d6006151c9a 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -112,7 +112,7 @@ unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
if (IS_ERR_OR_NULL(tmp_opp))
pr_err("%s: Invalid parameters\n", __func__);
else
- v = tmp_opp->u_volt;
+ v = tmp_opp->supply.u_volt;
return v;
}
@@ -246,10 +246,10 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
if (!opp->available)
continue;
- if (opp->u_volt_min < min_uV)
- min_uV = opp->u_volt_min;
- if (opp->u_volt_max > max_uV)
- max_uV = opp->u_volt_max;
+ if (opp->supply.u_volt_min < min_uV)
+ min_uV = opp->supply.u_volt_min;
+ if (opp->supply.u_volt_max > max_uV)
+ max_uV = opp->supply.u_volt_max;
}
rcu_read_unlock();
@@ -637,14 +637,14 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
if (IS_ERR(old_opp)) {
old_u_volt = 0;
} else {
- old_u_volt = old_opp->u_volt;
- old_u_volt_min = old_opp->u_volt_min;
- old_u_volt_max = old_opp->u_volt_max;
+ old_u_volt = old_opp->supply.u_volt;
+ old_u_volt_min = old_opp->supply.u_volt_min;
+ old_u_volt_max = old_opp->supply.u_volt_max;
}
- u_volt = opp->u_volt;
- u_volt_min = opp->u_volt_min;
- u_volt_max = opp->u_volt_max;
+ u_volt = opp->supply.u_volt;
+ u_volt_min = opp->supply.u_volt_min;
+ u_volt_max = opp->supply.u_volt_max;
reg = opp_table->regulator;
@@ -957,10 +957,11 @@ static bool _opp_supported_by_regulators(struct dev_pm_opp *opp,
struct regulator *reg = opp_table->regulator;
if (!IS_ERR(reg) &&
- !regulator_is_supported_voltage(reg, opp->u_volt_min,
- opp->u_volt_max)) {
+ !regulator_is_supported_voltage(reg, opp->supply.u_volt_min,
+ opp->supply.u_volt_max)) {
pr_warn("%s: OPP minuV: %lu maxuV: %lu, not supported by regulator\n",
- __func__, opp->u_volt_min, opp->u_volt_max);
+ __func__, opp->supply.u_volt_min,
+ opp->supply.u_volt_max);
return false;
}
@@ -993,11 +994,12 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
/* Duplicate OPPs */
dev_warn(dev, "%s: duplicate OPPs detected. Existing: freq: %lu, volt: %lu, enabled: %d. New: freq: %lu, volt: %lu, enabled: %d\n",
- __func__, opp->rate, opp->u_volt, opp->available,
- new_opp->rate, new_opp->u_volt, new_opp->available);
+ __func__, opp->rate, opp->supply.u_volt,
+ opp->available, new_opp->rate, new_opp->supply.u_volt,
+ new_opp->available);
- return opp->available && new_opp->u_volt == opp->u_volt ?
- 0 : -EEXIST;
+ return opp->available &&
+ new_opp->supply.u_volt == opp->supply.u_volt ? 0 : -EEXIST;
}
new_opp->opp_table = opp_table;
@@ -1064,9 +1066,9 @@ int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
/* populate the opp table */
new_opp->rate = freq;
tol = u_volt * opp_table->voltage_tolerance_v1 / 100;
- new_opp->u_volt = u_volt;
- new_opp->u_volt_min = u_volt - tol;
- new_opp->u_volt_max = u_volt + tol;
+ new_opp->supply.u_volt = u_volt;
+ new_opp->supply.u_volt_min = u_volt - tol;
+ new_opp->supply.u_volt_max = u_volt + tol;
new_opp->available = true;
new_opp->dynamic = dynamic;
diff --git a/drivers/base/power/opp/debugfs.c b/drivers/base/power/opp/debugfs.c
index ef1ae6b52042..c897676ca35f 100644
--- a/drivers/base/power/opp/debugfs.c
+++ b/drivers/base/power/opp/debugfs.c
@@ -63,16 +63,16 @@ int opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)
if (!debugfs_create_ulong("rate_hz", S_IRUGO, d, &opp->rate))
return -ENOMEM;
- if (!debugfs_create_ulong("u_volt_target", S_IRUGO, d, &opp->u_volt))
+ if (!debugfs_create_ulong("u_volt_target", S_IRUGO, d, &opp->supply.u_volt))
return -ENOMEM;
- if (!debugfs_create_ulong("u_volt_min", S_IRUGO, d, &opp->u_volt_min))
+ if (!debugfs_create_ulong("u_volt_min", S_IRUGO, d, &opp->supply.u_volt_min))
return -ENOMEM;
- if (!debugfs_create_ulong("u_volt_max", S_IRUGO, d, &opp->u_volt_max))
+ if (!debugfs_create_ulong("u_volt_max", S_IRUGO, d, &opp->supply.u_volt_max))
return -ENOMEM;
- if (!debugfs_create_ulong("u_amp", S_IRUGO, d, &opp->u_amp))
+ if (!debugfs_create_ulong("u_amp", S_IRUGO, d, &opp->supply.u_amp))
return -ENOMEM;
if (!debugfs_create_ulong("clock_latency_ns", S_IRUGO, d,
diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
index 5b3755e49731..bdf409d42126 100644
--- a/drivers/base/power/opp/of.c
+++ b/drivers/base/power/opp/of.c
@@ -148,14 +148,14 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
return -EINVAL;
}
- opp->u_volt = microvolt[0];
+ opp->supply.u_volt = microvolt[0];
if (count == 1) {
- opp->u_volt_min = opp->u_volt;
- opp->u_volt_max = opp->u_volt;
+ opp->supply.u_volt_min = opp->supply.u_volt;
+ opp->supply.u_volt_max = opp->supply.u_volt;
} else {
- opp->u_volt_min = microvolt[1];
- opp->u_volt_max = microvolt[2];
+ opp->supply.u_volt_min = microvolt[1];
+ opp->supply.u_volt_max = microvolt[2];
}
/* Search for "opp-microamp-<name>" */
@@ -173,7 +173,7 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
}
if (prop && !of_property_read_u32(opp->np, name, &val))
- opp->u_amp = val;
+ opp->supply.u_amp = val;
return 0;
}
@@ -303,9 +303,9 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
mutex_unlock(&opp_table_lock);
pr_debug("%s: turbo:%d rate:%lu uv:%lu uvmin:%lu uvmax:%lu latency:%lu\n",
- __func__, new_opp->turbo, new_opp->rate, new_opp->u_volt,
- new_opp->u_volt_min, new_opp->u_volt_max,
- new_opp->clock_latency_ns);
+ __func__, new_opp->turbo, new_opp->rate,
+ new_opp->supply.u_volt, new_opp->supply.u_volt_min,
+ new_opp->supply.u_volt_max, new_opp->clock_latency_ns);
/*
* Notify the changes in the availability of the operable
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 96cd30ac6c1d..8a02516542c2 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -61,10 +61,7 @@ extern struct list_head opp_tables;
* @turbo: true if turbo (boost) OPP
* @suspend: true if suspend OPP
* @rate: Frequency in hertz
- * @u_volt: Target voltage in microvolts corresponding to this OPP
- * @u_volt_min: Minimum voltage in microvolts corresponding to this OPP
- * @u_volt_max: Maximum voltage in microvolts corresponding to this OPP
- * @u_amp: Maximum current drawn by the device in microamperes
+ * @supply: Power supply voltage/current values
* @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's
* frequency from any other OPP's frequency.
* @opp_table: points back to the opp_table struct this opp belongs to
@@ -83,10 +80,8 @@ struct dev_pm_opp {
bool suspend;
unsigned long rate;
- unsigned long u_volt;
- unsigned long u_volt_min;
- unsigned long u_volt_max;
- unsigned long u_amp;
+ struct dev_pm_opp_supply supply;
+
unsigned long clock_latency_ns;
struct opp_table *opp_table;
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index bca26157f5b6..f69126e2bb59 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -24,6 +24,22 @@ enum dev_pm_opp_event {
OPP_EVENT_ADD, OPP_EVENT_REMOVE, OPP_EVENT_ENABLE, OPP_EVENT_DISABLE,
};
+/**
+ * struct dev_pm_opp_supply - Power supply voltage/current values
+ * @u_volt: Target voltage in microvolts corresponding to this OPP
+ * @u_volt_min: Minimum voltage in microvolts corresponding to this OPP
+ * @u_volt_max: Maximum voltage in microvolts corresponding to this OPP
+ * @u_amp: Maximum current drawn by the device in microamperes
+ *
+ * This structure stores the voltage/current values for a single power supply.
+ */
+struct dev_pm_opp_supply {
+ unsigned long u_volt;
+ unsigned long u_volt_min;
+ unsigned long u_volt_max;
+ unsigned long u_amp;
+};
+
#if defined(CONFIG_PM_OPP)
unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp);
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V4 03/10] PM / OPP: Don't use OPP structure outside of rcu protected section
From: Viresh Kumar @ 2016-11-24 11:36 UTC (permalink / raw)
To: Rafael Wysocki, nm, sboyd, Viresh Kumar
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1479986491.git.viresh.kumar@linaro.org>
The OPP structure must not be used out of the rcu protected section.
Cache the values to be used in separate variables instead.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
---
drivers/base/power/opp/core.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 4c7c6da7a989..056527a3fb4e 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -584,6 +584,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
struct clk *clk;
unsigned long freq, old_freq;
unsigned long u_volt, u_volt_min, u_volt_max;
+ unsigned long old_u_volt, old_u_volt_min, old_u_volt_max;
int ret;
if (unlikely(!target_freq)) {
@@ -633,6 +634,14 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
return ret;
}
+ if (IS_ERR(old_opp)) {
+ old_u_volt = 0;
+ } else {
+ old_u_volt = old_opp->u_volt;
+ old_u_volt_min = old_opp->u_volt_min;
+ old_u_volt_max = old_opp->u_volt_max;
+ }
+
u_volt = opp->u_volt;
u_volt_min = opp->u_volt_min;
u_volt_max = opp->u_volt_max;
@@ -677,9 +686,10 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
__func__, old_freq);
restore_voltage:
/* This shouldn't harm even if the voltages weren't updated earlier */
- if (!IS_ERR(old_opp))
- _set_opp_voltage(dev, reg, old_opp->u_volt,
- old_opp->u_volt_min, old_opp->u_volt_max);
+ if (old_u_volt) {
+ _set_opp_voltage(dev, reg, old_u_volt, old_u_volt_min,
+ old_u_volt_max);
+ }
return ret;
}
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V4 02/10] PM / OPP: Reword binding supporting multiple regulators per device
From: Viresh Kumar @ 2016-11-24 11:36 UTC (permalink / raw)
To: Rafael Wysocki, nm, sboyd, Viresh Kumar
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1479986491.git.viresh.kumar@linaro.org>
On certain platforms (like TI), DVFS for a single device (CPU) requires
configuring multiple power supplies.
The OPP bindings already contains binding and example to explain this
case, but it isn't sufficient.
- There is no way for the code parsing these bindings to know which
voltage values belong to which power supply.
- It is not possible to know the order in which the supplies need to be
configured while switching OPPs.
This patch clarifies on those details by mentioning that such
information is left for the implementation specific bindings to explain.
They may want to hardcode such details or implement their own properties
to get such information. All implementations using multiple regulators
for their devices must provide a binding document explaining their
implementation.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
Documentation/devicetree/bindings/opp/opp.txt | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index f0239f68d186..9f5ca4457b5f 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -86,8 +86,14 @@ properties.
Single entry is for target voltage and three entries are for <target min max>
voltages.
- Entries for multiple regulators must be present in the same order as
- regulators are specified in device's DT node.
+ Entries for multiple regulators shall be provided in the same field separated
+ by angular brackets <>. The OPP binding doesn't provide any provisions to
+ relate the values to their power supplies or the order in which the supplies
+ need to be configured and that is left for the implementation specific
+ binding.
+
+ Entries for all regulators shall be of the same size, i.e. either all use a
+ single value or triplets.
- opp-microvolt-<name>: Named opp-microvolt property. This is exactly similar to
the above opp-microvolt property, but allows multiple voltage ranges to be
@@ -104,10 +110,13 @@ properties.
Should only be set if opp-microvolt is set for the OPP.
- Entries for multiple regulators must be present in the same order as
- regulators are specified in device's DT node. If this property isn't required
- for few regulators, then this should be marked as zero for them. If it isn't
- required for any regulator, then this property need not be present.
+ Entries for multiple regulators shall be provided in the same field separated
+ by angular brackets <>. If current values aren't required for a regulator,
+ then it shall be filled with 0. If current values aren't required for any of
+ the regulators, then this field is not required. The OPP binding doesn't
+ provide any provisions to relate the values to their power supplies or the
+ order in which the supplies need to be configured and that is left for the
+ implementation specific binding.
- opp-microamp-<name>: Named opp-microamp property. Similar to
opp-microvolt-<name> property, but for microamp instead.
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V4 01/10] PM / OPP: Fix incorrect cpu-supply property in binding
From: Viresh Kumar @ 2016-11-24 11:36 UTC (permalink / raw)
To: Rafael Wysocki, nm, sboyd, Viresh Kumar
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1479986491.git.viresh.kumar@linaro.org>
The regulator bindings allow the "<name>-supply" property to define a
single parent supply and not a list of parents.
Fix the wrong example code present in OPP bindings.
While at it also change the compatible string as Rob pointed out earlier
that none of A7 implementation have multiple supplies for the CPU core.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
Documentation/devicetree/bindings/opp/opp.txt | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index ee91cbdd95ee..f0239f68d186 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -386,10 +386,12 @@ Example 4: Handling multiple regulators
/ {
cpus {
cpu@0 {
- compatible = "arm,cortex-a7";
+ compatible = "vendor,cpu-type";
...
- cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
+ vcc0-supply = <&cpu_supply0>;
+ vcc1-supply = <&cpu_supply1>;
+ vcc2-supply = <&cpu_supply2>;
operating-points-v2 = <&cpu0_opp_table>;
};
};
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V4 00/10] PM / OPP: Multiple regulator support
From: Viresh Kumar @ 2016-11-24 11:36 UTC (permalink / raw)
To: Rafael Wysocki, nm, sboyd
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
Hi,
Some platforms (like TI) have complex DVFS configuration for CPU
devices, where multiple regulators are required to be configured to
change DVFS state of the device. This was explained well by Nishanth
earlier [1].
One of the major complaints around multiple regulators case was that the
DT isn't responsible in any way to represent the order in which multiple
supplies need to be programmed, before or after a frequency change. It
was considered in this patch and such information is left for the
platform specific OPP driver now, which can register its own
opp_set_rate() callback with the OPP core and the OPP core will then
call it during DVFS.
The patches are tested on Exynos5250 (Dual A15). I have hacked around DT
and code to pass values for multiple regulators and verified that they
are all properly read by the kernel (using debugfs interface).
Dave Gerlach has already tested [2] it on the real TI platforms and it
works well for him.
This is rebased over: linux-next branch in the PM tree.
V3->V4:
- Separate out cpu-supply fix in the binding in a separate patch (Mark).
- Add more documentation to the binding to explain that the relation to
the supplies and the order of programming them is left for the
platform specific bindings and that every platform using multiple
regulators for their devices needs to provide a separate binding
document explaining their implementation (Mark).
- @Rob and Stephen: I have kept your Acks for the bindings as the
bindings only got a bit reworded (improved) since the time you guys
Acked them. Please let me know if you want more improvement in the
bindings now.
- V4 for 10/10 was sent earlier, which added a missing
rcu_read_unlock(). Nothing else changed in it.
- Added some missing Kernel documentation comments
V2->V3:
- The last patch is new
- Removed a debug leftover pr_info() message
- Renamed few names as s/set_rate/set_opp
- Removed a TODO comment (as it is done now with this series)
- created struct for min_uV and max_uV
- kerneldoc comments for structures in pm_opp.h
- s/const char */const char * const
- use kasprintf()
- Some more minor reformatting
- More Ack/RBY tags added
V1->V2:
- Ack from Rob for 1st patch
- Moved the supplies structure to pm_opp.h (Dave)
- Fixed an compilation warning.
--
viresh
[1] https://marc.info/?l=linux-pm&m=145684495832764&w=2
[2] https://marc.info/?l=linux-kernel&m=147924789305276&w=2
Viresh Kumar (10):
PM / OPP: Fix incorrect cpu-supply property in binding
PM / OPP: Reword binding supporting multiple regulators per device
PM / OPP: Don't use OPP structure outside of rcu protected section
PM / OPP: Manage supply's voltage/current in a separate structure
PM / OPP: Pass struct dev_pm_opp_supply to _set_opp_voltage()
PM / OPP: Add infrastructure to manage multiple regulators
PM / OPP: Separate out _generic_opp_set_rate()
PM / OPP: Allow platform specific custom set_opp() callbacks
PM / OPP: Don't WARN on multiple calls to dev_pm_opp_set_regulators()
PM / OPP: Don't assume platform doesn't have regulators
Documentation/devicetree/bindings/opp/opp.txt | 27 +-
drivers/base/power/opp/core.c | 511 ++++++++++++++++++++------
drivers/base/power/opp/debugfs.c | 52 ++-
drivers/base/power/opp/of.c | 105 ++++--
drivers/base/power/opp/opp.h | 22 +-
drivers/cpufreq/cpufreq-dt.c | 9 +-
include/linux/pm_opp.h | 67 +++-
7 files changed, 612 insertions(+), 181 deletions(-)
--
2.7.1.410.g6faf27b
^ permalink raw reply
* [PATCH] ARM: dts: sun6i: hummingbird: Enable USB OTG
From: Chen-Yu Tsai @ 2016-11-24 11:29 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
The A31 Hummingbird has a mini USB OTG port, and uses GPIO pins from the
SoC for ID pin and VBUS detection and VBUS control. The PMIC can also do
VBUS detection and control.
Here we prefer to use the PMIC's DRIVEVBUS function to control VBUS for
USB OTG, as that is the hardware default.
Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
index 83643bbd51dc..f094eeb6c499 100644
--- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
+++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
@@ -248,6 +248,7 @@
reg = <0x68>;
interrupt-parent = <&nmi_intc>;
interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+ x-powers,drive-vbus-en;
};
};
@@ -306,6 +307,11 @@
regulator-name = "vcc-dram";
};
+®_drivevbus {
+ regulator-name = "usb0-vbus";
+ status = "okay";
+};
+
®_usb1_vbus {
gpio = <&pio 7 24 GPIO_ACTIVE_HIGH>; /* PH24 */
status = "okay";
@@ -330,12 +336,25 @@
status = "okay";
};
+&usb_otg {
+ dr_mode = "otg";
+ status = "okay";
+};
+
+&usb_power_supply {
+ status = "okay";
+};
+
&usb1_vbus_pin_a {
/* different pin from sunxi-common-regulators */
pins = "PH24";
};
&usbphy {
+ usb0_id_det-gpio = <&pio 0 15 GPIO_ACTIVE_HIGH>; /* PA15 */
+ usb0_vbus_det-gpio = <&pio 0 16 GPIO_ACTIVE_HIGH>; /* PA16 */
+ usb0_vbus_power-supply = <&usb_power_supply>;
+ usb0_vbus-supply = <®_drivevbus>;
usb1_vbus-supply = <®_usb1_vbus>;
status = "okay";
};
--
2.10.2
^ permalink raw reply related
* Re: [RFC PATCH 2/5] dmaengine: allow sun6i-dma for more SoCs
From: Andre Przywara @ 2016-11-24 11:15 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai
Cc: Icenowy Zheng, linux-sunxi, linux-arm-kernel, Mark Rutland,
Rob Herring, devicetree
In-Reply-To: <20161124105516.tv2ybfgka5teiv2h@lukather>
On 24/11/16 10:55, Maxime Ripard wrote:
> On Thu, Nov 24, 2016 at 05:30:45PM +0800, Chen-Yu Tsai wrote:
>> On Thu, Nov 24, 2016 at 5:16 PM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> wrote:
>>> Hi,
>>>
>>> On 24/11/16 04:16, Chen-Yu Tsai wrote:
>>>> Hi,
>>>>
>>>> On Thu, Nov 24, 2016 at 9:17 AM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> wrote:
>>>>> The sun6i DMA driver is used in the Allwinner A64 and H5 SoC, which
>>>>> have arm64 capable cores. Add the generic sunxi config symbol to allow
>>>>> the driver to be selected by arm64 Kconfigs, which don't feature
>>>>> SoC specific MACH_xxxx configs.
>>>>>
>>>>> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
>>>>> ---
>>>>> drivers/dma/Kconfig | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>>>>> index af63a6b..003c284 100644
>>>>> --- a/drivers/dma/Kconfig
>>>>> +++ b/drivers/dma/Kconfig
>>>>> @@ -157,7 +157,7 @@ config DMA_SUN4I
>>>>>
>>>>> config DMA_SUN6I
>>>>> tristate "Allwinner A31 SoCs DMA support"
>>>>> - depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST
>>>>> + depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST || ARCH_SUNXI
>>>>
>>>> AFAIK ARCH_SUNXI encompasses/supersedes MACH_SUN*I.
>>>> (And I don't have to add MACH_SUN9I later :) )
>>>
>>> Sure, admittedly it was just a quick hack to get things going.
>>> Actually I don't know why we had a *depend* on those MACH_s before. I
>>> think technically it does not depend on a certain SoC (having the
>>> COMPILE_TEST in there hints on that). So what about:
>>
>> It was really because this DMA engine only comes with the later
>> SoCs. We have dma-sun4i for the older one.
>
> Indeed.
>
>> But yes, there's no reason why you can't build it for the earlier
>> SoC. It just doesn't get used.
>
> I'm still in favor of keeping the depends on. There's no point of
> compiling something we know have zero chance of running.
>
> (But that would be (ARCH_SUNXI && ARM64))
I am OK with that, just wondering if there is a definition of what
"depends" really means. My impression what that it's a about code
dependencies (requires a certain subsystem, for instance), not really if
it's useful in a particular configuration.
Cheers,
Andre.
^ permalink raw reply
* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Arnd Bergmann @ 2016-11-24 11:09 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Hilbert Zhang, Andrew Lunn, Ulf Hansson, Romain Perier,
Liuliu Zhao, Peng Zhu, Adrian Hunter, Nadav Haklai, Jack(SH) Zhu,
Victor Gu, Doug Jones, Jisheng Zhang, Yehuda Yitschak,
Wei(SOCP) Liu, Xueping Liu, Shiwu Zhang, Yu Cao,
Sebastian Hesselbarth, devicetree, Jason Cooper, Keji Zhang,
Kostya Porotchkin, Rob Herring, Ryan Gao
In-Reply-To: <f7334e41-39dd-f868-ff10-199afaebe926@marvell.com>
On Thursday, November 24, 2016 6:57:18 PM CET Ziji Hu wrote:
> >
> > Please explain in the changelog why this is not a generic
> > phy driver (or three of them).
> >
> Actually we tried to put the PHY code into Linux PHY framework.
> But it cannot fit in Linux common PHY framework.
>
> Our Xenon SDHC PHY register is a part of Xenon SDHC register set.
> Besides, during MMC initialization, MMC sequence has to call several PHY functions to complete timing setting.
> In those PHY setting functions, they have to access SDHC register and know current MMC setting, such as bus width, clock frequency and speed mode.
> As a result, we have to implement PHY under MMC directory.
>
Ok, that makes sense, just put the same text in the changelog comment.
Arnd
^ permalink raw reply
* Re: [RFC PATCH 3/5] arm64: defconfig: sunxi: include options for Allwinner H5 SoC
From: Andre Przywara @ 2016-11-24 11:09 UTC (permalink / raw)
To: Chen-Yu Tsai, Maxime Ripard
Cc: Icenowy Zheng, linux-sunxi, linux-arm-kernel, Mark Rutland,
Rob Herring, devicetree
In-Reply-To: <CAGb2v67AntaTBCTzJf_-oUcAqaFsQv3t4pp-E6jzpkXLNctKww-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi,
On 24/11/16 11:01, Chen-Yu Tsai wrote:
> On Thu, Nov 24, 2016 at 6:57 PM, Maxime Ripard
> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>> On Thu, Nov 24, 2016 at 01:17:13AM +0000, Andre Przywara wrote:
>>> The Allwinner H5 SoC is closely related to the H3 SoC, so select the
>>> basic pinctrl driver and the DMA driver to let a defconfig kernel boot
>>> on those boards.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
>>> ---
>>> arch/arm64/Kconfig.platforms | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>>> index cfbdf02..8300677 100644
>>> --- a/arch/arm64/Kconfig.platforms
>>> +++ b/arch/arm64/Kconfig.platforms
>>> @@ -5,8 +5,12 @@ config ARCH_SUNXI
>>> select GENERIC_IRQ_CHIP
>>> select PINCTRL
>>> select PINCTRL_SUN50I_A64
>>> + select PINCTRL_SUN8I_H3
>>> + select PINCTRL_SUN8I_H3_R
>>> + select DMA_SUN6I
>>
>> I'm not sure I want to get an ever growing select which will be an
>> union of all the drivers that all the arm64 Allwinner SoCs will
>> require.
>>
>> Select leaves no option to disable that option, and we have defconfig
>> to deal with that nicely.
> I have to agree. We should only select things that aren't selectable
> by the user. In our case, that's only the pinctrl drivers.
Right, I missed that.
> We should use default y (ARCH_SUNXI && ARM64) for every driver that
> has a prompt entry in Kconfig.
Sounds like a plan. I will look into this.
Thanks,
Andre.
^ permalink raw reply
* Re: [RFC PATCH 0/5] arm64: Allwinner H5 support
From: Andre Przywara @ 2016-11-24 11:05 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, Icenowy Zheng, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland,
Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161124105958.xzp4fr2fs4p42upx@lukather>
Hi Maxime,
thanks for looking at this.
On 24/11/16 10:59, Maxime Ripard wrote:
> On Thu, Nov 24, 2016 at 01:17:10AM +0000, Andre Przywara wrote:
>> This series adds support for the recently released Allwinner H5 SoC [1] and
>> the Orange Pi PC 2 board [2].
>> This exercise is rather easy this time, since the new SoC is very similar
>> to the existing H3 SoC and can thus share a lot of support.
>> To express this, the first patch splits the H3 .dtsi to allow reusing
>> it later. The last two patches add the H5 .dtsi and the .dts for the
>> first available board featuring this chip, based on that shared base DT.
>>
>> This is some early version, it's based on a merge of various -for-4.10
>> branches from Maxime's repository.
>> I can boot this on the OPi board and MMC and USB seem to work fine.
>> I haven't tested any other peripherals yet.
>> Some open issues:
>> - The naming: Following the Allwinner scheme this should be "sun50i-h5"
>> (which I use in this series), but it shares so much with the H3 that
>> "sun8i-h5" wouldn't be wrong either. It gets a bit weird with that shared
>> .dtsi, which I call sun8i-h3-h5.dtsi for now.
>> - The clocks and pinctrl look _almost_ similar. I may sound like a broken
>> record, but our habit of requiring kernel support for those almost identical
>> SoCs really bites us now. As the MMC got updated, I fear there is _one_
>> additional pin that we need for the HS400 transfer mode. Also I am afraid
>> the MMC clock may be slightly different due to the advanced MMC support.
>> At the moment this is not an issue, as the driver only support DDR50 at
>> most anyway, so we get away with it now.
>> I wonder if it's feasible to add those things to the existing H3 clocks
>> and pinctrl to avoid another set of drivers.
>> - I just see that I missed those patches that add just the names to the
>> binding docs. I will send them once we agreed on the naming.
>
> I don't have any major comments but I guess it all depends on the DT
> maintainers view on the symbolic link to share the DTSI.
I am curious too ;-)
But I saw symlinks for the RaspberryPi 3 (check
arch/arm64/boot/dts/broadcom) and VExpress, so I picked that low hanging
fruit ;-)
Cheers,
Andre.
^ permalink raw reply
* Re: [RFC PATCH 3/5] arm64: defconfig: sunxi: include options for Allwinner H5 SoC
From: Chen-Yu Tsai @ 2016-11-24 11:01 UTC (permalink / raw)
To: Maxime Ripard
Cc: Andre Przywara, Chen-Yu Tsai, Icenowy Zheng, linux-sunxi,
linux-arm-kernel, Mark Rutland, Rob Herring, devicetree
In-Reply-To: <20161124105725.va6gf6b3min74occ@lukather>
On Thu, Nov 24, 2016 at 6:57 PM, Maxime Ripard
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> On Thu, Nov 24, 2016 at 01:17:13AM +0000, Andre Przywara wrote:
>> The Allwinner H5 SoC is closely related to the H3 SoC, so select the
>> basic pinctrl driver and the DMA driver to let a defconfig kernel boot
>> on those boards.
>>
>> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
>> ---
>> arch/arm64/Kconfig.platforms | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>> index cfbdf02..8300677 100644
>> --- a/arch/arm64/Kconfig.platforms
>> +++ b/arch/arm64/Kconfig.platforms
>> @@ -5,8 +5,12 @@ config ARCH_SUNXI
>> select GENERIC_IRQ_CHIP
>> select PINCTRL
>> select PINCTRL_SUN50I_A64
>> + select PINCTRL_SUN8I_H3
>> + select PINCTRL_SUN8I_H3_R
>> + select DMA_SUN6I
>
> I'm not sure I want to get an ever growing select which will be an
> union of all the drivers that all the arm64 Allwinner SoCs will
> require.
>
> Select leaves no option to disable that option, and we have defconfig
> to deal with that nicely.
I have to agree. We should only select things that aren't selectable
by the user. In our case, that's only the pinctrl drivers.
We should use default y (ARCH_SUNXI && ARM64) for every driver that
has a prompt entry in Kconfig.
ChenYu
^ permalink raw reply
* Re: [RFC PATCH 0/5] arm64: Allwinner H5 support
From: Maxime Ripard @ 2016-11-24 10:59 UTC (permalink / raw)
To: Andre Przywara
Cc: Chen-Yu Tsai, Icenowy Zheng, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland,
Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479950235-26821-1-git-send-email-andre.przywara-5wv7dgnIgG8@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2036 bytes --]
On Thu, Nov 24, 2016 at 01:17:10AM +0000, Andre Przywara wrote:
> This series adds support for the recently released Allwinner H5 SoC [1] and
> the Orange Pi PC 2 board [2].
> This exercise is rather easy this time, since the new SoC is very similar
> to the existing H3 SoC and can thus share a lot of support.
> To express this, the first patch splits the H3 .dtsi to allow reusing
> it later. The last two patches add the H5 .dtsi and the .dts for the
> first available board featuring this chip, based on that shared base DT.
>
> This is some early version, it's based on a merge of various -for-4.10
> branches from Maxime's repository.
> I can boot this on the OPi board and MMC and USB seem to work fine.
> I haven't tested any other peripherals yet.
> Some open issues:
> - The naming: Following the Allwinner scheme this should be "sun50i-h5"
> (which I use in this series), but it shares so much with the H3 that
> "sun8i-h5" wouldn't be wrong either. It gets a bit weird with that shared
> .dtsi, which I call sun8i-h3-h5.dtsi for now.
> - The clocks and pinctrl look _almost_ similar. I may sound like a broken
> record, but our habit of requiring kernel support for those almost identical
> SoCs really bites us now. As the MMC got updated, I fear there is _one_
> additional pin that we need for the HS400 transfer mode. Also I am afraid
> the MMC clock may be slightly different due to the advanced MMC support.
> At the moment this is not an issue, as the driver only support DDR50 at
> most anyway, so we get away with it now.
> I wonder if it's feasible to add those things to the existing H3 clocks
> and pinctrl to avoid another set of drivers.
> - I just see that I missed those patches that add just the names to the
> binding docs. I will send them once we agreed on the naming.
I don't have any major comments but I guess it all depends on the DT
maintainers view on the symbolic link to share the DTSI.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v4 00/06] clocksource: sh_cmt: DT binding rework V4
From: Simon Horman @ 2016-11-24 10:58 UTC (permalink / raw)
To: Magnus Damm
Cc: linux-kernel, devicetree, laurent.pinchart+renesas, geert+renesas,
robh, daniel.lezcano, linux-renesas-soc, tglx
In-Reply-To: <20160314142342.17180.21352.sendpatchset@little-apple>
Hi Magnus,
On Mon, Mar 14, 2016 at 11:23:42PM +0900, Magnus Damm wrote:
> clocksource: sh_cmt: DT binding rework V4
>
> [PATCH v4 01/06] devicetree: bindings: Remove sh7372 CMT binding
> [PATCH v4 02/06] devicetree: bindings: R-Car Gen2 CMT0 and CMT1 bindings
> [PATCH v4 03/06] devicetree: bindings: r8a73a4 and R-Car Gen2 CMT bindings
> [PATCH v4 04/06] devicetree: bindings: Deprecate property, update example
> [PATCH v4 05/06] devicetree: bindings: Remove unused 32-bit CMT bindings
> [PATCH v4 06/06] devicetree: bindings: Remove deprecated properties
>
> Here is the latest and hopefully final take on updating the CMT DT
> bindings for R-Car Gen2. In total there are 6 patches that have acks
> and are ready to be picked up and merged. Other earlier posted changes
> such as driver modification and SoC DTS bits depend on this series.
I am wondering what the state of this work is.
I see only one minor review comment for this series.
It would be great to see it merged.
^ permalink raw reply
* Re: [RFC PATCH 3/5] arm64: defconfig: sunxi: include options for Allwinner H5 SoC
From: Maxime Ripard @ 2016-11-24 10:57 UTC (permalink / raw)
To: Andre Przywara
Cc: Chen-Yu Tsai, Icenowy Zheng, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland,
Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479950235-26821-4-git-send-email-andre.przywara-5wv7dgnIgG8@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1128 bytes --]
On Thu, Nov 24, 2016 at 01:17:13AM +0000, Andre Przywara wrote:
> The Allwinner H5 SoC is closely related to the H3 SoC, so select the
> basic pinctrl driver and the DMA driver to let a defconfig kernel boot
> on those boards.
>
> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
> ---
> arch/arm64/Kconfig.platforms | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index cfbdf02..8300677 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -5,8 +5,12 @@ config ARCH_SUNXI
> select GENERIC_IRQ_CHIP
> select PINCTRL
> select PINCTRL_SUN50I_A64
> + select PINCTRL_SUN8I_H3
> + select PINCTRL_SUN8I_H3_R
> + select DMA_SUN6I
I'm not sure I want to get an ever growing select which will be an
union of all the drivers that all the arm64 Allwinner SoCs will
require.
Select leaves no option to disable that option, and we have defconfig
to deal with that nicely.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ziji Hu @ 2016-11-24 10:57 UTC (permalink / raw)
To: Arnd Bergmann, Gregory CLEMENT
Cc: Ulf Hansson, Adrian Hunter, linux-mmc, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Rob Herring, devicetree, Thomas Petazzoni,
linux-arm-kernel, Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang,
Nadav Haklai, Ryan Gao, Doug Jones, Shiwu Zhang, Victor Gu,
Wei(SOCP) Liu, Wilson Ding, Xueping Liu
In-Reply-To: <2204525.IWIYQVjIXl@wuerfel>
Hi Arnd,
On 2016/11/24 17:56, Arnd Bergmann wrote:
> On Monday, October 31, 2016 12:09:56 PM CET Gregory CLEMENT wrote:
>> From: Ziji Hu <huziji@marvell.com>
>>
>> Marvell Xenon eMMC/SD/SDIO Host Controller contains PHY.
>> Three types of PHYs are supported.
>>
>> Add support to multiple types of PHYs init and configuration.
>> Add register definitions of PHYs.
>>
>> Signed-off-by: Hu Ziji <huziji@marvell.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>
>
> Please explain in the changelog why this is not a generic
> phy driver (or three of them).
>
Actually we tried to put the PHY code into Linux PHY framework.
But it cannot fit in Linux common PHY framework.
Our Xenon SDHC PHY register is a part of Xenon SDHC register set.
Besides, during MMC initialization, MMC sequence has to call several PHY functions to complete timing setting.
In those PHY setting functions, they have to access SDHC register and know current MMC setting, such as bus width, clock frequency and speed mode.
As a result, we have to implement PHY under MMC directory.
Thank you.
Best regards,
Hu Ziji
> Arnd
>
^ permalink raw reply
* Re: [RFC PATCH 2/5] dmaengine: allow sun6i-dma for more SoCs
From: Maxime Ripard @ 2016-11-24 10:55 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Andre Przywara, Icenowy Zheng, linux-sunxi, linux-arm-kernel,
Mark Rutland, Rob Herring, devicetree
In-Reply-To: <CAGb2v65G7=9ah+sEet=z5vss60kL5ZLSkNsAcGpwu8V6AWdEGA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2111 bytes --]
On Thu, Nov 24, 2016 at 05:30:45PM +0800, Chen-Yu Tsai wrote:
> On Thu, Nov 24, 2016 at 5:16 PM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> wrote:
> > Hi,
> >
> > On 24/11/16 04:16, Chen-Yu Tsai wrote:
> >> Hi,
> >>
> >> On Thu, Nov 24, 2016 at 9:17 AM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> wrote:
> >>> The sun6i DMA driver is used in the Allwinner A64 and H5 SoC, which
> >>> have arm64 capable cores. Add the generic sunxi config symbol to allow
> >>> the driver to be selected by arm64 Kconfigs, which don't feature
> >>> SoC specific MACH_xxxx configs.
> >>>
> >>> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
> >>> ---
> >>> drivers/dma/Kconfig | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> >>> index af63a6b..003c284 100644
> >>> --- a/drivers/dma/Kconfig
> >>> +++ b/drivers/dma/Kconfig
> >>> @@ -157,7 +157,7 @@ config DMA_SUN4I
> >>>
> >>> config DMA_SUN6I
> >>> tristate "Allwinner A31 SoCs DMA support"
> >>> - depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST
> >>> + depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST || ARCH_SUNXI
> >>
> >> AFAIK ARCH_SUNXI encompasses/supersedes MACH_SUN*I.
> >> (And I don't have to add MACH_SUN9I later :) )
> >
> > Sure, admittedly it was just a quick hack to get things going.
> > Actually I don't know why we had a *depend* on those MACH_s before. I
> > think technically it does not depend on a certain SoC (having the
> > COMPILE_TEST in there hints on that). So what about:
>
> It was really because this DMA engine only comes with the later
> SoCs. We have dma-sun4i for the older one.
Indeed.
> But yes, there's no reason why you can't build it for the earlier
> SoC. It just doesn't get used.
I'm still in favor of keeping the depends on. There's no point of
compiling something we know have zero chance of running.
(But that would be (ARCH_SUNXI && ARM64))
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v7 9/9] ARM: dts: omap3: Fix NAND device nodess
From: Roger Quadros @ 2016-11-24 10:49 UTC (permalink / raw)
To: Adam Ford
Cc: Tony Lindgren, computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ,
javier-0uQlZySMnqxg9hUCZPvPmw, fcooper-l0cyMroinI0, Sekhar Nori,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAHCN7xJqzgtuSW4KK93tHQh2WVRE4L5Zh6+5sGM+DZGU4qy5zg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Adam
On 24/11/16 00:34, Adam Ford wrote:
>
> On Tue, Feb 23, 2016 at 10:37 AM, Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org <mailto:rogerq-l0cyMroinI0@public.gmane.org>> wrote:
>
> Add compatible id, GPMC register resource and interrupt
> resource to NAND controller nodes.
>
> The GPMC node will provide an interrupt controller for the
> NAND IRQs.
>
> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org <mailto:rogerq-l0cyMroinI0@public.gmane.org>>
> ---
> arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts | 3 ++-
> arch/arm/boot/dts/logicpd-torpedo-som.dtsi | 8 ++++++--
> arch/arm/boot/dts/omap3-beagle.dts | 5 ++++-
> arch/arm/boot/dts/omap3-cm-t3x.dtsi | 6 +++++-
> arch/arm/boot/dts/omap3-devkit8000-common.dtsi | 4 ++++
> arch/arm/boot/dts/omap3-evm-37xx.dts | 8 ++++++--
> arch/arm/boot/dts/omap3-gta04.dtsi | 4 ++++
> arch/arm/boot/dts/omap3-igep.dtsi | 6 +++++-
> arch/arm/boot/dts/omap3-igep0020-common.dtsi | 4 ++--
> arch/arm/boot/dts/omap3-igep0030-common.dtsi | 4 ++++
> arch/arm/boot/dts/omap3-ldp.dts | 10 +++++++---
> arch/arm/boot/dts/omap3-lilly-a83x.dtsi | 6 +++++-
> arch/arm/boot/dts/omap3-overo-base.dtsi | 6 +++++-
> arch/arm/boot/dts/omap3-pandora-common.dtsi | 4 ++++
> arch/arm/boot/dts/omap3-tao3530.dtsi | 6 +++++-
> arch/arm/boot/dts/omap3.dtsi | 2 ++
> arch/arm/boot/dts/omap3430-sdp.dts | 6 +++++-
> 17 files changed, 75 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
> index fb13f18..d0211fc 100644
> --- a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
> +++ b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
> @@ -93,7 +93,8 @@
> };
>
> &gpmc {
> - ranges = <1 0 0x08000000 0x1000000>; /* CS1: 16MB for LAN9221 */
> + ranges = <0 0 0x30000000 0x1000000 /* CS0: 16MB for NAND */
> + 1 0 0x2c000000 0x1000000>; /* CS1: 16MB for LAN9221 */
>
> I just noticed this change earlier today, and I have question.
>
> There is a DTS and a DTSI file. The DTSI has the NAND information and the range was updated. If this was included in the dts, is there really a need to add the CS0 range info in the dts file as well?
>
> My preference would be to eliminate the new CS0 entry in the DTS file and leave the update in the DTSI. Is that reasonable, or am I missing something?
The ranges property will be overridden so you can't have only CS0 defined in .dtsi
and CS1 defined in the .dts. You will have to either define both in .dts and
remove the ranges property from the .dts or define both in the .dts.
AFAIK .dtsi file didn't have CS1 range so I had to define both CS0 and CS1 in the .dts.
>
>
> ethernet@gpmc {
> pinctrl-names = "default";
> diff --git a/arch/arm/boot/dts/logicpd-torpedo-som.dtsi b/arch/arm/boot/dts/logicpd-torpedo-som.dtsi
> index 7fed0bd..b46789a 100644
> --- a/arch/arm/boot/dts/logicpd-torpedo-som.dtsi
> +++ b/arch/arm/boot/dts/logicpd-torpedo-som.dtsi
> @@ -35,11 +35,15 @@
> };
>
> &gpmc {
> - ranges = <0 0 0x00000000 0x1000000>; /* CS0: 16MB for NAND */
> + ranges = <0 0 0x30000000 0x1000000>; /* CS0: 16MB for NAND */
>
>
> From what I can tell this exactly matches the same entry listed above in the dtsi.
>
Yes, but we don't have CS1 here.
If all torpedo boards don't have CS1 in use then we can't define CS1 here.
cheers,
-roger
> adam
>
>
> nand@0,0 {
> - linux,mtd-name = "micron,mt29f4g16abbda3w";
> + compatible = "ti,omap2-nand";
> reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> + interrupt-parent = <&gpmc>;
> + interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
> + <1 IRQ_TYPE_NONE>; /* termcount */
> + linux,mtd-name = "micron,mt29f4g16abbda3w";
> nand-bus-width = <16>;
> ti,nand-ecc-opt = "bch8";
> gpmc,sync-clk-ps = <0>;
> diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
> index 8ba465d..4602866 100644
> --- a/arch/arm/boot/dts/omap3-beagle.dts
> +++ b/arch/arm/boot/dts/omap3-beagle.dts
> @@ -384,8 +384,11 @@
>
> /* Chip select 0 */
> nand@0,0 {
> + compatible = "ti,omap2-nand";
> reg = <0 0 4>; /* NAND I/O window, 4 bytes */
> - interrupts = <20>;
> + interrupt-parent = <&gpmc>;
> + interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
> + <1 IRQ_TYPE_NONE>; /* termcount */
> ti,nand-ecc-opt = "ham1";
> nand-bus-width = <16>;
> #address-cells = <1>;
> diff --git a/arch/arm/boot/dts/omap3-cm-t3x.dtsi b/arch/arm/boot/dts/omap3-cm-t3x.dtsi
> index e5f7f5c..a8127bc 100644
> --- a/arch/arm/boot/dts/omap3-cm-t3x.dtsi
> +++ b/arch/arm/boot/dts/omap3-cm-t3x.dtsi
> @@ -261,10 +261,14 @@
> };
>
> &gpmc {
> - ranges = <0 0 0x00000000 0x01000000>;
> + ranges = <0 0 0x30000000 0x01000000>; /* CS0: 16MB for NAND */
>
> nand@0,0 {
> + compatible = "ti,omap2-nand";
> reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> + interrupt-parent = <&gpmc>;
> + interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
> + <1 IRQ_TYPE_NONE>; /* termcount */
> nand-bus-width = <8>;
> gpmc,device-width = <1>;
> ti,nand-ecc-opt = "sw";
> diff --git a/arch/arm/boot/dts/omap3-devkit8000-common.dtsi b/arch/arm/boot/dts/omap3-devkit8000-common.dtsi
> index 86850bb..b1b8ebf 100644
> --- a/arch/arm/boot/dts/omap3-devkit8000-common.dtsi
> +++ b/arch/arm/boot/dts/omap3-devkit8000-common.dtsi
> @@ -204,7 +204,11 @@
> ranges = <0 0 0x30000000 0x1000000>; /* CS0: 16MB for NAND */
>
> nand@0,0 {
> + compatible = "ti,omap2-nand";
> reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> + interrupt-parent = <&gpmc>;
> + interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
> + <1 IRQ_TYPE_NONE>; /* termcount */
> nand-bus-width = <16>;
> gpmc,device-width = <2>;
> ti,nand-ecc-opt = "sw";
> diff --git a/arch/arm/boot/dts/omap3-evm-37xx.dts b/arch/arm/boot/dts/omap3-evm-37xx.dts
> index ac18865..76056ba 100644
> --- a/arch/arm/boot/dts/omap3-evm-37xx.dts
> +++ b/arch/arm/boot/dts/omap3-evm-37xx.dts
> @@ -154,12 +154,16 @@
> };
>
> &gpmc {
> - ranges = <0 0 0x00000000 0x1000000>, /* CS0: 16MB for NAND */
> + ranges = <0 0 0x30000000 0x1000000>, /* CS0: 16MB for NAND */
> <5 0 0x2c000000 0x01000000>;
>
> nand@0,0 {
> + compatible = "ti,omap2-nand";
> + reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> + interrupt-parent = <&gpmc>;
> + interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
> + <1 IRQ_TYPE_NONE>; /* termcount */
> linux,mtd-name= "hynix,h8kds0un0mer-4em";
> - reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> nand-bus-width = <16>;
> gpmc,device-width = <2>;
> ti,nand-ecc-opt = "bch8";
> diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi
> index 5e2d643..ab9fb8f 100644
> --- a/arch/arm/boot/dts/omap3-gta04.dtsi
> +++ b/arch/arm/boot/dts/omap3-gta04.dtsi
> @@ -492,7 +492,11 @@
> ranges = <0 0 0x30000000 0x1000000>; /* CS0: 16MB for NAND */
>
> nand@0,0 {
> + compatible = "ti,omap2-nand";
> reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> + interrupt-parent = <&gpmc>;
> + interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
> + <1 IRQ_TYPE_NONE>; /* termcount */
> nand-bus-width = <16>;
> ti,nand-ecc-opt = "bch8";
>
> diff --git a/arch/arm/boot/dts/omap3-igep.dtsi b/arch/arm/boot/dts/omap3-igep.dtsi
> index 3caf062..81aec99 100644
> --- a/arch/arm/boot/dts/omap3-igep.dtsi
> +++ b/arch/arm/boot/dts/omap3-igep.dtsi
> @@ -95,8 +95,12 @@
>
> &gpmc {
> nand@0,0 {
> + compatible = "ti,omap2-nand";
> + reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> + interrupt-parent = <&gpmc>;
> + interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
> + <1 IRQ_TYPE_NONE>; /* termcount */
> linux,mtd-name= "micron,mt29c4g96maz";
> - reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> nand-bus-width = <16>;
> gpmc,device-width = <2>;
> ti,nand-ecc-opt = "bch8";
> diff --git a/arch/arm/boot/dts/omap3-igep0020-common.dtsi b/arch/arm/boot/dts/omap3-igep0020-common.dtsi
> index d90f12c..d6f839c 100644
> --- a/arch/arm/boot/dts/omap3-igep0020-common.dtsi
> +++ b/arch/arm/boot/dts/omap3-igep0020-common.dtsi
> @@ -210,8 +210,8 @@
> };
>
> &gpmc {
> - ranges = <0 0 0x00000000 0x20000000>,
> - <5 0 0x2c000000 0x01000000>;
> + ranges = <0 0 0x30000000 0x01000000>, /* CS0: 16MB for NAND */
> + <5 0 0x2c000000 0x01000000>; /* CS5: 16MB for ethernet */
>
> ethernet@gpmc {
> pinctrl-names = "default";
> diff --git a/arch/arm/boot/dts/omap3-igep0030-common.dtsi b/arch/arm/boot/dts/omap3-igep0030-common.dtsi
> index 640f066..cd91ef0 100644
> --- a/arch/arm/boot/dts/omap3-igep0030-common.dtsi
> +++ b/arch/arm/boot/dts/omap3-igep0030-common.dtsi
> @@ -58,3 +58,7 @@
> pinctrl-names = "default";
> pinctrl-0 = <&uart2_pins>;
> };
> +
> +&gpmc {
> + ranges = <0 0 0x30000000 0x01000000>; /* CS0: 16MB for NAND */
> +};
> diff --git a/arch/arm/boot/dts/omap3-ldp.dts b/arch/arm/boot/dts/omap3-ldp.dts
> index 5401630..2f353da 100644
> --- a/arch/arm/boot/dts/omap3-ldp.dts
> +++ b/arch/arm/boot/dts/omap3-ldp.dts
> @@ -97,12 +97,16 @@
> };
>
> &gpmc {
> - ranges = <0 0 0x00000000 0x01000000>,
> - <1 0 0x08000000 0x01000000>;
> + ranges = <0 0 0x30000000 0x1000000>, /* CS0 space, 16MB */
> + <1 0 0x08000000 0x1000000>; /* CS1 space, 16MB */
>
> nand@0,0 {
> + compatible = "ti,omap2-nand";
> + reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> + interrupt-parent = <&gpmc>;
> + interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
> + <1 IRQ_TYPE_NONE>; /* termcount */
> linux,mtd-name= "micron,nand";
> - reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> nand-bus-width = <16>;
> gpmc,device-width = <2>;
> ti,nand-ecc-opt = "bch8";
> diff --git a/arch/arm/boot/dts/omap3-lilly-a83x.dtsi b/arch/arm/boot/dts/omap3-lilly-a83x.dtsi
> index 93f8dfe..eff816e 100644
> --- a/arch/arm/boot/dts/omap3-lilly-a83x.dtsi
> +++ b/arch/arm/boot/dts/omap3-lilly-a83x.dtsi
> @@ -362,7 +362,11 @@
> <7 0 0x15000000 0x01000000>;
>
> nand@0,0 {
> - reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> + compatible = "ti,omap2-nand";
> + reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> + interrupt-parent = <&gpmc>;
> + interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
> + <1 IRQ_TYPE_NONE>; /* termcount */
> nand-bus-width = <16>;
> ti,nand-ecc-opt = "bch8";
> /* no elm on omap3 */
> diff --git a/arch/arm/boot/dts/omap3-overo-base.dtsi b/arch/arm/boot/dts/omap3-overo-base.dtsi
> index a29ad16..de256fa 100644
> --- a/arch/arm/boot/dts/omap3-overo-base.dtsi
> +++ b/arch/arm/boot/dts/omap3-overo-base.dtsi
> @@ -226,8 +226,12 @@
> ranges = <0 0 0x00000000 0x20000000>;
>
> nand@0,0 {
> + compatible = "ti,omap2-nand";
> linux,mtd-name= "micron,mt29c4g96maz";
> - reg = <0 0 0>;
> + reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> + interrupt-parent = <&gpmc>;
> + interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
> + <1 IRQ_TYPE_NONE>; /* termcount */
> nand-bus-width = <16>;
> gpmc,device-width = <2>;
> ti,nand-ecc-opt = "bch8";
> diff --git a/arch/arm/boot/dts/omap3-pandora-common.dtsi b/arch/arm/boot/dts/omap3-pandora-common.dtsi
> index 13e9d1f..bcf39d6 100644
> --- a/arch/arm/boot/dts/omap3-pandora-common.dtsi
> +++ b/arch/arm/boot/dts/omap3-pandora-common.dtsi
> @@ -546,7 +546,11 @@
> ranges = <0 0 0x30000000 0x1000000>; /* CS0: 16MB for NAND */
>
> nand@0,0 {
> + compatible = "ti,omap2-nand";
> reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> + interrupt-parent = <&gpmc>;
> + interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
> + <1 IRQ_TYPE_NONE>; /* termcount */
> nand-bus-width = <16>;
> ti,nand-ecc-opt = "sw";
>
> diff --git a/arch/arm/boot/dts/omap3-tao3530.dtsi b/arch/arm/boot/dts/omap3-tao3530.dtsi
> index ae5dbbd..644d3c8 100644
> --- a/arch/arm/boot/dts/omap3-tao3530.dtsi
> +++ b/arch/arm/boot/dts/omap3-tao3530.dtsi
> @@ -275,10 +275,14 @@
> };
>
> &gpmc {
> - ranges = <0 0 0x00000000 0x01000000>;
> + ranges = <0 0 0x30000000 0x01000000>; /* CS0: 16MB for NAND */
>
> nand@0,0 {
> + compatible = "ti,omap2-nand";
> reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> + interrupt-parent = <&gpmc>;
> + interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
> + <1 IRQ_TYPE_NONE>; /* termcount */
> nand-bus-width = <16>;
> gpmc,device-width = <2>; /* GPMC_DEVWIDTH_16BIT */
> ti,nand-ecc-opt = "sw";
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index d1ffabb..b41d07e 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -723,6 +723,8 @@
> gpmc,num-waitpins = <4>;
> #address-cells = <2>;
> #size-cells = <1>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> };
>
> usb_otg_hs: usb_otg_hs@480ab000 {
> diff --git a/arch/arm/boot/dts/omap3430-sdp.dts b/arch/arm/boot/dts/omap3430-sdp.dts
> index 16b0cdf..a0dc8d8 100644
> --- a/arch/arm/boot/dts/omap3430-sdp.dts
> +++ b/arch/arm/boot/dts/omap3430-sdp.dts
> @@ -103,10 +103,14 @@
> };
>
> nand@1,0 {
> + compatible = "ti,omap2-nand";
> + reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
> + interrupt-parent = <&gpmc>;
> + interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
> + <1 IRQ_TYPE_NONE>; /* termcount */
> linux,mtd-name= "micron,mt29f1g08abb";
> #address-cells = <1>;
> #size-cells = <1>;
> - reg = <1 0 4>; /* CS1, offset 0, IO size 4 */
> ti,nand-ecc-opt = "sw";
> nand-bus-width = <8>;
> gpmc,cs-on-ns = <0>;
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org <mailto:majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> More majordomo info at http://vger.kernel.org/majordomo-info.html <http://vger.kernel.org/majordomo-info.html>
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Ulf Hansson @ 2016-11-24 10:43 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Adrian Hunter, linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Thomas Petazzoni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Ziji Hu, Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang, Nadav Haklai,
Ryan Gao, Doug Jones, Shiwu Zhang, Victor Gu, Wei(SOCP)
In-Reply-To: <0390e7a05b6163deabb545f93729ea615eeaaee2.1477911954.git-series.gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On 31 October 2016 at 12:09, Gregory CLEMENT
<gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> From: Ziji Hu <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
>
> Add Xenon eMMC/SD/SDIO host controller core functionality.
> Add Xenon specific intialization process.
> Add Xenon specific mmc_host_ops APIs.
> Add Xenon specific register definitions.
>
> Add CONFIG_MMC_SDHCI_XENON support in drivers/mmc/host/Kconfig.
>
> Marvell Xenon SDHC conforms to SD Physical Layer Specification
> Version 3.01 and is designed according to the guidelines provided
> in the SD Host Controller Standard Specification Version 3.00.
>
> Signed-off-by: Hu Ziji <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> MAINTAINERS | 1 +-
> drivers/mmc/host/Kconfig | 9 +-
> drivers/mmc/host/Makefile | 3 +-
> drivers/mmc/host/sdhci-xenon.c | 594 ++++++++++++++++++++++++++++++++++-
> drivers/mmc/host/sdhci-xenon.h | 142 ++++++++-
> 5 files changed, 749 insertions(+), 0 deletions(-)
> create mode 100644 drivers/mmc/host/sdhci-xenon.c
> create mode 100644 drivers/mmc/host/sdhci-xenon.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 850a0afb0c8d..d92f4175574b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7608,6 +7608,7 @@ MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
> M: Ziji Hu <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> L: linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> S: Supported
> +F: drivers/mmc/host/sdhci-xenon.*
> F: Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>
> MATROX FRAMEBUFFER DRIVER
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 5274f503a39a..85a53623526a 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -798,3 +798,12 @@ config MMC_SDHCI_BRCMSTB
> Broadcom STB SoCs.
>
> If unsure, say Y.
> +
> +config MMC_SDHCI_XENON
> + tristate "Marvell Xenon eMMC/SD/SDIO SDHCI driver"
> + depends on MMC_SDHCI && MMC_SDHCI_PLTFM
> + help
> + This selects Marvell Xenon eMMC/SD/SDIO SDHCI.
> + If you have a machine with integrated Marvell Xenon SDHC IP,
> + say Y or M here.
> + If unsure, say N.
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index e2bdaaf43184..75eaf743486c 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -80,3 +80,6 @@ obj-$(CONFIG_MMC_SDHCI_BRCMSTB) += sdhci-brcmstb.o
> ifeq ($(CONFIG_CB710_DEBUG),y)
> CFLAGS-cb710-mmc += -DDEBUG
> endif
> +
> +obj-$(CONFIG_MMC_SDHCI_XENON) += sdhci-xenon-driver.o
> +sdhci-xenon-driver-y += sdhci-xenon.o
> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
> new file mode 100644
> index 000000000000..3ea059f2aaab
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-xenon.c
> @@ -0,0 +1,594 @@
> +/*
> + * Driver for Marvell SOC Platform Group Xenon SDHC as a platform device
> + *
> + * Copyright (C) 2016 Marvell, All Rights Reserved.
> + *
> + * Author: Hu Ziji <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> + * Date: 2016-8-24
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * Inspired by Jisheng Zhang <jszhang-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> + * Special thanks to Video BG4 project team.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/mmc.h>
> +#include <linux/mmc/sdio.h>
> +#include <linux/mmc/card.h>
> +#include <linux/mmc/host.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +
> +#include "sdhci-pltfm.h"
> +#include "sdhci.h"
> +#include "sdhci-xenon.h"
> +
> +/* Set SDCLK-off-while-idle */
> +static void xenon_set_sdclk_off_idle(struct sdhci_host *host,
> + unsigned char slot_idx, bool enable)
> +{
> + u32 reg;
> + u32 mask;
> +
> + reg = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> + /* Get the bit shift basing on the slot index */
> + mask = (0x1 << (SDCLK_IDLEOFF_ENABLE_SHIFT + slot_idx));
> + if (enable)
> + reg |= mask;
> + else
> + reg &= ~mask;
> +
> + sdhci_writel(host, reg, SDHC_SYS_OP_CTRL);
> +}
> +
> +/* Enable/Disable the Auto Clock Gating function */
> +static void xenon_set_acg(struct sdhci_host *host, bool enable)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> + if (enable)
> + reg &= ~AUTO_CLKGATE_DISABLE_MASK;
> + else
> + reg |= AUTO_CLKGATE_DISABLE_MASK;
> + sdhci_writel(host, reg, SDHC_SYS_OP_CTRL);
> +}
> +
> +/* Enable this slot */
> +static void xenon_enable_slot(struct sdhci_host *host,
> + unsigned char slot_idx)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> + reg |= (BIT(slot_idx) << SLOT_ENABLE_SHIFT);
> + sdhci_writel(host, reg, SDHC_SYS_OP_CTRL);
> +
> + /*
> + * Manually set the flag which all the slots require,
> + * including SD, eMMC, SDIO
> + */
> + host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
> +}
> +
> +/* Disable this slot */
> +static void xenon_disable_slot(struct sdhci_host *host,
> + unsigned char slot_idx)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> + reg &= ~(BIT(slot_idx) << SLOT_ENABLE_SHIFT);
> + sdhci_writel(host, reg, SDHC_SYS_OP_CTRL);
> +}
> +
> +/* Enable Parallel Transfer Mode */
> +static void xenon_enable_slot_parallel_tran(struct sdhci_host *host,
> + unsigned char slot_idx)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHC_SYS_EXT_OP_CTRL);
> + reg |= BIT(slot_idx);
> + sdhci_writel(host, reg, SDHC_SYS_EXT_OP_CTRL);
> +}
> +
> +static void xenon_slot_tuning_setup(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + u32 reg;
> +
> + /* Disable the Re-Tuning Request functionality */
> + reg = sdhci_readl(host, SDHC_SLOT_RETUNING_REQ_CTRL);
> + reg &= ~RETUNING_COMPATIBLE;
> + sdhci_writel(host, reg, SDHC_SLOT_RETUNING_REQ_CTRL);
> +
> + /* Disable the Re-tuning Event Signal Enable */
> + reg = sdhci_readl(host, SDHCI_SIGNAL_ENABLE);
> + reg &= ~SDHCI_INT_RETUNE;
> + sdhci_writel(host, reg, SDHCI_SIGNAL_ENABLE);
> +
> + /* Force to use Tuning Mode 1 */
> + host->tuning_mode = SDHCI_TUNING_MODE_1;
> + /* Set re-tuning period */
> + host->tuning_count = 1 << (priv->tuning_count - 1);
> +}
> +
> +/*
> + * Operations inside struct sdhci_ops
> + */
> +/* Recover the Register Setting cleared during SOFTWARE_RESET_ALL */
> +static void sdhci_xenon_reset_exit(struct sdhci_host *host,
> + unsigned char slot_idx, u8 mask)
> +{
> + /* Only SOFTWARE RESET ALL will clear the register setting */
> + if (!(mask & SDHCI_RESET_ALL))
> + return;
> +
> + /* Disable tuning request and auto-retuning again */
> + xenon_slot_tuning_setup(host);
> +
> + xenon_set_acg(host, true);
> +
> + xenon_set_sdclk_off_idle(host, slot_idx, false);
> +}
> +
> +static void sdhci_xenon_reset(struct sdhci_host *host, u8 mask)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> + sdhci_reset(host, mask);
> + sdhci_xenon_reset_exit(host, priv->slot_idx, mask);
> +}
> +
> +/*
> + * Xenon defines different values for HS200 and SDR104
> + * in Host_Control_2
> + */
> +static void xenon_set_uhs_signaling(struct sdhci_host *host,
> + unsigned int timing)
> +{
> + u16 ctrl_2;
> +
> + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + /* Select Bus Speed Mode for host */
> + ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
> + if (timing == MMC_TIMING_MMC_HS200)
> + ctrl_2 |= XENON_SDHCI_CTRL_HS200;
> + else if (timing == MMC_TIMING_UHS_SDR104)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR104;
> + else if (timing == MMC_TIMING_UHS_SDR12)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
> + else if (timing == MMC_TIMING_UHS_SDR25)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
> + else if (timing == MMC_TIMING_UHS_SDR50)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR50;
> + else if ((timing == MMC_TIMING_UHS_DDR50) ||
> + (timing == MMC_TIMING_MMC_DDR52))
> + ctrl_2 |= SDHCI_CTRL_UHS_DDR50;
> + else if (timing == MMC_TIMING_MMC_HS400)
> + ctrl_2 |= XENON_SDHCI_CTRL_HS400;
> + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> +}
> +
> +static const struct sdhci_ops sdhci_xenon_ops = {
> + .set_clock = sdhci_set_clock,
> + .set_bus_width = sdhci_set_bus_width,
> + .reset = sdhci_xenon_reset,
> + .set_uhs_signaling = xenon_set_uhs_signaling,
> + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> +};
> +
> +static const struct sdhci_pltfm_data sdhci_xenon_pdata = {
> + .ops = &sdhci_xenon_ops,
> + .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
> + SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER |
> + SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +};
> +
> +/*
> + * Xenon Specific Operations in mmc_host_ops
> + */
> +static void xenon_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + unsigned long flags;
> + u32 reg;
> +
> + /*
> + * HS400/HS200/eMMC HS doesn't have Preset Value register.
> + * However, sdhci_set_ios will read HS400/HS200 Preset register.
> + * Disable Preset Value register for HS400/HS200.
> + * eMMC HS with preset_enabled set will trigger a bug in
> + * get_preset_value().
> + */
> + spin_lock_irqsave(&host->lock, flags);
> + if ((ios->timing == MMC_TIMING_MMC_HS400) ||
> + (ios->timing == MMC_TIMING_MMC_HS200) ||
> + (ios->timing == MMC_TIMING_MMC_HS)) {
> + host->preset_enabled = false;
> + host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> +
> + reg = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + reg &= ~SDHCI_CTRL_PRESET_VAL_ENABLE;
> + sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
> + } else {
> + host->quirks2 &= ~SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> + }
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + sdhci_set_ios(mmc, ios);
> +
> + if (host->clock > DEFAULT_SDCLK_FREQ) {
> + spin_lock_irqsave(&host->lock, flags);
> + xenon_set_sdclk_off_idle(host, priv->slot_idx, true);
> + spin_unlock_irqrestore(&host->lock, flags);
> + }
> +}
> +
> +static int __emmc_signal_voltage_switch(struct mmc_host *mmc,
> + const unsigned char signal_voltage)
> +{
> + u32 ctrl;
> + unsigned char voltage_code;
> + struct sdhci_host *host = mmc_priv(mmc);
> +
> + if (signal_voltage == MMC_SIGNAL_VOLTAGE_330)
> + voltage_code = EMMC_VCCQ_3_3V;
> + else if (signal_voltage == MMC_SIGNAL_VOLTAGE_180)
> + voltage_code = EMMC_VCCQ_1_8V;
> + else
> + return -EINVAL;
> +
> + /*
> + * This host is for eMMC, XENON self-defined
> + * eMMC slot control register should be accessed
> + * instead of Host Control 2
> + */
> + ctrl = sdhci_readl(host, SDHC_SLOT_EMMC_CTRL);
> + ctrl &= ~EMMC_VCCQ_MASK;
> + ctrl |= voltage_code;
> + sdhci_writel(host, ctrl, SDHC_SLOT_EMMC_CTRL);
> +
> + /* There is no standard to determine this waiting period */
> + usleep_range(1000, 2000);
> +
> + /* Check whether io voltage switch is done */
> + ctrl = sdhci_readl(host, SDHC_SLOT_EMMC_CTRL);
> + ctrl &= EMMC_VCCQ_MASK;
> + /*
> + * This bit is set only when regulator feeds back the voltage switch
> + * results to Xenon SDHC.
> + * However, in actaul implementation, regulator might not provide
> + * this feedback.
> + * Thus we shall not rely on this bit to determine if switch failed.
> + * If the bit is not set, just throw a message.
> + * Besides, error code should not be returned.
> + */
> + if (ctrl != voltage_code)
> + dev_info(mmc_dev(mmc), "fail to detect eMMC signal voltage stable\n");
> + return 0;
> +}
> +
> +static int xenon_emmc_signal_voltage_switch(struct mmc_host *mmc,
> + struct mmc_ios *ios)
> +{
> + unsigned char voltage = ios->signal_voltage;
> +
> + if ((voltage == MMC_SIGNAL_VOLTAGE_330) ||
> + (voltage == MMC_SIGNAL_VOLTAGE_180))
> + return __emmc_signal_voltage_switch(mmc, voltage);
> +
> + dev_err(mmc_dev(mmc), "Unsupported signal voltage: %d\n",
> + voltage);
> + return -EINVAL;
This wrapper function seems unnessarry. It only adds a dev_err(), so
then might as well do that in __emmc_signal_voltage_switch().
> +}
> +
> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
> + struct mmc_ios *ios)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> + /*
> + * Before SD/SDIO set signal voltage, SD bus clock should be
> + * disabled. However, sdhci_set_clock will also disable the Internal
> + * clock in mmc_set_signal_voltage().
If that's the case then that is wrong in the generic sdhci code.
What's the reason why it can't be fixed there instead of having this
workaround?
> + * If Internal clock is disabled, the 3.3V/1.8V bit can not be updated.
> + * Thus here manually enable internal clock.
> + *
> + * After switch completes, it is unnecessary to disable internal clock,
> + * since keeping internal clock active obeys SD spec.
> + */
> + enable_xenon_internal_clk(host);
> +
> + if (priv->emmc_slot)
> + return xenon_emmc_signal_voltage_switch(mmc, ios);
> +
> + return sdhci_start_signal_voltage_switch(mmc, ios);
> +}
> +
> +/*
> + * After determining which slot is used for SDIO,
> + * some additional task is required.
> + */
> +static void xenon_init_card(struct mmc_host *mmc, struct mmc_card *card)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + u32 reg;
> + u8 slot_idx;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> + /* Link the card for delay adjustment */
> + priv->card_candidate = card;
> + /* Set tuning functionality of this slot */
> + xenon_slot_tuning_setup(host);
This looks weird. I assume this can be done as a part of the regular
tuning seqeunce!?
> +
> + slot_idx = priv->slot_idx;
> + if (!mmc_card_sdio(card)) {
> + /* Clear SDIO Card Inserted indication */
Why do you need this?
If you need to reset this, I think it's better to do it from
->set_ios() at MMC_POWER_OFF.
> + reg = sdhci_readl(host, SDHC_SYS_CFG_INFO);
> + reg &= ~(1 << (slot_idx + SLOT_TYPE_SDIO_SHIFT));
> + sdhci_writel(host, reg, SDHC_SYS_CFG_INFO);
> +
> + if (mmc_card_mmc(card)) {
> + mmc->caps |= MMC_CAP_NONREMOVABLE;
> + if (!(host->quirks2 & SDHCI_QUIRK2_NO_1_8_V))
> + mmc->caps |= MMC_CAP_1_8V_DDR;
> + /*
> + * Force to clear BUS_TEST to
> + * skip bus_test_pre and bus_test_post
> + */
> + mmc->caps &= ~MMC_CAP_BUS_WIDTH_TEST;
> + mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ |
> + MMC_CAP2_PACKED_CMD;
> + if (mmc->caps & MMC_CAP_8_BIT_DATA)
> + mmc->caps2 |= MMC_CAP2_HS400_1_8V;
Most of this can be specified as DT configurations. Please use that instead.
More importantly, please don't use the ->init_card() ops to assign
host caps. If not DT, please do it from ->probe().
> + }
> + } else {
> + /*
> + * Set SDIO Card Inserted indication
> + * to inform that the current slot is for SDIO
> + */
> + reg = sdhci_readl(host, SDHC_SYS_CFG_INFO);
> + reg |= (1 << (slot_idx + SLOT_TYPE_SDIO_SHIFT));
> + sdhci_writel(host, reg, SDHC_SYS_CFG_INFO);
So this makes sence to have in the ->init_card() ops. The rest above, not.
> + }
> +}
> +
> +static int xenon_execute_tuning(struct mmc_host *mmc, u32 opcode)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> +
> + if (host->timing == MMC_TIMING_UHS_DDR50)
> + return 0;
> +
> + return sdhci_execute_tuning(mmc, opcode);
> +}
> +
> +static void xenon_replace_mmc_host_ops(struct sdhci_host *host)
> +{
> + host->mmc_host_ops.set_ios = xenon_set_ios;
> + host->mmc_host_ops.start_signal_voltage_switch =
> + xenon_start_signal_voltage_switch;
> + host->mmc_host_ops.init_card = xenon_init_card;
> + host->mmc_host_ops.execute_tuning = xenon_execute_tuning;
> +}
> +
> +static int xenon_probe_dt(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct sdhci_host *host = platform_get_drvdata(pdev);
> + struct mmc_host *mmc = host->mmc;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + int err;
> + u32 slot_idx, nr_slot;
> + u32 tuning_count;
> + u32 reg;
> +
> + /* Standard MMC property */
> + err = mmc_of_parse(mmc);
> + if (err)
> + return err;
> +
> + /* Standard SDHCI property */
> + sdhci_get_of_property(pdev);
> +
> + /*
> + * Xenon Specific property:
> + * emmc: explicitly indicate whether this slot is for eMMC
> + * slotno: the index of slot. Refer to SDHC_SYS_CFG_INFO register
> + * tun-count: the interval between re-tuning
> + * PHY type: "sdhc phy", "emmc phy 5.0" or "emmc phy 5.1"
> + */
> + if (of_property_read_bool(np, "marvell,xenon-emmc"))
> + priv->emmc_slot = true;
So, you need this because of the eMMC voltage switch behaviour, right?
Then I would rather like to describe this a generic DT bindings for
the eMMC voltage level support. There have acutally been some earlier
discussions for this, but we haven't yet made some changes.
I think what is missing is a mmc-ddr-3_3v DT binding, which when set,
allows the host driver to accept I/O voltage switches to 3.3V. If not
supported the ->start_signal_voltage_switch() ops may return -EINVAL.
This would inform the mmc core to move on to the next supported
voltage level. There might be some minor additional changes to the mmc
card initialization sequence, but those should be simple.
I can help out to look into this, unless you want to do it yourself of course!?
> + else
> + priv->emmc_slot = false;
> +
> + if (!of_property_read_u32(np, "marvell,xenon-slotno", &slot_idx)) {
> + nr_slot = sdhci_readl(host, SDHC_SYS_CFG_INFO);
> + nr_slot &= NR_SUPPORTED_SLOT_MASK;
> + if (unlikely(slot_idx > nr_slot)) {
> + dev_err(mmc_dev(mmc), "Slot Index %d exceeds Number of slots %d\n",
> + slot_idx, nr_slot);
> + return -EINVAL;
> + }
> + } else {
> + priv->slot_idx = 0x0;
> + }
> +
> + if (!of_property_read_u32(np, "marvell,xenon-tun-count",
> + &tuning_count)) {
> + if (unlikely(tuning_count >= TMR_RETUN_NO_PRESENT)) {
> + dev_err(mmc_dev(mmc), "Wrong Re-tuning Count. Set default value %d\n",
> + DEF_TUNING_COUNT);
> + tuning_count = DEF_TUNING_COUNT;
> + }
> + } else {
> + priv->tuning_count = DEF_TUNING_COUNT;
> + }
To make the code a bit easier...
Maybe set "priv->tuning_count = DEF_TUNING_COUNT" before the "if", and
instead have the of_property_read_u32() to update the value when set.
> +
> + if (of_property_read_bool(np, "marvell,xenon-mask-conflict-err")) {
> + reg = sdhci_readl(host, SDHC_SYS_EXT_OP_CTRL);
> + reg |= MASK_CMD_CONFLICT_ERROR;
> + sdhci_writel(host, reg, SDHC_SYS_EXT_OP_CTRL);
> + }
> +
> + return err;
> +}
> +
> +static int xenon_slot_probe(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + u8 slot_idx = priv->slot_idx;
> +
> + /* Enable slot */
> + xenon_enable_slot(host, slot_idx);
> +
> + /* Enable ACG */
> + xenon_set_acg(host, true);
> +
> + /* Enable Parallel Transfer Mode */
> + xenon_enable_slot_parallel_tran(host, slot_idx);
> +
> + priv->timing = MMC_TIMING_FAKE;
> + priv->clock = 0;
What are these used for?
> +
> + return 0;
> +}
> +
> +static void xenon_slot_remove(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + u8 slot_idx = priv->slot_idx;
> +
> + /* disable slot */
> + xenon_disable_slot(host, slot_idx);
> +}
> +
> +static int sdhci_xenon_probe(struct platform_device *pdev)
> +{
> + struct sdhci_pltfm_host *pltfm_host;
> + struct sdhci_host *host;
> + struct clk *clk, *axi_clk;
> + struct sdhci_xenon_priv *priv;
> + int err;
> +
> + host = sdhci_pltfm_init(pdev, &sdhci_xenon_pdata,
> + sizeof(struct sdhci_xenon_priv));
> + if (IS_ERR(host))
> + return PTR_ERR(host);
> +
> + pltfm_host = sdhci_priv(host);
> + priv = sdhci_pltfm_priv(pltfm_host);
> +
> + xenon_set_acg(host, false);
> +
> + /*
> + * Link Xenon specific mmc_host_ops function,
> + * to replace standard ones in sdhci_ops.
> + */
> + xenon_replace_mmc_host_ops(host);
> +
> + clk = devm_clk_get(&pdev->dev, "core");
> + if (IS_ERR(clk)) {
> + dev_err(&pdev->dev, "Failed to setup input clk.\n");
> + err = PTR_ERR(clk);
> + goto free_pltfm;
> + }
> + clk_prepare_enable(clk);
Check error code.
> + pltfm_host->clk = clk;
Why not assign pltfm_host->clk immedately when doing devm_clk_get(),
that would make this a bit cleaner, right?
> +
> + /*
> + * Some SOCs require additional clock to
> + * manage AXI bus clock.
> + * It is optional.
> + */
> + axi_clk = devm_clk_get(&pdev->dev, "axi");
> + if (!IS_ERR(axi_clk)) {
> + clk_prepare_enable(axi_clk);
> + priv->axi_clk = axi_clk;
> + }
Same comments as for the above core clock.
> +
> + err = xenon_probe_dt(pdev);
> + if (err)
> + goto err_clk;
> +
> + err = xenon_slot_probe(host);
> + if (err)
> + goto err_clk;
> +
> + err = sdhci_add_host(host);
> + if (err)
> + goto remove_slot;
> +
> + return 0;
> +
> +remove_slot:
> + xenon_slot_remove(host);
> +err_clk:
> + clk_disable_unprepare(pltfm_host->clk);
> + if (!IS_ERR(axi_clk))
> + clk_disable_unprepare(axi_clk);
> +free_pltfm:
> + sdhci_pltfm_free(pdev);
> + return err;
> +}
> +
> +static int sdhci_xenon_remove(struct platform_device *pdev)
> +{
> + struct sdhci_host *host = platform_get_drvdata(pdev);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xFFFFFFFF);
> +
> + xenon_slot_remove(host);
> +
> + sdhci_remove_host(host, dead);
> +
> + clk_disable_unprepare(pltfm_host->clk);
> + clk_disable_unprepare(priv->axi_clk);
> +
> + sdhci_pltfm_free(pdev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sdhci_xenon_dt_ids[] = {
> + { .compatible = "marvell,xenon-sdhci",},
> + { .compatible = "marvell,armada-3700-sdhci",},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids);
> +
> +static struct platform_driver sdhci_xenon_driver = {
> + .driver = {
> + .name = "xenon-sdhci",
> + .of_match_table = sdhci_xenon_dt_ids,
> + .pm = &sdhci_pltfm_pmops,
> + },
> + .probe = sdhci_xenon_probe,
> + .remove = sdhci_xenon_remove,
> +};
> +
> +module_platform_driver(sdhci_xenon_driver);
> +
> +MODULE_DESCRIPTION("SDHCI platform driver for Marvell Xenon SDHC");
> +MODULE_AUTHOR("Hu Ziji <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mmc/host/sdhci-xenon.h b/drivers/mmc/host/sdhci-xenon.h
> new file mode 100644
> index 000000000000..4601d0a4b22f
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-xenon.h
I don't think you need a specific header for this, let's instead just
put everthing in the c-file.
> @@ -0,0 +1,142 @@
> +/*
> + * Copyright (C) 2016 Marvell, All Rights Reserved.
> + *
> + * Author: Hu Ziji <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> + * Date: 2016-8-24
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + */
> +#ifndef SDHCI_XENON_H_
> +#define SDHCI_XENON_H_
> +
> +#include <linux/clk.h>
> +#include <linux/mmc/card.h>
> +#include <linux/of.h>
> +#include "sdhci.h"
> +
> +/* Register Offset of SD Host Controller SOCP self-defined register */
> +#define SDHC_SYS_CFG_INFO 0x0104
> +#define SLOT_TYPE_SDIO_SHIFT 24
> +#define SLOT_TYPE_EMMC_MASK 0xFF
> +#define SLOT_TYPE_EMMC_SHIFT 16
> +#define SLOT_TYPE_SD_SDIO_MMC_MASK 0xFF
> +#define SLOT_TYPE_SD_SDIO_MMC_SHIFT 8
> +#define NR_SUPPORTED_SLOT_MASK 0x7
> +
> +#define SDHC_SYS_OP_CTRL 0x0108
> +#define AUTO_CLKGATE_DISABLE_MASK BIT(20)
> +#define SDCLK_IDLEOFF_ENABLE_SHIFT 8
> +#define SLOT_ENABLE_SHIFT 0
> +
> +#define SDHC_SYS_EXT_OP_CTRL 0x010C
> +#define MASK_CMD_CONFLICT_ERROR BIT(8)
> +
> +#define SDHC_SLOT_OP_STATUS_CTRL 0x0128
> +#define DELAY_90_DEGREE_MASK_EMMC5 BIT(7)
> +#define DELAY_90_DEGREE_SHIFT_EMMC5 7
> +#define EMMC_5_0_PHY_FIXED_DELAY_MASK 0x7F
> +#define EMMC_PHY_FIXED_DELAY_MASK 0xFF
> +#define EMMC_PHY_FIXED_DELAY_WINDOW_MIN (EMMC_PHY_FIXED_DELAY_MASK >> 3)
> +#define SDH_PHY_FIXED_DELAY_MASK 0x1FF
> +#define SDH_PHY_FIXED_DELAY_WINDOW_MIN (SDH_PHY_FIXED_DELAY_MASK >> 4)
> +
> +#define TUN_CONSECUTIVE_TIMES_SHIFT 16
> +#define TUN_CONSECUTIVE_TIMES_MASK 0x7
> +#define TUN_CONSECUTIVE_TIMES 0x4
> +#define TUNING_STEP_SHIFT 12
> +#define TUNING_STEP_MASK 0xF
> +#define TUNING_STEP_DIVIDER BIT(6)
> +
> +#define FORCE_SEL_INVERSE_CLK_SHIFT 11
> +
> +#define SDHC_SLOT_EMMC_CTRL 0x0130
> +#define ENABLE_DATA_STROBE BIT(24)
> +#define SET_EMMC_RSTN BIT(16)
> +#define DISABLE_RD_DATA_CRC BIT(14)
> +#define DISABLE_CRC_STAT_TOKEN BIT(13)
> +#define EMMC_VCCQ_MASK 0x3
> +#define EMMC_VCCQ_1_8V 0x1
> +#define EMMC_VCCQ_3_3V 0x3
> +
> +#define SDHC_SLOT_RETUNING_REQ_CTRL 0x0144
> +/* retuning compatible */
> +#define RETUNING_COMPATIBLE 0x1
> +
> +#define SDHC_SLOT_EXT_PRESENT_STATE 0x014C
> +#define LOCK_STATE 0x1
> +
> +#define SDHC_SLOT_DLL_CUR_DLY_VAL 0x0150
> +
> +/* Tuning Parameter */
> +#define TMR_RETUN_NO_PRESENT 0xF
> +#define DEF_TUNING_COUNT 0x9
> +
> +#define MMC_TIMING_FAKE 0xFF
> +
> +#define DEFAULT_SDCLK_FREQ (400000)
> +
> +/* Xenon specific Mode Select value */
> +#define XENON_SDHCI_CTRL_HS200 0x5
> +#define XENON_SDHCI_CTRL_HS400 0x6
For all defines above:
All these defines needs some *SDHCI* prefix. Can you please update that.
> +
> +struct sdhci_xenon_priv {
> + /*
> + * The bus_width, timing, and clock fields in below
> + * record the current setting of Xenon SDHC.
> + * Driver will call a Sampling Fixed Delay Adjustment
> + * if any setting is changed.
> + */
> + unsigned char bus_width;
> + unsigned char timing;
These two are not used. Please remove.
> + unsigned char tuning_count;
> + unsigned int clock;
"clock" isn't used, please remove.
> + struct clk *axi_clk;
> +
> + /* Slot idx */
> + u8 slot_idx;
> + /* Whether this slot is for eMMC */
> + bool emmc_slot;
> +
> + /*
> + * When initializing card, Xenon has to determine card type and
> + * adjust Sampling Fixed delay for the speed mode in which
> + * DLL tuning is not support.
> + * However, at that time, card structure is not linked to mmc_host.
> + * Thus a card pointer is added here to provide
> + * the delay adjustment function with the card structure
> + * of the card during initialization.
> + *
> + * It is only valid during initialization after it is updated in
> + * xenon_init_card().
> + * Do not access this variable in normal transfers after
> + * initialization completes.
> + */
> + struct mmc_card *card_candidate;
Not activley used in this change, please remove and let's discuss it
in the next step.
> +};
> +
> +static inline int enable_xenon_internal_clk(struct sdhci_host *host)
> +{
> + u32 reg;
> + u8 timeout;
> +
> + reg = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
> + reg |= SDHCI_CLOCK_INT_EN;
> + sdhci_writel(host, reg, SDHCI_CLOCK_CONTROL);
> + /* Wait max 20 ms */
> + timeout = 20;
> + while (!((reg = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
> + & SDHCI_CLOCK_INT_STABLE)) {
> + if (timeout == 0) {
> + pr_err("%s: Internal clock never stabilised.\n",
> + mmc_hostname(host->mmc));
> + return -ETIMEDOUT;
> + }
> + timeout--;
> + mdelay(1);
> + }
> +
> + return 0;
> +}
> +#endif
> --
> git-series 0.8.10
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox