On Sat, Mar 22, 2025 at 09:41:44AM -0400, Damien Le Moal wrote: > > + ap->ioaddr.data_addr = base; > > + ap->ioaddr.error_addr = base + 1 * 4; > > + ap->ioaddr.feature_addr = base + 1 * 4; > > + ap->ioaddr.nsect_addr = base + 2 * 4; > > + ap->ioaddr.lbal_addr = base + 3 * 4; > > + ap->ioaddr.lbam_addr = base + 4 * 4; > > + ap->ioaddr.lbah_addr = base + 5 * 4; > > + ap->ioaddr.device_addr = base + 6 * 4; > > + ap->ioaddr.status_addr = base + 7 * 4; > > + ap->ioaddr.command_addr = base + 7 * 4; > > + > > + ap->ioaddr.altstatus_addr = base + (0x1000 | (6UL << 2)); > > + ap->ioaddr.ctl_addr = base + (0x1000 | (6UL << 2)); > > It would be nice to have macro definitions for all the magic numbers you use for > offsets into base, to document these values. Thanks for the review Damien, unfortunately the original firmware this driver is based off doesn't define any macro for these regs/bits meaning. Other than that, i'll fix the rest and send a v2. -- bye, p.