Re: [PATCH v6 1/4] phy: qcom-ufs: add support for 20nm phy

2015-01-15 Thread ygardi
> Hi,
>
> On Sunday 11 January 2015 06:08 PM, Yaniv Gardi wrote:
>> This change adds a support for a 20nm qcom-ufs phy that is required in
>> platforms that use ufs-qcom controller.
>>
>> Signed-off-by: Yaniv Gardi 
>>
>> ---
>>  drivers/phy/Kconfig |   7 +
>>  drivers/phy/Makefile|   2 +
>>  drivers/phy/phy-qcom-ufs-i.h| 159 
>>  drivers/phy/phy-qcom-ufs-qmp-20nm.c | 257 +
>>  drivers/phy/phy-qcom-ufs-qmp-20nm.h | 235 
>>  drivers/phy/phy-qcom-ufs.c  | 745
>> 
>>  include/linux/phy/phy-qcom-ufs.h|  59 +++
>>  7 files changed, 1464 insertions(+)
>>  create mode 100644 drivers/phy/phy-qcom-ufs-i.h
>>  create mode 100644 drivers/phy/phy-qcom-ufs-qmp-20nm.c
>>  create mode 100644 drivers/phy/phy-qcom-ufs-qmp-20nm.h
>>  create mode 100644 drivers/phy/phy-qcom-ufs.c
>>  create mode 100644 include/linux/phy/phy-qcom-ufs.h
>
> I think a single header file should be sufficient here.
>

I believe 2 header files is a better design in this case.
one header file is internal, and should serve internally the phy drivers.
the other one that exposes APIs.
One header file compels us to expose our internal macros, and definitions
in include\linux\phy which is not recommended and not necessary.

the advantage of 2 header files will be more visible if you pick the 14nm
phy code as well. (change#3 in V6, or change#4 in V7)

thanks for your comment.


> It would be helpful if you can split this patch further. First add only
> the
> core ufs layer (Include a documentation on how this layer should be used
> in
> Documentation/phy/) and then use it to add the 14nm and 20nm PHYs.

thanks Kishon for your comment.
I accept the first comment, and in V7, the first change is divided into
2 patches as suggested:
one that adds common support for Qualcomm Technologies UFS Phys
and the other one adds the specific implementation of 20nm phy.

at this point i decided not to add a Documentation, as the
Qualcomm UFS PHY uses the generic PHY framework, and currently we don't
think any further documentation is needed in addition to the Generic Phy
documentation that is presented already in Documentation/phy.txt

thanks for reviewing the change.

>
> Thanks
> Kishon
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index ccad880..26a7623 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -277,4 +277,11 @@ config PHY_STIH41X_USB
>>Enable this to support the USB transceiver that is part of
>>STMicroelectronics STiH41x SoC series.
>>
>> +config PHY_QCOM_UFS
>> +tristate "Qualcomm UFS PHY driver"
>> +depends on OF && ARCH_MSM
>> +select GENERIC_PHY
>> +help
>> +  Support for UFS PHY on QCOM chipsets.
>> +
>>  endmenu
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index aa74f96..781b2fa 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -34,3 +34,5 @@ obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY)   +=
>> phy-spear1340-miphy.o
>>  obj-$(CONFIG_PHY_XGENE) += phy-xgene.o
>>  obj-$(CONFIG_PHY_STIH407_USB)   += phy-stih407-usb.o
>>  obj-$(CONFIG_PHY_STIH41X_USB)   += phy-stih41x-usb.o
>> +obj-$(CONFIG_PHY_QCOM_UFS)  += phy-qcom-ufs.o
>> +obj-$(CONFIG_PHY_QCOM_UFS)  += phy-qcom-ufs-qmp-20nm.o
>> diff --git a/drivers/phy/phy-qcom-ufs-i.h b/drivers/phy/phy-qcom-ufs-i.h
>> new file mode 100644
>> index 000..a175069
>> --- /dev/null
>> +++ b/drivers/phy/phy-qcom-ufs-i.h
>> @@ -0,0 +1,159 @@
>> +/*
>> + * Copyright (c) 2013-2015, Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * 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.
>> + *
>> + */
>> +
>> +#ifndef UFS_QCOM_PHY_I_H_
>> +#define UFS_QCOM_PHY_I_H_
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define readl_poll_timeout(addr, val, cond, sleep_us, timeout_us) \
>> +({ \
>> +ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
>> +might_sleep_if(timeout_us); \
>> +for (;;) { \
>> +(val) = readl(addr); \
>> +if (cond) \
>> +break; \
>> +if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
>> +(val) = readl(addr); \
>> +break; \
>> +} \
>> +if (sleep_us) \
>> +usleep_range(DIV_ROUND_UP(sleep_us, 4), sleep_us); \
>> +} \
>> +(cond) ? 0 : -ETIMEDOUT; \
>> +})
>> +

Re: [PATCH v6 1/4] phy: qcom-ufs: add support for 20nm phy

2015-01-15 Thread Kishon Vijay Abraham I
Hi,

On Sunday 11 January 2015 06:08 PM, Yaniv Gardi wrote:
> This change adds a support for a 20nm qcom-ufs phy that is required in
> platforms that use ufs-qcom controller.
> 
> Signed-off-by: Yaniv Gardi 
> 
> ---
>  drivers/phy/Kconfig |   7 +
>  drivers/phy/Makefile|   2 +
>  drivers/phy/phy-qcom-ufs-i.h| 159 
>  drivers/phy/phy-qcom-ufs-qmp-20nm.c | 257 +
>  drivers/phy/phy-qcom-ufs-qmp-20nm.h | 235 
>  drivers/phy/phy-qcom-ufs.c  | 745 
> 
>  include/linux/phy/phy-qcom-ufs.h|  59 +++
>  7 files changed, 1464 insertions(+)
>  create mode 100644 drivers/phy/phy-qcom-ufs-i.h
>  create mode 100644 drivers/phy/phy-qcom-ufs-qmp-20nm.c
>  create mode 100644 drivers/phy/phy-qcom-ufs-qmp-20nm.h
>  create mode 100644 drivers/phy/phy-qcom-ufs.c
>  create mode 100644 include/linux/phy/phy-qcom-ufs.h

I think a single header file should be sufficient here.

It would be helpful if you can split this patch further. First add only the
core ufs layer (Include a documentation on how this layer should be used in
Documentation/phy/) and then use it to add the 14nm and 20nm PHYs.

