On Wed, 26 Oct 2016 17:22:50 +0300
Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> On Tue, Oct 25, 2016 at 05:44:20PM +0200, Christian Gromm wrote:
> > From: Andrey Shvetsov <andrey.shvetsov@xxxxxx>
> >
> > This patch puts the synchronization procedure trigger for asynchronous
> > channels into the function hdm_configure_channel. Likewise, it removes
> > triggering of hardware specific synchronization for other channel types
> > from the probe function as it is not required.
> >
> > Signed-off-by: Andrey Shvetsov <andrey.shvetsov@xxxxxx>
> > Signed-off-by: Christian Gromm <christian.gromm@xxxxxxxxxxxxx>
> > ---
> > drivers/staging/most/hdm-usb/hdm_usb.c | 17 +++++++++--------
> > 1 file changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c b/drivers/staging/most/hdm-usb/hdm_usb.c
> > index 1a630e1..db11930 100644
> > --- a/drivers/staging/most/hdm-usb/hdm_usb.c
> > +++ b/drivers/staging/most/hdm-usb/hdm_usb.c
> > @@ -695,6 +695,15 @@ static int hdm_configure_channel(struct most_interface *iface, int channel,
> > - conf->buffer_size;
> > exit:
> > mdev->conf[channel] = *conf;
> > + if (conf->data_type == MOST_CH_ASYNC) {
> > + u16 ep = mdev->ep_address[channel];
> > + int err = drci_wr_reg(mdev->usb_device,
> > + DRCI_REG_BASE + DRCI_COMMAND + ep * 16,
> > + 1);
> > +
> > + if (err < 0)
> > + dev_warn(dev, "sync for ep%02x failed", ep);
> > + }
> > return 0;
>
> This code is weird, because we goto exit without checking the
> frame_size. It looks like it doesn't matter much but it's sort of
> puzzling what's going on. There weren't any comments to explain it.
>
The frame size is only needed if we are dealing with synchronous and
(in some cases) isochronous data. So you're right, the variable
frame_size is _not_ needed in case we be jumping to the 'exit' label
and hence, not being checked.
Haven't had the feeling that this is worth a comment. It isn't easy
to decide what needs a comment and what does not anyway. Then I would
probably also have to explain why we jump to 'exit' if we have
isochronous data and a packet_per_transaction value unequal to 0xff.
(I don't expect anyone to understand what this is supposed mean, unless
he is familiar with the network interface controller.)
So, let me know if a comment on the frame_size usage can fix the
confusion.
regards,
Chris
> regards,
> dan carpenter
>
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
