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

Marek Vasut
2019-06-17T10:23:04Z

On 6/17/19 9:43 AM, Lukasz Majewski wrote:
> 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.

Please do

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

Ah, because you have a compat string for the GPIO controllers, right.
Then please ignore this comment, this is better solution.

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

--
Best regards,
Marek Vasut