Re: [PATCH 1/3] Staging: iio: adis16209: Use SPDX identifier
Hi Shreeya, On Sat, Feb 17, 2018 at 09:34:56PM +0530, Shreeya Patel wrote: > Use SPDX identifier format instead of GPLv2. Also rearrange the > headers in alphabetical order. > > Signed-off-by: Shreeya Patel > --- > drivers/staging/iio/accel/adis16209.c | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/iio/accel/adis16209.c > b/drivers/staging/iio/accel/adis16209.c > index 7fcef9a..e3d9f80 100644 > --- a/drivers/staging/iio/accel/adis16209.c > +++ b/drivers/staging/iio/accel/adis16209.c > @@ -1,19 +1,18 @@ > +// SPDX-License-Identifier: GPL-2.0+ > /* > * ADIS16209 Dual-Axis Digital Inclinometer and Accelerometer > * > * Copyright 2010 Analog Devices Inc. > - * > - * Licensed under the GPL-2 or later. I see that you too are doing similar cleanup which I did a while ago https://lkml.org/lkml/2018/2/12/255 where I got some update suggestions for the patch series. It would be great if you could update this patch series consistent with the reviewers. For eg: in this patch you changed +// SPDX-License-Identifier: GPL-2.0+ and therefore MODULE_LICENSE("GPL v2"); should also be changed to MODULE_LICENSE("GPL"); as explained by Philippe Ombredanne to me in my patch series. I am not sure if you're subscribed to the IIO mailing list but you can find all the update suggestions from the above link. :) -- Thanks Himanshu Jha ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging:r8188eu: Use lib80211 to decrypt WEP-frames
Use native lib80211 WEP decrypt instead of custom implementation. Signed-off-by: Ivan Safonov --- drivers/staging/rtl8188eu/Kconfig| 2 + drivers/staging/rtl8188eu/core/rtw_recv.c| 2 +- drivers/staging/rtl8188eu/core/rtw_security.c| 80 +--- drivers/staging/rtl8188eu/include/rtw_security.h | 2 +- 4 files changed, 49 insertions(+), 37 deletions(-) diff --git a/drivers/staging/rtl8188eu/Kconfig b/drivers/staging/rtl8188eu/Kconfig index cb836c59d564..d787a091d3c1 100644 --- a/drivers/staging/rtl8188eu/Kconfig +++ b/drivers/staging/rtl8188eu/Kconfig @@ -4,6 +4,8 @@ config R8188EU depends on m select WIRELESS_EXT select WEXT_PRIV + select LIB80211 + select LIB80211_CRYPT_WEP ---help--- This option adds the Realtek RTL8188EU USB device such as TP-Link TL-WN725N. If built as a module, it will be called r8188eu. diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c b/drivers/staging/rtl8188eu/core/rtw_recv.c index 6506a1587df0..fe31ebbf36fb 100644 --- a/drivers/staging/rtl8188eu/core/rtw_recv.c +++ b/drivers/staging/rtl8188eu/core/rtw_recv.c @@ -404,7 +404,7 @@ static struct recv_frame *decryptor(struct adapter *padapter, switch (prxattrib->encrypt) { case _WEP40_: case _WEP104_: - rtw_wep_decrypt(padapter, (u8 *)precv_frame); + res = rtw_wep_decrypt(padapter, (u8 *)precv_frame); break; case _TKIP_: res = rtw_tkip_decrypt(padapter, (u8 *)precv_frame); diff --git a/drivers/staging/rtl8188eu/core/rtw_security.c b/drivers/staging/rtl8188eu/core/rtw_security.c index 5b1ef229df2a..72da86fdd264 100644 --- a/drivers/staging/rtl8188eu/core/rtw_security.c +++ b/drivers/staging/rtl8188eu/core/rtw_security.c @@ -18,6 +18,7 @@ #include #include #include +#include /* WEP related = */ @@ -195,48 +196,57 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) } -void rtw_wep_decrypt(struct adapter *padapter, u8 *precvframe) +int rtw_wep_decrypt(struct adapter *padapter, u8 *precvframe) { - /* exclude ICV */ - u8 crc[4]; - struct arc4context mycontext; - int length; - u32 keylength; - u8 *pframe, *payload, *iv, wepkey[16]; - u8 keyindex; struct rx_pkt_attrib*prxattrib = &(((struct recv_frame *)precvframe)->attrib); - struct security_priv *psecuritypriv = &padapter->securitypriv; + if ((prxattrib->encrypt == _WEP40_) || (prxattrib->encrypt == _WEP104_)) { + struct security_priv *psecuritypriv = &padapter->securitypriv; + struct sk_buff *skb = ((struct recv_frame *)precvframe)->pkt; + u8 *pframe = skb->data; + void *crypto_private = NULL; + int status = _SUCCESS; + const int keyindex = prxattrib->key_index; + struct lib80211_crypto_ops *crypto_ops = try_then_request_module(lib80211_get_crypto_ops("WEP"), "lib80211_crypt_wep"); + char iv[4], icv[4]; + + if (!crypto_ops) { + status = _FAIL; + goto exit; + } - pframe = (unsigned char *)((struct recv_frame *)precvframe)->pkt->data; + memcpy(iv, pframe + prxattrib->hdrlen, 4); + memcpy(icv, pframe + skb->len - 4, 4); - /* start to decrypt recvframe */ - if ((prxattrib->encrypt == _WEP40_) || (prxattrib->encrypt == _WEP104_)) { - iv = pframe+prxattrib->hdrlen; - keyindex = prxattrib->key_index; - keylength = psecuritypriv->dot11DefKeylen[keyindex]; - memcpy(&wepkey[0], iv, 3); - memcpy(&wepkey[3], &psecuritypriv->dot11DefKey[keyindex].skey[0], keylength); - length = ((struct recv_frame *)precvframe)->pkt->len-prxattrib->hdrlen-prxattrib->iv_len; - - payload = pframe+prxattrib->iv_len+prxattrib->hdrlen; - - /* decrypt payload include icv */ - arcfour_init(&mycontext, wepkey, 3+keylength); - arcfour_encrypt(&mycontext, payload, payload, length); - - /* calculate icv and compare the icv */ - *((__le32 *)crc) = getcrc32(payload, length - 4); - - if (crc[3] != payload[length-1] || - crc[2] != payload[length-2] || - crc[1] != payload[length-3] || - crc[0] != payload[length-4]) { - RT_TRACE(_module_rtl871x_security_c_, _drv_err_, -("rtw_wep_decrypt:icv error crc (%4ph)!=payload (%4ph)\n", -&crc, &payload[length-4])); + crypto_private = crypto_ops->init(keyindex); + if (!crypto_private) { +
Re: [PATCH 1/3] Staging: iio: adis16209: Use SPDX identifier
On Sun, 2018-02-18 at 17:01 +0530, Himanshu Jha wrote: > Hi Shreeya, > Hi Himanshu, > On Sat, Feb 17, 2018 at 09:34:56PM +0530, Shreeya Patel wrote: > > > > Use SPDX identifier format instead of GPLv2. Also rearrange the > > headers in alphabetical order. > > > > Signed-off-by: Shreeya Patel > > --- > > drivers/staging/iio/accel/adis16209.c | 7 +++ > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/staging/iio/accel/adis16209.c > > b/drivers/staging/iio/accel/adis16209.c > > index 7fcef9a..e3d9f80 100644 > > --- a/drivers/staging/iio/accel/adis16209.c > > +++ b/drivers/staging/iio/accel/adis16209.c > > @@ -1,19 +1,18 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > /* > > * ADIS16209 Dual-Axis Digital Inclinometer and Accelerometer > > * > > * Copyright 2010 Analog Devices Inc. > > - * > > - * Licensed under the GPL-2 or later. > I see that you too are doing similar cleanup which I did a while ago > https://lkml.org/lkml/2018/2/12/255 Yes, Jonathan suggested me to work on adis16209. Your patches were quite useful for me :) > where I got some update suggestions for the patch series. It would be > great if you could update this patch series consistent with the > reviewers. > > For eg: in this patch you changed > > +// SPDX-License-Identifier: GPL-2.0+ > > and therefore > > MODULE_LICENSE("GPL v2"); > > should also be changed to > > MODULE_LICENSE("GPL"); > > as explained by Philippe Ombredanne to me in my patch series. Great. I'll make the necessary changes. > > I am not sure if you're subscribed to the IIO mailing list but you > can find > all the update suggestions from the above link. :) Thanks for your suggestions > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] Staging: iio: adis16209: Use SPDX identifier
On Sun, 18 Feb 2018 17:07:57 +0530 Shreeya Patel wrote: > On Sun, 2018-02-18 at 17:01 +0530, Himanshu Jha wrote: > > Hi Shreeya, > > > Hi Himanshu, > > > On Sat, Feb 17, 2018 at 09:34:56PM +0530, Shreeya Patel wrote: > > > > > > Use SPDX identifier format instead of GPLv2. Also rearrange the > > > headers in alphabetical order. > > > > > > Signed-off-by: Shreeya Patel > > > --- > > > drivers/staging/iio/accel/adis16209.c | 7 +++ > > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/staging/iio/accel/adis16209.c > > > b/drivers/staging/iio/accel/adis16209.c > > > index 7fcef9a..e3d9f80 100644 > > > --- a/drivers/staging/iio/accel/adis16209.c > > > +++ b/drivers/staging/iio/accel/adis16209.c > > > @@ -1,19 +1,18 @@ > > > +// SPDX-License-Identifier: GPL-2.0+ > > > /* > > > * ADIS16209 Dual-Axis Digital Inclinometer and Accelerometer > > > * > > > * Copyright 2010 Analog Devices Inc. > > > - * > > > - * Licensed under the GPL-2 or later. > > I see that you too are doing similar cleanup which I did a while ago > > https://lkml.org/lkml/2018/2/12/255 > > Yes, Jonathan suggested me to work on adis16209. > Your patches were quite useful for me :) > > > where I got some update suggestions for the patch series. It would be > > great if you could update this patch series consistent with the > > reviewers. > > > > For eg: in this patch you changed > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > and therefore > > > > MODULE_LICENSE("GPL v2"); > > > > should also be changed to > > > > MODULE_LICENSE("GPL"); > > > > as explained by Philippe Ombredanne to me in my patch series. > I'm not sure that was exactly what Philippe was suggesting. He was making the point that the licensing was inconsistent without saying which option should be chosen. We will need to seek clarification from Analog Devices on what their opinion on this is. Lars / Michael, any clarification on the right way to resolve this inconsistency? > Great. > I'll make the necessary changes. > > > > > I am not sure if you're subscribed to the IIO mailing list but you > > can find > > all the update suggestions from the above link. :) > > Thanks for your suggestions > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] Staging: iio: adis16209: Use SPDX identifier
On Sat, 17 Feb 2018 21:34:56 +0530 Shreeya Patel wrote: > Use SPDX identifier format instead of GPLv2. Also rearrange the > headers in alphabetical order. As Dan pointed out for the Himanshu Jha's patches, the moment we see the word 'also' in a patch it pretty much makes it obvious that the author knows it is doing two things. Two patches please. This is all about making the changes trivial to review. Sometimes it may seem silly where we have two small changes like here, but when you are reviewing hundreds of patches it really does help. Jonathan > > Signed-off-by: Shreeya Patel > --- > drivers/staging/iio/accel/adis16209.c | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/iio/accel/adis16209.c > b/drivers/staging/iio/accel/adis16209.c > index 7fcef9a..e3d9f80 100644 > --- a/drivers/staging/iio/accel/adis16209.c > +++ b/drivers/staging/iio/accel/adis16209.c > @@ -1,19 +1,18 @@ > +// SPDX-License-Identifier: GPL-2.0+ > /* > * ADIS16209 Dual-Axis Digital Inclinometer and Accelerometer > * > * Copyright 2010 Analog Devices Inc. > - * > - * Licensed under the GPL-2 or later. > */ > > #include > #include > #include > +#include > +#include > #include > #include > #include > -#include > -#include > > #include > #include ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] Staging: iio: adis16209: Use SPDX identifier
On 02/18/2018 01:08 PM, Lars-Peter Clausen wrote: > On 02/18/2018 01:02 PM, Jonathan Cameron wrote: >> On Sun, 18 Feb 2018 17:07:57 +0530 >> Shreeya Patel wrote: >> >>> On Sun, 2018-02-18 at 17:01 +0530, Himanshu Jha wrote: Hi Shreeya, >>> Hi Himanshu, >>> On Sat, Feb 17, 2018 at 09:34:56PM +0530, Shreeya Patel wrote: > > Use SPDX identifier format instead of GPLv2. Also rearrange the > headers in alphabetical order. > > Signed-off-by: Shreeya Patel > --- > drivers/staging/iio/accel/adis16209.c | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/iio/accel/adis16209.c > b/drivers/staging/iio/accel/adis16209.c > index 7fcef9a..e3d9f80 100644 > --- a/drivers/staging/iio/accel/adis16209.c > +++ b/drivers/staging/iio/accel/adis16209.c > @@ -1,19 +1,18 @@ > +// SPDX-License-Identifier: GPL-2.0+ > /* > * ADIS16209 Dual-Axis Digital Inclinometer and Accelerometer > * > * Copyright 2010 Analog Devices Inc. > - * > - * Licensed under the GPL-2 or later. I see that you too are doing similar cleanup which I did a while ago https://lkml.org/lkml/2018/2/12/255 >>> >>> Yes, Jonathan suggested me to work on adis16209. >>> Your patches were quite useful for me :) >>> where I got some update suggestions for the patch series. It would be great if you could update this patch series consistent with the reviewers. For eg: in this patch you changed +// SPDX-License-Identifier: GPL-2.0+ and therefore MODULE_LICENSE("GPL v2"); should also be changed to MODULE_LICENSE("GPL"); as explained byPhilippe Ombredanne to me in my patch series. >>> >> I'm not sure that was exactly what Philippe was suggesting. >> He was making the point that the licensing was inconsistent without >> saying which option should be chosen. >> >> We will need to seek clarification from Analog Devices >> on what their opinion on this is. >> >> Lars / Michael, any clarification on the right way to resolve this >> inconsistency? > > I can't speak for the intended license for code I wasn't involved in. > > But I'd in general if there are conflicting licensing information and you > want to be on the safe side choose the more restrictive license. E.g. GPL2+ > is compatible with GPL2, but GPL2 is not compatible with GPL2+. So to be > compatible with both choose GPL2. This is not legal advice btw. I personally would stay away from messing with the licenses of code I do not own. Not everybody seems to agree yet that a SPDX tag is equivalent to a explicit licensing statement. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] Staging: iio: adis16209: Use SPDX identifier
On 02/18/2018 01:02 PM, Jonathan Cameron wrote: > On Sun, 18 Feb 2018 17:07:57 +0530 > Shreeya Patel wrote: > >> On Sun, 2018-02-18 at 17:01 +0530, Himanshu Jha wrote: >>> Hi Shreeya, >>> >> Hi Himanshu, >> >>> On Sat, Feb 17, 2018 at 09:34:56PM +0530, Shreeya Patel wrote: Use SPDX identifier format instead of GPLv2. Also rearrange the headers in alphabetical order. Signed-off-by: Shreeya Patel --- drivers/staging/iio/accel/adis16209.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/staging/iio/accel/adis16209.c b/drivers/staging/iio/accel/adis16209.c index 7fcef9a..e3d9f80 100644 --- a/drivers/staging/iio/accel/adis16209.c +++ b/drivers/staging/iio/accel/adis16209.c @@ -1,19 +1,18 @@ +// SPDX-License-Identifier: GPL-2.0+ /* * ADIS16209 Dual-Axis Digital Inclinometer and Accelerometer * * Copyright 2010 Analog Devices Inc. - * - * Licensed under the GPL-2 or later. >>> I see that you too are doing similar cleanup which I did a while ago >>> https://lkml.org/lkml/2018/2/12/255 >> >> Yes, Jonathan suggested me to work on adis16209. >> Your patches were quite useful for me :) >> >>> where I got some update suggestions for the patch series. It would be >>> great if you could update this patch series consistent with the >>> reviewers. >>> >>> For eg: in this patch you changed >>> >>> +// SPDX-License-Identifier: GPL-2.0+ >>> >>> and therefore >>> >>> MODULE_LICENSE("GPL v2"); >>> >>> should also be changed to >>> >>> MODULE_LICENSE("GPL"); >>> >>> as explained by Philippe Ombredanne to me in my patch series. >> > I'm not sure that was exactly what Philippe was suggesting. > He was making the point that the licensing was inconsistent without > saying which option should be chosen. > > We will need to seek clarification from Analog Devices > on what their opinion on this is. > > Lars / Michael, any clarification on the right way to resolve this > inconsistency? I can't speak for the intended license for code I wasn't involved in. But I'd in general if there are conflicting licensing information and you want to be on the safe side choose the more restrictive license. E.g. GPL2+ is compatible with GPL2, but GPL2 is not compatible with GPL2+. So to be compatible with both choose GPL2. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] Staging: iio: adis16209: Remove unnecessary comments and add suitable suffix
On Sat, 17 Feb 2018 21:37:37 +0530 Shreeya Patel wrote: > Remove some unnecessary comments and therefore, add _REG suffix > for registers. Group the control register and register field > macros together. > Also, align function arguments to match open parentheses using > tabs. Lots of different changes in here - one patch per change type please. (so probably 3 here) A few comments inline. Jonathan > > Signed-off-by: Shreeya Patel > --- > drivers/staging/iio/accel/adis16209.c | 209 > +++--- > 1 file changed, 69 insertions(+), 140 deletions(-) > > diff --git a/drivers/staging/iio/accel/adis16209.c > b/drivers/staging/iio/accel/adis16209.c > index e3d9f80..6101212 100644 > --- a/drivers/staging/iio/accel/adis16209.c > +++ b/drivers/staging/iio/accel/adis16209.c > @@ -19,136 +19,67 @@ > #include > #include > > -#define ADIS16209_STARTUP_DELAY 220 /* ms */ > - > -/* Flash memory write count */ > -#define ADIS16209_FLASH_CNT 0x00 > - > -/* Output, power supply */ > -#define ADIS16209_SUPPLY_OUT 0x02 > - > -/* Output, x-axis accelerometer */ > -#define ADIS16209_XACCL_OUT 0x04 > - > -/* Output, y-axis accelerometer */ > -#define ADIS16209_YACCL_OUT 0x06 > - > -/* Output, auxiliary ADC input */ > -#define ADIS16209_AUX_ADC0x08 > - > -/* Output, temperature */ > -#define ADIS16209_TEMP_OUT 0x0A > - > -/* Output, x-axis inclination */ > -#define ADIS16209_XINCL_OUT 0x0C > - > -/* Output, y-axis inclination */ > -#define ADIS16209_YINCL_OUT 0x0E > - > -/* Output, +/-180 vertical rotational position */ > -#define ADIS16209_ROT_OUT0x10 > - > -/* Calibration, x-axis acceleration offset null */ > -#define ADIS16209_XACCL_NULL 0x12 > - > -/* Calibration, y-axis acceleration offset null */ > -#define ADIS16209_YACCL_NULL 0x14 > - > -/* Calibration, x-axis inclination offset null */ > -#define ADIS16209_XINCL_NULL 0x16 > - > -/* Calibration, y-axis inclination offset null */ > -#define ADIS16209_YINCL_NULL 0x18 > - > -/* Calibration, vertical rotation offset null */ > -#define ADIS16209_ROT_NULL 0x1A > - > -/* Alarm 1 amplitude threshold */ > -#define ADIS16209_ALM_MAG1 0x20 > - > -/* Alarm 2 amplitude threshold */ > -#define ADIS16209_ALM_MAG2 0x22 > - > -/* Alarm 1, sample period */ > -#define ADIS16209_ALM_SMPL1 0x24 > - > -/* Alarm 2, sample period */ > -#define ADIS16209_ALM_SMPL2 0x26 > - > -/* Alarm control */ > -#define ADIS16209_ALM_CTRL 0x28 > - > -/* Auxiliary DAC data */ > -#define ADIS16209_AUX_DAC0x30 > - > -/* General-purpose digital input/output control */ > -#define ADIS16209_GPIO_CTRL 0x32 > - > -/* Miscellaneous control */ > -#define ADIS16209_MSC_CTRL 0x34 > - > -/* Internal sample period (rate) control */ > -#define ADIS16209_SMPL_PRD 0x36 > - > -/* Operation, filter configuration */ > -#define ADIS16209_AVG_CNT0x38 > - > -/* Operation, sleep mode control */ > -#define ADIS16209_SLP_CNT0x3A > - > -/* Diagnostics, system status register */ > -#define ADIS16209_DIAG_STAT 0x3C > - > -/* Operation, system command register */ > -#define ADIS16209_GLOB_CMD 0x3E > +#define ADIS16209_STARTUP_DELAY_MS 220 This one is a different type of change to the others, I would break it out in another patch. > +#define ADIS16209_FLASH_CNT_REG 0x00 > + > +/* Data Output Register Definitions */ > +#define ADIS16209_SUPPLY_OUT_REG 0x02 > +#define ADIS16209_XACCL_OUT_REG 0x04 > +#define ADIS16209_YACCL_OUT_REG 0x06 > +#define ADIS16209_AUX_ADC_REG0x08 > +#define ADIS16209_TEMP_OUT_REG 0x0A > +#define ADIS16209_XINCL_OUT_REG 0x0C For these last 3 I'm not sure that it is obvious what they are - I had to look at the datasheet. Perhaps a comment block explaining how these relate to the orientation of the device would be a useful addition. This may be a little 'controversial' so would be good to have this clearly laid out. > +#define ADIS16209_YINCL_OUT_REG 0x0E > +#define ADIS16209_ROT_OUT_REG0x10 > + > +/* Calibration Register Definitions */ > +#define ADIS16209_XACCL_NULL_REG 0x12 > +#define ADIS16209_YACCL_NULL_REG 0x14 > +#define ADIS16209_XINCL_NULL_REG 0x16 > +#define ADIS16209_YINCL_NULL_REG 0x18 > +#define ADIS16209_ROT_NULL_REG 0x1A This group are not necessarily clear in meaning. I would explain in the comment what is meant by NULL in this case. > + > +/* Alarm Register Definitions */ > +#define ADIS16209_ALM_MAG1_REG 0x20 > +#define ADIS16209_ALM_MAG2_REG 0x22 > +#define ADIS16209_ALM_SMPL1_REG 0x24 > +#define ADIS16209_ALM_SMPL2_REG 0x26 > +#define ADIS16209_ALM_CTRL_REG 0x28 Hmm. This is an interesting trade off. There isn't actually enough defined here to allow implementation of the ALM functionali
Re: [PATCH 3/3] Staging: iio: adis16209: Use sign_extend32 and adjust a switch statement
On Sat, 17 Feb 2018 21:51:35 +0530 Shreeya Patel wrote: > Use sign_extend32 function instead of manually coding it. Also, adjust a > switch block to explicitly match channels and return -EINVAL as default > case which improves code readability. Two items, two patches please. The -EINVAL is a bit of an odd one. We know it can never happen and in reality we only add it to suppress the static checker warnings that may otherwise ensue. I would definitely not say that it helps readability. The change to the switch statement makes it clear there are only two channels so is semantically clearer to my mind and it is that which I argued improved readability. Jonathan > > Signed-off-by: Shreeya Patel > --- > drivers/staging/iio/accel/adis16209.c | 15 ++- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/iio/accel/adis16209.c > b/drivers/staging/iio/accel/adis16209.c > index 6101212..f7c228b 100644 > --- a/drivers/staging/iio/accel/adis16209.c > +++ b/drivers/staging/iio/accel/adis16209.c > @@ -150,10 +150,16 @@ static int adis16209_read_raw(struct iio_dev *indio_dev, > switch (chan->type) { > case IIO_VOLTAGE: > *val = 0; > - if (chan->channel == 0) > + switch (chan->channel) { > + case 0: > *val2 = 305180; > - else > + break; > + case 1: > *val2 = 610500; > + break; > + default: > + return -EINVAL; > + } > return IIO_VAL_INT_PLUS_MICRO; > case IIO_TEMP: > *val = -470; > @@ -187,9 +193,8 @@ static int adis16209_read_raw(struct iio_dev *indio_dev, > ret = adis_read_reg_16(st, addr, &val16); > if (ret) > return ret; > - val16 &= (1 << bits) - 1; > - val16 = (s16)(val16 << (16 - bits)) >> (16 - bits); > - *val = val16; > + > + *val = sign_extend32(val16, bits - 1); > return IIO_VAL_INT; > } > return -EINVAL; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] Staging: iio: adis16209: Use SPDX identifier
On Sun, 18 Feb 2018 13:14:27 +0100 Lars-Peter Clausen wrote: > On 02/18/2018 01:08 PM, Lars-Peter Clausen wrote: > > On 02/18/2018 01:02 PM, Jonathan Cameron wrote: > >> On Sun, 18 Feb 2018 17:07:57 +0530 > >> Shreeya Patel wrote: > >> > >>> On Sun, 2018-02-18 at 17:01 +0530, Himanshu Jha wrote: > Hi Shreeya, > > >>> Hi Himanshu, > >>> > On Sat, Feb 17, 2018 at 09:34:56PM +0530, Shreeya Patel wrote: > > > > Use SPDX identifier format instead of GPLv2. Also rearrange the > > headers in alphabetical order. > > > > Signed-off-by: Shreeya Patel > > --- > > drivers/staging/iio/accel/adis16209.c | 7 +++ > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/staging/iio/accel/adis16209.c > > b/drivers/staging/iio/accel/adis16209.c > > index 7fcef9a..e3d9f80 100644 > > --- a/drivers/staging/iio/accel/adis16209.c > > +++ b/drivers/staging/iio/accel/adis16209.c > > @@ -1,19 +1,18 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > /* > > * ADIS16209 Dual-Axis Digital Inclinometer and Accelerometer > > * > > * Copyright 2010 Analog Devices Inc. > > - * > > - * Licensed under the GPL-2 or later. > I see that you too are doing similar cleanup which I did a while ago > https://lkml.org/lkml/2018/2/12/255 > >>> > >>> Yes, Jonathan suggested me to work on adis16209. > >>> Your patches were quite useful for me :) > >>> > where I got some update suggestions for the patch series. It would be > great if you could update this patch series consistent with the > reviewers. > > For eg: in this patch you changed > > +// SPDX-License-Identifier: GPL-2.0+ > > and therefore > > MODULE_LICENSE("GPL v2"); > > should also be changed to > > MODULE_LICENSE("GPL"); > > as explained by Philippe Ombredanne to me in my patch series. > >>> > >> I'm not sure that was exactly what Philippe was suggesting. > >> He was making the point that the licensing was inconsistent without > >> saying which option should be chosen. > >> > >> We will need to seek clarification from Analog Devices > >> on what their opinion on this is. > >> > >> Lars / Michael, any clarification on the right way to resolve this > >> inconsistency? > > > > I can't speak for the intended license for code I wasn't involved in. > > > > But I'd in general if there are conflicting licensing information and you > > want to be on the safe side choose the more restrictive license. E.g. GPL2+ > > is compatible with GPL2, but GPL2 is not compatible with GPL2+. So to be > > compatible with both choose GPL2. > > This is not legal advice btw. > > I personally would stay away from messing with the licenses of code I do not > own. Not everybody seems to agree yet that a SPDX tag is equivalent to a > explicit licensing statement. > OK, in that case let's not do the spdx conversions for these drivers. We probably need to fix the inconsistency however between the text and the module license tag. Doesn't need to be part of moving it out of staging however. Jonathan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: ks7010: Fix Unnecessary parentheses in ks_hostif.c
Hi Yash, Thank you for the patch! Yet something to improve: [auto build test ERROR on staging/staging-testing] [also build test ERROR on v4.16-rc1 next-20180216] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Yash-Omer/Staging-ks7010-Fix-Unnecessary-parentheses-in-ks_hostif-c/20180218-223344 config: i386-randconfig-x017-201807 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/staging/ks7010/ks_hostif.c: In function 'hostif_scan_indication': >> drivers/staging/ks7010/ks_hostif.c:853:26: error: expected ')' before 'priv' &(priv->aplist.ap priv->scan_ind_count - 1)); ^~~~ >> drivers/staging/ks7010/ks_hostif.c:853:8: error: passing argument 3 of >> 'get_ap_information' from incompatible pointer type >> [-Werror=incompatible-pointer-types] &(priv->aplist.ap priv->scan_ind_count - 1)); ^ drivers/staging/ks7010/ks_hostif.c:218:5: note: expected 'struct local_ap_t *' but argument is of type 'struct local_ap_t (*)[32]' int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, ^~ cc1: some warnings being treated as errors vim +853 drivers/staging/ks7010/ks_hostif.c 826 827 static 828 void hostif_scan_indication(struct ks_wlan_private *priv) 829 { 830 int i; 831 struct ap_info_t *ap_info; 832 833 DPRINTK(3, "scan_ind_count = %d\n", priv->scan_ind_count); 834 ap_info = (struct ap_info_t *)(priv->rxp); 835 836 if (priv->scan_ind_count) { 837 for (i = 0; i < priv->aplist.size; i++) { /* bssid check */ 838 if (memcmp(ap_info->bssid, 839 priv->aplist.ap[i].bssid, ETH_ALEN) != 0) 840 continue; 841 842 if (ap_info->frame_type == FRAME_TYPE_PROBE_RESP) 843 get_ap_information(priv, ap_info, 844 &priv->aplist.ap[i]); 845 return; 846 } 847 } 848 priv->scan_ind_count++; 849 if (priv->scan_ind_count < LOCAL_APLIST_MAX + 1) { 850 DPRINTK(4, " scan_ind_count=%d :: aplist.size=%d\n", 851 priv->scan_ind_count, priv->aplist.size); 852 get_ap_information(priv, (struct ap_info_t *)(priv->rxp), > 853 &(priv->aplist.ap > priv->scan_ind_count - 1)); 854 priv->aplist.size = priv->scan_ind_count; 855 } else { 856 DPRINTK(4, " count over :: scan_ind_count=%d\n", 857 priv->scan_ind_count); 858 } 859 } 860 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: ks7010: Fix Unnecessary parentheses in ks_hostif.c
Hi Yash, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on staging/staging-testing] [also build test WARNING on v4.16-rc1 next-20180216] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Yash-Omer/Staging-ks7010-Fix-Unnecessary-parentheses-in-ks_hostif-c/20180218-223344 config: i386-randconfig-a0-201807 (attached as .config) compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4 reproduce: # save the attached .config to linux build tree make ARCH=i386 All warnings (new ones prefixed by >>): drivers/staging/ks7010/ks_hostif.c: In function 'hostif_scan_indication': drivers/staging/ks7010/ks_hostif.c:853:26: error: expected ')' before 'priv' &(priv->aplist.ap priv->scan_ind_count - 1)); ^ >> drivers/staging/ks7010/ks_hostif.c:853:8: warning: passing argument 3 of >> 'get_ap_information' from incompatible pointer type &(priv->aplist.ap priv->scan_ind_count - 1)); ^ drivers/staging/ks7010/ks_hostif.c:218:5: note: expected 'struct local_ap_t *' but argument is of type 'struct local_ap_t (*)[32]' int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, ^ vim +/get_ap_information +853 drivers/staging/ks7010/ks_hostif.c 826 827 static 828 void hostif_scan_indication(struct ks_wlan_private *priv) 829 { 830 int i; 831 struct ap_info_t *ap_info; 832 833 DPRINTK(3, "scan_ind_count = %d\n", priv->scan_ind_count); 834 ap_info = (struct ap_info_t *)(priv->rxp); 835 836 if (priv->scan_ind_count) { 837 for (i = 0; i < priv->aplist.size; i++) { /* bssid check */ 838 if (memcmp(ap_info->bssid, 839 priv->aplist.ap[i].bssid, ETH_ALEN) != 0) 840 continue; 841 842 if (ap_info->frame_type == FRAME_TYPE_PROBE_RESP) 843 get_ap_information(priv, ap_info, 844 &priv->aplist.ap[i]); 845 return; 846 } 847 } 848 priv->scan_ind_count++; 849 if (priv->scan_ind_count < LOCAL_APLIST_MAX + 1) { 850 DPRINTK(4, " scan_ind_count=%d :: aplist.size=%d\n", 851 priv->scan_ind_count, priv->aplist.size); 852 get_ap_information(priv, (struct ap_info_t *)(priv->rxp), > 853 &(priv->aplist.ap > priv->scan_ind_count - 1)); 854 priv->aplist.size = priv->scan_ind_count; 855 } else { 856 DPRINTK(4, " count over :: scan_ind_count=%d\n", 857 priv->scan_ind_count); 858 } 859 } 860 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: rtl8712: make unsigned length for rtl8717_get{_wpa_,_wpa2_,_}ie
On Fri, 2018-02-16 at 15:48 +0100, Greg KH wrote: > Can you rebase both of these patches on the staging-testing branch of > the staging.git tree so that I can apply them? Right now they have > too > many conflicts. Rebased on staging-testing d92a1fa. v2 to come. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/2] staging: rtl8712: fix signedness of length to rtl8717_set_ie
rtl8717_set_it() takes an unsigned int pointer as length, fixed signedness in code using it. Sparse warnings: drivers/staging/rtl8712/ieee80211.c:191:53: warning: incorrect type in argument 5 (different signedness) drivers/staging/rtl8712/ieee80211.c:191:53:expected unsigned int [usertype] *frlen drivers/staging/rtl8712/ieee80211.c:191:53:got int * drivers/staging/rtl8712/ieee80211.c:197:57: warning: incorrect type in argument 5 (different signedness) drivers/staging/rtl8712/ieee80211.c:197:57:expected unsigned int [usertype] *frlen drivers/staging/rtl8712/ieee80211.c:197:57:got int * drivers/staging/rtl8712/ieee80211.c:199:63: warning: incorrect type in argument 5 (different signedness) drivers/staging/rtl8712/ieee80211.c:199:63:expected unsigned int [usertype] *frlen drivers/staging/rtl8712/ieee80211.c:199:63:got int * drivers/staging/rtl8712/ieee80211.c:202:67: warning: incorrect type in argument 5 (different signedness) drivers/staging/rtl8712/ieee80211.c:202:67:expected unsigned int [usertype] *frlen drivers/staging/rtl8712/ieee80211.c:202:67:got int * drivers/staging/rtl8712/ieee80211.c:206:73: warning: incorrect type in argument 5 (different signedness) drivers/staging/rtl8712/ieee80211.c:206:73:expected unsigned int [usertype] *frlen drivers/staging/rtl8712/ieee80211.c:206:73:got int * drivers/staging/rtl8712/ieee80211.c:209:75: warning: incorrect type in argument 5 (different signedness) drivers/staging/rtl8712/ieee80211.c:209:75:expected unsigned int [usertype] *frlen drivers/staging/rtl8712/ieee80211.c:209:75:got int * Signed-off-by: Stefano Manni --- drivers/staging/rtl8712/ieee80211.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/rtl8712/ieee80211.c b/drivers/staging/rtl8712/ieee80211.c index 612ccfe..7a4c00e 100644 --- a/drivers/staging/rtl8712/ieee80211.c +++ b/drivers/staging/rtl8712/ieee80211.c @@ -166,7 +166,8 @@ static uint r8712_get_rateset_len(u8 *rateset) int r8712_generate_ie(struct registry_priv *pregistrypriv) { - int sz = 0, rate_len; + int rate_len; + uint sz = 0; struct wlan_bssid_ex *pdev_network = &pregistrypriv->dev_network; u8 *ie = pdev_network->IEs; u16 beaconPeriod = (u16)pdev_network->Configuration.BeaconPeriod; -- 2.9.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/2] staging: rtl8712: make unsigned length for rtl8717_get{_wpa_, _wpa2_, _}ie
Fixed r8712_get_ie, r8712_get_wpa_ie, r8712_get_wpa2_ie to have a length as unsigned int pointer instead of signed. Sparse warnings: drivers/staging/rtl8712/rtl871x_ioctl_linux.c:173:27: warning: incorrect type in argument 3 (different signedness) drivers/staging/rtl8712/rtl871x_ioctl_linux.c:173:27:expected signed int *len drivers/staging/rtl8712/rtl871x_ioctl_linux.c:173:27:got unsigned int * drivers/staging/rtl8712/rtl871x_ioctl_linux.c:613:35: warning: incorrect type in argument 3 (different signedness) drivers/staging/rtl8712/rtl871x_ioctl_linux.c:613:35:expected signed int *len drivers/staging/rtl8712/rtl871x_ioctl_linux.c:613:35:got unsigned int * drivers/staging/rtl8712/rtl871x_ioctl_linux.c:1411:67: warning: incorrect type in argument 3 (different signedness) drivers/staging/rtl8712/rtl871x_ioctl_linux.c:1411:67:expected signed int *len drivers/staging/rtl8712/rtl871x_ioctl_linux.c:1411:67:got unsigned int * drivers/staging/rtl8712/rtl871x_ioctl_linux.c:1992:33: warning: incorrect type in argument 2 (different signedness) drivers/staging/rtl8712/rtl871x_ioctl_linux.c:1992:33:expected int *rsn_ie_len drivers/staging/rtl8712/rtl871x_ioctl_linux.c:1992:33:got unsigned int * drivers/staging/rtl8712/rtl871x_ioctl_linux.c:1998:33: warning: incorrect type in argument 2 (different signedness) drivers/staging/rtl8712/rtl871x_ioctl_linux.c:1998:33:expected int *rsn_ie_len drivers/staging/rtl8712/rtl871x_ioctl_linux.c:1998:33:got unsigned int * drivers/staging/rtl8712/rtl871x_mlme.c:1701:59: warning: incorrect type in argument 3 (different signedness) drivers/staging/rtl8712/rtl871x_mlme.c:1701:59:expected signed int *len drivers/staging/rtl8712/rtl871x_mlme.c:1701:59:got unsigned int * Signed-off-by: Stefano Manni --- drivers/staging/rtl8712/ieee80211.c| 8 drivers/staging/rtl8712/ieee80211.h| 6 +++--- drivers/staging/rtl8712/rtl871x_mlme.c | 3 ++- drivers/staging/rtl8712/rtl871x_xmit.c | 2 +- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/staging/rtl8712/ieee80211.c b/drivers/staging/rtl8712/ieee80211.c index 9872703..612ccfe 100644 --- a/drivers/staging/rtl8712/ieee80211.c +++ b/drivers/staging/rtl8712/ieee80211.c @@ -107,7 +107,7 @@ u8 *r8712_set_ie(u8 *pbuf, sint index, uint len, u8 *source, uint *frlen) * index: the information element id index, limit is the limit for search * --- */ -u8 *r8712_get_ie(u8 *pbuf, sint index, sint *len, sint limit) +u8 *r8712_get_ie(u8 *pbuf, sint index, uint *len, sint limit) { sint tmp, i; u8 *p; @@ -211,9 +211,9 @@ int r8712_generate_ie(struct registry_priv *pregistrypriv) return sz; } -unsigned char *r8712_get_wpa_ie(unsigned char *pie, int *wpa_ie_len, int limit) +unsigned char *r8712_get_wpa_ie(unsigned char *pie, uint *wpa_ie_len, int limit) { - int len; + u32 len; u16 val16; unsigned char wpa_oui_type[] = {0x00, 0x50, 0xf2, 0x01}; u8 *pbuf = pie; @@ -245,7 +245,7 @@ unsigned char *r8712_get_wpa_ie(unsigned char *pie, int *wpa_ie_len, int limit) return NULL; } -unsigned char *r8712_get_wpa2_ie(unsigned char *pie, int *rsn_ie_len, int limit) +unsigned char *r8712_get_wpa2_ie(unsigned char *pie, uint *rsn_ie_len, int limit) { return r8712_get_ie(pie, _WPA2_IE_ID_, rsn_ie_len, limit); } diff --git a/drivers/staging/rtl8712/ieee80211.h b/drivers/staging/rtl8712/ieee80211.h index 68fd65e..d605dfd 100644 --- a/drivers/staging/rtl8712/ieee80211.h +++ b/drivers/staging/rtl8712/ieee80211.h @@ -738,9 +738,9 @@ static inline int ieee80211_get_hdrlen(u16 fc) struct registry_priv; u8 *r8712_set_ie(u8 *pbuf, sint index, uint len, u8 *source, uint *frlen); -u8 *r8712_get_ie(u8 *pbuf, sint index, sint *len, sint limit); -unsigned char *r8712_get_wpa_ie(unsigned char *pie, int *rsn_ie_len, int limit); -unsigned char *r8712_get_wpa2_ie(unsigned char *pie, int *rsn_ie_len, +u8 *r8712_get_ie(u8 *pbuf, sint index, uint *len, sint limit); +unsigned char *r8712_get_wpa_ie(unsigned char *pie, uint *rsn_ie_len, int limit); +unsigned char *r8712_get_wpa2_ie(unsigned char *pie, uint *rsn_ie_len, int limit); int r8712_parse_wpa_ie(u8 *wpa_ie, int wpa_ie_len, int *group_cipher, int *pairwise_cipher); diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c index 7824508..ac547dd 100644 --- a/drivers/staging/rtl8712/rtl871x_mlme.c +++ b/drivers/staging/rtl8712/rtl871x_mlme.c @@ -1725,7 +1725,8 @@ unsigned int r8712_restructure_ht_ie(struct _adapter *padapter, u8 *in_ie, static void update_ht_cap(struct _adapter *padapter, u8 *pie, uint ie_len) { u8 *p, max_ampdu_sz; - int i, len; + int i; + uint len; struct sta_info *bmc_sta, *psta; struct ieee80211_ht_cap *pht_capie;
[PATCH 1/3] Staging: gdm724x: tty: Remove unnecessary macro 'gdm_tty_send'.
Remove the macro 'gdm_tty_send' which adds unnecessary complexity and has arguments that could mistakenly be evaluated multiple times. Signed-off-by: Quytelda Kahja --- drivers/staging/gdm724x/gdm_tty.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c index fc7682c18f20..1c3853bcfac2 100644 --- a/drivers/staging/gdm724x/gdm_tty.c +++ b/drivers/staging/gdm724x/gdm_tty.c @@ -37,8 +37,6 @@ #define MUX_TX_MAX_SIZE 2048 -#define gdm_tty_send(n, d, l, i, c, b) (\ - n->tty_dev->send_func(n->tty_dev->priv_dev, d, l, i, c, b)) #define gdm_tty_recv(n, c) (\ n->tty_dev->recv_func(n->tty_dev->priv_dev, c)) #define gdm_tty_send_control(n, r, v, d, l) (\ @@ -191,13 +189,12 @@ static int gdm_tty_write(struct tty_struct *tty, const unsigned char *buf, while (1) { sending_len = min(MUX_TX_MAX_SIZE, remain); - gdm_tty_send(gdm, -(void *)(buf + sent_len), -sending_len, -gdm->index, -gdm_tty_send_complete, -gdm - ); + gdm->tty_dev->send_func(gdm->tty_dev->priv_dev, + (void *)(buf + sent_len), + sending_len, + gdm->index, + gdm_tty_send_complete, + gdm); sent_len += sending_len; remain -= sending_len; if (remain <= 0) -- 2.16.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] Staging: gdm724x: tty: Remove unused macro 'gdm_tty_send_control'.
Remove the macro 'gdm_tty_send_control' which adds unnecessary complexity, is unused, and has arguments that could mistakenly be evaluated multiple times. Signed-off-by: Quytelda Kahja --- drivers/staging/gdm724x/gdm_tty.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c index 25357dc3d88c..3cdebb81ba63 100644 --- a/drivers/staging/gdm724x/gdm_tty.c +++ b/drivers/staging/gdm724x/gdm_tty.c @@ -37,9 +37,6 @@ #define MUX_TX_MAX_SIZE 2048 -#define gdm_tty_send_control(n, r, v, d, l) (\ - n->tty_dev->send_control(n->tty_dev->priv_dev, r, v, d, l)) - #define GDM_TTY_READY(gdm) (gdm && gdm->tty_dev && gdm->port.count) static struct tty_driver *gdm_driver[TTY_MAX_COUNT]; -- 2.16.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] Staging: gdm724x: tty: Remove unnecessary macro 'gdm_tty_recv'.
Remove the macro 'gdm_tty_recv' which adds unnecessary complexity and has arguments that could mistakenly be evaluated multiple times. Signed-off-by: Quytelda Kahja --- drivers/staging/gdm724x/gdm_tty.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c index 1c3853bcfac2..25357dc3d88c 100644 --- a/drivers/staging/gdm724x/gdm_tty.c +++ b/drivers/staging/gdm724x/gdm_tty.c @@ -37,8 +37,6 @@ #define MUX_TX_MAX_SIZE 2048 -#define gdm_tty_recv(n, c) (\ - n->tty_dev->recv_func(n->tty_dev->priv_dev, c)) #define gdm_tty_send_control(n, r, v, d, l) (\ n->tty_dev->send_control(n->tty_dev->priv_dev, r, v, d, l)) @@ -144,7 +142,8 @@ static int gdm_tty_recv_complete(void *data, if (!GDM_TTY_READY(gdm)) { if (complete == RECV_PACKET_PROCESS_COMPLETE) - gdm_tty_recv(gdm, gdm_tty_recv_complete); + gdm->tty_dev->recv_func(gdm->tty_dev->priv_dev, + gdm_tty_recv_complete); return TO_HOST_PORT_CLOSE; } @@ -158,7 +157,8 @@ static int gdm_tty_recv_complete(void *data, } if (complete == RECV_PACKET_PROCESS_COMPLETE) - gdm_tty_recv(gdm, gdm_tty_recv_complete); + gdm->tty_dev->recv_func(gdm->tty_dev->priv_dev, + gdm_tty_recv_complete); return 0; } @@ -253,7 +253,8 @@ int register_lte_tty_device(struct tty_dev *tty_dev, struct device *device) } for (i = 0; i < MAX_ISSUE_NUM; i++) - gdm_tty_recv(gdm, gdm_tty_recv_complete); + gdm->tty_dev->recv_func(gdm->tty_dev->priv_dev, + gdm_tty_recv_complete); return 0; } -- 2.16.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
I have been trying to reach you
My name is Gavin, from the US. I'm in Syria right now fighting IS. I want to get to know you better, if I may be so bold. I consider myself an easy-going man, and I am currently looking for a relationship in which I feel loved. Please tell me more about yourself, if you don't mind. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: bcm2048: Fix function argument alignment in radio-bcm2048.c.
Fix a coding style problem. Signed-off-by: Quytelda Kahja --- drivers/staging/media/bcm2048/radio-bcm2048.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c b/drivers/staging/media/bcm2048/radio-bcm2048.c index 06d1920150da..94141a11e51b 100644 --- a/drivers/staging/media/bcm2048/radio-bcm2048.c +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c @@ -1846,6 +1846,7 @@ static int bcm2048_deinit(struct bcm2048_device *bdev) static int bcm2048_probe(struct bcm2048_device *bdev) { int err; + u8 default_threshold = BCM2048_DEFAULT_RSSI_THRESHOLD; err = bcm2048_set_power_state(bdev, BCM2048_POWER_ON); if (err < 0) @@ -1863,8 +1864,7 @@ static int bcm2048_probe(struct bcm2048_device *bdev) if (err < 0) goto unlock; - err = bcm2048_set_fm_search_rssi_threshold(bdev, - BCM2048_DEFAULT_RSSI_THRESHOLD); + err = bcm2048_set_fm_search_rssi_threshold(bdev, default_threshold); if (err < 0) goto unlock; @@ -1942,9 +1942,9 @@ static irqreturn_t bcm2048_handler(int irq, void *dev) */ #define property_write(prop, type, mask, check) \ static ssize_t bcm2048_##prop##_write(struct device *dev, \ - struct device_attribute *attr, \ - const char *buf,\ - size_t count) \ + struct device_attribute *attr,\ + const char *buf, \ + size_t count) \ { \ struct bcm2048_device *bdev = dev_get_drvdata(dev); \ type value; \ @@ -1966,8 +1966,8 @@ static ssize_t bcm2048_##prop##_write(struct device *dev, \ #define property_read(prop, mask) \ static ssize_t bcm2048_##prop##_read(struct device *dev, \ - struct device_attribute *attr, \ - char *buf) \ +struct device_attribute *attr, \ +char *buf) \ { \ struct bcm2048_device *bdev = dev_get_drvdata(dev); \ int value; \ @@ -1985,8 +1985,8 @@ static ssize_t bcm2048_##prop##_read(struct device *dev, \ #define property_signed_read(prop, size, mask) \ static ssize_t bcm2048_##prop##_read(struct device *dev, \ - struct device_attribute *attr, \ - char *buf) \ +struct device_attribute *attr, \ +char *buf) \ { \ struct bcm2048_device *bdev = dev_get_drvdata(dev); \ size value; \ @@ -2005,8 +2005,8 @@ property_read(prop, mask) \ #define property_str_read(prop, size) \ static ssize_t bcm2048_##prop##_read(struct device *dev, \ - struct device_attribute *attr, \ - char *buf) \ +struct device_attribute *attr, \ +char *buf) \ { \ struct bcm2048_device *bdev = dev_get_drvdata(dev); \ int count; \ @@ -2175,7 +2175,7 @@ static int bcm2048_fops_release(struct file *file) } static __poll_t bcm2048_fops_poll(struct file *file, - struct poll_table_struct *pts) + struct poll_table_struct *pts) { struct bcm2048_device *bdev = video_drvdata(file); __poll_t retval = 0; -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] staging: wilc1000: fix line over 80 characters in wilc_spi_init()
On Fri, 16 Feb 2018 15:30:22 +0100 Greg KH wrote: > > diff --git a/drivers/staging/wilc1000/wilc_spi.c > > b/drivers/staging/wilc1000/wilc_spi.c > > index 30a9a61..fddc0db 100644 > > --- a/drivers/staging/wilc1000/wilc_spi.c > > +++ b/drivers/staging/wilc1000/wilc_spi.c > > @@ -835,7 +835,6 @@ static int wilc_spi_init(struct wilc *wilc, bool resume) > > struct spi_device *spi = to_spi_device(wilc->dev); > > u32 reg; > > u32 chipid; > > - > > static int isinit; > > You also deleted this line :( The blank line was not necessary between the variable's declaration. I should have done these changes in separate patch. I will take care to have these type of changes either in a separate patch or include the change details in description. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: bcm2048: Fix function argument alignment in radio-bcm2048.c.
On Sun, Feb 18, 2018 at 04:44:46PM -0800, Quytelda Kahja wrote: > Fix a coding style problem. > > Signed-off-by: Quytelda Kahja > --- > drivers/staging/media/bcm2048/radio-bcm2048.c | 24 > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c > b/drivers/staging/media/bcm2048/radio-bcm2048.c > index 06d1920150da..94141a11e51b 100644 > --- a/drivers/staging/media/bcm2048/radio-bcm2048.c > +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c > @@ -1846,6 +1846,7 @@ static int bcm2048_deinit(struct bcm2048_device *bdev) > static int bcm2048_probe(struct bcm2048_device *bdev) > { > int err; > + u8 default_threshold = BCM2048_DEFAULT_RSSI_THRESHOLD; > > err = bcm2048_set_power_state(bdev, BCM2048_POWER_ON); > if (err < 0) > @@ -1863,8 +1864,7 @@ static int bcm2048_probe(struct bcm2048_device *bdev) > if (err < 0) > goto unlock; > > - err = bcm2048_set_fm_search_rssi_threshold(bdev, > - BCM2048_DEFAULT_RSSI_THRESHOLD); > + err = bcm2048_set_fm_search_rssi_threshold(bdev, default_threshold); Nah. Don't do this. There were some of your earlier patches where I thought: gdm->tty_dev->send_func(... Could be shortened to: tty->send_func(... So sometimes introducing shorter aliases is the right thing. But here it just makes it look like a variable when it's a constant. It's makes the code slightly less readable. Just ignore the warning. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] staging: wilc1000: fix line over 80 chars in wilc_spi_clear_int_ext()
On Fri, 16 Feb 2018 20:16:02 +0200 Claudiu Beznea wrote: > Or you could use: > unsigned long expected_irqs, unexpected_irqs; > > expected_irqs = val & GENMASK(g_spi.int - 1, 0); > unexpected_irq = val & GENMASK(MAX_NUM_INT - 1, g_spi.int); > > for (i = 0; i < g_spi.nint && expected_irqs; i++) { > if (expected_irqs & BIT(i)) { > ret = wilc_spi_write_reg(wilc, 0x10c8 + i * 4, 1); > if (ret) { > dev_err(...); > goto _fail_; > } > } > } > > for (i = g_spi.nint; i < MAX_NUM_INT && unexpected_irq; i++) { > if (unexpected_irqs & BIT(i)) > dev_err(...); > Thanks for suggestion. I will take this input and make use of GENMASK macro to modify the function. In a separate patch will submit these changes. As there are other functions,where same macro can be used so will include them together in separate patch. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel