[U-Boot] [PATCH 2/6] mx6q: Add support for ECSPI through mxc_spi driver

[Stefano Babic]

On 18/01/2012 02:44, Eric Nelson wrote:
> On 01/17/2012 06:27 PM, Marek Vasut wrote:
>>> On 01/17/2012 04:19 PM, Marek Vasut wrote:
>>>>> Signed-off-by: Eric Nelson<eric.nelson at boundarydevices.com>
>>>>> +/* ECSPI registers */
>>>>> +struct cspi_regs {
>>>>> +    u32 rxdata;
>>>>> +    u32 txdata;
>>>>> +    u32 ctrl;
>>>>> +    u32 cfg;
>>>>> +    u32 intr;
>>>>> +    u32 dma;
>>>>> +    u32 stat;
>>>>> +    u32 period;
>>>>> +};
>>>>
>>>> Sigh ... it's no fun I can have only one remark :-)
>>>>
>>>> Is this part common for all imx-es ?
>>>
>>> All i.MX6's
>>>
>>> This is a cut&  paste from MX51.
>>>
>>> I was tempted to introduce an 'mxc_ecspi.h' to merge the declaration
>>> for i.MX5x and i.MX6 which share the ECSPI peripheral and 'mxc_cspi.h'
>>> for i.MX31 and i.MX35 that share the CSPI peripheral.
>>
>> But you don't even need this outside of the spi driver so just put it
>> into the
>> spi driver and be done with it. That'll solve your duplication issue.
>>
>> M
>>
> 
> I'll defer to Stefano on this one, since I did this in response
> to his request:

Yes, I admit I am guilty about this !

The layout of the CSPI registers is not exactly the same for all SOCs.
For example, the MXC_CSPICTRL_TC has a different position, as well as
the BITCOUNT (because the MX31 can send less bits in one shot) and
MXC_CSPICTRL_CHIPSELECT.

So they are similar, but not exactly the same.

Then we have the MX5 registers. Even if we check the CSPI (not eCSPI)
controller, the layout of the registers is different compared to the MX3
SOCs.

> The struct cspi_regs is already present in mx31, mx35, and mx51 headers,
> so I'm not breaking new ground here, only in the bitfield declarations.

Right, I see the same. The cspi_regs structure is already defined into
the imx_regs.h, only the bit layout was moved. And as the bit layout is
SOC dependent, I think the right place for it is inside the imx-regs.h
registers.

> My interpretation of Stefano's intent is to clean up the driver at the
> expense
> of extra defines in the arch-specific headers.

Yes, you're right - of course, I am open also to other solutions if they
are proofed to be better ;-).

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================