[RESEND PATCH 1/1] ARM: exynos_defconfig: Enable SBS battery support

[RESEND PATCH 1/1] ARM: exynos_defconfig: Enable SBS battery support

[Javier Martinez Canillas]

Many Exynos5 boards (e.g: Snow, Peach Pit and Pi) have
a SBS-compliant gas gauge battery. Enable it as module
so the needed support is available for these boards.

Suggested-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Acked-by: Kukjin Kim <kgene.kim@samsung.com>
---
 arch/arm/configs/exynos_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
index fc7d168..c390bb9 100644
--- a/arch/arm/configs/exynos_defconfig
+++ b/arch/arm/configs/exynos_defconfig
@@ -77,6 +77,7 @@ CONFIG_SPI_S3C64XX=y
 CONFIG_I2C_S3C2410=y
 CONFIG_DEBUG_GPIO=y
 CONFIG_POWER_SUPPLY=y
+CONFIG_BATTERY_SBS=m
 CONFIG_CHARGER_TPS65090=y
 # CONFIG_HWMON is not set
 CONFIG_THERMAL=y
-- 
2.0.0.rc2

[RESEND PATCH 1/1] ARM: exynos_defconfig: Enable SBS battery support

[Bartlomiej Zolnierkiewicz]

Hi,

On Monday, August 11, 2014 01:06:38 PM Javier Martinez Canillas wrote:
> Many Exynos5 boards (e.g: Snow, Peach Pit and Pi) have
> a SBS-compliant gas gauge battery. Enable it as module
> so the needed support is available for these boards.
> 
> Suggested-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Acked-by: Kukjin Kim <kgene.kim@samsung.com>
> ---
>  arch/arm/configs/exynos_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
> index fc7d168..c390bb9 100644
> --- a/arch/arm/configs/exynos_defconfig
> +++ b/arch/arm/configs/exynos_defconfig
> @@ -77,6 +77,7 @@ CONFIG_SPI_S3C64XX=y
>  CONFIG_I2C_S3C2410=y
>  CONFIG_DEBUG_GPIO=y
>  CONFIG_POWER_SUPPLY=y
> +CONFIG_BATTERY_SBS=m

Why not make it "=y"?

Rationale:
- currently no hardware related option uses "=m" in exynos_defconfig
- it would match the SBS option usage in multi_v7_defconfig

>  CONFIG_CHARGER_TPS65090=y
>  # CONFIG_HWMON is not set
>  CONFIG_THERMAL=y

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[RESEND PATCH 1/1] ARM: exynos_defconfig: Enable SBS battery support

[Javier Martinez Canillas]

Hello Bartlomiej,

On 08/11/2014 02:40 PM, Bartlomiej Zolnierkiewicz wrote:
>> index fc7d168..c390bb9 100644
>> --- a/arch/arm/configs/exynos_defconfig
>> +++ b/arch/arm/configs/exynos_defconfig
>> @@ -77,6 +77,7 @@ CONFIG_SPI_S3C64XX=y
>>  CONFIG_I2C_S3C2410=y
>>  CONFIG_DEBUG_GPIO=y
>>  CONFIG_POWER_SUPPLY=y
>> +CONFIG_BATTERY_SBS=m
> 
> Why not make it "=y"?
> 
> Rationale:
> - currently no hardware related option uses "=m" in exynos_defconfig
> - it would match the SBS option usage in multi_v7_defconfig
>
>>  CONFIG_CHARGER_TPS65090=y
>>  # CONFIG_HWMON is not set
>>  CONFIG_THERMAL=y
> 

I know but personally I think this should be changed. The idea of having a multi
platform kernel is to build a single kernel image that can be used to boot
different platforms. Not all platforms have a SBS-compliant battery so this
support shouldn't be built in the kernel image IMHO.

This also matches to what real users will do since distributions most likely
will have a minimal kernel and every possible hardware support will be enabled
as a loadable kernel module. This is what distros do for other platforms too.

If someone has a different use case and wants a kernel image that is optimized
for a particular platform then she has to create its own defconfig anyways.

> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 

Best regards,
Javier

[RESEND PATCH 1/1] ARM: exynos_defconfig: Enable SBS battery support

[Bartlomiej Zolnierkiewicz]

Hi,

On Monday, August 11, 2014 02:52:27 PM Javier Martinez Canillas wrote:
> Hello Bartlomiej,
> 
> On 08/11/2014 02:40 PM, Bartlomiej Zolnierkiewicz wrote:
> >> index fc7d168..c390bb9 100644
> >> --- a/arch/arm/configs/exynos_defconfig
> >> +++ b/arch/arm/configs/exynos_defconfig
> >> @@ -77,6 +77,7 @@ CONFIG_SPI_S3C64XX=y
> >>  CONFIG_I2C_S3C2410=y
> >>  CONFIG_DEBUG_GPIO=y
> >>  CONFIG_POWER_SUPPLY=y
> >> +CONFIG_BATTERY_SBS=m
> > 
> > Why not make it "=y"?
> > 
> > Rationale:
> > - currently no hardware related option uses "=m" in exynos_defconfig
> > - it would match the SBS option usage in multi_v7_defconfig
> >
> >>  CONFIG_CHARGER_TPS65090=y
> >>  # CONFIG_HWMON is not set
> >>  CONFIG_THERMAL=y
> > 
> 
> I know but personally I think this should be changed. The idea of having a multi
> platform kernel is to build a single kernel image that can be used to boot
> different platforms. Not all platforms have a SBS-compliant battery so this
> support shouldn't be built in the kernel image IMHO.
> 
> This also matches to what real users will do since distributions most likely
> will have a minimal kernel and every possible hardware support will be enabled
> as a loadable kernel module. This is what distros do for other platforms too.
> 
> If someone has a different use case and wants a kernel image that is optimized
> for a particular platform then she has to create its own defconfig anyways.

Distributions usually use their own configs anyway and the current most
popular use case for exynos_defconfig (not multi_v7_defconfig) seems to
be to build kernel image alone and use it without any modules:

$ grep "=m" arch/arm/configs/exynos_defconfig
CONFIG_DM_CRYPT=m

$ grep "=m" .config
CONFIG_NET_IP_TUNNEL=m
CONFIG_INET_TUNNEL=m
CONFIG_IPV6=m
CONFIG_INET6_XFRM_MODE_TRANSPORT=m
CONFIG_INET6_XFRM_MODE_TUNNEL=m
CONFIG_INET6_XFRM_MODE_BEET=m
CONFIG_IPV6_SIT=m
CONFIG_DM_CRYPT=m
CONFIG_CRYPTO_RNG=m
CONFIG_CRYPTO_ANSI_CPRNG=m

What I'm trying to say is that there is a high probability that people
will continue to use just the kernel image for exynos_defconfig and
will therefore miss SBS battery support altogether (which is only 3.6
kB of code more in the kernel image so there is no much gain in making
it modular currently).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[RESEND PATCH 1/1] ARM: exynos_defconfig: Enable SBS battery support

[Doug Anderson]

Bartlomiej,

