[flashrom] [PATCH 2/3] Add support for SPARC (maybe).

Stefan Tauner stefan.tauner at alumni.tuwien.ac.at
Tue Jan 27 09:37:01 CET 2015


On Mon, 26 Jan 2015 10:26:34 +0100
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at 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 at 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 at alumni.tuwien.ac.at>
> >>> Acked-by: Stefan Tauner <stefan.tauner at 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 :/

-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner




More information about the flashrom mailing list