Messages in this thread | | | From | Andy Shevchenko <> | Date | Mon, 4 Jul 2022 22:53:22 +0200 | Subject | Re: [PATCH v4 10/13] power: supply: mt6370: Add Mediatek MT6370 charger driver |
| |
On Mon, Jul 4, 2022 at 7:42 AM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
> Add Mediatek MT6370 charger driver.
...
> + This driver can also be built as a module. If so the module
If so,
> + will be called "mt6370-charger.ko".
No ".ko" part.
...
> +#define MT6370_MIVR_IBUS_TH_100_MA 100000
mA
...
> + if (mt6370_chg_fields[fd].range) { > + return linear_range_get_value(mt6370_chg_fields[fd].range, > + reg_val, val);
> + } else {
Redundant 'else'.
> + *val = reg_val; > + return 0; > + }
...
> + if (fd != F_VMIVR)
Why not positive conditional?
> + linear_range_get_selector_within(r, val, &val); > + else { > + ret = linear_range_get_selector_high(r, val, &val, &f); > + if (!ret) > + val = r->max_sel; > + }
Checkpatch didn't complain about the absence of {} ?
...
> + irq_num = platform_get_irq_byname(pdev, irq_name); > + if (irq_num >= 0) {
if (irq_num < 0) return;
> + if (en) > + enable_irq(irq_num); > + else > + disable_irq_nosync(irq_num); > + }
...
> + /* Check in otg mode or not */
OTG
...
> +static int mt6370_chg_set_online(struct mt6370_priv *priv, > + const union power_supply_propval *val) > +{ > + unsigned int pwr_rdy = !!val->intval;
Seems you are using int as boolean, do you need it specifically for something? Otherwise use boolean here.
> + > + mutex_lock(&priv->attach_lock); > + if (pwr_rdy == !!priv->attach) { > + dev_err(priv->dev, "pwr_rdy is same(%d)\n", pwr_rdy); > + mutex_unlock(&priv->attach_lock); > + return 0; > + } > + > + priv->attach = pwr_rdy; > + mutex_unlock(&priv->attach_lock); > + > + if (!queue_work(priv->wq, &priv->bc12_work)) > + dev_err(priv->dev, "bc12 work has already queued\n"); > + > + return 0; > +}
-- With Best Regards, Andy Shevchenko
| |