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

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

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

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), +};

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

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

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?

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

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@0 { + pad-offset = <111>; + mode-func = <1>; + protected; + }; + i2c6_sda@0 { + pad-offset = <112>; + mode-func = <1>; + protected; + }; + }; };

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)

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;

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

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

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()
participants (4)
-
Andy Shevchenko
-
Bin Meng
-
Georgii Staroselskii
-
Simon Glass