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. > 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"); > > >