* [PATCH v2 0/6] mfd: Add support for Khadas Microcontroller
@ 2020-05-12 13:26 Neil Armstrong
2020-05-12 13:26 ` [PATCH v2 1/6] dt-bindings: mfd: add Khadas Microcontroller bindings Neil Armstrong
` (5 more replies)
0 siblings, 6 replies; 18+ messages in thread
From: Neil Armstrong @ 2020-05-12 13:26 UTC (permalink / raw)
To: khilman, lee.jones, rui.zhang, daniel.lezcano, amit.kucheria,
srinivas.kandagatla
Cc: Neil Armstrong, linux-amlogic, linux-pm, linux-arm-kernel, linux-kernel
The new Khadas VIM2, VIM3 and Edge boards embeds an on-board microcontroller
connected via I2C.
This Microcontroller is present on the Khadas VIM1, VIM2, VIM3 and Edge
boards.
It has multiple boot control features like password check, power-on
options, power-off control and system FAN control on recent boards.
Thie serie adds :
- the bindings
- the MFD driver
- the HWMON cell driver
- the NVMEM cell driver
- updates MAINTAINERS
- add support into the Khadas VIM3/VIM3L DT
Patch 6 depends on [1].
Changes since RFC v1 at [2]:
- moved hwmon driver to thermal-only
- moved the SM1 thermal nodes in a separate serie
- added the bindings review tag from rob
[1] http://lore.kernel.org/r/20200512093916.19676-1-narmstrong@baylibre.com
[2] http://lore.kernel.org/r/20200421080102.22796-1-narmstrong@baylibre.com
Neil Armstrong (6):
dt-bindings: mfd: add Khadas Microcontroller bindings
mfd: add support for the Khadas System control Microcontroller
thermal: add support for the MCU controlled FAN on Khadas boards
nvmem: add support for the Khadas MCU Programmable User Memory
MAINTAINERS: add myself as maintainer for Khadas MCU drivers
arm64: dts: meson-khadas-vim3: add Khadas MCU nodes
.../devicetree/bindings/mfd/khadas,mcu.yaml | 44 +++++
MAINTAINERS | 10 +
.../boot/dts/amlogic/meson-khadas-vim3.dtsi | 23 +++
drivers/mfd/Kconfig | 14 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/khadas-mcu.c | 143 ++++++++++++++
drivers/nvmem/Kconfig | 8 +
drivers/nvmem/Makefile | 2 +
drivers/nvmem/khadas-mcu-user-mem.c | 128 +++++++++++++
drivers/thermal/Kconfig | 10 +
drivers/thermal/Makefile | 1 +
drivers/thermal/khadas_mcu_fan.c | 174 ++++++++++++++++++
include/linux/mfd/khadas-mcu.h | 91 +++++++++
13 files changed, 649 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
create mode 100644 drivers/mfd/khadas-mcu.c
create mode 100644 drivers/nvmem/khadas-mcu-user-mem.c
create mode 100644 drivers/thermal/khadas_mcu_fan.c
create mode 100644 include/linux/mfd/khadas-mcu.h
--
2.22.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/6] dt-bindings: mfd: add Khadas Microcontroller bindings
2020-05-12 13:26 [PATCH v2 0/6] mfd: Add support for Khadas Microcontroller Neil Armstrong
@ 2020-05-12 13:26 ` Neil Armstrong
2020-05-15 6:42 ` Amit Kucheria
2020-05-12 13:26 ` [PATCH v2 2/6] mfd: add support for the Khadas System control Microcontroller Neil Armstrong
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Neil Armstrong @ 2020-05-12 13:26 UTC (permalink / raw)
To: lee.jones, rui.zhang, daniel.lezcano, amit.kucheria,
srinivas.kandagatla, devicetree
Cc: Neil Armstrong, linux-amlogic, linux-pm, linux-arm-kernel,
linux-kernel, Rob Herring
This Microcontroller is present on the Khadas VIM1, VIM2, VIM3 and Edge
boards.
It has multiple boot control features like password check, power-on
options, power-off control and system FAN control on recent boards.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/mfd/khadas,mcu.yaml | 44 +++++++++++++++++++
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
diff --git a/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml b/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
new file mode 100644
index 000000000000..a3b976f101e8
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/khadas,mcu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Khadas on-board Microcontroller Device Tree Bindings
+
+maintainers:
+ - Neil Armstrong <narmstrong@baylibre.com>
+
+description: |
+ Khadas embeds a microcontroller on their VIM and Edge boards adding some
+ system feature as PWM Fan control (for VIM2 rev14 or VIM3), User memory
+ storage, IR/Key resume control, system power LED control and more.
+
+properties:
+ compatible:
+ enum:
+ - khadas,mcu # MCU revision is discoverable
+
+ "#cooling-cells": # Only needed for boards having FAN control feature
+ const: 2
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ khadas_mcu: system-controller@18 {
+ compatible = "khadas,mcu";
+ reg = <0x18>;
+ #cooling-cells = <2>;
+ };
+ };
--
2.22.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/6] mfd: add support for the Khadas System control Microcontroller
2020-05-12 13:26 [PATCH v2 0/6] mfd: Add support for Khadas Microcontroller Neil Armstrong
2020-05-12 13:26 ` [PATCH v2 1/6] dt-bindings: mfd: add Khadas Microcontroller bindings Neil Armstrong
@ 2020-05-12 13:26 ` Neil Armstrong
2020-05-20 9:01 ` Lee Jones
2020-05-12 13:26 ` [PATCH v2 3/6] thermal: add support for the MCU controlled FAN on Khadas boards Neil Armstrong
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Neil Armstrong @ 2020-05-12 13:26 UTC (permalink / raw)
To: lee.jones
Cc: Neil Armstrong, linux-amlogic, linux-pm, linux-arm-kernel, linux-kernel
This Microcontroller is present on the Khadas VIM1, VIM2, VIM3 and Edge
boards.
It has multiple boot control features like password check, power-on
options, power-off control and system FAN control on recent boards.
This implements a very basic MFD driver with the fan control and User
NVMEM cells.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
drivers/mfd/Kconfig | 14 ++++
drivers/mfd/Makefile | 1 +
drivers/mfd/khadas-mcu.c | 143 +++++++++++++++++++++++++++++++++
include/linux/mfd/khadas-mcu.h | 91 +++++++++++++++++++++
4 files changed, 249 insertions(+)
create mode 100644 drivers/mfd/khadas-mcu.c
create mode 100644 include/linux/mfd/khadas-mcu.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 0a59249198d3..b95091397052 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2003,6 +2003,20 @@ config MFD_WCD934X
This driver provides common support WCD934x audio codec and its
associated Pin Controller, Soundwire Controller and Audio codec.
+config MFD_KHADAS_MCU
+ tristate "Support for Khadas System control Microcontroller"
+ depends on I2C
+ depends on OF || COMPILE_TEST
+ select MFD_CORE
+ select REGMAP_I2C
+ help
+ Support for the Khadas System control Microcontroller interface present
+ on their VIM and Edge boards.
+
+ This driver provides common support for accessing the device,
+ additional drivers must be enabled in order to use the functionality
+ of the device.
+
menu "Multimedia Capabilities Port drivers"
depends on ARCH_SA1100
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f935d10cbf0f..0f1633b096bb 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -257,5 +257,6 @@ obj-$(CONFIG_MFD_ROHM_BD70528) += rohm-bd70528.o
obj-$(CONFIG_MFD_ROHM_BD71828) += rohm-bd71828.o
obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
obj-$(CONFIG_MFD_STMFX) += stmfx.o
+obj-$(CONFIG_MFD_KHADAS_MCU) += khadas-mcu.o
obj-$(CONFIG_SGI_MFD_IOC3) += ioc3.o
diff --git a/drivers/mfd/khadas-mcu.c b/drivers/mfd/khadas-mcu.c
new file mode 100644
index 000000000000..6d08fa2e373a
--- /dev/null
+++ b/drivers/mfd/khadas-mcu.c
@@ -0,0 +1,143 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Khadas System control Microcontroller
+ *
+ * Copyright (C) 2020 BayLibre SAS
+ * Author(s): Neil Armstrong <narmstrong@baylibre.com>
+ */
+#include <linux/bitfield.h>
+#include <linux/i2c.h>
+#include <linux/mfd/khadas-mcu.h>
+#include <linux/regmap.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+
+static bool khadas_mcu_reg_volatile(struct device *dev, unsigned int reg)
+{
+ if (reg >= KHADAS_MCU_USER_DATA_0_REG &&
+ reg < KHADAS_MCU_PWR_OFF_CMD_REG)
+ return true;
+
+ switch (reg) {
+ case KHADAS_MCU_PWR_OFF_CMD_REG:
+ case KHADAS_MCU_PASSWD_START_REG:
+ case KHADAS_MCU_CHECK_VEN_PASSWD_REG:
+ case KHADAS_MCU_CHECK_USER_PASSWD_REG:
+ case KHADAS_MCU_WOL_INIT_START_REG:
+ case KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool khadas_mcu_reg_writeable(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case KHADAS_MCU_PASSWD_VEN_0_REG:
+ case KHADAS_MCU_PASSWD_VEN_1_REG:
+ case KHADAS_MCU_PASSWD_VEN_2_REG:
+ case KHADAS_MCU_PASSWD_VEN_3_REG:
+ case KHADAS_MCU_PASSWD_VEN_4_REG:
+ case KHADAS_MCU_PASSWD_VEN_5_REG:
+ case KHADAS_MCU_MAC_0_REG:
+ case KHADAS_MCU_MAC_1_REG:
+ case KHADAS_MCU_MAC_2_REG:
+ case KHADAS_MCU_MAC_3_REG:
+ case KHADAS_MCU_MAC_4_REG:
+ case KHADAS_MCU_MAC_5_REG:
+ case KHADAS_MCU_USID_0_REG:
+ case KHADAS_MCU_USID_1_REG:
+ case KHADAS_MCU_USID_2_REG:
+ case KHADAS_MCU_USID_3_REG:
+ case KHADAS_MCU_USID_4_REG:
+ case KHADAS_MCU_USID_5_REG:
+ case KHADAS_MCU_VERSION_0_REG:
+ case KHADAS_MCU_VERSION_1_REG:
+ case KHADAS_MCU_DEVICE_NO_0_REG:
+ case KHADAS_MCU_DEVICE_NO_1_REG:
+ case KHADAS_MCU_FACTORY_TEST_REG:
+ case KHADAS_MCU_SHUTDOWN_NORMAL_STATUS_REG:
+ return false;
+ default:
+ return true;
+ }
+}
+
+static const struct regmap_config khadas_mcu_regmap_config = {
+ .reg_bits = 8,
+ .reg_stride = 1,
+ .val_bits = 8,
+ .max_register = KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG,
+ .volatile_reg = khadas_mcu_reg_volatile,
+ .writeable_reg = khadas_mcu_reg_writeable,
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static struct mfd_cell khadas_mcu_fan_cells[] = {
+ /* Feature supported only on VIM1/2 Rev13+ and VIM3 */
+ { .name = "khadas-mcu-fan-ctrl", },
+};
+
+static struct mfd_cell khadas_mcu_cells[] = {
+ /* Features supported on all board revisions */
+ { .name = "khadas-mcu-user-mem", },
+};
+
+static int khadas_mcu_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ struct khadas_mcu *khadas_mcu;
+ int ret;
+
+ khadas_mcu = devm_kzalloc(dev, sizeof(*khadas_mcu), GFP_KERNEL);
+ if (!khadas_mcu)
+ return -ENOMEM;
+
+ i2c_set_clientdata(client, khadas_mcu);
+
+ khadas_mcu->dev = dev;
+
+ khadas_mcu->map = devm_regmap_init_i2c(client,
+ &khadas_mcu_regmap_config);
+ if (IS_ERR(khadas_mcu->map)) {
+ ret = PTR_ERR(khadas_mcu->map);
+ dev_err(dev, "Failed to allocate register map: %d\n", ret);
+ return ret;
+ }
+
+ ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
+ khadas_mcu_cells,
+ ARRAY_SIZE(khadas_mcu_cells),
+ NULL, 0, NULL);
+ if (ret)
+ return ret;
+
+ if (of_find_property(dev->of_node, "#cooling-cells", NULL))
+ return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
+ khadas_mcu_fan_cells,
+ ARRAY_SIZE(khadas_mcu_fan_cells),
+ NULL, 0, NULL);
+
+ return 0;
+}
+
+static const struct of_device_id khadas_mcu_of_match[] = {
+ { .compatible = "khadas,mcu", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, khadas_mcu_of_match);
+
+static struct i2c_driver khadas_mcu_driver = {
+ .driver = {
+ .name = "khadas-mcu-core",
+ .of_match_table = of_match_ptr(khadas_mcu_of_match),
+ },
+ .probe = khadas_mcu_probe,
+};
+module_i2c_driver(khadas_mcu_driver);
+
+MODULE_DESCRIPTION("Khadas MCU core driver");
+MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/khadas-mcu.h b/include/linux/mfd/khadas-mcu.h
new file mode 100644
index 000000000000..2e68af21735c
--- /dev/null
+++ b/include/linux/mfd/khadas-mcu.h
@@ -0,0 +1,91 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Khadas System control Microcontroller Register map
+ *
+ * Copyright (C) 2020 BayLibre SAS
+ * Author(s): Neil Armstrong <narmstrong@baylibre.com>
+ */
+
+#ifndef MFD_KHADAS_MCU_H
+#define MFD_KHADAS_MCU_H
+
+#define KHADAS_MCU_PASSWD_VEN_0_REG 0x0 /* RO */
+#define KHADAS_MCU_PASSWD_VEN_1_REG 0x1 /* RO */
+#define KHADAS_MCU_PASSWD_VEN_2_REG 0x2 /* RO */
+#define KHADAS_MCU_PASSWD_VEN_3_REG 0x3 /* RO */
+#define KHADAS_MCU_PASSWD_VEN_4_REG 0x4 /* RO */
+#define KHADAS_MCU_PASSWD_VEN_5_REG 0x5 /* RO */
+#define KHADAS_MCU_MAC_0_REG 0x6 /* RO */
+#define KHADAS_MCU_MAC_1_REG 0x7 /* RO */
+#define KHADAS_MCU_MAC_2_REG 0x8 /* RO */
+#define KHADAS_MCU_MAC_3_REG 0x9 /* RO */
+#define KHADAS_MCU_MAC_4_REG 0xa /* RO */
+#define KHADAS_MCU_MAC_5_REG 0xb /* RO */
+#define KHADAS_MCU_USID_0_REG 0xc /* RO */
+#define KHADAS_MCU_USID_1_REG 0xd /* RO */
+#define KHADAS_MCU_USID_2_REG 0xe /* RO */
+#define KHADAS_MCU_USID_3_REG 0xf /* RO */
+#define KHADAS_MCU_USID_4_REG 0x10 /* RO */
+#define KHADAS_MCU_USID_5_REG 0x11 /* RO */
+#define KHADAS_MCU_VERSION_0_REG 0x12 /* RO */
+#define KHADAS_MCU_VERSION_1_REG 0x13 /* RO */
+#define KHADAS_MCU_DEVICE_NO_0_REG 0x14 /* RO */
+#define KHADAS_MCU_DEVICE_NO_1_REG 0x15 /* RO */
+#define KHADAS_MCU_FACTORY_TEST_REG 0x16 /* R */
+#define KHADAS_MCU_BOOT_MODE_REG 0x20 /* RW */
+#define KHADAS_MCU_BOOT_EN_WOL_REG 0x21 /* RW */
+#define KHADAS_MCU_BOOT_EN_RTC_REG 0x22 /* RW */
+#define KHADAS_MCU_BOOT_EN_EXP_REG 0x23 /* RW */
+#define KHADAS_MCU_BOOT_EN_IR_REG 0x24 /* RW */
+#define KHADAS_MCU_BOOT_EN_DCIN_REG 0x25 /* RW */
+#define KHADAS_MCU_BOOT_EN_KEY_REG 0x26 /* RW */
+#define KHADAS_MCU_KEY_MODE_REG 0x27 /* RW */
+#define KHADAS_MCU_LED_MODE_ON_REG 0x28 /* RW */
+#define KHADAS_MCU_LED_MODE_OFF_REG 0x29 /* RW */
+#define KHADAS_MCU_SHUTDOWN_NORMAL_REG 0x2c /* RW */
+#define KHADAS_MCU_MAC_SWITCH_REG 0x2d /* RW */
+#define KHADAS_MCU_MCU_SLEEP_MODE_REG 0x2e /* RW */
+#define KHADAS_MCU_IR_CODE1_0_REG 0x2f /* RW */
+#define KHADAS_MCU_IR_CODE1_1_REG 0x30 /* RW */
+#define KHADAS_MCU_IR_CODE1_2_REG 0x31 /* RW */
+#define KHADAS_MCU_IR_CODE1_3_REG 0x32 /* RW */
+#define KHADAS_MCU_USB_PCIE_SWITCH_REG 0x33 /* RW */
+#define KHADAS_MCU_IR_CODE2_0_REG 0x34 /* RW */
+#define KHADAS_MCU_IR_CODE2_1_REG 0x35 /* RW */
+#define KHADAS_MCU_IR_CODE2_2_REG 0x36 /* RW */
+#define KHADAS_MCU_IR_CODE2_3_REG 0x37 /* RW */
+#define KHADAS_MCU_PASSWD_USER_0_REG 0x40 /* RW */
+#define KHADAS_MCU_PASSWD_USER_1_REG 0x41 /* RW */
+#define KHADAS_MCU_PASSWD_USER_2_REG 0x42 /* RW */
+#define KHADAS_MCU_PASSWD_USER_3_REG 0x43 /* RW */
+#define KHADAS_MCU_PASSWD_USER_4_REG 0x44 /* RW */
+#define KHADAS_MCU_PASSWD_USER_5_REG 0x45 /* RW */
+#define KHADAS_MCU_USER_DATA_0_REG 0x46 /* RW 56 bytes */
+#define KHADAS_MCU_PWR_OFF_CMD_REG 0x80 /* WO */
+#define KHADAS_MCU_PASSWD_START_REG 0x81 /* WO */
+#define KHADAS_MCU_CHECK_VEN_PASSWD_REG 0x82 /* WO */
+#define KHADAS_MCU_CHECK_USER_PASSWD_REG 0x83 /* WO */
+#define KHADAS_MCU_SHUTDOWN_NORMAL_STATUS_REG 0x86 /* RO */
+#define KHADAS_MCU_WOL_INIT_START_REG 0x87 /* WO */
+#define KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG 0x88 /* WO */
+
+/* Boards */
+enum {
+ KHADAS_BOARD_VIM1 = 0x1,
+ KHADAS_BOARD_VIM2,
+ KHADAS_BOARD_VIM3,
+ KHADAS_BOARD_EDGE = 0x11,
+ KHADAS_BOARD_EDGE_V,
+};
+
+/**
+ * struct khadas_mcu_data - Khadas MCU MFD structure
+ * @device: device reference used for logs
+ * @map: register map
+ */
+struct khadas_mcu {
+ struct device *dev;
+ struct regmap *map;
+};
+
+#endif /* MFD_KHADAS_MCU_H */
--
2.22.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 3/6] thermal: add support for the MCU controlled FAN on Khadas boards
2020-05-12 13:26 [PATCH v2 0/6] mfd: Add support for Khadas Microcontroller Neil Armstrong
2020-05-12 13:26 ` [PATCH v2 1/6] dt-bindings: mfd: add Khadas Microcontroller bindings Neil Armstrong
2020-05-12 13:26 ` [PATCH v2 2/6] mfd: add support for the Khadas System control Microcontroller Neil Armstrong
@ 2020-05-12 13:26 ` Neil Armstrong
2020-05-15 6:41 ` Amit Kucheria
2020-05-12 13:26 ` [PATCH v2 4/6] nvmem: add support for the Khadas MCU Programmable User Memory Neil Armstrong
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Neil Armstrong @ 2020-05-12 13:26 UTC (permalink / raw)
To: srinivas.kandagatla, rui.zhang, daniel.lezcano, amit.kucheria
Cc: Neil Armstrong, linux-amlogic, linux-pm, linux-arm-kernel, linux-kernel
The new Khadas VIM2 and VIM3 boards controls the cooling fan via the
on-board microcontroller.
This implements the FAN control as thermal devices and as cell of the Khadas
MCU MFD driver.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
drivers/thermal/Kconfig | 10 ++
drivers/thermal/Makefile | 1 +
drivers/thermal/khadas_mcu_fan.c | 174 +++++++++++++++++++++++++++++++
3 files changed, 185 insertions(+)
create mode 100644 drivers/thermal/khadas_mcu_fan.c
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 91af271e9bb0..72b3960cc5ac 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -490,4 +490,14 @@ config SPRD_THERMAL
help
Support for the Spreadtrum thermal sensor driver in the Linux thermal
framework.
+
+config KHADAS_MCU_FAN_THERMAL
+ tristate "Khadas MCU controller FAN cooling support"
+ depends on OF || COMPILE_TEST
+ select MFD_CORE
+ select REGMAP
+ help
+ If you say yes here you get support for the FAN controlled
+ by the Microcontroller found on the Khadas VIM boards.
+
endif
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 8c8ed7b79915..460428c2122c 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -60,3 +60,4 @@ obj-$(CONFIG_ZX2967_THERMAL) += zx2967_thermal.o
obj-$(CONFIG_UNIPHIER_THERMAL) += uniphier_thermal.o
obj-$(CONFIG_AMLOGIC_THERMAL) += amlogic_thermal.o
obj-$(CONFIG_SPRD_THERMAL) += sprd_thermal.o
+obj-$(CONFIG_KHADAS_MCU_FAN_THERMAL) += khadas_mcu_fan.o
diff --git a/drivers/thermal/khadas_mcu_fan.c b/drivers/thermal/khadas_mcu_fan.c
new file mode 100644
index 000000000000..044d4aba8be2
--- /dev/null
+++ b/drivers/thermal/khadas_mcu_fan.c
@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Khadas MCU Controlled FAN driver
+ *
+ * Copyright (C) 2020 BayLibre SAS
+ * Author(s): Neil Armstrong <narmstrong@baylibre.com>
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/khadas-mcu.h>
+#include <linux/regmap.h>
+#include <linux/sysfs.h>
+#include <linux/thermal.h>
+
+#define MAX_LEVEL 3
+
+struct khadas_mcu_fan_ctx {
+ struct khadas_mcu *mcu;
+ unsigned int level;
+ struct thermal_cooling_device *cdev;
+};
+
+static int khadas_mcu_fan_set_level(struct khadas_mcu_fan_ctx *ctx,
+ unsigned int level)
+{
+ int ret;
+
+ ret = regmap_write(ctx->mcu->map, KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG,
+ level);
+ if (ret)
+ return ret;
+
+ ctx->level = level;
+
+ return 0;
+}
+
+static int khadas_mcu_fan_get_max_state(struct thermal_cooling_device *cdev,
+ unsigned long *state)
+{
+ struct khadas_mcu_fan_ctx *ctx = cdev->devdata;
+
+ if (!ctx)
+ return -EINVAL;
+
+ *state = MAX_LEVEL;
+
+ return 0;
+}
+
+static int khadas_mcu_fan_get_cur_state(struct thermal_cooling_device *cdev,
+ unsigned long *state)
+{
+ struct khadas_mcu_fan_ctx *ctx = cdev->devdata;
+
+ if (!ctx)
+ return -EINVAL;
+
+ *state = ctx->level;
+
+ return 0;
+}
+
+static int
+khadas_mcu_fan_set_cur_state(struct thermal_cooling_device *cdev,
+ unsigned long state)
+{
+ struct khadas_mcu_fan_ctx *ctx = cdev->devdata;
+
+ if (!ctx || (state > MAX_LEVEL))
+ return -EINVAL;
+
+ if (state == ctx->level)
+ return 0;
+
+ return khadas_mcu_fan_set_level(ctx, state);
+}
+
+static const struct thermal_cooling_device_ops khadas_mcu_fan_cooling_ops = {
+ .get_max_state = khadas_mcu_fan_get_max_state,
+ .get_cur_state = khadas_mcu_fan_get_cur_state,
+ .set_cur_state = khadas_mcu_fan_set_cur_state,
+};
+
+static int khadas_mcu_fan_probe(struct platform_device *pdev)
+{
+ struct khadas_mcu *mcu = dev_get_drvdata(pdev->dev.parent);
+ struct thermal_cooling_device *cdev;
+ struct device *dev = &pdev->dev;
+ struct khadas_mcu_fan_ctx *ctx;
+ int ret;
+
+ ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+ ctx->mcu = mcu;
+ platform_set_drvdata(pdev, ctx);
+
+ cdev = devm_thermal_of_cooling_device_register(dev->parent,
+ dev->parent->of_node, "khadas-mcu-fan", ctx,
+ &khadas_mcu_fan_cooling_ops);
+ if (IS_ERR(cdev)) {
+ ret = PTR_ERR(cdev);
+ dev_err(dev,
+ "Failed to register khadas-mcu-fan as cooling device: %d\n",
+ ret);
+ return ret;
+ }
+ ctx->cdev = cdev;
+ thermal_cdev_update(cdev);
+
+ return 0;
+}
+
+static int khadas_mcu_fan_disable(struct device *dev)
+{
+ struct khadas_mcu_fan_ctx *ctx = dev_get_drvdata(dev);
+ unsigned int level_save = ctx->level;
+ int ret;
+
+ ret = khadas_mcu_fan_set_level(ctx, 0);
+ if (ret)
+ return ret;
+
+ ctx->level = level_save;
+
+ return 0;
+}
+
+static void khadas_mcu_fan_shutdown(struct platform_device *pdev)
+{
+ khadas_mcu_fan_disable(&pdev->dev);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int khadas_mcu_fan_suspend(struct device *dev)
+{
+ return khadas_mcu_fan_disable(dev);
+}
+
+static int khadas_mcu_fan_resume(struct device *dev)
+{
+ struct khadas_mcu_fan_ctx *ctx = dev_get_drvdata(dev);
+
+ return khadas_mcu_fan_set_level(ctx, ctx->level);
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(khadas_mcu_fan_pm, khadas_mcu_fan_suspend,
+ khadas_mcu_fan_resume);
+
+static const struct platform_device_id khadas_mcu_fan_id_table[] = {
+ { .name = "khadas-mcu-fan-ctrl", },
+ {},
+};
+MODULE_DEVICE_TABLE(platform, khadas_mcu_fan_id_table);
+
+static struct platform_driver khadas_mcu_fan_driver = {
+ .probe = khadas_mcu_fan_probe,
+ .shutdown = khadas_mcu_fan_shutdown,
+ .driver = {
+ .name = "khadas-mcu-fan-ctrl",
+ .pm = &khadas_mcu_fan_pm,
+ },
+ .id_table = khadas_mcu_fan_id_table,
+};
+
+module_platform_driver(khadas_mcu_fan_driver);
+
+MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
+MODULE_DESCRIPTION("Khadas MCU FAN driver");
+MODULE_LICENSE("GPL");
--
2.22.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 4/6] nvmem: add support for the Khadas MCU Programmable User Memory
2020-05-12 13:26 [PATCH v2 0/6] mfd: Add support for Khadas Microcontroller Neil Armstrong
` (2 preceding siblings ...)
2020-05-12 13:26 ` [PATCH v2 3/6] thermal: add support for the MCU controlled FAN on Khadas boards Neil Armstrong
@ 2020-05-12 13:26 ` Neil Armstrong
2020-05-13 10:34 ` Srinivas Kandagatla
2020-05-12 13:26 ` [PATCH v2 5/6] MAINTAINERS: add myself as maintainer for Khadas MCU drivers Neil Armstrong
2020-05-12 13:26 ` [PATCH v2 6/6] arm64: dts: meson-khadas-vim3: add Khadas MCU nodes Neil Armstrong
5 siblings, 1 reply; 18+ messages in thread
From: Neil Armstrong @ 2020-05-12 13:26 UTC (permalink / raw)
To: srinivas.kandagatla
Cc: Neil Armstrong, linux-amlogic, linux-pm, linux-arm-kernel, linux-kernel
The new Khadas VIM2, VIM3 and Edge boards embeds an on-board microcontroller
offering a 56bytes User Programmable NVMEM array.
This array needs a password to be writable, thus a password sysfs file
has been added on the device node to unlock the NVMEM.
The default 6bytes password id: "Khadas"
This implements the user NVMEM devices as cell of the Khadas MCU MFD driver.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
drivers/nvmem/Kconfig | 8 ++
drivers/nvmem/Makefile | 2 +
drivers/nvmem/khadas-mcu-user-mem.c | 128 ++++++++++++++++++++++++++++
3 files changed, 138 insertions(+)
create mode 100644 drivers/nvmem/khadas-mcu-user-mem.c
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index d7b7f6d688e7..92cd4f6aa931 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -67,6 +67,14 @@ config JZ4780_EFUSE
To compile this driver as a module, choose M here: the module
will be called nvmem_jz4780_efuse.
+config NVMEM_KHADAS_MCU_USER_MEM
+ tristate "Khadas MCU User programmable memory support"
+ depends on MFD_KHADAS_MCU
+ depends on REGMAP
+ help
+ This is a driver for the MCU User programmable memory
+ available on the Khadas VIM and Edge boards.
+
config NVMEM_LPC18XX_EEPROM
tristate "NXP LPC18XX EEPROM Memory Support"
depends on ARCH_LPC18XX || COMPILE_TEST
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index a7c377218341..0516a309542d 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -17,6 +17,8 @@ obj-$(CONFIG_NVMEM_IMX_OCOTP_SCU) += nvmem-imx-ocotp-scu.o
nvmem-imx-ocotp-scu-y := imx-ocotp-scu.o
obj-$(CONFIG_JZ4780_EFUSE) += nvmem_jz4780_efuse.o
nvmem_jz4780_efuse-y := jz4780-efuse.o
+obj-$(CONFIG_NVMEM_KHADAS_MCU_USER_MEM) += nvmem-khadas-mcu-user-mem.o
+nvmem-khadas-mcu-user-mem-y := khadas-mcu-user-mem.o
obj-$(CONFIG_NVMEM_LPC18XX_EEPROM) += nvmem_lpc18xx_eeprom.o
nvmem_lpc18xx_eeprom-y := lpc18xx_eeprom.o
obj-$(CONFIG_NVMEM_LPC18XX_OTP) += nvmem_lpc18xx_otp.o
diff --git a/drivers/nvmem/khadas-mcu-user-mem.c b/drivers/nvmem/khadas-mcu-user-mem.c
new file mode 100644
index 000000000000..a1d5ae9a030c
--- /dev/null
+++ b/drivers/nvmem/khadas-mcu-user-mem.c
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Khadas MCU User programmable Memory
+ *
+ * Copyright (C) 2020 BayLibre SAS
+ * Author(s): Neil Armstrong <narmstrong@baylibre.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/nvmem-provider.h>
+#include <linux/mfd/khadas-mcu.h>
+#include <linux/regmap.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+static int khadas_mcu_user_mem_read(void *context, unsigned int offset,
+ void *val, size_t bytes)
+{
+ struct khadas_mcu *khadas_mcu = context;
+
+ return regmap_bulk_read(khadas_mcu->map,
+ KHADAS_MCU_USER_DATA_0_REG + offset,
+ val, bytes);
+}
+
+static int khadas_mcu_user_mem_write(void *context, unsigned int offset,
+ void *val, size_t bytes)
+{
+ struct khadas_mcu *khadas_mcu = context;
+
+ return regmap_bulk_write(khadas_mcu->map,
+ KHADAS_MCU_USER_DATA_0_REG + offset,
+ val, bytes);
+}
+
+static ssize_t password_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct khadas_mcu *khadas_mcu = dev_get_drvdata(dev);
+ int i, ret;
+
+ if (count < 6)
+ return -EINVAL;
+
+ ret = regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 1);
+ if (ret)
+ return ret;
+
+ for (i = 0 ; i < 6 ; ++i) {
+ ret = regmap_write(khadas_mcu->map,
+ KHADAS_MCU_CHECK_USER_PASSWD_REG,
+ buf[i]);
+ if (ret)
+ goto out;
+ }
+
+ ret = regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 0);
+ if (ret)
+ return ret;
+
+ return count;
+out:
+ regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 0);
+
+ return ret;
+}
+
+static DEVICE_ATTR_WO(password);
+
+static struct attribute *khadas_mcu_user_mem_sysfs_attributes[] = {
+ &dev_attr_password.attr,
+ NULL,
+};
+
+static const struct attribute_group khadas_mcu_user_mem_sysfs_attr_group = {
+ .attrs = khadas_mcu_user_mem_sysfs_attributes,
+};
+
+static int khadas_mcu_user_mem_probe(struct platform_device *pdev)
+{
+ struct khadas_mcu *khadas_mcu = dev_get_drvdata(pdev->dev.parent);
+ struct device *dev = &pdev->dev;
+ struct nvmem_device *nvmem;
+ struct nvmem_config *econfig;
+
+ econfig = devm_kzalloc(dev, sizeof(*econfig), GFP_KERNEL);
+ if (!econfig)
+ return -ENOMEM;
+
+ econfig->dev = pdev->dev.parent;
+ econfig->name = dev_name(pdev->dev.parent);
+ econfig->stride = 1;
+ econfig->word_size = 1;
+ econfig->reg_read = khadas_mcu_user_mem_read;
+ econfig->reg_write = khadas_mcu_user_mem_write;
+ econfig->size = 56;
+ econfig->priv = khadas_mcu;
+
+ platform_set_drvdata(pdev, khadas_mcu);
+
+ nvmem = devm_nvmem_register(&pdev->dev, econfig);
+ if (IS_ERR(nvmem))
+ return PTR_ERR(nvmem);
+
+ return sysfs_create_group(&pdev->dev.kobj,
+ &khadas_mcu_user_mem_sysfs_attr_group);
+}
+
+static const struct platform_device_id khadas_mcu_user_mem_id_table[] = {
+ { .name = "khadas-mcu-user-mem", },
+ {},
+};
+MODULE_DEVICE_TABLE(platform, khadas_mcu_user_mem_id_table);
+
+static struct platform_driver khadas_mcu_user_mem_driver = {
+ .probe = khadas_mcu_user_mem_probe,
+ .driver = {
+ .name = "khadas-mcu-user-mem",
+ },
+ .id_table = khadas_mcu_user_mem_id_table,
+};
+
+module_platform_driver(khadas_mcu_user_mem_driver);
+
+MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
+MODULE_DESCRIPTION("Khadas MCU User MEM driver");
+MODULE_LICENSE("GPL v2");
--
2.22.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 5/6] MAINTAINERS: add myself as maintainer for Khadas MCU drivers
2020-05-12 13:26 [PATCH v2 0/6] mfd: Add support for Khadas Microcontroller Neil Armstrong
` (3 preceding siblings ...)
2020-05-12 13:26 ` [PATCH v2 4/6] nvmem: add support for the Khadas MCU Programmable User Memory Neil Armstrong
@ 2020-05-12 13:26 ` Neil Armstrong
2020-05-12 13:26 ` [PATCH v2 6/6] arm64: dts: meson-khadas-vim3: add Khadas MCU nodes Neil Armstrong
5 siblings, 0 replies; 18+ messages in thread
From: Neil Armstrong @ 2020-05-12 13:26 UTC (permalink / raw)
To: lee.jones, srinivas.kandagatla, rui.zhang, daniel.lezcano, amit.kucheria
Cc: Neil Armstrong, linux-amlogic, linux-pm, linux-arm-kernel, linux-kernel
Add the HWMON and NVMEM drivers along the MFD drivers and header
as Maintained by myself.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
MAINTAINERS | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index b816a453b10e..609baa78d810 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9431,6 +9431,16 @@ F: include/linux/kdb.h
F: include/linux/kgdb.h
F: kernel/debug/
+KHADAS MCU MFD DRIVER
+M: Neil Armstrong <narmstrong@baylibre.com>
+L: linux-amlogic@lists.infradead.org
+S: Maintained
+F: Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
+F: drivers/mfd/khadas-mcu.c
+F: include/linux/mfd/khadas-mcu.h
+F: drivers/hwmon/khadas-mcu-fan.c
+F: drivers/nvmem/khadas-mcu-user-mem.c
+
KMEMLEAK
M: Catalin Marinas <catalin.marinas@arm.com>
S: Maintained
--
2.22.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 6/6] arm64: dts: meson-khadas-vim3: add Khadas MCU nodes
2020-05-12 13:26 [PATCH v2 0/6] mfd: Add support for Khadas Microcontroller Neil Armstrong
` (4 preceding siblings ...)
2020-05-12 13:26 ` [PATCH v2 5/6] MAINTAINERS: add myself as maintainer for Khadas MCU drivers Neil Armstrong
@ 2020-05-12 13:26 ` Neil Armstrong
5 siblings, 0 replies; 18+ messages in thread
From: Neil Armstrong @ 2020-05-12 13:26 UTC (permalink / raw)
To: khilman
Cc: Neil Armstrong, linux-amlogic, linux-pm, linux-arm-kernel, linux-kernel
Add the Khadas MCU node with active FAN thermal nodes for all the
Khadas VIM3 variants.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
.../boot/dts/amlogic/meson-khadas-vim3.dtsi | 23 +++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi b/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
index 094ecf2222bb..3325e54ea690 100644
--- a/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
@@ -183,6 +183,23 @@
hdmi-phandle = <&hdmi_tx>;
};
+&cpu_thermal {
+ trips {
+ cpu_active: cpu-active {
+ temperature = <80000>; /* millicelsius */
+ hysteresis = <2000>; /* millicelsius */
+ type = "active";
+ };
+ };
+
+ cooling-maps {
+ map {
+ trip = <&cpu_active>;
+ cooling-device = <&khadas_mcu THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+};
+
&ext_mdio {
external_phy: ethernet-phy@0 {
/* Realtek RTL8211F (0x001cc916) */
@@ -222,6 +239,12 @@
pinctrl-0 = <&i2c_ao_sck_pins>, <&i2c_ao_sda_pins>;
pinctrl-names = "default";
+ khadas_mcu: system-controller@18 {
+ compatible = "khadas,mcu";
+ reg = <0x18>;
+ #cooling-cells = <2>;
+ };
+
gpio_expander: gpio-controller@20 {
compatible = "ti,tca6408";
reg = <0x20>;
--
2.22.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/6] nvmem: add support for the Khadas MCU Programmable User Memory
2020-05-12 13:26 ` [PATCH v2 4/6] nvmem: add support for the Khadas MCU Programmable User Memory Neil Armstrong
@ 2020-05-13 10:34 ` Srinivas Kandagatla
2020-05-13 12:33 ` Neil Armstrong
0 siblings, 1 reply; 18+ messages in thread
From: Srinivas Kandagatla @ 2020-05-13 10:34 UTC (permalink / raw)
To: Neil Armstrong; +Cc: linux-amlogic, linux-pm, linux-arm-kernel, linux-kernel
On 12/05/2020 14:26, Neil Armstrong wrote:
> The new Khadas VIM2, VIM3 and Edge boards embeds an on-board microcontroller
> offering a 56bytes User Programmable NVMEM array.
>
> This array needs a password to be writable, thus a password sysfs file
> has been added on the device node to unlock the NVMEM.
Is this one time programmable? Or does it need to be unlocked for every
read/write?
How can you make sure that the memory is unlocked before making attempt
to read/write?
>
> The default 6bytes password id: "Khadas"
>
> This implements the user NVMEM devices as cell of the Khadas MCU MFD driver.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> drivers/nvmem/Kconfig | 8 ++
> drivers/nvmem/Makefile | 2 +
> drivers/nvmem/khadas-mcu-user-mem.c | 128 ++++++++++++++++++++++++++++
> 3 files changed, 138 insertions(+)
> create mode 100644 drivers/nvmem/khadas-mcu-user-mem.c
>
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index d7b7f6d688e7..92cd4f6aa931 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -67,6 +67,14 @@ config JZ4780_EFUSE
> To compile this driver as a module, choose M here: the module
> will be called nvmem_jz4780_efuse.
>
> +config NVMEM_KHADAS_MCU_USER_MEM
> + tristate "Khadas MCU User programmable memory support"
> + depends on MFD_KHADAS_MCU
> + depends on REGMAP
> + help
> + This is a driver for the MCU User programmable memory
> + available on the Khadas VIM and Edge boards.
> +
> config NVMEM_LPC18XX_EEPROM
> tristate "NXP LPC18XX EEPROM Memory Support"
> depends on ARCH_LPC18XX || COMPILE_TEST
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index a7c377218341..0516a309542d 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -17,6 +17,8 @@ obj-$(CONFIG_NVMEM_IMX_OCOTP_SCU) += nvmem-imx-ocotp-scu.o
> nvmem-imx-ocotp-scu-y := imx-ocotp-scu.o
> obj-$(CONFIG_JZ4780_EFUSE) += nvmem_jz4780_efuse.o
> nvmem_jz4780_efuse-y := jz4780-efuse.o
> +obj-$(CONFIG_NVMEM_KHADAS_MCU_USER_MEM) += nvmem-khadas-mcu-user-mem.o
> +nvmem-khadas-mcu-user-mem-y := khadas-mcu-user-mem.o
> obj-$(CONFIG_NVMEM_LPC18XX_EEPROM) += nvmem_lpc18xx_eeprom.o
> nvmem_lpc18xx_eeprom-y := lpc18xx_eeprom.o
> obj-$(CONFIG_NVMEM_LPC18XX_OTP) += nvmem_lpc18xx_otp.o
> diff --git a/drivers/nvmem/khadas-mcu-user-mem.c b/drivers/nvmem/khadas-mcu-user-mem.c
> new file mode 100644
> index 000000000000..a1d5ae9a030c
> --- /dev/null
> +++ b/drivers/nvmem/khadas-mcu-user-mem.c
> @@ -0,0 +1,128 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Khadas MCU User programmable Memory
> + *
> + * Copyright (C) 2020 BayLibre SAS
> + * Author(s): Neil Armstrong <narmstrong@baylibre.com>
> + */
> +
> +#include <linux/clk.h>
Why do we need this header?
> +#include <linux/module.h>
> +#include <linux/nvmem-provider.h>
> +#include <linux/mfd/khadas-mcu.h>
> +#include <linux/regmap.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +static int khadas_mcu_user_mem_read(void *context, unsigned int offset,
> + void *val, size_t bytes)
> +{
> + struct khadas_mcu *khadas_mcu = context;
> +
> + return regmap_bulk_read(khadas_mcu->map,
> + KHADAS_MCU_USER_DATA_0_REG + offset,
> + val, bytes);
> +}
> +
> +static int khadas_mcu_user_mem_write(void *context, unsigned int offset,
> + void *val, size_t bytes)
> +{
> + struct khadas_mcu *khadas_mcu = context;
> +
> + return regmap_bulk_write(khadas_mcu->map,
> + KHADAS_MCU_USER_DATA_0_REG + offset,
> + val, bytes);
> +}
> +
> +static ssize_t password_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct khadas_mcu *khadas_mcu = dev_get_drvdata(dev);
> + int i, ret;
> +
> + if (count < 6)
> + return -EINVAL;
Possibly this magic 6 value needs proper define. An additional check
here for > 6 would be better as well.
> +
> + ret = regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 1);
> + if (ret)
> + return ret;
> +
> + for (i = 0 ; i < 6 ; ++i) {
> + ret = regmap_write(khadas_mcu->map,
> + KHADAS_MCU_CHECK_USER_PASSWD_REG,
> + buf[i]);
> + if (ret)
> + goto out;
> + }
> +
> + ret = regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 0);
> + if (ret)
> + return ret;
> +
> + return count;
> +out:
> + regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 0);
> +
> + return ret;
> +}
> +
> +static DEVICE_ATTR_WO(password);
> +
> +static struct attribute *khadas_mcu_user_mem_sysfs_attributes[] = {
> + &dev_attr_password.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group khadas_mcu_user_mem_sysfs_attr_group = {
> + .attrs = khadas_mcu_user_mem_sysfs_attributes,
> +};
> +
> +static int khadas_mcu_user_mem_probe(struct platform_device *pdev)
> +{
> + struct khadas_mcu *khadas_mcu = dev_get_drvdata(pdev->dev.parent);
You could probably get away with dependency of this structure and the
header itself by directly getting hold of regmap from parent device.
> + struct device *dev = &pdev->dev;
> + struct nvmem_device *nvmem;
> + struct nvmem_config *econfig;
> +
> + econfig = devm_kzalloc(dev, sizeof(*econfig), GFP_KERNEL);
> + if (!econfig)
> + return -ENOMEM;
> +
> + econfig->dev = pdev->dev.parent;
> + econfig->name = dev_name(pdev->dev.parent);
> + econfig->stride = 1;
> + econfig->word_size = 1;
> + econfig->reg_read = khadas_mcu_user_mem_read;
> + econfig->reg_write = khadas_mcu_user_mem_write;
> + econfig->size = 56;
define 56 to make it more readable!
> + econfig->priv = khadas_mcu;
> +
> + platform_set_drvdata(pdev, khadas_mcu);
> +
> + nvmem = devm_nvmem_register(&pdev->dev, econfig);
> + if (IS_ERR(nvmem))
> + return PTR_ERR(nvmem);
> +
> + return sysfs_create_group(&pdev->dev.kobj,
> + &khadas_mcu_user_mem_sysfs_attr_group);
> +}
> +
> +static const struct platform_device_id khadas_mcu_user_mem_id_table[] = {
> + { .name = "khadas-mcu-user-mem", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(platform, khadas_mcu_user_mem_id_table);
> +
> +static struct platform_driver khadas_mcu_user_mem_driver = {
> + .probe = khadas_mcu_user_mem_probe,
> + .driver = {
> + .name = "khadas-mcu-user-mem",
> + },
> + .id_table = khadas_mcu_user_mem_id_table,
> +};
> +
> +module_platform_driver(khadas_mcu_user_mem_driver);
> +
> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
> +MODULE_DESCRIPTION("Khadas MCU User MEM driver");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/6] nvmem: add support for the Khadas MCU Programmable User Memory
2020-05-13 10:34 ` Srinivas Kandagatla
@ 2020-05-13 12:33 ` Neil Armstrong
2020-05-15 10:55 ` Srinivas Kandagatla
0 siblings, 1 reply; 18+ messages in thread
From: Neil Armstrong @ 2020-05-13 12:33 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: linux-amlogic, linux-pm, linux-arm-kernel, linux-kernel
On 13/05/2020 12:34, Srinivas Kandagatla wrote:
>
>
> On 12/05/2020 14:26, Neil Armstrong wrote:
>> The new Khadas VIM2, VIM3 and Edge boards embeds an on-board microcontroller
>> offering a 56bytes User Programmable NVMEM array.
>>
>> This array needs a password to be writable, thus a password sysfs file
>> has been added on the device node to unlock the NVMEM.
>
> Is this one time programmable? Or does it need to be unlocked for every read/write?
It can be rewritten, and needs the password to read & write.
>
> How can you make sure that the memory is unlocked before making attempt to read/write?
The only way to know it's unlock is reading back the password when unlocked.
>
>>
>> The default 6bytes password id: "Khadas"
>>
>> This implements the user NVMEM devices as cell of the Khadas MCU MFD driver.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> drivers/nvmem/Kconfig | 8 ++
>> drivers/nvmem/Makefile | 2 +
>> drivers/nvmem/khadas-mcu-user-mem.c | 128 ++++++++++++++++++++++++++++
>> 3 files changed, 138 insertions(+)
>> create mode 100644 drivers/nvmem/khadas-mcu-user-mem.c
>>
>> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
>> index d7b7f6d688e7..92cd4f6aa931 100644
>> --- a/drivers/nvmem/Kconfig
>> +++ b/drivers/nvmem/Kconfig
>> @@ -67,6 +67,14 @@ config JZ4780_EFUSE
>> To compile this driver as a module, choose M here: the module
>> will be called nvmem_jz4780_efuse.
>> +config NVMEM_KHADAS_MCU_USER_MEM
>> + tristate "Khadas MCU User programmable memory support"
>> + depends on MFD_KHADAS_MCU
>> + depends on REGMAP
>> + help
>> + This is a driver for the MCU User programmable memory
>> + available on the Khadas VIM and Edge boards.
>> +
>> config NVMEM_LPC18XX_EEPROM
>> tristate "NXP LPC18XX EEPROM Memory Support"
>> depends on ARCH_LPC18XX || COMPILE_TEST
>> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
>> index a7c377218341..0516a309542d 100644
>> --- a/drivers/nvmem/Makefile
>> +++ b/drivers/nvmem/Makefile
>> @@ -17,6 +17,8 @@ obj-$(CONFIG_NVMEM_IMX_OCOTP_SCU) += nvmem-imx-ocotp-scu.o
>> nvmem-imx-ocotp-scu-y := imx-ocotp-scu.o
>> obj-$(CONFIG_JZ4780_EFUSE) += nvmem_jz4780_efuse.o
>> nvmem_jz4780_efuse-y := jz4780-efuse.o
>> +obj-$(CONFIG_NVMEM_KHADAS_MCU_USER_MEM) += nvmem-khadas-mcu-user-mem.o
>> +nvmem-khadas-mcu-user-mem-y := khadas-mcu-user-mem.o
>> obj-$(CONFIG_NVMEM_LPC18XX_EEPROM) += nvmem_lpc18xx_eeprom.o
>> nvmem_lpc18xx_eeprom-y := lpc18xx_eeprom.o
>> obj-$(CONFIG_NVMEM_LPC18XX_OTP) += nvmem_lpc18xx_otp.o
>> diff --git a/drivers/nvmem/khadas-mcu-user-mem.c b/drivers/nvmem/khadas-mcu-user-mem.c
>> new file mode 100644
>> index 000000000000..a1d5ae9a030c
>> --- /dev/null
>> +++ b/drivers/nvmem/khadas-mcu-user-mem.c
>> @@ -0,0 +1,128 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for Khadas MCU User programmable Memory
>> + *
>> + * Copyright (C) 2020 BayLibre SAS
>> + * Author(s): Neil Armstrong <narmstrong@baylibre.com>
>> + */
>> +
>> +#include <linux/clk.h>
>
> Why do we need this header?
Will drop
>
>> +#include <linux/module.h>
>> +#include <linux/nvmem-provider.h>
>> +#include <linux/mfd/khadas-mcu.h>
>> +#include <linux/regmap.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +
>> +static int khadas_mcu_user_mem_read(void *context, unsigned int offset,
>> + void *val, size_t bytes)
>> +{
>> + struct khadas_mcu *khadas_mcu = context;
>> +
>> + return regmap_bulk_read(khadas_mcu->map,
>> + KHADAS_MCU_USER_DATA_0_REG + offset,
>> + val, bytes);
>> +}
>> +
>> +static int khadas_mcu_user_mem_write(void *context, unsigned int offset,
>> + void *val, size_t bytes)
>> +{
>> + struct khadas_mcu *khadas_mcu = context;
>> +
>> + return regmap_bulk_write(khadas_mcu->map,
>> + KHADAS_MCU_USER_DATA_0_REG + offset,
>> + val, bytes);
>> +}
>> +
>> +static ssize_t password_store(struct device *dev, struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct khadas_mcu *khadas_mcu = dev_get_drvdata(dev);
>> + int i, ret;
>> +
>> + if (count < 6)
>> + return -EINVAL;
>
> Possibly this magic 6 value needs proper define. An additional check here for > 6 would be better as well.
Ok
>
>> +
>> + ret = regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 1);
>> + if (ret)
>> + return ret;
>> +
>> + for (i = 0 ; i < 6 ; ++i) {
>> + ret = regmap_write(khadas_mcu->map,
>> + KHADAS_MCU_CHECK_USER_PASSWD_REG,
>> + buf[i]);
>> + if (ret)
>> + goto out;
>> + }
>> +
>> + ret = regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 0);
>> + if (ret)
>> + return ret;
>> +
>> + return count;
>> +out:
>> + regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 0);
>> +
>> + return ret;
>> +}
>> +
>> +static DEVICE_ATTR_WO(password);
>> +
>> +static struct attribute *khadas_mcu_user_mem_sysfs_attributes[] = {
>> + &dev_attr_password.attr,
>> + NULL,
>> +};
>> +
>> +static const struct attribute_group khadas_mcu_user_mem_sysfs_attr_group = {
>> + .attrs = khadas_mcu_user_mem_sysfs_attributes,
>> +};
>> +
>> +static int khadas_mcu_user_mem_probe(struct platform_device *pdev)
>> +{
>> + struct khadas_mcu *khadas_mcu = dev_get_drvdata(pdev->dev.parent);
>
> You could probably get away with dependency of this structure and the header itself by directly getting hold of regmap from parent device.
Ok
>
>
>> + struct device *dev = &pdev->dev;
>> + struct nvmem_device *nvmem;
>> + struct nvmem_config *econfig;
>> +
>> + econfig = devm_kzalloc(dev, sizeof(*econfig), GFP_KERNEL);
>> + if (!econfig)
>> + return -ENOMEM;
>> +
>> + econfig->dev = pdev->dev.parent;
>> + econfig->name = dev_name(pdev->dev.parent);
>> + econfig->stride = 1;
>> + econfig->word_size = 1;
>> + econfig->reg_read = khadas_mcu_user_mem_read;
>> + econfig->reg_write = khadas_mcu_user_mem_write;
>> + econfig->size = 56;
> define 56 to make it more readable!
Ok
>
>> + econfig->priv = khadas_mcu;
>> +
>> + platform_set_drvdata(pdev, khadas_mcu);
>> +
>> + nvmem = devm_nvmem_register(&pdev->dev, econfig);
>> + if (IS_ERR(nvmem))
>> + return PTR_ERR(nvmem);
>> +
>> + return sysfs_create_group(&pdev->dev.kobj,
>> + &khadas_mcu_user_mem_sysfs_attr_group);
>> +}
>> +
>> +static const struct platform_device_id khadas_mcu_user_mem_id_table[] = {
>> + { .name = "khadas-mcu-user-mem", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(platform, khadas_mcu_user_mem_id_table);
>> +
>> +static struct platform_driver khadas_mcu_user_mem_driver = {
>> + .probe = khadas_mcu_user_mem_probe,
>> + .driver = {
>> + .name = "khadas-mcu-user-mem",
>> + },
>> + .id_table = khadas_mcu_user_mem_id_table,
>> +};
>> +
>> +module_platform_driver(khadas_mcu_user_mem_driver);
>> +
>> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
>> +MODULE_DESCRIPTION("Khadas MCU User MEM driver");
>> +MODULE_LICENSE("GPL v2");
>>
Thanks for the review.
Neil
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/6] thermal: add support for the MCU controlled FAN on Khadas boards
2020-05-12 13:26 ` [PATCH v2 3/6] thermal: add support for the MCU controlled FAN on Khadas boards Neil Armstrong
@ 2020-05-15 6:41 ` Amit Kucheria
2020-05-15 8:05 ` Neil Armstrong
2020-06-02 8:26 ` Neil Armstrong
0 siblings, 2 replies; 18+ messages in thread
From: Amit Kucheria @ 2020-05-15 6:41 UTC (permalink / raw)
To: Neil Armstrong
Cc: Srini Kandagatla, Zhang Rui, Daniel Lezcano, linux-amlogic,
Linux PM list, lakml, LKML
On Tue, May 12, 2020 at 6:56 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> The new Khadas VIM2 and VIM3 boards controls the cooling fan via the
> on-board microcontroller.
>
> This implements the FAN control as thermal devices and as cell of the Khadas
> MCU MFD driver.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> drivers/thermal/Kconfig | 10 ++
> drivers/thermal/Makefile | 1 +
> drivers/thermal/khadas_mcu_fan.c | 174 +++++++++++++++++++++++++++++++
> 3 files changed, 185 insertions(+)
> create mode 100644 drivers/thermal/khadas_mcu_fan.c
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 91af271e9bb0..72b3960cc5ac 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -490,4 +490,14 @@ config SPRD_THERMAL
> help
> Support for the Spreadtrum thermal sensor driver in the Linux thermal
> framework.
> +
> +config KHADAS_MCU_FAN_THERMAL
> + tristate "Khadas MCU controller FAN cooling support"
> + depends on OF || COMPILE_TEST
Could you add a depends on the some board/SoC Kconfig option here so
this doesn't show up for non-Amlogic/non-Khadas boards?
Looks OK otherwise.
> + select MFD_CORE
> + select REGMAP
> + help
> + If you say yes here you get support for the FAN controlled
> + by the Microcontroller found on the Khadas VIM boards.
> +
> endif
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 8c8ed7b79915..460428c2122c 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -60,3 +60,4 @@ obj-$(CONFIG_ZX2967_THERMAL) += zx2967_thermal.o
> obj-$(CONFIG_UNIPHIER_THERMAL) += uniphier_thermal.o
> obj-$(CONFIG_AMLOGIC_THERMAL) += amlogic_thermal.o
> obj-$(CONFIG_SPRD_THERMAL) += sprd_thermal.o
> +obj-$(CONFIG_KHADAS_MCU_FAN_THERMAL) += khadas_mcu_fan.o
> diff --git a/drivers/thermal/khadas_mcu_fan.c b/drivers/thermal/khadas_mcu_fan.c
> new file mode 100644
> index 000000000000..044d4aba8be2
> --- /dev/null
> +++ b/drivers/thermal/khadas_mcu_fan.c
> @@ -0,0 +1,174 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Khadas MCU Controlled FAN driver
> + *
> + * Copyright (C) 2020 BayLibre SAS
> + * Author(s): Neil Armstrong <narmstrong@baylibre.com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/mfd/khadas-mcu.h>
> +#include <linux/regmap.h>
> +#include <linux/sysfs.h>
> +#include <linux/thermal.h>
> +
> +#define MAX_LEVEL 3
> +
> +struct khadas_mcu_fan_ctx {
> + struct khadas_mcu *mcu;
> + unsigned int level;
> + struct thermal_cooling_device *cdev;
> +};
> +
> +static int khadas_mcu_fan_set_level(struct khadas_mcu_fan_ctx *ctx,
> + unsigned int level)
> +{
> + int ret;
> +
> + ret = regmap_write(ctx->mcu->map, KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG,
> + level);
> + if (ret)
> + return ret;
> +
> + ctx->level = level;
> +
> + return 0;
> +}
> +
> +static int khadas_mcu_fan_get_max_state(struct thermal_cooling_device *cdev,
> + unsigned long *state)
> +{
> + struct khadas_mcu_fan_ctx *ctx = cdev->devdata;
> +
> + if (!ctx)
> + return -EINVAL;
> +
> + *state = MAX_LEVEL;
> +
> + return 0;
> +}
> +
> +static int khadas_mcu_fan_get_cur_state(struct thermal_cooling_device *cdev,
> + unsigned long *state)
> +{
> + struct khadas_mcu_fan_ctx *ctx = cdev->devdata;
> +
> + if (!ctx)
> + return -EINVAL;
> +
> + *state = ctx->level;
> +
> + return 0;
> +}
> +
> +static int
> +khadas_mcu_fan_set_cur_state(struct thermal_cooling_device *cdev,
> + unsigned long state)
> +{
> + struct khadas_mcu_fan_ctx *ctx = cdev->devdata;
> +
> + if (!ctx || (state > MAX_LEVEL))
> + return -EINVAL;
> +
> + if (state == ctx->level)
> + return 0;
> +
> + return khadas_mcu_fan_set_level(ctx, state);
> +}
> +
> +static const struct thermal_cooling_device_ops khadas_mcu_fan_cooling_ops = {
> + .get_max_state = khadas_mcu_fan_get_max_state,
> + .get_cur_state = khadas_mcu_fan_get_cur_state,
> + .set_cur_state = khadas_mcu_fan_set_cur_state,
> +};
> +
> +static int khadas_mcu_fan_probe(struct platform_device *pdev)
> +{
> + struct khadas_mcu *mcu = dev_get_drvdata(pdev->dev.parent);
> + struct thermal_cooling_device *cdev;
> + struct device *dev = &pdev->dev;
> + struct khadas_mcu_fan_ctx *ctx;
> + int ret;
> +
> + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return -ENOMEM;
> + ctx->mcu = mcu;
> + platform_set_drvdata(pdev, ctx);
> +
> + cdev = devm_thermal_of_cooling_device_register(dev->parent,
> + dev->parent->of_node, "khadas-mcu-fan", ctx,
> + &khadas_mcu_fan_cooling_ops);
> + if (IS_ERR(cdev)) {
> + ret = PTR_ERR(cdev);
> + dev_err(dev,
> + "Failed to register khadas-mcu-fan as cooling device: %d\n",
> + ret);
> + return ret;
> + }
> + ctx->cdev = cdev;
> + thermal_cdev_update(cdev);
> +
> + return 0;
> +}
> +
> +static int khadas_mcu_fan_disable(struct device *dev)
> +{
> + struct khadas_mcu_fan_ctx *ctx = dev_get_drvdata(dev);
> + unsigned int level_save = ctx->level;
> + int ret;
> +
> + ret = khadas_mcu_fan_set_level(ctx, 0);
> + if (ret)
> + return ret;
> +
> + ctx->level = level_save;
> +
> + return 0;
> +}
> +
> +static void khadas_mcu_fan_shutdown(struct platform_device *pdev)
> +{
> + khadas_mcu_fan_disable(&pdev->dev);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int khadas_mcu_fan_suspend(struct device *dev)
> +{
> + return khadas_mcu_fan_disable(dev);
> +}
> +
> +static int khadas_mcu_fan_resume(struct device *dev)
> +{
> + struct khadas_mcu_fan_ctx *ctx = dev_get_drvdata(dev);
> +
> + return khadas_mcu_fan_set_level(ctx, ctx->level);
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(khadas_mcu_fan_pm, khadas_mcu_fan_suspend,
> + khadas_mcu_fan_resume);
> +
> +static const struct platform_device_id khadas_mcu_fan_id_table[] = {
> + { .name = "khadas-mcu-fan-ctrl", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(platform, khadas_mcu_fan_id_table);
> +
> +static struct platform_driver khadas_mcu_fan_driver = {
> + .probe = khadas_mcu_fan_probe,
> + .shutdown = khadas_mcu_fan_shutdown,
> + .driver = {
> + .name = "khadas-mcu-fan-ctrl",
> + .pm = &khadas_mcu_fan_pm,
> + },
> + .id_table = khadas_mcu_fan_id_table,
> +};
> +
> +module_platform_driver(khadas_mcu_fan_driver);
> +
> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
> +MODULE_DESCRIPTION("Khadas MCU FAN driver");
> +MODULE_LICENSE("GPL");
> --
> 2.22.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add Khadas Microcontroller bindings
2020-05-12 13:26 ` [PATCH v2 1/6] dt-bindings: mfd: add Khadas Microcontroller bindings Neil Armstrong
@ 2020-05-15 6:42 ` Amit Kucheria
0 siblings, 0 replies; 18+ messages in thread
From: Amit Kucheria @ 2020-05-15 6:42 UTC (permalink / raw)
To: Neil Armstrong
Cc: Lee Jones, Zhang Rui, Daniel Lezcano, Srini Kandagatla,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-amlogic, Linux PM list, lakml, LKML, Rob Herring
On Tue, May 12, 2020 at 6:56 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> This Microcontroller is present on the Khadas VIM1, VIM2, VIM3 and Edge
> boards.
>
> It has multiple boot control features like password check, power-on
> options, power-off control and system FAN control on recent boards.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
> ---
> .../devicetree/bindings/mfd/khadas,mcu.yaml | 44 +++++++++++++++++++
> 1 file changed, 44 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
>
> diff --git a/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml b/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
> new file mode 100644
> index 000000000000..a3b976f101e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/khadas,mcu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Khadas on-board Microcontroller Device Tree Bindings
> +
> +maintainers:
> + - Neil Armstrong <narmstrong@baylibre.com>
> +
> +description: |
> + Khadas embeds a microcontroller on their VIM and Edge boards adding some
> + system feature as PWM Fan control (for VIM2 rev14 or VIM3), User memory
> + storage, IR/Key resume control, system power LED control and more.
> +
> +properties:
> + compatible:
> + enum:
> + - khadas,mcu # MCU revision is discoverable
> +
> + "#cooling-cells": # Only needed for boards having FAN control feature
> + const: 2
> +
> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + khadas_mcu: system-controller@18 {
> + compatible = "khadas,mcu";
> + reg = <0x18>;
> + #cooling-cells = <2>;
> + };
> + };
> --
> 2.22.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/6] thermal: add support for the MCU controlled FAN on Khadas boards
2020-05-15 6:41 ` Amit Kucheria
@ 2020-05-15 8:05 ` Neil Armstrong
2020-06-02 8:26 ` Neil Armstrong
1 sibling, 0 replies; 18+ messages in thread
From: Neil Armstrong @ 2020-05-15 8:05 UTC (permalink / raw)
To: Amit Kucheria
Cc: Srini Kandagatla, Zhang Rui, Daniel Lezcano, linux-amlogic,
Linux PM list, lakml, LKML
On 15/05/2020 08:41, Amit Kucheria wrote:
> On Tue, May 12, 2020 at 6:56 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> The new Khadas VIM2 and VIM3 boards controls the cooling fan via the
>> on-board microcontroller.
>>
>> This implements the FAN control as thermal devices and as cell of the Khadas
>> MCU MFD driver.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> drivers/thermal/Kconfig | 10 ++
>> drivers/thermal/Makefile | 1 +
>> drivers/thermal/khadas_mcu_fan.c | 174 +++++++++++++++++++++++++++++++
>> 3 files changed, 185 insertions(+)
>> create mode 100644 drivers/thermal/khadas_mcu_fan.c
>>
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index 91af271e9bb0..72b3960cc5ac 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -490,4 +490,14 @@ config SPRD_THERMAL
>> help
>> Support for the Spreadtrum thermal sensor driver in the Linux thermal
>> framework.
>> +
>> +config KHADAS_MCU_FAN_THERMAL
>> + tristate "Khadas MCU controller FAN cooling support"
>> + depends on OF || COMPILE_TEST
>
> Could you add a depends on the some board/SoC Kconfig option here so
> this doesn't show up for non-Amlogic/non-Khadas boards?
Sure,
Thanks.
Neil
>
> Looks OK otherwise.
>
>> + select MFD_CORE
>> + select REGMAP
>> + help
>> + If you say yes here you get support for the FAN controlled
>> + by the Microcontroller found on the Khadas VIM boards.
>> +
>> endif
>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>> index 8c8ed7b79915..460428c2122c 100644
>> --- a/drivers/thermal/Makefile
>> +++ b/drivers/thermal/Makefile
>> @@ -60,3 +60,4 @@ obj-$(CONFIG_ZX2967_THERMAL) += zx2967_thermal.o
>> obj-$(CONFIG_UNIPHIER_THERMAL) += uniphier_thermal.o
>> obj-$(CONFIG_AMLOGIC_THERMAL) += amlogic_thermal.o
>> obj-$(CONFIG_SPRD_THERMAL) += sprd_thermal.o
>> +obj-$(CONFIG_KHADAS_MCU_FAN_THERMAL) += khadas_mcu_fan.o
>> diff --git a/drivers/thermal/khadas_mcu_fan.c b/drivers/thermal/khadas_mcu_fan.c
>> new file mode 100644
>> index 000000000000..044d4aba8be2
>> --- /dev/null
>> +++ b/drivers/thermal/khadas_mcu_fan.c
>> @@ -0,0 +1,174 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Khadas MCU Controlled FAN driver
>> + *
>> + * Copyright (C) 2020 BayLibre SAS
>> + * Author(s): Neil Armstrong <narmstrong@baylibre.com>
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/mfd/khadas-mcu.h>
>> +#include <linux/regmap.h>
>> +#include <linux/sysfs.h>
>> +#include <linux/thermal.h>
>> +
>> +#define MAX_LEVEL 3
>> +
>> +struct khadas_mcu_fan_ctx {
>> + struct khadas_mcu *mcu;
>> + unsigned int level;
>> + struct thermal_cooling_device *cdev;
>> +};
>> +
>> +static int khadas_mcu_fan_set_level(struct khadas_mcu_fan_ctx *ctx,
>> + unsigned int level)
>> +{
>> + int ret;
>> +
>> + ret = regmap_write(ctx->mcu->map, KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG,
>> + level);
>> + if (ret)
>> + return ret;
>> +
>> + ctx->level = level;
>> +
>> + return 0;
>> +}
>> +
>> +static int khadas_mcu_fan_get_max_state(struct thermal_cooling_device *cdev,
>> + unsigned long *state)
>> +{
>> + struct khadas_mcu_fan_ctx *ctx = cdev->devdata;
>> +
>> + if (!ctx)
>> + return -EINVAL;
>> +
>> + *state = MAX_LEVEL;
>> +
>> + return 0;
>> +}
>> +
>> +static int khadas_mcu_fan_get_cur_state(struct thermal_cooling_device *cdev,
>> + unsigned long *state)
>> +{
>> + struct khadas_mcu_fan_ctx *ctx = cdev->devdata;
>> +
>> + if (!ctx)
>> + return -EINVAL;
>> +
>> + *state = ctx->level;
>> +
>> + return 0;
>> +}
>> +
>> +static int
>> +khadas_mcu_fan_set_cur_state(struct thermal_cooling_device *cdev,
>> + unsigned long state)
>> +{
>> + struct khadas_mcu_fan_ctx *ctx = cdev->devdata;
>> +
>> + if (!ctx || (state > MAX_LEVEL))
>> + return -EINVAL;
>> +
>> + if (state == ctx->level)
>> + return 0;
>> +
>> + return khadas_mcu_fan_set_level(ctx, state);
>> +}
>> +
>> +static const struct thermal_cooling_device_ops khadas_mcu_fan_cooling_ops = {
>> + .get_max_state = khadas_mcu_fan_get_max_state,
>> + .get_cur_state = khadas_mcu_fan_get_cur_state,
>> + .set_cur_state = khadas_mcu_fan_set_cur_state,
>> +};
>> +
>> +static int khadas_mcu_fan_probe(struct platform_device *pdev)
>> +{
>> + struct khadas_mcu *mcu = dev_get_drvdata(pdev->dev.parent);
>> + struct thermal_cooling_device *cdev;
>> + struct device *dev = &pdev->dev;
>> + struct khadas_mcu_fan_ctx *ctx;
>> + int ret;
>> +
>> + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>> + if (!ctx)
>> + return -ENOMEM;
>> + ctx->mcu = mcu;
>> + platform_set_drvdata(pdev, ctx);
>> +
>> + cdev = devm_thermal_of_cooling_device_register(dev->parent,
>> + dev->parent->of_node, "khadas-mcu-fan", ctx,
>> + &khadas_mcu_fan_cooling_ops);
>> + if (IS_ERR(cdev)) {
>> + ret = PTR_ERR(cdev);
>> + dev_err(dev,
>> + "Failed to register khadas-mcu-fan as cooling device: %d\n",
>> + ret);
>> + return ret;
>> + }
>> + ctx->cdev = cdev;
>> + thermal_cdev_update(cdev);
>> +
>> + return 0;
>> +}
>> +
>> +static int khadas_mcu_fan_disable(struct device *dev)
>> +{
>> + struct khadas_mcu_fan_ctx *ctx = dev_get_drvdata(dev);
>> + unsigned int level_save = ctx->level;
>> + int ret;
>> +
>> + ret = khadas_mcu_fan_set_level(ctx, 0);
>> + if (ret)
>> + return ret;
>> +
>> + ctx->level = level_save;
>> +
>> + return 0;
>> +}
>> +
>> +static void khadas_mcu_fan_shutdown(struct platform_device *pdev)
>> +{
>> + khadas_mcu_fan_disable(&pdev->dev);
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int khadas_mcu_fan_suspend(struct device *dev)
>> +{
>> + return khadas_mcu_fan_disable(dev);
>> +}
>> +
>> +static int khadas_mcu_fan_resume(struct device *dev)
>> +{
>> + struct khadas_mcu_fan_ctx *ctx = dev_get_drvdata(dev);
>> +
>> + return khadas_mcu_fan_set_level(ctx, ctx->level);
>> +}
>> +#endif
>> +
>> +static SIMPLE_DEV_PM_OPS(khadas_mcu_fan_pm, khadas_mcu_fan_suspend,
>> + khadas_mcu_fan_resume);
>> +
>> +static const struct platform_device_id khadas_mcu_fan_id_table[] = {
>> + { .name = "khadas-mcu-fan-ctrl", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(platform, khadas_mcu_fan_id_table);
>> +
>> +static struct platform_driver khadas_mcu_fan_driver = {
>> + .probe = khadas_mcu_fan_probe,
>> + .shutdown = khadas_mcu_fan_shutdown,
>> + .driver = {
>> + .name = "khadas-mcu-fan-ctrl",
>> + .pm = &khadas_mcu_fan_pm,
>> + },
>> + .id_table = khadas_mcu_fan_id_table,
>> +};
>> +
>> +module_platform_driver(khadas_mcu_fan_driver);
>> +
>> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
>> +MODULE_DESCRIPTION("Khadas MCU FAN driver");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.22.0
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/6] nvmem: add support for the Khadas MCU Programmable User Memory
2020-05-13 12:33 ` Neil Armstrong
@ 2020-05-15 10:55 ` Srinivas Kandagatla
2020-06-02 8:29 ` Neil Armstrong
0 siblings, 1 reply; 18+ messages in thread
From: Srinivas Kandagatla @ 2020-05-15 10:55 UTC (permalink / raw)
To: Neil Armstrong; +Cc: linux-amlogic, linux-pm, linux-arm-kernel, linux-kernel
On 13/05/2020 13:33, Neil Armstrong wrote:
> On 13/05/2020 12:34, Srinivas Kandagatla wrote:
>>
>>
>> On 12/05/2020 14:26, Neil Armstrong wrote:
>>> The new Khadas VIM2, VIM3 and Edge boards embeds an on-board microcontroller
>>> offering a 56bytes User Programmable NVMEM array.
>>>
>>> This array needs a password to be writable, thus a password sysfs file
>>> has been added on the device node to unlock the NVMEM.
>>
>> Is this one time programmable? Or does it need to be unlocked for every read/write?
>
> It can be rewritten, and needs the password to read & write.
>
>>
>> How can you make sure that the memory is unlocked before making attempt to read/write?
>
> The only way to know it's unlock is reading back the password when unlocked.
Current code has no such checks for every read/write?
and looks very disconnected w.r.t to password and read/writes.
If user attempts to read/write he will not be aware that he should
program the password first!
Also if the password is static or un-changed then why not just
statically program this from the driver rather than providing sysfs file?
I dont see how kernel nvmem read/write interface can make sure that
memory is unlocked?
Who is the real consumer for this provider?
--srini
>
>>
>>>
>>> The default 6bytes password id: "Khadas"
>>>
>>> This implements the user NVMEM devices as cell of the Khadas MCU MFD driver.
>>>
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>> drivers/nvmem/Kconfig | 8 ++
>>> drivers/nvmem/Makefile | 2 +
>>> drivers/nvmem/khadas-mcu-user-mem.c | 128 ++++++++++++++++++++++++++++
>>> 3 files changed, 138 insertions(+)
>>> create mode 100644 drivers/nvmem/khadas-mcu-user-mem.c
>>>
>>> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
>>> index d7b7f6d688e7..92cd4f6aa931 100644
>>> --- a/drivers/nvmem/Kconfig
>>> +++ b/drivers/nvmem/Kconfig
>>> @@ -67,6 +67,14 @@ config JZ4780_EFUSE
>>> To compile this driver as a module, choose M here: the module
>>> will be called nvmem_jz4780_efuse.
>>> +config NVMEM_KHADAS_MCU_USER_MEM
>>> + tristate "Khadas MCU User programmable memory support"
>>> + depends on MFD_KHADAS_MCU
>>> + depends on REGMAP
>>> + help
>>> + This is a driver for the MCU User programmable memory
>>> + available on the Khadas VIM and Edge boards.
>>> +
>>> config NVMEM_LPC18XX_EEPROM
>>> tristate "NXP LPC18XX EEPROM Memory Support"
>>> depends on ARCH_LPC18XX || COMPILE_TEST
>>> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
>>> index a7c377218341..0516a309542d 100644
>>> --- a/drivers/nvmem/Makefile
>>> +++ b/drivers/nvmem/Makefile
>>> @@ -17,6 +17,8 @@ obj-$(CONFIG_NVMEM_IMX_OCOTP_SCU) += nvmem-imx-ocotp-scu.o
>>> nvmem-imx-ocotp-scu-y := imx-ocotp-scu.o
>>> obj-$(CONFIG_JZ4780_EFUSE) += nvmem_jz4780_efuse.o
>>> nvmem_jz4780_efuse-y := jz4780-efuse.o
>>> +obj-$(CONFIG_NVMEM_KHADAS_MCU_USER_MEM) += nvmem-khadas-mcu-user-mem.o
>>> +nvmem-khadas-mcu-user-mem-y := khadas-mcu-user-mem.o
>>> obj-$(CONFIG_NVMEM_LPC18XX_EEPROM) += nvmem_lpc18xx_eeprom.o
>>> nvmem_lpc18xx_eeprom-y := lpc18xx_eeprom.o
>>> obj-$(CONFIG_NVMEM_LPC18XX_OTP) += nvmem_lpc18xx_otp.o
>>> diff --git a/drivers/nvmem/khadas-mcu-user-mem.c b/drivers/nvmem/khadas-mcu-user-mem.c
>>> new file mode 100644
>>> index 000000000000..a1d5ae9a030c
>>> --- /dev/null
>>> +++ b/drivers/nvmem/khadas-mcu-user-mem.c
>>> @@ -0,0 +1,128 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Driver for Khadas MCU User programmable Memory
>>> + *
>>> + * Copyright (C) 2020 BayLibre SAS
>>> + * Author(s): Neil Armstrong <narmstrong@baylibre.com>
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>
>> Why do we need this header?
>
> Will drop
>
>>
>>> +#include <linux/module.h>
>>> +#include <linux/nvmem-provider.h>
>>> +#include <linux/mfd/khadas-mcu.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +static int khadas_mcu_user_mem_read(void *context, unsigned int offset,
>>> + void *val, size_t bytes)
>>> +{
>>> + struct khadas_mcu *khadas_mcu = context;
>>> +
>>> + return regmap_bulk_read(khadas_mcu->map,
>>> + KHADAS_MCU_USER_DATA_0_REG + offset,
>>> + val, bytes);
>>> +}
>>> +
>>> +static int khadas_mcu_user_mem_write(void *context, unsigned int offset,
>>> + void *val, size_t bytes)
>>> +{
>>> + struct khadas_mcu *khadas_mcu = context;
>>> +
>>> + return regmap_bulk_write(khadas_mcu->map,
>>> + KHADAS_MCU_USER_DATA_0_REG + offset,
>>> + val, bytes);
>>> +}
>>> +
>>> +static ssize_t password_store(struct device *dev, struct device_attribute *attr,
>>> + const char *buf, size_t count)
>>> +{
>>> + struct khadas_mcu *khadas_mcu = dev_get_drvdata(dev);
>>> + int i, ret;
>>> +
>>> + if (count < 6)
>>> + return -EINVAL;
>>
>> Possibly this magic 6 value needs proper define. An additional check here for > 6 would be better as well.
>
> Ok
>
>>
>>> +
>>> + ret = regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 1);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + for (i = 0 ; i < 6 ; ++i) {
>>> + ret = regmap_write(khadas_mcu->map,
>>> + KHADAS_MCU_CHECK_USER_PASSWD_REG,
>>> + buf[i]);
>>> + if (ret)
>>> + goto out;
>>> + }
>>> +
>>> + ret = regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 0);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + return count;
>>> +out:
>>> + regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 0);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static DEVICE_ATTR_WO(password);
>>> +
>>> +static struct attribute *khadas_mcu_user_mem_sysfs_attributes[] = {
>>> + &dev_attr_password.attr,
>>> + NULL,
>>> +};
>>> +
>>> +static const struct attribute_group khadas_mcu_user_mem_sysfs_attr_group = {
>>> + .attrs = khadas_mcu_user_mem_sysfs_attributes,
>>> +};
>>> +
>>> +static int khadas_mcu_user_mem_probe(struct platform_device *pdev)
>>> +{
>>> + struct khadas_mcu *khadas_mcu = dev_get_drvdata(pdev->dev.parent);
>>
>> You could probably get away with dependency of this structure and the header itself by directly getting hold of regmap from parent device.
>
> Ok
>
>>
>>
>>> + struct device *dev = &pdev->dev;
>>> + struct nvmem_device *nvmem;
>>> + struct nvmem_config *econfig;
>>> +
>>> + econfig = devm_kzalloc(dev, sizeof(*econfig), GFP_KERNEL);
>>> + if (!econfig)
>>> + return -ENOMEM;
>>> +
>>> + econfig->dev = pdev->dev.parent;
>>> + econfig->name = dev_name(pdev->dev.parent);
>>> + econfig->stride = 1;
>>> + econfig->word_size = 1;
>>> + econfig->reg_read = khadas_mcu_user_mem_read;
>>> + econfig->reg_write = khadas_mcu_user_mem_write;
>>> + econfig->size = 56;
>> define 56 to make it more readable!
>
> Ok
>
>>
>>> + econfig->priv = khadas_mcu;
>>> +
>>> + platform_set_drvdata(pdev, khadas_mcu);
>>> +
>>> + nvmem = devm_nvmem_register(&pdev->dev, econfig);
>>> + if (IS_ERR(nvmem))
>>> + return PTR_ERR(nvmem);
>>> +
>>> + return sysfs_create_group(&pdev->dev.kobj,
>>> + &khadas_mcu_user_mem_sysfs_attr_group);
>>> +}
>>> +
>>> +static const struct platform_device_id khadas_mcu_user_mem_id_table[] = {
>>> + { .name = "khadas-mcu-user-mem", },
>>> + {},
>>> +};
>>> +MODULE_DEVICE_TABLE(platform, khadas_mcu_user_mem_id_table);
>>> +
>>> +static struct platform_driver khadas_mcu_user_mem_driver = {
>>> + .probe = khadas_mcu_user_mem_probe,
>>> + .driver = {
>>> + .name = "khadas-mcu-user-mem",
>>> + },
>>> + .id_table = khadas_mcu_user_mem_id_table,
>>> +};
>>> +
>>> +module_platform_driver(khadas_mcu_user_mem_driver);
>>> +
>>> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
>>> +MODULE_DESCRIPTION("Khadas MCU User MEM driver");
>>> +MODULE_LICENSE("GPL v2");
>>>
>
> Thanks for the review.
>
> Neil
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/6] mfd: add support for the Khadas System control Microcontroller
2020-05-12 13:26 ` [PATCH v2 2/6] mfd: add support for the Khadas System control Microcontroller Neil Armstrong
@ 2020-05-20 9:01 ` Lee Jones
2020-06-02 8:26 ` Neil Armstrong
0 siblings, 1 reply; 18+ messages in thread
From: Lee Jones @ 2020-05-20 9:01 UTC (permalink / raw)
To: Neil Armstrong; +Cc: linux-amlogic, linux-pm, linux-arm-kernel, linux-kernel
On Tue, 12 May 2020, Neil Armstrong wrote:
> This Microcontroller is present on the Khadas VIM1, VIM2, VIM3 and Edge
> boards.
>
> It has multiple boot control features like password check, power-on
> options, power-off control and system FAN control on recent boards.
>
> This implements a very basic MFD driver with the fan control and User
> NVMEM cells.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> drivers/mfd/Kconfig | 14 ++++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/khadas-mcu.c | 143 +++++++++++++++++++++++++++++++++
> include/linux/mfd/khadas-mcu.h | 91 +++++++++++++++++++++
> 4 files changed, 249 insertions(+)
> create mode 100644 drivers/mfd/khadas-mcu.c
> create mode 100644 include/linux/mfd/khadas-mcu.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 0a59249198d3..b95091397052 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2003,6 +2003,20 @@ config MFD_WCD934X
> This driver provides common support WCD934x audio codec and its
> associated Pin Controller, Soundwire Controller and Audio codec.
>
> +config MFD_KHADAS_MCU
> + tristate "Support for Khadas System control Microcontroller"
> + depends on I2C
> + depends on OF || COMPILE_TEST
> + select MFD_CORE
> + select REGMAP_I2C
> + help
> + Support for the Khadas System control Microcontroller interface present
> + on their VIM and Edge boards.
> +
> + This driver provides common support for accessing the device,
> + additional drivers must be enabled in order to use the functionality
> + of the device.
It would be good to describe the device here.
> menu "Multimedia Capabilities Port drivers"
> depends on ARCH_SA1100
>
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index f935d10cbf0f..0f1633b096bb 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -257,5 +257,6 @@ obj-$(CONFIG_MFD_ROHM_BD70528) += rohm-bd70528.o
> obj-$(CONFIG_MFD_ROHM_BD71828) += rohm-bd71828.o
> obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
> obj-$(CONFIG_MFD_STMFX) += stmfx.o
> +obj-$(CONFIG_MFD_KHADAS_MCU) += khadas-mcu.o
>
> obj-$(CONFIG_SGI_MFD_IOC3) += ioc3.o
> diff --git a/drivers/mfd/khadas-mcu.c b/drivers/mfd/khadas-mcu.c
> new file mode 100644
> index 000000000000..6d08fa2e373a
> --- /dev/null
> +++ b/drivers/mfd/khadas-mcu.c
> @@ -0,0 +1,143 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Khadas System control Microcontroller
> + *
> + * Copyright (C) 2020 BayLibre SAS
Nit: '\n' here please.
> + * Author(s): Neil Armstrong <narmstrong@baylibre.com>
> + */
> +#include <linux/bitfield.h>
> +#include <linux/i2c.h>
> +#include <linux/mfd/khadas-mcu.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
Alphabetical.
> +static bool khadas_mcu_reg_volatile(struct device *dev, unsigned int reg)
> +{
> + if (reg >= KHADAS_MCU_USER_DATA_0_REG &&
> + reg < KHADAS_MCU_PWR_OFF_CMD_REG)
> + return true;
> +
> + switch (reg) {
> + case KHADAS_MCU_PWR_OFF_CMD_REG:
> + case KHADAS_MCU_PASSWD_START_REG:
> + case KHADAS_MCU_CHECK_VEN_PASSWD_REG:
> + case KHADAS_MCU_CHECK_USER_PASSWD_REG:
> + case KHADAS_MCU_WOL_INIT_START_REG:
> + case KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static bool khadas_mcu_reg_writeable(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case KHADAS_MCU_PASSWD_VEN_0_REG:
> + case KHADAS_MCU_PASSWD_VEN_1_REG:
> + case KHADAS_MCU_PASSWD_VEN_2_REG:
> + case KHADAS_MCU_PASSWD_VEN_3_REG:
> + case KHADAS_MCU_PASSWD_VEN_4_REG:
> + case KHADAS_MCU_PASSWD_VEN_5_REG:
> + case KHADAS_MCU_MAC_0_REG:
> + case KHADAS_MCU_MAC_1_REG:
> + case KHADAS_MCU_MAC_2_REG:
> + case KHADAS_MCU_MAC_3_REG:
> + case KHADAS_MCU_MAC_4_REG:
> + case KHADAS_MCU_MAC_5_REG:
> + case KHADAS_MCU_USID_0_REG:
> + case KHADAS_MCU_USID_1_REG:
> + case KHADAS_MCU_USID_2_REG:
> + case KHADAS_MCU_USID_3_REG:
> + case KHADAS_MCU_USID_4_REG:
> + case KHADAS_MCU_USID_5_REG:
> + case KHADAS_MCU_VERSION_0_REG:
> + case KHADAS_MCU_VERSION_1_REG:
> + case KHADAS_MCU_DEVICE_NO_0_REG:
> + case KHADAS_MCU_DEVICE_NO_1_REG:
> + case KHADAS_MCU_FACTORY_TEST_REG:
> + case KHADAS_MCU_SHUTDOWN_NORMAL_STATUS_REG:
> + return false;
> + default:
> + return true;
> + }
> +}
> +
> +static const struct regmap_config khadas_mcu_regmap_config = {
> + .reg_bits = 8,
> + .reg_stride = 1,
> + .val_bits = 8,
> + .max_register = KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG,
> + .volatile_reg = khadas_mcu_reg_volatile,
> + .writeable_reg = khadas_mcu_reg_writeable,
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> +static struct mfd_cell khadas_mcu_fan_cells[] = {
> + /* Feature supported only on VIM1/2 Rev13+ and VIM3 */
Doesn't read great.
Consider reversing or make the sentence more succinct.
"VIM1/2 Rev13+ and VIM3 only"
> + { .name = "khadas-mcu-fan-ctrl", },
> +};
> +
> +static struct mfd_cell khadas_mcu_cells[] = {
> + /* Features supported on all board revisions */
I think we can omit this.
> + { .name = "khadas-mcu-user-mem", },
> +};
> +
> +static int khadas_mcu_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct device *dev = &client->dev;
> + struct khadas_mcu *khadas_mcu;
Prefer a rename to 'ddata'.
> + int ret;
> +
> + khadas_mcu = devm_kzalloc(dev, sizeof(*khadas_mcu), GFP_KERNEL);
> + if (!khadas_mcu)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(client, khadas_mcu);
> +
> + khadas_mcu->dev = dev;
> +
> + khadas_mcu->map = devm_regmap_init_i2c(client,
> + &khadas_mcu_regmap_config);
Prefer a rename to 'regmap'.
> + if (IS_ERR(khadas_mcu->map)) {
> + ret = PTR_ERR(khadas_mcu->map);
> + dev_err(dev, "Failed to allocate register map: %d\n", ret);
> + return ret;
> + }
> +
> + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
> + khadas_mcu_cells,
> + ARRAY_SIZE(khadas_mcu_cells),
> + NULL, 0, NULL);
> + if (ret)
> + return ret;
> +
> + if (of_find_property(dev->of_node, "#cooling-cells", NULL))
> + return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
> + khadas_mcu_fan_cells,
> + ARRAY_SIZE(khadas_mcu_fan_cells),
> + NULL, 0, NULL);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id khadas_mcu_of_match[] = {
> + { .compatible = "khadas,mcu", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, khadas_mcu_of_match);
> +
> +static struct i2c_driver khadas_mcu_driver = {
> + .driver = {
> + .name = "khadas-mcu-core",
> + .of_match_table = of_match_ptr(khadas_mcu_of_match),
> + },
> + .probe = khadas_mcu_probe,
> +};
> +module_i2c_driver(khadas_mcu_driver);
> +
> +MODULE_DESCRIPTION("Khadas MCU core driver");
> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/khadas-mcu.h b/include/linux/mfd/khadas-mcu.h
> new file mode 100644
> index 000000000000..2e68af21735c
> --- /dev/null
> +++ b/include/linux/mfd/khadas-mcu.h
> @@ -0,0 +1,91 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Khadas System control Microcontroller Register map
> + *
> + * Copyright (C) 2020 BayLibre SAS
Nit: '\n'
> + * Author(s): Neil Armstrong <narmstrong@baylibre.com>
> + */
> +
> +#ifndef MFD_KHADAS_MCU_H
> +#define MFD_KHADAS_MCU_H
> +
> +#define KHADAS_MCU_PASSWD_VEN_0_REG 0x0 /* RO */
Nit: Can you pad these please?
> +#define KHADAS_MCU_PASSWD_VEN_1_REG 0x1 /* RO */
> +#define KHADAS_MCU_PASSWD_VEN_2_REG 0x2 /* RO */
> +#define KHADAS_MCU_PASSWD_VEN_3_REG 0x3 /* RO */
> +#define KHADAS_MCU_PASSWD_VEN_4_REG 0x4 /* RO */
> +#define KHADAS_MCU_PASSWD_VEN_5_REG 0x5 /* RO */
> +#define KHADAS_MCU_MAC_0_REG 0x6 /* RO */
> +#define KHADAS_MCU_MAC_1_REG 0x7 /* RO */
> +#define KHADAS_MCU_MAC_2_REG 0x8 /* RO */
> +#define KHADAS_MCU_MAC_3_REG 0x9 /* RO */
> +#define KHADAS_MCU_MAC_4_REG 0xa /* RO */
> +#define KHADAS_MCU_MAC_5_REG 0xb /* RO */
> +#define KHADAS_MCU_USID_0_REG 0xc /* RO */
> +#define KHADAS_MCU_USID_1_REG 0xd /* RO */
> +#define KHADAS_MCU_USID_2_REG 0xe /* RO */
> +#define KHADAS_MCU_USID_3_REG 0xf /* RO */
> +#define KHADAS_MCU_USID_4_REG 0x10 /* RO */
> +#define KHADAS_MCU_USID_5_REG 0x11 /* RO */
> +#define KHADAS_MCU_VERSION_0_REG 0x12 /* RO */
> +#define KHADAS_MCU_VERSION_1_REG 0x13 /* RO */
> +#define KHADAS_MCU_DEVICE_NO_0_REG 0x14 /* RO */
> +#define KHADAS_MCU_DEVICE_NO_1_REG 0x15 /* RO */
> +#define KHADAS_MCU_FACTORY_TEST_REG 0x16 /* R */
> +#define KHADAS_MCU_BOOT_MODE_REG 0x20 /* RW */
> +#define KHADAS_MCU_BOOT_EN_WOL_REG 0x21 /* RW */
> +#define KHADAS_MCU_BOOT_EN_RTC_REG 0x22 /* RW */
> +#define KHADAS_MCU_BOOT_EN_EXP_REG 0x23 /* RW */
> +#define KHADAS_MCU_BOOT_EN_IR_REG 0x24 /* RW */
> +#define KHADAS_MCU_BOOT_EN_DCIN_REG 0x25 /* RW */
> +#define KHADAS_MCU_BOOT_EN_KEY_REG 0x26 /* RW */
> +#define KHADAS_MCU_KEY_MODE_REG 0x27 /* RW */
> +#define KHADAS_MCU_LED_MODE_ON_REG 0x28 /* RW */
> +#define KHADAS_MCU_LED_MODE_OFF_REG 0x29 /* RW */
> +#define KHADAS_MCU_SHUTDOWN_NORMAL_REG 0x2c /* RW */
> +#define KHADAS_MCU_MAC_SWITCH_REG 0x2d /* RW */
> +#define KHADAS_MCU_MCU_SLEEP_MODE_REG 0x2e /* RW */
> +#define KHADAS_MCU_IR_CODE1_0_REG 0x2f /* RW */
> +#define KHADAS_MCU_IR_CODE1_1_REG 0x30 /* RW */
> +#define KHADAS_MCU_IR_CODE1_2_REG 0x31 /* RW */
> +#define KHADAS_MCU_IR_CODE1_3_REG 0x32 /* RW */
> +#define KHADAS_MCU_USB_PCIE_SWITCH_REG 0x33 /* RW */
> +#define KHADAS_MCU_IR_CODE2_0_REG 0x34 /* RW */
> +#define KHADAS_MCU_IR_CODE2_1_REG 0x35 /* RW */
> +#define KHADAS_MCU_IR_CODE2_2_REG 0x36 /* RW */
> +#define KHADAS_MCU_IR_CODE2_3_REG 0x37 /* RW */
> +#define KHADAS_MCU_PASSWD_USER_0_REG 0x40 /* RW */
> +#define KHADAS_MCU_PASSWD_USER_1_REG 0x41 /* RW */
> +#define KHADAS_MCU_PASSWD_USER_2_REG 0x42 /* RW */
> +#define KHADAS_MCU_PASSWD_USER_3_REG 0x43 /* RW */
> +#define KHADAS_MCU_PASSWD_USER_4_REG 0x44 /* RW */
> +#define KHADAS_MCU_PASSWD_USER_5_REG 0x45 /* RW */
> +#define KHADAS_MCU_USER_DATA_0_REG 0x46 /* RW 56 bytes */
> +#define KHADAS_MCU_PWR_OFF_CMD_REG 0x80 /* WO */
> +#define KHADAS_MCU_PASSWD_START_REG 0x81 /* WO */
> +#define KHADAS_MCU_CHECK_VEN_PASSWD_REG 0x82 /* WO */
> +#define KHADAS_MCU_CHECK_USER_PASSWD_REG 0x83 /* WO */
> +#define KHADAS_MCU_SHUTDOWN_NORMAL_STATUS_REG 0x86 /* RO */
> +#define KHADAS_MCU_WOL_INIT_START_REG 0x87 /* WO */
> +#define KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG 0x88 /* WO */
> +
> +/* Boards */
I think the names make this superfluous.
> +enum {
> + KHADAS_BOARD_VIM1 = 0x1,
> + KHADAS_BOARD_VIM2,
> + KHADAS_BOARD_VIM3,
> + KHADAS_BOARD_EDGE = 0x11,
> + KHADAS_BOARD_EDGE_V,
> +};
> +
> +/**
> + * struct khadas_mcu_data - Khadas MCU MFD structure
Doesn't match the struct name.
Prefer you drop the 'MFD' part.
> + * @device: device reference used for logs
> + * @map: register map
> + */
> +struct khadas_mcu {
> + struct device *dev;
> + struct regmap *map;
> +};
> +
> +#endif /* MFD_KHADAS_MCU_H */
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/6] mfd: add support for the Khadas System control Microcontroller
2020-05-20 9:01 ` Lee Jones
@ 2020-06-02 8:26 ` Neil Armstrong
2020-06-02 8:33 ` Lee Jones
0 siblings, 1 reply; 18+ messages in thread
From: Neil Armstrong @ 2020-06-02 8:26 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-amlogic, linux-pm, linux-arm-kernel, linux-kernel
On 20/05/2020 11:01, Lee Jones wrote:
> On Tue, 12 May 2020, Neil Armstrong wrote:
>
>> This Microcontroller is present on the Khadas VIM1, VIM2, VIM3 and Edge
>> boards.
>>
>> It has multiple boot control features like password check, power-on
>> options, power-off control and system FAN control on recent boards.
>>
>> This implements a very basic MFD driver with the fan control and User
>> NVMEM cells.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> drivers/mfd/Kconfig | 14 ++++
>> drivers/mfd/Makefile | 1 +
>> drivers/mfd/khadas-mcu.c | 143 +++++++++++++++++++++++++++++++++
>> include/linux/mfd/khadas-mcu.h | 91 +++++++++++++++++++++
>> 4 files changed, 249 insertions(+)
>> create mode 100644 drivers/mfd/khadas-mcu.c
>> create mode 100644 include/linux/mfd/khadas-mcu.h
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index 0a59249198d3..b95091397052 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -2003,6 +2003,20 @@ config MFD_WCD934X
>> This driver provides common support WCD934x audio codec and its
>> associated Pin Controller, Soundwire Controller and Audio codec.
>>
>> +config MFD_KHADAS_MCU
>> + tristate "Support for Khadas System control Microcontroller"
>> + depends on I2C
>> + depends on OF || COMPILE_TEST
>> + select MFD_CORE
>> + select REGMAP_I2C
>> + help
>> + Support for the Khadas System control Microcontroller interface present
>> + on their VIM and Edge boards.
>> +
>> + This driver provides common support for accessing the device,
>> + additional drivers must be enabled in order to use the functionality
>> + of the device.
>
> It would be good to describe the device here.
Ok
>
>> menu "Multimedia Capabilities Port drivers"
>> depends on ARCH_SA1100
>>
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index f935d10cbf0f..0f1633b096bb 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -257,5 +257,6 @@ obj-$(CONFIG_MFD_ROHM_BD70528) += rohm-bd70528.o
>> obj-$(CONFIG_MFD_ROHM_BD71828) += rohm-bd71828.o
>> obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
>> obj-$(CONFIG_MFD_STMFX) += stmfx.o
>> +obj-$(CONFIG_MFD_KHADAS_MCU) += khadas-mcu.o
>>
>> obj-$(CONFIG_SGI_MFD_IOC3) += ioc3.o
>> diff --git a/drivers/mfd/khadas-mcu.c b/drivers/mfd/khadas-mcu.c
>> new file mode 100644
>> index 000000000000..6d08fa2e373a
>> --- /dev/null
>> +++ b/drivers/mfd/khadas-mcu.c
>> @@ -0,0 +1,143 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for Khadas System control Microcontroller
>> + *
>> + * Copyright (C) 2020 BayLibre SAS
>
> Nit: '\n' here please.
Ok
>
>> + * Author(s): Neil Armstrong <narmstrong@baylibre.com>
>> + */
>> +#include <linux/bitfield.h>
>> +#include <linux/i2c.h>
>> +#include <linux/mfd/khadas-mcu.h>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/core.h>
>> +#include <linux/module.h>
>
> Alphabetical.
Ok
>
>> +static bool khadas_mcu_reg_volatile(struct device *dev, unsigned int reg)
>> +{
>> + if (reg >= KHADAS_MCU_USER_DATA_0_REG &&
>> + reg < KHADAS_MCU_PWR_OFF_CMD_REG)
>> + return true;
>> +
>> + switch (reg) {
>> + case KHADAS_MCU_PWR_OFF_CMD_REG:
>> + case KHADAS_MCU_PASSWD_START_REG:
>> + case KHADAS_MCU_CHECK_VEN_PASSWD_REG:
>> + case KHADAS_MCU_CHECK_USER_PASSWD_REG:
>> + case KHADAS_MCU_WOL_INIT_START_REG:
>> + case KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG:
>> + return true;
>> + default:
>> + return false;
>> + }
>> +}
>> +
>> +static bool khadas_mcu_reg_writeable(struct device *dev, unsigned int reg)
>> +{
>> + switch (reg) {
>> + case KHADAS_MCU_PASSWD_VEN_0_REG:
>> + case KHADAS_MCU_PASSWD_VEN_1_REG:
>> + case KHADAS_MCU_PASSWD_VEN_2_REG:
>> + case KHADAS_MCU_PASSWD_VEN_3_REG:
>> + case KHADAS_MCU_PASSWD_VEN_4_REG:
>> + case KHADAS_MCU_PASSWD_VEN_5_REG:
>> + case KHADAS_MCU_MAC_0_REG:
>> + case KHADAS_MCU_MAC_1_REG:
>> + case KHADAS_MCU_MAC_2_REG:
>> + case KHADAS_MCU_MAC_3_REG:
>> + case KHADAS_MCU_MAC_4_REG:
>> + case KHADAS_MCU_MAC_5_REG:
>> + case KHADAS_MCU_USID_0_REG:
>> + case KHADAS_MCU_USID_1_REG:
>> + case KHADAS_MCU_USID_2_REG:
>> + case KHADAS_MCU_USID_3_REG:
>> + case KHADAS_MCU_USID_4_REG:
>> + case KHADAS_MCU_USID_5_REG:
>> + case KHADAS_MCU_VERSION_0_REG:
>> + case KHADAS_MCU_VERSION_1_REG:
>> + case KHADAS_MCU_DEVICE_NO_0_REG:
>> + case KHADAS_MCU_DEVICE_NO_1_REG:
>> + case KHADAS_MCU_FACTORY_TEST_REG:
>> + case KHADAS_MCU_SHUTDOWN_NORMAL_STATUS_REG:
>> + return false;
>> + default:
>> + return true;
>> + }
>> +}
>> +
>> +static const struct regmap_config khadas_mcu_regmap_config = {
>> + .reg_bits = 8,
>> + .reg_stride = 1,
>> + .val_bits = 8,
>> + .max_register = KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG,
>> + .volatile_reg = khadas_mcu_reg_volatile,
>> + .writeable_reg = khadas_mcu_reg_writeable,
>> + .cache_type = REGCACHE_RBTREE,
>> +};
>> +
>> +static struct mfd_cell khadas_mcu_fan_cells[] = {
>> + /* Feature supported only on VIM1/2 Rev13+ and VIM3 */
>
> Doesn't read great.
>
> Consider reversing or make the sentence more succinct.
>
> "VIM1/2 Rev13+ and VIM3 only"
Ok
>
>> + { .name = "khadas-mcu-fan-ctrl", },
>> +};
>> +
>> +static struct mfd_cell khadas_mcu_cells[] = {
>> + /* Features supported on all board revisions */
>
> I think we can omit this.
Ok
>
>> + { .name = "khadas-mcu-user-mem", },
>> +};
>> +
>> +static int khadas_mcu_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + struct device *dev = &client->dev;
>> + struct khadas_mcu *khadas_mcu;
>
> Prefer a rename to 'ddata'.
Ok
>
>> + int ret;
>> +
>> + khadas_mcu = devm_kzalloc(dev, sizeof(*khadas_mcu), GFP_KERNEL);
>> + if (!khadas_mcu)
>> + return -ENOMEM;
>> +
>> + i2c_set_clientdata(client, khadas_mcu);
>> +
>> + khadas_mcu->dev = dev;
>> +
>> + khadas_mcu->map = devm_regmap_init_i2c(client,
>> + &khadas_mcu_regmap_config);
>
Ok
> Prefer a rename to 'regmap'.
>
>> + if (IS_ERR(khadas_mcu->map)) {
>> + ret = PTR_ERR(khadas_mcu->map);
>> + dev_err(dev, "Failed to allocate register map: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
>> + khadas_mcu_cells,
>> + ARRAY_SIZE(khadas_mcu_cells),
>> + NULL, 0, NULL);
>> + if (ret)
>> + return ret;
>> +
>> + if (of_find_property(dev->of_node, "#cooling-cells", NULL))
>> + return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
>> + khadas_mcu_fan_cells,
>> + ARRAY_SIZE(khadas_mcu_fan_cells),
>> + NULL, 0, NULL);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id khadas_mcu_of_match[] = {
>> + { .compatible = "khadas,mcu", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, khadas_mcu_of_match);
>> +
>> +static struct i2c_driver khadas_mcu_driver = {
>> + .driver = {
>> + .name = "khadas-mcu-core",
>> + .of_match_table = of_match_ptr(khadas_mcu_of_match),
>> + },
>> + .probe = khadas_mcu_probe,
>> +};
>> +module_i2c_driver(khadas_mcu_driver);
>> +
>> +MODULE_DESCRIPTION("Khadas MCU core driver");
>> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/include/linux/mfd/khadas-mcu.h b/include/linux/mfd/khadas-mcu.h
>> new file mode 100644
>> index 000000000000..2e68af21735c
>> --- /dev/null
>> +++ b/include/linux/mfd/khadas-mcu.h
>> @@ -0,0 +1,91 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Khadas System control Microcontroller Register map
>> + *
>> + * Copyright (C) 2020 BayLibre SAS
>
> Nit: '\n'
Ok
>
>> + * Author(s): Neil Armstrong <narmstrong@baylibre.com>
>> + */
>> +
>> +#ifndef MFD_KHADAS_MCU_H
>> +#define MFD_KHADAS_MCU_H
>> +
>> +#define KHADAS_MCU_PASSWD_VEN_0_REG 0x0 /* RO */
>
> Nit: Can you pad these please?
Ok
>
>> +#define KHADAS_MCU_PASSWD_VEN_1_REG 0x1 /* RO */
>> +#define KHADAS_MCU_PASSWD_VEN_2_REG 0x2 /* RO */
>> +#define KHADAS_MCU_PASSWD_VEN_3_REG 0x3 /* RO */
>> +#define KHADAS_MCU_PASSWD_VEN_4_REG 0x4 /* RO */
>> +#define KHADAS_MCU_PASSWD_VEN_5_REG 0x5 /* RO */
>> +#define KHADAS_MCU_MAC_0_REG 0x6 /* RO */
>> +#define KHADAS_MCU_MAC_1_REG 0x7 /* RO */
>> +#define KHADAS_MCU_MAC_2_REG 0x8 /* RO */
>> +#define KHADAS_MCU_MAC_3_REG 0x9 /* RO */
>> +#define KHADAS_MCU_MAC_4_REG 0xa /* RO */
>> +#define KHADAS_MCU_MAC_5_REG 0xb /* RO */
>> +#define KHADAS_MCU_USID_0_REG 0xc /* RO */
>> +#define KHADAS_MCU_USID_1_REG 0xd /* RO */
>> +#define KHADAS_MCU_USID_2_REG 0xe /* RO */
>> +#define KHADAS_MCU_USID_3_REG 0xf /* RO */
>> +#define KHADAS_MCU_USID_4_REG 0x10 /* RO */
>> +#define KHADAS_MCU_USID_5_REG 0x11 /* RO */
>> +#define KHADAS_MCU_VERSION_0_REG 0x12 /* RO */
>> +#define KHADAS_MCU_VERSION_1_REG 0x13 /* RO */
>> +#define KHADAS_MCU_DEVICE_NO_0_REG 0x14 /* RO */
>> +#define KHADAS_MCU_DEVICE_NO_1_REG 0x15 /* RO */
>> +#define KHADAS_MCU_FACTORY_TEST_REG 0x16 /* R */
>> +#define KHADAS_MCU_BOOT_MODE_REG 0x20 /* RW */
>> +#define KHADAS_MCU_BOOT_EN_WOL_REG 0x21 /* RW */
>> +#define KHADAS_MCU_BOOT_EN_RTC_REG 0x22 /* RW */
>> +#define KHADAS_MCU_BOOT_EN_EXP_REG 0x23 /* RW */
>> +#define KHADAS_MCU_BOOT_EN_IR_REG 0x24 /* RW */
>> +#define KHADAS_MCU_BOOT_EN_DCIN_REG 0x25 /* RW */
>> +#define KHADAS_MCU_BOOT_EN_KEY_REG 0x26 /* RW */
>> +#define KHADAS_MCU_KEY_MODE_REG 0x27 /* RW */
>> +#define KHADAS_MCU_LED_MODE_ON_REG 0x28 /* RW */
>> +#define KHADAS_MCU_LED_MODE_OFF_REG 0x29 /* RW */
>> +#define KHADAS_MCU_SHUTDOWN_NORMAL_REG 0x2c /* RW */
>> +#define KHADAS_MCU_MAC_SWITCH_REG 0x2d /* RW */
>> +#define KHADAS_MCU_MCU_SLEEP_MODE_REG 0x2e /* RW */
>> +#define KHADAS_MCU_IR_CODE1_0_REG 0x2f /* RW */
>> +#define KHADAS_MCU_IR_CODE1_1_REG 0x30 /* RW */
>> +#define KHADAS_MCU_IR_CODE1_2_REG 0x31 /* RW */
>> +#define KHADAS_MCU_IR_CODE1_3_REG 0x32 /* RW */
>> +#define KHADAS_MCU_USB_PCIE_SWITCH_REG 0x33 /* RW */
>> +#define KHADAS_MCU_IR_CODE2_0_REG 0x34 /* RW */
>> +#define KHADAS_MCU_IR_CODE2_1_REG 0x35 /* RW */
>> +#define KHADAS_MCU_IR_CODE2_2_REG 0x36 /* RW */
>> +#define KHADAS_MCU_IR_CODE2_3_REG 0x37 /* RW */
>> +#define KHADAS_MCU_PASSWD_USER_0_REG 0x40 /* RW */
>> +#define KHADAS_MCU_PASSWD_USER_1_REG 0x41 /* RW */
>> +#define KHADAS_MCU_PASSWD_USER_2_REG 0x42 /* RW */
>> +#define KHADAS_MCU_PASSWD_USER_3_REG 0x43 /* RW */
>> +#define KHADAS_MCU_PASSWD_USER_4_REG 0x44 /* RW */
>> +#define KHADAS_MCU_PASSWD_USER_5_REG 0x45 /* RW */
>> +#define KHADAS_MCU_USER_DATA_0_REG 0x46 /* RW 56 bytes */
>> +#define KHADAS_MCU_PWR_OFF_CMD_REG 0x80 /* WO */
>> +#define KHADAS_MCU_PASSWD_START_REG 0x81 /* WO */
>> +#define KHADAS_MCU_CHECK_VEN_PASSWD_REG 0x82 /* WO */
>> +#define KHADAS_MCU_CHECK_USER_PASSWD_REG 0x83 /* WO */
>> +#define KHADAS_MCU_SHUTDOWN_NORMAL_STATUS_REG 0x86 /* RO */
>> +#define KHADAS_MCU_WOL_INIT_START_REG 0x87 /* WO */
>> +#define KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG 0x88 /* WO */
>> +
>> +/* Boards */
>
> I think the names make this superfluous.
Ok
>
>> +enum {
>> + KHADAS_BOARD_VIM1 = 0x1,
>> + KHADAS_BOARD_VIM2,
>> + KHADAS_BOARD_VIM3,
>> + KHADAS_BOARD_EDGE = 0x11,
>> + KHADAS_BOARD_EDGE_V,
>> +};
>> +
>> +/**
>> + * struct khadas_mcu_data - Khadas MCU MFD structure
>
> Doesn't match the struct name>
> Prefer you drop the 'MFD' part.
Ok
>
>> + * @device: device reference used for logs
>> + * @map: register map
>> + */
>> +struct khadas_mcu {
>> + struct device *dev;
>> + struct regmap *map;
>> +};
>> +
>> +#endif /* MFD_KHADAS_MCU_H */
>
Thanks for the review.
Neil
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/6] thermal: add support for the MCU controlled FAN on Khadas boards
2020-05-15 6:41 ` Amit Kucheria
2020-05-15 8:05 ` Neil Armstrong
@ 2020-06-02 8:26 ` Neil Armstrong
1 sibling, 0 replies; 18+ messages in thread
From: Neil Armstrong @ 2020-06-02 8:26 UTC (permalink / raw)
To: Amit Kucheria
Cc: Srini Kandagatla, Zhang Rui, Daniel Lezcano, linux-amlogic,
Linux PM list, lakml, LKML
On 15/05/2020 08:41, Amit Kucheria wrote:
> On Tue, May 12, 2020 at 6:56 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> The new Khadas VIM2 and VIM3 boards controls the cooling fan via the
>> on-board microcontroller.
>>
>> This implements the FAN control as thermal devices and as cell of the Khadas
>> MCU MFD driver.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> drivers/thermal/Kconfig | 10 ++
>> drivers/thermal/Makefile | 1 +
>> drivers/thermal/khadas_mcu_fan.c | 174 +++++++++++++++++++++++++++++++
>> 3 files changed, 185 insertions(+)
>> create mode 100644 drivers/thermal/khadas_mcu_fan.c
>>
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index 91af271e9bb0..72b3960cc5ac 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -490,4 +490,14 @@ config SPRD_THERMAL
>> help
>> Support for the Spreadtrum thermal sensor driver in the Linux thermal
>> framework.
>> +
>> +config KHADAS_MCU_FAN_THERMAL
>> + tristate "Khadas MCU controller FAN cooling support"
>> + depends on OF || COMPILE_TEST
>
> Could you add a depends on the some board/SoC Kconfig option here so
> this doesn't show up for non-Amlogic/non-Khadas boards?
Ok
>
> Looks OK otherwise.
>
>> + select MFD_CORE
>> + select REGMAP
>> + help
>> + If you say yes here you get support for the FAN controlled
>> + by the Microcontroller found on the Khadas VIM boards.
>> +
>> endif
>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>> index 8c8ed7b79915..460428c2122c 100644
>> --- a/drivers/thermal/Makefile
>> +++ b/drivers/thermal/Makefile
>> @@ -60,3 +60,4 @@ obj-$(CONFIG_ZX2967_THERMAL) += zx2967_thermal.o
>> obj-$(CONFIG_UNIPHIER_THERMAL) += uniphier_thermal.o
>> obj-$(CONFIG_AMLOGIC_THERMAL) += amlogic_thermal.o
>> obj-$(CONFIG_SPRD_THERMAL) += sprd_thermal.o
>> +obj-$(CONFIG_KHADAS_MCU_FAN_THERMAL) += khadas_mcu_fan.o
>> diff --git a/drivers/thermal/khadas_mcu_fan.c b/drivers/thermal/khadas_mcu_fan.c
>> new file mode 100644
>> index 000000000000..044d4aba8be2
>> --- /dev/null
>> +++ b/drivers/thermal/khadas_mcu_fan.c
>> @@ -0,0 +1,174 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Khadas MCU Controlled FAN driver
>> + *
>> + * Copyright (C) 2020 BayLibre SAS
>> + * Author(s): Neil Armstrong <narmstrong@baylibre.com>
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/mfd/khadas-mcu.h>
>> +#include <linux/regmap.h>
>> +#include <linux/sysfs.h>
>> +#include <linux/thermal.h>
>> +
>> +#define MAX_LEVEL 3
>> +
>> +struct khadas_mcu_fan_ctx {
>> + struct khadas_mcu *mcu;
>> + unsigned int level;
>> + struct thermal_cooling_device *cdev;
>> +};
>> +
>> +static int khadas_mcu_fan_set_level(struct khadas_mcu_fan_ctx *ctx,
>> + unsigned int level)
>> +{
>> + int ret;
>> +
>> + ret = regmap_write(ctx->mcu->map, KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG,
>> + level);
>> + if (ret)
>> + return ret;
>> +
>> + ctx->level = level;
>> +
>> + return 0;
>> +}
>> +
>> +static int khadas_mcu_fan_get_max_state(struct thermal_cooling_device *cdev,
>> + unsigned long *state)
>> +{
>> + struct khadas_mcu_fan_ctx *ctx = cdev->devdata;
>> +
>> + if (!ctx)
>> + return -EINVAL;
>> +
>> + *state = MAX_LEVEL;
>> +
>> + return 0;
>> +}
>> +
>> +static int khadas_mcu_fan_get_cur_state(struct thermal_cooling_device *cdev,
>> + unsigned long *state)
>> +{
>> + struct khadas_mcu_fan_ctx *ctx = cdev->devdata;
>> +
>> + if (!ctx)
>> + return -EINVAL;
>> +
>> + *state = ctx->level;
>> +
>> + return 0;
>> +}
>> +
>> +static int
>> +khadas_mcu_fan_set_cur_state(struct thermal_cooling_device *cdev,
>> + unsigned long state)
>> +{
>> + struct khadas_mcu_fan_ctx *ctx = cdev->devdata;
>> +
>> + if (!ctx || (state > MAX_LEVEL))
>> + return -EINVAL;
>> +
>> + if (state == ctx->level)
>> + return 0;
>> +
>> + return khadas_mcu_fan_set_level(ctx, state);
>> +}
>> +
>> +static const struct thermal_cooling_device_ops khadas_mcu_fan_cooling_ops = {
>> + .get_max_state = khadas_mcu_fan_get_max_state,
>> + .get_cur_state = khadas_mcu_fan_get_cur_state,
>> + .set_cur_state = khadas_mcu_fan_set_cur_state,
>> +};
>> +
>> +static int khadas_mcu_fan_probe(struct platform_device *pdev)
>> +{
>> + struct khadas_mcu *mcu = dev_get_drvdata(pdev->dev.parent);
>> + struct thermal_cooling_device *cdev;
>> + struct device *dev = &pdev->dev;
>> + struct khadas_mcu_fan_ctx *ctx;
>> + int ret;
>> +
>> + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>> + if (!ctx)
>> + return -ENOMEM;
>> + ctx->mcu = mcu;
>> + platform_set_drvdata(pdev, ctx);
>> +
>> + cdev = devm_thermal_of_cooling_device_register(dev->parent,
>> + dev->parent->of_node, "khadas-mcu-fan", ctx,
>> + &khadas_mcu_fan_cooling_ops);
>> + if (IS_ERR(cdev)) {
>> + ret = PTR_ERR(cdev);
>> + dev_err(dev,
>> + "Failed to register khadas-mcu-fan as cooling device: %d\n",
>> + ret);
>> + return ret;
>> + }
>> + ctx->cdev = cdev;
>> + thermal_cdev_update(cdev);
>> +
>> + return 0;
>> +}
>> +
>> +static int khadas_mcu_fan_disable(struct device *dev)
>> +{
>> + struct khadas_mcu_fan_ctx *ctx = dev_get_drvdata(dev);
>> + unsigned int level_save = ctx->level;
>> + int ret;
>> +
>> + ret = khadas_mcu_fan_set_level(ctx, 0);
>> + if (ret)
>> + return ret;
>> +
>> + ctx->level = level_save;
>> +
>> + return 0;
>> +}
>> +
>> +static void khadas_mcu_fan_shutdown(struct platform_device *pdev)
>> +{
>> + khadas_mcu_fan_disable(&pdev->dev);
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int khadas_mcu_fan_suspend(struct device *dev)
>> +{
>> + return khadas_mcu_fan_disable(dev);
>> +}
>> +
>> +static int khadas_mcu_fan_resume(struct device *dev)
>> +{
>> + struct khadas_mcu_fan_ctx *ctx = dev_get_drvdata(dev);
>> +
>> + return khadas_mcu_fan_set_level(ctx, ctx->level);
>> +}
>> +#endif
>> +
>> +static SIMPLE_DEV_PM_OPS(khadas_mcu_fan_pm, khadas_mcu_fan_suspend,
>> + khadas_mcu_fan_resume);
>> +
>> +static const struct platform_device_id khadas_mcu_fan_id_table[] = {
>> + { .name = "khadas-mcu-fan-ctrl", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(platform, khadas_mcu_fan_id_table);
>> +
>> +static struct platform_driver khadas_mcu_fan_driver = {
>> + .probe = khadas_mcu_fan_probe,
>> + .shutdown = khadas_mcu_fan_shutdown,
>> + .driver = {
>> + .name = "khadas-mcu-fan-ctrl",
>> + .pm = &khadas_mcu_fan_pm,
>> + },
>> + .id_table = khadas_mcu_fan_id_table,
>> +};
>> +
>> +module_platform_driver(khadas_mcu_fan_driver);
>> +
>> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
>> +MODULE_DESCRIPTION("Khadas MCU FAN driver");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.22.0
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/6] nvmem: add support for the Khadas MCU Programmable User Memory
2020-05-15 10:55 ` Srinivas Kandagatla
@ 2020-06-02 8:29 ` Neil Armstrong
0 siblings, 0 replies; 18+ messages in thread
From: Neil Armstrong @ 2020-06-02 8:29 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: linux-amlogic, linux-pm, linux-arm-kernel, linux-kernel
On 15/05/2020 12:55, Srinivas Kandagatla wrote:
>
>
> On 13/05/2020 13:33, Neil Armstrong wrote:
>> On 13/05/2020 12:34, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 12/05/2020 14:26, Neil Armstrong wrote:
>>>> The new Khadas VIM2, VIM3 and Edge boards embeds an on-board microcontroller
>>>> offering a 56bytes User Programmable NVMEM array.
>>>>
>>>> This array needs a password to be writable, thus a password sysfs file
>>>> has been added on the device node to unlock the NVMEM.
>>>
>>> Is this one time programmable? Or does it need to be unlocked for every read/write?
>>
>> It can be rewritten, and needs the password to read & write.
>>
>>>
>>> How can you make sure that the memory is unlocked before making attempt to read/write?
>>
>> The only way to know it's unlock is reading back the password when unlocked.
>
>
> Current code has no such checks for every read/write?
> and looks very disconnected w.r.t to password and read/writes.
>
> If user attempts to read/write he will not be aware that he should program the password first!
>
> Also if the password is static or un-changed then why not just statically program this from the driver rather than providing sysfs file?
The passworsd can be changed, how should this be taken in account ?
>
> I dont see how kernel nvmem read/write interface can make sure that memory is unlocked?
Not sure how to be sure, if locked all the user memory and password is read as 0xff
>
> Who is the real consumer for this provider?
well, it's more user-space, indeed without the in-kernel password unlock, kernel won't be able
to make great use of the data.
I'll drop for next serie until we have a clearer view.
Neil
>
> --srini
>
>
>>
>>>
>>>>
>>>> The default 6bytes password id: "Khadas"
>>>>
>>>> This implements the user NVMEM devices as cell of the Khadas MCU MFD driver.
>>>>
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>> ---
>>>> drivers/nvmem/Kconfig | 8 ++
>>>> drivers/nvmem/Makefile | 2 +
>>>> drivers/nvmem/khadas-mcu-user-mem.c | 128 ++++++++++++++++++++++++++++
>>>> 3 files changed, 138 insertions(+)
>>>> create mode 100644 drivers/nvmem/khadas-mcu-user-mem.c
>>>>
>>>> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
>>>> index d7b7f6d688e7..92cd4f6aa931 100644
>>>> --- a/drivers/nvmem/Kconfig
>>>> +++ b/drivers/nvmem/Kconfig
>>>> @@ -67,6 +67,14 @@ config JZ4780_EFUSE
>>>> To compile this driver as a module, choose M here: the module
>>>> will be called nvmem_jz4780_efuse.
>>>> +config NVMEM_KHADAS_MCU_USER_MEM
>>>> + tristate "Khadas MCU User programmable memory support"
>>>> + depends on MFD_KHADAS_MCU
>>>> + depends on REGMAP
>>>> + help
>>>> + This is a driver for the MCU User programmable memory
>>>> + available on the Khadas VIM and Edge boards.
>>>> +
>>>> config NVMEM_LPC18XX_EEPROM
>>>> tristate "NXP LPC18XX EEPROM Memory Support"
>>>> depends on ARCH_LPC18XX || COMPILE_TEST
>>>> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
>>>> index a7c377218341..0516a309542d 100644
>>>> --- a/drivers/nvmem/Makefile
>>>> +++ b/drivers/nvmem/Makefile
>>>> @@ -17,6 +17,8 @@ obj-$(CONFIG_NVMEM_IMX_OCOTP_SCU) += nvmem-imx-ocotp-scu.o
>>>> nvmem-imx-ocotp-scu-y := imx-ocotp-scu.o
>>>> obj-$(CONFIG_JZ4780_EFUSE) += nvmem_jz4780_efuse.o
>>>> nvmem_jz4780_efuse-y := jz4780-efuse.o
>>>> +obj-$(CONFIG_NVMEM_KHADAS_MCU_USER_MEM) += nvmem-khadas-mcu-user-mem.o
>>>> +nvmem-khadas-mcu-user-mem-y := khadas-mcu-user-mem.o
>>>> obj-$(CONFIG_NVMEM_LPC18XX_EEPROM) += nvmem_lpc18xx_eeprom.o
>>>> nvmem_lpc18xx_eeprom-y := lpc18xx_eeprom.o
>>>> obj-$(CONFIG_NVMEM_LPC18XX_OTP) += nvmem_lpc18xx_otp.o
>>>> diff --git a/drivers/nvmem/khadas-mcu-user-mem.c b/drivers/nvmem/khadas-mcu-user-mem.c
>>>> new file mode 100644
>>>> index 000000000000..a1d5ae9a030c
>>>> --- /dev/null
>>>> +++ b/drivers/nvmem/khadas-mcu-user-mem.c
>>>> @@ -0,0 +1,128 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Driver for Khadas MCU User programmable Memory
>>>> + *
>>>> + * Copyright (C) 2020 BayLibre SAS
>>>> + * Author(s): Neil Armstrong <narmstrong@baylibre.com>
>>>> + */
>>>> +
>>>> +#include <linux/clk.h>
>>>
>>> Why do we need this header?
>>
>> Will drop
>>
>>>
>>>> +#include <linux/module.h>
>>>> +#include <linux/nvmem-provider.h>
>>>> +#include <linux/mfd/khadas-mcu.h>
>>>> +#include <linux/regmap.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/platform_device.h>
>>>> +
>>>> +static int khadas_mcu_user_mem_read(void *context, unsigned int offset,
>>>> + void *val, size_t bytes)
>>>> +{
>>>> + struct khadas_mcu *khadas_mcu = context;
>>>> +
>>>> + return regmap_bulk_read(khadas_mcu->map,
>>>> + KHADAS_MCU_USER_DATA_0_REG + offset,
>>>> + val, bytes);
>>>> +}
>>>> +
>>>> +static int khadas_mcu_user_mem_write(void *context, unsigned int offset,
>>>> + void *val, size_t bytes)
>>>> +{
>>>> + struct khadas_mcu *khadas_mcu = context;
>>>> +
>>>> + return regmap_bulk_write(khadas_mcu->map,
>>>> + KHADAS_MCU_USER_DATA_0_REG + offset,
>>>> + val, bytes);
>>>> +}
>>>> +
>>>> +static ssize_t password_store(struct device *dev, struct device_attribute *attr,
>>>> + const char *buf, size_t count)
>>>> +{
>>>> + struct khadas_mcu *khadas_mcu = dev_get_drvdata(dev);
>>>> + int i, ret;
>>>> +
>>>> + if (count < 6)
>>>> + return -EINVAL;
>>>
>>> Possibly this magic 6 value needs proper define. An additional check here for > 6 would be better as well.
>>
>> Ok
>>
>>>
>>>> +
>>>> + ret = regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 1);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + for (i = 0 ; i < 6 ; ++i) {
>>>> + ret = regmap_write(khadas_mcu->map,
>>>> + KHADAS_MCU_CHECK_USER_PASSWD_REG,
>>>> + buf[i]);
>>>> + if (ret)
>>>> + goto out;
>>>> + }
>>>> +
>>>> + ret = regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 0);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + return count;
>>>> +out:
>>>> + regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 0);
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> +static DEVICE_ATTR_WO(password);
>>>> +
>>>> +static struct attribute *khadas_mcu_user_mem_sysfs_attributes[] = {
>>>> + &dev_attr_password.attr,
>>>> + NULL,
>>>> +};
>>>> +
>>>> +static const struct attribute_group khadas_mcu_user_mem_sysfs_attr_group = {
>>>> + .attrs = khadas_mcu_user_mem_sysfs_attributes,
>>>> +};
>>>> +
>>>> +static int khadas_mcu_user_mem_probe(struct platform_device *pdev)
>>>> +{
>>>> + struct khadas_mcu *khadas_mcu = dev_get_drvdata(pdev->dev.parent);
>>>
>>> You could probably get away with dependency of this structure and the header itself by directly getting hold of regmap from parent device.
>>
>> Ok
>>
>>>
>>>
>>>> + struct device *dev = &pdev->dev;
>>>> + struct nvmem_device *nvmem;
>>>> + struct nvmem_config *econfig;
>>>> +
>>>> + econfig = devm_kzalloc(dev, sizeof(*econfig), GFP_KERNEL);
>>>> + if (!econfig)
>>>> + return -ENOMEM;
>>>> +
>>>> + econfig->dev = pdev->dev.parent;
>>>> + econfig->name = dev_name(pdev->dev.parent);
>>>> + econfig->stride = 1;
>>>> + econfig->word_size = 1;
>>>> + econfig->reg_read = khadas_mcu_user_mem_read;
>>>> + econfig->reg_write = khadas_mcu_user_mem_write;
>>>> + econfig->size = 56;
>>> define 56 to make it more readable!
>>
>> Ok
>>
>>>
>>>> + econfig->priv = khadas_mcu;
>>>> +
>>>> + platform_set_drvdata(pdev, khadas_mcu);
>>>> +
>>>> + nvmem = devm_nvmem_register(&pdev->dev, econfig);
>>>> + if (IS_ERR(nvmem))
>>>> + return PTR_ERR(nvmem);
>>>> +
>>>> + return sysfs_create_group(&pdev->dev.kobj,
>>>> + &khadas_mcu_user_mem_sysfs_attr_group);
>>>> +}
>>>> +
>>>> +static const struct platform_device_id khadas_mcu_user_mem_id_table[] = {
>>>> + { .name = "khadas-mcu-user-mem", },
>>>> + {},
>>>> +};
>>>> +MODULE_DEVICE_TABLE(platform, khadas_mcu_user_mem_id_table);
>>>> +
>>>> +static struct platform_driver khadas_mcu_user_mem_driver = {
>>>> + .probe = khadas_mcu_user_mem_probe,
>>>> + .driver = {
>>>> + .name = "khadas-mcu-user-mem",
>>>> + },
>>>> + .id_table = khadas_mcu_user_mem_id_table,
>>>> +};
>>>> +
>>>> +module_platform_driver(khadas_mcu_user_mem_driver);
>>>> +
>>>> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
>>>> +MODULE_DESCRIPTION("Khadas MCU User MEM driver");
>>>> +MODULE_LICENSE("GPL v2");
>>>>
>>
>> Thanks for the review.
>>
>> Neil
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/6] mfd: add support for the Khadas System control Microcontroller
2020-06-02 8:26 ` Neil Armstrong
@ 2020-06-02 8:33 ` Lee Jones
0 siblings, 0 replies; 18+ messages in thread
From: Lee Jones @ 2020-06-02 8:33 UTC (permalink / raw)
To: Neil Armstrong; +Cc: linux-amlogic, linux-pm, linux-arm-kernel, linux-kernel
On Tue, 02 Jun 2020, Neil Armstrong wrote:
> On 20/05/2020 11:01, Lee Jones wrote:
> > On Tue, 12 May 2020, Neil Armstrong wrote:
> >
> >> This Microcontroller is present on the Khadas VIM1, VIM2, VIM3 and Edge
> >> boards.
> >>
> >> It has multiple boot control features like password check, power-on
> >> options, power-off control and system FAN control on recent boards.
> >>
> >> This implements a very basic MFD driver with the fan control and User
> >> NVMEM cells.
> >>
> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >> ---
> >> drivers/mfd/Kconfig | 14 ++++
> >> drivers/mfd/Makefile | 1 +
> >> drivers/mfd/khadas-mcu.c | 143 +++++++++++++++++++++++++++++++++
> >> include/linux/mfd/khadas-mcu.h | 91 +++++++++++++++++++++
> >> 4 files changed, 249 insertions(+)
> >> create mode 100644 drivers/mfd/khadas-mcu.c
> >> create mode 100644 include/linux/mfd/khadas-mcu.h
> >>
> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> >> index 0a59249198d3..b95091397052 100644
> >> --- a/drivers/mfd/Kconfig
> >> +++ b/drivers/mfd/Kconfig
> >> @@ -2003,6 +2003,20 @@ config MFD_WCD934X
> >> This driver provides common support WCD934x audio codec and its
> >> associated Pin Controller, Soundwire Controller and Audio codec.
> >>
> >> +config MFD_KHADAS_MCU
> >> + tristate "Support for Khadas System control Microcontroller"
> >> + depends on I2C
> >> + depends on OF || COMPILE_TEST
> >> + select MFD_CORE
> >> + select REGMAP_I2C
> >> + help
> >> + Support for the Khadas System control Microcontroller interface present
> >> + on their VIM and Edge boards.
> >> +
> >> + This driver provides common support for accessing the device,
> >> + additional drivers must be enabled in order to use the functionality
> >> + of the device.
> >
> > It would be good to describe the device here.
>
> Ok
If you agree with all review comments, there really is no need to
reply. It's a waste of your time and anyone else who cares enough to
search through looking for replies (as I just did).
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-06-02 8:33 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-12 13:26 [PATCH v2 0/6] mfd: Add support for Khadas Microcontroller Neil Armstrong
2020-05-12 13:26 ` [PATCH v2 1/6] dt-bindings: mfd: add Khadas Microcontroller bindings Neil Armstrong
2020-05-15 6:42 ` Amit Kucheria
2020-05-12 13:26 ` [PATCH v2 2/6] mfd: add support for the Khadas System control Microcontroller Neil Armstrong
2020-05-20 9:01 ` Lee Jones
2020-06-02 8:26 ` Neil Armstrong
2020-06-02 8:33 ` Lee Jones
2020-05-12 13:26 ` [PATCH v2 3/6] thermal: add support for the MCU controlled FAN on Khadas boards Neil Armstrong
2020-05-15 6:41 ` Amit Kucheria
2020-05-15 8:05 ` Neil Armstrong
2020-06-02 8:26 ` Neil Armstrong
2020-05-12 13:26 ` [PATCH v2 4/6] nvmem: add support for the Khadas MCU Programmable User Memory Neil Armstrong
2020-05-13 10:34 ` Srinivas Kandagatla
2020-05-13 12:33 ` Neil Armstrong
2020-05-15 10:55 ` Srinivas Kandagatla
2020-06-02 8:29 ` Neil Armstrong
2020-05-12 13:26 ` [PATCH v2 5/6] MAINTAINERS: add myself as maintainer for Khadas MCU drivers Neil Armstrong
2020-05-12 13:26 ` [PATCH v2 6/6] arm64: dts: meson-khadas-vim3: add Khadas MCU nodes Neil Armstrong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).