Re: [PATCH] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Tim Hockin

Pete Zaitcev wrote:

> > i2c is only in our stuff because the i2c core is not in the standard kernel
> > yet.  As soon as it is, I will make cobalt_i2c* go away.
> 
> I am puzzled by this comment. Did you look into drivers/i2c/?
> It certainly is a part of a stock kernel. The main user is
> the V4L, in drivers/media/video, but I think LM sensors use it too.

sorry, I meant to say:  The core is in, but the drivers for the adapters in
question are not.  They are part of lm_sensors, and as such, make it very
hard for us to maintain.  I have encouraged the lm_sensors crew to get at
LEAST the adapters/algorithms submitted for general inclusion.  Once that
is in, I will make cobalt_i2c go away.

Tim
-- 
Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
[EMAIL PROTECTED]
-
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] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Bogdan Costescu

On Fri, 1 Jun 2001, Pete Zaitcev wrote:

> > But, each time a user cats this proc file, the user is banging the
> > hardware.  What happens when a malicious user forks off 100 processes to
> > continually cat this file?  :)
>
> Nothing good, probably. Same story as /proc/apm, which only
> hits BIOS instead (and it's debateable what is better).

Hmm, the MII related ioctl's in some net drivers (checked for 3c59x,
tulip, eepro100) are also querying the hardware. And a user is allowed to
ask for this info (but not able to modify it).

Sincerely,

Bogdan Costescu

IWR - Interdisziplinaeres Zentrum fuer Wissenschaftliches Rechnen
Universitaet Heidelberg, INF 368, D-69120 Heidelberg, GERMANY
Telephone: +49 6221 54 8869, Telefax: +49 6221 54 8868
E-mail: [EMAIL PROTECTED]

-
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] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Pete Zaitcev

> But, each time a user cats this proc file, the user is banging the
> hardware.  What happens when a malicious user forks off 100 processes to
> continually cat this file?  :)

Nothing good, probably. Same story as /proc/apm, which only
hits BIOS instead (and it's debateable what is better).

> Security: don't you want CAP_RAW_IO or something before executing any of
> these ioctls?

Perhaps it's mode 600 in their distro...

> bug: you can't hold a spinlock while you are sleeping in copy_from_user

Yep... I meant to check for it but forgot.

-- Pete
-
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] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Pete Zaitcev

> From: Tim Hockin <[EMAIL PROTECTED]>
> Date: Thu, 31 May 2001 23:57:48 -0700 (PDT)

> > i2c framework is not used, I wonder why. Someone thought that
> > it was too heavy perhaps? If so, I disagree.
> 
> i2c is only in our stuff because the i2c core is not in the standard kernel
> yet.  As soon as it is, I will make cobalt_i2c* go away.

I am puzzled by this comment. Did you look into drivers/i2c/?
It certainly is a part of a stock kernel. The main user is
the V4L, in drivers/media/video, but I think LM sensors use it too.

-- Pete
-
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] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Jeff Garzik

Tim Hockin wrote:
> +int __init
> +cobalt_acpi_init(void)
> +{
> +   int err, reg;
> +   u16 addr;
> +   unsigned long flags;
> +
> +   if (cobt_is_5k()) {
> +   /* setup osb4 i/o regions */
> +   if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x20)))
> +   request_region(reg, 4, "OSB4 (pm1a_evt_blk)");
> +   if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x22)))
> +   request_region(reg, 2, "OSB4 (pm1a_cnt_blk)");
> +   if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x24)))
> +   request_region(reg, 4, "OSB4 (pm_tmr_blk)");
> +   if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x26)))
> +   request_region(reg, 6, "OSB4 (p_cnt_blk)");
> +   if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x28)))
> +   request_region(reg, 8, "OSB4 (gpe0_blk)");
> +   osb4_port = reg;
> +
> +   spin_lock_irqsave(_superio_lock, flags);
> +
> +   /* superi/o -- select pm logical device and get base address */
> +   outb(SUPERIO_LOGICAL_DEV, SUPERIO_INDEX_PORT);
> +   outb(SUPERIO_DEV_PM, SUPERIO_DATA_PORT);
> +   outb(SUPERIO_ADDR_HIGH, SUPERIO_INDEX_PORT);
> +   addr = inb(SUPERIO_DATA_PORT) << 8;
> +   outb(SUPERIO_ADDR_LOW, SUPERIO_INDEX_PORT);
> +   addr |= inb(SUPERIO_DATA_PORT);
> +   if (addr) {
> +   /* get registers */
> +   if ((reg = get_reg(addr, addr + 1, 0x08))) {
> +   request_region(reg, 4, "SUPERIO (pm1_evt_blk)");
> +   superio_port = reg;
> +   }
> +   if ((reg = get_reg(addr, addr + 1, 0x0c)))
> +   request_region(reg, 2, "SUPERIO (pm1_cnt_blk)");
> +   if ((reg = get_reg(addr, addr + 1, 0x0e)))
> +   request_region(reg, 16, "SUPERIO (gpe_blk)");

need to check for request_region failure.  since you have a lot of
request_region calls, you may want to consider breaking them out into a
separate function which returns success or failure, and handles details
of cleaning up after 4 request_regions succeed but the 5th one fails.

if (!request_region(region1))
goto out;
if (!request_region(region2))
goto cleanup_1;
if (!request_region(region3))
goto cleanup_2;
return 0;

cleanup_2:
release_region(region2);
cleanup_1:
release_region(region1);
out:
return -EBUSY;

> +   /* setup an interrupt handler for an ACPI SCI */
> +   err = request_irq(ACPI_IRQ, acpi_interrupt,
> + SA_SHIRQ, ACPI_NAME, (void *)ACPI_MAGIC);
> +   if (err) {
> +   EPRINTK("couldn't assign ACPI IRQ (%d)\n", ACPI_IRQ);
> +   return -1;
> +   }

return 'err' not -1 here?

> +bool 'Support for Cobalt Networks x86 servers' CONFIG_COBALT
> +
> +if [ "$CONFIG_COBALT" != "n" ]; then
> +   bool 'Gen III (3000 series) system support' CONFIG_COBALT_GEN_III
> +   bool 'Gen V (5000 series) system support' CONFIG_COBALT_GEN_V
> +   bool 'Create legacy /proc files' CONFIG_COBALT_OLDPROC
> +
> +   comment 'Cobalt hardware options'
> +
> +   bool 'Front panel (LCD) support' CONFIG_COBALT_LCD
> +   bool 'Software controlled LED support' CONFIG_COBALT_LED
> +   bool 'Serial number support' CONFIG_COBALT_SERNUM
> +   bool 'Watchdog timer support' CONFIG_COBALT_WDT
> +   bool 'Thermal sensor support' CONFIG_COBALT_THERMAL
> +   bool 'Fan tachometer support' CONFIG_COBALT_FANS
> +   bool 'Disk drive ruler support' CONFIG_COBALT_RULER
> +fi

Style:  s/bool '/bool '  /


> +#ifdef CONFIG_PROC_FS
> +#ifdef CONFIG_COBALT_OLDPROC
> +   proc_faninfo = create_proc_read_entry("faninfo", S_IFREG | S_IRUGO,
> +   NULL, fan_read_proc, NULL);
> +if (!proc_faninfo) {
> +   EPRINTK("can't create /proc/faninfo\n");
> +   }
> +#endif
> +   proc_cfaninfo = create_proc_read_entry("faninfo", S_IFREG | S_IRUGO,
> +   proc_cobalt, fan_read_proc, NULL);
> +if (!proc_cfaninfo) {
> +   EPRINTK("can't create /proc/cobalt/faninfo\n");
> +   }
> +#endif
> +
> +   printk("Cobalt Networks fan tachometers v1.2\n");
> +
> +   return 0;
> +}

