On Thu, 30 Jun 2011 23:49:39 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Am 28.06.2011 05:33 schrieb Stefan Tauner:
--- a/flashchips.c +++ b/flashchips.c @@ -8507,7 +8507,27 @@ const struct flashchip flashchips[] = { .write = write_jedec_1, .read = read_memmapped, },
+#if defined(CONFIG_INTERNAL) && (defined(__i386__) || defined(__x86_64__))
- {
.vendor = "Intel",.name = "Hardware Sequencing",.bustype = CHIP_BUSTYPE_SPI,.manufacture_id = INTEL_ID,.model_id = INTEL_HWSEQ,.total_size = 0,.page_size = 256,.tested = TEST_OK_PREW,.probe = ich_hwseq_probe,.block_erasers ={{ /* erase blocks will be set by the probing function */.block_erase = ich_hwseq_block_erase,}},.write = ich_hwseq_write_256,.read = ich_hwseq_read,- },
+#endif // defined(CONFIG_INTERNAL) && (defined(__i386__) || defined(__x86_64__)) { .vendor = "AMIC", .name = "unknown AMIC SPI chip",
I consider a chip called "Hardware Sequencing" to be a really evil thing. Hooking up a programmer with the help of a programmer-specific flash chip seems to be quite popular, and it was rejected each time and an alternative solution was found. That said, if probing for the real chip ID is impossible here and you only have read/write/erase primitives without any SPI access, this is essentially not a SPI bus chip, but a programmer-specific bus chip.
one could argue that software sequencing does not allow real SPI access either, only flashrom's architecture is working around it because it is versatile enough (and ichspi.c quite complex) and swseq allows the minimum set of operations (if not totally locked down). that's true for other programmers too (you named it87 can't handle what i naturally expected to be supported in the SFDP patch thread). so there is no clear line between "real" programmers and opaque flashing interfaces with a hidden SPI backend.
i do agree though that the line should be drawn here, or at least no further.
The struct flashchip should probably look more like this:
+#if defined(CONFIG_INTERNAL) && (defined(__i386__) || defined(__x86_64__))
- {
.vendor = "Unknown",.name = "Abstract opaque chip",.bustype = CHIP_BUSTYPE_ABSTRACT,.manufacture_id = ABSTRACT_MANUF_ID,.model_id = ABSTRACT_MODEL_ID,.total_size = 0,.page_size = 0,.tested = TEST_OK_PREW,.probe = abstract_probe, //abstract_probe points to programmer->abstract_probe which points to ich_hwseq_probe.block_erasers ={{ /* erase blocks will be set by the probing function */.block_erase = abstract_erase, // points to programmer->abstract_erase which points to ich_hwseq_block_erase}},.write = ich_hwseq_write_256,Same abstract_ game as above.
.read = ich_hwseq_read,Dito.
- },
+#endif // defined(CONFIG_INTERNAL) && (defined(__i386__) || defined(__x86_64__)) { .vendor = "AMIC", .name = "unknown AMIC SPI chip",
Yes, the programmer struct would have to be extended for this, but at least that allows us to drive anything without having to care about the bus if we can't access the bus anyway.
hm. your register parallel programmer patch does split the parallel part from the generic programmer core (good stuff!). that opaque thingy should probably reside in the core? or its own struct? anyway, i need a way to register or mark it active/supported. suggestions?
is it possible that there are multiple opaque interfaces? this is also a problem with my approach of course.
besides that i dont see any problems with the approach you have outlined, but the above decisions must be finalized and i guess your parallel programmer register patch should be in, before i start that.