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

[Lukasz Majewski <lukma@denx.de>]

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>