S_IFREG|S_IRUGO is the default, so simply pass zero mode for this
behavior.

But, each time a user cats this proc file, the user is banging the
hardware.  What happens when a malicious user forks off 100 processes to
continually cat this file?  :)

> +/* various file operations we support for this driver */
> +static struct file_operations lcd_fops = {
> +   read:   cobalt_lcd_read,
> +   ioctl:  cobalt_lcd_ioctl,
> +   

Re: [PATCH] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Jeff Garzik

General comments:

* Code looks really clean.  Nice work.
* Use module_init/exit.  I know, I know, you heard it before :)
* I dunno if Linus will take it as-is because he has been threatening to
stop taking PCI drives that use old-style PCI init for no good reason. 
(he even made me change a driver that used old-style PCI init for a good
reason :))
* There is a ton of procfs stuff in there.  I suppose !CONFIG_PROC_FS is
not a supported configuration on Cobalt?  :)


Tim Hockin wrote:
> +/* this is essentially an exported function - it is in the hwif structs */
> +static int
> +ruler_busproc_fn(ide_hwif_t *hwif, int arg)
[...]
> +   read_lock(_lock);
[...]
> +   read_unlock(_lock);

Should this be read_lock_bh, since other uses are in a timer function or
_irqsave/restore?


> +   /* The GPIO tied to the ID chip is on the PMU */
> +   id_dev = pci_find_device(PCI_VENDOR_ID_AL,
> +   PCI_DEVICE_ID_AL_M7101, NULL);

as mentioned earlier, pci_register_driver is preferred over "old style"
PCI.


> +   spin_lock_irqsave(_superio_lock, flags);
> +   outb(SUPERIO_LOGICAL_DEV, SUPERIO_INDEX_PORT);
> +   outb(SUPERIO_DEV_GPIO, SUPERIO_DATA_PORT);
> +   outb(SUPERIO_ADDR_HIGH, SUPERIO_INDEX_PORT);
> +   addr = inb(SUPERIO_DATA_PORT) << 8;
> +   outb(SUPERIO_ADDR_LOW, SUPERIO_INDEX_PORT);
> +   addr |= inb(SUPERIO_DATA_PORT);
> +   spin_unlock_irqrestore(_superio_lock, flags);

Nothing wrong here, just commenting that I wish other superio did this
sometimes in some cases... :)

> +static void __init
> +io_write_byte(unsigned char c)
> +{
> +   int i;
> +   unsigned long flags;
> +
> +   save_flags(flags);
> +
> +   for (i = 0; i < 8; i++, c >>= 1) {
> +   cli();
> +   if (c & 1) {
> +   /* Transmit a 1 */
> +   io_write(5);
> +   udelay(80);
> +   } else {
> +   /* Transmit a 0 */
> +   io_write(80);
> +   udelay(10);
> +   }
> +   restore_flags(flags);
> +   }
> +}

Can save/restore flags be replaced with a spinlock?


> +   /* get version from CVS */
> +   version = strchr("$Revision: 1.4 $", ':') + 2;
> +   if (version) {
> +   char *p;
> +
> +   strncpy(vstring, version, sizeof(vstring));
> +   if ((p = strchr(vstring, ' '))) {
> +   *p = '\0';
> +   }
> +   } else {
> +   strncpy(vstring, "unknown", sizeof(vstring));
> +   }

ug :)  It would be nice if this could be done at compile time

> +   proc_serialnum = create_proc_read_entry("serialnumber", 0, NULL,
> +   serialnum_read, NULL);
> +   if (!proc_serialnum) {
> +   EPRINTK("can't create /proc/serialnumber\n");
> +   }
> +#endif
> +   proc_chostid = create_proc_read_entry("hostid", 0, proc_cobalt,
> +   hostid_read, NULL);
> +   if (!proc_chostid) {
> +   EPRINTK("can't create /proc/cobalt/hostid\n");
> +   }
> +   proc_cserialnum = create_proc_read_entry("serialnumber", 0,
> +   proc_cobalt, serialnum_read, NULL);
> +   if (!proc_cserialnum) {
> +   EPRINTK("can't create /proc/cobalt/serialnumber\n");

security concern?

We disable CPU processor serial numbers on x86, maybe you want to make
everything except the output of /usr/bin/hostid priveleged?


> diff -ruN dist-2.4.5/drivers/cobalt/wdt.c cobalt-2.4.5/drivers/cobalt/wdt.c
> --- dist-2.4.5/drivers/cobalt/wdt.c Wed Dec 31 16:00:00 1969
> +++ cobalt-2.4.5/drivers/cobalt/wdt.c   Thu May 31 14:32:15 2001

Shouldn't this be stored with other watchdog timers?

> diff -ruN dist-2.4.5/include/linux/cobalt-acpi.h 
>cobalt-2.4.5/include/linux/cobalt-acpi.h
> --- dist-2.4.5/include/linux/cobalt-acpi.h  Wed Dec 31 16:00:00 1969
> +++ cobalt-2.4.5/include/linux/cobalt-acpi.hThu May 31 14:33:16 2001

Another ACPI user... neat!



> +/* the root of /proc/cobalt */
> +extern struct proc_dir_entry *proc_cobalt;

I am wondering if some of this stuff can be a sysctl instead of
procfs???

> +
> +#endif
> diff -ruN dist-2.4.5/include/linux/cobalt-i2c.h 
>cobalt-2.4.5/include/linux/cobalt-i2c.h
> --- dist-2.4.5/include/linux/cobalt-i2c.h   Wed Dec 31 16:00:00 1969
> +++ cobalt-2.4.5/include/linux/cobalt-i2c.h Thu May 31 14:33:16 2001

Sometimes I wish for a directory structure with:
* arch-specific includes
* platform-specific includes
* generic core includes

Then we could put this stuff in include/cobalt/* or
platform/cobalt/include or somesuch.  


> +extern cobt_sys_t cobt_type;
> +/* one for each major board-type - COBT_SUPPORT_* from  */
> +#define cobt_is_raq3()  (COBT_SUPPORT_GEN_III && cobt_type == COBT_RAQ3)
> +#define cobt_is_qube3()

Re: [PATCH] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Jeff Garzik

> > Off-hand I see old style initialization. Is it right for new driver?
> 
> the old-style init is because it is an old driver.  I want to do a full-on
> rework, but haven't had the time.

New-style init by itself shouldn't be hard to do, independent of a full
re-work...


> > 2. Spaces and tabs are mixed in funny ways, makes to cute effects
> > when quoting diffs.
> 
> I've tried to eliminate that when I see it - I'll give the diff a close
> examination.

Why not just run indent over the source before submitting.  That will
regularize this stuff, and ensure that you are close to
Documentation/CodingStyle.

Here is the command I use.  The first two options are the only really
importants ones...

indent -kr -i8 -npsl -pcs -l100 -lc120

(line length is 100 because line length 72/75/80 winds up wrapping long
printk and logic lines when typically the programmer didn't want them to
be wrapped)

-- 
Jeff Garzik  | Disbelief, that's why you fail.
Building 1024|
MandrakeSoft |
-
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] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Tim Hockin

