| Message ID | 1407755198-17568-1-git-send-email-javier.martinez@collabora.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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