From mboxrd@z Thu Jan  1 00:00:00 1970
From: Peng Fan <B51431@freescale.com>
Date: Sat, 24 Jan 2015 21:52:25 +0800
Subject: [U-Boot] [PATCH v4 2/3] pmic:pfuze implement pmic_mode_init
In-Reply-To: <54C0D136.5010004@denx.de>
References: <1421424314-21850-1-git-send-email-Peng.Fan@freescale.com>
	<1421424314-21850-3-git-send-email-Peng.Fan@freescale.com>
	<54C0D136.5010004@denx.de>
Message-ID: <54C3A399.1070006@freescale.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi, Stefano

On 1/22/2015 6:30 PM, Stefano Babic wrote:
> On 16/01/2015 17:05, Peng Fan wrote:
>> This patch is to implement pmic_mode_init function, and add prototype
>> in header file.
>>
>> This function is to set switching mode for pmic buck regulators to
>> improve system efficiency.
>>
>> Mode:
>> OFF: The regulator is switched off and the output voltage is discharged.
>> PFM: In this mode, the regulator is always in PFM mode, which
>>       is useful at light loads for optimized efficiency.
>> PWM: In this mode, the regulator is always in PWM mode operation
>>       regardless of load conditions.
>> APS: In this mode, the regulator moves automatically between
>>       pulse skipping mode and PWM mode depending on load conditions.
>>
>> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
>> ---
>>
>> Changes v4:
>>   Take Fabio's comments, fix return value using -EINVAL when error.
>>
>> Changes v3:
>>   delete the new feature, it is fine to use the original v1 patch version
>>
>> Changes v2:
>>   implement new feature to configure one regulator
>>
>>   board/freescale/common/pfuze.c | 36 ++++++++++++++++++++++++++++++++++++
>>   board/freescale/common/pfuze.h |  1 +
>>   2 files changed, 37 insertions(+)
>>
>> diff --git a/board/freescale/common/pfuze.c b/board/freescale/common/pfuze.c
>> index 2cd1794..c2a0105 100644
>> --- a/board/freescale/common/pfuze.c
>> +++ b/board/freescale/common/pfuze.c
>> @@ -8,6 +8,42 @@
>>   #include <power/pmic.h>
>>   #include <power/pfuze100_pmic.h>
>>   
>> +int pfuze_mode_init(struct pmic *p, u32 mode)
>> +{
>> +	unsigned char offset, i, switch_num;
>> +	u32 id, ret;
>> +
>> +	pmic_reg_read(p, PFUZE100_DEVICEID, &id);
>> +	id = id & 0xf;
>> +
>> +	if (id == 0) {
>> +		switch_num = 6;
>> +		offset = PFUZE100_SW1CMODE;
>> +	} else if (id == 1) {
>> +		switch_num = 4;
>> +		offset = PFUZE100_SW2MODE;
>> +	} else {
>> +		printf("Not supported, id=%d\n", id);
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = pmic_reg_write(p, PFUZE100_SW1ABMODE, mode);
>> +	if (ret < 0) {
>> +		printf("Set SW1AB mode error!\n");
>> +		return ret;
>> +	}
>> +
>> +	for (i = 0; i < switch_num - 1; i++) {
>> +		ret = pmic_reg_write(p, offset + i * 7, mode);
> Even if it looks clear why, I would suggest a define into
> pfuze100_pmic.h such as "SIZEOF_SWITCH (or what you prefer)" and drop
> the constant here.
The switch_num is dynamically set according to deviceid. I think 
hardcode it using a macro is not a good idea.
>
>> +		if (ret < 0) {
>> +			printf("Set switch%x mode error!\n", offset + i * 7);
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>   struct pmic *pfuze_common_init(unsigned char i2cbus)
>>   {
>>   	struct pmic *p;
>> diff --git a/board/freescale/common/pfuze.h b/board/freescale/common/pfuze.h
>> index 7a4126c..53cfc99 100644
>> --- a/board/freescale/common/pfuze.h
>> +++ b/board/freescale/common/pfuze.h
>> @@ -8,5 +8,6 @@
>>   #define __PFUZE_BOARD_HELPER__
>>   
>>   struct pmic *pfuze_common_init(unsigned char i2cbus);
>> +int pfuze_mode_init(struct pmic *p, u32 mode);
>>   
>>   #endif
>>
> Best regards,
> Stefano Babic
>
Regards,
Peng.