[PATCH 0/3] Small fixes for w1_bq27000 driver

[PATCH 0/3] Small fixes for w1_bq27000 driver

[NeilBrown]

The following three patches fix a problem with the w1 interface for
the bq27000 battery charge meter, and remove some unnecessary code.

thanks,
NeilBrown


---

NeilBrown (3):
      w1_bq27000: Only one thread can access the bq27000 at a time.
      w1_bq27000 - remove w1_bq27000_write
      w1_bq27000: remove unnecessary NULL test.


 drivers/w1/slaves/w1_bq27000.c |   19 ++-----------------
 1 files changed, 2 insertions(+), 17 deletions(-)

-- 
Signature

[PATCH 3/3] w1_bq27000: Only one thread can access the bq27000 at a time.

[NeilBrown]

If multiple threads try, they trip over each other badly.

Signed-off-by: NeilBrown <neilb@suse.de>
---

 drivers/w1/slaves/w1_bq27000.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/w1/slaves/w1_bq27000.c b/drivers/w1/slaves/w1_bq27000.c
index 6ae60aa..52ad812 100644
--- a/drivers/w1/slaves/w1_bq27000.c
+++ b/drivers/w1/slaves/w1_bq27000.c
@@ -31,8 +31,10 @@ static int w1_bq27000_read(struct device *dev, unsigned int reg)
 	u8 val;
 	struct w1_slave *sl = container_of(dev->parent, struct w1_slave, dev);
 
+	mutex_lock(&sl->master->mutex);
 	w1_write_8(sl->master, HDQ_CMD_READ | reg);
 	val = w1_read_8(sl->master);
+	mutex_unlock(&sl->master->mutex);
 
 	return val;
 }

[PATCH 1/3] w1_bq27000: remove unnecessary NULL test.

[NeilBrown]

As recent change means that we now dereference 'dev' before testing
for NULL.

That means either the change was wrong, or the test isn't needed.
As this function is only called from one driver (bq27x000_battery) and
it always passed a non-NULL dev, it seems good to assume that the
test isn't needed.

So remove it.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: NeilBrown <neilb@suse.de>
---

 drivers/w1/slaves/w1_bq27000.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/w1/slaves/w1_bq27000.c b/drivers/w1/slaves/w1_bq27000.c
index 8f10fd2..50d9af7 100644
--- a/drivers/w1/slaves/w1_bq27000.c
+++ b/drivers/w1/slaves/w1_bq27000.c
@@ -45,9 +45,6 @@ static int w1_bq27000_read(struct device *dev, unsigned int reg)
 	u8 val;
 	struct w1_slave *sl = container_of(dev->parent, struct w1_slave, dev);
 
-	if (!dev)
-		return 0;
-
 	w1_write_8(sl->master, HDQ_CMD_READ | reg);
 	val = w1_read_8(sl->master);
 

[PATCH 2/3] w1_bq27000 - remove w1_bq27000_write

[NeilBrown]

The function is never used so remove it to avoid bit-rot.
It can trivially be re-added if there is ever a need.

Signed-off-by: NeilBrown <neilb@suse.de>
---

 drivers/w1/slaves/w1_bq27000.c |   14 --------------
 1 files changed, 0 insertions(+), 14 deletions(-)

diff --git a/drivers/w1/slaves/w1_bq27000.c b/drivers/w1/slaves/w1_bq27000.c
index 50d9af7..6ae60aa 100644
--- a/drivers/w1/slaves/w1_bq27000.c
+++ b/drivers/w1/slaves/w1_bq27000.c
@@ -26,20 +26,6 @@
 
 static int F_ID;
 
