On Mon, 23 Nov 2020 at 07:30, Andrew Jeffery [off-list ref] 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 <redacted>

[...]

+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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel