| Message ID | 1450875495-2229-1-git-send-email-rsahu@apm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Wed, Dec 23, 2015 at 06:28:15PM +0530, Rameshswar Prasad Sahu wrote: > From: Rameshwar Prasad Sahu <rsahu@apm.com> > > For interrupt controller that doesn't support irq_disable and hardware > with level interrupt, an extra interrupt can be pending. This patch fixes > the issue by setting IRQ_DISABLE_UNLAZY flag for the interrupt line. > > Reference: http://git.kernel.org/tip/e9849777d0e27cdd2902805be51da73e7c79578c I seem to have got this patch thrice :( > > Signed-off-by: Rameshwar Prasad Sahu <rsahu@apm.com> > --- > drivers/dma/xgene-dma.c | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/drivers/dma/xgene-dma.c b/drivers/dma/xgene-dma.c > index 9dfa2b0..6363e84 100644 > --- a/drivers/dma/xgene-dma.c > +++ b/drivers/dma/xgene-dma.c > @@ -29,6 +29,7 @@ > #include <linux/dmapool.h> > #include <linux/interrupt.h> > #include <linux/io.h> > +#include <linux/irq.h> > #include <linux/module.h> > #include <linux/of_device.h> > > @@ -1610,6 +1611,7 @@ static int xgene_dma_request_irqs(struct xgene_dma *pdma) > /* Register DMA channel rx irq */ > for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++) { > chan = &pdma->chan[i]; > + irq_set_status_flags(chan->rx_irq, IRQ_DISABLE_UNLAZY); Why not use irq_settings_disable_unlazy(), at least read the reference you pointed out! > ret = devm_request_irq(chan->dev, chan->rx_irq, > xgene_dma_chan_ring_isr, > 0, chan->name, chan); > @@ -1620,6 +1622,7 @@ static int xgene_dma_request_irqs(struct xgene_dma *pdma) > > for (j = 0; j < i; j++) { > chan = &pdma->chan[i]; > + irq_clear_status_flags(chan->rx_irq, IRQ_DISABLE_UNLAZY); > devm_free_irq(chan->dev, chan->rx_irq, chan); > } > > @@ -1640,6 +1643,7 @@ static void xgene_dma_free_irqs(struct xgene_dma *pdma) > > for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++) { > chan = &pdma->chan[i]; > + irq_clear_status_flags(chan->rx_irq, IRQ_DISABLE_UNLAZY); > devm_free_irq(chan->dev, chan->rx_irq, chan); > } > } > -- > 1.7.1
Hi Vinod, On Wed, Jan 6, 2016 at 2:43 PM, Vinod Koul <vinod.koul@intel.com> wrote: > On Wed, Dec 23, 2015 at 06:28:15PM +0530, Rameshswar Prasad Sahu wrote: >> From: Rameshwar Prasad Sahu <rsahu@apm.com> >> >> For interrupt controller that doesn't support irq_disable and hardware >> with level interrupt, an extra interrupt can be pending. This patch fixes >> the issue by setting IRQ_DISABLE_UNLAZY flag for the interrupt line. >> >> Reference: http://git.kernel.org/tip/e9849777d0e27cdd2902805be51da73e7c79578c > > I seem to have got this patch thrice :( Due to mail failure (some issue was in my email client) to some email-id it was multiple times, > >> >> Signed-off-by: Rameshwar Prasad Sahu <rsahu@apm.com> >> --- >> drivers/dma/xgene-dma.c | 4 ++++ >> 1 files changed, 4 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/dma/xgene-dma.c b/drivers/dma/xgene-dma.c >> index 9dfa2b0..6363e84 100644 >> --- a/drivers/dma/xgene-dma.c >> +++ b/drivers/dma/xgene-dma.c >> @@ -29,6 +29,7 @@ >> #include <linux/dmapool.h> >> #include <linux/interrupt.h> >> #include <linux/io.h> >> +#include <linux/irq.h> >> #include <linux/module.h> >> #include <linux/of_device.h> >> >> @@ -1610,6 +1611,7 @@ static int xgene_dma_request_irqs(struct xgene_dma *pdma) >> /* Register DMA channel rx irq */ >> for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++) { >> chan = &pdma->chan[i]; >> + irq_set_status_flags(chan->rx_irq, IRQ_DISABLE_UNLAZY); > > Why not use irq_settings_disable_unlazy(), at least read the reference you > pointed out! irq_settings_disable_unlazy() is helper function to test IRQ_DISABLE_UNLAZY flag is set or not, it's not for setting this flag. FYI... +static inline bool irq_settings_disable_unlazy(struct irq_desc *desc) +{ + return desc->status_use_accessors & _IRQ_DISABLE_UNLAZY; +} > >> ret = devm_request_irq(chan->dev, chan->rx_irq, >> xgene_dma_chan_ring_isr, >> 0, chan->name, chan); >> @@ -1620,6 +1622,7 @@ static int xgene_dma_request_irqs(struct xgene_dma *pdma) >> >> for (j = 0; j < i; j++) { >> chan = &pdma->chan[i]; >> + irq_clear_status_flags(chan->rx_irq, IRQ_DISABLE_UNLAZY); >> devm_free_irq(chan->dev, chan->rx_irq, chan); >> } >> >> @@ -1640,6 +1643,7 @@ static void xgene_dma_free_irqs(struct xgene_dma *pdma) >> >> for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++) { >> chan = &pdma->chan[i]; >> + irq_clear_status_flags(chan->rx_irq, IRQ_DISABLE_UNLAZY); >> devm_free_irq(chan->dev, chan->rx_irq, chan); >> } >> } >> -- >> 1.7.1 > > -- > ~Vinod -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 06, 2016 at 02:51:07PM +0530, Rameshwar Sahu wrote: > Hi Vinod, > > On Wed, Jan 6, 2016 at 2:43 PM, Vinod Koul <vinod.koul@intel.com> wrote: > > On Wed, Dec 23, 2015 at 06:28:15PM +0530, Rameshswar Prasad Sahu wrote: > >> From: Rameshwar Prasad Sahu <rsahu@apm.com> > >> > >> For interrupt controller that doesn't support irq_disable and hardware > >> with level interrupt, an extra interrupt can be pending. This patch fixes > >> the issue by setting IRQ_DISABLE_UNLAZY flag for the interrupt line. > >> > >> Reference: http://git.kernel.org/tip/e9849777d0e27cdd2902805be51da73e7c79578c > > > > I seem to have got this patch thrice :( > Due to mail failure (some issue was in my email client) to some > email-id it was multiple times, git send-email ? > > > >> > >> Signed-off-by: Rameshwar Prasad Sahu <rsahu@apm.com> > >> --- > >> drivers/dma/xgene-dma.c | 4 ++++ > >> 1 files changed, 4 insertions(+), 0 deletions(-) > >> > >> diff --git a/drivers/dma/xgene-dma.c b/drivers/dma/xgene-dma.c > >> index 9dfa2b0..6363e84 100644 > >> --- a/drivers/dma/xgene-dma.c > >> +++ b/drivers/dma/xgene-dma.c > >> @@ -29,6 +29,7 @@ > >> #include <linux/dmapool.h> > >> #include <linux/interrupt.h> > >> #include <linux/io.h> > >> +#include <linux/irq.h> > >> #include <linux/module.h> > >> #include <linux/of_device.h> > >> > >> @@ -1610,6 +1611,7 @@ static int xgene_dma_request_irqs(struct xgene_dma *pdma) > >> /* Register DMA channel rx irq */ > >> for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++) { > >> chan = &pdma->chan[i]; > >> + irq_set_status_flags(chan->rx_irq, IRQ_DISABLE_UNLAZY); > > > > Why not use irq_settings_disable_unlazy(), at least read the reference you > > pointed out! > > irq_settings_disable_unlazy() is helper function to test > IRQ_DISABLE_UNLAZY flag is set or not, it's not for setting this flag. > FYI... > +static inline bool irq_settings_disable_unlazy(struct irq_desc *desc) > +{ > + return desc->status_use_accessors & _IRQ_DISABLE_UNLAZY; > +} Ah yes, I saw clear API and assumed there would be set. Then I think we should add a set helper as well as the usage is intended for drivers to set this flag Thomas, Any reason why you didn't add a set helper, only test and clear?
On Wed, Jan 6, 2016 at 3:01 PM, Vinod Koul <vinod.koul@intel.com> wrote: > On Wed, Jan 06, 2016 at 02:51:07PM +0530, Rameshwar Sahu wrote: >> Hi Vinod, >> >> On Wed, Jan 6, 2016 at 2:43 PM, Vinod Koul <vinod.koul@intel.com> wrote: >> > On Wed, Dec 23, 2015 at 06:28:15PM +0530, Rameshswar Prasad Sahu wrote: >> >> From: Rameshwar Prasad Sahu <rsahu@apm.com> >> >> >> >> For interrupt controller that doesn't support irq_disable and hardware >> >> with level interrupt, an extra interrupt can be pending. This patch fixes >> >> the issue by setting IRQ_DISABLE_UNLAZY flag for the interrupt line. >> >> >> >> Reference: http://git.kernel.org/tip/e9849777d0e27cdd2902805be51da73e7c79578c >> > >> > I seem to have got this patch thrice :( >> Due to mail failure (some issue was in my email client) to some >> email-id it was multiple times, > > git send-email ? Yes, was not able to send Arnd Bergmann. > >> > >> >> >> >> Signed-off-by: Rameshwar Prasad Sahu <rsahu@apm.com> >> >> --- >> >> drivers/dma/xgene-dma.c | 4 ++++ >> >> 1 files changed, 4 insertions(+), 0 deletions(-) >> >> >> >> diff --git a/drivers/dma/xgene-dma.c b/drivers/dma/xgene-dma.c >> >> index 9dfa2b0..6363e84 100644 >> >> --- a/drivers/dma/xgene-dma.c >> >> +++ b/drivers/dma/xgene-dma.c >> >> @@ -29,6 +29,7 @@ >> >> #include <linux/dmapool.h> >> >> #include <linux/interrupt.h> >> >> #include <linux/io.h> >> >> +#include <linux/irq.h> >> >> #include <linux/module.h> >> >> #include <linux/of_device.h> >> >> >> >> @@ -1610,6 +1611,7 @@ static int xgene_dma_request_irqs(struct xgene_dma *pdma) >> >> /* Register DMA channel rx irq */ >> >> for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++) { >> >> chan = &pdma->chan[i]; >> >> + irq_set_status_flags(chan->rx_irq, IRQ_DISABLE_UNLAZY); >> > >> > Why not use irq_settings_disable_unlazy(), at least read the reference you >> > pointed out! >> >> irq_settings_disable_unlazy() is helper function to test >> IRQ_DISABLE_UNLAZY flag is set or not, it's not for setting this flag. >> FYI... >> +static inline bool irq_settings_disable_unlazy(struct irq_desc *desc) >> +{ >> + return desc->status_use_accessors & _IRQ_DISABLE_UNLAZY; >> +} > > Ah yes, I saw clear API and assumed there would be set. Then I think we > should add a set helper as well as the usage is intended for drivers to > set this flag > > Thomas, > > Any reason why you didn't add a set helper, only test and clear? > > -- > ~Vinod -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 6 Jan 2016, Vinod Koul wrote: > On Wed, Jan 06, 2016 at 02:51:07PM +0530, Rameshwar Sahu wrote: > > >> @@ -1610,6 +1611,7 @@ static int xgene_dma_request_irqs(struct xgene_dma *pdma) > > >> /* Register DMA channel rx irq */ > > >> for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++) { > > >> chan = &pdma->chan[i]; > > >> + irq_set_status_flags(chan->rx_irq, IRQ_DISABLE_UNLAZY); > > > > > > Why not use irq_settings_disable_unlazy(), at least read the reference you > > > pointed out! > > > > irq_settings_disable_unlazy() is helper function to test > > IRQ_DISABLE_UNLAZY flag is set or not, it's not for setting this flag. > > FYI... > > +static inline bool irq_settings_disable_unlazy(struct irq_desc *desc) > > +{ > > + return desc->status_use_accessors & _IRQ_DISABLE_UNLAZY; > > +} > > Ah yes, I saw clear API and assumed there would be set. Then I think we > should add a set helper as well as the usage is intended for drivers to > set this flag > > Thomas, > > Any reason why you didn't add a set helper, only test and clear? Why would I? Those helpers are core internal and not usable in random drivers. Drivers have irq_set_status_flags()/irq_clear_status_flags() ... Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 6, 2016 at 3:40 PM, Suman Tripathi <stripathi@apm.com> wrote: > > > On Wed, Jan 6, 2016 at 3:35 PM, Thomas Gleixner <tglx@linutronix.de> wrote: >> >> On Wed, 6 Jan 2016, Vinod Koul wrote: >> > On Wed, Jan 06, 2016 at 02:51:07PM +0530, Rameshwar Sahu wrote: >> > > >> @@ -1610,6 +1611,7 @@ static int xgene_dma_request_irqs(struct >> > > >> xgene_dma *pdma) >> > > >> /* Register DMA channel rx irq */ >> > > >> for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++) { >> > > >> chan = &pdma->chan[i]; >> > > >> + irq_set_status_flags(chan->rx_irq, >> > > >> IRQ_DISABLE_UNLAZY); >> > > > >> > > > Why not use irq_settings_disable_unlazy(), at least read the >> > > > reference you >> > > > pointed out! >> > > >> > > irq_settings_disable_unlazy() is helper function to test >> > > IRQ_DISABLE_UNLAZY flag is set or not, it's not for setting this flag. >> > > FYI... >> > > +static inline bool irq_settings_disable_unlazy(struct irq_desc *desc) >> > > +{ >> > > + return desc->status_use_accessors & _IRQ_DISABLE_UNLAZY; >> > > +} >> > >> > Ah yes, I saw clear API and assumed there would be set. Then I think we >> > should add a set helper as well as the usage is intended for drivers to >> > set this flag >> > >> > Thomas, >> > >> > Any reason why you didn't add a set helper, only test and clear? >> >> Why would I? Those helpers are core internal and not usable in random >> drivers. > > > i think the problem is the name of the function. It should be something > > irq_check_settings_disable_unlazy resending in plain text mode. Sorry for this. I think the problem is the name of the function. It should be something irq_check_settings_disable_unlazy >> >> >> Drivers have irq_set_status_flags()/irq_clear_status_flags() ... >> >> Thanks, >> >> tglx >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > > > -- > Thanks, > with regards, > Suman Tripathi
On Wed, 6 Jan 2016, Suman Tripathi wrote: > On Wed, Jan 6, 2016 at 3:35 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > > Why would I? Those helpers are core internal and not usable in random > > drivers. > > > > i think the problem is the name of the function. It should be something > > irq_check_settings_disable_unlazy And why is that relevant to a driver? Again, those helpers are core internal and well understood. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 06, 2016 at 11:05:02AM +0100, Thomas Gleixner wrote: > On Wed, 6 Jan 2016, Vinod Koul wrote: > > On Wed, Jan 06, 2016 at 02:51:07PM +0530, Rameshwar Sahu wrote: > > > >> @@ -1610,6 +1611,7 @@ static int xgene_dma_request_irqs(struct xgene_dma *pdma) > > > >> /* Register DMA channel rx irq */ > > > >> for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++) { > > > >> chan = &pdma->chan[i]; > > > >> + irq_set_status_flags(chan->rx_irq, IRQ_DISABLE_UNLAZY); > > > > > > > > Why not use irq_settings_disable_unlazy(), at least read the reference you > > > > pointed out! > > > > > > irq_settings_disable_unlazy() is helper function to test > > > IRQ_DISABLE_UNLAZY flag is set or not, it's not for setting this flag. > > > FYI... > > > +static inline bool irq_settings_disable_unlazy(struct irq_desc *desc) > > > +{ > > > + return desc->status_use_accessors & _IRQ_DISABLE_UNLAZY; > > > +} > > > > Ah yes, I saw clear API and assumed there would be set. Then I think we > > should add a set helper as well as the usage is intended for drivers to > > set this flag > > > > Thomas, > > > > Any reason why you didn't add a set helper, only test and clear? > > Why would I? Those helpers are core internal and not usable in random drivers. > > Drivers have irq_set_status_flags()/irq_clear_status_flags() ... The effect of irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY) is same as providing a helper. If your recommendation is to use this by driver then I am find with the approach too Thanks
On Wed, Dec 23, 2015 at 06:28:15PM +0530, Rameshswar Prasad Sahu wrote: > From: Rameshwar Prasad Sahu <rsahu@apm.com> > > For interrupt controller that doesn't support irq_disable and hardware > with level interrupt, an extra interrupt can be pending. This patch fixes > the issue by setting IRQ_DISABLE_UNLAZY flag for the interrupt line. > > Reference: http://git.kernel.org/tip/e9849777d0e27cdd2902805be51da73e7c79578c > Applied now
diff --git a/drivers/dma/xgene-dma.c b/drivers/dma/xgene-dma.c index 9dfa2b0..6363e84 100644 --- a/drivers/dma/xgene-dma.c +++ b/drivers/dma/xgene-dma.c @@ -29,6 +29,7 @@ #include <linux/dmapool.h> #include <linux/interrupt.h> #include <linux/io.h> +#include <linux/irq.h> #include <linux/module.h> #include <linux/of_device.h> @@ -1610,6 +1611,7 @@ static int xgene_dma_request_irqs(struct xgene_dma *pdma) /* Register DMA channel rx irq */ for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++) { chan = &pdma->chan[i]; + irq_set_status_flags(chan->rx_irq, IRQ_DISABLE_UNLAZY); ret = devm_request_irq(chan->dev, chan->rx_irq, xgene_dma_chan_ring_isr, 0, chan->name, chan); @@ -1620,6 +1622,7 @@ static int xgene_dma_request_irqs(struct xgene_dma *pdma) for (j = 0; j < i; j++) { chan = &pdma->chan[i]; + irq_clear_status_flags(chan->rx_irq, IRQ_DISABLE_UNLAZY); devm_free_irq(chan->dev, chan->rx_irq, chan); } @@ -1640,6 +1643,7 @@ static void xgene_dma_free_irqs(struct xgene_dma *pdma) for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++) { chan = &pdma->chan[i]; + irq_clear_status_flags(chan->rx_irq, IRQ_DISABLE_UNLAZY); devm_free_irq(chan->dev, chan->rx_irq, chan); } }