RE: [PATCH v6 2/2] phy: intel: Add Keem Bay eMMC PHY support
Hi Vinod. Thanks for the review. > -Original Message- > From: Vinod Koul > Sent: Monday, July 13, 2020 1:40 PM > To: Wan Mohamad, Wan Ahmad Zainie > > Cc: kis...@ti.com; robh...@kernel.org; Shevchenko, Andriy > ; linux-kernel@vger.kernel.org; > devicet...@vger.kernel.org; MP, Sureshkumar > ; Raja Subramanian, Lakshmi Bai > > Subject: Re: [PATCH v6 2/2] phy: intel: Add Keem Bay eMMC PHY support > > On 02-07-20, 08:09, Wan Ahmad Zainie wrote: > > Add support for eMMC PHY on Intel Keem Bay SoC. > > > > Signed-off-by: Wan Ahmad Zainie > > > > --- > > drivers/phy/intel/Kconfig| 12 + > > drivers/phy/intel/Makefile | 1 + > > drivers/phy/intel/phy-keembay-emmc.c | 314 > > +++ > > 3 files changed, 327 insertions(+) > > create mode 100644 drivers/phy/intel/phy-keembay-emmc.c > > > > diff --git a/drivers/phy/intel/Kconfig b/drivers/phy/intel/Kconfig > > index 7b47682a4e0e..8ddda4fb95d2 100644 > > --- a/drivers/phy/intel/Kconfig > > +++ b/drivers/phy/intel/Kconfig > > @@ -22,3 +22,15 @@ config PHY_INTEL_EMMC > > select GENERIC_PHY > > help > > Enable this to support the Intel EMMC PHY > > + > > +config PHY_KEEMBAY_EMMC > > Pls keep this in alphabetical sort PHY_INTEL_ followed by PHY_KEEMBAY_ is alphabetically sorted. Could you please help to clarify? > > > + tristate "Intel Keem Bay EMMC PHY driver" > > + depends on ARM64 || COMPILE_TEST > > Intel and ARM64, aha, fun times! 😊 > > > + depends on OF && HAS_IOMEM > > + select GENERIC_PHY > > + select REGMAP_MMIO > > + help > > + Choose this option if you have an Intel Keem Bay SoC. > > + > > + To compile this driver as a module, choose M here: the module > > + will be called phy-keembay-emmc. > > phy-keembay-emmc.ko ? Is it a must? I saw few Kconfig files omit .ko. I can fix this in next version. > > > diff --git a/drivers/phy/intel/Makefile b/drivers/phy/intel/Makefile > > index 233d530dadde..6566334e7b77 100644 > > --- a/drivers/phy/intel/Makefile > > +++ b/drivers/phy/intel/Makefile > > @@ -1,3 +1,4 @@ > > # SPDX-License-Identifier: GPL-2.0 > > obj-$(CONFIG_PHY_INTEL_COMBO) += phy-intel-combo.o > > obj-$(CONFIG_PHY_INTEL_EMMC)+= phy-intel-emmc.o > > +obj-$(CONFIG_PHY_KEEMBAY_EMMC) += phy-keembay- > emmc.o > > here as well > > > +/* eMMC/SD/SDIO core/phy configuration registers */ > > +#define PHY_CFG_0 0x24 > > +#define SEL_DLY_TXCLK_MASKBIT(29) > > +#define SEL_DLY_TXCLK(x) (((x) << 29) & SEL_DLY_TXCLK_MASK) > > +#define OTAP_DLY_ENA_MASK BIT(27) > > +#define OTAP_DLY_ENA(x) (((x) << 27) & OTAP_DLY_ENA_MASK) > > +#define OTAP_DLY_SEL_MASK GENMASK(26, 23) > > +#define OTAP_DLY_SEL(x) (((x) << 23) & OTAP_DLY_SEL_MASK) > > why not a generic helper to do (x) << ffs(reg - 1) & reg ? > You can skip defining for each register that way! Is it something like this following? #define maskval(mask, val) (((val) << (ffs(mask) - 1)) & mask) > > -- > ~Vinod
Re: [PATCH v6 2/2] phy: intel: Add Keem Bay eMMC PHY support
On 02-07-20, 08:09, Wan Ahmad Zainie wrote: > Add support for eMMC PHY on Intel Keem Bay SoC. > > Signed-off-by: Wan Ahmad Zainie > --- > drivers/phy/intel/Kconfig| 12 + > drivers/phy/intel/Makefile | 1 + > drivers/phy/intel/phy-keembay-emmc.c | 314 +++ > 3 files changed, 327 insertions(+) > create mode 100644 drivers/phy/intel/phy-keembay-emmc.c > > diff --git a/drivers/phy/intel/Kconfig b/drivers/phy/intel/Kconfig > index 7b47682a4e0e..8ddda4fb95d2 100644 > --- a/drivers/phy/intel/Kconfig > +++ b/drivers/phy/intel/Kconfig > @@ -22,3 +22,15 @@ config PHY_INTEL_EMMC > select GENERIC_PHY > help > Enable this to support the Intel EMMC PHY > + > +config PHY_KEEMBAY_EMMC Pls keep this in alphabetical sort > + tristate "Intel Keem Bay EMMC PHY driver" > + depends on ARM64 || COMPILE_TEST Intel and ARM64, aha, fun times! > + depends on OF && HAS_IOMEM > + select GENERIC_PHY > + select REGMAP_MMIO > + help > + Choose this option if you have an Intel Keem Bay SoC. > + > + To compile this driver as a module, choose M here: the module > + will be called phy-keembay-emmc. phy-keembay-emmc.ko ? > diff --git a/drivers/phy/intel/Makefile b/drivers/phy/intel/Makefile > index 233d530dadde..6566334e7b77 100644 > --- a/drivers/phy/intel/Makefile > +++ b/drivers/phy/intel/Makefile > @@ -1,3 +1,4 @@ > # SPDX-License-Identifier: GPL-2.0 > obj-$(CONFIG_PHY_INTEL_COMBO)+= phy-intel-combo.o > obj-$(CONFIG_PHY_INTEL_EMMC)+= phy-intel-emmc.o > +obj-$(CONFIG_PHY_KEEMBAY_EMMC) += phy-keembay-emmc.o here as well > +/* eMMC/SD/SDIO core/phy configuration registers */ > +#define PHY_CFG_00x24 > +#define SEL_DLY_TXCLK_MASK BIT(29) > +#define SEL_DLY_TXCLK(x)(((x) << 29) & SEL_DLY_TXCLK_MASK) > +#define OTAP_DLY_ENA_MASK BIT(27) > +#define OTAP_DLY_ENA(x) (((x) << 27) & OTAP_DLY_ENA_MASK) > +#define OTAP_DLY_SEL_MASK GENMASK(26, 23) > +#define OTAP_DLY_SEL(x) (((x) << 23) & OTAP_DLY_SEL_MASK) why not a generic helper to do (x) << ffs(reg - 1) & reg ? You can skip defining for each register that way! -- ~Vinod
[PATCH v6 2/2] phy: intel: Add Keem Bay eMMC PHY support
Add support for eMMC PHY on Intel Keem Bay SoC. Signed-off-by: Wan Ahmad Zainie --- drivers/phy/intel/Kconfig| 12 + drivers/phy/intel/Makefile | 1 + drivers/phy/intel/phy-keembay-emmc.c | 314 +++ 3 files changed, 327 insertions(+) create mode 100644 drivers/phy/intel/phy-keembay-emmc.c diff --git a/drivers/phy/intel/Kconfig b/drivers/phy/intel/Kconfig index 7b47682a4e0e..8ddda4fb95d2 100644 --- a/drivers/phy/intel/Kconfig +++ b/drivers/phy/intel/Kconfig @@ -22,3 +22,15 @@ config PHY_INTEL_EMMC select GENERIC_PHY help Enable this to support the Intel EMMC PHY + +config PHY_KEEMBAY_EMMC + tristate "Intel Keem Bay EMMC PHY driver" + depends on ARM64 || COMPILE_TEST + depends on OF && HAS_IOMEM + select GENERIC_PHY + select REGMAP_MMIO + help + Choose this option if you have an Intel Keem Bay SoC. + + To compile this driver as a module, choose M here: the module + will be called phy-keembay-emmc. diff --git a/drivers/phy/intel/Makefile b/drivers/phy/intel/Makefile index 233d530dadde..6566334e7b77 100644 --- a/drivers/phy/intel/Makefile +++ b/drivers/phy/intel/Makefile @@ -1,3 +1,4 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_PHY_INTEL_COMBO) += phy-intel-combo.o obj-$(CONFIG_PHY_INTEL_EMMC)+= phy-intel-emmc.o +obj-$(CONFIG_PHY_KEEMBAY_EMMC) += phy-keembay-emmc.o diff --git a/drivers/phy/intel/phy-keembay-emmc.c b/drivers/phy/intel/phy-keembay-emmc.c new file mode 100644 index ..66b5931a749a --- /dev/null +++ b/drivers/phy/intel/phy-keembay-emmc.c @@ -0,0 +1,314 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Intel Keem Bay eMMC PHY driver + * Copyright (C) 2020 Intel Corporation + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* eMMC/SD/SDIO core/phy configuration registers */ +#define PHY_CFG_0 0x24 +#define SEL_DLY_TXCLK_MASKBIT(29) +#define SEL_DLY_TXCLK(x) (((x) << 29) & SEL_DLY_TXCLK_MASK) +#define OTAP_DLY_ENA_MASK BIT(27) +#define OTAP_DLY_ENA(x) (((x) << 27) & OTAP_DLY_ENA_MASK) +#define OTAP_DLY_SEL_MASK GENMASK(26, 23) +#define OTAP_DLY_SEL(x) (((x) << 23) & OTAP_DLY_SEL_MASK) +#define DLL_EN_MASK BIT(10) +#define DLL_EN(x) (((x) << 10) & DLL_EN_MASK) +#define PWR_DOWN_MASK BIT(0) +#define PWR_DOWN(x) (((x) << 0) & PWR_DOWN_MASK) + +#define PHY_CFG_2 0x2c +#define SEL_FREQ_MASK GENMASK(12, 10) +#define SEL_FREQ(x) (((x) << 10) & SEL_FREQ_MASK) + +#define PHY_STAT 0x40 +#define CAL_DONE_MASK BIT(6) +#define IS_CALDONE(x) ((x) & CAL_DONE_MASK) +#define DLL_RDY_MASK BIT(5) +#define IS_DLLRDY(x) ((x) & DLL_RDY_MASK) + +/* From ACS_eMMC51_16nFFC_RO1100_Userguide_v1p0.pdf p17 */ +#define FREQSEL_200M_170M 0x0 +#define FREQSEL_170M_140M 0x1 +#define FREQSEL_140M_110M 0x2 +#define FREQSEL_110M_80M 0x3 +#define FREQSEL_80M_50M0x4 + +struct keembay_emmc_phy { + struct regmap *syscfg; + struct clk *emmcclk; +}; + +static const struct regmap_config keembay_regmap_config = { + .reg_bits = 32, + .val_bits = 32, + .reg_stride = 4, +}; + +static int keembay_emmc_phy_power(struct phy *phy, bool on_off) +{ + struct keembay_emmc_phy *priv = phy_get_drvdata(phy); + unsigned int caldone; + unsigned int dllrdy; + unsigned int freqsel; + unsigned int mhz; + int ret; + + /* +* Keep phyctrl_pdb and phyctrl_endll low to allow +* initialization of CALIO state M/C DFFs +*/ + ret = regmap_update_bits(priv->syscfg, PHY_CFG_0, PWR_DOWN_MASK, +PWR_DOWN(0)); + if (ret) { + dev_err(&phy->dev, "CALIO power down bar failed: %d\n", ret); + return ret; + } + + ret = regmap_update_bits(priv->syscfg, PHY_CFG_0, DLL_EN_MASK, +DLL_EN(0)); + if (ret) { + dev_err(&phy->dev, "turn off the dll failed: %d\n", ret); + return ret; + } + + /* Already finish power off above */ + if (!on_off) + return 0; + + mhz = DIV_ROUND_CLOSEST(clk_get_rate(priv->emmcclk), 100); + if (mhz <= 200 && mhz >= 170) + freqsel = FREQSEL_200M_170M; + else if (mhz <= 170 && mhz >= 140) + freqsel = FREQSEL_170M_140M; + else if (mhz <= 140 && mhz >= 110) + freqsel = FREQSEL_140M_110M; + else if (mhz <= 110 && mhz >= 80) + freqsel = FREQSEL_110M_80M; + else if (mhz <= 80 && mhz >= 50) + freqsel = FREQSEL_80M_50M; + else + freqsel = 0x0; + + if (mhz < 50 || mhz > 200) + dev_warn(&phy->dev, "U