[U-Boot] [PATCH v2 0/5] Add a pinctrl driver for Merrifield to pinmux I2C#6

[U-Boot] [PATCH v2 0/5] Add a pinctrl driver for Merrifield to pinmux I2C#6

[Georgii Staroselskii]

We have lacked the support for any pinmuxing in U-Boot for Merrifield.
A need for pinmuxing some pins has arisen from the fact the I2C#6 is shared
with I2C#8 on Merrifield. The latter is not easily accessible because it's
a part of a separate MCU we don't have easy access to.

I2C#6 can be and should be made use of in Linux but couldn't because it
was blocked by the SCU.

The proposed change is to implement a minimalistic pinctrl driver that
reads the configuration off a device tree blob and configures the pins 
accordingly.

The driver needs some changes to SCU API and thus the addition of
scu_ipc_raw_command().

Andy Shevchenko has been helping me by making a prior review and made
a lot of suggestions about the general approach that should be taken.

He should also be given credit for the initial kernel code that I have
taken as a reference.

The code has been tested on 5 different Edisons on Intel Edison Breakout
board and a couple of custom Emlid PCBs by booting a kernel and issuing
a i2cdetect -r -y 6 and then loading a driver that made use of the bus.

I also created a Gist on Github with a lot of relevant information like

- output of `acpidump -o tables.dat`
- dmesg
- output of `grep -H 15 /sys/bus/acpi/devices/*/status`
- cat /sys/kernel/debug/gpio
- cat /sys/kernel/debug/pinctrl/INTC1002\:00/pins
- output of `i2cdetect -y -r 6` w/ and w/o external device on the bus

Here it is:
https://gist.github.com/staroselskii/097808e05fd609dbafd4fe5ebd618708

Changes in v2 (Sep 4, 2018)
* fix most of the cosmetic issues
* allow setting pinmodes other than 1
* add a missing docstring to scu_ipc_command()


Georgii Staroselskii (5):
  x86: cpu: introduce scu_ipc_raw_command()
  x86: tangier: pinmux: add API to configure protected pins
  x86: dts: edison: configure I2C#6 pins
  x86: tangier: acpi: add I2C6 node
  x86: cpu: add docstring to scu_ipc_command()

 arch/x86/cpu/tangier/Makefile                      |   2 +-
 arch/x86/cpu/tangier/pinmux.c                      | 196 +++++++++++++++++++++
 arch/x86/dts/edison.dts                            |  22 +++
 .../include/asm/arch-tangier/acpi/southcluster.asl |  10 ++
 arch/x86/include/asm/scu.h                         |   4 +
 arch/x86/lib/scu.c                                 |  62 +++++++
 6 files changed, 295 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/cpu/tangier/pinmux.c

-- 
2.7.4

[U-Boot] [PATCH v2 1/5] x86: cpu: introduce scu_ipc_raw_command()

[Georgii Staroselskii]

This interface will be used to configure properly some pins on
Merrifield that are shared with SCU.

scu_ipc_raw_command() writes SPTR and DPTR registers before sending
a command to SCU.

This code has been ported from Linux work done by Andy Shevchenko.

Signed-off-by: Georgii Staroselskii <georgii.staroselskii@emlid.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 arch/x86/include/asm/scu.h |  4 ++++
 arch/x86/lib/scu.c         | 51 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/arch/x86/include/asm/scu.h b/arch/x86/include/asm/scu.h
index 7ce5824..f5ec5a1 100644
--- a/arch/x86/include/asm/scu.h
+++ b/arch/x86/include/asm/scu.h
@@ -6,6 +6,8 @@
 #define _X86_ASM_SCU_IPC_H_
 
 /* IPC defines the following message types */
+#define IPCMSG_INDIRECT_READ	0x02
+#define IPCMSG_INDIRECT_WRITE	0x05
 #define IPCMSG_WARM_RESET	0xf0
 #define IPCMSG_COLD_RESET	0xf1
 #define IPCMSG_SOFT_RESET	0xf2
