Hi Cristian,

On Tue, Mar 30, 2021 at 11:54:42PM +0300, cristian.bir...@microchip.com wrote:
> From: Cristian Birsan <cristian.bir...@microchip.com>
> 
> This patch adds initial driver support for the new Microchip USB
> Type-C Port Controller (TCPC) embedded in sama7g5 SoC.
> 
> Signed-off-by: Cristian Birsan <cristian.bir...@microchip.com>
> ---
>  drivers/usb/typec/tcpm/Kconfig        |   8 +
>  drivers/usb/typec/tcpm/Makefile       |   1 +
>  drivers/usb/typec/tcpm/sama7g5_tcpc.c | 610 ++++++++++++++++++++++++++
>  3 files changed, 619 insertions(+)
>  create mode 100644 drivers/usb/typec/tcpm/sama7g5_tcpc.c
> 
> diff --git a/drivers/usb/typec/tcpm/Kconfig b/drivers/usb/typec/tcpm/Kconfig
> index 557f392fe24d..8ba0fd85741f 100644
> --- a/drivers/usb/typec/tcpm/Kconfig
> +++ b/drivers/usb/typec/tcpm/Kconfig
> @@ -52,6 +52,14 @@ config TYPEC_FUSB302
>         Type-C Port Controller Manager to provide USB PD and USB
>         Type-C functionalities.
>  
> +config TYPEC_SAMA7G5
> +     tristate "Microchip SAMA7G5 Type-C Port Controller driver"
> +     select REGMAP_MMIO
> +     help
> +       Say Y or M here if your system has SAMA7G5 TCPC controller.
> +       It works with Type-C Port Controller Manager to provide USB
> +       Type-C functionalities.
> +
>  config TYPEC_WCOVE
>       tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver"
>       depends on ACPI
> diff --git a/drivers/usb/typec/tcpm/Makefile b/drivers/usb/typec/tcpm/Makefile
> index 7d499f3569fd..9abe8a7ae1cc 100644
> --- a/drivers/usb/typec/tcpm/Makefile
> +++ b/drivers/usb/typec/tcpm/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_TYPEC_TCPM)             += tcpm.o
>  obj-$(CONFIG_TYPEC_FUSB302)          += fusb302.o
> +obj-$(CONFIG_TYPEC_SAMA7G5)          += sama7g5_tcpc.o
>  obj-$(CONFIG_TYPEC_WCOVE)            += typec_wcove.o
>  typec_wcove-y                                := wcove.o
>  obj-$(CONFIG_TYPEC_TCPCI)            += tcpci.o
> diff --git a/drivers/usb/typec/tcpm/sama7g5_tcpc.c 
> b/drivers/usb/typec/tcpm/sama7g5_tcpc.c
> new file mode 100644
> index 000000000000..2986c0fcc8a3
> --- /dev/null
> +++ b/drivers/usb/typec/tcpm/sama7g5_tcpc.c
> @@ -0,0 +1,610 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Microchip SAMA7G5 Type-C Port Controller Driver
> + *
> + * Copyright (C) 2021 Microchip Technology, Inc. and its subsidiaries
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/usb/pd.h>
> +#include <linux/usb/tcpm.h>
> +#include <linux/usb/typec.h>
> +
> +#define SAMA7G5_TCPC_GCLK                            32000
> +
> +/* TCPC registers offsets */
> +#define TCPC_CR                      0x80            /* TCPC Control 
> Register */
> +#define TCPC_UPC             0xA0            /* TCPC PHY Control Register */
> +#define TCPC_UPS             0xA4            /* TCPC PHY Status Register */
> +
> +#define TCPC_CR_RESET                0x54434301      /* Magic value */
> +
> +/* TCPC PHY Control Register */
> +#define TCPC_UPC_BCDETE              BIT(29)
> +#define TCPC_UPC_BCVSRCE     BIT(28)
> +#define      TCPC_UPC_BCDETSEL       BIT(27)

Why do you have a tab right after "#define" above?

> +#define      TCPC_UPC_BCIDPSRCE      BIT(26)

And here?

> +#define TCPC_UPC_DMPDFE              BIT(25)
> +#define TCPC_UPC_DMPDFD              BIT(24)
> +#define TCPC_UPC_IP_OFF              (0 << 12)
> +#define TCPC_UPC_IP_0P5              (1 << 12)
> +#define TCPC_UPC_IP_1P5              (2 << 12)
> +#define TCPC_UPC_IP_3P0              (3 << 12)
> +#define TCPC_UPC_THRESHOLD0  (0 << 8)
> +#define TCPC_UPC_THRESHOLD2  (2 << 8)
> +#define TCPC_UPC_THRESHOLD4  (4 << 8)
> +#define TCPC_UPC_THRESHOLD6  (6 << 8)
> +
> +/* TCPC PHY  Status Register */
> +#define TCPC_UPS_CC2RDT              BIT(4)
> +#define TCPC_UPS_CC1ID               BIT(3)
> +#define TCPC_UPS_CC_MASK     GENMASK(4, 3)
> +#define TCPC_UPS_CHGDCP              BIT(2)
> +#define TCPC_UPS_DM          BIT(1)
> +#define TCPC_UPS_DP          BIT(0)
> +
> +#define TCPC_VERSION         0xFC
> +
> +/* USB Type-C measurement timings */
> +#define T_CC_MEASURE         100 /* 100 ms */
> +
> +#define SAMA7G5_TCPC_VBUS_IRQFLAGS (IRQF_ONESHOT \
> +                        | IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING)
> +
> +struct sama7g5_tcpc {
> +     struct device *dev;
> +
> +     struct workqueue_struct *wq;
> +     struct delayed_work measure_work;
> +
> +     struct regmap *regmap;
> +     void __iomem *base;
> +
> +     struct clk *pclk;
> +     struct clk *gclk;
> +
> +     struct gpio_desc *vbus_pin;
> +     struct regulator *vbus;
> +
> +     /* lock for sharing states */
> +     struct mutex lock;
> +
> +     /* port status */
> +     enum typec_cc_polarity cc_polarity;
> +     enum typec_cc_status cc1_status;
> +     enum typec_cc_status cc2_status;
> +     enum typec_cc_status cc1_status_prev;
> +     enum typec_cc_status cc2_status_prev;
> +
> +     /* mutex used for VBUS detection */
> +     struct mutex vbus_mutex;
> +     int vbus_present;
> +     int vbus_present_prev;
> +
> +     unsigned int phy_status;
> +     unsigned int phy_status_old;
> +
> +     struct tcpc_dev tcpc;
> +     struct tcpm_port *tcpm;
> +};
> +
> +#define tcpc_to_sama7g5_tcpc(_tcpc_) \
> +             container_of(_tcpc_, struct sama7g5_tcpc, tcpc)
> +
> +static bool sama7g5_tcpc_readable_reg(struct device *dev, unsigned int reg)
> +{
> +     switch (reg) {
> +     case TCPC_CR:
> +     case TCPC_UPC:
> +     case TCPC_UPS:
> +             return true;
> +     default:
> +             return false;
> +     }
> +}
> +
> +static bool sama7g5_tcpc_writeable_reg(struct device *dev, unsigned int reg)
> +{
> +     switch (reg) {
> +     case TCPC_CR:
> +     case TCPC_UPC:
> +     case TCPC_UPS:
> +             return true;
> +     default:
> +             return false;
> +     }
> +}
> +
> +static const struct regmap_config sama7g5_tcpc_regmap_config = {
> +     .reg_bits = 32,
> +     .reg_stride = 4,
> +     .val_bits = 32,
> +     .max_register = TCPC_VERSION,
> +     .readable_reg = sama7g5_tcpc_readable_reg,
> +     .writeable_reg = sama7g5_tcpc_writeable_reg,
> +};
> +
> +static int sama7g5_tcpc_get_vbus(struct tcpc_dev *tcpc)
> +{
> +     struct sama7g5_tcpc *sama7g5_tcpc = tcpc_to_sama7g5_tcpc(tcpc);
> +     int ret;
> +
> +     mutex_lock(&sama7g5_tcpc->vbus_mutex);
> +     ret = sama7g5_tcpc->vbus_present ? 1 : 0;
> +     mutex_unlock(&sama7g5_tcpc->vbus_mutex);
> +
> +     return ret;
> +}
> +
> +static int sama7g5_tcpc_set_vbus(struct tcpc_dev *tcpc, bool on, bool sink)
> +{
> +     struct sama7g5_tcpc *sama7g5_tcpc = tcpc_to_sama7g5_tcpc(tcpc);
> +     int ret;
> +
> +     mutex_lock(&sama7g5_tcpc->vbus_mutex);
> +     if (on)
> +             ret = regulator_enable(sama7g5_tcpc->vbus);
> +     else
> +             ret = regulator_disable(sama7g5_tcpc->vbus);
> +     mutex_unlock(&sama7g5_tcpc->vbus_mutex);
> +
> +     return ret;
> +}
> +
> +static int sama7g5_tcpc_set_vconn(struct tcpc_dev *tcpc, bool on)
> +{
> +     /* VCONN is not supported */
> +     return -EPERM;
> +}
> +
> +static int sama7g5_tcpc_get_cc(struct tcpc_dev *tcpc, enum typec_cc_status 
> *cc1,
> +                            enum typec_cc_status *cc2)
> +{
> +     struct sama7g5_tcpc *sama7g5_tcpc = tcpc_to_sama7g5_tcpc(tcpc);
> +
> +     mutex_lock(&sama7g5_tcpc->lock);
> +     *cc1 = sama7g5_tcpc->cc1_status;
> +     *cc2 = sama7g5_tcpc->cc2_status;
> +     mutex_unlock(&sama7g5_tcpc->lock);
> +
> +     return 0;
> +}
> +
> +static int sama7g5_tcpc_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status 
> cc)
> +{
> +     struct sama7g5_tcpc *sama7g5_tcpc = tcpc_to_sama7g5_tcpc(tcpc);
> +     unsigned int ctrl;
> +     int ret = 0;
> +
> +     mutex_lock(&sama7g5_tcpc->lock);
> +     switch (cc) {
> +     case TYPEC_CC_RD:
> +             ctrl = TCPC_UPC_IP_OFF;
> +             break;
> +     case TYPEC_CC_RP_DEF:
> +             ctrl = TCPC_UPC_IP_0P5;
> +             break;
> +     default:
> +             ret =  -EINVAL;
> +             goto done;
> +     }
> +     ret = regmap_write(sama7g5_tcpc->regmap, TCPC_UPC, ctrl);
> +done:
> +     mutex_unlock(&sama7g5_tcpc->lock);
> +     return ret;
> +}
> +
> +static int sama7g5_tcpc_set_polarity(struct tcpc_dev *tcpc,
> +                                  enum typec_cc_polarity pol)
> +{
> +     return 0;
> +}
> +
> +static int sama7g5_tcpc_set_roles(struct tcpc_dev *tcpc, bool attached,
> +                        enum typec_role role, enum typec_data_role data)
> +{
> +     return 0;
> +}
> +
> +static int sama7g5_tcpc_set_pd_rx(struct tcpc_dev *tcpc, bool on)
> +{
> +     return -EPERM;
> +}
> +
> +static int sama7g5_tcpc_pd_transmit(struct tcpc_dev *tcpc,
> +                                 enum tcpm_transmit_type type,
> +                                 const struct pd_message *msg,
> +                                 unsigned int negotiated_rev)
> +{
> +     return -EPERM;
> +}
> +
> +static int sama7g5_tcpc_start_toggling(struct tcpc_dev *tcpc,
> +                                    enum typec_port_type port_type,
> +                                    enum typec_cc_status cc)
> +{
> +     return -EOPNOTSUPP;
> +}
> +
> +static void _sama7g5_tcpc_measure_snk(struct sama7g5_tcpc *sama7g5_tcpc)
> +{
> +     struct device *dev = sama7g5_tcpc->dev;
> +     int ret;
> +
> +     /* Save previous CC1/CC2 state */
> +     sama7g5_tcpc->cc1_status_prev = sama7g5_tcpc->cc1_status;
> +     sama7g5_tcpc->cc2_status_prev = sama7g5_tcpc->cc2_status;
> +
> +     /* Comparator Threshold 2 */
> +     ret = regmap_write(sama7g5_tcpc->regmap, TCPC_UPC, TCPC_UPC_IP_OFF |
> +                        TCPC_UPC_THRESHOLD2);

How about:

        ret = regmap_write(sama7g5_tcpc->regmap, TCPC_UPC,
                           TCPC_UPC_IP_OFF | TCPC_UPC_THRESHOLD2);

just to make it a bit easier to read?

> +     if (ret) {
> +             dev_err(dev, "failed to wite register: %d\n", ret);
> +             return;
> +     }
> +
> +     usleep_range(560, 1000);
> +
> +     ret = regmap_read(sama7g5_tcpc->regmap, TCPC_UPS,
> +                       &sama7g5_tcpc->phy_status);
> +     if (ret) {
> +             dev_err(dev, "failed to read register: %d\n", ret);
> +             return;
> +     }
> +
> +     if (!(sama7g5_tcpc->phy_status & TCPC_UPS_CC_MASK)) {
> +             /* VRa*/
> +             sama7g5_tcpc->cc1_status = TYPEC_CC_OPEN;
> +             sama7g5_tcpc->cc2_status = TYPEC_CC_OPEN;
> +             return;
> +     }
> +
> +     /* CC1 or CC2 is connected wait for PD messages to end ~ 30ms */
> +     usleep_range(30000, 35000);
> +
> +     /* Comparator Threshold 4 */
> +     sama7g5_tcpc->phy_status_old = sama7g5_tcpc->phy_status;
> +
> +     ret = regmap_write(sama7g5_tcpc->regmap, TCPC_UPC, TCPC_UPC_IP_OFF |
> +                        TCPC_UPC_THRESHOLD4);

And here:

        ret = regmap_write(sama7g5_tcpc->regmap, TCPC_UPC,
                           TCPC_UPC_IP_OFF | TCPC_UPC_THRESHOLD4);

> +     if (ret) {
> +             dev_err(dev, "failed to wite register: %d\n", ret);
> +             return;
> +     }
> +
> +     usleep_range(560, 1000);
> +     ret = regmap_read(sama7g5_tcpc->regmap, TCPC_UPS,
> +                       &sama7g5_tcpc->phy_status);
> +     if (ret) {
> +             dev_err(dev, "failed to read register: %d\n", ret);
> +             return;
> +     }
> +
> +     if ((sama7g5_tcpc->phy_status_old & TCPC_UPS_CC1ID) &&
> +         (!(sama7g5_tcpc->phy_status & TCPC_UPS_CC1ID))) {
> +             sama7g5_tcpc->cc1_status = TYPEC_CC_RP_DEF;
> +             sama7g5_tcpc->cc2_status = TYPEC_CC_OPEN;
> +             return;
> +     }
> +
> +     if ((sama7g5_tcpc->phy_status_old & TCPC_UPS_CC2RDT) &&
> +         (!(sama7g5_tcpc->phy_status & TCPC_UPS_CC2RDT))) {
> +             sama7g5_tcpc->cc1_status = TYPEC_CC_OPEN;
> +             sama7g5_tcpc->cc2_status = TYPEC_CC_RP_DEF;
> +             return;
> +     }
> +
> +     /* Comparator Threshold 6 */
> +     sama7g5_tcpc->phy_status_old = sama7g5_tcpc->phy_status;
> +
> +     ret = regmap_write(sama7g5_tcpc->regmap, TCPC_UPC, TCPC_UPC_IP_OFF |
> +                        TCPC_UPC_THRESHOLD6);

Ditto?

> +     if (ret) {
> +             dev_err(dev, "failed to wite register: %d\n", ret);
> +             return;
> +     }
> +
> +     usleep_range(560, 1000);
> +     ret = regmap_read(sama7g5_tcpc->regmap, TCPC_UPS,
> +                       &sama7g5_tcpc->phy_status);
> +     if (ret) {
> +             dev_err(dev, "failed to read register: %d\n", ret);
> +             return;
> +     }
> +
> +     if ((sama7g5_tcpc->phy_status_old & TCPC_UPS_CC1ID) &&
> +         (!(sama7g5_tcpc->phy_status & TCPC_UPS_CC1ID))) {
> +             sama7g5_tcpc->cc1_status = TYPEC_CC_RP_1_5;
> +             sama7g5_tcpc->cc2_status = TYPEC_CC_OPEN;
> +             return;
> +     }
> +
> +     if ((sama7g5_tcpc->phy_status_old & TCPC_UPS_CC1ID) &&
> +         ((sama7g5_tcpc->phy_status & TCPC_UPS_CC1ID))) {
> +             sama7g5_tcpc->cc1_status = TYPEC_CC_RP_3_0;
> +             sama7g5_tcpc->cc2_status = TYPEC_CC_OPEN;
> +             return;
> +     }
> +
> +     if ((sama7g5_tcpc->phy_status_old & TCPC_UPS_CC2RDT) &&
> +         (!(sama7g5_tcpc->phy_status & TCPC_UPS_CC2RDT))) {
> +             sama7g5_tcpc->cc1_status = TYPEC_CC_OPEN;
> +             sama7g5_tcpc->cc2_status = TYPEC_CC_RP_1_5;
> +             return;
> +     }
> +
> +     if ((sama7g5_tcpc->phy_status_old & TCPC_UPS_CC2RDT) &&
> +         ((sama7g5_tcpc->phy_status & TCPC_UPS_CC2RDT))) {
> +             sama7g5_tcpc->cc1_status = TYPEC_CC_OPEN;
> +             sama7g5_tcpc->cc2_status = TYPEC_CC_RP_3_0;
> +             return;
> +     }

It looks like you only use that phy_status_old member in this
function, so you could make it a local variable, no?

> +}
> +
> +static void sama7g5_tcpc_measure_work(struct work_struct *work)
> +{
> +     struct sama7g5_tcpc *port = container_of(work, struct sama7g5_tcpc,
> +                                              measure_work.work);
> +
> +     mutex_lock(&port->lock);
> +
> +     _sama7g5_tcpc_measure_snk(port);
> +
> +     /* Check if the state has changed and notify TCPM */
> +     if (port->cc1_status != port->cc1_status_prev ||
> +         port->cc2_status != port->cc2_status_prev)
> +             tcpm_cc_change(port->tcpm);

And those ccx_status_prev you are only using here, so you probable
don't need those members either.

> +     mod_delayed_work(port->wq, &port->measure_work,
> +                      msecs_to_jiffies(T_CC_MEASURE));
> +
> +     mutex_unlock(&port->lock);
> +}
> +
> +static int sama7g5_tcpc_init(struct tcpc_dev *tcpc)
> +{
> +     struct sama7g5_tcpc *sama7g5_tcpc = tcpc_to_sama7g5_tcpc(tcpc);
> +     int ret;
> +
> +     ret = regmap_write(sama7g5_tcpc->regmap, TCPC_CR, TCPC_CR_RESET);
> +     if (ret)
> +             return ret;
> +
> +     sama7g5_tcpc->wq =
> +             create_singlethread_workqueue(dev_name(sama7g5_tcpc->dev));

One line is enough for the above.

> +     if (!sama7g5_tcpc->wq)
> +             return -ENOMEM;
> +
> +     INIT_DELAYED_WORK(&sama7g5_tcpc->measure_work,
> +                       sama7g5_tcpc_measure_work);
> +
> +     sama7g5_tcpc->cc1_status = TYPEC_CC_OPEN;
> +     sama7g5_tcpc->cc2_status = TYPEC_CC_OPEN;
> +     sama7g5_tcpc->cc1_status_prev = TYPEC_CC_OPEN;
> +     sama7g5_tcpc->cc2_status_prev = TYPEC_CC_OPEN;
> +     sama7g5_tcpc->cc_polarity = TYPEC_POLARITY_CC1;
> +
> +     /* We do not have an interrupt so polling only */
> +     mod_delayed_work(sama7g5_tcpc->wq, &sama7g5_tcpc->measure_work,
> +                      msecs_to_jiffies(T_CC_MEASURE));
> +
> +     /* Enable VBUS detection */
> +     if (sama7g5_tcpc->vbus_pin)
> +             enable_irq(gpiod_to_irq(sama7g5_tcpc->vbus_pin));
> +
> +     return 0;
> +}
> +
> +static int vbus_is_present(struct sama7g5_tcpc *sama7g5_tcpc)
> +{
> +     if (sama7g5_tcpc->vbus_pin)
> +             return gpiod_get_value(sama7g5_tcpc->vbus_pin);
> +
> +     /* No Vbus detection: Assume always present */
> +     return 1;
> +}
> +
> +static irqreturn_t sama7g5_vbus_irq_thread(int irq, void *devid)
> +{
> +     struct sama7g5_tcpc *sama7g5_tcpc = devid;
> +
> +     /* debounce */
> +     udelay(10);
> +
> +     mutex_lock(&sama7g5_tcpc->vbus_mutex);
> +
> +     sama7g5_tcpc->vbus_present = vbus_is_present(sama7g5_tcpc);
> +     if (sama7g5_tcpc->vbus_present != sama7g5_tcpc->vbus_present_prev) {
> +             /* VBUS changed, notify TCPM */
> +             tcpm_vbus_change(sama7g5_tcpc->tcpm);
> +             sama7g5_tcpc->vbus_present_prev = sama7g5_tcpc->vbus_present;
> +     }

The vbus_present_prev looks like it could also be a local variable.

> +     mutex_unlock(&sama7g5_tcpc->vbus_mutex);
> +     return IRQ_HANDLED;
> +}
> +
> +static int sama7g5_tcpc_probe(struct platform_device *pdev)
> +{
> +     int ret;
> +     struct sama7g5_tcpc *sama7g5_tcpc;
> +
> +     struct resource *mem;
> +     void __iomem *base;
> +
> +     sama7g5_tcpc = devm_kzalloc(&pdev->dev, sizeof(*sama7g5_tcpc),
> +                                 GFP_KERNEL);
> +     if (!sama7g5_tcpc)
> +             return -ENOMEM;
> +
> +     mem =  platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +     base = devm_ioremap_resource(&pdev->dev, mem);
> +     if (IS_ERR(base))
> +             return PTR_ERR(base);
> +     sama7g5_tcpc->base = base;

        sama7g5_tcpc->base = devm_platform_ioremap_resourse(pdev, 0);
        if (IS_ERR(sama7g5_tcpc->base))
                return PTR_ERR(sama7g5_tcpc->base);

> +     sama7g5_tcpc->regmap =  devm_regmap_init_mmio(&pdev->dev, base,
> +                                             &sama7g5_tcpc_regmap_config);

You can align that properly too:

        sama7g5_tcpc->regmap =  devm_regmap_init_mmio(&pdev->dev, 
sama7g5_tcpc->base,
                                                      
&sama7g5_tcpc_regmap_config);

> +     if (IS_ERR(sama7g5_tcpc->regmap)) {
> +             dev_err(&pdev->dev, "Regmap init failed\n");
> +             return PTR_ERR(sama7g5_tcpc->regmap);
> +     }
> +
> +     /* Get the peripheral clock */
> +     sama7g5_tcpc->pclk = devm_clk_get(&pdev->dev, "pclk");
> +     if (IS_ERR(sama7g5_tcpc->pclk)) {
> +             ret = PTR_ERR(sama7g5_tcpc->pclk);
> +             dev_err(&pdev->dev,
> +                     "failed to get the peripheral clock: %d\n", ret);
> +             return ret;
> +     }
> +
> +     ret = clk_prepare_enable(sama7g5_tcpc->pclk);
> +     if (ret) {
> +             dev_err(&pdev->dev,
> +                     "failed to enable the peripheral clock: %d\n", ret);
> +             return ret;
> +     }
> +
> +     /* Get the generic clock */
> +     sama7g5_tcpc->gclk = devm_clk_get(&pdev->dev, "gclk");
> +     if (IS_ERR(sama7g5_tcpc->gclk)) {
> +             ret = PTR_ERR(sama7g5_tcpc->gclk);
> +             dev_err(&pdev->dev,
> +                     "failed to get the PMC generic clock: %d\n", ret);
> +             return ret;
> +     }
> +
> +     ret = clk_set_rate(sama7g5_tcpc->gclk, SAMA7G5_TCPC_GCLK);
> +     if (ret) {
> +             dev_err(&pdev->dev,
> +                     "unable to change gclk rate to: %u\n",
> +                     SAMA7G5_TCPC_GCLK);
> +             return ret;
> +     }
> +
> +     ret = clk_prepare_enable(sama7g5_tcpc->gclk);
> +     if (ret) {
> +             dev_err(&pdev->dev,
> +                     "failed to enable the generic clock: %d\n", ret);
> +             return ret;
> +     }
> +
> +     mutex_init(&sama7g5_tcpc->lock);
> +     mutex_init(&sama7g5_tcpc->vbus_mutex);
> +
> +     sama7g5_tcpc->vbus_pin = devm_gpiod_get_optional(&pdev->dev,
> +                             "microchip,vbus", GPIOD_IN);

You can align that properly.

> +     if (IS_ERR(sama7g5_tcpc->vbus_pin)) {
> +             ret = PTR_ERR(sama7g5_tcpc->vbus_pin);
> +             dev_err(&pdev->dev, "unable to claim vbus-gpio: %d\n", ret);
> +     }
> +
> +     sama7g5_tcpc->vbus = devm_regulator_get_optional(&pdev->dev, "vbus");
> +
> +     if (IS_ERR(sama7g5_tcpc->vbus)) {
> +             ret = PTR_ERR(sama7g5_tcpc->vbus);
> +             dev_err(&pdev->dev, "unable to claim vbus-supply: %d\n", ret);
> +     }
> +
> +     if (sama7g5_tcpc->vbus_pin) {
> +             irq_set_status_flags(gpiod_to_irq(sama7g5_tcpc->vbus_pin),
> +                                  IRQ_NOAUTOEN);
> +             ret = devm_request_threaded_irq(&pdev->dev,
> +                             gpiod_to_irq(sama7g5_tcpc->vbus_pin), NULL,
> +                             sama7g5_vbus_irq_thread,
> +                             SAMA7G5_TCPC_VBUS_IRQFLAGS,
> +                             "sama7g5_tcpc", sama7g5_tcpc);
> +             if (ret) {
> +                     sama7g5_tcpc->vbus_pin = NULL;
> +                     dev_warn(&pdev->dev,
> +                              "failed to request vbus irq; "
> +                              "assuming always on\n");
> +             }
> +     }
> +
> +     sama7g5_tcpc->dev = &pdev->dev;
> +     platform_set_drvdata(pdev, sama7g5_tcpc);
> +
> +     sama7g5_tcpc->tcpc.init = sama7g5_tcpc_init;
> +     sama7g5_tcpc->tcpc.get_vbus = sama7g5_tcpc_get_vbus;
> +     sama7g5_tcpc->tcpc.set_vbus = sama7g5_tcpc_set_vbus;
> +     sama7g5_tcpc->tcpc.set_cc = sama7g5_tcpc_set_cc;
> +     sama7g5_tcpc->tcpc.get_cc = sama7g5_tcpc_get_cc;
> +     sama7g5_tcpc->tcpc.set_polarity = sama7g5_tcpc_set_polarity;
> +     sama7g5_tcpc->tcpc.set_vconn = sama7g5_tcpc_set_vconn;
> +     sama7g5_tcpc->tcpc.start_toggling = sama7g5_tcpc_start_toggling;
> +     sama7g5_tcpc->tcpc.set_pd_rx = sama7g5_tcpc_set_pd_rx;
> +     sama7g5_tcpc->tcpc.set_roles = sama7g5_tcpc_set_roles;
> +     sama7g5_tcpc->tcpc.pd_transmit = sama7g5_tcpc_pd_transmit;
> +
> +     sama7g5_tcpc->tcpc.fwnode = device_get_named_child_node(&pdev->dev,
> +                                                             "connector");
> +     if (!sama7g5_tcpc->tcpc.fwnode) {
> +             dev_err(&pdev->dev, "Can't find connector node.\n");
> +             return -EINVAL;
> +     }
> +
> +     sama7g5_tcpc->tcpm = tcpm_register_port(sama7g5_tcpc->dev,
> +                                             &sama7g5_tcpc->tcpc);
> +     if (IS_ERR(sama7g5_tcpc->tcpm)) {
> +             fwnode_remove_software_node(sama7g5_tcpc->tcpc.fwnode);
> +             return PTR_ERR(sama7g5_tcpc->tcpm);
> +     }
> +
> +     return 0;
> +}
> +
> +static int sama7g5_tcpc_remove(struct platform_device *pdev)
> +{
> +     struct sama7g5_tcpc *sama7g5_tcpc;
> +
> +     sama7g5_tcpc = platform_get_drvdata(pdev);
> +
> +     /* Mask everything */
> +     if (sama7g5_tcpc->vbus_pin)
> +             disable_irq(gpiod_to_irq(sama7g5_tcpc->vbus_pin));
> +
> +
> +     if (!IS_ERR_OR_NULL(sama7g5_tcpc->tcpm))
> +             tcpm_unregister_port(sama7g5_tcpc->tcpm);
> +
> +     destroy_workqueue(sama7g5_tcpc->wq);
> +
> +     clk_disable_unprepare(sama7g5_tcpc->gclk);
> +     clk_disable_unprepare(sama7g5_tcpc->pclk);
> +
> +     return 0;
> +}
> +
> +static const struct of_device_id sama7g5_tcpc_dt_ids[] = {
> +     {
> +             .compatible = "microchip,sama7g5-tcpc",
> +     },
> +     { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, sama7g5_tcpc_dt_ids);
> +
> +static struct platform_driver sama7g5_tcpc_driver = {
> +     .probe  = sama7g5_tcpc_probe,
> +     .remove = sama7g5_tcpc_remove,
> +     .driver = {
> +             .name   = "microchip,sama7g5-tcpc",
> +             .of_match_table = sama7g5_tcpc_dt_ids,
> +     },
> +};
> +module_platform_driver(sama7g5_tcpc_driver);
> +
> +MODULE_AUTHOR("Cristian Birsan <cristian.bir...@microchip.com>");
> +MODULE_DESCRIPTION("Microchip SAMA7G5 Type-C Port Controller Driver");
> +MODULE_LICENSE("GPL");

thanks,

-- 
heikki

Reply via email to