Thread (22 messages) 22 messages, 5 authors, 2014-07-23

[PATCH 1/3] mfd: pm8921: Expose pm8xxx_read_irq_status

From: Bjorn Andersson <hidden>
Date: 2014-07-08 23:43:53
Also in: linux-arm-msm, linux-devicetree, lkml

On Tue, Jul 8, 2014 at 4:20 PM, Stephen Boyd [off-list ref] wrote:
On 07/07/14 18:26, Bjorn Andersson wrote:
quoted
@@ -65,6 +66,41 @@ struct pm_irq_chip {
      u8                      config[0];
 };

+int pm8xxx_read_irq_status(int irq)
+{
+     struct irq_data *d = irq_get_irq_data(irq);
+     struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
+     unsigned int pmirq = irqd_to_hwirq(d);
+     unsigned int bits;
+     int irq_bit;
+     u8 block;
+     int rc;
+
+     if (!chip) {
+             pr_err("Failed to resolve pm_irq_chip\n");
+             return -EINVAL;
+     }
Is this actually necessary? Presumably the driver wouldn't have even
probed unless there was a pmic to begin with.
I had a bug in the calling driver, passing the wrong irq down to this
function. But now that I think more about it I should probably check
for "d" being non-NULL. On the other hand, that will still just catch
a minor set of bugs as both of those will in most cases produce
"valid" pointers.

Maybe it's okay to just trust the caller to pass a valid irq? Or do
you have any other suggestion of sanity checking the input value?
Preferably without also passing the pm_irq_chip pointer.

[...]
Sad, the header file came back. I guess there isn't a way to put the
pinctrl driver inside the core mfd driver? Then we wouldn't need to
expose an "irq read line" function.
I continued my search and this needs to be accessed by gpio, mpp, adc,
charger, bms and usb(?). So we have to expose it in some form.
Actually Abhijeet proposed such an API in 2011 but it didn't go
anywhere[1]. If we had that API we should be able to call
read_irq_line() from the pinctrl driver whenever we want to get the
state of the gpio, plus the API is generic. We're going to need that API
anyway for things like USB insertion detection so it might make sense to
add it sooner rather than later.

[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-April/048319.html
From what I can see of this thread it was exposed as a way for drivers
to be able to query if an interrupt handler was called on raising or
falling edge. And based on the locking limitations of the
implementation we couldn't have used it anyways.

Our use case is different in that we're at any point in time
interested in reading out the status of the irq line, as the only way
of getting that status.

It might be a long shot, but let's spin a patch for our purpose and
run it by Tomas again.

Regards,
Bjorn
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help