All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Add iop driver for Sunplus SP7021
@ 2021-12-09  8:58 Tony Huang
  2021-12-09  8:58 ` [PATCH v3 1/2] dt-binding: misc: Add iop yaml file " Tony Huang
  2021-12-09  8:58 ` [PATCH v3 2/2] misc: Add iop driver " Tony Huang
  0 siblings, 2 replies; 14+ messages in thread
From: Tony Huang @ 2021-12-09  8:58 UTC (permalink / raw)
  To: robh+dt, devicetree, linux-kernel, derek.kiernan, dragan.cvetic,
	arnd, gregkh
  Cc: wells.lu, tony.huang, Tony Huang

Add iop driver for Sunplus SP7021 SOC

This is a patch series for iop driver for Sunplus SP7021 SOC.

Sunplus SP7021 is an ARM Cortex A7 (4 cores) based SoC. It integrates
many peripherals (ex: UART, I2C, SPI, SDIO, eMMC, USB, SD card and
etc.) into a single chip. It is designed for industrial control.

Refer to:
https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
https://tibbo.com/store/plus1.html

Tony Huang (2):
  dt-binding: misc: Add iop yaml file for Sunplus SP7021
  misc: Add iop driver for Sunplus SP7021

 .../devicetree/bindings/misc/sunplus-iop.yaml      |  61 ++++
 MAINTAINERS                                        |   6 +
 drivers/misc/Kconfig                               |  12 +
 drivers/misc/Makefile                              |   1 +
 drivers/misc/sunplus_iop.c                         | 395 +++++++++++++++++++++
 5 files changed, 475 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/sunplus-iop.yaml
 create mode 100644 drivers/misc/sunplus_iop.c

-- 
2.7.4


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 1/2] dt-binding: misc: Add iop yaml file for Sunplus SP7021
  2021-12-09  8:58 [PATCH v3 0/2] Add iop driver for Sunplus SP7021 Tony Huang
@ 2021-12-09  8:58 ` Tony Huang
  2021-12-14 19:32   ` Rob Herring
  2021-12-09  8:58 ` [PATCH v3 2/2] misc: Add iop driver " Tony Huang
  1 sibling, 1 reply; 14+ messages in thread
From: Tony Huang @ 2021-12-09  8:58 UTC (permalink / raw)
  To: robh+dt, devicetree, linux-kernel, derek.kiernan, dragan.cvetic,
	arnd, gregkh
  Cc: wells.lu, tony.huang, Tony Huang

Add iop yaml file for Sunplus SP7021

Signed-off-by: Tony Huang <tonyhuang.sunplus@gmail.com>
---
Changes in v3:
 - No change.

 .../devicetree/bindings/misc/sunplus-iop.yaml      | 61 ++++++++++++++++++++++
 MAINTAINERS                                        |  5 ++
 2 files changed, 66 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/sunplus-iop.yaml

diff --git a/Documentation/devicetree/bindings/misc/sunplus-iop.yaml b/Documentation/devicetree/bindings/misc/sunplus-iop.yaml
new file mode 100644
index 0000000..07cdbde
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/sunplus-iop.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) Sunplus Ltd. Co. 2021
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/sunplus-iop.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sunplus IOP(8051) controller
+
+maintainers:
+  - Tony Huang <tonyhuang.sunplus@gmail.com>
+
+description: |
+  Processor for I/O control, RTC wake-up procedure management,
+  and cooperation with CPU&PMC in power management.
+
+properties:
+  compatible:
+    enum:
+      - sunplus,sp7021-iop
+
+  reg:
+    items:
+      - description: IOP registers regions
+      - description: PMC registers regions
+      - description: MOON0 registers regions
+
+  reg-names:
+    items:
+      - const: iop
+      - const: iop_pmc
+      - const: moon0
+
+  interrupts:
+    maxItems: 2
+
+  memory-region:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - memory-region
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    iop: iop@9c000400 {
+        compatible = "sunplus,sp7021-iop";
+        reg = <0x9c000400 0x80>, <0x9c003100 0x80>, <0x9c000000 0x80>;
+        reg-names = "iop", "iop_pmc", "moon0";
+        interrupt-parent = <&intc>;
+        interrupts = <41 IRQ_TYPE_LEVEL_HIGH>, <42 IRQ_TYPE_LEVEL_HIGH>;
+        memory-region = <&iop_reserve>;
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 3b79fd4..071b5e6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17945,6 +17945,11 @@ L:	netdev@vger.kernel.org
 S:	Maintained
 F:	drivers/net/ethernet/dlink/sundance.c
 
+SUNPLUS IOP DRIVER
+M:	Tony Huang <tonyhuang.sunplus@gmail.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/misc/sunplu-iop.yaml
+
 SUPERH
 M:	Yoshinori Sato <ysato@users.sourceforge.jp>
 M:	Rich Felker <dalias@libc.org>
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 2/2] misc: Add iop driver for Sunplus SP7021
  2021-12-09  8:58 [PATCH v3 0/2] Add iop driver for Sunplus SP7021 Tony Huang
  2021-12-09  8:58 ` [PATCH v3 1/2] dt-binding: misc: Add iop yaml file " Tony Huang
