Thanks. As the patchset is accepted. Will commit another one to fix them.
> -----Original Message----- > From: glik...@secretlab.ca [mailto:glik...@secretlab.ca] On > Behalf Of Grant Likely > Sent: Tuesday, February 02, 2010 5:58 AM > To: Liu Yu-B13201 > Cc: ga...@kernel.crashing.org; da...@davemloft.net; > net...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH 2/4] mpc8569mds: Add bscr setting for rtbi mode > > On Thu, Jan 14, 2010 at 1:13 AM, Liu Yu <yu....@freescale.com> wrote: > > Signed-off-by: Liu Yu <yu....@freescale.com> > > --- > > arch/powerpc/platforms/85xx/mpc85xx_mds.c | 24 > ++++++++++++++++++++++++ > > 1 files changed, 24 insertions(+), 0 deletions(-) > > > > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c > b/arch/powerpc/platforms/85xx/mpc85xx_mds.c > > index c5028a2..0872e4a 100644 > > --- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c > > +++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c > > @@ -237,6 +237,8 @@ static void __init mpc85xx_mds_setup_arch(void) > > } else if (machine_is(mpc8569_mds)) { > > #define BCSR7_UCC12_GETHnRST (0x1 << 2) > > #define BCSR8_UEM_MARVELL_RST (0x1 << 1) > > +#define BCSR_UCC_RGMII (0x1 << 6) > > +#define BCSR_UCC_RTBI (0x1 << 5) > > /* > > * U-Boot mangles interrupt polarity > for Marvell PHYs, > > * so reset built-in and UEM Marvell > PHYs, this puts > > @@ -247,6 +249,28 @@ static void __init mpc85xx_mds_setup_arch(void) > > > > setbits8(&bcsr_regs[7], > BCSR7_UCC12_GETHnRST); > > clrbits8(&bcsr_regs[8], > BCSR8_UEM_MARVELL_RST); > > + > > + for (np = NULL; (np = > of_find_compatible_node(np, > > + "network", > > Don't match on the 'type' field. Replace "network" with NULL and just > rely on "ucc_geth" for matching. > > > + > "ucc_geth")) != NULL;) { > > + const unsigned int *prop; > > u32 please. > > Also, rather than reusing 'prop' for both char* and u32 values, which > forces you to use ugly casts, use 2 local variables here. > > > + int ucc_num; > > + > > + prop = of_get_property(np, > "cell-index", NULL); > > + if (prop == NULL) > > + continue; > > + > > + ucc_num = *prop - 1; > > Ugh. No bounds checking... > > > + > > + prop = of_get_property(np, > "phy-connection-type", NULL); > > + if (prop == NULL) > > + continue; > > + > > + if (strcmp("rtbi", (const > char *)prop) == 0) > > (This is the ugly cast I was talking about.) > > > + > clrsetbits_8(&bcsr_regs[7 + ucc_num], > > ...not having bounds checking could result in badness in this > array index. > > This patch is dangerous as written. > > Finally, while using cell-index seems convenient, I think it would be > better to have a lookup table of the index into the BCSR register > block from the UCC base address, which also gives you implicit bounds > checking. > > g. > > -- > Grant Likely, B.Sc., P.Eng. > Secret Lab Technologies Ltd. > > _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
