[patch 00/33] m68k patches for 2.6.22
Geert Uytterhoeven
Here are the m68k patches for 2.6.22. Please apply.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Christoph Hellwig
> From: Michael Schmitz <sch...@opal.biophys.uni-duesseldorf.de>
>
> SCSI should be working on a TT (but someone should really try!) but causes
> trouble on a Falcon (as in: it ate a filesystem of mine) at least when
> used concurrently with IDE. I have the notion it's because locking of the
> ST-DMA interrupt by IDE is broken in 2.6 (the IDE driver always complains
> about trying to release an already-released ST-DMA). Needs more work, but
> that's on the IDE or m68k interrupt side rather than SCSI.
> /*
> + * our own old-style timeout update
> + */
> +/*
> + * The strategy is to cause the timer code to call scsi_times_out()
> + * when the soonest timeout is pending.
> + * The arguments are used when we are queueing a new command, because
> + * we do not want to subtract the time used from this time, but when we
> + * set the timer, we want to take this value into account.
> + */
> +
> +int atari_scsi_update_timeout(Scsi_Cmnd * SCset, int timeout)
> +{
> + int rtn;
> +
> + /*
> + * We are using the new error handling code to actually register/deregister
> + * timers for timeout.
> + */
> +
> + if (!timer_pending(&SCset->eh_timeout)) {
> + rtn = 0;
> + } else {
> + rtn = SCset->eh_timeout.expires - jiffies;
> + }
> +
> + if (timeout == 0) {
> + del_timer(&SCset->eh_timeout);
> + SCset->eh_timeout.data = (unsigned long) NULL;
> + SCset->eh_timeout.expires = 0;
> + } else {
> + if (SCset->eh_timeout.data != (unsigned long) NULL)
> + del_timer(&SCset->eh_timeout);
> + SCset->eh_timeout.data = (unsigned long) SCset;
> + SCset->eh_timeout.expires = jiffies + timeout;
> + add_timer(&SCset->eh_timeout);
> + }
> + return rtn;
> +}
NACK, driver have no business messing around with eh_timeout.
> +/* former generic SCSI error handling stuff */
> +
> +#define SCSI_ABORT_SNOOZE 0
> +#define SCSI_ABORT_SUCCESS 1
> +#define SCSI_ABORT_PENDING 2
> +#define SCSI_ABORT_BUSY 3
> +#define SCSI_ABORT_NOT_RUNNING 4
> +#define SCSI_ABORT_ERROR 5
> +
> +#define SCSI_RESET_SNOOZE 0
> +#define SCSI_RESET_PUNT 1
> +#define SCSI_RESET_SUCCESS 2
> +#define SCSI_RESET_PENDING 3
> +#define SCSI_RESET_WAKEUP 4
> +#define SCSI_RESET_NOT_RUNNING 5
> +#define SCSI_RESET_ERROR 6
> +
> +#define SCSI_RESET_SYNCHRONOUS 0x01
> +#define SCSI_RESET_ASYNCHRONOUS 0x02
> +#define SCSI_RESET_SUGGEST_BUS_RESET 0x04
> +#define SCSI_RESET_SUGGEST_HOST_RESET 0x08
> +
> +#define SCSI_RESET_BUS_RESET 0x100
> +#define SCSI_RESET_HOST_RESET 0x200
> +#define SCSI_RESET_ACTION 0xff
Please don't use these anymore but rather kill all uses of them.
Also note that we'd be much happier if you killed the atari
specific NCR8350 and used the generic one instead.
Dmitry Torokhov
On 5/1/07, Geert Uytterhoeven <ge...@linux-m68k.org> wrote:
> +
> +/* These could be settable by some ioctl() in future... */
> +static unsigned int key_repeat_delay = DEFAULT_KEYB_REP_DELAY;
> +static unsigned int key_repeat_rate = DEFAULT_KEYB_REP_RATE;
> +
> +static unsigned char rep_scancode;
> +static struct timer_list atakeyb_rep_timer = {
> + .function = atakeyb_rep,
> +};
Is there a problem with repeat implementation in input core that
requres custom-made repeater here?
> +
> + // handle_scancode(scancode, !break_flag);
The code looks a bit dirty (old code commented out, C++ style comments).
> +
> +
> +int atari_kbdrate(struct kbd_repeat *k)
> +{
> + if (k->delay > 0) {
> + /* convert from msec to jiffies */
> + key_repeat_delay = (k->delay * HZ + 500) / 1000;
If this is really needed - msecs_to_jiffies().
>
> +config KEYBOARD_ATARI
> + tristate "Atari keyboard"
> + depends on ATARI
> + select ATARI_KBD_CORE
> + help
> + Say Y here if you are running Linux on any Atari and have a keyboard
> + attached.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called atakbd.
Can we spell it out: atarikbd or atari_kbd?
> +
> + if (!(atakbd_dev = input_allocate_device()))
> + return -ENOMEM;
> +
> + // need to init core driver if not already done so
> + if (atari_keyb_init())
Memory leak
> + return -ENODEV;
> +
> + atakbd_dev->name = "Atari Keyboard";
> + atakbd_dev->phys = "atakbd/input0";
> + atakbd_dev->id.bustype = BUS_ATARI;
> + atakbd_dev->id.vendor = 0x0001;
> + atakbd_dev->id.product = 0x0001;
> + atakbd_dev->id.version = 0x0100;
> +
> + atakbd_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REP);
> + atakbd_dev->keycode = atakbd_keycode;
> + atakbd_dev->keycodesize = sizeof(unsigned char);
> + atakbd_dev->keycodemax = ARRAY_SIZE(atakbd_keycode);
> +
> + for (i = 1; i < 0x72; i++) {
> + atakbd_keycode[i] = i;
> + set_bit(atakbd_keycode[i], atakbd_dev->keybit);
It looks like this driver is not using standard input event codes. If
Roman does not want to adjust keymaps on Amiga and Atari that should
be handled in legacy keyboard driver (drivers/char/keyboard.c). As it
is programs using /dev/input/eventX have no chance of working.
> + }
> +
> + input_register_device(atakbd_dev);
Error handling.
> +
> +static int mouse_threshold[2] = {2,2};
> +
> +#ifdef __MODULE__
> +MODULE_PARM(mouse_threshold, "2i");
MODULE_PARM is so 20th century...
> +#endif
> +#ifdef FIXED_ATARI_JOYSTICK
> +extern int atari_mouse_buttons;
> +#endif
> +static int atamouse_used = 0;
Initialization is not needed. Moreover the counter is not needed.
> +
> +static int atamouse_open(struct input_dev *dev)
> +{
> + if (atamouse_used++)
> + return 0;
No need to count, input core takes care of this.
> +
> +static int __init atamouse_init(void)
> +{
> + if (!MACH_IS_ATARI || !ATARIHW_PRESENT(ST_MFP))
> + return -ENODEV;
> +
> + if (!(atamouse_dev = input_allocate_device()))
> + return -ENOMEM;
> +
> + if (!(atari_keyb_init()))
Memory leak
> + return -ENODEV;
> +
> + atamouse_dev->name = "Atari mouse";
> + atamouse_dev->phys = "atamouse/input0";
> + atamouse_dev->id.bustype = BUS_ATARI;
> + atamouse_dev->id.vendor = 0x0001;
> + atamouse_dev->id.product = 0x0002;
> + atamouse_dev->id.version = 0x0100;
> +
> + atamouse_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REL);
> + atamouse_dev->relbit[0] = BIT(REL_X) | BIT(REL_Y);
> + atamouse_dev->keybit[LONG(BTN_LEFT)] = BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
> + atamouse_dev->open = atamouse_open;
> + atamouse_dev->close = atamouse_close;
> +
> + input_register_device(atamouse_dev);
Error handling.
> +
> + printk(KERN_INFO "input: %s at keyboard ACIA\n", atamouse_dev->name);
Input core already logs every input device registered, do we need to
repeat it in the driver?
--
Dmitry
Christoph Hellwig
irq code? The only architectures that don't have a conversion
in progress are m68k, m68knommu and arm26
Jeff Garzik
> From: Matthias Urlichs <sm...@smurf.noris.de>
>
> Macintosh CS89x0 Ethernet: Netif updates
> Addition of netif_stop_queue() before transmission by Michael Schmitz
> skb_copy_{from,to}_linear_data() conversion by Geert Uytterhoeven
>
> Signed-off-by: Geert Uytterhoeven <ge...@linux-m68k.org>
> ---
> drivers/net/Kconfig | 2 -
> drivers/net/mac89x0.c | 92 ++++++++++++++++++--------------------------------
> 2 files changed, 35 insertions(+), 59 deletions(-)
The ethernet patches I've seen so far look sane to me... ACK.
Since they were addressed to Linus and Andrew, I will presume that one
of those two penguins will apply your drivers/net/* patches.
Jeff
Christoph Hellwig
simpler if all the code was directly in atakeyb.c.
> +/*
> + * linux/atari/atakeyb.c
This is a good reason why filename comments are a really bad idea :)
> +/* state: 0: off; >0: in progress; >1: 0xf1 received */
> +static volatile int ikbd_self_test;
> +/* timestamp when last received a char */
> +static volatile unsigned long self_test_last_rcv;
Please don't use volatile variable in kernel code. While linux/m68k doesn't
support smp or preemptible kernels we should at least put in proper synchronization
at the API level.
> +/* bitmap of keys reported as broken */
> +static unsigned long broken_keys[128/(sizeof(unsigned long)*8)] = { 0, };
DECLARE_BITMAP()
> +typedef enum kb_state_t {
> + KEYBOARD, AMOUSE, RMOUSE, JOYSTICK, CLOCK, RESYNC
> +} KB_STATE_T;
> +
> +#define IS_SYNC_CODE(sc) ((sc) >= 0x04 && (sc) <= 0xfb)
> +
> +typedef struct keyboard_state {
> + unsigned char buf[6];
> + int len;
> + KB_STATE_T state;
> +} KEYBOARD_STATE;
Please kill the typedefs and shouting names.
> +#ifdef __MODULE__
> +MODULE_PARM(mouse_threshold, "2i");
> +#endif
__MODULE__ is never true and even if it was a MODULE_PARM wouldn't compile.
use an unconditional module_param instead.
> --- linux-m68k-2.6.21.orig/include/linux/input.h
> +++ linux-m68k-2.6.21/include/linux/input.h
> @@ -676,6 +676,7 @@ struct input_absinfo {
> #define BUS_I2C 0x18
> #define BUS_HOST 0x19
> #define BUS_GSC 0x1A
> +#define BUS_ATARI 0x1B
Is this really a separate bus? Should't we have a BUS_ONBOARD or so instead?
Dmitry Torokhov
>
> > --- linux-m68k-2.6.21.orig/include/linux/input.h
> > +++ linux-m68k-2.6.21/include/linux/input.h
> > @@ -676,6 +676,7 @@ struct input_absinfo {
> > #define BUS_I2C 0x18
> > #define BUS_HOST 0x19
> > #define BUS_GSC 0x1A
> > +#define BUS_ATARI 0x1B
>
> Is this really a separate bus? Should't we have a BUS_ONBOARD or so instead?
BUS_HOST shoudl fit the bill.
--
Dmitry
Christoph Hellwig
> +static int jazzsonic_open(struct net_device* dev) {
> + if (request_irq(dev->irq, &sonic_interrupt, IRQF_DISABLED, "sonic", dev)) {
> + printk(KERN_ERR "\n%s: unable to get IRQ %d.\n", dev->name, dev->irq);
> + return -EAGAIN;
> + }
> + return sonic_open(dev);
> +}
Opening brace for a function on a line of it's own please.
Also starting a printk with '\n' seems rather odd.
Antonino A. Daplas
> plain text document attachment (636-atafb.diff)
> From: Michael Schmitz <sch...@opal.biophys.uni-duesseldorf.de>
>
> Update the atari fb to 2.6 by Michael Schmitz,
Finally :-). That leaves only pm3fb that needs porting.
It still contains remnants of 2.4 code though, ie, FBCON_HAS_*, but
that's minor.
> Reformatting and rewrite of bit plane functions by Roman Zippel,
> A few more fixes by Geert Uytterhoeven.
>
> Signed-off-by: Michael Schmitz <sch...@debian.org>
> Signed-off-by: Roman Zippel <zip...@linux-m68k.org>
> Signed-off-by: Geert Uytterhoeven <ge...@linux-m68k.org>
Acked-by: Antonino Daplas <ada...@gmail.com>
Tony
Ville Syrjälä
> Recent cross-compilers are called m68k-linux-gnu-gcc instead of m68k-linux-gcc> arch/m68k/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-m68k-2.6.21.orig/arch/m68k/Makefile
> +++ linux-m68k-2.6.21/arch/m68k/Makefile
> @@ -21,7 +21,7 @@ AS += -m68020
> LDFLAGS := -m m68kelf
> ifneq ($(COMPILE_ARCH),$(ARCH))
> # prefix for cross-compiling binaries
> - CROSS_COMPILE = m68k-linux-
> + CROSS_COMPILE = m68k-linux-gnu-
> endif
>
> ifdef CONFIG_SUN3
What about using ?= so the user can override it. I sent a patch doing
just that but never got any response.
--
Ville Syrjälä
syr...@sci.fi
http://www.sci.fi/~syrjala/
Finn Thain
Install the built-in macsonic interrupt handler on both IRQs when using
via_alt_mapping. Otherwise the rare interrupt that still comes from the
nubus slot will wedge the nubus.
$ cat /proc/interrupts
auto 2: 89176 via2
auto 3: 744367 sonic
auto 4: 0 scc
auto 6: 318363 via1
auto 7: 0 NMI
mac 9: 119413 framebuffer vbl
mac 10: 1971 ADB
mac 14: 198517 timer
mac 17: 89104 nubus
mac 19: 72 Mac ESP SCSI
mac 56: 629 sonic
mac 62: 1142593 ide0
Version 1 of this patch had a bug where a nubus sonic card would register
two interrupt handlers. Only a built-in sonic needs both.
Versions 2 and 3 needed some cleanups, as Raylynn Knight and Christoph
Hellwig pointed out (thanks).
Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
drivers/net/jazzsonic.c | 23 +++++++++++++++++++----
drivers/net/macsonic.c | 46 ++++++++++++++++++++++++++++++++++++++++++----
drivers/net/sonic.c | 25 -------------------------
3 files changed, 61 insertions(+), 33 deletions(-)
Index: linux-2.6.21/drivers/net/jazzsonic.c
===================================================================
--- linux-2.6.21.orig/drivers/net/jazzsonic.c 2007-05-02 11:32:01.000000000 +1000
+++ linux-2.6.21/drivers/net/jazzsonic.c 2007-05-02 11:34:23.000000000 +1000
@@ -88,6 +88,23 @@ static unsigned short known_revisions[]
0xffff /* end of list */
};
+static int jazzsonic_open(struct net_device* dev)
+{
+ if (request_irq(dev->irq, &sonic_interrupt, IRQF_DISABLED, "sonic", dev)) {
+ printk(KERN_ERR "%s: unable to get IRQ %d.\n", dev->name, dev->irq);
+ return -EAGAIN;
+ }
+ return sonic_open(dev);
+}
+
+static int jazzsonic_close(struct net_device* dev)
+{
+ int err;
+ err = sonic_close(dev);
+ free_irq(dev->irq, dev);
+ return err;
+}
+
static int __init sonic_probe1(struct net_device *dev)
{
static unsigned version_printed;
@@ -169,8 +186,8 @@ static int __init sonic_probe1(struct ne
lp->rra_laddr = lp->rda_laddr + (SIZEOF_SONIC_RD * SONIC_NUM_RDS
* SONIC_BUS_SCALE(lp->dma_bitmode));
- dev->open = sonic_open;
- dev->stop = sonic_close;
+ dev->open = jazzsonic_open;
+ dev->stop = jazzsonic_close;
dev->hard_start_xmit = sonic_send_packet;
dev->get_stats = sonic_get_stats;
dev->set_multicast_list = &sonic_multicast_list;
@@ -260,8 +277,6 @@ MODULE_DESCRIPTION("Jazz SONIC ethernet
module_param(sonic_debug, int, 0);
MODULE_PARM_DESC(sonic_debug, "jazzsonic debug level (1-4)");
-#define SONIC_IRQ_FLAG IRQF_DISABLED
-
#include "sonic.c"
static int __devexit jazz_sonic_device_remove (struct platform_device *pdev)
Index: linux-2.6.21/drivers/net/macsonic.c
===================================================================
--- linux-2.6.21.orig/drivers/net/macsonic.c 2007-05-02 11:32:01.000000000 +1000
+++ linux-2.6.21/drivers/net/macsonic.c 2007-05-02 12:35:09.000000000 +1000
@@ -130,6 +130,46 @@ static inline void bit_reverse_addr(unsi
addr[i] = bitrev8(addr[i]);
}
+static irqreturn_t macsonic_interrupt(int irq, void *dev_id)
+{
+ irqreturn_t result;
+ unsigned long flags;
+
+ local_irq_save(flags);
+ result = sonic_interrupt(irq, dev_id);
+ local_irq_restore(flags);
+ return result;
+}
+
+static int macsonic_open(struct net_device* dev)
+{
+ if (request_irq(dev->irq, &sonic_interrupt, IRQ_FLG_FAST, "sonic", dev)) {
+ printk(KERN_ERR "%s: unable to get IRQ %d.\n", dev->name, dev->irq);
+ return -EAGAIN;
+ }
+ /* Under the A/UX interrupt scheme, the onboard SONIC interrupt comes
+ * in at priority level 3. However, we sometimes get the level 2 inter-
+ * rupt as well, which must prevent re-entrance of the sonic handler.
+ */
+ if (dev->irq == IRQ_AUTO_3)
+ if (request_irq(IRQ_NUBUS_9, &macsonic_interrupt, IRQ_FLG_FAST, "sonic", dev)) {
+ printk(KERN_ERR "%s: unable to get IRQ %d.\n", dev->name, IRQ_NUBUS_9);
+ free_irq(dev->irq, dev);
+ return -EAGAIN;
+ }
+ return sonic_open(dev);
+}
+
+static int macsonic_close(struct net_device* dev)
+{
+ int err;
+ err = sonic_close(dev);
+ free_irq(dev->irq, dev);
+ if (dev->irq == IRQ_AUTO_3)
+ free_irq(IRQ_NUBUS_9, dev);
+ return err;
+}
+
int __init macsonic_init(struct net_device* dev)
{
struct sonic_local* lp = netdev_priv(dev);
@@ -160,8 +200,8 @@ int __init macsonic_init(struct net_devi
lp->rra_laddr = lp->rda_laddr + (SIZEOF_SONIC_RD * SONIC_NUM_RDS
* SONIC_BUS_SCALE(lp->dma_bitmode));
- dev->open = sonic_open;
- dev->stop = sonic_close;
+ dev->open = macsonic_open;
+ dev->stop = macsonic_close;
dev->hard_start_xmit = sonic_send_packet;
dev->get_stats = sonic_get_stats;
dev->set_multicast_list = &sonic_multicast_list;
@@ -572,8 +612,6 @@ MODULE_DESCRIPTION("Macintosh SONIC ethe
module_param(sonic_debug, int, 0);
MODULE_PARM_DESC(sonic_debug, "macsonic debug level (1-4)");
-#define SONIC_IRQ_FLAG IRQ_FLG_FAST
-
#include "sonic.c"
static int __devexit mac_sonic_device_remove (struct platform_device *pdev)
Index: linux-2.6.21/drivers/net/sonic.c
===================================================================
--- linux-2.6.21.orig/drivers/net/sonic.c 2007-05-02 11:32:01.000000000 +1000
+++ linux-2.6.21/drivers/net/sonic.c 2007-05-02 11:32:02.000000000 +1000
@@ -50,29 +50,6 @@ static int sonic_open(struct net_device
if (sonic_debug > 2)
printk("sonic_open: initializing sonic driver.\n");
- /*
- * We don't need to deal with auto-irq stuff since we
- * hardwire the sonic interrupt.
- */
-/*
- * XXX Horrible work around: We install sonic_interrupt as fast interrupt.
- * This means that during execution of the handler interrupt are disabled
- * covering another bug otherwise corrupting data. This doesn't mean
- * this glue works ok under all situations.
- *
- * Note (dhd): this also appears to prevent lockups on the Macintrash
- * when more than one Ethernet card is installed (knock on wood)
- *
- * Note (fthain): whether the above is still true is anyones guess. Certainly
- * the buffer handling algorithms will not tolerate re-entrance without some
- * mutual exclusion added. Anyway, the memcpy has now been eliminated from the
- * rx code to make this a faster "fast interrupt".
- */
- if (request_irq(dev->irq, &sonic_interrupt, SONIC_IRQ_FLAG, "sonic", dev)) {
- printk(KERN_ERR "\n%s: unable to get IRQ %d .\n", dev->name, dev->irq);
- return -EAGAIN;
- }
-
for (i = 0; i < SONIC_NUM_RRS; i++) {
struct sk_buff *skb = dev_alloc_skb(SONIC_RBSIZE + 2);
if (skb == NULL) {
@@ -170,8 +147,6 @@ static int sonic_close(struct net_device
}
}
- free_irq(dev->irq, dev); /* release the IRQ */
-
return 0;
James Simmons
Will we see nubus ported over to the driver model soon :-)
James Simmons
> On Tue, 2007-05-01 at 22:32 +0200, Geert Uytterhoeven wrote:
> > plain text document attachment (636-atafb.diff)
> > From: Michael Schmitz <sch...@opal.biophys.uni-duesseldorf.de>
> >
> > Update the atari fb to 2.6 by Michael Schmitz,
>
> Finally :-). That leaves only pm3fb that needs porting.
>
> It still contains remnants of 2.4 code though, ie, FBCON_HAS_*, but
> that's minor.
Yeah!!! Its a good step forward. Needs more cleanup down the road.
> > Reformatting and rewrite of bit plane functions by Roman Zippel,
> > A few more fixes by Geert Uytterhoeven.
> >
> > Signed-off-by: Michael Schmitz <sch...@debian.org>
> > Signed-off-by: Roman Zippel <zip...@linux-m68k.org>
> > Signed-off-by: Geert Uytterhoeven <ge...@linux-m68k.org>
> Acked-by: Antonino Daplas <ada...@gmail.com>
Acked-by: James Simmons <jsim...@infradead.org>
Roman Zippel
On Tuesday 01 May 2007 22:49, Christoph Hellwig wrote:
> Btw, is there any chance you could update m68k to use the generic
> irq code? The only architectures that don't have a conversion
> in progress are m68k, m68knommu and arm26
I haven't looked seriously into it since it was pretty much rewritten.
What I need is very close control over the lowlevel handling to minimize the
number of indirect calls and to get the interrupt delivered as fast as
possible. (E.g. we even patch the assembler entry, so the basic flow control
is pretty much fixed after boot.)
I don't mind the if the general management is a bit more complex, but how do I
get rid of all the useless code?
At a bit closer look e.g. irq_desc would need a serious diet, it can produce a
pretty large table...
bye, Roman
Brad Boyer
> Will we see nubus ported over to the driver model soon :-)
I think it will happen eventually. However, my first priority
personally is to get the macio code working on non-PCI macs
to get the onboard stuff onto the driver model. I did look at
it, and NuBus does lend itself to this conversion due to the
fact that it does in general have devices that can be detected
at runtime and a lot of generic infrastructure. It's just a
matter of priorities and a lack of time all around.
Brad Boyer
fl...@allandria.com
Michael Schmitz
> >
> > Is this really a separate bus? Should't we have a BUS_ONBOARD or so instead?
>
> BUS_HOST shoudl fit the bill.
OK, I'll use that one.
Michael
Michael Schmitz
> >
> > Finally :-). That leaves only pm3fb that needs porting.
> >
> > It still contains remnants of 2.4 code though, ie, FBCON_HAS_*, but
> > that's minor.
>
> Yeah!!! Its a good step forward. Needs more cleanup down the road.
You bet. I'll kill the old cruft when I get to the pixmap support. My
keyboard and SCSI patches got trashed badly, so I'll need to work on those
first.
Michael
Michael Schmitz
> What's the reason for splitting this up? Things would be a quite a bit
> simpler if all the code was directly in atakeyb.c.
Simple: I assumed that keeping the input driver code with the other input
stuff was the Right Thing. I can move everything back to atakeyb.c if
that's preferred.
> > +/*
> > + * linux/atari/atakeyb.c
Shall I update that path, then? :-)
Honestly - no one had worked on the Atari code for years, and my number
one priority was to get it back into functional state. I'll keep your
suggestions in mind when merging the low-level and input parts.
Michael
Christoph Hellwig
> On Tue, 1 May 2007, Christoph Hellwig wrote:
>
> > What's the reason for splitting this up? Things would be a quite a bit
> > simpler if all the code was directly in atakeyb.c.
>
> Simple: I assumed that keeping the input driver code with the other input
> stuff was the Right Thing. I can move everything back to atakeyb.c if
> that's preferred.
I think keeping it all under arch/ makes more sense. drivers/input/
makes a lot of sense where we have keyboard controllers that are used
on various architectures with slightly different glue, or different
controllers for the same busses. IF you have really totally architecture
specific input devices that require intimate arch knowledge it probably
makes more sense to keep them there.
> > > +/*
> > > + * linux/atari/atakeyb.c
>
> Shall I update that path, then? :-)
Just remove it.
Michael Schmitz
> > > simpler if all the code was directly in atakeyb.c.
> >
> > Simple: I assumed that keeping the input driver code with the other input
> > stuff was the Right Thing. I can move everything back to atakeyb.c if
> > that's preferred.
>
> I think keeping it all under arch/ makes more sense. drivers/input/
> makes a lot of sense where we have keyboard controllers that are used
> on various architectures with slightly different glue, or different
> controllers for the same busses. IF you have really totally architecture
> specific input devices that require intimate arch knowledge it probably
> makes more sense to keep them there.
The keyboard controller hardware is used nowhere else, so
arch/m68k/atari/atakeyb.c it'll be.
Michael
Michael Schmitz
> > +static struct timer_list atakeyb_rep_timer = {
> > + .function = atakeyb_rep,
> > +};
>
> Is there a problem with repeat implementation in input core that
> requres custom-made repeater here?
Apparently not - the homebrew repeater has not been active pretty much
since I started to work on Atari in 2.6... time to kill that code.
> > +
> > + // handle_scancode(scancode, !break_flag);
>
> The code looks a bit dirty (old code commented out, C++ style comments).
You got that right.
> > +
> > +
> > +int atari_kbdrate(struct kbd_repeat *k)
> > +{
> > + if (k->delay > 0) {
> > + /* convert from msec to jiffies */
> > + key_repeat_delay = (k->delay * HZ + 500) / 1000;
>
> If this is really needed - msecs_to_jiffies().
That has to go as well (together with the repeater).
>
> >
> > +config KEYBOARD_ATARI
> > + tristate "Atari keyboard"
> > + depends on ATARI
> > + select ATARI_KBD_CORE
> > + help
> > + Say Y here if you are running Linux on any Atari and have a keyboard
> > + attached.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called atakbd.
>
> Can we spell it out: atarikbd or atari_kbd?
Could do that, yes. Christoph suggested to fold everythig into atakeyb.c
so modularization would go away here.
>
> > +
> > + if (!(atakbd_dev = input_allocate_device()))
> > + return -ENOMEM;
> > +
> > + // need to init core driver if not already done so
> > + if (atari_keyb_init())
>
> Memory leak
How so? If the core has been initialized already this will just return ...
> > + for (i = 1; i < 0x72; i++) {
> > + atakbd_keycode[i] = i;
> > + set_bit(atakbd_keycode[i], atakbd_dev->keybit);
>
> It looks like this driver is not using standard input event codes. If
> Roman does not want to adjust keymaps on Amiga and Atari that should
> be handled in legacy keyboard driver (drivers/char/keyboard.c). As it
> is programs using /dev/input/eventX have no chance of working.
The translation map should not have been overwritten like above, is that
what you mean?
My original patch didn't have that bit; scancodes were translated to
input keycodes using atakbd_keycode[scancode] instead. I'll have that
reverted...
> > + }
> > +
> > + input_register_device(atakbd_dev);
>
> Error handling.
Oops.
>
> > +
> > +static int mouse_threshold[2] = {2,2};
> > +
> > +#ifdef __MODULE__
> > +MODULE_PARM(mouse_threshold, "2i");
>
> MODULE_PARM is so 20th century...
Never tested, sorry.
> > +
> > +static int atamouse_open(struct input_dev *dev)
> > +{
> > + if (atamouse_used++)
> > + return 0;
>
> No need to count, input core takes care of this.
OK; I'll delete this.
> > +
> > + printk(KERN_INFO "input: %s at keyboard ACIA\n", atamouse_dev->name);
>
> Input core already logs every input device registered, do we need to
> repeat it in the driver?
Not really ... I'll merge atakeyb.c, atakbd.c and atamouse.c and send a
new patch to Geert.
Thanks,
Roman Zippel
On Thu, 3 May 2007, Michael Schmitz wrote:
> > > + for (i = 1; i < 0x72; i++) {
> > > + atakbd_keycode[i] = i;
> > > + set_bit(atakbd_keycode[i], atakbd_dev->keybit);
> >
> > It looks like this driver is not using standard input event codes.
Actually it does, it just sort of works because Atari generates mostly
AT keycodes.
> > If
> > Roman does not want to adjust keymaps on Amiga and Atari that should
> > be handled in legacy keyboard driver (drivers/char/keyboard.c). As it
> > is programs using /dev/input/eventX have no chance of working.
I explained already at a earlier occasion, why this "generic" keycode
thing is broken by design, which makes connecting multiple keyboards with
different mappings impossible.
bye, Roman
Dmitry Torokhov
On 5/3/07, Michael Schmitz <sch...@mail.biophys.uni-duesseldorf.de> wrote:
> >
> > > +
> > > + if (!(atakbd_dev = input_allocate_device()))
> > > + return -ENOMEM;
> > > +
> > > + // need to init core driver if not already done so
> > > + if (atari_keyb_init())
> >
> > Memory leak
>
> How so? If the core has been initialized already this will just return ...
>
You just allocated atakbd_dev. If atari_keyb_init() fails you leak it.
> > > + for (i = 1; i < 0x72; i++) {
> > > + atakbd_keycode[i] = i;
> > > + set_bit(atakbd_keycode[i], atakbd_dev->keybit);
> >
> > It looks like this driver is not using standard input event codes. If
> > Roman does not want to adjust keymaps on Amiga and Atari that should
> > be handled in legacy keyboard driver (drivers/char/keyboard.c). As it
> > is programs using /dev/input/eventX have no chance of working.
>
> The translation map should not have been overwritten like above, is that
> what you mean?
> My original patch didn't have that bit; scancodes were translated to
> input keycodes using atakbd_keycode[scancode] instead. I'll have that
> reverted...
>
Does KEY_1 actually maps to scancode 2 on atari?
Thanks,
--
Dmitry
Dmitry Torokhov
On 5/3/07, Roman Zippel <zip...@linux-m68k.org> wrote:
> Hi,
>
> On Thu, 3 May 2007, Michael Schmitz wrote:
>
> > > > + for (i = 1; i < 0x72; i++) {
> > > > + atakbd_keycode[i] = i;
> > > > + set_bit(atakbd_keycode[i], atakbd_dev->keybit);
> > >
> > > It looks like this driver is not using standard input event codes.
>
> Actually it does, it just sort of works because Atari generates mostly
> AT keycodes.
>
But not the input core codes, does it?
> > > If
> > > Roman does not want to adjust keymaps on Amiga and Atari that should
> > > be handled in legacy keyboard driver (drivers/char/keyboard.c). As it
> > > is programs using /dev/input/eventX have no chance of working.
>
> I explained already at a earlier occasion, why this "generic" keycode
> thing is broken by design, which makes connecting multiple keyboards with
> different mappings impossible.
>
No, I don't think so. Your points were:
1) You did not want to adjust your legacy keymap on Amiga
2) You want userspace programs to know how to program scancodes for
every type of keyboard and have different keymaps for different type
of keyboards (So you need to have n_kbd_types *
n_international_mappings keymaps).
Answering 1) I sent you once a patch for review that would have amikbd
send raw scancode events in additional to standard input events and
drivers/char/keyboard.c pick these raw events on amiga so both legacy
driver and evdev could work, but I never heard from you.
As far as 2) goes I think it is better to have unified keyboard map
across different types of keyboards and then overlay
internatinalization/other settings. And you still have per-keyboard
configurability as you can change scancode->keycode mapping on a
per-device basis via evdev ioctl.
--
Dmitry
Roman Zippel
On Thu, 3 May 2007, Dmitry Torokhov wrote:
> > I explained already at a earlier occasion, why this "generic" keycode
> > thing is broken by design, which makes connecting multiple keyboards with
> > different mappings impossible.
> >
>
> No, I don't think so. Your points were:
>
> 1) You did not want to adjust your legacy keymap on Amiga
Adjusting wouldn't be really a problem, if it had some value...
> 2) You want userspace programs to know how to program scancodes for
> every type of keyboard and have different keymaps for different type
> of keyboards (So you need to have n_kbd_types *
> n_international_mappings keymaps).
I never said that. Many keyboard _types_ need a separate key mapping.
Localization is a completely different problem (and could be solved via
separate localization tables).
Most of it can be solved in userspace and we wouldn't have to enumerate
every possible single key the kernel never cares about in <linux/input.h>.
> As far as 2) goes I think it is better to have unified keyboard map
> across different types of keyboards and then overlay
> internatinalization/other settings. And you still have per-keyboard
> configurability as you can change scancode->keycode mapping on a
> per-device basis via evdev ioctl.
You still completely ignore the problem of how said application should
properly support multiple keyboard mappings...
bye, Roman
Dmitry Torokhov
>
> On Thu, 3 May 2007, Dmitry Torokhov wrote:
>
> > > I explained already at a earlier occasion, why this "generic" keycode
> > > thing is broken by design, which makes connecting multiple keyboards with
> > > different mappings impossible.
> > >
> >
> > No, I don't think so. Your points were:
> >
> > 1) You did not want to adjust your legacy keymap on Amiga
>
> Adjusting wouldn't be really a problem, if it had some value...
>
> > 2) You want userspace programs to know how to program scancodes for
> > every type of keyboard and have different keymaps for different type
> > of keyboards (So you need to have n_kbd_types *
> > n_international_mappings keymaps).
>
> I never said that. Many keyboard _types_ need a separate key mapping.
> Localization is a completely different problem (and could be solved via
> separate localization tables).
> Most of it can be solved in userspace and we wouldn't have to enumerate
> every possible single key the kernel never cares about in <linux/input.h>.
>
I am not sure that solving it all in userspace is right solution.
Consider a sleep button. It can be hooked up via ACPI, located on AT
keyboard, USB keyboard or even on a remote control or some userspace
daemon getting data from the network. If kernel just passes raw data
to userspace then userspace programs need to know all these potential
sources and handle them separately. But with unified input device
interface userspace program only needs to monotor appearance of new
event devices, latch onto them and wait for EV_KEY/KEY_SLEEP. And it
is not much burden for the kernel because kernel already interfaces
with all these devices. In fact there probably savings because kernel
uses single interface layer.
> > As far as 2) goes I think it is better to have unified keyboard map
> > across different types of keyboards and then overlay
> > internatinalization/other settings. And you still have per-keyboard
> > configurability as you can change scancode->keycode mapping on a
> > per-device basis via evdev ioctl.
>
> You still completely ignore the problem of how said application should
> properly support multiple keyboard mappings...
>
I am afraid this statement is too vague, we need to discuss specific
scenarios...
Consider this scenario:
You use 2 PS/2 keyboards with your laptop - built-in and external one
on a separate serio port. Your built-in has media keys generating some
non-standard scancodes. The external one also has same media keys but
generating different set of keycodes. With unified input events you
can "fix" your keboards to generate same input events and the rest of
your stack does not care.
--
Dmitry
Roman Zippel
On Thu, 3 May 2007, Dmitry Torokhov wrote:
> > I never said that. Many keyboard _types_ need a separate key mapping.
> > Localization is a completely different problem (and could be solved via
> > separate localization tables).
> > Most of it can be solved in userspace and we wouldn't have to enumerate
> > every possible single key the kernel never cares about in <linux/input.h>.
> >
>
> I am not sure that solving it all in userspace is right solution.
What userspace are you talking about? There is one main user - X, for it
and all the rest you could provide a simple input library.
> Consider a sleep button. It can be hooked up via ACPI, located on AT
> keyboard, USB keyboard or even on a remote control or some userspace
> daemon getting data from the network. If kernel just passes raw data
> to userspace then userspace programs need to know all these potential
> sources and handle them separately. But with unified input device
> interface userspace program only needs to monotor appearance of new
> event devices, latch onto them and wait for EV_KEY/KEY_SLEEP. And it
> is not much burden for the kernel because kernel already interfaces
> with all these devices. In fact there probably savings because kernel
> uses single interface layer.
If the kernel would do it properly, you would have a point...
> > You still completely ignore the problem of how said application should
> > properly support multiple keyboard mappings...
> >
>
> I am afraid this statement is too vague, we need to discuss specific
> scenarios...
>
> Consider this scenario:
>
> You use 2 PS/2 keyboards with your laptop - built-in and external one
> on a separate serio port. Your built-in has media keys generating some
> non-standard scancodes. The external one also has same media keys but
> generating different set of keycodes. With unified input events you
> can "fix" your keboards to generate same input events and the rest of
> your stack does not care.
That's not the problem. Your keycode mapping has no idea about alternative
mappings. How does the application know that "Shift"+"2" may mean '"' or
'@'?
bye, Roman
Michael Schmitz
> > > > + if (atari_keyb_init())
> > >
> > > Memory leak
> >
> > How so? If the core has been initialized already this will just return ...
> >
>
> You just allocated atakbd_dev. If atari_keyb_init() fails you leak it.
I see. Won't probably happen, but there's no reason to take a risk.
> > > It looks like this driver is not using standard input event codes. If
> > > Roman does not want to adjust keymaps on Amiga and Atari that should
> > > be handled in legacy keyboard driver (drivers/char/keyboard.c). As it
> > > is programs using /dev/input/eventX have no chance of working.
> >
> > The translation map should not have been overwritten like above, is that
> > what you mean?
> > My original patch didn't have that bit; scancodes were translated to
> > input keycodes using atakbd_keycode[scancode] instead. I'll have that
> > reverted...
> >
>
> Does KEY_1 actually maps to scancode 2 on atari?
It sure does. For most of the keys, the identity mapping is in fact OK.
There's a lot of keys where this breaks, though. Plus the notorious
scancode 0 :-)
Michael
Geert Uytterhoeven
> On Tue, May 01, 2007 at 10:32:40PM +0200, Geert Uytterhoeven wrote:
> > Recent cross-compilers are called m68k-linux-gnu-gcc instead of m68k-linux-gcc
> >
> > Signed-off-by: Geert Uytterhoeven <ge...@linux-m68k.org>
> > ---
> > arch/m68k/Makefile | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > --- linux-m68k-2.6.21.orig/arch/m68k/Makefile
> > +++ linux-m68k-2.6.21/arch/m68k/Makefile
> > @@ -21,7 +21,7 @@ AS += -m68020
> > LDFLAGS := -m m68kelf
> > ifneq ($(COMPILE_ARCH),$(ARCH))
> > # prefix for cross-compiling binaries
> > - CROSS_COMPILE = m68k-linux-
> > + CROSS_COMPILE = m68k-linux-gnu-
> > endif
> >
> > ifdef CONFIG_SUN3
>
> What about using ?= so the user can override it. I sent a patch doing
> just that but never got any response.
I just gave it a try, and compilation failed because it used plain gcc
instead of m68k-linux-gnu-gcc.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Ville Syrjälä
> On Wed, 2 May 2007, Ville Syrj?l? wrote:
> > On Tue, May 01, 2007 at 10:32:40PM +0200, Geert Uytterhoeven wrote:
> > > Recent cross-compilers are called m68k-linux-gnu-gcc instead of m68k-linux-gcc
> > >
> > > Signed-off-by: Geert Uytterhoeven <ge...@linux-m68k.org>
> > > ---
> > > arch/m68k/Makefile | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > --- linux-m68k-2.6.21.orig/arch/m68k/Makefile
> > > +++ linux-m68k-2.6.21/arch/m68k/Makefile
> > > @@ -21,7 +21,7 @@ AS += -m68020
> > > LDFLAGS := -m m68kelf
> > > ifneq ($(COMPILE_ARCH),$(ARCH))
> > > # prefix for cross-compiling binaries
> > > - CROSS_COMPILE = m68k-linux-
> > > + CROSS_COMPILE = m68k-linux-gnu-
> > > endif
> > >
> > > ifdef CONFIG_SUN3
> >
> > What about using ?= so the user can override it. I sent a patch doing
> > just that but never got any response.
>
> I just gave it a try, and compilation failed because it used plain gcc
> instead of m68k-linux-gnu-gcc.
Argh. You're correct of course. Apparently I never tried it without
setting CROSS_COMPILE. The problem is in the top level Makefile:
CROSS_COMPILE ?=
and
export CROSS_COMPILE
If I remove those it works, but I don't know if removing them is a good
idea in general (especially the export). Maybe the ?= thing can be
removed and the export can be moved after including the arch Makefile?