Re: [PATCH] power: supply: bq27xxx: Return the value instead of -ENODATA
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
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
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
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
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
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
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
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
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
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
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
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
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
> > > > 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
> > > > 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
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
> -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
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
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
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
> > + 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
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
> > 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
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
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
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
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
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
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