lkml.org 
[lkml]   [2016]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 04/11] mmc: sdhci-of-arasan: Properly set corecfg_baseclkfreq on rk3399
From
Date
在 2016/6/8 6:44, Douglas Anderson 写道:
> In the the earlier change in this series ("Documentation: mmc:
> sdhci-of-arasan: Add soc-ctl-syscon for corecfg regs") we can see the
> mechansim for specifying a syscon to properly set corecfg registers in
> sdhci-of-arasan. Now let's use this mechanism to properly set
> corecfg_baseclkfreq on rk3399.
>
>>From [1] the corecfg_baseclkfreq is supposed to be set to:
> Base Clock Frequency for SD Clock.
> This is the frequency of the xin_clk.
>
> This is a relatively easy thing to do. Note that we assume that xin_clk
> is not dynamic and we can check the clock at probe time. If any real
> devices have a dynamic xin_clk future patches could register for
> notifiers for the clock.
>
> At the moment, setting corecfg_baseclkfreq is only supported for rk3399
> since we need a specific map for each implementation. The code is
> written in a generic way that should make this easy to extend to other
> SoCs. Note that a specific compatible string for rk3399 is already in
> use and so we add that to the table to match rk3399.
>
> [1]: https://arasan.com/wp-content/media/eMMC-5-1-Total-Solution_Rev-1-3.pdf
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> drivers/mmc/host/sdhci-of-arasan.c | 189 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 178 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index 3ff1711077c2..859ea1b21215 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -22,6 +22,8 @@
> #include <linux/module.h>
> #include <linux/of_device.h>
> #include <linux/phy/phy.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>

alphabetical order

> #include "sdhci-pltfm.h"
>
> #define SDHCI_ARASAN_CLK_CTRL_OFFSET 0x2c
> @@ -32,18 +34,115 @@
> #define CLK_CTRL_TIMEOUT_MASK (0xf << CLK_CTRL_TIMEOUT_SHIFT)
> #define CLK_CTRL_TIMEOUT_MIN_EXP 13
>
> +/*
> + * On some SoCs the syscon area has a feature where the upper 16-bits of
> + * each 32-bit register act as a write mask for the lower 16-bits. This allows
> + * atomic updates of the register without locking. This macro is used on SoCs
> + * that have that feature.
> + */
> +#define HIWORD_UPDATE(val, mask, shift) \
> + ((val) << (shift) | (mask) << ((shift) + 16))
> +
> +/**
> + * struct sdhci_arasan_soc_ctl_field - Field used in sdhci_arasan_soc_ctl_map
> + *
> + * @reg: Offset within the syscon of the register containing this field
> + * @width: Number of bits for this field
> + * @shift: Bit offset within @reg of this field (or -1 if not avail)
> + */
> +struct sdhci_arasan_soc_ctl_field {
> + u32 reg;
> + u16 width;
> + s16 shift;
> +};
> +
> +/**
> + * struct sdhci_arasan_soc_ctl_map - Map in syscon to corecfg registers
> + *
> + * It's up to the licensee of the Arsan IP block to make these available
> + * somewhere if needed. Presumably these will be scattered somewhere that's
> + * accessible via the syscon API.
> + *
> + * @baseclkfreq: Where to find corecfg_baseclkfreq
> + * @hiword_update: If true, use HIWORD_UPDATE to access the syscon
> + */
> +struct sdhci_arasan_soc_ctl_map {
> + struct sdhci_arasan_soc_ctl_field baseclkfreq;
> + bool hiword_update;
> +};
> +
> /**
> * struct sdhci_arasan_data
> - * @clk_ahb: Pointer to the AHB clock
> - * @phy: Pointer to the generic phy
> - * @phy_on: True if the PHY is turned on.
> + * @clk_ahb: Pointer to the AHB clock
> + * @phy: Pointer to the generic phy
> + * @phy_on: True if the PHY is turned on.
> + * @soc_ctl_base: Pointer to regmap for syscon for soc_ctl registers.
> + * @soc_ctl_map: Map to get offsets into soc_ctl registers.
> */
> struct sdhci_arasan_data {
> struct clk *clk_ahb;
> struct phy *phy;
> bool phy_on;
> +
> + struct regmap *soc_ctl_base;
> + const struct sdhci_arasan_soc_ctl_map *soc_ctl_map;
> +};
> +
> +static const struct sdhci_arasan_soc_ctl_map rk3399_soc_ctl_map = {
> + .baseclkfreq = { .reg = 0xf000, .width = 8, .shift = 8 },
> + .hiword_update = true,
> };
>
> +/**
> + * sdhci_arasan_syscon_write - Write to a field in soc_ctl registers
> + *
> + * This function allows writing to fields in sdhci_arasan_soc_ctl_map.
> + * Note that if a field is specified as not available (shift < 0) then
> + * this function will silently return an error code. It will be noisy
> + * and print errors for any other (unexpected) errors.
> + *
> + * @host: The sdhci_host
> + * @fld: The field to write to
> + * @val: The value to write
> + */
> +static int sdhci_arasan_syscon_write(struct sdhci_host *host,
> + const struct sdhci_arasan_soc_ctl_field *fld,
> + u32 val)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
> + struct regmap *soc_ctl_base = sdhci_arasan->soc_ctl_base;
> + u32 reg = fld->reg;
> + u16 width = fld->width;
> + s16 shift = fld->shift;
> + int ret;
> +
> + /*
> + * Silently return errors for shift < 0 so caller doesn't have
> + * to check for fields which are optional. For fields that
> + * are required then caller needs to do something special
> + * anyway.
> + */
> + if (shift < 0)
> + return -EINVAL;
> +
> + if (sdhci_arasan->soc_ctl_map->hiword_update)
> + ret = regmap_write(soc_ctl_base, reg,
> + HIWORD_UPDATE(val, GENMASK(width, 0),
> + shift));
> + else
> + ret = regmap_update_bits(soc_ctl_base, reg,
> + GENMASK(shift + width, shift),
> + val << shift);
> +
> + /* Yell about (unexpected) regmap errors */
> + if (ret)
> + pr_warn("%s: Regmap write fail: %d\n",
> + mmc_hostname(host->mmc), ret);
> +
> + return ret;
> +}
> +
> static unsigned int sdhci_arasan_get_timeout_clock(struct sdhci_host *host)
> {
> u32 div;
> @@ -191,9 +290,66 @@ static int sdhci_arasan_resume(struct device *dev)
> static SIMPLE_DEV_PM_OPS(sdhci_arasan_dev_pm_ops, sdhci_arasan_suspend,
> sdhci_arasan_resume);
>
> +static const struct of_device_id sdhci_arasan_of_match[] = {
> + /* SoC-specific compatible strings w/ soc_ctl_map */
> + {
> + .compatible = "rockchip,rk3399-sdhci-5.1",
> + .data = &rk3399_soc_ctl_map,
> + },
> +
> + /* Generic compatible below here */
> + { .compatible = "arasan,sdhci-8.9a" },
> + { .compatible = "arasan,sdhci-5.1" },
> + { .compatible = "arasan,sdhci-4.9a" },
> +
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_arasan_of_match);
> +
> +/**
> + * sdhci_arasan_update_baseclkfreq - Set corecfg_baseclkfreq
> + *
> + * The corecfg_baseclkfreq is supposed to contain the MHz of clk_xin. This
> + * function can be used to make that happen.
> + *
> + * NOTES:
> + * - Many existing devices don't seem to do this and work fine. To keep
> + * compatibility for old hardware where the device tree doesn't provide a
> + * register map, this function is a noop if a soc_ctl_map hasn't been provided
> + * for this platform.
> + * - It's assumed that clk_xin is not dynamic and that we use the SDHCI divider
> + * to achieve lower clock rates. That means that this function is called once
> + * at probe time and never called again.
> + *
> + * @host: The sdhci_host
> + */
> +static void sdhci_arasan_update_baseclkfreq(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
> + const struct sdhci_arasan_soc_ctl_map *soc_ctl_map =
> + sdhci_arasan->soc_ctl_map;
> + u32 mhz = DIV_ROUND_CLOSEST(clk_get_rate(pltfm_host->clk), 1000000);
> +

It's broken when reading capabilities reg on RK3399 platform
which means you should get it via clk framework. But you should consider
the non-broken case.

> + /* Having a map is optional */
> + if (!soc_ctl_map)
> + return;
> +
> + /* If we have a map, we expect to have a syscon */
> + if (!sdhci_arasan->soc_ctl_base) {
> + pr_warn("%s: Have regmap, but no soc-ctl-syscon\n",
> + mmc_hostname(host->mmc));
> + return;
> + }
> +
> + sdhci_arasan_syscon_write(host, &soc_ctl_map->baseclkfreq, mhz);

should we check the return value, and if not -EINVAL, we should give
another try?

> +}
> +
> static int sdhci_arasan_probe(struct platform_device *pdev)
> {
> int ret;
> + const struct of_device_id *match;
> + struct device_node *node;
> struct clk *clk_xin;
> struct sdhci_host *host;
> struct sdhci_pltfm_host *pltfm_host;
> @@ -207,6 +363,23 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
> pltfm_host = sdhci_priv(host);
> sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
>
> + match = of_match_node(sdhci_arasan_of_match, pdev->dev.of_node);
> + sdhci_arasan->soc_ctl_map = match->data;
> +
> + node = of_parse_phandle(pdev->dev.of_node, "arasan,soc-ctl-syscon", 0);

should it be within the scope of arasan,sdhci-5.1 or
rockchip,rk3399-sdhci-5.1?

> + if (node) {
> + sdhci_arasan->soc_ctl_base = syscon_node_to_regmap(node);
> + of_node_put(node);
> +
> + if (IS_ERR(sdhci_arasan->soc_ctl_base)) {
> + ret = PTR_ERR(sdhci_arasan->soc_ctl_base);
> + if (ret != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "Can't get syscon: %d\n",
> + ret);
> + goto err_pltfm_free;
> + }
> + }
> +
> sdhci_arasan->clk_ahb = devm_clk_get(&pdev->dev, "clk_ahb");
> if (IS_ERR(sdhci_arasan->clk_ahb)) {
> dev_err(&pdev->dev, "clk_ahb clock not found.\n");
> @@ -236,6 +409,8 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
> sdhci_get_of_property(pdev);
> pltfm_host->clk = clk_xin;
>
> + sdhci_arasan_update_baseclkfreq(host);
> +
> ret = mmc_of_parse(host->mmc);
> if (ret) {
> dev_err(&pdev->dev, "parsing dt failed (%u)\n", ret);
> @@ -301,14 +476,6 @@ static int sdhci_arasan_remove(struct platform_device *pdev)
> return ret;
> }
>
> -static const struct of_device_id sdhci_arasan_of_match[] = {
> - { .compatible = "arasan,sdhci-8.9a" },
> - { .compatible = "arasan,sdhci-5.1" },
> - { .compatible = "arasan,sdhci-4.9a" },
> - { }
> -};
> -MODULE_DEVICE_TABLE(of, sdhci_arasan_of_match);
> -
> static struct platform_driver sdhci_arasan_driver = {
> .driver = {
> .name = "sdhci-arasan",
>


--
Best Regards
Shawn Lin

\
 
 \ /
  Last update: 2016-06-13 11:21    [W:0.222 / U:1.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site