From: Mark Brown <broonie@kernel.org> To: atull@opensource.altera.com Cc: linux@roeck-us.net, jdelvare@suse.de, lm-sensors@lm-sensors.org, lgirdwood@gmail.com, robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, delicious.quinoa@gmail.com, dinguyen@opensource.altera.com, yvanderv@opensource.altera.com Subject: Re: [PATCH v4 3/4] pmbus: add regulator support Date: Thu, 2 Oct 2014 12:20:11 +0100 [thread overview] Message-ID: <20141002112011.GS4273@sirena.org.uk> (raw) In-Reply-To: <1412190349-16343-4-git-send-email-atull@opensource.altera.com> [-- Attachment #1: Type: text/plain, Size: 1860 bytes --] On Wed, Oct 01, 2014 at 02:05:49PM -0500, atull@opensource.altera.com wrote: > From: Alan Tull <atull@opensource.altera.com> > > Add support for simple on/off control of each channel. This is basically fine but the regulator API has moved on a bit with the DT handling over the time you've been working on this. > Signed-off-by: Alan Tull <atull@opensource.altera.com> > > v2: Remove '#include <linux/regulator/machine.h>' > Only one regulator per pmbus device Put this after the --- as covered in SubmittingPatches. > +static int _pmbus_regulator_on_off(struct regulator_dev *rdev, bool enable) > +{ > + struct device *dev = rdev_get_dev(rdev); > + struct i2c_client *client = to_i2c_client(dev->parent); > + u8 page = rdev_get_id(rdev); > + > + return pmbus_update_byte_data(client, page, PMBUS_OPERATION, > + PB_OPERATION_CONTROL_ON, > + enable ? PB_OPERATION_CONTROL_ON : 0); > +} > + > +static int pmbus_regulator_enable(struct regulator_dev *rdev) > +{ > + return _pmbus_regulator_on_off(rdev, 1); > +} I'm not sure factoring out the code actually won much here. > + np_regulators = of_get_child_by_name(dev->of_node, "regulators"); > + if (!np_regulators) > + return 0; > + > + ret = of_regulator_match(dev, np_regulators, info->reg_matches, > + info->num_regulators); > + of_node_put(np_regulators); We now have helpers in the regulator core for this - set of_match and regulators_node in the regulator descriptor and the core will resolve this stuff for you and... > + if (pdata && pdata->reg_init_data) { > + config.init_data = &pdata->reg_init_data[i]; > + } else { > + config.init_data = info->reg_matches[i].init_data; > + config.of_node = info->reg_matches[i].of_node; > + } ...you can just drop the else case (and check for reg_init_data) here. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie@kernel.org> To: atull@opensource.altera.com Cc: linux@roeck-us.net, jdelvare@suse.de, lm-sensors@lm-sensors.org, lgirdwood@gmail.com, robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, delicious.quinoa@gmail.com, dinguyen@opensource.altera.com, yvanderv@opensource.altera.com Subject: Re: [lm-sensors] [PATCH v4 3/4] pmbus: add regulator support Date: Thu, 02 Oct 2014 11:20:11 +0000 [thread overview] Message-ID: <20141002112011.GS4273@sirena.org.uk> (raw) In-Reply-To: <1412190349-16343-4-git-send-email-atull@opensource.altera.com> [-- Attachment #1.1: Type: text/plain, Size: 1860 bytes --] On Wed, Oct 01, 2014 at 02:05:49PM -0500, atull@opensource.altera.com wrote: > From: Alan Tull <atull@opensource.altera.com> > > Add support for simple on/off control of each channel. This is basically fine but the regulator API has moved on a bit with the DT handling over the time you've been working on this. > Signed-off-by: Alan Tull <atull@opensource.altera.com> > > v2: Remove '#include <linux/regulator/machine.h>' > Only one regulator per pmbus device Put this after the --- as covered in SubmittingPatches. > +static int _pmbus_regulator_on_off(struct regulator_dev *rdev, bool enable) > +{ > + struct device *dev = rdev_get_dev(rdev); > + struct i2c_client *client = to_i2c_client(dev->parent); > + u8 page = rdev_get_id(rdev); > + > + return pmbus_update_byte_data(client, page, PMBUS_OPERATION, > + PB_OPERATION_CONTROL_ON, > + enable ? PB_OPERATION_CONTROL_ON : 0); > +} > + > +static int pmbus_regulator_enable(struct regulator_dev *rdev) > +{ > + return _pmbus_regulator_on_off(rdev, 1); > +} I'm not sure factoring out the code actually won much here. > + np_regulators = of_get_child_by_name(dev->of_node, "regulators"); > + if (!np_regulators) > + return 0; > + > + ret = of_regulator_match(dev, np_regulators, info->reg_matches, > + info->num_regulators); > + of_node_put(np_regulators); We now have helpers in the regulator core for this - set of_match and regulators_node in the regulator descriptor and the core will resolve this stuff for you and... > + if (pdata && pdata->reg_init_data) { > + config.init_data = &pdata->reg_init_data[i]; > + } else { > + config.init_data = info->reg_matches[i].init_data; > + config.of_node = info->reg_matches[i].of_node; > + } ...you can just drop the else case (and check for reg_init_data) here. [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] [-- Attachment #2: Type: text/plain, Size: 153 bytes --] _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
next prev parent reply other threads:[~2014-10-02 11:20 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-10-01 19:05 [PATCH v4 0/4] regulator support for pmbus and ltc2978 atull 2014-10-01 19:05 ` [lm-sensors] " atull 2014-10-01 19:05 ` atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx 2014-10-01 19:05 ` [PATCH v4 1/4] hwmon: ltc2978: device tree bindings documentation atull 2014-10-01 19:05 ` [lm-sensors] " atull 2014-10-01 19:05 ` atull 2014-10-01 19:05 ` [PATCH v4 2/4] pmbus: core: add helpers for byte write and read modify write atull 2014-10-01 19:05 ` [lm-sensors] " atull 2014-10-01 19:05 ` atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx 2014-10-02 4:05 ` Guenter Roeck 2014-10-02 4:05 ` [lm-sensors] " Guenter Roeck 2014-10-02 4:05 ` Guenter Roeck 2014-10-01 19:05 ` [PATCH v4 3/4] pmbus: add regulator support atull 2014-10-01 19:05 ` [lm-sensors] " atull 2014-10-01 19:05 ` atull 2014-10-02 11:20 ` Mark Brown [this message] 2014-10-02 11:20 ` [lm-sensors] " Mark Brown 2014-10-02 13:22 ` Guenter Roeck 2014-10-02 13:22 ` [lm-sensors] " Guenter Roeck 2014-10-02 18:43 ` atull 2014-10-02 18:43 ` [lm-sensors] " atull 2014-10-02 18:43 ` atull 2014-10-01 20:07 ` [PATCH v4 0/4] regulator support for pmbus and ltc2978 Guenter Roeck 2014-10-01 20:07 ` [lm-sensors] " Guenter Roeck 2014-10-01 20:07 ` Guenter Roeck 2014-10-01 20:20 ` atull 2014-10-01 20:20 ` [lm-sensors] " atull 2014-10-01 20:20 ` atull 2014-10-02 3:25 ` Guenter Roeck 2014-10-02 3:25 ` [lm-sensors] " Guenter Roeck
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20141002112011.GS4273@sirena.org.uk \ --to=broonie@kernel.org \ --cc=atull@opensource.altera.com \ --cc=delicious.quinoa@gmail.com \ --cc=devicetree@vger.kernel.org \ --cc=dinguyen@opensource.altera.com \ --cc=galak@codeaurora.org \ --cc=ijc+devicetree@hellion.org.uk \ --cc=jdelvare@suse.de \ --cc=lgirdwood@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@roeck-us.net \ --cc=lm-sensors@lm-sensors.org \ --cc=mark.rutland@arm.com \ --cc=pawel.moll@arm.com \ --cc=robh+dt@kernel.org \ --cc=yvanderv@opensource.altera.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.