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