On 18 October 2013 12:38, Stefan Hajnoczi <stefa...@gmail.com> wrote: > On Thu, Oct 17, 2013 at 07:30:02PM -0700, Roy Franz wrote: >> For buffered writes, mask the length with the maximum supported >> length. This is required for block writes to work on the ARM vexpress >> platform, where the flash interface is 32 bits wide. For buffered writes >> to the 2 16 bit flashes on the interface, the length is repeated in each >> 16 bit word, and without this mask the two lengths are interpreted >> as a single 32 bit value that is very large.
>> @@ -378,6 +378,7 @@ static void pflash_write(pflash_t *pfl, hwaddr offset, >> >> break; >> case 0xe8: >> + value &= pfl->writeblock_size - 1; > > This patch feels weird. Should the 32-bit interface width be truncated > down to 16 bits before dispatching pflash_write()? > > It's not clear to me that truncating just in this specific case is > correct. But then I don't know the hardware :). I think the problem may be that we're incorrectly modelling the hardware's "2 side-by-side 16 bit wide flash chips" as "one 32 bit wide flash chip", because our flash device code doesn't support doing the former. Probably instead of a single "width" property we should have two, similar to the device tree binding's pair: - bank-width : Width (in bytes) of the bank. Equal to the device width times the number of interleaved chips. - device-width : (optional) Width of a single mtd chip. If omitted, assumed to be equal to 'bank-width'. However I'm not very familiar with how flash hardware works... -- PMM