Messages in this thread | | | From | Andy Shevchenko <> | Date | Thu, 23 Jun 2022 20:07:17 +0200 | Subject | Re: [PATCH v3 08/14] usb: typec: tcpci_mt6370: Add Mediatek MT6370 tcpci driver |
| |
On Thu, Jun 23, 2022 at 2:00 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote: > > From: ChiYuan Huang <cy_huang@richtek.com> > > 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
| |