On 01/11/2016 10:28 PM, Simon Glass wrote: > Hi, > > On 7 January 2016 at 23:46, Purna Chandra Mandal > <purna.man...@microchip.com> wrote: >> 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]. > OK, well it's up to you. > >>>> + 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. > You can add gaps to the struct also, as in padding fields, to make the > struct the right size. Anyway it's not required, a #define is fine > also.
ack. Will add gaps in the struct to match datasheet. >>>> + 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. > OK. I just mean to add a comment here, since you have no code in the > case and it looks incomplete. > >>>> + 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