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