RE: [PATCH v1 1/2] extcon: add Realtek DHC RTD SoC Type-C driver

From: Stanley Chang[昌育德]
Date: Fri Aug 25 2023 - 05:41:14 EST


Hi Chanwoo,

>
> Almost code looks good to me. I add some comment below. Please check
> them.
>
Thanks for your review.
>
> On 23. 8. 22. 19:28, Stanley Chang wrote:
> > This patch adds the extcon driver for Realtek DHC (digital home center)
> > RTD SoCs type-c module. This can be used to detect whether the port is
> > configured as a downstream or upstream facing port. And notify the status
> > of extcon to listeners.
> >
> > Signed-off-by: Stanley Chang <stanley_chang@xxxxxxxxxxx>
> > ---
> > drivers/extcon/Kconfig | 9 +
> > drivers/extcon/Makefile | 1 +
> > drivers/extcon/extcon-rtk-type-c.c | 1799
> ++++++++++++++++++++++++++++
> > 3 files changed, 1809 insertions(+)
> > create mode 100644 drivers/extcon/extcon-rtk-type-c.c
> >
> > diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
> > index 0ef1971d22bb..d670434321f2 100644
> > --- a/drivers/extcon/Kconfig
> > +++ b/drivers/extcon/Kconfig
> > @@ -190,4 +190,13 @@ config EXTCON_USBC_TUSB320
> > Say Y here to enable support for USB Type C cable detection
> extcon
> > support using a TUSB320.
> >
> > +config EXTCON_RTK_TYPE_C
> > + tristate "Realtek RTD SoC extcon Type-C Driver"
> > + depends on ARCH_REALTEK || COMPILE_TEST
> > + help
> > + Say Y here to enable extcon support for USB Type C cable detection
> > + when using the Realtek RTD SoC USB Type-C port.
> > + The DHC (Digital Home Hub) RTD series SoC contains a type c
> module.
> > + This driver will detect the status of the type-c port.
> > +
> > endif
> > diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
> > index 1b390d934ca9..f779adb5e4c7 100644
> > --- a/drivers/extcon/Makefile
> > +++ b/drivers/extcon/Makefile
> > @@ -25,3 +25,4 @@ obj-$(CONFIG_EXTCON_SM5502) += extcon-sm5502.o
> > obj-$(CONFIG_EXTCON_USB_GPIO) += extcon-usb-gpio.o
> > obj-$(CONFIG_EXTCON_USBC_CROS_EC) += extcon-usbc-cros-ec.o
> > obj-$(CONFIG_EXTCON_USBC_TUSB320) += extcon-usbc-tusb320.o
> > +obj-$(CONFIG_EXTCON_RTK_TYPE_C) += extcon-rtk-type-c.o
> > diff --git a/drivers/extcon/extcon-rtk-type-c.c
> b/drivers/extcon/extcon-rtk-type-c.c
> > new file mode 100644
> > index 000000000000..04d4d5128bdb
> > --- /dev/null
> > +++ b/drivers/extcon/extcon-rtk-type-c.c
> > @@ -0,0 +1,1799 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * * extcon-rtk-type-c.c - Realtek Extcon Type C driver
> > + *
> > + * Copyright (C) 2023 Realtek Semiconductor Corporation
> > + *
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/slab.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/io.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/syscalls.h>
> > +#include <linux/suspend.h>
> > +#include <linux/debugfs.h>
> > +#include <linux/extcon.h>
> > +#include <linux/extcon-provider.h>
> > +#include <linux/sys_soc.h>
> > +#include <linux/nvmem-consumer.h>
> > +#include <linux/usb/otg.h>
> > +#include <linux/usb/typec.h>
> > +
> > +struct cc_param {
> > + u32 rp_4p7k_code;
> > + u32 rp_36k_code;
> > + u32 rp_12k_code;
> > + u32 rd_code;
> > + u32 ra_code;
> > + u32 vref_2p6v;
> > + u32 vref_1p23v;
> > + u32 vref_0p8v;
> > + u32 vref_0p66v;
> > + u32 vref_0p4v;
> > + u32 vref_0p2v;
> > + u32 vref_1_1p6v;
> > + u32 vref_0_1p6v;
> > +};
> > +
> > +struct type_c_cfg {
> > + int parameter_ver; /* Parameter version */
> > + int cc_dfp_mode;
> > + struct cc_param cc1_param;
> > + struct cc_param cc2_param;
> > +
> > + u32 debounce_val;
> > + bool use_defalut_parameter;
> > +};
> > +
> > +struct type_c_data {
> > + void __iomem *reg_base;
> > + struct device *dev;
> > + struct extcon_dev *edev;
> > +
> > + u32 irq;
> > +
> > + /* rd control GPIO only for rtd1295 */
> > + unsigned int rd_ctrl_gpio;
> > +
> > + /* Parameters */
> > + struct type_c_cfg *type_c_cfg;
> > + u32 dfp_mode_rp_en;
> > + u32 ufp_mode_rd_en;
> > + u32 cc1_code;
> > + u32 cc2_code;
> > + u32 cc1_vref;
> > + u32 cc2_vref;
> > + u32 debounce; /* 1b,1us 7f,4.7us */
> > +
> > + /* type_c state */
> > + int connect_change;
> > +#define CONNECT_CHANGE 1
> > +#define CONNECT_NO_CHANGE 0
> > + int cc_mode; /* cc is host or device */
> > +#define IN_HOST_MODE 0x10
> > +#define IN_DEVICE_MODE 0x20
> > + int is_attach;
> > +#define IN_ATTACH 1
> > +#define TO_ATTACH 1
> > +#define IN_DETACH 0
> > +#define TO_DETACH 0
> > + int at_cc1;
> > +#define AT_CC1 1
> > +#define AT_CC2 0
> > +
> > + u32 int_status;
> > + u32 cc_status;
> > + /* protect the data member */
> > + spinlock_t lock;
> > + struct delayed_work delayed_work;
> > +
> > + bool rd_en_at_first;
> > +
> > + struct dentry *debug_dir;
> > +
> > +#ifdef CONFIG_TYPEC
> > + struct typec_port *port;
> > +#endif /* CONFIG_TYPEC */
> > +};
> > +
> > +/* Type C register offset */
> > +#define USB_TYPEC_CTRL_CC1_0 0x0
> > +#define USB_TYPEC_CTRL_CC1_1 0x4
> > +#define USB_TYPEC_CTRL_CC2_0 0x8
> > +#define USB_TYPEC_CTRL_CC2_1 0xC
> > +#define USB_TYPEC_STS 0x10
> > +#define USB_TYPEC_CTRL 0x14
> > +#define USB_DBUS_PWR_CTRL 0x18
>
> nitpick.
> Above defintions uses 'space' to keep the left-aligned of value.
> But, below defintions doesn't keep the left-aligned of value.
>
> I recommend that you better to use the same style for the readability.

Ok, I will revise.

