2012-02-19 02:10:48

by NeilBrown

[permalink] [raw]
Subject: [PATCH 0/3] Small fixes for w1_bq27000 driver

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


2012-02-19 02:10:57

by NeilBrown

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

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 <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---

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);


2012-02-19 02:11:05

by NeilBrown

[permalink] [raw]
Subject: [PATCH 2/3] w1_bq27000 - remove w1_bq27000_write

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 <[email protected]>
---

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;

2012-02-19 02:11:20

by NeilBrown

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

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

Signed-off-by: NeilBrown <[email protected]>
---

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;
}

2012-02-19 06:49:05

by Thomas Weber

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

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

2012-02-19 07:06:33

by NeilBrown

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

On Sun, 19 Feb 2012 07:50:23 +0100 Thomas Weber
<[email protected]> 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


Attachments:
signature.asc (828.00 B)

2012-02-20 11:19:59

by Thomas Weber

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

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
> <[email protected]> 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

2012-02-20 12:04:24

by Evgeniy Polyakov

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

Hi Neil

On Sun, Feb 19, 2012 at 01:10:00PM +1100, NeilBrown ([email protected]) 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 <[email protected]>

--
Evgeniy Polyakov

2012-02-20 13:57:01

by Thomas Weber

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

From: Paul Walmsley <[email protected]>

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

2012-02-20 15:22:39

by Evgeniy Polyakov

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

On Mon, Feb 20, 2012 at 02:56:00PM +0100, Thomas Weber ([email protected]) 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