| Message ID | 20210920190350.3860821-2-willmcvicker@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Kconfig: Update ARCH_EXYNOS select configs | expand |
On 20/09/2021 21:03, Will McVicker wrote: > COMMON_CLK_SAMSUNG is selected by ARCH_EXYNOS which forces this config > to be built-in when ARCH_EXYNOS is enabled. Switch the logic to use a > "default y if ARCH_EXYNOS" to provide flexibilty for vendors to disable > or modularize this driver. The clock drivers are essential, you cannot disable them for a generic kernel supporting ARCH_EXYNOS. Such kernel won't work properly on platforms. > > I verified the .config is identical with and without this change. > > Signed-off-by: Will McVicker <willmcvicker@google.com> > --- > arch/arm64/Kconfig.platforms | 1 - > drivers/clk/samsung/Kconfig | 1 + > 2 files changed, 1 insertion(+), 1 deletion(-) > NAK, please use get_maintainers.pl to Cc necessary folks. Best regards, Krzysztof
On Tue, Sep 21, 2021 at 9:31 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > On 20/09/2021 21:03, Will McVicker wrote: > > COMMON_CLK_SAMSUNG is selected by ARCH_EXYNOS which forces this config > > to be built-in when ARCH_EXYNOS is enabled. Switch the logic to use a > > "default y if ARCH_EXYNOS" to provide flexibilty for vendors to disable > > or modularize this driver. > > The clock drivers are essential, you cannot disable them for a generic > kernel supporting ARCH_EXYNOS. Such kernel won't work properly on platforms. Obviously it's not gonna work if the clock driver is not enabled at all. But does it work if you make the clock driver modular, and put it with all other essential driver modules in initramfs? Debugging would be hard, as the serial console driver also relies on clocks and PM Domains etc. If not, this patch should be NAKed, until it works with a modular clock driver. If yes, perhaps another line should be added (_before_ the other line)? + default m if ARCH_EXYNOS && MODULES default y if ARCH_EXYNOS However, many developers may want MODULES=y, but not want to bother with an initramfs. So perhaps we need a new symbol MINIMUM_GENERIC_KERNEL or so, protected by EXPERT, and make the driver default to m if that is enabled? Gr{oetje,eeting}s, Geert
On 21/09/2021 09:50, Geert Uytterhoeven wrote: > On Tue, Sep 21, 2021 at 9:31 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: >> On 20/09/2021 21:03, Will McVicker wrote: >>> COMMON_CLK_SAMSUNG is selected by ARCH_EXYNOS which forces this config >>> to be built-in when ARCH_EXYNOS is enabled. Switch the logic to use a >>> "default y if ARCH_EXYNOS" to provide flexibilty for vendors to disable >>> or modularize this driver. >> >> The clock drivers are essential, you cannot disable them for a generic >> kernel supporting ARCH_EXYNOS. Such kernel won't work properly on platforms. > > Obviously it's not gonna work if the clock driver is not enabled > at all. But does it work if you make the clock driver modular, and > put it with all other essential driver modules in initramfs? Debugging > would be hard, as the serial console driver also relies on clocks > and PM Domains etc. The kernel could boot without clock drivers (default settings from bootloader), probe clocks from initramfs and proceed with rootfs from eMMC/SD/net. In theory. However I have no reports that it ever worked. If there is such working upstream configuration, I don't mind here. Just please explain this in the commit msg. > > If not, this patch should be NAKed, until it works with a modular > clock driver. > > If yes, perhaps another line should be added (_before_ the other line)? > > + default m if ARCH_EXYNOS && MODULES > default y if ARCH_EXYNOS > > However, many developers may want MODULES=y, but not want to bother > with an initramfs. So perhaps we need a new symbol > MINIMUM_GENERIC_KERNEL or so, protected by EXPERT, and make the > driver default to m if that is enabled? Yeah, that's indeed a problem to solve. For most users (and distros) building kernel for Exynos this should be built-in by default. Anyway, the option is non-selectable so it cannot be converted to "m" or disabled. And this is claimed in the commit msg: "provide flexibilty for vendors to disable or modularize this driver." The commit does not achieve it. Best regards, Krzysztof
On Tue, Sep 21, 2021 at 1:35 AM Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> wrote: > > On 21/09/2021 09:50, Geert Uytterhoeven wrote: > > On Tue, Sep 21, 2021 at 9:31 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > >> On 20/09/2021 21:03, Will McVicker wrote: > >>> COMMON_CLK_SAMSUNG is selected by ARCH_EXYNOS which forces this config > >>> to be built-in when ARCH_EXYNOS is enabled. Switch the logic to use a > >>> "default y if ARCH_EXYNOS" to provide flexibilty for vendors to disable > >>> or modularize this driver. > >> > >> The clock drivers are essential, you cannot disable them for a generic > >> kernel supporting ARCH_EXYNOS. Such kernel won't work properly on platforms. > > > > Obviously it's not gonna work if the clock driver is not enabled > > at all. But does it work if you make the clock driver modular, and > > put it with all other essential driver modules in initramfs? Debugging > > would be hard, as the serial console driver also relies on clocks > > and PM Domains etc. > > The kernel could boot without clock drivers (default settings from > bootloader), probe clocks from initramfs and proceed with rootfs from > eMMC/SD/net. > > In theory. > > However I have no reports that it ever worked. If there is such working > upstream configuration, I don't mind here. Just please explain this in > the commit msg. > > > > > If not, this patch should be NAKed, until it works with a modular > > clock driver. > > > > If yes, perhaps another line should be added (_before_ the other line)? > > > > + default m if ARCH_EXYNOS && MODULES > > default y if ARCH_EXYNOS > > > > However, many developers may want MODULES=y, but not want to bother > > with an initramfs. So perhaps we need a new symbol > > MINIMUM_GENERIC_KERNEL or so, protected by EXPERT, and make the > > driver default to m if that is enabled? > > Yeah, that's indeed a problem to solve. For most users (and distros) > building kernel for Exynos this should be built-in by default. > > Anyway, the option is non-selectable so it cannot be converted to "m" or > disabled. And this is claimed in the commit msg: > "provide flexibilty for vendors to disable or modularize this driver." > > The commit does not achieve it. > > Best regards, > Krzysztof Thanks for the reviews! As Lee has explained in his replies, the intent of this series is to provide config flexibility to create a defconfig that allows us to move out SoC specific drivers in order to create a generic kernel that can be used across multiple devices with different SoCs. I'm sorry I added confusion by mentioning modularization. All of these drivers that I am modifying in this series can be modularized which is an ongoing effort, but is not addressed here and I don't believe that modularizing them should be a requirement before supporting enabling/disabling them. I will update the series with my patch that refactors the Samsung SoC drivers menuconfig to make these visible as well. Thanks, Will
On 21/09/2021 19:58, Will McVicker wrote: > On Tue, Sep 21, 2021 at 1:35 AM Krzysztof Kozlowski > <krzysztof.kozlowski@canonical.com> wrote: >> >> On 21/09/2021 09:50, Geert Uytterhoeven wrote: >>> On Tue, Sep 21, 2021 at 9:31 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: >>>> On 20/09/2021 21:03, Will McVicker wrote: >>>>> COMMON_CLK_SAMSUNG is selected by ARCH_EXYNOS which forces this config >>>>> to be built-in when ARCH_EXYNOS is enabled. Switch the logic to use a >>>>> "default y if ARCH_EXYNOS" to provide flexibilty for vendors to disable >>>>> or modularize this driver. >>>> >>>> The clock drivers are essential, you cannot disable them for a generic >>>> kernel supporting ARCH_EXYNOS. Such kernel won't work properly on platforms. >>> >>> Obviously it's not gonna work if the clock driver is not enabled >>> at all. But does it work if you make the clock driver modular, and >>> put it with all other essential driver modules in initramfs? Debugging >>> would be hard, as the serial console driver also relies on clocks >>> and PM Domains etc. >> >> The kernel could boot without clock drivers (default settings from >> bootloader), probe clocks from initramfs and proceed with rootfs from >> eMMC/SD/net. >> >> In theory. >> >> However I have no reports that it ever worked. If there is such working >> upstream configuration, I don't mind here. Just please explain this in >> the commit msg. >> >>> >>> If not, this patch should be NAKed, until it works with a modular >>> clock driver. >>> >>> If yes, perhaps another line should be added (_before_ the other line)? >>> >>> + default m if ARCH_EXYNOS && MODULES >>> default y if ARCH_EXYNOS >>> >>> However, many developers may want MODULES=y, but not want to bother >>> with an initramfs. So perhaps we need a new symbol >>> MINIMUM_GENERIC_KERNEL or so, protected by EXPERT, and make the >>> driver default to m if that is enabled? >> >> Yeah, that's indeed a problem to solve. For most users (and distros) >> building kernel for Exynos this should be built-in by default. >> >> Anyway, the option is non-selectable so it cannot be converted to "m" or >> disabled. And this is claimed in the commit msg: >> "provide flexibilty for vendors to disable or modularize this driver." >> >> The commit does not achieve it. >> >> Best regards, >> Krzysztof > > Thanks for the reviews! As Lee has explained in his replies, the > intent of this series is to provide config flexibility to create a > defconfig that allows us to move out SoC specific drivers in order to > create a generic kernel that can be used across multiple devices with > different SoCs. That's quite generic statement... or let me put it that way - we already have this ability to create a generic kernel supporting different SoCs. Exynos and other ARMv7 and ARMv8 platforms are multiplatform. Task is done. Please be more specific about use case and describe what exactly in current upstream multiplatform kernel is missing, what is not multiplatform enough. > I'm sorry I added confusion by mentioning > modularization. All of these drivers that I am modifying in this > series can be modularized which is an ongoing effort, but is not > addressed here and I don't believe that modularizing them should be a > requirement before supporting enabling/disabling them. Since the disabling the driver for a kernel supporting Exynos does not make any sense, then making it at least modular unfortunately it is a requirement. > I will update the series with my patch that refactors the Samsung SoC > drivers menuconfig to make these visible as well. I would first recommend to really describe your use case because my questions about this are still unanswered. Best regards, Krzysztof
On Tue, 21 Sep 2021, Krzysztof Kozlowski wrote: > On 21/09/2021 19:58, Will McVicker wrote: > > On Tue, Sep 21, 2021 at 1:35 AM Krzysztof Kozlowski > > <krzysztof.kozlowski@canonical.com> wrote: > >> > >> On 21/09/2021 09:50, Geert Uytterhoeven wrote: > >>> On Tue, Sep 21, 2021 at 9:31 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > >>>> On 20/09/2021 21:03, Will McVicker wrote: > >>>>> COMMON_CLK_SAMSUNG is selected by ARCH_EXYNOS which forces this config > >>>>> to be built-in when ARCH_EXYNOS is enabled. Switch the logic to use a > >>>>> "default y if ARCH_EXYNOS" to provide flexibilty for vendors to disable > >>>>> or modularize this driver. > >>>> > >>>> The clock drivers are essential, you cannot disable them for a generic > >>>> kernel supporting ARCH_EXYNOS. Such kernel won't work properly on platforms. > >>> > >>> Obviously it's not gonna work if the clock driver is not enabled > >>> at all. But does it work if you make the clock driver modular, and > >>> put it with all other essential driver modules in initramfs? Debugging > >>> would be hard, as the serial console driver also relies on clocks > >>> and PM Domains etc. > >> > >> The kernel could boot without clock drivers (default settings from > >> bootloader), probe clocks from initramfs and proceed with rootfs from > >> eMMC/SD/net. > >> > >> In theory. > >> > >> However I have no reports that it ever worked. If there is such working > >> upstream configuration, I don't mind here. Just please explain this in > >> the commit msg. > >> > >>> > >>> If not, this patch should be NAKed, until it works with a modular > >>> clock driver. > >>> > >>> If yes, perhaps another line should be added (_before_ the other line)? > >>> > >>> + default m if ARCH_EXYNOS && MODULES > >>> default y if ARCH_EXYNOS > >>> > >>> However, many developers may want MODULES=y, but not want to bother > >>> with an initramfs. So perhaps we need a new symbol > >>> MINIMUM_GENERIC_KERNEL or so, protected by EXPERT, and make the > >>> driver default to m if that is enabled? > >> > >> Yeah, that's indeed a problem to solve. For most users (and distros) > >> building kernel for Exynos this should be built-in by default. > >> > >> Anyway, the option is non-selectable so it cannot be converted to "m" or > >> disabled. And this is claimed in the commit msg: > >> "provide flexibilty for vendors to disable or modularize this driver." > >> > >> The commit does not achieve it. > >> > >> Best regards, > >> Krzysztof > > > > Thanks for the reviews! As Lee has explained in his replies, the > > intent of this series is to provide config flexibility to create a > > defconfig that allows us to move out SoC specific drivers in order to > > create a generic kernel that can be used across multiple devices with > > different SoCs. > > That's quite generic statement... or let me put it that way - we already > have this ability to create a generic kernel supporting different SoCs. > Exynos and other ARMv7 and ARMv8 platforms are multiplatform. > > Task is done. multi_v7_defconfig and ARMv8's defconfig are bloated monoliths which provide limited flexibility. Good for testing and messing around - not much good for real products. > Please be more specific about use case and describe what exactly in > current upstream multiplatform kernel is missing, what is not > multiplatform enough. The use-case is GKI. A realistic middle-ground between fully open source and real-world usage of the Linux kernel in a competitive technical arena. GKI aims to be as close to Mainline as possible, whilst allowing hardware vendors to supply their own software containing their perceived competitive edge and/or supporting not-yet-released hardware platforms. If you end up over-constraining the ability to configure the kernel in useful/meaningful ways, that makes one of the main (best intention) aims of GKI, (i.e. to have an upstream first ethos in order to be as close to upstream as possible) much more difficult. I put in a lot of effort to ensure GKI doesn't end up as just another fork of the Linux kernel. So far, so good, but flexibility and understanding is key. > > I'm sorry I added confusion by mentioning > > modularization. All of these drivers that I am modifying in this > > series can be modularized which is an ongoing effort, but is not > > addressed here and I don't believe that modularizing them should be a > > requirement before supporting enabling/disabling them. > > Since the disabling the driver for a kernel supporting Exynos does not > make any sense, then making it at least modular unfortunately it is a > requirement. I can go with that. > > I will update the series with my patch that refactors the Samsung SoC > > drivers menuconfig to make these visible as well. > > I would first recommend to really describe your use case because my > questions about this are still unanswered. Hopefully my replies have helped somewhat. Happy to discuss further if required. If all else fails, feel free to ping me on IRC (lag).
On 23/09/2021 14:57, Lee Jones wrote: > On Tue, 21 Sep 2021, Krzysztof Kozlowski wrote: > >> On 21/09/2021 19:58, Will McVicker wrote: >>> On Tue, Sep 21, 2021 at 1:35 AM Krzysztof Kozlowski >>> <krzysztof.kozlowski@canonical.com> wrote: >>>> >>>> On 21/09/2021 09:50, Geert Uytterhoeven wrote: >>>>> On Tue, Sep 21, 2021 at 9:31 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: >>>>>> On 20/09/2021 21:03, Will McVicker wrote: >>>>>>> COMMON_CLK_SAMSUNG is selected by ARCH_EXYNOS which forces this config >>>>>>> to be built-in when ARCH_EXYNOS is enabled. Switch the logic to use a >>>>>>> "default y if ARCH_EXYNOS" to provide flexibilty for vendors to disable >>>>>>> or modularize this driver. >>>>>> >>>>>> The clock drivers are essential, you cannot disable them for a generic >>>>>> kernel supporting ARCH_EXYNOS. Such kernel won't work properly on platforms. >>>>> >>>>> Obviously it's not gonna work if the clock driver is not enabled >>>>> at all. But does it work if you make the clock driver modular, and >>>>> put it with all other essential driver modules in initramfs? Debugging >>>>> would be hard, as the serial console driver also relies on clocks >>>>> and PM Domains etc. >>>> >>>> The kernel could boot without clock drivers (default settings from >>>> bootloader), probe clocks from initramfs and proceed with rootfs from >>>> eMMC/SD/net. >>>> >>>> In theory. >>>> >>>> However I have no reports that it ever worked. If there is such working >>>> upstream configuration, I don't mind here. Just please explain this in >>>> the commit msg. >>>> >>>>> >>>>> If not, this patch should be NAKed, until it works with a modular >>>>> clock driver. >>>>> >>>>> If yes, perhaps another line should be added (_before_ the other line)? >>>>> >>>>> + default m if ARCH_EXYNOS && MODULES >>>>> default y if ARCH_EXYNOS >>>>> >>>>> However, many developers may want MODULES=y, but not want to bother >>>>> with an initramfs. So perhaps we need a new symbol >>>>> MINIMUM_GENERIC_KERNEL or so, protected by EXPERT, and make the >>>>> driver default to m if that is enabled? >>>> >>>> Yeah, that's indeed a problem to solve. For most users (and distros) >>>> building kernel for Exynos this should be built-in by default. >>>> >>>> Anyway, the option is non-selectable so it cannot be converted to "m" or >>>> disabled. And this is claimed in the commit msg: >>>> "provide flexibilty for vendors to disable or modularize this driver." >>>> >>>> The commit does not achieve it. >>>> >>>> Best regards, >>>> Krzysztof >>> >>> Thanks for the reviews! As Lee has explained in his replies, the >>> intent of this series is to provide config flexibility to create a >>> defconfig that allows us to move out SoC specific drivers in order to >>> create a generic kernel that can be used across multiple devices with >>> different SoCs. >> >> That's quite generic statement... or let me put it that way - we already >> have this ability to create a generic kernel supporting different SoCs. >> Exynos and other ARMv7 and ARMv8 platforms are multiplatform. >> >> Task is done. > > multi_v7_defconfig and ARMv8's defconfig are bloated monoliths which > provide limited flexibility. Good for testing and messing around - > not much good for real products. I am not saying about defconfigs. I am saying that ARMv8 platform is multiplatform so we already solved the problem Will mentioned. :) > >> Please be more specific about use case and describe what exactly in >> current upstream multiplatform kernel is missing, what is not >> multiplatform enough. > > The use-case is GKI. A realistic middle-ground between fully open > source and real-world usage of the Linux kernel in a competitive > technical arena. GKI aims to be as close to Mainline as possible, > whilst allowing hardware vendors to supply their own software > containing their perceived competitive edge and/or supporting > not-yet-released hardware platforms. <grumpy mode> Therefore the use case is to not contribute anything upstream around ARCH_EXYNOS but use it in millions of devices downstream with hundreds of out-of-tree modules. The use case is to make life easy for the vendor and out-of-tree code, not for the upstream. Instead of promoting upstreaming, or leaning towards usptream in some balanced way, the use case is to entirely go to out-of-tree. I am not thinking here about edge or not-yet-released platforms but "ancient" in terms of current SoC business, e.g. 3-5 years old. </grumpy mode> > > If you end up over-constraining the ability to configure the kernel in > useful/meaningful ways, that makes one of the main (best intention) > aims of GKI, (i.e. to have an upstream first ethos in order to be as > close to upstream as possible) much more difficult. GKI encourages core kernel changes to be upstreamed but it is effectively the nail in the coffin of upstreaming vendor SoC changes. There is simply no incentive for less-cooperative vendor to upstream it's modules (except usual benefits like code quality and user support which are not important for less-cooperative vendors). The kernel should be configured mainly towards mainline platforms. Not the other way around. This of course does not stop it for supporting out-of-tree code, but I guess you also know that what's out-of-tree, it does not exist. :) > > I put in a lot of effort to ensure GKI doesn't end up as just another > fork of the Linux kernel. So far, so good, but flexibility and > understanding is key. > >>> I'm sorry I added confusion by mentioning >>> modularization. All of these drivers that I am modifying in this >>> series can be modularized which is an ongoing effort, but is not >>> addressed here and I don't believe that modularizing them should be a >>> requirement before supporting enabling/disabling them. >> >> Since the disabling the driver for a kernel supporting Exynos does not >> make any sense, then making it at least modular unfortunately it is a >> requirement. > > I can go with that. > >>> I will update the series with my patch that refactors the Samsung SoC >>> drivers menuconfig to make these visible as well. >> >> I would first recommend to really describe your use case because my >> questions about this are still unanswered. > > Hopefully my replies have helped somewhat. > > Happy to discuss further if required. > > If all else fails, feel free to ping me on IRC (lag). Thanks Lee, you described the use case. In general I like it and support it, except for what I wrote in the other mail. Vendor does not contribute much therefore there is no balance in upstreaming. Since none of other vendor's platforms are supported, I am looking only at what is supported. From that perspective - the change proposed by Will and previous guys, does not have much sense. My perspective probably would change a lot if vendor did contribute some of its non-edge platforms (3-5 years old)... especially that unlike few community guys (e.g. PostmarketOS), vendor has shit-tons of money and the hardware manuals. :) Instead of pushing this change, please let's give some incentive to the vendor for upstreaming anything. Best regards, Krzysztof
On Thu, 23 Sep 2021, Krzysztof Kozlowski wrote: > On 23/09/2021 14:57, Lee Jones wrote: > > On Tue, 21 Sep 2021, Krzysztof Kozlowski wrote: > > > >> On 21/09/2021 19:58, Will McVicker wrote: > >>> On Tue, Sep 21, 2021 at 1:35 AM Krzysztof Kozlowski > >>> <krzysztof.kozlowski@canonical.com> wrote: > >>>> > >>>> On 21/09/2021 09:50, Geert Uytterhoeven wrote: > >>>>> On Tue, Sep 21, 2021 at 9:31 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > >>>>>> On 20/09/2021 21:03, Will McVicker wrote: > >>>>>>> COMMON_CLK_SAMSUNG is selected by ARCH_EXYNOS which forces this config > >>>>>>> to be built-in when ARCH_EXYNOS is enabled. Switch the logic to use a > >>>>>>> "default y if ARCH_EXYNOS" to provide flexibilty for vendors to disable > >>>>>>> or modularize this driver. > >>>>>> > >>>>>> The clock drivers are essential, you cannot disable them for a generic > >>>>>> kernel supporting ARCH_EXYNOS. Such kernel won't work properly on platforms. > >>>>> > >>>>> Obviously it's not gonna work if the clock driver is not enabled > >>>>> at all. But does it work if you make the clock driver modular, and > >>>>> put it with all other essential driver modules in initramfs? Debugging > >>>>> would be hard, as the serial console driver also relies on clocks > >>>>> and PM Domains etc. > >>>> > >>>> The kernel could boot without clock drivers (default settings from > >>>> bootloader), probe clocks from initramfs and proceed with rootfs from > >>>> eMMC/SD/net. > >>>> > >>>> In theory. > >>>> > >>>> However I have no reports that it ever worked. If there is such working > >>>> upstream configuration, I don't mind here. Just please explain this in > >>>> the commit msg. > >>>> > >>>>> > >>>>> If not, this patch should be NAKed, until it works with a modular > >>>>> clock driver. > >>>>> > >>>>> If yes, perhaps another line should be added (_before_ the other line)? > >>>>> > >>>>> + default m if ARCH_EXYNOS && MODULES > >>>>> default y if ARCH_EXYNOS > >>>>> > >>>>> However, many developers may want MODULES=y, but not want to bother > >>>>> with an initramfs. So perhaps we need a new symbol > >>>>> MINIMUM_GENERIC_KERNEL or so, protected by EXPERT, and make the > >>>>> driver default to m if that is enabled? > >>>> > >>>> Yeah, that's indeed a problem to solve. For most users (and distros) > >>>> building kernel for Exynos this should be built-in by default. > >>>> > >>>> Anyway, the option is non-selectable so it cannot be converted to "m" or > >>>> disabled. And this is claimed in the commit msg: > >>>> "provide flexibilty for vendors to disable or modularize this driver." > >>>> > >>>> The commit does not achieve it. > >>>> > >>>> Best regards, > >>>> Krzysztof > >>> > >>> Thanks for the reviews! As Lee has explained in his replies, the > >>> intent of this series is to provide config flexibility to create a > >>> defconfig that allows us to move out SoC specific drivers in order to > >>> create a generic kernel that can be used across multiple devices with > >>> different SoCs. > >> > >> That's quite generic statement... or let me put it that way - we already > >> have this ability to create a generic kernel supporting different SoCs. > >> Exynos and other ARMv7 and ARMv8 platforms are multiplatform. > >> > >> Task is done. > > > > multi_v7_defconfig and ARMv8's defconfig are bloated monoliths which > > provide limited flexibility. Good for testing and messing around - > > not much good for real products. > > I am not saying about defconfigs. I am saying that ARMv8 platform is > multiplatform so we already solved the problem Will mentioned. :) > > > > >> Please be more specific about use case and describe what exactly in > >> current upstream multiplatform kernel is missing, what is not > >> multiplatform enough. > > > > The use-case is GKI. A realistic middle-ground between fully open > > source and real-world usage of the Linux kernel in a competitive > > technical arena. GKI aims to be as close to Mainline as possible, > > whilst allowing hardware vendors to supply their own software > > containing their perceived competitive edge and/or supporting > > not-yet-released hardware platforms. > > <grumpy mode> > Therefore the use case is to not contribute anything upstream around > ARCH_EXYNOS but use it in millions of devices downstream with hundreds > of out-of-tree modules. The use case is to make life easy for the vendor > and out-of-tree code, not for the upstream. Instead of promoting > upstreaming, or leaning towards usptream in some balanced way, the use > case is to entirely go to out-of-tree. > > I am not thinking here about edge or not-yet-released platforms but > "ancient" in terms of current SoC business, e.g. 3-5 years old. > </grumpy mode> > > > > > If you end up over-constraining the ability to configure the kernel in > > useful/meaningful ways, that makes one of the main (best intention) > > aims of GKI, (i.e. to have an upstream first ethos in order to be as > > close to upstream as possible) much more difficult. > > GKI encourages core kernel changes to be upstreamed but it is > effectively the nail in the coffin of upstreaming vendor SoC changes. > There is simply no incentive for less-cooperative vendor to upstream > it's modules (except usual benefits like code quality and user support > which are not important for less-cooperative vendors). > > The kernel should be configured mainly towards mainline platforms. Not > the other way around. This of course does not stop it for supporting > out-of-tree code, but I guess you also know that what's out-of-tree, it > does not exist. :) I'm not sure you've thought the above points through. :) How is that any of this different to Mainline? So long as you have the headers for the kernel you wish to compile against, you can create all the new modules you like in both cases. > > I put in a lot of effort to ensure GKI doesn't end up as just another > > fork of the Linux kernel. So far, so good, but flexibility and > > understanding is key. > > > >>> I'm sorry I added confusion by mentioning > >>> modularization. All of these drivers that I am modifying in this > >>> series can be modularized which is an ongoing effort, but is not > >>> addressed here and I don't believe that modularizing them should be a > >>> requirement before supporting enabling/disabling them. > >> > >> Since the disabling the driver for a kernel supporting Exynos does not > >> make any sense, then making it at least modular unfortunately it is a > >> requirement. > > > > I can go with that. > > > >>> I will update the series with my patch that refactors the Samsung SoC > >>> drivers menuconfig to make these visible as well. > >> > >> I would first recommend to really describe your use case because my > >> questions about this are still unanswered. > > > > Hopefully my replies have helped somewhat. > > > > Happy to discuss further if required. > > > > If all else fails, feel free to ping me on IRC (lag). > > Thanks Lee, you described the use case. In general I like it and support > it, except for what I wrote in the other mail. > > Vendor does not contribute much therefore there is no balance in > upstreaming. Since none of other vendor's platforms are supported, I am > looking only at what is supported. From that perspective - the change > proposed by Will and previous guys, does not have much sense. > > My perspective probably would change a lot if vendor did contribute some > of its non-edge platforms (3-5 years old)... especially that unlike few > community guys (e.g. PostmarketOS), vendor has shit-tons of money and > the hardware manuals. :) But no incentive to upstream code old (dead) platforms that they no longer make money from. We're not talking about kind-hearted individuals here. These are business entities. What is the business incentive to put hundreds of thousands of dollars into something with no RoI? > Instead of pushing this change, please let's give some incentive to the > vendor for upstreaming anything. Again, you're being specific. We would also like/need to make the same kinds of changes to other vendor configurations. One's which do contribute significantly at their own cost. The technical reasoning cannot be different because you do or don't like the way the company operates. Try to detach a little from your feelings during discussions which should be purely technical.
On 23/09/2021 16:18, Lee Jones wrote: >> Thanks Lee, you described the use case. In general I like it and support >> it, except for what I wrote in the other mail. >> >> Vendor does not contribute much therefore there is no balance in >> upstreaming. Since none of other vendor's platforms are supported, I am >> looking only at what is supported. From that perspective - the change >> proposed by Will and previous guys, does not have much sense. >> >> My perspective probably would change a lot if vendor did contribute some >> of its non-edge platforms (3-5 years old)... especially that unlike few >> community guys (e.g. PostmarketOS), vendor has shit-tons of money and >> the hardware manuals. :) > > But no incentive to upstream code old (dead) platforms that they no > longer make money from. We're not talking about kind-hearted > individuals here. These are business entities. > > What is the business incentive to put hundreds of thousands of dollars > into something with no RoI? Before you mentioned business entities refrain from upstreaming recent hardware. You question upstreaming not that recent, so basically business entity will claim it has zero incentives working with upstream. Actually there are incentives for both cases - better code quality for the pieces being base for future devices, selling mainline supported hardware to other businesses, eventually less work for themselves around keeping code in sync with mainline. All these are of course difficult to measure from business perspective. > >> Instead of pushing this change, please let's give some incentive to the >> vendor for upstreaming anything. > > Again, you're being specific. We would also like/need to make the > same kinds of changes to other vendor configurations. One's which do > contribute significantly at their own cost. Yes, I am specific because we talk here about specfic Kconfig changes for one specific ARM sub-architecture. Same set of changes can be applied to other SoCs and usually have more sense there because number of upstream platforms is bigger. If you have 10 different pinctrl drivers, you might decide to narrow the defconfigs to subset of it. If you have 2-3, the extra complexity does not matter and you just enable all of them. That's also decision we made few years ago internally in Samsung. > The technical reasoning cannot be different because you do or don't > like the way the company operates. Try to detach a little from > your feelings during discussions which should be purely technical. I mentioned the less-contributing vendor arguments because you said: >> Vendors are not able to upstream all functionality right away That's the side-topic in this discussion. Technically, all supported Exynos platforms require selecting ARCH_EXYNOS and require all drivers selected by ARCH_EXYNOS. If you mention some unsupported out-of-tree platforms, which I cannot audit/see/use, it is not a valid reason to change statement above. Make them supported, available to audit and check and statement above stops being valid. Best regards, Krzysztof
On 23/09/2021 18:27, Krzysztof Kozlowski wrote: > On 23/09/2021 16:18, Lee Jones wrote: >>> Thanks Lee, you described the use case. In general I like it and support >>> it, except for what I wrote in the other mail. >>> >>> Vendor does not contribute much therefore there is no balance in >>> upstreaming. Since none of other vendor's platforms are supported, I am >>> looking only at what is supported. From that perspective - the change >>> proposed by Will and previous guys, does not have much sense. >>> >>> My perspective probably would change a lot if vendor did contribute some >>> of its non-edge platforms (3-5 years old)... especially that unlike few >>> community guys (e.g. PostmarketOS), vendor has shit-tons of money and >>> the hardware manuals. :) >> >> But no incentive to upstream code old (dead) platforms that they no >> longer make money from. We're not talking about kind-hearted >> individuals here. These are business entities. >> >> What is the business incentive to put hundreds of thousands of dollars >> into something with no RoI? > > Before you mentioned business entities refrain from upstreaming recent > hardware. You question upstreaming not that recent, so basically > business entity will claim it has zero incentives working with upstream. Uh, this looks unparseable, I lost some words. Let me write again that part: Before you mentioned business entities refrain from upstreaming recent hardware. You question now upstreaming not that recent hardware, so basically business entity has no incentives to work at all with upstream on any platform. Neither newest, nor slightly older. > Actually there are incentives for both cases - better code quality for > the pieces being base for future devices, selling mainline supported > hardware to other businesses, eventually less work for themselves around > keeping code in sync with mainline. All these are of course difficult to > measure from business perspective. > >> >>> Instead of pushing this change, please let's give some incentive to the >>> vendor for upstreaming anything. >> >> Again, you're being specific. We would also like/need to make the >> same kinds of changes to other vendor configurations. One's which do >> contribute significantly at their own cost. > > Yes, I am specific because we talk here about specfic Kconfig changes > for one specific ARM sub-architecture. Same set of changes can be > applied to other SoCs and usually have more sense there because number > of upstream platforms is bigger. > > If you have 10 different pinctrl drivers, you might decide to narrow the > defconfigs to subset of it. If you have 2-3, the extra complexity does > not matter and you just enable all of them. That's also decision we made > few years ago internally in Samsung. > >> The technical reasoning cannot be different because you do or don't >> like the way the company operates. Try to detach a little from >> your feelings during discussions which should be purely technical. > > I mentioned the less-contributing vendor arguments because you said: > >>> Vendors are not able to upstream all functionality right away > > That's the side-topic in this discussion. > > Technically, all supported Exynos platforms require selecting > ARCH_EXYNOS and require all drivers selected by ARCH_EXYNOS. If you > mention some unsupported out-of-tree platforms, which I cannot > audit/see/use, it is not a valid reason to change statement above. Make > them supported, available to audit and check and statement above stops > being valid. > > Best regards, > Krzysztof > Best regards, Krzysztof
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms index b0ce18d4cc98..3a66ed43088d 100644 --- a/arch/arm64/Kconfig.platforms +++ b/arch/arm64/Kconfig.platforms @@ -91,7 +91,6 @@ config ARCH_BRCMSTB config ARCH_EXYNOS bool "ARMv8 based Samsung Exynos SoC family" - select COMMON_CLK_SAMSUNG select EXYNOS_CHIPID select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS select EXYNOS_PMU diff --git a/drivers/clk/samsung/Kconfig b/drivers/clk/samsung/Kconfig index 0441c4f73ac9..f3e189a06b03 100644 --- a/drivers/clk/samsung/Kconfig +++ b/drivers/clk/samsung/Kconfig @@ -2,6 +2,7 @@ # Recent Exynos platforms should just select COMMON_CLK_SAMSUNG: config COMMON_CLK_SAMSUNG bool "Samsung Exynos clock controller support" if COMPILE_TEST + default y if ARCH_EXYNOS select S3C64XX_COMMON_CLK if ARM && ARCH_S3C64XX select S5PV210_COMMON_CLK if ARM && ARCH_S5PV210 select EXYNOS_3250_COMMON_CLK if ARM && SOC_EXYNOS3250
COMMON_CLK_SAMSUNG is selected by ARCH_EXYNOS which forces this config to be built-in when ARCH_EXYNOS is enabled. Switch the logic to use a "default y if ARCH_EXYNOS" to provide flexibilty for vendors to disable or modularize this driver. I verified the .config is identical with and without this change. Signed-off-by: Will McVicker <willmcvicker@google.com> --- arch/arm64/Kconfig.platforms | 1 - drivers/clk/samsung/Kconfig | 1 + 2 files changed, 1 insertion(+), 1 deletion(-)