diff mbox

[U-Boot,v2,03/13] drivers: pinctrl: Add pinctrl driver for Microchip PIC32 microcontroller

Message ID 1451916050.27601.119.camel@microchip.com
State Superseded
Delegated to: Daniel Schwierzeck
Headers show

Commit Message

Purna Chandra Mandal Jan. 4, 2016, 2 p.m. UTC
Signed-off-by: Purna Chandra Mandal <purna.mandal@microchip.com>

---

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

Comments

Simon Glass Jan. 8, 2016, 3:34 a.m. UTC | #1
Hi Purna,

On 4 January 2016 at 07:00, Purna Chandra Mandal
<purna.mandal@microchip.com> wrote:
> Signed-off-by: Purna Chandra Mandal <purna.mandal@microchip.com>
>

Please add a commit message.

> ---
>
> 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.

> +
>  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.mandal@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?

> +       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

> +};
> +
> +#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?

> +};
> +
> +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?

> +       switch (param) {
> +       case PIN_CONFIG_PIC32_DIGITAL:
> +               writel(BIT(pin), &regs->ansel.clr);
> +               break;
> +       case PIN_CONFIG_PIC32_ANALOG:
> +               writel(BIT(pin), &regs->ansel.set);
> +               break;
> +       case PIN_CONFIG_INPUT_ENABLE:
> +               writel(BIT(pin), &regs->tris.set);
> +               break;
> +       case PIN_CONFIG_OUTPUT:
> +               writel(BIT(pin), &regs->tris.clr);
> +               break;
> +       case PIN_CONFIG_BIAS_PULL_UP:
> +               writel(BIT(pin), &regs->cnpu.set);
> +               break;
> +       case PIN_CONFIG_BIAS_PULL_DOWN:
> +               writel(BIT(pin), &regs->cnpd.set);
> +               break;
> +       case PIN_CONFIG_DRIVE_OPEN_DRAIN:
> +               writel(BIT(pin), &regs->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?

> +               /* 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 _?

> +               break;
> +       case PERIPH_ID_SDHCI:

/* No pin mux needed / already set? */

> +               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?

> +
> +       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
Purna Chandra Mandal Jan. 8, 2016, 6:46 a.m. UTC | #2
On 01/08/2016 09:04 AM, Simon Glass wrote:

> Hi Purna,
>
> On 4 January 2016 at 07:00, Purna Chandra Mandal
> <purna.mandal@microchip.com> wrote:
>> Signed-off-by: Purna Chandra Mandal <purna.mandal@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.mandal@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), &regs->ansel.clr);
>> +               break;
>> +       case PIN_CONFIG_PIC32_ANALOG:
>> +               writel(BIT(pin), &regs->ansel.set);
>> +               break;
>> +       case PIN_CONFIG_INPUT_ENABLE:
>> +               writel(BIT(pin), &regs->tris.set);
>> +               break;
>> +       case PIN_CONFIG_OUTPUT:
>> +               writel(BIT(pin), &regs->tris.clr);
>> +               break;
>> +       case PIN_CONFIG_BIAS_PULL_UP:
>> +               writel(BIT(pin), &regs->cnpu.set);
>> +               break;
>> +       case PIN_CONFIG_BIAS_PULL_DOWN:
>> +               writel(BIT(pin), &regs->cnpd.set);
>> +               break;
>> +       case PIN_CONFIG_DRIVE_OPEN_DRAIN:
>> +               writel(BIT(pin), &regs->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
Simon Glass Jan. 11, 2016, 4:58 p.m. UTC | #3
Hi,

On 7 January 2016 at 23:46, Purna Chandra Mandal
<purna.mandal@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.mandal@microchip.com> wrote:
>>> Signed-off-by: Purna Chandra Mandal <purna.mandal@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.mandal@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.

>
>>> +       switch (param) {
>>> +       case PIN_CONFIG_PIC32_DIGITAL:
>>> +               writel(BIT(pin), &regs->ansel.clr);
>>> +               break;
>>> +       case PIN_CONFIG_PIC32_ANALOG:
>>> +               writel(BIT(pin), &regs->ansel.set);
>>> +               break;
>>> +       case PIN_CONFIG_INPUT_ENABLE:
>>> +               writel(BIT(pin), &regs->tris.set);
>>> +               break;
>>> +       case PIN_CONFIG_OUTPUT:
>>> +               writel(BIT(pin), &regs->tris.clr);
>>> +               break;
>>> +       case PIN_CONFIG_BIAS_PULL_UP:
>>> +               writel(BIT(pin), &regs->cnpu.set);
>>> +               break;
>>> +       case PIN_CONFIG_BIAS_PULL_DOWN:
>>> +               writel(BIT(pin), &regs->cnpd.set);
>>> +               break;
>>> +       case PIN_CONFIG_DRIVE_OPEN_DRAIN:
>>> +               writel(BIT(pin), &regs->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
>
Purna Chandra Mandal Jan. 12, 2016, 5:02 a.m. UTC | #4
On 01/11/2016 10:28 PM, Simon Glass wrote:

> Hi,
>
> On 7 January 2016 at 23:46, Purna Chandra Mandal
> <purna.mandal@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.mandal@microchip.com> wrote:
>>>> Signed-off-by: Purna Chandra Mandal <purna.mandal@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.mandal@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), &regs->ansel.clr);
>>>> +               break;
>>>> +       case PIN_CONFIG_PIC32_ANALOG:
>>>> +               writel(BIT(pin), &regs->ansel.set);
>>>> +               break;
>>>> +       case PIN_CONFIG_INPUT_ENABLE:
>>>> +               writel(BIT(pin), &regs->tris.set);
>>>> +               break;
>>>> +       case PIN_CONFIG_OUTPUT:
>>>> +               writel(BIT(pin), &regs->tris.clr);
>>>> +               break;
>>>> +       case PIN_CONFIG_BIAS_PULL_UP:
>>>> +               writel(BIT(pin), &regs->cnpu.set);
>>>> +               break;
>>>> +       case PIN_CONFIG_BIAS_PULL_DOWN:
>>>> +               writel(BIT(pin), &regs->cnpd.set);
>>>> +               break;
>>>> +       case PIN_CONFIG_DRIVE_OPEN_DRAIN:
>>>> +               writel(BIT(pin), &regs->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
diff mbox

Patch

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.
+
 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.mandal@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;
+	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;
+};
+
+#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;
+};
+
+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);
+	switch (param) {
+	case PIN_CONFIG_PIC32_DIGITAL:
+		writel(BIT(pin), &regs->ansel.clr);
+		break;
+	case PIN_CONFIG_PIC32_ANALOG:
+		writel(BIT(pin), &regs->ansel.set);
+		break;
+	case PIN_CONFIG_INPUT_ENABLE:
+		writel(BIT(pin), &regs->tris.set);
+		break;
+	case PIN_CONFIG_OUTPUT:
+		writel(BIT(pin), &regs->tris.clr);
+		break;
+	case PIN_CONFIG_BIAS_PULL_UP:
+		writel(BIT(pin), &regs->cnpu.set);
+		break;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		writel(BIT(pin), &regs->cnpd.set);
+		break;
+	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+		writel(BIT(pin), &regs->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[] = {
+		/* 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);
+		break;
+	case PERIPH_ID_SDHCI:
+		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);
+
+	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),
+};