- Linaro-mm-sig - lists.linaro.org

Re: [PATCH v5 1/4] drm/panthor: introduce job cycle and timestamp accounting

by kernel test robot

Hi Adrián, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-misc/drm-misc-next] [also build test WARNING on linus/master v6.11-rc6 next-20240904] [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/Adri-n-Larumbe/drm-panthor-i… base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20240903202541.430225-2-adrian.larumbe%40collabor… patch subject: [PATCH v5 1/4] drm/panthor: introduce job cycle and timestamp accounting config: x86_64-buildonly-randconfig-002-20240904 (https://download.01.org/0day-ci/archive/20240904/202409042317.CRCMb6bs-lkp@…) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240904/202409042317.CRCMb6bs-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/202409042317.CRCMb6bs-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/gpu/drm/panthor/panthor_sched.c:322: warning: Excess struct member 'runnable' description in 'panthor_scheduler' drivers/gpu/drm/panthor/panthor_sched.c:322: warning: Excess struct member 'idle' description in 'panthor_scheduler' drivers/gpu/drm/panthor/panthor_sched.c:322: warning: Excess struct member 'waiting' description in 'panthor_scheduler' drivers/gpu/drm/panthor/panthor_sched.c:322: warning: Excess struct member 'has_ref' description in 'panthor_scheduler' drivers/gpu/drm/panthor/panthor_sched.c:322: warning: Excess struct member 'in_progress' description in 'panthor_scheduler' drivers/gpu/drm/panthor/panthor_sched.c:322: warning: Excess struct member 'stopped_groups' description in 'panthor_scheduler' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'mem' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'input' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'output' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'input_fw_va' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'output_fw_va' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'gpu_va' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'ref' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'gt' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'sync64' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'bo' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'offset' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'kmap' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'lock' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'id' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'seqno' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'last_fence' description in 'panthor_queue' drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'in_flight_jobs' description in 'panthor_queue' >> drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'slots' description in 'panthor_queue' >> drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'slot_count' description in 'panthor_queue' >> drivers/gpu/drm/panthor/panthor_sched.c:494: warning: Excess struct member 'profiling_seqno' description in 'panthor_queue' >> drivers/gpu/drm/panthor/panthor_sched.c:810: warning: Function parameter or struct member 'profiling_slot' not described in 'panthor_job' drivers/gpu/drm/panthor/panthor_sched.c:810: warning: Excess struct member 'start' description in 'panthor_job' drivers/gpu/drm/panthor/panthor_sched.c:810: warning: Excess struct member 'size' description in 'panthor_job' drivers/gpu/drm/panthor/panthor_sched.c:810: warning: Excess struct member 'latest_flush' description in 'panthor_job' drivers/gpu/drm/panthor/panthor_sched.c:810: warning: Excess struct member 'start' description in 'panthor_job' drivers/gpu/drm/panthor/panthor_sched.c:810: warning: Excess struct member 'end' description in 'panthor_job' >> drivers/gpu/drm/panthor/panthor_sched.c:810: warning: Excess struct member 'profile_slot' description in 'panthor_job' drivers/gpu/drm/panthor/panthor_sched.c:1731: warning: Function parameter or struct member 'ptdev' not described in 'panthor_sched_report_fw_events' drivers/gpu/drm/panthor/panthor_sched.c:1731: warning: Function parameter or struct member 'events' not described in 'panthor_sched_report_fw_events' drivers/gpu/drm/panthor/panthor_sched.c:2623: warning: Function parameter or struct member 'ptdev' not described in 'panthor_sched_report_mmu_fault' vim +494 drivers/gpu/drm/panthor/panthor_sched.c de85488138247d Boris Brezillon 2024-02-29 397 de85488138247d Boris Brezillon 2024-02-29 398 /** @ringbuf: Command stream ring-buffer. */ de85488138247d Boris Brezillon 2024-02-29 399 struct panthor_kernel_bo *ringbuf; de85488138247d Boris Brezillon 2024-02-29 400 de85488138247d Boris Brezillon 2024-02-29 401 /** @iface: Firmware interface. */ de85488138247d Boris Brezillon 2024-02-29 402 struct { de85488138247d Boris Brezillon 2024-02-29 403 /** @mem: FW memory allocated for this interface. */ de85488138247d Boris Brezillon 2024-02-29 404 struct panthor_kernel_bo *mem; de85488138247d Boris Brezillon 2024-02-29 405 de85488138247d Boris Brezillon 2024-02-29 406 /** @input: Input interface. */ de85488138247d Boris Brezillon 2024-02-29 407 struct panthor_fw_ringbuf_input_iface *input; de85488138247d Boris Brezillon 2024-02-29 408 de85488138247d Boris Brezillon 2024-02-29 409 /** @output: Output interface. */ de85488138247d Boris Brezillon 2024-02-29 410 const struct panthor_fw_ringbuf_output_iface *output; de85488138247d Boris Brezillon 2024-02-29 411 de85488138247d Boris Brezillon 2024-02-29 412 /** @input_fw_va: FW virtual address of the input interface buffer. */ de85488138247d Boris Brezillon 2024-02-29 413 u32 input_fw_va; de85488138247d Boris Brezillon 2024-02-29 414 de85488138247d Boris Brezillon 2024-02-29 415 /** @output_fw_va: FW virtual address of the output interface buffer. */ de85488138247d Boris Brezillon 2024-02-29 416 u32 output_fw_va; de85488138247d Boris Brezillon 2024-02-29 417 } iface; de85488138247d Boris Brezillon 2024-02-29 418 de85488138247d Boris Brezillon 2024-02-29 419 /** de85488138247d Boris Brezillon 2024-02-29 420 * @syncwait: Stores information about the synchronization object this de85488138247d Boris Brezillon 2024-02-29 421 * queue is waiting on. de85488138247d Boris Brezillon 2024-02-29 422 */ de85488138247d Boris Brezillon 2024-02-29 423 struct { de85488138247d Boris Brezillon 2024-02-29 424 /** @gpu_va: GPU address of the synchronization object. */ de85488138247d Boris Brezillon 2024-02-29 425 u64 gpu_va; de85488138247d Boris Brezillon 2024-02-29 426 de85488138247d Boris Brezillon 2024-02-29 427 /** @ref: Reference value to compare against. */ de85488138247d Boris Brezillon 2024-02-29 428 u64 ref; de85488138247d Boris Brezillon 2024-02-29 429 de85488138247d Boris Brezillon 2024-02-29 430 /** @gt: True if this is a greater-than test. */ de85488138247d Boris Brezillon 2024-02-29 431 bool gt; de85488138247d Boris Brezillon 2024-02-29 432 de85488138247d Boris Brezillon 2024-02-29 433 /** @sync64: True if this is a 64-bit sync object. */ de85488138247d Boris Brezillon 2024-02-29 434 bool sync64; de85488138247d Boris Brezillon 2024-02-29 435 de85488138247d Boris Brezillon 2024-02-29 436 /** @bo: Buffer object holding the synchronization object. */ de85488138247d Boris Brezillon 2024-02-29 437 struct drm_gem_object *obj; de85488138247d Boris Brezillon 2024-02-29 438 de85488138247d Boris Brezillon 2024-02-29 439 /** @offset: Offset of the synchronization object inside @bo. */ de85488138247d Boris Brezillon 2024-02-29 440 u64 offset; de85488138247d Boris Brezillon 2024-02-29 441 de85488138247d Boris Brezillon 2024-02-29 442 /** de85488138247d Boris Brezillon 2024-02-29 443 * @kmap: Kernel mapping of the buffer object holding the de85488138247d Boris Brezillon 2024-02-29 444 * synchronization object. de85488138247d Boris Brezillon 2024-02-29 445 */ de85488138247d Boris Brezillon 2024-02-29 446 void *kmap; de85488138247d Boris Brezillon 2024-02-29 447 } syncwait; de85488138247d Boris Brezillon 2024-02-29 448 de85488138247d Boris Brezillon 2024-02-29 449 /** @fence_ctx: Fence context fields. */ de85488138247d Boris Brezillon 2024-02-29 450 struct { de85488138247d Boris Brezillon 2024-02-29 451 /** @lock: Used to protect access to all fences allocated by this context. */ de85488138247d Boris Brezillon 2024-02-29 452 spinlock_t lock; de85488138247d Boris Brezillon 2024-02-29 453 de85488138247d Boris Brezillon 2024-02-29 454 /** de85488138247d Boris Brezillon 2024-02-29 455 * @id: Fence context ID. de85488138247d Boris Brezillon 2024-02-29 456 * de85488138247d Boris Brezillon 2024-02-29 457 * Allocated with dma_fence_context_alloc(). de85488138247d Boris Brezillon 2024-02-29 458 */ de85488138247d Boris Brezillon 2024-02-29 459 u64 id; de85488138247d Boris Brezillon 2024-02-29 460 de85488138247d Boris Brezillon 2024-02-29 461 /** @seqno: Sequence number of the last initialized fence. */ de85488138247d Boris Brezillon 2024-02-29 462 atomic64_t seqno; de85488138247d Boris Brezillon 2024-02-29 463 7b6f9ec6ad5112 Boris Brezillon 2024-07-03 464 /** 7b6f9ec6ad5112 Boris Brezillon 2024-07-03 465 * @last_fence: Fence of the last submitted job. 7b6f9ec6ad5112 Boris Brezillon 2024-07-03 466 * 7b6f9ec6ad5112 Boris Brezillon 2024-07-03 467 * We return this fence when we get an empty command stream. 7b6f9ec6ad5112 Boris Brezillon 2024-07-03 468 * This way, we are guaranteed that all earlier jobs have completed 7b6f9ec6ad5112 Boris Brezillon 2024-07-03 469 * when drm_sched_job::s_fence::finished without having to feed 7b6f9ec6ad5112 Boris Brezillon 2024-07-03 470 * the CS ring buffer with a dummy job that only signals the fence. 7b6f9ec6ad5112 Boris Brezillon 2024-07-03 471 */ 7b6f9ec6ad5112 Boris Brezillon 2024-07-03 472 struct dma_fence *last_fence; 7b6f9ec6ad5112 Boris Brezillon 2024-07-03 473 de85488138247d Boris Brezillon 2024-02-29 474 /** de85488138247d Boris Brezillon 2024-02-29 475 * @in_flight_jobs: List containing all in-flight jobs. de85488138247d Boris Brezillon 2024-02-29 476 * de85488138247d Boris Brezillon 2024-02-29 477 * Used to keep track and signal panthor_job::done_fence when the de85488138247d Boris Brezillon 2024-02-29 478 * synchronization object attached to the queue is signaled. de85488138247d Boris Brezillon 2024-02-29 479 */ de85488138247d Boris Brezillon 2024-02-29 480 struct list_head in_flight_jobs; de85488138247d Boris Brezillon 2024-02-29 481 } fence_ctx; 6f64890b41a576 Adrián Larumbe 2024-09-03 482 6f64890b41a576 Adrián Larumbe 2024-09-03 483 /** @profiling_info: Job profiling data slots and access information. */ 6f64890b41a576 Adrián Larumbe 2024-09-03 484 struct { 6f64890b41a576 Adrián Larumbe 2024-09-03 485 /** @slots: Kernel BO holding the slots. */ 6f64890b41a576 Adrián Larumbe 2024-09-03 486 struct panthor_kernel_bo *slots; 6f64890b41a576 Adrián Larumbe 2024-09-03 487 6f64890b41a576 Adrián Larumbe 2024-09-03 488 /** @slot_count: Number of jobs ringbuffer can hold at once. */ 6f64890b41a576 Adrián Larumbe 2024-09-03 489 u32 slot_count; 6f64890b41a576 Adrián Larumbe 2024-09-03 490 6f64890b41a576 Adrián Larumbe 2024-09-03 491 /** @profiling_seqno: Index of the next available profiling information slot. */ 6f64890b41a576 Adrián Larumbe 2024-09-03 492 u32 profiling_seqno; 6f64890b41a576 Adrián Larumbe 2024-09-03 493 } profiling_info; de85488138247d Boris Brezillon 2024-02-29 @494 }; de85488138247d Boris Brezillon 2024-02-29 495 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

9 months

1
0
0 0

Re: (subset) [PATCH v6 00/16] Add audio support for the MediaTek Genio 350-evk board

by Mark Brown

On Wed, 19 Jun 2024 16:46:36 +0200, Alexandre Mergnat wrote: > This serie aim to add the following audio support for the Genio 350-evk: > - Playback > - 2ch Headset Jack (Earphone) > - 1ch Line-out Jack (Speaker) > - 8ch HDMI Tx > - Capture > - 1ch DMIC (On-board Digital Microphone) > - 1ch AMIC (On-board Analogic Microphone) > - 1ch Headset Jack (External Analogic Microphone) > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [01/16] ASoC: dt-bindings: mediatek,mt8365-afe: Add audio afe document commit: ceb3ca2876243e3ea02f78b3d488b1f2d734de49 [02/16] ASoC: dt-bindings: mediatek,mt8365-mt6357: Add audio sound card document commit: 76d80dcdd55f70b28930edb97b96ee375e1cce5a [04/16] ASoC: mediatek: mt8365: Add common header commit: 38c7c9ddc74033406461d64e541bbc8268e77f73 [05/16] ASoC: mediatek: mt8365: Add audio clock control support commit: ef307b40b7f0042d54f020bccb3e728ced292282 [06/16] ASoC: mediatek: mt8365: Add I2S DAI support commit: 402bbb13a195caa83b3279ebecdabfb11ddee084 [07/16] ASoC: mediatek: mt8365: Add ADDA DAI support commit: 7c58c88e524180e8439acdfc44872325e7f6d33d [08/16] ASoC: mediatek: mt8365: Add DMIC DAI support commit: 1c50ec75ce6c0c6b5736499393e522f73e19d0cf [09/16] ASoC: mediatek: mt8365: Add PCM DAI support commit: 5097c0c8634d703e3c59cfb89831b7db9dc46339 [10/16] ASoc: mediatek: mt8365: Add a specific soundcard for EVK commit: 1bf6dbd75f7603dd026660bebf324f812200dc1b [11/16] ASoC: mediatek: mt8365: Add the AFE driver support commit: e1991d102bc2abb32331c462f8f3e77059c69578 [12/16] ASoC: codecs: add MT6357 support (no commit info) [13/16] ASoC: mediatek: Add MT8365 support (no commit info) All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark

9 months

1
0
0 0

Re: [PATCH v7 12/16] ASoC: codecs: add MT6357 support

by Mark Brown

On Mon, Jul 22, 2024 at 08:53:41AM +0200, amergnat(a)baylibre.com wrote: > From: Nicolas Belin <nbelin(a)baylibre.com> > > Add the support of MT6357 PMIC audio codec. This breaks the build: /build/stage/linux/sound/soc/codecs/mt6357.c: In function ‘mt6357_platform_driver_probe’: /build/stage/linux/sound/soc/codecs/mt6357.c:1824:55: error: too many arguments for format [-Werror=format-extra-args] 1824 | return dev_err_probe(&pdev->dev, ret, "Failed to parse dts\n", __func__); | ^~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors

9 months, 1 week

1
0
0 0

[PATCH] dma-buf: heaps: Fix off-by-one in CMA heap fault handler

by T.J. Mercier

Until VM_DONTEXPAND was added in commit 1c1914d6e8c6 ("dma-buf: heaps: Don't track CMA dma-buf pages under RssFile") it was possible to obtain a mapping larger than the buffer size via mremap and bypass the overflow check in dma_buf_mmap_internal. When using such a mapping to attempt to fault past the end of the buffer, the CMA heap fault handler also checks the fault offset against the buffer size, but gets the boundary wrong by 1. Fix the boundary check so that we don't read off the end of the pages array and insert an arbitrary page in the mapping. Reported-by: Xingyu Jin <xingyuj(a)google.com> Fixes: a5d2d29e24be ("dma-buf: heaps: Move heap-helper logic into the cma_heap implementation") Cc: stable(a)vger.kernel.org # Applicable >= 5.10. Needs adjustments only for 5.10. Signed-off-by: T.J. Mercier <tjmercier(a)google.com> --- drivers/dma-buf/heaps/cma_heap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index c384004b918e..93be88b805fe 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -165,7 +165,7 @@ static vm_fault_t cma_heap_vm_fault(struct vm_fault *vmf) struct vm_area_struct *vma = vmf->vma; struct cma_heap_buffer *buffer = vma->vm_private_data; - if (vmf->pgoff > buffer->pagecount) + if (vmf->pgoff >= buffer->pagecount) return VM_FAULT_SIGBUS; return vmf_insert_pfn(vma, vmf->address, page_to_pfn(buffer->pages[vmf->pgoff])); -- 2.46.0.469.g59c65b2a67-goog

9 months, 1 week

2
1
0 0

Re: [PATCH v4 4/5] udmabuf: udmabuf_create codestyle cleanup

by Dan Carpenter

Hi Huan, kernel test robot noticed the following build warnings: url: https://github.com/intel-lab-lkp/linux/commits/Huan-Yang/udmabuf-direct-map… base: 6a7917c89f219f09b1d88d09f376000914a52763 patch link: https://lore.kernel.org/r/20240822084342.1574914-5-link%40vivo.com patch subject: [PATCH v4 4/5] udmabuf: udmabuf_create codestyle cleanup config: x86_64-randconfig-161-20240829 (https://download.01.org/0day-ci/archive/20240829/202408291101.WAf552sW-lkp@…) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 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> | Reported-by: Dan Carpenter <dan.carpenter(a)linaro.org> | Closes: https://lore.kernel.org/r/202408291101.WAf552sW-lkp@intel.com/ smatch warnings: drivers/dma-buf/udmabuf.c:467 udmabuf_create() error: double free of 'folios' vim +/folios +467 drivers/dma-buf/udmabuf.c c1bbed66899726 Gurchetan Singh 2019-12-02 396 static long udmabuf_create(struct miscdevice *device, c1bbed66899726 Gurchetan Singh 2019-12-02 397 struct udmabuf_create_list *head, c1bbed66899726 Gurchetan Singh 2019-12-02 398 struct udmabuf_create_item *list) fbb0de79507819 Gerd Hoffmann 2018-08-27 399 { fb2c508270085b Huan Yang 2024-08-22 400 pgoff_t pgcnt = 0, pglimit, max_ipgcnt = 0; fb2c508270085b Huan Yang 2024-08-22 401 long ret = -EINVAL; fbb0de79507819 Gerd Hoffmann 2018-08-27 402 struct udmabuf *ubuf; fb2c508270085b Huan Yang 2024-08-22 403 struct folio **folios = NULL; fb2c508270085b Huan Yang 2024-08-22 404 u32 i, flags; fbb0de79507819 Gerd Hoffmann 2018-08-27 405 33f35429fc49c0 Gerd Hoffmann 2018-09-11 406 ubuf = kzalloc(sizeof(*ubuf), GFP_KERNEL); fbb0de79507819 Gerd Hoffmann 2018-08-27 407 if (!ubuf) fbb0de79507819 Gerd Hoffmann 2018-08-27 408 return -ENOMEM; fbb0de79507819 Gerd Hoffmann 2018-08-27 409 c6a3194c05e7e6 Vivek Kasireddy 2024-06-23 410 INIT_LIST_HEAD(&ubuf->unpin_list); dc4716d75154b3 Gerd Hoffmann 2018-09-11 411 pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; fbb0de79507819 Gerd Hoffmann 2018-08-27 412 for (i = 0; i < head->count; i++) { fb2c508270085b Huan Yang 2024-08-22 413 pgoff_t itempgcnt; fb2c508270085b Huan Yang 2024-08-22 414 fb2c508270085b Huan Yang 2024-08-22 415 if (!PAGE_ALIGNED(list[i].offset)) 0d17455ca85ecb Gerd Hoffmann 2018-09-11 416 goto err; fb2c508270085b Huan Yang 2024-08-22 417 if (!PAGE_ALIGNED(list[i].size)) 0d17455ca85ecb Gerd Hoffmann 2018-09-11 418 goto err; fb2c508270085b Huan Yang 2024-08-22 419 fb2c508270085b Huan Yang 2024-08-22 420 itempgcnt = list[i].size >> PAGE_SHIFT; fb2c508270085b Huan Yang 2024-08-22 421 pgcnt += itempgcnt; fb2c508270085b Huan Yang 2024-08-22 422 fb2c508270085b Huan Yang 2024-08-22 423 if (pgcnt > pglimit) 0d17455ca85ecb Gerd Hoffmann 2018-09-11 424 goto err; fb2c508270085b Huan Yang 2024-08-22 425 fb2c508270085b Huan Yang 2024-08-22 426 max_ipgcnt = max_t(unsigned long, itempgcnt, max_ipgcnt); fbb0de79507819 Gerd Hoffmann 2018-08-27 427 } 2b6dd600dd7257 Pavel Skripkin 2021-12-30 428 fb2c508270085b Huan Yang 2024-08-22 429 if (!pgcnt) 2b6dd600dd7257 Pavel Skripkin 2021-12-30 430 goto err; 2b6dd600dd7257 Pavel Skripkin 2021-12-30 431 fb2c508270085b Huan Yang 2024-08-22 432 ubuf->folios = kvmalloc_array(pgcnt, sizeof(*ubuf->folios), fbb0de79507819 Gerd Hoffmann 2018-08-27 433 GFP_KERNEL); 5e72b2b41a21e5 Vivek Kasireddy 2024-06-23 434 if (!ubuf->folios) { fbb0de79507819 Gerd Hoffmann 2018-08-27 435 ret = -ENOMEM; 0d17455ca85ecb Gerd Hoffmann 2018-09-11 436 goto err; fbb0de79507819 Gerd Hoffmann 2018-08-27 437 } fb2c508270085b Huan Yang 2024-08-22 438 fb2c508270085b Huan Yang 2024-08-22 439 ubuf->offsets = kvcalloc(pgcnt, sizeof(*ubuf->offsets), GFP_KERNEL); 0c8b91ef5100ea Vivek Kasireddy 2024-06-23 440 if (!ubuf->offsets) { 0c8b91ef5100ea Vivek Kasireddy 2024-06-23 441 ret = -ENOMEM; 0c8b91ef5100ea Vivek Kasireddy 2024-06-23 442 goto err; 0c8b91ef5100ea Vivek Kasireddy 2024-06-23 443 } fbb0de79507819 Gerd Hoffmann 2018-08-27 444 fb2c508270085b Huan Yang 2024-08-22 445 folios = kvmalloc_array(max_ipgcnt, sizeof(*folios), GFP_KERNEL); c6a3194c05e7e6 Vivek Kasireddy 2024-06-23 446 if (!folios) { c6a3194c05e7e6 Vivek Kasireddy 2024-06-23 447 ret = -ENOMEM; c6a3194c05e7e6 Vivek Kasireddy 2024-06-23 448 goto err; c6a3194c05e7e6 Vivek Kasireddy 2024-06-23 449 } c6a3194c05e7e6 Vivek Kasireddy 2024-06-23 450 fb2c508270085b Huan Yang 2024-08-22 451 for (i = 0; i < head->count; i++) { fb2c508270085b Huan Yang 2024-08-22 452 ret = __udmabuf_pin_list_folios(&list[i], ubuf, folios); There is a kfree(folios) hidden inside this function. It doesn't belong there. fb2c508270085b Huan Yang 2024-08-22 453 if (ret) 0d17455ca85ecb Gerd Hoffmann 2018-09-11 454 goto err; c6a3194c05e7e6 Vivek Kasireddy 2024-06-23 455 } 452dc1b0221804 Huan Yang 2024-08-22 456 kvfree(folios); ^^^^^^^^^^^^^^ A second free fbb0de79507819 Gerd Hoffmann 2018-08-27 457 5e72b2b41a21e5 Vivek Kasireddy 2024-06-23 458 flags = head->flags & UDMABUF_FLAGS_CLOEXEC ? O_CLOEXEC : 0; 5e72b2b41a21e5 Vivek Kasireddy 2024-06-23 459 ret = export_udmabuf(ubuf, device, flags); 5e72b2b41a21e5 Vivek Kasireddy 2024-06-23 460 if (ret < 0) 0d17455ca85ecb Gerd Hoffmann 2018-09-11 461 goto err; ^^^^^^^^ fbb0de79507819 Gerd Hoffmann 2018-08-27 462 5e72b2b41a21e5 Vivek Kasireddy 2024-06-23 463 return ret; fbb0de79507819 Gerd Hoffmann 2018-08-27 464 0d17455ca85ecb Gerd Hoffmann 2018-09-11 465 err: c6a3194c05e7e6 Vivek Kasireddy 2024-06-23 466 unpin_all_folios(&ubuf->unpin_list); fb2c508270085b Huan Yang 2024-08-22 @467 kvfree(folios); ^^^^^^^^^^^^^ Double free 452dc1b0221804 Huan Yang 2024-08-22 468 kvfree(ubuf->offsets); 452dc1b0221804 Huan Yang 2024-08-22 469 kvfree(ubuf->folios); fbb0de79507819 Gerd Hoffmann 2018-08-27 470 kfree(ubuf); fbb0de79507819 Gerd Hoffmann 2018-08-27 471 return ret; fbb0de79507819 Gerd Hoffmann 2018-08-27 472 } -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

9 months, 1 week

1
0
0 0

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

by Miquel Raynal

Hi Hui-Ping, hpchen0nvt(a)gmail.com wrote on Wed, 28 Aug 2024 10:47:17 +0800: > Dear Miquèl, > > Thank you for your reply. > > > > On 2024/8/24 上午 12:26, Miquel Raynal wrote: > > Hi, > > > > hpchen0nvt(a)gmail.com wrote on Wed, 21 Aug 2024 07:11:31 +0000: > > > >> Add dt-bindings for the Nuvoton MA35 SoC NAND Controller. > >> > >> Signed-off-by: Hui-Ping Chen <hpchen0nvt(a)gmail.com> > >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> > >> --- > >> .../bindings/mtd/nuvoton,ma35d1-nand.yaml | 93 +++++++++++++++++++ > >> 1 file changed, 93 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml > >> > >> diff --git a/Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml b/Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml > >> new file mode 100644 > >> index 000000000000..152784e73263 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml > >> @@ -0,0 +1,93 @@ > >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >> +%YAML 1.2 > >> +--- > >> +$id: http://devicetree.org/schemas/mtd/nuvoton,ma35d1-nand.yaml# > >> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >> + > >> +title: Nuvoton MA35D1 NAND Flash Interface (NFI) Controller > >> + > >> +maintainers: > >> + - Hui-Ping Chen <hpchen0nvt(a)gmail.com> > >> + > >> +allOf: > >> + - $ref: nand-controller.yaml# > >> + > >> +properties: > >> + compatible: > >> + enum: > >> + - nuvoton,ma35d1-nand > > Can we please use the -nand-controller suffix. A NAND is a the common > > name for a chip with storage inside. You are describing a host > > controller that can be connected to in order to talk to a NAND. > > > Okay, I will change it to nuvoton,ma35d1-nfi. > > Because in our platform, it is the NAND Flash Interface. nfi is not an acronym that is understandable by everyone. Please use -nand-controller. Don't be worried by the size of the string. You can use the acronym as prefix for your NAND controller functions though. Thanks, Miquèl

9 months, 1 week

1
0
0 0

linaro-mm-sig@lists.linaro.org you have 3 pending docs awaiting download

by Admin lists.linaro.org

9 months, 2 weeks

1
0
0 0

linaro-mm-sig@lists.linaro.org you have 3 pending docs awaiting download

by Admin lists.linaro.org

9 months, 2 weeks

1
0
0 0

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

by Miquel Raynal

Hi Hui-Ping, hpchen0nvt(a)gmail.com wrote on Wed, 21 Aug 2024 07:11:32 +0000: > Nuvoton MA35 SoCs NAND Flash Interface Controller > supports 2KB, 4KB and 8KB page size, and up to 8-bit, Suffix is: kiB > 12-bit, and 24-bit hardware ECC calculation circuit > to protect data communication. It's not the communication, it's the data itself. > > Signed-off-by: Hui-Ping Chen <hpchen0nvt(a)gmail.com> > --- > drivers/mtd/nand/raw/Kconfig | 8 + > drivers/mtd/nand/raw/Makefile | 1 + > drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c | 1068 ++++++++++++++++++++ > 3 files changed, 1077 insertions(+) > create mode 100644 drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c > > diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig > index 614257308516..932bf2215470 100644 > --- a/drivers/mtd/nand/raw/Kconfig > +++ b/drivers/mtd/nand/raw/Kconfig > @@ -448,6 +448,14 @@ config MTD_NAND_RENESAS > Enables support for the NAND controller found on Renesas R-Car > Gen3 and RZ/N1 SoC families. > > +config MTD_NAND_NVT_MA35 Is NVT so common or is it just one opportunity to save 4 chars in a Kconfig file?? I'd prefer something more easy to understand. > + tristate "Nuvoton MA35 SoC NAND controller" > + depends on ARCH_MA35 || COMPILE_TEST > + depends on OF > + help > + Enables support for the NAND controller found on > + the Nuvoton MA35 series SoCs. > + > comment "Misc" > > config MTD_SM_COMMON > diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile > index 25120a4afada..cdfdfee3f5f3 100644 > --- a/drivers/mtd/nand/raw/Makefile > +++ b/drivers/mtd/nand/raw/Makefile > @@ -57,6 +57,7 @@ obj-$(CONFIG_MTD_NAND_INTEL_LGM) += intel-nand-controller.o > obj-$(CONFIG_MTD_NAND_ROCKCHIP) += rockchip-nand-controller.o > obj-$(CONFIG_MTD_NAND_PL35X) += pl35x-nand-controller.o > obj-$(CONFIG_MTD_NAND_RENESAS) += renesas-nand-controller.o > +obj-$(CONFIG_MTD_NAND_NVT_MA35) += nuvoton_ma35d1_nand.o > > nand-objs := nand_base.o nand_legacy.o nand_bbt.o nand_timings.o nand_ids.o > nand-objs += nand_onfi.o > diff --git a/drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c b/drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c > new file mode 100644 > index 000000000000..b4586d7a7a45 > --- /dev/null > +++ b/drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c > @@ -0,0 +1,1068 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2024 Nuvoton Technology Corp. > + */ > +#include <linux/slab.h> > +#include <linux/init.h> > +#include <linux/module.h> > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/platform_device.h> > +#include <linux/delay.h> > +#include <linux/clk.h> > +#include <linux/err.h> > +#include <linux/of.h> > + Not sure why you didn't sort the below includes with the ones above? > +#include <linux/mtd/mtd.h> > +#include <linux/mtd/partitions.h> > +#include <linux/mtd/rawnand.h> > +#include <linux/dma-mapping.h> > +#include <linux/dmaengine.h> > + > +/* NFI DMA Registers */ > +#define MA35_NFI_REG_BUFFER0 (0x000) You don't need all these parentheses > +#define MA35_NFI_REG_DMACTL (0x400) > +#define DMA_EN BIT(0) > +#define DMA_RST BIT(1) > +#define DMA_BUSY BIT(9) > + > +#define MA35_NFI_REG_DMASA (0x408) > +#define MA35_NFI_REG_DMABCNT (0x40C) > +#define MA35_NFI_REG_DMAINTEN (0x410) > +#define MA35_NFI_REG_DMAINTSTS (0x414) > + > +/* NFI Global Registers */ > +#define MA35_NFI_REG_GCTL (0x800) > +#define NAND_EN BIT(3) > +#define MA35_NFI_REG_GINTEN (0x804) > +#define MA35_NFI_REG_GINTSTS (0x808) > + > +/* NAND-type Flash Registers */ > +#define MA35_NFI_REG_NANDCTL (0x8A0) > +#define SWRST BIT(0) > +#define DMA_W_EN BIT(1) > +#define DMA_R_EN BIT(2) > +#define ECC_CHK BIT(7) > +#define PROT3BEN BIT(8) > +#define PSIZE_2K (1 << 16) > +#define PSIZE_4K (2 << 16) > +#define PSIZE_8K (3 << 16) > +#define PSIZE_MASK (3 << 16) > +#define BCH_T24 BIT(18) > +#define BCH_T8 BIT(20) > +#define BCH_T12 BIT(21) > +#define BCH_NONE (0x0) > +#define BCH_MASK (0x1f << 18) > +#define ECC_EN BIT(23) > +#define DISABLE_CS0 BIT(25) > + > +#define MA35_NFI_REG_NANDTMCTL (0x8A4) > +#define MA35_NFI_REG_NANDINTEN (0x8A8) > +#define MA35_NFI_REG_NANDINTSTS (0x8AC) > +#define INT_DMA BIT(0) > +#define INT_ECC BIT(2) > +#define INT_RB0 BIT(10) > +#define INT_RB0_STS BIT(18) > + > +#define MA35_NFI_REG_NANDCMD (0x8B0) > +#define MA35_NFI_REG_NANDADDR (0x8B4) > +#define ENDADDR BIT(31) > + > +#define MA35_NFI_REG_NANDDATA (0x8B8) > +#define MA35_NFI_REG_NANDRACTL (0x8BC) > +#define MA35_NFI_REG_NANDECTL (0x8C0) > +#define ENABLE_WP (0x0) > +#define DISABLE_WP BIT(0) > + > +#define MA35_NFI_REG_NANDECCES0 (0x8D0) > +#define ECC_STATUS_MASK (0x3) > +#define ECC_ERR_CNT_MASK (0x1f) > + > +#define MA35_NFI_REG_NANDECCES1 (0x8D4) > +#define MA35_NFI_REG_NANDECCES2 (0x8D8) > +#define MA35_NFI_REG_NANDECCES3 (0x8DC) > + > +/* NAND-type Flash BCH Error Address Registers */ > +#define MA35_NFI_REG_NANDECCEA0 (0x900) > +#define MA35_NFI_REG_NANDECCEA1 (0x904) > +#define MA35_NFI_REG_NANDECCEA2 (0x908) > +#define MA35_NFI_REG_NANDECCEA3 (0x90C) > +#define MA35_NFI_REG_NANDECCEA4 (0x910) > +#define MA35_NFI_REG_NANDECCEA5 (0x914) > +#define MA35_NFI_REG_NANDECCEA6 (0x918) > +#define MA35_NFI_REG_NANDECCEA7 (0x91C) > +#define MA35_NFI_REG_NANDECCEA8 (0x920) > +#define MA35_NFI_REG_NANDECCEA9 (0x924) > +#define MA35_NFI_REG_NANDECCEA10 (0x928) > +#define MA35_NFI_REG_NANDECCEA11 (0x92C) > + > +/* 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 */ > +static const int ma35_parity[3][4] = { > + {0, 60, 92, 90}, /* for 2K */ > + {0, 120, 184, 180}, /* for 4K */ > + {0, 240, 368, 360}, /* for 8K */ Can you please create definitions for the matrix rows? (using an enum seems appropriate) And maybe an array of three structures would be best because I believe you're defining offsets for something which is not clear to the reader. > +}; > + > +struct ma35_nand_info { > + struct nand_controller controller; > + struct device *dev; > + void __iomem *regs; > + int irq; > + struct clk *clk; > + struct completion complete; > + > + struct mtd_info mtd; Please have a look at nand_to_mtd() > + struct nand_chip chip; Is there a single CS supported? Is there a single RB supported? > + struct mtd_partition *parts; No, this has nothing to do here. > + struct nand_ecclayout_user nand_oob; Deprecated structure > + int nr_parts; > + > + u32 bch; > + u8 *dma_buf; > + spinlock_t dma_lock; > + dma_addr_t dma_addr; > +}; > + > +static int ma35_ooblayout_ecc(struct mtd_info *mtd, int section, > + struct mtd_oob_region *oobregion) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + > + if (section) > + return -ERANGE; > + > + oobregion->length = chip->ecc.total; > + oobregion->offset = mtd->oobsize - oobregion->length; > + > + return 0; > +} > + > +static int ma35_ooblayout_free(struct mtd_info *mtd, int section, > + struct mtd_oob_region *oobregion) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + > + if (section) > + return -ERANGE; > + > + oobregion->length = mtd->oobsize - chip->ecc.total - 2; > + oobregion->offset = 2; > + > + return 0; > +} > + > +static const struct mtd_ooblayout_ops ma35_ooblayout_ops = { > + .free = ma35_ooblayout_free, > + .ecc = ma35_ooblayout_ecc, > +}; > + > +/* > + * Initialize hardware ECC > + */ > +static void ma35_nand_hwecc_init(struct ma35_nand_info *nand) > +{ > + struct mtd_info *mtd = nand_to_mtd(&nand->chip); > + > + /* reset nand controller */ Reset NAND > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | SWRST, > + nand->regs + MA35_NFI_REG_NANDCTL); I believe it's fine to do it on several lines and probably clearer. u32 reg = readl(); reg |= SOMETHING; writel(); No wait after the reset? > + /* Redundant area size */ > + writel(mtd->oobsize, nand->regs + MA35_NFI_REG_NANDRACTL); > + > + /* Protect redundant 3 bytes */ What does that mean? > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | PROT3BEN, > + nand->regs + MA35_NFI_REG_NANDCTL); > + > + /* Write the ECC parity codes automatically to NAND Flash */ > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | ECC_CHK, > + nand->regs + MA35_NFI_REG_NANDCTL); No, by default you should disabled the ECC engine. Then when you need it you enable/use/disable it. > + > + if (nand->bch == BCH_NONE) { > + /* Disable H/W ECC, ECC parity check enable bit during read page */ > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~ECC_EN), > + nand->regs + MA35_NFI_REG_NANDCTL); > + } else { > + /* Set BCH algorithm */ > + writel((readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~BCH_MASK)) | > + nand->bch, nand->regs + MA35_NFI_REG_NANDCTL); > + > + /* Enable H/W ECC, ECC parity check enable bit during read page */ > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | ECC_EN, > + nand->regs + MA35_NFI_REG_NANDCTL); > + } > + spin_lock_init(&nand->dma_lock); > +} > + > +static void ma35_nand_initialize(struct ma35_nand_info *nand) > +{ > + writel(NAND_EN, nand->regs + MA35_NFI_REG_GCTL); > +} > + > + > +/* Define some constants for BCH */ for the BCH hardware ECC engine > +/* define the total padding bytes for 512/1024 data segment */ > +#define BCH_PADDING_LEN_512 32 > +#define BCH_PADDING_LEN_1024 64 > +/* define the BCH parity code length for 512 bytes data pattern */ > +#define BCH_PARITY_LEN_T8 15 > +#define BCH_PARITY_LEN_T12 23 > +/* define the BCH parity code length for 1024 bytes data pattern */ > +#define BCH_PARITY_LEN_T24 45 > + Is T the strength? Can we name it strength instead? Please move the definitions at the top > +/* Correct data by BCH alrogithm */ > +static void ma35_nfi_correctdata(struct ma35_nand_info *nand, u8 index, > + u8 err_cnt, u8 *addr) correctdata vs correct, the naming needs to be improved > +{ > + u8 *ptr = (u8 *)((long)nand->regs + MA35_NFI_REG_NANDRA0); Haha, no, never. Please compile with C=1 and see how this explodes. Also, you can enable W=1 > + u32 field_len, padding_len, parity_len; > + u32 temp_data[24], temp_addr[24]; > + u32 total_field_num, page; > + u32 err_data[6]; > + u8 *smra_index; > + u8 i, j; > + > + /* assign parameters for different BCH and page size */ configurations > + switch (readl(nand->regs + MA35_NFI_REG_NANDCTL) & BCH_MASK) { > + case BCH_T24: > + field_len = 1024; > + parity_len = BCH_PARITY_LEN_T24; > + padding_len = BCH_PADDING_LEN_1024; > + break; > + case BCH_T12: > + field_len = 512; > + parity_len = BCH_PARITY_LEN_T12; > + padding_len = BCH_PADDING_LEN_512; > + break; > + case BCH_T8: > + field_len = 512; > + parity_len = BCH_PARITY_LEN_T8; > + padding_len = BCH_PADDING_LEN_512; > + break; > + default: > + pr_warn("NAND ERROR: invalid SMCR_BCH_TSEL = 0x%08X\n", > + (u32)(readl(nand->regs + MA35_NFI_REG_NANDCTL) & BCH_MASK)); > + return; > + } > + > + page = readl(nand->regs + MA35_NFI_REG_NANDCTL) & PSIZE_MASK; > + switch (page) { > + case PSIZE_8K: > + total_field_num = 8192 / field_len; break; > + case PSIZE_4K: > + total_field_num = 4096 / field_len; break; > + case PSIZE_2K: > + total_field_num = 2048 / field_len; break; Break on a new line > + default: > + pr_warn("NAND ERROR: invalid SMCR_PSIZE = 0x%08X\n", page); > + return; > + } > + > + /* got valid BCH_ECC_DATAx and parse them to temp_data[] > + * got the valid register number of BCH_ECC_DATAx since > + * one register include 4 error bytes > + */ > + j = err_cnt / 4; > + j++; > + if (j > 6) > + j = 6; /* there are 6 BCH_ECC_DATAx registers to support BCH T24 */ > + > + for (i = 0; i < j; i++) > + err_data[i] = readl(nand->regs + MA35_NFI_REG_NANDECCED0 + i*4); > + > + for (i = 0; i < j; i++) { > + temp_data[i*4+0] = err_data[i] & 0xff; > + temp_data[i*4+1] = (err_data[i] >> 8) & 0xff; > + temp_data[i*4+2] = (err_data[i] >> 16) & 0xff; > + temp_data[i*4+3] = (err_data[i] >> 24) & 0xff; > + } > + > + /* got valid REG_BCH_ECC_ADDRx and parse them to temp_addr[] > + * got the valid register number of REG_BCH_ECC_ADDRx since > + * one register include 2 error addresses > + */ > + j = err_cnt / 2; > + j++; > + if (j > 12) > + j = 12; /* there are 12 REG_BCH_ECC_ADDRx registers to support BCH T24 */ > + > + for (i = 0; i < j; i++) { > + /* 11 bits for error address */ > + temp_addr[i*2+0] = readl(nand->regs + MA35_NFI_REG_NANDECCEA0 + i*4) & 0x07ff; > + temp_addr[i*2+1] = (readl(nand->regs + MA35_NFI_REG_NANDECCEA0 + i*4)>>16) & 0x07ff; > + } > + > + /* pointer to begin address of field that with data error */ > + addr += (index-1) * field_len; > + > + /* correct each error bytes */ > + for (i = 0; i < err_cnt; i++) { > + /* for wrong data in field */ > + if (temp_addr[i] < field_len) > + *(addr+temp_addr[i]) ^= temp_data[i]; > + > + /* for wrong first-3-bytes in redundancy area */ > + else if (temp_addr[i] < (field_len+3)) { > + temp_addr[i] -= field_len; > + temp_addr[i] += (parity_len * (index-1)); /* field offset */ > + > + *(ptr + temp_addr[i]) ^= temp_data[i]; > + } > + /* for wrong parity code in redundancy area */ > + /* BCH_ERR_ADDRx = [data in field] + [3 bytes] + [xx] + [parity code] */ > + /* |<-- padding bytes -->| */ > + /* The BCH_ERR_ADDRx for last parity code always = field size + padding size. */ > + /* So, the first parity code = field size + padding size - parity code length. */ > + /* For example, for BCH T12, the first parity code = 512 + 32 - 23 = 521. */ > + /* That is, error byte address offset within field is */ > + else { > + temp_addr[i] = temp_addr[i] - (field_len + padding_len - parity_len); > + > + /* smra_index point to the first parity code of > + * first field in register SMRA0~n > + */ > + smra_index = (u8 *)(ptr + > + (readl(nand->regs+MA35_NFI_REG_NANDRACTL) & 0x1ff) - > + (parity_len * total_field_num)); > + > + /* final address = first parity code of first field + */ > + /* offset of fields + */ > + /* offset within field */ Coding style > + > + *((u8 *)smra_index + (parity_len * (index - 1)) + temp_addr[i]) > + ^= temp_data[i]; -ENOPARSE > + } > + } /* end of for (i < err_cnt) */ Useless comment > +} > + > +static int ma35_nfi_correct(struct nand_chip *chip, unsigned long addr) > +{ > + struct ma35_nand_info *nand = nand_get_controller_data(chip); > + struct mtd_info *mtd = nand_to_mtd(chip); > + int status, i, j, field = 0; > + int report_err = 0; > + int err_cnt = 0; > + > + if ((readl(nand->regs + MA35_NFI_REG_NANDCTL) & BCH_MASK) == BCH_T24) > + field = mtd->writesize / 1024; Can we call this a nchunks? Also, you're supposed to expect some DT properties (based on your bindings) and you're not using their values, it's strange. > + else > + field = mtd->writesize / 512; > + > + if (field < 4) > + field = 1; > + else > + field /= 4; > + > + for (j = 0; j < field; j++) { > + status = readl(nand->regs + MA35_NFI_REG_NANDECCES0 + j*4); > + if (!status) > + continue; Is this case relevant? Isn't it treated below? > + > + for (i = 1; i < 5; i++) { From 1 to 5? That's strange. > + if (!(status & ECC_STATUS_MASK)) { > + /* No error */ > + status >>= 8; You can't do that 5 times on a int. > + continue; > + > + } else if ((status & ECC_STATUS_MASK) == 0x01) { > + /* Correctable error */ > + err_cnt = (status >> 2) & ECC_ERR_CNT_MASK; Please use FIELD_GET() > + pr_warn("Field (%d, %d) have %d error!\n", j, i, err_cnt); dev_warn() > + ma35_nfi_correctdata(nand, j*4+i, err_cnt, (u8 *)addr); > + report_err += err_cnt; > + > + } else { > + /* uncorrectable error */ > + pr_warn("uncorrectable error! 0x%4x\n", status); > + return -1; > + } > + status >>= 8; > + } > + } > + return report_err; > +} > + > + > +static int ma35_nand_correct_data(struct nand_chip *chip, u_char *dat, > + u_char *read_ecc, u_char *calc_ecc) > +{ > + return 0; ? > +} > + > + > +static void ma35_nand_enable_hwecc(struct nand_chip *chip, int mode) > +{ ? > + > +} > + > +/* > + * Initial dma controller Initialize DMA > + */ > +static void ma35_nand_dmac_init(struct ma35_nand_info *nand) > +{ > + /* DMAC reset and enable */ > + writel(DMA_RST | DMA_EN, nand->regs + MA35_NFI_REG_DMACTL); > + writel(DMA_EN, nand->regs + MA35_NFI_REG_DMACTL); > + > + /* Clear DMA finished flag */ > + writel(INT_DMA | INT_ECC, nand->regs + MA35_NFI_REG_NANDINTSTS); > + > + init_completion(&nand->complete); > +} > + > +/* > + * read a byte from NAND controller Read (same below) > + */ > +static u8 ma35_nand_read_byte(struct nand_chip *chip) > +{ > + struct ma35_nand_info *nand = nand_get_controller_data(chip); > + u8 ret; > + > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0), > + nand->regs+MA35_NFI_REG_NANDCTL); > + ret = (u8)readl(nand->regs + MA35_NFI_REG_NANDDATA); readb? maybe you need to handle endianness? > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0, > + nand->regs + MA35_NFI_REG_NANDCTL); > + > + return ret; > +} > + > + > +/* > + * read data from NAND controller > + */ > +static void ma35_nand_read_buf(struct nand_chip *chip, u8 *buf, int len) > +{ > + struct ma35_nand_info *nand = nand_get_controller_data(chip); > + int i; > + > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0), > + nand->regs + MA35_NFI_REG_NANDCTL); > + for (i = 0; i < len; i++) > + buf[i] = (u8)readl(nand->regs + MA35_NFI_REG_NANDDATA); > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0, > + nand->regs + MA35_NFI_REG_NANDCTL); > +} > +/* > + * write data to NAND controller > + */ > + > +static void ma35_nand_write_buf(struct nand_chip *chip, const u8 *buf, int len) > +{ > + struct ma35_nand_info *nand = nand_get_controller_data(chip); > + int i; > + > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0), > + nand->regs + MA35_NFI_REG_NANDCTL); > + for (i = 0; i < len; i++) > + writel(buf[i], nand->regs + MA35_NFI_REG_NANDDATA); > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0, > + nand->regs + MA35_NFI_REG_NANDCTL); > +} > + > +/* > + * configure and start dma transfer > + */ > +static inline int ma35_nand_dma_transfer(struct nand_chip *chip, > + const u_char *addr, u32 len, int is_write) > +{ > + struct ma35_nand_info *nand = nand_get_controller_data(chip); > + struct mtd_info *mtd = nand_to_mtd(chip); > + unsigned long timeo = jiffies + HZ/2; > + dma_addr_t dma_addr; > + int ret; > + > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0), > + nand->regs+MA35_NFI_REG_NANDCTL); > + /* For save, wait DMAC to ready */ Please rework this comment > + while (1) { > + if ((readl(nand->regs + MA35_NFI_REG_DMACTL) & DMA_BUSY) == 0) > + break; > + if (time_after(jiffies, timeo)) > + return -ETIMEDOUT; readl_poll_timeout() > + } > + > + /* Reinitial dmac */ > + ma35_nand_dmac_init(nand); > + > + writel(mtd->oobsize, nand->regs + MA35_NFI_REG_NANDRACTL); > + > + /* setup and start DMA using dma_addr */ > + if (is_write) { > + u8 *ptr = (u8 *)((long)nand->regs + MA35_NFI_REG_NANDRA0); > + > + writel(INT_DMA, nand->regs + MA35_NFI_REG_NANDINTEN); > + /* To mark this page as dirty. */ > + if (ptr[3] == 0xFF) > + ptr[3] = 0; > + if (ptr[2] == 0xFF) > + ptr[2] = 0; > + > + /* Fill dma_addr */ > + dma_addr = dma_map_single(nand->dev, (void *)addr, len, DMA_TO_DEVICE); > + dma_sync_single_for_device(nand->dev, dma_addr, len, DMA_TO_DEVICE); > + ret = dma_mapping_error(nand->dev, dma_addr); > + if (ret) { > + dev_err(nand->dev, "dma mapping error\n"); > + return -EINVAL; > + } > + > + writel((unsigned long)dma_addr, nand->regs + MA35_NFI_REG_DMASA); > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DMA_W_EN, > + nand->regs + MA35_NFI_REG_NANDCTL); > + wait_for_completion_timeout(&nand->complete, msecs_to_jiffies(1000)); > + > + dma_unmap_single(nand->dev, dma_addr, len, DMA_TO_DEVICE); > + } else { > + writel(INT_DMA | INT_ECC, nand->regs + MA35_NFI_REG_NANDINTEN); > + /* Fill dma_addr */ > + dma_addr = dma_map_single(nand->dev, (void *)addr, len, DMA_FROM_DEVICE); > + ret = dma_mapping_error(nand->dev, dma_addr); > + if (ret) { > + dev_err(nand->dev, "dma mapping error\n"); > + return -EINVAL; > + } > + nand->dma_buf = (u8 *) addr; the cast should not be neede > + nand->dma_addr = dma_addr; > + > + writel((unsigned long)dma_addr, nand->regs + MA35_NFI_REG_DMASA); > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DMA_R_EN, > + nand->regs + MA35_NFI_REG_NANDCTL); > + wait_for_completion_timeout(&nand->complete, msecs_to_jiffies(1000)); Always check return values. > + > + dma_sync_single_for_cpu(nand->dev, dma_addr, len, DMA_FROM_DEVICE); > + dma_unmap_single(nand->dev, dma_addr, len, DMA_FROM_DEVICE); > + } > + > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0, > + nand->regs + MA35_NFI_REG_NANDCTL); > + > + return 0; > +} > + > +static void ma35_read_buf_dma(struct nand_chip *chip, u_char *buf, int len) > +{ > + struct mtd_info *mtd = nand_to_mtd(chip); > + > + if (len == mtd->writesize) > + ma35_nand_dma_transfer(chip, buf, len, 0x0); > + else > + ma35_nand_read_buf(chip, buf, len); > +} > + > +static void ma35_write_buf_dma(struct nand_chip *chip, const u_char *buf, int len) > +{ > + struct mtd_info *mtd = nand_to_mtd(chip); > + > + if (len == mtd->writesize) > + ma35_nand_dma_transfer(chip, (u_char *)buf, len, 0x1); > + else > + ma35_nand_write_buf(chip, buf, len); > +} > + This... > +static int ma35_nand_devready(struct nand_chip *chip) > +{ > + struct ma35_nand_info *nand = nand_get_controller_data(chip); > + unsigned int val; > + > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0), > + nand->regs+MA35_NFI_REG_NANDCTL); > + val = (readl(nand->regs + MA35_NFI_REG_NANDINTSTS) & INT_RB0_STS) ? 1 : 0; > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0, > + nand->regs+MA35_NFI_REG_NANDCTL); > + > + return val; > +} > + > +static int ma35_waitfunc(struct nand_chip *chip) > +{ > + struct ma35_nand_info *nand = nand_get_controller_data(chip); > + unsigned long timeo = jiffies; > + int status = -1; > + > + timeo += msecs_to_jiffies(400); > + > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0), > + nand->regs + MA35_NFI_REG_NANDCTL); > + while (time_before(jiffies, timeo)) { > + status = readl(nand->regs + MA35_NFI_REG_NANDINTSTS); > + if (status & INT_RB0) { > + writel(INT_RB0, nand->regs + MA35_NFI_REG_NANDINTSTS); > + status = 0; > + break; > + } > + cond_resched(); > + } > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0, > + nand->regs + MA35_NFI_REG_NANDCTL); > + > + return status; > +} > + > +static void ma35_nand_command(struct nand_chip *chip, u32 command, int column, int page_addr) > +{ > + struct ma35_nand_info *nand = nand_get_controller_data(chip); > + struct mtd_info *mtd = nand_to_mtd(chip); > + > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0), > + nand->regs + MA35_NFI_REG_NANDCTL); > + writel(INT_RB0, nand->regs + MA35_NFI_REG_NANDINTSTS); > + > + if (command == NAND_CMD_READOOB) { > + command = NAND_CMD_READ0; > + column += mtd->writesize; > + } > + > + switch (command) { > + case NAND_CMD_RESET: > + writel(command, nand->regs + MA35_NFI_REG_NANDCMD); > + break; > + > + case NAND_CMD_READID: > + writel(command, nand->regs + MA35_NFI_REG_NANDCMD); > + writel(ENDADDR|column, nand->regs + MA35_NFI_REG_NANDADDR); > + break; > + > + case NAND_CMD_PARAM: > + writel(command, nand->regs + MA35_NFI_REG_NANDCMD); > + writel(ENDADDR|column, nand->regs + MA35_NFI_REG_NANDADDR); > + ma35_waitfunc(chip); > + break; > + > + case NAND_CMD_READ0: > + writel(ENABLE_WP, nand->regs + MA35_NFI_REG_NANDECTL); > + writel(command, nand->regs + MA35_NFI_REG_NANDCMD); > + if (column != -1) { > + writel(column & 0xff, nand->regs + MA35_NFI_REG_NANDADDR); > + writel((column >> 8) & 0xff, nand->regs + MA35_NFI_REG_NANDADDR); > + } > + if (page_addr != -1) { > + writel(page_addr & 0xff, nand->regs + MA35_NFI_REG_NANDADDR); > + if (chip->options & NAND_ROW_ADDR_3) { > + writel((page_addr >> 8) & 0xff, > + nand->regs + MA35_NFI_REG_NANDADDR); > + writel(((page_addr >> 16) & 0xff) | ENDADDR, > + nand->regs + MA35_NFI_REG_NANDADDR); > + } else { > + writel(((page_addr >> 8) & 0xff) | ENDADDR, > + nand->regs + MA35_NFI_REG_NANDADDR); > + } > + } > + writel(NAND_CMD_READSTART, nand->regs + MA35_NFI_REG_NANDCMD); > + ma35_waitfunc(chip); > + break; > + > + case NAND_CMD_ERASE1: > + writel(DISABLE_WP, nand->regs + MA35_NFI_REG_NANDECTL); > + writel(command, nand->regs + MA35_NFI_REG_NANDCMD); > + writel(page_addr & 0xff, nand->regs + MA35_NFI_REG_NANDADDR); > + if (chip->options & NAND_ROW_ADDR_3) { > + writel((page_addr >> 8) & 0xff, > + nand->regs + MA35_NFI_REG_NANDADDR); > + writel(((page_addr >> 16) & 0xff) | ENDADDR, > + nand->regs + MA35_NFI_REG_NANDADDR); > + } else { > + writel(((page_addr >> 8) & 0xff) | ENDADDR, > + nand->regs + MA35_NFI_REG_NANDADDR); > + } > + break; > + > + case NAND_CMD_SEQIN: > + writel(DISABLE_WP, nand->regs + MA35_NFI_REG_NANDECTL); > + writel(command, nand->regs + MA35_NFI_REG_NANDCMD); > + writel(column & 0xff, nand->regs + MA35_NFI_REG_NANDADDR); > + writel(column >> 8, nand->regs + MA35_NFI_REG_NANDADDR); > + writel(page_addr & 0xff, nand->regs + MA35_NFI_REG_NANDADDR); > + if (chip->options & NAND_ROW_ADDR_3) { > + writel((page_addr >> 8) & 0xff, > + nand->regs + MA35_NFI_REG_NANDADDR); > + writel(((page_addr >> 16) & 0xff) | ENDADDR, > + nand->regs + MA35_NFI_REG_NANDADDR); > + } else { > + writel(((page_addr >> 8) & 0xff) | ENDADDR, > + nand->regs + MA35_NFI_REG_NANDADDR); > + } > + break; > + > + case NAND_CMD_STATUS: > + writel(DISABLE_WP, nand->regs + MA35_NFI_REG_NANDECTL); > + writel(command, nand->regs + MA35_NFI_REG_NANDCMD); > + break; > + > + default: > + writel(command, nand->regs + MA35_NFI_REG_NANDCMD); > + } > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0, > + nand->regs + MA35_NFI_REG_NANDCTL); > +} ... is totally legacy. ->exec_op() is the API to implement instead. > + > +/* select chip */ > +static void ma35_nand_select_chip(struct nand_chip *chip, int cs) > +{ > + struct ma35_nand_info *nand = nand_get_controller_data(chip); > + > + if (cs == 0) > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0), > + nand->regs + MA35_NFI_REG_NANDCTL); > + else > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0, > + nand->regs + MA35_NFI_REG_NANDCTL); > +} > + > +static int ma35_nand_calculate_ecc(struct nand_chip *chip, const u_char *dat, > + u_char *ecc_code) > +{ > + return 0; > +} > + > +static int ma35_nand_write_page_hwecc(struct nand_chip *chip, const u8 *buf, > + int oob_required, int page) > +{ > + struct ma35_nand_info *nand = nand_get_controller_data(chip); > + u8 *ptr = (u8 *)((long)nand->regs + MA35_NFI_REG_NANDRA0); > + struct mtd_info *mtd = nand_to_mtd(chip); > + u8 *ecc_calc = chip->ecc.calc_buf; > + > + memset((void *)ptr, 0xFF, mtd->oobsize); > + memcpy((void *)ptr, (void *)chip->oob_poi, mtd->oobsize - chip->ecc.total); > + > + ma35_nand_command(chip, NAND_CMD_SEQIN, 0, page); > + ma35_nand_dma_transfer(chip, buf, mtd->writesize, 0x1); > + ma35_nand_command(chip, NAND_CMD_PAGEPROG, -1, -1); > + ma35_waitfunc(chip); > + > + /* Copy parity code in NANDRA to calc */ > + memcpy((void *)ecc_calc, > + (void *)(ptr + (mtd->oobsize - chip->ecc.total)), > + chip->ecc.total); > + > + /* Copy parity code in calc to oob_poi */ > + memcpy((void *)(chip->oob_poi + (mtd->oobsize - chip->ecc.total)), > + (void *)ecc_calc, chip->ecc.total); > + > + return 0; > +} > + > +static int ma35_nand_read_page_hwecc_oob_first(struct nand_chip *chip, u8 *buf, > + int oob_required, int page) > +{ > + struct ma35_nand_info *nand = nand_get_controller_data(chip); > + char *ptr = (char *)((long)nand->regs + MA35_NFI_REG_NANDRA0); > + struct mtd_info *mtd = nand_to_mtd(chip); > + > + /* 1. read the OOB area */ > + ma35_nand_command(chip, NAND_CMD_READOOB, 0, page); > + ma35_nand_read_buf(chip, chip->oob_poi, mtd->oobsize); > + > + /* 2. copy OOB data to NANDRA for page read */ > + memcpy((void *)ptr, (void *)chip->oob_poi, mtd->oobsize); > + > + if ((*(ptr+2) != 0) && (*(ptr+3) != 0)) > + memset((void *)buf, 0xff, mtd->writesize); > + else { > + /* 3. read data from nand */ > + ma35_nand_command(chip, NAND_CMD_READ0, 0, page); > + ma35_nand_dma_transfer(chip, buf, mtd->writesize, 0x0); > + > + /* 4. restore OOB data from SMRA */ > + memcpy((void *)chip->oob_poi, (void *)ptr, mtd->oobsize); > + } > + > + return 0; > +} > + Not sure why you need these two implementations? Usually there is only one per controller and fully depends on your controller capabilities/limitations. If you have no limitation, just support the simple case. > +static void ma35_layout_oob_table(struct nand_ecclayout_user *oobtable, > + int oobsize, int eccbytes) > +{ > + oobtable->eccbytes = eccbytes; > + oobtable->oobavail = oobsize - SKIP_SPARE_BYTES - eccbytes; > + oobtable->oobfree[0].offset = SKIP_SPARE_BYTES; > + oobtable->oobfree[0].length = oobsize - eccbytes - oobtable->oobfree[0].offset; > + > + oobtable->oobfree[1].offset = 0; > + oobtable->oobfree[1].length = 0; > +} I'm not sure why you would need that. > + > +static int ma35_nand_read_oob_hwecc(struct nand_chip *chip, int page) > +{ > + struct ma35_nand_info *nand = nand_get_controller_data(chip); > + char *ptr = (char *)((long)nand->regs + MA35_NFI_REG_NANDRA0); > + struct mtd_info *mtd = nand_to_mtd(chip); > + > + ma35_nand_command(chip, NAND_CMD_READOOB, 0, page); > + ma35_nand_read_buf(chip, chip->oob_poi, mtd->oobsize); > + > + /* copy OOB data to NANDRA for page read */ > + memcpy((void *)ptr, (void *)chip->oob_poi, mtd->oobsize); > + > + if ((*(ptr+2) != 0) && (*(ptr+3) != 0)) > + memset((void *)chip->oob_poi, 0xff, mtd->oobsize); > + > + return 0; > +} > + > +static irqreturn_t ma35_nand_irq(int irq, void *id) > +{ > + struct ma35_nand_info *nand = (struct ma35_nand_info *)id; > + struct mtd_info *mtd = nand_to_mtd(&nand->chip); > + int stat = 0; > + u32 isr; > + > + spin_lock(&nand->dma_lock); > + > + isr = readl(nand->regs + MA35_NFI_REG_NANDINTSTS); > + if (isr & INT_ECC) { > + dma_sync_single_for_cpu(nand->dev, nand->dma_addr, mtd->writesize, > + DMA_FROM_DEVICE); > + stat = ma35_nfi_correct(&nand->chip, (unsigned long)nand->dma_buf); > + if (stat < 0) { > + mtd->ecc_stats.failed++; > + writel(DMA_RST | DMA_EN, nand->regs + MA35_NFI_REG_DMACTL); > + writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | SWRST, > + nand->regs + MA35_NFI_REG_NANDCTL); > + } else if (stat > 0) { > + mtd->ecc_stats.corrected += stat; /* Add corrected bit count */ > + } > + writel(INT_ECC, nand->regs + MA35_NFI_REG_NANDINTSTS); > + } > + if (isr & INT_DMA) { > + writel(INT_DMA, nand->regs + MA35_NFI_REG_NANDINTSTS); > + complete(&nand->complete); > + } > + spin_unlock(&nand->dma_lock); > + > + return IRQ_HANDLED; > +} > + > +static int ma35_nand_attach_chip(struct nand_chip *chip) > +{ > + struct ma35_nand_info *nand = nand_get_controller_data(chip); > + struct mtd_info *mtd = nand_to_mtd(chip); > + unsigned int reg; > + Please have a look at other controller drivers. You need to allow: - no correction - sw correction - hw BCH correction > + /* Set PSize */ > + reg = readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~PSIZE_MASK); > + if (mtd->writesize == 2048) > + writel(reg | PSIZE_2K, nand->regs + MA35_NFI_REG_NANDCTL); > + else if (mtd->writesize == 4096) > + writel(reg | PSIZE_4K, nand->regs + MA35_NFI_REG_NANDCTL); > + else if (mtd->writesize == 8192) > + writel(reg | PSIZE_8K, nand->regs + MA35_NFI_REG_NANDCTL); > + > + if (chip->ecc.strength == 0) { > + nand->bch = BCH_NONE; /* No ECC */ > + ma35_layout_oob_table(&nand->nand_oob, mtd->oobsize, > + ma35_parity[mtd->writesize>>12][0]); > + > + } else if (chip->ecc.strength <= 8) { > + nand->bch = BCH_T8; /* T8 */ > + ma35_layout_oob_table(&nand->nand_oob, mtd->oobsize, > + ma35_parity[mtd->writesize>>12][1]); > + > + } else if (chip->ecc.strength <= 12) { > + nand->bch = BCH_T12; /* T12 */ > + ma35_layout_oob_table(&nand->nand_oob, mtd->oobsize, > + ma35_parity[mtd->writesize>>12][2]); > + > + } else if (chip->ecc.strength <= 24) { > + nand->bch = BCH_T24; /* T24 */ > + ma35_layout_oob_table(&nand->nand_oob, mtd->oobsize, > + ma35_parity[mtd->writesize>>12][3]); > + > + } else { > + pr_warn("NAND Controller is not support this flash. (%d, %d)\n", > + mtd->writesize, mtd->oobsize); > + } > + > + chip->ecc.steps = mtd->writesize / chip->ecc.size; > + chip->ecc.bytes = nand->nand_oob.eccbytes / chip->ecc.steps; > + chip->ecc.total = nand->nand_oob.eccbytes; > + mtd_set_ooblayout(mtd, &ma35_ooblayout_ops); > + > + /* add mtd-id. The string should same as uboot definition */ > + mtd->name = "nand0"; > + > + ma35_nand_hwecc_init(nand); > + > + writel(ENABLE_WP, nand->regs + MA35_NFI_REG_NANDECTL); > + > + return 0; > +} > + > +static const struct nand_controller_ops ma35_nand_controller_ops = { > + .attach_chip = ma35_nand_attach_chip, > +}; > + > +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"); devm_clk_get_enabled() > + 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 */ Please convert this driver to ->exec_op(). No introduce of legacy hooks will be accepted. > + > + /* 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"); > + > + retval = devm_request_irq(&pdev->dev, nand->irq, ma35_nand_irq, > + IRQF_TRIGGER_HIGH, "ma35d1-nand", nand); > + if (retval) { > + dev_err(&pdev->dev, "failed to request NAND irq\n"); > + clk_disable_unprepare(nand->clk); > + return -ENXIO; > + } > + > + retval = nand_scan(chip, 1); s/retval/ret/ > + if (retval) > + return retval; > + > + if (mtd_device_register(mtd, nand->parts, nand->nr_parts)) { ret = if (ret) > + 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); > + int ret; > + > + devm_free_irq(&pdev->dev, nand->irq, nand); > + ret = mtd_device_unregister(nand_to_mtd(&nand->chip)); > + WARN_ON(ret); > + nand_cleanup(&nand->chip); > + clk_disable_unprepare(nand->clk); > +} > + > +/* 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 = { > + .name = "ma35d1-nand", > + .of_match_table = ma35_nfi_of_match, > + }, > + .probe = ma35_nand_probe, > + .remove_new = ma35_nand_remove, > + .suspend = ma35_nand_suspend, > + .resume = ma35_nand_resume, > +}; > + > +module_platform_driver(ma35_nand_driver); > + > +MODULE_DESCRIPTION("Nuvoton ma35 NAND driver"); > +MODULE_AUTHOR("Hui-Ping Chen <hpchen0nvt(a)gmail.com>"); > +MODULE_LICENSE("GPL"); Thanks, Miquèl

9 months, 2 weeks

1
0
0 0

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

by Miquel Raynal

Hi, hpchen0nvt(a)gmail.com wrote on Wed, 21 Aug 2024 07:11:31 +0000: > Add dt-bindings for the Nuvoton MA35 SoC NAND Controller. > > Signed-off-by: Hui-Ping Chen <hpchen0nvt(a)gmail.com> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> > --- > .../bindings/mtd/nuvoton,ma35d1-nand.yaml | 93 +++++++++++++++++++ > 1 file changed, 93 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml > > diff --git a/Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml b/Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml > new file mode 100644 > index 000000000000..152784e73263 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml > @@ -0,0 +1,93 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mtd/nuvoton,ma35d1-nand.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Nuvoton MA35D1 NAND Flash Interface (NFI) Controller > + > +maintainers: > + - Hui-Ping Chen <hpchen0nvt(a)gmail.com> > + > +allOf: > + - $ref: nand-controller.yaml# > + > +properties: > + compatible: > + enum: > + - nuvoton,ma35d1-nand Can we please use the -nand-controller suffix. A NAND is a the common name for a chip with storage inside. You are describing a host controller that can be connected to in order to talk to a NAND. Thanks, Miquèl

9 months, 2 weeks

1
0
0 0

[PATCH v5 0/9] dma-buf: heaps: Add restricted heap

by Yong Wu

The purpose of this patchset is for MediaTek secure video playback, and also to enable other potential uses of this in the future. The 'restricted dma-heap' will be used to allocate dma_buf objects that reference memory in the secure world that is inaccessible/unmappable by the non-secure (i.e. kernel/userspace) world. That memory will be used by the secure/ trusted world to store secure information (i.e. decrypted media content). The dma_bufs allocated from the kernel will be passed to V4L2 for video decoding (as input and output). They will also be used by the drm system for rendering of the content. This patchset adds two MediaTek restricted heaps and they will be used in v4l2[1] and drm[2]. 1) restricted_mtk_cm: secure chunk memory for MediaTek SVP (Secure Video Path). The buffer is reserved for the secure world after bootup and it is used for vcodec's ES/working buffer; 2) restricted_mtk_cma: secure CMA memory for MediaTek SVP. This buffer is dynamically reserved for the secure world and will be got when we start playing secure videos. Once the security video playing is complete, the CMA will be released. This heap is used for the vcodec's frame buffer. [1] https://lore.kernel.org/linux-mediatek/20240412090851.24999-1-yunfei.dong@m… [2] https://lore.kernel.org/linux-mediatek/20240403102701.369-1-shawn.sung@medi… Change note: v5: 1) Reconstruct TEE commands to allow the kernel to obtain the PA of the TEE buffer to initialize a valid sg table. 2) Previously, PA was hidden from the kernel. Then the kernel checks if this is restricted buffer by "if (sg_page(sg) == NULL)". In this version, we will add a new explicit interface (sg_dma_is_restricted) for users to determine whether this is a restricted buffer. 3) some words improve, like using "rheap". Rebase on v6.9-rc7. v4: https://lore.kernel.org/linux-mediatek/20240112092014.23999-1-yong.wu@media… 1) Rename the heap name from "secure" to "restricted". suggested from Simon/Pekka. There are still several "secure" string in MTK file since we use ARM platform in which we call this "secure world"/ "secure command". v3: https://lore.kernel.org/linux-mediatek/20231212024607.3681-1-yong.wu@mediat… 1) Separate the secure heap to a common file(secure_heap.c) and mtk special file (secure_heap_mtk.c), and put all the tee related code into our special file. 2) About dt-binding, Add "mediatek," prefix since this is Mediatek TEE firmware definition. 3) Remove the normal CMA heap which is a draft for qcom. Rebase on v6.7-rc1. v2: https://lore.kernel.org/linux-mediatek/20231111111559.8218-1-yong.wu@mediat… 1) Move John's patches into the vcodec patchset since they use the new dma heap interface directly. https://lore.kernel.org/linux-mediatek/20231106120423.23364-1-yunfei.dong@m… 2) Reword the dt-binding description. 3) Rename the heap name from mtk_svp to secure_mtk_cm. This means the current vcodec/DRM upstream code doesn't match this. 4) Add a normal CMA heap. currently it should be a draft version. 5) Regarding the UUID, I still use hard code, but put it in a private data which allow the others could set their own UUID. What's more, UUID is necessary for the session with TEE. If we don't have it, we can't communicate with the TEE, including the get_uuid interface, which tries to make uuid more generic, not working. If there is other way to make UUID more general, please free to tell me. v1: https://lore.kernel.org/linux-mediatek/20230911023038.30649-1-yong.wu@media… Base on v6.6-rc1. Yong Wu (9): dt-bindings: reserved-memory: Add mediatek,dynamic-restricted-region scatterlist: Add a flag for the restricted memory lib/scatterlist: Add sg_dup_table dma-buf: heaps: Initialize a restricted heap dma-buf: heaps: restricted_heap: Add private heap ops dma-buf: heaps: restricted_heap: Add dma_ops dma-buf: heaps: restricted_heap: Add MediaTek restricted heap and heap_init dma-buf: heaps: restricted_heap_mtk: Add TEE memory service call dma_buf: heaps: restricted_heap_mtk: Add a new CMA heap .../mediatek,dynamic-restricted-region.yaml | 43 ++ drivers/dma-buf/heaps/Kconfig | 16 + drivers/dma-buf/heaps/Makefile | 4 +- drivers/dma-buf/heaps/restricted_heap.c | 219 +++++++++ drivers/dma-buf/heaps/restricted_heap.h | 45 ++ drivers/dma-buf/heaps/restricted_heap_mtk.c | 423 ++++++++++++++++++ drivers/dma-buf/heaps/system_heap.c | 27 +- include/linux/scatterlist.h | 36 ++ lib/scatterlist.c | 26 ++ 9 files changed, 812 insertions(+), 27 deletions(-) create mode 100644 Documentation/devicetree/bindings/reserved-memory/mediatek,dynamic-restricted-region.yaml create mode 100644 drivers/dma-buf/heaps/restricted_heap.c create mode 100644 drivers/dma-buf/heaps/restricted_heap.h create mode 100644 drivers/dma-buf/heaps/restricted_heap_mtk.c -- 2.18.0

9 months, 2 weeks

6
17
0 0

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

9 months, 2 weeks

1
0
0 0

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

9 months, 3 weeks

1
0
0 0

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

9 months, 3 weeks

1
0
0 0

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

9 months, 3 weeks

1
0
0 0

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

9 months, 3 weeks

1
0
0 0

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.

9 months, 3 weeks

1
0
0 0

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

9 months, 4 weeks

1
0
0 0

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

9 months, 4 weeks

1
0
0 0

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

9 months, 4 weeks

1
0
0 0

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

9 months, 4 weeks

1
0
0 0

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

by Krzysztof Kozlowski

On 12/08/2024 05:00, 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> > --- > drivers/mtd/nand/raw/Kconfig | 8 + > drivers/mtd/nand/raw/Makefile | 1 + > drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c | 1109 ++++++++++++++++++++ > 3 files changed, 1118 insertions(+) > create mode 100644 drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c > > diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig > index 614257308516..932bf2215470 100644 > --- a/drivers/mtd/nand/raw/Kconfig > +++ b/drivers/mtd/nand/raw/Kconfig > @@ -448,6 +448,14 @@ config MTD_NAND_RENESAS > Enables support for the NAND controller found on Renesas R-Car > Gen3 and RZ/N1 SoC families. > > +config MTD_NAND_NVT_MA35 > + tristate "Nuvoton MA35 SoC NAND controller" > + depends on ARCH_MA35 || COMPILE_TEST > + depends on OF > + help > + Enables support for the NAND controller found on > + the Nuvoton MA35 series SoCs. > + > comment "Misc" > > config MTD_SM_COMMON > diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile > index 25120a4afada..cdfdfee3f5f3 100644 > --- a/drivers/mtd/nand/raw/Makefile > +++ b/drivers/mtd/nand/raw/Makefile > @@ -57,6 +57,7 @@ obj-$(CONFIG_MTD_NAND_INTEL_LGM) += intel-nand-controller.o > obj-$(CONFIG_MTD_NAND_ROCKCHIP) += rockchip-nand-controller.o > obj-$(CONFIG_MTD_NAND_PL35X) += pl35x-nand-controller.o > obj-$(CONFIG_MTD_NAND_RENESAS) += renesas-nand-controller.o > +obj-$(CONFIG_MTD_NAND_NVT_MA35) += nuvoton_ma35d1_nand.o > > nand-objs := nand_base.o nand_legacy.o nand_bbt.o nand_timings.o nand_ids.o > nand-objs += nand_onfi.o > diff --git a/drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c b/drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c > new file mode 100644 > index 000000000000..7445eb489c38 > --- /dev/null > +++ b/drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c > @@ -0,0 +1,1109 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2024 Nuvoton Technology Corp. > + */ > +#include <linux/slab.h> > +#include <linux/init.h> > +#include <linux/module.h> > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/platform_device.h> > +#include <linux/delay.h> > +#include <linux/clk.h> > +#include <linux/err.h> > +#include <linux/of.h> > + > +#include <linux/mtd/mtd.h> > +#include <linux/mtd/partitions.h> > +#include <linux/mtd/rawnand.h> > +#include <linux/dma-mapping.h> > +#include <linux/dmaengine.h> > + > +/* NFI DMA Registers */ > +#define MA35_NFI_REG_BUFFER0 (0x000) > +#define MA35_NFI_REG_DMACTL (0x400) > +#define DMA_EN BIT(0) > +#define DMA_RST BIT(1) > +#define DMA_BUSY BIT(9) > + > +#define MA35_NFI_REG_DMASA (0x408) > +#define MA35_NFI_REG_DMABCNT (0x40C) > +#define MA35_NFI_REG_DMAINTEN (0x410) > +#define MA35_NFI_REG_DMAINTSTS (0x414) > + > +/* NFI Global Registers */ > +#define MA35_NFI_REG_GCTL (0x800) > +#define NAND_EN BIT(3) > +#define MA35_NFI_REG_GINTEN (0x804) > +#define MA35_NFI_REG_GINTSTS (0x808) > + > +/* NAND-type Flash Registers */ > +#define MA35_NFI_REG_NANDCTL (0x8A0) > +#define SWRST BIT(0) > +#define DMA_W_EN BIT(1) > +#define DMA_R_EN BIT(2) > +#define ECC_CHK BIT(7) > +#define PROT3BEN BIT(8) > +#define PSIZE_2K (1 << 16) > +#define PSIZE_4K (2 << 16) > +#define PSIZE_8K (3 << 16) > +#define PSIZE_MASK (3 << 16) > +#define BCH_T24 BIT(18) > +#define BCH_T8 BIT(20) > +#define BCH_T12 BIT(21) > +#define BCH_MASK (0x1f << 18) > +#define ECC_EN BIT(23) > +#define DISABLE_CS0 BIT(25) > + > +#define MA35_NFI_REG_NANDTMCTL (0x8A4) > +#define MA35_NFI_REG_NANDINTEN (0x8A8) > +#define MA35_NFI_REG_NANDINTSTS (0x8AC) > +#define INT_DMA BIT(0) > +#define INT_ECC BIT(2) > +#define INT_RB0 BIT(10) > +#define INT_RB0_STS BIT(18) > + > +#define MA35_NFI_REG_NANDCMD (0x8B0) > +#define MA35_NFI_REG_NANDADDR (0x8B4) > +#define ENDADDR BIT(31) > + > +#define MA35_NFI_REG_NANDDATA (0x8B8) > +#define MA35_NFI_REG_NANDRACTL (0x8BC) > +#define MA35_NFI_REG_NANDECTL (0x8C0) > +#define ENABLE_WP (0x0) > +#define DISABLE_WP BIT(0) > + > +#define MA35_NFI_REG_NANDECCES0 (0x8D0) > +#define ECC_STATUS_MASK (0x3) > +#define ECC_ERR_CNT_MASK (0x1f) > + > +#define MA35_NFI_REG_NANDECCES1 (0x8D4) > +#define MA35_NFI_REG_NANDECCES2 (0x8D8) > +#define MA35_NFI_REG_NANDECCES3 (0x8DC) > + > +/* NAND-type Flash BCH Error Address Registers */ > +#define MA35_NFI_REG_NANDECCEA0 (0x900) > +#define MA35_NFI_REG_NANDECCEA1 (0x904) > +#define MA35_NFI_REG_NANDECCEA2 (0x908) > +#define MA35_NFI_REG_NANDECCEA3 (0x90C) > +#define MA35_NFI_REG_NANDECCEA4 (0x910) > +#define MA35_NFI_REG_NANDECCEA5 (0x914) > +#define MA35_NFI_REG_NANDECCEA6 (0x918) > +#define MA35_NFI_REG_NANDECCEA7 (0x91C) > +#define MA35_NFI_REG_NANDECCEA8 (0x920) > +#define MA35_NFI_REG_NANDECCEA9 (0x924) > +#define MA35_NFI_REG_NANDECCEA10 (0x928) > +#define MA35_NFI_REG_NANDECCEA11 (0x92C) > + > +/* 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? > +static const int g_i32ParityNum[3][eBCH_CNT] = { This is not a Linux coding style. Do not send Windows or some other platform-independent patches to review. > + {0, 60, 92, 90}, /* for 2K */ > + {0, 120, 184, 180}, /* for 4K */ > + {0, 240, 368, 360}, /* for 8K */ > +}; > + > + > +struct ma35_nand_info { > + struct nand_controller controller; > + struct device *dev; > + void __iomem *regs; > + int irq; > + struct clk *clk; > + struct completion complete; > + > + struct mtd_info mtd; > + struct nand_chip chip; > + struct mtd_partition *parts; > + int nr_parts; > + > + int eBCHAlgo; No, use Linux coding style. > + u8 *dma_buf; > + spinlock_t dma_lock; > + dma_addr_t dma_addr; > +}; > + > +static int ma35_ooblayout_ecc(struct mtd_info *mtd, int section, > + struct mtd_oob_region *oobregion) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + struct nand_ecc_ctrl *ecc = &chip->ecc; > + > + if (section || !ecc->total) > + return -ERANGE; > + > + oobregion->length = ecc->total; > + oobregion->offset = mtd->oobsize - oobregion->length; > + > + return 0; > +} ... this code looks poor, really poor. > +} > + > +static int ma35_nfi_correct(struct nand_chip *chip, unsigned long addr) > +{ > + struct ma35_nand_info *nand = nand_get_controller_data(chip); > + struct mtd_info *mtd = nand_to_mtd(chip); > + int uStatus, i, j, i32FieldNum = 0; > + int uReportErrCnt = 0; > + int uErrorCnt = 0; > + More of weird coding style. Use Linux coding style. Patch is for Linux. > + if ((readl(nand->regs + MA35_NFI_REG_NANDCTL) & BCH_MASK) == BCH_T24) > + i32FieldNum = mtd->writesize / 1024; > + else > + i32FieldNum = mtd->writesize / 512; ... > +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(struct ma35_nand_info), GFP_KERNEL); sizeof(*) > + 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 = of_clk_get(pdev->dev.of_node, 0); No, no. That's not how you get a clock. Use devm_clk_get(). > + 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"); > + return -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) { > + dev_err(&pdev->dev, "failed to get platform irq\n"); Syntax is return dev_err_probe > + return -EINVAL; > + } > + > + if (request_irq(nand->irq, (irq_handler_t)&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; > + } > + > + pr_info("ma35-nfi: registered successfully! mtdid=%s\n", mtd->name); Drop, does not help. Drivers do not use pr_xxx anyway. Why your code is so inconsistent? It's like random junk glued together. > + > + 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; > + > + ret = mtd_device_unregister(nand_to_mtd(chip)); > + WARN_ON(ret); > + nand_cleanup(chip); > + > + clk_disable_unprepare(nand->clk); > + > + kfree(nand); > + platform_set_drvdata(pdev, NULL); > +} > + > +/* 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 = { > + .name = "ma35d1-nand", > + .owner = THIS_MODULE, > + .of_match_table = of_match_ptr(ma35_nfi_of_match), Drop of_match_ptr. You have warnings here. > + }, > + .probe = ma35_nand_probe, > + .remove_new = ma35_nand_remove, > + .suspend = ma35_nand_suspend, > + .resume = ma35_nand_resume, > +}; > + > +static int __init ma35_nand_init(void) > +{ > + int ret; > + > + pr_info("ma35 mtd nand driver\n"); NAK, drop, useless and really pointless. > + > + ret = platform_driver_register(&ma35_nand_driver); > + if (ret) { > + pr_warn("nand: failed to add device driver %s\n", > + ma35_nand_driver.driver.name); > + return ret; > + } > + > + return ret; > +} > + > +static void __exit ma35_nand_exit(void) > +{ > + platform_driver_unregister(&ma35_nand_driver); > + pr_info("nand: unregistered successfully!\n"); NAK, drop, useless and really pointless. > +} > + > +module_init(ma35_nand_init); > +module_exit(ma35_nand_exit); Do not open-code these functions, there are wrappers for this. > + > +MODULE_DESCRIPTION("Nuvoton ma35 NAND driver"); > +MODULE_AUTHOR("Hui-Ping Chen <hpchen0nvt(a)gmail.com>"); > +MODULE_LICENSE("GPL"); Best regards, Krzysztof

9 months, 4 weeks

1
0
0 0

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

by Krzysztof Kozlowski

On 12/08/2024 05:00, 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 | 97 +++++++++++++++++++ > 1 file changed, 97 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml > > diff --git a/Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml b/Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml > new file mode 100644 > index 000000000000..988c43bc6e99 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml > @@ -0,0 +1,97 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mtd/nuvoton,ma35d1-nand.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Nuvoton MA35D1 NAND Flash Interface (NFI) Controller > + > +allOf: > + - $ref: nand-controller.yaml# allOf goes usually just before properties. > + > +maintainers: > + - Hui-Ping Chen <hpchen0nvt(a)gmail.com> > + > +properties: > + compatible: > + enum: > + - nuvoton,ma35d1-nand > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + minItems: 1 This cannot be min only. It's unconstrained. maxItems instead. There is no existing code like this so this should make you wonder... > + > +patternProperties: > + "^nand@[a-f0-9]$": > + type: object > + $ref: raw-nand-chip.yaml > + properties: > + nand-ecc-mode: > + const: hw No, drop the property. It does not make sense to have it const. Compatible defines it. > + > + nand-ecc-step-size: > + enum: [512, 1024] No defaults? So is this required? > + > + nand-ecc-strength: > + enum: [8, 12, 24] No defaults? So is this required? > + > + nand-bus-width: > + const: 8 Drop property. > + > + unevaluatedProperties: false > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h> > + > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + nand-controller@401A0000 { > + compatible = "nuvoton,ma35d1-nand"; > + reg = <0x0 0x401A0000 0x0 0x1000>; > + interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clk NAND_GATE>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + nand@0 { > + reg = <0>; > + > + nand-on-flash-bbt; > + nand-ecc-mode = "hw"; Drop > + nand-ecc-step-size = <512>; > + nand-ecc-strength = <8>; > + nand-bus-width = <8>; Drop > + > + partitions { > + compatible = "fixed-partitions"; > + #address-cells = <1>; > + #size-cells = <1>; > + > + uboot@0 { > + label = "nand-uboot"; > + read-only; > + reg = <0x0 0x300000>; > + }; > + }; > + }; > + }; > + }; > + > +... Best regards, Krzysztof

9 months, 4 weeks

1
0
0 0

[PATCH rdma-next 0/8] Introducing Multi-Path DMA Support for mlx5 RDMA Driver

by Leon Romanovsky

From: Leon Romanovsky <leonro(a)nvidia.com> From Yishai, Overview -------- This patch series aims to enable multi-path DMA support, allowing an mlx5 RDMA device to issue DMA commands through multiple paths. This feature is critical for improving performance and reaching line rate in certain environments where issuing PCI transactions over one path may be significantly faster than over another. These differences can arise from various PCI generations in the system or the specific system topology. To achieve this functionality, we introduced a data direct DMA device that can serve the RDMA device by issuing DMA transactions on its behalf. The main key features and changes are described below. Multi-Path Discovery -------------------- API Implementation: * Introduced an API to discover multiple paths for a given mlx5 RDMA device. IOCTL Command: * Added a new IOCTL command, MLX5_IB_METHOD_GET_DATA_DIRECT_SYSFS_PATH, to the DEVICE object. When an affiliated Data-Direct/DMA device is present, its sysfs path is returned. Feature Activation by mlx5 RDMA Application ------------------------------------------- UVERBS Extension: * Extended UVERBS_METHOD_REG_DMABUF_MR over UVERBS_OBJECT_MR to include mlx5 extended flags. Access Flag: * Introduced the MLX5_IB_UAPI_REG_DMABUF_ACCESS_DATA_DIRECT flag, allowing applications to request the use of the affiliated DMA device for DMABUF registration. Data-Direct/DMA Device ---------------------- New Driver: * Introduced a new driver to manage the new DMA PF device ID (0x2100). Its registration/un-registration is handled as part of the mlx5_ib init/exit flows, with mlx5 IB devices as its clients. Functionality: * The driver does not interface directly with the firmware (no command interface, no caps, etc.) but works over PCI to activate its DMA functionality. It serves as the DMA device for efficiently accessing other PCI devices (e.g., GPU PF) and reads its VUID over PCI to handle NICs registrations with the same VUID. mlx5 IB RDMA Device --------------------------- VUID Query: * Reads its affiliated DMA PF VUID via the QUERY_VUID command with the data_direct bit set. Driver Registration: * Registers with the DMA PF driver to be notified upon bind/unbind. Application Request Handling: * Uses the DMA PF device upon application request as described above. DMABUF over Umem ---------------- Introduced an option to obtain a DMABUF UMEM using a different DMA device instead of the IB device, allowing the device to register over IOMMU with the expected DMA device for a given buffer registration. Further details are provided in the commit logs of the patches in this series. Thanks Yishai Hadas (8): net/mlx5: Add IFC related stuff for data direct RDMA/mlx5: Introduce the 'data direct' driver RDMA/mlx5: Add the initialization flow to utilize the 'data direct' device RDMA/umem: Add support for creating pinned DMABUF umem with a given dma device RDMA/umem: Introduce an option to revoke DMABUF umem RDMA: Pass uverbs_attr_bundle as part of '.reg_user_mr_dmabuf' API RDMA/mlx5: Add support for DMABUF MR registrations with Data-direct RDMA/mlx5: Introduce GET_DATA_DIRECT_SYSFS_PATH ioctl drivers/infiniband/core/umem_dmabuf.c | 66 +++- drivers/infiniband/core/uverbs_std_types_mr.c | 2 +- drivers/infiniband/hw/bnxt_re/ib_verbs.c | 3 +- drivers/infiniband/hw/bnxt_re/ib_verbs.h | 2 +- drivers/infiniband/hw/efa/efa.h | 2 +- drivers/infiniband/hw/efa/efa_verbs.c | 4 +- drivers/infiniband/hw/irdma/verbs.c | 2 +- drivers/infiniband/hw/mlx5/Makefile | 1 + drivers/infiniband/hw/mlx5/cmd.c | 21 ++ drivers/infiniband/hw/mlx5/cmd.h | 2 + drivers/infiniband/hw/mlx5/data_direct.c | 227 +++++++++++++ drivers/infiniband/hw/mlx5/data_direct.h | 23 ++ drivers/infiniband/hw/mlx5/main.c | 125 +++++++ drivers/infiniband/hw/mlx5/mlx5_ib.h | 22 +- drivers/infiniband/hw/mlx5/mr.c | 304 +++++++++++++++--- drivers/infiniband/hw/mlx5/odp.c | 5 +- drivers/infiniband/hw/mlx5/std_types.c | 55 +++- drivers/infiniband/hw/mlx5/umr.c | 93 ++++-- drivers/infiniband/hw/mlx5/umr.h | 1 + include/linux/mlx5/mlx5_ifc.h | 51 ++- include/rdma/ib_umem.h | 18 ++ include/rdma/ib_verbs.h | 2 +- include/uapi/rdma/mlx5_user_ioctl_cmds.h | 9 + include/uapi/rdma/mlx5_user_ioctl_verbs.h | 4 + 24 files changed, 944 insertions(+), 100 deletions(-) create mode 100644 drivers/infiniband/hw/mlx5/data_direct.c create mode 100644 drivers/infiniband/hw/mlx5/data_direct.h -- 2.45.2

10 months

1
9
0 0

Re: [PATCH v2 0/5] Introduce DMA_HEAP_ALLOC_AND_READ_FILE heap flag

by Daniel Vetter

On Thu, Aug 01, 2024 at 10:53:45AM +0800, Huan Yang wrote: > > 在 2024/8/1 4:46, Daniel Vetter 写道: > > On Tue, Jul 30, 2024 at 08:04:04PM +0800, Huan Yang wrote: > > > 在 2024/7/30 17:05, Huan Yang 写道: > > > > 在 2024/7/30 16:56, Daniel Vetter 写道: > > > > > [????????? daniel.vetter(a)ffwll.ch ????????? > > > > > https://aka.ms/LearnAboutSenderIdentification?????????????] > > > > > > > > > > On Tue, Jul 30, 2024 at 03:57:44PM +0800, Huan Yang wrote: > > > > > > UDMA-BUF step: > > > > > > 1. memfd_create > > > > > > 2. open file(buffer/direct) > > > > > > 3. udmabuf create > > > > > > 4. mmap memfd > > > > > > 5. read file into memfd vaddr > > > > > Yeah this is really slow and the worst way to do it. You absolutely want > > > > > to start _all_ the io before you start creating the dma-buf, ideally > > > > > with > > > > > everything running in parallel. But just starting the direct I/O with > > > > > async and then creating the umdabuf should be a lot faster and avoid > > > > That's greate, Let me rephrase that, and please correct me if I'm wrong. > > > > > > > > UDMA-BUF step: > > > > 1. memfd_create > > > > 2. mmap memfd > > > > 3. open file(buffer/direct) > > > > 4. start thread to async read > > > > 3. udmabuf create > > > > > > > > With this, can improve > > > I just test with it. Step is: > > > > > > UDMA-BUF step: > > > 1. memfd_create > > > 2. mmap memfd > > > 3. open file(buffer/direct) > > > 4. start thread to async read > > > 5. udmabuf create > > > > > > 6 . join wait > > > > > > 3G file read all step cost 1,527,103,431ns, it's greate. > > Ok that's almost the throughput of your patch set, which I think is close > > enough. The remaining difference is probably just the mmap overhead, not > > sure whether/how we can do direct i/o to an fd directly ... in principle > > it's possible for any file that uses the standard pagecache. > > Yes, for mmap, IMO, now that we get all folios and pin it. That's mean all > pfn it's got when udmabuf created. > > So, I think mmap with page fault is helpless for save memory but increase > the mmap access cost.(maybe can save a little page table's memory) > > I want to offer a patchset to remove it and more suitable for folios > operate(And remove unpin list). And contains some fix patch. > > I'll send it when I test it's good. > > > About fd operation for direct I/O, maybe use sendfile or copy_file_range? > > sendfile base pipe buffer, it's low performance when I test is. > > copy_file_range can't work due to it's not the same file system. > > So, I can't find other way to do it. Can someone give some suggestions? Yeah direct I/O to pagecache without an mmap might be too niche to be supported. Maybe io_uring has something, but I guess as unlikely as anything else. -Sima -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

10 months

1
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig