On Mon, Jul 28, 2008 at 12:45:11PM +0100, Catalin Marinas wrote: > On Sun, 2008-07-27 at 12:08 +0100, Russell King - ARM Linux wrote: > > On Wed, Jul 23, 2008 at 03:10:45PM -0700, Andrew Morton wrote: > > > Well. Rather than doing things sequentially we could go parallel. > > > Russell could say "I'll look at them before 2.6.28 but please put them > > > into linux-next meanwhile". > > > > There have been some valid but (iirc) non-vocal objections to the > > Thumb-2 support from a few people. The biggest one is all the > > mess associated with supporting this "unified" assembler stuff. > > My first implementation of these patches (last year) created a separate > arch/ directory and a lot of people objected to this recommending to > merge it into the existing arch/arm. Once I posted the re-worked > implementation there were no big . I'm not surprised. People don't want to duplicate their machine support across two arch/arm for the same family of SoCs. > I agree that the syntax isn't as readable as before but this is mainly > because we need to support older binutils via conditional compilation > (the unified assembly syntax itself isn't as bad). IIRC, some people > were OK with this as long as, at some point, the conditional compilation > can be removed once most of the binutils in use support it (I think this > syntax is already 2 years old). How is that realistically achievable given the issues that I've raised over the impossibility of review and the issues with gcc not knowing how many instructions an asm() statement generates? > If you don't like the unified assembly syntax at all, I can't help much > (not even argue whether it makes sense or not, I wasn't involved in > designing it and it's pretty late to change it now anyway). If neither of us can "help much", then T2 won't be going in, and ARM Ltd is going to have to accept that T2 isn't going to be supported by mainline Linux. However, there is another solution to this: ARM Ltd could write a static checker to ensure that the 'itxxxx' instructions match the conditions in the following instructions, resulting in disagreements between them being spotted at build time, rather than having some data corrupting bug lurking around. I totally disagree with getting rid of those conditional suffixes even for T2 only code - doing so will make my objection even stronger against T2. Rather than getting rid of the conditional suffixes, use them as a continuing existing check that the 'itxxxx' instructions are correct. Then we can pick up on things like merge errors, bad patches, wrong 'itxxxx' covering the following instructions, etc. I think I've proven my point well - at least one of those 'itxxxx' instructions in your patch is wrong, but not obviously wrong from reading the patches. That's only one which looked at how the resulting code actually turns out. I didn't thoroughly check the others, so there could be other issues lurking in the missing patch context lines. > IMHO, I don't think the assembly syntax for an architecture is a valid > reason to reject patches. My objection is primerily on the readability of the code in the ARM kernel, and the viability of continued maintainence of it - which is an extremely important consideration and one which trumps *everything* else - and it's that basis which I'm rejecting the patches. That objection is directly caused by this crappy assembly syntax. So you can turn it into something political by saying that it's the assembly syntax I'm objecting to. My objection is _far_ more than that. > > My view is that I really don't like these patches; they make the > > code very unreadable and more prone to errors. They're going to > > cause us lots of problems in the future. Every git merge which > > touches any ARM file containing assembly is going to have to be > > _very_ carefully reviewed, and it's not always possible to catch > > all of those. > > > > The only suggestion I have to improving the situation is to recommend > > that someone rethinks their wizzy new assembly language format - which > > IMHO is currently only fit for "write once, read or modify never" > > assembly code. > > There were some public discussions in the past with Richard Earnshaw > (gcc/binutils) here in ARM regarding the assembly syntax but I don't > think it can be changed in a fully backwards compatible way while > supporting a slightly different instruction set. Were there? That's the first I've heard of them! The use of "public" here is actually rather annoying. They may have been "public" from the point of view of being on some random mailing list on the Internet, but that doesn't mean that everyone concerned knows that they're going on. And no one person can have a finger in every pie, or be subscribed to every mailing list on the planet and be able to track what's going on. I don't consider the discussions here as "public". They're "public" in the sense of addressing this community, but if we were to talk about changing the glibc<->kernel interface here only, and then claim to the glibc people "but the discussions were public" they'd laugh at us. > ???I would really like some clear statement from you to know where to > focus my attention (clean up the current patches even more or create > separate arch/arm-t2). AFAICT, you don't really like these patches > merged, in which case I'd have to focus on the second option. I've put my point of view forward in the responses to the patches. I'm not sure what "clear statement" you're really after. Also, I'm not entirely sure why there's a body of people who grumble about this stuff, but don't post their concerns... Maybe if that ARM Linux developers conference had gone ahead, this could have been discussed there in a broader manner and some way forward could have been reached. -- To unsubscribe from this list: send the line "unsubscribe linux-next" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
