Re: [PATCH 2/2] leds: rt5033: Add RT5033 Flash led device driver

[Lee Jones]

On Fri, 02 Oct 2015, Ingi Kim wrote:

> This patch adds device driver of Richtek RT5033 PMIC.
> The driver supports a current regulated output to drive
> white LEDs for camera flash.
> 
> Signed-off-by: Ingi Kim <ingi2.kim@xxxxxxxxxxx>
> ---
>  drivers/leds/Kconfig               |   8 ++
>  drivers/leds/Makefile              |   1 +
>  drivers/leds/leds-rt5033.c         | 222 +++++++++++++++++++++++++++++++++++++
>  drivers/mfd/rt5033.c               |   3 +
>  include/linux/mfd/rt5033-private.h |  67 +++++++++--
>  include/linux/mfd/rt5033.h         |  27 ++++-

Please pull the MFD changes out into to separate patch(es).

>  6 files changed, 317 insertions(+), 11 deletions(-)
>  create mode 100644 drivers/leds/leds-rt5033.c

[...]

> diff --git a/drivers/mfd/rt5033.c b/drivers/mfd/rt5033.c
> index d60f916..035421c 100644
> --- a/drivers/mfd/rt5033.c
> +++ b/drivers/mfd/rt5033.c
> @@ -47,6 +47,9 @@ static const struct mfd_cell rt5033_devs[] = {
>  	}, {
>  		.name = "rt5033-battery",
>  		.of_compatible = "richtek,rt5033-battery",
> +	}, {
> +		.name = "rt5033-led",
> +		.of_compatible = "richtek,rt5033-led",
>  	},
>  };

Needs to be in a patch of its own.

