lkml.org 
[lkml]   [2020]   [Jun]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 5/5] regulator: qcom: labibb: Add SC interrupt handling
    On Tue, Jun 02, 2020 at 03:39:24PM +0530, Sumit Semwal wrote:

    > static int qcom_labibb_regulator_enable(struct regulator_dev *rdev)
    > {
    > - return regulator_enable_regmap(rdev);
    > + int ret;
    > + struct labibb_regulator *reg = rdev_get_drvdata(rdev);
    > +
    > + ret = regulator_enable_regmap(rdev);
    > + if (ret >= 0)
    > + reg->enabled = true;

    Can we not read the register we just wrote to here?

    > + /*
    > + * The SC(short circuit) fault would trigger PBS(Portable Batch
    > + * System) to disable regulators for protection. This would
    > + * cause the SC_DETECT status being cleared so that it's not
    > + * able to get the SC fault status.
    > + * Check if the regulator is enabled in the driver but
    > + * disabled in hardware, this means a SC fault had happened
    > + * and SCP handling is completed by PBS.
    > + */
    > + if (!in_sc_err) {
    > +
    > + reg = labibb_reg->base + REG_LABIBB_ENABLE_CTL;
    > +
    > + ret = regmap_read_poll_timeout(labibb_reg->regmap,
    > + reg, val,
    > + !(val & LABIBB_CONTROL_ENABLE),
    > + POLLING_SCP_DONE_INTERVAL_US,
    > + POLLING_SCP_TIMEOUT);

    Why do we need a timeout here?

    > + NULL);
    > + regulator_unlock(labibb_reg->rdev);
    > + }
    > + return IRQ_HANDLED;

    This returns IRQ_HANDLED even if we didn't detect an interrupt source...
    Especially given the need to check to see if the regulator was turned
    off by the hardware it seems like there must be some false positives.

    > + } else {
    > + ret = devm_request_threaded_irq(reg->dev,
    > + sc_irq,
    > + NULL, labibb_sc_err_handler,
    > + IRQF_ONESHOT,
    > + "sc-err", reg);

    This looks like we're requesting the interrupt before we register the
    regulator which means the interrupt might fire without the regulator
    being there. The order of registration should be reversed.
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2020-06-02 14:23    [W:4.129 / U:1.128 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site