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

[maya]

Hi maxv,

there is a similar code fragment in ic/bwfm.c:bwfm_scan_node.
I am not sure what I'm looking at. Could it be wrong too?

Thanks.

On Tue, Jan 16, 2018 at 07:05:25AM +, Maxime Villard wrote:
> Module Name:  src
> Committed By: maxv
> Date: Tue Jan 16 07:05:25 UTC 2018
> 
> Modified Files:
>   src/sys/dev/pci: if_ipw.c if_iwi.c if_iwn.c
> 
> Log Message:
> Fix overflow.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.66 -r1.67 src/sys/dev/pci/if_ipw.c
> cvs rdiff -u -r1.104 -r1.105 src/sys/dev/pci/if_iwi.c
> cvs rdiff -u -r1.86 -r1.87 src/sys/dev/pci/if_iwn.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/pci/if_ipw.c
> diff -u src/sys/dev/pci/if_ipw.c:1.66 src/sys/dev/pci/if_ipw.c:1.67
> --- src/sys/dev/pci/if_ipw.c:1.66 Mon Oct 23 09:31:18 2017
> +++ src/sys/dev/pci/if_ipw.c  Tue Jan 16 07:05:24 2018
> @@ -1,4 +1,4 @@
> -/*   $NetBSD: if_ipw.c,v 1.66 2017/10/23 09:31:18 msaitoh Exp $  */
> +/*   $NetBSD: if_ipw.c,v 1.67 2018/01/16 07:05:24 maxv Exp $ */
>  /*   FreeBSD: src/sys/dev/ipw/if_ipw.c,v 1.15 2005/11/13 17:17:40 damien Exp 
> */
>  
>  /*-
> @@ -29,7 +29,7 @@
>   */
>  
>  #include 
> -__KERNEL_RCSID(0, "$NetBSD: if_ipw.c,v 1.66 2017/10/23 09:31:18 msaitoh Exp 
> $");
> +__KERNEL_RCSID(0, "$NetBSD: if_ipw.c,v 1.67 2018/01/16 07:05:24 maxv Exp $");
>  
>  /*-
>   * Intel(R) PRO/Wireless 2100 MiniPCI driver
> @@ -1001,12 +1001,13 @@ ipw_fix_channel(struct ieee80211com *ic,
>   efrm = mtod(m, uint8_t *) + m->m_len;
>  
>   frm += 12;  /* skip tstamp, bintval and capinfo fields */
> - while (frm < efrm) {
> - if (*frm == IEEE80211_ELEMID_DSPARMS)
> + while (frm + 2 < efrm) {
> + if (*frm == IEEE80211_ELEMID_DSPARMS) {
>  #if IEEE80211_CHAN_MAX < 255
> - if (frm[2] <= IEEE80211_CHAN_MAX)
> + if (frm[2] <= IEEE80211_CHAN_MAX)
>  #endif
> - ic->ic_curchan = >ic_channels[frm[2]];
> + ic->ic_curchan = >ic_channels[frm[2]];
> + }
>  
>   frm += frm[1] + 2;
>   }
> 
> Index: src/sys/dev/pci/if_iwi.c
> diff -u src/sys/dev/pci/if_iwi.c:1.104 src/sys/dev/pci/if_iwi.c:1.105
> --- src/sys/dev/pci/if_iwi.c:1.104Mon Oct 23 09:28:13 2017
> +++ src/sys/dev/pci/if_iwi.c  Tue Jan 16 07:05:24 2018
> @@ -1,4 +1,4 @@
> -/*   $NetBSD: if_iwi.c,v 1.104 2017/10/23 09:28:13 msaitoh Exp $  */
> +/*   $NetBSD: if_iwi.c,v 1.105 2018/01/16 07:05:24 maxv Exp $  */
>  /*   $OpenBSD: if_iwi.c,v 1.111 2010/11/15 19:11:57 damien Exp $ */
>  
>  /*-
> @@ -19,7 +19,7 @@
>   */
>  
>  #include 
> -__KERNEL_RCSID(0, "$NetBSD: if_iwi.c,v 1.104 2017/10/23 09:28:13 msaitoh Exp 
> $");
> +__KERNEL_RCSID(0, "$NetBSD: if_iwi.c,v 1.105 2018/01/16 07:05:24 maxv Exp 
> $");
>  
>  /*-
>   * Intel(R) PRO/Wireless 2200BG/2225BG/2915ABG driver
> @@ -1126,12 +1126,13 @@ iwi_fix_channel(struct ieee80211com *ic,
>   efrm = mtod(m, uint8_t *) + m->m_len;
>  
>   frm += 12;  /* skip tstamp, bintval and capinfo fields */
> - while (frm < efrm) {
> - if (*frm == IEEE80211_ELEMID_DSPARMS)
> + while (frm + 2 < efrm) {
> + if (*frm == IEEE80211_ELEMID_DSPARMS) {
>  #if IEEE80211_CHAN_MAX < 255
> - if (frm[2] <= IEEE80211_CHAN_MAX)
> + if (frm[2] <= IEEE80211_CHAN_MAX)
>  #endif
> - ic->ic_curchan = >ic_channels[frm[2]];
> + ic->ic_curchan = >ic_channels[frm[2]];
> + }
>  
>   frm += frm[1] + 2;
>   }
> 
> Index: src/sys/dev/pci/if_iwn.c
> diff -u src/sys/dev/pci/if_iwn.c:1.86 src/sys/dev/pci/if_iwn.c:1.87
> --- src/sys/dev/pci/if_iwn.c:1.86 Mon Oct 23 09:31:18 2017
> +++ src/sys/dev/pci/if_iwn.c  Tue Jan 16 07:05:24 2018
> @@ -1,4 +1,4 @@
> -/*   $NetBSD: if_iwn.c,v 1.86 2017/10/23 09:31:18 msaitoh Exp $  */
> +/*   $NetBSD: if_iwn.c,v 1.87 2018/01/16 07:05:24 maxv Exp $ */
>  /*   $OpenBSD: if_iwn.c,v 1.135 2014/09/10 07:22:09 dcoppa Exp $ */
>  
>  /*-
> @@ -22,7 +22,7 @@
>   * adapters.
>   */
>  #include 
> -__KERNEL_RCSID(0, "$NetBSD: if_iwn.c,v 1.86 2017/10/23 09:31:18 msaitoh Exp 
> $");
> +__KERNEL_RCSID(0, "$NetBSD: if_iwn.c,v 1.87 2018/01/16 07:05:24 maxv Exp $");
>  
>  #define IWN_USE_RBUF /* Use local storage for RX */
>  #undef IWN_HWCRYPTO  /* XXX does not even compile yet */
> @@ -6607,12 +6607,13 @@ iwn_fix_channel(struct ieee80211com *ic,
>   efrm = mtod(m, uint8_t *) + m->m_len;
>  
>   frm += 12;  /* skip tstamp, bintval and capinfo fields */
> - while (frm < efrm) {
> - if (*frm == IEEE80211_ELEMID_DSPARMS)
> + while (frm + 2 < efrm) {
> + if (*frm == IEEE80211_ELEMID_DSPARMS) {
>  #if IEEE80211_CHAN_MAX < 255
> - if (frm[2] <= IEEE80211_CHAN_MAX)
> + if (frm[2] <= 

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

[Masanobu SAITOH]

On 2017/11/30 12:53, SAITOH Masanobu wrote:

Module Name:src
Committed By:   msaitoh
Date:   Thu Nov 30 03:53:24 UTC 2017

Modified Files:
src/sys/dev/pci: if_wm.c

Log Message:
  Don't allocate MSI-X interrupt on 82583. 82583 chip has a MSI-X capability in
the PCI configuration space but it doesn't support it. At least the document
doesn't say anything about MSI-X. Fixes PR#57262 reported by


s/57262/52767/


Shinichi Doyashiki.

XXX pullup-8.


To generate a diff of this commit:
cvs rdiff -u -r1.544 -r1.545 src/sys/dev/pci/if_wm.c

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/pci/ixgbe

[Masanobu SAITOH]

On 2017/11/23 0:15, SAITOH Masanobu wrote:

Module Name:src
Committed By:   msaitoh
Date:   Wed Nov 22 15:15:09 UTC 2017

Modified Files:
src/sys/dev/pci/ixgbe: if_bypass.c ixgbe.c ixgbe.h

Log Message:
Fix a bug that bypass adapter's sysctls aren't set. Tested with non-genuine
X550-T2 bypass adapter:


 s/X550/X540/


- Call ixgbe_sysctl_instance() in ixgbe_bypass_init() to get sysctl's top node
   correctly.
- ixgbe_init_device_features() refers adapter->hw.bus.func for bypass adapter.
   Call set_lan_id() to set adapter->hw.bus.func before calling
   ixgbe_init_device_features(). Without this, bypass sysctl's are added to
   both the first and second port.
- Initalize node.sysctl_data before calling sysctl_lookup() to read correct
   value.


To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.2 src/sys/dev/pci/ixgbe/if_bypass.c
cvs rdiff -u -r1.112 -r1.113 src/sys/dev/pci/ixgbe/ixgbe.c
cvs rdiff -u -r1.27 -r1.28 src/sys/dev/pci/ixgbe/ixgbe.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/pci/ixgbe

[Masanobu SAITOH]

On 2017/06/27 19:33, SAITOH Masanobu wrote:

Module Name:src
Committed By:   msaitoh
Date:   Tue Jun 27 10:33:09 UTC 2017

Modified Files:
src/sys/dev/pci/ixgbe: ixgbe.c

Log Message:
  Fix a bug of ixg(4)'s media setting.

Before:
  ifconfig ixg0 media 100baseTX -> advertise 100Mbps only
  ifconfig ixg0 media 1000baseT -> advertise 1Gbps and 1000Mbps (NG)


s/1000Mbps/100Mbps/


  ifconfig ixg0 media 10Gbase-T -> advertise all (NG)
  ifconfig ixg0 media auto  -> advertise all

After:
  ifconfig ixg0 media 100baseTX -> advertise 100Mbps only
  ifconfig ixg0 media 1000baseT -> advertise 1Gbps only
  ifconfig ixg0 media 10Gbase-T -> advertise 10Gbps only
  ifconfig ixg0 media auto  -> advertise all


To generate a diff of this commit:
cvs rdiff -u -r1.93 -r1.94 src/sys/dev/pci/ixgbe/ixgbe.c

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

[Taylor R Campbell]

> Date: Thu, 2 Feb 2017 03:57:21 +
> From: co...@sdf.org
> 
> On Thu, Feb 02, 2017 at 03:41:22AM +, Jonathan A. Kollasch wrote:
> > @@ -432,6 +433,10 @@ wpi_detach(device_t self, int flags __un
> > pci_intr_disestablish(sc->sc_pct, sc->sc_ih);
> > sc->sc_ih = NULL;
> > }
> > +   if (sc->sc_pihp != NULL) {
> > +   pci_intr_release(sc->sc_pct, sc->sc_pihp, 1);
> > +   sc->sc_pihp = NULL;
> > +   }
> > mutex_enter(>sc_rsw_mtx);
> > sc->sc_dying = 1;
> > cv_signal(>sc_rsw_cv);
> > 
> 
> hmm, not introduced by you now, but is it safe that this function is 
> mutex_entering so late?
> shouldn't it mutex enter and set sc_dying first thing?

In this case, the purpose is of setting sc->sc_dying to cause
wpi_rsw_thread to terminate.  The only interesting thing that thread
does is wpi_getrfkill.  All that wpi_getrfkill uses is the device
register mapping (via wpi_mem_lock and wpi_mem_read) and sc->sc_rsw,
which wpi_detach tears down only after wpi_rsw_thread has terminated.
So I don't think there's a problem with that ordering.

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

[coypu]

On Thu, Feb 02, 2017 at 03:41:22AM +, Jonathan A. Kollasch wrote:
> @@ -432,6 +433,10 @@ wpi_detach(device_t self, int flags __un
>   pci_intr_disestablish(sc->sc_pct, sc->sc_ih);
>   sc->sc_ih = NULL;
>   }
> + if (sc->sc_pihp != NULL) {
> + pci_intr_release(sc->sc_pct, sc->sc_pihp, 1);
> + sc->sc_pihp = NULL;
> + }
>   mutex_enter(>sc_rsw_mtx);
>   sc->sc_dying = 1;
>   cv_signal(>sc_rsw_cv);
> 

hmm, not introduced by you now, but is it safe that this function is 
mutex_entering so late?
shouldn't it mutex enter and set sc_dying first thing?

thanks

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

[Christos Zoulas]

In article <58524a1b.5060...@netbsd.org>,
Nick Hudson   wrote:
>On 12/14/16 22:21, Christos Zoulas wrote:
>> Module Name: src
>> Committed By:christos
>> Date:Wed Dec 14 22:21:13 UTC 2016
>>
>> Modified Files:
>>  src/sys/dev/pci: if_sk.c
>>
>> Log Message:
>> Tidy up and make it look like the other drivers.
>
>Isn't ether_ioctl and an ifflags_cb (see ether_set_ifflags_cb) the 
>prefer method now?

Yes, we should just churn all the drivers for it.

christos

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

[Masanobu SAITOH]

On 2016/11/28 11:23, SAITOH Masanobu wrote:

Module Name:src
Committed By:   msaitoh
Date:   Mon Nov 28 02:23:33 UTC 2016

Modified Files:
src/sys/dev/pci: files.pci
src/sys/dev/pci/ixgbe: ixgbe.c ixgbe.h
Added Files:
src/sys/dev/pci/ixgbe: ix_txrx.c

Log Message:
 FreeBSD r280181 made new file ix_txrx.c and moved ixgbe.c and ixv's common


s/r280181/r280182/


code into it. Before sync with whole of them, just move ixgbe.c and ixv.c's
common code into ix_txrx.c from ixgbe.c. In this commit, only ixgbe.c is split
into the device dependent part and the common part. ixv.c isn't change to make
this commit no functional change. To use ixv.c with ix_txrx.c, it's required
to modify the common part's API and functions themselves.

 This commit is done to make the next change easy to understand.


To generate a diff of this commit:
cvs rdiff -u -r1.382 -r1.383 src/sys/dev/pci/files.pci
cvs rdiff -u -r0 -r1.1 src/sys/dev/pci/ixgbe/ix_txrx.c
cvs rdiff -u -r1.41 -r1.42 src/sys/dev/pci/ixgbe/ixgbe.c
cvs rdiff -u -r1.9 -r1.10 src/sys/dev/pci/ixgbe/ixgbe.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/pci

[coypu]

On Sun, Nov 27, 2016 at 12:15:56AM +0900, Kimihiro Nonaka wrote:
> Hi,
> 
> 2016-11-26 17:29 GMT+09:00  :
> 
> > This strangely causes fallpoint for sandpoint because:
> > sandpoint/include/pci_machdep.h:38:#define  __HAVE_PCI_MSI_MSIX
> > Does it really?
> 
> fixed
> 
> http://mail-index.netbsd.org/source-changes/2016/11/26/msg079370.html
> 
> regards,
> -- 
> Kimihiro Nonaka

Thanks!

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

[Kimihiro Nonaka]

Hi,

2016-11-26 17:29 GMT+09:00  :

> This strangely causes fallpoint for sandpoint because:
> sandpoint/include/pci_machdep.h:38:#define  __HAVE_PCI_MSI_MSIX
> Does it really?

fixed

http://mail-index.netbsd.org/source-changes/2016/11/26/msg079370.html

regards,
-- 
Kimihiro Nonaka

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

[coypu]

This strangely causes fallpoint for sandpoint because:
sandpoint/include/pci_machdep.h:38:#define  __HAVE_PCI_MSI_MSIX
Does it really?

Failure at 
http://releng.netbsd.org/builds/HEAD/201611252110Z/sandpoint.build.failed

if_re_pci.o: In function `re_pci_detach':
if_re_pci.c:(.text+0x70): undefined reference to `pci_intr_release'
if_re_pci.o: In function `re_pci_attach':
if_re_pci.c:(.text+0x37c): undefined reference to `pci_intr_alloc'
if_re_pci.c:(.text+0x3e0): undefined reference to `pci_intr_alloc'



On Fri, Nov 25, 2016 at 12:10:59PM +, Kengo NAKAHARA wrote:
> Module Name:  src
> Committed By: knakahara
> Date: Fri Nov 25 12:10:59 UTC 2016
> 
> Modified Files:
>   src/sys/dev/pci: pci_stub.c pcivar.h
> 
> Log Message:
> provide all PCI MSI/MSI-X manipulation stub functions.
> 
> "#ifdef __HAVE_PCI_MSI_MSIX" workaround such as nvme_pci(4) is not required
> any more.
> http://mail-index.netbsd.org/source-changes/2016/09/17/msg077799.html
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.5 -r1.6 src/sys/dev/pci/pci_stub.c
> cvs rdiff -u -r1.108 -r1.109 src/sys/dev/pci/pcivar.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/pci

[Michael]

On Wed, 16 Nov 2016 22:05:19 +
"Michael Lorenz"  wrote:

> Module Name:  src
> Committed By: macallan
> Date: Wed Nov 16 22:05:19 UTC 2016
> 
> Modified Files:
>   src/sys/dev/pci: voyager.c
> 
> Log Message:
> add more plumbing to pass a clockframe around so pwmclock can work again
> 
> Hi nick!

My brainfart, not his. Message changed on the server:

pass clockframe aropund in voyager_intr()

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

[Christos Zoulas]

On Nov 2,  9:02am, p...@whooppee.com (Paul Goyette) wrote:
-- Subject: Re: CVS commit: src/sys/dev/pci

| Even in the non-overflow case, if you write x-1 chars (not including the 
| trailing NUL) to a buffer of size x, the updated value of len will be 1 
| (and dest would point to the last valid character of the buffer).  We 
| would need additional code in the non-overflow path
| 
|   if (*len == 1)
|   *len = 0;
| 
| to maintain consistency with the overflow path.
| 

I guess what you have now is simpler; keep it :-)

christos

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

[Paul Goyette]

On Tue, 1 Nov 2016, Christos Zoulas wrote:


On Nov 2,  8:49am, p...@whooppee.com (Paul Goyette) wrote:
-- Subject: Re: CVS commit: src/sys/dev/pci

| > Why *len = 1 here? Shouldn't it be 0 since there is no more room left?
|
| No.  :)
|
| The maximum number of characters actually written by vsnprintf() will
| never exceed (len - 1).  So, dest gets incremented by the max, and len
| gets decremented by the max.
|
| There is always enough room left for vsnprintf() to create a new
| trailing NUL.

But that's doing extra work for no reason...


Perhaps.  But, since dest points to the last valid character of the 
output buffer (not to the first character after the buffer), there 
really is room for one more character.


Even in the non-overflow case, if you write x-1 chars (not including the 
trailing NUL) to a buffer of size x, the updated value of len will be 1 
(and dest would point to the last valid character of the buffer).  We 
would need additional code in the non-overflow path


if (*len == 1)
*len = 0;

to maintain consistency with the overflow path.


+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+--+--++

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

[Christos Zoulas]

On Nov 2,  8:49am, p...@whooppee.com (Paul Goyette) wrote:
-- Subject: Re: CVS commit: src/sys/dev/pci

| > Why *len = 1 here? Shouldn't it be 0 since there is no more room left?
| 
| No.  :)
| 
| The maximum number of characters actually written by vsnprintf() will 
| never exceed (len - 1).  So, dest gets incremented by the max, and len 
| gets decremented by the max.
| 
| There is always enough room left for vsnprintf() to create a new 
| trailing NUL.

But that's doing extra work for no reason...

christos

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

[Paul Goyette]

On Wed, 2 Nov 2016, Christos Zoulas wrote:


In article <20161102003956.35d12f...@cvs.netbsd.org>,
Paul Goyette  wrote:

-=-=-=-=-=-

+   /* Handle overflow */
+   if ((size_t)count >= *len) {
+   *dest += *len - 1;
+   *len = 1;


Why *len = 1 here? Shouldn't it be 0 since there is no more room left?


No.  :)

The maximum number of characters actually written by vsnprintf() will 
never exceed (len - 1).  So, dest gets incremented by the max, and len 
gets decremented by the max.


There is always enough room left for vsnprintf() to create a new 
trailing NUL.



+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+--+--++

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

[Christos Zoulas]

In article <20161102003956.35d12f...@cvs.netbsd.org>,
Paul Goyette  wrote:
>-=-=-=-=-=-
>
>+  /* Handle overflow */
>+  if ((size_t)count >= *len) {
>+  *dest += *len - 1;
>+  *len = 1;

Why *len = 1 here? Shouldn't it be 0 since there is no more room left?

christos

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

[Christos Zoulas]

On Oct 30,  6:24pm, p...@whooppee.com (Paul Goyette) wrote:
-- Subject: Re: CVS commit: src/sys/dev/pci

| One more revision.  This one updates the length parameter as well as the 
| dest pointer, so the caller doesn't need to recalculate on each call.
| 
| I've also changed it to return an int value rather than void:
| 
|   x = 0   success
|   x < 0   vsnprintf() failed (for userland code, errno will
|   contain details)
|   x > 0   success, however the output was truncated to avoid
|   overflowing the output buffer.
| 
| This variant seems to work just fine, both in-kernel and userland.

You don't need 2 static decls, you can put all the info in one.

christos

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

[Paul Goyette]

On Fri, 28 Oct 2016, Paul Goyette wrote:

Attached is a revised patch, which has a prototype/signature much more 
similar to snprintf(), more appropriate variable types, and improved 
comments/documentation.


I'll plan on committing this sometime late next week...


One more revision.  This one updates the length parameter as well as the 
dest pointer, so the caller doesn't need to recalculate on each call.


I've also changed it to return an int value rather than void:

x = 0   success
x < 0vsnprintf() failed (for userland code, errno will
contain details)
x > 0success, however the output was truncated to avoid
overflowing the output buffer.

This variant seems to work just fine, both in-kernel and userland.


+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+--+--++Index: pci_subr.c
===
RCS file: /cvsroot/src/sys/dev/pci/pci_subr.c,v
retrieving revision 1.152
diff -u -p -r1.152 pci_subr.c
--- pci_subr.c  20 Oct 2016 04:11:02 -  1.152
+++ pci_subr.c  30 Oct 2016 03:00:18 -
@@ -54,6 +54,7 @@ __KERNEL_RCSID(0, "$NetBSD: pci_subr.c,v
 #include 
 #else
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -567,6 +568,45 @@ static const struct pci_class pci_class[
 
 DEV_VERBOSE_DEFINE(pci);
 
+/*
+ * Append a formatted string to dest without writing more than len
+ * characters (including the trailing NUL character).  dest and len
+ * are updated for use in subsequent calls to snappendf().
+ *
+ * Returns 0 on success, a negative value if vnsprintf() fails, or
+ * a positive value if the dest buffer would have overflowed.
+ */
+static int
+snappendf(char **, size_t *, const char * restrict, ...) __printflike(3,4);
+
+static int
+snappendf(char **dest, size_t *len, const char * restrict fmt, ...)
+{
+   va_list ap;
+   int count;
+
+   va_start(ap, fmt);
+   count = vsnprintf(*dest, *len, fmt, ap);
+   va_end(ap);
+
+   /* Let vsnprintf() errors bubble up to caller */
+   if (count < 0 || *len == 0)
+   return count;
+
+   /* Handle overflow */
+   if ((size_t)count >= *len) {
+   *dest += *len - 1;
+   *len = 1;
+   return 1;
+   }
+
+   /* Update dest & len to point at trailing NUL */
+   *dest += count;
+   *len -= count;
+   
+   return 0;
+}
+
 void
 pci_devinfo(pcireg_t id_reg, pcireg_t class_reg, int showclass, char *cp,
 size_t l)
@@ -577,9 +617,6 @@ pci_devinfo(pcireg_t id_reg, pcireg_t cl
pci_revision_t revision;
char vendor[PCI_VENDORSTR_LEN], product[PCI_PRODUCTSTR_LEN];
const struct pci_class *classp, *subclassp, *interfacep;
-   char *ep;
-
-   ep = cp + l;
 
pciclass = PCI_CLASS(class_reg);
subclass = PCI_SUBCLASS(class_reg);
@@ -612,32 +649,31 @@ pci_devinfo(pcireg_t id_reg, pcireg_t cl
interfacep++;
}
 
-   cp += snprintf(cp, ep - cp, "%s %s", vendor, product);
+   (void)snappendf(, , "%s %s", vendor, product);
if (showclass) {
-   cp += snprintf(cp, ep - cp, " (");
+   (void)snappendf(, , " (");
if (classp->name == NULL)
-   cp += snprintf(cp, ep - cp,
-   "class 0x%02x, subclass 0x%02x", pciclass, 
subclass);
+   (void)snappendf(, ,
+   "class 0x%02x, subclass 0x%02x",
+   pciclass, subclass);
else {
if (subclassp == NULL || subclassp->name == NULL)
-   cp += snprintf(cp, ep - cp,
+   (void)snappendf(, ,
"%s, subclass 0x%02x",
classp->name, subclass);
else
-   cp += snprintf(cp, ep - cp, "%s %s",
+   (void)snappendf(, , "%s %s",
subclassp->name, classp->name);
}
if ((interfacep == NULL) || (interfacep->name == NULL)) {
if (interface != 0)
-   cp += snprintf(cp, ep - cp,
-   ", interface 0x%02x", interface);
+   (void)snappendf(, , ", interface 0x%02x",
+   interface);
} else if (strncmp(interfacep->name, "", 1) != 0)
-   cp += snprintf(cp, ep - cp, ", %s",
-   

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

[Christos Zoulas]

On Oct 28,  1:18pm, p...@whooppee.com (Paul Goyette) wrote:
-- Subject: Re: CVS commit: src/sys/dev/pci

| snappendprintf() seemed a rather unwieldly function name, so I called it 
| snappendf()!

:-) I read this like snapper at first.

| The attached diffs implement this function and use in throughout 
| pci_devinfo().
| 
| At some time in the future we will eventually get products whose verbose 
| description exceed the currently-allocated 256-byte buffer, so I really 
| think we should implement this (or something similar) to protect against 
| buffer overflows.  Does anyone have any major objections?  Are there 
| better alternatives than snappendf() that should be pursued instead?

I don't know. That looks ok except 'int len = end - *dest;' will produce
a warning unless casted. Taylor's suggestion which was similar was reasonable
too.

christos

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

[Paul Goyette]

...
or write a function that appends and moves the pointer, like

snappendprintf(char **dest, const char *end, fmt, ...)


snappendprintf() seemed a rather unwieldly function name, so I called it 
snappendf()!


The attached diffs implement this function and use in throughout 
pci_devinfo().



At some time in the future we will eventually get products whose verbose 
description exceed the currently-allocated 256-byte buffer, so I really think 
we should implement this (or something similar) to protect against buffer 
overflows.  Does anyone have any major objections?  Are there better 
alternatives than snappendf() that should be pursued instead?


Attached is a revised patch, which has a prototype/signature much more 
similar to snprintf(), more appropriate variable types, and improved 
comments/documentation.


I'll plan on committing this sometime late next week...

+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+--+--++Index: pci_subr.c
===
RCS file: /cvsroot/src/sys/dev/pci/pci_subr.c,v
retrieving revision 1.152
diff -u -p -r1.152 pci_subr.c
--- pci_subr.c  20 Oct 2016 04:11:02 -  1.152
+++ pci_subr.c  28 Oct 2016 08:58:59 -
@@ -54,6 +54,7 @@ __KERNEL_RCSID(0, "$NetBSD: pci_subr.c,v
 #include 
 #else
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -567,6 +568,33 @@ static const struct pci_class pci_class[
 
 DEV_VERBOSE_DEFINE(pci);
 
+/*
+ * Append a formatted string into dest without writing more than len
+ * characters (including the trailing NUL character).
+ *
+ * If vsnprintf() returns an error, *dest is unmodified.  Otherwise,
+ * *dest is updated to point to the last available character in the
+ * buffer (which is occupied by the string-terminating NUL).  It is
+ * not possible to differentiate between an output buffer that is
+ * exactly filled to capacity and one that would have overflowed.
+ */
+static void
+snappendf(char **, size_t, const char * restrict, ...) __printflike(3,4);
+
+static void
+snappendf(char **dest, size_t len, const char * restrict fmt, ...)
+{
+   va_list ap;
+   int count;
+
+   va_start(ap, fmt);
+   count = vsnprintf(*dest, len, fmt, ap);
+   va_end(ap);
+   if (count < 0 || len == 0)
+   return;
+   *dest += MIN((size_t)count, len - 1);
+}
+
 void
 pci_devinfo(pcireg_t id_reg, pcireg_t class_reg, int showclass, char *cp,
 size_t l)
@@ -612,32 +640,29 @@ pci_devinfo(pcireg_t id_reg, pcireg_t cl
interfacep++;
}
 
-   cp += snprintf(cp, ep - cp, "%s %s", vendor, product);
+   snappendf(, ep - cp, "%s %s", vendor, product);
if (showclass) {
-   cp += snprintf(cp, ep - cp, " (");
+   snappendf(, ep - cp, " (");
if (classp->name == NULL)
-   cp += snprintf(cp, ep - cp,
-   "class 0x%02x, subclass 0x%02x", pciclass, 
subclass);
+   snappendf(, ep - cp, "class 0x%02x, subclass 0x%02x",
+   pciclass, subclass);
else {
if (subclassp == NULL || subclassp->name == NULL)
-   cp += snprintf(cp, ep - cp,
-   "%s, subclass 0x%02x",
+   snappendf(, ep - cp, "%s, subclass 0x%02x",
classp->name, subclass);
else
-   cp += snprintf(cp, ep - cp, "%s %s",
+   snappendf(, ep - cp, "%s %s",
subclassp->name, classp->name);
}
if ((interfacep == NULL) || (interfacep->name == NULL)) {
if (interface != 0)
-   cp += snprintf(cp, ep - cp,
-   ", interface 0x%02x", interface);
+   snappendf(, ep - cp, ", interface 0x%02x",
+   interface);
} else if (strncmp(interfacep->name, "", 1) != 0)
-   cp += snprintf(cp, ep - cp, ", %s",
-   interfacep->name);
+   snappendf(, ep - cp, ", %s", interfacep->name);
if (revision != 0)
-   cp += snprintf(cp, ep - cp, ", revision 0x%02x",
-   revision);
-   cp += snprintf(cp, ep - cp, ")");
+   snappendf(, ep - cp, ", revision 0x%02x", revision);
+   snappendf(, ep - cp, ")");
}
 }
 

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

[Paul Goyette]

... I think that perhaps a variant that returns the
number of characters appended would be more useful. like
if (len >= max)
dest = end;
else
dest += len;

but, I would leave the whole thing alone instead :-)
or write a function that appends and moves the pointer, like

snappendprintf(char **dest, const char *end, fmt, ...)


snappendprintf() seemed a rather unwieldly function name, so I called it 
snappendf()!


The attached diffs implement this function and use in throughout 
pci_devinfo().



At some time in the future we will eventually get products whose verbose 
description exceed the currently-allocated 256-byte buffer, so I really 
think we should implement this (or something similar) to protect against 
buffer overflows.  Does anyone have any major objections?  Are there 
better alternatives than snappendf() that should be pursued instead?




+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+--+--++Index: pci_subr.c
===
RCS file: /cvsroot/src/sys/dev/pci/pci_subr.c,v
retrieving revision 1.152
diff -u -p -r1.152 pci_subr.c
--- pci_subr.c  20 Oct 2016 04:11:02 -  1.152
+++ pci_subr.c  28 Oct 2016 05:07:41 -
@@ -54,6 +54,7 @@ __KERNEL_RCSID(0, "$NetBSD: pci_subr.c,v
 #include 
 #else
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -567,6 +568,25 @@ static const struct pci_class pci_class[
 
 DEV_VERBOSE_DEFINE(pci);
 
+/*
+ * Append a formatted string into dest without writing beyond end, and
+ * update dest to point to next available character position
+ */
+static void
+snappendf(char **dest, const char *end, const char * restrict fmt, ...)
+{
+   va_list ap;
+   int count;
+   int len = end - *dest;
+
+   va_start(ap, fmt);
+   count = vsnprintf(*dest, len, fmt, ap);
+   va_end(ap);
+   if (count > len)
+   count = len;
+   *dest += count;
+}
+
 void
 pci_devinfo(pcireg_t id_reg, pcireg_t class_reg, int showclass, char *cp,
 size_t l)
@@ -612,32 +632,29 @@ pci_devinfo(pcireg_t id_reg, pcireg_t cl
interfacep++;
}
 
-   cp += snprintf(cp, ep - cp, "%s %s", vendor, product);
+   snappendf(, ep, "%s %s", vendor, product);
if (showclass) {
-   cp += snprintf(cp, ep - cp, " (");
+   snappendf(, ep, " (");
if (classp->name == NULL)
-   cp += snprintf(cp, ep - cp,
-   "class 0x%02x, subclass 0x%02x", pciclass, 
subclass);
+   snappendf(, ep, "class 0x%02x, subclass 0x%02x",
+   pciclass, subclass);
else {
if (subclassp == NULL || subclassp->name == NULL)
-   cp += snprintf(cp, ep - cp,
-   "%s, subclass 0x%02x",
+   snappendf(, ep, "%s, subclass 0x%02x",
classp->name, subclass);
else
-   cp += snprintf(cp, ep - cp, "%s %s",
+   snappendf(, ep, "%s %s",
subclassp->name, classp->name);
}
if ((interfacep == NULL) || (interfacep->name == NULL)) {
if (interface != 0)
-   cp += snprintf(cp, ep - cp,
-   ", interface 0x%02x", interface);
+   snappendf(, ep, ", interface 0x%02x",
+   interface);
} else if (strncmp(interfacep->name, "", 1) != 0)
-   cp += snprintf(cp, ep - cp, ", %s",
-   interfacep->name);
+   snappendf(, ep, ", %s", interfacep->name);
if (revision != 0)
-   cp += snprintf(cp, ep - cp, ", revision 0x%02x",
-   revision);
-   cp += snprintf(cp, ep - cp, ")");
+   snappendf(, ep, ", revision 0x%02x", revision);
+   snappendf(, ep, ")");
}
 }
 

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

[Paul Goyette]

On Wed, 26 Oct 2016, Paul Goyette wrote:


HOWEVER,

Looking at dev_untokenstring() it would appear that there is a potential 
problem there!  We have


   cp = buf + strlcat(buf, words + *token, len - 2);
   cp[0] = ' ';
   cp[1] = '\0';


Since strlcat(3) (quoting the man page) "return[s] the total length of the 
string [it] tried to create", it is entirely possible for cp to point beyond 
the end of the buffer.  Thus the insertion of a trailing space-between-word 
and the NUL character could occur on some random location.


It would seem to me the this code should be written as

newlen = strlcat(buf, word + *token, len - 2);
if (newlen > len - 2)
newlen = len - 2;
cp = buf + newlen;
cp[0] = ' ';
cp[1] = '\0';


I have confirmed that this actually is the root cause of the stack 
overflow problem.


With this fix, and the original buffer size of 64, I get a truncated 
product description (as expected), but the stack overflow is gone.


With this fix and the updated buffer size, I get the full description 
without any overflow.


With the original code (smaller buffer, above fix not included) I got 
consistent stack overflow instead of proper printing of the product 
description.


The above fix has been committed.


+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+--+--++

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

[Paul Goyette]

On Wed, 26 Oct 2016, Christos Zoulas wrote:


In article ,
Paul Goyette   wrote:

On Wed, 26 Oct 2016, matthew green wrote:


i think you're right that the 'cp' manipulation is the problem.
snprintf() will return the "desired" size, so upon the first
attempted overflow the 'cp' is moved beyond 'ep', and then the
next snprintf() gets a negative aka extremely massive value
for the buffer length and actual overflow happens here, and ssp
detects it.

the fix would be to change this:

cp += snprintf(cp, ep - cp, ...);

into this:

len = snprintf(cp, ep - cp, ...);
if (len > ep - cp)
return;
cp += len;

which is annoying because there are a lot of the former.


There's only 9 snprintf() calls.  I could simply provide a macro:

#define ADD_TEXT(dest, end, format, ...)\
{   \
int len, max = (end) - (dest);  \

len = snprintf((dest), max, (format), __VA_ARGS__); \
if (len > max)   \
return; \
(dest) += len;  \
}


Then all of the snprintf() calls become simply

ADD_TEXT(cp, ep, , ...);

(Of course, after last use I'd add a #undef ADD_TEXT to clean up...)


Do you really want to stop from attaching the driver because you could
not print it's name? ...


I don't see where this would prevent the device from attaching. It would 
simply return a truncated description...




... I think that perhaps a variant that returns the
number of characters appended would be more useful. like
if (len >= max)
dest = end;
else
dest += len;

but, I would leave the whole thing alone instead :-)
or write a function that appends and moves the pointer, like

snappendprintf(char **dest, const char *end, fmt, ...)



Well, as I have already pointed out to mrg@ in private Email, this 
change doesn't really fix the problem anyway!


During my earlier debugging I had confirmed that the caller of 
pci_devinfo() had provided a buffer of size 256, which is more than 
enough to contain the entire device description.  At no time did cp 
point beyond the end of the buffer, not even at the very end of the 
function.


The only actual "overflow" that occurs is within pci_findproduct().  It 
is the call to this function that provides a PCI_PRODUCTSTR_LEN-sized 
buffer.  I have confirmed that pci_findproduct() properly truncates its 
result and doesn't write beyond the buffer provided.  pci_devinfo() 
provides "char product[PCI_PRODUCTSTR_LEN]" (and similarly vendor[]).


With my specific device (0x8086, 0x6f8a),


"Core i7-6xxxK/Xeon-D Memory Cont"/* chars  0 - 31 */
"roller (Target Address, Thermal,"/* chars 32 - 63 */
" RAS)"   /* chars 64 - 68 */

and setting the length back to 64, the returned value was truncated (by 
dev_untokenstring() in src/dev/dev_verbose.c) at the 'm' in "Thermal".


So, I'm still unclear where the stack overflow actually occurs, and 
until that question is resolved, I'm not going to make any changes!



HOWEVER,

Looking at dev_untokenstring() it would appear that there is a potential 
problem there!  We have


cp = buf + strlcat(buf, words + *token, len - 2);
cp[0] = ' ';
cp[1] = '\0';


Since strlcat(3) (quoting the man page) "return[s] the total length of 
the string [it] tried to create", it is entirely possible for cp to 
point beyond the end of the buffer.  Thus the insertion of a trailing 
space-between-word and the NUL character could occur on some random 
location.


It would seem to me the this code should be written as

newlen = strlcat(buf, word + *token, len - 2);
if (newlen > len - 2)
newlen = len - 2;
cp = buf + newlen;
cp[0] = ' ';
cp[1] = '\0';

???


+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+--+--++

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

[Taylor R Campbell]

   Date: Wed, 26 Oct 2016 06:10:39 +0800 (PHT)
   From: Paul Goyette 

   There's only 9 snprintf() calls.  I could simply provide a macro:

   #define ADD_TEXT(dest, end, format, ...) \
   {\
   int len, max = (end) - (dest);   \

   len = snprintf((dest), max, (format), __VA_ARGS__); \
   if (len > max)   \
   return;  \
   (dest) += len;   \
   }


   Then all of the snprintf() calls become simply

   ADD_TEXT(cp, ep, , ...);

   (Of course, after last use I'd add a #undef ADD_TEXT to clean up...)

Maybe we should have a standard function to do this:

if ((error = snprintf_inplace(, , fmt, x, y, z)) != 0)
goto out;

would be equivalent to

nfmt = snprintf(p, n, fmt, x, y, z);
if (nfmt > n) {
n = nfmt;
error = ETRUNC;
goto out;
}
p += nfmt;
n -= nfmt;

or something like that, with the appropriate choice of update so that
it is easy either to report an error or to realloc a buffer and retry.
(Of course, then we need to decide whether p is const-qualified or
not, signed or unsigned or unqualified char,   Bleh.)

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

[Christos Zoulas]

In article ,
Paul Goyette   wrote:
>On Wed, 26 Oct 2016, matthew green wrote:
>
>> i think you're right that the 'cp' manipulation is the problem.
>> snprintf() will return the "desired" size, so upon the first
>> attempted overflow the 'cp' is moved beyond 'ep', and then the
>> next snprintf() gets a negative aka extremely massive value
>> for the buffer length and actual overflow happens here, and ssp
>> detects it.
>>
>> the fix would be to change this:
>>
>>  cp += snprintf(cp, ep - cp, ...);
>>
>> into this:
>>
>>  len = snprintf(cp, ep - cp, ...);
>>  if (len > ep - cp)
>>  return;
>>  cp += len;
>>
>> which is annoying because there are a lot of the former.
>
>There's only 9 snprintf() calls.  I could simply provide a macro:
>
>#define ADD_TEXT(dest, end, format, ...)   \
>   {   \
>   int len, max = (end) - (dest);  \
>
>   len = snprintf((dest), max, (format), __VA_ARGS__); \
>   if (len > max)  \
>   return; \
>   (dest) += len;  \
>   }
>
>
>Then all of the snprintf() calls become simply
>
>   ADD_TEXT(cp, ep, , ...);
>
>(Of course, after last use I'd add a #undef ADD_TEXT to clean up...)

Do you really want to stop from attaching the driver because you could
not print it's name? I think that perhaps a variant that returns the
number of characters appended would be more useful. like
if (len >= max)
dest = end;
else
dest += len;

but, I would leave the whole thing alone instead :-)
or write a function that appends and moves the pointer, like

snappendprintf(char **dest, const char *end, fmt, ...) 

christos

re: CVS commit: src/sys/dev/pci

[Paul Goyette]

On Wed, 26 Oct 2016, matthew green wrote:


i think you're right that the 'cp' manipulation is the problem.
snprintf() will return the "desired" size, so upon the first
attempted overflow the 'cp' is moved beyond 'ep', and then the
next snprintf() gets a negative aka extremely massive value
for the buffer length and actual overflow happens here, and ssp
detects it.

the fix would be to change this:

cp += snprintf(cp, ep - cp, ...);

into this:

len = snprintf(cp, ep - cp, ...);
if (len > ep - cp)
return;
cp += len;

which is annoying because there are a lot of the former.


There's only 9 snprintf() calls.  I could simply provide a macro:

#define ADD_TEXT(dest, end, format, ...)\
{   \
int len, max = (end) - (dest);  \

len = snprintf((dest), max, (format), __VA_ARGS__); \
if (len > max)   \
return; \
(dest) += len;  \
}


Then all of the snprintf() calls become simply

ADD_TEXT(cp, ep, , ...);

(Of course, after last use I'd add a #undef ADD_TEXT to clean up...)



+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+--+--++

re: CVS commit: src/sys/dev/pci

[Paul Goyette]

On Wed, 26 Oct 2016, matthew green wrote:


"Paul Goyette" writes:

Module Name:src
Committed By:   pgoyette
Date:   Tue Oct 25 05:43:40 UTC 2016

Modified Files:
src/sys/dev/pci: pci_verbose.h

Log Message:
Increase max string length for PCI Product names.  Affects only kernels
with PCIVERBOSE (or corresponding module).

We currently have a few product names that exceed the old limit, and
this is triggering an SSP check in pci_devinfo().  This commit doesn't
directly address the SSP issue, but pushes the can down the road...


i think this requires a kernel bump as the kernel can now write
more bytes than the driver may have provided.


Done - thanks for the reminder!


+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+--+--++

re: CVS commit: src/sys/dev/pci

[matthew green]

> We currently have a few product names that exceed the old limit, and
> this is triggering an SSP check in pci_devinfo().  This commit doesn't
> directly address the SSP issue, but pushes the can down the road...

i think you're right that the 'cp' manipulation is the problem.
snprintf() will return the "desired" size, so upon the first
attempted overflow the 'cp' is moved beyond 'ep', and then the
next snprintf() gets a negative aka extremely massive value
for the buffer length and actual overflow happens here, and ssp
detects it.

the fix would be to change this:

cp += snprintf(cp, ep - cp, ...);

into this:

len = snprintf(cp, ep - cp, ...);
if (len > ep - cp)
return;
cp += len;

which is annoying because there are a lot of the former.


.mrg.

re: CVS commit: src/sys/dev/pci

[matthew green]

"Paul Goyette" writes:
> Module Name:  src
> Committed By: pgoyette
> Date: Tue Oct 25 05:43:40 UTC 2016
> 
> Modified Files:
>   src/sys/dev/pci: pci_verbose.h
> 
> Log Message:
> Increase max string length for PCI Product names.  Affects only kernels
> with PCIVERBOSE (or corresponding module).
> 
> We currently have a few product names that exceed the old limit, and
> this is triggering an SSP check in pci_devinfo().  This commit doesn't
> directly address the SSP issue, but pushes the can down the road...

i think this requires a kernel bump as the kernel can now write
more bytes than the driver may have provided.


.mrg.

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

[David Holland]

On Sun, Jun 19, 2016 at 06:58:17AM +, David A. Holland wrote:
 > Modified Files:
 >  src/sys/dev/pci: arcmsr.c
 > 
 > Log Message:
 > Broaden the #if NBIO > 0 block. Should fix broken emips build.

Erm: "evbppc"

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

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

[NONAKA Kimihiro]

2015-11-07 7:11 GMT+09:00 matthew green :

> "NONAKA Kimihiro" writes:
>> Module Name:  src
>> Committed By: nonaka
>> Date: Fri Nov  6 14:22:17 UTC 2015
>>
>> Modified Files:
>>   src/sys/dev/pci: if_iwm.c if_iwmvar.h if_rtwn.c if_rtwnreg.h rtsx_pci.c
>>
>> Log Message:
>> Always use pci_intr_alloc(9)/pci_intr_release(9).
>
> doesn't this break platforms without __HAVE_PCI_MSI_MSIX?

knakahara@ provided pci_intr_alloc(9)/pci_intr_release(9) stub.

http://mail-index.netbsd.org/source-changes/2015/10/22/msg069597.html

Regards,
-- 
NONAKA Kimihiro

re: CVS commit: src/sys/dev/pci

[matthew green]

"NONAKA Kimihiro" writes:
> Module Name:  src
> Committed By: nonaka
> Date: Fri Nov  6 14:22:17 UTC 2015
> 
> Modified Files:
>   src/sys/dev/pci: if_iwm.c if_iwmvar.h if_rtwn.c if_rtwnreg.h rtsx_pci.c
> 
> Log Message:
> Always use pci_intr_alloc(9)/pci_intr_release(9).

doesn't this break platforms without __HAVE_PCI_MSI_MSIX?

i use wm(4) on sparc64 and have on alpha.


.mrg.

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

[Taylor R Campbell]

   Date: Sat, 07 Nov 2015 09:11:23 +1100
   from: matthew green 

   "NONAKA Kimihiro" writes:
   > Modified Files:
   >src/sys/dev/pci: if_iwm.c if_iwmvar.h if_rtwn.c if_rtwnreg.h rtsx_pci.c
   > 
   > Log Message:
   > Always use pci_intr_alloc(9)/pci_intr_release(9).

   doesn't this break platforms without __HAVE_PCI_MSI_MSIX?

   i use wm(4) on sparc64 and have on alpha.

It seems to me a better avenue would be to make a generic INTx-only
pci_intr_alloc/pci_intr_release so that we don't need preprocessor
conditionals to switch between the INTx-only and the INTx/MSI code.

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

[Antti Kantee]

On 25/08/15 06:54, matthew green wrote:

Antti Kantee writes:

Module Name:src
Committed By:   pooka
Date:   Mon Aug 24 23:52:18 UTC 2015

Modified Files:
src/sys/dev/pci: if_iwn.c

Log Message:
Remove #ifdef INET code.  Por que?  Because opt_inet.h was not included
and #ifdef INET was just a fancy way of saying #ifndef NetBSD.


h, while the code wasn't used before, are you sure it isn't
actually what we would rather be using?

i ask because i count 57 calls to arp_ifinit() in our tree today
(including this removed one), so i'm mostly convinced that it should
be called, but the failure mode of not doing so is less severe than
fails to work.


I assume that they're drivers which want to catch SIOCINITIFADDR instead 
of letting it pass through to ether_ioctl().  The call is 99% likely a 
leftover of porting the driver from OpenBSD.  At least when I ported iwm 
from NetBSD to OpenBSD, I added an #ifdef __OpenBSD__ block to call to 
arp_ifinit() from SIOCSIFADDR.  So, no, I don't think arp_ifinit() 
should be called from that driver.

re: CVS commit: src/sys/dev/pci

[matthew green]

Antti Kantee writes:
 Module Name:  src
 Committed By: pooka
 Date: Mon Aug 24 23:52:18 UTC 2015
 
 Modified Files:
   src/sys/dev/pci: if_iwn.c
 
 Log Message:
 Remove #ifdef INET code.  Por que?  Because opt_inet.h was not included
 and #ifdef INET was just a fancy way of saying #ifndef NetBSD.

h, while the code wasn't used before, are you sure it isn't
actually what we would rather be using?

i ask because i count 57 calls to arp_ifinit() in our tree today
(including this removed one), so i'm mostly convinced that it should
be called, but the failure mode of not doing so is less severe than
fails to work.


.mrg.

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

[Christos Zoulas]

In article 557e89ae.8070...@execsw.org,
Masanobu SAITOH  msai...@execsw.org wrote:

 I thought it's not required to wait API was fixed and committed.

I think is better to wait or ask before committing when there is
doubt.

 To support only MSI is easy, but MSI-X is not. This commit includes not
only pci_intr_establish() stuff but also include other MSI-X stuff to setup
MSI-X vector table and interrupt related functions. And, it'll take time
to check regression or stability to make it enable by default, so it's
important other people to test with it.

Even with the broken API, there is no need to triplicate the
interrupt allocation code, kcpuset distribution as I've shown before
in a different patch. Yes, it is more general if you copy the the
code because the tx/rx/link code *can potentially* be different,
but in this case it is not.

christos

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

[Masanobu SAITOH]

On 2015/06/14 5:42, Christos Zoulas wrote:
 In article 20150613154758.6971...@cvs.netbsd.org,
 SAITOH Masanobu source-changes-d@NetBSD.org wrote:
 -=-=-=-=-=-

 Module Name: src
 Committed By:msaitoh
 Date:Sat Jun 13 15:47:58 UTC 2015

 Modified Files:
  src/sys/dev/pci: if_wm.c if_wmreg.h

 Log Message:
 Add MSI/MSI-X support written by Kengo Nakahara. Some old devices' support
 is written by me. It's disabled by default. If you'd like to use, define
 WM_MSI_MSIX.

 Tested with:
  8254[3405617] (INTx even if it has MSI CAP because of a errata)
  8257[12], 82583 ICH8, ICH10, PCH2, PCH_LPT(I21[78]) (MSI)
  8257[456], 82580, I35[04], I21[01] (MSI-X)

 Not tested:
  82542, 82573, 80003, ICH9, PCH,

 
 I had raised quite a few issues about calcifying this interrupt API,
 also copying the code 3 times...
 
 christos

 I thought it's not required to wait API was fixed and committed.

 To support only MSI is easy, but MSI-X is not. This commit includes not
only pci_intr_establish() stuff but also include other MSI-X stuff to setup
MSI-X vector table and interrupt related functions. And, it'll take time
to check regression or stability to make it enable by default, so it's
important other people to test with it.

 The same work will be required for RAID controllders' drviers.

 Thanks.

-- 
---
SAITOH Masanobu (msai...@execsw.org
 msai...@netbsd.org)

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

[Christos Zoulas]

In article 20150613154758.6971...@cvs.netbsd.org,
SAITOH Masanobu source-changes-d@NetBSD.org wrote:
-=-=-=-=-=-

Module Name:   src
Committed By:  msaitoh
Date:  Sat Jun 13 15:47:58 UTC 2015

Modified Files:
   src/sys/dev/pci: if_wm.c if_wmreg.h

Log Message:
 Add MSI/MSI-X support written by Kengo Nakahara. Some old devices' support
is written by me. It's disabled by default. If you'd like to use, define
WM_MSI_MSIX.

Tested with:
   8254[3405617] (INTx even if it has MSI CAP because of a errata)
   8257[12], 82583 ICH8, ICH10, PCH2, PCH_LPT(I21[78]) (MSI)
   8257[456], 82580, I35[04], I21[01] (MSI-X)

Not tested:
   82542, 82573, 80003, ICH9, PCH,


I had raised quite a few issues about calcifying this interrupt API,
also copying the code 3 times...

christos

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

[Masanobu SAITOH]

On 2015/05/01 1:09, SAITOH Masanobu wrote:

Module Name:src
Committed By:   msaitoh
Date:   Thu Apr 30 16:09:06 UTC 2015

Modified Files:
src/sys/dev/pci: if_bge.c if_bgereg.h

Log Message:
  Use another firmware command in bge_asf_driver_up(). This change fixes
a bug that watchdog timeout occurs every 20-30 minutes on HP ML110 G6
reported enami@ in PR#49657.


 The value of this command is the same as Linux.
I've cvs admin-ed now.



To generate a diff of this commit:
cvs rdiff -u -r1.284 -r1.285 src/sys/dev/pci/if_bge.c
cvs rdiff -u -r1.89 -r1.90 src/sys/dev/pci/if_bgereg.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/pci

[Martin Husemann]

Ooops, I accidently didn't cc the list:

- Forwarded message from Martin Husemann mar...@duskware.de -

Date: Tue, 7 Apr 2015 22:30:23 +0200
From: Martin Husemann mar...@duskware.de
To: Masanobu SAITOH msai...@execsw.org
Subject: Re: CVS commit: src/sys/dev/pci
In-Reply-To: 5523b0f9.6000...@execsw.org
User-Agent: Mutt/1.4.2.3i

On Tue, Apr 07, 2015 at 07:27:05PM +0900, Masanobu SAITOH wrote:
 What does this change fix? To prevent panic on shutdown?

Yes: bge0 detaches (say on cpu0) while concurrently the callout triggers
(say on cpu1) and the internal data structures are freed concurrently to
the access (in bge_tick).

My notebook regulary crashed very late during shutdown, but I usually have
ddb.onpanic turned off, so I couldn't examine more closely.

The change makes sure all ongoing callouts have completed before the
detach frees internal stuff.

 Almost all drviers have the same code. Should we fix all of them
 like this?

I think so. Maybe the halt can be done unconditionally, but only if
callout_destroy() will be used imediately afterwards it is important
to do so.

ozaki-r and I have been chasing similar issues for a while and fixed
them whenever we could get hold of concrete lossage.

Martin

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

[Masanobu SAITOH]

Hi, Martin.

On 2015/04/06 16:38, Martin Husemann wrote:

Module Name:src
Committed By:   martin
Date:   Mon Apr  6 07:38:17 UTC 2015

Modified Files:
src/sys/dev/pci: if_bge.c

Log Message:
Make sure to halt (not just stop) the bge_tick callout during detach.


To generate a diff of this commit:
cvs rdiff -u -r1.280 -r1.281 src/sys/dev/pci/if_bge.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


What does this change fix? To prevent panic on shutdown?

Almost all drviers have the same code. Should we fix all of them
like this?

--
---
SAITOH Masanobu (msai...@execsw.org
 msai...@netbsd.org)

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

[Christos Zoulas]

In article 20150223130937.ddec...@cvs.netbsd.org,
NONAKA Kimihiro source-changes-d@NetBSD.org wrote:
-=-=-=-=-=-

Module Name:   src
Committed By:  nonaka
Date:  Mon Feb 23 13:09:37 UTC 2015

Modified Files:
   src/sys/dev/pci: if_iwm.c

Log Message:
CID 1271021: Overrunning array in-in_ridx of 15 bytes at byte offset
15 using index i (which evaluates to 15).

http://mail-index.netbsd.org/coverity-updates/2015/02/21/msg000115.html

Did you look at the full coverity report? I don't understand how this
can happen?

christos

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

[NONAKA Kimihiro]

I have not seen it.
At the moment, I don't think the problem happen.

2015-02-23 22:21 GMT+09:00 Christos Zoulas chris...@astron.com:
 In article 20150223130937.ddec...@cvs.netbsd.org,
 NONAKA Kimihiro source-changes-d@NetBSD.org wrote:
-=-=-=-=-=-

Module Name:   src
Committed By:  nonaka
Date:  Mon Feb 23 13:09:37 UTC 2015

Modified Files:
   src/sys/dev/pci: if_iwm.c

Log Message:
CID 1271021: Overrunning array in-in_ridx of 15 bytes at byte offset
15 using index i (which evaluates to 15).

http://mail-index.netbsd.org/coverity-updates/2015/02/21/msg000115.html

 Did you look at the full coverity report? I don't understand how this
 can happen?

 christos

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

[Christos Zoulas]

On Feb 23, 11:13pm, nona...@gmail.com (NONAKA Kimihiro) wrote:
-- Subject: Re: CVS commit: src/sys/dev/pci

| I have not seen it.
| At the moment, I don't think the problem happen.

Please revert the fix then, or check the coverity website for the
explanation why it thinks that the index can be 15. It does not
seem possible from my cursory examination and we don't sandbag the
code against impossible errors. It is probably a coverity false
positive.

christos

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

[NONAKA Kimihiro]

2015-02-24 0:25 GMT+09:00 Christos Zoulas chris...@zoulas.com:

 Please revert the fix then, or check the coverity website for the
 explanation why it thinks that the index can be 15. It does not
 seem possible from my cursory examination and we don't sandbag the
 code against impossible errors. It is probably a coverity false
 positive.

I've revert this change.
I don't know how to look the full coverity report. Would you mind to teach me?

Regards,
-- 
NONAKA Kimihiro

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

[Christos Zoulas]

In article 20150217101125.1591...@cvs.netbsd.org,
SAITOH Masanobu source-changes-d@NetBSD.org wrote:
-=-=-=-=-=-

Module Name:   src
Committed By:  msaitoh
Date:  Tue Feb 17 10:11:25 UTC 2015

Modified Files:
   src/sys/dev/pci: if_bge.c

Log Message:
 Print bit setting of bge_asf_mode if BGE_DEBUG is set.


To generate a diff of this commit:
cvs rdiff -u -r1.278 -r1.279 src/sys/dev/pci/if_bge.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/pci/if_bge.c
diff -u src/sys/dev/pci/if_bge.c:1.278 src/sys/dev/pci/if_bge.c:1.279
--- src/sys/dev/pci/if_bge.c:1.278 Wed Feb 11 23:07:13 2015
+++ src/sys/dev/pci/if_bge.c   Tue Feb 17 10:11:24 2015
@@ -1,4 +1,4 @@
-/*$NetBSD: if_bge.c,v 1.278 2015/02/11 23:07:13 msaitoh Exp $ */
+/*$NetBSD: if_bge.c,v 1.279 2015/02/17 10:11:24 msaitoh Exp $ */
 
 /*
  * Copyright (c) 2001 Wind River Systems
@@ -79,7 +79,7 @@
  */
 
 #include sys/cdefs.h
-__KERNEL_RCSID(0, $NetBSD: if_bge.c,v 1.278 2015/02/11 23:07:13
msaitoh Exp $);
+__KERNEL_RCSID(0, $NetBSD: if_bge.c,v 1.279 2015/02/17 10:11:24
msaitoh Exp $);
 
 #include sys/param.h
 #include sys/systm.h
@@ -6053,6 +6053,7 @@ bge_debug_info(struct bge_softc *sc)
   if (sc-bge_flags  BGEF_TSO)
   printf( - TSO\n);
 
+  /* PHY related */
   if (sc-bge_phy_flags  BGEPHYF_NO_3LED)
   printf( - No 3 LEDs\n);
   if (sc-bge_phy_flags  BGEPHYF_CRC_BUG)
@@ -6069,6 +6070,14 @@ bge_debug_info(struct bge_softc *sc)
   printf( - adjust trim\n);
   if (sc-bge_phy_flags  BGEPHYF_NO_WIRESPEED)
   printf( - no wirespeed\n);
+
+  /* ASF related */
+  if (sc-bge_asf_mode  ASF_ENABLE)
+  printf( - ASF enable\n);
+  if (sc-bge_asf_mode  ASF_NEW_HANDSHARE)
+  printf( - ASF new handshake\n);
+  if (sc-bge_asf_mode  ASF_STACKUP)
+  printf( - ASF stackup\n);

Candidates for snprintb()?

christos

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

[Christos Zoulas]

On Sep 27,  3:44pm, m...@m00nbsd.net (Maxime Villard) wrote:
-- Subject: Re: CVS commit: src/sys/dev/pci

| This fix is wrong.
| 
| - memset(p, sizeof(struct twa_param_9k *), 10);
| + memset(p, 0, sizeof(*p));
| 
| p is a [10] array, your memset only initializes the first pointer.
| 
| Seriously, guy, ...

Yes, this is the case where *p is wrong. Thank c for making this inconsistent!

christos

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

[Maxime Villard]

Module Name:src
Committed By:   christos
Date:   Sun Sep 21 17:11:07 UTC 2014

Modified Files:
src/sys/dev/pci: twa.c

Log Message:
fix memset size inconsistency



This fix is wrong.

-   memset(p, sizeof(struct twa_param_9k *), 10);
+   memset(p, 0, sizeof(*p));

p is a [10] array, your memset only initializes the first pointer.

Seriously, guy, ...

re: CVS commit: src/sys/dev/pci/hdaudio

[matthew green]

Nathanial Sloss writes:
 Module Name:  src
 Committed By: nat
 Date: Sun Sep 21 10:41:23 UTC 2014
 
 Modified Files:
   src/sys/dev/pci/hdaudio: hdafg.c
 
 Log Message:
 Enable sysbeep(4) to be heard on speakers on Toughbook mk1 and mk5
 computers.  Also on computers with SigmaTel STAC 9200(D), 9202(D)
 9404(D), 9205(D) hdaudio controllers as well as Realtek ALC 231
 hdaudio controllers reporting as ALC 269.
 
 Addresses PR 45778.
 Addresses PR 48495.
 
 This commit was approved by christos@

would you mind replacing the magic numbers you've added with
some sort of defines or so?  these lines:

+   nid = 0x01;

+   response = hdaudio_command(sc-sc_codec, nid,
+   HDAFG_GET_ANACTRL, 0x00);

+   nid = 0xf;
+   response = hdaudio_command(sc-sc_codec, nid,
+   CORB_SET_AMPLIFIER_GAIN_MUTE, 0x7100);

thanks.


.mrg.

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

[Joerg Sonnenberger]

On Tue, Jul 01, 2014 at 04:43:17PM -0500, Jonathan A. Kollasch wrote:
 On Tue, Jul 01, 2014 at 12:07:39AM +0200, Joerg Sonnenberger wrote:
  On Mon, Jun 30, 2014 at 09:33:40PM +, Jonathan A. Kollasch wrote:
   Module Name:  src
   Committed By: jakllsch
   Date: Mon Jun 30 21:33:40 UTC 2014
   
   Modified Files:
 src/sys/dev/pci: if_wpireg.h
   
   Log Message:
   Apply OpenBSD src/sys/dev/pci/if_wpireg.h 1.17.
  
  Can we get a better commit message in the future?
 
 Well, the whole goal here was to bring us closer to (historical) OpenBSD,
 and the original 1.17 OpenBSD message is nearly completely irrelevant
 what this commit did.

Reduce diff to ... is a perfectly valid commit message :)

Joerg

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

[Izumi Tsutsui]

joerg@ wrote:

  Log Message:
  Apply OpenBSD src/sys/dev/pci/if_wpireg.h 1.17.
 
 Can we get a better commit message in the future?

Our commit guideline also says Give proper credit.
http://www.netbsd.org/developers/commit-guidelines.html

---
Izumi Tsutsui

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

[Joerg Sonnenberger]

On Tue, Jul 01, 2014 at 10:16:38PM +0900, Izumi Tsutsui wrote:
 joerg@ wrote:
 
   Log Message:
   Apply OpenBSD src/sys/dev/pci/if_wpireg.h 1.17.
  
  Can we get a better commit message in the future?
 
 Our commit guideline also says Give proper credit.
 http://www.netbsd.org/developers/commit-guidelines.html

Well, I am quite willing to accept the message above as proper
attribution, but it doesn't tell what the actual change was...

Joerg

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

[Izumi Tsutsui]

joerg@ wrote:

 On Tue, Jul 01, 2014 at 10:16:38PM +0900, Izumi Tsutsui wrote:
  joerg@ wrote:
  
Log Message:
Apply OpenBSD src/sys/dev/pci/if_wpireg.h 1.17.
   
   Can we get a better commit message in the future?
  
  Our commit guideline also says Give proper credit.
  http://www.netbsd.org/developers/commit-guidelines.html
 
 Well, I am quite willing to accept the message above as proper
 attribution, but it doesn't tell what the actual change was...

It looks enough pointer.
http://www.openbsd.org/cgi-bin/cvsweb/src/sys/dev/pci/if_wpireg.h#rev1.17

Your commit messages often lack credit or pointers and
it's rather annoying...

---
Izumi Tsutsui

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

[Jonathan A. Kollasch]

On Tue, Jul 01, 2014 at 12:07:39AM +0200, Joerg Sonnenberger wrote:
 On Mon, Jun 30, 2014 at 09:33:40PM +, Jonathan A. Kollasch wrote:
  Module Name:src
  Committed By:   jakllsch
  Date:   Mon Jun 30 21:33:40 UTC 2014
  
  Modified Files:
  src/sys/dev/pci: if_wpireg.h
  
  Log Message:
  Apply OpenBSD src/sys/dev/pci/if_wpireg.h 1.17.
 
 Can we get a better commit message in the future?

Well, the whole goal here was to bring us closer to (historical) OpenBSD,
and the original 1.17 OpenBSD message is nearly completely irrelevant
what this commit did.

So to answer your question; maybe.

Jonathan Kollasch

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

[Joerg Sonnenberger]

On Mon, Jun 30, 2014 at 09:33:40PM +, Jonathan A. Kollasch wrote:
 Module Name:  src
 Committed By: jakllsch
 Date: Mon Jun 30 21:33:40 UTC 2014
 
 Modified Files:
   src/sys/dev/pci: if_wpireg.h
 
 Log Message:
 Apply OpenBSD src/sys/dev/pci/if_wpireg.h 1.17.

Can we get a better commit message in the future?

Joerg

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

[Joerg Sonnenberger]

On Fri, Feb 28, 2014 at 09:14:03AM +0200, Alan Barrett wrote:
 On Thu, 27 Feb 2014, Joerg Sonnenberger wrote:
 Modified Files:
  src/sys/dev/pci: if_ti.c
 
 Log Message:
 Remove impossible checks.
 
 If new bugs are introduced in the future, then these tests might no
 longer be impossible.  So I'd prefer to change them to KASSERT or
 KDASSERT rather than deleting them.

Given the nature of this checks and the failure mode of such changes, I
don't see a point in the assert.

Joerg

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

[Alan Barrett]

On Thu, 27 Feb 2014, Joerg Sonnenberger wrote:

Modified Files:
src/sys/dev/pci: if_ti.c

Log Message:
Remove impossible checks.


If new bugs are introduced in the future, then these tests might 
no longer be impossible.  So I'd prefer to change them to KASSERT 
or KDASSERT rather than deleting them.


--apb (Alan Barrett)

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

[Izumi Tsutsui]

 Module Name:  src
 Committed By: jakllsch
 Date: Thu Jan 16 18:41:10 UTC 2014
 
 Modified Files:
   src/sys/dev/pci: genfb_pci.c
 
 Log Message:
 Fix PR kern/46376 with Nat Sloss's patch (with slight modification).
 Serial console now works on x86 with genfb enabled.

Didn't you read the following messages?
http://mail-index.netbsd.org/tech-kern/2014/01/15/msg016421.html
http://mail-index.netbsd.org/tech-kern/2014/01/15/msg016425.html

---
Izumi Tsutsui

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

[Matt Thomas]

On Jan 9, 2014, at 9:28 AM, SAITOH Masanobu msai...@netbsd.org wrote:

 Module Name:  src
 Committed By: msaitoh
 Date: Thu Jan  9 17:28:05 UTC 2014
 
 Modified Files:
   src/sys/dev/pci: pcidevs
 
 Log Message:
 Remove 88SE9128(0x913a) entry. At least one of 88SE9128 chip's
 product ID is 0x9123. I have this one. Add new 88SE912X entry
 with 0x9123. OK'ed by jakllsch.

sys/dev/pci/ahcisata_pci.c:170:25: error:
'PCI_PRODUCT_MARVELL2_88SE9128' undeclared here (not in a function)

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

[Jukka Ruohonen]

On Wed, Oct 16, 2013 at 03:32:30PM -0400, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Wed Oct 16 19:32:30 UTC 2013
 
 Modified Files:
   src/sys/dev/pci: cs4281.c
 
 Log Message:
 use __USE()

What is __USE()? Might be worth a brief manual page...

- Jukka.

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

[Martin Husemann]

On Thu, Oct 17, 2013 at 09:22:02AM +0300, Jukka Ruohonen wrote:
  Log Message:
  use __USE()
 
 What is __USE()? Might be worth a brief manual page...

It needs very explicit wording in misc/style.

Martin

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

[Jukka Ruohonen]

On Thu, Oct 17, 2013 at 08:26:33AM +0200, Martin Husemann wrote:
 On Thu, Oct 17, 2013 at 09:22:02AM +0300, Jukka Ruohonen wrote:
   Log Message:
   use __USE()
  
  What is __USE()? Might be worth a brief manual page...

Sure, although I was thinking a brief note (with caveats) appended to the
existing __UNCONST(3).

- Jukka.

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

[Christos Zoulas]

On Oct 17,  9:22am, jruoho...@iki.fi (Jukka Ruohonen) wrote:
-- Subject: Re: CVS commit: src/sys/dev/pci

| On Wed, Oct 16, 2013 at 03:32:30PM -0400, Christos Zoulas wrote:
|  Module Name:src
|  Committed By:   christos
|  Date:   Wed Oct 16 19:32:30 UTC 2013
|  
|  Modified Files:
|  src/sys/dev/pci: cs4281.c
|  
|  Log Message:
|  use __USE()
| 
| What is __USE()? Might be worth a brief manual page...

Where are the other cdefs.h macros documented? Should we start another man
page?

christos

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

[David Laight]

On Mon, Jul 22, 2013 at 02:52:02PM +, Soren S. Jorvang wrote:
 Module Name:  src
 Committed By: soren
 Date: Mon Jul 22 14:52:02 UTC 2013
 
 Modified Files:
   src/sys/dev/pci: puc.c
 
 Log Message:
 Oops.

Not the best of commit messages!
'Fix compilation' would be better.

David

-- 
David Laight: da...@l8s.co.uk

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

[Martin Husemann]

On Tue, Jul 23, 2013 at 08:21:34AM +0100, David Laight wrote:
 Not the best of commit messages!
 'Fix compilation' would be better.

Alas, it stil doesn't:

../../../../dev/pci/puc.c: In function 'puc_attach':
../../../../dev/pci/puc.c:269:76: error: invalid operands to binary - (have 
'bus_space_handle_t' and 'int')


You can not assume bus_handle_t to be integral.

Martin

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

[Martin Husemann]

This patch at least makes it compile - but I have no idea if I properly kept
the semantics correct, and can't test it.

Martin
Index: puc.c
===
RCS file: /cvsroot/src/sys/dev/pci/puc.c,v
retrieving revision 1.35
diff -u -p -r1.35 puc.c
--- puc.c   22 Jul 2013 14:52:02 -  1.35
+++ puc.c   23 Jul 2013 07:27:16 -
@@ -266,8 +266,12 @@ puc_attach(device_t parent, device_t sel
sc-sc_desc-ports[i].offset, subregion_handle);
if (is_console) {
 sc-sc_bar_mappings[barindex].mapped = 1;
-   sc-sc_bar_mappings[barindex].h = 
subregion_handle -
-   sc-sc_desc-ports[i].offset;   /* XXX hack */
+   bus_space_subregion(sc-sc_bar_mappings[barindex].t,
+   subregion_handle,
+   sc-sc_desc-ports[i].offset,
+   sc-sc_bar_mappings[barindex].s
+   - sc-sc_desc-ports[i].offset,
+   sc-sc_bar_mappings[barindex].h);
}
 #endif
if (!sc-sc_bar_mappings[barindex].mapped) {

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

[Soren Jorvang]

On 23/07/2013, at 09.28, Martin Husemann mar...@duskware.de wrote:
 This patch at least makes it compile - but I have no idea if I properly kept
 the semantics correct, and can't test it.

It would probably work, at least on platforms with simple bus_space 
implementations, but taking a subregion that is outside the original region 
seems iffy, so I have just marked it as an x86 hack for now.


-- 
Soren

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

[Martin Husemann]

On Tue, Jul 23, 2013 at 09:43:30AM +0200, Soren Jorvang wrote:

 It would probably work, at least on platforms with simple bus_space
 implementations, but taking a subregion that is outside the original
 region seems iffy, so I have just marked it as an x86 hack for now.

No, it is strictly forbidden:

 The subregion described by the new handle starts at byte offset offset
 into the region described by handle, with the size given by size, and
 must be wholly contained within the original region.

What are you doing here, and why can't you do a full new bus_space_map here?
I guess resource handling would not matter a lot for the is_console case.
(I haven't looked at the changes in details)

Anyway, thanks for fixing!

Martin

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

[Soren Jorvang]

On 23/07/2013, at 09.54, Martin Husemann mar...@duskware.de wrote:
 What are you doing here, and why can't you do a full new bus_space_map here?
 I guess resource handling would not matter a lot for the is_console case.
 (I haven't looked at the changes in details)


puc.c currently first maps the full spaces from the PCI BAR's and then creates 
a subregion for each of the com/lpt devices according to the list of known 
devices in pucdata.c, sometimes more than one per BAR.

The pre-autoconf console code creates a bus_space handle (not a subregion of 
anything) for the console com, which conflicts with puc.c later trying to 
allocate the larger handle that the console is a subregion (but not a 
bus_space_subregion) of.

Really, puc.c should just be turned on its head, identifying sub-devices first 
and only then worrying about bus_space mappings, but there's also some 
device-specific code in there that I can't test, so I was reluctant to do that.


-- 
Soren

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

[Martin Husemann]

On Tue, Jul 23, 2013 at 10:09:28AM +0200, Soren Jorvang wrote:
 The pre-autoconf console code creates a bus_space handle (not a
 subregion of anything) for the console com, which conflicts with puc.c
 later trying to allocate the larger handle that the console is a
 subregion (but not a bus_space_subregion) of.

An alternative hack would be to unmap the original com console mapping
and update it with a subregion from the puc BAR mapping.

Martin

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

[Soren Jorvang]

On 23/07/2013, at 11.04, Martin Husemann mar...@duskware.de wrote:
 On Tue, Jul 23, 2013 at 10:09:28AM +0200, Soren Jorvang wrote:
 The pre-autoconf console code creates a bus_space handle (not a
 subregion of anything) for the console com, which conflicts with puc.c
 later trying to allocate the larger handle that the console is a
 subregion (but not a bus_space_subregion) of.
 
 An alternative hack would be to unmap the original com console mapping
 and update it with a subregion from the puc BAR mapping.

That should probably be part of the proper solution that I mentioned since 
pci_mapreg_map() only allows mapping whole BAR regions, but for it to be not 
just another x86 hack, the interaction between puc/com_puc/com needs to be 
thought out to avoid either killing the console during the child handoff or 
putting more comcn knowledge into puc.c.


-- 
Soren

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

[Martin Husemann]

On Mon, Jul 22, 2013 at 02:40:03PM +, Martin Husemann wrote:
 Module Name:  src
 Committed By: martin
 Date: Mon Jul 22 14:40:03 UTC 2013
 
 Modified Files:
   src/sys/dev/pci: puc.c
 
 Log Message:
 Unbreak the build - soren, please review!
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.33 -r1.34 src/sys/dev/pci/puc.c
 
 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.
 

Unfortunately this is by far not enough - please backout or fix, and test
a few GENERIC kernel builds, please!

Martin

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

[SAITOH Masanobu]

Hello.

(2013/07/04 1:35), Erik Fair wrote:
 It would be very helpful to document in bge(4) just which chips support jumbo 
 frames (and how big).

 I agree with you. and I knew bge.4 is very obsolete. :-(

 Heck, it would be even more helpful if that was a capability that ifconfig(8) 
 could report because drivers declare it.

Yes, I think ifconfig should report the hard limit of MTU, too.

http://mail-index.netbsd.org/tech-kern/2013/07/03/msg015330.html

OpenBSD has SIOCGIFHARDMTU and ifconfig print the limit using with the ioctl.
It could be implemented using proplib, but I think ioctl is better than proplib
to be consistent with SIOC[GS]MTU.


   Erik f...@netbsd.org
 
 On Jul 2, 2013, at 22:49 , SAITOH Masanobu msai...@netbsd.org wrote:
 
 Module Name: src
 Committed By:msaitoh
 Date:Wed Jul  3 05:49:36 UTC 2013

 Modified Files:
  src/sys/dev/pci: if_bge.c

 Log Message:
 Add BGE_JUMBO_CAPABLE flag to some chips. Before this commit,
 the following chips support the jumbo frame function:

  5700
  5701
  5702
  5703
  5704

 With this commit, the following chip support it, too:

  5714
  5780
  5717
  5718
  5719 (exclude rev. A0)
  5720
  57765
  57766


 To generate a diff of this commit:
 cvs rdiff -u -r1.253 -r1.254 src/sys/dev/pci/if_bge.c

 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)

  * 英語 - 自動検出
  * 英語
  * 日本語

  * 英語
  * 日本語

 javascript:void(0); #

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

[Erik Fair]

It would be very helpful to document in bge(4) just which chips support jumbo 
frames (and how big). Heck, it would be even more helpful if that was a 
capability that ifconfig(8) could report because drivers declare it.

Erik f...@netbsd.org

On Jul 2, 2013, at 22:49 , SAITOH Masanobu msai...@netbsd.org wrote:

 Module Name:  src
 Committed By: msaitoh
 Date: Wed Jul  3 05:49:36 UTC 2013
 
 Modified Files:
   src/sys/dev/pci: if_bge.c
 
 Log Message:
 Add BGE_JUMBO_CAPABLE flag to some chips. Before this commit,
 the following chips support the jumbo frame function:
 
   5700
   5701
   5702
   5703
   5704
 
 With this commit, the following chip support it, too:
 
   5714
   5780
   5717
   5718
   5719 (exclude rev. A0)
   5720
   57765
   57766
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.253 -r1.254 src/sys/dev/pci/if_bge.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/pci

[Michael]

On Sun, 30 Dec 2012 09:45:05 +
Michael Lorenz macal...@netbsd.org wrote:

 Module Name:  src
 Committed By: macallan
 Date: Sun Dec 30 09:45:05 UTC 2012
 
 Modified Files:
   src/sys/dev/pci: radeonfb.c radeonfbreg.h
 
 Log Message:
 Add support for R3xx.
 Whi

EFATFINGERS, fixed in the repository.
This should also wire up CRTCs, DACs and outputs in a (more) sane way
( and keep track of the way things are set up ) - DVI and VGA outputs
should now work properly on all supported cards.
I tested this on an iBook G4, XVR-100 and a generic, PCish RV350.

have fun
Michael

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

[Izumi Tsutsui]

 Modified Files:
   src/sys/dev/pci: if_wm.c
 
 Log Message:
 Use PRIxPADDR to print a DMA address. This fix a problem that if_wm.c
 can't compile with WM_DEBUG on non-64bit platforms.

sizeof(paddr_t) != sizeof(bus_addr_t) at least on sparc,
so using PRIx64 with an explicit (uint64_t) cast is safer,
I think.
(someone might claim we should add PRIxBUSADDRfoo, though)

---
Izumi Tsutsui

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

[SAITOH Masanobu]

Hi, tsutsui.

(2012/10/09 21:20), Izumi Tsutsui wrote:
 Modified Files:
  src/sys/dev/pci: if_wm.c

 Log Message:
 Use PRIxPADDR to print a DMA address. This fix a problem that if_wm.c
 can't compile with WM_DEBUG on non-64bit platforms.
 
 sizeof(paddr_t) != sizeof(bus_addr_t) at least on sparc,
 so using PRIx64 with an explicit (uint64_t) cast is safer,
 I think.
 (someone might claim we should add PRIxBUSADDRfoo, though)
 
 ---
 Izumi Tsutsui
 

 Is the following patch OK?

 And, I've noticed that cz, sk and yds have the same problem...

 (someone might claim we should add PRIxBUSADDRfoo, though)

I prefer this.



Index: if_wm.c
===
RCS file: /cvsroot/src/sys/dev/pci/if_wm.c,v
retrieving revision 1.235
diff -u -r1.235 if_wm.c
--- if_wm.c 9 Oct 2012 10:25:44 -   1.235
+++ if_wm.c 9 Oct 2012 14:24:41 -
@@ -2693,10 +2693,10 @@
lasttx = nexttx;

DPRINTF(WM_DEBUG_TX,
-   (%s: TX: desc %d: low %# PRIxPADDR , 
+   (%s: TX: desc %d: low %# PRIx64 , 
 len %#04zx\n,
device_xname(sc-sc_dev), nexttx,
-   curaddr  0xUL, curlen));
+   (uint64_t)curaddr, curlen));
}
}

@@ -3154,9 +3154,9 @@
sc-sc_nq_txdescs[nexttx].nqtx_data.nqtxd_fields =
htole32(fields);
DPRINTF(WM_DEBUG_TX,
-   (%s: TX: adv data desc %d 0x% PRIxPADDR \n,
+   (%s: TX: adv data desc %d 0x% PRIx64 \n,
device_xname(sc-sc_dev), nexttx,
-   dmamap-dm_segs[0].ds_addr));
+   (uint64_t)dmamap-dm_segs[0].ds_addr));
DPRINTF(WM_DEBUG_TX,
(\t 0x%08x%08x\n, fields,
(uint32_t)dmamap-dm_segs[0].ds_len | cmdlen));
@@ -3180,10 +3180,10 @@
lasttx = nexttx;

DPRINTF(WM_DEBUG_TX,
-   (%s: TX: desc %d: %# PRIxPADDR , 
+   (%s: TX: desc %d: %# PRIx64 , 
 len %#04zx\n,
device_xname(sc-sc_dev), nexttx,
-   dmamap-dm_segs[seg].ds_addr,
+   (uint64_t)dmamap-dm_segs[seg].ds_addr,
dmamap-dm_segs[seg].ds_len));
}



-- 
---
SAITOH Masanobu (msai...@execsw.org
 msai...@netbsd.org)

re: CVS commit: src/sys/dev/pci

[matthew green]

 On Sat, Sep 01, 2012 at 02:08:28AM +, Matt Thomas wrote:
  Module Name:src
  Committed By:   matt
  Date:   Sat Sep  1 02:08:28 UTC 2012
  
  Modified Files:
  src/sys/dev/pci: if_wm.c
  
  Log Message:
  Shut up gcc about some uninitialized variables.
 
 
 Hum, gcc is wrong here, cmdlen and fields are only used if do_csum == true,
 and do_csum cannot be changed between setting it to false and testing it.
 
 Is it dependant on compiler flags ?

FWIW, usually these sorts of problems only appear at -O3.  it's the
main reason i stopped using -O3 in general.


.mrg.

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

[Manuel Bouyer]

On Sat, Sep 01, 2012 at 02:08:28AM +, Matt Thomas wrote:
 Module Name:  src
 Committed By: matt
 Date: Sat Sep  1 02:08:28 UTC 2012
 
 Modified Files:
   src/sys/dev/pci: if_wm.c
 
 Log Message:
 Shut up gcc about some uninitialized variables.


Hum, gcc is wrong here, cmdlen and fields are only used if do_csum == true,
and do_csum cannot be changed between setting it to false and testing it.

Is it dependant on compiler flags ?

-- 
Manuel Bouyer bou...@antioche.eu.org
 NetBSD: 26 ans d'experience feront toujours la difference
--

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

[Masanobu SAITOH]

Hi.

(2012/08/30 7:06), Paul Goyette wrote:

On Wed, 29 Aug 2012, Paul Goyette wrote:


On Wed, 29 Aug 2012, Manuel Bouyer wrote:


Module Name:src
Committed By:bouyer
Date:Wed Aug 29 20:39:24 UTC 2012

Modified Files:
src/sys/dev/pci: if_wm.c if_wmreg.h

Log Message:
Make vlan and all ip/ip6 checksum offload work for the I350.

On newer devices, when using the legacy TX descriptors, vlan-related
flags
that were set on the last descriptor of a packet have to be set on the
first one.
For tso/checksum offloads, a new advanced descriptor format has to be
used.


snip


Tested on a I350, and a i80003 (which use the old format), both with and
without vlans, with and without checksum offloads.


Is there an easy way to identify which wm I have?  Does the following
device need this commit in order to use offload?  Or will this one
work with an earlier rev of the driver?

# dmesg | grep wm
wm0 at pci2 dev 0 function 0: Intel i82574L (rev. 0x00)
wm0: interrupting at ioapic1 pin 24
wm0: PCI-Express bus
wm0: 256 word (8 address bits) SPI EEPROM
wm0: Ethernet address bc:ae:c5:30:d9:8a
ukphy0 at wm0 phy 1: OUI 0x000ac2, model 0x000b, rev. 1


BTW, reason I am asking is that I had previously (about a year or so
ago) needed to disable the offload functions on the 82574 because it
wasn't working.  This commit seems to imply that it fixes only the I350
so I was wondering if some other change that I had missed might have
already fixed the 82574.



-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| Customer Service | 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  |
-


82575, 82576, 82580(ER) and I350 use the new queue mechanism.


--
---
SAITOH Masanobu (msai...@execsw.org
 msai...@netbsd.org)

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

[Paul Goyette]

On Thu, 30 Aug 2012, Masanobu SAITOH wrote:


BTW, reason I am asking is that I had previously (about a year or so
ago) needed to disable the offload functions on the 82574 because it
wasn't working.  This commit seems to imply that it fixes only the I350
so I was wondering if some other change that I had missed might have
already fixed the 82574.


82575, 82576, 82580(ER) and I350 use the new queue mechanism.


So the 82574 should already be working correctly?  Thanks, I will 
verify.



-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| Customer Service | 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/pci

[Manuel Bouyer]

On Thu, Aug 30, 2012 at 04:54:19AM -0700, Paul Goyette wrote:
 On Thu, 30 Aug 2012, Masanobu SAITOH wrote:
 
 BTW, reason I am asking is that I had previously (about a year or so
 ago) needed to disable the offload functions on the 82574 because it
 wasn't working.  This commit seems to imply that it fixes only the I350
 so I was wondering if some other change that I had missed might have
 already fixed the 82574.
 
 82575, 82576, 82580(ER) and I350 use the new queue mechanism.
 
 So the 82574 should already be working correctly?  Thanks, I will
 verify.

It should, yes. But now that programming docs are publically available,
we may be able to do something if it doesn't work.

-- 
Manuel Bouyer bou...@antioche.eu.org
 NetBSD: 26 ans d'experience feront toujours la difference
--

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

[Paul Goyette]

On Wed, 29 Aug 2012, Paul Goyette wrote:


On Wed, 29 Aug 2012, Manuel Bouyer wrote:


Module Name:src
Committed By:   bouyer
Date:   Wed Aug 29 20:39:24 UTC 2012

Modified Files:
src/sys/dev/pci: if_wm.c if_wmreg.h

Log Message:
Make vlan and all ip/ip6 checksum offload work for the I350.

On newer devices, when using the legacy TX descriptors, vlan-related flags
that were set on the last descriptor of a packet have to be set on the
first one.
For tso/checksum offloads, a new advanced descriptor format has to be
used.


snip


Tested on a I350, and a i80003 (which use the old format), both with and
without vlans, with and without checksum offloads.


Is there an easy way to identify which wm I have?  Does the following device 
need this commit in order to use offload?  Or will this one work with an 
earlier rev of the driver?


# dmesg | grep wm
wm0 at pci2 dev 0 function 0: Intel i82574L (rev. 0x00)
wm0: interrupting at ioapic1 pin 24
wm0: PCI-Express bus
wm0: 256 word (8 address bits) SPI EEPROM
wm0: Ethernet address bc:ae:c5:30:d9:8a
ukphy0 at wm0 phy 1: OUI 0x000ac2, model 0x000b, rev. 1


BTW, reason I am asking is that I had previously (about a year or so 
ago) needed to disable the offload functions on the 82574 because it 
wasn't working.  This commit seems to imply that it fixes only the I350 
so I was wondering if some other change that I had missed might have 
already fixed the 82574.




-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| Customer Service | 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/pci

[Paul Goyette]

Ooops - I will fix the log message shortly.

On Tue, 14 Feb 2012, Paul Goyette wrote:


Module Name:src
Committed By:   pgoyette
Date:   Tue Feb 14 15:08:07 UTC 2012

Modified Files:
src/sys/dev/pci: amdpm_smbus.c amdpmvar.h ichsmb.c nfsmb.c piixpm.c

Log Message:
/home/paul/COMMIT


To generate a diff of this commit:
cvs rdiff -u -r1.17 -r1.18 src/sys/dev/pci/amdpm_smbus.c
cvs rdiff -u -r1.7 -r1.8 src/sys/dev/pci/amdpmvar.h
cvs rdiff -u -r1.26 -r1.27 src/sys/dev/pci/ichsmb.c
cvs rdiff -u -r1.22 -r1.23 src/sys/dev/pci/nfsmb.c
cvs rdiff -u -r1.39 -r1.40 src/sys/dev/pci/piixpm.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


!DSPAM:4f3a78f52061782010291!





-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| Customer Service | 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/pci

[Marc Balmer]

Am 14.02.12 16:08, schrieb Paul Goyette:
 Module Name:  src
 Committed By: pgoyette
 Date: Tue Feb 14 15:08:07 UTC 2012
 
 Modified Files:
   src/sys/dev/pci: amdpm_smbus.c amdpmvar.h ichsmb.c nfsmb.c piixpm.c
 
 Log Message:
 /home/paul/COMMIT

could you please be a bit more verbose?

 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.17 -r1.18 src/sys/dev/pci/amdpm_smbus.c
 cvs rdiff -u -r1.7 -r1.8 src/sys/dev/pci/amdpmvar.h
 cvs rdiff -u -r1.26 -r1.27 src/sys/dev/pci/ichsmb.c
 cvs rdiff -u -r1.22 -r1.23 src/sys/dev/pci/nfsmb.c
 cvs rdiff -u -r1.39 -r1.40 src/sys/dev/pci/piixpm.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/pci/amdpm_smbus.c
 diff -u src/sys/dev/pci/amdpm_smbus.c:1.17 src/sys/dev/pci/amdpm_smbus.c:1.18
 --- src/sys/dev/pci/amdpm_smbus.c:1.17Sat Nov 19 02:39:14 2011
 +++ src/sys/dev/pci/amdpm_smbus.c Tue Feb 14 15:08:07 2012
 @@ -1,4 +1,4 @@
 -/*   $NetBSD: amdpm_smbus.c,v 1.17 2011/11/19 02:39:14 christos Exp $ */
 +/*   $NetBSD: amdpm_smbus.c,v 1.18 2012/02/14 15:08:07 pgoyette Exp $ */
  
  /*
   * Copyright (c) 2005 Anil Gopinath (anil_pub...@yahoo.com)
 @@ -32,14 +32,14 @@
   * AMD-8111 HyperTransport I/O Hub
   */
  #include sys/cdefs.h
 -__KERNEL_RCSID(0, $NetBSD: amdpm_smbus.c,v 1.17 2011/11/19 02:39:14 
 christos Exp $);
 +__KERNEL_RCSID(0, $NetBSD: amdpm_smbus.c,v 1.18 2012/02/14 15:08:07 
 pgoyette Exp $);
  
  #include sys/param.h
  #include sys/systm.h
  #include sys/kernel.h
  #include sys/device.h
  #include sys/rnd.h
 -#include sys/rwlock.h
 +#include sys/mutex.h
  
  #include dev/pci/pcireg.h
  #include dev/pci/pcivar.h
 @@ -83,7 +83,7 @@ amdpm_smbus_attach(struct amdpm_softc *s
   sc-sc_i2c.ic_write_byte = NULL;
   sc-sc_i2c.ic_exec = amdpm_smbus_exec;
  
 - rw_init(sc-sc_rwlock);
 + mutex_init(sc-sc_mutex, MUTEX_DEFAULT, IPL_NONE);
  
   iba.iba_tag = sc-sc_i2c;
   (void)config_found_ia(sc-sc_dev, i2cbus, iba, iicbus_print);
 @@ -94,7 +94,7 @@ amdpm_smbus_acquire_bus(void *cookie, in
  {
   struct amdpm_softc *sc = cookie;
  
 - rw_enter(sc-sc_rwlock, RW_WRITER);
 + mutex_enter(sc-sc_mutex);
   return 0;
  }
  
 @@ -103,7 +103,7 @@ amdpm_smbus_release_bus(void *cookie, in
  {
   struct amdpm_softc *sc = cookie;
  
 - rw_exit(sc-sc_rwlock);
 + mutex_exit(sc-sc_mutex);
  }
  
  static int
 
 Index: src/sys/dev/pci/amdpmvar.h
 diff -u src/sys/dev/pci/amdpmvar.h:1.7 src/sys/dev/pci/amdpmvar.h:1.8
 --- src/sys/dev/pci/amdpmvar.h:1.7Sat Nov 19 22:51:23 2011
 +++ src/sys/dev/pci/amdpmvar.hTue Feb 14 15:08:07 2012
 @@ -1,4 +1,4 @@
 -/*   $NetBSD: amdpmvar.h,v 1.7 2011/11/19 22:51:23 tls Exp $ */
 +/*   $NetBSD: amdpmvar.h,v 1.8 2012/02/14 15:08:07 pgoyette Exp $*/
  
  /*-
   * Copyright (c) 2002 The NetBSD Foundation, Inc.
 @@ -32,7 +32,7 @@
  #ifndef _DEV_PCI_AMDPMVAR_H_
  #define _DEV_PCI_AMDPMVAR_H_
  
 -#include sys/rwlock.h
 +#include sys/mutex.h
  
  struct amdpm_softc {
   struct device sc_dev;
 @@ -47,7 +47,7 @@ struct amdpm_softc {
  
   i2c_addr_t sc_smbus_slaveaddr;  /* address of smbus slave */
   struct i2c_controller sc_i2c;   /* i2c controller info */
 - krwlock_t sc_rwlock;
 + kmutex_t sc_mutex;
  
   void *sc_ih;
  
 
 Index: src/sys/dev/pci/ichsmb.c
 diff -u src/sys/dev/pci/ichsmb.c:1.26 src/sys/dev/pci/ichsmb.c:1.27
 --- src/sys/dev/pci/ichsmb.c:1.26 Mon Jan 30 19:41:19 2012
 +++ src/sys/dev/pci/ichsmb.c  Tue Feb 14 15:08:07 2012
 @@ -1,4 +1,4 @@
 -/*   $NetBSD: ichsmb.c,v 1.26 2012/01/30 19:41:19 drochner Exp $ */
 +/*   $NetBSD: ichsmb.c,v 1.27 2012/02/14 15:08:07 pgoyette Exp $ */
  /*   $OpenBSD: ichiic.c,v 1.18 2007/05/03 09:36:26 dlg Exp $ */
  
  /*
 @@ -22,13 +22,13 @@
   */
  
  #include sys/cdefs.h
 -__KERNEL_RCSID(0, $NetBSD: ichsmb.c,v 1.26 2012/01/30 19:41:19 drochner Exp 
 $);
 +__KERNEL_RCSID(0, $NetBSD: ichsmb.c,v 1.27 2012/02/14 15:08:07 pgoyette Exp 
 $);
  
  #include sys/param.h
  #include sys/device.h
  #include sys/errno.h
  #include sys/kernel.h
 -#include sys/rwlock.h
 +#include sys/mutex.h
  #include sys/proc.h
  
  #include sys/bus.h
 @@ -59,7 +59,7 @@ struct ichsmb_softc {
   int sc_poll;
  
   struct i2c_controller   sc_i2c_tag;
 - krwlock_t   sc_i2c_rwlock;
 + kmutex_tsc_i2c_mutex;
   struct {
   i2c_op_t op;
   void *   buf;
 @@ -166,7 +166,7 @@ ichsmb_attach(device_t parent, device_t 
   }
  
   /* Attach I2C bus */
 - rw_init(sc-sc_i2c_rwlock);
 + mutex_init(sc-sc_i2c_mutex, MUTEX_DEFAULT, IPL_NONE);
   sc-sc_i2c_tag.ic_cookie = sc;
   sc-sc_i2c_tag.ic_acquire_bus = ichsmb_i2c_acquire_bus;
   sc-sc_i2c_tag.ic_release_bus = ichsmb_i2c_release_bus;
 @@ -186,10 +186,10 @@ ichsmb_i2c_acquire_bus(void *cookie, int
  {
   struct 

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

[Paul Goyette]

Log message has been updated in cvs:

Replace the xxx_acquire()/xxx_release() rwlocks with mutexes.  There are
only RW_WRITERs for these, and no RW_READERs, so no need to incur the
extra overhead of allowing for both.  As discussed on tech-kern.

For piixpm and ichsmb, the acquire/release protocol needs to be used, 
even if the request is I2C_F_POLL'd (or if the device supports only

polled mode).  Otherwise multiple requests can be running at the same
time, and they stomp on each other and create anomolous results.

Part 2 addresses my PR kern/45889

3 ACKs from releng



On Tue, 14 Feb 2012, Marc Balmer wrote:


Am 14.02.12 16:08, schrieb Paul Goyette:

Module Name:src
Committed By:   pgoyette
Date:   Tue Feb 14 15:08:07 UTC 2012

Modified Files:
src/sys/dev/pci: amdpm_smbus.c amdpmvar.h ichsmb.c nfsmb.c piixpm.c

Log Message:
/home/paul/COMMIT


could you please be a bit more verbose?




To generate a diff of this commit:
cvs rdiff -u -r1.17 -r1.18 src/sys/dev/pci/amdpm_smbus.c
cvs rdiff -u -r1.7 -r1.8 src/sys/dev/pci/amdpmvar.h
cvs rdiff -u -r1.26 -r1.27 src/sys/dev/pci/ichsmb.c
cvs rdiff -u -r1.22 -r1.23 src/sys/dev/pci/nfsmb.c
cvs rdiff -u -r1.39 -r1.40 src/sys/dev/pci/piixpm.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/pci/amdpm_smbus.c
diff -u src/sys/dev/pci/amdpm_smbus.c:1.17 src/sys/dev/pci/amdpm_smbus.c:1.18
--- src/sys/dev/pci/amdpm_smbus.c:1.17  Sat Nov 19 02:39:14 2011
+++ src/sys/dev/pci/amdpm_smbus.c   Tue Feb 14 15:08:07 2012
@@ -1,4 +1,4 @@
-/* $NetBSD: amdpm_smbus.c,v 1.17 2011/11/19 02:39:14 christos Exp $ */
+/* $NetBSD: amdpm_smbus.c,v 1.18 2012/02/14 15:08:07 pgoyette Exp $ */

 /*
  * Copyright (c) 2005 Anil Gopinath (anil_pub...@yahoo.com)
@@ -32,14 +32,14 @@
  * AMD-8111 HyperTransport I/O Hub
  */
 #include sys/cdefs.h
-__KERNEL_RCSID(0, $NetBSD: amdpm_smbus.c,v 1.17 2011/11/19 02:39:14 christos Exp 
$);
+__KERNEL_RCSID(0, $NetBSD: amdpm_smbus.c,v 1.18 2012/02/14 15:08:07 pgoyette Exp 
$);

 #include sys/param.h
 #include sys/systm.h
 #include sys/kernel.h
 #include sys/device.h
 #include sys/rnd.h
-#include sys/rwlock.h
+#include sys/mutex.h

 #include dev/pci/pcireg.h
 #include dev/pci/pcivar.h
@@ -83,7 +83,7 @@ amdpm_smbus_attach(struct amdpm_softc *s
sc-sc_i2c.ic_write_byte = NULL;
sc-sc_i2c.ic_exec = amdpm_smbus_exec;

-   rw_init(sc-sc_rwlock);
+   mutex_init(sc-sc_mutex, MUTEX_DEFAULT, IPL_NONE);

iba.iba_tag = sc-sc_i2c;
(void)config_found_ia(sc-sc_dev, i2cbus, iba, iicbus_print);
@@ -94,7 +94,7 @@ amdpm_smbus_acquire_bus(void *cookie, in
 {
struct amdpm_softc *sc = cookie;

-   rw_enter(sc-sc_rwlock, RW_WRITER);
+   mutex_enter(sc-sc_mutex);
return 0;
 }

@@ -103,7 +103,7 @@ amdpm_smbus_release_bus(void *cookie, in
 {
struct amdpm_softc *sc = cookie;

-   rw_exit(sc-sc_rwlock);
+   mutex_exit(sc-sc_mutex);
 }

 static int

Index: src/sys/dev/pci/amdpmvar.h
diff -u src/sys/dev/pci/amdpmvar.h:1.7 src/sys/dev/pci/amdpmvar.h:1.8
--- src/sys/dev/pci/amdpmvar.h:1.7  Sat Nov 19 22:51:23 2011
+++ src/sys/dev/pci/amdpmvar.h  Tue Feb 14 15:08:07 2012
@@ -1,4 +1,4 @@
-/* $NetBSD: amdpmvar.h,v 1.7 2011/11/19 22:51:23 tls Exp $ */
+/* $NetBSD: amdpmvar.h,v 1.8 2012/02/14 15:08:07 pgoyette Exp $*/

 /*-
  * Copyright (c) 2002 The NetBSD Foundation, Inc.
@@ -32,7 +32,7 @@
 #ifndef _DEV_PCI_AMDPMVAR_H_
 #define _DEV_PCI_AMDPMVAR_H_

-#include sys/rwlock.h
+#include sys/mutex.h

 struct amdpm_softc {
struct device sc_dev;
@@ -47,7 +47,7 @@ struct amdpm_softc {

i2c_addr_t sc_smbus_slaveaddr;  /* address of smbus slave */
struct i2c_controller sc_i2c;   /* i2c controller info */
-   krwlock_t sc_rwlock;
+   kmutex_t sc_mutex;

void *sc_ih;


Index: src/sys/dev/pci/ichsmb.c
diff -u src/sys/dev/pci/ichsmb.c:1.26 src/sys/dev/pci/ichsmb.c:1.27
--- src/sys/dev/pci/ichsmb.c:1.26   Mon Jan 30 19:41:19 2012
+++ src/sys/dev/pci/ichsmb.cTue Feb 14 15:08:07 2012
@@ -1,4 +1,4 @@
-/* $NetBSD: ichsmb.c,v 1.26 2012/01/30 19:41:19 drochner Exp $ */
+/* $NetBSD: ichsmb.c,v 1.27 2012/02/14 15:08:07 pgoyette Exp $ */
 /* $OpenBSD: ichiic.c,v 1.18 2007/05/03 09:36:26 dlg Exp $ */

 /*
@@ -22,13 +22,13 @@
  */

 #include sys/cdefs.h
-__KERNEL_RCSID(0, $NetBSD: ichsmb.c,v 1.26 2012/01/30 19:41:19 drochner Exp 
$);
+__KERNEL_RCSID(0, $NetBSD: ichsmb.c,v 1.27 2012/02/14 15:08:07 pgoyette Exp 
$);

 #include sys/param.h
 #include sys/device.h
 #include sys/errno.h
 #include sys/kernel.h
-#include sys/rwlock.h
+#include sys/mutex.h
 #include sys/proc.h

 #include sys/bus.h
@@ -59,7 +59,7 @@ struct ichsmb_softc {
int sc_poll;

struct i2c_controller   sc_i2c_tag;
-   krwlock_t   sc_i2c_rwlock;
+   kmutex_tsc_i2c_mutex;

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

[Jonathan A. Kollasch]

On Wed, Jan 04, 2012 at 07:56:36AM +, Michael Lorenz wrote:
 Module Name:  src
 Committed By: macallan
 Date: Wed Jan  4 07:56:35 UTC 2012
 
 Modified Files:
   src/sys/dev/pci: r128fb.c
 
 Log Message:
 split putchar into separate methods for bitmap and alpha fonts, use
 FONT_IS_ALPHA()
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.23 -r1.24 src/sys/dev/pci/r128fb.c
 

This change seems to be missing changes to r128fbreg.h,
the likes of R128_CRTC_COLOR_8BIT aren't defined.

Jonathan Kollasch

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

[Martin Husemann]

On Thu, Dec 29, 2011 at 08:14:40PM +, Michael Lorenz wrote:
 Module Name:  src
 Committed By: macallan
 Date: Thu Dec 29 20:14:40 UTC 2011
 
 Modified Files:
   src/sys/dev/pci: radeonfb.c
 
 Log Message:
 rework putchar():
 - get rid of engine stalls when using the blitter to draw characters
 - add a wrapper for non-accelerated putchar() so we only wait for the
   engine when we actually want to scribble into video memory
 - rework accelerated putchar(), should work on R3xx now but needs testing

Did you forget to commit some associated header changes? This does not
compile for me...


Martin

re: CVS commit: src/sys/dev/pci

[Eduardo Horvath]

On Mon, 12 Dec 2011, matthew green wrote:

  On Mon, Dec 12, 2011 at 03:54:57PM +1100, matthew green wrote:
   
On Mon, Dec 12, 2011 at 02:44:15AM +, Jonathan A. Kollasch wrote:
 Module Name:  src
 Committed By: jakllsch
 Date: Mon Dec 12 02:44:15 UTC 2011
 
 Modified Files:
   src/sys/dev/pci: if_sip.c
 
 Log Message:
 Using BUS_DMA_NOCACHE for bus_dmamem_map() causes issues on (at 
 least) sparc64.

What kind of issues?
   
   this is maping normal memory uncached.  attempting to access this
   mapping causes data faults.
  
  Jonathan, please add some explanation to the commit message.
  
  Isn't this just covering a bug in the sparc64 bus_dma(9) implementation?
 
 that's what i have been wondering since writing the above.
 
 eeh, should we turn off CP/CV and E bits for ram BUS_DMA_NOCACHE
 addresses, instead of device memory?

NEVER turn on the CP or E bits for DRAM.  (I think it's the CP bit.  Need 
to review the manual.)

DRAM always goes through the E$ which does all the ECC checking.

I/O addresses do not go through the E$.

You have the choice of turning off the CV bit for DRAM, which means the 
data goes into the E$, but not the L1 D$ or I$.  

If you turn off the CP bit or turn on the E bit for DRAM you get data 
exceptions.

If you turn on the CP bit for I/O addresses you get data faults.

So don't do that.

(BTW, the synchonization domain for UltraSPARC class machines is the E$.  
All DMA to DRAM goes through the E$.)

Eduardo

re: CVS commit: src/sys/dev/pci

[Eduardo Horvath]

On Mon, 12 Dec 2011, matthew green wrote:

  On Mon, Dec 12, 2011 at 03:54:57PM +1100, matthew green wrote:
   
On Mon, Dec 12, 2011 at 02:44:15AM +, Jonathan A. Kollasch wrote:
 Module Name:  src
 Committed By: jakllsch
 Date: Mon Dec 12 02:44:15 UTC 2011
 
 Modified Files:
   src/sys/dev/pci: if_sip.c
 
 Log Message:
 Using BUS_DMA_NOCACHE for bus_dmamem_map() causes issues on (at 
 least) sparc64.

What kind of issues?
   
   this is maping normal memory uncached.  attempting to access this
   mapping causes data faults.
  
  Jonathan, please add some explanation to the commit message.
  
  Isn't this just covering a bug in the sparc64 bus_dma(9) implementation?
 
 that's what i have been wondering since writing the above.
 
 eeh, should we turn off CP/CV and E bits for ram BUS_DMA_NOCACHE
 addresses, instead of device memory?

Hm.  Let me look at that commit  (cvsweb is slo today...)  

Let's see... BUS_DMA_NOCACHE went into the man page in 2003 based on an 
x86 machine specific bit...  It was added to the sparc64 bus_dma.h at the 
same time...  Replacing an existing MD BUS_DMA_NOCACHE that was added to 
in 2000...

I can't figure out what the happened in x86 land... it seems to have moved 
across half a dozen files.

So...  The original BUS_DMA_NOCACHE for sparc64 had different semantics.  
It was supposed to be used for memory that is attached to the system 
through an I/O bus, such as the framebuffer on a PCI I/O card.

The options are:

1)  Remove the inherently MD BUS_DMA_NOCACHE flag from the man page so 
it's not used by people who don't understand why it's needed.

2)  Properly define the semantics of BUS_DMA_NOCACHE so people know when 
it needs to be used.

3)  Redefine that bit for sparc64.

Eduardo

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

[David Young]

On Mon, Dec 12, 2011 at 02:44:15AM +, Jonathan A. Kollasch wrote:
 Module Name:  src
 Committed By: jakllsch
 Date: Mon Dec 12 02:44:15 UTC 2011
 
 Modified Files:
   src/sys/dev/pci: if_sip.c
 
 Log Message:
 Using BUS_DMA_NOCACHE for bus_dmamem_map() causes issues on (at least) 
 sparc64.

What kind of issues?

Dave

-- 
David Young
dyo...@pobox.comUrbana, IL(217) 721-9981

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

[David Young]

On Mon, Dec 12, 2011 at 03:54:57PM +1100, matthew green wrote:
 
  On Mon, Dec 12, 2011 at 02:44:15AM +, Jonathan A. Kollasch wrote:
   Module Name:  src
   Committed By: jakllsch
   Date: Mon Dec 12 02:44:15 UTC 2011
   
   Modified Files:
 src/sys/dev/pci: if_sip.c
   
   Log Message:
   Using BUS_DMA_NOCACHE for bus_dmamem_map() causes issues on (at least) 
   sparc64.
  
  What kind of issues?
 
 this is maping normal memory uncached.  attempting to access this
 mapping causes data faults.

Jonathan, please add some explanation to the commit message.

Isn't this just covering a bug in the sparc64 bus_dma(9) implementation?

Dave

-- 
David Young
dyo...@pobox.comUrbana, IL(217) 721-9981

re: CVS commit: src/sys/dev/pci

[matthew green]

 On Mon, Dec 12, 2011 at 03:54:57PM +1100, matthew green wrote:
  
   On Mon, Dec 12, 2011 at 02:44:15AM +, Jonathan A. Kollasch wrote:
Module Name:src
Committed By:   jakllsch
Date:   Mon Dec 12 02:44:15 UTC 2011

Modified Files:
src/sys/dev/pci: if_sip.c

Log Message:
Using BUS_DMA_NOCACHE for bus_dmamem_map() causes issues on (at least) 
sparc64.
   
   What kind of issues?
  
  this is maping normal memory uncached.  attempting to access this
  mapping causes data faults.
 
 Jonathan, please add some explanation to the commit message.
 
 Isn't this just covering a bug in the sparc64 bus_dma(9) implementation?

that's what i have been wondering since writing the above.

eeh, should we turn off CP/CV and E bits for ram BUS_DMA_NOCACHE
addresses, instead of device memory?


.mrg.

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

[Takeshi Nakayama]

 Marc Balmer mbal...@netbsd.org wrote

 Module Name:  src
 Committed By: mbalmer
 Date: Sat Oct  8 10:21:16 UTC 2011
 
 Modified Files:
   src/sys/dev/pci: if_iwn.c
 
 Log Message:
 Make this compile again: Use the device_xname() macro to get the devic name.

It should use aprint_error_dev() just like other messages.

-- Takeshi Nakayama

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

[Marc Balmer]

Am 08.10.11 12:44, schrieb Takeshi Nakayama:
 Marc Balmer mbal...@netbsd.org wrote
 
 Module Name: src
 Committed By:mbalmer
 Date:Sat Oct  8 10:21:16 UTC 2011

 Modified Files:
  src/sys/dev/pci: if_iwn.c

 Log Message:
 Make this compile again: Use the device_xname() macro to get the devic name.
 
 It should use aprint_error_dev() just like other messages.

I only fixed the obviously wrong argument to make it compile again.  I
leave functional changes to those that use this code.

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

[J. Hannken-Illjes]

On Oct 8, 2011, at 12:42 AM, Roland Dowdeswell wrote:

 Module Name:  src
 Committed By: elric
 Date: Fri Oct  7 22:42:19 UTC 2011
 
 Modified Files:
   src/sys/dev/pci: if_iwn.c

This breaks building kernels:

sys/dev/pci/if_iwn.c: In function 'iwn5000_runtime_calib':
sys/dev/pci/if_iwn.c:4108:25: error: 'IWN5000_CALIB_DC' undeclared (first use 
in this function)
sys/dev/pci/if_iwn.c:4108:25: note: each undeclared identifier is reported only 
once for each function it appears in
sys/dev/pci/if_iwn.c: In function 'iwn_config':
sys/dev/pci/if_iwn.c:4130:18: error: request for member 'dv_xname' in something 
not a structure or union


--
Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)

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

[SAITOH Masanobu]

I'm sorry, I've committed the changes for the MII PHY part.
Please cvs update and retry.

Thanks.

(2011/05/20 14:28), Takahiro Kambe wrote:
 Hi,
 
 
 compile  GENERIC/if_wm.o
 /usr/src/sys/dev/pci/if_wm.c: In function 'wm_set_mdio_slow_mode_hv':
 /usr/src/sys/dev/pci/if_wm.c:7436: error: 'HV_KMRN_MODE_CTRL' undeclared 
 (first use in this function)
 /usr/src/sys/dev/pci/if_wm.c:7436: error: (Each undeclared identifier is 
 reported only once
 /usr/src/sys/dev/pci/if_wm.c:7436: error: for each function it appears in.)
 /usr/src/sys/dev/pci/if_wm.c:7438: error: 'HV_KMRN_MDIO_SLOW' undeclared 
 (first use in this function)
 
 Is it my specific problem?
 


-- 
---
SAITOH Masanobu (msai...@execsw.org
 msai...@netbsd.org)

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

[Takahiro Kambe]

Thanks for your quick fix.

Best regards.

-- 
Takahiro Kambe t...@back-street.net

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

[Takahiro Kambe]

Hi,


compile  GENERIC/if_wm.o
/usr/src/sys/dev/pci/if_wm.c: In function 'wm_set_mdio_slow_mode_hv':
/usr/src/sys/dev/pci/if_wm.c:7436: error: 'HV_KMRN_MODE_CTRL' undeclared (first 
use in this function)
/usr/src/sys/dev/pci/if_wm.c:7436: error: (Each undeclared identifier is 
reported only once
/usr/src/sys/dev/pci/if_wm.c:7436: error: for each function it appears in.)
/usr/src/sys/dev/pci/if_wm.c:7438: error: 'HV_KMRN_MDIO_SLOW' undeclared (first 
use in this function)

Is it my specific problem?

-- 
Takahiro Kambe t...@back-street.net