MII access (was [PATCH] support for Cobalt Networks (x86 only)systems)
[ As this is becoming more and more MII specific, I changed the subject ] On Sat, 2 Jun 2001, Alan Cox wrote: > > This only answered the first part of the question: when. How do you pass > > the "how long" info ? > > Does the same applies for the MII ioctl case ? > > The mtime tells you exactly that. Alan, please consider this situation: One application needs to poll link status with 1 second resolution. On a system where caching is done with an unknown cache expiring time, this application is sometimes fed incorrect data. So, you need a way to tell for how long this situation lasts. If you have a proc/ioctl interface for setting cache expiring time, this same interface can then be used for reading back this info. This application can then check that this value is lower than 1 second and if not, notify the user that it cannot run. As this thread started as a general hardware access problem, would only _one_ value for all these cases be sufficient ? Or each case should have its own timeout ? Anyway, for MII, accessing the status at sub-second intervals might be a legit one, so what measuring units should be used? > I disagree. A non priviledged app should not be able to poke around in MII > registers anyway. So you only have to cache the generic state of the link. At the beginning of this thread, Jeff said "calling the ioctls without priveleges is quite useful". Now if you say that there is no such case, the whole problem could simply be solved by checking for the appropriate priviledges. I just realized another thing, important (IMHO) if a normal user is still allowed to access MII: the drivers (checked for 3c59x, eepro100, tulip) do not verify that the value passed for register number is within the allowed range and use it as: int read_cmd = (0xf6 << 10) | ((phy_id & 0x1f) << 5) | location; (phy_id is the MII address and location is the register number). There is also no check that the MII address specified is actually in use by the driver, but this is used with mii-diag to query a MII which was not correctly identified (maybe this should be allowed for CAP_NET_ADMIN only ?) >From one of Don Becker pages: "MII transceivers have 32 management registers. The first 16 are reserved for standard-defined uses, and the remaining one are available for chip-specific features. Only the first seven registers are currently defined." Usually, the transceivers return garbage if you read from locations you are not supposed to (overwritting phy_ad). But if you begin overwritting the READ command (0xf6 above)... Something like this should do: int read_cmd = (0xf6 << 10) | ((phy_id & 0x1f) << 5) | (location & 0x1f); > You don't need timers. Too tired to think straight yesterday... You're right. And if you alloc 32*sizeof(int) (you want to keep jiffies, right ?) per netdevice, I think that it could even be done outside the driver. Hmm, most of my previous arguments are no longer valid 8-( -- 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 (forrealthis
On Fri, 1 Jun 2001, Bogdan Costescu wrote: > On Fri, 1 Jun 2001, jamal wrote: > > > One idea i have been toying with is to maintain hysteris or threshold of > > some form in dev_watchdog; > > AFAIK, dev_watchdog is right now used only for Tx (if I'm wrong, please > correct me!). So how do you sense link loss if you expect only high Rx > traffic ? > Good question. Makes me think. Thoughts further below. > > example: if watchdog timer expires threshold times, you declare the link > > dead and send netif_carrier_off netlink message. > > On recovery, you send netif_carrier_on > > I assume that you mean "on recovery" as in "first succesful hard_start_xmit". > right. > > Assumption: > > If the tx path is blocked, more than likely the link is down. > > Yes, but is this a good approximation ? I'm not saying that it's not, I'm > merely asking for counter-arguments. It is an indirect approximation. Note that if the system data is very asymetrical as in the case you pointed out, notification will take a long long time. You need a plan B. Still, the tx watchdogs are a good source of fault detection in the case of non-availabilty of MII detection and even with the presence of MII. I hate making this more complex than it should be: Since we already have a messaging system within the kernel and user<->kernel space aka "netlink" -- one could easily add a protocol in user space which "dynamically heartbeats" the devices. Control should come from user space; it would be a great idea to avoid ioctls. "Dynamic" in the above sense means trying to totaly avoid making it a synchronous poll. The poll rate is a function of how many packets go out that device per average measurement time. Basically, the period that the user space app dumps "hello" netlink packets to the kernel is a variable. cheers, jamal - 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/
MII access (was [PATCH] support for Cobalt Networks (x86 only)systems)
[ As this is becoming more and more MII specific, I changed the subject ] On Sat, 2 Jun 2001, Alan Cox wrote: This only answered the first part of the question: when. How do you pass the how long info ? Does the same applies for the MII ioctl case ? The mtime tells you exactly that. Alan, please consider this situation: One application needs to poll link status with 1 second resolution. On a system where caching is done with an unknown cache expiring time, this application is sometimes fed incorrect data. So, you need a way to tell for how long this situation lasts. If you have a proc/ioctl interface for setting cache expiring time, this same interface can then be used for reading back this info. This application can then check that this value is lower than 1 second and if not, notify the user that it cannot run. As this thread started as a general hardware access problem, would only _one_ value for all these cases be sufficient ? Or each case should have its own timeout ? Anyway, for MII, accessing the status at sub-second intervals might be a legit one, so what measuring units should be used? I disagree. A non priviledged app should not be able to poke around in MII registers anyway. So you only have to cache the generic state of the link. At the beginning of this thread, Jeff said calling the ioctls without priveleges is quite useful. Now if you say that there is no such case, the whole problem could simply be solved by checking for the appropriate priviledges. I just realized another thing, important (IMHO) if a normal user is still allowed to access MII: the drivers (checked for 3c59x, eepro100, tulip) do not verify that the value passed for register number is within the allowed range and use it as: int read_cmd = (0xf6 10) | ((phy_id 0x1f) 5) | location; (phy_id is the MII address and location is the register number). There is also no check that the MII address specified is actually in use by the driver, but this is used with mii-diag to query a MII which was not correctly identified (maybe this should be allowed for CAP_NET_ADMIN only ?) From one of Don Becker pages: MII transceivers have 32 management registers. The first 16 are reserved for standard-defined uses, and the remaining one are available for chip-specific features. Only the first seven registers are currently defined. Usually, the transceivers return garbage if you read from locations you are not supposed to (overwritting phy_ad). But if you begin overwritting the READ command (0xf6 above)... Something like this should do: int read_cmd = (0xf6 10) | ((phy_id 0x1f) 5) | (location 0x1f); You don't need timers. Too tired to think straight yesterday... You're right. And if you alloc 32*sizeof(int) (you want to keep jiffies, right ?) per netdevice, I think that it could even be done outside the driver. Hmm, most of my previous arguments are no longer valid 8-( -- 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 (forrealthis
On Fri, 1 Jun 2001, Bogdan Costescu wrote: On Fri, 1 Jun 2001, jamal wrote: One idea i have been toying with is to maintain hysteris or threshold of some form in dev_watchdog; AFAIK, dev_watchdog is right now used only for Tx (if I'm wrong, please correct me!). So how do you sense link loss if you expect only high Rx traffic ? Good question. Makes me think. Thoughts further below. example: if watchdog timer expires threshold times, you declare the link dead and send netif_carrier_off netlink message. On recovery, you send netif_carrier_on I assume that you mean on recovery as in first succesful hard_start_xmit. right. Assumption: If the tx path is blocked, more than likely the link is down. Yes, but is this a good approximation ? I'm not saying that it's not, I'm merely asking for counter-arguments. It is an indirect approximation. Note that if the system data is very asymetrical as in the case you pointed out, notification will take a long long time. You need a plan B. Still, the tx watchdogs are a good source of fault detection in the case of non-availabilty of MII detection and even with the presence of MII. I hate making this more complex than it should be: Since we already have a messaging system within the kernel and user-kernel space aka netlink -- one could easily add a protocol in user space which dynamically heartbeats the devices. Control should come from user space; it would be a great idea to avoid ioctls. Dynamic in the above sense means trying to totaly avoid making it a synchronous poll. The poll rate is a function of how many packets go out that device per average measurement time. Basically, the period that the user space app dumps hello netlink packets to the kernel is a variable. cheers, jamal - 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
[ OK, this time I cc'ed netdev 8-) ] On Fri, 1 Jun 2001, Alan Cox wrote: > Please re-read your comment. Then think about it. Then tell me how rate > limiting differs from caching to the application. For caching, the kernel establishes the rate with which the info is updated. There's nothing wrong, but how is the application to know if the value is actual or cached (from when, until when) ? That means that a single application that needs data more often than the caching rate will get bogus data and not know about it. With rate limiting, you always get new values, unless the limit is exceeded. When the limit is exceeded, you log and: - block any request until some timer is expired. The application can detect that it's been blocked and react. You can detect if there are several calls waiting and return the same value to all. - return error until some timer is expired. The application can again detect that. In both cases, the application is also capable of guessing the value of the delay. For one application which follows the rules (doesn't need data more often than the caching rate or doesn't exceed the rate limit) there is no difference, I agree. But when somebody is playing tricks while you need data, you have the chance of detecting this by using rate limits. And yes, I agree that either of them (cache or rate limit) should be modifiable through proc entry/ioctl/whatever. -- 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
Alan Cox <[EMAIL PROTECTED]> [01/06/01 10:32]: > > No way! If I implement a HA application which depends on link status, I > > want the info to be accurate, I don't want to know that 30 seconds ago I > > had good link. > > > > IMHO, rate limiting is the only solution. > > Please re-read your comment. Then think about it. Then tell me how rate limiting > differs from caching to the application. > I'd argue for rate limiting as the application only gets back new data, never a cached value n times in a row. With caching, you'd have to let the application know when the cached value was last read and how long it will be cached for. With rate limiting, you'd just block the app and get the new data to the app once the timer has expired. Or, if the app doesn't read the data until after the timer has expired, it will not block at all. Seems to me that you'd have less redundant application processing with rate limiting. - 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
> I'd argue for rate limiting as the application only gets back new data, > never a cached value n times in a row. Which is worse. I cat the proc file a few times and your HA app is unlucky. It now gets *NO* data for five minutes. If we cache the values it gets approximate data - 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 (forrealthis
On Fri, 1 Jun 2001, jamal wrote: > Jeff, Thanks for copying netdev. Wish more people would do that. Shame on me, I should have thought of that too... I joined lkml only about 2 weeks ago because netdev related topics are sometimes discussed only there... > Not really. > > One idea i have been toying with is to maintain hysteris or threshold of > some form in dev_watchdog; AFAIK, dev_watchdog is right now used only for Tx (if I'm wrong, please correct me!). So how do you sense link loss if you expect only high Rx traffic ? > example: if watchdog timer expires threshold times, you declare the link > dead and send netif_carrier_off netlink message. > On recovery, you send netif_carrier_on I assume that you mean "on recovery" as in "first succesful hard_start_xmit". > Assumption: > If the tx path is blocked, more than likely the link is down. Yes, but is this a good approximation ? I'm not saying that it's not, I'm merely asking for counter-arguments. -- 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
> No way! If I implement a HA application which depends on link status, I > want the info to be accurate, I don't want to know that 30 seconds ago I > had good link. > > IMHO, rate limiting is the only solution. Please re-read your comment. Then think about it. Then tell me how rate limiting differs from caching to the application. Alan - 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 (forrealthis
Jeff, Thanks for copying netdev. Wish more people would do that. On Fri, 1 Jun 2001, Bogdan Costescu wrote: > On Fri, 1 Jun 2001, Jeff Garzik wrote: > > > The loss and regain of link status should be proactively signalled to > > userspace using netlink or something similar. > > [ For the general discussion ] > I fully agree, but I just wanted to give an example of legit use from > user space of _current_ values from hardware. > > > Currently we have > > netif_carrier_{on,off,ok} but it is only passively checked. > > netif_carrier_{on,off} should probably schedule_task() to fire off a > > netlink message... > > [ Link status details ] > Just that not all NICs have hardware support (and/or not all drivers use > these facilities) for link status change notification using interrupts. > Right now, most drivers _poll_ for media status and based on the poll > rate, netif_carrier routines are (or should be) called. We can't make the > poll rate very small for the general case, as MII access is time > consuming (same discussion was some months ago when the bonding driver > was updated). However, for users who know that they need this info to be > more accurate (at the expense of CPU time), polling through ioctl's is the > only solution. Not really. One idea i have been toying with is to maintain hysteris or threshold of some form in dev_watchdog; example: if watchdog timer expires threshold times, you declare the link dead and send netif_carrier_off netlink message. On recovery, you send netif_carrier_on Assumption: If the tx path is blocked, more than likely the link is down. cheers, jamal - 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 (forrealthis
On Fri, 1 Jun 2001, David S. Miller wrote: > Don't such HA apps need to run as root anyways? Not necessarily, but eventually you can let root (CAP_NET_ADMIN, anyway) go through without any limitations, root can bring down the system at will in other ways. In addition, the rate limiting solution allows a warning to be issued when the limit is exceeded, so that the poor sysadmin knows what hit him 8-) -- 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 (forrealthis
On Fri, 1 Jun 2001, Jeff Garzik wrote: > The loss and regain of link status should be proactively signalled to > userspace using netlink or something similar. [ For the general discussion ] I fully agree, but I just wanted to give an example of legit use from user space of _current_ values from hardware. > Currently we have > netif_carrier_{on,off,ok} but it is only passively checked. > netif_carrier_{on,off} should probably schedule_task() to fire off a > netlink message... [ Link status details ] Just that not all NICs have hardware support (and/or not all drivers use these facilities) for link status change notification using interrupts. Right now, most drivers _poll_ for media status and based on the poll rate, netif_carrier routines are (or should be) called. We can't make the poll rate very small for the general case, as MII access is time consuming (same discussion was some months ago when the bonding driver was updated). However, for users who know that they need this info to be more accurate (at the expense of CPU time), polling through ioctl's is the only solution. [ Back to general discussion ] So far, to the problem of too often access to hardware, 2 solutions were proposed: 1. cache the values. You can then let the user shoot him-/her-self in the foot by making too many ioctl calls. But this prevent any legit use of current hardware state. 2. rate limiting. You don't let the user access the hardware too often (to be defined), so he/she can't shoot his-/her-self in the foot. Legit use of current hardware state is possible. IMHO, solution 2 is much better. Can you find situations when it's not ? -- 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 (forrealthis
Jeff Garzik writes: > For your HA application specifically, right now, I would suggest making > sure your net driver calls netif_carrier_xxx correctly, then checking > for IFF_RUNNING interface flag. IFF_RUNNING will disappear if the > interface is up, but there is no carrier [as according to > netif_carrier_ok]. Don't such HA apps need to run as root anyways? Regardless, I agree that, long term, the way to do this is via netlink. Later, David S. Miller [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 (forrealthis
Bogdan Costescu wrote: > No way! If I implement a HA application which depends on link status, I > want the info to be accurate, I don't want to know that 30 seconds ago I > had good link. To tangent a little bit, and add netdev to the CC... The loss and regain of link status should be proactively signalled to userspace using netlink or something similar. Currently we have netif_carrier_{on,off,ok} but it is only passively checked. netif_carrier_{on,off} should probably schedule_task() to fire off a netlink message... For your HA application specifically, right now, I would suggest making sure your net driver calls netif_carrier_xxx correctly, then checking for IFF_RUNNING interface flag. IFF_RUNNING will disappear if the interface is up, but there is no carrier [as according to netif_carrier_ok]. -- 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 (forrealthis
On Fri, 1 Jun 2001, Alan Cox wrote: > I am sure that to an unpriviledged application reporting back the same result > as we saw last time we asked the hardware unless it is over 30 seconds old > will work fine. Maybe 10 for link partner ? No way! If I implement a HA application which depends on link status, I want the info to be accurate, I don't want to know that 30 seconds ago I had good link. IMHO, rate limiting is the only solution. -- 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 realthis
> Only some of them can be cached... (some of the MIIs in some drivers > are already cached, in fact) you can't cache stuff like what your link > partner is advertising at the moment, or what your battery status is at > the moment. I am sure that to an unpriviledged application reporting back the same result as we saw last time we asked the hardware unless it is over 30 seconds old will work fine. Maybe 10 for link partner ? - 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 realthis
Alan Cox wrote: > > In both of these situations, calling the ioctls without priveleges is > > quite useful, so maybe rate-limiting for ioctls and proc files like this > > would be a good idea in general. > Many of them (like the MII and APM ones) the result can be cached Only some of them can be cached... (some of the MIIs in some drivers are already cached, in fact) you can't cache stuff like what your link partner is advertising at the moment, or what your battery status is at the moment. -- 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 realthis
> In both of these situations, calling the ioctls without priveleges is > quite useful, so maybe rate-limiting for ioctls and proc files like this > would be a good idea in general. Many of them (like the MII and APM ones) the result can be cached - 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 realthis time)
Bogdan Costescu wrote: > > 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). And a malicious user calling those at a high rate is bound to get ugly. In both of these situations, calling the ioctls without priveleges is quite useful, so maybe rate-limiting for ioctls and proc files like this would be a good idea in general. Jeff -- 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)
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
> + /* setup osb4 i/o regions */ > + if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x20))) > + request_region(reg, 4, "OSB4 (pm1a_evt_blk)"); Check request_region worked > +static int > +i2c_wait_for_smi(void) Obvious question - why duplicate the i2c layer > +/* device structure */ > +static struct miscdevice lcd_dev = { > + COBALT_LCD_MINOR, Is this an officially allocated minor ? > + /* Get the current cursor position */ > +case LCD_Get_Cursor_Pos: > +display.cursor_address = lcddev_read_inst(); > +display.cursor_address = display.cursor_address & 0x07F; > +copy_to_user((struct lcd_display *)arg, , dlen); Sleeping holding a spinlock > +case LCD_Set_Cursor_Pos: > +copy_from_user(, (struct lcd_display *)arg, dlen); Ditto. Also should check the return and return -EFAULT if so > +/* LED daemon sits on this, we wake it up once a key is pressed */ > +static ssize_t > +cobalt_lcd_read(struct file *file, char *buf, size_t count, loff_t *ppos) > +{ > + int buttons_now; > + static atomic_t lcd_waiters = ATOMIC_INIT(0); > + > + if (atomic_read(_waiters) > 0) > + return -EINVAL; > + atomic_inc(_waiters); Unsafe. Two people can execute the atomic_read before anyone executes the atomic_inc. You probably want test_and_set_bit() or a semaphore > + while (((buttons_now = button_pressed()) == 0) && > +!(signal_pending(current))) > + { > + current->state = TASK_INTERRUPTIBLE; We have a set_ function for this.. ALso what stops an ioctl occuring at the same instant ? - 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 realthis time)
Bogdan Costescu wrote: 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). And a malicious user calling those at a high rate is bound to get ugly. In both of these situations, calling the ioctls without priveleges is quite useful, so maybe rate-limiting for ioctls and proc files like this would be a good idea in general. Jeff -- 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 realthis
Only some of them can be cached... (some of the MIIs in some drivers are already cached, in fact) you can't cache stuff like what your link partner is advertising at the moment, or what your battery status is at the moment. I am sure that to an unpriviledged application reporting back the same result as we saw last time we asked the hardware unless it is over 30 seconds old will work fine. Maybe 10 for link partner ? - 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
[ OK, this time I cc'ed netdev 8-) ] On Fri, 1 Jun 2001, Alan Cox wrote: Please re-read your comment. Then think about it. Then tell me how rate limiting differs from caching to the application. For caching, the kernel establishes the rate with which the info is updated. There's nothing wrong, but how is the application to know if the value is actual or cached (from when, until when) ? That means that a single application that needs data more often than the caching rate will get bogus data and not know about it. With rate limiting, you always get new values, unless the limit is exceeded. When the limit is exceeded, you log and: - block any request until some timer is expired. The application can detect that it's been blocked and react. You can detect if there are several calls waiting and return the same value to all. - return error until some timer is expired. The application can again detect that. In both cases, the application is also capable of guessing the value of the delay. For one application which follows the rules (doesn't need data more often than the caching rate or doesn't exceed the rate limit) there is no difference, I agree. But when somebody is playing tricks while you need data, you have the chance of detecting this by using rate limits. And yes, I agree that either of them (cache or rate limit) should be modifiable through proc entry/ioctl/whatever. -- 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
Alan Cox [EMAIL PROTECTED] [01/06/01 10:32]: No way! If I implement a HA application which depends on link status, I want the info to be accurate, I don't want to know that 30 seconds ago I had good link. IMHO, rate limiting is the only solution. Please re-read your comment. Then think about it. Then tell me how rate limiting differs from caching to the application. I'd argue for rate limiting as the application only gets back new data, never a cached value n times in a row. With caching, you'd have to let the application know when the cached value was last read and how long it will be cached for. With rate limiting, you'd just block the app and get the new data to the app once the timer has expired. Or, if the app doesn't read the data until after the timer has expired, it will not block at all. Seems to me that you'd have less redundant application processing with rate limiting. - 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 (forrealthis
Bogdan Costescu wrote: No way! If I implement a HA application which depends on link status, I want the info to be accurate, I don't want to know that 30 seconds ago I had good link. To tangent a little bit, and add netdev to the CC... The loss and regain of link status should be proactively signalled to userspace using netlink or something similar. Currently we have netif_carrier_{on,off,ok} but it is only passively checked. netif_carrier_{on,off} should probably schedule_task() to fire off a netlink message... For your HA application specifically, right now, I would suggest making sure your net driver calls netif_carrier_xxx correctly, then checking for IFF_RUNNING interface flag. IFF_RUNNING will disappear if the interface is up, but there is no carrier [as according to netif_carrier_ok]. -- 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 (forrealthis
Jeff Garzik writes: For your HA application specifically, right now, I would suggest making sure your net driver calls netif_carrier_xxx correctly, then checking for IFF_RUNNING interface flag. IFF_RUNNING will disappear if the interface is up, but there is no carrier [as according to netif_carrier_ok]. Don't such HA apps need to run as root anyways? Regardless, I agree that, long term, the way to do this is via netlink. Later, David S. Miller [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 (forrealthis
On Fri, 1 Jun 2001, Jeff Garzik wrote: The loss and regain of link status should be proactively signalled to userspace using netlink or something similar. [ For the general discussion ] I fully agree, but I just wanted to give an example of legit use from user space of _current_ values from hardware. Currently we have netif_carrier_{on,off,ok} but it is only passively checked. netif_carrier_{on,off} should probably schedule_task() to fire off a netlink message... [ Link status details ] Just that not all NICs have hardware support (and/or not all drivers use these facilities) for link status change notification using interrupts. Right now, most drivers _poll_ for media status and based on the poll rate, netif_carrier routines are (or should be) called. We can't make the poll rate very small for the general case, as MII access is time consuming (same discussion was some months ago when the bonding driver was updated). However, for users who know that they need this info to be more accurate (at the expense of CPU time), polling through ioctl's is the only solution. [ Back to general discussion ] So far, to the problem of too often access to hardware, 2 solutions were proposed: 1. cache the values. You can then let the user shoot him-/her-self in the foot by making too many ioctl calls. But this prevent any legit use of current hardware state. 2. rate limiting. You don't let the user access the hardware too often (to be defined), so he/she can't shoot his-/her-self in the foot. Legit use of current hardware state is possible. IMHO, solution 2 is much better. Can you find situations when it's not ? -- 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 (forrealthis
On Fri, 1 Jun 2001, David S. Miller wrote: Don't such HA apps need to run as root anyways? Not necessarily, but eventually you can let root (CAP_NET_ADMIN, anyway) go through without any limitations, root can bring down the system at will in other ways. In addition, the rate limiting solution allows a warning to be issued when the limit is exceeded, so that the poor sysadmin knows what hit him 8-) -- 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 (forrealthis
Jeff, Thanks for copying netdev. Wish more people would do that. On Fri, 1 Jun 2001, Bogdan Costescu wrote: On Fri, 1 Jun 2001, Jeff Garzik wrote: The loss and regain of link status should be proactively signalled to userspace using netlink or something similar. [ For the general discussion ] I fully agree, but I just wanted to give an example of legit use from user space of _current_ values from hardware. Currently we have netif_carrier_{on,off,ok} but it is only passively checked. netif_carrier_{on,off} should probably schedule_task() to fire off a netlink message... [ Link status details ] Just that not all NICs have hardware support (and/or not all drivers use these facilities) for link status change notification using interrupts. Right now, most drivers _poll_ for media status and based on the poll rate, netif_carrier routines are (or should be) called. We can't make the poll rate very small for the general case, as MII access is time consuming (same discussion was some months ago when the bonding driver was updated). However, for users who know that they need this info to be more accurate (at the expense of CPU time), polling through ioctl's is the only solution. Not really. One idea i have been toying with is to maintain hysteris or threshold of some form in dev_watchdog; example: if watchdog timer expires threshold times, you declare the link dead and send netif_carrier_off netlink message. On recovery, you send netif_carrier_on Assumption: If the tx path is blocked, more than likely the link is down. cheers, jamal - 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 (forrealthis
On Fri, 1 Jun 2001, Alan Cox wrote: I am sure that to an unpriviledged application reporting back the same result as we saw last time we asked the hardware unless it is over 30 seconds old will work fine. Maybe 10 for link partner ? No way! If I implement a HA application which depends on link status, I want the info to be accurate, I don't want to know that 30 seconds ago I had good link. IMHO, rate limiting is the only solution. -- 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
No way! If I implement a HA application which depends on link status, I want the info to be accurate, I don't want to know that 30 seconds ago I had good link. IMHO, rate limiting is the only solution. Please re-read your comment. Then think about it. Then tell me how rate limiting differs from caching to the application. Alan - 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
+ /* setup osb4 i/o regions */ + if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x20))) + request_region(reg, 4, OSB4 (pm1a_evt_blk)); Check request_region worked +static int +i2c_wait_for_smi(void) Obvious question - why duplicate the i2c layer +/* device structure */ +static struct miscdevice lcd_dev = { + COBALT_LCD_MINOR, Is this an officially allocated minor ? + /* Get the current cursor position */ +case LCD_Get_Cursor_Pos: +display.cursor_address = lcddev_read_inst(); +display.cursor_address = display.cursor_address 0x07F; +copy_to_user((struct lcd_display *)arg, display, dlen); Sleeping holding a spinlock +case LCD_Set_Cursor_Pos: +copy_from_user(display, (struct lcd_display *)arg, dlen); Ditto. Also should check the return and return -EFAULT if so +/* LED daemon sits on this, we wake it up once a key is pressed */ +static ssize_t +cobalt_lcd_read(struct file *file, char *buf, size_t count, loff_t *ppos) +{ + int buttons_now; + static atomic_t lcd_waiters = ATOMIC_INIT(0); + + if (atomic_read(lcd_waiters) 0) + return -EINVAL; + atomic_inc(lcd_waiters); Unsafe. Two people can execute the atomic_read before anyone executes the atomic_inc. You probably want test_and_set_bit() or a semaphore + while (((buttons_now = button_pressed()) == 0) +!(signal_pending(current))) + { + current-state = TASK_INTERRUPTIBLE; We have a set_ function for this.. ALso what stops an ioctl occuring at the same instant ? - 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 realthis
Alan Cox wrote: In both of these situations, calling the ioctls without priveleges is quite useful, so maybe rate-limiting for ioctls and proc files like this would be a good idea in general. Many of them (like the MII and APM ones) the result can be cached Only some of them can be cached... (some of the MIIs in some drivers are already cached, in fact) you can't cache stuff like what your link partner is advertising at the moment, or what your battery status is at the moment. -- 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 (forrealthis
On Fri, 1 Jun 2001, jamal wrote: Jeff, Thanks for copying netdev. Wish more people would do that. Shame on me, I should have thought of that too... I joined lkml only about 2 weeks ago because netdev related topics are sometimes discussed only there... Not really. One idea i have been toying with is to maintain hysteris or threshold of some form in dev_watchdog; AFAIK, dev_watchdog is right now used only for Tx (if I'm wrong, please correct me!). So how do you sense link loss if you expect only high Rx traffic ? example: if watchdog timer expires threshold times, you declare the link dead and send netif_carrier_off netlink message. On recovery, you send netif_carrier_on I assume that you mean on recovery as in first succesful hard_start_xmit. Assumption: If the tx path is blocked, more than likely the link is down. Yes, but is this a good approximation ? I'm not saying that it's not, I'm merely asking for counter-arguments. -- 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
I'd argue for rate limiting as the application only gets back new data, never a cached value n times in a row. Which is worse. I cat the proc file a few times and your HA app is unlucky. It now gets *NO* data for five minutes. If we cache the values it gets approximate data - 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 realthis
In both of these situations, calling the ioctls without priveleges is quite useful, so maybe rate-limiting for ioctls and proc files like this would be a good idea in general. Many of them (like the MII and APM ones) the result can be cached - 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 realthis time)
Tim Hockin said once upon a time (Thu, 31 May 2001): > 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. I can vouch for these patches stability wise. I've been running an earlier version of these patches (for 2.4.4) on a Qube 3 acting as a firewall for 3 weeks without any problems. Incidently, that Qube 3 is running Red Hat 7.1 and using reiserfs on all filesystems except for /boot. Dax - 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/
[PATCH] support for Cobalt Networks (x86 only) systems (for real this time)
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)
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) +{ +
RE: [PATCH] support for Cobalt Networks (x86 only) systems
> Aattached is a (large, but self contained) patch for Cobalt Networks suport Is not? ;) - 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
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] - 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
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] - 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
Aattached is a (large, but self contained) patch for Cobalt Networks suport Is not? ;) - 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)
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)
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)
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 realthis time)
Tim Hockin said once upon a time (Thu, 31 May 2001): 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. I can vouch for these patches stability wise. I've been running an earlier version of these patches (for 2.4.4) on a Qube 3 acting as a firewall for 3 weeks without any problems. Incidently, that Qube 3 is running Red Hat 7.1 and using reiserfs on all filesystems except for /boot. Dax - 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/