Re: [PATCH v3] power/supply: Add ltc4162-l-charger

2021-01-04 Thread Mike Looijmans

Hello,

No worries on processing time. Only drawback is that since the project 
ended, I may no longer have access to the actual hardware for testing.


Further inline comments below.


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijm...@topicproducts.com
W: www.topicproducts.com

Please consider the environment before printing this e-mail
On 29-12-2020 15:57, Sebastian Reichel wrote:

Hi,

Sorry for slow processing of this. Driver looks mostly fine, but I
do have some comments.

On Mon, Nov 02, 2020 at 10:21:31AM +0100, Mike Looijmans wrote:

Add support for the LTC4162-L Li-Ion battery charger. The driver allows
reading back telemetry and to set some charging options like the input
current limit.

Signed-off-by: Mike Looijmans 
---
v2: Use microohm units instead of milliohm
 Add interrupt support using smbalert
 Support obtaining cell-count from devicetree
v3: Fix overflows in calculations involving resistor values
resent, mail bounced

  drivers/power/supply/Kconfig |   8 +
  drivers/power/supply/Makefile|   1 +
  drivers/power/supply/ltc4162-l-charger.c | 898 +++
  3 files changed, 907 insertions(+)
  create mode 100644 drivers/power/supply/ltc4162-l-charger.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index eec646c568b7..23000976cb42 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -513,6 +513,14 @@ config CHARGER_LT3651
  Say Y to include support for the Analog Devices (Linear Technology)
  LT3651 battery charger which reports its status via GPIO lines.
  
+config CHARGER_LTC4162L

+   tristate "LTC4162-L charger"
+   depends on I2C
+   select REGMAP_I2C
+   help
+ Say Y to include support for the Analog Devices (Linear Technology)
+ LTC4162-L battery charger connected to I2C.
+
  config CHARGER_MAX14577
tristate "Maxim MAX14577/77836 battery charger driver"
depends on MFD_MAX14577
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index dd4b86318cd9..17b1cf921c44 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_CHARGER_LP8788)  += lp8788-charger.o
  obj-$(CONFIG_CHARGER_GPIO)+= gpio-charger.o
  obj-$(CONFIG_CHARGER_MANAGER) += charger-manager.o
  obj-$(CONFIG_CHARGER_LT3651)  += lt3651-charger.o
+obj-$(CONFIG_CHARGER_LTC4162L) += ltc4162-l-charger.o
  obj-$(CONFIG_CHARGER_MAX14577)+= max14577_charger.o
  obj-$(CONFIG_CHARGER_DETECTOR_MAX14656)   += max14656_charger_detector.o
  obj-$(CONFIG_CHARGER_MAX77650)+= max77650-charger.o
diff --git a/drivers/power/supply/ltc4162-l-charger.c 
b/drivers/power/supply/ltc4162-l-charger.c
new file mode 100644
index ..b2f666113125
--- /dev/null
+++ b/drivers/power/supply/ltc4162-l-charger.c
@@ -0,0 +1,898 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *  Driver for Analog Devices (Linear Technology) LTC4162-L charger IC.
+ *  Copyright (C) 2020, Topic Embedded Products
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Registers (names based on what datasheet uses) */
+#define LTC4162L_EN_LIMIT_ALERTS_REG   0x0D
+#define LTC4162L_EN_CHARGER_STATE_ALERTS_REG   0x0E
+#define LTC4162L_EN_CHARGE_STATUS_ALERTS_REG   0x0F
+#define LTC4162L_CONFIG_BITS_REG   0x14
+#define LTC4162L_IIN_LIMIT_TARGET  0x15
+#define LTC4162L_ARM_SHIP_MODE 0x19
+#define LTC4162L_CHARGE_CURRENT_SETTING0X1A
+#define LTC4162L_VCHARGE_SETTING   0X1B
+#define LTC4162L_C_OVER_X_THRESHOLD0x1C
+#define LTC4162L_MAX_CV_TIME   0X1D
+#define LTC4162L_MAX_CHARGE_TIME   0X1E
+#define LTC4162L_CHARGER_CONFIG_BITS   0x29
+#define LTC4162L_CHARGER_STATE 0x34
+#define LTC4162L_CHARGE_STATUS 0x35
+#define LTC4162L_LIMIT_ALERTS_REG  0x36
+#define LTC4162L_CHARGER_STATE_ALERTS_REG  0x37
+#define LTC4162L_CHARGE_STATUS_ALERTS_REG  0x38
+#define LTC4162L_SYSTEM_STATUS_REG 0x39
+#define LTC4162L_VBAT  0x3A
+#define LTC4162L_VIN   0x3B
+#define LTC4162L_VOUT  0x3C
+#define LTC4162L_IBAT  0x3D
+#define LTC4162L_IIN   0x3E
+#define LTC4162L_DIE_TEMPERATURE   0x3F
+#define LTC4162L_THERMISTOR_VOLTAGE0x40
+#define LTC4162L_BSR   0x41
+#define LTC4162L_JEITA_REGION  0x42
+#define LTC4162L_CHEM_CELLS_REG0x43
+#define LTC4162L_ICHARGE_DAC   0x44
+#define LTC4162L_VCHARGE_DAC   0x45
+#define LTC4162L_IIN_LIMIT_DAC 

Re: [PATCH v3] power/supply: Add ltc4162-l-charger

2020-12-29 Thread Sebastian Reichel
Hi,

Sorry for slow processing of this. Driver looks mostly fine, but I
do have some comments.

On Mon, Nov 02, 2020 at 10:21:31AM +0100, Mike Looijmans wrote:
> Add support for the LTC4162-L Li-Ion battery charger. The driver allows
> reading back telemetry and to set some charging options like the input
> current limit.
> 
> Signed-off-by: Mike Looijmans 
> ---
> v2: Use microohm units instead of milliohm
> Add interrupt support using smbalert
> Support obtaining cell-count from devicetree
> v3: Fix overflows in calculations involving resistor values
> resent, mail bounced
> 
>  drivers/power/supply/Kconfig |   8 +
>  drivers/power/supply/Makefile|   1 +
>  drivers/power/supply/ltc4162-l-charger.c | 898 +++
>  3 files changed, 907 insertions(+)
>  create mode 100644 drivers/power/supply/ltc4162-l-charger.c
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index eec646c568b7..23000976cb42 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -513,6 +513,14 @@ config CHARGER_LT3651
> Say Y to include support for the Analog Devices (Linear Technology)
> LT3651 battery charger which reports its status via GPIO lines.
>  
> +config CHARGER_LTC4162L
> + tristate "LTC4162-L charger"
> + depends on I2C
> + select REGMAP_I2C
> + help
> +   Say Y to include support for the Analog Devices (Linear Technology)
> +   LTC4162-L battery charger connected to I2C.
> +
>  config CHARGER_MAX14577
>   tristate "Maxim MAX14577/77836 battery charger driver"
>   depends on MFD_MAX14577
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index dd4b86318cd9..17b1cf921c44 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -70,6 +70,7 @@ obj-$(CONFIG_CHARGER_LP8788)+= lp8788-charger.o
>  obj-$(CONFIG_CHARGER_GPIO)   += gpio-charger.o
>  obj-$(CONFIG_CHARGER_MANAGER)+= charger-manager.o
>  obj-$(CONFIG_CHARGER_LT3651) += lt3651-charger.o
> +obj-$(CONFIG_CHARGER_LTC4162L)   += ltc4162-l-charger.o
>  obj-$(CONFIG_CHARGER_MAX14577)   += max14577_charger.o
>  obj-$(CONFIG_CHARGER_DETECTOR_MAX14656)  += max14656_charger_detector.o
>  obj-$(CONFIG_CHARGER_MAX77650)   += max77650-charger.o
> diff --git a/drivers/power/supply/ltc4162-l-charger.c 
> b/drivers/power/supply/ltc4162-l-charger.c
> new file mode 100644
> index ..b2f666113125
> --- /dev/null
> +++ b/drivers/power/supply/ltc4162-l-charger.c
> @@ -0,0 +1,898 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + *  Driver for Analog Devices (Linear Technology) LTC4162-L charger IC.
> + *  Copyright (C) 2020, Topic Embedded Products
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* Registers (names based on what datasheet uses) */
> +#define LTC4162L_EN_LIMIT_ALERTS_REG 0x0D
> +#define LTC4162L_EN_CHARGER_STATE_ALERTS_REG 0x0E
> +#define LTC4162L_EN_CHARGE_STATUS_ALERTS_REG 0x0F
> +#define LTC4162L_CONFIG_BITS_REG 0x14
> +#define LTC4162L_IIN_LIMIT_TARGET0x15
> +#define LTC4162L_ARM_SHIP_MODE   0x19
> +#define LTC4162L_CHARGE_CURRENT_SETTING  0X1A
> +#define LTC4162L_VCHARGE_SETTING 0X1B
> +#define LTC4162L_C_OVER_X_THRESHOLD  0x1C
> +#define LTC4162L_MAX_CV_TIME 0X1D
> +#define LTC4162L_MAX_CHARGE_TIME 0X1E
> +#define LTC4162L_CHARGER_CONFIG_BITS 0x29
> +#define LTC4162L_CHARGER_STATE   0x34
> +#define LTC4162L_CHARGE_STATUS   0x35
> +#define LTC4162L_LIMIT_ALERTS_REG0x36
> +#define LTC4162L_CHARGER_STATE_ALERTS_REG0x37
> +#define LTC4162L_CHARGE_STATUS_ALERTS_REG0x38
> +#define LTC4162L_SYSTEM_STATUS_REG   0x39
> +#define LTC4162L_VBAT0x3A
> +#define LTC4162L_VIN 0x3B
> +#define LTC4162L_VOUT0x3C
> +#define LTC4162L_IBAT0x3D
> +#define LTC4162L_IIN 0x3E
> +#define LTC4162L_DIE_TEMPERATURE 0x3F
> +#define LTC4162L_THERMISTOR_VOLTAGE  0x40
> +#define LTC4162L_BSR 0x41
> +#define LTC4162L_JEITA_REGION0x42
> +#define LTC4162L_CHEM_CELLS_REG  0x43
> +#define LTC4162L_ICHARGE_DAC 0x44
> +#define LTC4162L_VCHARGE_DAC 0x45
> +#define LTC4162L_IIN_LIMIT_DAC   0x46
> +#define LTC4162L_VBAT_FILT   0x47
> +#define LTC4162L_INPUT_UNDERVOLTAGE_DAC  0x4B
> +
> +/* Enumeration as in datasheet. Individual bits are mutually exclusive. */
> +enum ltc4162l_state {
> + battery_detection = 2048,
> + charger_suspended = 256,
> + precharge = 128,   /* trickle on low bat 

Re: [PATCH v3] power/supply: Add ltc4162-l-charger

2020-12-03 Thread Mike Looijmans

Gentle ping, haven't seen any response...


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijm...@topicproducts.com
W: www.topicproducts.com

Please consider the environment before printing this e-mail
On 02-11-2020 10:21, Mike Looijmans wrote:

Add support for the LTC4162-L Li-Ion battery charger. The driver allows
reading back telemetry and to set some charging options like the input
current limit.

Signed-off-by: Mike Looijmans 
---
v2: Use microohm units instead of milliohm
 Add interrupt support using smbalert
 Support obtaining cell-count from devicetree
v3: Fix overflows in calculations involving resistor values
resent, mail bounced

  drivers/power/supply/Kconfig |   8 +
  drivers/power/supply/Makefile|   1 +
  drivers/power/supply/ltc4162-l-charger.c | 898 +++
  3 files changed, 907 insertions(+)
  create mode 100644 drivers/power/supply/ltc4162-l-charger.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index eec646c568b7..23000976cb42 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -513,6 +513,14 @@ config CHARGER_LT3651
  Say Y to include support for the Analog Devices (Linear Technology)
  LT3651 battery charger which reports its status via GPIO lines.
  
+config CHARGER_LTC4162L

+   tristate "LTC4162-L charger"
+   depends on I2C
+   select REGMAP_I2C
+   help
+ Say Y to include support for the Analog Devices (Linear Technology)
+ LTC4162-L battery charger connected to I2C.
+
  config CHARGER_MAX14577
tristate "Maxim MAX14577/77836 battery charger driver"
depends on MFD_MAX14577
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index dd4b86318cd9..17b1cf921c44 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_CHARGER_LP8788)  += lp8788-charger.o
  obj-$(CONFIG_CHARGER_GPIO)+= gpio-charger.o
  obj-$(CONFIG_CHARGER_MANAGER) += charger-manager.o
  obj-$(CONFIG_CHARGER_LT3651)  += lt3651-charger.o
+obj-$(CONFIG_CHARGER_LTC4162L) += ltc4162-l-charger.o
  obj-$(CONFIG_CHARGER_MAX14577)+= max14577_charger.o
  obj-$(CONFIG_CHARGER_DETECTOR_MAX14656)   += max14656_charger_detector.o
  obj-$(CONFIG_CHARGER_MAX77650)+= max77650-charger.o
diff --git a/drivers/power/supply/ltc4162-l-charger.c 
b/drivers/power/supply/ltc4162-l-charger.c
new file mode 100644
index ..b2f666113125
--- /dev/null
+++ b/drivers/power/supply/ltc4162-l-charger.c
@@ -0,0 +1,898 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *  Driver for Analog Devices (Linear Technology) LTC4162-L charger IC.
+ *  Copyright (C) 2020, Topic Embedded Products
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Registers (names based on what datasheet uses) */
+#define LTC4162L_EN_LIMIT_ALERTS_REG   0x0D
+#define LTC4162L_EN_CHARGER_STATE_ALERTS_REG   0x0E
+#define LTC4162L_EN_CHARGE_STATUS_ALERTS_REG   0x0F
+#define LTC4162L_CONFIG_BITS_REG   0x14
+#define LTC4162L_IIN_LIMIT_TARGET  0x15
+#define LTC4162L_ARM_SHIP_MODE 0x19
+#define LTC4162L_CHARGE_CURRENT_SETTING0X1A
+#define LTC4162L_VCHARGE_SETTING   0X1B
+#define LTC4162L_C_OVER_X_THRESHOLD0x1C
+#define LTC4162L_MAX_CV_TIME   0X1D
+#define LTC4162L_MAX_CHARGE_TIME   0X1E
+#define LTC4162L_CHARGER_CONFIG_BITS   0x29
+#define LTC4162L_CHARGER_STATE 0x34
+#define LTC4162L_CHARGE_STATUS 0x35
+#define LTC4162L_LIMIT_ALERTS_REG  0x36
+#define LTC4162L_CHARGER_STATE_ALERTS_REG  0x37
+#define LTC4162L_CHARGE_STATUS_ALERTS_REG  0x38
+#define LTC4162L_SYSTEM_STATUS_REG 0x39
+#define LTC4162L_VBAT  0x3A
+#define LTC4162L_VIN   0x3B
+#define LTC4162L_VOUT  0x3C
+#define LTC4162L_IBAT  0x3D
+#define LTC4162L_IIN   0x3E
+#define LTC4162L_DIE_TEMPERATURE   0x3F
+#define LTC4162L_THERMISTOR_VOLTAGE0x40
+#define LTC4162L_BSR   0x41
+#define LTC4162L_JEITA_REGION  0x42
+#define LTC4162L_CHEM_CELLS_REG0x43
+#define LTC4162L_ICHARGE_DAC   0x44
+#define LTC4162L_VCHARGE_DAC   0x45
+#define LTC4162L_IIN_LIMIT_DAC 0x46
+#define LTC4162L_VBAT_FILT 0x47
+#define LTC4162L_INPUT_UNDERVOLTAGE_DAC0x4B
+
+/* Enumeration as in datasheet. Individual bits are mutually exclusive. */
+enum ltc4162l_state {
+   battery_detection = 2048,
+   charger_suspended = 256,
+   precharge 

[PATCH v3] power/supply: Add ltc4162-l-charger

2020-11-02 Thread Mike Looijmans
Add support for the LTC4162-L Li-Ion battery charger. The driver allows
reading back telemetry and to set some charging options like the input
current limit.

Signed-off-by: Mike Looijmans 
---
v2: Use microohm units instead of milliohm
Add interrupt support using smbalert
Support obtaining cell-count from devicetree
v3: Fix overflows in calculations involving resistor values
resent, mail bounced

 drivers/power/supply/Kconfig |   8 +
 drivers/power/supply/Makefile|   1 +
 drivers/power/supply/ltc4162-l-charger.c | 898 +++
 3 files changed, 907 insertions(+)
 create mode 100644 drivers/power/supply/ltc4162-l-charger.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index eec646c568b7..23000976cb42 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -513,6 +513,14 @@ config CHARGER_LT3651
  Say Y to include support for the Analog Devices (Linear Technology)
  LT3651 battery charger which reports its status via GPIO lines.
 
+config CHARGER_LTC4162L
+   tristate "LTC4162-L charger"
+   depends on I2C
+   select REGMAP_I2C
+   help
+ Say Y to include support for the Analog Devices (Linear Technology)
+ LTC4162-L battery charger connected to I2C.
+
 config CHARGER_MAX14577
tristate "Maxim MAX14577/77836 battery charger driver"
depends on MFD_MAX14577
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index dd4b86318cd9..17b1cf921c44 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_CHARGER_LP8788)  += lp8788-charger.o
 obj-$(CONFIG_CHARGER_GPIO) += gpio-charger.o
 obj-$(CONFIG_CHARGER_MANAGER)  += charger-manager.o
 obj-$(CONFIG_CHARGER_LT3651)   += lt3651-charger.o
+obj-$(CONFIG_CHARGER_LTC4162L) += ltc4162-l-charger.o
 obj-$(CONFIG_CHARGER_MAX14577) += max14577_charger.o
 obj-$(CONFIG_CHARGER_DETECTOR_MAX14656)+= max14656_charger_detector.o
 obj-$(CONFIG_CHARGER_MAX77650) += max77650-charger.o
diff --git a/drivers/power/supply/ltc4162-l-charger.c 
b/drivers/power/supply/ltc4162-l-charger.c
new file mode 100644
index ..b2f666113125
--- /dev/null
+++ b/drivers/power/supply/ltc4162-l-charger.c
@@ -0,0 +1,898 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *  Driver for Analog Devices (Linear Technology) LTC4162-L charger IC.
+ *  Copyright (C) 2020, Topic Embedded Products
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Registers (names based on what datasheet uses) */
+#define LTC4162L_EN_LIMIT_ALERTS_REG   0x0D
+#define LTC4162L_EN_CHARGER_STATE_ALERTS_REG   0x0E
+#define LTC4162L_EN_CHARGE_STATUS_ALERTS_REG   0x0F
+#define LTC4162L_CONFIG_BITS_REG   0x14
+#define LTC4162L_IIN_LIMIT_TARGET  0x15
+#define LTC4162L_ARM_SHIP_MODE 0x19
+#define LTC4162L_CHARGE_CURRENT_SETTING0X1A
+#define LTC4162L_VCHARGE_SETTING   0X1B
+#define LTC4162L_C_OVER_X_THRESHOLD0x1C
+#define LTC4162L_MAX_CV_TIME   0X1D
+#define LTC4162L_MAX_CHARGE_TIME   0X1E
+#define LTC4162L_CHARGER_CONFIG_BITS   0x29
+#define LTC4162L_CHARGER_STATE 0x34
+#define LTC4162L_CHARGE_STATUS 0x35
+#define LTC4162L_LIMIT_ALERTS_REG  0x36
+#define LTC4162L_CHARGER_STATE_ALERTS_REG  0x37
+#define LTC4162L_CHARGE_STATUS_ALERTS_REG  0x38
+#define LTC4162L_SYSTEM_STATUS_REG 0x39
+#define LTC4162L_VBAT  0x3A
+#define LTC4162L_VIN   0x3B
+#define LTC4162L_VOUT  0x3C
+#define LTC4162L_IBAT  0x3D
+#define LTC4162L_IIN   0x3E
+#define LTC4162L_DIE_TEMPERATURE   0x3F
+#define LTC4162L_THERMISTOR_VOLTAGE0x40
+#define LTC4162L_BSR   0x41
+#define LTC4162L_JEITA_REGION  0x42
+#define LTC4162L_CHEM_CELLS_REG0x43
+#define LTC4162L_ICHARGE_DAC   0x44
+#define LTC4162L_VCHARGE_DAC   0x45
+#define LTC4162L_IIN_LIMIT_DAC 0x46
+#define LTC4162L_VBAT_FILT 0x47
+#define LTC4162L_INPUT_UNDERVOLTAGE_DAC0x4B
+
+/* Enumeration as in datasheet. Individual bits are mutually exclusive. */
+enum ltc4162l_state {
+   battery_detection = 2048,
+   charger_suspended = 256,
+   precharge = 128,   /* trickle on low bat voltage */
+   cc_cv_charge = 64, /* normal charge */
+   ntc_pause = 32,
+   timer_term = 16,
+   c_over_x_term = 8, /* battery is full */
+   max_charge_time_fault = 4,
+   bat_missing_fault = 2,
+   bat_short_fault = 1
+};
+
+/* Individual bits are mutually exclusive. Only active in charging states.*/
+enum ltc4162l_charge_status {
+