On Mon, Aug 19, 2019 at 11:44:16AM +0800, Ramuthevar,Vadivel MuruganX wrote:Thanks Andy, agreed.
From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@xxxxxxxxxxxxxxx>Adds -> Add.
Adds support for eMMC PHY on Intel's Lightning Mountain SoC.
agreed.+/* eMMC phy register definitions */BIT(0)
+#define EMMC_PHYCTRL0_REG 0xa8
+#define DR_TY_MASK GENMASK(30, 28)
+#define DR_TY_50OHM(x) ((~(x) << 28) & DR_TY_MASK)
+#define OTAPDLYENA BIT(14)
+#define OTAPDLYSEL_MASK GENMASK(13, 10)
+#define OTAPDLYSEL_SHIFT(x) (((x) << 10) & OTAPDLYSEL_MASK)
+
+#define EMMC_PHYCTRL1_REG 0xac
+#define PDB_MASK 1
Agreed, will update.+#define ENDLL_MASK BIT(7)These are inconsistent with above:
+#define ENDLL_VAL BIT(7)
+
+#define EMMC_PHYCTRL2_REG 0xb0
+#define FRQSEL_25M 0
+#define FRQSEL_150M 3
+#define FRQSEL_MASK GENMASK(24, 22)
+#define FRQSEL_SHIFT(x) ((x) << 22)
+
+#define EMMC_PHYSTAT_REG 0xbc
+#define CALDONE_MASK 1
+#define DLLRDY_MASK 1
+#define IS_CALDONE(x) ((((x) >> 9) & CALDONE_MASK) == 1)
+#define IS_DLLRDY(x) ((((x) >> 8) & DLLRDY_MASK) == 1)
#define CALDONE_MASK BIT(9)
...
#define IS_CALDONE ((x) & CALDONE_MASK)
Note redundant == part.
Agreed, re-structure the sentence.+static int intel_emmc_phy_power(struct phy *phy, bool on_off)
+{
+ * - PHY driver to probe
+ * - SDHCI driver to start probe
+ * - SDHCI driver to register it's clock
+ * - SDHCI driver to get the PHY
+ * - SDHCI driver to init the PHY
+ *
+ * The clock is optional, so upon any error we just set to NULL.No, the clock framework will do it for you.
+ *This comment is not correct...
+ * NOTE: we don't do anything special for EPROBE_DEFER here. Given the
+ * above expected use case, EPROBE_DEFER isn't sensible to expect, so
+ * it's just like any other error.
Agreed.+ */...because here you have to return an error...
+ priv->emmcclk = clk_get_optional(&phy->dev, "emmcclk");
+ if (IS_ERR(priv->emmcclk)) {
+ dev_warn(&phy->dev, "ERROR: getting emmcclk\n");
Agreed.+ priv->emmcclk = NULL;...and here is redundant assignment.
Thank you so much for the review comments, sure I will recheck and recollect your review comments of different patches+ }When you send out patches, check that you do this for latest version you got reviewed internally.
+
+ return 0;
+}