Dear Greg: > > > > IOP(8051) embedded inside SP7021 which is used as Processor for > > > > I/O control, monitor RTC interrupt 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@xxxxxxxxx> > > > > --- > > > > Changes in v6: > > > > - Added sysfs read/write description. > > > > - Modify sysfs read function. > > > > - Addressed comments from kernel test robot. > > > > > > > > Documentation/ABI/testing/sysfs-platform-soc@B | 25 ++ > > > > MAINTAINERS | 2 + > > > > drivers/misc/Kconfig | 12 + > > > > drivers/misc/Makefile | 1 + > > > > drivers/misc/sunplus_iop.c | 476 > > > +++++++++++++++++++++++++ > > > > 5 files changed, 516 insertions(+) create mode 100644 > > > > Documentation/ABI/testing/sysfs-platform-soc@B > > > > create mode 100644 drivers/misc/sunplus_iop.c > > > > > > > > diff --git a/Documentation/ABI/testing/sysfs-platform-soc@B > > > > b/Documentation/ABI/testing/sysfs-platform-soc@B > > > > new file mode 100644 > > > > index 0000000..6272919 > > > > --- /dev/null > > > > +++ b/Documentation/ABI/testing/sysfs-platform-soc@B > > > > @@ -0,0 +1,25 @@ > > > > +What: > /sys/devices/platform/soc@B/9c000400.iop/sp_iop_mailbox > > > > +Date: December 2021 > > > > +KernelVersion: 5.16 > > > > +Contact: Tony Huang <tonyhuang.sunplus@xxxxxxxxx> > > > > +Description: > > > > + Show 8051 mailbox0 data. > > > > > > What format is the data in? > > > > > > > Unsigned short > > So you are returning 16 bits, please describe the value as to what it will > contain and what it means. > > And what exactly does "8051" mean here? That is just some random > processor in the system, it needs to be described better please. > > > > > +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. > > > > > > I do not understand this sentence, what do you mean by it? Can you > > > provide a link to where the files that are required are? Why not > > > include them in the linux-firmware project? > > > > > > > 1.We will provide users with 8051 normal and standby source code. > > > Path: > https://sunplus.atlassian.net/wiki/spaces/doc/pages/610172933/How+to+use+I > +O+processor+8051+of+SP7021#5.-Write-C-or-assembly-source-files-for-IOP > > > 2.Users can follow the operation steps to generate normal.bin and > standby.bin. > > Path: > https://sunplus.atlassian.net/wiki/spaces/doc/pages/466190338/26.+IOP8051 > > > 26.5 How To Create 8051 bin file > > Please include stuff like this in the help text to make it more obvious. > OK. I will add in help text. > > > > +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 */ > > > > > > What are these comments numbering? > > > > > > > regs_moon0 is Group 0 moon register. > > The Group0 moon register range is 0x9c00000~0x9c00007F > > > /*00*/: 0x9c000000~0x9c000003 > > /*01~10*/:0x9c000004~0x9c00002b > > /*11~20*/:0x9c00002c~0x9c000053 > > /*21~30*/:0x9c000054~0x9c00007b > > /*31*/:0x9c00007c~0x9c00007f > > Ok, so the number is just the offset into a memory location? Please be > specific. > OK, I will modify it. > > > > > > > +}; > > > > + > > > > +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 */ > > > > > > Same here, what are these numbers? > > > > > > And why are they not lined up like the previous structure? > > > > > > > Sorry, I don't understand what you mean. Isn't this a struct? > > > Your comments are not lined up the same way the structure above has them. > OK, I will modify it. > > > > + struct mutex write_lock; > > > > + void *iop_regs; > > > > + void *pmc_regs; > > > > + void *moon0_regs; > > > > > > Why void pointers? You created structures above, use them! > > > > > > > Because I received "Reported-by: kernel test robot <lkp@xxxxxxxxx>", > warmming message. As follows: > > sparse warnings: (new ones prefixed by >>) > > > drivers/misc/sunplus_iop.c:94:39: sparse: sparse: cast removes address > space '__iomem' of expression > > drivers/misc/sunplus_iop.c:95:43: sparse: sparse: cast removes address > space '__iomem' of expression > > >> drivers/misc/sunplus_iop.c:100:16: sparse: sparse: incorrect type in > argument 1 (different address spaces) @@ expected void *p @@ > got void [noderef] __iomem *[assigned] iop_kernel_base @@ > > > drivers/misc/sunplus_iop.c:100:16: sparse: expected void *p > > > drivers/misc/sunplus_iop.c:100:16: sparse: got void [noderef] > __iomem *[assigned] iop_kernel_base > > > Ah, so these are actual pointers to the iomem memory? Or pointers to the > structures you have copied out? It is not obvious. > > > > > + 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); > > > > + > > > > + writel(WAKEUP_PIN, &p_iop_reg->iop_data0); > > > > + writel(iop->gpio_wakeup, &p_iop_reg->iop_data1); > > > > + > > > > + ret = readl_poll_timeout(&p_iop_reg->iop_data2, value, > > > > + (value & IOP_READY) == IOP_READY, 1000, 10000); > > > > + if (ret) { > > > > + dev_err(dev, "timed out\n"); > > > > + return ret; > > > > + } > > > > + > > > > + writel(RISC_READY, &p_iop_reg->iop_data2); > > > > + writel(0x00, &p_iop_reg->iop_data5); > > > > + writel(0x60, &p_iop_reg->iop_data6); > > > > + > > > > + ret = readl_poll_timeout(&p_iop_reg->iop_data7, value, > > > > + value == 0xaaaa, 1000, 10000); > > > > + if (ret) { > > > > + dev_err(dev, "timed out\n"); > > > > + return ret; > > > > + } > > > > + > > > > + writel(0xdd, &p_iop_reg->iop_data1);//8051 bin file call Ultra > > > > +low > > > function. > > > > + mdelay(10); > > > > > > Where did 10 come from? How do you know that is correct? > > > > > > > I need time to move stanby.bin code 16K data from SDRAM to 8051's icache. > > > 10msec should be enough > > Please document it, as it looks like the data was already sent so there should > not be any need for a delay on the Linux side. > > Or do a read which will ensure that the data is there, right? > Let me explain: 1. writel(0xdd, &p_iop_reg->iop_data1);//8051 bin file call Ultra low function < -----when system linux kernel wrtie mailbox1 iop_data1=0xdd. 8051's standby code see mailbox1=0xdd. I will move stanby.bin code 16K data from SDRAM to 8051's icache in 8051's standby code. I will turn off power through the 8051 register(DEF_PWR_EN_0=0) in 8051's standby code. 2. mdelay(10);<------- When the execution is here, the system linux kernel is about to be powered off The purpose: Do not let the system linux kernel continue to run other programs > > > > +static void sp_iop_platform_driver_shutdown(struct > > > > +platform_device > > > > +*pdev) { > > > > + struct sp_iop *iop = platform_get_drvdata(pdev); > > > > + struct regs_iop *p_iop_reg = (struct regs_iop *)iop->iop_regs; > > > > + unsigned int value; > > > > + > > > > + sp_iop_standby_mode(iop); > > > > + mdelay(10); > > > > > > Why sleep on shutdown? > > > > > > > I need time to switch from normal.bin code to standby.bin code. > > > Then why does the function call not wait until that happens? Why wait here? > sp_iop_standby_mode(iop); <--- I need ensure to switch from normal.bin code to standby.bin code. mdelay(10); <--- I will remove this delay time. I will read the data to confirm to replace the delay time Thanks
