Re: [RFC/PATCHv2 3/3] spi: dw-spi: Pointers select 16b vs. 32b DesignWare access

[Thor Thayer <tthayer@opensource.altera.com>]

On 03/10/2015 03:44 PM, Andy Shevchenko wrote:
> On Tue, Mar 10, 2015 at 10:34 PM, Thor Thayer
> <tthayer@opensource.altera.com> wrote:
>> On 03/09/2015 03:02 PM, Andy Shevchenko wrote:
>>>
>>> On Mon, Mar 9, 2015 at 9:47 PM, Thor Thayer
>>> <tthayer@opensource.altera.com> wrote:
>>>>
>>>> On 03/09/2015 01:54 PM, Andy Shevchenko wrote:
>>>
>>>
>>>> Yes, I just need the 32 bit write. I was trying to remain consistent but
>>>> I
>>>> agree that only changing only writes would minimize the changes.
>>>
>>>
>>> Which is still makes me anxious.
>>>
>>> I have briefly read a chapter for SPI in pdf you sent link to. I
>>> didn't find anything except SPI supports 32 bit data width.
>>>
>>> It would be nice if you ping your HW engineers and clarify what
>>> exactly is happening there.
>>>
>>
>> I confirmed with our HW engineer that writes are required to be 32 bits. We
>> are in the process of updating our documentation to explicitly say this.
>>
>> Since there are no byte enables on our 32 bit APB interface, writing only 1
>> or 2 bytes could corrupt the other bytes in the 32 bit word. The 32 bit
>> write enforces requiring an explicit read/modify/write for less than 32 bit
>> writes.
>>
>> Since reads are non-destructive (nothing is being written back into the
>> register), it is safe for all reads to get promoted to 32 bit reads.
>
> Thanks for detailed explanation!
>
>> Is it preferable to have both 32 bit reads and writes for consistency?
>>
>> Or is it preferable to make the minimal number of changes which would be to
>> only add a 32 bit write function?
>
> Both I think just to be more clear.
>
> However, I'm now considering what if we just replace
> dw_{write/read}w() by l-variants without any additional DT property
> and accessor functions?
> Would it work for both your cases (old chip, new chip)?
> On my side I may test this on Intel MID.
>

Yes, this would be the simplest solution. The l-variant certainly works 
on our legacy SoCs. I'll be curious to hear your testing results.

The data sheet mentions that registers are addressed at 32-bit 
boundaries to remain consistent with the AHB bus (Section 6.1 of 
dw_apb_ssi_db.pdf).  Additionally unused bits are reserved for writes 
and 0 for reads so this seems like a good solution.

My concern is the presence of legacy devices that I have no way of 
testing. Is a Request For Test in the body of the patch sufficient?