@ 2021-12-09  8:58 ` Tony Huang
  2021-12-09  9:50   ` Arnd Bergmann
  2021-12-09 10:10   ` Greg KH
  1 sibling, 2 replies; 14+ messages in thread
From: Tony Huang @ 2021-12-09  8:58 UTC (permalink / raw)
  To: robh+dt, devicetree, linux-kernel, derek.kiernan, dragan.cvetic,
	arnd, gregkh
  Cc: wells.lu, tony.huang, Tony Huang

IOP (IO Processor) embedded inside SP7021 which is used as
Processor for I/O control, RTC wake-up and cooperation with
CPU & PMC in power management purpose.
The IOP core is DQ8051, so also named IOP8051,
it supports dedicated JTAG debug pins which share with SP7021.
In standby mode operation, the power spec reach 400uA.

Signed-off-by: Tony Huang <tonyhuang.sunplus@gmail.com>
---
Changes in v3:
 - Addressed comments from Arnd Bergmann.
 - Addressed comments from Greg KH.
 - Addressed comments from kernel test robot.

 MAINTAINERS                |   1 +
 drivers/misc/Kconfig       |  12 ++
 drivers/misc/Makefile      |   1 +
 drivers/misc/sunplus_iop.c | 395 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 409 insertions(+)
 create mode 100644 drivers/misc/sunplus_iop.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 071b5e6..a763088 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17949,6 +17949,7 @@ SUNPLUS IOP DRIVER
 M:	Tony Huang <tonyhuang.sunplus@gmail.com>
 S:	Maintained
 F:	Documentation/devicetree/bindings/misc/sunplu-iop.yaml
+F:	drivers/misc/sunplus_iop.c
 
 SUPERH
 M:	Yoshinori Sato <ysato@users.sourceforge.jp>
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 0f5a49f..f19533b 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -470,6 +470,18 @@ config HISI_HIKEY_USB
 	  switching between the dual-role USB-C port and the USB-A host ports
 	  using only one USB controller.
 
+config SUNPLUS_IOP
+	tristate "Sunplus IOP support"
+	default ARCH_SUNPLUS
+	help
+	  Sunplus I/O processor (8051) driver.
+	  Processor for I/O control, RTC wake-up proceduce management,
+	  and cooperation with CPU&PMC in power management.
+	  Need Install DQ8051, The DQ8051 bin file generated by keil C.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ad525x_dpot.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index a086197..eafeab6 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_DW_XDATA_PCIE)	+= dw-xdata-pcie.o
 obj-$(CONFIG_PCI_ENDPOINT_TEST)	+= pci_endpoint_test.o
 obj-$(CONFIG_OCXL)		+= ocxl/
 obj-$(CONFIG_BCM_VK)		+= bcm-vk/
+obj-$(CONFIG_SUNPLUS_IOP)	+= sunplus_iop.o
 obj-y				+= cardreader/
 obj-$(CONFIG_PVPANIC)   	+= pvpanic/
 obj-$(CONFIG_HABANA_AI)		+= habanalabs/
diff --git a/drivers/misc/sunplus_iop.c b/drivers/misc/sunplus_iop.c
new file mode 100644
index 0000000..92ec0e9
--- /dev/null
+++ b/drivers/misc/sunplus_iop.c
@@ -0,0 +1,395 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/module.h>
+#include <linux/miscdevice.h>
+#include <linux/of_platform.h>
+#include <linux/firmware.h>
+#include <linux/dma-mapping.h>
+#include <linux/of_address.h>
+#include <linux/delay.h>
+
+enum IOP_Status_e {
+	IOP_SUCCESS,                /* successful */
+	IOP_ERR_IOP_BUSY,           /* IOP is busy */
+};
+
+struct regs_moon0 {
+	u32 stamp;         /* 00 */
+	u32 clken[10];     /* 01~10 */
+	u32 gclken[10];    /* 11~20 */
+	u32 reset[10];     /* 21~30 */
+	u32 sfg_cfg_mode;  /* 31 */
+};
+
+struct regs_iop {
+	u32 iop_control;/* 00 */
+	u32 iop_reg1;/* 01 */
+	u32 iop_bp;/* 02 */
+	u32 iop_regsel;/* 03 */
+	u32 iop_regout;/* 04 */
+	u32 iop_reg5;/* 05 */
+	u32 iop_resume_pcl;/* 06 */
+	u32 iop_resume_pch;/* 07 */
+	u32 iop_data0;/* 08 */
+	u32 iop_data1;/* 09 */
+	u32 iop_data2;/* 10 */
+	u32 iop_data3;/* 11 */
+	u32 iop_data4;/* 12 */
+	u32 iop_data5;/* 13 */
+	u32 iop_data6;/* 14 */
+	u32 iop_data7;/* 15 */
+	u32 iop_data8;/* 16 */
+	u32 iop_data9;/* 17 */
+	u32 iop_data10;/* 18 */
+	u32 iop_data11;/* 19 */
+	u32 iop_base_adr_l;/* 20 */
+	u32 iop_base_adr_h;/* 21 */
+	u32 Memory_bridge_control;/* 22 */
+	u32 iop_regmap_adr_l;/* 23 */
+	u32 iop_regmap_adr_h;/* 24 */
+	u32 iop_direct_adr;/* 25*/
+	u32 reserved[6];/* 26~31 */
+};
+
+struct regs_iop_pmc {
+	u32 PMC_TIMER;/* 00 */
+	u32 PMC_CTRL;/* 01 */
+	u32 XTAL27M_PASSWORD_I;/* 02 */
+	u32 XTAL27M_PASSWORD_II;/* 03 */
+	u32 XTAL32K_PASSWORD_I;/* 04 */
+	u32 XTAL32K_PASSWORD_II;/* 05 */
+	u32 CLK27M_PASSWORD_I;/* 06 */
+	u32 CLK27M_PASSWORD_II;/* 07 */
+	u32 PMC_TIMER2;/* 08 */
+	u32 reserved[23];/* 9~31 */
+};
+
+#define NORMAL_CODE_MAX_SIZE 0X1000
+#define STANDBY_CODE_MAX_SIZE 0x4000
+struct sp_iop {
+	struct miscdevice dev;			// iop device
+	struct mutex write_lock;
+	void __iomem *iop_regs;
+	void __iomem *pmc_regs;
+	void __iomem *moon0_regs;
+	int irq;
+	unsigned char iop_normal_code[NORMAL_CODE_MAX_SIZE];
+	unsigned char iop_standby_code[STANDBY_CODE_MAX_SIZE];
+	resource_size_t iop_mem_start;
+	resource_size_t iop_mem_size;
+};
+
+static void sp_iop_normal_mode(struct sp_iop *iop)
+{
+	struct regs_iop *p_iop_reg = (struct regs_iop *)iop->iop_regs;
+	struct regs_moon0 *p_moon0_reg = (struct regs_moon0 *)iop->moon0_regs;
+	void __iomem *iop_kernel_base;
+	unsigned int reg;
+
+	iop_kernel_base = ioremap(iop->iop_mem_start, NORMAL_CODE_MAX_SIZE);
+	memset(iop_kernel_base, 0, NORMAL_CODE_MAX_SIZE);
+	memcpy(iop_kernel_base, iop->iop_normal_code, NORMAL_CODE_MAX_SIZE);
+
+	writel(0x00100010, &p_moon0_reg->clken[0]);
+
+	reg = readl(&p_iop_reg->iop_control);
+	reg |= 0x01;
+	writel(reg, &p_iop_reg->iop_control);
+
+	reg = readl(&p_iop_reg->iop_control);
+	reg &= ~(0x8000);
+	writel(reg, &p_iop_reg->iop_control);
+
+	reg = readl(&p_iop_reg->iop_control);
+	reg |= 0x0200;//disable watchdog event reset IOP
+	writel(reg, &p_iop_reg->iop_control);
+
+	reg = (iop->iop_mem_start & 0xFFFF);
+	writel(reg, &p_iop_reg->iop_base_adr_l);
+	reg	= (iop->iop_mem_start >> 16);
+	writel(reg, &p_iop_reg->iop_base_adr_h);
+
+	reg = readl(&p_iop_reg->iop_control);
+	reg &= ~(0x01);
+	writel(reg, &p_iop_reg->iop_control);
+}
+
+static void sp_iop_standby_mode(struct sp_iop *iop)
+{
+	struct regs_iop *p_iop_reg = (struct regs_iop *)iop->iop_regs;
+	struct regs_moon0 *p_moon0_reg = (struct regs_moon0 *)iop->moon0_regs;
+	void __iomem *iop_kernel_base;
+	unsigned long reg;
+
+	iop_kernel_base = ioremap(iop->iop_mem_start, STANDBY_CODE_MAX_SIZE);
+	memset(iop_kernel_base, 0, STANDBY_CODE_MAX_SIZE);
+	memcpy(iop_kernel_base, iop->iop_standby_code, STANDBY_CODE_MAX_SIZE);
+
+	writel(0x00100010, &p_moon0_reg->clken[0]);
+
+	reg = readl(&p_iop_reg->iop_control);
+	reg |= 0x01;
+	writel(reg, &p_iop_reg->iop_control);
+
+	reg = readl(&p_iop_reg->iop_control);
+	reg &= ~(0x8000);
+	writel(reg, &p_iop_reg->iop_control);
+
+	reg = readl(&p_iop_reg->iop_control);
+	reg |= 0x0200;//disable watchdog event reset IOP
+	writel(reg, &p_iop_reg->iop_control);
+
+	reg = (iop->iop_mem_start & 0xFFFF);
+	writel(reg, &p_iop_reg->iop_base_adr_l);
+	reg = (iop->iop_mem_start >> 16);
+	writel(reg, &p_iop_reg->iop_base_adr_h);
+
+	reg = readl(&p_iop_reg->iop_control);
+	reg &= ~(0x01);
+	writel(reg, &p_iop_reg->iop_control);
+}
+
+#define IOP_READY	0x4
+#define RISC_READY	0x8
+static void sp_iop_shutdown(struct sp_iop *iop)
+{
+	struct regs_iop *p_iop_reg = (struct regs_iop *)iop->iop_regs;
+	struct regs_moon0 *p_moon0_reg = (struct regs_moon0 *)iop->moon0_regs;
+	struct regs_iop_pmc *p_iop_pmc_reg = (struct regs_iop_pmc *)iop->pmc_regs;
+	unsigned int reg;
+
+	writel(0x00100010, &p_moon0_reg->clken[0]);
+
+	reg = readl(&p_iop_reg->iop_control);
+	reg &= ~(0x8000);
+	writel(reg, &p_iop_reg->iop_control);
+
+	reg = readl(&p_iop_reg->iop_control);
+	reg |= 0x1;
+	writel(reg, &p_iop_reg->iop_control);
+
+	//PMC set
+	writel(0x00010001, &p_iop_pmc_reg->PMC_TIMER);
+	reg = readl(&p_iop_pmc_reg->PMC_CTRL);
+	reg |= 0x23;// disable system reset PMC, enalbe power down 27M, enable gating 27M
+	writel(reg, &p_iop_pmc_reg->PMC_CTRL);
+
+	writel(0x55aa00ff, &p_iop_pmc_reg->XTAL27M_PASSWORD_I);
+	writel(0x00ff55aa, &p_iop_pmc_reg->XTAL27M_PASSWORD_II);
+	writel(0xaa00ff55, &p_iop_pmc_reg->XTAL32K_PASSWORD_I);
+	writel(0xff55aa00, &p_iop_pmc_reg->XTAL32K_PASSWORD_II);
+	writel(0xaaff0055, &p_iop_pmc_reg->CLK27M_PASSWORD_I);
+	writel(0x5500aaff, &p_iop_pmc_reg->CLK27M_PASSWORD_II);
+	writel(0x01000100, &p_iop_pmc_reg->PMC_TIMER2);
+
+	//IOP Hardware IP reset
+	reg = readl(&p_moon0_reg->reset[0]);
+	reg |= 0x10;
+	writel(reg, (&p_moon0_reg->reset[0]));
+	reg &= ~(0x10);
+	writel(reg, (&p_moon0_reg->reset[0]));
+
+	writel(0x00ff0085, (iop->moon0_regs + 32*4*1 + 4*1));
+
+	reg = readl(iop->moon0_regs + 32*4*1 + 4*2);
+	reg |= 0x08000800;
+	writel(reg, (iop->moon0_regs + 32*4*1 + 4*2));
+
+	reg = readl(&p_iop_reg->iop_control);
+	reg |= 0x0200;//disable watchdog event reset IOP
+	writel(reg, &p_iop_reg->iop_control);
+
+	reg = (iop->iop_mem_start & 0xFFFF);
+	writel(reg, &p_iop_reg->iop_base_adr_l);
+	reg = (iop->iop_mem_start >> 16);
+	writel(reg, &p_iop_reg->iop_base_adr_h);
+
+	reg = readl(&p_iop_reg->iop_control);
+	reg &= ~(0x01);
+	writel(reg, &p_iop_reg->iop_control);
+	while ((p_iop_reg->iop_data2&IOP_READY) != IOP_READY)
+		;
+
+	writel(RISC_READY, &p_iop_reg->iop_data2);
+	writel(0x00, &p_iop_reg->iop_data5);
+	writel(0x60, &p_iop_reg->iop_data6);
+	while (1) {
+		if (p_iop_reg->iop_data7 == 0xaaaa)
+			break;
+	}
+
+	writel(0xdd, &p_iop_reg->iop_data1);//8051 bin file call Ultra low function.
+	mdelay(10);
+}
+
+static int  sp_iop_get_normal_code(struct device *dev, struct sp_iop *iop)
+{
+	const struct firmware *fw;
+	static const char file[] = "normal.bin";
+	unsigned int err, i;
+
+	dev_info(dev, "normal code\n");
+	err = request_firmware(&fw, file, dev);
+	if (err) {
+		dev_info(dev, "get bin file error\n");
+		return err;
+	}
+
+	for (i = 0; i < NORMAL_CODE_MAX_SIZE; i++) {
+		char temp;
+
+		temp = fw->data[i];
+		iop->iop_normal_code[i] = temp;
+	}
+	release_firmware(fw);
+	return err;
+}
+
+static int  sp_iop_get_standby_code(struct device *dev, struct sp_iop *iop)
+{
+	const struct firmware *fw;
+	static const char file[] = "standby.bin";
+	unsigned int err, i;
+
+	dev_info(dev, "standby code\n");
+	err = request_firmware(&fw, file, dev);
+	if (err) {
+		dev_info(dev, "get bin file error\n");
+		return err;
+	}
+
+	for (i = 0; i < STANDBY_CODE_MAX_SIZE; i++) {
+		char temp;
+
+		temp = fw->data[i];
+		iop->iop_standby_code[i] = temp;
+	}
+	release_firmware(fw);
+	return err;
+}
+
+static int sp_iop_get_resources(struct platform_device *pdev,
+	struct sp_iop *pstSpIOPInfo)
+{
+	struct resource *r;
+
+	r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "iop");
+	pstSpIOPInfo->iop_regs = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(pstSpIOPInfo->iop_regs)) {
+		dev_err(&pdev->dev, "ioremap fail\n");
+		return PTR_ERR(pstSpIOPInfo->iop_regs);
+	}
+
+	r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "iop_pmc");
+	pstSpIOPInfo->pmc_regs = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(pstSpIOPInfo->pmc_regs)) {
+		dev_err(&pdev->dev, "ioremap fail\n");
+		return PTR_ERR(pstSpIOPInfo->pmc_regs);
+	}
+
+	r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "moon0");
+	pstSpIOPInfo->moon0_regs = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(pstSpIOPInfo->moon0_regs)) {
+		dev_err(&pdev->dev, "ioremap fail\n");
+		return PTR_ERR(pstSpIOPInfo->moon0_regs);
+	}
+	return IOP_SUCCESS;
+}
+
+static int sp_iop_platform_driver_probe(struct platform_device *pdev)
+{
+	int ret = -ENXIO;
+	int rc;
+	struct sp_iop *iop;
+	struct device_node *memnp;
+	struct resource mem_res;
+
+	iop = devm_kzalloc(&pdev->dev, sizeof(struct sp_iop), GFP_KERNEL);
+	if (iop == NULL) {
+		ret	= -ENOMEM;
+		goto fail_kmalloc;
+	}
+	/* init */
+	mutex_init(&iop->write_lock);
+	/* register device */
+	iop->dev.name  = "sp_iop";
+	iop->dev.minor = MISC_DYNAMIC_MINOR;
+	ret = misc_register(&iop->dev);
+	if (ret != 0) {
+		dev_err(&pdev->dev, "sp_iop device register fail\n");
+		goto fail_regdev;
+	}
+
+	ret = sp_iop_get_resources(pdev, iop);
+
+	//Get reserve address
+	memnp = of_parse_phandle(pdev->dev.of_node, "memory-region", 0);
+	if (!memnp) {
+		dev_err(&pdev->dev, "no memory-region node\n");
+		return -EINVAL;
+	}
+
+	rc = of_address_to_resource(memnp, 0, &mem_res);
+	of_node_put(memnp);
+	if (rc) {
+		dev_err(&pdev->dev, "failed to translate memory-region to a resource\n");
+		return -EINVAL;
+	}
+
+	iop->iop_mem_start = mem_res.start;
+	iop->iop_mem_size = resource_size(&mem_res);
+
+	ret = sp_iop_get_normal_code(&pdev->dev, iop);
+	if (ret != 0) {
+		dev_err(&pdev->dev, "get normal code err=%d\n", ret);
+		return ret;
+	}
+
+	ret = sp_iop_get_standby_code(&pdev->dev, iop);
+	if (ret != 0) {
+		dev_err(&pdev->dev, "get standby code err=%d\n", ret);
+		return ret;
+	}
+
+	sp_iop_normal_mode(iop);
+	platform_set_drvdata(pdev, iop);
+	return 0;
+
+fail_regdev:
+	mutex_destroy(&iop->write_lock);
+fail_kmalloc:
+	return ret;
+
+
+}
+
+static void sp_iop_platform_driver_shutdown(struct platform_device *pdev)
+{
+	struct sp_iop *iop = platform_get_drvdata(pdev);
+
+	sp_iop_standby_mode(iop);
+	sp_iop_shutdown(iop);
+}
+
+static const struct of_device_id sp_iop_of_match[] = {
+	{ .compatible = "sunplus,sp7021-iop" },
+	{ /* sentinel */ },
+};
+
+MODULE_DEVICE_TABLE(of, sp_iop_of_match);
+
+static struct platform_driver sp_iop_platform_driver = {
+	.probe		= sp_iop_platform_driver_probe,
+	.shutdown	= sp_iop_platform_driver_shutdown,
+	.driver = {
+		.name	= "sunplus,sp7021-iop",
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(sp_iop_of_match),
+	}
+};
+
+module_platform_driver(sp_iop_platform_driver);
+
+MODULE_AUTHOR("Tony Huang <tonyhuang.sunplus@gmail.com>");
+MODULE_DESCRIPTION("Sunplus IOP Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/2] misc: Add iop driver for Sunplus SP7021
  2021-12-09  8:58 ` [PATCH v3 2/2] misc: Add iop driver " Tony Huang
