* [PATCH 06/10] staging: ozwpan: Make oz_hcd_pd_arrived() return a struct pointer
2013-08-13 17:29 [PATCH 05/10] staging: ozwpan: Remove unnecessary pointer check Rupesh Gujare
@ 2013-08-13 17:29 ` Rupesh Gujare
2013-08-13 17:29 ` [PATCH 07/10] staging: ozwpan: Make oz_hcd_pd_departed() take " Rupesh Gujare
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Rupesh Gujare @ 2013-08-13 17:29 UTC (permalink / raw)
To: devel; +Cc: dan.carpenter, linux-usb, linux-kernel, gregkh
oz_hcd_pd_arrived returns struct oz_port *, change function
declaration to avoid ambiguity.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Rupesh Gujare <rupesh.gujare@atmel.com>
---
drivers/staging/ozwpan/ozhcd.c | 4 ++--
drivers/staging/ozwpan/ozhcd.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
index 8633f0c..73d80f2 100644
--- a/drivers/staging/ozwpan/ozhcd.c
+++ b/drivers/staging/ozwpan/ozhcd.c
@@ -660,10 +660,10 @@ static inline void oz_hcd_put(struct oz_hcd *ozhcd)
* probably very rare indeed.
* Context: softirq
*/
-void *oz_hcd_pd_arrived(void *hpd)
+struct oz_port *oz_hcd_pd_arrived(void *hpd)
{
int i;
- void *hport = NULL;
+ struct oz_port *hport = NULL;
struct oz_hcd *ozhcd = NULL;
struct oz_endpoint *ep;
diff --git a/drivers/staging/ozwpan/ozhcd.h b/drivers/staging/ozwpan/ozhcd.h
index 9b30dfd..ef3202f 100644
--- a/drivers/staging/ozwpan/ozhcd.h
+++ b/drivers/staging/ozwpan/ozhcd.h
@@ -7,8 +7,8 @@
int oz_hcd_init(void);
void oz_hcd_term(void);
-void *oz_hcd_pd_arrived(void *ctx);
void oz_hcd_pd_departed(void *ctx);
+struct oz_port *oz_hcd_pd_arrived(void *ctx);
void oz_hcd_pd_reset(void *hpd, void *hport);
#endif /* _OZHCD_H */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 07/10] staging: ozwpan: Make oz_hcd_pd_departed() take a struct pointer.
2013-08-13 17:29 [PATCH 05/10] staging: ozwpan: Remove unnecessary pointer check Rupesh Gujare
2013-08-13 17:29 ` [PATCH 06/10] staging: ozwpan: Make oz_hcd_pd_arrived() return a struct pointer Rupesh Gujare
@ 2013-08-13 17:29 ` Rupesh Gujare
2013-08-13 17:35 ` Sergei Shtylyov
2013-08-13 17:29 ` [PATCH 08/10] staging: ozwpan: Remove unneeded initializers Rupesh Gujare
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Rupesh Gujare @ 2013-08-13 17:29 UTC (permalink / raw)
To: devel; +Cc: dan.carpenter, linux-usb, linux-kernel, gregkh
oz_hcd_pd_departed() takes struct oz_port pointer instead of
void *, change function declaration to avoid ambiguity.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Rupesh Gujare <rupesh.gujare@atmel.com>
---
drivers/staging/ozwpan/ozhcd.c | 4 ++--
drivers/staging/ozwpan/ozhcd.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
index 73d80f2..ed3ffeb 100644
--- a/drivers/staging/ozwpan/ozhcd.c
+++ b/drivers/staging/ozwpan/ozhcd.c
@@ -720,9 +720,9 @@ out:
* polled. We release the reference we hold on the PD.
* Context: softirq
*/
-void oz_hcd_pd_departed(void *hport)
+void oz_hcd_pd_departed(struct oz_port *hport)
{
- struct oz_port *port = (struct oz_port *)hport;
+ struct oz_port *port = hport;
struct oz_hcd *ozhcd;
void *hpd;
struct oz_endpoint *ep = NULL;
diff --git a/drivers/staging/ozwpan/ozhcd.h b/drivers/staging/ozwpan/ozhcd.h
index ef3202f..55e97b1 100644
--- a/drivers/staging/ozwpan/ozhcd.h
+++ b/drivers/staging/ozwpan/ozhcd.h
@@ -7,8 +7,8 @@
int oz_hcd_init(void);
void oz_hcd_term(void);
-void oz_hcd_pd_departed(void *ctx);
struct oz_port *oz_hcd_pd_arrived(void *ctx);
+void oz_hcd_pd_departed(struct oz_port *hport);
void oz_hcd_pd_reset(void *hpd, void *hport);
#endif /* _OZHCD_H */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 07/10] staging: ozwpan: Make oz_hcd_pd_departed() take a struct pointer.
2013-08-13 17:29 ` [PATCH 07/10] staging: ozwpan: Make oz_hcd_pd_departed() take " Rupesh Gujare
@ 2013-08-13 17:35 ` Sergei Shtylyov
2013-08-13 17:40 ` Rupesh Gujare
0 siblings, 1 reply; 12+ messages in thread
From: Sergei Shtylyov @ 2013-08-13 17:35 UTC (permalink / raw)
To: Rupesh Gujare; +Cc: devel, dan.carpenter, linux-usb, linux-kernel, gregkh
Hello.
On 08/13/2013 09:29 PM, Rupesh Gujare wrote:
> oz_hcd_pd_departed() takes struct oz_port pointer instead of
> void *, change function declaration to avoid ambiguity.
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Rupesh Gujare <rupesh.gujare@atmel.com>
> ---
> drivers/staging/ozwpan/ozhcd.c | 4 ++--
> drivers/staging/ozwpan/ozhcd.h | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
> diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
> index 73d80f2..ed3ffeb 100644
> --- a/drivers/staging/ozwpan/ozhcd.c
> +++ b/drivers/staging/ozwpan/ozhcd.c
> @@ -720,9 +720,9 @@ out:
> * polled. We release the reference we hold on the PD.
> * Context: softirq
> */
> -void oz_hcd_pd_departed(void *hport)
> +void oz_hcd_pd_departed(struct oz_port *hport)
> {
> - struct oz_port *port = (struct oz_port *)hport;
> + struct oz_port *port = hport;
Do you really need a copy? Isn't it better to rename the parameter and
remove this line altogether?
WBR, Sergei
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 07/10] staging: ozwpan: Make oz_hcd_pd_departed() take a struct pointer.
2013-08-13 17:35 ` Sergei Shtylyov
@ 2013-08-13 17:40 ` Rupesh Gujare
2013-08-13 17:46 ` Sergei Shtylyov
0 siblings, 1 reply; 12+ messages in thread
From: Rupesh Gujare @ 2013-08-13 17:40 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: devel, dan.carpenter, linux-usb, linux-kernel, gregkh
On 13/08/13 18:35, Sergei Shtylyov wrote:
> Hello.
>
> On 08/13/2013 09:29 PM, Rupesh Gujare wrote:
>
>> oz_hcd_pd_departed() takes struct oz_port pointer instead of
>> void *, change function declaration to avoid ambiguity.
>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>> Signed-off-by: Rupesh Gujare <rupesh.gujare@atmel.com>
>> ---
>> drivers/staging/ozwpan/ozhcd.c | 4 ++--
>> drivers/staging/ozwpan/ozhcd.h | 2 +-
>> 2 files changed, 3 insertions(+), 3 deletions(-)
>
>> diff --git a/drivers/staging/ozwpan/ozhcd.c
>> b/drivers/staging/ozwpan/ozhcd.c
>> index 73d80f2..ed3ffeb 100644
>> --- a/drivers/staging/ozwpan/ozhcd.c
>> +++ b/drivers/staging/ozwpan/ozhcd.c
>> @@ -720,9 +720,9 @@ out:
>> * polled. We release the reference we hold on the PD.
>> * Context: softirq
>> */
>> -void oz_hcd_pd_departed(void *hport)
>> +void oz_hcd_pd_departed(struct oz_port *hport)
>> {
>> - struct oz_port *port = (struct oz_port *)hport;
>> + struct oz_port *port = hport;
>
> Do you really need a copy? Isn't it better to rename the parameter
> and remove this line altogether?
>
> WBR, Sergei
>
Yes, that is the idea, for next patch series, as I don't want to mix
two changes in single patch.
--
Regards,
Rupesh Gujare
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 07/10] staging: ozwpan: Make oz_hcd_pd_departed() take a struct pointer.
2013-08-13 17:40 ` Rupesh Gujare
@ 2013-08-13 17:46 ` Sergei Shtylyov
2013-08-14 21:31 ` Dan Carpenter
0 siblings, 1 reply; 12+ messages in thread
From: Sergei Shtylyov @ 2013-08-13 17:46 UTC (permalink / raw)
To: Rupesh Gujare; +Cc: devel, dan.carpenter, linux-usb, linux-kernel, gregkh
On 08/13/2013 09:40 PM, Rupesh Gujare wrote:
>>> oz_hcd_pd_departed() takes struct oz_port pointer instead of
>>> void *, change function declaration to avoid ambiguity.
>>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>>> Signed-off-by: Rupesh Gujare <rupesh.gujare@atmel.com>
>>> ---
>>> drivers/staging/ozwpan/ozhcd.c | 4 ++--
>>> drivers/staging/ozwpan/ozhcd.h | 2 +-
>>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>> diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
>>> index 73d80f2..ed3ffeb 100644
>>> --- a/drivers/staging/ozwpan/ozhcd.c
>>> +++ b/drivers/staging/ozwpan/ozhcd.c
>>> @@ -720,9 +720,9 @@ out:
>>> * polled. We release the reference we hold on the PD.
>>> * Context: softirq
>>> */
>>> -void oz_hcd_pd_departed(void *hport)
>>> +void oz_hcd_pd_departed(struct oz_port *hport)
>>> {
>>> - struct oz_port *port = (struct oz_port *)hport;
>>> + struct oz_port *port = hport;
>> Do you really need a copy? Isn't it better to rename the parameter and
>> remove this line altogether?
>> WBR, Sergei
> Yes, that is the idea, for next patch series, as I don't want to mix two
> changes in single patch.
Don't think it's worth another series.
WBR, Sergei
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 07/10] staging: ozwpan: Make oz_hcd_pd_departed() take a struct pointer.
2013-08-13 17:46 ` Sergei Shtylyov
@ 2013-08-14 21:31 ` Dan Carpenter
0 siblings, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2013-08-14 21:31 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Rupesh Gujare, devel, linux-usb, linux-kernel, gregkh
On Tue, Aug 13, 2013 at 09:46:36PM +0400, Sergei Shtylyov wrote:
>
> >> Do you really need a copy? Isn't it better to rename the parameter and
> >>remove this line altogether?
>
> >>WBR, Sergei
>
> >Yes, that is the idea, for next patch series, as I don't want to mix two
> >changes in single patch.
>
> Don't think it's worth another series.
>
Yeah, it should have been done in the same patch, you're obviously
right. But Greg already merged them so it will have to be fixed in
follow on patches.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 08/10] staging: ozwpan: Remove unneeded initializers
2013-08-13 17:29 [PATCH 05/10] staging: ozwpan: Remove unnecessary pointer check Rupesh Gujare
2013-08-13 17:29 ` [PATCH 06/10] staging: ozwpan: Make oz_hcd_pd_arrived() return a struct pointer Rupesh Gujare
2013-08-13 17:29 ` [PATCH 07/10] staging: ozwpan: Make oz_hcd_pd_departed() take " Rupesh Gujare
@ 2013-08-13 17:29 ` Rupesh Gujare
2013-08-13 17:29 ` [PATCH 09/10] staging: ozwpan: Swap arguments of oz_ep_alloc() to match kmalloc() Rupesh Gujare
2013-08-13 17:29 ` [PATCH 10/10] staging: ozwpan: Separate success & failure case for oz_hcd_pd_arrived() Rupesh Gujare
4 siblings, 0 replies; 12+ messages in thread
From: Rupesh Gujare @ 2013-08-13 17:29 UTC (permalink / raw)
To: devel; +Cc: dan.carpenter, linux-usb, linux-kernel, gregkh
Remove variable initialization wherever it is not required.
Signed-off-by: Rupesh Gujare <rupesh.gujare@atmel.com>
---
drivers/staging/ozwpan/ozeltbuf.c | 2 +-
drivers/staging/ozwpan/ozhcd.c | 18 +++++++++---------
drivers/staging/ozwpan/ozpd.c | 4 ++--
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/ozwpan/ozeltbuf.c b/drivers/staging/ozwpan/ozeltbuf.c
index 4844d9f..cfc0f39 100644
--- a/drivers/staging/ozwpan/ozeltbuf.c
+++ b/drivers/staging/ozwpan/ozeltbuf.c
@@ -67,7 +67,7 @@ void oz_elt_buf_term(struct oz_elt_buf *buf)
*/
struct oz_elt_info *oz_elt_info_alloc(struct oz_elt_buf *buf)
{
- struct oz_elt_info *ei = NULL;
+ struct oz_elt_info *ei;
spin_lock_bh(&buf->lock);
if (buf->free_elts && buf->elt_pool) {
diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
index ed3ffeb..0fd3fea 100644
--- a/drivers/staging/ozwpan/ozhcd.c
+++ b/drivers/staging/ozwpan/ozhcd.c
@@ -373,7 +373,7 @@ static void oz_complete_urb(struct usb_hcd *hcd, struct urb *urb,
{
struct oz_hcd *ozhcd = oz_hcd_private(hcd);
unsigned long irq_state;
- struct oz_urb_link *cancel_urbl = NULL;
+ struct oz_urb_link *cancel_urbl;
spin_lock_irqsave(&g_tasklet_lock, irq_state);
usb_hcd_unlink_urb_from_ep(hcd, urb);
@@ -585,7 +585,7 @@ static struct urb *oz_find_urb_by_id(struct oz_port *port, int ep_ix,
{
struct oz_hcd *ozhcd = port->ozhcd;
struct urb *urb = NULL;
- struct oz_urb_link *urbl = NULL;
+ struct oz_urb_link *urbl;
struct oz_endpoint *ep;
spin_lock_bh(&ozhcd->hcd_lock);
@@ -664,7 +664,7 @@ struct oz_port *oz_hcd_pd_arrived(void *hpd)
{
int i;
struct oz_port *hport = NULL;
- struct oz_hcd *ozhcd = NULL;
+ struct oz_hcd *ozhcd;
struct oz_endpoint *ep;
ozhcd = oz_hcd_claim();
@@ -1423,7 +1423,7 @@ static void oz_clean_endpoints_for_config(struct usb_hcd *hcd,
*/
static void *oz_claim_hpd(struct oz_port *port)
{
- void *hpd = NULL;
+ void *hpd;
struct oz_hcd *ozhcd = port->ozhcd;
spin_lock_bh(&ozhcd->hcd_lock);
@@ -1444,7 +1444,7 @@ static void oz_process_ep0_urb(struct oz_hcd *ozhcd, struct urb *urb,
unsigned windex;
unsigned wvalue;
unsigned wlength;
- void *hpd = NULL;
+ void *hpd;
u8 req_id;
int rc = 0;
unsigned complete = 0;
@@ -1798,7 +1798,7 @@ static int oz_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
gfp_t mem_flags)
{
struct oz_hcd *ozhcd = oz_hcd_private(hcd);
- int rc = 0;
+ int rc;
int port_ix;
struct oz_port *port;
unsigned long irq_state;
@@ -1851,7 +1851,7 @@ static int oz_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
static struct oz_urb_link *oz_remove_urb(struct oz_endpoint *ep,
struct urb *urb)
{
- struct oz_urb_link *urbl = NULL;
+ struct oz_urb_link *urbl;
struct list_head *e;
if (unlikely(ep == NULL))
@@ -1878,7 +1878,7 @@ static struct oz_urb_link *oz_remove_urb(struct oz_endpoint *ep,
static int oz_hcd_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
{
struct oz_hcd *ozhcd = oz_hcd_private(hcd);
- struct oz_urb_link *urbl = NULL;
+ struct oz_urb_link *urbl;
int rc;
unsigned long irq_state;
@@ -2141,7 +2141,7 @@ static int oz_clear_port_feature(struct usb_hcd *hcd, u16 wvalue, u16 windex)
static int oz_get_port_status(struct usb_hcd *hcd, u16 windex, char *buf)
{
struct oz_hcd *ozhcd;
- u32 status = 0;
+ u32 status;
if ((windex < 1) || (windex > OZ_NB_PORTS))
return -EPIPE;
diff --git a/drivers/staging/ozwpan/ozpd.c b/drivers/staging/ozwpan/ozpd.c
index 95109fd..2514d79 100644
--- a/drivers/staging/ozwpan/ozpd.c
+++ b/drivers/staging/ozwpan/ozpd.c
@@ -337,7 +337,7 @@ void oz_pd_heartbeat(struct oz_pd *pd, u16 apps)
*/
void oz_pd_stop(struct oz_pd *pd)
{
- u16 stop_apps = 0;
+ u16 stop_apps;
oz_dbg(ON, "oz_pd_stop() State = 0x%x\n", pd->state);
oz_pd_indicate_farewells(pd);
@@ -362,7 +362,7 @@ void oz_pd_stop(struct oz_pd *pd)
int oz_pd_sleep(struct oz_pd *pd)
{
int do_stop = 0;
- u16 stop_apps = 0;
+ u16 stop_apps;
oz_polling_lock_bh();
if (pd->state & (OZ_PD_S_SLEEP | OZ_PD_S_STOPPED)) {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 09/10] staging: ozwpan: Swap arguments of oz_ep_alloc() to match kmalloc()
2013-08-13 17:29 [PATCH 05/10] staging: ozwpan: Remove unnecessary pointer check Rupesh Gujare
` (2 preceding siblings ...)
2013-08-13 17:29 ` [PATCH 08/10] staging: ozwpan: Remove unneeded initializers Rupesh Gujare
@ 2013-08-13 17:29 ` Rupesh Gujare
2013-08-13 17:35 ` Joe Perches
2013-08-13 17:29 ` [PATCH 10/10] staging: ozwpan: Separate success & failure case for oz_hcd_pd_arrived() Rupesh Gujare
4 siblings, 1 reply; 12+ messages in thread
From: Rupesh Gujare @ 2013-08-13 17:29 UTC (permalink / raw)
To: devel; +Cc: dan.carpenter, linux-usb, linux-kernel, gregkh
Swap arguments of oz_ep_alloc() to match kmalloc() for better readability.
Signed-off-by: Rupesh Gujare <rupesh.gujare@atmel.com>
---
drivers/staging/ozwpan/ozhcd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
index 0fd3fea..0b21c9f 100644
--- a/drivers/staging/ozwpan/ozhcd.c
+++ b/drivers/staging/ozwpan/ozhcd.c
@@ -327,7 +327,7 @@ static void oz_empty_link_pool(void)
* allocated it immediately follows the endpoint structure.
* Context: softirq
*/
-static struct oz_endpoint *oz_ep_alloc(gfp_t mem_flags, int buffer_size)
+static struct oz_endpoint *oz_ep_alloc(int buffer_size, gfp_t mem_flags)
{
struct oz_endpoint *ep =
kzalloc(sizeof(struct oz_endpoint)+buffer_size, mem_flags);
@@ -673,7 +673,7 @@ struct oz_port *oz_hcd_pd_arrived(void *hpd)
/* Allocate an endpoint object in advance (before holding hcd lock) to
* use for out endpoint 0.
*/
- ep = oz_ep_alloc(GFP_ATOMIC, 0);
+ ep = oz_ep_alloc(0, GFP_ATOMIC);
spin_lock_bh(&ozhcd->hcd_lock);
if (ozhcd->conn_port >= 0) {
spin_unlock_bh(&ozhcd->hcd_lock);
@@ -1268,7 +1268,7 @@ static int oz_build_endpoints_for_interface(struct usb_hcd *hcd,
}
}
- ep = oz_ep_alloc(mem_flags, buffer_size);
+ ep = oz_ep_alloc(buffer_size, mem_flags);
if (!ep) {
oz_clean_endpoints_for_interface(hcd, port, if_ix);
return -ENOMEM;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 10/10] staging: ozwpan: Separate success & failure case for oz_hcd_pd_arrived()
2013-08-13 17:29 [PATCH 05/10] staging: ozwpan: Remove unnecessary pointer check Rupesh Gujare
` (3 preceding siblings ...)
2013-08-13 17:29 ` [PATCH 09/10] staging: ozwpan: Swap arguments of oz_ep_alloc() to match kmalloc() Rupesh Gujare
@ 2013-08-13 17:29 ` Rupesh Gujare
2013-08-13 23:00 ` Dan Carpenter
4 siblings, 1 reply; 12+ messages in thread
From: Rupesh Gujare @ 2013-08-13 17:29 UTC (permalink / raw)
To: devel; +Cc: dan.carpenter, linux-usb, linux-kernel, gregkh
From: Dan Carpenter <dan.carpenter@oracle.com>
This patch separates success & failure block along with fixing
following issues:-
1. The way oz_hcd_pd_arrived() looks now it's easy to think we free "ep" but
actually we do this spaghetti thing of setting it to NULL on success.
2. It is hard to read it because there are unlocks scattered throughout.
3. Currently we set "ep" to NULL on the success path and then test it and or
free it. In current code you have to scroll to the start of the function
to read code.
Original patch was submitted by Dan here :-
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2013-August/040113.html
Signed-off-by: Rupesh Gujare <rupesh.gujare@atmel.com>
---
drivers/staging/ozwpan/ozhcd.c | 54 ++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
index 0b21c9f..4cd08da 100644
--- a/drivers/staging/ozwpan/ozhcd.c
+++ b/drivers/staging/ozwpan/ozhcd.c
@@ -668,50 +668,50 @@ struct oz_port *oz_hcd_pd_arrived(void *hpd)
struct oz_endpoint *ep;
ozhcd = oz_hcd_claim();
- if (ozhcd == NULL)
+ if (!ozhcd)
return NULL;
/* Allocate an endpoint object in advance (before holding hcd lock) to
* use for out endpoint 0.
*/
ep = oz_ep_alloc(0, GFP_ATOMIC);
+ if (!ep)
+ goto err_put;
+
spin_lock_bh(&ozhcd->hcd_lock);
- if (ozhcd->conn_port >= 0) {
- spin_unlock_bh(&ozhcd->hcd_lock);
- oz_dbg(ON, "conn_port >= 0\n");
- goto out;
- }
+ if (ozhcd->conn_port >= 0)
+ goto err_unlock;
+
for (i = 0; i < OZ_NB_PORTS; i++) {
struct oz_port *port = &ozhcd->ports[i];
+
spin_lock(&port->port_lock);
- if ((port->flags & OZ_PORT_F_PRESENT) == 0) {
+ if (!(port->flags & OZ_PORT_F_PRESENT)) {
oz_acquire_port(port, hpd);
spin_unlock(&port->port_lock);
break;
}
spin_unlock(&port->port_lock);
}
- if (i < OZ_NB_PORTS) {
- oz_dbg(ON, "Setting conn_port = %d\n", i);
- ozhcd->conn_port = i;
- /* Attach out endpoint 0.
- */
- ozhcd->ports[i].out_ep[0] = ep;
- ep = NULL;
- hport = &ozhcd->ports[i];
- spin_unlock_bh(&ozhcd->hcd_lock);
- if (ozhcd->flags & OZ_HDC_F_SUSPENDED) {
- oz_dbg(ON, "Resuming root hub\n");
- usb_hcd_resume_root_hub(ozhcd->hcd);
- }
- usb_hcd_poll_rh_status(ozhcd->hcd);
- } else {
- spin_unlock_bh(&ozhcd->hcd_lock);
- }
-out:
- if (ep) /* ep is non-null if not used. */
- oz_ep_free(NULL, ep);
+ if (i == OZ_NB_PORTS)
+ goto err_unlock;
+
+ ozhcd->conn_port = i;
+ hport = &ozhcd->ports[i];
+ hport->out_ep[0] = ep;
+ spin_unlock_bh(&ozhcd->hcd_lock);
+ if (ozhcd->flags & OZ_HDC_F_SUSPENDED)
+ usb_hcd_resume_root_hub(ozhcd->hcd);
+ usb_hcd_poll_rh_status(ozhcd->hcd);
oz_hcd_put(ozhcd);
+
return hport;
+
+err_unlock:
+ spin_unlock_bh(&ozhcd->hcd_lock);
+ oz_ep_free(NULL, ep);
+err_put:
+ oz_hcd_put(ozhcd);
+ return NULL;
}
/*------------------------------------------------------------------------------
--
1.7.9.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 10/10] staging: ozwpan: Separate success & failure case for oz_hcd_pd_arrived()
2013-08-13 17:29 ` [PATCH 10/10] staging: ozwpan: Separate success & failure case for oz_hcd_pd_arrived() Rupesh Gujare
@ 2013-08-13 23:00 ` Dan Carpenter
0 siblings, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2013-08-13 23:00 UTC (permalink / raw)
To: Rupesh Gujare; +Cc: devel, gregkh, linux-usb, linux-kernel
On Tue, Aug 13, 2013 at 06:29:26PM +0100, Rupesh Gujare wrote:
> From: Dan Carpenter <dan.carpenter@oracle.com>
>
> This patch separates success & failure block along with fixing
> following issues:-
>
> 1. The way oz_hcd_pd_arrived() looks now it's easy to think we free "ep" but
> actually we do this spaghetti thing of setting it to NULL on success.
>
> 2. It is hard to read it because there are unlocks scattered throughout.
>
> 3. Currently we set "ep" to NULL on the success path and then test it and or
> free it. In current code you have to scroll to the start of the function
> to read code.
>
> Original patch was submitted by Dan here :-
> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2013-August/040113.html
>
> Signed-off-by: Rupesh Gujare <rupesh.gujare@atmel.com>
Since you gave me the author tag for this then I'll sign off on this
as well.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
regards,
dan carpenter
^ permalink raw reply [flat|nested] 12+ messages in thread