linux-kernel - Re: [PATCH 1/2] Exynos4 NURI: configure regulators and PMIC

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: <BANLkTiniwMCW-ED4CX=qybMvk243Zdki4Q@mail.gmail.com>
Date:	Mon, 20 Jun 2011 15:26:08 +0900
From:	MyungJoo Ham <myungjoo.ham@...sung.com>
To:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
Cc:	linux-arm-kernel@...ts.infradead.org,
	Kukjin Kim <kgene.kim@...sung.com>,
	Russell King <linux@....linux.org.uk>,
	linux-kernel@...r.kernel.org, dg77.kim@...sung.com,
	kyungmin.park@...sung.com
Subject: Re: [PATCH 1/2] Exynos4 NURI: configure regulators and PMIC

On Sun, Jun 19, 2011 at 12:12 AM, Mark Brown
<broonie@...nsource.wolfsonmicro.com> wrote:
> On Thu, Jun 16, 2011 at 06:09:31PM +0900, MyungJoo Ham wrote:
>
>>  static struct regulator_init_data emmc_fixed_voltage_init_data = {
>>       .constraints            = {
>> +             .min_uV         = 2800000,
>> +             .max_uV         = 2800000,
>>               .name           = "VMEM_VDD_2.8V",
>>               .valid_ops_mask = REGULATOR_CHANGE_STATUS,
>
> Since the regualtor can't change voltage specifying the voltage here
> isn't going to achieve anything - to get the voltage reported through
> get_voltage() you need to put the voltage in the platform data for the
> fixed regulator.

Ah.. they are useless. I'll remove them.

>
>> +static struct regulator_consumer_supply nuri_max8997_ldo1_consumer[] = {
>> +     REGULATOR_SUPPLY("vadc", NULL), /* Used by CPU's ADC drv */
>> +};
>
> In the ADC regulator patch you called the supply vdd (though the chip
> normally calls it vadc so that's the better name)...

Um.. this happened as I have seperated NURI-board platform patch for
ADC and PMIC.
After ADC patch, that name became ("vdd", "s5p-adc").
I'll let them either be merged or be disjoint completely.

Anyway, do you think "vadc" is better than "vdd" for ADC drivers? The
circuit schematics says the pin on the consumer side is "VDD33_ADC"
(VDD 3.3V for ADC).


>
> Extra ' too.
>
>> +static struct regulator_consumer_supply nuri_max8997_ldo8_consumer[] = {
>> +     REGULATOR_SUPPLY("vusb_d", NULL), /* Used by CPU */
>> +     REGULATOR_SUPPLY("vdac", NULL), /* Used by CPU */
>> +};
>
> Another VADC?  For a different supply?

That's DAC (opposite to ADC).

>
>> +             .state_mem      = {
>> +                     .enabled        = 0,
>
> No need to initialize to zero.

Ok, I'll remove every zero-initialization from the patch.

>
>> +static struct regulator_init_data nuri_max8997_ldo10_data = {
>> +     .constraints    = {
>
> You should be able to use __initdata for a lot of this by the way.

Ah.. way to reduce some weight. Fine.

>
>> +#define NURI_PMIC_GPIO               EXYNOS4_GPX0(7)
>> +static void __init nuri_pmic_init(void)
>> +{
>> +     int gpio;
>> +
>> +     nuri_max8997_pdata.irq_base = irq_get_next_irq(IRQ_GPIO_END);
>> +     gpio = NURI_PMIC_GPIO;
>> +     gpio_request(gpio, "AP_PMIC_IRQ");
>> +     s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));
>> +     s3c_gpio_setpull(gpio, S3C_GPIO_PULL_NONE);
>> +}
>
> I'm not sure both the #define and the variable make sense here...
>

I've defined them because two statements are using the gpio address of
NURI_PMIC.



Thanks you very much!

- MyungJoo
-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
--
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/