On 07/31/2017 10:54 PM, Alexandru Gagniuc wrote:
> Hi Marek,
>
> Me again!
>
> On 07/29/2017 02:34 AM, Marek Vasut wrote:
>> On 07/29/2017 12:07 AM, Alexandru Gagniuc wrote:
>>> +static void aspi_drain_fifo(struct anarion_qspi *aspi, uint8_t *buf,
>>> size_t len)
>>> +{
>>> + uint32_t data;
>>
>> Is this stuff below something like ioread32_rep() ?
>>
> [snip]
>>> + aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
>>> + while (len >= 4) {
>>> + memcpy(&data, buf, sizeof(data));
>>> + aspi_write_reg(spi, ASPI_REG_DATA1, data);
>>
>> iowrite32_rep ?
>>
>>> + buf += 4;
>>> + len -= 4;
>>> + }
>
> I looked at using io(read|write)32_rep in these two places, and I've run
> into some issues.
>
> First, I'm seeing unaligned MMIO accesses, which are not supported on
> ARC. Note that 'buf' has an alignment of 1, while the register requires
> an alignment of 4. The memcpy() in-between takes care of that, which was
> the original intent.
>
> Other than that, we still need to break off the tail because we need to
> update ASPI_REG_BYTE_COUNT before writing/reading any more data from the
> FIFO. We have to keep track of the remainder, so we're not really saving
> any SLOC.
>
> I'd like to keep the original version as I find it to be much more
> symmetrical and readable.
Look at the other drivers, AFAIR the io*_rep() issue was solved over and
over again, maybe you can factor that code out.
> Thanks,
> Alex
>
>>> +
>>> + if (len) {
>>> + aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, len);
>>> + memcpy(&data, buf, len);
>>> + aspi_write_reg(spi, ASPI_REG_DATA1, data);
>>> + }
>>> +}
>
>
> Exhibit A: aspi_seed_fifo with writesl()
>
> static void aspi_seed_fifo(struct anarion_qspi *spi,
> const uint8_t *buf, size_t len)
> {
> uint32_t data;
> void __iomem *data_reg = (void *)(spi->regbase + ASPI_REG_DATA1);
>
> aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
> writesl(data_reg, buf, len / 4);
> buf += len & ~0x03;
> len &= 0x03;
>
> if (len) {
> aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, len);
> memcpy(&data, buf, len);
> aspi_write_reg(spi, ASPI_REG_DATA1, data);
> }
> }
--
Best regards,
Marek Vasut
