LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Al Viro <viro@ftp.linux.org.uk>, mgreer@mvista.com
Cc: Linus Torvalds <torvalds@osdl.org>,
	dhowells@redhat.com, linux-kernel@vger.kernel.org
Subject: Re: ... and more...
Date: Thu, 07 Dec 2006 13:08:00 +0000	[thread overview]
Message-ID: <1693.1165496880@redhat.com> (raw)
In-Reply-To: <20061206195006.GH4587@ftp.linux.org.uk>

Al Viro <viro@ftp.linux.org.uk> wrote:

> diff --git a/drivers/i2c/chips/m41t00.c b/drivers/i2c/chips/m41t00.c

This could be done differently.  m41t00_set() is only called on that global
variable, so why pass its address in as an argument?

The old code is also wrong: m41t00_set() was being passed a pointer to an
unsigned long, but then was casting it to a pointer to an int before
dereferencing it.  This works on a 32-bit platform, but it's just asking for
trouble on a 64-bit big-endian platform.  However, the driver can only be
configured when CONFIG_PPC32=y, so it will have gone unnoticed.

David
---
Fix m41t00_set() and its usage with workqueues

From: David Howells <dhowells@redhat.com>

Fix m41t00_set() and its usage with workqueues.  It doesn't really need an
argument since it always affects the same global variable.

Signed-Off-By: David Howells <dhowells@redhat.com>
---

 drivers/i2c/chips/m41t00.c |   18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/chips/m41t00.c b/drivers/i2c/chips/m41t00.c
index 420377c..98978ab 100644
--- a/drivers/i2c/chips/m41t00.c
+++ b/drivers/i2c/chips/m41t00.c
@@ -165,11 +165,13 @@ read_err:
 }
 EXPORT_SYMBOL_GPL(m41t00_get_rtc_time);
 
+static ulong new_time;
+
 static void
-m41t00_set(void *arg)
+m41t00_set(struct work_struct *unused)
 {
 	struct rtc_time	tm;
-	int nowtime = *(int *)arg;
+	int nowtime = new_time;
 	s32 sec, min, hour, day, mon, year;
 	u8 wbuf[9], *buf = &wbuf[1], msgbuf[1] = { 0 };
 	struct i2c_msg msgs[] = {
@@ -214,16 +216,8 @@ m41t00_set(void *arg)
 		dev_err(&save_client->dev, "m41t00_set: Write error\n");
 }
 
-static ulong new_time;
-/* well, isn't this API just _lovely_? */
-static void
-m41t00_barf(struct work_struct *unusable)
-{
-	m41t00_set(&new_time);
-}
-
 static struct workqueue_struct *m41t00_wq;
-static DECLARE_WORK(m41t00_work, m41t00_barf);
+static DECLARE_WORK(m41t00_work, m41t00_set);
 
 int
 m41t00_set_rtc_time(ulong nowtime)
@@ -233,7 +227,7 @@ m41t00_set_rtc_time(ulong nowtime)
 	if (in_interrupt())
 		queue_work(m41t00_wq, &m41t00_work);
 	else
-		m41t00_set(&new_time);
+		m41t00_set(NULL);
 
 	return 0;
 }

      parent reply	other threads:[~2006-12-07 13:09 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-06 18:41 work_struct-induced breakage, part 1 of fsck-knows-how-many Al Viro
2006-12-06 18:51 ` more work_struct-induced breakage Al Viro
2006-12-06 19:18   ` and more of the same Al Viro
2006-12-06 19:46     ` ... and more Al Viro
2006-12-06 19:50       ` Al Viro
2006-12-06 21:15       ` ... and then some Al Viro
2006-12-07 13:08       ` David Howells [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1693.1165496880@redhat.com \
    --to=dhowells@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgreer@mvista.com \
    --cc=torvalds@osdl.org \
    --cc=viro@ftp.linux.org.uk \
    --subject='Re: ... and more...' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).