Re: [PATCH v2 0/2] staging: rtl8723bs: Fix multiple trailing semicolons

[Luis de Bethencourt]

On 01/11/2018 01:19 AM, Joe Perches wrote:
> On Wed, 2018-01-10 at 17:01 +0000, Luis de Bethencourt wrote:
>> On 01/10/2018 04:51 PM, Luis de Bethencourt wrote:
>>> Hi,
>>>
>>> Removing the last patch from the original series after Dan's comment.
>>>
>>> Tomorrow, I will send a new patch that removes the commented out code. This 
>>> will
>>> replace that last patch.
>>>
>>> Thanks for the review,
>>> Luis
>>>
>>>
>>> Luis de Bethencourt (2):
>>>   staging: rtl8723bs: Fix trailing semicolon
>>>   staging: rtl8723bs: Fix trailing semicolon
>>>
>>>  drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c  | 2 +-
>>>  drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c | 2 +-
>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>
>> A little bit too late but I just realized that instead of LKML (which is 
>> down),
>> I can link to the archives of the driverdev mailing list instead. 
>>
>> Here is Dan's email which I reference above:
>> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-January/115392.html
>>
>> Thanks,
>> Luis 
> 
> If you really want to remove these, perhaps do it treewide.
> 
> $ git grep -P -n ";\s*;\s*$" -- "*.[ch]"|grep -vw for|grep -vP ":\s+;\s*;\s*$"
> arch/arc/kernel/setup.c:376:  char *opt_nm = NULL;;
> arch/arc/kernel/unwind.c:369: panic("Attention !!! Dwarf FDE parsing 
> errors\n");;
> arch/arm/include/asm/kvm_emulate.h:134:   return cpsr_mode > USR_MODE;;
> arch/arm/kernel/time.c:86:static clock_access_fn __read_boot_clock = 
> dummy_clock_access;;
> arch/arm64/kernel/ptrace.c:1422:  int err, idx = 
> compat_ptrace_hbp_num_to_idx(num);;
> arch/mips/include/asm/checksum.h:113: u32 sum = (__force u32)csum;;
> arch/powerpc/kvm/book3s_xive.c:160:   return -ENOMEM;;
> arch/powerpc/platforms/powernv/pci-ioda.c:1843:   return -ENODEV;;
> arch/x86/boot/compressed/eboot.c:442: u32 *handles = (u32 *)uga_handle;;
> arch/x86/boot/compressed/eboot.c:487: u64 *handles = (u64 *)uga_handle;;
> block/sed-opal.c:493: return -EOPNOTSUPP;;
> drivers/clocksource/mips-gic-timer.c:208: return -EINVAL;;
> drivers/clocksource/mips-gic-timer.c:213: return -EINVAL;;
> drivers/clocksource/timer-sun5i.c:337:return 
> PTR_ERR(timer_base);;
> drivers/dax/device.c:136: return NULL;;
> drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c:721: union lane_status 
> dpcd_lane_status[LANE_COUNT_DP_MAX] = {{{0}}};;
> drivers/gpu/drm/amd/powerplay/amd_powerplay.c:165:return 
> -EINVAL;;
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c:683:  aspace = NULL;;
> drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c:645:block = 
> (size >> page[i].shift) << page[i].shift;;
> drivers/gpu/drm/scheduler/gpu_scheduler.c:464:int i;;
> drivers/hwmon/lm75.c:103: break;;
> drivers/iommu/intel-svm.c:196:unsigned long mask = 
> __rounddown_pow_of_two(address ^ last);;
> drivers/md/dm-thin.c:1725:struct dm_thin_endio_hook *h = 
> dm_per_bio_data(bio, sizeof(struct dm_thin_endio_hook));;
> drivers/md/raid1.c::  r1_bio->behind_master_bio = behind_bio;;
> drivers/net/can/rx-offload.c:259: return can_rx_offload_init_queue(dev, 
> offload, weight);;
> drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c:488:
> mlx5e_close_channels(>channels);;
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h:1175:
> u8 dummy4_1:1;;
> drivers/pci/pci-driver.c:971: return pm_generic_freeze_late(dev);;
> drivers/powercap/intel_rapl.c:1212:   int nr_pl, ret;;
> drivers/soc/imx/gpc.c:351:return 
> PTR_ERR(domain->supply);;
> drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c:1055:  regEA4 = 
> PHY_QueryBBReg(pDM_Odm->Adapter, rRx_Power_Before_IQK_A_2, bMaskDWord);;
> drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c:156:  tmplong2 = 
> PHY_QueryBBReg(Adapter, rFPGA0_XA_HSSIParameter2|MaskforPhySet, bMaskDWord);;
> drivers/staging/rtl8723bs/os_dep/ioctl_linux.c:970:   ret = 
> -EINVAL;;
> drivers/target/target_core_user.c:1409:   return ret;;
> include/net/vxlan.h:304:  return features;;
> net/netfilter/nft_dynset.c:167:   err = 
> nft_validate_register_load(priv->sreg_key, set->klen);;
> samples/bpf/xdp_monitor_kern.c:107:   u64 *cnt;;
> scripts/mod/modpost.c:1688:   const char *tosec = sec_name(elf, 
> get_secindex(elf, sym));;
> sound/core/pcm_native

Re: [PATCH v2 0/2] staging: rtl8723bs: Fix multiple trailing semicolons

[Luis de Bethencourt]

On 01/10/2018 04:51 PM, Luis de Bethencourt wrote:
> Hi,
> 
> Removing the last patch from the original series after Dan's comment.
> 
> Tomorrow, I will send a new patch that removes the commented out code. This 
> will
> replace that last patch.
> 
> Thanks for the review,
> Luis
> 
> 
> Luis de Bethencourt (2):
>   staging: rtl8723bs: Fix trailing semicolon
>   staging: rtl8723bs: Fix trailing semicolon
> 
>  drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c  | 2 +-
>  drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 

A little bit too late but I just realized that instead of LKML (which is down),
I can link to the archives of the driverdev mailing list instead. 

Here is Dan's email which I reference above:
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-January/115392.html

Thanks,
Luis 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 0/2] staging: rtl8723bs: Fix multiple trailing semicolons

[Luis de Bethencourt]

Hi,

Removing the last patch from the original series after Dan's comment.

Tomorrow, I will send a new patch that removes the commented out code. This will
replace that last patch.

Thanks for the review,
Luis


Luis de Bethencourt (2):
  staging: rtl8723bs: Fix trailing semicolon
  staging: rtl8723bs: Fix trailing semicolon

 drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c  | 2 +-
 drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.15.1

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

[PATCH v2 2/2] staging: rtl8723bs: Fix trailing semicolon

[Luis de Bethencourt]

The trailing semicolon is an empty statement that does nothing.
Removing it since it has no purpose.

Signed-off-by: Luis de Bethencourt <lui...@kernel.org>
---
 drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c 
