Ashish,

Please use proper quotation ">" when you reply.

On 01/30/2018 06:24 AM, Ashish Kumar wrote:
> 
> 
> -----Original Message-----
> From: York Sun 
> Sent: Tuesday, January 30, 2018 2:54 AM
> To: Ashish Kumar <ashish.ku...@nxp.com>; u-boot@lists.denx.de
> Subject: Re: [PATCH] armv8: ls1088aqds: Add IFC-NOR as boot source for LS1088
> 
> On 01/01/2018 09:24 PM, Ashish Kumar wrote:
>> IFC-NOR and QSPI-NOR pins are muxed on SoC,so they cannot be accessed 
>> simultaneously, but IFC-NOR can be accessed along with SD-BOOT.
>>
>> ls1088aqds_sdcard_ifc_defconfig: is defconfig for SD as boot source 
>> and IFC-NOR to be used as flash, this will be used to write IFC-NOR 
>> image on IFC flash.
>> QSPI and DSPI cannot be accessed in this defconfig.
>>
>> IFC-NOR image is generated by ls1088aqds_defconfig.
>>
>> Signed-off-by: Ashish Kumar <ashish.ku...@nxp.com>
>> ---
>> depends on: 
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
>> chwork.ozlabs.org%2Fpatch%2F853615%2F&data=02%7C01%7Cyork.sun%40nxp.co
>> m%7Cc53b3ec4db4d48e5130a08d551a1253d%7C686ea1d3bc2b4c6fa92cd99c5c30163
>> 5%7C0%7C0%7C636504674905515652&sdata=j2VD3l2%2BIb7iYk8yvn3TOrvT38pPT8A
>> RRQjN0h%2FR5b0%3D&reserved=0
>>
>> Tested on 2018.01-rc3
>>
>> More accurate timing are used which where provided by validation team
>>
>> to switch to IFC-NOR use command for qspi prompt:
>> i2c mw 66 0x60 0x12; i2c mw 66 50 00;i2c mw 66 10 21
> 
> <snip>
> 
>> diff --git a/include/configs/ls1088aqds.h 
>> b/include/configs/ls1088aqds.h index 8fbf890..d5075c3 100644
>> --- a/include/configs/ls1088aqds.h
>> +++ b/include/configs/ls1088aqds.h
>> @@ -27,7 +27,6 @@ unsigned long get_board_ddr_clk(void);
>>  #define CONFIG_SYS_MMC_ENV_DEV              0
>>  #define CONFIG_ENV_SIZE                     0x2000
>>  #else
>> -#define CONFIG_ENV_IS_IN_FLASH
>>  #define CONFIG_ENV_ADDR                     (CONFIG_SYS_FLASH_BASE + 
>> 0x300000)
>>  #define CONFIG_ENV_SECT_SIZE                0x20000
>>  #define CONFIG_ENV_SIZE                     0x20000
>> @@ -41,8 +40,9 @@ unsigned long get_board_ddr_clk(void);
>>  #define CONFIG_SYS_CLK_FREQ         100000000
>>  #define CONFIG_DDR_CLK_FREQ         100000000
>>  #else
>> -#define CONFIG_SYS_CLK_FREQ         get_board_sys_clk()
>> -#define CONFIG_DDR_CLK_FREQ         get_board_ddr_clk()
>> +#define CONFIG_QIXIS_I2C_ACCESS
>> +#define CONFIG_SYS_CLK_FREQ         100000000
>> +#define CONFIG_DDR_CLK_FREQ         100000000
> 
> Are you sure you want to go down the path to hard-code clock speeds? You will 
> lose the ability to change clocks. Besides, you have identical hard-coded 
> value for both legs of the #if...#else..#endif.
> 
> I found that I was not able to access QIXIS_READ etc, so I had to revert to 
> fixed values.

You need to find the root cause. Qixis is on both IFC and I2C. Does NOR
flash takes the chip-select FPGA was using?

> 
> 
>>  #endif
>>  
>>  #define COUNTER_FREQUENCY_REAL              (CONFIG_SYS_CLK_FREQ/4)
>> @@ -87,16 +87,10 @@ unsigned long get_board_ddr_clk(void);
>>      CSPR_MSEL_NOR                                           | \
>>      CSPR_V)
>>  #define CONFIG_SYS_NOR_CSOR CSOR_NOR_ADM_SHIFT(12)
>> -#define CONFIG_SYS_NOR_FTIM0        (FTIM0_NOR_TACSE(0x4) | \
>> -                            FTIM0_NOR_TEADC(0x5) | \
>> -                            FTIM0_NOR_TEAHC(0x5))
>> -#define CONFIG_SYS_NOR_FTIM1        (FTIM1_NOR_TACO(0x35) | \
>> -                            FTIM1_NOR_TRAD_NOR(0x1a) |\
>> -                            FTIM1_NOR_TSEQRAD_NOR(0x13))
>> -#define CONFIG_SYS_NOR_FTIM2        (FTIM2_NOR_TCS(0x4) | \
>> -                            FTIM2_NOR_TCH(0x4) | \
>> -                            FTIM2_NOR_TWPH(0x0E) | \
>> -                            FTIM2_NOR_TWP(0x1c))
>> +#define CONFIG_SYS_NOR_FTIM0        (0xc0201410)
>> +#define CONFIG_SYS_NOR_FTIM1        (0x50009028)
>> +#define CONFIG_SYS_NOR_FTIM2        (0x0820501c)
>> +#define CONFIG_SYS_NOR_FTIM3        0x04000000
> 
> This is bad coding! Please use macros. Please triple-check your timing.
> I don't want to fix the timing again. See my commit for LS1046AQDS 1b7910a37c.
> 
> How do I find timing which are correct?, I borrowed the same from validation 
> scripts used by validation team.
> Should I use these "LS1046AQDS 1b7910a37c"

