Re: [PATCH] media: staging: davinci_vpfe: disallow building with COMPILE_TEST

2019-03-05 Thread Geert Uytterhoeven
Hi Arnd,

On Mon, Mar 4, 2019 at 9:30 PM Arnd Bergmann  wrote:
> The driver should really call dm365_isif_setup_pinmux() through a callback,
> but it runs platform specific code by itself, which never actually compiled:
>
> /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:2: error: 
> implicit declaration of function 'davinci_cfg_reg' 
> [-Werror,-Wimplicit-function-declaration]
> davinci_cfg_reg(DM365_VIN_CAM_WEN);
> ^
> /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:2: error: 
> this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
> /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:18: error: 
> use of undeclared identifier 'DM365_VIN_CAM_WEN'
> davinci_cfg_reg(DM365_VIN_CAM_WEN);
> ^
> /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2029:18: error: 
> use of undeclared identifier 'DM365_VIN_CAM_VD'
> davinci_cfg_reg(DM365_VIN_CAM_VD);
> ^
> /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2030:18: error: 
> use of undeclared identifier 'DM365_VIN_CAM_HD'
> davinci_cfg_reg(DM365_VIN_CAM_HD);
> ^
> /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2031:18: error: 
> use of undeclared identifier 'DM365_VIN_YIN4_7_EN'
> davinci_cfg_reg(DM365_VIN_YIN4_7_EN);
> ^
> /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2032:18: error: 
> use of undeclared identifier 'DM365_VIN_YIN0_3_EN'
> davinci_cfg_reg(DM365_VIN_YIN0_3_EN);
> ^
> 7 errors generated.

Which tree and which config is this?
This driver compiles fine with m68k/allmodconfig on v5.0?

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] media: staging/intel-ipu3: reduce kernel stack usage

2019-03-05 Thread Arnd Bergmann
On Tue, Mar 5, 2019 at 8:53 AM Sakari Ailus
 wrote:
> On Tue, Mar 05, 2019 at 12:25:18AM +, Cao, Bingbu wrote:

