Re: CVS commit: src/sys/dev/ic

[Jason Thorpe]

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_media, IFM_ETHER | IFM_10_T);
> +   ifmedia_set(>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

[Jason Thorpe]

> 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_media, IFM_ETHER | IFM_10_T);
+   ifmedia_set(>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

[Christos Zoulas]

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

[matthew green]

"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

[SAITOH Masanobu]

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

[Kamil Rytarowski]

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

[Michael van Elst]

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

[Constantine A. Murenin]

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

[Robert Elz]

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

[Nick Hudson]

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

[Robert Elz]

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

[Robert Elz]

Date:Sat, 21 Sep 2019 19:06:15 +0100
From:Nick Hudson 
Message-ID:  

  | 
http://src.illumos.org/source/search?q=PRIxBUSADDR=PRIxBUSADDRnetbsd-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

[Nick Hudson]

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=PRIxBUSADDRnetbsd-src

Re: CVS commit: src/sys/dev/ic

[Robert Elz]

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

[Jason Thorpe]

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

[Alistair Crooks]

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

[matthew green]

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

[Nick Hudson]

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

[matthew green]

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

[Nick Hudson]

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

[matthew green]

> 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

[Nick Hudson]

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

[Taylor R Campbell]

   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

[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...?

-- 
David A. Holland
dholl...@netbsd.org

Re: CVS commit: src/sys/dev/ic

[Nick Hudson]

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

[Tom Spindler]

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, _root, spipe, xfer);
> + struct slhci_transfers *t = >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_intr_lock);
> + t->rootintr = xfer;
> + mutex_exit(>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_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_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

[Christos Zoulas]

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

[matthew green]

"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

[J. Hannken-Illjes]

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 f...@netbsd.org wrote:

 Pull up to netbsd-7 and netbsd-6?
 
   Erik
 
 
 On Jul 22, 2015, at 01:33, Juergen Hannken-Illjes hann...@netbsd.org 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

[Erik Fair]

Pull up to netbsd-7 and netbsd-6?

Erik


 On Jul 22, 2015, at 01:33, Juergen Hannken-Illjes hann...@netbsd.org 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)

[Tobias Nygren]

On Thu, 02 Apr 2015 17:28:00 +0900
Masanobu SAITOH msai...@execsw.org 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)

[Paul Goyette]

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)

[Paul Goyette]

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

[Joerg Sonnenberger]

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

[Valery Ushakov]

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

[Taylor R Campbell]

   Date: Thu, 10 Apr 2014 10:22:14 -0700
   From: Brian Buhrow buh...@nfbcal.org

   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

[Brian Buhrow]

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

[Brian Buhrow]

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 buh...@netbsd.org
} 
}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

[Nick Hudson]

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

[matthew green]

 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

[Michael]

-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

[Izumi Tsutsui]

macallan@ wrote:

 On Thu, 12 Jan 2012 18:19:26 +0900
 Izumi Tsutsui tsut...@ceres.dti.ne.jp 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

[Michael]

Hello,

On Fri, 13 Jan 2012 23:05:13 +0900
Izumi Tsutsui tsut...@ceres.dti.ne.jp 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

[Izumi Tsutsui]

 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

[Michael]

Hello,

On Sat, 14 Jan 2012 01:43:36 +0900
Izumi Tsutsui tsut...@ceres.dti.ne.jp 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

[Izumi Tsutsui]

 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

[Michael]

Hello,

On Thu, 12 Jan 2012 18:19:26 +0900
Izumi Tsutsui tsut...@ceres.dti.ne.jp 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

[David Laight]

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

[Manuel Bouyer]

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 bou...@antioche.eu.org
 NetBSD: 26 ans d'experience feront toujours la difference
--

Re: CVS commit: src/sys/dev/ic

[Christos Zoulas]

In article 20101019222719.cb89217...@cvs.netbsd.org,
Jared D. McNeill source-changes-d@NetBSD.org 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

[David Laight]

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

[David Young]

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

[Andrew Doran]

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

[Michael]

-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

[Martin Husemann]

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

[Tobias Nygren]

  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

[David Young]

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

[David Laight]

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

[Izumi Tsutsui]

 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

[Joerg Sonnenberger]

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

[Izumi Tsutsui]

   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 (hme.c:1.77)

[Geoff Wing]

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