> Looks interesting. Seemingly literate use of spinlocks.

thanks - I gave it lots of thought.

> Off-hand I see old style initialization. Is it right for new driver?

the old-style init is because it is an old driver.  I want to do a full-on
rework, but haven't had the time.

> i2c framework is not used, I wonder why. Someone thought that
> it was too heavy perhaps? If so, I disagree.

i2c is only in our stuff because the i2c core is not in the standard kernel
yet.  As soon as it is, I will make cobalt_i2c* go away.

> if any alignment with lm-sensors is possible, for the sake of

yes - I have communicated with the lm-sensors crew.  It is very high on my
wishlist.

> lcd_read bounces reads with -EINVAL when another read is in
> progress. Gross.

as I said - I didn't write the LCD driver, I just had to port it up :)  I
want to re-do the whole paradigm of it (it has been ported forward since 
2.0.3x)

> 1.:
>   p = head;
>   while (p) {
>   p = p->next;
>   }
> 
> It is what for(;;) does.

I don't get it - are you saying you do or don't like the while (p)
approach?  I think it is clearer because it is more true ot the heuristic -
"start at the beginning and walk down the list".

> 2. Spaces and tabs are mixed in funny ways, makes to cute effects
> when quoting diffs.

I've tried to eliminate that when I see it - I'll give the diff a close
examination.

thanks for the feedback - it will be nice to not have to constantly port
all our changes to each kernel release.  There are still some patches (of
course) but I didn't submit them because they are VERY specific to cobalt -
for example in the ide probing calling cobalt_ruler_register().  Ifdefs
protect, but the overall appearance would be rejected, I suspect - no?

Tim
-
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] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Tim Hockin

 Looks interesting. Seemingly literate use of spinlocks.

thanks - I gave it lots of thought.

 Off-hand I see old style initialization. Is it right for new driver?

the old-style init is because it is an old driver.  I want to do a full-on
rework, but haven't had the time.

 i2c framework is not used, I wonder why. Someone thought that
 it was too heavy perhaps? If so, I disagree.

i2c is only in our stuff because the i2c core is not in the standard kernel
yet.  As soon as it is, I will make cobalt_i2c* go away.

 if any alignment with lm-sensors is possible, for the sake of

yes - I have communicated with the lm-sensors crew.  It is very high on my
wishlist.

 lcd_read bounces reads with -EINVAL when another read is in
 progress. Gross.

as I said - I didn't write the LCD driver, I just had to port it up :)  I
want to re-do the whole paradigm of it (it has been ported forward since 
2.0.3x)

 1.:
   p = head;
   while (p) {
   p = p-next;
   }
 
 It is what for(;;) does.

I don't get it - are you saying you do or don't like the while (p)
approach?  I think it is clearer because it is more true ot the heuristic -
start at the beginning and walk down the list.

 2. Spaces and tabs are mixed in funny ways, makes to cute effects
 when quoting diffs.

I've tried to eliminate that when I see it - I'll give the diff a close
examination.

thanks for the feedback - it will be nice to not have to constantly port
all our changes to each kernel release.  There are still some patches (of
course) but I didn't submit them because they are VERY specific to cobalt -
for example in the ide probing calling cobalt_ruler_register().  Ifdefs
protect, but the overall appearance would be rejected, I suspect - no?

Tim
-
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] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Jeff Garzik

  Off-hand I see old style initialization. Is it right for new driver?
 
 the old-style init is because it is an old driver.  I want to do a full-on
 rework, but haven't had the time.

New-style init by itself shouldn't be hard to do, independent of a full
re-work...


  2. Spaces and tabs are mixed in funny ways, makes to cute effects
  when quoting diffs.
 
 I've tried to eliminate that when I see it - I'll give the diff a close
 examination.

Why not just run indent over the source before submitting.  That will
regularize this stuff, and ensure that you are close to
Documentation/CodingStyle.

Here is the command I use.  The first two options are the only really
importants ones...

indent -kr -i8 -npsl -pcs -l100 -lc120

(line length is 100 because line length 72/75/80 winds up wrapping long
printk and logic lines when typically the programmer didn't want them to
be wrapped)

-- 
Jeff Garzik  | Disbelief, that's why you fail.
Building 1024|
MandrakeSoft |
-
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] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Jeff Garzik

General comments:

* Code looks really clean.  Nice work.
* Use module_init/exit.  I know, I know, you heard it before :)
* I dunno if Linus will take it as-is because he has been threatening to
stop taking PCI drives that use old-style PCI init for no good reason. 
(he even made me change a driver that used old-style PCI init for a good
reason :))
* There is a ton of procfs stuff in there.  I suppose !CONFIG_PROC_FS is
not a supported configuration on Cobalt?  :)


Tim Hockin wrote:
 +/* this is essentially an exported function - it is in the hwif structs */
 +static int
 +ruler_busproc_fn(ide_hwif_t *hwif, int arg)
[...]
 +   read_lock(ruler_lock);
[...]
 +   read_unlock(ruler_lock);

Should this be read_lock_bh, since other uses are in a timer function or
_irqsave/restore?


 +   /* The GPIO tied to the ID chip is on the PMU */
 +   id_dev = pci_find_device(PCI_VENDOR_ID_AL,
 +   PCI_DEVICE_ID_AL_M7101, NULL);

as mentioned earlier, pci_register_driver is preferred over old style
PCI.


 +   spin_lock_irqsave(cobalt_superio_lock, flags);
 +   outb(SUPERIO_LOGICAL_DEV, SUPERIO_INDEX_PORT);
 +   outb(SUPERIO_DEV_GPIO, SUPERIO_DATA_PORT);
 +   outb(SUPERIO_ADDR_HIGH, SUPERIO_INDEX_PORT);
 +   addr = inb(SUPERIO_DATA_PORT)  8;
 +   outb(SUPERIO_ADDR_LOW, SUPERIO_INDEX_PORT);
 +   addr |= inb(SUPERIO_DATA_PORT);
 +   spin_unlock_irqrestore(cobalt_superio_lock, flags);

Nothing wrong here, just commenting that I wish other superio did this
sometimes in some cases... :)

 +static void __init
 +io_write_byte(unsigned char c)
 +{
 +   int i;
 +   unsigned long flags;
 +
 +   save_flags(flags);
 +
 +   for (i = 0; i  8; i++, c = 1) {
 +   cli();
 +   if (c  1) {
 +   /* Transmit a 1 */
 +   io_write(5);
 +   udelay(80);
 +   } else {
 +   /* Transmit a 0 */
 +   io_write(80);
 +   udelay(10);
 +   }
 +   restore_flags(flags);
 +   }
 +}

