Re: [PATCH V1 2/6] regulator: da9062: DA9062 regulator driver
From: Mark Brown <broonie@kernel.org>
Date: 2015-04-18 11:48:59
Also in:
linux-devicetree, linux-rtc, linux-watchdog, lkml
On Fri, Apr 17, 2015 at 03:23:32PM +0100, S Twiss wrote:
+/* Regulator interrupt handlers */
+static irqreturn_t da9062_ldo_lim_event(int irq, void *data)
+{
+ struct da9062_regulators *regulators = data;
+ struct da9062 *hw = regulators->regulator[0].hw;
+ struct da9062_regulator *regl;
+ int bits, i, ret;
+
+ ret = regmap_read(hw->regmap, DA9062AA_STATUS_D, &bits);
+ if (ret < 0)
+ return IRQ_NONE;Please log an error for this, if we're having trouble talking to the device that seems like a serious issue.
+ for (i = regulators->n_regulators - 1; i >= 0; i--) {
+ regl = ®ulators->regulator[i];
+ if (regl->info->oc_event.reg != DA9062AA_STATUS_D)
+ continue;
+
+ if (BIT(regl->info->oc_event.lsb) & bits)
+ regulator_notifier_call_chain(regl->rdev,
+ REGULATOR_EVENT_OVER_CURRENT, NULL);
+ }
+
+ return IRQ_HANDLED;This will return IRQ_HANDLED even if none of the regulators were flagginng an event.
+static irqreturn_t da9062_vdd_warn_event(int irq, void *data)
+{
+ return IRQ_HANDLED;
+}Ignoring an interrupt is not usefully handling it - at the *least* this should be generating a log message.
+static struct da9062_regulators_pdata *da9062_parse_regulators_dt(
+ struct platform_device *pdev,
+ struct of_regulator_match **reg_matches)
+{
+ struct da9062_regulators_pdata *pdata;
+ struct da9062_regulator_data *rdata;
+ struct device_node *node;
+ int i, n, num;
+
+ node = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
+ if (!node) {
+ dev_err(&pdev->dev, "Regulators device node not found\n");
+ return ERR_PTR(-ENODEV);
+ }
+
+ num = of_regulator_match(&pdev->dev, node, da9062_matches,
+ ARRAY_SIZE(da9062_matches));Don't open code this, describe the DT names in the regualtor_desc and let the core register.
+ if (IS_ERR(pdata) || pdata->n_regulators == 0) {
+ dev_err(&pdev->dev,
+ "No regulators defined for the platform\n");
+ return PTR_ERR(pdata);
+ }
+
+ n_regulators = ARRAY_SIZE(local_regulator_info),This is broken, the set of regulators in the silicon is not a property of the platform. The driver should just register all the regualtors that are present in the silicon. I'm fairly sure I've been through this before...
+ ret = request_threaded_irq(irq, + NULL, da9062_vdd_warn_event, + IRQF_TRIGGER_LOW | IRQF_ONESHOT, + "VDD_WARN", regulators);
devm_request_threaded_irq().
Attachments
- signature.asc [application/pgp-signature] 473 bytes