Re: Say no to LARVAL

[Martin Pieuchot]

On 10/05/18(Thu) 22:49, Philip Guenther wrote:
> On Wed, May 9, 2018 at 3:31 AM, Martin Pieuchot  wrote:
> 
> > On 08/05/18(Tue) 14:12, Philip Guenther wrote:
> > > On Tue, 8 May 2018, Martin Pieuchot wrote:
> > > > The way our kernel allocates and populates new 'struct file *' is
> > > > currently a complete mess.  One of the problems is that it puts
> > > > incomplete descriptors on the global `filehead' list and on the
> > > > open file array `fd_ofiles'.  But to make sure that other threads
> > > > won't use such descriptors before they are ready, they are first
> > > > marked as LARVAL then unmarked via the FILE_SET_MATURE() macro.
> > >
> > > The reason we do that is to meet two goals:
> > > 1) we want to allocate the fd (and fail if we can't) before
> > >possibly blocking while completing the creation of the struct file.
> > >For example, in doaccept(), this thread being blocked waiting for a
> > new
> > >connection must not block other threads from making fd changes.
> >
> > But the way I understand it, allocating the fd isn't changed by my diff.
> > fdalloc(), called by falloc(), still allocates it and mark the
> > corresponding
> > bits with fd_used().
> >
> 
> Those bits don't block dup2() *targeting* the fd, or close of the fd.
> 
> 
> 
> > > 2) close(half_created) and dup2(old, half_created) must be safe: it must
> > >not leak the half_created file in the kernel, and on completion the
> > >half_created fd must either be closed or dup'ed to; the half-created
> > >file must not suddenly show up there later
> >
> > I don't understand, are you talking about close(2) and dup2(2)?  How
> > syscalls are related to half_created files?  How is that related to
> > my explanation below?
> >
> 
> thread 1: socket() -- > returns 3
> thread 1: bind(3, someaddr); listen(3);
> thread 1: accept(3) --> expected to return 4 (it's the first free fd), but
> blocks waiting for connection
> thread 2:   socket() --> returns 5 (if not, then explain that and
> ignore the rest of this sequences)
> thread 2:   dup2(0, 4) --> better not block!  after this fd 4 must be a
> dup of 0!
> thread 2:   connect(5, someaddr) --> unblocks thread 1
> 
> So what happens at the end of this?  Did accept() return 4 as expected and
> if so is fd 4 a duplicate of fd 0 or is it the accepted socket?  I argue
> that it must be a dup of 0 because dup2 succeeded, and the accept that
> completed later cannot close that fd.  That's how the current kernel
> behaves.
> 
> I haven't tested it, but my reading of the diff is that is fd 4 will be the
> accepted fd and the dup'ed copy of fd 0 will be lost, leaving it's
> reference count too high by one.

Here's an updated diff on top of the two bug fixes I sent today.  It
makes use of a new function, fd_inuse() to detect if a file is in the
LARVAL state and return an error in the case of dup2().

I'd appreciate review as this is a blocking issue for unlocking all
file related syscalls.

diff --git sys/kern/exec_script.c sys/kern/exec_script.c
index 0bfaea0aee0..1c7aae6f69b 100644
--- sys/kern/exec_script.c
+++ sys/kern/exec_script.c
@@ -170,17 +170,20 @@ check_shell:
 #endif
 
fdplock(p->p_fd);
-   error = falloc(p, 0, , >ep_fd);
-   fdpunlock(p->p_fd);
-   if (error)
+   error = falloc(p, , >ep_fd);
+   if (error) {
+   fdpunlock(p->p_fd);
goto fail;
+   }
 
epp->ep_flags |= EXEC_HASFD;
fp->f_type = DTYPE_VNODE;
fp->f_ops = 
fp->f_data = (caddr_t) scriptvp;
fp->f_flag = FREAD;
-   FILE_SET_MATURE(fp, p);
+   fdinsert(p->p_fd, epp->ep_fd, 0, fp);
+   fdpunlock(p->p_fd);
+   FRELE(fp, p);
}
 
/* set up the parameters for the recursive check_exec() call */
diff --git sys/kern/kern_descrip.c sys/kern/kern_descrip.c
index 1734fb5cff0..694ebf1a128 100644
--- sys/kern/kern_descrip.c
+++ sys/kern/kern_descrip.c
@@ -144,6 +144,23 @@ find_last_set(struct filedesc *fd, int last)
return i;
 }
 
