Re: [PATCH RFC] mm/memory_hotplug: Introduce memory block types

[Michal Hocko]

On Wed 03-10-18 19:00:29, David Hildenbrand wrote:
[...]
> Let me rephrase: You state that user space has to make the decision and
> that user should be able to set/reconfigure rules. That is perfectly fine.
> 
> But then we should give user space access to sufficient information to
> make a decision. This might be the type of memory as we learned (what
> some part of this patch proposes), but maybe later more, e.g. to which
> physical device memory belongs (e.g. to hotplug it all movable or all
> normal) ...

I am pretty sure that user knows he/she wants to use ballooning in
HyperV or Xen, or that the memory hotplug should be used as a "RAS"
feature to allow add and remove DIMMs for reliability. Why shouldn't we
have a package to deploy an appropriate set of udev rules for each of
those usecases? I am pretty sure you need some other plumbing to enable
them anyway (e.g. RAS would require to have movable_node kernel
parameters, ballooning a kernel module etc.).

Really, one udev script to rule them all will simply never work.
-- 
Michal Hocko
SUSE Labs
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH RFC] mm/memory_hotplug: Introduce memory block types

[Michal Hocko]

On Wed 03-10-18 19:14:05, David Hildenbrand wrote:
> On 03/10/2018 16:34, Vitaly Kuznetsov wrote:
> > Dave Hansen  writes:
> > 
> >> On 10/03/2018 06:52 AM, Vitaly Kuznetsov wrote:
> >>> It is more than just memmaps (e.g. forking udev process doing memory
> >>> onlining also needs memory) but yes, the main idea is to make the
> >>> onlining synchronous with hotplug.
> >>
> >> That's a good theoretical concern.
> >>
> >> But, is it a problem we need to solve in practice?
> > 
> > Yes, unfortunately. It was previously discovered that when we try to
> > hotplug tons of memory to a low memory system (this is a common scenario
> > with VMs) we end up with OOM because for all new memory blocks we need
> > to allocate page tables, struct pages, ... and we need memory to do
> > that. The userspace program doing memory onlining also needs memory to
> > run and in case it prefers to fork to handle hundreds of notfifications
> > ... well, it may get OOMkilled before it manages to online anything.
> > 
> > Allocating all kernel objects from the newly hotplugged blocks would
> > definitely help to manage the situation but as I said this won't solve
> > the 'forking udev' problem completely (it will likely remain in
> > 'extreme' cases only. We can probably work around it by onlining with a
> > dedicated process which doesn't do memory allocation).
> > 
> 
> I guess the problem is even worse. We always have two phases
> 
> 1. add memory - requires memory allocation
> 2. online memory - might require memory allocations e.g. for slab/slub
> 
> So if we just added memory but don't have sufficient memory to start a
> user space process to trigger onlining, then we most likely also don't
> have sufficient memory to online the memory right away (in some scenarios).
> 
> We would have to allocate all new memory for 1 and 2 from the memory to
> be onlined. I guess the latter part is less trivial.
> 
> So while onlining the memory from the kernel might make things a little
> more robust, we would still have the chance for OOM / onlining failing.

Yes, _theoretically_. Is this a practical problem for reasonable
configurations though? I mean, this will never be perfect and we simply
cannot support all possible configurations. We should focus on
reasonable subset of them. From my practical experience the vast
majority of memory is consumed by memmaps (roughly 1.5%). That is not a
lot but I agree that allocating that from the zone normal and off node
is not great. Especially the second part which is noticeable for whole
node hotplug.

I have a feeling that arguing about fork not able to proceed or OOMing
for the memory hotplug is a bit of a stretch and a sign a of
misconfiguration.
-- 
Michal Hocko
SUSE Labs
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: emxx_udc: Remove unused device_desc declaration

[Nathan Chancellor]

Clang warns:

drivers/staging/emxx_udc/emxx_udc.c:1373:37: warning: variable
'device_desc' is not needed and will not be emitted
[-Wunneeded-internal-declaration]
static struct usb_device_descriptor device_desc = {
^
1 warning generated.

This definition hasn't been attached to anything since the driver was
introduced in commit 33aa8d45a4fe ("staging: emxx_udc: Add Emma Mobile
USB Gadget driver") and neither GCC nor Clang emit any reference to the
variable in the final assembly. The only reason GCC doesn't warn about
this variable being unused is the sizeof function.

Reported-by: Nick Desaulniers 
Signed-off-by: Nathan Chancellor 
---

This seems kind of wrong given this is a USB driver but there isn't an
instance of a platform_driver in the kernel tree having a usb device
descriptor declaration so I'm unsure of how to handle this warning aside
from just removing the definition but I'm certainly open to suggestions.

 drivers/staging/emxx_udc/emxx_udc.c | 19 ---
 1 file changed, 19 deletions(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c 
b/drivers/staging/emxx_udc/emxx_udc.c
index 0e8d3f232fe9..65cc3d9af972 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -1368,25 +1368,6 @@ static void _nbu2ss_set_endpoint_stall(
}
 }
 
-/*-*/
-/* Device Descriptor */
-static struct usb_device_descriptor device_desc = {
-   .bLength  = sizeof(device_desc),
-   .bDescriptorType  = USB_DT_DEVICE,
-   .bcdUSB   = cpu_to_le16(0x0200),
-   .bDeviceClass = USB_CLASS_VENDOR_SPEC,
-   .bDeviceSubClass  = 0x00,
-   .bDeviceProtocol  = 0x00,
-   .bMaxPacketSize0  = 64,
-   .idVendor = cpu_to_le16(0x0409),
-   .idProduct= cpu_to_le16(0xfff0),
-   .bcdDevice= 0x,
-   .iManufacturer= 0x00,
-   .iProduct = 0x00,
-   .iSerialNumber= 0x00,
-   .bNumConfigurations   = 0x01,
-};
-
 /*-*/
 static void _nbu2ss_set_test_mode(struct nbu2ss_udc *udc, u32 mode)
 {
-- 
2.19.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: rtl8723bs: Remove unnecessary parentheses and dead commented code

[Nathan Chancellor]

Clang warns when multiple pairs of parentheses are used for a single
conditional statement.

drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c:3351:20: warning:
equality comparison with extraneous parentheses [-Wparentheses-equality]

The commented code is pointless since _HW_STATE_AP_ is handled right
below this block.

Link: https://github.com/ClangBuiltLinux/linux/issues/168
Signed-off-by: Nathan Chancellor 
---
 drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c 
b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
index 592917fc00aa..c7e55618b9a8 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
@@ -3348,7 +3348,7 @@ static void hw_var_set_opmode(struct adapter *padapter, 
u8 variable, u8 *val)
/*  disable atim wnd */
rtw_write8(padapter, REG_BCN_CTRL, 
DIS_TSF_UDT|EN_BCN_FUNCTION|DIS_ATIM);
/* rtw_write8(padapter, REG_BCN_CTRL, 0x18); */
-   } else if ((mode == _HW_STATE_ADHOC_) /*|| (mode == 
_HW_STATE_AP_)*/) {
+   } else if (mode == _HW_STATE_ADHOC_) {
ResumeTxBeacon(padapter);
rtw_write8(padapter, REG_BCN_CTRL, 
DIS_TSF_UDT|EN_BCN_FUNCTION|DIS_BCNQ_SUB);
} else if (mode == _HW_STATE_AP_) {
-- 
2.19.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: rtlwifi: Remove unnecessary parentheses

[Nathan Chancellor]

Clang warns when multiple pairs of parentheses are used for a single
conditional statement.

drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c:558:33: warning:
equality comparison with extraneous parentheses [-Wparentheses-equality]
} else if ((is_enable_la_mode == 1)) {
~~^~~~
drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c:558:33: note: remove
extraneous parentheses around the comparison to silence this warning
} else if ((is_enable_la_mode == 1)) {
   ~  ^   ~
drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c:558:33: note: use '='
to turn this equality comparison into an assignment
} else if ((is_enable_la_mode == 1)) {
  ^~
  =
1 warning generated.

Link: https://github.com/ClangBuiltLinux/linux/issues/172
Signed-off-by: Nathan Chancellor 
---
 drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c 
b/drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c
index 42020101380a..d6cea73fa185 100644
--- a/drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c
+++ b/drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c
@@ -555,7 +555,7 @@ void phydm_lamode_trigger_setting(void *dm_void, char 
input[][16], u32 *_used,
output + used, out_len - used,
"{En} {0:BB,1:BB_MAC,2:RF0,3:RF1,4:MAC}\n 
{BB:dbg_port[bit],BB_MAC:0-ok/1-fail/2-cca,MAC:ref} {DMA type} {TrigTime}\n 
{polling_time/ref_mask} {dbg_port} {0:P_Edge, 1:N_Edge} 
{SpRate:0-80M,1-40M,2-20M} {Capture num}\n");
/**/
-   } else if ((is_enable_la_mode == 1)) {
+   } else if (is_enable_la_mode == 1) {
PHYDM_SSCANF(input[2], DCMD_DECIMAL, &var1[1]);
 
trig_mode = (u8)var1[1];
-- 
2.19.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] staging: rtl8188eu: cleanup array declaration - style

[Joe Perches]

On Wed, 2018-10-03 at 22:43 +0200, Michael Straube wrote:
> Cleanup array declaration to clear two 'line over 80 characters'
> checkpatch warnings and improve readability.
[]
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c 
> b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
[]
> @@ -3836,24 +3836,20 @@ Following are the initialization functions for WiFi 
> MLME
>  
> */
>  
>  static struct mlme_handler mlme_sta_tbl[] = {
> - {WIFI_ASSOCREQ, "OnAssocReq",   &OnAssocReq},
> - {WIFI_ASSOCRSP, "OnAssocRsp",   &OnAssocRsp},
> - {WIFI_REASSOCREQ,   "OnReAssocReq", &OnAssocReq},
> - {WIFI_REASSOCRSP,   "OnReAssocRsp", &OnAssocRsp},
> - {WIFI_PROBEREQ, "OnProbeReq",   &OnProbeReq},
> - {WIFI_PROBERSP, "OnProbeRsp",   &OnProbeRsp},
> -
> - /*--
> - below 2 are reserved
> - ---*/
> - {0, "DoReserved",   
> &DoReserved},
> - {0, "DoReserved",   
> &DoReserved},
> - {WIFI_BEACON,   "OnBeacon", &OnBeacon},
> - {WIFI_ATIM, "OnATIM",   &OnAtim},
> - {WIFI_DISASSOC, "OnDisassoc",   &OnDisassoc},
> - {WIFI_AUTH, "OnAuth",   &OnAuthClient},
> - {WIFI_DEAUTH,   "OnDeAuth", &OnDeAuth},
> - {WIFI_ACTION,   "OnAction", &OnAction},
> + {WIFI_ASSOCREQ,   "OnAssocReq",   &OnAssocReq},
> + {WIFI_ASSOCRSP,   "OnAssocRsp",   &OnAssocRsp},
> + {WIFI_REASSOCREQ, "OnReAssocReq", &OnAssocReq},
> + {WIFI_REASSOCRSP, "OnReAssocRsp", &OnAssocRsp},
> + {WIFI_PROBEREQ,   "OnProbeReq",   &OnProbeReq},
> + {WIFI_PROBERSP,   "OnProbeRsp",   &OnProbeRsp},
> + {0,   "DoReserved",   &DoReserved},
> + {0,   "DoReserved",   &DoReserved},
> + {WIFI_BEACON, "OnBeacon", &OnBeacon},
> + {WIFI_ATIM,   "OnATIM",   &OnAtim},
> + {WIFI_DISASSOC,   "OnDisassoc",   &OnDisassoc},
> + {WIFI_AUTH,   "OnAuth",   &OnAuthClient},
> + {WIFI_DEAUTH, "OnDeAuth", &OnDeAuth},
> + {WIFI_ACTION, "OnAction", &OnAction},
>  };

Perhaps const here too, and as well, the struct
declaration could use const char * instead of char *



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [patch 00/11] x86/vdso: Cleanups, simmplifications and CLOCK_TAI support

[Andy Lutomirski]

On Wed, Oct 3, 2018 at 12:01 PM Marcelo Tosatti  wrote:
>
> On Tue, Oct 02, 2018 at 10:15:49PM -0700, Andy Lutomirski wrote:
> > Hi Vitaly, Paolo, Radim, etc.,
> >
> > On Fri, Sep 14, 2018 at 5:52 AM Thomas Gleixner  wrote:
> > >
> > > Matt attempted to add CLOCK_TAI support to the VDSO clock_gettime()
> > > implementation, which extended the clockid switch case and added yet
> > > another slightly different copy of the same code.
> > >
> > > Especially the extended switch case is problematic as the compiler tends 
> > > to
> > > generate a jump table which then requires to use retpolines. If jump 
> > > tables
> > > are disabled it adds yet another conditional to the existing maze.
> > >
> > > This series takes a different approach by consolidating the almost
> > > identical functions into one implementation for high resolution clocks and
> > > one for the coarse grained clock ids by storing the base data for each
> > > clock id in an array which is indexed by the clock id.
> > >
> >
> > I was trying to understand more of the implications of this patch
> > series, and I was again reminded that there is an entire extra copy of
> > the vclock reading code in arch/x86/kvm/x86.c.  And the purpose of
> > that code is very, very opaque.
> >
> > Can one of you explain what the code is even doing?  From a couple of
> > attempts to read through it, it's a whole bunch of
> > probably-extremely-buggy code that,
>
> Yes, probably.
>
> > drumroll please, tries to atomically read the TSC value and the time.  And 
> > decide whether the
> > result is "based on the TSC".
>
> I think "based on the TSC" refers to whether TSC clocksource is being
> used.
>
> > And then synthesizes a TSC-to-ns
> > multiplier and shift, based on *something other than the actual
> > multiply and shift used*.
> >
> > IOW, unless I'm totally misunderstanding it, the code digs into the
> > private arch clocksource data intended for the vDSO, uses a poorly
> > maintained copy of the vDSO code to read the time (instead of doing
> > the sane thing and using the kernel interfaces for this), and
> > propagates a totally made up copy to the guest.
>
> I posted kernel interfaces for this, and it was suggested to
> instead write a "in-kernel user of pvclock data".
>
> If you can get kernel interfaces to replace that, go for it. I prefer
> kernel interfaces as well.
>
> >  And gets it entirely
> > wrong when doing nested virt, since, unless there's some secret in
> > this maze, it doesn't acutlaly use the scaling factor from the host
> > when it tells the guest what to do.
> >
> > I am really, seriously tempted to send a patch to simply delete all
> > this code.
>
> If your patch which deletes the code gets the necessary features right,
> sure, go for it.
>
> > The correct way to do it is to hook
>
> Can you expand on the correct way to do it?
>
> > And I don't see how it's even possible to pass kvmclock correctly to
> > the L2 guest when L0 is hyperv.  KVM could pass *hyperv's* clock, but
> > L1 isn't notified when the data structure changes, so how the heck is
> > it supposed to update the kvmclock structure?
>
> I don't parse your question.

Let me ask it more intelligently: when the "reenlightenment" IRQ
happens, what tells KVM to do its own update for its guests?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/3] staging: rtl8188eu: fix line over 80 characters - style

[Joe Perches]

On Wed, 2018-10-03 at 22:43 +0200, Michael Straube wrote:
> Line break array declaration to clear a 'line over 80 characters'
> checkpatch warning. For consistency replace 0x0 with 0x00.
[]
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c 
> b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
[]
> @@ -39,7 +39,10 @@ extern unsigned char REALTEK_96B_IE[];
>  /
>  MCS rate definitions
>  */
> -unsigned charMCS_rate_1R[16] = {0xff, 0x00, 0x0, 0x0, 0x01, 0x0, 
> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0};
> +unsigned char MCS_rate_1R[16] = {
> + 0xff, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +};

Might as well make this const u8.

Here and as well the extern declarations in
drivers/staging/rtl8188eu/core/rtw_mlme.c
and
drivers/staging/rtl8188eu/include/rtw_mlme.h


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 12/13] staging: comedi: ni_660x: clean up pfi routing

[Spencer E. Olson]

Cleans up the pfi routing code to make it easier to follow, read, and also
to prepare to use this cleaned up code for enabling the device-global
routing interface for ni_660x devices.

Signed-off-by: Spencer E. Olson 
---
 drivers/staging/comedi/drivers/ni_660x.c | 72 ++--
 1 file changed, 56 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_660x.c 
b/drivers/staging/comedi/drivers/ni_660x.c
index 0dfaf8ed093d..59055f366138 100644
--- a/drivers/staging/comedi/drivers/ni_660x.c
+++ b/drivers/staging/comedi/drivers/ni_660x.c
@@ -596,6 +596,37 @@ static void ni_660x_select_pfi_output(struct comedi_device 
*dev,
ni_660x_write(dev, active_chip, bits, NI660X_IO_CFG(chan));
 }
 
+static void ni_660x_set_pfi_direction(struct comedi_device *dev,
+ unsigned int chan,
+ unsigned int direction)
+{
+   struct ni_660x_private *devpriv = dev->private;
+   u64 bit;
+
+   bit = 1ULL << chan;
+
+   if (direction == COMEDI_OUTPUT) {
+   devpriv->io_dir |= bit;
+   /* reset the output to currently assigned output value */
+   ni_660x_select_pfi_output(dev, chan, devpriv->io_cfg[chan]);
+   } else {
+   devpriv->io_dir &= ~bit;
+   /* set pin to high-z; do not change currently assigned route */
+   ni_660x_select_pfi_output(dev, chan, 0);
+   }
+}
+
+static unsigned int ni_660x_get_pfi_direction(struct comedi_device *dev,
+ unsigned int chan)
+{
+   struct ni_660x_private *devpriv = dev->private;
+   u64 bit;
+
+   bit = 1ULL << chan;
+
+   return (devpriv->io_dir & bit) ? COMEDI_OUTPUT : COMEDI_INPUT;
+}
+
 static int ni_660x_set_pfi_routing(struct comedi_device *dev,
   unsigned int chan, unsigned int source)
 {
@@ -614,36 +645,48 @@ static int ni_660x_set_pfi_routing(struct comedi_device 
*dev,
}
 
devpriv->io_cfg[chan] = source;
-   if (devpriv->io_dir & (1ULL << chan))
+   if (ni_660x_get_pfi_direction(dev, chan) == COMEDI_OUTPUT)
ni_660x_select_pfi_output(dev, chan, devpriv->io_cfg[chan]);
return 0;
 }
 
+static int ni_660x_get_pfi_routing(struct comedi_device *dev, unsigned int 
chan)
+{
+   struct ni_660x_private *devpriv = dev->private;
+
+   return devpriv->io_cfg[chan];
+}
+
+static void ni_660x_set_pfi_filter(struct comedi_device *dev,
+  unsigned int chan, unsigned int value)
+{
+   unsigned int val;
+
+   val = ni_660x_read(dev, 0, NI660X_IO_CFG(chan));
+   val &= ~NI660X_IO_CFG_IN_SEL_MASK(chan);
+   val |= NI660X_IO_CFG_IN_SEL(chan, value);
+   ni_660x_write(dev, 0, val, NI660X_IO_CFG(chan));
+}
+
 static int ni_660x_dio_insn_config(struct comedi_device *dev,
   struct comedi_subdevice *s,
   struct comedi_insn *insn,
   unsigned int *data)
 {
-   struct ni_660x_private *devpriv = dev->private;
unsigned int chan = CR_CHAN(insn->chanspec);
-   u64 bit = 1ULL << chan;
-   unsigned int val;
int ret;
 
switch (data[0]) {
case INSN_CONFIG_DIO_OUTPUT:
-   devpriv->io_dir |= bit;
-   ni_660x_select_pfi_output(dev, chan, devpriv->io_cfg[chan]);
+   ni_660x_set_pfi_direction(dev, chan, COMEDI_OUTPUT);
break;
 
case INSN_CONFIG_DIO_INPUT:
-   devpriv->io_dir &= ~bit;
-   ni_660x_select_pfi_output(dev, chan, 0);/* high-z */
+   ni_660x_set_pfi_direction(dev, chan, COMEDI_INPUT);
break;
 
case INSN_CONFIG_DIO_QUERY:
-   data[1] = (devpriv->io_dir & bit) ? COMEDI_OUTPUT
- : COMEDI_INPUT;
+   data[1] = ni_660x_get_pfi_direction(dev, chan);
break;
 
case INSN_CONFIG_SET_ROUTING:
@@ -653,14 +696,11 @@ static int ni_660x_dio_insn_config(struct comedi_device 
*dev,
break;
 
case INSN_CONFIG_GET_ROUTING:
-   data[1] = devpriv->io_cfg[chan];
+   data[1] = ni_660x_get_pfi_routing(dev, chan);
break;
 
case INSN_CONFIG_FILTER:
-   val = ni_660x_read(dev, 0, NI660X_IO_CFG(chan));
-   val &= ~NI660X_IO_CFG_IN_SEL_MASK(chan);
-   val |= NI660X_IO_CFG_IN_SEL(chan, data[1]);
-   ni_660x_write(dev, 0, val, NI660X_IO_CFG(chan));
+   ni_660x_set_pfi_filter(dev, chan, data[1]);
break;
 
default:
@@ -840,7 +880,7 @@ static int ni_660x_auto_attach(struct comedi_device *dev,
  : NI_660X_PFI_OUTPUT_COUNTER;
 
ni_660x_set_pfi_routing(dev, i, source);
-  

[PATCH v4 09/13] staging: comedi: tio: implement global tio/ctr routing

[Spencer E. Olson]

Adds ability to use device-global names in command args, in particular
cmd->start_arg (for NI_CtrArmStartTrigger), and cmd->scan_begin_arg or
cmd->convert_arg (either is used to specify NI_CtrGate, with preference
given to cmd->scan_begin_arg, if it is set).

The actual arguments of cmd->start_arg are not fully checked against known
register values for the particular devices because these are not documented
or currently known.  This follows the precedence of prior versions of the
tio driver.  Should these become known, they should be annotated in the
route_values tables and the set of lines in ni_tio_cmdtest should be
uncommented to allow the tests to be made.

This patch also adds interface functions that allow routes for particular
counter route destinations to be made/queried/unmade.  This allows overseer
modules to implement test_route, connect_route, and disconnect_route.  As a
part of these changes, various functions were cleaned up and clarified.

These new interface functions allow direct writing/reading of register
values.  This is an example of exactly what the new device-global access
was intended to solve:  the old interface was not consistent with other
portions of the ni_* drivers--it did not allow full register values to be
given for various MUXes.  Instead, the old interface _did_ abstract away
some of the actual hardware from the underlying devices, but it was not
consistent with any other NI hardware.  Allowing the device-global
identifiers to be used, the new patch provides for consistency across all
ni_* drivers.  One final note:  these changes provide for backwards
compatibility by allowing the older values to still be used in through the
pre-existing kernel interfaces--though not in the new device-global
test/dis/connect/route interfaces.

Signed-off-by: Spencer E. Olson 
---
 drivers/staging/comedi/drivers/ni_660x.c  |  18 +-
 .../staging/comedi/drivers/ni_mio_common.c|   6 +-
 drivers/staging/comedi/drivers/ni_tio.c   | 457 ++
 drivers/staging/comedi/drivers/ni_tio.h   |  42 +-
 .../staging/comedi/drivers/ni_tio_internal.h  |   2 +
 drivers/staging/comedi/drivers/ni_tiocmd.c|  66 ++-
 6 files changed, 476 insertions(+), 115 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_660x.c 
b/drivers/staging/comedi/drivers/ni_660x.c
index e521ed9d0887..498b2868c957 100644
--- a/drivers/staging/comedi/drivers/ni_660x.c
+++ b/drivers/staging/comedi/drivers/ni_660x.c
@@ -31,6 +31,7 @@
 
 #include "mite.h"
 #include "ni_tio.h"
+#include "ni_routes.h"
 
 /* See Register-Level Programmer Manual page 3.1 */
 enum ni_660x_register {
@@ -259,6 +260,7 @@ struct ni_660x_private {
unsigned int dma_cfg[NI660X_MAX_CHIPS];
unsigned int io_cfg[NI660X_NUM_PFI_CHANNELS];
u64 io_dir;
+   struct ni_route_tables routing_tables;
 };
 
 static void ni_660x_write(struct comedi_device *dev, unsigned int chip,
@@ -730,12 +732,23 @@ static int ni_660x_auto_attach(struct comedi_device *dev,
 
ni_660x_init_tio_chips(dev, board->n_chips);
 
+   /* prepare the device for globally-named routes. */
+   if (ni_assign_device_routes("ni_660x", board->name,
+   &devpriv->routing_tables) < 0) {
+   dev_warn(dev->class_dev, "%s: %s device has no signal routing 
table.\n",
+__func__, board->name);
+   dev_warn(dev->class_dev, "%s: High level NI signal names will 
not be available for this %s board.\n",
+__func__, board->name);
+   }
+
n_counters = board->n_chips * NI660X_COUNTERS_PER_CHIP;
gpct_dev = ni_gpct_device_construct(dev,
ni_660x_gpct_write,
ni_660x_gpct_read,
ni_gpct_variant_660x,
-   n_counters);
+   n_counters,
+   NI660X_COUNTERS_PER_CHIP,
+   &devpriv->routing_tables);
if (!gpct_dev)
return -ENOMEM;
devpriv->counter_dev = gpct_dev;
@@ -831,9 +844,6 @@ static int ni_660x_auto_attach(struct comedi_device *dev,
if (i < n_counters) {
struct ni_gpct *counter = &gpct_dev->counters[i];
 
-   counter->chip_index = i / NI660X_COUNTERS_PER_CHIP;
-   counter->counter_index = i % NI660X_COUNTERS_PER_CHIP;
-
s->type = COMEDI_SUBD_COUNTER;
s->subdev_flags = SDF_READABLE | SDF_WRITABLE |
  SDF_LSAMPL | SDF_CMD_READ;
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c 
b/drivers/staging/comedi/drivers/ni_mio_common.c
index d3290c28b1ce..d7862787e064 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/sta

[PATCH v4 13/13] staging: comedi: ni_660x: add device-global routing

[Spencer E. Olson]

Provides the device-global routing interface for ni_660x devices.  Using
the device-global names in comedi_cmd structures for commands was already
supported through the ni_tio module.

Signed-off-by: Spencer E. Olson 
---
 drivers/staging/comedi/drivers/ni_660x.c | 265 +++
 1 file changed, 265 insertions(+)

diff --git a/drivers/staging/comedi/drivers/ni_660x.c 
b/drivers/staging/comedi/drivers/ni_660x.c
index 59055f366138..e70a461e723f 100644
--- a/drivers/staging/comedi/drivers/ni_660x.c
+++ b/drivers/staging/comedi/drivers/ni_660x.c
@@ -568,6 +568,10 @@ static void ni_660x_select_pfi_output(struct comedi_device 
*dev,
unsigned int idle_chip = 0;
unsigned int bits;
 
+   if (chan >= NI_PFI(0))
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+
if (board->n_chips > 1) {
if (out_sel == NI_660X_PFI_OUTPUT_COUNTER &&
chan >= 8 && chan <= 23) {
@@ -603,6 +607,10 @@ static void ni_660x_set_pfi_direction(struct comedi_device 
*dev,
struct ni_660x_private *devpriv = dev->private;
u64 bit;
 
+   if (chan >= NI_PFI(0))
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+
bit = 1ULL << chan;
 
if (direction == COMEDI_OUTPUT) {
@@ -622,6 +630,10 @@ static unsigned int ni_660x_get_pfi_direction(struct 
comedi_device *dev,
struct ni_660x_private *devpriv = dev->private;
u64 bit;
 
+   if (chan >= NI_PFI(0))
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+
bit = 1ULL << chan;
 
return (devpriv->io_dir & bit) ? COMEDI_OUTPUT : COMEDI_INPUT;
@@ -632,6 +644,10 @@ static int ni_660x_set_pfi_routing(struct comedi_device 
*dev,
 {
struct ni_660x_private *devpriv = dev->private;
 
+   if (chan >= NI_PFI(0))
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+
switch (source) {
case NI_660X_PFI_OUTPUT_COUNTER:
if (chan < 8)
@@ -654,6 +670,10 @@ static int ni_660x_get_pfi_routing(struct comedi_device 
*dev, unsigned int chan)
 {
struct ni_660x_private *devpriv = dev->private;
 
+   if (chan >= NI_PFI(0))
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+
return devpriv->io_cfg[chan];
 }
 
@@ -662,6 +682,10 @@ static void ni_660x_set_pfi_filter(struct comedi_device 
*dev,
 {
unsigned int val;
 
+   if (chan >= NI_PFI(0))
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+
val = ni_660x_read(dev, 0, NI660X_IO_CFG(chan));
val &= ~NI660X_IO_CFG_IN_SEL_MASK(chan);
val |= NI660X_IO_CFG_IN_SEL(chan, value);
@@ -710,6 +734,240 @@ static int ni_660x_dio_insn_config(struct comedi_device 
*dev,
return insn->n;
 }
 
+static unsigned int _ni_get_valid_routes(struct comedi_device *dev,
+unsigned int n_pairs,
+unsigned int *pair_data)
+{
+   struct ni_660x_private *devpriv = dev->private;
+
+   return ni_get_valid_routes(&devpriv->routing_tables, n_pairs,
+  pair_data);
+}
+
+/*
+ * Retrieves the current source of the output selector for the given
+ * destination.  If the terminal for the destination is not already configured
+ * as an output, this function returns -EINVAL as error.
+ *
+ * Return: The register value of the destination output selector;
+ *-EINVAL if terminal is not configured for output.
+ */
+static inline int get_output_select_source(int dest, struct comedi_device *dev)
+{
+   struct ni_660x_private *devpriv = dev->private;
+   int reg = -1;
+
+   if (channel_is_pfi(dest)) {
+   if (ni_660x_get_pfi_direction(dev, dest) == COMEDI_OUTPUT)
+   reg = ni_660x_get_pfi_routing(dev, dest);
+   } else if (channel_is_rtsi(dest)) {
+   dev_dbg(dev->class_dev,
+   "%s: unhandled rtsi destination (%d) queried\n",
+   __func__, dest);
+   /*
+* The following can be enabled when RTSI routing info is
+* determined (not currently documented):
+* if (ni_get_rtsi_direction(dev, dest) == COMEDI_OUTPUT) {
+*  reg = ni_get_rtsi_routing(dev, dest);
+
+*  if (reg == NI_RTSI_OUTPUT_RGOUT0) {
+*  dest = NI_RGOUT0; ** prepare for lookup below **
+*  reg = get_rgout0_reg(dev);
+*  } else if (reg >= NI_RTSI_OUTPUT_RTSI_BRD(0) &&
+* reg <= NI_RTSI_OUTPUT_RTSI_BRD(3)) {
+*  const int i = reg - NI_RTSI_OUTPUT

[PATCH v4 07/13] staging: comedi: ni_mio_common: implement global pfi, rtsi routing

[Spencer E. Olson]

Implement device-global config interface for ni_mio devices.  In
particular, this patch implements:
INSN_DEVICE_CONFIG_TEST_ROUTE,
INSN_DEVICE_CONFIG_CONNECT_ROUTE,
INSN_DEVICE_CONFIG_DISCONNECT_ROUTE,
INSN_DEVICE_CONFIG_GET_ROUTES
for the ni mio devices.  This means that the new abstracted signal/terminal
names can be used to define signal routing with regards to the PFI
terminals and RTSI trigger bus lines.

This also adds ability to identify PFI and RTSI channels on the PFI and
RTSI subdevices using the new device-global names.  This does not change
the values that are set for channel output selections using the subdevice
interfaces--these still require direct register values.

Annotates and updates tables of register values to reflect this new
implementation status.

Signed-off-by: Spencer E. Olson 
---

Patch revisions & Notes:
  - [PATCH v4 07/13]: Rebased patchset on repaired patch "staging: comedi:
ni_mio_common: protect register write overflow" (that patch had a missing
brace and this patch depends on that earlier patch).

  - [PATCH v2 07/13]: This patch must be built upon an earlier patch recently
submitted and in the queue for integration:
"staging: comedi: ni_mio_common: protect register write overflow"

 .../staging/comedi/drivers/ni_mio_common.c| 685 --
 drivers/staging/comedi/drivers/ni_stc.h   |  68 ++
 2 files changed, 682 insertions(+), 71 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c 
b/drivers/staging/comedi/drivers/ni_mio_common.c
index d8c715c42fd1..c42d480df7ff 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -351,7 +351,8 @@ static const struct mio_regmap m_series_stc_write_regmap[] 
= {
[NISTC_AO_PERSONAL_REG] = { 0x19c, 2 },
[NISTC_RTSI_TRIGA_OUT_REG]  = { 0x19e, 2 },
[NISTC_RTSI_TRIGB_OUT_REG]  = { 0x1a0, 2 },
-   [NISTC_RTSI_BOARD_REG]  = { 0, 0 }, /* Unknown */
+   /* doc for following line: mhddk/nimseries/ChipObjects/tMSeries.h */
+   [NISTC_RTSI_BOARD_REG]  = { 0x1a2, 2 },
[NISTC_CFG_MEM_CLR_REG] = { 0x1a4, 2 },
[NISTC_ADC_FIFO_CLR_REG]= { 0x1a6, 2 },
[NISTC_DAC_FIFO_CLR_REG]= { 0x1a8, 2 },
@@ -4566,24 +4567,33 @@ static unsigned int ni_get_pfi_routing(struct 
comedi_device *dev,
 {
struct ni_private *devpriv = dev->private;
 
+   if (chan >= NI_PFI(0)) {
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+   }
return (devpriv->is_m_series)
? ni_m_series_get_pfi_routing(dev, chan)
: ni_old_get_pfi_routing(dev, chan);
 }
 
+/* Sets the output mux for the specified PFI channel. */
 static int ni_set_pfi_routing(struct comedi_device *dev,
  unsigned int chan, unsigned int source)
 {
struct ni_private *devpriv = dev->private;
 
+   if (chan >= NI_PFI(0)) {
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+   }
return (devpriv->is_m_series)
? ni_m_series_set_pfi_routing(dev, chan, source)
: ni_old_set_pfi_routing(dev, chan, source);
 }
 
-static int ni_config_filter(struct comedi_device *dev,
-   unsigned int pfi_channel,
-   enum ni_pfi_filter_select filter)
+static int ni_config_pfi_filter(struct comedi_device *dev,
+   unsigned int chan,
+   enum ni_pfi_filter_select filter)
 {
struct ni_private *devpriv = dev->private;
unsigned int bits;
@@ -4591,19 +4601,46 @@ static int ni_config_filter(struct comedi_device *dev,
if (!devpriv->is_m_series)
return -ENOTSUPP;
 
+   if (chan >= NI_PFI(0)) {
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+   }
+
bits = ni_readl(dev, NI_M_PFI_FILTER_REG);
-   bits &= ~NI_M_PFI_FILTER_SEL_MASK(pfi_channel);
-   bits |= NI_M_PFI_FILTER_SEL(pfi_channel, filter);
+   bits &= ~NI_M_PFI_FILTER_SEL_MASK(chan);
+   bits |= NI_M_PFI_FILTER_SEL(chan, filter);
ni_writel(dev, bits, NI_M_PFI_FILTER_REG);
return 0;
 }
 
+static void ni_set_pfi_direction(struct comedi_device *dev, int chan,
+unsigned int direction)
+{
+   if (chan >= NI_PFI(0)) {
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+   }
+   direction = (direction == COMEDI_OUTPUT) ? 1u : 0u;
+   ni_set_bits(dev, NISTC_IO_BIDIR_PIN_REG, 1 << chan, direction);
+}
+
+static int ni_get_pfi_direction(struct comedi_device *dev, int chan)
+{
+   struct ni_private *devpriv = dev->private;
+
+   if (chan >= NI_PFI(0)) {
+ 

[PATCH v4 11/13] staging: comedi: ni_660x: Add NI PCI-6608 to list of supported devices

[Spencer E. Olson]

Previously, only the PXI version of the NI-6608 board was supported.  This
change adds support for the PCI version as well.

Signed-off-by: Spencer E. Olson 
---
 drivers/staging/comedi/drivers/ni_660x.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/ni_660x.c 
b/drivers/staging/comedi/drivers/ni_660x.c
index 498b2868c957..0dfaf8ed093d 100644
--- a/drivers/staging/comedi/drivers/ni_660x.c
+++ b/drivers/staging/comedi/drivers/ni_660x.c
@@ -7,7 +7,7 @@
  * Driver: ni_660x
  * Description: National Instruments 660x counter/timer boards
  * Devices: [National Instruments] PCI-6601 (ni_660x), PCI-6602, PXI-6602,
- *   PXI-6608, PCI-6624, PXI-6624
+ *   PCI-6608, PXI-6608, PCI-6624, PXI-6624
  * Author: J.P. Mellor ,
  *   herman.bruynin...@mech.kuleuven.ac.be,
  *   wim.meeus...@mech.kuleuven.ac.be,
@@ -202,6 +202,7 @@ enum ni_660x_boardid {
BOARD_PCI6601,
BOARD_PCI6602,
BOARD_PXI6602,
+   BOARD_PCI6608,
BOARD_PXI6608,
BOARD_PCI6624,
BOARD_PXI6624
@@ -225,6 +226,10 @@ static const struct ni_660x_board ni_660x_boards[] = {
.name   = "PXI-6602",
.n_chips= 2,
},
+   [BOARD_PCI6608] = {
+   .name   = "PCI-6608",
+   .n_chips= 2,
+   },
[BOARD_PXI6608] = {
.name   = "PXI-6608",
.n_chips= 2,
@@ -925,6 +930,7 @@ static const struct pci_device_id ni_660x_pci_table[] = {
{ PCI_VDEVICE(NI, 0x1310), BOARD_PCI6602 },
{ PCI_VDEVICE(NI, 0x1360), BOARD_PXI6602 },
{ PCI_VDEVICE(NI, 0x2c60), BOARD_PCI6601 },
+   { PCI_VDEVICE(NI, 0x2db0), BOARD_PCI6608 },
{ PCI_VDEVICE(NI, 0x2cc0), BOARD_PXI6608 },
{ PCI_VDEVICE(NI, 0x1e30), BOARD_PCI6624 },
{ PCI_VDEVICE(NI, 0x1e40), BOARD_PXI6624 },
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 08/13] staging: comedi: ni_mio_common: implement output selection of GPFO_{0, 1}

[Spencer E. Olson]

Implement the ability to route various signals to NI_CtrOut(x) pin.  This
pin is also known as GPFO_{0,1} in the DAQ STC.

Signed-off-by: Spencer E. Olson 
---
 .../staging/comedi/drivers/ni_mio_common.c| 106 ++
 drivers/staging/comedi/drivers/ni_stc.h   |   6 +-
 2 files changed, 111 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c 
b/drivers/staging/comedi/drivers/ni_mio_common.c
index c42d480df7ff..d3290c28b1ce 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -5508,6 +5508,77 @@ static void ni_rtsi_init(struct comedi_device *dev)
set_rgout0_reg(0, dev);
 }
 
+/* Get route of GPFO_i/CtrOut pins */
+static inline int ni_get_gout_routing(unsigned int dest,
+ struct comedi_device *dev)
+{
+   struct ni_private *devpriv = dev->private;
+   unsigned int reg = devpriv->an_trig_etc_reg;
+
+   switch (dest) {
+   case 0:
+   if (reg & NISTC_ATRIG_ETC_GPFO_0_ENA)
+   return NISTC_ATRIG_ETC_GPFO_0_SEL_TO_SRC(reg);
+   break;
+   case 1:
+   if (reg & NISTC_ATRIG_ETC_GPFO_1_ENA)
+   return NISTC_ATRIG_ETC_GPFO_1_SEL_TO_SRC(reg);
+   break;
+   }
+
+   return -EINVAL;
+}
+
+/* Set route of GPFO_i/CtrOut pins */
+static inline int ni_disable_gout_routing(unsigned int dest,
+ struct comedi_device *dev)
+{
+   struct ni_private *devpriv = dev->private;
+
+   switch (dest) {
+   case 0:
+   devpriv->an_trig_etc_reg &= ~NISTC_ATRIG_ETC_GPFO_0_ENA;
+   break;
+   case 1:
+   devpriv->an_trig_etc_reg &= ~NISTC_ATRIG_ETC_GPFO_1_ENA;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   ni_stc_writew(dev, devpriv->an_trig_etc_reg, NISTC_ATRIG_ETC_REG);
+   return 0;
+}
+
+/* Set route of GPFO_i/CtrOut pins */
+static inline int ni_set_gout_routing(unsigned int src, unsigned int dest,
+ struct comedi_device *dev)
+{
+   struct ni_private *devpriv = dev->private;
+
+   switch (dest) {
+   case 0:
+   /* clear reg */
+   devpriv->an_trig_etc_reg &= ~NISTC_ATRIG_ETC_GPFO_0_SEL(-1);
+   /* set reg */
+   devpriv->an_trig_etc_reg |= NISTC_ATRIG_ETC_GPFO_0_ENA
+|  NISTC_ATRIG_ETC_GPFO_0_SEL(src);
+   break;
+   case 1:
+   /* clear reg */
+   devpriv->an_trig_etc_reg &= ~NISTC_ATRIG_ETC_GPFO_1_SEL;
+   src = src ? NISTC_ATRIG_ETC_GPFO_1_SEL : 0;
+   /* set reg */
+   devpriv->an_trig_etc_reg |= NISTC_ATRIG_ETC_GPFO_1_ENA | src;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   ni_stc_writew(dev, devpriv->an_trig_etc_reg, NISTC_ATRIG_ETC_REG);
+   return 0;
+}
+
 /*
  * Retrieves the current source of the output selector for the given
  * destination.  If the terminal for the destination is not already configured
@@ -5539,6 +5610,16 @@ static int get_output_select_source(int dest, struct 
comedi_device *dev)
reg = get_ith_rtsi_brd_reg(i, dev);
}
}
+   } else if (dest >= NI_CtrOut(0) && dest <= NI_CtrOut(-1)) {
+   /*
+* not handled by ni_tio.  Only available for GPFO registers in
+* e/m series.
+*/
+   dest -= NI_CtrOut(0);
+   if (dest > 1)
+   /* there are only two g_out outputs. */
+   return -EINVAL;
+   reg = ni_get_gout_routing(dest, dev);
} else {
dev_dbg(dev->class_dev, "%s: unhandled destination (%d) 
queried\n",
__func__, dest);
@@ -5616,6 +5697,17 @@ static int connect_route(unsigned int src, unsigned int 
dest,
 
ni_set_rtsi_direction(dev, dest, COMEDI_OUTPUT);
ni_set_rtsi_routing(dev, dest, reg);
+   } else if (dest >= NI_CtrOut(0) && dest <= NI_CtrOut(-1)) {
+   /*
+* not handled by ni_tio.  Only available for GPFO registers in
+* e/m series.
+*/
+   dest -= NI_CtrOut(0);
+   if (dest > 1)
+   /* there are only two g_out outputs. */
+   return -EINVAL;
+   if (ni_set_gout_routing(src, dest, dev))
+   return -EINVAL;
} else {
return -EINVAL;
}
@@ -5664,6 +5756,16 @@ static int disconnect_route(unsigned int src, unsigned 
int dest,
reg = default_rtsi_routing[dest - TRIGGER_LINE(0)];
ni_set_rtsi_direction(dev, dest, COMEDI_INPUT);
  

[PATCH v4 05/13] staging: comedi: add interface to ni routing table information

[Spencer E. Olson]

Adds interface and associated unittests for accessing/looking-up/validating
the new ni routing table information.

Signed-off-by: Spencer E. Olson 
---

Patch revisions:
  - [PATCH v3 05/13]: Simplify and clean up prototypes of functions for use with
besearch.

  - [PATCH v2 05/13]: Tweak Makefile to build routing info for newly added
hardware in updates to [PATCH v2 04/13].
  - [PATCH v2 05/13]: Fixes placement of "select COMEDI_NI_ROUTING" as per Ian's
suggestion.
  - [PATCH v2 05/13]: Removes a few inline function declarations in unit test.

 drivers/staging/comedi/Kconfig|   4 +
 drivers/staging/comedi/drivers/Makefile   |  27 +
 drivers/staging/comedi/drivers/ni_routes.c| 523 +++
 drivers/staging/comedi/drivers/ni_routes.h| 329 ++
 drivers/staging/comedi/drivers/ni_stc.h   |   4 +
 drivers/staging/comedi/drivers/tests/Makefile |   3 +-
 .../comedi/drivers/tests/ni_routes_test.c | 613 ++
 7 files changed, 1502 insertions(+), 1 deletion(-)
 create mode 100644 drivers/staging/comedi/drivers/ni_routes.c
 create mode 100644 drivers/staging/comedi/drivers/ni_routes.h
 create mode 100644 drivers/staging/comedi/drivers/tests/ni_routes_test.c

diff --git a/drivers/staging/comedi/Kconfig b/drivers/staging/comedi/Kconfig
index 583bce9bb18e..9ab1ee7d36bf 100644
--- a/drivers/staging/comedi/Kconfig
+++ b/drivers/staging/comedi/Kconfig
@@ -1313,5 +1313,9 @@ config COMEDI_NI_LABPC_ISADMA
 
 config COMEDI_NI_TIO
tristate
+   select COMEDI_NI_ROUTING
+
+config COMEDI_NI_ROUTING
+   tristate
 
 endif # COMEDI
diff --git a/drivers/staging/comedi/drivers/Makefile 
b/drivers/staging/comedi/drivers/Makefile
index 8cb518190fc7..b24ac00cab73 100644
--- a/drivers/staging/comedi/drivers/Makefile
+++ b/drivers/staging/comedi/drivers/Makefile
@@ -137,6 +137,33 @@ obj-$(CONFIG_COMEDI_VMK80XX)   += vmk80xx.o
 obj-$(CONFIG_COMEDI_MITE)  += mite.o
 obj-$(CONFIG_COMEDI_NI_TIO)+= ni_tio.o
 obj-$(CONFIG_COMEDI_NI_TIOCMD) += ni_tiocmd.o
+obj-$(CONFIG_COMEDI_NI_ROUTING)+= ni_routing.o
+ni_routing-objs+= ni_routes.o \
+  ni_routing/ni_route_values.o \
+  ni_routing/ni_route_values/ni_660x.o 
\
+  
ni_routing/ni_route_values/ni_eseries.o \
+  
ni_routing/ni_route_values/ni_mseries.o \
+  ni_routing/ni_device_routes.o \
+  
ni_routing/ni_device_routes/pxi-6030e.o \
+  
ni_routing/ni_device_routes/pci-6070e.o \
+  
ni_routing/ni_device_routes/pci-6220.o \
+  
ni_routing/ni_device_routes/pci-6221.o \
+  
ni_routing/ni_device_routes/pxi-6224.o \
+  
ni_routing/ni_device_routes/pxi-6225.o \
+  
ni_routing/ni_device_routes/pci-6229.o \
+  
ni_routing/ni_device_routes/pci-6251.o \
+  
ni_routing/ni_device_routes/pxi-6251.o \
+  
ni_routing/ni_device_routes/pxie-6251.o \
+  
ni_routing/ni_device_routes/pci-6254.o \
+  
ni_routing/ni_device_routes/pci-6259.o \
+  
ni_routing/ni_device_routes/pci-6534.o \
+  
ni_routing/ni_device_routes/pxie-6535.o \
+  
ni_routing/ni_device_routes/pci-6602.o \
+  
ni_routing/ni_device_routes/pci-6713.o \
+  
ni_routing/ni_device_routes/pci-6723.o \
+  
ni_routing/ni_device_routes/pci-6733.o \
+  
ni_routing/ni_device_routes/pxi-6733.o \
+  
ni_routing/ni_device_routes/pxie-6738.o
 obj-$(CONFIG_COMEDI_NI_LABPC)  += ni_labpc_common.o
 obj-$(CONFIG_COMEDI_NI_LABPC_ISADMA)   += ni_labpc_isadma.o
 
diff --git a/drivers/staging/comedi/drivers/ni_routes.c 
b/drivers/staging/comedi/drivers/ni_routes.c
new file mode 100644
index ..eb61494dc2bd
--- /dev/null
+++ b/drivers/staging/comedi/drivers/ni_routes.c
@@ -0,0 +1,523 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* vim: set ts=8 sw=8 noet tw=80 nowrap: */
+/*
+ *  comedi/drivers/ni_routes.c
+ *  Route information for NI boards.
+ *
+ *  COMEDI - Linux Control and Measurement Device Interface
+ *  Copyright (C) 2016 Spencer E. Olson 
+ *
+ *  This program is free software; you can redist

[PATCH v4 10/13] staging: comedi: ni_mio_common: create device-global access to tio

[Spencer E. Olson]

Adds tio sub-devices of ni_mio_common supported hardware to the
implementation of test_route, connect_route, disconnect_route.  This change
delegates the actual functionality to the ni_tio module.

Signed-off-by: Spencer E. Olson 
---
 drivers/staging/comedi/drivers/ni_mio_common.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c 
b/drivers/staging/comedi/drivers/ni_mio_common.c
index d7862787e064..ecb05b3f9d35 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -5620,6 +5620,8 @@ static int get_output_select_source(int dest, struct 
comedi_device *dev)
/* there are only two g_out outputs. */
return -EINVAL;
reg = ni_get_gout_routing(dest, dev);
+   } else if (channel_is_ctr(dest)) {
+   reg = ni_tio_get_routing(devpriv->counter_dev, dest);
} else {
dev_dbg(dev->class_dev, "%s: unhandled destination (%d) 
queried\n",
__func__, dest);
@@ -5708,6 +5710,13 @@ static int connect_route(unsigned int src, unsigned int 
dest,
return -EINVAL;
if (ni_set_gout_routing(src, dest, dev))
return -EINVAL;
+   } else if (channel_is_ctr(dest)) {
+   /*
+* we are adding back the channel modifier info to set
+* invert/edge info passed by the user
+*/
+   ni_tio_set_routing(devpriv->counter_dev, dest,
+  reg | (src & ~CR_CHAN(-1)));
} else {
return -EINVAL;
}
@@ -5766,6 +5775,8 @@ static int disconnect_route(unsigned int src, unsigned 
int dest,
/* there are only two g_out outputs. */
return -EINVAL;
reg = ni_disable_gout_routing(dest, dev);
+   } else if (channel_is_ctr(dest)) {
+   ni_tio_unset_routing(devpriv->counter_dev, dest);
} else {
return -EINVAL;
}
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 06/13] staging: comedi: ni_mio_common: implement new routing for TRIG_EXT

[Spencer E. Olson]

Use new signal routing capability for all comedi command *_src == TRIG_EXT
options.  This new interface allows the user specify signals and terminals
as TRIG_EXT sources using a very consistent naming convention. Furthermore,
the interface allows backwards compatibility to prior behavior of
specifying register-level (or near register-level) values as *_arg options
when *_src == TRIG_EXT.

Annotates and updates tables of register values to reflect this new
implementation status.

Signed-off-by: Spencer E. Olson 
---
 .../staging/comedi/drivers/ni_mio_common.c| 106 +++---
 1 file changed, 66 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c 
b/drivers/staging/comedi/drivers/ni_mio_common.c
index 44fcb3790113..d8c715c42fd1 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -2006,7 +2006,6 @@ static int ni_ai_cmdtest(struct comedi_device *dev, 
struct comedi_subdevice *s,
const struct ni_board_struct *board = dev->board_ptr;
struct ni_private *devpriv = dev->private;
int err = 0;
-   unsigned int tmp;
unsigned int sources;
 
/* Step 1 : check if triggers are trivially valid */
@@ -2047,12 +2046,9 @@ static int ni_ai_cmdtest(struct comedi_device *dev, 
struct comedi_subdevice *s,
err |= comedi_check_trigger_arg_is(&cmd->start_arg, 0);
break;
case TRIG_EXT:
-   tmp = CR_CHAN(cmd->start_arg);
-
-   if (tmp > 16)
-   tmp = 16;
-   tmp |= (cmd->start_arg & (CR_INVERT | CR_EDGE));
-   err |= comedi_check_trigger_arg_is(&cmd->start_arg, tmp);
+   err |= ni_check_trigger_arg_roffs(CR_CHAN(cmd->start_arg),
+ NI_AI_StartTrigger,
+ &devpriv->routing_tables, 1);
break;
}
 
@@ -2064,12 +2060,9 @@ static int ni_ai_cmdtest(struct comedi_device *dev, 
struct comedi_subdevice *s,
0xff);
} else if (cmd->scan_begin_src == TRIG_EXT) {
/* external trigger */
-   unsigned int tmp = CR_CHAN(cmd->scan_begin_arg);
-
-   if (tmp > 16)
-   tmp = 16;
-   tmp |= (cmd->scan_begin_arg & (CR_INVERT | CR_EDGE));
-   err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, tmp);
+   err |= ni_check_trigger_arg_roffs(CR_CHAN(cmd->scan_begin_arg),
+ NI_AI_SampleClock,
+ &devpriv->routing_tables, 1);
} else {/* TRIG_OTHER */
err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
}
@@ -2087,12 +2080,9 @@ static int ni_ai_cmdtest(struct comedi_device *dev, 
struct comedi_subdevice *s,
}
} else if (cmd->convert_src == TRIG_EXT) {
/* external trigger */
-   unsigned int tmp = CR_CHAN(cmd->convert_arg);
-
-   if (tmp > 16)
-   tmp = 16;
-   tmp |= (cmd->convert_arg & (CR_ALT_FILTER | CR_INVERT));
-   err |= comedi_check_trigger_arg_is(&cmd->convert_arg, tmp);
+   err |= ni_check_trigger_arg_roffs(CR_CHAN(cmd->convert_arg),
+ NI_AI_ConvertClock,
+ &devpriv->routing_tables, 1);
} else if (cmd->convert_src == TRIG_NOW) {
err |= comedi_check_trigger_arg_is(&cmd->convert_arg, 0);
}
@@ -2118,7 +2108,7 @@ static int ni_ai_cmdtest(struct comedi_device *dev, 
struct comedi_subdevice *s,
/* step 4: fix up any arguments */
 
if (cmd->scan_begin_src == TRIG_TIMER) {
-   tmp = cmd->scan_begin_arg;
+   unsigned int tmp = cmd->scan_begin_arg;
cmd->scan_begin_arg =
ni_timer_to_ns(dev, ni_ns_to_timer(dev,
   cmd->scan_begin_arg,
@@ -2128,7 +2118,7 @@ static int ni_ai_cmdtest(struct comedi_device *dev, 
struct comedi_subdevice *s,
}
if (cmd->convert_src == TRIG_TIMER) {
if (!devpriv->is_611x && !devpriv->is_6143) {
-   tmp = cmd->convert_arg;
+   unsigned int tmp = cmd->convert_arg;
cmd->convert_arg =
ni_timer_to_ns(dev, ni_ns_to_timer(dev,
   cmd->convert_arg,
@@ -2206,8 +2196,10 @@ static int ni_ai_cmd(struct comedi_device *dev, struct 
comedi_subdevice *s)
   NISTC_AI_TRIG_START1_SEL(0);
break;
case TRIG_EXT:
-   ai_trig |= NISTC_AI_TRIG_START1_SEL(CR_CHA

[PATCH v4 03/13] staging: comedi: add new device-global config interface

[Spencer E. Olson]

Adds interface for configuring options that are global to all sub-devices.
For now, only options to configure device-globally identified signal routes
have been defined.

Signed-off-by: Spencer E. Olson 
---
 drivers/staging/comedi/comedi.h  | 18 
 drivers/staging/comedi/comedi_fops.c | 69 
 drivers/staging/comedi/comedidev.h   | 14 ++
 drivers/staging/comedi/drivers.c | 19 
 4 files changed, 120 insertions(+)

diff --git a/drivers/staging/comedi/comedi.h b/drivers/staging/comedi/comedi.h
index 6d7ad76bed97..a13c4b9cc569 100644
--- a/drivers/staging/comedi/comedi.h
+++ b/drivers/staging/comedi/comedi.h
@@ -107,6 +107,7 @@
 #define INSN_WRITE (1 | INSN_MASK_WRITE)
 #define INSN_BITS  (2 | INSN_MASK_READ | INSN_MASK_WRITE)
 #define INSN_CONFIG(3 | INSN_MASK_READ | INSN_MASK_WRITE)
+#define INSN_DEVICE_CONFIG (INSN_CONFIG | INSN_MASK_SPECIAL)
 #define INSN_GTOD  (4 | INSN_MASK_READ | INSN_MASK_SPECIAL)
 #define INSN_WAIT  (5 | INSN_MASK_WRITE | INSN_MASK_SPECIAL)
 #define INSN_INTTRIG   (6 | INSN_MASK_WRITE | INSN_MASK_SPECIAL)
@@ -347,6 +348,23 @@ enum configuration_ids {
INSN_CONFIG_PWM_GET_H_BRIDGE = 5004
 };
 
+/**
+ * enum device_configuration_ids - COMEDI configuration instruction codes 
global
+ * to an entire device.
+ * @INSN_DEVICE_CONFIG_TEST_ROUTE: Validate the possibility of a
+ * globally-named route
+ * @INSN_DEVICE_CONFIG_CONNECT_ROUTE:  Connect a globally-named route
+ * @INSN_DEVICE_CONFIG_DISCONNECT_ROUTE:Disconnect a globally-named route
+ * @INSN_DEVICE_CONFIG_GET_ROUTES: Get a list of all globally-named routes
+ * that are valid for a particular device.
+ */
+enum device_config_route_ids {
+   INSN_DEVICE_CONFIG_TEST_ROUTE = 0,
+   INSN_DEVICE_CONFIG_CONNECT_ROUTE = 1,
+   INSN_DEVICE_CONFIG_DISCONNECT_ROUTE = 2,
+   INSN_DEVICE_CONFIG_GET_ROUTES = 3,
+};
+
 /**
  * enum comedi_digital_trig_op - operations for configuring a digital trigger
  * @COMEDI_DIGITAL_TRIG_DISABLE:   Return digital trigger to its default,
diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index e18b61cdbdeb..5e7c5e71260f 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1230,6 +1230,57 @@ static int check_insn_config_length(struct comedi_insn 
*insn,
return -EINVAL;
 }
 
+static int check_insn_device_config_length(struct comedi_insn *insn,
+  unsigned int *data)
+{
+   if (insn->n < 1)
+   return -EINVAL;
+
+   switch (data[0]) {
+   case INSN_DEVICE_CONFIG_TEST_ROUTE:
+   case INSN_DEVICE_CONFIG_CONNECT_ROUTE:
+   case INSN_DEVICE_CONFIG_DISCONNECT_ROUTE:
+   if (insn->n == 3)
+   return 0;
+   break;
+   case INSN_DEVICE_CONFIG_GET_ROUTES:
+   /*
+* Big enough for config_id and the length of the userland
+* memory buffer.  Additional length should be in factors of 2
+* to communicate any returned route pairs (source,destination).
+*/
+   if (insn->n >= 2)
+   return 0;
+   break;
+   }
+   return -EINVAL;
+}
+
+/**
+ * get_valid_routes() - Calls low-level driver get_valid_routes function to
+ * either return a count of valid routes to user, or copy
+ * of list of all valid device routes to buffer in
+ * userspace.
+ * @dev: comedi device pointer
+ * @data: data from user insn call.  The length of the data must be >= 2.
+ *   data[0] must contain the INSN_DEVICE_CONFIG config_id.
+ *   data[1](input) contains the number of _pairs_ for which memory is
+ *   allotted from the user.  If the user specifies '0', then only
+ *   the number of pairs available is returned.
+ *   data[1](output) returns either the number of pairs available (if none
+ *   where requested) or the number of _pairs_ that are copied back
+ *   to the user.
+ *   data[2::2] returns each (source, destination) pair.
+ *
+ * Return: -EINVAL if low-level driver does not allocate and return routes as
+ *expected.  Returns 0 otherwise.
+ */
+static int get_valid_routes(struct comedi_device *dev, unsigned int *data)
+{
+   data[1] = dev->get_valid_routes(dev, data[1], data + 2);
+   return 0;
+}
+
 static int parse_insn(struct comedi_device *dev, struct comedi_insn *insn,
  unsigned int *data, void *file)
 {
@@ -1293,6 +1344,24 @@ static int parse_insn(struct comedi_device *dev, struct 
comedi_insn *insn,
if (ret >= 0)
ret = 1;
break;
+

[PATCH v4 02/13] staging: comedi: add abstracted NI signal/terminal named constants

[Spencer E. Olson]

This change adds abstracted constants for National Instruments
terminal/signal names.

Some background:
  There have been significant confusions over the past many years for users
  when trying to understand how to connect to/from signals and terminals on
  NI hardware using comedi.  The major reason for this is that the actual
  register values were exposed and required to be used by users.  Several
  major reasons exist why this caused major confusion for users:

  1) The register values are _NOT_ in user documentation, but rather in
arcane locations, such as a few register programming manuals that are
increasingly hard to find and the NI-MHDDK (comments in in example
code).  There is no one place to find the various valid values of the
registers.

  2) The register values are _NOT_ completely consistent.  There is no way
to gain any sense of intuition of which values, or even enums one
should use for various registers.  There was some attempt in prior use
of comedi to name enums such that a user might know which enums should
be used for varying purposes, but the end-user had to gain a knowledge
of register values to correctly wield this approach.

  3) The names for signals and registers found in the various register
level programming manuals and vendor-provided documentation are _not_
even close to the same names that are in the end-user documentation.

Similar confusion, albeit less, plagued NI's previous version of their own
proprietary drivers.  Earlier than 2003, NI greatly simplified the
situation for users by releasing a new API that abstracted the names of
signals/terminals to a common and intuitive set of names.  In addition,
this new API provided a much more common interface to use for most of NI
hardware.

The names added here mirror the names chosen and well documented by NI.
These names are exposed to the user via the comedilib user library.  By
keeping the names in this format, in spite of the use of CamelScript,
maintenance will be greatly eased and confusion for users _and_ comedi
developers will be greatly reduced.

Signed-off-by: Spencer E. Olson 
---

Patch revisions:
  - [PATCH v2 02/13]: Update signal/terminal names found after adding additional
devices to routing list in [PATCH v2 04/13].

 drivers/staging/comedi/comedi.h | 151 
 1 file changed, 151 insertions(+)

diff --git a/drivers/staging/comedi/comedi.h b/drivers/staging/comedi/comedi.h
index bb961ac79b7e..6d7ad76bed97 100644
--- a/drivers/staging/comedi/comedi.h
+++ b/drivers/staging/comedi/comedi.h
@@ -928,6 +928,157 @@ enum i8254_mode {
I8254_BINARY = 0
 };
 
+/* *** BEGIN GLOBALLY-NAMED NI TERMINALS/SIGNALS *** */
+
+/*
+ * Common National Instruments Terminal/Signal names.
+ * Some of these have no NI_ prefix as they are useful for non-NI hardware, 
such
+ * as those that utilize the PXI/RTSI trigger lines.
+ *
+ * NOTE ABOUT THE CHOICE OF NAMES HERE AND THE CAMELSCRIPT:
+ *   The choice to use CamelScript and the exact names below is for
+ *   maintainability, clarity, similarity to manufacturer's documentation,
+ *   _and_ a mitigation for confusion that has plagued the use of these drivers
+ *   for years!
+ *
+ *   More detail:
+ *   There have been significant confusions over the past many years for users
+ *   when trying to understand how to connect to/from signals and terminals on
+ *   NI hardware using comedi.  The major reason for this is that the actual
+ *   register values were exposed and required to be used by users.  Several
+ *   major reasons exist why this caused major confusion for users:
+ *   1) The register values are _NOT_ in user documentation, but rather in
+ * arcane locations, such as a few register programming manuals that are
+ * increasingly hard to find and the NI MHDDK (comments in in example 
code).
+ * There is no one place to find the various valid values of the registers.
+ *   2) The register values are _NOT_ completely consistent.  There is no way 
to
+ * gain any sense of intuition of which values, or even enums one should 
use
+ * for various registers.  There was some attempt in prior use of comedi to
+ * name enums such that a user might know which enums should be used for
+ * varying purposes, but the end-user had to gain a knowledge of register
+ * values to correctly wield this approach.
+ *   3) The names for signals and registers found in the various register level
+ * programming manuals and vendor-provided documentation are _not_ even
+ * close to the same names that are in the end-user documentation.
+ *
+ *   Similar, albeit less, confusion plagued NI's previous version of their own
+ *   drivers.  Earlier than 2003, NI greatly simplified the situation for users
+ *   by releasing a new API that abstracted the names of signals/terminals to a
+ *   common and intuitive set of names.
+ *
+ *   The names below mirror the names chosen and well docu

[PATCH v4 01/13] staging: comedi: tests: add unittest framework for comedi

[Spencer E. Olson]

Adds a framework for unittests for comedi drivers.  It was certainly
possible to write some unit tests before and test various aspects of a
particular driver, but this framework makes this a bit easier and hopefully
inspires more unittest modules to be written.

Signed-off-by: Spencer E. Olson 
---
 drivers/staging/comedi/drivers/Makefile   |  1 +
 drivers/staging/comedi/drivers/tests/Makefile |  6 ++
 .../comedi/drivers/tests/example_test.c   | 72 +++
 .../staging/comedi/drivers/tests/unittest.h   | 63 
 4 files changed, 142 insertions(+)
 create mode 100644 drivers/staging/comedi/drivers/tests/Makefile
 create mode 100644 drivers/staging/comedi/drivers/tests/example_test.c
 create mode 100644 drivers/staging/comedi/drivers/tests/unittest.h

diff --git a/drivers/staging/comedi/drivers/Makefile 
b/drivers/staging/comedi/drivers/Makefile
index 98b42b47dfe1..8cb518190fc7 100644
--- a/drivers/staging/comedi/drivers/Makefile
+++ b/drivers/staging/comedi/drivers/Makefile
@@ -145,3 +145,4 @@ obj-$(CONFIG_COMEDI_8255_SA)+= 8255.o
 obj-$(CONFIG_COMEDI_AMPLC_DIO200)  += amplc_dio200_common.o
 obj-$(CONFIG_COMEDI_AMPLC_PC236)   += amplc_pc236_common.o
 obj-$(CONFIG_COMEDI_DAS08) += das08.o
+obj-$(CONFIG_COMEDI_TESTS) += tests/
diff --git a/drivers/staging/comedi/drivers/tests/Makefile 
b/drivers/staging/comedi/drivers/tests/Makefile
new file mode 100644
index ..1d58ede0bdf6
--- /dev/null
+++ b/drivers/staging/comedi/drivers/tests/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+# Makefile for comedi drivers unit tests
+#
+ccflags-$(CONFIG_COMEDI_DEBUG) := -DDEBUG
+
+obj-$(CONFIG_COMEDI_TESTS) += example_test.o
diff --git a/drivers/staging/comedi/drivers/tests/example_test.c 
b/drivers/staging/comedi/drivers/tests/example_test.c
new file mode 100644
index ..fc65158b8e8e
--- /dev/null
+++ b/drivers/staging/comedi/drivers/tests/example_test.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* vim: set ts=8 sw=8 noet tw=80 nowrap: */
+/*
+ *  comedi/drivers/tests/example_test.c
+ *  Example set of unit tests.
+ *
+ *  COMEDI - Linux Control and Measurement Device Interface
+ *  Copyright (C) 2016 Spencer E. Olson 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ */
+
+#include 
+
+#include "unittest.h"
+
+/* *** BEGIN fake board data *** */
+struct comedi_device {
+   const char *board_name;
+   int item;
+};
+
+static struct comedi_device dev = {
+   .board_name = "fake_device",
+};
+
+/* *** END fake board data *** */
+
+/* *** BEGIN fake data init *** */
+void init_fake(void)
+{
+   dev.item = 10;
+}
+
+/* *** END fake data init *** */
+
+void test0(void)
+{
+   init_fake();
+   unittest(dev.item != 11, "negative result\n");
+   unittest(dev.item == 10, "positive result\n");
+}
+
+/*  BEGIN simple module entry/exit functions  */
+static int __init unittest_enter(void)
+{
+   const unittest_fptr unit_tests[] = {
+   (unittest_fptr)test0,
+   NULL,
+   };
+
+   exec_unittests("example", unit_tests);
+   return 0;
+}
+
+static void __exit unittest_exit(void) { }
+
+module_init(unittest_enter);
+module_exit(unittest_exit);
+
+MODULE_AUTHOR("Spencer Olson ");
+MODULE_DESCRIPTION("Comedi unit-tests example");
+MODULE_LICENSE("GPL");
+/*  END simple module entry/exit functions  */
diff --git a/drivers/staging/comedi/drivers/tests/unittest.h 
b/drivers/staging/comedi/drivers/tests/unittest.h
new file mode 100644
index ..b8e622ea1de1
--- /dev/null
+++ b/drivers/staging/comedi/drivers/tests/unittest.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* vim: set ts=8 sw=8 noet tw=80 nowrap: */
+/*
+ *  comedi/drivers/tests/unittest.h
+ *  Simple framework for unittests for comedi drivers.
+ *
+ *  COMEDI - Linux Control and Measurement Device Interface
+ *  Copyright (C) 2016 Spencer E. Olson 
+ *  based of parts of drivers/of/unittest.c
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GN

[PATCH v4 00/13] device-global identifiers and routes introduced

[Spencer E. Olson]

This patch set is the second revision of a recent patch set of the same name.
Changes and notes:
  - [PATCH v4 07/13]: Rebased patchset on repaired patch "staging: comedi:
ni_mio_common: protect register write overflow" (that patch had a missing
brace and this patch depends on that earlier patch).

  - [PATCH v3 04/13]: Minor update in indentation for support tool.
  - [PATCH v3 05/13]: Simplify and clean up prototypes of functions for use with
besearch.

  - [PATCH v2 02/13]: Update signal/terminal names found after adding additional
devices to routing list in [PATCH v2 04/13].
  - [PATCH v2 04/13]: Add routing information for PXIe-6535 and PXIe-6738
devices.
  - [PATCH v2 04/13]: Implements Ian's suggestion to break up components of new
ni_routing module into multiple compile units so that .c files are not
included from .c files.
  - [PATCH v2 04/13]: Fixes various function prototypes and "const" variable
declarations as per Ian's suggestions.
  - [PATCH v2 05/13]: Tweak Makefile to build routing info for newly added
hardware in updates to [PATCH v2 04/13].
  - [PATCH v2 05/13]: Fixes placement of "select COMEDI_NI_ROUTING" to ensure
ni_routing module is enabled for all dependent modules.
  - [PATCH v2 05/13]: Removes a few inline function declarations in unit test.
  - [PATCH v2 07/13]: This patch must be built upon an earlier patch recently
submitted and in the queue for integration:
"staging: comedi: ni_mio_common: protect register write overflow"

--

This patchset introduces a new framework for providing and maintaining a
consistent namespace to define terminal/signal names for a set of comedi
devices.  This effort was primarily focused on supporting NI hardware, but the
interfaces introduced here can be implemented by all other hardware drivers, if
desired.  Otherwise, these new interfaces do not effect any interfaces
previously defined or prior use cases (i.e. backwards compatibility).

Some background:
  There have been significant confusions over the past many years for users
  when trying to understand how to connect to/from signals and terminals on
  NI hardware using comedi.  The major reason for this is that the actual
  register values were exposed and required to be used by end users.  Several
  major reasons exist why this caused major confusion for users:

  1) The register values are _NOT_ in user documentation, but rather in
arcane locations, such as a few register programming manuals that are
increasingly hard to find.  Some information is found in the register level
programming libraries provided by National Instruments (NI-MHDDK), but
many items are only vaguely found/mentioned in the comments of the NI-MHDDK
example code.  There is no one place to find the various valid values of the
registers.

  2) The register values are _NOT_ completely consistent.  There is no way to
gain any sense of intuition of which values, or even enums one should use
for various registers.  There was some attempt in prior use of comedi to
name enums such that a user might know which enums should be used for
varying purposes, but the end-user had to gain a knowledge of register
values to correctly wield this approach.

  3) The names for signals and registers found in the various register level
programming manuals and vendor-provided documentation are _not_ even
close to the same names that are in the end-user documentation.

  4) The sets of routes that are valid are not consistent from device to device.
One additional major challenge is that this information is not documented
and does not seem to be obtainable in any programmatic fashion, neither
through the proprietary NIDAQmx(-base) c-libraries, nor with register level
programming.  In fact, the only consistent source of this information is
through the proprietary NI-MAX software, which currently only runs on
Windows platforms.  A further challenge is that this information cannot be
exported from NI-MAX, except by screenshot.

Similar confusion, albeit less, plagued NI's previous version of their own
proprietary drivers.  Earlier than 2003, NI greatly simplified the situation for
users by releasing a new API that abstracted the names of signals/terminals to a
common and intuitive set of names.  In addition, this new API provided a much
more common interface to use for most of NI hardware.

Comedi already provides such a common interface for data-acquisition and control
hardware.  This effort complements comedi's abstraction layers by further
abstracting much more of the use cases for NI hardware, but allowing users _and_
developers to directly refer to NI documentation (user-level, register-level,
and the register-level examples of the NI-MHDDK).

The goal of these patches are:
  0) Allow current code to function as is, providing backwards compatibility to
the current interface, following a suggestion by Eric Piel.
  1) P

[PATCH v2] staging: comedi: ni_mio_common: protect register write overflow

[Spencer E. Olson]

Fixes two problems introduced as early as
commit 03aef4b6dc12  ("Staging: comedi: add ni_mio_common code"):
(1) Ensures that the last four bits of NISTC_RTSI_TRIGB_OUT_REG register is
not unduly overwritten on e-series devices.  On e-series devices, the
first three of the last four bits are reserved.  The last bit defines
the output selection of the RGOUT0 pin, otherwise known as
RTSI_Sub_Selection.  For m-series devices, these last four bits are
indeed used as the output selection of the RTSI7 pin (and the
RTSI_Sub_Selection bit for the RGOUT0 pin is moved to the
RTSI_Trig_Direction register.
(2) Allows all 4 RTSI_BRD lines to be treated as valid sources for RTSI
lines.

This patch also cleans up the ni_get_rtsi_routing command for readability.

Fixes: 03aef4b6dc12  ("Staging: comedi: add ni_mio_common code")
Signed-off-by: Spencer E. Olson 
---

Changes:
  - [PATCH v2]: Adds missing brace that had been corrected in a later patchset
but should have been correct here.  This patch is now independent and
complete.

 .../staging/comedi/drivers/ni_mio_common.c| 24 +--
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c 
b/drivers/staging/comedi/drivers/ni_mio_common.c
index 4dee2fc37aed..44fcb3790113 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -4980,7 +4980,10 @@ static int ni_valid_rtsi_output_source(struct 
comedi_device *dev,
case NI_RTSI_OUTPUT_G_SRC0:
case NI_RTSI_OUTPUT_G_GATE0:
case NI_RTSI_OUTPUT_RGOUT0:
-   case NI_RTSI_OUTPUT_RTSI_BRD_0:
+   case NI_RTSI_OUTPUT_RTSI_BRD(0):
+   case NI_RTSI_OUTPUT_RTSI_BRD(1):
+   case NI_RTSI_OUTPUT_RTSI_BRD(2):
+   case NI_RTSI_OUTPUT_RTSI_BRD(3):
return 1;
case NI_RTSI_OUTPUT_RTSI_OSC:
return (devpriv->is_m_series) ? 1 : 0;
@@ -5001,11 +5004,18 @@ static int ni_set_rtsi_routing(struct comedi_device 
*dev,
devpriv->rtsi_trig_a_output_reg |= NISTC_RTSI_TRIG(chan, src);
ni_stc_writew(dev, devpriv->rtsi_trig_a_output_reg,
  NISTC_RTSI_TRIGA_OUT_REG);
-   } else if (chan < 8) {
+   } else if (chan < NISTC_RTSI_TRIG_NUM_CHAN(devpriv->is_m_series)) {
devpriv->rtsi_trig_b_output_reg &= ~NISTC_RTSI_TRIG_MASK(chan);
devpriv->rtsi_trig_b_output_reg |= NISTC_RTSI_TRIG(chan, src);
ni_stc_writew(dev, devpriv->rtsi_trig_b_output_reg,
  NISTC_RTSI_TRIGB_OUT_REG);
+   } else if (chan != NISTC_RTSI_TRIG_OLD_CLK_CHAN) {
+   /* probably should never reach this, since the
+* ni_valid_rtsi_output_source above errors out if chan is too
+* high
+*/
+   dev_err(dev->class_dev, "%s: unknown rtsi channel\n", __func__);
+   return -EINVAL;
}
return 2;
 }
@@ -5021,12 +5031,12 @@ static unsigned int ni_get_rtsi_routing(struct 
comedi_device *dev,
} else if (chan < NISTC_RTSI_TRIG_NUM_CHAN(devpriv->is_m_series)) {
return NISTC_RTSI_TRIG_TO_SRC(chan,
  devpriv->rtsi_trig_b_output_reg);
-   } else {
-   if (chan == NISTC_RTSI_TRIG_OLD_CLK_CHAN)
-   return NI_RTSI_OUTPUT_RTSI_OSC;
-   dev_err(dev->class_dev, "bug! should never get here?\n");
-   return 0;
+   } else if (chan == NISTC_RTSI_TRIG_OLD_CLK_CHAN) {
+   return NI_RTSI_OUTPUT_RTSI_OSC;
}
+
+   dev_err(dev->class_dev, "%s: unknown rtsi channel\n", __func__);
+   return -EINVAL;
 }
 
 static int ni_rtsi_insn_config(struct comedi_device *dev,
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/3] staging: rtl8188eu: fix line over 80 characters - style

[Michael Straube]

Line break array declaration to clear a 'line over 80 characters'
checkpatch warning. For consistency replace 0x0 with 0x00.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index 4d55bbdf8fb7..f6171f07599d 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -39,7 +39,10 @@ extern unsigned char REALTEK_96B_IE[];
 /
 MCS rate definitions
 */
-unsigned char  MCS_rate_1R[16] = {0xff, 0x00, 0x0, 0x0, 0x01, 0x0, 0x0, 0x0, 
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0};
+unsigned char MCS_rate_1R[16] = {
+   0xff, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
+   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
 
 /
 ChannelPlan definitions
-- 
2.19.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/3] staging: rtl8188eu: rewrite if tests - style

[Michael Straube]

Rewrite if tests to clear a 'line over 80 characters' and
'Comparisons should place the constant on the right side of the test'
checkpatch warning.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index f03ac89736de..c6945164ee34 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -3957,7 +3957,7 @@ static void init_channel_list(struct adapter *padapter,
if (!has_channel(channel_set, chanset_size, ch))
continue;
 
-   if ((0 == padapter->registrypriv.ht_enable) && (8 == 
o->inc))
+   if (!padapter->registrypriv.ht_enable && o->inc == 8)
continue;
 
if ((0 == (padapter->registrypriv.cbw40_enable & 
BIT(1))) &&
-- 
2.19.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/3] staging: rtl8188eu: cleanup array declaration - style

[Michael Straube]

Cleanup array declaration to clear two 'line over 80 characters'
checkpatch warnings and improve readability.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 32 ---
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index f6171f07599d..f03ac89736de 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -3836,24 +3836,20 @@ Following are the initialization functions for WiFi MLME
 */
 
 static struct mlme_handler mlme_sta_tbl[] = {
-   {WIFI_ASSOCREQ, "OnAssocReq",   &OnAssocReq},
-   {WIFI_ASSOCRSP, "OnAssocRsp",   &OnAssocRsp},
-   {WIFI_REASSOCREQ,   "OnReAssocReq", &OnAssocReq},
-   {WIFI_REASSOCRSP,   "OnReAssocRsp", &OnAssocRsp},
-   {WIFI_PROBEREQ, "OnProbeReq",   &OnProbeReq},
-   {WIFI_PROBERSP, "OnProbeRsp",   &OnProbeRsp},
-
-   /*--
-   below 2 are reserved
-   ---*/
-   {0, "DoReserved",   
&DoReserved},
-   {0, "DoReserved",   
&DoReserved},
-   {WIFI_BEACON,   "OnBeacon", &OnBeacon},
-   {WIFI_ATIM, "OnATIM",   &OnAtim},
-   {WIFI_DISASSOC, "OnDisassoc",   &OnDisassoc},
-   {WIFI_AUTH, "OnAuth",   &OnAuthClient},
-   {WIFI_DEAUTH,   "OnDeAuth", &OnDeAuth},
-   {WIFI_ACTION,   "OnAction", &OnAction},
+   {WIFI_ASSOCREQ,   "OnAssocReq",   &OnAssocReq},
+   {WIFI_ASSOCRSP,   "OnAssocRsp",   &OnAssocRsp},
+   {WIFI_REASSOCREQ, "OnReAssocReq", &OnAssocReq},
+   {WIFI_REASSOCRSP, "OnReAssocRsp", &OnAssocRsp},
+   {WIFI_PROBEREQ,   "OnProbeReq",   &OnProbeReq},
+   {WIFI_PROBERSP,   "OnProbeRsp",   &OnProbeRsp},
+   {0,   "DoReserved",   &DoReserved},
+   {0,   "DoReserved",   &DoReserved},
+   {WIFI_BEACON, "OnBeacon", &OnBeacon},
+   {WIFI_ATIM,   "OnATIM",   &OnAtim},
+   {WIFI_DISASSOC,   "OnDisassoc",   &OnDisassoc},
+   {WIFI_AUTH,   "OnAuth",   &OnAuthClient},
+   {WIFI_DEAUTH, "OnDeAuth", &OnDeAuth},
+   {WIFI_ACTION, "OnAction", &OnAction},
 };
 
 int init_hw_mlme_ext(struct adapter *padapter)
-- 
2.19.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: staging: android: ion: aligned allocation support

[Alexey Skidanov]

On 10/03/2018 09:07 PM, Laura Abbott wrote:
> On 10/02/2018 07:27 AM, Alexey Skidanov wrote:
>> Hi,
>>
>> Sometimes HW requires memory buffer to be aligned in order to be used
>> properly.  Of course, we may overcome the lack of aligned allocation
>> support, but we may easily add it because CMA and gen_pool (used by
>> several heaps) already support it.
>>
>> Does someone have an objection to add it?
>>
>> Thanks,
>> Alexey
>>
> 
> The alignment option was removed from the allocation API before
> because the most common heap (system heap) didn't support it
> and it was causing more confusion. We've already mangled the
> ABI once so I really don't want to break it again. I'm not
> opposed to adding alignment support for the CMA via the allocation
> flags. 
Currently, the flags member is used to define the way the buffer will be
mapped - cached or uncached. So,if I understand you correct, we need to
add ION_FLAG_ALIGNED flag and to share 32 bit field between flags and
flags specific data (alignment value) ?

I'm probably going to remove the carveout and chunk heap because
> nobody has stepped up to figure out how to tie allocation of those
> to device tree or another method.
> 
> Thanks,
> Laura

Thanks,
Alexey
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [patch 00/11] x86/vdso: Cleanups, simmplifications and CLOCK_TAI support\

[Marcelo Tosatti]

On Wed, Oct 03, 2018 at 04:00:29PM -0300, Marcelo Tosatti wrote:
> On Tue, Oct 02, 2018 at 10:15:49PM -0700, Andy Lutomirski wrote:
> > Hi Vitaly, Paolo, Radim, etc.,
> > 
> > On Fri, Sep 14, 2018 at 5:52 AM Thomas Gleixner  wrote:
> > >
> > > Matt attempted to add CLOCK_TAI support to the VDSO clock_gettime()
> > > implementation, which extended the clockid switch case and added yet
> > > another slightly different copy of the same code.
> > >
> > > Especially the extended switch case is problematic as the compiler tends 
> > > to
> > > generate a jump table which then requires to use retpolines. If jump 
> > > tables
> > > are disabled it adds yet another conditional to the existing maze.
> > >
> > > This series takes a different approach by consolidating the almost
> > > identical functions into one implementation for high resolution clocks and
> > > one for the coarse grained clock ids by storing the base data for each
> > > clock id in an array which is indexed by the clock id.
> > >
> > 
> > I was trying to understand more of the implications of this patch
> > series, and I was again reminded that there is an entire extra copy of
> > the vclock reading code in arch/x86/kvm/x86.c.  And the purpose of
> > that code is very, very opaque.
> > 
> > Can one of you explain what the code is even doing?  From a couple of
> > attempts to read through it, it's a whole bunch of
> > probably-extremely-buggy code that, 
> 
> Yes, probably.
> 
> > drumroll please, tries to atomically read the TSC value and the time.  And 
> > decide whether the
> > result is "based on the TSC".  
> 
> I think "based on the TSC" refers to whether TSC clocksource is being
> used.
> 
> > And then synthesizes a TSC-to-ns
> > multiplier and shift, based on *something other than the actual
> > multiply and shift used*.
> > 
> > IOW, unless I'm totally misunderstanding it, the code digs into the
> > private arch clocksource data intended for the vDSO, uses a poorly
> > maintained copy of the vDSO code to read the time (instead of doing
> > the sane thing and using the kernel interfaces for this), and
> > propagates a totally made up copy to the guest.
> 
> I posted kernel interfaces for this, and it was suggested to 
> instead write a "in-kernel user of pvclock data".
> 
> If you can get kernel interfaces to replace that, go for it. I prefer
> kernel interfaces as well.

And cleanup patches, to make that code look nicer, are also very
welcome!

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [patch 00/11] x86/vdso: Cleanups, simmplifications and CLOCK_TAI support

[Marcelo Tosatti]

On Wed, Oct 03, 2018 at 11:22:58AM +0200, Vitaly Kuznetsov wrote:
> Andy Lutomirski  writes:
> 
> > Hi Vitaly, Paolo, Radim, etc.,
> >
> > On Fri, Sep 14, 2018 at 5:52 AM Thomas Gleixner  wrote:
> >>
> >> Matt attempted to add CLOCK_TAI support to the VDSO clock_gettime()
> >> implementation, which extended the clockid switch case and added yet
> >> another slightly different copy of the same code.
> >>
> >> Especially the extended switch case is problematic as the compiler tends to
> >> generate a jump table which then requires to use retpolines. If jump tables
> >> are disabled it adds yet another conditional to the existing maze.
> >>
> >> This series takes a different approach by consolidating the almost
> >> identical functions into one implementation for high resolution clocks and
> >> one for the coarse grained clock ids by storing the base data for each
> >> clock id in an array which is indexed by the clock id.
> >>
> >
> > I was trying to understand more of the implications of this patch
> > series, and I was again reminded that there is an entire extra copy of
> > the vclock reading code in arch/x86/kvm/x86.c.  And the purpose of
> > that code is very, very opaque.
> >
> > Can one of you explain what the code is even doing?  From a couple of
> > attempts to read through it, it's a whole bunch of
> > probably-extremely-buggy code that, drumroll please, tries to
> > atomically read the TSC value and the time.  And decide whether the
> > result is "based on the TSC".  And then synthesizes a TSC-to-ns
> > multiplier and shift, based on *something other than the actual
> > multiply and shift used*.
> >
> > IOW, unless I'm totally misunderstanding it, the code digs into the
> > private arch clocksource data intended for the vDSO, uses a poorly
> > maintained copy of the vDSO code to read the time (instead of doing
> > the sane thing and using the kernel interfaces for this), and
> > propagates a totally made up copy to the guest.  And gets it entirely
> > wrong when doing nested virt, since, unless there's some secret in
> > this maze, it doesn't acutlaly use the scaling factor from the host
> > when it tells the guest what to do.
> >
> > I am really, seriously tempted to send a patch to simply delete all
> > this code.  The correct way to do it is to hook
> 
> "I have discovered a truly marvelous proof of this, which this margin is
> too narrow to contain" :-)
> 
> There is a very long history of different (hardware) issues Marcelo was
> fighting with and the current code is the survived Frankenstein.

Right, the code has to handle different TSC modes.

>  E.g. it
> is very, very unclear what "catchup", "always catchup" and
> masterclock-less mode in general are and if we still need them.

Catchup means handling exposed (to guest) TSC frequency smaller than
HW TSC frequency.

That is "frankenstein" code, could be removed.

> That said I'm all for simplification. I'm not sure if we still need to
> care about buggy hardware though.

What simplification is that again? 


> >
> > And I don't see how it's even possible to pass kvmclock correctly to
> > the L2 guest when L0 is hyperv.  KVM could pass *hyperv's* clock, but
> > L1 isn't notified when the data structure changes, so how the heck is
> > it supposed to update the kvmclock structure?
> 
> Well, this kind of works in the the followin way:
> L1's clocksource is 'tsc_page' which is, basically, a compliment to TSC:
> two numbers provided by L0: offset and scale and KVM was tought to treat
> this clocksource as a good one (see b0c39dc68e3b "x86/kvm: Pass stable
> clocksource to guests when running nested on Hyper-V").
> 
> The notification you're talking about exists, it is called
> Reenligntenment, see 0092e4346f49 "x86/kvm: Support Hyper-V
> reenlightenment"). When TSC page changes (and this only happens when L1
> is migrated to a different host with a different TSC frequency and TSC
> scaling is not supported by the CPU) we receive an interrupt in L1 (at
> this moment all TSC accesses are emulated which guarantees the
> correctness of the readings), pause all L2 guests, update their kvmclock
> structures with new data (we already know the new TSC frequency) and
> then tell L0 that we're done and it can stop emulating TSC accesses.
> 
> (Nothing like this exists for KVM-on-KVM, by the way, when L1's
> clocksource is 'kvmclock' L2s won't get a stable kvmclock clocksource.)
> 
> -- 
> Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [patch 00/11] x86/vdso: Cleanups, simmplifications and CLOCK_TAI support

[Marcelo Tosatti]

On Tue, Oct 02, 2018 at 10:15:49PM -0700, Andy Lutomirski wrote:
> Hi Vitaly, Paolo, Radim, etc.,
> 
> On Fri, Sep 14, 2018 at 5:52 AM Thomas Gleixner  wrote:
> >
> > Matt attempted to add CLOCK_TAI support to the VDSO clock_gettime()
> > implementation, which extended the clockid switch case and added yet
> > another slightly different copy of the same code.
> >
> > Especially the extended switch case is problematic as the compiler tends to
> > generate a jump table which then requires to use retpolines. If jump tables
> > are disabled it adds yet another conditional to the existing maze.
> >
> > This series takes a different approach by consolidating the almost
> > identical functions into one implementation for high resolution clocks and
> > one for the coarse grained clock ids by storing the base data for each
> > clock id in an array which is indexed by the clock id.
> >
> 
> I was trying to understand more of the implications of this patch
> series, and I was again reminded that there is an entire extra copy of
> the vclock reading code in arch/x86/kvm/x86.c.  And the purpose of
> that code is very, very opaque.
> 
> Can one of you explain what the code is even doing?  From a couple of
> attempts to read through it, it's a whole bunch of
> probably-extremely-buggy code that, 

Yes, probably.

> drumroll please, tries to atomically read the TSC value and the time.  And 
> decide whether the
> result is "based on the TSC".  

I think "based on the TSC" refers to whether TSC clocksource is being
used.

> And then synthesizes a TSC-to-ns
> multiplier and shift, based on *something other than the actual
> multiply and shift used*.
> 
> IOW, unless I'm totally misunderstanding it, the code digs into the
> private arch clocksource data intended for the vDSO, uses a poorly
> maintained copy of the vDSO code to read the time (instead of doing
> the sane thing and using the kernel interfaces for this), and
> propagates a totally made up copy to the guest.

I posted kernel interfaces for this, and it was suggested to 
instead write a "in-kernel user of pvclock data".

If you can get kernel interfaces to replace that, go for it. I prefer
kernel interfaces as well.

>  And gets it entirely
> wrong when doing nested virt, since, unless there's some secret in
> this maze, it doesn't acutlaly use the scaling factor from the host
> when it tells the guest what to do.
> 
> I am really, seriously tempted to send a patch to simply delete all
> this code.  

If your patch which deletes the code gets the necessary features right,
sure, go for it.

> The correct way to do it is to hook

Can you expand on the correct way to do it?

> And I don't see how it's even possible to pass kvmclock correctly to
> the L2 guest when L0 is hyperv.  KVM could pass *hyperv's* clock, but
> L1 isn't notified when the data structure changes, so how the heck is
> it supposed to update the kvmclock structure?

I don't parse your question.

> 
> --Andy
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/6] Staging: rts5208: rtsx_card.c: Fixed all braces issues of the file

[Greg KH]

On Tue, Oct 02, 2018 at 07:11:26PM -0400, Maxime Desroches wrote:
> Fixed all the braces issues of the rtsx_card.c file
> 
> Signed-off-by: Maxime Desroches 
> ---
>  drivers/staging/rts5208/rtsx_card.c | 96 +++--
>  1 file changed, 37 insertions(+), 59 deletions(-)

None of the patches in this series applied to my staging-next tree at
all :(

Always work against either linux-next, or my staging-next tree when
submitting patches so you do not duplicate work others have already
done.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: staging: android: ion: aligned allocation support

[Laura Abbott]

On 10/02/2018 07:27 AM, Alexey Skidanov wrote:

Hi,

Sometimes HW requires memory buffer to be aligned in order to be used
properly.  Of course, we may overcome the lack of aligned allocation
support, but we may easily add it because CMA and gen_pool (used by
several heaps) already support it.

Does someone have an objection to add it?

Thanks,
Alexey



The alignment option was removed from the allocation API before
because the most common heap (system heap) didn't support it
and it was causing more confusion. We've already mangled the
ABI once so I really don't want to break it again. I'm not
opposed to adding alignment support for the CMA via the allocation
flags. I'm probably going to remove the carveout and chunk heap because
nobody has stepped up to figure out how to tie allocation of those
to device tree or another method.

Thanks,
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4] staging: android: ion: Add per-heap counters

[Laura Abbott]

On 09/30/2018 08:24 AM, Alexey Skidanov wrote:

Heap statistics have been removed and currently even basics statistics
are missing.

This patch creates per heap debugfs directory /sys/kernel/debug/
and adds the following counters:
- the number of allocated buffers;
- the number of allocated bytes;
- the number of allocated bytes watermark.



If none of the other Android people have strong opinions

Acked-by: Laura Abbott 


Signed-off-by: Alexey Skidanov 
---

v3:
Removed debugfs_create_dir() return value checking
v4:
Added spinlock to protect heap statistics

  drivers/staging/android/ion/ion.c | 50 ---
  drivers/staging/android/ion/ion.h | 10 +++-
  2 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index 9907332..6fd8979 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -95,6 +95,13 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap 
*heap,
goto err1;
}
  
+	spin_lock(&heap->stat_lock);

+   heap->num_of_buffers++;
+   heap->num_of_alloc_bytes += len;
+   if (heap->num_of_alloc_bytes > heap->alloc_bytes_wm)
+   heap->alloc_bytes_wm = heap->num_of_alloc_bytes;
+   spin_unlock(&heap->stat_lock);
+
INIT_LIST_HEAD(&buffer->attachments);
mutex_init(&buffer->lock);
mutex_lock(&dev->buffer_lock);
@@ -117,6 +124,11 @@ void ion_buffer_destroy(struct ion_buffer *buffer)
buffer->heap->ops->unmap_kernel(buffer->heap, buffer);
}
buffer->heap->ops->free(buffer);
+   spin_lock(&buffer->heap->stat_lock);
+   buffer->heap->num_of_buffers--;
+   buffer->heap->num_of_alloc_bytes -= buffer->size;
+   spin_unlock(&buffer->heap->stat_lock);
+
kfree(buffer);
  }
  
@@ -528,12 +540,15 @@ void ion_device_add_heap(struct ion_heap *heap)

  {
struct ion_device *dev = internal_dev;
int ret;
+   struct dentry *heap_root;
+   char debug_name[64];
  
  	if (!heap->ops->allocate || !heap->ops->free)

pr_err("%s: can not add heap with invalid ops struct.\n",
   __func__);
  
  	spin_lock_init(&heap->free_lock);

+   spin_lock_init(&heap->stat_lock);
heap->free_list_size = 0;
  
  	if (heap->flags & ION_HEAP_FLAG_DEFER_FREE)

@@ -546,6 +561,33 @@ void ion_device_add_heap(struct ion_heap *heap)
}
  
  	heap->dev = dev;

+   heap->num_of_buffers = 0;
+   heap->num_of_alloc_bytes = 0;
+   heap->alloc_bytes_wm = 0;
+
+   heap_root = debugfs_create_dir(heap->name, dev->debug_root);
+   debugfs_create_u64("num_of_buffers",
+  0444, heap_root,
+  &heap->num_of_buffers);
+   debugfs_create_u64("num_of_alloc_bytes",
+  0444,
+  heap_root,
+  &heap->num_of_alloc_bytes);
+   debugfs_create_u64("alloc_bytes_wm",
+  0444,
+  heap_root,
+  &heap->alloc_bytes_wm);
+
+   if (heap->shrinker.count_objects &&
+   heap->shrinker.scan_objects) {
+   snprintf(debug_name, 64, "%s_shrink", heap->name);
+   debugfs_create_file(debug_name,
+   0644,
+   heap_root,
+   heap,
+   &debug_shrink_fops);
+   }
+
down_write(&dev->lock);
heap->id = heap_id++;
/*
@@ -555,14 +597,6 @@ void ion_device_add_heap(struct ion_heap *heap)
plist_node_init(&heap->node, -heap->id);
plist_add(&heap->node, &dev->heaps);
  
-	if (heap->shrinker.count_objects && heap->shrinker.scan_objects) {

-   char debug_name[64];
-
-   snprintf(debug_name, 64, "%s_shrink", heap->name);
-   debugfs_create_file(debug_name, 0644, dev->debug_root,
-   heap, &debug_shrink_fops);
-   }
-
dev->heap_cnt++;
up_write(&dev->lock);
  }
diff --git a/drivers/staging/android/ion/ion.h 
b/drivers/staging/android/ion/ion.h
index 16cbd38..f487127 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -159,6 +159,9 @@ struct ion_heap_ops {
   * @task: task struct of deferred free thread
   * @debug_show:   called when heap debug file is read to add any
   *heap specific debug info to output
+ * @num_of_buffers the number of currently allocated buffers
+ * @num_of_alloc_bytes the number of allocated bytes
+ * @alloc_bytes_wm the number of allocated bytes watermark
   *
   * Represents a pool of memory from which buffers can be made.  In some
   * systems the only heap is regular system memory allocated via vmalloc.
@@ -179,9 +1

Re: [PATCH RFC] mm/memory_hotplug: Introduce memory block types

[David Hildenbrand]

On 03/10/2018 16:34, Vitaly Kuznetsov wrote:
> Dave Hansen  writes:
> 
>> On 10/03/2018 06:52 AM, Vitaly Kuznetsov wrote:
>>> It is more than just memmaps (e.g. forking udev process doing memory
>>> onlining also needs memory) but yes, the main idea is to make the
>>> onlining synchronous with hotplug.
>>
>> That's a good theoretical concern.
>>
>> But, is it a problem we need to solve in practice?
> 
> Yes, unfortunately. It was previously discovered that when we try to
> hotplug tons of memory to a low memory system (this is a common scenario
> with VMs) we end up with OOM because for all new memory blocks we need
> to allocate page tables, struct pages, ... and we need memory to do
> that. The userspace program doing memory onlining also needs memory to
> run and in case it prefers to fork to handle hundreds of notfifications
> ... well, it may get OOMkilled before it manages to online anything.
> 
> Allocating all kernel objects from the newly hotplugged blocks would
> definitely help to manage the situation but as I said this won't solve
> the 'forking udev' problem completely (it will likely remain in
> 'extreme' cases only. We can probably work around it by onlining with a
> dedicated process which doesn't do memory allocation).
> 

I guess the problem is even worse. We always have two phases

1. add memory - requires memory allocation
2. online memory - might require memory allocations e.g. for slab/slub

So if we just added memory but don't have sufficient memory to start a
user space process to trigger onlining, then we most likely also don't
have sufficient memory to online the memory right away (in some scenarios).

We would have to allocate all new memory for 1 and 2 from the memory to
be onlined. I guess the latter part is less trivial.

So while onlining the memory from the kernel might make things a little
more robust, we would still have the chance for OOM / onlining failing.

-- 

Thanks,

David / dhildenb
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH RFC] mm/memory_hotplug: Introduce memory block types

[David Hildenbrand]

On 03/10/2018 16:24, Michal Hocko wrote:
> On Wed 03-10-18 15:52:24, Vitaly Kuznetsov wrote:
> [...]
>>> As David said some of the memory cannot be onlined without further steps
>>> (e.g. when it is standby as David called it) and then I fail to see how
>>> eBPF help in any way.
>>
>> and also, we can fight till the end of days here trying to come up with
>> an onlining solution which would work for everyone and eBPF would move
>> this decision to distro level.
> 
> The point is that there is _no_ general onlining solution. This is
> basically policy which belongs to the userspace.
> 

As already stated, I guess we should then provide user space with
sufficient information to make a good decision (to implement rules).

The eBPF is basically the same idea, only the rules are formulated
differently and directly handle in the kernel. Still it might be e.e.
relevant if memory is standby memory (that's what I remember the
official s390x name), or something else.

Right now, the (udev) rules we have make assumptions based on general
system properties (s390x, HyperV ...).

-- 

Thanks,

David / dhildenb
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH RFC] mm/memory_hotplug: Introduce memory block types

[David Hildenbrand]

On 03/10/2018 15:54, Michal Hocko wrote:
> On Tue 02-10-18 17:25:19, David Hildenbrand wrote:
>> On 02/10/2018 15:47, Michal Hocko wrote:
> [...]
>>> Zone imbalance is an inherent problem of the highmem zone. It is
>>> essentially the highmem zone we all loved so much back in 32b days.
>>> Yes the movable zone doesn't have any addressing limitations so it is a
>>> bit more relaxed but considering the hotplug scenarios I have seen so
>>> far people just want to have full NUMA nodes movable to allow replacing
>>> DIMMs. And then we are back to square one and the zone imbalance issue.
>>> You have those regardless where memmaps are allocated from.
>>
>> Unfortunately yes. And things get more complicated as you are adding a
>> whole DIMMs and get notifications in the granularity of memory blocks.
>> Usually you are not interested in onlining any memory block of that DIMM
>> as MOVABLE as soon as you would have to online one memory block of that
>> DIMM as NORMAL - because that can already block the whole DIMM.
> 
> For the purpose of the hotremove, yes. But as Dave has noted people are
> (ab)using zone movable for other purposes - e.g. large pages.

That might be right for some very special use cases. For most of users
this is not the case (meaning it should be the default but if the user
wants to change it, he should be allowed to change it).

>  
> [...]
>>> Then the immediate question would be why to use memory hotplug for that
>>> at all? Why don't you simply start with a huge pre-allocated physical
>>> address space and balloon memory in an out per demand. Why do you want
>>> to inject new memory during the runtime?
>>
>> Let's assume you have a guest with 20GB size and eventually want to
>> allow to grow it to 4TB. You would have to allocate metadata for 4TB
>> right from the beginning. That's definitely now what we want. That is
>> why memory hotplug is used by e.g. XEN or Hyper-V. With Hyper-V, the
>> hypervisor even tells you at which places additional memory has been
>> made available.
> 
> Then you have to live with the fact that your hot added memory will be
> self hosted and find a way for ballooning to work with that. The price
> would be that some part of the memory is not really balloonable in the
> end.
> 
 1. is a reason why distributions usually don't configure
 "MEMORY_HOTPLUG_DEFAULT_ONLINE", because you really want the option for
 MOVABLE zone. That however implies, that e.g. for x86, you have to
 handle all new memory in user space, especially also HyperV memory.
 There, you then have to check for things like "isHyperV()" to decide
 "oh, yes, this should definitely not go to the MOVABLE zone".
>>>
>>> Why do you need a generic hotplug rule in the first place? Why don't you
>>> simply provide different set of rules for different usecases? Let users
>>> decide which usecase they prefer rather than try to be clever which
>>> almost always hits weird corner cases.
>>>
>>
>> Memory hotplug has to work as reliable as we can out of the box. Letting
>> the user make simple decisions like "oh, I am on hyper-V, I want to
>> online memory to the normal zone" does not feel right.
> 
> Users usually know what is their usecase and then it is just a matter of
> plumbing (e.g. distribution can provide proper tools to deploy those
> usecases) to chose the right and for user obscure way to make it work.

I disagree. If we can ship sane defaults, we should do that and allow to
make changes later on. This is how distributions have been working for
ever. But yes, allowing to make modifications is always a good idea to
tailor it to some special case user scenarios. (tuned or whatever we
have in place).

> 
>> But yes, we
>> should definitely allow to make modifications. So some sane default rule
>> + possible modification is usually a good idea.
>>
>> I think Dave has a point with using MOVABLE for huge page use cases. And
>> there might be other corner cases as you correctly state.
>>
>> I wonder if this patch itself minus modifying online/offline might make
>> sense. We can then implement simple rules in user space
>>
>> if (normal) {
>>  /* customers expect hotplugged DIMMs to be unpluggable */
>>  online_movable();
>> } else if (paravirt) {
>>  /* paravirt memory should as default always go to the NORMAL */
>>  online();
>> } else {
>>  /* standby memory will never get onlined automatically */
>> }
>>
>> Compared to having to guess what is to be done (isKVM(), isHyperV,
>> isS390 ...) and failing once this is no longer unique (e.g. virtio-mem
>> and ACPI support for x86 KVM).
> 
> I am worried that exporing a type will just push us even further to the
> corner. The current design is really simple and 2 stage and that is good
> because it allows for very different usecases. The more specific the API
> be the more likely we are going to hit "I haven't even dreamed somebody
> would be using hotplug for this thing". And I would bet this will happen
> sooner or later

Re: [patch 00/11] x86/vdso: Cleanups, simmplifications and CLOCK_TAI support

[Andy Lutomirski]

On Wed, Oct 3, 2018 at 8:10 AM Thomas Gleixner  wrote:
>
> On Wed, 3 Oct 2018, Andy Lutomirski wrote:
> > > On Oct 3, 2018, at 5:01 AM, Vitaly Kuznetsov  wrote:
> > > Not all Hyper-V hosts support reenlightenment notifications (and, if I'm
> > > not mistaken, you need to enable nesting for the VM to get the feature -
> > > and most VMs don't have this) so I think we'll have to keep Hyper-V
> > > vclock for the time being.
> > >
> > But this does suggest that the correct way to pass a clock through to an
> > L2 guest where L0 is HV is to make L1 use the “tsc” clock and L2 use
> > kvmclock (or something newer and better).  This would require adding
> > support for atomic frequency changes all the way through the timekeeping
> > and arch code.
> >
> > John, tglx, would that be okay or crazy?
>
> Not sure what you mean. I think I lost you somewhere on the way.
>

What I mean is: currently we have a clocksource called
""hyperv_clocksource_tsc_page".  Reading it does:

static u64 read_hv_clock_tsc(struct clocksource *arg)
{
u64 current_tick = hv_read_tsc_page(tsc_pg);

if (current_tick == U64_MAX)
rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);

return current_tick;
}

From Vitaly's email, it sounds like, on most (all?) hyperv systems
with nesting enabled, this clock is better behaved than it appears.
It sounds like the read behavior is that current_tick will never be
U64_MAX -- instead, the clock always works and, more importantly, the
actual scaling factor and offset only change observably on *guest*
request.

So why don't we we improve the actual "tsc" clocksource to understand
this?  ISTM the best model would be where the
__clocksource_update_freq_xyz() mechanism gets called so we can use it
like this:

clocksource_begin_update();
clocksource_update_mult_shift();
tell_hv_that_we_reenlightened();
clocksource_end_update();

Where clocksource_begin_update() bumps the seqcount for the vDSO and
takes all the locks, clocksource_update_mult_shift() updates
everything, and clocksource_end_update() makes the updated parameters
usable.

(AFAICT there are currently no clocksources at all in the entire
kernel that update their frequency on the fly using
__clocksource_update_xyz().  Unless I'm missing something, the x86 tsc
cpufreq hooks don't call into the core timekeeping at all, so I'm
assuming that the tsc clocksource is just unusable as a clocksource on
systems that change its frequency.)

Or we could keep the hyperv_clocksource_tsc_page clocksource but make
it use VCLOCK_TSC and a similar update mechanism.

I don't personally want to do this, because the timekeeping code is
subtle and I'm unfamiliar with it.  And I don't have *that* many spare
cycles :)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v5 1/3] staging: mt7621-mmc: Remove #if 0 blocks

[Nishad Kamdar]

On Tue, Oct 02, 2018 at 03:16:36PM -0700, Greg Kroah-Hartman wrote:
> On Mon, Oct 01, 2018 at 08:13:40PM +0530, Nishad Kamdar wrote:
> > This patch removes #if 0 code blocks and usages of
> > functions defined in the #if 0 blocks.
> > 
> > Signed-off-by: Nishad Kamdar 
> > ---
> >  drivers/staging/mt7621-mmc/dbg.c |  21 +--
> >  drivers/staging/mt7621-mmc/dbg.h |  14 --
> >  drivers/staging/mt7621-mmc/sd.c  | 269 ---
> >  3 files changed, 1 insertion(+), 303 deletions(-)
> 
> This patch did not apply :(

Oh, do you mean it gave conflicts while applying?
I'll check and resubmit it in that case.

Thanks for the review.

regards,
Nishad
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [patch 00/11] x86/vdso: Cleanups, simmplifications and CLOCK_TAI support

[Thomas Gleixner]

On Wed, 3 Oct 2018, Andy Lutomirski wrote:
> > On Oct 3, 2018, at 5:01 AM, Vitaly Kuznetsov  wrote:
> > Not all Hyper-V hosts support reenlightenment notifications (and, if I'm
> > not mistaken, you need to enable nesting for the VM to get the feature -
> > and most VMs don't have this) so I think we'll have to keep Hyper-V
> > vclock for the time being.
> > 
> But this does suggest that the correct way to pass a clock through to an
> L2 guest where L0 is HV is to make L1 use the “tsc” clock and L2 use
> kvmclock (or something newer and better).  This would require adding
> support for atomic frequency changes all the way through the timekeeping
> and arch code.
>
> John, tglx, would that be okay or crazy?

Not sure what you mean. I think I lost you somewhere on the way.

Thanks,

tglx___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH RFC] mm/memory_hotplug: Introduce memory block types

[Vitaly Kuznetsov]

Dave Hansen  writes:

> On 10/03/2018 06:52 AM, Vitaly Kuznetsov wrote:
>> It is more than just memmaps (e.g. forking udev process doing memory
>> onlining also needs memory) but yes, the main idea is to make the
>> onlining synchronous with hotplug.
>
> That's a good theoretical concern.
>
> But, is it a problem we need to solve in practice?

Yes, unfortunately. It was previously discovered that when we try to
hotplug tons of memory to a low memory system (this is a common scenario
with VMs) we end up with OOM because for all new memory blocks we need
to allocate page tables, struct pages, ... and we need memory to do
that. The userspace program doing memory onlining also needs memory to
run and in case it prefers to fork to handle hundreds of notfifications
... well, it may get OOMkilled before it manages to online anything.

Allocating all kernel objects from the newly hotplugged blocks would
definitely help to manage the situation but as I said this won't solve
the 'forking udev' problem completely (it will likely remain in
'extreme' cases only. We can probably work around it by onlining with a
dedicated process which doesn't do memory allocation).

-- 
Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH RFC] mm/memory_hotplug: Introduce memory block types

[Michal Hocko]

On Wed 03-10-18 15:52:24, Vitaly Kuznetsov wrote:
[...]
> > As David said some of the memory cannot be onlined without further steps
> > (e.g. when it is standby as David called it) and then I fail to see how
> > eBPF help in any way.
> 
> and also, we can fight till the end of days here trying to come up with
> an onlining solution which would work for everyone and eBPF would move
> this decision to distro level.

The point is that there is _no_ general onlining solution. This is
basically policy which belongs to the userspace.
-- 
Michal Hocko
SUSE Labs
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [patch 00/11] x86/vdso: Cleanups, simmplifications and CLOCK_TAI support

[Andy Lutomirski]

> On Oct 3, 2018, at 5:01 AM, Vitaly Kuznetsov  wrote:
> 
> Andy Lutomirski  writes:
> 
>>> On Oct 3, 2018, at 2:22 AM, Vitaly Kuznetsov  wrote:
>>> 
>>> Andy Lutomirski  writes:
>>> 
 Hi Vitaly, Paolo, Radim, etc.,
 
>>> The notification you're talking about exists, it is called
>>> Reenligntenment, see 0092e4346f49 "x86/kvm: Support Hyper-V
>>> reenlightenment"). When TSC page changes (and this only happens when L1
>>> is migrated to a different host with a different TSC frequency and TSC
>>> scaling is not supported by the CPU) we receive an interrupt in L1 (at
>>> this moment all TSC accesses are emulated which guarantees the
>>> correctness of the readings), pause all L2 guests, update their kvmclock
>>> structures with new data (we already know the new TSC frequency) and
>>> then tell L0 that we're done and it can stop emulating TSC accesses.
>> 
>> That’s delightful!  Does the emulation magic also work for L1 user
>> mode?
> 
> As far as I understand - yes, all rdtsc* calls will trap into L0.
> 
>> If so, couldn’t we drop the HyperV vclock entirely and just
>> fold the adjustment into the core timekeeping data?  (Preferably the
>> actual core data, which would require core changes, but it could
>> plausibly be done in arch code, too.)
> 
> Not all Hyper-V hosts support reenlightenment notifications (and, if I'm
> not mistaken, you need to enable nesting for the VM to get the feature -
> and most VMs don't have this) so I think we'll have to keep Hyper-V
> vclock for the time being.
> 
> 

But this does suggest that the correct way to pass a clock through to an L2 
guest where L0 is HV is to make L1 use the “tsc” clock and L2 use kvmclock (or 
something newer and better).  This would require adding support for atomic 
frequency changes all the way through the timekeeping and arch code.

John, tglx, would that be okay or crazy?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: rtl8712: add SPDX identifiers

[Michael Straube]

This satisfies a checkpatch warning and is the preferred
method for notating the license.

The SPDX identifier is a legally binding shorthand, which
can be used instead of the full boiler plate text.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8712/basic_types.h | 10 +-
 drivers/staging/rtl8712/drv_types.h   | 10 +-
 drivers/staging/rtl8712/ethernet.h| 10 +-
 drivers/staging/rtl8712/ieee80211.c   | 10 +-
 drivers/staging/rtl8712/ieee80211.h   | 13 +
 drivers/staging/rtl8712/mlme_linux.c  | 14 +-
 drivers/staging/rtl8712/mlme_osdep.h  | 14 +-
 drivers/staging/rtl8712/mp_custom_oid.h   | 14 +-
 drivers/staging/rtl8712/os_intfs.c| 10 +-
 drivers/staging/rtl8712/osdep_intf.h  | 14 +-
 drivers/staging/rtl8712/osdep_service.h   | 14 +-
 drivers/staging/rtl8712/recv_linux.c  | 14 +-
 drivers/staging/rtl8712/recv_osdep.h  | 14 +-
 drivers/staging/rtl8712/rtl8712_bitdef.h  | 15 +--
 drivers/staging/rtl8712/rtl8712_cmd.c | 14 +-
 drivers/staging/rtl8712/rtl8712_cmd.h | 14 +-
 drivers/staging/rtl8712/rtl8712_cmdctrl_bitdef.h  | 15 +--
 drivers/staging/rtl8712/rtl8712_cmdctrl_regdef.h  | 15 +--
 .../staging/rtl8712/rtl8712_debugctrl_bitdef.h| 15 +--
 .../staging/rtl8712/rtl8712_debugctrl_regdef.h| 15 +--
 .../staging/rtl8712/rtl8712_edcasetting_bitdef.h  | 14 +-
 .../staging/rtl8712/rtl8712_edcasetting_regdef.h  | 15 +--
 drivers/staging/rtl8712/rtl8712_efuse.c   | 14 +-
 drivers/staging/rtl8712/rtl8712_event.h   | 14 +-
 drivers/staging/rtl8712/rtl8712_fifoctrl_bitdef.h | 15 +--
 drivers/staging/rtl8712/rtl8712_fifoctrl_regdef.h | 15 +--
 drivers/staging/rtl8712/rtl8712_gp_bitdef.h   | 14 +-
 drivers/staging/rtl8712/rtl8712_gp_regdef.h   | 14 +-
 drivers/staging/rtl8712/rtl8712_hal.h | 14 +-
 .../staging/rtl8712/rtl8712_interrupt_bitdef.h| 15 +--
 drivers/staging/rtl8712/rtl8712_io.c  | 14 +-
 drivers/staging/rtl8712/rtl8712_led.c | 14 +-
 .../staging/rtl8712/rtl8712_macsetting_bitdef.h   | 15 +--
 .../staging/rtl8712/rtl8712_macsetting_regdef.h   | 15 +--
 .../staging/rtl8712/rtl8712_powersave_bitdef.h| 15 +--
 .../staging/rtl8712/rtl8712_powersave_regdef.h| 15 +--
 drivers/staging/rtl8712/rtl8712_ratectrl_bitdef.h | 15 +--
 drivers/staging/rtl8712/rtl8712_ratectrl_regdef.h | 14 +-
 drivers/staging/rtl8712/rtl8712_recv.c| 14 +-
 drivers/staging/rtl8712/rtl8712_recv.h| 14 +-
 drivers/staging/rtl8712/rtl8712_regdef.h  | 14 +-
 drivers/staging/rtl8712/rtl8712_security_bitdef.h | 15 +--
 drivers/staging/rtl8712/rtl8712_spec.h| 14 +-
 drivers/staging/rtl8712/rtl8712_syscfg_bitdef.h   | 14 +-
 drivers/staging/rtl8712/rtl8712_syscfg_regdef.h   | 14 +-
 drivers/staging/rtl8712/rtl8712_timectrl_bitdef.h | 15 +--
 drivers/staging/rtl8712/rtl8712_timectrl_regdef.h | 15 +--
 drivers/staging/rtl8712/rtl8712_wmac_bitdef.h | 14 +-
 drivers/staging/rtl8712/rtl8712_wmac_regdef.h | 14 +-
 drivers/staging/rtl8712/rtl8712_xmit.c| 14 +-
 drivers/staging/rtl8712/rtl8712_xmit.h| 14 +-
 drivers/staging/rtl8712/rtl871x_cmd.c | 14 +-
 drivers/staging/rtl8712/rtl871x_cmd.h | 14 +-
 drivers/staging/rtl8712/rtl871x_debug.h   | 14 +-
 drivers/staging/rtl8712/rtl871x_eeprom.c  | 14 +-
 drivers/staging/rtl8712/rtl871x_eeprom.h  | 15 +--
 drivers/staging/rtl8712/rtl871x_event.h   | 14 +-
 drivers/staging/rtl8712/rtl871x_ht.h  | 14 +-
 drivers/staging/rtl8712/rtl871x_io.c  | 14 +-
 drivers/staging/rtl8712/rtl871x_io.h  | 14 +-
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 14 +-
 drivers/staging/rtl8712/rtl871x_ioctl_rtl.c   | 14 +-
 drivers/staging/rtl8712/rtl871x_ioctl_rtl.h   | 14 +-
 drivers/staging/rtl8712/rtl871x_ioctl_set.c   | 14 +-
 drivers/staging/rtl8712/rtl871x_ioctl_set.h   | 14 +-
 drivers/staging/rtl8712/rtl871x_led.h | 14 +-
 drivers/staging/rtl8712/rtl871x_mlme.c| 14 +-
 drivers

Re: [PATCH RFC] mm/memory_hotplug: Introduce memory block types

[Dave Hansen]

On 10/03/2018 06:52 AM, Vitaly Kuznetsov wrote:
> It is more than just memmaps (e.g. forking udev process doing memory
> onlining also needs memory) but yes, the main idea is to make the
> onlining synchronous with hotplug.

That's a good theoretical concern.

But, is it a problem we need to solve in practice?

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH RFC] mm/memory_hotplug: Introduce memory block types

[Michal Hocko]

On Tue 02-10-18 17:25:19, David Hildenbrand wrote:
> On 02/10/2018 15:47, Michal Hocko wrote:
[...]
> > Zone imbalance is an inherent problem of the highmem zone. It is
> > essentially the highmem zone we all loved so much back in 32b days.
> > Yes the movable zone doesn't have any addressing limitations so it is a
> > bit more relaxed but considering the hotplug scenarios I have seen so
> > far people just want to have full NUMA nodes movable to allow replacing
> > DIMMs. And then we are back to square one and the zone imbalance issue.
> > You have those regardless where memmaps are allocated from.
> 
> Unfortunately yes. And things get more complicated as you are adding a
> whole DIMMs and get notifications in the granularity of memory blocks.
> Usually you are not interested in onlining any memory block of that DIMM
> as MOVABLE as soon as you would have to online one memory block of that
> DIMM as NORMAL - because that can already block the whole DIMM.

For the purpose of the hotremove, yes. But as Dave has noted people are
(ab)using zone movable for other purposes - e.g. large pages.
 
[...]
> > Then the immediate question would be why to use memory hotplug for that
> > at all? Why don't you simply start with a huge pre-allocated physical
> > address space and balloon memory in an out per demand. Why do you want
> > to inject new memory during the runtime?
> 
> Let's assume you have a guest with 20GB size and eventually want to
> allow to grow it to 4TB. You would have to allocate metadata for 4TB
> right from the beginning. That's definitely now what we want. That is
> why memory hotplug is used by e.g. XEN or Hyper-V. With Hyper-V, the
> hypervisor even tells you at which places additional memory has been
> made available.

Then you have to live with the fact that your hot added memory will be
self hosted and find a way for ballooning to work with that. The price
would be that some part of the memory is not really balloonable in the
end.

> >> 1. is a reason why distributions usually don't configure
> >> "MEMORY_HOTPLUG_DEFAULT_ONLINE", because you really want the option for
> >> MOVABLE zone. That however implies, that e.g. for x86, you have to
> >> handle all new memory in user space, especially also HyperV memory.
> >> There, you then have to check for things like "isHyperV()" to decide
> >> "oh, yes, this should definitely not go to the MOVABLE zone".
> > 
> > Why do you need a generic hotplug rule in the first place? Why don't you
> > simply provide different set of rules for different usecases? Let users
> > decide which usecase they prefer rather than try to be clever which
> > almost always hits weird corner cases.
> > 
> 
> Memory hotplug has to work as reliable as we can out of the box. Letting
> the user make simple decisions like "oh, I am on hyper-V, I want to
> online memory to the normal zone" does not feel right.

Users usually know what is their usecase and then it is just a matter of
plumbing (e.g. distribution can provide proper tools to deploy those
usecases) to chose the right and for user obscure way to make it work.

> But yes, we
> should definitely allow to make modifications. So some sane default rule
> + possible modification is usually a good idea.
> 
> I think Dave has a point with using MOVABLE for huge page use cases. And
> there might be other corner cases as you correctly state.
> 
> I wonder if this patch itself minus modifying online/offline might make
> sense. We can then implement simple rules in user space
> 
> if (normal) {
>   /* customers expect hotplugged DIMMs to be unpluggable */
>   online_movable();
> } else if (paravirt) {
>   /* paravirt memory should as default always go to the NORMAL */
>   online();
> } else {
>   /* standby memory will never get onlined automatically */
> }
> 
> Compared to having to guess what is to be done (isKVM(), isHyperV,
> isS390 ...) and failing once this is no longer unique (e.g. virtio-mem
> and ACPI support for x86 KVM).

I am worried that exporing a type will just push us even further to the
corner. The current design is really simple and 2 stage and that is good
because it allows for very different usecases. The more specific the API
be the more likely we are going to hit "I haven't even dreamed somebody
would be using hotplug for this thing". And I would bet this will happen
sooner or later.

Just look at how the whole auto onlining screwed the API to workaround
an implementation detail. It has created a one purpose behavior that
doesn't suite many usecases. Yet we have to live with that because
somebody really relies on it. Let's not repeat same errors.
-- 
Michal Hocko
SUSE Labs
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH RFC] mm/memory_hotplug: Introduce memory block types

[Vitaly Kuznetsov]

Michal Hocko  writes:

> On Wed 03-10-18 15:38:04, Vitaly Kuznetsov wrote:
>> David Hildenbrand  writes:
>> 
>> > On 02/10/2018 15:47, Michal Hocko wrote:
>> ...
>> >> 
>> >> Why do you need a generic hotplug rule in the first place? Why don't you
>> >> simply provide different set of rules for different usecases? Let users
>> >> decide which usecase they prefer rather than try to be clever which
>> >> almost always hits weird corner cases.
>> >> 
>> >
>> > Memory hotplug has to work as reliable as we can out of the box. Letting
>> > the user make simple decisions like "oh, I am on hyper-V, I want to
>> > online memory to the normal zone" does not feel right. But yes, we
>> > should definitely allow to make modifications.
>> 
>> Last time I was thinking about the imperfectness of the auto-online
>> solution we have and any other solution we're able to suggest an idea
>> came to my mind - what if we add an eBPF attach point to the
>> auto-onlining mechanism effecively offloading decision-making to
>> userspace. We'll of couse need to provide all required data (e.g. how
>> memory blocks are aligned with physical DIMMs as it makes no sense to
>> online part of DIMM as normal and the rest as movable as it's going to
>> be impossible to unplug such DIMM anyways).
>
> And how does that differ from the notification mechanism we have? Just
> by not relying on the process scheduling? If yes then this revolves
> around the implementation detail that you care about time-to-hot-add
> vs. time-to-online. And that is a solveable problem - just allocate
> memmaps from the hot-added memory.

It is more than just memmaps (e.g. forking udev process doing memory
onlining also needs memory) but yes, the main idea is to make the
onlining synchronous with hotplug.

>
> As David said some of the memory cannot be onlined without further steps
> (e.g. when it is standby as David called it) and then I fail to see how
> eBPF help in any way.

and also, we can fight till the end of days here trying to come up with
an onlining solution which would work for everyone and eBPF would move
this decision to distro level.

-- 
Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH RFC] mm/memory_hotplug: Introduce memory block types

[Michal Hocko]

On Wed 03-10-18 15:38:04, Vitaly Kuznetsov wrote:
> David Hildenbrand  writes:
> 
> > On 02/10/2018 15:47, Michal Hocko wrote:
> ...
> >> 
> >> Why do you need a generic hotplug rule in the first place? Why don't you
> >> simply provide different set of rules for different usecases? Let users
> >> decide which usecase they prefer rather than try to be clever which
> >> almost always hits weird corner cases.
> >> 
> >
> > Memory hotplug has to work as reliable as we can out of the box. Letting
> > the user make simple decisions like "oh, I am on hyper-V, I want to
> > online memory to the normal zone" does not feel right. But yes, we
> > should definitely allow to make modifications.
> 
> Last time I was thinking about the imperfectness of the auto-online
> solution we have and any other solution we're able to suggest an idea
> came to my mind - what if we add an eBPF attach point to the
> auto-onlining mechanism effecively offloading decision-making to
> userspace. We'll of couse need to provide all required data (e.g. how
> memory blocks are aligned with physical DIMMs as it makes no sense to
> online part of DIMM as normal and the rest as movable as it's going to
> be impossible to unplug such DIMM anyways).

And how does that differ from the notification mechanism we have? Just
by not relying on the process scheduling? If yes then this revolves
around the implementation detail that you care about time-to-hot-add
vs. time-to-online. And that is a solveable problem - just allocate
memmaps from the hot-added memory.

As David said some of the memory cannot be onlined without further steps
(e.g. when it is standby as David called it) and then I fail to see how
eBPF help in any way.
-- 
Michal Hocko
SUSE Labs
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4] memory_hotplug: Free pages as higher order

[Arun KS]

When free pages are done with higher order, time spend on
coalescing pages by buddy allocator can be reduced. With
section size of 256MB, hot add latency of a single section
shows improvement from 50-60 ms to less than 1 ms, hence
improving the hot add latency by 60%. Modify external
providers of online callback to align with the change.
Also remove prefetch from __free_pages_core().

Signed-off-by: Arun KS 
---
Changes since v3:
- renamed _free_pages_boot_core -> __free_pages_core.
- removed prefetch from __free_pages_core.
- removed xen_online_page().

Changes since v2:
- reuse code from __free_pages_boot_core().

Changes since v1:
- Removed prefetch().

Changes since RFC:
- Rebase.
- As suggested by Michal Hocko remove pages_per_block.
- Modifed external providers of online_page_callback.

v3: https://lore.kernel.org/patchwork/patch/992348/
v2: https://lore.kernel.org/patchwork/patch/991363/
v1: https://lore.kernel.org/patchwork/patch/989445/
RFC: https://lore.kernel.org/patchwork/patch/984754/

---
 drivers/hv/hv_balloon.c|  6 --
 drivers/xen/balloon.c  | 23 ++
 include/linux/memory_hotplug.h |  2 +-
 mm/internal.h  |  1 +
 mm/memory_hotplug.c| 44 ++
 mm/page_alloc.c| 14 +-
 6 files changed, 58 insertions(+), 32 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index b1b7880..c5bc0b5 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -771,7 +771,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned 
long size,
}
 }
 
-static void hv_online_page(struct page *pg)
+static int hv_online_page(struct page *pg, unsigned int order)
 {
struct hv_hotadd_state *has;
unsigned long flags;
@@ -783,10 +783,12 @@ static void hv_online_page(struct page *pg)
if ((pfn < has->start_pfn) || (pfn >= has->end_pfn))
continue;
 
-   hv_page_online_one(has, pg);
+   hv_bring_pgs_online(has, pfn, (1UL << order));
break;
}
spin_unlock_irqrestore(&dm_device.ha_lock, flags);
+
+   return 0;
 }
 
 static int pfn_covered(unsigned long start_pfn, unsigned long pfn_cnt)
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index e12bb25..58ddf48 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -390,8 +390,8 @@ static enum bp_state reserve_additional_memory(void)
 
/*
 * add_memory_resource() will call online_pages() which in its turn
-* will call xen_online_page() callback causing deadlock if we don't
-* release balloon_mutex here. Unlocking here is safe because the
+* will call xen_bring_pgs_online() callback causing deadlock if we
+* don't release balloon_mutex here. Unlocking here is safe because the
 * callers drop the mutex before trying again.
 */
mutex_unlock(&balloon_mutex);
@@ -411,15 +411,22 @@ static enum bp_state reserve_additional_memory(void)
return BP_ECANCELED;
 }
 
-static void xen_online_page(struct page *page)
+static int xen_bring_pgs_online(struct page *pg, unsigned int order)
 {
-   __online_page_set_limits(page);
+   unsigned long i, size = (1 << order);
+   unsigned long start_pfn = page_to_pfn(pg);
+   struct page *p;
 
+   pr_debug("Online %lu pages starting at pfn 0x%lx\n", size, start_pfn);
mutex_lock(&balloon_mutex);
-
-   __balloon_append(page);
-
+   for (i = 0; i < size; i++) {
+   p = pfn_to_page(start_pfn + i);
+   __online_page_set_limits(p);
+   __balloon_append(p);
+   }
mutex_unlock(&balloon_mutex);
+
+   return 0;
 }
 
 static int xen_memory_notifier(struct notifier_block *nb, unsigned long val, 
void *v)
@@ -744,7 +751,7 @@ static int __init balloon_init(void)
balloon_stats.max_retry_count = RETRY_UNLIMITED;
 
 #ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
-   set_online_page_callback(&xen_online_page);
+   set_online_page_callback(&xen_bring_pgs_online);
register_memory_notifier(&xen_memory_nb);
register_sysctl_table(xen_root);
 
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 34a2822..7b04c1d 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -87,7 +87,7 @@ extern int test_pages_in_a_zone(unsigned long start_pfn, 
unsigned long end_pfn,
unsigned long *valid_start, unsigned long *valid_end);
 extern void __offline_isolated_pages(unsigned long, unsigned long);
 
-typedef void (*online_page_callback_t)(struct page *page);
+typedef int (*online_page_callback_t)(struct page *page, unsigned int order);
 
 extern int set_online_page_callback(online_page_callback_t callback);
 extern int restore_online_page_callback(online_page_callback_t callback);
diff --git a/mm/internal.h b/mm/internal.h
index 87256ae..

Re: [PATCH RFC] mm/memory_hotplug: Introduce memory block types

[Vitaly Kuznetsov]

David Hildenbrand  writes:

> On 02/10/2018 15:47, Michal Hocko wrote:
...
>> 
>> Why do you need a generic hotplug rule in the first place? Why don't you
>> simply provide different set of rules for different usecases? Let users
>> decide which usecase they prefer rather than try to be clever which
>> almost always hits weird corner cases.
>> 
>
> Memory hotplug has to work as reliable as we can out of the box. Letting
> the user make simple decisions like "oh, I am on hyper-V, I want to
> online memory to the normal zone" does not feel right. But yes, we
> should definitely allow to make modifications.

Last time I was thinking about the imperfectness of the auto-online
solution we have and any other solution we're able to suggest an idea
came to my mind - what if we add an eBPF attach point to the
auto-onlining mechanism effecively offloading decision-making to
userspace. We'll of couse need to provide all required data (e.g. how
memory blocks are aligned with physical DIMMs as it makes no sense to
online part of DIMM as normal and the rest as movable as it's going to
be impossible to unplug such DIMM anyways).

-- 
Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 02/16] hv_balloon: Replace spin_is_locked() with lockdep

[Vitaly Kuznetsov]

Lance Roy  writes:

> lockdep_assert_held() is better suited to checking locking requirements,
> since it won't get confused when someone else holds the lock. This is
> also a step towards possibly removing spin_is_locked().
>
> Signed-off-by: Lance Roy 
> Cc: "K. Y. Srinivasan" 
> Cc: Haiyang Zhang 
> Cc: Stephen Hemminger 
> Cc: 
> ---
>  drivers/hv/hv_balloon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index b1b788082793..41631512ae97 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -689,7 +689,7 @@ static void hv_page_online_one(struct hv_hotadd_state 
> *has, struct page *pg)
>   __online_page_increment_counters(pg);
>   __online_page_free(pg);
>  
> - WARN_ON_ONCE(!spin_is_locked(&dm_device.ha_lock));
> + lockdep_assert_held(&dm_device.ha_lock);
>   dm_device.num_pages_onlined++;
>  }

Reviewed-by: Vitaly Kuznetsov 

However,

lockdep_assert_held() is a no-op when !CONFIG_LOCKDEP but this doesn't
really matter: hv_page_online_one() is static and it has only two call
sites, both taking the dm_device.ha_lock lock - so the warning may just
go away.

-- 
Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: comedi: ni_mio_common: protect register write overflow

[Ian Abbott]

On 03/10/2018 11:34, Ian Abbott wrote:

On 03/10/18 02:24, Spencer Olson wrote:

Should I resubmit this patch and the entire patch set from earlier
today, each separately?

The patch set from today titled "device-global identifiers and routes
introduced" _does_ depend on this patch that was missing the
brace--this is indicated in the patch notes as requested by Ian.


Hmm yes, it appears patch 07/13 fixes this one, as well as depending on it!

Personally, I'd be inclined to combine them into a single series of 14 
patches.


But do what Greg wants (separate series). :)

--
-=( Ian Abbott  || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268.  Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [patch 00/11] x86/vdso: Cleanups, simmplifications and CLOCK_TAI support

[Vitaly Kuznetsov]

Andy Lutomirski  writes:

>> On Oct 3, 2018, at 2:22 AM, Vitaly Kuznetsov  wrote:
>> 
>> Andy Lutomirski  writes:
>> 
>>> Hi Vitaly, Paolo, Radim, etc.,
>>> 
>> The notification you're talking about exists, it is called
>> Reenligntenment, see 0092e4346f49 "x86/kvm: Support Hyper-V
>> reenlightenment"). When TSC page changes (and this only happens when L1
>> is migrated to a different host with a different TSC frequency and TSC
>> scaling is not supported by the CPU) we receive an interrupt in L1 (at
>> this moment all TSC accesses are emulated which guarantees the
>> correctness of the readings), pause all L2 guests, update their kvmclock
>> structures with new data (we already know the new TSC frequency) and
>> then tell L0 that we're done and it can stop emulating TSC accesses.
>
> That’s delightful!  Does the emulation magic also work for L1 user
> mode?

As far as I understand - yes, all rdtsc* calls will trap into L0.

>  If so, couldn’t we drop the HyperV vclock entirely and just
> fold the adjustment into the core timekeeping data?  (Preferably the
> actual core data, which would require core changes, but it could
> plausibly be done in arch code, too.)

Not all Hyper-V hosts support reenlightenment notifications (and, if I'm
not mistaken, you need to enable nesting for the VM to get the feature -
and most VMs don't have this) so I think we'll have to keep Hyper-V
vclock for the time being.

-- 
Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: comedi: ni_mio_common: protect register write overflow

[Greg Kroah-Hartman]

On Tue, Oct 02, 2018 at 07:24:32PM -0600, Spencer Olson wrote:
> On Tue, Oct 2, 2018 at 6:16 PM Spencer Olson  wrote:
> >
> > On Tue, Oct 2, 2018 at 11:32 AM Greg Kroah-Hartman
> >  wrote:
> > >
> > > On Wed, Sep 19, 2018 at 10:17:19AM -0600, Spencer E. Olson wrote:
> > > > Fixes two problems introduced as early as
> > > > commit 03aef4b6dc12  ("Staging: comedi: add ni_mio_common code"):
> > > > (1) Ensures that the last four bits of NISTC_RTSI_TRIGB_OUT_REG 
> > > > register is
> > > > not unduly overwritten on e-series devices.  On e-series devices, 
> > > > the
> > > > first three of the last four bits are reserved.  The last bit 
> > > > defines
> > > > the output selection of the RGOUT0 pin, otherwise known as
> > > > RTSI_Sub_Selection.  For m-series devices, these last four bits are
> > > > indeed used as the output selection of the RTSI7 pin (and the
> > > > RTSI_Sub_Selection bit for the RGOUT0 pin is moved to the
> > > > RTSI_Trig_Direction register.
> > > > (2) Allows all 4 RTSI_BRD lines to be treated as valid sources for RTSI
> > > > lines.
> > > >
> > > > This patch also cleans up the ni_get_rtsi_routing command for 
> > > > readability.
> > > >
> > > > Fixes: 03aef4b6dc12  ("Staging: comedi: add ni_mio_common code")
> > > > Signed-off-by: Spencer E. Olson 
> > > > Reviewed-by: Ian Abbott 
> > > > Cc: stable 
> > > > ---
> > > > I thought I had already submitted this patch a while ago...  Whoops.
> > > >  .../staging/comedi/drivers/ni_mio_common.c| 24 +--
> > > >  1 file changed, 17 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c 
> > > > b/drivers/staging/comedi/drivers/ni_mio_common.c
> > > > index 4dee2fc37aed..4d0d0621780e 100644
> > > > --- a/drivers/staging/comedi/drivers/ni_mio_common.c
> > > > +++ b/drivers/staging/comedi/drivers/ni_mio_common.c
> > > > @@ -4980,7 +4980,10 @@ static int ni_valid_rtsi_output_source(struct 
> > > > comedi_device *dev,
> > > >   case NI_RTSI_OUTPUT_G_SRC0:
> > > >   case NI_RTSI_OUTPUT_G_GATE0:
> > > >   case NI_RTSI_OUTPUT_RGOUT0:
> > > > - case NI_RTSI_OUTPUT_RTSI_BRD_0:
> > > > + case NI_RTSI_OUTPUT_RTSI_BRD(0):
> > > > + case NI_RTSI_OUTPUT_RTSI_BRD(1):
> > > > + case NI_RTSI_OUTPUT_RTSI_BRD(2):
> > > > + case NI_RTSI_OUTPUT_RTSI_BRD(3):
> > > >   return 1;
> > > >   case NI_RTSI_OUTPUT_RTSI_OSC:
> > > >   return (devpriv->is_m_series) ? 1 : 0;
> > > > @@ -5001,11 +5004,18 @@ static int ni_set_rtsi_routing(struct 
> > > > comedi_device *dev,
> > > >   devpriv->rtsi_trig_a_output_reg |= NISTC_RTSI_TRIG(chan, 
> > > > src);
> > > >   ni_stc_writew(dev, devpriv->rtsi_trig_a_output_reg,
> > > > NISTC_RTSI_TRIGA_OUT_REG);
> > > > - } else if (chan < 8) {
> > > > + } else if (chan < NISTC_RTSI_TRIG_NUM_CHAN(devpriv->is_m_series)) 
> > > > {
> > > >   devpriv->rtsi_trig_b_output_reg &= 
> > > > ~NISTC_RTSI_TRIG_MASK(chan);
> > > >   devpriv->rtsi_trig_b_output_reg |= NISTC_RTSI_TRIG(chan, 
> > > > src);
> > > >   ni_stc_writew(dev, devpriv->rtsi_trig_b_output_reg,
> > > > NISTC_RTSI_TRIGB_OUT_REG);
> > > > + } else if (chan != NISTC_RTSI_TRIG_OLD_CLK_CHAN) {
> > > > + /* probably should never reach this, since the
> > > > +  * ni_valid_rtsi_output_source above errors out if chan 
> > > > is too
> > > > +  * high
> > > > +  */
> > > > + dev_err(dev->class_dev, "%s: unknown rtsi channel\n", 
> > > > __func__);
> > >
> > > This patch breaks the build very badly.  Always test-build your patches
> > > at the very least :(
> > >
> > > greg k-h
> >
> > I have been test building this with at least a fairly recent
> > staging-next rebase (rebase a week or two ago).  I'll rebase again and
> > check this out
> 
> So the problem had been that I had been compiling the entire time with
> my other patch set that I recently have just submitted.  When I split
> this patch off from that patch set, I had neglected to compile with it
> by itsself.  The issue was a forgotten "{" at the beginning of the
> last if statement.

Each patch has to be self-contained to not break the build :(

> Should I resubmit this patch and the entire patch set from earlier
> today, each separately?

This patch needed to be applied separately as it fixed a bug that needed
to be backported to the stable kernels, right?  So for that, I need a
single patch that works :)

If your other patches then depend on this patch being applied (which is
fine), you should rebase your other series on top of it and resend them.

> The patch set from today titled "device-global identifiers and routes
> introduced" _does_ depend on this patch that was missing the
> brace--this is indicated in the patch notes as requested by Ian.

Ok, good, so all is fine.  If you

Re: [PATCH] staging: comedi: ni_mio_common: protect register write overflow

[Ian Abbott]

On 03/10/18 02:24, Spencer Olson wrote:

On Tue, Oct 2, 2018 at 6:16 PM Spencer Olson  wrote:


On Tue, Oct 2, 2018 at 11:32 AM Greg Kroah-Hartman
 wrote:


On Wed, Sep 19, 2018 at 10:17:19AM -0600, Spencer E. Olson wrote:

[snip]

diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c 
b/drivers/staging/comedi/drivers/ni_mio_common.c
index 4dee2fc37aed..4d0d0621780e 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -4980,7 +4980,10 @@ static int ni_valid_rtsi_output_source(struct 
comedi_device *dev,
   case NI_RTSI_OUTPUT_G_SRC0:
   case NI_RTSI_OUTPUT_G_GATE0:
   case NI_RTSI_OUTPUT_RGOUT0:
- case NI_RTSI_OUTPUT_RTSI_BRD_0:
+ case NI_RTSI_OUTPUT_RTSI_BRD(0):
+ case NI_RTSI_OUTPUT_RTSI_BRD(1):
+ case NI_RTSI_OUTPUT_RTSI_BRD(2):
+ case NI_RTSI_OUTPUT_RTSI_BRD(3):
   return 1;
   case NI_RTSI_OUTPUT_RTSI_OSC:
   return (devpriv->is_m_series) ? 1 : 0;
@@ -5001,11 +5004,18 @@ static int ni_set_rtsi_routing(struct comedi_device 
*dev,
   devpriv->rtsi_trig_a_output_reg |= NISTC_RTSI_TRIG(chan, src);
   ni_stc_writew(dev, devpriv->rtsi_trig_a_output_reg,
 NISTC_RTSI_TRIGA_OUT_REG);
- } else if (chan < 8) {
+ } else if (chan < NISTC_RTSI_TRIG_NUM_CHAN(devpriv->is_m_series)) {
   devpriv->rtsi_trig_b_output_reg &= ~NISTC_RTSI_TRIG_MASK(chan);
   devpriv->rtsi_trig_b_output_reg |= NISTC_RTSI_TRIG(chan, src);
   ni_stc_writew(dev, devpriv->rtsi_trig_b_output_reg,
 NISTC_RTSI_TRIGB_OUT_REG);
+ } else if (chan != NISTC_RTSI_TRIG_OLD_CLK_CHAN) {
+ /* probably should never reach this, since the
+  * ni_valid_rtsi_output_source above errors out if chan is too
+  * high
+  */


While you're fixing it, could that be changed to the usual block comment 
format?



+ dev_err(dev->class_dev, "%s: unknown rtsi channel\n", __func__);


This patch breaks the build very badly.  Always test-build your patches
at the very least :(

greg k-h


I have been test building this with at least a fairly recent
staging-next rebase (rebase a week or two ago).  I'll rebase again and
check this out


So the problem had been that I had been compiling the entire time with
my other patch set that I recently have just submitted.  When I split
this patch off from that patch set, I had neglected to compile with it
by itsself.  The issue was a forgotten "{" at the beginning of the
last if statement.

Should I resubmit this patch and the entire patch set from earlier
today, each separately?

The patch set from today titled "device-global identifiers and routes
introduced" _does_ depend on this patch that was missing the
brace--this is indicated in the patch notes as requested by Ian.


Hmm yes, it appears patch 07/13 fixes this one, as well as depending on it!

Personally, I'd be inclined to combine them into a single series of 14 
patches.



I did just check to make sure that I had not made the same mistake on
the other patch set submitted earlier that was titled: "Add facility
to directly query subdevice timing".  That patch set is fine as is and
did not depend on any of the other patches I had been working on.


--
-=( Ian Abbott  || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268.  Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [patch 00/11] x86/vdso: Cleanups, simmplifications and CLOCK_TAI support

[Andy Lutomirski]

> On Oct 3, 2018, at 2:22 AM, Vitaly Kuznetsov  wrote:
> 
> Andy Lutomirski  writes:
> 
>> Hi Vitaly, Paolo, Radim, etc.,
>> 
>>> On Fri, Sep 14, 2018 at 5:52 AM Thomas Gleixner  wrote:
>>> 
>>> Matt attempted to add CLOCK_TAI support to the VDSO clock_gettime()
>>> implementation, which extended the clockid switch case and added yet
>>> another slightly different copy of the same code.
>>> 
>>> Especially the extended switch case is problematic as the compiler tends to
>>> generate a jump table which then requires to use retpolines. If jump tables
>>> are disabled it adds yet another conditional to the existing maze.
>>> 
>>> This series takes a different approach by consolidating the almost
>>> identical functions into one implementation for high resolution clocks and
>>> one for the coarse grained clock ids by storing the base data for each
>>> clock id in an array which is indexed by the clock id.
>>> 
>> 
>> I was trying to understand more of the implications of this patch
>> series, and I was again reminded that there is an entire extra copy of
>> the vclock reading code in arch/x86/kvm/x86.c.  And the purpose of
>> that code is very, very opaque.
>> 
>> Can one of you explain what the code is even doing?  From a couple of
>> attempts to read through it, it's a whole bunch of
>> probably-extremely-buggy code that, drumroll please, tries to
>> atomically read the TSC value and the time.  And decide whether the
>> result is "based on the TSC".  And then synthesizes a TSC-to-ns
>> multiplier and shift, based on *something other than the actual
>> multiply and shift used*.
>> 
>> IOW, unless I'm totally misunderstanding it, the code digs into the
>> private arch clocksource data intended for the vDSO, uses a poorly
>> maintained copy of the vDSO code to read the time (instead of doing
>> the sane thing and using the kernel interfaces for this), and
>> propagates a totally made up copy to the guest.  And gets it entirely
>> wrong when doing nested virt, since, unless there's some secret in
>> this maze, it doesn't acutlaly use the scaling factor from the host
>> when it tells the guest what to do.
>> 
>> I am really, seriously tempted to send a patch to simply delete all
>> this code.  The correct way to do it is to hook
> 
> "I have discovered a truly marvelous proof of this, which this margin is
> too narrow to contain" :-)
> 
> There is a very long history of different (hardware) issues Marcelo was
> fighting with and the current code is the survived Frankenstein. E.g. it
> is very, very unclear what "catchup", "always catchup" and
> masterclock-less mode in general are and if we still need them.
> 
> That said I'm all for simplification. I'm not sure if we still need to
> care about buggy hardware though.
> 
>> 
>> And I don't see how it's even possible to pass kvmclock correctly to
>> the L2 guest when L0 is hyperv.  KVM could pass *hyperv's* clock, but
>> L1 isn't notified when the data structure changes, so how the heck is
>> it supposed to update the kvmclock structure?
> 
> Well, this kind of works in the the followin way:
> L1's clocksource is 'tsc_page' which is, basically, a compliment to TSC:
> two numbers provided by L0: offset and scale and KVM was tought to treat
> this clocksource as a good one (see b0c39dc68e3b "x86/kvm: Pass stable
> clocksource to guests when running nested on Hyper-V").
> 
> The notification you're talking about exists, it is called
> Reenligntenment, see 0092e4346f49 "x86/kvm: Support Hyper-V
> reenlightenment"). When TSC page changes (and this only happens when L1
> is migrated to a different host with a different TSC frequency and TSC
> scaling is not supported by the CPU) we receive an interrupt in L1 (at
> this moment all TSC accesses are emulated which guarantees the
> correctness of the readings), pause all L2 guests, update their kvmclock
> structures with new data (we already know the new TSC frequency) and
> then tell L0 that we're done and it can stop emulating TSC accesses.

That’s delightful!  Does the emulation magic also work for L1 user mode?  If 
so, couldn’t we drop the HyperV vclock entirely and just fold the adjustment 
into the core timekeeping data?  (Preferably the actual core data, which would 
require core changes, but it could plausibly be done in arch code, too.)

> 
> (Nothing like this exists for KVM-on-KVM, by the way, when L1's
> clocksource is 'kvmclock' L2s won't get a stable kvmclock clocksource.)
> 
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [patch 00/11] x86/vdso: Cleanups, simmplifications and CLOCK_TAI support

[Vitaly Kuznetsov]

Andy Lutomirski  writes:

> Hi Vitaly, Paolo, Radim, etc.,
>
> On Fri, Sep 14, 2018 at 5:52 AM Thomas Gleixner  wrote:
>>
>> Matt attempted to add CLOCK_TAI support to the VDSO clock_gettime()
>> implementation, which extended the clockid switch case and added yet
>> another slightly different copy of the same code.
>>
>> Especially the extended switch case is problematic as the compiler tends to
>> generate a jump table which then requires to use retpolines. If jump tables
>> are disabled it adds yet another conditional to the existing maze.
>>
>> This series takes a different approach by consolidating the almost
>> identical functions into one implementation for high resolution clocks and
>> one for the coarse grained clock ids by storing the base data for each
>> clock id in an array which is indexed by the clock id.
>>
>
> I was trying to understand more of the implications of this patch
> series, and I was again reminded that there is an entire extra copy of
> the vclock reading code in arch/x86/kvm/x86.c.  And the purpose of
> that code is very, very opaque.
>
> Can one of you explain what the code is even doing?  From a couple of
> attempts to read through it, it's a whole bunch of
> probably-extremely-buggy code that, drumroll please, tries to
> atomically read the TSC value and the time.  And decide whether the
> result is "based on the TSC".  And then synthesizes a TSC-to-ns
> multiplier and shift, based on *something other than the actual
> multiply and shift used*.
>
> IOW, unless I'm totally misunderstanding it, the code digs into the
> private arch clocksource data intended for the vDSO, uses a poorly
> maintained copy of the vDSO code to read the time (instead of doing
> the sane thing and using the kernel interfaces for this), and
> propagates a totally made up copy to the guest.  And gets it entirely
> wrong when doing nested virt, since, unless there's some secret in
> this maze, it doesn't acutlaly use the scaling factor from the host
> when it tells the guest what to do.
>
> I am really, seriously tempted to send a patch to simply delete all
> this code.  The correct way to do it is to hook

"I have discovered a truly marvelous proof of this, which this margin is
too narrow to contain" :-)

There is a very long history of different (hardware) issues Marcelo was
fighting with and the current code is the survived Frankenstein. E.g. it
is very, very unclear what "catchup", "always catchup" and
masterclock-less mode in general are and if we still need them.

That said I'm all for simplification. I'm not sure if we still need to
care about buggy hardware though.

>
> And I don't see how it's even possible to pass kvmclock correctly to
> the L2 guest when L0 is hyperv.  KVM could pass *hyperv's* clock, but
> L1 isn't notified when the data structure changes, so how the heck is
> it supposed to update the kvmclock structure?

Well, this kind of works in the the followin way:
L1's clocksource is 'tsc_page' which is, basically, a compliment to TSC:
two numbers provided by L0: offset and scale and KVM was tought to treat
this clocksource as a good one (see b0c39dc68e3b "x86/kvm: Pass stable
clocksource to guests when running nested on Hyper-V").

The notification you're talking about exists, it is called
Reenligntenment, see 0092e4346f49 "x86/kvm: Support Hyper-V
reenlightenment"). When TSC page changes (and this only happens when L1
is migrated to a different host with a different TSC frequency and TSC
scaling is not supported by the CPU) we receive an interrupt in L1 (at
this moment all TSC accesses are emulated which guarantees the
correctness of the readings), pause all L2 guests, update their kvmclock
structures with new data (we already know the new TSC frequency) and
then tell L0 that we're done and it can stop emulating TSC accesses.

(Nothing like this exists for KVM-on-KVM, by the way, when L1's
clocksource is 'kvmclock' L2s won't get a stable kvmclock clocksource.)

-- 
Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel