Chunfeng Yun <chunfeng.yun@xxxxxxxxxxxx> 於 2021年1月19日 週二 下午3:38寫道: > > On Mon, 2021-01-18 at 16:28 +0800, ChiYuan Huang wrote: > > Guenter Roeck <linux@xxxxxxxxxxxx> 於 2021年1月18日 週一 上午1:43寫道: > > > > > > 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 > > > > > Ack > > > > Uses external 5V vbus regulator as the vbus source control. > > > > > > > Use > > > > > Ack > > > > Also adds the capability to report vsafe0v. > > > > > > > add > > > > > Ack > > > So far this driver works without regulator. Unless I am missing something, > > > this patch makes regulator support mandatory, meaning existing code will fail. > > > I am not sure if that is appropriate/acceptable. Can we be sure that this will > > > work for existing users of this driver ? > > > > > Yes, I already checked all the src/snk functionality based on the > > latest typec code. > > It'll be common for our TCPC. It didn't support for TCPC command. > > From the recent patches, actually, I have the local change to test the > > src capability. > > But I didn't submit it. It's almost the same to add set_vbus callback. > > That's why I submit this change after tcpci 'set_vbus callback' is added. > > > > > 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 ? > > > > > Yes, for the state change from src_attach_wait to src_attach, > > It need to meet the requirement that the vbus is at vsafe0v. > > So to disable it first is needed. > > And to prevent other users from enabling/disabling external vbus > > regulator in any case. > > I think we may change regulator_get to 'regulator_get_exclusive'. > > From the design, 5v regulator only can be controlled via typec framework. > > If other user touch it, it'll affect the typec state transition. > How about to process the case that even switch usb controller to device > mode, platform also need to keep vbus on? e.g. Iphone Carplay > > It must be processed by USBPD data role swap. Type C only decide the initial role (SNK: power snk and ufp; SRC: power src and DFP). Only USBPD can change the power/data/vconn role individually. > > > > + > > > > + 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"); > > > > > > > >