* [PATCH v2 0/2] Add iop driver for Sunplus SP7021 @ 2021-12-03 3:48 Tony Huang 2021-12-03 3:48 ` [PATCH v2 1/2] dt-binding: misc: Add iop yaml file " Tony Huang 2021-12-03 3:48 ` [PATCH v2 2/2] misc: Add iop driver " Tony Huang 0 siblings, 2 replies; 19+ messages in thread From: Tony Huang @ 2021-12-03 3:48 UTC (permalink / raw) To: derek.kiernan, dragan.cvetic, arnd, gregkh, linux-kernel, robh+dt, devicetree 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 | 1 + drivers/misc/Makefile | 1 + drivers/misc/iop/Kconfig | 13 + drivers/misc/iop/Makefile | 6 + drivers/misc/iop/sunplus_iop.c | 518 +++++++++++++++++++++ drivers/misc/iop/sunplus_iop.h | 64 +++ 8 files changed, 670 insertions(+) create mode 100644 Documentation/devicetree/bindings/misc/sunplus-iop.yaml create mode 100644 drivers/misc/iop/Kconfig create mode 100644 drivers/misc/iop/Makefile create mode 100644 drivers/misc/iop/sunplus_iop.c create mode 100644 drivers/misc/iop/sunplus_iop.h -- 2.7.4 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 1/2] dt-binding: misc: Add iop yaml file for Sunplus SP7021 2021-12-03 3:48 [PATCH v2 0/2] Add iop driver for Sunplus SP7021 Tony Huang @ 2021-12-03 3:48 ` Tony Huang 2021-12-03 3:48 ` [PATCH v2 2/2] misc: Add iop driver " Tony Huang 1 sibling, 0 replies; 19+ messages in thread From: Tony Huang @ 2021-12-03 3:48 UTC (permalink / raw) To: derek.kiernan, dragan.cvetic, arnd, gregkh, linux-kernel, robh+dt, devicetree 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 v2: - Added moon0 register in yaml file. .../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] 19+ messages in thread
* [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-03 3:48 [PATCH v2 0/2] Add iop driver for Sunplus SP7021 Tony Huang 2021-12-03 3:48 ` [PATCH v2 1/2] dt-binding: misc: Add iop yaml file " Tony Huang @ 2021-12-03 3:48 ` Tony Huang 2021-12-03 9:12 ` kernel test robot ` (4 more replies) 1 sibling, 5 replies; 19+ messages in thread From: Tony Huang @ 2021-12-03 3:48 UTC (permalink / raw) To: derek.kiernan, dragan.cvetic, arnd, gregkh, linux-kernel, robh+dt, devicetree 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 v2: - Addressed comments from Arnd Bergmann. - Addressed comments from Greg KH. - Addressed comments from kernel test robot. MAINTAINERS | 1 + drivers/misc/Kconfig | 1 + drivers/misc/Makefile | 1 + drivers/misc/iop/Kconfig | 13 ++ drivers/misc/iop/Makefile | 6 + drivers/misc/iop/sunplus_iop.c | 518 +++++++++++++++++++++++++++++++++++++++++ drivers/misc/iop/sunplus_iop.h | 64 +++++ 7 files changed, 604 insertions(+) create mode 100644 drivers/misc/iop/Kconfig create mode 100644 drivers/misc/iop/Makefile create mode 100644 drivers/misc/iop/sunplus_iop.c create mode 100644 drivers/misc/iop/sunplus_iop.h diff --git a/MAINTAINERS b/MAINTAINERS index 071b5e6..f14aa19 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/iop SUPERH M: Yoshinori Sato <ysato@users.sourceforge.jp> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 0f5a49f..8fbd9a6 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -481,6 +481,7 @@ source "drivers/misc/vmw_vmci/Kconfig" source "drivers/misc/genwqe/Kconfig" source "drivers/misc/echo/Kconfig" source "drivers/misc/cxl/Kconfig" +source "drivers/misc/iop/Kconfig" source "drivers/misc/ocxl/Kconfig" source "drivers/misc/bcm-vk/Kconfig" source "drivers/misc/cardreader/Kconfig" diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index a086197..5ab7fb0 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) += iop/ obj-y += cardreader/ obj-$(CONFIG_PVPANIC) += pvpanic/ obj-$(CONFIG_HABANA_AI) += habanalabs/ diff --git a/drivers/misc/iop/Kconfig b/drivers/misc/iop/Kconfig new file mode 100644 index 0000000..3afda5c --- /dev/null +++ b/drivers/misc/iop/Kconfig @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# IOP configuration +# + +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. \ No newline at end of file diff --git a/drivers/misc/iop/Makefile b/drivers/misc/iop/Makefile new file mode 100644 index 0000000..e44b021 --- /dev/null +++ b/drivers/misc/iop/Makefile @@ -0,0 +1,6 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# Makefile for the IOP module drivers. +# + +obj-$(CONFIG_SUNPLUS_IOP) += sunplus_iop.o diff --git a/drivers/misc/iop/sunplus_iop.c b/drivers/misc/iop/sunplus_iop.c new file mode 100644 index 0000000..bb8d0da --- /dev/null +++ b/drivers/misc/iop/sunplus_iop.c @@ -0,0 +1,518 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +#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> +#include "sunplus_iop.h" + +#define NORMAL_CODE_MAX_SIZE 0X1000 +#define STANDBY_CODE_MAX_SIZE 0x4000 +unsigned char iop_normal_code[NORMAL_CODE_MAX_SIZE]; +unsigned char iop_standby_code[STANDBY_CODE_MAX_SIZE]; +/* ---------------------------------------------------------------------------------------------- */ +resource_size_t SP_IOP_RESERVE_BASE; +resource_size_t SP_IOP_RESERVE_SIZE; +/* ---------------------------------------------------------------------------------------------- */ +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; +}; +/***************************************************************** + * G L O B A L D A T A + ******************************************************************/ +static struct sp_iop *iop; + +void iop_normal_mode(void) +{ + 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(SP_IOP_RESERVE_BASE, NORMAL_CODE_MAX_SIZE); + memset(iop_kernel_base, 0, NORMAL_CODE_MAX_SIZE); + memcpy(iop_kernel_base, 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 = (SP_IOP_RESERVE_BASE & 0xFFFF); + writel(reg, &p_iop_reg->iop_base_adr_l); + reg = (SP_IOP_RESERVE_BASE >> 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); +} + +void iop_standby_mode(void) +{ + 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(SP_IOP_RESERVE_BASE, STANDBY_CODE_MAX_SIZE); + memset(iop_kernel_base, 0, STANDBY_CODE_MAX_SIZE); + memcpy(iop_kernel_base, 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 = (SP_IOP_RESERVE_BASE & 0xFFFF); + writel(reg, &p_iop_reg->iop_base_adr_l); + reg = (SP_IOP_RESERVE_BASE >> 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); +} + +void get_iop_data(struct device *dev) +{ + struct regs_iop *p_iop_reg = (struct regs_iop *)iop->iop_regs; + unsigned short value_0, value_1, value_2, value_3, value_4, value_5; + unsigned short value_6, value_7, value_8, value_9, value_10, value_11; + + value_0 = readl(&p_iop_reg->iop_data0); + value_1 = readl(&p_iop_reg->iop_data1); + value_2 = readl(&p_iop_reg->iop_data2); + value_3 = readl(&p_iop_reg->iop_data3); + value_4 = readl(&p_iop_reg->iop_data4); + value_5 = readl(&p_iop_reg->iop_data5); + value_6 = readl(&p_iop_reg->iop_data6); + value_7 = readl(&p_iop_reg->iop_data7); + value_8 = readl(&p_iop_reg->iop_data8); + value_9 = readl(&p_iop_reg->iop_data9); + value_10 = readl(&p_iop_reg->iop_data10); + value_11 = readl(&p_iop_reg->iop_data11); + dev_info(dev, "%s(%d) iop_data0=%x iop_data1=%x iop_data2=%x iop_data3=%x\n", + __func__, __LINE__, value_0, value_1, value_2, value_3); + dev_info(dev, "%s(%d) iop_data4=%x iop_data5=%x iop_data6=%x iop_data7=%x\n", + __func__, __LINE__, value_4, value_5, value_6, value_7); + dev_info(dev, "%s(%d) iop_data8=%x iop_data9=%x iop_data10=%x iop_data11=%x\n", + __func__, __LINE__, value_8, value_9, value_10, value_11); +} + +#define IOP_READY 0x4 +#define RISC_READY 0x8 +void iop_suspend(void) +{ + 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 long 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 = (SP_IOP_RESERVE_BASE & 0xFFFF); + writel(reg, &p_iop_reg->iop_base_adr_l); + reg = (SP_IOP_RESERVE_BASE >> 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) + ; + + reg = readl(&p_iop_reg->iop_data2); + reg |= RISC_READY; + writel(reg, &p_iop_reg->iop_control); + + 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. +} + +void iop_shutdown(void) +{ + 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 = (SP_IOP_RESERVE_BASE & 0xFFFF); + writel(reg, &p_iop_reg->iop_base_adr_l); + reg = (SP_IOP_RESERVE_BASE >> 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); +} + +void iop_s1mode(void) +{ + struct regs_iop *p_iop_reg = (struct regs_iop *)iop->iop_regs; + + 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(0xee, &p_iop_reg->iop_data1);//8051 bin file call S1_mode function. +} + +static int get_normal_code(struct device *dev) +{ + 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_normal_code[i] = temp; + } + release_firmware(fw); + return err; +} + +static int get_standby_code(struct device *dev) +{ + 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_standby_code[i] = temp; + } + release_firmware(fw); + return err; +} + +static int sp_iop_open(struct inode *inode, struct file *pfile) +{ + return 0; +} + +static ssize_t sp_iop_read(struct file *pfile, char __user *ubuf, + size_t length, loff_t *offset) +{ + return 0; +} + +static ssize_t sp_iop_write(struct file *pfile, const char __user *ubuf, + size_t length, loff_t *offset) +{ + return 0; +} + +static int sp_iop_release(struct inode *inode, struct file *pfile) +{ + //dev_dbg(iop->dev, "Sunplus IOP module release\n"); + return 0; +} + +static const struct file_operations sp_iop_fops = { + .owner = THIS_MODULE, + .open = sp_iop_open, + .read = sp_iop_read, + .write = sp_iop_write, + .release = sp_iop_release, +}; + +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 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; + iop->dev.fops = &sp_iop_fops; + 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; + } + + SP_IOP_RESERVE_BASE = mem_res.start; + SP_IOP_RESERVE_SIZE = resource_size(&mem_res); + + ret = get_normal_code(&pdev->dev); + if (ret != 0) { + dev_err(&pdev->dev, "get normal code err=%d\n", ret); + return ret; + } + + ret = get_standby_code(&pdev->dev); + if (ret != 0) { + dev_err(&pdev->dev, "get standby code err=%d\n", ret); + return ret; + } + + iop_normal_mode(); + return 0; + +fail_regdev: + mutex_destroy(&iop->write_lock); +fail_kmalloc: + return ret; + + +} + +static int sp_iop_platform_driver_remove(struct platform_device *pdev) +{ + return 0; +} + +static int sp_iop_platform_driver_suspend(struct platform_device *pdev, pm_message_t state) +{ + return 0; +} + +static void sp_iop_platform_driver_shutdown(struct platform_device *pdev) +{ + +} + +void sp_iop_platform_driver_poweroff(void) +{ + iop_standby_mode(); + iop_shutdown(); +} + +static int sp_iop_platform_driver_resume(struct platform_device *pdev) +{ + return 0; +} + +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, + .remove = sp_iop_platform_driver_remove, + .suspend = sp_iop_platform_driver_suspend, + .shutdown = sp_iop_platform_driver_shutdown, + .resume = sp_iop_platform_driver_resume, + .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"); diff --git a/drivers/misc/iop/sunplus_iop.h b/drivers/misc/iop/sunplus_iop.h new file mode 100644 index 0000000..fcbfd26 --- /dev/null +++ b/drivers/misc/iop/sunplus_iop.h @@ -0,0 +1,64 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later*/ + +#ifndef __SP_IOP_H__ +#define __SP_IOP_H__ +#include <mach/io_map.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 */ +}; + +void sp_iop_platform_driver_poweroff(void); +#endif /* __SP_IOP_H__ */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-03 3:48 ` [PATCH v2 2/2] misc: Add iop driver " Tony Huang @ 2021-12-03 9:12 ` kernel test robot 2021-12-03 10:38 ` Greg KH ` (3 subsequent siblings) 4 siblings, 0 replies; 19+ messages in thread From: kernel test robot @ 2021-12-03 9:12 UTC (permalink / raw) To: Tony Huang, derek.kiernan, dragan.cvetic, arnd, gregkh, linux-kernel, robh+dt, devicetree Cc: kbuild-all, wells.lu, tony.huang, Tony Huang Hi Tony, I love your patch! Yet something to improve: [auto build test ERROR on char-misc/char-misc-testing] [also build test ERROR on robh/for-next soc/for-next linus/master v5.16-rc3 next-20211202] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Tony-Huang/Add-iop-driver-for-Sunplus-SP7021/20211203-114932 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 5d331b5922551637c586cdf5fdc1778910fc937f config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20211203/202112031753.KIjg9ffN-lkp@intel.com/config) compiler: arceb-elf-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/5a13966416937e820ad198a487deb9308cb86061 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Tony-Huang/Add-iop-driver-for-Sunplus-SP7021/20211203-114932 git checkout 5a13966416937e820ad198a487deb9308cb86061 # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): In file included from drivers/misc/iop/sunplus_iop.c:10: >> drivers/misc/iop/sunplus_iop.h:5:10: fatal error: mach/io_map.h: No such file or directory 5 | #include <mach/io_map.h> | ^~~~~~~~~~~~~~~ compilation terminated. vim +5 drivers/misc/iop/sunplus_iop.h 2 3 #ifndef __SP_IOP_H__ 4 #define __SP_IOP_H__ > 5 #include <mach/io_map.h> 6 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 @ 2021-12-03 9:12 ` kernel test robot 0 siblings, 0 replies; 19+ messages in thread From: kernel test robot @ 2021-12-03 9:12 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 2166 bytes --] Hi Tony, I love your patch! Yet something to improve: [auto build test ERROR on char-misc/char-misc-testing] [also build test ERROR on robh/for-next soc/for-next linus/master v5.16-rc3 next-20211202] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Tony-Huang/Add-iop-driver-for-Sunplus-SP7021/20211203-114932 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 5d331b5922551637c586cdf5fdc1778910fc937f config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20211203/202112031753.KIjg9ffN-lkp(a)intel.com/config) compiler: arceb-elf-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/5a13966416937e820ad198a487deb9308cb86061 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Tony-Huang/Add-iop-driver-for-Sunplus-SP7021/20211203-114932 git checkout 5a13966416937e820ad198a487deb9308cb86061 # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): In file included from drivers/misc/iop/sunplus_iop.c:10: >> drivers/misc/iop/sunplus_iop.h:5:10: fatal error: mach/io_map.h: No such file or directory 5 | #include <mach/io_map.h> | ^~~~~~~~~~~~~~~ compilation terminated. vim +5 drivers/misc/iop/sunplus_iop.h 2 3 #ifndef __SP_IOP_H__ 4 #define __SP_IOP_H__ > 5 #include <mach/io_map.h> 6 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-03 3:48 ` [PATCH v2 2/2] misc: Add iop driver " Tony Huang 2021-12-03 9:12 ` kernel test robot @ 2021-12-03 10:38 ` Greg KH 2021-12-06 3:50 ` Tony Huang 黃懷厚 2021-12-03 10:39 ` Greg KH ` (2 subsequent siblings) 4 siblings, 1 reply; 19+ messages in thread From: Greg KH @ 2021-12-03 10:38 UTC (permalink / raw) To: Tony Huang Cc: derek.kiernan, dragan.cvetic, arnd, linux-kernel, robh+dt, devicetree, wells.lu, tony.huang On Fri, Dec 03, 2021 at 11:48:45AM +0800, Tony Huang wrote: > +#define NORMAL_CODE_MAX_SIZE 0X1000 > +#define STANDBY_CODE_MAX_SIZE 0x4000 > +unsigned char iop_normal_code[NORMAL_CODE_MAX_SIZE]; > +unsigned char iop_standby_code[STANDBY_CODE_MAX_SIZE]; Please make your local variables static so that yhou do not polute the kernel's global symbol table. > +/* ---------------------------------------------------------------------------------------------- */ > +resource_size_t SP_IOP_RESERVE_BASE; > +resource_size_t SP_IOP_RESERVE_SIZE; > +/* ---------------------------------------------------------------------------------------------- */ Again, static. And why the odd comment lines? And those are not good variable names. > +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; > +}; > +/***************************************************************** > + * G L O B A L D A T A > + ******************************************************************/ Global where? What about the ones above? :) > +static struct sp_iop *iop; Why do you think you only have one device in the system? Please do not use a single variable like this. It is easy to make your driver handle an unlimited number of devices just as easy as to handle 1 device. Please do that instead and hang your device-specific data off of the correct data structures that the driver core gives you. > + > +void iop_normal_mode(void) Did you run sparse on this code? Please do so. Also, no need for a .h file for a driver that only has one .c file. thanks, greg k-h ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-03 10:38 ` Greg KH @ 2021-12-06 3:50 ` Tony Huang 黃懷厚 2021-12-06 7:04 ` Greg KH 0 siblings, 1 reply; 19+ messages in thread From: Tony Huang 黃懷厚 @ 2021-12-06 3:50 UTC (permalink / raw) To: Greg KH, Tony Huang Cc: derek.kiernan, dragan.cvetic, arnd, linux-kernel, robh+dt, devicetree, Wells Lu 呂芳騰 Dear Greg KH: > -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Friday, December 3, 2021 6:39 PM > To: Tony Huang <tonyhuang.sunplus@gmail.com> > Cc: derek.kiernan@xilinx.com; dragan.cvetic@xilinx.com; arnd@arndb.de; > linux-kernel@vger.kernel.org; robh+dt@kernel.org; devicetree@vger.kernel.org; > Wells Lu 呂芳騰 <wells.lu@sunplus.com>; Tony Huang 黃懷厚 > <tony.huang@sunplus.com> > Subject: Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 > > On Fri, Dec 03, 2021 at 11:48:45AM +0800, Tony Huang wrote: > > +#define NORMAL_CODE_MAX_SIZE 0X1000 > > +#define STANDBY_CODE_MAX_SIZE 0x4000 > > +unsigned char iop_normal_code[NORMAL_CODE_MAX_SIZE]; > > +unsigned char iop_standby_code[STANDBY_CODE_MAX_SIZE]; > > Please make your local variables static so that yhou do not polute the kernel's > global symbol table. > I will modify it. > > +/* > > +--------------------------------------------------------------------- > > +------------------------- */ resource_size_t SP_IOP_RESERVE_BASE; > > +resource_size_t SP_IOP_RESERVE_SIZE; > > +/* > > +--------------------------------------------------------------------- > > +------------------------- */ > > Again, static. > > And why the odd comment lines? > I will remove it. > And those are not good variable names. > > > +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; > > +}; > > > +/************************************************************** > *** > > + * G L O B A L D A T A > > + > **************************************************************** > **/ > > Global where? What about the ones above? :) > I will remove it. > > +static struct sp_iop *iop; > > Why do you think you only have one device in the system? Please do not use > a single variable like this. It is easy to make your driver handle an unlimited > number of devices just as easy as to handle 1 device. > Please do that instead and hang your device-specific data off of the correct > data structures that the driver core gives you. > I will modify it. > > + > > +void iop_normal_mode(void) > > Did you run sparse on this code? Please do so. > > Also, no need for a .h file for a driver that only has one .c file. > I need to keep sunplus_iop.h. Other files will use sp_iop_platform_driver_poweroff(void) in poweroff flow. > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-06 3:50 ` Tony Huang 黃懷厚 @ 2021-12-06 7:04 ` Greg KH 2021-12-06 15:02 ` Tony Huang 黃懷厚 0 siblings, 1 reply; 19+ messages in thread From: Greg KH @ 2021-12-06 7:04 UTC (permalink / raw) To: Tony Huang 黃懷厚 Cc: Tony Huang, derek.kiernan, dragan.cvetic, arnd, linux-kernel, robh+dt, devicetree, Wells Lu 呂芳騰 On Mon, Dec 06, 2021 at 03:50:03AM +0000, Tony Huang 黃懷厚 wrote: > > Also, no need for a .h file for a driver that only has one .c file. > > > > I need to keep sunglus_iop.h. Other files will use > sp_iop_platform_driver_poweroff(void) in poweroff flow. What other files? That is not included here, nor should other drivers be making that call, use the normal poweroff logic. thanks, greg k-h ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-06 7:04 ` Greg KH @ 2021-12-06 15:02 ` Tony Huang 黃懷厚 0 siblings, 0 replies; 19+ messages in thread From: Tony Huang 黃懷厚 @ 2021-12-06 15:02 UTC (permalink / raw) To: Greg KH Cc: Tony Huang, derek.kiernan, dragan.cvetic, arnd, linux-kernel, robh+dt, devicetree, Wells Lu 呂芳騰 Dear Greg KH: > > > Also, no need for a .h file for a driver that only has one .c file. > > > > > > > I need to keep sunglus_iop.h. Other files will use > > sp_iop_platform_driver_poweroff(void) in poweroff flow. > > What other files? That is not included here, nor should other drivers be > making that call, use the normal poweroff logic. > Okay, I will call the sp_iop_platform_driver_poweroff(void) through normal poweroff logic. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-03 3:48 ` [PATCH v2 2/2] misc: Add iop driver " Tony Huang 2021-12-03 9:12 ` kernel test robot 2021-12-03 10:38 ` Greg KH @ 2021-12-03 10:39 ` Greg KH 2021-12-06 6:48 ` Tony Huang 黃懷厚 2021-12-03 10:39 ` Greg KH 2021-12-03 12:17 ` Arnd Bergmann 4 siblings, 1 reply; 19+ messages in thread From: Greg KH @ 2021-12-03 10:39 UTC (permalink / raw) To: Tony Huang Cc: derek.kiernan, dragan.cvetic, arnd, linux-kernel, robh+dt, devicetree, wells.lu, tony.huang On Fri, Dec 03, 2021 at 11:48:45AM +0800, Tony Huang 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> > --- > Changes in v2: > - Addressed comments from Arnd Bergmann. > - Addressed comments from Greg KH. > - Addressed comments from kernel test robot. > > MAINTAINERS | 1 + > drivers/misc/Kconfig | 1 + > drivers/misc/Makefile | 1 + > drivers/misc/iop/Kconfig | 13 ++ > drivers/misc/iop/Makefile | 6 + Why do you need a subdirectory for a single .c file? thanks, greg k-h ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-03 10:39 ` Greg KH @ 2021-12-06 6:48 ` Tony Huang 黃懷厚 2021-12-06 8:06 ` Greg KH 0 siblings, 1 reply; 19+ messages in thread From: Tony Huang 黃懷厚 @ 2021-12-06 6:48 UTC (permalink / raw) To: Greg KH, Tony Huang Cc: derek.kiernan, dragan.cvetic, arnd, linux-kernel, robh+dt, devicetree, Wells Lu 呂芳騰 Dear Greg KH, Arnd: > -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Friday, December 3, 2021 6:39 PM > To: Tony Huang <tonyhuang.sunplus@gmail.com> > Cc: derek.kiernan@xilinx.com; dragan.cvetic@xilinx.com; arnd@arndb.de; > linux-kernel@vger.kernel.org; robh+dt@kernel.org; devicetree@vger.kernel.org; > Wells Lu 呂芳騰 <wells.lu@sunplus.com>; Tony Huang 黃懷厚 > <tony.huang@sunplus.com> > Subject: Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 > > On Fri, Dec 03, 2021 at 11:48:45AM +0800, Tony Huang 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> > > --- > > Changes in v2: > > - Addressed comments from Arnd Bergmann. > > - Addressed comments from Greg KH. > > - Addressed comments from kernel test robot. > > > > MAINTAINERS | 1 + > > drivers/misc/Kconfig | 1 + > > drivers/misc/Makefile | 1 + > > drivers/misc/iop/Kconfig | 13 ++ > > drivers/misc/iop/Makefile | 6 + > > Why do you need a subdirectory for a single .c file? > 1. Currently my bin file is placed in the root file system. I need to wait for the kernel booting to succeed before loading the bin code. 2. In addition, I need the kernel booting process to be able to mount the iop module and load bin file.I need to put bin file in /iop. Do you have a good way to load bin code during kernel booting? Thanks ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-06 6:48 ` Tony Huang 黃懷厚 @ 2021-12-06 8:06 ` Greg KH 2021-12-06 8:22 ` Tony Huang 黃懷厚 0 siblings, 1 reply; 19+ messages in thread From: Greg KH @ 2021-12-06 8:06 UTC (permalink / raw) To: Tony Huang 黃懷厚 Cc: Tony Huang, derek.kiernan, dragan.cvetic, arnd, linux-kernel, robh+dt, devicetree, Wells Lu 呂芳騰 On Mon, Dec 06, 2021 at 06:48:46AM +0000, Tony Huang 黃懷厚 wrote: > Dear Greg KH, Arnd: > > > -----Original Message----- > > From: Greg KH <gregkh@linuxfoundation.org> > > Sent: Friday, December 3, 2021 6:39 PM > > To: Tony Huang <tonyhuang.sunplus@gmail.com> > > Cc: derek.kiernan@xilinx.com; dragan.cvetic@xilinx.com; arnd@arndb.de; > > linux-kernel@vger.kernel.org; robh+dt@kernel.org; devicetree@vger.kernel.org; > > Wells Lu 呂芳騰 <wells.lu@sunplus.com>; Tony Huang 黃懷厚 > > <tony.huang@sunplus.com> > > Subject: Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 > > > > On Fri, Dec 03, 2021 at 11:48:45AM +0800, Tony Huang 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> > > > --- > > > Changes in v2: > > > - Addressed comments from Arnd Bergmann. > > > - Addressed comments from Greg KH. > > > - Addressed comments from kernel test robot. > > > > > > MAINTAINERS | 1 + > > > drivers/misc/Kconfig | 1 + > > > drivers/misc/Makefile | 1 + > > > drivers/misc/iop/Kconfig | 13 ++ > > > drivers/misc/iop/Makefile | 6 + > > > > Why do you need a subdirectory for a single .c file? > > > > 1. Currently my bin file is placed in the root file system. I need to > wait for the kernel booting to succeed before loading the bin code. What "bin file"? > 2. In addition, I need the kernel booting process to be able to mount > the iop module and load bin file.I need to put bin file in /iop. That is an odd directory location. > Do you have a good way to load bin code during kernel booting? Is this firmware? Put it in the normal location for firware that the kernel expects to see. thanks, greg k-h ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-06 8:06 ` Greg KH @ 2021-12-06 8:22 ` Tony Huang 黃懷厚 2021-12-06 8:29 ` Greg KH 0 siblings, 1 reply; 19+ messages in thread From: Tony Huang 黃懷厚 @ 2021-12-06 8:22 UTC (permalink / raw) To: Greg KH Cc: Tony Huang, derek.kiernan, dragan.cvetic, arnd, linux-kernel, robh+dt, devicetree, Wells Lu 呂芳騰 Dear Greg KH: > -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Monday, December 6, 2021 4:07 PM > To: Tony Huang 黃懷厚 <tony.huang@sunplus.com> > Cc: Tony Huang <tonyhuang.sunplus@gmail.com>; derek.kiernan@xilinx.com; > dragan.cvetic@xilinx.com; arnd@arndb.de; linux-kernel@vger.kernel.org; > robh+dt@kernel.org; devicetree@vger.kernel.org; Wells Lu 呂芳騰 > <wells.lu@sunplus.com> > Subject: Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 > > On Mon, Dec 06, 2021 at 06:48:46AM +0000, Tony Huang 黃懷厚 wrote: > > Dear Greg KH, Arnd: > > > > > -----Original Message----- > > > From: Greg KH <gregkh@linuxfoundation.org> > > > Sent: Friday, December 3, 2021 6:39 PM > > > To: Tony Huang <tonyhuang.sunplus@gmail.com> > > > Cc: derek.kiernan@xilinx.com; dragan.cvetic@xilinx.com; > > > arnd@arndb.de; linux-kernel@vger.kernel.org; robh+dt@kernel.org; > > > devicetree@vger.kernel.org; Wells Lu 呂芳騰 <wells.lu@sunplus.com>; > > > Tony Huang 黃懷厚 > > > <tony.huang@sunplus.com> > > > Subject: Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 > > > > > > On Fri, Dec 03, 2021 at 11:48:45AM +0800, Tony Huang 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> > > > > --- > > > > Changes in v2: > > > > - Addressed comments from Arnd Bergmann. > > > > - Addressed comments from Greg KH. > > > > - Addressed comments from kernel test robot. > > > > > > > > MAINTAINERS | 1 + > > > > drivers/misc/Kconfig | 1 + > > > > drivers/misc/Makefile | 1 + > > > > drivers/misc/iop/Kconfig | 13 ++ > > > > drivers/misc/iop/Makefile | 6 + > > > > > > Why do you need a subdirectory for a single .c file? > > > > > > > 1. Currently my bin file is placed in the root file system. I need to > > wait for the kernel booting to succeed before loading the bin code. > > What "bin file"? > IOP MODULE EXECUTES 8051 CODE Source code should reserve SDRAM memory area for IOP module code. 8051 bin file normal code and standby code can be placed in this area. The location area can be select by user. Normal code: Monitor CPU commands. Standby code: For RTC wake up, cooperation with CPU&PMC in power management When the system enters standby mode, 8051 bin file should be moved to I_Cache. I_Cache has 16K only. Standby code cannot exceed 16K. When the IOP module is mounted, CPU load 8051 codes (normal.bin) into memory. Iop_base_addr_l and iop_base_addr_h specify address. During system boot up, when the IOP is mounted, it will load 8051 normal code and start execute 8051 code. > > 2. In addition, I need the kernel booting process to be able to mount > > the iop module and load bin file.I need to put bin file in /iop. > > That is an odd directory location. > > > Do you have a good way to load bin code during kernel booting? > > Is this firmware? Put it in the normal location for firware that the kernel > expects to see. > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-06 8:22 ` Tony Huang 黃懷厚 @ 2021-12-06 8:29 ` Greg KH 0 siblings, 0 replies; 19+ messages in thread From: Greg KH @ 2021-12-06 8:29 UTC (permalink / raw) To: Tony Huang 黃懷厚 Cc: Tony Huang, derek.kiernan, dragan.cvetic, arnd, linux-kernel, robh+dt, devicetree, Wells Lu 呂芳騰 On Mon, Dec 06, 2021 at 08:22:01AM +0000, Tony Huang 黃懷厚 wrote: > Dear Greg KH: > > > -----Original Message----- > > From: Greg KH <gregkh@linuxfoundation.org> > > Sent: Monday, December 6, 2021 4:07 PM > > To: Tony Huang 黃懷厚 <tony.huang@sunplus.com> > > Cc: Tony Huang <tonyhuang.sunplus@gmail.com>; derek.kiernan@xilinx.com; > > dragan.cvetic@xilinx.com; arnd@arndb.de; linux-kernel@vger.kernel.org; > > robh+dt@kernel.org; devicetree@vger.kernel.org; Wells Lu 呂芳騰 > > <wells.lu@sunplus.com> > > Subject: Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 > > > > On Mon, Dec 06, 2021 at 06:48:46AM +0000, Tony Huang 黃懷厚 wrote: > > > Dear Greg KH, Arnd: > > > > > > > -----Original Message----- > > > > From: Greg KH <gregkh@linuxfoundation.org> > > > > Sent: Friday, December 3, 2021 6:39 PM > > > > To: Tony Huang <tonyhuang.sunplus@gmail.com> > > > > Cc: derek.kiernan@xilinx.com; dragan.cvetic@xilinx.com; > > > > arnd@arndb.de; linux-kernel@vger.kernel.org; robh+dt@kernel.org; > > > > devicetree@vger.kernel.org; Wells Lu 呂芳騰 <wells.lu@sunplus.com>; > > > > Tony Huang 黃懷厚 > > > > <tony.huang@sunplus.com> > > > > Subject: Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 > > > > > > > > On Fri, Dec 03, 2021 at 11:48:45AM +0800, Tony Huang 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> > > > > > --- > > > > > Changes in v2: > > > > > - Addressed comments from Arnd Bergmann. > > > > > - Addressed comments from Greg KH. > > > > > - Addressed comments from kernel test robot. > > > > > > > > > > MAINTAINERS | 1 + > > > > > drivers/misc/Kconfig | 1 + > > > > > drivers/misc/Makefile | 1 + > > > > > drivers/misc/iop/Kconfig | 13 ++ > > > > > drivers/misc/iop/Makefile | 6 + > > > > > > > > Why do you need a subdirectory for a single .c file? > > > > > > > > > > 1. Currently my bin file is placed in the root file system. I need to > > > wait for the kernel booting to succeed before loading the bin code. > > > > What "bin file"? > > > > IOP MODULE EXECUTES 8051 CODE > Source code should reserve SDRAM memory area for IOP module code. 8051 bin file normal code and standby code can be placed in this area. The location area can be select by user. > Normal code: Monitor CPU commands. > Standby code: For RTC wake up, cooperation with CPU&PMC in power management When the system enters standby mode, 8051 bin file should be moved to I_Cache. > I_Cache has 16K only. Standby code cannot exceed 16K. > When the IOP module is mounted, CPU load 8051 codes (normal.bin) into memory. > Iop_base_addr_l and iop_base_addr_h specify address. > During system boot up, when the IOP is mounted, it will load 8051 normal code and start execute 8051 code. So this is firmware, just put it in the normal firmware location. thanks, greg k-h ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-03 3:48 ` [PATCH v2 2/2] misc: Add iop driver " Tony Huang ` (2 preceding siblings ...) 2021-12-03 10:39 ` Greg KH @ 2021-12-03 10:39 ` Greg KH 2021-12-03 12:17 ` Arnd Bergmann 4 siblings, 0 replies; 19+ messages in thread From: Greg KH @ 2021-12-03 10:39 UTC (permalink / raw) To: Tony Huang Cc: derek.kiernan, dragan.cvetic, arnd, linux-kernel, robh+dt, devicetree, wells.lu, tony.huang On Fri, Dec 03, 2021 at 11:48:45AM +0800, Tony Huang wrote: > --- /dev/null > +++ b/drivers/misc/iop/sunplus_iop.c > @@ -0,0 +1,518 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later Are you sure about "or later"? I have to ask. Also, no copyright information on the file? Are you sure about that? thanks, greg k-h ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-03 3:48 ` [PATCH v2 2/2] misc: Add iop driver " Tony Huang ` (3 preceding siblings ...) 2021-12-03 10:39 ` Greg KH @ 2021-12-03 12:17 ` Arnd Bergmann 2021-12-06 3:42 ` Tony Huang 黃懷厚 4 siblings, 1 reply; 19+ messages in thread From: Arnd Bergmann @ 2021-12-03 12:17 UTC (permalink / raw) To: Tony Huang Cc: Derek Kiernan, Dragan Cvetic, Arnd Bergmann, gregkh, Linux Kernel Mailing List, Rob Herring, DTML, wells.lu, Tony Huang On Fri, Dec 3, 2021 at 4:48 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> > --- > Changes in v2: > - Addressed comments from Arnd Bergmann. > - Addressed comments from Greg KH. > - Addressed comments from kernel test robot. This looks much better already. > +#define NORMAL_CODE_MAX_SIZE 0X1000 > +#define STANDBY_CODE_MAX_SIZE 0x4000 > +unsigned char iop_normal_code[NORMAL_CODE_MAX_SIZE]; > +unsigned char iop_standby_code[STANDBY_CODE_MAX_SIZE]; I think these should be part of the sp_iop structure, not global variables. > +static struct sp_iop *iop; > + > +void iop_normal_mode(void) > +{ > + struct regs_iop *p_iop_reg = (struct regs_iop *)iop->iop_regs; local functions should generally be 'static' and take the 'struct sp_iop' instance pointer as the first argument. Functions that need to be global because they are used by other drivers (if any) should probably also be exported and have documentation above their definition. > +static int get_normal_code(struct device *dev) > +{ > + 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); The file name needs to clearly identify the device to avoid conflicts with other drivers. > +static int sp_iop_open(struct inode *inode, struct file *pfile) > +{ > + return 0; > +} > + > +static ssize_t sp_iop_read(struct file *pfile, char __user *ubuf, > + size_t length, loff_t *offset) > +{ > + return 0; > +} > + > +static ssize_t sp_iop_write(struct file *pfile, const char __user *ubuf, > + size_t length, loff_t *offset) > +{ > + return 0; > +} > + > +static int sp_iop_release(struct inode *inode, struct file *pfile) > +{ > + //dev_dbg(iop->dev, "Sunplus IOP module release\n"); > + return 0; > +} > + > +static const struct file_operations sp_iop_fops = { > + .owner = THIS_MODULE, > + .open = sp_iop_open, > + .read = sp_iop_read, > + .write = sp_iop_write, > + .release = sp_iop_release, > +}; This does nothing because all the callbacks are empty. You removed the inappropriate user space interfaces as I asked you to, but if there is no way for either kernel or user space to interact with the hardware, I don't see a point in merging the driver until you add a new interface that is usable. > +static int sp_iop_platform_driver_remove(struct platform_device *pdev) > +{ > + return 0; > +} > + > +static int sp_iop_platform_driver_suspend(struct platform_device *pdev, pm_message_t state) > +{ > + return 0; > +} > + > +static void sp_iop_platform_driver_shutdown(struct platform_device *pdev) > +{ > + > +} > + > +void sp_iop_platform_driver_poweroff(void) > +{ > + iop_standby_mode(); > + iop_shutdown(); > +} Something looks wrong here, maybe reread the documentation for runtime power management to find a way of putting the device into low-power mode when it is unused. > diff --git a/drivers/misc/iop/sunplus_iop.h b/drivers/misc/iop/sunplus_iop.h > new file mode 100644 > index 0000000..fcbfd26 > --- /dev/null > +++ b/drivers/misc/iop/sunplus_iop.h > @@ -0,0 +1,64 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later*/ > + > +#ifndef __SP_IOP_H__ > +#define __SP_IOP_H__ > +#include <mach/io_map.h> mach/io_map.h does not exist, so the driver won't compile. I don't think you need anything else, so it should be fine to remove the #include The rest of the header only describes the hardware itself, so I'd suggest merging all of it into the .c file. Arnd ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-03 12:17 ` Arnd Bergmann @ 2021-12-06 3:42 ` Tony Huang 黃懷厚 2021-12-06 8:04 ` Arnd Bergmann 0 siblings, 1 reply; 19+ messages in thread From: Tony Huang 黃懷厚 @ 2021-12-06 3:42 UTC (permalink / raw) To: Arnd Bergmann, Tony Huang Cc: Derek Kiernan, Dragan Cvetic, gregkh, Linux Kernel Mailing List, Rob Herring, DTML, Wells Lu 呂芳騰 Dear Arnd: > Subject: Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 > > On Fri, Dec 3, 2021 at 4:48 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> > > --- > > Changes in v2: > > - Addressed comments from Arnd Bergmann. > > - Addressed comments from Greg KH. > > - Addressed comments from kernel test robot. > > This looks much better already. > > > +#define NORMAL_CODE_MAX_SIZE 0X1000 > > +#define STANDBY_CODE_MAX_SIZE 0x4000 > > +unsigned char iop_normal_code[NORMAL_CODE_MAX_SIZE]; > > +unsigned char iop_standby_code[STANDBY_CODE_MAX_SIZE]; > > I think these should be part of the sp_iop structure, not global variables. > I will modify it. > > +static struct sp_iop *iop; > > + > > +void iop_normal_mode(void) > > +{ > > + struct regs_iop *p_iop_reg = (struct regs_iop *)iop->iop_regs; > > local functions should generally be 'static' and take the 'struct sp_iop' instance > pointer as the first argument. > > Functions that need to be global because they are used by other drivers (if any) > should probably also be exported and have documentation above their > definition. > I will mocify it. > > +static int get_normal_code(struct device *dev) { > > + 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); > > The file name needs to clearly identify the device to avoid conflicts with other > drivers. > I will moidfy it. > > +static int sp_iop_open(struct inode *inode, struct file *pfile) { > > + return 0; > > +} > > + > > +static ssize_t sp_iop_read(struct file *pfile, char __user *ubuf, > > + size_t length, loff_t *offset) { > > + return 0; > > +} > > + > > +static ssize_t sp_iop_write(struct file *pfile, const char __user *ubuf, > > + size_t length, loff_t *offset) { > > + return 0; > > +} > > + > > +static int sp_iop_release(struct inode *inode, struct file *pfile) { > > + //dev_dbg(iop->dev, "Sunplus IOP module release\n"); > > + return 0; > > +} > > + > > +static const struct file_operations sp_iop_fops = { > > + .owner = THIS_MODULE, > > + .open = sp_iop_open, > > + .read = sp_iop_read, > > + .write = sp_iop_write, > > + .release = sp_iop_release, > > +}; > > This does nothing because all the callbacks are empty. You removed the > inappropriate user space interfaces as I asked you to, but if there is no way for > either kernel or user space to interact with the hardware, I don't see a point in > merging the driver until you add a new interface that is usable. > I will modify sp_iop_read() to monitor IOP mailbox data. > > +static int sp_iop_platform_driver_remove(struct platform_device > > +*pdev) { > > + return 0; > > +} > > + > > +static int sp_iop_platform_driver_suspend(struct platform_device > > +*pdev, pm_message_t state) { > > + return 0; > > +} > > + > > +static void sp_iop_platform_driver_shutdown(struct platform_device > > +*pdev) { > > + > > +} > > + > > +void sp_iop_platform_driver_poweroff(void) > > +{ > > + iop_standby_mode(); > > + iop_shutdown(); > > +} > > Something looks wrong here, maybe reread the documentation for runtime > power management to find a way of putting the device into low-power mode > when it is unused. > When the poweroff command is executed, the run sp_iop_platform_driver_poweroff(void) function will enter the standby mode. The power off will be executed. In the system, IOP can continue to work when other modules in the system enter standby / power down modes to monitor whether the system wakes up through RTC. 26.4.2 IOP MODULE EXECUTES 8051 CODE Source code should reserve SDRAM memory area for IOP module code. 8051 bin file normal code and standby code can be placed in this area. The location area can be select by user. Normal code: Monitor CPU commands. Standby code: For RTC wake up, cooperation with CPU&PMC in power management When the system enters standby mode, 8051 bin file should be moved to I_Cache. I_Cache has 16K only. Standby code cannot exceed 16K. When the IOP module is mounted, CPU load 8051 codes (normal.bin) into memory. Iop_base_addr_l and iop_base_addr_h specify address. During system boot up, when the IOP is mounted, it will load 8051 normal code and start execute 8051 code. In the system, IOP can continue to work when other modules in the system enter standby / power down modes to monitor whether the system wakes up through RTC. In such a situation, the IOP will not be able to access the system memory. > > diff --git a/drivers/misc/iop/sunplus_iop.h > > b/drivers/misc/iop/sunplus_iop.h new file mode 100644 index > > 0000000..fcbfd26 > > --- /dev/null > > +++ b/drivers/misc/iop/sunplus_iop.h > > @@ -0,0 +1,64 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later*/ > > + > > +#ifndef __SP_IOP_H__ > > +#define __SP_IOP_H__ > > +#include <mach/io_map.h> > > mach/io_map.h does not exist, so the driver won't compile. I don't think you > need anything else, so it should be fine to remove the #include > I will remove io_map.h file. > The rest of the header only describes the hardware itself, so I'd suggest > merging all of it into the .c file. > I need to keep sunplus_iop.h. Other files will use sp_iop_platform_driver_poweroff(void) in poweroff flow. When the poweroff command is executed, the run sp_iop_platform_driver_poweroff(void) function will enter the standby mode. The power off will be executed. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-06 3:42 ` Tony Huang 黃懷厚 @ 2021-12-06 8:04 ` Arnd Bergmann 2021-12-06 8:23 ` Tony Huang 黃懷厚 0 siblings, 1 reply; 19+ messages in thread From: Arnd Bergmann @ 2021-12-06 8:04 UTC (permalink / raw) To: Tony Huang 黃懷厚 Cc: Arnd Bergmann, Tony Huang, Derek Kiernan, Dragan Cvetic, gregkh, Linux Kernel Mailing List, Rob Herring, DTML, Wells Lu 呂芳騰 On Mon, Dec 6, 2021 at 4:42 AM Tony Huang 黃懷厚 <tony.huang@sunplus.com> wrote: > > Subject: Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 > > On Fri, Dec 3, 2021 at 4:48 AM Tony Huang <tonyhuang.sunplus@gmail.com> wrote: > > > + > > > +static const struct file_operations sp_iop_fops = { > > > + .owner = THIS_MODULE, > > > + .open = sp_iop_open, > > > + .read = sp_iop_read, > > > + .write = sp_iop_write, > > > + .release = sp_iop_release, > > > +}; > > > > This does nothing because all the callbacks are empty. You removed the > > inappropriate user space interfaces as I asked you to, but if there is no way for > > either kernel or user space to interact with the hardware, I don't see a point in > > merging the driver until you add a new interface that is usable. > > > > I will modify sp_iop_read() to monitor IOP mailbox data. Why is this a useful interface to have? If this is only for debugging, a tracepoint may be more useful than a full character device. > > Something looks wrong here, maybe reread the documentation for runtime > > power management to find a way of putting the device into low-power mode > > when it is unused. > > > > When the poweroff command is executed, the run sp_iop_platform_driver_poweroff(void) > function will enter the standby mode. The power off will be executed. > In the system, IOP can continue to work when other modules in the system enter > standby / power down modes to monitor whether the system wakes up through RTC. Ok, in that case you can probably just remove the empty callback functions. Arnd ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 2021-12-06 8:04 ` Arnd Bergmann @ 2021-12-06 8:23 ` Tony Huang 黃懷厚 0 siblings, 0 replies; 19+ messages in thread From: Tony Huang 黃懷厚 @ 2021-12-06 8:23 UTC (permalink / raw) To: Arnd Bergmann Cc: Tony Huang, Derek Kiernan, Dragan Cvetic, gregkh, Linux Kernel Mailing List, Rob Herring, DTML, Wells Lu 呂芳騰 Dear Arnd: > -----Original Message----- > From: Arnd Bergmann <arnd@arndb.de> > Sent: Monday, December 6, 2021 4:04 PM > To: Tony Huang 黃懷厚 <tony.huang@sunplus.com> > Cc: Arnd Bergmann <arnd@arndb.de>; Tony Huang > <tonyhuang.sunplus@gmail.com>; Derek Kiernan <derek.kiernan@xilinx.com>; > Dragan Cvetic <dragan.cvetic@xilinx.com>; gregkh > <gregkh@linuxfoundation.org>; Linux Kernel Mailing List > <linux-kernel@vger.kernel.org>; Rob Herring <robh+dt@kernel.org>; DTML > <devicetree@vger.kernel.org>; Wells Lu 呂芳騰 <wells.lu@sunplus.com> > Subject: Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 > > On Mon, Dec 6, 2021 at 4:42 AM Tony Huang 黃懷厚 > <tony.huang@sunplus.com> wrote: > > > Subject: Re: [PATCH v2 2/2] misc: Add iop driver for Sunplus SP7021 > > > On Fri, Dec 3, 2021 at 4:48 AM Tony Huang > <tonyhuang.sunplus@gmail.com> wrote: > > > > + > > > > +static const struct file_operations sp_iop_fops = { > > > > + .owner = THIS_MODULE, > > > > + .open = sp_iop_open, > > > > + .read = sp_iop_read, > > > > + .write = sp_iop_write, > > > > + .release = sp_iop_release, > > > > +}; > > > > > > This does nothing because all the callbacks are empty. You removed > > > the inappropriate user space interfaces as I asked you to, but if > > > there is no way for either kernel or user space to interact with the > > > hardware, I don't see a point in merging the driver until you add a new > interface that is usable. > > > > > > > I will modify sp_iop_read() to monitor IOP mailbox data. > > Why is this a useful interface to have? If this is only for debugging, a tracepoint > may be more useful than a full character device. > Yes, it is for debugging. > > > Something looks wrong here, maybe reread the documentation for > > > runtime power management to find a way of putting the device into > > > low-power mode when it is unused. > > > > > > > When the poweroff command is executed, the run > > sp_iop_platform_driver_poweroff(void) > > function will enter the standby mode. The power off will be executed. > > In the system, IOP can continue to work when other modules in the > > system enter standby / power down modes to monitor whether the system > wakes up through RTC. > > Ok, in that case you can probably just remove the empty callback functions. > > Arnd ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2021-12-06 15:04 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-03 3:48 [PATCH v2 0/2] Add iop driver for Sunplus SP7021 Tony Huang 2021-12-03 3:48 ` [PATCH v2 1/2] dt-binding: misc: Add iop yaml file " Tony Huang 2021-12-03 3:48 ` [PATCH v2 2/2] misc: Add iop driver " Tony Huang 2021-12-03 9:12 ` kernel test robot 2021-12-03 9:12 ` kernel test robot 2021-12-03 10:38 ` Greg KH 2021-12-06 3:50 ` Tony Huang 黃懷厚 2021-12-06 7:04 ` Greg KH 2021-12-06 15:02 ` Tony Huang 黃懷厚 2021-12-03 10:39 ` Greg KH 2021-12-06 6:48 ` Tony Huang 黃懷厚 2021-12-06 8:06 ` Greg KH 2021-12-06 8:22 ` Tony Huang 黃懷厚 2021-12-06 8:29 ` Greg KH 2021-12-03 10:39 ` Greg KH 2021-12-03 12:17 ` Arnd Bergmann 2021-12-06 3:42 ` Tony Huang 黃懷厚 2021-12-06 8:04 ` Arnd Bergmann 2021-12-06 8:23 ` 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.