Re: CVS commit: src/sys/dev/ic
Oops, never mind, I hadn't yet seen the follow-up revert. > On Aug 1, 2022, at 8:29 AM, Jason Thorpe wrote: > > > >> On Aug 1, 2022, at 12:34 AM, Michael van Elst wrote: >> >> Module Name: src >> Committed By: mlelstv >> Date: Mon Aug 1 07:34:28 UTC 2022 >> >> Modified Files: >> src/sys/dev/ic: ahcisata_core.c bcmgenet.c nslm7x.c nvmereg.h nvmevar.h >> rtl8169.c tulip.c tulipreg.h >> >> Log Message: >> Also fix shift values for SCT constants. > > ??? > > diff -u src/sys/dev/ic/tulip.c:1.205 src/sys/dev/ic/tulip.c:1.206 > --- src/sys/dev/ic/tulip.c:1.205Sat Jun 25 02:46:15 2022 > +++ src/sys/dev/ic/tulip.c Mon Aug 1 07:34:28 2022 > @@ -1,4 +1,4 @@ > -/* $NetBSD: tulip.c,v 1.205 2022/06/25 02:46:15 tsutsui Exp $ */ > +/* $NetBSD: tulip.c,v 1.206 2022/08/01 07:34:28 mlelstv Exp $ */ > > /*- > * Copyright (c) 1998, 1999, 2000, 2002 The NetBSD Foundation, Inc. > @@ -36,7 +36,7 @@ > */ > > #include > -__KERNEL_RCSID(0, "$NetBSD: tulip.c,v 1.205 2022/06/25 02:46:15 tsutsui Exp > $"); > +__KERNEL_RCSID(0, "$NetBSD: tulip.c,v 1.206 2022/08/01 07:34:28 mlelstv Exp > $"); > > > #include > @@ -4394,7 +4394,7 @@ > */ > >/* XXX This should be auto-sense. */ > - ifmedia_set(&mii->mii_media, IFM_ETHER | IFM_10_T); > + ifmedia_set(&mii->mii_media, IFM_ETHER | IFM_10_5); > >tlp_print_media(sc); > } > > >> >> >> To generate a diff of this commit: >> cvs rdiff -u -r1.105 -r1.106 src/sys/dev/ic/ahcisata_core.c >> cvs rdiff -u -r1.11 -r1.12 src/sys/dev/ic/bcmgenet.c >> cvs rdiff -u -r1.74 -r1.75 src/sys/dev/ic/nslm7x.c >> cvs rdiff -u -r1.17 -r1.18 src/sys/dev/ic/nvmereg.h >> cvs rdiff -u -r1.24 -r1.25 src/sys/dev/ic/nvmevar.h >> cvs rdiff -u -r1.172 -r1.173 src/sys/dev/ic/rtl8169.c >> cvs rdiff -u -r1.205 -r1.206 src/sys/dev/ic/tulip.c >> cvs rdiff -u -r1.41 -r1.42 src/sys/dev/ic/tulipreg.h >> >> Please note that diffs are not public domain; they are subject to the >> copyright notices on the relevant files. >> > > -- thorpej -- thorpej
Re: CVS commit: src/sys/dev/ic
> On Aug 1, 2022, at 12:34 AM, Michael van Elst wrote: > > Module Name: src > Committed By: mlelstv > Date: Mon Aug 1 07:34:28 UTC 2022 > > Modified Files: > src/sys/dev/ic: ahcisata_core.c bcmgenet.c nslm7x.c nvmereg.h nvmevar.h >rtl8169.c tulip.c tulipreg.h > > Log Message: > Also fix shift values for SCT constants. ??? diff -u src/sys/dev/ic/tulip.c:1.205 src/sys/dev/ic/tulip.c:1.206 --- src/sys/dev/ic/tulip.c:1.205Sat Jun 25 02:46:15 2022 +++ src/sys/dev/ic/tulip.c Mon Aug 1 07:34:28 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: tulip.c,v 1.205 2022/06/25 02:46:15 tsutsui Exp $ */ +/* $NetBSD: tulip.c,v 1.206 2022/08/01 07:34:28 mlelstv Exp $ */ /*- * Copyright (c) 1998, 1999, 2000, 2002 The NetBSD Foundation, Inc. @@ -36,7 +36,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: tulip.c,v 1.205 2022/06/25 02:46:15 tsutsui Exp $"); +__KERNEL_RCSID(0, "$NetBSD: tulip.c,v 1.206 2022/08/01 07:34:28 mlelstv Exp $"); #include @@ -4394,7 +4394,7 @@ */ /* XXX This should be auto-sense. */ - ifmedia_set(&mii->mii_media, IFM_ETHER | IFM_10_T); + ifmedia_set(&mii->mii_media, IFM_ETHER | IFM_10_5); tlp_print_media(sc); } > > > To generate a diff of this commit: > cvs rdiff -u -r1.105 -r1.106 src/sys/dev/ic/ahcisata_core.c > cvs rdiff -u -r1.11 -r1.12 src/sys/dev/ic/bcmgenet.c > cvs rdiff -u -r1.74 -r1.75 src/sys/dev/ic/nslm7x.c > cvs rdiff -u -r1.17 -r1.18 src/sys/dev/ic/nvmereg.h > cvs rdiff -u -r1.24 -r1.25 src/sys/dev/ic/nvmevar.h > cvs rdiff -u -r1.172 -r1.173 src/sys/dev/ic/rtl8169.c > cvs rdiff -u -r1.205 -r1.206 src/sys/dev/ic/tulip.c > cvs rdiff -u -r1.41 -r1.42 src/sys/dev/ic/tulipreg.h > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > -- thorpej
Re: CVS commit: src/sys/dev/ic
In article <18083.1593053...@splode.eterna.com.au>, matthew green wrote: >"Jaromir Dolecek" writes: >> Module Name: src >> Committed By:jdolecek >> Date:Wed Jun 24 19:55:25 UTC 2020 >> >> Modified Files: >> src/sys/dev/ic: ibm561.c >> >> Log Message: >> avoid allocating almost 5k struct ibm561data on stack in ibm561_cninit(); >> it's too early for kmem_alloc(), so use static variable in BSS > >this seems particularly wasteful for a driver that won't >be useful for most systems. > >seems like a candidate for allow-listing instead, and as >it seems to only be relevant for alpha systems, that have >a fairly large stack (16K), and this will be called with >a fairly short call stack. I agree; the BSS kludge is ugly in general and should only be used sparingly. christos
re: CVS commit: src/sys/dev/ic
"Jaromir Dolecek" writes: > Module Name: src > Committed By: jdolecek > Date: Wed Jun 24 19:55:25 UTC 2020 > > Modified Files: > src/sys/dev/ic: ibm561.c > > Log Message: > avoid allocating almost 5k struct ibm561data on stack in ibm561_cninit(); > it's too early for kmem_alloc(), so use static variable in BSS this seems particularly wasteful for a driver that won't be useful for most systems. seems like a candidate for allow-listing instead, and as it seems to only be relevant for alpha systems, that have a fairly large stack (16K), and this will be called with a fairly short call stack. .mrg.
Re: CVS commit: src/sys/dev/ic
On 2020/03/24 12:45, SAITOH Masanobu wrote: > Module Name: src > Committed By: msaitoh > Date: Tue Mar 24 03:45:26 UTC 2020 > > Modified Files: > src/sys/dev/ic: spdmem.c spdmemvar.h > > Log Message: > - Define some new parameters of DDR3 SPD ROM. > - Use fine timebase parameters for time calculation on DDR3. This change > makes PC3- value + and tAA-tRCD-tRP value > more correctly on newer DD3.> > > To generate a diff of this commit: > cvs rdiff -u -r1.33 -r1.34 src/sys/dev/ic/spdmem.c > cvs rdiff -u -r1.14 -r1.15 src/sys/dev/ic/spdmemvar.h > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > -- --- SAITOH Masanobu (msai...@execsw.org msai...@netbsd.org)
Re: CVS commit: src/sys/dev/ic
On 22.10.2019 14:09, Martin Husemann wrote: > Module Name: src > Committed By: martin > Date: Tue Oct 22 12:09:11 UTC 2019 > > Modified Files: > src/sys/dev/ic: wdc.c > > Log Message: > Fix channel locking - patch from Christos. > > > #include > -__KERNEL_RCSID(0, "$NetBSD: wdc.c,v 1.292 2019/09/14 17:11:39 tsutsui Exp > $"); > +__KERNEL_RCSID(0, "$NetBSD: wdc.c,v 1.293 2019/10/22 12:09:11 martin Exp $"); > > #include "opt_ata.h" > #include "opt_wdc.h" > @@ -295,15 +295,16 @@ wdc_drvprobe(struct ata_channel *chp) > u_int8_t st0 = 0, st1 = 0; > int i, j, error, tfd; > > + ata_channel_lock(chp); > if (atabus_alloc_drives(chp, wdc->wdc_maxdrives) != 0) Missing ata_channel_unlock(chp)? Noted by mjg@freebsd. > return; > if (wdcprobe1(chp, 0) == 0) { > /* No drives, abort the attach here. */ > atabus_free_drives(chp); > + ata_channel_unlock(chp); > return; > } > signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/sys/dev/ic
On Wed, Oct 02, 2019 at 03:44:21AM +, Constantine A. Murenin wrote: > I'm getting a page fault trap after this patch, at netbsd:dk_open(), in > VirtualBox 6.0.12 r133076 with an empty NVME controller. There is a bug in dk_open when attachment didn't complete yet or failed. I'm about to fix that. I expect that is the same, but please provide the details. -- Michael van Elst Internet: mlel...@serpens.de "A potential Snark may lurk in every tree."
Re: CVS commit: src/sys/dev/ic
I'm getting a page fault trap after this patch, at netbsd:dk_open(), in VirtualBox 6.0.12 r133076 with an empty NVME controller. C. On 2019-W40-2 10:59 +, Michael van Elst wrote: Module Name:src Committed By: mlelstv Date: Tue Oct 1 10:59:50 UTC 2019 Modified Files: src/sys/dev/ic: ld_nvme.c Log Message: Don't attach an ld device if the format descriptor is unsupported/unused. To generate a diff of this commit: cvs rdiff -u -r1.22 -r1.23 src/sys/dev/ic/ld_nvme.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src/sys/dev/ic
Date:Sun, 22 Sep 2019 07:23:00 +0100 From:Nick Hudson Message-ID: <793d2380-8d1a-78ab-3682-0468aea0d...@gmx.co.uk> | I was merely pointing out that it exists already. Understood, and thanks - at a minimum that will avoid adding it with a different name. | I'm happy to help adding it to other ports. There's certainly no harm in that, for future use, but for right now there's no real need. kre
Re: CVS commit: src/sys/dev/ic
On 21/09/2019 20:19, Robert Elz wrote: Date:Sun, 22 Sep 2019 01:23:41 +0700 From:Robert Elz Message-ID: <8235.1569090...@jinx.noi.kre.to> | we'd need it in all the other ports before it can be used in MD code. I meant MI code of course... Sure. I was merely pointing out that it exists already. I'm happy to help adding it to other ports. Nick
Re: CVS commit: src/sys/dev/ic
Date:Sun, 22 Sep 2019 01:23:41 +0700 From:Robert Elz Message-ID: <8235.1569090...@jinx.noi.kre.to> | we'd need it in all the other ports before it can be used in MD code. I meant MI code of course... kre
Re: CVS commit: src/sys/dev/ic
Date:Sat, 21 Sep 2019 19:06:15 +0100 From:Nick Hudson Message-ID: | http://src.illumos.org/source/search?q=PRIxBUSADDR&defs=PRIxBUSADDR&refs=&path=&hist=&project=netbsd-src That shows that mips and arm have PRIxBUSADDR - we'd need it in all the other ports before it can be used in MD code. kre
Re: CVS commit: src/sys/dev/ic
On 21/09/2019 17:26, Jason Thorpe wrote: > Should we make a PRIxxx macro for it? > > -- thorpej > Sent from my iPhone. > >> On Sep 21, 2019, at 5:57 AM, Robert Elz wrote: >> >> Module Name:src >> Committed By:kre >> Date:Sat Sep 21 12:57:25 UTC 2019 >> >> Modified Files: >> src/sys/dev/ic: mpt.c >> >> Log Message: >> bus_addt_t is different widths on different archs, so there is no >> one simple %?x format that will always work to print it. Cast to >> intmax_t and use %jx which should work everywhere. http://src.illumos.org/source/search?q=PRIxBUSADDR&defs=PRIxBUSADDR&refs=&path=&hist=&project=netbsd-src
Re: CVS commit: src/sys/dev/ic
Date:Sat, 21 Sep 2019 09:26:21 -0700 From:Jason Thorpe Message-ID: | Should we make a PRIxxx macro for it? [since I deleted the context: "it" is bus_addr_t] Perhaps, for this particular case it doesn't really matter, but if code is likely to be printing buss_addr_t type values often enough that it might make a difference, that might be worthwhile. kre
Re: CVS commit: src/sys/dev/ic
Should we make a PRIxxx macro for it? -- thorpej Sent from my iPhone. > On Sep 21, 2019, at 5:57 AM, Robert Elz wrote: > > Module Name:src > Committed By:kre > Date:Sat Sep 21 12:57:25 UTC 2019 > > Modified Files: >src/sys/dev/ic: mpt.c > > Log Message: > bus_addt_t is different widths on different archs, so there is no > one simple %?x format that will always work to print it. Cast to > intmax_t and use %jx which should work everywhere. > > > To generate a diff of this commit: > cvs rdiff -u -r1.19 -r1.20 src/sys/dev/ic/mpt.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. >
Re: CVS commit: src/sys/dev/ic
Awesome, Jared - many thanks for this! On Fri, 28 Jun 2019 at 08:08, Jared D. McNeill wrote: > Module Name:src > Committed By: jmcneill > Date: Fri Jun 28 15:08:47 UTC 2019 > > Modified Files: > src/sys/dev/ic: nvme.c nvmevar.h > > Log Message: > Fix a performance issue where one busy queue can starve all other queues. > > In normal operations with multiple queues, the nvme driver will attempt > to schedule I/O requests on the submitting CPU. This breaks down when any > one of the queues becomes full; the driver returns EAGAIN to the disk > layer, which causes the disk layer to stop submitting more requests until > the blocked request is consumed. When space becomes available in the full > queue, it pulls the next buffer from the bufq and fills the queue again, > until finally hitting EAGAIN and preventing other queues from processing > requests. > > Two changes here to fix the problem: > > - When processing requests from the bufq, attempt to assign them to the >queue associated with the CPU that originated the request. > - If that queue is busy, try to find another queue with available space >before returning EAGAIN. This way, only when all queues are full will >the disk layer stop submitting more requests. > > Now for some real numbers. On a Rockchip RK3399 board (6 CPUs), with 6 > concurrent readers: > > Old code: > 4294967296 bytes transferred in 52.420 secs (81933752 bytes/sec) > 4294967296 bytes transferred in 53.969 secs (79582117 bytes/sec) > 4294967296 bytes transferred in 55.391 secs (77539082 bytes/sec) > 4294967296 bytes transferred in 55.649 secs (77179595 bytes/sec) > 4294967296 bytes transferred in 56.102 secs (76556402 bytes/sec) > 4294967296 bytes transferred in 72.901 secs (58915066 bytes/sec) > > New code: > 4294967296 bytes transferred in 37.171 secs (115546186 bytes/sec) > 4294967296 bytes transferred in 37.611 secs (114194445 bytes/sec) > 4294967296 bytes transferred in 37.655 secs (114061009 bytes/sec) > 4294967296 bytes transferred in 38.247 secs (112295534 bytes/sec) > 4294967296 bytes transferred in 38.496 secs (111569183 bytes/sec) > 4294967296 bytes transferred in 38.595 secs (111282997 bytes/sec) > > > To generate a diff of this commit: > cvs rdiff -u -r1.43 -r1.44 src/sys/dev/ic/nvme.c > cvs rdiff -u -r1.19 -r1.20 src/sys/dev/ic/nvmevar.h > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > >
re: CVS commit: src/sys/dev/ic
> > also, what's IST_MPSAFE? sounds like some platform specific > > thing. > > Sure. Each platform should have it's own. Unless someone wants to make an > MI intr_establish. then it's not useful for MI driver. you quote spl(9): >At the time of writing, the global kernel_lock is automatically >acquired for interrupts at this level, in order to support >device drivers that do not provide their own multiprocessor syn- >chronization. A future release of the system may allow the >automatic acquisition of kernel_lock to be disabled for individ- >ual interrupt handlers. and then: > > in MI drivers IPL_VM vs IPL_SCHED is the mpsafe tag. > > Not according to spl(9). my point is that when you use IPL_VM you may end up taking the kernel lock even if you don't need it. it implies "not mpsafe". back when ad reduced things to the current level, the plan would have been to obsolete IPL_VM entirely and simply have one level between IPL_NONE and IPL_HIGH, but that was too hard to do all at once, and IPL_VM for old code was kept, with the advice to avoid IPL_VM for the future. note that pretty much all other modern platforms have obsoleted any ability to block some interrupts -- you either block or allow -- and they have been shown to remain performant, so while we can talk about the ability to block some but not higher level interupts from what i can tell about modern compupting, it doesn't really matter any more. (in my world view, the restriction differences between IPL_VM and IPL_SCHED should be removed, and kmem should be allowed in interrupt handlers -- we've had kmem_intr_alloc() as the real back end for over 10 years at this point, time to accept it as the reality.) > > if you're wanting to restore some level for drivers to use > > below IPL_SCHED, that may be a reasonable thing to consider > > (though it's been abandoned by most platforms AFAICT), but > > that will require a fairly large rearrangement of spl(9) > > and the interfaces -- either adding a new level entirely > > (IPL_VMSAFE? :-) or adding an mpsafe tag everywhere, so i'm > > really not sure it's a good idea. > > Why is the latter not possible? that latter is possible, but it's fairly huge change -- how many MI calls to *intr_establish() exist that will need to be udpate? .mrg.
Re: CVS commit: src/sys/dev/ic
On 31/05/2019 07:27, matthew green wrote: > Nick Hudson writes: >> On 30/05/2019 21:46, matthew green wrote: Committed By: tnn Date: Thu May 30 07:37:17 UTC 2019 Modified Files: src/sys/dev/ic: ssdfb.c Log Message: - include uvm.h before uvm_device.h - don't need IPL_SCHED here >>> >>> the IPL_SCHED change seems backwards to me. >>> >>> IPL_VM is the "this driver is not updated to MPSAFE yet" level, >>> but IPL_SCHED is the MPSAFE level. >>> >>> ie, we should be striving to remove any uses of IPL_VM, not >>> moving (back) to them. >> >> In my view we should be using IPL_VM + IST_MPSAFE (and not IPL_SCHED) if >> the driver is MPSAFE. > > why? > > IPL_VM is basically synonymous with !MPSAFE in the current > kernel -- lots of subsystems will take kernel lock if you > use IPL_VM. From spl(9)... Code running at this level endures the same restrictions as at IPL_SCHED, but may use the deprecated malloc(9) or endorsed pool_cache(9) interfaces to allocate memory. At the time of writing, the global kernel_lock is automatically acquired for interrupts at this level, in order to support device drivers that do not provide their own multiprocessor syn- chronization. A future release of the system may allow the automatic acquisition of kernel_lock to be disabled for individ- ual interrupt handlers. > also, what's IST_MPSAFE? sounds like some platform specific > thing. Sure. Each platform should have it's own. Unless someone wants to make an MI intr_establish. > in MI drivers IPL_VM vs IPL_SCHED is the mpsafe tag. Not according to spl(9). > if you're wanting to restore some level for drivers to use > below IPL_SCHED, that may be a reasonable thing to consider > (though it's been abandoned by most platforms AFAICT), but > that will require a fairly large rearrangement of spl(9) > and the interfaces -- either adding a new level entirely > (IPL_VMSAFE? :-) or adding an mpsafe tag everywhere, so i'm > really not sure it's a good idea. Why is the latter not possible? > > for now, IPL_VM should be avoided for MPSAFE code. At this point I disagree :) Nick
re: CVS commit: src/sys/dev/ic
Nick Hudson writes: > On 30/05/2019 21:46, matthew green wrote: > >> Committed By: tnn > >> Date: Thu May 30 07:37:17 UTC 2019 > >> > >> Modified Files: > >>src/sys/dev/ic: ssdfb.c > >> > >> Log Message: > >> - include uvm.h before uvm_device.h > >> - don't need IPL_SCHED here > > > > the IPL_SCHED change seems backwards to me. > > > > IPL_VM is the "this driver is not updated to MPSAFE yet" level, > > but IPL_SCHED is the MPSAFE level. > > > > ie, we should be striving to remove any uses of IPL_VM, not > > moving (back) to them. > > In my view we should be using IPL_VM + IST_MPSAFE (and not IPL_SCHED) if > the driver is MPSAFE. why? IPL_VM is basically synonymous with !MPSAFE in the current kernel -- lots of subsystems will take kernel lock if you use IPL_VM. also, what's IST_MPSAFE? sounds like some platform specific thing. in MI drivers IPL_VM vs IPL_SCHED is the mpsafe tag. if you're wanting to restore some level for drivers to use below IPL_SCHED, that may be a reasonable thing to consider (though it's been abandoned by most platforms AFAICT), but that will require a fairly large rearrangement of spl(9) and the interfaces -- either adding a new level entirely (IPL_VMSAFE? :-) or adding an mpsafe tag everywhere, so i'm really not sure it's a good idea. for now, IPL_VM should be avoided for MPSAFE code. .mrg.
Re: CVS commit: src/sys/dev/ic
On 30/05/2019 21:46, matthew green wrote: Committed By: tnn Date: Thu May 30 07:37:17 UTC 2019 Modified Files: src/sys/dev/ic: ssdfb.c Log Message: - include uvm.h before uvm_device.h - don't need IPL_SCHED here the IPL_SCHED change seems backwards to me. IPL_VM is the "this driver is not updated to MPSAFE yet" level, but IPL_SCHED is the MPSAFE level. ie, we should be striving to remove any uses of IPL_VM, not moving (back) to them. In my view we should be using IPL_VM + IST_MPSAFE (and not IPL_SCHED) if the driver is MPSAFE. Nick
re: CVS commit: src/sys/dev/ic
> Committed By: tnn > Date: Thu May 30 07:37:17 UTC 2019 > > Modified Files: > src/sys/dev/ic: ssdfb.c > > Log Message: > - include uvm.h before uvm_device.h > - don't need IPL_SCHED here the IPL_SCHED change seems backwards to me. IPL_VM is the "this driver is not updated to MPSAFE yet" level, but IPL_SCHED is the MPSAFE level. ie, we should be striving to remove any uses of IPL_VM, not moving (back) to them. thanks. .mrg.
Re: CVS commit: src/sys/dev/ic
On 01/12/2018 15:07, Jaromir Dolecek wrote: > -#define NVME_ID_CTRLR_ONCS_SET_FEATURES __BIT(4) > +#define NVME_ID_CTRLR_ONCS_SAVE __BIT(4) Unintended? sbin/nvmectl/identify.c:(cdata->oncs & NVME_ID_CTRLR_ONCS_SET_FEATURES) ? Nick
Re: CVS commit: src/sys/dev/ic
Date: Mon, 19 Sep 2016 01:36:14 + From: David Holland On Sun, Sep 18, 2016 at 09:52:37PM +, Jaromir Dolecek wrote: > Modified Files: > src/sys/dev/ic: ld_nvme.c > > Log Message: > must use PR_NOWAIT also during ldattach()/dkwedge discover, our > i/o is there called with a spin lock held, which triggers > LOCKDEBUG panic That sounds like it should be corrected upstream of nvme...? Here's the relevant call tree: ld_diskstart acquires sc->sc_mutex (an IPL_VM spin lock) calls sc->sc_start = ld_nvme_start calls ld_nvme_dobio calls nvme_ns_get_ctx with PR_WAITOK or PR_NOWAIT releases sc->sc_mutex In this call tree, it is *never* correct to use PR_WAITOK, because the spin lock is unconditionally held. And the only other caller of ld_nvme_dobio is ld_nvme_dump, which is probably not allowed to block either for other reasons -- which presumably led to the abuse of cpu_(soft)intr_p here. Instead, you should prime nvme_ns_ctx_cache up front, e.g. by calling pool_cache_setlowat in ld_nvme_attach (and maintaining a count of the nvme instances), and always use PR_NOWAIT. Also, you need to handle failure in nvme_ns_get_ctx. From a cursory examination of dk_start, it looks like returning EAGAIN will DTRT -- dk_start will retry the block later. (Incidentally, it looks like nvme is statically wired to use a maximum queue depth of 128. In that case, you could just use a fixed array of 128 entries -- that's just a few pages of RAM. But maybe you want to be able to change the queue depth later.)
Re: CVS commit: src/sys/dev/ic
On Sun, Sep 18, 2016 at 09:52:37PM +, Jaromir Dolecek wrote: > Modified Files: > src/sys/dev/ic: ld_nvme.c > > Log Message: > must use PR_NOWAIT also during ldattach()/dkwedge discover, our i/o is there > called with a spin lock held, which triggers LOCKDEBUG panic That sounds like it should be corrected upstream of nvme...? -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/dev/ic
On 06/28/16 17:00, Nick Hudson wrote: Module Name:src Committed By: skrll Date: Tue Jun 28 16:00:32 UTC 2016 Modified Files: src/sys/dev/ic: sl811hs.c Log Message: Add slhci_memtest which is run when SLHCI_DEBUG is defined. oops... this is from Felix Deichmann - I'll fix the message. Nick
Re: CVS commit: src/sys/dev/ic
Don't know if it's your change, but for the alpha... /usr/src/netbsd/sys/dev/ic/sl811hs.c: In function 'slhci_root_start': /usr/src/netbsd/sys/dev/ic/sl811hs.c:989:21: error: variable 'spipe' set but not used [-Werror=unused-but-set-variable] struct slhci_pipe *spipe; ^ cc1: all warnings being treated as errors *** [sl811hs.o] Error code 1 nbmake[2]: stopped in /obj/nbobj/alfobj/usr/src/netbsd/sys/arch/alpha/compile/GENERIC.MP On Mon, May 16, 2016 at 03:09:29PM +, Nick Hudson wrote: > Module Name: src > Committed By: skrll > Date: Mon May 16 15:09:29 UTC 2016 > > Modified Files: > src/sys/dev/ic: sl811hs.c > > Log Message: > Simplify and fixup roothub interrupt transfers to work as well as before > nick-nhusb. > > > To generate a diff of this commit: > cvs rdiff -u -r1.74 -r1.75 src/sys/dev/ic/sl811hs.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > > Modified files: > > Index: src/sys/dev/ic/sl811hs.c > diff -u src/sys/dev/ic/sl811hs.c:1.74 src/sys/dev/ic/sl811hs.c:1.75 > --- src/sys/dev/ic/sl811hs.c:1.74 Mon May 16 08:00:25 2016 > +++ src/sys/dev/ic/sl811hs.c Mon May 16 15:09:29 2016 > @@ -1,4 +1,4 @@ > -/* $NetBSD: sl811hs.c,v 1.74 2016/05/16 08:00:25 skrll Exp $ */ > +/* $NetBSD: sl811hs.c,v 1.75 2016/05/16 15:09:29 skrll Exp $ */ > > /* > * Not (c) 2007 Matthew Orgass > @@ -68,7 +68,7 @@ > */ > > #include > -__KERNEL_RCSID(0, "$NetBSD: sl811hs.c,v 1.74 2016/05/16 08:00:25 skrll Exp > $"); > +__KERNEL_RCSID(0, "$NetBSD: sl811hs.c,v 1.75 2016/05/16 15:09:29 skrll Exp > $"); > > #include "opt_slhci.h" > > @@ -524,8 +524,6 @@ static void slhci_insert(struct slhci_so > static usbd_status slhci_clear_feature(struct slhci_softc *, unsigned int); > static usbd_status slhci_set_feature(struct slhci_softc *, unsigned int); > static void slhci_get_status(struct slhci_softc *, usb_port_status_t *); > -static usbd_status slhci_root(struct slhci_softc *, struct slhci_pipe *, > -struct usbd_xfer *); > > #define SLHCIHIST_FUNC()USBHIST_FUNC() > #define SLHCIHIST_CALLED() USBHIST_CALLED(slhcidebug) > @@ -993,7 +991,20 @@ slhci_root_start(struct usbd_xfer *xfer) > spipe = SLHCI_PIPE2SPIPE(xfer->ux_pipe); > sc = SLHCI_XFER2SC(xfer); > > - return slhci_lock_call(sc, &slhci_root, spipe, xfer); > + struct slhci_transfers *t = &sc->sc_transfers; > + > + LK_SLASSERT(spipe != NULL && xfer != NULL, sc, spipe, xfer, return > + USBD_CANCELLED); > + > + DLOG(D_TRACE, "%s start", pnames(SLHCI_XFER_TYPE(xfer)), 0,0,0); > + > + KASSERT(spipe->ptype == PT_ROOT_INTR); > + > + mutex_enter(&sc->sc_intr_lock); > + t->rootintr = xfer; > + mutex_exit(&sc->sc_intr_lock); > + > + return USBD_IN_PROGRESS; > } > > usbd_status > @@ -3080,32 +3091,6 @@ slhci_get_status(struct slhci_softc *sc, > DLOG(D_ROOT, "status=%#.4x, change=%#.4x", status, change, 0,0); > } > > -static usbd_status > -slhci_root(struct slhci_softc *sc, struct slhci_pipe *spipe, > -struct usbd_xfer *xfer) > -{ > - SLHCIHIST_FUNC(); SLHCIHIST_CALLED(); > - struct slhci_transfers *t; > - > - t = &sc->sc_transfers; > - > - LK_SLASSERT(spipe != NULL && xfer != NULL, sc, spipe, xfer, return > - USBD_CANCELLED); > - > - DLOG(D_TRACE, "%s start", pnames(SLHCI_XFER_TYPE(xfer)), 0,0,0); > - KASSERT(mutex_owned(&sc->sc_intr_lock)); > - > - KASSERT(spipe->ptype == PT_ROOT_INTR); > -#if 0 > - LK_SLASSERT(t->rootintr == NULL, sc, spipe, xfer, return > - USBD_CANCELLED); > -#endif > - t->rootintr = xfer; > - if (t->flags & F_CHANGE) > - t->flags |= F_ROOTINTR; > - return USBD_IN_PROGRESS; > -} > - > static int > slhci_roothub_ctrl(struct usbd_bus *bus, usb_device_request_t *req, > void *buf, int buflen) >
Re: CVS commit: src/sys/dev/ic
In message <20160510211554.b0062f...@cvs.netbsd.org> on Tue, 10 May 2016 21:15:54 +, "Nick Hudson" wrote: > Module Name: src > Committed By: skrll > Date: Tue May 10 21:15:54 UTC 2016 > > Modified Files: > src/sys/dev/ic: sl811hs.c > > Log Message: > Remove comment about splusb and replace with KASSERT(mutex_owned()) Added KASSERT use sc before initialize it. @@ -1296,6 +1296,8 @@ struct slhci_softc *sc; struct slhci_pipe *spipe; + KASSERT(mutex_owned(&sc->sc_lock)); + spipe = SLHCI_PIPE2SPIPE(xfer->ux_pipe); if (spipe == NULL) -- Takahiro Kambe
Re: CVS commit: src/sys/dev/ic
In article <27894.1461718...@splode.eterna.com.au>, matthew green wrote: >"Christos Zoulas" writes: >> Module Name: src >> Committed By:christos >> Date:Tue Apr 26 21:17:20 UTC 2016 >> >> Modified Files: >> src/sys/dev/ic: rt2860reg.h >> Added Files: >> src/sys/dev/ic: rt2860.c rt2860var.h >> >> Log Message: >> Unmodified OpenBSD sources (except Ids) >... could you describe what these sources do? :-) There are part of if_ral_pci.c for the 2860 variant of the chip. christos
re: CVS commit: src/sys/dev/ic
"Christos Zoulas" writes: > Module Name: src > Committed By: christos > Date: Tue Apr 26 21:17:20 UTC 2016 > > Modified Files: > src/sys/dev/ic: rt2860reg.h > Added Files: > src/sys/dev/ic: rt2860.c rt2860var.h > > Log Message: > Unmodified OpenBSD sources (except Ids) ... could you describe what these sources do? :-)
Re: CVS commit: src/sys/dev/ic
Sure, will request pullups after one week (Jul 29). -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany) On 27 Jul 2015, at 02:15, Erik Fair wrote: > Pull up to netbsd-7 and netbsd-6? > > Erik > > >> On Jul 22, 2015, at 01:33, Juergen Hannken-Illjes wrote: >> >> Module Name: src >> Committed By:hannken >> Date:Wed Jul 22 08:33:51 UTC 2015 >> >> Modified Files: >> src/sys/dev/ic: mpt_netbsd.c >> >> Log Message: >> Adapter leaks requests when mpt_event_notify_reply() has to acknowledge >> an event leading to "adapter resource shortage" messages when the scsipi >> subsystem tries to use all adapt_openings. >> >> Change mpt_ctlop() to free the request on event MPI_FUNCTION_EVENT_ACK. >> >> Tested on a SunFire X4275 with Symbios Logic SAS1068E (1000:0058, rev. 4). >> >> >> To generate a diff of this commit: >> cvs rdiff -u -r1.31 -r1.32 src/sys/dev/ic/mpt_netbsd.c >> >> Please note that diffs are not public domain; they are subject to the >> copyright notices on the relevant files.
Re: CVS commit: src/sys/dev/ic
Pull up to netbsd-7 and netbsd-6? Erik > On Jul 22, 2015, at 01:33, Juergen Hannken-Illjes wrote: > > Module Name: src > Committed By: hannken > Date: Wed Jul 22 08:33:51 UTC 2015 > > Modified Files: > src/sys/dev/ic: mpt_netbsd.c > > Log Message: > Adapter leaks requests when mpt_event_notify_reply() has to acknowledge > an event leading to "adapter resource shortage" messages when the scsipi > subsystem tries to use all adapt_openings. > > Change mpt_ctlop() to free the request on event MPI_FUNCTION_EVENT_ACK. > > Tested on a SunFire X4275 with Symbios Logic SAS1068E (1000:0058, rev. 4). > > > To generate a diff of this commit: > cvs rdiff -u -r1.31 -r1.32 src/sys/dev/ic/mpt_netbsd.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. >
Re: CVS commit: src/sys/dev/ic (spdmem.c)
On Thu, 02 Apr 2015 17:28:00 +0900 Masanobu SAITOH wrote: > A few weeks ago, I got a 2-way Xeon machine to test Intel X540 support. > Afrer finishing X540 test, I noticed that the machine had DDR4. I tried > to check the spdmem. It seemed that the machine's spdmem is not connected > via C612's internal i2c bus but via others. I stopped writing DDR4 support > and just added the basic types only because I couldn't test with the machine. This seems to be common on current generation server/workstation hardware. I think the SPD devices are hidden behind an i2c multiplexer in a platform specific way, possibly to avoid fan-out problems or interference with smbus out-of-band management functions. To read SPD on these machines the multiplexer (which should be addressable on the main segment, check with i2cscan) must first be configured to pass though the correct bus segment.
Re: CVS commit: src/sys/dev/ic (spdmem.c)
Thank you. A few weeks ago, I got a 2-way Xeon machine to test Intel X540 support. Afrer finishing X540 test, I noticed that the machine had DDR4. I tried to check the spdmem. It seemed that the machine's spdmem is not connecte via C612's internal i2c bus but via others. I stopped writing DDR4 support and just added the basic types only because I couldn't test with the machine. The machine was returned today. I've just down-loaded the JEDEC Annex L document to get the DDR4 SPD specs. Once I get my machine back on the network I can look at adding full DDR4 support to spdmem.[ch] code. Give me a couple of weeks. Also, if anyone out there has a machine with DDR4 memory and willing to test an update, please let me know! - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: CVS commit: src/sys/dev/ic (spdmem.c)
Hi, Paul. On 2015/04/01 13:57, Paul Goyette wrote: Module Name:src Committed By: msaitoh Date: Fri Mar 27 05:33:08 UTC 2015 Modified Files: src/sys/dev/ic: spdmem.c spdmemreg.h Log Message: Add DDR4 support a bit. Looking at the actual code change here @@ -76,6 +76,7 @@ static const char* spdmem_basic_types[] "DDR2 SDRAM FB", "DDR2 SDRAM FB Probe", "DDR3 SDRAM" +"DDR4 SDRAM" }; static const char* spdmem_superset_types[] = { There appears to be a missing comma, leading to the bug I just reported in http://mail-index.netbsd.org/current-users/2015/04/01/msg027013.html :) I would fix it myself, but currently my NetBSD machine is not networked. Thank you. A few weeks ago, I got a 2-way Xeon machine to test Intel X540 support. Afrer finishing X540 test, I noticed that the machine had DDR4. I tried to check the spdmem. It seemed that the machine's spdmem is not connected via C612's internal i2c bus but via others. I stopped writing DDR4 support and just added the basic types only because I couldn't test with the machine. The machine was returned today. Thanks. - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | - -- --- SAITOH Masanobu (msai...@execsw.org msai...@netbsd.org)
Re: CVS commit: src/sys/dev/ic (spdmem.c)
Module Name:src Committed By: msaitoh Date: Fri Mar 27 05:33:08 UTC 2015 Modified Files: src/sys/dev/ic: spdmem.c spdmemreg.h Log Message: Add DDR4 support a bit. Looking at the actual code change here @@ -76,6 +76,7 @@ static const char* spdmem_basic_types[] "DDR2 SDRAM FB", "DDR2 SDRAM FB Probe", "DDR3 SDRAM" + "DDR4 SDRAM" }; static const char* spdmem_superset_types[] = { There appears to be a missing comma, leading to the bug I just reported in http://mail-index.netbsd.org/current-users/2015/04/01/msg027013.html :) I would fix it myself, but currently my NetBSD machine is not networked. - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: CVS commit: src/sys/dev/ic
On Tue, Oct 14, 2014 at 14:54:17 +0200, Joerg Sonnenberger wrote: > On Tue, Oct 14, 2014 at 12:56:48AM +, Valeriy E. Ushakov wrote: > > Module Name:src > > Committed By: uwe > > Date: Tue Oct 14 00:56:48 UTC 2014 > > > > Modified Files: > > src/sys/dev/ic: rtl8169.c > > > > Log Message: > > RealTek 8139C+ incorrectly identifies UDP checksum 0x as bad. > > Force software recalculation of UDP checksum if bad checksum is > > reported by the hardware. > > Is the problem just the one-complement handling? Could we check if the > UDP checksum in the header is 0 first , not 0 (which means "no checksum"). > and only do the recalculation then? We can, but I didn't feel like trying to squeeze that logic into 24 character width available or restructuring the code to avoid deep nesting. Since I didn't get around to do that in the last year or so, I decided that committing a fix that makes things work is more important. -uwe
Re: CVS commit: src/sys/dev/ic
On Tue, Oct 14, 2014 at 12:56:48AM +, Valeriy E. Ushakov wrote: > Module Name: src > Committed By: uwe > Date: Tue Oct 14 00:56:48 UTC 2014 > > Modified Files: > src/sys/dev/ic: rtl8169.c > > Log Message: > RealTek 8139C+ incorrectly identifies UDP checksum 0x as bad. > Force software recalculation of UDP checksum if bad checksum is > reported by the hardware. Is the problem just the one-complement handling? Could we check if the UDP checksum in the header is 0 first and only do the recalculation then? Joerg
Re: CVS commit: src/sys/dev/ic
Hello. Thanks for the detailed response. I've already gone through and, I believe, fixed the indentation issues with the code. Those fixes are checked in as V1.21 of the mpt_netbsd.c file. I'll next go through and make your suggested changes for comments and long lines. -thanks -Brian
Re: CVS commit: src/sys/dev/ic
Date: Thu, 10 Apr 2014 10:22:14 -0700 From: Brian Buhrow hello. Yes, if you could be more specific about what you're asking for, I'll be happy to make the fixes. Here are the ones I see. Most of these are indentation issues. I know these all look like minor nits, but maintaining a consistent style is important for quickly eyeballing mistakes (like `goto fail') and avoiding visual dissonance while following code. > --- sys/dev/ic/mpt_netbsd.c > +++ sys/dev/ic/mpt_netbsd.c > ... > static voidmpt_scsipi_request(struct scsipi_channel *, > scsipi_adapter_req_t, void *); > static voidmpt_minphys(struct buf *); > +static int mpt_ioctl(struct scsipi_channel *, u_long, void *, int, > + struct proc *); Last line should be indented like the mpt_scsipi_request continuation line above: four columns past the function name. > +/*Save the output of the config so we can rescan the bus in case of errors*/ > + mpt->sc_scsibus_dv = config_found(mpt->sc_dev, &mpt->sc_channel, > scsiprint); Comment should be indented with the code and have spaces around the text, and lines should wrap at 80 lines: /* * Save the output of the config so we can rescan the bus in * case of errors. */ mpt->sc_scsibus_dv = config_found(mpt->sc_dev, &mpt->sc_channel, scsiprint); (In this case, the comment is long enough it should be a multiline comment.) > - } > +nrepl = mpt_drain_queue(mpt); > return (nrepl != 0); > } The `nrepl =' line should be indented. It is also not necessary -- you can omit the nrepl variable altogether and just do `return mpt_drain_queue(mpt) != 0;'. > + int s, nrepl = 0; > + > +if (req->xfer == NULL) { > + printf("mpt_timeout: NULL xfer for request index 0x%x, > sequenc 0x%x\n", Again, indent, and break lines. Also omit the extra intertoken space: if (req->xfer == NULL) { printf("mpt_timeout: NULL xfer for request index 0x%x," " sequence 0x%x\n", req->index, seq->sequence); return; } Long strings can be split across lines with cpp concatenation. > @@ -373,11 +373,28 @@ mpt_timeout(void *arg) > mpt->timeouts++; > if (mpt_intr(mpt)) { > if (req->sequence != oseq) { > + mpt->success ++; No need for space in `mpt->success++;'. > + /* > +*Ensure the IOC is really done giving us data since it appears it can > +*sometimes fail to give us interrupts under heavy load. > +*/ Space after `*' in comments. > + if (nrepl ) { No need for space here. > + mpt->success ++; Same. > + /* don't try a hard reset since this mangles the PCI > configuration registers */ Multiline comment (and prefer normal punctuation rules). > + /* don't really need to mpt_free_request() since > mpt_init() below will free all requests anyway */ Same. > +static > +int mpt_drain_queue(mpt_softc_t *mpt) Function name at beginning of line: static int mpt_drain_queue(mpt_softc_t *mpt) > + int restart = 0; /*nonzero if we need to restart the IOC*/ /* nonzero if we need to start the IOC */ > - switch (le16toh(mpt_reply->IOCStatus)) { > + switch ((le16toh(mpt_reply->IOCStatus) & MPI_IOCSTATUS_MASK)) { Omit needless parentheses. > + /* > +*FreeBSD and Linux indicate this is a phase error between > +*the IOC and the drive itself. > + *When this happens, the IOC becomes unhappy and stops > processing > + *all transactions. Call mpt_timeout which knows how to > + *get the IOC back on its feet. > +*/ Fill paragraph, match indentation, space between `*' and text: /* * FreeBSD and Linux indicate this is a phase error * between the IOC and the drive itself. When this * happens, the IOC becomes unhappy and stops * processing all transactions. Call mpt_timeout * which knows how to get the IOC back on its feet. */ > +mpt_prt(mpt,"mpt_done: IOC indicates protocol error -- > recovering..."); > + xs->error = XS_TIMEOUT; Match indentation. > + if (le16toh(mpt_reply->IOCStatus) & > MPI_IOCSTATUS_FLAG_LOG_INFO_AVAILABLE) { Split long line. > + /*mpt_enable_ints(mpt);*/ Don't comment out code; if anything, `#if notyet' or `#if 0' it. > + return(0); > + default: > + return (ENOTTY); Omit needless parentheses around a simple return operand. > --- sys/dev/ic/mpt_netbsd.h > +++ sys/dev/ic/mpt_netbsd.h > ... > + device_t sc_scsibus_dv; /*So we can rescan in case of errors*/ /* So we can rescan in case of errors */
Re: CVS commit: src/sys/dev/ic
hello. Yes, if you could be more specific about what you're asking for, I'll be happy to make the fixes. -thanks -Brian On Apr 7, 12:34pm, Taylor R Campbell wrote: } Subject: CVS commit: src/sys/dev/ic } [Resent in case you're not on source-changes-d.] } }Date: Tue, 1 Apr 2014 23:57:54 + }From: "Brian Buhrow" } }Modified Files: }src/sys/dev/ic: mpt_netbsd.c mpt_netbsd.h } }Log Message: }Checking in changes to improve error handling. Specifically: } } There are a lot of KNF issues in this change, making it hard to read. } Could you please fix them? (Let me know if you want me to be more } specific.) >-- End of excerpt from Taylor R Campbell
Re: CVS commit: src/sys/dev/ic
On 09/22/13 10:21, Adam Ciarcinski wrote: Module Name:src Committed By: adam Date: Sun Sep 22 09:21:56 UTC 2013 Modified Files: src/sys/dev/ic: sl811hs.c Log Message: Removed duplicated lines introduced in version 1.36. Thanks, Nick
re: CVS commit: src/sys/dev/ic
> Module Name: src > Committed By: jdc > Date: Mon Feb 4 18:29:56 UTC 2013 > > Modified Files: > src/sys/dev/ic: gem.c > > Log Message: > Halt the RX watchdog callout when stopping. ah, is this why my gem(4) would crash at reboot time if it was marked for auto-detach at shutdown? .mrg.
Re: CVS commit: src/sys/dev/ic
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hello, On Oct 20, 2012, at 9:31 AM, Michael Lorenz wrote: Module Name:src Committed By: macallan Date: Sat Oct 20 13:31:09 UTC 2012 Modified Files: src/sys/dev/ic: i128.c i128var.h Log Message: don't sync after each drawing op, ass functions to wait for the engine to get ready for more commands or idle *groan* fixed in the repository... have fun Michael -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (Darwin) Comment: GPGTools - http://gpgtools.org iQEVAwUBUIKv8cpnzkX8Yg2nAQJB7QgAqRZO/A6XW6CZzRJxe2M+lSITjI6YYSOc cd02S3sCYIYQr9d8D/7H9oPyJZk4sRkVDb3QzoHKyOW6vtk25a/QDOYN7W7W2moo zPJfPa6rvFgyVt9Q5kgNyNXxXOiDHqZk+PrYqxXWeqjc9SSDPBJSa7Yy8dR3Ti9q YopRhajNjId+UffGWCVRLlbxQehrLhrNgg/UMzQd4drDoQIzscqRJ2NIH2iIm7Lq 71G9WzQ22dT0tUXFlbb7hXVedY+LXYlpH0xWlvc2lex1givEMS+vTaVrViZQT9XX dbwoMEOefT8zT2rLrphFxl4Ulw+SJvaMEyQdlBC/jUqeQmTt05i0cQ== =vaMW -END PGP SIGNATURE-
Re: CVS commit: src/sys/dev/ic
On Wed, Aug 29, 2012 at 06:23:37PM +0100, David Laight wrote: > On Wed, Aug 29, 2012 at 04:50:10PM +, Jonathan A. Kollasch wrote: > > Log Message: > > mvsata(4) DMA data structures are already __packed, but as the hardware > > requires them to be 8-byte aligned, add __aligned(8) too, so that accesses > > on strict alignment platforms are more efficent. > > Sounds like it would be better to remove the overall __packed and use > explicit __aligned(n) on the members that are not correctly aligned. > That seems much more prone to error, and also may not necessarily produce a proper sizeof(). Jonathan Kollasch
Re: CVS commit: src/sys/dev/ic
On Wed, Aug 29, 2012 at 04:50:10PM +, Jonathan A. Kollasch wrote: > Module Name: src > Committed By: jakllsch > Date: Wed Aug 29 16:50:10 UTC 2012 > > Modified Files: > src/sys/dev/ic: mvsatareg.h > > Log Message: > mvsata(4) DMA data structures are already __packed, but as the hardware > requires them to be 8-byte aligned, add __aligned(8) too, so that accesses > on strict alignment platforms are more efficent. Sounds like it would be better to remove the overall __packed and use explicit __aligned(n) on the members that are not correctly aligned. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/sys/dev/ic
Hello, On Sat, 14 Jan 2012 01:43:36 +0900 Izumi Tsutsui wrote: > > I should have added compat versions of wsfont_find() and wsfont_matches() > > before > > trying to fix every driver. > > Well I thought two days window would be okay to revert changed APIs > but now param.h won't be reverted... I'm sorry I misunderstood you then. I thought it would be more sensible to just bump the kernel version instead of reverting a dozen or so purely mechanical changes all over the place. Many of the drivers should no longer need to call wsfont_find() anyway since they only do so in order to get around deficiencies in rasops_init() - namely the fact that it didn't pick a sane font for the requested terminal size and display resolution ( it just picked the first that didn't result in an insanely small terminal window ) have fun Michael
Re: CVS commit: src/sys/dev/ic
> I should have added compat versions of wsfont_find() and wsfont_matches() > before trying to fix every driver. Well I thought two days window would be okay to revert changed APIs but now param.h won't be reverted... --- Izumi Tsutsui
Re: CVS commit: src/sys/dev/ic
Hello, On Fri, 13 Jan 2012 23:05:13 +0900 Izumi Tsutsui wrote: > - No man page update after API changes I'll update the man pages. > - evbarm and hpcarm (and zaurus) are still broken on daily build >due to API changes Should work now. > The kernel version number represents ABI compatibility > regardless of whether there are actual referers or not. Then I'm going to bump the kernel version. I should have added compat versions of wsfont_find() and wsfont_matches() before trying to fix every driver. have fun Michael
Re: CVS commit: src/sys/dev/ic
macallan@ wrote: > On Thu, 12 Jan 2012 18:19:26 +0900 > Izumi Tsutsui wrote: > > > > Module Name: src > > > Committed By: macallan > > > Date: Wed Jan 11 20:41:28 UTC 2012 > > > > > > Modified Files: > > > src/sys/dev/ic: igsfb.c vga.c vga_raster.c > > > > > > Log Message: > > > wsfont_matches() and wsfont_find() take an extra parameter now > > > > Isn't it possible to provide compatible wrapper functions (without > > an extra parameter) rather than changing all existing API callers? > > > > So that you don't have to bump kernel version for modular(7) and > > all third parties (including ongoing porting efforts) don't > > have to fix their drivers on updating code base. > > That would be trivial to do. The reason I didn't do it right away > was that I had no idea there were so many drivers that call > wsfont_find() - at least that shouldn't be necessary anymore > in most cases, now that rasops_init() at least tries to pick > a sensible font for the screen / terminal size requested. > In vga it makes sense ( in order to get an 8 pixels wide > font and nothing else ), in igsfb not so much. - No discussion/review on public tech lists against public API changes - No man page update after API changes - It seems you didn't notice breakage even in MI vga.c on your first commit - evbarm and hpcarm (and zaurus) are still broken on daily build due to API changes It looks your changes are a bit rude and violate our commit guidelines unless they were approved by Core or other responsible persons. > Also, I'm not aware of any modular wsdisplay drivers. The kernel version number represents ABI compatibility regardless of whether there are actual referers or not. If you don't want to bump version, keep ABI compatibility as you say it's trivial, or ask Core if your changes are okay. Thanks, --- Izumi Tsutsui
Re: CVS commit: src/sys/dev/ic
Hello, On Thu, 12 Jan 2012 18:19:26 +0900 Izumi Tsutsui wrote: > > Module Name:src > > Committed By: macallan > > Date: Wed Jan 11 20:41:28 UTC 2012 > > > > Modified Files: > > src/sys/dev/ic: igsfb.c vga.c vga_raster.c > > > > Log Message: > > wsfont_matches() and wsfont_find() take an extra parameter now > > Isn't it possible to provide compatible wrapper functions (without > an extra parameter) rather than changing all existing API callers? > > So that you don't have to bump kernel version for modular(7) and > all third parties (including ongoing porting efforts) don't > have to fix their drivers on updating code base. That would be trivial to do. The reason I didn't do it right away was that I had no idea there were so many drivers that call wsfont_find() - at least that shouldn't be necessary anymore in most cases, now that rasops_init() at least tries to pick a sensible font for the screen / terminal size requested. In vga it makes sense ( in order to get an 8 pixels wide font and nothing else ), in igsfb not so much. Also, I'm not aware of any modular wsdisplay drivers. have fun Michael
Re: CVS commit: src/sys/dev/ic
> Module Name: src > Committed By: macallan > Date: Wed Jan 11 20:41:28 UTC 2012 > > Modified Files: > src/sys/dev/ic: igsfb.c vga.c vga_raster.c > > Log Message: > wsfont_matches() and wsfont_find() take an extra parameter now Isn't it possible to provide compatible wrapper functions (without an extra parameter) rather than changing all existing API callers? So that you don't have to bump kernel version for modular(7) and all third parties (including ongoing porting efforts) don't have to fix their drivers on updating code base. Thanks, --- Izumi Tsutsui
Re: CVS commit: src/sys/dev/ic
On Wed, Nov 02, 2011 at 04:54:51PM +, Jonathan A. Kollasch wrote: > Module Name: src > Committed By: jakllsch > Date: Wed Nov 2 16:54:51 UTC 2011 > > Modified Files: > src/sys/dev/ic: ahcisatareg.h > > Log Message: > Additionally apply __aligned(8) to all __packed hardware data structures. > (The hardware actually requires much larger alignment on these structures > (128 to 1024 bytes), but 8 is big enough for the compiler to generate more > efficient code on strict alignment architectures.) If these structures only have the odd field that needs non-native alignments (eg 32bit on 16bit alignment or 64bit on 32bit alignment) then it can be achieved by applying __aligned() to that single field. That also allows the compiler to generate larger-than-byte accesses on strict alignment systems. See the mbr definition in (IIRC) disklabel.h and the 64bit types in amd64's compat32 for examples. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/sys/dev/ic
On Sun, Aug 28, 2011 at 05:32:21AM -0400, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Sun Aug 28 09:32:21 UTC 2011 > > Modified Files: > src/sys/dev/ic: wdc.c > > Log Message: > Make this compile again without WDC_NO_IDS. Ops, sorry. Looks like I commited the wrong version of the patch, before cleanups. The only cleanup needed so far was to remove the #error and a KASSERT() that I added while testing on the fuloong. I just commited the right version of this code. Sorry for this mess ... -- Manuel Bouyer NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/dev/ic
In article <20101019222719.cb89217...@cvs.netbsd.org>, Jared D. McNeill wrote: >-=-=-=-=-=- > >Module Name: src >Committed By: jmcneill >Date: Tue Oct 19 22:27:19 UTC 2010 > >Modified Files: > src/sys/dev/ic: pcdisplay_subr.c > >Log Message: >When disabling the hardware cursor, use the 'cursor disable' bit in the >cursor start register. I think this only accidentally worked for the past >11 years because the start and end scanlines were both set to 0x10. > >See also http://www.osdever.net/FreeVGA/vga/crtcreg.htm#0A > > >To generate a diff of this commit: >cvs rdiff -u -r1.34 -r1.35 src/sys/dev/ic/pcdisplay_subr.c > >Please note that diffs are not public domain; they are subject to the >copyright notices on the relevant files. I think that the vga code is in bad need for some symbolic constants. christos
Re: CVS commit: src/sys/dev/ic
On Wed, Mar 31, 2010 at 04:06:47PM -0500, David Young wrote: > > > > bus_space_barrier() doesn't flush ... barriers only enforce > > the ordering of operations (and, of course, with respect to > > non-overlapping addresses ... obviously reads after writes of the > > same address in code will be enforced on the bus without an explicit > > barrier). > > Right. Putting the question another way, Is it important that reading > the register we wrote "lands" the write as a side-effect? It will have that effect, and, in many cases (think of writes being 'posted' on the hardware itself - inside the final PCI device) a readback is the only way to actually force a write to complete. This is true almost regardless of the cpu and bus architecture. > Do we expect that on sparc64, the bus barrier also "lands" the > write as a side-effect? The bus barrier is extremely unlikely to be able to get the write past the first PCI bus. The usual time this causes grief is when the write is a request to remove an IRQ, and is executed immediately before the ISR returns. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/sys/dev/ic
On Wed, Mar 31, 2010 at 01:19:10PM -0700, Jason Thorpe wrote: > > On Mar 31, 2010, at 7:45 AM, David Young wrote: > > > On Wed, Mar 31, 2010 at 05:09:41AM +, Michael Lorenz wrote: > >> Module Name: src > >> Committed By: macallan > >> Date: Wed Mar 31 05:09:41 UTC 2010 > >> > >> Modified Files: > >>src/sys/dev/ic: pcf8584.c > >> > >> Log Message: > >> Do as OpenSolaris does and read the status register after each write. > >> Now this driver works on my Blade 2500. > > > > void > > pcfiic_write(struct pcfiic_softc *sc, bus_size_t r, u_int8_t v) > > { > > + volatile uint8_t junk; > >bus_space_write_1(sc->sc_iot, sc->sc_ioh, sc->sc_regmap[r], v); > > + junk = bus_space_read_1(sc->sc_iot, sc->sc_ioh, PCF_S1); > >bus_space_barrier(sc->sc_iot, sc->sc_ioh, sc->sc_regmap[r], 1, > >BUS_SPACE_BARRIER_WRITE); > > } > > > > I wonder, does the device need the read, or is the bus_space_barrier() > > insufficient to flush the write to the device? > > bus_space_barrier() doesn't flush ... barriers only enforce > the ordering of operations (and, of course, with respect to > non-overlapping addresses ... obviously reads after writes of the > same address in code will be enforced on the bus without an explicit > barrier). Right. Putting the question another way, Is it important that reading the register we wrote "lands" the write as a side-effect? Do we expect that on sparc64, the bus barrier also "lands" the write as a side-effect? It sounds like the answer to both questions may be "no," and the write-to-write delay is key. Dave -- David Young OJC Technologies dyo...@ojctech.com Urbana, IL * (217) 278-3933
Re: CVS commit: src/sys/dev/ic
On Mar 31, 2010, at 7:45 AM, David Young wrote: > On Wed, Mar 31, 2010 at 05:09:41AM +, Michael Lorenz wrote: >> Module Name: src >> Committed By:macallan >> Date:Wed Mar 31 05:09:41 UTC 2010 >> >> Modified Files: >> src/sys/dev/ic: pcf8584.c >> >> Log Message: >> Do as OpenSolaris does and read the status register after each write. >> Now this driver works on my Blade 2500. > > void > pcfiic_write(struct pcfiic_softc *sc, bus_size_t r, u_int8_t v) > { > + volatile uint8_t junk; >bus_space_write_1(sc->sc_iot, sc->sc_ioh, sc->sc_regmap[r], v); > + junk = bus_space_read_1(sc->sc_iot, sc->sc_ioh, PCF_S1); >bus_space_barrier(sc->sc_iot, sc->sc_ioh, sc->sc_regmap[r], 1, >BUS_SPACE_BARRIER_WRITE); > } > > I wonder, does the device need the read, or is the bus_space_barrier() > insufficient to flush the write to the device? bus_space_barrier() doesn't flush ... barriers only enforce the ordering of operations (and, of course, with respect to non-overlapping addresses ... obviously reads after writes of the same address in code will be enforced on the bus without an explicit barrier). > > Dave > > -- > David Young OJC Technologies > dyo...@ojctech.com Urbana, IL * (217) 278-3933 -- thorpej
Re: CVS commit: src/sys/dev/ic
> > I wonder, does the device need the read, or is the bus_space_barrier() > > insufficient to flush the write to the device? > > I thave no idea. It works without the read on some machines - might be > a quirk in the Blade 2500's hardware. Either way we can probably get > rid of the bus_space_barrier(). IIRC the pcf8584 has a timing constraint on consecutive bus writes. Something like 5 clocks minimum delay at 10MHz. The dummy read may be a sun-devised way to ensure this constraint is met (I don't think the ebus hardware enforces it).
Re: CVS commit: src/sys/dev/ic
On Wed, Mar 31, 2010 at 02:03:11PM -0400, Michael wrote: > On Mar 31, 2010, at 10:45 AM, David Young wrote: > >I wonder, does the device need the read, or is the bus_space_barrier() > >insufficient to flush the write to the device? > > I thave no idea. It works without the read on some machines - might be > a quirk in the Blade 2500's hardware. Either way we can probably get > rid of the bus_space_barrier(). The barrier here just makes sure the write that just happened will be completed before the next write operation to the same register. It does not have any influence on the write you added. It might make sense to move the read past the barrier and modify that to be BUS_SPACE_BARRIER_READ|BUS_SPACE_BARRIER_WRITE. Or try the modified barrier w/o the added read - maybe that is already enough. However, if your pci bridge is not very wiered I doubt that this is the problem - i.e. my bet would be on the device requiring a small delay or actually the read cycle. Martin
Re: CVS commit: src/sys/dev/ic
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hello, On Mar 31, 2010, at 10:45 AM, David Young wrote: On Wed, Mar 31, 2010 at 05:09:41AM +, Michael Lorenz wrote: Module Name:src Committed By: macallan Date: Wed Mar 31 05:09:41 UTC 2010 Modified Files: src/sys/dev/ic: pcf8584.c Log Message: Do as OpenSolaris does and read the status register after each write. Now this driver works on my Blade 2500. void pcfiic_write(struct pcfiic_softc *sc, bus_size_t r, u_int8_t v) { + volatile uint8_t junk; bus_space_write_1(sc->sc_iot, sc->sc_ioh, sc->sc_regmap[r], v); + junk = bus_space_read_1(sc->sc_iot, sc->sc_ioh, PCF_S1); bus_space_barrier(sc->sc_iot, sc->sc_ioh, sc->sc_regmap[r], 1, BUS_SPACE_BARRIER_WRITE); } I wonder, does the device need the read, or is the bus_space_barrier() insufficient to flush the write to the device? I thave no idea. It works without the read on some machines - might be a quirk in the Blade 2500's hardware. Either way we can probably get rid of the bus_space_barrier(). have fun Michael -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (Darwin) iQEVAwUBS7OOX8pnzkX8Yg2nAQIfgwgAmhqpCkhfn5XBZ9bvj2ycADxjBVGg/7Cb TGSOLhJ9lizzNZHYh9V2Vtwf3tRgLRS4xLrA6T/UyT5fO+t+uTWzCF77P30LIxrx faoEPxrOwehxF0inVK/aaxjbOiiAN7zMBMwswC7dJOtG6m6pdpOMAo3mHU4zXWq4 7nFRcrdy1eOc/fGvxuwgnDPvf7CyriMKRMkO7m8QQCKnG4C1XDQ5VJYkLorPKqZm UEwYl/JhQz1LdTJ6l8SUxAzoCprT7du/iaZvxOHdAsRNtPRhA1pc7E/fFOJg9e3M 1+GXWcY/8SUTTNhUNEreTGPycM6nf6AhjusBFZYeK9Q/lxSF0KI6WQ== =gpun -END PGP SIGNATURE-
Re: CVS commit: src/sys/dev/ic
On Wed, Mar 31, 2010 at 09:45:40AM -0500, David Young wrote: > On Wed, Mar 31, 2010 at 05:09:41AM +, Michael Lorenz wrote: > > Module Name:src > > Committed By: macallan > > Date: Wed Mar 31 05:09:41 UTC 2010 > > > > Modified Files: > > src/sys/dev/ic: pcf8584.c > > > > Log Message: > > Do as OpenSolaris does and read the status register after each write. > > Now this driver works on my Blade 2500. > > void > pcfiic_write(struct pcfiic_softc *sc, bus_size_t r, u_int8_t v) > { > + volatile uint8_t junk; > bus_space_write_1(sc->sc_iot, sc->sc_ioh, sc->sc_regmap[r], v); > + junk = bus_space_read_1(sc->sc_iot, sc->sc_ioh, PCF_S1); > bus_space_barrier(sc->sc_iot, sc->sc_ioh, sc->sc_regmap[r], 1, > BUS_SPACE_BARRIER_WRITE); > } > > I wonder, does the device need the read, or is the bus_space_barrier() > insufficient to flush the write to the device? Additionally, should this read not be after the barrier?
Re: CVS commit: src/sys/dev/ic
On Wed, Mar 31, 2010 at 05:09:41AM +, Michael Lorenz wrote: > Module Name: src > Committed By: macallan > Date: Wed Mar 31 05:09:41 UTC 2010 > > Modified Files: > src/sys/dev/ic: pcf8584.c > > Log Message: > Do as OpenSolaris does and read the status register after each write. > Now this driver works on my Blade 2500. void pcfiic_write(struct pcfiic_softc *sc, bus_size_t r, u_int8_t v) { + volatile uint8_t junk; bus_space_write_1(sc->sc_iot, sc->sc_ioh, sc->sc_regmap[r], v); + junk = bus_space_read_1(sc->sc_iot, sc->sc_ioh, PCF_S1); bus_space_barrier(sc->sc_iot, sc->sc_ioh, sc->sc_regmap[r], 1, BUS_SPACE_BARRIER_WRITE); } I wonder, does the device need the read, or is the bus_space_barrier() insufficient to flush the write to the device? Dave -- David Young OJC Technologies dyo...@ojctech.com Urbana, IL * (217) 278-3933
Re: CVS commit: src/sys/dev/ic
> > > Eh ??? > > > if (foo) bar(); else baz(); > > > will probably execute faster than: > > > (*bar_baz)(); > > > since the indirect jump is (IIRC) always unpredicted. > > > > What do you have to predict on unconditional branches? > > The branch destination. E.g. the CPU can't start prefetching from the > bar(). Probably you guys should check diffs first: + if (sc->write_mbuf == NULL) + sc->write_mbuf = dp8390_write_mbuf; : - if (sc->write_mbuf) - len = (*sc->write_mbuf)(sc, m0, buffer); - else - len = dp8390_write_mbuf(sc, m0, buffer); + len = (*sc->write_mbuf)(sc, m0, buffer); i.e. + if (!foo) foo = bar; : - if (foo) (*foo)() else inlined_bar() + (*foo)() Few (no?) drivers use the latter inlined one so no befefit of prefetch with predict. --- Izumi Tsutsui
Re: CVS commit: src/sys/dev/ic
On Mon, Mar 01, 2010 at 12:28:17AM +0900, Izumi Tsutsui wrote: > > Eh ??? > > if (foo) bar(); else baz(); > > will probably execute faster than: > > (*bar_baz)(); > > since the indirect jump is (IIRC) always unpredicted. > > What do you have to predict on unconditional branches? The branch destination. E.g. the CPU can't start prefetching from the bar(). Joerg
Re: CVS commit: src/sys/dev/ic
> Eh ??? > if (foo) bar(); else baz(); > will probably execute faster than: > (*bar_baz)(); > since the indirect jump is (IIRC) always unpredicted. What do you have to predict on unconditional branches? Anyway, the most major user of MI dp8390 is NE2000 and it doesn't use inlined ones at all. -- Izumi Tsutsui
Re: CVS commit: src/sys/dev/ic
On Sat, Feb 27, 2010 at 04:40:12AM +, Izumi Tsutsui wrote: > > Log Message: > Always call device dependent functions via pointers rather than > using conditionals to switch inline functions for modern processors. Eh ??? if (foo) bar(); else baz(); will probably execute faster than: (*bar_baz)(); since the indirect jump is (IIRC) always unpredicted. the 'if' version stands a chance of correctly predicting the jump. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/sys/dev/ic
> Module Name: src > Committed By: dyoung > Date: Thu Aug 27 20:23:32 UTC 2009 > > Modified Files: > src/sys/dev/ic: tulipreg.h > > Log Message: > Make descriptors packed w/ 4-byte alignment. Which compiler explicitly requires it? @@ -158,7 +158,7 @@ volatile u_int32_t td_ctl; /* Control and Byte Counts */ volatile u_int32_t td_bufaddr1; /* Buffer Address 1 */ volatile u_int32_t td_bufaddr2; /* Buffer Address 2 */ -}; +} __packed __aligned(4); --- Izumi Tsutsui
Re: CVS commit: src/sys/dev/ic
On Thu, Jul 16, 2009 at 01:30:10AM +, David Young wrote: > Module Name: src > Committed By: dyoung > Date: Thu Jul 16 01:30:10 UTC 2009 > > Modified Files: > src/sys/dev/ic: mfi.c > > Log Message: > Try to detach ancestors (sd0 at scsibus0, scsibus0 at mfi0), first, > to avoid dangling references to envsys(4) sensors and such if > detaching the ancestors should fail. Err, that should say "descendants." Dave -- David Young OJC Technologies dyo...@ojctech.com Urbana, IL * (217) 278-3933
Re: CVS commit: src/sys/dev/ic (hme.c:1.77)
Hi, > Did you forget to commit hmereg.h? Yes. Argh! Commited now. Thanks! J -- My other computer also runs NetBSD/Sailing at Newbiggin http://www.netbsd.org// http://www.newbigginsailingclub.org/
Re: CVS commit: src/sys/dev/ic (hme.c:1.77)
On Wednesday 2009-05-06 20:40 +, Julian Coleman output: :Module Name: src :Committed By: jdc :Date: Wed May 6 20:40:19 UTC 2009 : :Modified Files: : src/sys/dev/ic: hme.c : :Log Message: :Check for internal PHY first, so that it always attaches first, even when :we have an MII transeiver attached. :Count all collision and error counters. :Handle counter overflow and RXTERR. : :Tested on U60 HME, PCI HME (501-5019) and SBus Sunswift (501-2739) [...] :cvs rdiff -u -r1.76 -r1.77 src/sys/dev/ic/hme.c Did you forget to commit hmereg.h? Regards, Geoff
Re: CVS commit: src/sys/dev/ic
On Sat, Mar 21, 2009 at 03:08:34AM +0900, Izumi Tsutsui wrote: > > > Modified Files: > > > src/sys/dev/ic: rtl8169.c rtl81x9var.h > > > > > > Log Message: > > > Access LDPS register in re_reset() only on 8169S single chip variants. > > > From OpenBSD and FreeBSD drivers via PR kern/41009, and > > > Realtek-supplied FreeBSD driver. > > > > We should probably pull this up to 5.0.. my HP desktop falls into the > > hole reported in the PR quite often. > > Can you confirm committed version on newer chips? It works on the following chip: re0 at pci3 dev 0 function 0: RealTek 8168B/8111B PCIe Gigabit Ethernet (rev. 0x02) re0: interrupting at ioapic0 pin 18 re0: Unknown revision (0x3c40) re0: Ethernet address aa:bb:cc:dd:ee:ff re0: using 256 tx descriptors rgephy0 at re0 phy 7: RTL8169S/8110S/8211 1000BASE-T media interface, rev. 2 rgephy0: 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, 1000baseT, 1000baseT-FDX, auto I have not yet tried you later changes to that driveryet, but will do it shortly. Thanks! --rafal -- Time is an illusion; lunchtime, doubly so. |/\/\| Rafal Boni -- Ford Prefect |\/\/| ra...@pobox.com