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