| 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
| ||
|
Message-id: <561252C4.7@samsung.com>
Date: Mon, 05 Oct 2015 19:36:52 +0900
From: Ingi Kim <ingi2.kim@...sung.com>
To: Lee Jones <lee.jones@...aro.org>
Cc: robh+dt@...nel.org, pawel.moll@....com, mark.rutland@....com,
ijc+devicetree@...lion.org.uk, galak@...eaurora.org,
rpurdie@...ys.net, j.anaszewski@...sung.com, sameo@...ux.intel.com,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-leds@...r.kernel.org
Subject: Re: [PATCH 2/2] leds: rt5033: Add RT5033 Flash led device driver
Hello Lee Jones,
Thanks for the review.
I'll try to divide this patch and change name and comment.
Then push ver2 patch soon.
On 2015년 10월 05일 16:21, Lee Jones wrote:
> 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@...sung.com>
>> ---
>> 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__ */
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists