[PATCH] DS1WM: decouple host IRQ and INTR active state settings.

[PATCH] DS1WM: decouple host IRQ and INTR active state settings.

[Philipp Zabel]

The DS1WM driver incorrectly infers the IAS bit (1-wire interrupt active
high) from IRQ settings. There are devices that have IAS=0 but still need
the IRQ to trigger on a rising edge. With this patch, machines with DS1WM
that need IAS=1 have to set .active_high=1 in the ds1wm_platform_data.

Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
---
 drivers/w1/masters/ds1wm.c |    9 +++++----
 include/linux/ds1wm.h      |    1 +
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/w1/masters/ds1wm.c b/drivers/w1/masters/ds1wm.c
index 5747997..688e435 100644
--- a/drivers/w1/masters/ds1wm.c
+++ b/drivers/w1/masters/ds1wm.c
@@ -361,11 +361,12 @@ static int ds1wm_probe(struct platform_device *pdev)
 		goto err1;
 	}
 	ds1wm_data->irq = res->start;
-	ds1wm_data->active_high = (res->flags & IORESOURCE_IRQ_HIGHEDGE) ?
-		1 : 0;
+	ds1wm_data->active_high = plat->active_high;
 
-	set_irq_type(ds1wm_data->irq, ds1wm_data->active_high ?
-			IRQ_TYPE_EDGE_RISING : IRQ_TYPE_EDGE_FALLING);
+	if (res->flags & IORESOURCE_IRQ_HIGHEDGE)
+		set_irq_type(ds1wm_data->irq, IRQ_TYPE_EDGE_RISING);
+	if (res->flags & IORESOURCE_IRQ_LOWEDGE)
+		set_irq_type(ds1wm_data->irq, IRQ_TYPE_EDGE_FALLING);
 
 	ret = request_irq(ds1wm_data->irq, ds1wm_isr, IRQF_DISABLED,
 			  "ds1wm", ds1wm_data);
diff --git a/include/linux/ds1wm.h b/include/linux/ds1wm.h
index 31f6e3c..d3c65e4 100644
--- a/include/linux/ds1wm.h
+++ b/include/linux/ds1wm.h
@@ -6,6 +6,7 @@ struct ds1wm_platform_data {
 			     * e.g. on h5xxx and h2200 this is 2
 			     * (registers aligned to 4-byte boundaries),
 			     * while on hx4700 this is 1 */
+	int active_high;
 	void (*enable)(struct platform_device *pdev);
 	void (*disable)(struct platform_device *pdev);
 };
-- 
1.5.3.7

Re: [PATCH] DS1WM: decouple host IRQ and INTR active state settings.

[Evgeniy Polyakov]

Hi.

On Sun, Jan 06, 2008 at 02:46:14PM +0100, Philipp Zabel (philipp.zabel@gmail.com) wrote:
> The DS1WM driver incorrectly infers the IAS bit (1-wire interrupt active
> high) from IRQ settings. There are devices that have IAS=0 but still need
> the IRQ to trigger on a rising edge. With this patch, machines with DS1WM
> that need IAS=1 have to set .active_high=1 in the ds1wm_platform_data.
> 
> Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>

I have no problem with this patch, thank you.

Andrew, please apply.

Signed-off-by: Evgeniy Polyakov <johnpol@2ka.mipt.ru>


-- 
	Evgeniy Polyakov

Re: [PATCH] DS1WM: decouple host IRQ and INTR active state settings.

[Matt Reimer]

On Sun, 2008-01-06 at 14:46 +0100, Philipp Zabel wrote:
> The DS1WM driver incorrectly infers the IAS bit (1-wire interrupt active
> high) from IRQ settings. There are devices that have IAS=0 but still need
> the IRQ to trigger on a rising edge. With this patch, machines with DS1WM
> that need IAS=1 have to set .active_high=1 in the ds1wm_platform_data.
> 
> Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>

Looks good to me.

Signed-off-by: Matt Reimer <mreimer@vpop.net>

Matt

Re: [PATCH] DS1WM: decouple host IRQ and INTR active state settings.

[Andrew Morton]

On Sun, 06 Jan 2008 14:46:14 +0100
Philipp Zabel <philipp.zabel@gmail.com> wrote:

> The DS1WM driver incorrectly infers the IAS bit (1-wire interrupt active
> high) from IRQ settings. There are devices that have IAS=0 but still need
> the IRQ to trigger on a rising edge. With this patch, machines with DS1WM
> that need IAS=1 have to set .active_high=1 in the ds1wm_platform_data.
> 
> Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
> ---
>  drivers/w1/masters/ds1wm.c |    9 +++++----
>  include/linux/ds1wm.h      |    1 +
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/w1/masters/ds1wm.c b/drivers/w1/masters/ds1wm.c
> index 5747997..688e435 100644
> --- a/drivers/w1/masters/ds1wm.c
> +++ b/drivers/w1/masters/ds1wm.c
> @@ -361,11 +361,12 @@ static int ds1wm_probe(struct platform_device *pdev)
>  		goto err1;
>  	}
>  	ds1wm_data->irq = res->start;
> -	ds1wm_data->active_high = (res->flags & IORESOURCE_IRQ_HIGHEDGE) ?
> -		1 : 0;
> +	ds1wm_data->active_high = plat->active_high;
>  
> -	set_irq_type(ds1wm_data->irq, ds1wm_data->active_high ?
> -			IRQ_TYPE_EDGE_RISING : IRQ_TYPE_EDGE_FALLING);
> +	if (res->flags & IORESOURCE_IRQ_HIGHEDGE)
> +		set_irq_type(ds1wm_data->irq, IRQ_TYPE_EDGE_RISING);
> +	if (res->flags & IORESOURCE_IRQ_LOWEDGE)
> +		set_irq_type(ds1wm_data->irq, IRQ_TYPE_EDGE_FALLING);
>  
>  	ret = request_irq(ds1wm_data->irq, ds1wm_isr, IRQF_DISABLED,
>  			  "ds1wm", ds1wm_data);
> diff --git a/include/linux/ds1wm.h b/include/linux/ds1wm.h
> index 31f6e3c..d3c65e4 100644
> --- a/include/linux/ds1wm.h
> +++ b/include/linux/ds1wm.h
> @@ -6,6 +6,7 @@ struct ds1wm_platform_data {
>  			     * e.g. on h5xxx and h2200 this is 2
>  			     * (registers aligned to 4-byte boundaries),
>  			     * while on hx4700 this is 1 */
> +	int active_high;
>  	void (*enable)(struct platform_device *pdev);
>  	void (*disable)(struct platform_device *pdev);
>  };

But no drivers are converted to set ds1wm_platform_data.active_high.  Won't
IORESOURCE_IRQ_HIGHEDGE devices be broken by this change?

Re: [PATCH] DS1WM: decouple host IRQ and INTR active state settings.

[Matt Reimer]

On Mon, 2008-01-07 at 15:10 -0800, Andrew Morton wrote:
> On Sun, 06 Jan 2008 14:46:14 +0100
> Philipp Zabel <philipp.zabel@gmail.com> wrote:
> 
> > The DS1WM driver incorrectly infers the IAS bit (1-wire interrupt active
> > high) from IRQ settings. There are devices that have IAS=0 but still need
> > the IRQ to trigger on a rising edge. With this patch, machines with DS1WM
> > that need IAS=1 have to set .active_high=1 in the ds1wm_platform_data.

> But no drivers are converted to set ds1wm_platform_data.active_high.  Won't
> IORESOURCE_IRQ_HIGHEDGE devices be broken by this change?

Good point; I think you're right. I'd guess the other platforms that use
this driver are in the handhelds.org tree, but I've been out of the loop
a while. Philipp, is this the case?

Matt

Re: [PATCH] DS1WM: decouple host IRQ and INTR active state settings.

[pHilipp Zabel]

On Jan 8, 2008 1:13 AM, Matt Reimer <mreimer@vpop.net> wrote:
> On Mon, 2008-01-07 at 15:10 -0800, Andrew Morton wrote:
> > On Sun, 06 Jan 2008 14:46:14 +0100
> > Philipp Zabel <philipp.zabel@gmail.com> wrote:
> >
> > > The DS1WM driver incorrectly infers the IAS bit (1-wire interrupt active
> > > high) from IRQ settings. There are devices that have IAS=0 but still need
> > > the IRQ to trigger on a rising edge. With this patch, machines with DS1WM
> > > that need IAS=1 have to set .active_high=1 in the ds1wm_platform_data.
>
> > But no drivers are converted to set ds1wm_platform_data.active_high.  Won't
> > IORESOURCE_IRQ_HIGHEDGE devices be broken by this change?
>
> Good point; I think you're right. I'd guess the other platforms that use
> this driver are in the handhelds.org tree, but I've been out of the loop
> a while. Philipp, is this the case?

Yes, I think so. I am only aware of four chips that include a DS1WM:
HTC's ASIC3, PASIC2 and PASIC3 and Samsung SAMCOP.
All of those drivers have yet to be submitted.

I will also apply this patch to hh.org CVS and fix up the devices that are
affected by this change (aximx30, blueangel, magician, h1900, h4000,
h5400, himalaya, hx4700, sable, universal).
But none of those set IORESOURCE_IRQ_HIGHEDGE (most are just
missing the IORESOURCE_IRQ_LOWEDGE flag). I am not sure about
the status of rx3000 or other devices that might live in other trees.

I'm currently cleaning up the PASIC2/3 driver. After that I'll try to help
cleaning up ASIC3 and finally getting it ready for submission.
A whole load of devices in the hh.org tree depend on it.

regards
Philipp

Re: [PATCH] DS1WM: decouple host IRQ and INTR active state settings.

[Andrew Morton]

On Tue, 8 Jan 2008 09:21:55 +0100 "pHilipp Zabel" <philipp.zabel@gmail.com> wrote:

> On Jan 8, 2008 1:13 AM, Matt Reimer <mreimer@vpop.net> wrote:
> > On Mon, 2008-01-07 at 15:10 -0800, Andrew Morton wrote:
> > > On Sun, 06 Jan 2008 14:46:14 +0100
> > > Philipp Zabel <philipp.zabel@gmail.com> wrote:
> > >
> > > > The DS1WM driver incorrectly infers the IAS bit (1-wire interrupt active
> > > > high) from IRQ settings. There are devices that have IAS=0 but still need
> > > > the IRQ to trigger on a rising edge. With this patch, machines with DS1WM
> > > > that need IAS=1 have to set .active_high=1 in the ds1wm_platform_data.
> >
> > > But no drivers are converted to set ds1wm_platform_data.active_high.  Won't
> > > IORESOURCE_IRQ_HIGHEDGE devices be broken by this change?
> >
> > Good point; I think you're right. I'd guess the other platforms that use
> > this driver are in the handhelds.org tree, but I've been out of the loop
> > a while. Philipp, is this the case?
> 
> Yes, I think so. I am only aware of four chips that include a DS1WM:
> HTC's ASIC3, PASIC2 and PASIC3 and Samsung SAMCOP.
> All of those drivers have yet to be submitted.
> 
> I will also apply this patch to hh.org CVS and fix up the devices that are
> affected by this change (aximx30, blueangel, magician, h1900, h4000,
> h5400, himalaya, hx4700, sable, universal).
> But none of those set IORESOURCE_IRQ_HIGHEDGE (most are just
> missing the IORESOURCE_IRQ_LOWEDGE flag). I am not sure about
> the status of rx3000 or other devices that might live in other trees.
> 
> I'm currently cleaning up the PASIC2/3 driver. After that I'll try to help
> cleaning up ASIC3 and finally getting it ready for submission.
> A whole load of devices in the hh.org tree depend on it.
> 

Guys, I'm thinking that by the time we actually need this patch in the
mainline tree, it may well be obsolete.  So I should drop the copy I have?

Re: [PATCH] DS1WM: decouple host IRQ and INTR active state settings.

[pHilipp Zabel]

Hi Andrew,

On Feb 6, 2008 8:26 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Tue, 8 Jan 2008 09:21:55 +0100 "pHilipp Zabel" <philipp.zabel@gmail.com> wrote:
>
> > On Jan 8, 2008 1:13 AM, Matt Reimer <mreimer@vpop.net> wrote:
> > > On Mon, 2008-01-07 at 15:10 -0800, Andrew Morton wrote:
> > > > On Sun, 06 Jan 2008 14:46:14 +0100
> > > > Philipp Zabel <philipp.zabel@gmail.com> wrote:
> > > >
> > > > > The DS1WM driver incorrectly infers the IAS bit (1-wire interrupt active
> > > > > high) from IRQ settings. There are devices that have IAS=0 but still need
> > > > > the IRQ to trigger on a rising edge. With this patch, machines with DS1WM
> > > > > that need IAS=1 have to set .active_high=1 in the ds1wm_platform_data.
> > >
> > > > But no drivers are converted to set ds1wm_platform_data.active_high.  Won't
> > > > IORESOURCE_IRQ_HIGHEDGE devices be broken by this change?
> > >
> > > Good point; I think you're right. I'd guess the other platforms that use
> > > this driver are in the handhelds.org tree, but I've been out of the loop
> > > a while. Philipp, is this the case?
> >
> > Yes, I think so. I am only aware of four chips that include a DS1WM:
> > HTC's ASIC3, PASIC2 and PASIC3 and Samsung SAMCOP.
> > All of those drivers have yet to be submitted.
> >
> > I will also apply this patch to hh.org CVS and fix up the devices that are
> > affected by this change (aximx30, blueangel, magician, h1900, h4000,
> > h5400, himalaya, hx4700, sable, universal).
> > But none of those set IORESOURCE_IRQ_HIGHEDGE (most are just
> > missing the IORESOURCE_IRQ_LOWEDGE flag). I am not sure about
> > the status of rx3000 or other devices that might live in other trees.
> >
> > I'm currently cleaning up the PASIC2/3 driver. After that I'll try to help
> > cleaning up ASIC3 and finally getting it ready for submission.
> > A whole load of devices in the hh.org tree depend on it.
> >
>
> Guys, I'm thinking that by the time we actually need this patch in the
> mainline tree, it may well be obsolete.  So I should drop the copy I have?

I obviously think it should be applied.
But if you prefer that I resend once it is actually needed by
something in the tree, I'll happily do that, too.
The PASIC2/3 mfd driver is currently waiting for something like Dmitry
Baryshkov's clocklib patches to go in.

regards
Philipp

Re: [PATCH] DS1WM: decouple host IRQ and INTR active state settings.

[Andrew Morton]

On Wed, 6 Feb 2008 15:35:14 +0100 "pHilipp Zabel" <philipp.zabel@gmail.com> wrote:

> > Guys, I'm thinking that by the time we actually need this patch in the
> > mainline tree, it may well be obsolete.  So I should drop the copy I have?
> 
> I obviously think it should be applied.
> But if you prefer that I resend once it is actually needed by
> something in the tree, I'll happily do that, too.
> The PASIC2/3 mfd driver is currently waiting for something like Dmitry
> Baryshkov's clocklib patches to go in.

Well.  It's a very small patch - I'm not particularly fussed either way.
As long as you gus are sure it's safe and will be useful then I guess we
can proceed with it?

Re: [PATCH] DS1WM: decouple host IRQ and INTR active state settings.

[Matt Reimer]

On Wed, 2008-02-06 at 10:01 -0800, Andrew Morton wrote:
> On Wed, 6 Feb 2008 15:35:14 +0100 "pHilipp Zabel" <philipp.zabel@gmail.com> wrote:
> 
> > > Guys, I'm thinking that by the time we actually need this patch in the
> > > mainline tree, it may well be obsolete.  So I should drop the copy I have?
> > 
> > I obviously think it should be applied.
> > But if you prefer that I resend once it is actually needed by
> > something in the tree, I'll happily do that, too.
> > The PASIC2/3 mfd driver is currently waiting for something like Dmitry
> > Baryshkov's clocklib patches to go in.
> 
> Well.  It's a very small patch - I'm not particularly fussed either way.
> As long as you gus are sure it's safe and will be useful then I guess we
> can proceed with it?

I agree with Philipp, and think it should go in.

Matt