Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
On 09/15/2014 09:55 PM, Arnd Bergmann wrote: > On Sunday 14 September 2014, Chen Gang wrote: >> Hello Maintainers: >> >> Is this patch worthy enough to be applied? >> >> Welcome any ideas, suggestions or completions. > > I find it hard to judge whether it's worth it or not. You gave one example > for a driver that needs this, but what would be your estimate on how > many other drivers have the same problem, either producing a #warning > or misbehaving if neither symbol is set? > For me, it is also hard to judge, so at present, we can assume it is not worthy enough for other individual modules. > Having all architectures consistently report the endianess would be nice, > but we have to weigh the advantages (currently broken code, potential > simplifications) against the regression risk. > Yeah, so our 'goal' is OK, but we need focus on 'how'. At present, firstly need modify 'init/Kconfig', and then scan all architectures one by one, let each related patch pass checking by the related maintainers (which can avoid the risk). Thanks. -- Chen Gang Open share and attitude like air water and life which God blessed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
On Sunday 14 September 2014, Chen Gang wrote: > Hello Maintainers: > > Is this patch worthy enough to be applied? > > Welcome any ideas, suggestions or completions. I find it hard to judge whether it's worth it or not. You gave one example for a driver that needs this, but what would be your estimate on how many other drivers have the same problem, either producing a #warning or misbehaving if neither symbol is set? Having all architectures consistently report the endianess would be nice, but we have to weigh the advantages (currently broken code, potential simplifications) against the regression risk. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
On Sunday 14 September 2014, Chen Gang wrote: Hello Maintainers: Is this patch worthy enough to be applied? Welcome any ideas, suggestions or completions. I find it hard to judge whether it's worth it or not. You gave one example for a driver that needs this, but what would be your estimate on how many other drivers have the same problem, either producing a #warning or misbehaving if neither symbol is set? Having all architectures consistently report the endianess would be nice, but we have to weigh the advantages (currently broken code, potential simplifications) against the regression risk. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
On 09/15/2014 09:55 PM, Arnd Bergmann wrote: On Sunday 14 September 2014, Chen Gang wrote: Hello Maintainers: Is this patch worthy enough to be applied? Welcome any ideas, suggestions or completions. I find it hard to judge whether it's worth it or not. You gave one example for a driver that needs this, but what would be your estimate on how many other drivers have the same problem, either producing a #warning or misbehaving if neither symbol is set? For me, it is also hard to judge, so at present, we can assume it is not worthy enough for other individual modules. Having all architectures consistently report the endianess would be nice, but we have to weigh the advantages (currently broken code, potential simplifications) against the regression risk. Yeah, so our 'goal' is OK, but we need focus on 'how'. At present, firstly need modify 'init/Kconfig', and then scan all architectures one by one, let each related patch pass checking by the related maintainers (which can avoid the risk). Thanks. -- Chen Gang Open share and attitude like air water and life which God blessed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
Hello Maintainers: Is this patch worthy enough to be applied? Welcome any ideas, suggestions or completions. Thanks. On 09/03/2014 07:47 PM, Chen Gang wrote: > On 09/02/2014 02:13 PM, Chen Gang wrote: >> On 9/2/14 13:17, Paul Gortmaker wrote: >>> >>> OK, but that was not at _all_ what I thought when looking at this... >>> >>> Instead I saw a well intentioned, but perhaps not fully thought out >>> attempt at fixing a largely irrelevant randconfig/allmodconfig of a >>> 1990's vintage ISDN driver coming from that x86-only era, built against >>> an architecture that will never use or support it (microblaze). >>> >>> In today's world, we'd probably not accept a new ethernet driver or >>> filesystem if it was incapable of handling both BE and LE (exception >>> being SoC ethernet physically bound to one specific CPU, of course.) >>> So the justification given in the commit log for expanding the scope to >>> better deal with the stuff found in ISDN and the like was questionable. >>> >> > > I guess, we are mainly focus on: "Is it worthy enough to add *ENDIAN in > init/Kconfig?" > > It seems really few modules need depend on LE or BE (although I am still > not quite sure). If it is true, for me, only for architectures, it is > still worthy enough to let all duplicated *ENDIAN to one place. > > If it is still worthy, maybe this patch need be improvement (especially, > for its comments), so please help check when you have time, thanks. > > > Welcome any other members' ideas, suggestions, or completions. > > Thanks. > >> After a simple search, for crypto, it may be endian sensitive, and for >> architectures may be endian sensitive, in config time. >> >> bash-3.2# find ./ | grep Kconfig | xargs grep depend | grep ENDIAN >> .//arch/arm/mm/Kconfig:depends on ARCH_SUPPORTS_BIG_ENDIAN >> .//arch/arm/mm/Kconfig:depends on CPU_BIG_ENDIAN >> .//arch/arm/mm/Kconfig:depends on CPU_BIG_ENDIAN >> .//arch/arm64/Kconfig: depends on OF && !CPU_BIG_ENDIAN >> .//arch/mips/Kconfig: depends on SYS_SUPPORTS_BIG_ENDIAN >> .//arch/mips/Kconfig: depends on SYS_SUPPORTS_LITTLE_ENDIAN >> .//arch/mips/Kconfig: depends on SGI_IP22 || SGI_IP28 || (SNI_RM && >> CPU_LITTLE_ENDIAN) >> .//arch/powerpc/platforms/Kconfig.cputype: depends on !CPU_LITTLE_ENDIAN >> .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 && >> !CPU_LITTLE_ENDIAN >> .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 && >> !CPU_LITTLE_ENDIAN >> .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 && >> !CPU_LITTLE_ENDIAN >> .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 && >> !CPU_LITTLE_ENDIAN >> .//arch/powerpc/platforms/Kconfig.cputype: depends on CPU_LITTLE_ENDIAN >> [...] >> .//crypto/Kconfig: depends on ARM && KERNEL_MODE_NEON && !CPU_BIG_ENDIAN >> .//crypto/Kconfig: depends on ARM && KERNEL_MODE_NEON && !CPU_BIG_ENDIAN >> .//drivers/crypto/Kconfig: depends on PPC64 && IBMVIO && !CPU_LITTLE_ENDIAN >> [...] >> >> It is a simple search, so I am not sure whether have other modules also >> need LE or BE. >> > > Thanks. > -- Chen Gang Open share and attitude like air water and life which God blessed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
Hello Maintainers: Is this patch worthy enough to be applied? Welcome any ideas, suggestions or completions. Thanks. On 09/03/2014 07:47 PM, Chen Gang wrote: On 09/02/2014 02:13 PM, Chen Gang wrote: On 9/2/14 13:17, Paul Gortmaker wrote: OK, but that was not at _all_ what I thought when looking at this... Instead I saw a well intentioned, but perhaps not fully thought out attempt at fixing a largely irrelevant randconfig/allmodconfig of a 1990's vintage ISDN driver coming from that x86-only era, built against an architecture that will never use or support it (microblaze). In today's world, we'd probably not accept a new ethernet driver or filesystem if it was incapable of handling both BE and LE (exception being SoC ethernet physically bound to one specific CPU, of course.) So the justification given in the commit log for expanding the scope to better deal with the stuff found in ISDN and the like was questionable. I guess, we are mainly focus on: Is it worthy enough to add *ENDIAN in init/Kconfig? It seems really few modules need depend on LE or BE (although I am still not quite sure). If it is true, for me, only for architectures, it is still worthy enough to let all duplicated *ENDIAN to one place. If it is still worthy, maybe this patch need be improvement (especially, for its comments), so please help check when you have time, thanks. Welcome any other members' ideas, suggestions, or completions. Thanks. After a simple search, for crypto, it may be endian sensitive, and for architectures may be endian sensitive, in config time. bash-3.2# find ./ | grep Kconfig | xargs grep depend | grep ENDIAN .//arch/arm/mm/Kconfig:depends on ARCH_SUPPORTS_BIG_ENDIAN .//arch/arm/mm/Kconfig:depends on CPU_BIG_ENDIAN .//arch/arm/mm/Kconfig:depends on CPU_BIG_ENDIAN .//arch/arm64/Kconfig: depends on OF !CPU_BIG_ENDIAN .//arch/mips/Kconfig: depends on SYS_SUPPORTS_BIG_ENDIAN .//arch/mips/Kconfig: depends on SYS_SUPPORTS_LITTLE_ENDIAN .//arch/mips/Kconfig: depends on SGI_IP22 || SGI_IP28 || (SNI_RM CPU_LITTLE_ENDIAN) .//arch/powerpc/platforms/Kconfig.cputype: depends on !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype: depends on CPU_LITTLE_ENDIAN [...] .//crypto/Kconfig: depends on ARM KERNEL_MODE_NEON !CPU_BIG_ENDIAN .//crypto/Kconfig: depends on ARM KERNEL_MODE_NEON !CPU_BIG_ENDIAN .//drivers/crypto/Kconfig: depends on PPC64 IBMVIO !CPU_LITTLE_ENDIAN [...] It is a simple search, so I am not sure whether have other modules also need LE or BE. Thanks. -- Chen Gang Open share and attitude like air water and life which God blessed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
On 09/02/2014 02:13 PM, Chen Gang wrote: > On 9/2/14 13:17, Paul Gortmaker wrote: >> >> OK, but that was not at _all_ what I thought when looking at this... >> >> Instead I saw a well intentioned, but perhaps not fully thought out >> attempt at fixing a largely irrelevant randconfig/allmodconfig of a >> 1990's vintage ISDN driver coming from that x86-only era, built against >> an architecture that will never use or support it (microblaze). >> >> In today's world, we'd probably not accept a new ethernet driver or >> filesystem if it was incapable of handling both BE and LE (exception >> being SoC ethernet physically bound to one specific CPU, of course.) >> So the justification given in the commit log for expanding the scope to >> better deal with the stuff found in ISDN and the like was questionable. >> > I guess, we are mainly focus on: "Is it worthy enough to add *ENDIAN in init/Kconfig?" It seems really few modules need depend on LE or BE (although I am still not quite sure). If it is true, for me, only for architectures, it is still worthy enough to let all duplicated *ENDIAN to one place. If it is still worthy, maybe this patch need be improvement (especially, for its comments), so please help check when you have time, thanks. Welcome any other members' ideas, suggestions, or completions. Thanks. > After a simple search, for crypto, it may be endian sensitive, and for > architectures may be endian sensitive, in config time. > > bash-3.2# find ./ | grep Kconfig | xargs grep depend | grep ENDIAN > .//arch/arm/mm/Kconfig: depends on ARCH_SUPPORTS_BIG_ENDIAN > .//arch/arm/mm/Kconfig: depends on CPU_BIG_ENDIAN > .//arch/arm/mm/Kconfig: depends on CPU_BIG_ENDIAN > .//arch/arm64/Kconfig: depends on OF && !CPU_BIG_ENDIAN > .//arch/mips/Kconfig: depends on SYS_SUPPORTS_BIG_ENDIAN > .//arch/mips/Kconfig: depends on SYS_SUPPORTS_LITTLE_ENDIAN > .//arch/mips/Kconfig: depends on SGI_IP22 || SGI_IP28 || (SNI_RM && > CPU_LITTLE_ENDIAN) > .//arch/powerpc/platforms/Kconfig.cputype: depends on !CPU_LITTLE_ENDIAN > .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 && > !CPU_LITTLE_ENDIAN > .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 && > !CPU_LITTLE_ENDIAN > .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 && > !CPU_LITTLE_ENDIAN > .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 && > !CPU_LITTLE_ENDIAN > .//arch/powerpc/platforms/Kconfig.cputype: depends on CPU_LITTLE_ENDIAN > [...] > .//crypto/Kconfig: depends on ARM && KERNEL_MODE_NEON && !CPU_BIG_ENDIAN > .//crypto/Kconfig: depends on ARM && KERNEL_MODE_NEON && !CPU_BIG_ENDIAN > .//drivers/crypto/Kconfig: depends on PPC64 && IBMVIO && !CPU_LITTLE_ENDIAN > [...] > > It is a simple search, so I am not sure whether have other modules also > need LE or BE. > Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
On 09/02/2014 02:13 PM, Chen Gang wrote: On 9/2/14 13:17, Paul Gortmaker wrote: OK, but that was not at _all_ what I thought when looking at this... Instead I saw a well intentioned, but perhaps not fully thought out attempt at fixing a largely irrelevant randconfig/allmodconfig of a 1990's vintage ISDN driver coming from that x86-only era, built against an architecture that will never use or support it (microblaze). In today's world, we'd probably not accept a new ethernet driver or filesystem if it was incapable of handling both BE and LE (exception being SoC ethernet physically bound to one specific CPU, of course.) So the justification given in the commit log for expanding the scope to better deal with the stuff found in ISDN and the like was questionable. I guess, we are mainly focus on: Is it worthy enough to add *ENDIAN in init/Kconfig? It seems really few modules need depend on LE or BE (although I am still not quite sure). If it is true, for me, only for architectures, it is still worthy enough to let all duplicated *ENDIAN to one place. If it is still worthy, maybe this patch need be improvement (especially, for its comments), so please help check when you have time, thanks. Welcome any other members' ideas, suggestions, or completions. Thanks. After a simple search, for crypto, it may be endian sensitive, and for architectures may be endian sensitive, in config time. bash-3.2# find ./ | grep Kconfig | xargs grep depend | grep ENDIAN .//arch/arm/mm/Kconfig: depends on ARCH_SUPPORTS_BIG_ENDIAN .//arch/arm/mm/Kconfig: depends on CPU_BIG_ENDIAN .//arch/arm/mm/Kconfig: depends on CPU_BIG_ENDIAN .//arch/arm64/Kconfig: depends on OF !CPU_BIG_ENDIAN .//arch/mips/Kconfig: depends on SYS_SUPPORTS_BIG_ENDIAN .//arch/mips/Kconfig: depends on SYS_SUPPORTS_LITTLE_ENDIAN .//arch/mips/Kconfig: depends on SGI_IP22 || SGI_IP28 || (SNI_RM CPU_LITTLE_ENDIAN) .//arch/powerpc/platforms/Kconfig.cputype: depends on !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype: depends on PPC_BOOK3S_64 !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype: depends on CPU_LITTLE_ENDIAN [...] .//crypto/Kconfig: depends on ARM KERNEL_MODE_NEON !CPU_BIG_ENDIAN .//crypto/Kconfig: depends on ARM KERNEL_MODE_NEON !CPU_BIG_ENDIAN .//drivers/crypto/Kconfig: depends on PPC64 IBMVIO !CPU_LITTLE_ENDIAN [...] It is a simple search, so I am not sure whether have other modules also need LE or BE. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
On 9/2/14 13:17, Paul Gortmaker wrote: > [Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using] > On 01/09/2014 (Mon 10:01) H. Peter Anvin wrote: > >> On 09/01/2014 09:08 AM, Paul Gortmaker wrote: >>>>> >>>> diff --git a/init/Kconfig b/init/Kconfig >>>> index ac033c3..f301cc8 100644 >>>> --- a/init/Kconfig >>>> +++ b/init/Kconfig >>>> @@ -23,6 +23,12 @@ config CONSTRUCTORS >>>> config IRQ_WORK >>>>bool >>>> >>>> +config CPU_LITTLE_ENDIAN >>>> + bool >>>> + >>>> +config CPU_BIG_ENDIAN >>>> + bool >>> >>> Perhaps you should take a cursory look at what already exists in tree >>> before blindly trying to add more to it? >>> >>> $ git grep CPU_BIG_ENDIAN | wc -l >>> 88 >>> >> >> The whole point of this patchset is to make these already widely-used >> options universal across the tree. > > OK, but that was not at _all_ what I thought when looking at this... > > Instead I saw a well intentioned, but perhaps not fully thought out > attempt at fixing a largely irrelevant randconfig/allmodconfig of a > 1990's vintage ISDN driver coming from that x86-only era, built against > an architecture that will never use or support it (microblaze). > > In today's world, we'd probably not accept a new ethernet driver or > filesystem if it was incapable of handling both BE and LE (exception > being SoC ethernet physically bound to one specific CPU, of course.) > So the justification given in the commit log for expanding the scope to > better deal with the stuff found in ISDN and the like was questionable. > After a simple search, for crypto, it may be endian sensitive, and for architectures may be endian sensitive, in config time. bash-3.2# find ./ | grep Kconfig | xargs grep depend | grep ENDIAN .//arch/arm/mm/Kconfig: depends on ARCH_SUPPORTS_BIG_ENDIAN .//arch/arm/mm/Kconfig: depends on CPU_BIG_ENDIAN .//arch/arm/mm/Kconfig: depends on CPU_BIG_ENDIAN .//arch/arm64/Kconfig:depends on OF && !CPU_BIG_ENDIAN .//arch/mips/Kconfig: depends on SYS_SUPPORTS_BIG_ENDIAN .//arch/mips/Kconfig: depends on SYS_SUPPORTS_LITTLE_ENDIAN .//arch/mips/Kconfig: depends on SGI_IP22 || SGI_IP28 || (SNI_RM && CPU_LITTLE_ENDIAN) .//arch/powerpc/platforms/Kconfig.cputype:depends on !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype:depends on PPC_BOOK3S_64 && !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype:depends on PPC_BOOK3S_64 && !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype:depends on PPC_BOOK3S_64 && !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype:depends on PPC_BOOK3S_64 && !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype:depends on CPU_LITTLE_ENDIAN [...] .//crypto/Kconfig:depends on ARM && KERNEL_MODE_NEON && !CPU_BIG_ENDIAN .//crypto/Kconfig:depends on ARM && KERNEL_MODE_NEON && !CPU_BIG_ENDIAN .//drivers/crypto/Kconfig:depends on PPC64 && IBMVIO && !CPU_LITTLE_ENDIAN [...] It is a simple search, so I am not sure whether have other modules also need LE or BE. > Secondly, I don't think it is well known that Kbuild will tolerate > multiply defined symbols of the exact same name, and since that isn't > mentioned in the commit log (as documented and/or tested), I envisioned > this breaking powerpc and other arch who already define one (or both) > of these two. I found multi-define _is_ documented as supported in > Documentation/kbuild but I still wonder how much it is used and how > well it handles things like in powerpc, where one of them (LE?) also > lists a "select ... " line and help text under the bool. > > So if we want to do this, I'd suggest a commit log that really gets > to the intended point; something along the lines of: > > -- > > Currently we have some architectures defining their own bool for > CPU_LITTLE_ENDIAN and/or CPU_BIG_ENDIAN. As of 3.17-rc3 we have: > > CPU_BIG_ENDIAN: arc, arm, arm64, c6x, mips, powerpc, sh > CPU_LITTLE_ENDIAN: m32r, mips, powerpc, sh > > Note that the scope does not cover all arch, which reduces the utility > value of these items in generic and/or arch independent code, as > neither may be defined, making tests on their values inconclusive. > > To fix the above, the goal is to make both items present in an arch > independent Kconfig. We can do this immediately without having to > simultaneously touch the existing arch bool definitions because... > > T
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
On 9/2/14 13:17, Paul Gortmaker wrote: [Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using] On 01/09/2014 (Mon 10:01) H. Peter Anvin wrote: On 09/01/2014 09:08 AM, Paul Gortmaker wrote: diff --git a/init/Kconfig b/init/Kconfig index ac033c3..f301cc8 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -23,6 +23,12 @@ config CONSTRUCTORS config IRQ_WORK bool +config CPU_LITTLE_ENDIAN + bool + +config CPU_BIG_ENDIAN + bool Perhaps you should take a cursory look at what already exists in tree before blindly trying to add more to it? $ git grep CPU_BIG_ENDIAN | wc -l 88 The whole point of this patchset is to make these already widely-used options universal across the tree. OK, but that was not at _all_ what I thought when looking at this... Instead I saw a well intentioned, but perhaps not fully thought out attempt at fixing a largely irrelevant randconfig/allmodconfig of a 1990's vintage ISDN driver coming from that x86-only era, built against an architecture that will never use or support it (microblaze). In today's world, we'd probably not accept a new ethernet driver or filesystem if it was incapable of handling both BE and LE (exception being SoC ethernet physically bound to one specific CPU, of course.) So the justification given in the commit log for expanding the scope to better deal with the stuff found in ISDN and the like was questionable. After a simple search, for crypto, it may be endian sensitive, and for architectures may be endian sensitive, in config time. bash-3.2# find ./ | grep Kconfig | xargs grep depend | grep ENDIAN .//arch/arm/mm/Kconfig: depends on ARCH_SUPPORTS_BIG_ENDIAN .//arch/arm/mm/Kconfig: depends on CPU_BIG_ENDIAN .//arch/arm/mm/Kconfig: depends on CPU_BIG_ENDIAN .//arch/arm64/Kconfig:depends on OF !CPU_BIG_ENDIAN .//arch/mips/Kconfig: depends on SYS_SUPPORTS_BIG_ENDIAN .//arch/mips/Kconfig: depends on SYS_SUPPORTS_LITTLE_ENDIAN .//arch/mips/Kconfig: depends on SGI_IP22 || SGI_IP28 || (SNI_RM CPU_LITTLE_ENDIAN) .//arch/powerpc/platforms/Kconfig.cputype:depends on !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype:depends on PPC_BOOK3S_64 !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype:depends on PPC_BOOK3S_64 !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype:depends on PPC_BOOK3S_64 !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype:depends on PPC_BOOK3S_64 !CPU_LITTLE_ENDIAN .//arch/powerpc/platforms/Kconfig.cputype:depends on CPU_LITTLE_ENDIAN [...] .//crypto/Kconfig:depends on ARM KERNEL_MODE_NEON !CPU_BIG_ENDIAN .//crypto/Kconfig:depends on ARM KERNEL_MODE_NEON !CPU_BIG_ENDIAN .//drivers/crypto/Kconfig:depends on PPC64 IBMVIO !CPU_LITTLE_ENDIAN [...] It is a simple search, so I am not sure whether have other modules also need LE or BE. Secondly, I don't think it is well known that Kbuild will tolerate multiply defined symbols of the exact same name, and since that isn't mentioned in the commit log (as documented and/or tested), I envisioned this breaking powerpc and other arch who already define one (or both) of these two. I found multi-define _is_ documented as supported in Documentation/kbuild but I still wonder how much it is used and how well it handles things like in powerpc, where one of them (LE?) also lists a select ... line and help text under the bool. So if we want to do this, I'd suggest a commit log that really gets to the intended point; something along the lines of: -- Currently we have some architectures defining their own bool for CPU_LITTLE_ENDIAN and/or CPU_BIG_ENDIAN. As of 3.17-rc3 we have: CPU_BIG_ENDIAN: arc, arm, arm64, c6x, mips, powerpc, sh CPU_LITTLE_ENDIAN: m32r, mips, powerpc, sh Note that the scope does not cover all arch, which reduces the utility value of these items in generic and/or arch independent code, as neither may be defined, making tests on their values inconclusive. To fix the above, the goal is to make both items present in an arch independent Kconfig. We can do this immediately without having to simultaneously touch the existing arch bool definitions because... This change was tested on a powerpc LE config since it has help text and a select line, and the behaviour of both before and after the change was found to be ... -- Also, having the suggested change Cc'd to linux-arch would be sensible, I think. And one might want to make it a series, showing the follow on arch specific commits you have planned that select an endian, since the maintainers may want evidence it will be carried to completion and they also may have something additional to say (as in the case of arch/arm where there is already CPU_ENDIAN_BE8 and CPU_ENDIAN_BE32 Kconfig items). Paul. -- OK, thanks, what
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
[Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using] On 01/09/2014 (Mon 10:01) H. Peter Anvin wrote: > On 09/01/2014 09:08 AM, Paul Gortmaker wrote: > >>> > >> diff --git a/init/Kconfig b/init/Kconfig > >> index ac033c3..f301cc8 100644 > >> --- a/init/Kconfig > >> +++ b/init/Kconfig > >> @@ -23,6 +23,12 @@ config CONSTRUCTORS > >> config IRQ_WORK > >>bool > >> > >> +config CPU_LITTLE_ENDIAN > >> + bool > >> + > >> +config CPU_BIG_ENDIAN > >> + bool > > > > Perhaps you should take a cursory look at what already exists in tree > > before blindly trying to add more to it? > > > > $ git grep CPU_BIG_ENDIAN | wc -l > > 88 > > > > The whole point of this patchset is to make these already widely-used > options universal across the tree. OK, but that was not at _all_ what I thought when looking at this... Instead I saw a well intentioned, but perhaps not fully thought out attempt at fixing a largely irrelevant randconfig/allmodconfig of a 1990's vintage ISDN driver coming from that x86-only era, built against an architecture that will never use or support it (microblaze). In today's world, we'd probably not accept a new ethernet driver or filesystem if it was incapable of handling both BE and LE (exception being SoC ethernet physically bound to one specific CPU, of course.) So the justification given in the commit log for expanding the scope to better deal with the stuff found in ISDN and the like was questionable. Secondly, I don't think it is well known that Kbuild will tolerate multiply defined symbols of the exact same name, and since that isn't mentioned in the commit log (as documented and/or tested), I envisioned this breaking powerpc and other arch who already define one (or both) of these two. I found multi-define _is_ documented as supported in Documentation/kbuild but I still wonder how much it is used and how well it handles things like in powerpc, where one of them (LE?) also lists a "select ... " line and help text under the bool. So if we want to do this, I'd suggest a commit log that really gets to the intended point; something along the lines of: -- Currently we have some architectures defining their own bool for CPU_LITTLE_ENDIAN and/or CPU_BIG_ENDIAN. As of 3.17-rc3 we have: CPU_BIG_ENDIAN: arc, arm, arm64, c6x, mips, powerpc, sh CPU_LITTLE_ENDIAN: m32r, mips, powerpc, sh Note that the scope does not cover all arch, which reduces the utility value of these items in generic and/or arch independent code, as neither may be defined, making tests on their values inconclusive. To fix the above, the goal is to make both items present in an arch independent Kconfig. We can do this immediately without having to simultaneously touch the existing arch bool definitions because... This change was tested on a powerpc LE config since it has help text and a select line, and the behaviour of both before and after the change was found to be ... -- Also, having the suggested change Cc'd to linux-arch would be sensible, I think. And one might want to make it a series, showing the follow on arch specific commits you have planned that "select" an endian, since the maintainers may want evidence it will be carried to completion and they also may have something additional to say (as in the case of arch/arm where there is already CPU_ENDIAN_BE8 and CPU_ENDIAN_BE32 Kconfig items). Paul. -- > > -hpa > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
On 09/02/2014 01:01 AM, H. Peter Anvin wrote: > On 09/01/2014 09:08 AM, Paul Gortmaker wrote: >>> diff --git a/init/Kconfig b/init/Kconfig >>> index ac033c3..f301cc8 100644 >>> --- a/init/Kconfig >>> +++ b/init/Kconfig >>> @@ -23,6 +23,12 @@ config CONSTRUCTORS >>> config IRQ_WORK >>> bool >>> >>> +config CPU_LITTLE_ENDIAN >>> + bool >>> + >>> +config CPU_BIG_ENDIAN >>> + bool >> >> Perhaps you should take a cursory look at what already exists in tree >> before blindly trying to add more to it? >> >> $ git grep CPU_BIG_ENDIAN | wc -l >> 88 >> > > The whole point of this patchset is to make these already widely-used > options universal across the tree. > Yeah, maybe the patch comments need be improved (mention about what you said above) to avoid other members' misunderstanding. If it is necessary (complete the comments), please let me know (I shall send patch v2 for it). Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
On 09/01/2014 09:08 AM, Paul Gortmaker wrote: >>> >> diff --git a/init/Kconfig b/init/Kconfig >> index ac033c3..f301cc8 100644 >> --- a/init/Kconfig >> +++ b/init/Kconfig >> @@ -23,6 +23,12 @@ config CONSTRUCTORS >> config IRQ_WORK >> bool >> >> +config CPU_LITTLE_ENDIAN >> +bool >> + >> +config CPU_BIG_ENDIAN >> +bool > > Perhaps you should take a cursory look at what already exists in tree > before blindly trying to add more to it? > > $ git grep CPU_BIG_ENDIAN | wc -l > 88 > The whole point of this patchset is to make these already widely-used options universal across the tree. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
[[PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using] On 01/09/2014 (Mon 23:46) Chen Gang wrote: > Some individual modules want to know the architecture's endian attribute > during config time, or may cause compiling break, one sample is below: > (with allm dconfig under microblaze): > > CC [M] drivers/isdn/hisax/nj_s.o > drivers/isdn/hisax/nj_s.c: In function 'setup_netjet_s': > drivers/isdn/hisax/nj_s.c:265:2: error: #error "not running on big endian > machines now" >#error "not running on big endian machines now" > > So add endian attributes for all architectures using (next, will let all > architectures choose their endians during config time). > > Signed-off-by: Chen Gang > --- > init/Kconfig | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/init/Kconfig b/init/Kconfig > index ac033c3..f301cc8 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -23,6 +23,12 @@ config CONSTRUCTORS > config IRQ_WORK > bool > > +config CPU_LITTLE_ENDIAN > + bool > + > +config CPU_BIG_ENDIAN > + bool Perhaps you should take a cursory look at what already exists in tree before blindly trying to add more to it? $ git grep CPU_BIG_ENDIAN | wc -l 88 Paul. -- > + > config BUILDTIME_EXTABLE_SORT > bool > > -- > 1.7.11.7 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
Some individual modules want to know the architecture's endian attribute during config time, or may cause compiling break, one sample is below: (with allm dconfig under microblaze): CC [M] drivers/isdn/hisax/nj_s.o drivers/isdn/hisax/nj_s.c: In function 'setup_netjet_s': drivers/isdn/hisax/nj_s.c:265:2: error: #error "not running on big endian machines now" #error "not running on big endian machines now" So add endian attributes for all architectures using (next, will let all architectures choose their endians during config time). Signed-off-by: Chen Gang --- init/Kconfig | 6 ++ 1 file changed, 6 insertions(+) diff --git a/init/Kconfig b/init/Kconfig index ac033c3..f301cc8 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -23,6 +23,12 @@ config CONSTRUCTORS config IRQ_WORK bool +config CPU_LITTLE_ENDIAN + bool + +config CPU_BIG_ENDIAN + bool + config BUILDTIME_EXTABLE_SORT bool -- 1.7.11.7 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
Some individual modules want to know the architecture's endian attribute during config time, or may cause compiling break, one sample is below: (with allm dconfig under microblaze): CC [M] drivers/isdn/hisax/nj_s.o drivers/isdn/hisax/nj_s.c: In function 'setup_netjet_s': drivers/isdn/hisax/nj_s.c:265:2: error: #error not running on big endian machines now #error not running on big endian machines now So add endian attributes for all architectures using (next, will let all architectures choose their endians during config time). Signed-off-by: Chen Gang gang.chen.5...@gmail.com --- init/Kconfig | 6 ++ 1 file changed, 6 insertions(+) diff --git a/init/Kconfig b/init/Kconfig index ac033c3..f301cc8 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -23,6 +23,12 @@ config CONSTRUCTORS config IRQ_WORK bool +config CPU_LITTLE_ENDIAN + bool + +config CPU_BIG_ENDIAN + bool + config BUILDTIME_EXTABLE_SORT bool -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
[[PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using] On 01/09/2014 (Mon 23:46) Chen Gang wrote: Some individual modules want to know the architecture's endian attribute during config time, or may cause compiling break, one sample is below: (with allm dconfig under microblaze): CC [M] drivers/isdn/hisax/nj_s.o drivers/isdn/hisax/nj_s.c: In function 'setup_netjet_s': drivers/isdn/hisax/nj_s.c:265:2: error: #error not running on big endian machines now #error not running on big endian machines now So add endian attributes for all architectures using (next, will let all architectures choose their endians during config time). Signed-off-by: Chen Gang gang.chen.5...@gmail.com --- init/Kconfig | 6 ++ 1 file changed, 6 insertions(+) diff --git a/init/Kconfig b/init/Kconfig index ac033c3..f301cc8 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -23,6 +23,12 @@ config CONSTRUCTORS config IRQ_WORK bool +config CPU_LITTLE_ENDIAN + bool + +config CPU_BIG_ENDIAN + bool Perhaps you should take a cursory look at what already exists in tree before blindly trying to add more to it? $ git grep CPU_BIG_ENDIAN | wc -l 88 Paul. -- + config BUILDTIME_EXTABLE_SORT bool -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
On 09/01/2014 09:08 AM, Paul Gortmaker wrote: diff --git a/init/Kconfig b/init/Kconfig index ac033c3..f301cc8 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -23,6 +23,12 @@ config CONSTRUCTORS config IRQ_WORK bool +config CPU_LITTLE_ENDIAN +bool + +config CPU_BIG_ENDIAN +bool Perhaps you should take a cursory look at what already exists in tree before blindly trying to add more to it? $ git grep CPU_BIG_ENDIAN | wc -l 88 The whole point of this patchset is to make these already widely-used options universal across the tree. -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
On 09/02/2014 01:01 AM, H. Peter Anvin wrote: On 09/01/2014 09:08 AM, Paul Gortmaker wrote: diff --git a/init/Kconfig b/init/Kconfig index ac033c3..f301cc8 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -23,6 +23,12 @@ config CONSTRUCTORS config IRQ_WORK bool +config CPU_LITTLE_ENDIAN + bool + +config CPU_BIG_ENDIAN + bool Perhaps you should take a cursory look at what already exists in tree before blindly trying to add more to it? $ git grep CPU_BIG_ENDIAN | wc -l 88 The whole point of this patchset is to make these already widely-used options universal across the tree. Yeah, maybe the patch comments need be improved (mention about what you said above) to avoid other members' misunderstanding. If it is necessary (complete the comments), please let me know (I shall send patch v2 for it). Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using
[Re: [PATCH] init/Kconfig: Add ENDIAN attributes for all architectures using] On 01/09/2014 (Mon 10:01) H. Peter Anvin wrote: On 09/01/2014 09:08 AM, Paul Gortmaker wrote: diff --git a/init/Kconfig b/init/Kconfig index ac033c3..f301cc8 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -23,6 +23,12 @@ config CONSTRUCTORS config IRQ_WORK bool +config CPU_LITTLE_ENDIAN + bool + +config CPU_BIG_ENDIAN + bool Perhaps you should take a cursory look at what already exists in tree before blindly trying to add more to it? $ git grep CPU_BIG_ENDIAN | wc -l 88 The whole point of this patchset is to make these already widely-used options universal across the tree. OK, but that was not at _all_ what I thought when looking at this... Instead I saw a well intentioned, but perhaps not fully thought out attempt at fixing a largely irrelevant randconfig/allmodconfig of a 1990's vintage ISDN driver coming from that x86-only era, built against an architecture that will never use or support it (microblaze). In today's world, we'd probably not accept a new ethernet driver or filesystem if it was incapable of handling both BE and LE (exception being SoC ethernet physically bound to one specific CPU, of course.) So the justification given in the commit log for expanding the scope to better deal with the stuff found in ISDN and the like was questionable. Secondly, I don't think it is well known that Kbuild will tolerate multiply defined symbols of the exact same name, and since that isn't mentioned in the commit log (as documented and/or tested), I envisioned this breaking powerpc and other arch who already define one (or both) of these two. I found multi-define _is_ documented as supported in Documentation/kbuild but I still wonder how much it is used and how well it handles things like in powerpc, where one of them (LE?) also lists a select ... line and help text under the bool. So if we want to do this, I'd suggest a commit log that really gets to the intended point; something along the lines of: -- Currently we have some architectures defining their own bool for CPU_LITTLE_ENDIAN and/or CPU_BIG_ENDIAN. As of 3.17-rc3 we have: CPU_BIG_ENDIAN: arc, arm, arm64, c6x, mips, powerpc, sh CPU_LITTLE_ENDIAN: m32r, mips, powerpc, sh Note that the scope does not cover all arch, which reduces the utility value of these items in generic and/or arch independent code, as neither may be defined, making tests on their values inconclusive. To fix the above, the goal is to make both items present in an arch independent Kconfig. We can do this immediately without having to simultaneously touch the existing arch bool definitions because... This change was tested on a powerpc LE config since it has help text and a select line, and the behaviour of both before and after the change was found to be ... -- Also, having the suggested change Cc'd to linux-arch would be sensible, I think. And one might want to make it a series, showing the follow on arch specific commits you have planned that select an endian, since the maintainers may want evidence it will be carried to completion and they also may have something additional to say (as in the case of arch/arm where there is already CPU_ENDIAN_BE8 and CPU_ENDIAN_BE32 Kconfig items). Paul. -- -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
