linux-kernel - Re: [PATCH v3] regulator: add new regulator driver for lp872x

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAF+7xWmNOFaYhLRxDa5z62W1pWvUt460cNSTJpaxZ6C+-9azwg@mail.gmail.com>
Date:	Tue, 19 Jun 2012 15:58:02 +0800
From:	Axel Lin <axel.lin@...il.com>
To:	"Kim, Milo" <Milo.Kim@...com>
Cc:	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Girdwood, Liam" <lrg@...com>
Subject: Re: [PATCH v3] regulator: add new regulator driver for lp872x

2012/6/19 Kim, Milo <Milo.Kim@...com>:
> This driver supports TI/National LP8720, LP8725 PMIC.
>
You might want to add more commit description here.
e.g. ( I just copy-paste from your first version.)

This driver supports TI/National Semiconductor LP8720 and LP8725 PMU.
LP8720 : 5 LDOs and 1 BUCK
LP8725 : 7 LDOs and 2 BUCKS
These ICs have similar register map for controlling regulators.

Datasheet
---------
LP8720 : http://www.ti.com/litv/pdf/snvs575a
LP8725 : http://www.ti.com/lit/gpn/lp8725
I2C compatible interface
------------------------
The regmap APIs are used for accessing the registers
Supported regulator operations
------------------------------
* list_voltage/set_voltage_sel/get_voltage_sel
  : voltage tables are used for selecting specific voltage
* enable/disable/is_enabled/enable_time
* set_mode/get_mode
  : BUCK specific operations. Forced pwm and normal mode are selective
* set_current_limit/get_current_limit
  : current limit operations for lp8725 BUCKs
Platform data
-------------
3 mandatory and 1 optional data are defined.
* general_config : value of GENERAL_CFG register is platform specific data
* regulator_data : regulator init data with id in platform side
* num_regulators : numbers of regulator_data
* get_dvs_pin_state : used for selecting buck output register
    DVS input is platform specific pin for choosing buck register address

Below description should be put under --- line, so it won't display in
commit log.

> patch v3
>
> (a) replace few if-statements with switch-statements
> (b) use devm_gpio_request_one() for configuring dvs gpio
> : replace gpio_request() and gpio_direction_output() with devm_gpio_request_one()
>  remove gpio_free() code
>
> (c) enhanced code on finding matched regulator init data
> : change max loop count. fixed loop count (9) -> variable count (6 or 9)
>
> Signed-off-by: Milo(Woogyom) Kim <milo.kim@...com>

The code looks good to me,
Reviewed-by: Axel Lin <axel.lin@...il.com>

Regards,
Axel
--
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/