Hi,
On Tue, Jul 30, 2013 at 01:04:28PM +0530, Sourav Poddar wrote:
> Hi Felipe,
> On Monday 29 July 2013 06:05 PM, Felipe Balbi wrote:
> >Hi,
> >
> >On Mon, Jul 29, 2013 at 04:45:02PM +0530, Sourav Poddar wrote:
> >>>>>>>>+ irq = platform_get_irq(pdev, 0);
> >>>>>>>>+ if (irq< 0) {
> >>>>>>>>+ dev_err(&pdev->dev, "no irq resource?\n");
> >>>>>>>>+ return irq;
> >>>>>>>>+ }
> >>>>>>>>+
> >>>>>>>>+ spin_lock_init(&qspi->lock);
> >>>>>>>>+
> >>>>>>>>+ qspi->base = devm_ioremap_resource(&pdev->dev, r);
> >>>>>>>>+ if (IS_ERR(qspi->base)) {
> >>>>>>>>+ ret = PTR_ERR(qspi->base);
> >>>>>>>>+ goto free_master;
> >>>>>>>>+ }
> >>>>>>>>+
> >>>>>>>>+ ret = devm_request_threaded_irq(&pdev->dev, irq, ti_qspi_isr,
> >>>>>>>>+ ti_qspi_threaded_isr, IRQF_NO_SUSPEND |
> >>>>>>>>IRQF_ONESHOT,
> >>>>>>>why do you need IRQF_NO_SUSPEND ?
> >>>>>>>
> >>>>>>I should get away with this.
> >>>>>why ? Do you need or do you *not* need it ? And in either case, why ?
> >>>>>
> >>>>I was thinking, this will keep the irqs up even when we are
> >>>>hitting suspend, we will not be prepared to handle it. ?
> >>>won't be prepared in what way ?
> >>>
> >>Our driver will be down, so the irq might go un-serviced.
> >only if you wrote the driver that way. IRQ subsystem doesn't know the
> >state of the device, all it knows is that in case an IRQ fires, it needs
> >to call the registered IRQ handler.
> >
> >Now the thing is:
> >
> >Initially you had the flag setup, so I asked why you needed it. I
> >expected you to tell me why you think QSPI's IRQ shouldn't be disabled
> >during suspend and the implications of disabling it.
> >
> >Instead you just changed your mind and decided to remove the flag.
> >
> >Because you changed your mind with no explanation, that tells me you
> >haven't fully grasped how that flag works and what it means to set (or
> >not set) it.
> >
> >My question now is simply: why don't you need that flag ? What are the
> >implications of setting that flag ? How would your driver behave if an
> >IRQ fired while your driver was suspended ? Unless you understand what
> >it does, how can you understand the behavior or the driver ?
> >
> >cheers
> >
> We dont need IRQF_NO_SUSPEND flag in our qspi controller.
>
> Qspi driver need to be prevented from receiving interrupts during
> system wide suspend/hibernation.
> "suspend_device_irqs" in kernel/irq/pm.c marks interrupt line in use
> and sets IRQS_SUSPENDED
> for them.
> If we use "IRQF_NO_SUSPEND", we will bypass setting this
> IRQS_SUSPENDED flag, which is not.
> desired
>
>
> For this, interrupt lines need to
> and this function is provided for this purpose.
> * It marks all interrupt lines in use, except for the timer ones, as
> disabled
> * and sets the IRQS_SUSPENDED flag for each of them.
>
> This flag gets used in __disable_irq api(kernel/irq/manage.c) whichsome formatting issues in the mail, but good that you looked at the code. Cheers -- balbi
signature.asc
Description: Digital signature
