Re: [PATCH v3] can: kvaser_usb: handle rx msg correctly

[Olivier Sobrie <olivier@sobrie.be>]

Hi Marc,

On Wed, May 15, 2013 at 12:05:31PM +0200, Marc Kleine-Budde wrote:
> On 05/07/2013 10:05 PM, Olivier Sobrie wrote:
> > From: Jonas Peterson <jonas.peterson@gmail.com>
> > 
> > Unlike Kvaser Leaf light devices, some other Kvaser devices (like USBcan
> > Pro, USBcan R) receive CAN messages in CMD_LOG_MESSAGE frames. This
> > patch adds support for it.
> > 
> > Signed-off-by: Jonas Peterson <jonas.peterson@gmail.com>
> > Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
> > ---
> > v3:
> >  - add KVASER_EXTENDED_FRAME define
> >  - call kvaser_usb_rx_error() in kvaser_usb_rx_can_msg when the frame is
> >    a CMD_LOG_MESSAGE and flag MSG_FLAG_ERROR_FRAME is set.
> > 
> > By reading the patch again I saw that I mixed up kvaser_usb_rx_can_msg and
>                                                                      ^^^
> 
> Do you want to merge kvaser_usb_rx_can_msg() or kvaser_usb_rx_can_err()?

No, grrr I mixed up kvaser_usb_rx_can_msg and kvaser_usb_rx_can_err in my
comment. I meant kvaser_usb_rx_can_err() and kvaser_usb_rx_error().

> 
> > kvaser_usb_rx_error when I cleaned the patch... If fixed that in this
> > version.
> > I should maybe merge these two functions to avoid confusion...
> 
> If you want to merge kvaser_usb_rx_can_err() and kvaser_usb_rx_error()
> it would make sense, to have the error handling functionality in one
> function. I haven't looked at the code, though.

Yes that's what I'll maybe do, merging the two error functions in one.
I'll look at this more deeply when I've a more time.

> 
> However, this would be a separate patch. I'm applying this patch as it
> is to can/master with linux-stable on Cc.

Ok fine for me.

Thanks,

-- 
Olivier