Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
On Tue, Apr 04, 2017 at 05:44:05PM -0700, Steve Longerbeam wrote:
>
>
> On 04/04/2017 05:40 PM, Steve Longerbeam wrote:
> >
> >
> >On 04/04/2017 04:10 PM, Russell King - ARM Linux wrote:
> >>On Thu, Mar 30, 2017 at 07:25:49PM +0200, Philipp Zabel wrote:
> >>>The TVP5150 DT bindings specify a single output port (port 0) that
> >>>corresponds to the video output pad (pad 1, DEMOD_PAD_VID_OUT).
> >>>
> >>>Signed-off-by: Philipp Zabel
> >>>---
> >>>I'm trying to get this to work with a TVP5150 analog TV decoder, and the
> >>>first problem is that this device doesn't have pad 0 as its single
> >>>output pad. Instead, as a MEDIA_ENT_F_ATV_DECODER entity, it has for
> >>>pads (input, video out, vbi out, audio out), and video out is pad 1,
> >>>whereas the device tree only defines a single port (0).
> >>
> >>Looking at the patch, it's highlighted another review point with
> >>Steve's driver.
> >>
> >>>diff --git a/drivers/staging/media/imx/imx-media-dev.c
> >>>b/drivers/staging/media/imx/imx-media-dev.c
> >>>index 17e2386a3ca3a..c52d6ca797965 100644
> >>>--- a/drivers/staging/media/imx/imx-media-dev.c
> >>>+++ b/drivers/staging/media/imx/imx-media-dev.c
> >>>@@ -267,6 +267,15 @@ static int imx_media_create_link(struct
> >>>imx_media_dev *imxmd,
> >>> source_pad = link->local_pad;
> >>> sink_pad = link->remote_pad;
> >>>
> >>>+/*
> >>>+ * If the source subdev is an analog video decoder with a single
> >>>source
> >>>+ * port, assume that this port 0 corresponds to the
> >>>DEMOD_PAD_VID_OUT
> >>>+ * entity pad.
> >>>+ */
> >>>+if (source->entity.function == MEDIA_ENT_F_ATV_DECODER &&
> >>>+local_sd->num_sink_pads == 0 && local_sd->num_src_pads == 1)
> >>>+source_pad = DEMOD_PAD_VID_OUT;
> >>>+
> >>> v4l2_info(&imxmd->v4l2_dev, "%s: %s:%d -> %s:%d\n", __func__,
> >>> source->name, source_pad, sink->name, sink_pad);
> >>
> >>What is "local" and what is "remote" here? It seems that, in the case of
> >>a link being created with the sensor(etc), it's all back to front.
> >>
> >>Eg, I see locally:
> >>
> >>imx-media: imx_media_create_link: imx219 0-0010:0 -> imx6-mipi-csi2:0
> >>
> >>So here, "source" is the imx219 (the sensor), and sink is
> >>"imx6-mipi-csi2"
> >>(part of the iMX6 capture.) However, this makes "local_sd" the subdev of
> >>the sensor, and "remote_sd" the subdev of the CSI2 interface - which is
> >>totally back to front - this code is part of the iMX6 capture system,
> >>so "local" implies that it should be part of that, and the "remote" thing
> >>would be the sensor.
> >>
> >>Hence, this seems completely confused. I'd suggest that:
> >>
> >>(a) the "pad->pad.flags & MEDIA_PAD_FL_SINK" test in
> >>imx_media_create_link()
> >>is moved into imx_media_create_links(), and placed here instead:
> >>
> >>for (j = 0; j < num_pads; j++) {
> >>pad = &local_sd->pad[j];
> >>
> >>if (pad->pad.flags & MEDIA_PAD_FL_SINK)
> >>continue;
> >>
> >>...
> >>}
> >>
> >>as the pad isn't going to spontaneously change this flag while we
> >>consider each individual link.
> >
> >
> >Sure, I can do that. It would avoid iterating unnecessarily through the
> >pad's links if the pad is a sink.
> >
> >
> >> However, maybe the test should be:
> >>
> >>if (!(pad->pad.flags & MEDIA_PAD_FL_SOURCE))
> >>
> >>?
> >>
> >
> >maybe that is more intuitive.
> >
> >
> >>(b) the terms "local" and "remote" in imx_media_create_link() are
> >>replaced with "source" and "sink" respectively, since this will
> >>now be called with a guaranteed source pad.
> >
> >Agreed. I'll change the arg and local var names.
> >
> >>
> >>As for Philipp's solution, I'm not sure what the correct solution for
> >>something like this is. It depends how you view "hardware interface"
> >>as defined by video-interfaces.txt, and whether the pads on the TVP5150
> >>represent the hardware interfaces. If we take the view that the pads
> >>do represent hardware interfaces, then using the reg= property on the
> >>port node will solve this problem.
> >
> >And the missing port nodes would have to actually be defined first.
> >According to Philipp they aren't, only a single output port 0.
> >
> >
> >>
> >>If not, it would mean that we would have to have the iMX capture code
> >>scan the pads on the source device, looking for outputs - but that
> >>runs into a problem - if the source device has multiple outputs, does
> >>the reg= property specify the output pad index or the pad number,
> >
> >And how do we even know the data direction of a DT port. Is it an input,
> >an output, bidirectional? The OF graph parsing in imx-media-of.c can't
> >determine a port's direction if it encounters a device it doesn't
> >recognize that has multiple ports. For now that is not really a problem
> >because upstream from the video mux and csi-2 receiver it's expected
> >there will only be original sources of video with only one sour
Re: [PATCH 03/24] staging: ks7010: add BUG_ON() to catch programmer error
On Tue, Apr 04, 2017 at 09:59:30PM +1000, Tobin C. Harding wrote: > On Tue, Apr 04, 2017 at 01:21:33PM +0300, Dan Carpenter wrote: > > On Sun, Apr 02, 2017 at 10:18:09AM +1000, Tobin C. Harding wrote: > > > Constant is used to allocate memory for a buffer, then buffer is > > > filled upto 'size' which is passed as a parameter. If 'size' is bigger > > > than the constant then the buffer will overflow. Function has internal > > > linkage so this can only happen due to programmer error. BUG_ON() is > > > designed for catching these cases. Currently there is only one call > > > site and it is correct, adding BUG_ON() will potentially save > > > developer time if later changes to the code are incorrect. > > > > > > Use BUG_ON() to guard buffer write size in function with internal linkage. > > > > Nah. Adding these BUG_ON() calls everywhere is ugly. There are tons > > and tons of assumptions like this in the kernel and we'd have to add > > thousands of BUG_ON()s to cover everything. > > So do you mean we should we not make the assumptions, or that we should > check input arguments and return an error if assumptions are violated? > I read that BUG_ON() is not liked in many cases, is there any cases > where BUG_ON() is good to use? I tried to read up on it and from what > I could gather this was the use case. Did I miss-read? > The most important valid excuse for using BUG_ON() is for place where continuing in an invalid probably means we're going to corrupt the file system. It's easier to talk about bad uses of BUG_ON(). BUG_ON(p == NULL); This is wrong for several reasons, because first you should just audit all the code so that p is never NULL. Second if p is NULL and we dereference it then the kernel will Oops and generate a stack trace so the BUG_ON() doesn't add any value. Also when the kernel Oopses you still might be able to type, save a dmesg and debug the problem. If it does a hard stop then you have to take a photo on your phone and type in the data again which is obviously a pain. Finally, sprinkling extra BUG_ON() droppings here and there makes the code messier and hurts readability. Say it's a core kernel function and you can't audit all the callers because many of them will be written in the future, then probably it's better to just add proper error handling and perhaps a WARN_ON() for debugging. Device drivers normally can't corrupt the filesystem so probably they shouldn't be adding a bunch of BUG_ON() calls. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
On Wed, 2017-04-05 at 09:21 +0100, Russell King - ARM Linux wrote:
[...]
> > Actually what was I thinking, the TVP5150 is already an example of
> > such a device.
> >
> > All of this could be solved if there was some direction information
> > in port nodes.
>
> I disagree.
>
> Philipp identified that the TVP5150 has four pads:
>
> * Input pad
> * Video output pad
> * VBI output pad
> * Audio output pad
I didn't think hard enough about this earlier, but there are really only
two hardware interfaces on TVP5150. The ADC input, which can be
connected to either of two composite input pins (AIP1A, AIP1B), or use
both for s-video, and the digital output connected to pins (YOUT[7:0]).
VBI data can be transferred via the output pins during horizontal or
vertical blanking, if I understand correctly, or read from a FIFO via
I2C.
There is no apparent support for audio data whatsoever, and the only
mention of audio in the data manual is a vague reference about an "audio
interface available on the TVP5150" providing a clock to an audio
interface between an external audio decoder and the backend processor.
Further, commit 55606310e77f ("[media] tvp5150: create the expected
number of pads") creates DEMOD_NUM_PADS pads, but doesn't mention or
initialize the audio pad. It clearly expects the value of DEMOD_NUM_PADS
to be 3. And indeed the fourth pad was added later in commit
bddc418787cc ("[media] au0828: use standard demod pads struct").
So to me it looks like the VBI and audio pads should be removed from
TVP5150.
> So, it has one input and three outputs. How does marking the direction
> in the port node (which would indicate that there was a data flow out of
> TVP5150 into the iMX6 capture) help identify which of those pads should
> be used?
>
> It would eliminate the input pad, but you still have three output pads
> to choose from.
>
> So no, your idea can't work.
In this case, removal of the VBI and audio pads might make this work,
but in general this is true. In my opinion, to make this truly generic,
we need an interface to ask the driver which media entity pad a given
device tree port corresponds to, as there might not even be a single
media entity corresponding to all ports for more complex devices.
> As I already stated, I believe that this case is already covered by
> video-interfaces.txt:
>
> If more than
> one port is present in a device node or there is more than one endpoint at a
> port, or port node needs to be associated with a selected hardware
> interface,
> ^^
> a common scheme using '#address-cells', '#size-cells' and 'reg' properties
> is
> used.
>
> So, according to that, you do not need to have more than one port node
> to use the reg property - it's _either_ more than one port _or_ to
> select the hardware interface.
I don't think enforcing a 1:1 correspondence between device tree node
and media entity and enforcing port reg property == entity pad index is
a good idea in the long run. Binding errors are going to be made that
will have to be worked around at the driver level, and more complex
devices might have to create multiple media entities / v4l2 subdevices
with external ports that interface with the of graph.
> It all hinges on whether you consider the video output, VBI output or
> audio output to be separate hardware interfaces that the singular
> specified "port" node needs to select between.
>
> There's another reason why the TVP5150 binding looks wrong and broken,
> however. How does the audio output get routed to other parts of the
> system if you're using the video output, and how is that relationship
> defined? It's a v4l2 subdev pad, so it would need to be part of the
> v4l2 subdev graph. It sounds to me like the binding was created with
> a narrow focused "this is the board in front of me, it only has video
> wired up, I'm not going to consider other use cases" blinkered view.
I think the output part is accurate, as the audio pad is an artifact of
an unrelated change. I'm not so sure about the VBI pad, but I think that
shouldn't exist either. The input pad, on the other hand, not having any
of graph representation in the device tree seems a bit strange. There
was a custom binding for the inputs, that got quickly reverted:
https://patchwork.kernel.org/patch/8395521/
regards
Philipp
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
On Tue, 2017-04-04 at 15:11 -0700, Steve Longerbeam wrote: > > On 03/30/2017 10:25 AM, Philipp Zabel wrote: > > The TVP5150 DT bindings specify a single output port (port 0) that > > corresponds to the video output pad (pad 1, DEMOD_PAD_VID_OUT). > > > > Signed-off-by: Philipp Zabel > > --- > > I'm trying to get this to work with a TVP5150 analog TV decoder, and the > > first problem is that this device doesn't have pad 0 as its single > > output pad. Instead, as a MEDIA_ENT_F_ATV_DECODER entity, it has for > > pads (input, video out, vbi out, audio out), and video out is pad 1, > > whereas the device tree only defines a single port (0). > > Shouldn't the DT bindings define ports for these other pads? In this case, probably yes for the input pad, certainly no for the VBI/audio pads. See the other mail. > I haven't seen this documented anywhere, but shouldn't there > be a 1:1 correspondence between DT ports and media pads? Not in general. But imagine a dual HDMI->CSI2 converter, for example. We don't support this yet, but that might reasonably be described in the device tree as a single device with two output ports. But the internal representation would be two completely separate v4l2 subdevices. regards Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [lustre-devel] [PATCH] drivers/staging/lustre: Coding-guideline: Missing a blank line after declarations
On Apr 4, 2017, at 03:38, Dan Carpenter wrote:
>
> On Tue, Apr 04, 2017 at 02:45:26PM +0530, Pushkar Jambhlekar wrote:
>> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_page.c
>> b/drivers/staging/lustre/lustre/obdclass/cl_page.c
>> index cd9a40c..71fcc4c 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/cl_page.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/cl_page.c
>> @@ -482,6 +482,7 @@ void cl_page_disown0(const struct lu_env *env,
>> int cl_page_is_owned(const struct cl_page *pg, const struct cl_io *io)
>> {
>> struct cl_io *top = cl_io_top((struct cl_io *)io);
>> +
>> LINVRNT(cl_object_same(pg->cp_obj, io->ci_obj));
>> return pg->cp_state == CPS_OWNED && pg->cp_owner == top;
>> }
>
> This is not related to the patch but I don't understand CLOBINVRNT() and
> LINVRNT().
>
> # define LINVRNT(exp) LASSERT(exp)
> # define LINVRNT(exp) ((void)sizeof !!(exp))
>
> Why do we do the sizeof() instead of just an empty define? The compiler
> calculates the size at compile time and doesn't execute the expression
> so it's the same as an empty define so far as I can tell.
Even though sizeof() is evaluated at compile time and not runtime, it will at
least evaluate the expression "exp" at compile time. This is useful to avoid
"unused variable" warnings, syntax errors, etc. in that code when the more
expensive LINVRNT() checking is enabled, but is disabled most of the time.
With an empty expression this wouldn't happen at all, and errors may creep in.
Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel Corporation
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: lustre cleanup macros in libcfs_private.h
On Apr 3, 2017, at 15:13, Craig Inches wrote:
>
> This resolves a checkpatch warning that "Single statement macros should
> not use a do {} while (0) loop" by removing the loop and adjusting line
> length accordingly.
>
> Signed-off-by: Craig Inches
> ---
> .../lustre/include/linux/libcfs/libcfs_private.h | 51 +++---
> 1 file changed, 15 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> index 2dae857..150454f 100644
> --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> @@ -87,12 +87,9 @@ do {
> \
> #define LIBCFS_VMALLOC_SIZE (2 << PAGE_SHIFT) /* 2 pages */
> #endif
>
> -#define LIBCFS_ALLOC_PRE(size, mask) \
> -do { \
> - LASSERT(!in_interrupt() || \
> - ((size) <= LIBCFS_VMALLOC_SIZE && \
> - !gfpflags_allow_blocking(mask))); \
> -} while (0)
> +#define LIBCFS_ALLOC_PRE(size, mask) \
> + LASSERT(!in_interrupt() || ((size) <= LIBCFS_VMALLOC_SIZE \
> + && !gfpflags_allow_blocking(mask)))
(style) keep operators at the end of the previous line, rather than the start
of the continued line
>
> #define LIBCFS_ALLOC_POST(ptr, size) \
> do { \
> @@ -187,46 +184,28 @@ void cfs_array_free(void *vars);
> #if LASSERT_ATOMIC_ENABLED
>
> /** assert value of @a is equal to @v */
> -#define LASSERT_ATOMIC_EQ(a, v) \
> -do { \
> - LASSERTF(atomic_read(a) == v, \
> - "value: %d\n", atomic_read((a)));\
> -} while (0)
> +#define LASSERT_ATOMIC_EQ(a, v) LASSERTF(atomic_read(a) == v,
> \
> + "value: %d\n", atomic_read((a)))
Minor nit - in cases like this where you need to split the line anyway, it
is cleaner (IMHO) to keep the whole statement together:
#define LASSERT_ATOMIC_EQ(a, v) \
LASSERTF(atomic_read(a) == v, "value: %d\n", atomic_read((a)))
Cheers, Andreas
>
> /** assert value of @a is unequal to @v */
> -#define LASSERT_ATOMIC_NE(a, v) \
> -do { \
> - LASSERTF(atomic_read(a) != v, \
> - "value: %d\n", atomic_read((a)));\
> -} while (0)
> +#define LASSERT_ATOMIC_NE(a, v) LASSERTF(atomic_read(a) != v,
> \
> + "value: %d\n", atomic_read((a)))
>
> /** assert value of @a is little than @v */
> -#define LASSERT_ATOMIC_LT(a, v) \
> -do { \
> - LASSERTF(atomic_read(a) < v,\
> - "value: %d\n", atomic_read((a)));\
> -} while (0)
> +#define LASSERT_ATOMIC_LT(a, v) LASSERTF(atomic_read(a) < v, \
> + "value: %d\n", atomic_read((a)))
>
> /** assert value of @a is little/equal to @v */
> -#define LASSERT_ATOMIC_LE(a, v) \
> -do { \
> - LASSERTF(atomic_read(a) <= v, \
> - "value: %d\n", atomic_read((a)));\
> -} while (0)
> +#define LASSERT_ATOMIC_LE(a, v) LASSERTF(atomic_read(a) <= v,
> \
> + "value: %d\n", atomic_read((a)))
>
> /** assert value of @a is great than @v */
> -#define LASSERT_ATOMIC_GT(a, v) \
> -do { \
> - LASSERTF(atomic_read(a) > v,\
> - "value: %d\n", atomic_read((a)));\
> -} while (0)
> +#define LASSERT_ATOMIC_GT(a, v) LASSERTF(atomic_read(a) > v, \
> + "value: %d\n", atomic_read((a)))
>
> /** assert value of @a is great/equal to @v */
> -#define LASSERT_ATOMIC_GE(a, v) \
> -do { \
> - LASSERTF(atomic_read(a) >= v, \
> - "value: %d\n", atomic_read((a)));\
> -} while (0)
> +#define LASSERT_ATOMIC_GE(a, v) LASSERTF(atomic_read(a) >= v,
> \
> + "value: %d\n", atomic_read((a)))
>
> /** assert value of @a is great than @v1 and little than @v2 */
> #d
Re: [PATCH v6 17/39] platform: add video-multiplexer subdevice driver
Hi! > + * video stream multiplexer controlled via gpio or syscon > + * > + * Copyright (C) 2013 Pengutronix, Sascha Hauer > + * Copyright (C) 2016 Pengutronix, Philipp Zabel This is actually quite interesting. Same email address for two people... Plus, I believe this wants to say that copyright is with Pengutronix, not Sascha and Philipp. In that case you probably want to list copyright and authors separately? -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 19/39] media: Add i.MX media core driver
Hi! > +https://boundarydevices.com/products/nit6x_5mp I'd use /product/ in url, as it redirects there. > +https://boundarydevices.com/product/nit6x_5mp_mipi ..and for consistency. > +The following example configures a direct conversion pipeline to capture > +from the OV5640, transmitting on MIPI CSI-2 virtual channel 1. $sensorfmt > +can be any format supported by the OV5640. $sensordim is the frame > +dimension part of $sensorfmt (minus the mbus pixel code). $outputfmt can > +be any format supported by the ipu1_ic_prpenc entity at its output pad: > + > +.. code-block:: none > + > + # Setup links > + media-ctl -l "'ov5640 1-003c':0 -> 'imx6-mipi-csi2':0[1]" > + media-ctl -l "'imx6-mipi-csi2':2 -> 'ipu1_csi1':0[1]" > + media-ctl -l "'ipu1_csi1':1 -> 'ipu1_ic_prp':0[1]" > + media-ctl -l "'ipu1_ic_prp':1 -> 'ipu1_ic_prpenc':0[1]" > + media-ctl -l "'ipu1_ic_prpenc':1 -> 'ipu1_ic_prpenc capture':0[1]" > + # Configure pads > + media-ctl -V "'ov5640 1-003c':0 [fmt:$sensorfmt field:none]" > + media-ctl -V "'imx6-mipi-csi2':2 [fmt:$sensorfmt field:none]" > + media-ctl -V "'ipu1_csi1':1 [fmt:AYUV32/$sensordim field:none]" > + media-ctl -V "'ipu1_ic_prp':1 [fmt:AYUV32/$sensordim field:none]" > + media-ctl -V "'ipu1_ic_prpenc':1 [fmt:$outputfmt field:none]" > + > +Streaming can then begin on "ipu1_ic_prpenc capture" node. The v4l2-ctl > +tool can be used to select any supported YUV or RGB pixelformat on the > +capture device node. Nothing for you to fix here, but we should really create some library-like interface for media-ctl. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 19/39] media: Add i.MX media core driver
Hi! > +References > +-- > + > +[1] "i.MX 6Dual/6Quad Applications Processor Reference Manual" > +[2] "i.MX 6Solo/6DualLite Applications Processor Reference Manual" [2] is not present in the text above. [1] is there many times. What is purpose of this section? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 03/24] staging: ks7010: add BUG_ON() to catch programmer error
On Wed, Apr 05, 2017 at 11:59:31AM +0300, Dan Carpenter wrote: > On Tue, Apr 04, 2017 at 09:59:30PM +1000, Tobin C. Harding wrote: > > On Tue, Apr 04, 2017 at 01:21:33PM +0300, Dan Carpenter wrote: > > > On Sun, Apr 02, 2017 at 10:18:09AM +1000, Tobin C. Harding wrote: > > > > Constant is used to allocate memory for a buffer, then buffer is > > > > filled upto 'size' which is passed as a parameter. If 'size' is bigger > > > > than the constant then the buffer will overflow. Function has internal > > > > linkage so this can only happen due to programmer error. BUG_ON() is > > > > designed for catching these cases. Currently there is only one call > > > > site and it is correct, adding BUG_ON() will potentially save > > > > developer time if later changes to the code are incorrect. > > > > > > > > Use BUG_ON() to guard buffer write size in function with internal > > > > linkage. > > > > > > Nah. Adding these BUG_ON() calls everywhere is ugly. There are tons > > > and tons of assumptions like this in the kernel and we'd have to add > > > thousands of BUG_ON()s to cover everything. > > > > So do you mean we should we not make the assumptions, or that we should > > check input arguments and return an error if assumptions are violated? > > I read that BUG_ON() is not liked in many cases, is there any cases > > where BUG_ON() is good to use? I tried to read up on it and from what > > I could gather this was the use case. Did I miss-read? > > > > The most important valid excuse for using BUG_ON() is for place > where continuing in an invalid probably means we're going to corrupt the > file system. It's easier to talk about bad uses of BUG_ON(). > > BUG_ON(p == NULL); > > This is wrong for several reasons, because first you should just audit > all the code so that p is never NULL. Second if p is NULL and we > dereference it then the kernel will Oops and generate a stack trace so > the BUG_ON() doesn't add any value. Also when the kernel Oopses you > still might be able to type, save a dmesg and debug the problem. If > it does a hard stop then you have to take a photo on your phone and > type in the data again which is obviously a pain. Finally, sprinkling > extra BUG_ON() droppings here and there makes the code messier and hurts > readability. > > Say it's a core kernel function and you can't audit all the callers > because many of them will be written in the future, then probably it's > better to just add proper error handling and perhaps a WARN_ON() for > debugging. > > Device drivers normally can't corrupt the filesystem so probably they > shouldn't be adding a bunch of BUG_ON() calls. > > regards, > dan carpenter Got it. thanks, Tobin. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 17/39] platform: add video-multiplexer subdevice driver
Am Mittwoch, den 05.04.2017, 13:18 +0200 schrieb Pavel Machek: > Hi! > > > + * video stream multiplexer controlled via gpio or syscon > > + * > > + * Copyright (C) 2013 Pengutronix, Sascha Hauer > > + * Copyright (C) 2016 Pengutronix, Philipp Zabel > > This is actually quite interesting. Same email address for two > people... > > Plus, I believe this wants to say that copyright is with Pengutronix, > not Sascha and Philipp. In that case you probably want to list > copyright and authors separately? > Nope, copyright doesn't get transferred to the employer within the rules of the German "Urheberrecht", but stays at the original author of the code. Same email is just to ensure that any requests regarding this code get routed to the right people, even if someone leaves the company or is temporarily unavailable. kernel@ is a list for the Pengutronix kernel team. Regards, Lucas ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] unisys: visornic: Replace symbolic perms with octal
> -Original Message-
> From: Thomas Jespersen [mailto:laumann.tho...@gmail.com]
> Sent: Tuesday, April 4, 2017 3:15 PM
> To: Kershner, David A
> Cc: gre...@linuxfoundation.org; Sell, Timothy C
> ; Binder, David Anthony
> ; Frisch, Jon ;
> erik.arfvid...@unisys.com; *S-Par-Maintainer
> ; de...@driverdev.osuosl.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH] unisys: visornic: Replace symbolic perms with octal
>
Should the subject line say "staging: unisys: visornic" instead of just
"unisys: visornic:"?
Besides that, I'm fine with this patch and have tested it on s-Par.
David Kershner
> Replace symbolic permissions S_IRUSR and S_IWUSR for their octal
> counterparts
>
> Signed-off-by: Thomas Jespersen
> ---
> drivers/staging/unisys/visornic/visornic_main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/unisys/visornic/visornic_main.c
> b/drivers/staging/unisys/visornic/visornic_main.c
> index feece91..1008337 100644
> --- a/drivers/staging/unisys/visornic/visornic_main.c
> +++ b/drivers/staging/unisys/visornic/visornic_main.c
> @@ -2146,11 +2146,11 @@ static int visornic_init(void)
> if (!visornic_debugfs_dir)
> return err;
>
> - ret = debugfs_create_file("info", S_IRUSR, visornic_debugfs_dir,
> NULL,
> + ret = debugfs_create_file("info", 0400, visornic_debugfs_dir, NULL,
> &debugfs_info_fops);
> if (!ret)
> goto cleanup_debugfs;
> - ret = debugfs_create_file("enable_ints", S_IWUSR,
> visornic_debugfs_dir,
> + ret = debugfs_create_file("enable_ints", 0200, visornic_debugfs_dir,
> NULL, &debugfs_enable_ints_fops);
> if (!ret)
> goto cleanup_debugfs;
> --
> 2.10.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: fsl-mc/dpio: Fix early writing of valid bit
Commands written to the QMan software portals have a valid bit in the "verb" field of the command that, when set with the right value, notifies the hardware that the command is fully written and ready to be processed. The "verb" field should be the last one to be written in the swp command registers, after all other fields are filled in. The current implementation doesn't follow this rule for all commands, which may result in an incompletely configured command being processed by the hardware. Enforce the correct order of writes to avoid this situation. Signed-off-by: Ioana Radulescu --- drivers/staging/fsl-mc/bus/dpio/qbman-portal.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c b/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c index c75f546b24fa..2a3ea29d9b43 100644 --- a/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c +++ b/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c @@ -616,13 +616,16 @@ int qbman_swp_pull(struct qbman_swp *s, struct qbman_pull_desc *d) return -EBUSY; } s->vdq.storage = (void *)d->rsp_addr_virt; - d->tok = QMAN_DQ_TOKEN_VALID; p = qbman_get_cmd(s, QBMAN_CENA_SWP_VDQCR); - *p = *d; + p->numf = d->numf; + p->tok = QMAN_DQ_TOKEN_VALID; + p->dq_src = d->dq_src; + p->rsp_addr = d->rsp_addr; + p->rsp_addr_virt = d->rsp_addr_virt; dma_wmb(); /* Set the verb byte, have to substitute in the valid-bit */ - p->verb |= s->vdq.valid_bit; + p->verb = d->verb | s->vdq.valid_bit; s->vdq.valid_bit ^= QB_VALID_BIT; return 0; @@ -1004,7 +1007,6 @@ int qbman_swp_CDAN_set(struct qbman_swp *s, u16 channelid, return -EBUSY; /* Encode the caller-provided attributes */ - p->verb = 0; p->ch = cpu_to_le16(channelid); p->we = we_mask; if (cdan_en) -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] Staging: lustre cleanup macros in libcfs_private.h
This resolves a checkpatch warning that "Single statement macros should
not use a do {} while (0) loop" by removing the loop and adjusting line
length accordingly.
Signed-off-by: Craig Inches
---
Changes in v2:
- Kept statements together
- Kept operator on previous line
.../lustre/include/linux/libcfs/libcfs_private.h | 51 +++---
1 file changed, 15 insertions(+), 36 deletions(-)
diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
index 2dae857..e774c75 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
@@ -87,12 +87,9 @@ do {
\
#define LIBCFS_VMALLOC_SIZE(2 << PAGE_SHIFT) /* 2 pages */
#endif
-#define LIBCFS_ALLOC_PRE(size, mask) \
-do { \
- LASSERT(!in_interrupt() || \
- ((size) <= LIBCFS_VMALLOC_SIZE && \
-!gfpflags_allow_blocking(mask))); \
-} while (0)
+#define LIBCFS_ALLOC_PRE(size, mask) \
+ LASSERT(!in_interrupt() || ((size) <= LIBCFS_VMALLOC_SIZE &&\
+ !gfpflags_allow_blocking(mask)))
#define LIBCFS_ALLOC_POST(ptr, size) \
do { \
@@ -187,46 +184,28 @@ void cfs_array_free(void *vars);
#if LASSERT_ATOMIC_ENABLED
/** assert value of @a is equal to @v */
-#define LASSERT_ATOMIC_EQ(a, v) \
-do { \
- LASSERTF(atomic_read(a) == v, \
-"value: %d\n", atomic_read((a)));\
-} while (0)
+#define LASSERT_ATOMIC_EQ(a, v)\
+ LASSERTF(atomic_read(a) == v, "value: %d\n", atomic_read((a)))
/** assert value of @a is unequal to @v */
-#define LASSERT_ATOMIC_NE(a, v) \
-do { \
- LASSERTF(atomic_read(a) != v, \
-"value: %d\n", atomic_read((a)));\
-} while (0)
+#define LASSERT_ATOMIC_NE(a, v)\
+ LASSERTF(atomic_read(a) != v, "value: %d\n", atomic_read((a)))
/** assert value of @a is little than @v */
-#define LASSERT_ATOMIC_LT(a, v) \
-do { \
- LASSERTF(atomic_read(a) < v,\
-"value: %d\n", atomic_read((a)));\
-} while (0)
+#define LASSERT_ATOMIC_LT(a, v)\
+ LASSERTF(atomic_read(a) < v, "value: %d\n", atomic_read((a)))
/** assert value of @a is little/equal to @v */
-#define LASSERT_ATOMIC_LE(a, v) \
-do { \
- LASSERTF(atomic_read(a) <= v, \
-"value: %d\n", atomic_read((a)));\
-} while (0)
+#define LASSERT_ATOMIC_LE(a, v)\
+ LASSERTF(atomic_read(a) <= v, "value: %d\n", atomic_read((a)))
/** assert value of @a is great than @v */
-#define LASSERT_ATOMIC_GT(a, v) \
-do { \
- LASSERTF(atomic_read(a) > v,\
-"value: %d\n", atomic_read((a)));\
-} while (0)
+#define LASSERT_ATOMIC_GT(a, v)\
+ LASSERTF(atomic_read(a) > v, "value: %d\n", atomic_read((a)))
/** assert value of @a is great/equal to @v */
-#define LASSERT_ATOMIC_GE(a, v) \
-do { \
- LASSERTF(atomic_read(a) >= v, \
-"value: %d\n", atomic_read((a)));\
-} while (0)
+#define LASSERT_ATOMIC_GE(a, v)\
+ LASSERTF(atomic_read(a) >= v, "value: %d\n", atomic_read((a)))
/** assert value of @a is great than @v1 and little than @v2 */
#define LASSERT_ATOMIC_GT_LT(a, v1, v2) \
--
2.10.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 21/21] docs-rst: fix usb cross-references
As some USB documentation files got moved, adjust their cross-references to their new place. Signed-off-by: Mauro Carvalho Chehab --- Documentation/ABI/stable/sysfs-bus-usb| 2 +- Documentation/driver-api/usb/URB.rst | 2 ++ Documentation/driver-api/usb/callbacks.rst| 4 ++-- Documentation/driver-api/usb/error-codes.rst | 2 ++ Documentation/driver-api/usb/persist.rst | 2 ++ Documentation/driver-api/usb/power-management.rst | 2 +- Documentation/driver-api/usb/usb.rst | 4 ++-- Documentation/power/swsusp.txt| 2 +- drivers/staging/most/hdm-usb/hdm_usb.c| 2 +- drivers/usb/core/Kconfig | 2 +- 10 files changed, 15 insertions(+), 9 deletions(-) diff --git a/Documentation/ABI/stable/sysfs-bus-usb b/Documentation/ABI/stable/sysfs-bus-usb index 831f15d9672f..b832eeff 100644 --- a/Documentation/ABI/stable/sysfs-bus-usb +++ b/Documentation/ABI/stable/sysfs-bus-usb @@ -9,7 +9,7 @@ Description: hubs this facility is always enabled and their device directories will not contain this file. - For more information, see Documentation/usb/persist.txt. + For more information, see Documentation/driver-api/usb/persist.rst. What: /sys/bus/usb/devices/.../power/autosuspend Date: March 2007 diff --git a/Documentation/driver-api/usb/URB.rst b/Documentation/driver-api/usb/URB.rst index c4a141f29477..61a54da9fce9 100644 --- a/Documentation/driver-api/usb/URB.rst +++ b/Documentation/driver-api/usb/URB.rst @@ -1,3 +1,5 @@ +.. _usb-urb: + USB Request Block (URB) ~~~ diff --git a/Documentation/driver-api/usb/callbacks.rst b/Documentation/driver-api/usb/callbacks.rst index 93a8d53e27e7..2b80cf54bcc3 100644 --- a/Documentation/driver-api/usb/callbacks.rst +++ b/Documentation/driver-api/usb/callbacks.rst @@ -8,7 +8,7 @@ Usbcore will call into a driver through callbacks defined in the driver structure and through the completion handler of URBs a driver submits. Only the former are in the scope of this document. These two kinds of callbacks are completely independent of each other. Information on the -completion callback can be found in Documentation/usb/URB.txt. +completion callback can be found in :ref:`usb-urb`. The callbacks defined in the driver structure are: @@ -53,7 +53,7 @@ The callbacks defined in the driver structure are: The ioctl interface (2) should be used only if you have a very good reason. Sysfs is preferred these days. The PM callbacks are covered -separately in Documentation/usb/power-management.txt. +separately in :ref:`usb-power-management`. Calling conventions === diff --git a/Documentation/driver-api/usb/error-codes.rst b/Documentation/driver-api/usb/error-codes.rst index 9c11a0fd16cb..a3e84bfac776 100644 --- a/Documentation/driver-api/usb/error-codes.rst +++ b/Documentation/driver-api/usb/error-codes.rst @@ -1,3 +1,5 @@ +.. _usb-error-codes: + USB Error codes ~~~ diff --git a/Documentation/driver-api/usb/persist.rst b/Documentation/driver-api/usb/persist.rst index ea1b43f0559e..08cafc6292c1 100644 --- a/Documentation/driver-api/usb/persist.rst +++ b/Documentation/driver-api/usb/persist.rst @@ -1,3 +1,5 @@ +.. _usb-persist: + USB device persistence during system suspend diff --git a/Documentation/driver-api/usb/power-management.rst b/Documentation/driver-api/usb/power-management.rst index c068257f6d27..79beb807996b 100644 --- a/Documentation/driver-api/usb/power-management.rst +++ b/Documentation/driver-api/usb/power-management.rst @@ -328,7 +328,7 @@ possible to work around the hibernation-forces-disconnect problem by using the USB Persist facility.) The ``reset_resume`` method is used by the USB Persist facility (see -``Documentation/usb/persist.txt``) and it can also be used under certain +:ref:`usb-persist`) and it can also be used under certain circumstances when ``CONFIG_USB_PERSIST`` is not enabled. Currently, if a device is reset during a resume and the driver does not have a ``reset_resume`` method, the driver won't receive any notification about diff --git a/Documentation/driver-api/usb/usb.rst b/Documentation/driver-api/usb/usb.rst index 5ebaf669704c..6824089ef4c8 100644 --- a/Documentation/driver-api/usb/usb.rst +++ b/Documentation/driver-api/usb/usb.rst @@ -424,8 +424,8 @@ header. Unless noted otherwise, the ioctl requests described here will update the modification time on the usbfs file to which they are applied (unless they fail). A return of zero indicates success; otherwise, a -standard USB error code is returned. (These are documented in -``Documentation/usb/error-codes.txt`` in your kernel sources.) +standard USB error code is returned (These are documented in +:ref:`usb-error-codes`). Each of these files multiplexes access to several I/O streams,
RE: [PATCH] drivers/staging/lustre: Coding-guideline: Missing a blank line after declarations
> On Tue, Apr 04, 2017 at 02:45:26PM +0530, Pushkar Jambhlekar wrote:
> > diff --git a/drivers/staging/lustre/lustre/obdclass/cl_page.c
> > b/drivers/staging/lustre/lustre/obdclass/cl_page.c
> > index cd9a40c..71fcc4c 100644
> > --- a/drivers/staging/lustre/lustre/obdclass/cl_page.c
> > +++ b/drivers/staging/lustre/lustre/obdclass/cl_page.c
> > @@ -482,6 +482,7 @@ void cl_page_disown0(const struct lu_env *env,
> > int cl_page_is_owned(const struct cl_page *pg, const struct cl_io *io)
> > {
> > struct cl_io *top = cl_io_top((struct cl_io *)io);
> > +
> > LINVRNT(cl_object_same(pg->cp_obj, io->ci_obj));
> > return pg->cp_state == CPS_OWNED && pg->cp_owner == top; }
>
> This is not related to the patch but I don't understand CLOBINVRNT() and
> LINVRNT().
>
> # define LINVRNT(exp) LASSERT(exp)
> # define LINVRNT(exp) ((void)sizeof !!(exp))
>
> Why do we do the sizeof() instead of just an empty define? The compiler
> calculates the size at compile time and doesn't execute the expression so
> it's the
> same as an empty define so far as I can tell.
This is someone's attempt to avoid the unused variable warnings which would
occur
in some places when LINVRNT(exp) is defined as ((void)0).
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
Hello Philipp, On Wed, Apr 5, 2017 at 5:34 AM, Philipp Zabel wrote: > On Wed, 2017-04-05 at 09:21 +0100, Russell King - ARM Linux wrote: [snip] > I think the output part is accurate, as the audio pad is an artifact of > an unrelated change. I'm not so sure about the VBI pad, but I think that > shouldn't exist either. The input pad, on the other hand, not having any > of graph representation in the device tree seems a bit strange. There Agreed, there should be a OF graph representation (and also a MC representation) of the input PADs. The tvp5150 driver currently has hardcoded as input TVP5150_COMPOSITE1 (AIP1B), so it won't work for a board that has the composite connector wired to TVP5150_COMPOSITE0 (AIP1A) neither will work for S-Video (AIP1A + AIP1B). > was a custom binding for the inputs, that got quickly reverted: > https://patchwork.kernel.org/patch/8395521/ > Yes, that was my first attempt to have input connector support for tvp5150. The patches were merged without a detailed review of the DT bindings and latter were found to be wrong. Instead of having a driver specific DT binding, a generic binding that could be used by any device should be defined. The latest proposal was [0] but that was also found to be inadequate. After a lot of discussion on #v4l, the best approach we could come with was something like like [1]. But I was busy with other stuff and never found time to do the driver changes. By the way, only the DT bindings portion from the first approach got reverted but the patch reverting the driver changes never made to mainline. Could you please test if [2] doesn't cause any issues to you so the left over can be finally removed? > regards > Philipp > [0]: https://lkml.org/lkml/2016/4/12/983 [1]: https://hastebin.com/kadagidino.diff [2]: https://patchwork.kernel.org/patch/9472623/ Best regards, Javier ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
Em Wed, 05 Apr 2017 11:34:19 +0200
Philipp Zabel escreveu:
> On Wed, 2017-04-05 at 09:21 +0100, Russell King - ARM Linux wrote:
> [...]
> > > Actually what was I thinking, the TVP5150 is already an example of
> > > such a device.
> > >
> > > All of this could be solved if there was some direction information
> > > in port nodes.
> >
> > I disagree.
> >
> > Philipp identified that the TVP5150 has four pads:
> >
> > * Input pad
> > * Video output pad
> > * VBI output pad
> > * Audio output pad
>
> I didn't think hard enough about this earlier, but there are really only
> two hardware interfaces on TVP5150. The ADC input, which can be
> connected to either of two composite input pins (AIP1A, AIP1B), or use
> both for s-video, and the digital output connected to pins (YOUT[7:0]).
>
> VBI data can be transferred via the output pins during horizontal or
> vertical blanking, if I understand correctly, or read from a FIFO via
> I2C.
>
> There is no apparent support for audio data whatsoever, and the only
> mention of audio in the data manual is a vague reference about an "audio
> interface available on the TVP5150" providing a clock to an audio
> interface between an external audio decoder and the backend processor.
>
> Further, commit 55606310e77f ("[media] tvp5150: create the expected
> number of pads") creates DEMOD_NUM_PADS pads, but doesn't mention or
> initialize the audio pad. It clearly expects the value of DEMOD_NUM_PADS
> to be 3. And indeed the fourth pad was added later in commit
> bddc418787cc ("[media] au0828: use standard demod pads struct").
>
> So to me it looks like the VBI and audio pads should be removed from
> TVP5150.
There are a number of drivers that can work with different
types of TV demodulators. Typical examples of such hardware can be
found at em28xx, saa7134, cx88 drivers (among lots of other drivers).
Those drivers don't use the subdev API. Instead, they use a generic
helper function with sets the pipelines, based on the pad number.
The problem here is that, currently, both MC API and MC core
lacks a way to identify PAD ports per type, as the only information
that a bridge driver has is a pad number. So, in order for a
generic helper function to work, we had to hardcode pad numbers,
in a way that it would work for all possible types of demods.
It shouldn't be hard to add a "pad_type" information at media_pad
struct, but passing such info to userspace requires a new API
(we're calling it as "properties API"). Sakari was meant to send
us an updated RFC for it[1] with a patchset, back in 2015, but
this never happened.
[1] https://linuxtv.org/news.php?entry=2015-08-17.mchehab
Each vendor chooses the demod using some random criteria
(usually, they use whatever costs less by the time they create a
hardware model). Newer versions of the same hardware frequently
has a different model.
For example, in the case of em28xx driver, there are currently
several types of demods supported there, like (among other options):
- demods with tda9887, with is actually part of a tuner that has
an outputs for IF luminance, IF chrominance and IF audio;
- demods with xc3028, with is an integrated tuner + audio demod,
with outputs both video as a baseband signal and audio data via IF,
(or outputs a single IF signal, for Digital TV);
- demods with both xc3028 and msp3400. The last one transforms
audio IF into I2S;
- demods with saa711x (supported by saa7115 driver), with may or may
not have raw VBI and/or sliced VBI, depending on the specific model,
with is detected during driver's probe.
The generic function should be robust enough to handle all above
cases.
Without a way to report the PAD type to userspace, applications
that need to setup or recognize such pipelines need to hardcode the
pad numbers on userspace. That's, by the way, one of the issues why
it is currently impossible to write a full generic MC plugin at libv4l:
there's currently no way for userspace to recognize what type of
signals each PAD input or output carries.
So, in short, the tvp5150 demod doesn't decode audio, but there
are other demods that do it.
In the case of VBI, tvp5150 has actually two ways of reporting
it:
1) via YOUT[7:0] pins. VBI information is transmitted as a
set of raw samples, via an ancillary data block, during
vertical/horizontal blanking intervals. So, yes, it shares
the same hardware output, although the VBI contents are
actually multiplexed there. Please notice that not all
video out PADS encapsulate raw VBI the same way as tvp5150
(and some devices even don't support raw VBI, like saa7110 and
some models supported by saa7115 driver).
2) via an interrupt that indicates that it decoded VBI data. The
VBI information itself is there on FIFO, accessible via a set of
registers (see "VBI Data processor" chapter at the datasheet).
Currently, the driver doesn't support (2), because, at the time
I wrote the driver, I didn't find a way to read the interrupt
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
> Currently, the driver doesn't support (2), because, at the time > I wrote the driver, I didn't find a way to read the interrupts generated > by tvp5150 at em28xx[1], due to the lack of em28xx documentation, > but adding support for it shoudn't be hard. I may eventually do it > when I have some time to play with my ISEE hardware. For what it's worth, I doubt most of the em28xx designs have the tvp5150 interrupt request line connected in any way. You would likely have to poll the FIFO status register via I2C, or use the feature to embed the sliced data into as VANC data in the 656 output (as described in sec 3.9 of the tvp5150am1 spec). Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
Em Wed, 5 Apr 2017 11:39:06 -0400 Devin Heitmueller escreveu: > > Currently, the driver doesn't support (2), because, at the time > > I wrote the driver, I didn't find a way to read the interrupts generated > > by tvp5150 at em28xx[1], due to the lack of em28xx documentation, > > but adding support for it shoudn't be hard. I may eventually do it > > when I have some time to play with my ISEE hardware. > > For what it's worth, I doubt most of the em28xx designs have the > tvp5150 interrupt request line connected in any way. True. But, on embedded hardware, such line may be connected into the SoC. Actually, from the IGEPv3 expansion diagram: https://www.isee.biz/support/downloads/item/igepv2-expansion-rc-schematics The INT line is connected to CAM_IRQ. That's connected to GPIO_154 pin at OMAP3. So, on a first glance, it seems possible to use it, instead of polling. > You would likely > have to poll the FIFO status register via I2C, Yes, I considered this option when I wrote the driver. It could work, although it would likely have some performance drawback, as the driver would need to poll it at least 60 times per second. > or use the feature to > embed the sliced data into as VANC data in the 656 output (as > described in sec 3.9 of the tvp5150am1 spec). True, but the bridge driver would need to handle such data. I remember I looked on this when I wrote the driver, but I was unable to find a way for em28xx to parse (or forward) such data packets. Thanks, Mauro ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 28/38] Annotate hardware config module parameters in drivers/staging/media/
When the kernel is running in secure boot mode, we lock down the kernel to
prevent userspace from modifying the running kernel image. Whilst this
includes prohibiting access to things like /dev/mem, it must also prevent
access by means of configuring driver modules in such a way as to cause a
device to access or modify the kernel image.
To this end, annotate module_param* statements that refer to hardware
configuration and indicate for future reference what type of parameter they
specify. The parameter parser in the core sees this information and can
skip such parameters with an error message if the kernel is locked down.
The module initialisation then runs as normal, but just sees whatever the
default values for those parameters is.
Note that we do still need to do the module initialisation because some
drivers have viable defaults set in case parameters aren't specified and
some drivers support automatic configuration (e.g. PNP or PCI) in addition
to manually coded parameters.
This patch annotates drivers in drivers/staging/media/.
Suggested-by: Alan Cox
Signed-off-by: David Howells
cc: Mauro Carvalho Chehab
cc: Greg Kroah-Hartman
cc: linux-me...@vger.kernel.org
cc: de...@driverdev.osuosl.org
---
drivers/staging/media/lirc/lirc_sir.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/lirc/lirc_sir.c
b/drivers/staging/media/lirc/lirc_sir.c
index c6c3de94adaa..dde46dd8cabb 100644
--- a/drivers/staging/media/lirc/lirc_sir.c
+++ b/drivers/staging/media/lirc/lirc_sir.c
@@ -826,10 +826,10 @@ MODULE_AUTHOR("Milan Pikula");
#endif
MODULE_LICENSE("GPL");
-module_param(io, int, S_IRUGO);
+module_param_hw(io, int, ioport, S_IRUGO);
MODULE_PARM_DESC(io, "I/O address base (0x3f8 or 0x2f8)");
-module_param(irq, int, S_IRUGO);
+module_param_hw(irq, int, irq, S_IRUGO);
MODULE_PARM_DESC(irq, "Interrupt (4 or 3)");
module_param(threshold, int, S_IRUGO);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 29/38] Annotate hardware config module parameters in drivers/staging/speakup/
When the kernel is running in secure boot mode, we lock down the kernel to prevent userspace from modifying the running kernel image. Whilst this includes prohibiting access to things like /dev/mem, it must also prevent access by means of configuring driver modules in such a way as to cause a device to access or modify the kernel image. To this end, annotate module_param* statements that refer to hardware configuration and indicate for future reference what type of parameter they specify. The parameter parser in the core sees this information and can skip such parameters with an error message if the kernel is locked down. The module initialisation then runs as normal, but just sees whatever the default values for those parameters is. Note that we do still need to do the module initialisation because some drivers have viable defaults set in case parameters aren't specified and some drivers support automatic configuration (e.g. PNP or PCI) in addition to manually coded parameters. This patch annotates drivers in drivers/staging/speakup/. Suggested-by: Alan Cox Signed-off-by: David Howells cc: Greg Kroah-Hartman cc: spea...@linux-speakup.org cc: de...@driverdev.osuosl.org --- drivers/staging/speakup/speakup_acntpc.c |2 +- drivers/staging/speakup/speakup_dtlk.c |2 +- drivers/staging/speakup/speakup_keypc.c |2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/speakup/speakup_acntpc.c b/drivers/staging/speakup/speakup_acntpc.c index c7fab261d860..b6fbf9de1f85 100644 --- a/drivers/staging/speakup/speakup_acntpc.c +++ b/drivers/staging/speakup/speakup_acntpc.c @@ -307,7 +307,7 @@ static void accent_release(void) speakup_info.port_tts = 0; } -module_param_named(port, port_forced, int, 0444); +module_param_hw_named(port, port_forced, int, ioport, 0444); module_param_named(start, synth_acntpc.startup, short, 0444); MODULE_PARM_DESC(port, "Set the port for the synthesizer (override probing)."); diff --git a/drivers/staging/speakup/speakup_dtlk.c b/drivers/staging/speakup/speakup_dtlk.c index e2bf20806d8d..9c097fda07b0 100644 --- a/drivers/staging/speakup/speakup_dtlk.c +++ b/drivers/staging/speakup/speakup_dtlk.c @@ -378,7 +378,7 @@ static void dtlk_release(void) speakup_info.port_tts = 0; } -module_param_named(port, port_forced, int, 0444); +module_param_hw_named(port, port_forced, int, ioport, 0444); module_param_named(start, synth_dtlk.startup, short, 0444); MODULE_PARM_DESC(port, "Set the port for the synthesizer (override probing)."); diff --git a/drivers/staging/speakup/speakup_keypc.c b/drivers/staging/speakup/speakup_keypc.c index 10f4964782e2..e653b52175b8 100644 --- a/drivers/staging/speakup/speakup_keypc.c +++ b/drivers/staging/speakup/speakup_keypc.c @@ -309,7 +309,7 @@ static void keynote_release(void) synth_port = 0; } -module_param_named(port, port_forced, int, 0444); +module_param_hw_named(port, port_forced, int, ioport, 0444); module_param_named(start, synth_keypc.startup, short, 0444); MODULE_PARM_DESC(port, "Set the port for the synthesizer (override probing)."); ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 30/38] Annotate hardware config module parameters in drivers/staging/vme/
When the kernel is running in secure boot mode, we lock down the kernel to prevent userspace from modifying the running kernel image. Whilst this includes prohibiting access to things like /dev/mem, it must also prevent access by means of configuring driver modules in such a way as to cause a device to access or modify the kernel image. To this end, annotate module_param* statements that refer to hardware configuration and indicate for future reference what type of parameter they specify. The parameter parser in the core sees this information and can skip such parameters with an error message if the kernel is locked down. The module initialisation then runs as normal, but just sees whatever the default values for those parameters is. Note that we do still need to do the module initialisation because some drivers have viable defaults set in case parameters aren't specified and some drivers support automatic configuration (e.g. PNP or PCI) in addition to manually coded parameters. This patch annotates drivers in drivers/staging/vme/. Suggested-by: Alan Cox Signed-off-by: David Howells cc: Martyn Welch cc: Manohar Vanga cc: Greg Kroah-Hartman cc: de...@driverdev.osuosl.org --- drivers/staging/vme/devices/vme_pio2_core.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/vme/devices/vme_pio2_core.c b/drivers/staging/vme/devices/vme_pio2_core.c index 20a2d835fdaa..367535b4b77f 100644 --- a/drivers/staging/vme/devices/vme_pio2_core.c +++ b/drivers/staging/vme/devices/vme_pio2_core.c @@ -466,16 +466,16 @@ static void __exit pio2_exit(void) /* These are required for each board */ MODULE_PARM_DESC(bus, "Enumeration of VMEbus to which the board is connected"); -module_param_array(bus, int, &bus_num, 0444); +module_param_hw_array(bus, int, other, &bus_num, 0444); MODULE_PARM_DESC(base, "Base VME address for PIO2 Registers"); -module_param_array(base, long, &base_num, 0444); +module_param_hw_array(base, long, other, &base_num, 0444); MODULE_PARM_DESC(vector, "VME IRQ Vector (Lower 4 bits masked)"); -module_param_array(vector, int, &vector_num, 0444); +module_param_hw_array(vector, int, other, &vector_num, 0444); MODULE_PARM_DESC(level, "VME IRQ Level"); -module_param_array(level, int, &level_num, 0444); +module_param_hw_array(level, int, other, &level_num, 0444); MODULE_PARM_DESC(variant, "Last 4 characters of PIO2 board variant"); module_param_array(variant, charp, &variant_num, 0444); ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
>> For what it's worth, I doubt most of the em28xx designs have the >> tvp5150 interrupt request line connected in any way. > > True. But, on embedded hardware, such line may be connected into the > SoC. Actually, from the IGEPv3 expansion diagram: > > > https://www.isee.biz/support/downloads/item/igepv2-expansion-rc-schematics > > The INT line is connected to CAM_IRQ. That's connected to GPIO_154 pin > at OMAP3. > > So, on a first glance, it seems possible to use it, instead of polling. To be clear, I wasn't suggesting that the IRQ request line on the tvp5150 couldn't be supported in general (for example, for those embedded targets which have it wired up to a host processor). I'm just saying you shouldn't expect it to work on most (perhaps all) em28xx designs which have the tvp5150. In fact on some em28xx designs the pin is used as a GPIO output tied to a mux to control input selection. Hence blindly enabling the interrupt request line by default would do all sorts of bad things. >> You would likely >> have to poll the FIFO status register via I2C, > > Yes, I considered this option when I wrote the driver. It could work, > although it would likely have some performance drawback, as the driver > would need to poll it at least 60 times per second. > >> or use the feature to >> embed the sliced data into as VANC data in the 656 output (as >> described in sec 3.9 of the tvp5150am1 spec). > > True, but the bridge driver would need to handle such data. Correct. > I remember I looked on this when I wrote the driver, but I was > unable to find a way for em28xx to parse (or forward) such > data packets. I'm pretty sure it's possible, but I haven't looked at the datasheets in a number of years and don't recall the details. Hardware VBI splicing is supported by a number of decoders but it's rarely used on commodity PCs (the Conexant and NXP decoders support it as well). That said, I won't argue there might be some value on really low end platforms. All I would ask is that if you do introduce any such functionality into the tvp5150 driver for some embedded application that you please not break support for devices such as the em28xx. Thanks, Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
Em Wed, 5 Apr 2017 13:02:52 -0400 Devin Heitmueller escreveu: > > I remember I looked on this when I wrote the driver, but I was > > unable to find a way for em28xx to parse (or forward) such > > data packets. > > I'm pretty sure it's possible, but I haven't looked at the datasheets > in a number of years and don't recall the details. > > Hardware VBI splicing is supported by a number of decoders but it's > rarely used on commodity PCs (the Conexant and NXP decoders support it > as well). That said, I won't argue there might be some value on > really low end platforms. All I would ask is that if you do introduce > any such functionality into the tvp5150 driver for some embedded > application that you please not break support for devices such as the > em28xx. Yeah, sure. If I write such patchset[1], it will be optional, and will depend on a DT variable (or platform_data) setup that would tell what GPIO pin should be used for interrupt. Not providing it should either disable such feature or enable it via polling. [1] Please notice that I don't have any demand of doing it. Yet, I may do it for fun on my spare time:-) I added in the past an initial support for sliced VBI API on em28xx, with got reverted on changeset 1d179eeedc8cb48712bc236ec82ec6c63af42008, mainly due to the lack of such feature on tvp5150. So, such code was never tested (and likely need fixes/updates), but if I end by adding sliced VBI support on tvp5150 and on OMAP3, I may add support for it on em28xx too, using I2C poll. On such case, I'll likely add a modprobe parameter to enable such feature. Thanks, Mauro ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 17/39] platform: add video-multiplexer subdevice driver
On Wed 2017-04-05 13:58:39, Lucas Stach wrote: > Am Mittwoch, den 05.04.2017, 13:18 +0200 schrieb Pavel Machek: > > Hi! > > > > > + * video stream multiplexer controlled via gpio or syscon > > > + * > > > + * Copyright (C) 2013 Pengutronix, Sascha Hauer > > > + * Copyright (C) 2016 Pengutronix, Philipp Zabel > > > > This is actually quite interesting. Same email address for two > > people... > > > > Plus, I believe this wants to say that copyright is with Pengutronix, > > not Sascha and Philipp. In that case you probably want to list > > copyright and authors separately? > > > Nope, copyright doesn't get transferred to the employer within the rules > of the German "Urheberrecht", but stays at the original author of the > code. Ok, then I guess it should be Copyright (C) 2013 Sascha Hauer Work sponsored by Pengutronix, use as contact address or something? I know license change is not on the table, but I guess it is good to get legal stuff right. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 0/2] scsi: storvsc: Add support for FC rport
The updated patch set provides a way for drivers ( specifically storvsc in this case ) that expose virturalized fc devices but that do not expose rports to be manually scanned. This is done via creating a pseudo rport in storvsc and a corresponding dummy initiator rport role in the fc transport. Changes since v2: - Additional patch adding FC_PORT_ROLE_FCP_DUMMY_INITIATOR role to fc_transport - Changed storvsc rport role to FC_PORT_ROLE_FCP_DUMMY_INITIATOR Changes since v1: - Fix fc_rport_identifiers init [Stephen Hemminger] - Better error checking Cathy Avery (2): scsi: scsi_transport_fc: Add dummy initiator role to rport scsi: storvsc: Add support for FC rport. drivers/scsi/scsi_transport_fc.c | 10 ++ drivers/scsi/storvsc_drv.c | 23 ++- include/scsi/scsi_transport_fc.h | 1 + 3 files changed, 25 insertions(+), 9 deletions(-) -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 2/2] scsi: storvsc: Add support for FC rport.
Included in the current storvsc driver for Hyper-V is the ability
to access luns on an FC fabric via a virtualized fiber channel
adapter exposed by the Hyper-V host. The driver also attaches to
the FC transport to allow host and port names to be published under
/sys/class/fc_host/hostX. Current customer tools running on the VM
require that these names be available in the well known standard
location under fc_host/hostX.
This patch stubs in an rport per fc_host and sets its rport role
as FC_PORT_ROLE_FCP_DUMMY_INITIATOR to indicate to the fc_transport
that it is a pseudo rport in order to scan the scsi stack via
echo "- - -" > /sys/class/scsi_host/hostX/scan.
Signed-off-by: Cathy Avery
---
drivers/scsi/storvsc_drv.c | 23 ++-
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 016639d..1ec8579 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -476,6 +476,9 @@ struct storvsc_device {
*/
u64 node_name;
u64 port_name;
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+ struct fc_rport *rport;
+#endif
};
struct hv_host_device {
@@ -1823,19 +1826,27 @@ static int storvsc_probe(struct hv_device *device,
target = (device->dev_instance.b[5] << 8 |
device->dev_instance.b[4]);
ret = scsi_add_device(host, 0, target, 0);
- if (ret) {
- scsi_remove_host(host);
- goto err_out2;
- }
+ if (ret)
+ goto err_out3;
}
#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
if (host->transportt == fc_transport_template) {
+ struct fc_rport_identifiers ids = {
+ .roles = FC_PORT_ROLE_FCP_DUMMY_INITIATOR,
+ };
+
fc_host_node_name(host) = stor_device->node_name;
fc_host_port_name(host) = stor_device->port_name;
+ stor_device->rport = fc_remote_port_add(host, 0, &ids);
+ if (!stor_device->rport)
+ goto err_out3;
}
#endif
return 0;
+err_out3:
+ scsi_remove_host(host);
+
err_out2:
/*
* Once we have connected with the host, we would need to
@@ -1861,8 +1872,10 @@ static int storvsc_remove(struct hv_device *dev)
struct Scsi_Host *host = stor_device->host;
#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
- if (host->transportt == fc_transport_template)
+ if (host->transportt == fc_transport_template) {
+ fc_remote_port_delete(stor_device->rport);
fc_remove_host(host);
+ }
#endif
scsi_remove_host(host);
storvsc_dev_remove(dev);
--
2.5.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/2] scsi: scsi_transport_fc: Add dummy initiator role to rport
This patch allows scsi drivers that expose virturalized fibre channel
devices but that do not expose rports to successfully rescan the scsi
bus via echo "- - -" > /sys/class/scsi_host/hostX/scan.
Drivers can create a pseudo rport and indicate
FC_PORT_ROLE_FCP_DUMMY_INITIATOR as the rport's role in
fc_rport_identifiers. This insures that a valid scsi_target_id
is assigned to the newly created rport and it can meet the
requirements of fc_user_scan_tgt calling scsi_scan_target.
Signed-off-by: Cathy Avery
---
drivers/scsi/scsi_transport_fc.c | 10 ++
include/scsi/scsi_transport_fc.h | 1 +
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 2d753c9..de85602 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -289,9 +289,10 @@ static const struct {
u32 value;
char*name;
} fc_port_role_names[] = {
- { FC_PORT_ROLE_FCP_TARGET, "FCP Target" },
- { FC_PORT_ROLE_FCP_INITIATOR, "FCP Initiator" },
- { FC_PORT_ROLE_IP_PORT, "IP Port" },
+ { FC_PORT_ROLE_FCP_TARGET, "FCP Target" },
+ { FC_PORT_ROLE_FCP_INITIATOR, "FCP Initiator" },
+ { FC_PORT_ROLE_IP_PORT, "IP Port" },
+ { FC_PORT_ROLE_FCP_DUMMY_INITIATOR, "FCP Dummy Initiator" },
};
fc_bitfield_name_search(port_roles, fc_port_role_names)
@@ -2628,7 +2629,8 @@ fc_remote_port_create(struct Scsi_Host *shost, int
channel,
spin_lock_irqsave(shost->host_lock, flags);
rport->number = fc_host->next_rport_number++;
- if (rport->roles & FC_PORT_ROLE_FCP_TARGET)
+ if ((rport->roles & FC_PORT_ROLE_FCP_TARGET) ||
+ (rport->roles & FC_PORT_ROLE_FCP_DUMMY_INITIATOR))
rport->scsi_target_id = fc_host->next_target_id++;
else
rport->scsi_target_id = -1;
diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h
index b21b8aa5..6e208bb 100644
--- a/include/scsi/scsi_transport_fc.h
+++ b/include/scsi/scsi_transport_fc.h
@@ -162,6 +162,7 @@ enum fc_tgtid_binding_type {
#define FC_PORT_ROLE_FCP_TARGET0x01
#define FC_PORT_ROLE_FCP_INITIATOR 0x02
#define FC_PORT_ROLE_IP_PORT 0x04
+#define FC_PORT_ROLE_FCP_DUMMY_INITIATOR 0x08
/* The following are for compatibility */
#define FC_RPORT_ROLE_UNKNOWN FC_PORT_ROLE_UNKNOWN
--
2.5.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8192u: ieee80211: Fix space required after }.
This was reported by checkpatch.pl:
ERROR: space required after that close brace '}'
Signed-off-by: Valerio Genovese
---
drivers/staging/rtl8192u/ieee80211/rtl819x_BA.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_BA.h
b/drivers/staging/rtl8192u/ieee80211/rtl819x_BA.h
index 2c398ca..5218f27 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_BA.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_BA.h
@@ -31,7 +31,7 @@ typedef union _SEQUENCE_CONTROL{
struct {
u16 FragNum:4;
u16 SeqNum:12;
- }field;
+ } field;
}SEQUENCE_CONTROL, *PSEQUENCE_CONTROL;
typedef union _BA_PARAM_SET {
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] format-security: move static strings to const
While examining output from trial builds with -Wformat-security enabled,
many strings were found that should be defined as "const", or as a char
array instead of char pointer. This makes some static analysis easier,
by producing fewer false positives.
As these are all trivial changes, it seemed best to put them all in
a single patch rather than chopping them up per maintainer.
Signed-off-by: Kees Cook
---
arch/arm/mach-omap2/board-n8x0.c | 2 +-
arch/mips/dec/prom/init.c | 6 +++---
arch/mips/kernel/traps.c | 4 ++--
drivers/char/dsp56k.c | 2 +-
drivers/cpufreq/powernow-k8.c | 3 ++-
drivers/gpu/drm/drm_fb_helper.c | 2 +-
drivers/net/ethernet/amd/atarilance.c | 4 ++--
drivers/net/ethernet/amd/declance.c | 2 +-
drivers/net/ethernet/amd/sun3lance.c | 3 ++-
drivers/net/ethernet/cirrus/mac89x0.c | 2 +-
drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h | 2 +-
drivers/net/ethernet/natsemi/sonic.h | 2 +-
drivers/net/ethernet/toshiba/tc35815.c| 2 +-
drivers/net/fddi/defxx.c | 2 +-
drivers/net/hippi/rrunner.c | 3 ++-
drivers/staging/most/mostcore/core.c | 2 +-
drivers/tty/n_hdlc.c | 10 +-
drivers/tty/serial/st-asc.c | 2 +-
net/decnet/af_decnet.c| 3 ++-
19 files changed, 31 insertions(+), 27 deletions(-)
diff --git a/arch/arm/mach-omap2/board-n8x0.c b/arch/arm/mach-omap2/board-n8x0.c
index 6b6fda65fb3b..91272db09fa3 100644
--- a/arch/arm/mach-omap2/board-n8x0.c
+++ b/arch/arm/mach-omap2/board-n8x0.c
@@ -117,7 +117,7 @@ static struct musb_hdrc_platform_data tusb_data = {
static void __init n8x0_usb_init(void)
{
int ret = 0;
- static char announce[] __initdata = KERN_INFO "TUSB 6010\n";
+ static const char announce[] __initconst = KERN_INFO "TUSB 6010\n";
/* PM companion chip power control pin */
ret = gpio_request_one(TUSB6010_GPIO_ENABLE, GPIOF_OUT_INIT_LOW,
diff --git a/arch/mips/dec/prom/init.c b/arch/mips/dec/prom/init.c
index 4e1761e0a09a..d88eb7a6662b 100644
--- a/arch/mips/dec/prom/init.c
+++ b/arch/mips/dec/prom/init.c
@@ -88,7 +88,7 @@ void __init which_prom(s32 magic, s32 *prom_vec)
void __init prom_init(void)
{
extern void dec_machine_halt(void);
- static char cpu_msg[] __initdata =
+ static const char cpu_msg[] __initconst =
"Sorry, this kernel is compiled for a wrong CPU type!\n";
s32 argc = fw_arg0;
s32 *argv = (void *)fw_arg1;
@@ -111,7 +111,7 @@ void __init prom_init(void)
#if defined(CONFIG_CPU_R3000)
if ((current_cpu_type() == CPU_R4000SC) ||
(current_cpu_type() == CPU_R4400SC)) {
- static char r4k_msg[] __initdata =
+ static const char r4k_msg[] __initconst =
"Please recompile with \"CONFIG_CPU_R4x00 = y\".\n";
printk(cpu_msg);
printk(r4k_msg);
@@ -122,7 +122,7 @@ void __init prom_init(void)
#if defined(CONFIG_CPU_R4X00)
if ((current_cpu_type() == CPU_R3000) ||
(current_cpu_type() == CPU_R3000A)) {
- static char r3k_msg[] __initdata =
+ static const char r3k_msg[] __initconst =
"Please recompile with \"CONFIG_CPU_R3000 = y\".\n";
printk(cpu_msg);
printk(r3k_msg);
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index c7d17cfb32f6..2c717db50380 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -2256,8 +2256,8 @@ void set_handler(unsigned long offset, void *addr,
unsigned long size)
local_flush_icache_range(ebase + offset, ebase + offset + size);
}
-static char panic_null_cerr[] =
- "Trying to set NULL cache error exception handler";
+static const char panic_null_cerr[] =
+ "Trying to set NULL cache error exception handler\n";
/*
* Install uncached CPU exception handler.
diff --git a/drivers/char/dsp56k.c b/drivers/char/dsp56k.c
index 50aa9ba91f25..0d7b577e0ff0 100644
--- a/drivers/char/dsp56k.c
+++ b/drivers/char/dsp56k.c
@@ -489,7 +489,7 @@ static const struct file_operations dsp56k_fops = {
/** Init and module functions **/
-static char banner[] __initdata = KERN_INFO "DSP56k driver installed\n";
+static const char banner[] __initconst = KERN_INFO "DSP56k driver installed\n";
static int __init dsp56k_init_driver(void)
{
diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
index 0b5bf135b090..062d71434e47 100644
--- a/drivers/cpufreq/powernow-k8.c
+++ b/drivers/cpufreq/powernow-k8.c
@@ -1171,7 +1171,8 @@ static struct cpufreq_driver cpufreq_amd64_driver = {
static void __request_acpi_cpufreq(void)
{
-
Re: [PATCH] staging: rtl8192u: ieee80211: Fix space required after }.
On Wed, 2017-04-05 at 21:23 +0200, Valerio Genovese wrote:
> This was reported by checkpatch.pl:
> ERROR: space required after that close brace '}'
>
> Signed-off-by: Valerio Genovese
> ---
> drivers/staging/rtl8192u/ieee80211/rtl819x_BA.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_BA.h
> b/drivers/staging/rtl8192u/ieee80211/rtl819x_BA.h
> index 2c398ca..5218f27 100644
> --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_BA.h
> +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_BA.h
> @@ -31,7 +31,7 @@ typedef union _SEQUENCE_CONTROL{
> struct {
> u16 FragNum:4;
> u16 SeqNum:12;
> - }field;
> + } field;
> }SEQUENCE_CONTROL, *PSEQUENCE_CONTROL;
What about the one on the next line?
$ ./scripts/checkpatch.pl -f drivers/staging/rtl8192u/ieee80211/rtl819x_BA.h
--types=spacing --terse
drivers/staging/rtl8192u/ieee80211/rtl819x_BA.h:29: WARNING: missing space
after union definition
drivers/staging/rtl8192u/ieee80211/rtl819x_BA.h:34: ERROR: space required after
that close brace '}'
drivers/staging/rtl8192u/ieee80211/rtl819x_BA.h:35: ERROR: space required after
that close brace '}'
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
