All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Adam Ward <adam.ward@diasemi.com>
Cc: Rob Herring <robh+dt@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Vincent Whitchurch <vincent.whitchurch@axis.com>,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 7/9] regulator: da9121: add current support
Date: Fri, 20 Nov 2020 13:17:08 +0000	[thread overview]
Message-ID: <20201120131708.GD6751@sirena.org.uk> (raw)
In-Reply-To: <a9b30f4aa8813bf262fd0eb8e007253582f61602.1605868780.git.Adam.Ward.opensource@diasemi.com>

[-- Attachment #1: Type: text/plain, Size: 802 bytes --]

On Fri, Nov 20, 2020 at 12:14:57PM +0000, Adam Ward wrote:

> +static bool da9121_rdev_to_buck_reg_mask(struct regulator_dev *rdev, bool mode,
> +					 unsigned int *reg, unsigned int *msk)
> +{
> +	struct da9121 *chip = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +
> +	switch (id) {
> +	case DA9121_IDX_BUCK1:
> +		if (mode) {
> +			*reg = DA9121_REG_BUCK_BUCK1_4;
> +			*msk = DA9121_MASK_BUCK_BUCKx_4_CHx_A_MODE;
> +		} else {
> +			*reg = DA9121_REG_BUCK_BUCK1_2;
> +			*msk = DA9121_MASK_BUCK_BUCKx_2_CHx_ILIM;
> +		}
> +		break;

This is really not an easy interface to understand, the boolean flag
saying which register set to pick is not particularly obvious to read
and won't scale.  The usual pattern with things like this is to store
information in driver data at probe time.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2020-11-20 13:17 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-20 12:14 [PATCH 0/9] regulator: da9121: extend support to variants, add features Adam Ward
2020-11-20 12:14 ` [PATCH 1/9] regulator: Update DA9121 dt-bindings Adam Ward
2020-11-20 13:47   ` Vincent Whitchurch
2020-11-25  9:21     ` Vincent Whitchurch
2020-11-27 13:01       ` Adam Ward
2020-11-27 14:59         ` Mark Brown
2020-11-20 12:14 ` [PATCH 2/9] regulator: da9121: Add header file Adam Ward
2020-11-20 12:14 ` [PATCH 3/9] regulator: da9121: Add device variants Adam Ward
2020-11-20 12:14 ` [PATCH 4/9] regulator: da9121: Add device variant details and respective regmaps Adam Ward
2020-11-20 12:45   ` Mark Brown
2020-11-20 12:14 ` [PATCH 5/9] regulator: da9121: Add support for device variants via devicetree Adam Ward
2020-11-20 12:51   ` Mark Brown
2020-11-20 12:14 ` [PATCH 6/9] regulator: da9121: Update registration to support multiple buck variants Adam Ward
2020-11-20 13:06   ` Mark Brown
2020-11-20 12:14 ` [PATCH 7/9] regulator: da9121: add current support Adam Ward
2020-11-20 13:17   ` Mark Brown [this message]
2020-11-20 12:14 ` [PATCH 8/9] regulator: da9121: add mode support Adam Ward
2020-11-20 12:14 ` [PATCH 9/9] regulator: da9121: add interrupt support Adam Ward
2020-11-20 13:45   ` Mark Brown

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=20201120131708.GD6751@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=adam.ward@diasemi.com \
    --cc=devicetree@vger.kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=vincent.whitchurch@axis.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: link
Be 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.