| Message ID | 1487667873-3493-1-git-send-email-marcin.rokicki@tieto.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
On 21-2-2017 10:04, Marcin Rokicki wrote: > The empty 'return;' statement in a void function should be > used to return from somewhere else then the end. > > Signed-off-by: Marcin Rokicki <marcin.rokicki@tieto.com> > --- > drivers/net/wireless/ath/ath10k/core.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c > index 59729aa..d65850e 100644 > --- a/drivers/net/wireless/ath/ath10k/core.c > +++ b/drivers/net/wireless/ath/ath10k/core.c > @@ -2268,7 +2268,7 @@ static void ath10k_core_register_work(struct work_struct *work) > status = ath10k_core_probe_fw(ar); > if (status) { > ath10k_err(ar, "could not probe fw (%d)\n", status); > - goto err; > + return; The subject seems misleading as you add a return here. Also removing the goto will break the desired behavior as expressed in the TODO below. > } > > status = ath10k_mac_register(ar); > @@ -2307,11 +2307,10 @@ static void ath10k_core_register_work(struct work_struct *work) > ath10k_mac_unregister(ar); > err_release_fw: > ath10k_core_free_firmware_files(ar); > -err: > - /* TODO: It's probably a good idea to release device from the driver > - * but calling device_release_driver() here will cause a deadlock. > - */ > - return; So to me removing this return statement and no more is sufficient for this patch. Just leave the label and TODO comment in place or fix it properly by scheduling a "release driver" work here or whatever is needed to prevent the deadlock. Regards, Arend > + > +/* TODO: It's probably a good idea to release device from the driver > + * but calling device_release_driver() here will cause a deadlock. > + */ > } > > int ath10k_core_register(struct ath10k *ar, u32 chip_id) >
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c index 59729aa..d65850e 100644 --- a/drivers/net/wireless/ath/ath10k/core.c +++ b/drivers/net/wireless/ath/ath10k/core.c @@ -2268,7 +2268,7 @@ static void ath10k_core_register_work(struct work_struct *work) status = ath10k_core_probe_fw(ar); if (status) { ath10k_err(ar, "could not probe fw (%d)\n", status); - goto err; + return; } status = ath10k_mac_register(ar); @@ -2307,11 +2307,10 @@ static void ath10k_core_register_work(struct work_struct *work) ath10k_mac_unregister(ar); err_release_fw: ath10k_core_free_firmware_files(ar); -err: - /* TODO: It's probably a good idea to release device from the driver - * but calling device_release_driver() here will cause a deadlock. - */ - return; + +/* TODO: It's probably a good idea to release device from the driver + * but calling device_release_driver() here will cause a deadlock. + */ } int ath10k_core_register(struct ath10k *ar, u32 chip_id)
The empty 'return;' statement in a void function should be used to return from somewhere else then the end. Signed-off-by: Marcin Rokicki <marcin.rokicki@tieto.com> --- drivers/net/wireless/ath/ath10k/core.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)