On Mon, Aug 11, 2014 at 8:42 AM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
>
> Hi,
>
> On Monday, August 11, 2014 02:52:27 PM Javier Martinez Canillas wrote:
>> Hello Bartlomiej,
>>
>> On 08/11/2014 02:40 PM, Bartlomiej Zolnierkiewicz wrote:
>> >> index fc7d168..c390bb9 100644
>> >> --- a/arch/arm/configs/exynos_defconfig
>> >> +++ b/arch/arm/configs/exynos_defconfig
>> >> @@ -77,6 +77,7 @@ CONFIG_SPI_S3C64XX=y
>> >>  CONFIG_I2C_S3C2410=y
>> >>  CONFIG_DEBUG_GPIO=y
>> >>  CONFIG_POWER_SUPPLY=y
>> >> +CONFIG_BATTERY_SBS=m
>> >
>> > Why not make it "=y"?
>> >
>> > Rationale:
>> > - currently no hardware related option uses "=m" in exynos_defconfig
>> > - it would match the SBS option usage in multi_v7_defconfig
>> >
>> >>  CONFIG_CHARGER_TPS65090=y
>> >>  # CONFIG_HWMON is not set
>> >>  CONFIG_THERMAL=y
>> >
>>
>> I know but personally I think this should be changed. The idea of having a multi
>> platform kernel is to build a single kernel image that can be used to boot
>> different platforms. Not all platforms have a SBS-compliant battery so this
>> support shouldn't be built in the kernel image IMHO.
>>
>> This also matches to what real users will do since distributions most likely
>> will have a minimal kernel and every possible hardware support will be enabled
>> as a loadable kernel module. This is what distros do for other platforms too.
>>
>> If someone has a different use case and wants a kernel image that is optimized
>> for a particular platform then she has to create its own defconfig anyways.
>
> Distributions usually use their own configs anyway and the current most
> popular use case for exynos_defconfig (not multi_v7_defconfig) seems to
> be to build kernel image alone and use it without any modules:
>
> $ grep "=m" arch/arm/configs/exynos_defconfig
> CONFIG_DM_CRYPT=m
>
> $ grep "=m" .config
> CONFIG_NET_IP_TUNNEL=m
> CONFIG_INET_TUNNEL=m
> CONFIG_IPV6=m
> CONFIG_INET6_XFRM_MODE_TRANSPORT=m
> CONFIG_INET6_XFRM_MODE_TUNNEL=m
> CONFIG_INET6_XFRM_MODE_BEET=m
> CONFIG_IPV6_SIT=m
> CONFIG_DM_CRYPT=m
> CONFIG_CRYPTO_RNG=m
> CONFIG_CRYPTO_ANSI_CPRNG=m
>
> What I'm trying to say is that there is a high probability that people
> will continue to use just the kernel image for exynos_defconfig and
> will therefore miss SBS battery support altogether (which is only 3.6
> kB of code more in the kernel image so there is no much gain in making
> it modular currently).

I'm not against making it =y for exynos_defconfig.  I do pretty
strongly agree that the multi_v7 version should be =m eventually,
though.  We'd need to do everything we can to make that kernel
smaller.

-Doug

[RESEND PATCH 1/1] ARM: exynos_defconfig: Enable SBS battery support

[Javier Martinez Canillas]

Hello,

On 08/11/2014 05:59 PM, Doug Anderson wrote:
> Bartlomiej,
> 
> On Mon, Aug 11, 2014 at 8:42 AM, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
>>
>> Hi,
>>
>> On Monday, August 11, 2014 02:52:27 PM Javier Martinez Canillas wrote:
>>> Hello Bartlomiej,
>>>
>>> On 08/11/2014 02:40 PM, Bartlomiej Zolnierkiewicz wrote:
>>> >> index fc7d168..c390bb9 100644
>>> >> --- a/arch/arm/configs/exynos_defconfig
>>> >> +++ b/arch/arm/configs/exynos_defconfig
>>> >> @@ -77,6 +77,7 @@ CONFIG_SPI_S3C64XX=y
>>> >>  CONFIG_I2C_S3C2410=y
>>> >>  CONFIG_DEBUG_GPIO=y
>>> >>  CONFIG_POWER_SUPPLY=y
>>> >> +CONFIG_BATTERY_SBS=m
>>> >
>>> > Why not make it "=y"?
>>> >
>>> > Rationale:
>>> > - currently no hardware related option uses "=m" in exynos_defconfig
>>> > - it would match the SBS option usage in multi_v7_defconfig
>>> >
>>> >>  CONFIG_CHARGER_TPS65090=y
>>> >>  # CONFIG_HWMON is not set
>>> >>  CONFIG_THERMAL=y
>>> >
>>>
>>> I know but personally I think this should be changed. The idea of having a multi
>>> platform kernel is to build a single kernel image that can be used to boot
>>> different platforms. Not all platforms have a SBS-compliant battery so this
>>> support shouldn't be built in the kernel image IMHO.
>>>
>>> This also matches to what real users will do since distributions most likely
>>> will have a minimal kernel and every possible hardware support will be enabled
>>> as a loadable kernel module. This is what distros do for other platforms too.
>>>
>>> If someone has a different use case and wants a kernel image that is optimized
>>> for a particular platform then she has to create its own defconfig anyways.
>>
>> Distributions usually use their own configs anyway and the current most
>> popular use case for exynos_defconfig (not multi_v7_defconfig) seems to
>> be to build kernel image alone and use it without any modules:
>>
>> $ grep "=m" arch/arm/configs/exynos_defconfig
>> CONFIG_DM_CRYPT=m
>>
>> $ grep "=m" .config
>> CONFIG_NET_IP_TUNNEL=m
>> CONFIG_INET_TUNNEL=m
>> CONFIG_IPV6=m
>> CONFIG_INET6_XFRM_MODE_TRANSPORT=m
>> CONFIG_INET6_XFRM_MODE_TUNNEL=m
>> CONFIG_INET6_XFRM_MODE_BEET=m
>> CONFIG_IPV6_SIT=m
>> CONFIG_DM_CRYPT=m
>> CONFIG_CRYPTO_RNG=m
>> CONFIG_CRYPTO_ANSI_CPRNG=m
>>
>> What I'm trying to say is that there is a high probability that people
>> will continue to use just the kernel image for exynos_defconfig and
>> will therefore miss SBS battery support altogether (which is only 3.6
>> kB of code more in the kernel image so there is no much gain in making
>> it modular currently).
> 
> I'm not against making it =y for exynos_defconfig.  I do pretty
> strongly agree that the multi_v7 version should be =m eventually,
> though.  We'd need to do everything we can to make that kernel
> smaller.
>

Same for me. I don't have such a strong opinion about this so if you think that
I should re-spin to change to =m, I will.

I do think that we should try to keep the delta between exynos_defconfig and
multi_v7_defconfig as small as possible and eventually even get rid of
exynos_defconfig. Since building everything as built-in and having a config
targeted to a single platform is not aligned with the effort to have support for
multi platforms kernels.

> -Doug
> 

Best regards,
Javier

[RESEND PATCH 1/1] ARM: exynos_defconfig: Enable SBS battery support

[Bartlomiej Zolnierkiewicz]

Hi,

On Monday, August 11, 2014 06:23:01 PM Javier Martinez Canillas wrote:
> Hello,
> 
> On 08/11/2014 05:59 PM, Doug Anderson wrote:
> > Bartlomiej,
> > 
> > On Mon, Aug 11, 2014 at 8:42 AM, Bartlomiej Zolnierkiewicz
> > <b.zolnierkie@samsung.com> wrote:
> >>
> >> Hi,
> >>
> >> On Monday, August 11, 2014 02:52:27 PM Javier Martinez Canillas wrote:
> >>> Hello Bartlomiej,
> >>>
> >>> On 08/11/2014 02:40 PM, Bartlomiej Zolnierkiewicz wrote:
> >>> >> index fc7d168..c390bb9 100644
> >>> >> --- a/arch/arm/configs/exynos_defconfig
> >>> >> +++ b/arch/arm/configs/exynos_defconfig
> >>> >> @@ -77,6 +77,7 @@ CONFIG_SPI_S3C64XX=y
> >>> >>  CONFIG_I2C_S3C2410=y
> >>> >>  CONFIG_DEBUG_GPIO=y
> >>> >>  CONFIG_POWER_SUPPLY=y
> >>> >> +CONFIG_BATTERY_SBS=m
> >>> >
> >>> > Why not make it "=y"?
> >>> >
> >>> > Rationale:
> >>> > - currently no hardware related option uses "=m" in exynos_defconfig
> >>> > - it would match the SBS option usage in multi_v7_defconfig
> >>> >
> >>> >>  CONFIG_CHARGER_TPS65090=y
> >>> >>  # CONFIG_HWMON is not set
> >>> >>  CONFIG_THERMAL=y
> >>> >
> >>>
> >>> I know but personally I think this should be changed. The idea of having a multi
> >>> platform kernel is to build a single kernel image that can be used to boot
> >>> different platforms. Not all platforms have a SBS-compliant battery so this
> >>> support shouldn't be built in the kernel image IMHO.
> >>>
> >>> This also matches to what real users will do since distributions most likely
> >>> will have a minimal kernel and every possible hardware support will be enabled
> >>> as a loadable kernel module. This is what distros do for other platforms too.
> >>>
> >>> If someone has a different use case and wants a kernel image that is optimized
> >>> for a particular platform then she has to create its own defconfig anyways.
> >>
> >> Distributions usually use their own configs anyway and the current most
> >> popular use case for exynos_defconfig (not multi_v7_defconfig) seems to
> >> be to build kernel image alone and use it without any modules:
> >>
> >> $ grep "=m" arch/arm/configs/exynos_defconfig
> >> CONFIG_DM_CRYPT=m
> >>
> >> $ grep "=m" .config
> >> CONFIG_NET_IP_TUNNEL=m
> >> CONFIG_INET_TUNNEL=m
> >> CONFIG_IPV6=m
> >> CONFIG_INET6_XFRM_MODE_TRANSPORT=m
> >> CONFIG_INET6_XFRM_MODE_TUNNEL=m
> >> CONFIG_INET6_XFRM_MODE_BEET=m
> >> CONFIG_IPV6_SIT=m
> >> CONFIG_DM_CRYPT=m
> >> CONFIG_CRYPTO_RNG=m
> >> CONFIG_CRYPTO_ANSI_CPRNG=m
> >>
> >> What I'm trying to say is that there is a high probability that people
> >> will continue to use just the kernel image for exynos_defconfig and
> >> will therefore miss SBS battery support altogether (which is only 3.6
> >> kB of code more in the kernel image so there is no much gain in making
> >> it modular currently).
> > 
> > I'm not against making it =y for exynos_defconfig.  I do pretty
> > strongly agree that the multi_v7 version should be =m eventually,
> > though.  We'd need to do everything we can to make that kernel
> > smaller.
> >
> 
> Same for me. I don't have such a strong opinion about this so if you think that
> I should re-spin to change to =m, I will.
> 
> I do think that we should try to keep the delta between exynos_defconfig and
> multi_v7_defconfig as small as possible and eventually even get rid of
> exynos_defconfig. Since building everything as built-in and having a config

I completely agree.  I proposed exynos_defconfig removal as soon as
Exynos gained multiplatform support (and before exynos_defconfig
started getting out-of-sync with multi_v7_defconfig).  There were
arguments that it is still useful in some cases.  I think that if
it would be possible to go from a modular multi_v7_defconfig to
a modular/built-in single platform config (using a script?) all
such use cases will be covered.

> targeted to a single platform is not aligned with the effort to have support for
> multi platforms kernels.

Currently even for multi_v7_defconfig "=m" seems to be an exception
from the general rule:

$ grep "=m" arch/arm/configs/multi_v7_defconfig 
CONFIG_INET6_AH=m
CONFIG_INET6_ESP=m
CONFIG_INET6_IPCOMP=m
CONFIG_IPV6_MIP6=m
CONFIG_IPV6_TUNNEL=m
CONFIG_CFG80211=m
CONFIG_MAC80211=m
CONFIG_BRCMFMAC=m
CONFIG_RT2X00=m
CONFIG_RT2800USB=m

$ wc -l arch/arm/configs/multi_v7_defconfig 
405 arch/arm/configs/multi_v7_defconfig

so I would vote for CONFIG_BATTERY_SBS=y for both configs.  Especially
since it results in only 3.6 kB bigger kernel image (0.05% kernel size
increase for kernel image built with exynos_defconfig, probably a lot
less for multi_v7_defconfig one).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[RESEND PATCH 1/1] ARM: exynos_defconfig: Enable SBS battery support

[Doug Anderson]

Hi,

On Mon, Aug 11, 2014 at 10:28 AM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
>
> Hi,
>
> On Monday, August 11, 2014 06:23:01 PM Javier Martinez Canillas wrote:
>> Hello,
>>
>> On 08/11/2014 05:59 PM, Doug Anderson wrote:
>> > Bartlomiej,
>> >
>> > On Mon, Aug 11, 2014 at 8:42 AM, Bartlomiej Zolnierkiewicz
>> > <b.zolnierkie@samsung.com> wrote:
>> >>
>> >> Hi,
>> >>
>> >> On Monday, August 11, 2014 02:52:27 PM Javier Martinez Canillas wrote:
>> >>> Hello Bartlomiej,
>> >>>
>> >>> On 08/11/2014 02:40 PM, Bartlomiej Zolnierkiewicz wrote:
>> >>> >> index fc7d168..c390bb9 100644
>> >>> >> --- a/arch/arm/configs/exynos_defconfig
>> >>> >> +++ b/arch/arm/configs/exynos_defconfig
>> >>> >> @@ -77,6 +77,7 @@ CONFIG_SPI_S3C64XX=y
>> >>> >>  CONFIG_I2C_S3C2410=y
>> >>> >>  CONFIG_DEBUG_GPIO=y
>> >>> >>  CONFIG_POWER_SUPPLY=y
>> >>> >> +CONFIG_BATTERY_SBS=m
>> >>> >
>> >>> > Why not make it "=y"?
>> >>> >
>> >>> > Rationale:
>> >>> > - currently no hardware related option uses "=m" in exynos_defconfig
>> >>> > - it would match the SBS option usage in multi_v7_defconfig
>> >>> >
>> >>> >>  CONFIG_CHARGER_TPS65090=y
>> >>> >>  # CONFIG_HWMON is not set
>> >>> >>  CONFIG_THERMAL=y
>> >>> >
>> >>>
>> >>> I know but personally I think this should be changed. The idea of having a multi
>> >>> platform kernel is to build a single kernel image that can be used to boot
>> >>> different platforms. Not all platforms have a SBS-compliant battery so this
>> >>> support shouldn't be built in the kernel image IMHO.
>> >>>
>> >>> This also matches to what real users will do since distributions most likely
>> >>> will have a minimal kernel and every possible hardware support will be enabled
>> >>> as a loadable kernel module. This is what distros do for other platforms too.
>> >>>
>> >>> If someone has a different use case and wants a kernel image that is optimized
>> >>> for a particular platform then she has to create its own defconfig anyways.
>> >>
>> >> Distributions usually use their own configs anyway and the current most
>> >> popular use case for exynos_defconfig (not multi_v7_defconfig) seems to
>> >> be to build kernel image alone and use it without any modules:
>> >>
>> >> $ grep "=m" arch/arm/configs/exynos_defconfig
>> >> CONFIG_DM_CRYPT=m
>> >>
>> >> $ grep "=m" .config
>> >> CONFIG_NET_IP_TUNNEL=m
>> >> CONFIG_INET_TUNNEL=m
>> >> CONFIG_IPV6=m
>> >> CONFIG_INET6_XFRM_MODE_TRANSPORT=m
>> >> CONFIG_INET6_XFRM_MODE_TUNNEL=m
>> >> CONFIG_INET6_XFRM_MODE_BEET=m
>> >> CONFIG_IPV6_SIT=m
>> >> CONFIG_DM_CRYPT=m
>> >> CONFIG_CRYPTO_RNG=m
>> >> CONFIG_CRYPTO_ANSI_CPRNG=m
>> >>
>> >> What I'm trying to say is that there is a high probability that people
>> >> will continue to use just the kernel image for exynos_defconfig and
>> >> will therefore miss SBS battery support altogether (which is only 3.6
>> >> kB of code more in the kernel image so there is no much gain in making
>> >> it modular currently).
>> >
>> > I'm not against making it =y for exynos_defconfig.  I do pretty
>> > strongly agree that the multi_v7 version should be =m eventually,
>> > though.  We'd need to do everything we can to make that kernel
>> > smaller.
>> >
>>
>> Same for me. I don't have such a strong opinion about this so if you think that
>> I should re-spin to change to =m, I will.
>>
>> I do think that we should try to keep the delta between exynos_defconfig and
>> multi_v7_defconfig as small as possible and eventually even get rid of
>> exynos_defconfig. Since building everything as built-in and having a config
>
> I completely agree.  I proposed exynos_defconfig removal as soon as
> Exynos gained multiplatform support (and before exynos_defconfig
> started getting out-of-sync with multi_v7_defconfig).  There were
> arguments that it is still useful in some cases.  I think that if
> it would be possible to go from a modular multi_v7_defconfig to
> a modular/built-in single platform config (using a script?) all
> such use cases will be covered.

There's one really nice thing about having an "exynos_defconfig".  It
makes it a little easier to tease apart what's actually needed if you
just want to build a product kernel that support exynos.

Most product kernels shipping in embedded devices only support one or
maybe a small number of SoCs.  If you need to try to figure out what
parts of the "multi_v7" config is relevant then it's very hard.

It's also nice to help measure multiplatform code bloat.  You can keep
measuring the output kernel size of the exynos kernel to see how much
bloat all of the mutliplatform stuff has added.


> so I would vote for CONFIG_BATTERY_SBS=y for both configs.  Especially
> since it results in only 3.6 kB bigger kernel image (0.05% kernel size
> increase for kernel image built with exynos_defconfig, probably a lot
> less for multi_v7_defconfig one).

Death by 1000 cuts is a very painful death indeed, even if each cut is small.

-Doug

[RESEND PATCH 1/1] ARM: exynos_defconfig: Enable SBS battery support

[Javier Martinez Canillas]

Hello Bartlomiej,

On 08/11/2014 07:28 PM, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On Monday, August 11, 2014 06:23:01 PM Javier Martinez Canillas wrote:
>> Hello,
>> 
>> On 08/11/2014 05:59 PM, Doug Anderson wrote:
>> > Bartlomiej,
>> > 
>> > On Mon, Aug 11, 2014 at 8:42 AM, Bartlomiej Zolnierkiewicz
>> > <b.zolnierkie@samsung.com> wrote:
>> >>
>> >> Hi,
>> >>
>> >> On Monday, August 11, 2014 02:52:27 PM Javier Martinez Canillas wrote:
>> >>> Hello Bartlomiej,
>> >>>
>> >>> On 08/11/2014 02:40 PM, Bartlomiej Zolnierkiewicz wrote:
>> >>> >> index fc7d168..c390bb9 100644
>> >>> >> --- a/arch/arm/configs/exynos_defconfig
>> >>> >> +++ b/arch/arm/configs/exynos_defconfig
>> >>> >> @@ -77,6 +77,7 @@ CONFIG_SPI_S3C64XX=y
>> >>> >>  CONFIG_I2C_S3C2410=y
>> >>> >>  CONFIG_DEBUG_GPIO=y
>> >>> >>  CONFIG_POWER_SUPPLY=y
>> >>> >> +CONFIG_BATTERY_SBS=m
>> >>> >
>> >>> > Why not make it "=y"?
>> >>> >
>> >>> > Rationale:
>> >>> > - currently no hardware related option uses "=m" in exynos_defconfig
>> >>> > - it would match the SBS option usage in multi_v7_defconfig
>> >>> >
>> >>> >>  CONFIG_CHARGER_TPS65090=y
>> >>> >>  # CONFIG_HWMON is not set
>> >>> >>  CONFIG_THERMAL=y
>> >>> >
>> >>>
>> >>> I know but personally I think this should be changed. The idea of having a multi
>> >>> platform kernel is to build a single kernel image that can be used to boot
>> >>> different platforms. Not all platforms have a SBS-compliant battery so this
>> >>> support shouldn't be built in the kernel image IMHO.
>> >>>
>> >>> This also matches to what real users will do since distributions most likely
>> >>> will have a minimal kernel and every possible hardware support will be enabled
>> >>> as a loadable kernel module. This is what distros do for other platforms too.
>> >>>
>> >>> If someone has a different use case and wants a kernel image that is optimized
>> >>> for a particular platform then she has to create its own defconfig anyways.
>> >>
>> >> Distributions usually use their own configs anyway and the current most
>> >> popular use case for exynos_defconfig (not multi_v7_defconfig) seems to
>> >> be to build kernel image alone and use it without any modules:
>> >>
>> >> $ grep "=m" arch/arm/configs/exynos_defconfig
>> >> CONFIG_DM_CRYPT=m
>> >>
>> >> $ grep "=m" .config
>> >> CONFIG_NET_IP_TUNNEL=m
>> >> CONFIG_INET_TUNNEL=m
>> >> CONFIG_IPV6=m
>> >> CONFIG_INET6_XFRM_MODE_TRANSPORT=m
>> >> CONFIG_INET6_XFRM_MODE_TUNNEL=m
>> >> CONFIG_INET6_XFRM_MODE_BEET=m
>> >> CONFIG_IPV6_SIT=m
>> >> CONFIG_DM_CRYPT=m
>> >> CONFIG_CRYPTO_RNG=m
>> >> CONFIG_CRYPTO_ANSI_CPRNG=m
>> >>
>> >> What I'm trying to say is that there is a high probability that people
>> >> will continue to use just the kernel image for exynos_defconfig and
>> >> will therefore miss SBS battery support altogether (which is only 3.6
>> >> kB of code more in the kernel image so there is no much gain in making
>> >> it modular currently).
>> > 
>> > I'm not against making it =y for exynos_defconfig.  I do pretty
>> > strongly agree that the multi_v7 version should be =m eventually,
>> > though.  We'd need to do everything we can to make that kernel
>> > smaller.
>> >
>> 
>> Same for me. I don't have such a strong opinion about this so if you think that
>> I should re-spin to change to =m, I will.
>> 
>> I do think that we should try to keep the delta between exynos_defconfig and
>> multi_v7_defconfig as small as possible and eventually even get rid of
>> exynos_defconfig. Since building everything as built-in and having a config
> 
> I completely agree.  I proposed exynos_defconfig removal as soon as
> Exynos gained multiplatform support (and before exynos_defconfig
> started getting out-of-sync with multi_v7_defconfig).  There were
> arguments that it is still useful in some cases.  I think that if
> it would be possible to go from a modular multi_v7_defconfig to
> a modular/built-in single platform config (using a script?) all
> such use cases will be covered.
> 
>> targeted to a single platform is not aligned with the effort to have support for
>> multi platforms kernels.
> 
> Currently even for multi_v7_defconfig "=m" seems to be an exception
> from the general rule:
> 
> $ grep "=m" arch/arm/configs/multi_v7_defconfig 
> CONFIG_INET6_AH=m
> CONFIG_INET6_ESP=m
> CONFIG_INET6_IPCOMP=m
> CONFIG_IPV6_MIP6=m
> CONFIG_IPV6_TUNNEL=m
> CONFIG_CFG80211=m
> CONFIG_MAC80211=m
> CONFIG_BRCMFMAC=m
> CONFIG_RT2X00=m
> CONFIG_RT2800USB=m
> 
> $ wc -l arch/arm/configs/multi_v7_defconfig 
> 405 arch/arm/configs/multi_v7_defconfig
> 

