Re: [PATCH v2 3/7] leds: leds-mt6323: Specify registers and specs in platform data

Pavel Machek
2023-04-13T10:58:50Z

[-- Attachment #1.1: Type: text/plain, Size: 2716 bytes --]

Hi!

> In order to enhance the flexibility of this driver and let it support
> more than just one MediaTek LEDs IP for more than just one PMIC,
> add platform data structure specifying the register offsets and
> data that commonly varies between different IPs.
>
> This commit brings no functional changes.

> @@ -63,12 +61,9 @@
> #define MT6323_ISINK_CH_EN_MASK(i) BIT(i)
> #define MT6323_ISINK_CH_EN(i) BIT(i)
>
> -#define MT6323_MAX_PERIOD 10000
> -#define MT6323_MAX_LEDS 4
> -#define MT6323_MAX_BRIGHTNESS 6
> -#define MT6323_UNIT_DUTY 3125
> -#define MT6323_CAL_HW_DUTY(o, p) DIV_ROUND_CLOSEST((o) * 100000ul,\
> - (p) * MT6323_UNIT_DUTY)
> +#define MAX_SUPPORTED_LEDS 8
> +#define MT6323_CAL_HW_DUTY(o, p, u) DIV_ROUND_CLOSEST((o) * 100000ul,\
> + (p) * (u))
>

You increase number of supported leds from 4 to 8. That's ok, but I'd
not call it "no functional changes".

> +/**
> + * struct mt6323_regs - register spec for the LED device
> + * @top_ckpdn: Offset to ISINK_CKPDN[0..x] registers
> + * @num_top_ckpdn: Number of ISINK_CKPDN registers
> + * @top_ckcon: Offset to ISINK_CKCON[0..x] registers
> + * @num_top_ckcon: Number of ISINK_CKCON registers
> + * @isink_con: Offset to ISINKx_CON[0..x] registers
> + * @num_isink_con: Number of ISINKx_CON registers
> + * @isink_max_regs: Number of ISINK[0..x] registers
> + * @isink_en_ctrl: Offset to ISINK_EN_CTRL register
> + */
> +struct mt6323_regs {
> + const u16 *top_ckpdn;
> + u8 num_top_ckpdn;
> + const u16 *top_ckcon;
> + u8 num_top_ckcon;
> + const u16 *isink_con;
> + u8 num_isink_con;
> + u8 isink_max_regs;
> + u16 isink_en_ctrl;
> +};

I'd use ints here. Should get similar memory usage and maybe less code
size. But ... no need to resubmit just for this.

> @@ -469,8 +525,31 @@ static int mt6323_led_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct mt6323_regs mt6323_registers = {
> + .top_ckpdn = (const u16[]){ 0x102, 0x106, 0x10e },
> + .num_top_ckpdn = 3,
> + .top_ckcon = (const u16[]){ 0x120, 0x126 },
> + .num_top_ckcon = 2,
> + .isink_con = (const u16[]){ 0x330, 0x332, 0x334 },
> + .num_isink_con = 3,
> + .isink_max_regs = 4, /* ISINK[0..3] */
> + .isink_en_ctrl = 0x356,
> +};
> +
> +static const struct mt6323_hwspec mt6323_spec = {
> + .max_period = 10000,
> + .max_leds = 4,
> + .max_brightness = 6,
> + .unit_duty = 3125,
> +};
> +
> +static const struct mt6323_data mt6323_pdata = {
> + .regs = &mt6323_registers,
> + .spec = &mt6323_spec,
> +};
> +

Acked-by: Pavel Machek <pavel@ucw.cz>

BR,
Pavel

--
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]