Add iop driver for Sunplus SP7021

Add iop yaml file for Sunplus SP7021

Signed-off-by Tony Huang <[email protected]>

---
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

Documentation/devicetree/bindings/misc/sunplus-iop.yaml new file +25 -0

new file mode 100644
index 0000000..07cdbde
@@ -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 <[email protected]>
+
+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

Documentation/devicetree/bindings/misc/sunplus-iop.yaml new file +26 -0

+
+  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>;
+    };
+...

MAINTAINERS +5 -0

index 3b79fd4..071b5e6 100644
@@ -17945,6 +17945,11 @@ L:	[email protected]
 S:	Maintained
 F:	drivers/net/ethernet/dlink/sundance.c
 
+SUNPLUS IOP DRIVER
+M:	Tony Huang <[email protected]>
+S:	Maintained
+F:	Documentation/devicetree/bindings/misc/sunplu-iop.yaml
+
 SUPERH
 M:	Yoshinori Sato <[email protected]>
 M:	Rich Felker <[email protected]>

Signature · 1 line

2.7.4

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 <[email protected]>

Quoted 9 lines · 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 <[email protected]>

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.

Quoted 9 lines · 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 <[email protected]>

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.

Dear Arnd:

Quoted 23 lines

Quoted 9 lines · 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 <[email protected]>

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.

Quoted 9 lines

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

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines · 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.

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

Signed-off-by Tony Huang <[email protected]>

Thanks for the improvements, this again looks better than the previous

version.

Quoted 9 lines

Quoted 9 lines

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.

Quoted 5 lines

Quoted 5 lines

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".

Quoted 4 lines

Quoted 4 lines

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.

Quoted 3 lines

Quoted 1 line

Q:"with respective kernel subsystems in order to show that the abstraction

works."

Quoted 7 lines

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?

Dear gregkh, arnd:

Quoted 46 lines

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines · 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.

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

Signed-off-by Tony Huang <[email protected]>

Thanks for the improvements, this again looks better than the previous

version.

Quoted 9 lines

Quoted 9 lines

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.

Quoted 5 lines

Quoted 5 lines

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".

Quoted 4 lines

Quoted 4 lines

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.

Quoted 3 lines

Quoted 1 line

Q:"with respective kernel subsystems in order to show that the abstraction

works."

Quoted 7 lines

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

Quoted 2 lines

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.

Quoted 52 lines

Quoted 52 lines

Quoted 10 lines

Quoted 10 lines

Quoted 8 lines · 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.

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

Signed-off-by Tony Huang <[email protected]>

Thanks for the improvements, this again looks better than the previous

version.

Quoted 10 lines

Quoted 10 lines

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.

Quoted 6 lines

Quoted 6 lines

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".

Quoted 5 lines

Quoted 5 lines

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.

Quoted 4 lines

Quoted 1 line

Q:"with respective kernel subsystems in order to show that the abstraction

works."

Quoted 7 lines

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

Quoted 4 lines

Quoted 2 lines

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?

Quoted 1 line

1.The 8051 has a register to control the power-on and power-off of the

system(Linux kernel).

Quoted 2 lines

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.

Quoted 6 lines

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.

Quoted 23 lines

Quoted 9 lines · 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 <[email protected]>

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.

Quoted 9 lines

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

drivers/misc/sunplus_iop.c line 219

Quoted 4 lines

drivers/misc/sunplus_iop.c new file +4 -0

+	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.

Review

Quoted 2 lines

(quoted diff) +2 -0

+
+	writel(0xdd, &p_iop_reg->iop_data1);//8051 bin file call Ultra low

function.

drivers/misc/sunplus_iop.c line 231

Quoted 10 lines

drivers/misc/sunplus_iop.c new file +10 -0

+	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