[U-Boot] [PATCH v3 4/5] ARM: imx: pinctrl: Add support for i.MX28 mxs pinctrl driver

Lukasz Majewski
2019-06-17T07:43:46Z

Hi Marek,

> On 6/16/19 12:34 AM, Lukasz Majewski wrote:
> > The code responsible for setting proper values in the MUX registers
> > (in the mxs_pinctrl_set_state()) has been ported from Barebox
> > project (branch: master, SHA1:
> > eb3b0f7414cd8102844dd16b1c789e445e8947f8, file:
> > drivers/pinctrl/pinctrl-mxs.c).
>
> The format of a commit, when referenced, is documented here:
> https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html
> e.g. e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")
>
> Although. maybe you should port this from Linux instead ?

The code from linux is a bit more convoluted and verbose. I may re-look
into the kernel version.

>
> > As the pinctrl node in the imx28.dtsi file has gpio pins nodes as
> > subnodes, it was necessary to use 'dm_scan_fdt_dev()' (as a .bind
> > method) to also make them 'visible' by the DM's "gpio_mxs" driver.
>
> Look at drivers/pinctrl/renesas/pfc-r7s72100.c r7s72100_pfc_probe() ,
> I think that one deals with the exact same problem .

It looks like this is the same problem.

However, the dm_scan_fdt_dev() is exactly for that, when combined
with .bind method from DM.

>
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> >
> > ---
> >
> > Changes in v3:
> > - Set more apropriate tags
> >
> > Changes in v2: None
> >
> > drivers/pinctrl/nxp/Kconfig | 10 +++
> > drivers/pinctrl/nxp/Makefile | 1 +
> > drivers/pinctrl/nxp/pinctrl-mxs.c | 164
> > ++++++++++++++++++++++++++++++++++++++ 3 files changed, 175
> > insertions(+) create mode 100644 drivers/pinctrl/nxp/pinctrl-mxs.c
> >
> > diff --git a/drivers/pinctrl/nxp/Kconfig
> > b/drivers/pinctrl/nxp/Kconfig index 61f93be42d..f2e67ca231 100644
> > --- a/drivers/pinctrl/nxp/Kconfig
> > +++ b/drivers/pinctrl/nxp/Kconfig
> > @@ -89,6 +89,16 @@ config PINCTRL_IMX8M
> > only parses the 'fsl,pins' property and configure related
> > registers.
> >
> > +config PINCTRL_MXS
> > + bool "NXP MXS pinctrl driver"
> > + depends on ARCH_MX28 && PINCTRL_FULL
> > + help
> > + Say Y here to enable the i.MX mxs pinctrl driver
> > +
> > + This option provides a simple pinctrl driver for i.MX
> > mxs SoC
> > + familiy, e.g. i.MX28. This feature depends on device tree
> > + configuration.
> > +
> > config PINCTRL_VYBRID
> > bool "Vybrid (vf610) pinctrl driver"
> > depends on ARCH_VF610 && PINCTRL_FULL
> > diff --git a/drivers/pinctrl/nxp/Makefile
> > b/drivers/pinctrl/nxp/Makefile index b340d9448a..b86448aac9 100644
> > --- a/drivers/pinctrl/nxp/Makefile
> > +++ b/drivers/pinctrl/nxp/Makefile
> > @@ -6,4 +6,5 @@ obj-$(CONFIG_PINCTRL_IMX7ULP) +=
> > pinctrl-imx7ulp.o obj-$(CONFIG_PINCTRL_IMX_SCU) +=
> > pinctrl-scu.o obj-$(CONFIG_PINCTRL_IMX8) +=
> > pinctrl-imx8.o obj-$(CONFIG_PINCTRL_IMX8M) +=
> > pinctrl-imx8m.o +obj-$(CONFIG_PINCTRL_MXS) +=
> > pinctrl-mxs.o obj-$(CONFIG_PINCTRL_VYBRID) +=
> > pinctrl-vf610.o diff --git a/drivers/pinctrl/nxp/pinctrl-mxs.c
> > b/drivers/pinctrl/nxp/pinctrl-mxs.c new file mode 100644
> > index 0000000000..42b1b7998b
> > --- /dev/null
> > +++ b/drivers/pinctrl/nxp/pinctrl-mxs.c
> > @@ -0,0 +1,164 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2019 DENX Software Engineering
> > + * Lukasz Majewski, DENX Software Engineering, lukma at denx.de
> > + *
> > + * This work is based on drivers/pinctrl/pinctrl-mxs.c
> > + * SHA1: eb3b0f7414cd8102844dd16b1c789e445e8947f8
> > + * from Barebox project.
> > + *
> > + * Author of original Barebox pinctrl-mxs.c:
> > + * Copyright (c) 2015 Sascha Hauer <s.hauer@pengutronix.de>
> > + */
> > +
> > +#include <common.h>
> > +#include <linux/io.h>
> > +#include <linux/err.h>
> > +#include <dm.h>
> > +#include <dm/pinctrl.h>
> > +#include <dm/read.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +#define PINID(bank, pin) ((bank) * 32 + (pin))
> > +#define MUXID_TO_PINID(m) PINID((m) >> 12 & 0xf, (m) >> 4 &
> > 0xff) +#define MUXID_TO_MUXSEL(m) ((m) & 0xf)
> > +#define PINID_TO_BANK(p) ((p) >> 5)
> > +#define PINID_TO_PIN(p) ((p) % 32)
> > +
> > +#define STMP_OFFSET_REG_SET 0x4
> > +#define STMP_OFFSET_REG_CLR 0x8
> > +#define STMP_OFFSET_REG_TOG 0xc
> > +
> > +struct mxs_pinctrl_priv {
> > + void __iomem *base;
> > +};
> > +
> > +static int mxs_pinctrl_set_state(struct udevice *dev, struct
> > udevice *config) +{
> > + int ma_present = 0, vol_present = 0, pull_present = 0;
> > + struct mxs_pinctrl_priv *iomux = dev_get_priv(dev);
> > + u32 *pin_data, val, ma, vol, pull;
> > + int npins, size, i, ret;
> > +
> > + debug("\n%s: set state: %s\n", __func__, config->name);
> > +
> > + size = dev_read_size(config, "fsl,pinmux-ids");
> > + if (size < 0)
> > + return size;
> > +
> > + if (!size || size % 4) {
> > + dev_err(dev, "Invalid fsl,pinmux-ids property in
> > %s\n",
> > + config->name);
> > + return -EINVAL;
> > + }
> > +
> > + npins = size / 4;
> > +
> > + pin_data = devm_kzalloc(dev, size, 0);
>
> Is this ever free'd() ?
>

Ach... right the devm_kfree() shall be used on the exit from the
function.

> [...]
>

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190617/91817d89/attachment.sig>