+static __inline int
+fd_inuse(struct filedesc *fdp, int fd)
+{
+   u_int off = fd >> NDENTRYSHIFT;
+
+   if (fdp->fd_lomap[off] & (1 << (fd & NDENTRYMASK)))
+   return 1;
+
+   if (fdp->fd_lomap[off] != ~0)
+   return 0;
+
+   if (fdp->fd_himap[off >> NDENTRYSHIFT] & (1 << (off & NDENTRYMASK)))
+   return 1;
+
+   return 0;
+}
+
 static __inline void
 fd_used(struct filedesc *fdp, int fd)
 {
@@ -190,7 +207,7 @@ fd_iterfile(struct file *fp, struct proc *p)
nfp = LIST_NEXT(fp, f_list);
 
/* don't FREF when f_count == 0 to avoid race in fdrop() */
-   while (nfp != NULL && (nfp->f_count == 0 || !FILE_IS_USABLE(nfp)))
+   while (nfp != NULL && nfp->f_count == 0)
nfp = LIST_NEXT(nfp, f_list);

hppa.html broken link, fix

[Marcus MERIGHI]

Now that hppa is mentioned every day...

Index: hppa.html
===
RCS file: /cvs/www/hppa.html,v
retrieving revision 1.274
diff -u -p -u -r1.274 hppa.html
--- hppa.html   2 Apr 2018 02:48:19 -   1.274
+++ hppa.html   21 May 2018 11:37:44 -
@@ -47,7 +47,7 @@ Others are definitely welcome to contrib
 
 This project was started in those days when the only
 open source operating systems for HP PA-RISC computers were
-http://www.cs.utah.edu/projects/flux/lites/html;>Lites and
+http://www.cs.utah.edu/flux/lites/html;>Lites and
 http://www.mklinux.org;>MkLinux.
 These two sources were a major supply of information and
 code for initial development of the OpenBSD/hppa port.

Re: acpithinkpad(4): port-replicator hkey event

[Tobias Tschinkowitz]

On Sun, May 06, 2018 at 11:43:43PM +0200, Tobias Tschinkowitz wrote:
> On Sun, May 06, 2018 at 09:31:55AM -0700, Mike Larkin wrote:
> > On Fri, May 04, 2018 at 08:28:04PM +0200, Tobias Tschinkowitz wrote:
> > > Hi @tech,
> > > 
> > > i thought it would be a good idea to delegate the port-replicator
> > > docking event to the sysctl(2) hw.sensors substree so that if the
> > > thinkpad gets docked/undocked one could react to it by using
> > > sensorsd(8).
> > > 
> > > Here is my diff:
> > > 
> > 
> > Can you regenerate with proper whitespace/indenting please?
> > 
> > -ml
> > 
> > 
> > > Index: acpithinkpad.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> > > retrieving revision 1.58
> > > diff -u -p -r1.58 acpithinkpad.c
> > > --- acpithinkpad.c  12 Aug 2017 17:33:51 -  1.58
> > > +++ acpithinkpad.c  4 May 2018 18:20:43 -
> > > @@ -110,8 +110,10 @@
> > >  #defineTHINKPAD_TABLET_SCREEN_CHANGED  0x60c0
> > >  #defineTHINKPAD_SWITCH_WIRELESS0x7000
> > > 
> > > -#define THINKPAD_NSENSORS 9
> > > +#define THINKPAD_NSENSORS 10
> > >  #define THINKPAD_NTEMPSENSORS 8
> > > +#define THINKPAD_SENSOR_FANRPM THINKPAD_NTEMPSENSORS
> > > +#define THINKPAD_SENSOR_PORTREPL   THINKPAD_NTEMPSENSORS + 1
> > > 
> > >  #define THINKPAD_ECOFFSET_VOLUME   0x30
> > >  #define THINKPAD_ECOFFSET_VOLUME_MUTE_MASK 0x40
> > > @@ -232,9 +234,18 @@ thinkpad_sensor_attach(struct acpithinkp
> > > sensor_attach(>sc_sensdev, >sc_sens[i]);
> > > }
> > > 
> > > -   /* Add fan probe */
> > > -   sc->sc_sens[i].type = SENSOR_FANRPM;
> > > -   sensor_attach(>sc_sensdev, >sc_sens[i]);
> > > +/* Add fan probe */
> > > +sc->sc_sens[THINKPAD_SENSOR_FANRPM].type = SENSOR_FANRPM;
> > > +sensor_attach(>sc_sensdev,
> > > +  >sc_sens[THINKPAD_SENSOR_FANRPM]);
> > > +
> > > +/* Add port replicator indicator */
> > > +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].type = SENSOR_INDICATOR;
> > > +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = SENSOR_S_UNKNOWN;
> > > +strlcpy(sc->sc_sens[THINKPAD_SENSOR_PORTREPL].desc, "port 
> > > replicator",
> > > +sizeof(sc->sc_sens[THINKPAD_SENSOR_PORTREPL].desc));
> > > +sensor_attach(>sc_sensdev,
> > > +  >sc_sens[THINKPAD_SENSOR_PORTREPL]);
> > > 
> > > sensordev_install(>sc_sensdev);
> > >  }
> > > @@ -260,7 +271,7 @@ thinkpad_sensor_refresh(void *arg)
> > > /* Read fan RPM */
> > > acpiec_read(sc->sc_ec, THINKPAD_ECOFFSET_FANLO, 1, );
> > > acpiec_read(sc->sc_ec, THINKPAD_ECOFFSET_FANHI, 1, );
> > > -   sc->sc_sens[i].value = ((hi << 8L) + lo);
> > > +   sc->sc_sens[THINKPAD_SENSOR_FANRPM].value = ((hi << 8L) + lo);
> > >  }
> > > 
> > >  void
> > > @@ -421,6 +432,14 @@ thinkpad_hotkey(struct aml_node *node, i
> > > case THINKPAD_BACKLIGHT_CHANGED:
> > > thinkpad_get_brightness(sc);
> > > break;
> > > +case THINKPAD_PORT_REPL_DOCKED:
> > > +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].value = 1;
> > > +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
> > > SENSOR_S_OK;
> > > +break;
> > > +case THINKPAD_PORT_REPL_UNDOCKED:
> > > +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].value = 0;
> > > +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
> > > SENSOR_S_OK;
> > > +break;
> > > default:
> > > /* unknown or boring event */
> > > DPRINTF(("%s: unhandled event 0x%03llx\n", 
> > > DEVNAME(sc),
> > > 
> > > 
> > > Greetings, Tobias
> > > 
> > 
> 
> Hi,
> 
> here is the corrected diff. I hope this one is okay.
> 
> Greetings, Tobias
> 
> Index: acpithinkpad.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 acpithinkpad.c
> --- acpithinkpad.c12 Aug 2017 17:33:51 -  1.58
> +++ acpithinkpad.c6 May 2018 21:37:05 -
> @@ -110,9 +110,12 @@
>  #define  THINKPAD_TABLET_SCREEN_CHANGED  0x60c0
>  #define  THINKPAD_SWITCH_WIRELESS0x7000
>  
> -#define THINKPAD_NSENSORS 9
> +#define THINKPAD_NSENSORS 10
>  #define THINKPAD_NTEMPSENSORS 8
>  
> +#define THINKPAD_SENSOR_FANRPM   THINKPAD_NTEMPSENSORS
> +#define THINKPAD_SENSOR_PORTREPL THINKPAD_NTEMPSENSORS + 1
> +
>  #define THINKPAD_ECOFFSET_VOLUME 0x30
>  #define THINKPAD_ECOFFSET_VOLUME_MUTE_MASK 0x40
>  #define THINKPAD_ECOFFSET_FANLO  0x84
> @@ -233,8 +236,16 @@ thinkpad_sensor_attach(struct acpithinkp
>   }
>  
>   /* Add fan probe */
> - sc->sc_sens[i].type = SENSOR_FANRPM;

Re: cosmetic sdio diff

[Stefan Sperling]

On Mon, May 21, 2018 at 01:35:31PM +0200, Mark Kettenis wrote:
> This diff improves the way we printf "not configured" sdio devices.
> 
> Before:
> 
> (manufacturer 0x2d0, product 0x4334)at sdmmc1 function 2 not configured
> 
> After:
> 
> manufacturer 0x02d0, product 0x4334 at sdmmc1 function 2 not configured
> 
> This is somewhat similar to how we print "unknown" pci devices,
> although I left out printing "unknown" here since we don't have a
> table wit known vendors/products.
> 
> ok?
> 

ok stsp@

> 
> Index: dev/sdmmc/sdmmc_io.c
> ===
> RCS file: /cvs/src/sys/dev/sdmmc/sdmmc_io.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 sdmmc_io.c
> --- dev/sdmmc/sdmmc_io.c  11 Feb 2018 20:58:40 -  1.32
> +++ dev/sdmmc/sdmmc_io.c  21 May 2018 11:31:31 -
> @@ -305,19 +305,18 @@ sdmmc_print(void *aux, const char *pnp)
>   if (i != 0)
>   printf("\"");
>  
> - if (cis->manufacturer != SDMMC_VENDOR_INVALID &&
> + if (cis->manufacturer != SDMMC_VENDOR_INVALID ||
>   cis->product != SDMMC_PRODUCT_INVALID) {
> - printf("%s(", i ? " " : "");
> + printf("%s", i ? " " : "");
>   if (cis->manufacturer != SDMMC_VENDOR_INVALID)
> - printf("manufacturer 0x%x%s",
> + printf("manufacturer 0x%04x%s",
>   cis->manufacturer,
>   cis->product == SDMMC_PRODUCT_INVALID ?
>   "" : ", ");
>   if (cis->product != SDMMC_PRODUCT_INVALID)
> - printf("product 0x%x", cis->product);
> - printf(")");
> + printf("product 0x%04x", cis->product);
>   }
> - printf("%sat %s", i ? " " : "", pnp);
> + printf(" at %s", pnp);
>   }
>   printf(" function %d", sf->number);
>  
> 

Re: acpithinkpad(4): port-replicator hkey event

[Stefan Sperling]

On Mon, May 21, 2018 at 11:35:54AM +0200, Tobias Tschinkowitz wrote:
> As the last diff still had formatting issues i fixed that.

OK by me.

Nitpick: There are overlong lines here (please stay <= 80 columns).

> + sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
> SENSOR_S_OK;

> + sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
> SENSOR_S_OK;


These should be wrapped after = and indented according to style(9) by
3 tabs + 4 spaces, i.e.:

sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status =
SENSOR_S_OK;

doaccept() & NET_LOCK() recursion

[Martin Pieuchot]

FILE_SET_MATURE() shouldn't be called while holding the NET_LOCK().
Because if it releases the last reference of a file it will call
soclose() and try to grab the lock again.

I posted a regression test in the following thread:
https://marc.info/?l=openbsd-tech=152637351632752=2

Diff below fixes that, ok?

Index: kern/uipc_syscalls.c
===
RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.170
diff -u -p -r1.170 uipc_syscalls.c
--- kern/uipc_syscalls.c8 May 2018 08:53:41 -   1.170
+++ kern/uipc_syscalls.c21 May 2018 11:34:54 -
@@ -338,6 +338,8 @@ doaccept(struct proc *p, int sock, struc
fp->f_flag = FREAD | FWRITE | nflag;
fp->f_ops = 
error = soaccept(so, nam);
+out:
+   sounlock(s);
if (!error && name != NULL)
error = copyaddrout(p, nam, name, namelen, anamelen);
if (!error) {
@@ -349,8 +351,6 @@ doaccept(struct proc *p, int sock, struc
FILE_SET_MATURE(fp, p);
*retval = tmpfd;
}
-out:
-   sounlock(s);
m_freem(nam);
if (error) {
fdplock(fdp);

dwiic(4) fix

[Mark Kettenis]

The diff below fixes I2C_OP_WRITE_WITH_STOP operations.  Currently we
run the read completion code for those operations, but since there is
no data to read, this fails.  Instead, this waits until a stop is
detected.  That doesn't seem to work for I2C_F_POLL operations though,
so in that case we wait until the bus is idle.  This also adds
interlocks (using splbio/splx) on the existing tsleep() operations.

This doesn't fix all the issues with the driver.  In particular it
seems that large reads still cause problems.  I think a somewhat more
invasive re-write of the dwiic_i2c_exec() function is needed.  But I'd
like to get this in before I embark on that.

ok?


Index: dev/ic/dwiic.c
===
RCS file: /cvs/src/sys/dev/ic/dwiic.c,v
retrieving revision 1.3
diff -u -p -r1.3 dwiic.c
--- dev/ic/dwiic.c  19 Jan 2018 18:20:38 -  1.3
+++ dev/ic/dwiic.c  21 May 2018 10:37:50 -
@@ -192,6 +192,7 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
u_int32_t ic_con, st, cmd, resp;
int retries, tx_limit, rx_avail, x, readpos;
uint8_t *b;
+   int s;
 
if (sc->sc_busy)
return 1;
@@ -245,12 +246,14 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
if (flags & I2C_F_POLL)
DELAY(200);
else {
+   s = splbio();
dwiic_read(sc, DW_IC_CLR_INTR);
dwiic_write(sc, DW_IC_INTR_MASK, DW_IC_INTR_TX_EMPTY);
 
if (tsleep(>sc_writewait, PRIBIO, "dwiic", hz / 2) != 0)
printf("%s: timed out waiting for tx_empty intr\n",
sc->sc_dev.dv_xname);
+   splx(s);
}
 
/* send our command, one byte at a time */
@@ -320,7 +323,7 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
 * As TXFLR fills up, we need to clear it out by reading all
 * available data.
 */
-   while (tx_limit == 0 || x == len) {
+   while (I2C_OP_READ_P(op) && (tx_limit == 0 || x == len)) {
DPRINTF(("%s: %s: tx_limit %d, sent %d read reqs\n",
sc->sc_dev.dv_xname, __func__, tx_limit, x));
 
@@ -332,6 +335,7 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
DELAY(50);
}
} else {
+   s = splbio();
dwiic_read(sc, DW_IC_CLR_INTR);
dwiic_write(sc, DW_IC_INTR_MASK,
DW_IC_INTR_RX_FULL);
@@ -341,6 +345,7 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
printf("%s: timed out waiting for "
"rx_full intr\n",
sc->sc_dev.dv_xname);
+   splx(s);
 
rx_avail = dwiic_read(sc, DW_IC_RXFLR);
}
@@ -378,6 +383,32 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
}
}
 
+   if (I2C_OP_STOP_P(op) && I2C_OP_WRITE_P(op)) {
+   if (flags & I2C_F_POLL) {
+   /* wait for bus to be idle */
+   for (retries = 100; retries > 0; retries--) {
+   st = dwiic_read(sc, DW_IC_STATUS);
+   if (!(st & DW_IC_STATUS_ACTIVITY))
+   break;
+   DELAY(1000);
+   }
+   if (st & DW_IC_STATUS_ACTIVITY)
+   printf("%s: timed out waiting for bus idle\n",
+   sc->sc_dev.dv_xname);
+   } else {
+   s = splbio();
+   while (sc->sc_busy) {
+   dwiic_write(sc, DW_IC_INTR_MASK,
+   DW_IC_INTR_STOP_DET);
+   if (tsleep(>sc_busy, PRIBIO, "dwiic",
+   hz / 2) != 0)
+   printf("%s: timed out waiting for "
+   "stop intr\n",
+   sc->sc_dev.dv_xname);
+   }
+   splx(s);
+   }
+   }
sc->sc_busy = 0;
 
return 0;
@@ -449,6 +480,11 @@ dwiic_intr(void *arg)
DPRINTF(("%s: %s: waking up writer\n",
sc->sc_dev.dv_xname, __func__));
wakeup(>sc_writewait);
+   }
+   if (stat & DW_IC_INTR_STOP_DET) {
+   dwiic_write(sc, DW_IC_INTR_MASK, 0);
+   sc->sc_busy = 0;
+   wakeup(>sc_busy);
}
}
 

Re: doaccept() & NET_LOCK() recursion

[Martin Pieuchot]

On 21/05/18(Mon) 14:18, Alexander Bluhm wrote:
> On Mon, May 21, 2018 at 01:41:45PM +0200, Martin Pieuchot wrote:
> > Diff below fixes that, ok?
> 
> This part modifies the struct socket.  It would run without
> netlock in your diff.

Indeed, updated diff.

Index: kern/uipc_syscalls.c
===
RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.170
diff -u -p -r1.170 uipc_syscalls.c
--- kern/uipc_syscalls.c8 May 2018 08:53:41 -   1.170
+++ kern/uipc_syscalls.c21 May 2018 13:09:28 -
@@ -340,24 +340,25 @@ doaccept(struct proc *p, int sock, struc
error = soaccept(so, nam);
if (!error && name != NULL)
error = copyaddrout(p, nam, name, namelen, anamelen);
+out:
if (!error) {
if (nflag & FNONBLOCK)
so->so_state |= SS_NBIO;
else
so->so_state &= ~SS_NBIO;
+   sounlock(s);
fp->f_data = so;
FILE_SET_MATURE(fp, p);
*retval = tmpfd;
-   }
-out:
-   sounlock(s);
-   m_freem(nam);
-   if (error) {
+   } else {
+   sounlock(s);
fdplock(fdp);
fdremove(fdp, tmpfd);
closef(fp, p);
fdpunlock(fdp);
}
+
+   m_freem(nam);
FRELE(headfp, p);
return (error);
 }

Re: doaccept() & NET_LOCK() recursion

[Alexander Bluhm]

On Mon, May 21, 2018 at 03:14:45PM +0200, Martin Pieuchot wrote:
> On 21/05/18(Mon) 14:18, Alexander Bluhm wrote:
> > On Mon, May 21, 2018 at 01:41:45PM +0200, Martin Pieuchot wrote:
> > > Diff below fixes that, ok?
> > 
> > This part modifies the struct socket.  It would run without
> > netlock in your diff.
> 
> Indeed, updated diff.

OK bluhm@

> Index: kern/uipc_syscalls.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
> retrieving revision 1.170
> diff -u -p -r1.170 uipc_syscalls.c
> --- kern/uipc_syscalls.c  8 May 2018 08:53:41 -   1.170
> +++ kern/uipc_syscalls.c  21 May 2018 13:09:28 -
> @@ -340,24 +340,25 @@ doaccept(struct proc *p, int sock, struc
>   error = soaccept(so, nam);
>   if (!error && name != NULL)
>   error = copyaddrout(p, nam, name, namelen, anamelen);
> +out:
>   if (!error) {
>   if (nflag & FNONBLOCK)
>   so->so_state |= SS_NBIO;
>   else
>   so->so_state &= ~SS_NBIO;
> + sounlock(s);
>   fp->f_data = so;
>   FILE_SET_MATURE(fp, p);
>   *retval = tmpfd;
> - }
> -out:
> - sounlock(s);
> - m_freem(nam);
> - if (error) {
> + } else {
> + sounlock(s);
>   fdplock(fdp);
>   fdremove(fdp, tmpfd);
>   closef(fp, p);
>   fdpunlock(fdp);
>   }
> +
> + m_freem(nam);
>   FRELE(headfp, p);
>   return (error);
>  }

Re: Wake the acpi thread up for gpio events

[Martin Pieuchot]

On 20/05/18(Sun) 11:28, Mark Kettenis wrote:
> > Date: Sun, 20 May 2018 10:44:49 +0200
> > From: Martin Pieuchot 
> > 
> > On 19/05/18(Sat) 21:39, Mark Kettenis wrote:
> > > Without the wakeup, the event doesn't get scheduled until some other
> > > event wakes up the acpi thread.  On one of my machines the gpio event
> > > reads a status byte over i2c in repsonse of a gpio event.  Without the
> > > wakeup that status byte has often been cleared/overwritten by the time
> > > the event gets scheduled.
> > > 
> > > ok?
> > 
> > ok mpi@
> > 
> > Then why not make acpi_addtask() call acpi_wakeup() if it could enqueue
> > a task?  This is how task_add(9) work.  Most of the code paths already
> > use the pattern your suggesting. 
> 
> I was thinking the same thing.  It seems acpi_interrupt() schedules a
> couple of tasks and does a single wakeup.  There are also some
> scheduling sleep-related tasks that don't have an acpi_wakeup()
> associated with them.  I don't see a fundamental reason why we can't
> do this though.  But maybe I'm missing something?

I don't see any reason either.

cosmetic sdio diff

[Mark Kettenis]

This diff improves the way we printf "not configured" sdio devices.

Before:

(manufacturer 0x2d0, product 0x4334)at sdmmc1 function 2 not configured

After:

manufacturer 0x02d0, product 0x4334 at sdmmc1 function 2 not configured

This is somewhat similar to how we print "unknown" pci devices,
although I left out printing "unknown" here since we don't have a
table wit known vendors/products.

ok?


Index: dev/sdmmc/sdmmc_io.c
===
RCS file: /cvs/src/sys/dev/sdmmc/sdmmc_io.c,v
retrieving revision 1.32
diff -u -p -r1.32 sdmmc_io.c
--- dev/sdmmc/sdmmc_io.c11 Feb 2018 20:58:40 -  1.32
+++ dev/sdmmc/sdmmc_io.c21 May 2018 11:31:31 -
@@ -305,19 +305,18 @@ sdmmc_print(void *aux, const char *pnp)
if (i != 0)
printf("\"");
 
-   if (cis->manufacturer != SDMMC_VENDOR_INVALID &&
+   if (cis->manufacturer != SDMMC_VENDOR_INVALID ||
cis->product != SDMMC_PRODUCT_INVALID) {
-   printf("%s(", i ? " " : "");
+   printf("%s", i ? " " : "");
if (cis->manufacturer != SDMMC_VENDOR_INVALID)
-   printf("manufacturer 0x%x%s",
+   printf("manufacturer 0x%04x%s",
cis->manufacturer,
cis->product == SDMMC_PRODUCT_INVALID ?
"" : ", ");
if (cis->product != SDMMC_PRODUCT_INVALID)
-   printf("product 0x%x", cis->product);
-   printf(")");
+   printf("product 0x%04x", cis->product);
}
-   printf("%sat %s", i ? " " : "", pnp);
+   printf(" at %s", pnp);
}
printf(" function %d", sf->number);
 

Re: doaccept() & NET_LOCK() recursion

[Alexander Bluhm]

On Mon, May 21, 2018 at 01:41:45PM +0200, Martin Pieuchot wrote:
> Diff below fixes that, ok?

This part modifies the struct socket.  It would run without
netlock in your diff.

if (nflag & FNONBLOCK)
so->so_state |= SS_NBIO;
else
so->so_state &= ~SS_NBIO;

bluhm

> Index: kern/uipc_syscalls.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
> retrieving revision 1.170
> diff -u -p -r1.170 uipc_syscalls.c
> --- kern/uipc_syscalls.c  8 May 2018 08:53:41 -   1.170
> +++ kern/uipc_syscalls.c  21 May 2018 11:34:54 -
> @@ -338,6 +338,8 @@ doaccept(struct proc *p, int sock, struc
>   fp->f_flag = FREAD | FWRITE | nflag;
>   fp->f_ops = 
>   error = soaccept(so, nam);
> +out:
> + sounlock(s);
>   if (!error && name != NULL)
>   error = copyaddrout(p, nam, name, namelen, anamelen);
>   if (!error) {
> @@ -349,8 +351,6 @@ doaccept(struct proc *p, int sock, struc
>   FILE_SET_MATURE(fp, p);
>   *retval = tmpfd;
>   }
> -out:
> - sounlock(s);
>   m_freem(nam);
>   if (error) {
>   fdplock(fdp);

Re: errors in usage.c - libusbhid

[Martin Pieuchot]

On 18/05/18(Fri) 10:01, David Bern wrote:
> Hello!
> 
> Have been using libusbhid and discovered a couple of discrepancies in
> the man-page (libusbhid.3) and the source of usage.c
> 
> Some are just factual misses, but I also got (what I think is) 2 errors.
> I will try to explain them;
> 
> 1. (This is the I think is an error but not sure). The man-page tells me
> that hid_usage_in_page and hid_parse_usage_in_page are each
> others inverse.
> If I haven't misunderstood the practical meaning of inverse in this
> case then this should be true:
> x == hid_parse_usage_in_page(hid_usage_in_page(x)).
> 
> My observation:
> The main reason to why this isnt true, is that hid_usage_in_page()
> returns the data of pages[k].page_contents[j].name
> while hid_parse_usage_in_page() expects the data to
> contain "%s:%s", pages[k].name, pages[k].page_contents[j].name
> 
> The reason I ask instead of just posting working code is this:
> Am I misunderstanding the manual? In that case, the solution I want
> to send in is a change in that sentence
> Or Is the manual correct and the behavior of hid_usage_in_page() wrong,
> is this something I can correct without breaking other systems?
> 
> 
> 2. The second error I found is located in hid_parse_usage_in_page().
> It is unable to parse values found in page Button.
> 
> My observation:
> usage.c is using a standard table named usb_hid_pages. In that table
> we got a page called Buttons.
> the usages in that page is shortened to "*  Button %d".
> I believe this is the cause of why hid_parse_usage_in_page() is getting
> the pagesize "wrong" and therefor unable to parse any button in the
> button page. I guess this is the case with other similar cases as well.
> 
> my conclusion is that it would be possible to handle similar cases in a
> similar way as it is handled in hid_usage_in_page().
> 
> 
> As this is my first "issue" I would love to get as much feedback as
> possible so I can work on delivering a desirable and usable patch in
> my first try.

Just send a diff, then we'll look at it 8)

Re: acpithinkpad(4): port-replicator hkey event

[Tobias Tschinkowitz]

On Mon, May 21, 2018 at 11:56:37AM +0200, Stefan Sperling wrote:
> On Mon, May 21, 2018 at 11:35:54AM +0200, Tobias Tschinkowitz wrote:
> > As the last diff still had formatting issues i fixed that.
> 
> OK by me.
> 
> Nitpick: There are overlong lines here (please stay <= 80 columns).
> 
> > +   sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
> > SENSOR_S_OK;
> 
> > +   sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
> > SENSOR_S_OK;
> 
> 
> These should be wrapped after = and indented according to style(9) by
> 3 tabs + 4 spaces, i.e.:
> 
>   sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status =
>   SENSOR_S_OK;
> 

Fixed that, thanks for the information


Index: acpithinkpad.c
===
RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
retrieving revision 1.58
diff -u -p -r1.58 acpithinkpad.c
--- acpithinkpad.c  12 Aug 2017 17:33:51 -  1.58
+++ acpithinkpad.c  21 May 2018 10:49:03 -
@@ -110,9 +110,12 @@
 #defineTHINKPAD_TABLET_SCREEN_CHANGED  0x60c0
 #defineTHINKPAD_SWITCH_WIRELESS0x7000
 
-#define THINKPAD_NSENSORS 9
+#define THINKPAD_NSENSORS 10
 #define THINKPAD_NTEMPSENSORS 8
 
+#define THINKPAD_SENSOR_FANRPM THINKPAD_NTEMPSENSORS
+#define THINKPAD_SENSOR_PORTREPL   THINKPAD_NTEMPSENSORS + 1
+
 #define THINKPAD_ECOFFSET_VOLUME   0x30
 #define THINKPAD_ECOFFSET_VOLUME_MUTE_MASK 0x40
 #define THINKPAD_ECOFFSET_FANLO0x84
@@ -233,8 +236,15 @@ thinkpad_sensor_attach(struct acpithinkp
}
 
/* Add fan probe */
-   sc->sc_sens[i].type = SENSOR_FANRPM;
-   sensor_attach(>sc_sensdev, >sc_sens[i]);
+   sc->sc_sens[THINKPAD_SENSOR_FANRPM].type = SENSOR_FANRPM;
+   sensor_attach(>sc_sensdev, >sc_sens[THINKPAD_SENSOR_FANRPM]);
+
+   /* Add port replicator indicator */
+   sc->sc_sens[THINKPAD_SENSOR_PORTREPL].type = SENSOR_INDICATOR;
+   sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = SENSOR_S_UNKNOWN;
+   strlcpy(sc->sc_sens[THINKPAD_SENSOR_PORTREPL].desc, "port replicator",
+   sizeof(sc->sc_sens[THINKPAD_SENSOR_PORTREPL].desc));
+   sensor_attach(>sc_sensdev, >sc_sens[THINKPAD_SENSOR_PORTREPL]);
 
sensordev_install(>sc_sensdev);
 }
@@ -260,7 +270,7 @@ thinkpad_sensor_refresh(void *arg)
/* Read fan RPM */
acpiec_read(sc->sc_ec, THINKPAD_ECOFFSET_FANLO, 1, );
acpiec_read(sc->sc_ec, THINKPAD_ECOFFSET_FANHI, 1, );
-   sc->sc_sens[i].value = ((hi << 8L) + lo);
+   sc->sc_sens[THINKPAD_SENSOR_FANRPM].value = ((hi << 8L) + lo);
 }
 
 void
@@ -420,6 +430,16 @@ thinkpad_hotkey(struct aml_node *node, i
break;
case THINKPAD_BACKLIGHT_CHANGED:
thinkpad_get_brightness(sc);
+   break;
+   case THINKPAD_PORT_REPL_DOCKED:
+   sc->sc_sens[THINKPAD_SENSOR_PORTREPL].value = 1;
+   sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
+   SENSOR_S_OK;
+   break;
+   case THINKPAD_PORT_REPL_UNDOCKED:
+   sc->sc_sens[THINKPAD_SENSOR_PORTREPL].value = 0;
+   sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
+   SENSOR_S_OK;
break;
default:
/* unknown or boring event */

cdce0 troubles on RPi3, few ways to fix, which one to choose?

[Karel Gardas]

Hi,

I do have 2 USB ethernet adapters based on Realtek 8153:

TP-Link UE 300
Lenovo Thinkpad USB 3.0 Gigabit Adapter

both adapters work well with 6.3-current on amd64 platform. If however I try to 
use them on RPi3 with 6.3-current, both lead to system freeze. As I see it now, 
there may be two possible ways to fix that:

1) linux way: Linux consider Realtek 8153 non conformant to CDC Ethernet specs. 
So it blacklists them from csc_ether driver and uses rtl815x driver for them:
https://github.com/torvalds/linux/blob/master/drivers/net/usb/cdc_ether.c. I've 
verified that on Raspbian too.

2) netbsd way: netbsd somehow manages to run both adapters on RPi3 fine using 
cdce driver. I'm yet to analyze differences between OBSD's and NBSD's cdce 
driver, but certainly NetBSD dwc2 USB driver looks more modern than this from 
6.3-current so my hypotesis is that more up to date dwc2 is responsible for 
working cdce using both adapters on NetBSD.

I've somehow tested linux way on TP-Link adapter by using patch below and I can 
verify that with that TP-Link runs fine on RPi3. I've also tested adding 
vendor/product code for Thinkpad adapter into cdce driver, but the only result 
was not OS freeze (so kind of progress), but

cdce0: usb error on tx: TIMEOUT
cdce0: watchdog timeout

messages printed from time to time to the console and network not working.

Basically speaking linux way is 2 lines change to fix both adapters on RPi3. 
NetBSD way is quite huge patch updating dwc2 USB driver + changes required by 
OpenBSD on top of that. Before pursuing this path I'd like to ask if this would 
be preferred way, or if the linux way is preferred in this case so I'd just 
create a small patch for it.

Thanks,
Karel

diff --git a/sys/dev/usb/if_cdce.c b/sys/dev/usb/if_cdce.c
index c24f849c4fd..737e21ac8ba 100644
--- a/sys/dev/usb/if_cdce.c
+++ b/sys/dev/usb/if_cdce.c
@@ -66,7 +66,7 @@
 #include 
 
 #include 
-
+#define CDCE_DEBUG 1
 #ifdef CDCE_DEBUG
 #define DPRINTFN(n, x) do { if (cdcedebug > (n)) printf x; } while (0)
 int cdcedebug = 0;
@@ -103,6 +103,7 @@ const struct cdce_type cdce_devs[] = {
 {{ USB_VENDOR_NETCHIP, USB_PRODUCT_NETCHIP_ETHERNETGADGET }, 0 },
 {{ USB_VENDOR_COMPAQ, USB_PRODUCT_COMPAQ_IPAQLINUX }, 0 },
 {{ USB_VENDOR_AMBIT, USB_PRODUCT_AMBIT_NTL_250 }, CDCE_SWAPUNION },
+{{ USB_VENDOR_LENOVO, USB_PRODUCT_LENOVO_THINKPAD_USB }, 0 }
 };
 #define cdce_lookup(v, p) \
 ((const struct cdce_type *)usb_lookup(cdce_devs, v, p))
diff --git a/sys/dev/usb/if_ure.c b/sys/dev/usb/if_ure.c
index b6c6c99ef34..fa28d163859 100644
--- a/sys/dev/usb/if_ure.c
+++ b/sys/dev/usb/if_ure.c
@@ -72,7 +72,8 @@ int   uredebug = 0;
 
 const struct usb_devno ure_devs[] = {
{ USB_VENDOR_REALTEK, USB_PRODUCT_REALTEK_RTL8152 },
-   { USB_VENDOR_REALTEK, USB_PRODUCT_REALTEK_RTL8153 }
+   { USB_VENDOR_REALTEK, USB_PRODUCT_REALTEK_RTL8153 },
+   { USB_VENDOR_TPLINK, USB_PRODUCT_TPLINK_RTL8153 }
 };
 
 inture_match(struct device *, void *, void *);
diff --git a/sys/dev/usb/usbdevs b/sys/dev/usb/usbdevs
index bc977839062..ae7bcce9ee6 100644
--- a/sys/dev/usb/usbdevs
+++ b/sys/dev/usb/usbdevs
@@ -2510,6 +2510,7 @@ product LEADTEK 9531  0x2101  9531 GPS
 /* Lenovo products */
 product LENOVO AX88179 0x304b  AX88179
 product LENOVO ETHERNET0x7203  USB 2.0 Ethernet
+product LENOVO THINKPAD_USB0x7205  ThinkPad USB 3.0 Ethernet Adapter
 
 /* Lexar products */
 product LEXAR JUMPSHOT 0x0001  jumpSHOT CompactFlash
@@ -4260,6 +4261,7 @@ product TOSHIBA HSDPA 0x1302  HSDPA
 product TPLINK RTL8192CU   0x0100  RTL8192CU
 product TPLINK RTL8812AU   0x0101  RTL8812AU
 product TPLINK RTL8188EUS  0x010C  RTL8188EUS
+product TPLINK RTL8153 0x0601  RTL8153
 
 /* Trek Technology products */
 product TREK THUMBDRIVE0x  ThumbDrive
diff --git a/sys/dev/usb/usbdevs.h b/sys/dev/usb/usbdevs.h
index 7c47e33fc74..769ddfe3083 100644
--- a/sys/dev/usb/usbdevs.h
+++ b/sys/dev/usb/usbdevs.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: usbdevs.h,v 1.696 2018/04/11 04:18:18 bket Exp $  */
+/* $OpenBSD$   */
 
 /*
  * THIS FILE IS AUTOMATICALLY GENERATED.  DO NOT EDIT.
@@ -2517,6 +2517,7 @@
 /* Lenovo products */
 #defineUSB_PRODUCT_LENOVO_AX88179  0x304b  /* AX88179 */
 #defineUSB_PRODUCT_LENOVO_ETHERNET 0x7203  /* USB 2.0 
Ethernet */
+#defineUSB_PRODUCT_LENOVO_THINKPAD_USB 0x7205  /* ThinkPad USB 
3.0 Ethernet Adapter */
 
 /* Lexar products */
 #defineUSB_PRODUCT_LEXAR_JUMPSHOT  0x0001  /* jumpSHOT 
CompactFlash */
@@ -4267,6 +4268,7 @@
 #defineUSB_PRODUCT_TPLINK_RTL8192CU0x0100  /* RTL8192CU */
 #defineUSB_PRODUCT_TPLINK_RTL8812AU0x0101  /* RTL8812AU */
 #defineUSB_PRODUCT_TPLINK_RTL8188EUS   0x010C  /* RTL8188EUS */
+#defineUSB_PRODUCT_TPLINK_RTL8153  0x0601  /* RTL8153 */
 
 

Re: unbuffered fread does not set error/EOF flags

[Vadim Zhukov]

#if 0
2018-05-05 22:17 GMT+03:00 Vadim Zhukov :
> 2018-05-05 22:12 GMT+03:00 Vadim Zhukov :
>> Hi all!
>>
>> Recently I was working on a program that uses stdio functions heavily.
>> While hunting for a bug, I've temporarily disabled buffering (via
>> setvbuf(f, NULL, _IONBF, 0)) and realized that program behavior was
>> changed heavily. The cause was that fread() stopped setting error flag
>> after hitting EAGAIN. It looks like its code is missing setting
>> EOF/error flags totally in case of unbuffered read, and first patch
>> (below) fixes this.
>>
>> I'm not sure if "r" variable value should be propagated to fp->_r, or
>> not. I've assumed that since things work without it now, this doesn't
>> need a change. And headache stops me from further investigation for now,
>> sorry.
>>
>> I understand that use stdio functions without buffering is somewhat
>> exotic, and that's for sure is the reason noone found this bug yet. But
>> this is still a bad thing, IMHO.
>>
>> While looking through the libc code I've also found that __sread() drops
>> __SOFF flag unconditionally for all read errors. IMHO, the EAGAIN case
>> shouldn't be affected here, since, obviously current offset doesn't
>> change and is still well-known.
>>
>> So... any comments, or ever okays? :)
>
> Doing "cvs diff" before mailing the patch works better than doing after.
> Sorry.
>
> --
> WBR,
>  Vadim Zhukov
>
>
> Index: stdio/fread.c
> ===
> RCS file: /cvs/src/lib/libc/stdio/fread.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 fread.c
> --- stdio/fread.c21 Sep 2016 04:38:56 -   1.15
> +++ stdio/fread.c5 May 2018 19:14:27 -
> @@ -79,6 +79,10 @@ fread(void *buf, size_t size, size_t cou
> p += r;
> resid -= r;
> }
> +if (r == 0)
> +fp->_flags |= __SEOF;
> +else if (r < 0)
> +fp->_flags |= __SERR;
> FUNLOCKFILE(fp);
> return ((total - resid) / size);
> }
> Index: stdio/stdio.c
> ===
> RCS file: /cvs/src/lib/libc/stdio/stdio.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 stdio.c
> --- stdio/stdio.c21 Sep 2016 04:38:56 -   1.10
> +++ stdio/stdio.c5 May 2018 19:14:27 -
> @@ -32,6 +32,7 @@
>  */
>
> #include 
> +#include 
> #include 
> #include "local.h"
>
> @@ -49,7 +50,7 @@ __sread(void *cookie, char *buf, int n)
> /* if the read succeeded, update the current offset */
> if (ret >= 0)
> fp->_offset += ret;
> -else
> +else if (errno != EAGAIN)
> fp->_flags &= ~__SOFF; /* paranoia */
> return (ret);
> }

So, is anybody interested? Here is a program that demonstrates the bug.
The buffered and unbuffered input should produce same values here,
but it doesn't. The patch above fixes the situation.

--
WBR,
  Vadim Zhukov
#endif


#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

int
main(int argc, char **argv) {
FILE*f1, *f2;
ssize_t  nwf1, nwf2, nrf1, nrf2;
int  pair1[2], pair2[2], rv = 0;
char msg[4096];

arc4random_buf(msg, sizeof(msg));

if (pipe2(pair1, O_NONBLOCK) == -1)
err(1, "pipe");
if (pipe2(pair2, O_NONBLOCK) == -1)
err(1, "pipe");

if ((f1 = fdopen(pair1[0], "r")) == NULL)
err(1, "fdopen");
if ((f2 = fdopen(pair2[0], "r")) == NULL)
err(1, "fdopen");

setvbuf(f2, NULL, _IONBF, 0);

nwf1 = write(pair1[1], msg, sizeof(msg));
printf("sent %zd bytes in buffered pipe\n", nwf1);
nwf2 = write(pair2[1], msg, sizeof(msg));
printf("sent %zd bytes in unbuffered pipe\n", nwf2);

printf("first read:\n");

nrf1 = fread(msg, 1, nwf1, f1);
printf("  buffered: fread()=>%zd, feof()=>%d, ferror()=>%d\n",
nrf1, feof(f1),   ferror(f1));
if (feof(f1))
rv++;
if (ferror(f1))
rv++;

nrf2 = fread(msg, 1, nwf2, f2);
printf("unbuffered: fread()=>%zd, feof()=>%d, ferror()=>%d\n",
nrf2, feof(f2),   ferror(f2));
if (feof(f2))
rv++;
if (ferror(f2))
rv++;

printf("second read:\n");

nrf1 = fread(msg, 1, nwf1, f1);
printf("  buffered: fread()=>%zd, feof()=>%d, ferror()=>%d\n",
nrf1, feof(f1),   ferror(f1));
if (feof(f1))
rv++;
if (!ferror(f1))
rv++;

nrf2 = fread(msg, 1, nwf2, f2);
printf("unbuffered: fread()=>%zd, feof()=>%d, ferror()=>%d\n",
nrf2, feof(f2),   ferror(f2));
if (feof(f2))
rv++;
if (!ferror(f2))
rv++;

   

Re: acpithinkpad(4): port-replicator hkey event

[Mike Larkin]

On Mon, May 21, 2018 at 12:55:13PM +0200, Tobias Tschinkowitz wrote:
> On Mon, May 21, 2018 at 11:56:37AM +0200, Stefan Sperling wrote:
> > On Mon, May 21, 2018 at 11:35:54AM +0200, Tobias Tschinkowitz wrote:
> > > As the last diff still had formatting issues i fixed that.
> > 
> > OK by me.
> > 
> > Nitpick: There are overlong lines here (please stay <= 80 columns).
> > 
> > > + sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
> > > SENSOR_S_OK;
> > 
> > > + sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
> > > SENSOR_S_OK;
> > 
> > 
> > These should be wrapped after = and indented according to style(9) by
> > 3 tabs + 4 spaces, i.e.:
> > 
> > sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status =
> > SENSOR_S_OK;
> > 
> 
> Fixed that, thanks for the information
> 

This delay is on me, sorry folks.

I'll get to it when I can.

-ml

> 
> Index: acpithinkpad.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 acpithinkpad.c
> --- acpithinkpad.c12 Aug 2017 17:33:51 -  1.58
> +++ acpithinkpad.c21 May 2018 10:49:03 -
> @@ -110,9 +110,12 @@
>  #define  THINKPAD_TABLET_SCREEN_CHANGED  0x60c0
>  #define  THINKPAD_SWITCH_WIRELESS0x7000
>  
> -#define THINKPAD_NSENSORS 9
> +#define THINKPAD_NSENSORS 10
>  #define THINKPAD_NTEMPSENSORS 8
>  
> +#define THINKPAD_SENSOR_FANRPM   THINKPAD_NTEMPSENSORS
> +#define THINKPAD_SENSOR_PORTREPL THINKPAD_NTEMPSENSORS + 1
> +
>  #define THINKPAD_ECOFFSET_VOLUME 0x30
>  #define THINKPAD_ECOFFSET_VOLUME_MUTE_MASK 0x40
>  #define THINKPAD_ECOFFSET_FANLO  0x84
> @@ -233,8 +236,15 @@ thinkpad_sensor_attach(struct acpithinkp
>   }
>  
>   /* Add fan probe */
> - sc->sc_sens[i].type = SENSOR_FANRPM;
> - sensor_attach(>sc_sensdev, >sc_sens[i]);
> + sc->sc_sens[THINKPAD_SENSOR_FANRPM].type = SENSOR_FANRPM;
> + sensor_attach(>sc_sensdev, >sc_sens[THINKPAD_SENSOR_FANRPM]);
> +
> + /* Add port replicator indicator */
> + sc->sc_sens[THINKPAD_SENSOR_PORTREPL].type = SENSOR_INDICATOR;
> + sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = SENSOR_S_UNKNOWN;
> + strlcpy(sc->sc_sens[THINKPAD_SENSOR_PORTREPL].desc, "port replicator",
> + sizeof(sc->sc_sens[THINKPAD_SENSOR_PORTREPL].desc));
> + sensor_attach(>sc_sensdev, >sc_sens[THINKPAD_SENSOR_PORTREPL]);
>  
>   sensordev_install(>sc_sensdev);
>  }
> @@ -260,7 +270,7 @@ thinkpad_sensor_refresh(void *arg)
>   /* Read fan RPM */
>   acpiec_read(sc->sc_ec, THINKPAD_ECOFFSET_FANLO, 1, );
>   acpiec_read(sc->sc_ec, THINKPAD_ECOFFSET_FANHI, 1, );
> - sc->sc_sens[i].value = ((hi << 8L) + lo);
> + sc->sc_sens[THINKPAD_SENSOR_FANRPM].value = ((hi << 8L) + lo);
>  }
>  
>  void
> @@ -420,6 +430,16 @@ thinkpad_hotkey(struct aml_node *node, i
>   break;
>   case THINKPAD_BACKLIGHT_CHANGED:
>   thinkpad_get_brightness(sc);
> + break;
> + case THINKPAD_PORT_REPL_DOCKED:
> + sc->sc_sens[THINKPAD_SENSOR_PORTREPL].value = 1;
> + sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
> + SENSOR_S_OK;
> + break;
> + case THINKPAD_PORT_REPL_UNDOCKED:
> + sc->sc_sens[THINKPAD_SENSOR_PORTREPL].value = 0;
> + sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
> + SENSOR_S_OK;
>   break;
>   default:
>   /* unknown or boring event */
> 

Re: errors in usage.c - libusbhid

[David Bern]

First diff "solves" point 1 & 2. Second diff is on the parts of the manual
that does not match the usbhid.h

Index: usage.c
===
RCS file: /cvs/src/lib/libusbhid/usage.c,v
retrieving revision 1.16
diff -u -p -r1.16 usage.c
--- usage.c 8 Oct 2014 04:49:36 -   1.16
+++ usage.c 21 May 2018 21:06:24 -
@@ -224,6 +224,7 @@ hid_usage_in_page(unsigned int u)
 {
int i = HID_USAGE(u), j, k, us;
int page = HID_PAGE(u);
+   char usage_name[100];
static char b[100];

for (k = 0; k < npages; k++)
@@ -234,12 +235,18 @@ hid_usage_in_page(unsigned int u)
for (j = 0; j < pages[k].pagesize; j++) {
us = pages[k].page_contents[j].usage;
if (us == -1) {
-   snprintf(b, sizeof b,
+   snprintf(usage_name, sizeof usage_name,
pages[k].page_contents[j].name, i);
+   snprintf(b, sizeof b,
+   "%s:%s", pages[k].name, usage_name);
+   return b;
+   }
+   if (us == i) {
+   snprintf(b, sizeof b,
+   "%s:%s", pages[k].name,
+   pages[k].page_contents[j].name);
return b;
}
-   if (us == i)
-   return pages[k].page_contents[j].name;
}
  bad:
snprintf(b, sizeof b, "0x%04x", i);
@@ -265,8 +272,9 @@ int
 hid_parse_usage_in_page(const char *name)
 {
const char *sep;
+   const char *usage_sep;
unsigned int l;
-   int k, j;
+   int k, j, us, parsed_usage;

sep = strchr(name, ':');
if (sep == NULL)
@@ -277,10 +285,20 @@ hid_parse_usage_in_page(const char *name
goto found;
return -1;
  found:
+   usage_sep = strchr(sep, '_');
sep++;
-   for (j = 0; j < pages[k].pagesize; j++)
+   for (j = 0; j < pages[k].pagesize; j++) {
+   us = pages[k].page_contents[j].usage;
+   if (us == -1) {
+   if (usage_sep == NULL)
+   return -1;
+   if (sscanf(usage_sep, "_%d", _usage))
+   return (pages[k].usage << 16 |
+   parsed_usage);
+   }
if (strcmp(pages[k].page_contents[j].name, sep) == 0)
return (pages[k].usage << 16) |
pages[k].page_contents[j].usage;
+   }
return -1;
 }

Index: usbhid.3
===
RCS file: /cvs/src/lib/libusbhid/usbhid.3,v
retrieving revision 1.17
diff -u -p -r1.17 usbhid.3
--- usbhid.313 May 2014 14:05:02 -  1.17
+++ usbhid.321 May 2018 21:02:21 -
@@ -65,22 +65,22 @@
 .Fn hid_report_size "report_desc_t d" "hid_kind_t k" "int id"
 .Ft int
 .Fn hid_locate "report_desc_t d" "u_int usage" "hid_kind_t k" "hid_item_t
*h" "int id"
-.Ft char *
+.Ft const char *
 .Fn hid_usage_page "int i"
-.Ft char *
+.Ft const char *
 .Fn hid_usage_in_page "u_int u"
 .Ft int
 .Fn hid_parse_usage_page "const char *"
-.Ft char *
+.Ft int
 .Fn hid_parse_usage_in_page "const char *"
 .Ft void
 .Fn hid_init "char *file"
 .Ft int
 .Fn hid_start "char *file"
-.Ft int
+.Ft int32_t
 .Fn hid_get_data "void *data" "hid_item_t *h"
 .Ft void
-.Fn hid_set_data "void *data" "hid_item_t *h" "u_int data"
+.Fn hid_set_data "void *data" "hid_item_t *h" "int32_t data"
 .Sh DESCRIPTION
 The
 .Nm



2018-05-21 12:07 GMT+02:00 Martin Pieuchot :

> On 18/05/18(Fri) 10:01, David Bern wrote:
> > Hello!
> >
> > Have been using libusbhid and discovered a couple of discrepancies in
> > the man-page (libusbhid.3) and the source of usage.c
> >
> > Some are just factual misses, but I also got (what I think is) 2 errors.
> > I will try to explain them;
> >
> > 1. (This is the I think is an error but not sure). The man-page tells me
> > that hid_usage_in_page and hid_parse_usage_in_page are each
> > others inverse.
> > If I haven't misunderstood the practical meaning of inverse in this
> > case then this should be true:
> > x == hid_parse_usage_in_page(hid_usage_in_page(x)).
> >
> > My observation:
> > The main reason to why this isnt true, is that hid_usage_in_page()
> > returns the data of pages[k].page_contents[j].name
> > while hid_parse_usage_in_page() expects the data to
> > contain "%s:%s", pages[k].name, pages[k].page_contents[j].name
> >
> > The reason I ask instead of just posting working code is this:
> > Am I misunderstanding the manual? In that case, the solution I want
> > to send in is a change in that sentence
> > Or Is the manual correct and the behavior of hid_usage_in_page() wrong,
> > is this something I can correct without breaking other systems?
> >
> >
> > 2. The second 

INT33F4 device information needed

[Mark Kettenis]

Hi Folks,

For a driver that I'm working on I need a variety of acpidump output
to get a better understanding of how the hardware behaves.  If you
have a machine that has the following line in its dmesg:

  "INT33F4" at iic0 addr 0x34 not configured

please send me:

1. A tar file with all the files in /var/db/acpi.

2. A full dmesg.

This device is typically found on cheap Intel Atom laptops/tablets/2-in-1s.

Thanks,

Mark

Re: Drop memory barriers from _aromic_lock() on armv7 and arm64

[Philip Guenther]

On Mon, May 14, 2018 at 11:36 AM, Mark Kettenis 
wrote:

> Since the callers now add the barriers we can drop them here.
>
> ok?
>

ok guenther@

Re: dwiic(4) fix

[Mike Larkin]

On Mon, May 21, 2018 at 12:44:47PM +0200, Mark Kettenis wrote:
> The diff below fixes I2C_OP_WRITE_WITH_STOP operations.  Currently we
> run the read completion code for those operations, but since there is
> no data to read, this fails.  Instead, this waits until a stop is
> detected.  That doesn't seem to work for I2C_F_POLL operations though,
> so in that case we wait until the bus is idle.  This also adds
> interlocks (using splbio/splx) on the existing tsleep() operations.
> 
> This doesn't fix all the issues with the driver.  In particular it
> seems that large reads still cause problems.  I think a somewhat more
> invasive re-write of the dwiic_i2c_exec() function is needed.  But I'd
> like to get this in before I embark on that.
> 
> ok?
> 

The diff reads ok to me, do you want us to do any sort of testing here before
commit?

-ml

> 
> Index: dev/ic/dwiic.c
> ===
> RCS file: /cvs/src/sys/dev/ic/dwiic.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 dwiic.c
> --- dev/ic/dwiic.c19 Jan 2018 18:20:38 -  1.3
> +++ dev/ic/dwiic.c21 May 2018 10:37:50 -
> @@ -192,6 +192,7 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
>   u_int32_t ic_con, st, cmd, resp;
>   int retries, tx_limit, rx_avail, x, readpos;
>   uint8_t *b;
> + int s;
>  
>   if (sc->sc_busy)
>   return 1;
> @@ -245,12 +246,14 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
>   if (flags & I2C_F_POLL)
>   DELAY(200);
>   else {
> + s = splbio();
>   dwiic_read(sc, DW_IC_CLR_INTR);
>   dwiic_write(sc, DW_IC_INTR_MASK, DW_IC_INTR_TX_EMPTY);
>  
>   if (tsleep(>sc_writewait, PRIBIO, "dwiic", hz / 2) != 0)
>   printf("%s: timed out waiting for tx_empty intr\n",
>   sc->sc_dev.dv_xname);
> + splx(s);
>   }
>  
>   /* send our command, one byte at a time */
> @@ -320,7 +323,7 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
>* As TXFLR fills up, we need to clear it out by reading all
>* available data.
>*/
> - while (tx_limit == 0 || x == len) {
> + while (I2C_OP_READ_P(op) && (tx_limit == 0 || x == len)) {
>   DPRINTF(("%s: %s: tx_limit %d, sent %d read reqs\n",
>   sc->sc_dev.dv_xname, __func__, tx_limit, x));
>  
> @@ -332,6 +335,7 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
>   DELAY(50);
>   }
>   } else {
> + s = splbio();
>   dwiic_read(sc, DW_IC_CLR_INTR);
>   dwiic_write(sc, DW_IC_INTR_MASK,
>   DW_IC_INTR_RX_FULL);
> @@ -341,6 +345,7 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
>   printf("%s: timed out waiting for "
>   "rx_full intr\n",
>   sc->sc_dev.dv_xname);
> + splx(s);
>  
>   rx_avail = dwiic_read(sc, DW_IC_RXFLR);
>   }
> @@ -378,6 +383,32 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
>   }
>   }
>  
> + if (I2C_OP_STOP_P(op) && I2C_OP_WRITE_P(op)) {
> + if (flags & I2C_F_POLL) {
> + /* wait for bus to be idle */
> + for (retries = 100; retries > 0; retries--) {
> + st = dwiic_read(sc, DW_IC_STATUS);
> + if (!(st & DW_IC_STATUS_ACTIVITY))
> + break;
> + DELAY(1000);
> + }
> + if (st & DW_IC_STATUS_ACTIVITY)
> + printf("%s: timed out waiting for bus idle\n",
> + sc->sc_dev.dv_xname);
> + } else {
> + s = splbio();
> + while (sc->sc_busy) {
> + dwiic_write(sc, DW_IC_INTR_MASK,
> + DW_IC_INTR_STOP_DET);
> + if (tsleep(>sc_busy, PRIBIO, "dwiic",
> + hz / 2) != 0)
> + printf("%s: timed out waiting for "
> + "stop intr\n",
> + sc->sc_dev.dv_xname);
> + }
> + splx(s);
> + }
> + }
>   sc->sc_busy = 0;
>  
>   return 0;
> @@ -449,6 +480,11 @@ dwiic_intr(void *arg)
>   DPRINTF(("%s: %s: waking up writer\n",
>   sc->sc_dev.dv_xname, __func__));
>   wakeup(>sc_writewait);
> + }
> + if (stat &