[{"id":1777028,"web_url":"http://patchwork.ozlabs.org/comment/1777028/","msgid":"<2c2de1bb-1396-6d1c-efb0-3411c40f638c@roeck-us.net>","list_archive_url":null,"date":"2017-09-28T13:25:54","subject":"Re: [PATCH 2/4] hwmon: (ina2xx) Make max expected current\n\tconfigurable","submitter":{"id":21889,"url":"http://patchwork.ozlabs.org/api/people/21889/","name":"Guenter Roeck","email":"linux@roeck-us.net"},"content":"On 09/28/2017 05:50 AM, Maciej Purski wrote:\n> Max expected current is used for calculating calibration register value,\n> Current LSB and Power LSB according to equations found in ina datasheet.\n> Max expected current is now implicitly set to default value,\n> which is 2^15, thanks to which Current LSB is equal to 1 mA and\n> Power LSB is equal to 20000 uW or 25000 uW depending on ina model.\n> \n> Make max expected current configurable, just like it's already done\n> with shunt resistance: from device tree, platform_data or later\n> from sysfs. On each max_expected_current change, calculate new values\n> for Current LSB and Power LSB. According to datasheet Current LSB should\n> be calculated by dividing max expected current by 2^15, as values read\n> from device registers are in this case 16-bit integers. Power LSB\n> is calculated by multiplying Current LSB by a factor, which is defined\n> in ina documentation.\n> \n> Signed-off-by: Maciej Purski \n> ---\n> drivers/hwmon/ina2xx.c | 105 +++++++++++++++++++++++++++++++++++++++++++------\n> 1 file changed, 93 insertions(+), 12 deletions(-)\n> \n> diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c\n> index 62e38fa..d956013 100644\n> --- a/drivers/hwmon/ina2xx.c\n> +++ b/drivers/hwmon/ina2xx.c\n> @@ -80,6 +80,8 @@\n> /* common attrs, ina226 attrs and NULL */\n> #define INA2XX_MAX_ATTRIBUTE_GROUPS\t3\n> \n> +#define INA2XX_MAX_EXPECTED_A_DEFAULT (1 << 15) /* current_lsb = 1 mA */\n> +\n> /*\n> * Both bus voltage and shunt voltage conversion times for ina226 are set\n> * to 0b0100 on POR, which translates to 2200 microseconds in total.\n> @@ -100,13 +102,16 @@ struct ina2xx_config {\n> \tint shunt_div;\n> \tint bus_voltage_shift;\n> \tint bus_voltage_lsb;\t/* uV */\n> -\tint power_lsb;\t\t/* uW */\n> +\tint power_lsb_factor;\n> };\n> \n> struct ina2xx_data {\n> \tconst struct ina2xx_config *config;\n> \n> -\tlong rshunt;\n> +\tlong rshunt;\t\t\t\t/* uOhms */\n> +\tunsigned int max_expected_current;\t/* mA */\n> +\tint current_lsb;\t\t\t/* uA */\n> +\tint power_lsb;\t\t\t\t/* uW */\n> \tstruct mutex config_lock;\n> \tstruct regmap *regmap;\n> \n> @@ -121,7 +126,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n> \t\t.shunt_div = 100,\n> \t\t.bus_voltage_shift = 3,\n> \t\t.bus_voltage_lsb = 4000,\n> -\t\t.power_lsb = 20000,\n> +\t\t.power_lsb_factor = 20,\n> \t},\n> \t[ina226] = {\n> \t\t.config_default = INA226_CONFIG_DEFAULT,\n> @@ -130,7 +135,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n> \t\t.shunt_div = 400,\n> \t\t.bus_voltage_shift = 0,\n> \t\t.bus_voltage_lsb = 1250,\n> -\t\t.power_lsb = 25000,\n> +\t\t.power_lsb_factor = 25,\n> \t},\n> };\n> \n> @@ -169,10 +174,17 @@ static u16 ina226_interval_to_reg(int interval)\n> \treturn INA226_SHIFT_AVG(avg_bits);\n> }\n> \n> +/*\n> + * Calculate calibration value according to equation 1 in ina226 datasheet\n> + * http://www.ti.com/lit/ds/symlink/ina226.pdf.\n> + * Current LSB is in uA and RShunt is in uOhms, so in order to keep\n> + * calibration value scaled RShunt must be converted to mOhms.\n> + */\n> static int ina2xx_calibrate(struct ina2xx_data *data)\n> {\n> +\tint r_shunt = DIV_ROUND_CLOSEST(data->rshunt, 1000);\n> \tu16 val = DIV_ROUND_CLOSEST(data->config->calibration_factor,\n> -\t\t\t\t data->rshunt);\n> +\t\t\t\t data->current_lsb * r_shunt);\n> \n> \treturn regmap_write(data->regmap, INA2XX_CALIBRATION, val);\n> }\n> @@ -187,13 +199,28 @@ static int ina2xx_init(struct ina2xx_data *data)\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> -\t/*\n> -\t * Set current LSB to 1mA, shunt is in uOhms\n> -\t * (equation 13 in datasheet).\n> -\t */\n> \treturn ina2xx_calibrate(data);\n> }\n> \n> +/*\n> + * Set max_expected_current (mA) and calculate current_lsb (uA),\n> + * according to equation 2 in ina226 datasheet. Power LSB is calculated\n> + * by multiplying Current LSB by a given factor, which may vary depending\n> + * on ina version.\n> + */\n> +static int set_max_expected_current(struct ina2xx_data *data, unsigned int val)\n> +{\n> +\tif (val <= 0 || val > data->config->calibration_factor)\n> +\t\treturn -EINVAL;\n> +\n> +\tdata->max_expected_current = val;\n> +\tdata->current_lsb = DIV_ROUND_CLOSEST(data->max_expected_current * 1000,\n> +\t\t\t\t\t 1 << 15);\n> +\tdata->power_lsb = data->current_lsb * data->config->power_lsb_factor;\n> +\n> +\treturn 0;\n> +}\n> +\n> static int ina2xx_read_reg(struct device *dev, int reg, unsigned int *regval)\n> {\n> \tstruct ina2xx_data *data = dev_get_drvdata(dev);\n> @@ -268,11 +295,11 @@ static int ina2xx_get_value(struct ina2xx_data *data, u8 reg,\n> \t\tval = DIV_ROUND_CLOSEST(val, 1000);\n> \t\tbreak;\n> \tcase INA2XX_POWER:\n> -\t\tval = regval * data->config->power_lsb;\n> +\t\tval = regval * data->power_lsb;\n> \t\tbreak;\n> \tcase INA2XX_CURRENT:\n> -\t\t/* signed register, LSB=1mA (selected), in mA */\n> -\t\tval = (s16)regval;\n> +\t\tval = (s16)regval * data->current_lsb;\n> +\t\tval = DIV_ROUND_CLOSEST(val, 1000);\n> \t\tbreak;\n> \tcase INA2XX_CALIBRATION:\n> \t\tval = DIV_ROUND_CLOSEST(data->config->calibration_factor,\n> @@ -369,6 +396,39 @@ static ssize_t ina226_show_interval(struct device *dev,\n> \treturn snprintf(buf, PAGE_SIZE, \"%d\\n\", ina226_reg_to_interval(regval));\n> }\n> \n> +static ssize_t ina2xx_max_expected_current_show(struct device *dev,\n> +\t\t\t\t\t struct device_attribute *attr,\n> +\t\t\t\t\t char *buf)\n> +{\n> +\tstruct ina2xx_data *data = dev_get_drvdata(dev);\n> +\n> +\treturn sprintf(buf, \"%d\\n\", data->max_expected_current);\n> +}\n> +\n> +static ssize_t ina2xx_max_expected_current_set(struct device *dev,\n> +\t\t\t\t\t struct device_attribute *attr,\n> +\t\t\t\t\t const char *buf, size_t len)\n> +{\n> +\tstruct ina2xx_data *data = dev_get_drvdata(dev);\n> +\tunsigned long val;\n> +\tint ret;\n> +\n> +\tret = kstrtoul((const char *) buf, 10, &val);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tret = set_max_expected_current(data, val);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\t/* Update the Calibration register */\n> +\tret = ina2xx_calibrate(data);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\treturn len;\n> +}\n> +\n> /* shunt voltage */\n> static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, ina2xx_show_value, NULL,\n> \t\t\t INA2XX_SHUNT_VOLTAGE);\n> @@ -390,6 +450,11 @@ static SENSOR_DEVICE_ATTR(shunt_resistor, S_IRUGO | S_IWUSR,\n> \t\t\t ina2xx_show_value, ina2xx_set_shunt,\n> \t\t\t INA2XX_CALIBRATION);\n> \n> +/* max expected current */\n> +static SENSOR_DEVICE_ATTR(max_expected_current, S_IRUGO | S_IWUSR,\n> +\t\t\t ina2xx_max_expected_current_show,\n> +\t\t\t ina2xx_max_expected_current_set, 0);\n> +\n> /* update interval (ina226 only) */\n> static SENSOR_DEVICE_ATTR(update_interval, S_IRUGO | S_IWUSR,\n> \t\t\t ina226_show_interval, ina226_set_interval, 0);\n> @@ -401,6 +466,7 @@ static struct attribute *ina2xx_attrs[] = {\n> \t&sensor_dev_attr_curr1_input.dev_attr.attr,\n> \t&sensor_dev_attr_power1_input.dev_attr.attr,\n> \t&sensor_dev_attr_shunt_resistor.dev_attr.attr,\n> +\t&sensor_dev_attr_max_expected_current.dev_attr.attr,\n\nWe do have standard attributes (presumably currX_max or possibly currX_crit).\nPlease use them, or explain in detail why you don't.\n\nGuenter\n\n> \tNULL,\n> };\n> \n> @@ -453,6 +519,21 @@ static int ina2xx_probe(struct i2c_client *client,\n> \n> \tdata->rshunt = val;\n> \n> +\tif (of_property_read_u32(dev->of_node, \"max-expected-current\",\n> +\t\t\t\t &val) < 0) {\n> +\t\tstruct ina2xx_platform_data *pdata =\n> +\t\t dev_get_platdata(&client->dev);\n> +\n> +\t\tif (pdata && pdata->max_mA != 0)\n> +\t\t\tval = pdata->max_mA;\n> +\t\telse\n> +\t\t\tval = INA2XX_MAX_EXPECTED_A_DEFAULT;\n> +\t}\n> +\n> +\tret = set_max_expected_current(data, val);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> \tina2xx_regmap_config.max_register = data->config->registers;\n> \n> \tdata->regmap = devm_regmap_init_i2c(client, &ina2xx_regmap_config);\n> \n\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"J9efJu6s\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2wSD1vqSz9tXd\n\tfor ;\n\tThu, 28 Sep 2017 23:26:04 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753149AbdI1N0C (ORCPT\n\t);\n\tThu, 28 Sep 2017 09:26:02 -0400","from mail-pg0-f67.google.com ([74.125.83.67]:36952 \"EHLO\n\tmail-pg0-f67.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753010AbdI1NZ6 (ORCPT\n\t); Thu, 28 Sep 2017 09:25:58 -0400","by mail-pg0-f67.google.com with SMTP id 125so1734451pgj.4;\n\tThu, 28 Sep 2017 06:25:58 -0700 (PDT)","from server.roeck-us.net\n\t(108-223-40-66.lightspeed.sntcca.sbcglobal.net. [108.223.40.66])\n\tby smtp.gmail.com with ESMTPSA id\n\tk25sm2778958pgf.13.2017.09.28.06.25.55\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 28 Sep 2017 06:25:56 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=sender:subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=uFe7Yc5CEbTFIX9DhZ/sHVGgxUr4d3R8KPTk9Jxnz5o=;\n\tb=J9efJu6ssxhkP3YGF/nD5G/rTQ8J3wEgaFXFLiGXGZ4Wt6B8niTDqD1GnAtC1J0wc9\n\thlumgnkB69pjzWl4o84dq7/3fPbeAypfDSxZRxi2HOmaKm2LBTkmEa7ovvG+BVSI1DAb\n\tqnL5lavwIPJ7S+B/+/zKVs4ZNZtfwniVuiq0q3dxMR/ucf4wh2vdOkLltufMTsUr99F6\n\txHvGSXEmqJCmpxEs14KgRmyAIw1TMcuxsIoSqwi8Ef3c9+/FhwyDuFj00UpCDpGEvNUH\n\tYQ/fYfCXokRnmG6h48ZTKE5LackLsDKdTh0KqcL9B7xCLL2uHf/1aWGPFpuvBokkB6pi\n\tVmNw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:sender:subject:to:cc:references:from:message-id\n\t:date:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=uFe7Yc5CEbTFIX9DhZ/sHVGgxUr4d3R8KPTk9Jxnz5o=;\n\tb=XRzfjdFJ4jJBPlrPxbWdZTUidUJnwEb3NqE0wJ4AterWNtjwdQMKFVyy/fAyj+mImy\n\tW/ge5A7VvMfUmZjLq4OzeQQaMhr964JP1RvcZO8kiByuXgUmxULYZuxFIlX+uzjw0iMD\n\tWiI3xr+4BIip7rtRJ2JE2QpIKZX41GMje0vobpiG+38b12DG2aRnnJJ+MEbf/hwRSsvT\n\tez8zeVNNV//IZKPNuikMJk3lNsimcnVU479dAwaX/hxmj6j0xkRHAlRTHldPoe2eJgNS\n\tmm4MkIsbPFt+vDIweu5Z0ZNvXU+HjoEYtVqte98HF8mAdQ0DOJHGwTXodFzV5dy4tQkf\n\tTjag==","X-Gm-Message-State":"AHPjjUjgR8V96Omho7WuDUyNv2Lx4nIjJ2goJ7gvcgdXiHQLraqP4Z+z\n\tdK+KMi1fv8G64CKSh9zDsNE=","X-Google-Smtp-Source":"AOwi7QAYitjgL6yh3FvYnuPDUBm51afuh/vHQmTMltbh42KD+rZ+bHZMfXaKMmiPlKbb7/wFv5z6rw==","X-Received":"by 10.98.78.203 with SMTP id c194mr4429432pfb.152.1506605157913; \n\tThu, 28 Sep 2017 06:25:57 -0700 (PDT)","Subject":"Re: [PATCH 2/4] hwmon: (ina2xx) Make max expected current\n\tconfigurable","To":"Maciej Purski , devicetree@vger.kernel.org,\n\tlinux-hwmon@vger.kernel.org, linux-doc@vger.kernel.org,\n\tlinux-arm-kernel@lists.infradead.org,\n\tlinux-samsung-soc@vger.kernel.org, linux-iio@vger.kernel.org","Cc":"Rob Herring , Mark Rutland ,\n\tJean Delvare , Jonathan Corbet ,\n\tRussell King , Kukjin Kim , \n\tKrzysztof Kozlowski ,\n\tJonathan Cameron , \n\tHartmut Knaack , Lars-Peter Clausen ,\n\tPeter Meerwald-Stadler ,\n\tBartlomiej Zolnierkiewicz ,\n\tMarek Szyprowski ","References":"<1506603015-27202-1-git-send-email-m.purski@samsung.com>\n\t\n\t<1506603015-27202-3-git-send-email-m.purski@samsung.com>","From":"Guenter Roeck ","Message-ID":"<2c2de1bb-1396-6d1c-efb0-3411c40f638c@roeck-us.net>","Date":"Thu, 28 Sep 2017 06:25:54 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<1506603015-27202-3-git-send-email-m.purski@samsung.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1778004,"web_url":"http://patchwork.ozlabs.org/comment/1778004/","msgid":"<20171001112934.360c840b@archlinux>","list_archive_url":null,"date":"2017-10-01T10:29:34","subject":"Re: [PATCH 1/4] iio: adc: ina2xx: Make max expected current\n\tconfigurable","submitter":{"id":10151,"url":"http://patchwork.ozlabs.org/api/people/10151/","name":"Jonathan Cameron","email":"jic23@kernel.org"},"content":"On Thu, 28 Sep 2017 14:50:12 +0200\nMaciej Purski wrote:\n\n> Max expected current is used for calculating calibration register value,\n> Current LSB and Power LSB according to equations found in ina datasheet.\n> Max expected current is now implicitly set to default value,\n> which is 2^15, thanks to which Current LSB is equal to 1 mA and\n> Power LSB is equal to 20000 uW or 25000 uW depending on ina model.\n> \n> Make max expected current configurable, just like it's already done\n> with shunt resistance: from device tree, platform_data or later\n> from sysfs. On each max_expected_current change, calculate new values\n> for Current LSB and Power LSB. According to datasheet Current LSB should\n> be calculated by dividing max expected current by 2^15, as values read\n> from device registers are in this case 16-bit integers. Power LSB\n> is calculated by multiplying Current LSB by a factor, which is defined\n> in ina documentation.\n\nOne odd bit of casting inline. Also this is new userspace ABI.\nIt needs documenting in \n\nDocumentation/ABI/testing/sysfs-bus-iio* as appropriate.\nI'm also unclear on one element about this - is it a value used only\nfor calibration or are we talking about the actual 'range' of the device?\n\nThe interpretation of this value isn't clear against the more general\nABI.\n\nIn particular it is it in raw units (adc counts) or mA? Docs say\nthat but the naming of the attribute doesn't make this clear.\n\nAlso I'm unconvinced this isn't better represented using the\nrange specifications available for any IIO attribute on the raw\nvalue in combination with adjusting the scale value.\nNote not many drivers yet provide ranges on their raw outputs\nbut we do have core support for it. I've been meaning to start\npushing this out into drivers, but been busy since we introduced\nthe core support. The dpot-dac driver does use it for examplel\n\nThis moves the burden of calculating the 1lsb value to userspace,\nbut importantly it would give us a consistent ABI where this fits\nin with existing elements (largely buy not introducing any new\nones :).\n\nThanks,\n\nJonathan\n> \n> Signed-off-by: Maciej Purski \n> ---\n> drivers/iio/adc/ina2xx-adc.c | 110 ++++++++++++++++++++++++++++++-----\n> include/linux/platform_data/ina2xx.h | 2 +\n> 2 files changed, 98 insertions(+), 14 deletions(-)\n> \n> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c\n> index f387b97..883fede 100644\n> --- a/drivers/iio/adc/ina2xx-adc.c\n> +++ b/drivers/iio/adc/ina2xx-adc.c\n> @@ -56,6 +56,7 @@\n> #define INA226_DEFAULT_IT\t\t1110\n> \n> #define INA2XX_RSHUNT_DEFAULT 10000\n> +#define INA2XX_MAX_EXPECTED_A_DEFAULT\t(1 << 15)\t/* current_lsb = 1 mA */\n> \n> /*\n> * bit masks for reading the settings in the configuration register\n> @@ -114,7 +115,7 @@ struct ina2xx_config {\n> \tint shunt_div;\n> \tint bus_voltage_shift;\n> \tint bus_voltage_lsb;\t/* uV */\n> -\tint power_lsb;\t\t/* uW */\n> +\tint power_lsb_factor;\n> \tenum ina2xx_ids chip_id;\n> };\n> \n> @@ -123,7 +124,10 @@ struct ina2xx_chip_info {\n> \tstruct task_struct *task;\n> \tconst struct ina2xx_config *config;\n> \tstruct mutex state_lock;\n> -\tunsigned int shunt_resistor;\n> +\tunsigned int shunt_resistor;\t\t/* uOhms */\n> +\tunsigned int max_expected_current;\t/* mA */\n> +\tint current_lsb;\t\t\t/* uA */\n> +\tint power_lsb;\t\t\t\t/* uW */\n> \tint avg;\n> \tint int_time_vbus; /* Bus voltage integration time uS */\n> \tint int_time_vshunt; /* Shunt voltage integration time uS */\n> @@ -137,7 +141,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n> \t\t.shunt_div = 100,\n> \t\t.bus_voltage_shift = 3,\n> \t\t.bus_voltage_lsb = 4000,\n> -\t\t.power_lsb = 20000,\n> +\t\t.power_lsb_factor = 20,\n> \t\t.chip_id = ina219,\n> \t},\n> \t[ina226] = {\n> @@ -146,7 +150,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n> \t\t.shunt_div = 400,\n> \t\t.bus_voltage_shift = 0,\n> \t\t.bus_voltage_lsb = 1250,\n> -\t\t.power_lsb = 25000,\n> +\t\t.power_lsb_factor = 25,\n> \t\t.chip_id = ina226,\n> \t},\n> };\n> @@ -210,14 +214,15 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,\n> \n> \t\tcase INA2XX_POWER:\n> \t\t\t/* processed (mW) = raw*lsb (uW) / 1000 */\n> -\t\t\t*val = chip->config->power_lsb;\n> +\t\t\t*val = chip->power_lsb;\n> \t\t\t*val2 = 1000;\n> \t\t\treturn IIO_VAL_FRACTIONAL;\n> \n> \t\tcase INA2XX_CURRENT:\n> -\t\t\t/* processed (mA) = raw (mA) */\n> -\t\t\t*val = 1;\n> -\t\t\treturn IIO_VAL_INT;\n> +\t\t\t/* processed (mA) = raw*lsb (uA) / 1000 */\n> +\t\t\t*val = chip->current_lsb;\n> +\t\t\t*val2 = 1000;\n> +\t\t\treturn IIO_VAL_FRACTIONAL;\n> \t\t}\n> \t}\n> \n> @@ -434,24 +439,47 @@ static ssize_t ina2xx_allow_async_readout_store(struct device *dev,\n> }\n> \n> /*\n> - * Set current LSB to 1mA, shunt is in uOhms\n> - * (equation 13 in datasheet). We hardcode a Current_LSB\n> - * of 1.0 x10-6. The only remaining parameter is RShunt.\n> + * Calculate calibration value according to equation 1 in ina226 datasheet\n> + * http://www.ti.com/lit/ds/symlink/ina226.pdf.\n> + * Current LSB is in uA and RShunt is in uOhms, so RShunt should be\n> + * converted to mOhms in order to keep the scale.\n> * There is no need to expose the CALIBRATION register\n> * to the user for now. But we need to reset this register\n> - * if the user updates RShunt after driver init, e.g upon\n> - * reading an EEPROM/Probe-type value.\n> + * if the user updates RShunt or max expected current after driver\n> + * init, e.g upon reading an EEPROM/Probe-type value.\n> */\n> static int ina2xx_set_calibration(struct ina2xx_chip_info *chip)\n> {\n> +\tunsigned int rshunt = DIV_ROUND_CLOSEST(chip->shunt_resistor, 1000);\n> \tu16 regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,\n> -\t\t\t\t chip->shunt_resistor);\n> +\t\t\t\t chip->current_lsb * rshunt);\n> \n> \treturn regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);\n> }\n> \n> +/*\n> + * Set max_expected_current (mA) and calculate current_lsb (uA),\n> + * according to equation 2 in ina226 datasheet. Power LSB is calculated\n> + * by multiplying Current LSB by a given factor, which may vary depending\n> + * on ina version.\n> + */\n> +static int set_max_expected_current(struct ina2xx_chip_info *chip,\n> +\t\t\t\t unsigned int val)\n> +{\n> +\tif (val <= 0 || val > chip->config->calibration_factor)\n> +\t\treturn -EINVAL;\n> +\n> +\tchip->max_expected_current = val;\n> +\tchip->current_lsb = DIV_ROUND_CLOSEST(chip->max_expected_current * 1000,\n> +\t\t\t\t\t 1 << 15);\n> +\tchip->power_lsb = chip->current_lsb * chip->config->power_lsb_factor;\n> +\n> +\treturn 0;\n> +}\n> +\n> static int set_shunt_resistor(struct ina2xx_chip_info *chip, unsigned int val)\n> {\n> +\n> \tif (val <= 0 || val > chip->config->calibration_factor)\n> \t\treturn -EINVAL;\n> \n> @@ -493,6 +521,39 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,\n> \treturn len;\n> }\n> \n> +static ssize_t ina2xx_max_expected_current_show(struct device *dev,\n> +\t\t\t\t\t struct device_attribute *attr,\n> +\t\t\t\t\t char *buf)\n> +{\n> +\tstruct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));\n> +\n> +\treturn sprintf(buf, \"%d\\n\", chip->max_expected_current);\n> +}\n> +\n> +static ssize_t ina2xx_max_expected_current_store(struct device *dev,\n> +\t\t\t\t\t struct device_attribute *attr,\n> +\t\t\t\t\t const char *buf, size_t len)\n> +{\n> +\tstruct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));\n> +\tunsigned long val;\n> +\tint ret;\n> +\n> +\tret = kstrtoul((const char *) buf, 10, &val);\n\nOdd bit of casting given that's what it already is...\n\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tret = set_max_expected_current(chip, val);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\t/* Update the Calibration register */\n> +\tret = ina2xx_set_calibration(chip);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\treturn len;\n> +}\n> +\n> #define INA219_CHAN(_type, _index, _address) { \\\n> \t.type = (_type), \\\n> \t.address = (_address), \\\n> @@ -755,10 +816,15 @@ static IIO_DEVICE_ATTR(in_shunt_resistor, S_IRUGO | S_IWUSR,\n> \t\t ina2xx_shunt_resistor_show,\n> \t\t ina2xx_shunt_resistor_store, 0);\n> \n> +static IIO_DEVICE_ATTR(in_max_expected_current, S_IRUGO | S_IWUSR,\n> +\t\t ina2xx_max_expected_current_show,\n> +\t\t ina2xx_max_expected_current_store, 0);\n> +\n> static struct attribute *ina219_attributes[] = {\n> \t&iio_dev_attr_in_allow_async_readout.dev_attr.attr,\n> \t&iio_const_attr_ina219_integration_time_available.dev_attr.attr,\n> \t&iio_dev_attr_in_shunt_resistor.dev_attr.attr,\n> +\t&iio_dev_attr_in_max_expected_current.dev_attr.attr,\n> \tNULL,\n> };\n> \n> @@ -766,6 +832,7 @@ static struct attribute *ina226_attributes[] = {\n> \t&iio_dev_attr_in_allow_async_readout.dev_attr.attr,\n> \t&iio_const_attr_ina226_integration_time_available.dev_attr.attr,\n> \t&iio_dev_attr_in_shunt_resistor.dev_attr.attr,\n> +\t&iio_dev_attr_in_max_expected_current.dev_attr.attr,\n> \tNULL,\n> };\n> \n> @@ -851,6 +918,21 @@ static int ina2xx_probe(struct i2c_client *client,\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\tif (of_property_read_u32(client->dev.of_node,\n> +\t\t\t\t \"max-expected-current\", &val) < 0) {\n> +\t\tstruct ina2xx_platform_data *pdata =\n> +\t\t dev_get_platdata(&client->dev);\n> +\n> +\t\tif (pdata && pdata->max_mA != 0)\n> +\t\t\tval = pdata->max_mA;\n> +\t\telse\n> +\t\t\tval = INA2XX_MAX_EXPECTED_A_DEFAULT;\n> +\t}\n> +\n> +\tret = set_max_expected_current(chip, val);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> \t/* Patch the current config register with default. */\n> \tval = chip->config->config_default;\n> \n> diff --git a/include/linux/platform_data/ina2xx.h b/include/linux/platform_data/ina2xx.h\n> index 9abc0ca..f02b1d8 100644\n> --- a/include/linux/platform_data/ina2xx.h\n> +++ b/include/linux/platform_data/ina2xx.h\n> @@ -13,7 +13,9 @@\n> /**\n> * struct ina2xx_platform_data - ina2xx info\n> * @shunt_uohms\t\tshunt resistance in microohms\n> + * @max_mA\t\tmax expected current in mA\n> */\n> struct ina2xx_platform_data {\n> \tlong shunt_uohms;\n> +\tint max_mA;\n> };\n\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","mail.kernel.org;\n\tdmarc=none (p=none dis=none) header.from=kernel.org","mail.kernel.org;\n\tspf=none smtp.mailfrom=jic23@kernel.org"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y4hPN70kzz9t2S\n\tfor ;\n\tSun, 1 Oct 2017 21:29:44 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751020AbdJAK3m (ORCPT\n\t);\n\tSun, 1 Oct 2017 06:29:42 -0400","from mail.kernel.org ([198.145.29.99]:44974 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751005AbdJAK3k (ORCPT );\n\tSun, 1 Oct 2017 06:29:40 -0400","from archlinux (cpc91196-cmbg18-2-0-cust659.5-4.cable.virginm.net\n\t[81.96.234.148])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby mail.kernel.org (Postfix) with ESMTPSA id C65F8218CC;\n\tSun, 1 Oct 2017 10:29:36 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org C65F8218CC","Date":"Sun, 1 Oct 2017 11:29:34 +0100","From":"Jonathan Cameron ","To":"Maciej Purski ","Cc":"devicetree@vger.kernel.org, linux-hwmon@vger.kernel.org,\n\tlinux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,\n\tlinux-samsung-soc@vger.kernel.org, linux-iio@vger.kernel.org,\n\tRob Herring , Mark Rutland ,\n\tGuenter Roeck , Jean Delvare , \n\tJonathan Corbet , Russell King ,\n\tKukjin Kim , Krzysztof Kozlowski , \n\tHartmut Knaack , Lars-Peter Clausen ,\n\tPeter Meerwald-Stadler ,\n\tBartlomiej Zolnierkiewicz ,\n\tMarek Szyprowski ","Subject":"Re: [PATCH 1/4] iio: adc: ina2xx: Make max expected current\n\tconfigurable","Message-ID":"<20171001112934.360c840b@archlinux>","In-Reply-To":"<1506603015-27202-2-git-send-email-m.purski@samsung.com>","References":"<1506603015-27202-1-git-send-email-m.purski@samsung.com>\n\t\n\t<1506603015-27202-2-git-send-email-m.purski@samsung.com>","X-Mailer":"Claws Mail 3.15.1-dirty (GTK+ 2.24.31; x86_64-pc-linux-gnu)","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1779477,"web_url":"http://patchwork.ozlabs.org/comment/1779477/","msgid":"","list_archive_url":null,"date":"2017-10-04T07:11:31","subject":"Re: [PATCH 1/4] iio: adc: ina2xx: Make max expected current\n\tconfigurable","submitter":{"id":72100,"url":"http://patchwork.ozlabs.org/api/people/72100/","name":"Maciej Purski","email":"m.purski@samsung.com"},"content":"On 10/01/2017 12:29 PM, Jonathan Cameron wrote:\n> On Thu, 28 Sep 2017 14:50:12 +0200\n> Maciej Purski wrote:\n> \n>> Max expected current is used for calculating calibration register value,\n>> Current LSB and Power LSB according to equations found in ina datasheet.\n>> Max expected current is now implicitly set to default value,\n>> which is 2^15, thanks to which Current LSB is equal to 1 mA and\n>> Power LSB is equal to 20000 uW or 25000 uW depending on ina model.\n>>\n>> Make max expected current configurable, just like it's already done\n>> with shunt resistance: from device tree, platform_data or later\n>> from sysfs. On each max_expected_current change, calculate new values\n>> for Current LSB and Power LSB. According to datasheet Current LSB should\n>> be calculated by dividing max expected current by 2^15, as values read\n>> from device registers are in this case 16-bit integers. Power LSB\n>> is calculated by multiplying Current LSB by a factor, which is defined\n>> in ina documentation.\n> \n> One odd bit of casting inline. Also this is new userspace ABI.\n> It needs documenting in\n> \n> Documentation/ABI/testing/sysfs-bus-iio* as appropriate.\n> I'm also unclear on one element about this - is it a value used only\n> for calibration or are we talking about the actual 'range' of the device?\n> \n\nThis is used for calibration. The device measures directly only voltage.\nHowever, it has also current and power registers. Their values are\ncalculated by the device using the calibration register which is calculated\nusing max expected current. So I guess that it's not what you mean\nby the actual 'range' of the device.\n\n> The interpretation of this value isn't clear against the more general\n> ABI.\n> \n> In particular it is it in raw units (adc counts) or mA? Docs say\n> that but the naming of the attribute doesn't make this clear.\n> \n\nIt's in mA. I can make it clear in the attribute name.\n\n> Also I'm unconvinced this isn't better represented using the\n> range specifications available for any IIO attribute on the raw\n> value in combination with adjusting the scale value.\n> Note not many drivers yet provide ranges on their raw outputs\n> but we do have core support for it. I've been meaning to start\n> pushing this out into drivers, but been busy since we introduced\n> the core support. The dpot-dac driver does use it for examplel\n>\n\n\nI'm not sure if what I'm about to add is similar to what is done\nin the mentioned dpot-dac driver. It seems that the callback read_avail\nreturns information on raw values which can be obtained from the device.\nWhat I need is an adjustable value, which is then used by the device internally\nin order to calculate current with the requested precision. Max expected current\nis also used for calculating the scale value.\nTell me if I'm wrong. Maybe I misunderstood the 'range' concept in IIO and\nyour solution fits in here.\n\nBest regards,\n\n\tMaciej\n> This moves the burden of calculating the 1lsb value to userspace,\n> but importantly it would give us a consistent ABI where this fits\n> in with existing elements (largely buy not introducing any new\n> ones :).\n> \n> Thanks,\n> \n> Jonathan\n>>\n>> Signed-off-by: Maciej Purski \n>> ---\n>> drivers/iio/adc/ina2xx-adc.c | 110 ++++++++++++++++++++++++++++++-----\n>> include/linux/platform_data/ina2xx.h | 2 +\n>> 2 files changed, 98 insertions(+), 14 deletions(-)\n>>\n>> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c\n>> index f387b97..883fede 100644\n>> --- a/drivers/iio/adc/ina2xx-adc.c\n>> +++ b/drivers/iio/adc/ina2xx-adc.c\n>> @@ -56,6 +56,7 @@\n>> #define INA226_DEFAULT_IT\t\t1110\n>> \n>> #define INA2XX_RSHUNT_DEFAULT 10000\n>> +#define INA2XX_MAX_EXPECTED_A_DEFAULT\t(1 << 15)\t/* current_lsb = 1 mA */\n>> \n>> /*\n>> * bit masks for reading the settings in the configuration register\n>> @@ -114,7 +115,7 @@ struct ina2xx_config {\n>> \tint shunt_div;\n>> \tint bus_voltage_shift;\n>> \tint bus_voltage_lsb;\t/* uV */\n>> -\tint power_lsb;\t\t/* uW */\n>> +\tint power_lsb_factor;\n>> \tenum ina2xx_ids chip_id;\n>> };\n>> \n>> @@ -123,7 +124,10 @@ struct ina2xx_chip_info {\n>> \tstruct task_struct *task;\n>> \tconst struct ina2xx_config *config;\n>> \tstruct mutex state_lock;\n>> -\tunsigned int shunt_resistor;\n>> +\tunsigned int shunt_resistor;\t\t/* uOhms */\n>> +\tunsigned int max_expected_current;\t/* mA */\n>> +\tint current_lsb;\t\t\t/* uA */\n>> +\tint power_lsb;\t\t\t\t/* uW */\n>> \tint avg;\n>> \tint int_time_vbus; /* Bus voltage integration time uS */\n>> \tint int_time_vshunt; /* Shunt voltage integration time uS */\n>> @@ -137,7 +141,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n>> \t\t.shunt_div = 100,\n>> \t\t.bus_voltage_shift = 3,\n>> \t\t.bus_voltage_lsb = 4000,\n>> -\t\t.power_lsb = 20000,\n>> +\t\t.power_lsb_factor = 20,\n>> \t\t.chip_id = ina219,\n>> \t},\n>> \t[ina226] = {\n>> @@ -146,7 +150,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n>> \t\t.shunt_div = 400,\n>> \t\t.bus_voltage_shift = 0,\n>> \t\t.bus_voltage_lsb = 1250,\n>> -\t\t.power_lsb = 25000,\n>> +\t\t.power_lsb_factor = 25,\n>> \t\t.chip_id = ina226,\n>> \t},\n>> };\n>> @@ -210,14 +214,15 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,\n>> \n>> \t\tcase INA2XX_POWER:\n>> \t\t\t/* processed (mW) = raw*lsb (uW) / 1000 */\n>> -\t\t\t*val = chip->config->power_lsb;\n>> +\t\t\t*val = chip->power_lsb;\n>> \t\t\t*val2 = 1000;\n>> \t\t\treturn IIO_VAL_FRACTIONAL;\n>> \n>> \t\tcase INA2XX_CURRENT:\n>> -\t\t\t/* processed (mA) = raw (mA) */\n>> -\t\t\t*val = 1;\n>> -\t\t\treturn IIO_VAL_INT;\n>> +\t\t\t/* processed (mA) = raw*lsb (uA) / 1000 */\n>> +\t\t\t*val = chip->current_lsb;\n>> +\t\t\t*val2 = 1000;\n>> +\t\t\treturn IIO_VAL_FRACTIONAL;\n>> \t\t}\n>> \t}\n>> \n>> @@ -434,24 +439,47 @@ static ssize_t ina2xx_allow_async_readout_store(struct device *dev,\n>> }\n>> \n>> /*\n>> - * Set current LSB to 1mA, shunt is in uOhms\n>> - * (equation 13 in datasheet). We hardcode a Current_LSB\n>> - * of 1.0 x10-6. The only remaining parameter is RShunt.\n>> + * Calculate calibration value according to equation 1 in ina226 datasheet\n>> + * http://www.ti.com/lit/ds/symlink/ina226.pdf.\n>> + * Current LSB is in uA and RShunt is in uOhms, so RShunt should be\n>> + * converted to mOhms in order to keep the scale.\n>> * There is no need to expose the CALIBRATION register\n>> * to the user for now. But we need to reset this register\n>> - * if the user updates RShunt after driver init, e.g upon\n>> - * reading an EEPROM/Probe-type value.\n>> + * if the user updates RShunt or max expected current after driver\n>> + * init, e.g upon reading an EEPROM/Probe-type value.\n>> */\n>> static int ina2xx_set_calibration(struct ina2xx_chip_info *chip)\n>> {\n>> +\tunsigned int rshunt = DIV_ROUND_CLOSEST(chip->shunt_resistor, 1000);\n>> \tu16 regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,\n>> -\t\t\t\t chip->shunt_resistor);\n>> +\t\t\t\t chip->current_lsb * rshunt);\n>> \n>> \treturn regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);\n>> }\n>> \n>> +/*\n>> + * Set max_expected_current (mA) and calculate current_lsb (uA),\n>> + * according to equation 2 in ina226 datasheet. Power LSB is calculated\n>> + * by multiplying Current LSB by a given factor, which may vary depending\n>> + * on ina version.\n>> + */\n>> +static int set_max_expected_current(struct ina2xx_chip_info *chip,\n>> +\t\t\t\t unsigned int val)\n>> +{\n>> +\tif (val <= 0 || val > chip->config->calibration_factor)\n>> +\t\treturn -EINVAL;\n>> +\n>> +\tchip->max_expected_current = val;\n>> +\tchip->current_lsb = DIV_ROUND_CLOSEST(chip->max_expected_current * 1000,\n>> +\t\t\t\t\t 1 << 15);\n>> +\tchip->power_lsb = chip->current_lsb * chip->config->power_lsb_factor;\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> static int set_shunt_resistor(struct ina2xx_chip_info *chip, unsigned int val)\n>> {\n>> +\n>> \tif (val <= 0 || val > chip->config->calibration_factor)\n>> \t\treturn -EINVAL;\n>> \n>> @@ -493,6 +521,39 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,\n>> \treturn len;\n>> }\n>> \n>> +static ssize_t ina2xx_max_expected_current_show(struct device *dev,\n>> +\t\t\t\t\t struct device_attribute *attr,\n>> +\t\t\t\t\t char *buf)\n>> +{\n>> +\tstruct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));\n>> +\n>> +\treturn sprintf(buf, \"%d\\n\", chip->max_expected_current);\n>> +}\n>> +\n>> +static ssize_t ina2xx_max_expected_current_store(struct device *dev,\n>> +\t\t\t\t\t struct device_attribute *attr,\n>> +\t\t\t\t\t const char *buf, size_t len)\n>> +{\n>> +\tstruct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));\n>> +\tunsigned long val;\n>> +\tint ret;\n>> +\n>> +\tret = kstrtoul((const char *) buf, 10, &val);\n> \n> Odd bit of casting given that's what it already is...\n> \n>> +\tif (ret)\n>> +\t\treturn ret;\n>> +\n>> +\tret = set_max_expected_current(chip, val);\n>> +\tif (ret)\n>> +\t\treturn ret;\n>> +\n>> +\t/* Update the Calibration register */\n>> +\tret = ina2xx_set_calibration(chip);\n>> +\tif (ret)\n>> +\t\treturn ret;\n>> +\n>> +\treturn len;\n>> +}\n>> +\n>> #define INA219_CHAN(_type, _index, _address) { \\\n>> \t.type = (_type), \\\n>> \t.address = (_address), \\\n>> @@ -755,10 +816,15 @@ static IIO_DEVICE_ATTR(in_shunt_resistor, S_IRUGO | S_IWUSR,\n>> \t\t ina2xx_shunt_resistor_show,\n>> \t\t ina2xx_shunt_resistor_store, 0);\n>> \n>> +static IIO_DEVICE_ATTR(in_max_expected_current, S_IRUGO | S_IWUSR,\n>> +\t\t ina2xx_max_expected_current_show,\n>> +\t\t ina2xx_max_expected_current_store, 0);\n>> +\n>> static struct attribute *ina219_attributes[] = {\n>> \t&iio_dev_attr_in_allow_async_readout.dev_attr.attr,\n>> \t&iio_const_attr_ina219_integration_time_available.dev_attr.attr,\n>> \t&iio_dev_attr_in_shunt_resistor.dev_attr.attr,\n>> +\t&iio_dev_attr_in_max_expected_current.dev_attr.attr,\n>> \tNULL,\n>> };\n>> \n>> @@ -766,6 +832,7 @@ static struct attribute *ina226_attributes[] = {\n>> \t&iio_dev_attr_in_allow_async_readout.dev_attr.attr,\n>> \t&iio_const_attr_ina226_integration_time_available.dev_attr.attr,\n>> \t&iio_dev_attr_in_shunt_resistor.dev_attr.attr,\n>> +\t&iio_dev_attr_in_max_expected_current.dev_attr.attr,\n>> \tNULL,\n>> };\n>> \n>> @@ -851,6 +918,21 @@ static int ina2xx_probe(struct i2c_client *client,\n>> \tif (ret)\n>> \t\treturn ret;\n>> \n>> +\tif (of_property_read_u32(client->dev.of_node,\n>> +\t\t\t\t \"max-expected-current\", &val) < 0) {\n>> +\t\tstruct ina2xx_platform_data *pdata =\n>> +\t\t dev_get_platdata(&client->dev);\n>> +\n>> +\t\tif (pdata && pdata->max_mA != 0)\n>> +\t\t\tval = pdata->max_mA;\n>> +\t\telse\n>> +\t\t\tval = INA2XX_MAX_EXPECTED_A_DEFAULT;\n>> +\t}\n>> +\n>> +\tret = set_max_expected_current(chip, val);\n>> +\tif (ret)\n>> +\t\treturn ret;\n>> +\n>> \t/* Patch the current config register with default. */\n>> \tval = chip->config->config_default;\n>> \n>> diff --git a/include/linux/platform_data/ina2xx.h b/include/linux/platform_data/ina2xx.h\n>> index 9abc0ca..f02b1d8 100644\n>> --- a/include/linux/platform_data/ina2xx.h\n>> +++ b/include/linux/platform_data/ina2xx.h\n>> @@ -13,7 +13,9 @@\n>> /**\n>> * struct ina2xx_platform_data - ina2xx info\n>> * @shunt_uohms\t\tshunt resistance in microohms\n>> + * @max_mA\t\tmax expected current in mA\n>> */\n>> struct ina2xx_platform_data {\n>> \tlong shunt_uohms;\n>> +\tint max_mA;\n>> };\n> \n> \n> \n> \n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y6Rsd6YPdz9sP1\n\tfor ;\n\tWed, 4 Oct 2017 18:11:49 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751156AbdJDHLr (ORCPT\n\t);\n\tWed, 4 Oct 2017 03:11:47 -0400","from mailout2.w1.samsung.com ([210.118.77.12]:58456 \"EHLO\n\tmailout2.w1.samsung.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750945AbdJDHLp (ORCPT\n\t); Wed, 4 Oct 2017 03:11:45 -0400","from eucas1p2.samsung.com (unknown [182.198.249.207])\n\tby mailout2.w1.samsung.com (KnoxPortal) with ESMTP id\n\t20171004071142euoutp020d207ba6e5c6f66d0384b086cc6a87d7~qTHfMsSz90759107591euoutp02U;\n\tWed, 4 Oct 2017 07:11:42 +0000 (GMT)","from eusmges2.samsung.com (unknown [203.254.199.241]) by\n\teucas1p1.samsung.com (KnoxPortal) with ESMTP id\n\t20171004071141eucas1p14cab42b8061ec7252ab9f94012ef5076~qTHebbIpy2603826038eucas1p1r;\n\tWed, 4 Oct 2017 07:11:41 +0000 (GMT)","from eucas1p2.samsung.com ( [182.198.249.207]) by\n\teusmges2.samsung.com (EUCPMTA) with SMTP id F4.99.12907.DA984D95;\n\tWed, 4 Oct 2017 08:11:41 +0100 (BST)","from eusmgms2.samsung.com (unknown [182.198.249.180]) by\n\teucas1p1.samsung.com (KnoxPortal) with ESMTP id\n\t20171004071141eucas1p1f196b7b6b1c101b6b3f21504aa688aee~qTHduKHAt2071720717eucas1p1V;\n\tWed, 4 Oct 2017 07:11:41 +0000 (GMT)","from eusync1.samsung.com ( [203.254.199.211]) by\n\teusmgms2.samsung.com (EUCPMTA) with SMTP id E4.AC.20118.DA984D95;\n\tWed, 4 Oct 2017 08:11:41 +0100 (BST)","from [106.120.51.25] by eusync1.samsung.com (Oracle Communications\n\tMessaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTPA\n\tid <0OXA0021ZEN8EZ80@eusync1.samsung.com>;\n\tWed, 04 Oct 2017 08:11:41 +0100 (BST)"],"X-AuditID":"cbfec7f1-f793a6d00000326b-b9-59d489ad282d","Subject":"Re: [PATCH 1/4] iio: adc: ina2xx: Make max expected current\n\tconfigurable","To":"Jonathan Cameron ","Cc":"devicetree@vger.kernel.org, linux-hwmon@vger.kernel.org,\n\tlinux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,\n\tlinux-samsung-soc@vger.kernel.org, linux-iio@vger.kernel.org,\n\tRob Herring , Mark Rutland ,\n\tGuenter Roeck , Jean Delvare , \n\tJonathan Corbet , Russell King ,\n\tKukjin Kim , Krzysztof Kozlowski , \n\tHartmut Knaack , Lars-Peter Clausen ,\n\tPeter Meerwald-Stadler ,\n\tBartlomiej Zolnierkiewicz ,\n\tMarek Szyprowski ","From":"Maciej Purski ","Message-id":"","Date":"Wed, 04 Oct 2017 09:11:31 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-version":"1.0","In-reply-to":"<20171001112934.360c840b@archlinux>","Content-type":"text/plain; charset=\"utf-8\"; format=\"flowed\"","Content-language":"en-US","Content-transfer-encoding":"7bit","X-Brightmail-Tracker":["H4sIAAAAAAAAA02Sa0iTURjHO+9lezUXp2X1pGK1kMjSSqxOZWEX4qUv3T6Udl36YpE39qZk\n\tBs1k8xKYOSxb0FreYk20tcwsL4yhktY07YYlgWJlFzPRtMByvhP89nvO8zuc5/9wOFo5wfpx\n\tpxPPCppEdbxK5s1UN427QipyuqLW1DYEkftFlSzpa8xCxOR8wZLn+fvIx0sWilzp/UqT2n/f\n\taOJyVclJicHEElvva5aY9SUMyfr6EJFbzh8MKXLVU8RRWIdIn7mNIhXOD3JS+qaDIn9rm+RE\n\tV+eUR/ry1ltWxHe+7qD5n0PHeJslR8Y/KLnIF+cZWL7km4PlXZlOxD/+rZXzeXYL4ivtrxh+\n\t2Ba41yfaOyJWiD+dKmhWbz3hfWqw3CVP1kWfayxuZbWobVcu8uIAh8PEjU+MxAugvadSlou8\n\tOSUuRWC29lDuhhIPI6grnDN9oW+ggJakMgTdTc2sVPQj+G65M3VjHj4AdeYm5GZfvByuFXVR\n\tbonG9Sz03LNOFhwnw8FgzTrudhR4K3SPmGk3MzgILjdapkaajw9Bu22ckpy5MGbomTr3wmvg\n\tjzGLdTONN0H/hM7DCyFT946ReDE8sH6fmhTwFznYC7tkUoSdUOn4IZd4Hgw02z0cAJ2Gy55d\n\tpEPHSK3HvwAZ3VUeZzMMF9hp6YE5UFB9nXZnAayAbL1SUnjIKy2gJN4GL5/2MtKCxhEYnhnY\n\tfLTYOCOPcUYG44wMxhkZbiPGgnyFFDEhThDDQkV1gpiSGBcak5RgQ5O/snWieagGDbZsciDM\n\tIZWPQpvXGaVk1aliWoIDAUerfBWjuq4opSJWnXZe0CQd16TEC6ID+XOMaqFiS7Q+Sonj1GeF\n\tM4KQLGimuxTn5adF4VdD9MXLRpOFtXH3EjtWRZ7stBcKTzKig0cehon+Y/ZjkS4Zsy5/vKyl\n\t5m7gYZO6PPZoNo5catlhers/uf5RwPolZVfeB69siJm/7OCOPdobbMTAkbGbjsBLGZ93z1ZW\n\t+PAbzCty2p80/9qJ0/TlEfj59u7W6+2LTOmqXZ/fz0rdqGLEU+q1wbRGVP8HbhZstpEDAAA=","H4sIAAAAAAAAA+NgFrrCIsWRmVeSWpSXmKPExsVy+t/xy7prO69EGvxZzWaxccZ6VosnB9oZ\n\tLeYfOcdqcXZCoMWDplVMFv2PXzNb7Pr/htni/PkN7BZLJs9ntdj0+BqrxcK2JSwW7a+3MlrM\n\tO/KOxWLG+X1MFoem7mW0eLLwDJPF2iN32S2WXr/IZPF71zF2i9a9R9gdRDzWzFvD6HH52kVm\n\tjw8f4zw2repk89i8pN5jcd9kVo8lbw6xepxvPsLosfN7A7tH35ZVjB7rt1xl8fi8SS6AJ4rL\n\tJiU1J7MstUjfLoEr4/3y8+wFrVEVBxafZm1gPOPWxcjJISFgIvHk1SRmCFtM4sK99WxdjFwc\n\tQgJLGCX6euazQjjPGCVa3yxgBakSFgiW2LvwGCOILSKgLjFtxhUmkCJmgQOsEvOOXmSH6PjJ\n\tKHHq6k4gh4ODTUBLYk17PEgDr4CdxO2vC8HWsQioSnQfWMUCUiIqECGxYSM/RImgxI/J91hA\n\tbE4BA4lfs9rB9jILmEl8eXkYyhaXaG69yQJhy0tsXvOWeQKj4Cwk7bOQtMxC0jILScsCRpZV\n\tjCKppcW56bnFRnrFibnFpXnpesn5uZsYgXG97djPLTsYu94FH2IU4GBU4uHV6LkcKcSaWFZc\n\tmXuIUYKDWUmE91vrlUgh3pTEyqrUovz4otKc1OJDjNIcLErivL17VkcKCaQnlqRmp6YWpBbB\n\tZJk4OKUaGFWffeOO/pXI5Bect2/qmihW5kyFJ9+zMwKmmL19emChye9QUzXVGzN2twaHnq87\n\t0/Yg/t+sgBZ5swJ+Nt9IU/7pbrdnFp755dawifWn6/WTLX0chx7r978T1q2/cefe+Y+mXYtm\n\tfmO1OiLbXXm5ozT2zZ/D2xvOz1kmNHcFy03OX2dL52rO2K/EUpyRaKjFXFScCABnmei45wIA\n\tAA=="],"X-CMS-MailID":"20171004071141eucas1p1f196b7b6b1c101b6b3f21504aa688aee","X-Msg-Generator":"CA","X-Sender-IP":"182.198.249.180","X-Local-Sender":"=?utf-8?q?Maciej_Purski=1BSecurity_=28TP=29=1BSamsung_Ele?=\n\t=?utf-8?q?ctronics=1BTrainee_=28=29?=","X-Global-Sender":"=?utf-8?q?Maciej_Purski=1BSecurity_=28TP=29=1BSamsung_El?=\n\t=?utf-8?q?ectronics=1BTrainee_=28=29?=","X-Sender-Code":"=?utf-8?q?C10=1BEHQ=1BC10CD02CD027395?=","CMS-TYPE":"201P","X-CMS-RootMailID":"20170928125053eucas1p19e757ac5c3774f54c7cd0ee4525decbe","X-RootMTR":"20170928125053eucas1p19e757ac5c3774f54c7cd0ee4525decbe","References":"<1506603015-27202-1-git-send-email-m.purski@samsung.com>\n\t\n\t<1506603015-27202-2-git-send-email-m.purski@samsung.com>\n\t<20171001112934.360c840b@archlinux>","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1782227,"web_url":"http://patchwork.ozlabs.org/comment/1782227/","msgid":"<20171008104756.6eaa62c5@archlinux>","list_archive_url":null,"date":"2017-10-08T09:47:56","subject":"Re: [PATCH 1/4] iio: adc: ina2xx: Make max expected current\n\tconfigurable","submitter":{"id":10151,"url":"http://patchwork.ozlabs.org/api/people/10151/","name":"Jonathan Cameron","email":"jic23@kernel.org"},"content":"On Wed, 04 Oct 2017 09:11:31 +0200\nMaciej Purski wrote:\n\n> On 10/01/2017 12:29 PM, Jonathan Cameron wrote:\n> > On Thu, 28 Sep 2017 14:50:12 +0200\n> > Maciej Purski wrote:\n> > \n> >> Max expected current is used for calculating calibration register value,\n> >> Current LSB and Power LSB according to equations found in ina datasheet.\n> >> Max expected current is now implicitly set to default value,\n> >> which is 2^15, thanks to which Current LSB is equal to 1 mA and\n> >> Power LSB is equal to 20000 uW or 25000 uW depending on ina model.\n> >>\n> >> Make max expected current configurable, just like it's already done\n> >> with shunt resistance: from device tree, platform_data or later\n> >> from sysfs. On each max_expected_current change, calculate new values\n> >> for Current LSB and Power LSB. According to datasheet Current LSB should\n> >> be calculated by dividing max expected current by 2^15, as values read\n> >> from device registers are in this case 16-bit integers. Power LSB\n> >> is calculated by multiplying Current LSB by a factor, which is defined\n> >> in ina documentation. \n> > \n> > One odd bit of casting inline. Also this is new userspace ABI.\n> > It needs documenting in\n> > \n> > Documentation/ABI/testing/sysfs-bus-iio* as appropriate.\n> > I'm also unclear on one element about this - is it a value used only\n> > for calibration or are we talking about the actual 'range' of the device?\n> > \n> \n> This is used for calibration. The device measures directly only voltage.\n> However, it has also current and power registers. Their values are\n> calculated by the device using the calibration register which is calculated\n> using max expected current. So I guess that it's not what you mean\n> by the actual 'range' of the device.\n> \n> > The interpretation of this value isn't clear against the more general\n> > ABI.\n> > \n> > In particular it is it in raw units (adc counts) or mA? Docs say\n> > that but the naming of the attribute doesn't make this clear.\n> > \n> \n> It's in mA. I can make it clear in the attribute name.\n> \n> > Also I'm unconvinced this isn't better represented using the\n> > range specifications available for any IIO attribute on the raw\n> > value in combination with adjusting the scale value.\n> > Note not many drivers yet provide ranges on their raw outputs\n> > but we do have core support for it. I've been meaning to start\n> > pushing this out into drivers, but been busy since we introduced\n> > the core support. The dpot-dac driver does use it for examplel\n> > \n> \n> \n> I'm not sure if what I'm about to add is similar to what is done\n> in the mentioned dpot-dac driver. It seems that the callback read_avail\n> returns information on raw values which can be obtained from the device.\n> What I need is an adjustable value, which is then used by the device internally\n> in order to calculate current with the requested precision. Max expected current\n> is also used for calculating the scale value.\n> Tell me if I'm wrong. Maybe I misunderstood the 'range' concept in IIO and\n> your solution fits in here.\n> \n\nI think I answered this in the other branch of the thread.\n_calibscale is what you want here as it's internal to the device.\n\nIt's not one often used for ADCs but quite a few other types of\ndevice provide some front end analog adjustment (whilst it is digital\nhere, it is applied within the device - so we don't need to care).\n\nJonathan\n\n> Best regards,\n> \n> \tMaciej\n> > This moves the burden of calculating the 1lsb value to userspace,\n> > but importantly it would give us a consistent ABI where this fits\n> > in with existing elements (largely buy not introducing any new\n> > ones :).\n> > \n> > Thanks,\n> > \n> > Jonathan \n> >>\n> >> Signed-off-by: Maciej Purski \n> >> ---\n> >> drivers/iio/adc/ina2xx-adc.c | 110 ++++++++++++++++++++++++++++++-----\n> >> include/linux/platform_data/ina2xx.h | 2 +\n> >> 2 files changed, 98 insertions(+), 14 deletions(-)\n> >>\n> >> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c\n> >> index f387b97..883fede 100644\n> >> --- a/drivers/iio/adc/ina2xx-adc.c\n> >> +++ b/drivers/iio/adc/ina2xx-adc.c\n> >> @@ -56,6 +56,7 @@\n> >> #define INA226_DEFAULT_IT\t\t1110\n> >> \n> >> #define INA2XX_RSHUNT_DEFAULT 10000\n> >> +#define INA2XX_MAX_EXPECTED_A_DEFAULT\t(1 << 15)\t/* current_lsb = 1 mA */\n> >> \n> >> /*\n> >> * bit masks for reading the settings in the configuration register\n> >> @@ -114,7 +115,7 @@ struct ina2xx_config {\n> >> \tint shunt_div;\n> >> \tint bus_voltage_shift;\n> >> \tint bus_voltage_lsb;\t/* uV */\n> >> -\tint power_lsb;\t\t/* uW */\n> >> +\tint power_lsb_factor;\n> >> \tenum ina2xx_ids chip_id;\n> >> };\n> >> \n> >> @@ -123,7 +124,10 @@ struct ina2xx_chip_info {\n> >> \tstruct task_struct *task;\n> >> \tconst struct ina2xx_config *config;\n> >> \tstruct mutex state_lock;\n> >> -\tunsigned int shunt_resistor;\n> >> +\tunsigned int shunt_resistor;\t\t/* uOhms */\n> >> +\tunsigned int max_expected_current;\t/* mA */\n> >> +\tint current_lsb;\t\t\t/* uA */\n> >> +\tint power_lsb;\t\t\t\t/* uW */\n> >> \tint avg;\n> >> \tint int_time_vbus; /* Bus voltage integration time uS */\n> >> \tint int_time_vshunt; /* Shunt voltage integration time uS */\n> >> @@ -137,7 +141,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n> >> \t\t.shunt_div = 100,\n> >> \t\t.bus_voltage_shift = 3,\n> >> \t\t.bus_voltage_lsb = 4000,\n> >> -\t\t.power_lsb = 20000,\n> >> +\t\t.power_lsb_factor = 20,\n> >> \t\t.chip_id = ina219,\n> >> \t},\n> >> \t[ina226] = {\n> >> @@ -146,7 +150,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n> >> \t\t.shunt_div = 400,\n> >> \t\t.bus_voltage_shift = 0,\n> >> \t\t.bus_voltage_lsb = 1250,\n> >> -\t\t.power_lsb = 25000,\n> >> +\t\t.power_lsb_factor = 25,\n> >> \t\t.chip_id = ina226,\n> >> \t},\n> >> };\n> >> @@ -210,14 +214,15 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,\n> >> \n> >> \t\tcase INA2XX_POWER:\n> >> \t\t\t/* processed (mW) = raw*lsb (uW) / 1000 */\n> >> -\t\t\t*val = chip->config->power_lsb;\n> >> +\t\t\t*val = chip->power_lsb;\n> >> \t\t\t*val2 = 1000;\n> >> \t\t\treturn IIO_VAL_FRACTIONAL;\n> >> \n> >> \t\tcase INA2XX_CURRENT:\n> >> -\t\t\t/* processed (mA) = raw (mA) */\n> >> -\t\t\t*val = 1;\n> >> -\t\t\treturn IIO_VAL_INT;\n> >> +\t\t\t/* processed (mA) = raw*lsb (uA) / 1000 */\n> >> +\t\t\t*val = chip->current_lsb;\n> >> +\t\t\t*val2 = 1000;\n> >> +\t\t\treturn IIO_VAL_FRACTIONAL;\n> >> \t\t}\n> >> \t}\n> >> \n> >> @@ -434,24 +439,47 @@ static ssize_t ina2xx_allow_async_readout_store(struct device *dev,\n> >> }\n> >> \n> >> /*\n> >> - * Set current LSB to 1mA, shunt is in uOhms\n> >> - * (equation 13 in datasheet). We hardcode a Current_LSB\n> >> - * of 1.0 x10-6. The only remaining parameter is RShunt.\n> >> + * Calculate calibration value according to equation 1 in ina226 datasheet\n> >> + * http://www.ti.com/lit/ds/symlink/ina226.pdf.\n> >> + * Current LSB is in uA and RShunt is in uOhms, so RShunt should be\n> >> + * converted to mOhms in order to keep the scale.\n> >> * There is no need to expose the CALIBRATION register\n> >> * to the user for now. But we need to reset this register\n> >> - * if the user updates RShunt after driver init, e.g upon\n> >> - * reading an EEPROM/Probe-type value.\n> >> + * if the user updates RShunt or max expected current after driver\n> >> + * init, e.g upon reading an EEPROM/Probe-type value.\n> >> */\n> >> static int ina2xx_set_calibration(struct ina2xx_chip_info *chip)\n> >> {\n> >> +\tunsigned int rshunt = DIV_ROUND_CLOSEST(chip->shunt_resistor, 1000);\n> >> \tu16 regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,\n> >> -\t\t\t\t chip->shunt_resistor);\n> >> +\t\t\t\t chip->current_lsb * rshunt);\n> >> \n> >> \treturn regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);\n> >> }\n> >> \n> >> +/*\n> >> + * Set max_expected_current (mA) and calculate current_lsb (uA),\n> >> + * according to equation 2 in ina226 datasheet. Power LSB is calculated\n> >> + * by multiplying Current LSB by a given factor, which may vary depending\n> >> + * on ina version.\n> >> + */\n> >> +static int set_max_expected_current(struct ina2xx_chip_info *chip,\n> >> +\t\t\t\t unsigned int val)\n> >> +{\n> >> +\tif (val <= 0 || val > chip->config->calibration_factor)\n> >> +\t\treturn -EINVAL;\n> >> +\n> >> +\tchip->max_expected_current = val;\n> >> +\tchip->current_lsb = DIV_ROUND_CLOSEST(chip->max_expected_current * 1000,\n> >> +\t\t\t\t\t 1 << 15);\n> >> +\tchip->power_lsb = chip->current_lsb * chip->config->power_lsb_factor;\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> static int set_shunt_resistor(struct ina2xx_chip_info *chip, unsigned int val)\n> >> {\n> >> +\n> >> \tif (val <= 0 || val > chip->config->calibration_factor)\n> >> \t\treturn -EINVAL;\n> >> \n> >> @@ -493,6 +521,39 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,\n> >> \treturn len;\n> >> }\n> >> \n> >> +static ssize_t ina2xx_max_expected_current_show(struct device *dev,\n> >> +\t\t\t\t\t struct device_attribute *attr,\n> >> +\t\t\t\t\t char *buf)\n> >> +{\n> >> +\tstruct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));\n> >> +\n> >> +\treturn sprintf(buf, \"%d\\n\", chip->max_expected_current);\n> >> +}\n> >> +\n> >> +static ssize_t ina2xx_max_expected_current_store(struct device *dev,\n> >> +\t\t\t\t\t struct device_attribute *attr,\n> >> +\t\t\t\t\t const char *buf, size_t len)\n> >> +{\n> >> +\tstruct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));\n> >> +\tunsigned long val;\n> >> +\tint ret;\n> >> +\n> >> +\tret = kstrtoul((const char *) buf, 10, &val); \n> > \n> > Odd bit of casting given that's what it already is...\n> > \n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> +\tret = set_max_expected_current(chip, val);\n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> +\t/* Update the Calibration register */\n> >> +\tret = ina2xx_set_calibration(chip);\n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> +\treturn len;\n> >> +}\n> >> +\n> >> #define INA219_CHAN(_type, _index, _address) { \\\n> >> \t.type = (_type), \\\n> >> \t.address = (_address), \\\n> >> @@ -755,10 +816,15 @@ static IIO_DEVICE_ATTR(in_shunt_resistor, S_IRUGO | S_IWUSR,\n> >> \t\t ina2xx_shunt_resistor_show,\n> >> \t\t ina2xx_shunt_resistor_store, 0);\n> >> \n> >> +static IIO_DEVICE_ATTR(in_max_expected_current, S_IRUGO | S_IWUSR,\n> >> +\t\t ina2xx_max_expected_current_show,\n> >> +\t\t ina2xx_max_expected_current_store, 0);\n> >> +\n> >> static struct attribute *ina219_attributes[] = {\n> >> \t&iio_dev_attr_in_allow_async_readout.dev_attr.attr,\n> >> \t&iio_const_attr_ina219_integration_time_available.dev_attr.attr,\n> >> \t&iio_dev_attr_in_shunt_resistor.dev_attr.attr,\n> >> +\t&iio_dev_attr_in_max_expected_current.dev_attr.attr,\n> >> \tNULL,\n> >> };\n> >> \n> >> @@ -766,6 +832,7 @@ static struct attribute *ina226_attributes[] = {\n> >> \t&iio_dev_attr_in_allow_async_readout.dev_attr.attr,\n> >> \t&iio_const_attr_ina226_integration_time_available.dev_attr.attr,\n> >> \t&iio_dev_attr_in_shunt_resistor.dev_attr.attr,\n> >> +\t&iio_dev_attr_in_max_expected_current.dev_attr.attr,\n> >> \tNULL,\n> >> };\n> >> \n> >> @@ -851,6 +918,21 @@ static int ina2xx_probe(struct i2c_client *client,\n> >> \tif (ret)\n> >> \t\treturn ret;\n> >> \n> >> +\tif (of_property_read_u32(client->dev.of_node,\n> >> +\t\t\t\t \"max-expected-current\", &val) < 0) {\n> >> +\t\tstruct ina2xx_platform_data *pdata =\n> >> +\t\t dev_get_platdata(&client->dev);\n> >> +\n> >> +\t\tif (pdata && pdata->max_mA != 0)\n> >> +\t\t\tval = pdata->max_mA;\n> >> +\t\telse\n> >> +\t\t\tval = INA2XX_MAX_EXPECTED_A_DEFAULT;\n> >> +\t}\n> >> +\n> >> +\tret = set_max_expected_current(chip, val);\n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> \t/* Patch the current config register with default. */\n> >> \tval = chip->config->config_default;\n> >> \n> >> diff --git a/include/linux/platform_data/ina2xx.h b/include/linux/platform_data/ina2xx.h\n> >> index 9abc0ca..f02b1d8 100644\n> >> --- a/include/linux/platform_data/ina2xx.h\n> >> +++ b/include/linux/platform_data/ina2xx.h\n> >> @@ -13,7 +13,9 @@\n> >> /**\n> >> * struct ina2xx_platform_data - ina2xx info\n> >> * @shunt_uohms\t\tshunt resistance in microohms\n> >> + * @max_mA\t\tmax expected current in mA\n> >> */\n> >> struct ina2xx_platform_data {\n> >> \tlong shunt_uohms;\n> >> +\tint max_mA;\n> >> }; \n> > \n> > \n> > \n> > \n\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","mail.kernel.org;\n\tdmarc=none (p=none dis=none) header.from=kernel.org","mail.kernel.org;\n\tspf=none smtp.mailfrom=jic23@kernel.org"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y8z880g3wz9t4B\n\tfor ;\n\tSun, 8 Oct 2017 20:48:07 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751509AbdJHJsG (ORCPT\n\t);\n\tSun, 8 Oct 2017 05:48:06 -0400","from mail.kernel.org ([198.145.29.99]:56078 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751463AbdJHJsD (ORCPT );\n\tSun, 8 Oct 2017 05:48:03 -0400","from archlinux (cpc91196-cmbg18-2-0-cust659.5-4.cable.virginm.net\n\t[81.96.234.148])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby mail.kernel.org (Postfix) with ESMTPSA id 52B9621911;\n\tSun, 8 Oct 2017 09:47:59 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org 52B9621911","Date":"Sun, 8 Oct 2017 10:47:56 +0100","From":"Jonathan Cameron ","To":"Maciej Purski ","Cc":"devicetree@vger.kernel.org, linux-hwmon@vger.kernel.org,\n\tlinux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,\n\tlinux-samsung-soc@vger.kernel.org, linux-iio@vger.kernel.org,\n\tRob Herring , Mark Rutland ,\n\tGuenter Roeck , Jean Delvare , \n\tJonathan Corbet , Russell King ,\n\tKukjin Kim , Krzysztof Kozlowski , \n\tHartmut Knaack , Lars-Peter Clausen ,\n\tPeter Meerwald-Stadler ,\n\tBartlomiej Zolnierkiewicz ,\n\tMarek Szyprowski ","Subject":"Re: [PATCH 1/4] iio: adc: ina2xx: Make max expected current\n\tconfigurable","Message-ID":"<20171008104756.6eaa62c5@archlinux>","In-Reply-To":"","References":"<1506603015-27202-1-git-send-email-m.purski@samsung.com>\n\t\n\t<1506603015-27202-2-git-send-email-m.purski@samsung.com>\n\t<20171001112934.360c840b@archlinux>\n\t","X-Mailer":"Claws Mail 3.15.1-dirty (GTK+ 2.24.31; x86_64-pc-linux-gnu)","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1782586,"web_url":"http://patchwork.ozlabs.org/comment/1782586/","msgid":"<662441b9-1b04-3e8b-baac-5852ba873ca5@samsung.com>","list_archive_url":null,"date":"2017-10-09T08:08:42","subject":"Re: [PATCH 1/4] iio: adc: ina2xx: Make max expected current\n\tconfigurable","submitter":{"id":72100,"url":"http://patchwork.ozlabs.org/api/people/72100/","name":"Maciej Purski","email":"m.purski@samsung.com"},"content":"On 10/08/2017 11:47 AM, Jonathan Cameron wrote:\n> On Wed, 04 Oct 2017 09:11:31 +0200\n> Maciej Purski wrote:\n> \n>> On 10/01/2017 12:29 PM, Jonathan Cameron wrote:\n>>> On Thu, 28 Sep 2017 14:50:12 +0200\n>>> Maciej Purski wrote:\n>>> \n>>>> Max expected current is used for calculating calibration register value,\n>>>> Current LSB and Power LSB according to equations found in ina datasheet.\n>>>> Max expected current is now implicitly set to default value,\n>>>> which is 2^15, thanks to which Current LSB is equal to 1 mA and\n>>>> Power LSB is equal to 20000 uW or 25000 uW depending on ina model.\n>>>>\n>>>> Make max expected current configurable, just like it's already done\n>>>> with shunt resistance: from device tree, platform_data or later\n>>>> from sysfs. On each max_expected_current change, calculate new values\n>>>> for Current LSB and Power LSB. According to datasheet Current LSB should\n>>>> be calculated by dividing max expected current by 2^15, as values read\n>>>> from device registers are in this case 16-bit integers. Power LSB\n>>>> is calculated by multiplying Current LSB by a factor, which is defined\n>>>> in ina documentation.\n>>>\n>>> One odd bit of casting inline. Also this is new userspace ABI.\n>>> It needs documenting in\n>>>\n>>> Documentation/ABI/testing/sysfs-bus-iio* as appropriate.\n>>> I'm also unclear on one element about this - is it a value used only\n>>> for calibration or are we talking about the actual 'range' of the device?\n>>> \n>>\n>> This is used for calibration. The device measures directly only voltage.\n>> However, it has also current and power registers. Their values are\n>> calculated by the device using the calibration register which is calculated\n>> using max expected current. So I guess that it's not what you mean\n>> by the actual 'range' of the device.\n>>\n>>> The interpretation of this value isn't clear against the more general\n>>> ABI.\n>>>\n>>> In particular it is it in raw units (adc counts) or mA? Docs say\n>>> that but the naming of the attribute doesn't make this clear.\n>>> \n>>\n>> It's in mA. I can make it clear in the attribute name.\n>>\n>>> Also I'm unconvinced this isn't better represented using the\n>>> range specifications available for any IIO attribute on the raw\n>>> value in combination with adjusting the scale value.\n>>> Note not many drivers yet provide ranges on their raw outputs\n>>> but we do have core support for it. I've been meaning to start\n>>> pushing this out into drivers, but been busy since we introduced\n>>> the core support. The dpot-dac driver does use it for examplel\n>>> \n>>\n>>\n>> I'm not sure if what I'm about to add is similar to what is done\n>> in the mentioned dpot-dac driver. It seems that the callback read_avail\n>> returns information on raw values which can be obtained from the device.\n>> What I need is an adjustable value, which is then used by the device internally\n>> in order to calculate current with the requested precision. Max expected current\n>> is also used for calculating the scale value.\n>> Tell me if I'm wrong. Maybe I misunderstood the 'range' concept in IIO and\n>> your solution fits in here.\n>>\n> \n> I think I answered this in the other branch of the thread.\n> _calibscale is what you want here as it's internal to the device.\n> \n> It's not one often used for ADCs but quite a few other types of\n> device provide some front end analog adjustment (whilst it is digital\n> here, it is applied within the device - so we don't need to care).\n> \n> Jonathan\n\nThank you for your explanation. Calibscale seems suitable for me in this case,\nbut what do you think I should do then with SCALE attribute? Should I get rid of \nit for current and use only calibscale? Or should I use both calibscale and \nscale attributes and for current they will be the same value?\n\nI should mention that currenst_lsb value is also used for calculating power_lsb\nas they have a fixed ratio (20 or 25) given in the documentation.\n\nThanks,\n\nMaciej\n\n> \n>> Best regards,\n>>\n>> \tMaciej\n>>> This moves the burden of calculating the 1lsb value to userspace,\n>>> but importantly it would give us a consistent ABI where this fits\n>>> in with existing elements (largely buy not introducing any new\n>>> ones :).\n>>>\n>>> Thanks,\n>>>\n>>> Jonathan\n>>>>\n>>>> Signed-off-by: Maciej Purski \n>>>> ---\n>>>> drivers/iio/adc/ina2xx-adc.c | 110 ++++++++++++++++++++++++++++++-----\n>>>> include/linux/platform_data/ina2xx.h | 2 +\n>>>> 2 files changed, 98 insertions(+), 14 deletions(-)\n>>>>\n>>>> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c\n>>>> index f387b97..883fede 100644\n>>>> --- a/drivers/iio/adc/ina2xx-adc.c\n>>>> +++ b/drivers/iio/adc/ina2xx-adc.c\n>>>> @@ -56,6 +56,7 @@\n>>>> #define INA226_DEFAULT_IT\t\t1110\n>>>> \n>>>> #define INA2XX_RSHUNT_DEFAULT 10000\n>>>> +#define INA2XX_MAX_EXPECTED_A_DEFAULT\t(1 << 15)\t/* current_lsb = 1 mA */\n>>>> \n>>>> /*\n>>>> * bit masks for reading the settings in the configuration register\n>>>> @@ -114,7 +115,7 @@ struct ina2xx_config {\n>>>> \tint shunt_div;\n>>>> \tint bus_voltage_shift;\n>>>> \tint bus_voltage_lsb;\t/* uV */\n>>>> -\tint power_lsb;\t\t/* uW */\n>>>> +\tint power_lsb_factor;\n>>>> \tenum ina2xx_ids chip_id;\n>>>> };\n>>>> \n>>>> @@ -123,7 +124,10 @@ struct ina2xx_chip_info {\n>>>> \tstruct task_struct *task;\n>>>> \tconst struct ina2xx_config *config;\n>>>> \tstruct mutex state_lock;\n>>>> -\tunsigned int shunt_resistor;\n>>>> +\tunsigned int shunt_resistor;\t\t/* uOhms */\n>>>> +\tunsigned int max_expected_current;\t/* mA */\n>>>> +\tint current_lsb;\t\t\t/* uA */\n>>>> +\tint power_lsb;\t\t\t\t/* uW */\n>>>> \tint avg;\n>>>> \tint int_time_vbus; /* Bus voltage integration time uS */\n>>>> \tint int_time_vshunt; /* Shunt voltage integration time uS */\n>>>> @@ -137,7 +141,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n>>>> \t\t.shunt_div = 100,\n>>>> \t\t.bus_voltage_shift = 3,\n>>>> \t\t.bus_voltage_lsb = 4000,\n>>>> -\t\t.power_lsb = 20000,\n>>>> +\t\t.power_lsb_factor = 20,\n>>>> \t\t.chip_id = ina219,\n>>>> \t},\n>>>> \t[ina226] = {\n>>>> @@ -146,7 +150,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n>>>> \t\t.shunt_div = 400,\n>>>> \t\t.bus_voltage_shift = 0,\n>>>> \t\t.bus_voltage_lsb = 1250,\n>>>> -\t\t.power_lsb = 25000,\n>>>> +\t\t.power_lsb_factor = 25,\n>>>> \t\t.chip_id = ina226,\n>>>> \t},\n>>>> };\n>>>> @@ -210,14 +214,15 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,\n>>>> \n>>>> \t\tcase INA2XX_POWER:\n>>>> \t\t\t/* processed (mW) = raw*lsb (uW) / 1000 */\n>>>> -\t\t\t*val = chip->config->power_lsb;\n>>>> +\t\t\t*val = chip->power_lsb;\n>>>> \t\t\t*val2 = 1000;\n>>>> \t\t\treturn IIO_VAL_FRACTIONAL;\n>>>> \n>>>> \t\tcase INA2XX_CURRENT:\n>>>> -\t\t\t/* processed (mA) = raw (mA) */\n>>>> -\t\t\t*val = 1;\n>>>> -\t\t\treturn IIO_VAL_INT;\n>>>> +\t\t\t/* processed (mA) = raw*lsb (uA) / 1000 */\n>>>> +\t\t\t*val = chip->current_lsb;\n>>>> +\t\t\t*val2 = 1000;\n>>>> +\t\t\treturn IIO_VAL_FRACTIONAL;\n>>>> \t\t}\n>>>> \t}\n>>>> \n>>>> @@ -434,24 +439,47 @@ static ssize_t ina2xx_allow_async_readout_store(struct device *dev,\n>>>> }\n>>>> \n>>>> /*\n>>>> - * Set current LSB to 1mA, shunt is in uOhms\n>>>> - * (equation 13 in datasheet). We hardcode a Current_LSB\n>>>> - * of 1.0 x10-6. The only remaining parameter is RShunt.\n>>>> + * Calculate calibration value according to equation 1 in ina226 datasheet\n>>>> + * http://www.ti.com/lit/ds/symlink/ina226.pdf.\n>>>> + * Current LSB is in uA and RShunt is in uOhms, so RShunt should be\n>>>> + * converted to mOhms in order to keep the scale.\n>>>> * There is no need to expose the CALIBRATION register\n>>>> * to the user for now. But we need to reset this register\n>>>> - * if the user updates RShunt after driver init, e.g upon\n>>>> - * reading an EEPROM/Probe-type value.\n>>>> + * if the user updates RShunt or max expected current after driver\n>>>> + * init, e.g upon reading an EEPROM/Probe-type value.\n>>>> */\n>>>> static int ina2xx_set_calibration(struct ina2xx_chip_info *chip)\n>>>> {\n>>>> +\tunsigned int rshunt = DIV_ROUND_CLOSEST(chip->shunt_resistor, 1000);\n>>>> \tu16 regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,\n>>>> -\t\t\t\t chip->shunt_resistor);\n>>>> +\t\t\t\t chip->current_lsb * rshunt);\n>>>> \n>>>> \treturn regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);\n>>>> }\n>>>> \n>>>> +/*\n>>>> + * Set max_expected_current (mA) and calculate current_lsb (uA),\n>>>> + * according to equation 2 in ina226 datasheet. Power LSB is calculated\n>>>> + * by multiplying Current LSB by a given factor, which may vary depending\n>>>> + * on ina version.\n>>>> + */\n>>>> +static int set_max_expected_current(struct ina2xx_chip_info *chip,\n>>>> +\t\t\t\t unsigned int val)\n>>>> +{\n>>>> +\tif (val <= 0 || val > chip->config->calibration_factor)\n>>>> +\t\treturn -EINVAL;\n>>>> +\n>>>> +\tchip->max_expected_current = val;\n>>>> +\tchip->current_lsb = DIV_ROUND_CLOSEST(chip->max_expected_current * 1000,\n>>>> +\t\t\t\t\t 1 << 15);\n>>>> +\tchip->power_lsb = chip->current_lsb * chip->config->power_lsb_factor;\n>>>> +\n>>>> +\treturn 0;\n>>>> +}\n>>>> +\n>>>> static int set_shunt_resistor(struct ina2xx_chip_info *chip, unsigned int val)\n>>>> {\n>>>> +\n>>>> \tif (val <= 0 || val > chip->config->calibration_factor)\n>>>> \t\treturn -EINVAL;\n>>>> \n>>>> @@ -493,6 +521,39 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,\n>>>> \treturn len;\n>>>> }\n>>>> \n>>>> +static ssize_t ina2xx_max_expected_current_show(struct device *dev,\n>>>> +\t\t\t\t\t struct device_attribute *attr,\n>>>> +\t\t\t\t\t char *buf)\n>>>> +{\n>>>> +\tstruct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));\n>>>> +\n>>>> +\treturn sprintf(buf, \"%d\\n\", chip->max_expected_current);\n>>>> +}\n>>>> +\n>>>> +static ssize_t ina2xx_max_expected_current_store(struct device *dev,\n>>>> +\t\t\t\t\t struct device_attribute *attr,\n>>>> +\t\t\t\t\t const char *buf, size_t len)\n>>>> +{\n>>>> +\tstruct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));\n>>>> +\tunsigned long val;\n>>>> +\tint ret;\n>>>> +\n>>>> +\tret = kstrtoul((const char *) buf, 10, &val);\n>>>\n>>> Odd bit of casting given that's what it already is...\n>>> \n>>>> +\tif (ret)\n>>>> +\t\treturn ret;\n>>>> +\n>>>> +\tret = set_max_expected_current(chip, val);\n>>>> +\tif (ret)\n>>>> +\t\treturn ret;\n>>>> +\n>>>> +\t/* Update the Calibration register */\n>>>> +\tret = ina2xx_set_calibration(chip);\n>>>> +\tif (ret)\n>>>> +\t\treturn ret;\n>>>> +\n>>>> +\treturn len;\n>>>> +}\n>>>> +\n>>>> #define INA219_CHAN(_type, _index, _address) { \\\n>>>> \t.type = (_type), \\\n>>>> \t.address = (_address), \\\n>>>> @@ -755,10 +816,15 @@ static IIO_DEVICE_ATTR(in_shunt_resistor, S_IRUGO | S_IWUSR,\n>>>> \t\t ina2xx_shunt_resistor_show,\n>>>> \t\t ina2xx_shunt_resistor_store, 0);\n>>>> \n>>>> +static IIO_DEVICE_ATTR(in_max_expected_current, S_IRUGO | S_IWUSR,\n>>>> +\t\t ina2xx_max_expected_current_show,\n>>>> +\t\t ina2xx_max_expected_current_store, 0);\n>>>> +\n>>>> static struct attribute *ina219_attributes[] = {\n>>>> \t&iio_dev_attr_in_allow_async_readout.dev_attr.attr,\n>>>> \t&iio_const_attr_ina219_integration_time_available.dev_attr.attr,\n>>>> \t&iio_dev_attr_in_shunt_resistor.dev_attr.attr,\n>>>> +\t&iio_dev_attr_in_max_expected_current.dev_attr.attr,\n>>>> \tNULL,\n>>>> };\n>>>> \n>>>> @@ -766,6 +832,7 @@ static struct attribute *ina226_attributes[] = {\n>>>> \t&iio_dev_attr_in_allow_async_readout.dev_attr.attr,\n>>>> \t&iio_const_attr_ina226_integration_time_available.dev_attr.attr,\n>>>> \t&iio_dev_attr_in_shunt_resistor.dev_attr.attr,\n>>>> +\t&iio_dev_attr_in_max_expected_current.dev_attr.attr,\n>>>> \tNULL,\n>>>> };\n>>>> \n>>>> @@ -851,6 +918,21 @@ static int ina2xx_probe(struct i2c_client *client,\n>>>> \tif (ret)\n>>>> \t\treturn ret;\n>>>> \n>>>> +\tif (of_property_read_u32(client->dev.of_node,\n>>>> +\t\t\t\t \"max-expected-current\", &val) < 0) {\n>>>> +\t\tstruct ina2xx_platform_data *pdata =\n>>>> +\t\t dev_get_platdata(&client->dev);\n>>>> +\n>>>> +\t\tif (pdata && pdata->max_mA != 0)\n>>>> +\t\t\tval = pdata->max_mA;\n>>>> +\t\telse\n>>>> +\t\t\tval = INA2XX_MAX_EXPECTED_A_DEFAULT;\n>>>> +\t}\n>>>> +\n>>>> +\tret = set_max_expected_current(chip, val);\n>>>> +\tif (ret)\n>>>> +\t\treturn ret;\n>>>> +\n>>>> \t/* Patch the current config register with default. */\n>>>> \tval = chip->config->config_default;\n>>>> \n>>>> diff --git a/include/linux/platform_data/ina2xx.h b/include/linux/platform_data/ina2xx.h\n>>>> index 9abc0ca..f02b1d8 100644\n>>>> --- a/include/linux/platform_data/ina2xx.h\n>>>> +++ b/include/linux/platform_data/ina2xx.h\n>>>> @@ -13,7 +13,9 @@\n>>>> /**\n>>>> * struct ina2xx_platform_data - ina2xx info\n>>>> * @shunt_uohms\t\tshunt resistance in microohms\n>>>> + * @max_mA\t\tmax expected current in mA\n>>>> */\n>>>> struct ina2xx_platform_data {\n>>>> \tlong shunt_uohms;\n>>>> +\tint max_mA;\n>>>> };\n>>>\n>>>\n>>>\n>>> \n> \n> \n> \n> \n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y9XvC1tZ2z9tY0\n\tfor ;\n\tMon, 9 Oct 2017 19:08:55 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753977AbdJIIIx (ORCPT\n\t);\n\tMon, 9 Oct 2017 04:08:53 -0400","from mailout1.w1.samsung.com ([210.118.77.11]:34955 \"EHLO\n\tmailout1.w1.samsung.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753864AbdJIIIs (ORCPT\n\t); Mon, 9 Oct 2017 04:08:48 -0400","from eucas1p1.samsung.com (unknown [182.198.249.206])\n\tby mailout1.w1.samsung.com (KnoxPortal) with ESMTP id\n\t20171009080846euoutp0143cb146e48928bbbe374b29361d796d2~r2Huuv-j_0573505735euoutp01F;\n\tMon, 9 Oct 2017 08:08:46 +0000 (GMT)","from eusmges1.samsung.com (unknown [203.254.199.239]) by\n\teucas1p2.samsung.com (KnoxPortal) with ESMTP id\n\t20171009080845eucas1p234f7780aaa335a679ef03ca808299147~r2Ht9Wl_R1339113391eucas1p2N;\n\tMon, 9 Oct 2017 08:08:45 +0000 (GMT)","from eucas1p2.samsung.com ( [182.198.249.207]) by\n\teusmges1.samsung.com (EUCPMTA) with SMTP id 31.FE.12576.C8E2BD95;\n\tMon, 9 Oct 2017 09:08:44 +0100 (BST)","from eusmgms1.samsung.com (unknown [182.198.249.179]) by\n\teucas1p1.samsung.com (KnoxPortal) with ESMTP id\n\t20171009080844eucas1p18a060c484db93b7592cd9d95ded52aa5~r2HtUx8tE1954419544eucas1p1J;\n\tMon, 9 Oct 2017 08:08:44 +0000 (GMT)","from eusync3.samsung.com ( [203.254.199.213]) by\n\teusmgms1.samsung.com (EUCPMTA) with SMTP id 27.D6.18832.C8E2BD95;\n\tMon, 9 Oct 2017 09:08:44 +0100 (BST)","from [106.120.51.25] by eusync3.samsung.com (Oracle Communications\n\tMessaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTPA\n\tid <0OXJ003TXQMJIK70@eusync3.samsung.com>;\n\tMon, 09 Oct 2017 09:08:44 +0100 (BST)"],"X-AuditID":"cbfec7ef-f79ee6d000003120-cb-59db2e8c99ba","Subject":"Re: [PATCH 1/4] iio: adc: ina2xx: Make max expected current\n\tconfigurable","To":"Jonathan Cameron ","Cc":"devicetree@vger.kernel.org, linux-hwmon@vger.kernel.org,\n\tlinux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,\n\tlinux-samsung-soc@vger.kernel.org, linux-iio@vger.kernel.org,\n\tRob Herring , Mark Rutland ,\n\tGuenter Roeck , Jean Delvare , \n\tJonathan Corbet , Russell King ,\n\tKukjin Kim , Krzysztof Kozlowski , \n\tHartmut Knaack , Lars-Peter Clausen ,\n\tPeter Meerwald-Stadler ,\n\tBartlomiej Zolnierkiewicz ,\n\tMarek Szyprowski ","From":"Maciej Purski ","Message-id":"<662441b9-1b04-3e8b-baac-5852ba873ca5@samsung.com>","Date":"Mon, 09 Oct 2017 10:08:42 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-version":"1.0","In-reply-to":"<20171008104756.6eaa62c5@archlinux>","Content-type":"text/plain; charset=\"utf-8\"; format=\"flowed\"","Content-language":"en-US","Content-transfer-encoding":"7bit","X-Brightmail-Tracker":["H4sIAAAAAAAAA02Sa0hTYRjHec9lO4qz07R8s0iaWCBkGkUvllLQh0OQZAmlQjraQSU12VFL\n\ts9LMa6lzSxtGmrX5YU7UaZZLh82ldLG58lKOhuLlQ5omdqVcNY+F337/5/k/D8//5aVw8TLp\n\tSyWlprPyVGmyROBOdPT9sO68EWSPDp427UOt6mYSTfUUAVRneUWiAUUkGr+qw1DF5CyOjL/n\n\tcGS1tgiRRlVHIsPkCInqCzUEKpp9AFCtZZ5AaqsJQ+aqboCm6l9iqMnyXoi0ozYM/TT2CVFB\n\tt0V40JvR1+oB82bEhjOfFk8zBl2JgGnTXGHul6tIRjNnJhlrvgUwnd9yhUx5uw4wze3DBLNk\n\t2HrMI8b9gIxNTspk5bvC490Tdb8KhGmjCRee9peBXNATWQrcKEjvgboGBcbzRjjoaBaUAndK\n\tTDcAqJzvxXmxBKBN+xP8m3BWPsf+u8au20lezACo7SklXS4v+gTsru9bmfCmd8Bq9dDKBE6b\n\tSOho1P8VFCWgA6G+KM7lEdHhcNxQhrvKBB0AFxxHXOUN9Ck4aPiB8Zb18LvKQbjYjQ6GU6UV\n\tK+txOhTOOAtInn1gfsE7gmc/2Kb/iPNHO4TwmjmH58Ow0li/GtkLfuhvF/K8BZYUP1mtX4S2\n\tL0YBzzkwz96y6tkPl5TtOL/fEyo7bq2cDGkRLC4U8xYGlmuVq2sOwdddkwT/PEMYXLjTSyiA\n\tX82aODVrItSsiVCzJsJdQOiAN5vBpSSwXEgQJ03hMlITgs6cSzGAv5/yhbN/4RGYzo8yA5oC\n\tEg/RW/VYtJiUZnJZKWYAKVziLXq80R4tFsmkWdms/FycPCOZ5cxgM0VIfERhMYXRYjpBms6e\n\tZdk0Vv6vi1Fuvrlgw2I26zse51yWbF/QNqqjsg0BM7v9Y/UBR8X6Obs8rWpOJrnRGvFMpAht\n\tPXpJomycj0jf+/nz8Ddn2MkAz4duPT5fcZ/i2Zm2m06ZIraYE6kGcjcVrovff778u2kiuMnQ\n\tZcqq3lblu3XMpgGXJxpUdEXep3tNm49jnf61Q7dLJASXKA0JxOWc9A+z5+dykAMAAA==","H4sIAAAAAAAAA+NgFrrMIsWRmVeSWpSXmKPExsVy+t/xq7o9ercjDW72yVhsnLGe1eLJgXZG\n\ti/lHzrFanJ0QaPGgaRWTRf/j18wWu/6/YbY4f34Du8WSyfNZLTY9vsZqsbBtCYtF++utjBbz\n\tjrxjsZhxfh+TxaGpexktniw8w2Sx9shddoul1y8yWfzedYzdonXvEXYHEY8189Ywely+dpHZ\n\t48PHOI9NqzrZPDYvqfdY3DeZ1WPJm0OsHuebjzB67PzewO7Rt2UVo8f6LVdZPD5vkgvgieKy\n\tSUnNySxLLdK3S+DKWPWnlb3genrF0eO9jA2MBwK7GDk5JARMJP5NPMUEYYtJXLi3nq2LkYtD\n\tSGAJo8SmG73sEM4zRonm1p+MIFXCAsESexceA7NFBNQlps24wgRSxCxwgFVi3tGLUB1XmCRu\n\tPHoO5HBwsAloSaxpjwdp4BWwk3iwqZcZJMwioCrx/p4XiCkqECGxYSM/RIWgxI/J91hAbE4B\n\tA4knXf1gq5gFzCS+vDzMCmGLA51zkwXClpfYvOYt8wRGwVlI2mchaZmFpGUWkpYFjCyrGEVS\n\tS4tz03OLDfWKE3OLS/PS9ZLzczcxAqN627Gfm3cwXtoYfIhRgINRiYe3YdqtSCHWxLLiytxD\n\tjBIczEoivLvFbkcK8aYkVlalFuXHF5XmpBYfYpTmYFES5+3dszpSSCA9sSQ1OzW1ILUIJsvE\n\twSnVwBi5NKWyxe53UuA610Xt66xsPv0P0hU99tn4kXrqdJ0cy8Z9vkU1L/00NBvM1hU8NIlw\n\tCcjR7NqnsWKFkanmhoNnlj7L4t0zx4SzaoGZ17ZzCjftfyanLpfIjL9+yu7b2fVvS7Z/Llsy\n\t6czCuCtyLqrOyj/L59pzlfztkWANO7GwITrWruKSqRJLcUaioRZzUXEiAFM4nC/mAgAA"],"X-CMS-MailID":"20171009080844eucas1p18a060c484db93b7592cd9d95ded52aa5","X-Msg-Generator":"CA","X-Sender-IP":"182.198.249.179","X-Local-Sender":"=?utf-8?q?Maciej_Purski=1BSecurity_=28TP=29=1BSamsung_Ele?=\n\t=?utf-8?q?ctronics=1BTrainee_=28=29?=","X-Global-Sender":"=?utf-8?q?Maciej_Purski=1BSecurity_=28TP=29=1BSamsung_El?=\n\t=?utf-8?q?ectronics=1BTrainee_=28=29?=","X-Sender-Code":"=?utf-8?q?C10=1BEHQ=1BC10CD02CD027395?=","CMS-TYPE":"201P","X-CMS-RootMailID":"20170928125053eucas1p19e757ac5c3774f54c7cd0ee4525decbe","X-RootMTR":"20170928125053eucas1p19e757ac5c3774f54c7cd0ee4525decbe","References":"<1506603015-27202-1-git-send-email-m.purski@samsung.com>\n\t\n\t<1506603015-27202-2-git-send-email-m.purski@samsung.com>\n\t<20171001112934.360c840b@archlinux>\n\t\n\t<20171008104756.6eaa62c5@archlinux>","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1782843,"web_url":"http://patchwork.ozlabs.org/comment/1782843/","msgid":"<20171009143539.00002222@huawei.com>","list_archive_url":null,"date":"2017-10-09T13:35:39","subject":"Re: [PATCH 1/4] iio: adc: ina2xx: Make max expected current\n\tconfigurable","submitter":{"id":71988,"url":"http://patchwork.ozlabs.org/api/people/71988/","name":"Jonathan Cameron","email":"Jonathan.Cameron@huawei.com"},"content":"On Mon, 9 Oct 2017 10:08:42 +0200\nMaciej Purski wrote:\n\n> On 10/08/2017 11:47 AM, Jonathan Cameron wrote:\n> > On Wed, 04 Oct 2017 09:11:31 +0200\n> > Maciej Purski wrote:\n> > \n> >> On 10/01/2017 12:29 PM, Jonathan Cameron wrote: \n> >>> On Thu, 28 Sep 2017 14:50:12 +0200\n> >>> Maciej Purski wrote:\n> >>> \n> >>>> Max expected current is used for calculating calibration register value,\n> >>>> Current LSB and Power LSB according to equations found in ina datasheet.\n> >>>> Max expected current is now implicitly set to default value,\n> >>>> which is 2^15, thanks to which Current LSB is equal to 1 mA and\n> >>>> Power LSB is equal to 20000 uW or 25000 uW depending on ina model.\n> >>>>\n> >>>> Make max expected current configurable, just like it's already done\n> >>>> with shunt resistance: from device tree, platform_data or later\n> >>>> from sysfs. On each max_expected_current change, calculate new values\n> >>>> for Current LSB and Power LSB. According to datasheet Current LSB should\n> >>>> be calculated by dividing max expected current by 2^15, as values read\n> >>>> from device registers are in this case 16-bit integers. Power LSB\n> >>>> is calculated by multiplying Current LSB by a factor, which is defined\n> >>>> in ina documentation. \n> >>>\n> >>> One odd bit of casting inline. Also this is new userspace ABI.\n> >>> It needs documenting in\n> >>>\n> >>> Documentation/ABI/testing/sysfs-bus-iio* as appropriate.\n> >>> I'm also unclear on one element about this - is it a value used only\n> >>> for calibration or are we talking about the actual 'range' of the device?\n> >>> \n> >>\n> >> This is used for calibration. The device measures directly only voltage.\n> >> However, it has also current and power registers. Their values are\n> >> calculated by the device using the calibration register which is calculated\n> >> using max expected current. So I guess that it's not what you mean\n> >> by the actual 'range' of the device.\n> >> \n> >>> The interpretation of this value isn't clear against the more general\n> >>> ABI.\n> >>>\n> >>> In particular it is it in raw units (adc counts) or mA? Docs say\n> >>> that but the naming of the attribute doesn't make this clear.\n> >>> \n> >>\n> >> It's in mA. I can make it clear in the attribute name.\n> >> \n> >>> Also I'm unconvinced this isn't better represented using the\n> >>> range specifications available for any IIO attribute on the raw\n> >>> value in combination with adjusting the scale value.\n> >>> Note not many drivers yet provide ranges on their raw outputs\n> >>> but we do have core support for it. I've been meaning to start\n> >>> pushing this out into drivers, but been busy since we introduced\n> >>> the core support. The dpot-dac driver does use it for examplel\n> >>> \n> >>\n> >>\n> >> I'm not sure if what I'm about to add is similar to what is done\n> >> in the mentioned dpot-dac driver. It seems that the callback read_avail\n> >> returns information on raw values which can be obtained from the device.\n> >> What I need is an adjustable value, which is then used by the device internally\n> >> in order to calculate current with the requested precision. Max expected current\n> >> is also used for calculating the scale value.\n> >> Tell me if I'm wrong. Maybe I misunderstood the 'range' concept in IIO and\n> >> your solution fits in here.\n> >> \n> > \n> > I think I answered this in the other branch of the thread.\n> > _calibscale is what you want here as it's internal to the device.\n> > \n> > It's not one often used for ADCs but quite a few other types of\n> > device provide some front end analog adjustment (whilst it is digital\n> > here, it is applied within the device - so we don't need to care).\n> > \n> > Jonathan \n> \n> Thank you for your explanation. Calibscale seems suitable for me in this case,\n> but what do you think I should do then with SCALE attribute? Should I get rid of \n> it for current and use only calibscale? Or should I use both calibscale and \n> scale attributes and for current they will be the same value?\n\nYou'll have to leave it as it is existing ABI. It won't have the same value\nas calibscale. Calibscale is for internal changes that don't effect the raw\nvalue. scale is to be applied by userspace to the raw value. As I understand it\nhere the calibscale value should have no effect on scale.\n> \n> I should mention that currenst_lsb value is also used for calculating power_lsb\n> as they have a fixed ratio (20 or 25) given in the documentation.\n\nThen expose it for power as well (appropriately adjusted).\nIn IIO ABI it is fine to have two elements addressing the same underlying hardware\nvalue - rule is you change something, you check everything else hasn't changed.\n\nJonathan \n> \n> Thanks,\n> \n> Maciej\n> \n> > \n> >> Best regards,\n> >>\n> >> \tMaciej \n> >>> This moves the burden of calculating the 1lsb value to userspace,\n> >>> but importantly it would give us a consistent ABI where this fits\n> >>> in with existing elements (largely buy not introducing any new\n> >>> ones :).\n> >>>\n> >>> Thanks,\n> >>>\n> >>> Jonathan \n> >>>>\n> >>>> Signed-off-by: Maciej Purski \n> >>>> ---\n> >>>> drivers/iio/adc/ina2xx-adc.c | 110 ++++++++++++++++++++++++++++++-----\n> >>>> include/linux/platform_data/ina2xx.h | 2 +\n> >>>> 2 files changed, 98 insertions(+), 14 deletions(-)\n> >>>>\n> >>>> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c\n> >>>> index f387b97..883fede 100644\n> >>>> --- a/drivers/iio/adc/ina2xx-adc.c\n> >>>> +++ b/drivers/iio/adc/ina2xx-adc.c\n> >>>> @@ -56,6 +56,7 @@\n> >>>> #define INA226_DEFAULT_IT\t\t1110\n> >>>> \n> >>>> #define INA2XX_RSHUNT_DEFAULT 10000\n> >>>> +#define INA2XX_MAX_EXPECTED_A_DEFAULT\t(1 << 15)\t/* current_lsb = 1 mA */\n> >>>> \n> >>>> /*\n> >>>> * bit masks for reading the settings in the configuration register\n> >>>> @@ -114,7 +115,7 @@ struct ina2xx_config {\n> >>>> \tint shunt_div;\n> >>>> \tint bus_voltage_shift;\n> >>>> \tint bus_voltage_lsb;\t/* uV */\n> >>>> -\tint power_lsb;\t\t/* uW */\n> >>>> +\tint power_lsb_factor;\n> >>>> \tenum ina2xx_ids chip_id;\n> >>>> };\n> >>>> \n> >>>> @@ -123,7 +124,10 @@ struct ina2xx_chip_info {\n> >>>> \tstruct task_struct *task;\n> >>>> \tconst struct ina2xx_config *config;\n> >>>> \tstruct mutex state_lock;\n> >>>> -\tunsigned int shunt_resistor;\n> >>>> +\tunsigned int shunt_resistor;\t\t/* uOhms */\n> >>>> +\tunsigned int max_expected_current;\t/* mA */\n> >>>> +\tint current_lsb;\t\t\t/* uA */\n> >>>> +\tint power_lsb;\t\t\t\t/* uW */\n> >>>> \tint avg;\n> >>>> \tint int_time_vbus; /* Bus voltage integration time uS */\n> >>>> \tint int_time_vshunt; /* Shunt voltage integration time uS */\n> >>>> @@ -137,7 +141,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n> >>>> \t\t.shunt_div = 100,\n> >>>> \t\t.bus_voltage_shift = 3,\n> >>>> \t\t.bus_voltage_lsb = 4000,\n> >>>> -\t\t.power_lsb = 20000,\n> >>>> +\t\t.power_lsb_factor = 20,\n> >>>> \t\t.chip_id = ina219,\n> >>>> \t},\n> >>>> \t[ina226] = {\n> >>>> @@ -146,7 +150,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n> >>>> \t\t.shunt_div = 400,\n> >>>> \t\t.bus_voltage_shift = 0,\n> >>>> \t\t.bus_voltage_lsb = 1250,\n> >>>> -\t\t.power_lsb = 25000,\n> >>>> +\t\t.power_lsb_factor = 25,\n> >>>> \t\t.chip_id = ina226,\n> >>>> \t},\n> >>>> };\n> >>>> @@ -210,14 +214,15 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,\n> >>>> \n> >>>> \t\tcase INA2XX_POWER:\n> >>>> \t\t\t/* processed (mW) = raw*lsb (uW) / 1000 */\n> >>>> -\t\t\t*val = chip->config->power_lsb;\n> >>>> +\t\t\t*val = chip->power_lsb;\n> >>>> \t\t\t*val2 = 1000;\n> >>>> \t\t\treturn IIO_VAL_FRACTIONAL;\n> >>>> \n> >>>> \t\tcase INA2XX_CURRENT:\n> >>>> -\t\t\t/* processed (mA) = raw (mA) */\n> >>>> -\t\t\t*val = 1;\n> >>>> -\t\t\treturn IIO_VAL_INT;\n> >>>> +\t\t\t/* processed (mA) = raw*lsb (uA) / 1000 */\n> >>>> +\t\t\t*val = chip->current_lsb;\n> >>>> +\t\t\t*val2 = 1000;\n> >>>> +\t\t\treturn IIO_VAL_FRACTIONAL;\n> >>>> \t\t}\n> >>>> \t}\n> >>>> \n> >>>> @@ -434,24 +439,47 @@ static ssize_t ina2xx_allow_async_readout_store(struct device *dev,\n> >>>> }\n> >>>> \n> >>>> /*\n> >>>> - * Set current LSB to 1mA, shunt is in uOhms\n> >>>> - * (equation 13 in datasheet). We hardcode a Current_LSB\n> >>>> - * of 1.0 x10-6. The only remaining parameter is RShunt.\n> >>>> + * Calculate calibration value according to equation 1 in ina226 datasheet\n> >>>> + * http://www.ti.com/lit/ds/symlink/ina226.pdf.\n> >>>> + * Current LSB is in uA and RShunt is in uOhms, so RShunt should be\n> >>>> + * converted to mOhms in order to keep the scale.\n> >>>> * There is no need to expose the CALIBRATION register\n> >>>> * to the user for now. But we need to reset this register\n> >>>> - * if the user updates RShunt after driver init, e.g upon\n> >>>> - * reading an EEPROM/Probe-type value.\n> >>>> + * if the user updates RShunt or max expected current after driver\n> >>>> + * init, e.g upon reading an EEPROM/Probe-type value.\n> >>>> */\n> >>>> static int ina2xx_set_calibration(struct ina2xx_chip_info *chip)\n> >>>> {\n> >>>> +\tunsigned int rshunt = DIV_ROUND_CLOSEST(chip->shunt_resistor, 1000);\n> >>>> \tu16 regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,\n> >>>> -\t\t\t\t chip->shunt_resistor);\n> >>>> +\t\t\t\t chip->current_lsb * rshunt);\n> >>>> \n> >>>> \treturn regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);\n> >>>> }\n> >>>> \n> >>>> +/*\n> >>>> + * Set max_expected_current (mA) and calculate current_lsb (uA),\n> >>>> + * according to equation 2 in ina226 datasheet. Power LSB is calculated\n> >>>> + * by multiplying Current LSB by a given factor, which may vary depending\n> >>>> + * on ina version.\n> >>>> + */\n> >>>> +static int set_max_expected_current(struct ina2xx_chip_info *chip,\n> >>>> +\t\t\t\t unsigned int val)\n> >>>> +{\n> >>>> +\tif (val <= 0 || val > chip->config->calibration_factor)\n> >>>> +\t\treturn -EINVAL;\n> >>>> +\n> >>>> +\tchip->max_expected_current = val;\n> >>>> +\tchip->current_lsb = DIV_ROUND_CLOSEST(chip->max_expected_current * 1000,\n> >>>> +\t\t\t\t\t 1 << 15);\n> >>>> +\tchip->power_lsb = chip->current_lsb * chip->config->power_lsb_factor;\n> >>>> +\n> >>>> +\treturn 0;\n> >>>> +}\n> >>>> +\n> >>>> static int set_shunt_resistor(struct ina2xx_chip_info *chip, unsigned int val)\n> >>>> {\n> >>>> +\n> >>>> \tif (val <= 0 || val > chip->config->calibration_factor)\n> >>>> \t\treturn -EINVAL;\n> >>>> \n> >>>> @@ -493,6 +521,39 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,\n> >>>> \treturn len;\n> >>>> }\n> >>>> \n> >>>> +static ssize_t ina2xx_max_expected_current_show(struct device *dev,\n> >>>> +\t\t\t\t\t struct device_attribute *attr,\n> >>>> +\t\t\t\t\t char *buf)\n> >>>> +{\n> >>>> +\tstruct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));\n> >>>> +\n> >>>> +\treturn sprintf(buf, \"%d\\n\", chip->max_expected_current);\n> >>>> +}\n> >>>> +\n> >>>> +static ssize_t ina2xx_max_expected_current_store(struct device *dev,\n> >>>> +\t\t\t\t\t struct device_attribute *attr,\n> >>>> +\t\t\t\t\t const char *buf, size_t len)\n> >>>> +{\n> >>>> +\tstruct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));\n> >>>> +\tunsigned long val;\n> >>>> +\tint ret;\n> >>>> +\n> >>>> +\tret = kstrtoul((const char *) buf, 10, &val); \n> >>>\n> >>> Odd bit of casting given that's what it already is...\n> >>> \n> >>>> +\tif (ret)\n> >>>> +\t\treturn ret;\n> >>>> +\n> >>>> +\tret = set_max_expected_current(chip, val);\n> >>>> +\tif (ret)\n> >>>> +\t\treturn ret;\n> >>>> +\n> >>>> +\t/* Update the Calibration register */\n> >>>> +\tret = ina2xx_set_calibration(chip);\n> >>>> +\tif (ret)\n> >>>> +\t\treturn ret;\n> >>>> +\n> >>>> +\treturn len;\n> >>>> +}\n> >>>> +\n> >>>> #define INA219_CHAN(_type, _index, _address) { \\\n> >>>> \t.type = (_type), \\\n> >>>> \t.address = (_address), \\\n> >>>> @@ -755,10 +816,15 @@ static IIO_DEVICE_ATTR(in_shunt_resistor, S_IRUGO | S_IWUSR,\n> >>>> \t\t ina2xx_shunt_resistor_show,\n> >>>> \t\t ina2xx_shunt_resistor_store, 0);\n> >>>> \n> >>>> +static IIO_DEVICE_ATTR(in_max_expected_current, S_IRUGO | S_IWUSR,\n> >>>> +\t\t ina2xx_max_expected_current_show,\n> >>>> +\t\t ina2xx_max_expected_current_store, 0);\n> >>>> +\n> >>>> static struct attribute *ina219_attributes[] = {\n> >>>> \t&iio_dev_attr_in_allow_async_readout.dev_attr.attr,\n> >>>> \t&iio_const_attr_ina219_integration_time_available.dev_attr.attr,\n> >>>> \t&iio_dev_attr_in_shunt_resistor.dev_attr.attr,\n> >>>> +\t&iio_dev_attr_in_max_expected_current.dev_attr.attr,\n> >>>> \tNULL,\n> >>>> };\n> >>>> \n> >>>> @@ -766,6 +832,7 @@ static struct attribute *ina226_attributes[] = {\n> >>>> \t&iio_dev_attr_in_allow_async_readout.dev_attr.attr,\n> >>>> \t&iio_const_attr_ina226_integration_time_available.dev_attr.attr,\n> >>>> \t&iio_dev_attr_in_shunt_resistor.dev_attr.attr,\n> >>>> +\t&iio_dev_attr_in_max_expected_current.dev_attr.attr,\n> >>>> \tNULL,\n> >>>> };\n> >>>> \n> >>>> @@ -851,6 +918,21 @@ static int ina2xx_probe(struct i2c_client *client,\n> >>>> \tif (ret)\n> >>>> \t\treturn ret;\n> >>>> \n> >>>> +\tif (of_property_read_u32(client->dev.of_node,\n> >>>> +\t\t\t\t \"max-expected-current\", &val) < 0) {\n> >>>> +\t\tstruct ina2xx_platform_data *pdata =\n> >>>> +\t\t dev_get_platdata(&client->dev);\n> >>>> +\n> >>>> +\t\tif (pdata && pdata->max_mA != 0)\n> >>>> +\t\t\tval = pdata->max_mA;\n> >>>> +\t\telse\n> >>>> +\t\t\tval = INA2XX_MAX_EXPECTED_A_DEFAULT;\n> >>>> +\t}\n> >>>> +\n> >>>> +\tret = set_max_expected_current(chip, val);\n> >>>> +\tif (ret)\n> >>>> +\t\treturn ret;\n> >>>> +\n> >>>> \t/* Patch the current config register with default. */\n> >>>> \tval = chip->config->config_default;\n> >>>> \n> >>>> diff --git a/include/linux/platform_data/ina2xx.h b/include/linux/platform_data/ina2xx.h\n> >>>> index 9abc0ca..f02b1d8 100644\n> >>>> --- a/include/linux/platform_data/ina2xx.h\n> >>>> +++ b/include/linux/platform_data/ina2xx.h\n> >>>> @@ -13,7 +13,9 @@\n> >>>> /**\n> >>>> * struct ina2xx_platform_data - ina2xx info\n> >>>> * @shunt_uohms\t\tshunt resistance in microohms\n> >>>> + * @max_mA\t\tmax expected current in mA\n> >>>> */\n> >>>> struct ina2xx_platform_data {\n> >>>> \tlong shunt_uohms;\n> >>>> +\tint max_mA;\n> >>>> }; \n> >>>\n> >>>\n> >>>\n> >>> \n> > \n> > \n> > \n> > \n> --\n> To unsubscribe from this list: send the line \"unsubscribe linux-iio\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y9hFK2lq4z9rxm\n\tfor ;\n\tTue, 10 Oct 2017 00:40:05 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1754294AbdJINkB (ORCPT\n\t);\n\tMon, 9 Oct 2017 09:40:01 -0400","from szxga04-in.huawei.com ([45.249.212.190]:7951 \"EHLO\n\tszxga04-in.huawei.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1754561AbdJINjz (ORCPT\n\t); Mon, 9 Oct 2017 09:39:55 -0400","from 172.30.72.59 (EHLO DGGEMS410-HUB.china.huawei.com)\n\t([172.30.72.59])\n\tby dggrg04-dlp.huawei.com (MOS 4.4.6-GA FastPath queued)\n\twith ESMTP id DIS30951; Mon, 09 Oct 2017 21:38:17 +0800 (CST)","from localhost (10.206.48.115) by DGGEMS410-HUB.china.huawei.com\n\t(10.3.19.210) with Microsoft SMTP Server (TLS) id 14.3.301.0;\n\tMon, 9 Oct 2017 21:36:01 +0800"],"Date":"Mon, 9 Oct 2017 14:35:39 +0100","From":"Jonathan Cameron ","To":"Maciej Purski ","CC":"Jonathan Cameron , ,\n\t, ,\n\t,\n\t, ,\n\tRob Herring , Mark Rutland ,\n\tGuenter Roeck , Jean Delvare , \n\tJonathan Corbet , Russell King ,\n\tKukjin Kim , Krzysztof Kozlowski , \n\tHartmut Knaack , Lars-Peter Clausen ,\n\tPeter Meerwald-Stadler ,\n\t\"Bartlomiej Zolnierkiewicz\" ,\n\tMarek Szyprowski ","Subject":"Re: [PATCH 1/4] iio: adc: ina2xx: Make max expected current\n\tconfigurable","Message-ID":"<20171009143539.00002222@huawei.com>","In-Reply-To":"<662441b9-1b04-3e8b-baac-5852ba873ca5@samsung.com>","References":"<1506603015-27202-1-git-send-email-m.purski@samsung.com>\n\t\n\t<1506603015-27202-2-git-send-email-m.purski@samsung.com>\n\t<20171001112934.360c840b@archlinux>\n\t\n\t<20171008104756.6eaa62c5@archlinux>\n\t<662441b9-1b04-3e8b-baac-5852ba873ca5@samsung.com>","Organization":"Huawei","X-Mailer":"Claws Mail 3.15.0 (GTK+ 2.24.31; x86_64-w64-mingw32)","MIME-Version":"1.0","Content-Type":"text/plain; charset=\"US-ASCII\"","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[10.206.48.115]","X-CFilter-Loop":"Reflected","X-Mirapoint-Virus-RAPID-Raw":"score=unknown(0),\n\trefid=str=0001.0A090204.59DB7BCB.015C, ss=1, re=0.000, recu=0.000,\n\treip=0.000, cl=1, cld=1, fgs=0, ip=0.0.0.0,\n\tso=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32","X-Mirapoint-Loop-Id":"8f3449373ea3bd23a951fcd2c9055bc1","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1784707,"web_url":"http://patchwork.ozlabs.org/comment/1784707/","msgid":"<3a64c5a7-6acb-41dd-a18d-6b92b2bef560@samsung.com>","list_archive_url":null,"date":"2017-10-11T14:42:53","subject":"Re: [PATCH 1/4] iio: adc: ina2xx: Make max expected current\n\tconfigurable","submitter":{"id":72100,"url":"http://patchwork.ozlabs.org/api/people/72100/","name":"Maciej Purski","email":"m.purski@samsung.com"},"content":"On 10/09/2017 03:35 PM, Jonathan Cameron wrote:\n> On Mon, 9 Oct 2017 10:08:42 +0200\n> Maciej Purski wrote:\n> \n>> On 10/08/2017 11:47 AM, Jonathan Cameron wrote:\n>>> On Wed, 04 Oct 2017 09:11:31 +0200\n>>> Maciej Purski wrote:\n>>> \n>>>> On 10/01/2017 12:29 PM, Jonathan Cameron wrote:\n>>>>> On Thu, 28 Sep 2017 14:50:12 +0200\n>>>>> Maciej Purski wrote:\n>>>>> \n>>>>>> Max expected current is used for calculating calibration register value,\n>>>>>> Current LSB and Power LSB according to equations found in ina datasheet.\n>>>>>> Max expected current is now implicitly set to default value,\n>>>>>> which is 2^15, thanks to which Current LSB is equal to 1 mA and\n>>>>>> Power LSB is equal to 20000 uW or 25000 uW depending on ina model.\n>>>>>>\n>>>>>> Make max expected current configurable, just like it's already done\n>>>>>> with shunt resistance: from device tree, platform_data or later\n>>>>>> from sysfs. On each max_expected_current change, calculate new values\n>>>>>> for Current LSB and Power LSB. According to datasheet Current LSB should\n>>>>>> be calculated by dividing max expected current by 2^15, as values read\n>>>>>> from device registers are in this case 16-bit integers. Power LSB\n>>>>>> is calculated by multiplying Current LSB by a factor, which is defined\n>>>>>> in ina documentation.\n>>>>>\n>>>>> One odd bit of casting inline. Also this is new userspace ABI.\n>>>>> It needs documenting in\n>>>>>\n>>>>> Documentation/ABI/testing/sysfs-bus-iio* as appropriate.\n>>>>> I'm also unclear on one element about this - is it a value used only\n>>>>> for calibration or are we talking about the actual 'range' of the device?\n>>>>> \n>>>>\n>>>> This is used for calibration. The device measures directly only voltage.\n>>>> However, it has also current and power registers. Their values are\n>>>> calculated by the device using the calibration register which is calculated\n>>>> using max expected current. So I guess that it's not what you mean\n>>>> by the actual 'range' of the device.\n>>>> \n>>>>> The interpretation of this value isn't clear against the more general\n>>>>> ABI.\n>>>>>\n>>>>> In particular it is it in raw units (adc counts) or mA? Docs say\n>>>>> that but the naming of the attribute doesn't make this clear.\n>>>>> \n>>>>\n>>>> It's in mA. I can make it clear in the attribute name.\n>>>> \n>>>>> Also I'm unconvinced this isn't better represented using the\n>>>>> range specifications available for any IIO attribute on the raw\n>>>>> value in combination with adjusting the scale value.\n>>>>> Note not many drivers yet provide ranges on their raw outputs\n>>>>> but we do have core support for it. I've been meaning to start\n>>>>> pushing this out into drivers, but been busy since we introduced\n>>>>> the core support. The dpot-dac driver does use it for examplel\n>>>>> \n>>>>\n>>>>\n>>>> I'm not sure if what I'm about to add is similar to what is done\n>>>> in the mentioned dpot-dac driver. It seems that the callback read_avail\n>>>> returns information on raw values which can be obtained from the device.\n>>>> What I need is an adjustable value, which is then used by the device internally\n>>>> in order to calculate current with the requested precision. Max expected current\n>>>> is also used for calculating the scale value.\n>>>> Tell me if I'm wrong. Maybe I misunderstood the 'range' concept in IIO and\n>>>> your solution fits in here.\n>>>> \n>>>\n>>> I think I answered this in the other branch of the thread.\n>>> _calibscale is what you want here as it's internal to the device.\n>>>\n>>> It's not one often used for ADCs but quite a few other types of\n>>> device provide some front end analog adjustment (whilst it is digital\n>>> here, it is applied within the device - so we don't need to care).\n>>>\n>>> Jonathan\n>>\n>> Thank you for your explanation. Calibscale seems suitable for me in this case,\n>> but what do you think I should do then with SCALE attribute? Should I get rid of\n>> it for current and use only calibscale? Or should I use both calibscale and\n>> scale attributes and for current they will be the same value?\n> \n> You'll have to leave it as it is existing ABI. It won't have the same value\n> as calibscale. Calibscale is for internal changes that don't effect the raw\n> value. scale is to be applied by userspace to the raw value. As I understand it\n> here the calibscale value should have no effect on scale.\n\n\nSorry, but now I'm a little bit confused. Which value would you\nlike me to substitute with calibscale - max_expected_current or current_lsb?\nBoth values do have effect on the scale, as scale is basically\ncurrent_lsb / 1000 and on the raw value, as current register is calculated\ninternally using current_lsb.\n\nThe content of current register is always RAW unless we set current_lsb to 1. \nSee the documentation:\nhttp://www.ti.com/lit/ds/symlink/ina231.pdf\npage 15\n\nThanks,\n\nMaciej\n\n>>\n>> I should mention that currenst_lsb value is also used for calculating power_lsb\n>> as they have a fixed ratio (20 or 25) given in the documentation.\n> \n> Then expose it for power as well (appropriately adjusted).\n> In IIO ABI it is fine to have two elements addressing the same underlying hardware\n> value - rule is you change something, you check everything else hasn't changed.\n> \n> Jonathan >>\n>> Thanks,\n>>\n>> Maciej\n>>\n>>> \n>>>> Best regards,\n>>>>\n>>>> \tMaciej\n>>>>> This moves the burden of calculating the 1lsb value to userspace,\n>>>>> but importantly it would give us a consistent ABI where this fits\n>>>>> in with existing elements (largely buy not introducing any new\n>>>>> ones :).\n>>>>>\n>>>>> Thanks,\n>>>>>\n>>>>> Jonathan\n>>>>>>\n>>>>>> Signed-off-by: Maciej Purski \n>>>>>> ---\n>>>>>> drivers/iio/adc/ina2xx-adc.c | 110 ++++++++++++++++++++++++++++++-----\n>>>>>> include/linux/platform_data/ina2xx.h | 2 +\n>>>>>> 2 files changed, 98 insertions(+), 14 deletions(-)\n>>>>>>\n>>>>>> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c\n>>>>>> index f387b97..883fede 100644\n>>>>>> --- a/drivers/iio/adc/ina2xx-adc.c\n>>>>>> +++ b/drivers/iio/adc/ina2xx-adc.c\n>>>>>> @@ -56,6 +56,7 @@\n>>>>>> #define INA226_DEFAULT_IT\t\t1110\n>>>>>> \n>>>>>> #define INA2XX_RSHUNT_DEFAULT 10000\n>>>>>> +#define INA2XX_MAX_EXPECTED_A_DEFAULT\t(1 << 15)\t/* current_lsb = 1 mA */\n>>>>>> \n>>>>>> /*\n>>>>>> * bit masks for reading the settings in the configuration register\n>>>>>> @@ -114,7 +115,7 @@ struct ina2xx_config {\n>>>>>> \tint shunt_div;\n>>>>>> \tint bus_voltage_shift;\n>>>>>> \tint bus_voltage_lsb;\t/* uV */\n>>>>>> -\tint power_lsb;\t\t/* uW */\n>>>>>> +\tint power_lsb_factor;\n>>>>>> \tenum ina2xx_ids chip_id;\n>>>>>> };\n>>>>>> \n>>>>>> @@ -123,7 +124,10 @@ struct ina2xx_chip_info {\n>>>>>> \tstruct task_struct *task;\n>>>>>> \tconst struct ina2xx_config *config;\n>>>>>> \tstruct mutex state_lock;\n>>>>>> -\tunsigned int shunt_resistor;\n>>>>>> +\tunsigned int shunt_resistor;\t\t/* uOhms */\n>>>>>> +\tunsigned int max_expected_current;\t/* mA */\n>>>>>> +\tint current_lsb;\t\t\t/* uA */\n>>>>>> +\tint power_lsb;\t\t\t\t/* uW */\n>>>>>> \tint avg;\n>>>>>> \tint int_time_vbus; /* Bus voltage integration time uS */\n>>>>>> \tint int_time_vshunt; /* Shunt voltage integration time uS */\n>>>>>> @@ -137,7 +141,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n>>>>>> \t\t.shunt_div = 100,\n>>>>>> \t\t.bus_voltage_shift = 3,\n>>>>>> \t\t.bus_voltage_lsb = 4000,\n>>>>>> -\t\t.power_lsb = 20000,\n>>>>>> +\t\t.power_lsb_factor = 20,\n>>>>>> \t\t.chip_id = ina219,\n>>>>>> \t},\n>>>>>> \t[ina226] = {\n>>>>>> @@ -146,7 +150,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n>>>>>> \t\t.shunt_div = 400,\n>>>>>> \t\t.bus_voltage_shift = 0,\n>>>>>> \t\t.bus_voltage_lsb = 1250,\n>>>>>> -\t\t.power_lsb = 25000,\n>>>>>> +\t\t.power_lsb_factor = 25,\n>>>>>> \t\t.chip_id = ina226,\n>>>>>> \t},\n>>>>>> };\n>>>>>> @@ -210,14 +214,15 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,\n>>>>>> \n>>>>>> \t\tcase INA2XX_POWER:\n>>>>>> \t\t\t/* processed (mW) = raw*lsb (uW) / 1000 */\n>>>>>> -\t\t\t*val = chip->config->power_lsb;\n>>>>>> +\t\t\t*val = chip->power_lsb;\n>>>>>> \t\t\t*val2 = 1000;\n>>>>>> \t\t\treturn IIO_VAL_FRACTIONAL;\n>>>>>> \n>>>>>> \t\tcase INA2XX_CURRENT:\n>>>>>> -\t\t\t/* processed (mA) = raw (mA) */\n>>>>>> -\t\t\t*val = 1;\n>>>>>> -\t\t\treturn IIO_VAL_INT;\n>>>>>> +\t\t\t/* processed (mA) = raw*lsb (uA) / 1000 */\n>>>>>> +\t\t\t*val = chip->current_lsb;\n>>>>>> +\t\t\t*val2 = 1000;\n>>>>>> +\t\t\treturn IIO_VAL_FRACTIONAL;\n>>>>>> \t\t}\n>>>>>> \t}\n>>>>>> \n>>>>>> @@ -434,24 +439,47 @@ static ssize_t ina2xx_allow_async_readout_store(struct device *dev,\n>>>>>> }\n>>>>>> \n>>>>>> /*\n>>>>>> - * Set current LSB to 1mA, shunt is in uOhms\n>>>>>> - * (equation 13 in datasheet). We hardcode a Current_LSB\n>>>>>> - * of 1.0 x10-6. The only remaining parameter is RShunt.\n>>>>>> + * Calculate calibration value according to equation 1 in ina226 datasheet\n>>>>>> + * http://www.ti.com/lit/ds/symlink/ina226.pdf.\n>>>>>> + * Current LSB is in uA and RShunt is in uOhms, so RShunt should be\n>>>>>> + * converted to mOhms in order to keep the scale.\n>>>>>> * There is no need to expose the CALIBRATION register\n>>>>>> * to the user for now. But we need to reset this register\n>>>>>> - * if the user updates RShunt after driver init, e.g upon\n>>>>>> - * reading an EEPROM/Probe-type value.\n>>>>>> + * if the user updates RShunt or max expected current after driver\n>>>>>> + * init, e.g upon reading an EEPROM/Probe-type value.\n>>>>>> */\n>>>>>> static int ina2xx_set_calibration(struct ina2xx_chip_info *chip)\n>>>>>> {\n>>>>>> +\tunsigned int rshunt = DIV_ROUND_CLOSEST(chip->shunt_resistor, 1000);\n>>>>>> \tu16 regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,\n>>>>>> -\t\t\t\t chip->shunt_resistor);\n>>>>>> +\t\t\t\t chip->current_lsb * rshunt);\n>>>>>> \n>>>>>> \treturn regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);\n>>>>>> }\n>>>>>> \n>>>>>> +/*\n>>>>>> + * Set max_expected_current (mA) and calculate current_lsb (uA),\n>>>>>> + * according to equation 2 in ina226 datasheet. Power LSB is calculated\n>>>>>> + * by multiplying Current LSB by a given factor, which may vary depending\n>>>>>> + * on ina version.\n>>>>>> + */\n>>>>>> +static int set_max_expected_current(struct ina2xx_chip_info *chip,\n>>>>>> +\t\t\t\t unsigned int val)\n>>>>>> +{\n>>>>>> +\tif (val <= 0 || val > chip->config->calibration_factor)\n>>>>>> +\t\treturn -EINVAL;\n>>>>>> +\n>>>>>> +\tchip->max_expected_current = val;\n>>>>>> +\tchip->current_lsb = DIV_ROUND_CLOSEST(chip->max_expected_current * 1000,\n>>>>>> +\t\t\t\t\t 1 << 15);\n>>>>>> +\tchip->power_lsb = chip->current_lsb * chip->config->power_lsb_factor;\n>>>>>> +\n>>>>>> +\treturn 0;\n>>>>>> +}\n>>>>>> +\n>>>>>> static int set_shunt_resistor(struct ina2xx_chip_info *chip, unsigned int val)\n>>>>>> {\n>>>>>> +\n>>>>>> \tif (val <= 0 || val > chip->config->calibration_factor)\n>>>>>> \t\treturn -EINVAL;\n>>>>>> \n>>>>>> @@ -493,6 +521,39 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,\n>>>>>> \treturn len;\n>>>>>> }\n>>>>>> \n>>>>>> +static ssize_t ina2xx_max_expected_current_show(struct device *dev,\n>>>>>> +\t\t\t\t\t struct device_attribute *attr,\n>>>>>> +\t\t\t\t\t char *buf)\n>>>>>> +{\n>>>>>> +\tstruct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));\n>>>>>> +\n>>>>>> +\treturn sprintf(buf, \"%d\\n\", chip->max_expected_current);\n>>>>>> +}\n>>>>>> +\n>>>>>> +static ssize_t ina2xx_max_expected_current_store(struct device *dev,\n>>>>>> +\t\t\t\t\t struct device_attribute *attr,\n>>>>>> +\t\t\t\t\t const char *buf, size_t len)\n>>>>>> +{\n>>>>>> +\tstruct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));\n>>>>>> +\tunsigned long val;\n>>>>>> +\tint ret;\n>>>>>> +\n>>>>>> +\tret = kstrtoul((const char *) buf, 10, &val);\n>>>>>\n>>>>> Odd bit of casting given that's what it already is...\n>>>>> \n>>>>>> +\tif (ret)\n>>>>>> +\t\treturn ret;\n>>>>>> +\n>>>>>> +\tret = set_max_expected_current(chip, val);\n>>>>>> +\tif (ret)\n>>>>>> +\t\treturn ret;\n>>>>>> +\n>>>>>> +\t/* Update the Calibration register */\n>>>>>> +\tret = ina2xx_set_calibration(chip);\n>>>>>> +\tif (ret)\n>>>>>> +\t\treturn ret;\n>>>>>> +\n>>>>>> +\treturn len;\n>>>>>> +}\n>>>>>> +\n>>>>>> #define INA219_CHAN(_type, _index, _address) { \\\n>>>>>> \t.type = (_type), \\\n>>>>>> \t.address = (_address), \\\n>>>>>> @@ -755,10 +816,15 @@ static IIO_DEVICE_ATTR(in_shunt_resistor, S_IRUGO | S_IWUSR,\n>>>>>> \t\t ina2xx_shunt_resistor_show,\n>>>>>> \t\t ina2xx_shunt_resistor_store, 0);\n>>>>>> \n>>>>>> +static IIO_DEVICE_ATTR(in_max_expected_current, S_IRUGO | S_IWUSR,\n>>>>>> +\t\t ina2xx_max_expected_current_show,\n>>>>>> +\t\t ina2xx_max_expected_current_store, 0);\n>>>>>> +\n>>>>>> static struct attribute *ina219_attributes[] = {\n>>>>>> \t&iio_dev_attr_in_allow_async_readout.dev_attr.attr,\n>>>>>> \t&iio_const_attr_ina219_integration_time_available.dev_attr.attr,\n>>>>>> \t&iio_dev_attr_in_shunt_resistor.dev_attr.attr,\n>>>>>> +\t&iio_dev_attr_in_max_expected_current.dev_attr.attr,\n>>>>>> \tNULL,\n>>>>>> };\n>>>>>> \n>>>>>> @@ -766,6 +832,7 @@ static struct attribute *ina226_attributes[] = {\n>>>>>> \t&iio_dev_attr_in_allow_async_readout.dev_attr.attr,\n>>>>>> \t&iio_const_attr_ina226_integration_time_available.dev_attr.attr,\n>>>>>> \t&iio_dev_attr_in_shunt_resistor.dev_attr.attr,\n>>>>>> +\t&iio_dev_attr_in_max_expected_current.dev_attr.attr,\n>>>>>> \tNULL,\n>>>>>> };\n>>>>>> \n>>>>>> @@ -851,6 +918,21 @@ static int ina2xx_probe(struct i2c_client *client,\n>>>>>> \tif (ret)\n>>>>>> \t\treturn ret;\n>>>>>> \n>>>>>> +\tif (of_property_read_u32(client->dev.of_node,\n>>>>>> +\t\t\t\t \"max-expected-current\", &val) < 0) {\n>>>>>> +\t\tstruct ina2xx_platform_data *pdata =\n>>>>>> +\t\t dev_get_platdata(&client->dev);\n>>>>>> +\n>>>>>> +\t\tif (pdata && pdata->max_mA != 0)\n>>>>>> +\t\t\tval = pdata->max_mA;\n>>>>>> +\t\telse\n>>>>>> +\t\t\tval = INA2XX_MAX_EXPECTED_A_DEFAULT;\n>>>>>> +\t}\n>>>>>> +\n>>>>>> +\tret = set_max_expected_current(chip, val);\n>>>>>> +\tif (ret)\n>>>>>> +\t\treturn ret;\n>>>>>> +\n>>>>>> \t/* Patch the current config register with default. */\n>>>>>> \tval = chip->config->config_default;\n>>>>>> \n>>>>>> diff --git a/include/linux/platform_data/ina2xx.h b/include/linux/platform_data/ina2xx.h\n>>>>>> index 9abc0ca..f02b1d8 100644\n>>>>>> --- a/include/linux/platform_data/ina2xx.h\n>>>>>> +++ b/include/linux/platform_data/ina2xx.h\n>>>>>> @@ -13,7 +13,9 @@\n>>>>>> /**\n>>>>>> * struct ina2xx_platform_data - ina2xx info\n>>>>>> * @shunt_uohms\t\tshunt resistance in microohms\n>>>>>> + * @max_mA\t\tmax expected current in mA\n>>>>>> */\n>>>>>> struct ina2xx_platform_data {\n>>>>>> \tlong shunt_uohms;\n>>>>>> +\tint max_mA;\n>>>>>> };\n>>>>>\n>>>>>\n>>>>>\n>>>>> \n>>>\n>>>\n>>>\n>>> \n>> --\n>> To unsubscribe from this list: send the line \"unsubscribe linux-iio\" in\n>> the body of a message to majordomo@vger.kernel.org\n>> More majordomo info at http://vger.kernel.org/majordomo-info.html\n> \n> \n> \n> \n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yBxY44nz1z9s7M\n\tfor ;\n\tThu, 12 Oct 2017 01:43:04 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751816AbdJKOnC (ORCPT\n\t);\n\tWed, 11 Oct 2017 10:43:02 -0400","from mailout1.w1.samsung.com ([210.118.77.11]:34056 \"EHLO\n\tmailout1.w1.samsung.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751814AbdJKOnA (ORCPT\n\t); Wed, 11 Oct 2017 10:43:00 -0400","from eucas1p2.samsung.com (unknown [182.198.249.207])\n\tby mailout1.w1.samsung.com (KnoxPortal) with ESMTP id\n\t20171011144256euoutp011fc5875778e6ea1d606eb65b0cb329cb~siydrgHVu1854218542euoutp01X;\n\tWed, 11 Oct 2017 14:42:56 +0000 (GMT)","from eusmges1.samsung.com (unknown [203.254.199.239]) by\n\teucas1p2.samsung.com (KnoxPortal) with ESMTP id\n\t20171011144255eucas1p29ae9a068cc1c330d2fa68aaac743fce2~siyc_bmZy0862908629eucas1p2N;\n\tWed, 11 Oct 2017 14:42:55 +0000 (GMT)","from eucas1p1.samsung.com ( [182.198.249.206]) by\n\teusmges1.samsung.com (EUCPMTA) with SMTP id 93.F7.12576.FED2ED95;\n\tWed, 11 Oct 2017 15:42:55 +0100 (BST)","from eusmgms2.samsung.com (unknown [182.198.249.180]) by\n\teucas1p1.samsung.com (KnoxPortal) with ESMTP id\n\t20171011144255eucas1p1425846c8742dfb0e37568ebe5ffc4259~siycU9nWI2590125901eucas1p1V;\n\tWed, 11 Oct 2017 14:42:55 +0000 (GMT)","from eusync3.samsung.com ( [203.254.199.213]) by\n\teusmgms2.samsung.com (EUCPMTA) with SMTP id 06.DE.20118.FED2ED95;\n\tWed, 11 Oct 2017 15:42:55 +0100 (BST)","from [106.120.51.25] by eusync3.samsung.com (Oracle Communications\n\tMessaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTPA\n\tid <0OXN002WHY7IBN90@eusync3.samsung.com>;\n\tWed, 11 Oct 2017 15:42:55 +0100 (BST)"],"X-AuditID":"cbfec7ef-f79ee6d000003120-13-59de2def127c","Subject":"Re: [PATCH 1/4] iio: adc: ina2xx: Make max expected current\n\tconfigurable","To":"Jonathan Cameron ","Cc":"Jonathan Cameron , devicetree@vger.kernel.org,\n\tlinux-hwmon@vger.kernel.org, linux-doc@vger.kernel.org,\n\tlinux-arm-kernel@lists.infradead.org,\n\tlinux-samsung-soc@vger.kernel.org, linux-iio@vger.kernel.org,\n\tRob Herring , Mark Rutland ,\n\tGuenter Roeck , Jean Delvare , \n\tJonathan Corbet , Russell King ,\n\tKukjin Kim , Krzysztof Kozlowski , \n\tHartmut Knaack , Lars-Peter Clausen ,\n\tPeter Meerwald-Stadler ,\n\tBartlomiej Zolnierkiewicz ,\n\tMarek Szyprowski ","From":"Maciej Purski ","Message-id":"<3a64c5a7-6acb-41dd-a18d-6b92b2bef560@samsung.com>","Date":"Wed, 11 Oct 2017 16:42:53 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-version":"1.0","In-reply-to":"<20171009143539.00002222@huawei.com>","Content-type":"text/plain; charset=\"utf-8\"; format=\"flowed\"","Content-language":"en-US","Content-transfer-encoding":"7bit","X-Brightmail-Tracker":["H4sIAAAAAAAAA02Se0hTURzHO7uPXVeL29T82ZNWGQhahn8czKIXdMvMXlBGaEMvGjq1TSMr\n\taGpqD1LbeohYOlqDluGay8q0Yo0V+ZgmTSWVYGrWzArRskhrXgv/+5zf+fx+/L6HwxCyc/QC\n\t5mhaJq9KU6TKaQlZ6xhvCfkS0hu7pnBgFb5fWk3hvueFCFfYWyjcXLIHv881ibBJ76JxsdtD\n\t4LrJIQI7nWYxNugqKGxxuyisLzCQuNDzAOGb9mESlzqfirDtagPCffomEb5n7xHj2x1tIvyr\n\tziHG+Q128UZ/rupmFeLaXW0E9/VbHHfW/pniLKbzNFdjOMPdKtJRnGHIRnHOPDviHn/XiLki\n\tqwlx1da3JDdiWbJbekgSmcinHj3Oq1ZvOCJJzq2toTOuZZ+oszpFGtQdfwH5MMCGg7G9Bgk8\n\tH1p7q+kLSMLIWCOC4ntdlHAYQfBqpIT81zHUNEr8t4abXk+3DCAYNI6JvZYvuw8a9I6puX7s\n\tWhju1CKvRLD9FDw4W/xXYhiaDYaqwqk9pOwGuOzREF4m2ZXQ6LxLe9mfPQitlnGR4MyDH7re\n\tqS182DC4M9kzVSfYCBiYyKcEDoC8/C5S4KVQU/WZELYeFcPb8QMCbwXHk5zpNL7w6aVVLPAi\n\taNddnK6fgrbROlrg05DzzjztrIMRrZUQ5s8Fbe11whsFWCmcK5AJCgdFt7UigTfBm3o3KbyP\n\tmYBW7Q26BC0tmxGnbEaEshkRymZEqESkCfnxWWplEq8OC1UrlOqstKTQhHSlBf39n40TL788\n\tQv15+22IZZB8jpTt6I6VUYrj6mylDQFDyP2k9O+eWJk0UZF9klelx6uyUnm1DS1kSHmAdP2h\n\tglgZm6TI5FN4PoNX/bsVMT4LNKi8Pmh5oM63s/vntq3Bl+b5f9JtKu+bfB3UORYU3RgjX5xo\n\tTXFF4Cg0lrKFjF+RFRcV+SI0O7c1R2M8UelKyHDfODaMwisOf5wVrRwL2WkPcDcnuDevivlw\n\tMVyydzDC8Uu77Eredn1i86hjF78tweAbMrHY89C8PNA4Owqe8TuwnFQnK8KCCZVa8QfOhkSx\n\tmwMAAA==","H4sIAAAAAAAAA02Ra0hTYRiA/XYuOw6Hx6X5pZKwCkPS1C58UFlR0KE/MwxrBdrIg4pOZcdZ\n\t60/TqStDs6yQgZo0s7aZOpeYOqm5LFNbKk4DXRelEi8VOcpu5hyB/56X93l4f7wUJnLhIVRG\n\tdh6ryJZliUkB3v/32WjU5yiXNOZSkR9qqWoi0PRjLUC19pcEGqw4ht4WGnjIUOck0dWpWQx1\n\tLM9hyOFo5iN9ZS2BzFNOAtWV6HGknX0IUI19AUdVjm4est20AjRdN8BDjfZJPqofG+KhXx29\n\tfFRstfMPBDGmGhNgRpxDGPPlazJTZJ8nGLPhMsm06i8yd8orCUY/ZyMYh8YOmEff1Xym3GIA\n\tTJNlFGe+mTcmCE8J9qayWRn5rGJ7/BlBemFbK5l7S3W+w+LgqcFESinwpSC9E84NuDEvr4ev\n\tXE1kKRBQIloPYO3d24R3+ADgz4EuwmOtoxOhta4XeDiQjoML49eBR8LojwRsLx8B3qIZgw7r\n\t+EpBUSQdCU3a1XNCOh5em1WvnsPpLbDfYSQ9ShB9Eja3+HuVAPij0oV72JeOhfeXJ3kexujd\n\tcHGmh/ByMNQUv8a9HA5bTfNYBQjQrcl1axLdmkS3JrkNcAMIZJWcPE3OxUVzMjmnzE6LPpsj\n\tN4OVr7f1LlnaQelCog3QFBD7CemxCamIkOVzKrkNQAoTBwrJP5NSkTBVprrAKnJSFMoslrOB\n\tUAoXBwvLuoxSEZ0my2MzWTaXVfzf8ijfEDXwmU7u2+W+IjmYZB18/8ZS6BoxBtjDJGWbC4Yt\n\tN45q9rTfy/DvlHQ2LGWblAUln7LqE96x4SmU4IWzOjNhqedEQ+ZxXULbEeMG0bbuJ30pD07v\n\tCG10VR+OKXAPq3wk+3tZaVzYsnbfoa0zDW3u5z5ay2+Nn2sxIgLN1jzddC4pXYxz6bLYSEzB\n\tyf4BOHZgYvECAAA="],"X-CMS-MailID":"20171011144255eucas1p1425846c8742dfb0e37568ebe5ffc4259","X-Msg-Generator":"CA","X-Sender-IP":"182.198.249.180","X-Local-Sender":"=?utf-8?q?Maciej_Purski=1BSecurity_=28TP=29=1BSamsung_Ele?=\n\t=?utf-8?q?ctronics=1BTrainee_=28=29?=","X-Global-Sender":"=?utf-8?q?Maciej_Purski=1BSecurity_=28TP=29=1BSamsung_El?=\n\t=?utf-8?q?ectronics=1BTrainee_=28=29?=","X-Sender-Code":"=?utf-8?q?C10=1BEHQ=1BC10CD02CD027395?=","CMS-TYPE":"201P","X-CMS-RootMailID":"20170928125053eucas1p19e757ac5c3774f54c7cd0ee4525decbe","X-RootMTR":"20170928125053eucas1p19e757ac5c3774f54c7cd0ee4525decbe","References":"<1506603015-27202-1-git-send-email-m.purski@samsung.com>\n\t\n\t<1506603015-27202-2-git-send-email-m.purski@samsung.com>\n\t<20171001112934.360c840b@archlinux>\n\t\n\t<20171008104756.6eaa62c5@archlinux>\n\t<662441b9-1b04-3e8b-baac-5852ba873ca5@samsung.com>\n\t<20171009143539.00002222@huawei.com>","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1784760,"web_url":"http://patchwork.ozlabs.org/comment/1784760/","msgid":"<20171011165941.00006914@huawei.com>","list_archive_url":null,"date":"2017-10-11T15:59:41","subject":"Re: [PATCH 1/4] iio: adc: ina2xx: Make max expected current\n\tconfigurable","submitter":{"id":71988,"url":"http://patchwork.ozlabs.org/api/people/71988/","name":"Jonathan Cameron","email":"Jonathan.Cameron@huawei.com"},"content":"On Wed, 11 Oct 2017 16:42:53 +0200\nMaciej Purski wrote:\n\n> On 10/09/2017 03:35 PM, Jonathan Cameron wrote:\n> > On Mon, 9 Oct 2017 10:08:42 +0200\n> > Maciej Purski wrote:\n> > \n> >> On 10/08/2017 11:47 AM, Jonathan Cameron wrote: \n> >>> On Wed, 04 Oct 2017 09:11:31 +0200\n> >>> Maciej Purski wrote:\n> >>> \n> >>>> On 10/01/2017 12:29 PM, Jonathan Cameron wrote: \n> >>>>> On Thu, 28 Sep 2017 14:50:12 +0200\n> >>>>> Maciej Purski wrote:\n> >>>>> \n> >>>>>> Max expected current is used for calculating calibration register value,\n> >>>>>> Current LSB and Power LSB according to equations found in ina datasheet.\n> >>>>>> Max expected current is now implicitly set to default value,\n> >>>>>> which is 2^15, thanks to which Current LSB is equal to 1 mA and\n> >>>>>> Power LSB is equal to 20000 uW or 25000 uW depending on ina model.\n> >>>>>>\n> >>>>>> Make max expected current configurable, just like it's already done\n> >>>>>> with shunt resistance: from device tree, platform_data or later\n> >>>>>> from sysfs. On each max_expected_current change, calculate new values\n> >>>>>> for Current LSB and Power LSB. According to datasheet Current LSB should\n> >>>>>> be calculated by dividing max expected current by 2^15, as values read\n> >>>>>> from device registers are in this case 16-bit integers. Power LSB\n> >>>>>> is calculated by multiplying Current LSB by a factor, which is defined\n> >>>>>> in ina documentation. \n> >>>>>\n> >>>>> One odd bit of casting inline. Also this is new userspace ABI.\n> >>>>> It needs documenting in\n> >>>>>\n> >>>>> Documentation/ABI/testing/sysfs-bus-iio* as appropriate.\n> >>>>> I'm also unclear on one element about this - is it a value used only\n> >>>>> for calibration or are we talking about the actual 'range' of the device?\n> >>>>> \n> >>>>\n> >>>> This is used for calibration. The device measures directly only voltage.\n> >>>> However, it has also current and power registers. Their values are\n> >>>> calculated by the device using the calibration register which is calculated\n> >>>> using max expected current. So I guess that it's not what you mean\n> >>>> by the actual 'range' of the device.\n> >>>> \n> >>>>> The interpretation of this value isn't clear against the more general\n> >>>>> ABI.\n> >>>>>\n> >>>>> In particular it is it in raw units (adc counts) or mA? Docs say\n> >>>>> that but the naming of the attribute doesn't make this clear.\n> >>>>> \n> >>>>\n> >>>> It's in mA. I can make it clear in the attribute name.\n> >>>> \n> >>>>> Also I'm unconvinced this isn't better represented using the\n> >>>>> range specifications available for any IIO attribute on the raw\n> >>>>> value in combination with adjusting the scale value.\n> >>>>> Note not many drivers yet provide ranges on their raw outputs\n> >>>>> but we do have core support for it. I've been meaning to start\n> >>>>> pushing this out into drivers, but been busy since we introduced\n> >>>>> the core support. The dpot-dac driver does use it for examplel\n> >>>>> \n> >>>>\n> >>>>\n> >>>> I'm not sure if what I'm about to add is similar to what is done\n> >>>> in the mentioned dpot-dac driver. It seems that the callback read_avail\n> >>>> returns information on raw values which can be obtained from the device.\n> >>>> What I need is an adjustable value, which is then used by the device internally\n> >>>> in order to calculate current with the requested precision. Max expected current\n> >>>> is also used for calculating the scale value.\n> >>>> Tell me if I'm wrong. Maybe I misunderstood the 'range' concept in IIO and\n> >>>> your solution fits in here.\n> >>>> \n> >>>\n> >>> I think I answered this in the other branch of the thread.\n> >>> _calibscale is what you want here as it's internal to the device.\n> >>>\n> >>> It's not one often used for ADCs but quite a few other types of\n> >>> device provide some front end analog adjustment (whilst it is digital\n> >>> here, it is applied within the device - so we don't need to care).\n> >>>\n> >>> Jonathan \n> >>\n> >> Thank you for your explanation. Calibscale seems suitable for me in this case,\n> >> but what do you think I should do then with SCALE attribute? Should I get rid of\n> >> it for current and use only calibscale? Or should I use both calibscale and\n> >> scale attributes and for current they will be the same value? \n> > \n> > You'll have to leave it as it is existing ABI. It won't have the same value\n> > as calibscale. Calibscale is for internal changes that don't effect the raw\n> > value. scale is to be applied by userspace to the raw value. As I understand it\n> > here the calibscale value should have no effect on scale. \n> \n> \n> Sorry, but now I'm a little bit confused. Which value would you\n> like me to substitute with calibscale - max_expected_current or current_lsb?\n\nAs it is an internal scale it actually doesn't matter which one you use. Both\nare valid.\n\n> Both values do have effect on the scale, as scale is basically\n> current_lsb / 1000 and on the raw value, as current register is calculated\n> internally using current_lsb.\n\nHmm The terminology in the datasheet is confusing. I read it as these were\nintended to 'calibrate' the system. They aren't - they are intended to apply\na scaling to make the output use the full range. \n\nSo they will effect the scale which is nasty.\nThe annoying bit about this is that means the key value you should be modifying\nis scale and you should not use calibscale at all. It isn't appropriate because\nthat is not meant to effect the scale of the exposed value. It's for calibration\nto correct device inaccuracies. That isn't true here.\n\nAnyhow, so scale is all you should need. A bit ugly but there we are.\n\n\n> \n> The content of current register is always RAW unless we set current_lsb to 1. \n> See the documentation:\n> http://www.ti.com/lit/ds/symlink/ina231.pdf\n> page 15\n> \n> Thanks,\n> \n> Maciej\n> \n> >>\n> >> I should mention that currenst_lsb value is also used for calculating power_lsb\n> >> as they have a fixed ratio (20 or 25) given in the documentation. \n> > \n> > Then expose it for power as well (appropriately adjusted).\n> > In IIO ABI it is fine to have two elements addressing the same underlying hardware\n> > value - rule is you change something, you check everything else hasn't changed.\n> > \n> > Jonathan >> \n> >> Thanks,\n> >>\n> >> Maciej\n> >> \n> >>> \n> >>>> Best regards,\n> >>>>\n> >>>> \tMaciej \n> >>>>> This moves the burden of calculating the 1lsb value to userspace,\n> >>>>> but importantly it would give us a consistent ABI where this fits\n> >>>>> in with existing elements (largely buy not introducing any new\n> >>>>> ones :).\n> >>>>>\n> >>>>> Thanks,\n> >>>>>\n> >>>>> Jonathan \n> >>>>>>\n> >>>>>> Signed-off-by: Maciej Purski \n> >>>>>> ---\n> >>>>>> drivers/iio/adc/ina2xx-adc.c | 110 ++++++++++++++++++++++++++++++-----\n> >>>>>> include/linux/platform_data/ina2xx.h | 2 +\n> >>>>>> 2 files changed, 98 insertions(+), 14 deletions(-)\n> >>>>>>\n> >>>>>> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c\n> >>>>>> index f387b97..883fede 100644\n> >>>>>> --- a/drivers/iio/adc/ina2xx-adc.c\n> >>>>>> +++ b/drivers/iio/adc/ina2xx-adc.c\n> >>>>>> @@ -56,6 +56,7 @@\n> >>>>>> #define INA226_DEFAULT_IT\t\t1110\n> >>>>>> \n> >>>>>> #define INA2XX_RSHUNT_DEFAULT 10000\n> >>>>>> +#define INA2XX_MAX_EXPECTED_A_DEFAULT\t(1 << 15)\t/* current_lsb = 1 mA */\n> >>>>>> \n> >>>>>> /*\n> >>>>>> * bit masks for reading the settings in the configuration register\n> >>>>>> @@ -114,7 +115,7 @@ struct ina2xx_config {\n> >>>>>> \tint shunt_div;\n> >>>>>> \tint bus_voltage_shift;\n> >>>>>> \tint bus_voltage_lsb;\t/* uV */\n> >>>>>> -\tint power_lsb;\t\t/* uW */\n> >>>>>> +\tint power_lsb_factor;\n> >>>>>> \tenum ina2xx_ids chip_id;\n> >>>>>> };\n> >>>>>> \n> >>>>>> @@ -123,7 +124,10 @@ struct ina2xx_chip_info {\n> >>>>>> \tstruct task_struct *task;\n> >>>>>> \tconst struct ina2xx_config *config;\n> >>>>>> \tstruct mutex state_lock;\n> >>>>>> -\tunsigned int shunt_resistor;\n> >>>>>> +\tunsigned int shunt_resistor;\t\t/* uOhms */\n> >>>>>> +\tunsigned int max_expected_current;\t/* mA */\n> >>>>>> +\tint current_lsb;\t\t\t/* uA */\n> >>>>>> +\tint power_lsb;\t\t\t\t/* uW */\n> >>>>>> \tint avg;\n> >>>>>> \tint int_time_vbus; /* Bus voltage integration time uS */\n> >>>>>> \tint int_time_vshunt; /* Shunt voltage integration time uS */\n> >>>>>> @@ -137,7 +141,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n> >>>>>> \t\t.shunt_div = 100,\n> >>>>>> \t\t.bus_voltage_shift = 3,\n> >>>>>> \t\t.bus_voltage_lsb = 4000,\n> >>>>>> -\t\t.power_lsb = 20000,\n> >>>>>> +\t\t.power_lsb_factor = 20,\n> >>>>>> \t\t.chip_id = ina219,\n> >>>>>> \t},\n> >>>>>> \t[ina226] = {\n> >>>>>> @@ -146,7 +150,7 @@ static const struct ina2xx_config ina2xx_config[] = {\n> >>>>>> \t\t.shunt_div = 400,\n> >>>>>> \t\t.bus_voltage_shift = 0,\n> >>>>>> \t\t.bus_voltage_lsb = 1250,\n> >>>>>> -\t\t.power_lsb = 25000,\n> >>>>>> +\t\t.power_lsb_factor = 25,\n> >>>>>> \t\t.chip_id = ina226,\n> >>>>>> \t},\n> >>>>>> };\n> >>>>>> @@ -210,14 +214,15 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,\n> >>>>>> \n> >>>>>> \t\tcase INA2XX_POWER:\n> >>>>>> \t\t\t/* processed (mW) = raw*lsb (uW) / 1000 */\n> >>>>>> -\t\t\t*val = chip->config->power_lsb;\n> >>>>>> +\t\t\t*val = chip->power_lsb;\n> >>>>>> \t\t\t*val2 = 1000;\n> >>>>>> \t\t\treturn IIO_VAL_FRACTIONAL;\n> >>>>>> \n> >>>>>> \t\tcase INA2XX_CURRENT:\n> >>>>>> -\t\t\t/* processed (mA) = raw (mA) */\n> >>>>>> -\t\t\t*val = 1;\n> >>>>>> -\t\t\treturn IIO_VAL_INT;\n> >>>>>> +\t\t\t/* processed (mA) = raw*lsb (uA) / 1000 */\n> >>>>>> +\t\t\t*val = chip->current_lsb;\n> >>>>>> +\t\t\t*val2 = 1000;\n> >>>>>> +\t\t\treturn IIO_VAL_FRACTIONAL;\n> >>>>>> \t\t}\n> >>>>>> \t}\n> >>>>>> \n> >>>>>> @@ -434,24 +439,47 @@ static ssize_t ina2xx_allow_async_readout_store(struct device *dev,\n> >>>>>> }\n> >>>>>> \n> >>>>>> /*\n> >>>>>> - * Set current LSB to 1mA, shunt is in uOhms\n> >>>>>> - * (equation 13 in datasheet). We hardcode a Current_LSB\n> >>>>>> - * of 1.0 x10-6. The only remaining parameter is RShunt.\n> >>>>>> + * Calculate calibration value according to equation 1 in ina226 datasheet\n> >>>>>> + * http://www.ti.com/lit/ds/symlink/ina226.pdf.\n> >>>>>> + * Current LSB is in uA and RShunt is in uOhms, so RShunt should be\n> >>>>>> + * converted to mOhms in order to keep the scale.\n> >>>>>> * There is no need to expose the CALIBRATION register\n> >>>>>> * to the user for now. But we need to reset this register\n> >>>>>> - * if the user updates RShunt after driver init, e.g upon\n> >>>>>> - * reading an EEPROM/Probe-type value.\n> >>>>>> + * if the user updates RShunt or max expected current after driver\n> >>>>>> + * init, e.g upon reading an EEPROM/Probe-type value.\n> >>>>>> */\n> >>>>>> static int ina2xx_set_calibration(struct ina2xx_chip_info *chip)\n> >>>>>> {\n> >>>>>> +\tunsigned int rshunt = DIV_ROUND_CLOSEST(chip->shunt_resistor, 1000);\n> >>>>>> \tu16 regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,\n> >>>>>> -\t\t\t\t chip->shunt_resistor);\n> >>>>>> +\t\t\t\t chip->current_lsb * rshunt);\n> >>>>>> \n> >>>>>> \treturn regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);\n> >>>>>> }\n> >>>>>> \n> >>>>>> +/*\n> >>>>>> + * Set max_expected_current (mA) and calculate current_lsb (uA),\n> >>>>>> + * according to equation 2 in ina226 datasheet. Power LSB is calculated\n> >>>>>> + * by multiplying Current LSB by a given factor, which may vary depending\n> >>>>>> + * on ina version.\n> >>>>>> + */\n> >>>>>> +static int set_max_expected_current(struct ina2xx_chip_info *chip,\n> >>>>>> +\t\t\t\t unsigned int val)\n> >>>>>> +{\n> >>>>>> +\tif (val <= 0 || val > chip->config->calibration_factor)\n> >>>>>> +\t\treturn -EINVAL;\n> >>>>>> +\n> >>>>>> +\tchip->max_expected_current = val;\n> >>>>>> +\tchip->current_lsb = DIV_ROUND_CLOSEST(chip->max_expected_current * 1000,\n> >>>>>> +\t\t\t\t\t 1 << 15);\n> >>>>>> +\tchip->power_lsb = chip->current_lsb * chip->config->power_lsb_factor;\n> >>>>>> +\n> >>>>>> +\treturn 0;\n> >>>>>> +}\n> >>>>>> +\n> >>>>>> static int set_shunt_resistor(struct ina2xx_chip_info *chip, unsigned int val)\n> >>>>>> {\n> >>>>>> +\n> >>>>>> \tif (val <= 0 || val > chip->config->calibration_factor)\n> >>>>>> \t\treturn -EINVAL;\n> >>>>>> \n> >>>>>> @@ -493,6 +521,39 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,\n> >>>>>> \treturn len;\n> >>>>>> }\n> >>>>>> \n> >>>>>> +static ssize_t ina2xx_max_expected_current_show(struct device *dev,\n> >>>>>> +\t\t\t\t\t struct device_attribute *attr,\n> >>>>>> +\t\t\t\t\t char *buf)\n> >>>>>> +{\n> >>>>>> +\tstruct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));\n> >>>>>> +\n> >>>>>> +\treturn sprintf(buf, \"%d\\n\", chip->max_expected_current);\n> >>>>>> +}\n> >>>>>> +\n> >>>>>> +static ssize_t ina2xx_max_expected_current_store(struct device *dev,\n> >>>>>> +\t\t\t\t\t struct device_attribute *attr,\n> >>>>>> +\t\t\t\t\t const char *buf, size_t len)\n> >>>>>> +{\n> >>>>>> +\tstruct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));\n> >>>>>> +\tunsigned long val;\n> >>>>>> +\tint ret;\n> >>>>>> +\n> >>>>>> +\tret = kstrtoul((const char *) buf, 10, &val); \n> >>>>>\n> >>>>> Odd bit of casting given that's what it already is...\n> >>>>> \n> >>>>>> +\tif (ret)\n> >>>>>> +\t\treturn ret;\n> >>>>>> +\n> >>>>>> +\tret = set_max_expected_current(chip, val);\n> >>>>>> +\tif (ret)\n> >>>>>> +\t\treturn ret;\n> >>>>>> +\n> >>>>>> +\t/* Update the Calibration register */\n> >>>>>> +\tret = ina2xx_set_calibration(chip);\n> >>>>>> +\tif (ret)\n> >>>>>> +\t\treturn ret;\n> >>>>>> +\n> >>>>>> +\treturn len;\n> >>>>>> +}\n> >>>>>> +\n> >>>>>> #define INA219_CHAN(_type, _index, _address) { \\\n> >>>>>> \t.type = (_type), \\\n> >>>>>> \t.address = (_address), \\\n> >>>>>> @@ -755,10 +816,15 @@ static IIO_DEVICE_ATTR(in_shunt_resistor, S_IRUGO | S_IWUSR,\n> >>>>>> \t\t ina2xx_shunt_resistor_show,\n> >>>>>> \t\t ina2xx_shunt_resistor_store, 0);\n> >>>>>> \n> >>>>>> +static IIO_DEVICE_ATTR(in_max_expected_current, S_IRUGO | S_IWUSR,\n> >>>>>> +\t\t ina2xx_max_expected_current_show,\n> >>>>>> +\t\t ina2xx_max_expected_current_store, 0);\n> >>>>>> +\n> >>>>>> static struct attribute *ina219_attributes[] = {\n> >>>>>> \t&iio_dev_attr_in_allow_async_readout.dev_attr.attr,\n> >>>>>> \t&iio_const_attr_ina219_integration_time_available.dev_attr.attr,\n> >>>>>> \t&iio_dev_attr_in_shunt_resistor.dev_attr.attr,\n> >>>>>> +\t&iio_dev_attr_in_max_expected_current.dev_attr.attr,\n> >>>>>> \tNULL,\n> >>>>>> };\n> >>>>>> \n> >>>>>> @@ -766,6 +832,7 @@ static struct attribute *ina226_attributes[] = {\n> >>>>>> \t&iio_dev_attr_in_allow_async_readout.dev_attr.attr,\n> >>>>>> \t&iio_const_attr_ina226_integration_time_available.dev_attr.attr,\n> >>>>>> \t&iio_dev_attr_in_shunt_resistor.dev_attr.attr,\n> >>>>>> +\t&iio_dev_attr_in_max_expected_current.dev_attr.attr,\n> >>>>>> \tNULL,\n> >>>>>> };\n> >>>>>> \n> >>>>>> @@ -851,6 +918,21 @@ static int ina2xx_probe(struct i2c_client *client,\n> >>>>>> \tif (ret)\n> >>>>>> \t\treturn ret;\n> >>>>>> \n> >>>>>> +\tif (of_property_read_u32(client->dev.of_node,\n> >>>>>> +\t\t\t\t \"max-expected-current\", &val) < 0) {\n> >>>>>> +\t\tstruct ina2xx_platform_data *pdata =\n> >>>>>> +\t\t dev_get_platdata(&client->dev);\n> >>>>>> +\n> >>>>>> +\t\tif (pdata && pdata->max_mA != 0)\n> >>>>>> +\t\t\tval = pdata->max_mA;\n> >>>>>> +\t\telse\n> >>>>>> +\t\t\tval = INA2XX_MAX_EXPECTED_A_DEFAULT;\n> >>>>>> +\t}\n> >>>>>> +\n> >>>>>> +\tret = set_max_expected_current(chip, val);\n> >>>>>> +\tif (ret)\n> >>>>>> +\t\treturn ret;\n> >>>>>> +\n> >>>>>> \t/* Patch the current config register with default. */\n> >>>>>> \tval = chip->config->config_default;\n> >>>>>> \n> >>>>>> diff --git a/include/linux/platform_data/ina2xx.h b/include/linux/platform_data/ina2xx.h\n> >>>>>> index 9abc0ca..f02b1d8 100644\n> >>>>>> --- a/include/linux/platform_data/ina2xx.h\n> >>>>>> +++ b/include/linux/platform_data/ina2xx.h\n> >>>>>> @@ -13,7 +13,9 @@\n> >>>>>> /**\n> >>>>>> * struct ina2xx_platform_data - ina2xx info\n> >>>>>> * @shunt_uohms\t\tshunt resistance in microohms\n> >>>>>> + * @max_mA\t\tmax expected current in mA\n> >>>>>> */\n> >>>>>> struct ina2xx_platform_data {\n> >>>>>> \tlong shunt_uohms;\n> >>>>>> +\tint max_mA;\n> >>>>>> }; \n> >>>>>\n> >>>>>\n> >>>>>\n> >>>>> \n> >>>\n> >>>\n> >>>\n> >>> \n> >> --\n> >> To unsubscribe from this list: send the line \"unsubscribe linux-iio\" in\n> >> the body of a message to majordomo@vger.kernel.org\n> >> More majordomo info at http://vger.kernel.org/majordomo-info.html \n> > \n> > \n> > \n> > \n> --\n> To unsubscribe from this list: send the line \"unsubscribe linux-iio\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yBzH539J7z9s83\n\tfor ;\n\tThu, 12 Oct 2017 03:01:05 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1757254AbdJKQBD (ORCPT\n\t);\n\tWed, 11 Oct 2017 12:01:03 -0400","from szxga04-in.huawei.com ([45.249.212.190]:7973 \"EHLO\n\tszxga04-in.huawei.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751932AbdJKQBA (ORCPT\n\t); Wed, 11 Oct 2017 12:01:00 -0400","from 172.30.72.60 (EHLO DGGEMS401-HUB.china.huawei.com)\n\t([172.30.72.60])\n\tby dggrg04-dlp.huawei.com (MOS 4.4.6-GA FastPath queued)\n\twith ESMTP id DIW25202; Thu, 12 Oct 2017 00:00:07 +0800 (CST)","from localhost (10.206.48.115) by DGGEMS401-HUB.china.huawei.com\n\t(10.3.19.201) with Microsoft SMTP Server (TLS) id 14.3.301.0;\n\tWed, 11 Oct 2017 23:59:54 +0800"],"Date":"Wed, 11 Oct 2017 16:59:41 +0100","From":"Jonathan Cameron ","To":"Maciej Purski ","CC":"Jonathan Cameron , ,\n\t, ,\n\t,\n\t, ,\n\tRob Herring , Mark Rutland ,\n\tGuenter Roeck , Jean Delvare , \n\tJonathan Corbet , Russell King ,\n\tKukjin Kim , Krzysztof Kozlowski , \n\tHartmut Knaack , Lars-Peter Clausen ,\n\tPeter Meerwald-Stadler ,\n\t\"Bartlomiej Zolnierkiewicz\" ,\n\tMarek Szyprowski ","Subject":"Re: [PATCH 1/4] iio: adc: ina2xx: Make max expected current\n\tconfigurable","Message-ID":"<20171011165941.00006914@huawei.com>","In-Reply-To":"<3a64c5a7-6acb-41dd-a18d-6b92b2bef560@samsung.com>","References":"<1506603015-27202-1-git-send-email-m.purski@samsung.com>\n\t\n\t<1506603015-27202-2-git-send-email-m.purski@samsung.com>\n\t<20171001112934.360c840b@archlinux>\n\t\n\t<20171008104756.6eaa62c5@archlinux>\n\t<662441b9-1b04-3e8b-baac-5852ba873ca5@samsung.com>\n\t<20171009143539.00002222@huawei.com>\n\t<3a64c5a7-6acb-41dd-a18d-6b92b2bef560@samsung.com>","Organization":"Huawei","X-Mailer":"Claws Mail 3.15.0 (GTK+ 2.24.31; x86_64-w64-mingw32)","MIME-Version":"1.0","Content-Type":"text/plain; charset=\"US-ASCII\"","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[10.206.48.115]","X-CFilter-Loop":"Reflected","X-Mirapoint-Virus-RAPID-Raw":"score=unknown(0),\n\trefid=str=0001.0A020202.59DE400B.00DA, ss=1, re=0.000, recu=0.000,\n\treip=0.000, cl=1, cld=1, fgs=0, ip=0.0.0.0,\n\tso=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32","X-Mirapoint-Loop-Id":"8f3449373ea3bd23a951fcd2c9055bc1","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}}]