Linaro-mm-sig August 2024

linaro-mm-sig@lists.linaro.org

14 participants
29 discussions

Re: [PATCH v2 1/4] udmabuf: cancel mmap page fault, direct map it

by Christian König

Am 12.08.24 um 04:49 schrieb Huan Yang: > > 在 2024/8/10 9:28, Kasireddy, Vivek 写道: >> [Some people who received this message don't often get email from >> vivek.kasireddy(a)intel.com. Learn why this is important at >> https://aka.ms/LearnAboutSenderIdentification ] >> >> Hi Huan, >> >>> The current udmabuf mmap uses a page fault mechanism to populate the >>> vma. >>> >>> However, the current udmabuf has already obtained and pinned the folio >>> upon completion of the creation.This means that the physical memory has >>> already been acquired, rather than being accessed dynamically. The >>> current page fault method only saves some page table memory. >>> >>> As a result, the page fault mechanism has lost its purpose as a >>> demanding >>> page. Due to the fact that page fault requires trapping into kernel >>> mode >>> and filling in when accessing the corresponding virtual address in >>> mmap, >>> this means that user mode access to virtual addresses needs to trap >>> into >>> kernel mode. >>> >>> Therefore, when creating a large size udmabuf, this represents a >>> considerable overhead. >>> >>> The current patch removes the page fault method of mmap and >>> instead fills it directly when mmap is triggered. >> I think it makes sense to populate the vma when the first fault is >> triggered >> instead of doing it during mmap. This is because the userspace may call >> mmap but does not actually use the data. Qemu works this way >> depending on > Yes, the idea of this is also related to the concept of page fault. > > However, the folio has already been pinned during creation. I think > using the page fault > > again is theoretically sound, but it may not save memory, only > increase context switch overhead. This is not about saving memory but rather correctness and desired handling. A mmap() operation is for creating the VMA and *not* filling the page tables. That might work but is not really a desired approach. Regards, Christian. > > >> whether opengl is available in the environment or not. >> >>> Signed-off-by: Huan Yang <link(a)vivo.com> >>> --- >>> drivers/dma-buf/udmabuf.c | 39 >>> ++++++++++++++++----------------------- >>> 1 file changed, 16 insertions(+), 23 deletions(-) >>> >>> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c >>> index 047c3cd2ceff..475268d4ebb1 100644 >>> --- a/drivers/dma-buf/udmabuf.c >>> +++ b/drivers/dma-buf/udmabuf.c >>> @@ -38,36 +38,29 @@ struct udmabuf_folio { >>> struct list_head list; >>> }; >>> >>> -static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) >>> -{ >>> - struct vm_area_struct *vma = vmf->vma; >>> - struct udmabuf *ubuf = vma->vm_private_data; >>> - pgoff_t pgoff = vmf->pgoff; >>> - unsigned long pfn; >>> - >>> - if (pgoff >= ubuf->pagecount) >>> - return VM_FAULT_SIGBUS; >>> - >>> - pfn = folio_pfn(ubuf->folios[pgoff]); >>> - pfn += ubuf->offsets[pgoff] >> PAGE_SHIFT; >>> - >>> - return vmf_insert_pfn(vma, vmf->address, pfn); >>> -} >>> - >>> -static const struct vm_operations_struct udmabuf_vm_ops = { >>> - .fault = udmabuf_vm_fault, >>> -}; >>> - >>> static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct >>> *vma) >>> { >>> struct udmabuf *ubuf = buf->priv; >>> + unsigned long addr; >>> + unsigned long end; >>> + unsigned long pgoff; >>> + int ret; >>> >>> if ((vma->vm_flags & (VM_SHARED | VM_MAYSHARE)) == 0) >>> return -EINVAL; >>> >>> - vma->vm_ops = &udmabuf_vm_ops; >>> - vma->vm_private_data = ubuf; >>> - vm_flags_set(vma, VM_PFNMAP | VM_DONTEXPAND | >>> VM_DONTDUMP); >>> + for (pgoff = vma->vm_pgoff, end = vma->vm_end, addr = vma- >>>> vm_start; >>> + addr < end; pgoff++, addr += PAGE_SIZE) { >>> + struct page *page = >>> + folio_page(ubuf->folios[pgoff], >>> + ubuf->offsets[pgoff] >> PAGE_SHIFT); >> Please don't use struct page pointers, given the recent conversion to >> use >> only folios in udmabuf driver. I think what you are trying to do >> above can >> be done using only folios. > Yes, just use pfn. Consider of HVO, must use this. >> >>> + >>> + ret = remap_pfn_range(vma, addr, page_to_pfn(page), >>> PAGE_SIZE, >>> + vma->vm_page_prot); >> Could you please retain the use of vmf_insert_pfn() here, given the >> simplicity, >> among other reasons? > I will make the correction. > > Thanks. >> >> Thanks, >> Vivek >> >>> + if (ret) >>> + return ret; >>> + } >>> + >>> return 0; >>> } >>> >>> -- >>> 2.45.2

