Re: [PATCH 1/2] usb typec: mt6360: Add support for mt6360 Type-C driver

2020-08-26 Thread Guenter Roeck
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

2020-08-26 Thread 啟原黃
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

2020-08-26 Thread Guenter Roeck
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

2020-08-26 Thread 啟原黃
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

2020-08-26 Thread Guenter Roeck
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)
> +