[U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses
York Sun
york.sun at nxp.com
Thu Apr 21 18:19:40 CEST 2016
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 at provenrun.com>
>> Sent: Thursday, April 21, 2016 12:53 AM
>> To: york sun <york.sun at nxp.com>
>> Subject: Re: [U-Boot] [PATCH] arm: Fix SCFG ICID reg addresses
>> CC: u-boot at lists.denx.de,Alison Wang <alison.wang at 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 at 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)
More information about the U-Boot
mailing list