diff mbox series

[U-Boot,v2,2/5] x86: tangier: pinmux: add API to configure protected pins

Message ID 1536071645-25229-3-git-send-email-georgii.staroselskii@emlid.com
State Superseded
Delegated to: Bin Meng
Headers show
Series Add a pinctrl driver for Merrifield to pinmux I2C#6 | expand

Commit Message

Georgii Staroselskii Sept. 4, 2018, 2:34 p.m. UTC
This API is going to be used to configure some pins that are protected
for simple modification.

It's not a comprehensive pinctrl driver but can be turned into one
when we need this in the future. Now it is planned to be used only
in one place. So that's why I decided not to pollute the codebase with a
full-blown pinctrl-merrifield nobody will use.

This driver reads corresponding fields in DT and configures pins
accordingly.

The "protected" flag is used to distinguish configuration of SCU-owned
pins from the ordinary ones.

The code has been adapted from Linux work done by Andy Shevchenko
in pinctrl-merrfifield.c

Signed-off-by: Georgii Staroselskii <georgii.staroselskii@emlid.com>
---
 arch/x86/cpu/tangier/Makefile |   2 +-
 arch/x86/cpu/tangier/pinmux.c | 196 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 197 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/cpu/tangier/pinmux.c

Comments

Andy Shevchenko Sept. 4, 2018, 2:57 p.m. UTC | #1
On Tue, Sep 4, 2018 at 5:34 PM Georgii Staroselskii
<georgii.staroselskii@emlid.com> wrote:
>
> This API is going to be used to configure some pins that are protected
> for simple modification.
>
> It's not a comprehensive pinctrl driver but can be turned into one
> when we need this in the future. Now it is planned to be used only
> in one place. So that's why I decided not to pollute the codebase with a
> full-blown pinctrl-merrifield nobody will use.
>
> This driver reads corresponding fields in DT and configures pins
> accordingly.
>
> The "protected" flag is used to distinguish configuration of SCU-owned
> pins from the ordinary ones.
>
> The code has been adapted from Linux work done by Andy Shevchenko
> in pinctrl-merrfifield.c
>

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Georgii Staroselskii <georgii.staroselskii@emlid.com>
> ---
>  arch/x86/cpu/tangier/Makefile |   2 +-
>  arch/x86/cpu/tangier/pinmux.c | 196 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 197 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/cpu/tangier/pinmux.c
>
> diff --git a/arch/x86/cpu/tangier/Makefile b/arch/x86/cpu/tangier/Makefile
> index 8274482..68f4a32 100644
> --- a/arch/x86/cpu/tangier/Makefile
> +++ b/arch/x86/cpu/tangier/Makefile
> @@ -2,5 +2,5 @@
>  #
>  # Copyright (c) 2017 Intel Corporation
>
> -obj-y += car.o tangier.o sdram.o sysreset.o
> +obj-y += car.o tangier.o sdram.o sysreset.o pinmux.o
>  obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi.o
> diff --git a/arch/x86/cpu/tangier/pinmux.c b/arch/x86/cpu/tangier/pinmux.c
> new file mode 100644
> index 0000000..4a9fc89
> --- /dev/null
> +++ b/arch/x86/cpu/tangier/pinmux.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2018 Emlid Limited
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <dm/pinctrl.h>
> +#include <fdtdec.h>
> +#include <regmap.h>
> +#include <syscon.h>
> +#include <asm/cpu.h>
> +#include <asm/scu.h>
> +#include <linux/io.h>
> +
> +#define BUFCFG_OFFSET                          0x100
> +
> +#define MRFLD_FAMILY_LEN                       0x400
> +
> +/* These are taken from Linux kernel */
> +#define MRFLD_PINMODE_MASK                     0x07
> +
> +#define pin_to_bufno(f, p)                     ((p) - (f)->pin_base)
> +
> +struct mrfld_family {
> +       unsigned int family_number;
> +       unsigned int pin_base;
> +       size_t npins;
> +       void __iomem *regs;
> +};
> +
> +#define MRFLD_FAMILY(b, s, e)                          \
> +       {                                               \
> +               .family_number = (b),                   \
> +               .pin_base = (s),                        \
> +               .npins = (e) - (s) + 1,                 \
> +       }
> +
> +/* Now we only support I2C family of pins */
> +static struct mrfld_family mrfld_families[] = {
> +       MRFLD_FAMILY(7, 101, 114),
> +};
> +
> +struct mrfld_pinctrl {
> +       const struct mrfld_family *families;
> +       size_t nfamilies;
> +};
> +
> +static const struct mrfld_family *
> +mrfld_get_family(struct mrfld_pinctrl *mp, unsigned int pin)
> +{
> +       const struct mrfld_family *family;
> +       unsigned int i;
> +
> +       for (i = 0; i < mp->nfamilies; i++) {
> +               family = &mp->families[i];
> +               if (pin >= family->pin_base &&
> +                   pin < family->pin_base + family->npins)
> +                       return family;
> +       }
> +
> +       pr_err("failed to find family for pin %u\n", pin);
> +       return NULL;
> +}
> +
> +static void __iomem *
> +mrfld_get_bufcfg(struct mrfld_pinctrl *pinctrl, unsigned int pin)
> +{
> +       const struct mrfld_family *family;
> +       unsigned int bufno;
> +
> +       family =  mrfld_get_family(pinctrl, pin);
> +       if (!family)
> +               return NULL;
> +
> +       bufno = pin_to_bufno(family, pin);
> +
> +       return family->regs + BUFCFG_OFFSET + bufno * 4;
> +}
> +
> +static void
> +mrfld_setup_families(void *base_addr,
> +                    struct mrfld_family *families, unsigned int nfam)
> +{
> +       for (int i = 0; i < nfam; i++) {
> +               struct mrfld_family *family = &families[i];
> +
> +               family->regs = base_addr +
> +                              family->family_number * MRFLD_FAMILY_LEN;
> +       }
> +}
> +
> +static int mrfld_pinconfig_protected(unsigned int pin, u32 mask, u32 bits)
> +{
> +       struct mrfld_pinctrl *pinctrl;
> +       struct udevice *dev;
> +       void __iomem *bufcfg;
> +       u32 v, value;
> +       int ret;
> +
> +       ret = syscon_get_by_driver_data(X86_SYSCON_PINCONF, &dev);
> +       if (ret)
> +               return ret;
> +
> +       pinctrl = dev_get_priv(dev);
> +
> +       bufcfg = mrfld_get_bufcfg(pinctrl, pin);
> +       if (!bufcfg)
> +               return -EINVAL;
> +
> +       value = readl(bufcfg);
> +
> +       v = (value & ~mask) | (bits & mask);
> +
> +       debug("scu: v: 0x%x p: 0x%x bits: %d, mask: %d bufcfg: 0x%p\n",
> +             v, (u32)bufcfg, bits, mask, bufcfg);
> +
> +       ret = scu_ipc_raw_command(IPCMSG_INDIRECT_WRITE, 0, &v, 4,
> +                                 NULL, 0, (u32)bufcfg, 0);
> +       if (ret)
> +               pr_err("Failed to set mode via SCU for pin %u (%d)\n",
> +                      pin, ret);
> +
> +       return ret;
> +}
> +
> +static int mrfld_pinctrl_cfg_pin(int pin_node)
> +{
> +       bool is_protected;
> +       int pad_offset;
> +       int mode;
> +       u32 mask;
> +       int ret;
> +
> +       /* For now we only support just protected Family of pins */
> +       is_protected = fdtdec_get_bool(gd->fdt_blob, pin_node, "protected");
> +       if (!is_protected)
> +               return -ENOTSUPP;
> +
> +       pad_offset = fdtdec_get_int(gd->fdt_blob, pin_node, "pad-offset", -1);
> +       if (pad_offset == -1)
> +               return -EINVAL;
> +
> +       mode = fdtdec_get_int(gd->fdt_blob, pin_node, "mode-func", -1);
> +       if (mode == -1)
> +               return -EINVAL;
> +
> +       mask = MRFLD_PINMODE_MASK;
> +
> +       /* We don't support modes not in range 0..7 */
> +       if (mode & ~mask)
> +               return -ENOTSUPP;
> +
> +       ret = mrfld_pinconfig_protected(pad_offset, mask, mode);
> +
> +       return ret;
> +}
> +
> +static int tangier_pinctrl_probe(struct udevice *dev)
> +{
> +       void *base_addr = syscon_get_first_range(X86_SYSCON_PINCONF);
> +       struct mrfld_pinctrl *pinctrl = dev_get_priv(dev);
> +       int pin_node;
> +       int ret;
> +
> +       mrfld_setup_families(base_addr, mrfld_families,
> +                            ARRAY_SIZE(mrfld_families));
> +
> +       pinctrl->families = mrfld_families;
> +       pinctrl->nfamilies = ARRAY_SIZE(mrfld_families);
> +
> +       for (pin_node = fdt_first_subnode(gd->fdt_blob, dev_of_offset(dev));
> +            pin_node > 0;
> +            pin_node = fdt_next_subnode(gd->fdt_blob, pin_node)) {
> +               ret = mrfld_pinctrl_cfg_pin(pin_node);
> +               if (ret) {
> +                       pr_err("%s: invalid configuration for the pin %d\n",
> +                              __func__, pin_node);
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct udevice_id tangier_pinctrl_match[] = {
> +       { .compatible = "intel,pinctrl-tangier", .data = X86_SYSCON_PINCONF },
> +       { /* sentinel */ }
> +};
> +
> +U_BOOT_DRIVER(tangier_pinctrl) = {
> +       .name = "tangier_pinctrl",
> +       .id = UCLASS_SYSCON,
> +       .of_match = tangier_pinctrl_match,
> +       .probe = tangier_pinctrl_probe,
> +       .priv_auto_alloc_size = sizeof(struct mrfld_pinctrl),
> +};
> --
> 2.7.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Simon Glass Sept. 5, 2018, 3:24 p.m. UTC | #2
Hi Georgi,

On 4 September 2018 at 07:34, Georgii Staroselskii
<georgii.staroselskii@emlid.com> wrote:
> This API is going to be used to configure some pins that are protected
> for simple modification.
>
> It's not a comprehensive pinctrl driver but can be turned into one
> when we need this in the future. Now it is planned to be used only
> in one place. So that's why I decided not to pollute the codebase with a
> full-blown pinctrl-merrifield nobody will use.
>
> This driver reads corresponding fields in DT and configures pins
> accordingly.
>
> The "protected" flag is used to distinguish configuration of SCU-owned
> pins from the ordinary ones.
>
> The code has been adapted from Linux work done by Andy Shevchenko
> in pinctrl-merrfifield.c
>
> Signed-off-by: Georgii Staroselskii <georgii.staroselskii@emlid.com>
> ---
>  arch/x86/cpu/tangier/Makefile |   2 +-
>  arch/x86/cpu/tangier/pinmux.c | 196 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 197 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/cpu/tangier/pinmux.c
>

Please can you use the livetree API (dev_read_...)?

Regards,
Simon
Georgii Staroselskii Sept. 5, 2018, 3:44 p.m. UTC | #3
On Wed, Sep 05, 2018 at 09:24:40AM -0600, Simon Glass wrote:
> Hi Georgi,
> 
> On 4 September 2018 at 07:34, Georgii Staroselskii
> <georgii.staroselskii@emlid.com> wrote:
> > This API is going to be used to configure some pins that are protected
> > for simple modification.
> >
> > It's not a comprehensive pinctrl driver but can be turned into one
> > when we need this in the future. Now it is planned to be used only
> > in one place. So that's why I decided not to pollute the codebase with a
> > full-blown pinctrl-merrifield nobody will use.
> >
> > This driver reads corresponding fields in DT and configures pins
> > accordingly.
> >
> > The "protected" flag is used to distinguish configuration of SCU-owned
> > pins from the ordinary ones.
> >
> > The code has been adapted from Linux work done by Andy Shevchenko
> > in pinctrl-merrfifield.c
> >
> > Signed-off-by: Georgii Staroselskii <georgii.staroselskii@emlid.com>
> > ---
> >  arch/x86/cpu/tangier/Makefile |   2 +-
> >  arch/x86/cpu/tangier/pinmux.c | 196 ++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 197 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/x86/cpu/tangier/pinmux.c
> >
> 
> Please can you use the livetree API (dev_read_...)?
> 
> Regards,
> Simon

Sure. Will do. It will need CONFIG_OF_LIVE=y for edison_defconfig to be
set. Is there any other modifications or possible regressions that I need
to take into account? Or if I just stick to doc/driver-model/livetree.txt
things should go smoothly?
Simon Glass Sept. 9, 2018, 1:06 a.m. UTC | #4
Hi Georgii,


On 5 September 2018 at 09:44, Georgii Staroselskii
<georgii.staroselskii@emlid.com> wrote:
>
> On Wed, Sep 05, 2018 at 09:24:40AM -0600, Simon Glass wrote:
> > Hi Georgi,
> >
> > On 4 September 2018 at 07:34, Georgii Staroselskii
> > <georgii.staroselskii@emlid.com> wrote:
> > > This API is going to be used to configure some pins that are protected
> > > for simple modification.
> > >
> > > It's not a comprehensive pinctrl driver but can be turned into one
> > > when we need this in the future. Now it is planned to be used only
> > > in one place. So that's why I decided not to pollute the codebase with a
> > > full-blown pinctrl-merrifield nobody will use.
> > >
> > > This driver reads corresponding fields in DT and configures pins
> > > accordingly.
> > >
> > > The "protected" flag is used to distinguish configuration of SCU-owned
> > > pins from the ordinary ones.
> > >
> > > The code has been adapted from Linux work done by Andy Shevchenko
> > > in pinctrl-merrfifield.c
> > >
> > > Signed-off-by: Georgii Staroselskii <georgii.staroselskii@emlid.com>
> > > ---
> > >  arch/x86/cpu/tangier/Makefile |   2 +-
> > >  arch/x86/cpu/tangier/pinmux.c | 196 ++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 197 insertions(+), 1 deletion(-)
> > >  create mode 100644 arch/x86/cpu/tangier/pinmux.c
> > >
> >
> > Please can you use the livetree API (dev_read_...)?
> >
> > Regards,
> > Simon
>
> Sure. Will do. It will need CONFIG_OF_LIVE=y for edison_defconfig to be
> set. Is there any other modifications or possible regressions that I need
> to take into account? Or if I just stick to doc/driver-model/livetree.txt
> things should go smoothly?

We lack unit tests in the code - currently it is tested by the
standard sandbox driver-model tests. which run in flat-tree and
live-tree versions. I did hit a problem recently, so be a little
suspicious. But normally it is transparent.

Note that flat-tree is used before relocation and in SPL, regardless
of the setting of CONFIG_OF_LIVE.

You don't really need to enable OF_LIVE if you don't want to. That is
actually a separate thing from which API you use.

Regards,
Simon
diff mbox series

Patch

diff --git a/arch/x86/cpu/tangier/Makefile b/arch/x86/cpu/tangier/Makefile
index 8274482..68f4a32 100644
--- a/arch/x86/cpu/tangier/Makefile
+++ b/arch/x86/cpu/tangier/Makefile
@@ -2,5 +2,5 @@ 
 #
 # Copyright (c) 2017 Intel Corporation
 
-obj-y += car.o tangier.o sdram.o sysreset.o
+obj-y += car.o tangier.o sdram.o sysreset.o pinmux.o
 obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi.o
diff --git a/arch/x86/cpu/tangier/pinmux.c b/arch/x86/cpu/tangier/pinmux.c
new file mode 100644
index 0000000..4a9fc89
--- /dev/null
+++ b/arch/x86/cpu/tangier/pinmux.c
@@ -0,0 +1,196 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2018 Emlid Limited
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <dm/pinctrl.h>
+#include <fdtdec.h>
+#include <regmap.h>
+#include <syscon.h>
+#include <asm/cpu.h>
+#include <asm/scu.h>
+#include <linux/io.h>
+
+#define BUFCFG_OFFSET				0x100
+
+#define MRFLD_FAMILY_LEN			0x400
+
+/* These are taken from Linux kernel */
+#define MRFLD_PINMODE_MASK			0x07
+
+#define pin_to_bufno(f, p)			((p) - (f)->pin_base)
+
+struct mrfld_family {
+	unsigned int family_number;
+	unsigned int pin_base;
+	size_t npins;
+	void __iomem *regs;
+};
+
+#define MRFLD_FAMILY(b, s, e)				\
+	{						\
+		.family_number = (b),			\
+		.pin_base = (s),			\
+		.npins = (e) - (s) + 1,			\
+	}
+
+/* Now we only support I2C family of pins */
+static struct mrfld_family mrfld_families[] = {
+	MRFLD_FAMILY(7, 101, 114),
+};
+
+struct mrfld_pinctrl {
+	const struct mrfld_family *families;
+	size_t nfamilies;
+};
+
+static const struct mrfld_family *
+mrfld_get_family(struct mrfld_pinctrl *mp, unsigned int pin)
+{
+	const struct mrfld_family *family;
+	unsigned int i;
+
+	for (i = 0; i < mp->nfamilies; i++) {
+		family = &mp->families[i];
+		if (pin >= family->pin_base &&
+		    pin < family->pin_base + family->npins)
+			return family;
+	}
+
+	pr_err("failed to find family for pin %u\n", pin);
+	return NULL;
+}
+
+static void __iomem *
+mrfld_get_bufcfg(struct mrfld_pinctrl *pinctrl, unsigned int pin)
+{
+	const struct mrfld_family *family;
+	unsigned int bufno;
+
+	family =  mrfld_get_family(pinctrl, pin);
+	if (!family)
+		return NULL;
+
+	bufno = pin_to_bufno(family, pin);
+
+	return family->regs + BUFCFG_OFFSET + bufno * 4;
+}
+
+static void
+mrfld_setup_families(void *base_addr,
+		     struct mrfld_family *families, unsigned int nfam)
+{
+	for (int i = 0; i < nfam; i++) {
+		struct mrfld_family *family = &families[i];
+
+		family->regs = base_addr +
+			       family->family_number * MRFLD_FAMILY_LEN;
+	}
+}
+
+static int mrfld_pinconfig_protected(unsigned int pin, u32 mask, u32 bits)
+{
+	struct mrfld_pinctrl *pinctrl;
+	struct udevice *dev;
+	void __iomem *bufcfg;
+	u32 v, value;
+	int ret;
+
+	ret = syscon_get_by_driver_data(X86_SYSCON_PINCONF, &dev);
+	if (ret)
+		return ret;
+
+	pinctrl = dev_get_priv(dev);
+
+	bufcfg = mrfld_get_bufcfg(pinctrl, pin);
+	if (!bufcfg)
+		return -EINVAL;
+
+	value = readl(bufcfg);
+
+	v = (value & ~mask) | (bits & mask);
+
+	debug("scu: v: 0x%x p: 0x%x bits: %d, mask: %d bufcfg: 0x%p\n",
+	      v, (u32)bufcfg, bits, mask, bufcfg);
+
+	ret = scu_ipc_raw_command(IPCMSG_INDIRECT_WRITE, 0, &v, 4,
+				  NULL, 0, (u32)bufcfg, 0);
+	if (ret)
+		pr_err("Failed to set mode via SCU for pin %u (%d)\n",
+		       pin, ret);
+
+	return ret;
+}
+
+static int mrfld_pinctrl_cfg_pin(int pin_node)
+{
+	bool is_protected;
+	int pad_offset;
+	int mode;
+	u32 mask;
+	int ret;
+
+	/* For now we only support just protected Family of pins */
+	is_protected = fdtdec_get_bool(gd->fdt_blob, pin_node, "protected");
+	if (!is_protected)
+		return -ENOTSUPP;
+
+	pad_offset = fdtdec_get_int(gd->fdt_blob, pin_node, "pad-offset", -1);
+	if (pad_offset == -1)
+		return -EINVAL;
+
+	mode = fdtdec_get_int(gd->fdt_blob, pin_node, "mode-func", -1);
+	if (mode == -1)
+		return -EINVAL;
+
+	mask = MRFLD_PINMODE_MASK;
+
+	/* We don't support modes not in range 0..7 */
+	if (mode & ~mask)
+		return -ENOTSUPP;
+
+	ret = mrfld_pinconfig_protected(pad_offset, mask, mode);
+
+	return ret;
+}
+
+static int tangier_pinctrl_probe(struct udevice *dev)
+{
+	void *base_addr = syscon_get_first_range(X86_SYSCON_PINCONF);
+	struct mrfld_pinctrl *pinctrl = dev_get_priv(dev);
+	int pin_node;
+	int ret;
+
+	mrfld_setup_families(base_addr, mrfld_families,
+			     ARRAY_SIZE(mrfld_families));
+
+	pinctrl->families = mrfld_families;
+	pinctrl->nfamilies = ARRAY_SIZE(mrfld_families);
+
+	for (pin_node = fdt_first_subnode(gd->fdt_blob, dev_of_offset(dev));
+	     pin_node > 0;
+	     pin_node = fdt_next_subnode(gd->fdt_blob, pin_node)) {
+		ret = mrfld_pinctrl_cfg_pin(pin_node);
+		if (ret) {
+			pr_err("%s: invalid configuration for the pin %d\n",
+			       __func__, pin_node);
+		}
+	}
+
+	return 0;
+}
+
+static const struct udevice_id tangier_pinctrl_match[] = {
+	{ .compatible = "intel,pinctrl-tangier", .data = X86_SYSCON_PINCONF },
+	{ /* sentinel */ }
+};
+
+U_BOOT_DRIVER(tangier_pinctrl) = {
+	.name = "tangier_pinctrl",
+	.id = UCLASS_SYSCON,
+	.of_match = tangier_pinctrl_match,
+	.probe = tangier_pinctrl_probe,
+	.priv_auto_alloc_size = sizeof(struct mrfld_pinctrl),
+};