Thanks
Kishon
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index ccad880..26a7623 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -277,4 +277,11 @@ config PHY_STIH41X_USB
> Enable this to support the USB transceiver that is part of
> STMicroelectronics STiH41x SoC series.
>  
> +config PHY_QCOM_UFS
> + tristate "Qualcomm UFS PHY driver"
> + depends on OF && ARCH_MSM
> + select GENERIC_PHY
> + help
> +   Support for UFS PHY on QCOM chipsets.
> +
>  endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index aa74f96..781b2fa 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -34,3 +34,5 @@ obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY)+= 
> phy-spear1340-miphy.o
>  obj-$(CONFIG_PHY_XGENE)  += phy-xgene.o
>  obj-$(CONFIG_PHY_STIH407_USB)+= phy-stih407-usb.o
>  obj-$(CONFIG_PHY_STIH41X_USB)+= phy-stih41x-usb.o
> +obj-$(CONFIG_PHY_QCOM_UFS)   += phy-qcom-ufs.o
> +obj-$(CONFIG_PHY_QCOM_UFS)   += phy-qcom-ufs-qmp-20nm.o
> diff --git a/drivers/phy/phy-qcom-ufs-i.h b/drivers/phy/phy-qcom-ufs-i.h
> new file mode 100644
> index 000..a175069
> --- /dev/null
> +++ b/drivers/phy/phy-qcom-ufs-i.h
> @@ -0,0 +1,159 @@
> +/*
> + * Copyright (c) 2013-2015, Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * 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.
> + *
> + */
> +
> +#ifndef UFS_QCOM_PHY_I_H_
> +#define UFS_QCOM_PHY_I_H_
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define readl_poll_timeout(addr, val, cond, sleep_us, timeout_us) \
> +({ \
> + ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
> + might_sleep_if(timeout_us); \
> + for (;;) { \
> + (val) = readl(addr); \
> + if (cond) \
> + break; \
> + if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
> + (val) = readl(addr); \
> + break; \
> + } \
> + if (sleep_us) \
> + usleep_range(DIV_ROUND_UP(sleep_us, 4), sleep_us); \
> + } \
> + (cond) ? 0 : -ETIMEDOUT; \
> +})
> +
> +#define UFS_QCOM_PHY_CAL_ENTRY(reg, val) \
> + {   \
> + .reg_offset = reg,  \
> + .cfg_value = val,   \
> + }
> +
> +#define UFS_QCOM_PHY_NAME_LEN30
> +
> +enum {
> + MASK_SERDES_START   = 0x1,
> + MASK_PCS_READY  = 0x1,
> +};
> +
> +enum {
> + OFFSET_SERDES_START = 0x0,
> +};
> +
> +struct ufs_qcom_phy_stored_attributes {
> + u32 att;
> + u32 value;
> +};
> +
> +struct ufs_qcom_phy_calibration {
> + u32 reg_offset;
> + u32 cfg_value;
> +};
> +
> +struct ufs_qcom_phy_vreg {
> + const char *name;
> + struct regulator *reg;
> + int max_uA;
> + int min_uV;
> + int max_uV;
> + bool enabled;
> + bool is_always_on;
> +};
> +
> +struct ufs_qcom_phy {
> + struct list_head list;
> + struct device *dev;
> + void __iomem *mmio;
> + void __iomem *dev_ref_clk_ctrl_mmio;
> + struct clk *tx_iface_clk;
> + struct clk *rx_iface_clk;
> + bool is_iface_clk_enabled;
> + struct clk *ref_clk_src;
> + struct clk *ref_clk_parent;

Re: [PATCH v6 1/4] phy: qcom-ufs: add support for 20nm phy

2015-01-15 Thread Kishon Vijay Abraham I
Hi,

On Sunday 11 January 2015 06:08 PM, Yaniv Gardi wrote:
 This change adds a support for a 20nm qcom-ufs phy that is required in
 platforms that use ufs-qcom controller.
 
 Signed-off-by: Yaniv Gardi yga...@codeaurora.org
 
 ---
  drivers/phy/Kconfig |   7 +
  drivers/phy/Makefile|   2 +
  drivers/phy/phy-qcom-ufs-i.h| 159 
  drivers/phy/phy-qcom-ufs-qmp-20nm.c | 257 +
  drivers/phy/phy-qcom-ufs-qmp-20nm.h | 235 
  drivers/phy/phy-qcom-ufs.c  | 745 
 
  include/linux/phy/phy-qcom-ufs.h|  59 +++
  7 files changed, 1464 insertions(+)
  create mode 100644 drivers/phy/phy-qcom-ufs-i.h
  create mode 100644 drivers/phy/phy-qcom-ufs-qmp-20nm.c
  create mode 100644 drivers/phy/phy-qcom-ufs-qmp-20nm.h
  create mode 100644 drivers/phy/phy-qcom-ufs.c
  create mode 100644 include/linux/phy/phy-qcom-ufs.h

I think a single header file should be sufficient here.

It would be helpful if you can split this patch further. First add only the
core ufs layer (Include a documentation on how this layer should be used in
Documentation/phy/) and then use it to add the 14nm and 20nm PHYs.

Thanks
Kishon
 
 diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
 index ccad880..26a7623 100644
 --- a/drivers/phy/Kconfig
 +++ b/drivers/phy/Kconfig
 @@ -277,4 +277,11 @@ config PHY_STIH41X_USB
 Enable this to support the USB transceiver that is part of
 STMicroelectronics STiH41x SoC series.
  
 +config PHY_QCOM_UFS
 + tristate Qualcomm UFS PHY driver
 + depends on OF  ARCH_MSM
 + select GENERIC_PHY
 + help
 +   Support for UFS PHY on QCOM chipsets.
 +
  endmenu
 diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
 index aa74f96..781b2fa 100644
 --- a/drivers/phy/Makefile
 +++ b/drivers/phy/Makefile
 @@ -34,3 +34,5 @@ obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY)+= 
 phy-spear1340-miphy.o
  obj-$(CONFIG_PHY_XGENE)  += phy-xgene.o
  obj-$(CONFIG_PHY_STIH407_USB)+= phy-stih407-usb.o
  obj-$(CONFIG_PHY_STIH41X_USB)+= phy-stih41x-usb.o
 +obj-$(CONFIG_PHY_QCOM_UFS)   += phy-qcom-ufs.o
 +obj-$(CONFIG_PHY_QCOM_UFS)   += phy-qcom-ufs-qmp-20nm.o
 diff --git a/drivers/phy/phy-qcom-ufs-i.h b/drivers/phy/phy-qcom-ufs-i.h
 new file mode 100644
 index 000..a175069
 --- /dev/null
 +++ b/drivers/phy/phy-qcom-ufs-i.h
 @@ -0,0 +1,159 @@
 +/*
 + * Copyright (c) 2013-2015, Linux Foundation. All rights reserved.
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 and
 + * only version 2 as published by the Free Software Foundation.
 + *
 + * 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.
 + *
 + */
 +
 +#ifndef UFS_QCOM_PHY_I_H_
 +#define UFS_QCOM_PHY_I_H_
 +
 +#include linux/module.h
 +#include linux/clk.h
 +#include linux/regulator/consumer.h
 +#include linux/slab.h
 +
 +#include linux/phy/phy-qcom-ufs.h
 +#include linux/platform_device.h
 +#include linux/io.h
 +#include linux/delay.h
 +
 +#define readl_poll_timeout(addr, val, cond, sleep_us, timeout_us) \
 +({ \
 + ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
 + might_sleep_if(timeout_us); \
 + for (;;) { \
 + (val) = readl(addr); \
 + if (cond) \
 + break; \
 + if (timeout_us  ktime_compare(ktime_get(), timeout)  0) { \
 + (val) = readl(addr); \
 + break; \
 + } \
 + if (sleep_us) \
 + usleep_range(DIV_ROUND_UP(sleep_us, 4), sleep_us); \
 + } \
 + (cond) ? 0 : -ETIMEDOUT; \
 +})
 +
 +#define UFS_QCOM_PHY_CAL_ENTRY(reg, val) \
 + {   \
 + .reg_offset = reg,  \
 + .cfg_value = val,   \
 + }
 +
 +#define UFS_QCOM_PHY_NAME_LEN30
 +
 +enum {
 + MASK_SERDES_START   = 0x1,
 + MASK_PCS_READY  = 0x1,
 +};
 +
 +enum {
 + OFFSET_SERDES_START = 0x0,
 +};
 +
 +struct ufs_qcom_phy_stored_attributes {
 + u32 att;
 + u32 value;
 +};
 +
 +struct ufs_qcom_phy_calibration {
 + u32 reg_offset;
 + u32 cfg_value;
 +};
 +
 +struct ufs_qcom_phy_vreg {
 + const char *name;
 + struct regulator *reg;
 + int max_uA;
 + int min_uV;
 + int max_uV;
 + bool enabled;
 + bool is_always_on;
 +};
 +
 +struct ufs_qcom_phy {
 + struct list_head list;
 + struct device *dev;
 + void __iomem *mmio;
 + void __iomem *dev_ref_clk_ctrl_mmio;
 + struct clk *tx_iface_clk;
 + struct clk *rx_iface_clk;
 + bool is_iface_clk_enabled;
 + struct clk *ref_clk_src;
 + struct clk *ref_clk_parent;

Re: [PATCH v6 1/4] phy: qcom-ufs: add support for 20nm phy

2015-01-15 Thread ygardi
 Hi,

 On Sunday 11 January 2015 06:08 PM, Yaniv Gardi wrote:
 This change adds a support for a 20nm qcom-ufs phy that is required in
 platforms that use ufs-qcom controller.

 Signed-off-by: Yaniv Gardi yga...@codeaurora.org

 ---
  drivers/phy/Kconfig |   7 +
  drivers/phy/Makefile|   2 +
  drivers/phy/phy-qcom-ufs-i.h| 159 
  drivers/phy/phy-qcom-ufs-qmp-20nm.c | 257 +
  drivers/phy/phy-qcom-ufs-qmp-20nm.h | 235 
  drivers/phy/phy-qcom-ufs.c  | 745
 
  include/linux/phy/phy-qcom-ufs.h|  59 +++
  7 files changed, 1464 insertions(+)
  create mode 100644 drivers/phy/phy-qcom-ufs-i.h
  create mode 100644 drivers/phy/phy-qcom-ufs-qmp-20nm.c
  create mode 100644 drivers/phy/phy-qcom-ufs-qmp-20nm.h
  create mode 100644 drivers/phy/phy-qcom-ufs.c
  create mode 100644 include/linux/phy/phy-qcom-ufs.h

 I think a single header file should be sufficient here.


I believe 2 header files is a better design in this case.
one header file is internal, and should serve internally the phy drivers.
the other one that exposes APIs.
One header file compels us to expose our internal macros, and definitions
in include\linux\phy which is not recommended and not necessary.

the advantage of 2 header files will be more visible if you pick the 14nm
phy code as well. (change#3 in V6, or change#4 in V7)

thanks for your comment.


 It would be helpful if you can split this patch further. First add only
 the
 core ufs layer (Include a documentation on how this layer should be used
 in
 Documentation/phy/) and then use it to add the 14nm and 20nm PHYs.

thanks Kishon for your comment.
I accept the first comment, and in V7, the first change is divided into
2 patches as suggested:
one that adds common support for Qualcomm Technologies UFS Phys
and the other one adds the specific implementation of 20nm phy.

at this point i decided not to add a Documentation, as the
Qualcomm UFS PHY uses the generic PHY framework, and currently we don't
think any further documentation is needed in addition to the Generic Phy
documentation that is presented already in Documentation/phy.txt

thanks for reviewing the change.


 Thanks
 Kishon

 diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
 index ccad880..26a7623 100644
 --- a/drivers/phy/Kconfig
 +++ b/drivers/phy/Kconfig
 @@ -277,4 +277,11 @@ config PHY_STIH41X_USB
Enable this to support the USB transceiver that is part of
STMicroelectronics STiH41x SoC series.

 +config PHY_QCOM_UFS
 +tristate Qualcomm UFS PHY driver
 +depends on OF  ARCH_MSM
 +select GENERIC_PHY
 +help
 +  Support for UFS PHY on QCOM chipsets.
 +
  endmenu
 diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
 index aa74f96..781b2fa 100644
 --- a/drivers/phy/Makefile
 +++ b/drivers/phy/Makefile
 @@ -34,3 +34,5 @@ obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY)   +=
 phy-spear1340-miphy.o
  obj-$(CONFIG_PHY_XGENE) += phy-xgene.o
  obj-$(CONFIG_PHY_STIH407_USB)   += phy-stih407-usb.o
  obj-$(CONFIG_PHY_STIH41X_USB)   += phy-stih41x-usb.o
 +obj-$(CONFIG_PHY_QCOM_UFS)  += phy-qcom-ufs.o
 +obj-$(CONFIG_PHY_QCOM_UFS)  += phy-qcom-ufs-qmp-20nm.o
 diff --git a/drivers/phy/phy-qcom-ufs-i.h b/drivers/phy/phy-qcom-ufs-i.h
 new file mode 100644
 index 000..a175069
 --- /dev/null
 +++ b/drivers/phy/phy-qcom-ufs-i.h
 @@ -0,0 +1,159 @@
 +/*
 + * Copyright (c) 2013-2015, Linux Foundation. All rights reserved.
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 and
 + * only version 2 as published by the Free Software Foundation.
 + *
 + * 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.
 + *
 + */
 +
 +#ifndef UFS_QCOM_PHY_I_H_
 +#define UFS_QCOM_PHY_I_H_
 +
 +#include linux/module.h
 +#include linux/clk.h
 +#include linux/regulator/consumer.h
 +#include linux/slab.h
 +
 +#include linux/phy/phy-qcom-ufs.h
 +#include linux/platform_device.h
 +#include linux/io.h
 +#include linux/delay.h
 +
 +#define readl_poll_timeout(addr, val, cond, sleep_us, timeout_us) \
 +({ \
 +ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
 +might_sleep_if(timeout_us); \
 +for (;;) { \
 +(val) = readl(addr); \
 +if (cond) \
 +break; \
 +if (timeout_us  ktime_compare(ktime_get(), timeout)  0) { \
 +(val) = readl(addr); \
 +break; \
 +} \
 +if (sleep_us) \
 +usleep_range(DIV_ROUND_UP(sleep_us, 4), sleep_us); \
 +} \
 +(cond) ? 0 : -ETIMEDOUT; \
 +})
 +
 +#define UFS_QCOM_PHY_CAL_ENTRY(reg, val)\
 +{