Re: [PATCH] Omnikey Cardman 4000 driver
On Mon, Sep 05, 2005 at 12:44:37PM -0700, Nish Aravamudan wrote: > It looks like all callers of these functions pass in milliseconds? Any > chance you can get rid of these two and use msleep_interruptible() and > msleep() instead? As long as you are not using these functions around > wait-queues, you are ok (which I think is the case here). Ok, I've changed the driver accordingly and I'll repost after some more testing. -- - Harald Welte <[EMAIL PROTECTED]> http://gnumonks.org/ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6) pgpUMyh6qs3p1.pgp Description: PGP signature
Re: [PATCH] Omnikey Cardman 4000 driver
On Mon, Sep 05, 2005 at 12:44:37PM -0700, Nish Aravamudan wrote: It looks like all callers of these functions pass in milliseconds? Any chance you can get rid of these two and use msleep_interruptible() and msleep() instead? As long as you are not using these functions around wait-queues, you are ok (which I think is the case here). Ok, I've changed the driver accordingly and I'll repost after some more testing. -- - Harald Welte [EMAIL PROTECTED] http://gnumonks.org/ Privacy in residential applications is a desirable marketing option. (ETSI EN 300 175-7 Ch. A6) pgpUMyh6qs3p1.pgp Description: PGP signature
Re: [PATCH] Omnikey Cardman 4000 driver
On Mon, Sep 05, 2005 at 10:36:57PM +0200, Jesper Juhl wrote: > On 9/6/05, Harald Welte <[EMAIL PROTECTED]> wrote: > > Hi! > > > [snip] > > > > Please consider mergin mainline, thanks. > > > [snip] > > Wouldn't it be better to first merge it in -mm and get some wider > testing before pushing for mainline? From my past experience (I'm involved in writing smartcard reader drivers for some time now), users of smartcard readers tend to be "typical corporate users" who won't run anything but the distribution kernel. I really doubt that the drivers would get much more testing in -mm than they would in mainline. Also, what is the point of putting entirely new code (no changes to existing code!) into -mm? I understand that changes to existing code can inarguably benefit from some testing in -mm first. But new drivers? Where's the point? The devices are not supported in the existing kernel, so it cannot get worse by having a driver (even if it still contains one or the other bug). -- - Harald Welte <[EMAIL PROTECTED]> http://gnumonks.org/ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6) pgp3yXjcwvAf4.pgp Description: PGP signature
Re: [PATCH] Omnikey Cardman 4000 driver
On Mon, Sep 05, 2005 at 10:36:57PM +0200, Jesper Juhl wrote: On 9/6/05, Harald Welte [EMAIL PROTECTED] wrote: Hi! [snip] Please consider mergin mainline, thanks. [snip] Wouldn't it be better to first merge it in -mm and get some wider testing before pushing for mainline? From my past experience (I'm involved in writing smartcard reader drivers for some time now), users of smartcard readers tend to be typical corporate users who won't run anything but the distribution kernel. I really doubt that the drivers would get much more testing in -mm than they would in mainline. Also, what is the point of putting entirely new code (no changes to existing code!) into -mm? I understand that changes to existing code can inarguably benefit from some testing in -mm first. But new drivers? Where's the point? The devices are not supported in the existing kernel, so it cannot get worse by having a driver (even if it still contains one or the other bug). -- - Harald Welte [EMAIL PROTECTED] http://gnumonks.org/ Privacy in residential applications is a desirable marketing option. (ETSI EN 300 175-7 Ch. A6) pgp3yXjcwvAf4.pgp Description: PGP signature
Re: [PATCH] Omnikey Cardman 4000 driver
On 9/6/05, Harald Welte <[EMAIL PROTECTED]> wrote: > Hi! > [snip] > > Please consider mergin mainline, thanks. > [snip] Wouldn't it be better to first merge it in -mm and get some wider testing before pushing for mainline? -- Jesper Juhl <[EMAIL PROTECTED]> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Omnikey Cardman 4000 driver
On 9/6/05, Harald Welte <[EMAIL PROTECTED]> wrote: > Hi! > > Following-up to the Cardman 4040 driver, I'm now sumitting a driver for > the Cardman 4000 reader. It is, too, a PCMCIA smartcard reader and the > predecessor of the 4040. > > From a technical point of view, the two devices have nothing in common, > so there is no possibility of code sharing. > > Please consider mergin mainline, thanks. > > Note: The patch is incremental to the Cardman 4040 driver that has > already been submitted. > [snip] Please see my CodingStyle comments for your previous driver as well as the cleanup patch I just posted for that one. A lot of those issues apply to your new driver as well - care to clean that up? -- Jesper Juhl <[EMAIL PROTECTED]> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Omnikey Cardman 4000 driver
On 9/5/05, Harald Welte <[EMAIL PROTECTED]> wrote:
> Hi!
>
> Following-up to the Cardman 4040 driver, I'm now sumitting a driver for
> the Cardman 4000 reader. It is, too, a PCMCIA smartcard reader and the
> predecessor of the 4040.
>
> From a technical point of view, the two devices have nothing in common,
> so there is no possibility of code sharing.
> --- /dev/null
> +++ b/drivers/char/pcmcia/cm4000_cs.c
> +/* interruptible_pause() */
> +static inline void ipause(unsigned long amount)
> +{
> + current->state = TASK_INTERRUPTIBLE;
> + schedule_timeout(amount);
> +}
> +
> +/* uninterruptible_pause() */
> +static inline void upause(unsigned long amount)
> +{
> + current->state = TASK_UNINTERRUPTIBLE;
> + schedule_timeout(amount);
> +}
It looks like all callers of these functions pass in milliseconds? Any
chance you can get rid of these two and use msleep_interruptible() and
msleep() instead? As long as you are not using these functions around
wait-queues, you are ok (which I think is the case here). If you are
using wait-queues with these sleeps, then please use
schedule_timeout_interruptible() and
schedule_timeout_uninterruptible() from the -mm tree.
Thanks,
Nish
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Omnikey Cardman 4000 driver
On 9/5/05, Harald Welte [EMAIL PROTECTED] wrote:
Hi!
Following-up to the Cardman 4040 driver, I'm now sumitting a driver for
the Cardman 4000 reader. It is, too, a PCMCIA smartcard reader and the
predecessor of the 4040.
From a technical point of view, the two devices have nothing in common,
so there is no possibility of code sharing.
snip
--- /dev/null
+++ b/drivers/char/pcmcia/cm4000_cs.c
snip
+/* interruptible_pause() */
+static inline void ipause(unsigned long amount)
+{
+ current-state = TASK_INTERRUPTIBLE;
+ schedule_timeout(amount);
+}
+
+/* uninterruptible_pause() */
+static inline void upause(unsigned long amount)
+{
+ current-state = TASK_UNINTERRUPTIBLE;
+ schedule_timeout(amount);
+}
It looks like all callers of these functions pass in milliseconds? Any
chance you can get rid of these two and use msleep_interruptible() and
msleep() instead? As long as you are not using these functions around
wait-queues, you are ok (which I think is the case here). If you are
using wait-queues with these sleeps, then please use
schedule_timeout_interruptible() and
schedule_timeout_uninterruptible() from the -mm tree.
Thanks,
Nish
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Omnikey Cardman 4000 driver
On 9/6/05, Harald Welte [EMAIL PROTECTED] wrote: Hi! Following-up to the Cardman 4040 driver, I'm now sumitting a driver for the Cardman 4000 reader. It is, too, a PCMCIA smartcard reader and the predecessor of the 4040. From a technical point of view, the two devices have nothing in common, so there is no possibility of code sharing. Please consider mergin mainline, thanks. Note: The patch is incremental to the Cardman 4040 driver that has already been submitted. [snip] Please see my CodingStyle comments for your previous driver as well as the cleanup patch I just posted for that one. A lot of those issues apply to your new driver as well - care to clean that up? -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Omnikey Cardman 4000 driver
On 9/6/05, Harald Welte [EMAIL PROTECTED] wrote: Hi! [snip] Please consider mergin mainline, thanks. [snip] Wouldn't it be better to first merge it in -mm and get some wider testing before pushing for mainline? -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
