| 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: Mon, 9 Apr 2018 18:14:33 +0200
From: Sebastian Reichel <sebastian.reichel@...labora.co.uk>
To: Daniel Thompson <daniel.thompson@...aro.org>
Cc: Milo Kim <Milo.Kim@...com>, Lee Jones <lee.jones@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Tony Lindgren <tony@...mide.com>,
Jingoo Han <jingoohan1@...il.com>,
Mark Rutland <mark.rutland@....com>,
linux-kernel@...r.kernel.org, linux-fbdev@...r.kernel.org,
devicetree@...r.kernel.org, linux-omap@...r.kernel.org
Subject: Re: [PATCHv4 08/10] backlight: add TI LMU backlight driver
Hi Daniel,
On Wed, Apr 04, 2018 at 03:57:39PM +0100, Daniel Thompson wrote:
> On Fri, Mar 30, 2018 at 07:24:12PM +0200, Sebastian Reichel wrote:
> > This adds backlight support for the following TI LMU
> > chips: LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.
> >
> > Signed-off-by: Milo Kim <milo.kim@...com>
> > [add LED subsystem support for keyboard backlight and rework DT
>
> Milo's mail has be bouncing for a very long time now. Did they really
> sign off this code or is this intended to be an authorship credit?
I took over his patches from ~ a year ago and reworked them.
> > binding according to Rob Herrings feedback]
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@...labora.co.uk>
> > ---
> > drivers/video/backlight/Kconfig | 7 +
> > drivers/video/backlight/Makefile | 3 +
> > drivers/video/backlight/ti-lmu-backlight-core.c | 666 ++++++++++++++++++++++++
> > drivers/video/backlight/ti-lmu-backlight-data.c | 304 +++++++++++
> > drivers/video/backlight/ti-lmu-backlight-data.h | 95 ++++
> > 5 files changed, 1075 insertions(+)
> > create mode 100644 drivers/video/backlight/ti-lmu-backlight-core.c
> > create mode 100644 drivers/video/backlight/ti-lmu-backlight-data.c
> > create mode 100644 drivers/video/backlight/ti-lmu-backlight-data.h
> >
> > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> > index 5d2d0d7e8100..27e6c5a0add8 100644
> > --- a/drivers/video/backlight/Kconfig
> > +++ b/drivers/video/backlight/Kconfig
> > @@ -427,6 +427,13 @@ config BACKLIGHT_SKY81452
> > To compile this driver as a module, choose M here: the module will
> > be called sky81452-backlight
> >
> > +config BACKLIGHT_TI_LMU
> > + tristate "Backlight driver for TI LMU"
> > + depends on BACKLIGHT_CLASS_DEVICE && MFD_TI_LMU
> > + help
> > + Say Y to enable the backlight driver for TI LMU devices.
> > + This supports LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.
> > +
> > config BACKLIGHT_TPS65217
> > tristate "TPS65217 Backlight"
> > depends on BACKLIGHT_CLASS_DEVICE && MFD_TPS65217
> > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> > index 19da71d518bf..a1132d3dfd4c 100644
> > --- a/drivers/video/backlight/Makefile
> > +++ b/drivers/video/backlight/Makefile
> > @@ -53,6 +53,9 @@ obj-$(CONFIG_BACKLIGHT_PM8941_WLED) += pm8941-wled.o
> > obj-$(CONFIG_BACKLIGHT_PWM) += pwm_bl.o
> > obj-$(CONFIG_BACKLIGHT_SAHARA) += kb3886_bl.o
> > obj-$(CONFIG_BACKLIGHT_SKY81452) += sky81452-backlight.o
> > +ti-lmu-backlight-objs := ti-lmu-backlight-core.o \
> > + ti-lmu-backlight-data.o
> > +obj-$(CONFIG_BACKLIGHT_TI_LMU) += ti-lmu-backlight.o
> > obj-$(CONFIG_BACKLIGHT_TOSA) += tosa_bl.o
> > obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
> > obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o
> > diff --git a/drivers/video/backlight/ti-lmu-backlight-core.c b/drivers/video/backlight/ti-lmu-backlight-core.c
> > new file mode 100644
> > index 000000000000..a6099581edd7
> > --- /dev/null
> > +++ b/drivers/video/backlight/ti-lmu-backlight-core.c
> > @@ -0,0 +1,666 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2015 Texas Instruments
> > + * Copyright 2018 Sebastian Reichel
> > + *
> > + * TI LMU Backlight driver, based on previous work from
> > + * Milo Kim <milo.kim@...com>
> > + */
> > +
> > +#include <linux/backlight.h>
> > +#include <linux/bitops.h>
> > +#include <linux/device.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/leds.h>
> > +#include <linux/mfd/ti-lmu.h>
> > +#include <linux/mfd/ti-lmu-register.h>
> > +#include <linux/module.h>
> > +#include <linux/notifier.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include "ti-lmu-backlight-data.h"
> > +
> > +enum ti_lmu_bl_ctrl_mode {
> > + BL_REGISTER_BASED,
> > + BL_PWM_BASED,
> > +};
> > +
> > +enum ti_lmu_bl_type {
> > + TI_LMU_BL, /* backlight userspace interface */
> > + TI_LMU_LED, /* led userspace interface */
> > +};
> > +
> > +enum ti_lmu_bl_ramp_mode {
> > + BL_RAMP_UP,
> > + BL_RAMP_DOWN,
> > +};
> > +
> > +#define DEFAULT_PWM_NAME "lmu-backlight"
> > +#define NUM_DUAL_CHANNEL 2
> > +#define LMU_BACKLIGHT_DUAL_CHANNEL_USED (BIT(0) | BIT(1))
> > +#define LMU_BACKLIGHT_11BIT_LSB_MASK (BIT(0) | BIT(1) | BIT(2))
> > +#define LMU_BACKLIGHT_11BIT_MSB_SHIFT 3
> > +
> > +struct ti_lmu_bank {
> > + struct device *dev;
> > + int bank_id;
> > + const struct ti_lmu_bl_cfg *cfg;
> > + struct ti_lmu *lmu;
> > + const char *label;
> > + int leds;
> > + int current_brightness;
> > + u32 default_brightness;
> > + u32 ramp_up_msec;
> > + u32 ramp_down_msec;
> > + u32 pwm_period;
> > + enum ti_lmu_bl_ctrl_mode mode;
> > + enum ti_lmu_bl_type type;
> > +
> > + struct notifier_block nb;
> > +
> > + struct backlight_device *backlight;
> > + struct led_classdev *led;
> > +};
> > +
> > +static int ti_lmu_bl_enable(struct ti_lmu_bank *lmu_bank, bool enable)
> > +{
> > + struct regmap *regmap = lmu_bank->lmu->regmap;
> > + unsigned long enable_time = lmu_bank->cfg->reginfo->enable_usec;
> > + u8 *reg = lmu_bank->cfg->reginfo->enable;
> > + u8 mask = BIT(lmu_bank->bank_id);
> > + u8 val = (enable == true) ? mask : 0;
> > + int ret;
> > +
> > + if (!reg)
> > + return -EINVAL;
> > +
> > + ret = regmap_update_bits(regmap, *reg, mask, val);
> > + if (ret)
> > + return ret;
> > +
> > + if (enable_time > 0)
> > + usleep_range(enable_time, enable_time + 100);
> > +
> > + return 0;
> > +}
> > +
> > +static int ti_lmu_bl_update_brightness_register(struct ti_lmu_bank *lmu_bank,
> > + int brightness)
> > +{
> > + const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
> > + const struct ti_lmu_bl_reg *reginfo = cfg->reginfo;
> > + struct regmap *regmap = lmu_bank->lmu->regmap;
> > + u8 reg, val;
> > + int ret;
> > +
> > + /*
> > + * Brightness register update
> > + *
> > + * 11 bit dimming: update LSB bits and write MSB byte.
> > + * MSB brightness should be shifted.
> > + * 8 bit dimming: write MSB byte.
> > + */
> > + if (cfg->max_brightness == MAX_BRIGHTNESS_11BIT) {
> > + reg = reginfo->brightness_lsb[lmu_bank->bank_id];
> > + ret = regmap_update_bits(regmap, reg,
> > + LMU_BACKLIGHT_11BIT_LSB_MASK,
> > + brightness);
> > + if (ret)
> > + return ret;
> > +
> > + val = brightness >> LMU_BACKLIGHT_11BIT_MSB_SHIFT;
> > + } else {
> > + val = brightness;
> > + }
> > +
> > + reg = reginfo->brightness_msb[lmu_bank->bank_id];
> > + return regmap_write(regmap, reg, val);
> > +}
> > +
> > +static int ti_lmu_bl_pwm_ctrl(struct ti_lmu_bank *lmu_bank, int brightness)
> > +{
> > + int max_brightness = lmu_bank->cfg->max_brightness;
> > + struct pwm_state state = { };
> > + int ret;
> > +
> > + if (!lmu_bank->lmu->pwm) {
> > + dev_err(lmu_bank->dev, "Missing PWM device!\n");
> > + return -ENODEV;
> > + }
> > +
> > + pwm_init_state(lmu_bank->lmu->pwm, &state);
> > + state.period = lmu_bank->pwm_period;
> > + state.duty_cycle = brightness * state.period / max_brightness;
> > +
> > + if (state.duty_cycle)
> > + state.enabled = true;
> > + else
> > + state.enabled = false;
> > +
> > + ret = pwm_apply_state(lmu_bank->lmu->pwm, &state);
> > + if (ret)
> > + dev_err(lmu_bank->dev, "Failed to configure PWM: %d", ret);
> > +
> > + return ret;
> > +}
> > +
> > +static int ti_lmu_bl_set_brightness(struct ti_lmu_bank *lmu_bank,
> > + int brightness)
> > +{
> > + const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
> > + bool enable = brightness > 0;
> > + int ret;
> > +
> > + ret = ti_lmu_bl_enable(lmu_bank, enable);
> > + if (ret)
> > + return ret;
> > +
> > + if (lmu_bank->mode == BL_PWM_BASED) {
> > + ti_lmu_bl_pwm_ctrl(lmu_bank, brightness);
> > +
> > + switch (cfg->pwm_action) {
> > + case UPDATE_PWM_ONLY:
> > + /* No register update is required */
> > + return 0;
> > + case UPDATE_MAX_BRT:
> > + /*
> > + * PWM can start from any non-zero code and dim down
> > + * to zero. So, brightness register should be updated
> > + * even in PWM mode.
> > + */
>
> This comment could do with a little expansion (I assume the bank's
> brightness register means something different when in PWM mode but the
> flow of the code is tricky to read).
I will consult the manual and see how exactly the PWM stuff
is supposed to work. The platform I need this driver for
(Motorola Droid 4) does not have/use the PWM feature.
> > + if (brightness > 0)
>
> Isn't this "enable"?
Yes, good catch!
> > + brightness = MAX_BRIGHTNESS_11BIT;
> > + else
> > + brightness = 0;
> > + break;
> > + default:
>
> case UPDATE_PWM_AND_BRT_REGISTER:
ok.
>
> > + break;
> > + }
> > + }
> > +
> > + lmu_bank->current_brightness = brightness;
> > +
> > + return ti_lmu_bl_update_brightness_register(lmu_bank, brightness);
> > +}
> > [...]
Thanks for the review.
-- Sebastian
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists