Re: [PATCH v1 3/4] spi/xilinx: Simplify irq allocation

[Michal Simek]

On 07/08/2013 06:26 PM, Mark Brown wrote:
> On Mon, Jul 08, 2013 at 05:48:14PM +0200, Michal Simek wrote:
>> On 07/08/2013 04:49 PM, Mark Brown wrote:
> 
>>> Is it definitely safe to leave the IRQ hanging around after the master
>>> has been freed - there's no possibility of a late error interrupt or
>>> something?
> 
>> I think it is more generic question if this race condition is fine
>> for all drivers which are using devres groups.
> 
> Well, it's mainly an issue for IRQs - the other resources don't initiate
> events by themselves which is what causes the issue.  It just needs a
> bit of extra care so I wanted to check that this has been thought of.
> 
>> I have just looked at it and devres_release_all() is called where
>> driver is unload and irq are disabled there.
> 
> The problem is the gap between the resources used to handle the IRQ
> being freed and the IRQ itself being freed - if the hardware can be
> guaranteed to be idle then that's fine but we need to be sure that is
> OK.  Otherwise the interrupt handler may get run and be looking at a
> resource which was freed which would be unfortunate.
> 
>> btw: What's the proper way for spi driver unregistration?
> 
>> spi_unregistered_master() (which also free private structure)
>> and
>> spi_master_put()?
> 
> Yes.

Just a follow up on this.
Which function free private structures registered by spi_alloc_master function?
Is it in spi_master_put()?

The reason why I am asking is where clk_xx functions should be added.
I see them between these two functions in sifr for example.

And also I see in drivers in error probe path that drivers are calling kfree(master)
but they are not doing in remove part (like spi-davinci.c).

I just want to clear this in our zynq drivers before we send them out.

Thanks,
Michal




-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform

Attachment: signature.asc
Description: OpenPGP digital signature