4 months

Re: [PATCH v2 2/2] mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC

by kernel test robot

Hi Hui-Ping, kernel test robot noticed the following build warnings: [auto build test WARNING on mtd/nand/next] [also build test WARNING on linus/master v6.11-rc4 next-20240820] [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#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Hui-Ping-Chen/dt-bindings-mt… base: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next patch link: https://lore.kernel.org/r/20240819092037.110260-3-hpchen0nvt%40gmail.com patch subject: [PATCH v2 2/2] mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC config: parisc-randconfig-r123-20240820 (https://download.01.org/0day-ci/archive/20240820/202408201329.v0SoF3eS-lkp@…) compiler: hppa-linux-gcc (GCC) 14.1.0 reproduce: (https://download.01.org/0day-ci/archive/20240820/202408201329.v0SoF3eS-lkp@…) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp(a)intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408201329.v0SoF3eS-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:231:20: sparse: sparse: cast removes address space '__iomem' of expression >> drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:736:20: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected unsigned char [usertype] *ptr @@ got unsigned char [noderef] [usertype] __iomem * @@ drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:736:20: sparse: expected unsigned char [usertype] *ptr drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:736:20: sparse: got unsigned char [noderef] [usertype] __iomem * >> drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:764:22: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected char *ptr @@ got char [noderef] __iomem * @@ drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:764:22: sparse: expected char *ptr drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:764:22: sparse: got char [noderef] __iomem * drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:803:22: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected char *ptr @@ got char [noderef] __iomem * @@ drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:803:22: sparse: expected char *ptr drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:803:22: sparse: got char [noderef] __iomem * drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:506:28: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected unsigned char [usertype] *ptr @@ got unsigned char [noderef] [usertype] __iomem * @@ drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:506:28: sparse: expected unsigned char [usertype] *ptr drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:506:28: sparse: got unsigned char [noderef] [usertype] __iomem * drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:506:28: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected unsigned char [usertype] *ptr @@ got unsigned char [noderef] [usertype] __iomem * @@ drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:506:28: sparse: expected unsigned char [usertype] *ptr drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:506:28: sparse: got unsigned char [noderef] [usertype] __iomem * drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:506:28: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected unsigned char [usertype] *ptr @@ got unsigned char [noderef] [usertype] __iomem * @@ drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:506:28: sparse: expected unsigned char [usertype] *ptr drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:506:28: sparse: got unsigned char [noderef] [usertype] __iomem * drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:506:28: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected unsigned char [usertype] *ptr @@ got unsigned char [noderef] [usertype] __iomem * @@ drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:506:28: sparse: expected unsigned char [usertype] *ptr drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:506:28: sparse: got unsigned char [noderef] [usertype] __iomem * vim +736 drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c 731 732 static int ma35_nand_write_page_hwecc(struct nand_chip *chip, const u8 *buf, 733 int oob_required, int page) 734 { 735 struct ma35_nand_info *nand = nand_get_controller_data(chip); > 736 u8 *ptr = (u8 __iomem *)(nand->regs + MA35_NFI_REG_NANDRA0); 737 struct mtd_info *mtd = nand_to_mtd(chip); 738 u8 *ecc_calc = chip->ecc.calc_buf; 739 740 memset((void *)ptr, 0xFF, mtd->oobsize); 741 memcpy((void *)ptr, (void *)chip->oob_poi, mtd->oobsize - chip->ecc.total); 742 743 ma35_nand_command(chip, NAND_CMD_SEQIN, 0, page); 744 ma35_nand_dma_transfer(chip, buf, mtd->writesize, 0x1); 745 ma35_nand_command(chip, NAND_CMD_PAGEPROG, -1, -1); 746 ma35_waitfunc(chip); 747 748 /* Copy parity code in NANDRA to calc */ 749 memcpy((void *)ecc_calc, 750 (void *)(ptr + (mtd->oobsize - chip->ecc.total)), 751 chip->ecc.total); 752 753 /* Copy parity code in calc to oob_poi */ 754 memcpy((void *)(chip->oob_poi + (mtd->oobsize - chip->ecc.total)), 755 (void *)ecc_calc, chip->ecc.total); 756 757 return 0; 758 } 759 760 static int ma35_nand_read_page_hwecc_oob_first(struct nand_chip *chip, u8 *buf, 761 int oob_required, int page) 762 { 763 struct ma35_nand_info *nand = nand_get_controller_data(chip); > 764 char *ptr = (char __iomem *)(nand->regs + MA35_NFI_REG_NANDRA0); 765 struct mtd_info *mtd = nand_to_mtd(chip); 766 767 /* 1. read the OOB area */ 768 ma35_nand_command(chip, NAND_CMD_READOOB, 0, page); 769 ma35_nand_read_buf(chip, chip->oob_poi, mtd->oobsize); 770 771 /* 2. copy OOB data to NANDRA for page read */ 772 memcpy((void *)ptr, (void *)chip->oob_poi, mtd->oobsize); 773 774 if ((*(ptr+2) != 0) && (*(ptr+3) != 0)) 775 memset((void *)buf, 0xff, mtd->writesize); 776 else { 777 /* 3. read data from nand */ 778 ma35_nand_command(chip, NAND_CMD_READ0, 0, page); 779 ma35_nand_dma_transfer(chip, buf, mtd->writesize, 0x0); 780 781 /* 4. restore OOB data from SMRA */ 782 memcpy((void *)chip->oob_poi, (void *)ptr, mtd->oobsize); 783 } 784 785 return 0; 786 } 787 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

4 months

Re: [PATCH v2 2/2] mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC

by Krzysztof Kozlowski

On Mon, Aug 19, 2024 at 09:20:37AM +0000, Hui-Ping Chen wrote: > Nuvoton MA35 SoCs NAND Flash Interface Controller > supports 2KB, 4KB and 8KB page size, and up to 8-bit, > 12-bit, and 24-bit hardware ECC calculation circuit > to protect data communication. > > Signed-off-by: Hui-Ping Chen <hpchen0nvt(a)gmail.com> ... > +static int ma35_nand_probe(struct platform_device *pdev) > +{ > + struct ma35_nand_info *nand; > + struct nand_chip *chip; > + struct mtd_info *mtd; > + int retval = 0; > + > + nand = devm_kzalloc(&pdev->dev, sizeof(*nand), GFP_KERNEL); > + if (!nand) > + return -ENOMEM; > + > + nand_controller_init(&nand->controller); > + > + nand->regs = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(nand->regs)) > + return PTR_ERR(nand->regs); > + > + nand->dev = &pdev->dev; > + chip = &nand->chip; > + mtd = nand_to_mtd(chip); > + nand_set_controller_data(chip, nand); > + nand_set_flash_node(chip, pdev->dev.of_node); > + > + mtd->priv = chip; > + mtd->owner = THIS_MODULE; > + mtd->dev.parent = &pdev->dev; > + > + nand->clk = devm_clk_get(&pdev->dev, "nand_gate"); > + if (IS_ERR(nand->clk)) > + return dev_err_probe(&pdev->dev, PTR_ERR(nand->clk), > + "failed to find nand clock\n"); > + > + retval = clk_prepare_enable(nand->clk); > + if (retval < 0) { > + dev_err(&pdev->dev, "failed to enable clock\n"); > + retval = -ENXIO; > + } > + > + nand->chip.controller = &nand->controller; > + > + chip->legacy.cmdfunc = ma35_nand_command; > + chip->legacy.waitfunc = ma35_waitfunc; > + chip->legacy.read_byte = ma35_nand_read_byte; > + chip->legacy.select_chip = ma35_nand_select_chip; > + chip->legacy.read_buf = ma35_read_buf_dma; > + chip->legacy.write_buf = ma35_write_buf_dma; > + chip->legacy.dev_ready = ma35_nand_devready; > + chip->legacy.chip_delay = 25; /* us */ > + > + /* Read OOB data first, then HW read page */ > + chip->ecc.hwctl = ma35_nand_enable_hwecc; > + chip->ecc.calculate = ma35_nand_calculate_ecc; > + chip->ecc.correct = ma35_nand_correct_data; > + chip->ecc.write_page = ma35_nand_write_page_hwecc; > + chip->ecc.read_page = ma35_nand_read_page_hwecc_oob_first; > + chip->ecc.read_oob = ma35_nand_read_oob_hwecc; > + chip->options |= (NAND_NO_SUBPAGE_WRITE | NAND_USES_DMA); > + > + ma35_nand_initialize(nand); > + platform_set_drvdata(pdev, nand); > + > + nand->controller.ops = &ma35_nand_controller_ops; > + > + nand->irq = platform_get_irq(pdev, 0); > + if (nand->irq < 0) > + return dev_err_probe(&pdev->dev, nand->irq, > + "failed to get platform irq\n"); > + > + if (request_irq(nand->irq, ma35_nand_irq, IRQF_TRIGGER_HIGH, "ma35d1-nand", nand)) { > + dev_err(&pdev->dev, "Error requesting NAND IRQ\n"); > + return -ENXIO; > + } > + > + retval = nand_scan(chip, 1); > + if (retval) > + return retval; > + > + if (mtd_device_register(mtd, nand->parts, nand->nr_parts)) { > + nand_cleanup(chip); > + devm_kfree(&pdev->dev, nand); > + return retval; > + } > + > + return retval; > +} > + > +static void ma35_nand_remove(struct platform_device *pdev) > +{ > + struct ma35_nand_info *nand = platform_get_drvdata(pdev); > + struct nand_chip *chip = &nand->chip; > + int ret; > + Where do you release IRQ handler? > + ret = mtd_device_unregister(nand_to_mtd(chip)); > + WARN_ON(ret); > + nand_cleanup(chip); > + > + clk_disable_unprepare(nand->clk); > + > + kfree(nand); NAK, you never tested your code. > + platform_set_drvdata(pdev, NULL); Why? Drop. > +} > + > +/* PM Support */ > +#ifdef CONFIG_PM > +static int ma35_nand_suspend(struct platform_device *pdev, pm_message_t pm) > +{ > + struct ma35_nand_info *nand = platform_get_drvdata(pdev); > + unsigned long timeo = jiffies + HZ/2; > + > + /* wait DMAC to ready */ > + while (1) { > + if ((readl(nand->regs + MA35_NFI_REG_DMACTL) & DMA_BUSY) == 0) > + break; > + if (time_after(jiffies, timeo)) > + return -ETIMEDOUT; > + } > + > + clk_disable(nand->clk); > + > + return 0; > +} > + > +static int ma35_nand_resume(struct platform_device *pdev) > +{ > + struct ma35_nand_info *nand = platform_get_drvdata(pdev); > + > + clk_enable(nand->clk); > + ma35_nand_hwecc_init(nand); > + ma35_nand_dmac_init(nand); > + > + return 0; > +} > + > +#else > +#define ma35_nand_suspend NULL > +#define ma35_nand_resume NULL > +#endif > + > +static const struct of_device_id ma35_nfi_of_match[] = { > + { .compatible = "nuvoton,ma35d1-nand" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, ma35_nfi_of_match); > + > +static struct platform_driver ma35_nand_driver = { > + .driver = { Messed indentation. > + .name = "ma35d1-nand", > + .owner = THIS_MODULE, Drop. Please do not upstream some 10 year old code... Use recent code as template, not 10yo stuff... Best regards, Krzysztof

