Message ID | 20200702000934.3258-1-wan.ahmad.zainie.wan.mohamad@intel.com |
---|---|
Headers | show |
Series | phy: intel: Add Keem Bay eMMC PHY support | expand |
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 <wan.ahmad.zainie.wan.mohamad@intel.com> > --- > 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_0 0x24 > +#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!
Hi Vinod. Thanks for the review. > -----Original Message----- > From: Vinod Koul <vkoul@kernel.org> > Sent: Monday, July 13, 2020 1:40 PM > To: Wan Mohamad, Wan Ahmad Zainie > <wan.ahmad.zainie.wan.mohamad@intel.com> > Cc: kishon@ti.com; robh+dt@kernel.org; Shevchenko, Andriy > <andriy.shevchenko@intel.com>; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org; MP, Sureshkumar > <sureshkumar.mp@intel.com>; Raja Subramanian, Lakshmi Bai > <lakshmi.bai.raja.subramanian@intel.com> > 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 > > <wan.ahmad.zainie.wan.mohamad@intel.com> > > --- > > 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_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! Is it something like this following? #define maskval(mask, val) (((val) << (ffs(mask) - 1)) & mask) > > -- > ~Vinod