Messages in this thread | | | Date | Wed, 11 Mar 2020 12:30:06 +0530 | From | kgunda@codeauro ... | Subject | Re: [PATCH V3 3/4] backlight: qcom-wled: Add support for WLED5 peripheral in PM8150L |
| |
On 2020-03-10 21:15, Daniel Thompson wrote: > On Mon, Mar 09, 2020 at 06:56:01PM +0530, Kiran Gunda wrote: >> Add support for WLED5 peripheral that is present on PM8150L PMICs. >> >> PM8150L WLED supports the following: >> - Two modulators and each sink can use any of the modulator >> - Multiple CABC selection options >> - Multiple brightness width selection (12 bits to 15 bits) >> >> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org> > > Mostly just style comments below... > > >> --- >> .../bindings/leds/backlight/qcom-wled.yaml | 39 +++ >> drivers/video/backlight/qcom-wled.c | 336 >> ++++++++++++++++++++- > > Shouldn't the bindings and driver be separate? > > Ok. I will split it out in to a separate patch in next post. >> 2 files changed, 374 insertions(+), 1 deletion(-) >> >> diff --git >> a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml >> b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml >> index d334f81..e0dadc4 100644 >> --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml >> +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml >> @@ -20,6 +20,7 @@ properties: >> - qcom,pm8941-wled >> - qcom,pmi8998-wled >> - qcom,pm660l-wled >> + - qcom,pm8150l-wled >> >> reg: >> maxItems: 1 >> @@ -28,10 +29,23 @@ properties: >> maxItems: 1 >> description: >> brightness value on boot, value from 0-4095. >> + For pm8150l this value vary from 0-4095 or 0-32767 >> + depending on the brightness control mode. If CABC is >> + enabled 0-4095 range is used. > > Is this a pm8150l restriction or a WLED5 restriction (will other WLED5 > have different ranges)? > It is a WLED5 restriction which is used in pm8150l PMIC. > >> allOf: >> - $ref: /schemas/types.yaml#/definitions/uint32 >> default: 2048 >> >> + max-brightness: >> + maxItems: 1 >> + description: >> + Maximum brightness level. Allowed values are, >> + for pmi8998 it is 0-4095. >> + For pm8150l, this can be either 4095 or 32767. > > Ditto. > It is a WLED5 restriction which is used in pm8150l PMIC. > >> + If CABC is enabled, this is capped to 4095. >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/uint32 >> + >> label: >> maxItems: 1 >> description: >> @@ -124,6 +138,31 @@ properties: >> value for PM8941 from 1 to 3. Default 2 >> For PMI8998 from 1 to 4. >> >> + qcom,modulator-sel: >> + maxItems: 1 >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + Selects the modulator used for brightness modulation. >> + Allowed values are, >> + 0 - Modulator A >> + 1 - Modulator B >> + If not specified, then modulator A will be used by default. >> + This property is applicable only to WLED5 peripheral. >> + >> + qcom,cabc-sel: >> + maxItems: 1 >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + Selects the CABC pin signal used for brightness modulation. >> + Allowed values are, >> + 0 - CABC disabled >> + 1 - CABC 1 >> + 2 - CABC 2 >> + 3 - External signal (e.g. LPG) is used for dimming >> + This property is applicable only to WLED5 peripheral. >> + >> interrupts: >> maxItems: 2 >> description: >> diff --git a/drivers/video/backlight/qcom-wled.c >> b/drivers/video/backlight/qcom-wled.c >> index b73f273..edbbcb2 100644 >> --- a/drivers/video/backlight/qcom-wled.c >> +++ b/drivers/video/backlight/qcom-wled.c >> @@ -19,6 +19,8 @@ >> #define WLED_DEFAULT_BRIGHTNESS 2048 >> #define WLED_SOFT_START_DLY_US 10000 >> #define WLED3_SINK_REG_BRIGHT_MAX 0xFFF >> +#define WLED5_SINK_REG_BRIGHT_MAX_12B 0xFFF >> +#define WLED5_SINK_REG_BRIGHT_MAX_15B 0x7FFF >> >> /* WLED3/WLED4 control registers */ >> #define WLED3_CTRL_REG_FAULT_STATUS 0x08 >> @@ -40,6 +42,7 @@ >> >> #define WLED3_CTRL_REG_OVP 0x4d >> #define WLED3_CTRL_REG_OVP_MASK GENMASK(1, 0) >> +#define WLED5_CTRL_REG_OVP_MASK GENMASK(3, 0) >> >> #define WLED3_CTRL_REG_ILIMIT 0x4e >> #define WLED3_CTRL_REG_ILIMIT_MASK GENMASK(2, 0) >> @@ -101,6 +104,40 @@ >> >> #define WLED4_SINK_REG_BRIGHT(n) (0x57 + (n * 0x10)) >> >> +/* WLED5 specific sink registers */ >> +#define WLED5_SINK_REG_MOD_A_EN 0x50 >> +#define WLED5_SINK_REG_MOD_B_EN 0x60 >> +#define WLED5_SINK_REG_MOD_EN_MASK BIT(7) >> + >> +#define WLED5_SINK_REG_MOD_A_SRC_SEL 0x51 >> +#define WLED5_SINK_REG_MOD_B_SRC_SEL 0x61 >> +#define WLED5_SINK_REG_MOD_SRC_SEL_HIGH 0 >> +#define WLED5_SINK_REG_MOD_SRC_SEL_EXT 0x03 >> +#define WLED5_SINK_REG_MOD_SRC_SEL_MASK GENMASK(1, 0) >> + >> +#define WLED5_SINK_REG_MOD_A_BRIGHTNESS_WIDTH_SEL 0x52 >> +#define WLED5_SINK_REG_MOD_B_BRIGHTNESS_WIDTH_SEL 0x62 >> +#define WLED5_SINK_REG_BRIGHTNESS_WIDTH_12B 0 >> +#define WLED5_SINK_REG_BRIGHTNESS_WIDTH_15B 1 >> + >> +#define WLED5_SINK_REG_MOD_A_BRIGHTNESS_LSB 0x53 >> +#define WLED5_SINK_REG_MOD_A_BRIGHTNESS_MSB 0x54 >> +#define WLED5_SINK_REG_MOD_B_BRIGHTNESS_LSB 0x63 >> +#define WLED5_SINK_REG_MOD_B_BRIGHTNESS_MSB 0x64 >> + >> +#define WLED5_SINK_REG_MOD_SYNC_BIT 0x65 >> +#define WLED5_SINK_REG_SYNC_MOD_A_BIT BIT(0) >> +#define WLED5_SINK_REG_SYNC_MOD_B_BIT BIT(1) >> +#define WLED5_SINK_REG_SYNC_MASK GENMASK(1, 0) >> + >> +/* WLED5 specific per-'string' registers below */ >> +#define WLED5_SINK_REG_STR_FULL_SCALE_CURR(n) (0x72 + (n * 0x10)) >> + >> +#define WLED5_SINK_REG_STR_SRC_SEL(n) (0x73 + (n * 0x10)) >> +#define WLED5_SINK_REG_SRC_SEL_MOD_A 0 >> +#define WLED5_SINK_REG_SRC_SEL_MOD_B 1 >> +#define WLED5_SINK_REG_SRC_SEL_MASK GENMASK(1, 0) >> + >> struct wled_var_cfg { >> const u32 *values; >> u32 (*fn)(u32); >> @@ -125,6 +162,8 @@ struct wled_config { >> u32 num_strings; >> u32 string_i_limit; >> u32 enabled_strings[WLED_MAX_STRINGS]; > >> + u32 mod_sel; >> + u32 cabc_sel; > > Please explain cabc_sel (wled5) versus cabc_en (wled4). > Ok. WLED5 has 2 CABC modules, from which one can be selected/enabled. wled4 has only one CABC module and it is just enabled based on the user input. I will add a comment in next post.
>> bool cs_out_en; >> bool ext_gen; >> bool cabc; >> @@ -164,6 +203,27 @@ struct wled { >> int (*wled_ovp_delay)(struct wled *wled); >> }; >> >> +enum wled5_mod_sel { >> + MOD_A, >> + MOD_B, >> + MOD_MAX, >> +}; >> + >> +static const u8 wled5_brightness_reg[MOD_MAX] = { >> + [MOD_A] = WLED5_SINK_REG_MOD_A_BRIGHTNESS_LSB, >> + [MOD_B] = WLED5_SINK_REG_MOD_B_BRIGHTNESS_LSB, >> +}; >> + >> +static const u8 wled5_src_sel_reg[MOD_MAX] = { >> + [MOD_A] = WLED5_SINK_REG_MOD_A_SRC_SEL, >> + [MOD_B] = WLED5_SINK_REG_MOD_B_SRC_SEL, >> +}; >> + >> +static const u8 wled5_brt_wid_sel_reg[MOD_MAX] = { >> + [MOD_A] = WLED5_SINK_REG_MOD_A_BRIGHTNESS_WIDTH_SEL, >> + [MOD_B] = WLED5_SINK_REG_MOD_B_BRIGHTNESS_WIDTH_SEL, >> +}; >> + >> static int wled3_set_brightness(struct wled *wled, u16 brightness) >> { >> int rc, i; >> @@ -182,6 +242,25 @@ static int wled3_set_brightness(struct wled >> *wled, u16 brightness) >> return 0; >> } >> >> +static int wled5_set_brightness(struct wled *wled, u16 brightness) >> +{ >> + int rc, offset; >> + u16 low_limit = wled->max_brightness * 1 / 1000; >> + u8 v[2]; >> + >> + /* WLED5's lower limit is 0.1% */ >> + if (brightness < low_limit) >> + brightness = low_limit; >> + >> + v[0] = brightness & 0xff; >> + v[1] = (brightness >> 8) & 0x7f; >> + >> + offset = wled5_brightness_reg[wled->cfg.mod_sel]; >> + rc = regmap_bulk_write(wled->regmap, wled->sink_addr + offset, >> + v, 2); >> + return rc; >> +} >> + > > Can we keep the same ordering throughout the file (wled3, wled4, > wled5)? > Most of the wled5 callbacks seem to have been inserted above wled4. > Sure. Will do it in next post. > > Daniel.
| |