4 months

Re: [PATCH v2 1/2] dt-bindings: mtd: nuvoton,ma35d1-nand: add new bindings

by Krzysztof Kozlowski

On Mon, Aug 19, 2024 at 09:20:36AM +0000, Hui-Ping Chen wrote: > Add dt-bindings for the Nuvoton MA35 SoC NAND Controller. > > Signed-off-by: Hui-Ping Chen <hpchen0nvt(a)gmail.com> > --- > .../bindings/mtd/nuvoton,ma35d1-nand.yaml | 93 +++++++++++++++++++ > 1 file changed, 93 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> Best regards, Krzysztof

4 months

Re: [PATCH v3 5/5] udmabuf: remove udmabuf_folio

by kernel test robot

Hi Huan, kernel test robot noticed the following build warnings: [auto build test WARNING on 033a4691702cdca3a613256b0623b8eeacb4985e] url: https://github.com/intel-lab-lkp/linux/commits/Huan-Yang/udmabuf-cancel-mma… base: 033a4691702cdca3a613256b0623b8eeacb4985e patch link: https://lore.kernel.org/r/20240813090518.3252469-6-link%40vivo.com patch subject: [PATCH v3 5/5] udmabuf: remove udmabuf_folio config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20240816/202408162012.cL9pnFSm-lkp@…) compiler: s390-linux-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240816/202408162012.cL9pnFSm-lkp@…) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp(a)intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408162012.cL9pnFSm-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/dma-buf/udmabuf.c:175: warning: Function parameter or struct member 'ubuf' not described in 'unpin_all_folios' vim +175 drivers/dma-buf/udmabuf.c 17a7ce20349045 Gurchetan Singh 2019-12-02 165 d934739404652b Huan Yang 2024-08-13 166 /** d934739404652b Huan Yang 2024-08-13 167 * unpin_all_folios: unpin each folio we pinned in create d934739404652b Huan Yang 2024-08-13 168 * The udmabuf set all folio in folios and pinned it, but for large folio, d934739404652b Huan Yang 2024-08-13 169 * We may have only used a small portion of the physical in the folio. d934739404652b Huan Yang 2024-08-13 170 * we will repeatedly, sequentially set the folio into the array to ensure d934739404652b Huan Yang 2024-08-13 171 * that the offset can index the correct folio at the corresponding index. d934739404652b Huan Yang 2024-08-13 172 * Hence, we only need to unpin the first iterred folio. d934739404652b Huan Yang 2024-08-13 173 */ d934739404652b Huan Yang 2024-08-13 174 static void unpin_all_folios(struct udmabuf *ubuf) c6a3194c05e7e6 Vivek Kasireddy 2024-06-23 @175 { d934739404652b Huan Yang 2024-08-13 176 pgoff_t pg; d934739404652b Huan Yang 2024-08-13 177 struct folio *last = NULL; c6a3194c05e7e6 Vivek Kasireddy 2024-06-23 178 d934739404652b Huan Yang 2024-08-13 179 for (pg = 0; pg < ubuf->pagecount; pg++) { d934739404652b Huan Yang 2024-08-13 180 struct folio *tmp = ubuf->folios[pg]; c6a3194c05e7e6 Vivek Kasireddy 2024-06-23 181 d934739404652b Huan Yang 2024-08-13 182 if (tmp == last) d934739404652b Huan Yang 2024-08-13 183 continue; c6a3194c05e7e6 Vivek Kasireddy 2024-06-23 184 d934739404652b Huan Yang 2024-08-13 185 last = tmp; d934739404652b Huan Yang 2024-08-13 186 unpin_folio(tmp); d934739404652b Huan Yang 2024-08-13 187 } c6a3194c05e7e6 Vivek Kasireddy 2024-06-23 188 } c6a3194c05e7e6 Vivek Kasireddy 2024-06-23 189 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