Can save/restore flags be replaced with a spinlock?


 +   /* get version from CVS */
 +   version = strchr($Revision: 1.4 $, ':') + 2;
 +   if (version) {
 +   char *p;
 +
 +   strncpy(vstring, version, sizeof(vstring));
 +   if ((p = strchr(vstring, ' '))) {
 +   *p = '\0';
 +   }
 +   } else {
 +   strncpy(vstring, unknown, sizeof(vstring));
 +   }

ug :)  It would be nice if this could be done at compile time

 +   proc_serialnum = create_proc_read_entry(serialnumber, 0, NULL,
 +   serialnum_read, NULL);
 +   if (!proc_serialnum) {
 +   EPRINTK(can't create /proc/serialnumber\n);
 +   }
 +#endif
 +   proc_chostid = create_proc_read_entry(hostid, 0, proc_cobalt,
 +   hostid_read, NULL);
 +   if (!proc_chostid) {
 +   EPRINTK(can't create /proc/cobalt/hostid\n);
 +   }
 +   proc_cserialnum = create_proc_read_entry(serialnumber, 0,
 +   proc_cobalt, serialnum_read, NULL);
 +   if (!proc_cserialnum) {
 +   EPRINTK(can't create /proc/cobalt/serialnumber\n);

security concern?

We disable CPU processor serial numbers on x86, maybe you want to make
everything except the output of /usr/bin/hostid priveleged?


 diff -ruN dist-2.4.5/drivers/cobalt/wdt.c cobalt-2.4.5/drivers/cobalt/wdt.c
 --- dist-2.4.5/drivers/cobalt/wdt.c Wed Dec 31 16:00:00 1969
 +++ cobalt-2.4.5/drivers/cobalt/wdt.c   Thu May 31 14:32:15 2001

Shouldn't this be stored with other watchdog timers?

 diff -ruN dist-2.4.5/include/linux/cobalt-acpi.h 
cobalt-2.4.5/include/linux/cobalt-acpi.h
 --- dist-2.4.5/include/linux/cobalt-acpi.h  Wed Dec 31 16:00:00 1969
 +++ cobalt-2.4.5/include/linux/cobalt-acpi.hThu May 31 14:33:16 2001

Another ACPI user... neat!



 +/* the root of /proc/cobalt */
 +extern struct proc_dir_entry *proc_cobalt;

I am wondering if some of this stuff can be a sysctl instead of
procfs???

 +
 +#endif
 diff -ruN dist-2.4.5/include/linux/cobalt-i2c.h 
cobalt-2.4.5/include/linux/cobalt-i2c.h
 --- dist-2.4.5/include/linux/cobalt-i2c.h   Wed Dec 31 16:00:00 1969
 +++ cobalt-2.4.5/include/linux/cobalt-i2c.h Thu May 31 14:33:16 2001

Sometimes I wish for a directory structure with:
* arch-specific includes
* platform-specific includes
* generic core includes

Then we could put this stuff in include/cobalt/* or
platform/cobalt/include or somesuch.  


 +extern cobt_sys_t cobt_type;
 +/* one for each major board-type - COBT_SUPPORT_* from linux/cobalt.h */
 +#define cobt_is_raq3()  (COBT_SUPPORT_GEN_III  cobt_type == COBT_RAQ3)
 +#define cobt_is_qube3() (COBT_SUPPORT_GEN_III  cobt_type == COBT_QUBE3)
 +#define 

Re: [PATCH] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Jeff Garzik

Tim Hockin wrote:
 +int __init
 +cobalt_acpi_init(void)
 +{
 +   int err, reg;
 +   u16 addr;
 +   unsigned long flags;
 +
 +   if (cobt_is_5k()) {
 +   /* setup osb4 i/o regions */
 +   if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x20)))
 +   request_region(reg, 4, OSB4 (pm1a_evt_blk));
 +   if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x22)))
 +   request_region(reg, 2, OSB4 (pm1a_cnt_blk));
 +   if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x24)))
 +   request_region(reg, 4, OSB4 (pm_tmr_blk));
 +   if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x26)))
 +   request_region(reg, 6, OSB4 (p_cnt_blk));
 +   if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x28)))
 +   request_region(reg, 8, OSB4 (gpe0_blk));
 +   osb4_port = reg;
 +
 +   spin_lock_irqsave(cobalt_superio_lock, flags);
 +
 +   /* superi/o -- select pm logical device and get base address */
 +   outb(SUPERIO_LOGICAL_DEV, SUPERIO_INDEX_PORT);
 +   outb(SUPERIO_DEV_PM, SUPERIO_DATA_PORT);
 +   outb(SUPERIO_ADDR_HIGH, SUPERIO_INDEX_PORT);
 +   addr = inb(SUPERIO_DATA_PORT)  8;
 +   outb(SUPERIO_ADDR_LOW, SUPERIO_INDEX_PORT);
 +   addr |= inb(SUPERIO_DATA_PORT);
 +   if (addr) {
 +   /* get registers */
 +   if ((reg = get_reg(addr, addr + 1, 0x08))) {
 +   request_region(reg, 4, SUPERIO (pm1_evt_blk));
 +   superio_port = reg;
 +   }
 +   if ((reg = get_reg(addr, addr + 1, 0x0c)))
 +   request_region(reg, 2, SUPERIO (pm1_cnt_blk));
 +   if ((reg = get_reg(addr, addr + 1, 0x0e)))
 +   request_region(reg, 16, SUPERIO (gpe_blk));

need to check for request_region failure.  since you have a lot of
request_region calls, you may want to consider breaking them out into a
separate function which returns success or failure, and handles details
of cleaning up after 4 request_regions succeed but the 5th one fails.

if (!request_region(region1))
goto out;
if (!request_region(region2))
goto cleanup_1;
if (!request_region(region3))
goto cleanup_2;
return 0;

cleanup_2:
release_region(region2);
cleanup_1:
release_region(region1);
out:
return -EBUSY;

 +   /* setup an interrupt handler for an ACPI SCI */
 +   err = request_irq(ACPI_IRQ, acpi_interrupt,
 + SA_SHIRQ, ACPI_NAME, (void *)ACPI_MAGIC);
 +   if (err) {
 +   EPRINTK(couldn't assign ACPI IRQ (%d)\n, ACPI_IRQ);
 +   return -1;
 +   }

return 'err' not -1 here?

 +bool 'Support for Cobalt Networks x86 servers' CONFIG_COBALT
 +
 +if [ $CONFIG_COBALT != n ]; then
 +   bool 'Gen III (3000 series) system support' CONFIG_COBALT_GEN_III
 +   bool 'Gen V (5000 series) system support' CONFIG_COBALT_GEN_V
 +   bool 'Create legacy /proc files' CONFIG_COBALT_OLDPROC
 +
 +   comment 'Cobalt hardware options'
 +
 +   bool 'Front panel (LCD) support' CONFIG_COBALT_LCD
 +   bool 'Software controlled LED support' CONFIG_COBALT_LED
 +   bool 'Serial number support' CONFIG_COBALT_SERNUM
 +   bool 'Watchdog timer support' CONFIG_COBALT_WDT
 +   bool 'Thermal sensor support' CONFIG_COBALT_THERMAL
 +   bool 'Fan tachometer support' CONFIG_COBALT_FANS
 +   bool 'Disk drive ruler support' CONFIG_COBALT_RULER
 +fi

Style:  s/bool '/bool '  /


 +#ifdef CONFIG_PROC_FS
 +#ifdef CONFIG_COBALT_OLDPROC
 +   proc_faninfo = create_proc_read_entry(faninfo, S_IFREG | S_IRUGO,
 +   NULL, fan_read_proc, NULL);
 +if (!proc_faninfo) {
 +   EPRINTK(can't create /proc/faninfo\n);
 +   }
 +#endif
 +   proc_cfaninfo = create_proc_read_entry(faninfo, S_IFREG | S_IRUGO,
 +   proc_cobalt, fan_read_proc, NULL);
 +if (!proc_cfaninfo) {
 +   EPRINTK(can't create /proc/cobalt/faninfo\n);
 +   }
 +#endif
 +
 +   printk(Cobalt Networks fan tachometers v1.2\n);
 +
 +   return 0;
 +}

S_IFREG|S_IRUGO is the default, so simply pass zero mode for this
behavior.

But, each time a user cats this proc file, the user is banging the
hardware.  What happens when a malicious user forks off 100 processes to
continually cat this file?  :)

 +/* various file operations we support for this driver */
 +static struct file_operations lcd_fops = {
 +   read:   cobalt_lcd_read,
 +   ioctl:  cobalt_lcd_ioctl,
 +   open:   cobalt_lcd_open,
 +};

Needs owner field assigned a value.

 +static int
 +cobalt_lcd_ioctl(struct 

Re: [PATCH] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Pete Zaitcev

 From: Tim Hockin [EMAIL PROTECTED]
 Date: Thu, 31 May 2001 23:57:48 -0700 (PDT)

  i2c framework is not used, I wonder why. Someone thought that
  it was too heavy perhaps? If so, I disagree.
 
 i2c is only in our stuff because the i2c core is not in the standard kernel
 yet.  As soon as it is, I will make cobalt_i2c* go away.

I am puzzled by this comment. Did you look into drivers/i2c/?
It certainly is a part of a stock kernel. The main user is
the V4L, in drivers/media/video, but I think LM sensors use it too.

-- Pete
-
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] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Pete Zaitcev

 But, each time a user cats this proc file, the user is banging the
 hardware.  What happens when a malicious user forks off 100 processes to
 continually cat this file?  :)

Nothing good, probably. Same story as /proc/apm, which only
hits BIOS instead (and it's debateable what is better).

 Security: don't you want CAP_RAW_IO or something before executing any of
 these ioctls?

Perhaps it's mode 600 in their distro...

 bug: you can't hold a spinlock while you are sleeping in copy_from_user

Yep... I meant to check for it but forgot.

-- Pete
-
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] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Bogdan Costescu

On Fri, 1 Jun 2001, Pete Zaitcev wrote:

  But, each time a user cats this proc file, the user is banging the
  hardware.  What happens when a malicious user forks off 100 processes to
  continually cat this file?  :)

 Nothing good, probably. Same story as /proc/apm, which only
 hits BIOS instead (and it's debateable what is better).

Hmm, the MII related ioctl's in some net drivers (checked for 3c59x,
tulip, eepro100) are also querying the hardware. And a user is allowed to
ask for this info (but not able to modify it).

