Re: [PATCH 2/2] phy: add a driver for the Rockchip SoC internal eMMC PHY

2016-01-04 Thread Shawn Lin

Hi Kishon,
On 2016/1/4 15:45, Kishon Vijay Abraham I wrote:

Hi,

On Tuesday 29 December 2015 07:22 AM, Shawn Lin wrote:

This patch to add a generic PHY driver for ROCKCHIP eMMC PHY.


[...]


+
+struct rockchip_emmc_phy {
+   unsigned intreg_offset;
+   struct regmap   *reg_base;
+   struct phy  *phy;


The phy looks unnecessary.


right, got it.


+   boolstate;


hmm.. I want to have some sort of state machine in phy core so that individual
PHY drivers don't have to maintain the state. However I'm not sure if all the
PHY's will require such mechanism.



In general, phy core is the best place to maintain it.
Presumably, phy driver maintainer is much likely to
maintain the corresponding caller driver at the same time, so he/she
should be in charge of maintaining the on/off, init/exit pairs. From
this point, it doesn't need the state machine. But, I'm not sure if the
caller driver will always keep the on/off pairs correct.


Anyway, I will remove it from this driver. And may we request a RFC
for all sub-phy drivers to discuss this issue if we want to add the
state machine into phy core. How about?



+};
+
+static int rockchip_emmc_phy_power(struct rockchip_emmc_phy *rk_phy,


[...]


+}
+
+static int rockchip_emmc_phy_init(struct phy *phy)
+{
+   rockchip_emmc_phy_power_on(phy);


do only phy initialization here, power on can be done later.


yep.


+   return 0;
+}
+
+static int rockchip_emmc_phy_exit(struct phy *phy)
+{
+   rockchip_emmc_phy_power_off(phy);


same here.

Thanks
Kishon






--
Best Regards
Shawn Lin

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] phy: add a driver for the Rockchip SoC internal eMMC PHY

2016-01-03 Thread Kishon Vijay Abraham I
Hi,

