[PATCH 0/3] staging: most: fix driver issues

[PATCH 0/3] staging: most: fix driver issues

[Christian Gromm]

This patch set is needed to fix up issues of the USB module and the
networking module.

Andrey Shvetsov (3):
  staging: most: aim-networking: keep channels closed if ndo_open fails
  staging: most: hdm-usb: do h/w specific synchronization at
    configuration time
  staging: most: hdm-usb: introduce synchronization function

 drivers/staging/most/aim-network/networking.c | 38 +++++++++++++++------------
 drivers/staging/most/hdm-usb/hdm_usb.c        | 37 +++++++++++++-------------
 2 files changed, 40 insertions(+), 35 deletions(-)

-- 
1.9.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/3] staging: most: aim-networking: keep channels closed if ndo_open fails

[Christian Gromm]

From: Andrey Shvetsov <andrey.shvetsov@k2l.de>

This patch stops all started channels whenever the function most_nd_open
returns an error. Additionally, it renames variable wait_res to ret for
the consistency.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-network/networking.c | 38 +++++++++++++++------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/most/aim-network/networking.c b/drivers/staging/most/aim-network/networking.c
index fe0d516..ce1764c 100644
--- a/drivers/staging/most/aim-network/networking.c
+++ b/drivers/staging/most/aim-network/networking.c
@@ -181,7 +181,7 @@ static int most_nd_set_mac_address(struct net_device *dev, void *p)
 static int most_nd_open(struct net_device *dev)
 {
 	struct net_dev_context *nd = dev->ml_priv;
-	long wait_res;
+	long ret;
 
 	netdev_info(dev, "open net device\n");
 
@@ -203,26 +203,30 @@ static int most_nd_open(struct net_device *dev)
 		return -EBUSY;
 	}
 
-	nd->channels_opened = true;
-	netif_wake_queue(dev);
-
-	if (is_valid_ether_addr(dev->dev_addr))
-		return 0;
-
-	nd->iface->request_netinfo(nd->iface, nd->tx.ch_id);
-	wait_res = wait_for_completion_interruptible_timeout(
-			   &nd->mac_compl, msecs_to_jiffies(5000));
-	if (!wait_res) {
-		netdev_err(dev, "mac timeout\n");
-		return -EBUSY;
-	}
+	if (!is_valid_ether_addr(dev->dev_addr)) {
+		nd->iface->request_netinfo(nd->iface, nd->tx.ch_id);
+		ret = wait_for_completion_interruptible_timeout(
+			      &nd->mac_compl, msecs_to_jiffies(5000));
+		if (!ret) {
+			netdev_err(dev, "mac timeout\n");
+			ret = -EBUSY;
+			goto err;
+		}
 
-	if (wait_res < 0) {
-		netdev_warn(dev, "mac waiting interrupted\n");
-		return wait_res;
+		if (ret < 0) {
+			netdev_warn(dev, "mac waiting interrupted\n");
+			goto err;
+		}
 	}
 
+	nd->channels_opened = true;
+	netif_wake_queue(dev);
 	return 0;
+
+err:
+	most_stop_channel(nd->iface, nd->tx.ch_id, &aim);
+	most_stop_channel(nd->iface, nd->rx.ch_id, &aim);
+	return ret;
 }
 
 static int most_nd_stop(struct net_device *dev)
-- 
1.9.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/3] staging: most: hdm-usb: do h/w specific synchronization at configuration time

[Christian Gromm]

From: Andrey Shvetsov <andrey.shvetsov@k2l.de>

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@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 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;
 }
 
@@ -1111,7 +1120,6 @@ static void destroy_most_dci_obj(struct most_dci_obj *p)
 	struct most_channel_capability *tmp_cap;
 	struct usb_endpoint_descriptor *ep_desc;
 	int ret = 0;
-	int err;
 
 	if (!mdev)
 		goto exit_ENOMEM;
@@ -1187,13 +1195,6 @@ static void destroy_most_dci_obj(struct most_dci_obj *p)
 		tmp_cap++;
 		init_usb_anchor(&mdev->busy_urbs[i]);
 		spin_lock_init(&mdev->channel_lock[i]);
-		err = drci_wr_reg(usb_dev,
-				  DRCI_REG_BASE + DRCI_COMMAND +
-				  ep_desc->bEndpointAddress * 16,
-				  1);
-		if (err < 0)
-			dev_warn(dev, "DCI Sync for EP %02x failed",
-				 ep_desc->bEndpointAddress);
 	}
 	dev_notice(dev, "claimed gadget: Vendor=%4.4x ProdID=%4.4x Bus=%02x Device=%02x\n",
 		   le16_to_cpu(usb_dev->descriptor.idVendor),
-- 
1.9.1

[PATCH 3/3] staging: most: hdm-usb: introduce synchronization function

[Christian Gromm]

From: Andrey Shvetsov <andrey.shvetsov@k2l.de>

This patch introduces the function start_sync_ep() and relocates the
triggers for synchronization to this function.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/hdm-usb/hdm_usb.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c b/drivers/staging/most/hdm-usb/hdm_usb.c
index db11930..3433646 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -182,6 +182,11 @@ static inline int drci_wr_reg(struct usb_device *dev, u16 reg, u16 data)
 			       5 * HZ);
 }
 
+static inline int start_sync_ep(struct usb_device *usb_dev, u16 ep)
+{
+	return drci_wr_reg(usb_dev, DRCI_REG_BASE + DRCI_COMMAND + ep * 16, 1);
+}
+
 /**
  * get_stream_frame_size - calculate frame size of current configuration
  * @cfg: channel configuration
@@ -697,11 +702,8 @@ static int hdm_configure_channel(struct most_interface *iface, int channel,
 	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)
+		if (start_sync_ep(mdev->usb_device, ep) < 0)
 			dev_warn(dev, "sync for ep%02x failed", ep);
 	}
 	return 0;
@@ -987,6 +989,7 @@ static ssize_t store_value(struct most_dci_obj *dci_obj,
 	u16 val;
 	u16 reg_addr;
 	const char *name = attr->attr.name;
+	struct usb_device *usb_dev = dci_obj->usb_device;
 	int err = kstrtou16(buf, 16, &val);
 
 	if (err)
@@ -997,18 +1000,15 @@ static ssize_t store_value(struct most_dci_obj *dci_obj,
 		return count;
 	}
 
-	if (!strcmp(name, "arb_value")) {
-		reg_addr = dci_obj->reg_addr;
-	} else if (!strcmp(name, "sync_ep")) {
-		u16 ep = val;
-
-		reg_addr = DRCI_REG_BASE + DRCI_COMMAND + ep * 16;
-		val = 1;
-	} else if (get_static_reg_addr(ro_regs, name, &reg_addr)) {
+	if (!strcmp(name, "arb_value"))
+		err = drci_wr_reg(usb_dev, dci_obj->reg_addr, val);
+	else if (!strcmp(name, "sync_ep"))
+		err = start_sync_ep(usb_dev, val);
+	else if (!get_static_reg_addr(ro_regs, name, &reg_addr))
+		err = drci_wr_reg(usb_dev, reg_addr, val);
+	else
 		return -EFAULT;
-	}
 
-	err = drci_wr_reg(dci_obj->usb_device, reg_addr, val);
 	if (err < 0)
 		return err;
 
-- 
1.9.1

Re: [PATCH 2/3] staging: most: hdm-usb: do h/w specific synchronization at configuration time

[Dan Carpenter]

On Tue, Oct 25, 2016 at 05:44:20PM +0200, Christian Gromm wrote:
> From: Andrey Shvetsov <andrey.shvetsov@k2l.de>
> 
> 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@k2l.de>
> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> ---
>  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.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] staging: most: hdm-usb: do h/w specific synchronization at configuration time

[Christian Gromm]

On Wed, 26 Oct 2016 17:22:50 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> On Tue, Oct 25, 2016 at 05:44:20PM +0200, Christian Gromm wrote:
> > From: Andrey Shvetsov <andrey.shvetsov@k2l.de>
> > 
> > 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@k2l.de>
> > Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> > ---
> >  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@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] staging: most: hdm-usb: do h/w specific synchronization at configuration time

[Dan Carpenter]

On Thu, Oct 27, 2016 at 10:57:07AM +0200, Christian Gromm wrote:
> On Wed, 26 Oct 2016 17:22:50 +0300
> Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> > On Tue, Oct 25, 2016 at 05:44:20PM +0200, Christian Gromm wrote:
> > > From: Andrey Shvetsov <andrey.shvetsov@k2l.de>
> > > 
> > > 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@k2l.de>
> > > Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> > > ---
> > >  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.

A comment would be nice, yes.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] staging: most: hdm-usb: do h/w specific synchronization at configuration time

[Christian Gromm]

On Thu, 27 Oct 2016 12:00:28 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> On Thu, Oct 27, 2016 at 10:57:07AM +0200, Christian Gromm wrote:
> > On Wed, 26 Oct 2016 17:22:50 +0300
> > Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > 
> > > On Tue, Oct 25, 2016 at 05:44:20PM +0200, Christian Gromm wrote:
> > > > From: Andrey Shvetsov <andrey.shvetsov@k2l.de>
> > > > 
> > > > 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@k2l.de>
> > > > Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> > > > ---
> > > >  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.
> 
> A comment would be nice, yes.

Fine. It would be cool if this patch set will be accepted and I'll be
sending in a new patch that adds the desired comment.

Greg, does this make sense to you?

thanks,
Chris

> 
> regards,
> dan carpenter
> 

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] staging: most: hdm-usb: do h/w specific synchronization at configuration time

[Greg KH]

On Thu, Oct 27, 2016 at 01:00:47PM +0200, Christian Gromm wrote:
> On Thu, 27 Oct 2016 12:00:28 +0300
> Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> > On Thu, Oct 27, 2016 at 10:57:07AM +0200, Christian Gromm wrote:
> > > On Wed, 26 Oct 2016 17:22:50 +0300
> > > Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > > 
> > > > On Tue, Oct 25, 2016 at 05:44:20PM +0200, Christian Gromm wrote:
> > > > > From: Andrey Shvetsov <andrey.shvetsov@k2l.de>
> > > > > 
> > > > > 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@k2l.de>
> > > > > Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> > > > > ---
> > > > >  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.
> > 
> > A comment would be nice, yes.
> 
> Fine. It would be cool if this patch set will be accepted and I'll be
> sending in a new patch that adds the desired comment.
> 
> Greg, does this make sense to you?

Yes, that's fine.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] staging: most: hdm-usb: do h/w specific synchronization at configuration time

[Dan Carpenter]

Yes yes.  I wasn't commenting on the patch, which seems fine.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel