diff mbox

[v2,2/6] mfd: axp20x: Add register bits for axp20x-ac-power

Message ID 1462093047-7885-3-git-send-email-haas@computerlinguist.org (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Michael Haas May 1, 2016, 8:57 a.m. UTC 
This change adds some register bit definitions used by the
axp20x-ac-power driver.

Signed-off-by: Michael Haas <haas@computerlinguist.org>
---
 include/linux/mfd/axp20x.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Chen-Yu Tsai May 1, 2016, 9:48 a.m. UTC | #1 
Hi,

On Sun, May 1, 2016 at 4:57 PM, Michael Haas <haas@computerlinguist.org> wrote:
> This change adds some register bit definitions used by the
> axp20x-ac-power driver.
>
> Signed-off-by: Michael Haas <haas@computerlinguist.org>
> ---
>  include/linux/mfd/axp20x.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index d82e7d5..c4c6dfa 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -90,6 +90,17 @@ enum {
>  #define AXP22X_ALDO3_V_OUT             0x2a
>  #define AXP22X_CHRG_CTRL3              0x35
>
> +
> +/* Fields of AXP20X_PWR_INPUT_STATUS */
> +#define AXP20X_PWR_STATUS_AC_PRESENT     BIT(7)
> +#define AXP20X_PWR_STATUS_AC_AVAILABLE   BIT(6)
> +#define AXP20X_PWR_STATUS_AC_VBUS_SHORT  BIT(1)
> +#define AXP20X_PWR_STATUS_AC_VBUS_SEL    BIT(0)
> +
> +/* Fields of AXP20X_ADC_EN1 */
> +#define AXP20X_ADC_EN1_ACIN_VOLT BIT(5)
> +#define AXP20X_ADC_EN1_ACIN_CURR BIT(4)
> +

We keep the bit definitions of each register in each separate driver.
The drivers only define the ones they use.

ChenYu

>  /* Interrupt */
>  #define AXP152_IRQ1_EN                 0x40
>  #define AXP152_IRQ2_EN                 0x41
> --
> 2.8.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Haas May 1, 2016, 2:24 p.m. UTC | #2 
On 05/01/2016 11:48 AM, Chen-Yu Tsai wrote:
> Hi,
> 
> On Sun, May 1, 2016 at 4:57 PM, Michael Haas <haas@computerlinguist.org> wrote:
>> This change adds some register bit definitions used by the
>> axp20x-ac-power driver.
>>
>> Signed-off-by: Michael Haas <haas@computerlinguist.org>
>> ---
>>  include/linux/mfd/axp20x.h | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
>> index d82e7d5..c4c6dfa 100644
>> --- a/include/linux/mfd/axp20x.h
>> +++ b/include/linux/mfd/axp20x.h
>> @@ -90,6 +90,17 @@ enum {
>>  #define AXP22X_ALDO3_V_OUT             0x2a
>>  #define AXP22X_CHRG_CTRL3              0x35
>>
>> +
>> +/* Fields of AXP20X_PWR_INPUT_STATUS */
>> +#define AXP20X_PWR_STATUS_AC_PRESENT     BIT(7)
>> +#define AXP20X_PWR_STATUS_AC_AVAILABLE   BIT(6)
>> +#define AXP20X_PWR_STATUS_AC_VBUS_SHORT  BIT(1)
>> +#define AXP20X_PWR_STATUS_AC_VBUS_SEL    BIT(0)
>> +
>> +/* Fields of AXP20X_ADC_EN1 */
>> +#define AXP20X_ADC_EN1_ACIN_VOLT BIT(5)
>> +#define AXP20X_ADC_EN1_ACIN_CURR BIT(4)
>> +
> 
> We keep the bit definitions of each register in each separate driver.
> The drivers only define the ones they use.
> 
> ChenYu

Hi ChenYu,

i believe Maxime Ripard requested that these defines be moved to the
header: https://groups.google.com/d/msg/linux-sunxi/nEUg87cV6KI/TvdB6MBZBAAJ

What do you think?

Thanks for the review!

Michael

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chen-Yu Tsai May 2, 2016, 1:35 a.m. UTC | #3 
On Sun, May 1, 2016 at 10:24 PM, Michael Haas <haas@computerlinguist.org> wrote:
> On 05/01/2016 11:48 AM, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Sun, May 1, 2016 at 4:57 PM, Michael Haas <haas@computerlinguist.org> wrote:
>>> This change adds some register bit definitions used by the
>>> axp20x-ac-power driver.
>>>
>>> Signed-off-by: Michael Haas <haas@computerlinguist.org>
>>> ---
>>>  include/linux/mfd/axp20x.h | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
>>> index d82e7d5..c4c6dfa 100644
>>> --- a/include/linux/mfd/axp20x.h
>>> +++ b/include/linux/mfd/axp20x.h
>>> @@ -90,6 +90,17 @@ enum {
>>>  #define AXP22X_ALDO3_V_OUT             0x2a
>>>  #define AXP22X_CHRG_CTRL3              0x35
>>>
>>> +
>>> +/* Fields of AXP20X_PWR_INPUT_STATUS */
>>> +#define AXP20X_PWR_STATUS_AC_PRESENT     BIT(7)
>>> +#define AXP20X_PWR_STATUS_AC_AVAILABLE   BIT(6)
>>> +#define AXP20X_PWR_STATUS_AC_VBUS_SHORT  BIT(1)
>>> +#define AXP20X_PWR_STATUS_AC_VBUS_SEL    BIT(0)
>>> +
>>> +/* Fields of AXP20X_ADC_EN1 */
>>> +#define AXP20X_ADC_EN1_ACIN_VOLT BIT(5)
>>> +#define AXP20X_ADC_EN1_ACIN_CURR BIT(4)
>>> +
>>
>> We keep the bit definitions of each register in each separate driver.
>> The drivers only define the ones they use.
>>
>> ChenYu
>
> Hi ChenYu,
>
> i believe Maxime Ripard requested that these defines be moved to the
> header: https://groups.google.com/d/msg/linux-sunxi/nEUg87cV6KI/TvdB6MBZBAAJ
>
> What do you think?

My argument is kind of weak, and really comes down to preference.

Currently the register bit definitions are scattered in various drivers,
which is fine given they are really specific to the part of hardware the
driver supports. Gathering them all together might increase the size of
the header file substantially. As I see it the chanses that bits from
one part are going to be used in another are rather small.

Some register address macros are shared, such as for the 2 power supply
drivers, and for the regmap definitions. So those would need to go in a
shared header anyway.


Regards
ChenYu
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maxime Ripard May 4, 2016, 6:08 p.m. UTC | #4 
Hi,

On Mon, May 02, 2016 at 09:35:01AM +0800, Chen-Yu Tsai wrote:
> On Sun, May 1, 2016 at 10:24 PM, Michael Haas <haas@computerlinguist.org> wrote:
> > On 05/01/2016 11:48 AM, Chen-Yu Tsai wrote:
> >> Hi,
> >>
> >> On Sun, May 1, 2016 at 4:57 PM, Michael Haas <haas@computerlinguist.org> wrote:
> >>> This change adds some register bit definitions used by the
> >>> axp20x-ac-power driver.
> >>>
> >>> Signed-off-by: Michael Haas <haas@computerlinguist.org>
> >>> ---
> >>>  include/linux/mfd/axp20x.h | 11 +++++++++++
> >>>  1 file changed, 11 insertions(+)
> >>>
> >>> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> >>> index d82e7d5..c4c6dfa 100644
> >>> --- a/include/linux/mfd/axp20x.h
> >>> +++ b/include/linux/mfd/axp20x.h
> >>> @@ -90,6 +90,17 @@ enum {
> >>>  #define AXP22X_ALDO3_V_OUT             0x2a
> >>>  #define AXP22X_CHRG_CTRL3              0x35
> >>>
> >>> +
> >>> +/* Fields of AXP20X_PWR_INPUT_STATUS */
> >>> +#define AXP20X_PWR_STATUS_AC_PRESENT     BIT(7)
> >>> +#define AXP20X_PWR_STATUS_AC_AVAILABLE   BIT(6)
> >>> +#define AXP20X_PWR_STATUS_AC_VBUS_SHORT  BIT(1)
> >>> +#define AXP20X_PWR_STATUS_AC_VBUS_SEL    BIT(0)
> >>> +
> >>> +/* Fields of AXP20X_ADC_EN1 */
> >>> +#define AXP20X_ADC_EN1_ACIN_VOLT BIT(5)
> >>> +#define AXP20X_ADC_EN1_ACIN_CURR BIT(4)
> >>> +
> >>
> >> We keep the bit definitions of each register in each separate driver.
> >> The drivers only define the ones they use.
> >>
> >> ChenYu
> >
> > Hi ChenYu,
> >
> > i believe Maxime Ripard requested that these defines be moved to the
> > header: https://groups.google.com/d/msg/linux-sunxi/nEUg87cV6KI/TvdB6MBZBAAJ
> >
> > What do you think?
> 
> My argument is kind of weak, and really comes down to preference.
> 
> Currently the register bit definitions are scattered in various drivers,
> which is fine given they are really specific to the part of hardware the
> driver supports. Gathering them all together might increase the size of
> the header file substantially. As I see it the chanses that bits from
> one part are going to be used in another are rather small.
> 
> Some register address macros are shared, such as for the 2 power supply
> drivers, and for the regmap definitions. So those would need to go in a
> shared header anyway.

Sorry for the misunderstanding. I was assuming that having all the
registers and associated bits would be better off in a common header
where all the drivers could refer to, but you're the maintainer on
that on, so it's up to you ;)

Maxime
diff mbox

Patch

diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index d82e7d5..c4c6dfa 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -90,6 +90,17 @@  enum {
 #define AXP22X_ALDO3_V_OUT		0x2a
 #define AXP22X_CHRG_CTRL3		0x35
 
+
+/* Fields of AXP20X_PWR_INPUT_STATUS */
+#define AXP20X_PWR_STATUS_AC_PRESENT     BIT(7)
+#define AXP20X_PWR_STATUS_AC_AVAILABLE   BIT(6)
+#define AXP20X_PWR_STATUS_AC_VBUS_SHORT  BIT(1)
+#define AXP20X_PWR_STATUS_AC_VBUS_SEL    BIT(0)
+
+/* Fields of AXP20X_ADC_EN1 */
+#define AXP20X_ADC_EN1_ACIN_VOLT BIT(5)
+#define AXP20X_ADC_EN1_ACIN_CURR BIT(4)
+
 /* Interrupt */
 #define AXP152_IRQ1_EN			0x40
 #define AXP152_IRQ2_EN			0x41