Re: [PATCH v6 2/3] leds: aw2013: Add AW2027 support

[David Heidelberg <david@ixit.cz>]

On 13/11/2026 01:01, Paul Sajna via B4 Relay wrote:
> From: Paul Sajna <sajattack@postmarketos.org>
> 
> The AW2027 is basically the same as the AW2013.
> 
> It differs in:
> - the max current (30mA rather than 15)
> - the number of current levels for colour-mixing (16 rather than 4)
> - the supply voltage max
> - the charge indicator (unsupported by our driver at this time)
> 
> The max current register (imax) also moves from LCFG to GCR2, and the
> selectable current levels for colour-mixing (LCFG_CUR) take their place.
> 
> The current driver implementation provided here sets the global imax to
> it's maximum, and scales each led to the closest value using the local
> imax. It's the best result achievable short of adding custom sysfs
> entries and mucking up the linux interfaces.
> 
> Comparison of all AW20XX chips https://www.awinic.com/en/productCompareList?id=47&ids=1022,74,96,75,72,71,928,927,73,76,578
> AW2027 datasheet: https://doc.awinic.com/doc/202507/77b87097-27bf-48cd-af9b-9b615064c1e1.pdf
> Vendor driver: https://www.awinic.com/en/driver/AW2027DNR/454
> 
> Signed-off-by: Paul Sajna <sajattack@postmarketos.org>
> ---
>   drivers/leds/leds-aw2013.c | 190 ++++++++++++++++++++++++++++++++++++++-------
>   1 file changed, 163 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/leds/leds-aw2013.c b/drivers/leds/leds-aw2013.c
> index cf20e94920fd..5ff9887086a6 100644
> --- a/drivers/leds/leds-aw2013.c
> +++ b/drivers/leds/leds-aw2013.c
> @@ -1,5 +1,5 @@
>   // SPDX-License-Identifier: GPL-2.0+
> -// Driver for Awinic AW2013 3-channel LED driver
> +// Driver for Awinic AW2013/AW2027 3-channel LED drivers
>   
>   #include <linux/i2c.h>
>   #include <linux/leds.h>
> @@ -20,13 +20,15 @@
>   #define AW2013_GCR 0x01
>   #define AW2013_GCR_ENABLE BIT(0)
>   
> +#define AW20XX_GCR2 0x04
> +
>   /* LED channel enable register */
>   #define AW2013_LCTR 0x30
>   #define AW2013_LCTR_LE(x) BIT((x))
>   
>   /* LED channel control registers */
>   #define AW2013_LCFG(x) (0x31 + (x))
> -#define AW2013_LCFG_IMAX_MASK (BIT(0) | BIT(1)) // Should be 0-3
> +#define AW2013_IMAX_MASK (BIT(0) | BIT(1)) // Should be 0-3
>   #define AW2013_LCFG_MD BIT(4)
>   #define AW2013_LCFG_FI BIT(5)
>   #define AW2013_LCFG_FO BIT(6)
> @@ -51,6 +53,16 @@
>   
>   #define AW2013_TIME_STEP 130 /* ms */
>   
> +/* AW2027-specific defines */
> +#define AW2016_23_27_33_CHIPID 0x09 /* vendor shared same chipid between several chips */
> +#define AW2027_REG_MAX 0x7F /* copied from vendor driver, but only up to 0x3F is documented */

just in case you'll be sending next revision, 0x7f (hex should go always lower 
case).

> +#define AW2027_LOCAL_IMAX_MASK GENMASK(3, 0)
> +
> +#define AW2027_GLOBAL_IMAX_15MA 0
> +#define AW2027_GLOBAL_IMAX_30MA 1
> +#define AW2027_GLOBAL_IMAX_5MA  2
> +#define AW2027_GLOBAL_IMAX_10MA 3
> +
>   struct aw2013;
>   

[...]

> @@ -142,9 +261,7 @@ static int aw2013_chip_enable(struct aw2013 *chip)
>   
>   static bool aw2013_chip_in_use(struct aw2013 *chip)
>   {
> -	int i;

My comment regarding to using C99 was aimed only at the new code, as you 
introducing it. If you modernizing existing code, you should do that in separate 
commit. Let maintainer decide if the split is needed here or it's ok as is :)

David

[...]