@ 2021-12-09  9:50   ` Arnd Bergmann
  2021-12-10  1:30     ` Tony Huang 黃懷厚
  2021-12-17 15:16     ` Tony Huang 黃懷厚
  2021-12-09 10:10   ` Greg KH
  1 sibling, 2 replies; 14+ messages in thread
From: Arnd Bergmann @ 2021-12-09  9:50 UTC (permalink / raw)
  To: Tony Huang
  Cc: Rob Herring, DTML, Linux Kernel Mailing List, Derek Kiernan,
	Dragan Cvetic, Arnd Bergmann, gregkh,
	Wells Lu 呂芳騰,
	Tony Huang

On Thu, Dec 9, 2021 at 9:58 AM Tony Huang <tonyhuang.sunplus@gmail.com> wrote:
>
> IOP (IO Processor) embedded inside SP7021 which is used as
> Processor for I/O control, RTC wake-up and cooperation with
> CPU & PMC in power management purpose.
> The IOP core is DQ8051, so also named IOP8051,
> it supports dedicated JTAG debug pins which share with SP7021.
> In standby mode operation, the power spec reach 400uA.
>
> Signed-off-by: Tony Huang <tonyhuang.sunplus@gmail.com>

Thanks for the improvements, this again looks better than the previous version.
I still have some minor comments, and there are a couple of details I have
commented on before that would need to be addressed, but let's focus
on the one main issue for now:

