[PATCH 5/5] arm: mach-omap2: am33xx: Add device structure for spi

[Vignesh Raghavendra]

Hi,

On 9/7/20 4:02 PM, Faiz Abbas wrote:
> Hi Vignesh,
> 
> On 07/09/20 1:48 pm, Vignesh Raghavendra wrote:
>>
>>
>> On 9/7/20 12:36 PM, Faiz Abbas wrote:
>>> Hi Lokesh,
>>>
>>> On 07/09/20 12:08 pm, Lokesh Vutla wrote:
>>>>
[...]
>>>>>  struct omap3_spi_priv {
>>>>>  	struct mcspi *regs;
>>>>>  	unsigned int cs;
>>>>> diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
>>>>> index 9c4ef369c5..db1a89ad30 100644
>>>>> --- a/include/configs/am335x_evm.h
>>>>> +++ b/include/configs/am335x_evm.h
>>>>> @@ -281,6 +281,10 @@
>>>>>  #endif
>>>>>  
>>>>>  /* SPI flash. */
>>>>> +#if CONFIG_IS_ENABLED(DM_SPI)
>>>>> +#define AM33XX_SPI_BASE		0x48030000
>>
>> Could this be more specific? AM33XX_SPI<instance num>_BASE?
> 
> Isn't there only one McSPI instance in am335x?
> 

No, there are 2 SPI ports on AM335x (see arch/arm/dts/am33xx.dtsi).

>>
>>>>> +#define AM33XX_SPI_OFFSET	(AM33XX_SPI_BASE + OMAP4_MCSPI_REG_OFFSET)
>>>>
>>>> Can we get the SPI base from DT?
>>>>
>>>
>>> We are doing that in U-boot (see the ofdata_to_platdata() callback in patch 4).
>>> We need hardcoded static platdata for SPL. Was this not clear from the commit
>>> message?
>>>
>>
>> Then why not move these defines to arch/arm/mach-omap2/am33xx/board.c as
>> well?
> 
> All the other base addresses used in arch/arm/mach-omap2/am33xx/board.c are included
> from here. For example see UART platdata (struct ns16550_platdata am33xx_serial[]).
> 

UART is bad example as those #defines were added in 2012 which predate
platdata introduction...

Besides what happens when derivative of AM335x (see
include/configs/am335x_*.h) want to enable SPI boot? Would each such
files need to duplicate this snippet?