On 20-07-20, 15:25, Nagarjuna Kristam wrote:

> +#define USB2_BATTERY_CHRG_OTGPADX_CTL0(x)    (0x80 + (x) * 0x40)
> +#define  PD_CHG                                      BIT(0)
> +#define  VDCD_DET_FILTER_EN                  BIT(4)
> +#define  VDAT_DET                            BIT(5)
> +#define  VDAT_DET_FILTER_EN                  BIT(8)
> +#define  OP_SINK_EN                          BIT(9)
> +#define  OP_SRC_EN                           BIT(10)
> +#define  ON_SINK_EN                          BIT(11)
> +#define  ON_SRC_EN                           BIT(12)
> +#define  OP_I_SRC_EN                         BIT(13)
> +#define  ZIP_FILTER_EN                               BIT(21)
> +#define  ZIN_FILTER_EN                               BIT(25)
> +#define  DCD_DETECTED                                BIT(26)
> +
> +#define USB2_BATTERY_CHRG_OTGPADX_CTL1(x)    (0x84 + (x) * 0x40)
> +#define  PD_VREG                             BIT(6)
> +#define  VREG_LEV(x)                         (((x) & 0x3) << 7)
> +#define  VREG_DIR(x)                         (((x) & 0x3) << 11)

GENMASK and FIELD_PREP please

> +static void
> +tegra_xusb_padctl_set_debounce_time(struct tegra_xusb_padctl *padctl,
> +                                 u32 debounce)
> +{
> +     u32 value;
> +
> +     value = padctl_readl(padctl,
> +             XUSB_PADCTL_USB2_BATTERY_CHRG_TDCD_DBNC_TIMER_0);

Single line

> +static void
> +tegra_xusb_padctl_charger_detect_on(struct tegra_xusb_padctl *padctl, u32 
> index)
> +{
> +     u32 value;
> +
> +     value = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
> +     value &= ~USB2_OTG_PD_ZI;
> +     padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
> +
> +     value = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
> +     value |= (USB2_OTG_PD2 | USB2_OTG_PD2_OVRD_EN);
> +     padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
> +
> +     value = padctl_readl(padctl, USB2_BATTERY_CHRG_OTGPADX_CTL0(index));
> +     value &= ~PD_CHG;
> +     padctl_writel(padctl, value, USB2_BATTERY_CHRG_OTGPADX_CTL0(index));

maybe a padctl_updatel() helper to do read, modify and write

> +static void tegra_xusb_padctl_dcd(struct tegra_xusb_padctl *padctl, u32 
> index)
> +{
> +     u32 value;
> +     unsigned int offset;
> +     bool ret = false;
> +
> +     /* Turn on IDP_SRC */
> +     value = padctl_readl(padctl, USB2_BATTERY_CHRG_OTGPADX_CTL0(index));
> +     value |= OP_I_SRC_EN;
> +     padctl_writel(padctl, value, USB2_BATTERY_CHRG_OTGPADX_CTL0(index));
> +
> +     /* Turn on D- pull-down resistor */
> +     value = padctl_readl(padctl, USB2_BATTERY_CHRG_OTGPADX_CTL1(index));
> +     value |= USBON_RPD_OVRD_VAL;
> +     padctl_writel(padctl, value, USB2_BATTERY_CHRG_OTGPADX_CTL1(index));
> +
> +     /* Wait for TDCD_DBNC (DCD debounce), refer to BC1.2 spec Table 5 */
> +     usleep_range(10000, 20000);
> +
> +     offset = USB2_BATTERY_CHRG_OTGPADX_CTL0(index);
> +     ret = readl_poll_timeout(padctl->regs + offset, value,
> +                              value & DCD_DETECTED, 20000,
> +                              TDCD_TIMEOUT_MS * 1000);
> +     if (!ret)
> +             dev_warn(padctl->dev, "%s: DCD timeout.", __func__);

Not dev_err() ? and continue after this?

> +static bool
> +tegra_xusb_padctl_primary_secondary(struct tegra_xusb_padctl *padctl, u32 
> index,
> +                                 bool is_primary)
> +{
> +     u32 value;
> +     u32 config = is_primary ? (OP_SRC_EN | ON_SINK_EN) :
> +                               (ON_SRC_EN | OP_SINK_EN);
> +     bool ret = false;
> +
> +     if (is_primary)
> +             /* data contact detection */
> +             tegra_xusb_padctl_dcd(padctl, index);
> +
> +     /* Source D- to D+ */
> +     value = padctl_readl(padctl, USB2_BATTERY_CHRG_OTGPADX_CTL0(index));
> +     value |= config;
> +     padctl_writel(padctl, value, USB2_BATTERY_CHRG_OTGPADX_CTL0(index));
> +
> +     /*
> +      * Wait for TVDPSRC_ON/TVDMSRC_ON(D+/- voltage source on time),
> +      * refer to BC1.2 spec Table 5
> +      */
> +     msleep(40);
> +
> +     value = padctl_readl(padctl, USB2_BATTERY_CHRG_OTGPADX_CTL0(index));
> +     if (is_primary)
> +             ret = !!(value & VDAT_DET);
> +     else
> +             ret = !(value & VDAT_DET);

How about 

        ret = !(value & VDAT_DET);
        if (is_primary)
                ret = !ret;

> +#define VON_DIV2P0_DET BIT(0)
> +#define VON_DIV2P7_DET BIT(1)
> +#define VOP_DIV2P0_DET BIT(2)
> +#define VOP_DIV2P7_DET BIT(3)
> +
> +#define VREG_CUR_LEVEL_0        500
> +#define VREG_CUR_LEVEL_1        900
> +#define VREG_CUR_LEVEL_2        1500
> +#define VREG_CUR_LEVEL_3        2000

tabs or spaces, pick one please, not both

> +
> +#define IS_CUR_IN_RANGE(ma, low, high)  \
> +     ((ma >= VREG_CUR_LEVEL_##low) && (ma <= (VREG_CUR_LEVEL_##high - 1)))
> +#define VREG_LVL(ma, level)     IS_CUR_IN_RANGE(ma, level, level + 1)
> +
> +static void tegra_xusb_padctl_vbus_pad_portection(struct tegra_xusb_port 
> *port)
> +{
> +     struct tegra_xusb_padctl *padctl = port->padctl;
> +     int level = 0;
> +     enum tegra_vbus_dir dir = TEGRA_VBUS_SINK;
> +     int max_ua, min_ua;

reverse christmas tree please

> +static enum usb_charger_type tegra_xusb_charger_detect(struct usb_phy 
> *usb_phy)
> +{
> +     struct tegra_xusb_port *port = container_of(usb_phy,
> +                                                 struct tegra_xusb_port,
> +                                                 usb_phy);

how about:

        struct tegra_xusb_port *port = 
                        container_of(usb_phy, struct tegra_xusb_port, usb_phy);
-- 
~Vinod

Reply via email to