If you know what you are doing, calculate the timing. If you don't, use
my timing as suggested. Do not use magic numbers.

> 
>>  #define CONFIG_SYS_NOR_FTIM3        0x04000000
>>  #define CONFIG_SYS_IFC_CCR  0x01000000
>>  
>> @@ -193,7 +187,7 @@ unsigned long get_board_ddr_clk(void);
>>                                      | CSPR_MSEL_GPCM \
>>                                      | CSPR_V)
>>  
>> -#define CONFIG_SYS_FPGA_AMASK               IFC_AMASK(64*1024)
>> +#define SYS_FPGA_AMASK              IFC_AMASK(64 * 1024)
>>  #if defined(CONFIG_QSPI_BOOT) || defined(CONFIG_SD_BOOT_QSPI)
>>  #define CONFIG_SYS_FPGA_CSOR                CSOR_GPCM_ADM_SHIFT(0)
>>  #else
>> @@ -222,7 +216,7 @@ unsigned long get_board_ddr_clk(void);
>>  #define CONFIG_SYS_CSPR2_EXT                CONFIG_SYS_FPGA_CSPR_EXT
>>  #define CONFIG_SYS_CSPR2            CONFIG_SYS_FPGA_CSPR
>>  #define CONFIG_SYS_CSPR2_FINAL              SYS_FPGA_CSPR_FINAL
>> -#define CONFIG_SYS_AMASK2           CONFIG_SYS_FPGA_AMASK
>> +#define CONFIG_SYS_AMASK2           SYS_FPGA_AMASK
>>  #define CONFIG_SYS_CSOR2            CONFIG_SYS_FPGA_CSOR
>>  #define CONFIG_SYS_CS2_FTIM0                SYS_FPGA_CS_FTIM0
>>  #define CONFIG_SYS_CS2_FTIM1                SYS_FPGA_CS_FTIM1
>> @@ -258,13 +252,13 @@ unsigned long get_board_ddr_clk(void);
>>  #define CONFIG_SYS_CS2_FTIM3                CONFIG_SYS_NAND_FTIM3
>>  #define CONFIG_SYS_CSPR3_EXT                CONFIG_SYS_FPGA_CSPR_EXT
>>  #define CONFIG_SYS_CSPR3            CONFIG_SYS_FPGA_CSPR
>> -#define CONFIG_SYS_CSPR3_FINAL              CONFIG_SYS_FPGA_CSPR_FINAL
>> -#define CONFIG_SYS_AMASK3           CONFIG_SYS_FPGA_AMASK
>> +#define CONFIG_SYS_CSPR3_FINAL              SYS_FPGA_CSPR_FINAL
>> +#define CONFIG_SYS_AMASK3           SYS_FPGA_AMASK
>>  #define CONFIG_SYS_CSOR3            CONFIG_SYS_FPGA_CSOR
>> -#define CONFIG_SYS_CS3_FTIM0                CONFIG_SYS_FPGA_CS_FTIM0
>> -#define CONFIG_SYS_CS3_FTIM1                CONFIG_SYS_FPGA_CS_FTIM1
>> -#define CONFIG_SYS_CS3_FTIM2                CONFIG_SYS_FPGA_CS_FTIM2
>> -#define CONFIG_SYS_CS3_FTIM3                CONFIG_SYS_FPGA_CS_FTIM3
>> +#define CONFIG_SYS_CS3_FTIM0                SYS_FPGA_CS_FTIM0
>> +#define CONFIG_SYS_CS3_FTIM1                SYS_FPGA_CS_FTIM1
>> +#define CONFIG_SYS_CS3_FTIM2                SYS_FPGA_CS_FTIM2
>> +#define CONFIG_SYS_CS3_FTIM3                SYS_FPGA_CS_FTIM3
> 
> These changes have nothing to do with IFC-NOR.
> 
> 
> If I keep CONFIG_SYS_FPGA_CS_FTIM3, it ask to move to Kconfig. Should I move 
> these out in separate patch ?
> 

First, these macros are local. They don't need to be prefixed with
CONFIG_. Second, FPGA timing has nothing to do with this patch. Please
send a separated cleanup patch.

York
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to