All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexandru Ardelean <ardeleanalex@gmail.com>
To: Alexandru Ardelean <alexandru.ardelean@analog.com>
Cc: linux-hwmon@vger.kernel.org,
	devicetree <devicetree@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Guenter Roeck <linux@roeck-us.net>,
	jdelvare@suse.com, "Thoren, Mark" <mark.thoren@analog.com>
Subject: Re: [PATCH v2 3/4] hwmon: (ltc2945): add support for sense resistor
Date: Wed, 18 Nov 2020 16:43:07 +0200	[thread overview]
Message-ID: <CA+U=Dsqh1MDGjWfULdnmw5+2tTD4dutiodqr+uCdPhsFMbs=iQ@mail.gmail.com> (raw)
In-Reply-To: <20201111091259.46773-4-alexandru.ardelean@analog.com>

On Wed, Nov 11, 2020 at 11:08 AM Alexandru Ardelean
<alexandru.ardelean@analog.com> wrote:
>
> The sense resistor is a parameter of the board. It should be configured in
> the driver via a device-tree / ACPI property, so that the proper current
> measurements can be done in the driver.
>
> It shouldn't be necessary that userspace need to know about the value of
> the resistor. It makes things a bit harder to make the application code
> portable from one board to another.
>
> This change implements support for the sense resistor to be configured from
> DT/ACPI and used in current calculations.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
>  drivers/hwmon/ltc2945.c | 53 ++++++++++++++++++++---------------------
>  1 file changed, 26 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c
> index 6d4569a25471..909dd92a7a20 100644
> --- a/drivers/hwmon/ltc2945.c
> +++ b/drivers/hwmon/ltc2945.c
> @@ -61,9 +61,11 @@
>  /**
>   * struct ltc2945_state - driver instance specific data
>   * @regmap             regmap object to access device registers
> + * @r_sense_uohm       current sense resistor value
>   */
>  struct ltc2945_state {
>         struct regmap           *regmap;
> +       u32                     r_sense_uohm;
>  };
>
>  static inline bool is_power_reg(u8 reg)
> @@ -101,9 +103,8 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg)
>         case LTC2945_MAX_POWER_THRES_H:
>         case LTC2945_MIN_POWER_THRES_H:
>                 /*
> -                * Convert to uW by assuming current is measured with
> -                * an 1mOhm sense resistor, similar to current
> -                * measurements.
> +                * Convert to uW by and scale it with the configured
> +                * sense resistor, similar to current measurements.
>                  * Control register bit 0 selects if voltage at SENSE+/VDD
>                  * or voltage at ADIN is used to measure power.
>                  */
> @@ -112,10 +113,10 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg)
>                         return ret;
>                 if (control & CONTROL_MULT_SELECT) {
>                         /* 25 mV * 25 uV = 0.625 uV resolution. */
> -                       val *= 625LL;
> +                       val = DIV_ROUND_CLOSEST_ULL(val * 625LL * 1000, st->r_sense_uohm);
>                 } else {
>                         /* 0.5 mV * 25 uV = 0.0125 uV resolution. */
> -                       val = (val * 25LL) >> 1;
> +                       val = DIV_ROUND_CLOSEST_ULL(val * 25LL * 1000, st->r_sense_uohm) >> 1;
>                 }
>                 break;
>         case LTC2945_VIN_H:
> @@ -140,13 +141,10 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg)
>         case LTC2945_MAX_SENSE_THRES_H:
>         case LTC2945_MIN_SENSE_THRES_H:
>                 /*
> -                * 25 uV resolution. Convert to current as measured with
> -                * an 1 mOhm sense resistor, in mA. If a different sense
> -                * resistor is installed, calculate the actual current by
> -                * dividing the reported current by the sense resistor value
> -                * in mOhm.
> +                * 25 uV resolution. Convert to current and scale it
> +                * with the value of the sense resistor.
>                  */
> -               val *= 25;
> +               val = DIV_ROUND_CLOSEST_ULL(val * 25 * 1000, st->r_sense_uohm);
>                 break;
>         default:
>                 return -EINVAL;
> @@ -169,9 +167,8 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg,
>         case LTC2945_MAX_POWER_THRES_H:
>         case LTC2945_MIN_POWER_THRES_H:
>                 /*
> -                * Convert to register value by assuming current is measured
> -                * with an 1mOhm sense resistor, similar to current
> -                * measurements.
> +                * Convert to register value, scale it with the configured sense
> +                * resistor value, similar to current measurements.
>                  * Control register bit 0 selects if voltage at SENSE+/VDD
>                  * or voltage at ADIN is used to measure power, which in turn
>                  * determines register calculations.
> @@ -181,14 +178,10 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg,
>                         return ret;
>                 if (control & CONTROL_MULT_SELECT) {
>                         /* 25 mV * 25 uV = 0.625 uV resolution. */
> -                       val = DIV_ROUND_CLOSEST_ULL(val, 625);
> +                       val = DIV_ROUND_CLOSEST_ULL(val * 1000, 625 * st->r_sense_uohm);