On Tuesday 29 December 2015 07:22 AM, Shawn Lin wrote:
> This patch to add a generic PHY driver for ROCKCHIP eMMC PHY.
> Access the PHY via registers provided by GRF (general register
> files) module.
> 
> Signed-off-by: Shawn Lin 
> ---
> 
>  drivers/phy/Kconfig |   7 ++
>  drivers/phy/Makefile|   1 +
>  drivers/phy/phy-rockchip-emmc.c | 256 
> 
>  3 files changed, 264 insertions(+)
>  create mode 100644 drivers/phy/phy-rockchip-emmc.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 03cb3ea..47d16e3 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -320,6 +320,13 @@ config PHY_ROCKCHIP_USB
>   help
> Enable this to support the Rockchip USB 2.0 PHY.
>  
> +config PHY_ROCKCHIP_EMMC
> + tristate "Rockchip EMMC PHY Driver"
> + depends on ARCH_ROCKCHIP && OF
> + select GENERIC_PHY
> + help
> +   Enable this to support the Rockchip EMMC PHY.
> +
>  config PHY_ST_SPEAR1310_MIPHY
>   tristate "ST SPEAR1310-MIPHY driver"
>   select GENERIC_PHY
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 075db1a..ed8f175 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -35,6 +35,7 @@ phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)  += 
> phy-s5pv210-usb2.o
>  obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o
>  obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)  += phy-qcom-apq8064-sata.o
>  obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
> +obj-$(CONFIG_PHY_ROCKCHIP_EMMC) += phy-rockchip-emmc.o
>  obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)  += phy-qcom-ipq806x-sata.o
>  obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY) += phy-spear1310-miphy.o
>  obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY) += phy-spear1340-miphy.o
> diff --git a/drivers/phy/phy-rockchip-emmc.c b/drivers/phy/phy-rockchip-emmc.c
> new file mode 100644
> index 000..78f51a2
> --- /dev/null
> +++ b/drivers/phy/phy-rockchip-emmc.c
> @@ -0,0 +1,256 @@
> +/*
> + * Rockchip emmc PHY driver
> + *
> + * Copyright (C) 2015 Shawn Lin 
> + * Copyright (C) 2015 ROCKCHIP, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * The higher 16-bit of this register is used for write protection
> + * only if BIT(x + 16) set to 1 the BIT(x) can be written.
> + */
> +#define HIWORD_UPDATE(val, mask, shift) \
> + ((val) << (shift) | (mask) << ((shift) + 16))
> +
> +/* Register definition */
> +#define GRF_EMMCPHY_CON0 0x0
> +#define GRF_EMMCPHY_CON1 0x4
> +#define GRF_EMMCPHY_CON2 0x8
> +#define GRF_EMMCPHY_CON3 0xc
> +#define GRF_EMMCPHY_CON4 0x10
> +#define GRF_EMMCPHY_CON5 0x14
> +#define GRF_EMMCPHY_CON6 0x18
> +#define GRF_EMMCPHY_STATUS   0x20
> +
> +#define PHYCTRL_PDB_MASK 0x1
> +#define PHYCTRL_PDB_SHIFT0x0
> +#define PHYCTRL_PDB_PWR_ON   0x1
> +#define PHYCTRL_PDB_PWR_OFF  0x0
> +#define PHYCTRL_ENDLL_MASK   0x1
> +#define PHYCTRL_ENDLL_SHIFT 0x1
> +#define PHYCTRL_ENDLL_ENABLE 0x1
> +#define PHYCTRL_ENDLL_DISABLE0x0
> +#define PHYCTRL_CALDONE_MASK 0x1
> +#define PHYCTRL_CALDONE_SHIFT   0x6
> +#define PHYCTRL_CALDONE_DONE 0x1
> +#define PHYCTRL_CALDONE_GOING0x0
> +#define PHYCTRL_DLLRDY_MASK  0x1
> +#define PHYCTRL_DLLRDY_SHIFT 0x5
> +#define PHYCTRL_DLLRDY_DONE  0x1
> +#define PHYCTRL_DLLRDY_GOING 0x0
> +
> +struct rockchip_emmc_phy {
> + unsigned intreg_offset;
> + struct regmap   *reg_base;
> + struct phy  *phy;

The phy looks unnecessary.
> + boolstate;

hmm.. I want to have some sort of state machine in phy core so that individual
PHY drivers don't have to maintain the state. However I'm not sure if all the
PHY's will require such mechanism.

> +};
> +
> +static int rockchip_emmc_phy_power(struct rockchip_emmc_phy *rk_phy,
> +bool on_off)
> +{
> + unsigned int caldone;
> + unsigned int dllrdy;
> +
> + /*
> +  * Keep phyctrl_pdb and phyctrl_endll low to allow
> +  * initialization of CALIO state M/C DFFs
> +  */
> + regmap_write(rk_phy->reg_base,
> +  rk_phy->reg_offset + GRF_EMMCPHY_CON6,
> +  HIWORD_UPDATE(PHYCTRL_PDB_PWR_OFF,
> +PHYCTRL_PDB_MASK,
> +PHYCTRL_PDB_SHIFT));
> + regmap_write(rk_phy->reg_base,
> +  rk_phy->reg_offset + GRF_EMMCPHY_CON6,
> +  HIWORD_UP

[PATCH 2/2] phy: add a driver for the Rockchip SoC internal eMMC PHY

2015-12-28 Thread Shawn Lin
This patch to add a generic PHY driver for ROCKCHIP eMMC PHY.
Access the PHY via registers provided by GRF (general register
files) module.

Signed-off-by: Shawn Lin 
---

 drivers/phy/Kconfig |   7 ++
 drivers/phy/Makefile|   1 +
 drivers/phy/phy-rockchip-emmc.c | 256 
 3 files changed, 264 insertions(+)
 create mode 100644 drivers/phy/phy-rockchip-emmc.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 03cb3ea..47d16e3 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -320,6 +320,13 @@ config PHY_ROCKCHIP_USB
help
  Enable this to support the Rockchip USB 2.0 PHY.
 
+config PHY_ROCKCHIP_EMMC
+   tristate "Rockchip EMMC PHY Driver"
+   depends on ARCH_ROCKCHIP && OF
+   select GENERIC_PHY
+   help
+ Enable this to support the Rockchip EMMC PHY.
+
 config PHY_ST_SPEAR1310_MIPHY
tristate "ST SPEAR1310-MIPHY driver"
select GENERIC_PHY
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 075db1a..ed8f175 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -35,6 +35,7 @@ phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)+= 
phy-s5pv210-usb2.o
 obj-$(CONFIG_PHY_EXYNOS5_USBDRD)   += phy-exynos5-usbdrd.o
 obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)+= phy-qcom-apq8064-sata.o
 obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
