[patch] Close file descriptor before exiting ldd

[Nan Xiao]

Hi tech@,

FYI, thanks!

Best Regards
Nan Xiao

Index: ldd.c
===
RCS file: /cvs/src/libexec/ld.so/ldd/ldd.c,v
retrieving revision 1.21
diff -u -p -r1.21 ldd.c
--- ldd.c   2 Jul 2017 19:06:12 -   1.21
+++ ldd.c   27 Sep 2017 02:21:00 -
@@ -130,8 +130,10 @@ doit(char *name)
return 1;
}

-   if ((phdr = reallocarray(NULL, ehdr.e_phnum, sizeof(Elf_Phdr))) == NULL)
+   if ((phdr = reallocarray(NULL, ehdr.e_phnum, sizeof(Elf_Phdr))) == 
NULL) {
+   close(fd);
err(1, "reallocarray");
+   }
size = ehdr.e_phnum * sizeof(Elf_Phdr);

if (pread(fd, phdr, size, ehdr.e_phoff) != size) {

Re: New: netctl(8) - cli network-location manager

[Aaron Poffenberger]

I should add, I thought about adding commands like rcctl's
`get|getdef|set` commands to do full interface configuration (`set
nwid wapname` or `set bssid `) but wanted to get feedback on
the current approach before going to far down the rabbit hole.

--Aaron

* Aaron Poffenberger  [2017-09-26 20:39:29 -0500]:

> Attached is a cli utility to manage network locations. It's modeled
> after rcctl(8).
> 
> It doesn't attempt to replace netstart(8) or ifconfig(8). It works
> with them by storing location information in a directory and
> symlinking the hostname.if files to the hostname.if for the selected
> location.
> 
> At the moment if doesn't handle auto discovery of WAPs, but I have
> code I wrote earlier I'm rewriting and integrating.
> 
> netctl is written in pure shell so in theory it could be called at
> boot time. To do that, I'd probably split netctl into two pieces
> similar to the way rcctl(8) and rc.subr(8) work. One file for
> inclusion in /etc and the main ctl for /usr.
> 
> The code could be integrated into netstart at some later date.
> 
> In addition to the netctl utility I've included a man page.
> 
> Comments or feedback would be appreciated.
> 
> --Aaron
> 
> --- /dev/null Tue Sep 26 20:26:45 2017
> +++ netctlTue Sep 26 20:16:28 2017
> @@ -0,0 +1,450 @@
> +#!/bin/sh
> +#
> +# $OpenBSD$
> +#
> +# Copyright (c) 2017 Aaron Poffenberger 
> +#
> +# Permission to use, copy, modify, and distribute this software for any
> +# purpose with or without fee is hereby granted, provided that the above
> +# copyright notice and this permission notice appear in all copies.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> +# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> +# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> +# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> +# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> +# cmd OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS cmd, ARISING OUT OF
> +# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> +#
> +
> +# Turn off Strict Bourne shell mode.
> +set +o sh
> +
> +readonly __progname="netctl"
> +readonly TRUE=0
> +readonly FALSE=1
> +readonly HN_DIR=${HN_DIR:-/etc/hostname.d}
> +
> +# Log an error
> +# Usage: log_err msg [exit code]
> +# Cheerfully copied from rc.subr(8)
> +# Copyright (c) 2010, 2011, 2014-2017 Antoine Jacoutot 
> 
> +# Copyright (c) 2010, 2011 Ingo Schwarze 
> +# Copyright (c) 2010, 2011, 2014 Robert Nagy 
> +log_err() {
> + [ -n "${1}" ] && echo "${1}" 1>&2
> + [ -n "${2}" ] && exit "${2}" || exit 1
> +}
> +
> +# Log a warning
> +# Usage: log_err msg [exit code]
> +log_warning() {
> + log_msg "${1}" 1>&2
> +}
> +
> +# Log a message
> +# Usage: log_msg msg [right justify length]
> +log_msg() {
> + local -R${2:-0} _msg="${1}"
> +
> + [ -n "${_msg}" ] && echo "${_msg}"
> +}
> +
> +# Log a message to stdout or stderr
> +# Usage: usage [2]
> +usage() {
> + echo \
> + "usage:  netctl [-h]
> + netctl ls [lsarg ...]
> + netctl create|delete location ...
> + netctl [-dr] switch location [interface ...]
> + netctl enable|disable [configuration ...]
> + netctl start|stop|restart [interface ...]
> + netctl [-v] scan [interface ...]" 1>&${1:-1}
> +}
> +
> +# Get interface configuration
> +# Expects variables to be typeset/local from calling fn
> +# Usage: get_if_conf if1
> +get_if_conf() {
> + # use co-process to preserve values set in while loop
> + ifconfig $1 |&
> + while IFS=' ' read -r -p _l; do
> + _key=${_l%%*([[:blank:]])*(:) *}
> + _val=${_l##*([[:blank:]])*${_key}*(:)*([[:blank:]])}
> +
> + [[ ${_key} == 'groups' ]] && _groups=${_val}
> + [[ ${_key} == 'media' ]] && _media=${_val}
> + [[ ${_key} == 'status' ]] && _status=${_val}
> + [[ ${_key} == 'ieee80211' ]] && _ieee80211=${_val}
> + [[ ${_key} == 'inet' ]] && _inet=${_val}
> + [[ ${_key} == 'inet6' ]] && _inet6=${_val}
> + done
> +}
> +
> +# Get interfaces
> +# Expects variable _ifs to be typeset/local from calling fn
> +# Usage: get_ifs if1
> +get_ifs() {
> + local _if _excl_keys
> + # exclude network pseudo-devices
> + set -A _pseudo_devices $(ifconfig -C)
> +
> + # use co-process to preserve values set in while loop
> + ifconfig $1 |&
> + while IFS=' ' read -r -p _l; do
> + [[ "${_l}" == *flags* ]] || continue
> +
> + _if=${_l%%*([[:blank:]])*():*}
> +
> + [ -z "${_if}" ] && continue
> +
> + #_if=${_l%%*([[:blank:]])*():*}
> +
> + # exclude if-type (san _if num) in pseudo-devices
> + [[ " ${_pseudo_devices[*]}0 " == *" ${_if%%*([[:digit:]])} "* 
> ]] &&
> + 

ASCII Drawings in Manual Pages ?

[Tom Smyth]

Hello Lads, Ladies
what is the consensus about ascii drawings
for Manual pages ?
eg for point to point adddressing could we include

inet 10.3.4.5 0x 10.1.2.3
  +
  |
 +-+  |  +--+
 |  Router A   |  v  |Router B  |
 | +-++--+  |
 +-+ ^   +--+
 |
 |
inet 10.1.2.3 0x 10.3.4.5

or is the view dependent on users chosen terminal
font so you don't bother with them ?

Thanks

Tom smyth

New: netctl(8) - cli network-location manager

[Aaron Poffenberger]

Attached is a cli utility to manage network locations. It's modeled
after rcctl(8).

It doesn't attempt to replace netstart(8) or ifconfig(8). It works
with them by storing location information in a directory and
symlinking the hostname.if files to the hostname.if for the selected
location.

At the moment if doesn't handle auto discovery of WAPs, but I have
code I wrote earlier I'm rewriting and integrating.

netctl is written in pure shell so in theory it could be called at
boot time. To do that, I'd probably split netctl into two pieces
similar to the way rcctl(8) and rc.subr(8) work. One file for
inclusion in /etc and the main ctl for /usr.

The code could be integrated into netstart at some later date.

In addition to the netctl utility I've included a man page.

Comments or feedback would be appreciated.

--Aaron

--- /dev/null   Tue Sep 26 20:26:45 2017
+++ netctl  Tue Sep 26 20:16:28 2017
@@ -0,0 +1,450 @@
+#!/bin/sh
+#
+# $OpenBSD$
+#
+# Copyright (c) 2017 Aaron Poffenberger 
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+# cmd OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS cmd, ARISING OUT OF
+# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+#
+
+# Turn off Strict Bourne shell mode.
+set +o sh
+
+readonly __progname="netctl"
+readonly TRUE=0
+readonly FALSE=1
+readonly HN_DIR=${HN_DIR:-/etc/hostname.d}
+
+# Log an error
+# Usage: log_err msg [exit code]
+# Cheerfully copied from rc.subr(8)
+# Copyright (c) 2010, 2011, 2014-2017 Antoine Jacoutot 
+# Copyright (c) 2010, 2011 Ingo Schwarze 
+# Copyright (c) 2010, 2011, 2014 Robert Nagy 
+log_err() {
+   [ -n "${1}" ] && echo "${1}" 1>&2
+   [ -n "${2}" ] && exit "${2}" || exit 1
+}
+
+# Log a warning
+# Usage: log_err msg [exit code]
+log_warning() {
+   log_msg "${1}" 1>&2
+}
+
+# Log a message
+# Usage: log_msg msg [right justify length]
+log_msg() {
+   local -R${2:-0} _msg="${1}"
+
+   [ -n "${_msg}" ] && echo "${_msg}"
+}
+
+# Log a message to stdout or stderr
+# Usage: usage [2]
+usage() {
+   echo \
+   "usage:  netctl [-h]
+   netctl ls [lsarg ...]
+   netctl create|delete location ...
+   netctl [-dr] switch location [interface ...]
+   netctl enable|disable [configuration ...]
+   netctl start|stop|restart [interface ...]
+   netctl [-v] scan [interface ...]" 1>&${1:-1}
+}
+
+# Get interface configuration
+# Expects variables to be typeset/local from calling fn
+# Usage: get_if_conf if1
+get_if_conf() {
+   # use co-process to preserve values set in while loop
+   ifconfig $1 |&
+   while IFS=' ' read -r -p _l; do
+   _key=${_l%%*([[:blank:]])*(:) *}
+   _val=${_l##*([[:blank:]])*${_key}*(:)*([[:blank:]])}
+
+   [[ ${_key} == 'groups' ]] && _groups=${_val}
+   [[ ${_key} == 'media' ]] && _media=${_val}
+   [[ ${_key} == 'status' ]] && _status=${_val}
+   [[ ${_key} == 'ieee80211' ]] && _ieee80211=${_val}
+   [[ ${_key} == 'inet' ]] && _inet=${_val}
+   [[ ${_key} == 'inet6' ]] && _inet6=${_val}
+   done
+}
+
+# Get interfaces
+# Expects variable _ifs to be typeset/local from calling fn
+# Usage: get_ifs if1
+get_ifs() {
+   local _if _excl_keys
+   # exclude network pseudo-devices
+   set -A _pseudo_devices $(ifconfig -C)
+
+   # use co-process to preserve values set in while loop
+   ifconfig $1 |&
+   while IFS=' ' read -r -p _l; do
+   [[ "${_l}" == *flags* ]] || continue
+
+   _if=${_l%%*([[:blank:]])*():*}
+
+   [ -z "${_if}" ] && continue
+
+   #_if=${_l%%*([[:blank:]])*():*}
+
+   # exclude if-type (san _if num) in pseudo-devices
+   [[ " ${_pseudo_devices[*]}0 " == *" ${_if%%*([[:digit:]])} "* 
]] &&
+   continue
+
+   _ifs[${#_ifs[*]}]="${_if}"
+   done
+
+   [ -n "${_ifs}" ] || return 1
+}
+
+get_locations() {
+   local _l
+
+   ls -p "${HN_DIR}" |&
+   # use co-process to preserve values set in while loop
+   while IFS='  ' read -r -p _l ; do
+   [[ "${_l}" == *([[:blank:]])*/ ]] || continue
+   _locations[${#_locations[*]}]="${_l%%/}"
+   done
+}
+
+get_configurations() {
+   local _l _location
+
+   _location="$1"
+
+   ls -p 

Re: int32_t float word of M_PI_4

[Mark Kettenis]

Jan Stary schreef op 2017-09-27 00:45:

The int32_t float word of M_PI_4 is 0x3f490fdb, not 0x3f490fd8.


What bug are you trying to fix?


On Sep 26 11:57:29, i...@darwinsys.com wrote:

On 2017-09-26 11:41 AM, Jan Stary wrote:
>double 0.785398, high  word 0x3fe921fb
>float  0.785398, float word 0x3f490fdb
>
> In case of double, it's exactly 0x3fe921fb.
> But why then does s_sinf.c use 0x3f490fd8 instead of 0x3f490fdb?
>
Educated guess? Because 8 and upper-case B look alike when you've had 
too

much beer or when you left your reading glasses in the car.


Index: s_sinf.c
===
RCS file: /cvs/src/lib/libm/src/s_sinf.c,v
retrieving revision 1.4
diff -u -p -r1.4 s_sinf.c
--- s_sinf.c12 Sep 2016 19:47:02 -  1.4
+++ s_sinf.c26 Sep 2017 22:42:26 -
@@ -26,7 +26,7 @@ sinf(float x)

 /* |x| ~< pi/4 */
ix &= 0x7fff;
-   if(ix <= 0x3f490fd8) return __kernel_sinf(x,z,0);
+   if(ix <= 0x3f490fdb) return __kernel_sinf(x,z,0);

 /* sin(Inf or NaN) is NaN */
else if (ix>=0x7f80) return x-x;
Index: s_cosf.c
===
RCS file: /cvs/src/lib/libm/src/s_cosf.c,v
retrieving revision 1.5
diff -u -p -r1.5 s_cosf.c
--- s_cosf.c12 Sep 2016 19:47:02 -  1.5
+++ s_cosf.c26 Sep 2017 22:42:26 -
@@ -26,7 +26,7 @@ cosf(float x)

 /* |x| ~< pi/4 */
ix &= 0x7fff;
-   if(ix <= 0x3f490fd8) return __kernel_cosf(x,z);
+   if(ix <= 0x3f490fdb) return __kernel_cosf(x,z);

 /* cos(Inf or NaN) is NaN */
else if (ix>=0x7f80) return x-x;

int32_t float word of M_PI_4

[Jan Stary]

The int32_t float word of M_PI_4 is 0x3f490fdb, not 0x3f490fd8.

Jan

On Sep 26 11:57:29, i...@darwinsys.com wrote:
> On 2017-09-26 11:41 AM, Jan Stary wrote:
> >double 0.785398, high  word 0x3fe921fb
> >float  0.785398, float word 0x3f490fdb
> > 
> > In case of double, it's exactly 0x3fe921fb.
> > But why then does s_sinf.c use 0x3f490fd8 instead of 0x3f490fdb?
> > 
> Educated guess? Because 8 and upper-case B look alike when you've had too
> much beer or when you left your reading glasses in the car.

Index: s_sinf.c
===
RCS file: /cvs/src/lib/libm/src/s_sinf.c,v
retrieving revision 1.4
diff -u -p -r1.4 s_sinf.c
--- s_sinf.c12 Sep 2016 19:47:02 -  1.4
+++ s_sinf.c26 Sep 2017 22:42:26 -
@@ -26,7 +26,7 @@ sinf(float x)
 
 /* |x| ~< pi/4 */
ix &= 0x7fff;
-   if(ix <= 0x3f490fd8) return __kernel_sinf(x,z,0);
+   if(ix <= 0x3f490fdb) return __kernel_sinf(x,z,0);
 
 /* sin(Inf or NaN) is NaN */
else if (ix>=0x7f80) return x-x;
Index: s_cosf.c
===
RCS file: /cvs/src/lib/libm/src/s_cosf.c,v
retrieving revision 1.5
diff -u -p -r1.5 s_cosf.c
--- s_cosf.c12 Sep 2016 19:47:02 -  1.5
+++ s_cosf.c26 Sep 2017 22:42:26 -
@@ -26,7 +26,7 @@ cosf(float x)
 
 /* |x| ~< pi/4 */
ix &= 0x7fff;
-   if(ix <= 0x3f490fd8) return __kernel_cosf(x,z);
+   if(ix <= 0x3f490fdb) return __kernel_cosf(x,z);
 
 /* cos(Inf or NaN) is NaN */
else if (ix>=0x7f80) return x-x;

Re: preliminary kabylake support for inteldrm

[Base Pr1me]

Sorry, have to send this from gmail right now. WOO, suspend works! I'll
keep testing this week.

On Tue, Sep 26, 2017 at 4:07 PM, Robert Nagy  wrote:

>
> Hi
>
> This is an updated diff for preliminary kabylake support for 6.2,
> this needs extensive testing on all inteldrm variants.
>
> This diff is also in snapshots now so please, test, test test!
>
> Thank you
>
> Index: sys/dev/pci/drm/i915_pciids.h
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915_pciids.h,v
> retrieving revision 1.3
> diff -u -p -u -r1.3 i915_pciids.h
> --- sys/dev/pci/drm/i915_pciids.h   1 Jul 2017 16:14:10 -   1.3
> +++ sys/dev/pci/drm/i915_pciids.h   26 Sep 2017 14:57:52 -
> @@ -295,4 +295,40 @@
> INTEL_VGA_DEVICE(0x5A84, info), /* APL HD Graphics 505 */ \
> INTEL_VGA_DEVICE(0x5A85, info)  /* APL HD Graphics 500 */
>
> +#define INTEL_KBL_GT1_IDS(info)\
> +   INTEL_VGA_DEVICE(0x5913, info), /* ULT GT1.5 */ \
> +   INTEL_VGA_DEVICE(0x5915, info), /* ULX GT1.5 */ \
> +   INTEL_VGA_DEVICE(0x5917, info), /* DT  GT1.5 */ \
> +   INTEL_VGA_DEVICE(0x5906, info), /* ULT GT1 */ \
> +   INTEL_VGA_DEVICE(0x590E, info), /* ULX GT1 */ \
> +   INTEL_VGA_DEVICE(0x5902, info), /* DT  GT1 */ \
> +   INTEL_VGA_DEVICE(0x590B, info), /* Halo GT1 */ \
> +   INTEL_VGA_DEVICE(0x590A, info) /* SRV GT1 */
> +
> +#define INTEL_KBL_GT2_IDS(info)\
> +   INTEL_VGA_DEVICE(0x5916, info), /* ULT GT2 */ \
> +   INTEL_VGA_DEVICE(0x5921, info), /* ULT GT2F */ \
> +   INTEL_VGA_DEVICE(0x591E, info), /* ULX GT2 */ \
> +   INTEL_VGA_DEVICE(0x5912, info), /* DT  GT2 */ \
> +   INTEL_VGA_DEVICE(0x591B, info), /* Halo GT2 */ \
> +   INTEL_VGA_DEVICE(0x591A, info), /* SRV GT2 */ \
> +   INTEL_VGA_DEVICE(0x591D, info) /* WKS GT2 */
> +
> +#define INTEL_KBL_GT3_IDS(info)\
> +   INTEL_VGA_DEVICE(0x5926, info), /* ULT GT3 */ \
> +   INTEL_VGA_DEVICE(0x592B, info), /* Halo GT3 */ \
> +   INTEL_VGA_DEVICE(0x592A, info) /* SRV GT3 */
> +
> +#define INTEL_KBL_GT4_IDS(info)\
> +   INTEL_VGA_DEVICE(0x5932, info), /* DT  GT4 */ \
> +   INTEL_VGA_DEVICE(0x593B, info), /* Halo GT4 */ \
> +   INTEL_VGA_DEVICE(0x593A, info), /* SRV GT4 */ \
> +   INTEL_VGA_DEVICE(0x593D, info)  /* WKS GT4 */
> +
> +#define INTEL_KBL_IDS(info) \
> +   INTEL_KBL_GT1_IDS(info), \
> +   INTEL_KBL_GT2_IDS(info), \
> +   INTEL_KBL_GT3_IDS(info), \
> +   INTEL_KBL_GT4_IDS(info)
> +
>  #endif /* _I915_PCIIDS_H */
> Index: sys/dev/pci/drm/i915/i915_devlist.h
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_devlist.h,v
> retrieving revision 1.6
> diff -u -p -u -r1.6 i915_devlist.h
> --- sys/dev/pci/drm/i915/i915_devlist.h 6 Jul 2017 10:09:26 -
>  1.6
> +++ sys/dev/pci/drm/i915/i915_devlist.h 26 Sep 2017 14:57:52 -
> @@ -154,4 +154,26 @@ static const struct pci_matchid i915_dev
> { 0x8086, 0x1a85 },
> { 0x8086, 0x5a84 },
> { 0x8086, 0x5a85 },
> +   { 0x8086, 0x5913 },
> +   { 0x8086, 0x5915 },
> +   { 0x8086, 0x5917 },
> +   { 0x8086, 0x5906 },
> +   { 0x8086, 0x590e },
> +   { 0x8086, 0x5902 },
> +   { 0x8086, 0x590b },
> +   { 0x8086, 0x590a },
> +   { 0x8086, 0x5916 },
> +   { 0x8086, 0x5921 },
> +   { 0x8086, 0x591e },
> +   { 0x8086, 0x5912 },
> +   { 0x8086, 0x591b },
> +   { 0x8086, 0x591a },
> +   { 0x8086, 0x591d },
> +   { 0x8086, 0x5926 },
> +   { 0x8086, 0x592b },
> +   { 0x8086, 0x592a },
> +   { 0x8086, 0x5932 },
> +   { 0x8086, 0x593b },
> +   { 0x8086, 0x593a },
> +   { 0x8086, 0x593d },
>  };
> Index: sys/dev/pci/drm/i915/i915_dma.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_dma.c,v
> retrieving revision 1.25
> diff -u -p -u -r1.25 i915_dma.c
> --- sys/dev/pci/drm/i915/i915_dma.c 1 Jul 2017 16:14:10 -
>  1.25
> +++ sys/dev/pci/drm/i915/i915_dma.c 26 Sep 2017 14:57:52 -
> @@ -712,7 +712,8 @@ static void gen9_sseu_info_init(struct d
>  * supports EU power gating on devices with more than one EU
>  * pair per subslice.
> */
> -   info->has_slice_pg = (IS_SKYLAKE(dev) && (info->slice_total > 1));
> +   info->has_slice_pg = ((IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) &&
> +  (info->slice_total > 1));
> info->has_subslice_pg = (IS_BROXTON(dev) && (info->subslice_total
> > 1));
> info->has_eu_pg = (info->eu_per_subslice > 2);
>  }
> @@ -858,6 +859,37 @@ static void intel_device_info_runtime_in
> DRM_INFO("Display fused off, disabling\n");
> info->num_pipes = 0;
> }
> +   } else if (info->num_pipes > 0 && 

Xr cos(3) in sin(3)

[Jan Stary]

Every trigonometric function's manpage references every other,
with a single exception: sin(3) does not Xr cos 3.

Jan

Index: sin.3
===
RCS file: /cvs/src/lib/libm/man/sin.3,v
retrieving revision 1.16
diff -u -p -r1.16 sin.3
--- sin.3   17 Jul 2013 05:42:11 -  1.16
+++ sin.3   26 Sep 2017 22:18:57 -
@@ -72,6 +72,7 @@ functions return the sine value.
 .Xr asin 3 ,
 .Xr atan 3 ,
 .Xr atan2 3 ,
+.Xr cos 3 ,
 .Xr cosh 3 ,
 .Xr sinh 3 ,
 .Xr tan 3 ,

Re: preliminary kabylake support for inteldrm

[Robert Nagy]

Hi

This is an updated diff for preliminary kabylake support for 6.2,
this needs extensive testing on all inteldrm variants.

This diff is also in snapshots now so please, test, test test!

Thank you

Index: sys/dev/pci/drm/i915_pciids.h
===
RCS file: /cvs/src/sys/dev/pci/drm/i915_pciids.h,v
retrieving revision 1.3
diff -u -p -u -r1.3 i915_pciids.h
--- sys/dev/pci/drm/i915_pciids.h   1 Jul 2017 16:14:10 -   1.3
+++ sys/dev/pci/drm/i915_pciids.h   26 Sep 2017 14:57:52 -
@@ -295,4 +295,40 @@
INTEL_VGA_DEVICE(0x5A84, info), /* APL HD Graphics 505 */ \
INTEL_VGA_DEVICE(0x5A85, info)  /* APL HD Graphics 500 */
 
+#define INTEL_KBL_GT1_IDS(info)\
+   INTEL_VGA_DEVICE(0x5913, info), /* ULT GT1.5 */ \
+   INTEL_VGA_DEVICE(0x5915, info), /* ULX GT1.5 */ \
+   INTEL_VGA_DEVICE(0x5917, info), /* DT  GT1.5 */ \
+   INTEL_VGA_DEVICE(0x5906, info), /* ULT GT1 */ \
+   INTEL_VGA_DEVICE(0x590E, info), /* ULX GT1 */ \
+   INTEL_VGA_DEVICE(0x5902, info), /* DT  GT1 */ \
+   INTEL_VGA_DEVICE(0x590B, info), /* Halo GT1 */ \
+   INTEL_VGA_DEVICE(0x590A, info) /* SRV GT1 */
+
+#define INTEL_KBL_GT2_IDS(info)\
+   INTEL_VGA_DEVICE(0x5916, info), /* ULT GT2 */ \
+   INTEL_VGA_DEVICE(0x5921, info), /* ULT GT2F */ \
+   INTEL_VGA_DEVICE(0x591E, info), /* ULX GT2 */ \
+   INTEL_VGA_DEVICE(0x5912, info), /* DT  GT2 */ \
+   INTEL_VGA_DEVICE(0x591B, info), /* Halo GT2 */ \
+   INTEL_VGA_DEVICE(0x591A, info), /* SRV GT2 */ \
+   INTEL_VGA_DEVICE(0x591D, info) /* WKS GT2 */
+
+#define INTEL_KBL_GT3_IDS(info)\
+   INTEL_VGA_DEVICE(0x5926, info), /* ULT GT3 */ \
+   INTEL_VGA_DEVICE(0x592B, info), /* Halo GT3 */ \
+   INTEL_VGA_DEVICE(0x592A, info) /* SRV GT3 */
+
+#define INTEL_KBL_GT4_IDS(info)\
+   INTEL_VGA_DEVICE(0x5932, info), /* DT  GT4 */ \
+   INTEL_VGA_DEVICE(0x593B, info), /* Halo GT4 */ \
+   INTEL_VGA_DEVICE(0x593A, info), /* SRV GT4 */ \
+   INTEL_VGA_DEVICE(0x593D, info)  /* WKS GT4 */
+
+#define INTEL_KBL_IDS(info) \
+   INTEL_KBL_GT1_IDS(info), \
+   INTEL_KBL_GT2_IDS(info), \
+   INTEL_KBL_GT3_IDS(info), \
+   INTEL_KBL_GT4_IDS(info)
+
 #endif /* _I915_PCIIDS_H */
Index: sys/dev/pci/drm/i915/i915_devlist.h
===
RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_devlist.h,v
retrieving revision 1.6
diff -u -p -u -r1.6 i915_devlist.h
--- sys/dev/pci/drm/i915/i915_devlist.h 6 Jul 2017 10:09:26 -   1.6
+++ sys/dev/pci/drm/i915/i915_devlist.h 26 Sep 2017 14:57:52 -
@@ -154,4 +154,26 @@ static const struct pci_matchid i915_dev
{ 0x8086, 0x1a85 },
{ 0x8086, 0x5a84 },
{ 0x8086, 0x5a85 },
+   { 0x8086, 0x5913 },
+   { 0x8086, 0x5915 },
+   { 0x8086, 0x5917 },
+   { 0x8086, 0x5906 },
+   { 0x8086, 0x590e },
+   { 0x8086, 0x5902 },
+   { 0x8086, 0x590b },
+   { 0x8086, 0x590a },
+   { 0x8086, 0x5916 },
+   { 0x8086, 0x5921 },
+   { 0x8086, 0x591e },
+   { 0x8086, 0x5912 },
+   { 0x8086, 0x591b },
+   { 0x8086, 0x591a },
+   { 0x8086, 0x591d },
+   { 0x8086, 0x5926 },
+   { 0x8086, 0x592b },
+   { 0x8086, 0x592a },
+   { 0x8086, 0x5932 },
+   { 0x8086, 0x593b },
+   { 0x8086, 0x593a },
+   { 0x8086, 0x593d },
 };
Index: sys/dev/pci/drm/i915/i915_dma.c
===
RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_dma.c,v
retrieving revision 1.25
diff -u -p -u -r1.25 i915_dma.c
--- sys/dev/pci/drm/i915/i915_dma.c 1 Jul 2017 16:14:10 -   1.25
+++ sys/dev/pci/drm/i915/i915_dma.c 26 Sep 2017 14:57:52 -
@@ -712,7 +712,8 @@ static void gen9_sseu_info_init(struct d
 * supports EU power gating on devices with more than one EU
 * pair per subslice.
*/
-   info->has_slice_pg = (IS_SKYLAKE(dev) && (info->slice_total > 1));
+   info->has_slice_pg = ((IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) &&
+  (info->slice_total > 1));
info->has_subslice_pg = (IS_BROXTON(dev) && (info->subslice_total > 1));
info->has_eu_pg = (info->eu_per_subslice > 2);
 }
@@ -858,6 +859,37 @@ static void intel_device_info_runtime_in
DRM_INFO("Display fused off, disabling\n");
info->num_pipes = 0;
}
+   } else if (info->num_pipes > 0 && INTEL_INFO(dev)->gen == 9) {
+   u32 dfsm = I915_READ(SKL_DFSM);
+   u8 disabled_mask = 0;
+   bool invalid;
+   int num_bits;
+
+   if (dfsm & SKL_DFSM_PIPE_A_DISABLE)
+   disabled_mask |= BIT(PIPE_A);
+   if (dfsm & SKL_DFSM_PIPE_B_DISABLE)
+   disabled_mask |= BIT(PIPE_B);
+ 

Re: delaying the start of ifstated in /etc/rc

[Rob Pierce]

On Wed, Aug 30, 2017 at 08:30:52PM -0400, Rob Pierce wrote:
> Depending on the use case for ifstated, dependencies may exist with other
> daemons for performing interface checks and/or external tests. For example,
> one might use ifstated to check a dhcpd enabled interface, or connectivity to
> a vmd virtual machine.
> 
> Does anyone have any objections with delaying the start of ifstated?
> 
> Comments? Ok?

For the record, this is a bad diff; ifstated should start prior to disabling
the carp interlock. Starting ifstated after dhcpd might make sense for the
common use case, but maybe it is best for this to remain as is with local
modifications applied based on local use cases.

Rob

> Index: src/etc/rc
> ===
> RCS file: /cvs/src/etc/rc,v
> retrieving revision 1.517
> diff -u -p -r1.517 rc
> --- src/etc/rc29 Aug 2017 16:56:13 -  1.517
> +++ src/etc/rc31 Aug 2017 00:17:06 -
> @@ -558,7 +558,7 @@ echo 'preserving editor files.'; /usr/li
>  run_upgrade_script sysmerge
>  
>  echo -n 'starting network daemons:'
> -start_daemon ldomd sshd switchd snmpd ldpd ripd ospfd ospf6d bgpd ifstated
> +start_daemon ldomd sshd switchd snmpd ldpd ripd ospfd ospf6d bgpd
>  start_daemon relayd dhcpd dhcrelay mrouted dvmrpd radiusd eigrpd
>  
>  if ifconfig lo0 inet6 >/dev/null 2>&1; then
> @@ -569,7 +569,7 @@ fi
>  
>  start_daemon hostapd lpd smtpd slowcgi httpd ftpd
>  start_daemon ftpproxy ftpproxy6 tftpd tftpproxy identd inetd rarpd bootparamd
> -start_daemon rbootd mopd vmd spamd spamlogd sndiod
> +start_daemon rbootd mopd vmd spamd spamlogd sndiod ifstated
>  echo '.'
>  
>  # If rc.firsttime exists, run it just once, and make sure it is deleted.
> 

Re: sin() implementation

[Jimmy Hess]

On Tue, Sep 26, 2017 at 6:27 AM, Jan Stary  wrote:


> s_sin.c normalizes the argument to [-pi/4, +pi/4].
> This is how |x| <= pi/4 is tested:
> GET_HIGH_WORD(ix,x);
> ix &= 0x7fff;
> if(ix <= 0x3fe921fb) return __kernel_sin(x,z,0);
>
> Why is it done like that? Is it faster or more portable
> or in any way better that comparing the value itself to M_PI_4?
>

See  s_cos.c  s_tan.c;   and many others start with a similar block.
Probably the integer comparison is efficient (fewer average CPU and bus
resources per second tied up per operation).   It will  depend upon your
computer's  characteristics,  and  applications running, and how many times
per second  sin()  is  invoked   on average.

Assuming the compiler optimization of the above code block is as good as it
could be:
that should hinge on whether a single integer copy, and 32-bit integer
subtraction
and Bitwise AND on your CPU will burn more processor work cycles than  two
floating-point copy,
two  floating-point subtraction plus   one  sign negate  operation against
IEEE 64-bit double.

The basic rule of thumb says  that if a  performance-significant task can
be completed using Integers
instead, then maximum effort should be made to avoid/minimize unnecessary
usage of
floating-point operations  (without an excessive increase in the # of
operations),


As  3 to 4 times the processor work cycles for execution path to
 3-argument  64-bit floating point
calculation Versus the corresponding 3-argument operation using 32-bit
integers.

More floating point operations/sec = lower system efficiency,  or
less-effective use of the overall available
compute capacity Versus   the use of  processors'  regular,  typically
more-optimized ALU pipelines.

> if (-M_PI_4 < x && x < M_PI_4)


> >For Inf and NaN arguments, NaN is returned as follows:
> >/* sin(Inf or NaN) is NaN */
> >else if (ix>=0x7ff0) return x-x;
>


> > Again, why the integer conversion? Would
> >
> >else if (isinf(x) || isnan(x))


> Also, is "return x-x" just a fast/clever way to return NaN
>

Well,  the original code is applying a >=  test,whereas isinf() and
isnan() are
shown to be equality  tests  that check for two specific values of the
exponent field.
The  ||  branching is involving more computations though

General application code should  use  the new  isinf(),  isnan(),   and
nan("")  macros,
but they have not always been part of the C standard,  and I imagine this
code predates it.

The  NaN/InF values are  runtime-specific in the current C standard,   so
it is
only sort of legitimate to do that,   Because  this  code is part of the
 libc runtime's
implementation itself.

In this context,   the libc implementation has enough information to safely
rely
on the assumption to make the decision without resorting to a macro call
 that
invokes a number of other operations,   and so

Is free  and perfectly fine  to use that   ix >= 0x7ff0  to  equate
to a  NaN  condition.

That appears to potentially streamline the number of computations required
for this case,
so there  might actually be a performance benefit to not use the macros
here as well.

-- 
-JH

Re: armv7 / H3 i2c gates+resets + quirk for >=sun6i-a31-i2c compat.

[Mike Larkin]

On Tue, Sep 26, 2017 at 07:14:45PM +0200, Patrick Wildt wrote:
> On Tue, Sep 26, 2017 at 09:48:41AM -0700, Mike Larkin wrote:
> > On Tue, Sep 26, 2017 at 03:08:40PM +0300, Artturi Alm wrote:
> > > Hi,
> > > 
> > > diff does add the quirk needed on sun6i-a31, and above including sun8i-h3,
> > > for which it does also add related gates+resets.
> > > 
> > > -Artturi
> > > 
> > 
> > what bug does this fix?
> 
> I think the messages he intended to write were:
> 
> Add support for Allwinner's H3 I2C clocks, so that we can properly reset
> and turn on the I2C controller.
> 
> and
> 
> The Allwinner H3 SoC uses the same I2C controller as the other Allwinner
> SoCs, but there's a single difference:  The IFLG flag is inverted on the
> H3, so in that case we must set instead of clear it.
> 
> Or something like that.
> 
> Patrick
> 

Thanks Patrick, that's much clearer.

-ml

> > 
> > > 
> > > diff --git a/sys/dev/fdt/sxiccmu_clocks.h b/sys/dev/fdt/sxiccmu_clocks.h
> > > index 8b25ac42bd4..f82613e0125 100644
> > > --- a/sys/dev/fdt/sxiccmu_clocks.h
> > > +++ b/sys/dev/fdt/sxiccmu_clocks.h
> > > @@ -95,6 +95,10 @@ struct sxiccmu_ccu_bit sun50i_a64_gates[] = {
> > >  
> > >  #define H3_CLK_BUS_PIO   54
> > >  
> > > +#define H3_CLK_BUS_I2C0  59
> > > +#define H3_CLK_BUS_I2C1  60
> > > +#define H3_CLK_BUS_I2C2  61
> > > +
> > >  #define H3_CLK_BUS_UART0 62
> > >  #define H3_CLK_BUS_UART1 63
> > >  #define H3_CLK_BUS_UART2 64
> > > @@ -128,6 +132,9 @@ struct sxiccmu_ccu_bit sun8i_h3_gates[] = {
> > >   [H3_CLK_BUS_OHCI2] = { 0x0060, 30 },
> > >   [H3_CLK_BUS_OHCI3] = { 0x0060, 31 },
> > >   [H3_CLK_BUS_PIO]   = { 0x0068, 5 },
> > > + [H3_CLK_BUS_I2C0] = { 0x006c, 0, H3_CLK_APB2 },
> > > + [H3_CLK_BUS_I2C1] = { 0x006c, 1, H3_CLK_APB2 },
> > > + [H3_CLK_BUS_I2C2] = { 0x006c, 2, H3_CLK_APB2 },
> > >   [H3_CLK_BUS_UART0] = { 0x006c, 16, H3_CLK_APB2 },
> > >   [H3_CLK_BUS_UART1] = { 0x006c, 17, H3_CLK_APB2 },
> > >   [H3_CLK_BUS_UART2] = { 0x006c, 18, H3_CLK_APB2 },
> > > @@ -195,6 +202,10 @@ struct sxiccmu_ccu_bit sun50i_a64_resets[] = {
> > >  
> > >  #define H3_RST_BUS_EPHY  39
> > >  
> > > +#define H3_RST_BUS_I2C0  46
> > > +#define H3_RST_BUS_I2C1  47
> > > +#define H3_RST_BUS_I2C2  48
> > > +
> > >  struct sxiccmu_ccu_bit sun8i_h3_resets[] = {
> > >   [H3_RST_USB_PHY0] =  { 0x00cc, 0 },
> > >   [H3_RST_USB_PHY1] =  { 0x00cc, 1 },
> > > @@ -213,4 +224,7 @@ struct sxiccmu_ccu_bit sun8i_h3_resets[] = {
> > >   [H3_RST_BUS_OHCI2] = { 0x02c0, 30 },
> > >   [H3_RST_BUS_OHCI3] = { 0x02c0, 31 },
> > >   [H3_RST_BUS_EPHY]  = { 0x02c8, 2 },
> > > + [H3_RST_BUS_I2C0] =  { 0x02d8, 0 },
> > > + [H3_RST_BUS_I2C1] =  { 0x02d8, 1 },
> > > + [H3_RST_BUS_I2C2] =  { 0x02d8, 2 },
> > >  };
> > > diff --git a/sys/dev/fdt/sxitwi.c b/sys/dev/fdt/sxitwi.c
> > > index f53f2bfd594..e98b36f9588 100644
> > > --- a/sys/dev/fdt/sxitwi.c
> > > +++ b/sys/dev/fdt/sxitwi.c
> > > @@ -144,6 +144,7 @@ struct sxitwi_softc {
> > >   bus_space_handle_t   sc_ioh;
> > >   int  sc_node;
> > >   u_intsc_started;
> > > + u_intsc_twsien_iflg;
> > >   struct i2c_controllersc_ic;
> > >   struct rwlocksc_buslock;
> > >   void*sc_ih;
> > > @@ -179,7 +180,8 @@ sxitwi_match(struct device *parent, void *match, void 
> > > *aux)
> > >   struct fdt_attach_args *faa = aux;
> > >  
> > >   return (OF_is_compatible(faa->fa_node, "allwinner,sun4i-a10-i2c") ||
> > > - OF_is_compatible(faa->fa_node, "allwinner,sun7i-a20-i2c"));
> > > + OF_is_compatible(faa->fa_node, "allwinner,sun7i-a20-i2c") ||
> > > + OF_is_compatible(faa->fa_node, "allwinner,sun6i-a31-i2c"));
> > >  }
> > >  
> > >  void
> > > @@ -206,6 +208,8 @@ sxitwi_attach(struct device *parent, struct device 
> > > *self, void *aux)
> > >   rw_init(>sc_buslock, sc->sc_dev.dv_xname);
> > >  
> > >   sc->sc_started = 0;
> > > + sc->sc_twsien_iflg = CONTROL_TWSIEN | (OF_is_compatible(sc->sc_node,
> > > + "allwinner,sun6i-a31-i2c") ? CONTROL_IFLG : 0);
> > >   sc->sc_ic.ic_cookie = sc;
> > >   sc->sc_ic.ic_acquire_bus = sxitwi_acquire_bus;
> > >   sc->sc_ic.ic_release_bus = sxitwi_release_bus;
> > > @@ -220,6 +224,7 @@ sxitwi_attach(struct device *parent, struct device 
> > > *self, void *aux)
> > >  
> > >   /* Enable clock */
> > >   clock_enable(faa->fa_node, NULL);
> > > + reset_deassert_all(faa->fa_node);
> > >  
> > >   /*
> > >* Set clock rate to 100kHz. From the datasheet:
> > > @@ -358,13 +363,11 @@ sxitwi_send_stop(void *v, int flags)
> > >  {
> > >   struct sxitwi_softc *sc = v;
> > >   int retry = TWSI_RETRY_COUNT;
> > > - u_int control;
> > >  
> > >   sc->sc_started = 0;
> > >  
> > >   /* Interrupt is not generated for STAT_NRS. */
> > > - control = CONTROL_STOP | CONTROL_TWSIEN;
> > > - sxitwi_write_4(sc, TWSI_CONTROL, control);
> > > + sxitwi_write_4(sc, TWSI_CONTROL, CONTROL_STOP | sc->sc_twsien_iflg);
> > >   while (--retry > 

Re: armv7 / H3 i2c gates+resets + quirk for >=sun6i-a31-i2c compat.

[Patrick Wildt]

On Tue, Sep 26, 2017 at 09:48:41AM -0700, Mike Larkin wrote:
> On Tue, Sep 26, 2017 at 03:08:40PM +0300, Artturi Alm wrote:
> > Hi,
> > 
> > diff does add the quirk needed on sun6i-a31, and above including sun8i-h3,
> > for which it does also add related gates+resets.
> > 
> > -Artturi
> > 
> 
> what bug does this fix?

I think the messages he intended to write were:

Add support for Allwinner's H3 I2C clocks, so that we can properly reset
and turn on the I2C controller.

and

The Allwinner H3 SoC uses the same I2C controller as the other Allwinner
SoCs, but there's a single difference:  The IFLG flag is inverted on the
H3, so in that case we must set instead of clear it.

Or something like that.

Patrick

> 
> > 
> > diff --git a/sys/dev/fdt/sxiccmu_clocks.h b/sys/dev/fdt/sxiccmu_clocks.h
> > index 8b25ac42bd4..f82613e0125 100644
> > --- a/sys/dev/fdt/sxiccmu_clocks.h
> > +++ b/sys/dev/fdt/sxiccmu_clocks.h
> > @@ -95,6 +95,10 @@ struct sxiccmu_ccu_bit sun50i_a64_gates[] = {
> >  
> >  #define H3_CLK_BUS_PIO 54
> >  
> > +#define H3_CLK_BUS_I2C059
> > +#define H3_CLK_BUS_I2C160
> > +#define H3_CLK_BUS_I2C261
> > +
> >  #define H3_CLK_BUS_UART0   62
> >  #define H3_CLK_BUS_UART1   63
> >  #define H3_CLK_BUS_UART2   64
> > @@ -128,6 +132,9 @@ struct sxiccmu_ccu_bit sun8i_h3_gates[] = {
> > [H3_CLK_BUS_OHCI2] = { 0x0060, 30 },
> > [H3_CLK_BUS_OHCI3] = { 0x0060, 31 },
> > [H3_CLK_BUS_PIO]   = { 0x0068, 5 },
> > +   [H3_CLK_BUS_I2C0] = { 0x006c, 0, H3_CLK_APB2 },
> > +   [H3_CLK_BUS_I2C1] = { 0x006c, 1, H3_CLK_APB2 },
> > +   [H3_CLK_BUS_I2C2] = { 0x006c, 2, H3_CLK_APB2 },
> > [H3_CLK_BUS_UART0] = { 0x006c, 16, H3_CLK_APB2 },
> > [H3_CLK_BUS_UART1] = { 0x006c, 17, H3_CLK_APB2 },
> > [H3_CLK_BUS_UART2] = { 0x006c, 18, H3_CLK_APB2 },
> > @@ -195,6 +202,10 @@ struct sxiccmu_ccu_bit sun50i_a64_resets[] = {
> >  
> >  #define H3_RST_BUS_EPHY39
> >  
> > +#define H3_RST_BUS_I2C046
> > +#define H3_RST_BUS_I2C147
> > +#define H3_RST_BUS_I2C248
> > +
> >  struct sxiccmu_ccu_bit sun8i_h3_resets[] = {
> > [H3_RST_USB_PHY0] =  { 0x00cc, 0 },
> > [H3_RST_USB_PHY1] =  { 0x00cc, 1 },
> > @@ -213,4 +224,7 @@ struct sxiccmu_ccu_bit sun8i_h3_resets[] = {
> > [H3_RST_BUS_OHCI2] = { 0x02c0, 30 },
> > [H3_RST_BUS_OHCI3] = { 0x02c0, 31 },
> > [H3_RST_BUS_EPHY]  = { 0x02c8, 2 },
> > +   [H3_RST_BUS_I2C0] =  { 0x02d8, 0 },
> > +   [H3_RST_BUS_I2C1] =  { 0x02d8, 1 },
> > +   [H3_RST_BUS_I2C2] =  { 0x02d8, 2 },
> >  };
> > diff --git a/sys/dev/fdt/sxitwi.c b/sys/dev/fdt/sxitwi.c
> > index f53f2bfd594..e98b36f9588 100644
> > --- a/sys/dev/fdt/sxitwi.c
> > +++ b/sys/dev/fdt/sxitwi.c
> > @@ -144,6 +144,7 @@ struct sxitwi_softc {
> > bus_space_handle_t   sc_ioh;
> > int  sc_node;
> > u_intsc_started;
> > +   u_intsc_twsien_iflg;
> > struct i2c_controllersc_ic;
> > struct rwlocksc_buslock;
> > void*sc_ih;
> > @@ -179,7 +180,8 @@ sxitwi_match(struct device *parent, void *match, void 
> > *aux)
> > struct fdt_attach_args *faa = aux;
> >  
> > return (OF_is_compatible(faa->fa_node, "allwinner,sun4i-a10-i2c") ||
> > -   OF_is_compatible(faa->fa_node, "allwinner,sun7i-a20-i2c"));
> > +   OF_is_compatible(faa->fa_node, "allwinner,sun7i-a20-i2c") ||
> > +   OF_is_compatible(faa->fa_node, "allwinner,sun6i-a31-i2c"));
> >  }
> >  
> >  void
> > @@ -206,6 +208,8 @@ sxitwi_attach(struct device *parent, struct device 
> > *self, void *aux)
> > rw_init(>sc_buslock, sc->sc_dev.dv_xname);
> >  
> > sc->sc_started = 0;
> > +   sc->sc_twsien_iflg = CONTROL_TWSIEN | (OF_is_compatible(sc->sc_node,
> > +   "allwinner,sun6i-a31-i2c") ? CONTROL_IFLG : 0);
> > sc->sc_ic.ic_cookie = sc;
> > sc->sc_ic.ic_acquire_bus = sxitwi_acquire_bus;
> > sc->sc_ic.ic_release_bus = sxitwi_release_bus;
> > @@ -220,6 +224,7 @@ sxitwi_attach(struct device *parent, struct device 
> > *self, void *aux)
> >  
> > /* Enable clock */
> > clock_enable(faa->fa_node, NULL);
> > +   reset_deassert_all(faa->fa_node);
> >  
> > /*
> >  * Set clock rate to 100kHz. From the datasheet:
> > @@ -358,13 +363,11 @@ sxitwi_send_stop(void *v, int flags)
> >  {
> > struct sxitwi_softc *sc = v;
> > int retry = TWSI_RETRY_COUNT;
> > -   u_int control;
> >  
> > sc->sc_started = 0;
> >  
> > /* Interrupt is not generated for STAT_NRS. */
> > -   control = CONTROL_STOP | CONTROL_TWSIEN;
> > -   sxitwi_write_4(sc, TWSI_CONTROL, control);
> > +   sxitwi_write_4(sc, TWSI_CONTROL, CONTROL_STOP | sc->sc_twsien_iflg);
> > while (--retry > 0) {
> > if (sxitwi_read_4(sc, TWSI_STATUS) == STAT_NRS)
> > return 0;
> > @@ -458,7 +461,7 @@ sxitwi_wait(struct sxitwi_softc *sc, u_int control, 
> > u_int expect, int flags)
> > delay(5);
> > if 

Re: sin() implementation

[Otto Moerbeek]

On Tue, Sep 26, 2017 at 01:27:52PM +0200, Jan Stary wrote:

> I picked sin() as an example while trying to walk though the implementation
> of (pieces of) libm.  If someone has the time for it, I have some questions.
> 
> I understand the implementation originaly stems from Sun's libm of 1993.
> (As does that of FreeBSD and NetBSD.) It has been tweaked over the years,
> but the code that actually computes the values is mostly untouched.
> 
> s_sin.c normalizes the argument to [-pi/4, +pi/4].
> This is how |x| <= pi/4 is tested:
> 
>   GET_HIGH_WORD(ix,x);
>   ix &= 0x7fff;
>   if(ix <= 0x3fe921fb) return __kernel_sin(x,z,0);
> 
> Why is it done like that? Is it faster or more portable
> or in any way better that comparing the value itself to M_PI_4?
> (Or was it, in 1993?)

The code is written in a way to use as little as fp ops as possible.

> 
> For Inf and NaN arguments, NaN is returned as follows:
> 
>   /* sin(Inf or NaN) is NaN */
>   else if (ix>=0x7ff0) return x-x;
> 
> Again, why the integer conversion? Would
> 
>   else if (isinf(x) || isnan(x))
> 
> be slower? Less portable?

These functions were not generally available when the code was
written. iirc these were only standarized in C99.

> 
> Also, is "return x-x" just a fast/clever way to return NaN
> for x being Inf _or_ NaN, preferable to return nan("")?

Same as above.

-Otto


> 
> I have other questions in k_sin.c and elsewhere, but I'll stop here.
> Please see my naive diff below. I tried this on amd64, macppc and armv7;
> regress/lib/libm/fpaccuracy results in zero difference. The only
> other thing I tried is "sox -n out.raw synth 10 sin" which also gives
> the same result. (That's hardly "testing" of course.)
> 
> Surely I am missing something elementary.
> I'll be glad for any insight.
> 
>   Thank you for your time
> 
>   Jan
> 
> 
> Index: s_sin.c
> ===
> RCS file: /cvs/src/lib/libm/src/s_sin.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 s_sin.c
> --- s_sin.c   12 Sep 2016 19:47:02 -  1.11
> +++ s_sin.c   26 Sep 2017 08:54:03 -
> @@ -50,17 +50,12 @@ double
>  sin(double x)
>  {
>   double y[2],z=0.0;
> - int32_t n, ix;
> + int32_t n;
>  
> -/* High word of x. */
> - GET_HIGH_WORD(ix,x);
> -
> -/* |x| ~< pi/4 */
> - ix &= 0x7fff;
> - if(ix <= 0x3fe921fb) return __kernel_sin(x,z,0);
> -
> -/* sin(Inf or NaN) is NaN */
> - else if (ix>=0x7ff0) return x-x;
> + if (-M_PI_4 < x && x < M_PI_4)
> + return __kernel_sin(x,z,0);
> + else if (isinf(x) || isnan(x))
> + return nan("");
>  
>  /* argument reduction needed */
>   else {

Re: armv7 / H3 i2c gates+resets + quirk for >=sun6i-a31-i2c compat.

[Mike Larkin]

On Tue, Sep 26, 2017 at 03:08:40PM +0300, Artturi Alm wrote:
> Hi,
> 
> diff does add the quirk needed on sun6i-a31, and above including sun8i-h3,
> for which it does also add related gates+resets.
> 
> -Artturi
> 

what bug does this fix?

> 
> diff --git a/sys/dev/fdt/sxiccmu_clocks.h b/sys/dev/fdt/sxiccmu_clocks.h
> index 8b25ac42bd4..f82613e0125 100644
> --- a/sys/dev/fdt/sxiccmu_clocks.h
> +++ b/sys/dev/fdt/sxiccmu_clocks.h
> @@ -95,6 +95,10 @@ struct sxiccmu_ccu_bit sun50i_a64_gates[] = {
>  
>  #define H3_CLK_BUS_PIO   54
>  
> +#define H3_CLK_BUS_I2C0  59
> +#define H3_CLK_BUS_I2C1  60
> +#define H3_CLK_BUS_I2C2  61
> +
>  #define H3_CLK_BUS_UART0 62
>  #define H3_CLK_BUS_UART1 63
>  #define H3_CLK_BUS_UART2 64
> @@ -128,6 +132,9 @@ struct sxiccmu_ccu_bit sun8i_h3_gates[] = {
>   [H3_CLK_BUS_OHCI2] = { 0x0060, 30 },
>   [H3_CLK_BUS_OHCI3] = { 0x0060, 31 },
>   [H3_CLK_BUS_PIO]   = { 0x0068, 5 },
> + [H3_CLK_BUS_I2C0] = { 0x006c, 0, H3_CLK_APB2 },
> + [H3_CLK_BUS_I2C1] = { 0x006c, 1, H3_CLK_APB2 },
> + [H3_CLK_BUS_I2C2] = { 0x006c, 2, H3_CLK_APB2 },
>   [H3_CLK_BUS_UART0] = { 0x006c, 16, H3_CLK_APB2 },
>   [H3_CLK_BUS_UART1] = { 0x006c, 17, H3_CLK_APB2 },
>   [H3_CLK_BUS_UART2] = { 0x006c, 18, H3_CLK_APB2 },
> @@ -195,6 +202,10 @@ struct sxiccmu_ccu_bit sun50i_a64_resets[] = {
>  
>  #define H3_RST_BUS_EPHY  39
>  
> +#define H3_RST_BUS_I2C0  46
> +#define H3_RST_BUS_I2C1  47
> +#define H3_RST_BUS_I2C2  48
> +
>  struct sxiccmu_ccu_bit sun8i_h3_resets[] = {
>   [H3_RST_USB_PHY0] =  { 0x00cc, 0 },
>   [H3_RST_USB_PHY1] =  { 0x00cc, 1 },
> @@ -213,4 +224,7 @@ struct sxiccmu_ccu_bit sun8i_h3_resets[] = {
>   [H3_RST_BUS_OHCI2] = { 0x02c0, 30 },
>   [H3_RST_BUS_OHCI3] = { 0x02c0, 31 },
>   [H3_RST_BUS_EPHY]  = { 0x02c8, 2 },
> + [H3_RST_BUS_I2C0] =  { 0x02d8, 0 },
> + [H3_RST_BUS_I2C1] =  { 0x02d8, 1 },
> + [H3_RST_BUS_I2C2] =  { 0x02d8, 2 },
>  };
> diff --git a/sys/dev/fdt/sxitwi.c b/sys/dev/fdt/sxitwi.c
> index f53f2bfd594..e98b36f9588 100644
> --- a/sys/dev/fdt/sxitwi.c
> +++ b/sys/dev/fdt/sxitwi.c
> @@ -144,6 +144,7 @@ struct sxitwi_softc {
>   bus_space_handle_t   sc_ioh;
>   int  sc_node;
>   u_intsc_started;
> + u_intsc_twsien_iflg;
>   struct i2c_controllersc_ic;
>   struct rwlocksc_buslock;
>   void*sc_ih;
> @@ -179,7 +180,8 @@ sxitwi_match(struct device *parent, void *match, void 
> *aux)
>   struct fdt_attach_args *faa = aux;
>  
>   return (OF_is_compatible(faa->fa_node, "allwinner,sun4i-a10-i2c") ||
> - OF_is_compatible(faa->fa_node, "allwinner,sun7i-a20-i2c"));
> + OF_is_compatible(faa->fa_node, "allwinner,sun7i-a20-i2c") ||
> + OF_is_compatible(faa->fa_node, "allwinner,sun6i-a31-i2c"));
>  }
>  
>  void
> @@ -206,6 +208,8 @@ sxitwi_attach(struct device *parent, struct device *self, 
> void *aux)
>   rw_init(>sc_buslock, sc->sc_dev.dv_xname);
>  
>   sc->sc_started = 0;
> + sc->sc_twsien_iflg = CONTROL_TWSIEN | (OF_is_compatible(sc->sc_node,
> + "allwinner,sun6i-a31-i2c") ? CONTROL_IFLG : 0);
>   sc->sc_ic.ic_cookie = sc;
>   sc->sc_ic.ic_acquire_bus = sxitwi_acquire_bus;
>   sc->sc_ic.ic_release_bus = sxitwi_release_bus;
> @@ -220,6 +224,7 @@ sxitwi_attach(struct device *parent, struct device *self, 
> void *aux)
>  
>   /* Enable clock */
>   clock_enable(faa->fa_node, NULL);
> + reset_deassert_all(faa->fa_node);
>  
>   /*
>* Set clock rate to 100kHz. From the datasheet:
> @@ -358,13 +363,11 @@ sxitwi_send_stop(void *v, int flags)
>  {
>   struct sxitwi_softc *sc = v;
>   int retry = TWSI_RETRY_COUNT;
> - u_int control;
>  
>   sc->sc_started = 0;
>  
>   /* Interrupt is not generated for STAT_NRS. */
> - control = CONTROL_STOP | CONTROL_TWSIEN;
> - sxitwi_write_4(sc, TWSI_CONTROL, control);
> + sxitwi_write_4(sc, TWSI_CONTROL, CONTROL_STOP | sc->sc_twsien_iflg);
>   while (--retry > 0) {
>   if (sxitwi_read_4(sc, TWSI_STATUS) == STAT_NRS)
>   return 0;
> @@ -458,7 +461,7 @@ sxitwi_wait(struct sxitwi_softc *sc, u_int control, u_int 
> expect, int flags)
>   delay(5);
>   if (!(flags & I2C_F_POLL))
>   control |= CONTROL_INTEN;
> - sxitwi_write_4(sc, TWSI_CONTROL, control | CONTROL_TWSIEN);
> + sxitwi_write_4(sc, TWSI_CONTROL, control | sc->sc_twsien_iflg);
>  
>   timo = 0;
>   do {
> 

Re: sin() implementation

[Otto Moerbeek]

On Tue, Sep 26, 2017 at 06:21:16PM +0200, Jan Stary wrote:

> These (diff below) seem to be obvious typos in k_sin.c,
> but only in comments. The comment that says
> 
>   if x < 2^-27 (hx<0x3e40), return x with inexact if x!=0
> 
> also puzzles me a bit: what the code does is
> 
>   GET_HIGH_WORD(ix,x);
>   ix &= 0x7fff;   /* high word of x */
>   if(ix<0x3e40)   /* |x| < 2**-27 */
>   {if((int)x==0) return x;}   /* generate inexact */
> 
> If the high word of a double x is less than 0x3e40,
> how could (int)x be anything else than zero?

The result might be zero, but you want the inexact flag to be set in
those cases. I think case 2 describes it correctly: only return 0 with
the inexact flag not set if x equals zero.

-Otto

> 
> The "if x!= 0" also puzzles me: the sine of zero _is_ zero, exactly.
> What would change with this:
> 
>   if (ix < 0x3e40)
>   return x;
> 
> What is the role of the iy indicator?
> Is it easier/faster to test the integer indicator for iy == 0
> than it would be to test y == 0 itself? Or is there another reason?
> 
>  Jan
> 
> 
> Index: k_sin.c
> ===
> RCS file: /cvs/src/lib/libm/src/k_sin.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 k_sin.c
> --- k_sin.c   27 Oct 2009 23:59:30 -  1.3
> +++ k_sin.c   26 Sep 2017 16:07:45 -
> @@ -10,15 +10,15 @@
>   * 
>   */
>  
> -/* __kernel_sin( x, y, iy)
> +/* __kernel_sin(x, y, iy)
>   * kernel sin function on [-pi/4, pi/4], pi/4 ~ 0.7854
>   * Input x is assumed to be bounded by ~pi/4 in magnitude.
>   * Input y is the tail of x.
> - * Input iy indicates whether y is 0. (if iy=0, y assume to be 0). 
> + * Input iy indicates whether y is 0. (if iy=0, assume y to be 0). 
>   *
>   * Algorithm
>   *   1. Since sin(-x) = -sin(x), we need only to consider positive x. 
> - *   2. if x < 2^-27 (hx<0x3e40 0), return x with inexact if x!=0.
> + *   2. if x < 2^-27 (hx<0x3e40), return x with inexact if x!=0.
>   *   3. sin(x) is approximated by a polynomial of degree 13 on
>   *  [0,pi/4]
>   *313

Re: sin() implementation

[Jan Stary]

On Sep 26 17:41:17, h...@stare.cz wrote:
> > > s_sin.c normalizes the argument to [-pi/4, +pi/4].
> > > This is how |x| <= pi/4 is tested:
> > > 
> > >   GET_HIGH_WORD(ix,x);
> > >   ix &= 0x7fff;
> > >   if(ix <= 0x3fe921fb) return __kernel_sin(x,z,0);
> > > 
> > > Why is it done like that? Is it faster or more portable
> > > or in any way better that comparing the value itself to M_PI_4?
> > > (Or was it, in 1993?)
> > 
> > Hm, is 0x3fe921fb the nearest float <= pi/4?
> > s_sinf.c uses 0x3f490fd8 for that test.
> 
> Yes.
> 
> #include 
> #include 
> #include "math_private.h"
> 
> int
> main()
> {
>   double d = M_PI_4;
>   double f = M_PI_4;

float f, sorry.

>   int32_t i, j;
> 
>   GET_HIGH_WORD(i, d);
>   GET_FLOAT_WORD(j, f);
> 
>   printf("double %f, high  word %#x\n", d, i);
>   printf("float  %f, float word %#x\n", f, j);
> 
>   return 0;
> }
> 
> This says
> 
>   double 0.785398, high  word 0x3fe921fb
>   float  0.785398, float word 0x3f490fdb
> 
> In case of double, it's exactly 0x3fe921fb.
> But why then does s_sinf.c use 0x3f490fd8 instead of 0x3f490fdb?
> 
>   Jan
> 

Re: sin() implementation

[Jan Stary]

These (diff below) seem to be obvious typos in k_sin.c,
but only in comments. The comment that says

if x < 2^-27 (hx<0x3e40), return x with inexact if x!=0

also puzzles me a bit: what the code does is

GET_HIGH_WORD(ix,x);
ix &= 0x7fff;   /* high word of x */
if(ix<0x3e40)   /* |x| < 2**-27 */
{if((int)x==0) return x;}   /* generate inexact */

If the high word of a double x is less than 0x3e40,
how could (int)x be anything else than zero?

The "if x!= 0" also puzzles me: the sine of zero _is_ zero, exactly.
What would change with this:

if (ix < 0x3e40)
return x;

What is the role of the iy indicator?
Is it easier/faster to test the integer indicator for iy == 0
than it would be to test y == 0 itself? Or is there another reason?

 Jan


Index: k_sin.c
===
RCS file: /cvs/src/lib/libm/src/k_sin.c,v
retrieving revision 1.3
diff -u -p -r1.3 k_sin.c
--- k_sin.c 27 Oct 2009 23:59:30 -  1.3
+++ k_sin.c 26 Sep 2017 16:07:45 -
@@ -10,15 +10,15 @@
  * 
  */
 
-/* __kernel_sin( x, y, iy)
+/* __kernel_sin(x, y, iy)
  * kernel sin function on [-pi/4, pi/4], pi/4 ~ 0.7854
  * Input x is assumed to be bounded by ~pi/4 in magnitude.
  * Input y is the tail of x.
- * Input iy indicates whether y is 0. (if iy=0, y assume to be 0). 
+ * Input iy indicates whether y is 0. (if iy=0, assume y to be 0). 
  *
  * Algorithm
  * 1. Since sin(-x) = -sin(x), we need only to consider positive x. 
- * 2. if x < 2^-27 (hx<0x3e40 0), return x with inexact if x!=0.
+ * 2. if x < 2^-27 (hx<0x3e40), return x with inexact if x!=0.
  * 3. sin(x) is approximated by a polynomial of degree 13 on
  *[0,pi/4]
  *  313

Re: sin() implementation

[Jan Stary]

> > s_sin.c normalizes the argument to [-pi/4, +pi/4].
> > This is how |x| <= pi/4 is tested:
> > 
> > GET_HIGH_WORD(ix,x);
> > ix &= 0x7fff;
> > if(ix <= 0x3fe921fb) return __kernel_sin(x,z,0);
> > 
> > Why is it done like that? Is it faster or more portable
> > or in any way better that comparing the value itself to M_PI_4?
> > (Or was it, in 1993?)
> 
> Hm, is 0x3fe921fb the nearest float <= pi/4?
> s_sinf.c uses 0x3f490fd8 for that test.

Yes.

#include 
#include 
#include "math_private.h"

int
main()
{
double d = M_PI_4;
double f = M_PI_4;
int32_t i, j;

GET_HIGH_WORD(i, d);
GET_FLOAT_WORD(j, f);

printf("double %f, high  word %#x\n", d, i);
printf("float  %f, float word %#x\n", f, j);

return 0;
}

This says

  double 0.785398, high  word 0x3fe921fb
  float  0.785398, float word 0x3f490fdb

In case of double, it's exactly 0x3fe921fb.
But why then does s_sinf.c use 0x3f490fd8 instead of 0x3f490fdb?

Jan

Re: sin() implementation

[Jan Stary]

On Sep 26 13:27:52, h...@stare.cz wrote:
> I picked sin() as an example while trying to walk though the implementation
> of (pieces of) libm.  If someone has the time for it, I have some questions.
> 
> I understand the implementation originaly stems from Sun's libm of 1993.
> (As does that of FreeBSD and NetBSD.)

And Android https://android.googlesource.com/platform/external/fdlibm/
and Julia http://openlibm.org/, apparently.

> It has been tweaked over the years,
> but the code that actually computes the values is mostly untouched.
> 
> s_sin.c normalizes the argument to [-pi/4, +pi/4].
> This is how |x| <= pi/4 is tested:
> 
>   GET_HIGH_WORD(ix,x);
>   ix &= 0x7fff;
>   if(ix <= 0x3fe921fb) return __kernel_sin(x,z,0);
> 
> Why is it done like that? Is it faster or more portable
> or in any way better that comparing the value itself to M_PI_4?
> (Or was it, in 1993?)

Hm, is 0x3fe921fb the nearest float <= pi/4?
s_sinf.c uses 0x3f490fd8 for that test.


Jan

> For Inf and NaN arguments, NaN is returned as follows:
> 
>   /* sin(Inf or NaN) is NaN */
>   else if (ix>=0x7ff0) return x-x;
> 
> Again, why the integer conversion? Would
> 
>   else if (isinf(x) || isnan(x))
> 
> be slower? Less portable?
> 
> Also, is "return x-x" just a fast/clever way to return NaN
> for x being Inf _or_ NaN, preferable to return nan("")?
> 
> I have other questions in k_sin.c and elsewhere, but I'll stop here.
> Please see my naive diff below. I tried this on amd64, macppc and armv7;
> regress/lib/libm/fpaccuracy results in zero difference. The only
> other thing I tried is "sox -n out.raw synth 10 sin" which also gives
> the same result. (That's hardly "testing" of course.)
> 
> Surely I am missing something elementary.
> I'll be glad for any insight.
> 
>   Thank you for your time
> 
>   Jan
> 
> 
> Index: s_sin.c
> ===
> RCS file: /cvs/src/lib/libm/src/s_sin.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 s_sin.c
> --- s_sin.c   12 Sep 2016 19:47:02 -  1.11
> +++ s_sin.c   26 Sep 2017 08:54:03 -
> @@ -50,17 +50,12 @@ double
>  sin(double x)
>  {
>   double y[2],z=0.0;
> - int32_t n, ix;
> + int32_t n;
>  
> -/* High word of x. */
> - GET_HIGH_WORD(ix,x);
> -
> -/* |x| ~< pi/4 */
> - ix &= 0x7fff;
> - if(ix <= 0x3fe921fb) return __kernel_sin(x,z,0);
> -
> -/* sin(Inf or NaN) is NaN */
> - else if (ix>=0x7ff0) return x-x;
> + if (-M_PI_4 < x && x < M_PI_4)
> + return __kernel_sin(x,z,0);
> + else if (isinf(x) || isnan(x))
> + return nan("");
>  
>  /* argument reduction needed */
>   else {
> 

armv7 / H3 i2c gates+resets + quirk for >=sun6i-a31-i2c compat.

[Artturi Alm]

Hi,

diff does add the quirk needed on sun6i-a31, and above including sun8i-h3,
for which it does also add related gates+resets.

-Artturi


diff --git a/sys/dev/fdt/sxiccmu_clocks.h b/sys/dev/fdt/sxiccmu_clocks.h
index 8b25ac42bd4..f82613e0125 100644
--- a/sys/dev/fdt/sxiccmu_clocks.h
+++ b/sys/dev/fdt/sxiccmu_clocks.h
@@ -95,6 +95,10 @@ struct sxiccmu_ccu_bit sun50i_a64_gates[] = {
 
 #define H3_CLK_BUS_PIO 54
 
+#define H3_CLK_BUS_I2C059
+#define H3_CLK_BUS_I2C160
+#define H3_CLK_BUS_I2C261
+
 #define H3_CLK_BUS_UART0   62
 #define H3_CLK_BUS_UART1   63
 #define H3_CLK_BUS_UART2   64
@@ -128,6 +132,9 @@ struct sxiccmu_ccu_bit sun8i_h3_gates[] = {
[H3_CLK_BUS_OHCI2] = { 0x0060, 30 },
[H3_CLK_BUS_OHCI3] = { 0x0060, 31 },
[H3_CLK_BUS_PIO]   = { 0x0068, 5 },
+   [H3_CLK_BUS_I2C0] = { 0x006c, 0, H3_CLK_APB2 },
+   [H3_CLK_BUS_I2C1] = { 0x006c, 1, H3_CLK_APB2 },
+   [H3_CLK_BUS_I2C2] = { 0x006c, 2, H3_CLK_APB2 },
[H3_CLK_BUS_UART0] = { 0x006c, 16, H3_CLK_APB2 },
[H3_CLK_BUS_UART1] = { 0x006c, 17, H3_CLK_APB2 },
[H3_CLK_BUS_UART2] = { 0x006c, 18, H3_CLK_APB2 },
@@ -195,6 +202,10 @@ struct sxiccmu_ccu_bit sun50i_a64_resets[] = {
 
 #define H3_RST_BUS_EPHY39
 
+#define H3_RST_BUS_I2C046
+#define H3_RST_BUS_I2C147
+#define H3_RST_BUS_I2C248
+
 struct sxiccmu_ccu_bit sun8i_h3_resets[] = {
[H3_RST_USB_PHY0] =  { 0x00cc, 0 },
[H3_RST_USB_PHY1] =  { 0x00cc, 1 },
@@ -213,4 +224,7 @@ struct sxiccmu_ccu_bit sun8i_h3_resets[] = {
[H3_RST_BUS_OHCI2] = { 0x02c0, 30 },
[H3_RST_BUS_OHCI3] = { 0x02c0, 31 },
[H3_RST_BUS_EPHY]  = { 0x02c8, 2 },
+   [H3_RST_BUS_I2C0] =  { 0x02d8, 0 },
+   [H3_RST_BUS_I2C1] =  { 0x02d8, 1 },
+   [H3_RST_BUS_I2C2] =  { 0x02d8, 2 },
 };
diff --git a/sys/dev/fdt/sxitwi.c b/sys/dev/fdt/sxitwi.c
index f53f2bfd594..e98b36f9588 100644
--- a/sys/dev/fdt/sxitwi.c
+++ b/sys/dev/fdt/sxitwi.c
@@ -144,6 +144,7 @@ struct sxitwi_softc {
bus_space_handle_t   sc_ioh;
int  sc_node;
u_intsc_started;
+   u_intsc_twsien_iflg;
struct i2c_controllersc_ic;
struct rwlocksc_buslock;
void*sc_ih;
@@ -179,7 +180,8 @@ sxitwi_match(struct device *parent, void *match, void *aux)
struct fdt_attach_args *faa = aux;
 
return (OF_is_compatible(faa->fa_node, "allwinner,sun4i-a10-i2c") ||
-   OF_is_compatible(faa->fa_node, "allwinner,sun7i-a20-i2c"));
+   OF_is_compatible(faa->fa_node, "allwinner,sun7i-a20-i2c") ||
+   OF_is_compatible(faa->fa_node, "allwinner,sun6i-a31-i2c"));
 }
 
 void
@@ -206,6 +208,8 @@ sxitwi_attach(struct device *parent, struct device *self, 
void *aux)
rw_init(>sc_buslock, sc->sc_dev.dv_xname);
 
sc->sc_started = 0;
+   sc->sc_twsien_iflg = CONTROL_TWSIEN | (OF_is_compatible(sc->sc_node,
+   "allwinner,sun6i-a31-i2c") ? CONTROL_IFLG : 0);
sc->sc_ic.ic_cookie = sc;
sc->sc_ic.ic_acquire_bus = sxitwi_acquire_bus;
sc->sc_ic.ic_release_bus = sxitwi_release_bus;
@@ -220,6 +224,7 @@ sxitwi_attach(struct device *parent, struct device *self, 
void *aux)
 
/* Enable clock */
clock_enable(faa->fa_node, NULL);
+   reset_deassert_all(faa->fa_node);
 
/*
 * Set clock rate to 100kHz. From the datasheet:
@@ -358,13 +363,11 @@ sxitwi_send_stop(void *v, int flags)
 {
struct sxitwi_softc *sc = v;
int retry = TWSI_RETRY_COUNT;
-   u_int control;
 
sc->sc_started = 0;
 
/* Interrupt is not generated for STAT_NRS. */
-   control = CONTROL_STOP | CONTROL_TWSIEN;
-   sxitwi_write_4(sc, TWSI_CONTROL, control);
+   sxitwi_write_4(sc, TWSI_CONTROL, CONTROL_STOP | sc->sc_twsien_iflg);
while (--retry > 0) {
if (sxitwi_read_4(sc, TWSI_STATUS) == STAT_NRS)
return 0;
@@ -458,7 +461,7 @@ sxitwi_wait(struct sxitwi_softc *sc, u_int control, u_int 
expect, int flags)
delay(5);
if (!(flags & I2C_F_POLL))
control |= CONTROL_INTEN;
-   sxitwi_write_4(sc, TWSI_CONTROL, control | CONTROL_TWSIEN);
+   sxitwi_write_4(sc, TWSI_CONTROL, control | sc->sc_twsien_iflg);
 
timo = 0;
do {

sin() implementation

[Jan Stary]

I picked sin() as an example while trying to walk though the implementation
of (pieces of) libm.  If someone has the time for it, I have some questions.

I understand the implementation originaly stems from Sun's libm of 1993.
(As does that of FreeBSD and NetBSD.) It has been tweaked over the years,
but the code that actually computes the values is mostly untouched.

s_sin.c normalizes the argument to [-pi/4, +pi/4].
This is how |x| <= pi/4 is tested:

GET_HIGH_WORD(ix,x);
ix &= 0x7fff;
if(ix <= 0x3fe921fb) return __kernel_sin(x,z,0);

Why is it done like that? Is it faster or more portable
or in any way better that comparing the value itself to M_PI_4?
(Or was it, in 1993?)

For Inf and NaN arguments, NaN is returned as follows:

/* sin(Inf or NaN) is NaN */
else if (ix>=0x7ff0) return x-x;

Again, why the integer conversion? Would

else if (isinf(x) || isnan(x))

be slower? Less portable?

Also, is "return x-x" just a fast/clever way to return NaN
for x being Inf _or_ NaN, preferable to return nan("")?

I have other questions in k_sin.c and elsewhere, but I'll stop here.
Please see my naive diff below. I tried this on amd64, macppc and armv7;
regress/lib/libm/fpaccuracy results in zero difference. The only
other thing I tried is "sox -n out.raw synth 10 sin" which also gives
the same result. (That's hardly "testing" of course.)

Surely I am missing something elementary.
I'll be glad for any insight.

Thank you for your time

Jan


Index: s_sin.c
===
RCS file: /cvs/src/lib/libm/src/s_sin.c,v
retrieving revision 1.11
diff -u -p -r1.11 s_sin.c
--- s_sin.c 12 Sep 2016 19:47:02 -  1.11
+++ s_sin.c 26 Sep 2017 08:54:03 -
@@ -50,17 +50,12 @@ double
 sin(double x)
 {
double y[2],z=0.0;
-   int32_t n, ix;
+   int32_t n;
 
-/* High word of x. */
-   GET_HIGH_WORD(ix,x);
-
-/* |x| ~< pi/4 */
-   ix &= 0x7fff;
-   if(ix <= 0x3fe921fb) return __kernel_sin(x,z,0);
-
-/* sin(Inf or NaN) is NaN */
-   else if (ix>=0x7ff0) return x-x;
+   if (-M_PI_4 < x && x < M_PI_4)
+   return __kernel_sin(x,z,0);
+   else if (isinf(x) || isnan(x))
+   return nan("");
 
 /* argument reduction needed */
else {

Re: pfctl always prints warning when flushes ruleset

[Alexandr Nedvedicky]

Hello Mike,

> 
> Please make sure a pfctl regress doesn't run into issues with this
> diff.  Otherwise OK mikeb.
> 

the regress/src/sbin/pfctl did not spot any issues. will commit my
change later today.


thanks a lot
regards
sasha

Re: pfctl can do a better job when handling ioctl(2) errors

[Alexandr Nedvedicky]

Hello Mike,

thanks for looking at it.
> 
> Can you please reverse the check so that adding conditions is possible
> and the default is in the 'else' branch, i.e.
> 
>   if (errno == EINVAL)
>   errx(1, "Anchor '%s' not found.\n", anchorname);
>   else
>   err(1, "pfctl_clear_rules");

good point, makes sense

> 
> uipc_mbuf.c are clearly unrelated, but what about the chunk below?
yes uipc_mbuf.c is clear martian (I've forgot to update my branch).
> 
> > @@ -850,7 +860,7 @@ pfctl_show_rules(int dev, char *path, in
> >  * to the kernel.
> >  */
> > if ((p = strrchr(anchorname, '/')) != NULL &&
> > -   p[1] == '*' && p[2] == '\0') {
> > +   ((p[1] == '*' && p[2] == '\0') || (p[1] == '\0'))) {
> > p[0] = '\0';
> > }
> >  
> 
> I think this requires an explanation.

The change above belongs to other patch I hope to send. The other patch
sanitizes anchor name. For example names:
Foo/*
Foo/
will be treated as Foo consistently. Let's save the topic for another
email thread.

below is updated patch. The list of changes is as follows:

- removed uipc_mbuf.c changes
- removed anchorname sanitization at pfctl_show_rules()
- inverting the if (... = EINVAL) test as suggested by Mike
- using warnx() in favor of fprintf(stderr, ...), it makes my
  change consistent with existing code.

thanks and
regards
sasha

8<---8<---8<--8<
diff -r 215db23c6b05 src/sbin/pfctl/pfctl.c
--- src/sbin/pfctl/pfctl.c  Mon Sep 25 13:38:48 2017 +0200
+++ src/sbin/pfctl/pfctl.c  Tue Sep 26 13:01:48 2017 +0200
@@ -318,13 +318,23 @@ void
 pfctl_clear_rules(int dev, int opts, char *anchorname)
 {
struct pfr_buffer t;
+   char*p;
+
+   p = strrchr(anchorname, '/');
+   if (p != NULL && p[1] == '\0')
+   errx(1, "%s: bad anchor name %s", __func__, anchorname);
 
memset(, 0, sizeof(t));
t.pfrb_type = PFRB_TRANS;
+
if (pfctl_add_trans(, PF_TRANS_RULESET, anchorname) ||
pfctl_trans(dev, , DIOCXBEGIN, 0) ||
-   pfctl_trans(dev, , DIOCXCOMMIT, 0))
-   err(1, "pfctl_clear_rules");
+   pfctl_trans(dev, , DIOCXCOMMIT, 0)) {
+   if (errno == EINVAL)
+   errx(1, "Anchor '%s' not found.", anchorname);
+   else
+   err(1, "pfctl_clear_rules");
+   }
if ((opts & PF_OPT_QUIET) == 0)
fprintf(stderr, "rules cleared\n");
 }
@@ -871,7 +881,10 @@ pfctl_show_rules(int dev, char *path, in
if (opts & PF_OPT_SHOWALL) {
pr.rule.action = PF_PASS;
if (ioctl(dev, DIOCGETRULES, )) {
-   warn("DIOCGETRULES");
+   if (errno == EINVAL)
+   warnx("Anchor '%s' not found.", anchorname);
+   else
+   warn("DIOCGETRULES");
ret = -1;
goto error;
}
@@ -886,7 +899,10 @@ pfctl_show_rules(int dev, char *path, in
 
pr.rule.action = PF_PASS;
if (ioctl(dev, DIOCGETRULES, )) {
-   warn("DIOCGETRULES");
+   if (errno == EINVAL)
+   warnx("Anchor '%s' not found.", anchorname);
+   else
+   warn("DIOCGETRULES");
ret = -1;
goto error;
}
8<---8<---8<--8<

Re: pfctl can do a better job when handling ioctl(2) errors

[Mike Belopuhov]

On Tue, Sep 26, 2017 at 11:37 +0200, Alexandr Nedvedicky wrote:
> Hello,
> 
> whenever administrator asks pfctl to modify/remove anchor, which does not
> exist, the pfctl(8) prints warning 'pfctl: DIOCGETRULES: Invalid argument'.
> Few users on Solaris wants pfctl(8) to be more helpful.
> 
> The 'Invalid Argument' (EINVAL) is returned when particular anchor is not
> found. Patch below changes pfctl output to this:
> 
> # pfctl -sA
> # pfctl -a Foo -sr 
> Anchor 'Foo' not found.
> # 
> 
> OK?
>

Can you please reverse the check so that adding conditions is possible
and the default is in the 'else' branch, i.e.

if (errno == EINVAL)
errx(1, "Anchor '%s' not found.\n", anchorname);
else
err(1, "pfctl_clear_rules");

uipc_mbuf.c are clearly unrelated, but what about the chunk below?

> @@ -850,7 +860,7 @@ pfctl_show_rules(int dev, char *path, in
>* to the kernel.
>*/
>   if ((p = strrchr(anchorname, '/')) != NULL &&
> - p[1] == '*' && p[2] == '\0') {
> + ((p[1] == '*' && p[2] == '\0') || (p[1] == '\0'))) {
>   p[0] = '\0';
>   }
>  

I think this requires an explanation.

> thanks and
> regards
> sasha
> 
> 8<---8<---8<--8<
> diff -r 215db23c6b05 src/sbin/pfctl/pfctl.c
> --- src/sbin/pfctl/pfctl.cMon Sep 25 13:38:48 2017 +0200
> +++ src/sbin/pfctl/pfctl.cTue Sep 26 11:36:50 2017 +0200
> @@ -318,13 +318,23 @@ void
>  pfctl_clear_rules(int dev, int opts, char *anchorname)
>  {
>   struct pfr_buffer t;
> + char*p;
> +
> + p = strrchr(anchorname, '/');
> + if (p != NULL && p[1] == '\0')
> + errx(1, "%s: bad anchor name %s", __func__, anchorname);
>  
>   memset(, 0, sizeof(t));
>   t.pfrb_type = PFRB_TRANS;
> +
>   if (pfctl_add_trans(, PF_TRANS_RULESET, anchorname) ||
>   pfctl_trans(dev, , DIOCXBEGIN, 0) ||
> - pfctl_trans(dev, , DIOCXCOMMIT, 0))
> - err(1, "pfctl_clear_rules");
> + pfctl_trans(dev, , DIOCXCOMMIT, 0)) {
> + if (errno != EINVAL)
> + err(1, "pfctl_clear_rules");
> + else
> + errx(1, "Anchor '%s' not found.\n", anchorname);
> + }
>   if ((opts & PF_OPT_QUIET) == 0)
>   fprintf(stderr, "rules cleared\n");
>  }
> @@ -850,7 +860,7 @@ pfctl_show_rules(int dev, char *path, in
>* to the kernel.
>*/
>   if ((p = strrchr(anchorname, '/')) != NULL &&
> - p[1] == '*' && p[2] == '\0') {
> + ((p[1] == '*' && p[2] == '\0') || (p[1] == '\0'))) {
>   p[0] = '\0';
>   }
>  
> @@ -871,7 +881,11 @@ pfctl_show_rules(int dev, char *path, in
>   if (opts & PF_OPT_SHOWALL) {
>   pr.rule.action = PF_PASS;
>   if (ioctl(dev, DIOCGETRULES, )) {
> - warn("DIOCGETRULES");
> + if (errno != EINVAL)
> + warn("DIOCGETRULES");
> + else
> + fprintf(stderr, "Anchor '%s' not found.\n",
> + anchorname);
>   ret = -1;
>   goto error;
>   }
> @@ -886,7 +900,10 @@ pfctl_show_rules(int dev, char *path, in
>  
>   pr.rule.action = PF_PASS;
>   if (ioctl(dev, DIOCGETRULES, )) {
> - warn("DIOCGETRULES");
> + if (errno != EINVAL)
> + warn("DIOCGETRULES");
> + else
> + fprintf(stderr, "Anchor '%s' not found.\n", anchorname);
>   ret = -1;
>   goto error;
>   }
> diff -r 215db23c6b05 src/sys/kern/uipc_mbuf.c
> --- src/sys/kern/uipc_mbuf.c  Mon Sep 25 13:38:48 2017 +0200
> +++ src/sys/kern/uipc_mbuf.c  Tue Sep 26 11:36:50 2017 +0200
> @@ -1,4 +1,4 @@
> -/*   $OpenBSD: uipc_mbuf.c,v 1.249 2017/09/15 18:13:05 bluhm Exp $   */
> +/*   $OpenBSD: uipc_mbuf.c,v 1.248 2017/05/27 16:41:10 bluhm Exp $   */
>  /*   $NetBSD: uipc_mbuf.c,v 1.15.4.1 1996/06/13 17:11:44 cgd Exp $   */
>  
>  /*
> @@ -804,13 +804,12 @@ m_adj(struct mbuf *mp, int req_len)
>   struct mbuf *m;
>   int count;
>  
> - if (mp == NULL)
> + if ((m = mp) == NULL)
>   return;
>   if (len >= 0) {
>   /*
>* Trim from head.
>*/
> - m = mp;
>   while (m != NULL && len > 0) {
>   if (m->m_len <= len) {
>   len -= m->m_len;
> @@ -834,7 +833,6 @@ m_adj(struct mbuf *mp, int req_len)
>*/
>   len = -len;
>   count = 0;
> - m = mp;
>   for (;;) {
>   count += m->m_len;
>   if (m->m_next == NULL)
> @@ -855,16 +853,15 @@ m_adj(struct mbuf *mp, int req_len)
>* Find the mbuf 

Re: pfctl always prints warning when flushes ruleset

[Mike Belopuhov]

On Tue, Sep 26, 2017 at 11:15 +0200, Alexandr Nedvedicky wrote:
> Hello,
> 
> few users on Solaris don't like to read warning 'Anchor or Ruleset' does not
> exist:
> 
> # echo 'pass' |pfctl -a foo -f -
> # pfctl -a foo -Fa
> rules cleared
> pfctl: Anchor or Ruleset does not exist.
> # 
> 
> the commands above did work well, the 'pfctl: Anchor ...' warning message
> is kind of invalid. The code path, which ends up with warning, starts in
> pfct main() function:
> 
> 2518 case 'a':
> 2519 pfctl_clear_rules(dev, opts, anchorname);
> 2520 pfctl_clear_tables(anchorname, opts);
> 2521 if (ifaceopt && *ifaceopt) {
> 2522 warnx("don't specify an interface with 
> -Fall");
> 2523 usage();
> 2524 /* NOTREACHED */
> 2525 }
> 
> we call pfctl_clear_rules(), which flushes all rules from anchor. The anchor
> becomes empty. If there are no tables attached to anchor, then
> pf_remove_if_empty_ruleset() called on behalf of DIOCXCOMMIT also removes the
> anchor from table. No wonder the pfctl_clear_tables() invoked later at line 
> 2520
> can not find anchor, which it is searching table to be flushed.
> 
> The patch below just swaps line 2519 and 2520 to put the operations to correct
> order. With patch in place the warning is gone.
> 
> # echo 'pass' |pfctl -a foo -f -
> # pfctl -a foo -Fa
> 0 tables deleted.
> rules cleared
> # 
> 
> Also pfctl still prints warning in expected case:
> # pfctl -sA
> # pfctl -a foo -FT
> pfctl: Anchor or Ruleset does not exist.
> #
> 
> OK?
>

Please make sure a pfctl regress doesn't run into issues with this
diff.  Otherwise OK mikeb.

> thanks and
> regards
> sasha
> 
> 8<---8<---8<--8<
> diff -r 215db23c6b05 src/sbin/pfctl/pfctl.c
> --- src/sbin/pfctl/pfctl.c  Mon Sep 25 13:38:48 2017 +0200
> +++ src/sbin/pfctl/pfctl.c  Tue Sep 26 11:15:26 2017 +0200
> @@ -2516,8 +2516,8 @@ main(int argc, char *argv[])
> pfctl_clear_stats(dev, ifaceopt, opts);
> break;
> case 'a':
> +   pfctl_clear_tables(anchorname, opts);
> pfctl_clear_rules(dev, opts, anchorname);
> -   pfctl_clear_tables(anchorname, opts);
> if (ifaceopt && *ifaceopt) {
> warnx("don't specify an interface with 
> -Fall");
> usage();
> 8<---8<---8<--8<
> 
> 
> 
> 
> 

pfctl can do a better job when handling ioctl(2) errors

[Alexandr Nedvedicky]

Hello,

whenever administrator asks pfctl to modify/remove anchor, which does not
exist, the pfctl(8) prints warning 'pfctl: DIOCGETRULES: Invalid argument'.
Few users on Solaris wants pfctl(8) to be more helpful.

The 'Invalid Argument' (EINVAL) is returned when particular anchor is not
found. Patch below changes pfctl output to this:

# pfctl -sA
# pfctl -a Foo -sr 
Anchor 'Foo' not found.
# 

OK?

thanks and
regards
sasha

8<---8<---8<--8<
diff -r 215db23c6b05 src/sbin/pfctl/pfctl.c
--- src/sbin/pfctl/pfctl.c  Mon Sep 25 13:38:48 2017 +0200
+++ src/sbin/pfctl/pfctl.c  Tue Sep 26 11:36:50 2017 +0200
@@ -318,13 +318,23 @@ void
 pfctl_clear_rules(int dev, int opts, char *anchorname)
 {
struct pfr_buffer t;
+   char*p;
+
+   p = strrchr(anchorname, '/');
+   if (p != NULL && p[1] == '\0')
+   errx(1, "%s: bad anchor name %s", __func__, anchorname);
 
memset(, 0, sizeof(t));
t.pfrb_type = PFRB_TRANS;
+
if (pfctl_add_trans(, PF_TRANS_RULESET, anchorname) ||
pfctl_trans(dev, , DIOCXBEGIN, 0) ||
-   pfctl_trans(dev, , DIOCXCOMMIT, 0))
-   err(1, "pfctl_clear_rules");
+   pfctl_trans(dev, , DIOCXCOMMIT, 0)) {
+   if (errno != EINVAL)
+   err(1, "pfctl_clear_rules");
+   else
+   errx(1, "Anchor '%s' not found.\n", anchorname);
+   }
if ((opts & PF_OPT_QUIET) == 0)
fprintf(stderr, "rules cleared\n");
 }
@@ -850,7 +860,7 @@ pfctl_show_rules(int dev, char *path, in
 * to the kernel.
 */
if ((p = strrchr(anchorname, '/')) != NULL &&
-   p[1] == '*' && p[2] == '\0') {
+   ((p[1] == '*' && p[2] == '\0') || (p[1] == '\0'))) {
p[0] = '\0';
}
 
@@ -871,7 +881,11 @@ pfctl_show_rules(int dev, char *path, in
if (opts & PF_OPT_SHOWALL) {
pr.rule.action = PF_PASS;
if (ioctl(dev, DIOCGETRULES, )) {
-   warn("DIOCGETRULES");
+   if (errno != EINVAL)
+   warn("DIOCGETRULES");
+   else
+   fprintf(stderr, "Anchor '%s' not found.\n",
+   anchorname);
ret = -1;
goto error;
}
@@ -886,7 +900,10 @@ pfctl_show_rules(int dev, char *path, in
 
pr.rule.action = PF_PASS;
if (ioctl(dev, DIOCGETRULES, )) {
-   warn("DIOCGETRULES");
+   if (errno != EINVAL)
+   warn("DIOCGETRULES");
+   else
+   fprintf(stderr, "Anchor '%s' not found.\n", anchorname);
ret = -1;
goto error;
}
diff -r 215db23c6b05 src/sys/kern/uipc_mbuf.c
--- src/sys/kern/uipc_mbuf.cMon Sep 25 13:38:48 2017 +0200
+++ src/sys/kern/uipc_mbuf.cTue Sep 26 11:36:50 2017 +0200
@@ -1,4 +1,4 @@
-/* $OpenBSD: uipc_mbuf.c,v 1.249 2017/09/15 18:13:05 bluhm Exp $   */
+/* $OpenBSD: uipc_mbuf.c,v 1.248 2017/05/27 16:41:10 bluhm Exp $   */
 /* $NetBSD: uipc_mbuf.c,v 1.15.4.1 1996/06/13 17:11:44 cgd Exp $   */
 
 /*
@@ -804,13 +804,12 @@ m_adj(struct mbuf *mp, int req_len)
struct mbuf *m;
int count;
 
-   if (mp == NULL)
+   if ((m = mp) == NULL)
return;
if (len >= 0) {
/*
 * Trim from head.
 */
-   m = mp;
while (m != NULL && len > 0) {
if (m->m_len <= len) {
len -= m->m_len;
@@ -834,7 +833,6 @@ m_adj(struct mbuf *mp, int req_len)
 */
len = -len;
count = 0;
-   m = mp;
for (;;) {
count += m->m_len;
if (m->m_next == NULL)
@@ -855,16 +853,15 @@ m_adj(struct mbuf *mp, int req_len)
 * Find the mbuf with last data, adjust its length,
 * and toss data from remaining mbufs on chain.
 */
-   if (mp->m_flags & M_PKTHDR)
-   mp->m_pkthdr.len = count;
m = mp;
-   for (;;) {
+   if (m->m_flags & M_PKTHDR)
+   m->m_pkthdr.len = count;
+   for (; m; m = m->m_next) {
if (m->m_len >= count) {
m->m_len = count;
break;
}
count -= m->m_len;
-   m = m->m_next;
}
while ((m = m->m_next) != NULL)
m->m_len = 0;
8<---8<---8<--8<

pfctl always prints warning when flushes ruleset

[Alexandr Nedvedicky]

Hello,

few users on Solaris don't like to read warning 'Anchor or Ruleset' does not
exist:

# echo 'pass' |pfctl -a foo -f -
# pfctl -a foo -Fa
rules cleared
pfctl: Anchor or Ruleset does not exist.
# 

the commands above did work well, the 'pfctl: Anchor ...' warning message
is kind of invalid. The code path, which ends up with warning, starts in
pfct main() function:

2518 case 'a':
2519 pfctl_clear_rules(dev, opts, anchorname);
2520 pfctl_clear_tables(anchorname, opts);
2521 if (ifaceopt && *ifaceopt) {
2522 warnx("don't specify an interface with 
-Fall");
2523 usage();
2524 /* NOTREACHED */
2525 }

we call pfctl_clear_rules(), which flushes all rules from anchor. The anchor
becomes empty. If there are no tables attached to anchor, then
pf_remove_if_empty_ruleset() called on behalf of DIOCXCOMMIT also removes the
anchor from table. No wonder the pfctl_clear_tables() invoked later at line 2520
can not find anchor, which it is searching table to be flushed.

The patch below just swaps line 2519 and 2520 to put the operations to correct
order. With patch in place the warning is gone.

# echo 'pass' |pfctl -a foo -f -
# pfctl -a foo -Fa
0 tables deleted.
rules cleared
# 

Also pfctl still prints warning in expected case:
# pfctl -sA
# pfctl -a foo -FT
pfctl: Anchor or Ruleset does not exist.
#

OK?

thanks and
regards
sasha

8<---8<---8<--8<
diff -r 215db23c6b05 src/sbin/pfctl/pfctl.c
--- src/sbin/pfctl/pfctl.c  Mon Sep 25 13:38:48 2017 +0200
+++ src/sbin/pfctl/pfctl.c  Tue Sep 26 11:15:26 2017 +0200
@@ -2516,8 +2516,8 @@ main(int argc, char *argv[])
pfctl_clear_stats(dev, ifaceopt, opts);
break;
case 'a':
+   pfctl_clear_tables(anchorname, opts);
pfctl_clear_rules(dev, opts, anchorname);
-   pfctl_clear_tables(anchorname, opts);
if (ifaceopt && *ifaceopt) {
warnx("don't specify an interface with -Fall");
usage();
8<---8<---8<--8<