Hi Finn,
On Mon, Mar 5, 2018 at 2:01 PM, Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, 5 Mar 2018, Michael Schmitz wrote:
>
>> >> +fail_unmap_dma_regs:
>> >> + if (ioaddr > 0xffffff)
>> >> + iounmap(esp->dma_regs);
>> >
>> > I think you need to test for ZORRO_PROD_PHASE5_BLIZZARD_1230_IV_1260 here?
>>
>> On second thought - no, I don't. the ID check above only determines what
>> Zorro-3 address is ioremapped, but in each case where ioaddr > 0xffffff,
>> something will have been mapped at this point.
>>
>
> I think you are talking about esp->regs? For esp->dma_regs, the ioremap is
> conditional on ent->id, but the unmap is not.
The details of the ioremap are conditional on the ID, but the fact
that the ioremap happens (and hence esp->dma_regs is an ioremapped
address) is not. All Zorro-3 boards have to have both their regs and
dma_regs remapped.
What's confusing is that there is only a single Zorro-3 board
currently supported by the driver. Others will be added and I"ll use a
switch statement to pick the appropriate address based on the ID. That
might make it clearer.
>
>> >> +}
>> >> +
>> >> +static void zorro_esp_remove_one(struct zorro_dev *z)
>> >> +{
>> >> + struct Scsi_Host *host = zorro_get_drvdata(z);
>> >> + struct esp *esp = shost_priv(host);
>> >> +
>> >> + scsi_esp_unregister(esp);
>> >> +
>> >> + /* Disable interrupts. Perhaps use disable_irq instead ... */
>> >> +
>> >> + free_irq(host->irq, esp);
>> >> + dma_free_coherent(esp->dev, 16,
>> >> + esp->command_block,
>> >> + esp->command_block_dma);
>> >> +
>> >> + if (host->base > 0xffffff) {
>> >> + iounmap(esp->dma_regs);
>> >
>> > Do you need to test for ZORRO_PROD_PHASE5_BLIZZARD_1230_IV_1260 first?
>>
>> I can't - ent->id is not available here...
>
> Maybe store ent->id in the private struct to get around that?
Yes, that could be done. I still think it's not needed.
Cheers,
Michael
>
> --
