On Wed, Dec 1, 2010 at 3:50 PM, Sascha Hauer <s.ha...@pengutronix.de> wrote:
> Hi Yong,
>
> On Wed, Dec 01, 2010 at 03:15:55PM +0800, Yong Shen wrote:
>> Hi there,
>>
>> This is the v2 with some changes according to comments from v1. There
>> will be few patches coming out after this one, for mc13892 regulator
>> to share some code with mc13783.
>>
>> Still, cause the firewall problem, I send out patch this way. Please
>> give comments inline and use attached patch for testing.
>
> This patch definitely goes into the right direction. Some comments
> inline.
>
>>
>> Yong
>>
>> From b3451070f4665c90f11e600a8722f86b68437ded Mon Sep 17 00:00:00 2001
>> From: Yong Shen <yong.s...@linaro.org>
>> Date: Tue, 30 Nov 2010 14:11:34 +0800
>> Subject: [PATCH] make mc13783 regulator code generic
>>
>> move some common functions and micros of mc13783 regulaor driver to
>> a seperate file, which makes it possible for mc13892 to share code.
>>
>> Signed-off-by: Yong Shen <yong.s...@linaro.org>
>> ---
>> drivers/regulator/Kconfig | 4 +
>> drivers/regulator/Makefile | 1 +
>> drivers/regulator/mc13783-regulator.c | 325
>> ++++------------------------
>> drivers/regulator/mc13xxx-regulator-core.c | 239 ++++++++++++++++++++
>> include/linux/mfd/mc13783.h | 67 +++---
>> include/linux/regulator/mc13xxx.h | 101 +++++++++
>> 6 files changed, 425 insertions(+), 312 deletions(-)
>> create mode 100644 drivers/regulator/mc13xxx-regulator-core.c
>> create mode 100644 include/linux/regulator/mc13xxx.h
>>
>> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
>> index dd30e88..63c2bdd 100644
>> --- a/drivers/regulator/Kconfig
>> +++ b/drivers/regulator/Kconfig
>> @@ -186,9 +186,13 @@ config REGULATOR_PCAP
>> This driver provides support for the voltage regulators of the
>> PCAP2 PMIC.
>>
>> +config REGULATOR_MC13XXX_CORE
>> + tristate "Support regulators on Freescale MC13xxx PMIC"
>> +
>
> This does not need a prompt. Selecting only this option does not make
> sense and it will be selected by the mc13783/mc13892 code anyway.
>
>> config REGULATOR_MC13783
>> tristate "Support regulators on Freescale MC13783 PMIC"
>> depends on MFD_MC13783
>> + select REGULATOR_MC13XXX_CORE
>> help
>> Say y here to support the regulators found on the Freescale MC13783
>> PMIC.
>> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
>> index bff8157..11876be 100644
>> --- a/drivers/regulator/Makefile
>> +++ b/drivers/regulator/Makefile
>> @@ -30,6 +30,7 @@ obj-$(CONFIG_REGULATOR_DA903X) += da903x.o
>> obj-$(CONFIG_REGULATOR_PCF50633) += pcf50633-regulator.o
>> obj-$(CONFIG_REGULATOR_PCAP) += pcap-regulator.o
>> obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o
>> +obj-$(CONFIG_REGULATOR_MC13XXX_CORE) += mc13xxx-regulator-core.o
>> obj-$(CONFIG_REGULATOR_AB3100) += ab3100.o
>>
>> obj-$(CONFIG_REGULATOR_TPS65023) += tps65023-regulator.o
>
> [snip]
>
>> diff --git a/include/linux/mfd/mc13783.h b/include/linux/mfd/mc13783.h
>> index b4c741e..7d0f3d6 100644
>> --- a/include/linux/mfd/mc13783.h
>> +++ b/include/linux/mfd/mc13783.h
>> @@ -1,4 +1,5 @@
>> /*
>> + * Copyright 2010 Yong Shen <yong.s...@linaro.org>
>> * Copyright 2009-2010 Pengutronix
>> * Uwe Kleine-Koenig <u.kleine-koe...@pengutronix.de>
>> *
>> @@ -122,39 +123,39 @@ int mc13783_adc_do_conversion(struct mc13783
>> *mc13783, unsigned int mode,
>> unsigned int channel, unsigned int *sample);
>>
>>
>> -#define MC13783_SW_SW1A 0
>> -#define MC13783_SW_SW1B 1
>> -#define MC13783_SW_SW2A 2
>> -#define MC13783_SW_SW2B 3
>> -#define MC13783_SW_SW3 4
>> -#define MC13783_SW_PLL 5
>> -#define MC13783_REGU_VAUDIO 6
>> -#define MC13783_REGU_VIOHI 7
>> -#define MC13783_REGU_VIOLO 8
>> -#define MC13783_REGU_VDIG 9
>> -#define MC13783_REGU_VGEN 10
>> -#define MC13783_REGU_VRFDIG 11
>> -#define MC13783_REGU_VRFREF 12
>> -#define MC13783_REGU_VRFCP 13
>> -#define MC13783_REGU_VSIM 14
>> -#define MC13783_REGU_VESIM 15
>> -#define MC13783_REGU_VCAM 16
>> -#define MC13783_REGU_VRFBG 17
>> -#define MC13783_REGU_VVIB 18
>> -#define MC13783_REGU_VRF1 19
>> -#define MC13783_REGU_VRF2 20
>> -#define MC13783_REGU_VMMC1 21
>> -#define MC13783_REGU_VMMC2 22
>> -#define MC13783_REGU_GPO1 23
>> -#define MC13783_REGU_GPO2 24
>> -#define MC13783_REGU_GPO3 25
>> -#define MC13783_REGU_GPO4 26
>> -#define MC13783_REGU_V1 27
>> -#define MC13783_REGU_V2 28
>> -#define MC13783_REGU_V3 29
>> -#define MC13783_REGU_V4 30
>> -#define MC13783_REGU_PWGT1SPI 31
>> -#define MC13783_REGU_PWGT2SPI 32
>
> These have users. If you really want to change them you must change
> the users aswell. Also, I would suggest an additional patch for this.
> Remember, one topic per patch. Renaming things is a topic that can
> easily be split out.
My bad, I had not noticed that. In this case, I will split it into
two. The renaming patch goes first followed with code restructuring
patch.
>
>> +#define MC13783_REG_SW1A 0
>> +#define MC13783_REG_SW1B 1
>> +#define MC13783_REG_SW2A 2
>> +#define MC13783_REG_SW2B 3
>> +#define MC13783_REG_SW3 4
>> +#define MC13783_REG_PLL 5
>> +#define MC13783_REG_VAUDIO 6
>> +#define MC13783_REG_VIOHI 7
>> +#define MC13783_REG_VIOLO 8
>> +#define MC13783_REG_VDIG 9
>> +#define MC13783_REG_VGEN 10
>> +#define MC13783_REG_VRFDIG 11
>> +#define MC13783_REG_VRFREF 12
>> +#define MC13783_REG_VRFCP 13
>> +#define MC13783_REG_VSIM 14
>> +#define MC13783_REG_VESIM 15
>> +#define MC13783_REG_VCAM 16
>> +#define MC13783_REG_VRFBG 17
>> +#define MC13783_REG_VVIB 18
>> +#define MC13783_REG_VRF1 19
>> +#define MC13783_REG_VRF2 20
>> +#define MC13783_REG_VMMC1 21
>> +#define MC13783_REG_VMMC2 22
>> +#define MC13783_REG_GPO1 23
>> +#define MC13783_REG_GPO2 24
>> +#define MC13783_REG_GPO3 25
>> +#define MC13783_REG_GPO4 26
>> +#define MC13783_REG_V1 27
>> +#define MC13783_REG_V2 28
>> +#define MC13783_REG_V3 29
>> +#define MC13783_REG_V4 30
>> +#define MC13783_REG_PWGT1SPI 31
>> +#define MC13783_REG_PWGT2SPI 32
>>
>> #define MC13783_IRQ_ADCDONE MC13XXX_IRQ_ADCDONE
>> #define MC13783_IRQ_ADCBISDONE MC13XXX_IRQ_ADCBISDONE
>> diff --git a/include/linux/regulator/mc13xxx.h
>> b/include/linux/regulator/mc13xxx.h
>> new file mode 100644
>> index 0000000..a60c9be
>> --- /dev/null
>> +++ b/include/linux/regulator/mc13xxx.h
>
> The things in this file are private to the driver. IMO this should go to
> drivers/regulator/mc13xxx.h.
>
>> @@ -0,0 +1,101 @@
>> +/*
>> + * mc13xxx.h - regulators for the Freescale mc13xxx PMIC
>> + *
>> + * Copyright (C) 2010 Yong Shen <yong.s...@linaro.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#ifndef __LINUX_REGULATOR_MC13XXX_H
>> +#define __LINUX_REGULATOR_MC13XXX_H
>> +
>> +#include <linux/regulator/driver.h>
>> +
>> +struct mc13xxx_regulator {
>> + struct regulator_desc desc;
>> + int reg;
>> + int enable_bit;
>> + int vsel_reg;
>> + int vsel_shift;
>> + int vsel_mask;
>> + int hi_bit;
>> + int const *voltages;
>> +};
>> +
>> +struct mc13xxx_regulator_priv {
>> + struct mc13xxx *mc13xxx;
>> + u32 powermisc_pwgt_state;
>> + struct mc13xxx_regulator *mc13xxx_regulators;
>> + struct regulator_dev *regulators[];
>> +};
>> +
>> +extern int mc13xxx_sw_regulator(struct regulator_dev *rdev);
>> +extern int mc13xxx_sw_regulator_is_enabled(struct regulator_dev *rdev);
>> +extern int mc13xxx_get_best_voltage_index(struct regulator_dev *rdev,
>> + int min_uV, int max_uV);
>> +extern int mc13xxx_regulator_list_voltage(struct regulator_dev *rdev,
>> + unsigned selector);
>> +extern int mc13xxx_fixed_regulator_set_voltage(struct regulator_dev *rdev,
>> + int min_uV, int max_uV);
>> +extern int mc13xxx_fixed_regulator_get_voltage(struct regulator_dev *rdev);
>> +
>> +extern struct regulator_ops mc13xxx_regulator_ops;
>> +extern struct regulator_ops mc13xxx_fixed_regulator_ops;
>> +
>> +#define MC13xxx_DEFINE(prefix, _name, _reg, _vsel_reg, _voltages, _ops)
>> \
>> + [prefix ## _name] = { \
>> + .desc = { \
>> + .name = #prefix "_" #_name, \
>> + .n_voltages = ARRAY_SIZE(_voltages), \
>> + .ops = &_ops, \
>> + .type = REGULATOR_VOLTAGE, \
>> + .id = prefix ## _name, \
>> + .owner = THIS_MODULE, \
>> + }, \
>> + .reg = prefix ## _reg, \
>> + .enable_bit = prefix ## _reg ## _ ## _name ## EN, \
>> + .vsel_reg = prefix ## _vsel_reg, \
>> + .vsel_shift = prefix ## _vsel_reg ## _ ## _name ## VSEL,\
>> + .vsel_mask = prefix ## _vsel_reg ## _ ## _name ## VSEL_M,\
>> + .voltages = _voltages, \
>> + }
>> +
>> +#define MC13xxx_FIXED_DEFINE(prefix, _name, _reg, _voltages, _ops) \
>> + [prefix ## _name] = { \
>> + .desc = { \
>> + .name = #prefix "_" #_name, \
>> + .n_voltages = ARRAY_SIZE(_voltages), \
>> + .ops = &_ops, \
>> + .type = REGULATOR_VOLTAGE, \
>> + .id = prefix ## _name, \
>> + .owner = THIS_MODULE, \
>> + }, \
>> + .reg = prefix ## _reg, \
>> + .enable_bit = prefix ## _reg ## _ ## _name ## EN, \
>> + .voltages = _voltages, \
>> + }
>> +
>> +#define MC13xxx_GPO_DEFINE(prefix, _name, _reg, _voltages, _ops) \
>> + [prefix ## _name] = { \
>> + .desc = { \
>> + .name = #prefix "_" #_name, \
>> + .n_voltages = ARRAY_SIZE(_voltages), \
>> + .ops = &_ops, \
>> + .type = REGULATOR_VOLTAGE, \
>> + .id = prefix ## _name, \
>> + .owner = THIS_MODULE, \
>> + }, \
>> + .reg = prefix ## _reg, \
>> + .enable_bit = prefix ## _reg ## _ ## _name ## EN, \
>> + .voltages = _voltages, \
>> + }
>> +
>> +#define MC13xxx_DEFINE_SW(_name, _reg, _vsel_reg, _voltages, ops) \
>> + MC13xxx_DEFINE(SW, _name, _reg, _vsel_reg, _voltages, ops)
>> +#define MC13xxx_DEFINE_REGU(_name, _reg, _vsel_reg, _voltages, ops) \
>> + MC13xxx_DEFINE(REGU, _name, _reg, _vsel_reg, _voltages, ops)
>> +
>> +#endif
>> --
>> 1.7.0.4
>>
>> Cheers
>> Yong
>
>
>
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
>
_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