> > +
> > +#define ENABLE_CC1 0x1
> > +#define ENABLE_CC2 0x2
> > +#define DISABLE_CC 0x0
> > +
> > +/* Bit mapping USB_TYPEC_CTRL_CC1_0 and USB_TYPEC_CTRL_CC2_0 */
> > +#define PLR_EN BIT(29)
> > +#define CC_SWITCH_MASK (BIT(29) | BIT(28) | BIT(27))
> > +#define CC_CODE_MASK (0xfffff << 7)
> > +#define rp4pk_code(val) ((0x1f & (val)) << 22)
> > +#define code_rp4pk(val) (((val) >> 22) & 0x1f)
> > +#define rp36k_code(val) ((0x1f & (val)) << 17)
> > +#define code_rp36k(val) (((val) >> 17) & 0x1f)
> > +#define rp12k_code(val) ((0x1f & (val)) << 12)
> > +#define code_rp12k(val) (((val) >> 12) & 0x1f)
> > +#define rd_code(val) ((0x1f & (val)) << 7)
> > +#define code_rd(val) (((val) >> 7) & 0x1f)
> > +#define dfp_mode(val) ((0x3 & (val)) << 5)
> > +#define EN_RP4P7K BIT(4)
> > +#define EN_RP36K BIT(3)
> > +#define EN_RP12K BIT(2)
> > +#define EN_RD BIT(1)
> > +#define EN_CC_DET BIT(0)
> > +
> > +#define CC_MODE_UFP 0x0
> > +#define CC_MODE_DFP_USB 0x1
> > +#define CC_MODE_DFP_1_5 0x2
> > +#define CC_MODE_DFP_3_0 0x3
> > +
> > +/*
> > + * PARAMETER_V0:
> > + * Realtek Kylin rtd1295
> > + * Realtek Hercules rtd1395
> > + * Realtek Thor rtd1619
> > + * Realtek Hank rtd1319
> > + * Realtek Groot rtd1312c
> > + * PARAMETER_V1:
> > + * Realtek Stark rtd1619b
> > + * Realtek Parker rtd1319d
> > + * Realtek Danvers rtd1315e
> > + */
> > +enum parameter_version {
> > + PARAMETER_V0 = 0,
> > + PARAMETER_V1 = 1,
> > +};
> > +
> > +/* Bit mapping USB_TYPEC_CTRL_CC1_1 and USB_TYPEC_CTRL_CC2_1 */
> > +#define V0_vref_2p6v(val) ((0xf & (val)) << 26) /* Bit 29 for groot */
> > +#define V0_vref_1p23v(val) ((0xf & (val)) << 22)
> > +#define V0_vref_0p8v(val) ((0xf & (val)) << 18)
> > +#define V0_vref_0p66v(val) ((0xf & (val)) << 14)
> > +#define V0_vref_0p4v(val) ((0x7 & (val)) << 11)
> > +#define V0_vref_0p2v(val) ((0x7 & (val)) << 8)
> > +#define V0_vref_1_1p6v(val) ((0xf & (val)) << 4)
> > +#define V0_vref_0_1p6v(val) ((0xf & (val)) << 0)
> > +
> > +#define V0_decode_2p6v(val) (((val) >> 26) & 0xf) /* Bit 29 for groot */
> > +#define V0_decode_1p23v(val) (((val) >> 22) & 0xf)
> > +#define V0_decode_0p8v(val) (((val) >> 18) & 0xf)
> > +#define V0_decode_0p66v(val) (((val) >> 14) & 0xf)
> > +#define V0_decode_0p4v(val) (((val) >> 11) & 0x7)
> > +#define V0_decode_0p2v(val) (((val) >> 8) & 0x7)
> > +#define V0_decode_1_1p6v(val) (((val) >> 4) & 0xf)
> > +#define V0_decode_0_1p6v(val) (((val) >> 0) & 0xf)
> > +
> > +/* new Bit mapping USB_TYPEC_CTRL_CC1_1 and
> USB_TYPEC_CTRL_CC2_1 */
> > +#define V1_vref_2p6v(val) ((0xf & (val)) << 28)
> > +#define V1_vref_1p23v(val) ((0xf & (val)) << 24)
> > +#define V1_vref_0p8v(val) ((0xf & (val)) << 20)
> > +#define V1_vref_0p66v(val) ((0xf & (val)) << 16)
> > +#define V1_vref_0p4v(val) ((0xf & (val)) << 12)
> > +#define V1_vref_0p2v(val) ((0xf & (val)) << 8)
> > +#define V1_vref_1_1p6v(val) ((0xf & (val)) << 4)
> > +#define V1_vref_0_1p6v(val) ((0xf & (val)) << 0)
> > +
> > +#define V1_decode_2p6v(val) (((val) >> 28) & 0xf)
> > +#define V1_decode_1p23v(val) (((val) >> 24) & 0xf)
> > +#define V1_decode_0p8v(val) (((val) >> 20) & 0xf)
> > +#define V1_decode_0p66v(val) (((val) >> 16) & 0xf)
> > +#define V1_decode_0p4v(val) (((val) >> 12) & 0xf)
> > +#define V1_decode_0p2v(val) (((val) >> 8) & 0xf)
> > +#define V1_decode_1_1p6v(val) (((val) >> 4) & 0xf)
> > +#define V1_decode_0_1p6v(val) (((val) >> 0) & 0xf)
> > +
> > +/* Bit mapping USB_TYPEC_STS */
> > +#define DET_STS 0x7
> > +#define CC1_DET_STS (DET_STS)
> > +#define CC2_DET_STS (DET_STS << 3)
> > +#define DET_STS_RA 0x1
> > +#define DET_STS_RD 0x3
> > +#define DET_STS_RP 0x1
> > +#define CC1_DET_STS_RA (DET_STS_RA)
> > +#define CC1_DET_STS_RD (DET_STS_RD)
> > +#define CC1_DET_STS_RP (DET_STS_RP)
> > +#define CC2_DET_STS_RA (DET_STS_RA << 3)
> > +#define CC2_DET_STS_RD (DET_STS_RD << 3)
> > +#define CC2_DET_STS_RP (DET_STS_RP << 3)
> > +
> > +/* Bit mapping USB_TYPEC_CTRL */
> > +#define CC2_INT_EN BIT(11)
> > +#define CC1_INT_EN BIT(10)
> > +#define CC2_INT_STS BIT(9)
> > +#define CC1_INT_STS BIT(8)
> > +#define DEBOUNCE_TIME_MASK 0xff
> > +#define DEBOUNCE_EN BIT(0)
> > +#define ENABLE_TYPE_C_DETECT (CC1_INT_EN | CC2_INT_EN)
> > +#define ALL_CC_INT_STS (CC1_INT_STS | CC2_INT_STS)
> > +
> > +/* Parameter */
> > +#define DETECT_TIME 50 /* ms */
> > +
> > +static const unsigned int usb_type_c_cable[] = {
> > + EXTCON_USB,
> > + EXTCON_USB_HOST,
> > + EXTCON_NONE,
> > +};
> > +
> > +enum usb_data_roles {
> > + DR_NONE,
> > + DR_HOST,
> > + DR_DEVICE,
> > +};
> > +
> > +static const struct soc_device_attribute rtk_soc_kylin[] = {
> > + { .family = "Realtek Kylin", },
> > + { /* empty */ }
> > +};
> > +
> > +static int rtd129x_switch_type_c_plug_config(struct type_c_data *type_c,
> > + int dr_mode, int cc)
> > +{
> > + void __iomem *reg = type_c->reg_base + USB_TYPEC_CTRL_CC1_0;
> > + int val_cc;
> > +
> > +#define TYPE_C_EN_SWITCH BIT(29)
> > +#define TYPE_C_TXRX_SEL (BIT(28) | BIT(27))
> > +#define TYPE_C_SWITCH_MASK (TYPE_C_EN_SWITCH | TYPE_C_TXRX_SEL)
> > +#define TYPE_C_ENABLE_CC1 TYPE_C_EN_SWITCH
> > +#define TYPE_C_ENABLE_CC2 (TYPE_C_EN_SWITCH | TYPE_C_TXRX_SEL)
> > +#define TYPE_C_DISABLE_CC ~TYPE_C_SWITCH_MASK
> > +
> > + val_cc = readl(reg);
>
> I'd like you to use regmap interface to access the register
> by using regmap_read, regmap_write. You can create the regmap instance
> via devm_regmap_init_mmio() on probe instead of using 'type_c->reg_base'
> at the multipe point.
>
> For example,
> struct regmap_config rtk_regmap_config = {
> .reg_bits = 32,
> .val_bits = 32,
> };
>
> void __iomem *base;
>
> base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> if (IS_ERR(base))
> return PTR_ERR(base);
>
> regmap = devm_regmap_init_mmio(dev, base,
> &rtk_regmap_config);
>
> ---
>
> And then just call regmap_read without any calculation between
> base address and specific register.
>
> regmap_read(regmap, USB_TYPEC_CTRL_CC1_0)

I will study this method.

>
> > + val_cc &= ~TYPE_C_SWITCH_MASK;
> > +
> > + if (cc == DISABLE_CC) {
> > + val_cc &= TYPE_C_DISABLE_CC;
> > + } else if (cc == ENABLE_CC1) {
> > + val_cc |= TYPE_C_ENABLE_CC1;
> > + } else if (cc == ENABLE_CC2) {
> > + val_cc |= TYPE_C_ENABLE_CC2;
> > + } else {
> > + dev_err(type_c->dev, "%s: Error cc setting cc=0x%x\n",
> __func__, cc);
> > + return -EINVAL;
> > + }
> > + writel(val_cc, reg);
> > +
> > + mdelay(1);
> Maybe, it depends on h/w constraints. Better to explain the comment
> of why adding delay at here. Also, this patch uses mdelay/msleep at
> the multiple point. I recommend that add the comment of reason to add
> mdelay/msleep.

Ok, I will add.
>
> > +
> > + dev_dbg(type_c->dev, "%s: cc=0x%x val_cc=0x%x
> usb_typec_ctrl_cc1_0=0x%x\n",
> > + __func__, cc, val_cc, readl(reg));
> > +
> > + return 0;
> > +}
> > +
> > +static inline void switch_type_c_plug_config(struct type_c_data *type_c,
> > + int dr_mode, int cc)
> > +{
> > + int ret = 0;
> > +
> > + if (soc_device_match(rtk_soc_kylin))
> > + ret = rtd129x_switch_type_c_plug_config(type_c, dr_mode,
> cc);
> > +
> > + if (ret < 0)
> > + dev_err(type_c->dev, "%s: Error set type c plug config\n",
> > + __func__);
> > +}
> > +
> > +static void switch_type_c_dr_mode(struct type_c_data *type_c, int
> dr_mode, int cc)
> > +{
> > + bool is_host = false;
> > + bool is_device = false;
> > + bool polarity = false;
> > + bool vbus = false;
> > + bool ss = true;
> > +
> > + switch_type_c_plug_config(type_c, dr_mode, cc);
> > + if (cc == ENABLE_CC2)
> > + polarity = true;
> > +
> > + switch (dr_mode) {
> > + case USB_DR_MODE_HOST:
> > + is_host = true;
> > + break;
> > + case USB_DR_MODE_PERIPHERAL:
> > + is_device = true;
> > + vbus = true;
> > + break;
> > + default:
> > + dev_dbg(type_c->dev, "%s dr_mode=%d ==> no host or
> device\n",
> > + __func__, dr_mode);
> > + break;
> > + }
> > +
> > + dev_dbg(type_c->dev, "%s is_host=%d is_device=%d vbus=%d
> polarity=%d\n",
> > + __func__, is_host, is_device, vbus, polarity);
> > +
> > + /* for EXTCON_USB device mode */
> > + extcon_set_state(type_c->edev, EXTCON_USB, is_device);
> > + extcon_set_property(type_c->edev, EXTCON_USB,
> > + EXTCON_PROP_USB_VBUS,
> > + (union extcon_property_value)(int)vbus);
> > + extcon_set_property(type_c->edev, EXTCON_USB,
> > + EXTCON_PROP_USB_TYPEC_POLARITY,
> > + (union extcon_property_value)(int)polarity);
> > + extcon_set_property(type_c->edev, EXTCON_USB,
> > + EXTCON_PROP_USB_SS,
> > + (union extcon_property_value)(int)ss);
> > +
> > + /* for EXTCON_USB_HOST host mode */
> > + extcon_set_state(type_c->edev, EXTCON_USB_HOST, is_host);
> > + extcon_set_property(type_c->edev, EXTCON_USB_HOST,
> > + EXTCON_PROP_USB_VBUS,
> > + (union extcon_property_value)(int)vbus);
> > + extcon_set_property(type_c->edev, EXTCON_USB_HOST,
> > + EXTCON_PROP_USB_TYPEC_POLARITY,
> > + (union extcon_property_value)(int)polarity);
> > + extcon_set_property(type_c->edev, EXTCON_USB_HOST,
> > + EXTCON_PROP_USB_SS,
> > + (union extcon_property_value)(int)ss);
> > +
> > + /* sync EXTCON_USB and EXTCON_USB_HOST */
> > + extcon_sync(type_c->edev, EXTCON_USB);
> > + extcon_sync(type_c->edev, EXTCON_USB_HOST);
> > +
> > +#ifdef CONFIG_TYPEC> + if (type_c->port) {
> > + switch (dr_mode) {
> > + case USB_DR_MODE_HOST:
> > + typec_set_data_role(type_c->port, TYPEC_HOST);
> > + typec_set_pwr_role(type_c->port,
> TYPEC_SOURCE);
> > + break;
> > + case USB_DR_MODE_PERIPHERAL:
> > + typec_set_data_role(type_c->port,
> TYPEC_DEVICE);
> > + typec_set_pwr_role(type_c->port, TYPEC_SINK);
> > + break;
> > + default:
> > + dev_dbg(type_c->dev, "%s unknown
> dr_mode=%d\n",
> > + __func__, dr_mode);
> > + break;
> > + }
> > + }
> > +#endif
> > +}
> > +
> > +/* device attached/detached */
> > +static int device_attached(struct type_c_data *type_c, u32 cc)
> > +{
> > + void __iomem *reg = type_c->reg_base + USB_TYPEC_CTRL;
> > +
> > + cancel_delayed_work(&type_c->delayed_work);
> > +
> > + switch_type_c_dr_mode(type_c, USB_DR_MODE_HOST, cc);
> > +
> > + writel(ENABLE_TYPE_C_DETECT | readl(reg), reg);
> > +
> > + return 0;
> > +}
>
> device_attached() funciton is same with host_connected()
> except for USR_BR_MODE_HOST, USB_DR_MODE_PERIPHERAL setting.
>
> You can use only one function with 3rd paramer (dr_mode) as following
> in order to reduce the duplicated code.
> For example,
> static int connector_attached(struct type_c_data *type_c, u32 cc, int
> dr_mode)

I will simplify this.

> > +
> > +static int device_detached(struct type_c_data *type_c, u32 cc)
> > +{
> > + void __iomem *reg = type_c->reg_base + USB_TYPEC_CTRL;
> > +
> > + writel(~ENABLE_TYPE_C_DETECT & readl(reg), reg);
> > +
> > + switch_type_c_dr_mode(type_c, 0, cc);
> > +
> > + schedule_delayed_work(&type_c->delayed_work,
> msecs_to_jiffies(DETECT_TIME));
> > +
> > + return 0;
> > +}
>
> device_detached is perfectly same with host_disconnected.
> You can use the only one function to reduce the duplicated code
>
> > +
> > +/* host connect/disconnect*/
> > +static int host_connected(struct type_c_data *type_c, u32 cc)
> > +{
> > + void __iomem *reg = type_c->reg_base + USB_TYPEC_CTRL;
> > +
> > + cancel_delayed_work(&type_c->delayed_work);
> > +
> > + switch_type_c_dr_mode(type_c, USB_DR_MODE_PERIPHERAL, cc);
> > +
> > + writel(ENABLE_TYPE_C_DETECT | readl(reg), reg);
> > + return 0;
> > +}
> > +
> > +static int host_disconnected(struct type_c_data *type_c, u32 cc)
> > +{
> > + void __iomem *reg = type_c->reg_base + USB_TYPEC_CTRL;
> > +
> > + writel(~ENABLE_TYPE_C_DETECT & readl(reg), reg);
> > +
> > + switch_type_c_dr_mode(type_c, 0, cc);
> > +
> > + schedule_delayed_work(&type_c->delayed_work,
> msecs_to_jiffies(DETECT_TIME));
> > +
> > + return 0;
> > +}
> > +
> > +/* detect host device switch */
> > +static int __detect_host_device(struct type_c_data *type_c, u32
> rp_or_rd_en)
> > +{
> > + struct device *dev = type_c->dev;
> > + void __iomem *reg_base = type_c->reg_base;
> > + unsigned int gpio = type_c->rd_ctrl_gpio;
> > + u32 cc1_config, cc2_config, default_ctrl;
> > + u32 cc1_switch = 0;
> > +
> > + default_ctrl = readl(reg_base + USB_TYPEC_CTRL) &
> DEBOUNCE_TIME_MASK;
> > + writel(default_ctrl, reg_base + USB_TYPEC_CTRL);
> > +
> > + cc1_config = readl(reg_base + USB_TYPEC_CTRL_CC1_0);
> > + cc2_config = readl(reg_base + USB_TYPEC_CTRL_CC2_0);
> > +
> > + cc1_config &= ~EN_CC_DET;
> > + cc2_config &= ~EN_CC_DET;
> > + writel(cc1_config, reg_base + USB_TYPEC_CTRL_CC1_0);
> > + writel(cc2_config, reg_base + USB_TYPEC_CTRL_CC2_0);
> > +
> > + if (soc_device_match(rtk_soc_kylin))
> > + cc1_switch = cc1_config & CC_SWITCH_MASK;
> > +
> > + cc1_config &= CC_CODE_MASK;
> > + cc1_config |= rp_or_rd_en | cc1_switch;
> > + cc2_config &= CC_CODE_MASK;
> > + cc2_config |= rp_or_rd_en;
> > + writel(cc2_config, reg_base + USB_TYPEC_CTRL_CC2_0);
> > + writel(cc1_config, reg_base + USB_TYPEC_CTRL_CC1_0);
> > +
> > + /* For kylin to disable external rd control gpio */
> > + if (soc_device_match(rtk_soc_kylin)) {
> > + if (gpio != -1 && gpio_is_valid(gpio)) {
> > + if (gpio_direction_output(gpio, 1))
> > + dev_err(dev, "%s ERROR rd_ctrl_gpio=1
> fail\n", __func__);
> > + }
> > + }
> > +
> > + cc1_config |= EN_CC_DET;
> > + cc2_config |= EN_CC_DET;
> > + writel(cc1_config, reg_base + USB_TYPEC_CTRL_CC1_0);
> > + writel(cc2_config, reg_base + USB_TYPEC_CTRL_CC2_0);
> > +
> > + return 0;
> > +}
> > +
> > +static int detect_device(struct type_c_data *type_c)
> > +{
> > + return __detect_host_device(type_c, type_c->dfp_mode_rp_en);
> > +}
> > +
> > +static int detect_host(struct type_c_data *type_c)
> > +{
> > + return __detect_host_device(type_c, type_c->ufp_mode_rd_en);
> > +}
> > +
> > +static int host_device_switch_detection(struct type_c_data *type_c)
> > +{
> > + if (type_c->cc_mode == IN_HOST_MODE) {
> > + type_c->cc_mode = IN_DEVICE_MODE;
> > + detect_host(type_c);
> > + } else {
> > + type_c->cc_mode = IN_HOST_MODE;
> > + detect_device(type_c);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int detect_type_c_state(struct type_c_data *type_c)
> > +{
> > + struct device *dev = type_c->dev;
> > + void __iomem *reg_base = type_c->reg_base;
> > + u32 int_status, cc_status, cc_status_check;
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&type_c->lock, flags);
> > +
> > + int_status = readl(reg_base + USB_TYPEC_CTRL);
> > + cc_status = readl(reg_base + USB_TYPEC_STS);
> > +
> > + type_c->connect_change = CONNECT_NO_CHANGE;
> > +
> > + switch (type_c->cc_mode | type_c->is_attach) {
> > + case IN_HOST_MODE | IN_ATTACH:
> > + if (((cc_status & CC1_DET_STS) == CC1_DET_STS) &&
> type_c->at_cc1 == AT_CC1) {
> > + dev_dbg(dev, "IN host mode and cc1 device
> detach (cc_status=0x%x)",
> > + cc_status);
> > + type_c->is_attach = TO_DETACH;
> > + type_c->connect_change = CONNECT_CHANGE;
> > + } else if (((cc_status & CC2_DET_STS) == CC2_DET_STS) &&
> > + type_c->at_cc1 == AT_CC2) {
> > + dev_dbg(dev, "IN host mode and cc2 device
> detach (cc_status=0x%x)",
> > + cc_status);
> > + type_c->is_attach = TO_DETACH;
> > + type_c->connect_change = CONNECT_CHANGE;
> > + }
> > + break;
> > + case IN_HOST_MODE | IN_DETACH:
> > + cc_status_check = readl(reg_base + USB_TYPEC_STS);
> > + if (cc_status_check != (CC1_DET_STS | CC2_DET_STS)) {
> > + if (in_interrupt()) {
> > + mdelay(300);
> > + } else {
> > + spin_unlock_irqrestore(&type_c->lock,
> flags);
> > + msleep(300);
> > + spin_lock_irqsave(&type_c->lock, flags);
> > + }
>
> As I commented above, it seems that there are some hardware constrains
> which requires the some delay when changing the h/w state.
> I recommend that you need to add the comment of why adding the
> delays/sleep.

Ok, I will add.

> > + cc_status_check = readl(reg_base +
> USB_TYPEC_STS);
> > + }
> > + if (cc_status != cc_status_check) {
> > + dev_warn(dev, "IN_HOST_MODE: cc_status
> (0x%x) != cc_status_check (0x%x)\n",
> > + cc_status, cc_status_check);
> > + cc_status = readl(reg_base + USB_TYPEC_STS);
> > + }
> > +
> > + if ((cc_status & CC1_DET_STS) == CC1_DET_STS_RD) {
> > + dev_dbg(dev, "IN host mode and cc1 device
> attach (cc_status=0x%x)",
> > + cc_status);
> > + type_c->is_attach = TO_ATTACH;
> > + type_c->at_cc1 = AT_CC1;
> > + type_c->connect_change = CONNECT_CHANGE;
> > + } else if ((cc_status & CC2_DET_STS) == CC2_DET_STS_RD)
> {
> > + dev_dbg(dev, "In host mode and cc2 device attach
> (cc_status=0x%x)",
> > + cc_status);
> > + type_c->is_attach = TO_ATTACH;
> > + type_c->at_cc1 = AT_CC2;
> > + type_c->connect_change = CONNECT_CHANGE;
> > + }
> > + break;
> > + case IN_DEVICE_MODE | IN_ATTACH:
> > + if ((cc_status & CC1_DET_STS) < CC1_DET_STS_RP ||
> > + (cc_status & CC2_DET_STS) < CC2_DET_STS_RP) {
> > + /* Add a sw debounce to filter cc signal sent from
> apple pd adapter */
> > + mdelay(5);
> > + cc_status_check = readl(reg_base +
> USB_TYPEC_STS);
> > +
> > + if (cc_status != cc_status_check) {
> > + dev_dbg(dev, "IN_DEVICE_MODE:
> cc_status (0x%x) != cc_status_check (0x%x) maybe use a pd adapter\n",
> > + cc_status, cc_status_check);
> > + cc_status = cc_status_check;
> > + }
> > + }
> > +
> > + if ((cc_status & CC1_DET_STS) < CC1_DET_STS_RP &&
> type_c->at_cc1 == AT_CC1) {
> > + dev_dbg(dev, "IN device mode and cc1 host
> disconnect (cc_status=0x%x)",
> > + cc_status);
> > + type_c->is_attach = TO_DETACH;
> > + type_c->connect_change = CONNECT_CHANGE;
> > + } else if ((cc_status & CC2_DET_STS) < CC2_DET_STS_RP
> &&
> > + type_c->at_cc1 == AT_CC2) {
> > + dev_dbg(dev, "IN device mode and cc2 host
> disconnect (cc_status=0x%x)",
> > + cc_status);
> > + type_c->is_attach = TO_DETACH;
> > + type_c->connect_change = CONNECT_CHANGE;
> > + }
> > + break;
> > + case IN_DEVICE_MODE | IN_DETACH:
> > + cc_status_check = readl(reg_base + USB_TYPEC_STS);
> > + if (cc_status_check != 0x0) {
> > + if (in_interrupt()) {
> > + mdelay(300);
> > + } else {
> > + spin_unlock_irqrestore(&type_c->lock,
> flags);
> > + msleep(300);
> > + spin_lock_irqsave(&type_c->lock, flags);
> > + }
>
> ditto.
>
> > + cc_status_check = readl(reg_base +
> USB_TYPEC_STS);
> > + }
> > +
> > + if (cc_status != cc_status_check) {
> > + dev_warn(dev, "IN_DEVICE_MODE: cc_status
> (0x%x) != cc_status_check (0x%x)\n",
> > + cc_status, cc_status_check);
> > + cc_status = readl(reg_base + USB_TYPEC_STS);
> > + }
> > +
> > + if ((cc_status & CC1_DET_STS) >= CC1_DET_STS_RP) {
> > + dev_dbg(dev, "IN device mode and cc1 host
> connect (cc_status=0x%x)",
> > + cc_status);
> > + type_c->at_cc1 = AT_CC1;
> > + type_c->is_attach = TO_ATTACH;
> > + type_c->connect_change = CONNECT_CHANGE;
> > + } else if ((cc_status & CC2_DET_STS) >= CC2_DET_STS_RP) {
> > + dev_dbg(dev, "IN device mode and cc2 host
> connect (cc_status=0x%x)",
> > + cc_status);
> > + type_c->at_cc1 = AT_CC2;
> > + type_c->is_attach = TO_ATTACH;
> > + type_c->connect_change = CONNECT_CHANGE;
> > + }
> > + break;
> > + default:
> > + dev_err(dev, "error host or device mode (cc_mode=%d,
> is_attach=%d) ",
> > + type_c->cc_mode, type_c->is_attach);
> > + }
> > +
> > + type_c->int_status = int_status;
> > + type_c->cc_status = cc_status;
> > +
> > + spin_unlock_irqrestore(&type_c->lock, flags);
> > + return 0;
> > +}
> > +
> > +static void host_device_switch(struct work_struct *work)
> > +{
> > + struct type_c_data *type_c = container_of(work, struct type_c_data,
> > +
> delayed_work.work);
> > + struct device *dev = type_c->dev;
> > + unsigned long flags;
> > + int connect_change = 0;
> > + int cc_mode = 0;
> > + int is_attach = 0;
> > + int at_cc1 = 0;
> > +
> > + spin_lock_irqsave(&type_c->lock, flags);
> > + if (type_c->connect_change)
> > + connect_change = type_c->connect_change;
> > + spin_unlock_irqrestore(&type_c->lock, flags);
>
> Better to convert this code as following if type_c->connect_change is NULL,
> you can reduce the lock/unlok operation of spin_lock. As you knew, spin_lock is
> expensive.
>
> if (type_c->connect_change) {
> spin_lock_irqsave(&type_c->lock, flags);
> connect_change = type_c->connect_change;
> spin_unlock_irqrestore(&type_c->lock, flags);
> }

I want to protect type_c->connect_change.
Moving the lock to the if statement loses its original purpose.

> > +
> > + if (!connect_change)
> > + detect_type_c_state(type_c);
> > +
> > + spin_lock_irqsave(&type_c->lock, flags);
> > + if (type_c->connect_change) {
> > + connect_change = type_c->connect_change;
> > + cc_mode = type_c->cc_mode;
> > + is_attach = type_c->is_attach;
> > + at_cc1 = type_c->at_cc1;
> > + type_c->connect_change = CONNECT_NO_CHANGE;
> > + } else {
> > + host_device_switch_detection(type_c);
> > +
> > + schedule_delayed_work(&type_c->delayed_work,
> msecs_to_jiffies(DETECT_TIME));
> > + }
> > + spin_unlock_irqrestore(&type_c->lock, flags);
> > +
> > + if (connect_change) {
>
> nitpick.
> You can reduce the depth of indentation when connector_change is 0
> by changing the code as following: If possible, I always to try
> to reduce the indentation depth for the readability.
>
> if (!connect_change)
> return
>
> dev_dbg(dev, "%s: usb cable connection change\n", __func__);
> if (cc_mode == IN_HOST_MODE) {
> ......
>
Ok, I will revise.
> > + dev_dbg(dev, "%s: usb cable connection change\n",
> __func__);
> > + if (cc_mode == IN_HOST_MODE) {
> > + if (is_attach && at_cc1)
> > + device_attached(type_c, ENABLE_CC1);
> > + else if (is_attach && !at_cc1)
> > + device_attached(type_c, ENABLE_CC2);
> > + else
> > + device_detached(type_c, DISABLE_CC);
> > + } else if (cc_mode == IN_DEVICE_MODE) {
> > + if (is_attach && at_cc1)
> > + host_connected(type_c, ENABLE_CC1);
> > + else if (is_attach && !at_cc1)
> > + host_connected(type_c, ENABLE_CC2);
> > + else
> > + host_disconnected(type_c,
> DISABLE_CC);
> > + } else {
> > + dev_err(dev, "Error: IN unknown mode %d to %s
> at %s (cc_status=0x%x)\n",
> > + cc_mode, is_attach ? "attach" :
> "detach",
> > + at_cc1 ? "cc1" : "cc2",
> type_c->cc_status);
> > + }
> > + dev_info(dev, "Connection change OK: IN %s mode to %s at
> %s (cc_status=0x%x)\n",
> > + cc_mode == IN_HOST_MODE ? "host" : "device",
> > + is_attach ? "attach" : "detach",
> > + at_cc1 ? "cc1" : "cc2", type_c->cc_status);
> > + }
> > +}
> > +
> > +static irqreturn_t type_c_detect_irq(int irq, void *__data)
> > +{
> > + struct type_c_data *type_c = (struct type_c_data *)__data;
> > + struct device *dev = type_c->dev;
> > + void __iomem *reg = type_c->reg_base + USB_TYPEC_CTRL;
> > + unsigned long flags;
> > +
> > + detect_type_c_state(type_c);
> > +
> > + spin_lock_irqsave(&type_c->lock, flags);
> > +
> > + if (type_c->connect_change) {
> > + dev_dbg(dev, "%s: IN %s mode to %s (at %s interrupt)
> int_status=0x%x, cc_status=0x%x",
> > + __func__,
> > + type_c->cc_mode == IN_HOST_MODE ? "host" :
> "device",
> > + type_c->is_attach ? "attach" : "detach",
> > + type_c->at_cc1 ? "cc1" : "cc2",
> > + type_c->int_status, type_c->cc_status);
> > +
> > + /* clear interrupt status */
> > + writel(~ALL_CC_INT_STS & readl(reg), reg);
> > +
> > + cancel_delayed_work(&type_c->delayed_work);
> > + schedule_delayed_work(&type_c->delayed_work,
> msecs_to_jiffies(0));
> > + } else {
> > + static int local_count;
> > +
> > + if (local_count++ > 10) {
>
> It seems that clear interrupt status once every 10 times.
> I think that it is not generic. You need to add the comment of why trying to
> clear
> interrupt once every 10 times.

I will add the comment.
> > + /* clear interrupt status */
> > + writel(~ALL_CC_INT_STS & readl(reg), reg);
> > + local_count = 0;
> > + }
> > + }
> > +
> > + spin_unlock_irqrestore(&type_c->lock, flags);
> > +
> > + return IRQ_HANDLED;
> > +}
>
> (snip)
>
> > +
> > +static int extcon_rtk_type_c_init(struct type_c_data *type_c)
> > +{
> > + struct device *dev = type_c->dev;
> > + unsigned long flags;
> > + void __iomem *reg;
> > + int val;
> > +
> > + if ((type_c->rd_ctrl_gpio != -1) &&
>
> nitpick.
> After getting the gpio number by using of_get_named_gpio() function,
> type_c->rd_ctrl_gpio is error number which is returned of_get_named_gpio().
>
> In order to specify the more correctly, how about changing it as following?
> Because I don't use the '-1' constant variable without any constant defintion.
> Someone cannot understand the meaning of '-1'.
>
> if (!(type_c->rd_ctrl_gpio < 0))

I will revise.

> > + gpio_request(type_c->rd_ctrl_gpio, dev->of_node->name))
> > + dev_err(dev, "%s ERROR Request rd_ctrl_gpio (id=%d)
> fail\n",
> > + __func__, type_c->rd_ctrl_gpio);
> > +
> > + spin_lock_irqsave(&type_c->lock, flags);
> > +
> > + /* set parameter */
> > + reg = type_c->reg_base + USB_TYPEC_CTRL_CC1_0;
> > + val = readl(reg);
> > + val = (~CC_CODE_MASK & val) | (type_c->cc1_code &
> CC_CODE_MASK);
> > + writel(val, reg);
> > +
> > + reg = type_c->reg_base + USB_TYPEC_CTRL_CC2_0;
> > + val = readl(reg);
> > + val = (~CC_CODE_MASK & val) | (type_c->cc2_code &
> CC_CODE_MASK);
> > +
> > + reg = type_c->reg_base + USB_TYPEC_CTRL_CC1_1;
> > + writel(type_c->cc1_vref, reg);
> > +
> > + reg = type_c->reg_base + USB_TYPEC_CTRL_CC2_1;
> > + writel(type_c->cc2_vref, reg);
> > +
> > + reg = type_c->reg_base + USB_TYPEC_CTRL;
> > + val = readl(reg);
> > + val = (~DEBOUNCE_TIME_MASK & val) | (type_c->debounce &
> DEBOUNCE_TIME_MASK);
> > +
> > + dev_info(dev, "First check USB_DR_MODE_PERIPHERAL");
> > + type_c->cc_mode = IN_DEVICE_MODE;
> > + type_c->is_attach = IN_DETACH;
> > + type_c->connect_change = CONNECT_NO_CHANGE;
> > +
> > + detect_host(type_c);
> > +
> > + spin_unlock_irqrestore(&type_c->lock, flags);
> > +
> > + schedule_delayed_work(&type_c->delayed_work,
> msecs_to_jiffies(0));
> > +
> > +#ifdef CONFIG_TYPEC
>
> Don'n need to check 'CONFIG_TYPEC' defintion wiht ifdef.
> 'ifdef' might make the code more complicated.
>
> If type_c is optional, just print warning message
> when typec_regiser_port returns error.
>
> This patch is already check whether 'type_c->port' is NULL or not
> to handle the code related to typec.

If I remove check 'CONFIG_TYPEC' definition,
it will fail the build when CONFIG_TYPEC=n.
Another option is to select CONFIG_TYPEC in Kconfig.

> > + if (!type_c->port) {
>
> (snip)
>
> > +
> > + type_c->port = typec_register_port(type_c->dev,
> &typec_cap);
> > + if (IS_ERR(type_c->port))
> > + return PTR_ERR(type_c->port);
> > + }> +#endif /* CONFIG_TYPEC */
> > +
> > + return 0;
> > +}
> > +
> > +static int extcon_rtk_type_c_edev_register(struct type_c_data *type_c)
> > +{
> > + struct device *dev = type_c->dev;
> > + int ret = 0;
> > +
> > + type_c->edev = devm_extcon_dev_allocate(dev, usb_type_c_cable);
> > + if (IS_ERR(type_c->edev)) {
> > + dev_err(dev, "failed to allocate extcon device\n");
> > + return -ENOMEM;
> > + }
> > +
> > + ret = devm_extcon_dev_register(dev, type_c->edev);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to register extcon device\n");
> > + return ret;
> > + }
> > +
> > + extcon_set_property_capability(type_c->edev, EXTCON_USB,
> > + EXTCON_PROP_USB_VBUS);
> > + extcon_set_property_capability(type_c->edev, EXTCON_USB,
> > +
> EXTCON_PROP_USB_TYPEC_POLARITY);
> > + extcon_set_property_capability(type_c->edev, EXTCON_USB,
> > + EXTCON_PROP_USB_SS);
> > +
> > + extcon_set_property_capability(type_c->edev, EXTCON_USB_HOST,
> > + EXTCON_PROP_USB_VBUS);
> > + extcon_set_property_capability(type_c->edev, EXTCON_USB_HOST,
> > +
> EXTCON_PROP_USB_TYPEC_POLARITY);
> > + extcon_set_property_capability(type_c->edev, EXTCON_USB_HOST,
> > + EXTCON_PROP_USB_SS);
> > +
> > + return ret;
> > +}
> > +
> > +static int extcon_rtk_type_c_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct type_c_data *type_c;
> > + const struct type_c_cfg *type_c_cfg;
> > + unsigned int gpio;
> > + int ret = 0;
> > +
> > + type_c = devm_kzalloc(dev, sizeof(*type_c), GFP_KERNEL);
> > + if (!type_c)
> > + return -ENOMEM;
> > +
> > + type_c->reg_base = devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(type_c->reg_base))
> > + return PTR_ERR(type_c->reg_base);
> > +
> > + type_c->dev = dev;
> > +
> > + type_c->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> > + if (type_c->irq <= 0) {
> > + dev_err(&pdev->dev, "Type C driver with no IRQ. Check %s
> setup!\n",
> > + dev_name(&pdev->dev));
> > + ret = -ENODEV;
> > + goto err1;
> > + }
> > +
> > + ret = devm_request_irq(dev, type_c->irq, type_c_detect_irq,
> > + IRQF_SHARED, "type_c_detect", type_c);
> > +
> > + spin_lock_init(&type_c->lock);
> > +
> > + type_c->rd_ctrl_gpio = -1;
> > + if (soc_device_match(rtk_soc_kylin)) {
> > + gpio = of_get_named_gpio(dev->of_node,
> "realtek,rd-ctrl-gpio", 0);
> > + if (gpio_is_valid(gpio)) {
> > + type_c->rd_ctrl_gpio = gpio;
> > + dev_dbg(dev, "%s get rd-ctrl-gpio (id=%d) OK\n",
> __func__, gpio);
> > + } else {
> > + dev_err(dev, "Error rd_ctrl-gpio no found");
> > + }
> > + }
> > +
> > + type_c_cfg = of_device_get_match_data(dev);
> > + if (!type_c_cfg) {
> > + dev_err(dev, "type_c config are not assigned!\n");
> > + return -EINVAL;
> > + }
> > +
> > + type_c->type_c_cfg = devm_kzalloc(dev, sizeof(*type_c_cfg),
> GFP_KERNEL);
> > +
> > + memcpy(type_c->type_c_cfg, type_c_cfg, sizeof(*type_c_cfg));
> > +
> > + if (setup_type_c_parameter(type_c)) {
> > + dev_err(dev, "ERROR: %s to setup type c parameter!!",
> __func__);
> > + ret = -EINVAL;
> > + goto err1;
> > + }
> > +
> > + INIT_DELAYED_WORK(&type_c->delayed_work, host_device_switch);
> > +
> > + ret = extcon_rtk_type_c_init(type_c);
> > + if (ret) {
> > + dev_err(dev, "%s failed to init type_c\n", __func__);
> > + goto err1;
> > + }
> > +
> > + platform_set_drvdata(pdev, type_c);
> > +
> > + ret = extcon_rtk_type_c_edev_register(type_c);
> > +
> > + create_debug_files(type_c);
> > +
> > + return 0;
> > +
> > +err1:
> > + dev_err(&pdev->dev, "%s: Probe fail, %d\n", __func__, ret);
> > +
> > + return ret;
> > +}
> > +
> > +static void extcon_rtk_type_c_remove(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct type_c_data *type_c = dev_get_drvdata(dev);
> > + u32 default_ctrl;
> > + unsigned long flags;
> > +
> > + remove_debug_files(type_c);
> > +
> > +#ifdef CONFIG_TYPEC
> > + if (type_c->port) {
> > + typec_unregister_port(type_c->port);
> > + type_c->port = NULL;
> > + }
> > +#endif
> > +
> > + cancel_delayed_work_sync(&type_c->delayed_work);
> > + flush_delayed_work(&type_c->delayed_work);
> > + WARN_ON_ONCE(delayed_work_pending(&type_c->delayed_work));
> > +
> > + spin_lock_irqsave(&type_c->lock, flags);
> > + /* disable interrupt */
> > + default_ctrl = readl(type_c->reg_base + USB_TYPEC_CTRL) &
> > + DEBOUNCE_TIME_MASK;
> > + writel(default_ctrl, type_c->reg_base + USB_TYPEC_CTRL);
> > +
> > + /* disable cc detect, rp, rd */
> > + writel(PLR_EN, type_c->reg_base + USB_TYPEC_CTRL_CC1_0);
> > + writel(0, type_c->reg_base + USB_TYPEC_CTRL_CC2_0);
> > +
> > + spin_unlock_irqrestore(&type_c->lock, flags);
> > +
> > + free_irq(type_c->irq, type_c);
> > +}
> > +
> > +static const struct type_c_cfg rtd1295_type_c_cfg = {
> > + .parameter_ver = PARAMETER_V0,
> > + .cc_dfp_mode = CC_MODE_DFP_3_0,
> > + .cc1_param = { .rp_4p7k_code = 0xb,
> > + .rp_36k_code = 0x17,
> > + .rp_12k_code = 0x10,
> > + .rd_code = 0,
> > + .ra_code = 0,
> > + .vref_2p6v = 0x0,
> > + .vref_1p23v = 0x0,
> > + .vref_0p8v = 0x3,
> > + .vref_0p66v = 0x0,
> > + .vref_0p4v = 0x0,
> > + .vref_0p2v = 0x4,
> > + .vref_1_1p6v = 0,
> > + .vref_0_1p6v = 0 },
> > + .cc2_param = { .rp_4p7k_code = 0xc,
> > + .rp_36k_code = 0x17,
> > + .rp_12k_code = 0x12,
> > + .rd_code = 0,
> > + .ra_code = 0,
> > + .vref_2p6v = 0x2,
> > + .vref_1p23v = 0x0,
> > + .vref_0p8v = 0x3,
> > + .vref_0p66v = 0x0,
> > + .vref_0p4v = 0x0,
> > + .vref_0p2v = 0x5,
> > + .vref_1_1p6v = 0,
> > + .vref_0_1p6v = 0 },
> > + .debounce_val = 0x7f, /* 1b,1us 7f,4.7us */
> > + .use_defalut_parameter = false,
> > +};
> > +
> > +static const struct type_c_cfg rtd1395_type_c_cfg = {
> > + .parameter_ver = PARAMETER_V0,
> > + .cc_dfp_mode = CC_MODE_DFP_3_0,
> > + .cc1_param = { .rp_4p7k_code = 0xc,
> > + .rp_36k_code = 0xb,
> > + .rp_12k_code = 0xe,
> > + .rd_code = 0x10,
> > + .ra_code = 0x0,
> > + .vref_2p6v = 0x0,
> > + .vref_1p23v = 0x1,
> > + .vref_0p8v = 0x0,
> > + .vref_0p66v = 0x0,
> > + .vref_0p4v = 0x3,
> > + .vref_0p2v = 0x0,
> > + .vref_1_1p6v = 0x7,
> > + .vref_0_1p6v = 0x7 },
> > + .cc2_param = { .rp_4p7k_code = 0xb,
> > + .rp_36k_code = 0x9,
> > + .rp_12k_code = 0xe,
> > + .rd_code = 0xf,
> > + .ra_code = 0x0,
> > + .vref_2p6v = 0x1,
> > + .vref_1p23v = 0x3,
> > + .vref_0p8v = 0x3,
> > + .vref_0p66v = 0x2,
> > + .vref_0p4v = 0x3,
> > + .vref_0p2v = 0x2,
> > + .vref_1_1p6v = 0x7,
> > + .vref_0_1p6v = 0x7 },
> > + .debounce_val = 0x7f, /* 1b,1us 7f,4.7us */
> > + .use_defalut_parameter = false,
> > +};
> > +
> > +static const struct type_c_cfg rtd1619_type_c_cfg = {
> > + .parameter_ver = PARAMETER_V0,
> > + .cc_dfp_mode = CC_MODE_DFP_3_0,
> > + .cc1_param = { .rp_4p7k_code = 0xc,
> > + .rp_36k_code = 0xf,
> > + .rp_12k_code = 0xe,
> > + .rd_code = 0x11,
> > + .ra_code = 0x0,
> > + .vref_2p6v = 0x5,
> > + .vref_1p23v = 0x7,
> > + .vref_0p8v = 0xa,
> > + .vref_0p66v = 0xa,
> > + .vref_0p4v = 0x3,
> > + .vref_0p2v = 0x2,
> > + .vref_1_1p6v = 0x7,
> > + .vref_0_1p6v = 0x7 },
> > + .cc2_param = { .rp_4p7k_code = 0xc,
> > + .rp_36k_code = 0xf,
> > + .rp_12k_code = 0xe,
> > + .rd_code = 0xf,
> > + .ra_code = 0x0,
> > + .vref_2p6v = 0x5,
> > + .vref_1p23v = 0x8,
> > + .vref_0p8v = 0xa,
> > + .vref_0p66v = 0xa,
> > + .vref_0p4v = 0x3,
> > + .vref_0p2v = 0x2,
> > + .vref_1_1p6v = 0x7,
> > + .vref_0_1p6v = 0x7 },
> > + .debounce_val = 0x7f, /* 1b,1us 7f,4.7us */
> > + .use_defalut_parameter = false,
> > +};
> > +
> > +static const struct type_c_cfg rtd1319_type_c_cfg = {
> > + .parameter_ver = PARAMETER_V0,
> > + .cc_dfp_mode = CC_MODE_DFP_1_5,
> > + .cc1_param = { .rp_4p7k_code = 0x9,
> > + .rp_36k_code = 0xe,
> > + .rp_12k_code = 0x9,
> > + .rd_code = 0x9,
> > + .ra_code = 0x7,
> > + .vref_2p6v = 0x3,
> > + .vref_1p23v = 0x7,
> > + .vref_0p8v = 0x7,
> > + .vref_0p66v = 0x6,
> > + .vref_0p4v = 0x2,
> > + .vref_0p2v = 0x3,
> > + .vref_1_1p6v = 0x4,
> > + .vref_0_1p6v = 0x7 },
> > + .cc2_param = { .rp_4p7k_code = 0x8,
> > + .rp_36k_code = 0xe,
> > + .rp_12k_code = 0x9,
> > + .rd_code = 0x9,
> > + .ra_code = 0x7,
> > + .vref_2p6v = 0x3,
> > + .vref_1p23v = 0x7,
> > + .vref_0p8v = 0x7,
> > + .vref_0p66v = 0x6,
> > + .vref_0p4v = 0x3,
> > + .vref_0p2v = 0x3,
> > + .vref_1_1p6v = 0x6,
> > + .vref_0_1p6v = 0x7 },
> > + .debounce_val = 0x7f, /* 1b,1us 7f,4.7us */
> > + .use_defalut_parameter = false,
> > +};
> > +
> > +static const struct type_c_cfg rtd1312c_type_c_cfg = {
> > + .parameter_ver = PARAMETER_V0,
> > + .cc_dfp_mode = CC_MODE_DFP_1_5,
> > + .cc1_param = { .rp_4p7k_code = 0xe,
> > + .rp_36k_code = 0xc,
> > + .rp_12k_code = 0xc,
> > + .rd_code = 0xa,
> > + .ra_code = 0x3,
> > + .vref_2p6v = 0xa,
> > + .vref_1p23v = 0x7,
> > + .vref_0p8v = 0x7,
> > + .vref_0p66v = 0x7,
> > + .vref_0p4v = 0x4,
> > + .vref_0p2v = 0x4,
> > + .vref_1_1p6v = 0x7,
> > + .vref_0_1p6v = 0x7 },
> > + .cc2_param = { .rp_4p7k_code = 0xe,
> > + .rp_36k_code = 0xc,
> > + .rp_12k_code = 0xc,
> > + .rd_code = 0xa,
> > + .ra_code = 0x3,
> > + .vref_2p6v = 0xa,
> > + .vref_1p23v = 0x7,
> > + .vref_0p8v = 0x7,
> > + .vref_0p66v = 0x7,
> > + .vref_0p4v = 0x4,
> > + .vref_0p2v = 0x4,
> > + .vref_1_1p6v = 0x7,
> > + .vref_0_1p6v = 0x7 },
> > + .debounce_val = 0x7f, /* 1b,1us 7f,4.7us */
> > + .use_defalut_parameter = false,
> > +};
> > +
> > +static const struct type_c_cfg rtd1619b_type_c_cfg = {
> > + .parameter_ver = PARAMETER_V1,
> > + .cc_dfp_mode = CC_MODE_DFP_1_5,
> > + .cc1_param = { .rp_4p7k_code = 0xf,
> > + .rp_36k_code = 0xf,
> > + .rp_12k_code = 0xf,
> > + .rd_code = 0xf,
> > + .ra_code = 0x7,
> > + .vref_2p6v = 0x9,
> > + .vref_1p23v = 0x7,
> > + .vref_0p8v = 0x9,
> > + .vref_0p66v = 0x8,
> > + .vref_0p4v = 0x7,
> > + .vref_0p2v = 0x9,
> > + .vref_1_1p6v = 0x7,
> > + .vref_0_1p6v = 0x7 },
> > + .cc2_param = { .rp_4p7k_code = 0xf,
> > + .rp_36k_code = 0xf,
> > + .rp_12k_code = 0xf,
> > + .rd_code = 0xf,
> > + .ra_code = 0x7,
> > + .vref_1p23v = 0x7,
> > + .vref_0p8v = 0x9,
> > + .vref_0p66v = 0x8,
> > + .vref_0p4v = 0x7,
> > + .vref_0p2v = 0x8,
> > + .vref_1_1p6v = 0x7,
> > + .vref_0_1p6v = 0x7 },
> > + .debounce_val = 0x7f, /* 1b,1us 7f,4.7us */
> > + .use_defalut_parameter = false,
> > +};
> > +
> > +static const struct type_c_cfg rtd1319d_type_c_cfg = {
> > + .parameter_ver = PARAMETER_V1,
> > + .cc_dfp_mode = CC_MODE_DFP_1_5,
> > + .cc1_param = { .rp_4p7k_code = 0xe,
> > + .rp_36k_code = 0x3,
> > + .rp_12k_code = 0xe,
> > + .rd_code = 0xf,
> > + .ra_code = 0x6,
> > + .vref_2p6v = 0x7,
> > + .vref_1p23v = 0x7,
> > + .vref_0p8v = 0x8,
> > + .vref_0p66v = 0x7,
> > + .vref_0p4v = 0x7,
> > + .vref_0p2v = 0x7,
> > + .vref_1_1p6v = 0x7,
> > + .vref_0_1p6v = 0x7 },
> > + .cc2_param = { .rp_4p7k_code = 0xe,
> > + .rp_36k_code = 0x3,
> > + .rp_12k_code = 0xe,
> > + .rd_code = 0xf,
> > + .ra_code = 0x6,
> > + .vref_2p6v = 0x7,
> > + .vref_1p23v = 0x7,
> > + .vref_0p8v = 0x8,
> > + .vref_0p66v = 0x7,
> > + .vref_0p4v = 0x7,
> > + .vref_0p2v = 0x8,
> > + .vref_1_1p6v = 0x7,
> > + .vref_0_1p6v = 0x7 },
> > + .debounce_val = 0x7f, /* 1b,1us 7f,4.7us */
> > + .use_defalut_parameter = false,
> > +};
> > +
> > +static const struct type_c_cfg rtd1315e_type_c_cfg = {
> > + .parameter_ver = PARAMETER_V1,
> > + .cc_dfp_mode = CC_MODE_DFP_1_5,
> > + .cc1_param = { .rp_4p7k_code = 0xe,
> > + .rp_36k_code = 0x3,
> > + .rp_12k_code = 0xe,
> > + .rd_code = 0xf,
> > + .ra_code = 0x6,
> > + .vref_2p6v = 0x7,
> > + .vref_1p23v = 0x7,
> > + .vref_0p8v = 0x8,
> > + .vref_0p66v = 0x7,
> > + .vref_0p4v = 0x7,
> > + .vref_0p2v = 0x7,
> > + .vref_1_1p6v = 0x7,
> > + .vref_0_1p6v = 0x7 },
> > + .cc2_param = { .rp_4p7k_code = 0xe,
> > + .rp_36k_code = 0x3,
> > + .rp_12k_code = 0xe,
> > + .rd_code = 0xf,
> > + .ra_code = 0x6,
> > + .vref_2p6v = 0x7,
> > + .vref_1p23v = 0x7,
> > + .vref_0p8v = 0x8,
> > + .vref_0p66v = 0x7,
> > + .vref_0p4v = 0x7,
> > + .vref_0p2v = 0x8,
> > + .vref_1_1p6v = 0x7,
> > + .vref_0_1p6v = 0x7 },
> > + .debounce_val = 0x7f, /* 1b,1us 7f,4.7us */
> > + .use_defalut_parameter = false,
> > +};
> > +
> > +static const struct of_device_id extcon_rtk_type_c_match[] = {
> > + { .compatible = "realtek,rtd1295-type-c", .data =
> &rtd1295_type_c_cfg },
> > + { .compatible = "realtek,rtd1312c-type-c", .data =
> &rtd1312c_type_c_cfg },
> > + { .compatible = "realtek,rtd1315e-type-c", .data =
> &rtd1315e_type_c_cfg },
> > + { .compatible = "realtek,rtd1319-type-c", .data =
> &rtd1319_type_c_cfg },
> > + { .compatible = "realtek,rtd1319d-type-c", .data =
> &rtd1319d_type_c_cfg },
> > + { .compatible = "realtek,rtd1395-type-c", .data =
> &rtd1395_type_c_cfg },
> > + { .compatible = "realtek,rtd1619-type-c", .data =
> &rtd1619_type_c_cfg },
> > + { .compatible = "realtek,rtd1619b-type-c", .data =
> &rtd1619b_type_c_cfg },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, extcon_rtk_type_c_match);
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int extcon_rtk_type_c_prepare(struct device *dev)
> > +{
> > + struct type_c_data *type_c = dev_get_drvdata(dev);
> > + u32 default_ctrl;
> > + unsigned long flags;
> > +
> > + cancel_delayed_work_sync(&type_c->delayed_work);
> > + flush_delayed_work(&type_c->delayed_work);
> > + WARN_ON_ONCE(delayed_work_pending(&type_c->delayed_work));
> > +
> > + spin_lock_irqsave(&type_c->lock, flags);
> > + /* disable interrupt */
> > + default_ctrl = readl(type_c->reg_base + USB_TYPEC_CTRL) &
> > + DEBOUNCE_TIME_MASK;
> > + writel(default_ctrl, type_c->reg_base + USB_TYPEC_CTRL);
> > +
> > + /* disable cc detect, rp, rd */
> > + writel(PLR_EN, type_c->reg_base + USB_TYPEC_CTRL_CC1_0);
> > + writel(0, type_c->reg_base + USB_TYPEC_CTRL_CC2_0);
> > +
> > + spin_unlock_irqrestore(&type_c->lock, flags);
> > +
> > + return 0;
> > +}
> > +
> > +static void extcon_rtk_type_c_complete(struct device *dev)
> > +{
> > + /* nothing */
> > +}
> > +
> > +static int extcon_rtk_type_c_suspend(struct device *dev)
> > +{
> > + struct type_c_data *type_c = dev_get_drvdata(dev);
> > +
> > + if (type_c->rd_ctrl_gpio != -1)
>
> ditto.
>
> > + gpio_free(type_c->rd_ctrl_gpio);
> > +
> > + return 0;
> > +}
> > +
> > +static int extcon_rtk_type_c_resume(struct device *dev)
> > +{
> > + struct type_c_data *type_c = dev_get_drvdata(dev);
> > +
> > + extcon_rtk_type_c_init(type_c);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct dev_pm_ops extcon_rtk_type_c_pm_ops = {
> > + SET_LATE_SYSTEM_SLEEP_PM_OPS(extcon_rtk_type_c_suspend,
> extcon_rtk_type_c_resume)
> > + .prepare = extcon_rtk_type_c_prepare,
> > + .complete = extcon_rtk_type_c_complete,
> > +};
> > +
> > +#define DEV_PM_OPS (&extcon_rtk_type_c_pm_ops)
> > +#else
> > +#define DEV_PM_OPS NULL
> > +#endif /* CONFIG_PM_SLEEP */
> > +
> > +static struct platform_driver extcon_rtk_type_c_driver = {
> > + .probe = extcon_rtk_type_c_probe,
> > + .remove_new = extcon_rtk_type_c_remove,
> > + .driver = {
> > + .name = "extcon-rtk-type_c",
> > + .of_match_table = extcon_rtk_type_c_match,
> > + .pm = DEV_PM_OPS,
> > + },
> > +};
> > +
> > +module_platform_driver(extcon_rtk_type_c_driver);
> > +
> > +MODULE_DESCRIPTION("Realtek Extcon Type C driver");
> > +MODULE_ALIAS("platform:extcon-rtk-type-c");
> > +MODULE_AUTHOR("Stanley Chang <stanley_chang@xxxxxxxxxxx>");
> > +MODULE_LICENSE("GPL");
>

Thanks,
Stanley