Re: [PATCH] power: supply: bq27xxx: Return the value instead of -ENODATA

2021-03-31 Thread Hermes Zhang
On 3/31/21 10:02 PM, Pali Rohár wrote:
> @@ -1655,9 +1655,6 @@ static int bq27xxx_battery_read_time(struct 
> bq27xxx_device_info *di, u8 reg)
>   return tval;
>   }
>  
> - if (tval == 65535)
> - return -ENODATA;
> -
>   return tval * 60;
> I'm not sure if this is correct change. If value 65535 is special which
> indicates that data are not available then driver should not return
> (converted) value 65535*60. If -ENODATA is there to indicate that data
> are not available then -ENODATA should not be used.
>
> And if there is application which does not handle -ENODATA for state
> when data are not available then it is a bug in application.

Yeah, I just have a feeling return -ENODATA for time_to_full/empty is
not good here. Because:

1. From chip datasheet, it mentioned return 65535 when it's not
available (e.g. read time_to_full when discharging), but the driver
changes behavior here.

2. There is other case will return -ENODATA (e.g. the gauge not
calibrated), so it will confuse application which is real failure.

Could we change the value in minute instead of seconds in
bq27xxx_battery_read_time(), so that means driver do nothing but only
pass the value from the chip?


Best Regards,

Hermes




[PATCH] power: supply: bq27xxx: Return the value instead of -ENODATA

2021-03-31 Thread Hermes Zhang
From: Hermes Zhang 

It might be better to return value (e.g. 65535) instead of an error
(e.g. No data available) for the time property.

Normally a common function will handle the read string and parse to
integer for all the properties, but will have problem when read the
time property because need to handle the NODATA error as non-error.
So it will make simple for application which indicate success when
read a number, otherwise as an error to handle.

Signed-off-by: Hermes Zhang 
---
 drivers/power/supply/bq27xxx_battery.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/power/supply/bq27xxx_battery.c 
b/drivers/power/supply/bq27xxx_battery.c
index 4c4a7b1c64c5..b75e54aa8ada 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -1655,9 +1655,6 @@ static int bq27xxx_battery_read_time(struct 
bq27xxx_device_info *di, u8 reg)
return tval;
}
 
-   if (tval == 65535)
-   return -ENODATA;
-
return tval * 60;
 }
 
-- 
2.20.1



Re: [PATCH v2 1/2] dt-binding: leds: Document leds-multi-gpio bindings

2021-03-29 Thread Hermes Zhang
On 3/28/21 2:12 AM, Rob Herring wrote:
>> +
>> +  led-gpios:
>> +description: Array of one or more GPIOs pins used to control the LED.
>> +minItems: 1
>> +maxItems: 8  # Should be enough
>> +
>> +  led-states:
>> +description: |
>> +  The array list the supported states here which will map to brightness
>> +  from 0 to maximum. Each item in the array will present all the GPIOs
>> +  value by bit.
>> +$ref: /schemas/types.yaml#/definitions/uint8-array
>> +minItems: 1
>> +maxItems: 256 # Should be enough
> Isn't this the same as the standard 'brightness-levels' from backlight 
> binding? The index is the level and the value is the h/w specific 
> setting.

Yes, it seems same.


Best Regards,

Hermes



Re: [PATCH v2 2/2] leds: leds-multi-gpio: Add multiple GPIOs LED driver

2021-03-29 Thread Hermes Zhang
On 3/26/21 9:49 PM, Pavel Machek wrote:
>> +of_property_read_string(node, "default-state", );
>> +if (!strcmp(state, "on"))
>> +multi_gpio_led_set(>cdev, priv->cdev.max_brightness);
>> +else
>> +multi_gpio_led_set(>cdev, 0);
> No need for default-state handling, unless you are using it.
>
We will use it, to make the LED default on or off.


Best Regards,

Hermes



[PATCH v3] leds: leds-multi-gpio: Add multiple GPIOs LED driver

2021-03-29 Thread Hermes Zhang
From: Hermes Zhang 

Introduce a new multiple GPIOs LED driver. This LED will made of
multiple GPIOs (up to 8) and will map different brightness to different
GPIOs states which defined in dts file.

Signed-off-by: Hermes Zhang 
---

Notes:
changes v3:
- Remove LEDS_SIMPLE menu
- Minro code style fix

changes v2:
- use max_brightness(2^gpio numbers) instead of LED_FULL
- malloc the priv in once
- move the code to simple folder

 drivers/leds/Kconfig  |   3 +
 drivers/leds/Makefile |   3 +
 drivers/leds/simple/Kconfig   |  12 +++
 drivers/leds/simple/Makefile  |   3 +
 drivers/leds/simple/leds-multi-gpio.c | 144 ++
 5 files changed, 165 insertions(+)
 create mode 100644 drivers/leds/simple/Kconfig
 create mode 100644 drivers/leds/simple/Makefile
 create mode 100644 drivers/leds/simple/leds-multi-gpio.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index b6742b4231bf..f95217b2fcf1 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -937,4 +937,7 @@ source "drivers/leds/trigger/Kconfig"
 comment "LED Blink"
 source "drivers/leds/blink/Kconfig"
 
+comment "LED Simple"
+source "drivers/leds/simple/Kconfig"
+
 endif # NEW_LEDS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 2a698df9da57..22cba8dfd6a9 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -111,3 +111,6 @@ obj-$(CONFIG_LEDS_TRIGGERS) += trigger/
 
 # LED Blink
 obj-$(CONFIG_LEDS_BLINK)+= blink/
+
+# LED Simple
+obj-$(CONFIG_LEDS_SIMPLE)  += simple/
diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simple/Kconfig
new file mode 100644
index ..98540b6c8d7a
--- /dev/null
+++ b/drivers/leds/simple/Kconfig
@@ -0,0 +1,12 @@
+config LEDS_MULTI_GPIO
+   tristate "LED Support for multiple GPIOs LED"
+   depends on LEDS_CLASS
+   depends on GPIOLIB
+   depends on OF
+   help
+ This option enables support for a multiple GPIOs LED. Such LED is made
+ of multiple GPIOs and could change the brightness by setting different
+ states of the GPIOs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called leds-multi-gpio.
diff --git a/drivers/leds/simple/Makefile b/drivers/leds/simple/Makefile
new file mode 100644
index ..2ba655fdc175
--- /dev/null
+++ b/drivers/leds/simple/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_LEDS_MULTI_GPIO)  += leds-multi-gpio.o
diff --git a/drivers/leds/simple/leds-multi-gpio.c 
b/drivers/leds/simple/leds-multi-gpio.c
new file mode 100644
index ..ede033354156
--- /dev/null
+++ b/drivers/leds/simple/leds-multi-gpio.c
@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Axis Communications AB
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+
+#define MAX_GPIO_NUM  8
+
+struct multi_gpio_led_priv {
+   struct led_classdev cdev;
+
+   struct gpio_descs *gpios;
+
+   u16 nr_states;
+
+   u8 states[0];
+};
+
+
+static void multi_gpio_led_set(struct led_classdev *led_cdev,
+   enum led_brightness value)
+{
+   struct multi_gpio_led_priv *priv;
+   int idx;
+
+   DECLARE_BITMAP(values, MAX_GPIO_NUM);
+
+   priv = container_of(led_cdev, struct multi_gpio_led_priv, cdev);
+
+   idx = value > led_cdev->max_brightness ? led_cdev->max_brightness : 
value;
+
+   values[0] = priv->states[idx];
+
+   gpiod_set_array_value(priv->gpios->ndescs, priv->gpios->desc,
+   priv->gpios->info, values);
+}
+
+static int multi_gpio_led_probe(struct platform_device *pdev)
+{
+   struct device *dev = >dev;
+   struct device_node *node = dev->of_node;
+   struct multi_gpio_led_priv *priv = NULL;
+   int ret;
+   const char *state = NULL;
+   struct led_init_data init_data = {};
+   struct gpio_descs *gpios;
+   u16 nr_states;
+
+   gpios = devm_gpiod_get_array(dev, "led", GPIOD_OUT_LOW);
+   if (IS_ERR(gpios))
+   return PTR_ERR(gpios);
+
+   if (gpios->ndescs >= MAX_GPIO_NUM) {
+   dev_err(dev, "Too many GPIOs\n");
+   return -EINVAL;
+   }
+
+   ret = of_property_count_u8_elems(node, "led-states");
+   if (ret < 0)
+   return ret;
+
+   if (ret != (1 << gpios->ndescs)) {
+   dev_err(dev, "led-states number should equal to 2^led-gpios\n");
+   return -EINVAL;
+   }
+
+   nr_states = ret;
+
+   priv = devm_kzalloc(dev, sizeof(struct multi_gpio_led_priv) + nr_states,
+   GFP_KERNEL);
+   if (!priv)
+   return -ENOMEM;
+
+   ret = of_property_read_u8_array(node, "led-stat

Re: [PATCH v2 2/2] leds: leds-multi-gpio: Add multiple GPIOs LED driver

2021-03-26 Thread Hermes Zhang
On 3/26/21 1:56 PM, Alexander Dahl wrote:
>> +
>> +module_platform_driver(multi_gpio_led_driver);
>> +
>> +MODULE_AUTHOR("Hermes Zhang ");
>> +MODULE_DESCRIPTION("Multiple GPIOs LED driver");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_ALIAS("platform:leds-multi-gpio");
> I did not review thouroughly, but in my mail the indentation looks
> wrong. Did checkpatch complain?

Sorry, I forgot to check the style before commit, but seems one problem
about extra space:

$ chkernel
ERROR: space prohibited before that ',' (ctx:WxW)
#164: FILE: drivers/leds/simple/leds-multi-gpio.c:76:
++ sizeof(u8) * nr_states , GFP_KERNEL);
   ^

I will fix it in next commit.


Best Regards,

Hermes




[PATCH v2 2/2] leds: leds-multi-gpio: Add multiple GPIOs LED driver

2021-03-25 Thread Hermes Zhang
From: Hermes Zhang 

Introduce a new multiple GPIOs LED driver. This LED will made of
multiple GPIOs (up to 8) and will map different brightness to different
GPIOs states which defined in dts file.

Signed-off-by: Hermes Zhang 
---
 drivers/leds/Kconfig  |   3 +
 drivers/leds/Makefile |   3 +
 drivers/leds/simple/Kconfig   |  23 
 drivers/leds/simple/Makefile  |   3 +
 drivers/leds/simple/leds-multi-gpio.c | 144 ++
 5 files changed, 176 insertions(+)
 create mode 100644 drivers/leds/simple/Kconfig
 create mode 100644 drivers/leds/simple/Makefile
 create mode 100644 drivers/leds/simple/leds-multi-gpio.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index b6742b4231bf..f95217b2fcf1 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -937,4 +937,7 @@ source "drivers/leds/trigger/Kconfig"
 comment "LED Blink"
 source "drivers/leds/blink/Kconfig"
 
+comment "LED Simple"
+source "drivers/leds/simple/Kconfig"
+
 endif # NEW_LEDS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 2a698df9da57..26917d93fa03 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -111,3 +111,6 @@ obj-$(CONFIG_LEDS_TRIGGERS) += trigger/
 
 # LED Blink
 obj-$(CONFIG_LEDS_BLINK)+= blink/
+
+# LED Blink
+obj-$(CONFIG_LEDS_SIMPLE)  += simple/
diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simple/Kconfig
new file mode 100644
index ..7aef82701f86
--- /dev/null
+++ b/drivers/leds/simple/Kconfig
@@ -0,0 +1,23 @@
+menuconfig LEDS_SIMPLE
+   bool "Simple LED support"
+   depends on LEDS_CLASS
+   help
+ This option enables simple leds support for the leds class.
+ If unsure, say Y.
+
+if LEDS_SIMPLE
+
+config LEDS_MULTI_GPIO
+   tristate "LED Support for multiple GPIOs LED"
+   depends on LEDS_CLASS
+   depends on GPIOLIB
+   depends on OF
+   help
+ This option enables support for a multiple GPIOs LED. Such LED is made
+ of multiple GPIOs and could change the brightness by setting different
+ states of the GPIOs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called leds-multi-gpio.
+
+endif # LEDS_SIMPLE
diff --git a/drivers/leds/simple/Makefile b/drivers/leds/simple/Makefile
new file mode 100644
index ..2ba655fdc175
--- /dev/null
+++ b/drivers/leds/simple/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_LEDS_MULTI_GPIO)  += leds-multi-gpio.o
diff --git a/drivers/leds/simple/leds-multi-gpio.c 
b/drivers/leds/simple/leds-multi-gpio.c
new file mode 100644
index ..14fdaa5a2aac
--- /dev/null
+++ b/drivers/leds/simple/leds-multi-gpio.c
@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Axis Communications AB
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+
+#define MAX_GPIO_NUM  8
+
+struct multi_gpio_led_priv {
+   struct led_classdev cdev;
+
+   struct gpio_descs *gpios;
+
+   u16 nr_states;
+
+   u8 states[0];
+};
+
+
+static void multi_gpio_led_set(struct led_classdev *led_cdev,
+   enum led_brightness value)
+{
+   struct multi_gpio_led_priv *priv;
+   int idx;
+
+   DECLARE_BITMAP(values, MAX_GPIO_NUM);
+
+   priv = container_of(led_cdev, struct multi_gpio_led_priv, cdev);
+
+   idx = value > led_cdev->max_brightness ? led_cdev->max_brightness : 
value;
+
+   values[0] = priv->states[idx];
+
+   gpiod_set_array_value(priv->gpios->ndescs, priv->gpios->desc,
+   priv->gpios->info, values);
+}
+
+static int multi_gpio_led_probe(struct platform_device *pdev)
+{
+   struct device *dev = >dev;
+   struct device_node *node = dev->of_node;
+   struct multi_gpio_led_priv *priv = NULL;
+   int ret;
+   const char *state = NULL;
+   struct led_init_data init_data = {};
+   struct gpio_descs *gpios;
+   u16 nr_states;
+
+   gpios = devm_gpiod_get_array(dev, "led", GPIOD_OUT_LOW);
+   if (IS_ERR(gpios))
+   return PTR_ERR(gpios);
+
+   if (gpios->ndescs >= MAX_GPIO_NUM) {
+   dev_err(dev, "Too many GPIOs\n");
+   return -EINVAL;
+   }
+
+   ret = of_property_count_u8_elems(node, "led-states");
+   if (ret < 0)
+   return ret;
+
+   if (ret != 1 << gpios->ndescs) {
+   dev_err(dev, "led-states number should equal to 2^led-gpios\n");
+   return -EINVAL;
+   }
+
+   nr_states = ret;
+
+   priv = devm_kzalloc(dev, sizeof(struct multi_gpio_led_priv)
+   + sizeof(u8) * nr_states , GFP_KERNEL);
+   if (!priv)
+   return -ENOMEM;
+
+   ret = of_property_read_u8_

[PATCH v2 1/2] dt-binding: leds: Document leds-multi-gpio bindings

2021-03-25 Thread Hermes Zhang
From: Hermes Zhang 

This binding represents LED devices which are controller with
multiple GPIO lines in order to achieve more than two brightness
states.

Signed-off-by: Hermes Zhang 
---
 .../bindings/leds/leds-multi-gpio.yaml| 50 +++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml

diff --git a/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml 
b/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
new file mode 100644
index ..1549f21e8d6e
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-multi-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Multiple GPIOs LED driver
+
+maintainers:
+  - Hermes Zhang 
+
+description:
+  This will support some LED made of multiple GPIOs and the brightness of the
+  LED could map to different states of the GPIOs.
+
+properties:
+  compatible:
+const: multi-gpio-led
+
+  led-gpios:
+description: Array of one or more GPIOs pins used to control the LED.
+minItems: 1
+maxItems: 8  # Should be enough
+
+  led-states:
+description: |
+  The array list the supported states here which will map to brightness
+  from 0 to maximum. Each item in the array will present all the GPIOs
+  value by bit.
+$ref: /schemas/types.yaml#/definitions/uint8-array
+minItems: 1
+maxItems: 256 # Should be enough
+
+required:
+  - compatible
+  - led-gpios
+  - led-states
+
+additionalProperties: false
+
+examples:
+  - |
+gpios-led {
+  compatible = "multi-gpio-led";
+
+  led-gpios = < 23 0x1>,
+  < 24 0x1>;
+  led-states = /bits/ 8 <0x00 0x01 0x02 0x03>;
+};
+...
-- 
2.20.1



[PATCH v2 0/2] New multiple GPIOs LED driver

2021-03-25 Thread Hermes Zhang
From: Hermes Zhang 

changes v2:
- use max_brightness(=2^gpio number) instead of LED_FULL
- alloc priv at once
- move driver code to new simple folder
- update commit message for dt-binding commit
- move dt-binding commit before driver code

Hermes Zhang (2):
  dt-binding: leds: Document leds-multi-gpio bindings
  leds: leds-multi-gpio: Add multiple GPIOs LED driver

 .../bindings/leds/leds-multi-gpio.yaml|  50 ++
 drivers/leds/Kconfig  |   3 +
 drivers/leds/Makefile |   3 +
 drivers/leds/simple/Kconfig   |  23 +++
 drivers/leds/simple/Makefile  |   3 +
 drivers/leds/simple/leds-multi-gpio.c | 144 ++
 6 files changed, 226 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
 create mode 100644 drivers/leds/simple/Kconfig
 create mode 100644 drivers/leds/simple/Makefile
 create mode 100644 drivers/leds/simple/leds-multi-gpio.c

-- 
2.20.1



Re: [PATCH 1/2] leds: leds-multi-gpio: Add multiple GPIOs LED driver

2021-03-25 Thread Hermes Zhang
Hi Marek,

On 3/24/21 5:34 PM, Marek Behun wrote:
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
> Why do you include slab.h?
Yeah, I will clean it in next commit.
>> +
>> +
>> +static void multi_gpio_led_set(struct led_classdev *led_cdev,
>> +enum led_brightness value)
>> +{
>> +struct multi_gpio_led_priv *priv;
>> +int idx;
>> +
>> +DECLARE_BITMAP(values, MAX_GPIO_NUM);
>> +
>> +priv = container_of(led_cdev, struct multi_gpio_led_priv, cdev);
>> +
>> +idx = (value - LED_OFF) * priv->nr_states / (LED_FULL + 1);
> LED_FULL / LED_OFF are deprecated, don't use them.

Then could I use just 0 (instead LED_OFF) and led_cdev->max_brightness

(instead of LED_FULL) here? The idea here is map the states defined in dts

to the full brightness range.

> +
> + priv->nr_states = ret;
> + priv->states = devm_kzalloc(dev, sizeof(*priv->states) * 
> priv->nr_states, GFP_KERNEL);
> + if (!priv->states)
> + return -ENOMEM;
> +
> + ret = of_property_read_u8_array(node, "led-states", priv->states, 
> priv->nr_states);
> + if (ret)
> + return ret;
> +
> + priv->cdev.max_brightness = LED_FULL;
>  max_brightness is not 255 (= LED_FULL). max_brightness must be
> derived from the led-states property.

Yeah, I will fix this. the max-brightness should for the whole LED,
right? So

it will at same level with led-states.





[PATCH 2/2] dt-binding: leds: Document leds-multi-gpio bindings

2021-03-24 Thread Hermes Zhang
From: Hermes Zhang 

Document the device tree bindings of the multiple GPIOs LED driver
Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml.

Signed-off-by: Hermes Zhang 
---
 .../bindings/leds/leds-multi-gpio.yaml| 50 +++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml

diff --git a/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml 
b/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
new file mode 100644
index ..6f2b47487b90
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-multi-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Multiple GPIOs LED driver
+
+maintainers:
+  - Hermes Zhang 
+
+description:
+  This will support some LED made of multiple GPIOs and the brightness of the
+  LED could map to different states of the GPIOs.
+
+properties:
+  compatible:
+const: multi-gpio-led
+
+  led-gpios:
+description: Array of one or more GPIOs pins used to control the LED.
+minItems: 1
+maxItems: 8  # Should be enough
+
+  led-states:
+description: |
+  The array list the supported states here which will map to brightness
+  from 0 to maximum. Each item in the array will present all the GPIOs
+  value by bit.
+$ref: /schemas/types.yaml#/definitions/uint8-array
+minItems: 1
+maxItems: 16 # Should be enough
+
+required:
+  - compatible
+  - led-gpios
+  - led-states
+
+additionalProperties: false
+
+examples:
+  - |
+gpios-led {
+  compatible = "multi-gpio-led";
+
+  led-gpios = < 23 0x1>,
+  < 24 0x1>;
+  led-states = /bits/ 8 <0x00 0x01 0x02 0x03>;
+};
+...
-- 
2.20.1



[PATCH 1/2] leds: leds-multi-gpio: Add multiple GPIOs LED driver

2021-03-24 Thread Hermes Zhang
From: Hermes Zhang 

Introduce a new multiple GPIOs LED driver. This LED will made of
multiple GPIOs (up to 8) and will map different brightness to different
GPIOs states which defined in dts file.

Signed-off-by: Hermes Zhang 
---
 drivers/leds/Kconfig   |  12 +++
 drivers/leds/Makefile  |   1 +
 drivers/leds/leds-multi-gpio.c | 140 +
 3 files changed, 153 insertions(+)
 create mode 100644 drivers/leds/leds-multi-gpio.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index b6742b4231bf..e3ff84080192 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -370,6 +370,18 @@ config LEDS_GPIO
  defined as platform devices and/or OpenFirmware platform devices.
  The code to use these bindings can be selected below.
 
+config LEDS_MULTI_GPIO
+   tristate "LED Support for multiple GPIOs LED"
+   depends on LEDS_CLASS
+   depends on GPIOLIB
+   help
+ This option enables support for a multiple GPIOs LED. Such LED is made
+ of multiple GPIOs and could change the brightness by setting different
+ states of the GPIOs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called leds-multi-gpio.
+
 config LEDS_LP3944
tristate "LED Support for N.S. LP3944 (Fun Light) I2C chip"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 2a698df9da57..984201ec5375 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_LEDS_DA903X) += leds-da903x.o
 obj-$(CONFIG_LEDS_DA9052)  += leds-da9052.o
 obj-$(CONFIG_LEDS_FSG) += leds-fsg.o
 obj-$(CONFIG_LEDS_GPIO)+= leds-gpio.o
+obj-$(CONFIG_LEDS_MULTI_GPIO)  += leds-multi-gpio.o
 obj-$(CONFIG_LEDS_GPIO_REGISTER)   += leds-gpio-register.o
 obj-$(CONFIG_LEDS_HP6XX)   += leds-hp6xx.o
 obj-$(CONFIG_LEDS_INTEL_SS4200)+= leds-ss4200.o
diff --git a/drivers/leds/leds-multi-gpio.c b/drivers/leds/leds-multi-gpio.c
new file mode 100644
index ..54d92c81a476
--- /dev/null
+++ b/drivers/leds/leds-multi-gpio.c
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Axis Communications AB
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MAX_GPIO_NUM  8
+
+struct multi_gpio_led_priv {
+   struct led_classdev cdev;
+
+   struct gpio_descs *gpios;
+
+   u8 *states;
+   int nr_states;
+};
+
+
+static void multi_gpio_led_set(struct led_classdev *led_cdev,
+   enum led_brightness value)
+{
+   struct multi_gpio_led_priv *priv;
+   int idx;
+
+   DECLARE_BITMAP(values, MAX_GPIO_NUM);
+
+   priv = container_of(led_cdev, struct multi_gpio_led_priv, cdev);
+
+   idx = (value - LED_OFF) * priv->nr_states / (LED_FULL + 1);
+
+   values[0] = priv->states[idx];
+
+   gpiod_set_array_value(priv->gpios->ndescs, priv->gpios->desc,
+   priv->gpios->info, values);
+}
+
+static int multi_gpio_led_probe(struct platform_device *pdev)
+{
+   struct device *dev = >dev;
+   struct device_node *node = dev->of_node;
+   struct multi_gpio_led_priv *priv = NULL;
+   int ret;
+   const char *state = NULL;
+   struct led_init_data init_data = {};
+
+   priv = devm_kzalloc(dev, sizeof(struct multi_gpio_led_priv), 
GFP_KERNEL);
+   if (!priv)
+   return -ENOMEM;
+
+   priv->gpios = devm_gpiod_get_array(dev, "led", GPIOD_OUT_LOW);
+   if (IS_ERR(priv->gpios))
+   return PTR_ERR(priv->gpios);
+
+   if (priv->gpios->ndescs >= MAX_GPIO_NUM) {
+   dev_err(dev, "Too many GPIOs\n");
+   return -EINVAL;
+   }
+
+   ret = of_property_count_u8_elems(node, "led-states");
+   if (ret < 0)
+   return ret;
+
+   priv->nr_states = ret;
+   priv->states = devm_kzalloc(dev, sizeof(*priv->states) * 
priv->nr_states, GFP_KERNEL);
+   if (!priv->states)
+   return -ENOMEM;
+
+   ret = of_property_read_u8_array(node, "led-states", priv->states, 
priv->nr_states);
+   if (ret)
+   return ret;
+
+   priv->cdev.max_brightness = LED_FULL;
+   priv->cdev.default_trigger = of_get_property(node, 
"linux,default-trigger", NULL);
+   priv->cdev.brightness_set = multi_gpio_led_set;
+
+   init_data.fwnode = of_fwnode_handle(node);
+
+   ret = devm_led_classdev_register_ext(dev, >cdev, _data);
+   if (ret < 0)
+   return ret;
+
+   of_property_read_string(node, "default-state", );
+   if (!strcmp(state, "on"))
+   multi_gpio_led_set(>cdev, LED_FULL

[PATCH 0/2] New multiple GPIOs LED driver

2021-03-24 Thread Hermes Zhang
From: Hermes Zhang 

*** BLURB HERE ***

New multiple GPIOs LED driver


Hermes Zhang (2):
  leds: leds-multi-gpio: Add multiple GPIOs LED driver
  dt-binding: leds: Document leds-multi-gpio bindings

 .../bindings/leds/leds-multi-gpio.yaml|  50 +++
 drivers/leds/Kconfig  |  12 ++
 drivers/leds/Makefile |   1 +
 drivers/leds/leds-multi-gpio.c| 140 ++
 4 files changed, 203 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
 create mode 100644 drivers/leds/leds-multi-gpio.c

-- 
2.20.1



RE: [PATCH v3] dt-binding: leds: Document leds-multi-gpio bindings

2021-03-24 Thread Hermes Zhang
> >
> > Notes:
> > Add maxItems
> 
> What about the other part of the series? I think you should send both
> patches together with an introduction message on both. If you only change
> one patch for a new version spin of the series, just send the other one
> unchanged.
> 
> (It makes no sense to merge the binding as long as the driver is not merged,
> otherwise you would end up with a binding without driver. So keeping them
> together should help reviewers and maintainers.)
> 

Hi Alexander,

The other part is here: https://lore.kernel.org/patchwork/patch/1399875/, so do 
you mean I need to combine these two as one commit? Or is there anyway to link 
them together? Thanks.

I'm first time to commit a new driver, sorry for that.

Best Regards,
Hermes


RE: [PATCH v2] dt-binding: leds: Document leds-multi-gpio bindings

2021-03-23 Thread Hermes Zhang
> >
> > Hi Rob,
> >
> > Thanks. Yes, now I can see the warning, but I could not understand what
> was wrong? Could you give some hint?
> 
> I think you need 'maxItems' in addition to minItems.

Exactly! Thanks for the suggestion.

Best Regards,
Hermes


[PATCH v3] dt-binding: leds: Document leds-multi-gpio bindings

2021-03-23 Thread Hermes Zhang
From: Hermes Zhang 

Document the device tree bindings of the multiple GPIOs LED driver
Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml.

Signed-off-by: Hermes Zhang 
---

Notes:
Add maxItems

 .../bindings/leds/leds-multi-gpio.yaml| 50 +++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml

diff --git a/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml 
b/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
new file mode 100644
index ..6f2b47487b90
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-multi-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Multiple GPIOs LED driver
+
+maintainers:
+  - Hermes Zhang 
+
+description:
+  This will support some LED made of multiple GPIOs and the brightness of the
+  LED could map to different states of the GPIOs.
+
+properties:
+  compatible:
+const: multi-gpio-led
+
+  led-gpios:
+description: Array of one or more GPIOs pins used to control the LED.
+minItems: 1
+maxItems: 8  # Should be enough
+
+  led-states:
+description: |
+  The array list the supported states here which will map to brightness
+  from 0 to maximum. Each item in the array will present all the GPIOs
+  value by bit.
+$ref: /schemas/types.yaml#/definitions/uint8-array
+minItems: 1
+maxItems: 16 # Should be enough
+
+required:
+  - compatible
+  - led-gpios
+  - led-states
+
+additionalProperties: false
+
+examples:
+  - |
+gpios-led {
+  compatible = "multi-gpio-led";
+
+  led-gpios = < 23 0x1>,
+  < 24 0x1>;
+  led-states = /bits/ 8 <0x00 0x01 0x02 0x03>;
+};
+...
-- 
2.20.1



RE: [PATCH v2] dt-binding: leds: Document leds-multi-gpio bindings

2021-03-22 Thread Hermes Zhang
> -Original Message-
> From: Rob Herring 
> Sent: 2021年3月23日 1:38
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-
> review/Documentation/devicetree/bindings/leds/leds-multi-
> gpio.example.dt.yaml: gpios-led: led-states: 'oneOf' conditional failed, one
> must be fixed:
>   [[0, 1, 2, 3]] is too short
>   [0, 1, 2, 3] is too long
>   From schema: /builds/robherring/linux-dt-
> review/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
> 

Hi Rob,

Thanks. Yes, now I can see the warning, but I could not understand what was 
wrong? Could you give some hint? 

Best Regards,
Hermes


[PATCH v2] dt-binding: leds: Document leds-multi-gpio bindings

2021-03-22 Thread Hermes Zhang
From: Hermes Zhang 

Document the device tree bindings of the multiple GPIOs LED driver
Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml.

Signed-off-by: Hermes Zhang 
---

Notes:
Fix typo and missing item

 .../bindings/leds/leds-multi-gpio.yaml| 49 +++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml

diff --git a/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml 
b/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
new file mode 100644
index ..bc19d1758d92
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-multi-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Multiple GPIOs LED driver
+
+maintainers:
+  - Hermes Zhang 
+
+description:
+  This will support some LED made of multiple GPIOs and the brightness of the
+  LED could map to different states of the GPIOs.
+
+properties:
+  compatible:
+const: multi-gpio-led
+
+  led-gpios:
+description: Array of one or more GPIOs pins used to control the LED.
+minItems: 1
+maxItems: 8  # Should be enough
+
+  led-states:
+description: |
+  The array list the supported states here which will map to brightness
+  from 0 to maximum. Each item in the array will present all the GPIOs
+  value by bit.
+$ref: /schemas/types.yaml#/definitions/uint8-array
+minItems: 1
+
+required:
+  - compatible
+  - led-gpios
+  - led-states
+
+additionalProperties: false
+
+examples:
+  - |
+gpios-led {
+  compatible = "multi-gpio-led";
+
+  led-gpios = < 23 0x1>,
+  < 24 0x1>;
+  led-states = /bits/ 8 <0x00 0x01 0x02 0x03>;
+};
+...
-- 
2.20.1



[PATCH] dt-binding: leds: Document leds-multi-gpio bindings

2021-03-22 Thread Hermes Zhang
From: Hermes Zhang 

Document the device tree bindings of the multiple GPIOs LED driver
Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml.

Signed-off-by: Hermes Zhang 
---
 .../bindings/leds/leds-multi-gpio.yaml| 47 +++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml

diff --git a/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml 
b/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
new file mode 100644
index ..09e7b60a800e
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-multi-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Multiple GPIOs LED driver
+
+maintainers:
+  - Hermes Zhang 
+
+description:
+  This will support some LED made of multiple GPIOs and the brightness of the
+  LED could map to different states of the GPIOs.
+
+properties:
+  compatible:
+const: multi-gpio-led
+
+  led-gpios:
+description: Array of one or more GPIOs pins used to control the LED.
+minItems: 1
+maxItems: 8  # Should be enough
+
+  led-states:
+description: |
+  The array list the supported states here which will map to brightness
+  from 0 to maximum. Each item in the array will present all the GPIOs
+  value by bit.
+$ref: /schemas/types.yaml#/definitions/uint8-array
+minItems: 1
+
+required:
+  - compatible
+  - led-gpios
+  - led-states
+
+examples:
+  - |
+gpios-led {
+  compatible = "multi-gpio-led";
+
+  led-gpios = < 23 0x1>,
+  < 24 0x1>;
+  led-states = /bit/ 8 <0x00 0x01 0x02 0x03>;
+};
+...
-- 
2.20.1



[PATCH] leds: leds-multi-gpio: Add multiple GPIOs LED driver

2021-03-22 Thread Hermes Zhang
From: Hermes Zhang 

Introduce a new multiple GPIOs LED driver. This LED will made of
multiple GPIOs (up to 8) and will map different brightness to different
GPIOs states which defined in dts file.

Signed-off-by: Hermes Zhang 
---
 drivers/leds/Kconfig   |  12 +++
 drivers/leds/Makefile  |   1 +
 drivers/leds/leds-multi-gpio.c | 140 +
 3 files changed, 153 insertions(+)
 create mode 100644 drivers/leds/leds-multi-gpio.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index b6742b4231bf..e3ff84080192 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -370,6 +370,18 @@ config LEDS_GPIO
  defined as platform devices and/or OpenFirmware platform devices.
  The code to use these bindings can be selected below.
 
+config LEDS_MULTI_GPIO
+   tristate "LED Support for multiple GPIOs LED"
+   depends on LEDS_CLASS
+   depends on GPIOLIB
+   help
+ This option enables support for a multiple GPIOs LED. Such LED is made
+ of multiple GPIOs and could change the brightness by setting different
+ states of the GPIOs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called leds-multi-gpio.
+
 config LEDS_LP3944
tristate "LED Support for N.S. LP3944 (Fun Light) I2C chip"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 2a698df9da57..984201ec5375 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_LEDS_DA903X) += leds-da903x.o
 obj-$(CONFIG_LEDS_DA9052)  += leds-da9052.o
 obj-$(CONFIG_LEDS_FSG) += leds-fsg.o
 obj-$(CONFIG_LEDS_GPIO)+= leds-gpio.o
+obj-$(CONFIG_LEDS_MULTI_GPIO)  += leds-multi-gpio.o
 obj-$(CONFIG_LEDS_GPIO_REGISTER)   += leds-gpio-register.o
 obj-$(CONFIG_LEDS_HP6XX)   += leds-hp6xx.o
 obj-$(CONFIG_LEDS_INTEL_SS4200)+= leds-ss4200.o
diff --git a/drivers/leds/leds-multi-gpio.c b/drivers/leds/leds-multi-gpio.c
new file mode 100644
index ..54d92c81a476
--- /dev/null
+++ b/drivers/leds/leds-multi-gpio.c
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Axis Communications AB
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MAX_GPIO_NUM  8
+
+struct multi_gpio_led_priv {
+   struct led_classdev cdev;
+
+   struct gpio_descs *gpios;
+
+   u8 *states;
+   int nr_states;
+};
+
+
+static void multi_gpio_led_set(struct led_classdev *led_cdev,
+   enum led_brightness value)
+{
+   struct multi_gpio_led_priv *priv;
+   int idx;
+
+   DECLARE_BITMAP(values, MAX_GPIO_NUM);
+
+   priv = container_of(led_cdev, struct multi_gpio_led_priv, cdev);
+
+   idx = (value - LED_OFF) * priv->nr_states / (LED_FULL + 1);
+
+   values[0] = priv->states[idx];
+
+   gpiod_set_array_value(priv->gpios->ndescs, priv->gpios->desc,
+   priv->gpios->info, values);
+}
+
+static int multi_gpio_led_probe(struct platform_device *pdev)
+{
+   struct device *dev = >dev;
+   struct device_node *node = dev->of_node;
+   struct multi_gpio_led_priv *priv = NULL;
+   int ret;
+   const char *state = NULL;
+   struct led_init_data init_data = {};
+
+   priv = devm_kzalloc(dev, sizeof(struct multi_gpio_led_priv), 
GFP_KERNEL);
+   if (!priv)
+   return -ENOMEM;
+
+   priv->gpios = devm_gpiod_get_array(dev, "led", GPIOD_OUT_LOW);
+   if (IS_ERR(priv->gpios))
+   return PTR_ERR(priv->gpios);
+
+   if (priv->gpios->ndescs >= MAX_GPIO_NUM) {
+   dev_err(dev, "Too many GPIOs\n");
+   return -EINVAL;
+   }
+
+   ret = of_property_count_u8_elems(node, "led-states");
+   if (ret < 0)
+   return ret;
+
+   priv->nr_states = ret;
+   priv->states = devm_kzalloc(dev, sizeof(*priv->states) * 
priv->nr_states, GFP_KERNEL);
+   if (!priv->states)
+   return -ENOMEM;
+
+   ret = of_property_read_u8_array(node, "led-states", priv->states, 
priv->nr_states);
+   if (ret)
+   return ret;
+
+   priv->cdev.max_brightness = LED_FULL;
+   priv->cdev.default_trigger = of_get_property(node, 
"linux,default-trigger", NULL);
+   priv->cdev.brightness_set = multi_gpio_led_set;
+
+   init_data.fwnode = of_fwnode_handle(node);
+
+   ret = devm_led_classdev_register_ext(dev, >cdev, _data);
+   if (ret < 0)
+   return ret;
+
+   of_property_read_string(node, "default-state", );
+   if (!strcmp(state, "on"))
+   multi_gpio_led_set(>cdev, LED_FULL

RE: [PATCH] leds: leds-dual-gpio: Add dual GPIO LEDs driver

2021-03-17 Thread Hermes Zhang
> > +   priv = devm_kzalloc(dev, sizeof(struct gpio_dual_leds_priv),
> GFP_KERNEL);
> > +   if (!priv)
> > +   return -ENOMEM;
> > +
> > +   priv->low_gpio = devm_gpiod_get(dev, "low", GPIOD_OUT_LOW);
> > +   ret = PTR_ERR_OR_ZERO(priv->low_gpio);
> > +   if (ret) {
> > +   dev_err(dev, "cannot get low-gpios %d\n", ret);
> > +   return ret;
> > +   }
> > +
> > +   priv->high_gpio = devm_gpiod_get(dev, "high", GPIOD_OUT_LOW);
> > +   ret = PTR_ERR_OR_ZERO(priv->high_gpio);
> > +   if (ret) {
> > +   dev_err(dev, "cannot get high-gpios %d\n", ret);
> > +   return ret;
> > +   }
> 
> Actually... I'd call it led-0 and led-1 or something. Someone may/will come
> with 4-bit GPIO LED one day, and it would be cool if this could be used with
> minimal effort.
> 
> Calling it multi_led in the driver/bindings would bnot be bad, either.
> 

Hi all,

I have try to use leds-regulator to implement my case, most works. But the only 
thing doesn't work is the enable-gpio. In my case, we don't have a real enable 
gpio, so when we set LED_OFF, it could not off the LED as we expected. 

So I think I will back to the new multi LED driver, but make it more generic. 

Best Regards,
Hermes


RE: [PATCH] leds: leds-dual-gpio: Add dual GPIO LEDs driver

2021-03-12 Thread Hermes Zhang
Hi Alexander,

> Am Donnerstag, 11. März 2021, 14:04:08 CET schrieb Hermes Zhang:
> > From: Hermes Zhang 
> >
> > Introduce a new Dual GPIO LED driver. These two GPIOs LED will act as
> > one LED as normal GPIO LED but give the possibility to change the
> > intensity in four levels: OFF, LOW, MIDDLE and HIGH.
> 
> Interesting use case. Is there any real world hardware wired like that you
> could point to?
> 

Yes, we have the HW, it's not a chip but just some circuit to made of.
 
> > +config LEDS_DUAL_GPIO
> > +   tristate "LED Support for Dual GPIO connected LEDs"
> > +   depends on LEDS_CLASS
> > +   depends on GPIOLIB || COMPILE_TEST
> > +   help
> > + This option enables support for the two LEDs connected to GPIO
> > + outputs. These two GPIO LEDs act as one LED in the sysfs and
> > + perform different intensity by enable either one of them or both.
> 
> Well, although I never had time to implement that, I suspect that could
> conflict if someone will eventually write a driver for two pin dual color LEDs
> connected to GPIO pins.  We actually do that on our hardware and I know
> others do, too.
> 
> I asked about that back in 2019, see this thread:
> 
> https://www.spinics.net/lists/linux-leds/msg11665.html
> 
> At the time the multicolor framework was not yet merged, so today I would
> probably make something which either uses the multicolor framework or at
> least has a similar interface to userspace. However, it probably won't 
> surprise
> you all, this is not highest priority on my ToDo list. ;-)
> 
> (What we actually do is pretend those are separate LEDs and ignore the
> conflicting case where both GPIOs are on and the LED is dark then.)
> 

Yes, that case seems conflict with mine, the pattern for me is like:

P1 | P2 | LED
-- + -- + -
 0 |  0 | off
 0 |  1 | Any color
 1 |  0 | Any color
 1 |  1 | both on

Now I'm investigate another way from Marek's suggestion by using 
REGULATOR_GPIO, to see if could meet my requirement. If yes, then I do think no 
new  driver is needed.

Best Regards,
Hermes



RE: [PATCH] leds: leds-dual-gpio: Add dual GPIO LEDs driver

2021-03-11 Thread Hermes Zhang
> 
> Sorry, leds-regulator has only a binary state LED.
> 
> Maybe you could extend leds-regulator to be able to use all regulator states?
> 
> Or you can extend leds-gpio driver to support N states via log N gpios,
> instead of adding new driver.

It seems a good idea to extend leds-gpio, so in my case, I should have such dts:

 63 leds {
 64 compatible = "gpio-leds";
 65 
 66 recording_front {
 67 label = "recording_front:red";
 68 gpios = < 130 GPIO_ACTIVE_HIGH>, < 129 
GPIO_ACTIVE_HIGH>;
 69 default-state = "off";
 70 };
 71 };

For my case, two leds is enough, but it sill easy to extend the support number 
bigger than two. And the length of gpios array is not fixed, so it could 
compatible with exist "gpio-leds" dts, right? 

If this idea work, should I create a new commit or still in this track (V2)?

Best Regards,
Hermes 

> 
> Marek


[PATCH] leds: leds-dual-gpio: Add dual GPIO LEDs driver

2021-03-11 Thread Hermes Zhang
From: Hermes Zhang 

Introduce a new Dual GPIO LED driver. These two GPIOs LED will act as
one LED as normal GPIO LED but give the possibility to change the
intensity in four levels: OFF, LOW, MIDDLE and HIGH.
---
 drivers/leds/Kconfig  |   9 +++
 drivers/leds/Makefile |   1 +
 drivers/leds/leds-dual-gpio.c | 136 ++
 3 files changed, 146 insertions(+)
 create mode 100644 drivers/leds/leds-dual-gpio.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index b6742b4231bf..bc374d3b40ef 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -370,6 +370,15 @@ config LEDS_GPIO
  defined as platform devices and/or OpenFirmware platform devices.
  The code to use these bindings can be selected below.
 
+config LEDS_DUAL_GPIO
+   tristate "LED Support for Dual GPIO connected LEDs"
+   depends on LEDS_CLASS
+   depends on GPIOLIB || COMPILE_TEST
+   help
+ This option enables support for the two LEDs connected to GPIO
+ outputs. These two GPIO LEDs act as one LED in the sysfs and
+ perform different intensity by enable either one of them or both.
+
 config LEDS_LP3944
tristate "LED Support for N.S. LP3944 (Fun Light) I2C chip"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 2a698df9da57..10015cc81f79 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_LEDS_DA903X) += leds-da903x.o
 obj-$(CONFIG_LEDS_DA9052)  += leds-da9052.o
 obj-$(CONFIG_LEDS_FSG) += leds-fsg.o
 obj-$(CONFIG_LEDS_GPIO)+= leds-gpio.o
+obj-$(CONFIG_LEDS_DUAL_GPIO)   += leds-dual-gpio.o
 obj-$(CONFIG_LEDS_GPIO_REGISTER)   += leds-gpio-register.o
 obj-$(CONFIG_LEDS_HP6XX)   += leds-hp6xx.o
 obj-$(CONFIG_LEDS_INTEL_SS4200)+= leds-ss4200.o
diff --git a/drivers/leds/leds-dual-gpio.c b/drivers/leds/leds-dual-gpio.c
new file mode 100644
index ..5d3b9be46f4b
--- /dev/null
+++ b/drivers/leds/leds-dual-gpio.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * LEDs driver for GPIOs
+ *
+ * Copyright (C) 2021 Axis Communications AB
+ * Hermes Zhang 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define GPIO_LOGICAL_ON   1
+#define GPIO_LOGICAL_OFF  0
+
+struct gpio_dual_leds_priv {
+   struct gpio_desc *low_gpio;
+   struct gpio_desc *high_gpio;
+   struct led_classdev cdev;
+};
+
+
+static void gpio_dual_led_set(struct led_classdev *led_cdev,
+   enum led_brightness value)
+{
+   struct gpio_dual_leds_priv *priv;
+
+   priv = container_of(led_cdev, struct gpio_dual_leds_priv, cdev);
+
+   if (value == LED_FULL) {
+   gpiod_set_value(priv->low_gpio, GPIO_LOGICAL_ON);
+   gpiod_set_value(priv->high_gpio, GPIO_LOGICAL_ON);
+   } else if (value < LED_FULL && value > LED_HALF) {
+   /* Enable high only */
+   gpiod_set_value(priv->low_gpio, GPIO_LOGICAL_OFF);
+   gpiod_set_value(priv->high_gpio, GPIO_LOGICAL_ON);
+   } else if (value <= LED_HALF && value > LED_OFF) {
+   /* Enable low only */
+   gpiod_set_value(priv->low_gpio, GPIO_LOGICAL_ON);
+   gpiod_set_value(priv->high_gpio, GPIO_LOGICAL_OFF);
+   } else {
+   gpiod_set_value(priv->low_gpio, GPIO_LOGICAL_OFF);
+   gpiod_set_value(priv->high_gpio, GPIO_LOGICAL_OFF);
+   }
+}
+
+static int gpio_dual_led_probe(struct platform_device *pdev)
+{
+   struct device *dev = >dev;
+   struct device_node *node = dev->of_node;
+   struct gpio_dual_leds_priv *priv = NULL;
+   int ret;
+   const char *state;
+
+   priv = devm_kzalloc(dev, sizeof(struct gpio_dual_leds_priv), 
GFP_KERNEL);
+   if (!priv)
+   return -ENOMEM;
+
+   priv->low_gpio = devm_gpiod_get(dev, "low", GPIOD_OUT_LOW);
+   ret = PTR_ERR_OR_ZERO(priv->low_gpio);
+   if (ret) {
+   dev_err(dev, "cannot get low-gpios %d\n", ret);
+   return ret;
+   }
+
+   priv->high_gpio = devm_gpiod_get(dev, "high", GPIOD_OUT_LOW);
+   ret = PTR_ERR_OR_ZERO(priv->high_gpio);
+   if (ret) {
+   dev_err(dev, "cannot get high-gpios %d\n", ret);
+   return ret;
+   }
+
+   priv->cdev.name = of_get_property(node, "label", NULL);
+   priv->cdev.max_brightness = LED_FULL;
+   priv->cdev.default_trigger =
+ of_get_property(node, "linux,default-trigger", NULL);
+   priv->cdev.brightness_set = gpio_dual_led_set;
+
+   ret = devm_led_classdev_register(dev, >cdev);
+   if (ret < 0)
+ 

Re: [PATCH v2] power: supply: bq27xxx: Supporrt CHARGE_NOW for bq27z561/bq28z610/bq34z100

2020-12-22 Thread Hermes Zhang
On 12/22/20 7:53 PM, Pali Rohár wrote:
> On Tuesday 22 December 2020 19:07:20 Hermes Zhang wrote:
>> From: Hermes Zhang 
>>
>> The CHARGE_NOW is map to REG_NAC for all the gauge chips beofre. But for
>> some chips (e.g. bq27z561) which doesn't have the REG_NAC, we use REG_RC
>> (remaining capacity) for CHARGE_NOW.
>>
>> Signed-off-by: Hermes Zhang 
>> ---
>>
>> Notes:
>>  Set correct REG_RC for all the chips if have
>>  
>>  keep INVALID_REG_ADDR for bq27521, as we could not find
>>  the datasheet and seems no one use it now.
> This chip is used in Nokia N950 and Nokia N9. Pavel implemented kernel
> support, adding to loop.
>
> Public information about it are at:
> https://elinux.org/N950#sn27521_register_map

Thanks for the info. From the link it seem bq27521 have neither NAC and 
RC register.


Best Regards,

Hermes

>
>>   drivers/power/supply/bq27xxx_battery.c | 35 +-
>>   1 file changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/power/supply/bq27xxx_battery.c 
>> b/drivers/power/supply/bq27xxx_battery.c
>> index 315e0909e6a4..774aa376653e 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -110,6 +110,7 @@ enum bq27xxx_reg_index {
>>  BQ27XXX_REG_TTES,   /* Time-to-Empty Standby */
>>  BQ27XXX_REG_TTECP,  /* Time-to-Empty at Constant Power */
>>  BQ27XXX_REG_NAC,/* Nominal Available Capacity */
>> +BQ27XXX_REG_RC, /* Remaining Capacity */
>>  BQ27XXX_REG_FCC,/* Full Charge Capacity */
>>  BQ27XXX_REG_CYCT,   /* Cycle Count */
>>  BQ27XXX_REG_AE, /* Available Energy */
>> @@ -145,6 +146,7 @@ static u8
>>  [BQ27XXX_REG_TTES] = 0x1c,
>>  [BQ27XXX_REG_TTECP] = 0x26,
>>  [BQ27XXX_REG_NAC] = 0x0c,
>> +[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>>  [BQ27XXX_REG_FCC] = 0x12,
>>  [BQ27XXX_REG_CYCT] = 0x2a,
>>  [BQ27XXX_REG_AE] = 0x22,
>> @@ -169,6 +171,7 @@ static u8
>>  [BQ27XXX_REG_TTES] = 0x1c,
>>  [BQ27XXX_REG_TTECP] = 0x26,
>>  [BQ27XXX_REG_NAC] = 0x0c,
>> +[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>>  [BQ27XXX_REG_FCC] = 0x12,
>>  [BQ27XXX_REG_CYCT] = 0x2a,
>>  [BQ27XXX_REG_AE] = INVALID_REG_ADDR,
>> @@ -193,6 +196,7 @@ static u8
>>  [BQ27XXX_REG_TTES] = 0x1a,
>>  [BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>>  [BQ27XXX_REG_NAC] = 0x0c,
>> +[BQ27XXX_REG_RC] = 0x10,
>>  [BQ27XXX_REG_FCC] = 0x12,
>>  [BQ27XXX_REG_CYCT] = 0x2a,
>>  [BQ27XXX_REG_AE] = INVALID_REG_ADDR,
>> @@ -215,6 +219,7 @@ static u8
>>  [BQ27XXX_REG_TTES] = 0x1c,
>>  [BQ27XXX_REG_TTECP] = 0x26,
>>  [BQ27XXX_REG_NAC] = 0x0c,
>> +[BQ27XXX_REG_RC] = 0x10,
>>  [BQ27XXX_REG_FCC] = 0x12,
>>  [BQ27XXX_REG_CYCT] = 0x2a,
>>  [BQ27XXX_REG_AE] = 0x22,
>> @@ -237,6 +242,7 @@ static u8
>>  [BQ27XXX_REG_TTES] = 0x1a,
>>  [BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>>  [BQ27XXX_REG_NAC] = 0x0c,
>> +[BQ27XXX_REG_RC] = 0x10,
>>  [BQ27XXX_REG_FCC] = 0x12,
>>  [BQ27XXX_REG_CYCT] = 0x1e,
>>  [BQ27XXX_REG_AE] = INVALID_REG_ADDR,
>> @@ -257,6 +263,7 @@ static u8
>>  [BQ27XXX_REG_TTES] = 0x1c,
>>  [BQ27XXX_REG_TTECP] = 0x26,
>>  [BQ27XXX_REG_NAC] = 0x0c,
>> +[BQ27XXX_REG_RC] = 0x10,
>>  [BQ27XXX_REG_FCC] = 0x12,
>>  [BQ27XXX_REG_CYCT] = INVALID_REG_ADDR,
>>  [BQ27XXX_REG_AE] = 0x22,
>> @@ -277,6 +284,7 @@ static u8
>>  [BQ27XXX_REG_TTES] = 0x1c,
>>  [BQ27XXX_REG_TTECP] = 0x26,
>>  [BQ27XXX_REG_NAC] = 0x0c,
>> +[BQ27XXX_REG_RC] = 0x10,
>>  [BQ27XXX_REG_FCC] = 0x12,
>>  [BQ27XXX_REG_CYCT] = 0x2a,
>>  [BQ27XXX_REG_AE] = 0x22,
>> @@ -297,6 +305,7 @@ static u8
>>  [BQ27XXX_REG_TTES] = 0x1c,
>>  [BQ27XXX_REG_TTECP] = 0x26,
>>  [BQ27XXX_REG_NAC] = 0x0c,
>> +[BQ27XXX_REG_RC] = 0x10,
>>  [BQ27XXX_REG_FCC] = 0x12,
>>  [BQ27XXX_REG_CYCT] = 0x2a,
>>  [BQ27XXX_REG_AE] = 0x2

[PATCH v2] power: supply: bq27xxx: Supporrt CHARGE_NOW for bq27z561/bq28z610/bq34z100

2020-12-22 Thread Hermes Zhang
From: Hermes Zhang 

The CHARGE_NOW is map to REG_NAC for all the gauge chips beofre. But for
some chips (e.g. bq27z561) which doesn't have the REG_NAC, we use REG_RC
(remaining capacity) for CHARGE_NOW.

Signed-off-by: Hermes Zhang 
---

Notes:
Set correct REG_RC for all the chips if have

keep INVALID_REG_ADDR for bq27521, as we could not find
the datasheet and seems no one use it now.

 drivers/power/supply/bq27xxx_battery.c | 35 +-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/bq27xxx_battery.c 
b/drivers/power/supply/bq27xxx_battery.c
index 315e0909e6a4..774aa376653e 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -110,6 +110,7 @@ enum bq27xxx_reg_index {
BQ27XXX_REG_TTES,   /* Time-to-Empty Standby */
BQ27XXX_REG_TTECP,  /* Time-to-Empty at Constant Power */
BQ27XXX_REG_NAC,/* Nominal Available Capacity */
+   BQ27XXX_REG_RC, /* Remaining Capacity */
BQ27XXX_REG_FCC,/* Full Charge Capacity */
BQ27XXX_REG_CYCT,   /* Cycle Count */
BQ27XXX_REG_AE, /* Available Energy */
@@ -145,6 +146,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1c,
[BQ27XXX_REG_TTECP] = 0x26,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x2a,
[BQ27XXX_REG_AE] = 0x22,
@@ -169,6 +171,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1c,
[BQ27XXX_REG_TTECP] = 0x26,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x2a,
[BQ27XXX_REG_AE] = INVALID_REG_ADDR,
@@ -193,6 +196,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1a,
[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = 0x10,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x2a,
[BQ27XXX_REG_AE] = INVALID_REG_ADDR,
@@ -215,6 +219,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1c,
[BQ27XXX_REG_TTECP] = 0x26,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = 0x10,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x2a,
[BQ27XXX_REG_AE] = 0x22,
@@ -237,6 +242,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1a,
[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = 0x10,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x1e,
[BQ27XXX_REG_AE] = INVALID_REG_ADDR,
@@ -257,6 +263,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1c,
[BQ27XXX_REG_TTECP] = 0x26,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = 0x10,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = INVALID_REG_ADDR,
[BQ27XXX_REG_AE] = 0x22,
@@ -277,6 +284,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1c,
[BQ27XXX_REG_TTECP] = 0x26,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = 0x10,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x2a,
[BQ27XXX_REG_AE] = 0x22,
@@ -297,6 +305,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1c,
[BQ27XXX_REG_TTECP] = 0x26,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = 0x10,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x2a,
[BQ27XXX_REG_AE] = 0x22,
@@ -317,6 +326,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1c,
[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = 0x10,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x1e,
[BQ27XXX_REG_AE] = INVALID_REG_ADDR,
@@ -337,6 +347,7 @@ static u8
[BQ27XXX_REG_TTES] = INVALID_REG_ADDR,
[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
[BQ27XXX_REG_NAC] = INVALID_REG_ADDR,
+   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
[BQ27XXX_REG_FCC] = INVALID_REG_ADDR,
[BQ27XXX_REG_CYCT] = INVALID_REG_ADDR,
[BQ27XXX_REG_AE] = INVALID_REG_ADDR,
@@ -361,6 +372,7 @@ static u8
[BQ27XXX_REG_TTES] = INVALID_REG_ADDR,
[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = 0x10,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x2a

Re: [PATCH] power: supply: bq27xxx: Supporrt CHARGE_NOW for bq27z561/bq28z610/bq34z100

2020-12-21 Thread Hermes Zhang
On 12/18/20 5:42 PM, Pali Rohár wrote:
> On Thursday 17 December 2020 12:03:24 Hermes Zhang wrote:
>> Hi Pali,
>>
>>  From the TI spec (e.g. 
>> https://www.ti.com/lit/ug/tidu077/tidu077.pdf?ts=1608206347022_url=https%253A%252F%252Fwww.google.com%252F)
>>  , the NAC and RC (RemainingCapacity) are different:
>>
>> 4.5 NominalAvailableCapacity( ): 0x08/0x09
>> This read-only command pair returns the uncompensated (less than C/20 load) 
>> battery capacity
>> remaining. Units are mAh.
>>
>> 4.7 RemainingCapacity( ): 0x0c/0x0d
>> This read-only command pair returns the compensated battery capacity 
>> remaining. Units are mAh.
> Ok, thank you for explanation!
>
>> But for some chip e.g. bq27z561 it doesn't have the NAC Reg, so I prefer to 
>> use RC instead.
> I see that in your patch every configuration has either NAC or RC, but
> none both (and some has none of them). Does it mean that every chip has
> NAC or RC, but not both?

No, some chip will have both (e.g. bq27421), the purpose is to make the 
CHARGE_NOW property work for some chip doesn't have the NAC reg. E.g. we 
upgrade the gauge chip from one chip has the NAC but to another chip 
doesn't have, so to be keep the same API for the application, we make 
the CHARGE_NOW point to the RC reg value. For the other chips we keep 
the same settings as before.

Do you think I need to also set the right REG value for all the chips 
which have the RC reg?


Best Regards,

Hermes

>
>> Best Regards,
>> Hermes
>>
>> -Original Message-
>> From: Pali Rohár 
>> Sent: 2020年12月17日 19:57
>> To: Hermes Zhang 
>> Cc: Dan Murphy ; Sebastian Reichel ; kernel 
>> ; Hermes Zhang ; 
>> linux...@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH] power: supply: bq27xxx: Supporrt CHARGE_NOW for 
>> bq27z561/bq28z610/bq34z100
>>
>> On Thursday 17 December 2020 19:47:37 Hermes Zhang wrote:
>>> From: Hermes Zhang 
>>>
>>> The CHARGE_NOW is map to REG_NAC for all the gauge chips beofre. But
>>> for some chips (e.g. bq27z561) which doesn't have the REG_NAC, we use
>>> REG_RC (remaining capacity) for CHARGE_NOW.
>> Hello! What is the difference between NAC and RC? Is not it same thing?
>> I'm asking because for me from this patch for power supply API purpose it is 
>> the same thing... And therefore if it does not make sense to define for 
>> bq27z561 chip BQ27XXX_REG_NAC reg value to value which you used in 
>> BQ27XXX_REG_RC (to simplify whole implementation).
>>
>>> Signed-off-by: Hermes Zhang 
>>> ---
>>>   drivers/power/supply/bq27xxx_battery.c | 35
>>> +-
>>>   1 file changed, 34 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/power/supply/bq27xxx_battery.c
>>> b/drivers/power/supply/bq27xxx_battery.c
>>> index 315e0909e6a4..c1a49a598e9b 100644
>>> --- a/drivers/power/supply/bq27xxx_battery.c
>>> +++ b/drivers/power/supply/bq27xxx_battery.c
>>> @@ -110,6 +110,7 @@ enum bq27xxx_reg_index {
>>> BQ27XXX_REG_TTES,   /* Time-to-Empty Standby */
>>> BQ27XXX_REG_TTECP,  /* Time-to-Empty at Constant Power */
>>> BQ27XXX_REG_NAC,/* Nominal Available Capacity */
>>> +   BQ27XXX_REG_RC, /* Remaining Capacity */
>>> BQ27XXX_REG_FCC,/* Full Charge Capacity */
>>> BQ27XXX_REG_CYCT,   /* Cycle Count */
>>> BQ27XXX_REG_AE, /* Available Energy */
>>> @@ -145,6 +146,7 @@ static u8
>>> [BQ27XXX_REG_TTES] = 0x1c,
>>> [BQ27XXX_REG_TTECP] = 0x26,
>>> [BQ27XXX_REG_NAC] = 0x0c,
>>> +   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>>> [BQ27XXX_REG_FCC] = 0x12,
>>> [BQ27XXX_REG_CYCT] = 0x2a,
>>> [BQ27XXX_REG_AE] = 0x22,
>>> @@ -169,6 +171,7 @@ static u8
>>> [BQ27XXX_REG_TTES] = 0x1c,
>>> [BQ27XXX_REG_TTECP] = 0x26,
>>> [BQ27XXX_REG_NAC] = 0x0c,
>>> +   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>>> [BQ27XXX_REG_FCC] = 0x12,
>>> [BQ27XXX_REG_CYCT] = 0x2a,
>>> [BQ27XXX_REG_AE] = INVALID_REG_ADDR, @@ -193,6 +196,7 @@ static 
>>> u8
>>> [BQ27XXX_REG_TTES] = 0x1a,
>>> [BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>>> [BQ27XXX_REG_NAC] = 0x0c,
>>> +   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>>> [BQ27XXX_REG_FCC] = 0x12,
>>>  

RE: [PATCH] power: supply: bq27xxx: Supporrt CHARGE_NOW for bq27z561/bq28z610/bq34z100

2020-12-17 Thread Hermes Zhang
Hi Pali,

From the TI spec (e.g. 
https://www.ti.com/lit/ug/tidu077/tidu077.pdf?ts=1608206347022_url=https%253A%252F%252Fwww.google.com%252F)
 , the NAC and RC (RemainingCapacity) are different:

4.5 NominalAvailableCapacity( ): 0x08/0x09
This read-only command pair returns the uncompensated (less than C/20 load) 
battery capacity
remaining. Units are mAh.

4.7 RemainingCapacity( ): 0x0c/0x0d
This read-only command pair returns the compensated battery capacity remaining. 
Units are mAh.

But for some chip e.g. bq27z561 it doesn't have the NAC Reg, so I prefer to use 
RC instead.

Best Regards,
Hermes

-Original Message-
From: Pali Rohár  
Sent: 2020年12月17日 19:57
To: Hermes Zhang 
Cc: Dan Murphy ; Sebastian Reichel ; kernel 
; Hermes Zhang ; 
linux...@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] power: supply: bq27xxx: Supporrt CHARGE_NOW for 
bq27z561/bq28z610/bq34z100

On Thursday 17 December 2020 19:47:37 Hermes Zhang wrote:
> From: Hermes Zhang 
> 
> The CHARGE_NOW is map to REG_NAC for all the gauge chips beofre. But 
> for some chips (e.g. bq27z561) which doesn't have the REG_NAC, we use 
> REG_RC (remaining capacity) for CHARGE_NOW.

Hello! What is the difference between NAC and RC? Is not it same thing?
I'm asking because for me from this patch for power supply API purpose it is 
the same thing... And therefore if it does not make sense to define for 
bq27z561 chip BQ27XXX_REG_NAC reg value to value which you used in 
BQ27XXX_REG_RC (to simplify whole implementation).

> Signed-off-by: Hermes Zhang 
> ---
>  drivers/power/supply/bq27xxx_battery.c | 35 
> +-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/bq27xxx_battery.c 
> b/drivers/power/supply/bq27xxx_battery.c
> index 315e0909e6a4..c1a49a598e9b 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -110,6 +110,7 @@ enum bq27xxx_reg_index {
>   BQ27XXX_REG_TTES,   /* Time-to-Empty Standby */
>   BQ27XXX_REG_TTECP,  /* Time-to-Empty at Constant Power */
>   BQ27XXX_REG_NAC,/* Nominal Available Capacity */
> + BQ27XXX_REG_RC, /* Remaining Capacity */
>   BQ27XXX_REG_FCC,/* Full Charge Capacity */
>   BQ27XXX_REG_CYCT,   /* Cycle Count */
>   BQ27XXX_REG_AE, /* Available Energy */
> @@ -145,6 +146,7 @@ static u8
>   [BQ27XXX_REG_TTES] = 0x1c,
>   [BQ27XXX_REG_TTECP] = 0x26,
>   [BQ27XXX_REG_NAC] = 0x0c,
> + [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>   [BQ27XXX_REG_FCC] = 0x12,
>   [BQ27XXX_REG_CYCT] = 0x2a,
>   [BQ27XXX_REG_AE] = 0x22,
> @@ -169,6 +171,7 @@ static u8
>   [BQ27XXX_REG_TTES] = 0x1c,
>   [BQ27XXX_REG_TTECP] = 0x26,
>   [BQ27XXX_REG_NAC] = 0x0c,
> + [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>   [BQ27XXX_REG_FCC] = 0x12,
>   [BQ27XXX_REG_CYCT] = 0x2a,
>   [BQ27XXX_REG_AE] = INVALID_REG_ADDR, @@ -193,6 +196,7 @@ static 
> u8
>   [BQ27XXX_REG_TTES] = 0x1a,
>   [BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>   [BQ27XXX_REG_NAC] = 0x0c,
> + [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>   [BQ27XXX_REG_FCC] = 0x12,
>   [BQ27XXX_REG_CYCT] = 0x2a,
>   [BQ27XXX_REG_AE] = INVALID_REG_ADDR, @@ -215,6 +219,7 @@ static 
> u8
>   [BQ27XXX_REG_TTES] = 0x1c,
>   [BQ27XXX_REG_TTECP] = 0x26,
>   [BQ27XXX_REG_NAC] = 0x0c,
> + [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>   [BQ27XXX_REG_FCC] = 0x12,
>   [BQ27XXX_REG_CYCT] = 0x2a,
>   [BQ27XXX_REG_AE] = 0x22,
> @@ -237,6 +242,7 @@ static u8
>   [BQ27XXX_REG_TTES] = 0x1a,
>   [BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>   [BQ27XXX_REG_NAC] = 0x0c,
> + [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>   [BQ27XXX_REG_FCC] = 0x12,
>   [BQ27XXX_REG_CYCT] = 0x1e,
>   [BQ27XXX_REG_AE] = INVALID_REG_ADDR, @@ -257,6 +263,7 @@ static 
> u8
>   [BQ27XXX_REG_TTES] = 0x1c,
>   [BQ27XXX_REG_TTECP] = 0x26,
>   [BQ27XXX_REG_NAC] = 0x0c,
> + [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>   [BQ27XXX_REG_FCC] = 0x12,
>   [BQ27XXX_REG_CYCT] = INVALID_REG_ADDR,
>   [BQ27XXX_REG_AE] = 0x22,
> @@ -277,6 +284,7 @@ static u8
>   [BQ27XXX_REG_TTES] = 0x1c,
>   [BQ27XXX_REG_TTECP] = 0x26,
>   [BQ27XXX_REG_NAC] = 0x0c,
> + [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>

[PATCH] power: supply: bq27xxx: Supporrt CHARGE_NOW for bq27z561/bq28z610/bq34z100

2020-12-17 Thread Hermes Zhang
From: Hermes Zhang 

The CHARGE_NOW is map to REG_NAC for all the gauge chips beofre. But for
some chips (e.g. bq27z561) which doesn't have the REG_NAC, we use REG_RC
(remaining capacity) for CHARGE_NOW.

Signed-off-by: Hermes Zhang 
---
 drivers/power/supply/bq27xxx_battery.c | 35 +-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/bq27xxx_battery.c 
b/drivers/power/supply/bq27xxx_battery.c
index 315e0909e6a4..c1a49a598e9b 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -110,6 +110,7 @@ enum bq27xxx_reg_index {
BQ27XXX_REG_TTES,   /* Time-to-Empty Standby */
BQ27XXX_REG_TTECP,  /* Time-to-Empty at Constant Power */
BQ27XXX_REG_NAC,/* Nominal Available Capacity */
+   BQ27XXX_REG_RC, /* Remaining Capacity */
BQ27XXX_REG_FCC,/* Full Charge Capacity */
BQ27XXX_REG_CYCT,   /* Cycle Count */
BQ27XXX_REG_AE, /* Available Energy */
@@ -145,6 +146,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1c,
[BQ27XXX_REG_TTECP] = 0x26,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x2a,
[BQ27XXX_REG_AE] = 0x22,
@@ -169,6 +171,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1c,
[BQ27XXX_REG_TTECP] = 0x26,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x2a,
[BQ27XXX_REG_AE] = INVALID_REG_ADDR,
@@ -193,6 +196,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1a,
[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x2a,
[BQ27XXX_REG_AE] = INVALID_REG_ADDR,
@@ -215,6 +219,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1c,
[BQ27XXX_REG_TTECP] = 0x26,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x2a,
[BQ27XXX_REG_AE] = 0x22,
@@ -237,6 +242,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1a,
[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x1e,
[BQ27XXX_REG_AE] = INVALID_REG_ADDR,
@@ -257,6 +263,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1c,
[BQ27XXX_REG_TTECP] = 0x26,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = INVALID_REG_ADDR,
[BQ27XXX_REG_AE] = 0x22,
@@ -277,6 +284,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1c,
[BQ27XXX_REG_TTECP] = 0x26,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x2a,
[BQ27XXX_REG_AE] = 0x22,
@@ -297,6 +305,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1c,
[BQ27XXX_REG_TTECP] = 0x26,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x2a,
[BQ27XXX_REG_AE] = 0x22,
@@ -317,6 +326,7 @@ static u8
[BQ27XXX_REG_TTES] = 0x1c,
[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x1e,
[BQ27XXX_REG_AE] = INVALID_REG_ADDR,
@@ -337,6 +347,7 @@ static u8
[BQ27XXX_REG_TTES] = INVALID_REG_ADDR,
[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
[BQ27XXX_REG_NAC] = INVALID_REG_ADDR,
+   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
[BQ27XXX_REG_FCC] = INVALID_REG_ADDR,
[BQ27XXX_REG_CYCT] = INVALID_REG_ADDR,
[BQ27XXX_REG_AE] = INVALID_REG_ADDR,
@@ -361,6 +372,7 @@ static u8
[BQ27XXX_REG_TTES] = INVALID_REG_ADDR,
[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
[BQ27XXX_REG_NAC] = 0x0c,
+   [BQ27XXX_REG_RC] = INVALID_REG_ADDR,
[BQ27XXX_REG_FCC] = 0x12,
[BQ27XXX_REG_CYCT] = 0x2a,
[BQ27XXX_REG_AE] = INVALID_REG_ADDR,
@@ -382,6 +394,7 @@ static u8