-void w1_bq27000_write(struct device *dev, u8 buf, u8 reg)
-{
-	struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
-
-	if (!dev) {
-		pr_info("Could not obtain slave dev ptr\n");
-		return;
-	}
-
-	w1_write_8(sl->master, HDQ_CMD_WRITE | reg);
-	w1_write_8(sl->master, buf);
-}
-EXPORT_SYMBOL(w1_bq27000_write);
-
 static int w1_bq27000_read(struct device *dev, unsigned int reg)
 {
 	u8 val;

Re: [PATCH 0/3] Small fixes for w1_bq27000 driver

[Thomas Weber]

On 02/19/2012 03:10 AM, NeilBrown wrote:
> The following three patches fix a problem with the w1 interface for
> the bq27000 battery charge meter, and remove some unnecessary code.
>
> thanks,
> NeilBrown
>
>
> ---
>
> NeilBrown (3):
>        w1_bq27000: Only one thread can access the bq27000 at a time.
>        w1_bq27000 - remove w1_bq27000_write
>        w1_bq27000: remove unnecessary NULL test.
>
>
>   drivers/w1/slaves/w1_bq27000.c |   19 ++-----------------
>   1 files changed, 2 insertions(+), 17 deletions(-)
>
Hello Neil,
I have the following problem. The board not correctly reads the values 
from the bq27000. It also doesn't detects if the bq27000 is connected. I 
think it always reads 0xFF.

Do you have an omap3 board and a bq27000 battery?

Regards,

Thomas

Re: [PATCH 0/3] Small fixes for w1_bq27000 driver

[NeilBrown]

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

On Sun, 19 Feb 2012 07:50:23 +0100 Thomas Weber
<thomas.weber.linux@googlemail.com> wrote:

> On 02/19/2012 03:10 AM, NeilBrown wrote:
> > The following three patches fix a problem with the w1 interface for
> > the bq27000 battery charge meter, and remove some unnecessary code.
> >
> > thanks,
> > NeilBrown
> >
> >
> > ---
> >
> > NeilBrown (3):
> >        w1_bq27000: Only one thread can access the bq27000 at a time.
> >        w1_bq27000 - remove w1_bq27000_write
> >        w1_bq27000: remove unnecessary NULL test.
> >
> >
> >   drivers/w1/slaves/w1_bq27000.c |   19 ++-----------------
> >   1 files changed, 2 insertions(+), 17 deletions(-)
> >
> Hello Neil,
> I have the following problem. The board not correctly reads the values 
> from the bq27000. It also doesn't detects if the bq27000 is connected. I 
> think it always reads 0xFF.
> 
> Do you have an omap3 board and a bq27000 battery?

Yes, I have an OMAP3 with the HDQ line connected to a battery with a bq27000
in it.  It now works fine.

When I first started working on it I would sometimes get problems with lots
of 0xFF being read, usually with lots of delays.

One problem was that during boot there would often be multiple threads
accessing the battery and tripping over each other.
The third patch of this set fixes that.

The other problem was power saving issues in the OMAP3 drivers.  These
we address with lots of help from Paul Walmsley.

You can find my current set of HDQ patches here:

http://neil.brown.name/git?p=gta04;a=shortlog;h=refs/heads/hdq

 (or git://neil.brown.name/gta04 hdq)

Most of these are from Paul.

Please let me know if they help.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 0/3] Small fixes for w1_bq27000 driver

[Thomas Weber]

Hello Neil,

thanks for the link to your git repo. I checked today with an
oscilloscope and there were no communication between the omap and the
battery. I then merged your whole hdq branch and now it works.

Thanks. Have to look, which commit makes it work.


Thomas


On 19.02.2012 08:06, NeilBrown wrote:
> On Sun, 19 Feb 2012 07:50:23 +0100 Thomas Weber
> <thomas.weber.linux@googlemail.com> wrote:
> 
>> On 02/19/2012 03:10 AM, NeilBrown wrote:
>>> The following three patches fix a problem with the w1 interface for
>>> the bq27000 battery charge meter, and remove some unnecessary code.
>>>
>>> thanks,
>>> NeilBrown
>>>
>>>
>>> ---
>>>
>>> NeilBrown (3):
>>>        w1_bq27000: Only one thread can access the bq27000 at a time.
>>>        w1_bq27000 - remove w1_bq27000_write
>>>        w1_bq27000: remove unnecessary NULL test.
>>>
>>>
>>>   drivers/w1/slaves/w1_bq27000.c |   19 ++-----------------
>>>   1 files changed, 2 insertions(+), 17 deletions(-)
>>>
>> Hello Neil,
>> I have the following problem. The board not correctly reads the values 
>> from the bq27000. It also doesn't detects if the bq27000 is connected. I 
>> think it always reads 0xFF.
>>
>> Do you have an omap3 board and a bq27000 battery?
> 
> Yes, I have an OMAP3 with the HDQ line connected to a battery with a bq27000
> in it.  It now works fine.
> 
> When I first started working on it I would sometimes get problems with lots
> of 0xFF being read, usually with lots of delays.
> 
> One problem was that during boot there would often be multiple threads
> accessing the battery and tripping over each other.
> The third patch of this set fixes that.
> 
> The other problem was power saving issues in the OMAP3 drivers.  These
> we address with lots of help from Paul Walmsley.
> 
> You can find my current set of HDQ patches here:
> 
> http://neil.brown.name/git?p=gta04;a=shortlog;h=refs/heads/hdq
> 
>  (or git://neil.brown.name/gta04 hdq)
> 
> Most of these are from Paul.
> 
> Please let me know if they help.
> 
> NeilBrown

[PATCH] OMAP: hdq: software-supervised control of HDQ_ICLK

[Thomas Weber]

From: Paul Walmsley <paul@pwsan.com>

Hello Neil, hello Paul,

the patch below fixes the communication between omap3 and
bq27000 battery.

I got the patch from Neil's git tree and removed the part for hdq1w.

Regards,
Thomas

Do not automatically enable interface clock autoidle on
HDQ_ICLK. Require the hwmod layer to use software gating on
HDQ_ICLK. This is because the CM FSM that handles HDQ_FCLK
appears to not be working as expected.
---
 arch/arm/mach-omap2/clock3xxx_data.c    |    1 +
 arch/arm/plat-omap/clock.c              |    4 +++-
 arch/arm/plat-omap/include/plat/clock.h |    1 +
 3 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/clock3xxx_data.c b/arch/arm/mach-omap2/clock3xxx_data.c
index d75e5f6..ac95afd 100644
--- a/arch/arm/mach-omap2/clock3xxx_data.c
+++ b/arch/arm/mach-omap2/clock3xxx_data.c
@@ -1882,6 +1882,7 @@ static struct clk hdq_ick = {
 	.enable_bit	= OMAP3430_EN_HDQ_SHIFT,
 	.clkdm_name	= "core_l4_clkdm",
 	.recalc		= &followparent_recalc,
+	.flags		= SWSUP_ICLK_IDLE,
 };
 
 static struct clk mcspi4_ick = {
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index 567e4b5..3fce16a 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -348,7 +348,9 @@ int omap_clk_enable_autoidle_all(void)
 	spin_lock_irqsave(&clockfw_lock, flags);
 
 	list_for_each_entry(c, &clocks, node)
-		if (c->ops->allow_idle)
+		if (c->flags & SWSUP_ICLK_IDLE && c->ops->deny_idle)
+			c->ops->deny_idle(c);
+		else if (c->ops->allow_idle)
 			c->ops->allow_idle(c);
 
 	spin_unlock_irqrestore(&clockfw_lock, flags);
diff --git a/arch/arm/plat-omap/include/plat/clock.h b/arch/arm/plat-omap/include/plat/clock.h
index 240a7b9..f4f6e7b 100644
--- a/arch/arm/plat-omap/include/plat/clock.h
+++ b/arch/arm/plat-omap/include/plat/clock.h
@@ -191,6 +191,7 @@ struct dpll_data {
 #define ENABLE_ON_INIT		(1 << 3)	/* Enable upon framework init */
 #define INVERT_ENABLE		(1 << 4)	/* 0 enables, 1 disables */
 #define CLOCK_CLKOUTX2		(1 << 5)
+#define SWSUP_ICLK_IDLE		(1 << 6)
 
 /**
  * struct clk - OMAP struct clk
-- 
1.7.8.4

Re: [PATCH] OMAP: hdq: software-supervised control of HDQ_ICLK

[Evgeniy Polyakov]

On Mon, Feb 20, 2012 at 02:56:00PM +0100, Thomas Weber (weber@corscience.de) wrote:
> the patch below fixes the communication between omap3 and
> bq27000 battery.
> 
> I got the patch from Neil's git tree and removed the part for hdq1w.

Guys, can you submit whole patchset without splitting it into patches,
which eventually may conflict with the same tree sent by someone else?

-- 
	Evgeniy Polyakov

Re: [PATCH 0/3] Small fixes for w1_bq27000 driver

[Evgeniy Polyakov]

Hi Neil

On Sun, Feb 19, 2012 at 01:10:00PM +1100, NeilBrown (neilb@suse.de) wrote:
> The following three patches fix a problem with the w1 interface for
> the bq27000 battery charge meter, and remove some unnecessary code.

Whole patchset looks very good, thank you
Greg, please pull it into your tree

Acked-by: Evgeniy Polyakov <zbr@ioremap.net>

-- 
	Evgeniy Polyakov