The driver still doesn't actually /do/ anything: you load the firmware when
the driver is loaded, and you shut it down when the driver is removed, but
otherwise there is no way to interact with the iop. You had the miscdevice
earlier, and you still register that, but there are no file_operations
associated
with it, so it still doesn't have any effect.

In the original version you had a couple of user-side interfaces, for which
Greg and I commented that they were not using the correct abstractions,
and you still list them in the changelog text as "I/O control, RTC wake-up
and cooperation with CPU & PMC in power management".

If you want to make any progress with adding the driver, I'd say you
should implement at least two of those high-level interfaces that interact
with the respective kernel subsystems in order to show that the
abstraction works.

        Arnd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/2] misc: Add iop driver for Sunplus SP7021
  2021-12-09  8:58 ` [PATCH v3 2/2] misc: Add iop driver " Tony Huang
  2021-12-09  9:50   ` Arnd Bergmann
@ 2021-12-09 10:10   ` Greg KH
  2021-12-10  1:19     ` Tony Huang 黃懷厚
  1 sibling, 1 reply; 14+ messages in thread
From: Greg KH @ 2021-12-09 10:10 UTC (permalink / raw)
  To: Tony Huang
  Cc: robh+dt, devicetree, linux-kernel, derek.kiernan, dragan.cvetic,
	arnd, wells.lu, tony.huang

On Thu, Dec 09, 2021 at 04:58:09PM +0800, Tony Huang wrote:
> +	while (1) {
> +		if (p_iop_reg->iop_data7 == 0xaaaa)
> +			break;
> +	}

You can not create a loop that could never exit.  Please fix.

Also, what prevents the compiler from optimizing this away into nothing?
This code does not look correct at all.

> +
> +	writel(0xdd, &p_iop_reg->iop_data1);//8051 bin file call Ultra low function.
> +	mdelay(10);
> +}
> +
> +static int  sp_iop_get_normal_code(struct device *dev, struct sp_iop *iop)
> +{
> +	const struct firmware *fw;
> +	static const char file[] = "normal.bin";
> +	unsigned int err, i;
> +
> +	dev_info(dev, "normal code\n");

Please always delete your debugging code before submitting a patch.

Also, please run checkpatch.pl on your changes before submitting them.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH v3 2/2] misc: Add iop driver for Sunplus SP7021
  2021-12-09 10:10   ` Greg KH
@ 2021-12-10  1:19     ` Tony Huang 黃懷厚
  0 siblings, 0 replies; 14+ messages in thread
