Re: [PATCH] Staging: greybus: gpio.c - style fix
On Sun, Jan 22, 2017 at 05:35:17PM +1300, Derek Robson wrote: > Fixed bare use of 'unsigned', Found using checkpatch > > Signed-off-by: Derek Robson > --- > drivers/staging/greybus/gpio.c | 22 +++--- > 1 file changed, 11 insertions(+), 11 deletions(-) This patch doesn't apply to my tree at all, are you sure you made it against linux-next? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 1/7] staging: vchiq_arm: Add compat ioctl for create service
I'm sorry but I really hate this again... :/
On Sat, Jan 21, 2017 at 09:48:10AM -0800, Michael Zoran wrote:
> This change is the first in a set of changes to add compat ioctls for
> vc04_services.
> In the change set, each ioctl modifed is pulled into a compat and native
> specific
> wrapper that calls a platform neutral handler function.
>
> At this time only the ioctls needed for compat are handled, but
> a general pattern is developed that can be used for cleaning up
> the other ioctls.
>
> This change contains the general framework and the ioctl handler for
> the create service ioctl.
>
> Signed-off-by: Michael Zoran
> ---
> .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 360
> -
> 1 file changed, 276 insertions(+), 84 deletions(-)
>
> 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 1dc8627e65b0..a5f5d5b6f938 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -47,6 +47,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include "vchiq_core.h"
> @@ -506,6 +507,254 @@ vchiq_ioc_queue_message(VCHIQ_SERVICE_HANDLE_T handle,
> &context, total_size);
> }
>
> +struct vchiq_ioctl_ctxt;
> +typedef long (*vchiq_ioctl_cpyret_handler_t)(struct vchiq_ioctl_ctxt *ctxt);
> +
> +/*
> + * struct vchiq_ioctl_ctxt
> + *
> + * Holds context information across a single ioctl call from user mode.
> + * This structure is used to reduce the number of parameters passed
> + * to each of the handler functions to process the ioctl.
> + */
> +
> +struct vchiq_ioctl_ctxt {
> + struct file *file;
> + unsigned int cmd;
> + unsigned long arg;
> + void *args;
> + VCHIQ_INSTANCE_T instance;
> + VCHIQ_STATUS_T status;
> + VCHIQ_SERVICE_T *service;
> + vchiq_ioctl_cpyret_handler_t cpyret_handler;
> +};
> +
This doesn't simplify anything at all... It just hides the parameters
and makes things more complicated on the caller and the implementor.
Also "args" is now a void pointer which makes me very sad. :(
> +typedef long (*vchiq_ioctl_handler_t)(struct vchiq_ioctl_ctxt *ctxt);
> +
> +static long
> +vchiq_map_status(VCHIQ_STATUS_T status)
> +{
> + switch (status) {
> + case VCHIQ_SUCCESS:
> + return 0;
> + case VCHIQ_ERROR:
> + return -EIO;
> + case VCHIQ_RETRY:
> + return -EINTR;
> + default:
> + return -EIO;
> + }
> +}
I don't really like this either... We should be getting rid of these
custom error codes not formalizing them. :(
> +
> +static long
> +vchiq_dispatch_ioctl(vchiq_ioctl_handler_t handler,
> + struct file *file, unsigned int cmd, unsigned long arg) {
> + struct vchiq_ioctl_ctxt ctxt;
> + long ret = 0;
> +
> + ctxt.file = file;
> + ctxt.cmd= cmd;
> + ctxt.arg= arg;
> + ctxt.args = NULL;
> + ctxt.instance = file->private_data;
> + ctxt.status = VCHIQ_SUCCESS;
> + ctxt.service= NULL;
> + ctxt.cpyret_handler = NULL;
> +
> + vchiq_log_trace(vchiq_arm_log_level,
> + "vchiq_ioctl - instance %pK, cmd %s, arg %lx",
> + ctxt.instance,
> + ioctl_names[_IOC_NR(cmd)],
> + arg);
> +
> + ret = handler(&ctxt);
> +
> + if (ctxt.service)
> + unlock_service(ctxt.service);
> +
> + if (ret < 0)
> + vchiq_log_info(vchiq_arm_log_level,
> +" ioctl instance %lx, cmd %s -> status %d, %ld",
> +(unsigned long)ctxt.instance,
> +ioctl_names[_IOC_NR(cmd)],
> +ctxt.status, ret);
> + else
> + vchiq_log_trace(vchiq_arm_log_level,
> + " ioctl instance %lx, cmd %s -> status %d,
> %ld",
> + (unsigned long)ctxt.instance,
> + ioctl_names[_IOC_NR(cmd)],
> + ctxt.status, ret);
> +
> + return ret;
I don't see the point of any of this debugging. Just leave it out. Use
ftrace.
> +}
> +
> +static long
> +vchiq_ioctl_do_create_service(struct vchiq_ioctl_ctxt *ctxt)
Finally, we have reached he meat of this patch. Please, pull this
bit out in patch #1 like I asked. Then in the next patch implement
the compat ioctl.
I don't like the cpyret function pointers either...
This stuff should really be very simple. I feel like you're engineering
it to be way harder than it needs to be. I'm sorry. :(
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproj
Re: [PATCH v3 00/24] i.MX Media Driver
On Fri, 2017-01-20 at 21:39 +0100, Hans Verkuil wrote:
[...]
> > There is a VDIC entity in the i.MX IPU that performs de-interlacing with
> > hardware filters for motion compensation. Some of the motion compensation
> > modes ("low" and "medium" motion) require that the VDIC receive video
> > frame fields from memory buffers (dedicated dma channels in the
> > IPU are used to transfer those buffers into the VDIC).
> >
> > So one option to support those modes would be to pass the raw buffers
> > from a camera sensor up to userspace to a capture device, and then pass
> > them back to the VDIC for de-interlacing using a mem2mem device.
> >
> > Philipp and I are both in agreement that, since userland is not interested
> > in the intermediate interlaced buffers in this case, but only the final
> > result (motion compensated, de-interlaced frames), it is more efficient
> > to provide a media link that allows passing those intermediate frames
> > directly from a camera source pad to VDIC sink pad, without having
> > to route them through userspace.
> >
> > So in order to support that, I've implemented a simple FIFO dma buffer
> > queue in the driver to allow passing video buffers directly from a source
> > to a sink. It is modeled loosely off the vb2 state machine and API, but
> > simpler (for instance it only allows contiguous, cache-coherent buffers).
> >
> > This is where Philipp has an argument, that this should be done with a
> > new API in videobuf2.
That is one part of the argument. I'm glad to understand now that we
agree about this.
> > And I'm actually in total agreement with that. I definitely agree that there
> > should be a mechanism in the media framework that allows passing video
> > buffers from a source pad to a sink pad using a software queue, with no
> > involvement from userland.
That is the other part of the argument. I do not agree that these
software queue "links" should be presented to userspace as media pad
links between two entities of a media device.
First, that would limit the links to subdevices contained in the same
media graph, while this should work between any two capture and output
queues of different devices.
Assume for example, we want to encode the captured, deinterlaced video
to h.264 with the coda VPU driver. A software queue link could be
established between the CSI capture and the VDIC deinterlacer input,
just as between the VDIC deinterlacer output and the coda VPU input.
Technically, there would be no difference between those two linked
capture/output queue pairs. But the coda driver is a completely separate
mem2mem device. And since it is not part of the i.MX media graph, there
is no entity pad to link to.
Or assume there is an USB analog capture device that produces interlaced
frames. I think it should be possible to connect its capture queue to
the VDIC deinterlacer output queue just the same way as linking the CSI
to the VDIC (in software queue mode).
Second, the subdevice pad formats describe wire formats, not memory
formats. The user might want to choose between 4:2:2 and 4:2:0
subsampled YUV formats for the intermediate buffer, for example,
depending on memory bandwidth constraints and quality requirements. This
is impossible with the media entity / subdevice pad links.
I think an interface where userspace configures the capture and output
queues via v4l2 API, passes dma buffers around from one to the other
queue, and then puts both queues into a free running mode would be a
much better fit for this mechanism.
> > My only disagreement is when this should be implemented. I think it is
> > fine to keep my custom implementation of this in the driver for now. Once
> > an extension of vb2 is ready to support this feature, it would be fairly
> > straightforward to strip out my custom implementation and go with the
> > new API.
>
> For a staging driver this isn't necessary, as long as it is documented in
> the TODO file that this needs to be fixed before it can be moved out of
> staging. The whole point of staging is that there is still work to be
> done in the driver, after all :-)
Absolutely. The reason I am arguing against merging the mem2mem media
control links so vehemently is that I am convinced the userspace
interface is wrong, and I am afraid that even though in staging, it
might become established.
regards
Philipp
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 00/24] i.MX Media Driver
On 01/23/2017 12:00 PM, Philipp Zabel wrote:
> On Fri, 2017-01-20 at 21:39 +0100, Hans Verkuil wrote:
> [...]
>>> There is a VDIC entity in the i.MX IPU that performs de-interlacing with
>>> hardware filters for motion compensation. Some of the motion compensation
>>> modes ("low" and "medium" motion) require that the VDIC receive video
>>> frame fields from memory buffers (dedicated dma channels in the
>>> IPU are used to transfer those buffers into the VDIC).
>>>
>>> So one option to support those modes would be to pass the raw buffers
>>> from a camera sensor up to userspace to a capture device, and then pass
>>> them back to the VDIC for de-interlacing using a mem2mem device.
>>>
>>> Philipp and I are both in agreement that, since userland is not interested
>>> in the intermediate interlaced buffers in this case, but only the final
>>> result (motion compensated, de-interlaced frames), it is more efficient
>>> to provide a media link that allows passing those intermediate frames
>>> directly from a camera source pad to VDIC sink pad, without having
>>> to route them through userspace.
>>>
>>> So in order to support that, I've implemented a simple FIFO dma buffer
>>> queue in the driver to allow passing video buffers directly from a source
>>> to a sink. It is modeled loosely off the vb2 state machine and API, but
>>> simpler (for instance it only allows contiguous, cache-coherent buffers).
>>>
>>> This is where Philipp has an argument, that this should be done with a
>>> new API in videobuf2.
>
> That is one part of the argument. I'm glad to understand now that we
> agree about this.
>
>>> And I'm actually in total agreement with that. I definitely agree that there
>>> should be a mechanism in the media framework that allows passing video
>>> buffers from a source pad to a sink pad using a software queue, with no
>>> involvement from userland.
>
> That is the other part of the argument. I do not agree that these
> software queue "links" should be presented to userspace as media pad
> links between two entities of a media device.
>
> First, that would limit the links to subdevices contained in the same
> media graph, while this should work between any two capture and output
> queues of different devices.
> Assume for example, we want to encode the captured, deinterlaced video
> to h.264 with the coda VPU driver. A software queue link could be
> established between the CSI capture and the VDIC deinterlacer input,
> just as between the VDIC deinterlacer output and the coda VPU input.
> Technically, there would be no difference between those two linked
> capture/output queue pairs. But the coda driver is a completely separate
> mem2mem device. And since it is not part of the i.MX media graph, there
> is no entity pad to link to.
> Or assume there is an USB analog capture device that produces interlaced
> frames. I think it should be possible to connect its capture queue to
> the VDIC deinterlacer output queue just the same way as linking the CSI
> to the VDIC (in software queue mode).
>
> Second, the subdevice pad formats describe wire formats, not memory
> formats. The user might want to choose between 4:2:2 and 4:2:0
> subsampled YUV formats for the intermediate buffer, for example,
> depending on memory bandwidth constraints and quality requirements. This
> is impossible with the media entity / subdevice pad links.
>
> I think an interface where userspace configures the capture and output
> queues via v4l2 API, passes dma buffers around from one to the other
> queue, and then puts both queues into a free running mode would be a
> much better fit for this mechanism.
>
>>> My only disagreement is when this should be implemented. I think it is
>>> fine to keep my custom implementation of this in the driver for now. Once
>>> an extension of vb2 is ready to support this feature, it would be fairly
>>> straightforward to strip out my custom implementation and go with the
>>> new API.
>>
>> For a staging driver this isn't necessary, as long as it is documented in
>> the TODO file that this needs to be fixed before it can be moved out of
>> staging. The whole point of staging is that there is still work to be
>> done in the driver, after all :-)
>
> Absolutely. The reason I am arguing against merging the mem2mem media
> control links so vehemently is that I am convinced the userspace
> interface is wrong, and I am afraid that even though in staging, it
> might become established.
As long as it is mentioned in the TODO, and ideally in the Kconfig as well,
then I'm fine with it.
The big advantage of being in the kernel is that it is much easier to start
providing fixes, improvements, etc. If you use a staging driver you know
that there is no guarantee whatsoever with respect to stable ABI/APIs.
Regards,
Hans
___
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
Hi Steve,
On Sun, 2017-01-22 at 18:31 -0800, Steve Longerbeam wrote:
>
> On 01/16/2017 05:47 AM, Philipp Zabel wrote:
> > On Sat, 2017-01-14 at 14:46 -0800, Steve Longerbeam wrote:
> > [...]
> +Unprocessed Video Capture:
> +--
> +
> +Send frames directly from sensor to camera interface, with no
> +conversions:
> +
> +-> ipu_smfc -> camif
> >>> I'd call this capture interface, this is not just for cameras. Or maybe
> >>> idmac if you want to mirror hardware names?
> >> Camif is so named because it is the V4L2 user interface for video
> >> capture. I suppose it could be named "capif", but that doesn't role
> >> off the tongue quite as well.
> > Agreed, capif sounds weird. I find camif a bit confusing though, because
> > Samsung S3C has a camera interface that is actually called "CAMIF".
>
> how about simply "capture" ?
That sounds good to me.
[...]
> > Chapter 37.4.2.3 "FCW & FCR - Format converter write and read" in the
> > IDMAC chapter states that all internal submodules only work on 8-bit per
> > component formats with four components: YUVA or RGBA.
>
> Right, the "direct" IPU internal (that do not transfer buffers to/from
> memory via IDMAC channels) should only allow the IPU internal
> formats YUVA and RGBA. I get you now.
>
> The "direct" pads now only accept MEDIA_BUS_FMT_AYUV8_1X32 and
> MEDIA_BUS_FMT_ARGB_1X32.
>
> Those pads are:
>
> ipu_csi:1
> ipu_vdic:1
> ipu_ic_prp:0
> ipu_ic_prp:1
> ipu_ic_prpenc:0
> ipu_ic_prpenc:1
> ipu_ic_prpvf:0
> ipu_ic_prpvf:1
Yes, that is what I meant. The csi:1 can then be extended to support
additional expanded/packed/raw formats for the SMFC->memory path.
> >> There does not appear to be support in the FSU for linking a write channel
> >> to the VDIC read channels (8, 9, 10) according to VDI_SRC_SEL field. There
> >> is support for the direct link from CSI (which I am using), but that's
> >> not an
> >> IDMAC channel link.
> >>
> >> There is a PRP_SRC_SEL field, with linking from IDMAC (SMFC) channels
> >> 0..2 (and 3? it's not clear, and not clear whether this includes channel
> >> 1).
> > As I read it, that is 0 and 2 only, no idea why. But since there are
> > only 2 CSIs, that shouldn't be a problem.
>
> ipu_csi1 is now transferring on IDMAC/SMFC channel 2 (ipu_csi0 still
> at channel 0).
Ok.
> +static const u32 power_off_seq[] = {
> +IMX_MEDIA_GRP_ID_IC_PP,
> +IMX_MEDIA_GRP_ID_IC_PRPVF,
> +IMX_MEDIA_GRP_ID_IC_PRPENC,
> +IMX_MEDIA_GRP_ID_SMFC,
> +IMX_MEDIA_GRP_ID_CSI,
> +IMX_MEDIA_GRP_ID_VIDMUX,
> +IMX_MEDIA_GRP_ID_SENSOR,
> +IMX_MEDIA_GRP_ID_CSI2,
> +};
> >>> This seems somewhat arbitrary. Why is a power sequence needed?
> >> The CSI-2 receiver must be powered up before the sensor, that's the
> >> only requirement IIRC. The others have no s_power requirement. So I
> >> can probably change this to power up in the frontend -> backend order
> >> (IC_PP to sensor). And vice-versa for power off.
> > Yes, I think that should work (see below).
>
> Actually there are problems using this, see below.
[...]
> +EXPORT_SYMBOL_GPL(imx_media_pipeline_set_power);
> >>> This should really be handled by v4l2_pipeline_pm_use.
> >> I thought about this earlier, but v4l2_pipeline_pm_use() seems to be
> >> doing some other stuff that bothered me, at least that's what I remember.
> >> I will revisit this.
> > I have used it with a tc358743 -> mipi-csi2 pipeline, it didn't cause
> > any problems. It would be better to reuse and, if necessary, fix the
> > existing infrastructure where available.
>
> I tried this API, by switching to v4l2_pipeline_pm_use() in camif
> open/release,
> and switched to v4l2_pipeline_link_notify() instead of
> imx_media_link_notify()
> in the media driver's media_device_ops.
>
> This API assumes the video device has an open file handle while the media
> links are being established. This doesn't work for me, I want to be able to
> establish the links using 'media-ctl -l', and that won't work unless
> there is an
> open file handle on the video capture device node.
>
> Also, I looked into calling v4l2_pipeline_pm_use() during
> imx_media_link_notify(),
> instead of imx_media_pipeline_set_power(). Again there are problems with
> that.
>
> First, v4l2_pipeline_pm_use() acquires the graph mutex, so it can't be
> called inside
> link_notify which already acquires that lock. The header for this
> function also
> clearly states it should only be called in open/release.
So why not call it in open/release then?
> 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 call
Re: [PATCH V2 1/7] staging: vchiq_arm: Add compat ioctl for create service
I'm attaching how I would basically like to see patch 1/7 done. It's not perfect, I did it as quickly as possible and haven't tested anything. I would normally be more careful if I were sending this for inclusion so there are no sign offs etc. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[1] staging: vchiq_arm: pull create_service() into it's own function
Pull vchiq_ioctl_create_service() into its own function. I did this
as simply as I could and left checkpatch.pl warnings in if they were in
the original code.
There is a slight functional change in that we skip some debugging but
we should be using ftrace for that and get rid of the debug code anyway.
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 cb0b7ca36b1e..f9f69ca501b2 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -506,6 +506,91 @@ vchiq_ioc_queue_message(VCHIQ_SERVICE_HANDLE_T handle,
&context, total_size);
}
+static int vchiq_ioctl_create_service(struct file *file, unsigned int cmd,
unsigned long arg)
+{
+ VCHIQ_INSTANCE_T instance = file->private_data;
+ VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
+ VCHIQ_SERVICE_T *service = NULL;
+ VCHIQ_CREATE_SERVICE_T args;
+ USER_SERVICE_T *user_service = NULL;
+ void *userdata;
+ int srvstate;
+ int ret = 0;
+
+ if (copy_from_user(&args, (const void __user *)arg,
+ sizeof(args)) != 0) {
+ return -EFAULT;
+ }
+
+ user_service = kmalloc(sizeof(USER_SERVICE_T), GFP_KERNEL);
+ if (!user_service)
+ return -ENOMEM;
+
+ if (args.is_open) {
+ if (!instance->connected) {
+ kfree(user_service);
+ return -ENOTCONN;
+ }
+ srvstate = VCHIQ_SRVSTATE_OPENING;
+ } else {
+ srvstate =
+instance->connected ?
+VCHIQ_SRVSTATE_LISTENING :
+VCHIQ_SRVSTATE_HIDDEN;
+ }
+
+ userdata = args.params.userdata;
+ args.params.callback = service_callback;
+ args.params.userdata = user_service;
+ service = vchiq_add_service_internal(
+ instance->state,
+ &args.params, srvstate,
+ instance, user_service_free);
+
+ if (service != NULL) {
+ user_service->service = service;
+ user_service->userdata = userdata;
+ user_service->instance = instance;
+ user_service->is_vchi = (args.is_vchi != 0);
+ user_service->dequeue_pending = 0;
+ user_service->close_pending = 0;
+ user_service->message_available_pos =
+ instance->completion_remove - 1;
+ user_service->msg_insert = 0;
+ user_service->msg_remove = 0;
+ sema_init(&user_service->insert_event, 0);
+ sema_init(&user_service->remove_event, 0);
+ sema_init(&user_service->close_event, 0);
+
+ if (args.is_open) {
+ status = vchiq_open_service_internal
+ (service, instance->pid);
+ if (status != VCHIQ_SUCCESS) {
+ vchiq_remove_service(service->handle);
+ service = NULL;
+ ret = (status == VCHIQ_RETRY) ?
+ -EINTR : -EIO;
+ return ret;
+ }
+ }
+
+ if (copy_to_user((void __user *)
+ &(((VCHIQ_CREATE_SERVICE_T __user *)
+ arg)->handle),
+ (const void *)&service->handle,
+ sizeof(service->handle)) != 0) {
+ ret = -EFAULT;
+ vchiq_remove_service(service->handle);
+ }
+
+ } else {
+ ret = -EEXIST;
+ kfree(user_service);
+ }
+
+ return ret;
+}
+
/
*
* vchiq_ioctl
@@ -576,90 +661,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
"vchiq: could not connect: %d", status);
break;
- case VCHIQ_IOC_CREATE_SERVICE: {
- VCHIQ_CREATE_SERVICE_T args;
- USER_SERVICE_T *user_service = NULL;
- void *userdata;
- int srvstate;
-
- if (copy_from_user
-(&args, (const void __user *)arg,
- sizeof(args)) != 0) {
- ret = -EFAULT;
- break;
- }
-
- user_service = kmalloc(sizeof(USER_SERVICE_T), GFP_KERNEL);
- if (!user_service) {
- ret = -ENOMEM;
- break;
- }
-
- if (args.is_open) {
- if (!instance->connected) {
-
[2] staging: vchiq_arm: cleanup vchiq_ioctl_create_service()
In my last patch, I moved code to vchiq_ioctl_create_service() but
without cleaning it up or fixing any checkpatch warnings. In this
patch I have cleaned the function up extensively. There are no
functional changes:
1) We don't need the "ret" variable and can return directly.
2) Remove some stray curly braces.
3) Use sizeof(*user_service) instead of sizeof(USER_SERVICE_T).
4) Remove line breaks that aren't required and add tabs here and there.
5) I flipped the "if (service != NULL) {" condition around and return
directly on error. That lets us remove more line breakas.
6) I removed a "service = NULL;" assignment that isn't needed any more
from before the return if vchiq_open_service_internal() fails.
It's several different kinds of cleanups but it's all local to one
function so I think it qualifies as "one thing per patch."
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 f9f69ca501b2..2cc43a724554 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -515,14 +515,11 @@ static int vchiq_ioctl_create_service(struct file *file,
unsigned int cmd, unsig
USER_SERVICE_T *user_service = NULL;
void *userdata;
int srvstate;
- int ret = 0;
- if (copy_from_user(&args, (const void __user *)arg,
- sizeof(args)) != 0) {
+ if (copy_from_user(&args, (const void __user *)arg, sizeof(args)))
return -EFAULT;
- }
- user_service = kmalloc(sizeof(USER_SERVICE_T), GFP_KERNEL);
+ user_service = kmalloc(sizeof(*user_service), GFP_KERNEL);
if (!user_service)
return -ENOMEM;
@@ -533,62 +530,52 @@ static int vchiq_ioctl_create_service(struct file *file,
unsigned int cmd, unsig
}
srvstate = VCHIQ_SRVSTATE_OPENING;
} else {
- srvstate =
-instance->connected ?
-VCHIQ_SRVSTATE_LISTENING :
-VCHIQ_SRVSTATE_HIDDEN;
+ srvstate = instance->connected ?
+ VCHIQ_SRVSTATE_LISTENING :
+ VCHIQ_SRVSTATE_HIDDEN;
}
userdata = args.params.userdata;
args.params.callback = service_callback;
args.params.userdata = user_service;
- service = vchiq_add_service_internal(
- instance->state,
- &args.params, srvstate,
- instance, user_service_free);
-
- if (service != NULL) {
- user_service->service = service;
- user_service->userdata = userdata;
- user_service->instance = instance;
- user_service->is_vchi = (args.is_vchi != 0);
- user_service->dequeue_pending = 0;
- user_service->close_pending = 0;
- user_service->message_available_pos =
- instance->completion_remove - 1;
- user_service->msg_insert = 0;
- user_service->msg_remove = 0;
- sema_init(&user_service->insert_event, 0);
- sema_init(&user_service->remove_event, 0);
- sema_init(&user_service->close_event, 0);
-
- if (args.is_open) {
- status = vchiq_open_service_internal
- (service, instance->pid);
- if (status != VCHIQ_SUCCESS) {
- vchiq_remove_service(service->handle);
- service = NULL;
- ret = (status == VCHIQ_RETRY) ?
- -EINTR : -EIO;
- return ret;
- }
- }
+ service = vchiq_add_service_internal(instance->state,
+&args.params, srvstate,
+instance, user_service_free);
+ if (!service) {
+ kfree(user_service);
+ return -EEXIST;
+ }
- if (copy_to_user((void __user *)
- &(((VCHIQ_CREATE_SERVICE_T __user *)
- arg)->handle),
- (const void *)&service->handle,
- sizeof(service->handle)) != 0) {
- ret = -EFAULT;
+ user_service->service = service;
+ user_service->userdata = userdata;
+ user_service->instance = instance;
+ user_service->is_vchi = (args.is_vchi != 0);
+ user_service->dequeue_pending = 0;
+ user_service->close_pending = 0;
+ user_service->message_available_pos = instance->completion_remove - 1;
+ user_service->msg_insert = 0;
+ user_service->msg_remove = 0;
+ sema_init(&user_service->inser
[3] staging: vchiq_arm: remove some ugly casting
Let's start treating "arg" as a user pointer instead of an unsigned
long earlier so we can remove some ugly casts.
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 2cc43a724554..5caf53942604 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -506,7 +506,8 @@ vchiq_ioc_queue_message(VCHIQ_SERVICE_HANDLE_T handle,
&context, total_size);
}
-static int vchiq_ioctl_create_service(struct file *file, unsigned int cmd,
unsigned long arg)
+static int vchiq_ioctl_create_service(struct file *file, unsigned int cmd,
+ VCHIQ_CREATE_SERVICE_T __user *uargs)
{
VCHIQ_INSTANCE_T instance = file->private_data;
VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
@@ -516,7 +517,7 @@ static int vchiq_ioctl_create_service(struct file *file,
unsigned int cmd, unsig
void *userdata;
int srvstate;
- if (copy_from_user(&args, (const void __user *)arg, sizeof(args)))
+ if (copy_from_user(&args, uargs, sizeof(args)))
return -EFAULT;
user_service = kmalloc(sizeof(*user_service), GFP_KERNEL);
@@ -568,9 +569,8 @@ static int vchiq_ioctl_create_service(struct file *file,
unsigned int cmd, unsig
}
}
- if (copy_to_user((void __user *)
-&(((VCHIQ_CREATE_SERVICE_T __user *)arg)->handle),
-&service->handle, sizeof(service->handle))) {
+ if (copy_to_user(&uargs->handle, &service->handle,
+sizeof(service->handle))) {
vchiq_remove_service(service->handle);
return -EFAULT;
}
@@ -649,7 +649,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
break;
case VCHIQ_IOC_CREATE_SERVICE:
- return vchiq_ioctl_create_service(file, cmd, arg);
+ return vchiq_ioctl_create_service(file, cmd,
+ (void __user *)arg);
case VCHIQ_IOC_CLOSE_SERVICE: {
VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[4] staging: vchiq_arm: make do_ioctl_create_service() a separate function
We want to implement a COMPAT version of this function so let's abstract
away all the copy_to/from_user bits.
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 5caf53942604..0ffe4bf8d826 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -506,25 +506,23 @@ vchiq_ioc_queue_message(VCHIQ_SERVICE_HANDLE_T handle,
&context, total_size);
}
-static int vchiq_ioctl_create_service(struct file *file, unsigned int cmd,
- VCHIQ_CREATE_SERVICE_T __user *uargs)
+static int
+do_ioctl_create_service(struct file *file,
+ VCHIQ_CREATE_SERVICE_T *args,
+ int __user *handle)
{
VCHIQ_INSTANCE_T instance = file->private_data;
VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
VCHIQ_SERVICE_T *service = NULL;
- VCHIQ_CREATE_SERVICE_T args;
USER_SERVICE_T *user_service = NULL;
void *userdata;
int srvstate;
- if (copy_from_user(&args, uargs, sizeof(args)))
- return -EFAULT;
-
user_service = kmalloc(sizeof(*user_service), GFP_KERNEL);
if (!user_service)
return -ENOMEM;
- if (args.is_open) {
+ if (args->is_open) {
if (!instance->connected) {
kfree(user_service);
return -ENOTCONN;
@@ -536,11 +534,11 @@ static int vchiq_ioctl_create_service(struct file *file,
unsigned int cmd,
VCHIQ_SRVSTATE_HIDDEN;
}
- userdata = args.params.userdata;
- args.params.callback = service_callback;
- args.params.userdata = user_service;
+ userdata = args->params.userdata;
+ args->params.callback = service_callback;
+ args->params.userdata = user_service;
service = vchiq_add_service_internal(instance->state,
-&args.params, srvstate,
+&args->params, srvstate,
instance, user_service_free);
if (!service) {
kfree(user_service);
@@ -550,7 +548,7 @@ static int vchiq_ioctl_create_service(struct file *file,
unsigned int cmd,
user_service->service = service;
user_service->userdata = userdata;
user_service->instance = instance;
- user_service->is_vchi = (args.is_vchi != 0);
+ user_service->is_vchi = (args->is_vchi != 0);
user_service->dequeue_pending = 0;
user_service->close_pending = 0;
user_service->message_available_pos = instance->completion_remove - 1;
@@ -559,7 +557,7 @@ static int vchiq_ioctl_create_service(struct file *file,
unsigned int cmd,
sema_init(&user_service->insert_event, 0);
sema_init(&user_service->remove_event, 0);
sema_init(&user_service->close_event, 0);
- if (args.is_open) {
+ if (args->is_open) {
status = vchiq_open_service_internal(service, instance->pid);
if (status != VCHIQ_SUCCESS) {
vchiq_remove_service(service->handle);
@@ -569,8 +567,7 @@ static int vchiq_ioctl_create_service(struct file *file,
unsigned int cmd,
}
}
- if (copy_to_user(&uargs->handle, &service->handle,
-sizeof(service->handle))) {
+ if (copy_to_user(handle, &service->handle, sizeof(service->handle))) {
vchiq_remove_service(service->handle);
return -EFAULT;
}
@@ -578,6 +575,18 @@ static int vchiq_ioctl_create_service(struct file *file,
unsigned int cmd,
return 0;
}
+static int
+vchiq_ioctl_create_service(struct file *file, unsigned int cmd,
+ VCHIQ_CREATE_SERVICE_T __user *uargs)
+{
+ VCHIQ_CREATE_SERVICE_T args;
+
+ if (copy_from_user(&args, uargs, sizeof(args)))
+ return -EFAULT;
+
+ return do_ioctl_create_service(file, &args, &uargs->handle);
+}
+
/
*
* vchiq_ioctl
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[5] staging: vchiq_arm: add compat create_service ioctl
This implements the compat IOCTL for VCHIQ_IOC_CREATE_SERVICE32
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 0ffe4bf8d826..6ea33cdf3976 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -47,6 +47,7 @@
#include
#include
#include
+#include
#include
#include "vchiq_core.h"
@@ -587,6 +588,53 @@ vchiq_ioctl_create_service(struct file *file, unsigned int
cmd,
return do_ioctl_create_service(file, &args, &uargs->handle);
}
+#if defined(CONFIG_COMPAT)
+struct vchiq_service_base32 {
+ int fourcc;
+ compat_uptr_t callback;
+ compat_uptr_t userdata;
+};
+
+struct vchiq_service_params32 {
+ int fourcc;
+ compat_uptr_t callback;
+ compat_uptr_t userdata;
+ short version; /* Increment for non-trivial changes */
+ short version_min; /* Update for incompatible changes */
+};
+
+struct vchiq_create_service32 {
+ struct vchiq_service_params32 params;
+ int is_open;
+ int is_vchi;
+ unsigned int handle;/* OUT */
+};
+
+#define VCHIQ_IOC_CREATE_SERVICE32 \
+ _IOWR(VCHIQ_IOC_MAGIC, 2, struct vchiq_create_service32)
+
+static int
+vchiq_ioctl_create_service_compat(struct file *file, unsigned int cmd,
+ struct vchiq_create_service32 *uargs)
+{
+ struct vchiq_create_service32 args32;
+ VCHIQ_CREATE_SERVICE_T args;
+
+ if (copy_from_user(&args32, uargs, sizeof(args32)))
+ return -EFAULT;
+
+ args.params.fourcc = args32.params.fourcc;
+ args.params.callback= compat_ptr(args32.params.callback);
+ args.params.userdata= compat_ptr(args32.params.userdata);
+ args.params.version = args32.params.version;
+ args.params.version_min = args32.params.version_min;
+ args.is_open= args32.is_open;
+ args.is_vchi= args32.is_vchi;
+
+ return do_ioctl_create_service(file, &args, &uargs->handle);
+}
+#endif
+
/
*
* vchiq_ioctl
@@ -1218,6 +1266,20 @@ vchiq_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
return ret;
}
+#if defined(CONFIG_COMPAT)
+static long
+vchiq_ioctl_compat(struct file *file, unsigned int cmd, unsigned long arg)
+{
+ switch (cmd) {
+ case VCHIQ_IOC_CREATE_SERVICE32:
+ return vchiq_ioctl_create_service_compat(file, cmd,
+(void __user *)arg);
+ default:
+ return vchiq_ioctl(file, cmd, arg);
+ }
+}
+#endif
+
/
*
* vchiq_open
@@ -1672,6 +1734,9 @@ static const struct file_operations
vchiq_fops = {
.owner = THIS_MODULE,
.unlocked_ioctl = vchiq_ioctl,
+#if defined(CONFIG_COMPAT)
+ .compat_ioctl = vchiq_ioctl_compat,
+#endif
.open = vchiq_open,
.release = vchiq_release,
.read = vchiq_read
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 1/4] staging: greybus: operation: add generic asynchronous timeout operation support
On Wed, Jan 04, 2017 at 12:11:18AM +, Bryan O'Donoghue wrote: > This patch adds a generic mechanism for handling timeouts of asynchronous > operations to operation.c. After doing a gb_operation_request_send() it > schedules a delayed worker. When the delayed worker's timer expires the > worker runs and does a gb_operation_cancel(). A gb_operation_cancel() > operation will result in the operation completion callback being called > with -ETIMEDOUT as the result code. If an operation completion has already > taken place then the gb_operation_cancel() has no effect. This patch means > that each driver doing asynchronous operations doesn't have to have its own > special timeout mechanisms, it just waits for operation.c to provide it > with the appropriate status code for a given operation. So I finally found some time to dig into this. I've had some doubts about using delayed work this way since it was first suggested, even if this would still be an improvement over the custom timeout implementation in loopback of course. The idea all along was to generalise the timeout handling in core, but this had to be pushed out for v2 due to too many other things requiring attention. Expiring timers is more expensive than cancelling them, something which would be the common case. By throwing in a workqueue in the mix, this penalty for every (async) operation becomes even more costly. I've implemented generic timeout support in core using just a per-operation timer instead. This simplifies the synchronous operation handling somewhat, but specifically provides an efficient timeout mechanism for driver to use for asynchronous operations as well. The rest of your series can be rebased on top with minimal effort. Thanks, Johan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 3/4] staging: greybus: loopback: convert loopback to use generic async operations
On Wed, Jan 04, 2017 at 12:11:20AM +, Bryan O'Donoghue wrote:
> Loopback has its own internal method for tracking and timing out
> asynchronous operations however previous patches make it possible to use
> functionality provided by operation.c to do this instead. Using the code in
> operation.c means we can completely subtract the timer, the work-queue, the
> kref and the cringe-worthy 'pending' flag. The completion callback
> triggered by operation.c will provide an authoritative result code -
> including -ETIMEDOUT for asynchronous operations. Tested to ensure the
> existing stats given by loopback for synchronous, asynchronous and
> asynchronous+timeout still hold.
Good that you verified and mention that here, but not sure adding all
the numbers below is needed. What test setup are you using by the way?
Also note that this one no longer applies due to a trivial clean up
patch in staging-next.
A couple of comments below.
> Before:
> gb_loopback_test -i 1000 -s 128 -t sink -p
> 1970-1-1 7:30:8
> test:sink
> path:gb_loopback0
> size:128
> iterations: 1000
> errors: 0
> async: Disabled
> requests per-sec:min=432, max=436, average=434.662994, jitter=4
> ap-throughput B/s: min=67535 max=68036 average=67807.421875 jitter=501
> ap-latency usec: min=1626 max=3559 average=2287.077881 jitter=1933
> apbridge-latency usec: min=0 max=0 average=0.00 jitter=0
> gbphy-latency usec: min=0 max=0 average=0.00 jitter=0
[...]
> Signed-off-by: Bryan O'Donoghue
> Signed-off-by: Mitchell Tasman
> ---
> drivers/staging/greybus/loopback.c | 167
> +++--
> 1 file changed, 32 insertions(+), 135 deletions(-)
> @@ -615,32 +513,29 @@ static int gb_loopback_async_operation(struct
> gb_loopback *gb, int type,
> if (request_size)
> memcpy(operation->request->payload, request, request_size);
>
> + gb_operation_set_data(operation, op_async);
> +
> op_async->gb = gb;
> op_async->operation = operation;
> op_async->completion = completion;
>
> - spin_lock_irqsave(&gb_dev.lock, flags);
> - list_add_tail(&op_async->entry, &gb_dev.list_op_async);
> - spin_unlock_irqrestore(&gb_dev.lock, flags);
> -
> do_gettimeofday(&op_async->ts);
> - op_async->pending = true;
> +
> atomic_inc(&gb->outstanding_operations);
> +
> mutex_lock(&gb->mutex);
Do you still need this lock?
> - ret = gb_operation_request_send(operation,
> + ret = gb_operation_request_send_timeout(operation,
> + jiffies_to_msecs(gb->jiffy_timeout),
> gb_loopback_async_operation_callback,
> GFP_KERNEL);
> if (ret)
> goto error;
>
> - setup_timer(&op_async->timer, gb_loopback_async_operation_timeout,
> - (unsigned long)operation->id);
> - op_async->timer.expires = jiffies + gb->jiffy_timeout;
> - add_timer(&op_async->timer);
> -
> goto done;
> error:
> - gb_loopback_async_operation_put(op_async);
> + atomic_dec(&gb->outstanding_operations);
> + gb_operation_put(operation);
> + kfree(op_async);
> done:
> mutex_unlock(&gb->mutex);
> return ret;
> @@ -1045,8 +940,10 @@ static int gb_loopback_fn(void *data)
> error = gb_loopback_async_sink(gb, size);
> }
>
> - if (error)
> + if (error) {
> gb->error++;
> + gb->iteration_count++;
> + }
This is an unrelated bug fix that should go in it's own patch, right?
The iteration count should be incremented on errors regardless of this
change.
> } else {
> /* We are effectively single threaded here */
> if (type == GB_LOOPBACK_TYPE_PING)
Thanks,
Johan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 4/4] staging: greybus: loopback: convert to use msecs internally
On Wed, Jan 04, 2017 at 12:11:21AM +, Bryan O'Donoghue wrote: > The API presented by operation.h expects milliseconds to be passed in. > This patch drops the conversion from user-input microseconds-to-jiffies and > from jiffies-to-milliseconds and instead converts directly from > microseconds-to-milliseconds. The new minimum timeout will be one > millisecond as opposed to one jiffy. You also reduced the maximum in a similar way (1 ms <= 1 jiffie). You could mention that here as well. > Signed-off-by: Bryan O'Donoghue Reviewed-by: Johan Hovold > --- > drivers/staging/greybus/loopback.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > -/* Min/max values in jiffies */ > +/* Min/max values in milliseconds */ > #define GB_LOOPBACK_TIMEOUT_MIN 1 > #define GB_LOOPBACK_TIMEOUT_MAX 1 Thanks, Johan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: greybus: operation: add generic timeout support
Add a struct timer_list to struct gb_operation and use that to implement
generic operation timeouts.
This simplifies the synchronous operation handling somewhat while also
providing a generic timeout mechanism that drivers can use for
asynchronous operations.
Signed-off-by: Johan Hovold
---
drivers/staging/greybus/loopback.c | 1 +
drivers/staging/greybus/operation.c | 50 ++---
drivers/staging/greybus/operation.h | 2 ++
3 files changed, 39 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/greybus/loopback.c
b/drivers/staging/greybus/loopback.c
index 6c2a41c638c3..4bee33f62fd4 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -629,6 +629,7 @@ static int gb_loopback_async_operation(struct gb_loopback
*gb, int type,
mutex_lock(&gb->mutex);
ret = gb_operation_request_send(operation,
gb_loopback_async_operation_callback,
+ 0,
GFP_KERNEL);
if (ret)
goto error;
diff --git a/drivers/staging/greybus/operation.c
b/drivers/staging/greybus/operation.c
index 0123109a1070..3023012808d9 100644
--- a/drivers/staging/greybus/operation.c
+++ b/drivers/staging/greybus/operation.c
@@ -273,18 +273,40 @@ static void gb_operation_request_handle(struct
gb_operation *operation)
static void gb_operation_work(struct work_struct *work)
{
struct gb_operation *operation;
+ int ret;
operation = container_of(work, struct gb_operation, work);
- if (gb_operation_is_incoming(operation))
+ if (gb_operation_is_incoming(operation)) {
gb_operation_request_handle(operation);
- else
+ } else {
+ ret = del_timer_sync(&operation->timer);
+ if (!ret) {
+ /* Cancel request message if scheduled by timeout. */
+ if (gb_operation_result(operation) == -ETIMEDOUT)
+ gb_message_cancel(operation->request);
+ }
+
operation->callback(operation);
+ }
gb_operation_put_active(operation);
gb_operation_put(operation);
}
+static void gb_operation_timeout(unsigned long arg)
+{
+ struct gb_operation *operation = (void *)arg;
+
+ if (gb_operation_result_set(operation, -ETIMEDOUT)) {
+ /*
+* A stuck request message will be cancelled from the
+* workqueue.
+*/
+ queue_work(gb_operation_completion_wq, &operation->work);
+ }
+}
+
static void gb_operation_message_init(struct gb_host_device *hd,
struct gb_message *message, u16 operation_id,
size_t payload_size, u8 type)
@@ -518,6 +540,9 @@ gb_operation_create_common(struct gb_connection
*connection, u8 type,
gfp_flags)) {
goto err_request;
}
+
+ setup_timer(&operation->timer, gb_operation_timeout,
+ (unsigned long)operation);
}
operation->flags = op_flags;
@@ -679,6 +704,7 @@ static void gb_operation_sync_callback(struct gb_operation
*operation)
* gb_operation_request_send() - send an operation request message
* @operation: the operation to initiate
* @callback: the operation completion callback
+ * @timeout: operation timeout in milliseconds, or zero for no timeout
* @gfp: the memory flags to use for any allocations
*
* The caller has filled in any payload so the request message is ready to go.
@@ -693,6 +719,7 @@ static void gb_operation_sync_callback(struct gb_operation
*operation)
*/
int gb_operation_request_send(struct gb_operation *operation,
gb_operation_callback callback,
+ unsigned int timeout,
gfp_t gfp)
{
struct gb_connection *connection = operation->connection;
@@ -742,6 +769,11 @@ int gb_operation_request_send(struct gb_operation
*operation,
if (ret)
goto err_put_active;
+ if (timeout) {
+ operation->timer.expires = jiffies + msecs_to_jiffies(timeout);
+ add_timer(&operation->timer);
+ }
+
return 0;
err_put_active:
@@ -763,26 +795,16 @@ int gb_operation_request_send_sync_timeout(struct
gb_operation *operation,
unsigned int timeout)
{
int ret;
- unsigned long timeout_jiffies;
ret = gb_operation_request_send(operation, gb_operation_sync_callback,
- GFP_KERNEL);
+ timeout, GFP_KERNEL);
if (ret)
return ret;
- if (timeout)
- timeout_jiffies = mse
Re: [PATCH v3 2/4] staging: greybus: operation: add private data with get/set accessors
On Wed, Jan 04, 2017 at 12:11:19AM +, Bryan O'Donoghue wrote:
> Asynchronous operation completion handler's lives are made easier if there
> is a generic pointer that can store private data associated with the
> operation. This patch adds a pointer field to operation.h and get/set
s/operation.h/struct gb_operation/
> methods to access that pointer.
>
> Signed-off-by: Bryan O'Donoghue
Reviewed-by: Johan Hovold
> ---
> drivers/staging/greybus/operation.h | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/staging/greybus/operation.h
> b/drivers/staging/greybus/operation.h
> index b8c0ba1..d43c116 100644
> --- a/drivers/staging/greybus/operation.h
> +++ b/drivers/staging/greybus/operation.h
> @@ -105,6 +105,8 @@ struct gb_operation {
>
> int active;
> struct list_headlinks; /* connection->operations */
> +
> + void*private;
> };
Thanks,
Johan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: greybus: timesync: validate platform state callback
When tearingdown timesync, and not in arche platform, the state platform
callback is not initialized. That will trigger the following NULL
dereferencing.
CallTrace:
? gb_timesync_platform_unlock_bus+0x11/0x20 [greybus]
gb_timesync_teardown+0x85/0xc0 [greybus]
gb_timesync_svc_remove+0xab/0x190 [greybus]
gb_svc_del+0x29/0x110 [greybus]
gb_hd_del+0x14/0x20 [greybus]
ap_disconnect+0x24/0x60 [gb_es2]
usb_unbind_interface+0x7a/0x2c0
__device_release_driver+0x96/0x150
device_release_driver+0x1e/0x30
bus_remove_device+0xe7/0x130
device_del+0x116/0x230
usb_disable_device+0x97/0x1f0
usb_disconnect+0x80/0x260
hub_event+0x5ca/0x10e0
process_one_work+0x126/0x3b0
worker_thread+0x55/0x4c0
? process_one_work+0x3b0/0x3b0
kthread+0xc4/0xe0
? kthread_park+0xb0/0xb0
ret_from_fork+0x22/0x30
So, fix that by adding checks before use the callback.
Cc: # 4.9.x
Signed-off-by: Rui Miguel Silva
---
Greg,
This patch does not contain the "commit upstream" snippet since you
already removed the timesync code from the -next tree. So, It would never get
there. However this will affect the greybus users of 4.9.x.
drivers/staging/greybus/timesync_platform.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/timesync_platform.c
b/drivers/staging/greybus/timesync_platform.c
index 113f3d6c4b3a..7d3ccc2bc98c 100644
--- a/drivers/staging/greybus/timesync_platform.c
+++ b/drivers/staging/greybus/timesync_platform.c
@@ -45,13 +45,18 @@ u32 gb_timesync_platform_get_clock_rate(void)
int gb_timesync_platform_lock_bus(struct gb_timesync_svc *pdata)
{
+ if (!arche_platform_change_state_cb)
+ return 0;
+
return arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_TIME_SYNC,
pdata);
}
void gb_timesync_platform_unlock_bus(void)
{
- arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_ACTIVE, NULL);
+ if (arche_platform_change_state_cb)
+ arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_ACTIVE,
+ NULL);
}
static const struct of_device_id arch_timer_of_match[] = {
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: operation: add generic timeout support
On 23/01/17 12:04, Johan Hovold wrote:
> +static void gb_operation_timeout(unsigned long arg)
> +{
> + struct gb_operation *operation = (void *)arg;
> +
> + if (gb_operation_result_set(operation, -ETIMEDOUT)) {
> + /*
> + * A stuck request message will be cancelled from the
> + * workqueue.
> + */
> + queue_work(gb_operation_completion_wq, &operation->work);
> + }
> +}
> +
If queue_work fails, you will never wake up the waiter.
> -
> - ret = wait_for_completion_interruptible_timeout(&operation->completion,
> - timeout_jiffies);
> + ret = wait_for_completion_interruptible(&operation->completion);
Note, that's a case I explicitly handle (failure to queue the work) in
the async set I sent out.
On gbsim at any rate, with a sufficient number of outstanding operations
in-flight it's possible to cause queue_work() to fail. Since you're in a
timer callback you're atomic so you can't call gb_operation_cancel()
here directly, those two reasons are why I did it in a work-queue as
opposed to a timer.
That's why I trapped that error at the send() stage of the operation -
because testing it out on gbsim showed messages getting lost @ the
queue_work stage.
---
bod
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: operation: add generic timeout support
On Mon, Jan 23, 2017 at 02:32:48PM +, Bryan O'Donoghue wrote:
> On 23/01/17 12:04, Johan Hovold wrote:
>
> > +static void gb_operation_timeout(unsigned long arg)
> > +{
> > + struct gb_operation *operation = (void *)arg;
> > +
> > + if (gb_operation_result_set(operation, -ETIMEDOUT)) {
> > + /*
> > +* A stuck request message will be cancelled from the
> > +* workqueue.
> > +*/
> > + queue_work(gb_operation_completion_wq, &operation->work);
> > + }
> > +}
> > +
>
> If queue_work fails, you will never wake up the waiter.
How could queue_work fail here? The operation result is always set
exactly once before being queued so this is fine.
> > -
> > - ret = wait_for_completion_interruptible_timeout(&operation->completion,
> > - timeout_jiffies);
> > + ret = wait_for_completion_interruptible(&operation->completion);
>
> Note, that's a case I explicitly handle (failure to queue the work) in
> the async set I sent out.
Yep, I noticed that, but that was for queueing your timeout work at
submission. And you queued unconditionally, so you could potentially
fail to queue if an operation was submitted twice, even if that would in
itself be a driver bug.
> On gbsim at any rate, with a sufficient number of outstanding operations
> in-flight it's possible to cause queue_work() to fail. Since you're in a
> timer callback you're atomic so you can't call gb_operation_cancel()
> here directly, those two reasons are why I did it in a work-queue as
> opposed to a timer.
Yeah, there are some non-trivial issues to deal with here (connection
tear down is another). I'm quite sure you won't be able to see
queue_work failing with this patch though.
> That's why I trapped that error at the send() stage of the operation -
> because testing it out on gbsim showed messages getting lost @ the
> queue_work stage.
Yeah, (with some minor modifications) the patch you ended up submitting
would have worked, just not as efficiently.
Thanks,
Johan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: operation: add generic timeout support
On 23/01/17 15:13, Johan Hovold wrote:
> On Mon, Jan 23, 2017 at 02:32:48PM +, Bryan O'Donoghue wrote:
>> On 23/01/17 12:04, Johan Hovold wrote:
>>
>>> +static void gb_operation_timeout(unsigned long arg)
>>> +{
>>> + struct gb_operation *operation = (void *)arg;
>>> +
>>> + if (gb_operation_result_set(operation, -ETIMEDOUT)) {
>>> + /*
>>> +* A stuck request message will be cancelled from the
>>> +* workqueue.
>>> +*/
>>> + queue_work(gb_operation_completion_wq, &operation->work);
>>> + }
>>> +}
>>> +
>>
>> If queue_work fails, you will never wake up the waiter.
>
> How could queue_work fail here? The operation result is always set
> exactly once before being queued so this is fine.
Perhaps it relates to a bug elsewhere, though I struggle to see how we
could feasibly re-use the work associated with an in-flight gb_message.
Could you trap the result code and BUG_ON()/WARN_ON() for it so that we
can debug this a little bit further ?
>
>>> -
>>> - ret = wait_for_completion_interruptible_timeout(&operation->completion,
>>> - timeout_jiffies);
>>> + ret = wait_for_completion_interruptible(&operation->completion);
>>
>> Note, that's a case I explicitly handle (failure to queue the work) in
>> the async set I sent out.
>
> Yep, I noticed that, but that was for queueing your timeout work at
> submission. And you queued unconditionally, so you could potentially
> fail to queue if an operation was submitted twice, even if that would in
> itself be a driver bug.
Have you run this through loopback without any # of in-flight operations
constrained ?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 1/7] staging: vchiq_arm: Add compat ioctl for create service
On Mon, 2017-01-23 at 14:38 +0300, Dan Carpenter wrote: > I'm attaching how I would basically like to see patch 1/7 done. It's > not perfect, I did it as quickly as possible and haven't tested > anything. I would normally be more careful if I were sending this > for > inclusion so there are no sign offs etc. Thanks, Dan. I appreciate the clarify on what you are looking for. Sounds like if I go that route it's going to take some time. I'm thinking that if I do this I should deal with one ioctl cleanup per series instead of aiming for the goal of getting compat to work. What's your opinion of building the compat_ioctl function on top of the existing code? Using the internal compat functions to build a 64 bit version of the data on the user mode stack? With that approach none of the existing code would need to be touched although the end result wouldn't be as nice or maintainable. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: timesync: validate platform state callback
Hi Johan,
On Mon, Jan 23, 2017 at 04:28:08PM +0100, Johan Hovold wrote:
On Mon, Jan 23, 2017 at 02:22:44PM +, Rui Miguel Silva wrote:
When tearingdown timesync, and not in arche platform, the state platform
callback is not initialized. That will trigger the following NULL
dereferencing.
CallTrace:
? gb_timesync_platform_unlock_bus+0x11/0x20 [greybus]
gb_timesync_teardown+0x85/0xc0 [greybus]
gb_timesync_svc_remove+0xab/0x190 [greybus]
gb_svc_del+0x29/0x110 [greybus]
gb_hd_del+0x14/0x20 [greybus]
ap_disconnect+0x24/0x60 [gb_es2]
usb_unbind_interface+0x7a/0x2c0
__device_release_driver+0x96/0x150
device_release_driver+0x1e/0x30
bus_remove_device+0xe7/0x130
device_del+0x116/0x230
usb_disable_device+0x97/0x1f0
usb_disconnect+0x80/0x260
hub_event+0x5ca/0x10e0
process_one_work+0x126/0x3b0
worker_thread+0x55/0x4c0
? process_one_work+0x3b0/0x3b0
kthread+0xc4/0xe0
? kthread_park+0xb0/0xb0
ret_from_fork+0x22/0x30
So, fix that by adding checks before use the callback.
Cc: # 4.9.x
Signed-off-by: Rui Miguel Silva
---
Greg,
This patch does not contain the "commit upstream" snippet since you
already removed the timesync code from the -next tree. So, It would never get
there. However this will affect the greybus users of 4.9.x.
This code is still in Linus tree and 4.10-rc so this is just a normal
fix even if this is no longer an issue in -next.
Yeah, but this code is not in staging-next anymore, It was mainly an
headsup for that, this patch will not apply to that tree.
drivers/staging/greybus/timesync_platform.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/timesync_platform.c
b/drivers/staging/greybus/timesync_platform.c
index 113f3d6c4b3a..7d3ccc2bc98c 100644
--- a/drivers/staging/greybus/timesync_platform.c
+++ b/drivers/staging/greybus/timesync_platform.c
@@ -45,13 +45,18 @@ u32 gb_timesync_platform_get_clock_rate(void)
int gb_timesync_platform_lock_bus(struct gb_timesync_svc *pdata)
{
+ if (!arche_platform_change_state_cb)
+ return 0;
+
return arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_TIME_SYNC,
pdata);
}
void gb_timesync_platform_unlock_bus(void)
{
- arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_ACTIVE, NULL);
+ if (arche_platform_change_state_cb)
+ arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_ACTIVE,
+ NULL);
I'd use the same pattern here:
if (!arche_platform_change_state_cb)
return;
arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_ACTIVE, NULL);
I also notice I forgot the Fixes: tag, so I will add your suggestion
to my v2.
Thanks Johan,
Cheers,
Rui
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: timesync: validate platform state callback
On Mon, Jan 23, 2017 at 02:22:44PM +, Rui Miguel Silva wrote:
> When tearingdown timesync, and not in arche platform, the state platform
> callback is not initialized. That will trigger the following NULL
> dereferencing.
> CallTrace:
>
> ? gb_timesync_platform_unlock_bus+0x11/0x20 [greybus]
> gb_timesync_teardown+0x85/0xc0 [greybus]
> gb_timesync_svc_remove+0xab/0x190 [greybus]
> gb_svc_del+0x29/0x110 [greybus]
> gb_hd_del+0x14/0x20 [greybus]
> ap_disconnect+0x24/0x60 [gb_es2]
> usb_unbind_interface+0x7a/0x2c0
> __device_release_driver+0x96/0x150
> device_release_driver+0x1e/0x30
> bus_remove_device+0xe7/0x130
> device_del+0x116/0x230
> usb_disable_device+0x97/0x1f0
> usb_disconnect+0x80/0x260
> hub_event+0x5ca/0x10e0
> process_one_work+0x126/0x3b0
> worker_thread+0x55/0x4c0
> ? process_one_work+0x3b0/0x3b0
> kthread+0xc4/0xe0
> ? kthread_park+0xb0/0xb0
> ret_from_fork+0x22/0x30
>
> So, fix that by adding checks before use the callback.
>
> Cc: # 4.9.x
> Signed-off-by: Rui Miguel Silva
> ---
> Greg,
> This patch does not contain the "commit upstream" snippet since you
> already removed the timesync code from the -next tree. So, It would never get
> there. However this will affect the greybus users of 4.9.x.
This code is still in Linus tree and 4.10-rc so this is just a normal
fix even if this is no longer an issue in -next.
> drivers/staging/greybus/timesync_platform.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/timesync_platform.c
> b/drivers/staging/greybus/timesync_platform.c
> index 113f3d6c4b3a..7d3ccc2bc98c 100644
> --- a/drivers/staging/greybus/timesync_platform.c
> +++ b/drivers/staging/greybus/timesync_platform.c
> @@ -45,13 +45,18 @@ u32 gb_timesync_platform_get_clock_rate(void)
>
> int gb_timesync_platform_lock_bus(struct gb_timesync_svc *pdata)
> {
> + if (!arche_platform_change_state_cb)
> + return 0;
> +
> return arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_TIME_SYNC,
> pdata);
> }
>
> void gb_timesync_platform_unlock_bus(void)
> {
> - arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_ACTIVE, NULL);
> + if (arche_platform_change_state_cb)
> + arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_ACTIVE,
> +NULL);
I'd use the same pattern here:
if (!arche_platform_change_state_cb)
return;
arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_ACTIVE, NULL);
Thanks,
Johan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: greybus: timesync: validate platform state callback
When tearingdown timesync, and not in arche platform, the state platform
callback is not initialized. That will trigger the following NULL
dereferencing.
CallTrace:
? gb_timesync_platform_unlock_bus+0x11/0x20 [greybus]
gb_timesync_teardown+0x85/0xc0 [greybus]
gb_timesync_svc_remove+0xab/0x190 [greybus]
gb_svc_del+0x29/0x110 [greybus]
gb_hd_del+0x14/0x20 [greybus]
ap_disconnect+0x24/0x60 [gb_es2]
usb_unbind_interface+0x7a/0x2c0
__device_release_driver+0x96/0x150
device_release_driver+0x1e/0x30
bus_remove_device+0xe7/0x130
device_del+0x116/0x230
usb_disable_device+0x97/0x1f0
usb_disconnect+0x80/0x260
hub_event+0x5ca/0x10e0
process_one_work+0x126/0x3b0
worker_thread+0x55/0x4c0
? process_one_work+0x3b0/0x3b0
kthread+0xc4/0xe0
? kthread_park+0xb0/0xb0
ret_from_fork+0x22/0x30
So, fix that by adding checks before use the callback.
Fixes: 970dc85bd95d ("greybus: timesync: Add timesync core driver")
Cc: # 4.9.x
Signed-off-by: Rui Miguel Silva
---
Greg,
This patch does not contain the "commit upstream" snippet since you
already removed the timesync code from the -next tree, even though it is still
in 4.10-rc. However this will affect the greybus users of 4.9.x. If you
meanwhile pull that remove to rc, this patch is only necessary in stable 4.9.x.
v1->v2:
- add Fixes tag to changelog
Johan Hovold:
- for symmetry with the lock, adjust the unlock_bus function accordantly.
drivers/staging/greybus/timesync_platform.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/staging/greybus/timesync_platform.c
b/drivers/staging/greybus/timesync_platform.c
index 113f3d6c4b3a..27f75b17679b 100644
--- a/drivers/staging/greybus/timesync_platform.c
+++ b/drivers/staging/greybus/timesync_platform.c
@@ -45,12 +45,18 @@ u32 gb_timesync_platform_get_clock_rate(void)
int gb_timesync_platform_lock_bus(struct gb_timesync_svc *pdata)
{
+ if (!arche_platform_change_state_cb)
+ return 0;
+
return arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_TIME_SYNC,
pdata);
}
void gb_timesync_platform_unlock_bus(void)
{
+ if (!arche_platform_change_state_cb)
+ return;
+
arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_ACTIVE, NULL);
}
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: greybus: timesync: validate platform state callback
On Mon, Jan 23, 2017 at 04:32:57PM +, Rui Miguel Silva wrote:
> When tearingdown timesync, and not in arche platform, the state platform
> callback is not initialized. That will trigger the following NULL
> dereferencing.
> CallTrace:
>
> ? gb_timesync_platform_unlock_bus+0x11/0x20 [greybus]
> gb_timesync_teardown+0x85/0xc0 [greybus]
> gb_timesync_svc_remove+0xab/0x190 [greybus]
> gb_svc_del+0x29/0x110 [greybus]
> gb_hd_del+0x14/0x20 [greybus]
> ap_disconnect+0x24/0x60 [gb_es2]
> usb_unbind_interface+0x7a/0x2c0
> __device_release_driver+0x96/0x150
> device_release_driver+0x1e/0x30
> bus_remove_device+0xe7/0x130
> device_del+0x116/0x230
> usb_disable_device+0x97/0x1f0
> usb_disconnect+0x80/0x260
> hub_event+0x5ca/0x10e0
> process_one_work+0x126/0x3b0
> worker_thread+0x55/0x4c0
> ? process_one_work+0x3b0/0x3b0
> kthread+0xc4/0xe0
> ? kthread_park+0xb0/0xb0
> ret_from_fork+0x22/0x30
>
> So, fix that by adding checks before use the callback.
>
> Fixes: 970dc85bd95d ("greybus: timesync: Add timesync core driver")
> Cc: # 4.9.x
> Signed-off-by: Rui Miguel Silva
Reviewed-by: Johan Hovold
___
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
Radim Krcmar writes:
> 2017-01-19 15:16+0100, 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 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
>> ---
>
> It is a nice coincidence that KVM is working on a PTP driver as well,
> https://lkml.org/lkml/2017/1/20/247, and it uses more precise/accurate
> method of converting the host time that Hyper-V could also use.
>
> Hyper-V provides {host_time, ref_time} tuple, but gettime64() requires
> that you return just host_time and a new "ref_time" is then computed to
> be in the middle of two guest_time reads.
> I recommend you use getcrosststamp PTP callback, which allows you to
> provide the tuple. Userspace can then use PTP_SYS_OFFSET_PRECISE
> ioctl.
Thanks, good suggestion,
as far as I see PTP_SYS_OFFSET_PRECISE support was just added to chrony:
https://git.tuxfamily.org/chrony/chrony.git/commit/?id=31b6a1a8f23147077df3c6a64518d082c35e
I'll implement getcrosststamp() as well but I'll probably have to wait
till K. Y.'s restructuring (https://lkml.org/lkml/2016/12/30/260) lands
and do my series on top of that. We'll also be using the TSC page
clocksource (when available) to not do the unneeded vmexit.
>
> KVM patches also proposes to change PTP_SYS_OFFSET, so when gettime64
> callback is not implemented, the ioctl uses getcrosststamp instead,
> which would avoid code duplication and improve precision/accuracy.
It's probably still worth it to have the 'lightweight' gettime64()
implementation as going through the getcrosststamp() routine (see
get_device_system_crosststamp() which we'll probably use for Hyper-V
also -- it's not very simple) every time and throwing away half of the
result doesn't look optimal.
--
Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: operation: add generic timeout support
On Mon, Jan 23, 2017 at 03:39:43PM +, Bryan O'Donoghue wrote:
> On 23/01/17 15:13, Johan Hovold wrote:
> > On Mon, Jan 23, 2017 at 02:32:48PM +, Bryan O'Donoghue wrote:
> >> On 23/01/17 12:04, Johan Hovold wrote:
> >>
> >>> +static void gb_operation_timeout(unsigned long arg)
> >>> +{
> >>> + struct gb_operation *operation = (void *)arg;
> >>> +
> >>> + if (gb_operation_result_set(operation, -ETIMEDOUT)) {
> >>> + /*
> >>> + * A stuck request message will be cancelled from the
> >>> + * workqueue.
> >>> + */
> >>> + queue_work(gb_operation_completion_wq, &operation->work);
> >>> + }
> >>> +}
> >>> +
> >>
> >> If queue_work fails, you will never wake up the waiter.
> >
> > How could queue_work fail here? The operation result is always set
> > exactly once before being queued so this is fine.
>
> Perhaps it relates to a bug elsewhere, though I struggle to see how we
> could feasibly re-use the work associated with an in-flight gb_message.
Yeah, and the check you added is even for a different struct
delayed_work, so I really don't think we have anything to worry about
here.
> Could you trap the result code and BUG_ON()/WARN_ON() for it so that we
> can debug this a little bit further ?
IIUC you've only seen this with some of your unfinished work during
development (and for a different work struct), so I don't think we need
to add a WARN_ON yet.
> >>> -
> >>> - ret = wait_for_completion_interruptible_timeout(&operation->completion,
> >>> - timeout_jiffies);
> >>> + ret = wait_for_completion_interruptible(&operation->completion);
> >>
> >> Note, that's a case I explicitly handle (failure to queue the work) in
> >> the async set I sent out.
> >
> > Yep, I noticed that, but that was for queueing your timeout work at
> > submission. And you queued unconditionally, so you could potentially
> > fail to queue if an operation was submitted twice, even if that would in
> > itself be a driver bug.
>
> Have you run this through loopback without any # of in-flight operations
> constrained ?
No, not yet as that would require also your changes to the loopback
driver (the loopback driver still has its custom timeout implementation
after this patch).
I tested this using gbsim and synchronous operations as my Ara build
setup is currently broken.
Perhaps you can rerun the tests you saw the queue_work fail with? Are
you using gbsim or real hardware to test with?
Thanks,
Johan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: fbtft: fix checkpatch decimal permissions error
Fix the following errors: Use 4 digit octal (0777) not decimal permissions Signed-off-by: Abdul Rauf --- 32 instances of this error has been fixed --- drivers/staging/fbtft/fb_hx8340bn.c | 2 +- drivers/staging/fbtft/fb_pcd8544.c | 4 ++-- drivers/staging/fbtft/fb_ssd1289.c | 2 +- drivers/staging/fbtft/fb_tls8204.c | 2 +- drivers/staging/fbtft/fb_uc1611.c| 12 ++-- drivers/staging/fbtft/fb_watterott.c | 2 +- drivers/staging/fbtft/fbtft-core.c | 2 +- drivers/staging/fbtft/fbtft_device.c | 38 ++-- 8 files changed, 32 insertions(+), 32 deletions(-) diff --git a/drivers/staging/fbtft/fb_hx8340bn.c b/drivers/staging/fbtft/fb_hx8340bn.c index 9970ed74bb38..c24331cc179c 100644 --- a/drivers/staging/fbtft/fb_hx8340bn.c +++ b/drivers/staging/fbtft/fb_hx8340bn.c @@ -37,7 +37,7 @@ "3 3 17 8 4 7 05 7 6 0 3 1 6 0 0 " static bool emulate; -module_param(emulate, bool, 0); +module_param(emulate, bool, ); MODULE_PARM_DESC(emulate, "Force emulation in 9-bit mode"); static int init_display(struct fbtft_par *par) diff --git a/drivers/staging/fbtft/fb_pcd8544.c b/drivers/staging/fbtft/fb_pcd8544.c index a4710dc067ef..636cc83594bc 100644 --- a/drivers/staging/fbtft/fb_pcd8544.c +++ b/drivers/staging/fbtft/fb_pcd8544.c @@ -33,11 +33,11 @@ #define DEFAULT_GAMMA "40" /* gamma controls the contrast in this driver */ static unsigned int tc; -module_param(tc, uint, 0); +module_param(tc, uint, ); MODULE_PARM_DESC(tc, "TC[1:0] Temperature coefficient: 0-3 (default: 0)"); static unsigned int bs = 4; -module_param(bs, uint, 0); +module_param(bs, uint, ); MODULE_PARM_DESC(bs, "BS[2:0] Bias voltage level: 0-7 (default: 4)"); static int init_display(struct fbtft_par *par) diff --git a/drivers/staging/fbtft/fb_ssd1289.c b/drivers/staging/fbtft/fb_ssd1289.c index 25f9fbe1e76f..6dc085846948 100644 --- a/drivers/staging/fbtft/fb_ssd1289.c +++ b/drivers/staging/fbtft/fb_ssd1289.c @@ -30,7 +30,7 @@ "02 03 2 5 7 5 4 2 4 2" static unsigned int reg11 = 0x6040; -module_param(reg11, uint, 0); +module_param(reg11, uint, ); MODULE_PARM_DESC(reg11, "Register 11h value"); static int init_display(struct fbtft_par *par) diff --git a/drivers/staging/fbtft/fb_tls8204.c b/drivers/staging/fbtft/fb_tls8204.c index ea2ddacb9468..ae8393b023e4 100644 --- a/drivers/staging/fbtft/fb_tls8204.c +++ b/drivers/staging/fbtft/fb_tls8204.c @@ -36,7 +36,7 @@ #define DEFAULT_GAMMA "40" static unsigned int bs = 4; -module_param(bs, uint, 0); +module_param(bs, uint, ); MODULE_PARM_DESC(bs, "BS[2:0] Bias voltage level: 0-7 (default: 4)"); static int init_display(struct fbtft_par *par) diff --git a/drivers/staging/fbtft/fb_uc1611.c b/drivers/staging/fbtft/fb_uc1611.c index b33b73f17da4..48e3b3fd9fed 100644 --- a/drivers/staging/fbtft/fb_uc1611.c +++ b/drivers/staging/fbtft/fb_uc1611.c @@ -42,30 +42,30 @@ /* BR -> actual ratio: 0-3 -> 5, 10, 11, 13 */ static unsigned int ratio = 2; -module_param(ratio, uint, 0); +module_param(ratio, uint, ); MODULE_PARM_DESC(ratio, "BR[1:0] Bias voltage ratio: 0-3 (default: 2)"); static unsigned int gain = 3; -module_param(gain, uint, 0); +module_param(gain, uint, ); MODULE_PARM_DESC(gain, "GN[1:0] Bias voltage gain: 0-3 (default: 3)"); static unsigned int pot = 16; -module_param(pot, uint, 0); +module_param(pot, uint, ); MODULE_PARM_DESC(pot, "PM[6:0] Bias voltage pot.: 0-63 (default: 16)"); /* TC -> % compensation per deg C: 0-3 -> -.05, -.10, -.015, -.20 */ static unsigned int temp; -module_param(temp, uint, 0); +module_param(temp, uint, ); MODULE_PARM_DESC(temp, "TC[1:0] Temperature compensation: 0-3 (default: 0)"); /* PC[1:0] -> LCD capacitance: 0-3 -> <20nF, 20-28 nF, 29-40 nF, 40-56 nF */ static unsigned int load = 1; -module_param(load, uint, 0); +module_param(load, uint, ); MODULE_PARM_DESC(load, "PC[1:0] Panel Loading: 0-3 (default: 1)"); /* PC[3:2] -> V_LCD: 0, 1, 3 -> ext., int. with ratio = 5, int. standard */ static unsigned int pump = 3; -module_param(pump, uint, 0); +module_param(pump, uint, ); MODULE_PARM_DESC(pump, "PC[3:2] Pump control: 0,1,3 (default: 3)"); static int init_display(struct fbtft_par *par) diff --git a/drivers/staging/fbtft/fb_watterott.c b/drivers/staging/fbtft/fb_watterott.c index a52e28a48825..429304546b44 100644 --- a/drivers/staging/fbtft/fb_watterott.c +++ b/drivers/staging/fbtft/fb_watterott.c @@ -40,7 +40,7 @@ #define COLOR_RGB565 16 static short mode = 565; -module_param(mode, short, 0); +module_param(mode, short, ); MODULE_PARM_DESC(mode, "RGB color transfer mode: 332, 565 (default)"); static void write_reg8_bus8(struct fbtft_par *par, int len, ...) diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index 4e13090c7fbd..51c4481db451 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c
Re: [PATCH 2/2] scsi: storvsc: Add support for FC lightweight host.
Hi, There is no way to issue a lip directly as the current client for this feature ( storvsc ) does not handle that request as a physical fc hba can. Storvsc only has two fc attributes exposed - port_name and node_name. You can rescan the bus with the standard echo "- - -" > /sys/class/scsi_host/hostX/scan. Cathy On 01/22/2017 10:13 PM, Fam Zheng wrote: On Wed, 01/18 15:28, Cathy Avery wrote: Enable FC lightweight host option so that the luns exposed by the driver may be manually scanned. Hi Cathy, out of curiosity: how does this relate to issue_lip operation? And how to trigger manual scan with this patch? Fam ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 00/24] i.MX Media Driver
On 01/23/2017 03:00 AM, Philipp Zabel wrote:
On Fri, 2017-01-20 at 21:39 +0100, Hans Verkuil wrote:
[...]
There is a VDIC entity in the i.MX IPU that performs de-interlacing with
hardware filters for motion compensation. Some of the motion compensation
modes ("low" and "medium" motion) require that the VDIC receive video
frame fields from memory buffers (dedicated dma channels in the
IPU are used to transfer those buffers into the VDIC).
So one option to support those modes would be to pass the raw buffers
from a camera sensor up to userspace to a capture device, and then pass
them back to the VDIC for de-interlacing using a mem2mem device.
Philipp and I are both in agreement that, since userland is not interested
in the intermediate interlaced buffers in this case, but only the final
result (motion compensated, de-interlaced frames), it is more efficient
to provide a media link that allows passing those intermediate frames
directly from a camera source pad to VDIC sink pad, without having
to route them through userspace.
So in order to support that, I've implemented a simple FIFO dma buffer
queue in the driver to allow passing video buffers directly from a source
to a sink. It is modeled loosely off the vb2 state machine and API, but
simpler (for instance it only allows contiguous, cache-coherent buffers).
This is where Philipp has an argument, that this should be done with a
new API in videobuf2.
That is one part of the argument. I'm glad to understand now that we
agree about this.
And I'm actually in total agreement with that. I definitely agree that there
should be a mechanism in the media framework that allows passing video
buffers from a source pad to a sink pad using a software queue, with no
involvement from userland.
That is the other part of the argument. I do not agree that these
software queue "links" should be presented to userspace as media pad
links between two entities of a media device.
First, that would limit the links to subdevices contained in the same
media graph, while this should work between any two capture and output
queues of different devices.
It sounds like we are talking about two different new proposed features.
My proposal is to implement a software buffer queue between pads.
Beyond enabling the link between pads using the existing media controller
API, userspace is not involved after that. The fact that this link is
accomplished
with a software buffer queue is not known, and doesn't need to be known,
by userspace.
Your proposal, if I have it right, is to allow linking two v4l2 device
vb2 queues
(i.e. /dev/videoX -> /dev/videoY), using a new user level API, in a free-run
mode such that v4l2 buffers get passed from one device's vb2 queue to the
other without requiring the v4l2 user program to actively forward those
buffers.
There isn't anything that would preclude one from the other, they can
both exist. But they are different ideas. One implements software queues
at the _pad level_ and is opaque to userspace, the other links queues
at the _device level_ using a new user API, but once the link is
established,
also does not require any involvement from userspace.
What I'm saying is we can do _both_.
Assume for example, we want to encode the captured, deinterlaced video
to h.264 with the coda VPU driver. A software queue link could be
established between the CSI capture and the VDIC deinterlacer input,
That's already available in the media graph. By linking CSI and
VDIC entities. The capture device will then already be providing
de-interlaced video, and ...
just as between the VDIC deinterlacer output and the coda VPU input.
Technically, there would be no difference between those two linked
capture/output queue pairs. But the coda driver is a completely separate
mem2mem device. And since it is not part of the i.MX media graph, there
is no entity pad to link to.
your free-run queue linking could then be used to link the (already)
de-interlaced stream to the coda device for h.264 encode.
The other idea would be to eventually make the coda device part of
the media graph as an entity. Then this link would instead be via pads.
Or assume there is an USB analog capture device that produces interlaced
frames. I think it should be possible to connect its capture queue to
the VDIC deinterlacer output queue just the same way as linking the CSI
to the VDIC (in software queue mode).
Right, for devices that are outside the i.MX media graph, such as a USB
capture device (or coda), access to the i.MX entities such as the VDIC would
require an i.MX mem2mem device with media links to the VDIC. The USB
capture device would forward its captured frames to mem2mem (maybe
using your free-run vb2 queue linking idea):
usb device -> i.mx mem2mem device -> VDIC entity -> i.mx mem2mem device
Second, the subdevice pad formats describe wire formats, not memory
formats. The user might want to choose between 4:2:2 and 4:2:0
subsampled YUV formats for the intermedia
[PATCH 02/14] vmbus: constify parameters where possible
Functions that just query state of ring buffer can have parameters
marked const.
Signed-off-by: Stephen Hemminger
---
drivers/hv/hyperv_vmbus.h | 4 ++--
drivers/hv/ring_buffer.c | 20 +---
include/linux/hyperv.h| 12 ++--
3 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 2463ef93c1f6..fd91045ad677 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -288,8 +288,8 @@ int hv_ringbuffer_read(struct vmbus_channel *channel,
void *buffer, u32 buflen, u32 *buffer_actual_len,
u64 *requestid, bool raw);
-void hv_ringbuffer_get_debuginfo(struct hv_ring_buffer_info *ring_info,
- struct hv_ring_buffer_debug_info *debug_info);
+void hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info,
+struct hv_ring_buffer_debug_info *debug_info);
void hv_begin_read(struct hv_ring_buffer_info *rbi);
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 65f562ee9843..e24b10e4fcb3 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -115,11 +115,9 @@ hv_set_next_write_location(struct hv_ring_buffer_info
*ring_info,
/* Get the next read location for the specified ring buffer. */
static inline u32
-hv_get_next_read_location(struct hv_ring_buffer_info *ring_info)
+hv_get_next_read_location(const struct hv_ring_buffer_info *ring_info)
{
- u32 next = ring_info->ring_buffer->read_index;
-
- return next;
+ return ring_info->ring_buffer->read_index;
}
/*
@@ -127,8 +125,8 @@ hv_get_next_read_location(struct hv_ring_buffer_info
*ring_info)
* This allows the caller to skip.
*/
static inline u32
-hv_get_next_readlocation_withoffset(struct hv_ring_buffer_info *ring_info,
-u32 offset)
+hv_get_next_readlocation_withoffset(const struct hv_ring_buffer_info
*ring_info,
+ u32 offset)
{
u32 next = ring_info->ring_buffer->read_index;
@@ -149,7 +147,7 @@ hv_set_next_read_location(struct hv_ring_buffer_info
*ring_info,
/* Get the size of the ring buffer. */
static inline u32
-hv_get_ring_buffersize(struct hv_ring_buffer_info *ring_info)
+hv_get_ring_buffersize(const struct hv_ring_buffer_info *ring_info)
{
return ring_info->ring_datasize;
}
@@ -166,7 +164,7 @@ hv_get_ring_bufferindices(struct hv_ring_buffer_info
*ring_info)
* Assume there is enough room. Handles wrap-around in src case only!!
*/
static u32 hv_copyfrom_ringbuffer(
- struct hv_ring_buffer_info *ring_info,
+ const struct hv_ring_buffer_info *ring_info,
void*dest,
u32 destlen,
u32 start_read_offset)
@@ -190,7 +188,7 @@ static u32 hv_copyfrom_ringbuffer(
static u32 hv_copyto_ringbuffer(
struct hv_ring_buffer_info *ring_info,
u32 start_write_offset,
- void*src,
+ const void *src,
u32 srclen)
{
void *ring_buffer = hv_get_ring_buffer(ring_info);
@@ -205,8 +203,8 @@ static u32 hv_copyto_ringbuffer(
}
/* Get various debug metrics for the specified ring buffer. */
-void hv_ringbuffer_get_debuginfo(struct hv_ring_buffer_info *ring_info,
- struct hv_ring_buffer_debug_info *debug_info)
+void hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info,
+struct hv_ring_buffer_debug_info *debug_info)
{
u32 bytes_avail_towrite;
u32 bytes_avail_toread;
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index f29a14949446..35328cfa0e80 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -138,8 +138,8 @@ struct hv_ring_buffer_info {
* for the specified ring buffer
*/
static inline void
-hv_get_ringbuffer_availbytes(struct hv_ring_buffer_info *rbi,
- u32 *read, u32 *write)
+hv_get_ringbuffer_availbytes(const struct hv_ring_buffer_info *rbi,
+u32 *read, u32 *write)
{
u32 read_loc, write_loc, dsize;
@@ -153,7 +153,7 @@ hv_get_ringbuffer_availbytes(struct hv_ring_buffer_info
*rbi,
*read = dsize - *write;
}
-static inline u32 hv_get_bytes_to_read(struct hv_ring_buffer_info *rbi)
+static inline u32 hv_get_bytes_to_read(const struct hv_ring_buffer_info *rbi)
{
u32 read_loc, write_loc, dsize, read;
@@ -167,7 +167,7 @@ static inline u32 hv_get_bytes_to_read(struct
hv_ring_buffer_info *rbi)
return read;
}
-static inline u32 hv_get_bytes_to_write(struct hv_ring_buffer_info *rbi)
+static inline u32 hv_get_bytes_to_write(const struct hv_ring_buffer_info *rbi)
{
u32 read_loc, write_loc, dsize, write;
@@
[PATCH 09/14] vmbus: remove unnecessary initialization
Don't initialize variables that are then set a few lines later.
Signed-off-by: Stephen Hemminger
---
drivers/hv/ring_buffer.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 04b5c5fce7ae..31b279919253 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -258,14 +258,13 @@ void hv_ringbuffer_cleanup(struct hv_ring_buffer_info
*ring_info)
int hv_ringbuffer_write(struct vmbus_channel *channel,
struct kvec *kv_list, u32 kv_count)
{
- int i = 0;
+ int i;
u32 bytes_avail_towrite;
- u32 totalbytes_towrite = 0;
-
+ u32 totalbytes_towrite = sizeof(u64);
u32 next_write_location;
u32 old_write;
- u64 prev_indices = 0;
- unsigned long flags = 0;
+ u64 prev_indices;
+ unsigned long flags;
struct hv_ring_buffer_info *outring_info = &channel->outbound;
if (channel->rescind)
@@ -274,8 +273,6 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
for (i = 0; i < kv_count; i++)
totalbytes_towrite += kv_list[i].iov_len;
- totalbytes_towrite += sizeof(u64);
-
spin_lock_irqsave(&outring_info->ring_lock, flags);
bytes_avail_towrite = hv_get_bytes_to_write(outring_info);
@@ -330,7 +327,7 @@ int hv_ringbuffer_read(struct vmbus_channel *channel,
u64 *requestid, bool raw)
{
u32 bytes_avail_toread;
- u32 next_read_location = 0;
+ u32 next_read_location;
u64 prev_indices = 0;
struct vmpacket_descriptor desc;
u32 offset;
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 04/14] vmbus: eliminate unnecessary wrapper functions
The accessor functions for ring buffer are only used once and only in
one file, the code is clearer without them.
Signed-off-by: Stephen Hemminger
---
drivers/hv/ring_buffer.c | 45 +
1 file changed, 9 insertions(+), 36 deletions(-)
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index e24b10e4fcb3..e408886ef171 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -96,30 +96,6 @@ static void hv_signal_on_write(u32 old_write, struct
vmbus_channel *channel,
vmbus_setevent(channel);
}
-/* Get the next write location for the specified ring buffer. */
-static inline u32
-hv_get_next_write_location(struct hv_ring_buffer_info *ring_info)
-{
- u32 next = ring_info->ring_buffer->write_index;
-
- return next;
-}
-
-/* Set the next write location for the specified ring buffer. */
-static inline void
-hv_set_next_write_location(struct hv_ring_buffer_info *ring_info,
-u32 next_write_location)
-{
- ring_info->ring_buffer->write_index = next_write_location;
-}
-
-/* Get the next read location for the specified ring buffer. */
-static inline u32
-hv_get_next_read_location(const struct hv_ring_buffer_info *ring_info)
-{
- return ring_info->ring_buffer->read_index;
-}
-
/*
* Get the next read location + offset for the specified ring buffer.
* This allows the caller to skip.
@@ -170,7 +146,7 @@ static u32 hv_copyfrom_ringbuffer(
u32 start_read_offset)
{
void *ring_buffer = hv_get_ring_buffer(ring_info);
- u32 ring_buffer_size = hv_get_ring_buffersize(ring_info);
+ u32 ring_buffer_size = ring_info->ring_datasize;
memcpy(dest, ring_buffer + start_read_offset, destlen);
@@ -192,7 +168,7 @@ static u32 hv_copyto_ringbuffer(
u32 srclen)
{
void *ring_buffer = hv_get_ring_buffer(ring_info);
- u32 ring_buffer_size = hv_get_ring_buffersize(ring_info);
+ u32 ring_buffer_size = ring_info->ring_datasize;
memcpy(ring_buffer + start_write_offset, src, srclen);
@@ -319,9 +295,9 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
}
/* Write to the ring buffer */
- next_write_location = hv_get_next_write_location(outring_info);
+ next_write_location = outring_info->ring_buffer->write_index;
- old_write = next_write_location;
+ old_write = outring_info->ring_buffer->write_index;
for (i = 0; i < kv_count; i++) {
next_write_location = hv_copyto_ringbuffer(outring_info,
@@ -331,8 +307,7 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
}
/* Set previous packet start */
- prev_indices = hv_get_ring_bufferindices(outring_info);
-
+ prev_indices = (u64)outring_info->ring_buffer->write_index << 32;
next_write_location = hv_copyto_ringbuffer(outring_info,
next_write_location,
&prev_indices,
@@ -342,8 +317,7 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
virt_mb();
/* Now, update the write location */
- hv_set_next_write_location(outring_info, next_write_location);
-
+ outring_info->ring_buffer->write_index = next_write_location;
if (lock)
spin_unlock_irqrestore(&outring_info->ring_lock, flags);
@@ -386,10 +360,9 @@ int hv_ringbuffer_read(struct vmbus_channel *channel,
return ret;
}
- next_read_location = hv_get_next_read_location(inring_info);
- next_read_location = hv_copyfrom_ringbuffer(inring_info, &desc,
- sizeof(desc),
- next_read_location);
+ next_read_location = hv_copyfrom_ringbuffer(inring_info,
+ &desc, sizeof(desc),
+ inring_info->ring_buffer->read_index);
offset = raw ? 0 : (desc.offset8 << 3);
packetlen = (desc.len8 << 3) - offset;
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/14] vmbus: remove useless return's
No need for empty return at end of void function Signed-off-by: Stephen Hemminger --- drivers/hv/hv_balloon.c | 2 -- drivers/hv/hv_fcopy.c| 2 -- drivers/hv/hv_kvp.c | 2 -- drivers/hv/hv_snapshot.c | 2 -- drivers/hv/ring_buffer.c | 2 -- drivers/hv/vmbus_drv.c | 2 -- include/linux/hyperv.h | 2 -- 7 files changed, 14 deletions(-) diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c index 14c3dc4bd23c..2f4484dc772e 100644 --- a/drivers/hv/hv_balloon.c +++ b/drivers/hv/hv_balloon.c @@ -721,8 +721,6 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size, 5*HZ); post_status(&dm_device); } - - return; } static void hv_online_page(struct page *pg) diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c index e47d8c9db03a..cb91053b852d 100644 --- a/drivers/hv/hv_fcopy.c +++ b/drivers/hv/hv_fcopy.c @@ -177,8 +177,6 @@ static void fcopy_send_data(struct work_struct *dummy) } } kfree(smsg_out); - - return; } /* diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c index 3abfc5983c97..7baad4083a47 100644 --- a/drivers/hv/hv_kvp.c +++ b/drivers/hv/hv_kvp.c @@ -471,8 +471,6 @@ kvp_send_key(struct work_struct *dummy) } kfree(message); - - return; } /* diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c index 4e543dbb731a..cf88dbfd7f32 100644 --- a/drivers/hv/hv_snapshot.c +++ b/drivers/hv/hv_snapshot.c @@ -203,8 +203,6 @@ static void vss_send_op(void) } kfree(vss_msg); - - return; } static void vss_handle_request(struct work_struct *dummy) diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c index 2cd402986858..65f562ee9843 100644 --- a/drivers/hv/ring_buffer.c +++ b/drivers/hv/ring_buffer.c @@ -94,8 +94,6 @@ static void hv_signal_on_write(u32 old_write, struct vmbus_channel *channel, */ if (old_write == READ_ONCE(rbi->ring_buffer->read_index)) vmbus_setevent(channel); - - return; } /* Get the next write location for the specified ring buffer. */ diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 8e81346114d4..d4e80e902436 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -785,8 +785,6 @@ static void vmbus_shutdown(struct device *child_device) if (drv->shutdown) drv->shutdown(dev); - - return; } diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index 7ea20bd7cdd1..f29a14949446 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1515,8 +1515,6 @@ static inline void hv_signal_on_read(struct vmbus_channel *channel) if (cur_write_sz >= pending_sz) vmbus_setevent(channel); - - return; } /* -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 07/14] vmbus: remove conditional locking of vmbus_write
All current usage of vmbus write uses the acquire_lock flag, therefore having it be optional is unnecessary. This also fixes a sparse warning since sparse doesn't like when a function has conditional locking. Signed-off-by: Stephen Hemminger --- drivers/hv/channel.c | 13 - drivers/hv/channel_mgmt.c | 1 - drivers/hv/hyperv_vmbus.h | 3 +-- drivers/hv/ring_buffer.c | 11 --- include/linux/hyperv.h| 15 --- 5 files changed, 9 insertions(+), 34 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index fd73ce1340be..61e2cc1b653f 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -651,7 +651,6 @@ int vmbus_sendpacket_ctl(struct vmbus_channel *channel, void *buffer, u32 packetlen_aligned = ALIGN(packetlen, sizeof(u64)); struct kvec bufferlist[3]; u64 aligned_data = 0; - bool lock = channel->acquire_ring_lock; int num_vecs = ((bufferlen != 0) ? 3 : 1); @@ -670,7 +669,7 @@ int vmbus_sendpacket_ctl(struct vmbus_channel *channel, void *buffer, bufferlist[2].iov_base = &aligned_data; bufferlist[2].iov_len = (packetlen_aligned - packetlen); - return hv_ringbuffer_write(channel, bufferlist, num_vecs, lock); + return hv_ringbuffer_write(channel, bufferlist, num_vecs); } EXPORT_SYMBOL(vmbus_sendpacket_ctl); @@ -718,12 +717,10 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel, u32 packetlen_aligned; struct kvec bufferlist[3]; u64 aligned_data = 0; - bool lock = channel->acquire_ring_lock; if (pagecount > MAX_PAGE_BUFFER_COUNT) return -EINVAL; - /* * Adjust the size down since vmbus_channel_packet_page_buffer is the * largest size we support @@ -755,7 +752,7 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel, bufferlist[2].iov_base = &aligned_data; bufferlist[2].iov_len = (packetlen_aligned - packetlen); - return hv_ringbuffer_write(channel, bufferlist, 3, lock); + return hv_ringbuffer_write(channel, bufferlist, 3); } EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer_ctl); @@ -790,7 +787,6 @@ int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel, u32 packetlen_aligned; struct kvec bufferlist[3]; u64 aligned_data = 0; - bool lock = channel->acquire_ring_lock; packetlen = desc_size + bufferlen; packetlen_aligned = ALIGN(packetlen, sizeof(u64)); @@ -810,7 +806,7 @@ int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel, bufferlist[2].iov_base = &aligned_data; bufferlist[2].iov_len = (packetlen_aligned - packetlen); - return hv_ringbuffer_write(channel, bufferlist, 3, lock); + return hv_ringbuffer_write(channel, bufferlist, 3); } EXPORT_SYMBOL_GPL(vmbus_sendpacket_mpb_desc); @@ -828,7 +824,6 @@ int vmbus_sendpacket_multipagebuffer(struct vmbus_channel *channel, u32 packetlen_aligned; struct kvec bufferlist[3]; u64 aligned_data = 0; - bool lock = channel->acquire_ring_lock; u32 pfncount = NUM_PAGES_SPANNED(multi_pagebuffer->offset, multi_pagebuffer->len); @@ -867,7 +862,7 @@ int vmbus_sendpacket_multipagebuffer(struct vmbus_channel *channel, bufferlist[2].iov_base = &aligned_data; bufferlist[2].iov_len = (packetlen_aligned - packetlen); - return hv_ringbuffer_write(channel, bufferlist, 3, lock); + return hv_ringbuffer_write(channel, bufferlist, 3); } EXPORT_SYMBOL_GPL(vmbus_sendpacket_multipagebuffer); diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index 49d77be90ca4..b196bdfea8c8 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -304,7 +304,6 @@ static struct vmbus_channel *alloc_channel(void) if (!channel) return NULL; - channel->acquire_ring_lock = true; spin_lock_init(&channel->inbound_lock); spin_lock_init(&channel->lock); diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index 0a884cef4193..0c9516d0bf36 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -277,8 +277,7 @@ int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info, void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info); int hv_ringbuffer_write(struct vmbus_channel *channel, - struct kvec *kv_list, - u32 kv_count, bool lock); + struct kvec *kv_list, u32 kv_count); int hv_ringbuffer_read(struct vmbus_channel *channel, void *buffer, u32 buflen, u32 *buffer_actual_len, diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c index 2325242029c5..04b5c5fce7ae 100644 --- a/drivers/hv/ring_buffer.c +++ b/drivers/hv/ring_buffer.c @@ -256,7 +256,7 @@ void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring
[PATCH 10/14] vmbus: fix spelling errors
Several spelling errors in comments
Signed-off-by: Stephen Hemminger
---
drivers/hv/channel.c | 10 +-
drivers/hv/hv_kvp.c | 10 +-
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index b0ce91a7ad24..a3c2289af9c3 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -333,7 +333,7 @@ static int create_gpadl_header(void *kbuffer, u32 size,
* Gpadl is u32 and we are using a pointer which could
* be 64-bit
* This is governed by the guest/host protocol and
-* so the hypervisor gurantees that this is ok.
+* so the hypervisor guarantees that this is ok.
*/
for (i = 0; i < pfncurr; i++)
gpadl_body->pfn[i] = slow_virt_to_phys(
@@ -380,7 +380,7 @@ static int create_gpadl_header(void *kbuffer, u32 size,
}
/*
- * vmbus_establish_gpadl - Estabish a GPADL for the specified buffer
+ * vmbus_establish_gpadl - Establish a GPADL for the specified buffer
*
* @channel: a channel
* @kbuffer: from kmalloc or vmalloc
@@ -732,7 +732,7 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel
*channel,
/* Setup the descriptor */
desc.type = VM_PKT_DATA_USING_GPA_DIRECT;
desc.flags = flags;
- desc.dataoffset8 = descsize >> 3; /* in 8-bytes grandularity */
+ desc.dataoffset8 = descsize >> 3; /* in 8-bytes granularity */
desc.length8 = (u16)(packetlen_aligned >> 3);
desc.transactionid = requestid;
desc.rangecount = pagecount;
@@ -791,7 +791,7 @@ int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
/* Setup the descriptor */
desc->type = VM_PKT_DATA_USING_GPA_DIRECT;
desc->flags = VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED;
- desc->dataoffset8 = desc_size >> 3; /* in 8-bytes grandularity */
+ desc->dataoffset8 = desc_size >> 3; /* in 8-bytes granularity */
desc->length8 = (u16)(packetlen_aligned >> 3);
desc->transactionid = requestid;
desc->rangecount = 1;
@@ -841,7 +841,7 @@ int vmbus_sendpacket_multipagebuffer(struct vmbus_channel
*channel,
/* Setup the descriptor */
desc.type = VM_PKT_DATA_USING_GPA_DIRECT;
desc.flags = VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED;
- desc.dataoffset8 = descsize >> 3; /* in 8-bytes grandularity */
+ desc.dataoffset8 = descsize >> 3; /* in 8-bytes granularity */
desc.length8 = (u16)(packetlen_aligned >> 3);
desc.transactionid = requestid;
desc.rangecount = 1;
diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index 7baad4083a47..5a60febe5301 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -56,7 +56,7 @@
*
* While the request/response protocol is guaranteed by the host, we further
* ensure this by serializing packet processing in this driver - we do not
- * read additional packets from the VMBUs until the current packet is fully
+ * read additional packets from the VMBUS until the current packet is fully
* handled.
*/
@@ -385,7 +385,7 @@ kvp_send_key(struct work_struct *dummy)
* the max lengths specified. We will however, reserve room
* for the string terminating character - in the utf16s_utf8s()
* function we limit the size of the buffer where the converted
-* string is placed to HV_KVP_EXCHANGE_MAX_*_SIZE -1 to gaurantee
+* string is placed to HV_KVP_EXCHANGE_MAX_*_SIZE -1 to guarantee
* that the strings can be properly terminated!
*/
@@ -519,7 +519,7 @@ kvp_respond_to_host(struct hv_kvp_msg *msg_to_host, int
error)
*/
if (error) {
/*
-* Something failed or we have timedout;
+* Something failed or we have timed out;
* terminate the current host-side iteration.
*/
goto response_done;
@@ -593,8 +593,8 @@ kvp_respond_to_host(struct hv_kvp_msg *msg_to_host, int
error)
* This callback is invoked when we get a KVP message from the host.
* The host ensures that only one KVP transaction can be active at a time.
* KVP implementation in Linux needs to forward the key to a user-mde
- * component to retrive the corresponding value. Consequently, we cannot
- * respond to the host in the conext of this callback. Since the host
+ * component to retrieve the corresponding value. Consequently, we cannot
+ * respond to the host in the context of this callback. Since the host
* guarantees that at most only one transaction can be active at a time,
* we stash away the transaction state in a set of global variables.
*/
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 06/14] vmbus: remove no longer used signal_policy
The explicit signal policy is no longer used. A different mechanism
will be added later when xmit_more is supported.
Signed-off-by: Stephen Hemminger
---
include/linux/hyperv.h | 18 --
1 file changed, 18 deletions(-)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 35328cfa0e80..b691a6530e85 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -670,11 +670,6 @@ struct hv_input_signal_event_buffer {
struct hv_input_signal_event event;
};
-enum hv_signal_policy {
- HV_SIGNAL_POLICY_DEFAULT = 0,
- HV_SIGNAL_POLICY_EXPLICIT,
-};
-
enum hv_numa_policy {
HV_BALANCED = 0,
HV_LOCALIZED,
@@ -837,13 +832,6 @@ struct vmbus_channel {
*/
struct list_head percpu_list;
/*
-* Host signaling policy: The default policy will be
-* based on the ring buffer state. We will also support
-* a policy where the client driver can have explicit
-* signaling control.
-*/
- enum hv_signal_policy signal_policy;
- /*
* On the channel send side, many of the VMBUS
* device drivers explicity serialize access to the
* outgoing ring buffer. Give more control to the
@@ -904,12 +892,6 @@ static inline bool is_hvsock_channel(const struct
vmbus_channel *c)
VMBUS_CHANNEL_TLNPI_PROVIDER_OFFER);
}
-static inline void set_channel_signal_state(struct vmbus_channel *c,
- enum hv_signal_policy policy)
-{
- c->signal_policy = policy;
-}
-
static inline void set_channel_affinity_state(struct vmbus_channel *c,
enum hv_numa_policy policy)
{
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 08/14] vmbus: remove unused kickq argument to sendpacket
Since sendpacket no longer uses kickq arguement remove it.
Remove it no longer used xmit_more in sendpacket in netvsc as well.
Signed-off-by: Stephen Hemminger
---
drivers/hv/channel.c| 17 +++--
drivers/net/hyperv/netvsc.c | 21 +++--
include/linux/hyperv.h | 6 ++
3 files changed, 12 insertions(+), 32 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 61e2cc1b653f..b0ce91a7ad24 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -643,8 +643,8 @@ void vmbus_close(struct vmbus_channel *channel)
EXPORT_SYMBOL_GPL(vmbus_close);
int vmbus_sendpacket_ctl(struct vmbus_channel *channel, void *buffer,
- u32 bufferlen, u64 requestid,
- enum vmbus_packet_type type, u32 flags, bool kick_q)
+u32 bufferlen, u64 requestid,
+enum vmbus_packet_type type, u32 flags)
{
struct vmpacket_descriptor desc;
u32 packetlen = sizeof(struct vmpacket_descriptor) + bufferlen;
@@ -692,7 +692,7 @@ int vmbus_sendpacket(struct vmbus_channel *channel, void
*buffer,
enum vmbus_packet_type type, u32 flags)
{
return vmbus_sendpacket_ctl(channel, buffer, bufferlen, requestid,
- type, flags, true);
+ type, flags);
}
EXPORT_SYMBOL(vmbus_sendpacket);
@@ -704,11 +704,9 @@ EXPORT_SYMBOL(vmbus_sendpacket);
* explicitly.
*/
int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel,
-struct hv_page_buffer pagebuffers[],
-u32 pagecount, void *buffer, u32 bufferlen,
-u64 requestid,
-u32 flags,
-bool kick_q)
+ struct hv_page_buffer pagebuffers[],
+ u32 pagecount, void *buffer, u32 bufferlen,
+ u64 requestid, u32 flags)
{
int i;
struct vmbus_channel_packet_page_buffer desc;
@@ -767,8 +765,7 @@ int vmbus_sendpacket_pagebuffer(struct vmbus_channel
*channel,
{
u32 flags = VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED;
return vmbus_sendpacket_pagebuffer_ctl(channel, pagebuffers, pagecount,
- buffer, bufferlen, requestid,
- flags, true);
+ buffer, bufferlen, requestid,
flags);
}
EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer);
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 5a1cc089acb7..e326e68f9f6d 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -723,8 +723,6 @@ static u32 netvsc_copy_to_send_buf(struct netvsc_device
*net_device,
char *dest = start + (section_index * net_device->send_section_size)
+ pend_size;
int i;
- bool is_data_pkt = (skb != NULL) ? true : false;
- bool xmit_more = (skb != NULL) ? skb->xmit_more : false;
u32 msg_size = 0;
u32 padding = 0;
u32 remain = packet->total_data_buflen % net_device->pkt_align;
@@ -732,7 +730,7 @@ static u32 netvsc_copy_to_send_buf(struct netvsc_device
*net_device,
packet->page_buf_cnt;
/* Add padding */
- if (is_data_pkt && xmit_more && remain &&
+ if (skb && skb->xmit_more && remain &&
!packet->cp_partial) {
padding = net_device->pkt_align - remain;
rndis_msg->msg_len += padding;
@@ -772,7 +770,6 @@ static inline int netvsc_send_pkt(
int ret;
struct hv_page_buffer *pgbuf;
u32 ring_avail = hv_ringbuf_avail_percent(&out_channel->outbound);
- bool xmit_more = (skb != NULL) ? skb->xmit_more : false;
nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT;
if (skb != NULL) {
@@ -796,16 +793,6 @@ static inline int netvsc_send_pkt(
if (out_channel->rescind)
return -ENODEV;
- /*
-* It is possible that once we successfully place this packet
-* on the ringbuffer, we may stop the queue. In that case, we want
-* to notify the host independent of the xmit_more flag. We don't
-* need to be precise here; in the worst case we may signal the host
-* unnecessarily.
-*/
- if (ring_avail < (RING_AVAIL_PERCENT_LOWATER + 1))
- xmit_more = false;
-
if (packet->page_buf_cnt) {
pgbuf = packet->cp_partial ? (*pb) +
packet->rmsg_pgcnt : (*pb);
@@ -815,15 +802,13 @@ static inline int netvsc_send_pkt(
&nvmsg,
sizeof(struct
nvsp_message),
Re: [PATCH v3 16/24] media: Add i.MX media core driver
On 01/23/2017 03:13 AM, Philipp Zabel wrote: Hi Steve, On Sun, 2017-01-22 at 18:31 -0800, Steve Longerbeam wrote: On 01/16/2017 05:47 AM, Philipp Zabel wrote: On Sat, 2017-01-14 at 14:46 -0800, Steve Longerbeam wrote: [...] +Unprocessed Video Capture: +-- + +Send frames directly from sensor to camera interface, with no +conversions: + +-> ipu_smfc -> camif I'd call this capture interface, this is not just for cameras. Or maybe idmac if you want to mirror hardware names? Camif is so named because it is the V4L2 user interface for video capture. I suppose it could be named "capif", but that doesn't role off the tongue quite as well. Agreed, capif sounds weird. I find camif a bit confusing though, because Samsung S3C has a camera interface that is actually called "CAMIF". how about simply "capture" ? That sounds good to me. done. This should really be handled by v4l2_pipeline_pm_use. I thought about this earlier, but v4l2_pipeline_pm_use() seems to be doing some other stuff that bothered me, at least that's what I remember. I will revisit this. I have used it with a tc358743 -> mipi-csi2 pipeline, it didn't cause any problems. It would be better to reuse and, if necessary, fix the existing infrastructure where available. I tried this API, by switching to v4l2_pipeline_pm_use() in camif open/release, and switched to v4l2_pipeline_link_notify() instead of imx_media_link_notify() in the media driver's media_device_ops. This API assumes the video device has an open file handle while the media links are being established. This doesn't work for me, I want to be able to establish the links using 'media-ctl -l', and that won't work unless there is an open file handle on the video capture device node. Also, I looked into calling v4l2_pipeline_pm_use() during imx_media_link_notify(), instead of imx_media_pipeline_set_power(). Again there are problems with that. First, v4l2_pipeline_pm_use() acquires the graph mutex, so it can't be called inside link_notify which already acquires that lock. The header for this function also clearly states it should only be called in open/release. So why not call it in open/release then? er, see above (?) 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. Because that is the precedence, all other media drivers do pipeline power on/off at link_notify. And v4l2_pipeline_link_notify() was written as a link_notify method. Shouldn't that rather be done for all subdevices in the pipeline when the corresponding capture device is opened? that won't work. There's no guarantee the links will be established at capture device open time. It seems to me that powering up the pipeline should be the last step before userspace actually starts the capture. Well, I'm ok with moving pipeline power on/off to start/stop streaming. I would actually prefer to do it then, I only chose at link_notify because of precedence. I'll look into it. Steve ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 05/14] vmbus: drop no longer used kick_q argument
The flag to cause notification of host is unused after
commit a01a291a282f7c2e ("Drivers: hv: vmbus: Base host signaling
strictly on the ring state"). Therefore remove it from the ring
buffer internal API.
Signed-off-by: Stephen Hemminger
---
drivers/hv/channel.c | 13 -
drivers/hv/hyperv_vmbus.h | 5 ++---
drivers/hv/ring_buffer.c | 8 +++-
3 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index de98dcf7ca09..fd73ce1340be 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -670,9 +670,7 @@ int vmbus_sendpacket_ctl(struct vmbus_channel *channel,
void *buffer,
bufferlist[2].iov_base = &aligned_data;
bufferlist[2].iov_len = (packetlen_aligned - packetlen);
- return hv_ringbuffer_write(channel, bufferlist, num_vecs,
- lock, kick_q);
-
+ return hv_ringbuffer_write(channel, bufferlist, num_vecs, lock);
}
EXPORT_SYMBOL(vmbus_sendpacket_ctl);
@@ -757,8 +755,7 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel
*channel,
bufferlist[2].iov_base = &aligned_data;
bufferlist[2].iov_len = (packetlen_aligned - packetlen);
- return hv_ringbuffer_write(channel, bufferlist, 3,
- lock, kick_q);
+ return hv_ringbuffer_write(channel, bufferlist, 3, lock);
}
EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer_ctl);
@@ -813,8 +810,7 @@ int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
bufferlist[2].iov_base = &aligned_data;
bufferlist[2].iov_len = (packetlen_aligned - packetlen);
- return hv_ringbuffer_write(channel, bufferlist, 3,
- lock, true);
+ return hv_ringbuffer_write(channel, bufferlist, 3, lock);
}
EXPORT_SYMBOL_GPL(vmbus_sendpacket_mpb_desc);
@@ -871,8 +867,7 @@ int vmbus_sendpacket_multipagebuffer(struct vmbus_channel
*channel,
bufferlist[2].iov_base = &aligned_data;
bufferlist[2].iov_len = (packetlen_aligned - packetlen);
- return hv_ringbuffer_write(channel, bufferlist, 3,
- lock, true);
+ return hv_ringbuffer_write(channel, bufferlist, 3, lock);
}
EXPORT_SYMBOL_GPL(vmbus_sendpacket_multipagebuffer);
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 669b01b03257..0a884cef4193 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -277,9 +277,8 @@ int hv_ringbuffer_init(struct hv_ring_buffer_info
*ring_info,
void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info);
int hv_ringbuffer_write(struct vmbus_channel *channel,
- struct kvec *kv_list,
- u32 kv_count, bool lock,
- bool kick_q);
+ struct kvec *kv_list,
+ u32 kv_count, bool lock);
int hv_ringbuffer_read(struct vmbus_channel *channel,
void *buffer, u32 buflen, u32 *buffer_actual_len,
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index e408886ef171..2325242029c5 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -77,8 +77,7 @@ u32 hv_end_read(struct hv_ring_buffer_info *rbi)
* host logic is fixed.
*/
-static void hv_signal_on_write(u32 old_write, struct vmbus_channel *channel,
- bool kick_q)
+static void hv_signal_on_write(u32 old_write, struct vmbus_channel *channel)
{
struct hv_ring_buffer_info *rbi = &channel->outbound;
@@ -257,8 +256,7 @@ void hv_ringbuffer_cleanup(struct hv_ring_buffer_info
*ring_info)
/* Write to the ring buffer. */
int hv_ringbuffer_write(struct vmbus_channel *channel,
- struct kvec *kv_list, u32 kv_count, bool lock,
- bool kick_q)
+ struct kvec *kv_list, u32 kv_count, bool lock)
{
int i = 0;
u32 bytes_avail_towrite;
@@ -322,7 +320,7 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
if (lock)
spin_unlock_irqrestore(&outring_info->ring_lock, flags);
- hv_signal_on_write(old_write, channel, kick_q);
+ hv_signal_on_write(old_write, channel);
if (channel->rescind)
return -ENODEV;
--
2.11.0
___
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
On 01/23/2017 05:38 PM, 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. Because that is the precedence, all other media drivers do pipeline power on/off at link_notify. And v4l2_pipeline_link_notify() was written as a link_notify method. Shouldn't that rather be done for all subdevices in the pipeline when the corresponding capture device is opened? that won't work. There's no guarantee the links will be established at capture device open time. ugh, maybe v4l2_pipeline_pm_use() would work at open/release. If there are no links yet, it would basically be a no-op. And stream on requires opening the device, and the pipeline links should be established by then, so this might be fine, looking into this too. Steve It seems to me that powering up the pipeline should be the last step before userspace actually starts the capture. Well, I'm ok with moving pipeline power on/off to start/stop streaming. I would actually prefer to do it then, I only chose at link_notify because of precedence. I'll look into it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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(-) -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 03/14] vmbus: use kernel bitops for traversing interrupt mask
Use standard kernel operations for find first set bit to traverse
the channel bit array. This has added benefit of speeding up
lookup on 64 bit and because it uses find first set instruction.
Signed-off-by: Stephen Hemminger
---
drivers/hv/channel.c | 8 ++-
drivers/hv/connection.c | 55 +++
drivers/hv/hyperv_vmbus.h | 16 --
drivers/hv/vmbus_drv.c| 4 +---
4 files changed, 29 insertions(+), 54 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 205c976037e1..de98dcf7ca09 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -47,12 +47,8 @@ void vmbus_setevent(struct vmbus_channel *channel)
* For channels marked as in "low latency" mode
* bypass the monitor page mechanism.
*/
- if ((channel->offermsg.monitor_allocated) &&
- (!channel->low_latency)) {
- /* Each u32 represents 32 channels */
- sync_set_bit(channel->offermsg.child_relid & 31,
- (unsigned long *) vmbus_connection.send_int_page +
- (channel->offermsg.child_relid >> 5));
+ if (channel->offermsg.monitor_allocated && !channel->low_latency) {
+ vmbus_send_interrupt(channel->offermsg.child_relid);
/* Get the child to parent monitor page */
monitorpage = vmbus_connection.monitor_pages[1];
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 307a5a8937f6..1766ef03e78d 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -379,17 +379,11 @@ static void process_chn_event(u32 relid)
*/
void vmbus_on_event(unsigned long data)
{
- u32 dword;
- u32 maxdword;
- int bit;
- u32 relid;
- u32 *recv_int_page = NULL;
- void *page_addr;
- int cpu = smp_processor_id();
- union hv_synic_event_flags *event;
+ unsigned long *recv_int_page;
+ u32 maxbits, relid;
if (vmbus_proto_version < VERSION_WIN8) {
- maxdword = MAX_NUM_CHANNELS_SUPPORTED >> 5;
+ maxbits = MAX_NUM_CHANNELS_SUPPORTED;
recv_int_page = vmbus_connection.recv_int_page;
} else {
/*
@@ -397,35 +391,24 @@ void vmbus_on_event(unsigned long data)
* can be directly checked to get the id of the channel
* that has the interrupt pending.
*/
- maxdword = HV_EVENT_FLAGS_DWORD_COUNT;
- page_addr = hv_context.synic_event_page[cpu];
- event = (union hv_synic_event_flags *)page_addr +
+ int cpu = smp_processor_id();
+ void *page_addr = hv_context.synic_event_page[cpu];
+ union hv_synic_event_flags *event
+ = (union hv_synic_event_flags *)page_addr +
VMBUS_MESSAGE_SINT;
- recv_int_page = event->flags32;
- }
-
+ maxbits = HV_EVENT_FLAGS_COUNT;
+ recv_int_page = event->flags;
+ }
- /* Check events */
- if (!recv_int_page)
+ if (unlikely(!recv_int_page))
return;
- for (dword = 0; dword < maxdword; dword++) {
- if (!recv_int_page[dword])
- continue;
- for (bit = 0; bit < 32; bit++) {
- if (sync_test_and_clear_bit(bit,
- (unsigned long *)&recv_int_page[dword])) {
- relid = (dword << 5) + bit;
-
- if (relid == 0)
- /*
-* Special case - vmbus
-* channel protocol msg
-*/
- continue;
+ for_each_set_bit(relid, recv_int_page, maxbits) {
+ if (sync_test_and_clear_bit(relid, recv_int_page)) {
+ /* Special case - vmbus channel protocol msg */
+ if (relid != 0)
process_chn_event(relid);
- }
}
}
}
@@ -491,12 +474,8 @@ void vmbus_set_event(struct vmbus_channel *channel)
{
u32 child_relid = channel->offermsg.child_relid;
- if (!channel->is_dedicated_interrupt) {
- /* Each u32 represents 32 channels */
- sync_set_bit(child_relid & 31,
- (unsigned long *)vmbus_connection.send_int_page +
- (child_relid >> 5));
- }
+ if (!channel->is_dedicated_interrupt)
+ vmbus_send_interrupt(child_relid);
hv_do_hypercall(HVCALL_SIGNAL_EVENT, channel->sig_event, NULL);
}
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index fd91045ad677..669b01b03257 100644
--- a/drivers/hv/hype
Re: [PATCH v3 20/24] media: imx: Add Camera Interface subdev driver
On 01/20/2017 06:38 AM, Hans Verkuil wrote:
On 01/07/2017 03:11 AM, Steve Longerbeam wrote:
+static int vidioc_querycap(struct file *file, void *fh,
+ struct v4l2_capability *cap)
+{
+ strncpy(cap->driver, "imx-media-camif", sizeof(cap->driver) - 1);
+ strncpy(cap->card, "imx-media-camif", sizeof(cap->card) - 1);
+ cap->bus_info[0] = 0;
Should be set to something like 'platform:imx-media-camif'. v4l2-compliance
should
complain about this.
Right, I've fixed this already as part of v4l2-compliance testing.
+ cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
+ cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
Set device_caps in struct video_device, then drop these two lines since
the core will set these up based on the device_caps field in struct
video_device.
done.
+
+static int camif_enum_input(struct file *file, void *fh,
+ struct v4l2_input *input)
+{
+ struct camif_priv *priv = video_drvdata(file);
+ struct imx_media_subdev *sensor;
+ int index = input->index;
+
+ sensor = imx_media_find_sensor(priv->md, &priv->sd.entity);
+ if (IS_ERR(sensor)) {
+ v4l2_err(&priv->sd, "no sensor attached\n");
+ return PTR_ERR(sensor);
+ }
+
+ if (index >= sensor->input.num)
+ return -EINVAL;
+
+ input->type = V4L2_INPUT_TYPE_CAMERA;
+ strncpy(input->name, sensor->input.name[index], sizeof(input->name));
+
+ if (index == priv->current_input) {
+ v4l2_subdev_call(sensor->sd, video, g_input_status,
+&input->status);
+ v4l2_subdev_call(sensor->sd, video, querystd, &input->std);
Wrong op, use g_tvnorms instead.
done.
Steve
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/8] staging: lustre: lnet: change wire protocol typedefs to proper structure
On Sat, 2017-01-21 at 19:40 -0500, James Simmons wrote: > The upstream kernel requires proper structures so > convert nearly all the LNet wire protocols typedefs in > the LNet core. Thanks. Perhaps s/\bWIRE_ATTR\b/__packed/g one day too ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [greybus-dev] [PATCH v2] staging: greybus: timesync: validate platform state callback
On 23-01-17, 16:32, Rui Miguel Silva wrote:
> When tearingdown timesync, and not in arche platform, the state platform
> callback is not initialized. That will trigger the following NULL
> dereferencing.
> CallTrace:
>
> ? gb_timesync_platform_unlock_bus+0x11/0x20 [greybus]
> gb_timesync_teardown+0x85/0xc0 [greybus]
> gb_timesync_svc_remove+0xab/0x190 [greybus]
> gb_svc_del+0x29/0x110 [greybus]
> gb_hd_del+0x14/0x20 [greybus]
> ap_disconnect+0x24/0x60 [gb_es2]
> usb_unbind_interface+0x7a/0x2c0
> __device_release_driver+0x96/0x150
> device_release_driver+0x1e/0x30
> bus_remove_device+0xe7/0x130
> device_del+0x116/0x230
> usb_disable_device+0x97/0x1f0
> usb_disconnect+0x80/0x260
> hub_event+0x5ca/0x10e0
> process_one_work+0x126/0x3b0
> worker_thread+0x55/0x4c0
> ? process_one_work+0x3b0/0x3b0
> kthread+0xc4/0xe0
> ? kthread_park+0xb0/0xb0
> ret_from_fork+0x22/0x30
>
> So, fix that by adding checks before use the callback.
>
> Fixes: 970dc85bd95d ("greybus: timesync: Add timesync core driver")
> Cc: # 4.9.x
> Signed-off-by: Rui Miguel Silva
> ---
> Greg,
> This patch does not contain the "commit upstream" snippet since you
> already removed the timesync code from the -next tree, even though it is still
> in 4.10-rc. However this will affect the greybus users of 4.9.x. If you
> meanwhile pull that remove to rc, this patch is only necessary in stable
> 4.9.x.
>
> v1->v2:
>- add Fixes tag to changelog
>Johan Hovold:
> - for symmetry with the lock, adjust the unlock_bus function
> accordantly.
Reviewed-by: Viresh Kumar
--
viresh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
lening bieden
Goede dag, Dit is Norton Finance aanbod. Bij Norton maken we arrangingment van een lening of remortgage eenvoudig, zelfs als je met pensioen, zelfstandige of als je slechte krediet geschiedenis of CCJs. Omdat we een makelaar en niet een bank, we zijn niet beperkt tot onze eigen producten. Wat betekent meer keuze en minder afwijzingen. Dus als je hebt krediet in het verleden geweigerd, zullen we willen om u te helpen. Bij Norton Finance zijn wij verplicht om de behandeling van onze klanten eerlijk en het verstrekken van een vriendelijke, flexibele en professionele service. We zullen de formaliteiten tot een minimum beperken en samen met u een snelle reactie op uw lening aanvraag te garanderen. Als trotse leden van zowel de AFB (Vereniging van Financiën makelaars) de AMI (Vereniging van Mortgage Tussenpersonen) en de FSA (Financial Services Authority) we zorgvuldig hun regels en voorschriften voor uw geruststelling en bescherming te volgen. En we lening tegen lage rente van 3%. Gelieve indien u geïnteresseerd bent in onze lening bieden vriendelijk contact met ons op via deze e-mail: nortonfinance...@gmail.com INFORMATIE NODIG Jullie namen... Adres: ... Telefoon: ... Benodigd Duur: ... Bezetting: ... Maandelijks Inkomen Level: Geslacht: ... Geboortedatum: Staat: .. Land: .. Doel: . Ontmoeting uw financiële behoeften is onze trots. Dr, Peter Ibrahim ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Drivers: hv: vmbus: finally fix hv_need_to_signal_on_read()
Commit a389fcfd2cb5 ("Drivers: hv: vmbus: Fix signaling logic in
hv_need_to_signal_on_read()")
added the proper mb(), but removed the test "prev_write_sz < pending_sz"
when making the signal decision.
As a result, the guest can signal the host unnecessarily,
and then the host can throttle the guest because the host
thinks the guest is buggy or malicious; finally the user
running stress test can perceive intermittent freeze of
the guest.
This patch brings back the test, and properly handles the
in-place consumption APIs used by NetVSC (see get_next_pkt_raw(),
put_pkt_raw() and commit_rd_index()).
Fixes: a389fcfd2cb5 ("Drivers: hv: vmbus: Fix signaling logic in
hv_need_to_signal_on_read()")
Signed-off-by: Dexuan Cui
Reported-by: Rolf Neugebauer
Tested-by: Rolf Neugebauer
Cc: "K. Y. Srinivasan"
Cc: Haiyang Zhang
Cc: Stephen Hemminger
Cc:
---
The patch also changes drivers/net/hyperv/netvsc.c (Haiyang Zhang
and Stephen Hemminger have known about this patch).
This is a must, because only applying the VMBus changes will
break the netvsc driver, so the patch should go through the
char-misc tree as a whole.
The changes to netvsc.c are small enough, and hence there should be
no conflict.
drivers/hv/ring_buffer.c| 1 +
drivers/net/hyperv/netvsc.c | 6 ++
include/linux/hyperv.h | 32 ++--
3 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index cd49cb1..308dbda 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -383,6 +383,7 @@ int hv_ringbuffer_read(struct vmbus_channel *channel,
return ret;
}
+ init_cached_read_index(channel);
next_read_location = hv_get_next_read_location(inring_info);
next_read_location = hv_copyfrom_ringbuffer(inring_info, &desc,
sizeof(desc),
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 5a1cc08..86e5749 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -1295,6 +1295,9 @@ void netvsc_channel_cb(void *context)
ndev = hv_get_drvdata(device);
buffer = get_per_channel_state(channel);
+ /* commit_rd_index() -> hv_signal_on_read() needs this. */
+ init_cached_read_index(channel);
+
do {
desc = get_next_pkt_raw(channel);
if (desc != NULL) {
@@ -1347,6 +1350,9 @@ void netvsc_channel_cb(void *context)
bufferlen = bytes_recvd;
}
+
+ init_cached_read_index(channel);
+
} while (1);
if (bufferlen > NETVSC_PACKET_SIZE)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 42fe43f..183efde 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -128,6 +128,7 @@ struct hv_ring_buffer_info {
u32 ring_data_startoffset;
u32 priv_write_index;
u32 priv_read_index;
+ u32 cached_read_index;
};
/*
@@ -180,6 +181,19 @@ static inline u32 hv_get_bytes_to_write(struct
hv_ring_buffer_info *rbi)
return write;
}
+static inline u32 hv_get_cached_bytes_to_write(
+ const struct hv_ring_buffer_info *rbi)
+{
+ u32 read_loc, write_loc, dsize, write;
+
+ dsize = rbi->ring_datasize;
+ read_loc = rbi->cached_read_index;
+ write_loc = rbi->ring_buffer->write_index;
+
+ write = write_loc >= read_loc ? dsize - (write_loc - read_loc) :
+ read_loc - write_loc;
+ return write;
+}
/*
* VMBUS version is 32 bit entity broken up into
* two 16 bit quantities: major_number. minor_number.
@@ -1488,7 +1502,7 @@ hv_get_ring_buffer(struct hv_ring_buffer_info *ring_info)
static inline void hv_signal_on_read(struct vmbus_channel *channel)
{
- u32 cur_write_sz;
+ u32 cur_write_sz, cached_write_sz;
u32 pending_sz;
struct hv_ring_buffer_info *rbi = &channel->inbound;
@@ -1512,12 +1526,24 @@ static inline void hv_signal_on_read(struct
vmbus_channel *channel)
cur_write_sz = hv_get_bytes_to_write(rbi);
- if (cur_write_sz >= pending_sz)
+ if (cur_write_sz < pending_sz)
+ return;
+
+ cached_write_sz = hv_get_cached_bytes_to_write(rbi);
+ if (cached_write_sz < pending_sz)
vmbus_setevent(channel);
return;
}
+static inline void
+init_cached_read_index(struct vmbus_channel *channel)
+{
+ struct hv_ring_buffer_info *rbi = &channel->inbound;
+
+ rbi->cached_read_index = rbi->ring_buffer->read_index;
+}
+
/*
* An API to support in-place processing of incoming VMBUS packets.
*/
@@ -1569,6 +1595,8 @@ static inline void put_pkt_raw(struct vmbus_channel
*channel,
* This call commits the read index and potentially signals the host.
* Here is the pattern for using the "in-place" consumption APIs:
*
+ * init_cached_read_index();
+ *
* while (get_next_pkt_raw() {