Again, I think this is more of a consequence of people adding the config they
use (which as you said in most cases is just built everything in the kernel to
avoid copying modules) than something that was made on purpose.

> so I would vote for CONFIG_BATTERY_SBS=y for both configs.  Especially
> since it results in only 3.6 kB bigger kernel image (0.05% kernel size
> increase for kernel image built with exynos_defconfig, probably a lot
> less for multi_v7_defconfig one).
> 

I'll repost with =y for exynos_defconfig but I really think that for
multi_v7_defconfig we should have everything as a module if possible. It's just
3.6 kB but if every single platform do the same the kernel will be quite bloated
for little gain.

> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 

Best regards,
Javier

[RESEND PATCH 1/1] ARM: exynos_defconfig: Enable SBS battery support

[Doug Anderson]

Javier,

On Mon, Aug 11, 2014 at 4:06 AM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> Many Exynos5 boards (e.g: Snow, Peach Pit and Pi) have
> a SBS-compliant gas gauge battery. Enable it as module
> so the needed support is available for these boards.
>
> Suggested-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Acked-by: Kukjin Kim <kgene.kim@samsung.com>
> ---
>  arch/arm/configs/exynos_defconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
> index fc7d168..c390bb9 100644
> --- a/arch/arm/configs/exynos_defconfig
> +++ b/arch/arm/configs/exynos_defconfig
> @@ -77,6 +77,7 @@ CONFIG_SPI_S3C64XX=y
>  CONFIG_I2C_S3C2410=y
>  CONFIG_DEBUG_GPIO=y
>  CONFIG_POWER_SUPPLY=y
> +CONFIG_BATTERY_SBS=m
>  CONFIG_CHARGER_TPS65090=y
>  # CONFIG_HWMON is not set
>  CONFIG_THERMAL=y

I'm good with this, so:

Reviewed-by: Doug Anderson <dianders@chromium.org>

To address Bartlomiej: =m is what's in ChromeOS for exynos, though I
notice that other platforms in the ChromeOS tree have =y.  I'm not
sure why there is a difference, but it's probably just carelessness.

Normally in ChromeOS we use "=m" for drivers that can afford to wait
until after the UI is up and running.  That helps improve boot time
since the user can use the device sooner (and it's OK if the battery
driver takes a few extra seconds to load).  The only things that
really should be "=y" are drivers that are critical to load early or
that are needed for the user to interact with the system.  In this
case you could argue that loading sbs earlier means that if the system
is low on battery it will get back to "shutdown" sooner.  I'm not sure
getting the shutdown a few seconds earlier is really critical,
though...

-Doug