From: Tony Huang 黃懷厚 @ 2021-12-10  1:19 UTC (permalink / raw)
  To: Greg KH, Tony Huang
  Cc: robh+dt, devicetree, linux-kernel, derek.kiernan, dragan.cvetic,
	arnd, Wells Lu 呂芳騰

Dear Greg:

> On Thu, Dec 09, 2021 at 04:58:09PM +0800, Tony Huang wrote:
> > +	while (1) {
> > +		if (p_iop_reg->iop_data7 == 0xaaaa)
> > +			break;
> > +	}
> 
> You can not create a loop that could never exit.  Please fix.
> 

OK, I will modify it.

> Also, what prevents the compiler from optimizing this away into nothing?
> This code does not look correct at all.
> 
> > +
> > +	writel(0xdd, &p_iop_reg->iop_data1);//8051 bin file call Ultra low
> function.
> > +	mdelay(10);
> > +}
> > +
> > +static int  sp_iop_get_normal_code(struct device *dev, struct sp_iop
> > +*iop) {
> > +	const struct firmware *fw;
> > +	static const char file[] = "normal.bin";
> > +	unsigned int err, i;
> > +
> > +	dev_info(dev, "normal code\n");
> 
> Please always delete your debugging code before submitting a patch.
> 

OK, I will delete it.

> Also, please run checkpatch.pl on your changes before submitting them.
> 
> thanks,
> 
> greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH v3 2/2] misc: Add iop driver for Sunplus SP7021
  2021-12-09  9:50   ` Arnd Bergmann
@ 2021-12-10  1:30     ` Tony Huang 黃懷厚
  2021-12-17 15:16     ` Tony Huang 黃懷厚
  1 sibling, 0 replies; 14+ messages in thread
From: Tony Huang 黃懷厚 @ 2021-12-10  1:30 UTC (permalink / raw)
  To: Arnd Bergmann, Tony Huang
  Cc: Rob Herring, DTML, Linux Kernel Mailing List, Derek Kiernan,
	Dragan Cvetic, gregkh, Wells Lu 呂芳騰

Dear Arnd:

> On Thu, Dec 9, 2021 at 9:58 AM Tony Huang <tonyhuang.sunplus@gmail.com>
> wrote:
> >
> > IOP (IO Processor) embedded inside SP7021 which is used as Processor
> > for I/O control, RTC wake-up and cooperation with CPU & PMC in power
> > management purpose.
> > The IOP core is DQ8051, so also named IOP8051, it supports dedicated
> > JTAG debug pins which share with SP7021.
> > In standby mode operation, the power spec reach 400uA.
> >
> > Signed-off-by: Tony Huang <tonyhuang.sunplus@gmail.com>
> 
> Thanks for the improvements, this again looks better than the previous version.
> I still have some minor comments, and there are a couple of details I have
> commented on before that would need to be addressed, but let's focus on the
> one main issue for now:
> 
> The driver still doesn't actually /do/ anything: you load the firmware when the
> driver is loaded, and you shut it down when the driver is removed, but
> otherwise there is no way to interact with the iop. You had the miscdevice
> earlier, and you still register that, but there are no file_operations associated
> with it, so it still doesn't have any effect.
> 
> In the original version you had a couple of user-side interfaces, for which Greg
> and I commented that they were not using the correct abstractions, and you
> still list them in the changelog text as "I/O control, RTC wake-up and
> cooperation with CPU & PMC in power management".
> 
> If you want to make any progress with adding the driver, I'd say you should
> implement at least two of those high-level interfaces that interact with the
> respective kernel subsystems in order to show that the abstraction works.
> 

OK, I will modify it.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/2] dt-binding: misc: Add iop yaml file for Sunplus SP7021
  2021-12-09  8:58 ` [PATCH v3 1/2] dt-binding: misc: Add iop yaml file " Tony Huang
@ 2021-12-14 19:32   ` Rob Herring
  0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2021-12-14 19:32 UTC (permalink / raw)
  To: Tony Huang
  Cc: devicetree, linux-kernel, derek.kiernan, dragan.cvetic, arnd,
	gregkh, wells.lu, tony.huang

On Thu, Dec 09, 2021 at 04:58:08PM +0800, Tony Huang wrote:
> Add iop yaml file for Sunplus SP7021
> 
> Signed-off-by: Tony Huang <tonyhuang.sunplus@gmail.com>
> ---
> Changes in v3:
>  - No change.
> 
>  .../devicetree/bindings/misc/sunplus-iop.yaml      | 61 ++++++++++++++++++++++
>  MAINTAINERS                                        |  5 ++
>  2 files changed, 66 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/sunplus-iop.yaml
> 
> diff --git a/Documentation/devicetree/bindings/misc/sunplus-iop.yaml b/Documentation/devicetree/bindings/misc/sunplus-iop.yaml
> new file mode 100644
> index 0000000..07cdbde
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/sunplus-iop.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) Sunplus Ltd. Co. 2021
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/misc/sunplus-iop.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sunplus IOP(8051) controller
> +
> +maintainers:
> +  - Tony Huang <tonyhuang.sunplus@gmail.com>
> +
> +description: |
> +  Processor for I/O control, RTC wake-up procedure management,
> +  and cooperation with CPU&PMC in power management.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - sunplus,sp7021-iop
> +
> +  reg:
> +    items:
> +      - description: IOP registers regions
> +      - description: PMC registers regions
> +      - description: MOON0 registers regions
> +
> +  reg-names:
> +    items:
> +      - const: iop
> +      - const: iop_pmc
> +      - const: moon0
> +
> +  interrupts:
> +    maxItems: 2

Need to define what each interrupt is.

> +
> +  memory-region:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - memory-region
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    iop: iop@9c000400 {
> +        compatible = "sunplus,sp7021-iop";
> +        reg = <0x9c000400 0x80>, <0x9c003100 0x80>, <0x9c000000 0x80>;
> +        reg-names = "iop", "iop_pmc", "moon0";
> +        interrupt-parent = <&intc>;
> +        interrupts = <41 IRQ_TYPE_LEVEL_HIGH>, <42 IRQ_TYPE_LEVEL_HIGH>;
> +        memory-region = <&iop_reserve>;
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3b79fd4..071b5e6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17945,6 +17945,11 @@ L:	netdev@vger.kernel.org
>  S:	Maintained
>  F:	drivers/net/ethernet/dlink/sundance.c
>  
> +SUNPLUS IOP DRIVER
> +M:	Tony Huang <tonyhuang.sunplus@gmail.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/misc/sunplu-iop.yaml
> +
>  SUPERH
>  M:	Yoshinori Sato <ysato@users.sourceforge.jp>
>  M:	Rich Felker <dalias@libc.org>
> -- 
> 2.7.4
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH v3 2/2] misc: Add iop driver for Sunplus SP7021
  2021-12-09  9:50   ` Arnd Bergmann
  2021-12-10  1:30     ` Tony Huang 黃懷厚
@ 2021-12-17 15:16     ` Tony Huang 黃懷厚
  2021-12-17 15:31       ` gregkh
  2021-12-17 16:09       ` Arnd Bergmann
  1 sibling, 2 replies; 14+ messages in thread
