On 1/21/21 4:13 AM, Lauri Kasanen wrote: > + > +static blk_status_t get_seg(struct request *req) Why not add prefix to the above function with n64 just like other function. Also, to me name of the function seems a bit generic, can it be more descriptive without being too long ? > +{ > + u32 bstart = blk_rq_pos(req) * 512; > + u32 len = blk_rq_cur_bytes(req); > + void *dst = bio_data(req->bio); > + > + if (bstart + len > size) > + return BLK_STS_IOERR; > + > + bstart += start; > + > + while (len) { > + const u32 curlen = len < BUFSIZE ? len : BUFSIZE; Why not use min_t instead of conditional operators which are discouraged in the kernel code. (this is coming from someone who got yelled at using ?: multiple times :P) > + > + dma_sync_single_for_device(disk_to_dev(disk), dma_addr, > + BUFSIZE, DMA_FROM_DEVICE); > + > + n64cart_wait_dma(); > + > + n64cart_write_reg(PI_DRAM_REG, dma_addr); > + n64cart_write_reg(PI_CART_REG, > + (bstart | CART_DOMAIN) & CART_MAX); > + n64cart_write_reg(PI_WRITE_REG, curlen - 1); > + > + n64cart_wait_dma(); > + > + dma_sync_single_for_cpu(disk_to_dev(disk), dma_addr, > + BUFSIZE, DMA_FROM_DEVICE); > + > + memcpy(dst, buf, curlen); > + > + len -= curlen; > + dst += curlen; > + bstart += curlen; > + } > + > + return BLK_STS_OK; > +}