4 months, 1 week

Re: [PATCH v7 00/16] Add audio support for the MediaTek Genio 350-evk board

by Mark Brown

On Wed, Aug 14, 2024 at 10:23:12AM +0200, Alexandre Mergnat wrote: > Simple gentle ping, the serie seems ready to be applied. Please don't send content free pings and please allow a reasonable time for review. People get busy, go on holiday, attend conferences and so on so unless there is some reason for urgency (like critical bug fixes) please allow at least a couple of weeks for review. If there have been review comments then people may be waiting for those to be addressed. Sending content free pings adds to the mail volume (if they are seen at all) which is often the problem and since they can't be reviewed directly if something has gone wrong you'll have to resend the patches anyway, so sending again is generally a better approach though there are some other maintainers who like them - if in doubt look at how patches for the subsystem are normally handled.

4 months, 1 week

Re: [PATCH 2/2] mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC

by kernel test robot

Hi Hui-Ping, kernel test robot noticed the following build warnings: [auto build test WARNING on mtd/nand/next] [also build test WARNING on linus/master v6.11-rc3 next-20240813] [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#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Hui-Ping-Chen/dt-bindings-mt… base: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next patch link: https://lore.kernel.org/r/20240812030045.20831-3-hpchen0nvt%40gmail.com patch subject: [PATCH 2/2] mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC config: arc-randconfig-r133-20240813 (https://download.01.org/0day-ci/archive/20240814/202408140515.pbHkvxZD-lkp@…) compiler: arc-elf-gcc (GCC) 13.2.0 reproduce: (https://download.01.org/0day-ci/archive/20240814/202408140515.pbHkvxZD-lkp@…) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp(a)intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408140515.pbHkvxZD-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:119:3: sparse: sparse: symbol 'E_BCHALGORITHM' was not declared. Should it be static? >> drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:333:28: sparse: sparse: cast removes address space '__iomem' of expression drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:348:39: sparse: sparse: cast removes address space '__iomem' of expression >> drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:746:41: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected char *register ptr @@ got void [noderef] __iomem * @@ drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:746:41: sparse: expected char *register ptr drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:746:41: sparse: got void [noderef] __iomem * drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:760:18: sparse: sparse: cast removes address space '__iomem' of expression >> drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:774:32: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected char *ptr @@ got void [noderef] __iomem * @@ drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:774:32: sparse: expected char *ptr drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:774:32: sparse: got void [noderef] __iomem * drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:813:32: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected char *ptr @@ got void [noderef] __iomem * @@ drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:813:32: sparse: expected char *ptr drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:813:32: sparse: got void [noderef] __iomem * drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:516:49: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected char *register ptr @@ got void [noderef] __iomem * @@ drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:516:49: sparse: expected char *register ptr drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:516:49: sparse: got void [noderef] __iomem * drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:516:49: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected char *register ptr @@ got void [noderef] __iomem * @@ drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:516:49: sparse: expected char *register ptr drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:516:49: sparse: got void [noderef] __iomem * drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:516:49: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected char *register ptr @@ got void [noderef] __iomem * @@ drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:516:49: sparse: expected char *register ptr drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:516:49: sparse: got void [noderef] __iomem * drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:516:49: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected char *register ptr @@ got void [noderef] __iomem * @@ drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:516:49: sparse: expected char *register ptr drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:516:49: sparse: got void [noderef] __iomem * vim +/E_BCHALGORITHM +119 drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c 111 112 /* BCH algorithm related constants and variables */ 113 enum { 114 eBCH_NONE = 0, 115 eBCH_T8, 116 eBCH_T12, 117 eBCH_T24, 118 eBCH_CNT > 119 } E_BCHALGORITHM; 120 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

4 months, 1 week

Re: [PATCH 2/2] mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC

by kernel test robot

Hi Hui-Ping, kernel test robot noticed the following build warnings: [auto build test WARNING on mtd/nand/next] [also build test WARNING on linus/master v6.11-rc3 next-20240812] [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#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Hui-Ping-Chen/dt-bindings-mt… base: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next patch link: https://lore.kernel.org/r/20240812030045.20831-3-hpchen0nvt%40gmail.com patch subject: [PATCH 2/2] mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20240812/202408122007.quTiDXPR-lkp@…) compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240812/202408122007.quTiDXPR-lkp@…) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp(a)intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408122007.quTiDXPR-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c:991:29: warning: cast from 'irqreturn_t (*)(int, struct ma35_nand_info *)' (aka 'enum irqreturn (*)(int, struct ma35_nand_info *)') to 'irq_handler_t' (aka 'enum irqreturn (*)(int, void *)') converts to incompatible function type [-Wcast-function-type-strict] 991 | if (request_irq(nand->irq, (irq_handler_t)&ma35_nand_irq, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 warning generated. vim +991 drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c 921 922 static int ma35_nand_probe(struct platform_device *pdev) 923 { 924 struct ma35_nand_info *nand; 925 struct nand_chip *chip; 926 struct mtd_info *mtd; 927 int retval = 0; 928 929 nand = devm_kzalloc(&pdev->dev, sizeof(struct ma35_nand_info), GFP_KERNEL); 930 if (!nand) 931 return -ENOMEM; 932 933 nand_controller_init(&nand->controller); 934 935 nand->regs = devm_platform_ioremap_resource(pdev, 0); 936 if (IS_ERR(nand->regs)) 937 return PTR_ERR(nand->regs); 938 939 nand->dev = &pdev->dev; 940 chip = &nand->chip; 941 mtd = nand_to_mtd(chip); 942 nand_set_controller_data(chip, nand); 943 nand_set_flash_node(chip, pdev->dev.of_node); 944 945 mtd->priv = chip; 946 mtd->owner = THIS_MODULE; 947 mtd->dev.parent = &pdev->dev; 948 949 nand->clk = of_clk_get(pdev->dev.of_node, 0); 950 if (IS_ERR(nand->clk)) 951 return dev_err_probe(&pdev->dev, PTR_ERR(nand->clk), 952 "failed to find nand clock\n"); 953 954 retval = clk_prepare_enable(nand->clk); 955 if (retval < 0) { 956 dev_err(&pdev->dev, "Failed to enable clock\n"); 957 return -ENXIO; 958 } 959 960 nand->chip.controller = &nand->controller; 961 962 chip->legacy.cmdfunc = ma35_nand_command; 963 chip->legacy.waitfunc = ma35_waitfunc; 964 chip->legacy.read_byte = ma35_nand_read_byte; 965 chip->legacy.select_chip = ma35_nand_select_chip; 966 chip->legacy.read_buf = ma35_read_buf_dma; 967 chip->legacy.write_buf = ma35_write_buf_dma; 968 chip->legacy.dev_ready = ma35_nand_devready; 969 chip->legacy.chip_delay = 25; /* us */ 970 971 /* Read OOB data first, then HW read page */ 972 chip->ecc.hwctl = ma35_nand_enable_hwecc; 973 chip->ecc.calculate = ma35_nand_calculate_ecc; 974 chip->ecc.correct = ma35_nand_correct_data; 975 chip->ecc.write_page = ma35_nand_write_page_hwecc; 976 chip->ecc.read_page = ma35_nand_read_page_hwecc_oob_first; 977 chip->ecc.read_oob = ma35_nand_read_oob_hwecc; 978 chip->options |= (NAND_NO_SUBPAGE_WRITE | NAND_USES_DMA); 979 980 ma35_nand_initialize(nand); 981 platform_set_drvdata(pdev, nand); 982 983 nand->controller.ops = &ma35_nand_controller_ops; 984 985 nand->irq = platform_get_irq(pdev, 0); 986 if (nand->irq < 0) { 987 dev_err(&pdev->dev, "failed to get platform irq\n"); 988 return -EINVAL; 989 } 990 > 991 if (request_irq(nand->irq, (irq_handler_t)&ma35_nand_irq, 992 IRQF_TRIGGER_HIGH, "ma35d1-nand", nand)) { 993 dev_err(&pdev->dev, "Error requesting NAND IRQ\n"); 994 return -ENXIO; 995 } 996 997 retval = nand_scan(chip, 1); 998 if (retval) 999 return retval; 1000 1001 if (mtd_device_register(mtd, nand->parts, nand->nr_parts)) { 1002 nand_cleanup(chip); 1003 devm_kfree(&pdev->dev, nand); 1004 return retval; 1005 } 1006 1007 pr_info("ma35-nfi: registered successfully! mtdid=%s\n", mtd->name); 1008 1009 return retval; 1010 } 1011 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

4 months, 1 week

Re: [PATCH 2/2] mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC

by Krzysztof Kozlowski

On 12/08/2024 11:10, Hui-Ping Chen wrote: >>> + >>> +/* NAND-type Flash BCH Error Data Registers */ >>> +#define MA35_NFI_REG_NANDECCED0 (0x960) >>> +#define MA35_NFI_REG_NANDECCED1 (0x964) >>> +#define MA35_NFI_REG_NANDECCED2 (0x968) >>> +#define MA35_NFI_REG_NANDECCED3 (0x96C) >>> +#define MA35_NFI_REG_NANDECCED4 (0x970) >>> +#define MA35_NFI_REG_NANDECCED5 (0x974) >>> + >>> +/* NAND-type Flash Redundant Area Registers */ >>> +#define MA35_NFI_REG_NANDRA0 (0xA00) >>> +#define MA35_NFI_REG_NANDRA1 (0xA04) >>> + >>> +#define SKIP_SPARE_BYTES 4 >>> + >>> +/* BCH algorithm related constants and variables */ >>> +enum { >>> + eBCH_NONE = 0, >>> + eBCH_T8, >>> + eBCH_T12, >>> + eBCH_T24, >>> + eBCH_CNT >>> +} E_BCHALGORITHM; >>> + >>> +static const int g_i32BCHAlgoIdx[eBCH_CNT] = {BCH_T8, BCH_T8, BCH_T12, BCH_T24}; >>> +static struct nand_ecclayout_user ma35_nand_oob; >> Why this is file-scope? > > I will remove the `static`. No, why this cannot be instance dependent? Quick looks says it could. And should. Best regards, Krzysztof

4 months, 1 week

Re: [PATCH 1/2] dt-bindings: mtd: nuvoton,ma35d1-nand: add new bindings

by Krzysztof Kozlowski

On 12/08/2024 11:02, Hui-Ping Chen wrote: > > >>> + >>> + nand-ecc-step-size: >>> + enum: [512, 1024] >> No defaults? So is this required? > > This is required, but I will also add a default. If this is required and should be in required: list. Default does not make sense then... it contradicts the point of being required. > > > >>> + >>> + nand-ecc-strength: >>> + enum: [8, 12, 24] >> No defaults? So is this required? > > This is required, but I will also add a default. Ditto Best regards, Krzysztof

4 months, 1 week

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig August 2024