[PATCH] map-ram chip driver: 16-bit block transfer

[Arnd Bergmann]

On Wed, Aug 9, 2017 at 8:10 PM, Boris Brezillon
<boris.brezillon at free-electrons.com> wrote:
> +Arnd for the memcpy_to/fromio aspects.
>
> Hi Matt,
>
> Subject should be something like "mtd: map: support 16-bit block
> transfers"
>
> Le Fri,  4 Aug 2017 08:02:20 -0500,
> Matt Weber <matthew.weber at rockwellcollins.com> a écrit :
>
>> From: sgtandel <sanjay.tandel at rockwellcollins.com>
>>
>> This patch adds 16-bit I/O memory write function (map_copy_to16) in map-ram
>> driver. map-ram driver's block write function(i.e map_copy_to) uses
>> memcpy_toio() function for block transfer. memcpy_toio() is limited to do
>> single byte write, irrespective of bankwidth.
>
> That's not true. Depending on the platform (+ config options)
> memcpy_to/fromio() can be a simple redirection to memcpy, and
> memcpy is likely to be optimized to do 32bit or 64bit accesses when
> possible (IOW, when alignment allows it).

right. I think we also have a helper that always does 32-bit
transfers, but I can't find the name at the moment.

>> +{
>> +     const unsigned char *t = to;
>> +
>> +     if (!(IS_ALIGNED((u32)from, sizeof(u16)))) {
>> +             from = (void __iomem *)ALIGN((u32)from, sizeof(u16))
>> +                                                     - sizeof(u16);
>> +             *(u8 *)t = (u8)((readw(from) & 0xff00) >> 8);
>> +             count--;
>> +             t++;
>> +             from += 2;
>> +     }
>> +     while (count >= 2) {
>> +             *(u16 *)t = readw(from);
>> +             count -= 2;
>> +             t += 2;
>> +             from += 2;
>> +     };
>> +     while (count > 0) {
>> +             *(u8 *)t = (u8)(readw(from) & 0x00ff);
>> +             count--;
>> +             t++;
>> +             from++;
>> +     }
>> +}

This looks wrong on bit-endian kernels, which do a byte-swap
in readw. This is one of the few places that probably should use
__raw_readw().

>>  static inline void inline_map_copy_from(struct map_info *map, void *to, unsigned long from, ssize_t len)
>>  {
>>       if (map->cached)
>>               memcpy(to, (char *)map->cached + from, len);
>> +     else if (map_bankwidth_is_2(map))
>> +             map_copy_from16(to, map->virt + from, len);
>>       else
>>               memcpy_fromio(to, map->virt + from, len);
>
> We're likely to face the same problem with map_bankwidth_is_4()
> (AKA 32bit busses), right? Why not patching the code to handler this
> case?
>
> BTW, I'm not sure what kind of guarantees are provided by
> memcpy_to/fromio(), but I'd expect them to optimize things when they
> can, so they might use 32bit or 64bit accesses when alignment
> authorizes it. Will that work correctly even on 8bit chips?

I think it may or may not work, depending on the particular bus
interface.

        Arnd