[PATCH] Video/UVC: Fix unaligned exceptions in uvc video driver. - Kernel

This is a discussion on [PATCH] Video/UVC: Fix unaligned exceptions in uvc video driver. - Kernel ; From: Michael Hennerich buffer can be odd aligned on some NOMMU machine such as Blackfin Signed-off-by: Michael Hennerich Signed-off-by: Bryan Wu --- drivers/media/video/uvc/uvc_driver.c | 37 +++++++++++++++++---------------- 1 files changed, 19 insertions(+), 18 deletions(-) diff --git a/drivers/media/video/uvc/uvc_driver.c b/drivers/media/video/uvc/uvc_driver.c index d7ad060..9b4f469 100644 ...

+ Reply to Thread
Results 1 to 5 of 5

Thread: [PATCH] Video/UVC: Fix unaligned exceptions in uvc video driver.

  1. [PATCH] Video/UVC: Fix unaligned exceptions in uvc video driver.

    From: Michael Hennerich

    buffer can be odd aligned on some NOMMU machine such as Blackfin

    Signed-off-by: Michael Hennerich
    Signed-off-by: Bryan Wu
    ---
    drivers/media/video/uvc/uvc_driver.c | 37 +++++++++++++++++----------------
    1 files changed, 19 insertions(+), 18 deletions(-)

    diff --git a/drivers/media/video/uvc/uvc_driver.c b/drivers/media/video/uvc/uvc_driver.c
    index d7ad060..9b4f469 100644
    --- a/drivers/media/video/uvc/uvc_driver.c
    +++ b/drivers/media/video/uvc/uvc_driver.c
    @@ -31,6 +31,7 @@
    #include
    #include
    #include
    +#include
    #include

    #include
    @@ -432,20 +433,20 @@ static int uvc_parse_format(struct uvc_device *dev,

    frame->bFrameIndex = buffer[3];
    frame->bmCapabilities = buffer[4];
    - frame->wWidth = le16_to_cpup((__le16 *)&buffer[5]);
    - frame->wHeight = le16_to_cpup((__le16 *)&buffer[7]);
    - frame->dwMinBitRate = le32_to_cpup((__le32 *)&buffer[9]);
    - frame->dwMaxBitRate = le32_to_cpup((__le32 *)&buffer[13]);
    + frame->wWidth = le16_to_cpu(get_unaligned((__le16 *) &buffer[5]));
    + frame->wHeight = le16_to_cpu(get_unaligned((__le16 *) &buffer[7]));
    + frame->dwMinBitRate = le32_to_cpu(get_unaligned((__le32 *) &buffer[9]));
    + frame->dwMaxBitRate = le32_to_cpu(get_unaligned((__le32 *) &buffer[13]));
    if (ftype != VS_FRAME_FRAME_BASED) {
    frame->dwMaxVideoFrameBufferSize =
    - le32_to_cpup((__le32 *)&buffer[17]);
    + le32_to_cpu(get_unaligned((__le32 *) &buffer[17]));
    frame->dwDefaultFrameInterval =
    - le32_to_cpup((__le32 *)&buffer[21]);
    + le32_to_cpu(get_unaligned((__le32 *) &buffer[21]));
    frame->bFrameIntervalType = buffer[25];
    } else {
    frame->dwMaxVideoFrameBufferSize = 0;
    frame->dwDefaultFrameInterval =
    - le32_to_cpup((__le32 *)&buffer[17]);
    + le32_to_cpu(get_unaligned((__le32 *) &buffer[17]));
    frame->bFrameIntervalType = buffer[21];
    }
    frame->dwFrameInterval = *intervals;
    @@ -468,7 +469,7 @@ static int uvc_parse_format(struct uvc_device *dev,
    * some other divisions by zero which could happen.
    */
    for (i = 0; i < n; ++i) {
    - interval = le32_to_cpup((__le32 *)&buffer[26+4*i]);
    + interval = le32_to_cpu(get_unaligned((__le32 *) &buffer[26+4*i]));
    *(*intervals)++ = interval ? interval : 1;
    }

    @@ -814,7 +815,7 @@ static int uvc_parse_vendor_control(struct uvc_device *dev,
    memcpy(unit->extension.guidExtensionCode, &buffer[4], 16);
    unit->extension.bNumControls = buffer[20];
    unit->extension.bNrInPins =
    - le16_to_cpup((__le16 *)&buffer[21]);
    + le16_to_cpu(get_unaligned((__le16 *) &buffer[21]));
    unit->extension.baSourceID = (__u8 *)unit + sizeof *unit;
    memcpy(unit->extension.baSourceID, &buffer[22], p);
    unit->extension.bControlSize = buffer[22+p];
    @@ -858,8 +859,8 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
    return -EINVAL;
    }

    - dev->uvc_version = le16_to_cpup((__le16 *)&buffer[3]);
    - dev->clock_frequency = le32_to_cpup((__le32 *)&buffer[7]);
    + dev->uvc_version = le16_to_cpu(get_unaligned((__le16 *) &buffer[3]));
    + dev->clock_frequency = le32_to_cpu(get_unaligned((__le32 *) &buffer[7]));

    /* Parse all USB Video Streaming interfaces. */
    for (i = 0; i < n; ++i) {
    @@ -886,7 +887,7 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
    /* Make sure the terminal type MSB is not null, otherwise it
    * could be confused with a unit.
    */
    - type = le16_to_cpup((__le16 *)&buffer[4]);
    + type = le16_to_cpu(get_unaligned((__le16 *) &buffer[4]));
    if ((type & 0xff00) == 0) {
    uvc_trace(UVC_TRACE_DESCR, "device %d videocontrol "
    "interface %d INPUT_TERMINAL %d has invalid "
    @@ -928,11 +929,11 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
    term->camera.bControlSize = n;
    term->camera.bmControls = (__u8 *)term + sizeof *term;
    term->camera.wObjectiveFocalLengthMin =
    - le16_to_cpup((__le16 *)&buffer[8]);
    + le16_to_cpu(get_unaligned((__le16 *) &buffer[8]));
    term->camera.wObjectiveFocalLengthMax =
    - le16_to_cpup((__le16 *)&buffer[10]);
    + le16_to_cpu(get_unaligned((__le16 *) &buffer[10]));
    term->camera.wOcularFocalLength =
    - le16_to_cpup((__le16 *)&buffer[12]);
    + le16_to_cpu(get_unaligned((__le16 *) &buffer[12]));
    memcpy(term->camera.bmControls, &buffer[15], n);
    } else if (UVC_ENTITY_TYPE(term) == ITT_MEDIA_TRANSPORT_INPUT) {
    term->media.bControlSize = n;
    @@ -968,7 +969,7 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
    /* Make sure the terminal type MSB is not null, otherwise it
    * could be confused with a unit.
    */
    - type = le16_to_cpup((__le16 *)&buffer[4]);
    + type = le16_to_cpu(get_unaligned((__le16 *) &buffer[4]));
    if ((type & 0xff00) == 0) {
    uvc_trace(UVC_TRACE_DESCR, "device %d videocontrol "
    "interface %d OUTPUT_TERMINAL %d has invalid "
    @@ -1042,7 +1043,7 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
    unit->type = buffer[2];
    unit->processing.bSourceID = buffer[4];
    unit->processing.wMaxMultiplier =
    - le16_to_cpup((__le16 *)&buffer[5]);
    + le16_to_cpu(get_unaligned((__le16 *) &buffer[5]));
    unit->processing.bControlSize = buffer[7];
    unit->processing.bmControls = (__u8 *)unit + sizeof *unit;
    memcpy(unit->processing.bmControls, &buffer[8], n);
    @@ -1078,7 +1079,7 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
    memcpy(unit->extension.guidExtensionCode, &buffer[4], 16);
    unit->extension.bNumControls = buffer[20];
    unit->extension.bNrInPins =
    - le16_to_cpup((__le16 *)&buffer[21]);
    + le16_to_cpu(get_unaligned((__le16 *) &buffer[21]));
    unit->extension.baSourceID = (__u8 *)unit + sizeof *unit;
    memcpy(unit->extension.baSourceID, &buffer[22], p);
    unit->extension.bControlSize = buffer[22+p];
    --
    1.5.6.3
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH] Video/UVC: Fix unaligned exceptions in uvc video driver.

    Hi Bryan, Michael,

    Thanks for the patch.

    On Thursday 06 November 2008, Bryan Wu wrote:
    > From: Michael Hennerich
    >
    > buffer can be odd aligned on some NOMMU machine such as Blackfin


    The comment is a bit misleading. Buffers can be odd-aligned independently off
    the machine type. The issue comes from machines that can't access unaligned
    memory. Something like "Fix access to unaligned memory" would be better.

    > Signed-off-by: Michael Hennerich
    > Signed-off-by: Bryan Wu
    > ---
    > drivers/media/video/uvc/uvc_driver.c | 37
    > +++++++++++++++++---------------- 1 files changed, 19 insertions(+), 18
    > deletions(-)
    >
    > diff --git a/drivers/media/video/uvc/uvc_driver.c
    > b/drivers/media/video/uvc/uvc_driver.c index d7ad060..9b4f469 100644
    > --- a/drivers/media/video/uvc/uvc_driver.c
    > +++ b/drivers/media/video/uvc/uvc_driver.c
    > @@ -31,6 +31,7 @@
    > #include
    > #include
    > #include
    > +#include
    > #include
    >
    > #include
    > @@ -432,20 +433,20 @@ static int uvc_parse_format(struct uvc_device *dev,
    >
    > frame->bFrameIndex = buffer[3];
    > frame->bmCapabilities = buffer[4];
    > - frame->wWidth = le16_to_cpup((__le16 *)&buffer[5]);
    > - frame->wHeight = le16_to_cpup((__le16 *)&buffer[7]);
    > - frame->dwMinBitRate = le32_to_cpup((__le32 *)&buffer[9]);
    > - frame->dwMaxBitRate = le32_to_cpup((__le32 *)&buffer[13]);
    > + frame->wWidth = le16_to_cpu(get_unaligned((__le16 *) &buffer[5]));


    What about using get_unaligned_le16 and get_unaligned_le32 directly ? Lines
    would be shorter and could be kept behind the 80 columns limit more easily.

    Tell me if you want to resubmit or if I should make the modification myself
    (including the patch description).

    > + frame->wHeight = le16_to_cpu(get_unaligned((__le16 *) &buffer[7]));
    > + frame->dwMinBitRate = le32_to_cpu(get_unaligned((__le32 *) &buffer[9]));
    > + frame->dwMaxBitRate = le32_to_cpu(get_unaligned((__le32 *)
    > &buffer[13])); if (ftype != VS_FRAME_FRAME_BASED) {
    > frame->dwMaxVideoFrameBufferSize =
    > - le32_to_cpup((__le32 *)&buffer[17]);
    > + le32_to_cpu(get_unaligned((__le32 *) &buffer[17]));
    > frame->dwDefaultFrameInterval =
    > - le32_to_cpup((__le32 *)&buffer[21]);
    > + le32_to_cpu(get_unaligned((__le32 *) &buffer[21]));
    > frame->bFrameIntervalType = buffer[25];
    > } else {
    > frame->dwMaxVideoFrameBufferSize = 0;
    > frame->dwDefaultFrameInterval =
    > - le32_to_cpup((__le32 *)&buffer[17]);
    > + le32_to_cpu(get_unaligned((__le32 *) &buffer[17]));
    > frame->bFrameIntervalType = buffer[21];
    > }
    > frame->dwFrameInterval = *intervals;
    > @@ -468,7 +469,7 @@ static int uvc_parse_format(struct uvc_device *dev,
    > * some other divisions by zero which could happen.
    > */
    > for (i = 0; i < n; ++i) {
    > - interval = le32_to_cpup((__le32 *)&buffer[26+4*i]);
    > + interval = le32_to_cpu(get_unaligned((__le32 *) &buffer[26+4*i]));
    > *(*intervals)++ = interval ? interval : 1;
    > }
    >
    > @@ -814,7 +815,7 @@ static int uvc_parse_vendor_control(struct uvc_device
    > *dev, memcpy(unit->extension.guidExtensionCode, &buffer[4], 16);
    > unit->extension.bNumControls = buffer[20];
    > unit->extension.bNrInPins =
    > - le16_to_cpup((__le16 *)&buffer[21]);
    > + le16_to_cpu(get_unaligned((__le16 *) &buffer[21]));
    > unit->extension.baSourceID = (__u8 *)unit + sizeof *unit;
    > memcpy(unit->extension.baSourceID, &buffer[22], p);
    > unit->extension.bControlSize = buffer[22+p];
    > @@ -858,8 +859,8 @@ static int uvc_parse_standard_control(struct uvc_device
    > *dev, return -EINVAL;
    > }
    >
    > - dev->uvc_version = le16_to_cpup((__le16 *)&buffer[3]);
    > - dev->clock_frequency = le32_to_cpup((__le32 *)&buffer[7]);
    > + dev->uvc_version = le16_to_cpu(get_unaligned((__le16 *) &buffer[3]));
    > + dev->clock_frequency = le32_to_cpu(get_unaligned((__le32 *)
    > &buffer[7]));
    >
    > /* Parse all USB Video Streaming interfaces. */
    > for (i = 0; i < n; ++i) {
    > @@ -886,7 +887,7 @@ static int uvc_parse_standard_control(struct uvc_device
    > *dev, /* Make sure the terminal type MSB is not null, otherwise it
    > * could be confused with a unit.
    > */
    > - type = le16_to_cpup((__le16 *)&buffer[4]);
    > + type = le16_to_cpu(get_unaligned((__le16 *) &buffer[4]));
    > if ((type & 0xff00) == 0) {
    > uvc_trace(UVC_TRACE_DESCR, "device %d videocontrol "
    > "interface %d INPUT_TERMINAL %d has invalid "
    > @@ -928,11 +929,11 @@ static int uvc_parse_standard_control(struct
    > uvc_device *dev, term->camera.bControlSize = n;
    > term->camera.bmControls = (__u8 *)term + sizeof *term;
    > term->camera.wObjectiveFocalLengthMin =
    > - le16_to_cpup((__le16 *)&buffer[8]);
    > + le16_to_cpu(get_unaligned((__le16 *) &buffer[8]));
    > term->camera.wObjectiveFocalLengthMax =
    > - le16_to_cpup((__le16 *)&buffer[10]);
    > + le16_to_cpu(get_unaligned((__le16 *) &buffer[10]));
    > term->camera.wOcularFocalLength =
    > - le16_to_cpup((__le16 *)&buffer[12]);
    > + le16_to_cpu(get_unaligned((__le16 *) &buffer[12]));
    > memcpy(term->camera.bmControls, &buffer[15], n);
    > } else if (UVC_ENTITY_TYPE(term) == ITT_MEDIA_TRANSPORT_INPUT) {
    > term->media.bControlSize = n;
    > @@ -968,7 +969,7 @@ static int uvc_parse_standard_control(struct uvc_device
    > *dev, /* Make sure the terminal type MSB is not null, otherwise it
    > * could be confused with a unit.
    > */
    > - type = le16_to_cpup((__le16 *)&buffer[4]);
    > + type = le16_to_cpu(get_unaligned((__le16 *) &buffer[4]));
    > if ((type & 0xff00) == 0) {
    > uvc_trace(UVC_TRACE_DESCR, "device %d videocontrol "
    > "interface %d OUTPUT_TERMINAL %d has invalid "
    > @@ -1042,7 +1043,7 @@ static int uvc_parse_standard_control(struct
    > uvc_device *dev, unit->type = buffer[2];
    > unit->processing.bSourceID = buffer[4];
    > unit->processing.wMaxMultiplier =
    > - le16_to_cpup((__le16 *)&buffer[5]);
    > + le16_to_cpu(get_unaligned((__le16 *) &buffer[5]));
    > unit->processing.bControlSize = buffer[7];
    > unit->processing.bmControls = (__u8 *)unit + sizeof *unit;
    > memcpy(unit->processing.bmControls, &buffer[8], n);
    > @@ -1078,7 +1079,7 @@ static int uvc_parse_standard_control(struct
    > uvc_device *dev, memcpy(unit->extension.guidExtensionCode, &buffer[4], 16);
    > unit->extension.bNumControls = buffer[20];
    > unit->extension.bNrInPins =
    > - le16_to_cpup((__le16 *)&buffer[21]);
    > + le16_to_cpu(get_unaligned((__le16 *) &buffer[21]));
    > unit->extension.baSourceID = (__u8 *)unit + sizeof *unit;
    > memcpy(unit->extension.baSourceID, &buffer[22], p);
    > unit->extension.bControlSize = buffer[22+p];


    Cheers,

    Laurent Pinchart
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH] Video/UVC: Fix unaligned exceptions in uvc video driver.

    On Sun, Nov 9, 2008 at 8:55 PM, Laurent Pinchart
    wrote:
    > Hi Bryan, Michael,
    >
    > Thanks for the patch.
    >
    > On Thursday 06 November 2008, Bryan Wu wrote:
    >> From: Michael Hennerich
    >>
    >> buffer can be odd aligned on some NOMMU machine such as Blackfin

    >
    > The comment is a bit misleading. Buffers can be odd-aligned independently off
    > the machine type. The issue comes from machines that can't access unaligned
    > memory. Something like "Fix access to unaligned memory" would be better.
    >
    >> Signed-off-by: Michael Hennerich
    >> Signed-off-by: Bryan Wu
    >> ---
    >> drivers/media/video/uvc/uvc_driver.c | 37
    >> +++++++++++++++++---------------- 1 files changed, 19 insertions(+), 18
    >> deletions(-)
    >>
    >> diff --git a/drivers/media/video/uvc/uvc_driver.c
    >> b/drivers/media/video/uvc/uvc_driver.c index d7ad060..9b4f469 100644
    >> --- a/drivers/media/video/uvc/uvc_driver.c
    >> +++ b/drivers/media/video/uvc/uvc_driver.c
    >> @@ -31,6 +31,7 @@
    >> #include
    >> #include
    >> #include
    >> +#include
    >> #include
    >>
    >> #include
    >> @@ -432,20 +433,20 @@ static int uvc_parse_format(struct uvc_device *dev,
    >>
    >> frame->bFrameIndex = buffer[3];
    >> frame->bmCapabilities = buffer[4];
    >> - frame->wWidth = le16_to_cpup((__le16 *)&buffer[5]);
    >> - frame->wHeight = le16_to_cpup((__le16 *)&buffer[7]);
    >> - frame->dwMinBitRate = le32_to_cpup((__le32 *)&buffer[9]);
    >> - frame->dwMaxBitRate = le32_to_cpup((__le32 *)&buffer[13]);
    >> + frame->wWidth = le16_to_cpu(get_unaligned((__le16 *) &buffer[5]));

    >
    > What about using get_unaligned_le16 and get_unaligned_le32 directly ? Lines
    > would be shorter and could be kept behind the 80 columns limit more easily.
    >
    > Tell me if you want to resubmit or if I should make the modification myself
    > (including the patch description).
    >


    I guess Michael will update this patch according to you review. Then
    we will resubmit it.

    Thanks a lot
    -Bryan
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. RE: [PATCH] Video/UVC: Fix unaligned exceptions in uvc video driver.



    >-----Original Message-----
    >From: Laurent Pinchart [mailto:laurent.pinchart@skynet.be]
    >Sent: Sunday, November 09, 2008 1:55 PM
    >To: Bryan Wu
    >Cc: linux-uvc-devel@lists.berlios.de; video4linux-list@redhat.com;

    linux-
    >kernel@vger.kernel.org; Michael Hennerich
    >Subject: Re: [PATCH] Video/UVC: Fix unaligned exceptions in uvc video
    >driver.
    >
    >Hi Bryan, Michael,
    >
    >Thanks for the patch.
    >
    >On Thursday 06 November 2008, Bryan Wu wrote:
    >> From: Michael Hennerich
    >>
    >> buffer can be odd aligned on some NOMMU machine such as Blackfin

    >
    >The comment is a bit misleading. Buffers can be odd-aligned

    independently
    >off
    >the machine type. The issue comes from machines that can't access

    unaligned
    >memory. Something like "Fix access to unaligned memory" would be

    better.
    >
    >> Signed-off-by: Michael Hennerich
    >> Signed-off-by: Bryan Wu
    >> ---
    >> drivers/media/video/uvc/uvc_driver.c | 37
    >> +++++++++++++++++---------------- 1 files changed, 19 insertions(+),

    18
    >> deletions(-)
    >>
    >> diff --git a/drivers/media/video/uvc/uvc_driver.c
    >> b/drivers/media/video/uvc/uvc_driver.c index d7ad060..9b4f469 100644
    >> --- a/drivers/media/video/uvc/uvc_driver.c
    >> +++ b/drivers/media/video/uvc/uvc_driver.c
    >> @@ -31,6 +31,7 @@
    >> #include
    >> #include
    >> #include
    >> +#include
    >> #include
    >>
    >> #include
    >> @@ -432,20 +433,20 @@ static int uvc_parse_format(struct uvc_device

    *dev,
    >>
    >> frame->bFrameIndex = buffer[3];
    >> frame->bmCapabilities = buffer[4];
    >> - frame->wWidth = le16_to_cpup((__le16 *)&buffer[5]);
    >> - frame->wHeight = le16_to_cpup((__le16 *)&buffer[7]);
    >> - frame->dwMinBitRate = le32_to_cpup((__le32

    *)&buffer[9]);
    >> - frame->dwMaxBitRate = le32_to_cpup((__le32

    *)&buffer[13]);
    >> + frame->wWidth = le16_to_cpu(get_unaligned((__le16 *)

    >&buffer[5]));
    >
    >What about using get_unaligned_le16 and get_unaligned_le32 directly ?

    Lines
    >would be shorter and could be kept behind the 80 columns limit more

    easily.
    >
    >Tell me if you want to resubmit or if I should make the modification

    myself
    >(including the patch description).


    Laurent,

    Well - I just used the same style already used in various other places
    in the uvc driver. - Just wanted to be consistent.

    If you don't mind doing the changes (including the patch description),
    please go ahead.

    Thanks and best regards,
    Michael

    >
    >> + frame->wHeight = le16_to_cpu(get_unaligned((__le16 *)

    >&buffer[7]));
    >> + frame->dwMinBitRate = le32_to_cpu(get_unaligned((__le32

    *)
    >&buffer[9]));
    >> + frame->dwMaxBitRate = le32_to_cpu(get_unaligned((__le32

    *)
    >> &buffer[13])); if (ftype != VS_FRAME_FRAME_BASED) {
    >> frame->dwMaxVideoFrameBufferSize =
    >> - le32_to_cpup((__le32 *)&buffer[17]);
    >> + le32_to_cpu(get_unaligned((__le32 *)

    &buffer[17]));
    >> frame->dwDefaultFrameInterval =
    >> - le32_to_cpup((__le32 *)&buffer[21]);
    >> + le32_to_cpu(get_unaligned((__le32 *)

    &buffer[21]));
    >> frame->bFrameIntervalType = buffer[25];
    >> } else {
    >> frame->dwMaxVideoFrameBufferSize = 0;
    >> frame->dwDefaultFrameInterval =
    >> - le32_to_cpup((__le32 *)&buffer[17]);
    >> + le32_to_cpu(get_unaligned((__le32 *)

    &buffer[17]));
    >> frame->bFrameIntervalType = buffer[21];
    >> }
    >> frame->dwFrameInterval = *intervals;
    >> @@ -468,7 +469,7 @@ static int uvc_parse_format(struct uvc_device

    *dev,
    >> * some other divisions by zero which could happen.
    >> */
    >> for (i = 0; i < n; ++i) {
    >> - interval = le32_to_cpup((__le32

    *)&buffer[26+4*i]);
    >> + interval = le32_to_cpu(get_unaligned((__le32 *)

    >&buffer[26+4*i]));
    >> *(*intervals)++ = interval ? interval : 1;
    >> }
    >>
    >> @@ -814,7 +815,7 @@ static int uvc_parse_vendor_control(struct

    uvc_device
    >> *dev, memcpy(unit->extension.guidExtensionCode, &buffer[4], 16);
    >> unit->extension.bNumControls = buffer[20];
    >> unit->extension.bNrInPins =
    >> - le16_to_cpup((__le16 *)&buffer[21]);
    >> + le16_to_cpu(get_unaligned((__le16 *)

    &buffer[21]));
    >> unit->extension.baSourceID = (__u8 *)unit + sizeof

    *unit;
    >> memcpy(unit->extension.baSourceID, &buffer[22], p);
    >> unit->extension.bControlSize = buffer[22+p];
    >> @@ -858,8 +859,8 @@ static int uvc_parse_standard_control(struct

    >uvc_device
    >> *dev, return -EINVAL;
    >> }
    >>
    >> - dev->uvc_version = le16_to_cpup((__le16 *)&buffer[3]);
    >> - dev->clock_frequency = le32_to_cpup((__le32

    *)&buffer[7]);
    >> + dev->uvc_version = le16_to_cpu(get_unaligned((__le16 *)

    >&buffer[3]));
    >> + dev->clock_frequency = le32_to_cpu(get_unaligned((__le32

    *)
    >> &buffer[7]));
    >>
    >> /* Parse all USB Video Streaming interfaces. */
    >> for (i = 0; i < n; ++i) {
    >> @@ -886,7 +887,7 @@ static int uvc_parse_standard_control(struct

    >uvc_device
    >> *dev, /* Make sure the terminal type MSB is not null, otherwise it
    >> * could be confused with a unit.
    >> */
    >> - type = le16_to_cpup((__le16 *)&buffer[4]);
    >> + type = le16_to_cpu(get_unaligned((__le16 *)

    &buffer[4]));
    >> if ((type & 0xff00) == 0) {
    >> uvc_trace(UVC_TRACE_DESCR, "device %d

    videocontrol "
    >> "interface %d INPUT_TERMINAL %d has

    invalid "
    >> @@ -928,11 +929,11 @@ static int uvc_parse_standard_control(struct
    >> uvc_device *dev, term->camera.bControlSize = n;
    >> term->camera.bmControls = (__u8 *)term + sizeof

    *term;
    >> term->camera.wObjectiveFocalLengthMin =
    >> - le16_to_cpup((__le16 *)&buffer[8]);
    >> + le16_to_cpu(get_unaligned((__le16 *)

    &buffer[8]));
    >> term->camera.wObjectiveFocalLengthMax =
    >> - le16_to_cpup((__le16 *)&buffer[10]);
    >> + le16_to_cpu(get_unaligned((__le16 *)

    &buffer[10]));
    >> term->camera.wOcularFocalLength =
    >> - le16_to_cpup((__le16 *)&buffer[12]);
    >> + le16_to_cpu(get_unaligned((__le16 *)

    &buffer[12]));
    >> memcpy(term->camera.bmControls, &buffer[15], n);
    >> } else if (UVC_ENTITY_TYPE(term) ==

    ITT_MEDIA_TRANSPORT_INPUT)
    >{
    >> term->media.bControlSize = n;
    >> @@ -968,7 +969,7 @@ static int uvc_parse_standard_control(struct

    >uvc_device
    >> *dev, /* Make sure the terminal type MSB is not null, otherwise it
    >> * could be confused with a unit.
    >> */
    >> - type = le16_to_cpup((__le16 *)&buffer[4]);
    >> + type = le16_to_cpu(get_unaligned((__le16 *)

    &buffer[4]));
    >> if ((type & 0xff00) == 0) {
    >> uvc_trace(UVC_TRACE_DESCR, "device %d

    videocontrol "
    >> "interface %d OUTPUT_TERMINAL %d has

    invalid "
    >> @@ -1042,7 +1043,7 @@ static int uvc_parse_standard_control(struct
    >> uvc_device *dev, unit->type = buffer[2];
    >> unit->processing.bSourceID = buffer[4];
    >> unit->processing.wMaxMultiplier =
    >> - le16_to_cpup((__le16 *)&buffer[5]);
    >> + le16_to_cpu(get_unaligned((__le16 *)

    &buffer[5]));
    >> unit->processing.bControlSize = buffer[7];
    >> unit->processing.bmControls = (__u8 *)unit + sizeof

    *unit;
    >> memcpy(unit->processing.bmControls, &buffer[8], n);
    >> @@ -1078,7 +1079,7 @@ static int uvc_parse_standard_control(struct
    >> uvc_device *dev, memcpy(unit->extension.guidExtensionCode,

    &buffer[4],
    >16);
    >> unit->extension.bNumControls = buffer[20];
    >> unit->extension.bNrInPins =
    >> - le16_to_cpup((__le16 *)&buffer[21]);
    >> + le16_to_cpu(get_unaligned((__le16 *)

    &buffer[21]));
    >> unit->extension.baSourceID = (__u8 *)unit + sizeof

    *unit;
    >> memcpy(unit->extension.baSourceID, &buffer[22], p);
    >> unit->extension.bControlSize = buffer[22+p];

    >
    >Cheers,
    >
    >Laurent Pinchart

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH] Video/UVC: Fix unaligned exceptions in uvc video driver.

    On Monday 10 November 2008, Hennerich, Michael wrote:
    > > -----Original Message-----
    > > From: Laurent Pinchart [mailto:laurent.pinchart@skynet.be]
    > > Sent: Sunday, November 09, 2008 1:55 PM
    > > To: Bryan Wu
    > > Cc: linux-uvc-devel@lists.berlios.de; video4linux-list@redhat.com;
    > > linux-kernel@vger.kernel.org; Michael Hennerich
    > > Subject: Re: [PATCH] Video/UVC: Fix unaligned exceptions in uvc video
    > > driver.
    > >
    > > Hi Bryan, Michael,
    > >
    > > Thanks for the patch.
    > >
    > > On Thursday 06 November 2008, Bryan Wu wrote:
    > > > From: Michael Hennerich
    > > >
    > > > buffer can be odd aligned on some NOMMU machine such as Blackfin

    > >
    > > The comment is a bit misleading. Buffers can be odd-aligned independently
    > > off the machine type. The issue comes from machines that can't access
    > > unaligned memory. Something like "Fix access to unaligned memory" would be
    > > better.

    [snip]

    > > What about using get_unaligned_le16 and get_unaligned_le32 directly ?
    > > Lines would be shorter and could be kept behind the 80 columns limit more
    > > easily.
    > > Tell me if you want to resubmit or if I should make the modification
    > > myself (including the patch description).

    >
    > Laurent,
    >
    > Well - I just used the same style already used in various other places
    > in the uvc driver. - Just wanted to be consistent.


    No worries. get_unaligned_le{16|32} has been introduced in the kernel recently
    and I haven't updated the uvcvideo driver like I should have. I'll fix that.

    > If you don't mind doing the changes (including the patch description),
    > please go ahead.


    Ok I'll fix and submit the patch.

    Best regards,

    Laurent Pinchart
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread