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

[Faiz Abbas]

Hi Vignesh,

On 07/09/20 5:49 pm, Vignesh Raghavendra wrote:
> 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).

I see. I'll make the change in v2.

> 
>>>
>>>>>> +#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?
> 
Ok. I'll move the base address to the board file.

Thanks,
Faiz