I think that in this ltc2945_val_to_reg(), I should do the math the
other way around.
i.e.    val = DIV_ROUND_CLOSEST_ULL(val * st->r_sense_uohm, 625000);

I got confused initially and did it wrong.

>                 } else {
> -                       /*
> -                        * 0.5 mV * 25 uV = 0.0125 uV resolution.
> -                        * Divide first to avoid overflow;
> -                        * accept loss of accuracy.
> -                        */
> -                       val = DIV_ROUND_CLOSEST_ULL(val, 25) * 2;
> +                       /* 0.5 mV * 25 uV = 0.0125 uV resolution. */
> +                       val = DIV_ROUND_CLOSEST_ULL(val * 2 * 1000, 25 * st->r_sense_uohm);
>                 }
>                 break;
>         case LTC2945_VIN_H:
> @@ -213,13 +206,10 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg,
>         case LTC2945_MAX_SENSE_THRES_H:
>         case LTC2945_MIN_SENSE_THRES_H:
>                 /*
> -                * 25 uV resolution. Convert to current as measured with
> -                * an 1 mOhm sense resistor, in mA. If a different sense
> -                * resistor is installed, calculate the actual current by
> -                * dividing the reported current by the sense resistor value
> -                * in mOhm.
> +                * 25 uV resolution. Convert to current and scale it
> +                * with the value of the sense resistor, in mA.
>                  */
> -               val = DIV_ROUND_CLOSEST_ULL(val, 25);
> +               val = DIV_ROUND_CLOSEST_ULL(val * 1000, 25 * st->r_sense_uohm);
>                 break;
>         default:
>                 return -EINVAL;
> @@ -475,6 +465,15 @@ static int ltc2945_probe(struct i2c_client *client)
>                 return PTR_ERR(regmap);
>         }
>
> +       if (device_property_read_u32(dev, "shunt-resistor-micro-ohms",
> +                                    &st->r_sense_uohm))
> +               st->r_sense_uohm = 1000;
> +
> +       if (st->r_sense_uohm == 0) {
> +               dev_err(dev, "Zero value provided for sense resistor in DT");
> +               return -EINVAL;
> +       }
> +
>         st->regmap = regmap;
>
>         /* Clear faults */
> --
> 2.17.1
>

  reply	other threads:[~2020-11-18 14:44 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-11  9:12 [PATCH v2 0/4] hwmon: (ltc2945): add support for sense resistor Alexandru Ardelean
2020-11-11  9:12 ` [PATCH v2 1/4] hwmon: (ltc2945): wrap regmap into an ltc2945_state struct Alexandru Ardelean
2020-11-11  9:12 ` [PATCH v2 2/4] docs: hwmon: (ltc2945): change type of val to ULL in ltc2945_val_to_reg() Alexandru Ardelean
2020-11-11 14:54   ` Guenter Roeck
2020-11-11 15:28     ` Alexandru Ardelean
2020-11-11 15:44       ` Guenter Roeck
2020-11-18 14:40     ` Alexandru Ardelean
2020-11-18 15:19       ` Guenter Roeck
2020-11-13  7:25   ` kernel test robot
2020-11-11  9:12 ` [PATCH v2 3/4] hwmon: (ltc2945): add support for sense resistor Alexandru Ardelean
2020-11-18 14:43   ` Alexandru Ardelean [this message]
2020-11-18 15:21     ` Guenter Roeck
2020-11-11  9:12 ` [PATCH v2 4/4] dt-bindings: hwmon: ltc2945: add device tree doc for ltc2945 Alexandru Ardelean
2020-11-16 17:36   ` Rob Herring

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='CA+U=Dsqh1MDGjWfULdnmw5+2tTD4dutiodqr+uCdPhsFMbs=iQ@mail.gmail.com' \
    --to=ardeleanalex@gmail.com \
    --cc=alexandru.ardelean@analog.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jdelvare@suse.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=mark.thoren@analog.com \
    --cc=robh+dt@kernel.org \
    /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.