> > > struct v4l2_pix_format_mplane *const in =
> > > &q[IPU3_CSS_QUEUE_IN].fmt.mpix;
> > > struct v4l2_pix_format_mplane *const out = @@ -1753,6 +1754,11 @@
> > > int imgu_css_fmt_try(struct imgu_css *css,
> > > &q[IPU3_CSS_QUEUE_VF].fmt.mpix;
> > > int i, s, ret;
> > >
> > > +   if (!q) {
> > > +   ret = -ENOMEM;
> > > +   goto out;
> > > +   }
> > [Cao, Bingbu]
> > The goto here is wrong, you can just report an error, and I prefer it is 
> > next to the alloc.
>
> I agree, the goto is just not needed.

Should I remove all the gotos then and do an explicit kfree() in each
error path?

I'd prefer not to mix the two styles, as that can lead to subtle mistakes
when the code is refactored again.

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


Re: [PATCH] media: staging/intel-ipu3: reduce kernel stack usage

2019-03-05 Thread Sakari Ailus
On Tue, Mar 05, 2019 at 09:40:24AM +0100, Arnd Bergmann wrote:
> On Tue, Mar 5, 2019 at 8:53 AM Sakari Ailus
>  wrote:
> > On Tue, Mar 05, 2019 at 12:25:18AM +, Cao, Bingbu wrote:
> 
> > > > struct v4l2_pix_format_mplane *const in =
> > > > &q[IPU3_CSS_QUEUE_IN].fmt.mpix;
> > > > struct v4l2_pix_format_mplane *const out = @@ -1753,6 +1754,11 @@
> > > > int imgu_css_fmt_try(struct imgu_css *css,
> > > > &q[IPU3_CSS_QUEUE_VF].fmt.mpix;
> > > > int i, s, ret;
> > > >
> > > > +   if (!q) {
> > > > +   ret = -ENOMEM;
> > > > +   goto out;
> > > > +   }
> > > [Cao, Bingbu]
> > > The goto here is wrong, you can just report an error, and I prefer it is 
> > > next to the alloc.
> >
> > I agree, the goto is just not needed.
> 
> Should I remove all the gotos then and do an explicit kfree() in each
> error path?
> 
> I'd prefer not to mix the two styles, as that can lead to subtle mistakes
> when the code is refactored again.

In this case there's no need for kfree as q is NULL. Goto is often useful
if you need to do things to undo stuff that was done earlier in the
function but it's not the case here. I prefer keeping the rest gotos.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] media: staging: davinci_vpfe: disallow building with COMPILE_TEST

2019-03-05 Thread Arnd Bergmann
On Tue, Mar 5, 2019 at 9:05 AM Geert Uytterhoeven  wrote:
> On Mon, Mar 4, 2019 at 9:30 PM Arnd Bergmann  wrote:
> > The driver should really call dm365_isif_setup_pinmux() through a callback,
> > but it runs platform specific code by itself, which never actually compiled:
> >
> > /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:2: error: 
> > implicit declaration of function 'davinci_cfg_reg' 
> > [-Werror,-Wimplicit-function-declaration]
> > davinci_cfg_reg(DM365_VIN_CAM_WEN);
> > ^
> > /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:2: error: 
> > this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
> > /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:18: 
> > error: use of undeclared identifier 'DM365_VIN_CAM_WEN'
> > davinci_cfg_reg(DM365_VIN_CAM_WEN);
> > ^
> > /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2029:18: 
> > error: use of undeclared identifier 'DM365_VIN_CAM_VD'
> > davinci_cfg_reg(DM365_VIN_CAM_VD);
> > ^
> > /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2030:18: 
> > error: use of undeclared identifier 'DM365_VIN_CAM_HD'
> > davinci_cfg_reg(DM365_VIN_CAM_HD);
> > ^
> > /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2031:18: 
> > error: use of undeclared identifier 'DM365_VIN_YIN4_7_EN'
> > davinci_cfg_reg(DM365_VIN_YIN4_7_EN);
> > ^
> > /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2032:18: 
> > error: use of undeclared identifier 'DM365_VIN_YIN0_3_EN'
> > davinci_cfg_reg(DM365_VIN_YIN0_3_EN);
> > ^
> > 7 errors generated.
>
> Which tree and which config is this?
> This driver compiles fine with m68k/allmodconfig on v5.0?

Ah, thanks for checking, I found the real issue now:

The Makefile contains a nasty hack that makes it work /almost/ everywhere

# Allow building it with COMPILE_TEST on other archs
ifndef CONFIG_ARCH_DAVINCI
ccflags-y += -I $(srctree)/arch/arm/mach-davinci/include/
endif

This is something we probably don't want to do, but it mostly happens to
do the right thing for compile testing. The case I ran into is the rare
exception of arch/arm/mach-omap1, which has a different mach/mux.h
header, so the '#include ' in the driver gets the omap
file rather than the davinci file, and then misses the davinci_cfg_reg()
declaration and the macros.

One way to work around this is to pile on to the hack by adding
'depends on !ARCH_OMAP1'. Should we do that, or is there
a better way out? Do we actually still need the staging driver
in addition to the one in drivers/media/platform/davinci ?

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


Re: [PATCH] media: staging/intel-ipu3: reduce kernel stack usage

2019-03-05 Thread Arnd Bergmann
On Tue, Mar 5, 2019 at 9:47 AM Sakari Ailus
 wrote:
> On Tue, Mar 05, 2019 at 09:40:24AM +0100, Arnd Bergmann wrote:
> > On Tue, Mar 5, 2019 at 8:53 AM Sakari Ailus  
> > wrote:
> > > On Tue, Mar 05, 2019 at 12:25:18AM +, Cao, Bingbu wrote:
> >
> > > > > struct v4l2_pix_format_mplane *const in =
> > > > > &q[IPU3_CSS_QUEUE_IN].fmt.mpix;
> > > > > struct v4l2_pix_format_mplane *const out = @@ -1753,6 +1754,11 @@
> > > > > int imgu_css_fmt_try(struct imgu_css *css,
> > > > > &q[IPU3_CSS_QUEUE_VF].fmt.mpix;
> > > > > int i, s, ret;
> > > > >
> > > > > +   if (!q) {
> > > > > +   ret = -ENOMEM;
> > > > > +   goto out;
> > > > > +   }
> > > > [Cao, Bingbu]
> > > > The goto here is wrong, you can just report an error, and I prefer it 
> > > > is next to the alloc.
> > >
> > > I agree, the goto is just not needed.
> >
> > Should I remove all the gotos then and do an explicit kfree() in each
> > error path?
> >
> > I'd prefer not to mix the two styles, as that can lead to subtle mistakes
> > when the code is refactored again.
>
> In this case there's no need for kfree as q is NULL. Goto is often useful
> if you need to do things to undo stuff that was done earlier in the
> function but it's not the case here. I prefer keeping the rest gotos.

Ok, I'll send an updated patch the way you suggested then.

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


Re: [bug report] habanalabs: add virtual memory and MMU modules

2019-03-05 Thread Dan Carpenter
On Tue, Mar 05, 2019 at 08:34:50AM +, Omer Shpigelman wrote:
> From: Dan Carpenter 
> Sent: Tuesday, 5 March 2019 8:40
> > Hello Omer Shpigelman,
> > 
> > The patch 0feaf86d4e69: "habanalabs: add virtual memory and MMU
> > modules" from Feb 16, 2019, leads to the following static checker
> > warning:
> > 
> > drivers/misc/habanalabs/memory.c:96 alloc_device_memory()
> > warn: integer overflows '(args->alloc.mem_size + (page_size - 1)) >>
> > page_shift'
> > 
> 
> You are correct, we'll send a fix shortly.
> BTW which static checker did you use? we use Sparse and Smatch and they 
> didn't catch that.

It's not released because it's pretty rubbish.  Checking for integer
overflows is hard.  And then when you find a real integer overflow, it
can can be deliberate and/or harmless.

regards,
dan carpenter

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


Re: [PATCH] staging: comedi: ni_mio_common: Fix divide-by-zero for DIO cmdtest

2019-03-05 Thread Dan Carpenter
On Mon, Mar 04, 2019 at 02:33:54PM +, Ian Abbott wrote:
> `ni_cdio_cmdtest()` validates Comedi asynchronous commands for the DIO
> subdevice (subdevice 2) of supported National Instruments M-series
> cards.  It is called when handling the `COMEDI_CMD` and `COMEDI_CMDTEST`
> ioctls for this subdevice.  There are two causes for a possible
> divide-by-zero error when validating that the `stop_arg` member of the
> passed-in command is not too large.
> 
> The first cause for the divide-by-zero is that calls to
> `comedi_bytes_per_scan()` are only valid once the command has been
> copied to `s->async->cmd`, but that copy is only done for the
> `COMEDI_CMD` ioctl.  For the `COMEDI_CMDTEST` ioctl, it will use
> whatever was left there by the previous `COMEDI_CMD` ioctl, if any.
> (This is very likely, as it is usual for the application to use
> `COMEDI_CMDTEST` before `COMEDI_CMD`.) If there has been no previous,
> valid `COMEDI_CMD` for this subdevice, then `comedi_bytes_per_scan()`
> will return 0, so the subsequent division in `ni_cdio_cmdtest()` of
> `s->async->prealloc_bufsz / comedi_bytes_per_scan(s)` will be a
> divide-by-zero error.  To fix this error, call a new function
> `comedi_bytes_per_scan_cmd(s, cmd)`, based on the existing
> `comedi_bytes_per_scan(s)` but using a specified `struct comedi_cmd` for
> its calculations.  (Also refactor `comedi_bytes_per_scan()` to call the
> new function.)
> 
> Once the first cause for the divide-by-zero has been fixed, the second
> cause is that `comedi_bytes_per_scan_cmd()` can legitimately return 0 if
> the `scan_end_arg` member of the `struct comedi_cmd` being tested is 0.
> Fix it by only performing the division (and validating that `stop_arg`
> is no more than the maximum value) if `comedi_bytes_per_scan_cmd()`
> returns a non-zero value.
> 
> The problem was reported on the COMEDI mailing list here:
> https://groups.google.com/forum/#!topic/comedi_list/4t9WlHzMhKM
> 

Can you give Ivan a Reported-by tag?  It's a public mailing list so
that shouldn't be a problem.

> Fixes: f164cbf98fa8 ("staging: comedi: ni_mio_common: add finite regeneration 
> to dio output")
> Cc:  # 4.6+
> Cc: Spencer E. Olson 
> Signed-off-by: Ian Abbott 
> ---

regards,
dan carpenter


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


Re: [PATCH] staging: comedi: ni_mio_common: Fix divide-by-zero for DIO cmdtest

2019-03-05 Thread Ian Abbott

On 05/03/2019 11:10, Dan Carpenter wrote:

On Mon, Mar 04, 2019 at 02:33:54PM +, Ian Abbott wrote:

The problem was reported on the COMEDI mailing list here:
https://groups.google.com/forum/#!topic/comedi_list/4t9WlHzMhKM



Can you give Ivan a Reported-by tag?  It's a public mailing list so
that shouldn't be a problem.


I can do, but I don't know his full name.  Will that be a problem?




Fixes: f164cbf98fa8 ("staging: comedi: ni_mio_common: add finite regeneration to dio 
output")
Cc:  # 4.6+
Cc: Spencer E. Olson 
Signed-off-by: Ian Abbott 
---


regards,
dan carpenter





--
-=( 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] staging: comedi: ni_mio_common: Fix divide-by-zero for DIO cmdtest

2019-03-05 Thread Dan Carpenter
On Tue, Mar 05, 2019 at 11:32:18AM +, Ian Abbott wrote:
> On 05/03/2019 11:10, Dan Carpenter wrote:
> > On Mon, Mar 04, 2019 at 02:33:54PM +, Ian Abbott wrote:
> > > The problem was reported on the COMEDI mailing list here:
> > > https://groups.google.com/forum/#!topic/comedi_list/4t9WlHzMhKM
> > > 
> > 
> > Can you give Ivan a Reported-by tag?  It's a public mailing list so
> > that shouldn't be a problem.
> 
> I can do, but I don't know his full name.  Will that be a problem?
> 

Nah, it's not a problem.  People should fix their email headers to
reflect what they want to be called.

Or you could ask but I don't think I have ever asked for someone's name
I've always just gone with their email header name.

regards,
dan carpenter

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


Re: [PATCH] staging: comedi: ni_mio_common: Fix divide-by-zero for DIO cmdtest

2019-03-05 Thread Ian Abbott

On 05/03/2019 11:39, Dan Carpenter wrote:

On Tue, Mar 05, 2019 at 11:32:18AM +, Ian Abbott wrote:

On 05/03/2019 11:10, Dan Carpenter wrote:

On Mon, Mar 04, 2019 at 02:33:54PM +, Ian Abbott wrote:

The problem was reported on the COMEDI mailing list here:
https://groups.google.com/forum/#!topic/comedi_list/4t9WlHzMhKM



Can you give Ivan a Reported-by tag?  It's a public mailing list so
that shouldn't be a problem.


I can do, but I don't know his full name.  Will that be a problem?



Nah, it's not a problem.  People should fix their email headers to
reflect what they want to be called.

Or you could ask but I don't think I have ever asked for someone's name
I've always just gone with their email header name.


In this case, Ivan just signed off with that name and it didn't appear 
in the email headers.


--
-=( 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


[PATCH v2] x86, hyperv: fix kernel panic when kexec on HyperV

2019-03-05 Thread Kairui Song
After commit 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments"),
kexec will fail with a kernel panic like this:

kexec_core: Starting new kernel
BUG: unable to handle kernel NULL pointer dereference at 
PGD 800057995067 P4D 800057995067 PUD 57990067 PMD 0
Oops: 0002 [#1] SMP PTI
CPU: 0 PID: 1016 Comm: kexec Not tainted 4.18.16-300.fc29.x86_64 #1
Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS 
Hyper-V UEFI Release v3.0 03/02/2018
RIP: 0010:0xc901d000
Code: Bad RIP value.
RSP: 0018:c9000495bcf0 EFLAGS: 00010046
RAX:  RBX: c901d000 RCX: 00020015
RDX: 7f553000 RSI:  RDI: c9000495bd28
RBP: 0002 R08:  R09: 8238aaf8
R10: 8238aae0 R11:  R12: 88007f553008
R13: 0001 R14: 8800ff553000 R15: 
FS:  7ff5c0e67b80() GS:880078e0() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: c901cfd6 CR3: 4f678006 CR4: 003606f0
Call Trace:
 ? __send_ipi_mask+0x1c6/0x2d0
 ? hv_send_ipi_mask_allbutself+0x6d/0xb0
 ? mp_save_irq+0x70/0x70
 ? __ioapic_read_entry+0x32/0x50
 ? ioapic_read_entry+0x39/0x50
 ? clear_IO_APIC_pin+0xb8/0x110
 ? native_stop_other_cpus+0x6e/0x170
 ? native_machine_shutdown+0x22/0x40
 ? kernel_kexec+0x136/0x156
 ? __do_sys_reboot+0x1be/0x210
 ? kmem_cache_free+0x1b1/0x1e0
 ? __dentry_kill+0x10b/0x160
 ? _cond_resched+0x15/0x30
 ? dentry_kill+0x47/0x170
 ? dput.part.34+0xc6/0x100
 ? __fput+0x147/0x220
 ? _cond_resched+0x15/0x30
 ? task_work_run+0x38/0xa0
 ? do_syscall_64+0x5b/0x160
 ? entry_SYSCALL_64_after_hwframe+0x44/0xa9
Modules linked in: ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack 
ebtable_nat ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 
ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_conntrack_ipv4 
nf_defrag_ipv4 nf_nat_ipv4 nf_nat iptable_mangle iptable_raw iptable_security 
nf_conntrack ip_set nfnetlink ebtable_filter ebtables ip6table_filter 
ip6_tables sunrpc vfat fat crct10dif_pclmul crc32_pclmul ghash_clmulni_intel 
intel_rapl_perf hv_balloon joydev xfs libcrc32c hv_storvsc serio_raw 
scsi_transport_fc hv_netvsc hyperv_keyboard hyperv_fb hid_hyperv crc32c_intel 
hv_vmbus

That's because now we may use hypercall for sending IPI, the
hypercall page will be reset very early upon kexec reboot, but kexec
will need to send IPI for stopping CPUs, and it will reference this
no longer usable page, then kernel panics.

To fix it, simply reset hv_hypercall_pg to NULL before the page is
reset to avoid any misuse, IPI sending will fallback to use non
hypercall based method. This only happens on kexec / kdump so setting to
NULL should be good enough.

Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments")
Signed-off-by: Kairui Song 

---

Update from V1:
- Add comment for the barrier.

 arch/x86/hyperv/hv_init.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 7abb09e2eeb8..34aa1e953dfc 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -406,6 +406,12 @@ void hyperv_cleanup(void)
/* Reset our OS id */
wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
 
+   /* Cleanup hypercall page reference before reset the page */
+   hv_hypercall_pg = NULL;
+
+   /* Make sure page reference is cleared before wrmsr */
+   wmb();
+
/* Reset the hypercall page */
hypercall_msr.as_uint64 = 0;
wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
-- 
2.20.1

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


Re: [RFC PATCH] x86, hyperv: fix kernel panic when kexec on HyperV VM

2019-03-05 Thread Peter Zijlstra
On Wed, Feb 27, 2019 at 10:55:46PM +0800, Kairui Song wrote:
> On Wed, Feb 27, 2019 at 8:02 PM Peter Zijlstra  wrote:
> >
> > On Tue, Feb 26, 2019 at 11:56:15PM +0800, Kairui Song wrote:
> > >  arch/x86/hyperv/hv_init.c | 4 
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> > > index 7abb09e2eeb8..92291c18d716 100644
> > > --- a/arch/x86/hyperv/hv_init.c
> > > +++ b/arch/x86/hyperv/hv_init.c
> > > @@ -406,6 +406,10 @@ void hyperv_cleanup(void)
> > >   /* Reset our OS id */
> > >   wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
> > >
> > > + /* Cleanup page reference before reset the page */
> > > + hv_hypercall_pg = NULL;
> > > + wmb();
> >
> > What do we need that SFENCE for? Any why does it lack a comment?
> 
> Hi, that's for ensuring the hv_hypercall_pg is reset to NULL before
> the following wrmsr call. The wrmsr call will make the pointer address
> invalid.

WRMSR is a serializing instruction (except for TSC_DEADLINE and the
X2APIC).

> I can add some comment in V2 if this is OK.

Barriers must always have a comment.

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


Re: [PATCH v2] x86, hyperv: fix kernel panic when kexec on HyperV

2019-03-05 Thread Peter Zijlstra
On Tue, Mar 05, 2019 at 08:17:03PM +0800, Kairui Song wrote:
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 7abb09e2eeb8..34aa1e953dfc 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -406,6 +406,12 @@ void hyperv_cleanup(void)
>   /* Reset our OS id */
>   wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
>  
> + /* Cleanup hypercall page reference before reset the page */
> + hv_hypercall_pg = NULL;
> +
> + /* Make sure page reference is cleared before wrmsr */

This comment forgets to tell us who cares about this. And why the wrmsr
itself isn't serializing enough.

> + wmb();
> +
>   /* Reset the hypercall page */
>   hypercall_msr.as_uint64 = 0;
>   wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);

That looks like a fake MSR; and you're telling me that VMEXIT doesn't
serialize?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [bug report] habanalabs: add virtual memory and MMU modules

2019-03-05 Thread Omer Shpigelman
From: Dan Carpenter 
Sent: Tuesday, 5 March 2019 8:40
> Hello Omer Shpigelman,
> 
> The patch 0feaf86d4e69: "habanalabs: add virtual memory and MMU
> modules" from Feb 16, 2019, leads to the following static checker
> warning:
> 
>   drivers/misc/habanalabs/memory.c:96 alloc_device_memory()
>   warn: integer overflows '(args->alloc.mem_size + (page_size - 1)) >>
> page_shift'
> 

You are correct, we'll send a fix shortly.
BTW which static checker did you use? we use Sparse and Smatch and they didn't 
catch that.

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


[PATCH] [v2] media: staging/intel-ipu3: reduce kernel stack usage

2019-03-05 Thread Arnd Bergmann
The imgu_css_queue structure is too large to be put on the kernel
stack, as we can see in 32-bit builds:

drivers/staging/media/ipu3/ipu3-css.c: In function 'imgu_css_fmt_try':
drivers/staging/media/ipu3/ipu3-css.c:1863:1: error: the frame size of 1172 
bytes is larger than 1024 bytes [-Werror=frame-larger-than=]

By dynamically allocating this array, the stack usage goes down to an
acceptable 140 bytes for the same x86-32 configuration.

Fixes: f5f2e4273518 ("media: staging/intel-ipu3: Add css pipeline programming")
Signed-off-by: Arnd Bergmann 
---
v2: restructure to use 'return -ENOMEM' instead of goto for failed
allocation.
---
 drivers/staging/media/ipu3/ipu3-css.c | 35 ++-
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/media/ipu3/ipu3-css.c 
b/drivers/staging/media/ipu3/ipu3-css.c
index 15ab77e4b766..e7f1898874fd 100644
--- a/drivers/staging/media/ipu3/ipu3-css.c
+++ b/drivers/staging/media/ipu3/ipu3-css.c
@@ -3,6 +3,7 @@
 
 #include 
 #include 
+#include 
 
 #include "ipu3-css.h"
 #include "ipu3-css-fw.h"
@@ -1744,15 +1745,18 @@ int imgu_css_fmt_try(struct imgu_css *css,
struct v4l2_rect *const bds = &r[IPU3_CSS_RECT_BDS];
struct v4l2_rect *const env = &r[IPU3_CSS_RECT_ENVELOPE];
struct v4l2_rect *const gdc = &r[IPU3_CSS_RECT_GDC];
-   struct imgu_css_queue q[IPU3_CSS_QUEUES];
-   struct v4l2_pix_format_mplane *const in =
-   &q[IPU3_CSS_QUEUE_IN].fmt.mpix;
-   struct v4l2_pix_format_mplane *const out =
-   &q[IPU3_CSS_QUEUE_OUT].fmt.mpix;
-   struct v4l2_pix_format_mplane *const vf =
-   &q[IPU3_CSS_QUEUE_VF].fmt.mpix;
+   struct imgu_css_queue *q;
+   struct v4l2_pix_format_mplane *in, *out, *vf;
int i, s, ret;
 
+   q = kcalloc(IPU3_CSS_QUEUES, sizeof(struct imgu_css_queue), GFP_KERNEL);
+   if (!q)
+   return -ENOMEM;
+
+   in  = &q[IPU3_CSS_QUEUE_IN].fmt.mpix;
+   out = &q[IPU3_CSS_QUEUE_OUT].fmt.mpix;
+   vf  = &q[IPU3_CSS_QUEUE_VF].fmt.mpix;
+
/* Adjust all formats, get statistics buffer sizes and formats */
for (i = 0; i < IPU3_CSS_QUEUES; i++) {
if (fmts[i])
@@ -1766,7 +1770,8 @@ int imgu_css_fmt_try(struct imgu_css *css,
IPU3_CSS_QUEUE_TO_FLAGS(i))) {
dev_notice(css->dev, "can not initialize queue %s\n",
   qnames[i]);
-   return -EINVAL;
+   ret = -EINVAL;
+   goto out;
}
}
for (i = 0; i < IPU3_CSS_RECTS; i++) {
@@ -1788,7 +1793,8 @@ int imgu_css_fmt_try(struct imgu_css *css,
if (!imgu_css_queue_enabled(&q[IPU3_CSS_QUEUE_IN]) ||
!imgu_css_queue_enabled(&q[IPU3_CSS_QUEUE_OUT])) {
dev_warn(css->dev, "required queues are disabled\n");
-   return -EINVAL;
+   ret = -EINVAL;
+   goto out;
}
 
if (!imgu_css_queue_enabled(&q[IPU3_CSS_QUEUE_OUT])) {
@@ -1829,7 +1835,8 @@ int imgu_css_fmt_try(struct imgu_css *css,
ret = imgu_css_find_binary(css, pipe, q, r);
if (ret < 0) {
dev_err(css->dev, "failed to find suitable binary\n");
-   return -EINVAL;
+   ret = -EINVAL;
+   goto out;
}
css->pipes[pipe].bindex = ret;
 
@@ -1843,7 +1850,8 @@ int imgu_css_fmt_try(struct imgu_css *css,
IPU3_CSS_QUEUE_TO_FLAGS(i))) {
dev_err(css->dev,
"final resolution adjustment failed\n");
-   return -EINVAL;
+   ret = -EINVAL;
+   goto out;
}
*fmts[i] = q[i].fmt.mpix;
}
@@ -1859,7 +1867,10 @@ int imgu_css_fmt_try(struct imgu_css *css,
 bds->width, bds->height, gdc->width, gdc->height,
 out->width, out->height, vf->width, vf->height);
 
-   return 0;
+   ret = 0;
+out:
+   kfree(q);
+   return ret;
 }
 
 int imgu_css_fmt_set(struct imgu_css *css,
-- 
2.20.0

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


[PATCH] [v2] media: staging: davinci_vpfe: disallow building with COMPILE_TEST

2019-03-05 Thread Arnd Bergmann
The driver should really call dm365_isif_setup_pinmux() through a callback,
but uses a hack to include a davinci specific machine header file when
compile testing instead. This works almost everywhere, but not on the
ARM omap1 platform, which has another header named mach/mux.h. This
causes a build failure:

drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:2: error: implicit 
declaration of function 'davinci_cfg_reg' 
[-Werror,-Wimplicit-function-declaration]
davinci_cfg_reg(DM365_VIN_CAM_WEN);
^
drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:2: error: this function 
declaration is not a prototype [-Werror,-Wstrict-prototypes]
drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:18: error: use of 
undeclared identifier 'DM365_VIN_CAM_WEN'
davinci_cfg_reg(DM365_VIN_CAM_WEN);
^
drivers/staging/media/davinci_vpfe/dm365_isif.c:2029:18: error: use of 
undeclared identifier 'DM365_VIN_CAM_VD'
davinci_cfg_reg(DM365_VIN_CAM_VD);
^
drivers/staging/media/davinci_vpfe/dm365_isif.c:2030:18: error: use of 
undeclared identifier 'DM365_VIN_CAM_HD'
davinci_cfg_reg(DM365_VIN_CAM_HD);
^
drivers/staging/media/davinci_vpfe/dm365_isif.c:2031:18: error: use of 
undeclared identifier 'DM365_VIN_YIN4_7_EN'
davinci_cfg_reg(DM365_VIN_YIN4_7_EN);
^
drivers/staging/media/davinci_vpfe/dm365_isif.c:2032:18: error: use of 
undeclared identifier 'DM365_VIN_YIN0_3_EN'
davinci_cfg_reg(DM365_VIN_YIN0_3_EN);
^
7 errors generated.

Exclude omap1 from compile-testing, under the assumption that all others
still work.

Fixes: 4907c73deefe ("media: staging: davinci_vpfe: allow building with 
COMPILE_TEST")
Signed-off-by: Arnd Bergmann 
---
 drivers/staging/media/davinci_vpfe/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/davinci_vpfe/Kconfig 
b/drivers/staging/media/davinci_vpfe/Kconfig
index aea449a8dbf8..76818cc48ddc 100644
--- a/drivers/staging/media/davinci_vpfe/Kconfig
+++ b/drivers/staging/media/davinci_vpfe/Kconfig
@@ -1,7 +1,7 @@
 config VIDEO_DM365_VPFE
tristate "DM365 VPFE Media Controller Capture Driver"
depends on VIDEO_V4L2
-   depends on (ARCH_DAVINCI_DM365 && !VIDEO_DM365_ISIF) || COMPILE_TEST
+   depends on (ARCH_DAVINCI_DM365 && !VIDEO_DM365_ISIF) || (COMPILE_TEST 
&& !ARCH_OMAP1)
depends on VIDEO_V4L2_SUBDEV_API
depends on VIDEO_DAVINCI_VPBE_DISPLAY
select VIDEOBUF2_DMA_CONTIG
-- 
2.20.0

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


Re: [RFC PATCH] x86, hyperv: fix kernel panic when kexec on HyperV VM

2019-03-05 Thread Kairui Song
On Tue, Mar 5, 2019 at 8:28 PM Peter Zijlstra  wrote:
>
> On Wed, Feb 27, 2019 at 10:55:46PM +0800, Kairui Song wrote:
> > On Wed, Feb 27, 2019 at 8:02 PM Peter Zijlstra  wrote:
> > >
> > > On Tue, Feb 26, 2019 at 11:56:15PM +0800, Kairui Song wrote:
> > > >  arch/x86/hyperv/hv_init.c | 4 
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> > > > index 7abb09e2eeb8..92291c18d716 100644
> > > > --- a/arch/x86/hyperv/hv_init.c
> > > > +++ b/arch/x86/hyperv/hv_init.c
> > > > @@ -406,6 +406,10 @@ void hyperv_cleanup(void)
> > > >   /* Reset our OS id */
> > > >   wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
> > > >
> > > > + /* Cleanup page reference before reset the page */
> > > > + hv_hypercall_pg = NULL;
> > > > + wmb();
> > >
> > > What do we need that SFENCE for? Any why does it lack a comment?
> >
> > Hi, that's for ensuring the hv_hypercall_pg is reset to NULL before
> > the following wrmsr call. The wrmsr call will make the pointer address
> > invalid.
>
> WRMSR is a serializing instruction (except for TSC_DEADLINE and the
> X2APIC).
>

Many thanks for the info, I'm not aware of the exception condition, V2
is sent, will drop the barrier in V3 then.

-- 
Best Regards,
Kairui Song
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2] x86, hyperv: fix kernel panic when kexec on HyperV

2019-03-05 Thread Kairui Song
On Tue, Mar 5, 2019 at 8:33 PM Peter Zijlstra  wrote:
>
> On Tue, Mar 05, 2019 at 08:17:03PM +0800, Kairui Song wrote:
> > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> > index 7abb09e2eeb8..34aa1e953dfc 100644
> > --- a/arch/x86/hyperv/hv_init.c
> > +++ b/arch/x86/hyperv/hv_init.c
> > @@ -406,6 +406,12 @@ void hyperv_cleanup(void)
> >   /* Reset our OS id */
> >   wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
> >
> > + /* Cleanup hypercall page reference before reset the page */
> > + hv_hypercall_pg = NULL;
> > +
> > + /* Make sure page reference is cleared before wrmsr */
>
> This comment forgets to tell us who cares about this. And why the wrmsr
> itself isn't serializing enough.
>
> > + wmb();
> > +
> >   /* Reset the hypercall page */
> >   hypercall_msr.as_uint64 = 0;
> >   wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
>
> That looks like a fake MSR; and you're telling me that VMEXIT doesn't
> serialize?

Thanks for the review, seem I being a bit paranoid on this. Will drop
it and send a v3 if no one has any other complaint.

--
Best Regards,
Kairui Song
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] [v2] media: staging/intel-ipu3: reduce kernel stack usage

2019-03-05 Thread Sakari Ailus
On Tue, Mar 05, 2019 at 02:26:29PM +0100, Arnd Bergmann wrote:
> The imgu_css_queue structure is too large to be put on the kernel
> stack, as we can see in 32-bit builds:
> 
> drivers/staging/media/ipu3/ipu3-css.c: In function 'imgu_css_fmt_try':
> drivers/staging/media/ipu3/ipu3-css.c:1863:1: error: the frame size of 1172 
> bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
> 
> By dynamically allocating this array, the stack usage goes down to an
> acceptable 140 bytes for the same x86-32 configuration.
> 
> Fixes: f5f2e4273518 ("media: staging/intel-ipu3: Add css pipeline 
> programming")
> Signed-off-by: Arnd Bergmann 
> ---
> v2: restructure to use 'return -ENOMEM' instead of goto for failed
> allocation.

Thanks, Arnd! All three applied.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2] x86, hyperv: fix kernel panic when kexec on HyperV

2019-03-05 Thread Peter Zijlstra
On Tue, Mar 05, 2019 at 09:34:13PM +0800, Kairui Song wrote:
> On Tue, Mar 5, 2019 at 8:33 PM Peter Zijlstra  wrote:
> >
> > On Tue, Mar 05, 2019 at 08:17:03PM +0800, Kairui Song wrote:
> > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> > > index 7abb09e2eeb8..34aa1e953dfc 100644
> > > --- a/arch/x86/hyperv/hv_init.c
> > > +++ b/arch/x86/hyperv/hv_init.c
> > > @@ -406,6 +406,12 @@ void hyperv_cleanup(void)
> > >   /* Reset our OS id */
> > >   wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
> > >
> > > + /* Cleanup hypercall page reference before reset the page */
> > > + hv_hypercall_pg = NULL;
> > > +
> > > + /* Make sure page reference is cleared before wrmsr */
> >
> > This comment forgets to tell us who cares about this. And why the wrmsr
> > itself isn't serializing enough.
> >
> > > + wmb();
> > > +
> > >   /* Reset the hypercall page */
> > >   hypercall_msr.as_uint64 = 0;
> > >   wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> >
> > That looks like a fake MSR; and you're telling me that VMEXIT doesn't
> > serialize?
> 
> Thanks for the review, seem I being a bit paranoid on this. Will drop
> it and send a v3 if no one has any other complaint.

Also; our wrmsr implementation has a "memory" clobber on, which means
you don't even need a compiler barrier to force emit that store before
the wrmsr.

You might want to retain the comment that explains this ordering though.

hv_hypercall_pg = NULL;
/*
 * text that explains why hv_hypercall_pg must be set NULL
 * before the wrmsr
 */
wrmsrl(...);

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


Re: [PATCH 0/3] pci-hyperv: fix memory leak and add pci_destroy_slot()

2019-03-05 Thread Stephen Hemminger
On Mon, 4 Mar 2019 21:34:47 +
Dexuan Cui  wrote:

> Patch #1 fixes a memory leak caused by incorrectly-maintained hpdev->refs.
> 
> Patch #2 and #3 make sure the "slot" is removed in all the scenarios.
> Without them, in the quick hot-add/hot-remove test, systemd-dev may easily
> crash when trying to access a dangling sys file in /sys/bus/pci/slots/:
> "BUG: unable to handle kernel paging request".
> 
> BTW, Patch #2 was posted on Feb 7, 2019, and this is the v2: the change
> to hv_eject_device_work() in v1 is removed, as the change is only needed
> when we hot-remove the device and remove the pci-hyperv driver at the 
> same time. It looks more work is required to make this scenaro work
> correctly, and since removing the driver is not really a "usual" usage,
> we can address this scenario in the future.
> 
> Please review the patchset.
> 
> Dexuan Cui (3):
>   PCI: hv: Fix a memory leak in hv_eject_device_work()
>   PCI: hv: Add hv_pci_remove_slots() when we unload the driver
>   PCI: hv: Add pci_destroy_slot() in pci_devices_present_work(), if
> necessary
> 
>  drivers/pci/controller/pci-hyperv.c | 23 +++
>  1 file changed, 23 insertions(+)


Thanks for fixing this.

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


Re: [PATCH] binder: fix race between munmap() and direct reclaim

2019-03-05 Thread Joel Fernandes
On Sat, Mar 02, 2019 at 08:27:44AM -0800, Todd Kjos wrote:
> On Fri, Mar 1, 2019 at 11:57 PM Greg KH  wrote:
> >
> > On Fri, Mar 01, 2019 at 03:06:06PM -0800, Todd Kjos wrote:
> > > An munmap() on a binder device causes binder_vma_close() to be called
> > > which clears the alloc->vma pointer.
> > >
> > > If direct reclaim causes binder_alloc_free_page() to be called, there
> > > is a race where alloc->vma is read into a local vma pointer and then
> > > used later after the mm->mmap_sem is acquired. This can result in
> > > calling zap_page_range() with an invalid vma which manifests as a
> > > use-after-free in zap_page_range().
> > >
> > > The fix is to check alloc->vma after acquiring the mmap_sem (which we
> > > were acquiring anyway) and skip zap_page_range() if it has changed
> > > to NULL.
> > >
> > > Signed-off-by: Todd Kjos 

Awesome patch, 
Reviewed-by: Joel Fernandes (Google) 

thanks!
 
 - Joel

> > > ---
> >
> > Any specific commit that this fixes?
> 
> No, it's been there a long time.
> 
> > And should it be marked for stable releases?
> 
> It is needed in stable (back to 4.4), but will need to be backported.
> Should I post backported versions targeting the specific releases now?
> I was thinking we'd wait for this one to land. I think we'll need 1
> patch for 4.4/4.9 and a second one for 4.14/4.19 (and some of those
> backported patches will have conflicts when merged down to android-4.X
> -- I think the 4.14/4.19 version will apply to all the android
> branches). Let me know how you want to handle this.
> 
> >
> > thanks,
> >
> > greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: comedi: ni_mio_common: Fix divide-by-zero for DIO cmdtest

2019-03-05 Thread Sasha Levin
Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: f164cbf98fa8 staging: comedi: ni_mio_common: add finite 
regeneration to dio output.

The bot has tested the following trees: v4.20.13, v4.19.26, v4.14.104, v4.9.161.

v4.20.13: Build OK!
v4.19.26: Failed to apply! Possible dependencies:
56d0b826d39f ("staging: comedi: ni_mio_common: implement new routing for 
TRIG_EXT")

v4.14.104: Failed to apply! Possible dependencies:
56d0b826d39f ("staging: comedi: ni_mio_common: implement new routing for 
TRIG_EXT")

v4.9.161: Failed to apply! Possible dependencies:
56d0b826d39f ("staging: comedi: ni_mio_common: implement new routing for 
TRIG_EXT")


How should we proceed with this patch?

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


[PATCH] hv: utils: fix coding style

2019-03-05 Thread Jesús Castro
Checkpatch script is showing a coding style error and is showing:

ERROR: else should follow close brace '}'
+   }
+   else if (hvt->mode == HVUTIL_TRANSPORT_NETLINK) {

This commit fixes the coding style, not a functional change.

Signed-off-by: Jesús Castro 
---
 drivers/hv/hv_utils_transport.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/hv/hv_utils_transport.c b/drivers/hv/hv_utils_transport.c
index 832777527936..eff5419b7b35 100644
--- a/drivers/hv/hv_utils_transport.c
+++ b/drivers/hv/hv_utils_transport.c
@@ -139,8 +139,7 @@ static int hvt_op_open(struct inode *inode, struct file 
*file)
 * device gets released.
 */
hvt->mode = HVUTIL_TRANSPORT_CHARDEV;
-   }
-   else if (hvt->mode == HVUTIL_TRANSPORT_NETLINK) {
+   } else if (hvt->mode == HVUTIL_TRANSPORT_NETLINK) {
/*
 * We're switching from netlink communication to using char
 * device. Issue the reset first.
-- 
2.21.0

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


[PATCH] staging/board/board.c: Fix compiler error of_find_all_nodes()

2019-03-05 Thread Arnold J Chand
Fix implicit-function-declaration error by 'extern'-ing the function in
the file

Signed-off-by: Arnold Chand 
---
 drivers/staging/board/board.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/board/board.c b/drivers/staging/board/board.c
index cb6feb34dd40..3025ee9f5517 100644
--- a/drivers/staging/board/board.c
+++ b/drivers/staging/board/board.c
@@ -22,6 +22,8 @@
 static struct device_node *irqc_node __initdata;
 static unsigned int irqc_base __initdata;
 
+extern struct device_node *of_find_all_nodes(struct device_node *prev);
+
 static bool find_by_address(u64 base_address)
 {
struct device_node *dn = of_find_all_nodes(NULL);
-- 
2.20.1


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


Re: [PATCH] hv: utils: fix coding style

2019-03-05 Thread Joe Perches
On Tue, 2019-03-05 at 17:45 -0600, Jesús Castro wrote:
> Checkpatch script is showing a coding style error and is showing:
> 
> ERROR: else should follow close brace '}'
> + }
> + else if (hvt->mode == HVUTIL_TRANSPORT_NETLINK) {
> 
> This commit fixes the coding style, not a functional change.
[]
> diff --git a/drivers/hv/hv_utils_transport.c b/drivers/hv/hv_utils_transport.c
[]
> @@ -139,8 +139,7 @@ static int hvt_op_open(struct inode *inode, struct file 
> *file)
>* device gets released.
>*/
>   hvt->mode = HVUTIL_TRANSPORT_CHARDEV;
> - }
> - else if (hvt->mode == HVUTIL_TRANSPORT_NETLINK) {
> + } else if (hvt->mode == HVUTIL_TRANSPORT_NETLINK) {
>   /*
>* We're switching from netlink communication to using char
>* device. Issue the reset first.

Rather than merely shutting up checkpatch warnings,
please try to improve the code for humans to read.

This block is probably better as a switch/case and
the separate bool issue_reset automatic seems
unnecessary as it's only used in one case.

Perhaps:
---
 drivers/hv/hv_utils_transport.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/hv/hv_utils_transport.c b/drivers/hv/hv_utils_transport.c
index 832777527936..6b27dd7be9bb 100644
--- a/drivers/hv/hv_utils_transport.c
+++ b/drivers/hv/hv_utils_transport.c
@@ -125,35 +125,35 @@ static int hvt_op_open(struct inode *inode, struct file 
*file)
 {
struct hvutil_transport *hvt;
int ret = 0;
-   bool issue_reset = false;
 
hvt = container_of(file->f_op, struct hvutil_transport, fops);
 
mutex_lock(&hvt->lock);
 
-   if (hvt->mode == HVUTIL_TRANSPORT_DESTROY) {
-   ret = -EBADF;
-   } else if (hvt->mode == HVUTIL_TRANSPORT_INIT) {
+   switch (hvt->mode) {
+   case HVUTIL_TRANSPORT_INIT:
/*
 * Switching to CHARDEV mode. We switch bach to INIT when
 * device gets released.
 */
hvt->mode = HVUTIL_TRANSPORT_CHARDEV;
-   }
-   else if (hvt->mode == HVUTIL_TRANSPORT_NETLINK) {
+   break;
+   case HVUTIL_TRANSPORT_NETLINK:
/*
 * We're switching from netlink communication to using char
 * device. Issue the reset first.
 */
-   issue_reset = true;
hvt->mode = HVUTIL_TRANSPORT_CHARDEV;
-   } else {
+   hvt_reset(hvt);
+   break;
+   case HVUTIL_TRANSPORT_DESTROY:
+   ret = -EBADF;
+   break;
+   default:
ret = -EBUSY;
+   break;
}
 
-   if (issue_reset)
-   hvt_reset(hvt);
-
mutex_unlock(&hvt->lock);
 
return ret;


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


Re: [PATCH] staging/board/board.c: Fix compiler error of_find_all_nodes()

2019-03-05 Thread Dan Carpenter
Change the subsystem prefix to: [PATCH] Staging: board: ...

On Tue, Mar 05, 2019 at 11:39:30PM +, Arnold J Chand wrote:
> Fix implicit-function-declaration error by 'extern'-ing the function in
> the file
> 
> Signed-off-by: Arnold Chand 
> ---
>  drivers/staging/board/board.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/board/board.c b/drivers/staging/board/board.c
> index cb6feb34dd40..3025ee9f5517 100644
> --- a/drivers/staging/board/board.c
> +++ b/drivers/staging/board/board.c
> @@ -22,6 +22,8 @@
>  static struct device_node *irqc_node __initdata;
>  static unsigned int irqc_base __initdata;
>  
> +extern struct device_node *of_find_all_nodes(struct device_node *prev);
> +

We already include  so what's going on here?  The driver
compiles for me.  Anyway, fix the includes, don't do this.

regards,
dan carpenter

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