Re: [PATCH 23/29] atari_scsi: Convert to platform device

[Michael Schmitz <schmitzmic@gmail.com>]

Hi Finn,

>>>> Can these be handled through the platform_device's resources?
>>>>
>>>>     
>>>>         
>>> I don't know.
>>>   
>>>       
>> Should be possible - I've seen that used in the ISP116x HCD driver 
>> (arch-dependent post-register access delay function passed via platform 
>> data).
>>     
>
> Yes, possible, but is it desirable?
>   

Only long term, i.e. if we can find a way to fold all the separate 
*_NCR5380 drivers into a core one.


>
> If there's no third configuration then I see little to be gained from 
> completely parameterizing the driver using a bunch of resources.
>
> What am I missing? Why would it be desirable to have bits of driver code 
> in arch/m68k/ and other bits of the same driver kept elsewhere in the 
> tree (in drivers/)?
>
>   

Again, only makes sense if the goal is to reduce the driver to a core 
driver with implementation bits separate.

>>>> (and IRQ_NONE is wrong, you should use 0)
>>>>
>>>>     
>>>>         
>>>>> +       if (IS_A_TT()) {
>>>>>       
>>>>>           
>>>> Check for instance->irq instead?
>>>>     
>>>>         
>>> Yes, you'd think so, but a later patch (not in this set) would have to 
>>> change it back to IS_A_TT().
>>>
>>> Further patches align atari_NCR5380.c with NCR5380.c, such that the 
>>> core driver then checks host->irq to find out whether an IRQ is in 
>>> use. For Atari ST, request_irq() is not called even though the ST DMA 
>>> irq is in use.
>>>   
>>>       
>> That's why the IRQ is set to 0, I guess? Works for me.
>>     
>
> My patch tests for IS_A_TT not 0 because 0 has a different meaning 
> depending on which core driver you use. I don't want to support three 
> forks of NCR5380.c. One of the objectives of this patch series is to try 
> to move toward convergence on a common core driver.
>   

I wasn't talking about the use of IS_A_TT() here, rather about what 
host->irq = 0 achieves. Anyway, in the context of atari_scsi.c, 
host->irq == 0 is synonymous with !IS_A_TT(). If
host->irq == 0 is problematic with the long term goal of a singe 5380 
core driver, we'll just have to pick a value which means 'driver uses 
IRQ but you're not to fiddle with it'.
>> The old code states that setting instance->irq = 0 keeps the midlevel 
>> from tampering with the SCSI IRQ during queuecmd which would interfere 
>> with IDE and floppy.
>>     
>
> I don't know what you mean. AFAIK, the SCSI mid layer doesn't care about 
> instance->irq.
>   

You're right - that's another obsolete comment then. The midlevel uses 
spinlocks now for ages, this means we can set instance->irq to the 
actual IRQ used.

>> I guess this is still relevant - I would not have seen the ST-DMA locked 
>> by IDE during SCSI queuecmd otherwise.
>>     
>
> Lock-ups were due to disabling local IRQs. Please see,
>   

Absolutely right, but that's not what I meant. I assumed the midlevel 
disabled interrupts during queuecommand - it does not, so the point is 
moot. If using the actual IRQ for instance->irq helps at all, no 
objection to that.

SCSI commands can still be queued while IDE IO is in flight - that is 
what the 'ST-DMA locked by IDE' above refers to.

Cheers,

    Michael