Hi Miquèl, > El 12 jun 2020, a las 9:02, Miquel Raynal <[email protected]> > escribió: > > Hi Álvaro, > > Álvaro Fernández Rojas <[email protected]> wrote on Thu, 11 Jun 2020 > 18:14:20 +0200: > >> Hi Florian, >> >>> El 11 jun 2020, a las 17:42, Florian Fainelli <[email protected]> >>> escribió: >>> >>> >>> >>> On 6/11/2020 8:16 AM, Álvaro Fernández Rojas wrote: >>>> Hi Miquel, >>>> >>>>> El 11 jun 2020, a las 9:55, Miquel Raynal <[email protected]> >>>>> escribió: >>>>> >>>>> Hi Álvaro, >>>>> >>>>> Álvaro Fernández Rojas <[email protected]> wrote on Mon, 8 Jun 2020 >>>>> 18:06:49 +0200: >>>>> >>>>>> Instead of trying to parse CFE version string, which is customized by >>>>>> some >>>>>> vendors, let's just check that "CFE1" was passed on argument 3. >>>>>> >>>>>> Signed-off-by: Álvaro Fernández Rojas <[email protected]> >>>>>> Signed-off-by: Jonas Gorski <[email protected]> >>>>>> --- >>>>>> v2: use CFE_EPTSEAL definition and avoid using an additional funtion. >>>>>> >>>>>> drivers/mtd/parsers/bcm63xxpart.c | 29 ++++------------------------- >>>>>> 1 file changed, 4 insertions(+), 25 deletions(-) >>>>>> >>>>>> diff --git a/drivers/mtd/parsers/bcm63xxpart.c >>>>>> b/drivers/mtd/parsers/bcm63xxpart.c >>>>>> index 78f90c6c18fd..493a75b2f266 100644 >>>>>> --- a/drivers/mtd/parsers/bcm63xxpart.c >>>>>> +++ b/drivers/mtd/parsers/bcm63xxpart.c >>>>>> @@ -22,6 +22,9 @@ >>>>>> #include <linux/mtd/partitions.h> >>>>>> #include <linux/of.h> >>>>>> >>>>>> +#include <asm/bootinfo.h> >>>>>> +#include <asm/fw/cfe/cfe_api.h> >>>>> >>>>> Are you sure both includes are needed? >>>> >>>> asm/bootinfo.h is needed for fw_arg3 and asm/fw/cfe/cfe_api.h is needed >>>> for CFE_EPTSEAL. >>>> >>>>> >>>>> I don't think it is a good habit to include asm/ headers, are you sure >>>>> there is not another header doing it just fine? >>>> >>>> Both are needed unless you want to add another definition of CFE_EPTSEAL >>>> value. >>>> There are currently two CFE magic definitions, the one in >>>> asm/fw/cfe/cfe_api.h and another one in bcm47xxpart.c: >>>> https://github.com/torvalds/linux/blob/master/arch/mips/include/asm/fw/cfe/cfe_api.h#L28 >>>> https://github.com/torvalds/linux/blob/master/drivers/mtd/parsers/bcm47xxpart.c#L33 >>>> >>> >>> The caveat with that approach is that this reduces the compilation >>> surface to MIPS and BMIPS_GENERIC and BCM63XX only, which is a bit >>> small. If we could move the CFE definitions to a shared header, and >>> consolidate the value used by bcm47xxpart.c as well, that would allow us >>> to build the bcm63xxpart.c file with COMPILE_TEST on other >>> architectures. This does not really have functional value, but for >>> maintainers like Miquel, it allows them to quickly test their entire >>> drivers/mtd/ directory. >> >> I don’t think fw_arg3 available on non mips archs, is it? >> I’m happy to move it to a shared header (which would be a good location for >> this?), but if I’m right it would still be restricted to MIPS. > > Restricting a definition to MIPS, even if it makes sense for you is > very limiting for me. I need to be able to build as much drivers as I > can from my laptop and verify they at least compile correctly. If I need > a MIPS toolchain, an ARC toolchain, a PowerPC, an ARM, an ARM64 and > whatever other funky toolchain to do just that in X steps: it's very > painful. We have been adding COMPILE_TEST dependencies on as much > drivers as we could and we want to continue moving forward. Using such > include would need to drop the COMPILE_TEST condition from Kconfig and > this is not something I am willing to do.
I totally understand and agree with your point, but I still think that there could be a solution which would be valid for both of us. > > Thanks for your understanding :) The current way of detecting CFE isn’t the proper one. Thank you for understanding that too. > Miquèl Best regards, Álvaro.
