On Fri, Mar 18, 2011 at 02:35:24PM +0300, [email protected] wrote: > diff --git a/arch/powerpc/platforms/512x/clock.c > b/arch/powerpc/platforms/512x/clock.c > index 3dc2a8d..5cadf8e 100644 > --- a/arch/powerpc/platforms/512x/clock.c > +++ b/arch/powerpc/platforms/512x/clock.c > @@ -606,6 +606,21 @@ static void rate_clks_init(void) > */ > struct clk dev_clks[2][32]; > > +char *mpc512x_select_psc_compat(void) > +{ > + char *psc_compats[] = { > + "fsl,mpc5121-psc", > + "fsl,mpc5125-psc" > + }; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(psc_compats); i++) > + if (of_find_compatible_node(NULL, NULL, psc_compats[i])) > + return psc_compats[i]; > + > + return NULL; > +}
Function looks good to me. Shouldn't that rather be in mpc512x_shared?
> +// IOCTL registers for USB1/FEC2
No c++-style comments, please (here and later).
> +static void mpc5125_psc_iopad_init(void __iomem *ioctl, char *name)
> +{
> + struct device_node *np;
> + const u32 *cell_index;
> + char *default_psc = "fsl,mpc5125-psc";
> + char *psc_name;
> +
> + if (name)
> + psc_name = name;
> + else
> + psc_name = default_psc;
Caller sets name to NULL. Is this really used?
> +
> + for_each_compatible_node(np, NULL, psc_name) {
> + cell_index = of_get_property(np, "cell-index", NULL);
I seem to recall 'cell-index' is deprecated. Grant?
> + if (cell_index) {
> + u8 __iomem *pscioctl;
> + int psc_num = *cell_index;
> + if (psc_num > 1)
> + continue;
> +
> + pscioctl = ioctl + PSC_TO_IOCTL_OFFSET(psc_num);
> + out_8(pscioctl + IOCTL_PSCx_0, IOCTL_PSCx_0_MODE); //
> NOTE maybe wrong
Why is it 'maybe wrong'? Can it be improved somehow?
> + out_8(pscioctl + IOCTL_PSCx_1, IOCTL_DEFAULT_MODE);
> + out_8(pscioctl + IOCTL_PSCx_2, IOCTL_DEFAULT_MODE);
> + out_8(pscioctl + IOCTL_PSCx_3, IOCTL_DEFAULT_MODE);
> + out_8(pscioctl + IOCTL_PSCx_4, IOCTL_DEFAULT_MODE);
The defines make it much more readable, thanks.
> + }
> + }
> +}
Is this function really board-specific or platform specific?
> +static void mpc5125_fec2_usb_io_init(void __iomem *ioctl, int isusb)
> +{
> + int i;
> + const u8 offset[12] = {
> + IOCTL_USB1_DATA0, IOCTL_USB1_DATA1,
> + IOCTL_USB1_DATA2, IOCTL_USB1_DATA3,
> + IOCTL_USB1_DATA4, IOCTL_USB1_DATA5,
> + IOCTL_USB1_DATA6, IOCTL_USB1_DATA7,
> + IOCTL_USB1_STOP, IOCTL_USB1_CLK,
> + IOCTL_USB1_NEXT, IOCTL_USB1_DIR
> + };
> + u8 mode;
> +
> + mode = (isusb) ? IOCTL_DEFAULT_MODE : IOCTL_FEC2_MODE;
> + for (i = 0; i < ARRAY_SIZE(offset); i++)
> + out_8(ioctl + offset[i], mode);
> +}
Same question here and later. If it is board specific, the function name should
have something like 'twr' in it; but a few things seem mpc5125-generic
to me, if I am not mistaken?
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
_______________________________________________ Linuxppc-dev mailing list [email protected] https://lists.ozlabs.org/listinfo/linuxppc-dev
