On 13-02-24, 17:32, 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 <heiko@xxxxxxxxx> > Signed-off-by: Heiko Stuebner <heiko@xxxxxxxxx> > Co-developed-by: Zhang Yubing <yubing.zhang@xxxxxxxxxxxxxx> > Signed-off-by: Zhang Yubing <yubing.zhang@xxxxxxxxxxxxxx> > Co-developed-by: Frank Wang <frank.wang@xxxxxxxxxxxxxx> > Signed-off-by: Frank Wang <frank.wang@xxxxxxxxxxxxxx> > Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> > --- > drivers/phy/rockchip/Kconfig | 12 + > drivers/phy/rockchip/Makefile | 1 + > drivers/phy/rockchip/phy-rockchip-usbdp.c | 1639 +++++++++++++++++++++ > 3 files changed, 1652 insertions(+) > create mode 100644 drivers/phy/rockchip/phy-rockchip-usbdp.c > > diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig > index 94360fc96a6f..d21b458c1d18 100644 > --- a/drivers/phy/rockchip/Kconfig > +++ b/drivers/phy/rockchip/Kconfig > @@ -107,3 +107,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 7eab129230d1..25d2e1355db7 100644 > --- a/drivers/phy/rockchip/Makefile > +++ b/drivers/phy/rockchip/Makefile > @@ -11,3 +11,4 @@ obj-$(CONFIG_PHY_ROCKCHIP_PCIE) += phy-rockchip-pcie.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..8b1ace2aaa98 > --- /dev/null > +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c > @@ -0,0 +1,1639 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Rockchip USBDP Combo PHY with Samsung IP block driver > + * > + * Copyright (C) 2021 Rockchip Electronics Co., Ltd > + */ > + > +#include <linux/bitfield.h> > +#include <linux/bits.h> > +#include <linux/clk.h> > +#include <linux/clk-provider.h> > +#include <linux/delay.h> > +#include <linux/gpio.h> > +#include <linux/io.h> > +#include <linux/iopoll.h> > +#include <linux/kernel.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> Thats a lot of headers, do you need all? > +static inline int rk_udphy_grfreg_write(struct regmap *base, > + const struct rk_udphy_grf_reg *reg, bool en) > +{ > + u32 val, mask, tmp; > + > + tmp = en ? reg->enable : reg->disable; > + mask = GENMASK(reg->bitend, reg->bitstart); > + val = (tmp << reg->bitstart) | (mask << BIT_WRITEABLE_SHIFT); Perhaps FIELD_PREP|GET for these? > +static int rk_udphy_dplane_get(struct rk_udphy *udphy) > +{ > + int dp_lanes; > + > + switch (udphy->mode) { > + case UDPHY_MODE_DP: > + dp_lanes = 4; > + break; empty line after break makes it more readable > + case UDPHY_MODE_DP_USB: > + dp_lanes = 2; > + break; > + case UDPHY_MODE_USB: > + fallthrough; > + default: > + dp_lanes = 0; > + break; > + } > + > + return dp_lanes; > +} > + > +static int rk_udphy_dplane_enable(struct rk_udphy *udphy, int dp_lanes) > +{ > + u32 val = 0; > + int i; > + > + for (i = 0; i < dp_lanes; i++) > + val |= BIT(udphy->dp_lane_sel[i]); > + > + regmap_update_bits(udphy->pma_regmap, CMN_LANE_MUX_AND_EN_OFFSET, CMN_DP_LANE_EN_ALL, > + FIELD_PREP(CMN_DP_LANE_EN_ALL, val)); > + > + if (!dp_lanes) > + regmap_update_bits(udphy->pma_regmap, CMN_DP_RSTN_OFFSET, > + CMN_DP_CMN_RSTN, FIELD_PREP(CMN_DP_CMN_RSTN, 0x0)); > + > + return 0; there is no error generation in the fn, these kind of fn should really be not returning anything > +static int rk_udphy_usb3_phy_init(struct phy *phy) > +{ > + struct rk_udphy *udphy = phy_get_drvdata(phy); > + int ret = 0; > + > + mutex_lock(&udphy->mutex); > + /* DP only or high-speed, disable U3 port */ > + if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) { > + rk_udphy_u3_port_disable(udphy, true); > + goto unlock; no power up in that case? > + } > + > + ret = rk_udphy_power_on(udphy, UDPHY_MODE_USB); > + > +unlock: > + mutex_unlock(&udphy->mutex); > + return ret; > +} -- ~Vinod