* Re: Shared i2c adapter locking
From: Jean Delvare @ 2009-11-17 9:35 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Ben Hutchings, David Miller, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-next-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mika Kuoppala, Linux I2C
In-Reply-To: <20091117193313.b750804e.sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
Hi Stephen,
On Tue, 17 Nov 2009 19:33:13 +1100, Stephen Rothwell wrote:
> Hi Jean,
>
> On Thu, 5 Nov 2009 15:07:15 +0100 Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
> >
> > > Presumably this is meant for net-next-2.6, and you'll implement
> >
> > Actually I meant to push this to Linus immediately, through my i2c
> > tree. This is essentially a no-op: the binary code will be the same as
> > before the patch, so guaranteed to be safe, and this will solve
> > conflicts in linux-next.
>
> Any word on this?
Haven't you read this:
http://www.spinics.net/lists/linux-next/msg07839.html
and the 3 following posts?
If you did and something is still not clear, please let me know. My
understanding is that the action token is in David's hands now.
--
Jean Delvare
^ permalink raw reply
* Re: Shared i2c adapter locking
From: Stephen Rothwell @ 2009-11-17 8:33 UTC (permalink / raw)
To: Jean Delvare
Cc: Ben Hutchings, David Miller, netdev, linux-next, linux-kernel,
Mika Kuoppala, Linux I2C
In-Reply-To: <20091105150715.692b5200@hyperion.delvare>
[-- Attachment #1: Type: text/plain, Size: 533 bytes --]
Hi Jean,
On Thu, 5 Nov 2009 15:07:15 +0100 Jean Delvare <khali@linux-fr.org> wrote:
>
> > Presumably this is meant for net-next-2.6, and you'll implement
>
> Actually I meant to push this to Linus immediately, through my i2c
> tree. This is essentially a no-op: the binary code will be the same as
> before the patch, so guaranteed to be safe, and this will solve
> conflicts in linux-next.
Any word on this?
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH] at24: use timeout also for read
From: Jean Delvare @ 2009-11-16 18:56 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
ivanov.aleks-Re5JQEeQqe8AvxtiuMwx3w, David Brownell
In-Reply-To: <20091116185030.GB21491-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On Mon, 16 Nov 2009 19:50:30 +0100, Wolfram Sang wrote:
> On Sun, Nov 08, 2009 at 09:14:57PM +0100, Wolfram Sang wrote:
> > Writes may take some time on EEPROMs, so for consecutive writes, we already
> > have a loop waiting for the EEPROM to become ready. Use such a loop for reads,
> > too, in case somebody wants to immediately read after a write. Detailed bug
> > report and test case can be found here:
> >
> > http://article.gmane.org/gmane.linux.drivers.i2c/4660
> >
> > Reported-by: Aleksandar Ivanov <ivanov.aleks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > Cc: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> > Cc: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
>
> Jean, you are probably busy, just a short note will do: Is this scheduled for
> 2.6.33?
Yes it is, but I must find some time to review and test your change
first. Busy...
--
Jean Delvare
^ permalink raw reply
* Re: [PATCH] at24: use timeout also for read
From: Wolfram Sang @ 2009-11-16 18:50 UTC (permalink / raw)
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA
Cc: ivanov.aleks-Re5JQEeQqe8AvxtiuMwx3w, David Brownell, Jean Delvare
In-Reply-To: <1257711297-19927-1-git-send-email-w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]
On Sun, Nov 08, 2009 at 09:14:57PM +0100, Wolfram Sang wrote:
> Writes may take some time on EEPROMs, so for consecutive writes, we already
> have a loop waiting for the EEPROM to become ready. Use such a loop for reads,
> too, in case somebody wants to immediately read after a write. Detailed bug
> report and test case can be found here:
>
> http://article.gmane.org/gmane.linux.drivers.i2c/4660
>
> Reported-by: Aleksandar Ivanov <ivanov.aleks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> Cc: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Jean, you are probably busy, just a short note will do: Is this scheduled for
2.6.33?
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: yield() in i2c non-happy paths hits BUG under -rt patch
From: Mark Brown @ 2009-11-16 15:56 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Jean Delvare, Sven-Thorsten Dietrich, Leon Woestenberg,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, rt-users,
Ben Dooks (embedded platforms), Peter Zijlstra, LKML
In-Reply-To: <alpine.LFD.2.00.0911132139560.24119-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
On Fri, Nov 13, 2009 at 11:03:39PM +0100, Thomas Gleixner wrote:
> Q: Why put people yield() into their code ?
> A: Because:
> - it is less nasty than busy waiting for a long time
> - it works better
...
> I can see the idea of using yield() to let other tasks making progress
> in situations where the hardware is a design failure as well,
> e.g. bitbang devices. But if we have to deal with hardware which is
> crap by design yield() is the worst of all answers simply due to its
> horrible semantics.
What other options are there available for the first case (which is
often why things work better with the use of yield) that don't involve
sleeps, or is the idea that in situations like this drivers should
always sleep?
^ permalink raw reply
* [PATCH 23/22] i2c-designware: i2c_dw_handle_tx_abort: Use dev_dbg() for NOACK cases
From: Shinya Kuribayashi @ 2009-11-16 11:40 UTC (permalink / raw)
To: baruch-NswTu9S1W3P6gbPvEgmw2w, ben-linux-elnMNo+KYs3YtjvyW6yDsg,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <4AF419B6.1000802-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
In the case of no-ACKs, we don't want to see dev_err() messages in the
console, because some utilities like i2c-tools are capable of printing
decorated console output. This patch will ease such situations.
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
---
Hi Ben,
This patch can be applied on the top of the v2 patchset (as 23/22).
I've tested the patch with both no-ACK cases and arbitration case.
As for errors other than NOACKs, it's worth doing dev_err().
drivers/i2c/busses/i2c-designware.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index 4534d45..9e18ef9 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -496,13 +496,18 @@ static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
unsigned long abort_source = dev->abort_source;
int i;
+ if (abort_source & DW_IC_TX_ABRT_NOACK) {
+ for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
+ dev_dbg(dev->dev,
+ "%s: %s\n", __func__, abort_sources[i]);
+ return -EREMOTEIO;
+ }
+
for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);
if (abort_source & DW_IC_TX_ARB_LOST)
return -EAGAIN;
- else if (abort_source & DW_IC_TX_ABRT_NOACK)
- return -EREMOTEIO;
else if (abort_source & DW_IC_TX_ABRT_GCALL_READ)
return -EINVAL; /* wrong msgs[] data */
else
--
1.6.5.2
^ permalink raw reply related
* Re: [PATCH v2 21/22] i2c-designware: Tx abort cleanups
From: Shinya Kuribayashi @ 2009-11-16 11:27 UTC (permalink / raw)
To: Ben Dooks; +Cc: baruch-NswTu9S1W3P6gbPvEgmw2w, linux-i2c-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20091114222249.GM13398-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org>
Ben Dooks wrote:
>>> + for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
>>> + dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);
>> Dev_err() might be annoying when we use sort of misc utilities
>> such as i2c-tools. In case of no ACKs, we sometimes don't want
>> to see error messages in the console, because such tools are
>> sometimes capable of generating human-friendly console output
>> like,
>>
>> root@localhost:/root> i2cdetect -y -r 2
>> 0 1 2 3 4 5 6 7 8 9 a b c d e f
>> 00: -- -- -- -- -- -- -- -- -- -- -- -- --
>> 10: -- -- -- -- -- -- -- -- ...
>>
>> root@localhost:/root> i2cdump -y 0 0x49 b
>> 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
>> 00: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
>> 10: XX XX XX XX XX XX ....
>>
>> Then how do I change dev_err() here? I'm not sure dev_dbg() or
>> other variants are acceptable for no ACKs or arbitration cases.
>> Could someone give me a comment? I'll prepare patches for it.
>
> Maybe in the case of no-ack, this could be a debug message and erro
> for the case where you have an arbitration problem or other error that
> you wouldn't normally find on an I2C bus?
Thanks, I'll send an additional patch shortly.
--
Shinya Kuribayashi
NEC Electronics
^ permalink raw reply
* Re: [PATCH 4/4] i2c : Map I2C adapter number to platform ID number
From: Ben Dooks @ 2009-11-16 10:03 UTC (permalink / raw)
To: Kevin Wells
Cc: Ben Dooks, vitalywool-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <083DF309106F364B939360100EC290F804F53CC644-SIPbe8o7cfX8DdpCu65jn8FrZmdRls4ZQQ4Iyu8u01E@public.gmane.org>
On Thu, Nov 12, 2009 at 12:34:17AM +0100, Kevin Wells wrote:
> Map I2C adapter number to platform ID number
>
> Signed-off-by: Kevin Wells <kevin.wells-3arQi8VN3Tc@public.gmane.org>
I've queued this onto i2c-next-misc ready for the next kernel merge
window, as it isn't really a bugfix. The rest are on i2c-fixes-pnx
and will pushed in a few days time.
--
Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
^ permalink raw reply
* Re: [uml-user] [PATCH 00/12] move compat_ioctl handling into drivers
From: Arnd Bergmann @ 2009-11-16 1:02 UTC (permalink / raw)
To: Neil Brown
Cc: linux-usb, user-mode-linux-user, H. Peter Anvin, Paul Clements,
Wolfram Sang, Andre Noll, Pavel Machek, linux-i2c, Jens Axboe,
Jan Blunck, Ian Kent, Nick Piggin, linux-scsi, Kay Sievers,
Alon Bar-Lev, Alan Stern, KOSAKI Motohiro, Doug Gilbert,
David Vrabel, Alexey Dobriyan, user-mode-linux-devel, Jeff Dike,
Oliver Neukum, linux-raid, Tejun Heo
In-Reply-To: <20091116115715.600e346f@notabene.brown>
On Monday 16 November 2009, Neil Brown wrote:
> I would take the md ones into my tree, but I suspect that if
> everyone did that we would end up with lots of conflicts in
> fs/compat_ioctl.c.
>
> So how about I just take the changes to md.c, and give you an:
>
> Acked-by: NeilBrown <neilb@suse.de>
>
> for the changes to fs/compat_ioctl.c
> ??
Yes, that is exactly what I was trying to suggest. Thanks,
Arnd <><
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
^ permalink raw reply
* Re: [uml-user] [PATCH 00/12] move compat_ioctl handling into drivers
From: Neil Brown @ 2009-11-16 0:57 UTC (permalink / raw)
Cc: linux-usb, user-mode-linux-user, H. Peter Anvin, Paul Clements,
Wolfram Sang, Andre Noll, Pavel Machek, linux-i2c, Jens Axboe,
Jan Blunck, Ian Kent, Nick Piggin, user-mode-linux-devel,
linux-scsi, Kay Sievers, Alon Bar-Lev, Alan Stern,
KOSAKI Motohiro, Doug Gilbert, David Vrabel, Alexey Dobriyan,
Arnd Bergmann, Jeff Dike, Oliver Neukum, linux-raid
In-Reply-To: <1258331227-1694-1-git-send-email-arnd@arndb.de>
On Mon, 16 Nov 2009 00:26:55 +0000
Arnd Bergmann <arnd@arndb.de> wrote:
> This is the second series of patches on compat_ioctl handling,
> moving compat handlers into more drivers. Most of these
> patches consist on a part that adds a compat_ioctl method
> in one driver and another part removing the respective
> section from fs/compat_ioctl.c. The fs/compat_ioctl.c
> portion depends on other patches[1], but the driver code can
> be applied independently.
>
> If some of you feel responsible for the code I patched, please
> look at my changes and if they are ok, either take them into
> your tree or give me an Acked-by for them.
I would take the md ones into my tree, but I suspect that if
everyone did that we would end up with lots of conflicts in
fs/compat_ioctl.c.
So how about I just take the changes to md.c, and give you an:
Acked-by: NeilBrown <neilb@suse.de>
for the changes to fs/compat_ioctl.c
??
NeilBrown
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
^ permalink raw reply
* [PATCH 07/12] i2cdev: move compat_ioctl handling into driver
From: Arnd Bergmann @ 2009-11-16 0:27 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Jean Delvare (PC drivers, core),
Ben Dooks (embedded platforms), Wolfram Sang, linux-i2c
In-Reply-To: <1258331227-1694-1-git-send-email-arnd@arndb.de>
Doing all the compat_ioctl handling in the i2c driver itself
removes special cases from fs/compat_ioctl.c and makes it possible
to optimize this case better.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: "Jean Delvare (PC drivers, core)" <khali@linux-fr.org>
Cc: "Ben Dooks (embedded platforms)" <ben-linux@fluff.org>
Cc: Wolfram Sang <w.sang@pengutronix.de>
Cc: linux-i2c@vger.kernel.org
---
drivers/i2c/i2c-dev.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++
fs/compat_ioctl.c | 119 -------------------------------------------------
2 files changed, 117 insertions(+), 119 deletions(-)
diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index 7e13d2d..fde0c9e 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -35,6 +35,7 @@
#include <linux/i2c.h>
#include <linux/i2c-dev.h>
#include <linux/smp_lock.h>
+#include <linux/compat.h>
#include <linux/jiffies.h>
#include <asm/uaccess.h>
@@ -439,6 +440,119 @@ static long i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
return 0;
}
+#ifdef CONFIG_COMPAT
+struct i2c_msg32 {
+ u16 addr;
+ u16 flags;
+ u16 len;
+ compat_caddr_t buf;
+};
+
+struct i2c_rdwr_ioctl_data32 {
+ compat_caddr_t msgs; /* struct i2c_msg __user *msgs */
+ u32 nmsgs;
+};
+
+struct i2c_smbus_ioctl_data32 {
+ u8 read_write;
+ u8 command;
+ u32 size;
+ compat_caddr_t data; /* union i2c_smbus_data *data */
+};
+
+struct i2c_rdwr_aligned {
+ struct i2c_rdwr_ioctl_data cmd;
+ struct i2c_msg msgs[0];
+};
+
+static int compat_i2c_rdwr_ioctl(struct file *filp, unsigned int cmd,
+ struct i2c_rdwr_ioctl_data32 __user *udata)
+{
+ struct i2c_rdwr_aligned __user *tdata;
+ struct i2c_msg __user *tmsgs;
+ struct i2c_msg32 __user *umsgs;
+ compat_caddr_t datap;
+ int nmsgs, i;
+
+ if (get_user(nmsgs, &udata->nmsgs))
+ return -EFAULT;
+ if (nmsgs > I2C_RDRW_IOCTL_MAX_MSGS)
+ return -EINVAL;
+
+ if (get_user(datap, &udata->msgs))
+ return -EFAULT;
+ umsgs = compat_ptr(datap);
+
+ tdata = compat_alloc_user_space(sizeof(*tdata) +
+ nmsgs * sizeof(struct i2c_msg));
+ tmsgs = &tdata->msgs[0];
+
+ if (put_user(nmsgs, &tdata->cmd.nmsgs) ||
+ put_user(tmsgs, &tdata->cmd.msgs))
+ return -EFAULT;
+
+ for (i = 0; i < nmsgs; i++) {
+ if (copy_in_user(&tmsgs[i].addr, &umsgs[i].addr, 3*sizeof(u16)))
+ return -EFAULT;
+ if (get_user(datap, &umsgs[i].buf) ||
+ put_user(compat_ptr(datap), &tmsgs[i].buf))
+ return -EFAULT;
+ }
+ return i2cdev_ioctl(filp, cmd, (unsigned long)tdata);
+}
+
+static int compat_i2c_smbus_ioctl(struct file *filp, unsigned int cmd,
+ struct i2c_smbus_ioctl_data32 __user *udata)
+{
+ struct i2c_smbus_ioctl_data __user *tdata;
+ compat_caddr_t datap;
+
+ tdata = compat_alloc_user_space(sizeof(*tdata));
+ if (tdata == NULL)
+ return -ENOMEM;
+ if (!access_ok(VERIFY_WRITE, tdata, sizeof(*tdata)))
+ return -EFAULT;
+
+ if (!access_ok(VERIFY_READ, udata, sizeof(*udata)))
+ return -EFAULT;
+
+ if (__copy_in_user(&tdata->read_write, &udata->read_write, 2 * sizeof(u8)))
+ return -EFAULT;
+ if (__copy_in_user(&tdata->size, &udata->size, 2 * sizeof(u32)))
+ return -EFAULT;
+ if (__get_user(datap, &udata->data) ||
+ __put_user(compat_ptr(datap), &tdata->data))
+ return -EFAULT;
+
+ return i2cdev_ioctl(filp, cmd, (unsigned long)tdata);
+}
+
+static int compat_i2c_funcs(struct file *filp, unsigned int cmd,
+ compat_ulong_t __user *argp)
+{
+ struct i2c_client *client = filp->private_data;
+ compat_ulong_t funcs;
+ funcs = i2c_get_functionality(client->adapter);
+ return put_user(funcs, argp);
+}
+
+static long i2cdev_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+ void __user *argp = compat_ptr(arg);
+
+ switch (cmd) {
+ case I2C_FUNCS:
+ return compat_i2c_funcs(filp, cmd, argp);
+ case I2C_RDWR:
+ return compat_i2c_rdwr_ioctl(filp, cmd, argp);
+ case I2C_SMBUS:
+ return compat_i2c_smbus_ioctl(filp, cmd, argp);
+ }
+
+ return i2cdev_ioctl(filp, cmd, arg);
+}
+#endif /* CONFIG_COMPAT */
+
static int i2cdev_open(struct inode *inode, struct file *file)
{
unsigned int minor = iminor(inode);
@@ -501,6 +615,9 @@ static const struct file_operations i2cdev_fops = {
.read = i2cdev_read,
.write = i2cdev_write,
.unlocked_ioctl = i2cdev_ioctl,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = i2cdev_compat_ioctl,
+#endif
.open = i2cdev_open,
.release = i2cdev_release,
};
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index a762fb1..b419459 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -117,21 +117,6 @@
#include <asm/fbio.h>
#endif
-static int w_long(unsigned int fd, unsigned int cmd,
- compat_ulong_t __user *argp)
-{
- mm_segment_t old_fs = get_fs();
- int err;
- unsigned long val;
-
- set_fs (KERNEL_DS);
- err = sys_ioctl(fd, cmd, (unsigned long)&val);
- set_fs (old_fs);
- if (!err && put_user(val, argp))
- return -EFAULT;
- return err;
-}
-
struct compat_video_event {
int32_t type;
compat_time_t timestamp;
@@ -691,96 +676,6 @@ static int do_usbdevfs_discsignal(unsigned int fd, unsigned int cmd,
return err;
}
-/*
- * I2C layer ioctls
- */
-
-struct i2c_msg32 {
- u16 addr;
- u16 flags;
- u16 len;
- compat_caddr_t buf;
-};
-
-struct i2c_rdwr_ioctl_data32 {
- compat_caddr_t msgs; /* struct i2c_msg __user *msgs */
- u32 nmsgs;
-};
-
-struct i2c_smbus_ioctl_data32 {
- u8 read_write;
- u8 command;
- u32 size;
- compat_caddr_t data; /* union i2c_smbus_data *data */
-};
-
-struct i2c_rdwr_aligned {
- struct i2c_rdwr_ioctl_data cmd;
- struct i2c_msg msgs[0];
-};
-
-static int do_i2c_rdwr_ioctl(unsigned int fd, unsigned int cmd,
- struct i2c_rdwr_ioctl_data32 __user *udata)
-{
- struct i2c_rdwr_aligned __user *tdata;
- struct i2c_msg __user *tmsgs;
- struct i2c_msg32 __user *umsgs;
- compat_caddr_t datap;
- int nmsgs, i;
-
- if (get_user(nmsgs, &udata->nmsgs))
- return -EFAULT;
- if (nmsgs > I2C_RDRW_IOCTL_MAX_MSGS)
- return -EINVAL;
-
- if (get_user(datap, &udata->msgs))
- return -EFAULT;
- umsgs = compat_ptr(datap);
-
- tdata = compat_alloc_user_space(sizeof(*tdata) +
- nmsgs * sizeof(struct i2c_msg));
- tmsgs = &tdata->msgs[0];
-
- if (put_user(nmsgs, &tdata->cmd.nmsgs) ||
- put_user(tmsgs, &tdata->cmd.msgs))
- return -EFAULT;
-
- for (i = 0; i < nmsgs; i++) {
- if (copy_in_user(&tmsgs[i].addr, &umsgs[i].addr, 3*sizeof(u16)))
- return -EFAULT;
- if (get_user(datap, &umsgs[i].buf) ||
- put_user(compat_ptr(datap), &tmsgs[i].buf))
- return -EFAULT;
- }
- return sys_ioctl(fd, cmd, (unsigned long)tdata);
-}
-
-static int do_i2c_smbus_ioctl(unsigned int fd, unsigned int cmd,
- struct i2c_smbus_ioctl_data32 __user *udata)
-{
- struct i2c_smbus_ioctl_data __user *tdata;
- compat_caddr_t datap;
-
- tdata = compat_alloc_user_space(sizeof(*tdata));
- if (tdata == NULL)
- return -ENOMEM;
- if (!access_ok(VERIFY_WRITE, tdata, sizeof(*tdata)))
- return -EFAULT;
-
- if (!access_ok(VERIFY_READ, udata, sizeof(*udata)))
- return -EFAULT;
-
- if (__copy_in_user(&tdata->read_write, &udata->read_write, 2 * sizeof(u8)))
- return -EFAULT;
- if (__copy_in_user(&tdata->size, &udata->size, 2 * sizeof(u32)))
- return -EFAULT;
- if (__get_user(datap, &udata->data) ||
- __put_user(compat_ptr(datap), &tdata->data))
- return -EFAULT;
-
- return sys_ioctl(fd, cmd, (unsigned long)tdata);
-}
-
#define RTC_IRQP_READ32 _IOR('p', 0x0b, compat_ulong_t)
#define RTC_IRQP_SET32 _IOW('p', 0x0c, compat_ulong_t)
#define RTC_EPOCH_READ32 _IOR('p', 0x0d, compat_ulong_t)
@@ -1341,13 +1236,6 @@ COMPATIBLE_IOCTL(USBDEVFS_SUBMITURB32)
COMPATIBLE_IOCTL(USBDEVFS_REAPURB32)
COMPATIBLE_IOCTL(USBDEVFS_REAPURBNDELAY32)
COMPATIBLE_IOCTL(USBDEVFS_CLEAR_HALT)
-/* i2c */
-COMPATIBLE_IOCTL(I2C_SLAVE)
-COMPATIBLE_IOCTL(I2C_SLAVE_FORCE)
-COMPATIBLE_IOCTL(I2C_TENBIT)
-COMPATIBLE_IOCTL(I2C_PEC)
-COMPATIBLE_IOCTL(I2C_RETRIES)
-COMPATIBLE_IOCTL(I2C_TIMEOUT)
/* hiddev */
COMPATIBLE_IOCTL(HIDIOCGVERSION)
COMPATIBLE_IOCTL(HIDIOCAPPLICATION)
@@ -1533,13 +1421,6 @@ static long do_ioctl_trans(int fd, unsigned int cmd,
return do_usbdevfs_bulk(fd, cmd, argp);
case USBDEVFS_DISCSIGNAL32:
return do_usbdevfs_discsignal(fd, cmd, argp);
- /* i2c */
- case I2C_FUNCS:
- return w_long(fd, cmd, argp);
- case I2C_RDWR:
- return do_i2c_rdwr_ioctl(fd, cmd, argp);
- case I2C_SMBUS:
- return do_i2c_smbus_ioctl(fd, cmd, argp);
/* Not implemented in the native kernel */
case RTC_IRQP_READ32:
case RTC_IRQP_SET32:
--
1.6.3.3
^ permalink raw reply related
* [uml-user] [PATCH 00/12] move compat_ioctl handling into drivers
From: Arnd Bergmann @ 2009-11-16 0:26 UTC (permalink / raw)
To: linux-kernel
Cc: linux-usb, user-mode-linux-user, Neil Brown, H. Peter Anvin,
Paul Clements, Andre Noll, Pavel Machek, linux-i2c, Jens Axboe,
Jan Blunck, Ian Kent, Nick Piggin, user-mode-linux-devel,
linux-scsi, Kay Sievers, Alon Bar-Lev, Alan Stern,
KOSAKI Motohiro, Doug Gilbert, David Vrabel, Alexey Dobriyan,
Arnd Bergmann, Jeff Dike, Oliver Neukum, linux-raid, Tejun
This is the second series of patches on compat_ioctl handling,
moving compat handlers into more drivers. Most of these
patches consist on a part that adds a compat_ioctl method
in one driver and another part removing the respective
section from fs/compat_ioctl.c. The fs/compat_ioctl.c
portion depends on other patches[1], but the driver code can
be applied independently.
If some of you feel responsible for the code I patched, please
look at my changes and if they are ok, either take them into
your tree or give me an Acked-by for them.
I have tested the patches for sg, usbdevfs and autofs, which
was easily done in qemu, but I'm lacking test cases for
most of the other ones.
Arnd <><
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Alon Bar-Lev <alon.barlev@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andre Noll <maan@systemlinux.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: autofs@linux.kernel.org
Cc: "Ben Dooks (embedded platforms)" <ben-linux@fluff.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: David Vrabel <david.vrabel@csr.com>
Cc: Doug Gilbert <dgilbert@interlog.com>
Cc: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ian Kent <raven@themaw.net>
Cc: James E.J. Bottomley <James.Bottomley@suse.de>
Cc: Jan Blunck <jblunck@suse.de>
Cc: "Jean Delvare (PC drivers, core)" <khali@linux-fr.org>
Cc: Jeff Dike <jdike@addtoit.com>
Cc: Jens Axboe <jens.axboe@oracle.com>
Cc: Kay Sievers <kay.sievers@vrfy.org>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: linux-i2c@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-raid@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Neil Brown <neilb@suse.de>
Cc: Nick Piggin <npiggin@suse.de>
Cc: Oliver Neukum <oliver@neukum.org>
Cc: Paul Clements <Paul.Clements@steeleye.com>
Cc: Pavel Machek <pavel@suse.cz>
Cc: Tejun Heo <tj@kernel.org>
Cc: user-mode-linux-devel@lists.sourceforge.net
Cc: user-mode-linux-user@lists.sourceforge.net
Cc: Wolfram Sang <w.sang@pengutronix.de>
Arnd Bergmann (12):
arch/um: handle compat_ioctl in tty line driver
scsi/sg: move compat_ioctl handling into sg driver
autofs/autofs4: move compat_ioctl handling into fs
raw: partly fix compat_ioctl handling on non-x86
nbd: add compat_ioctl method
smbfs: do compat_ioctl handling in place
i2cdev: move compat_ioctl handling into driver
md: move compat_ioctl handling into md.c
lp: move compat_ioctl handling into lp.c
usbdevfs: move compat_ioctl handling to devio.c
hamradio/mkiss: fix typo in compat_ioctl
compat_ioctl: remove unused handlers
arch/um/drivers/line.c | 6 +
arch/um/drivers/ssl.c | 1 +
arch/um/drivers/stdio_console.c | 1 +
arch/um/include/shared/line.h | 2 +
drivers/block/nbd.c | 7 +-
drivers/char/lp.c | 115 +++++++--
drivers/char/raw.c | 157 ++++++++++---
drivers/i2c/i2c-dev.c | 117 +++++++++
drivers/md/md.c | 23 ++
drivers/net/hamradio/mkiss.c | 2 +-
drivers/scsi/sg.c | 182 ++++++++++++++-
drivers/usb/core/devio.c | 110 ++++++++-
fs/autofs/root.c | 67 +++++-
fs/autofs4/root.c | 69 +++++-
fs/compat_ioctl.c | 504 +--------------------------------------
fs/smbfs/dir.c | 5 +-
fs/smbfs/file.c | 3 +-
fs/smbfs/ioctl.c | 48 ++++-
fs/smbfs/proto.h | 3 +-
include/linux/auto_fs.h | 1 +
include/linux/usbdevice_fs.h | 26 ++
kernel/trace/blktrace.c | 1 +
22 files changed, 856 insertions(+), 594 deletions(-)
[1] http://patchwork.kernel.org/bundle/arnd/compat-ioctl-cleanup/?state=*
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
^ permalink raw reply
* عزيزتي الفائز
From: مايكروسوفت اليانصيب @ 2009-11-15 20:21 UTC (permalink / raw)
عزيزتي الفائز
هذا هو أن أبلغكم أن البريد الإلكتروني الخاص بك قد حصل على جائزة من لمشاورة
وشركة مايكروسوفت البريد الإلكتروني نهاية العام يوجه الطفرة التي عقدت 10th من
نوفمبر 2009. ملكك
البريد الإلكتروني قد فاز كنت £ 500،000.00 (خمسمائة ألف جنيه بريطاني العظمى)
للحصول على جائزتك ، يرجى الاتصال بك وكيل معهد غودارد منظومة الائتمانية السيد
بيترسون والاتصال
معه عبر البريد الإلكتروني على الفور داخل 24hrs.
مع المعلومات الواردة أدناه.
معهد غودارد منظومة السيد بيترسون
مايكروسوفت تعزيز جائزة فريق
رئيس قسم المطالبات الفوز
البريد الالكتروني : ms.claims01@yahoo.com.hk
1.Full الاسم :..............
2.Occupation :..............
3.Age :..............
4.Sex :................
5.Nationality :...........
6.Tel :..............
7.Amount وون :.........................
خالص الشكر والامتنان لبيل غيتس وله associates.We
أتمنى لكم أفضل من luck.Thank لك لكونه جزءا من حياتنا نهاية العام الترويجية
جائزة برنامج وتوجه الذكرى التذكارية.
بصدق ،
Dr.Gary ستوفر
رئيس دائرة خدمة العملاء
مايكروسوفت تعزيز
^ permalink raw reply
* Re: [PATCH v2 21/22] i2c-designware: Tx abort cleanups
From: Ben Dooks @ 2009-11-14 22:22 UTC (permalink / raw)
To: Shinya Kuribayashi
Cc: baruch-NswTu9S1W3P6gbPvEgmw2w, ben-linux-elnMNo+KYs3YtjvyW6yDsg,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4AFB8F3A.6060208-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
On Thu, Nov 12, 2009 at 01:29:46PM +0900, Shinya Kuribayashi wrote:
> Baruch,
>
> Shinya Kuribayashi wrote:
> >* ABRT_MASTER_DIS: Fix a typo.
> >
> >* i2c_dw_handle_tx_abort: Return an appropriate error number
> > depending on abort_source.
> >
> >* i2c_dw_xfer: Add a missing abort_source initialization.
> >
> >Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
>
> [ ... ]
>
> >@@ -472,6 +490,24 @@ i2c_dw_read(struct dw_i2c_dev *dev)
> > }
> > }
> >+static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
> >+{
> >+ unsigned long abort_source = dev->abort_source;
> >+ int i;
> >+
> >+ for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
> >+ dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);
>
> Dev_err() might be annoying when we use sort of misc utilities
> such as i2c-tools. In case of no ACKs, we sometimes don't want
> to see error messages in the console, because such tools are
> sometimes capable of generating human-friendly console output
> like,
>
> root@localhost:/root> i2cdetect -y -r 2
> 0 1 2 3 4 5 6 7 8 9 a b c d e f
> 00: -- -- -- -- -- -- -- -- -- -- -- -- --
> 10: -- -- -- -- -- -- -- -- ...
>
> root@localhost:/root> i2cdump -y 0 0x49 b
> 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
> 00: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
> 10: XX XX XX XX XX XX ....
>
> Then how do I change dev_err() here? I'm not sure dev_dbg() or
> other variants are acceptable for no ACKs or arbitration cases.
> Could someone give me a comment? I'll prepare patches for it.
Maybe in the case of no-ack, this could be a debug message and erro
for the case where you have an arbitration problem or other error that
you wouldn't normally find on an I2C bus?
> Thanks in advance,
>
> >+ if (abort_source & DW_IC_TX_ARB_LOST)
> >+ return -EAGAIN;
> >+ else if (abort_source & DW_IC_TX_ABRT_NOACK)
> >+ return -EREMOTEIO;
> >+ else if (abort_source & DW_IC_TX_ABRT_GCALL_READ)
> >+ return -EINVAL; /* wrong msgs[] data */
> >+ else
> >+ return -EIO;
> >+}
> >+
> > /*
> > * Prepare controller for a transaction and call i2c_dw_xfer_msg
> > */
>
> --
> Shinya Kuribayashi
> NEC Electronics
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
^ permalink raw reply
* Re: yield() in i2c non-happy paths hits BUG under -rt patch
From: Jean Delvare @ 2009-11-14 18:02 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Sven-Thorsten Dietrich, Leon Woestenberg, linux-i2c, rt-users,
Ben Dooks (embedded platforms), Peter Zijlstra, LKML
In-Reply-To: <alpine.LFD.2.00.0911132139560.24119@localhost.localdomain>
Hi Thomas,
On Fri, 13 Nov 2009 23:03:39 +0100 (CET), Thomas Gleixner wrote:
> Jean,
>
> On Thu, 12 Nov 2009, Jean Delvare wrote:
> > As far as I can see, yield() doesn't have clear semantics attached.
> > It's more of a utility function everyone could use as they see fit. In
> > that respect, I understand your concerns about the coders' expectations
> > and how they could be dismissed by RT. OTOH, I don't think that asking
> > all developers to get rid of yield() because it _may not be_
> > RT-compliant will lead you anywhere.
> >
> > In the 3 occurrences I've looked at, yield() is used as a way to
> > busy-wait in a multitask-friendly way. What other use cases do you
> > expect? I've never seen yield() used as a way to avoid deadlocks, which
> > seems to be what you fear. I guess it could statistically be used that
> > way, but obviously I wouldn't recommend it. Anything else?
> >
> > I would recommend that you audit the various use cases of yield(), and
> > then offer replacements for the cases which are RT-unfriendly, leaving
> > the rest alone and possibly clarifying the intended use of yield() to
> > avoid future problems.
> >
> > In the i2c-algo-bit case, which started this thread, I can't really see
> > what "something more explicit of an implementation" would be. yield()
> > is as optimum as you can get, only delaying the processing if other
> > tasks want to run. A sleep or a delay would delay the processing
> > unconditionally, for an arbitrary amount of time, with no good reason.
> > Removing yield() would increase the latency. This particular feature of
> > i2c-algo-bit isn't necessarily terribly useful, but the code does the
> > right thing, regardless of RT, so I just can't see any reason to change
> > it.
>
> The problem with yield() is not RT specific. Let's just look at the
> yield semantics in mainline:
>
> From kernel/sched_fair.c
>
> unsigned int __read_mostly sysctl_sched_compat_yield;
> ...
> static void yield_task_fair(struct rq *rq)
> {
> if (likely(!sysctl_sched_compat_yield) && curr->policy != SCHED_BATCH) {
> ...
> return;
> }
> }
>
> So even in mainline yield() is doing nothing vs. latencies and is not
> multitask friendly by default. It's just not complaining about it.
I admit I have no clue what you're talking about. What I see is:
/**
* yield - yield the current processor to other threads.
*
* This is a shortcut for kernel-space yielding - it marks the
* thread runnable and calls sys_sched_yield().
*/
void __sched yield(void)
{
set_current_state(TASK_RUNNING);
sys_sched_yield();
}
I have no clue where sys_sched_yield is defined, but I trust the
comment as to what sched() is doing.
> yield itself is a design failure in almost all of its aspects. It's
> the poor mans replacement for proper async notification.
All the use cases in the i2c subsystem have nothing to do with async
notification.
> Q: Why put people yield() into their code ?
> A: Because:
> - it is less nasty than busy waiting for a long time
That's what I've seen, yes.
> - it works better
This is vague.
> - it solves the hang
In which case it is definitely a design failure, granted.
> - it happened to be in the driver they copied
> - ....
>
> At least 90% of the in kernel users of yield() are either a bug or a
> design problem or lack of understanding or all of those.
>
> I can see the idea of using yield() to let other tasks making progress
> in situations where the hardware is a design failure as well,
> e.g. bitbang devices. But if we have to deal with hardware which is
> crap by design yield() is the worst of all answers simply due to its
> horrible semantics.
>
> Let's look at the code in question:
>
> for (i = 0; i <= retries; i++) {
> ret = i2c_outb(i2c_adap, addr);
> if (ret == 1 || i == retries)
> break;
> bit_dbg(3, &i2c_adap->dev, "emitting stop condition\n");
> i2c_stop(adap);
> udelay(adap->udelay);
> yield();
>
> We are in the error path as we failed to communicate with the device
> in the first place. So there a two scenarios:
>
> 1) the device is essential for the boot process:
>
> you have hit a problem anyway
No, you have not. If you exhaust all the retries, then yes you have a
problem. But if the first attempt fails and the second works, then all
is fine. And this can happen. This is the whole point of the retry loop!
>
> 2) this is just some random device probing:
>
> who cares ?
"Who cares" about what? Me, I certainly care about this probing not
taking too much time, to not slow down the boot process for example
(framebuffer drivers do such probes over the DDC bus.)
On top of this, this may not be "random device probing" but a real
access to a known device, which is busy and thus doesn't ack its
address. This is permitted by I2C. A common case is I2C EEPROMs, which
are busy when writing a page, and need some time before they will ack
their address again. But it will happen.
> So in both cases the following change is a noop vs. latency and
> whatever your concerns are:
>
> - udelay(adap->udelay);
> - yield();
> + msleep(1);
This introduces up to 20 ms of delay (at HZ=100) for each retry.
"retries" being 3 by default, this means up to 60 ms total per
transaction. So you can't claim it is equivalent to the original code,
it simply is not, timing-wise.
Then again, this particular piece of code may go away someday, because I
see no reason why i2c-algo-bit would retry automatically in this
particular case, when other I2C adapter drivers do not. It would seem
better to let the caller determine how long to wait and/or how many
times to retry, depending on the target device.
But my initial point still holds: if you are unhappy about yield,
discuss it with Ingo, Peter, Linus or anyone else who cares, and change
it and/or delete it. Asking all driver authors/maintainers to stop
using this function but leaving it defined without a deprecation
warning won't lead you anywhere. Developers will add new calls faster
than you remove them.
--
Jean Delvare
^ permalink raw reply
* Re: yield() in i2c non-happy paths hits BUG under -rt patch
From: Thomas Gleixner @ 2009-11-13 22:03 UTC (permalink / raw)
To: Jean Delvare
Cc: Sven-Thorsten Dietrich, Leon Woestenberg, linux-i2c, rt-users,
Ben Dooks (embedded platforms), Peter Zijlstra, LKML
In-Reply-To: <20091112211255.09cd884a@hyperion.delvare>
Jean,
On Thu, 12 Nov 2009, Jean Delvare wrote:
> As far as I can see, yield() doesn't have clear semantics attached.
> It's more of a utility function everyone could use as they see fit. In
> that respect, I understand your concerns about the coders' expectations
> and how they could be dismissed by RT. OTOH, I don't think that asking
> all developers to get rid of yield() because it _may not be_
> RT-compliant will lead you anywhere.
>
> In the 3 occurrences I've looked at, yield() is used as a way to
> busy-wait in a multitask-friendly way. What other use cases do you
> expect? I've never seen yield() used as a way to avoid deadlocks, which
> seems to be what you fear. I guess it could statistically be used that
> way, but obviously I wouldn't recommend it. Anything else?
>
> I would recommend that you audit the various use cases of yield(), and
> then offer replacements for the cases which are RT-unfriendly, leaving
> the rest alone and possibly clarifying the intended use of yield() to
> avoid future problems.
>
> In the i2c-algo-bit case, which started this thread, I can't really see
> what "something more explicit of an implementation" would be. yield()
> is as optimum as you can get, only delaying the processing if other
> tasks want to run. A sleep or a delay would delay the processing
> unconditionally, for an arbitrary amount of time, with no good reason.
> Removing yield() would increase the latency. This particular feature of
> i2c-algo-bit isn't necessarily terribly useful, but the code does the
> right thing, regardless of RT, so I just can't see any reason to change
> it.
The problem with yield() is not RT specific. Let's just look at the
yield semantics in mainline:
>From kernel/sched_fair.c
unsigned int __read_mostly sysctl_sched_compat_yield;
...
static void yield_task_fair(struct rq *rq)
{
if (likely(!sysctl_sched_compat_yield) && curr->policy != SCHED_BATCH) {
...
return;
}
}
So even in mainline yield() is doing nothing vs. latencies and is not
multitask friendly by default. It's just not complaining about it.
yield itself is a design failure in almost all of its aspects. It's
the poor mans replacement for proper async notification.
Q: Why put people yield() into their code ?
A: Because:
- it is less nasty than busy waiting for a long time
- it works better
- it solves the hang
- it happened to be in the driver they copied
- ....
At least 90% of the in kernel users of yield() are either a bug or a
design problem or lack of understanding or all of those.
I can see the idea of using yield() to let other tasks making progress
in situations where the hardware is a design failure as well,
e.g. bitbang devices. But if we have to deal with hardware which is
crap by design yield() is the worst of all answers simply due to its
horrible semantics.
Let's look at the code in question:
for (i = 0; i <= retries; i++) {
ret = i2c_outb(i2c_adap, addr);
if (ret == 1 || i == retries)
break;
bit_dbg(3, &i2c_adap->dev, "emitting stop condition\n");
i2c_stop(adap);
udelay(adap->udelay);
yield();
We are in the error path as we failed to communicate with the device
in the first place. So there a two scenarios:
1) the device is essential for the boot process:
you have hit a problem anyway
2) this is just some random device probing:
who cares ?
So in both cases the following change is a noop vs. latency and
whatever your concerns are:
- udelay(adap->udelay);
- yield();
+ msleep(1);
Thanks,
tglx
^ permalink raw reply
* Re: yield() in i2c non-happy paths hits BUG under -rt patch
From: Jean Delvare @ 2009-11-12 20:12 UTC (permalink / raw)
To: Sven-Thorsten Dietrich
Cc: Leon Woestenberg, linux-i2c-u79uwXL29TY76Z2rM5mHXA, rt-users,
Ben Dooks (embedded platforms)
In-Reply-To: <4AF7148C.9090706-IsH+rWyeNGyzjR9+/8zPv5owlv4uC7bZ@public.gmane.org>
Hello Sven,
On Sun, 08 Nov 2009 10:57:16 -0800, Sven-Thorsten Dietrich wrote:
> On 11/07/2009 12:01 PM, Jean Delvare wrote:
> >
> > One thing I do not understand: if yield() is a bug to RT kernels, then
> > we would have to remove them all? But so far, yield() still exists in
> > the kernel tree, and it serves a purpose. Are you going to ask all
> > developers to remove all occurrences of yield() in their code? Doesn't
> > sound terribly realistic.
>
> The flaw in using yield() with RT priorities, is that it doesn't do what
> you expect.
You know, I did not really expect anything in the first place, given
how little I know about RT ;)
> The scheduler will run, and pick the highest-priority thread, which is
> the one yield()-ing.
Unless there are several real-time threads running, I presume?
> So the risk is, that whatever the yield() intended to do, it won't do,
> and worse, that you will end up endlessly yielding to yourself, locking
> the machine.
>
> So the call is for something more explicit of an implementation.
This seem to imply an affirmative answer to my initial question: your
plan is to get rid of the ~500 occurrences of yield() throughout the
kernel tree?
As far as I can see, yield() doesn't have clear semantics attached.
It's more of a utility function everyone could use as they see fit. In
that respect, I understand your concerns about the coders' expectations
and how they could be dismissed by RT. OTOH, I don't think that asking
all developers to get rid of yield() because it _may not be_
RT-compliant will lead you anywhere.
In the 3 occurrences I've looked at, yield() is used as a way to
busy-wait in a multitask-friendly way. What other use cases do you
expect? I've never seen yield() used as a way to avoid deadlocks, which
seems to be what you fear. I guess it could statistically be used that
way, but obviously I wouldn't recommend it. Anything else?
I would recommend that you audit the various use cases of yield(), and
then offer replacements for the cases which are RT-unfriendly, leaving
the rest alone and possibly clarifying the intended use of yield() to
avoid future problems.
In the i2c-algo-bit case, which started this thread, I can't really see
what "something more explicit of an implementation" would be. yield()
is as optimum as you can get, only delaying the processing if other
tasks want to run. A sleep or a delay would delay the processing
unconditionally, for an arbitrary amount of time, with no good reason.
Removing yield() would increase the latency. This particular feature of
i2c-algo-bit isn't necessarily terribly useful, but the code does the
right thing, regardless of RT, so I just can't see any reason to change
it.
--
Jean Delvare
^ permalink raw reply
* Re: [PATCH] I2C: OMAP3: PM: (re)init for every transfer to support off-mode
From: Kevin Hilman @ 2009-11-12 19:29 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <636c5030909301506p7da5f016m5148ce6cd279e704-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Sep 30, 2009 at 2:06 PM, Kevin Hilman
<khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org> wrote:
> On Mon, Aug 3, 2009 at 3:11 PM, Kevin Hilman
> <khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org> wrote:
>> Ben Dooks wrote:
>>>
>>> On Tue, Jul 21, 2009 at 04:09:03PM -0700, Kevin Hilman wrote:
>>>>
>>>> From: Rajendra Nayak <rnayak-l0cyMroinI0@public.gmane.org>
>>>>
>>>> Because of OMAP off-mode, powerdomain can go off when I2C is idle.
>>>> Save enough state, and do a re-init for each transfer.
>>>>
>>>> Additional save/restore state added by Jagadeesh Bhaskar Pakaravoor
>>>> (SYSC_REG) and Aaro Koskinen (wakeup sources.)
>>>>
>>>> Also, The OMAP3430 TRM states:
>>>>
>>>> "During active mode (I2Ci.I2C_CON[15] I2C_EN bit is set to 1), make no
>>>> changes to the I2Ci.I2C_SCLL and I2Ci.I2C_SCLH registers. Changes may
>>>> result in unpredictable behavior."
>>>>
>>>> Hence, the I2C_EN bit should be clearer when modifying these
>>>> registers. Please note that clearing the entire I2C_CON register to
>>>> disable the I2C module is safe, because the I2C_CON register is
>>>> re-configured for each transfer.
>>>
>>> should this be applied as a bugfix, or kept for next merge window?
>>
>> next merge window is fine.
>>
>
> Ben,
>
> It doesn't look like this made it in during the 2.6.32 merge window.
> Can you queue it for the -rc series?
>
ping
^ permalink raw reply
* Re: i2c-pnx driver issues
From: Ben Dooks @ 2009-11-12 9:34 UTC (permalink / raw)
To: Kevin Wells
Cc: Ben Dooks, vitalywool-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <083DF309106F364B939360100EC290F804F53CC645-SIPbe8o7cfX8DdpCu65jn8FrZmdRls4ZQQ4Iyu8u01E@public.gmane.org>
On Thu, Nov 12, 2009 at 12:37:36AM +0100, Kevin Wells wrote:
>
> Thanks Ben,
>
> I've submitted the changes as 4 individual patches. We also have some
> other enhancements for this driver coming up soon, but these 4 patches
> fix critical issues in the current pnx driver.
ok, will look at applying them and pushing before the weekend
--
Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
^ permalink raw reply
* Re: [PATCH v2 21/22] i2c-designware: Tx abort cleanups
From: Shinya Kuribayashi @ 2009-11-12 4:29 UTC (permalink / raw)
To: baruch-NswTu9S1W3P6gbPvEgmw2w, ben-linux-elnMNo+KYs3YtjvyW6yDsg,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4AF41BED.1050406-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
Baruch,
Shinya Kuribayashi wrote:
> * ABRT_MASTER_DIS: Fix a typo.
>
> * i2c_dw_handle_tx_abort: Return an appropriate error number
> depending on abort_source.
>
> * i2c_dw_xfer: Add a missing abort_source initialization.
>
> Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
[ ... ]
> @@ -472,6 +490,24 @@ i2c_dw_read(struct dw_i2c_dev *dev)
> }
> }
>
> +static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
> +{
> + unsigned long abort_source = dev->abort_source;
> + int i;
> +
> + for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
> + dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);
Dev_err() might be annoying when we use sort of misc utilities
such as i2c-tools. In case of no ACKs, we sometimes don't want
to see error messages in the console, because such tools are
sometimes capable of generating human-friendly console output
like,
root@localhost:/root> i2cdetect -y -r 2
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: -- -- -- -- -- -- -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- ...
root@localhost:/root> i2cdump -y 0 0x49 b
0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
00: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
10: XX XX XX XX XX XX ....
Then how do I change dev_err() here? I'm not sure dev_dbg() or
other variants are acceptable for no ACKs or arbitration cases.
Could someone give me a comment? I'll prepare patches for it.
Thanks in advance,
> + if (abort_source & DW_IC_TX_ARB_LOST)
> + return -EAGAIN;
> + else if (abort_source & DW_IC_TX_ABRT_NOACK)
> + return -EREMOTEIO;
> + else if (abort_source & DW_IC_TX_ABRT_GCALL_READ)
> + return -EINVAL; /* wrong msgs[] data */
> + else
> + return -EIO;
> +}
> +
> /*
> * Prepare controller for a transaction and call i2c_dw_xfer_msg
> */
--
Shinya Kuribayashi
NEC Electronics
^ permalink raw reply
* RE: i2c-pnx driver issues
From: Kevin Wells @ 2009-11-11 23:37 UTC (permalink / raw)
To: Ben Dooks
Cc: vitalywool-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20091111212152.GK13398-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org>
Thanks Ben,
I've submitted the changes as 4 individual patches. We also have some
other enhancements for this driver coming up soon, but these 4 patches
fix critical issues in the current pnx driver.
thanks,
Kevin Wells
NXP Semiconductors
> -----Original Message-----
> From: Ben Dooks [mailto:ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org]
> Sent: Wednesday, November 11, 2009 1:22 PM
> To: Kevin Wells
> Cc: vitalywool-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: i2c-pnx driver issues
>
> On Wed, Nov 11, 2009 at 01:21:45AM +0100, Kevin Wells wrote:
> > Sorry - I've resent with a more meaningful subject (than 'welcome')!
> >
> > Hi,
>
> gah, please word-wrap your lines to less than 77 characters per line,
> it makes it awfully difficult to read them on mailers in terminals.
>
> > We use the i2c-pnx.c driver on our device (NXP LPC3xxx devices), but have
> had some
> > issues with compilation and general use. I hope these changes can be of
> use and can
> > get back into the mainline. I'm happy to test any changes for the driver
> here on
> > our boards.
> >
> > The i2c-pnx.c file doesn't seem to compile (even with the pnx4008
> platform selected).
> > It looks like several important header files are not included in the
> driver
> > (mach/i2c.h and asm/io.h). We have another board that uses this same IP,
> but with a
> > slight reordering of the registers, so the i2c.h file in the mach area
> (which defines
> > register offsets) is important where it is now.
>
> asm/io.h should not be included directly, <linux/io.h> is the proper
> include file.
>
> > For systems with a tick rate of 100Hz, the I2C_PNX_TIMEOUT value of 10mS
> give only
> > 1 jiffie before the transfer times out. We've noticed on some transfers
> that the
> > jiffie may tick immediately after the expire count is set, causing the
> transfer in
> > progress to timeout before 10mS is up. A small test to make sure jiffies
> was at
> > least 2 fixed this.
> >
> > We have multiple I2C busses on our system. The bus id value in the
> platform definition
> > area (in the arm/mach- area) wasn't being correctly matched to the
> specific I2C
> > bus. This would cause some devices to not match to the correct I2C bus.
>
> That should be a ok to do as long as you are the primary i2c block
> on the systtem.
>
> > The 'buf' field in the I2C transfer descriptor was typed as a char. In
> the I2C
> > driver, data values in the buffer pointer to by buf were being sign
> extended into
> > the stop and start bit positions (8 and 9). For I2C transfers where a
> data byte had
> > bit 7 set, both the start and stop bits were also getting set in the
> hardware.
> >
> > The complete patch is listed below:
>
> any chance of having this patch in a form that could be easily
> merged? If you need more information on this, read the kernel
> documentation on the subject in Documentation/SubmittingPatches on how
> to format patches.
>
> even better, split the changes into individual bugfixes.
>
> > diff -Naur -X linux-2.6.32-rc6-ref/Documentation/dontdiff linux-2.6.32-
> rc6-ref/drivers/i2c/busses/i2c-pnx.c linux-2.6.32-
> rc6/drivers/i2c/busses/i2c-pnx.c
> > --- linux-2.6.32-rc6-ref/drivers/i2c/busses/i2c-pnx.c 2009-11-03
> 11:37:49.000000000 -0800
> > +++ linux-2.6.32-rc6/drivers/i2c/busses/i2c-pnx.c 2009-11-10
> 14:09:11.000000000 -0800
> > @@ -20,8 +20,10 @@
> > #include <linux/platform_device.h>
> > #include <linux/i2c-pnx.h>
> > #include <mach/hardware.h>
> > +#include <mach/i2c.h>
> > #include <asm/irq.h>
> > #include <asm/uaccess.h>
> > +#include <asm/io.h>
> >
> > #define I2C_PNX_TIMEOUT 10 /* msec */
> > #define I2C_PNX_SPEED_KHZ 100
> > @@ -54,6 +56,9 @@
> > struct timer_list *timer = &data->mif.timer;
> > int expires = I2C_PNX_TIMEOUT / (1000 / HZ);
> >
> > + if (expires <= 1)
> > + expires = 2;
> > +
> > del_timer_sync(timer);
> >
> > dev_dbg(&adap->dev, "Timer armed at %lu plus %u jiffies.\n",
>
> adding -p to the diff options makes life easier to work out what
> is being changed.
>
> > @@ -633,7 +638,8 @@
> >
> > /* Register this adapter with the I2C subsystem */
> > i2c_pnx->adapter->dev.parent = &pdev->dev;
> > - ret = i2c_add_adapter(i2c_pnx->adapter);
> > + i2c_pnx->adapter->nr = pdev->id;
> > + ret = i2c_add_numbered_adapter(i2c_pnx->adapter);
> > if (ret < 0) {
> > dev_err(&pdev->dev, "I2C: Failed to add bus\n");
> > goto out_irq;
> > diff -Naur -X linux-2.6.32-rc6-ref/Documentation/dontdiff linux-2.6.32-
> rc6-ref/include/linux/i2c-pnx.h linux-2.6.32-rc6/include/linux/i2c-pnx.h
> > --- linux-2.6.32-rc6-ref/include/linux/i2c-pnx.h 2009-11-03
> 11:37:49.000000000 -0800
> > +++ linux-2.6.32-rc6/include/linux/i2c-pnx.h 2009-11-10
> 14:16:55.000000000 -0800
> > @@ -21,7 +21,7 @@
> > int mode; /* Interface mode */
> > struct completion complete; /* I/O completion */
> > struct timer_list timer; /* Timeout */
> > - char * buf; /* Data buffer */
> > + u8 * buf; /* Data buffer */
> > int len; /* Length of data buffer */
> > };
> >
> > thanks,
> > Kevin Wells
> > NXP Semiconductors
>
> --
> Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)
>
> 'a smiley only costs 4 bytes'
^ permalink raw reply
* [PATCH 4/4] i2c : Map I2C adapter number to platform ID number
From: Kevin Wells @ 2009-11-11 23:34 UTC (permalink / raw)
To: Ben Dooks
Cc: vitalywool-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20091111212152.GK13398-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org>
Map I2C adapter number to platform ID number
Signed-off-by: Kevin Wells <kevin.wells-3arQi8VN3Tc@public.gmane.org>
---
i2c-pnx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- drivers/i2c/busses/i2c-pnx.c.orig 2009-11-11 14:54:30.000000000 -0800
+++ drivers/i2c/busses/i2c-pnx.c 2009-11-11 15:02:30.000000000 -0800
@@ -633,7 +633,8 @@ static int __devinit i2c_pnx_probe(struc
/* Register this adapter with the I2C subsystem */
i2c_pnx->adapter->dev.parent = &pdev->dev;
- ret = i2c_add_adapter(i2c_pnx->adapter);
+ i2c_pnx->adapter->nr = pdev->id;
+ ret = i2c_add_numbered_adapter(i2c_pnx->adapter);
if (ret < 0) {
dev_err(&pdev->dev, "I2C: Failed to add bus\n");
goto out_irq;
^ permalink raw reply
* [PATCH 3/4] i2c : Limit minimum jiffie timeout to 2
From: Kevin Wells @ 2009-11-11 23:28 UTC (permalink / raw)
To: Ben Dooks
Cc: vitalywool-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20091111212152.GK13398-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org>
Limit minimum jiffie timeout to 2 to prevent early timeout on systems
with low tick rates
Signed-off-by: Kevin Wells <kevin.wells-3arQi8VN3Tc@public.gmane.org>
---
i2c-pnx.c | 3 +++
1 file changed, 3 insertions(+)
--- drivers/i2c/busses/i2c-pnx.c.orig 2009-11-11 14:54:30.000000000 -0800
+++ drivers/i2c/busses/i2c-pnx.c 2009-11-11 15:00:17.000000000 -0800
@@ -54,6 +54,9 @@ static inline void i2c_pnx_arm_timer(str
struct timer_list *timer = &data->mif.timer;
int expires = I2C_PNX_TIMEOUT / (1000 / HZ);
+ if (expires <= 1)
+ expires = 2;
+
del_timer_sync(timer);
dev_dbg(&adap->dev, "Timer armed at %lu plus %u jiffies.\n",
^ permalink raw reply
* [PATCH 2/4] i2c : Added missing mach/i2c.h and linux/io.h header file includes
From: Kevin Wells @ 2009-11-11 23:25 UTC (permalink / raw)
To: Ben Dooks
Cc: vitalywool-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20091111212152.GK13398-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org>
Added missing mach/i2c.h and linux/io.h header file includes
Signed-off-by: Kevin Wells <kevin.wells-3arQi8VN3Tc@public.gmane.org>
---
i2c-pnx.c | 2 ++
1 file changed, 2 insertions(+)
--- drivers/i2c/busses/i2c-pnx.c.orig 2009-11-11 14:54:30.000000000 -0800
+++ drivers/i2c/busses/i2c-pnx.c 2009-11-11 14:55:14.000000000 -0800
@@ -19,7 +19,9 @@
#include <linux/completion.h>
#include <linux/platform_device.h>
#include <linux/i2c-pnx.h>
+#include <linux/io.h>
#include <mach/hardware.h>
+#include <mach/i2c.h>
#include <asm/irq.h>
#include <asm/uaccess.h>
^ permalink raw reply
* [PATCH 1/4] i2c : Made buf type unsigned to prevent sign extension
From: Kevin Wells @ 2009-11-11 23:23 UTC (permalink / raw)
To: Ben Dooks
Cc: vitalywool-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20091111212152.GK13398-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org>
Made buf type unsigned to prevent sign extension
Signed-off-by: Kevin Wells <kevin.wells-3arQi8VN3Tc@public.gmane.org>
---
i2c-pnx.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- include/linux/i2c-pnx.h.orig 2009-11-11 14:47:28.000000000 -0800
+++ include/linux/i2c-pnx.h 2009-11-10 14:16:55.000000000 -0800
@@ -21,7 +21,7 @@ struct i2c_pnx_mif {
int mode; /* Interface mode */
struct completion complete; /* I/O completion */
struct timer_list timer; /* Timeout */
- char * buf; /* Data buffer */
+ u8 * buf; /* Data buffer */
int len; /* Length of data buffer */
};
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox