Re: [PATCH v2 1/2] extcon-intel-cht-wc: Make charger detection co-existed with OTG host mode

[Andy Shevchenko]

On Wed, Feb 20, 2019 at 12:24:40AM +0300, Yauhen Kharuzhy wrote:
> Whiskey Cove Cherry Trail PMIC requires disabling OTG host mode before
> of charger detection procedure. Do this by manipulationg of CHGRCTRL1
> register.
> 
> Source: APCI DSDT code of Lenovo Yoga Book YB1-X91L and open-sourced
> Intel's drivers.

Some minor comments below.

Otherwise,

Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

> -#define CHT_WC_CHGRCTRL1		0x5e17
> +#define CHT_WC_CHGRCTRL1			0x5e17

Not related change?

> +#define CHT_WC_CHGRCTRL1_DBPEN_MASK		BIT(7)

Drop the _MASK, it's one bit anyway.

> +#define CHT_WC_CHGRCTRL1_OTGMODE		BIT(6)
> +#define CHT_WC_CHGRCTRL1_FTEMP_EVENT		BIT(5)
> +#define CHT_WC_CHGRCTRL1_FUSB_INLMT_1500	BIT(4)
> +#define CHT_WC_CHGRCTRL1_FUSB_INLMT_900		BIT(3)
> +#define CHT_WC_CHGRCTRL1_FUSB_INLMT_500		BIT(2)
> +#define CHT_WC_CHGRCTRL1_FUSB_INLMT_150		BIT(1)
> +#define CHT_WC_CHGRCTRL1_FUSB_INLMT_100		BIT(0)

I think better to keep ascending order.

> +static void cht_wc_extcon_set_otgmode(struct cht_wc_extcon_data *ext,
> +				      bool enable)
> +{
> +	unsigned int chgrctrl1;
> +	int ret;
> +
> +	ret = regmap_read(ext->regmap, CHT_WC_CHGRCTRL1, &chgrctrl1);
> +	if (ret) {
> +		dev_err(ext->dev, "Error reading CHGRCTRL1 reg: %d\n", ret);
> +		return;
> +	}
> +
> +	if (enable)
> +		chgrctrl1 |= CHT_WC_CHGRCTRL1_OTGMODE;
> +	else
> +		chgrctrl1 &= ~(CHT_WC_CHGRCTRL1_OTGMODE);

Redundant parens.

> +
> +	ret = regmap_write(ext->regmap, CHT_WC_CHGRCTRL1, chgrctrl1);
> +	if (ret)
> +		dev_err(ext->dev,
> +			"Error writing CHGRCTRL1 OTG mode bit: %d\n", ret);
> +}

-- 
With Best Regards,
Andy Shevchenko