> diff --git a/include/linux/mfd/rt5033-private.h b/include/linux/mfd/rt5033-private.h
> index 1b63fc2..21c3aff 100644
> --- a/include/linux/mfd/rt5033-private.h
> +++ b/include/linux/mfd/rt5033-private.h
> @@ -25,15 +25,15 @@ enum rt5033_reg {
>  	/* Reserved 0x09~0x18 */
>  	RT5033_REG_RT_CTRL1		= 0x19,
>  	/* Reserved 0x1A~0x20 */
> -	RT5033_REG_FLED_FUNCTION1	= 0x21,
> -	RT5033_REG_FLED_FUNCTION2	= 0x22,
> -	RT5033_REG_FLED_STROBE_CTRL1	= 0x23,
> -	RT5033_REG_FLED_STROBE_CTRL2	= 0x24,
> -	RT5033_REG_FLED_CTRL1		= 0x25,
> -	RT5033_REG_FLED_CTRL2		= 0x26,
> -	RT5033_REG_FLED_CTRL3		= 0x27,
> -	RT5033_REG_FLED_CTRL4		= 0x28,
> -	RT5033_REG_FLED_CTRL5		= 0x29,
> +	RT5033_REG_FL_FUNCTION1		= 0x21,
> +	RT5033_REG_FL_FUNCTION2		= 0x22,
> +	RT5033_REG_FL_STROBE_CTRL1	= 0x23,
> +	RT5033_REG_FL_STROBE_CTRL2	= 0x24,
> +	RT5033_REG_FL_CTRL1		= 0x25,
> +	RT5033_REG_FL_CTRL2		= 0x26,
> +	RT5033_REG_FL_CTRL3		= 0x27,
> +	RT5033_REG_FL_CTRL4		= 0x28,
> +	RT5033_REG_FL_CTRL5		= 0x29,

Why this change?

The previous naming convention was better.

>  	/* Reserved 0x2A~0x40 */
>  	RT5033_REG_CTRL			= 0x41,
>  	RT5033_REG_BUCK_CTRL		= 0x42,
> @@ -93,6 +93,55 @@ enum rt5033_reg {
>  #define RT5033_RT_CTRL1_UUG_MASK	0x02
>  #define RT5033_RT_HZ_MASK		0x01
>  
> +/* RT5033 FLED Function1 register */
> +#define RT5033_FL_FLED1_MASK		0x94
> +#define RT5033_FL_STROBE_SEL		0x04
> +#define RT5033_FL_PIN_CTRL		0x10
> +#define RT5033_FL_RESET			0x80
> +
> +/* RT5033 FLED Function2 register */
> +#define RT5033_FL_FLED2_MASK		0x81
> +#define RT5033_FL_ENFLED		0x01
> +#define RT5033_FL_SREG_STROBE		0x80
> +
> +/* RT5033 FLED Strobe Control1 */
> +#define RT5033_FL_STRBCTRL1_MASK	0xFF
> +#define RT5033_FL_STRBCTRL1_TM_CUR_MAX	0xE0
> +#define RT5033_FL_STRBCTRL1_FL_CUR_DEF	0x0D
> +#define RT5033_FL_STRBCTRL1_FL_CUR_MAX	0x1F
> +
> +/* RT5033 FLED Strobe Control2 */
> +#define RT5033_FL_STRBCTRL2_MASK	0x3F
> +#define RT5033_FL_STRBCTRL2_TM_DEF	0x0F
> +#define RT5033_FL_STRBCTRL2_TM_MAX	0x3F
> +
> +/* RT5033 FLED Control1 */
> +#define RT5033_FL_CTRL1_MASK		0xF7
> +#define RT5033_FL_CTRL1_TORCH_CUR_DEF	0x20
> +#define RT5033_FL_CTRL1_TORCH_CUR_MAX	0xF0
> +#define RT5033_FL_CTRL1_LBP_DEF		0x02
> +#define RT5033_FL_CTRL1_LBP_MAX		0x07
> +
> +/* RT5033 FLED Control2 */
> +#define RT5033_FL_CTRL2_MASK		0xFF
> +#define RT5033_FL_CTRL2_VMID_DEF	0x37
> +#define RT5033_FL_CTRL2_VMID_MAX	0x3F
> +#define RT5033_FL_CTRL2_TRACK_ALIVE	0x40
> +#define RT5033_FL_CTRL2_MID_TRACK	0x80
> +
> +/* RT5033 FLED Control4 */
> +#define RT5033_FL_CTRL4_MASK		0xE0
> +#define RT5033_FL_CTRL4_RESV		0x14
> +#define RT5033_FL_CTRL4_VTRREG_DEF	0x40
> +#define RT5033_FL_CTRL4_VTRREG_MAX	0x60
> +#define RT5033_FL_CTRL4_TRACK_CLK	0x80
> +
> +/* RT5033 FLED Control5 */
> +#define RT5033_FL_CTRL5_MASK		0xC0
> +#define RT5033_FL_CTRL5_RESV		0x10
> +#define RT5033_FL_CTRL5_TA_GOOD		0x40
> +#define RT5033_FL_CTRL5_TA_EXIST	0x80
> +
>  /* RT5033 control register */
>  #define RT5033_CTRL_FCCM_BUCK_MASK		0x00
>  #define RT5033_CTRL_BUCKOMS_MASK		0x01
> diff --git a/include/linux/mfd/rt5033.h b/include/linux/mfd/rt5033.h
> index 6cff5cf..45c97b7 100644
> --- a/include/linux/mfd/rt5033.h
> +++ b/include/linux/mfd/rt5033.h
> @@ -12,10 +12,11 @@
>  #ifndef __RT5033_H__
>  #define __RT5033_H__
>  
> -#include <linux/regulator/consumer.h>
> +#include <linux/led-class-flash.h>
>  #include <linux/i2c.h>
> -#include <linux/regmap.h>
>  #include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>  
>  /* RT5033 regulator IDs */
>  enum rt5033_regulators {
> @@ -59,4 +60,26 @@ struct rt5033_charger {
>  	struct rt5033_charger_data	*chg;
>  };
>  
> +/* RT5033 led platform data */
> +
> +struct rt5033_led_config_data {
> +	/* maximum LED current in movie mode */
> +	u32 torch_max_microamp;
> +	/* maximum LED current in flash mode */
> +	u32 flash_max_microamp;
> +	/* maximum flash timeout */
> +	u32 flash_max_timeout;
> +	/* max LED brightness level */
> +	enum led_brightness max_brightness;
> +};

Rid these comments.  Use kerneldoc format instead.

> +struct rt5033_led {
> +	struct device		*dev;
> +	struct rt5033_dev	*rt5033;
> +	struct regmap		*regmap;
> +
> +	/* Related LED class device */
> +	struct led_classdev	cdev;
> +};
> +
>  #endif /* __RT5033_H__ */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/