* [U-Boot] [PATCH 0/1]
@ 2016-04-12 12:15 Vincent Siles
2016-04-12 12:15 ` [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses Vincent Siles
0 siblings, 1 reply; 11+ messages in thread
From: Vincent Siles @ 2016-04-12 12:15 UTC (permalink / raw)
To: u-boot
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(-)
--
1.9.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses 2016-04-12 12:15 [U-Boot] [PATCH 0/1] Vincent Siles @ 2016-04-12 12:15 ` Vincent Siles 2016-04-20 16:53 ` York Sun 0 siblings, 1 reply; 11+ messages in thread From: Vincent Siles @ 2016-04-12 12:15 UTC (permalink / raw) To: u-boot 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. Signed-off-by: Vincent Siles <vincent.siles@provenrun.com> --- board/freescale/common/ls102xa_stream_id.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/board/freescale/common/ls102xa_stream_id.c b/board/freescale/common/ls102xa_stream_id.c index f434269..2a4ef3e 100644 --- a/board/freescale/common/ls102xa_stream_id.c +++ b/board/freescale/common/ls102xa_stream_id.c @@ -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); } } -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses 2016-04-12 12:15 ` [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses Vincent Siles @ 2016-04-20 16:53 ` York Sun 2016-04-21 7:53 ` Vincent Siles 0 siblings, 1 reply; 11+ messages in thread From: York Sun @ 2016-04-20 16:53 UTC (permalink / raw) To: u-boot On 04/12/2016 05:28 AM, Vincent Siles wrote: > 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. > > Signed-off-by: Vincent Siles <vincent.siles@provenrun.com> > --- > > board/freescale/common/ls102xa_stream_id.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/board/freescale/common/ls102xa_stream_id.c b/board/freescale/common/ls102xa_stream_id.c > index f434269..2a4ef3e 100644 > --- a/board/freescale/common/ls102xa_stream_id.c > +++ b/board/freescale/common/ls102xa_stream_id.c > @@ -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); > } > } > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses 2016-04-20 16:53 ` York Sun @ 2016-04-21 7:53 ` Vincent Siles 2016-04-21 15:45 ` york sun 0 siblings, 1 reply; 11+ messages in thread From: Vincent Siles @ 2016-04-21 7:53 UTC (permalink / raw) To: u-boot Hi ! On 20-04-16 09:53:32, York Sun wrote: > On 04/12/2016 05:28 AM, Vincent Siles wrote: > > 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. > > > > Signed-off-by: Vincent Siles <vincent.siles@provenrun.com> > > --- > > > > board/freescale/common/ls102xa_stream_id.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/board/freescale/common/ls102xa_stream_id.c b/board/freescale/common/ls102xa_stream_id.c > > index f434269..2a4ef3e 100644 > > --- a/board/freescale/common/ls102xa_stream_id.c > > +++ b/board/freescale/common/ls102xa_stream_id.c > > @@ -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); > > } > > } > > > > 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 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_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; } 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_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 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) +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> ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses 2016-04-21 7:53 ` Vincent Siles @ 2016-04-21 15:45 ` york sun 2016-04-21 15:56 ` Vincent Siles 0 siblings, 1 reply; 11+ messages in thread From: york sun @ 2016-04-21 15:45 UTC (permalink / raw) To: u-boot I prefer to use void *. York Sent from my phone -------- Original Message -------- From: Vincent Siles <vincent.siles@provenrun.com> Sent: Thursday, April 21, 2016 12:53 AM To: york sun <york.sun@nxp.com> Subject: Re: [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses CC: u-boot at lists.denx.de,Alison Wang <alison.wang@freescale.com> Hi ! On 20-04-16 09:53:32, York Sun wrote: > On 04/12/2016 05:28 AM, Vincent Siles wrote: > > 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. > > > > Signed-off-by: Vincent Siles <vincent.siles@provenrun.com> > > --- > > > > board/freescale/common/ls102xa_stream_id.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/board/freescale/common/ls102xa_stream_id.c b/board/freescale/common/ls102xa_stream_id.c > > index f434269..2a4ef3e 100644 > > --- a/board/freescale/common/ls102xa_stream_id.c > > +++ b/board/freescale/common/ls102xa_stream_id.c > > @@ -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); > > } > > } > > > > 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 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_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; } 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_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 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) +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) ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses 2016-04-21 15:45 ` york sun @ 2016-04-21 15:56 ` Vincent Siles 2016-04-21 16:19 ` York Sun 0 siblings, 1 reply; 11+ messages in thread From: Vincent Siles @ 2016-04-21 15:56 UTC (permalink / raw) To: u-boot On 21-04-16 15:45:05, york sun wrote: > 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 > > Sent from my phone > > -------- Original Message -------- > From: Vincent Siles <vincent.siles@provenrun.com> > Sent: Thursday, April 21, 2016 12:53 AM > To: york sun <york.sun@nxp.com> > Subject: Re: [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses > CC: u-boot at lists.denx.de,Alison Wang <alison.wang@freescale.com> > > Hi ! > > On 20-04-16 09:53:32, York Sun wrote: > > On 04/12/2016 05:28 AM, Vincent Siles wrote: > > > 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. > > > > > > Signed-off-by: Vincent Siles <vincent.siles@provenrun.com> > > > --- > > > > > > board/freescale/common/ls102xa_stream_id.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/board/freescale/common/ls102xa_stream_id.c > b/board/freescale/common/ls102xa_stream_id.c > > > index f434269..2a4ef3e 100644 > > > --- a/board/freescale/common/ls102xa_stream_id.c > > > +++ b/board/freescale/common/ls102xa_stream_id.c > > > @@ -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); > > > } > > > } > > > > > > > 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 > > 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_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; > } > 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_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 > 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) > +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/855abe46/attachment.sig> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses 2016-04-21 15:56 ` Vincent Siles @ 2016-04-21 16:19 ` York Sun 2016-04-22 7:52 ` [U-Boot] [PATCH v2 1/2] " Vincent Siles 0 siblings, 1 reply; 11+ messages in thread From: York Sun @ 2016-04-21 16:19 UTC (permalink / raw) To: u-boot On 04/21/2016 08:56 AM, Vincent Siles wrote: > On 21-04-16 15:45:05, york sun wrote: >> 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 ? 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 > > Vincent > >> >> Sent from my phone >> >> -------- Original Message -------- >> From: Vincent Siles <vincent.siles@provenrun.com> >> Sent: Thursday, April 21, 2016 12:53 AM >> To: york sun <york.sun@nxp.com> >> Subject: Re: [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses >> CC: u-boot at lists.denx.de,Alison Wang <alison.wang@freescale.com> >> >> Hi ! >> >> On 20-04-16 09:53:32, York Sun wrote: >> > On 04/12/2016 05:28 AM, Vincent Siles wrote: >> > > 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. >> > > >> > > Signed-off-by: Vincent Siles <vincent.siles@provenrun.com> >> > > --- >> > > >> > > board/freescale/common/ls102xa_stream_id.c | 6 +++--- >> > > 1 file changed, 3 insertions(+), 3 deletions(-) >> > > >> > > diff --git a/board/freescale/common/ls102xa_stream_id.c >> b/board/freescale/common/ls102xa_stream_id.c >> > > index f434269..2a4ef3e 100644 >> > > --- a/board/freescale/common/ls102xa_stream_id.c >> > > +++ b/board/freescale/common/ls102xa_stream_id.c >> > > @@ -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); >> > > } >> > > } >> > > >> > >> > 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 >> >> 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_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; >> } >> 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_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 >> 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) >> +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) ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/2] arm: Fix SCFG ICID reg addresses 2016-04-21 16:19 ` York Sun @ 2016-04-22 7:52 ` Vincent Siles 2016-04-22 7:52 ` [U-Boot] [PATCH v2 2/2] arm: uniform usage of u32 in ls102x caam config Vincent Siles 2016-05-24 17:13 ` [U-Boot] [PATCH v2 1/2] arm: Fix SCFG ICID reg addresses York Sun 0 siblings, 2 replies; 11+ messages in thread From: Vincent Siles @ 2016-04-22 7:52 UTC (permalink / raw) To: u-boot 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. Signed-off-by: Vincent Siles <vincent.siles@provenrun.com> --- 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(-) diff --git a/board/freescale/common/ls102xa_stream_id.c b/board/freescale/common/ls102xa_stream_id.c index f434269..39e7b30 100644 --- a/board/freescale/common/ls102xa_stream_id.c +++ b/board/freescale/common/ls102xa_stream_id.c @@ -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) -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/2] arm: uniform usage of u32 in ls102x caam config 2016-04-22 7:52 ` [U-Boot] [PATCH v2 1/2] " Vincent Siles @ 2016-04-22 7:52 ` Vincent Siles 2016-05-24 17:13 ` York Sun 2016-05-24 17:13 ` [U-Boot] [PATCH v2 1/2] arm: Fix SCFG ICID reg addresses York Sun 1 sibling, 1 reply; 11+ messages in thread From: Vincent Siles @ 2016-04-22 7:52 UTC (permalink / raw) To: u-boot Mix usage of uint32_t and u32 fixed in favor of u32 Signed-off-by: Vincent Siles <vincent.siles@provenrun.com> --- 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(-) diff --git a/board/freescale/common/ls102xa_stream_id.c b/board/freescale/common/ls102xa_stream_id.c index 39e7b30..3d5404e 100644 --- a/board/freescale/common/ls102xa_stream_id.c +++ b/board/freescale/common/ls102xa_stream_id.c @@ -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); } } -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/2] arm: uniform usage of u32 in ls102x caam config 2016-04-22 7:52 ` [U-Boot] [PATCH v2 2/2] arm: uniform usage of u32 in ls102x caam config Vincent Siles @ 2016-05-24 17:13 ` York Sun 0 siblings, 0 replies; 11+ messages in thread From: York Sun @ 2016-05-24 17:13 UTC (permalink / raw) To: u-boot On 04/22/2016 12:52 AM, Vincent Siles wrote: > Mix usage of uint32_t and u32 fixed in favor of u32 > > Signed-off-by: Vincent Siles <vincent.siles@provenrun.com> > > --- > > 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(-) > Applied to u-boot-fsl-qoriq master, awaiting upstream. Thanks. York ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/2] arm: Fix SCFG ICID reg addresses 2016-04-22 7:52 ` [U-Boot] [PATCH v2 1/2] " Vincent Siles 2016-04-22 7:52 ` [U-Boot] [PATCH v2 2/2] arm: uniform usage of u32 in ls102x caam config Vincent Siles @ 2016-05-24 17:13 ` York Sun 1 sibling, 0 replies; 11+ messages in thread From: York Sun @ 2016-05-24 17:13 UTC (permalink / raw) To: u-boot On 04/22/2016 12:52 AM, Vincent Siles wrote: > 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. > > Signed-off-by: Vincent Siles <vincent.siles@provenrun.com> > --- > > 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(-) Applied to u-boot-fsl-qoriq master, awaiting upstream. Thanks. York ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-05-24 17:13 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-12 12:15 [U-Boot] [PATCH 0/1] Vincent Siles 2016-04-12 12:15 ` [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses Vincent Siles 2016-04-20 16:53 ` York Sun 2016-04-21 7:53 ` Vincent Siles 2016-04-21 15:45 ` york sun 2016-04-21 15:56 ` Vincent Siles 2016-04-21 16:19 ` York Sun 2016-04-22 7:52 ` [U-Boot] [PATCH v2 1/2] " Vincent Siles 2016-04-22 7:52 ` [U-Boot] [PATCH v2 2/2] arm: uniform usage of u32 in ls102x caam config Vincent Siles 2016-05-24 17:13 ` York Sun 2016-05-24 17:13 ` [U-Boot] [PATCH v2 1/2] arm: Fix SCFG ICID reg addresses York Sun
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox