drivers: pinctrl: Add pinctrl driver for Microchip PIC32 microcontroller

PATCH Superseded Purna Chandra Mandal 2 participants 5 messages Jan 4, 2016 – Jan 12, 2016 Pin control

lore

Share & tools

lore

Purna Chandra Mandal

author

2016-01-04 14:00 UTC #0 lore raw

PATCH v2 3/13 drivers: pinctrl: Add pinctrl driver for Microchip PIC32 microcontroller

Signed-off-by Purna Chandra Mandal <[email protected]>

---

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

drivers/pinctrl/Kconfig +6 -0

index 57e6142..a4acaf3 100644
@@ -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"

drivers/pinctrl/Makefile +1 -0

index 70d25dc..b4f4650 100644
@@ -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

drivers/pinctrl/pinctrl_pic32.c new file +284 -0

284 lines not on this page · raw message

Signature · 1 line

1.8.3.1

Purna Chandra Mandal

author

2016-01-08 06:46 UTC #2 lore raw

PATCH v2 3/13 drivers: pinctrl: Add pinctrl driver for Microchip PIC32 microcontroller

Quoted 7 lines · Simon Glass

Hi Purna,
Quoted 2 lines · Purna Chandra Mandal
Signed-off-by Purna Chandra Mandal <[email protected]>
Please add a commit message.

Ack. will add.

Quoted 19 lines

---

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

drivers/pinctrl/Kconfig +0 -0

index 57e6142..a4acaf3 100644
@@ -131,6 +131,12 @@ config PINCTRL_SANDBOX
          actually does nothing but print debug messages when pinctrl
          operations are invoked.

Quoted 8 lines

drivers/pinctrl/Kconfig config PINCTRL_SANDBOX · line 138
Quoted 5 lines
drivers/pinctrl/Kconfig +5 -0
+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.

Quoted 56 lines

drivers/pinctrl/Kconfig +1 -0

+
 endif
 
 source "drivers/pinctrl/uniphier/Kconfig"

drivers/pinctrl/Makefile +1 -0

index 70d25dc..b4f4650 100644
@@ -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

drivers/pinctrl/pinctrl_pic32.c new file +37 -0

new file mode 100644
index 0000000..043f589
@@ -0,0 +1,284 @@
+/*
+ * Pinctrl driver for Microchip PIC32 SoCs
+ * Copyright (c) 2015 Microchip Technology Inc.
+ * Written by Purna Chandra Mandal <[email protected]>
+ *
+ * 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

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c line 44
Quoted 7 lines
drivers/pinctrl/pinctrl_pic32.c new file +7 -0
+
+/* 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].

Quoted 9 lines

drivers/pinctrl/pinctrl_pic32.c new file +9 -0

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

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c line 60

Quoted 7 lines

drivers/pinctrl/pinctrl_pic32.c new file +7 -0

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

Quoted 15 lines

drivers/pinctrl/pinctrl_pic32.c new file +15 -0

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

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c line 82
Quoted 7 lines
drivers/pinctrl/pinctrl_pic32.c new file +7 -0
+       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.

Quoted 9 lines

drivers/pinctrl/pinctrl_pic32.c line 90

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c new file +8 -0

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

Quoted 38 lines

drivers/pinctrl/pinctrl_pic32.c new file +38 -0

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

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c line 135

Quoted 7 lines

drivers/pinctrl/pinctrl_pic32.c new file +7 -0

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

Quoted 47 lines

drivers/pinctrl/pinctrl_pic32.c new file +47 -0

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

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c line 189

Quoted 7 lines

drivers/pinctrl/pinctrl_pic32.c new file +7 -0

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

Quoted 3 lines

drivers/pinctrl/pinctrl_pic32.c line 191
Quoted 2 lines
drivers/pinctrl/pinctrl_pic32.c new file +2 -0
+               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.

Quoted 69 lines

drivers/pinctrl/pinctrl_pic32.c new file +69 -0

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

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c line 267
Quoted 7 lines
drivers/pinctrl/pinctrl_pic32.c new file +7 -0
+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.

Quoted 23 lines

Quoted 18 lines

drivers/pinctrl/pinctrl_pic32.c new file +17 -1

+
+       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),
+};
--

Quoted 3 lines

1.8.3.1

Regards,
Simon

Simon Glass

2016-01-11 16:58 UTC #3 lore raw

PATCH v2 3/13 drivers: pinctrl: Add pinctrl driver for Microchip PIC32 microcontroller

Hi,

Quoted 111 lines · Purna Chandra Mandal

Quoted 7 lines · Simon Glass
Hi Purna,
Quoted 2 lines · Purna Chandra Mandal
Signed-off-by Purna Chandra Mandal <[email protected]>
Please add a commit message.
Ack. will add.
Quoted 19 lines
Quoted 19 lines
---
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
drivers/pinctrl/Kconfig +0 -0
index 57e6142..a4acaf3 100644
@@ -131,6 +131,12 @@ config PINCTRL_SANDBOX
          actually does nothing but print debug messages when pinctrl
          operations are invoked.
 
Quoted 8 lines
drivers/pinctrl/Kconfig config PINCTRL_SANDBOX · line 138
Quoted 5 lines
drivers/pinctrl/Kconfig +5 -0
+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.
Quoted 56 lines
Quoted 56 lines
drivers/pinctrl/Kconfig +1 -0
+
 endif
 
 source "drivers/pinctrl/uniphier/Kconfig"
drivers/pinctrl/Makefile +1 -0
index 70d25dc..b4f4650 100644
@@ -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
drivers/pinctrl/pinctrl_pic32.c new file +37 -0
new file mode 100644
index 0000000..043f589
@@ -0,0 +1,284 @@
+/*
+ * Pinctrl driver for Microchip PIC32 SoCs
+ * Copyright (c) 2015 Microchip Technology Inc.
+ * Written by Purna Chandra Mandal <[email protected]>
+ *
+ * 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
Quoted 8 lines
drivers/pinctrl/pinctrl_pic32.c line 44
Quoted 7 lines
drivers/pinctrl/pinctrl_pic32.c new file +7 -0
+
+/* 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?

Quoted 3 lines

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.

Quoted 59 lines

Quoted 10 lines

drivers/pinctrl/pinctrl_pic32.c new file +9 -0

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

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c line 60

Quoted 7 lines

drivers/pinctrl/pinctrl_pic32.c new file +7 -0

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

Quoted 15 lines

drivers/pinctrl/pinctrl_pic32.c new file +15 -0

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

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c line 82
Quoted 7 lines
drivers/pinctrl/pinctrl_pic32.c new file +7 -0
+       PERIPH_ID_SQI,
+};
+
+struct pic32_pinctrl_priv {
+       void __iomem *mux_in;
+       void __iomem *mux_out;
+       void __iomem *pinconf;
Should these be structure pointers?

Quoted 9 lines

drivers/pinctrl/pinctrl_pic32.c line 90

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c new file +8 -0

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

Quoted 2 lines

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.

Quoted 108 lines

Quoted 39 lines

drivers/pinctrl/pinctrl_pic32.c new file +38 -0

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

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c line 135

Quoted 7 lines

drivers/pinctrl/pinctrl_pic32.c new file +7 -0

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

Quoted 47 lines

drivers/pinctrl/pinctrl_pic32.c new file +47 -0

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

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c line 189

Quoted 7 lines

drivers/pinctrl/pinctrl_pic32.c new file +7 -0

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

Quoted 7 lines

Quoted 3 lines
drivers/pinctrl/pinctrl_pic32.c line 191
Quoted 2 lines
drivers/pinctrl/pinctrl_pic32.c new file +2 -0
+               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.

Quoted 105 lines

Quoted 70 lines

drivers/pinctrl/pinctrl_pic32.c new file +69 -0

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

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c line 267
Quoted 7 lines
drivers/pinctrl/pinctrl_pic32.c new file +7 -0
+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.

Quoted 23 lines

Quoted 18 lines

drivers/pinctrl/pinctrl_pic32.c new file +17 -1

+
+       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),
+};
--

Quoted 3 lines

1.8.3.1

Regards,
Simon

Purna Chandra Mandal

author

2016-01-12 05:02 UTC #4 lore raw

PATCH v2 3/13 drivers: pinctrl: Add pinctrl driver for Microchip PIC32 microcontroller

Quoted 172 lines · Simon Glass

Hi,
Quoted 108 lines · Purna Chandra Mandal
Quoted 7 lines · Simon Glass
Hi Purna,
Quoted 2 lines · Purna Chandra Mandal
Signed-off-by Purna Chandra Mandal <[email protected]>
Please add a commit message.
Ack. will add.
Quoted 19 lines
Quoted 19 lines
---
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
drivers/pinctrl/Kconfig +0 -0
index 57e6142..a4acaf3 100644
@@ -131,6 +131,12 @@ config PINCTRL_SANDBOX
          actually does nothing but print debug messages when pinctrl
          operations are invoked.
 
Quoted 8 lines
drivers/pinctrl/Kconfig config PINCTRL_SANDBOX · line 138
Quoted 5 lines
drivers/pinctrl/Kconfig +5 -0
+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.
Quoted 56 lines
Quoted 56 lines
drivers/pinctrl/Kconfig +1 -0
+
 endif
 
 source "drivers/pinctrl/uniphier/Kconfig"
drivers/pinctrl/Makefile +1 -0
index 70d25dc..b4f4650 100644
@@ -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
drivers/pinctrl/pinctrl_pic32.c new file +37 -0
new file mode 100644
index 0000000..043f589
@@ -0,0 +1,284 @@
+/*
+ * Pinctrl driver for Microchip PIC32 SoCs
+ * Copyright (c) 2015 Microchip Technology Inc.
+ * Written by Purna Chandra Mandal <[email protected]>
+ *
+ * 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
Quoted 8 lines
drivers/pinctrl/pinctrl_pic32.c line 44
Quoted 7 lines
drivers/pinctrl/pinctrl_pic32.c new file +7 -0
+
+/* 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?
Quoted 3 lines
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.
Quoted 55 lines
Quoted 9 lines
Quoted 9 lines
drivers/pinctrl/pinctrl_pic32.c new file +9 -0
+       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;
+};
+
Quoted 8 lines
drivers/pinctrl/pinctrl_pic32.c line 60
Quoted 7 lines
drivers/pinctrl/pinctrl_pic32.c new file +7 -0
+#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.
Quoted 15 lines
Quoted 15 lines
drivers/pinctrl/pinctrl_pic32.c new file +15 -0
+};
+
+#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,
Quoted 8 lines
drivers/pinctrl/pinctrl_pic32.c line 82
Quoted 7 lines
drivers/pinctrl/pinctrl_pic32.c new file +7 -0
+       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.
Quoted 9 lines
drivers/pinctrl/pinctrl_pic32.c line 90
Quoted 8 lines
drivers/pinctrl/pinctrl_pic32.c new file +8 -0
+};
+
+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?

Quoted 5 lines

Quoted 2 lines
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.

Quoted 216 lines

Quoted 103 lines

Quoted 38 lines

drivers/pinctrl/pinctrl_pic32.c new file +38 -0

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

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c line 135

Quoted 7 lines

drivers/pinctrl/pinctrl_pic32.c new file +7 -0

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

Quoted 47 lines

drivers/pinctrl/pinctrl_pic32.c new file +47 -0

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

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c line 189

Quoted 7 lines

drivers/pinctrl/pinctrl_pic32.c new file +7 -0

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

Quoted 8 lines

ack. Will remove.
Quoted 3 lines
drivers/pinctrl/pinctrl_pic32.c line 191
Quoted 2 lines
drivers/pinctrl/pinctrl_pic32.c new file +2 -0
+               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.

Quoted 102 lines

Quoted 69 lines

drivers/pinctrl/pinctrl_pic32.c new file +69 -0

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

Quoted 8 lines

drivers/pinctrl/pinctrl_pic32.c line 267
Quoted 7 lines
drivers/pinctrl/pinctrl_pic32.c new file +7 -0
+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.

Quoted 23 lines

Quoted 18 lines

drivers/pinctrl/pinctrl_pic32.c new file +17 -1

+
+       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),
+};
--

Quoted 3 lines

1.8.3.1

Regards,
Simon