Re: [PATCH V3 3/4] backlight: qcom-wled: Add support for WLED5 peripheral in PM8150L
From: Daniel Thompson <hidden>
Date: 2020-03-10 15:45:20
Also in:
dri-devel, linux-arm-msm, linux-devicetree, linux-leds, lkml
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 <redacted>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?
quoted hunk ↗ jump to hunk
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)?
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.
quoted hunk ↗ jump to hunk
+ 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).
quoted hunk ↗ jump to hunk
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. Daniel.