Re: [patch] xpad: match xbox 360 devices with interface info (fwd)

2008-03-03 Thread Andrew Morton 
On Mon, 3 Mar 2008 17:57:08 +0100 (CET)
Jiri Kosina [EMAIL PROTECTED] wrote:

 While Dmitry is lost ... 
 
 -- 
 Jiri Kosina
 SUSE Labs
 
 
 -- Forwarded message --
 Date: Tue, 26 Feb 2008 02:44:24 +0200
 From: Anssi Hannula [EMAIL PROTECTED]
 To: Dmitry Torokhov [EMAIL PROTECTED]
 Cc:  linux-input@atrey.karlin.mff.cuni.cz, Jan Kratochvil [EMAIL 
 PROTECTED]
 Subject: [patch] xpad: match xbox 360 devices with interface info
 
 Match Xbox 360 controllers using the interface info, i.e. interface
 class 255 (Vendor specific), subclass 93 and protocol 1, instead of
 specifying the device ids individually. As the class is vendor-specific,
 we have to still match against vendor id as well, though.

this was hopelessly mangled: leading spaces removed, wordwrapped, etc.

Resend, please.

Re: [PATCH 3/3] Blackfin serial driver: this driver enable SPORTs on Blackfin emulate UART

2008-02-03 Thread Andrew Morton 

My review comments for this patch remain unaddressed so
I have put it on hold.

Re: [BUG] New Kernel Bugs

2007-11-13 Thread Andrew Morton 
On Tue, 13 Nov 2007 04:32:07 -0800 (PST) David Miller [EMAIL PROTECTED] wrote:

 From: Andrew Morton [EMAIL PROTECTED]
 Date: Tue, 13 Nov 2007 04:12:59 -0800
 
  On Tue, 13 Nov 2007 03:58:24 -0800 (PST) David Miller [EMAIL PROTECTED] 
  wrote:
  
   From: Andrew Morton [EMAIL PROTECTED]
   Date: Tue, 13 Nov 2007 03:49:16 -0800
   
Do you believe that our response to bug reports is adequate?
   
   Do you feel that making us feel and look like shit helps?
  
  That doesn't answer my question.
  
  See, first we need to work out whether we have a problem.  If we do this,
  then we can then have a think about what to do about it.
  
  I tried to convince the 2006 KS attendees that we have a problem and I
  resoundingly failed.  People seemed to think that we're doing OK.
  
  But it appears that data such as this contradicts that belief.
  
  This is not a minor matter.  If the kernel _is_ slowly deteriorating then
  this won't become readily apparent until it has been happening for a number
  of years.  By that stage there will be so much work to do to get us back to
  an acceptable level that it will take a huge effort.  And it will take a
  long time after that for the kerel to get its reputation back.
  
  So it is important that we catch deterioration *early* if it is happening.
 
 You tell me what I should spend my time working on, and I promise to
 do it OK? :-)

My suggestion: regressions.

If we're really active in chasing down the regressions then I think we can
be confident that the kernel isn't deteriorating.  Probably it will be
improving as we also fix some always-been-there bugs.

I think that we're fairly good about working the regressions in
Adrian/Michal/Rafael's lists but once Linus releases 2.6.x we tend to let
the unsolved ones slide, and we don't pay as much attention to the
regressions which 2.6.x testers report.

 For example, if I have a choice between a TCP crash just about anyone
 can hit and some obscure issue only reported with some device nearly
 nobody has, which one should I analyze and work on?
 
 That's the problem.  All of us prioritize and it means the chaff
 collects at the bottom.  You cannot fix that except by getting more
 bug fixers so that the chaff pile has a chance to get smaller.
 
 Luckily if the report being ignored isn't chaff, it will show up again
 (and again and again) and this triggers a reprioritization because not
 only is the bug no longer chaff, it also now got a lot of information
 tagged to it so it's a double worthwhile investment to work on the
 problem.
 
 I think a lot of bugs that aren't getting looked at are simply
 sitting in some early stage of this process.

Yes, that's a useful technique.  If multiple people are being hurt a lot by
a bug then that's a more important one to fix than the single-person
minor-irritant bug.

otoh that doesn't work very well with driver/platform bugs.  Often these
are regressions which only a single person can reproduce within the time
window which we have in which we can fix it.  If we don't fix it in that
window it'll go out to distros and presumably some more people will hit it.

