[U-Boot] [PATCH 0/1]
cover [U-Boot] [PATCH 0/1]
In order to activate the TZASC for the LS1021a board, one need to correctly configure the SMMU, by setting ICID for each peripherals in the SCFG ICID registers. The computation of the addresses of such registers is invalid due to wrong pointer arithmetic.
Also please note that there is no mention of the following entry in the Reference manual of the LS1021A board.
arch/arm/cpu/armv7/ls102xa/soc.c:138
struct smmu_stream_id dev_stream_id[] = {
...
{ 0x114, 0x06, "qDMA" },
...Vincent Siles (1):
arm: Fix SCFG ICID reg addresses
board/freescale/common/ls102xa_stream_id.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Signature · 1 line
1.9.1
PATCH 1/1 arm: Fix SCFG ICID reg addresses
On the LS102x boards, in order to initialize the ICID values of masters, the dev_stream_id array holds absolute offsets from the base of SCFG.
In ls102xa_config_ssmu_stream_id, the base pointer is cast to uint32_t * before adding the offset, leading to an invalid address. Casting it to unsigned char * solves the issue.
Also minor cosmetic renaming of uint32_t into u32 to be consistent in the whole file.
---
board/freescale/common/ls102xa_stream_id.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
board/freescale/common/ls102xa_stream_id.c +3 -3
@@ -10,11 +10,11 @@void ls102xa_config_smmu_stream_id (struct smmu_stream_id *id,uint32_t num) { -uint32_t *scfg = (uint32_t *)CONFIG_SYS_FSL_SCFG_ADDR ; +unsigned char *scfg = (unsigned char *)CONFIG_SYS_FSL_SCFG_ADDR ;int i;for (i =0 ; i < num; i++) -out_be32 (scfg + id[i].offset, id[i].stream_id); +out_be32 ((u32 *)(scfg + id[i].offset), id[i].stream_id); }void ls1021x_config_caam_stream_id (struct liodn_id_table *tbl,int size)
@@ -28,6 +28,6 @@ void ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, int size)else liodn = tbl[i].id[0 ]; -out_le32 ((uint32_t *)(tbl[i].reg_offset), liodn); +out_le32 ((u32 *)(tbl[i].reg_offset), liodn); } }
Signature · 1 line
1.9.1
PATCH 1/1 arm: Fix SCFG ICID reg addresses
Quoted 36 lines · Vincent Siles
On the LS102x boards, in order to initialize the ICID values of masters, the
dev_stream_idarray holds absolute offsets from the base of SCFG.In
ls102xa_config_ssmu_stream_id, the base pointer is cast touint32_t* before adding the offset, leading to an invalid address. Casting it to unsigned char * solves the issue.Also minor cosmetic renaming of
uint32_tinto u32 to be consistent in the whole file.Signed-off-by Vincent Siles <[email protected]>---
board/freescale/common/ls102xa_stream_id.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)board/freescale/common/ls102xa_stream_id.c +2
-2@@ -10,11 +10,11 @@void ls102xa_config_smmu_stream_id (struct smmu_stream_id *id,uint32_t num) { -uint32_t *scfg = (uint32_t *)CONFIG_SYS_FSL_SCFG_ADDR ; +unsigned char *scfg = (unsigned char *)CONFIG_SYS_FSL_SCFG_ADDR ;int i;for (i =0 ; i < num; i++) -out_be32 (scfg + id[i].offset, id[i].stream_id); +out_be32 ((u32 *)(scfg + id[i].offset), id[i].stream_id); }void ls1021x_config_caam_stream_id (struct liodn_id_table *tbl,int size)
board/freescale/common/ls102xa_stream_id.c void ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, int size) · line 33
Quoted 7 lines
board/freescale/common/ls102xa_stream_id.c +1
-1@@ -28,6 +28,6 @@ void ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, int size)else liodn = tbl[i].id[0 ]; -out_le32 ((uint32_t *)(tbl[i].reg_offset), liodn); +out_le32 ((u32 *)(tbl[i].reg_offset), liodn); } }
If the size of the pointer is an issue, maybe you ca use "void *"? Can you check if "struct ccsr_scfg" should/can be used?
York
PATCH 1/1 arm: Fix SCFG ICID reg addresses
Hi !
Quoted 48 lines · York Sun
Quoted 36 lines · Vincent Siles
On the LS102x boards, in order to initialize the ICID values of masters, the
dev_stream_idarray holds absolute offsets from the base of SCFG.In
ls102xa_config_ssmu_stream_id, the base pointer is cast touint32_t* before adding the offset, leading to an invalid address. Casting it to unsigned char * solves the issue.Also minor cosmetic renaming of
uint32_tinto u32 to be consistent in the whole file.Signed-off-by Vincent Siles <[email protected]>---
board/freescale/common/ls102xa_stream_id.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)board/freescale/common/ls102xa_stream_id.c +2
-2@@ -10,11 +10,11 @@void ls102xa_config_smmu_stream_id (struct smmu_stream_id *id,uint32_t num) { -uint32_t *scfg = (uint32_t *)CONFIG_SYS_FSL_SCFG_ADDR ; +unsigned char *scfg = (unsigned char *)CONFIG_SYS_FSL_SCFG_ADDR ;int i;for (i =0 ; i < num; i++) -out_be32 (scfg + id[i].offset, id[i].stream_id); +out_be32 ((u32 *)(scfg + id[i].offset), id[i].stream_id); }void ls1021x_config_caam_stream_id (struct liodn_id_table *tbl,int size)board/freescale/common/ls102xa_stream_id.c void ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, int size) · line 33
Quoted 7 lines
board/freescale/common/ls102xa_stream_id.c +1
-1@@ -28,6 +28,6 @@ void ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, int size)else liodn = tbl[i].id[0 ]; -out_le32 ((uint32_t *)(tbl[i].reg_offset), liodn); +out_le32 ((u32 *)(tbl[i].reg_offset), liodn); } }If the size of the pointer is an issue, maybe you ca use "void *"? Can you check if "struct
ccsr_scfg" should/can be used?
Quoted 1 line
York
By using the 'struct ccsr_scfg' type we won't be able to have the same kind of loop. Since the code is board dependent, I'm not sure it really matters.
Here what I would do instead. Tell me which style do you prefer.
Best,
Vincent
---
arch/arm/cpu/armv7/ls102xa/soc.c +16 -17
@@ -30,21 +30,21 @@ struct liodn_id_table sec_liodn_tbl[] = { SET_SEC_DECO_LIODN_ENTRY(7, 0x10, 0x10), }; -struct smmu_stream_id dev_stream_id[] = { - {0x100 ,0x01 ,"ETSEC MAC1" }, - {0x104 ,0x02 ,"ETSEC MAC2" }, - {0x108 ,0x03 ,"ETSEC MAC3" }, - {0x10c ,0x04 ,"PEX1" }, - {0x110 ,0x05 ,"PEX2" }, - {0x114 ,0x06 ,"qDMA" }, - {0x118 ,0x07 ,"SATA" }, - {0x11c ,0x08 ,"USB3" }, - {0x120 ,0x09 ,"QE" }, - {0x124 ,0x0a ,"eSDHC" }, - {0x128 ,0x0b ,"eMA" }, - {0x14c ,0x0c ,"2D-ACE" }, - {0x150 ,0x0d ,"USB2" }, - {0x18c ,0x0e ,"DEBUG" }, +struct smmu_stream_id dev_stream_id = { + .mac1 =0x01 , + .mac2 =0x02 , + .mac3 =0x03 , + .pex1 =0x04 , + .pex2 =0x05 , + .dma =0x06 , + .sata =0x07 , + .usb3 =0x08 , + .qe =0x09 , + .sdhc =0x0a , + .adma =0x0b , + .dcu =0x0c , + .usb2 =0x0d , + .debug =0x0e };unsigned int get_soc_major_rev (void )
@@ -131,8 +131,7 @@ int ls102xa_smmu_stream_id_init(void)ls1021x_config_caam_stream_id (sec_liodn_tbl,ARRAY_SIZE (sec_liodn_tbl)); -ls102xa_config_smmu_stream_id (dev_stream_id, -ARRAY_SIZE (dev_stream_id)); +ls102xa_config_smmu_stream_id (&dev_stream_id);return 0 ; }
arch/arm/include/asm/arch-ls102xa/ls102xa_stream_id.h +15 -4
@@ -64,11 +64,22 @@ struct liodn_id_table { };struct smmu_stream_id { -uint16_t offset; -uint16_t stream_id; -char dev_name[32 ]; +uint16_t mac1; +uint16_t mac2; +uint16_t mac3; +uint16_t pex1; +uint16_t pex2; +uint16_t dma; +uint16_t sata; +uint16_t usb3; +uint16_t qe; +uint16_t sdhc; +uint16_t adma; +uint16_t dcu; +uint16_t usb2; +uint16_t debug; };void ls1021x_config_caam_stream_id (struct liodn_id_table *tbl,int size); -void ls102xa_config_smmu_stream_id (struct smmu_stream_id *id,uint32_t num); +void ls102xa_config_smmu_stream_id (struct smmu_stream_id *id);#endif
board/freescale/common/ls102xa_stream_id.c +17 -6
@@ -7,14 +7,25 @@#include <common.h> #include <asm/io.h> #include <asm/arch/ls102xa_stream_id.h> +#include <asm/arch/immap_ls102xa.h> -void ls102xa_config_smmu_stream_id (struct smmu_stream_id *id,uint32_t num) +void ls102xa_config_smmu_stream_id (struct smmu_stream_id *id) { -uint32_t *scfg = (uint32_t *)CONFIG_SYS_FSL_SCFG_ADDR ; -int i; - -for (i =0 ; i < num; i++) -out_be32 (scfg + id[i].offset, id[i].stream_id); +struct ccsr_scfg *scfg = (struct ccsr_scfg *)CONFIG_SYS_FSL_SCFG_ADDR ; + scfg->mac1_streamid = id->mac1; + scfg->mac2_streamid = id->mac2; + scfg->mac3_streamid = id->mac3; + scfg->pex1_streamid = id->pex1; + scfg->pex2_streamid = id->pex2; + scfg->dma_streamid = id->dma; + scfg->sata_streamid = id->sata; + scfg->usb3_streamid = id->usb3; + scfg->qe_streamid = id->qe; + scfg->sdhc_streamid = id->sdhc; + scfg->adma_streamid = id->adma; + scfg->dcu_streamid = id->dcu; + scfg->usb2_streamid = id->usb2; + scfg->debug_streamid = id->debug; }void ls1021x_config_caam_stream_id (struct liodn_id_table *tbl,int size)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160421/8a9422a8/attachment.sig>
PATCH 1/1 arm: Fix SCFG ICID reg addresses
I prefer to use void *.
York
Sent from my phone
-------- Original Message --------
From: Vincent Siles <[email protected]>
Sent: Thursday, April 21, 2016 12:53 AM
To: york sun <[email protected]>
Subject: Re: [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses
CC: u-boot at lists.denx.de,Alison Wang <[email protected]>
Hi !
Quoted 48 lines · York Sun
Quoted 36 lines · Vincent Siles
On the LS102x boards, in order to initialize the ICID values of masters, the
dev_stream_idarray holds absolute offsets from the base of SCFG.In
ls102xa_config_ssmu_stream_id, the base pointer is cast touint32_t* before adding the offset, leading to an invalid address. Casting it to unsigned char * solves the issue.Also minor cosmetic renaming of
uint32_tinto u32 to be consistent in the whole file.Signed-off-by Vincent Siles <[email protected]>---
board/freescale/common/ls102xa_stream_id.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)board/freescale/common/ls102xa_stream_id.c +2
-2@@ -10,11 +10,11 @@void ls102xa_config_smmu_stream_id (struct smmu_stream_id *id,uint32_t num) { -uint32_t *scfg = (uint32_t *)CONFIG_SYS_FSL_SCFG_ADDR ; +unsigned char *scfg = (unsigned char *)CONFIG_SYS_FSL_SCFG_ADDR ;int i;for (i =0 ; i < num; i++) -out_be32 (scfg + id[i].offset, id[i].stream_id); +out_be32 ((u32 *)(scfg + id[i].offset), id[i].stream_id); }void ls1021x_config_caam_stream_id (struct liodn_id_table *tbl,int size)board/freescale/common/ls102xa_stream_id.c void ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, int size) · line 33
Quoted 7 lines
board/freescale/common/ls102xa_stream_id.c +1
-1@@ -28,6 +28,6 @@ void ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, int size)else liodn = tbl[i].id[0 ]; -out_le32 ((uint32_t *)(tbl[i].reg_offset), liodn); +out_le32 ((u32 *)(tbl[i].reg_offset), liodn); } }If the size of the pointer is an issue, maybe you ca use "void *"? Can you check if "struct
ccsr_scfg" should/can be used?
Quoted 1 line
York
By using the 'struct ccsr_scfg' type we won't be able to have the same kind of loop. Since the code is board dependent, I'm not sure it really matters.
Here what I would do instead. Tell me which style do you prefer.
Best,
Vincent
---
arch/arm/cpu/armv7/ls102xa/soc.c +0 -0
@@ -30,21 +30,21 @@ struct liodn_id_table sec_liodn_tbl[] = {
SET_SEC_DECO_LIODN_ENTRY(7, 0x10, 0x10),
};
-struct smmu_stream_id dev_stream_id[] = {
- { 0x100, 0x01, "ETSEC MAC1" },
- { 0x104, 0x02, "ETSEC MAC2" },
- { 0x108, 0x03, "ETSEC MAC3" },
- { 0x10c, 0x04, "PEX1" },
- { 0x110, 0x05, "PEX2" },
- { 0x114, 0x06, "qDMA" },
- { 0x118, 0x07, "SATA" },
- { 0x11c, 0x08, "USB3" },
- { 0x120, 0x09, "QE" },
- { 0x124, 0x0a, "eSDHC" },
- { 0x128, 0x0b, "eMA" },
- { 0x14c, 0x0c, "2D-ACE" },
- { 0x150, 0x0d, "USB2" },
- { 0x18c, 0x0e, "DEBUG" },
+struct smmu_stream_id dev_stream_id = {
+ .mac1 = 0x01,
+ .mac2 = 0x02,
+ .mac3 = 0x03,
+ .pex1 = 0x04,
+ .pex2 = 0x05,
+ .dma = 0x06,
+ .sata = 0x07,
+ .usb3 = 0x08,
+ .qe = 0x09,
+ .sdhc = 0x0a,
+ .adma = 0x0b,
+ .dcu = 0x0c,
+ .usb2 = 0x0d,
+ .debug = 0x0e
};
unsigned int get_soc_major_rev(void)
@@ -131,8 +131,7 @@ int ls102xa_smmu_stream_id_init(void) ls1021x_config_caam_stream_id(sec_liodn_tbl,
ARRAY_SIZE(sec_liodn_tbl));
- ls102xa_config_smmu_stream_id(dev_stream_id,
- ARRAY_SIZE(dev_stream_id));
+ ls102xa_config_smmu_stream_id(&dev_stream_id);
return 0;
}
arch/arm/include/asm/arch-ls102xa/ls102xa_stream_id.h +0 -0
@@ -64,11 +64,22 @@ struct liodn_id_table {
};
struct smmu_stream_id {
- uint16_t offset;
- uint16_t stream_id;
- char dev_name[32];
+ uint16_t mac1;
+ uint16_t mac2;
+ uint16_t mac3;
+ uint16_t pex1;
+ uint16_t pex2;
+ uint16_t dma;
+ uint16_t sata;
+ uint16_t usb3;
+ uint16_t qe;
+ uint16_t sdhc;
+ uint16_t adma;
+ uint16_t dcu;
+ uint16_t usb2;
+ uint16_t debug;
};
void ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, int size); -void ls102xa_config_smmu_stream_id(struct smmu_stream_id *id, uint32_t num); +void ls102xa_config_smmu_stream_id(struct smmu_stream_id *id); #endif
board/freescale/common/ls102xa_stream_id.c +0 -0
@@ -7,14 +7,25 @@
#include <common.h>
#include <asm/io.h>
#include <asm/arch/ls102xa_stream_id.h>
+#include <asm/arch/immap_ls102xa.h>
-void ls102xa_config_smmu_stream_id(struct smmu_stream_id *id, uint32_t num) +void ls102xa_config_smmu_stream_id(struct smmu_stream_id *id)
{
- uint32_t *scfg = (uint32_t *)CONFIG_SYS_FSL_SCFG_ADDR;
- int i;
-
- for (i = 0; i < num; i++)
- out_be32(scfg + id[i].offset, id[i].stream_id);
+ struct ccsr_scfg *scfg = (struct ccsr_scfg *)CONFIG_SYS_FSL_SCFG_ADDR;
+ scfg->mac1_streamid = id->mac1;
+ scfg->mac2_streamid = id->mac2;
+ scfg->mac3_streamid = id->mac3;
+ scfg->pex1_streamid = id->pex1;
+ scfg->pex2_streamid = id->pex2;
+ scfg->dma_streamid = id->dma;
+ scfg->sata_streamid = id->sata;
+ scfg->usb3_streamid = id->usb3;
+ scfg->qe_streamid = id->qe;
+ scfg->sdhc_streamid = id->sdhc;
+ scfg->adma_streamid = id->adma;
+ scfg->dcu_streamid = id->dcu;
+ scfg->usb2_streamid = id->usb2;
+ scfg->debug_streamid = id->debug;
}
void ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, int size)
PATCH 1/1 arm: Fix SCFG ICID reg addresses
Quoted 3 lines · york sun
I prefer to use void *.
York
I'm not sure what you mean by "use void *". We need to compute the address of the registers, so you can't directly use void * as it does not allow pointer arithmetic. Either you use unsigned char * with full offset, uint32_t * with offset/sizeof(uint32_t) or the direct mapping using struct ccsr_scfg * and field access.
Could you elaborate ?
Vincent
Quoted 203 lines
Sent from my phone
-------- Original Message --------
From: Vincent Siles <[email protected]>
Sent: Thursday, April 21, 2016 12:53 AM
To: york sun <[email protected]>
Subject: Re: [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses
CC: u-boot at lists.denx.de,Alison Wang <[email protected]>Hi !
Quoted 2 lines · York Sun
Quoted 1 line · Vincent Siles
On the LS102x boards, in order to initialize the ICID values of
masters,
Quoted 3 lines
Quoted 3 lines
the
dev_stream_idarray holds absolute offsets from the base of SCFG.In
ls102xa_config_ssmu_stream_id, the base pointer is cast touint32_t*
Quoted 6 lines
Quoted 6 lines
before adding the offset, leading to an invalid address. Casting it to unsigned char * solves the issue.
Also minor cosmetic renaming of
uint32_tinto u32 to be consistent in the whole file.Quoted 7 lines
Review
Quoted 7 lines
Signed-off-by Vincent Siles <[email protected]>---
board/freescale/common/ls102xa_stream_id.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)(quoted diff) +0
-0
b/board/freescale/common/ls102xa_stream_id.cQuoted 6 lines
board/freescale/common/ls102xa_stream_id.c line 11
Quoted 6 lines
index f434269..2a4ef3e 100644
board/freescale/common/ls102xa_stream_id.c +0
-0@@ -10,11 +10,11 @@ void ls102xa_config_smmu_stream_id(struct smmu_stream_id *id,
uint32_tnum)Quoted 11 lines
board/freescale/common/ls102xa_stream_id.c line 20
Quoted 11 lines
board/freescale/common/ls102xa_stream_id.c +2
-2{ -uint32_t *scfg = (uint32_t *)CONFIG_SYS_FSL_SCFG_ADDR ; +unsigned char *scfg = (unsigned char *)CONFIG_SYS_FSL_SCFG_ADDR ;int i;for (i =0 ; i < num; i++) -out_be32 (scfg + id[i].offset, id[i].stream_id); +out_be32 ((u32 *)(scfg + id[i].offset), id[i].stream_id); } void ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, intsize)
Quoted 1 line
board/freescale/common/ls102xa_stream_id.c void ls1021x_config_caam_stream_id(struct
Quoted 1 line
board/freescale/common/ls102xa_stream_id.c +0
-0@@ -28,6 +28,6 @@ void ls1021x_config_caam_stream_id(struct
liodn_id_table*tbl, int size)Quoted 10 lines
board/freescale/common/ls102xa_stream_id.c void ls1021x_config_caam_stream_id(struct · line 33
Quoted 8 lines
board/freescale/common/ls102xa_stream_id.c +1
-1else liodn = tbl[i].id[0 ]; -out_le32 ((uint32_t *)(tbl[i].reg_offset), liodn); +out_le32 ((u32 *)(tbl[i].reg_offset), liodn); } }If the size of the pointer is an issue, maybe you ca use "void *"? Can
you check
Quoted 3 lines
if "struct
ccsr_scfg" should/can be used?York
By using the 'struct
ccsr_scfg' type we won't be able to have the same kind of loop. Since the code is board dependent, I'm not sure it really matters.Here what I would do instead. Tell me which style do you prefer.
Best,
Vincent---
diff --git a/arch/arm/cpu/armv7/ls102xa/soc.c b/arch/arm/cpu/armv7/ls102xa/soc.c index b1b0c71..9c78efc 100644 --- a/arch/arm/cpu/armv7/ls102xa/soc.c +++ b/arch/arm/cpu/armv7/ls102xa/soc.c @@ -30,21 +30,21 @@ struct liodn_id_table sec_liodn_tbl[] = { SET_SEC_DECO_LIODN_ENTRY(7, 0x10, 0x10), };-struct
smmu_stream_iddev_stream_id[] = {
- {0x100,0x01, "ETSEC MAC1" },
- {0x104,0x02, "ETSEC MAC2" },
- {0x108,0x03, "ETSEC MAC3" },
- {0x10c,0x04, "PEX1" },
- {0x110,0x05, "PEX2" },
- {0x114,0x06, "qDMA" },
- {0x118,0x07, "SATA" },
- {0x11c,0x08, "USB3" },
- {0x120,0x09, "QE" },
- {0x124,0x0a, "eSDHC" },
- {0x128,0x0b, "eMA" },
- {0x14c,0x0c, "2D-ACE" },
- {0x150,0x0d, "USB2" },
- {0x18c,0x0e, "DEBUG" },
+structsmmu_stream_iddev_stream_id= {
+ .mac1 =0x01,
+ .mac2 =0x02,
+ .mac3 =0x03,
+ .pex1 =0x04,
+ .pex2 =0x05,
+ .dma =0x06,
+ .sata =0x07,
+ .usb3 =0x08,
+ .qe =0x09,
+ .sdhc =0x0a,
+ .adma =0x0b,
+ .dcu =0x0c,
+ .usb2 =0x0d,
+ .debug =0x0e
};unsigned int get_soc_major_rev(void) @@ -131,8 +131,7 @@ int ls102xa_smmu_stream_id_init(void) ls1021x_config_caam_stream_id(sec_liodn_tbl, ARRAY_SIZE(sec_liodn_tbl));- ls102xa_config_smmu_stream_id(dev_stream_id,
-ARRAY_SIZE(dev_stream_id));
+ls102xa_config_smmu_stream_id(&dev_stream_id);return 0; } diff --git a/arch/arm/include/asm/arch-ls102xa/ls102xa_stream_id.h b/arch/arm/include/asm/arch-ls102xa/ls102xa_stream_id.h index fa571b3..3815673 100644 --- a/arch/arm/include/asm/arch-ls102xa/ls102xa_stream_id.h +++ b/arch/arm/include/asm/arch-ls102xa/ls102xa_stream_id.h @@ -64,11 +64,22 @@ struct liodn_id_table { };struct
smmu_stream_id{
-uint16_toffset;
-uint16_tstream_id;
- char dev_name[32];
+uint16_tmac1;
+uint16_tmac2;
+uint16_tmac3;
+uint16_tpex1;
+uint16_tpex2;
+uint16_tdma;
+uint16_tsata;
+uint16_tusb3;
+uint16_tqe;
+uint16_tsdhc;
+uint16_tadma;
+uint16_tdcu;
+uint16_tusb2;
+uint16_tdebug;
};void ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, int size); -void ls102xa_config_smmu_stream_id(struct smmu_stream_id *id, uint32_t num); +void ls102xa_config_smmu_stream_id(struct smmu_stream_id *id); #endif diff --git a/board/freescale/common/ls102xa_stream_id.c b/board/freescale/common/ls102xa_stream_id.c index f434269..941f22d 100644 --- a/board/freescale/common/ls102xa_stream_id.c +++ b/board/freescale/common/ls102xa_stream_id.c @@ -7,14 +7,25 @@ #include <common.h> #include <asm/io.h> #include <asm/arch/ls102xa_stream_id.h> +#include <asm/arch/immap_ls102xa.h>-void
ls102xa_config_smmu_stream_id(struct smmu_stream_id *id, uint32_t num)
+voidls102xa_config_smmu_stream_id(struct smmu_stream_id *id){
-uint32_t*scfg = (uint32_t*)CONFIG_SYS_FSL_SCFG_ADDR;
- int i;
-
- for (i = 0; i < num; i++)
-out_be32(scfg + id[i].offset, id[i].stream_id);
+ structccsr_scfg*scfg = (structccsr_scfg*)CONFIG_SYS_FSL_SCFG_ADDR;
+scfg->mac1_streamid=id->mac1;
+scfg->mac2_streamid=id->mac2;
+scfg->mac3_streamid=id->mac3;
+scfg->pex1_streamid=id->pex1;
+scfg->pex2_streamid=id->pex2;
+scfg->dma_streamid=id->dma;
+scfg->sata_streamid=id->sata;
+scfg->usb3_streamid=id->usb3;
+scfg->qe_streamid=id->qe;
+scfg->sdhc_streamid=id->sdhc;
+scfg->adma_streamid=id->adma;
+scfg->dcu_streamid=id->dcu;
+scfg->usb2_streamid=id->usb2;
+scfg->debug_streamid=id->debug;
}
Quoted 1 line
void
ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, int size)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160421/855abe46/attachment.sig>
PATCH 1/1 arm: Fix SCFG ICID reg addresses
Quoted 5 lines · Vincent Siles
Quoted 3 lines · york sun
I prefer to use void *.
York
Quoted 7 lines
I'm not sure what you mean by "use void *". We need to compute the address of the registers, so you can't directly use void * as it does not allow pointer arithmetic. Either you use unsigned char * with full offset,
uint32_t* with offset/sizeof(uint32_t) or the direct mapping using structccsr_scfg* and field access.Could you elaborate ?
The reason of your change is (u32 *) + int doesn't yield the correct offset. So you fix it with (u8 *) + int. I was suggesting to use (void *) + int. Of course you need to cast to (u32 *) before using out_be32.
York
Quoted 207 lines
Vincent
Quoted 204 lines
Sent from my phone
-------- Original Message --------
From: Vincent Siles <[email protected]>
Sent: Thursday, April 21, 2016 12:53 AM
To: york sun <[email protected]>
Subject: Re: [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses
CC: u-boot at lists.denx.de,Alison Wang <[email protected]>Hi !
Quoted 2 lines · York Sun
Quoted 1 line · Vincent Siles
On the LS102x boards, in order to initialize the ICID values of
masters,
Quoted 3 lines
Quoted 3 lines
the
dev_stream_idarray holds absolute offsets from the base of SCFG.In
ls102xa_config_ssmu_stream_id, the base pointer is cast touint32_t*
Quoted 6 lines
Quoted 6 lines
before adding the offset, leading to an invalid address. Casting it to unsigned char * solves the issue.
Also minor cosmetic renaming of
uint32_tinto u32 to be consistent in the whole file.Quoted 7 lines
Review
Quoted 7 lines
Signed-off-by Vincent Siles <[email protected]>---
board/freescale/common/ls102xa_stream_id.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)(quoted diff) +0
-0
b/board/freescale/common/ls102xa_stream_id.cQuoted 6 lines
board/freescale/common/ls102xa_stream_id.c line 11
Quoted 6 lines
index f434269..2a4ef3e 100644
board/freescale/common/ls102xa_stream_id.c +0
-0@@ -10,11 +10,11 @@ void ls102xa_config_smmu_stream_id(struct smmu_stream_id *id,
uint32_tnum)Quoted 11 lines
board/freescale/common/ls102xa_stream_id.c line 20
Quoted 11 lines
board/freescale/common/ls102xa_stream_id.c +2
-2{ -uint32_t *scfg = (uint32_t *)CONFIG_SYS_FSL_SCFG_ADDR ; +unsigned char *scfg = (unsigned char *)CONFIG_SYS_FSL_SCFG_ADDR ;int i;for (i =0 ; i < num; i++) -out_be32 (scfg + id[i].offset, id[i].stream_id); +out_be32 ((u32 *)(scfg + id[i].offset), id[i].stream_id); } void ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, intsize)
Quoted 1 line
board/freescale/common/ls102xa_stream_id.c void ls1021x_config_caam_stream_id(struct
Quoted 1 line
board/freescale/common/ls102xa_stream_id.c +0
-0@@ -28,6 +28,6 @@ void ls1021x_config_caam_stream_id(struct
liodn_id_table*tbl, int size)Quoted 10 lines
board/freescale/common/ls102xa_stream_id.c void ls1021x_config_caam_stream_id(struct · line 33
Quoted 8 lines
board/freescale/common/ls102xa_stream_id.c +1
-1else liodn = tbl[i].id[0 ]; -out_le32 ((uint32_t *)(tbl[i].reg_offset), liodn); +out_le32 ((u32 *)(tbl[i].reg_offset), liodn); } }If the size of the pointer is an issue, maybe you ca use "void *"? Can
you check
Quoted 3 lines
if "struct
ccsr_scfg" should/can be used?York
By using the 'struct
ccsr_scfg' type we won't be able to have the same kind of loop. Since the code is board dependent, I'm not sure it really matters.Here what I would do instead. Tell me which style do you prefer.
Best,
Vincent---
diff --git a/arch/arm/cpu/armv7/ls102xa/soc.c b/arch/arm/cpu/armv7/ls102xa/soc.c index b1b0c71..9c78efc 100644 --- a/arch/arm/cpu/armv7/ls102xa/soc.c +++ b/arch/arm/cpu/armv7/ls102xa/soc.c @@ -30,21 +30,21 @@ struct liodn_id_table sec_liodn_tbl[] = { SET_SEC_DECO_LIODN_ENTRY(7, 0x10, 0x10), };-struct
smmu_stream_iddev_stream_id[] = {
- {0x100,0x01, "ETSEC MAC1" },
- {0x104,0x02, "ETSEC MAC2" },
- {0x108,0x03, "ETSEC MAC3" },
- {0x10c,0x04, "PEX1" },
- {0x110,0x05, "PEX2" },
- {0x114,0x06, "qDMA" },
- {0x118,0x07, "SATA" },
- {0x11c,0x08, "USB3" },
- {0x120,0x09, "QE" },
- {0x124,0x0a, "eSDHC" },
- {0x128,0x0b, "eMA" },
- {0x14c,0x0c, "2D-ACE" },
- {0x150,0x0d, "USB2" },
- {0x18c,0x0e, "DEBUG" },
+structsmmu_stream_iddev_stream_id= {
+ .mac1 =0x01,
+ .mac2 =0x02,
+ .mac3 =0x03,
+ .pex1 =0x04,
+ .pex2 =0x05,
+ .dma =0x06,
+ .sata =0x07,
+ .usb3 =0x08,
+ .qe =0x09,
+ .sdhc =0x0a,
+ .adma =0x0b,
+ .dcu =0x0c,
+ .usb2 =0x0d,
+ .debug =0x0e
};unsigned int get_soc_major_rev(void) @@ -131,8 +131,7 @@ int ls102xa_smmu_stream_id_init(void) ls1021x_config_caam_stream_id(sec_liodn_tbl, ARRAY_SIZE(sec_liodn_tbl));- ls102xa_config_smmu_stream_id(dev_stream_id,
-ARRAY_SIZE(dev_stream_id));
+ls102xa_config_smmu_stream_id(&dev_stream_id);return 0; } diff --git a/arch/arm/include/asm/arch-ls102xa/ls102xa_stream_id.h b/arch/arm/include/asm/arch-ls102xa/ls102xa_stream_id.h index fa571b3..3815673 100644 --- a/arch/arm/include/asm/arch-ls102xa/ls102xa_stream_id.h +++ b/arch/arm/include/asm/arch-ls102xa/ls102xa_stream_id.h @@ -64,11 +64,22 @@ struct liodn_id_table { };struct
smmu_stream_id{
-uint16_toffset;
-uint16_tstream_id;
- char dev_name[32];
+uint16_tmac1;
+uint16_tmac2;
+uint16_tmac3;
+uint16_tpex1;
+uint16_tpex2;
+uint16_tdma;
+uint16_tsata;
+uint16_tusb3;
+uint16_tqe;
+uint16_tsdhc;
+uint16_tadma;
+uint16_tdcu;
+uint16_tusb2;
+uint16_tdebug;
};void ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, int size); -void ls102xa_config_smmu_stream_id(struct smmu_stream_id *id, uint32_t num); +void ls102xa_config_smmu_stream_id(struct smmu_stream_id *id); #endif diff --git a/board/freescale/common/ls102xa_stream_id.c b/board/freescale/common/ls102xa_stream_id.c index f434269..941f22d 100644 --- a/board/freescale/common/ls102xa_stream_id.c +++ b/board/freescale/common/ls102xa_stream_id.c @@ -7,14 +7,25 @@ #include <common.h> #include <asm/io.h> #include <asm/arch/ls102xa_stream_id.h> +#include <asm/arch/immap_ls102xa.h>-void
ls102xa_config_smmu_stream_id(struct smmu_stream_id *id, uint32_t num)
+voidls102xa_config_smmu_stream_id(struct smmu_stream_id *id){
-uint32_t*scfg = (uint32_t*)CONFIG_SYS_FSL_SCFG_ADDR;
- int i;
-
- for (i = 0; i < num; i++)
-out_be32(scfg + id[i].offset, id[i].stream_id);
+ structccsr_scfg*scfg = (structccsr_scfg*)CONFIG_SYS_FSL_SCFG_ADDR;
+scfg->mac1_streamid=id->mac1;
+scfg->mac2_streamid=id->mac2;
+scfg->mac3_streamid=id->mac3;
+scfg->pex1_streamid=id->pex1;
+scfg->pex2_streamid=id->pex2;
+scfg->dma_streamid=id->dma;
+scfg->sata_streamid=id->sata;
+scfg->usb3_streamid=id->usb3;
+scfg->qe_streamid=id->qe;
+scfg->sdhc_streamid=id->sdhc;
+scfg->adma_streamid=id->adma;
+scfg->dcu_streamid=id->dcu;
+scfg->usb2_streamid=id->usb2;
+scfg->debug_streamid=id->debug;
}void
ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, int size)
PATCH v2 1/2 arm: Fix SCFG ICID reg addresses
On the LS102x boards, in order to initialize the ICID values of masters, the dev_stream_id array holds absolute offsets from the base of SCFG.
In ls102xa_config_ssmu_stream_id, the base pointer is cast to uint32_t * before adding the offset, leading to an invalid address. Casting it to void * solves the issue.
---
Changes in v2:
- switch from (u8 *) to (void *)
- split modifications in two patches
board/freescale/common/ls102xa_stream_id.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
board/freescale/common/ls102xa_stream_id.c +2 -2
@@ -10,11 +10,11 @@void ls102xa_config_smmu_stream_id (struct smmu_stream_id *id,uint32_t num) { -uint32_t *scfg = (uint32_t *)CONFIG_SYS_FSL_SCFG_ADDR ; +void *scfg = (void *)CONFIG_SYS_FSL_SCFG_ADDR ;int i;for (i =0 ; i < num; i++) -out_be32 (scfg + id[i].offset, id[i].stream_id); +out_be32 ((u32 *)(scfg + id[i].offset), id[i].stream_id); }void ls1021x_config_caam_stream_id (struct liodn_id_table *tbl,int size)
Signature · 1 line
1.9.1
PATCH v2 2/2 arm: uniform usage of u32 in ls102x caam config
Mix usage of uint32_t and u32 fixed in favor of u32
---
Changes in v2:
- Casting to void * instead of u8 *
- Splitted commit into two seperate ones
board/freescale/common/ls102xa_stream_id.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
board/freescale/common/ls102xa_stream_id.c +1 -1
@@ -28,6 +28,6 @@ void ls1021x_config_caam_stream_id(struct liodn_id_table *tbl, int size)else liodn = tbl[i].id[0 ]; -out_le32 ((uint32_t *)(tbl[i].reg_offset), liodn); +out_le32 ((u32 *)(tbl[i].reg_offset), liodn); } }
Signature · 1 line
1.9.1
PATCH v2 2/2 arm: uniform usage of u32 in ls102x caam config
Quoted 4 lines · Vincent Siles
Mix usage of
uint32_tand u32 fixed in favor of u32Signed-off-by Vincent Siles <[email protected]>
Quoted 9 lines
---
Changes in v2:
- Casting to void * instead of u8 *
- Splitted commit into two seperate onesboard/freescale/common/ls102xa_stream_id.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to u-boot-fsl-qoriq master, awaiting upstream.
Thanks.
York
PATCH v2 1/2 arm: Fix SCFG ICID reg addresses
In reply to Vincent SilesQuoted 8 lines · Vincent Siles
On the LS102x boards, in order to initialize the ICID values of masters, the
dev_stream_idarray holds absolute offsets from the base of SCFG.In
ls102xa_config_ssmu_stream_id, the base pointer is cast touint32_t* before adding the offset, leading to an invalid address. Casting it to void * solves the issue.Signed-off-by Vincent Siles <[email protected]>
Quoted 8 lines
---
Changes in v2:
- switch from (u8 *) to (void *)
- split modifications in two patchesboard/freescale/common/ls102xa_stream_id.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Applied to u-boot-fsl-qoriq master, awaiting upstream.
Thanks.
York