On Wed, Feb 20, 2019 at 12:24:41AM +0300, Yauhen Kharuzhy wrote: > In some configuration external charger "#charge enable" signal is > connected to PMIC. Enable it at device probing to allow charging. > > Save CHGRCTRL0 and CHGDISCTR registers at driver probing and restore > them at driver unbind to re-enable hardware charging control if it was > enabled before. > > Tested at Lenovo Yoga Book (YB1-X91L).
My comments below. After addressing them, Reviewed-by: Andy Shevchenko <andriy.shevche...@linux.intel.com> > +#define CHT_WC_CHGDISCTRL_OUTPUT BIT(0) Simple _OUT as per documentation. > +/* 0 - open drain, 1 - regular output */ Regular means push-pull. > +#define CHT_WC_CHGDISCTRL_DRV_OD_DIS BIT(4) _DRV as per documentation. > +#define CHT_WC_CHGDISCTRL_MODE_HW BIT(6) _FN as per documentation. > +static void cht_wc_extcon_enable_charging(struct cht_wc_extcon_data *ext, > + bool enable) > +{ > + unsigned int val; > + int ret; > + > + val = enable ? 0 : CHT_WC_CHGDISCTRL_OUTPUT; Can be assigned in definition block. > + ret = regmap_update_bits(ext->regmap, CHT_WC_CHGDISCTRL, > + CHT_WC_CHGDISCTRL_OUTPUT, val); > + if (ret) > + dev_err(ext->dev, "Error updating CHGDISCTRL reg: %d\n", ret); > +} > + val = enable ? 0 : CHT_WC_CHGDISCTRL_MODE_HW; > + ret = regmap_update_bits(ext->regmap, CHT_WC_CHGDISCTRL, > + CHT_WC_CHGDISCTRL_MODE_HW, val); Indentation. > + if (ret) > + dev_err(ext->dev, > + "Error setting sw control for charger enable: %d\n", > + ret); > +static int cht_wc_save_initial_state(struct cht_wc_extcon_data *ext) > +{ > + int ret; > + > + /* > + * Save the external charger control output state for restoring it at > + * driver unbinding You may move "at" to the next line and add a period at the end. > + */ > + ret = regmap_read(ext->regmap, CHT_WC_CHGDISCTRL, > + &ext->chgdisctrl_saved); > + if (ret) { > + dev_err(ext->dev, "Error reading CHGDISCTRL: %d\n", > + ret); > + return ret; > + } > + > + ret = regmap_read(ext->regmap, CHT_WC_CHGRCTRL0, > + &ext->chgrctrl0_saved); > + if (ret) { > + dev_err(ext->dev, "Error reading CHGRCTRL0: %d\n", > + ret); > + return ret; > + } > + > + return 0; > +} -- With Best Regards, Andy Shevchenko