Re: [PATCH 3/4] regulator: as3722: add regulator driver for AMS AS3722
From: Mark Brown <broonie@kernel.org>
Date: 2013-09-17 11:41:42
Also in:
linux-gpio, lkml
On Tue, Sep 17, 2013 at 12:15:37PM +0530, Laxman Dewangan wrote:
+ ams,enable-oc-config: Enable overcurrent configuration of the + ams,oc-trip-threshold-perphase: Overcurrent trip threshold + ams,oc-alarm-threshold-perphase: Overcurrent alarm threshold
These look like you should be implementing the current setting operations in the framework, probably mapping to the alarm limit if nothing else.
+static int as3722_ldo_get_voltage_sel(struct regulator_dev *rdev)
+{
+ int ret;
+
+ ret = regulator_get_voltage_sel_regmap(rdev);
+ if (ret >= 0x40)
+ ret -= 0x1B;
+ return ret;
+}This looks very strange. What's going on here? A gap in the selectors? If that's the case you probably want to be using linear ranges.
+static struct regulator_ops as3722_ldo_extcntrl_ops = {
+ .get_voltage_sel = as3722_ldo_get_voltage_sel,
+ .set_voltage_sel = as3722_ldo_set_voltage_sel,
+ .list_voltage = regulator_list_voltage_linear,
+};You ought to be providing map_voltage() too, same for some if not all of the other regulators.
+static unsigned int as3722_sd_get_mode(struct regulator_dev *dev)
+{
+ struct as3722_regulators *as3722_regs = rdev_get_drvdata(dev);
+ struct as3722 *as3722 = as3722_regs->as3722;
+ int id = rdev_get_id(dev);
+ u32 val;
+ int ret;
+
+ if (!as3722_reg_lookup[id].control_reg)
+ return -ERANGE;That seems a bit of a random error code. -ENOTSUPP?
+static int as3722_sd_list_voltage(struct regulator_dev *rdev, unsigned selector)
+{
+ if (selector >= AS3722_SD2_VSEL_MAX)
+ return -EINVAL;
+
+ selector++;
+ if (selector <= 0x40)
+ return 600000 + selector * 12500;
+ if (selector <= 0x70)
+ return 1400000 + (selector - 0x40) * 25000;
+ if (selector <= 0x7F)
+ return 2600000 + (selector - 0x70) * 50000;
+ return -EINVAL;
+}Use linear ranges.
+static struct of_regulator_match as3722_regulator_matches[] = {
+ {.name = "sd0", },
{ .name = "sd0", },
+ config.of_node = as3722_regulator_matches[id].of_node;
+ rdev = regulator_register(&as3722_regs->desc[id], &config);
+ if (IS_ERR(rdev)) {devm_regualtor_register().
+ if (reg_config->ext_control) {
+ ret = regulator_enable_regmap(rdev);
+ if (ret < 0) {
+ dev_err(&pdev->dev,
+ "Regulator %d enable failed: %d\n",
+ id, ret);
+ goto scrub;
+ }This looks wrong... why is the regualtor being enabled by something other than the core?
+static int __init as3722_regulator_init(void)
+{
+ return platform_driver_register(&as3722_regulator_driver);
+}
+subsys_initcall(as3722_regulator_init);
+
+static void __exit as3722_regulator_exit(void)
+{
+ platform_driver_unregister(&as3722_regulator_driver);
+}
+module_exit(as3722_regulator_exit);module_platform_driver().
Attachments
- signature.asc [application/pgp-signature] 836 bytes