From: Tony Huang 黃懷厚 @ 2021-12-17 15:16 UTC (permalink / raw)
  To: Arnd Bergmann, Tony Huang
  Cc: Rob Herring, DTML, Linux Kernel Mailing List, Derek Kiernan,
	Dragan Cvetic, gregkh, Wells Lu 呂芳騰

Dear Arnd:

> On Thu, Dec 9, 2021 at 9:58 AM Tony Huang <tonyhuang.sunplus@gmail.com>
> wrote:
> >
> > IOP (IO Processor) embedded inside SP7021 which is used as Processor
> > for I/O control, RTC wake-up and cooperation with CPU & PMC in power
> > management purpose.
> > The IOP core is DQ8051, so also named IOP8051, it supports dedicated
> > JTAG debug pins which share with SP7021.
> > In standby mode operation, the power spec reach 400uA.
> >
> > Signed-off-by: Tony Huang <tonyhuang.sunplus@gmail.com>
> 
> Thanks for the improvements, this again looks better than the previous version.
> I still have some minor comments, and there are a couple of details I have
> commented on before that would need to be addressed, but let's focus on the
> one main issue for now:
> 
> The driver still doesn't actually /do/ anything: you load the firmware when the
> driver is loaded, and you shut it down when the driver is removed, but
> otherwise there is no way to interact with the iop. You had the miscdevice
> earlier, and you still register that, but there are no file_operations associated
> with it, so it still doesn't have any effect.
> 
> In the original version you had a couple of user-side interfaces, for which Greg
> and I commented that they were not using the correct abstractions, and you
> still list them in the changelog text as "I/O control, RTC wake-up and
> cooperation with CPU & PMC in power management".
> 
> If you want to make any progress with adding the driver, I'd say you should
> implement at least two of those high-level interfaces that interact with the
> respective kernel subsystems in order to show that the abstraction works.
> 

Q:"with respective kernel subsystems in order to show that the abstraction works."
May I ask you about repective kernel subsystem.
If I use the file_operation method
Provide user can read and write IOP(8051)'s register.
Is this a repective kernel subsystem?
if not
There are other driver code can give me reference


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/2] misc: Add iop driver for Sunplus SP7021
  2021-12-17 15:16     ` Tony Huang 黃懷厚
@ 2021-12-17 15:31       ` gregkh
  2021-12-18 12:53         ` Tony Huang 黃懷厚
  2021-12-17 16:09       ` Arnd Bergmann
  1 sibling, 1 reply; 14+ messages in thread
From: gregkh @ 2021-12-17 15:31 UTC (permalink / raw)
  To: Tony Huang 黃懷厚
  Cc: Arnd Bergmann, Tony Huang, Rob Herring, DTML,
	Linux Kernel Mailing List, Derek Kiernan, Dragan Cvetic,
	Wells Lu 呂芳騰

On Fri, Dec 17, 2021 at 03:16:53PM +0000, Tony Huang 黃懷厚 wrote:
> Dear Arnd:
> 
> > On Thu, Dec 9, 2021 at 9:58 AM Tony Huang <tonyhuang.sunplus@gmail.com>
> > wrote:
> > >
> > > IOP (IO Processor) embedded inside SP7021 which is used as Processor
> > > for I/O control, RTC wake-up and cooperation with CPU & PMC in power
> > > management purpose.
> > > The IOP core is DQ8051, so also named IOP8051, it supports dedicated
> > > JTAG debug pins which share with SP7021.
> > > In standby mode operation, the power spec reach 400uA.
> > >
> > > Signed-off-by: Tony Huang <tonyhuang.sunplus@gmail.com>
> > 
> > Thanks for the improvements, this again looks better than the previous version.
> > I still have some minor comments, and there are a couple of details I have
> > commented on before that would need to be addressed, but let's focus on the
> > one main issue for now:
> > 
> > The driver still doesn't actually /do/ anything: you load the firmware when the
> > driver is loaded, and you shut it down when the driver is removed, but
> > otherwise there is no way to interact with the iop. You had the miscdevice
> > earlier, and you still register that, but there are no file_operations associated
> > with it, so it still doesn't have any effect.
> > 
> > In the original version you had a couple of user-side interfaces, for which Greg
> > and I commented that they were not using the correct abstractions, and you
> > still list them in the changelog text as "I/O control, RTC wake-up and
> > cooperation with CPU & PMC in power management".
> > 
> > If you want to make any progress with adding the driver, I'd say you should
> > implement at least two of those high-level interfaces that interact with the
> > respective kernel subsystems in order to show that the abstraction works.
> > 
> 
> Q:"with respective kernel subsystems in order to show that the abstraction works."
> May I ask you about repective kernel subsystem.
> If I use the file_operation method
> Provide user can read and write IOP(8051)'s register.
> Is this a repective kernel subsystem?
> if not
> There are other driver code can give me reference
> 

I still do not understand what the goal of this driver is.

What is the problem that you are needing to solve?  What needs to access
this hardware, and what exactly was this hardware designed to do?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/2] misc: Add iop driver for Sunplus SP7021
  2021-12-17 15:16     ` Tony Huang 黃懷厚
  2021-12-17 15:31       ` gregkh
@ 2021-12-17 16:09       ` Arnd Bergmann
  1 sibling, 0 replies; 14+ messages in thread
From: Arnd Bergmann @ 2021-12-17 16:09 UTC (permalink / raw)
  To: Tony Huang 黃懷厚
  Cc: Arnd Bergmann, Tony Huang, Rob Herring, DTML,
	Linux Kernel Mailing List, Derek Kiernan, Dragan Cvetic, gregkh,
	Wells Lu 呂芳騰

On Fri, Dec 17, 2021 at 4:16 PM Tony Huang 黃懷厚 <tony.huang@sunplus.com> wrote:
> > On Thu, Dec 9, 2021 at 9:58 AM Tony Huang <tonyhuang.sunplus@gmail.com>
> > wrote:
> > >
> > > IOP (IO Processor) embedded inside SP7021 which is used as Processor
> > > for I/O control, RTC wake-up and cooperation with CPU & PMC in power
> > > management purpose.
> > > The IOP core is DQ8051, so also named IOP8051, it supports dedicated
> > > JTAG debug pins which share with SP7021.
> > > In standby mode operation, the power spec reach 400uA.
> > >
> > > Signed-off-by: Tony Huang <tonyhuang.sunplus@gmail.com>
> >
> > Thanks for the improvements, this again looks better than the previous version.
> > I still have some minor comments, and there are a couple of details I have
> > commented on before that would need to be addressed, but let's focus on the
> > one main issue for now:
> >
> > The driver still doesn't actually /do/ anything: you load the firmware when the
> > driver is loaded, and you shut it down when the driver is removed, but
> > otherwise there is no way to interact with the iop. You had the miscdevice
> > earlier, and you still register that, but there are no file_operations associated
> > with it, so it still doesn't have any effect.
> >
> > In the original version you had a couple of user-side interfaces, for which Greg
> > and I commented that they were not using the correct abstractions, and you
> > still list them in the changelog text as "I/O control, RTC wake-up and
> > cooperation with CPU & PMC in power management".
> >
> > If you want to make any progress with adding the driver, I'd say you should
> > implement at least two of those high-level interfaces that interact with the
> > respective kernel subsystems in order to show that the abstraction works.
> >
>
> Q:"with respective kernel subsystems in order to show that the abstraction works."
> May I ask you about repective kernel subsystem.
> If I use the file_operation method
> Provide user can read and write IOP(8051)'s register.
> Is this a repective kernel subsystem?
> if not
> There are other driver code can give me reference

- For gpio, there are lots of drivers in drivers/gpio/
- For RTC, there are drivers in drivers/rtc/
- For suspending the CPU core, there are drivers in drivers/cpuidle/
- For turning off the system, you can find drivers in drivers/power/reset/

Any of these drivers can interface with your kernel driver in various ways
that avoid adding a custom miscdevice.

         Arnd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH v3 2/2] misc: Add iop driver for Sunplus SP7021
  2021-12-17 15:31       ` gregkh