So I don't see much alternative here to the traditional
work-with-the-originator way of resolving it.

git bisection should really help us with these regressions but it doesn't
appear that people are using as much as one would like.  I'm hoping that
the very good http://www.kernel.org/doc/local/git-quick.html will help us
out here.  Thanks to the mystery person who prepared that.

Re: [BUG] New Kernel Bugs

2007-11-13 Thread Andrew Morton 
On Tue, 13 Nov 2007 19:32:19 + Russell King [EMAIL PROTECTED] wrote:

 There's another issue I want to raise concerning bugzilla.  We have the
 classic case of not enough people reading bugzilla bugs - which is one
 of the biggest problems with bugzilla.  Virtually no one in the ARM
 community looks for ARM bugs in bugzilla.

Nor should they.

 Let's not forget that it would be a waste of time for people to manually
 check bugzilla for ARM bugs.  There's soo few people reporting ARM bugs
 into bugzilla that a weekly manual check by every maintainer would just
 return the same old boring results for months and months at a time.

I screen all bugzilla reports.  100% of them.

- I'll try to establish whether it is a regression

- I'll solicit any extra information which I believe the reveloper will need

- I'll ensure that an appropriate developer has seen the report

And yes, the number of arm-specific reports in there is very small.

 It would be far more productive if the ARM category was deleted from
 bugzilla and the few people who use bugzilla reported their bugs on the
 mailing list.  We've a couple of thousand people on the ARM kernel
 mailing list at the moment - that's 3 orders of magnitude more of eyes
 than look at bugzilla.

Is that [EMAIL PROTECTED]

If so, MANITAINERS claims that it is subscribers-only.  That would cause
some bug reporters to give up and go away.

Re: [BUG] New Kernel Bugs

2007-11-13 Thread Andrew Morton 
On Tue, 13 Nov 2007 22:18:01 + Russell King [EMAIL PROTECTED] wrote:

 On Tue, Nov 13, 2007 at 12:52:22PM -0800, Andrew Morton wrote:
  On Tue, 13 Nov 2007 19:32:19 + Russell King [EMAIL PROTECTED] wrote:
   There's another issue I want to raise concerning bugzilla.  We have the
   classic case of not enough people reading bugzilla bugs - which is one
   of the biggest problems with bugzilla.  Virtually no one in the ARM
   community looks for ARM bugs in bugzilla.
  
  Nor should they.
 
 So what you're saying is...
 
   Let's not forget that it would be a waste of time for people to manually
   check bugzilla for ARM bugs.  There's soo few people reporting ARM bugs
   into bugzilla that a weekly manual check by every maintainer would just
   return the same old boring results for months and months at a time.
  
  I screen all bugzilla reports.  100% of them.
  
  - I'll try to establish whether it is a regression
  
  - I'll solicit any extra information which I believe the reveloper will need
  
  - I'll ensure that an appropriate developer has seen the report
  
  And yes, the number of arm-specific reports in there is very small.
 
 that just because you do this everyone in a select clique, who you include
 me in, should be doing this as well.
 
 No.  Thank.  You.

No, I don't mean that at all and this was very plainly obviously from my very
clearly written email.  Let me try again.

No, no subsystem developer needs to monitor new bugzilla reports.  This is
because *I do it for them*.  I will actively make them aware of new reports
which I believe are legitimate and which contain sufficient information for
them to be able to take further action.

   It would be far more productive if the ARM category was deleted from
   bugzilla and the few people who use bugzilla reported their bugs on the
   mailing list.  We've a couple of thousand people on the ARM kernel
   mailing list at the moment - that's 3 orders of magnitude more of eyes
   than look at bugzilla.
  
  Is that [EMAIL PROTECTED]
 
 Yes.
 
  If so, MANITAINERS claims that it is subscribers-only.  That would cause
  some bug reporters to give up and go away.
 
 Find some other mailing list; I'm not hosting *nor* am I willing to run a
 non-subscribers only mailing list.  Period.  Not negotiable, so don't even
 try to change my mind.

Making a list subscribers-only will cause some bug reports to be lost.

Tradeoffs are involved, against which decisions must be made.  You have
made yours.

Re: [BUG] New Kernel Bugs