Sincerely,

Bogdan Costescu

IWR - Interdisziplinaeres Zentrum fuer Wissenschaftliches Rechnen
Universitaet Heidelberg, INF 368, D-69120 Heidelberg, GERMANY
Telephone: +49 6221 54 8869, Telefax: +49 6221 54 8868
E-mail: [EMAIL PROTECTED]

-
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] support for Cobalt Networks (x86 only) systems (for real this time)

2001-06-01 Thread Tim Hockin

Pete Zaitcev wrote:

  i2c is only in our stuff because the i2c core is not in the standard kernel
  yet.  As soon as it is, I will make cobalt_i2c* go away.
 
 I am puzzled by this comment. Did you look into drivers/i2c/?
 It certainly is a part of a stock kernel. The main user is
 the V4L, in drivers/media/video, but I think LM sensors use it too.

sorry, I meant to say:  The core is in, but the drivers for the adapters in
question are not.  They are part of lm_sensors, and as such, make it very
hard for us to maintain.  I have encouraged the lm_sensors crew to get at
LEAST the adapters/algorithms submitted for general inclusion.  Once that
is in, I will make cobalt_i2c go away.

Tim
-- 
Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
[EMAIL PROTECTED]
-
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] support for Cobalt Networks (x86 only) systems (for real this time)

2001-05-31 Thread Pete Zaitcev

> Aattached is a (large, but self contained) patch for Cobalt Networks suport
> for x86 systems (RaQ3, RaQ4, Qube3, RaQXTR).  Please let me know if there
> is anything that would prevent this from general inclusion in the next
> release.

Looks interesting. Seemingly literate use of spinlocks.

Off-hand I see old style initialization. Is it right for new driver?

i2c framework is not used, I wonder why. Someone thought that
it was too heavy perhaps? If so, I disagree.

Also, I am curious
if any alignment with lm-sensors is possible, for the sake of
common userland tools? If we managed that, PSARC would eat their
hearts out - they tried to do it since E-250 shipped.

lcd_read bounces reads with -EINVAL when another read is in
progress. Gross.

Nitpicking:

1.:
p = head;
while (p) {
p = p->next;
}

It is what for(;;) does.

2. Spaces and tabs are mixed in funny ways, makes to cute effects
when quoting diffs.

-- Pete
-
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/



[PATCH] support for Cobalt Networks (x86 only) systems (for real this time)

2001-05-31 Thread Tim Hockin

apparently, LKML silently (!) bounces messages > a certain size.  So I'll
try smaller patches.  This is part 2/2 of the general Cobalt support.

Alan,

Aattached is a (large, but self contained) patch for Cobalt Networks suport
for x86 systems (RaQ3, RaQ4, Qube3, RaQXTR).  Please let me know if there
is anything that would prevent this from general inclusion in the next
release.

(patch against 2.4.5)

Thanks

Tim
-- 
Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
[EMAIL PROTECTED]