+obj-$(CONFIG_PHY_ROCKCHIP_EMMC) += phy-rockchip-emmc.o
 obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)+= phy-qcom-ipq806x-sata.o
 obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY)   += phy-spear1310-miphy.o
 obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY)   += phy-spear1340-miphy.o
diff --git a/drivers/phy/phy-rockchip-emmc.c b/drivers/phy/phy-rockchip-emmc.c
new file mode 100644
index 000..78f51a2
--- /dev/null
+++ b/drivers/phy/phy-rockchip-emmc.c
@@ -0,0 +1,256 @@
+/*
+ * Rockchip emmc PHY driver
+ *
+ * Copyright (C) 2015 Shawn Lin 
+ * Copyright (C) 2015 ROCKCHIP, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * The higher 16-bit of this register is used for write protection
+ * only if BIT(x + 16) set to 1 the BIT(x) can be written.
+ */
+#define HIWORD_UPDATE(val, mask, shift) \
+   ((val) << (shift) | (mask) << ((shift) + 16))
+
+/* Register definition */
+#define GRF_EMMCPHY_CON0   0x0
+#define GRF_EMMCPHY_CON1   0x4
+#define GRF_EMMCPHY_CON2   0x8
+#define GRF_EMMCPHY_CON3   0xc
+#define GRF_EMMCPHY_CON4   0x10
+#define GRF_EMMCPHY_CON5   0x14
+#define GRF_EMMCPHY_CON6   0x18
+#define GRF_EMMCPHY_STATUS 0x20
+
+#define PHYCTRL_PDB_MASK   0x1
+#define PHYCTRL_PDB_SHIFT  0x0
+#define PHYCTRL_PDB_PWR_ON 0x1
+#define PHYCTRL_PDB_PWR_OFF0x0
+#define PHYCTRL_ENDLL_MASK 0x1
+#define PHYCTRL_ENDLL_SHIFT 0x1
+#define PHYCTRL_ENDLL_ENABLE   0x1
+#define PHYCTRL_ENDLL_DISABLE  0x0
+#define PHYCTRL_CALDONE_MASK   0x1
+#define PHYCTRL_CALDONE_SHIFT   0x6
+#define PHYCTRL_CALDONE_DONE   0x1
+#define PHYCTRL_CALDONE_GOING  0x0
+#define PHYCTRL_DLLRDY_MASK0x1
+#define PHYCTRL_DLLRDY_SHIFT   0x5
+#define PHYCTRL_DLLRDY_DONE0x1
+#define PHYCTRL_DLLRDY_GOING   0x0
+
+struct rockchip_emmc_phy {
+   unsigned intreg_offset;
+   struct regmap   *reg_base;
+   struct phy  *phy;
+   boolstate;
+};
+
+static int rockchip_emmc_phy_power(struct rockchip_emmc_phy *rk_phy,
+  bool on_off)
+{
+   unsigned int caldone;
+   unsigned int dllrdy;
+
+   /*
+* Keep phyctrl_pdb and phyctrl_endll low to allow
+* initialization of CALIO state M/C DFFs
+*/
+   regmap_write(rk_phy->reg_base,
+rk_phy->reg_offset + GRF_EMMCPHY_CON6,
+HIWORD_UPDATE(PHYCTRL_PDB_PWR_OFF,
+  PHYCTRL_PDB_MASK,
+  PHYCTRL_PDB_SHIFT));
+   regmap_write(rk_phy->reg_base,
+rk_phy->reg_offset + GRF_EMMCPHY_CON6,
+HIWORD_UPDATE(PHYCTRL_ENDLL_DISABLE,
+  PHYCTRL_ENDLL_MASK,
+  PHYCTRL_ENDLL_SHIFT));
+
+   /* Already finish power_off above */
+   if (on_off == PHYCTRL_PDB_PWR_OFF)
+   return 0;
+
+   /*
+* According to the user manual, calpad calibration
+* cycle takes more than 2us without the minimal recommended
+* value, so we may need a little margin here
+