| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 10 Mar 2020 15:45:12 +0000
From: Daniel Thompson <daniel.thompson@...aro.org>
To: Kiran Gunda <kgunda@...eaurora.org>
Cc: bjorn.andersson@...aro.org, jingoohan1@...il.com,
lee.jones@...aro.org, b.zolnierkie@...sung.com,
dri-devel@...ts.freedesktop.org, jacek.anaszewski@...il.com,
pavel@....cz, robh+dt@...nel.org, mark.rutland@....com,
linux-leds@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, Dan Murphy <dmurphy@...com>,
Andy Gross <agross@...nel.org>, linux-arm-msm@...r.kernel.org,
linux-fbdev@...r.kernel.org
Subject: Re: [PATCH V3 3/4] backlight: qcom-wled: Add support for WLED5
peripheral in PM8150L
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@...eaurora.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?
> 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.
> + 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).
> 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.
Powered by blists - more mailing lists