Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
On 28/12/2016 18:09, Roman Kagan wrote: > Am I correct assuming that QEMU is currently the only user of > arch/x86/include/uapi/asm/hyperv.h? > > Then I think we're fine withdrawing it from uapi as a whole and letting > QEMU pull it in through its header-harvesting scripts (as does now > anyway). This would lift all licensing and longterm API stability > expectations. Actually, QEMU's header-harvesting scripts use uapi/ headers exclusively, since they are built on "make headers_install". The extra cleanups that QEMU does on top are to allow compilation of the headers on non-Linux machines. They don't really do much more than changing Linux (linux/types.h) integer types to the C99 (stdint.h) equivalents. Paolo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] greybus: timesync: replace init_timer with setup_timer
On Tue, Dec 13, 2016 at 10:39:57PM +0800, Xie Qirong wrote: > The combination of init_timer and setting up the data and function field > manually is equivalent to calling setup_timer(). This is an api > consolidation only and improves readability. > > Signed-off-by: Xie Qirong > --- > > setup_timer.cocci suggested the following improvement: > drivers/staging/greybus/timesync.c:1058:1-11: Use setup_timer function > for function on line 1059. > > Patch was compile checked with: x86_64_defconfig + CONFIG_STAGING=y, > CONFIG_GREYBUS=m > > Kernel version: 4.9.0 (localversion-next is next-20161213) > > drivers/staging/greybus/timesync.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/greybus/timesync.c > b/drivers/staging/greybus/timesync.c > index 29e6c1c..af83e12 100644 > --- a/drivers/staging/greybus/timesync.c > +++ b/drivers/staging/greybus/timesync.c > @@ -1055,10 +1055,9 @@ int gb_timesync_svc_add(struct gb_svc *svc) > goto done; > } > > - init_timer(×ync_svc->ktime_timer); > - timesync_svc->ktime_timer.function = gb_timesync_ktime_timer_fn; > + setup_timer(×ync_svc->ktime_timer, gb_timesync_ktime_timer_fn, > + (unsigned long)timesync_svc); Please make sure to align the continuation line to the open parenthesis (which is the style followed in this file). > timesync_svc->ktime_timer.expires = jiffies + GB_TIMESYNC_KTIME_UPDATE; > - timesync_svc->ktime_timer.data = (unsigned long)timesync_svc; > add_timer(×ync_svc->ktime_timer); > done: > mutex_unlock(&gb_timesync_svc_list_mutex); When resending you can add my Reviewed-by: Johan Hovold Thanks, Johan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: arche-apb-ctrl: fix unused warnings on resume/suspend
On Fri, Dec 16, 2016 at 07:33:02PM -0500, Jérémy Lefaure wrote: > When CONFIG_PM_SLEEP is disabled, SIMPLE_DEV_PM_OPS does not use > arche_apb_ctrl_resume and arche_apb_ctrl_suspend functions: > > drivers/staging/greybus/arche-apb-ctrl.c:478:12: warning: > ‘arche_apb_ctrl_resume’ defined but not used [-Wunused-function] > static int arche_apb_ctrl_resume(struct device *dev) > ^ > drivers/staging/greybus/arche-apb-ctrl.c:464:12: warning: > ‘arche_apb_ctrl_suspend’ defined but not used [-Wunused-function] > static int arche_apb_ctrl_suspend(struct device *dev) > ^~ > > Adding __maybe_unused to the declaration of these functions removes the > warnings. > > Signed-off-by: Jérémy Lefaure Acked-by: Johan Hovold Thanks, Johan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/6] staging: fbtft: convert fbtft_reset() to be non-atomic
First of all, fbtft in current state doesn't allow to override GPIOs to be
optional, like "reset" one. It might be a bug somewhere, but rather out of
scope of this fix.
Second, not all GPIOs available on the board would be SoC based, some of them
might sit on I2C GPIO expanders, for example, on Intel Edison/Arduino, and thus
any communication with them might sleep.
Besides that using udelay() and mdelay() is kinda resource wasteful.
Summarize all of the above, convert fbtft_reset() function to non-atomic
variant by using gpio_set_value_cansleep(), usleep_range(), and msleep().
To avoid potential use in atomic context annotate it via might_sleep() macro.
Signed-off-by: Andy Shevchenko
---
drivers/staging/fbtft/fbtft-core.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c
b/drivers/staging/fbtft/fbtft-core.c
index bbe89c9c4fb9..e8bf0d1ec11f 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -336,11 +336,14 @@ static void fbtft_reset(struct fbtft_par *par)
{
if (par->gpio.reset == -1)
return;
+
+ might_sleep();
+
fbtft_par_dbg(DEBUG_RESET, par, "%s()\n", __func__);
- gpio_set_value(par->gpio.reset, 0);
- udelay(20);
- gpio_set_value(par->gpio.reset, 1);
- mdelay(120);
+ gpio_set_value_cansleep(par->gpio.reset, 0);
+ usleep_range(20, 40);
+ gpio_set_value_cansleep(par->gpio.reset, 1);
+ msleep(120);
}
static void fbtft_update_display(struct fbtft_par *par, unsigned int
start_line,
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 5/6] staging: fbtft: fb_ssd1306: Support smaller screen sizes
There is 64x48 display exists. In order to support that set multiplexer
to 48 pixels and window address to proper position in the graphic display
data RAM.
Signed-off-by: Andy Shevchenko
---
drivers/staging/fbtft/fb_ssd1306.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/drivers/staging/fbtft/fb_ssd1306.c
b/drivers/staging/fbtft/fb_ssd1306.c
index 80fc57029fee..bede2d5a5571 100644
--- a/drivers/staging/fbtft/fb_ssd1306.c
+++ b/drivers/staging/fbtft/fb_ssd1306.c
@@ -62,6 +62,8 @@ static int init_display(struct fbtft_par *par)
write_reg(par, 0xA8);
if (par->info->var.yres == 64)
write_reg(par, 0x3F);
+ else if (par->info->var.yres == 48)
+ write_reg(par, 0x2F);
else
write_reg(par, 0x1F);
@@ -95,6 +97,9 @@ static int init_display(struct fbtft_par *par)
if (par->info->var.yres == 64)
/* A[4]=1b, Alternative COM pin configuration */
write_reg(par, 0x12);
+ else if (par->info->var.yres == 48)
+ /* A[4]=1b, Alternative COM pin configuration */
+ write_reg(par, 0x12);
else
/* A[4]=0b, Sequential COM pin configuration */
write_reg(par, 0x02);
@@ -124,6 +129,19 @@ static int init_display(struct fbtft_par *par)
return 0;
}
+static void set_addr_win_64x48(struct fbtft_par *par)
+{
+ /* Set Column Address */
+ write_reg(par, 0x21);
+ write_reg(par, 0x20);
+ write_reg(par, 0x5F);
+
+ /* Set Page Address */
+ write_reg(par, 0x22);
+ write_reg(par, 0x0);
+ write_reg(par, 0x5);
+}
+
static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int ye)
{
/* Set Lower Column Start Address for Page Addressing Mode */
@@ -132,6 +150,9 @@ static void set_addr_win(struct fbtft_par *par, int xs, int
ys, int xe, int ye)
write_reg(par, 0x10 | 0x0);
/* Set Display Start Line */
write_reg(par, 0x40 | 0x0);
+
+ if (par->info->var.xres == 64 && par->info->var.yres == 48)
+ set_addr_win_64x48(par);
}
static int blank(struct fbtft_par *par, bool on)
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 6/6] staging: fbtft: fb_ssd1306: Refactor write_vmem()
Refactor write_vmem() for sake of readability.
While here, fix indentation in one comment.
Signed-off-by: Andy Shevchenko
---
drivers/staging/fbtft/fb_ssd1306.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/fbtft/fb_ssd1306.c
b/drivers/staging/fbtft/fb_ssd1306.c
index bede2d5a5571..76f7da3c7703 100644
--- a/drivers/staging/fbtft/fb_ssd1306.c
+++ b/drivers/staging/fbtft/fb_ssd1306.c
@@ -84,7 +84,7 @@ static int init_display(struct fbtft_par *par)
/* Vertical addressing mode */
write_reg(par, 0x01);
- /*Set Segment Re-map */
+ /* Set Segment Re-map */
/* column address 127 is mapped to SEG0 */
write_reg(par, 0xA0 | 0x1);
@@ -183,26 +183,24 @@ static int set_gamma(struct fbtft_par *par, unsigned long
*curves)
static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
{
u16 *vmem16 = (u16 *)par->info->screen_buffer;
+ u32 xres = par->info->var.xres;
+ u32 yres = par->info->var.yres;
u8 *buf = par->txbuf.buf;
int x, y, i;
int ret = 0;
- for (x = 0; x < par->info->var.xres; x++) {
- for (y = 0; y < par->info->var.yres/8; y++) {
+ for (x = 0; x < xres; x++) {
+ for (y = 0; y < yres / 8; y++) {
*buf = 0x00;
for (i = 0; i < 8; i++)
- *buf |= (vmem16[(y * 8 + i) *
- par->info->var.xres + x] ?
-1 : 0) << i;
+ *buf |= (vmem16[(y * 8 + i) * xres + x] ? 1 :
0) << i;
buf++;
}
}
/* Write data */
gpio_set_value(par->gpio.dc, 1);
- ret = par->fbtftops.write(par, par->txbuf.buf,
- par->info->var.xres * par->info->var.yres /
- 8);
+ ret = par->fbtftops.write(par, par->txbuf.buf, xres * yres / 8);
if (ret < 0)
dev_err(par->info->device, "write failed and returned: %d\n",
ret);
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/6] staging: fbtft: propagate error code from kstrto*()
kstrto*() functions return proper error code. Do propogate it to the user. Signed-off-by: Andy Shevchenko --- drivers/staging/fbtft/fbtft-sysfs.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/staging/fbtft/fbtft-sysfs.c b/drivers/staging/fbtft/fbtft-sysfs.c index 8d8bd12b90a1..7ec5330a292f 100644 --- a/drivers/staging/fbtft/fbtft-sysfs.c +++ b/drivers/staging/fbtft/fbtft-sysfs.c @@ -14,11 +14,7 @@ static int get_next_ulong(char **str_p, unsigned long *val, char *sep, int base) if (!p_val) return -EINVAL; - ret = kstrtoul(p_val, base, val); - if (ret) - return -EINVAL; - - return 0; + return kstrtoul(p_val, base, val); } int fbtft_gamma_parse_str(struct fbtft_par *par, unsigned long *curves, -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/6] fbtft: make it work with DMA enabled SPI
This series enables 64x48 OLED display and fixes the driver to work with DMA enabled SPI properly. Has been tested on Intel Edison board with Adafruit 2'8" and SSD1306 64x48 (Sparkfun for Intel Edison) OLED displays at their maximum speed (25MHz and 10MHz). Andy Shevchenko (6): staging: fbtft: convert fbtft_reset() to be non-atomic staging: fbtft: do not override DMA coherent mask staging: fbtft: fallback to usual allocation when DMA fails staging: fbtft: propagate error code from kstrto*() staging: fbtft: fb_ssd1306: Support smaller screen sizes staging: fbtft: fb_ssd1306: Refactor write_vmem() drivers/staging/fbtft/fb_ssd1306.c | 37 - drivers/staging/fbtft/fbtft-core.c | 15 +-- drivers/staging/fbtft/fbtft-sysfs.c | 6 +- 3 files changed, 38 insertions(+), 20 deletions(-) -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/6] staging: fbtft: do not override DMA coherent mask
Usually it's not consumer's business to override resources passed from
provider, in particularly DMA coherent mask.
Signed-off-by: Andy Shevchenko
---
drivers/staging/fbtft/fbtft-core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c
b/drivers/staging/fbtft/fbtft-core.c
index e8bf0d1ec11f..226be8c09768 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -841,7 +841,6 @@ struct fb_info *fbtft_framebuffer_alloc(struct
fbtft_display *display,
if (txbuflen > 0) {
#ifdef CONFIG_HAS_DMA
if (dma) {
- dev->coherent_dma_mask = ~0;
txbuf = dmam_alloc_coherent(dev, txbuflen,
&par->txbuf.dma, GFP_DMA);
} else
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/6] staging: fbtft: fallback to usual allocation when DMA fails
Fall back to usual allocation method if DMA coherent allocation fails.
SPI framework will map and use DMA mapped memory when possible.
Signed-off-by: Andy Shevchenko
---
drivers/staging/fbtft/fbtft-core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c
b/drivers/staging/fbtft/fbtft-core.c
index 226be8c09768..9f024986aff4 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -843,7 +843,8 @@ struct fb_info *fbtft_framebuffer_alloc(struct
fbtft_display *display,
if (dma) {
txbuf = dmam_alloc_coherent(dev, txbuflen,
&par->txbuf.dma, GFP_DMA);
- } else
+ }
+ if (!txbuf)
#endif
{
txbuf = devm_kzalloc(par->info->device,
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/6] fbtft: make it work with DMA enabled SPI
On Mon, 2017-01-02 at 13:35 +0200, Andy Shevchenko wrote: > This series enables 64x48 OLED display and fixes the driver to work > with DMA > enabled SPI properly. > > Has been tested on Intel Edison board with Adafruit 2'8" and SSD1306 > 64x48 > (Sparkfun for Intel Edison) OLED displays at their maximum speed > (25MHz and > 10MHz). It should be v1, but here we are. > > Andy Shevchenko (6): > staging: fbtft: convert fbtft_reset() to be non-atomic > staging: fbtft: do not override DMA coherent mask > staging: fbtft: fallback to usual allocation when DMA fails > staging: fbtft: propagate error code from kstrto*() > staging: fbtft: fb_ssd1306: Support smaller screen sizes > staging: fbtft: fb_ssd1306: Refactor write_vmem() > > drivers/staging/fbtft/fb_ssd1306.c | 37 > - > drivers/staging/fbtft/fbtft-core.c | 15 +-- > drivers/staging/fbtft/fbtft-sysfs.c | 6 +- > 3 files changed, 38 insertions(+), 20 deletions(-) > -- Andy Shevchenko Intel Finland Oy ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: add host device function pointer checks
On Tue, Dec 20, 2016 at 02:49:27PM -0600, Jason Hrycay wrote: > Add sanity checks for cport_quiesce and cport_clear before invoking the > callbacks as these function pointers are not required during the host > device registration. This follows the logic implemented elsewhere for > various other function pointers. Yeah, I allowed for some inconsistency here given that these callbacks are mandatory on our current platform. No harm in checking this way though (well, at least as long as we remember to set the pointers). > Signed-off-by: Jason Hrycay Acked-by: Johan Hovold Thanks, Johan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: greybus: svc_watchdog: Fix spaces on a single definition statement
On Thu, Dec 22, 2016 at 04:22:02PM +0100, Emmanuil Chatzipetru wrote:
> Fix coding style issue caught by checkpatch.pl related to the following
> warning:
> - "CHECK: spaces preferred around that '*' (ctx:VxV) "
>
> Signed-off-by: Emmanuil Chatzipetru
Acked-by: Johan Hovold
> ---
> v2: Add svc_watchdog to the subject line
>
> drivers/staging/greybus/svc_watchdog.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/svc_watchdog.c
> b/drivers/staging/greybus/svc_watchdog.c
> index 3729460fb954..5edff71f0773 100644
> --- a/drivers/staging/greybus/svc_watchdog.c
> +++ b/drivers/staging/greybus/svc_watchdog.c
> @@ -11,7 +11,7 @@
> #include
> #include "greybus.h"
>
> -#define SVC_WATCHDOG_PERIOD (2*HZ)
> +#define SVC_WATCHDOG_PERIOD (2 * HZ)
>
> struct gb_svc_watchdog {
> struct delayed_work work;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: greybus: svc_watchdog: replace printk() with pr_err()
On Thu, Dec 22, 2016 at 04:22:03PM +0100, Emmanuil Chatzipetru wrote:
> Fix coding style issue caught by checkpatch.pl related to the following
> warning:
> - "WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then
> dev_err(dev, ... then pr_err(... to printk(KERN_ERR ."
>
> Signed-off-by: Emmanuil Chatzipetru
> ---
> v2: Add svc_watchdog to the subject line
Thanks for the update.
Acked-by: Johan Hovold
> drivers/staging/greybus/svc_watchdog.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/svc_watchdog.c
> b/drivers/staging/greybus/svc_watchdog.c
> index 5edff71f0773..d8af2d5d0025 100644
> --- a/drivers/staging/greybus/svc_watchdog.c
> +++ b/drivers/staging/greybus/svc_watchdog.c
> @@ -56,7 +56,7 @@ static void greybus_reset(struct work_struct *work)
> NULL,
> };
>
> - printk(KERN_ERR "svc_watchdog: calling \"%s %s\" to reset greybus
> network!\n",
> + pr_err("svc_watchdog: calling \"%s %s\" to reset greybus network!\n",
> argv[0], argv[1]);
> call_usermodehelper(start_path, argv, envp, UMH_WAIT_EXEC);
> }
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/2] staging: greybus: loopback: use gb_loopback_async_wait_all don't spin
On Thu, Dec 22, 2016 at 12:37:28AM +, Bryan O'Donoghue wrote: > Currently the greybus-loopback thread logic spins around waiting for > send_count == iteration_max which on real hardware doesn't make a > difference to us but in simulation is excruciatingly slow, anti-social and > bad manners. Use the existing gb_loopback_async_wait_all() function to gate > continuing when the send_count == iteration_max and go to sleep until > there's something worthwhile to-do. > > Signed-off-by: Bryan O'Donoghue Reviewed-by: Johan Hovold ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] staging: greybus: loopback_test: Fix race preventing test completion
Adding Axel on CC.
On Thu, Dec 22, 2016 at 12:37:29AM +, Bryan O'Donoghue wrote:
> commit 9250c0ee2626 ("greybus: Loopback_test: use poll instead of
> inotify") changes the flow of determining when to break out of a loop
> polling for loopback test completion.
>
> The clause is_complete() which determines if all tests are complete - as
> used is subject to a race condition where one of the tests has completed
> but at least one other test has not. On real hardware this typically
> doesn't present itself however in gbsim - which is a lot slower due in-part
> to a panopoly of printouts - we see that running a loopback test to more
> than one Interface in gbsim will fail in all instances printing out
> "Iteration count did not finish".
So I've ignored the user-space tool so far, but the description above
does not seem to explain why there is a race here. Could please expand
this a bit?
> We don't even need to have the poll() loop timeout tests in since the
> kernel threads themselves should eventually complete - and if not it's a
> kernel bug. A user of the tool can still terminate tests by doing a
> ctrl-c.
No, we still want to be well-behaved and time out if the test fails to
complete. Consider automatic testing. But you don't seem to be changing
any behaviour in this respect below?
> This patch fixes the race by ensuring the poll() loop waits for all active
> Interfaces to complete - as opposed to the first active Interface before
> breaking the loop.
Really? It looks to me like the current code tries to wait for
notifications on all fds before breaking the loop. Why do say its just
the first?
> Signed-off-by: Bryan O'Donoghue
> ---
> drivers/staging/greybus/tools/loopback_test.c | 10 +-
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/staging/greybus/tools/loopback_test.c
> b/drivers/staging/greybus/tools/loopback_test.c
> index f7f4cd6..f053d4fe 100644
> --- a/drivers/staging/greybus/tools/loopback_test.c
> +++ b/drivers/staging/greybus/tools/loopback_test.c
> @@ -747,7 +747,7 @@ static int wait_for_complete(struct loopback_test *t)
> if (t->poll_timeout.tv_sec != 0)
> ts = &t->poll_timeout;
>
> - while (1) {
> + while (!is_complete(t)) {
>
> ret = ppoll(t->fds, t->poll_count, ts, &mask_old);
> if (ret <= 0) {
> @@ -763,14 +763,6 @@ static int wait_for_complete(struct loopback_test *t)
> number_of_events++;
> }
> }
> -
> - if (number_of_events == t->poll_count)
> - break;
> - }
> -
> - if (!is_complete(t)) {
> - fprintf(stderr, "Iteration count did not finish!\n");
> - return -1;
> }
>
> return 0;
Johan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/5] staging: greybus: operation: add asynchronous gb_operation_cancel
On Tue, Dec 27, 2016 at 01:01:35PM +, Bryan O'Donoghue wrote: > Later patches don't want or need to serialize the cancellation of an > operation. This patch adds gb_operation_cancel_async() as a simple subset > of the existing gb_operation_cancel() sans the synchronous wait on the > cancellation queue. This one is not needed and complicates the interface for no good reason (the async suffix also falsely indicates that this function could be called from atomic context). Just cancel synchronously in the delayed worker function you add later in the series. Johan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] staging: greybus: loopback_test: Fix race preventing test completion
Hi Bryan,
On Mon, Jan 2, 2017 at 3:32 PM, Johan Hovold wrote:
> Adding Axel on CC.
>
> On Thu, Dec 22, 2016 at 12:37:29AM +, Bryan O'Donoghue wrote:
>> commit 9250c0ee2626 ("greybus: Loopback_test: use poll instead of
>> inotify") changes the flow of determining when to break out of a loop
>> polling for loopback test completion.
>>
>> The clause is_complete() which determines if all tests are complete - as
>> used is subject to a race condition where one of the tests has completed
>> but at least one other test has not. On real hardware this typically
>> doesn't present itself however in gbsim - which is a lot slower due in-part
>> to a panopoly of printouts - we see that running a loopback test to more
>> than one Interface in gbsim will fail in all instances printing out
>> "Iteration count did not finish".
>
Im not sure why you might be getting this error. I think the while(1) loop
should have exited when each interface has sent its event, and all the
tests are finished.
If you see "Iteration count did not finish" it means that you got an PRI event
for each polled fd, but the iteration_count does not equal iteration_max on one
of the interfaces.
if the issue is 100% reproducible and if the patch below fixes the issue it
seems that the iteration_count will eventually complete, in which case it
makes me think that either an interface sent an event before it had finished
the test, or some interface sent more than one event?
it would be nice to trace each event that the application is getting and
from which fd it is, and looking at the iteration_count when that happens.
> So I've ignored the user-space tool so far, but the description above
> does not seem to explain why there is a race here. Could please expand
> this a bit?
>
>> We don't even need to have the poll() loop timeout tests in since the
>> kernel threads themselves should eventually complete - and if not it's a
>> kernel bug. A user of the tool can still terminate tests by doing a
>> ctrl-c.
>
> No, we still want to be well-behaved and time out if the test fails to
> complete. Consider automatic testing. But you don't seem to be changing
> any behaviour in this respect below?
>
>> This patch fixes the race by ensuring the poll() loop waits for all active
>> Interfaces to complete - as opposed to the first active Interface before
>> breaking the loop.
>
> Really? It looks to me like the current code tries to wait for
> notifications on all fds before breaking the loop. Why do say its just
> the first?
>
>> Signed-off-by: Bryan O'Donoghue
>> ---
>> drivers/staging/greybus/tools/loopback_test.c | 10 +-
>> 1 file changed, 1 insertion(+), 9 deletions(-)
>>
>> diff --git a/drivers/staging/greybus/tools/loopback_test.c
>> b/drivers/staging/greybus/tools/loopback_test.c
>> index f7f4cd6..f053d4fe 100644
>> --- a/drivers/staging/greybus/tools/loopback_test.c
>> +++ b/drivers/staging/greybus/tools/loopback_test.c
>> @@ -747,7 +747,7 @@ static int wait_for_complete(struct loopback_test *t)
>> if (t->poll_timeout.tv_sec != 0)
>> ts = &t->poll_timeout;
>>
>> - while (1) {
>> + while (!is_complete(t)) {
>>
>> ret = ppoll(t->fds, t->poll_count, ts, &mask_old);
>> if (ret <= 0) {
>> @@ -763,14 +763,6 @@ static int wait_for_complete(struct loopback_test *t)
>> number_of_events++;
>> }
>> }
>> -
>> - if (number_of_events == t->poll_count)
>> - break;
if you remove this check you can remove the number_of_events variable
and logic to increment it, but i think something else might be hiding here.
>> - }
>> -
>> - if (!is_complete(t)) {
>> - fprintf(stderr, "Iteration count did not finish!\n");
>> - return -1;
>> }
>>
>> return 0;
>
> Johan
Regards
Axel.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/6] staging: fbtft: propagate error code from kstrto*()
Hi Andy,
[auto build test WARNING on staging/staging-testing]
[also build test WARNING on v4.10-rc2 next-20161224]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Andy-Shevchenko/fbtft-make-it-work-with-DMA-enabled-SPI/20170102-201151
config: openrisc-allyesconfig (attached as .config)
compiler: or32-linux-gcc (GCC) 4.5.1-or32-1.0rc1
reproduce:
wget
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=openrisc
All warnings (new ones prefixed by >>):
drivers/staging/fbtft/fbtft-sysfs.c: In function 'get_next_ulong':
>> drivers/staging/fbtft/fbtft-sysfs.c:7:6: warning: unused variable 'ret'
vim +/ret +7 drivers/staging/fbtft/fbtft-sysfs.c
c296d5f9 Thomas Petazzoni 2014-12-31 1 #include "fbtft.h"
f3e5df43 Peter Poklop 2015-03-18 2 #include "internal.h"
c296d5f9 Thomas Petazzoni 2014-12-31 3
c296d5f9 Thomas Petazzoni 2014-12-31 4 static int get_next_ulong(char
**str_p, unsigned long *val, char *sep, int base)
c296d5f9 Thomas Petazzoni 2014-12-31 5 {
c296d5f9 Thomas Petazzoni 2014-12-31 6char *p_val;
c296d5f9 Thomas Petazzoni 2014-12-31 @7int ret;
c296d5f9 Thomas Petazzoni 2014-12-31 8
c296d5f9 Thomas Petazzoni 2014-12-31 9if (!str_p || !(*str_p))
c296d5f9 Thomas Petazzoni 2014-12-31 10return -EINVAL;
c296d5f9 Thomas Petazzoni 2014-12-31 11
c296d5f9 Thomas Petazzoni 2014-12-31 12p_val = strsep(str_p, sep);
c296d5f9 Thomas Petazzoni 2014-12-31 13
c296d5f9 Thomas Petazzoni 2014-12-31 14if (!p_val)
c296d5f9 Thomas Petazzoni 2014-12-31 15return -EINVAL;
:: The code at line 7 was first introduced by commit
:: c296d5f9957c03994a699d6739c27d4581a9f6c7 staging: fbtft: core support
:: TO: Thomas Petazzoni
:: CC: Greg Kroah-Hartman
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/6] staging: fbtft: do not override DMA coherent mask
Den 02.01.2017 12:35, skrev Andy Shevchenko:
Usually it's not consumer's business to override resources passed from
provider, in particularly DMA coherent mask.
Signed-off-by: Andy Shevchenko
---
drivers/staging/fbtft/fbtft-core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c
b/drivers/staging/fbtft/fbtft-core.c
index e8bf0d1ec11f..226be8c09768 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -841,7 +841,6 @@ struct fb_info *fbtft_framebuffer_alloc(struct
fbtft_display *display,
if (txbuflen > 0) {
#ifdef CONFIG_HAS_DMA
if (dma) {
- dev->coherent_dma_mask = ~0;
Can we make this conditional like in of_dma_configure():
if (!dev->coherent_dma_mask)
dev->coherent_dma_mask = DMA_BIT_MASK(32);
If not, I guess the mask has to be set before adding the spi device in
fbtft_device.c to keep it from breaking.
Noralf.
txbuf = dmam_alloc_coherent(dev, txbuflen,
&par->txbuf.dma, GFP_DMA);
} else
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 1/2] staging: iio: ad7606: replace range/range_available with corresponding scale
On 12/30/2016 08:59 PM, Jonathan Cameron wrote: > On 11/12/16 02:47, Eva Rachel Retuya wrote: >> Eliminate the non-standard attributes in_voltage_range and >> in_voltage_range_available. Implement in_voltage_scale_available in place >> of these attributes and update the SCALE accordingly. The array >> scale_avail is introduced to hold the available scale values. >> >> Signed-off-by: Eva Rachel Retuya > Looks good to me. > > Lars, if you have a minute to look over this as well that would > be great. I think this is OK as is, since it is an improvement over the current situation. But there is still the open issue that we don't know the scale setting if pin is hardwired. I'd like to see that addressed before the driver is moved out of staging. Similar for the oversampling ratio. Consider this Acked-by: Lars-Peter Clausen ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 2/2] staging: iio: ad7606: move out of staging
On 12/30/2016 09:16 PM, Jonathan Cameron wrote: > On 11/12/16 02:47, Eva Rachel Retuya wrote: >> Move the ad7606 driver from staging/iio/adc to iio/adc. Also, update the >> corresponding Makefile and Kconfig associated with the change. >> >> Signed-off-by: Eva Rachel Retuya > Personally (and this is much debated ;) I prefer for the odd case > of staging graduations, that git isn't run with the -M parameter so we > have the whole driver to comment on. > > Anyhow, just casting my eyes over the code so here are some notes: > 1. The whole computing scale available values on the fly seems rather over the > top as they can be easily precomputed and stored in a static const array. > There are only 2 of them! > > 2. There are some single line comments still using multiline syntax (now > I'm really nitpicking ;) > > 3. A slight oddity has crept in with the cleanup of attributes. We now > prevent the appearance of the _scale_available / oversampling_ratio_available > attributes if the gpios are attached, but not _scale or _oversampling. > (oops). If we are going to do this dance, then we should also have an > alternative set of iio_chan_spec array to reflect this. > > 4. I'd drop the 'More devices added in future' comment. It's now the future > and they haven't been yet ;) > I actually have a patch adding more devices, that just waits for the driver to be moved out of staging. But still the comment can be removed it is not really meaningful. > 5. We can write oversampling ratio, but queries to the write_get_fmt will > return an error for that. Probably want to fix that unless I'm missing > something. > > 6. There are some ancient references to 'ring' buffers in the comments and > function naming. We switched over from my hand rolled ring buffer to the > kfifo buffer we now use ages ago. Would be nice to clear those out. > > None of these are really blockers to it moving out of staging > (except the bugs we introduced in the cleanup), but might be nice > to do one last series pinning those down then get a final review done to > see if we missed anything else. > > Been a long time since I looked at this one in any depth and it's certainly > heading in the right direction! ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
On Wed, 28 Dec 2016 20:09:44 +0300 Roman Kagan wrote: > On Wed, Dec 21, 2016 at 07:54:11PM +, KY Srinivasan wrote: > > > > > > > -Original Message- > > > From: Stephen Hemminger [mailto:step...@networkplumber.org] > > > Sent: Wednesday, December 21, 2016 10:03 AM > > > To: Christoph Hellwig > > > Cc: Paolo Bonzini ; Roman Kagan > > > ; Radim Krčmář ; KY > > > Srinivasan ; Vitaly Kuznetsov > > > ; k...@vger.kernel.org; Denis V . Lunev > > > ; Haiyang Zhang ; > > > x...@kernel.org; linux-ker...@vger.kernel.org; Ingo Molnar > > > ; H. Peter Anvin ; > > > de...@linuxdriverproject.org; Thomas Gleixner > > > Subject: Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi > > > > > > On Wed, 21 Dec 2016 09:58:36 -0800 > > > Christoph Hellwig wrote: > > > > > > > On Wed, Dec 21, 2016 at 09:50:49AM -0800, Stephen Hemminger wrote: > > > > > Lastly, there is licensing issues on headers. It would be good to > > > > > have any > > > > > userspace ABI headers licensed with a more liberal license so that > > > > > BSD > > > and DPDK drivers > > > > > could use them directly. Right now each one reinvents. > > > > > > > > Microsoft could easily solves this problem by offering a suitably > > > > liberally licensed header documenting the full HyperV guest protocol > > > > that Linux and other projects could use. > > > > > > The issue is if same header file mixes kernel and userspace API stuff. > > > > > > Once the files are arranged right, I will submit trivial change to > > > comments > > > to indicate the liberal licensing of userspace API headers. > > > > Let us take this one step at a time. I know for a fact that not all the > > guest host > > protocols on Hyper-V are guaranteed to be stable. Some of the protocols are > > part of > > the published MSFT standards such RNDIS and these obviously are guaranteed > > to be > > stable. For the rest it is less clear. The fact that we need to ensure > > compatibility of existing > > Windows guests tells me that any host side changes will be versioned and > > the hosts will always > > support older guests. > > > > I would like to minimize what we include in the uapi header; especially > > when MSFT has made no guarantees > > with regards how they may be evolved. I will also work on getting some > > clarity on both stability and > > under what license we would expose the uapi header. > > Am I correct assuming that QEMU is currently the only user of > arch/x86/include/uapi/asm/hyperv.h? > > Then I think we're fine withdrawing it from uapi as a whole and letting > QEMU pull it in through its header-harvesting scripts (as does now > anyway). This would lift all licensing and longterm API stability > expectations. > > Roman. Thanks, that prevents lots of problems. That is how I handle iproute2 as well. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] hv_util: adjust system time smoothly
(First, I have to admit I'm not a timekeeping expert ...) With TimeSync version 4 protocol support we started updating system time continuously through the whole lifetime of Hyper-V guests. Every 5 seconds there is a time sample from the host which triggers do_settimeofday[64](). While the time from the host is very accurate such adjustments may cause issues: - Time is jumping forward and backward, some applications may misbehave. - In case an NTP client is run in parallel things may go south, e.g. when an NTP client tries to adjust tick/frequency with ADJ_TICK/ADJ_FREQUENCY the Hyper-V module will not see this changes and time will oscillate and never converge. - Systemd starts annoying you by printing "Time has been changed" every 5 seconds to the system log. With this series I suggest to use do_adjtimex() to adjust time. My tests show that such method gives equally good time convergence but avoids all the drawbacks described above. Vitaly Kuznetsov (4): timekeeping: export do_adjtimex() to modules hv_util: switch to using timespec64 hv_util: use do_adjtimex() to update system time hv_util: improve time adjustment accuracy by disabling interrupts drivers/hv/hv_util.c | 33 + kernel/time/timekeeping.c | 1 + 2 files changed, 30 insertions(+), 4 deletions(-) -- 2.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] hv_util: improve time adjustment accuracy by disabling interrupts
If we happen to receive interrupts during hv_set_host_time() execution
our adjustments may get inaccurate. Make the whole function atomic.
Unfortunately, we can's call do_settimeofday64() with interrupts
disabled as some cross-CPU work is being done but this call happens
very rarely.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hv_util.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 4c0fbb0..233d5cb 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -186,6 +186,9 @@ static void hv_set_host_time(struct work_struct *work)
u64 newtime;
struct timespec64 host_ts, our_ts;
struct timex txc = {0};
+ unsigned long flags;
+
+ local_irq_save(flags);
wrk = container_of(work, struct adj_time_work, work);
@@ -214,6 +217,7 @@ static void hv_set_host_time(struct work_struct *work)
/* Try adjusting time by using phase adjustment if possible */
if (abs(delta) > MAXPHASE) {
+ local_irq_restore(flags);
do_settimeofday64(&host_ts);
return;
}
@@ -225,6 +229,8 @@ static void hv_set_host_time(struct work_struct *work)
txc.status = STA_PLL;
txc.offset = delta;
do_adjtimex(&txc);
+
+ local_irq_restore(flags);
}
/*
--
2.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] hv_util: switch to using timespec64
do_settimeofday() is deprecated, use do_settimeofday64() instead. Signed-off-by: Vitaly Kuznetsov --- drivers/hv/hv_util.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c index e770774..94719eb 100644 --- a/drivers/hv/hv_util.c +++ b/drivers/hv/hv_util.c @@ -184,7 +184,7 @@ static void hv_set_host_time(struct work_struct *work) struct adj_time_work*wrk; s64 host_tns; u64 newtime; - struct timespec host_ts; + struct timespec64 host_ts; wrk = container_of(work, struct adj_time_work, work); @@ -203,9 +203,9 @@ static void hv_set_host_time(struct work_struct *work) newtime += (current_tick - wrk->ref_time); } host_tns = (newtime - WLTIMEDELTA) * 100; - host_ts = ns_to_timespec(host_tns); + host_ts = ns_to_timespec64(host_tns); - do_settimeofday(&host_ts); + do_settimeofday64(&host_ts); } /* -- 2.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] hv_util: use do_adjtimex() to update system time
With TimeSync version 4 protocol support we started updating system time
continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
there is a time sample from the host which triggers do_settimeofday[64]().
While the time from the host is very accurate such adjustments may cause
issues:
- Time is jumping forward and backward, some applications may misbehave.
- In case an NTP client is run in parallel things may go south, e.g. when
an NTP client tries to adjust tick/frequency with ADJ_TICK/ADJ_FREQUENCY
the Hyper-V module will not see this changes and time will oscillate and
never converge.
- Systemd starts annoying you by printing "Time has been changed" every 5
seconds to the system log.
Instead of calling do_settimeofday64() we can pretend being an NTP client
and use do_adjtimex().
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hv_util.c | 25 ++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 94719eb..4c0fbb0 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -182,9 +182,10 @@ struct adj_time_work {
static void hv_set_host_time(struct work_struct *work)
{
struct adj_time_work*wrk;
- s64 host_tns;
+ s64 host_tns, our_tns, delta;
u64 newtime;
- struct timespec64 host_ts;
+ struct timespec64 host_ts, our_ts;
+ struct timex txc = {0};
wrk = container_of(work, struct adj_time_work, work);
@@ -205,7 +206,25 @@ static void hv_set_host_time(struct work_struct *work)
host_tns = (newtime - WLTIMEDELTA) * 100;
host_ts = ns_to_timespec64(host_tns);
- do_settimeofday64(&host_ts);
+ getnstimeofday64(&our_ts);
+ our_tns = timespec64_to_ns(&our_ts);
+
+ /* Difference between our time and host time */
+ delta = host_tns - our_tns;
+
+ /* Try adjusting time by using phase adjustment if possible */
+ if (abs(delta) > MAXPHASE) {
+ do_settimeofday64(&host_ts);
+ return;
+ }
+
+ txc.modes = ADJ_TICK | ADJ_FREQUENCY | ADJ_OFFSET | ADJ_NANO |
+ ADJ_STATUS;
+ txc.tick = TICK_USEC;
+ txc.freq = 0;
+ txc.status = STA_PLL;
+ txc.offset = delta;
+ do_adjtimex(&txc);
}
/*
--
2.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] timekeeping: export do_adjtimex() to modules
While do_adjtimex() is available to userspace via adjtimex syscall it is not available to modules which may want to implement in-kernel 'NTP clients'. Hyper-V hv_utils is going to be the first one. Signed-off-by: Vitaly Kuznetsov --- kernel/time/timekeeping.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index da233cd..ae4f24f 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -2312,6 +2312,7 @@ int do_adjtimex(struct timex *txc) return ret; } +EXPORT_SYMBOL_GPL(do_adjtimex); #ifdef CONFIG_NTP_PPS /** -- 2.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/7] hv: CPU onlining/offlining fixes and improvements
Vitaly Kuznetsov writes: > Some time ago we forbade CPU offlining for Hyper-V and this was sufficient > if you boot with all CPUs onlined. Turns out, people may want to limit the > number online CPUs by passing 'maxcpus=' kernel parameter and we hit a > crash in Hyper-V code in this case. After some thinking, I think we may not > only fix the crash but also make the offlining prevention fine-grained: we > need to prevent from offlining CPUs which have VMBus channels attached > only. All offlined CPUs may always be onlined. > > PATCH1 fixes a bug which is not directly related to the series, I hit it > while testing hv_vmbus module unload with this series. > > Vitaly Kuznetsov (7): > hv: acquire vmbus_connection.channel_mutex in vmbus_free_channels() > hv: allocate synic pages for all present CPUs > hv: init percpu_list in hv_synic_alloc() > hv: change clockevents unbind tactics > hv: check all present cpus in vmbus_wait_for_unload() > hv: switch to cpuhp state machine for synic init/cleanup > hv: make CPU offlining prevention fine-grained K. Y., it seems that for some reason only patches 1 and 4 from this series made it upstream (and to char-misc tree). Could you please resend the rest to Greg? Please let me know if you want me to rebase/retest. Thanks! -- Vitaly ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/4] hv_util: improve time adjustment accuracy by disabling interrupts
On Mon, 2 Jan 2017 20:41:14 +0100
Vitaly Kuznetsov wrote:
> If we happen to receive interrupts during hv_set_host_time() execution
> our adjustments may get inaccurate. Make the whole function atomic.
> Unfortunately, we can's call do_settimeofday64() with interrupts
> disabled as some cross-CPU work is being done but this call happens
> very rarely.
>
> Signed-off-by: Vitaly Kuznetsov
> ---
> drivers/hv/hv_util.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index 4c0fbb0..233d5cb 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -186,6 +186,9 @@ static void hv_set_host_time(struct work_struct *work)
> u64 newtime;
> struct timespec64 host_ts, our_ts;
> struct timex txc = {0};
> + unsigned long flags;
> +
> + local_irq_save(flags);
>
> wrk = container_of(work, struct adj_time_work, work);
>
> @@ -214,6 +217,7 @@ static void hv_set_host_time(struct work_struct *work)
>
> /* Try adjusting time by using phase adjustment if possible */
> if (abs(delta) > MAXPHASE) {
> + local_irq_restore(flags);
> do_settimeofday64(&host_ts);
> return;
> }
> @@ -225,6 +229,8 @@ static void hv_set_host_time(struct work_struct *work)
> txc.status = STA_PLL;
> txc.offset = delta;
> do_adjtimex(&txc);
> +
> + local_irq_restore(flags);
Yes, it should be atomic, but local irq save/restore is not sufficient
protection
because it does not protect against premptible kernel. Why not a mutex? or a
spinlock?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/20] i.MX Media Driver
Hi Steve, On Thu, Dec 29, 2016 at 8:27 PM, Steve Longerbeam wrote: > This is a media driver for video capture on i.MX. > > Refer to Documentation/media/v4l-drivers/imx.rst for example capture > pipelines on SabreSD, SabreAuto, and SabreLite reference platforms. > > This patchset includes the OF graph layout as proposed by Philipp Zabel, > with only minor changes which are enumerated in the patch header. Patches 13, 14 and 19 miss your Signed-off-by tag. Tested the whole series on a mx6qsabresd: Tested-by: Fabio Estevam ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 3/4] hv_util: use do_adjtimex() to update system time
> -Original Message-
> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> Sent: Monday, January 2, 2017 11:41 AM
> To: de...@linuxdriverproject.org
> Cc: linux-ker...@vger.kernel.org; KY Srinivasan ;
> Haiyang Zhang ; John Stultz
> ; Thomas Gleixner ; Alex Ng
> (LIS)
> Subject: [PATCH 3/4] hv_util: use do_adjtimex() to update system time
>
> With TimeSync version 4 protocol support we started updating system time
> continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
> there is a time sample from the host which triggers do_settimeofday[64]().
> While the time from the host is very accurate such adjustments may cause
> issues:
> - Time is jumping forward and backward, some applications may misbehave.
> - In case an NTP client is run in parallel things may go south, e.g. when
> an NTP client tries to adjust tick/frequency with ADJ_TICK/ADJ_FREQUENCY
> the Hyper-V module will not see this changes and time will oscillate and
> never converge.
> - Systemd starts annoying you by printing "Time has been changed" every 5
> seconds to the system log.
These are all good points. I am working on a patch to address point 2.
It will allow new TimeSync behavior to be disabled even if the TimeSync IC is
enabled from the host. This can be set to prevent TimeSync IC from interfering
with NTP client.
>
> Instead of calling do_settimeofday64() we can pretend being an NTP client
> and use do_adjtimex().
>
> Signed-off-by: Vitaly Kuznetsov
> ---
> drivers/hv/hv_util.c | 25 ++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c index
> 94719eb..4c0fbb0 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -182,9 +182,10 @@ struct adj_time_work { static void
> hv_set_host_time(struct work_struct *work) {
> struct adj_time_work*wrk;
> - s64 host_tns;
> + s64 host_tns, our_tns, delta;
> u64 newtime;
> - struct timespec64 host_ts;
> + struct timespec64 host_ts, our_ts;
> + struct timex txc = {0};
>
> wrk = container_of(work, struct adj_time_work, work);
>
> @@ -205,7 +206,25 @@ static void hv_set_host_time(struct work_struct
> *work)
> host_tns = (newtime - WLTIMEDELTA) * 100;
> host_ts = ns_to_timespec64(host_tns);
>
> - do_settimeofday64(&host_ts);
> + getnstimeofday64(&our_ts);
> + our_tns = timespec64_to_ns(&our_ts);
> +
> + /* Difference between our time and host time */
> + delta = host_tns - our_tns;
> +
> + /* Try adjusting time by using phase adjustment if possible */
> + if (abs(delta) > MAXPHASE) {
> + do_settimeofday64(&host_ts);
> + return;
> + }
We should also call do_settimeofday64() if the host sends flag
ICTIMESYNCFLAG_SYNC. This is a signal from host that the guest
shall sync with host time immediately (often when the guest has
just booted).
> +
> + txc.modes = ADJ_TICK | ADJ_FREQUENCY | ADJ_OFFSET |
> ADJ_NANO |
> + ADJ_STATUS;
> + txc.tick = TICK_USEC;
> + txc.freq = 0;
I'm not familiar with the ADJ_FREQUENCY flag. What does setting this to 'zero'
achieve?
Are there any side-effects from doing this?
> + txc.status = STA_PLL;
> + txc.offset = delta;
> + do_adjtimex(&txc);
Might be a good idea to handle the return code from do_adjtimex() and log
something
in case of error.
> }
>
> /*
> --
> 2.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
