Re: [patch 2/2] [media] s2255drv: fix some endian bugs

dean anderson
2012-02-23T01:34:48Z

Hi Dan,

The original code has an issue on big endian hardware. The patch looks ok.

Signed-off-by: Dean Anderson <linux-dev@sensoray.com>

On 2/16/2012 10:44 PM, Dan Carpenter wrote:
> I don't have this hardware and I don't know the subsystem very well. So
> please review this patch carefully. The original code definitely looks
> buggy though.
>
> Sparse complains about some endian bugs where little endian bugs are
> treated as cpu endian.
>
> Signed-off-by: Dan Carpenter<dan.carpenter@oracle.com>
> ---
>
> diff --git a/drivers/media/video/s2255drv.c b/drivers/media/video/s2255drv.c
> index 3505242..4894cbb 100644
> --- a/drivers/media/video/s2255drv.c
> +++ b/drivers/media/video/s2255drv.c
> @@ -134,7 +134,7 @@
>
> /* usb config commands */
> #define IN_DATA_TOKEN cpu_to_le32(0x2255c0de)
> -#define CMD_2255 cpu_to_le32(0xc2255000)
> +#define CMD_2255 0xc2255000
> #define CMD_SET_MODE cpu_to_le32((CMD_2255 | 0x10))
> #define CMD_START cpu_to_le32((CMD_2255 | 0x20))
> #define CMD_STOP cpu_to_le32((CMD_2255 | 0x30))
> @@ -2025,7 +2025,7 @@ static int save_frame(struct s2255_dev *dev, struct s2255_pipeinfo *pipe_info)
> pdata[1]);
> offset = jj + PREFIX_SIZE;
> bframe = 1;
> - cc = pdword[1];
> + cc = le32_to_cpu(pdword[1]);
> if (cc>= MAX_CHANNELS) {
> printk(KERN_ERR
> "bad channel\n");
> @@ -2034,22 +2034,22 @@ static int save_frame(struct s2255_dev *dev, struct s2255_pipeinfo *pipe_info)
> /* reverse it */
> dev->cc = G_chnmap[cc];
> channel =&dev->channel[dev->cc];
> - payload = pdword[3];
> + payload = le32_to_cpu(pdword[3]);
> if (payload> channel->req_image_size) {
> channel->bad_payload++;
> /* discard the bad frame */
> return -EINVAL;
> }
> channel->pkt_size = payload;
> - channel->jpg_size = pdword[4];
> + channel->jpg_size = le32_to_cpu(pdword[4]);
> break;
> case S2255_MARKER_RESPONSE:
>
> pdata += DEF_USB_BLOCK;
> jj += DEF_USB_BLOCK;
> - if (pdword[1]>= MAX_CHANNELS)
> + if (le32_to_cpu(pdword[1])>= MAX_CHANNELS)
> break;
> - cc = G_chnmap[pdword[1]];
> + cc = G_chnmap[le32_to_cpu(pdword[1])];
> if (cc>= MAX_CHANNELS)
> break;
> channel =&dev->channel[cc];
> @@ -2072,11 +2072,11 @@ static int save_frame(struct s2255_dev *dev, struct s2255_pipeinfo *pipe_info)
> wake_up(&dev->fw_data->wait_fw);
> break;
> case S2255_RESPONSE_STATUS:
> - channel->vidstatus = pdword[3];
> + channel->vidstatus = le32_to_cpu(pdword[3]);
> channel->vidstatus_ready = 1;
> wake_up(&channel->wait_vidstatus);
> dprintk(5, "got vidstatus %x chan %d\n",
> - pdword[3], cc);
> + le32_to_cpu(pdword[3]), cc);
> break;
> default:
> printk(KERN_INFO "s2255 unknown resp\n");
> @@ -2603,10 +2603,11 @@ static int s2255_probe(struct usb_interface *interface,
> __le32 *pRel;
> pRel = (__le32 *)&dev->fw_data->fw->data[fw_size - 4];
> printk(KERN_INFO "s2255 dsp fw version %x\n", *pRel);
> - dev->dsp_fw_ver = *pRel;
> - if (*pRel< S2255_CUR_DSP_FWVER)
> + dev->dsp_fw_ver = le32_to_cpu(*pRel);
> + if (dev->dsp_fw_ver< S2255_CUR_DSP_FWVER)
> printk(KERN_INFO "s2255: f2255usb.bin out of date.\n");
> - if (dev->pid == 0x2257&& *pRel< S2255_MIN_DSP_COLORFILTER)
> + if (dev->pid == 0x2257&&
> + dev->dsp_fw_ver< S2255_MIN_DSP_COLORFILTER)
> printk(KERN_WARNING "s2255: 2257 requires firmware %d"
> " or above.\n", S2255_MIN_DSP_COLORFILTER);
> }
>