On 01/08/2016 09:04 AM, Simon Glass wrote: > Hi Purna, > > On 4 January 2016 at 07:00, Purna Chandra Mandal > <purna.man...@microchip.com> wrote: >> Signed-off-by: Purna Chandra Mandal <purna.man...@microchip.com> >> > Please add a commit message.
Ack. will add. >> --- >> >> Changes in v2: >> - add routine to configure pin properties >> >> drivers/pinctrl/Kconfig | 6 + >> drivers/pinctrl/Makefile | 1 + >> drivers/pinctrl/pinctrl_pic32.c | 284 >> ++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 291 insertions(+) >> create mode 100644 drivers/pinctrl/pinctrl_pic32.c >> >> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig >> index 57e6142..a4acaf3 100644 >> --- a/drivers/pinctrl/Kconfig >> +++ b/drivers/pinctrl/Kconfig >> @@ -131,6 +131,12 @@ config PINCTRL_SANDBOX >> actually does nothing but print debug messages when pinctrl >> operations are invoked. >> >> +config PIC32_PINCTRL >> + bool "Microchip PIC32 pin-control driver" >> + depends on DM && MACH_PIC32 >> + help >> + Support pin multiplexing control on Microchip PIC32 SoCs. > Please add a bit more detail here. What type of functions use pinmux? > Does the pinmux work on a per-pin or per-function basis, or use > groups? Try to add some useful info. Ack. Will add more information here. In PIC32 pin controller is combination of gpio-controller, pin mux and pin config. Remappable peripherals are assigned pins through per-pin based muxing logic. And pin configuration are performed through port registers which are shared along with gpio controller. >> + >> endif >> >> source "drivers/pinctrl/uniphier/Kconfig" >> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile >> index 70d25dc..b4f4650 100644 >> --- a/drivers/pinctrl/Makefile >> +++ b/drivers/pinctrl/Makefile >> @@ -9,3 +9,4 @@ obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip/ >> obj-$(CONFIG_PINCTRL_SANDBOX) += pinctrl-sandbox.o >> >> obj-$(CONFIG_ARCH_UNIPHIER) += uniphier/ >> +obj-$(CONFIG_PIC32_PINCTRL) += pinctrl_pic32.o >> diff --git a/drivers/pinctrl/pinctrl_pic32.c >> b/drivers/pinctrl/pinctrl_pic32.c >> new file mode 100644 >> index 0000000..043f589 >> --- /dev/null >> +++ b/drivers/pinctrl/pinctrl_pic32.c >> @@ -0,0 +1,284 @@ >> +/* >> + * Pinctrl driver for Microchip PIC32 SoCs >> + * Copyright (c) 2015 Microchip Technology Inc. >> + * Written by Purna Chandra Mandal <purna.man...@microchip.com> >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> +#include <common.h> >> +#include <dm.h> >> +#include <errno.h> >> +#include <asm/io.h> >> +#include <dm/pinctrl.h> >> +#include <dm/root.h> >> +#include <mach/pic32.h> >> + >> +DECLARE_GLOBAL_DATA_PTR; >> + >> +/* Peripheral PORTA-PORTK / PORT0-PORT9 */ >> +enum { >> + PIC32_PORT_A = 0, >> + PIC32_PORT_B = 1, >> + PIC32_PORT_C = 2, >> + PIC32_PORT_D = 3, >> + PIC32_PORT_E = 4, >> + PIC32_PORT_F = 5, >> + PIC32_PORT_G = 6, >> + PIC32_PORT_H = 7, >> + PIC32_PORT_J = 8, /* no PORT_I */ >> + PIC32_PORT_K = 9, >> + PIC32_PORT_MAX >> +}; >> + >> +/* Input pinmux reg offset */ >> +#define U1RXR 0x0068 >> +#define U2RXR 0x0070 >> +#define SDI1R 0x009c >> +#define SDI2R 0x00a8 >> + >> +/* Output pinmux reg offset */ >> +#define PPS_OUT(__port, __pin) (((__port) * 16 + (__pin)) << 2) >> + >> +/* Port config/control registers */ >> +struct pic32_reg_port { >> + struct pic32_reg_atomic ansel; > What is pic32_reg_atomic? Can we use u32 instead? For fast and atomic manipulation of registers h/w designers provided a set of interfaces/registers {raw, clear, set, invert} for some of target register. 'struct pic32_reg_atomic' refers to this set as defined in [patch 01/13]. >> + struct pic32_reg_atomic tris; >> + struct pic32_reg_atomic port; >> + struct pic32_reg_atomic lat; >> + struct pic32_reg_atomic odc; >> + struct pic32_reg_atomic cnpu; >> + struct pic32_reg_atomic cnpd; >> + struct pic32_reg_atomic cncon; >> +}; >> + >> +#define PIN_CONFIG_PIC32_DIGITAL (PIN_CONFIG_END + 1) >> +#define PIN_CONFIG_PIC32_ANALOG (PIN_CONFIG_END + 2) >> + >> +struct pic32_pin_config { >> + u16 port; >> + u16 pin; >> + u32 flags; > comments on this structure and members ack. Will add. >> +}; >> + >> +#define PIN_CONFIG(_prt, _pin, _cfg) \ >> + {.port = (_prt), .pin = (_pin), .flags = (_cfg), } >> + >> +enum { >> + PERIPH_ID_UART1, >> + PERIPH_ID_UART2, >> + PERIPH_ID_ETH, >> + PERIPH_ID_USB, >> + PERIPH_ID_SDHCI, >> + PERIPH_ID_I2C1, >> + PERIPH_ID_I2C2, >> + PERIPH_ID_SPI1, >> + PERIPH_ID_SPI2, >> + PERIPH_ID_SQI, >> +}; >> + >> +struct pic32_pinctrl_priv { >> + void __iomem *mux_in; >> + void __iomem *mux_out; >> + void __iomem *pinconf; > Should these be structure pointers? Member 'pinconf' can be declared as 'struct pic32_reg_port' pointer. But ports are not continues (there are big hole between two subsequent ports in address space) so finally its usage needs arithmetic. >> +}; >> + >> +static int pic32_pinconfig_one(struct pic32_pinctrl_priv *priv, >> + u32 port, u32 pin, u32 param) >> +{ >> + struct pic32_reg_port *regs; >> + >> + regs = (struct pic32_reg_port *)(priv->pinconf + port * 0x100); > What is 0x100? It should perhaps be a #define? As mentioned above each pic32_reg_port has large unused address-space and is spaced at 0x100 bytes from the other. Will add #define accordingly. >> + switch (param) { >> + case PIN_CONFIG_PIC32_DIGITAL: >> + writel(BIT(pin), ®s->ansel.clr); >> + break; >> + case PIN_CONFIG_PIC32_ANALOG: >> + writel(BIT(pin), ®s->ansel.set); >> + break; >> + case PIN_CONFIG_INPUT_ENABLE: >> + writel(BIT(pin), ®s->tris.set); >> + break; >> + case PIN_CONFIG_OUTPUT: >> + writel(BIT(pin), ®s->tris.clr); >> + break; >> + case PIN_CONFIG_BIAS_PULL_UP: >> + writel(BIT(pin), ®s->cnpu.set); >> + break; >> + case PIN_CONFIG_BIAS_PULL_DOWN: >> + writel(BIT(pin), ®s->cnpd.set); >> + break; >> + case PIN_CONFIG_DRIVE_OPEN_DRAIN: >> + writel(BIT(pin), ®s->odc.set); >> + break; >> + default: >> + break; >> + } >> + >> + return 0; >> +} >> + >> +static int pic32_pinconfig_set(struct pic32_pinctrl_priv *priv, >> + struct pic32_pin_config *list, int count) >> +{ >> + int i; >> + >> + for (i = 0 ; i < count; i++) >> + pic32_pinconfig_one(priv, list[i].port, >> + list[i].pin, list[i].flags); >> + >> + return 0; >> +} >> + >> +static void _eth_pin_config(struct udevice *dev) >> +{ >> + struct pic32_pinctrl_priv *priv = dev_get_priv(dev); >> + struct pic32_pin_config configs[] = { > static const? ack. >> + /* EMDC - D11 */ >> + PIN_CONFIG(PIC32_PORT_D, 11, PIN_CONFIG_PIC32_DIGITAL), >> + PIN_CONFIG(PIC32_PORT_D, 11, PIN_CONFIG_OUTPUT), >> + /* ETXEN */ >> + PIN_CONFIG(PIC32_PORT_D, 6, PIN_CONFIG_PIC32_DIGITAL), >> + PIN_CONFIG(PIC32_PORT_D, 6, PIN_CONFIG_OUTPUT), >> + /* ECRSDV */ >> + PIN_CONFIG(PIC32_PORT_H, 13, PIN_CONFIG_PIC32_DIGITAL), >> + PIN_CONFIG(PIC32_PORT_H, 13, PIN_CONFIG_INPUT_ENABLE), >> + /* ERXD0 */ >> + PIN_CONFIG(PIC32_PORT_H, 8, PIN_CONFIG_PIC32_DIGITAL), >> + PIN_CONFIG(PIC32_PORT_H, 8, PIN_CONFIG_INPUT_ENABLE), >> + PIN_CONFIG(PIC32_PORT_H, 8, PIN_CONFIG_BIAS_PULL_DOWN), >> + /* ERXD1 */ >> + PIN_CONFIG(PIC32_PORT_H, 5, PIN_CONFIG_PIC32_DIGITAL), >> + PIN_CONFIG(PIC32_PORT_H, 5, PIN_CONFIG_INPUT_ENABLE), >> + PIN_CONFIG(PIC32_PORT_H, 5, PIN_CONFIG_BIAS_PULL_DOWN), >> + /* EREFCLK */ >> + PIN_CONFIG(PIC32_PORT_J, 11, PIN_CONFIG_PIC32_DIGITAL), >> + PIN_CONFIG(PIC32_PORT_J, 11, PIN_CONFIG_INPUT_ENABLE), >> + /* ETXD1 */ >> + PIN_CONFIG(PIC32_PORT_J, 9, PIN_CONFIG_PIC32_DIGITAL), >> + PIN_CONFIG(PIC32_PORT_J, 9, PIN_CONFIG_OUTPUT), >> + /* ETXD0 */ >> + PIN_CONFIG(PIC32_PORT_J, 8, PIN_CONFIG_PIC32_DIGITAL), >> + PIN_CONFIG(PIC32_PORT_J, 8, PIN_CONFIG_OUTPUT), >> + /* EMDIO */ >> + PIN_CONFIG(PIC32_PORT_J, 1, PIN_CONFIG_PIC32_DIGITAL), >> + PIN_CONFIG(PIC32_PORT_J, 1, PIN_CONFIG_INPUT_ENABLE), >> + /* ERXERR */ >> + PIN_CONFIG(PIC32_PORT_F, 3, PIN_CONFIG_PIC32_DIGITAL), >> + PIN_CONFIG(PIC32_PORT_F, 3, PIN_CONFIG_INPUT_ENABLE), >> + }; >> + >> + pic32_pinconfig_set(priv, configs, ARRAY_SIZE(configs)); >> +} >> + >> +static int pic32_pinctrl_request(struct udevice *dev, int func, int flags) >> +{ >> + struct pic32_pinctrl_priv *priv = dev_get_priv(dev); >> + >> + switch (func) { >> + case PERIPH_ID_UART2: >> + /* PPS for U2 RX/TX */ >> + writel(0x02, priv->mux_out + PPS_OUT(PIC32_PORT_G, 9)); >> + writel(0x05, priv->mux_in + U2RXR); /* B0 */ >> + /* set digital mode */ >> + pic32_pinconfig_one(priv, PIC32_PORT_G, 9, >> + PIN_CONFIG_PIC32_DIGITAL); >> + pic32_pinconfig_one(priv, PIC32_PORT_B, 0, >> + PIN_CONFIG_PIC32_DIGITAL); >> + break; >> + case PERIPH_ID_ETH: >> + _eth_pin_config(dev); > Do you need the _? ack. Will remove. >> + break; >> + case PERIPH_ID_SDHCI: > /* No pin mux needed / already set? */ In PIC32 pin-controlling and pin-muxing are required only for remappable peripherals. And non-remappable peripherals have default pins assigned, so no muxing required. SDHCI, USB are example of non-remappable peripherals. >> + break; >> + case PERIPH_ID_USB: >> + break; >> + default: >> + debug("%s: unknown-unhandled case\n", __func__); >> + break; >> + } >> + >> + return 0; >> +} >> + >> +static int pic32_pinctrl_get_periph_id(struct udevice *dev, >> + struct udevice *periph) >> +{ >> + int ret; >> + u32 cell[2]; >> + >> + ret = fdtdec_get_int_array(gd->fdt_blob, periph->of_offset, >> + "interrupts", cell, ARRAY_SIZE(cell)); >> + if (ret < 0) >> + return -EINVAL; >> + >> + /* interrupt number */ >> + switch (cell[0]) { >> + case 112 ... 114: >> + return PERIPH_ID_UART1; >> + case 145 ... 147: >> + return PERIPH_ID_UART2; >> + case 109 ... 111: >> + return PERIPH_ID_SPI1; >> + case 142 ... 144: >> + return PERIPH_ID_SPI2; >> + case 115 ... 117: >> + return PERIPH_ID_I2C1; >> + case 148 ... 150: >> + return PERIPH_ID_I2C2; >> + case 132 ... 133: >> + return PERIPH_ID_USB; >> + case 169: >> + return PERIPH_ID_SQI; >> + case 191: >> + return PERIPH_ID_SDHCI; >> + case 153: >> + return PERIPH_ID_ETH; >> + default: >> + break; >> + } >> + >> + return -ENOENT; >> +} >> + >> +static int pic32_pinctrl_set_state_simple(struct udevice *dev, >> + struct udevice *periph) >> +{ >> + int func; >> + >> + debug("%s: periph %s\n", __func__, periph->name); >> + func = pic32_pinctrl_get_periph_id(dev, periph); >> + if (func < 0) >> + return func; >> + return pic32_pinctrl_request(dev, func, 0); >> +} >> + >> +static struct pinctrl_ops pic32_pinctrl_ops = { >> + .set_state_simple = pic32_pinctrl_set_state_simple, >> + .request = pic32_pinctrl_request, >> + .get_periph_id = pic32_pinctrl_get_periph_id, >> +}; >> + >> +static int pic32_pinctrl_probe(struct udevice *dev) >> +{ >> + struct pic32_pinctrl_priv *priv = dev_get_priv(dev); >> + >> + priv->mux_in = pic32_ioremap(PPS_IN_BASE); >> + priv->mux_out = pic32_ioremap(PPS_OUT_BASE); >> + priv->pinconf = pic32_ioremap(PINCTRL_BASE); > If you are using device tree, why not read these values from there? Ack. Will add. >> + >> + return 0; >> +} >> + >> +static const struct udevice_id pic32_pinctrl_ids[] = { >> + { .compatible = "microchip,pic32mzda-pinctrl" }, >> + { } >> +}; >> + >> +U_BOOT_DRIVER(pinctrl_pic32) = { >> + .name = "pinctrl_pic32", >> + .id = UCLASS_PINCTRL, >> + .of_match = pic32_pinctrl_ids, >> + .ops = &pic32_pinctrl_ops, >> + .probe = pic32_pinctrl_probe, >> + .priv_auto_alloc_size = sizeof(struct pic32_pinctrl_priv), >> +}; >> -- >> 1.8.3.1 >> >> > Regards, > Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot