From: Ulf Hansson <ulf.hansson@linaro.org>
To: Andrew Jeffery <andrew@aj.id.au>
Cc: "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
Rob Herring <robh+dt@kernel.org>, Joel Stanley <joel@jms.id.au>,
Adrian Hunter <adrian.hunter@intel.com>,
DTML <devicetree@vger.kernel.org>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
linux-aspeed <linux-aspeed@lists.ozlabs.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
ryan_chen@aspeedtech.com
Subject: Re: [PATCH v3 1/3] mmc: sdhci-of-aspeed: Expose phase delay tuning
Date: Tue, 24 Nov 2020 15:12:17 +0100 [thread overview]
Message-ID: <CAPDyKFrC9vp5gtpFC5L1K17uN059GsJ2zF4f7-_=sFEQ5BBRpw@mail.gmail.com> (raw)
In-Reply-To: <20201123063004.337345-2-andrew@aj.id.au>
On Mon, 23 Nov 2020 at 07:30, Andrew Jeffery <andrew@aj.id.au> wrote:
>
> The Aspeed SD/eMMC controllers feature up to two SDHCIs alongside a
> a set of "global" configuration registers. The global configuration
> registers house controller-specific settings that aren't exposed by the
> SDHCI, one example being a register for phase tuning.
>
> The phase tuning feature is new in the AST2600 design. It's exposed as a
> single register in the global register set and controls both the input
> and output phase adjustment for each slot. As the settings are
> slot-specific, the values to program are extracted from properties in
> the SDHCI devicetree nodes.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
[...]
>
> +static void
> +aspeed_sdhci_of_parse_phase(struct device_node *np, const char *prop,
> + struct aspeed_sdhci_phase_param *phase)
> +{
> + int degrees[2] = {0};
> + int rc;
> +
> + rc = of_property_read_variable_u32_array(np, prop, degrees, 2, 0);
> + phase->set = rc == 2;
> + if (phase->set) {
> + phase->in_deg = degrees[0];
> + phase->out_deg = degrees[1];
> + }
> +}
> +
> +static int aspeed_sdhci_of_parse(struct platform_device *pdev,
> + struct aspeed_sdhci *sdhci)
> +{
> + struct device_node *np;
> + struct device *dev;
> +
> + if (!sdhci->phase_desc)
> + return 0;
> +
> + dev = &pdev->dev;
> + np = dev->of_node;
> +
> + aspeed_sdhci_of_parse_phase(np, "clk-phase-legacy",
> + &sdhci->phase_param[MMC_TIMING_LEGACY]);
> + aspeed_sdhci_of_parse_phase(np, "clk-phase-mmc-hs",
> + &sdhci->phase_param[MMC_TIMING_MMC_HS]);
> + aspeed_sdhci_of_parse_phase(np, "clk-phase-sd-hs",
> + &sdhci->phase_param[MMC_TIMING_SD_HS]);
> + aspeed_sdhci_of_parse_phase(np, "clk-phase-uhs-sdr12",
> + &sdhci->phase_param[MMC_TIMING_UHS_SDR12]);
> + aspeed_sdhci_of_parse_phase(np, "clk-phase-uhs-sdr25",
> + &sdhci->phase_param[MMC_TIMING_UHS_SDR25]);
> + aspeed_sdhci_of_parse_phase(np, "clk-phase-uhs-sdr50",
> + &sdhci->phase_param[MMC_TIMING_UHS_SDR50]);
> + aspeed_sdhci_of_parse_phase(np, "clk-phase-uhs-sdr104",
> + &sdhci->phase_param[MMC_TIMING_UHS_SDR104]);
> + aspeed_sdhci_of_parse_phase(np, "clk-phase-uhs-ddr50",
> + &sdhci->phase_param[MMC_TIMING_UHS_DDR50]);
> + aspeed_sdhci_of_parse_phase(np, "clk-phase-mmc-ddr52",
> + &sdhci->phase_param[MMC_TIMING_MMC_DDR52]);
> + aspeed_sdhci_of_parse_phase(np, "clk-phase-mmc-hs200",
> + &sdhci->phase_param[MMC_TIMING_MMC_HS200]);
> +
> + return 0;
> +}
If it's not too much to ask, would you mind adding a helper function
to the mmc core, as to let us avoid open coding? Then we should be
able to move the sdhci-of-arasan driver to use this as well.
Perhaps the definition of the helper could look something like this:
int mmc_of_parse_clk_phase(struct mmc_host *host, struct mmc_clk_phase
*phases) (or something along those lines)
I think the struct mmc_clk_phase could be something that is stored in
the host specific struct, rather than in the common struct mmc_host
(to avoid sprinkle it with unnecessary data).
Moreover, we should probably use the device_property_* APIs instead of
the DT specific of_property_*.
[...]
Kind regards
Uffe
next prev parent reply other threads:[~2020-11-24 14:12 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-23 6:30 [PATCH v3 0/3] mmc: sdhci-of-aspeed: Expose phase delay tuning Andrew Jeffery
2020-11-23 6:30 ` [PATCH v3 1/3] " Andrew Jeffery
2020-11-24 14:12 ` Ulf Hansson [this message]
2020-11-25 22:52 ` Andrew Jeffery
2020-11-23 6:30 ` [PATCH v3 2/3] mmc: sdhci-of-aspeed: Add AST2600 bus clock support Andrew Jeffery
2020-11-23 6:30 ` [PATCH v3 3/3] ARM: dts: rainier: Add eMMC clock phase compensation Andrew Jeffery
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAPDyKFrC9vp5gtpFC5L1K17uN059GsJ2zF4f7-_=sFEQ5BBRpw@mail.gmail.com' \
--to=ulf.hansson@linaro.org \
--cc=adrian.hunter@intel.com \
--cc=andrew@aj.id.au \
--cc=devicetree@vger.kernel.org \
--cc=joel@jms.id.au \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-aspeed@lists.ozlabs.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=robh+dt@kernel.org \
--cc=ryan_chen@aspeedtech.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).