Re: [flashrom] [PATCH 7/8] ichspi:c add support for Intel Hardware Sequencing

[Carl-Daniel Hailfinger]

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. 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.

Regards,
Carl-Daniel