2007-11-13 Thread Andrew Morton 
On Tue, 13 Nov 2007 23:24:14 +0100 Jörn Engel [EMAIL PROTECTED] wrote:

 On Tue, 13 November 2007 13:56:58 -0800, Andrew Morton wrote:
  
  It's relatively common that a regression in subsystem A will manifest as a
  failure in subsystem B, and the report initially lands on the desk of the
  subsystem B developers.
  
  But that's OK.  The subsystem B people are the ones with the expertise to
  be able to work out where the bug resides and to help the subsystem A
  people understand what went wrong.
  
  Alas, sometimes the B people will just roll eyes and do nothing because
  they know the problem wasn't in their code.  Sometimes.
 
 And sometimes the A people will ignore the B people after the root cause
 has been worked out.  Do you have a good idea how to shame A into
 action?  Should I put you on Cc:?  Right now I'm in the eye-rolling
 phase.
 

Well, that's the problem, isn't it?

The best I can come up with is to suggest that all the info be captured in
a bugzilla report so that at least it doesn't get forgotten about.

I suppose that other options are

a) try to fix it yourself.  I'll take the patch and as long as we make a
   big enough mess of it, someone who knows what they're doing might fix it
   for real.

b) If it was a regression, identify the offending commit and we'll just
   revert it.

Re: [BUG] New Kernel Bugs

2007-11-13 Thread Andrew Morton 
On Tue, 13 Nov 2007 23:09:37 + Russell King [EMAIL PROTECTED] wrote:

 On Tue, Nov 13, 2007 at 02:32:01PM -0800, Andrew Morton wrote:
  On Tue, 13 Nov 2007 22:18:01 + Russell King [EMAIL PROTECTED] wrote:
   On Tue, Nov 13, 2007 at 12:52:22PM -0800, Andrew Morton wrote:
On Tue, 13 Nov 2007 19:32:19 + Russell King [EMAIL PROTECTED] 
wrote:
  No, I don't mean that at all and this was very plainly obviously from my 
  very
  clearly written email.  Let me try again.
 
 If you screen all bugzilla reports then you'll know that bug #9356 arrived
 at about 1400 GMT yesterday.  It's hardly surprising then that your utterly
 crappy responses to Natalie's message (which, incidentally, wasn't copied
 to me) sent within 24 hours of that report cause *great* annoyance.
 
  No, no subsystem developer needs to monitor new bugzilla reports.  This is
  because *I do it for them*.  I will actively make them aware of new reports
  which I believe are legitimate and which contain sufficient information for
  them to be able to take further action.
 
 On the whole you do an excellent job with feeding the bug reports to
 people, and while I recognise that you're only human, things do
 occasionally go wrong.  For instance, sending clearly marked Samsung
 S3C bugs to me rather than Ben Dooks (who's in MAINTAINERS for those
 platforms.)


Well whatever, sorry.  But this is in the noise floor.  Point is: many bug
reports aren't being attended to.


 It would be far more productive if the ARM category was deleted from
 bugzilla and the few people who use bugzilla reported their bugs on 
 the
 mailing list.  We've a couple of thousand people on the ARM kernel
 mailing list at the moment - that's 3 orders of magnitude more of eyes
 than look at bugzilla.

Is that [EMAIL PROTECTED]
   
   Yes.
   
If so, MANITAINERS claims that it is subscribers-only.  That would cause
some bug reporters to give up and go away.
   
   Find some other mailing list; I'm not hosting *nor* am I willing to run a
   non-subscribers only mailing list.  Period.  Not negotiable, so don't even
   try to change my mind.
  
  Making a list subscribers-only will cause some bug reports to be lost.
  
  Tradeoffs are involved, against which decisions must be made.  You have
  made yours.
 
 So how are they lost when they're held in a moderation queue and are
 either accepted, a useful response given to the original poster, or
 are forwarded to someone who can deal with the issue.
 
 I don't think subscribers only describes my lists - we don't devnull
 stuff just because the poster is not a subscriber.

Oh, OK, as long as there really is a human paying attention to those things
then that's fine.  When one is on the sending end of these things one never
knows how long it will take, not whether it will even happen.

Re: [BUG] New Kernel Bugs

2007-11-13 Thread Andrew Morton 
On Tue, 13 Nov 2007 17:11:36 -0800 Stephen Hemminger [EMAIL PROTECTED] wrote:

 On Tue, 13 Nov 2007 19:52:17 -0500
 Chuck Ebbert [EMAIL PROTECTED] wrote:
 
  On 11/13/2007 04:12 PM, Alan Cox wrote:
   Bug fixing is not about finding someone to blame, it's about getting the 
   bug fixed.
   
   Partly - its also about understanding why the bug occurred and making it
   not happen again.
  
  Very few people think about that part.
 
 Why does the kernel have very few useful tests?