@@ -23,5 +25,7 @@ struct ipc_ifwi_version {
 /* Issue commands to the SCU with or without data */
 int scu_ipc_simple_command(u32 cmd, u32 sub);
 int scu_ipc_command(u32 cmd, u32 sub, u32 *in, int inlen, u32 *out, int outlen);
+int scu_ipc_raw_command(u32 cmd, u32 sub, u32 *in, int inlen, u32 *out,
+			int outlen, u32 dptr, u32 sptr);
 
 #endif	/* _X86_ASM_SCU_IPC_H_ */
diff --git a/arch/x86/lib/scu.c b/arch/x86/lib/scu.c
index caa04c6..4054838 100644
--- a/arch/x86/lib/scu.c
+++ b/arch/x86/lib/scu.c
@@ -102,6 +102,57 @@ static int scu_ipc_cmd(struct ipc_regs *regs, u32 cmd, u32 sub,
 }
 
 /**
+ * scu_ipc_raw_command() - IPC command with data and pointers
+ * @cmd:    IPC command code.
+ * @sub:    IPC command sub type.
+ * @in:     input data of this IPC command.
+ * @inlen:  input data length in dwords.
+ * @out:    output data of this IPC command.
+ * @outlen: output data length in dwords.
+ * @dptr:   data writing to SPTR register.
+ * @sptr:   data writing to DPTR register.
+ *
+ * Send an IPC command to SCU with input/output data and source/dest pointers.
+ *
+ * Return:  an IPC error code or 0 on success.
+ */
+int scu_ipc_raw_command(u32 cmd, u32 sub, u32 *in, int inlen, u32 *out,
+			int outlen, u32 dptr, u32 sptr)
+{
+	int inbuflen = DIV_ROUND_UP(inlen, 4);
+	struct udevice *dev;
+	struct scu *scu;
+	int ret;
+
+	ret = syscon_get_by_driver_data(X86_SYSCON_SCU, &dev);
+	if (ret)
+		return ret;
+
+	scu = dev_get_priv(dev);
+
+	/* Up to 16 bytes */
+	if (inbuflen > 4)
+		return -EINVAL;
+
+	writel(dptr, &scu->regs->dptr);
+	writel(sptr, &scu->regs->sptr);
+
+	/*
+	 * SRAM controller doesn't support 8-bit writes, it only
+	 * supports 32-bit writes, so we have to copy input data into
+	 * the temporary buffer, and SCU FW will use the inlen to
+	 * determine the actual input data length in the temporary
+	 * buffer.
+	 */
+
+	u32 inbuf[4] = {0};
+
+	memcpy(inbuf, in, inlen);
+
+	return scu_ipc_cmd(scu->regs, cmd, sub, inbuf, inlen, out, outlen);
+}
+
+/**
  * scu_ipc_simple_command() - send a simple command
  * @cmd: command
  * @sub: sub type
-- 
2.7.4

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

[Georgii Staroselskii]

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

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] [PATCH v2 2/5] x86: tangier: pinmux: add API to configure protected pins

[Andy Shevchenko]

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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot



-- 
With Best Regards,
Andy Shevchenko

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

[Simon Glass]

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

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

[Georgii Staroselskii]

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?

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

[Simon Glass]

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

[U-Boot] [PATCH v2 3/5] x86: dts: edison: configure I2C#6 pins

[Georgii Staroselskii]

Now that we have the pinctrl driver for Merrifield in place we can make
use of it and set I2C#6 pins appropriately.

Initial configuration came from the firmware.  Which quite likely has
been used in the phones, where that is not part of Atom peripheral, is
in use. Thus we need to override the leftover.

Signed-off-by: Georgii Staroselskii <georgii.staroselskii@emlid.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
 arch/x86/dts/edison.dts | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/x86/dts/edison.dts b/arch/x86/dts/edison.dts
index 1da7f54..0dd7240 100644
--- a/arch/x86/dts/edison.dts
+++ b/arch/x86/dts/edison.dts
@@ -85,4 +85,26 @@
 		compatible = "intel,reset-tangier";
 		u-boot,dm-pre-reloc;
 	};
+
+	pinctrl {
+		compatible = "intel,pinctrl-tangier";
+		reg = <0xff0c0000 0x8000>;
+
+		/*
+		 * Initial configuration came from the firmware.
+		 * Which quite likely has been used in the phones, where I2C #8,
+		 * that is not part of Atom peripheral, is in use.
+		 * Thus we need to override the leftover.
+		 */
+		i2c6_scl at 0 {
+			pad-offset = <111>;
+			mode-func = <1>;
+			protected;
+		};
+		i2c6_sda at 0 {
+			pad-offset = <112>;
+			mode-func = <1>;
+			protected;
+		};
+	};
 };
-- 
2.7.4

[U-Boot] [PATCH v2 4/5] x86: tangier: acpi: add I2C6 node

[Georgii Staroselskii]

Now that we have I2C#6 working, it's time to add a corresponsing
ACPI binding.

Signed-off-by: Georgii Staroselskii <georgii.staroselskii@emlid.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
 arch/x86/include/asm/arch-tangier/acpi/southcluster.asl | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/include/asm/arch-tangier/acpi/southcluster.asl b/arch/x86/include/asm/arch-tangier/acpi/southcluster.asl
index b200e9f..7cdc4b2 100644
--- a/arch/x86/include/asm/arch-tangier/acpi/southcluster.asl
+++ b/arch/x86/include/asm/arch-tangier/acpi/southcluster.asl
@@ -231,6 +231,16 @@ Device (PCI0)
         }
     }
 
+    Device (I2C6)
+    {
+        Name (_ADR, 0x00090001)
+
+        Method (_STA, 0, NotSerialized)
+        {
+            Return (STA_VISIBLE)
+        }
+    }
+
     Device (GPIO)
     {
         Name (_ADR, 0x000c0000)
-- 
2.7.4

[U-Boot] [PATCH v2 5/5] x86: cpu: add docstring to scu_ipc_command()

[Georgii Staroselskii]

These comments were copied from the Linux kernel driver in
drivers/platform/x86/intel_scu_ipc.c

Signed-off-by: Georgii Staroselskii <georgii.staroselskii@emlid.com>
---
 arch/x86/lib/scu.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/x86/lib/scu.c b/arch/x86/lib/scu.c
index 4054838..ab945d2 100644
--- a/arch/x86/lib/scu.c
+++ b/arch/x86/lib/scu.c
@@ -180,6 +180,17 @@ int scu_ipc_simple_command(u32 cmd, u32 sub)
 	return scu_ipc_check_status(scu->regs);
 }
 
