* Smarter Kconfig help @ 2019-03-05 17:31 ` Russell King - ARM Linux admin 0 siblings, 0 replies; 24+ messages in thread From: Russell King - ARM Linux admin @ 2019-03-05 17:31 UTC (permalink / raw) To: linux-arm-kernel, linux-kernel Cc: Lucas Stach, Peng Hao, Mark Rutland, Andy Shevchenko, Greg Kroah-Hartman Guys, We need to be smarter when writing Kconfig help. I'm just going through updating my build trees with the results of 5.0 development, and a number of the help texts are next to useless. For example, PVPANIC - is this something that should be enabled for a host or guest kernel? Answer: you have to read the driver code to find out. IMX_IRQSTEER - which i.MX SoCs does this apply to? The help text just says: "Support for the i.MX IRQSTEER interrupt multiplexer/remapper." which doesn't say which SoCs this should be enabled for - it turns out that grepping for the driver's DT compatible string, none of the 32-bit ARM cores have support for this, yet we still default it to enabled there. It seems the help text should at the very least tell the user that this is not applicable to i.MX SoCs with 32-bit ARM cores. I'm sure there's many other instances of this... I suspect that it's caused by review concentrating mostly on the technical aspects of the code and the Kconfig help text just gets forgotten about. Can we at least improve these two options please? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ^ permalink raw reply [flat|nested] 24+ messages in thread
* Smarter Kconfig help @ 2019-03-05 17:31 ` Russell King - ARM Linux admin 0 siblings, 0 replies; 24+ messages in thread From: Russell King - ARM Linux admin @ 2019-03-05 17:31 UTC (permalink / raw) To: linux-arm-kernel, linux-kernel Cc: Mark Rutland, Greg Kroah-Hartman, Andy Shevchenko, Peng Hao, Lucas Stach Guys, We need to be smarter when writing Kconfig help. I'm just going through updating my build trees with the results of 5.0 development, and a number of the help texts are next to useless. For example, PVPANIC - is this something that should be enabled for a host or guest kernel? Answer: you have to read the driver code to find out. IMX_IRQSTEER - which i.MX SoCs does this apply to? The help text just says: "Support for the i.MX IRQSTEER interrupt multiplexer/remapper." which doesn't say which SoCs this should be enabled for - it turns out that grepping for the driver's DT compatible string, none of the 32-bit ARM cores have support for this, yet we still default it to enabled there. It seems the help text should at the very least tell the user that this is not applicable to i.MX SoCs with 32-bit ARM cores. I'm sure there's many other instances of this... I suspect that it's caused by review concentrating mostly on the technical aspects of the code and the Kconfig help text just gets forgotten about. Can we at least improve these two options please? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help 2019-03-05 17:31 ` Russell King - ARM Linux admin @ 2019-03-06 9:35 ` Mark Rutland -1 siblings, 0 replies; 24+ messages in thread From: Mark Rutland @ 2019-03-06 9:35 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: linux-arm-kernel, linux-kernel, Lucas Stach, Peng Hao, Andy Shevchenko, Greg Kroah-Hartman On Tue, Mar 05, 2019 at 05:31:12PM +0000, Russell King - ARM Linux admin wrote: > Guys, Hi Russell, > We need to be smarter when writing Kconfig help. I'm just going > through updating my build trees with the results of 5.0 development, > and a number of the help texts are next to useless. For example, > > PVPANIC - is this something that should be enabled for a host or > guest kernel? Answer: you have to read the driver code to find out. When I looked at the help text: This driver provides support for the pvpanic device. pvpanic is a paravirtualized device provided by QEMU; it lets a virtual machine (guest) communicate panic events to the host. ... it seemed clear to me that this was for a guest, given the text says QEMU provides the device. I guess you read that as meaning QEMU asks the host kernel to provide the device to the guest? Do you have a suggestion for how to word that unambiguously? > IMX_IRQSTEER - which i.MX SoCs does this apply to? The help text > just says: > > "Support for the i.MX IRQSTEER interrupt multiplexer/remapper." > > which doesn't say which SoCs this should be enabled for - it turns > out that grepping for the driver's DT compatible string, none of > the 32-bit ARM cores have support for this, yet we still default > it to enabled there. It seems the help text should at the very > least tell the user that this is not applicable to i.MX SoCs with > 32-bit ARM cores. > > I'm sure there's many other instances of this... I suspect that > it's caused by review concentrating mostly on the technical aspects > of the code and the Kconfig help text just gets forgotten about. Just to be clear, in general what you want is for Kconfig help to be clearer about *when* an option is relevant, right? I'll try to bear that in mind when reviewing in future. Thanks, Mark. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help @ 2019-03-06 9:35 ` Mark Rutland 0 siblings, 0 replies; 24+ messages in thread From: Mark Rutland @ 2019-03-06 9:35 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: Peng Hao, Greg Kroah-Hartman, linux-kernel, Andy Shevchenko, linux-arm-kernel, Lucas Stach On Tue, Mar 05, 2019 at 05:31:12PM +0000, Russell King - ARM Linux admin wrote: > Guys, Hi Russell, > We need to be smarter when writing Kconfig help. I'm just going > through updating my build trees with the results of 5.0 development, > and a number of the help texts are next to useless. For example, > > PVPANIC - is this something that should be enabled for a host or > guest kernel? Answer: you have to read the driver code to find out. When I looked at the help text: This driver provides support for the pvpanic device. pvpanic is a paravirtualized device provided by QEMU; it lets a virtual machine (guest) communicate panic events to the host. ... it seemed clear to me that this was for a guest, given the text says QEMU provides the device. I guess you read that as meaning QEMU asks the host kernel to provide the device to the guest? Do you have a suggestion for how to word that unambiguously? > IMX_IRQSTEER - which i.MX SoCs does this apply to? The help text > just says: > > "Support for the i.MX IRQSTEER interrupt multiplexer/remapper." > > which doesn't say which SoCs this should be enabled for - it turns > out that grepping for the driver's DT compatible string, none of > the 32-bit ARM cores have support for this, yet we still default > it to enabled there. It seems the help text should at the very > least tell the user that this is not applicable to i.MX SoCs with > 32-bit ARM cores. > > I'm sure there's many other instances of this... I suspect that > it's caused by review concentrating mostly on the technical aspects > of the code and the Kconfig help text just gets forgotten about. Just to be clear, in general what you want is for Kconfig help to be clearer about *when* an option is relevant, right? I'll try to bear that in mind when reviewing in future. Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help 2019-03-06 9:35 ` Mark Rutland @ 2019-03-06 12:50 ` Russell King - ARM Linux admin -1 siblings, 0 replies; 24+ messages in thread From: Russell King - ARM Linux admin @ 2019-03-06 12:50 UTC (permalink / raw) To: Mark Rutland Cc: linux-arm-kernel, linux-kernel, Lucas Stach, Peng Hao, Andy Shevchenko, Greg Kroah-Hartman Hi Mark, On Wed, Mar 06, 2019 at 09:35:21AM +0000, Mark Rutland wrote: > On Tue, Mar 05, 2019 at 05:31:12PM +0000, Russell King - ARM Linux admin wrote: > > Guys, > > Hi Russell, > > > We need to be smarter when writing Kconfig help. I'm just going > > through updating my build trees with the results of 5.0 development, > > and a number of the help texts are next to useless. For example, > > > > PVPANIC - is this something that should be enabled for a host or > > guest kernel? Answer: you have to read the driver code to find out. > > When I looked at the help text: > > This driver provides support for the pvpanic device. pvpanic is > a paravirtualized device provided by QEMU; it lets a virtual machine > (guest) communicate panic events to the host. > > ... it seemed clear to me that this was for a guest, given the text says > QEMU provides the device. I guess you read that as meaning QEMU asks the > host kernel to provide the device to the guest? Yes - that's exactly where the confusion was. It could be something like a tap network device to allow a guest access to a facility on the host, or it could be something that the guest kernel uses to communicate with its host environment. > Do you have a suggestion for how to word that unambiguously? It used to be normal to include a sugestion in the help text that guided when an option should be enabled. So, adding something like: "Say Y here if you are building a kernel for a virtual machine." would make it clear. This used to be standard throughout the kernel, but it seems in recent years, this has been omitted. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help @ 2019-03-06 12:50 ` Russell King - ARM Linux admin 0 siblings, 0 replies; 24+ messages in thread From: Russell King - ARM Linux admin @ 2019-03-06 12:50 UTC (permalink / raw) To: Mark Rutland Cc: Peng Hao, Greg Kroah-Hartman, linux-kernel, Andy Shevchenko, linux-arm-kernel, Lucas Stach Hi Mark, On Wed, Mar 06, 2019 at 09:35:21AM +0000, Mark Rutland wrote: > On Tue, Mar 05, 2019 at 05:31:12PM +0000, Russell King - ARM Linux admin wrote: > > Guys, > > Hi Russell, > > > We need to be smarter when writing Kconfig help. I'm just going > > through updating my build trees with the results of 5.0 development, > > and a number of the help texts are next to useless. For example, > > > > PVPANIC - is this something that should be enabled for a host or > > guest kernel? Answer: you have to read the driver code to find out. > > When I looked at the help text: > > This driver provides support for the pvpanic device. pvpanic is > a paravirtualized device provided by QEMU; it lets a virtual machine > (guest) communicate panic events to the host. > > ... it seemed clear to me that this was for a guest, given the text says > QEMU provides the device. I guess you read that as meaning QEMU asks the > host kernel to provide the device to the guest? Yes - that's exactly where the confusion was. It could be something like a tap network device to allow a guest access to a facility on the host, or it could be something that the guest kernel uses to communicate with its host environment. > Do you have a suggestion for how to word that unambiguously? It used to be normal to include a sugestion in the help text that guided when an option should be enabled. So, adding something like: "Say Y here if you are building a kernel for a virtual machine." would make it clear. This used to be standard throughout the kernel, but it seems in recent years, this has been omitted. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help 2019-03-05 17:31 ` Russell King - ARM Linux admin @ 2019-03-06 9:45 ` Lucas Stach -1 siblings, 0 replies; 24+ messages in thread From: Lucas Stach @ 2019-03-06 9:45 UTC (permalink / raw) To: Russell King - ARM Linux admin, linux-arm-kernel, linux-kernel Cc: Peng Hao, Mark Rutland, Andy Shevchenko, Greg Kroah-Hartman Hi Russell, Am Dienstag, den 05.03.2019, 17:31 +0000 schrieb Russell King - ARM Linux admin: > Guys, > > We need to be smarter when writing Kconfig help. I'm just going > through updating my build trees with the results of 5.0 development, > and a number of the help texts are next to useless. For example, > > PVPANIC - is this something that should be enabled for a host or > guest kernel? Answer: you have to read the driver code to find out. > > IMX_IRQSTEER - which i.MX SoCs does this apply to? The help text > just says: > > "Support for the i.MX IRQSTEER interrupt multiplexer/remapper." > > which doesn't say which SoCs this should be enabled for - it turns > out that grepping for the driver's DT compatible string, none of > the 32-bit ARM cores have support for this, yet we still default > it to enabled there. It seems the help text should at the very > least tell the user that this is not applicable to i.MX SoCs with > 32-bit ARM cores. > > I'm sure there's many other instances of this... I suspect that > it's caused by review concentrating mostly on the technical aspects > of the code and the Kconfig help text just gets forgotten about. > > Can we at least improve these two options please? While I totally agree that the irqsteer driver should only default Y on 64bit i.MX SoCs, I'm not really sure what to do about the Kconfig help text. Enumerating SoCs in the Kconfig of a IP block driver is always prone to get outdated. Just as the first example I've been able to come up with, the MMC_SDHCI_ESDHC_IMX help text says: "This selects the Freescale eSDHC/uSDHC controller support found on i.MX25, i.MX35 i.MX5x and i.MX6x.", while the same IP block is to be found on i.MX7 and various i.MX8. For the Kconfig user, who needs to decide if an option is relevant, outdated SoC information is probably just as bad as no information at all. Regards, Lucas ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help @ 2019-03-06 9:45 ` Lucas Stach 0 siblings, 0 replies; 24+ messages in thread From: Lucas Stach @ 2019-03-06 9:45 UTC (permalink / raw) To: Russell King - ARM Linux admin, linux-arm-kernel, linux-kernel Cc: Mark Rutland, Greg Kroah-Hartman, Andy Shevchenko, Peng Hao Hi Russell, Am Dienstag, den 05.03.2019, 17:31 +0000 schrieb Russell King - ARM Linux admin: > Guys, > > We need to be smarter when writing Kconfig help. I'm just going > through updating my build trees with the results of 5.0 development, > and a number of the help texts are next to useless. For example, > > PVPANIC - is this something that should be enabled for a host or > guest kernel? Answer: you have to read the driver code to find out. > > IMX_IRQSTEER - which i.MX SoCs does this apply to? The help text > just says: > > "Support for the i.MX IRQSTEER interrupt multiplexer/remapper." > > which doesn't say which SoCs this should be enabled for - it turns > out that grepping for the driver's DT compatible string, none of > the 32-bit ARM cores have support for this, yet we still default > it to enabled there. It seems the help text should at the very > least tell the user that this is not applicable to i.MX SoCs with > 32-bit ARM cores. > > I'm sure there's many other instances of this... I suspect that > it's caused by review concentrating mostly on the technical aspects > of the code and the Kconfig help text just gets forgotten about. > > Can we at least improve these two options please? While I totally agree that the irqsteer driver should only default Y on 64bit i.MX SoCs, I'm not really sure what to do about the Kconfig help text. Enumerating SoCs in the Kconfig of a IP block driver is always prone to get outdated. Just as the first example I've been able to come up with, the MMC_SDHCI_ESDHC_IMX help text says: "This selects the Freescale eSDHC/uSDHC controller support found on i.MX25, i.MX35 i.MX5x and i.MX6x.", while the same IP block is to be found on i.MX7 and various i.MX8. For the Kconfig user, who needs to decide if an option is relevant, outdated SoC information is probably just as bad as no information at all. Regards, Lucas _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help 2019-03-06 9:45 ` Lucas Stach @ 2019-03-06 9:51 ` Russell King - ARM Linux admin -1 siblings, 0 replies; 24+ messages in thread From: Russell King - ARM Linux admin @ 2019-03-06 9:51 UTC (permalink / raw) To: Lucas Stach Cc: linux-arm-kernel, linux-kernel, Mark Rutland, Greg Kroah-Hartman, Andy Shevchenko, Peng Hao On Wed, Mar 06, 2019 at 10:45:52AM +0100, Lucas Stach wrote: > Hi Russell, > > Am Dienstag, den 05.03.2019, 17:31 +0000 schrieb Russell King - ARM Linux admin: > > Guys, > > > > We need to be smarter when writing Kconfig help. I'm just going > > through updating my build trees with the results of 5.0 development, > > and a number of the help texts are next to useless. For example, > > > > PVPANIC - is this something that should be enabled for a host or > > guest kernel? Answer: you have to read the driver code to find out. > > > > IMX_IRQSTEER - which i.MX SoCs does this apply to? The help text > > just says: > > > > "Support for the i.MX IRQSTEER interrupt multiplexer/remapper." > > > > which doesn't say which SoCs this should be enabled for - it turns > > out that grepping for the driver's DT compatible string, none of > > the 32-bit ARM cores have support for this, yet we still default > > it to enabled there. It seems the help text should at the very > > least tell the user that this is not applicable to i.MX SoCs with > > 32-bit ARM cores. > > > > I'm sure there's many other instances of this... I suspect that > > it's caused by review concentrating mostly on the technical aspects > > of the code and the Kconfig help text just gets forgotten about. > > > > Can we at least improve these two options please? > > While I totally agree that the irqsteer driver should only default Y on > 64bit i.MX SoCs, I'm not really sure what to do about the Kconfig help > text. > > Enumerating SoCs in the Kconfig of a IP block driver is always prone to > get outdated. Just as the first example I've been able to come up with, > the MMC_SDHCI_ESDHC_IMX help text says: "This selects the Freescale > eSDHC/uSDHC controller support found on i.MX25, i.MX35 i.MX5x and > i.MX6x.", while the same IP block is to be found on i.MX7 and various > i.MX8. > > For the Kconfig user, who needs to decide if an option is relevant, > outdated SoC information is probably just as bad as no information at > all. How about "This option does not apply to AArch32 based SoCs" or "This option should be enabled for i.MX7 and later SoCs" ? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help @ 2019-03-06 9:51 ` Russell King - ARM Linux admin 0 siblings, 0 replies; 24+ messages in thread From: Russell King - ARM Linux admin @ 2019-03-06 9:51 UTC (permalink / raw) To: Lucas Stach Cc: Mark Rutland, Peng Hao, Greg Kroah-Hartman, linux-kernel, Andy Shevchenko, linux-arm-kernel On Wed, Mar 06, 2019 at 10:45:52AM +0100, Lucas Stach wrote: > Hi Russell, > > Am Dienstag, den 05.03.2019, 17:31 +0000 schrieb Russell King - ARM Linux admin: > > Guys, > > > > We need to be smarter when writing Kconfig help. I'm just going > > through updating my build trees with the results of 5.0 development, > > and a number of the help texts are next to useless. For example, > > > > PVPANIC - is this something that should be enabled for a host or > > guest kernel? Answer: you have to read the driver code to find out. > > > > IMX_IRQSTEER - which i.MX SoCs does this apply to? The help text > > just says: > > > > "Support for the i.MX IRQSTEER interrupt multiplexer/remapper." > > > > which doesn't say which SoCs this should be enabled for - it turns > > out that grepping for the driver's DT compatible string, none of > > the 32-bit ARM cores have support for this, yet we still default > > it to enabled there. It seems the help text should at the very > > least tell the user that this is not applicable to i.MX SoCs with > > 32-bit ARM cores. > > > > I'm sure there's many other instances of this... I suspect that > > it's caused by review concentrating mostly on the technical aspects > > of the code and the Kconfig help text just gets forgotten about. > > > > Can we at least improve these two options please? > > While I totally agree that the irqsteer driver should only default Y on > 64bit i.MX SoCs, I'm not really sure what to do about the Kconfig help > text. > > Enumerating SoCs in the Kconfig of a IP block driver is always prone to > get outdated. Just as the first example I've been able to come up with, > the MMC_SDHCI_ESDHC_IMX help text says: "This selects the Freescale > eSDHC/uSDHC controller support found on i.MX25, i.MX35 i.MX5x and > i.MX6x.", while the same IP block is to be found on i.MX7 and various > i.MX8. > > For the Kconfig user, who needs to decide if an option is relevant, > outdated SoC information is probably just as bad as no information at > all. How about "This option does not apply to AArch32 based SoCs" or "This option should be enabled for i.MX7 and later SoCs" ? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help 2019-03-06 9:51 ` Russell King - ARM Linux admin @ 2019-03-06 10:49 ` Andy Shevchenko -1 siblings, 0 replies; 24+ messages in thread From: Andy Shevchenko @ 2019-03-06 10:49 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: Lucas Stach, linux-arm Mailing List, Linux Kernel Mailing List, Mark Rutland, Greg Kroah-Hartman, Peng Hao On Wed, Mar 6, 2019 at 11:52 AM Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: > On Wed, Mar 06, 2019 at 10:45:52AM +0100, Lucas Stach wrote: > > Am Dienstag, den 05.03.2019, 17:31 +0000 schrieb Russell King - ARM Linux admin: > > While I totally agree that the irqsteer driver should only default Y on > > 64bit i.MX SoCs, I'm not really sure what to do about the Kconfig help > > text. > > > > Enumerating SoCs in the Kconfig of a IP block driver is always prone to > > get outdated. Just as the first example I've been able to come up with, > > the MMC_SDHCI_ESDHC_IMX help text says: "This selects the Freescale > > eSDHC/uSDHC controller support found on i.MX25, i.MX35 i.MX5x and > > i.MX6x.", while the same IP block is to be found on i.MX7 and various > > i.MX8. > > > > For the Kconfig user, who needs to decide if an option is relevant, > > outdated SoC information is probably just as bad as no information at > > all. > > How about "This option does not apply to AArch32 based SoCs" or Negative is usually error prone. > "This option should be enabled for i.MX7 and later SoCs" ? Why it should be? It can / could be I guess. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help @ 2019-03-06 10:49 ` Andy Shevchenko 0 siblings, 0 replies; 24+ messages in thread From: Andy Shevchenko @ 2019-03-06 10:49 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: Mark Rutland, Peng Hao, Greg Kroah-Hartman, Linux Kernel Mailing List, linux-arm Mailing List, Lucas Stach On Wed, Mar 6, 2019 at 11:52 AM Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: > On Wed, Mar 06, 2019 at 10:45:52AM +0100, Lucas Stach wrote: > > Am Dienstag, den 05.03.2019, 17:31 +0000 schrieb Russell King - ARM Linux admin: > > While I totally agree that the irqsteer driver should only default Y on > > 64bit i.MX SoCs, I'm not really sure what to do about the Kconfig help > > text. > > > > Enumerating SoCs in the Kconfig of a IP block driver is always prone to > > get outdated. Just as the first example I've been able to come up with, > > the MMC_SDHCI_ESDHC_IMX help text says: "This selects the Freescale > > eSDHC/uSDHC controller support found on i.MX25, i.MX35 i.MX5x and > > i.MX6x.", while the same IP block is to be found on i.MX7 and various > > i.MX8. > > > > For the Kconfig user, who needs to decide if an option is relevant, > > outdated SoC information is probably just as bad as no information at > > all. > > How about "This option does not apply to AArch32 based SoCs" or Negative is usually error prone. > "This option should be enabled for i.MX7 and later SoCs" ? Why it should be? It can / could be I guess. -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help 2019-03-06 10:49 ` Andy Shevchenko @ 2019-03-06 11:34 ` Russell King - ARM Linux admin -1 siblings, 0 replies; 24+ messages in thread From: Russell King - ARM Linux admin @ 2019-03-06 11:34 UTC (permalink / raw) To: Andy Shevchenko Cc: Lucas Stach, linux-arm Mailing List, Linux Kernel Mailing List, Mark Rutland, Greg Kroah-Hartman, Peng Hao On Wed, Mar 06, 2019 at 12:49:40PM +0200, Andy Shevchenko wrote: > On Wed, Mar 6, 2019 at 11:52 AM Russell King - ARM Linux admin > <linux@armlinux.org.uk> wrote: > > On Wed, Mar 06, 2019 at 10:45:52AM +0100, Lucas Stach wrote: > > > Am Dienstag, den 05.03.2019, 17:31 +0000 schrieb Russell King - ARM Linux admin: > > > > While I totally agree that the irqsteer driver should only default Y on > > > 64bit i.MX SoCs, I'm not really sure what to do about the Kconfig help > > > text. > > > > > > Enumerating SoCs in the Kconfig of a IP block driver is always prone to > > > get outdated. Just as the first example I've been able to come up with, > > > the MMC_SDHCI_ESDHC_IMX help text says: "This selects the Freescale > > > eSDHC/uSDHC controller support found on i.MX25, i.MX35 i.MX5x and > > > i.MX6x.", while the same IP block is to be found on i.MX7 and various > > > i.MX8. > > > > > > For the Kconfig user, who needs to decide if an option is relevant, > > > outdated SoC information is probably just as bad as no information at > > > all. > > > > How about "This option does not apply to AArch32 based SoCs" or > > Negative is usually error prone. > > > "This option should be enabled for i.MX7 and later SoCs" ? > > Why it should be? It can / could be I guess. I've no idea. I'm just making suggestions - if you have anything better, let's hear it. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help @ 2019-03-06 11:34 ` Russell King - ARM Linux admin 0 siblings, 0 replies; 24+ messages in thread From: Russell King - ARM Linux admin @ 2019-03-06 11:34 UTC (permalink / raw) To: Andy Shevchenko Cc: Mark Rutland, Peng Hao, Greg Kroah-Hartman, Linux Kernel Mailing List, linux-arm Mailing List, Lucas Stach On Wed, Mar 06, 2019 at 12:49:40PM +0200, Andy Shevchenko wrote: > On Wed, Mar 6, 2019 at 11:52 AM Russell King - ARM Linux admin > <linux@armlinux.org.uk> wrote: > > On Wed, Mar 06, 2019 at 10:45:52AM +0100, Lucas Stach wrote: > > > Am Dienstag, den 05.03.2019, 17:31 +0000 schrieb Russell King - ARM Linux admin: > > > > While I totally agree that the irqsteer driver should only default Y on > > > 64bit i.MX SoCs, I'm not really sure what to do about the Kconfig help > > > text. > > > > > > Enumerating SoCs in the Kconfig of a IP block driver is always prone to > > > get outdated. Just as the first example I've been able to come up with, > > > the MMC_SDHCI_ESDHC_IMX help text says: "This selects the Freescale > > > eSDHC/uSDHC controller support found on i.MX25, i.MX35 i.MX5x and > > > i.MX6x.", while the same IP block is to be found on i.MX7 and various > > > i.MX8. > > > > > > For the Kconfig user, who needs to decide if an option is relevant, > > > outdated SoC information is probably just as bad as no information at > > > all. > > > > How about "This option does not apply to AArch32 based SoCs" or > > Negative is usually error prone. > > > "This option should be enabled for i.MX7 and later SoCs" ? > > Why it should be? It can / could be I guess. I've no idea. I'm just making suggestions - if you have anything better, let's hear it. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help 2019-03-06 11:34 ` Russell King - ARM Linux admin @ 2019-03-06 12:42 ` Russell King - ARM Linux admin -1 siblings, 0 replies; 24+ messages in thread From: Russell King - ARM Linux admin @ 2019-03-06 12:42 UTC (permalink / raw) To: Andy Shevchenko Cc: Mark Rutland, Peng Hao, Greg Kroah-Hartman, Linux Kernel Mailing List, linux-arm Mailing List, Lucas Stach On Wed, Mar 06, 2019 at 11:34:36AM +0000, Russell King - ARM Linux admin wrote: > On Wed, Mar 06, 2019 at 12:49:40PM +0200, Andy Shevchenko wrote: > > On Wed, Mar 6, 2019 at 11:52 AM Russell King - ARM Linux admin > > <linux@armlinux.org.uk> wrote: > > > On Wed, Mar 06, 2019 at 10:45:52AM +0100, Lucas Stach wrote: > > > > Am Dienstag, den 05.03.2019, 17:31 +0000 schrieb Russell King - ARM Linux admin: > > > > > > While I totally agree that the irqsteer driver should only default Y on > > > > 64bit i.MX SoCs, I'm not really sure what to do about the Kconfig help > > > > text. > > > > > > > > Enumerating SoCs in the Kconfig of a IP block driver is always prone to > > > > get outdated. Just as the first example I've been able to come up with, > > > > the MMC_SDHCI_ESDHC_IMX help text says: "This selects the Freescale > > > > eSDHC/uSDHC controller support found on i.MX25, i.MX35 i.MX5x and > > > > i.MX6x.", while the same IP block is to be found on i.MX7 and various > > > > i.MX8. > > > > > > > > For the Kconfig user, who needs to decide if an option is relevant, > > > > outdated SoC information is probably just as bad as no information at > > > > all. > > > > > > How about "This option does not apply to AArch32 based SoCs" or > > > > Negative is usually error prone. > > > > > "This option should be enabled for i.MX7 and later SoCs" ? > > > > Why it should be? It can / could be I guess. > > I've no idea. I'm just making suggestions - if you have anything > better, let's hear it. In case it isn't clear, this is the *exact* point here - I don't know whether this option should be enabled for iMX6 or not, and the only way I found out was to grep the dts files in arch/arm/boot/dts for the driver's compatible string. What that reveals is that *no* 32-bit dts files contain the compatible string, and so I summise that *no* 32-bit iMX SoC should have this driver enabled. Given that I don't know for certain, _I_ can't write a proper help text for this - I can only make suggestions which _might_ fit what the reality actually is. The excuse that "we can't list the explicit SoCs" to me seems to be a very lame excuse for a poor, one line help text that says absolutely nothing that can't already be gathered from the option line itself. So the current help text might as well be deleted - it's that "useful". The best that I can come up with right now, given what little I know from grepping the 32-bit DTS files, is that the help text should at least indicate that it *isn't* applicable to 32-bit SoCs, or if you prefer, *is* applicable to 64-bit SoCs. Beyond that, I have no information to formulate a better suggestion. This is precisely why I say that we need to be smarter about Kconfig help text: if it gives no useful information, it might as well not even exist, and we might as well be honest and have Kconfig say that there is no help available for the option. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help @ 2019-03-06 12:42 ` Russell King - ARM Linux admin 0 siblings, 0 replies; 24+ messages in thread From: Russell King - ARM Linux admin @ 2019-03-06 12:42 UTC (permalink / raw) To: Andy Shevchenko Cc: Mark Rutland, Peng Hao, Greg Kroah-Hartman, Linux Kernel Mailing List, linux-arm Mailing List, Lucas Stach On Wed, Mar 06, 2019 at 11:34:36AM +0000, Russell King - ARM Linux admin wrote: > On Wed, Mar 06, 2019 at 12:49:40PM +0200, Andy Shevchenko wrote: > > On Wed, Mar 6, 2019 at 11:52 AM Russell King - ARM Linux admin > > <linux@armlinux.org.uk> wrote: > > > On Wed, Mar 06, 2019 at 10:45:52AM +0100, Lucas Stach wrote: > > > > Am Dienstag, den 05.03.2019, 17:31 +0000 schrieb Russell King - ARM Linux admin: > > > > > > While I totally agree that the irqsteer driver should only default Y on > > > > 64bit i.MX SoCs, I'm not really sure what to do about the Kconfig help > > > > text. > > > > > > > > Enumerating SoCs in the Kconfig of a IP block driver is always prone to > > > > get outdated. Just as the first example I've been able to come up with, > > > > the MMC_SDHCI_ESDHC_IMX help text says: "This selects the Freescale > > > > eSDHC/uSDHC controller support found on i.MX25, i.MX35 i.MX5x and > > > > i.MX6x.", while the same IP block is to be found on i.MX7 and various > > > > i.MX8. > > > > > > > > For the Kconfig user, who needs to decide if an option is relevant, > > > > outdated SoC information is probably just as bad as no information at > > > > all. > > > > > > How about "This option does not apply to AArch32 based SoCs" or > > > > Negative is usually error prone. > > > > > "This option should be enabled for i.MX7 and later SoCs" ? > > > > Why it should be? It can / could be I guess. > > I've no idea. I'm just making suggestions - if you have anything > better, let's hear it. In case it isn't clear, this is the *exact* point here - I don't know whether this option should be enabled for iMX6 or not, and the only way I found out was to grep the dts files in arch/arm/boot/dts for the driver's compatible string. What that reveals is that *no* 32-bit dts files contain the compatible string, and so I summise that *no* 32-bit iMX SoC should have this driver enabled. Given that I don't know for certain, _I_ can't write a proper help text for this - I can only make suggestions which _might_ fit what the reality actually is. The excuse that "we can't list the explicit SoCs" to me seems to be a very lame excuse for a poor, one line help text that says absolutely nothing that can't already be gathered from the option line itself. So the current help text might as well be deleted - it's that "useful". The best that I can come up with right now, given what little I know from grepping the 32-bit DTS files, is that the help text should at least indicate that it *isn't* applicable to 32-bit SoCs, or if you prefer, *is* applicable to 64-bit SoCs. Beyond that, I have no information to formulate a better suggestion. This is precisely why I say that we need to be smarter about Kconfig help text: if it gives no useful information, it might as well not even exist, and we might as well be honest and have Kconfig say that there is no help available for the option. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help 2019-03-06 12:42 ` Russell King - ARM Linux admin @ 2019-03-06 20:16 ` Enrico Weigelt, metux IT consult -1 siblings, 0 replies; 24+ messages in thread From: Enrico Weigelt, metux IT consult @ 2019-03-06 20:16 UTC (permalink / raw) To: Russell King - ARM Linux admin, Andy Shevchenko Cc: Mark Rutland, Peng Hao, Greg Kroah-Hartman, Linux Kernel Mailing List, linux-arm Mailing List, Lucas Stach On 06.03.19 13:42, Russell King - ARM Linux admin wrote: > In case it isn't clear, this is the *exact* point here - I don't know> whether this option should be enabled for iMX6 or not, and the only> way I found out was to grep the dts files in arch/arm/boot/dts for> the driver's compatible string. What that reveals is that *no* 32-bit> dts files contain the compatible string, and so I summise that *no*> 32-bit iMX SoC should have this driver enabled. The problem is a bit more generic. I often have to spend lots of time to find out which configs to enable on a specific board, to get certain features (eg. network, sata, display, gpu, ...). Even worse: many options require other stuff enabled before even showing up. And when disabling unneeded stuff, it leaves lots of other things enabled. (we don't have some `apt autoremove` kconfig counterpart :(). I've decided to cope w/ that on a higher level and written a little config generator tool for that - here you can enable high level features (eg. 'network' or 'display', etc) and it will generate the actual .config: https://github.com/metux/kmct > The excuse that "we can't list the explicit SoCs" to me seems to be> a very lame excuse Maybe this actually means "nobody here volounteered to actually maintain these help texts" ? > The best that I can come up with right now, given what little I know> from grepping the 32-bit DTS files, is that the help text should at> least indicate that it *isn't* applicable to 32-bit SoCs, or if you> prefer, *is* applicable to 64-bit SoCs. Beyond that, I have no> information to formulate a better suggestion. Perhaps just fix the text based on your knowledge and send a patch to the maintainers. They'll propably tell you if it's incorrect. --mtx -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help @ 2019-03-06 20:16 ` Enrico Weigelt, metux IT consult 0 siblings, 0 replies; 24+ messages in thread From: Enrico Weigelt, metux IT consult @ 2019-03-06 20:16 UTC (permalink / raw) To: Russell King - ARM Linux admin, Andy Shevchenko Cc: Mark Rutland, Peng Hao, Greg Kroah-Hartman, Linux Kernel Mailing List, linux-arm Mailing List, Lucas Stach On 06.03.19 13:42, Russell King - ARM Linux admin wrote: > In case it isn't clear, this is the *exact* point here - I don't know> whether this option should be enabled for iMX6 or not, and the only> way I found out was to grep the dts files in arch/arm/boot/dts for> the driver's compatible string. What that reveals is that *no* 32-bit> dts files contain the compatible string, and so I summise that *no*> 32-bit iMX SoC should have this driver enabled. The problem is a bit more generic. I often have to spend lots of time to find out which configs to enable on a specific board, to get certain features (eg. network, sata, display, gpu, ...). Even worse: many options require other stuff enabled before even showing up. And when disabling unneeded stuff, it leaves lots of other things enabled. (we don't have some `apt autoremove` kconfig counterpart :(). I've decided to cope w/ that on a higher level and written a little config generator tool for that - here you can enable high level features (eg. 'network' or 'display', etc) and it will generate the actual .config: https://github.com/metux/kmct > The excuse that "we can't list the explicit SoCs" to me seems to be> a very lame excuse Maybe this actually means "nobody here volounteered to actually maintain these help texts" ? > The best that I can come up with right now, given what little I know> from grepping the 32-bit DTS files, is that the help text should at> least indicate that it *isn't* applicable to 32-bit SoCs, or if you> prefer, *is* applicable to 64-bit SoCs. Beyond that, I have no> information to formulate a better suggestion. Perhaps just fix the text based on your knowledge and send a patch to the maintainers. They'll propably tell you if it's incorrect. --mtx -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help 2019-03-06 20:16 ` Enrico Weigelt, metux IT consult @ 2019-03-06 20:22 ` Russell King - ARM Linux admin -1 siblings, 0 replies; 24+ messages in thread From: Russell King - ARM Linux admin @ 2019-03-06 20:22 UTC (permalink / raw) To: Enrico Weigelt, metux IT consult Cc: Andy Shevchenko, Mark Rutland, Peng Hao, Greg Kroah-Hartman, Linux Kernel Mailing List, linux-arm Mailing List, Lucas Stach On Wed, Mar 06, 2019 at 09:16:02PM +0100, Enrico Weigelt, metux IT consult wrote: > On 06.03.19 13:42, Russell King - ARM Linux admin wrote: > > > In case it isn't clear, this is the *exact* point here - I don't know> whether this option should be enabled for iMX6 or not, and the only> > way I found out was to grep the dts files in arch/arm/boot/dts for> the > driver's compatible string. What that reveals is that *no* 32-bit> dts > files contain the compatible string, and so I summise that *no*> 32-bit > iMX SoC should have this driver enabled. > The problem is a bit more generic. I often have to spend lots of time > to find out which configs to enable on a specific board, to get certain > features (eg. network, sata, display, gpu, ...). Even worse: many > options require other stuff enabled before even showing up. And when > disabling unneeded stuff, it leaves lots of other things enabled. > (we don't have some `apt autoremove` kconfig counterpart :(). > > I've decided to cope w/ that on a higher level and written a little > config generator tool for that - here you can enable high level > features (eg. 'network' or 'display', etc) and it will generate the > actual .config: > > https://github.com/metux/kmct > > > The excuse that "we can't list the explicit SoCs" to me seems to be> a very lame excuse > > Maybe this actually means "nobody here volounteered to actually maintain > these help texts" ? > > > The best that I can come up with right now, given what little I know> from grepping the 32-bit DTS files, is that the help text should at> > least indicate that it *isn't* applicable to 32-bit SoCs, or if you> > prefer, *is* applicable to 64-bit SoCs. Beyond that, I have no> > information to formulate a better suggestion. > Perhaps just fix the text based on your knowledge and send a patch to > the maintainers. They'll propably tell you if it's incorrect. Frankly, no. I don't want to be going round endlessly writing help texts. We need the effort to be properly distributed - we need those who _know_ the feature they're developing to write a proper help text. One way to achieve that is to make a proper informative help text a pre-requisit of accepting the code. The quality of the help text is just as important as the quality of the code, and we really should be paying the same amount of attention to both. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help @ 2019-03-06 20:22 ` Russell King - ARM Linux admin 0 siblings, 0 replies; 24+ messages in thread From: Russell King - ARM Linux admin @ 2019-03-06 20:22 UTC (permalink / raw) To: Enrico Weigelt, metux IT consult Cc: Mark Rutland, Peng Hao, Greg Kroah-Hartman, Linux Kernel Mailing List, Andy Shevchenko, linux-arm Mailing List, Lucas Stach On Wed, Mar 06, 2019 at 09:16:02PM +0100, Enrico Weigelt, metux IT consult wrote: > On 06.03.19 13:42, Russell King - ARM Linux admin wrote: > > > In case it isn't clear, this is the *exact* point here - I don't know> whether this option should be enabled for iMX6 or not, and the only> > way I found out was to grep the dts files in arch/arm/boot/dts for> the > driver's compatible string. What that reveals is that *no* 32-bit> dts > files contain the compatible string, and so I summise that *no*> 32-bit > iMX SoC should have this driver enabled. > The problem is a bit more generic. I often have to spend lots of time > to find out which configs to enable on a specific board, to get certain > features (eg. network, sata, display, gpu, ...). Even worse: many > options require other stuff enabled before even showing up. And when > disabling unneeded stuff, it leaves lots of other things enabled. > (we don't have some `apt autoremove` kconfig counterpart :(). > > I've decided to cope w/ that on a higher level and written a little > config generator tool for that - here you can enable high level > features (eg. 'network' or 'display', etc) and it will generate the > actual .config: > > https://github.com/metux/kmct > > > The excuse that "we can't list the explicit SoCs" to me seems to be> a very lame excuse > > Maybe this actually means "nobody here volounteered to actually maintain > these help texts" ? > > > The best that I can come up with right now, given what little I know> from grepping the 32-bit DTS files, is that the help text should at> > least indicate that it *isn't* applicable to 32-bit SoCs, or if you> > prefer, *is* applicable to 64-bit SoCs. Beyond that, I have no> > information to formulate a better suggestion. > Perhaps just fix the text based on your knowledge and send a patch to > the maintainers. They'll propably tell you if it's incorrect. Frankly, no. I don't want to be going round endlessly writing help texts. We need the effort to be properly distributed - we need those who _know_ the feature they're developing to write a proper help text. One way to achieve that is to make a proper informative help text a pre-requisit of accepting the code. The quality of the help text is just as important as the quality of the code, and we really should be paying the same amount of attention to both. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help 2019-03-06 20:22 ` Russell King - ARM Linux admin @ 2019-03-09 3:25 ` Randy Dunlap -1 siblings, 0 replies; 24+ messages in thread From: Randy Dunlap @ 2019-03-09 3:25 UTC (permalink / raw) To: Russell King - ARM Linux admin, Enrico Weigelt, metux IT consult Cc: Andy Shevchenko, Mark Rutland, Peng Hao, Greg Kroah-Hartman, Linux Kernel Mailing List, linux-arm Mailing List, Lucas Stach, Andrew Morton On 3/6/19 12:22 PM, Russell King - ARM Linux admin wrote: > On Wed, Mar 06, 2019 at 09:16:02PM +0100, Enrico Weigelt, metux IT consult wrote: >> On 06.03.19 13:42, Russell King - ARM Linux admin wrote: >> >>> In case it isn't clear, this is the *exact* point here - I don't know> whether this option should be enabled for iMX6 or not, and the only> >> way I found out was to grep the dts files in arch/arm/boot/dts for> the >> driver's compatible string. What that reveals is that *no* 32-bit> dts >> files contain the compatible string, and so I summise that *no*> 32-bit >> iMX SoC should have this driver enabled. >> The problem is a bit more generic. I often have to spend lots of time >> to find out which configs to enable on a specific board, to get certain >> features (eg. network, sata, display, gpu, ...). Even worse: many >> options require other stuff enabled before even showing up. And when >> disabling unneeded stuff, it leaves lots of other things enabled. >> (we don't have some `apt autoremove` kconfig counterpart :(). >> >> I've decided to cope w/ that on a higher level and written a little >> config generator tool for that - here you can enable high level >> features (eg. 'network' or 'display', etc) and it will generate the >> actual .config: >> >> https://github.com/metux/kmct >> >>> The excuse that "we can't list the explicit SoCs" to me seems to be> a very lame excuse >> >> Maybe this actually means "nobody here volounteered to actually maintain >> these help texts" ? >> >>> The best that I can come up with right now, given what little I know> from grepping the 32-bit DTS files, is that the help text should at> >> least indicate that it *isn't* applicable to 32-bit SoCs, or if you> >> prefer, *is* applicable to 64-bit SoCs. Beyond that, I have no> >> information to formulate a better suggestion. >> Perhaps just fix the text based on your knowledge and send a patch to >> the maintainers. They'll propably tell you if it's incorrect. > > Frankly, no. I don't want to be going round endlessly writing help > texts. > > We need the effort to be properly distributed - we need those who > _know_ the feature they're developing to write a proper help text. > One way to achieve that is to make a proper informative help text > a pre-requisit of accepting the code. Ack. > The quality of the help text is just as important as the quality of > the code, and we really should be paying the same amount of attention > to both. It goes much further than this IMHO, such as: - #including the needed header files and not #including header files that are not used. - using the correct Kconfig dependencies and selects - testing builds with multiple kconfig settings (as applicable) - builds should be clean, and that means without newly added warnings as well as build errors I.e., the build testing that the 0day kernel robot does and that Arnd and I do and that a few other people do should not catch nearly as many build problems as they do. The developer who knows the code should put due diligence into the entire package, not just the (driver) source code and building/testing with one default config. Documentation/process/submit-checklist.rst has a more thorough list. -- ~Randy ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help @ 2019-03-09 3:25 ` Randy Dunlap 0 siblings, 0 replies; 24+ messages in thread From: Randy Dunlap @ 2019-03-09 3:25 UTC (permalink / raw) To: Russell King - ARM Linux admin, Enrico Weigelt, metux IT consult Cc: Mark Rutland, Peng Hao, Greg Kroah-Hartman, Linux Kernel Mailing List, Andy Shevchenko, Andrew Morton, linux-arm Mailing List, Lucas Stach On 3/6/19 12:22 PM, Russell King - ARM Linux admin wrote: > On Wed, Mar 06, 2019 at 09:16:02PM +0100, Enrico Weigelt, metux IT consult wrote: >> On 06.03.19 13:42, Russell King - ARM Linux admin wrote: >> >>> In case it isn't clear, this is the *exact* point here - I don't know> whether this option should be enabled for iMX6 or not, and the only> >> way I found out was to grep the dts files in arch/arm/boot/dts for> the >> driver's compatible string. What that reveals is that *no* 32-bit> dts >> files contain the compatible string, and so I summise that *no*> 32-bit >> iMX SoC should have this driver enabled. >> The problem is a bit more generic. I often have to spend lots of time >> to find out which configs to enable on a specific board, to get certain >> features (eg. network, sata, display, gpu, ...). Even worse: many >> options require other stuff enabled before even showing up. And when >> disabling unneeded stuff, it leaves lots of other things enabled. >> (we don't have some `apt autoremove` kconfig counterpart :(). >> >> I've decided to cope w/ that on a higher level and written a little >> config generator tool for that - here you can enable high level >> features (eg. 'network' or 'display', etc) and it will generate the >> actual .config: >> >> https://github.com/metux/kmct >> >>> The excuse that "we can't list the explicit SoCs" to me seems to be> a very lame excuse >> >> Maybe this actually means "nobody here volounteered to actually maintain >> these help texts" ? >> >>> The best that I can come up with right now, given what little I know> from grepping the 32-bit DTS files, is that the help text should at> >> least indicate that it *isn't* applicable to 32-bit SoCs, or if you> >> prefer, *is* applicable to 64-bit SoCs. Beyond that, I have no> >> information to formulate a better suggestion. >> Perhaps just fix the text based on your knowledge and send a patch to >> the maintainers. They'll propably tell you if it's incorrect. > > Frankly, no. I don't want to be going round endlessly writing help > texts. > > We need the effort to be properly distributed - we need those who > _know_ the feature they're developing to write a proper help text. > One way to achieve that is to make a proper informative help text > a pre-requisit of accepting the code. Ack. > The quality of the help text is just as important as the quality of > the code, and we really should be paying the same amount of attention > to both. It goes much further than this IMHO, such as: - #including the needed header files and not #including header files that are not used. - using the correct Kconfig dependencies and selects - testing builds with multiple kconfig settings (as applicable) - builds should be clean, and that means without newly added warnings as well as build errors I.e., the build testing that the 0day kernel robot does and that Arnd and I do and that a few other people do should not catch nearly as many build problems as they do. The developer who knows the code should put due diligence into the entire package, not just the (driver) source code and building/testing with one default config. Documentation/process/submit-checklist.rst has a more thorough list. -- ~Randy _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help 2019-03-06 9:45 ` Lucas Stach @ 2019-03-21 20:36 ` Pavel Machek -1 siblings, 0 replies; 24+ messages in thread From: Pavel Machek @ 2019-03-21 20:36 UTC (permalink / raw) To: Lucas Stach Cc: Russell King - ARM Linux admin, linux-arm-kernel, linux-kernel, Peng Hao, Mark Rutland, Andy Shevchenko, Greg Kroah-Hartman [-- Attachment #1: Type: text/plain, Size: 2303 bytes --] On Wed 2019-03-06 10:45:52, Lucas Stach wrote: > Hi Russell, > > Am Dienstag, den 05.03.2019, 17:31 +0000 schrieb Russell King - ARM Linux admin: > > Guys, > > > > We need to be smarter when writing Kconfig help. I'm just going > > through updating my build trees with the results of 5.0 development, > > and a number of the help texts are next to useless. For example, > > > > PVPANIC - is this something that should be enabled for a host or > > guest kernel? Answer: you have to read the driver code to find out. > > > > IMX_IRQSTEER - which i.MX SoCs does this apply to? The help text > > just says: > > > > "Support for the i.MX IRQSTEER interrupt multiplexer/remapper." > > > > which doesn't say which SoCs this should be enabled for - it turns > > out that grepping for the driver's DT compatible string, none of > > the 32-bit ARM cores have support for this, yet we still default > > it to enabled there. It seems the help text should at the very > > least tell the user that this is not applicable to i.MX SoCs with > > 32-bit ARM cores. > > > > I'm sure there's many other instances of this... I suspect that > > it's caused by review concentrating mostly on the technical aspects > > of the code and the Kconfig help text just gets forgotten about. > > > > Can we at least improve these two options please? > > While I totally agree that the irqsteer driver should only default Y on > 64bit i.MX SoCs, I'm not really sure what to do about the Kconfig help > text. > > Enumerating SoCs in the Kconfig of a IP block driver is always prone to > get outdated. Just as the first example I've been able to come up with, > the MMC_SDHCI_ESDHC_IMX help text says: "This selects the Freescale > eSDHC/uSDHC controller support found on i.MX25, i.MX35 i.MX5x and > i.MX6x.", while the same IP block is to be found on i.MX7 and various > i.MX8. "This selects the Freescale eSDHC/uSDHC controller support found on i.MX25, i.MX35 i.MX5x, i.MX6x and later SoCs.". Really, there are way too many silicon blocks, and you can't expect Kconfig user to know them all... Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Smarter Kconfig help @ 2019-03-21 20:36 ` Pavel Machek 0 siblings, 0 replies; 24+ messages in thread From: Pavel Machek @ 2019-03-21 20:36 UTC (permalink / raw) To: Lucas Stach Cc: Mark Rutland, Peng Hao, Greg Kroah-Hartman, Russell King - ARM Linux admin, linux-kernel, Andy Shevchenko, linux-arm-kernel [-- Attachment #1.1: Type: text/plain, Size: 2303 bytes --] On Wed 2019-03-06 10:45:52, Lucas Stach wrote: > Hi Russell, > > Am Dienstag, den 05.03.2019, 17:31 +0000 schrieb Russell King - ARM Linux admin: > > Guys, > > > > We need to be smarter when writing Kconfig help. I'm just going > > through updating my build trees with the results of 5.0 development, > > and a number of the help texts are next to useless. For example, > > > > PVPANIC - is this something that should be enabled for a host or > > guest kernel? Answer: you have to read the driver code to find out. > > > > IMX_IRQSTEER - which i.MX SoCs does this apply to? The help text > > just says: > > > > "Support for the i.MX IRQSTEER interrupt multiplexer/remapper." > > > > which doesn't say which SoCs this should be enabled for - it turns > > out that grepping for the driver's DT compatible string, none of > > the 32-bit ARM cores have support for this, yet we still default > > it to enabled there. It seems the help text should at the very > > least tell the user that this is not applicable to i.MX SoCs with > > 32-bit ARM cores. > > > > I'm sure there's many other instances of this... I suspect that > > it's caused by review concentrating mostly on the technical aspects > > of the code and the Kconfig help text just gets forgotten about. > > > > Can we at least improve these two options please? > > While I totally agree that the irqsteer driver should only default Y on > 64bit i.MX SoCs, I'm not really sure what to do about the Kconfig help > text. > > Enumerating SoCs in the Kconfig of a IP block driver is always prone to > get outdated. Just as the first example I've been able to come up with, > the MMC_SDHCI_ESDHC_IMX help text says: "This selects the Freescale > eSDHC/uSDHC controller support found on i.MX25, i.MX35 i.MX5x and > i.MX6x.", while the same IP block is to be found on i.MX7 and various > i.MX8. "This selects the Freescale eSDHC/uSDHC controller support found on i.MX25, i.MX35 i.MX5x, i.MX6x and later SoCs.". Really, there are way too many silicon blocks, and you can't expect Kconfig user to know them all... Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2019-03-21 20:36 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-05 17:31 Smarter Kconfig help Russell King - ARM Linux admin 2019-03-05 17:31 ` Russell King - ARM Linux admin 2019-03-06 9:35 ` Mark Rutland 2019-03-06 9:35 ` Mark Rutland 2019-03-06 12:50 ` Russell King - ARM Linux admin 2019-03-06 12:50 ` Russell King - ARM Linux admin 2019-03-06 9:45 ` Lucas Stach 2019-03-06 9:45 ` Lucas Stach 2019-03-06 9:51 ` Russell King - ARM Linux admin 2019-03-06 9:51 ` Russell King - ARM Linux admin 2019-03-06 10:49 ` Andy Shevchenko 2019-03-06 10:49 ` Andy Shevchenko 2019-03-06 11:34 ` Russell King - ARM Linux admin 2019-03-06 11:34 ` Russell King - ARM Linux admin 2019-03-06 12:42 ` Russell King - ARM Linux admin 2019-03-06 12:42 ` Russell King - ARM Linux admin 2019-03-06 20:16 ` Enrico Weigelt, metux IT consult 2019-03-06 20:16 ` Enrico Weigelt, metux IT consult 2019-03-06 20:22 ` Russell King - ARM Linux admin 2019-03-06 20:22 ` Russell King - ARM Linux admin 2019-03-09 3:25 ` Randy Dunlap 2019-03-09 3:25 ` Randy Dunlap 2019-03-21 20:36 ` Pavel Machek 2019-03-21 20:36 ` Pavel Machek
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.