Tests would of course be nice, but they aren't very useful(!)

Looking at this list which Natalie has generated I see around thirty which
are dependent on the right hardware and ten which are not.  This ratio is
typical, I think.  In fact I'd say that more than 75% of reported bugs are
dependent on hardware.

So the best test of all for the kernel is run it on a different machine. 
This is why we are so dependent upon our volunteer testers/reporters to
be able to do kernel development.

  Lack of interest? resources? expertise?
 Ideally each new feature would just be a small add on to an existing test.

Sure.  For system-call-visible features it would be good to do that.

But this tends not to be where bugs get exposed.  Because the original
developer can 100% exercise such code.  That isn't the case with
driver/arch/platform changes.

 Unlike developing new features which seems to grow well with more developers.
 Bug fixing also seems to be a scarcity process. There often seems to be
 a very few people that understand the problem well enough or have the 
 necessary
 hardware to reproduce and fix the problem.

We're 100% dead if having the hardware is a prerequisite to fixing a bug.
The terminal state there is that the kernel runs on about 200 machines
worldwide.  We have to work with reporters via email to fix these sorts of
things.  As we of course do.

 Recent changes like tickless and scheduler rework were well thought out and 
 caused
 very little impact to 90% of the users. The problem is the 10% who do have 
 problems.
 Worse, the developers often only hear about the a small sample of those.

Yes.  An unknown number of people just shrug and go back to an old kernel.

Re: [PATCH] Fujitsu application panel driver

