| Message ID | 1291219286-2798-1-git-send-email-prafulla@marvell.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Prafulla, On Thu, Dec 2, 2010 at 12:01 AM, Prafulla Wadaskar <prafulla@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 <prafulla@marvell.com> > --- > Please Ignore earlier patch, below bufix is added to this patch > > Change log v3: > Fixed: Read-modify-Write a mfg register as per configuration > ANDed with MASK value before ORing for each configuration > > REPOST-V1: > I am sorry for the repost-V1 :-( > MASK values changed to relevent (copy paste mistake) > > Regards.. > Prafulla . . > > drivers/gpio/Makefile | 1 + > drivers/gpio/mfp.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++ > include/mfp.h | 97 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 208 insertions(+), 0 deletions(-) > create mode 100644 drivers/gpio/mfp.c > create mode 100644 include/mfp.h > > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > index 398024c..f6903d5 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_MFP) += mfp.o > COBJS-$(CONFIG_MXC_GPIO) += mxc_gpio.o > COBJS-$(CONFIG_PCA953X) += pca953x.o > COBJS-$(CONFIG_S5P) += s5p_gpio.o > diff --git a/drivers/gpio/mfp.c b/drivers/gpio/mfp.c > new file mode 100644 > index 0000000..f26af97 > --- /dev/null > +++ b/drivers/gpio/mfp.c > @@ -0,0 +1,110 @@ > +/* > + * (C) Copyright 2010 > + * Marvell Semiconductor <www.marvell.com> > + * Written-by: Prafulla Wadaskar <prafulla@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 <mfp.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 > + > +/* > + * mfp_config > + * > + * On most of Marvell SoCs (ex. ARMADA100) there is Multi-Funtion-Pin > + * configuration registers to configure each GPIO/Function pin on the > + * SoC. > + * > + * This function reads the array of values for > + * MFPR_X registers and programms them into respective > + * Multi-Function Pin registers. > + * It supports - Alternate Function Selection programming. > + * > + * Whereas, > + * The Configureation value is constructed using ARMD_MFP() > + * array consists of 32bit values as- > + * Bits 31-16 : Mfp instance number (i.e. MFPR no. to be programmed) > + * Bits 15-13 : PULL_UP/PULL_DOWN selection > + * Bits 11:10 : Pin Driver strength > + * Bits 6-4 : Edge detection configuration > + * Bits 2-0 : Alternate Function Selection > + * > + * For more details please refer respective Product Software Manual > + */ > +void mfp_config(u32 *mfp_cfgs) > +{ > + u32 *p_mfpr = NULL; > + u32 val, cfg_val, mfpr_no; > + > + do { > + cfg_val = *mfp_cfgs++; > + /* exit if End of configuration table detected */ > + if (cfg_val == MFP_EOC) > + break; > + /* abstract mfpr tobe programmed from configuration value */ > + mfpr_no = (cfg_val & MFP_PINNO_MASK) >> 16; > + BUG_ON(mfpr_no >= MFP_PIN_MAX); > + > + /* the offset address are divided in three regions and not > + * consecutive, this corrects the same (Ref: Specs: A1.1) */ > + p_mfpr = (u32 *)MFPR_BASE; > + if ( mfpr_no < 37) > + p_mfpr += (0x004c / 4) + mfpr_no; > + else if ( mfpr_no >= 56) > + p_mfpr += (0x00e0 / 4) + (mfpr_no - 56); > + else > + p_mfpr += (mfpr_no - 37); This three regions is only meaningful for armada100, so when we use the mfp.c to other cpu, like pxa920, how could we reuse this setting? Obviously, pxa920 is different with armada100 regions arrangement, with the start mfp of ND_IO[15]. Best regards, Lei
> -----Original Message----- > From: Lei Wen [mailto:adrian.wenl@gmail.com] > Sent: Wednesday, December 01, 2010 7:51 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 v3 3/7] [REPOST-1] gpio: Add Multi-Function- > Pin configuration driver for Marvell SoCs > > Hi Prafulla, ...snip... > > + > > + /* the offset address are divided in three regions and > not > > + * consecutive, this corrects the same (Ref: Specs: > A1.1) */ > > + p_mfpr = (u32 *)MFPR_BASE; > > + if ( mfpr_no < 37) > > + p_mfpr += (0x004c / 4) + mfpr_no; > > + else if ( mfpr_no >= 56) > > + p_mfpr += (0x00e0 / 4) + (mfpr_no - 56); > > + else > > + p_mfpr += (mfpr_no - 37); > > This three regions is only meaningful for armada100, so when we use > the mfp.c to other cpu, like pxa920, > how could we reuse this setting? Obviously, pxa920 is different with > armada100 regions arrangement, with > the start mfp of ND_IO[15]. Currently mfp is added for armada100, I don't know what will be diffs with other SoCs. When other SoC will come up, we will modify this file :-) Regards.. Prafulla . .
Hi Prafulla, On Thu, Dec 2, 2010 at 2:54 PM, Prafulla Wadaskar <prafulla@marvell.com> wrote: > > >> -----Original Message----- >> From: Lei Wen [mailto:adrian.wenl@gmail.com] >> Sent: Wednesday, December 01, 2010 7:51 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 v3 3/7] [REPOST-1] gpio: Add Multi-Function- >> Pin configuration driver for Marvell SoCs >> >> Hi Prafulla, > ...snip... >> > + >> > + /* the offset address are divided in three regions and >> not >> > + * consecutive, this corrects the same (Ref: Specs: >> A1.1) */ >> > + p_mfpr = (u32 *)MFPR_BASE; >> > + if ( mfpr_no < 37) >> > + p_mfpr += (0x004c / 4) + mfpr_no; >> > + else if ( mfpr_no >= 56) >> > + p_mfpr += (0x00e0 / 4) + (mfpr_no - 56); >> > + else >> > + p_mfpr += (mfpr_no - 37); >> >> This three regions is only meaningful for armada100, so when we use >> the mfp.c to other cpu, like pxa920, >> how could we reuse this setting? Obviously, pxa920 is different with >> armada100 regions arrangement, with >> the start mfp of ND_IO[15]. > > Currently mfp is added for armada100, I don't know what will be diffs with other SoCs. > When other SoC will come up, we will modify this file :-) Since the mfp.c is for generic case, and should be made as generic at the beginning... Current this is only hard coding, I think provide some kind of mapping would be good. Best regards, Lei
> -----Original Message----- > From: Lei Wen [mailto:adrian.wenl@gmail.com] > Sent: Thursday, December 02, 2010 12:29 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 v3 3/7] [REPOST-1] gpio: Add Multi-Function- > Pin configuration driver for Marvell SoCs > > Hi Prafulla, > > On Thu, Dec 2, 2010 at 2:54 PM, Prafulla Wadaskar <prafulla@marvell.com> > wrote: > > > > > >> -----Original Message----- > >> From: Lei Wen [mailto:adrian.wenl@gmail.com] > >> Sent: Wednesday, December 01, 2010 7:51 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 v3 3/7] [REPOST-1] gpio: Add Multi- > Function- > >> Pin configuration driver for Marvell SoCs > >> > >> Hi Prafulla, > > ...snip... > >> > + > >> > + /* the offset address are divided in three regions > and > >> not > >> > + * consecutive, this corrects the same (Ref: Specs: > >> A1.1) */ > >> > + p_mfpr = (u32 *)MFPR_BASE; > >> > + if ( mfpr_no < 37) > >> > + p_mfpr += (0x004c / 4) + mfpr_no; > >> > + else if ( mfpr_no >= 56) > >> > + p_mfpr += (0x00e0 / 4) + (mfpr_no - 56); > >> > + else > >> > + p_mfpr += (mfpr_no - 37); > >> > >> This three regions is only meaningful for armada100, so when we use > >> the mfp.c to other cpu, like pxa920, > >> how could we reuse this setting? Obviously, pxa920 is different with > >> armada100 regions arrangement, with > >> the start mfp of ND_IO[15]. > > > > Currently mfp is added for armada100, I don't know what will be diffs > with other SoCs. > > When other SoC will come up, we will modify this file :-) > > Since the mfp.c is for generic case, and should be made as generic at > the beginning... Sure.. but I don't know how it will be on other SoCs? Apart from that what would be other impact? So in my opinion, let's keep it for future updates. With reference to your precise concern, I will create a macro in asm/arch/mfp.h moving below code there. + if ( mfpr_no < 37) + p_mfpr += (0x004c / 4) + mfpr_no; + else if ( mfpr_no >= 56) + p_mfpr += (0x00e0 / 4) + (mfpr_no - 56); + else + p_mfpr += (mfpr_no - 37) This will help to isolate it as SoC specific implementation, I will post REPOST-2 for the same Thanks and Regards.. Prafulla . .
Dear Prafulla Wadaskar, In message <F766E4F80769BD478052FB6533FA745D19A9291DBC@SC-VEXCH4.marvell.com> you wrote: > > > Since the mfp.c is for generic case, and should be made as generic at > > the beginning... > > Sure.. but I don't know how it will be on other SoCs? > Apart from that what would be other impact? > So in my opinion, let's keep it for future updates. No, this is the wrong approach. Already now you realize that this code does not reallys cale well with future extensions, because you put all the logic in your code. I think this is a bad approach here. Instead, make the whole thing data driven - implement only minimal code that waks through a table provided by the user. Instead of putting the logic in the code, try to put it in the data. > With reference to your precise concern, I will create a macro in asm/arch/mfp.h > moving below code there. > > + if ( mfpr_no < 37) > + p_mfpr += (0x004c / 4) + mfpr_no; > + else if ( mfpr_no >= 56) > + p_mfpr += (0x00e0 / 4) + (mfpr_no - 56); > + else > + p_mfpr += (mfpr_no - 37) No, please don't. It's ugly, not readable and not maintainable. Best regards, Wolfgang Denk
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 398024c..f6903d5 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_MFP) += mfp.o COBJS-$(CONFIG_MXC_GPIO) += mxc_gpio.o COBJS-$(CONFIG_PCA953X) += pca953x.o COBJS-$(CONFIG_S5P) += s5p_gpio.o diff --git a/drivers/gpio/mfp.c b/drivers/gpio/mfp.c new file mode 100644 index 0000000..f26af97 --- /dev/null +++ b/drivers/gpio/mfp.c @@ -0,0 +1,110 @@ +/* + * (C) Copyright 2010 + * Marvell Semiconductor <www.marvell.com> + * Written-by: Prafulla Wadaskar <prafulla@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 <mfp.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 + +/* + * mfp_config + * + * On most of Marvell SoCs (ex. ARMADA100) there is Multi-Funtion-Pin + * configuration registers to configure each GPIO/Function pin on the + * SoC. + * + * This function reads the array of values for + * MFPR_X registers and programms them into respective + * Multi-Function Pin registers. + * It supports - Alternate Function Selection programming. + * + * Whereas, + * The Configureation value is constructed using ARMD_MFP() + * array consists of 32bit values as- + * Bits 31-16 : Mfp instance number (i.e. MFPR no. to be programmed) + * Bits 15-13 : PULL_UP/PULL_DOWN selection + * Bits 11:10 : Pin Driver strength + * Bits 6-4 : Edge detection configuration + * Bits 2-0 : Alternate Function Selection + * + * For more details please refer respective Product Software Manual + */ +void mfp_config(u32 *mfp_cfgs) +{ + u32 *p_mfpr = NULL; + u32 val, cfg_val, mfpr_no; + + do { + cfg_val = *mfp_cfgs++; + /* exit if End of configuration table detected */ + if (cfg_val == MFP_EOC) + break; + /* abstract mfpr tobe programmed from configuration value */ + mfpr_no = (cfg_val & MFP_PINNO_MASK) >> 16; + BUG_ON(mfpr_no >= MFP_PIN_MAX); + + /* the offset address are divided in three regions and not + * consecutive, this corrects the same (Ref: Specs: A1.1) */ + p_mfpr = (u32 *)MFPR_BASE; + if ( mfpr_no < 37) + p_mfpr += (0x004c / 4) + mfpr_no; + else if ( mfpr_no >= 56) + p_mfpr += (0x00e0 / 4) + (mfpr_no - 56); + else + p_mfpr += (mfpr_no - 37); + /*p_mfpr contains address of register to be programmed */ + + /* Read-modify-Write a mfg register as per configuration */ + val = readl(p_mfpr); + if (cfg_val & MFP_CFG_AF) { + /* Abstract and program Afternate-Func Selection */ + val &= ~MFP_AF_MASK; + val |= cfg_val & MFP_AF_MASK; + } if (cfg_val & MFP_CFG_EDGE) { + /* Abstract and program Edge configuration */ + val &= ~MFP_LPM_EDGE_MASK; + val |= cfg_val & MFP_LPM_EDGE_MASK; + } if (cfg_val & MFP_CFG_DRIVE) { + /* Abstract and program Drive configuration */ + val &= ~MFP_DRIVE_MASK; + val |= cfg_val & MFP_DRIVE_MASK; + } if (cfg_val & MFP_CFG_PULL) { + /* Abstract and program Pullup/down configuration */ + val &= ~MFP_PULL_MASK; + val |= cfg_val & MFP_PULL_MASK; + } + writel(val, p_mfpr); + } while (1); + /* + * perform a read-back of any MFPR register to make sure the + * previous writings are finished + */ + readl(p_mfpr); +} diff --git a/include/mfp.h b/include/mfp.h new file mode 100644 index 0000000..176acda --- /dev/null +++ b/include/mfp.h @@ -0,0 +1,97 @@ +/* + * (C) Copyright 2010 + * Marvell Semiconductor <www.marvell.com> + * Written-by: Prafulla Wadaskar <prafulla@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 + */ + +#ifndef __MFP_H +#define __MFP_H + +/* + * Header file for MultiFunctionPin (MFP) Configururation framework + * + * Processors Supported: + * 1. Marvell ARMADA100 Processors + * + * processor to be supported should be added here + */ + +#define MFP_EOC 0xffffffff /* flag to indicate end-of-configuration */ + +/* + * Possible MFP configuration is represented by a 32-bit unsigned integer + * + * bit 0.. 2 - Alternate Function Selection + * bit 4.. 6 - Edge Detection + * bit 7.. 9 - Type of configuration + * bit 10..11 - Drive Strength + * bit 13..15 - Run Mode Pull State + * bit 16..31 - Used to hold MFP number to be configured + * + * to facilitate the definition, the following macros are provided + */ + +#define MFP_AF0 (0x0 << 0) +#define MFP_AF1 (0x1 << 0) +#define MFP_AF2 (0x2 << 0) +#define MFP_AF3 (0x3 << 0) +#define MFP_AF4 (0x4 << 0) +#define MFP_AF5 (0x5 << 0) +#define MFP_AF6 (0x6 << 0) +#define MFP_AF7 (0x7 << 0) +#define MFP_AF_MASK (0x7 << 0) + +#define MFP_LPM_EDGE_NONE (0x0 << 4) +#define MFP_LPM_EDGE_RISE (0x1 << 4) +#define MFP_LPM_EDGE_FALL (0x2 << 4) +#define MFP_LPM_EDGE_BOTH (0x3 << 4) +#define MFP_LPM_EDGE_MASK (0x3 << 4) + +/* unused bits are used to identify config type */ +#define MFP_CFG_AF (0x1 << 7) +#define MFP_CFG_DRIVE (0x1 << 8) +#define MFP_CFG_EDGE (0x1 << 9) +#define MFP_CFG_PULL (0x1 << 3) + +#define MFP_DRIVE_VERY_SLOW (0x0 << 10) +#define MFP_DRIVE_SLOW (0x1 << 10) +#define MFP_DRIVE_MEDIUM (0x2 << 10) +#define MFP_DRIVE_FAST (0x3 << 10) +#define MFP_DRIVE_MASK (0x3 << 10) + +#define MFP_PULL_NONE (0x0 << 13) +#define MFP_PULL_LOW (0x1 << 13) +#define MFP_PULL_HIGH (0x2 << 13) +#define MFP_PULL_BOTH (0x3 << 13) +#define MFP_PULL_FLOAT (0x4 << 13) +#define MFP_PULL_MASK (0x7 << 13) + +#define MFP_PINNO_MASK (0xffff << 16) + +#define MFP_AF(af) (MFP_CFG_AF | MFP_AF##af) +#define MFP_DRIVE(drv) (MFP_CFG_DRIVE | MFP_DRIVE_##drv) +#define MFP_EDGE(edge) (MFP_CFG_EDGE | MFP_LPM_EDGE_##edge) +#define MFP_PULL(pull) (MFP_CFG_PULL | MFP_PULL_##pull) +#define MFP(pin) (MFP_PINNO_MASK & (pin << 16)) + +void mfp_config(u32 *mfp_cfgs); + +#endif /* __MFP_H */
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 <prafulla@marvell.com> --- Please Ignore earlier patch, below bufix is added to this patch Change log v3: Fixed: Read-modify-Write a mfg register as per configuration ANDed with MASK value before ORing for each configuration REPOST-V1: I am sorry for the repost-V1 :-( MASK values changed to relevent (copy paste mistake) Regards.. Prafulla . . drivers/gpio/Makefile | 1 + drivers/gpio/mfp.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++ include/mfp.h | 97 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+), 0 deletions(-) create mode 100644 drivers/gpio/mfp.c create mode 100644 include/mfp.h