| Message ID | 1408479811-26088-1-git-send-email-bjorn.andersson@sonymobile.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 19, 2014 at 1:23 PM, Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote: > Introduce the irq_read_line() function to allow device drivers to read > the current logical state of an input when the hardware only exposes > this through status bits in the interrupt controller. > > The new function is backed by a new callback function in the irq_chip - > irq_read_line() - that can be implemented by irq_chips that owns such > status bits. > > Based on rfc patch from April 2011 by Abhijeet. > > Cc: Abhijeet Dharmapurikar <adharmap@codeaurora.org> > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> ping? -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 4 Sep 2014, Bjorn Andersson wrote: > On Tue, Aug 19, 2014 at 1:23 PM, Bjorn Andersson > <bjorn.andersson@sonymobile.com> wrote: > > Introduce the irq_read_line() function to allow device drivers to read > > the current logical state of an input when the hardware only exposes > > this through status bits in the interrupt controller. > > > > The new function is backed by a new callback function in the irq_chip - > > irq_read_line() - that can be implemented by irq_chips that owns such > > status bits. > > > > Based on rfc patch from April 2011 by Abhijeet. > > > > Cc: Abhijeet Dharmapurikar <adharmap@codeaurora.org> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> > > ping? Sorry, slipped through the cracks. I was talking about this to Marc last week and he needs it for yet another reason. He had some thoughts about the state representation, so I wait for him to comment. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 21 October 2014 11:22:40 Thomas Gleixner wrote: > On Thu, 4 Sep 2014, Bjorn Andersson wrote: > > > On Tue, Aug 19, 2014 at 1:23 PM, Bjorn Andersson > > <bjorn.andersson@sonymobile.com> wrote: > > > Introduce the irq_read_line() function to allow device drivers to read > > > the current logical state of an input when the hardware only exposes > > > this through status bits in the interrupt controller. > > > > > > The new function is backed by a new callback function in the irq_chip - > > > irq_read_line() - that can be implemented by irq_chips that owns such > > > status bits. > > > > > > Based on rfc patch from April 2011 by Abhijeet. > > > > > > Cc: Abhijeet Dharmapurikar <adharmap@codeaurora.org> > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> > > > > ping? > > Sorry, slipped through the cracks. I was talking about this to Marc > last week and he needs it for yet another reason. He had some thoughts > about the state representation, so I wait for him to comment. I looked up my older email as well and found which gpio driver had the requirement to get the irq status from the upstream controller. It was "gpio: Add APM X-Gene standby GPIO controller driver". I'm adding the APM developers on Cc here so they can participate in the discussion. As a brief summary: APM have a GPIO driver that needs to read the status from the ARM GIC in order to implement gpio_get_value. The KVM hypervisor needs multiple bits from the GIC in order to implement suspend/resume, and Qualcomm MSM (as Bjorn wrote above) also needs to expose the status of the IRQ line like APM does. These should all use the same interface for the interrupt controller, but the interface still needs to be added. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi all, On 21/10/14 10:22, Thomas Gleixner wrote: > On Thu, 4 Sep 2014, Bjorn Andersson wrote: > >> On Tue, Aug 19, 2014 at 1:23 PM, Bjorn Andersson >> <bjorn.andersson@sonymobile.com> wrote: >>> Introduce the irq_read_line() function to allow device drivers to read >>> the current logical state of an input when the hardware only exposes >>> this through status bits in the interrupt controller. >>> >>> The new function is backed by a new callback function in the irq_chip - >>> irq_read_line() - that can be implemented by irq_chips that owns such >>> status bits. >>> >>> Based on rfc patch from April 2011 by Abhijeet. >>> >>> Cc: Abhijeet Dharmapurikar <adharmap@codeaurora.org> >>> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> >> >> ping? > > Sorry, slipped through the cracks. I was talking about this to Marc > last week and he needs it for yet another reason. He had some thoughts > about the state representation, so I wait for him to comment. Thanks for putting me in the loop. For the record, here's the RFC I posted back in June: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266328.html and the patch implementing a similar concept: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266331.html Basic idea is that you can read (and possibly write back) various low-level attributes (pending, masked, active) that an interrupt controller may implement. Given your use case, we should loose the checks on the interrupt being forwarded, as this makes little sense outside of virtualization. I'm planning to respin the series this week, as I have a number of changes (there is hardly any need for the various states to be reported atomically, for example), and a number of bugs have been found. Thanks, M.
On Tue, Oct 21, 2014 at 2:30 AM, Arnd Bergmann <arnd@arndb.de> wrote: > On Tuesday 21 October 2014 11:22:40 Thomas Gleixner wrote: >> On Thu, 4 Sep 2014, Bjorn Andersson wrote: >> >> > On Tue, Aug 19, 2014 at 1:23 PM, Bjorn Andersson >> > <bjorn.andersson@sonymobile.com> wrote: >> > > Introduce the irq_read_line() function to allow device drivers to read >> > > the current logical state of an input when the hardware only exposes >> > > this through status bits in the interrupt controller. >> > > >> > > The new function is backed by a new callback function in the irq_chip - >> > > irq_read_line() - that can be implemented by irq_chips that owns such >> > > status bits. >> > > >> > > Based on rfc patch from April 2011 by Abhijeet. >> > > >> > > Cc: Abhijeet Dharmapurikar <adharmap@codeaurora.org> >> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> >> > >> > ping? >> >> Sorry, slipped through the cracks. I was talking about this to Marc >> last week and he needs it for yet another reason. He had some thoughts >> about the state representation, so I wait for him to comment. > > I looked up my older email as well and found which gpio driver had the > requirement to get the irq status from the upstream controller. It was > "gpio: Add APM X-Gene standby GPIO controller driver". I'm adding the > APM developers on Cc here so they can participate in the discussion. > > As a brief summary: APM have a GPIO driver that needs to read the status > from the ARM GIC in order to implement gpio_get_value. Thanks, we will rebase our standby gpio patch on top of this change. The KVM hypervisor > needs multiple bits from the GIC in order to implement suspend/resume, > and Qualcomm MSM (as Bjorn wrote above) also needs to expose the status > of the IRQ line like APM does. > > These should all use the same interface for the interrupt controller, > but the interface still needs to be added. > > Arnd > CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, > is for the sole use of the intended recipient(s) and contains information > that is confidential and proprietary to Applied Micro Circuits Corporation or its subsidiaries. > It is to be used solely for the purpose of furthering the parties' business relationship. > All unauthorized review, use, disclosure or distribution is prohibited. > If you are not the intended recipient, please contact the sender by reply e-mail > and destroy all copies of the original message. > -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 21, 2014 at 2:34 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > Hi all, > > On 21/10/14 10:22, Thomas Gleixner wrote: >> On Thu, 4 Sep 2014, Bjorn Andersson wrote: >> >>> On Tue, Aug 19, 2014 at 1:23 PM, Bjorn Andersson >>> <bjorn.andersson@sonymobile.com> wrote: >>>> Introduce the irq_read_line() function to allow device drivers to read >>>> the current logical state of an input when the hardware only exposes >>>> this through status bits in the interrupt controller. >>>> >>>> The new function is backed by a new callback function in the irq_chip - >>>> irq_read_line() - that can be implemented by irq_chips that owns such >>>> status bits. >>>> >>>> Based on rfc patch from April 2011 by Abhijeet. >>>> >>>> Cc: Abhijeet Dharmapurikar <adharmap@codeaurora.org> >>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> >>> >>> ping? >> >> Sorry, slipped through the cracks. I was talking about this to Marc >> last week and he needs it for yet another reason. He had some thoughts >> about the state representation, so I wait for him to comment. > > Thanks for putting me in the loop. For the record, here's the RFC I > posted back in June: > Thanks for having a similar problem as me :) > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266328.html > > and the patch implementing a similar concept: > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266331.html > I would prefer to see that you had explicit functions for the various things that you would like to set and get. It would in my eyes give cleaner client code, at the cost of a few extra functions. > Basic idea is that you can read (and possibly write back) various > low-level attributes (pending, masked, active) that an interrupt > controller may implement. Given your use case, we should loose the > checks on the interrupt being forwarded, as this makes little sense > outside of virtualization. > Relaxing the forwarding requirement seems to solve my use cases. May I ask why your accessors would only be made available for forwarded IRQs - at a framework level? Stephen Boyd talked about the need to be able to mask/unmask interrupts from client code in the Qualcomm platform as well - most likely to block wakeup sources(?) > I'm planning to respin the series this week, as I have a number of > changes (there is hardly any need for the various states to be reported > atomically, for example), and a number of bugs have been found. > Sounds good, for clarity I would like them to be explicit separate functions. But as long as it's not limited to forwarded IRQs we should be able to use it. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bjorn, On 24/10/14 18:31, Bjorn Andersson wrote: > On Tue, Oct 21, 2014 at 2:34 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> Hi all, >> >> On 21/10/14 10:22, Thomas Gleixner wrote: >>> On Thu, 4 Sep 2014, Bjorn Andersson wrote: >>> >>>> On Tue, Aug 19, 2014 at 1:23 PM, Bjorn Andersson >>>> <bjorn.andersson@sonymobile.com> wrote: >>>>> Introduce the irq_read_line() function to allow device drivers to read >>>>> the current logical state of an input when the hardware only exposes >>>>> this through status bits in the interrupt controller. >>>>> >>>>> The new function is backed by a new callback function in the irq_chip - >>>>> irq_read_line() - that can be implemented by irq_chips that owns such >>>>> status bits. >>>>> >>>>> Based on rfc patch from April 2011 by Abhijeet. >>>>> >>>>> Cc: Abhijeet Dharmapurikar <adharmap@codeaurora.org> >>>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> >>>> >>>> ping? >>> >>> Sorry, slipped through the cracks. I was talking about this to Marc >>> last week and he needs it for yet another reason. He had some thoughts >>> about the state representation, so I wait for him to comment. >> >> Thanks for putting me in the loop. For the record, here's the RFC I >> posted back in June: >> > > Thanks for having a similar problem as me :) > >> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266328.html >> >> and the patch implementing a similar concept: >> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266331.html >> > > I would prefer to see that you had explicit functions for the various things > that you would like to set and get. It would in my eyes give cleaner client > code, at the cost of a few extra functions. I don't really like the idea of having a gazillion of accessors for each individual property. I'd rather have an extensible API. >> Basic idea is that you can read (and possibly write back) various >> low-level attributes (pending, masked, active) that an interrupt >> controller may implement. Given your use case, we should loose the >> checks on the interrupt being forwarded, as this makes little sense >> outside of virtualization. >> > > Relaxing the forwarding requirement seems to solve my use cases. May I ask why > your accessors would only be made available for forwarded IRQs - at a framework > level? This was never intended for wider use, and restricting it to forwarded interrupts was a concious design decision, as I find the idea of a device driver messing with the internal state of the interrupt controller positively repulsive... But hey, the more the merrier. > Stephen Boyd talked about the need to be able to mask/unmask interrupts from > client code in the Qualcomm platform as well - most likely to block wakeup > sources(?) What's wrong with irq_disable? >> I'm planning to respin the series this week, as I have a number of >> changes (there is hardly any need for the various states to be reported >> atomically, for example), and a number of bugs have been found. >> > > Sounds good, for clarity I would like them to be explicit separate functions. > But as long as it's not limited to forwarded IRQs we should be able to use it. I just pushed out a branch: git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git irq/irqchip_state Please let me know if that's useful for you. Thanks, M.
On 10/24/2014 10:59 AM, Marc Zyngier wrote: > Hi Bjorn, > > On 24/10/14 18:31, Bjorn Andersson wrote: >> >> Stephen Boyd talked about the need to be able to mask/unmask interrupts from >> client code in the Qualcomm platform as well - most likely to block wakeup >> sources(?) > What's wrong with irq_disable? The problem is irq_disable() is lazy and doesn't actually disable the interrupt. This is the scenario. During idle we want to communicate with another processor in the SoC and tell it to turn off something after we go to idle. The communication mechanism uses some shared memory and an interrupt to trigger the other processor to go look at what we told it to do. When the other processor is done handling the message it will send us an ack interrupt. We want to temporarily ignore that ack interrupt because 1) we're going to execute a wfi to trigger a deep idle and any pending interrupt will abort that instruction and 2) if the interrupt comes after we execute the wfi it will wake us up out of deep idle unnecessarily and waste power. There isn't anything in the communication protocol that says we don't want the interrupt to be sent and it would probably make things more complicated anyway. The solution is to mask the interrupt before we send the message, and then execute the wfi. Once we wake up, we unmask the interrupt and then handle the ack.
On Fri, Oct 24, 2014 at 10:59 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > Hi Bjorn, > > On 24/10/14 18:31, Bjorn Andersson wrote: >> On Tue, Oct 21, 2014 at 2:34 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: [..] >> I would prefer to see that you had explicit functions for the various things >> that you would like to set and get. It would in my eyes give cleaner client >> code, at the cost of a few extra functions. > > I don't really like the idea of having a gazillion of accessors for each > individual property. I'd rather have an extensible API. > Okay, either way will work. Feel free to proceed with your approach. >>> Basic idea is that you can read (and possibly write back) various >>> low-level attributes (pending, masked, active) that an interrupt >>> controller may implement. Given your use case, we should loose the >>> checks on the interrupt being forwarded, as this makes little sense >>> outside of virtualization. >>> >> >> Relaxing the forwarding requirement seems to solve my use cases. May I ask why >> your accessors would only be made available for forwarded IRQs - at a framework >> level? > > This was never intended for wider use, and restricting it to forwarded > interrupts was a concious design decision, as I find the idea of a > device driver messing with the internal state of the interrupt > controller positively repulsive... But hey, the more the merrier. > Makes sense and now that it comes without the forwarding part it looks reasonable. [..] >>> I'm planning to respin the series this week, as I have a number of >>> changes (there is hardly any need for the various states to be reported >>> atomically, for example), and a number of bugs have been found. >>> >> >> Sounds good, for clarity I would like them to be explicit separate functions. >> But as long as it's not limited to forwarded IRQs we should be able to use it. > > I just pushed out a branch: > git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git > irq/irqchip_state > > Please let me know if that's useful for you. > I think that my irq_read_line() would be equivalent of irq_get_irqchip_state(IRQCHIP_STATE_PENDING), i.e. the state of the interrupt ignoring masking. And the rest would be EINVAL. So I think this would work out just fine for us! Is ACTIVE the line level with the mask considered and setting ACTIVE, would that be the same as acking the interrupt? What's the expected behaviour of setting PENDING? I would appreciate if you commented the state enum, to make it obvious what pending and active means. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 24 Oct 2014, Stephen Boyd wrote: > On 10/24/2014 10:59 AM, Marc Zyngier wrote: > > Hi Bjorn, > > > > On 24/10/14 18:31, Bjorn Andersson wrote: > >> > >> Stephen Boyd talked about the need to be able to mask/unmask interrupts from > >> client code in the Qualcomm platform as well - most likely to block wakeup > >> sources(?) > > What's wrong with irq_disable? > > The problem is irq_disable() is lazy and doesn't actually disable the > interrupt. Nothing prevents you from adding your own irq_disable() callback for those interrupts. Just the default is lazy. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Oct 25 2014 at 12:12:55 am BST, Bjorn Andersson <bjorn@kryo.se> wrote: > On Fri, Oct 24, 2014 at 10:59 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> I just pushed out a branch: >> git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git >> irq/irqchip_state >> >> Please let me know if that's useful for you. >> > > I think that my irq_read_line() would be equivalent of > irq_get_irqchip_state(IRQCHIP_STATE_PENDING), i.e. the state of the > interrupt ignoring masking. And the rest would be EINVAL. > So I think this would work out just fine for us! Excellent. > Is ACTIVE the line level with the mask considered and setting ACTIVE, > would that be the same as acking the interrupt? ACTIVE describes the state that exists once the interrupt has been ACKed, and before it has been EOIed. It indicates an interrupt "in progress". It probably doesn't make any sense for most users, but is extremely useful with KVM: when a device is shared between VMs (most obvious one is a simple timer), the interrupt state is part of the whole context, and must be save/restored as well. > What's the expected behaviour of setting PENDING? Its expected behaviour is to inject an interrupt, just as if the device signaled an interrupt. Probably not easy to implement on anything but the ARM GICs. > I would appreciate if you commented the state enum, to make it obvious > what pending and active means. Done. I'll probably post this today. Thanks, M.
On 10/25/2014 01:34 AM, Thomas Gleixner wrote: > On Fri, 24 Oct 2014, Stephen Boyd wrote: > >> On 10/24/2014 10:59 AM, Marc Zyngier wrote: >>> Hi Bjorn, >>> >>> On 24/10/14 18:31, Bjorn Andersson wrote: >>>> Stephen Boyd talked about the need to be able to mask/unmask interrupts from >>>> client code in the Qualcomm platform as well - most likely to block wakeup >>>> sources(?) >>> What's wrong with irq_disable? >> The problem is irq_disable() is lazy and doesn't actually disable the >> interrupt. > Nothing prevents you from adding your own irq_disable() callback for > those interrupts. Just the default is lazy. > > Ok, if we did that it would be global for the entire arm gic right? I see: void irq_disable(struct irq_desc *desc) { irq_state_set_disabled(desc); if (desc->irq_data.chip->irq_disable) { desc->irq_data.chip->irq_disable(&desc->irq_data); irq_state_set_masked(desc); } } so we would need to add some return value to irq_disable() so that we could tell if this particular interrupt needs to be disabled or not or we would need to set a different chip for this particular interrupt with the irq_disable callback set? Plus any scheme would need to be SoC specific somehow and be setup early when the gic is probed. Maybe we can encode this information in the DT specifier somehow to indicate that we want disable_irq() to actually mask the irq? This is all under the assumption that we can't just force every gic interrupt to mask on disable.
On Sat, Oct 25, 2014 at 2:22 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > On Sat, Oct 25 2014 at 12:12:55 am BST, Bjorn Andersson <bjorn@kryo.se> wrote: >> On Fri, Oct 24, 2014 at 10:59 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >>> I just pushed out a branch: >>> git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git >>> irq/irqchip_state >>> >>> Please let me know if that's useful for you. >>> >> >> I think that my irq_read_line() would be equivalent of >> irq_get_irqchip_state(IRQCHIP_STATE_PENDING), i.e. the state of the >> interrupt ignoring masking. And the rest would be EINVAL. >> So I think this would work out just fine for us! > > Excellent. > Hi Marc, We gave this some more thought and read up on what "pending" and "active" means according to the ARM GIC - PENDING is not what we want :( In the Qualcomm pmic we have two interrupt status registers "interrupt" and "real-time". I think the "interrupt" status register would be the one related to your defined constants. However what we need to access in our use cases are the "real-time" status register, which basically is a representation of the input to the interrupt logic. As far as I can see the GIC does not offer anything like that, but I hope we could add another constant to your enum list and utilise your api for this. I'm not entirely sure what we should call it though, IRQCHIP_STATE_LEVEL seems somewhat conflicting with level trigger and the Qualcomm name IRQCHIP_STATE_REALTIME isn't very self describing. Maybe IRQCHIP_STATE_LINE_LEVEL? Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 28/10/14 15:41, Bjorn Andersson wrote: > On Sat, Oct 25, 2014 at 2:22 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> On Sat, Oct 25 2014 at 12:12:55 am BST, Bjorn Andersson <bjorn@kryo.se> wrote: >>> On Fri, Oct 24, 2014 at 10:59 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >>>> I just pushed out a branch: >>>> git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git >>>> irq/irqchip_state >>>> >>>> Please let me know if that's useful for you. >>>> >>> >>> I think that my irq_read_line() would be equivalent of >>> irq_get_irqchip_state(IRQCHIP_STATE_PENDING), i.e. the state of the >>> interrupt ignoring masking. And the rest would be EINVAL. >>> So I think this would work out just fine for us! >> >> Excellent. >> > > Hi Marc, > > We gave this some more thought and read up on what "pending" and > "active" means according to the ARM GIC - PENDING is not what we want > :( > > In the Qualcomm pmic we have two interrupt status registers > "interrupt" and "real-time". I think the "interrupt" status register > would be the one related to your defined constants. However what we > need to access in our use cases are the "real-time" status register, > which basically is a representation of the input to the interrupt > logic. Fancy. This really look like a i2c GPIO expander (not my best memories...). > As far as I can see the GIC does not offer anything like that, but I > hope we could add another constant to your enum list and utilise your > api for this. > > I'm not entirely sure what we should call it though, > IRQCHIP_STATE_LEVEL seems somewhat conflicting with level trigger and > the Qualcomm name IRQCHIP_STATE_REALTIME isn't very self describing. > Maybe IRQCHIP_STATE_LINE_LEVEL? Sure, that should be descriptive enough. I really we don't get too many of these though... Do you want me to wrap this into the next version? Thanks, M.
On Tue, Oct 28, 2014 at 9:05 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > On 28/10/14 15:41, Bjorn Andersson wrote: >> On Sat, Oct 25, 2014 at 2:22 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: [..] >> In the Qualcomm pmic we have two interrupt status registers >> "interrupt" and "real-time". I think the "interrupt" status register >> would be the one related to your defined constants. However what we >> need to access in our use cases are the "real-time" status register, >> which basically is a representation of the input to the interrupt >> logic. > > Fancy. This really look like a i2c GPIO expander (not my best memories...). > It really is a kitchen sink, with among other things a set of gpios. Due to limitations in address space many of the input bits are handled through banked reads of the interrupt status bits. So that's where we have access to gpio input, but also things like battery availability, usb connected, charging status and so on. >> As far as I can see the GIC does not offer anything like that, but I >> hope we could add another constant to your enum list and utilise your >> api for this. >> >> I'm not entirely sure what we should call it though, >> IRQCHIP_STATE_LEVEL seems somewhat conflicting with level trigger and >> the Qualcomm name IRQCHIP_STATE_REALTIME isn't very self describing. >> Maybe IRQCHIP_STATE_LINE_LEVEL? > > Sure, that should be descriptive enough. I really we don't get too many > of these though... > > Do you want me to wrap this into the next version? > Yes please. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h index 698ad05..8f3af5d 100644 --- a/include/linux/interrupt.h +++ b/include/linux/interrupt.h @@ -361,6 +361,7 @@ static inline int disable_irq_wake(unsigned int irq) return irq_set_irq_wake(irq, 0); } +extern int irq_read_line(unsigned int irq); #ifdef CONFIG_IRQ_FORCED_THREADING extern bool force_irqthreads; diff --git a/include/linux/irq.h b/include/linux/irq.h index 0d998d8..f656590 100644 --- a/include/linux/irq.h +++ b/include/linux/irq.h @@ -294,6 +294,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d) * @irq_retrigger: resend an IRQ to the CPU * @irq_set_type: set the flow type (IRQ_TYPE_LEVEL/etc.) of an IRQ * @irq_set_wake: enable/disable power-management wake-on of an IRQ + * @irq_read_line: read the logical state of an irq line * @irq_bus_lock: function to lock access to slow bus (i2c) chips * @irq_bus_sync_unlock:function to sync and unlock slow bus (i2c) chips * @irq_cpu_online: configure an interrupt source for a secondary CPU @@ -326,6 +327,7 @@ struct irq_chip { int (*irq_retrigger)(struct irq_data *data); int (*irq_set_type)(struct irq_data *data, unsigned int flow_type); int (*irq_set_wake)(struct irq_data *data, unsigned int on); + int (*irq_read_line)(struct irq_data *data); void (*irq_bus_lock)(struct irq_data *data); void (*irq_bus_sync_unlock)(struct irq_data *data); diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c index 3dc6a61..33bf9c7 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -565,6 +565,31 @@ int irq_set_irq_wake(unsigned int irq, unsigned int on) } EXPORT_SYMBOL(irq_set_irq_wake); +/** + * irq_read_line - read the logical state of an irq line + * @irq: interrupt to read + * + * This function may be called from IRQ context only when + * desc->chip->bus_lock and desc->chip->bus_sync_unlock are NULL ! + */ +int irq_read_line(unsigned int irq) +{ + struct irq_desc *desc; + unsigned long flags; + int ret = -ENOSYS; + + desc = irq_get_desc_buslock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL); + if (!desc) + return -EINVAL; + + if (desc->irq_data.chip->irq_read_line) + ret = desc->irq_data.chip->irq_read_line(&desc->irq_data); + + irq_put_desc_busunlock(desc, flags); + return ret; +} +EXPORT_SYMBOL_GPL(irq_read_line); + /* * Internal function that tells the architecture code whether a * particular irq has been exclusively allocated or is available
Introduce the irq_read_line() function to allow device drivers to read the current logical state of an input when the hardware only exposes this through status bits in the interrupt controller. The new function is backed by a new callback function in the irq_chip - irq_read_line() - that can be implemented by irq_chips that owns such status bits. Based on rfc patch from April 2011 by Abhijeet. Cc: Abhijeet Dharmapurikar <adharmap@codeaurora.org> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> --- Due to address size limitations in the SSBI protocol used to communicate with various Qualcomm PMICs most accesses are banked. Furthermore the input/status bits for the adc, battery monitor, charger, gpio, mpp and usb blocks are all banked as interrupt status bits. Therefor to be able to implement any of those drivers we need to make a call into the pm8xxx irq_chip code to acquire the state of those bits - as my first hack [1] did. The patch is based on Abhijeet's RFC found here [2], but my reasons for having this accessor function is different. [1] https://lkml.org/lkml/2014/7/7/892 [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-April/048277.html include/linux/interrupt.h | 1 + include/linux/irq.h | 2 ++ kernel/irq/manage.c | 25 +++++++++++++++++++++++++ 3 files changed, 28 insertions(+)