2007-10-24 Thread Andrew Morton 
On Tue, 23 Oct 2007 12:55:55 -0700
Stephen Hemminger [EMAIL PROTECTED] wrote:

 This driver supports the application buttons on some Fujitsu Lifebook
 laptops. It is based on the earlier apanel driver done by Jochen
 Eisenger, but with many changes.  The original driver used ioctl's and
 a separate user space program (see http://apanel.sourceforge.net). This
 driver hooks into the input subsystem so that the normal keys act as
 expected without a daemon. In addition to buttons, the Mail Led is
 handled via LEDs class device.
 
 The driver now supports redefinable keymaps and no longer has to
 have a DMI table for all Fujitsu laptops.
 
 I thought about mixing this driver should be integrated into the Fujitsu 
 laptop
 extras driver that handles backlight, but rejected the idea because
 it wasn't clear if all the Fujitsu laptops supported both.
 
 ...

 +
 +/* Magic constants in BIOS that tell about buttons */
 +enum apanel_devid {
 + APANEL_DEV_NONE   = 0,
 + APANEL_DEV_APPBTN = 1,
 + APANEL_DEV_CDBTN  = 2,
 + APANEL_DEV_LCD= 3,
 + APANEL_DEV_LED= 4,
 + APANEL_DEV_MAX,
 +};

APANEL_DEV_MAX == 5.

 +enum apanel_chip {
 + CHIP_NONE= 0,
 + CHIP_OZ992C  = 1,
 + CHIP_OZ163T  = 2,
 + CHIP_OZ711M3 = 4,
 +};
 +
 +/* Result of BIOS snooping/probing -- what features are supported */
 +static enum apanel_chip device_chip[APANEL_DEV_MAX];
 +
 +/* names for APANEL_XXX */
 +static const char *device_names[APANEL_DEV_MAX] __initdata = {

We just wasted sizeof(char *)?

 + [APANEL_DEV_APPBTN] = Application Buttons,
 + [APANEL_DEV_LCD]= LCD,
 + [APANEL_DEV_LED]= LED,
 + [APANEL_DEV_CDBTN]  = CD Buttons,
 +};
 +
 +
 +/* Poll for key changes
 + *
 + * Read Application keys via SMI
 + *  A (0x4), B (0x8), Internet (0x2), Email (0x1).
 + *
 + * CD keys:
 + * Forward (0x100), Rewind (0x200), Stop (0x400), Pause (0x800)
 + */
 +static void apanel_poll(struct input_polled_dev *ipdev)
 +{
 + struct apanel *ap = ipdev-private;
 + struct input_dev *idev = ipdev-input;
 + u8 cmd = device_chip[APANEL_DEV_APPBTN] == CHIP_OZ992C ? 0 : 8;
 + s32 data;
 + int i;
 +
 + data = i2c_smbus_read_word_data(ap-client, cmd);
 + if (data  0)
 + return; /* ignore errors (due to ACPI??) */
 +
 + /* write back to clear latch */
 + i2c_smbus_write_word_data(ap-client, cmd, 0);
 +
 + if (!data)
 + return;
 +
 + dev_dbg(ipdev-input-dev, APANEL : data %#x\n, data);
 + for (i = 0; i  ipdev-input-keycodemax; i++)
 + if (1ul  i  data)

You made me google for the c precedence table.

 + report_key(idev, ap-keymap[i]);
 +}

 ...

Re: [PATCH 3/3] Blackfin serial driver: this driver enable SPORTs on Blackfin emulate UART

2007-10-15 Thread Andrew Morton 

 Subject: [PATCH 3/3] Blackfin serial driver: this driver enable SPORTs on 
 Blackfin emulate UART

That's a bit hard to parse.

On Thu, 11 Oct 2007 18:23:40 +0800
Bryan Wu [EMAIL PROTECTED] wrote:

 Signed-off-by: Bryan Wu [EMAIL PROTECTED]
 ---
  drivers/serial/Kconfig   |   43 +++
  drivers/serial/Makefile  |1 +
  drivers/serial/bfin_sport_uart.c |  614 
 ++
  drivers/serial/bfin_sport_uart.h |   63 
  include/linux/serial_core.h  |2 +
  5 files changed, 723 insertions(+), 0 deletions(-)
  create mode 100644 drivers/serial/bfin_sport_uart.c
  create mode 100644 drivers/serial/bfin_sport_uart.h
 
 diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
 index 81b52b7..f3bfbe1 100644
 --- a/drivers/serial/Kconfig
 +++ b/drivers/serial/Kconfig
 @@ -1259,4 +1259,47 @@ config SERIAL_OF_PLATFORM
 Currently, only 8250 compatible ports are supported, but
 others can easily be added.
  
 +config SERIAL_BFIN_SPORT
 + tristate Blackfin SPORT emulate UART (EXPERIMENTAL)
 + depends on BFIN  EXPERIMENTAL
 + select SERIAL_CORE
 + help
 +   Enble support SPORT emulate UART on Blackfin series.

There's a typo there.  Also the text is pretty meaningless - I'd fix it up
but I'm not sure what it's trying to tell us!

 +//#define DEBUG

Probably this shouldn't be here at all - DEBUG is passed in from the gcc
command line.

 +#include linux/module.h
 +#include linux/ioport.h
 +#include linux/init.h
 +#include linux/console.h
 +#include linux/sysrq.h
 +#include linux/platform_device.h
 +#include linux/tty.h
 +#include linux/tty_flip.h
 +#include linux/serial_core.h
 +
 +#include asm/delay.h
 +#include asm/portmux.h
 +
 +#include bfin_sport_uart.h
 +
 +unsigned short bfin_uart_pin_req_sport0[] =
 + {P_SPORT0_TFS, P_SPORT0_DTPRI, P_SPORT0_TSCLK, P_SPORT0_RFS, \
 +  P_SPORT0_DRPRI, P_SPORT0_RSCLK, P_SPORT0_DRSEC, P_SPORT0_DTSEC, 0};
 +
 +unsigned short bfin_uart_pin_req_sport1[] =
 + {P_SPORT1_TFS, P_SPORT1_DTPRI, P_SPORT1_TSCLK, P_SPORT1_RFS, \
 + P_SPORT1_DRPRI, P_SPORT1_RSCLK, P_SPORT1_DRSEC, P_SPORT1_DTSEC, 0};

I don't think these need global scope?

 ...

 +/* Reqeust IRQ, Setup clock */
 +static int sport_startup(struct uart_port *port)
 +{
 + struct sport_uart_port *up = (struct sport_uart_port *)port;
 + char buffer[20];
 + int retval;
 +
 + pr_debug(%s enter\n, __FUNCTION__);
 + memset(buffer, 20, '\0');

I don't think this memset is needed.

 + snprintf(buffer, 20, %s rx, up-name);
 + retval = request_irq(up-rx_irq, sport_uart_rx_irq, IRQF_SAMPLE_RANDOM, 
 buffer, up);
 + if (retval) {
 + printk(KERN_ERR Unable to request interrupt %s\n, buffer);
 + return retval;
 + }
 +
 + snprintf(buffer, 20, %s tx, up-name);
 + retval = request_irq(up-tx_irq, sport_uart_tx_irq, IRQF_SAMPLE_RANDOM, 
 buffer, up);
 + if (retval) {
 + printk(KERN_ERR Unable to request interrupt %s\n, buffer);
 + goto fail1;
 + }
 +
 + snprintf(buffer, 20, %s err, up-name);
 + retval = request_irq(up-err_irq, sport_uart_err_irq, 
 IRQF_SAMPLE_RANDOM, buffer, up);
 + if (retval) {
 + printk(KERN_ERR Unable to request interrupt %s\n, buffer);
 + goto fail2;
 + }

It is not a good idea to create files in /proc which have spaces in their
names.  Yes, userspace _should_ be able to cope with that in all cases, but
all software sucks, even including userspace ;)

I'd suggest that we be defensive here, and avoid using spaces in filenames.

 + if (port-line) {
 + if (peripheral_request_list(bfin_uart_pin_req_sport1, DRV_NAME))
 + goto fail3;
 + } else {
 + if (peripheral_request_list(bfin_uart_pin_req_sport0, DRV_NAME))
 + goto fail3;
 + }
 +
 + sport_uart_setup(up, get_sclk(), port-uartclk);
 +
 + /* Enable receive interrupt */
 + SPORT_PUT_RCR1(up, (SPORT_GET_RCR1(up) | RSPEN));
 + SSYNC();
 +
 + return 0;
 +
 +
 +fail3:
 + printk(KERN_ERR DRV_NAME
 + : Requesting Peripherals failed\n);

s/P/p/

 + free_irq(up-err_irq, up);
 +fail2:
 + free_irq(up-tx_irq, up);
 +fail1:
 + free_irq(up-rx_irq, up);
 +
 + return retval;
 +
 +}

 ...

 +static void sport_shutdown(struct uart_port *port)
 +{
 + struct sport_uart_port *up = (struct sport_uart_port *)port;

That's a bit ugly.  Perhaps using container_of() would be clearer.  it
would also remove the requirement that uart_port be the first member of
sport_uart_port.

 + pr_debug(%s enter\n, __FUNCTION__);
 +
 + /* Disable sport */
 + SPORT_PUT_TCR1(up, (SPORT_GET_TCR1(up)  ~TSPEN));
 + SPORT_PUT_RCR1(up, (SPORT_GET_RCR1(up)  ~RSPEN));
 + SSYNC();
 +
 + if (port-line) {
 + peripheral_free_list(bfin_uart_pin_req_sport1);
 + } else {
 +

Re: [PATCH 3/3] Blackfin serial driver: this driver enable SPORTs on Blackfin emulate UART

2007-10-15 Thread Andrew Morton 
On Mon, 15 Oct 2007 18:03:35 -0400
Mike Frysinger [EMAIL PROTECTED] wrote:

  It is not a good idea to create files in /proc which have spaces in their
  names.  Yes, userspace _should_ be able to cope with that in all cases, but
  all software sucks, even including userspace ;)
 
  I'd suggest that we be defensive here, and avoid using spaces in filenames.
 
 i'm not sure i follow ... these are the names given to request_irq()
 which means this is what shows up in /proc/interrupts ... does this
 function also create an actual file somewhere in /proc that i am not
 aware of ?

err, umm, yeah.  But the same argument applies: it is imprudent to have
space-containing records in /proc/interrupts.

However it seems that we've already done that in several places so I guess
any /proc/interrupts-parsing programs are already coping with it OK.

Re: sysfs change of input/event devices in 2.6.23rc breaks udev

2007-09-15 Thread Andrew Morton 
On Mon, 10 Sep 2007 09:24:04 -0400 Dmitry Torokhov [EMAIL PROTECTED] wrote:

 On 9/10/07, Greg KH [EMAIL PROTECTED] wrote:
  On Mon, Sep 10, 2007 at 01:28:47AM -0400, Dmitry Torokhov wrote:
   On Sunday 09 September 2007 19:03, Kay Sievers wrote:
On 9/8/07, Anssi Hannula [EMAIL PROTECTED] wrote:

 However, the change that broke id_path of udev is that
 /sys/class/input/event5/device is now a symlink to the inputX 
 directory
 instead of being the same as the device symlink in inputX directory,
 i.e. to ../../../devices/platform/pcspkr in this case.

 Udev id_path uses that directory to construct the ID_PATH variable.
 Should the sysfs structure be reverted or should udev be adapted to
 handle traversing /device symlink twice? I think the former, as there
 should be considerably more time to adapt udev for coming changes in 
 sysfs.
   
Udev's path_id script is too dumb to follow the device link of
stacked class devices in the CONFIG_SYSFS_DEPRECATED=y layout. Does
this change fix it for you?
  
http://git.kernel.org/?p=linux/hotplug/udev.git;a=commitdiff_plain;h=b1ac36ff5e3756cefc79967a26280056da31bf6f
   
  
   Hmm, fixing udev is good but users will not get the change in time. I 
   think we
   need to adjust SYSFS_DEPRECATED code to produce old results. Something 
   like the
   patch below. I wonder what Greg would think...
 
  Hm, I don't understand.  Didn't the original conversion of the input
  layer by Kay not have this kind of problem?  What did your changes do
  differently to cause this driver core change to be needed?
 
 
 If I understand it correctly Kay's convesion had the same issue. With
 class devices device link points to class_dev-device instead of
 class_dev-parent. If you want to keep compatibility with old sysfs
 layout when moving from class devices to regular devices then you need
 to skip couple of parents till you get to real device. This only
 matters for input because this was the only subsystem with class
 devices stacked.
 

wonders where the rest of this thread went to

Did this userspace-visible post-2.6.22 regression get fixed?

Re: [PATCH]: Add command-line option to i8042 to completely disable it

2007-07-25 Thread Andrew Morton 
On Mon, 23 Jul 2007 13:05:22 -0400 Chris Lalancette [EMAIL PROTECTED] wrote:

 (I tried to send this patch to linux-input@, but it seems to be currently 
 having
 some problems, so I'm going directly to LKML).
 
 Certain (broken) pieces of South Bridge hardware will respond to
 i8042_read_status() on boot with 0x0, despite there not being a real i8042
 controller hooked up in the south bridge.  This can cause the detection for 
 the
 i8042 to return a phantom device, which hangs up later initialization.  Note
 that using i8042.nokbd and/or i8042.noaux do not help with this, since 
 this
 shows up during i8042_controller_check() (before either of those options are
 checked).  This patch adds a command-line option i8042.disable, which just
 completely disables any checking for the i8042 controller.

That's an unfortunate fix.  Is there really no way in which we
can auto-detect such a situation without requiring a manual
setting?

Re: [RFC 2/2] Input: phantom, add a new driver

2007-04-20 Thread Andrew Morton 
On Tue, 17 Apr 2007 22:02:10 +0200 (CEST) Jiri Slaby [EMAIL PROTECTED] wrote:

 phantom, add a new driver
 
 Sensable Phantom is a up to 7DOF force feedback (up to 6DOF FF) device. It's
 atypical, so it's based on the new added FF_RAW effect.
 
 diff --git a/drivers/input/misc/phantom.c b/drivers/input/misc/phantom.c
 new file mode 100644
 index 000..58f55cd
 --- /dev/null
 +++ b/drivers/input/misc/phantom.c
 @@ -0,0 +1,387 @@
 +/*
 + *  Copyright (C) 2005-2007 Jiri Slaby [EMAIL PROTECTED]
 + *
 + *  This program is free software; you can redistribute it and/or modify
 + *  it under the terms of the GNU General Public License as published by
 + *  the Free Software Foundation; either version 2 of the License, or
 + *  (at your option) any later version.
 + *
 + *  You need an userspace library to cooperate with this driver. It (and 
 other
 + *  info) may be obtained here:
 + *  http://www.fi.muni.cz/~xslaby/phantom.html
 + */
 +
 +#include linux/kernel.h
 +#include linux/module.h
 +#include linux/device.h
 +#include linux/pci.h
 +#include linux/fs.h
 +#include linux/poll.h
 +#include linux/interrupt.h
 +#include linux/input.h
 +
 +#include asm/io.h
 +
 +#define PHANTOM_VERSION  n0.9.4

That's an impressive version number ;)

 +#define PHM_MAX_TORQUES  3
 +
 +#define PHN_CONTROL  0x6
 +#define PHN_CTL_AMP  0x1
 +#define PHN_CTL_BUT  0x2
 +#define PHN_CTL_IRQ  0x10
 +
 +#define PHN_IRQCTL   0x4c
 +
 +#define PHN_ZERO_FORCE   2048

wonders what all those do

 +#define PCI_ENCODER(dev, axis) ((0 - (int)ioread32((dev)-iaddr + (axis)))  
 \
 + 0x)

Is there any reason why this cannot be a lower-cased inline C function? 
Nicer to read, typesafe, etc.

 +#define PHB_RUNNING  1
 +#define PHB_RESET2
 +
 +static struct PH_CLASSTYPE *phantom_class;

I guess that PH_CLASSTYPE is some protect-me-from-gregkh compatibility
thing.  But there isn't such a macro in the tree.  I switched this to plain
old `class'.

 +struct phantom_device {
 + void __iomem *caddr;
 + u32 __iomem *iaddr;
 + u32 __iomem *oaddr;
 + u32 amp_bit;
 + s16 torques[PHM_MAX_TORQUES];
 + unsigned long status;
 +
 + struct input_dev *idev;
 +};
 +
 +static int phantom_status(struct phantom_device *dev, unsigned long newstat)
 +{
 + pr_debug(phantom_status %lx %lx\n, dev-status, newstat);
 +
 + if (!(dev-status  PHB_RUNNING)  (newstat  PHB_RUNNING)) {
 + iowrite32(PHN_CTL_IRQ | PHN_CTL_AMP, dev-iaddr + PHN_CONTROL);
 + dev-amp_bit = PHN_CTL_IRQ;
 + iowrite32(0x43, dev-caddr + PHN_IRQCTL);
 + } else if ((dev-status  PHB_RUNNING)  !(newstat  PHB_RUNNING))
 + iowrite32(0, dev-caddr + PHN_IRQCTL);
 +
 + dev-status = newstat;
 +
 + return 0;
 +}
 +
 +static void phantom_close(struct input_dev *idev)
 +{
 + struct phantom_device *dev = idev-private;
 +
 + phantom_status(dev, dev-status  ~PHB_RUNNING);
 +}
 +
 +static void phantom_reset(struct phantom_device *dev)
 +{
 + pr_debug(resetting\n);
 +
 + iowrite32(~0x1f, dev-iaddr);
 + wmb();
 + iowrite32(0x1f, dev-iaddr);
 + dev-status |= PHB_RESET;
 +}

Does the wmb here actually do anything useful?  If so, a comment is needed
because it isn't possible to work out why that statement is there by
reading the code.

 ...

 +
 +static irqreturn_t phantom_isr(int irq, void *data)
 +{
 + struct phantom_device *dev = data;
 + struct input_dev *idev = dev-idev;
 + unsigned int a, hw_status;
 +
 + hw_status = ioread32(dev-iaddr + PHN_CONTROL);
 + if (!(hw_status  PHN_CTL_IRQ))
 + return IRQ_NONE;
 +
 + iowrite32(0, dev-iaddr);
 + wmb();
 + iowrite32(0xc0, dev-iaddr);

there too.

 + if (unlikely(idev == NULL))
 + return IRQ_HANDLED;

Can this happen?  If so, a comment explaining why would be nice.

 + input_report_abs(idev, ABS_X, PCI_ENCODER(dev, 0));
 + input_report_abs(idev, ABS_Y, PCI_ENCODER(dev, 1));
 + input_report_abs(idev, ABS_Z, PCI_ENCODER(dev, 2));
 + input_report_abs(idev, ABS_RX, PCI_ENCODER(dev, 3));
 + input_report_abs(idev, ABS_RY, PCI_ENCODER(dev, 4));
 + input_report_abs(idev, ABS_RZ, PCI_ENCODER(dev, 5));
 + input_report_key(idev, BTN_0, !!(hw_status  PHN_CTL_BUT));
 + input_sync(idev);
 + input_event(idev, EV_SYN, SYN_CONFIG, 0);
 +
 + for (a = 0; a  PHM_MAX_TORQUES; a++)
 + iowrite32(dev-torques[a] + PHN_ZERO_FORCE, dev-oaddr + a);
 + iowrite32(dev-amp_bit, dev-iaddr + PHN_CONTROL);
 + dev-amp_bit ^= PHN_CTL_AMP;
 +
 + return IRQ_HANDLED;
 +}
 +
 +/**

A plain old

/*
 * Init and deinit driver
 */

is sufficient and typical.

But this is not actually the most useful of comments anyway ;)

 + * Init and deinit