Re: General device properties API
Hi, > How do I know what properties are going to be available in a driver? > Is it part of an internal kernel<->kernel interface like device > properties, or just whatever is passed through the OF/FDT or ACPI > firmware? I think that we need a standard form for each type of information. The source would be the firmware, but MI code is responsible for converting that into the standard form for drivers to use. > If the latter, what happens if I want to use the same driver on a > system that might use OF/FDT or ACPI, where OF/FDT or ACPI provide the > same information by different spellings like `mac-address' vs > `ethernet-address' (hypothetical)? Or is this intended only for > drivers where the binding to a particular type of firmware device tree > is baked into the driver, so this kind of thing won't ever come up? We can decide on the name that we'll use (e.g. `ethernet_address') and use that in all the drivers. The driver might also need to query other properties, so there will have to be co-ordination between the code that reads the firmware (which properties to store?) and the device driver. An example might be GPIO's, which have a large number of hardware uses. We'll want to be able to read or overlay information about the pins from the firmware, and then to setup other subsystems from the driver, based on the information provided. A temporary solution is: https://nxr.netbsd.org/xref/src/sys/arch/sparc64/sparc64/ofw_patch.c#130 https://nxr.netbsd.org/xref/src/sys/dev/i2c/pcf8574.c#176 where I encoded information about the pins and (later in the device driver) attach sysmon or led. This should be done in a better (standardised) way because we need to do similar on other hardware. Again, there isn't a common way across hardware, so we'll have to make our own. Regards, Julian
Re: SCSI scanners
Hi, > If it's really the case that SANE works with these, then that seems ok. > (I actually have a UMAX scsi scsanner but haven't powered it on in > years.) The scanners that we support are also on the SANE project lists [*], although the driver is marked as unsupported. > I wonder though if this is causing the kind of trouble that uscanner > caused. It's possible to use either our driver or SANE, but not both in the same kernel. Our driver uses ss(4) and SANE uses uk(4), and uk(4) attaches to SCSI devices where no other driver has attached. However, at this point in time, I would guess either that everyone with one of these scanners is using it with ss(4) removed from their kernel configuration, or that no-one is using them any longer. Removing the driver removes the maintenance cost for us. Regards, Julian
SCSI scanners (Was: uscanner)
Hi all, Can we get rid of the SCSI scanner support as well? It only supports old HP and Mustek scanners, and its functionality is superseded by SANE (which sends the relevant SCSI commands from userland). Regards, Julian
IIC locking when shutting down
Hi all,
When testing some iic changes, we saw this panic on shutdown (on a 4 CPU
system):
[ 356534.4468099] Skipping crash dump on recursive panic
[ 356534.5093055] panic: lock error: Mutex: mutex_vector_exit,747: assertion
failed: MUTEX_OWNER(mtx->mtx_owner) == curthread: lock 0x1037806c8 cpu 0 lwp
0x1044e6e80
CPU 0 and 2 are both in the iic code (for backtraces, see below). I think
that I know why this happened. On CPU 0 we crash when we release the I2C
bus lock at:
https://nxr.netbsd.org/xref/src/sys/arch/sparc64/dev/pcf8591_envctrl.c#296
presumably because we didn't acquire it, because CPU 2 is running code that
acquired the lock:
https://nxr.netbsd.org/xref/src/sys/dev/i2c/lm75.c#352
If I'm reading the code correctly, this means that either we're either
polling or that the controller iic_acquire_bus() routine failed. As pcfiic
doesn't have an iic_acquire_bus() routine, we must be polling. Neither
ecadc nor lm75 (lmtemp) set the polling flag, and the sparc64 frontend
(pcfiic_ebus) only sets it if we don't have an interrupt, but I can see
interrupts in the node (/pci/ebus@1/SUNW,envctrl@14,60 on this E450).
However, we call iic_op_flags() at the start of ic_acquire_bus():
https://nxr.netbsd.org/xref/src/sys/dev/i2c/i2c_exec.c#62
so I think that what happened was:
- we set the polling flag in iic_op_flags() during shutdown
- iic_acquire_bus() acquired the mutex for the 1st caller (lmtemp)
- iic_acquire_bus() returned EBUSY for the 2nd caller (ecadc)
- we don't handle errors from iic_acquire_bus() in the driver code
- we continue and then call iic_release_bus()
- boom
It looks like every call to iic_acquire_bus() will need to handle the error
return. We seem to do this in a few drivers, but not all of them. However,
is there a simple solution that I'm missing?
Regards,
Julian
- - -
Backtraces:
db{0}> mach cpu 0
db{0}> bt
panic(1aa5210, 1a9d000, 19e79a8, 2eb, 1a9cfc0, 1037806c8) at netbsd:panic+0x20
lockdebug_abort(19e79a8, 2eb, 1037806c8, 1c606c0, 1a9cfc0, e0048000) at
netbsd:lockdebug_abort+0xa4
mutex_spin_exit(1037806c8, 1044e6e80, 0, 0, 0, 1cefa9c00) at
netbsd:mutex_spin_exit+0xc4
ecadc_refresh(103780a40, 1042aa8e0, e0048000, 0, 1c60c98, 1042aa5c0) at
netbsd:ecadc_refresh+0x98
sysmon_envsys_refresh_sensor(103780a40, 1042aa8e0, 1044e6e80, 1a65000,
1cc1000, 1044e6e80) at netbsd:sysmon_envsys_refresh_sensor+0x1c
sme_events_worker(103780b10, 103780a40, 1044e6e80, 1042aa8e0, 103780a40,
1044dce80) at netbsd:sme_events_worker+0x130
workqueue_worker(1037811c0, 103781218, 103781228, 103781208, 103781200, 0) at
netbsd:workqueue_worker+0xf8
lwp_trampoline(f0075a4c, 113800, 112f40, fff7fdf8, 0, fff7fc78) at
netbsd:lwp_trampoline+0x8
db{1}> mach cpu 2
db{2}> bt
pcfiic_xmit(103780680, 0, 1cefb1b38, 1, , 1) at
netbsd:pcfiic_xmit+0xa4
pcfiic_i2c_exec(103780680, 4d, 4d, 1, 1cefb1b40, 1cefb1b40) at
netbsd:pcfiic_i2c_exec+0x68
iic_exec(1037806c0, 1, 4d, 1cefb1b38, 1, 1cefb1b40) at netbsd:iic_exec+0x1a4
lmtemp_temp_read(104288200, 0, 1cefb1c0c, 0, 104288200, 1c60c98) at
netbsd:lmtemp_temp_read+0x40
lmtemp_refresh(103780cc0, 104288228, e0048000, 0, 1cee98000, 104288200) at
netbsd:lmtemp_refresh+0x20
sysmon_envsys_refresh_sensor(103780cc0, 104288228, 1044e7700, 1a65000,
1cc1000, 1044e7700) at netbsd:sysmon_envsys_refresh_sensor+0x1c
sme_events_worker(103780d90, 103780cc0, 1044e7700, 104288228, 103780cc0,
1044dd080) at netbsd:sme_events_worker+0x130
workqueue_worker(103781440, 103781498, 1037814a8, 103781488, 103781480, 0) at
netbsd:workqueue_worker+0xf8
lwp_trampoline(f0075a4c, 113800, 112f40, fff7fdf8, 0, fff7fc78) at
netbsd:lwp_trampoline+0x8
sysmon_envsys race
Hi all,
While testing changes to an envsys driver, I saw this crash on shutdown:
[ 1651.0108940] cpu0: data fault: pc=155ea68 rpc=101db8ca4 addr=0
[ 1651.0108940] kernel trap 30: data access exception
Stopped in pid 0.5 (system) at netbsd:mutex_oncpu.part.0+0x8: ldx
[%g1 + 0x18], %g2
db{0}> bt
sme_events_check(101db6718, 101d8a041, 0, 1c63348, 101db6640, 101d8a040) at
netbsd:sme_events_check+0xc
This is:
line 739 mutex_enter(>sme_work_mtx);
The driver runs sysmon_envsys_destroy() in its detach routine. Looking at
the code, it looks like that could race with sme_events_check() whilst the
sme sensors list is being removed - they both start by checking that
sme != NULL but sysmon_envsys_destroy() could remove the sme structure
whilst sme_events_check() is running. I'm guessing that's what happened
in the above case. Note, that I only saw this once in about 50 reboots,
so it's quite rare.
It seems sensible to take the sme_mtx in sysmon_envsys_destroy(), but
that just reduces the window - sme_events_check() checks sme != NULL and
the mutexs are part of the sme structure that we want to remove.
There is code in sysmon_envsys_sensor_detach() which removes callouts,
so a better solution might be to call sysmon_envsys_sensor_detach() from
sysmon_envsys_destroy(), or audit every driver to check that is done.
Any other solution appreciated.
Regards,
Julian
Re: New header for GPIO-like definitions
Hi,
> > Might I suggest before we go to deep down the rabbit hole that you take a
> > look at the GPIO FDT bindings?
> >
> >
> > https://github.com/devicetree-org/devicetree-source/blob/master/Bindings/gpio/gpio.txt
Looking at the bindings, there are definitions of polarity, which I need
but I don't see anything about the use of the pin, apart from the naming.
For example. the LEDs are all named "LED N". The attached patch provides
the information that I need, but perhaps it's perfereble to encode the
pin-specific information in strings? Following the FDT model, I could
use:
polarity="ACTIVE_LOW"
name="LED activity"
name="LED disk5_fault"
name="INDICATOR key_diag"
name="INDICATOR disk5_present"
and similar, then parse that in MI code.
Regards,
Julian
Index: gpio.h
===
RCS file: /cvsroot/src/sys/sys/gpio.h,v
retrieving revision 1.16
diff -u -r1.16 gpio.h
--- gpio.h 19 May 2018 13:59:06 - 1.16
+++ gpio.h 27 Oct 2020 11:12:47 -
@@ -26,6 +26,8 @@
/* GPIO pin states */
#define GPIO_PIN_LOW 0x00/* low level (logical 0) */
#define GPIO_PIN_HIGH 0x01/* high level (logical 1) */
+#define GPIO_ACTIVE_LOWGPIO_PIN_LOW/* pin active at
logical 0 */
+#define GPIO_ACTIVE_HIGH GPIO_PIN_HIGH /* pin active at logical 1 */
/* Max name length of a pin */
#define GPIOMAXNAME64
@@ -79,6 +81,11 @@
#defineGPIO_INTR_MODE_MASK (GPIO_INTR_EDGE_MASK | \
GPIO_INTR_LEVEL_MASK)
+/* GPIO pin connections that are set up in hardware */
+#define GPIO_LED 0x0001 /* connected to LED */
+#define GPIO_INDICATOR 0x0002 /* connected to indicator */
+#define GPIO_INTERRUPT 0x0004 /* connected to intr src */
+
/* GPIO controller description */
struct gpio_info {
int gpio_npins; /* total number of pins available */
Re: New header for GPIO-like definitions
Hi, > Sorry guys for chiming in a little late, but I have a keen interest in this > area, and there are lots of ways this needs to be harmonized across the > system, including the ability to have a generic way to specify that an > interrupt source, for example, is tied to a GPIO pin (useful for things like > i2c temp and light sensors that can generate interrupts when thresholds are > crossed). I sent the mail because I wasn't sure about the best way to pass the details between the MI and MD parts, so getting feedback before definiting a new API or mechanism is good. Having the ability to tie separate components together would be useful too. For an old example, the sparc port hardcodes the logic for powering the serial ports and for displaying disk/net status on the Sparcbook3: https://nxr.netbsd.org/xref/src/sys/arch/sparc/dev/tctrl.c#1477 https://nxr.netbsd.org/xref/src/sys/arch/sparc/dev/zs.c#991 and having a generic way of specifying this would be a lot better. On the hardware I'm looking at now, lighting the PSU fault LED when we receive a PSU failure indication makes a lot of sense, and that requires connecting together 2 separate GPIO's as well as envsys. > Might I suggest before we go to deep down the rabbit hole that you take a > look at the GPIO FDT bindings? > > > https://github.com/devicetree-org/devicetree-source/blob/master/Bindings/gpio/gpio.txt > > Right now to use that sort of thing in the NetBSD kernel, you need to be an > FDT'ized platform, but I have some experimental changes brewing in one of my > hacked-up trees that attempts to make it more generic with the concepts being > available on more platforms (and possibly backed by device's properties > dictionary). Using the device's properties would work for me. The existing code already uses device properties to pass pin information from MD to MI drivers: https://nxr.netbsd.org/xref/src/sys/arch/sparc64/sparc64/ofw_patch.c#add_gpio_LED so having a standard dictionary and using that would be great. Do you have a proposed dictionary/mapping for the FDT bindings? Regards, Julian
Re: New header for GPIO-like definitions
Hi, > > The definitions of pins are coming from hardware-specific properties. > > That's what I missed. On a device you are dealing with, pin N is > *always* wired to an LED because that's how it comes from the factory. > My head was in maker-land where there is an LED because someone wired > one up. Exactly. In this case, it's the front panel and other LED's that show status or faults, plus indicators connected to sensors to detect if disks or PSU's are present, failed, etc. For a particular machine model, they are known (or observed and become known :-). > I just remember lights before LED, and the fact that they are LED vs > incandescent is not important to how they are used. I don't know what's > next. > > But given there is an led system, there is no incremental harm and it > seems ok. I think that we'll probably not find a GPIO that's wired to a non-LED light. If it becomes important, we can always add another definition for it. > > Yes, just reading the logic level to display whether the "thing" connected > > is on or off. A better name would be appreciated. Maybe "INDICATOR", which > > would match the envsys name "ENVSYS_INDICATOR"? > > Or even "GPIO_ENVSYS_INDICATOR" because there might be some binary > inputs later that get hooked up to some other kind of framework. That makes sense. Maybe GPIO_PIN_ENVSYS_INDICATOR although that's getting a bit long. I wonder if the naming is clear enough without "PIN" in the name but I thought that it made sense to be more specific. > > Hopefully, the above is enough, but maybe a code snippet would help (this > > snippet is only for LED's, but similar applies for other types). In the > > hardware-specific driver, I add the pins to proplib: > > > > add_gpio_pin(pins, "disk_fault", GPIO_PIN_LED, > > 0, GPIO_PIN_ACT_LOW, -1); > > ... > > So I see the ACT_LOW. > > GPIO_PIN_LED is an output, but presumably this means that one can no > longer use it with GPIO and only via led_. Which seems fine. Is that > what you mean? Yes, exactly. Also, I'm not sure that it makes sense to expose this type of GPIO to userspace apart from through the LED or envsys frameworks or similar. However, I don't want to discount that in future so grouping pins into input or output could make sense. > > Then, in the MD driver I have: > Do you mean MD, or MI? Thanks - MI. > I 95% follow, but I am convinced that what you are doing is ok, so to be > clear I have no objections. Thank you! I'll upload a patch with all the changes - that might make the whole picture easier to see. Regards, Julian
Re: New header for GPIO-like definitions
Hi,
Thanks for the comments!
> > #define GPIO_PIN_LED0x01
> > #define GPIO_PIN_SENSOR 0x02
> >
> > Does this seem reasonable, or is there a better way to do this?
> I don't really understand how this is different from in/out.
> Presumably this is coming from some request from userspace originally,
> where someone, perhaps in a config file, has told the system how a pin
> is hooked up.
The definitions of pins are coming from hardware-specific properties.
In the driver, I'd like to be able to handle requests based on what is
connected to the pin. For example, for LED's, attach them to the LED
framework using led_attach(), and for sensors add them to envsys.
I wasn't planning to use this for userland, but it might be useful for
a config file (as you mention).
> LED seems overly specific. Presumably you care that the output does
> something like "makes a light". But I don't understand why your API
> cares about light vs noise. And I don't see an active high/low in your
> proposal. So I don't understand how this is different from just
> "controllable binary output"
As above, I want to be able to route the pin to the correct internal
subsystem in the GPIO driver.
> I am also not following SENSOR.DO you just mean "reads if the logic
> level at the pin is high or low".
>
> I don't think you mean using i2c bitbang for a temp sensor.
Yes, just reading the logic level to display whether the "thing" connected
is on or off. A better name would be appreciated. Maybe "INDICATOR", which
would match the envsys name "ENVSYS_INDICATOR"?
> Perhaps you could step back and explain the bigger picture and what's
> awkward currently. I don't doubt you that more is needed, but I am not
> able to understand enough to discuss.
Hopefully, the above is enough, but maybe a code snippet would help (this
snippet is only for LED's, but similar applies for other types). In the
hardware-specific driver, I add the pins to proplib:
add_gpio_pin(pins, "disk_fault", GPIO_PIN_LED,
0, GPIO_PIN_ACT_LOW, -1);
...
add_gpio_pin(pins, "power", GPIO_PIN_INPUT,
5, GPIO_PIN_ACT_LOW, -1);
add_gpio_pin(pins, "key_normal", GPIO_PIN_SENSOR,
6, GPIO_PIN_ACT_LOW, -1);
where add_gpio_pin() has:
prop_dictionary_set_cstring(pin, "name", name);
prop_dictionary_set_uint32(pin, "type", type);
prop_dictionary_set_uint32(pin, "pin", num);
prop_dictionary_set_bool(pin, "active_high", act);
Then, in the MD driver I have:
pin = prop_array_get(pins, i);
prop_dictionary_get_uint32(pin, "type", );
switch (type) {
case GPIO_PIN_LED:
...
led_attach(n, l, pcf8574_get, pcf8574_set);
and because of the way that this chip works, I also need to know in advance
which pins are input and which are output, to avoid inadvertently changing
the input pins to output when writing to the chip. For that, generic
GPIO_PIN_IS_INPUT and GPIO_PIN_IS_OUTPUT definitions might be useful too.
Regards,
Julian
New header for GPIO-like definitions
Hi all, I'm adding a driver and hardware-specific properties for GPIO's (which pins control LED's, which control sensors, etc). I need to be able to pass the pin information from the arch-specific configuration to the MI driver. I'd like to add a new dev/gpio/gpiotypes.h, so that I can share the defintions between the MI and MD code, e.g.: #define GPIO_PIN_LED0x01 #define GPIO_PIN_SENSOR 0x02 Does this seem reasonable, or is there a better way to do this? Regards, Julian
Re: Patching sdmmmc capabilities for old controllers
Hi all,
> I'd like to add a capability in sdmmcvar.h to show that the controller only
> supports v1.0. This is different from the other capabilities there which
> are supported (i.e. not unsupported) capabilities. The reason to do this
> is to avoid touching every other driver to add a v1.1+ capability, so it's
> the least intrusive way that I can see of supporting this controller.
Rather than add the capability, I received comments that handling this in
the existing code would be better, so I've attached another patch that does
that. Stylistic note, I added a goto to match other places in the code,
but it might be nicer to take this block and make it a separate function
instead.
Regards,
Julian
? sys/dev/sdmmc/sdmmc_mem.c.dist
? sys/dev/sdmmc/sdmmcvar.h.dist
cvs diff: Diffing sys/dev/sdmmc
Index: sys/dev/sdmmc/sdmmc_mem.c
===
RCS file: /cvsroot/src/sys/dev/sdmmc/sdmmc_mem.c,v
retrieving revision 1.68.2.1
diff -u -r1.68.2.1 sdmmc_mem.c
--- sys/dev/sdmmc/sdmmc_mem.c 25 Feb 2020 18:40:43 - 1.68.2.1
+++ sys/dev/sdmmc/sdmmc_mem.c 7 May 2020 16:22:25 -
@@ -833,9 +833,14 @@
DPRINTF(("%s: switch func mode 0\n", SDMMCDEVNAME(sc)));
error = sdmmc_mem_sd_switch(sf, 0, 1, 0, );
if (error) {
- aprint_error_dev(sc->sc_dev,
- "switch func mode 0 failed\n");
- return error;
+ if (error == EINVAL) {
+ /* Not supported by controller */
+ goto skipswitchfuncs;
+ } else {
+ aprint_error_dev(sc->sc_dev,
+ "switch func mode 0 failed\n");
+ return error;
+ }
}
support_func = SFUNC_STATUS_GROUP(, 1);
@@ -887,6 +892,7 @@
delay(25);
}
}
+skipswitchfuncs:
/* update bus clock */
if (sc->sc_busclk > sf->csd.tran_speed)
Patching sdmmmc capabilities for old controllers
Hi all,
We have a driver for the Winbond W83l518D, which is an old SDMMC controller
that only supports SDMMC version 1.0. This means that it doesn't handle
newer opcodes. Unfortunately, instead of passing them through to the card,
it fails to handle them. Our current code (specifically sdmmc_mem.c:830)
relies on sending the switch function opcode, which is one of the opcodes
that this controller doesn't support.
I'd like to add a capability in sdmmcvar.h to show that the controller only
supports v1.0. This is different from the other capabilities there which
are supported (i.e. not unsupported) capabilities. The reason to do this
is to avoid touching every other driver to add a v1.1+ capability, so it's
the least intrusive way that I can see of supporting this controller.
Patch attached and comments appreciated.
Regards,
Julian
? sys/dev/sdmmc/sdmmc_mem.c.dist
? sys/dev/sdmmc/sdmmcvar.h.dist
cvs diff: Diffing sys/dev/sdmmc
Index: sys/dev/sdmmc/sdmmc_mem.c
===
RCS file: /cvsroot/src/sys/dev/sdmmc/sdmmc_mem.c,v
retrieving revision 1.68.2.1
diff -u -r1.68.2.1 sdmmc_mem.c
--- sys/dev/sdmmc/sdmmc_mem.c 25 Feb 2020 18:40:43 - 1.68.2.1
+++ sys/dev/sdmmc/sdmmc_mem.c 7 May 2020 07:18:39 -
@@ -829,7 +829,8 @@
best_func = 0;
if (sf->scr.sd_spec >= SCR_SD_SPEC_VER_1_10 &&
- ISSET(sf->csd.ccc, SD_CSD_CCC_SWITCH)) {
+ ISSET(sf->csd.ccc, SD_CSD_CCC_SWITCH) &&
+ !ISSET(sc->sc_caps, SMC_CAPS_ONLY_V10)) {
DPRINTF(("%s: switch func mode 0\n", SDMMCDEVNAME(sc)));
error = sdmmc_mem_sd_switch(sf, 0, 1, 0, );
if (error) {
Index: sys/dev/sdmmc/sdmmcvar.h
===
RCS file: /cvsroot/src/sys/dev/sdmmc/sdmmcvar.h,v
retrieving revision 1.30.4.1
diff -u -r1.30.4.1 sdmmcvar.h
--- sys/dev/sdmmc/sdmmcvar.h25 Feb 2020 18:40:43 - 1.30.4.1
+++ sys/dev/sdmmc/sdmmcvar.h7 May 2020 07:18:39 -
@@ -254,6 +254,7 @@
| SMC_CAPS_UHS_SDR104 \
| SMC_CAPS_UHS_DDR50)
#define SMC_CAPS_MMC_HS200 __BIT(15) /* eMMC HS200 timing */
+#define SMC_CAPS_ONLY_V10 __BIT(29) /* controller is v1.0 only */
#define SMC_CAPS_POLLING __BIT(30) /* driver supports cmd polling
*/
/* function */
Re: ehci: fix error handling?
Hi all, > attach always 'succeeds' in the sense that after attach has been called, > detach will always be called. The detach routine should tear down > everything that needs tearing down and not do things that will fail. > Perhaps the init could simply be done before the attach routine gets the > chance to fail? Some drivers mark the progress through attach so that detach will work correctly in all cases. Can we do the same here? For example, see sc->sc_att_stage in: https://nxr.netbsd.org/xref/src/sys/dev/ic/gem.c and also how gem_partial_detach() is called when the attach fails part way through. Regards, Julian
envsys design questions
Hi, I've been looking at some of the sensors using the envsys framework, and adding the ability to get and set the hardware (sensor chip) limits. Two aspects of envsys, related to initial values, seem strange to me: 1) we call setlimits immediately after getlimits 2) we call getlimits before reading sensor values 1) is entertaining because if we read a spurious value, the driver could then program it back to the hardware. (I have had this happen - a reading of 0xff (-1'C) was written back and the machine powered off instantly!). The workaround was to ignore 0xff in this driver, but it doesn't handle the general case of spurious readings, and if the hardware does a power off, you can't tell what happens without debugging in the driver [#]. It also seems pointless to me, because even if everything is read correctly, we needlessly write those same values back to the hardware. 2) is awkward for chips that might not have sensors attached. For example, if the fan reading is 0 at boot, we can assume that no fan is attached and not set limits for that sensor. However, because envsys sets the limits first, we have to read the values in the driver before setting the limits to detect this. Altering the envsys framework to read the sensors before the limits seems more sensible to me. Is there a reason that envsys works like this? Does anyone know of any adverse impact if I: 1) don't call setlimits at boot time, andonly call it if requested (e.g by processing either sysctl.conf or envsys.conf) 2) read the sensor values at boot before calling getlimits ? Alternatively, is there a better way that I've missed? Regards, J [#] This means that your machine will just shut down part way through boot. As soon as interrupts are enabled and envsys runs, the limits that were read in getlimits will be written back to the chip in setlimits. -- My other computer runs NetBSD too - http://www.netbsd.org/
Re: dbcool, envsys, powerd shutting down my machine
Hi, > The above is not necessarily correct -- many of the chips supported by > the lm(4) driver, on NetBSD, OpenBSD and DragonFly BSD, certainly do, > in fact, implement a whole bunch of fan-controlling features; see > http://sensors.cnst.su/fanctl/ for a sample BSD implementation of the > fan controlling logic. Ah - I looked at the LM manuals, as I had the references handy, and assumed the others were similar. They are almost different enough to have separate drivers. However, I think that it makes sense to add the changes in the patch to our driver. > Winbond W83627HF is supported by both http://mdoc.su/o,n,d/wbsio.4 and > http://mdoc.su/n,o,d/lm.4, and is one of the chips supported by the > fanctl patch above, too. Looks like wbsio handles the attachment glue (rather than lm at iic). > However, the above manual from Tyan doesn't appear to specify how > exactly are the fans actually wired up (e.g., to which pins on which > chip). That seems fairly standard. If the firmware can display the sensor values, it's possible to do some correlation, but otherwise I haven't found a good way, especially with the range of temperatures and voltage sources that can be measured. As an example where the firmware can show the values, I can translate sensors on a Sun fire V240: [adm1026hm0] F0.RS: 5720 RPM ... MB.P0.T_CORE:47.000 degC ... MB.V_GBE_+2V5: 2.508 V ... [lmtemp0] MB.T_ENC:11.000 degC which is nice. Regards, J PS. Approximate translations: F0.RS fan 0 rotational speed MB.P0.T_CORE main board processor 0 temperature (core) MB.V_GBE_+2V5 main board gigabit ethernet +2.5V MB.T_ENC main board temperature (enclosure) -- My other computer runs NetBSD too - http://www.netbsd.org/
Re: dbcool, envsys, powerd shutting down my machine
Hi, > Depends on whether you're on AMD's virtual degC scale they use for their CPU > temps or it's real degC's. I didn't realise that some CPU's don't report real values. So, it could be the CPU temperature after all. > Also, if we assume that the dbcool chip controls the CPU fan(s), it makes > more > sense for it to measure the CPU temp than the VRM temp, right? Looking at one of the datasheets at random (ADT7476), the fan speeds can be controlled either by a single sensor, or a mix of all the sensors, so one would have to read more chip registers to find out exactly how it is set up. > sysctl hw.dbcool0 gives > hw.dbcool0.fan_ctl_0.behavior = manual > hw.dbcool0.fan_ctl_0.min_duty = 27 > hw.dbcool0.fan_ctl_0.cur_duty = 100 > but that's with BIOS CPU fan regulation disabled. Looking at the driver, I don't think that we alter the duty-cycle, so having the fans run at 100% is sensible. However, this shouldn't lead to a problem of over temperature. > > Is there a dbcool1 r2_temp with a similar value and limits? > No dbcool1. But I only have one CPU installed on that board. I guessed that there would be 2 chips of the same type, but it appears not. > The full envstat output is > Current CritMax WarnMax WarnMin CritMin Unit > [dbcool0] > fan1: N/A > fan2: N/A > fan3: 2869 RPM > fan4: N/A > [lm0] > Fan0: N/A > Fan1: 3245 RPM > Fan2: N/A I don't think that the chips that lm supports have fan speed adjustment, so lm0 fan1 will always run at 100%. It does make sense for dbcool0 fan3 to be connected to the CPU fan, but it's hard to be sure. > Yes, of course. I was wondering where that limit came from. I see. All the registers that control the sensor chips can be written or read via i2c/smbus. The firmware or BIOS should set them up sensibly, and we don't alter them automatically in the drivers. For example, I have: Current CritMax WarnMax WarnMin CritMin Unit [admtemp0] internal:23.000 70.000-65.000 degC external:44.000 110.000-65.000 degC where "internal" is the ambient temperature and "external" is the CPU temperature, and OFW has set up the maximum limits, but has left the default minimum limits. On this machine (Sun Blade 2000), if the limits are exceeded, it will power down instantly in hardware. > > Also, I see that the latest BIOS version appears to be 3.09 - is that the > > version that you have? > I'll have a look (I will need to physically go there or can I read this from > some dmesg or sysctl?). It looks like pkgsrc/sysutils/dmidecode might be able to do this. Regards, J -- My other computer runs NetBSD too - http://www.netbsd.org/
Re: dbcool, envsys, powerd shutting down my machine
Hi, > In general, I personally don't think it ever makes sense to shutdown > by default when the temperature is exceeded, since most of these > sensors aren't really all that reliable (especially if you're getting > them over i2c, with potential bus locking issues and race conditions > with BIOS / IPMI; getting a bit sidelined, at the very least, the > sensor values should be dampened, which is what's done in OpenBSD's > sensorsd, not sure if anything similar is done here). A lot of sensor chips have a separate hardware alert signals that are activated if a temperature is outside the limits (either "warning" or "critical" or both). Some hardware will shut down without OS intervention if this signal is asserted. So, it's good to have the correct values set to avoid damage, and some hardware enforces that. I have seen spurious values read from i2c chips, so perhaps it does make sense to wait for more than one reading that is outside the limit. Note, that the sensor chip itself will usually have something similar - for example waiting until 3 readings are outside the range before activating the hardware alert. Not sure if we need dampening, but ignoring spurious values would be good (although it wouldn't help here). Regards, J -- My other computer runs NetBSD too - http://www.netbsd.org/
Re: machfb MMIO versus SPARCle OFW
Eww. IIRC that's supposed to turn off the register block that lives in the upper 2KB of each half aperture, it didn't cause any problems with other other Sun or Apple mach64 OFW that I have here. Should really only be done when we have 8MB VRAM and the registers would overlap with it. Not sure why it causes the problem on SPARCle then. It has a Rage L Mobility, which I don't think is in any other machine that we use mach64 on (I had to add it to the match routine). Also, it's possible that the firmware is slightly different from the standard Sun version. Does sparcle's mach64 have 8MB VRAM? I've got an 8MB PGX64 to play with. Yes. dmesg is: machfb0 at pci0 dev 19 function 0: ATI Technologies Rage L Mobility (PCI) (rev. 0x64) machfb0: using MMIO aperture machfb0: 16 MB aperture at 0x0300, 4 KB registers at 0xa000 machfb0: 128 KB ROM at 0x0002 machfb0: 8192 KB SDRAM 55.815 MHz, maximum RAMDAC clock 230 MHz machfb0: initial resolution 1400x1050 at 8 bpp machfb0: attached to /dev/fb0 Actually, we can probably leave the registers enabled anyway, and only turn them off if something tries to mmap that last bit of video RAM. IIRC machfb itself doesn't need VRAM access at all, and the whole song dance is mostly intended for X. Not altering the BUS_CNTL register would seem easier. However, detach does work if that's not possible. Thanks, J -- My other computer also runs NetBSD/Sailing at Newbiggin http://www.netbsd.org// http://www.newbigginsailingclub.org/
Re: machfb MMIO versus SPARCle OFW
Hi, well -- this doesn't help ddb or dropping to the prom directly does it? Strangely, droppping to DDB or to the PROM from DDB works fine. It's just halt and reboot that don't work. Thanks, J PS. I didn't test a kernel witout DDB. -- My other computer also runs NetBSD/Sailing at Newbiggin http://www.netbsd.org// http://www.newbigginsailingclub.org/
machfb MMIO versus SPARCle OFW
Hi,
I tracked down why my SPARCle (SPARC laptop) appears to lock up on halt.
The cause is:
http://mail-index.NetBSD.org/source-changes/2012/08/15/msg036624.html
where we use the MMIO registers and alter the BUS_CNTL register. This is
a problem because altering the BUS_CNTL register in this way [*] causes the
OFW console to stop working when we halt.
The solution seemed fairly easy - add a detach routine to machfb to
restore the BUS_CNTL register (patch attached). However, this fails
because wsdisplay0 attaches at machfb0, so we also need a detach routine
for wsdisplay. As wsdisplay0 is the console, we have to force the detach,
so it seemed reasonable to add an additional flag for that (wscons patch
also attached). This works, and halt now returns to the OFW prompt.
However, I have a question about the wscons patch - there is no
wsdisplay_noemul_detach(), so is it sensible to call only
wsdisplay_emul_detach() on detach, or is there configuration where
wsdisplay* at machfb could be wsdisplay_noemul?
Thanks,
J
PS. Patches against against current from 2012/08/15.
PPS. Not yet tested on non-console machfb.
[*] cvs rdiff -u -r1.80 -r1.81 src/sys/dev/pci/machfb.c
--
NetBSD: simple; works; documented/Sailing at Newbiggin
http://www.netbsd.org// http://www.newbigginsailingclub.org/
Index: sys/dev/pci/machfb.c
===
RCS file: /cvsroot/src/sys/dev/pci/machfb.c,v
retrieving revision 1.82
diff -u -r1.82 machfb.c
--- sys/dev/pci/machfb.c15 Aug 2012 17:43:59 - 1.82
+++ sys/dev/pci/machfb.c21 Oct 2013 19:36:36 -
@@ -141,6 +141,7 @@
int mclk_post_div;
int mclk_fb_div;
int sc_clock; /* which clock to use */
+ int sc_use_mmio;/* using MMIO register */
struct videomode *sc_my_mode;
int sc_edid_size;
@@ -191,6 +192,7 @@
{ PCI_PRODUCT_ATI_RAGE_MOB_M3_AGP, 23 },
{ PCI_PRODUCT_ATI_RAGE_MOBILITY, 23 },
#endif
+ { PCI_PRODUCT_ATI_RAGE_L_MOB_M1_PCI, 23 },
{ PCI_PRODUCT_ATI_RAGE_LT_PRO_AGP, 23 },
{ PCI_PRODUCT_ATI_RAGE_LT_PRO, 23 },
{ PCI_PRODUCT_ATI_RAGE_LT, 23 },
@@ -235,9 +237,10 @@
static int mach64_match(device_t, cfdata_t, void *);
static voidmach64_attach(device_t, device_t, void *);
+static int mach64_detach(device_t, int);
-CFATTACH_DECL_NEW(machfb, sizeof(struct mach64_softc), mach64_match,
mach64_attach,
-NULL, NULL);
+CFATTACH_DECL3_NEW(machfb, sizeof(struct mach64_softc), mach64_match,
+mach64_attach, mach64_detach, NULL, NULL, NULL, DVF_DETACH_SHUTDOWN);
static voidmach64_init(struct mach64_softc *);
static int mach64_get_memsize(struct mach64_softc *);
@@ -521,7 +524,6 @@
pcireg_t screg;
uint32_t reg;
const pcireg_t enables = PCI_COMMAND_MEM_ENABLE;
- int use_mmio = FALSE;
sc-sc_dev = self;
sc-sc_pc = pa-pa_pc;
@@ -535,6 +537,7 @@
sc-sc_iot = pa-pa_iot;
sc-sc_accessops.ioctl = mach64_ioctl;
sc-sc_accessops.mmap = mach64_mmap;
+ sc-sc_use_mmio = FALSE;
pci_aprint_devinfo(pa, Graphics processor);
#ifdef MACHFB_DEBUG
@@ -578,10 +581,10 @@
* functions
*/
aprint_normal_dev(sc-sc_dev, using MMIO aperture\n);
- use_mmio = TRUE;
+ sc-sc_use_mmio = TRUE;
}
}
- if (!use_mmio) {
+ if (!sc-sc_use_mmio) {
if (bus_space_map(sc-sc_memt, sc-sc_aperbase, sc-sc_apersize,
BUS_SPACE_MAP_LINEAR, sc-sc_memh)) {
panic(%s: failed to map aperture,
@@ -824,7 +827,7 @@
aa.accesscookie = sc-vd;
config_found(self, aa, wsemuldisplaydevprint);
- if (use_mmio) {
+ if (sc-sc_use_mmio) {
/*
* Now that we took over, turn off the aperture registers if we
* don't use them. Can't do this earlier since on some hardware
@@ -840,6 +843,24 @@
}
static int
+mach64_detach(device_t self, int flags)
+{
+ struct mach64_softc *sc = device_private(self);
+ uint32_t reg;
+
+ if (sc-sc_use_mmio) {
+ reg = regr(sc, BUS_CNTL);
+ aprint_debug_dev(sc-sc_dev, BUS_CNTL: %08x\n, reg);
+ reg = ~BUS_APER_REG_DIS;
+ regw(sc, BUS_CNTL, reg);
+
+ bus_space_unmap(sc-sc_regt, sc-sc_regh, sc-sc_regsize);
+ } else
+ bus_space_unmap(sc-sc_memt, sc-sc_memh, sc-sc_apersize);
+ return 0;
+}
+
+static int
machfb_drm_print(void *aux, const char *pnp)
{
if (pnp)
Index: sys/dev/wscons/wsconsio.h
===
RCS file: /cvsroot/src/sys/dev/wscons/wsconsio.h,v
retrieving revision 1.104
diff -u -r1.104 wsconsio.h
---
Re: pckb[cd] changes for sparc64
Hi,
I'd like to add support for the keyboard on the Tadpole SPARCle laptops.
This uses a PC (8042) keyboard controller with two main differences. The
first is that the aux port does't probe until after an initial timeout.
The second is that the keyboard does not do scan code translation, so we
have to translate the sequences in software.
After some more testing, the pckbport changes needed extra work. The
extra keys that exist other PC keyboards are now mapped to better locations
and the Sun-specific keys have a wscons mapping (they don't yet work in X).
Updated patch (just the files changed from the previous) attached.
Thanks,
J
PS. Also tested on shark.
--
NetBSD: simple; works; documented/Sailing at Newbiggin
http://www.netbsd.org// http://www.newbigginsailingclub.org/
Index: sys/dev/pckbport/pckbd.c
===
RCS file: /cvsroot/src/sys/dev/pckbport/pckbd.c,v
retrieving revision 1.29
diff -u -r1.29 pckbd.c
--- sys/dev/pckbport/pckbd.c24 Feb 2010 22:38:08 - 1.29
+++ sys/dev/pckbport/pckbd.c1 Oct 2012 09:31:53 -
@@ -100,9 +100,12 @@
pckbport_tag_t t_kbctag;
pckbport_slot_t t_kbcslot;
+ int t_translating;
+
int t_lastchar;
int t_extended0;
int t_extended1;
+ int t_releasing;
struct pckbd_softc *t_sc; /* back pointer */
};
@@ -167,7 +170,9 @@
void pckbd_bell(u_int, u_int, u_int, int);
-intpckbd_set_xtscancode(pckbport_tag_t, pckbport_slot_t);
+intpckbd_scancode_translate(struct pckbd_internal *, int);
+intpckbd_set_xtscancode(pckbport_tag_t, pckbport_slot_t,
+ struct pckbd_internal *);
intpckbd_init(struct pckbd_internal *, pckbport_tag_t, pckbport_slot_t,
int);
void pckbd_input(void *, int);
@@ -179,9 +184,10 @@
struct pckbd_internal pckbd_consdata;
int
-pckbd_set_xtscancode(pckbport_tag_t kbctag, pckbport_slot_t kbcslot)
+pckbd_set_xtscancode(pckbport_tag_t kbctag, pckbport_slot_t kbcslot,
+struct pckbd_internal *id)
{
- int res;
+ int xt, res = 0;
u_char cmd[2];
/*
@@ -192,12 +198,14 @@
* known to not work on some PS/2 machines. We try desperately to deal
* with this by checking the (lack of a) translate bit in the 8042 and
* attempting to set the keyboard to XT mode. If this all fails, well,
-* tough luck.
+* tough luck. If the PCKBC_CANT_TRANSLATE pckbc flag was set, we
+* enable software translation.
*
* XXX It would perhaps be a better choice to just use AT scan codes
* and not bother with this.
*/
- if (pckbport_xt_translation(kbctag, kbcslot, 1)) {
+ xt = pckbport_xt_translation(kbctag, kbcslot, 1);
+ if (xt == 1) {
/* The 8042 is translating for us; use AT codes. */
cmd[0] = KBC_SETTABLE;
cmd[1] = 2;
@@ -216,6 +224,12 @@
pckbport_flush(kbctag, kbcslot);
res = 0;
}
+ if (id != NULL)
+ id-t_translating = 1;
+ } else if (xt == -1) {
+ /* Software translation required */
+ if (id != NULL)
+ id-t_translating = 0;
} else {
/* Stupid 8042; set keyboard to XT codes. */
cmd[0] = KBC_SETTABLE;
@@ -223,6 +237,8 @@
res = pckbport_poll_cmd(kbctag, kbcslot, cmd, 2, 0, 0, 0);
if (res)
aprint_debug(pckbd: error setting scanset 1\n);
+ if (id != NULL)
+ id-t_translating = 1;
}
return res;
}
@@ -331,7 +347,7 @@
*/
pckbport_flush(pa-pa_tag, pa-pa_slot);
- if (pckbd_set_xtscancode(pa-pa_tag, pa-pa_slot))
+ if (pckbd_set_xtscancode(pa-pa_tag, pa-pa_slot, NULL))
return 0;
return 2;
@@ -421,7 +437,7 @@
}
res = pckbd_set_xtscancode(sc-id-t_kbctag,
- sc-id-t_kbcslot);
+ sc-id-t_kbcslot, sc-id);
if (res)
return res;
@@ -446,21 +462,395 @@
return 0;
}
+const u_int8_t pckbd_xtbl[] = {
+/* 0x00 */
+ 0,
+ 0x43, /* F9 */
+ 0x89, /* SunStop */
+ 0x3f, /* F5 */
+ 0x3d, /* F3 */
+ 0x3b, /* F1 */
+ 0x3c, /* F2 */
+ 0x58, /* F12 */
+ 0,
+ 0x44, /* F10 */
+ 0x42, /* F8 */
+ 0x40, /* F6 */
+ 0x3e, /* F4 */
+ 0x0f, /* Tab */
+ 0x29, /* ` ~ */
+ 0,
+/* 0x10 */
+ 0,
+ 0x38, /* Left Alt */
+ 0x2a, /* Left Shift */
+
Re: Parent device selection in kernel configuration
Hi, OK. So the driver doesn't change -- nor the features available to the device, itself. E.g., it doesn't *add* (or remove) any capabilities that aren't always present, regardless. That's correct. The driver is often split up into the code to handle the chip itself (src/sys/dev/ic/*) and the frontends which handle the bus attachments (src/sys/dev/isa/*, src/sys/dev/pci/*), etc. Also, for example, some IC's are only available as PCI, so the driver code and the attachment code are in a single file. (consider something like a network interface... would *how* it is attached affect whether or not wake on LAN was supported?) This is more likely to be something that is supported or not based on the particular chipset. I'm not sure quite where wake on LAN would fit, as this is something external to the standard functioning of the network chip. Perhaps a better example would be a driver for a family of chips, where only some support TCP/UDP checksum offload. The bus attachment wouldn't alter this ... If, for example, there were extra levels of indirection in the ISR's one way or another. Or, some forced sharing of resources in one approach that were not present in others... ... as the bus attachments handle the indirection for interrupts, memory or I/O mapping, etc. For example, looking at the AMD 7990 ethernet driver, the common driver code is in src/sys/dev/ic/am7990.c, and the bus-specific parts are in (for example): ISA src/sys/dev/isa/if_le_isa.c ISAPnP sys/dev/isapnp/if_le_isapnp.c SBUSsrc/sys/dev/sbus/if_le.c If you look at the attachment routines for the above, you can see that they set up the bus-specific parts, and they all call am7990_config() for the common attachment. This means that a kernel can support that IC, but only some attachments for it, by only including some le at * lines. I'm trying to trim down the kernel for deployment on a cluster of 1GHz/1GB *diskless* machines. So, anything I can purge from the memory footprint is space that I can use for something else. Code that is effectively *dead* doesn't buy me anything at runtime. And, if one approach tied up extra data structures that weren't necessary with a different approach... A simple thing to do is remove support for all the bus-types (and devices for that bus) that aren't present. Then, you could remove all the devices that aren't or won't be present. After that, you might want to remove some parts of the kernel that you don't want. For example, you could decide to remove RAIDframe, or IPSEC, or FFS. With a 1GHz/1GB machine, I'm not sure that this will make that much difference though. Thanks, J -- My other computer also runs NetBSD/Sailing at Newbiggin http://www.netbsd.org// http://www.newbigginsailingclub.org/
pckb[cd] changes for sparc64
Hi,
I'd like to add support for the keyboard on the Tadpole SPARCle laptops.
This uses a PC (8042) keyboard controller with two main differences. The
first is that the aux port does't probe until after an initial timeout.
The second is that the keyboard does not do scan code translation, so we
have to translate the sequences in software.
There is already support for this keyboard in OpenBSD, and the attached
patch (sparc64-8042.patch) is based on their sys/dev/pckbc/pckbd.c,
revision 1.15 and sys/dev/ic/pckbc.c, revisions 1.10, 1.16, 1.17. A new
pckbc at ebus attachment (pckbc_ebus.c) is based on our existing sparc
pckbc_js.c attachment. Note, that the patch doesn't include the changes
to every other caller of pckbc_cnattach() to add the flags argument.
The keyboard also generates scancodes for the extra keys that appear on
normal Sun keyboards. I've mapped these to the same codes ones that we
use for these keys in the USB code (see our ukbd.c, revision 1.123).
Any comments (especially on a better/cleaner way to do this)?
Thanks,
J
--
NetBSD: simple; works; documented/Sailing at Newbiggin
http://www.netbsd.org// http://www.newbigginsailingclub.org/
Index: sys/arch/sparc64/conf/files.sparc64
===
RCS file: /cvsroot/src/sys/arch/sparc64/conf/files.sparc64,v
retrieving revision 1.138
diff -u -r1.138 files.sparc64
--- sys/arch/sparc64/conf/files.sparc64 3 Mar 2012 03:21:16 - 1.138
+++ sys/arch/sparc64/conf/files.sparc64 21 Sep 2012 12:58:58 -
@@ -153,6 +153,11 @@
attach com at ebus with com_ebus
file arch/sparc64/dev/com_ebus.c com_ebus
+# ebus PS/2 keyboard attachment for Tadpole SPARCle, etc.
+include dev/pckbport/files.pckbport
+attach pckbc at ebus with pckbc_ebus
+file arch/sparc64/dev/pckbc_ebus.c
+
device zstty {}: tty
attach zstty at zs
file dev/ic/z8530tty.c zstty needs-flag
Index: sys/dev/ic/pckbc.c
===
RCS file: /cvsroot/src/sys/dev/ic/pckbc.c,v
retrieving revision 1.53
diff -u -r1.53 pckbc.c
--- sys/dev/ic/pckbc.c 2 Feb 2012 19:43:03 - 1.53
+++ sys/dev/ic/pckbc.c 21 Sep 2012 12:58:59 -
@@ -354,6 +354,20 @@
t-t_haveaux = 1;
bus_space_write_1(iot, ioh_d, 0, 0x5a); /* a random value */
res = pckbc_poll_data1(t, PCKBC_AUX_SLOT);
+
+ /*
+* The following is needed to find the aux port on the Tadpole
+* SPARCle.
+*/
+ if (res == -1 ISSET(t-t_flags, PCKBC_NEED_AUXWRITE)) {
+ /* Read of aux echo timed out, try again */
+ if (!pckbc_send_cmd(iot, ioh_c, KBC_AUXWRITE))
+ goto nomouse;
+ if (!pckbc_wait_output(iot, ioh_c))
+ goto nomouse;
+ bus_space_write_1(iot, ioh_d, 0, 0x5a);
+ res = pckbc_poll_data1(t, PCKBC_AUX_SLOT);
+ }
if (res != -1) {
/*
* In most cases, the 0x5a gets echoed.
@@ -365,6 +379,7 @@
if (pckbc_attach_slot(sc, PCKBC_AUX_SLOT))
cmdbits |= KC8_MENABLE;
} else {
+
#ifdef PCKBCDEBUG
printf(pckbc: aux echo test failed\n);
#endif
@@ -395,6 +410,9 @@
struct pckbc_internal *t = self;
int ison;
+ if (ISSET(t-t_flags, PCKBC_CANT_TRANSLATE))
+ return (-1);
+
if (slot != PCKBC_KBD_SLOT) {
/* translation only for kbd slot */
if (on)
@@ -602,7 +620,7 @@
int
pckbc_cnattach(bus_space_tag_t iot, bus_addr_t addr,
- bus_size_t cmd_offset, pckbc_slot_t slot)
+ bus_size_t cmd_offset, pckbc_slot_t slot, int flags)
{
bus_space_handle_t ioh_d, ioh_c;
#ifdef PCKBC_CNATTACH_SELFTEST
@@ -622,6 +640,7 @@
pckbc_consdata.t_ioh_d = ioh_d;
pckbc_consdata.t_ioh_c = ioh_c;
pckbc_consdata.t_addr = addr;
+ pckbc_consdata.t_flags = flags;
callout_init(pckbc_consdata.t_cleanup, 0);
/* flush */
Index: sys/dev/ic/pckbcvar.h
===
RCS file: /cvsroot/src/sys/dev/ic/pckbcvar.h,v
retrieving revision 1.19
diff -u -r1.19 pckbcvar.h
--- sys/dev/ic/pckbcvar.h 2 Feb 2012 19:43:03 - 1.19
+++ sys/dev/ic/pckbcvar.h 21 Sep 2012 12:58:59 -
@@ -62,6 +62,9 @@
bus_space_handle_t t_ioh_d, t_ioh_c; /* data port, cmd port */
bus_addr_t t_addr;
u_char t_cmdbyte; /* shadow */
+ int t_flags;
+#definePCKBC_CANT_TRANSLATE0x0001 /* can't translate to XT
scancodes */
+#definePCKBC_NEED_AUXWRITE 0x0002 /* need auxwrite command to
find aux */
int t_haveaux; /* controller has an aux port */
struct pckbc_slotdata *t_slotdata[PCKBC_NSLOTS];
@@ -102,7 +105,7 @@
/* More normal calls from attach routines */
void
Re: Crashes in uaudio
Hi, so the attached patch increases the space for frequencies at the end of usb_audio_streaming_type1_descriptor (to AUFMT_MAX_FREQUENCIES), and sets auf-frequency_type to AUFMT_MAX_FREQUENCIES if the hardware reports more than we can handle. Patch committed. Thanks, J -- My other computer also runs NetBSD/Sailing at Newbiggin http://www.netbsd.org// http://www.newbigginsailingclub.org/
Re: Crashes in uaudio
Hi,
So, it seems that we are overwriting
the end of usb_audio_streaming_type1_descriptor for every descriptor where
the number of rates is more than 2. The problem is that we only notice when
we come to read. So, I think that we should set:
uByte tSamFreq[3*AUFMT_MAX_FREQUENCIES];
in the usb_audio_streaming_type1_descriptor definition (uaudioreg.h), make
sure that we only copy the correct maximum number of bytes when setting it
up, and remove the test at line 1852 of uaudio.c. We probably should also
set:
auf-frequency_type = AUFMT_MAX_FREQUENCIES;
in the test at line 1846, just in case we do meet a device which advertises
more frequencies than we can handle (if we don't, we could end up reading
memory after the end of the usb_audio_streaming_type1_descriptor).
Looking at the initialisation of the audio descriptors in uaudio_process_as(),
the descripters are pointers into a larger buffer, and we already check the
length against the buffer size, for example:
http://nxr.netbsd.org/source/xref/src/sys/dev/usb/uaudio.c#1578
so the attached patch increases the space for frequencies at the end of
usb_audio_streaming_type1_descriptor (to AUFMT_MAX_FREQUENCIES), and sets
auf-frequency_type to AUFMT_MAX_FREQUENCIES if the hardware reports more
than we can handle. Because of the change to the descriptor definition,
we need an extra include in umidi.c and umidi_quirks.c, but I think that
this is simpler than splitting out the definition of AUFMT_MAX_FREQUENCIES
into a separate uaudiovar.h.
Comments?
Thanks,
J
PS. Patch is against the jmcneill-usbmp branch, but the bug is in current
too.
--
My other computer also runs NetBSD/Sailing at Newbiggin
http://www.netbsd.org// http://www.newbigginsailingclub.org/
cvs diff: Diffing .
Index: uaudio.c
===
RCS file: /cvsroot/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.124.2.5
diff -u -r1.124.2.5 uaudio.c
--- uaudio.c29 Apr 2012 23:05:01 - 1.124.2.5
+++ uaudio.c14 May 2012 09:51:16 -
@@ -1847,12 +1847,8 @@
aprint_error(%s: please increase
AUFMT_MAX_FREQUENCIES to %d\n,
__func__, t1desc-bSamFreqType);
- break;
- }
- if (j = 2) {
- aprint_error(%s: too much tSamFreq:
- %d\n,
- __func__, t1desc-bSamFreqType);
+ auf-frequency_type =
+ AUFMT_MAX_FREQUENCIES;
break;
}
auf-frequency[j] = UA_GETSAMP(t1desc, j);
Index: uaudioreg.h
===
RCS file: /cvsroot/src/sys/dev/usb/uaudioreg.h,v
retrieving revision 1.15
diff -u -r1.15 uaudioreg.h
--- uaudioreg.h 28 Apr 2008 20:23:59 - 1.15
+++ uaudioreg.h 14 May 2012 09:51:16 -
@@ -113,7 +113,7 @@
uByte bBitResolution;
uByte bSamFreqType;
#define UA_SAMP_CONTNUOUS 0
- uByte tSamFreq[3*2]; /* room for low and high */
+ uByte tSamFreq[3*AUFMT_MAX_FREQUENCIES];
#define UA_GETSAMP(p, n) ((p)-tSamFreq[(n)*3+0] | ((p)-tSamFreq[(n)*3+1]
8) | ((p)-tSamFreq[(n)*3+2] 16))
#define UA_SAMP_LO(p) UA_GETSAMP(p, 0)
#define UA_SAMP_HI(p) UA_GETSAMP(p, 1)
Index: umidi.c
===
RCS file: /cvsroot/src/sys/dev/usb/umidi.c,v
retrieving revision 1.53.2.5
diff -u -r1.53.2.5 umidi.c
--- umidi.c 25 Feb 2012 10:26:24 - 1.53.2.5
+++ umidi.c 14 May 2012 09:51:17 -
@@ -52,6 +52,7 @@
#include dev/usb/usbdi.h
#include dev/usb/usbdi_util.h
+#include dev/auconv.h
#include dev/usb/usbdevs.h
#include dev/usb/uaudioreg.h
#include dev/usb/umidireg.h
Index: umidi_quirks.c
===
RCS file: /cvsroot/src/sys/dev/usb/umidi_quirks.c,v
retrieving revision 1.16.32.1
diff -u -r1.16.32.1 umidi_quirks.c
--- umidi_quirks.c 18 Feb 2012 07:35:10 - 1.16.32.1
+++ umidi_quirks.c 14 May 2012 09:51:17 -
@@ -49,6 +49,7 @@
#include dev/usb/usbdi.h
#include dev/usb/usbdi_util.h
+#include dev/auconv.h
#include dev/usb/usbdevs.h
#include dev/usb/uaudioreg.h
#include dev/usb/umidireg.h
Re: Early panic in uvm_map_prepare()
Hi, I tried booting a Sun E3500 (sparc64) with today's current, but it dies early with: panic: uvm_km_bootstrap: could not reserve kernel kmem This turned out to be because the kernel map was not large enough for machines with 8GB or more of main memory. I've restricted sparc64 NKMEMPAGES to 2.5GB in revision 1.49 of src/sys/arch/sparc64/include/param.h: http://mail-index.NetBSD.org/source-changes/2012/03/09/msg032636.html Thanks, J -- My other computer also runs NetBSD/Sailing at Newbiggin http://www.netbsd.org// http://www.newbigginsailingclub.org/
ncr53c9x fallout from asserting kernel lock in scsipi_base
Hi,
After the change in revision 1.156 of src/sys/dev/scsipi/scsipi_base.c to
assert that the kernel lock is held in scsipi_lookup_periph(), my SBus-based
sparc64 crashed with:
panic: kernel diagnostic assertion KERNEL_LOCKED_P() failed: file
/usr/src/sys/dev/scsipi/scsipi_base.c, line 221
The backtrace is:
kern_assert
scsipi_lookup_periph
scsipi_async_event
ncr53c9x_update_xfer_mode
ncr53c9x_init
ncr53c9x_attach
The attached allows it to boot and run, but is it correct?
Thanks,
J
--
My other computer also runs NetBSD/Sailing at Newbiggin
http://www.netbsd.org// http://www.newbigginsailingclub.org/
Index: src/sys/dev/ic/ncr53c9x.c
===
RCS file: /cvsroot/src/sys/dev/ic/ncr53c9x.c,v
retrieving revision 1.143
diff -u -r1.143 ncr53c9x.c
--- src/sys/dev/ic/ncr53c9x.c 31 Jul 2011 18:39:00 - 1.143
+++ src/sys/dev/ic/ncr53c9x.c 9 Mar 2012 14:41:02 -
@@ -546,7 +546,10 @@
ti-offset = 0;
ti-cfg3 = 0;
+ /* We need to have the kernel lock in scsipi_lookup_periph() */
+ KERNEL_LOCK(1, curlwp);
ncr53c9x_update_xfer_mode(sc, r);
+ KERNEL_UNLOCK_ONE(curlwp);
}
if (doreset) {
@@ -558,7 +561,9 @@
}
/* Notify upper layer */
+ KERNEL_LOCK(1, curlwp);
scsipi_async_event(sc-sc_channel, ASYNC_EVENT_RESET, NULL);
+ KERNEL_UNLOCK_ONE(curlwp);
}
/*
Re: RAIDframe component replacement
Hi, Yes, of course. But I haven't hardwired SCSI IDs to sd instances. So I didn't know whether the new SCSI Target 0 would become sd0. It will attach with the same unit number as the one that was detached. The autoconf code removes the entry when the disk is detached and the new disk attaches with the first free entry. So, if you have wired down the entries or not, an identical replacement will have the same number. If you detach more than one, make sure that you reattach them in reverse order of detach. Thanks, J -- My other computer also runs NetBSD/Sailing at Newbiggin http://www.netbsd.org// http://www.newbigginsailingclub.org/
