lkml.org 
[lkml]   [2010]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] MMC: move regulator handling closer to core v3
    On Sun,  5 Sep 2010 11:05:38 +0200
    Linus Walleij <linus.walleij@stericsson.com> wrote:

    > After discovering a problem in regulator reference counting I
    > took Mark Brown's advice to move the reference count into the
    > MMC core by making the regulator status a member of
    > struct mmc_host.
    >
    >
    > ...
    >
    > -static inline void pxamci_set_power(struct pxamci_host *host, unsigned int vdd)
    > +static inline void pxamci_set_power(struct pxamci_host *host,
    > + unsigned char power_mode,
    > + unsigned int vdd)
    > {
    > int on;
    >
    > -#ifdef CONFIG_REGULATOR
    > - if (host->vcc)
    > - mmc_regulator_set_ocr(host->vcc, vdd);
    > -#endif
    > + if (host->vcc) {
    > + int ret;
    > +
    > + if (power_mode == MMC_POWER_UP)
    > + ret = mmc_regulator_set_ocr(host->mmc, host->vcc, vdd);
    > + else if (power_mode == MMC_POWER_OFF)
    > + ret = mmc_regulator_set_ocr(host->mmc, host->vcc, 0);
    > + }

    There's no point in copying the return value into a local then ignoring
    it. mmc_regulator_set_ocr() can return a negative errno so we should
    test for that, clean up and propagate the error.

    If we really do deliberately ignore the error then there should be a
    code comment which excuses this behaviour and perhaps a warning printk.

    The same comments apply to mmci_set_ios().

    omap_hsmmc_1_set_power() gets it right.

    Why doesn't omap_hsmmc_23_set_sleep() run .before_set_reg() and
    .after_set_reg()?

    >
    > ...
    >


    \
     
     \ /
      Last update: 2010-09-09 00:55    [W:0.022 / U:63.648 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site