Re: [v3 PATCH 3/5] phy: Add USB Type-C PHY driver for rk3399

[Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>]

Hi,

On Thursday 23 June 2016 06:21 PM, Chris Zhong wrote:
> Add a PHY provider driver for the rk3399 SoC Type-c PHY. The USB
> Type-C PHY is designed to support the USB3 and DP applications. The
> PHY basically has two main components: USB3 and DisplyPort. USB3
> operates in SuperSpeed mode and the DP can operate at RBR, HBR and
> HBR2 data rates.
> 
> Signed-off-by: Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Signed-off-by: Kever Yang <kever.yang-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> 
> ---
> 
> Changes in v3:
> - remove the phy framework(Kishon Vijay Abraham I)
> - add parentheses around the macro
> - use a single space between type and name
> - add spaces after opening and before closing braces.
> - use u16 for register value
> - remove type-c phy header file
> - CodingStyle optimization
> - use some cable extcon to get type-c port information
> - add a extcon to notify Display Port
> 
> Changes in v2:
> - select RESET_CONTROLLER
> - alphabetic order
> - modify some spelling mistakes
> - make mode cleaner
> - use bool for enable/disable
> - check all of the return value
> - return a better err number
> - use more readx_poll_timeout()
> - clk_disable_unprepare(tcphy->clk_ref);
> - remove unuse functions, rockchip_typec_phy_power_on/off
> - remove unnecessary typecast from void *
> - use dts node to distinguish between phys.
> 
> Changes in v1:
> - update the licence note
> - init core clock to 50MHz
> - use extcon API
> - remove unused global
> - add some comments for magic num
> - change usleep_range(1000, 2000) tousleep_range(1000, 1050)
> - remove __func__ from dev_err
> - return err number when get clk failed
> - remove ADDR_ADJ define
> - use devm_clk_get(&pdev->dev, "tcpdcore")
> 
>  drivers/phy/Kconfig              |    8 +
>  drivers/phy/Makefile             |    1 +
>  drivers/phy/phy-rockchip-typec.c | 1027 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1036 insertions(+)
>  create mode 100644 drivers/phy/phy-rockchip-typec.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 26566db..ec87b3a 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -351,6 +351,14 @@ config PHY_ROCKCHIP_DP
>  	help
>  	  Enable this to support the Rockchip Display Port PHY.
>  
> +config PHY_ROCKCHIP_TYPEC
> +	tristate "Rockchip TYPEC PHY Driver"
> +	depends on ARCH_ROCKCHIP && OF
> +	select GENERIC_PHY

Why? None of the generic PHY API's are used here. Why do you want select
generic PHY?
> +	select RESET_CONTROLLER

The driver also uses extcon. That has to be selected as well.

And since this driver doesn't use phy framework, I feel this should probably
end up in drivers/extcon and not drivers/phy.

Thanks
Kishon