Re: [PATCH V2] pcmcia/yenta: add module parameter for O2 speedups

2010-01-11 Thread Wolfram Sang
Hi,

 (1)
 The default setting of O2_6933 should be on?

It was set to off because of this bug-report:

http://www.mail-archive.com/linux-pcmcia@lists.infradead.org/msg02048.html

The patch went through this list multiple times and there were no complaints.

 (2)
 How about the name enable_prefetch instead of o2_speedup?

Hmm, prefetch is only for read, for write it is called burst, so I took this
name. Also, I'd think it should have o2 somewhere in the name, as it doesn't
do anything on other controllers. I also didn't find existing parameters doing
something similar, otherwise I would have tried to group them.

Regards,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature
___
Linux PCMCIA reimplementation list
http://lists.infradead.org/mailman/listinfo/linux-pcmcia


Re: [PATCH V2] pcmcia/yenta: add module parameter for O2 speedups

2010-01-10 Thread Dominik Brodowski
Hey,

On Sun, Jan 10, 2010 at 09:41:24AM +0100, Wolfram Sang wrote:
 O2-bridges can do read prefetch and write burst. However, for some 
 combinations
 of older bridges and cards, this causes problems, so it is disabled for those
 bridges. Now, as some users know their setup works with the speedups enabled, 
 a
 new parameter is introduced to the driver. Now, a user can specifically enable
 or disable these features, while the default is what we have today: detect the
 bridge and decide accordingly. Fixes Bugzilla entry 15014.
 
 Simplify and unify the printouts, fix a whitespace issue while we are here.
 
 Signed-off-by: Wolfram Sang w.s...@pengutronix.de
 Tested-by: frod...@gmail.com
 Cc: Dominik Brodowski li...@dominikbrodowski.net
 ---
 
 Changes since V1:
 
 - fix a whitespace issue
 - add the comment user may override
 
  drivers/pcmcia/o2micro.h  |   41 
 +++--
  drivers/pcmcia/yenta_socket.c |5 +
  2 files changed, 32 insertions(+), 14 deletions(-)
 
 diff --git a/drivers/pcmcia/o2micro.h b/drivers/pcmcia/o2micro.h
 index 624442f..72d8f33 100644
 --- a/drivers/pcmcia/o2micro.h
 +++ b/drivers/pcmcia/o2micro.h
 @@ -116,13 +116,12 @@ static int o2micro_override(struct yenta_socket *socket)
* from Eric Still, 02Micro.
*/
   u8 a, b;
 + bool use_speedup;
  
   if (PCI_FUNC(socket-dev-devfn) == 0) {
   a = config_readb(socket, O2_RESERVED1);
   b = config_readb(socket, O2_RESERVED2);
 -
 - dev_printk(KERN_INFO, socket-dev-dev,
 -O2: res at 0x94/0xD4: %02x/%02x\n, a, b);
 + dev_dbg(socket-dev-dev, O2: 0x94/0xD4: %02x/%02x\n, a, b);
  
   switch (socket-dev-device) {
   /*
 @@ -135,23 +134,37 @@ static int o2micro_override(struct yenta_socket *socket)
   case PCI_DEVICE_ID_O2_6812:
   case PCI_DEVICE_ID_O2_6832:
   case PCI_DEVICE_ID_O2_6836:
 - case PCI_DEVICE_ID_O2_6933:
 - dev_printk(KERN_INFO, socket-dev-dev,
 -Yenta O2: old bridge, disabling read 
 -prefetch/write burst\n);
 - config_writeb(socket, O2_RESERVED1,
 -   a  ~(O2_RES_READ_PREFETCH | 
 O2_RES_WRITE_BURST));
 - config_writeb(socket, O2_RESERVED2,
 -   b  ~(O2_RES_READ_PREFETCH | 
 O2_RES_WRITE_BURST));
 + case PCI_DEVICE_ID_O2_6933:
 + use_speedup = false;
   break;
 -
   default:
 - dev_printk(KERN_INFO , socket-dev-dev,
 -O2: enabling read prefetch/write burst\n);
 + use_speedup = true;
 + break;
 + }
 +
 + /* the user may override our decision */
 + if (strcasecmp(o2_speedup, on) == 0)
 + use_speedup = true;
 + else if (strcasecmp(o2_speedup, off) == 0)
 + use_speedup = false;
 + else if (strcasecmp(o2_speedup, default) != 0)
 + dev_warn(socket-dev-dev,
 + O2: Unknown parameter, using 'default');
 +
 + if (use_speedup) {
 + dev_info(socket-dev-dev,
 + O2: enabling read prefetch/write burst\n);
   config_writeb(socket, O2_RESERVED1,
 a | O2_RES_READ_PREFETCH | 
 O2_RES_WRITE_BURST);
   config_writeb(socket, O2_RESERVED2,
 b | O2_RES_READ_PREFETCH | 
 O2_RES_WRITE_BURST);
 + } else {
 + dev_info(socket-dev-dev,
 + O2: disabling read prefetch/write burst\n);
 + config_writeb(socket, O2_RESERVED1,
 +   a  ~(O2_RES_READ_PREFETCH | 
 O2_RES_WRITE_BURST));
 + config_writeb(socket, O2_RESERVED2,
 +   b  ~(O2_RES_READ_PREFETCH | 
 O2_RES_WRITE_BURST));
   }
   }
  
 diff --git a/drivers/pcmcia/yenta_socket.c b/drivers/pcmcia/yenta_socket.c
 index e4d12ac..041a75a 100644
 --- a/drivers/pcmcia/yenta_socket.c
 +++ b/drivers/pcmcia/yenta_socket.c
 @@ -37,6 +37,11 @@ static int pwr_irqs_off;
  module_param(pwr_irqs_off, bool, 0644);
  MODULE_PARM_DESC(pwr_irqs_off, Force IRQs off during power-on of slot. Use 
 only when seeing IRQ storms!);
  
 +static char o2_speedup[] = default;
 +module_param_string(o2_speedup, o2_speedup, sizeof(o2_speedup), 0444);
 +MODULE_PARM_DESC(o2_speedup, Use prefetch/burst for O2-bridges: 'on', 'off' 
 
 + or 'default' (uses recommended behaviour for the detected bridge));

Is using a string module_param the preferred case for such tristates now?

Best,
Dominik


Re: [PATCH V2] pcmcia/yenta: add module parameter for O2 speedups

2010-01-10 Thread Komuro
Hi,

(1)
The default setting of O2_6933 should be on?

(2)
How about the name enable_prefetch instead of o2_speedup?




O2-bridges can do read prefetch and write burst. However, for some combinatio
ns
of older bridges and cards, this causes problems, so it is disabled for those
bridges. Now, as some users know their setup works with the speedups enabled,
 a
new parameter is introduced to the driver. Now, a user can specifically enabl
e
or disable these features, while the default is what we have today: detect th
e
bridge and decide accordingly. Fixes Bugzilla entry 15014.

Simplify and unify the printouts, fix a whitespace issue while we are here.

Signed-off-by: Wolfram Sang w.s...@pengutronix.de
Tested-by: frod...@gmail.com
Cc: Dominik Brodowski li...@dominikbrodowski.net
---


Best Regards
Komuro


___
Linux PCMCIA reimplementation list
http://lists.infradead.org/mailman/listinfo/linux-pcmcia