by Vinod Koul

[permalink] [raw]

Subject: Re: [PATCH v4 02/10] phy: rockchip: add usbdp combo phy driver

On 25-03-24, 15:15, Sebastian Reichel wrote:
> This adds a new USBDP combo PHY with Samsung IP block driver.
>
> The driver get lane mux and mapping info in 2 ways, supporting
> DisplayPort alternate mode or parsing from DT. When parsing from DT,
> the property "rockchip,dp-lane-mux" provide the DP mux and mapping
> info. This is needed when the PHY is not used with TypeC Alt-Mode.
> For example if the USB3 interface of the PHY is connected to a USB
> Type A connector and the DP interface is connected to a DisplayPort
> connector.
>
> When do DP link training, need to set lane number, link rate, swing,
> and pre-emphasis via PHY configure interface.
>
> Co-developed-by: Heiko Stuebner <[email protected]>
> Signed-off-by: Heiko Stuebner <[email protected]>
> Co-developed-by: Zhang Yubing <[email protected]>
> Signed-off-by: Zhang Yubing <[email protected]>
> Co-developed-by: Frank Wang <[email protected]>
> Signed-off-by: Frank Wang <[email protected]>
> Tested-by: Heiko Stuebner <[email protected]>
> Signed-off-by: Sebastian Reichel <[email protected]>
> ---
> drivers/phy/rockchip/Kconfig | 12 +
> drivers/phy/rockchip/Makefile | 1 +
> drivers/phy/rockchip/phy-rockchip-usbdp.c | 1612 +++++++++++++++++++++
> 3 files changed, 1625 insertions(+)
> create mode 100644 drivers/phy/rockchip/phy-rockchip-usbdp.c
>
> diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
> index a34f67bb7e61..c3d62243b474 100644
> --- a/drivers/phy/rockchip/Kconfig
> +++ b/drivers/phy/rockchip/Kconfig
> @@ -115,3 +115,15 @@ config PHY_ROCKCHIP_USB
> select GENERIC_PHY
> help
> Enable this to support the Rockchip USB 2.0 PHY.
> +
> +config PHY_ROCKCHIP_USBDP
> + tristate "Rockchip USBDP COMBO PHY Driver"
> + depends on ARCH_ROCKCHIP && OF
> + select GENERIC_PHY
> + select TYPEC
> + help
> + Enable this to support the Rockchip USB3.0/DP combo PHY with
> + Samsung IP block. This is required for USB3 support on RK3588.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called phy-rockchip-usbdp
> diff --git a/drivers/phy/rockchip/Makefile b/drivers/phy/rockchip/Makefile
> index 3d911304e654..010a824e32ce 100644
> --- a/drivers/phy/rockchip/Makefile
> +++ b/drivers/phy/rockchip/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_PHY_ROCKCHIP_SAMSUNG_HDPTX) += phy-rockchip-samsung-hdptx.o
> obj-$(CONFIG_PHY_ROCKCHIP_SNPS_PCIE3) += phy-rockchip-snps-pcie3.o
> obj-$(CONFIG_PHY_ROCKCHIP_TYPEC) += phy-rockchip-typec.o
> obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
> +obj-$(CONFIG_PHY_ROCKCHIP_USBDP) += phy-rockchip-usbdp.o
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> new file mode 100644
> index 000000000000..38dc96cfe403
> --- /dev/null
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -0,0 +1,1612 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Rockchip USBDP Combo PHY with Samsung IP block driver
> + *
> + * Copyright (C) 2021 Rockchip Electronics Co., Ltd

we are in 2024!

> + */
> +
> +#include <dt-bindings/phy/phy.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>

Do you need both

> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/usb/ch9.h>
> +#include <linux/usb/typec_dp.h>
> +#include <linux/usb/typec_mux.h>

That seems a bit more of headers, do you need all of them?

> +static int rk_udphy_setup(struct rk_udphy *udphy)
> +{
> + int ret = 0;

Superfluous init

> +static int rk_udphy_parse_dt(struct rk_udphy *udphy)
> +{
> + struct device *dev = udphy->dev;
> + struct device_node *np = dev_of_node(dev);
> + enum usb_device_speed maximum_speed;
> + int ret;
> +
> + udphy->u2phygrf = syscon_regmap_lookup_by_phandle(np, "rockchip,u2phy-grf");
> + if (IS_ERR(udphy->u2phygrf))
> + return dev_err_probe(dev, PTR_ERR(udphy->u2phygrf), "failed to get u2phy-grf\n");
> +
> + udphy->udphygrf = syscon_regmap_lookup_by_phandle(np, "rockchip,usbdpphy-grf");
> + if (IS_ERR(udphy->udphygrf))
> + return dev_err_probe(dev, PTR_ERR(udphy->udphygrf), "failed to get usbdpphy-grf\n");
> +
> + udphy->usbgrf = syscon_regmap_lookup_by_phandle(np, "rockchip,usb-grf");
> + if (IS_ERR(udphy->usbgrf))
> + return dev_err_probe(dev, PTR_ERR(udphy->usbgrf), "failed to get usb-grf\n");
> +
> + udphy->vogrf = syscon_regmap_lookup_by_phandle(np, "rockchip,vo-grf");
> + if (IS_ERR(udphy->vogrf))
> + return dev_err_probe(dev, PTR_ERR(udphy->vogrf), "failed to get vo-grf\n");
> +
> + ret = rk_udphy_parse_lane_mux_data(udphy);
> + if (ret)
> + return ret;
> +
> + udphy->sbu1_dc_gpio = devm_gpiod_get_optional(dev, "sbu1-dc", GPIOD_OUT_LOW);
> + if (IS_ERR(udphy->sbu1_dc_gpio))
> + return PTR_ERR(udphy->sbu1_dc_gpio);
> +
> + udphy->sbu2_dc_gpio = devm_gpiod_get_optional(dev, "sbu2-dc", GPIOD_OUT_LOW);
> + if (IS_ERR(udphy->sbu2_dc_gpio))
> + return PTR_ERR(udphy->sbu2_dc_gpio);
> +
> + if (device_property_present(dev, "maximum-speed")) {
> + maximum_speed = usb_get_maximum_speed(dev);
> + udphy->hs = maximum_speed <= USB_SPEED_HIGH ? true : false;
> + }
> +
> + ret = rk_udphy_clk_init(udphy, dev);
> + if (ret)
> + return ret;
> +
> + ret = rk_udphy_reset_init(udphy, dev);
> + if (ret)
> + return ret;
> +
> + return 0;

return rk_udphy_reset_init()

> +static const struct phy_ops rk_udphy_dp_phy_ops = {
> + .init = rk_udphy_dp_phy_init,
> + .exit = rk_udphy_dp_phy_exit,
> + .power_on = rk_udphy_dp_phy_power_on,
> + .power_off = rk_udphy_dp_phy_power_off,
> + .configure = rk_udphy_dp_phy_configure,
> + .owner = THIS_MODULE,
> +};
> +
> +static int rk_udphy_usb3_phy_init(struct phy *phy)
> +{
> + struct rk_udphy *udphy = phy_get_drvdata(phy);
> + int ret = 0;

Superfluous init here too

--
~Vinod