Re: [PATCH 001] staging: wlan-ng: Add tabstop preceding the statement

[Joe Perches]

On Mon, 2017-01-30 at 17:44 +0100, Greg Kroah-Hartman wrote:
> On Mon, Jan 30, 2017 at 11:31:42AM -0500, Maksymilian Piechota wrote:
> > This patch fixes the checkpatch.pl warning:
> > 
> > WARNING: Statements should start on a tabstop
> > 
> > Signed-off-by: Maksymilian Piechota 
> > ---
> >  drivers/staging/wlan-ng/prism2mgmt.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/wlan-ng/prism2mgmt.c 
> > b/drivers/staging/wlan-ng/prism2mgmt.c
> > index 16fb2d3..2d67125 100644
> > --- a/drivers/staging/wlan-ng/prism2mgmt.c
> > +++ b/drivers/staging/wlan-ng/prism2mgmt.c
> > @@ -1308,7 +1308,7 @@ int prism2mgmt_wlansniff(struct wlandevice *wlandev, 
> > void *msgp)
> > hw->sniffhdr = 0;
> > wlandev->netdev->type = ARPHRD_IEEE80211_PRISM;
> > } else
> > -   if ((msg->wlanheader.status ==
> > +   if ((msg->wlanheader.status ==
> >  P80211ENUM_msgitem_status_data_ok)
> > && (msg->wlanheader.data == P80211ENUM_truth_true)) {
> > hw->sniffhdr = 1;
> 
> Hm, this all doesn't look correct now, does it?  Please fix up the whole
> if statement here.

Ideally, it'd look something like:
  
/* Set the driver state */
/* Do we want the prism2 header? */
if (msg->prismheader.status == 
P80211ENUM_msgitem_status_data_ok &&
msg->prismheader.data == P80211ENUM_truth_true) {
hw->sniffhdr = 0;
wlandev->netdev->type = ARPHRD_IEEE80211_PRISM;
} else if (msg->wlanheader.status == 
P80211ENUM_msgitem_status_data_ok &&
   msg->wlanheader.data == P80211ENUM_truth_true) {
hw->sniffhdr = 1;
wlandev->netdev->type = ARPHRD_IEEE80211_PRISM;
} else {
wlandev->netdev->type = ARPHRD_IEEE80211;
}

with the unnecessary parentheses removed,
the logical continuations at the end-of-line,
and the else if on a single line.

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

[PATCH v3] staging: wlan-ng: add missing byte order conversion

[Igor Pylypiv]

Conversion macros le16_to_cpu was removed and that caused new sparse warning

sparse output:
drivers/staging/wlan-ng/p80211netdev.c:241:44: warning: incorrect type in 
argument 2 (different base types)
drivers/staging/wlan-ng/p80211netdev.c:241:44:expected unsigned short 
[unsigned] [usertype] fc
drivers/staging/wlan-ng/p80211netdev.c:241:44:got restricted __le16 
[usertype] fc

Fixes: 7ad82572348c ("staging:wlan-ng:Fix sparse warning")
Signed-off-by: Igor Pylypiv 
---
 drivers/staging/wlan-ng/p80211netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wlan-ng/p80211netdev.c 
b/drivers/staging/wlan-ng/p80211netdev.c
index 53dbbd6..021fb23 100644
--- a/drivers/staging/wlan-ng/p80211netdev.c
+++ b/drivers/staging/wlan-ng/p80211netdev.c
@@ -237,7 +237,7 @@ static int p80211_convert_to_ether(struct wlandevice 
*wlandev,
struct p80211_hdr_a3 *hdr;

hdr = (struct p80211_hdr_a3 *)skb->data;
-   if (p80211_rx_typedrop(wlandev, hdr->fc))
+   if (p80211_rx_typedrop(wlandev, le16_to_cpu(hdr->fc)))
return CONV_TO_ETHER_SKIPPED;

/* perform mcast filtering: allow my local address through but reject
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] Fixes: 7ad82572348c ("staging:wlan-ng:Fix sparse warning")

[Greg Kroah-Hartman]

On Mon, Jan 30, 2017 at 08:07:25PM -0800, Igor Pylypiv wrote:
> Conversion macros le16_to_cpu was removed and that caused new sparse warning
> 
> sparse output:
> drivers/staging/wlan-ng/p80211netdev.c:241:44: warning: incorrect type in 
> argument 2 (different base types)
> drivers/staging/wlan-ng/p80211netdev.c:241:44:expected unsigned short 
> [unsigned] [usertype] fc
> drivers/staging/wlan-ng/p80211netdev.c:241:44:got restricted __le16 
> [usertype] fc
> 
> Signed-off-by: Igor Pylypiv 
> ---
>  drivers/staging/wlan-ng/p80211netdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

"Fixes:" goes down here in the area where Signed-off-by: is, not in the
subject line.  Please fix up the subject to be "correct" and match up
the way that other commits for this driver looks like and resend.

thanks,

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

RE: [PATCH] [linux-next]staging: unisys: visornic: Fix typo in visornic_main.c

[Kershner, David A]

> -Original Message-
> From: Masanari Iida [mailto:standby2...@gmail.com]
> Sent: Sunday, January 29, 2017 9:33 PM
> To: linux-ker...@vger.kernel.org; gre...@linuxfoundation.org; Kershner,
> David A ; de...@driverdev.osuosl.org
> Cc: Masanari Iida 
> Subject: [PATCH] [linux-next]staging: unisys: visornic: Fix typo in
> visornic_main.c
> 
> This patch fix some spelling typos found in visornic_main.c
> 

Acked-by: David Kershner  

> Signed-off-by: Masanari Iida 
> ---
>  drivers/staging/unisys/visornic/visornic_main.c | 22 +++---
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visornic/visornic_main.c
> b/drivers/staging/unisys/visornic/visornic_main.c
> index 0a8f36125f5b..c44c430b966f 100644
> --- a/drivers/staging/unisys/visornic/visornic_main.c
> +++ b/drivers/staging/unisys/visornic/visornic_main.c
> @@ -423,7 +423,7 @@ send_enbdis(struct net_device *netdev, int state,
> 
>  /**
>   *   visornic_disable_with_timeout - Disable network adapter
> - *   @netdev: netdevice to disale
> + *   @netdev: netdevice to disable
>   *   @timeout: timeout to wait for disable
>   *
>   *   Disable the network adapter and inform the IO Partition that we
> @@ -532,7 +532,7 @@ init_rcv_bufs(struct net_device *netdev, struct
> visornic_devdata *devdata)
>   return -ENOMEM;
>   count = i;
> 
> - /* Ensure we can alloc 2/3rd of the requeested number of buffers.
> + /* Ensure we can alloc 2/3rd of the requested number of buffers.
>* 2/3 is an arbitrary choice; used also in ndis init.c
>*/
>   if (count < ((2 * devdata->num_rcv_bufs) / 3)) {
> @@ -561,7 +561,7 @@ init_rcv_bufs(struct net_device *netdev, struct
> visornic_devdata *devdata)
>   *
>   *   Sends enable to IOVM, inits, and posts receive buffers to IOVM
>   *   timeout is defined in msecs (timeout of 0 specifies infinite wait)
> - *   Return 0 for success, negavite for failure.
> + *   Return 0 for success, negative for failure.
>   */
>  static int
>  visornic_enable_with_timeout(struct net_device *netdev, const int
> timeout)
> @@ -750,7 +750,7 @@ static inline bool vnic_hit_low_watermark(struct
> visornic_devdata *devdata,
>   *   @skb: Packet to be sent
>   *   @netdev: net device the packet is being sent from
>   *
> - *   Convert the skb to a cmdrsp so the IO Partition can undersand it.
> + *   Convert the skb to a cmdrsp so the IO Partition can understand it.
>   *   Send the XMIT command to the IO Partition for processing. This
>   *   function is protected from concurrent calls by a spinlock xmit_lock
>   *   in the net_device struct, but as soon as the function returns it
> @@ -1097,7 +1097,7 @@ repost_return(struct uiscmdrsp *cmdrsp, struct
> visornic_devdata *devdata,
>   *
>   *   Got a receive packet back from the IO Part, handle it and send
>   *   it up the stack.
> - *   Returns 1 iff an skb was receieved, otherwise 0
> + *   Returns 1 iff an skb was received, otherwise 0
>   */
>  static int
>  visornic_rx(struct uiscmdrsp *cmdrsp)
> @@ -1227,7 +1227,7 @@ visornic_rx(struct uiscmdrsp *cmdrsp)
>   }
>   }
> 
> - /* set up packet's protocl type using ethernet header - this
> + /* set up packet's protocol type using ethernet header - this
>* sets up skb->pkt_type & it also PULLS out the eth header
>*/
>   skb->protocol = eth_type_trans(skb, netdev);
> @@ -1549,7 +1549,7 @@ drain_resp_queue(struct uiscmdrsp *cmdrsp,
> struct visornic_devdata *devdata)
>   *   @cmdrsp: io channel command response message
>   *   @devdata: visornic device to drain
>   *
> - *   Drain the respones queue of any responses from the IO partition.
> + *   Drain the response queue of any responses from the IO partition.
>   *   Process the responses as we get them.
>   *   Returns when response queue is empty or when the thread stops.
>   */
> @@ -1665,7 +1665,7 @@ static int visornic_poll(struct napi_struct *napi, int
> budget)
>   *   poll_for_irq- Checks the status of the response queue.
>   *   @v: void pointer to the visronic devdata
>   *
> - *   Main function of the vnic_incoming thread. Peridocially check the
> + *   Main function of the vnic_incoming thread. Periodically check the
>   *   response queue and drain it if needed.
>   *   Returns when thread has stopped.
>   */
> @@ -1711,7 +1711,7 @@ static int visornic_probe(struct visor_device *dev)
>   netdev->watchdog_timeo = 5 * HZ;
>   SET_NETDEV_DEV(netdev, >device);
> 
> - /* Get MAC adddress from channel and read it into the device. */
> + /* Get MAC address from channel and read it into the device. */
>   netdev->addr_len = ETH_ALEN;
>   channel_offset = offsetof(struct spar_io_channel_protocol,
> vnic.macaddr);
> @@ -1835,7 +1835,7 @@ static int visornic_probe(struct visor_device *dev)
>   /* 

[PATCH v2] Fixes: 7ad82572348c ("staging:wlan-ng:Fix sparse warning")

[Igor Pylypiv]

Conversion macros le16_to_cpu was removed and that caused new sparse warning

sparse output:
drivers/staging/wlan-ng/p80211netdev.c:241:44: warning: incorrect type in 
argument 2 (different base types)
drivers/staging/wlan-ng/p80211netdev.c:241:44:expected unsigned short 
[unsigned] [usertype] fc
drivers/staging/wlan-ng/p80211netdev.c:241:44:got restricted __le16 
[usertype] fc

Signed-off-by: Igor Pylypiv 
---
 drivers/staging/wlan-ng/p80211netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wlan-ng/p80211netdev.c 
b/drivers/staging/wlan-ng/p80211netdev.c
index 53dbbd6..021fb23 100644
--- a/drivers/staging/wlan-ng/p80211netdev.c
+++ b/drivers/staging/wlan-ng/p80211netdev.c
@@ -237,7 +237,7 @@ static int p80211_convert_to_ether(struct wlandevice 
*wlandev,
struct p80211_hdr_a3 *hdr;

hdr = (struct p80211_hdr_a3 *)skb->data;
-   if (p80211_rx_typedrop(wlandev, hdr->fc))
+   if (p80211_rx_typedrop(wlandev, le16_to_cpu(hdr->fc)))
return CONV_TO_ETHER_SKIPPED;

/* perform mcast filtering: allow my local address through but reject
--
2.7.4

Fixed commit message according to Documentation/process/submitting-patches.rst 
(section 2)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 22/24] media: imx: Add MIPI CSI-2 OV5640 sensor subdev driver

[Steve Longerbeam]

On 01/30/2017 03:29 PM, Russell King - ARM Linux wrote:

On Fri, Jan 06, 2017 at 06:11:40PM -0800, Steve Longerbeam wrote:

+config IMX_OV5640_MIPI
+   tristate "OmniVision OV5640 MIPI CSI-2 camera support"
+   depends on GPIOLIB && VIDEO_IMX_CAMERA
+   select IMX_MIPI_CSI2
+   default y

Why is this defaulting to y?  New drivers should not default to enabled
unless they are replacing some already pre-existing functionality.
Ditto for the other camera driver.


The ov564x sensors are required for the SabreSD and SabreLite/Nitrogen
reference boards (if VIDEO_IMX_CAMERA is enabled that is). But they're not
required for other platforms, so you're right I shouldn't have defaulted 
these

to yes. Fixed.


Steve

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

Re: [PATCH 60/60] staging: lustre: libcfs: fix minimum size check for libcfs ioctl

[James Simmons]

> It looks like what happened is there were two patches applied out of
> sync.  Let's add a fixes tag and CC the original author.

So the only problem here is the commit message. I will update it then.
 
> Fixes: ed2f549dc0f6 ("staging: lustre: libcfs: test if userland data is to 
> small")
> 
> This patch was probably correct when it was written but commit
> 1290932728e5 ("staging: lustre: Dynamic LNet Configuration (DLC) IOCTL
> changes") ended up getting applied first so the size was wrong.
> 
> The lstcon_ioctl_entry() function doesn't have enough size checking.

This sounds like a separate patch. I will open a ticket about this and
your comments below.

> Also I'm uncomfortable with:
> 
>   data = container_of(hdr, struct libcfs_ioctl_data, ioc_hdr);
> 
> If hdr isn't the first member of the struct then the code is broken but
> container_of() implies that that isn't a hard requirement.  It should
> just be:
> 
>   data = (struct libcfs_ioctl_data *)hdr;

Don't know if hdr being first is a hard requirment. Doug, Amir do you know 
if it is an requirement? 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 6/6] staging: bcm2835-v4l2: Apply spelling fixes from checkpatch.

[Joe Perches]

On Mon, 2017-01-30 at 12:05 -0800, Eric Anholt wrote:
> Joe Perches  writes:
> 
> > On Fri, 2017-01-27 at 13:55 -0800, Eric Anholt wrote:
> > > Generated with checkpatch.pl --fix-inplace and git add -p out of the
> > > results.
> > 
> > Maybe another.
> > 
> > > diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c 
> > > b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
> > 
> > []
> > > @@ -239,7 +239,7 @@ static int bulk_receive(struct vchiq_mmal_instance 
> > > *instance,
> > >   pr_err("buffer list empty trying to submit bulk receive\n");
> > >  
> > >   /* todo: this is a serious error, we should never have
> > > -  * commited a buffer_to_host operation to the mmal
> > > +  * committed a buffer_to_host operation to the mmal
> > >* port without the buffer to back it up (underflow
> > >* handling) and there is no obvious way to deal with
> > >* this - how is the mmal servie going to react when
> > 
> > Perhaps s/servie/service/ ?
> 
> I was trying to restrict this patch to just the fixes from checkpatch.

That's the wrong thing to do if you're fixing
spelling defects.  checkpatch is just one mechanism
to identify some, and definitely not all, typos and
spelling defects.

If you fixing, fix.  Don't just rely on the brainless
tools, use your decidedly non-mechanical brain.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Steve Longerbeam]

On 01/30/2017 04:31 PM, Russell King - ARM Linux wrote:

On Fri, Jan 06, 2017 at 06:11:39PM -0800, Steve Longerbeam wrote:

+++ b/drivers/staging/media/imx/imx-mipi-csi2.c

...

+#define DEVICE_NAME "imx6-mipi-csi2"

Why is the device/driver named imx6-mipi-csi2, but the module named
imx-mipi-csi2 - could there be some consistency here please?


right, that was missed after renaming the device node. Fixed.

Steve

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

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

On 01/30/2017 05:06 PM, Russell King - ARM Linux wrote:

On Tue, Jan 31, 2017 at 12:45:11AM +, Russell King - ARM Linux wrote:

Trying this driver with an imx219 camera (which works with Philipp's
driver) results in not much happening... no /dev/media* node for it,
no subdevs, no nothing.  No clues as to what's missing either.  Only
messages from imx-media are from registering the various subdevs.

Another issue:

imx_csi 5491  4
imx_camif  11654  4
imx_ic 23961  8
imx_smfc6639  4
imx_media  23308  1 imx_csi
imx_mipi_csi2   5544  1
imx_media_common   12701  6 
imx_csi,imx_smfc,imx_media,imx_mipi_csi2,imx_camif,imx_ic
imx219 21205  2

So how does one remove any of these modules, say, while developing a
camera driver?  Having to reboot to test an update makes it painfully
slow for testing.


Unload is not working yet, it's on the TODO list.

But FWIW, here's how it currently looks in version 4
(on the SabreSD):

imx_media_csi   9663  4
imx_media_ic   12688  6
imx_media_capture  10201  2 imx_media_ic,imx_media_csi
imx_media_vdic  6909  2
imx_mipi_csi2   6293  1
ov5640_mipi25988  1
imx_media  15532  0
imx_media_common   16093  6 
imx_media_ic,imx_media,imx_media_csi,imx_mipi_cs

i2,imx_media_capture,imx_media_vdic


Steve




Philipp's driver can do this (once the unload bugs are fixed, which I
have patches for).



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

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

On 01/30/2017 04:45 PM, Russell King - ARM Linux wrote:


Hi,

Trying this driver with an imx219 camera (which works with Philipp's
driver) results in not much happening... no /dev/media* node for it,
no subdevs, no nothing.  No clues as to what's missing either.  Only
messages from imx-media are from registering the various subdevs.

[   37.444877] imx-media: Registered subdev imx6-mipi-csi2
[   37.444973] imx-media: Registered subdev imx219 0-0010
[   38.868740] imx-media: Registered subdev ipu1_ic_prpenc
[   38.869265] imx-media: Registered subdev ipu1_ic_prpvf
[   38.869425] imx-media: Registered subdev ipu1_ic_pp0
[   38.870086] imx-media: Registered subdev ipu1_ic_pp1
[   38.871510] imx-media: Registered subdev ipu2_ic_prpenc
[   38.871743] imx-media: Registered subdev ipu1_smfc0
[   38.873043] imx-media: Registered subdev ipu1_smfc1
[   38.873225] imx-media: Registered subdev ipu2_ic_prpvf
[   38.875027] imx-media: Registered subdev ipu2_smfc0
[   38.875320] imx-media: Registered subdev ipu2_ic_pp0
[   38.877148] imx-media: Registered subdev ipu2_smfc1
[   38.877436] imx-media: Registered subdev ipu2_ic_pp1
[   38.932089] imx-media: Registered subdev camif0
[   38.956538] imx-media: Registered subdev camif1
[   38.959148] imx-media: Registered subdev camif2
[   38.964353] imx-media: Registered subdev camif3
[  206.502077] imx-media: Registered subdev ipu1_csi0
[  206.503304] imx-media: Registered subdev ipu1_csi1
[  206.503814] imx-media: Registered subdev ipu2_csi0
[  206.504281] imx-media: Registered subdev ipu2_csi1

I also get:

[   37.200072] imx6-mipi-csi2: data lanes: 2
[   37.200077] imx6-mipi-csi2: flags: 0x0200

and from what I can see, all modules from drivers/staging/media/imx/ are
loaded (had to load imx-csi by hand because of the brokenness in the
drivers/gpu/ipu code attaching an device_node pointer after registering
the platform device, which changes what userspace sees in the modalias
file.)

Any clues at what to look at?


Hi Russell,

I'm not familiar with IMX219, can you send me the source for the
imx219 subdev? I don't see it in 4.10-rc1.

I'm also having trouble finding a datasheet for it, but from what
I've read, it has a MIPI CSI-2 interface. It should work fine as long
as it presents a single source pad, registers asynchronously, and
sets its entity function to MEDIA_ENT_F_CAM_SENSOR.

Since I see it was registered asynchronously from the above, it
must have been added to the device tree. But given that there
is no /dev/media? node, the media driver is probably waiting for
another subdev to register, I don't know what that would be.

Can you send me the full patch on top of the v3 driver and I'll
try to find what's missing.

Edit: I see a subdev that is missing: the video mux. Did you enable
CONFIG_VIDEO_MULTIPLEXER?

Finally, what platform does this IMX219 sensor module plug into?


Steve

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

Re: [PATCH v3 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Tue, Jan 31, 2017 at 12:45:11AM +, Russell King - ARM Linux wrote:
> Trying this driver with an imx219 camera (which works with Philipp's
> driver) results in not much happening... no /dev/media* node for it,
> no subdevs, no nothing.  No clues as to what's missing either.  Only
> messages from imx-media are from registering the various subdevs.

Another issue:

imx_csi 5491  4
imx_camif  11654  4
imx_ic 23961  8
imx_smfc6639  4
imx_media  23308  1 imx_csi
imx_mipi_csi2   5544  1
imx_media_common   12701  6 
imx_csi,imx_smfc,imx_media,imx_mipi_csi2,imx_camif,imx_ic
imx219 21205  2

So how does one remove any of these modules, say, while developing a
camera driver?  Having to reboot to test an update makes it painfully
slow for testing.

Philipp's driver can do this (once the unload bugs are fixed, which I
have patches for).

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 13/60] staging: lustre: obdclass: health_check to report unhealthy upon LBUG

[James Simmons]

> Wat?
> 
> I'm sorry but this patch makes no sense at all.
> 
> On Sat, Jan 28, 2017 at 07:04:41PM -0500, James Simmons wrote:
> > From: Bruno Faccini 
> > 
> > When a LBUG has occurred, without panic_on_lbug being set,
> > health_check sysfs file must return an unhealthy state.
> 
> Why?

Its a patch being applied out of order issue. Originally this
patch was written before this was moved to sysfs and the original
code didn't return right after printing "LBUG". The move to
sysfs changed this behavior. I will fix up the out of tree code
in this case.
 
> > 
> > Signed-off-by: Bruno Faccini 
> > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-7486
> > Reviewed-on: http://review.whamcloud.com/17981
> > Reviewed-by: Bobi Jam 
> > Reviewed-by: Niu Yawei 
> > Reviewed-by: James Simmons 
> > Reviewed-by: Oleg Drokin 
> > Signed-off-by: James Simmons 
> > ---
> >  drivers/staging/lustre/lustre/obdclass/linux/linux-module.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c 
> > b/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
> > index 22e6d1f..ef25db6 100644
> > --- a/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
> > +++ b/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
> > @@ -224,8 +224,10 @@ static ssize_t pinger_show(struct kobject *kobj, 
> > struct attribute *attr,
> > int i;
> > size_t len = 0;
> >  
> > -   if (libcfs_catastrophe)
> > -   return sprintf(buf, "LBUG\n");
> > +   if (libcfs_catastrophe) {
> > +   len = sprintf(buf, "LBUG\n");
> 
> This line is dead code, now.
> 
> > +   healthy = false;
> > +   }
> >  
> 
> regards,
> dan carpenter
> 
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 60/60] staging: lustre: libcfs: fix minimum size check for libcfs ioctl

[James Simmons]

> On Mon, Jan 30, 2017 at 01:51:56PM +0300, Dan Carpenter wrote:
> > The lstcon_ioctl_entry() function doesn't have enough size checking.
> 
> Actually, the lstcon_ioctl_entry() would have been fine before we apply
> this [patch 60/60]...  As near as I can tell, no in kernel code is
> negatively affected by the bug this patch fixes.

There is one, the ioctl IOC_LIBCFS_GET_LNET_STATS was affected by this
bug. That is how it was founded.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:18PM -0800, Steve Longerbeam wrote:
> Philipp Zabel (3):
>   ARM: dts: imx6qdl: Add mipi_ipu1/2 multiplexers, mipi_csi, and their
> connections
>   add mux and video interface bridge entity functions
>   platform: add video-multiplexer subdevice driver
> 
> Steve Longerbeam (21):
>   [media] dt-bindings: Add bindings for i.MX media driver
>   ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node
>   ARM: dts: imx6qdl: add media device
>   ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround
>   ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors
>   ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors
>   ARM: dts: imx6-sabreauto: create i2cmux for i2c3
>   ARM: dts: imx6-sabreauto: add reset-gpios property for max7310_b
>   ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture
>   ARM: dts: imx6-sabreauto: add the ADV7180 video decoder
>   UAPI: Add media UAPI Kbuild file
>   media: Add userspace header file for i.MX
>   media: Add i.MX media core driver
>   media: imx: Add CSI subdev driver
>   media: imx: Add SMFC subdev driver
>   media: imx: Add IC subdev drivers
>   media: imx: Add Camera Interface subdev driver
>   media: imx: Add MIPI CSI-2 Receiver subdev driver
>   media: imx: Add MIPI CSI-2 OV5640 sensor subdev driver
>   media: imx: Add Parallel OV5642 sensor subdev driver
>   ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers

Hi,

Trying this driver with an imx219 camera (which works with Philipp's
driver) results in not much happening... no /dev/media* node for it,
no subdevs, no nothing.  No clues as to what's missing either.  Only
messages from imx-media are from registering the various subdevs.

[   37.444877] imx-media: Registered subdev imx6-mipi-csi2
[   37.444973] imx-media: Registered subdev imx219 0-0010
[   38.868740] imx-media: Registered subdev ipu1_ic_prpenc
[   38.869265] imx-media: Registered subdev ipu1_ic_prpvf
[   38.869425] imx-media: Registered subdev ipu1_ic_pp0
[   38.870086] imx-media: Registered subdev ipu1_ic_pp1
[   38.871510] imx-media: Registered subdev ipu2_ic_prpenc
[   38.871743] imx-media: Registered subdev ipu1_smfc0
[   38.873043] imx-media: Registered subdev ipu1_smfc1
[   38.873225] imx-media: Registered subdev ipu2_ic_prpvf
[   38.875027] imx-media: Registered subdev ipu2_smfc0
[   38.875320] imx-media: Registered subdev ipu2_ic_pp0
[   38.877148] imx-media: Registered subdev ipu2_smfc1
[   38.877436] imx-media: Registered subdev ipu2_ic_pp1
[   38.932089] imx-media: Registered subdev camif0
[   38.956538] imx-media: Registered subdev camif1
[   38.959148] imx-media: Registered subdev camif2
[   38.964353] imx-media: Registered subdev camif3
[  206.502077] imx-media: Registered subdev ipu1_csi0
[  206.503304] imx-media: Registered subdev ipu1_csi1
[  206.503814] imx-media: Registered subdev ipu2_csi0
[  206.504281] imx-media: Registered subdev ipu2_csi1

I also get:

[   37.200072] imx6-mipi-csi2: data lanes: 2
[   37.200077] imx6-mipi-csi2: flags: 0x0200

and from what I can see, all modules from drivers/staging/media/imx/ are
loaded (had to load imx-csi by hand because of the brokenness in the
drivers/gpu/ipu code attaching an device_node pointer after registering
the platform device, which changes what userspace sees in the modalias
file.)

Any clues at what to look at?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:39PM -0800, Steve Longerbeam wrote:
> +++ b/drivers/staging/media/imx/imx-mipi-csi2.c
...
> +#define DEVICE_NAME "imx6-mipi-csi2"

Why is the device/driver named imx6-mipi-csi2, but the module named
imx-mipi-csi2 - could there be some consistency here please?

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:39PM -0800, Steve Longerbeam wrote:
> +static void imxcsi2_enable(struct imxcsi2_dev *csi2, bool enable)
> +{
> + if (enable) {
> + imxcsi2_write(csi2, 0x, CSI2_PHY_SHUTDOWNZ);
> + imxcsi2_write(csi2, 0x, CSI2_DPHY_RSTZ);
> + imxcsi2_write(csi2, 0x, CSI2_RESETN);
> + } else {
> + imxcsi2_write(csi2, 0x0, CSI2_PHY_SHUTDOWNZ);
> + imxcsi2_write(csi2, 0x0, CSI2_DPHY_RSTZ);
> + imxcsi2_write(csi2, 0x0, CSI2_RESETN);
> + }
> +}
> +
> +static void imxcsi2_reset(struct imxcsi2_dev *csi2)
> +{
> + imxcsi2_enable(csi2, false);
> +
> + imxcsi2_write(csi2, 0x0001, CSI2_PHY_TST_CTRL0);
> + imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL1);
> + imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL0);
> + imxcsi2_write(csi2, 0x0002, CSI2_PHY_TST_CTRL0);
> + imxcsi2_write(csi2, 0x00010044, CSI2_PHY_TST_CTRL1);
> + imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL0);
> + imxcsi2_write(csi2, 0x0014, CSI2_PHY_TST_CTRL1);
> + imxcsi2_write(csi2, 0x0002, CSI2_PHY_TST_CTRL0);
> + imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL0);
> +
> + imxcsi2_enable(csi2, true);
> +}
> +
> +static int imxcsi2_dphy_wait(struct imxcsi2_dev *csi2)
> +{
> + u32 reg;
> + int i;
> +
> + /* wait for mipi sensor ready */
> + for (i = 0; i < 50; i++) {
> + reg = imxcsi2_read(csi2, CSI2_PHY_STATE);
> + if (reg != 0x200)
> + break;
> + usleep_range(1, 2);
> + }
> +
> + if (i >= 50) {
> + v4l2_err(>sd,
> +  "wait for clock lane timeout, phy_state = 0x%08x\n",
> +  reg);
> + return -ETIME;
> + }
> +
> + /* wait for mipi stable */
> + for (i = 0; i < 50; i++) {
> + reg = imxcsi2_read(csi2, CSI2_ERR1);
> + if (reg == 0x0)
> + break;
> + usleep_range(1, 2);
> + }
> +
> + if (i >= 50) {
> + v4l2_err(>sd,
> +  "wait for controller timeout, err1 = 0x%08x\n",
> +  reg);
> + return -ETIME;
> + }
> +
> + /* finally let's wait for active clock on the clock lane */
> + for (i = 0; i < 50; i++) {
> + reg = imxcsi2_read(csi2, CSI2_PHY_STATE);
> + if (reg & (1 << 8))
> + break;
> + usleep_range(1, 2);
> + }
> +
> + if (i >= 50) {
> + v4l2_err(>sd,
> +  "wait for active clock timeout, phy_state = 0x%08x\n",
> +  reg);
> + return -ETIME;
> + }
> +
> + v4l2_info(>sd, "ready, dphy version 0x%x\n",
> +   imxcsi2_read(csi2, CSI2_VERSION));
> +
> + return 0;
> +}
...
> +static int imxcsi2_s_power(struct v4l2_subdev *sd, int on)
> +{
> + struct imxcsi2_dev *csi2 = sd_to_dev(sd);
> +
> + if (on && !csi2->on) {
> + v4l2_info(>sd, "power ON\n");
> + clk_prepare_enable(csi2->cfg_clk);
> + clk_prepare_enable(csi2->dphy_clk);
> + imxcsi2_set_lanes(csi2);
> + imxcsi2_reset(csi2);

The iMX6 manuals call for a very specific seven sequence of initialisation
for CSI2, which begins with:

1. reset the D-PHY.
2. place MIPI sensor in LP-11 state
3. perform D-PHY initialisation
4. configure CSI2 lanes and de-assert resets and shutdown signals

Since you reset the CSI2 at power up and then release it, how do you
guarantee that the published sequence is followed?

With Philipp's driver, this is easy, because there is a prepare_stream
callback which gives the sensor an opportunity to get everything
correctly configured according to the negotiated parameters, and place
the sensor in LP-11 state.

Some sensors do not power up in LP-11 state, but need to be programmed
fully before being asked to momentarily stream.  Only at that point is
the sensor guaranteed to be in the required LP-11 state.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 22/24] media: imx: Add MIPI CSI-2 OV5640 sensor subdev driver

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:40PM -0800, Steve Longerbeam wrote:
> +config IMX_OV5640_MIPI
> +   tristate "OmniVision OV5640 MIPI CSI-2 camera support"
> +   depends on GPIOLIB && VIDEO_IMX_CAMERA
> +   select IMX_MIPI_CSI2
> +   default y

Why is this defaulting to y?  New drivers should not default to enabled
unless they are replacing some already pre-existing functionality.
Ditto for the other camera driver.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v2 0/2] scsi: storvsc: Add support for FC lightweight host.

[KY Srinivasan]

> -Original Message-
> From: Christoph Hellwig [mailto:h...@infradead.org]
> Sent: Sunday, January 29, 2017 12:56 AM
> To: KY Srinivasan 
> Cc: Christoph Hellwig ; Cathy Avery
> ; Haiyang Zhang ;
> j...@linux.vnet.ibm.com; martin.peter...@oracle.com;
> dan.carpen...@oracle.com; de...@linuxdriverproject.org; linux-
> ker...@vger.kernel.org; linux-s...@vger.kernel.org; f...@redhat.com
> Subject: Re: [PATCH v2 0/2] scsi: storvsc: Add support for FC lightweight 
> host.
> 
> On Sun, Jan 29, 2017 at 12:35:32AM +, KY Srinivasan wrote:
> > Windows has chosen this model for virtualizing FC devices to the guest -
> > without rports (or vports). As I noted in my earlier email, James came
> > up with this notion of a lightweight template almost a year ago. We can
> > certainly pick a  more appropriate name and include better documentation.
> 
> Can we take a step back and figure out what you're trying to
> archive here.
> 
> storsvc is a paravirtualized device interface, and whatever underlies
> it should be of no relevance for the guest.
> 
> Despite that fact Microsoft apparently wants to expose a FC-like
> port_name and node_name to guests for some virtual disks.  Can you
> please explain what the guest is supposed to use them for?

My initial implementation of the virtual FC support on Hyper-V was exactly
what you are proposing here - FC disks were being treated exactly as SCSI
disks. We then had customers (HP Enterprise) that were building storage 
appliances based on FC disks on the host and they required port name and 
node name information to be published in sysfs so that their management tools
would work. They required the port and node ID information to be published
precisely the way the FC transport would publish them (in terms of location).

> 
> And second I'd like to understand what the fascination with the FC
> transport class is to expose these two attributes.  Given that
> your sysfs layout will be entirely different from real FC devices
> I simply don't see any need for that.
> 
> Why can't this whole thing simply be solved by adding sdev_attrs
> for the port_name and node_name to storsvc directly?

No fascination with the FC transport other than it already allowed for 
publishing the
port and node IDs. We can certainly look at other ways to publish this 
information -
the proposal by James to have this "light weight" FC transport was precisely 
for this reason.

Regards,

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

Re: [PATCH v3 06/24] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:24PM -0800, Steve Longerbeam wrote:
> + ov5640: camera@40 {
> + compatible = "ovti,ov5640";
> + pinctrl-names = "default";
> + pinctrl-0 = <_ov5640>;
> + clocks = <_xclk>;
> + clock-names = "xclk";
> + reg = <0x40>;
> + xclk = <2200>;
> + reset-gpios = < 5 GPIO_ACTIVE_LOW>; /* NANDF_D5 */
> + pwdn-gpios = < 9 GPIO_ACTIVE_HIGH>; /* NANDF_WP_B */
> +
> + port {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ov5640_to_mipi_csi: endpoint@1 {
> + reg = <1>;
> + remote-endpoint = <_csi_from_mipi_sensor>;
> + data-lanes = <0 1>;
> + clock-lanes = <2>;

How do you envision a four-lane sensor being described?

data-lanes = <0 1 3 4>;
clock-lanes = <2>;

?

The binding document for video-interfaces.txt says:

- clock-lanes: an array of physical clock lane indexes. Position of an entry
  determines the logical lane number, while the value of an entry indicates
  physical lane, e.g. for a MIPI CSI-2 bus we could have "clock-lanes = <0>;",
  which places the clock lane on hardware lane 0. This property is valid for
  serial busses only (e.g. MIPI CSI-2). Note that for the MIPI CSI-2 bus this
  array contains only one entry.

So I think you need to have a good reason to make the clock lane non-zero.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:39PM -0800, Steve Longerbeam wrote:
> Adds MIPI CSI-2 Receiver subdev driver. This subdev is required
> for sensors with a MIPI CSI2 interface.
> 
> Signed-off-by: Steve Longerbeam 

Applying: media: imx: Add MIPI CSI-2 Receiver subdev driver
.git/rebase-apply/patch:522: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 19/24] media: imx: Add IC subdev drivers

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:37PM -0800, Steve Longerbeam wrote:
> This is a set of three media entity subdevice drivers for the i.MX
> Image Converter. The i.MX IC module contains three independent
> "tasks":
> 
> - Pre-processing Encode task: video frames are routed directly from
>   the CSI and can be scaled, color-space converted, and rotated.
>   Scaled output is limited to 1024x1024 resolution. Output frames
>   are routed to the camera interface entities (camif).
> 
> - Pre-processing Viewfinder task: this task can perform the same
>   conversions as the pre-process encode task, but in addition can
>   be used for hardware motion compensated deinterlacing. Frames can
>   come either directly from the CSI or from the SMFC entities (memory
>   buffers via IDMAC channels). Scaled output is limited to 1024x1024
>   resolution. Output frames can be routed to various sinks including
>   the post-processing task entities.
> 
> - Post-processing task: same conversions as pre-process encode. However
>   this entity sends frames to the i.MX IPU image converter which supports
>   image tiling, which allows scaled output up to 4096x4096 resolution.
>   Output frames can be routed to the camera interfaces.
> 
> Signed-off-by: Steve Longerbeam 

Applying: media: imx: Add IC subdev drivers
.git/rebase-apply/patch:3054: new blank line at EOF.
+
warning: 1 line adds whitespace errors.


-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 17/24] media: imx: Add CSI subdev driver

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:35PM -0800, Steve Longerbeam wrote:
> This is a media entity subdevice for the i.MX Camera
> Serial Interface module.
> 
> Signed-off-by: Steve Longerbeam 

warning: 3 lines add whitespace errors.
Applying: media: imx: Add CSI subdev driver
.git/rebase-apply/patch:38: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 16/24] media: Add i.MX media core driver

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:34PM -0800, Steve Longerbeam wrote:
> Add the core media driver for i.MX SOC.
> 
> Signed-off-by: Steve Longerbeam 

Applying: media: Add i.MX media core driver
.git/rebase-apply/patch:516: new blank line at EOF.
+
.git/rebase-apply/patch:528: new blank line at EOF.
+
.git/rebase-apply/patch:556: new blank line at EOF.
+
warning: 3 lines add whitespace errors.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 06/24] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:24PM -0800, Steve Longerbeam wrote:
> diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts 
> b/arch/arm/boot/dts/imx6q-sabrelite.dts
> index 66d10d8..9e2d26d 100644
> --- a/arch/arm/boot/dts/imx6q-sabrelite.dts
> +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
> @@ -52,3 +52,9 @@
>   {
>   status = "okay";
>  };
> +
> +_csi1_from_mipi_vc1 {
> + data-lanes = <0 1>;
> + clock-lanes = <2>;
> +};
> +
> diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi 
> b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> index 795b5a5..bca9fed 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
...
> +/* Incoming port from sensor */
> +_csi_from_mipi_sensor {
> +remote-endpoint = <_to_mipi_csi>;
> +data-lanes = <0 1>;
> +clock-lanes = <2>;
> +};
> +

Applying: ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors
.git/rebase-apply/patch:33: new blank line at EOF.
+
.git/rebase-apply/patch:201: new blank line at EOF.
+
warning: 2 lines add whitespace errors.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: Need a second set of eyeballs for a possible startup race condition in vc04_services/vchiq.

[Michael Zoran]

On Tue, 2017-01-31 at 00:01 +0300, Dan Carpenter wrote:
> It's hard to review this because there are no callers and the hash
> you're talking about is an RPI hash...  You have no idea how lazy I
> am.
> 
> You're right, that code looks racy but it's almost certainly harmless
> depending on how it's called.  It has to race at the very very
> beginning
> otherwise it's fine.

I wonder if out of caution it would make sense to start stripping out
some of these entry points that don't have an obvious caller in the
kernel tree?

Anyway, here is the change from the linux-next tree.  The change 
essentially converts a call to a custom function
mutex_lock_interruptible_killable to simply mutex_lock_killable.


commit b826d73b3024485677163253b59ef9bd187ff765
Author: Arnd Bergmann 
Date:   Wed Nov 16 16:39:05 2016 +0100

staging: vc04_services: remove duplicate mutex_lock_interruptible

The driver tries to redefine mutex_lock_interruptible as an open-
coded
mutex_lock_killable, but that definition clashes with the normal
mutex_lock_interruptible definition when CONFIG_DEBUG_LOCK_ALLOC
is set:

staging/vc04_services/interface/vchiq_arm/vchiq_killable.h:67:0:
error: "mutex_lock_interruptible" redefined [-Werror]
 #define mutex_lock_interruptible mutex_lock_interruptible_killable
include/linux/mutex.h:161:0: note: this is the location of the
previous definition

This simply removes the private implementation and uses the
normal mutex_lock_killable directly.

We could do the same for the down_interruptible_killable here, but
it's better to just remove the semaphores entirely from the driver,
which also takes care of that.

Signed-off-by: Arnd Bergmann 
Signed-off-by: Greg Kroah-Hartman 

diff --git
a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index d0435a05ea35..0d987898b4f8 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -554,7 +554,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
    ret = -EINVAL;
    break;
    }
-   rc = mutex_lock_interruptible(>state-
>mutex);
+   rc = mutex_lock_killable(>state->mutex);
    if (rc != 0) {
    vchiq_log_error(vchiq_arm_log_level,
    "vchiq: connect: could not lock mutex
for "
diff --git
a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
index 5efc62ffb2f5..7ea29665bd0c 100644
---
a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
+++
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
@@ -72,7 +72,7 @@ void
vchiq_add_connected_callback(VCHIQ_CONNECTED_CALLBACK_T callback)
 {
    connected_init();
 
-   if (mutex_lock_interruptible(_connected_mutex) != 0)
+   if (mutex_lock_killable(_connected_mutex) != 0)
    return;
 
    if (g_connected)
@@ -107,7 +107,7 @@ void vchiq_call_connected_callbacks(void)
 
    connected_init();
 
-   if (mutex_lock_interruptible(_connected_mutex) != 0)
+   if (mutex_lock_killable(_connected_mutex) != 0)
    return;
 
    for (i = 0; i <  g_num_deferred_callbacks; i++)
diff --git
a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 7440db2ce40b..028e90bc1cdc 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -794,7 +794,7 @@ queue_message(VCHIQ_STATE_T *state, VCHIQ_SERVICE_T
*service,
    WARN_ON(!(stride <= VCHIQ_SLOT_SIZE));
 
    if (!(flags & QMFLAGS_NO_MUTEX_LOCK) &&
-   (mutex_lock_interruptible(>slot_mutex) != 0))
+   (mutex_lock_killable(>slot_mutex) != 0))
    return VCHIQ_RETRY;
 
    if (type == VCHIQ_MSG_DATA) {
@@ -863,7 +863,7 @@ queue_message(VCHIQ_STATE_T *state, VCHIQ_SERVICE_T
*service,
    return VCHIQ_RETRY;
    if (service->closing)
    return VCHIQ_ERROR;
-   if (mutex_lock_interruptible(
>slot_mutex) != 0)
+   if (mutex_lock_killable(>slot_mutex) !=
0)
    return VCHIQ_RETRY;
    if (service->srvstate != VCHIQ_SRVSTATE_OPEN)
{
    /* The service has been closed */
@@ -1033,7 +1033,7 @@ queue_message_sync(VCHIQ_STATE_T *state,
VCHIQ_SERVICE_T *service,
    local = state->local;
 
    if ((VCHIQ_MSG_TYPE(msgid) != VCHIQ_MSG_RESUME) &&
-   

Re: Need a second set of eyeballs for a possible startup race condition in vc04_services/vchiq.

[Dan Carpenter]

It's hard to review this because there are no callers and the hash
you're talking about is an RPI hash...  You have no idea how lazy I am.

You're right, that code looks racy but it's almost certainly harmless
depending on how it's called.  It has to race at the very very beginning
otherwise it's fine.

regards,
dan carpenter

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

Re: [PATCH 6/6] staging: bcm2835-v4l2: Apply spelling fixes from checkpatch.

[Eric Anholt]

Joe Perches  writes:

> On Fri, 2017-01-27 at 13:55 -0800, Eric Anholt wrote:
>> Generated with checkpatch.pl --fix-inplace and git add -p out of the
>> results.
>
> Maybe another.
>
>> diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c 
>> b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
> []
>> @@ -239,7 +239,7 @@ static int bulk_receive(struct vchiq_mmal_instance 
>> *instance,
>>  pr_err("buffer list empty trying to submit bulk receive\n");
>>  
>>  /* todo: this is a serious error, we should never have
>> - * commited a buffer_to_host operation to the mmal
>> + * committed a buffer_to_host operation to the mmal
>>   * port without the buffer to back it up (underflow
>>   * handling) and there is no obvious way to deal with
>>   * this - how is the mmal servie going to react when
>
> Perhaps s/servie/service/ ?

I was trying to restrict this patch to just the fixes from checkpatch.


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

Re: [PATCH RESEND] staging: media: lirc: use new parport device model

[Sean Young]

On Sat, Jan 21, 2017 at 12:55:54AM +, Sudip Mukherjee wrote:
> From: Sudip Mukherjee 
> 
> Modify lirc_parallel driver to use the new parallel port device model.
> 
> Signed-off-by: Sudip Mukherjee 
> ---
> 
> Resending after more than one year.
> Prevoius patch is at https://patchwork.kernel.org/patch/7883591/

Since noone ported lirc_parallel to rc-core, the lirc_parallel staging
driver has been droppped from the current media tree.

I have ported a few other lirc drivers to rc-core but I never found
anyone using lirc_parallel or the hardware itself.


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

Re: [PATCH] staging: bcm2835-audio: Strengthen build dependencies

[Michael Zoran]

On Mon, 2017-01-30 at 08:56 -0800, Eric Anholt wrote:
> Michael Zoran  writes:
> 
> > This driver makes no sense outside of ARM or ARM64.
> > Add an explicit build dependency on:
> > (ARM || ARM64 || COMPILE_TEST)
> > 
> > Also set the default build to n
> > 
> > Signed-off-by: Michael Zoran 
> > ---
> >  drivers/staging/bcm2835-audio/Kconfig | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/staging/bcm2835-audio/Kconfig
> > b/drivers/staging/bcm2835-audio/Kconfig
> > index 32a2ff9ef9b2..840faa21f665 100644
> > --- a/drivers/staging/bcm2835-audio/Kconfig
> > +++ b/drivers/staging/bcm2835-audio/Kconfig
> > @@ -1,6 +1,8 @@
> >  config SND_BCM2835
> >  tristate "BCM2835 ALSA driver"
> >  depends on ARCH_BCM2835 && BCM2835_VCHIQ && SND
> > +   depends on (ARM || ARM64 || COMPILE_TEST)
> > +   default n
> >  select SND_PCM
> >  help
> >    Say Y or M if you want to support BCM2835 Alsa pcm card
> > driver
> 
> We've already got a DEPENDS on ARCH_BCM2835, how would that be
> present
> without one of those three being set?
> 

I can't think of a reason why ARCH_BCM2835 would be set without ARM ||
ARM64.  I'm just trying to be cautious.  I still would like the
"default n" added just as a precaution.  I thought the default was n
unless provided but I see other drivers doing that and build config has
some complex options for upgrading old configurations that I don't
completely understand.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Staging: media: lirc: style fix, using octal file permissions

[Sean Young]

On Sat, Jan 07, 2017 at 04:02:55PM +1300, Derek Robson wrote:
> Change file permissions to octal style.
> Found using checkpatch
> 
> Signed-off-by: Derek Robson 
> ---
>  drivers/staging/media/lirc/lirc_imon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/lirc/lirc_imon.c 
> b/drivers/staging/media/lirc/lirc_imon.c
> index 1e650fba4a92..6c8a4a15278e 100644
> --- a/drivers/staging/media/lirc/lirc_imon.c
> +++ b/drivers/staging/media/lirc/lirc_imon.c
> @@ -182,7 +182,7 @@ MODULE_DESCRIPTION(MOD_DESC);
>  MODULE_VERSION(MOD_VERSION);
>  MODULE_LICENSE("GPL");
>  MODULE_DEVICE_TABLE(usb, imon_usb_id_table);
> -module_param(debug, int, S_IRUGO | S_IWUSR);
> +module_param(debug, int, 0644);
>  MODULE_PARM_DESC(debug, "Debug messages: 0=no, 1=yes(default: no)");
>  
>  static void free_imon_context(struct imon_context *context)

In the current media tree, drivers/staging/media/lirc/lirc_imon.c has
been merged with drivers/media/rc/imon.c already, I'm afraid. This
patch no longer applies.


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

RE: [PATCH 00/14] hv: vmbus cleanup patches

[Stephen Hemminger]

Why don't I rebase (on char-misc-next) and put the functional patches in first 
then the cleanups

-Original Message-
From: KY Srinivasan 
Sent: Saturday, January 28, 2017 10:09 AM
To: Stephen Hemminger ; Haiyang Zhang 

Cc: de...@linuxdriverproject.org; Stephen Hemminger 
Subject: RE: [PATCH 00/14] hv: vmbus cleanup patches



> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Monday, January 23, 2017 5:40 PM
> To: KY Srinivasan ; Haiyang Zhang 
> 
> Cc: de...@linuxdriverproject.org; Stephen Hemminger 
> 
> Subject: [PATCH 00/14] hv: vmbus cleanup patches
> 
> No new functionality in this set. It just involves removing unused 
> argments, no longer used code, and style fixes
> 
> Stephen Hemminger (14):
>   vmbus: remove useless return's
>   vmbus: constify parameters where possible
>   vmbus: use kernel bitops for traversing interrupt mask
>   vmbus: eliminate unnecessary wrapper functions
>   vmbus: drop no longer used kick_q argument
>   vmbus: remove no longer used signal_policy
>   vmbus: remove conditional locking of vmbus_write
>   vmbus: remove unused kickq argument to sendpacket
>   vmbus: remove unnecessary initialization
>   vmbus: fix spelling errors
>   netvsc: remove no longer needed receive staging buffers
>   vmbus: remove per channel state
>   vmbus: callback is in softirq not workqueue
>   vmbus: remove empty function
> 
>  drivers/hv/channel.c  |  53 +++
>  drivers/hv/channel_mgmt.c |   1 -
>  drivers/hv/connection.c   |  55 +++-
>  drivers/hv/hv.c   |  10 
>  drivers/hv/hv_balloon.c   |   2 -
>  drivers/hv/hv_fcopy.c |   2 -
>  drivers/hv/hv_kvp.c   |  12 ++---
>  drivers/hv/hv_snapshot.c  |   2 -
>  drivers/hv/hyperv_vmbus.h |  26 +-
>  drivers/hv/ring_buffer.c  |  93 ++
>  drivers/hv/vmbus_drv.c|  12 +
>  drivers/net/hyperv/hyperv_net.h   |   5 --
>  drivers/net/hyperv/netvsc.c   | 104 
> +-
>  drivers/net/hyperv/rndis_filter.c |  11 
>  include/linux/hyperv.h|  72 --
>  15 files changed, 104 insertions(+), 356 deletions(-)

Stephen, I only got the first 10 patches of this series. Can you please resend 
the series.

Regards,

K. Y
> 
> --
> 2.11.0

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

Re: [PATCH] staging: bcm2835-audio: Strengthen build dependencies

[Eric Anholt]

Michael Zoran  writes:

> This driver makes no sense outside of ARM or ARM64.
> Add an explicit build dependency on:
> (ARM || ARM64 || COMPILE_TEST)
>
> Also set the default build to n
>
> Signed-off-by: Michael Zoran 
> ---
>  drivers/staging/bcm2835-audio/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/staging/bcm2835-audio/Kconfig 
> b/drivers/staging/bcm2835-audio/Kconfig
> index 32a2ff9ef9b2..840faa21f665 100644
> --- a/drivers/staging/bcm2835-audio/Kconfig
> +++ b/drivers/staging/bcm2835-audio/Kconfig
> @@ -1,6 +1,8 @@
>  config SND_BCM2835
>  tristate "BCM2835 ALSA driver"
>  depends on ARCH_BCM2835 && BCM2835_VCHIQ && SND
> + depends on (ARM || ARM64 || COMPILE_TEST)
> + default n
>  select SND_PCM
>  help
>Say Y or M if you want to support BCM2835 Alsa pcm card driver

We've already got a DEPENDS on ARCH_BCM2835, how would that be present
without one of those three being set?



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

Re: [PATCH 001] staging: wlan-ng: Add tabstop preceding the statement

[Greg Kroah-Hartman]

On Mon, Jan 30, 2017 at 11:31:42AM -0500, Maksymilian Piechota wrote:
> This patch fixes the checkpatch.pl warning:
> 
> WARNING: Statements should start on a tabstop
> 
> Signed-off-by: Maksymilian Piechota 
> ---
>  drivers/staging/wlan-ng/prism2mgmt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/wlan-ng/prism2mgmt.c 
> b/drivers/staging/wlan-ng/prism2mgmt.c
> index 16fb2d3..2d67125 100644
> --- a/drivers/staging/wlan-ng/prism2mgmt.c
> +++ b/drivers/staging/wlan-ng/prism2mgmt.c
> @@ -1308,7 +1308,7 @@ int prism2mgmt_wlansniff(struct wlandevice *wlandev, 
> void *msgp)
>   hw->sniffhdr = 0;
>   wlandev->netdev->type = ARPHRD_IEEE80211_PRISM;
>   } else
> - if ((msg->wlanheader.status ==
> + if ((msg->wlanheader.status ==
>P80211ENUM_msgitem_status_data_ok)
>   && (msg->wlanheader.data == P80211ENUM_truth_true)) {
>   hw->sniffhdr = 1;

Hm, this all doesn't look correct now, does it?  Please fix up the whole
if statement here.

thanks,

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

[PATCH 001] staging: wlan-ng: Add tabstop preceding the statement

[Maksymilian Piechota]

This patch fixes the checkpatch.pl warning:

WARNING: Statements should start on a tabstop

Signed-off-by: Maksymilian Piechota 
---
 drivers/staging/wlan-ng/prism2mgmt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wlan-ng/prism2mgmt.c 
b/drivers/staging/wlan-ng/prism2mgmt.c
index 16fb2d3..2d67125 100644
--- a/drivers/staging/wlan-ng/prism2mgmt.c
+++ b/drivers/staging/wlan-ng/prism2mgmt.c
@@ -1308,7 +1308,7 @@ int prism2mgmt_wlansniff(struct wlandevice *wlandev, void 
*msgp)
hw->sniffhdr = 0;
wlandev->netdev->type = ARPHRD_IEEE80211_PRISM;
} else
-   if ((msg->wlanheader.status ==
+   if ((msg->wlanheader.status ==
 P80211ENUM_msgitem_status_data_ok)
&& (msg->wlanheader.data == P80211ENUM_truth_true)) {
hw->sniffhdr = 1;
-- 
2.1.4

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

Re: [PATCH 1/1] iio: fixed symbolic permission references 'S_IRUGO | S_IWUSR'

[Greg KH]

On Mon, Jan 30, 2017 at 11:47:35AM +0100, Hartmut Knaack wrote:
> Artur Lorincz schrieb am 30.01.2017 um 11:10:
> > Replaced the symbolic permission references S_IRUGO and S_IWUSR with their
> > octal counterparts.
> 
> Hi Artur,
> patches concerning IIO should be sent to the linux-iio mailing list, and
> former maintainers of the file should be CC'ed.
> Now, regarding your patch, I am not convinced that this is an improvement.
> Those symbols are usually used to improve readability, which is not given
> with more or less magical numbers.

Sorry, but no, they should be replaced, as these are "well known" magic
numbers :)

thanks,

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

Re: [PATCH 1/1] iio: fixed symbolic permission references 'S_IRUGO | S_IWUSR'

[Greg KH]

On Mon, Jan 30, 2017 at 11:10:36AM +0100, Artur Lorincz wrote:
> Replaced the symbolic permission references S_IRUGO and S_IWUSR with their
> octal counterparts.
> 
> Signed-off-by: Artur Lorincz 
> ---
>  drivers/staging/iio/adc/ad7192.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Always use scripts/get_maintainer.pl to determine who to send a patch
to.

Please fix up and try again.

thanks,

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

Re: [PATCH v3 16/24] media: Add i.MX media core driver

[Russell King - ARM Linux]

On Mon, Jan 23, 2017 at 12:13:26PM +0100, Philipp Zabel wrote:
> Hi Steve,
> 
> On Sun, 2017-01-22 at 18:31 -0800, Steve Longerbeam wrote:
> > Second, ignoring the above locking issue for a moment, 
> > v4l2_pipeline_pm_use()
> > will call s_power on the sensor _first_, then the mipi csi-2 s_power, 
> > when executing
> > media-ctl -l '"ov5640 1-003c":0 -> "imx6-mipi-csi2":0[1]'. Which is the 
> > wrong order.
> > In my version which enforces the correct power on order, the mipi csi-2 
> > s_power
> > is called first in that link setup, followed by the sensor.
> 
> I don't understand why you want to power up subdevs as soon as the links
> are established. Shouldn't that rather be done for all subdevices in the
> pipeline when the corresponding capture device is opened?
> It seems to me that powering up the pipeline should be the last step
> before userspace actually starts the capture.

I agree with Philipp here - configuration of the software pipeline
shouldn't result in hardware being forced to be powered up.  That's
more of a decision for the individual sub-driver than for core.

Executing media-ctl to enable a link between two sub-device endpoints
should really be a matter of setting the software state, and when the
video device is opened for streaming, surely that's when the hardware
in the chain between the source and the capture device should be
powered up and programmed.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 00/24] i.MX Media Driver

[Russell King - ARM Linux]

> The central issue seems to be that I think media pad links / media bus
> formats should describe physical links, such as parallel or serial
> buses, and the formats of pixels flowing through them, whereas Steve
> would like to extend them to describe software transports and in-memory
> formats.

This probably isn't the right place to attach this comment in this
thread, but... the issue of media bus formats matching physical buses
is an argument that I think is already lost.

For example, take the 10-bit bayer formats:

#define MEDIA_BUS_FMT_SBGGR10_1X10  0x3007
#define MEDIA_BUS_FMT_SGBRG10_1X10  0x300e
#define MEDIA_BUS_FMT_SGRBG10_1X10  0x300a
#define MEDIA_BUS_FMT_SRGGB10_1X10  0x300f

These are commonly used on CSI serial buses (see the smiapp driver for
example).  From the description at the top of the file, it says the
1X10 means that one pixel is transferred as one 10-bit sample.

However, the format on wire is somewhat different - four pixels are
transmitted over five bytes:

P0  P1  P2  P3  P0  P1  P2  P3
8-bit   8-bit   8-bit   8-bit   2-bit   2-bit   2-bit   2-bit

This gives two problems:
1) it doesn't fit in any sensible kind of "one pixel transferred as
   N M-bit samples" description because the pixel/sample values
   (depending how you look at them) are broken up.

2) changing this will probably be a user visible change, as things
   like smiapp are already in use.

So, I think what we actually have is the media bus formats describing
the _logical_ bus format.  Yes, one pixel is transferred as one 10-bit
sample in this case.

To help illustrate my point, consider the difference between
MEDIA_BUS_FMT_RGB565_1X16 and MEDIA_BUS_FMT_RGB565_2X8_BE or
MEDIA_BUS_FMT_RGB565_2X8_LE.  RGB565_1X16 means 1 pixel over an effective
16-bit wide bus (if it's not 16-bit, then it has to be broken up into
separate "samples".)  RGB565_2X8 means 1 pixel as two 8-bit samples.

So, the 10-bit bayer is 1 pixel as 1.25 bytes.  Or is it, over a serial
bus.  Using the RGB565 case, 10-bit bayer over a 4 lane CSI bus becomes
interesting:

first byte  2nd 3rd
lane 1  P0 9:2  S0  P7 9:2
lane 2  P1 9:2  P4 9:2  S1
lane 3  P2 9:2  P5 9:2  P8 9:2
lane 4  P3 9:2  P6 9:2  P9 9:2

S0 = P0/P1/P2/P3 least significant two bits
S1 = P4/P5/P6/P7 least significant two bits

or 2 lane CSI:
first byte  2nd 3rd 4th 5th
lane 1  P0 9:2  P2  S0  P5  P7
lane 2  P1 9:2  P3  P4  P6  S1

or 1 lane CSI:
lane 1  P0 P1 P2 P3 S0 P4 P5 P6 P7 S1 P8 P9 ...

etc.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Need a second set of eyeballs for a possible startup race condition in vc04_services/vchiq.

[Michael Zoran]

Resending to a larger e-mail list...

On Mon, 2017-01-30 at 04:57 -0800, Michael Zoran wrote:
> I'm looking at linux-next:
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
> 
> First it looks this is some kind of startup notification mechanism
> and
> it is used by the custom RPI kernel on www.github.com. Client drivers
> call vchiq_add_connected_callback to register for a notification that
> the driver is good to go.
> 
> But I see a few interesting issues:
> 
> 1. vchiq_add_connected_callback can be called by multiple clients at
> the same time.  If this happens they will all find that g_once_init
> isn't set, so all the clients will call mutex_init at the same time. 
> That's sounds like a possible corruption issue. 
> 
> 2. On a multiprocessor machine, I didn't think it was OK to simply
> set
> g_once_init without any kind of protection either since it may not
> propagate to the other cores.  You know some kind of barrier
> instruction.
> 
> 3. A change was made in checkin
> 826d73b3024485677163253b59ef9bd187ff765.
> 
> This changed the function mutex_lock_interruptable which was at that
> time a macro to a custom function called
> mutex_lock_interruptable_killable to simply mutex_lock_killable. The
> old function does a dance with setting the process signal masks, but
> mutex_lock_killable doesn't.
> 
> Like I said, I'm new to trying to contribute to the linux kernel. But
> I'm not sure the two are completely a drop in replacement. I was
> wondering if the change is doing the correct thing?
> 
> It looks a fix would be easy to implement just by adding an atomic
> exchange instead of a simple test and set.
> 
> Thoughts?
> 
> 
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
> 
> static void connected_init(void)
> {
> if (!g_once_init) {
> mutex_init(_connected_mutex);
> g_once_init = 1;
> }
> }
> 
> /
> **
> **
> *
> * This function is used to defer initialization until the vchiq stack
> is
> * initialized. If the stack is already initialized, then the callback
> will
> * be made immediately, otherwise it will be deferred until
> * vchiq_call_connected_callbacks is called.
> *
> *
> **
> /
> 
> void vchiq_add_connected_callback(VCHIQ_CONNECTED_CALLBACK_T
> callback)
> {
> connected_init();
> 
> if (mutex_lock_killable(_connected_mutex) != 0)
> return;
> 
> if (g_connected)
> /* We're already connected. Call the callback
> immediately. */
> 
> callback();
> else {
> if (g_num_deferred_callbacks >= MAX_CALLBACKS)
> vchiq_log_error(vchiq_core_log_level,
> "There already %d callback registered
> -
> "
> "please increase MAX_CALLBACKS",
> g_num_deferred_callbacks);
> else {
> g_deferred_callback[g_num_deferred_callbacks]
> =
> callback;
> g_num_deferred_callbacks++;
> }
> }
> mutex_unlock(_connected_mutex);
> }
> 
> /
> **
> **
> *
> * This function is called by the vchiq stack once it has been
> connected
> to
> * the videocore and clients can start to use the stack.
> *
> *
> **
> /
> 
> void vchiq_call_connected_callbacks(void)
> {
> int i;
> 
> connected_init();
> 
> if (mutex_lock_killable(_connected_mutex) != 0)
> return;
> 
> for (i = 0; i <  g_num_deferred_callbacks; i++)
> g_deferred_callback[i]();
> 
> g_num_deferred_callbacks = 0;
> g_connected = 1;
> mutex_unlock(_connected_mutex);
> }
> EXPORT_SYMBOL(vchiq_add_connected_callback);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Need a second set of eyeballs for a possible startup race condition in vc04_services/vchiq.

[Michael Zoran]

I'm looking at linux-next:
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c

First it looks this is some kind of startup notification mechanism and
it is used by the custom RPI kernel on www.github.com. Client drivers
call vchiq_add_connected_callback to register for a notification that
the driver is good to go.

But I see a few interesting issues:

1. vchiq_add_connected_callback can be called by multiple clients at
the same time.  If this happens they will all find that g_once_init
isn't set, so all the clients will call mutex_init at the same time. 
That's sounds like a possible corruption issue. 

2. On a multiprocessor machine, I didn't think it was OK to simply set
g_once_init without any kind of protection either since it may not
propagate to the other cores.  You know some kind of barrier
instruction.

3. A change was made in checkin
826d73b3024485677163253b59ef9bd187ff765.

This changed the function mutex_lock_interruptable which was at that
time a macro to a custom function called
mutex_lock_interruptable_killable to simply mutex_lock_killable. The
old function does a dance with setting the process signal masks, but
mutex_lock_killable doesn't.

Like I said, I'm new to trying to contribute to the linux kernel. But
I'm not sure the two are completely a drop in replacement. I was
wondering if the change is doing the correct thing?

It looks a fix would be easy to implement just by adding an atomic
exchange instead of a simple test and set.

Thoughts?


drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c

static void connected_init(void)
{
if (!g_once_init) {
mutex_init(_connected_mutex);
g_once_init = 1;
}
}

/**
**
*
* This function is used to defer initialization until the vchiq stack
is
* initialized. If the stack is already initialized, then the callback
will
* be made immediately, otherwise it will be deferred until
* vchiq_call_connected_callbacks is called.
*
***
/

void vchiq_add_connected_callback(VCHIQ_CONNECTED_CALLBACK_T callback)
{
connected_init();

if (mutex_lock_killable(_connected_mutex) != 0)
return;

if (g_connected)
/* We're already connected. Call the callback
immediately. */

callback();
else {
if (g_num_deferred_callbacks >= MAX_CALLBACKS)
vchiq_log_error(vchiq_core_log_level,
"There already %d callback registered -
"
"please increase MAX_CALLBACKS",
g_num_deferred_callbacks);
else {
g_deferred_callback[g_num_deferred_callbacks] =
callback;
g_num_deferred_callbacks++;
}
}
mutex_unlock(_connected_mutex);
}

/**
**
*
* This function is called by the vchiq stack once it has been connected
to
* the videocore and clients can start to use the stack.
*
***
/

void vchiq_call_connected_callbacks(void)
{
int i;

connected_init();

if (mutex_lock_killable(_connected_mutex) != 0)
return;

for (i = 0; i <  g_num_deferred_callbacks; i++)
g_deferred_callback[i]();

g_num_deferred_callbacks = 0;
g_connected = 1;
mutex_unlock(_connected_mutex);
}
EXPORT_SYMBOL(vchiq_add_connected_callback);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 13/60] staging: lustre: obdclass: health_check to report unhealthy upon LBUG

[Dan Carpenter]

Wat?

I'm sorry but this patch makes no sense at all.

On Sat, Jan 28, 2017 at 07:04:41PM -0500, James Simmons wrote:
> From: Bruno Faccini 
> 
> When a LBUG has occurred, without panic_on_lbug being set,
> health_check sysfs file must return an unhealthy state.

Why?

> 
> Signed-off-by: Bruno Faccini 
> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-7486
> Reviewed-on: http://review.whamcloud.com/17981
> Reviewed-by: Bobi Jam 
> Reviewed-by: Niu Yawei 
> Reviewed-by: James Simmons 
> Reviewed-by: Oleg Drokin 
> Signed-off-by: James Simmons 
> ---
>  drivers/staging/lustre/lustre/obdclass/linux/linux-module.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c 
> b/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
> index 22e6d1f..ef25db6 100644
> --- a/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
> +++ b/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
> @@ -224,8 +224,10 @@ static ssize_t pinger_show(struct kobject *kobj, struct 
> attribute *attr,
>   int i;
>   size_t len = 0;
>  
> - if (libcfs_catastrophe)
> - return sprintf(buf, "LBUG\n");
> + if (libcfs_catastrophe) {
> + len = sprintf(buf, "LBUG\n");

This line is dead code, now.

> + healthy = false;
> + }
>  

regards,
dan carpenter

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

Re: [PATCH 05/60] staging: lustre: llite: check request != NULL in ll_migrate

[Dan Carpenter]

On Sat, Jan 28, 2017 at 07:04:33PM -0500, James Simmons wrote:
> From: wang di 
> 
> Check if the request is NULL, before retrieve reply body
> from the request.
> 
> Signed-off-by: wang di 
> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-7396
> Reviewed-on: http://review.whamcloud.com/17079
> Reviewed-by: John L. Hammond 
> Reviewed-by: Andreas Dilger 
> Signed-off-by: James Simmons 
> ---
>  drivers/staging/lustre/lustre/llite/file.c | 41 
> +-
>  1 file changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/llite/file.c 
> b/drivers/staging/lustre/lustre/llite/file.c
> index a1e51a5..b681e15 100644
> --- a/drivers/staging/lustre/lustre/llite/file.c
> +++ b/drivers/staging/lustre/lustre/llite/file.c
> @@ -2656,28 +2656,33 @@ int ll_migrate(struct inode *parent, struct file 
> *file, int mdtidx,
>   if (!rc)
>   ll_update_times(request, parent);
>  

I don't like how we return a non-NULL request on many error paths.  It
would be simpler to understand if mdc_rename() freed request on error.
So mdc_reint() fails but we still continue?  I don't understand that but
there are no comments about it.


> - body = req_capsule_server_get(>rq_pill, _MDT_BODY);
> - if (!body) {
> - rc = -EPROTO;
> - goto out_free;
> - }
> + if (request) {
> + body = req_capsule_server_get(>rq_pill, _MDT_BODY);
> + if (!body) {
> + rc = -EPROTO;
> + goto out_free;

We should call ptlrpc_req_finished(request) before returning on this
path.

regards,
dan carpenter


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

[PATCH v5 1/3] hv_util: switch to using timespec64

[Vitaly Kuznetsov]

do_settimeofday() is deprecated, use do_settimeofday64() instead.

Signed-off-by: Vitaly Kuznetsov 
Acked-by: John Stultz 
Acked-by: Thomas Gleixner 
---
 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 7c90e33..51528bf 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -185,7 +185,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);
 
@@ -204,9 +204,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(_ts);
+   do_settimeofday64(_ts);
 }
 
 /*
-- 
2.9.3

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

[PATCH v5 2/3] hv: export current Hyper-V clocksource

[Vitaly Kuznetsov]

As a preparation to implementing Hyper-V PTP device supporting
.getcrosststamp we need to export a reference to the current Hyper-V
clocksource in use (MSR or TSC page).

Signed-off-by: Vitaly Kuznetsov 
---
 arch/x86/hyperv/hv_init.c   | 13 +
 arch/x86/include/asm/mshyperv.h |  3 +++
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index b371d0e..c224b7d 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -99,6 +99,9 @@ static struct clocksource hyperv_cs_msr = {
 };
 
 static void *hypercall_pg;
+struct clocksource *hyperv_cs;
+EXPORT_SYMBOL_GPL(hyperv_cs);
+
 /*
  * This function is to be invoked early in the boot sequence after the
  * hypervisor has been detected.
@@ -141,10 +144,10 @@ void hyperv_init(void)
union hv_x64_msr_hypercall_contents tsc_msr;
 
tsc_pg = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL);
-   if (!tsc_pg) {
-   clocksource_register_hz(_cs_msr, 
NSEC_PER_SEC/100);
-   return;
-   }
+   if (!tsc_pg)
+   goto register_msr_cs;
+
+   hyperv_cs = _cs_tsc;
 
rdmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64);
 
@@ -161,6 +164,8 @@ void hyperv_init(void)
 * the partition counter.
 */
 
+register_msr_cs:
+   hyperv_cs = _cs_msr;
if (ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE)
clocksource_register_hz(_cs_msr, NSEC_PER_SEC/100);
 }
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index f8dc370..d54833d 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -3,6 +3,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 /*
@@ -168,6 +169,8 @@ void hv_setup_crash_handler(void (*handler)(struct pt_regs 
*regs));
 void hv_remove_crash_handler(void);
 
 #if IS_ENABLED(CONFIG_HYPERV)
+extern struct clocksource *hyperv_cs;
+
 void hyperv_init(void);
 void hyperv_report_panic(struct pt_regs *regs);
 bool hv_is_hypercall_page_setup(void);
-- 
2.9.3

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

[PATCH v5 3/3] hv_utils: implement Hyper-V PTP source

[Vitaly Kuznetsov]

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 server runs in parallel and uses something else for time
  sync (network, PTP,...) system time will never converge.
- Systemd starts annoying you by printing "Time has been changed" every 5
  seconds to the system log.

Instead of doing in-kernel time adjustments offload the work to an
NTP client by exposing TimeSync messages as a PTP device. Users may now
decide what they want to use as a source.

I tested the solution with chrony, the config was:

 refclock PHC /dev/ptp0 poll 3 dpoll -2 offset 0

The result I'm seeing is accurate enough, the time delta between the guest
and the host is almost always within [-10us, +10us], the in-kernel solution
was giving us comparable results.

I also tried implementing PPS device instead of PTP by using not currently
used Hyper-V synthetic timers (we use only one of four for clockevent) but
with PPS source only chrony wasn't able to give me the required accuracy,
the delta often more that 100us.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/hv_util.c | 179 +++
 1 file changed, 152 insertions(+), 27 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 51528bf..f3c03b2 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -27,6 +27,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 #include "hyperv_vmbus.h"
@@ -182,29 +184,15 @@ struct adj_time_work {
 
 static void hv_set_host_time(struct work_struct *work)
 {
-   struct adj_time_work*wrk;
-   s64 host_tns;
-   u64 newtime;
+   struct adj_time_work *wrk;
struct timespec64 host_ts;
+   u64 reftime, newtime;
 
wrk = container_of(work, struct adj_time_work, work);
 
-   newtime = wrk->host_time;
-   if (ts_srv_version > TS_VERSION_3) {
-   /*
-* Some latency has been introduced since Hyper-V generated
-* its time sample. Take that latency into account before
-* using TSC reference time sample from Hyper-V.
-*
-* This sample is given by TimeSync v4 and above hosts.
-*/
-   u64 current_tick;
-
-   hv_get_current_tick(current_tick);
-   newtime += (current_tick - wrk->ref_time);
-   }
-   host_tns = (newtime - WLTIMEDELTA) * 100;
-   host_ts = ns_to_timespec64(host_tns);
+   reftime = hyperv_cs->read(hyperv_cs);
+   newtime = wrk->host_time + (reftime - wrk->ref_time);
+   host_ts = ns_to_timespec64((newtime - WLTIMEDELTA) * 100);
 
do_settimeofday64(_ts);
 }
@@ -223,22 +211,60 @@ static void hv_set_host_time(struct work_struct *work)
  * to discipline the clock.
  */
 static struct adj_time_work  wrk;
-static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 flags)
+
+/*
+ * The last time sample, received from the host. PTP device responds to
+ * requests by using this data and the current partition-wide time reference
+ * count.
+ */
+static struct {
+   u64 host_time;
+   u64 ref_time;
+   struct system_time_snapshot snap;
+   spinlock_t  lock;
+} host_ts;
+
+static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 adj_flags)
 {
+   unsigned long flags;
+   u64 cur_reftime;
 
/*
 * This check is safe since we are executing in the
 * interrupt context and time synch messages arre always
 * delivered on the same CPU.
 */
-   if (work_pending())
-   return;
-
-   wrk.host_time = hosttime;
-   wrk.ref_time = reftime;
-   wrk.flags = flags;
-   if ((flags & (ICTIMESYNCFLAG_SYNC | ICTIMESYNCFLAG_SAMPLE)) != 0) {
+   if (adj_flags & ICTIMESYNCFLAG_SYNC) {
+   /* Queue a job to do do_settimeofday64() */
+   if (work_pending())
+   return;
+
+   wrk.host_time = hosttime;
+   wrk.ref_time = reftime;
+   wrk.flags = adj_flags;
schedule_work();
+   } else {
+   /*
+* Save the adjusted time sample from the host and the snapshot
+* of the current system time for PTP device.
+*/
+   spin_lock_irqsave(_ts.lock, flags);
+
+   cur_reftime = hyperv_cs->read(hyperv_cs);
+   host_ts.host_time = hosttime;
+   host_ts.ref_time = cur_reftime;
+   ktime_get_snapshot(_ts.snap);
+
+   /*
+   

[PATCH v5 0/3] hv_util: adjust system time smoothly

[Vitaly Kuznetsov]

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 server runs in parallel and uses something else for time
  sync (network, PTP,...) system time will never converge.
- Systemd starts annoying you by printing "Time has been changed" every 5
  seconds to the system log.

Instead of doing in-kernel time adjustments offload the work to an
NTP client by exposing TimeSync messages as a PTP device. Users may now
decide what they want to use as a source.

Changes since v4:
- Rebase to the current char-misc-next tree [K. Y.]
- Drop const qualifier from ptp_hyperv_info [kbuild test robot]
- Implement .getcrosststamp() callback [Radim Krcmar]

Vitaly Kuznetsov (3):
  hv_util: switch to using timespec64
  hv: export current Hyper-V clocksource
  hv_utils: implement Hyper-V PTP source

 arch/x86/hyperv/hv_init.c   |  13 ++-
 arch/x86/include/asm/mshyperv.h |   3 +
 drivers/hv/hv_util.c| 183 +---
 3 files changed, 166 insertions(+), 33 deletions(-)

-- 
2.9.3

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

Re: [PATCH] staging: greybus: operation: add generic timeout support

[Bryan O'Donoghue]

On 27/01/17 09:55, Greg Kroah-Hartman wrote:
> On Tue, Jan 24, 2017 at 03:57:54PM +, Bryan O'Donoghue wrote:
>> On 24/01/17 15:09, Johan Hovold wrote:
>>
>>> Bryan, if you could consider my review feedback and respin your last
>>> three patches on top of my timeout patch, I think we should be good.
>>
>> Sure thing
> 
> Bryan, can I get an Acked-by: or Reviewed-by: for this patch so I can
> take it?
> 
> thanks,
> 
> greg k-h
> 

Yes, sorry Greg, busy flying.

I'll get to this by COB Wednesday GMT.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: greybus: loopback: fix broken udelay

[Bryan O'Donoghue]

On 26/01/17 11:37, Johan Hovold wrote:
> The loopback driver allows the user to set a minimum delay of up to one
> second to be inserted between test iterations (i.e. request
> submissions). The delay is currently specified in microseconds and is
> implemented using udelay.
> 
> Busy looping for long periods is not just anti-social; udelay must not
> be used for delays longer than a few milliseconds due to the risk of
> integer overflow.
> 
> Replace the broken udelay with a usleep_range with a 100 us range for
> short delays (< 20 ms) and otherwise revert to using msleep.
> 
> Fixes: b36f04fa9417 ("greybus: loopback: Convert thread delay to
> microseconds")
> Signed-off-by: Johan Hovold 
> ---
>  drivers/staging/greybus/loopback.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/greybus/loopback.c 
> b/drivers/staging/greybus/loopback.c
> index 6c2a41c638c3..a8329daf1e57 100644
> --- a/drivers/staging/greybus/loopback.c
> +++ b/drivers/staging/greybus/loopback.c
> @@ -1061,8 +1061,13 @@ static int gb_loopback_fn(void *data)
>   gb_loopback_calculate_stats(gb, !!error);
>   }
>   gb->send_count++;
> - if (us_wait)
> - udelay(us_wait);
> +
> + if (us_wait) {
> + if (us_wait < 2)
> + usleep_range(us_wait, us_wait + 100);
> + else
> + msleep(us_wait / 1000);
> + }
>   }
>  
>   gb_pm_runtime_put_autosuspend(bundle);
> 

Makes sense to me, thanks.

Acked-by: Bryan O'Donoghue 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 60/60] staging: lustre: libcfs: fix minimum size check for libcfs ioctl

[Dan Carpenter]

On Mon, Jan 30, 2017 at 01:51:56PM +0300, Dan Carpenter wrote:
> The lstcon_ioctl_entry() function doesn't have enough size checking.

Actually, the lstcon_ioctl_entry() would have been fine before we apply
this [patch 60/60]...  As near as I can tell, no in kernel code is
negatively affected by the bug this patch fixes.

regards,
dan carpenter


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

[patch] staging: lustre: libcfs: double copy bug

[Dan Carpenter]

The problem is that we copy hdr.ioc_len, we verify it, then we copy it
again without checking to see if it has changed in between the two
copies.

This could result in an information leak.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c 
b/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c
index 3f5d58babc2f..075826bd3a2a 100644
--- a/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c
+++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c
@@ -122,7 +122,7 @@ int libcfs_ioctl_getdata(struct libcfs_ioctl_hdr **hdr_pp,
 const struct libcfs_ioctl_hdr __user *uhdr)
 {
struct libcfs_ioctl_hdr hdr;
-   int err = 0;
+   int err;
 
if (copy_from_user(, uhdr, sizeof(hdr)))
return -EFAULT;
@@ -150,9 +150,20 @@ int libcfs_ioctl_getdata(struct libcfs_ioctl_hdr **hdr_pp,
return -ENOMEM;
 
if (copy_from_user(*hdr_pp, uhdr, hdr.ioc_len)) {
-   LIBCFS_FREE(*hdr_pp, hdr.ioc_len);
err = -EFAULT;
+   goto free;
}
+
+   if ((*hdr_pp)->ioc_version != hdr.ioc_version ||
+   (*hdr_pp)->ioc_len != hdr.ioc_len) {
+   err = -EINVAL;
+   goto free;
+   }
+
+   return 0;
+
+free:
+   LIBCFS_FREE(*hdr_pp, hdr.ioc_len);
return err;
 }
 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 60/60] staging: lustre: libcfs: fix minimum size check for libcfs ioctl

[Dan Carpenter]

It looks like what happened is there were two patches applied out of
sync.  Let's add a fixes tag and CC the original author.

Fixes: ed2f549dc0f6 ("staging: lustre: libcfs: test if userland data is to 
small")

This patch was probably correct when it was written but commit
1290932728e5 ("staging: lustre: Dynamic LNet Configuration (DLC) IOCTL
changes") ended up getting applied first so the size was wrong.

The lstcon_ioctl_entry() function doesn't have enough size checking.
Also I'm uncomfortable with:

data = container_of(hdr, struct libcfs_ioctl_data, ioc_hdr);

If hdr isn't the first member of the struct then the code is broken but
container_of() implies that that isn't a hard requirement.  It should
just be:

data = (struct libcfs_ioctl_data *)hdr;

regards,
dan carpenter


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

Re: [PATCH 1/1] iio: fixed symbolic permission references 'S_IRUGO | S_IWUSR'

[Hartmut Knaack]

Artur Lorincz schrieb am 30.01.2017 um 11:10:
> Replaced the symbolic permission references S_IRUGO and S_IWUSR with their
> octal counterparts.

Hi Artur,
patches concerning IIO should be sent to the linux-iio mailing list, and
former maintainers of the file should be CC'ed.
Now, regarding your patch, I am not convinced that this is an improvement.
Those symbols are usually used to improve readability, which is not given
with more or less magical numbers.
Thanks,

Hartmut

> 
> Signed-off-by: Artur Lorincz 
> ---
>  drivers/staging/iio/adc/ad7192.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7192.c 
> b/drivers/staging/iio/adc/ad7192.c
> index 1fb68c0..4fc8588 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -342,9 +342,9 @@ static int ad7192_setup(struct ad7192_state *st,
> 
>  static IIO_DEVICE_ATTR_NAMED(in_v_m_v_scale_available,
>in_voltage-voltage_scale_available,
> -  S_IRUGO, ad7192_show_scale_available, NULL, 0);
> +  0444, ad7192_show_scale_available, NULL, 0);
> 
> -static IIO_DEVICE_ATTR(in_voltage_scale_available, S_IRUGO,
> +static IIO_DEVICE_ATTR(in_voltage_scale_available, 0444,
>  ad7192_show_scale_available, NULL, 0);
> 
>  static ssize_t ad7192_show_ac_excitation(struct device *dev,
> @@ -412,11 +412,11 @@ static ssize_t ad7192_set(struct device *dev,
>   return ret ? ret : len;
>  }
> 
> -static IIO_DEVICE_ATTR(bridge_switch_en, S_IRUGO | S_IWUSR,
> +static IIO_DEVICE_ATTR(bridge_switch_en, 0644,
>  ad7192_show_bridge_switch, ad7192_set,
>  AD7192_REG_GPOCON);
> 
> -static IIO_DEVICE_ATTR(ac_excitation_en, S_IRUGO | S_IWUSR,
> +static IIO_DEVICE_ATTR(ac_excitation_en, 0644,
>  ad7192_show_ac_excitation, ad7192_set,
>  AD7192_REG_MODE);
> 
> --
> 1.9.1
> 
> 



0xFAC89148.asc
Description: application/pgp-keys


signature.asc
Description: OpenPGP digital signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 2/2] hv_utils: implement Hyper-V PTP source

[Vitaly Kuznetsov]

KY Srinivasan  writes:

>> -Original Message-
>> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
>> Sent: Thursday, January 19, 2017 6:17 AM
>> To: de...@linuxdriverproject.org
>> Cc: Thomas Gleixner ; linux-ker...@vger.kernel.org;
>> Haiyang Zhang ; KY Srinivasan
>> ; John Stultz ; Alex Ng (LIS)
>> ; Stephen Hemminger
>> ; Olaf Hering ; Richard
>> Cochran ; Radim Krcmar
>> 
>> Subject: [PATCH v4 2/2] hv_utils: implement Hyper-V PTP source
>> 
>> 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 server runs in parallel and uses something else for time
>>   sync (network, PTP,...) system time will never converge.
>> - Systemd starts annoying you by printing "Time has been changed" every 5
>>   seconds to the system log.
>> 
>> Instead of doing in-kernel time adjustments offload the work to an
>> NTP client by exposing TimeSync messages as a PTP device. Users may now
>> decide what they want to use as a source.
>> 
>> I tested the solution with chrony, the config was:
>> 
>>  refclock PHC /dev/ptp0 poll 3 precision 1e-9
>> 
>> The result I'm seeing is accurate enough, the time delta between the guest
>> and the host is almost always within [-10us, +10us], the in-kernel solution
>> was giving us comparable results.
>> 
>> I also tried implementing PPS device instead of PTP by using not currently
>> used Hyper-V synthetic timers (we use only one of four for clockevent) but
>> with PPS source only chrony wasn't able to give me the required accuracy,
>> the delta often more that 100us.
>> 
>> Signed-off-by: Vitaly Kuznetsov 
>
> Vitaly,
>
> This patch does not apply. Please rebase and send.
>

Sure, I'm currently testing v5 which includes the .getcrosststamp()
callback support. Will be sending it out shortly.

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