On 4/20/22 11:21, Zheyu Ma wrote: > On Wed, Apr 13, 2022 at 11:42 AM Damien Le Moal > <damien.lemoal@xxxxxxxxxxxxxxxxxx> wrote: >> >> On 4/12/22 15:34, Zheyu Ma wrote: >>> On Mon, Apr 11, 2022 at 7:53 AM Damien Le Moal >>> <damien.lemoal@xxxxxxxxxxxxxxxxxx> wrote: >>>> >>>> On 4/10/22 15:30, Zheyu Ma wrote: >>>>> Hello, >>>>> >>>>> I found a bug in the pata_marvell module. >>>>> When probing the driver, it seems to trigger the error path and >>>>> executes the function marvell_cable_detect(), but the >>>>> 'ap->ioaddr.bmdma_addr' is not initialized, which causes a warning. >>>> >>>> I do not have this hardware so I cannot debug this. Please debug it and >>>> send a patch. bmdma_addr is normally set in ata_pci_bmdma_init(), but some >>>> drivers set it manually in their probe functions. No idea about the >>>> marvell driver, I have not checked it. >>> >>> To be honest I don't have a good solution to this problem, because >>> other drivers don't have similar behavior. The marvell driver doesn't >>> even initialize 'bmdma_addr' before calling 'cable_detect'. >> >> Then this is the bug that needs to be fixed, no ? >> >>> So a simple idea I have is to check if 'bmdma_addr' is 0 before >>> reading it and if so return the error code ATA_CBL_NONE. >> >> And if indeed, even after it is initialized it is still 0, then yes, this >> change seems sensible. > > Sorry for the late reply, I found the root cause of this issue. > The marvell driver execute the ata_pci_bmdma_init() function, but the > driver just returned at the following code snippet. > > if (pci_resource_start(pdev, 4) == 0) { > ata_bmdma_nodma(host, "BAR4 is zero"); > return; > } > > So the driver didn't initialize the 'bmdma_addr' but used it in the > cable_detect() function. > It seems that the problem is caused by the hardware, is this a bug > that we should fix? So it looks like your adapter is saying: I do not support DMA. In that case, having bmdma_addr as 0 should be expected and pata_marvel_cable_detect() should check the address before attempting an ioread8(). It is weird that the cable information is in that bar though... In any case, you should check the adapter specs to verify how the cable type can be detected. And if unknown when bmdma_addr is 0, then just return ATA_CBL_PATA_UNK. > > Zheyu Ma > > > Zheyu Ma -- Damien Le Moal Western Digital Research