[...] > + regs = (__raw_readl(sreg->base_addr) & ~BM_POWER_LEVEL_TRG); I suspect you should be using the *_relaxed accessors rather than the __raw_* accessors. [...] > +static int mxs_regulator_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + struct device_node *parent; > + struct regulator_desc *rdesc; > + struct regulator_dev *rdev; > + struct mxs_regulator *sreg; > + struct regulator_init_data *initdata; > + struct regulation_constraints *con; > + struct regulator_config config = { }; > + void __iomem *base_addr = NULL; > + void __iomem *power_addr = NULL; > + u64 regaddr64 = 0; > + const u32 *regaddr_p; > + u32 val = 0; > + int ret; > + > + if (!np) { > + dev_err(dev, "missing device tree\n"); > + return -EINVAL; > + } > + > + /* get device base address */ > + base_addr = of_iomap(np, 0); > + if (!base_addr) > + return -ENXIO; > + > + parent = of_get_parent(np); > + if (!parent) > + return -ENXIO; Leak of the (successfully mapped) base_addr. > + > + power_addr = of_iomap(parent, 0); > + if (!power_addr) > + return -ENXIO; Leak of base_addr and dangling refcount on parent. These apply to all subsequent returns. > + > + regaddr_p = of_get_address(np, 0, NULL, NULL); of_get_address returns a __be32*, not a u32*, so sparse will be very unhappy here... > + if (regaddr_p) > + regaddr64 = of_translate_address(np, regaddr_p); ...and as of_translate_address returns a u64 you'll need a separate variable for the input and output. > + > + if (!regaddr64) { > + dev_err(dev, "no or invalid reg property set\n"); > + return -EINVAL; > + } > + > + initdata = of_get_regulator_init_data(dev, np); > + if (!initdata) > + return -EINVAL; > + > + ret = of_property_read_u32(np, "mxs-max-reg-val", > + &val); > + if (!val) { > + dev_err(dev, "no or invalid mxs-max-reg-val property set\n"); > + return ret; > + } > + > + dev_info(dev, "regulator found\n"); > + > + sreg = devm_kzalloc(dev, sizeof(*sreg), GFP_KERNEL); > + if (!sreg) > + return -ENOMEM; > + sreg->initdata = initdata; > + sreg->name = of_get_property(np, "regulator-name", NULL); I'm not keen on using of_get_property here. We have no idea if regulator-name is even a string (it should be, but we have no guarantee). > + sreg->cur_uA = 0; > + sreg->cur_uV = 0; > + sreg->base_addr = base_addr; > + sreg->power_addr = power_addr; > + init_waitqueue_head(&sreg->wait_q); > + spin_lock_init(&sreg->lock); > + sreg->max_reg_val = val; > + > + rdesc = &sreg->rdesc; > + rdesc->name = sreg->name; > + rdesc->owner = THIS_MODULE; > + rdesc->ops = &mxs_rops; > + > + if (strcmp(rdesc->name, "overall_current") == 0) > + rdesc->type = REGULATOR_CURRENT; > + else > + rdesc->type = REGULATOR_VOLTAGE; Wouldn't it make more sense to explicitly match the names you expect? > + con = &initdata->constraints; > + rdesc->n_voltages = sreg->max_reg_val; > + rdesc->min_uV = con->min_uV; > + rdesc->uV_step = (con->max_uV - con->min_uV) / sreg->max_reg_val; > + rdesc->linear_min_sel = 0; > + rdesc->vsel_reg = regaddr64; > + rdesc->vsel_mask = BM_POWER_LEVEL_TRG; > + > + config.dev = &pdev->dev; > + config.init_data = initdata; > + config.driver_data = sreg; > + config.of_node = np; > + > + pr_debug("probing regulator %s %s %d\n", > + sreg->name, > + rdesc->name, > + pdev->id); Aren't those two names always the same per the code above? > + > + /* register regulator */ > + rdev = devm_regulator_register(dev, rdesc, &config); > + > + if (IS_ERR(rdev)) { > + dev_err(&pdev->dev, "failed to register %s\n", > + rdesc->name); > + return PTR_ERR(rdev); > + } > + > + if (sreg->max_uA) { > + struct regulator *regu; > + > + regu = regulator_get(NULL, sreg->name); > + sreg->nb.notifier_call = reg_callback; > + regulator_register_notifier(regu, &sreg->nb); > + } > + > + platform_set_drvdata(pdev, rdev); > + > + of_property_read_u32(np, "mxs-default-microvolt", > + &val); > + > + if (val) > + mxs_set_voltage(rdev, val, val, NULL); As I mentioned in my comments on the binding, I'd like to know why this is necessary and if it is why it shouldn't be a standardised property. Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html