@ 2021-12-18 12:53         ` Tony Huang 黃懷厚
  2021-12-18 13:05           ` gregkh
  0 siblings, 1 reply; 14+ messages in thread
From: Tony Huang 黃懷厚 @ 2021-12-18 12:53 UTC (permalink / raw)
  To: gregkh
  Cc: Arnd Bergmann, Tony Huang, Rob Herring, DTML,
	Linux Kernel Mailing List, Derek Kiernan, Dragan Cvetic,
	Wells Lu 呂芳騰

Dear gregkh, arnd:

> > > wrote:
> > > >
> > > > IOP (IO Processor) embedded inside SP7021 which is used as
> > > > Processor for I/O control, RTC wake-up and cooperation with CPU &
> > > > PMC in power management purpose.
> > > > The IOP core is DQ8051, so also named IOP8051, it supports
> > > > dedicated JTAG debug pins which share with SP7021.
> > > > In standby mode operation, the power spec reach 400uA.
> > > >
> > > > Signed-off-by: Tony Huang <tonyhuang.sunplus@gmail.com>
> > >
> > > Thanks for the improvements, this again looks better than the previous
> version.
> > > I still have some minor comments, and there are a couple of details
> > > I have commented on before that would need to be addressed, but
> > > let's focus on the one main issue for now:
> > >
> > > The driver still doesn't actually /do/ anything: you load the
> > > firmware when the driver is loaded, and you shut it down when the
> > > driver is removed, but otherwise there is no way to interact with
> > > the iop. You had the miscdevice earlier, and you still register
> > > that, but there are no file_operations associated with it, so it still doesn't
> have any effect.
> > >
> > > In the original version you had a couple of user-side interfaces,
> > > for which Greg and I commented that they were not using the correct
> > > abstractions, and you still list them in the changelog text as "I/O
> > > control, RTC wake-up and cooperation with CPU & PMC in power
> management".
> > >
> > > If you want to make any progress with adding the driver, I'd say you
> > > should implement at least two of those high-level interfaces that
> > > interact with the respective kernel subsystems in order to show that the
> abstraction works.
> > >
> >
> > Q:"with respective kernel subsystems in order to show that the abstraction
> works."
> > May I ask you about repective kernel subsystem.
> > If I use the file_operation method
> > Provide user can read and write IOP(8051)'s register.
> > Is this a repective kernel subsystem?
> > if not
> > There are other driver code can give me reference
> >
> 
> I still do not understand what the goal of this driver is.
> 

When the poweroff command is executed.
1.The 8051 has a register to control the power-on and power-off of the system(Linux kernel).
 If you turn off the power through the 8051 register(DEF_PWR_EN_0=0),
 The current measured by the circuit board is 0.4mA only. In order to save power.
2.The power is not turned off through the 8051 register.
 The current measured on the circuit board is 33mA
3.When the system linux kerenl is powered off. /driver/rtc, /driver/gpio cannot operate.
  8051 is still alive and operational
  8051 has RTC register. When the time is up, 8051 powers on the system
  The 8051 can detect GPIO0~7 pins, and GPIO pin high/low can be used as a power-on judgment mechanism for the system.

> What is the problem that you are needing to solve?  What needs to access
> this hardware, and what exactly was this hardware designed to do?
> 

for example:	
SP7021 8051 has three power states:S0, S1 and S3.	
S0:Default domain(linux kernel) is on. IOP domain of 8051 is on. AO domain of 8051 is on.	
S1:Default domain(linux kernel) is off. IOP domain of 8051 is on. AO domain of 8051 is on.	
S3:Default domain(linux kernel) is off. IOP domain of 8051 is off. AO domain of 8051 is on.	
The user can use the 12bytes mailbox register to notify 8051.8051 needs to turn off which domain.	
I need custom miscdevice.
User<--->misc/sunplus_iop<--->Hardware(8051)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/2] misc: Add iop driver for Sunplus SP7021
  2021-12-18 12:53         ` Tony Huang 黃懷厚
@ 2021-12-18 13:05           ` gregkh
  2021-12-18 17:30             ` Tony Huang 黃懷厚
  0 siblings, 1 reply; 14+ messages in thread
From: gregkh @ 2021-12-18 13:05 UTC (permalink / raw)
  To: Tony Huang 黃懷厚
  Cc: Arnd Bergmann, Tony Huang, Rob Herring, DTML,
	Linux Kernel Mailing List, Derek Kiernan, Dragan Cvetic,
	Wells Lu 呂芳騰

On Sat, Dec 18, 2021 at 12:53:08PM +0000, Tony Huang 黃懷厚 wrote:
> Dear gregkh, arnd:
> 
> > > > wrote:
> > > > >
> > > > > IOP (IO Processor) embedded inside SP7021 which is used as
> > > > > Processor for I/O control, RTC wake-up and cooperation with CPU &
> > > > > PMC in power management purpose.
> > > > > The IOP core is DQ8051, so also named IOP8051, it supports
> > > > > dedicated JTAG debug pins which share with SP7021.
> > > > > In standby mode operation, the power spec reach 400uA.
> > > > >
> > > > > Signed-off-by: Tony Huang <tonyhuang.sunplus@gmail.com>
> > > >
> > > > Thanks for the improvements, this again looks better than the previous
> > version.
> > > > I still have some minor comments, and there are a couple of details
> > > > I have commented on before that would need to be addressed, but
> > > > let's focus on the one main issue for now:
> > > >
> > > > The driver still doesn't actually /do/ anything: you load the
> > > > firmware when the driver is loaded, and you shut it down when the
> > > > driver is removed, but otherwise there is no way to interact with
> > > > the iop. You had the miscdevice earlier, and you still register
> > > > that, but there are no file_operations associated with it, so it still doesn't
> > have any effect.
> > > >
> > > > In the original version you had a couple of user-side interfaces,
> > > > for which Greg and I commented that they were not using the correct
> > > > abstractions, and you still list them in the changelog text as "I/O
> > > > control, RTC wake-up and cooperation with CPU & PMC in power
> > management".
> > > >
> > > > If you want to make any progress with adding the driver, I'd say you
> > > > should implement at least two of those high-level interfaces that
> > > > interact with the respective kernel subsystems in order to show that the
> > abstraction works.
> > > >
> > >
> > > Q:"with respective kernel subsystems in order to show that the abstraction
> > works."
> > > May I ask you about repective kernel subsystem.
> > > If I use the file_operation method
> > > Provide user can read and write IOP(8051)'s register.
> > > Is this a repective kernel subsystem?
> > > if not
> > > There are other driver code can give me reference
> > >
> > 
> > I still do not understand what the goal of this driver is.
> > 
> 
> When the poweroff command is executed.

What exactly do you mean by this?  The power off command that is sent to
the kernel from userspace?

> 1.The 8051 has a register to control the power-on and power-off of the system(Linux kernel).
>  If you turn off the power through the 8051 register(DEF_PWR_EN_0=0),
>  The current measured by the circuit board is 0.4mA only. In order to save power.
> 2.The power is not turned off through the 8051 register.
>  The current measured on the circuit board is 33mA
> 3.When the system linux kerenl is powered off. /driver/rtc, /driver/gpio cannot operate.
>   8051 is still alive and operational
>   8051 has RTC register. When the time is up, 8051 powers on the system
>   The 8051 can detect GPIO0~7 pins, and GPIO pin high/low can be used as a power-on judgment mechanism for the system.

So what do you need to do?  When the kernel needs to shut down the
system, send a message to this hardware device?  If so, great, just tie
into the normal powerdown sequence, this is nothing new.

If not, I do not understand.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH v3 2/2] misc: Add iop driver for Sunplus SP7021
  2021-12-18 13:05           ` gregkh
