Re: [patch v1 2/5] regulator: da9055 change irq state to default

Re: [patch v1 2/5] regulator: da9055 change irq state to default

[Ashish Jangam]

> -----Original Message-----
> 
> On Fri, Jan 18, 2013 at 10:52:45AM +0530, Ashish Jangam wrote:
> > This patch changes the irq state from high to the now default low
> > state.
> > 
> > This patch is dependent on the DA9055 MFD.
> 
> Why is this change required and why is there a dependency here?
> 
It has been decided to have nIRQ therefore the main mfd IRQ state needs
to be changed. And since mfd children follows mfd irq state therefore
the dependency; though in case of mfd this may not affect.  

Re: [patch v1 2/5] regulator: da9055 change irq state to default

[Mark Brown]

On Mon, Jan 21, 2013 at 01:34:18PM +0530, Ashish Jangam wrote:
> > On Fri, Jan 18, 2013 at 10:52:45AM +0530, Ashish Jangam wrote:

> > > This patch changes the irq state from high to the now default low
> > > state.

> > > This patch is dependent on the DA9055 MFD.

> > Why is this change required and why is there a dependency here?

> It has been decided to have nIRQ therefore the main mfd IRQ state needs

Won't this break all existing systems?

> to be changed. And since mfd children follows mfd irq state therefore
> the dependency; though in case of mfd this may not affect.  

What makes you say that there is a dependency here?  It's really not at
all obvious why a change to the primary IRQ signalling mechanism would
affect the internal interrupts of the device.

Re: [patch v1 2/5] regulator: da9055 change irq state to default

[Ashish Jangam]

> -----Original Message-----
> > > > This patch changes the irq state from high to the now default low
> > > > state.
> 
> > > > This patch is dependent on the DA9055 MFD.
> 
> > > Why is this change required and why is there a dependency here?
> 
> > It has been decided to have nIRQ therefore the main mfd IRQ state needs
> 
> Won't this break all existing systems?
Since all of the existing users are using nIRQ therefore it was decided
to have the default state as nIRQ.
> 
> > to be changed. And since mfd children follows mfd irq state therefore
> > the dependency; though in case of mfd this may not affect.  
> 
> What makes you say that there is a dependency here?  It's really not at
> all obvious why a change to the primary IRQ signalling mechanism would
> affect the internal interrupts of the device.
Yes, functionally this dependency should not matter.However, if
mfd/primary irq state is low and its components are high then it shall
not look good. Therefore the term dependency was used, probably I should
have written it in a different manner.

Re: [patch v1 2/5] regulator: da9055 change irq state to default

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 676 bytes --]

On Mon, Jan 21, 2013 at 02:05:12PM +0530, Ashish Jangam wrote:

> > What makes you say that there is a dependency here?  It's really not at
> > all obvious why a change to the primary IRQ signalling mechanism would
> > affect the internal interrupts of the device.

> Yes, functionally this dependency should not matter.However, if
> mfd/primary irq state is low and its components are high then it shall
> not look good. Therefore the term dependency was used, probably I should
> have written it in a different manner.

This logic doesn't follow terribly directly - looking at what you've
posted it in fact seems that the trigger type is irrelevant in the
function drivers?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [patch v1 2/5] regulator: da9055 change irq state to default

[Ashish Jangam]

On Thu, 2013-01-24 at 00:03 +0800, Mark Brown wrote:
> On Mon, Jan 21, 2013 at 02:05:12PM +0530, Ashish Jangam wrote:
> 
> > > What makes you say that there is a dependency here?  It's really not at
> > > all obvious why a change to the primary IRQ signalling mechanism would
> > > affect the internal interrupts of the device.
> 
> > Yes, functionally this dependency should not matter.However, if
> > mfd/primary irq state is low and its components are high then it shall
> > not look good. Therefore the term dependency was used, probably I should
> > have written it in a different manner.
> 
> This logic doesn't follow terribly directly - looking at what you've
> posted it in fact seems that the trigger type is irrelevant in the
> function drivers?
Though it is functionally irrelevant but in the code setting the primary
irq to low and functional driver to high may confuse the code reader.
However I leave this decision to you.

[patch v1 2/5] regulator: da9055 change irq state to default

[Ashish Jangam]

This patch changes the irq state from high to the now default low
state.

This patch is dependent on the DA9055 MFD.

This patch is tested on smdk6410 board.

Signed-off-by: Ashish Jangam <ashish.jangam@kpitcummins.com>
---
 drivers/regulator/da9055-regulator.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/regulator/da9055-regulator.c b/drivers/regulator/da9055-regulator.c
index 3022109..d4297c2 100644
--- a/drivers/regulator/da9055-regulator.c
+++ b/drivers/regulator/da9055-regulator.c
@@ -579,7 +579,7 @@ static int da9055_regulator_probe(struct platform_device *pdev)
 		irq = regmap_irq_get_virq(da9055->irq_data, irq);
 		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
 						da9055_ldo5_6_oc_irq,
-						IRQF_TRIGGER_HIGH |
+						IRQF_TRIGGER_LOW |
 						IRQF_ONESHOT |
 						IRQF_PROBE_SHARED,
 						pdev->name, regulator);
-- 
1.7.0.4

Re: [patch v1 2/5] regulator: da9055 change irq state to default

[Mark Brown]

On Fri, Jan 18, 2013 at 10:52:45AM +0530, Ashish Jangam wrote:
> This patch changes the irq state from high to the now default low
> state.
> 
> This patch is dependent on the DA9055 MFD.

Why is this change required and why is there a dependency here?