+/**
+ *  scu_ipc_command   -   command with data
+ *  @cmd: command
+ *  @sub: sub type
+ *  @in: input data
+ *  @inlen: input length in dwords
+ *  @out: output data
+ *  @outlen: output length in dwords
+ *
+ *  Issue a command to the SCU which involves data transfers.
+ */
 int scu_ipc_command(u32 cmd, u32 sub, u32 *in, int inlen, u32 *out, int outlen)
 {
 	struct scu *scu;
-- 
2.7.4

[U-Boot] [PATCH v2 5/5] x86: cpu: add docstring to scu_ipc_command()

[Andy Shevchenko]

On Tue, Sep 4, 2018 at 5:37 PM Georgii Staroselskii
<georgii.staroselskii@emlid.com> wrote:
>
> These comments were copied from the Linux kernel driver in
> drivers/platform/x86/intel_scu_ipc.c
>

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

> Signed-off-by: Georgii Staroselskii <georgii.staroselskii@emlid.com>
> ---
>  arch/x86/lib/scu.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/arch/x86/lib/scu.c b/arch/x86/lib/scu.c
> index 4054838..ab945d2 100644
> --- a/arch/x86/lib/scu.c
> +++ b/arch/x86/lib/scu.c
> @@ -180,6 +180,17 @@ int scu_ipc_simple_command(u32 cmd, u32 sub)
>         return scu_ipc_check_status(scu->regs);
>  }
>
> +/**
> + *  scu_ipc_command   -   command with data
> + *  @cmd: command
> + *  @sub: sub type
> + *  @in: input data
> + *  @inlen: input length in dwords
> + *  @out: output data
> + *  @outlen: output length in dwords
> + *
> + *  Issue a command to the SCU which involves data transfers.
> + */
>  int scu_ipc_command(u32 cmd, u32 sub, u32 *in, int inlen, u32 *out, int outlen)
>  {
>         struct scu *scu;
> --
> 2.7.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot



-- 
With Best Regards,
Andy Shevchenko

[U-Boot] [PATCH v2 0/5] Add a pinctrl driver for Merrifield to pinmux I2C#6

[Bin Meng]

On Tue, Sep 4, 2018 at 10:34 PM Georgii Staroselskii
<georgii.staroselskii@emlid.com> wrote:
>
> We have lacked the support for any pinmuxing in U-Boot for Merrifield.
> A need for pinmuxing some pins has arisen from the fact the I2C#6 is shared
> with I2C#8 on Merrifield. The latter is not easily accessible because it's
> a part of a separate MCU we don't have easy access to.
>
> I2C#6 can be and should be made use of in Linux but couldn't because it
> was blocked by the SCU.
>
> The proposed change is to implement a minimalistic pinctrl driver that
> reads the configuration off a device tree blob and configures the pins
> accordingly.
>
> The driver needs some changes to SCU API and thus the addition of
> scu_ipc_raw_command().
>
> Andy Shevchenko has been helping me by making a prior review and made
> a lot of suggestions about the general approach that should be taken.
>
> He should also be given credit for the initial kernel code that I have
> taken as a reference.
>
> The code has been tested on 5 different Edisons on Intel Edison Breakout
> board and a couple of custom Emlid PCBs by booting a kernel and issuing
> a i2cdetect -r -y 6 and then loading a driver that made use of the bus.
>
> I also created a Gist on Github with a lot of relevant information like
>
> - output of `acpidump -o tables.dat`
> - dmesg
> - output of `grep -H 15 /sys/bus/acpi/devices/*/status`
> - cat /sys/kernel/debug/gpio
> - cat /sys/kernel/debug/pinctrl/INTC1002\:00/pins
> - output of `i2cdetect -y -r 6` w/ and w/o external device on the bus
>
> Here it is:
> https://gist.github.com/staroselskii/097808e05fd609dbafd4fe5ebd618708
>
> Changes in v2 (Sep 4, 2018)
> * fix most of the cosmetic issues
> * allow setting pinmodes other than 1
> * add a missing docstring to scu_ipc_command()
>

Somehow I did not receive all 5 patches in the v2. Only 2/3/5 is in my
inbox. It looks good and I will wait some comments from others (if
any) for another day or two before applying this.

>
> Georgii Staroselskii (5):
>   x86: cpu: introduce scu_ipc_raw_command()
>   x86: tangier: pinmux: add API to configure protected pins
>   x86: dts: edison: configure I2C#6 pins
>   x86: tangier: acpi: add I2C6 node
>   x86: cpu: add docstring to scu_ipc_command()
>
>  arch/x86/cpu/tangier/Makefile                      |   2 +-
>  arch/x86/cpu/tangier/pinmux.c                      | 196 +++++++++++++++++++++
>  arch/x86/dts/edison.dts                            |  22 +++
>  .../include/asm/arch-tangier/acpi/southcluster.asl |  10 ++
>  arch/x86/include/asm/scu.h                         |   4 +
>  arch/x86/lib/scu.c                                 |  62 +++++++
>  6 files changed, 295 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/cpu/tangier/pinmux.c
>

Regards,
Bin

[U-Boot] [PATCH v2 0/5] Add a pinctrl driver for Merrifield to pinmux I2C#6

[Andy Shevchenko]

On Wed, Sep 5, 2018 at 7:41 AM Bin Meng <bmeng.cn@gmail.com> wrote:

> Somehow I did not receive all 5 patches in the v2. Only 2/3/5 is in my
> inbox. It looks good and I will wait some comments from others (if
> any) for another day or two before applying this.

Hmm... At least now I can see your email address in the list of mails
I got received.
But taking into consideration previous experience with the mails from
Georgii I might point to his end for problems.

Nevertheless, all of them are available through patchwork:
https://patchwork.ozlabs.org/patch/965948/

> > Georgii Staroselskii (5):
> >   x86: cpu: introduce scu_ipc_raw_command()
> >   x86: tangier: pinmux: add API to configure protected pins
> >   x86: dts: edison: configure I2C#6 pins
> >   x86: tangier: acpi: add I2C6 node
> >   x86: cpu: add docstring to scu_ipc_command()



-- 
With Best Regards,
Andy Shevchenko