[U-Boot] [PATCH v2 03/13] drivers: pinctrl: Add pinctrl driver for Microchip PIC32 microcontroller
Purna Chandra Mandal
purna.mandal at microchip.com
Tue Jan 12 06:02:42 CET 2016
On 01/11/2016 10:28 PM, Simon Glass wrote:
> Hi,
>
> On 7 January 2016 at 23:46, Purna Chandra Mandal
> <purna.mandal at 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 at microchip.com> wrote:
>>>> Signed-off-by: Purna Chandra Mandal <purna.mandal at 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 at microchip.com>
>>>> + *
>>>> + * SPDX-License-Identifier: GPL-2.0+
>>>> + */
>>>> +#include <common.h>
>>>> +#include <dm.h>
>>>> +#include <errno.h>
>>>> +#include <asm/io.h>
>>>> +#include <dm/pinctrl.h>
>>>> +#include <dm/root.h>
>>>> +#include <mach/pic32.h>
>>>> +
>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>> +
>>>> +/* Peripheral PORTA-PORTK / PORT0-PORT9 */
>>>> +enum {
>>>> + PIC32_PORT_A = 0,
>>>> + PIC32_PORT_B = 1,
>>>> + PIC32_PORT_C = 2,
>>>> + PIC32_PORT_D = 3,
>>>> + PIC32_PORT_E = 4,
>>>> + PIC32_PORT_F = 5,
>>>> + PIC32_PORT_G = 6,
>>>> + PIC32_PORT_H = 7,
>>>> + PIC32_PORT_J = 8, /* no PORT_I */
>>>> + PIC32_PORT_K = 9,
>>>> + PIC32_PORT_MAX
>>>> +};
>>>> +
>>>> +/* Input pinmux reg offset */
>>>> +#define U1RXR 0x0068
>>>> +#define U2RXR 0x0070
>>>> +#define SDI1R 0x009c
>>>> +#define SDI2R 0x00a8
>>>> +
>>>> +/* Output pinmux reg offset */
>>>> +#define PPS_OUT(__port, __pin) (((__port) * 16 + (__pin)) << 2)
>>>> +
>>>> +/* Port config/control registers */
>>>> +struct pic32_reg_port {
>>>> + struct pic32_reg_atomic ansel;
>>> What is pic32_reg_atomic? Can we use u32 instead?
>> For fast and atomic manipulation of registers h/w designers provided a
>> set of interfaces/registers {raw, clear, set, invert} for some of target register.
>> 'struct pic32_reg_atomic' refers to this set as defined in [patch 01/13].
> OK, well it's up to you.
>
>>>> + struct pic32_reg_atomic tris;
>>>> + struct pic32_reg_atomic port;
>>>> + struct pic32_reg_atomic lat;
>>>> + struct pic32_reg_atomic odc;
>>>> + struct pic32_reg_atomic cnpu;
>>>> + struct pic32_reg_atomic cnpd;
>>>> + struct pic32_reg_atomic cncon;
>>>> +};
>>>> +
>>>> +#define PIN_CONFIG_PIC32_DIGITAL (PIN_CONFIG_END + 1)
>>>> +#define PIN_CONFIG_PIC32_ANALOG (PIN_CONFIG_END + 2)
>>>> +
>>>> +struct pic32_pin_config {
>>>> + u16 port;
>>>> + u16 pin;
>>>> + u32 flags;
>>> comments on this structure and members
>> ack. Will add.
>>
>>>> +};
>>>> +
>>>> +#define PIN_CONFIG(_prt, _pin, _cfg) \
>>>> + {.port = (_prt), .pin = (_pin), .flags = (_cfg), }
>>>> +
>>>> +enum {
>>>> + PERIPH_ID_UART1,
>>>> + PERIPH_ID_UART2,
>>>> + PERIPH_ID_ETH,
>>>> + PERIPH_ID_USB,
>>>> + PERIPH_ID_SDHCI,
>>>> + PERIPH_ID_I2C1,
>>>> + PERIPH_ID_I2C2,
>>>> + PERIPH_ID_SPI1,
>>>> + PERIPH_ID_SPI2,
>>>> + PERIPH_ID_SQI,
>>>> +};
>>>> +
>>>> +struct pic32_pinctrl_priv {
>>>> + void __iomem *mux_in;
>>>> + void __iomem *mux_out;
>>>> + void __iomem *pinconf;
>>> Should these be structure pointers?
>> Member 'pinconf' can be declared as 'struct pic32_reg_port' pointer.
>> But ports are not continues (there are big hole between two subsequent ports in address space)
>> so finally its usage needs arithmetic.
>>
>>>> +};
>>>> +
>>>> +static int pic32_pinconfig_one(struct pic32_pinctrl_priv *priv,
>>>> + u32 port, u32 pin, u32 param)
>>>> +{
>>>> + struct pic32_reg_port *regs;
>>>> +
>>>> + regs = (struct pic32_reg_port *)(priv->pinconf + port * 0x100);
>>> What is 0x100? It should perhaps be a #define?
>> As mentioned above each pic32_reg_port has large unused address-space and is spaced at 0x100 bytes from the other.
>> Will add #define accordingly.
> You can add gaps to the struct also, as in padding fields, to make the
> struct the right size. Anyway it's not required, a #define is fine
> also.
ack. Will add gaps in the struct to match datasheet.
>>>> + switch (param) {
>>>> + case PIN_CONFIG_PIC32_DIGITAL:
>>>> + writel(BIT(pin), ®s->ansel.clr);
>>>> + break;
>>>> + case PIN_CONFIG_PIC32_ANALOG:
>>>> + writel(BIT(pin), ®s->ansel.set);
>>>> + break;
>>>> + case PIN_CONFIG_INPUT_ENABLE:
>>>> + writel(BIT(pin), ®s->tris.set);
>>>> + break;
>>>> + case PIN_CONFIG_OUTPUT:
>>>> + writel(BIT(pin), ®s->tris.clr);
>>>> + break;
>>>> + case PIN_CONFIG_BIAS_PULL_UP:
>>>> + writel(BIT(pin), ®s->cnpu.set);
>>>> + break;
>>>> + case PIN_CONFIG_BIAS_PULL_DOWN:
>>>> + writel(BIT(pin), ®s->cnpd.set);
>>>> + break;
>>>> + case PIN_CONFIG_DRIVE_OPEN_DRAIN:
>>>> + writel(BIT(pin), ®s->odc.set);
>>>> + break;
>>>> + default:
>>>> + break;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int pic32_pinconfig_set(struct pic32_pinctrl_priv *priv,
>>>> + struct pic32_pin_config *list, int count)
>>>> +{
>>>> + int i;
>>>> +
>>>> + for (i = 0 ; i < count; i++)
>>>> + pic32_pinconfig_one(priv, list[i].port,
>>>> + list[i].pin, list[i].flags);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void _eth_pin_config(struct udevice *dev)
>>>> +{
>>>> + struct pic32_pinctrl_priv *priv = dev_get_priv(dev);
>>>> + struct pic32_pin_config configs[] = {
>>> static const?
>> ack.
>>
>>>> + /* EMDC - D11 */
>>>> + PIN_CONFIG(PIC32_PORT_D, 11, PIN_CONFIG_PIC32_DIGITAL),
>>>> + PIN_CONFIG(PIC32_PORT_D, 11, PIN_CONFIG_OUTPUT),
>>>> + /* ETXEN */
>>>> + PIN_CONFIG(PIC32_PORT_D, 6, PIN_CONFIG_PIC32_DIGITAL),
>>>> + PIN_CONFIG(PIC32_PORT_D, 6, PIN_CONFIG_OUTPUT),
>>>> + /* ECRSDV */
>>>> + PIN_CONFIG(PIC32_PORT_H, 13, PIN_CONFIG_PIC32_DIGITAL),
>>>> + PIN_CONFIG(PIC32_PORT_H, 13, PIN_CONFIG_INPUT_ENABLE),
>>>> + /* ERXD0 */
>>>> + PIN_CONFIG(PIC32_PORT_H, 8, PIN_CONFIG_PIC32_DIGITAL),
>>>> + PIN_CONFIG(PIC32_PORT_H, 8, PIN_CONFIG_INPUT_ENABLE),
>>>> + PIN_CONFIG(PIC32_PORT_H, 8, PIN_CONFIG_BIAS_PULL_DOWN),
>>>> + /* ERXD1 */
>>>> + PIN_CONFIG(PIC32_PORT_H, 5, PIN_CONFIG_PIC32_DIGITAL),
>>>> + PIN_CONFIG(PIC32_PORT_H, 5, PIN_CONFIG_INPUT_ENABLE),
>>>> + PIN_CONFIG(PIC32_PORT_H, 5, PIN_CONFIG_BIAS_PULL_DOWN),
>>>> + /* EREFCLK */
>>>> + PIN_CONFIG(PIC32_PORT_J, 11, PIN_CONFIG_PIC32_DIGITAL),
>>>> + PIN_CONFIG(PIC32_PORT_J, 11, PIN_CONFIG_INPUT_ENABLE),
>>>> + /* ETXD1 */
>>>> + PIN_CONFIG(PIC32_PORT_J, 9, PIN_CONFIG_PIC32_DIGITAL),
>>>> + PIN_CONFIG(PIC32_PORT_J, 9, PIN_CONFIG_OUTPUT),
>>>> + /* ETXD0 */
>>>> + PIN_CONFIG(PIC32_PORT_J, 8, PIN_CONFIG_PIC32_DIGITAL),
>>>> + PIN_CONFIG(PIC32_PORT_J, 8, PIN_CONFIG_OUTPUT),
>>>> + /* EMDIO */
>>>> + PIN_CONFIG(PIC32_PORT_J, 1, PIN_CONFIG_PIC32_DIGITAL),
>>>> + PIN_CONFIG(PIC32_PORT_J, 1, PIN_CONFIG_INPUT_ENABLE),
>>>> + /* ERXERR */
>>>> + PIN_CONFIG(PIC32_PORT_F, 3, PIN_CONFIG_PIC32_DIGITAL),
>>>> + PIN_CONFIG(PIC32_PORT_F, 3, PIN_CONFIG_INPUT_ENABLE),
>>>> + };
>>>> +
>>>> + pic32_pinconfig_set(priv, configs, ARRAY_SIZE(configs));
>>>> +}
>>>> +
>>>> +static int pic32_pinctrl_request(struct udevice *dev, int func, int flags)
>>>> +{
>>>> + struct pic32_pinctrl_priv *priv = dev_get_priv(dev);
>>>> +
>>>> + switch (func) {
>>>> + case PERIPH_ID_UART2:
>>>> + /* PPS for U2 RX/TX */
>>>> + writel(0x02, priv->mux_out + PPS_OUT(PIC32_PORT_G, 9));
>>>> + writel(0x05, priv->mux_in + U2RXR); /* B0 */
>>>> + /* set digital mode */
>>>> + pic32_pinconfig_one(priv, PIC32_PORT_G, 9,
>>>> + PIN_CONFIG_PIC32_DIGITAL);
>>>> + pic32_pinconfig_one(priv, PIC32_PORT_B, 0,
>>>> + PIN_CONFIG_PIC32_DIGITAL);
>>>> + break;
>>>> + case PERIPH_ID_ETH:
>>>> + _eth_pin_config(dev);
>>> Do you need the _?
>> ack. Will remove.
>>
>>>> + break;
>>>> + case PERIPH_ID_SDHCI:
>>> /* No pin mux needed / already set? */
>> In PIC32 pin-controlling and pin-muxing are required only for remappable peripherals.
>> And non-remappable peripherals have default pins assigned, so no muxing required.
>> SDHCI, USB are example of non-remappable peripherals.
> OK. I just mean to add a comment here, since you have no code in the
> case and it looks incomplete.
>
>>>> + break;
>>>> + case PERIPH_ID_USB:
>>>> + break;
>>>> + default:
>>>> + debug("%s: unknown-unhandled case\n", __func__);
>>>> + break;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int pic32_pinctrl_get_periph_id(struct udevice *dev,
>>>> + struct udevice *periph)
>>>> +{
>>>> + int ret;
>>>> + u32 cell[2];
>>>> +
>>>> + ret = fdtdec_get_int_array(gd->fdt_blob, periph->of_offset,
>>>> + "interrupts", cell, ARRAY_SIZE(cell));
>>>> + if (ret < 0)
>>>> + return -EINVAL;
>>>> +
>>>> + /* interrupt number */
>>>> + switch (cell[0]) {
>>>> + case 112 ... 114:
>>>> + return PERIPH_ID_UART1;
>>>> + case 145 ... 147:
>>>> + return PERIPH_ID_UART2;
>>>> + case 109 ... 111:
>>>> + return PERIPH_ID_SPI1;
>>>> + case 142 ... 144:
>>>> + return PERIPH_ID_SPI2;
>>>> + case 115 ... 117:
>>>> + return PERIPH_ID_I2C1;
>>>> + case 148 ... 150:
>>>> + return PERIPH_ID_I2C2;
>>>> + case 132 ... 133:
>>>> + return PERIPH_ID_USB;
>>>> + case 169:
>>>> + return PERIPH_ID_SQI;
>>>> + case 191:
>>>> + return PERIPH_ID_SDHCI;
>>>> + case 153:
>>>> + return PERIPH_ID_ETH;
>>>> + default:
>>>> + break;
>>>> + }
>>>> +
>>>> + return -ENOENT;
>>>> +}
>>>> +
>>>> +static int pic32_pinctrl_set_state_simple(struct udevice *dev,
>>>> + struct udevice *periph)
>>>> +{
>>>> + int func;
>>>> +
>>>> + debug("%s: periph %s\n", __func__, periph->name);
>>>> + func = pic32_pinctrl_get_periph_id(dev, periph);
>>>> + if (func < 0)
>>>> + return func;
>>>> + return pic32_pinctrl_request(dev, func, 0);
>>>> +}
>>>> +
>>>> +static struct pinctrl_ops pic32_pinctrl_ops = {
>>>> + .set_state_simple = pic32_pinctrl_set_state_simple,
>>>> + .request = pic32_pinctrl_request,
>>>> + .get_periph_id = pic32_pinctrl_get_periph_id,
>>>> +};
>>>> +
>>>> +static int pic32_pinctrl_probe(struct udevice *dev)
>>>> +{
>>>> + struct pic32_pinctrl_priv *priv = dev_get_priv(dev);
>>>> +
>>>> + priv->mux_in = pic32_ioremap(PPS_IN_BASE);
>>>> + priv->mux_out = pic32_ioremap(PPS_OUT_BASE);
>>>> + priv->pinconf = pic32_ioremap(PINCTRL_BASE);
>>> If you are using device tree, why not read these values from there?
>> Ack. Will add.
>>
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static const struct udevice_id pic32_pinctrl_ids[] = {
>>>> + { .compatible = "microchip,pic32mzda-pinctrl" },
>>>> + { }
>>>> +};
>>>> +
>>>> +U_BOOT_DRIVER(pinctrl_pic32) = {
>>>> + .name = "pinctrl_pic32",
>>>> + .id = UCLASS_PINCTRL,
>>>> + .of_match = pic32_pinctrl_ids,
>>>> + .ops = &pic32_pinctrl_ops,
>>>> + .probe = pic32_pinctrl_probe,
>>>> + .priv_auto_alloc_size = sizeof(struct pic32_pinctrl_priv),
>>>> +};
>>>> --
>>>> 1.8.3.1
>>>>
>>>>
>>> Regards,
>>> Simon
More information about the U-Boot
mailing list