diff -ruN dist-2.4.5/drivers/cobalt/README cobalt-2.4.5/drivers/cobalt/README
--- dist-2.4.5/drivers/cobalt/READMEWed Dec 31 16:00:00 1969
+++ cobalt-2.4.5/drivers/cobalt/README  Thu May 31 14:32:15 2001
@@ -0,0 +1,19 @@
+Notes on Cobalt's drivers:
+
+You will notice in several places constructs such as this:
+
+   if (cobt_is_3k()) {
+   foo();
+   } else if (cobt_is_5k()) {
+   bar();
+   }
+
+The goal here is to only compile in code that is needed, but to allow one to
+define support for both 3k and 5k (and more?) style systems.  The systype
+check macros are very simple and clean.  They check whether config-time
+support for the generation has been enabled, and (if so) whether systype
+detected the specified generation.  This leaves the code free from #ifdef
+cruft, but lets the compiler throw out unsupported generation-specific code
+with if (0) detection.
+
+--
diff -ruN dist-2.4.5/drivers/cobalt/ruler.c cobalt-2.4.5/drivers/cobalt/ruler.c
--- dist-2.4.5/drivers/cobalt/ruler.c   Wed Dec 31 16:00:00 1969
+++ cobalt-2.4.5/drivers/cobalt/ruler.c Thu May 31 14:32:15 2001
@@ -0,0 +1,393 @@
+/* 
+ * cobalt ruler driver 
+ * Copyright (c) 2000, Cobalt Networks, Inc.
+ * $Id: ruler.c,v 1.10 2001/05/30 07:19:48 thockin Exp $
+ *
+ * author: [EMAIL PROTECTED], [EMAIL PROTECTED]
+ *
+ * This should be SMP safe.  There are two critical pieces of data, and thsu
+ * two locks.  The ruler_lock protects the arrays of channels(hwifs) and
+ * busproc function pointers.  These are only ever written in the
+ * register/unregister functions but read in several other places.  A
+ * read/write lock is appropriate.  The second lock is the lock on the sled
+ * led state and the I2C_DEV_RULER.  It gets called from timer context, so
+ * irqsave it. The global switches and sled_leds are atomic_t. --TPH
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define RULER_TIMEOUT  (HZ >> 1)  /* .5s */
+#define MAX_COBT_DRIVES4
+#define LED_SLED0  (1 << 3)
+#define LED_SLED1  (1 << 2)
+#define LED_SLED2  (1 << 1)
+#define LED_SLED3  (1 << 0)
+
+/* all of this is for gen V */
+static struct timer_list cobalt_ruler_timer;
+static rwlock_t ruler_lock = RW_LOCK_UNLOCKED;
+static spinlock_t rled_lock = SPIN_LOCK_UNLOCKED;
+static ide_hwif_t *channels[MAX_COBT_DRIVES];
+static ide_busproc_t *busprocs[MAX_COBT_DRIVES];
+/* NOTE: switches is a bitmask of DETACHED sleds */
+static atomic_t switches = ATOMIC_INIT(0); 
+static atomic_t sled_leds = ATOMIC_INIT(0);
+static int sled_led_map[] = {LED_SLED0, LED_SLED1, LED_SLED2, LED_SLED3};
+static int ruler_detect;
+
+static inline u8
+read_switches(void)
+{
+   u8 state = 0;
+   if (cobt_is_5k()) {
+   int tries = 3;
+
+   /* i2c can be busy, and this can read wrong - try a few times */
+   while (tries--) {
+   state = cobalt_i2c_read_byte(COBALT_I2C_DEV_DRV_SWITCH, 
+   0);
+   if ((state & 0xf0) != 0xf0) {
+   break;
+   }
+   }
+   }
+
+   return state;
+}
+
+/*
+ * deal with sled leds: LED on means OK to remove
+ * NOTE: all the reset lines are kept high. 
+ * NOTE: the reset lines are in the reverse order of the switches. 
+ */
+static void
+set_sled_leds(u8 leds)
+{
+   if (cobt_is_5k()) {
+   unsigned long flags;
+
+   spin_lock_irqsave(_lock, flags);
+
+   atomic_set(_leds, leds);
+   leds |= 0xf0;
+   cobalt_i2c_write_byte(COBALT_I2C_DEV_RULER, 0, leds);
+
+   spin_unlock_irqrestore(_lock, flags);
+   }
+}
+
+static inline u8
+get_sled_leds(void)
+{
+   return atomic_read(_leds);
+}
+
+/* this must be called with the ruler_lock held for read */
+static int
+do_busproc(int idx, ide_hwif_t *hwif, int arg)
+{
+   if (cobt_is_5k()) {
+   /* sed sled LEDs */
+   switch (arg) {
+   case BUSSTATE_ON:
+   set_sled_leds(get_sled_leds() & 
+   ~sled_led_map[idx]);
+  

[PATCH] support for Cobalt Networks (x86 only) systems (for real this time)

2001-05-31 Thread Tim Hockin

apparently, LKML silently (!) bounces messages > a certain size.  So I'll
try smaller patches.  This is part 1/2 of the general Cobalt support.

Alan,

Aattached is a (large, but self contained) patch for Cobalt Networks suport
for x86 systems (RaQ3, RaQ4, Qube3, RaQXTR).  Please let me know if there
is anything that would prevent this from general inclusion in the next
release.

(patch against 2.4.5)

Thanks

Tim
-- 
Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
[EMAIL PROTECTED]

diff -ruN dist-2.4.5/drivers/cobalt/acpi.c cobalt-2.4.5/drivers/cobalt/acpi.c
--- dist-2.4.5/drivers/cobalt/acpi.cWed Dec 31 16:00:00 1969
+++ cobalt-2.4.5/drivers/cobalt/acpi.c  Thu May 31 14:32:15 2001
@@ -0,0 +1,218 @@
+/* 
+ * cobalt acpi driver 
+ * Copyright (c) 2000, Cobalt Networks, Inc.
+ * Copyright (c) 2001, Sun Microsystems, Inc.
+ * $Id: acpi.c,v 1.10 2001/05/30 07:19:47 thockin Exp $
+ *
+ * author: [EMAIL PROTECTED], [EMAIL PROTECTED]
+ *
+ * this driver just sets stuff up for ACPI interrupts
+ *
+ * if acpi support really existed in the kernel, we would read
+ * data from the ACPI tables. however, it doesn't. as a result,
+ * we use some hardcoded values. 
+ *
+ * This should be SMP safe.  The only data that needs protection is the acpi
+ * handler list.  It gets scanned at timer-interrupts, must use
+ * irqsave/restore locks. Read/write locks would be useful if there were any
+ * other times that the list was read but never written. --TPH
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define POWER_BUTTON_SHUTDOWN 0
+
+#define ACPI_IRQ   10   /* XXX: hardcoded interrupt */
+#define ACPI_NAME  "sci"
+#define ACPI_MAGIC 0xc0b7ac21
+
+#define SUPERIO_EVENT  0xff
+#define OSB4_EVENT 0x40
+#define OSB4_INDEX_PORT0x0cd6
+#define OSB4_DATA_PORT 0x0cd7
+
+/* for registering ACPI handlers */
+struct acpi_handler {
+   void (*function)(int irq, void *dev_id, struct pt_regs *regs);
+   struct acpi_handler *next;
+   struct acpi_handler *prev;
+};
+struct acpi_handler *acpi_handler_list;
+
+static spinlock_t acpi_lock = SPIN_LOCK_UNLOCKED;
+/* these two are for gen V */
+static u16 osb4_port;
+static u16 superio_port;
+
+static u16 
+get_reg(u16 index, u16 data, u8 port)
+{
+   if (cobt_is_5k()) {
+   u16 reg;
+
+   outb(port, index);
+   reg = inb(data);
+   outb(port + 1, index);
+   reg |= inb(data) << 8;
+   return reg;
+   }
+
+   return 0;
+}
+
+static void 
+acpi_interrupt(int irq, void *dev_id, struct pt_regs *regs)
+{
+   unsigned long flags, events;
+   struct acpi_handler *p;
+
+   spin_lock_irqsave(_lock, flags);
+
+   if (cobt_is_5k()) {
+   /* save the superio events */
+   events = inb(superio_port) | (inb(superio_port + 1) << 8);
+   
+   /* clear SCI interrupt generation */
+   outb(OSB4_EVENT, osb4_port); 
+   outb(SUPERIO_EVENT, superio_port);
+   outb(SUPERIO_EVENT, superio_port + 1);
+   }
+
+   /* call the ACPI handlers */
+   p = acpi_handler_list;
+   while (p) {
+   p->function(irq, dev_id, regs);
+   p = p->next;
+   }
+
+   spin_unlock_irqrestore(_lock, flags);
+}
+
+int
+cobalt_acpi_register_handler(void (*function)(int, void *, struct pt_regs *))
+{
+   struct acpi_handler *newh;
+   unsigned long flags;
+
+   newh = kmalloc(sizeof(*newh), GFP_ATOMIC);
+   if (!newh) {
+   EPRINTK("can't allocate memory for handler %p\n", function);
+   return -1;
+   }
+
+   spin_lock_irqsave(_lock, flags);
+
+   /* head insert */
+   newh->function = function;
+   newh->next = acpi_handler_list;
+   newh->prev = NULL;
+   if (acpi_handler_list) {
+   acpi_handler_list->prev = newh;
+   }
+   acpi_handler_list = newh;   
+
+   spin_unlock_irqrestore(_lock, flags);
+
+   return 0;
+}
+
+int
+cobalt_acpi_unregister_handler(void (*function)(int, void *, struct pt_regs *))
+{
+   struct acpi_handler *p;
+   unsigned long flags;
+   int r = -1;
+
+   spin_lock_irqsave(_lock, flags);
+
+   p = acpi_handler_list;
+   while (p) {
+   if (p->function == function) {
+   if (p->prev) {
+   p->prev->next = p->next;
+   }
+   if (p->next) {
+   p->next->prev = p->prev;
+   }
+   r = 0;
+   break;
+   }
+   p = p->next;
+   }
+
+   spin_unlock_irqrestore(_lock, flags);
+
+   return r;
+}
+
+int __init 
+cobalt_acpi_init(void)
+{
+ 

[PATCH] support for Cobalt Networks (x86 only) systems (for real this time)

2001-05-31 Thread Tim Hockin

apparently, LKML silently (!) bounces messages  a certain size.  So I'll
try smaller patches.  This is part 2/2 of the general Cobalt support.

Alan,

Aattached is a (large, but self contained) patch for Cobalt Networks suport
for x86 systems (RaQ3, RaQ4, Qube3, RaQXTR).  Please let me know if there
is anything that would prevent this from general inclusion in the next
release.

(patch against 2.4.5)

Thanks

Tim
-- 
Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
[EMAIL PROTECTED]

diff -ruN dist-2.4.5/drivers/cobalt/README cobalt-2.4.5/drivers/cobalt/README
--- dist-2.4.5/drivers/cobalt/READMEWed Dec 31 16:00:00 1969
+++ cobalt-2.4.5/drivers/cobalt/README  Thu May 31 14:32:15 2001
@@ -0,0 +1,19 @@
+Notes on Cobalt's drivers:
+
+You will notice in several places constructs such as this:
+
+   if (cobt_is_3k()) {
+   foo();
+   } else if (cobt_is_5k()) {
+   bar();
+   }
+
+The goal here is to only compile in code that is needed, but to allow one to
+define support for both 3k and 5k (and more?) style systems.  The systype
+check macros are very simple and clean.  They check whether config-time
+support for the generation has been enabled, and (if so) whether systype
+detected the specified generation.  This leaves the code free from #ifdef
+cruft, but lets the compiler throw out unsupported generation-specific code
+with if (0) detection.
+
+--
diff -ruN dist-2.4.5/drivers/cobalt/ruler.c cobalt-2.4.5/drivers/cobalt/ruler.c
--- dist-2.4.5/drivers/cobalt/ruler.c   Wed Dec 31 16:00:00 1969
+++ cobalt-2.4.5/drivers/cobalt/ruler.c Thu May 31 14:32:15 2001
@@ -0,0 +1,393 @@
+/* 
+ * cobalt ruler driver 
+ * Copyright (c) 2000, Cobalt Networks, Inc.
+ * $Id: ruler.c,v 1.10 2001/05/30 07:19:48 thockin Exp $
+ *
+ * author: [EMAIL PROTECTED], [EMAIL PROTECTED]
+ *
+ * This should be SMP safe.  There are two critical pieces of data, and thsu
+ * two locks.  The ruler_lock protects the arrays of channels(hwifs) and
+ * busproc function pointers.  These are only ever written in the
+ * register/unregister functions but read in several other places.  A
+ * read/write lock is appropriate.  The second lock is the lock on the sled
+ * led state and the I2C_DEV_RULER.  It gets called from timer context, so
+ * irqsave it. The global switches and sled_leds are atomic_t. --TPH
+ */
+
+#include stdarg.h
+#include stddef.h
+#include linux/init.h
+#include linux/sched.h
+#include linux/timer.h
+#include linux/config.h
+#include linux/pci.h
+#include linux/proc_fs.h
+#include linux/sched.h
+#include linux/ioport.h
+#include linux/ide.h
+#include linux/hdreg.h
+#include linux/notifier.h
+#include linux/sysctl.h
+#include linux/reboot.h
+#include linux/delay.h
+#include linux/ide.h
+#include asm/io.h
+#include linux/cobalt.h
+#include linux/cobalt-systype.h
+#include linux/cobalt-i2c.h
+#include linux/cobalt-acpi.h
+#include linux/cobalt-led.h
+
+#define RULER_TIMEOUT  (HZ  1)  /* .5s */
+#define MAX_COBT_DRIVES4
+#define LED_SLED0  (1  3)
+#define LED_SLED1  (1  2)
+#define LED_SLED2  (1  1)
+#define LED_SLED3  (1  0)
+
+/* all of this is for gen V */
+static struct timer_list cobalt_ruler_timer;
+static rwlock_t ruler_lock = RW_LOCK_UNLOCKED;
+static spinlock_t rled_lock = SPIN_LOCK_UNLOCKED;
+static ide_hwif_t *channels[MAX_COBT_DRIVES];
+static ide_busproc_t *busprocs[MAX_COBT_DRIVES];
+/* NOTE: switches is a bitmask of DETACHED sleds */
+static atomic_t switches = ATOMIC_INIT(0); 
+static atomic_t sled_leds = ATOMIC_INIT(0);
+static int sled_led_map[] = {LED_SLED0, LED_SLED1, LED_SLED2, LED_SLED3};
+static int ruler_detect;
+
+static inline u8
+read_switches(void)
+{
+   u8 state = 0;
+   if (cobt_is_5k()) {
+   int tries = 3;
+
+   /* i2c can be busy, and this can read wrong - try a few times */
+   while (tries--) {
+   state = cobalt_i2c_read_byte(COBALT_I2C_DEV_DRV_SWITCH, 
+   0);
+   if ((state  0xf0) != 0xf0) {
+   break;
+   }
+   }
+   }
+
+   return state;
+}
+
+/*
+ * deal with sled leds: LED on means OK to remove
+ * NOTE: all the reset lines are kept high. 
+ * NOTE: the reset lines are in the reverse order of the switches. 
+ */
+static void
+set_sled_leds(u8 leds)
+{
+   if (cobt_is_5k()) {
+   unsigned long flags;
+
+   spin_lock_irqsave(rled_lock, flags);
+
+   atomic_set(sled_leds, leds);
+   leds |= 0xf0;
+   cobalt_i2c_write_byte(COBALT_I2C_DEV_RULER, 0, leds);
+
+   spin_unlock_irqrestore(rled_lock, flags);
+   }
+}
+
+static inline u8
+get_sled_leds(void)
+{
+   return atomic_read(sled_leds);
+}
+
+/* this must be called with the ruler_lock held for read */
+static int
+do_busproc(int idx, 

[PATCH] support for Cobalt Networks (x86 only) systems (for real this time)

2001-05-31 Thread Tim Hockin

apparently, LKML silently (!) bounces messages  a certain size.  So I'll
try smaller patches.  This is part 1/2 of the general Cobalt support.

Alan,

Aattached is a (large, but self contained) patch for Cobalt Networks suport
for x86 systems (RaQ3, RaQ4, Qube3, RaQXTR).  Please let me know if there
is anything that would prevent this from general inclusion in the next
release.

(patch against 2.4.5)

Thanks

Tim
-- 
Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
[EMAIL PROTECTED]

diff -ruN dist-2.4.5/drivers/cobalt/acpi.c cobalt-2.4.5/drivers/cobalt/acpi.c
--- dist-2.4.5/drivers/cobalt/acpi.cWed Dec 31 16:00:00 1969
+++ cobalt-2.4.5/drivers/cobalt/acpi.c  Thu May 31 14:32:15 2001
@@ -0,0 +1,218 @@
+/* 
+ * cobalt acpi driver 
+ * Copyright (c) 2000, Cobalt Networks, Inc.
+ * Copyright (c) 2001, Sun Microsystems, Inc.
+ * $Id: acpi.c,v 1.10 2001/05/30 07:19:47 thockin Exp $
+ *
+ * author: [EMAIL PROTECTED], [EMAIL PROTECTED]
+ *
+ * this driver just sets stuff up for ACPI interrupts
+ *
+ * if acpi support really existed in the kernel, we would read
+ * data from the ACPI tables. however, it doesn't. as a result,
+ * we use some hardcoded values. 
+ *
+ * This should be SMP safe.  The only data that needs protection is the acpi
+ * handler list.  It gets scanned at timer-interrupts, must use
+ * irqsave/restore locks. Read/write locks would be useful if there were any
+ * other times that the list was read but never written. --TPH
+ */
+
+#include stdarg.h
+#include stddef.h
+#include linux/init.h
+#include linux/sched.h
+#include linux/config.h
+#include linux/pci.h
+#include linux/proc_fs.h
+#include linux/sched.h
+#include linux/ioport.h
+#include linux/delay.h
+#include linux/spinlock.h
+#include asm/io.h
+#include asm/irq.h
+#include linux/cobalt.h
+#include linux/cobalt-systype.h
+#include linux/cobalt-acpi.h
+#include linux/cobalt-superio.h
+
+#define POWER_BUTTON_SHUTDOWN 0
+
+#define ACPI_IRQ   10   /* XXX: hardcoded interrupt */
+#define ACPI_NAME  sci
+#define ACPI_MAGIC 0xc0b7ac21
+
+#define SUPERIO_EVENT  0xff
+#define OSB4_EVENT 0x40
+#define OSB4_INDEX_PORT0x0cd6
+#define OSB4_DATA_PORT 0x0cd7
+
+/* for registering ACPI handlers */
+struct acpi_handler {
+   void (*function)(int irq, void *dev_id, struct pt_regs *regs);
+   struct acpi_handler *next;
+   struct acpi_handler *prev;
+};
+struct acpi_handler *acpi_handler_list;
+
+static spinlock_t acpi_lock = SPIN_LOCK_UNLOCKED;
+/* these two are for gen V */
+static u16 osb4_port;
+static u16 superio_port;
+
+static u16 
+get_reg(u16 index, u16 data, u8 port)
+{
+   if (cobt_is_5k()) {
+   u16 reg;
+
+   outb(port, index);
+   reg = inb(data);
+   outb(port + 1, index);
+   reg |= inb(data)  8;
+   return reg;
+   }
+
+   return 0;
+}
+
+static void 
+acpi_interrupt(int irq, void *dev_id, struct pt_regs *regs)
+{
+   unsigned long flags, events;
+   struct acpi_handler *p;
+
+   spin_lock_irqsave(acpi_lock, flags);
+
+   if (cobt_is_5k()) {
+   /* save the superio events */
+   events = inb(superio_port) | (inb(superio_port + 1)  8);
+   
+   /* clear SCI interrupt generation */
+   outb(OSB4_EVENT, osb4_port); 
+   outb(SUPERIO_EVENT, superio_port);
+   outb(SUPERIO_EVENT, superio_port + 1);
+   }
+
+   /* call the ACPI handlers */
+   p = acpi_handler_list;
+   while (p) {
+   p-function(irq, dev_id, regs);
+   p = p-next;
+   }
+
+   spin_unlock_irqrestore(acpi_lock, flags);
+}
+
+int
+cobalt_acpi_register_handler(void (*function)(int, void *, struct pt_regs *))
+{
+   struct acpi_handler *newh;
+   unsigned long flags;
+
+   newh = kmalloc(sizeof(*newh), GFP_ATOMIC);
+   if (!newh) {
+   EPRINTK(can't allocate memory for handler %p\n, function);
+   return -1;
+   }
+
+   spin_lock_irqsave(acpi_lock, flags);
+
+   /* head insert */
+   newh-function = function;
+   newh-next = acpi_handler_list;
+   newh-prev = NULL;
+   if (acpi_handler_list) {
+   acpi_handler_list-prev = newh;
+   }
+   acpi_handler_list = newh;   
+
+   spin_unlock_irqrestore(acpi_lock, flags);
+
+   return 0;
+}
+
+int
+cobalt_acpi_unregister_handler(void (*function)(int, void *, struct pt_regs *))
+{
+   struct acpi_handler *p;
+   unsigned long flags;
+   int r = -1;
+
+   spin_lock_irqsave(acpi_lock, flags);
+
+   p = acpi_handler_list;
+   while (p) {
+   if (p-function == function) {
+   if (p-prev) {
+   p-prev-next = p-next;
+   }
+   if (p-next) {
+   p-next-prev = p-prev;
+   }
+  

Re: [PATCH] support for Cobalt Networks (x86 only) systems (for real this time)

2001-05-31 Thread Pete Zaitcev

 Aattached is a (large, but self contained) patch for Cobalt Networks suport
 for x86 systems (RaQ3, RaQ4, Qube3, RaQXTR).  Please let me know if there
 is anything that would prevent this from general inclusion in the next
 release.

Looks interesting. Seemingly literate use of spinlocks.

Off-hand I see old style initialization. Is it right for new driver?

i2c framework is not used, I wonder why. Someone thought that
it was too heavy perhaps? If so, I disagree.

Also, I am curious
if any alignment with lm-sensors is possible, for the sake of
common userland tools? If we managed that, PSARC would eat their
hearts out - they tried to do it since E-250 shipped.

lcd_read bounces reads with -EINVAL when another read is in
progress. Gross.

Nitpicking:

1.:
p = head;
while (p) {
p = p-next;
}

It is what for(;;) does.

2. Spaces and tabs are mixed in funny ways, makes to cute effects
when quoting diffs.

-- Pete
-
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/