On Mon, 26 Jan 2015 10:26:34 +0100 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
On 26.01.2015 09:38, Stefan Tauner wrote:
On Sun, 25 Jan 2015 03:04:09 +0100 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
On 19.01.2015 21:39, Stefan Tauner wrote:
Does (cross-)compile but is not run-tested.
Signed-off-by: Stefan Tauner stefan.tauner@alumni.tuwien.ac.at Acked-by: Stefan Tauner stefan.tauner@alumni.tuwien.ac.at
Makefile | 2 +- hwaccess.h | 8 ++++++++ platform.h | 5 ++++- 3 files changed, 13 insertions(+), 2 deletions(-)
hwaccess.c needs to be patched as well. Specifically, static inline void sync_primitive(void) needs either a comment why sync_primitive is unneeded or a a code snippet with correct code.
Not sure if I read http://lxr.free-electrons.com/source/arch/sparc/include/asm/barrier_64.h#L47 correctly, but we may need
__asm__ __volatile__("membar #StoreLoad":::"memory")
Mh. That might not be needed if /dev/mem uses a special access mode which enforces memory access ordering in Sparc. Someone else with Sparc knowledge needs to check this.
That said, the patch looks correct apart from the missing hwaccess.c stuff mentioned above.
I doubt that we will find something competent enough to answer that quickly. What about adding the information above to hwaccess.c so that it is available in-tree to everybody in case something does not work without a memory barrier?
"Get it to compile first, work about correctness later"... not exactly a flashrom principle, but in this case it helps with code coverage and eliminates blacklists in package building, so be it. Are there any architectures left for building on modern debian? AArch64, Alpha? Adding the barrier info as comment is probably the best way to do it, and maybe disabling all non-USB/non-serial programmers. That would also completely eliminate the effect of memory barriers.
That said, the least we can do is make sure the internal programmer is never built for SPARC (no code to do anything useful).
PCI "port" programmers are disabled for anything != x86 AFAICS. "Normal/other" PCI programmers are enabled everywhere in general AFAICT. Is that really true?
The internal programmer is a bit complicated and I am not sure how to proceed. It would be great to document the status quo somewhere in the code: which architectures support which features and require which libraries and what needs to be done to get the others working. Partially this is done already with the error messages within internal_init() but the code is much too complicated to get a consistent view easily.
One interesting bit for example is this: /* FIXME: Remove this unconditional abort once all PCI drivers are * converted to use little-endian accesses for memory BARs. */ That explains why the internal programmer is completely disabled for little-endian devices... I am just not sure if it is still valid. AFAICS the drivers all use the hwaccess.c function to access PCI space and those have implicit endian conversions built in. Can you give me an example of a failing driver routine please?
Can you please try to document the internal programmer and PCI-related stuff somewhere in one or two spots (e.g. above internal_init() and/or hwaccess.c)? Maybe a abbreviated version of that should end up in an autogenerated wiki table eventually. I just don't have an idea how yet... but that might get better when I know what I am talking about :)
For the release I think we should completely disable all PCI-based modules (including the internal programmer) on SPARC. Is there an easy way to do that or do I need something similar to the "PCI port" disabling that checks and sets all related CONFIG_ programmer variables to no and adds it to UNSUPPORTED_FEATURES instead. Also, I wonder if that should be done for other architectures as well. I really can't tell :/