b/drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c
index 21ec890fd60c..e34d133075c0 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c
@@ -153,7 +153,7 @@ static u32 phy_RFSerialRead_8723B(
NewOffset = Offset;
 
if (eRFPath == RF_PATH_A) {
-   tmplong2 = PHY_QueryBBReg(Adapter, 
rFPGA0_XA_HSSIParameter2|MaskforPhySet, bMaskDWord);;
+   tmplong2 = PHY_QueryBBReg(Adapter, 
rFPGA0_XA_HSSIParameter2|MaskforPhySet, bMaskDWord);
tmplong2 = (tmplong2 & (~bLSSIReadAddress)) | (NewOffset<<23) | 
bLSSIReadEdge;  /* T65 RF */
PHY_SetBBReg(Adapter, rFPGA0_XA_HSSIParameter2|MaskforPhySet, 
bMaskDWord, tmplong2&(~bLSSIReadEdge));
} else {
-- 
2.15.1

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

[PATCH v2 1/2] staging: rtl8723bs: Fix trailing semicolon

[Luis de Bethencourt]

The trailing semicolon is an empty statement that does no operation.
Removing it since it doesn't do anything.

Signed-off-by: Luis de Bethencourt <lui...@kernel.org>
---
 drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c 
b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
index c16e147d8adc..a99a863be656 100644
--- a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
+++ b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
@@ -1052,7 +1052,7 @@ static u8 phy_PathB_RxIQK8723B(struct adapter *padapter, 
bool configPathB)
 
/*  Check failed */
regEAC = PHY_QueryBBReg(pDM_Odm->Adapter, rRx_Power_After_IQK_A_2, 
bMaskDWord);
-   regEA4 = PHY_QueryBBReg(pDM_Odm->Adapter, rRx_Power_Before_IQK_A_2, 
bMaskDWord);;
+   regEA4 = PHY_QueryBBReg(pDM_Odm->Adapter, rRx_Power_Before_IQK_A_2, 
bMaskDWord);
 
ODM_RT_TRACE(pDM_Odm, ODM_COMP_CALIBRATION, ODM_DBG_LOUD,  ("0xeac = 
0x%x\n", regEAC));
ODM_RT_TRACE(pDM_Odm, ODM_COMP_CALIBRATION, ODM_DBG_LOUD, ("0xea4 = 
0x%x, 0xeac = 0x%x\n", regEA4, regEAC));
-- 
2.15.1

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

[PATCH 0/3] staging: rtl8723bs: Fix multiple trailing semicolons

[Luis de Bethencourt]

Hi,

This is a follow-up of yesterday's water-testing patch [0]. Just 3 more
similar cases of trailing semicolons.

Sending them separately to atomize the changes, but happy to merge them all
into one. It is easier to first try separate and later merge if people want me
to.

In patch 3/3, not sure why this file has so many lines of commented out code.
Maybe I should remove the commented out code instead. pcur_network is clearly
not being used. What do you all think?

Over the weekend I will start running a mainline kernel on my CHIP, and looking
at this driver in more depth. Fun!

Thanks,
Luis


[0] Sorry I don't have a link, LKML is down

Luis de Bethencourt (3):
  staging: rtl8723bs: Fix trailing semicolon
  staging: rtl8723bs: Fix trailing semicolon
  staging: rtl8723bs: Fix trailing semicolon in comment

 drivers/staging/rtl8723bs/core/rtw_mlme.c   | 2 +-
 drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c  | 2 +-
 drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.15.1

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

[PATCH 1/3] staging: rtl8723bs: Fix trailing semicolon

[Luis de Bethencourt]

The trailing semicolon is an empty statement that does no operation.
Removing it since it doesn't do anything.

Signed-off-by: Luis de Bethencourt <lui...@kernel.org>
---
 drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c 
b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
index c16e147d8adc..a99a863be656 100644
--- a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
+++ b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
@@ -1052,7 +1052,7 @@ static u8 phy_PathB_RxIQK8723B(struct adapter *padapter, 
bool configPathB)
 
/*  Check failed */
regEAC = PHY_QueryBBReg(pDM_Odm->Adapter, rRx_Power_After_IQK_A_2, 
bMaskDWord);
-   regEA4 = PHY_QueryBBReg(pDM_Odm->Adapter, rRx_Power_Before_IQK_A_2, 
bMaskDWord);;
+   regEA4 = PHY_QueryBBReg(pDM_Odm->Adapter, rRx_Power_Before_IQK_A_2, 
bMaskDWord);
 
ODM_RT_TRACE(pDM_Odm, ODM_COMP_CALIBRATION, ODM_DBG_LOUD,  ("0xeac = 
0x%x\n", regEAC));
ODM_RT_TRACE(pDM_Odm, ODM_COMP_CALIBRATION, ODM_DBG_LOUD, ("0xea4 = 
0x%x, 0xeac = 0x%x\n", regEA4, regEAC));
-- 
2.15.1

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

[PATCH 3/3] staging: rtl8723bs: Fix trailing semicolon in comment

[Luis de Bethencourt]

Even though the code is commented out, it has a small bug. The trailing
semicolon is an empty statement that does no operation. Removing it since
it doesn't do anything.

Signed-off-by: Luis de Bethencourt <lui...@kernel.org>
---
 drivers/staging/rtl8723bs/core/rtw_mlme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c 
b/drivers/staging/rtl8723bs/core/rtw_mlme.c
index fe739eb2cf7d..7d1f19b2960d 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
@@ -2889,7 +2889,7 @@ void rtw_update_ht_cap(struct adapter *padapter, u8 *pie, 
uint ie_len, u8 channe
struct ht_priv  *phtpriv = >htpriv;
/* struct recv_priv *precvpriv = >recvpriv; */
struct registry_priv *pregistrypriv = >registrypriv;
-   /* struct wlan_network *pcur_network = &(pmlmepriv->cur_network);; */
+   /* struct wlan_network *pcur_network = &(pmlmepriv->cur_network); */
struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
struct mlme_ext_info *pmlmeinfo = &(pmlmeext->mlmext_info);
u8 cbw40_enable = 0;
-- 
2.15.1

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

[PATCH 2/3] staging: rtl8723bs: Fix trailing semicolon

[Luis de Bethencourt]

The trailing semicolon is an empty statement that does nothing.
Removing it since it has no purpose.

Signed-off-by: Luis de Bethencourt <lui...@kernel.org>
---
 drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c 
b/drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c
index 21ec890fd60c..e34d133075c0 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c
@@ -153,7 +153,7 @@ static u32 phy_RFSerialRead_8723B(
NewOffset = Offset;
 
if (eRFPath == RF_PATH_A) {
-   tmplong2 = PHY_QueryBBReg(Adapter, 
rFPGA0_XA_HSSIParameter2|MaskforPhySet, bMaskDWord);;
+   tmplong2 = PHY_QueryBBReg(Adapter, 
rFPGA0_XA_HSSIParameter2|MaskforPhySet, bMaskDWord);
tmplong2 = (tmplong2 & (~bLSSIReadAddress)) | (NewOffset<<23) | 
bLSSIReadEdge;  /* T65 RF */
PHY_SetBBReg(Adapter, rFPGA0_XA_HSSIParameter2|MaskforPhySet, 
bMaskDWord, tmplong2&(~bLSSIReadEdge));
} else {
-- 
2.15.1

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

Re: [PATCH v2] staging: rtl8723bs: Fix trailing semicolon

[Luis de Bethencourt]

On 01/09/2018 04:46 PM, Greg Kroah-Hartman wrote:
> On Tue, Jan 09, 2018 at 04:17:01PM +0000, Luis de Bethencourt wrote:
>> Fix trailing semicolon.
> 
> Please be more explicit.  That says what it does, but not _why_ :)
> 

After walking around with my head down in shame I sent a new version.

Thanks!
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3] staging: rtl8723bs: Fix trailing semicolon

[Luis de Bethencourt]

The trailing semicolon is an empty statement that does no operation.
Removing it since it doesn't do anything.

Signed-off-by: Luis de Bethencourt <lui...@kernel.org>
---

Hi all,

Testing the waters with this cleanup patch. If it is OK to send these there are
two very similar ones, plus more in general. Happy to send them.

I have a C.H.I.P. board, which is why this driver is interesting for me to play
with.

Thanks for the review so far,
Luis

Changes since v1:
  - Added a changelog body to the commit message

Changes since v2:
  - New and improved changelog explaining why the reason for the fix

 drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c 
b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
index 3fca0c2d4c8d..da59496f6e04 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
@@ -967,7 +967,7 @@ static int rtw_wx_set_mode(struct net_device *dev, struct 
iw_request_info *a,
break;
 
default :
-   ret = -EINVAL;;
+   ret = -EINVAL;
RT_TRACE(_module_rtl871x_ioctl_os_c, _drv_err_, ("\n 
Mode: %s is not supported \n", iw_operation_mode[wrqu->mode]));
goto exit;
}
-- 
2.15.1

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

[PATCH v2] staging: rtl8723bs: Fix trailing semicolon

[Luis de Bethencourt]

Fix trailing semicolon.

Signed-off-by: Luis de Bethencourt <lui...@kernel.org>
---

Changes since v1:
  - Added a changelog body to the commit message

 drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c 
b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
index 3fca0c2d4c8d..da59496f6e04 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
@@ -967,7 +967,7 @@ static int rtw_wx_set_mode(struct net_device *dev, struct 
iw_request_info *a,
break;
 
default :
-   ret = -EINVAL;;
+   ret = -EINVAL;
RT_TRACE(_module_rtl871x_ioctl_os_c, _drv_err_, ("\n 
Mode: %s is not supported \n", iw_operation_mode[wrqu->mode]));
goto exit;
}
-- 
2.15.1

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

Re: [PATCH] staging: rtl8723bs: Fix trailing semicolon

[Luis de Bethencourt]

On 01/09/2018 04:11 PM, Hans de Goede wrote:
> Hi,
> 
> On 09-01-18 17:02, Luis de Bethencourt wrote:
>> On 01/09/2018 03:20 PM, Greg Kroah-Hartman wrote:
>>> On Tue, Jan 09, 2018 at 03:09:27PM +0000, Luis de Bethencourt wrote:
>>>> Signed-off-by: Luis de Bethencourt <lui...@kernel.org>
>>>> ---
>>>
>>> I can't take patches without any changelog text, sorry :(
>>>
>>
>> Hi Greg,
>>
>> I have done some research around previous patches to this driver and
>> I am not entirely sure what you mean.
>>
>> Sorry, but I need to ask before sending a v2.
>>
>> Do you mind I'm missing a small summary in the commit message? Fix is
>> so small I thought just the commit subject would be enough.
>>
>> It is something different let me know and I will correct it.
> 
> I'm not entirely sure, but I believe Greg may have a script for this,
> given the amount of mail he receives and commits without a changelog
> in general not being a good idea.
> 
> What I usually do with trivial patches like this, is simply take the
> part after the last : in the subject, append a "." and voila you've
> a changelog txt. So in this case that would simply be:
> 
> Fix trailing semicolon.
> 
> Regards,
> 
> Hans
> 
> 
> 

Hi Hans,

That was my plan! Resending.

I like your detailed step-by-step guide :)

Thanks,
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rtl8723bs: Fix trailing semicolon

[Luis de Bethencourt]

On 01/09/2018 03:20 PM, Greg Kroah-Hartman wrote:
> On Tue, Jan 09, 2018 at 03:09:27PM +0000, Luis de Bethencourt wrote:
>> Signed-off-by: Luis de Bethencourt <lui...@kernel.org>
>> ---
> 
> I can't take patches without any changelog text, sorry :(
> 

Hi Greg,

I have done some research around previous patches to this driver and
I am not entirely sure what you mean.

Sorry, but I need to ask before sending a v2.

Do you mind I'm missing a small summary in the commit message? Fix is
so small I thought just the commit subject would be enough.

It is something different let me know and I will correct it.

Thanks,
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: rtl8723bs: Fix trailing semicolon

[Luis de Bethencourt]

Signed-off-by: Luis de Bethencourt <lui...@kernel.org>
---

Hi all,

Testing the waters with this cleanup patch. If it is OK to send these there are
two very similar ones, plus more in general. Happy to send them.

I have a C.H.I.P. board, which is why this driver is interesting for me to play
with.

Thanks,
Luis


 drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c 
b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
index 3fca0c2d4c8d..da59496f6e04 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
@@ -967,7 +967,7 @@ static int rtw_wx_set_mode(struct net_device *dev, struct 
iw_request_info *a,
break;
 
default :
-   ret = -EINVAL;;
+   ret = -EINVAL;
RT_TRACE(_module_rtl871x_ioctl_os_c, _drv_err_, ("\n 
Mode: %s is not supported \n", iw_operation_mode[wrqu->mode]));
goto exit;
}
-- 
2.15.1

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

[PATCH 3/3] staging: lustre: llite: Remove redundant else keyword

[Luis de Bethencourt]

There is no need to use 'else' if in main branch 'return' is present.

Signed-off-by: Luis de Bethencourt <lui...@kernel.org>
---
 drivers/staging/lustre/lustre/llite/vvp_io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/vvp_io.c 
b/drivers/staging/lustre/lustre/llite/vvp_io.c
index bfae98e82d6f..e7a4778e02e4 100644
--- a/drivers/staging/lustre/lustre/llite/vvp_io.c
+++ b/drivers/staging/lustre/lustre/llite/vvp_io.c
@@ -699,7 +699,7 @@ static int vvp_io_read_start(const struct lu_env *env,
result = vvp_prep_size(env, obj, io, pos, tot, );
if (result != 0)
return result;
-   else if (exceed != 0)
+   if (exceed != 0)
goto out;
 
LU_OBJECT_HEADER(D_INODE, env, >co_lu,
-- 
2.15.1

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

[PATCH 1/3] staging: lustre: llite: Remove else after goto

[Luis de Bethencourt]

If an "if" branch is terminated by a "goto", there's no need to have an
"else" statement and an indented block of code.

Remove the "else" statement to simplify the code flow.

Signed-off-by: Luis de Bethencourt <lui...@kernel.org>
---

Hi,

The following patches remove unneeded 'else' after a 'goto' or 'return'.
They are meant to just make the code more readable and aren't functional
changes.

Thanks,
Luis

 drivers/staging/lustre/lustre/llite/dir.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/dir.c 
b/drivers/staging/lustre/lustre/llite/dir.c
index 5b2e47c246f3..f5b67a4923e3 100644
--- a/drivers/staging/lustre/lustre/llite/dir.c
+++ b/drivers/staging/lustre/lustre/llite/dir.c
@@ -1339,9 +1339,9 @@ static long ll_dir_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg)
   cmd == LL_IOC_MDC_GETINFO)) {
rc = 0;
goto skip_lmm;
-   } else {
-   goto out_req;
}
+
+   goto out_req;
}
 
if (cmd == IOC_MDC_GETFILESTRIPE ||
-- 
2.15.1

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

[PATCH 2/3] staging: lustre: llite: Remove redundant else keyword

[Luis de Bethencourt]

There is no need to use 'else' if in main branch 'goto' is present.

Signed-off-by: Luis de Bethencourt <lui...@kernel.org>
---
 drivers/staging/lustre/lustre/llite/llite_lib.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c 
b/drivers/staging/lustre/lustre/llite/llite_lib.c
index 8666f1e81ade..e84719662edf 100644
--- a/drivers/staging/lustre/lustre/llite/llite_lib.c
+++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
@@ -236,7 +236,9 @@ static int client_common_fill_super(struct super_block *sb, 
char *md, char *dt,
   "An MDT (md %s) is performing recovery, of 
which this client is not a part. Please wait for recovery to complete, abort, 
or time out.\n",
   md);
goto out;
-   } else if (err) {
+   }
+
+   if (err) {
CERROR("cannot connect to %s: rc = %d\n", md, err);
goto out;
}
-- 
2.15.1

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

Re: Greybus Future

[Luis de Bethencourt]

On 01/11/16 11:18, Bryan O'Donoghue wrote:
> On Mon, 2016-10-31 at 08:50 -0500, Alex Elder wrote:
>> My suggestion is to create a majordomo list:
>> grey...@kernel.org
> 
> I think this is a great idea. We need a forum where we can discuss
> changes to the specification and the firmware - it would be a bit anti-
> social to be blitzing de...@driverdev.osuosl.org with totally non-
> driver development related traffic.
> 
> ---
> bod

I also think this is a great idea and would enjoy following it in its
own mailing list.

Let me know if I can help.

Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 1/7] lib: string: add functions to case-convert strings

[Luis de Bethencourt]

On 11/07/16 23:46, Markus Mayer wrote:
> On 9 July 2016 at 08:30, Markus Mayer <markus.ma...@broadcom.com> wrote:
>> On 9 July 2016 at 05:04, Luis de Bethencourt <lui...@osg.samsung.com> wrote:
>>> On 08/07/16 23:43, Markus Mayer wrote:
>>>> Add a collection of generic functions to convert strings to lowercase
>>>> or uppercase.
>>>>
>>>> Changing the case of a string (with or without copying it first) seems
>>>> to be a recurring requirement in the kernel that is currently being
>>>> solved by several duplicated implementations doing the same thing. This
>>>> change aims at reducing this code duplication.
>>>>
>>>> The new functions are
>>>> void strlcpytoupper(char *dst, const char *src, size_t len);
>>>> void strlcpytolower(char *dst, const char *src, size_t len);
>>>> void strcpytoupper(char *dst, const char *src);
>>>> void strcpytolower(char *dst, const char *src);
>>>> void strtoupper(char *s);
>>>> void strtolower(char *s);
>>>>
>>>> The "str[l]cpyto*" versions of the function take a destination string
>>>> and a source string as arguments. The "strlcpyto*" versions additionally
>>>> take a length argument like strlcpy() itself. Lastly, the strto*
>>>> functions take a single string argument and modify the passed-in string.
>>>>
>>>> Like strlcpy(), and unlike strncpy(), the functions guarantee NULL
>>>> termination of the destination string.
>>>>
>>>> Signed-off-by: Markus Mayer <mma...@broadcom.com>
>>>> ---
>>>>  include/linux/string.h | 40 
>>>>  lib/string.c   | 38 ++
>>>>  2 files changed, 78 insertions(+)
>>>>
>>>> diff --git a/include/linux/string.h b/include/linux/string.h
>>>> index 26b6f6a..36c9d14 100644
>>>> --- a/include/linux/string.h
>>>> +++ b/include/linux/string.h
>>>> @@ -116,6 +116,8 @@ extern void * memchr(const void *,int,__kernel_size_t);
>>>>  #endif
>>>>  void *memchr_inv(const void *s, int c, size_t n);
>>>>  char *strreplace(char *s, char old, char new);
>>>> +extern void strlcpytoupper(char *dst, const char *src, size_t len);
>>>> +extern void strlcpytolower(char *dst, const char *src, size_t len);
>>>>
>>>>  extern void kfree_const(const void *x);
>>>>
>>>> @@ -169,4 +171,42 @@ static inline const char *kbasename(const char *path)
>>>>   return tail ? tail + 1 : path;
>>>>  }
>>>>
>>>> +/**
>>>> + * strcpytoupper - Copy string and convert to uppercase.
>>>> + * @dst: The buffer to store the result.
>>>> + * @src: The string to convert to uppercase.
>>>> + */
>>>> +static inline void strcpytoupper(char *dst, const char *src)
>>>> +{
>>>> + strlcpytoupper(dst, src, -1);
>>>> +}
>>>> +
>>>
>>> Why not use SIZE_MAX instead of -1?
>>
>> Sure. I'll change all four of them. Thanks.
> 
> Turns out there's actually a circular dependency here. SIZE_MAX is
> defined in linux/kernel.h. So, string.h would need to include
> kernel.h. But kernel.h, by way of several other headers, includes
> string.h.
> 
> Attempting to include kernel.h in string.h then leads to something like this:
> 
>   CHK include/config/kernel.release
>   CHK include/generated/uapi/linux/version.h
>   CHK include/generated/utsrelease.h
>   CC  scripts/mod/devicetable-offsets.s
>   CHK include/generated/timeconst.h
> In file included from include/linux/printk.h:289:0,
>  from include/linux/kernel.h:13,
>  from include/linux/string.h:11,
>  from include/uapi/linux/uuid.h:21,
>  from include/linux/uuid.h:19,
>  from include/linux/mod_devicetable.h:12,
>  from scripts/mod/devicetable-offsets.c:2:
> include/linux/dynamic_debug.h: In function ‘ddebug_dyndbg_module_param_cb’:
> include/linux/dynamic_debug.h:122:2: error: implicit declaration of
> function ‘strstr’ [-Werror=implicit-function-declaration]
>   if (strstr(param, "dyndbg")) {
>   ^
> include/linux/dynamic_debug.h:122:6: warning: incompatible implicit
> declaration of built-in function ‘strstr’ [enabled by default]
>   if (strstr(param, "dyndbg")) {
>   ^
> 

Re: [PATCH v3 1/7] lib: string: add functions to case-convert strings

[Luis de Bethencourt]

On 08/07/16 23:43, Markus Mayer wrote:
> Add a collection of generic functions to convert strings to lowercase
> or uppercase.
> 
> Changing the case of a string (with or without copying it first) seems
> to be a recurring requirement in the kernel that is currently being
> solved by several duplicated implementations doing the same thing. This
> change aims at reducing this code duplication.
> 
> The new functions are
> void strlcpytoupper(char *dst, const char *src, size_t len);
> void strlcpytolower(char *dst, const char *src, size_t len);
> void strcpytoupper(char *dst, const char *src);
> void strcpytolower(char *dst, const char *src);
> void strtoupper(char *s);
> void strtolower(char *s);
> 
> The "str[l]cpyto*" versions of the function take a destination string
> and a source string as arguments. The "strlcpyto*" versions additionally
> take a length argument like strlcpy() itself. Lastly, the strto*
> functions take a single string argument and modify the passed-in string.
> 
> Like strlcpy(), and unlike strncpy(), the functions guarantee NULL
> termination of the destination string.
> 
> Signed-off-by: Markus Mayer 
> ---
>  include/linux/string.h | 40 
>  lib/string.c   | 38 ++
>  2 files changed, 78 insertions(+)
> 
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 26b6f6a..36c9d14 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -116,6 +116,8 @@ extern void * memchr(const void *,int,__kernel_size_t);
>  #endif
>  void *memchr_inv(const void *s, int c, size_t n);
>  char *strreplace(char *s, char old, char new);
> +extern void strlcpytoupper(char *dst, const char *src, size_t len);
> +extern void strlcpytolower(char *dst, const char *src, size_t len);
>  
>  extern void kfree_const(const void *x);
>  
> @@ -169,4 +171,42 @@ static inline const char *kbasename(const char *path)
>   return tail ? tail + 1 : path;
>  }
>  
> +/**
> + * strcpytoupper - Copy string and convert to uppercase.
> + * @dst: The buffer to store the result.
> + * @src: The string to convert to uppercase.
> + */
> +static inline void strcpytoupper(char *dst, const char *src)
> +{
> + strlcpytoupper(dst, src, -1);
> +}
> +

Why not use SIZE_MAX instead of -1?

> +/**
> + * strcpytolower - Copy string and convert to lowercase.
> + * @dst: The buffer to store the result.
> + * @src: The string to convert to lowercase.
> + */
> +static inline void strcpytolower(char *dst, const char *src)
> +{
> + strlcpytolower(dst, src, -1);
> +}
> +

Same here, and the 2 below :)

Thanks Markus,
Luis

> +/**
> + * strtoupper - Convert string to uppercase.
> + * @s: The string to operate on.
> + */
> +static inline void strtoupper(char *s)
> +{
> + strlcpytoupper(s, s, -1);
> +}
> +
> +/**
> + * strtolower - Convert string to lowercase.
> + * @s: The string to operate on.
> + */
> +static inline void strtolower(char *s)
> +{
> + strlcpytolower(s, s, -1);
> +}
> +
>  #endif /* _LINUX_STRING_H_ */
> diff --git a/lib/string.c b/lib/string.c
> index ed83562..fd8c427 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -952,3 +952,41 @@ char *strreplace(char *s, char old, char new)
>   return s;
>  }
>  EXPORT_SYMBOL(strreplace);
> +
> +/**
> + * strlcpytoupper - Copy a length-limited string and convert to uppercase.
> + * @dst: The buffer to store the result.
> + * @src: The string to convert to uppercase.
> + * @len: Maximum string length. May be SIZE_MAX (-1) to set no limit.
> + */
> +void strlcpytoupper(char *dst, const char *src, size_t len)
> +{
> + size_t i;
> +
> + if (!len)
> + return;
> +
> + for (i = 0; i < len && src[i]; ++i)
> + dst[i] = toupper(src[i]);
> + dst[i < len ? i : i - 1] = '\0';
> +}
> +EXPORT_SYMBOL(strlcpytoupper);
> +
> +/**
> + * strlcpytolower - Copy a length-limited string and convert to lowercase.
> + * @dst: The buffer to store the result.
> + * @src: The string to convert to lowercase.
> + * @len: Maximum string length. May be SIZE_MAX (-1) to set no limit.
> + */
> +void strlcpytolower(char *dst, const char *src, size_t len)
> +{
> + size_t i;
> +
> + if (!len)
> + return;
> +
> + for (i = 0; i < len && src[i]; ++i)
> + dst[i] = tolower(src[i]);
> + dst[i < len ? i : i - 1] = '\0';
> +}
> +EXPORT_SYMBOL(strlcpytolower);
> 

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

Re: [PATCH] Staging: gdm724x: gdm_tty: Fixed a checkpatch check issue.

[Luis de Bethencourt]

On 08/07/16 20:32, Samuele Baisi wrote:
> 
> 
> Il giorno ven 8 lug 2016 alle 20:17, Luis de Bethencourt 
> <lui...@osg.samsung.com> ha scritto:
>> On 05/07/16 12:33, Samuele Baisi wrote:
>>>  Removed a blankline after an opening bracket.
>>>
>>>  Signed-off-by: Samuele Baisi <cicci...@gmail.com>
>>>  ---
>>>   drivers/staging/gdm724x/gdm_tty.c | 1 -
>>>   1 file changed, 1 deletion(-)
>>>
>>>  diff --git a/drivers/staging/gdm724x/gdm_tty.c 
>>> b/drivers/staging/gdm724x/gdm_tty.c
>>>  index eb7e252..ae39663 100644
>>>  --- a/drivers/staging/gdm724x/gdm_tty.c
>>>  +++ b/drivers/staging/gdm724x/gdm_tty.c
>>>  @@ -225,7 +225,6 @@ int register_lte_tty_device(struct tty_dev *tty_dev, 
>>> struct device *device)
>>>   int j;
>>>
>>>   for (i = 0; i < TTY_MAX_COUNT; i++) {
>>>  -
>>>   gdm = kmalloc(sizeof(*gdm), GFP_KERNEL);
>>>   if (!gdm)
>>>   return -ENOMEM;
>>>
>>
>> Patch applies cleanly and removes the checkpath issue.
>>
>> Acked-by: Luis de Bethencourt <lui...@osg.samsung.com>
>>
>> Samuele,
>>
>> It is a good idea to include the issue you are solving in the commit
>> message, in this case it would be:
>>
>> Removed a blankline after an opening bracket.
>>
>> CHECK: Blank lines aren't necessary after an open brace '{'
>> #228: FILE: drivers/staging/gdm724x/gdm_tty.c:228:
>> +   for (i = 0; i < TTY_MAX_COUNT; i++) {
>> +
>>
>> Signed-off-by:
>> ---
>>
>> Thanks :)
>> Luis
> 
> Ok, next time I'll try to be more clear.
> 
> Thank you very much and best regards.
> 
> Samuele
> 

No problem Samuele.

Looking forward to more patches from you soon :)
Feel free to CC me in them.

Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Staging: rtl8712: Fixed brace and comment style issue

[Luis de Bethencourt]

On 03/07/16 04:54, Anuradha Weeraman wrote:
> Fixed issues with coding style.
> 
> Signed-off-by: Anuradha Weeraman 
> ---
>  drivers/staging/rtl8712/xmit_linux.c | 15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/rtl8712/xmit_linux.c 
> b/drivers/staging/rtl8712/xmit_linux.c
> index 695f9b9..1e86133 100644
> --- a/drivers/staging/rtl8712/xmit_linux.c
> +++ b/drivers/staging/rtl8712/xmit_linux.c
> @@ -91,7 +91,8 @@ void r8712_set_qos(struct pkt_file *ppktfile, struct 
> pkt_attrib *pattrib)
>   } else {
>   /* "When priority processing of data frames is supported,
>* a STA's SME should send EAPOL-Key frames at the highest
> -  * priority." */
> +  * priority."
> +  */
>  
>   if (pattrib->ether_type == 0x888e)
>   UserPriority = 7;
> @@ -162,16 +163,16 @@ int r8712_xmit_entry(_pkt *pkt, struct  net_device 
> *pnetdev)
>   struct _adapter *padapter = netdev_priv(pnetdev);
>   struct xmit_priv *pxmitpriv = &(padapter->xmitpriv);
>  
> - if (!r8712_if_up(padapter)) {
> + if (!r8712_if_up(padapter))
>   goto _xmit_entry_drop;
> - }
> +
>   pxmitframe = r8712_alloc_xmitframe(pxmitpriv);
> - if (!pxmitframe) {
> + if (!pxmitframe)
>   goto _xmit_entry_drop;
> - }
> - if ((!r8712_update_attrib(padapter, pkt, >attrib))) {
> +
> + if ((!r8712_update_attrib(padapter, pkt, >attrib)))
>   goto _xmit_entry_drop;
> - }
> +
>   padapter->ledpriv.LedControlHandler(padapter, LED_CTL_TX);
>   pxmitframe->pkt = pkt;
>   if (r8712_pre_xmit(padapter, pxmitframe)) {
> 

Patch applies cleanly and removes the checkpatch issues related to
unnecessary braces and trailing */ in a different line.

Hi Anuradha,

It is a good idea to include the checkpatch issue you are solving in
the commit message, in this case it would be:

Fixed issues with coding style

WARNING: Block comments use a trailing */ on a separate line
#94: FILE: drivers/staging/rtl8712/xmit_linux.c:94:
+* priority." */

WARNING: braces {} are not necessary for single statement blocks
#165: FILE: drivers/staging/rtl8712/xmit_linux.c:165:
+   if (!r8712_if_up(padapter)) {
+   goto _xmit_entry_drop;
+   }
---

You can send each type of checkpatch issue in an separate patch. If
you do this, please fix the other issues as well :) There are some
CamelCase problems and the GPL header needs fixing.

Thanks 
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Staging: gdm724x: gdm_tty: Fixed a checkpatch check issue.

[Luis de Bethencourt]

On 05/07/16 12:33, Samuele Baisi wrote:
> Removed a blankline after an opening bracket.
> 
> Signed-off-by: Samuele Baisi <cicci...@gmail.com>
> ---
>  drivers/staging/gdm724x/gdm_tty.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/staging/gdm724x/gdm_tty.c 
> b/drivers/staging/gdm724x/gdm_tty.c
> index eb7e252..ae39663 100644
> --- a/drivers/staging/gdm724x/gdm_tty.c
> +++ b/drivers/staging/gdm724x/gdm_tty.c
> @@ -225,7 +225,6 @@ int register_lte_tty_device(struct tty_dev *tty_dev, 
> struct device *device)
>   int j;
>  
>   for (i = 0; i < TTY_MAX_COUNT; i++) {
> -
>   gdm = kmalloc(sizeof(*gdm), GFP_KERNEL);
>   if (!gdm)
>   return -ENOMEM;
> 

Patch applies cleanly and removes the checkpath issue.

Acked-by: Luis de Bethencourt <lui...@osg.samsung.com>

Samuele,

It is a good idea to include the issue you are solving in the commit
message, in this case it would be:

Removed a blankline after an opening bracket.

CHECK: Blank lines aren't necessary after an open brace '{'
#228: FILE: drivers/staging/gdm724x/gdm_tty.c:228:
+   for (i = 0; i < TTY_MAX_COUNT; i++) {
+

Signed-off-by:
---

Thanks :)
Luis

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

Re: [PATCH 01/15 RESEND] staging: dgnc: remove redundant NULL checks in

[Luis de Bethencourt]

On 04/07/16 12:36, Daeseok Youn wrote:
> The dgnc_block_til_ready() is only used in dgnc_tty_open().
> The unit data(struct un_t) was stored into tty->driver_data in 
> dgnc_tty_open().
> And also tty and un were tested about NULL so these variables doesn't
> need to check for NULL in dgnc_block_til_ready().
> 
> Signed-off-by: Daeseok Youn 
> ---
> This patch seems to be missed for a long time.
> I resend this patch without any updates.
> 
>  drivers/staging/dgnc/dgnc_tty.c | 9 ++---
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
> index 4eeecc9..6758859 100644
> --- a/drivers/staging/dgnc/dgnc_tty.c
> +++ b/drivers/staging/dgnc/dgnc_tty.c
> @@ -1172,17 +1172,12 @@ static int dgnc_block_til_ready(struct tty_struct 
> *tty,
>   struct channel_t *ch)
>  {
>   int retval = 0;
> - struct un_t *un = NULL;
> + struct un_t *un = tty->driver_data;
>   unsigned long flags;
>   uintold_flags = 0;
>   int sleep_on_un_flags = 0;
>  
> - if (!tty || tty->magic != TTY_MAGIC || !file || !ch ||
> - ch->magic != DGNC_CHANNEL_MAGIC)
> - return -ENXIO;
> -
> - un = tty->driver_data;
> - if (!un || un->magic != DGNC_UNIT_MAGIC)
> + if (!file)
>   return -ENXIO;
>  
>   spin_lock_irqsave(>ch_lock, flags);
> 

Hi,

Just curious. Are you confident of removing the tty->magic != TTY_MAGIC check?

>From what I've seen that one can catch potential errors. I might be wrong.

Thanks,
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] MAINTAINERS: Update maintainer entry for wilc1000

[Luis de Bethencourt]

On 27/06/16 15:10, Nicolas Ferre wrote:
> From: Aditya Shankar <aditya.shan...@microchip.com>
> 
> Take the maintenance of the Atmel WIFI staging driver wilc1000.
> Former maintainers are no more with Atmel.
> 
> Reported-by: Loic Lefort <loic.lef...@atmel.com>
> Signed-off-by: Aditya Shankar <aditya.shan...@microchip.com>
> Signed-off-by: Ganesh Krishna <ganesh.kris...@microchip.com>
> Signed-off-by: Nicolas Ferre <nicolas.fe...@atmel.com>
> ---
> Hi Luis, Greg, 
> 
> I'd like that we don't delete the entry for this wilc1000 driver
> but change it with the patch below.
> After the Microchip - Atmel merger, Aditya and Ganesh will take over the
> maintenance of this driver and will continue the work that our former
> colleagues started.
> 
> Thanks, bye.
> 

Nice :) This is good news

I am happy to help as well if I can get my hands on the hardware.

Acked-by: Luis de Bethencourt <lui...@osg.samsung.com>

Thanks,
Luis


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

[PATCH v3] MAINTAINERS: remove unreachable people that have left Atmel

[Luis de Bethencourt]

When sending an email regarding a patch to wilc1000, you get the following
automatic replies:
"Sung_hoon Cho is no longer with Atmel Corporation."
"Kang_hee Park is no longer with Atmel Corporation."
"Dong_ho Shin is no longer with Atmel Corporation."
"Bo_moon Kim is no longer with Atmel Corporation."
"Min_su Kim is no longer with Atmel Corporation."
"Jun_hyuk Lee is no longer with Atmel Corporation."

Removing this entry since none of the maintainers are reachable.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---

Hi,

This is unfortunate, but Greg recommended we drop the whole entry for now [0].

If nobody with more experience volunteers, I am happy to continue cleaning and
supporting this driver. This hardware is used by people in the Raspberry Pi
community, which is why I think it is worth it [1].

I am going to contact Atmel and see if they can provide me with a more complete
datasheet than they one in their website [2]. Since I would need this, plus the
hardware, before I can commit to anything.

Hopefully somebody from Atmel steps up since they are the experts.

Thanks,
Luis



[0] https://lkml.org/lkml/2016/6/26/202
[1] https://hackaday.io/project/9759-yet-another-pizero-wi-fi-hat
[2] http://www.atmel.com/Images/Atmel-42491-WILC1000B-MUT_Datasheet.pdf

 MAINTAINERS | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2d059dd..8bd968f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11075,17 +11075,6 @@ M: Forest Bond <for...@alittletooquiet.net>
 S: Odd Fixes
 F: drivers/staging/vt665?/
 
-STAGING - WILC1000 WIFI DRIVER
-M: Johnny Kim <johnny@atmel.com>
-M: Austin Shin <austin.s...@atmel.com>
-M: Chris Park <chris.p...@atmel.com>
-M: Tony Cho <tony@atmel.com>
-M: Glen Lee <glen@atmel.com>
-M: Leo Kim <leo@atmel.com>
-L: linux-wirel...@vger.kernel.org
-S: Supported
-F: drivers/staging/wilc1000/
-
 STAGING - XGI Z7,Z9,Z11 PCI DISPLAY DRIVER
 M: Arnaud Patard <arnaud.pat...@rtp-net.org>
 S: Odd Fixes
-- 
2.6.4

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

Re: [PATCH v2 1/2] staging: wilc1000: fix error handling in wilc_debugfs_init()

[Luis de Bethencourt]

On 25/06/16 23:16, Greg KH wrote:
> On Sat, Jun 25, 2016 at 10:43:33PM +0100, Luis de Bethencourt wrote:
>> On 25/06/16 22:36, Greg KH wrote:
>>> On Thu, Jun 23, 2016 at 01:36:17PM +0100, Luis de Bethencourt wrote:
>>>> The common format to check if a function returned an error pointer is to
>>>> use PTR_ERR(). Instead of ERR_PTR() which is used to return said errors.
>>>>
>>>> Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
>>>> Reviewed-by: Julian Calaby <julian.cal...@gmail.com>
>>>> ---
>>>>  drivers/staging/wilc1000/wilc_debugfs.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/staging/wilc1000/wilc_debugfs.c 
>>>> b/drivers/staging/wilc1000/wilc_debugfs.c
>>>> index fcbc95d..48797dc 100644
>>>> --- a/drivers/staging/wilc1000/wilc_debugfs.c
>>>> +++ b/drivers/staging/wilc1000/wilc_debugfs.c
>>>> @@ -107,7 +107,7 @@ static int __init wilc_debugfs_init(void)
>>>>struct wilc_debugfs_info_t *info;
>>>>  
>>>>wilc_dir = debugfs_create_dir("wilc_wifi", NULL);
>>>> -  if (wilc_dir ==  ERR_PTR(-ENODEV)) {
>>>> +  if (PTR_ERR(wilc_dir) == -ENODEV) {
>>>>/* it's not error. the debugfs is just not being enabled. */
>>>>printk("ERR, kernel has built without debugfs support\n");
>>>>return 0;
>>>
>>> No, the best way to do this is to just ignore the return value, you
>>> don't care about it.  It can be passed back into any debugfs calls just
>>> fine.
>>>
>>> So don't check the value and all is good, debugfs was written in a way
>>> to make it _easy_ to use, no need for fancy error checking at all with
>>> it.
>>>
>>> thanks,
>>>
>>> greg k-h
>>>
>>
>> Thanks for the review Greg.
>>
>> Just to make sure. You are proposing I just drop the 3 if checks? [0]
>>
>> If that's what you mean I will send a patch as soon as you confirm :)
>>
>> Happy hacking,
>> Luis
>>
>>
>>
>> [0] Making the function look like this:
>> static int __init wilc_debugfs_init(void)
>> {
>> int i;
>>
>> struct dentry *debugfs_files;
>> struct wilc_debugfs_info_t *info;
>>
>> wilc_dir = debugfs_create_dir("wilc_wifi", NULL);
>> for (i = 0; i < ARRAY_SIZE(debugfs_info); i++) {
>> info = _info[i];
>> debugfs_files = debugfs_create_file(info->name,
>> info->perm,
>> wilc_dir,
>> >data,
>> >fops);
> 
> Why even assign anything to debugfs_files?
> 

Sorry Greg.

I was just confirming I understood your suggestion, and pasted that
without cleaning the code.

Thanks for review and idea!
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3] staging: wilc1000: fix error handling in wilc_debugfs_init()

[Luis de Bethencourt]

We can just ignore the return value from debugfs_create_dir() and
debugfs_create_file(). The second one already interanlly checks the dentry
created by the first before creating the file.

debugfs was written so it would be easy to use, no need for error checking.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
Suggested-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/wilc1000/wilc_debugfs.c | 29 +
 1 file changed, 5 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_debugfs.c 
b/drivers/staging/wilc1000/wilc_debugfs.c
index fcbc95d..b052628 100644
--- a/drivers/staging/wilc1000/wilc_debugfs.c
+++ b/drivers/staging/wilc1000/wilc_debugfs.c
@@ -102,35 +102,16 @@ static struct wilc_debugfs_info_t debugfs_info[] = {
 static int __init wilc_debugfs_init(void)
 {
int i;
-
-   struct dentry *debugfs_files;
struct wilc_debugfs_info_t *info;
 
wilc_dir = debugfs_create_dir("wilc_wifi", NULL);
-   if (wilc_dir ==  ERR_PTR(-ENODEV)) {
-   /* it's not error. the debugfs is just not being enabled. */
-   printk("ERR, kernel has built without debugfs support\n");
-   return 0;
-   }
-
-   if (!wilc_dir) {
-   printk("ERR, debugfs create dir\n");
-   return -1;
-   }
-
for (i = 0; i < ARRAY_SIZE(debugfs_info); i++) {
info = _info[i];
-   debugfs_files = debugfs_create_file(info->name,
-   info->perm,
-   wilc_dir,
-   >data,
-   >fops);
-
-   if (!debugfs_files) {
-   printk("ERR fail to create the debugfs file, %s\n", 
info->name);
-   debugfs_remove_recursive(wilc_dir);
-   return -1;
-   }
+   debugfs_create_file(info->name,
+   info->perm,
+   wilc_dir,
+   >data,
+   >fops);
}
return 0;
 }
-- 
2.6.4

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

Re: [PATCH v2 1/2] staging: wilc1000: fix error handling in wilc_debugfs_init()

[Luis de Bethencourt]

On 25/06/16 22:36, Greg KH wrote:
> On Thu, Jun 23, 2016 at 01:36:17PM +0100, Luis de Bethencourt wrote:
>> The common format to check if a function returned an error pointer is to
>> use PTR_ERR(). Instead of ERR_PTR() which is used to return said errors.
>>
>> Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
>> Reviewed-by: Julian Calaby <julian.cal...@gmail.com>
>> ---
>>  drivers/staging/wilc1000/wilc_debugfs.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/wilc1000/wilc_debugfs.c 
>> b/drivers/staging/wilc1000/wilc_debugfs.c
>> index fcbc95d..48797dc 100644
>> --- a/drivers/staging/wilc1000/wilc_debugfs.c
>> +++ b/drivers/staging/wilc1000/wilc_debugfs.c
>> @@ -107,7 +107,7 @@ static int __init wilc_debugfs_init(void)
>>  struct wilc_debugfs_info_t *info;
>>  
>>  wilc_dir = debugfs_create_dir("wilc_wifi", NULL);
>> -if (wilc_dir ==  ERR_PTR(-ENODEV)) {
>> +if (PTR_ERR(wilc_dir) == -ENODEV) {
>>  /* it's not error. the debugfs is just not being enabled. */
>>  printk("ERR, kernel has built without debugfs support\n");
>>  return 0;
> 
> No, the best way to do this is to just ignore the return value, you
> don't care about it.  It can be passed back into any debugfs calls just
> fine.
> 
> So don't check the value and all is good, debugfs was written in a way
> to make it _easy_ to use, no need for fancy error checking at all with
> it.
> 
> thanks,
> 
> greg k-h
> 

Thanks for the review Greg.

Just to make sure. You are proposing I just drop the 3 if checks? [0]

If that's what you mean I will send a patch as soon as you confirm :)

Happy hacking,
Luis



[0] Making the function look like this:
static int __init wilc_debugfs_init(void)
{
int i;

struct dentry *debugfs_files;
struct wilc_debugfs_info_t *info;

wilc_dir = debugfs_create_dir("wilc_wifi", NULL);
for (i = 0; i < ARRAY_SIZE(debugfs_info); i++) {
info = _info[i];
debugfs_files = debugfs_create_file(info->name,
info->perm,
wilc_dir,
>data,
>fops);
}
return 0;
}

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

Re: [PATCH] staging: wilc1000: arrays can't be NULL

[Luis de Bethencourt]

On 24/06/16 00:54, Julian Calaby wrote:
> Hi Luis,
> 
> On Fri, Jun 24, 2016 at 9:50 AM, Luis de Bethencourt
> <lui...@osg.samsung.com> wrote:
>> On 24/06/16 00:15, Julian Calaby wrote:
>>> Hi Joe,
>>>
>>> On Fri, Jun 24, 2016 at 5:24 AM, Joe Perches <j...@perches.com> wrote:
>>>> On Thu, 2016-06-23 at 18:57 +0100, Luis de Bethencourt wrote:
>>>>> hif_drv->usr_scan_req.net.net_info[i] contains found_net_info structs
>>>>> which have the following element:
>>>>> u8 bssid[6];
>>>> []
>>>>> I am aware this patch gives a few checkpatch.pl warnings about lines being
>>>>> over 80 characters. Fixing that would be a completely different issue, and
>>>>> a lengthy one since the file has loads of them.
>>>>>
>>>>> Hopefully somebody else picks that up. Maybe I should send a hit to the
>>>>> kernelnewbies mailing list :)
>>>>
>>>> Or not.
>>>>
>>>> really_long_identifiers™ makes using 80 columns silly.
>>>>
>>>> The hungarian could probably be converted though.
>>>
>>> The main developers of this driver are slowly working through the
>>> driver's style issues, which is part of the reason why it's in
>>> staging.
>>>
>>> Thanks,
>>>
>>
>> I understand Julian,
>>
>> All the maintainers listed in the MAINTAINERS file are in CC. I will wait for
>> them to OK the suggestion of sending a patch fixing the Hungarian Notation.
> 
> I was letting you know that this work is going to happen, not
> dissuading you from doing it.
> 
>> Didn't mean to step on your toes. I just wanted to help.
> 
> No toes were stepped on. As I said, this was not a "don't do that"
> message, this was an "it's going to happen eventually" message.
> 
>> Code in staging is cared for by a lot of people :)
> 
> Indeed it is.
> 
> Thanks,
> 

Gotcha.

I will send the Hungarian Notation change tomorrow. Since it is some small help.

I will let the memcpy/memcp to ether_addr_* change for the maintainers. I 
believe
it will happen soon.

Thanks for your input.

Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: wilc1000: arrays can't be NULL

[Luis de Bethencourt]

On 24/06/16 00:15, Julian Calaby wrote:
> Hi Joe,
> 
> On Fri, Jun 24, 2016 at 5:24 AM, Joe Perches <j...@perches.com> wrote:
>> On Thu, 2016-06-23 at 18:57 +0100, Luis de Bethencourt wrote:
>>> hif_drv->usr_scan_req.net.net_info[i] contains found_net_info structs
>>> which have the following element:
>>> u8 bssid[6];
>> []
>>> I am aware this patch gives a few checkpatch.pl warnings about lines being
>>> over 80 characters. Fixing that would be a completely different issue, and
>>> a lengthy one since the file has loads of them.
>>>
>>> Hopefully somebody else picks that up. Maybe I should send a hit to the
>>> kernelnewbies mailing list :)
>>
>> Or not.
>>
>> really_long_identifiers™ makes using 80 columns silly.
>>
>> The hungarian could probably be converted though.
> 
> The main developers of this driver are slowly working through the
> driver's style issues, which is part of the reason why it's in
> staging.
> 
> Thanks,
> 

I understand Julian,

All the maintainers listed in the MAINTAINERS file are in CC. I will wait for
them to OK the suggestion of sending a patch fixing the Hungarian Notation.

Didn't mean to step on your toes. I just wanted to help.

Code in staging is cared for by a lot of people :)

Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: wilc1000: arrays can't be NULL

[Luis de Bethencourt]

On 23/06/16 20:24, Joe Perches wrote:
> On Thu, 2016-06-23 at 18:57 +0100, Luis de Bethencourt wrote:
>> hif_drv->usr_scan_req.net.net_info[i] contains found_net_info structs
>> which have the following element:
>> u8 bssid[6];
> []
>> I am aware this patch gives a few checkpatch.pl warnings about lines being
>> over 80 characters. Fixing that would be a completely different issue, and
>> a lengthy one since the file has loads of them.
>>
>> Hopefully somebody else picks that up. Maybe I should send a hit to the
>> kernelnewbies mailing list :)
> 
> Or not.
> 
> really_long_identifiers™ makes using 80 columns silly.

I agree. Not a priority, at all.

> 
> The hungarian could probably be converted though.
> 

I could look into this tomorrow.

I noticed, for example these 3 in the same function:
struct wid strWIDList[8];
u32 u32WidsCount = 0, dummyval = 0;
u8 *pu8CurrByte = NULL;

Not pretty and cleaning those should take little time.

> A log of the memcpy and memcpy uses could probably be
> converted to ether_addr_ too.
> 

Switching memcpy for ether_addr_copy and memcmp for ether_addr_equal.

I could send a patch for this as well, but I would need to have somebody
test it for me. Or maybe get this hardware for myself and do it properly.

Do you approve of my original patch?

Thanks for the review :)
Luis


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

[PATCH] staging: wilc1000: arrays can't be NULL

[Luis de Bethencourt]

hif_drv->usr_scan_req.net.net_info[i] contains found_net_info structs
which have the following element:
u8 bssid[6];

pstrNetworkInfo, of type network_info, also contains an u8 array named
bssid.

request->ssids is an array of cfg80211_ssid structs. Making ssid:
u8 ssid[IEEE80211_MAX_SSID_LEN];

In these 3 cases the arrays are being checked against NULL, which can't
happen. Removing the checks since they will always be true.

Found with smatch:
drivers/staging/wilc1000/host_interface.c:1234 Handle_RcvdNtwrkInfo() warn: 
this array is probably non-NULL. 'hif_drv->usr_scan_req.net_info[i].bssid'
drivers/staging/wilc1000/host_interface.c:1235 Handle_RcvdNtwrkInfo() warn: 
this array is probably non-NULL. 'pstrNetworkInfo->bssid'
drivers/staging/wilc1000/host_interface.c:1253 Handle_RcvdNtwrkInfo() warn: 
this array is probably non-NULL. 
'hif_drv->usr_scan_req.net_info[hif_drv->usr_scan_req.rcvd_ch_cnt].bssid'
drivers/staging/wilc1000/host_interface.c:1254 Handle_RcvdNtwrkInfo() warn: 
this array is probably non-NULL. 'pstrNetworkInfo->bssid'

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
Hi,

I am aware this patch gives a few checkpatch.pl warnings about lines being
over 80 characters. Fixing that would be a completely different issue, and
a lengthy one since the file has loads of them.

Hopefully somebody else picks that up. Maybe I should send a hit to the
kernelnewbies mailing list :)

Thanks,
Luis


 drivers/staging/wilc1000/host_interface.c | 38 ++-
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c |  3 +-
 2 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 9535842..7d5745a 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -1231,17 +1231,14 @@ static s32 Handle_RcvdNtwrkInfo(struct wilc_vif *vif,
}
 
for (i = 0; i < hif_drv->usr_scan_req.rcvd_ch_cnt; i++) {
-   if ((hif_drv->usr_scan_req.net_info[i].bssid) &&
-   (pstrNetworkInfo->bssid)) {
-   if 
(memcmp(hif_drv->usr_scan_req.net_info[i].bssid,
-  pstrNetworkInfo->bssid, 6) == 0) {
-   if (pstrNetworkInfo->rssi <= 
hif_drv->usr_scan_req.net_info[i].rssi) {
-   goto done;
-   } else {
-   
hif_drv->usr_scan_req.net_info[i].rssi = pstrNetworkInfo->rssi;
-   bNewNtwrkFound = false;
-   break;
-   }
+   if (memcmp(hif_drv->usr_scan_req.net_info[i].bssid,
+  pstrNetworkInfo->bssid, 6) == 0) {
+   if (pstrNetworkInfo->rssi <= 
hif_drv->usr_scan_req.net_info[i].rssi) {
+   goto done;
+   } else {
+   hif_drv->usr_scan_req.net_info[i].rssi 
= pstrNetworkInfo->rssi;
+   bNewNtwrkFound = false;
+   break;
}
}
}
@@ -1250,20 +1247,17 @@ static s32 Handle_RcvdNtwrkInfo(struct wilc_vif *vif,
if (hif_drv->usr_scan_req.rcvd_ch_cnt < 
MAX_NUM_SCANNED_NETWORKS) {

hif_drv->usr_scan_req.net_info[hif_drv->usr_scan_req.rcvd_ch_cnt].rssi = 
pstrNetworkInfo->rssi;
 
-   if 
(hif_drv->usr_scan_req.net_info[hif_drv->usr_scan_req.rcvd_ch_cnt].bssid &&
-   pstrNetworkInfo->bssid) {
-   
memcpy(hif_drv->usr_scan_req.net_info[hif_drv->usr_scan_req.rcvd_ch_cnt].bssid,
-  pstrNetworkInfo->bssid, 6);
+   
memcpy(hif_drv->usr_scan_req.net_info[hif_drv->usr_scan_req.rcvd_ch_cnt].bssid,
+  pstrNetworkInfo->bssid, 6);
 
-   hif_drv->usr_scan_req.rcvd_ch_cnt++;
+   hif_drv->usr_scan_req.rcvd_ch_cnt++;
 
-   pstrNetworkInfo->new_network = true;
-   pJoinParams = 
host_int_ParseJoinBssParam(pstrNetworkInfo);
+   pstrNetworkInfo->new_network = true;
+   pJoinParams = 
host_int_Par

[PATCH v2 2/2] staging: wilc1000: fix error values in wilc_debugfs_init()

[Luis de Bethencourt]

If there was an error, returning -EINVAL is more appropriate than -1.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
Reviewed-by: Julian Calaby <julian.cal...@gmail.com>
---
 drivers/staging/wilc1000/wilc_debugfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_debugfs.c 
b/drivers/staging/wilc1000/wilc_debugfs.c
index 48797dc..6252931 100644
--- a/drivers/staging/wilc1000/wilc_debugfs.c
+++ b/drivers/staging/wilc1000/wilc_debugfs.c
@@ -115,7 +115,7 @@ static int __init wilc_debugfs_init(void)
 
if (!wilc_dir) {
printk("ERR, debugfs create dir\n");
-   return -1;
+   return -EINVAL;
}
 
for (i = 0; i < ARRAY_SIZE(debugfs_info); i++) {
@@ -129,7 +129,7 @@ static int __init wilc_debugfs_init(void)
if (!debugfs_files) {
printk("ERR fail to create the debugfs file, %s\n", 
info->name);
debugfs_remove_recursive(wilc_dir);
-   return -1;
+   return -EINVAL;
}
}
return 0;
-- 
2.6.4

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

[PATCH v2 1/2] staging: wilc1000: fix error handling in wilc_debugfs_init()

[Luis de Bethencourt]

The common format to check if a function returned an error pointer is to
use PTR_ERR(). Instead of ERR_PTR() which is used to return said errors.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
Reviewed-by: Julian Calaby <julian.cal...@gmail.com>
---
 drivers/staging/wilc1000/wilc_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wilc1000/wilc_debugfs.c 
b/drivers/staging/wilc1000/wilc_debugfs.c
index fcbc95d..48797dc 100644
--- a/drivers/staging/wilc1000/wilc_debugfs.c
+++ b/drivers/staging/wilc1000/wilc_debugfs.c
@@ -107,7 +107,7 @@ static int __init wilc_debugfs_init(void)
struct wilc_debugfs_info_t *info;
 
wilc_dir = debugfs_create_dir("wilc_wifi", NULL);
-   if (wilc_dir ==  ERR_PTR(-ENODEV)) {
+   if (PTR_ERR(wilc_dir) == -ENODEV) {
/* it's not error. the debugfs is just not being enabled. */
printk("ERR, kernel has built without debugfs support\n");
return 0;
-- 
2.6.4

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

Re: [PATCH] staging: wilc1000: fix error handling in wilc_debugfs_init()

[Luis de Bethencourt]

On 23/06/16 02:29, Julian Calaby wrote:
> Hi All,
> 
> On Wed, Jun 22, 2016 at 10:39 PM, Luis de Bethencourt
> <lui...@osg.samsung.com> wrote:
>> The common format to check if a function returned an error pointer is to
>> use PTR_ERR(). Instead of ERR_PTR() which is used to return said errors.
>>
>> Also, if there was an error returning -EINVAL instead of -1 is more
>> appropriate.
> 
> These two changes could be argued to be separate changes deserving of
> their own patches.
> 
>> Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
> 
> However if everyone else is ok with that, this is:
> 
> Reviewed-by: Julian Calaby <julian.cal...@gmail.com>
> 
> Thanks,
> 

Hi Julian,

If you don't mind I will resend as two separate patches and include your
Reviewed-by in both.

Thanks for the review,
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[RESEND PATCH v2 2/2] staging: iio: accel: add error check

[Luis de Bethencourt]

Go to error_ret if sca3000_read_ctrl_reg() failed.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/iio/accel/sca3000_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/iio/accel/sca3000_core.c 
b/drivers/staging/iio/accel/sca3000_core.c
index ec12181..b3f6dbf 100644
--- a/drivers/staging/iio/accel/sca3000_core.c
+++ b/drivers/staging/iio/accel/sca3000_core.c
@@ -1046,6 +1046,8 @@ static int sca3000_clean_setup(struct sca3000_state *st)
 
/* Disable ring buffer */
ret = sca3000_read_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL);
+   if (ret < 0)
+   goto error_ret;
ret = sca3000_write_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL,
 (ret & SCA3000_OUT_CTRL_PROT_MASK)
 | SCA3000_OUT_CTRL_BUF_X_EN
-- 
2.5.1

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

[RESEND PATCH v2 1/2] staging: iio: accel: fix error check

[Luis de Bethencourt]

sca3000_read_ctrl_reg() returns a negative number on failure, check for
this instead of zero.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
Reviewed-by: Andrew F. Davis <a...@ti.com>
Reviewed-by: Jonathan Cameron <ji...@kernel.org>
---
 drivers/staging/iio/accel/sca3000_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/accel/sca3000_core.c 
b/drivers/staging/iio/accel/sca3000_core.c
index a8f533a..ec12181 100644
--- a/drivers/staging/iio/accel/sca3000_core.c
+++ b/drivers/staging/iio/accel/sca3000_core.c
@@ -594,7 +594,7 @@ static ssize_t sca3000_read_frequency(struct device *dev,
goto error_ret_mut;
ret = sca3000_read_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL);
mutex_unlock(>lock);
-   if (ret)
+   if (ret < 0)
goto error_ret;
val = ret;
if (base_freq > 0)
-- 
2.5.1

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

[RESEND PATCH] staging: gdm724x: gdm_usb: Remove ignored value

[Luis de Bethencourt]

The value assigned to ret will be overwritten before it could be read in a
future iteration of the loop. Removing the unnecessary assignment.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/gdm724x/gdm_usb.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/gdm724x/gdm_usb.c 
b/drivers/staging/gdm724x/gdm_usb.c
index d650d77..15a7e81 100644
--- a/drivers/staging/gdm724x/gdm_usb.c
+++ b/drivers/staging/gdm724x/gdm_usb.c
@@ -415,10 +415,10 @@ static void do_rx(struct work_struct *work)
switch (cmd_evt) {
case LTE_GET_INFORMATION_RESULT:
if (set_mac_address(hci->data, r->cb_data) == 0) {
-   ret = r->callback(r->cb_data,
- r->buf,
- r->urb->actual_length,
- KERNEL_THREAD);
+   r->callback(r->cb_data,
+   r->buf,
+   r->urb->actual_length,
+   KERNEL_THREAD);
}
break;
 
-- 
2.5.1

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

[PATCH] staging: wilc1000: fix error handling in wilc_debugfs_init()

[Luis de Bethencourt]

The common format to check if a function returned an error pointer is to
use PTR_ERR(). Instead of ERR_PTR() which is used to return said errors.

Also, if there was an error returning -EINVAL instead of -1 is more
appropriate.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/wilc1000/wilc_debugfs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_debugfs.c 
b/drivers/staging/wilc1000/wilc_debugfs.c
index fcbc95d..6252931 100644
--- a/drivers/staging/wilc1000/wilc_debugfs.c
+++ b/drivers/staging/wilc1000/wilc_debugfs.c
@@ -107,7 +107,7 @@ static int __init wilc_debugfs_init(void)
struct wilc_debugfs_info_t *info;
 
wilc_dir = debugfs_create_dir("wilc_wifi", NULL);
-   if (wilc_dir ==  ERR_PTR(-ENODEV)) {
+   if (PTR_ERR(wilc_dir) == -ENODEV) {
/* it's not error. the debugfs is just not being enabled. */
printk("ERR, kernel has built without debugfs support\n");
return 0;
@@ -115,7 +115,7 @@ static int __init wilc_debugfs_init(void)
 
if (!wilc_dir) {
printk("ERR, debugfs create dir\n");
-   return -1;
+   return -EINVAL;
}
 
for (i = 0; i < ARRAY_SIZE(debugfs_info); i++) {
@@ -129,7 +129,7 @@ static int __init wilc_debugfs_init(void)
if (!debugfs_files) {
printk("ERR fail to create the debugfs file, %s\n", 
info->name);
debugfs_remove_recursive(wilc_dir);
-   return -1;
+   return -EINVAL;
}
}
return 0;
-- 
2.5.1

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

Re: [PATCH v2 4/4] Staging: comedi: fix line over 80 characters issue in s626.c

[Luis de Bethencourt]

On 18/06/16 04:51, Greg KH wrote:
> On Mon, May 30, 2016 at 03:21:27PM +0100, Luis de Bethencourt wrote:
>> On 28/05/16 06:26, Ravishankar Karkala Mallikarjunayya wrote:
>>> This is a patch to the s626.c file that fixes up a  line over 80
>>> characters issues found by the checkpatch.pl tool.
>>>
>>> Signed-off-by: Ravishankar Karkala Mallikarjunayya 
>>> <ravishankark...@gmail.com>
>>
>> Looks good :)
>>
>> This is the only instance of the line over 80 characters Warning in that 
>> file.
>>
>> Like previous patches, no need to mention the file being fixed in the commit
>> message. It isn't a big deal though.
>>
>> +1
> 
> Not much I can do with at "+1", please respond with a:
>   Acked-by:
> or even better:
>   Reviewed-by:
> 
> with the proper info, so I can add it to the patch and give you credit
> for reviewing it.
> 
> thanks,
> 
> greg k-h
> 

Reviewed-by: Luis de Bethencourt <lui...@osg.samsung.com>

Sorry, sometimes I am not sure if my comments warrant an Acked-by/Reviewed-by.

Will always add one from now on.

Thanks :)
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/6] staging: i4l: icn: do not use return as a function

[Luis de Bethencourt]

On 11/06/16 22:10, Sudip Mukherjee wrote:
> return is not a function so no need to use the parenthesis.
> 
> Signed-off-by: Sudip Mukherjee 
> ---
>  drivers/staging/i4l/icn/icn.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 

All the patches in this series look like good style fixes to me.

Thanks Sudip :)
Luis

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

Re: [PATCH] staging:vt6656:card.c:Fix comment block issue

[Luis de Bethencourt]

On 11/06/16 21:08, Rithvik Patibandla wrote:
> Fix "Block comments use * on subsequent lines" and "Block comments use
> */ on trailing lines" warnings thrown by checkpatch.pl
> 
> Signed-off-by: Rithvik Patibandla 
> ---
>  drivers/staging/vt6656/card.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
> index a382fc6..607834c 100644
> --- a/drivers/staging/vt6656/card.c
> +++ b/drivers/staging/vt6656/card.c
> @@ -47,7 +47,8 @@
>  #include "usbpipe.h"
>  
>  /* const u16 cwRXBCNTSFOff[MAX_RATE] =
> -   {17, 34, 96, 192, 34, 23, 17, 11, 8, 5, 4, 3}; */
> + *   {17, 34, 96, 192, 34, 23, 17, 11, 8, 5, 4, 3};
> + */
>  
>  static const u16 cwRXBCNTSFOff[MAX_RATE] = {
>   192, 96, 34, 17, 34, 23, 17, 11, 8, 5, 4, 3
> 

Hi Rithvik,

The following warning is indeed fixed :) This patch looks good.

WARNING: Block comments use * on subsequent lines
#50: FILE: drivers/staging/vt6656/card.c:50:
+/* const u16 cwRXBCNTSFOff[MAX_RATE] =
+   {17, 34, 96, 192, 34, 23, 17, 11, 8, 5, 4, 3}; */


There are 32 other checks that checkpatch complains about in that file.
Please send fixes for those as well.

Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/8] Lustre: Multiple assignments removal.

[Luis de Bethencourt]

On 10/06/16 03:35, Oleg Drokin wrote:
> These patches remove multiple assignments in Lustre, that makes
> checkpatch (somewhat understandably) unhappy.
> 
> Nathaniel Clark (8):
>   staging/lustre/osc: Fix Multiple Assignment Warnings
>   staging/lustre/fid: Fix Multiple Assignments
>   staging/lustre/ldlm: Fix Multiple Assignments
>   staging/lustre/llite: Fix Multiple Assignments
>   staging/lustre/lov: Fix Multiple Assignments
>   staging/lustre/obdclass: Fix Multiple Assignments
>   staging/lustre/ptlrpc: Fix Multiple Assignments
>   staging/lustre/lmv: Fix Multiple Assignments
> 
>  drivers/staging/lustre/lustre/fid/fid_request.c |  6 --
>  drivers/staging/lustre/lustre/ldlm/ldlm_lib.c   |  3 ++-
>  drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c |  8 +---
>  drivers/staging/lustre/lustre/llite/llite_lib.c |  3 ++-
>  drivers/staging/lustre/lustre/llite/namei.c |  3 ++-
>  drivers/staging/lustre/lustre/llite/vvp_io.c|  3 ++-
>  drivers/staging/lustre/lustre/lmv/lmv_obd.c |  3 +--
>  drivers/staging/lustre/lustre/lov/lov_obd.c |  9 -
>  drivers/staging/lustre/lustre/obdclass/llog.c   |  6 --
>  drivers/staging/lustre/lustre/osc/osc_cache.c   |  6 --
>  drivers/staging/lustre/lustre/osc/osc_io.c  | 12 
>  drivers/staging/lustre/lustre/osc/osc_lock.c|  3 ++-
>  drivers/staging/lustre/lustre/osc/osc_request.c |  6 --
>  drivers/staging/lustre/lustre/ptlrpc/client.c   |  6 --
>  drivers/staging/lustre/lustre/ptlrpc/import.c   |  3 ++-
>  drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c |  3 ++-
>  drivers/staging/lustre/lustre/ptlrpc/pinger.c   |  3 ++-
>  drivers/staging/lustre/lustre/ptlrpc/sec_null.c |  3 ++-
>  18 files changed, 56 insertions(+), 33 deletions(-)
> 

Hi,

Documentation/CodingStyle says (in line 65):
"Don't put multiple assignments on a single line either.  Kernel coding style
is super simple.  Avoid tricky expressions."

This patch set looks good.
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] vme: Update documentation to match api

[Luis de Bethencourt]

On 05/06/16 21:35, Martyn Welch wrote:
> The vme_register_driver() api changed in commit 5d6abf379d73 ("staging:
> vme: make match() driver specific to improve non-VME64x support") but the
> documentation wasn't updated. Update the documentation to match the API.
> 
> Signed-off-by: Martyn Welch 
> ---

Nice catch :)

Thanks Martyn

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

Re: [PATCH] staging: rtl8712: Fixed multiple parenthesis alignment warnings in ieee80211.c

[Luis de Bethencourt]

On 08/06/16 13:25, Parth Sane wrote:
> No mate, that aint just cutting it.
> I tried putting it, maybe i’m doing something wrong?
> Regards,
> Parth
>> On 08-Jun-2016, at 5:46 PM, Luis de Bethencourt <lui...@osg.samsung.com> 
>> wrote:
>>
>> On 08/06/16 12:58, Parth Sane wrote:
>>> I’m trying to fix line 224 but its simply not happening. Can you help me 
>>> with this?
>>> Regards,
>>> Parth
>>>> On 08-Jun-2016, at 5:19 PM, Luis de Bethencourt <lui...@osg.samsung.com> 
>>>> wrote:
>>>>
>>>> On 08/06/16 12:37, Parth Sane wrote:
>>>>>
>>>>>> On 08-Jun-2016, at 5:01 PM, Luis de Bethencourt <lui...@osg.samsung.com> 
>>>>>> wrote:
>>>>>>
>>>>>> On 08/06/16 12:27, Parth Sane wrote:
>>>>>>> Multiple parenthesis alignment warnings were thrown by checkpatch in 
>>>>>>> ieee80211.c
>>>>>>> This patch effectively fixes that.
>>>>>>>
>>>>>>> Signed-off-by: Parth Sane <laerdevstud...@gmail.com>
>>>>>>>
>>>>>>> ---
>>>>>>> drivers/staging/rtl8712/ieee80211.c | 25 -
>>>>>>> 1 file changed, 12 insertions(+), 13 deletions(-)
>>>>>>
>>>>>> Some of the lines you change are over 80 characters. You could fix those 
>>>>>> as well in
>>>>>> a second patch :)
>>>>>>
>>>>>> Thanks,
>>>>>> Luis
>>>>>>
>>>>>
>>>>> You’re right on that regard. Maybe once this is accepted I can come back 
>>>>> to other warnings. I will submit other patches meanwhile.
>>>>> I seriously want to see this driver in the next release!
>>>>> Regards,
>>>>> Parth
>>>>>
>>>>
>>>> You can resend this patch and the one fixing the 80 char limit with the 
>>>> subject
>>>> prefix [RESEND PATCH v2 x/2].
>>>>
>>>> AFAIK this driver is already in the release, check menuconfig under
>>>> "Device Drivers > Staging drivers > Realtek RTL8172"
>>>>
>>>> Thanks,
>>>> Luis
>>>
>>
>> Sure.
>>
>> You want something like this:
>> -   if (memcmp((pbuf + 2), wpa_oui_type, 
>> sizeof(wpa_oui_type)))
>> +   if (memcmp((pbuf + 2), wpa_oui_type,
>> +  sizeof(wpa_oui_type)))
>>
>> You could check for references of how other parts of the code have solved it
>> by doing:
>> git grep memcpy * | grep -v ";"
>>
>> I hope that helps.
>> Luis
>>
>>
>>
>> Reviewed-by: Luis de Bethencourt <lui...@osg.samsung.com>
> 

What is the output you get when it doesn't work. Without that it is hard for me
to be of help.

Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rtl8712: Fixed multiple parenthesis alignment warnings in ieee80211.c

[Luis de Bethencourt]

On 08/06/16 12:58, Parth Sane wrote:
> I’m trying to fix line 224 but its simply not happening. Can you help me with 
> this?
> Regards,
> Parth
>> On 08-Jun-2016, at 5:19 PM, Luis de Bethencourt <lui...@osg.samsung.com> 
>> wrote:
>>
>> On 08/06/16 12:37, Parth Sane wrote:
>>>
>>>> On 08-Jun-2016, at 5:01 PM, Luis de Bethencourt <lui...@osg.samsung.com> 
>>>> wrote:
>>>>
>>>> On 08/06/16 12:27, Parth Sane wrote:
>>>>> Multiple parenthesis alignment warnings were thrown by checkpatch in 
>>>>> ieee80211.c
>>>>> This patch effectively fixes that.
>>>>>
>>>>> Signed-off-by: Parth Sane <laerdevstud...@gmail.com>
>>>>>
>>>>> ---
>>>>> drivers/staging/rtl8712/ieee80211.c | 25 -
>>>>> 1 file changed, 12 insertions(+), 13 deletions(-)
>>>>
>>>> Some of the lines you change are over 80 characters. You could fix those 
>>>> as well in
>>>> a second patch :)
>>>>
>>>> Thanks,
>>>> Luis
>>>>
>>>
>>> You’re right on that regard. Maybe once this is accepted I can come back to 
>>> other warnings. I will submit other patches meanwhile.
>>> I seriously want to see this driver in the next release!
>>> Regards,
>>> Parth
>>>
>>
>> You can resend this patch and the one fixing the 80 char limit with the 
>> subject
>> prefix [RESEND PATCH v2 x/2].
>>
>> AFAIK this driver is already in the release, check menuconfig under
>> "Device Drivers > Staging drivers > Realtek RTL8172"
>>
>> Thanks,
>> Luis
> 

Sure.

You want something like this:
-   if (memcmp((pbuf + 2), wpa_oui_type, 
sizeof(wpa_oui_type)))
+   if (memcmp((pbuf + 2), wpa_oui_type,
+  sizeof(wpa_oui_type)))

You could check for references of how other parts of the code have solved it
by doing:
git grep memcpy * | grep -v ";" 

I hope that helps.
Luis



Reviewed-by: Luis de Bethencourt <lui...@osg.samsung.com>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rtl8712: Fixed multiple parenthesis alignment warnings in ieee80211.c

[Luis de Bethencourt]

On 08/06/16 12:37, Parth Sane wrote:
> 
>> On 08-Jun-2016, at 5:01 PM, Luis de Bethencourt <lui...@osg.samsung.com> 
>> wrote:
>>
>> On 08/06/16 12:27, Parth Sane wrote:
>>> Multiple parenthesis alignment warnings were thrown by checkpatch in 
>>> ieee80211.c
>>> This patch effectively fixes that.
>>>
>>> Signed-off-by: Parth Sane <laerdevstud...@gmail.com>
>>>
>>> ---
>>> drivers/staging/rtl8712/ieee80211.c | 25 -
>>> 1 file changed, 12 insertions(+), 13 deletions(-)
>>
>> Some of the lines you change are over 80 characters. You could fix those as 
>> well in
>> a second patch :)
>>
>> Thanks,
>> Luis
>>
> 
> You’re right on that regard. Maybe once this is accepted I can come back to 
> other warnings. I will submit other patches meanwhile.
> I seriously want to see this driver in the next release!
> Regards,
> Parth
> 

You can resend this patch and the one fixing the 80 char limit with the subject
prefix [RESEND PATCH v2 x/2].

AFAIK this driver is already in the release, check menuconfig under
"Device Drivers > Staging drivers > Realtek RTL8172"

Thanks,
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rtl8712: Fixed multiple parenthesis alignment warnings in ieee80211.c

[Luis de Bethencourt]

On 08/06/16 12:27, Parth Sane wrote:
> Multiple parenthesis alignment warnings were thrown by checkpatch in 
> ieee80211.c
> This patch effectively fixes that.
> 
> Signed-off-by: Parth Sane 
> 
> ---
>  drivers/staging/rtl8712/ieee80211.c | 25 -
>  1 file changed, 12 insertions(+), 13 deletions(-)

Some of the lines you change are over 80 characters. You could fix those as 
well in
a second patch :)

Thanks,
Luis

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

Re: [PATCH] staging: iio: ad5933: fix handling of settling time cycles

[Luis de Bethencourt]

On 01/06/16 17:22, Lars-Peter Clausen wrote:
> On 06/01/2016 05:55 PM, Luis de Bethencourt wrote:
>> Correctly handle the settling time cycles value. The else branch was an
>> impossible condition (> 1022 in the else branch of > 511) and the handling
>> of the values was dividing by 2 and 4, with a left shift, instead of
>> multiplying.
>>
>> Based on the Table 13 at the bottom of Page 25 of the Data Sheet:
>> http://www.analog.com/media/en/technical-documentation/data-sheets/AD5933.pdf
>>
>> Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
>> ---
>>
>> Hi,
>>
>> I decided to use the hexadecimal values instead of (1 << 10) and (1 << 9), 
>> for
>> briefness, I could resend using those instead if it is prefered.
>>
>> I also decided to use multiplications instead of right-shifts for 
>> readability.
>> I could use change that as well.
>>
>> Thanks,
>> Luis
>>
>>  drivers/staging/iio/impedance-analyzer/ad5933.c | 11 +++
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c 
>> b/drivers/staging/iio/impedance-analyzer/ad5933.c
>> index 9f43976..3a2cf8f3 100644
>> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
>> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
>> @@ -444,10 +444,13 @@ static ssize_t ad5933_store(struct device *dev,
>>  st->settling_cycles = val;
>>  
>>  /* 2x, 4x handling, see datasheet */
>> -if (val > 511)
>> -val = (val >> 1) | (1 << 9);
>> -else if (val > 1022)
>> -val = (val >> 2) | (3 << 9);
>> +if (val & 0x400 && val & 0x200) {
>> +val &= 0x1ff;
>> +val *= 4;
>> +} else if (val & 0x200) {
>> +val &= 0x1ff;
>> +val *= 2;
>> +}
> 
> This does not look correct. D10 and D9 select an additional multiplier of
> either 1, 2 or 4. So dividing the value before writing it to the register is
> the right approach in that case. Just flipping the order in which the
> conditions are evaluated should be sufficient.
> 
>>  
>>  dat = cpu_to_be16(val);
>>  ret = ad5933_i2c_write(st->client,
>>
> 

I misunderstood the register being read instead of being written. Looking at it
now, I have no idea why. Sorry.

Will resend a patch flipping the order of conditions.

Thanks for the review,
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: iio: ad5933: fix handling of settling time cycles

[Luis de Bethencourt]

Correctly handle the settling time cycles value. The else branch was an
impossible condition (> 1022 in the else branch of > 511) and the handling
of the values was dividing by 2 and 4, with a left shift, instead of
multiplying.

Based on the Table 13 at the bottom of Page 25 of the Data Sheet:
http://www.analog.com/media/en/technical-documentation/data-sheets/AD5933.pdf

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---

Hi,

I decided to use the hexadecimal values instead of (1 << 10) and (1 << 9), for
briefness, I could resend using those instead if it is prefered.

I also decided to use multiplications instead of right-shifts for readability.
I could use change that as well.

Thanks,
Luis

 drivers/staging/iio/impedance-analyzer/ad5933.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c 
b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 9f43976..3a2cf8f3 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -444,10 +444,13 @@ static ssize_t ad5933_store(struct device *dev,
st->settling_cycles = val;
 
/* 2x, 4x handling, see datasheet */
-   if (val > 511)
-   val = (val >> 1) | (1 << 9);
-   else if (val > 1022)
-   val = (val >> 2) | (3 << 9);
+   if (val & 0x400 && val & 0x200) {
+   val &= 0x1ff;
+   val *= 4;
+   } else if (val & 0x200) {
+   val &= 0x1ff;
+   val *= 2;
+   }
 
dat = cpu_to_be16(val);
ret = ad5933_i2c_write(st->client,
-- 
2.5.1

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

Re: [PATCH 1/2] staging: iio: accel: fix error check

[Luis de Bethencourt]

On 31/05/16 22:08, Luis de Bethencourt wrote:
> sca3000_read_ctrl_reg() returns a negative number on failure, check for
> this instead of zero.
> 
> Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
> Reviewed-by: Andrew F. Davis <a...@ti.com>
> Reviewed-by: Jonathan Cameron <ji...@kernel.org>
> ---
> 
> Patch corrected thanks to comments from Andrew F. Davis and Jonathan Cameron.
> https://lkml.org/lkml/2016/5/31/862
> 
> Thanks,
> Luis
> 
>  drivers/staging/iio/accel/sca3000_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/accel/sca3000_core.c 
> b/drivers/staging/iio/accel/sca3000_core.c
> index a8f533a..ec12181 100644
> --- a/drivers/staging/iio/accel/sca3000_core.c
> +++ b/drivers/staging/iio/accel/sca3000_core.c
> @@ -594,7 +594,7 @@ static ssize_t sca3000_read_frequency(struct device *dev,
>   goto error_ret_mut;
>   ret = sca3000_read_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL);
>   mutex_unlock(>lock);
> - if (ret)
> + if (ret < 0)
>   goto error_ret;
>   val = ret;
>   if (base_freq > 0)
> 

I sent this with git send-email using --subject-prefix="PATCH v2", no idea why 
the version wasn't
added.

Apologies,
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/2] staging: iio: accel: fix error check

[Luis de Bethencourt]

sca3000_read_ctrl_reg() returns a negative number on failure, check for
this instead of zero.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
Reviewed-by: Andrew F. Davis <a...@ti.com>
Reviewed-by: Jonathan Cameron <ji...@kernel.org>
---

Patch corrected thanks to comments from Andrew F. Davis and Jonathan Cameron.
https://lkml.org/lkml/2016/5/31/862

Thanks,
Luis

 drivers/staging/iio/accel/sca3000_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/accel/sca3000_core.c 
b/drivers/staging/iio/accel/sca3000_core.c
index a8f533a..ec12181 100644
--- a/drivers/staging/iio/accel/sca3000_core.c
+++ b/drivers/staging/iio/accel/sca3000_core.c
@@ -594,7 +594,7 @@ static ssize_t sca3000_read_frequency(struct device *dev,
goto error_ret_mut;
ret = sca3000_read_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL);
mutex_unlock(>lock);
-   if (ret)
+   if (ret < 0)
goto error_ret;
val = ret;
if (base_freq > 0)
-- 
2.5.1

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

[PATCH 2/2] staging: iio: accel: add error check

[Luis de Bethencourt]

Go to error_ret if sca3000_read_ctrl_reg() failed.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---

Noticed this while inspecting the code for the 1/2 patch.

Thanks,
Luis

 drivers/staging/iio/accel/sca3000_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/iio/accel/sca3000_core.c 
b/drivers/staging/iio/accel/sca3000_core.c
index ec12181..b3f6dbf 100644
--- a/drivers/staging/iio/accel/sca3000_core.c
+++ b/drivers/staging/iio/accel/sca3000_core.c
@@ -1046,6 +1046,8 @@ static int sca3000_clean_setup(struct sca3000_state *st)
 
/* Disable ring buffer */
ret = sca3000_read_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL);
+   if (ret < 0)
+   goto error_ret;
ret = sca3000_write_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL,
 (ret & SCA3000_OUT_CTRL_PROT_MASK)
 | SCA3000_OUT_CTRL_BUF_X_EN
-- 
2.5.1

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

Re: [PATCH] staging: iio: accel: remove impossible condition

[Luis de Bethencourt]

On 31/05/16 21:23, Jonathan Cameron wrote:
> 
> 
> On 31 May 2016 20:47:50 BST, Luis de Bethencourt <lui...@osg.samsung.com> 
> wrote:
>> val is set to the value of ret right after ret is checked. If ret is
>> not
>> zero it goes to error_ret. So only value ret can have is zero, which
>> makes
>> the switch (val & 0x03) only match the case 0x00. Removing the switch
>> and
>> since val is only used for this, removing val as well.
> There is clearly an issue here. However it looks like it is that if(ret) 
> which is wrong.
> 
> The code as it stands clearly doesn't work as intended. Fixing the bug would 
> be
> 
>  more useful than removing code that 'should' be accessible.
> 
> I happen to fire the relevant hardware up yesterday for the first time in a 
> while so
> 
> can easily verify the operation of a fix if you want to take another look.
> 
> Jonathan

Jonathan and Andrew are right.

sca3000_read_ctrl_reg() returns a negative number on failure.

So line 597 should be: if (ret < 0)

If everything goes well in sca3000_read_ctrl_reg() it returns st->rx[0], which 
explains
the switch case.

I am going to send a new patch with this fix.

Thanks so much for the review and sorry for the initial confusion,
Luis

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

[PATCH] staging: iio: accel: remove impossible condition

[Luis de Bethencourt]

val is set to the value of ret right after ret is checked. If ret is not
zero it goes to error_ret. So only value ret can have is zero, which makes
the switch (val & 0x03) only match the case 0x00. Removing the switch and
since val is only used for this, removing val as well.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/iio/accel/sca3000_core.c | 16 ++--
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/iio/accel/sca3000_core.c 
b/drivers/staging/iio/accel/sca3000_core.c
index a8f533a..94656f6 100644
--- a/drivers/staging/iio/accel/sca3000_core.c
+++ b/drivers/staging/iio/accel/sca3000_core.c
@@ -586,7 +586,7 @@ static ssize_t sca3000_read_frequency(struct device *dev,
 {
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct sca3000_state *st = iio_priv(indio_dev);
-   int ret, len = 0, base_freq = 0, val;
+   int ret, len = 0, base_freq = 0;
 
mutex_lock(>lock);
ret = __sca3000_get_base_freq(st, st->info, _freq);
@@ -596,20 +596,8 @@ static ssize_t sca3000_read_frequency(struct device *dev,
mutex_unlock(>lock);
if (ret)
goto error_ret;
-   val = ret;
if (base_freq > 0)
-   switch (val & 0x03) {
-   case 0x00:
-   case 0x03:
-   len = sprintf(buf, "%d\n", base_freq);
-   break;
-   case 0x01:
-   len = sprintf(buf, "%d\n", base_freq / 2);
-   break;
-   case 0x02:
-   len = sprintf(buf, "%d\n", base_freq / 4);
-   break;
-   }
+   len = sprintf(buf, "%d\n", base_freq);
 
return len;
 error_ret_mut:
-- 
2.5.1

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

[PATCH] staging: gdm724x: gdm_usb: Remove ignored value

[Luis de Bethencourt]

The value assigned to ret will be overwritten before it could be read in a
future iteration of the loop. Removing the unnecessary assignment.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/gdm724x/gdm_usb.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/gdm724x/gdm_usb.c 
b/drivers/staging/gdm724x/gdm_usb.c
index d650d77..15a7e81 100644
--- a/drivers/staging/gdm724x/gdm_usb.c
+++ b/drivers/staging/gdm724x/gdm_usb.c
@@ -415,10 +415,10 @@ static void do_rx(struct work_struct *work)
switch (cmd_evt) {
case LTE_GET_INFORMATION_RESULT:
if (set_mac_address(hci->data, r->cb_data) == 0) {
-   ret = r->callback(r->cb_data,
- r->buf,
- r->urb->actual_length,
- KERNEL_THREAD);
+   r->callback(r->cb_data,
+   r->buf,
+   r->urb->actual_length,
+   KERNEL_THREAD);
}
break;
 
-- 
2.5.1

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

Re: [PATCH 2/2] staging: slicoss: fix camel case check

[Luis de Bethencourt]

On 29/05/16 14:31, Jaime Arrocha wrote:
> From: Jaime Arrocha 
> 
> Fixed several checks about camel case use provided by checkpatch.pl.
> CHECK: Avoid CamelCase
> Changes verified to work by compiling module.
> 
> Signed-off-by: Jaime Arrocha 

Purpose of the patch is good but it fails to apply in my clone of the 
staging-testing
branch. Not sure why.

Can you check it applies cleanly in your side?

Thanks,
Luis


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

Re: [PATCH v2 4/4] Staging: comedi: fix line over 80 characters issue in s626.c

[Luis de Bethencourt]

On 28/05/16 06:26, Ravishankar Karkala Mallikarjunayya wrote:
> This is a patch to the s626.c file that fixes up a  line over 80
> characters issues found by the checkpatch.pl tool.
> 
> Signed-off-by: Ravishankar Karkala Mallikarjunayya 

Looks good :)

This is the only instance of the line over 80 characters Warning in that file.

Like previous patches, no need to mention the file being fixed in the commit
message. It isn't a big deal though.

+1

Luis

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

Re: [PATCH v2 3/4] Staging: comedi:Fix unsigned int to bare use of unsigned issue in s626.c

[Luis de Bethencourt]

On 28/05/16 06:26, Ravishankar Karkala Mallikarjunayya wrote:
> This is a patch to the s626.c file that fixes up a
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> found by the checkpatch.pl tool
> 
> Signed-off-by: Ravishankar Karkala Mallikarjunayya 

The patch looks good :) adding the int makes it more verbose.

Just the same comment as the previous patch, we can see in the diff which file
is affected by this. You can just say:
"This fixes a WARNING: Prefer 'unsigned int' ..."

Thanks,
Luis

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

Re: [PATCH v2 2/4] Staging: comedi: Fix WARNING issue in s626.c

[Luis de Bethencourt]

On 28/05/16 06:26, Ravishankar Karkala Mallikarjunayya wrote:
> This is a patch to the s626.c file that fixes up a Block comments
> issues found by the checkpatch.pl tool.
> 
> i.e. Block comments use a trailing */ on a separate line
> 
> Signed-off-by: Ravishankar Karkala Mallikarjunayya 

We can see in the diff this is making changes in s626.c :) no need to mention
that in the commit message.

Not sure moving the opening and closing of comments (/* and */) to a new line
makes things more readable. checkpatch is a great tool but it gives suggestions,
not hard rules.
Just a comment since I've seen multiple people follow checkpatch blindly, and
it is best to use your discretion.

Maintainer of this code will probably have a preference.

Thank you for your patches Ravishankar.

Luis

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

Re: [PATCH v2 1/4] staging : comedi : fix type issues in s626.c

[Luis de Bethencourt]

On 28/05/16 06:26, Ravishankar Karkala Mallikarjunayya wrote:
> This is a patch to the s626.c file that fixes up a type issues like
> i.e Prefer kernel type 'u8' over 'uint8_t'
> Prefer kernel type 'u16' over 'uint16_t'
> Prefer kernel type 'u32' over 'uint32_t'
> Prefer kernel type 's16' over 'int16_t'
> Prefer kernel type 's32' over 'int32_t'
> found by the checkpatch.pl tool.
> 
> Signed-off-by: Ravishankar Karkala Mallikarjunayya 

>From the CodingStyle [0]:

"Although it would only take a short amount of time for the eyes and
brain to become accustomed to the standard types like 'uint32_t',
some people object to their use anyway.

Therefore, the Linux-specific 'u8/u16/u32/u64' types and their
signed equivalents which are identical to standard types are
permitted -- although they are not mandatory in new code of your
own.

When editing existing code which already uses one or the other set
of types, you should conform to the existing choices in that code."

Not sure why checkpatch complains about this when the CodingStyle suggests
to conform to the choice in the existing code.

Anybody can shed some light on this?

Thanks,
Luis



[0] 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle#n318

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

Re: [PATCH 7/7] staging: dgnc: remove useless variable 'ch_intr_rx'

[Luis de Bethencourt]

On 27/05/16 07:55, Daeseok Youn wrote:
> The 'ch_intr_rx' variable was used only for increasing.
> So the 'ch_intr_rx' variable is not useful for this driver.
> 
> Signed-off-by: Daeseok Youn 
> ---
>  drivers/staging/dgnc/dgnc_cls.c| 1 -
>  drivers/staging/dgnc/dgnc_driver.h | 2 --
>  drivers/staging/dgnc/dgnc_neo.c| 2 --
>  3 files changed, 5 deletions(-)
> 
> diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c
> index 695f8ce..fb74968 100644
> --- a/drivers/staging/dgnc/dgnc_cls.c
> +++ b/drivers/staging/dgnc/dgnc_cls.c
> @@ -387,7 +387,6 @@ static inline void cls_parse_isr(struct dgnc_board *brd, 
> uint port)
>   /* Receive Interrupt pending */
>   if (isr & (UART_IIR_RDI | UART_IIR_RDI_TIMEOUT)) {
>   /* Read data from uart -> queue */
> - ch->ch_intr_rx++;
>   cls_copy_data_from_uart_to_queue(ch);
>   dgnc_check_queue_flow_control(ch);
>   }
> diff --git a/drivers/staging/dgnc/dgnc_driver.h 
> b/drivers/staging/dgnc/dgnc_driver.h
> index 8bdf556..3092abe 100644
> --- a/drivers/staging/dgnc/dgnc_driver.h
> +++ b/drivers/staging/dgnc/dgnc_driver.h
> @@ -377,8 +377,6 @@ struct channel_t {
>   ulong   ch_xon_sends;   /* Count of xons transmitted */
>   ulong   ch_xoff_sends;  /* Count of xoffs transmitted */
>  
> - ulong   ch_intr_rx; /* Count of interrupts */
> -
>   /* /proc// entries */
>   struct proc_dir_entry *proc_entry_pointer;
>   struct dgnc_proc_entry *dgnc_channel_table;
> diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
> index 574a227..5ef5b3e 100644
> --- a/drivers/staging/dgnc/dgnc_neo.c
> +++ b/drivers/staging/dgnc/dgnc_neo.c
> @@ -399,7 +399,6 @@ static inline void neo_parse_isr(struct dgnc_board *brd, 
> uint port)
>  
>   if (isr & (UART_17158_IIR_RDI_TIMEOUT | UART_IIR_RDI)) {
>   /* Read data from uart -> queue */
> - ch->ch_intr_rx++;
>   neo_copy_data_from_uart_to_queue(ch);
>  
>   /* Call our tty layer to enforce queue flow control if 
> needed. */
> @@ -512,7 +511,6 @@ static inline void neo_parse_lsr(struct dgnc_board *brd, 
> uint port)
>   ch->ch_cached_lsr |= linestatus;
>  
>   if (ch->ch_cached_lsr & UART_LSR_DR) {
> - ch->ch_intr_rx++;
>   /* Read data from uart -> queue */
>   neo_copy_data_from_uart_to_queue(ch);
>   spin_lock_irqsave(>ch_lock, flags);
> 

Applied and checked them all. They look good.

Surprising amount of unused variables. Only set but never read.

Thanks Daeseok!
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/7] staging: dgnc: remove unused variable 'intr_modem'

[Luis de Bethencourt]

On 27/05/16 07:53, Daeseok Youn wrote:
> The 'intr_modem' variable was used only for increasing.
> So the 'intr_modem' variable is not useful for this driver.
> 
> Signed-off-by: Daeseok Youn 
> ---
>  drivers/staging/dgnc/dgnc_cls.c| 1 -
>  drivers/staging/dgnc/dgnc_driver.h | 1 -
>  drivers/staging/dgnc/dgnc_neo.c| 1 -
>  3 files changed, 3 deletions(-)
> 
> diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c
> index dcb808f..8cb7361 100644
> --- a/drivers/staging/dgnc/dgnc_cls.c
> +++ b/drivers/staging/dgnc/dgnc_cls.c
> @@ -406,7 +406,6 @@ static inline void cls_parse_isr(struct dgnc_board *brd, 
> uint port)
>  
>   /* CTS/RTS change of state */
>   if (isr & UART_IIR_CTSRTS) {
> - brd->intr_modem++;
>   ch->ch_intr_modem++;
>   /*
>* Don't need to do anything, the cls_parse_modem
> diff --git a/drivers/staging/dgnc/dgnc_driver.h 
> b/drivers/staging/dgnc/dgnc_driver.h
> index 7ac33ed..2caeff7 100644
> --- a/drivers/staging/dgnc/dgnc_driver.h
> +++ b/drivers/staging/dgnc/dgnc_driver.h
> @@ -183,7 +183,6 @@ struct dgnc_board {
>   uintnasync; /* Number of ports on card */
>  
>   uintirq;/* Interrupt request number */
> - ulong   intr_modem; /* Count of interrupts */
>   ulong   intr_tx;/* Count of interrupts */
>   ulong   intr_rx;/* Count of interrupts */
>  
> diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
> index 3f8bbb3..a3e100b 100644
> --- a/drivers/staging/dgnc/dgnc_neo.c
> +++ b/drivers/staging/dgnc/dgnc_neo.c
> @@ -452,7 +452,6 @@ static inline void neo_parse_isr(struct dgnc_board *brd, 
> uint port)
>* If we get here, this means the hardware is doing 
> auto flow control.
>* Check to see whether RTS/DTR or CTS/DSR caused this 
> interrupt.
>*/
> - brd->intr_modem++;
>   ch->ch_intr_modem++;
>   cause = readb(>ch_neo_uart->mcr);
>   /* Which pin is doing auto flow? RTS or DTR? */
> 

Looks good.

Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/7] staging: dgnc: remove useless variable 'intr_count'

[Luis de Bethencourt]

On 27/05/16 07:52, Daeseok Youn wrote:
> The 'intr_count' variable was used only for increasing.
> So the 'intr_count' variable is not useful for this driver.
> 
> Signed-off-by: Daeseok Youn 
> ---
>  drivers/staging/dgnc/dgnc_cls.c| 2 --
>  drivers/staging/dgnc/dgnc_driver.h | 1 -
>  drivers/staging/dgnc/dgnc_neo.c| 2 --
>  3 files changed, 5 deletions(-)
> 
> diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c
> index 46c050c..dcb808f 100644
> --- a/drivers/staging/dgnc/dgnc_cls.c
> +++ b/drivers/staging/dgnc/dgnc_cls.c
> @@ -760,8 +760,6 @@ static irqreturn_t cls_intr(int irq, void *voidbrd)
>  
>   spin_lock_irqsave(>bd_intr_lock, flags);
>  
> - brd->intr_count++;
> -
>   /*
>* Check the board's global interrupt offset to see if we
>* we actually do have an interrupt pending for us.
> diff --git a/drivers/staging/dgnc/dgnc_driver.h 
> b/drivers/staging/dgnc/dgnc_driver.h
> index 95ec729..7ac33ed 100644
> --- a/drivers/staging/dgnc/dgnc_driver.h
> +++ b/drivers/staging/dgnc/dgnc_driver.h
> @@ -183,7 +183,6 @@ struct dgnc_board {
>   uintnasync; /* Number of ports on card */
>  
>   uintirq;/* Interrupt request number */
> - ulong   intr_count; /* Count of interrupts */
>   ulong   intr_modem; /* Count of interrupts */
>   ulong   intr_tx;/* Count of interrupts */
>   ulong   intr_rx;/* Count of interrupts */
> diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
> index ba57e95..3f8bbb3 100644
> --- a/drivers/staging/dgnc/dgnc_neo.c
> +++ b/drivers/staging/dgnc/dgnc_neo.c
> @@ -926,8 +926,6 @@ static irqreturn_t neo_intr(int irq, void *voidbrd)
>   if (!brd || brd->magic != DGNC_BOARD_MAGIC)
>   return IRQ_NONE;
>  
> - brd->intr_count++;
> -
>   /* Lock out the slow poller from running on this board. */
>   spin_lock_irqsave(>bd_intr_lock, flags);
>  
> 

Good catch. intr_count is set and incremented but never read.

Thanks,
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2 V2] staging: dgnc: remove redundant null check in

[Luis de Bethencourt]

On 27/05/16 02:43, Daeseok Youn wrote:
> the "brd" was already checked for NULL before calling dgnc_do_remap().
> 
> the dgnc_do_remap() function was called only
> from the dgnc_found_board() and the DGNC_BOARD_MAGIC value
> was assigned to "brd->magic" in dgcn_found_board(). So it doesn't
> need to check about magic value.
> 
> Signed-off-by: Daeseok Youn 
> ---
> V2: Adds more comments to Change Log.
> 
>  drivers/staging/dgnc/dgnc_driver.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/staging/dgnc/dgnc_driver.c 
> b/drivers/staging/dgnc/dgnc_driver.c
> index 22257d2..1882ef5 100644
> --- a/drivers/staging/dgnc/dgnc_driver.c
> +++ b/drivers/staging/dgnc/dgnc_driver.c
> @@ -599,9 +599,6 @@ static int dgnc_finalize_board_init(struct dgnc_board 
> *brd)
>   */
>  static void dgnc_do_remap(struct dgnc_board *brd)
>  {
> - if (!brd || brd->magic != DGNC_BOARD_MAGIC)
> - return;
> -
>   brd->re_map_membase = ioremap(brd->membase, 0x1000);
>  }
>  
> 

Looks good.

Thanks for the fixes Daeseok :)

Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2 v2] staging: dgnc: remove redundant NULL check for brd

[Luis de Bethencourt]

On 27/05/16 02:42, Daeseok Youn wrote:
> the "brd" value cannot be NULL in dgnc_finalize_board_init().
> Because "brd" as a parameter of this function was already
> checked for NULL.
> 
> the dgnc_finalize_board_init() as a static function was called
> only from dgnc_found_board() function and brd->magic value
> was assigned once in dgnc_found_board(). So it doesn't need to
> check for DGNC_BOARD_MAGIC value.
> 
> Signed-off-by: Daeseok Youn 
> ---
> V2: Adds more comments to Change Log.
> 
>  drivers/staging/dgnc/dgnc_driver.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/staging/dgnc/dgnc_driver.c 
> b/drivers/staging/dgnc/dgnc_driver.c
> index af2e835..22257d2 100644
> --- a/drivers/staging/dgnc/dgnc_driver.c
> +++ b/drivers/staging/dgnc/dgnc_driver.c
> @@ -579,9 +579,6 @@ static int dgnc_finalize_board_init(struct dgnc_board 
> *brd)
>  {
>   int rc = 0;
>  
> - if (!brd || brd->magic != DGNC_BOARD_MAGIC)
> - return -ENODEV;
> -
>   if (brd->irq) {
>   rc = request_irq(brd->irq, brd->bd_ops->intr,
>IRQF_SHARED, "DGNC", brd);
> 

Looks good.

Thanks for the fixes Daeseok :)

Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/3] fix issues in prism2usb.c.

[Luis de Bethencourt]

On 26/05/16 13:32, Kevin wrote:
> On Thu, May 26, 2016 at 01:24:28PM +0100, Luis de Bethencourt wrote:
>> On 26/05/16 04:22, Kevin McKinney wrote:
>>> This patchset will address issues reported by checkpatch.pl tool
>>> for file prism2usb.c.
>>>
>>> Kevin McKinney (3):
>>>   Staging: wlan-ng: fix alignment not matching.
>>>   Staging: wlan-ng: fix unnecessary parentheses.
>>>   Staging: wlan-ng: fix comparison to NULL issue.
>>>
>>>  drivers/staging/wlan-ng/prism2usb.c | 14 +++---
>>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>>
>>
>> Hello Kevin,
>>
>> Thanks for fixing the issue.
>>
>> A note for the future, when you resend a patch series you should
>> mark it as a new version. So the system maintainer knows the evolution
>> of the patchset and to drop the earlier versions in his process.
>>
>> Look at this random example I just found:
>> https://lkml.org/lkml/2016/5/26/161
>>
>> If you are using git format-patch, this is done with:
>> git format-patch --subject-prefix="PATCH v2"
> 
> Okay, I will be sure to put the patch series in the subject. Thanks for the 
> feedback.
>  
>> Thanks,
>> Luis

My pleasure Kevin,

Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Staging: android: ion: fixed a kzalloc coding style issue.

[Luis de Bethencourt]

On 26/05/16 09:01, Shubham Bansal wrote:
> Fixed a coding style issue. Issue reported by checkpatch.pl.
> 
> Signed-off-by: Shubham Bansal 
> ---
>  drivers/staging/android/ion/ion.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/android/ion/ion.c 
> b/drivers/staging/android/ion/ion.c
> index 8536567..2217ccb 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -184,7 +184,7 @@ static struct ion_buffer *ion_buffer_create(struct 
> ion_heap *heap,
>   struct scatterlist *sg;
>   int i, ret;
>  
> - buffer = kzalloc(sizeof(struct ion_buffer), GFP_KERNEL);
> + buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
>   if (!buffer)
>   return ERR_PTR(-ENOMEM);
>  
> 


This patch looks good.

It is not a big issue, but have it in mind for the future. When fixing issues
reported by checkpatch, it is nice to paste the output warning of checkpatch
in the commit message.

Look at this example:
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=e7a142aaa09fa5db015fd176d6943f888665829f

Thanks,
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: dgnc: remove redundant NULL check for brd

[Luis de Bethencourt]

On 26/05/16 05:56, DaeSeok Youn wrote:
> 2016-05-26 6:48 GMT+09:00 Luis de Bethencourt <lui...@osg.samsung.com>:
>> On 20/05/16 10:51, Daeseok Youn wrote:
>>> the "brd" value cannot be NULL in dgnc_finalize_board_init().
>>> Because "brd" as a parameter of this function was already
>>> checked for NULL.
>>>
>>> Signed-off-by: Daeseok Youn <daeseok.y...@gmail.com>
>>> ---
>>>  drivers/staging/dgnc/dgnc_driver.c | 3 ---
>>>  1 file changed, 3 deletions(-)
>>>
>>> diff --git a/drivers/staging/dgnc/dgnc_driver.c 
>>> b/drivers/staging/dgnc/dgnc_driver.c
>>> index af2e835..22257d2 100644
>>> --- a/drivers/staging/dgnc/dgnc_driver.c
>>> +++ b/drivers/staging/dgnc/dgnc_driver.c
>>> @@ -579,9 +579,6 @@ static int dgnc_finalize_board_init(struct dgnc_board 
>>> *brd)
>>>  {
>>>   int rc = 0;
>>>
>>> - if (!brd || brd->magic != DGNC_BOARD_MAGIC)
>>> - return -ENODEV;
>>> -
>>>   if (brd->irq) {
>>>   rc = request_irq(brd->irq, brd->bd_ops->intr,
>>>IRQF_SHARED, "DGNC", brd);
>>>
>>
>> This is partially correct, the check for brd being NULL is in line 371.
> Hi Luis,
> 
> Yes, right. but also brd was assigned the value DGNC_BOARD_MAGIC in line 384.
> brd->magic = DGNC_BOARD_MAGIC;
> and also dgnc_finalize_board_init() as a static function is only
> called in dgnc_found_board(), right?
> 
>>
>> But there is a second check for brd->magic != DGNC_BOARD_MAGIC. Do you want
>> to keep that one?
> So.. I think it doesn't need to check about DGNC_BOARD_MAGIC.

This is good. I was asking just to make sure it was your intention.

Please add the reason to drop that second check in the commit message as well. 
So people
reading the git log can understand both parts. For both patches.

Thanks for the fixes :)

Luis

> 
>>
>> Also, how did you find this patch. It is useful to mention this in the commit
>> message if it was through some static analysis tool. For people using these 
>> tools
>> in the future.
> There are some static analysis tool for checking linux kernel code.
> But I didn't use
> those tools for this patch. sometimes, I usually run "smatch" tool for
> checking linux kernel
> code.
> 
> thanks.
> regards,
> Daeseok.
> 
>>
>> Thanks for the patch :)
>> Luis

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

Re: [PATCH 0/3] fix issues in prism2usb.c.

[Luis de Bethencourt]

On 26/05/16 04:22, Kevin McKinney wrote:
> This patchset will address issues reported by checkpatch.pl tool
> for file prism2usb.c.
> 
> Kevin McKinney (3):
>   Staging: wlan-ng: fix alignment not matching.
>   Staging: wlan-ng: fix unnecessary parentheses.
>   Staging: wlan-ng: fix comparison to NULL issue.
> 
>  drivers/staging/wlan-ng/prism2usb.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 

Hello Kevin,

Thanks for fixing the issue.

A note for the future, when you resend a patch series you should
mark it as a new version. So the system maintainer knows the evolution
of the patchset and to drop the earlier versions in his process.

Look at this random example I just found:
https://lkml.org/lkml/2016/5/26/161

If you are using git format-patch, this is done with:
git format-patch --subject-prefix="PATCH v2"

Thanks,
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/3] fix wlan-ng/prism2usb.c

[Luis de Bethencourt]

On 20/05/16 19:04, Kevin McKinney wrote:
> This patchset will fix the below issues reported checkpatch.pl tool.
> 
> Kevin McKinney (3):
>   Staging: wlan-ng: fix Alignment should match open parenthesis.
> This patch fixes alignment of parenthesis found by checkpatch.pl
> tool.
>   Staging: wlan-ng: fix unnecessary parentheses around interface-dev.   
>  This is a path to fix unnecessary parentheses around interface-dev
> found by checkpatch.pl tool.
>   Staging: wlan-ng: fix comparison to NULL. This patch fixes
> comparison to NULL found by checkpatch.pl tool.
> 
>  drivers/staging/wlan-ng/prism2usb.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 

Hi Kevin,

You have the entire commit message as the subject.

For example, if I apply the first patch it shows like this:
Staging: wlan-ng: fix Alignment should match open parenthesis. This patch 
fixes alignment of parenthesis found by checkpatch.pl tool.

Signed-off-by: Kevin McKinney 

This breaks the commit message formatting [0]
Move the line starting with "This patch" down to two lines below.

Not sure if they are hard rules but the subject line should be
not much longer than 50 characters. And then the body of the message
should be 72 characters wide, since git indents it.

Check a bunch of random recent commits in the git log and you will
get a feel of the format.

Thanks,
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging/vme: explicitly use `unsigned int` instead of `unsigned`

[Luis de Bethencourt]

On 20/05/16 17:27, Daniel Lockyer wrote:
> Kernel code style is to prefer the former so add `int` to the method
> parameters.
> 
> Signed-off-by: Daniel Lockyer 
> ---
>  drivers/staging/vme/devices/vme_pio2_gpio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/vme/devices/vme_pio2_gpio.c 
> b/drivers/staging/vme/devices/vme_pio2_gpio.c
> index 6d361201..765cb2d 100644
> --- a/drivers/staging/vme/devices/vme_pio2_gpio.c
> +++ b/drivers/staging/vme/devices/vme_pio2_gpio.c
> @@ -92,7 +92,7 @@ static void pio2_gpio_set(struct gpio_chip *chip,
>  }
> 
>  /* Directionality configured at board build - send appropriate response */
> -static int pio2_gpio_dir_in(struct gpio_chip *chip, unsigned offset)
> +static int pio2_gpio_dir_in(struct gpio_chip *chip, unsigned int offset)
>  {
>   int data;
>   struct pio2_card *card = gpiochip_get_data(chip);
> @@ -111,7 +111,7 @@ static int pio2_gpio_dir_in(struct gpio_chip *chip, 
> unsigned offset)
>  }
> 
>  /* Directionality configured at board build - send appropriate response */
> -static int pio2_gpio_dir_out(struct gpio_chip *chip, unsigned offset, int 
> value)
> +static int pio2_gpio_dir_out(struct gpio_chip *chip, unsigned int offset, 
> int value)
>  {
>   int data;
>   struct pio2_card *card = gpiochip_get_data(chip);
> 

Unfortunately this doesn't apply cleanly on the staging-testing branch [0]

AFAIK Greg pushes new staging patches onto this branch.

Please fix so it does apply cleanly, the patch idea looks good.

Thanks :)
Luis



[0] 
https://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/log/?h=staging-testing

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

Re: [PATCH 2/2] staging: dgnc: remove redundant null check in

[Luis de Bethencourt]

On 20/05/16 10:51, Daeseok Youn wrote:
> the "brd" was already checked for NULL before calling dgnc_do_remap().
> 
> Signed-off-by: Daeseok Youn 
> ---
>  drivers/staging/dgnc/dgnc_driver.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/staging/dgnc/dgnc_driver.c 
> b/drivers/staging/dgnc/dgnc_driver.c
> index 22257d2..1882ef5 100644
> --- a/drivers/staging/dgnc/dgnc_driver.c
> +++ b/drivers/staging/dgnc/dgnc_driver.c
> @@ -599,9 +599,6 @@ static int dgnc_finalize_board_init(struct dgnc_board 
> *brd)
>   */
>  static void dgnc_do_remap(struct dgnc_board *brd)
>  {
> - if (!brd || brd->magic != DGNC_BOARD_MAGIC)
> - return;
> -
>   brd->re_map_membase = ioremap(brd->membase, 0x1000);
>  }
>  
> 

Same comment as the 1/2 patch.

Do you want to keep the brd->magic != DGNC_BOARD_MAGIC check?

Thanks,
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: dgnc: remove redundant NULL check for brd

[Luis de Bethencourt]

On 20/05/16 10:51, Daeseok Youn wrote:
> the "brd" value cannot be NULL in dgnc_finalize_board_init().
> Because "brd" as a parameter of this function was already
> checked for NULL.
> 
> Signed-off-by: Daeseok Youn 
> ---
>  drivers/staging/dgnc/dgnc_driver.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/staging/dgnc/dgnc_driver.c 
> b/drivers/staging/dgnc/dgnc_driver.c
> index af2e835..22257d2 100644
> --- a/drivers/staging/dgnc/dgnc_driver.c
> +++ b/drivers/staging/dgnc/dgnc_driver.c
> @@ -579,9 +579,6 @@ static int dgnc_finalize_board_init(struct dgnc_board 
> *brd)
>  {
>   int rc = 0;
>  
> - if (!brd || brd->magic != DGNC_BOARD_MAGIC)
> - return -ENODEV;
> -
>   if (brd->irq) {
>   rc = request_irq(brd->irq, brd->bd_ops->intr,
>IRQF_SHARED, "DGNC", brd);
> 

This is partially correct, the check for brd being NULL is in line 371.

But there is a second check for brd->magic != DGNC_BOARD_MAGIC. Do you want
to keep that one?

Also, how did you find this patch. It is useful to mention this in the commit
message if it was through some static analysis tool. For people using these 
tools
in the future.

Thanks for the patch :)
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 1/1] staging: unisys: remove unused struct members

[Luis de Bethencourt]

On 06/05/16 02:05, David Kershner wrote:
> From: Erik Arfvidson 
> 
> The following struct members were never used:
>  putfile_active_buffer::pnext
>  putfile_request::file_request_number
>  putfile_request::data_sequence_number
> 
> Signed-off-by: Erik Arfvidson 
> Signed-off-by: David Kershner 
> ---
>  drivers/staging/unisys/visorbus/visorchipset.c | 6 --
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorbus/visorchipset.c 
> b/drivers/staging/unisys/visorbus/visorchipset.c
> index 7522559..f0bbf52 100644
> --- a/drivers/staging/unisys/visorbus/visorchipset.c
> +++ b/drivers/staging/unisys/visorbus/visorchipset.c
> @@ -151,8 +151,6 @@ struct putfile_active_buffer {
>   /* a payload from a controlvm message, containing a file data buffer */
>   struct parser_context *parser_ctx;
>   /* points within data area of parser_ctx to next byte of data */
> - u8 *pnext;
> - /* # bytes left from  to the end of this data buffer */
>   size_t bytes_remaining;
>  };
>  
> @@ -166,14 +164,10 @@ struct putfile_request {
>  
>   /* header from original TransmitFile request */
>   struct controlvm_message_header controlvm_header;
> - u64 file_request_number;/* from original TransmitFile request */
>  
>   /* link to next struct putfile_request */
>   struct list_head next_putfile_request;
>  
> - /* most-recent sequence number supplied via a controlvm message */
> - u64 data_sequence_number;
> -
>   /* head of putfile_buffer_entry list, which describes the data to be
>* supplied as putfile data;
>* - this list is added to when controlvm messages come in that supply
> 

Looks good.

Small note, when it is one patch and not a series. Instead of having a cover 
email
[0/1] you can write comments about the patch that won't end in the git log right
after the "---" line, after the Signed-off-bys and the diff.

Here are two examples:
https://lkml.org/lkml/2016/5/5/483
https://lkml.org/lkml/2016/5/5/335

Thanks :)
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: unisys: visorchannel: correct variable misspelling

[Luis de Bethencourt]

On 05/05/16 05:54, David Kershner wrote:
> From: David Binder 
> 
> Fixes the spelling of a global variable passed into a kernel macro.
> 
> Signed-off-by: David Binder 
> Signed-off-by: David Kershner 
> ---
>  drivers/staging/unisys/visorbus/visorchipset.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/unisys/visorbus/visorchipset.c 
> b/drivers/staging/unisys/visorbus/visorchipset.c
> index bb0ca31..83faf63 100644
> --- a/drivers/staging/unisys/visorbus/visorchipset.c
> +++ b/drivers/staging/unisys/visorbus/visorchipset.c
> @@ -2342,7 +2342,7 @@ module_param_named(major, visorchipset_major, int, 
> S_IRUGO);
>  MODULE_PARM_DESC(visorchipset_major,
>"major device number to use for the device node");
>  module_param_named(visorbusregwait, visorchipset_visorbusregwait, int, 
> S_IRUGO);
> -MODULE_PARM_DESC(visorchipset_visorbusreqwait,
> +MODULE_PARM_DESC(visorchipset_visorbusregwait,
>"1 to have the module wait for the visor bus to register");
>  
>  module_init(init_unisys);
> 

Good catch! Now it matches the integer declared in line 61.

Looks good,
Luis

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

Re: [PATCH] staging: unisys: remove unused struct members

[Luis de Bethencourt]

On 05/05/16 05:52, David Kershner wrote:
> From: Erik Arfvidson 
> 
> The following struct members were never used:
>  putfile_active_buffer::pnext
>  putfile_request::file_request_number
>  putfile_request::data_sequence_number
> 
> Signed-off-by: Erik Arfvidson 
> Signed-off-by: David Kershner 
> ---
>  drivers/staging/unisys/visorbus/visorchipset.c | 6 --
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorbus/visorchipset.c 
> b/drivers/staging/unisys/visorbus/visorchipset.c
> index 6d9bbae..bb0ca31 100644
> --- a/drivers/staging/unisys/visorbus/visorchipset.c
> +++ b/drivers/staging/unisys/visorbus/visorchipset.c
> @@ -150,8 +150,6 @@ static LIST_HEAD(putfile_request_list);
>  struct putfile_active_buffer {
>   /* a payload from a controlvm message, containing a file data buffer */
>   struct parser_context *parser_ctx;
> - /* points within data area of parser_ctx to next byte of data */
> - u8 *pnext;
>   /* # bytes left from  to the end of this data buffer */

Small thing but, now this comment mentions a pnext that isn't there.

Thanks,
Luis

>   size_t bytes_remaining;
>  };
> @@ -166,14 +164,10 @@ struct putfile_request {
>  
>   /* header from original TransmitFile request */
>   struct controlvm_message_header controlvm_header;
> - u64 file_request_number;/* from original TransmitFile request */
>  
>   /* link to next struct putfile_request */
>   struct list_head next_putfile_request;
>  
> - /* most-recent sequence number supplied via a controlvm message */
> - u64 data_sequence_number;
> -
>   /* head of putfile_buffer_entry list, which describes the data to be
>* supplied as putfile data;
>* - this list is added to when controlvm messages come in that supply
> 

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

Re: [PATCH v6] rtl8712: Fixed alignment to match open parenthesis

[Luis de Bethencourt]

On 26/04/16 15:47, Parth Sane wrote:
> Hi,
> Yes you are right on that regard. I did do that in my previous patches.
> It’s just something that I will have to remember now onwards. Thanks.
> Regards,
> Parth Sane

Sure. It isn't a big issue, just thought it was worth mentioning as a tip.

Please keep submitting fixes. Appreciated.

Luis

>> On 26-Apr-2016, at 8:14 PM, Luis de Bethencourt <lui...@osg.samsung.com> 
>> wrote:
>>
>> On 26/04/16 15:33, Parth Sane wrote:
>>> Hi,
>>> Thanks for the feedback. I did find this issue with the assistance 
>>> checkpatch.pl script.
>>> Regards,
>>> Parth Sane
>>
>> Normally people mention the checkpatch warning they are fixing.
>>
>> For example:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5f10ef7dce02d8e36665974aca0977cda58c1122
>>
>> Verbosity in the commit message is a good thing :)
>>
>> I have tested that your patch still applies cleanly to Greg's 
>> staging-testing branch.
>>
>> Thanks,
>> Luis
>>
>>>> On 26-Apr-2016, at 8:00 PM, Luis de Bethencourt <lui...@osg.samsung.com> 
>>>> wrote:
>>>>
>>>> On 25/04/16 16:43, Parth Sane wrote:
>>>>> Fixed alignment to match open parenthesis.
>>>>>
>>>>> Signed-off-by: Parth Sane <laerdevstud...@gmail.com>
>>>>>
>>>>> ---
>>>>> Changes in v6:
>>>>>   -Added line before Signed-off message
>>>>>
>>>>
>>>> This last version looks good to me. Did you find this issue with 
>>>> checkpatch.pl?
>>>>
>>>> Thanks,
>>>> Luis
>>>>
>>>
> 

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

Re: [PATCH v6] rtl8712: Fixed alignment to match open parenthesis

[Luis de Bethencourt]

On 26/04/16 15:33, Parth Sane wrote:
> Hi,
> Thanks for the feedback. I did find this issue with the assistance 
> checkpatch.pl script.
> Regards,
> Parth Sane

Normally people mention the checkpatch warning they are fixing.

For example:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5f10ef7dce02d8e36665974aca0977cda58c1122

Verbosity in the commit message is a good thing :)

I have tested that your patch still applies cleanly to Greg's staging-testing 
branch.

Thanks,
Luis

>> On 26-Apr-2016, at 8:00 PM, Luis de Bethencourt <lui...@osg.samsung.com> 
>> wrote:
>>
>> On 25/04/16 16:43, Parth Sane wrote:
>>> Fixed alignment to match open parenthesis.
>>>
>>> Signed-off-by: Parth Sane <laerdevstud...@gmail.com>
>>>
>>> ---
>>> Changes in v6:
>>> -Added line before Signed-off message
>>>
>>
>> This last version looks good to me. Did you find this issue with 
>> checkpatch.pl?
>>
>> Thanks,
>> Luis
>>
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6] rtl8712: Fixed alignment to match open parenthesis

[Luis de Bethencourt]

On 25/04/16 16:43, Parth Sane wrote:
> Fixed alignment to match open parenthesis.
> 
> Signed-off-by: Parth Sane 
> 
> ---
> Changes in v6:
>   -Added line before Signed-off message
> 

This last version looks good to me. Did you find this issue with checkpatch.pl?

Thanks,
Luis

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

Re: [PATCH] staging: speakup: (coding style) Add spaces around operands (checkpatch checks)

[Luis de Bethencourt]

Hi al,

Looks good and silences the checkpatch warnings.

Christian,
There are more checkpatch warnings in this file. Could you fix them as well?


Reviewed-by: Luis de Bethencourt <lui...@osg.samsung.com>


On Thu, Nov 19, 2015 at 02:40:59PM +0100, Christian Colic wrote:
> Fix checkpatch check: CHECK: spaces preferred around that '-' (ctx:VxV)
> Add spaces around operands to fix these warnings.
> 
> Signed-off-by: Christian Colic <colic.christ...@gmail.com>
> ---
>  drivers/staging/speakup/keyhelp.c | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/speakup/keyhelp.c 
> b/drivers/staging/speakup/keyhelp.c
> index 02d5c70..d14089a 100644
> --- a/drivers/staging/speakup/keyhelp.c
> +++ b/drivers/staging/speakup/keyhelp.c
> @@ -74,7 +74,7 @@ static void build_key_data(void)
>   for (i = 0; i < nstates; i++, kp++) {
>   if (!*kp)
>   continue;
> - if ((state_tbl[i]&16) != 0 && *kp == SPK_KEY)
> + if ((state_tbl[i] & 16) != 0 && *kp == SPK_KEY)
>   continue;
>   counters[*kp]++;
>   }
> @@ -83,7 +83,7 @@ static void build_key_data(void)
>   if (counters[i] == 0)
>   continue;
>   key_offsets[i] = offset;
> - offset += (counters[i]+1);
> + offset += (counters[i] + 1);
>   if (offset >= MAXKEYS)
>   break;
>   }
> @@ -97,7 +97,7 @@ static void build_key_data(void)
>   ch1 = *kp++;
>   if (!ch1)
>   continue;
> - if ((state_tbl[i]&16) != 0 && ch1 == SPK_KEY)
> + if ((state_tbl[i] & 16) != 0 && ch1 == SPK_KEY)
>   continue;
>   key = (state_tbl[i] << 8) + ch;
>   counters[ch1]--;
> @@ -130,14 +130,14 @@ static int help_init(void)
>   int i;
>   int num_funcs = MSG_FUNCNAMES_END - MSG_FUNCNAMES_START + 1;
>  
> - state_tbl = spk_our_keys[0]+SHIFT_TBL_SIZE+2;
> + state_tbl = spk_our_keys[0] + SHIFT_TBL_SIZE + 2;
>   for (i = 0; i < num_funcs; i++) {
>   char *cur_funcname = spk_msg_get(MSG_FUNCNAMES_START + i);
>  
>   if (start == *cur_funcname)
>   continue;
>   start = *cur_funcname;
> - letter_offsets[(start&31)-1] = i;
> + letter_offsets[(start & 31) - 1] = i;
>   }
>   return 0;
>  }
> @@ -160,12 +160,12 @@ int spk_handle_help(struct vc_data *vc, u_char type, 
> u_char ch, u_short key)
>   ch |= 32; /* lower case */
>   if (ch < 'a' || ch > 'z')
>   return -1;
> - if (letter_offsets[ch-'a'] == -1) {
> + if (letter_offsets[ch - 'a'] == -1) {
>   synth_printf(spk_msg_get(MSG_NO_COMMAND), ch);
>   synth_printf("\n");
>   return 1;
>   }
> - cur_item = letter_offsets[ch-'a'];
> + cur_item = letter_offsets[ch - 'a'];
>   } else if (type == KT_CUR) {
>   if (ch == 0
>   && (MSG_FUNCNAMES_START + cur_item + 1) <=
> @@ -186,7 +186,7 @@ int spk_handle_help(struct vc_data *vc, u_char type, 
> u_char ch, u_short key)
>   name = NULL;
>   if ((type != KT_SPKUP) && (key > 0) && (key <= num_key_names)) {
>   synth_printf("%s\n",
> - spk_msg_get(MSG_KEYNAMES_START + key-1));
> + spk_msg_get(MSG_KEYNAMES_START + key - 1));
>   return 1;
>   }
>   for (i = 0; funcvals[i] != 0 && !name; i++) {
> @@ -195,7 +195,7 @@ int spk_handle_help(struct vc_data *vc, u_char type, 
> u_char ch, u_short key)
>   }
>   if (!name)
>   return -1;
> - kp = spk_our_keys[key]+1;
> + kp = spk_our_keys[key] + 1;
>   for (i = 0; i < nstates; i++) {
>   if (ch == kp[i])
>   break;
> -- 
> 2.5.0
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ft1000: Remove boolean comparisons

[Luis de Bethencourt]

On 25/10/15 02:20, Greg KH wrote:
> On Sun, Oct 18, 2015 at 09:58:15PM +0100, Luis de Bethencourt wrote:
>> Boolean tests do not need explicit comparison to true or false.
>>
>> Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
>> ---
>>  drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 4 ++--
>>  drivers/staging/ft1000/ft1000-usb/ft1000_hw.c| 2 +-
>>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> This driver just got deleted from the tree, sorry :(
> 

Hi Greg,

No problem. I see the commit that removed this driver [0] mentions the radio
technology service has been discontinued.

Great to see drivers removed when they become obsolete instead of zombily
maintained.

Thanks for the review,
Luis


[0] 
https://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/commit/?h=staging-testing=6512edec48b2ccfe9bb969ce26ebbbcd49de6c4b
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[RESEND PATCH] staging: wilc1000: return -ENOMEM when kmalloc failed

[Luis de Bethencourt]

The driver is using -1 instead of the -ENOMEM defined macro to specify that
a buffer allocation failed.

Fixes smatch warning and similars:
drivers/staging/wilc1000/host_interface.c:1757 Handle_Key() warn:
returning -1 instead of -ENOMEM is sloppy

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---

Hi,

Resending because previous version didn't apply against staging-next due to the
recent changes on this file. Refreshed.

Thanks,
Luis

 drivers/staging/wilc1000/host_interface.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 5f81eab..064e34c 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -1754,7 +1754,7 @@ static int Handle_Key(struct host_if_drv *hif_drv,
 
if (pu8keybuf == NULL) {
PRINT_ER("No buffer to send Key\n");
-   return -1;
+   return -ENOMEM;
}
 
kfree(pstrHostIFkeyAttr->attr.wep.key);
@@ -1776,7 +1776,7 @@ static int Handle_Key(struct host_if_drv *hif_drv,
pu8keybuf = kmalloc(pstrHostIFkeyAttr->attr.wep.key_len 
+ 2, GFP_KERNEL);
if (!pu8keybuf) {
PRINT_ER("No buffer to send Key\n");
-   return -1;
+   return -ENOMEM;
}
pu8keybuf[0] = pstrHostIFkeyAttr->attr.wep.index;
memcpy(pu8keybuf + 1, 
>attr.wep.key_len, 1);
@@ -1823,7 +1823,7 @@ static int Handle_Key(struct host_if_drv *hif_drv,
pu8keybuf = kzalloc(RX_MIC_KEY_MSG_LEN, GFP_KERNEL);
if (!pu8keybuf) {
PRINT_ER("No buffer to send RxGTK Key\n");
-   ret = -1;
+   ret = -ENOMEM;
goto _WPARxGtk_end_case_;
}
 
@@ -1858,7 +1858,7 @@ static int Handle_Key(struct host_if_drv *hif_drv,
pu8keybuf = kzalloc(RX_MIC_KEY_MSG_LEN, GFP_KERNEL);
if (pu8keybuf == NULL) {
PRINT_ER("No buffer to send RxGTK Key\n");
-   ret = -1;
+   ret = -ENOMEM;
goto _WPARxGtk_end_case_;
}
 
@@ -1887,7 +1887,7 @@ static int Handle_Key(struct host_if_drv *hif_drv,
 _WPARxGtk_end_case_:
kfree(pstrHostIFkeyAttr->attr.wpa.key);
kfree(pstrHostIFkeyAttr->attr.wpa.seq);
-   if (ret == -1)
+   if (ret)
return ret;
 
break;
@@ -1899,7 +1899,7 @@ _WPARxGtk_end_case_:
pu8keybuf = kmalloc(PTK_KEY_MSG_LEN + 1, GFP_KERNEL);
if (!pu8keybuf) {
PRINT_ER("No buffer to send PTK Key\n");
-   ret = -1;
+   ret = -ENOMEM;
goto _WPAPtk_end_case_;
 
}
@@ -1931,7 +1931,7 @@ _WPARxGtk_end_case_:
pu8keybuf = kmalloc(PTK_KEY_MSG_LEN, GFP_KERNEL);
if (!pu8keybuf) {
PRINT_ER("No buffer to send PTK Key\n");
-   ret = -1;
+   ret = -ENOMEM;
goto _WPAPtk_end_case_;
 
}
@@ -1954,7 +1954,7 @@ _WPARxGtk_end_case_:
 
 _WPAPtk_end_case_:
kfree(pstrHostIFkeyAttr->attr.wpa.key);
-   if (ret == -1)
+   if (ret)
return ret;
 
break;
@@ -1967,7 +1967,7 @@ _WPAPtk_end_case_:
pu8keybuf = kmalloc((pstrHostIFkeyAttr->attr.pmkid.numpmkid * 
PMKSA_KEY_LEN) + 1, GFP_KERNEL);
if (!pu8keybuf) {
PRINT_ER("No buffer to send PMKSA Key\n");
-   return -1;
+   return -ENOMEM;
}
 
pu8keybuf[0] = pstrHostIFkeyAttr->attr.pmkid.numpmkid;
-- 
2.5.1

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

[PATCH] staging: rtl8188eu: fix misleading indentation

[Luis de Bethencourt]

Code is correct, i needs to be moved back by 2 to correct for the last
iteration of the while loop, since READ_NEXT_PAIR advances two. Fixing
the misleading indentation.

Fix a smatch warning:
drivers/staging/rtl8188eu/hal/rf_cfg.c:217
rtl88e_phy_config_rf_with_headerfile() warn: curly braces intended?

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/rtl8188eu/hal/rf_cfg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/hal/rf_cfg.c 
b/drivers/staging/rtl8188eu/hal/rf_cfg.c
index 067649a..a3f1aba 100644
--- a/drivers/staging/rtl8188eu/hal/rf_cfg.c
+++ b/drivers/staging/rtl8188eu/hal/rf_cfg.c
@@ -214,7 +214,7 @@ static bool rtl88e_phy_config_rf_with_headerfile(struct 
adapter *adapt)
while (v2 != 0xDEAD && v2 != 0xCDEF &&
   v2 != 0xCDCD && i < array_len - 2)
READ_NEXT_PAIR(v1, v2, i);
-   i -= 2;
+   i -= 2;
} else {
READ_NEXT_PAIR(v1, v2, i);
while (v2 != 0xDEAD && v2 != 0xCDEF &&
-- 
2.5.1

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

[PATCH] staging: lustre: o2iblnd: fix misleading indentation

[Luis de Bethencourt]

The code is correct, the indentation is misleading. Only the the return rc
is part of the conditional statement if rc != 0.

Fix a smatch warning:
drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c:2157
kiblnd_hdev_setup_mrs() warn: curly braces intended?

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---

Hi,

This patch gives a warning in checkpatch.pl for the line:
if (hdev->ibh_mrs == NULL)

It would be cleaner if this was 'if (!hdev->ibh_mrs)' instead. I didn't fix
this as well because the file has 41 instances of comparisons with NULL. I
would be happy to do so in a patch in reply to this one if it would be good.

Thanks,
Luis

 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c| 28 +++---
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c 
b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
index c2cc4e4..7c730e3 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
@@ -2154,23 +2154,23 @@ static int kiblnd_hdev_setup_mrs(kib_hca_dev_t *hdev)
if (rc != 0)
return rc;
 
-   LIBCFS_ALLOC(hdev->ibh_mrs, 1 * sizeof(*hdev->ibh_mrs));
-   if (hdev->ibh_mrs == NULL) {
-   CERROR("Failed to allocate MRs table\n");
-   return -ENOMEM;
-   }
+   LIBCFS_ALLOC(hdev->ibh_mrs, 1 * sizeof(*hdev->ibh_mrs));
+   if (hdev->ibh_mrs == NULL) {
+   CERROR("Failed to allocate MRs table\n");
+   return -ENOMEM;
+   }
 
-   hdev->ibh_mrs[0] = NULL;
-   hdev->ibh_nmrs   = 1;
+   hdev->ibh_mrs[0] = NULL;
+   hdev->ibh_nmrs   = 1;
 
-   mr = ib_get_dma_mr(hdev->ibh_pd, acflags);
-   if (IS_ERR(mr)) {
-   CERROR("Failed ib_get_dma_mr : %ld\n", PTR_ERR(mr));
-   kiblnd_hdev_cleanup_mrs(hdev);
-   return PTR_ERR(mr);
-   }
+   mr = ib_get_dma_mr(hdev->ibh_pd, acflags);
+   if (IS_ERR(mr)) {
+   CERROR("Failed ib_get_dma_mr : %ld\n", PTR_ERR(mr));
+   kiblnd_hdev_cleanup_mrs(hdev);
+   return PTR_ERR(mr);
+   }
 
-   hdev->ibh_mrs[0] = mr;
+   hdev->ibh_mrs[0] = mr;
 
return 0;
 }
-- 
2.5.1

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

[PATCH] staging: wilc1000: return -ENOMEM when kmalloc failed

[Luis de Bethencourt]

The driver is using -1 instead of the -ENOMEM defined macro to specify that
a buffer allocation failed.

Fixes smatch warning and similars:
drivers/staging/wilc1000/host_interface.c:1782 Handle_Key() warn:
returning -1 instead of -ENOMEM is sloppy

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/wilc1000/host_interface.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 38fead4..ec47a28 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -1756,7 +1756,7 @@ static int Handle_Key(struct host_if_drv *hif_drv,
 
if (pu8keybuf == NULL) {
PRINT_ER("No buffer to send Key\n");
-   return -1;
+   return -ENOMEM;
}
 
kfree(pstrHostIFkeyAttr->attr.wep.key);
@@ -1779,7 +1779,7 @@ static int Handle_Key(struct host_if_drv *hif_drv,
pu8keybuf = kmalloc(pstrHostIFkeyAttr->attr.wep.key_len 
+ 2, GFP_KERNEL);
if (pu8keybuf == NULL) {
PRINT_ER("No buffer to send Key\n");
-   return -1;
+   return -ENOMEM;
}
pu8keybuf[0] = pstrHostIFkeyAttr->attr.wep.index;
memcpy(pu8keybuf + 1, 
>attr.wep.key_len, 1);
@@ -1826,7 +1826,7 @@ static int Handle_Key(struct host_if_drv *hif_drv,
pu8keybuf = kzalloc(RX_MIC_KEY_MSG_LEN, GFP_KERNEL);
if (pu8keybuf == NULL) {
PRINT_ER("No buffer to send RxGTK Key\n");
-   ret = -1;
+   ret = -ENOMEM;
goto _WPARxGtk_end_case_;
}
 
@@ -1861,7 +1861,7 @@ static int Handle_Key(struct host_if_drv *hif_drv,
pu8keybuf = kzalloc(RX_MIC_KEY_MSG_LEN, GFP_KERNEL);
if (pu8keybuf == NULL) {
PRINT_ER("No buffer to send RxGTK Key\n");
-   ret = -1;
+   ret = -ENOMEM;
goto _WPARxGtk_end_case_;
}
 
@@ -1890,7 +1890,7 @@ static int Handle_Key(struct host_if_drv *hif_drv,
 _WPARxGtk_end_case_:
kfree(pstrHostIFkeyAttr->attr.wpa.key);
kfree(pstrHostIFkeyAttr->attr.wpa.seq);
-   if (ret == -1)
+   if (ret)
return ret;
 
break;
@@ -1905,7 +1905,7 @@ _WPARxGtk_end_case_:
 
if (pu8keybuf == NULL) {
PRINT_ER("No buffer to send PTK Key\n");
-   ret = -1;
+   ret = -ENOMEM;
goto _WPAPtk_end_case_;
 
}
@@ -1940,7 +1940,7 @@ _WPARxGtk_end_case_:
 
if (pu8keybuf == NULL) {
PRINT_ER("No buffer to send PTK Key\n");
-   ret = -1;
+   ret = -ENOMEM;
goto _WPAPtk_end_case_;
 
}
@@ -1963,7 +1963,7 @@ _WPARxGtk_end_case_:
 
 _WPAPtk_end_case_:
kfree(pstrHostIFkeyAttr->attr.wpa.key);
-   if (ret == -1)
+   if (ret)
return ret;
 
break;
@@ -1976,7 +1976,7 @@ _WPAPtk_end_case_:
pu8keybuf = kmalloc((pstrHostIFkeyAttr->attr.pmkid.numpmkid * 
PMKSA_KEY_LEN) + 1, GFP_KERNEL);
if (pu8keybuf == NULL) {
PRINT_ER("No buffer to send PMKSA Key\n");
-   return -1;
+   return -ENOMEM;
}
 
pu8keybuf[0] = pstrHostIFkeyAttr->attr.pmkid.numpmkid;
-- 
2.5.1

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

[PATCH] staging: rtl8723au: core: rtw_wlan_util: fix misleading indentation

[Luis de Bethencourt]

For loop is outside of the else branch of the above conditional statement.
Fixing misleading indentation.

Fix a smatch warning:
drivers/staging/rtl8723au/core/rtw_wlan_util.c:528
WMMOnAssocRsp23a() warn: curly braces intended?

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/rtl8723au/core/rtw_wlan_util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_wlan_util.c 
b/drivers/staging/rtl8723au/core/rtw_wlan_util.c
index 5e87360..cc2b84b 100644
--- a/drivers/staging/rtl8723au/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8723au/core/rtw_wlan_util.c
@@ -525,7 +525,7 @@ void WMMOnAssocRsp23a(struct rtw_adapter *padapter)
else
aSifsTime = 16;
 
-   for (i = 0; i < 4; i++) {
+   for (i = 0; i < 4; i++) {
ACI = (pmlmeinfo->WMM_param.ac_param[i].ACI_AIFSN >> 5) & 0x03;
ACM = (pmlmeinfo->WMM_param.ac_param[i].ACI_AIFSN >> 4) & 0x01;
 
-- 
2.5.1

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

[PATCH v2 0/3] staging: code cleanup

[Luis de Bethencourt]

Hi,

I initially submitted a patch to remove all the explicit boolean comparisons
in rtl8712. These code changes included two instances where braces needed to be
added to clean the output of checkpatch.pl on this patch.

Mike Rapaport correctly pointed out those changes had to go in a separate
patch. Not wanting to only fix those two instances, but cover the whole
rtl8712, I've added a second patch fixing these.

The checkpatch.pl of the second patch included some checks about needing spaces
around operands. Including this fix as well for a nicer checkpatch.pl output.
Plus also making it cover all rtl8712.

In other words, these are a series of recursive cleanup changes.

Thanks for the review Mike,
Luis


Luis de Bethencourt (3):
  staging: rtl8712: Remove boolean comparisons
  staging: rtl8712: braces should be used on all arms
  staging: rtl8712: spaces preferred around operands

 drivers/staging/rtl8712/hal_init.c|  11 +-
 drivers/staging/rtl8712/ieee80211.c   |  36 ++---
 drivers/staging/rtl8712/mlme_linux.c  |   2 +-
 drivers/staging/rtl8712/os_intfs.c|   6 +-
 drivers/staging/rtl8712/rtl8712_cmd.c |  27 ++--
 drivers/staging/rtl8712/rtl8712_efuse.c   | 138 ++-
 drivers/staging/rtl8712/rtl8712_io.c  |   6 +-
 drivers/staging/rtl8712/rtl8712_led.c | 187 +-
 drivers/staging/rtl8712/rtl8712_recv.c|  53 
 drivers/staging/rtl8712/rtl8712_xmit.c|  74 +-
 drivers/staging/rtl8712/rtl871x_cmd.c |  25 ++--
 drivers/staging/rtl8712/rtl871x_eeprom.c  |  22 +--
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 153 +++--
 drivers/staging/rtl8712/rtl871x_ioctl_rtl.c   |  53 +---
 drivers/staging/rtl8712/rtl871x_ioctl_set.c   |  33 ++---
 drivers/staging/rtl8712/rtl871x_mlme.c| 144 ++--
 drivers/staging/rtl8712/rtl871x_mlme.h|   2 +-
 drivers/staging/rtl8712/rtl871x_mp.c  |  23 ++--
 drivers/staging/rtl8712/rtl871x_mp_ioctl.c|  38 +++---
 drivers/staging/rtl8712/rtl871x_pwrctrl.c |   6 +-
 drivers/staging/rtl8712/rtl871x_recv.c|  70 +-
 drivers/staging/rtl8712/rtl871x_security.c| 133 +-
 drivers/staging/rtl8712/rtl871x_sta_mgt.c |  10 +-
 drivers/staging/rtl8712/rtl871x_xmit.c|  95 ++---
 drivers/staging/rtl8712/usb_halinit.c |  13 +-
 drivers/staging/rtl8712/usb_intf.c|  17 +--
 drivers/staging/rtl8712/usb_ops.c |  20 +--
 drivers/staging/rtl8712/usb_ops_linux.c   |  20 +--
 drivers/staging/rtl8712/xmit_linux.c  |   4 +-
 29 files changed, 753 insertions(+), 668 deletions(-)

-- 
2.5.3

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

[PATCH v2 1/3] staging: rtl8712: Remove boolean comparisons

[Luis de Bethencourt]

Boolean tests do not need explicit comparison to true or false.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/rtl8712/os_intfs.c|   4 +-
 drivers/staging/rtl8712/rtl8712_cmd.c |   9 +-
 drivers/staging/rtl8712/rtl8712_efuse.c   |  56 -
 drivers/staging/rtl8712/rtl8712_io.c  |   6 +-
 drivers/staging/rtl8712/rtl8712_led.c | 164 +-
 drivers/staging/rtl8712/rtl8712_recv.c|  25 ++--
 drivers/staging/rtl8712/rtl8712_xmit.c|   4 +-
 drivers/staging/rtl8712/rtl871x_cmd.c |   8 +-
 drivers/staging/rtl8712/rtl871x_eeprom.c  |  22 ++--
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c |  56 -
 drivers/staging/rtl8712/rtl871x_ioctl_rtl.c   |  12 +-
 drivers/staging/rtl8712/rtl871x_ioctl_set.c   |  26 ++--
 drivers/staging/rtl8712/rtl871x_mlme.c|  77 ++--
 drivers/staging/rtl8712/rtl871x_mlme.h|   2 +-
 drivers/staging/rtl8712/rtl871x_mp_ioctl.c|  16 +--
 drivers/staging/rtl8712/rtl871x_pwrctrl.c |   6 +-
 drivers/staging/rtl8712/rtl871x_recv.c|  39 +++---
 drivers/staging/rtl8712/rtl871x_security.c|   4 +-
 drivers/staging/rtl8712/rtl871x_sta_mgt.c |   6 +-
 drivers/staging/rtl8712/rtl871x_xmit.c|  38 +++---
 drivers/staging/rtl8712/usb_intf.c|   4 +-
 drivers/staging/rtl8712/usb_ops_linux.c   |  10 +-
 drivers/staging/rtl8712/xmit_linux.c  |   4 +-
 23 files changed, 291 insertions(+), 307 deletions(-)

diff --git a/drivers/staging/rtl8712/os_intfs.c 
b/drivers/staging/rtl8712/os_intfs.c
index 6805f79..92ab59d 100644
--- a/drivers/staging/rtl8712/os_intfs.c
+++ b/drivers/staging/rtl8712/os_intfs.c
@@ -177,7 +177,7 @@ static int r871x_net_set_mac_address(struct net_device 
*pnetdev, void *p)
struct _adapter *padapter = netdev_priv(pnetdev);
struct sockaddr *addr = p;
 
-   if (padapter->bup == false)
+   if (!padapter->bup)
ether_addr_copy(pnetdev->dev_addr, addr->sa_data);
return 0;
 }
@@ -384,7 +384,7 @@ static int netdev_open(struct net_device *pnetdev)
struct _adapter *padapter = netdev_priv(pnetdev);
 
mutex_lock(>mutex_start);
-   if (padapter->bup == false) {
+   if (!padapter->bup) {
padapter->bDriverStopped = false;
padapter->bSurpriseRemoved = false;
padapter->bup = true;
diff --git a/drivers/staging/rtl8712/rtl8712_cmd.c 
b/drivers/staging/rtl8712/rtl8712_cmd.c
index f70235c..39b6320 100644
--- a/drivers/staging/rtl8712/rtl8712_cmd.c
+++ b/drivers/staging/rtl8712/rtl8712_cmd.c
@@ -85,7 +85,7 @@ static void query_fw_rx_phy_status(struct _adapter *padapter)
u32 val32 = 0;
int pollingcnts = 50;
 
-   if (check_fwstate(>mlmepriv, _FW_LINKED) == true) {
+   if (check_fwstate(>mlmepriv, _FW_LINKED)) {
r8712_write32(padapter, IOCMD_CTRL_REG, 0xf401);
msleep(100);
/* Wait FW complete IO Cmd */
@@ -324,8 +324,7 @@ int r8712_cmd_thread(void *context)
while (1) {
if ((_down_sema(&(pcmdpriv->cmd_queue_sema))) == _FAIL)
break;
-   if ((padapter->bDriverStopped == true) ||
-   (padapter->bSurpriseRemoved == true))
+   if (padapter->bDriverStopped || padapter->bSurpriseRemoved)
break;
if (r8712_register_cmd_alive(padapter) != _SUCCESS)
continue;
@@ -375,8 +374,8 @@ _next:
pcmdbuf += 2; /* 8 bytes alignment */
memcpy((u8 *)pcmdbuf, pcmd->parmbuf, pcmd->cmdsz);
while (check_cmd_fifo(padapter, wr_sz) == _FAIL) {
-   if ((padapter->bDriverStopped == true) ||
-   (padapter->bSurpriseRemoved == true))
+   if (padapter->bDriverStopped ||
+   padapter->bSurpriseRemoved)
break;
msleep(100);
continue;
diff --git a/drivers/staging/rtl8712/rtl8712_efuse.c 
b/drivers/staging/rtl8712/rtl8712_efuse.c
index d957169..13d4b8d 100644
--- a/drivers/staging/rtl8712/rtl8712_efuse.c
+++ b/drivers/staging/rtl8712/rtl8712_efuse.c
@@ -39,7 +39,7 @@ static void efuse_reg_ctrl(struct _adapter *padapter, u8 
bPowerOn)
 {
u8 tmpu8 = 0;
 
-   if (true == bPowerOn) {
+   if (bPowerOn) {
/* -e-fuse pwr & clk reg ctrl ---
 * Enable LDOE25 Macro Block
 */
@@ -127,7 +127,7 @@ static u8 efuse_one_byte_rw(struct _adapter *padapter, u8 
bRead, u16 addr,
tmpv8 = ((u8)((addr >> 8) & 0x03)) |
 (r871

[PATCH v2 2/3] staging: rtl8712: braces should be used on all arms

[Luis de Bethencourt]

Fix all instances of the following checkpatch.pl check:
CHECK: braces {} should be used on all arms of this statement

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/rtl8712/hal_init.c| 11 +++---
 drivers/staging/rtl8712/ieee80211.c   | 15 +---
 drivers/staging/rtl8712/rtl8712_cmd.c |  6 ++--
 drivers/staging/rtl8712/rtl8712_efuse.c   | 20 ++-
 drivers/staging/rtl8712/rtl8712_led.c | 15 +---
 drivers/staging/rtl8712/rtl8712_recv.c| 16 +
 drivers/staging/rtl8712/rtl8712_xmit.c| 10 +++---
 drivers/staging/rtl8712/rtl871x_cmd.c |  7 ++--
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 50 ---
 drivers/staging/rtl8712/rtl871x_ioctl_rtl.c   | 40 +
 drivers/staging/rtl8712/rtl871x_ioctl_set.c   |  3 +-
 drivers/staging/rtl8712/rtl871x_mlme.c| 29 ++--
 drivers/staging/rtl8712/rtl871x_mp.c  |  6 ++--
 drivers/staging/rtl8712/rtl871x_mp_ioctl.c| 18 ++
 drivers/staging/rtl8712/rtl871x_recv.c| 27 +--
 drivers/staging/rtl8712/rtl871x_security.c| 21 +++
 drivers/staging/rtl8712/rtl871x_sta_mgt.c |  4 +--
 drivers/staging/rtl8712/rtl871x_xmit.c| 37 +++-
 drivers/staging/rtl8712/usb_halinit.c |  3 +-
 drivers/staging/rtl8712/usb_intf.c|  9 ++---
 drivers/staging/rtl8712/usb_ops_linux.c   |  6 ++--
 21 files changed, 216 insertions(+), 137 deletions(-)

diff --git a/drivers/staging/rtl8712/hal_init.c 
b/drivers/staging/rtl8712/hal_init.c
index e0ae57d..8008efe 100644
--- a/drivers/staging/rtl8712/hal_init.c
+++ b/drivers/staging/rtl8712/hal_init.c
@@ -191,9 +191,9 @@ static u8 rtl8712_dl_fw(struct _adapter *padapter)
imem_sz = fwhdr.img_IMEM_size;
do {
memset(ptx_desc, 0, TXDESC_SIZE);
-   if (imem_sz >  MAX_DUMP_FWSZ/*49152*/)
+   if (imem_sz >  MAX_DUMP_FWSZ/*49152*/) {
dump_imem_sz = MAX_DUMP_FWSZ;
-   else {
+   } else {
dump_imem_sz = imem_sz;
ptx_desc->txdw0 |= cpu_to_le32(BIT(28));
}
@@ -309,8 +309,9 @@ static u8 rtl8712_dl_fw(struct _adapter *padapter)
}
if (i == 0)
goto exit_fail;
-   } else
+   } else {
goto exit_fail;
+   }
ret = _SUCCESS;
 
 exit_fail:
@@ -386,9 +387,9 @@ uint rtl871x_hal_init(struct _adapter *padapter)
return _FAIL;
if (padapter->halpriv.hal_bus_init(padapter) != _SUCCESS)
return _FAIL;
-   if (rtl8712_hal_init(padapter) == _SUCCESS)
+   if (rtl8712_hal_init(padapter) == _SUCCESS) {
padapter->hw_init_completed = true;
-   else {
+   } else {
padapter->hw_init_completed = false;
return _FAIL;
}
diff --git a/drivers/staging/rtl8712/ieee80211.c 
b/drivers/staging/rtl8712/ieee80211.c
index 261a973..3c38fcb 100644
--- a/drivers/staging/rtl8712/ieee80211.c
+++ b/drivers/staging/rtl8712/ieee80211.c
@@ -304,8 +304,9 @@ int r8712_parse_wpa_ie(u8 *wpa_ie, int wpa_ie_len, int 
*group_cipher,
*group_cipher = r8712_get_wpa_cipher_suite(pos);
pos += WPA_SELECTOR_LEN;
left -= WPA_SELECTOR_LEN;
-   } else if (left > 0)
+   } else if (left > 0) {
return _FAIL;
+   }
/*pairwise_cipher*/
if (left >= 2) {
count = le16_to_cpu(*(u16 *)pos);
@@ -318,8 +319,9 @@ int r8712_parse_wpa_ie(u8 *wpa_ie, int wpa_ie_len, int 
*group_cipher,
pos += WPA_SELECTOR_LEN;
left -= WPA_SELECTOR_LEN;
}
-   } else if (left == 1)
+   } else if (left == 1) {
return _FAIL;
+   }
return _SUCCESS;
 }
 
@@ -344,8 +346,9 @@ int r8712_parse_wpa2_ie(u8 *rsn_ie, int rsn_ie_len, int 
*group_cipher,
*group_cipher = r8712_get_wpa2_cipher_suite(pos);
pos += RSN_SELECTOR_LEN;
left -= RSN_SELECTOR_LEN;
-   } else if (left > 0)
+   } else if (left > 0) {
return _FAIL;
+   }
/*pairwise_cipher*/
if (left >= 2) {
count = le16_to_cpu(*(u16 *)pos);
@@ -358,8 +361,9 @@ int r8712_parse_wpa2_ie(u8 *rsn_ie, int rsn_ie_len, int 
*group_cipher,
pos += RSN_SELECTOR_LEN;
left -= RSN_SELECTOR_LEN;
}
-   } else if (left == 1)
+   } else if (left == 1) {
return _FAIL;
+   }
return _SUCCESS;
 }
 
@@ -385,8 +389,9 @@ int r8712_get_sec_ie(u8 *in_

[PATCH v2 3/3] staging: rtl8712: spaces preferred around operands

[Luis de Bethencourt]

Clean up all instances of checkpatch.pl checks:
CHECK: spaces preferred around that '+'
(and other operands)

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/rtl8712/ieee80211.c   |  21 ++---
 drivers/staging/rtl8712/mlme_linux.c  |   2 +-
 drivers/staging/rtl8712/os_intfs.c|   2 +-
 drivers/staging/rtl8712/rtl8712_cmd.c |  12 +--
 drivers/staging/rtl8712/rtl8712_efuse.c   |  74 +-
 drivers/staging/rtl8712/rtl8712_led.c |   8 +-
 drivers/staging/rtl8712/rtl8712_recv.c|  12 +--
 drivers/staging/rtl8712/rtl8712_xmit.c|  60 +++---
 drivers/staging/rtl8712/rtl871x_cmd.c |  10 +--
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c |  47 +--
 drivers/staging/rtl8712/rtl871x_ioctl_rtl.c   |   3 +-
 drivers/staging/rtl8712/rtl871x_ioctl_set.c   |   4 +-
 drivers/staging/rtl8712/rtl871x_mlme.c|  40 +-
 drivers/staging/rtl8712/rtl871x_mp.c  |  17 ++--
 drivers/staging/rtl8712/rtl871x_mp_ioctl.c|   4 +-
 drivers/staging/rtl8712/rtl871x_recv.c|   8 +-
 drivers/staging/rtl8712/rtl871x_security.c| 108 ++
 drivers/staging/rtl8712/rtl871x_xmit.c|  20 ++---
 drivers/staging/rtl8712/usb_halinit.c |  10 +--
 drivers/staging/rtl8712/usb_intf.c|   4 +-
 drivers/staging/rtl8712/usb_ops.c |  20 ++---
 drivers/staging/rtl8712/usb_ops_linux.c   |   4 +-
 22 files changed, 256 insertions(+), 234 deletions(-)

diff --git a/drivers/staging/rtl8712/ieee80211.c 
b/drivers/staging/rtl8712/ieee80211.c
index 3c38fcb..974ca02 100644
--- a/drivers/staging/rtl8712/ieee80211.c
+++ b/drivers/staging/rtl8712/ieee80211.c
@@ -336,7 +336,8 @@ int r8712_parse_wpa2_ie(u8 *rsn_ie, int rsn_ie_len, int 
*group_cipher,
/* No RSN IE - fail silently */
return _FAIL;
}
-   if ((*rsn_ie != _WPA2_IE_ID_) || (*(rsn_ie+1) != (u8)(rsn_ie_len - 2)))
+   if ((*rsn_ie != _WPA2_IE_ID_) ||
+   (*(rsn_ie + 1) != (u8)(rsn_ie_len - 2)))
return _FAIL;
pos = rsn_ie;
pos += 4;
@@ -381,16 +382,16 @@ int r8712_get_sec_ie(u8 *in_ie, uint in_len, u8 *rsn_ie, 
u16 *rsn_len,
if ((authmode == _WPA_IE_ID_) &&
(!memcmp(_ie[cnt + 2], _oui[0], 4))) {
memcpy(wpa_ie, _ie[cnt], in_ie[cnt + 1] + 2);
-   *wpa_len = in_ie[cnt+1]+2;
+   *wpa_len = in_ie[cnt + 1] + 2;
cnt += in_ie[cnt + 1] + 2;  /*get next */
} else {
if (authmode == _WPA2_IE_ID_) {
memcpy(rsn_ie, _ie[cnt],
in_ie[cnt + 1] + 2);
-   *rsn_len = in_ie[cnt+1] + 2;
-   cnt += in_ie[cnt+1] + 2;  /*get next*/
+   *rsn_len = in_ie[cnt + 1] + 2;
+   cnt += in_ie[cnt + 1] + 2;  /*get next*/
} else {
-   cnt += in_ie[cnt+1] + 2;   /*get next*/
+   cnt += in_ie[cnt + 1] + 2;   /*get next*/
}
}
}
@@ -408,14 +409,14 @@ int r8712_get_wps_ie(u8 *in_ie, uint in_len, u8 *wps_ie, 
uint *wps_ielen)
while (cnt < in_len) {
eid = in_ie[cnt];
if ((eid == _WPA_IE_ID_) &&
-   (!memcmp(_ie[cnt+2], wps_oui, 4))) {
-   memcpy(wps_ie, _ie[cnt], in_ie[cnt+1]+2);
-   *wps_ielen = in_ie[cnt+1]+2;
-   cnt += in_ie[cnt+1]+2;
+   (!memcmp(_ie[cnt + 2], wps_oui, 4))) {
+   memcpy(wps_ie, _ie[cnt], in_ie[cnt + 1] + 2);
+   *wps_ielen = in_ie[cnt + 1] + 2;
+   cnt += in_ie[cnt + 1] + 2;
match = true;
break;
}
-   cnt += in_ie[cnt+1]+2; /* goto next */
+   cnt += in_ie[cnt + 1] + 2; /* goto next */
}
return match;
 }
diff --git a/drivers/staging/rtl8712/mlme_linux.c 
b/drivers/staging/rtl8712/mlme_linux.c
index 6d253bf..e4e4bde 100644
--- a/drivers/staging/rtl8712/mlme_linux.c
+++ b/drivers/staging/rtl8712/mlme_linux.c
@@ -163,7 +163,7 @@ void r8712_report_sec_ie(struct _adapter *adapter, u8 
authmode, u8 *sec_ie)
p += sprintf(p, "%02x", sec_ie[i]);
p += sprintf(p, ")");
memset(, 0, sizeof(wrqu));
-   wrqu.data.length = p-buff;
+   wrqu.data.length = p - buff;
wrqu.data.length = (wrqu.data.length < IW_CUSTOM_MAX) ?
   wrqu.data.length : IW_CUSTOM_MAX;
wireless_send_event(a

[PATCH] staging: ft1000: Remove boolean comparisons

[Luis de Bethencourt]

Boolean tests do not need explicit comparison to true or false.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 4 ++--
 drivers/staging/ft1000/ft1000-usb/ft1000_hw.c| 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c 
b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
index eecfa37..dfb5da93 100644
--- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
+++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
@@ -1531,7 +1531,7 @@ static int ft1000_copy_up_pkt(struct net_device *dev)
tempword = ft1000_read_reg(dev, FT1000_REG_DFIFO);
*pbuffer++ = (u8) (tempword >> 8);
*pbuffer++ = (u8)tempword;
-   if (ft1000_chkcard(dev) == false) {
+   if (!ft1000_chkcard(dev)) {
kfree_skb(skb);
return FAILURE;
}
@@ -1822,7 +1822,7 @@ static irqreturn_t ft1000_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
 
-   if (ft1000_chkcard(dev) == false) {
+   if (!ft1000_chkcard(dev)) {
ft1000_disable_interrupts(dev);
return IRQ_HANDLED;
}
diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_hw.c 
b/drivers/staging/ft1000/ft1000-usb/ft1000_hw.c
index 9ea32ce..22582be 100644
--- a/drivers/staging/ft1000/ft1000-usb/ft1000_hw.c
+++ b/drivers/staging/ft1000/ft1000-usb/ft1000_hw.c
@@ -1465,7 +1465,7 @@ int ft1000_poll(void *dev_id)
u16 modulo;
u16 portid;
 
-   if (ft1000_chkcard(dev) == FALSE) {
+   if (!ft1000_chkcard(dev)) {
pr_debug("failed\n");
return -1;
}
-- 
2.5.1

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

[PATCH] staging: emxx_udc: Remove boolean comparisons

[Luis de Bethencourt]

Boolean tests do not need explicit comparison to true or false.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/emxx_udc/emxx_udc.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c 
b/drivers/staging/emxx_udc/emxx_udc.c
index 0492747..26c8d5b 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -1267,7 +1267,7 @@ static int _nbu2ss_start_transfer(
/* EPn */
if (ep->direct == USB_DIR_OUT) {
/* OUT */
-   if (bflag == FALSE)
+   if (!bflag)
nret = _nbu2ss_epn_out_transfer(udc, ep, req);
} else {
/* IN */
@@ -1509,7 +1509,7 @@ static inline int _nbu2ss_req_feature(struct nbu2ss_udc 
*udc, bool bset)
if (0x == (wIndex & 0xFF70)) {
if (selector == USB_ENDPOINT_HALT) {
ep_adrs = wIndex & 0xFF;
-   if (bset == FALSE) {
+   if (!bset) {
_nbu2ss_endpoint_toggle_reset(
udc, ep_adrs);
}
@@ -1756,7 +1756,7 @@ static inline int _nbu2ss_decode_request(struct 
nbu2ss_udc *udc)
}
}
 
-   if (bcall_back == FALSE) {
+   if (!bcall_back) {
if (udc->ep0state == EP0_IN_STATUS_PHASE) {
if (nret >= 0) {
/*--*/
@@ -2044,7 +2044,7 @@ static inline void _nbu2ss_epn_in_dma_int(
 
preq = >req;
 
-   if (req->dma_flag == FALSE)
+   if (!req->dma_flag)
return;
 
preq->actual += req->div_len;
@@ -2756,7 +2756,7 @@ static int nbu2ss_ep_queue(
bflag = list_empty(>queue);
list_add_tail(>queue, >queue);
 
-   if ((bflag != FALSE) && (ep->stalled == FALSE)) {
+   if (bflag && !ep->stalled) {
 
result = _nbu2ss_start_transfer(udc, ep, req, FALSE);
if (result < 0) {
-- 
2.5.1

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

[PATCH] staging: rtl8712: Remove boolean comparisons

[Luis de Bethencourt]

Boolean tests do not need explicit comparison to true or false.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/rtl8712/os_intfs.c|   4 +-
 drivers/staging/rtl8712/rtl8712_cmd.c |   9 +-
 drivers/staging/rtl8712/rtl8712_efuse.c   |  49 
 drivers/staging/rtl8712/rtl8712_io.c  |   6 +-
 drivers/staging/rtl8712/rtl8712_led.c | 164 +-
 drivers/staging/rtl8712/rtl8712_recv.c|  25 ++--
 drivers/staging/rtl8712/rtl8712_xmit.c|   4 +-
 drivers/staging/rtl8712/rtl871x_cmd.c |   8 +-
 drivers/staging/rtl8712/rtl871x_eeprom.c  |  22 ++--
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c |  56 -
 drivers/staging/rtl8712/rtl871x_ioctl_rtl.c   |  12 +-
 drivers/staging/rtl8712/rtl871x_ioctl_set.c   |  26 ++--
 drivers/staging/rtl8712/rtl871x_mlme.c|  77 ++--
 drivers/staging/rtl8712/rtl871x_mlme.h|   2 +-
 drivers/staging/rtl8712/rtl871x_mp_ioctl.c|  16 +--
 drivers/staging/rtl8712/rtl871x_pwrctrl.c |   6 +-
 drivers/staging/rtl8712/rtl871x_recv.c|  39 +++---
 drivers/staging/rtl8712/rtl871x_security.c|   4 +-
 drivers/staging/rtl8712/rtl871x_sta_mgt.c |   6 +-
 drivers/staging/rtl8712/rtl871x_xmit.c|  41 +++
 drivers/staging/rtl8712/usb_intf.c|   4 +-
 drivers/staging/rtl8712/usb_ops_linux.c   |  10 +-
 drivers/staging/rtl8712/xmit_linux.c  |   4 +-
 23 files changed, 292 insertions(+), 302 deletions(-)

diff --git a/drivers/staging/rtl8712/os_intfs.c 
b/drivers/staging/rtl8712/os_intfs.c
index 6805f79..92ab59d 100644
--- a/drivers/staging/rtl8712/os_intfs.c
+++ b/drivers/staging/rtl8712/os_intfs.c
@@ -177,7 +177,7 @@ static int r871x_net_set_mac_address(struct net_device 
*pnetdev, void *p)
struct _adapter *padapter = netdev_priv(pnetdev);
struct sockaddr *addr = p;
 
-   if (padapter->bup == false)
+   if (!padapter->bup)
ether_addr_copy(pnetdev->dev_addr, addr->sa_data);
return 0;
 }
@@ -384,7 +384,7 @@ static int netdev_open(struct net_device *pnetdev)
struct _adapter *padapter = netdev_priv(pnetdev);
 
mutex_lock(>mutex_start);
-   if (padapter->bup == false) {
+   if (!padapter->bup) {
padapter->bDriverStopped = false;
padapter->bSurpriseRemoved = false;
padapter->bup = true;
diff --git a/drivers/staging/rtl8712/rtl8712_cmd.c 
b/drivers/staging/rtl8712/rtl8712_cmd.c
index f70235c..39b6320 100644
--- a/drivers/staging/rtl8712/rtl8712_cmd.c
+++ b/drivers/staging/rtl8712/rtl8712_cmd.c
@@ -85,7 +85,7 @@ static void query_fw_rx_phy_status(struct _adapter *padapter)
u32 val32 = 0;
int pollingcnts = 50;
 
-   if (check_fwstate(>mlmepriv, _FW_LINKED) == true) {
+   if (check_fwstate(>mlmepriv, _FW_LINKED)) {
r8712_write32(padapter, IOCMD_CTRL_REG, 0xf401);
msleep(100);
/* Wait FW complete IO Cmd */
@@ -324,8 +324,7 @@ int r8712_cmd_thread(void *context)
while (1) {
if ((_down_sema(&(pcmdpriv->cmd_queue_sema))) == _FAIL)
break;
-   if ((padapter->bDriverStopped == true) ||
-   (padapter->bSurpriseRemoved == true))
+   if (padapter->bDriverStopped || padapter->bSurpriseRemoved)
break;
if (r8712_register_cmd_alive(padapter) != _SUCCESS)
continue;
@@ -375,8 +374,8 @@ _next:
pcmdbuf += 2; /* 8 bytes alignment */
memcpy((u8 *)pcmdbuf, pcmd->parmbuf, pcmd->cmdsz);
while (check_cmd_fifo(padapter, wr_sz) == _FAIL) {
-   if ((padapter->bDriverStopped == true) ||
-   (padapter->bSurpriseRemoved == true))
+   if (padapter->bDriverStopped ||
+   padapter->bSurpriseRemoved)
break;
msleep(100);
continue;
diff --git a/drivers/staging/rtl8712/rtl8712_efuse.c 
b/drivers/staging/rtl8712/rtl8712_efuse.c
index d957169..83d418d 100644
--- a/drivers/staging/rtl8712/rtl8712_efuse.c
+++ b/drivers/staging/rtl8712/rtl8712_efuse.c
@@ -39,7 +39,7 @@ static void efuse_reg_ctrl(struct _adapter *padapter, u8 
bPowerOn)
 {
u8 tmpu8 = 0;
 
-   if (true == bPowerOn) {
+   if (bPowerOn) {
/* -e-fuse pwr & clk reg ctrl ---
 * Enable LDOE25 Macro Block
 */
@@ -127,7 +127,7 @@ static u8 efuse_one_byte_rw(struct _adapter *padapter, u8 
bRead, u16 addr,
tmpv8 = ((u8)((addr >> 8) & 0x03)) |
 (r871

[PATCH v3] staging: rtl8192u: simplify conditional

[Luis de Bethencourt]

The code can be much cleaner and readable by simplifying the conditional
statement.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---

Hi,

Dan Carpenter pointed out that the check needs to be >= and not just > because
ieee->state can change between the two if statements.

Thanks for the review!
Luis

 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c 
b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
index f2d09fe..8b09c40 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
@@ -466,13 +466,9 @@ void ieee80211_softmac_scan_syncro(struct ieee80211_device 
*ieee)
/* this prevent excessive time wait when we
 * need to wait for a syncro scan to end..
 */
-   if(ieee->state < IEEE80211_LINKED)
-   ;
-   else
-   if (ieee->sync_scan_hurryup)
+   if (ieee->state >= IEEE80211_LINKED && ieee->sync_scan_hurryup)
goto out;
 
-
msleep_interruptible_rsl(IEEE80211_SOFTMAC_SCAN_TIME);
 
}
-- 
2.5.1

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

[PATCH v2] staging: rtl8192u: simplify conditional statement

[Luis de Bethencourt]

The code can be much cleaner and readable by simplifying the conditional
statement.

Only need to check if (ieee->state > IEEE80211_LINKED) and not >= because
(ieee->state == IEEE80211_LINKED) is already checked a few lines above.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---

Sorry for the typo in the previous version and not double checking.

Shame,
Luis

 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c 
b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
index c443e2e..f85e5ff 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
@@ -466,10 +466,7 @@ void ieee80211_softmac_scan_syncro(struct ieee80211_device 
*ieee)
/* this prevent excessive time wait when we
 * need to wait for a syncro scan to end..
 */
-   if(ieee->state < IEEE80211_LINKED)
-   ;
-   else
-   if (ieee->sync_scan_hurryup)
+   if (ieee->state > IEEE80211_LINKED && ieee->sync_scan_hurryup)
goto out;
 
 
-- 
2.5.1

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

[PATCH] staging: wilc1000: Remove boolean comparisons

[Luis de Bethencourt]

Boolean tests do not need explicit comparison to true or false.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 706912b..16a2a5f 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -363,7 +363,7 @@ static void CfgScanResult(enum scan_event enuScanEvent, 
tstrNetworkInfo *pstrNet
struct cfg80211_bss *bss = NULL;
 
priv = (struct wilc_priv *)pUserVoid;
-   if (priv->bCfgScanning == true) {
+   if (priv->bCfgScanning) {
if (enuScanEvent == SCAN_EVENT_NETWORK_FOUND) {
wiphy = priv->dev->ieee80211_ptr->wiphy;
 
@@ -391,7 +391,7 @@ static void CfgScanResult(enum scan_event enuScanEvent, 
tstrNetworkInfo *pstrNet
   "BeaconPeriod: %d\n", 
channel->center_freq, (((s32)pstrNetworkInfo->s8rssi) * 100),
   pstrNetworkInfo->u16CapInfo, 
pstrNetworkInfo->u16BeaconPeriod);
 
-   if (pstrNetworkInfo->bNewNetwork == true) {
+   if (pstrNetworkInfo->bNewNetwork) {
if (priv->u32RcvdChCount < 
MAX_NUM_SCANNED_NETWORKS) { /* TODO: mostafa: to be replaced by */
/*   max_scan_ssids 
*/
PRINT_D(CFG80211_DBG, "Network 
%s found\n", pstrNetworkInfo->au8ssid);
@@ -1947,7 +1947,7 @@ void WILC_WFI_p2p_rx (struct net_device *dev, u8 *buff, 
u32 size)
if (ieee80211_is_action(buff[FRAME_TYPE_ID])) {
PRINT_D(GENERIC_DBG, "Rx Action Frame Type: %x %x\n", 
buff[ACTION_SUBTYPE_ID], buff[P2P_PUB_ACTION_SUBTYPE]);
 
-   if (priv->bCfgScanning == true && 
time_after_eq(jiffies, (unsigned long)pstrWFIDrv->u64P2p_MgmtTimeout)) {
+   if (priv->bCfgScanning && time_after_eq(jiffies, 
(unsigned long)pstrWFIDrv->u64P2p_MgmtTimeout)) {
PRINT_D(GENERIC_DBG, "Receiving action frames 
from wrong channels\n");
return;
}
@@ -2360,7 +2360,7 @@ static int mgmt_tx_cancel_wait(struct wiphy *wiphy,
PRINT_D(GENERIC_DBG, "Tx Cancel wait :%lu\n", jiffies);
pstrWFIDrv->u64P2p_MgmtTimeout = jiffies;
 
-   if (priv->bInP2PlistenState == false) {
+   if (!priv->bInP2PlistenState) {
cfg80211_remain_on_channel_expired(priv->wdev,
   
priv->strRemainOnChanParams.u64ListenCookie,
   
priv->strRemainOnChanParams.pstrListenChan,
-- 
2.5.1

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

[PATCH] staging: rtl8192u: simplify conditional statement

[Luis de Bethencourt]

The code can be much cleaner and readable by simplifying the conditional
statement.

Only need to check if (ieee->state > IEEE80211_LINKED) and not >= because
(ieee->state == IEEE80211_LINKED) is already checked a few lines above.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c 
b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
index c443e2e..1c2d1a4 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
@@ -466,10 +466,7 @@ void ieee80211_softmac_scan_syncro(struct ieee80211_device 
*ieee)
/* this prevent excessive time wait when we
 * need to wait for a syncro scan to end..
 */
-   if(ieee->state < IEEE80211_LINKED)
-   ;
-   else
-   if (ieee->sync_scan_hurryup)
+   if(!ieee->state > IEEE80211_LINKED && ieee->sync_scan_hurryup)
goto out;
 
 
-- 
2.5.1

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