* [PATCH 0/2] [ARM] Dove: add support for MPP configuration @ 2010-10-27 8:11 Mike Rapoport 2010-10-27 8:12 ` [PATCH 1/2] [ARM] Dove: add support for GPIOs 64-71 Mike Rapoport 2010-10-27 8:12 ` [PATCH 2/2] [ARM] Dove: add support for multi-purpose pins configuration Mike Rapoport 0 siblings, 2 replies; 10+ messages in thread From: Mike Rapoport @ 2010-10-27 8:11 UTC (permalink / raw) To: linux-arm-kernel These patches add support for multi-purpose pins configuration on Marvell Dove and enable use of GPIOs 64-71 Mike Rapoport (2): [ARM] Dove: add support for GPIOs 64-71 [ARM] Dove: add support for multi-purpose pins configuration arch/arm/mach-dove/Makefile | 2 +- arch/arm/mach-dove/include/mach/dove.h | 9 ++- arch/arm/mach-dove/include/mach/gpio.h | 6 +- arch/arm/mach-dove/mpp.c | 215 ++++++++++++++++++++++++++++++++ arch/arm/mach-dove/mpp.h | 214 +++++++++++++++++++++++++++++++ 5 files changed, 442 insertions(+), 4 deletions(-) create mode 100644 arch/arm/mach-dove/mpp.c create mode 100644 arch/arm/mach-dove/mpp.h -- 1.7.3.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] [ARM] Dove: add support for GPIOs 64-71 2010-10-27 8:11 [PATCH 0/2] [ARM] Dove: add support for MPP configuration Mike Rapoport @ 2010-10-27 8:12 ` Mike Rapoport 2010-10-27 8:12 ` [PATCH 2/2] [ARM] Dove: add support for multi-purpose pins configuration Mike Rapoport 1 sibling, 0 replies; 10+ messages in thread From: Mike Rapoport @ 2010-10-27 8:12 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Mike Rapoport <mike@compulab.co.il> --- arch/arm/mach-dove/include/mach/dove.h | 1 + arch/arm/mach-dove/include/mach/gpio.h | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-dove/include/mach/dove.h b/arch/arm/mach-dove/include/mach/dove.h index f6a0839..148f4f0 100644 --- a/arch/arm/mach-dove/include/mach/dove.h +++ b/arch/arm/mach-dove/include/mach/dove.h @@ -131,6 +131,7 @@ #define DOVE_RESET_SAMPLE_LO (DOVE_MPP_VIRT_BASE | 0x014) #define DOVE_RESET_SAMPLE_HI (DOVE_MPP_VIRT_BASE | 0x018) #define DOVE_GPIO_VIRT_BASE (DOVE_SB_REGS_VIRT_BASE | 0xd0400) +#define DOVE_GPIO2_VIRT_BASE (DOVE_SB_REGS_VIRT_BASE | 0xe8400) #define DOVE_MPP_GENERAL_VIRT_BASE (DOVE_SB_REGS_VIRT_BASE | 0xe803c) #define DOVE_AU1_SPDIFO_GPIO_EN (1 << 1) #define DOVE_NAND_GPIO_EN (1 << 0) diff --git a/arch/arm/mach-dove/include/mach/gpio.h b/arch/arm/mach-dove/include/mach/gpio.h index 0ee70ff..340bb7a 100644 --- a/arch/arm/mach-dove/include/mach/gpio.h +++ b/arch/arm/mach-dove/include/mach/gpio.h @@ -14,12 +14,14 @@ #include <plat/gpio.h> #include <asm-generic/gpio.h> /* cansleep wrappers */ -#define GPIO_MAX 64 +#define GPIO_MAX 72 #define GPIO_BASE_LO (DOVE_GPIO_VIRT_BASE + 0x00) #define GPIO_BASE_HI (DOVE_GPIO_VIRT_BASE + 0x20) -#define GPIO_BASE(pin) ((pin < 32) ? GPIO_BASE_LO : GPIO_BASE_HI) +#define GPIO_BASE(pin) ((pin < 32) ? GPIO_BASE_LO : \ + ((pin < 64) ? GPIO_BASE_HI : \ + DOVE_GPIO2_VIRT_BASE)) #define GPIO_OUT(pin) (GPIO_BASE(pin) + 0x00) #define GPIO_IO_CONF(pin) (GPIO_BASE(pin) + 0x04) -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] [ARM] Dove: add support for multi-purpose pins configuration 2010-10-27 8:11 [PATCH 0/2] [ARM] Dove: add support for MPP configuration Mike Rapoport 2010-10-27 8:12 ` [PATCH 1/2] [ARM] Dove: add support for GPIOs 64-71 Mike Rapoport @ 2010-10-27 8:12 ` Mike Rapoport 2010-10-27 9:01 ` Saeed Bishara 2010-10-27 12:46 ` saeed bishara 1 sibling, 2 replies; 10+ messages in thread From: Mike Rapoport @ 2010-10-27 8:12 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Mike Rapoport <mike@compulab.co.il> --- arch/arm/mach-dove/Makefile | 2 +- arch/arm/mach-dove/include/mach/dove.h | 8 +- arch/arm/mach-dove/mpp.c | 215 ++++++++++++++++++++++++++++++++ arch/arm/mach-dove/mpp.h | 214 +++++++++++++++++++++++++++++++ 4 files changed, 437 insertions(+), 2 deletions(-) create mode 100644 arch/arm/mach-dove/mpp.c create mode 100644 arch/arm/mach-dove/mpp.h diff --git a/arch/arm/mach-dove/Makefile b/arch/arm/mach-dove/Makefile index 7ab3be5..1515f61 100644 --- a/arch/arm/mach-dove/Makefile +++ b/arch/arm/mach-dove/Makefile @@ -1,3 +1,3 @@ -obj-y += common.o addr-map.o irq.o pcie.o +obj-y += common.o addr-map.o irq.o pcie.o mpp.o obj-$(CONFIG_MACH_DOVE_DB) += dove-db-setup.o diff --git a/arch/arm/mach-dove/include/mach/dove.h b/arch/arm/mach-dove/include/mach/dove.h index 148f4f0..27b4145 100644 --- a/arch/arm/mach-dove/include/mach/dove.h +++ b/arch/arm/mach-dove/include/mach/dove.h @@ -136,10 +136,16 @@ #define DOVE_AU1_SPDIFO_GPIO_EN (1 << 1) #define DOVE_NAND_GPIO_EN (1 << 0) #define DOVE_MPP_CTRL4_VIRT_BASE (DOVE_GPIO_VIRT_BASE + 0x40) - +#define DOVE_SPI_GPIO_SEL (1 << 5) +#define DOVE_UART1_GPIO_SEL (1 << 4) +#define DOVE_AU1_GPIO_SEL (1 << 3) +#define DOVE_CAM_GPIO_SEL (1 << 2) +#define DOVE_SD1_GPIO_SEL (1 << 1) +#define DOVE_SD0_GPIO_SEL (1 << 0) /* Power Management */ #define DOVE_PMU_VIRT_BASE (DOVE_SB_REGS_VIRT_BASE | 0xd0000) +#define DOVE_PMU_SIG_CTRL (DOVE_PMU_VIRT_BASE + 0x802c) /* Real Time Clock */ #define DOVE_RTC_PHYS_BASE (DOVE_SB_REGS_PHYS_BASE | 0xd8500) diff --git a/arch/arm/mach-dove/mpp.c b/arch/arm/mach-dove/mpp.c new file mode 100644 index 0000000..919dd7f --- /dev/null +++ b/arch/arm/mach-dove/mpp.c @@ -0,0 +1,215 @@ +/* + * arch/arm/mach-dove/mpp.c + * + * MPP functions for Marvell Dove SoCs + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include <linux/kernel.h> +#include <linux/gpio.h> +#include <linux/io.h> + +#include <mach/dove.h> + +#include "mpp.h" + +#define MPP_NR_REGS 4 +#define MPP_CTRL(i) ((i) == 3 ? \ + DOVE_MPP_CTRL4_VIRT_BASE : \ + DOVE_MPP_VIRT_BASE + (i) * 4) +#define PMU_SIG_REGS 2 +#define PMU_SIG_CTRL(i) (DOVE_PMU_SIG_CTRL + (i) * 4) + +struct dove_mpp_grp { + int start; + int end; +}; + +static struct dove_mpp_grp dove_mpp_grp[] = { + [MPP_24_39] = { + .start = 24, + .end = 39, + }, + [MPP_40_45] = { + .start = 40, + .end = 45, + }, + [MPP_46_51] = { + .start = 40, + .end = 45, + }, + [MPP_58_61] = { + .start = 58, + .end = 61, + }, + [MPP_62_63] = { + .start = 62, + .end = 63, + }, +}; + +static void dove_mpp_gpio_mode(int start, int end, int gpio_mode) +{ + int i; + + for (i = start; i <= end; i++) + orion_gpio_set_valid(i, gpio_mode); +} + +static void dove_mpp_dump_regs(void) +{ +#ifdef DEBUG + int i; + + pr_debug("MPP_CTRL regs:"); + for (i = 0; i < MPP_NR_REGS; i++) + printk(" %08x", readl(MPP_CTRL(i))); + printk("\n"); + + pr_debug("PMU_SIG_CTRL regs:"); + for (i = 0; i < PMU_SIG_REGS; i++) + printk(" %08x", readl(PMU_SIG_CTRL(i))); + printk("\n"); + + pr_debug("PMU_MPP_GENERAL_CTRL: %08x\n", readl(DOVE_PMU_MPP_GENERAL_CTRL)); + pr_debug("MPP_GENERAL: %08x\n", readl(DOVE_MPP_GENERAL_VIRT_BASE)); +#endif +} + +static void dove_mpp_cfg_nfc(int sel) +{ + u32 mpp_gen_cfg = readl(DOVE_MPP_GENERAL_VIRT_BASE); + + mpp_gen_cfg &= ~0x1; + mpp_gen_cfg |= sel; + writel(mpp_gen_cfg, DOVE_MPP_GENERAL_VIRT_BASE); + + dove_mpp_gpio_mode(64, 71, GPIO_OUTPUT_OK); +} + +static void dove_mpp_cfg_au1(int sel) +{ + u32 mpp_ctrl4 = readl(DOVE_MPP_CTRL4_VIRT_BASE); + u32 ssp_ctrl1 = readl(DOVE_SSP_CTRL_STATUS_1); + u32 mpp_gen_ctrl = readl(DOVE_MPP_GENERAL_VIRT_BASE); + u32 global_cfg_2 = readl(DOVE_GLOBAL_CONFIG_2); + + mpp_ctrl4 &= ~(DOVE_AU1_GPIO_SEL); + ssp_ctrl1 &= ~(DOVE_SSP_ON_AU1); + mpp_gen_ctrl &= ~(DOVE_AU1_SPDIFO_GPIO_EN); + global_cfg_2 &= ~(DOVE_TWSI_OPTION3_GPIO); + + if (!sel || sel == 0x2) + dove_mpp_gpio_mode(52, 57, 0); + else + dove_mpp_gpio_mode(52, 57, GPIO_OUTPUT_OK | GPIO_INPUT_OK); + + if (sel & 0x1) { + global_cfg_2 |= DOVE_TWSI_OPTION3_GPIO; + dove_mpp_gpio_mode(56, 57, 0); + } + if (sel & 0x2) { + mpp_gen_ctrl |= DOVE_AU1_SPDIFO_GPIO_EN; + dove_mpp_gpio_mode(57, 57, GPIO_OUTPUT_OK | GPIO_INPUT_OK); + } + if (sel & 0x4) { + ssp_ctrl1 |= DOVE_SSP_ON_AU1; + dove_mpp_gpio_mode(52, 55, 0); + } + if (sel & 0x8) + mpp_ctrl4 |= DOVE_AU1_GPIO_SEL; + + writel(mpp_ctrl4, DOVE_MPP_CTRL4_VIRT_BASE); + writel(ssp_ctrl1, DOVE_SSP_CTRL_STATUS_1); + writel(mpp_gen_ctrl, DOVE_MPP_GENERAL_VIRT_BASE); + writel(global_cfg_2, DOVE_GLOBAL_CONFIG_2); +} + +static void dove_mpp_conf_grp(int num, int sel, u32 *mpp_ctrl) +{ + int start = dove_mpp_grp[num].start; + int end = dove_mpp_grp[num].start; + int gpio_mode = sel ? GPIO_OUTPUT_OK | GPIO_INPUT_OK : 0; + + *mpp_ctrl &= ~(0x1 << num); + *mpp_ctrl |= sel << num; + + dove_mpp_gpio_mode(start, end, gpio_mode); +} + +void __init dove_mpp_conf(unsigned int *mpp_list) +{ + u32 mpp_ctrl[MPP_NR_REGS]; + u32 pmu_mpp_ctrl = 0; + u32 pmu_sig_ctrl[PMU_SIG_REGS]; + int i; + + /* Initialize gpiolib. */ + orion_gpio_init(); + + for (i = 0; i < MPP_NR_REGS; i++) + mpp_ctrl[i] = readl(MPP_CTRL(i)); + + for (i = 0; i < PMU_SIG_REGS; i++) + pmu_sig_ctrl[i] = readl(PMU_SIG_CTRL(i)); + + pmu_mpp_ctrl = readl(DOVE_PMU_MPP_GENERAL_CTRL); + + dove_mpp_dump_regs(); + + while (*mpp_list != MPP_END) { + unsigned int num = MPP_NUM(*mpp_list); + unsigned int sel = MPP_SEL(*mpp_list); + int shift, gpio_mode; + + if (num > MPP_MAX) { + pr_err("dove: invalid MPP number (%u)\n", num); + goto next_pin; + } + + if (*mpp_list & MPP_NFC_MASK) { + dove_mpp_cfg_nfc(sel); + goto next_pin; + } + + if (*mpp_list & MPP_AU1_MASK) { + dove_mpp_cfg_au1(sel); + goto next_pin; + } + + if (*mpp_list & MPP_GRP_MASK) { + dove_mpp_conf_grp(num, sel, &mpp_ctrl[3]); + goto next_pin; + } + + shift = (num & 7) << 2; + if (*mpp_list & MPP_PMU_MASK) { + pmu_mpp_ctrl |= (0x1 << num); + pmu_sig_ctrl[num / 8] &= ~(0xf << shift); + pmu_sig_ctrl[num / 8] |= ~(0xf << shift); + gpio_mode = 0; + } else { + mpp_ctrl[num / 8] &= ~(0xf << shift); + mpp_ctrl[num / 8] |= sel << shift; + gpio_mode = GPIO_OUTPUT_OK | GPIO_INPUT_OK; + } + + orion_gpio_set_valid(num, gpio_mode); + +next_pin: + mpp_list++; + } + + for (i = 0; i < MPP_NR_REGS; i++) + writel(mpp_ctrl[i], MPP_CTRL(i)); + + for (i = 0; i < PMU_SIG_REGS; i++) + writel(pmu_sig_ctrl[i], PMU_SIG_CTRL(i)); + + writel(pmu_mpp_ctrl, DOVE_PMU_MPP_GENERAL_CTRL); + + dove_mpp_dump_regs(); +} diff --git a/arch/arm/mach-dove/mpp.h b/arch/arm/mach-dove/mpp.h new file mode 100644 index 0000000..c7b8acf --- /dev/null +++ b/arch/arm/mach-dove/mpp.h @@ -0,0 +1,214 @@ +#ifndef __ARCH_DOVE_MPP_CODED_H +#define __ARCH_DOVE_MPP_CODED_H + +#define MPP(_num, _mode, _pmu, _grp, _au1, _nfc) ( \ +/* MPP/group number */ ((_num) & 0xff) | \ +/* MPP select value */ (((_mode) & 0xf) << 8) | \ +/* MPP PMU */ ((!!(_pmu)) << 12) | \ +/* group flag */ ((!!(_grp)) << 13) | \ +/* AU1 flag */ ((!!(_au1)) << 14) | \ +/* NFCE flag */ ((!!(_nfc)) << 15)) + +#define MPP_MAX 71 + +#define MPP_NUM(x) ((x) & 0xff) +#define MPP_SEL(x) (((x) >> 8) & 0xf) + +#define MPP_PMU_MASK MPP(0, 0x0, 1, 0, 0, 0) +#define MPP_GRP_MASK MPP(0, 0x0, 0, 1, 0, 0) +#define MPP_AU1_MASK MPP(0, 0x0, 0, 0, 1, 0) +#define MPP_NFC_MASK MPP(0, 0x0, 0, 0, 0, 1) +#define MPP_END MPP(0xff, 0xf, 1, 1, 1, 1) + +#define MPP_PMU_DRIVE_0 0x1 +#define MPP_PMU_DRIVE_1 0x2 +#define MPP_PMU_SDI 0x3 +#define MPP_PMU_CPU_PWRDWN 0x4 +#define MPP_PMU_STBY_PWRDWN 0x5 +#define MPP_PMU_CORE_PWR_GOOD 0x8 +#define MPP_PMU_BAT_FAULT 0xa +#define MPP_PMU_EXT0_WU 0xb +#define MPP_PMU_EXT1_WU 0xc +#define MPP_PMU_EXT2_WU 0xd +#define MPP_PMU_BLINK 0xe +#define MPP_PMU(_num, _func) MPP((_num), MPP_PMU_##_func, 1, 0, 0, 0) + +#define MPP0_GPIO0 MPP(0, 0x0, 0, 0, 0, 0) +#define MPP0_UA2_RTSn MPP(0, 0x2, 0, 0, 0, 0) +#define MPP0_SDIO0_CD MPP(0, 0x3, 0, 0, 0, 0) +#define MPP0_LCD0_PWM MPP(0, 0xf, 0, 0, 0, 0) + +#define MPP1_GPIO1 MPP(1, 0x0, 0, 0, 0, 0) +#define MPP1_UA2_CTSn MPP(1, 0x2, 0, 0, 0, 0) +#define MPP1_SDIO0_WP MPP(1, 0x3, 0, 0, 0, 0) +#define MPP1_LCD1_PWM MPP(1, 0xf, 0, 0, 0, 0) + +#define MPP2_GPIO2 MPP(2, 0x0, 0, 0, 0, 0) +#define MPP2_SATA_PRESENT MPP(2, 0x1, 0, 0, 0, 0) +#define MPP2_UA2_TXD MPP(2, 0x2, 0, 0, 0, 0) +#define MPP2_SDIO0_BUS_POWER MPP(2, 0x3, 0, 0, 0, 0) +#define MPP2_UA_RTSn1 MPP(2, 0x4, 0, 0, 0, 0) + +#define MPP3_GPIO3 MPP(3, 0x0, 0, 0, 0, 0) +#define MPP3_SATA_ACT MPP(3, 0x1, 0, 0, 0, 0) +#define MPP3_UA2_RXD MPP(3, 0x2, 0, 0, 0, 0) +#define MPP3_SDIO0_LED_CTRL MPP(3, 0x3, 0, 0, 0, 0) +#define MPP3_UA_CTSn1 MPP(3, 0x4, 0, 0, 0, 0) +#define MPP3_SPI_2_CS1 MPP(3, 0xf, 0, 0, 0, 0) + +#define MPP4_GPIO4 MPP(4, 0x0, 0, 0, 0, 0) +#define MPP4_UA3_RTSn MPP(4, 0x2, 0, 0, 0, 0) +#define MPP4_SDIO1_CD MPP(4, 0x3, 0, 0, 0, 0) +#define MPP4_SPI_1_MISO MPP(4, 0x4, 0, 0, 0, 0) + +#define MPP5_GPIO5 MPP(5, 0x0, 0, 0, 0, 0) +#define MPP5_UA3_CTSn MPP(5, 0x2, 0, 0, 0, 0) +#define MPP5_SDIO1_WP MPP(5, 0x3, 0, 0, 0, 0) +#define MPP5_SPI_1_CS MPP(5, 0x4, 0, 0, 0, 0) + +#define MPP6_GPIO6 MPP(6, 0x0, 0, 0, 0, 0) +#define MPP6_UA3_TXD MPP(6, 0x2, 0, 0, 0, 0) +#define MPP6_SDIO1_BUS_POWER MPP(6, 0x3, 0, 0, 0, 0) +#define MPP6_SPI_1_MOSI MPP(6, 0x4, 0, 0, 0, 0) + +#define MPP7_GPIO7 MPP( 7, 0x0, 0, 0, 0, 0) +#define MPP7_UA3_RXD MPP( 7, 0x2, 0, 0, 0, 0) +#define MPP7_SDIO1_LED_CTRL MPP( 7, 0x3, 0, 0, 0, 0) +#define MPP7_SPI_1_SCK MPP( 7, 0x4, 0, 0, 0, 0) + +#define MPP8_GPIO8 MPP( 8, 0x0, 0, 0, 0, 0) +#define MPP8_WD_RST_OUT MPP( 8, 0x1, 0, 0, 0, 0) + +#define MPP9_GPIO9 MPP( 9, 0x0, 0, 0, 0, 0) +#define MPP9_PEX1_CLKREQn MPP( 9, 0x5, 0, 0, 0, 0) + +#define MPP10_GPIO10 MPP(10, 0x0, 0, 0, 0, 0) +#define MPP10_SSP_SCLK MPP(10, 0x5, 0, 0, 0, 0) + +#define MPP11_GPIO11 MPP(11, 0x0, 0, 0, 0, 0) +#define MPP11_SATA_PRESENT MPP(11, 0x1, 0, 0, 0, 0) +#define MPP11_SATA_ACT MPP(11, 0x2, 0, 0, 0, 0) +#define MPP11_SDIO0_LED_CTRL MPP(11, 0x3, 0, 0, 0, 0) +#define MPP11_SDIO1_LED_CTRL MPP(11, 0x4, 0, 0, 0, 0) +#define MPP11_PEX0_CLKREQn MPP(11, 0x5, 0, 0, 0, 0) + +#define MPP12_GPIO12 MPP(12, 0x0, 0, 0, 0, 0) +#define MPP12_SATA_ACT MPP(12, 0x1, 0, 0, 0, 0) +#define MPP12_UA2_RTSn MPP(12, 0x2, 0, 0, 0, 0) +#define MPP12_AD0_I2S_EXT_MCLK MPP(12, 0x3, 0, 0, 0, 0) +#define MPP12_SDIO1_CD MPP(12, 0x4, 0, 0, 0, 0) + +#define MPP13_GPIO13 MPP(13, 0x0, 0, 0, 0, 0) +#define MPP13_UA2_CTSn MPP(13, 0x2, 0, 0, 0, 0) +#define MPP13_AD1_I2S_EXT_MCLK MPP(13, 0x3, 0, 0, 0, 0) +#define MPP13_SDIO1WP MPP(13, 0x4, 0, 0, 0, 0) +#define MPP13_SSP_EXTCLK MPP(13, 0x5, 0, 0, 0, 0) + +#define MPP14_GPIO14 MPP(14, 0x0, 0, 0, 0, 0) +#define MPP14_UA2_TXD MPP(14, 0x2, 0, 0, 0, 0) +#define MPP14_SDIO1_BUS_POWER MPP(14, 0x4, 0, 0, 0, 0) +#define MPP14_SSP_RXD MPP(14, 0x5, 0, 0, 0, 0) + +#define MPP15_GPIO15 MPP(15, 0x0, 0, 0, 0, 0) +#define MPP15_UA2_RXD MPP(15, 0x2, 0, 0, 0, 0) +#define MPP15_SDIO1_LED_CTRL MPP(15, 0x4, 0, 0, 0, 0) +#define MPP15_SSP_SFRM MPP(15, 0x5, 0, 0, 0, 0) + +#define MPP16_GPIO16 MPP(16, 0x0, 0, 0, 0, 0) +#define MPP16_UA3_RTSn MPP(16, 0x2, 0, 0, 0, 0) +#define MPP16_SDIO0_CD MPP(16, 0x3, 0, 0, 0, 0) +#define MPP16_SPI_2_CS1 MPP(16, 0x4, 0, 0, 0, 0) +#define MPP16_AC97_SDATA_IN1 MPP(16, 0x5, 0, 0, 0, 0) + +#define MPP17_GPIO17 MPP(17, 0x0, 0, 0, 0, 0) +#define MPP17_AC97_SYSCLK_OUT MPP(17, 0x1, 0, 0, 0, 0) +#define MPP17_UA3_CTSn MPP(17, 0x2, 0, 0, 0, 0) +#define MPP17_SDIO0_WP MPP(17, 0x3, 0, 0, 0, 0) +#define MPP17_TW_SDA2 MPP(17, 0x4, 0, 0, 0, 0) +#define MPP17_AC97_SDATA_IN2 MPP(17, 0x5, 0, 0, 0, 0) + +#define MPP18_GPIO18 MPP(18, 0x0, 0, 0, 0, 0) +#define MPP18_UA3_TXD MPP(18, 0x2, 0, 0, 0, 0) +#define MPP18_SDIO0_BUS_POWER MPP(18, 0x3, 0, 0, 0, 0) +#define MPP18_LCD0_PWM MPP(18, 0x4, 0, 0, 0, 0) +#define MPP18_AC_SDATA_IN3 MPP(18, 0x5, 0, 0, 0, 0) + +#define MPP19_GPIO19 MPP(19, 0x0, 0, 0, 0, 0) +#define MPP19_UA3_RXD MPP(19, 0x2, 0, 0, 0, 0) +#define MPP19_SDIO0_LED_CTRL MPP(19, 0x3, 0, 0, 0, 0) +#define MPP19_TW_SCK2 MPP(19, 0x4, 0, 0, 0, 0) + +#define MPP20_GPIO20 MPP(20, 0x0, 0, 0, 0, 0) +#define MPP20_AC97_SYSCLK_OUT MPP(20, 0x1, 0, 0, 0, 0) +#define MPP20_SPI_2_MISO MPP(20, 0x2, 0, 0, 0, 0) +#define MPP20_SDIO1_CD MPP(20, 0x3, 0, 0, 0, 0) +#define MPP20_SDIO0_CD MPP(20, 0x5, 0, 0, 0, 0) +#define MPP20_SPI_1_MISO MPP(20, 0x6, 0, 0, 0, 0) + +#define MPP21_GPIO21 MPP(21, 0x0, 0, 0, 0, 0) +#define MPP21_UA1_RTSn MPP(21, 0x1, 0, 0, 0, 0) +#define MPP21_SPI_2_CS0 MPP(21, 0x2, 0, 0, 0, 0) +#define MPP21_SDIO1_WP MPP(21, 0x3, 0, 0, 0, 0) +#define MPP21_SSP_SFRM MPP(21, 0x4, 0, 0, 0, 0) +#define MPP21_SDIO0_WP MPP(21, 0x5, 0, 0, 0, 0) +#define MPP21_SPI_1_CS MPP(21, 0x6, 0, 0, 0, 0) + +#define MPP22_GPIO22 MPP(22, 0x0, 0, 0, 0, 0) +#define MPP22_UA1_CTSn MPP(22, 0x1, 0, 0, 0, 0) +#define MPP22_SPI_2_MOSI MPP(22, 0x2, 0, 0, 0, 0) +#define MPP22_SDIO1_BUS_POWER MPP(22, 0x3, 0, 0, 0, 0) +#define MPP22_SSP_TXD MPP(22, 0x4, 0, 0, 0, 0) +#define MPP22_SDIO0_BUS_POWER MPP(22, 0x5, 0, 0, 0, 0) +#define MPP22_SPI_1_MOSI MPP(22, 0x6, 0, 0, 0, 0) + +#define MPP23_GPIO23 MPP(23, 0x0, 0, 0, 0, 0) +#define MPP23_SPI_2_SCK MPP(23, 0x2, 0, 0, 0, 0) +#define MPP23_SDIO1_LED_CTRL MPP(23, 0x3, 0, 0, 0, 0) +#define MPP23_SSP_SCLK MPP(23, 0x4, 0, 0, 0, 0) +#define MPP23_SDIO0_LED_CTRL MPP(23, 0x5, 0, 0, 0, 0) +#define MPP23_SPI_1_SCK MPP(23, 0x6, 0, 0, 0, 0) + +/* for MPP groups _num is a group index */ +enum dove_mpp_grp_idx { + MPP_24_39 = 2, + MPP_40_45 = 0, + MPP_46_51 = 1, + MPP_58_61 = 5, + MPP_62_63 = 4, +}; + +#define MPP24_39_GPIO MPP(MPP_24_39, 0x1, 0, 1, 0, 0) +#define MPP24_39_CAM MPP(MPP_24_39, 0x0, 0, 1, 0, 0) + +#define MPP40_45_GPIO MPP(MPP_40_45, 0x1, 0, 1, 0, 0) +#define MPP40_45_SD0 MPP(MPP_40_45, 0x0, 0, 1, 0, 0) + +#define MPP46_51_GPIO MPP(MPP_46_51, 0x1, 0, 1, 0, 0) +#define MPP46_51_SD1 MPP(MPP_46_51, 0x0, 0, 1, 0, 0) + +#define MPP58_61_GPIO MPP(MPP_58_61, 0x1, 0, 1, 0, 0) +#define MPP58_61_SPI MPP(MPP_58_61, 0x0, 0, 1, 0, 0) + +#define MPP62_63_GPIO MPP(MPP_62_63, 0x1, 0, 1, 0, 0) +#define MPP62_63_UA1 MPP(MPP_62_63, 0x0, 0, 1, 0, 0) + +/* The MPP[64:71] control differs from other groups */ +#define MPP64_71_GPO MPP(0, 0x1, 0, 0, 0, 1) +#define MPP64_71_NFC MPP(0, 0x0, 0, 0, 0, 1) + +/* + * The MPP[52:57] functionality is encoded by 4 bits in different + * registers. The _num field in this case encodes those bits in + * correspodence with Table 135 of 88AP510 Functional specification + */ +#define MPP52_57_AU1 MPP(0, 0x0, 0, 0, 1, 0) +#define MPP52_57_AU1_GPIO57 MPP(0, 0x2, 0, 0, 1, 0) +#define MPP52_57_GPIO MPP(0, 0xa, 0, 0, 1, 0) +#define MPP52_57_TW_GPIO MPP(0, 0xb, 0, 0, 1, 0) +#define MPP52_57_AU1_SSP MPP(0, 0xc, 0, 0, 1, 0) +#define MPP52_57_SSP_GPIO MPP(0, 0xe, 0, 0, 1, 0) +#define MPP52_57_SSP_TW MPP(0, 0xf, 0, 0, 1, 0) + +void dove_mpp_conf(unsigned int *mpp_list); + +#endif /* __ARCH_DOVE_MPP_CODED_H */ -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] [ARM] Dove: add support for multi-purpose pins configuration 2010-10-27 8:12 ` [PATCH 2/2] [ARM] Dove: add support for multi-purpose pins configuration Mike Rapoport @ 2010-10-27 9:01 ` Saeed Bishara 2010-10-27 9:15 ` Mike Rapoport 2010-10-27 12:46 ` saeed bishara 1 sibling, 1 reply; 10+ messages in thread From: Saeed Bishara @ 2010-10-27 9:01 UTC (permalink / raw) To: linux-arm-kernel +static void dove_mpp_conf_grp(int num, int sel, u32 *mpp_ctrl) +{ + int start = dove_mpp_grp[num].start; + int end = dove_mpp_grp[num].start; Shouldn't that be dove_mpp_grp[num].end ? saeed ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] [ARM] Dove: add support for multi-purpose pins configuration 2010-10-27 9:01 ` Saeed Bishara @ 2010-10-27 9:15 ` Mike Rapoport 0 siblings, 0 replies; 10+ messages in thread From: Mike Rapoport @ 2010-10-27 9:15 UTC (permalink / raw) To: linux-arm-kernel On 10/27/10 11:01, Saeed Bishara wrote: > +static void dove_mpp_conf_grp(int num, int sel, u32 *mpp_ctrl) > +{ > + int start = dove_mpp_grp[num].start; > + int end = dove_mpp_grp[num].start; > Shouldn't that be dove_mpp_grp[num].end ? oops.. thanks, will fix and resend > saeed -- Sincerely yours, Mike. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] [ARM] Dove: add support for multi-purpose pins configuration 2010-10-27 8:12 ` [PATCH 2/2] [ARM] Dove: add support for multi-purpose pins configuration Mike Rapoport 2010-10-27 9:01 ` Saeed Bishara @ 2010-10-27 12:46 ` saeed bishara 2010-10-27 14:30 ` Mike Rapoport 1 sibling, 1 reply; 10+ messages in thread From: saeed bishara @ 2010-10-27 12:46 UTC (permalink / raw) To: linux-arm-kernel > + > + ? ? ? ? ? ? ? shift = (num & 7) << 2; > + ? ? ? ? ? ? ? if (*mpp_list & MPP_PMU_MASK) { > + ? ? ? ? ? ? ? ? ? ? ? pmu_mpp_ctrl |= (0x1 << num); > + ? ? ? ? ? ? ? ? ? ? ? pmu_sig_ctrl[num / 8] &= ~(0xf << shift); > + ? ? ? ? ? ? ? ? ? ? ? pmu_sig_ctrl[num / 8] |= ~(0xf << shift); the ~ in line hints something wrong > diff --git a/arch/arm/mach-dove/mpp.h b/arch/arm/mach-dove/mpp.h > new file mode 100644 > index 0000000..c7b8acf > --- /dev/null > +++ b/arch/arm/mach-dove/mpp.h > @@ -0,0 +1,214 @@ > +#ifndef __ARCH_DOVE_MPP_CODED_H > +#define __ARCH_DOVE_MPP_CODED_H > + > +#define MPP(_num, _mode, _pmu, _grp, _au1, _nfc) ( ? ? \ > +/* MPP/group number */ ? ? ? ? ((_num) & 0xff) | ? ? ? ? ? ? ? \ > +/* MPP select value */ ? ? ? ? (((_mode) & 0xf) << 8) | ? ? ? ?\ > +/* MPP PMU */ ? ? ? ? ? ? ? ? ?((!!(_pmu)) << 12) | ? ? ? ? ? ?\ > +/* group flag */ ? ? ? ? ? ? ? ((!!(_grp)) << 13) | ? ? ? ? ? ?\ > +/* AU1 flag */ ? ? ? ? ? ? ? ? ((!!(_au1)) << 14) | ? ? ? ? ? ?\ > +/* NFCE flag */ ? ? ? ? ? ? ? ? ? ? ? ?((!!(_nfc)) << 15)) in order to make these defines readable, I suggest to add defines for the flags, for example: #define GROUP (1 << 13) then MPP defines look like: #define MPPX_GPIO MPP(MPP_X, 0x1, GROUP|AU1) > + > +#define MPP20_GPIO20 ? ? ? ? ? MPP(20, 0x0, 0, 0, 0, 0) > +#define MPP20_AC97_SYSCLK_OUT ?MPP(20, 0x1, 0, 0, 0, 0) > +#define MPP20_SPI_2_MISO ? ? ? MPP(20, 0x2, 0, 0, 0, 0) I'ld rather call this LCD_SPI instead of SPI_2 to make it clear that this SPI is different ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] [ARM] Dove: add support for multi-purpose pins configuration 2010-10-27 12:46 ` saeed bishara @ 2010-10-27 14:30 ` Mike Rapoport 2010-10-27 15:16 ` saeed bishara 0 siblings, 1 reply; 10+ messages in thread From: Mike Rapoport @ 2010-10-27 14:30 UTC (permalink / raw) To: linux-arm-kernel On 10/27/10 14:46, saeed bishara wrote: >> + >> + shift = (num & 7) << 2; >> + if (*mpp_list & MPP_PMU_MASK) { >> + pmu_mpp_ctrl |= (0x1 << num); >> + pmu_sig_ctrl[num / 8] &= ~(0xf << shift); >> + pmu_sig_ctrl[num / 8] |= ~(0xf << shift); > the ~ in line hints something wrong indeed :) >> diff --git a/arch/arm/mach-dove/mpp.h b/arch/arm/mach-dove/mpp.h >> new file mode 100644 >> index 0000000..c7b8acf >> --- /dev/null >> +++ b/arch/arm/mach-dove/mpp.h >> @@ -0,0 +1,214 @@ >> +#ifndef __ARCH_DOVE_MPP_CODED_H >> +#define __ARCH_DOVE_MPP_CODED_H >> + >> +#define MPP(_num, _mode, _pmu, _grp, _au1, _nfc) ( \ >> +/* MPP/group number */ ((_num) & 0xff) | \ >> +/* MPP select value */ (((_mode) & 0xf) << 8) | \ >> +/* MPP PMU */ ((!!(_pmu)) << 12) | \ >> +/* group flag */ ((!!(_grp)) << 13) | \ >> +/* AU1 flag */ ((!!(_au1)) << 14) | \ >> +/* NFCE flag */ ((!!(_nfc)) << 15)) > in order to make these defines readable, I suggest to add defines for > the flags, for example: > #define GROUP (1 << 13) > then MPP defines look like: > > #define MPPX_GPIO MPP(MPP_X, 0x1, GROUP|AU1) I think we can make it even more readable with #define MPP_PIN(_num, _sel) MPP(_num, _sel, 0, 0, 0, 0) #define MPP_GRP(_grp, _sel) MPP(_grp, _sel, 0, 1, 0, 0) #define MPP_GRP_AU1(_sel) MPP(0, _sel, 0, 0, 1, 0) #define MPP_GRP_NFC(_sel) MPP(0, _sel, 0, 0, 0, 1) What do you think? > >> + >> +#define MPP20_GPIO20 MPP(20, 0x0, 0, 0, 0, 0) >> +#define MPP20_AC97_SYSCLK_OUT MPP(20, 0x1, 0, 0, 0, 0) >> +#define MPP20_SPI_2_MISO MPP(20, 0x2, 0, 0, 0, 0) > I'ld rather call this LCD_SPI instead of SPI_2 to make it clear that > this SPI is different -- Sincerely yours, Mike. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] [ARM] Dove: add support for multi-purpose pins configuration 2010-10-27 14:30 ` Mike Rapoport @ 2010-10-27 15:16 ` saeed bishara 2010-10-27 15:38 ` Eric Miao 0 siblings, 1 reply; 10+ messages in thread From: saeed bishara @ 2010-10-27 15:16 UTC (permalink / raw) To: linux-arm-kernel On Wed, Oct 27, 2010 at 4:30 PM, Mike Rapoport <mike@compulab.co.il> wrote: > On 10/27/10 14:46, saeed bishara wrote: >>> + >>> + ? ? ? ? ? ? ? shift = (num & 7) << 2; >>> + ? ? ? ? ? ? ? if (*mpp_list & MPP_PMU_MASK) { >>> + ? ? ? ? ? ? ? ? ? ? ? pmu_mpp_ctrl |= (0x1 << num); >>> + ? ? ? ? ? ? ? ? ? ? ? pmu_sig_ctrl[num / 8] &= ~(0xf << shift); >>> + ? ? ? ? ? ? ? ? ? ? ? pmu_sig_ctrl[num / 8] |= ~(0xf << shift); >> the ?~ in line hints something wrong > > indeed :) > >>> diff --git a/arch/arm/mach-dove/mpp.h b/arch/arm/mach-dove/mpp.h >>> new file mode 100644 >>> index 0000000..c7b8acf >>> --- /dev/null >>> +++ b/arch/arm/mach-dove/mpp.h >>> @@ -0,0 +1,214 @@ >>> +#ifndef __ARCH_DOVE_MPP_CODED_H >>> +#define __ARCH_DOVE_MPP_CODED_H >>> + >>> +#define MPP(_num, _mode, _pmu, _grp, _au1, _nfc) ( ? ? \ >>> +/* MPP/group number */ ? ? ? ? ((_num) & 0xff) | ? ? ? ? ? ? ? \ >>> +/* MPP select value */ ? ? ? ? (((_mode) & 0xf) << 8) | ? ? ? ?\ >>> +/* MPP PMU */ ? ? ? ? ? ? ? ? ?((!!(_pmu)) << 12) | ? ? ? ? ? ?\ >>> +/* group flag */ ? ? ? ? ? ? ? ((!!(_grp)) << 13) | ? ? ? ? ? ?\ >>> +/* AU1 flag */ ? ? ? ? ? ? ? ? ((!!(_au1)) << 14) | ? ? ? ? ? ?\ >>> +/* NFCE flag */ ? ? ? ? ? ? ? ? ? ? ? ?((!!(_nfc)) << 15)) >> in order to make these defines readable, I suggest to add defines for >> the flags, for example: >> #define GROUP ? (1 << 13) >> then MPP defines look like: >> >> #define MPPX_GPIO ? ? ? ? ?MPP(MPP_X, 0x1, GROUP|AU1) > > I think we can make it even more readable with > > #define MPP_PIN(_num, _sel) ? ? MPP(_num, _sel, 0, 0, 0, 0) > #define MPP_GRP(_grp, _sel) ? ? MPP(_grp, _sel, 0, 1, 0, 0) > #define MPP_GRP_AU1(_sel) ? ? ? MPP(0, _sel, 0, 0, 1, 0) > #define MPP_GRP_NFC(_sel) ? ? ? MPP(0, _sel, 0, 0, 0, 1) > > What do you think? when you have multiple flags in the MPP macro, people would think that those flags are not exlusive, in other words, mpp can have multiple flags. so I suggest you to use a type (gpr, au1, nfc or pmu). > >> >>> + >>> +#define MPP20_GPIO20 ? ? ? ? ? MPP(20, 0x0, 0, 0, 0, 0) >>> +#define MPP20_AC97_SYSCLK_OUT ?MPP(20, 0x1, 0, 0, 0, 0) >>> +#define MPP20_SPI_2_MISO ? ? ? MPP(20, 0x2, 0, 0, 0, 0) >> I'ld rather call this LCD_SPI instead of SPI_2 to make it clear that >> this SPI is different > > > -- > Sincerely yours, > Mike. > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] [ARM] Dove: add support for multi-purpose pins configuration 2010-10-27 15:16 ` saeed bishara @ 2010-10-27 15:38 ` Eric Miao 2010-10-28 7:57 ` Saeed Bishara 0 siblings, 1 reply; 10+ messages in thread From: Eric Miao @ 2010-10-27 15:38 UTC (permalink / raw) To: linux-arm-kernel On Wed, Oct 27, 2010 at 11:16 AM, saeed bishara <saeed.bishara@gmail.com> wrote: > On Wed, Oct 27, 2010 at 4:30 PM, Mike Rapoport <mike@compulab.co.il> wrote: >> On 10/27/10 14:46, saeed bishara wrote: >>>> + >>>> + ? ? ? ? ? ? ? shift = (num & 7) << 2; >>>> + ? ? ? ? ? ? ? if (*mpp_list & MPP_PMU_MASK) { >>>> + ? ? ? ? ? ? ? ? ? ? ? pmu_mpp_ctrl |= (0x1 << num); >>>> + ? ? ? ? ? ? ? ? ? ? ? pmu_sig_ctrl[num / 8] &= ~(0xf << shift); >>>> + ? ? ? ? ? ? ? ? ? ? ? pmu_sig_ctrl[num / 8] |= ~(0xf << shift); >>> the ?~ in line hints something wrong >> >> indeed :) >> >>>> diff --git a/arch/arm/mach-dove/mpp.h b/arch/arm/mach-dove/mpp.h >>>> new file mode 100644 >>>> index 0000000..c7b8acf >>>> --- /dev/null >>>> +++ b/arch/arm/mach-dove/mpp.h >>>> @@ -0,0 +1,214 @@ >>>> +#ifndef __ARCH_DOVE_MPP_CODED_H >>>> +#define __ARCH_DOVE_MPP_CODED_H >>>> + >>>> +#define MPP(_num, _mode, _pmu, _grp, _au1, _nfc) ( ? ? \ >>>> +/* MPP/group number */ ? ? ? ? ((_num) & 0xff) | ? ? ? ? ? ? ? \ >>>> +/* MPP select value */ ? ? ? ? (((_mode) & 0xf) << 8) | ? ? ? ?\ >>>> +/* MPP PMU */ ? ? ? ? ? ? ? ? ?((!!(_pmu)) << 12) | ? ? ? ? ? ?\ >>>> +/* group flag */ ? ? ? ? ? ? ? ((!!(_grp)) << 13) | ? ? ? ? ? ?\ >>>> +/* AU1 flag */ ? ? ? ? ? ? ? ? ((!!(_au1)) << 14) | ? ? ? ? ? ?\ >>>> +/* NFCE flag */ ? ? ? ? ? ? ? ? ? ? ? ?((!!(_nfc)) << 15)) >>> in order to make these defines readable, I suggest to add defines for >>> the flags, for example: >>> #define GROUP ? (1 << 13) >>> then MPP defines look like: >>> >>> #define MPPX_GPIO ? ? ? ? ?MPP(MPP_X, 0x1, GROUP|AU1) >> >> I think we can make it even more readable with >> >> #define MPP_PIN(_num, _sel) ? ? MPP(_num, _sel, 0, 0, 0, 0) >> #define MPP_GRP(_grp, _sel) ? ? MPP(_grp, _sel, 0, 1, 0, 0) >> #define MPP_GRP_AU1(_sel) ? ? ? MPP(0, _sel, 0, 0, 1, 0) >> #define MPP_GRP_NFC(_sel) ? ? ? MPP(0, _sel, 0, 0, 0, 1) >> >> What do you think? > when you have multiple flags in the MPP macro, people would think that > those flags are not exlusive, in other words, mpp can have multiple > flags. so I suggest you to use a type (gpr, au1, nfc or pmu). > Hi Saeed, I think it's a good idea to simplify the macros as Mike suggested, we are doing the same thing for PXA. There is actually another benefit of defining like that in addition to the readability, - it's less error-prone. In practice, there is always a fixed combination of flags for a macro like MPPx_GPIO, even if the flags are combinational. I'm not that enthusiastic but it's something to think about. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] [ARM] Dove: add support for multi-purpose pins configuration 2010-10-27 15:38 ` Eric Miao @ 2010-10-28 7:57 ` Saeed Bishara 0 siblings, 0 replies; 10+ messages in thread From: Saeed Bishara @ 2010-10-28 7:57 UTC (permalink / raw) To: linux-arm-kernel >-----Original Message----- >From: Eric Miao [mailto:eric.y.miao at gmail.com] >Sent: Wednesday, October 27, 2010 5:38 PM >To: saeed bishara >Cc: Mike Rapoport; Saeed Bishara; linux-arm-kernel at lists.infradead.org >Subject: Re: [PATCH 2/2] [ARM] Dove: add support for >multi-purpose pins configuration > >On Wed, Oct 27, 2010 at 11:16 AM, saeed bishara ><saeed.bishara@gmail.com> wrote: >> On Wed, Oct 27, 2010 at 4:30 PM, Mike Rapoport ><mike@compulab.co.il> wrote: >>> On 10/27/10 14:46, saeed bishara wrote: >>>>> + >>>>> + ? ? ? ? ? ? ? shift = (num & 7) << 2; >>>>> + ? ? ? ? ? ? ? if (*mpp_list & MPP_PMU_MASK) { >>>>> + ? ? ? ? ? ? ? ? ? ? ? pmu_mpp_ctrl |= (0x1 << num); >>>>> + ? ? ? ? ? ? ? ? ? ? ? pmu_sig_ctrl[num / 8] &= ~(0xf << shift); >>>>> + ? ? ? ? ? ? ? ? ? ? ? pmu_sig_ctrl[num / 8] |= ~(0xf << shift); >>>> the ?~ in line hints something wrong >>> >>> indeed :) >>> >>>>> diff --git a/arch/arm/mach-dove/mpp.h b/arch/arm/mach-dove/mpp.h >>>>> new file mode 100644 >>>>> index 0000000..c7b8acf >>>>> --- /dev/null >>>>> +++ b/arch/arm/mach-dove/mpp.h >>>>> @@ -0,0 +1,214 @@ >>>>> +#ifndef __ARCH_DOVE_MPP_CODED_H >>>>> +#define __ARCH_DOVE_MPP_CODED_H >>>>> + >>>>> +#define MPP(_num, _mode, _pmu, _grp, _au1, _nfc) ( ? ? \ >>>>> +/* MPP/group number */ ? ? ? ? ((_num) & 0xff) | ? ? ? ? ? ? ? \ >>>>> +/* MPP select value */ ? ? ? ? (((_mode) & 0xf) << 8) | ? ? ? ?\ >>>>> +/* MPP PMU */ ? ? ? ? ? ? ? ? ?((!!(_pmu)) << 12) | ? ? ? ? ? ?\ >>>>> +/* group flag */ ? ? ? ? ? ? ? ((!!(_grp)) << 13) | ? ? ? ? ? ?\ >>>>> +/* AU1 flag */ ? ? ? ? ? ? ? ? ((!!(_au1)) << 14) | ? ? ? ? ? ?\ >>>>> +/* NFCE flag */ ? ? ? ? ? ? ? ? ? ? ? ?((!!(_nfc)) << 15)) >>>> in order to make these defines readable, I suggest to add >defines for >>>> the flags, for example: >>>> #define GROUP ? (1 << 13) >>>> then MPP defines look like: >>>> >>>> #define MPPX_GPIO ? ? ? ? ?MPP(MPP_X, 0x1, GROUP|AU1) >>> >>> I think we can make it even more readable with >>> >>> #define MPP_PIN(_num, _sel) ? ? MPP(_num, _sel, 0, 0, 0, 0) >>> #define MPP_GRP(_grp, _sel) ? ? MPP(_grp, _sel, 0, 1, 0, 0) >>> #define MPP_GRP_AU1(_sel) ? ? ? MPP(0, _sel, 0, 0, 1, 0) >>> #define MPP_GRP_NFC(_sel) ? ? ? MPP(0, _sel, 0, 0, 0, 1) >>> >>> What do you think? >> when you have multiple flags in the MPP macro, people would >think that >> those flags are not exlusive, in other words, mpp can have multiple >> flags. so I suggest you to use a type (gpr, au1, nfc or pmu). >> > >Hi Saeed, > >I think it's a good idea to simplify the macros as Mike >suggested, we are doing >the same thing for PXA. There is actually another benefit of >defining like that >in addition to the readability, - it's less error-prone. > >In practice, there is always a fixed combination of flags for a macro >like MPPx_GPIO, >even if the flags are combinational. > >I'm not that enthusiastic but it's something to think about. Ok, Mike, go ahead with your latest suggestion. > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-10-28 7:57 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-10-27 8:11 [PATCH 0/2] [ARM] Dove: add support for MPP configuration Mike Rapoport 2010-10-27 8:12 ` [PATCH 1/2] [ARM] Dove: add support for GPIOs 64-71 Mike Rapoport 2010-10-27 8:12 ` [PATCH 2/2] [ARM] Dove: add support for multi-purpose pins configuration Mike Rapoport 2010-10-27 9:01 ` Saeed Bishara 2010-10-27 9:15 ` Mike Rapoport 2010-10-27 12:46 ` saeed bishara 2010-10-27 14:30 ` Mike Rapoport 2010-10-27 15:16 ` saeed bishara 2010-10-27 15:38 ` Eric Miao 2010-10-28 7:57 ` Saeed Bishara
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).