lkml.org 
[lkml]   [2021]   [Nov]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2 13/20] power: supply: bq25890: Support higher charging voltages through Pump Express+ protocol
    On Sun, Nov 14, 2021 at 7:04 PM Hans de Goede <hdegoede@redhat.com> wrote:
    >
    > From: Yauhen Kharuzhy <jekhor@gmail.com>
    >
    > Add a "linux,pump-express-vbus-max" property which indicates if the Pump
    > Express+ protocol should be used to increase the charging protocol.
    >
    > If this new property is set and a DCP charger is detected then request
    > a higher charging voltage through the Pump Express+ protocol.
    >
    > So far this new property is only used on X86/ACPI (non devicetree) devs,
    > IOW it is not used in actual devicetree files. The devicetree-bindings
    > maintainers have requested properties like these to not be added to the
    > devicetree-bindings, so the new property is deliberately not added
    > to the existing devicetree-bindings.
    >
    > Changes by Hans de Goede:
    > - Port to my bq25890 patch-series + various cleanups
    > - Make behavior configurable through a new "linux,pump-express-vbus-max"
    > device-property
    > - Sleep 1 second before re-checking the Vbus voltage after requesting
    > it to be raised, to ensure that the ADC has time to sampled the new Vbus
    > - Add VBUSV bq25890_tables[] entry and use it in bq25890_get_vbus_voltage()
    > - Tweak commit message

    ...

    > +#define PUMP_EXPRESS_START_DELAY (5 * HZ)
    > +#define PUMP_EXPRESS_MAX_TRIES 6
    > +#define PUMP_EXPRESS_VBUS_MARGIN 1000000

    Units? Perhaps "_uV"?

    ...

    > + dev_dbg(bq->dev, "input voltage = %d mV\n", voltage);

    Just to be sure, is it indeed "mV" and not "uV"?

    ...

    > + while (bq25890_field_read(bq, F_PUMPX_UP) == 1)
    > + msleep(100);

    Infinite loop?

    Sounds like a good candidate to switch to read_poll_timeout() // note
    it accepts any type of (op) with a variadic number of args.

    ...

    > +error:

    error_print: ?

    > + dev_err(bq->dev, "Failed to request hi-voltage charging\n");

    ...

    > + ret = device_property_read_u32(bq->dev, "linux,pump-express-vbus-max",
    > + &bq->pump_express_vbus_max);
    > + if (ret < 0)
    > + bq->pump_express_vbus_max = 0;

    Isn't it 0 by default?

    Anyway, for all optional properties one may use

    bq->...property... = $default;
    device_property_read_u32(bq->dev, "linux,...property...", &bq->...property...);

    I.e. no conditional needed.

    --
    With Best Regards,
    Andy Shevchenko

    \
     
     \ /
      Last update: 2021-11-16 12:16    [W:7.491 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site