lkml.org 
[lkml]   [2019]   [Feb]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 2/2] extcon intel-cht-wc: Enable external charger
    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.shevchenko@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


    \
     
     \ /
      Last update: 2019-02-20 14:09    [W:3.851 / U:0.244 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site