Re: [PATCH 1/2] usb typec: mt6360: Add support for mt6360 Type-C driver
On 8/26/20 6:11 PM, 啟原黃 wrote: > Guenter Roeck 於 2020年8月27日 週四 上午9:02寫道: >> >> On 8/26/20 5:59 PM, 啟原黃 wrote: >> [ ... ] > +static int mt6360_tcpc_remove(struct platform_device *pdev) > +{ > + struct mt6360_tcpc_info *mti = platform_get_drvdata(pdev); > + > + tcpci_unregister_port(mti->tcpci); That leaves interrupts enabled, which might be racy because interrupts are still enabled here. >>> M..., yes, it will cause the race condition during module remove. >>> I'll add disable_irq before tcpci unregister to prevent it. >> >> Or just set TCPC_ALERT_MASK to 0, as in tcpci.c. > Both are right, Thx. I'll choose one. > > BTW, it seems enum typec_cc_status is used by tcpci.h. > If I don't include tcpm.h, it will raise a warning during the compile time. Ok, thanks for the note (that means tcpci.h should include linux/usb/tcpm.h, really). Guenter
Re: [PATCH 1/2] usb typec: mt6360: Add support for mt6360 Type-C driver
Guenter Roeck 於 2020年8月27日 週四 上午9:02寫道: > > On 8/26/20 5:59 PM, 啟原黃 wrote: > [ ... ] > >>> +static int mt6360_tcpc_remove(struct platform_device *pdev) > >>> +{ > >>> + struct mt6360_tcpc_info *mti = platform_get_drvdata(pdev); > >>> + > >>> + tcpci_unregister_port(mti->tcpci); > >> > >> That leaves interrupts enabled, which might be racy > >> because interrupts are still enabled here. > > M..., yes, it will cause the race condition during module remove. > > I'll add disable_irq before tcpci unregister to prevent it. > > Or just set TCPC_ALERT_MASK to 0, as in tcpci.c. Both are right, Thx. I'll choose one. BTW, it seems enum typec_cc_status is used by tcpci.h. If I don't include tcpm.h, it will raise a warning during the compile time. > > Guenter
Re: [PATCH 1/2] usb typec: mt6360: Add support for mt6360 Type-C driver
On 8/26/20 5:59 PM, 啟原黃 wrote: [ ... ] >>> +static int mt6360_tcpc_remove(struct platform_device *pdev) >>> +{ >>> + struct mt6360_tcpc_info *mti = platform_get_drvdata(pdev); >>> + >>> + tcpci_unregister_port(mti->tcpci); >> >> That leaves interrupts enabled, which might be racy >> because interrupts are still enabled here. > M..., yes, it will cause the race condition during module remove. > I'll add disable_irq before tcpci unregister to prevent it. Or just set TCPC_ALERT_MASK to 0, as in tcpci.c. Guenter
Re: [PATCH 1/2] usb typec: mt6360: Add support for mt6360 Type-C driver
Guenter Roeck 於 2020年8月27日 週四 上午4:44寫道: > > On 8/26/20 4:16 AM, cy_huang wrote: > > From: ChiYuan Huang > > > > Mediatek MT6360 is a multi-functional IC that includes USB Type-C. > > It works with Type-C Port Controller Manager to provide USB PD > > and USB Type-C functionalities. > > > > Signed-off-by: ChiYuan Huang > > --- > > drivers/usb/typec/tcpm/Kconfig| 8 ++ > > drivers/usb/typec/tcpm/Makefile | 1 + > > drivers/usb/typec/tcpm/tcpci_mt6360.c | 212 > > ++ > > 3 files changed, 221 insertions(+) > > create mode 100644 drivers/usb/typec/tcpm/tcpci_mt6360.c > > > > diff --git a/drivers/usb/typec/tcpm/Kconfig b/drivers/usb/typec/tcpm/Kconfig > > index fa3f393..58a64e1 100644 > > --- a/drivers/usb/typec/tcpm/Kconfig > > +++ b/drivers/usb/typec/tcpm/Kconfig > > @@ -27,6 +27,14 @@ config TYPEC_RT1711H > > Type-C Port Controller Manager to provide USB PD and USB > > Type-C functionalities. > > > > +config TYPEC_MT6360 > > + tristate "Mediatek MT6360 Type-C driver" > > + depends on MFD_MT6360 > > + help > > + Mediatek MT6360 is a multi-functional IC that includes > > + USB Type-C. It works with Type-C Port Controller Manager > > + to provide USB PD and USB Type-C functionalities. > > + > > endif # TYPEC_TCPCI > > > > config TYPEC_FUSB302 > > diff --git a/drivers/usb/typec/tcpm/Makefile > > b/drivers/usb/typec/tcpm/Makefile > > index a5ff6c8..7592ccb 100644 > > --- a/drivers/usb/typec/tcpm/Makefile > > +++ b/drivers/usb/typec/tcpm/Makefile > > @@ -5,3 +5,4 @@ obj-$(CONFIG_TYPEC_WCOVE) += typec_wcove.o > > typec_wcove-y:= wcove.o > > obj-$(CONFIG_TYPEC_TCPCI)+= tcpci.o > > obj-$(CONFIG_TYPEC_RT1711H) += tcpci_rt1711h.o > > +obj-$(CONFIG_TYPEC_MT6360) += tcpci_mt6360.o > > diff --git a/drivers/usb/typec/tcpm/tcpci_mt6360.c > > b/drivers/usb/typec/tcpm/tcpci_mt6360.c > > new file mode 100644 > > index ..6a28193 > > --- /dev/null > > +++ b/drivers/usb/typec/tcpm/tcpci_mt6360.c > > @@ -0,0 +1,212 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +// > > +// Copyright (C) 2020 MediaTek Inc. > > +// > > +// Author: ChiYuan Huang > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > Is that needed ? Ah, sorry, for some testing, I forget to remove it. > > > + > > +#include "tcpci.h" > > + > > +#define MT6360_REG_VCONNCTRL10x8C > > +#define MT6360_REG_MODECTRL2 0x8F > > +#define MT6360_REG_SWRESET 0xA0 > > +#define MT6360_REG_DEBCTRL1 0xA1 > > +#define MT6360_REG_DRPCTRL1 0xA2 > > +#define MT6360_REG_DRPCTRL2 0xA3 > > +#define MT6360_REG_I2CTORST 0xBF > > +#define MT6360_REG_RXCTRL2 0xCF > > +#define MT6360_REG_CTDCTRL2 0xEC > > + > > +/* MT6360_REG_VCONNCTRL1 */ > > +#define MT6360_VCONNCL_ENABLEBIT(0) > > +/* MT6360_REG_RXCTRL2 */ > > +#define MT6360_OPEN40M_ENABLEBIT(7) > > +/* MT6360_REG_CTDCTRL2 */ > > +#define MT6360_RPONESHOT_ENABLE BIT(6) > > + > > +struct mt6360_tcpc_info { > > + struct tcpci_data tdata; > > + struct tcpci *tcpci; > > + struct device *dev; > > + int irq; > > +}; > > + > > +static inline int mt6360_tcpc_read16(struct regmap *regmap, > > + unsigned int reg, u16 *val) > > +{ > > + return regmap_raw_read(regmap, reg, val, sizeof(u16)); > > +} > > + > > +static inline int mt6360_tcpc_write16(struct regmap *regmap, > > + unsigned int reg, u16 val) > > +{ > > + return regmap_raw_write(regmap, reg, , sizeof(u16)); > > +} > > + > > +static int mt6360_tcpc_init(struct tcpci *tcpci, struct tcpci_data *tdata) > > +{ > > + struct regmap *regmap = tdata->regmap; > > + int ret; > > + > > + ret = regmap_write(regmap, MT6360_REG_SWRESET, 0x01); > > + if (ret) > > + return ret; > > + > > + /* after reset command, wait 1~2ms to wait IC action */ > > + usleep_range(1000, 2000); > > + > > + /* write all alert to masked */ > > + ret = mt6360_tcpc_write16(regmap, TCPC_ALERT_MASK, 0); > > + if (ret) > > + return ret; > > + > > + /* config I2C timeout reset enable , and timeout to 200ms */ > > + ret = regmap_write(regmap, MT6360_REG_I2CTORST, 0x8F); > > + if (ret) > > + return ret; > > + > > + /* config CC Detect Debounce : 26.7*val us */ > > + ret = regmap_write(regmap, MT6360_REG_DEBCTRL1, 0x10); > > + if (ret) > > + return ret; > > + > > + /* DRP Toggle Cycle : 51.2 + 6.4*val ms */ > > + ret = regmap_write(regmap, MT6360_REG_DRPCTRL1, 4); > > + if (ret) > > + return ret; > > + > > + /* DRP Duyt Ctrl : dcSRC: /1024 */ > > + ret = mt6360_tcpc_write16(regmap, MT6360_REG_DRPCTRL2, 330); > > + if (ret) > > + return ret; > > + > > + /* Enable VCONN Current Limit function */ > > + ret = regmap_update_bits(regmap,
Re: [PATCH 1/2] usb typec: mt6360: Add support for mt6360 Type-C driver
On 8/26/20 4:16 AM, cy_huang wrote: > From: ChiYuan Huang > > Mediatek MT6360 is a multi-functional IC that includes USB Type-C. > It works with Type-C Port Controller Manager to provide USB PD > and USB Type-C functionalities. > > Signed-off-by: ChiYuan Huang > --- > drivers/usb/typec/tcpm/Kconfig| 8 ++ > drivers/usb/typec/tcpm/Makefile | 1 + > drivers/usb/typec/tcpm/tcpci_mt6360.c | 212 > ++ > 3 files changed, 221 insertions(+) > create mode 100644 drivers/usb/typec/tcpm/tcpci_mt6360.c > > diff --git a/drivers/usb/typec/tcpm/Kconfig b/drivers/usb/typec/tcpm/Kconfig > index fa3f393..58a64e1 100644 > --- a/drivers/usb/typec/tcpm/Kconfig > +++ b/drivers/usb/typec/tcpm/Kconfig > @@ -27,6 +27,14 @@ config TYPEC_RT1711H > Type-C Port Controller Manager to provide USB PD and USB > Type-C functionalities. > > +config TYPEC_MT6360 > + tristate "Mediatek MT6360 Type-C driver" > + depends on MFD_MT6360 > + help > + Mediatek MT6360 is a multi-functional IC that includes > + USB Type-C. It works with Type-C Port Controller Manager > + to provide USB PD and USB Type-C functionalities. > + > endif # TYPEC_TCPCI > > config TYPEC_FUSB302 > diff --git a/drivers/usb/typec/tcpm/Makefile b/drivers/usb/typec/tcpm/Makefile > index a5ff6c8..7592ccb 100644 > --- a/drivers/usb/typec/tcpm/Makefile > +++ b/drivers/usb/typec/tcpm/Makefile > @@ -5,3 +5,4 @@ obj-$(CONFIG_TYPEC_WCOVE) += typec_wcove.o > typec_wcove-y:= wcove.o > obj-$(CONFIG_TYPEC_TCPCI)+= tcpci.o > obj-$(CONFIG_TYPEC_RT1711H) += tcpci_rt1711h.o > +obj-$(CONFIG_TYPEC_MT6360) += tcpci_mt6360.o > diff --git a/drivers/usb/typec/tcpm/tcpci_mt6360.c > b/drivers/usb/typec/tcpm/tcpci_mt6360.c > new file mode 100644 > index ..6a28193 > --- /dev/null > +++ b/drivers/usb/typec/tcpm/tcpci_mt6360.c > @@ -0,0 +1,212 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// > +// Copyright (C) 2020 MediaTek Inc. > +// > +// Author: ChiYuan Huang > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include Is that needed ? > + > +#include "tcpci.h" > + > +#define MT6360_REG_VCONNCTRL10x8C > +#define MT6360_REG_MODECTRL2 0x8F > +#define MT6360_REG_SWRESET 0xA0 > +#define MT6360_REG_DEBCTRL1 0xA1 > +#define MT6360_REG_DRPCTRL1 0xA2 > +#define MT6360_REG_DRPCTRL2 0xA3 > +#define MT6360_REG_I2CTORST 0xBF > +#define MT6360_REG_RXCTRL2 0xCF > +#define MT6360_REG_CTDCTRL2 0xEC > + > +/* MT6360_REG_VCONNCTRL1 */ > +#define MT6360_VCONNCL_ENABLEBIT(0) > +/* MT6360_REG_RXCTRL2 */ > +#define MT6360_OPEN40M_ENABLEBIT(7) > +/* MT6360_REG_CTDCTRL2 */ > +#define MT6360_RPONESHOT_ENABLE BIT(6) > + > +struct mt6360_tcpc_info { > + struct tcpci_data tdata; > + struct tcpci *tcpci; > + struct device *dev; > + int irq; > +}; > + > +static inline int mt6360_tcpc_read16(struct regmap *regmap, > + unsigned int reg, u16 *val) > +{ > + return regmap_raw_read(regmap, reg, val, sizeof(u16)); > +} > + > +static inline int mt6360_tcpc_write16(struct regmap *regmap, > + unsigned int reg, u16 val) > +{ > + return regmap_raw_write(regmap, reg, , sizeof(u16)); > +} > + > +static int mt6360_tcpc_init(struct tcpci *tcpci, struct tcpci_data *tdata) > +{ > + struct regmap *regmap = tdata->regmap; > + int ret; > + > + ret = regmap_write(regmap, MT6360_REG_SWRESET, 0x01); > + if (ret) > + return ret; > + > + /* after reset command, wait 1~2ms to wait IC action */ > + usleep_range(1000, 2000); > + > + /* write all alert to masked */ > + ret = mt6360_tcpc_write16(regmap, TCPC_ALERT_MASK, 0); > + if (ret) > + return ret; > + > + /* config I2C timeout reset enable , and timeout to 200ms */ > + ret = regmap_write(regmap, MT6360_REG_I2CTORST, 0x8F); > + if (ret) > + return ret; > + > + /* config CC Detect Debounce : 26.7*val us */ > + ret = regmap_write(regmap, MT6360_REG_DEBCTRL1, 0x10); > + if (ret) > + return ret; > + > + /* DRP Toggle Cycle : 51.2 + 6.4*val ms */ > + ret = regmap_write(regmap, MT6360_REG_DRPCTRL1, 4); > + if (ret) > + return ret; > + > + /* DRP Duyt Ctrl : dcSRC: /1024 */ > + ret = mt6360_tcpc_write16(regmap, MT6360_REG_DRPCTRL2, 330); > + if (ret) > + return ret; > + > + /* Enable VCONN Current Limit function */ > + ret = regmap_update_bits(regmap, MT6360_REG_VCONNCTRL1, > MT6360_VCONNCL_ENABLE, > + MT6360_VCONNCL_ENABLE); > + if (ret) > + return ret; > + > + /* Enable cc open 40ms when pmic send vsysuv signal */ > + ret = regmap_update_bits(regmap, MT6360_REG_RXCTRL2, > MT6360_OPEN40M_ENABLE, > + MT6360_OPEN40M_ENABLE); > + if (ret) > +