> -----Original Message-----
> From: Lei Wen [mailto:adrian.w...@gmail.com]
> Sent: Tuesday, December 07, 2010 8:53 PM
> To: Prafulla Wadaskar
> Cc: u-boot@lists.denx.de; Eric Miao; Manas Saksena; Lei Wen; Yu Tang;
> Ashish Karkare; Kiran Vedere; Prabhanjan Sarnaik
> Subject: Re: [U-Boot] [PATCH v4 2/7] gpio: Add Multi-Function-Pin
> configuration driver for Marvell SoCs
>
> Hi Prafulla,
>
> On Wed, Dec 8, 2010 at 1:06 AM, Prafulla Wadaskar <prafu...@marvell.com>
> wrote:
> > Most of the Marvell SoCs has Multi Function Pin (MFP) configuration
> registers
> > For ex. ARMADA100.
> >
> > These registers are programmed to expose the specific functionality
> > associated with respective SoC Pins
> >
> > This driver provides configuration APIs,
> > using them, configuration need to be done in board specific code
> >
> > for ex- following code configures MFPs 107 and 108 for UART_TX/RX
> functionality
> >
> > int board_early_init_f(void)
> > {
> > u32 mfp_cfg[] = {
> > /* Console on UART1 */
> > MFP107_UART1_RXD,
> > MFP108_UART1_TXD,
> > MFP_EOC /*End of configureation*/
> > };
> > /* configure MFP's */
> > mfp_config(mfp_cfg);
> > return 0;
> > }
> >
> > Signed-off-by: Prafulla Wadaskar <prafu...@marvell.com>
> > ---
> > Changelog v4:
> > 1. Driver renamed as mvmfp
> > 2. Re-architected mvmfp driver as per review feedback
> >
> > drivers/gpio/Makefile | 1 +
> > drivers/gpio/mvmfp.c | 90
> ++++++++++++++++++++++++++++++++++++++++++++
> > include/mvmfp.h | 100
> +++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 191 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/gpio/mvmfp.c
> > create mode 100644 include/mvmfp.h
> >
> > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> > index 398024c..a5fa2b5 100644
> > --- a/drivers/gpio/Makefile
> > +++ b/drivers/gpio/Makefile
> > @@ -27,6 +27,7 @@ LIB := $(obj)libgpio.o
> >
> > COBJS-$(CONFIG_AT91_GPIO) += at91_gpio.o
> > COBJS-$(CONFIG_KIRKWOOD_GPIO) += kw_gpio.o
> > +COBJS-$(CONFIG_MARVELL_MFP) += mvmfp.o
> > COBJS-$(CONFIG_MXC_GPIO) += mxc_gpio.o
> > COBJS-$(CONFIG_PCA953X) += pca953x.o
> > COBJS-$(CONFIG_S5P) += s5p_gpio.o
> > diff --git a/drivers/gpio/mvmfp.c b/drivers/gpio/mvmfp.c
> > new file mode 100644
> > index 0000000..3472278
> > --- /dev/null
> > +++ b/drivers/gpio/mvmfp.c
> > @@ -0,0 +1,90 @@
> > +/*
> > + * (C) Copyright 2010
> > + * Marvell Semiconductor <www.marvell.com>
> > + * Written-by: Prafulla Wadaskar <prafu...@marvell.com>,
> > + *
> > + * See file CREDITS for list of people who contributed to this
> > + * project.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of
> > + * the License, or (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> > + * MA 02110-1301 USA
> > + */
> > +
> > +#include <common.h>
> > +#include <asm/io.h>
> > +#include <mvmfp.h>
> > +#include <asm/arch/mfp.h>
> > +#ifdef CONFIG_ARMADA100
> > +#include <asm/arch/armada100.h>
> > +#define MFPR_BASE ARMD1_MFPR_BASE
> > +#else
> > +#error Unsupported SoC...
> > +#endif
>
> Why not directly name a CONFIG_MFPR_BASE, and define its value in the
> config file?
Otherway, We can eliminate #define MFPR_BASE here and define the same in
asm/arch/armada100.h instead of ARMD1_MFPR_BASE
> If we do like this ifdef, we may need do add each arch here, seems
> some kind of redundant?
This is required here, see drivers/serial/serial.c
...snip...
> > + /* Write a mfg register as per configuration */
> > + if (cfg_val & MFP_AF_FLAG) {
> > + /* Abstract and program Afternate-Func Selection
> */
> > + val &= ~MFP_AF_MASK;
> Do we need to do this & here? For val is only 0 here...
This can be removed.
> Should not it be more concise like:
> writel(cfg_val, p_mfpr);
NACK, cfg_val have some more stuff like offset, flags, those are not required
to write on mfp register.
Regards..
Prafulla . .
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
