Chunfeng Yun <chunfeng.yun@xxxxxxxxxxxx> 於 2021年1月19日 週二 下午3:33寫道: > > On Sun, 2021-01-17 at 09:43 -0800, Guenter Roeck wrote: > > On 1/15/21 6:13 AM, cy_huang wrote: > > > From: ChiYuan Huang <cy_huang@xxxxxxxxxxx> > > > > > > MT6360 not support for TCPC command to control source and sink. > > > > does not > > > > > Uses external 5V vbus regulator as the vbus source control. > > > > > Use > > > > > Also adds the capability to report vsafe0v. > > > > > add > > > > So far this driver works without regulator. Unless I am missing something, > > this patch makes regulator support mandatory, meaning existing code will fail. > If don't provide vbus-supply in DTS, regulator framework will provide a > dummy regulator, so the code will not fail. In the last reply, I will change from regulator_get to regulator_get_exclusive, it will return -ENODEV. The IS_ERR can catch this situation, no dummy regulator will be returned. And assume no vbus 5v for source & snk attached, It will cause typec state machine repeated from drp -> src_attach_wait -> src_attached -> PD_T_PS_SOURCE_on timeout. It will be stuck in the loop until snk detached. > > I am not sure if that is appropriate/acceptable. Can we be sure that this will > > work for existing users of this driver ? > > > > > Thanks, > > Guenter > > > > > Signed-off-by: ChiYuan Huang <cy_huang@xxxxxxxxxxx> > > > --- > > > drivers/usb/typec/tcpm/tcpci_mt6360.c | 29 +++++++++++++++++++++++++++++ > > > 1 file changed, 29 insertions(+) > > > > > > diff --git a/drivers/usb/typec/tcpm/tcpci_mt6360.c b/drivers/usb/typec/tcpm/tcpci_mt6360.c > > > index f1bd9e0..0edf4b6 100644 > > > --- a/drivers/usb/typec/tcpm/tcpci_mt6360.c > > > +++ b/drivers/usb/typec/tcpm/tcpci_mt6360.c > > > @@ -11,6 +11,7 @@ > > > #include <linux/of.h> > > > #include <linux/platform_device.h> > > > #include <linux/regmap.h> > > > +#include <linux/regulator/consumer.h> > > > #include <linux/usb/tcpm.h> > > > > > > #include "tcpci.h" > > > @@ -36,6 +37,7 @@ struct mt6360_tcpc_info { > > > struct tcpci_data tdata; > > > struct tcpci *tcpci; > > > struct device *dev; > > > + struct regulator *vbus; > > > int irq; > > > }; > > > > > > @@ -51,6 +53,27 @@ static inline int mt6360_tcpc_write16(struct regmap *regmap, > > > return regmap_raw_write(regmap, reg, &val, sizeof(u16)); > > > } > > > > > > +static int mt6360_tcpc_set_vbus(struct tcpci *tcpci, struct tcpci_data *data, bool src, bool snk) > > > +{ > > > + struct mt6360_tcpc_info *mti = container_of(data, struct mt6360_tcpc_info, tdata); > > > + int ret; > > > + > > > + /* To correctly handle the already enabled vbus and disable its supply first */ > > > + if (regulator_is_enabled(mti->vbus)) { > > > + ret = regulator_disable(mti->vbus); > > > + if (ret) > > > + return ret; > > > + } > > > > Is it really a good idea to disable vbus if it happens to be already enabled > > and there is (another ?) request to enable it ? > > > > > + > > > + if (src) { > > > + ret = regulator_enable(mti->vbus); > > > + if (ret) > > > + return ret; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > static int mt6360_tcpc_init(struct tcpci *tcpci, struct tcpci_data *tdata) > > > { > > > struct regmap *regmap = tdata->regmap; > > > @@ -138,7 +161,13 @@ static int mt6360_tcpc_probe(struct platform_device *pdev) > > > if (mti->irq < 0) > > > return mti->irq; > > > > > > + mti->vbus = devm_regulator_get(&pdev->dev, "vbus"); > > > + if (IS_ERR(mti->vbus)) > > > + return PTR_ERR(mti->vbus); > > > + > > > mti->tdata.init = mt6360_tcpc_init; > > > + mti->tdata.set_vbus = mt6360_tcpc_set_vbus; > > > + mti->tdata.vbus_vsafe0v = 1; > > > mti->tcpci = tcpci_register_port(&pdev->dev, &mti->tdata); > > > if (IS_ERR(mti->tcpci)) { > > > dev_err(&pdev->dev, "Failed to register tcpci port\n"); > > > > > >