On Thu, Jun 23, 2022 at 2:00 PM ChiaEn Wu <peterwu.pub@xxxxxxxxx> wrote: > > From: ChiYuan Huang <cy_huang@xxxxxxxxxxx> > > Add chip level mt6370 tcpci driver. ... > +config TYPEC_TCPCI_MT6370 > + tristate "Mediatek MT6370 Type-C driver" > + depends on MFD_MT6370 > + help > + Mediatek MT6370 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. What will be the module name? ... > +#include <linux/bits.h> > +#include <linux/interrupt.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> No user of this header is found in this file. > +#include <linux/platform_device.h> > +#include <linux/pm_wakeup.h> > +#include <linux/pm_wakeirq.h> > +#include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > +#include <linux/usb/tcpm.h> > +#include "tcpci.h" ... > + if (did == MT6370_TCPC_DID_A) { > + ret = regmap_write(data->regmap, TCPC_FAULT_CTRL, 0x80); > + if (ret) > + return ret; return regmap_write(...); > + } > + > + return 0; ... > + if (ret && !source) > + ret = regulator_disable(priv->vbus); > + else if (!ret && source) > + ret = regulator_enable(priv->vbus); > + else > + ret = 0; > + > + return ret; Can it be if (ret && ...) return regulator_disable(...); if (!ret && ...) return regulator_enable(...); return 0; ? ... > + if (!priv->tcpci_data.regmap) { > + dev_err(&pdev->dev, "Failed to init regmap\n"); > + return -ENODEV; > + } return dev_err_probe(...); ? ... > + if (ret) { > + dev_err(&pdev->dev, "Failed to check vendor info (%d)\n", ret); > + return ret; > + } Ditto. ... > + priv->irq = platform_get_irq(pdev, 0); > + if (priv->irq < 0) { > + dev_err(&pdev->dev, "Failed to get TCPC irq (%d)\n", priv->irq); The message like this is printed in case of error inside platform_get_irq(), no need to duplicate. > + return priv->irq; > + } ... > + priv->tcpci = tcpci_register_port(&pdev->dev, &priv->tcpci_data); > + if (IS_ERR(priv->tcpci)) { > + dev_err(&pdev->dev, "Failed to register tcpci port\n"); > + return PTR_ERR(priv->tcpci); return dev_err_probe(); ? > + } ... > + if (ret) { > + dev_err(&pdev->dev, "Failed to allocate irq (%d)\n", ret); > + tcpci_unregister_port(priv->tcpci); > + return ret; Ditto. > + } -- With Best Regards, Andy Shevchenko