@ 2021-12-18 17:30             ` Tony Huang 黃懷厚
  0 siblings, 0 replies; 14+ messages in thread
From: Tony Huang 黃懷厚 @ 2021-12-18 17:30 UTC (permalink / raw)
  To: gregkh
  Cc: Arnd Bergmann, Tony Huang, Rob Herring, DTML,
	Linux Kernel Mailing List, Derek Kiernan, Dragan Cvetic,
	Wells Lu 呂芳騰

Dear gregkh, arnd:


> >
> > > > > wrote:
> > > > > >
> > > > > > IOP (IO Processor) embedded inside SP7021 which is used as
> > > > > > Processor for I/O control, RTC wake-up and cooperation with
> > > > > > CPU & PMC in power management purpose.
> > > > > > The IOP core is DQ8051, so also named IOP8051, it supports
> > > > > > dedicated JTAG debug pins which share with SP7021.
> > > > > > In standby mode operation, the power spec reach 400uA.
> > > > > >
> > > > > > Signed-off-by: Tony Huang <tonyhuang.sunplus@gmail.com>
> > > > >
> > > > > Thanks for the improvements, this again looks better than the
> > > > > previous
> > > version.
> > > > > I still have some minor comments, and there are a couple of
> > > > > details I have commented on before that would need to be
> > > > > addressed, but let's focus on the one main issue for now:
> > > > >
> > > > > The driver still doesn't actually /do/ anything: you load the
> > > > > firmware when the driver is loaded, and you shut it down when
> > > > > the driver is removed, but otherwise there is no way to interact
> > > > > with the iop. You had the miscdevice earlier, and you still
> > > > > register that, but there are no file_operations associated with
> > > > > it, so it still doesn't
> > > have any effect.
> > > > >
> > > > > In the original version you had a couple of user-side
> > > > > interfaces, for which Greg and I commented that they were not
> > > > > using the correct abstractions, and you still list them in the
> > > > > changelog text as "I/O control, RTC wake-up and cooperation with
> > > > > CPU & PMC in power
> > > management".
> > > > >
> > > > > If you want to make any progress with adding the driver, I'd say
> > > > > you should implement at least two of those high-level interfaces
> > > > > that interact with the respective kernel subsystems in order to
> > > > > show that the
> > > abstraction works.
> > > > >
> > > >
> > > > Q:"with respective kernel subsystems in order to show that the
> > > > abstraction
> > > works."
> > > > May I ask you about repective kernel subsystem.
> > > > If I use the file_operation method Provide user can read and write
> > > > IOP(8051)'s register.
> > > > Is this a repective kernel subsystem?
> > > > if not
> > > > There are other driver code can give me reference
> > > >
> > >
> > > I still do not understand what the goal of this driver is.
> > >
> >
> > When the poweroff command is executed.
> 
> What exactly do you mean by this?  The power off command that is sent to
> the kernel from userspace?
> 

I type "poweroff" in command line.

> > 1.The 8051 has a register to control the power-on and power-off of the
> system(Linux kernel).
> >  If you turn off the power through the 8051 register(DEF_PWR_EN_0=0),
> > The current measured by the circuit board is 0.4mA only. In order to save
> power.
> > 2.The power is not turned off through the 8051 register.
> >  The current measured on the circuit board is 33mA 3.When the system
> > linux kerenl is powered off. /driver/rtc, /driver/gpio cannot operate.
> >   8051 is still alive and operational
> >   8051 has RTC register. When the time is up, 8051 powers on the system
> >   The 8051 can detect GPIO0~7 pins, and GPIO pin high/low can be used as
> a power-on judgment mechanism for the system.
> 
> So what do you need to do?  When the kernel needs to shut down the system,
> send a message to this hardware device?  If so, great, just tie into the normal
> powerdown sequence, this is nothing new.
> 

Yes, 									
poweroff command ---> run misc/sunplus_ioc.c/sp_iop_platform_driver_shutdown()---->switch standby.bin for 8051--->setting 8051 PMC register---> sent a message to 8051			
Q:Why need switch standby.bin for 8051?
A: 8051 has two bin files, normal.bin and standby.bin in rootfs.
Normally, 8051 executes normal.bin code. Normal.bin code size can exceed 16K.
When system linux kernel is shutdown, 8051 is to execute standby.bin code.
Standby.bin code cannot exceed 16K bytes. Because 8051 Icache size is 16k
8051 runs with standby.bin code in the Icache before the system(linux kernel) is poweroff.
So I need misc/sunplus_iop.c interface to switch bin code and sends message to 8051.

Thanks

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-12-18 17:31 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-09  8:58 [PATCH v3 0/2] Add iop driver for Sunplus SP7021 Tony Huang
2021-12-09  8:58 ` [PATCH v3 1/2] dt-binding: misc: Add iop yaml file " Tony Huang
2021-12-14 19:32   ` Rob Herring
2021-12-09  8:58 ` [PATCH v3 2/2] misc: Add iop driver " Tony Huang
2021-12-09  9:50   ` Arnd Bergmann
2021-12-10  1:30     ` Tony Huang 黃懷厚
2021-12-17 15:16     ` Tony Huang 黃懷厚
2021-12-17 15:31       ` gregkh
2021-12-18 12:53         ` Tony Huang 黃懷厚
2021-12-18 13:05           ` gregkh
2021-12-18 17:30             ` Tony Huang 黃懷厚
2021-12-17 16:09       ` Arnd Bergmann
2021-12-09 10:10   ` Greg KH
2021-12-10  1:19     ` Tony Huang 黃懷厚

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.