Re: [Outreachy kernel] Re: [PATCH] staging: irda: Replace printk() with appropriate net_*macro_ratelimited()

[SIMRAN SINGHAL]

On Tue, Mar 6, 2018 at 2:33 AM, Julia Lawall  wrote:
>
>
> On Mon, 5 Mar 2018, Arushi Singhal wrote:
>
>>
>>
>> On Mon, Mar 5, 2018 at 3:33 PM, Dan Carpenter  
>> wrote:
>>   On Mon, Mar 05, 2018 at 04:02:06AM +0530, Arushi Singhal wrote:
>>   > Replace printk having a log level with the appropriate
>>   > net_*macro_ratelimited.
>>   > It's better to use actual device name as a prefix in error messages.
>>   > Indentation is also changed, to fix the  checkpatch issue if line is 
>> not
>>   > exceding 80 characters.
>>   >
>>   > Signed-off-by: Arushi Singhal 
>>   > ---
>>   >  .../rtl8192u/ieee80211/ieee80211_crypt_ccmp.c  | 22 
>> +++---
>>   >  1 file changed, 11 insertions(+), 11 deletions(-)
>>   >
>>   > diff --git 
>> a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_ccmp.c 
>> b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_ccmp.c
>>   > index e6648f7..200fe5f 100644
>>   > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_ccmp.c
>>   > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_ccmp.c
>>   > @@ -73,7 +73,7 @@ static void *ieee80211_ccmp_init(int key_idx)
>>   >
>>   >   priv->tfm = (void *)crypto_alloc_cipher("aes", 0, 
>> CRYPTO_ALG_ASYNC);
>>   >   if (IS_ERR(priv->tfm)) {
>>   > - printk(KERN_DEBUG "ieee80211_crypt_ccmp: could not 
>> allocate crypto API aes\n");
>>   > + net_dbg_ratelimited("ieee80211_crypt_ccmp: could not 
>> allocate crypto API aes\n");
>>
>>
>>   This should probably just be deleted.
>>
>> Hello Dan
>> Why we should make this change?
>> According to me it's use here to print debug message.
>> Wanted to know more about your thought on this.
>>
>>   >   priv->tfm = NULL;
>>   >   goto fail;
>>   >   }
>>   > @@ -276,22 +276,22 @@ static int ieee80211_ccmp_decrypt(struct 
>> sk_buff *skb, int hdr_len, void *priv)
>>   >   keyidx = pos[3];
>>   >   if (!(keyidx & (1 << 5))) {
>>   >   if (net_ratelimit()) {
>>   > - printk(KERN_DEBUG "CCMP: received packet 
>> without ExtIV flag from %pM\n",
>>   > - hdr->addr2);
>>   > + net_dbg_ratelimited("CCMP: received packet 
>> without ExtIV flag from %pM\n",
>>   > + hdr->addr2);
>>   >   }
>>
>>   The "if (net_ratelimit()) " already means the message is ratelimited so
>>   it's net_dbg() ok?  I think so, but I've never looked at this before.
>>
>>
>> net_dbg() is an implicit declaration of a function. So we can't do this 
>> change.
>
> Perhaps you can add the header file that declares net_dbg.

Hi Julia

There is such header file which declares net_dbg.
We can use dev_dbg() for all struct device object.
We can use netdev_dbg() for all struct netdevice object.
For net subsystem when there is no struct device object we can go with
net_dbg_ratelimited (preferred) or pr_debug.

I have understood this from my readings. If I am wrong somewhere
please correct me.

Thanks
Arushi

>
> julia
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/alpine.DEB.2.20.1803052203001.2763%40hadrien.
> For more options, visit https://groups.google.com/d/optout.

[PATCH] staging: rtl8192u: ieee80211: Convert printks to pr_

[simran singhal]

Use the current logging style.
Coalesce formats where appropriate.

Signed-off-by: simran singhal 
---
 drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c | 34 ++-
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c 
b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
index 7a31510..d1a86bb 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
@@ -236,8 +236,8 @@ ieee80211_rx_frame_mgmt(struct ieee80211_device *ieee, 
struct sk_buff *skb,
 
#ifdef NOT_YET
if (ieee->iw_mode == IW_MODE_MASTER) {
-   printk(KERN_DEBUG "%s: Master mode not yet supported.\n",
-  ieee->dev->name);
+   pr_debug("%s: Master mode not yet supported.\n",
+ieee->dev->name);
return 0;
 /*
   hostap_update_sta_ps(ieee, (struct hostap_ieee80211_hdr_4addr *)
@@ -265,9 +265,8 @@ ieee80211_rx_frame_mgmt(struct ieee80211_device *ieee, 
struct sk_buff *skb,
 
if (ieee->iw_mode == IW_MODE_MASTER) {
if (type != WLAN_FC_TYPE_MGMT && type != WLAN_FC_TYPE_CTRL) {
-   printk(KERN_DEBUG "%s: unknown management frame "
-  "(type=0x%02x, stype=0x%02x) dropped\n",
-  skb->dev->name, type, stype);
+   pr_debug("%s: unknown management frame (type=0x%02x, 
stype=0x%02x) dropped\n",
+skb->dev->name, type, stype);
return -1;
}
 
@@ -275,8 +274,8 @@ ieee80211_rx_frame_mgmt(struct ieee80211_device *ieee, 
struct sk_buff *skb,
return 0;
}
 
-   printk(KERN_DEBUG "%s: hostap_rx_frame_mgmt: management frame "
-  "received in non-Host AP mode\n", skb->dev->name);
+   pr_debug("%s: hostap_rx_frame_mgmt: management frame received in 
non-Host AP mode\n",
+skb->dev->name);
return -1;
#endif
 }
@@ -354,9 +353,8 @@ ieee80211_rx_frame_decrypt(struct ieee80211_device *ieee, 
struct sk_buff *skb,
if (ieee->tkip_countermeasures &&
strcmp(crypt->ops->name, "TKIP") == 0) {
if (net_ratelimit()) {
-   printk(KERN_DEBUG "%s: TKIP countermeasures: dropped "
-  "received packet from %pM\n",
-  ieee->dev->name, hdr->addr2);
+   pr_debug("%s: TKIP countermeasures: dropped received 
packet from %pM\n",
+ieee->dev->name, hdr->addr2);
}
return -1;
}
@@ -403,9 +401,8 @@ ieee80211_rx_frame_decrypt_msdu(struct ieee80211_device 
*ieee, struct sk_buff *s
res = crypt->ops->decrypt_msdu(skb, keyidx, hdrlen, crypt->priv);
atomic_dec(&crypt->refcnt);
if (res < 0) {
-   printk(KERN_DEBUG "%s: MSDU decryption/MIC verification failed"
-  " (SA=%pM keyidx=%d)\n",
-  ieee->dev->name, hdr->addr2, keyidx);
+   pr_debug("%s: MSDU decryption/MIC verification failed (SA=%pM 
keyidx=%d)\n",
+ieee->dev->name, hdr->addr2, keyidx);
return -1;
}
 
@@ -923,8 +920,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct 
sk_buff *skb,
stats = &ieee->stats;
 
if (skb->len < 10) {
-   printk(KERN_INFO "%s: SKB length < 10\n",
-  dev->name);
+   pr_info("%s: SKB length < 10\n", dev->name);
goto rx_dropped;
}
 
@@ -1170,9 +1166,8 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct 
sk_buff *skb,
flen -= hdrlen;
 
if (frag_skb->tail + flen > frag_skb->end) {
-   printk(KERN_WARNING "%s: host decrypted and "
-  "reassembled frame did not fit skb\n",
-  dev->name);
+   pr_warn("%s: host decrypted and reassembled frame did 
not fit skb\n",
+   dev->name);
ieee80211_frag_cache_invalidate(ieee, hdr);
goto rx_dropped;
}
@@ -2049,8 +2044,7 @@ int ieee80211_parse_info_param(struct ieee80211_device 
*ieee,
}
break;
case MFIE_TYPE_QOS_PARAMETER:
-   printk(KERN_ERR
-  "QoS Error need to parse QOS_PARAMETER IE\n");
+   pr_err("QoS Error need to parse QOS_PARAMETER IE\n");
break;
 
case MFIE_TYPE_COUNTRY:
-- 
2.7.4

[PATCH] staging: lustre: lnet: selftest: Change the type of variable to bool

[simran singhal]

This patch changes the type of variable done from int to boolean. As it
is been used as a boolean in the function sfw_test_rpc_done(). It also
makes the code more readable and bool data type also requires less
memory in comparison to int data type.

Signed-off-by: simran singhal 
---
 drivers/staging/lustre/lnet/selftest/framework.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/selftest/framework.c 
b/drivers/staging/lustre/lnet/selftest/framework.c
index ef27bff..3789df6 100644
--- a/drivers/staging/lustre/lnet/selftest/framework.c
+++ b/drivers/staging/lustre/lnet/selftest/framework.c
@@ -869,7 +869,7 @@ sfw_test_rpc_done(struct srpc_client_rpc *rpc)
 {
struct sfw_test_unit *tsu = rpc->crpc_priv;
struct sfw_test_instance *tsi = tsu->tsu_instance;
-   int done = 0;
+   bool done = false;
 
tsi->tsi_ops->tso_done_rpc(tsu, rpc);
 
@@ -883,7 +883,7 @@ sfw_test_rpc_done(struct srpc_client_rpc *rpc)
/* batch is stopping or loop is done or get error */
if (tsi->tsi_stopping || !tsu->tsu_loop ||
(rpc->crpc_status && tsi->tsi_stoptsu_onerr))
-   done = 1;
+   done = true;
 
/* dec ref for poster */
srpc_client_rpc_decref(rpc);
-- 
2.7.4

[PATCH] staging: rtl8723bs: hal: Use (true/false) in assignment to bool

[simran singhal]

This patch assigns (true/false) to boolean EDCCA_State instead of (1/0).
And, there is no need of comparing EDCCA_State explicitly with constant
1.

Signed-off-by: simran singhal 
---
 drivers/staging/rtl8723bs/hal/odm_DIG.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/odm_DIG.c 
b/drivers/staging/rtl8723bs/hal/odm_DIG.c
index 202c52f..0bde944 100644
--- a/drivers/staging/rtl8723bs/hal/odm_DIG.c
+++ b/drivers/staging/rtl8723bs/hal/odm_DIG.c
@@ -278,11 +278,11 @@ void odm_Adaptivity(void *pDM_VOID, u8 IGI)
 
if (!pDM_Odm->ForceEDCCA) {
if (pDM_Odm->RSSI_Min > pDM_Odm->AdapEn_RSSI)
-   EDCCA_State = 1;
+   EDCCA_State = true;
else if (pDM_Odm->RSSI_Min < (pDM_Odm->AdapEn_RSSI - 5))
-   EDCCA_State = 0;
+   EDCCA_State = false;
} else
-   EDCCA_State = 1;
+   EDCCA_State = true;
 
if (
pDM_Odm->bLinked &&
@@ -305,7 +305,7 @@ void odm_Adaptivity(void *pDM_VOID, u8 IGI)
)
);
 
-   if (EDCCA_State == 1) {
+   if (EDCCA_State) {
Diff = IGI_target-(s8)IGI;
TH_L2H_dmc = pDM_Odm->TH_L2H_ini + Diff;
if (TH_L2H_dmc > 10)
-- 
2.7.4

[PATCH] staging: sm750fb: Remove typedefs from _logical_chip_type_t and _clock_type_t

[Simran Singhal]

This patch removes typedefs from enum _logical_chip_type_t and enum
_clock_type_t and rename them to logical_chip_type_t and clock_type_t
respectively.

Fix checkpatch warning:
WARNING: do not add new typedefs

Signed-off-by: Simran Singhal 
---
 drivers/staging/sm750fb/ddk750_chip.c |  4 ++--
 drivers/staging/sm750fb/ddk750_chip.h | 14 ++
 drivers/staging/sm750fb/ddk750_mode.c |  2 +-
 drivers/staging/sm750fb/ddk750_mode.h |  2 +-
 drivers/staging/sm750fb/sm750_hw.c|  2 +-
 5 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_chip.c 
b/drivers/staging/sm750fb/ddk750_chip.c
index 944dd25..db4d2fc 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -7,9 +7,9 @@
 
 #define MHz(x) ((x) * 100)
 
-static logical_chip_type_t chip;
+static enum logical_chip_type_t chip;
 
-logical_chip_type_t sm750_get_chip_type(void)
+enum logical_chip_type_t sm750_get_chip_type(void)
 {
return chip;
 }
diff --git a/drivers/staging/sm750fb/ddk750_chip.h 
b/drivers/staging/sm750fb/ddk750_chip.h
index 2c7a9b9..df1f586 100644
--- a/drivers/staging/sm750fb/ddk750_chip.h
+++ b/drivers/staging/sm750fb/ddk750_chip.h
@@ -23,25 +23,23 @@ static inline void poke32(u32 data, u32 addr)
 }
 
 /* This is all the chips recognized by this library */
-typedef enum _logical_chip_type_t {
+enum logical_chip_type_t {
SM_UNKNOWN,
SM718,
SM750,
SM750LE,
-}
-logical_chip_type_t;
+};
 
-typedef enum _clock_type_t {
+enum clock_type_t {
MXCLK_PLL,
PRIMARY_PLL,
SECONDARY_PLL,
VGA0_PLL,
VGA1_PLL,
-}
-clock_type_t;
+};
 
 struct pll_value {
-   clock_type_t clockType;
+   enum clock_type_t clockType;
unsigned long inputFreq; /* Input clock frequency to the PLL */
 
/* Use this when clockType = PANEL_PLL */
@@ -93,7 +91,7 @@ struct initchip_param {
/* More initialization parameter can be added if needed */
 };
 
-logical_chip_type_t sm750_get_chip_type(void);
+enum logical_chip_type_t sm750_get_chip_type(void);
 void sm750_set_chip_type(unsigned short devId, u8 revId);
 unsigned int sm750_calc_pll_value(unsigned int request, struct  pll_value 
*pll);
 unsigned int sm750_format_pll_reg(struct pll_value *pPLL);
diff --git a/drivers/staging/sm750fb/ddk750_mode.c 
b/drivers/staging/sm750fb/ddk750_mode.c
index bb673e1..6242770 100644
--- a/drivers/staging/sm750fb/ddk750_mode.c
+++ b/drivers/staging/sm750fb/ddk750_mode.c
@@ -205,7 +205,7 @@ static int programModeRegisters(struct mode_parameter 
*pModeParam,
return ret;
 }
 
-int ddk750_setModeTiming(struct mode_parameter *parm, clock_type_t clock)
+int ddk750_setModeTiming(struct mode_parameter *parm, enum clock_type_t clock)
 {
struct pll_value pll;
unsigned int uiActualPixelClk;
diff --git a/drivers/staging/sm750fb/ddk750_mode.h 
b/drivers/staging/sm750fb/ddk750_mode.h
index d5eae36..78a3cc7 100644
--- a/drivers/staging/sm750fb/ddk750_mode.h
+++ b/drivers/staging/sm750fb/ddk750_mode.h
@@ -32,5 +32,5 @@ struct mode_parameter {
enum spolarity clock_phase_polarity;
 };
 
-int ddk750_setModeTiming(struct mode_parameter *parm, clock_type_t clock);
+int ddk750_setModeTiming(struct mode_parameter *parm, enum clock_type_t clock);
 #endif
diff --git a/drivers/staging/sm750fb/sm750_hw.c 
b/drivers/staging/sm750fb/sm750_hw.c
index baf1bbd..8836e324 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -253,7 +253,7 @@ int hw_sm750_crtc_setMode(struct lynxfb_crtc *crtc,
int ret, fmt;
u32 reg;
struct mode_parameter modparm;
-   clock_type_t clock;
+   enum clock_type_t clock;
struct sm750_dev *sm750_dev;
struct lynxfb_par *par;
 
-- 
2.7.4

[PATCH] iio: pressure: zpa2326: Remove unnecessary cast on void pointer

[simran singhal]

The following Coccinelle script was used to detect this:
@r@
expression x;
void* e;
type T;
identifier f;
@@
(
  *((T *)e)
|
  ((T *)x)[...]
|
  ((T*)x)->f
|
- (T*)
  e
)

Signed-off-by: simran singhal 
---
 drivers/iio/pressure/zpa2326.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/pressure/zpa2326.c b/drivers/iio/pressure/zpa2326.c
index c720c3a..e58a0ad 100644
--- a/drivers/iio/pressure/zpa2326.c
+++ b/drivers/iio/pressure/zpa2326.c
@@ -751,7 +751,7 @@ static void zpa2326_suspend(struct iio_dev *indio_dev)
  */
 static irqreturn_t zpa2326_handle_irq(int irq, void *data)
 {
-   struct iio_dev *indio_dev = (struct iio_dev *)data;
+   struct iio_dev *indio_dev = data;
 
if (iio_buffer_enabled(indio_dev)) {
/* Timestamping needed for buffered sampling only. */
@@ -790,7 +790,7 @@ static irqreturn_t zpa2326_handle_irq(int irq, void *data)
  */
 static irqreturn_t zpa2326_handle_threaded_irq(int irq, void *data)
 {
-   struct iio_dev *indio_dev = (struct iio_dev *)data;
+   struct iio_dev *indio_dev = data;
struct zpa2326_private *priv = iio_priv(indio_dev);
unsigned intval;
boolcont;
-- 
2.7.4

[PATCH] iio: imu: st_lsm6dsx: Remove unnecessary cast on void pointer

[simran singhal]

The following Coccinelle script was used to detect this:
@r@
expression x;
void* e;
type T;
identifier f;
@@
(
  *((T *)e)
|
  ((T *)x)[...]
|
  ((T*)x)->f
|
- (T*)
  e
)

Signed-off-by: simran singhal 
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c 
b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index e11653d..f570b48 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -364,7 +364,7 @@ static int st_lsm6dsx_update_fifo(struct iio_dev *iio_dev, 
bool enable)
 
 static irqreturn_t st_lsm6dsx_handler_irq(int irq, void *private)
 {
-   struct st_lsm6dsx_hw *hw = (struct st_lsm6dsx_hw *)private;
+   struct st_lsm6dsx_hw *hw = private;
struct st_lsm6dsx_sensor *sensor;
int i;
 
@@ -388,7 +388,7 @@ static irqreturn_t st_lsm6dsx_handler_irq(int irq, void 
*private)
 
 static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private)
 {
-   struct st_lsm6dsx_hw *hw = (struct st_lsm6dsx_hw *)private;
+   struct st_lsm6dsx_hw *hw = private;
int count;
 
mutex_lock(&hw->fifo_lock);
-- 
2.7.4

[PATCH] iio: humidity: hts221_buffer: Remove unnecessary cast on void pointer

[simran singhal]

The following Coccinelle script was used to detect this:
@r@
expression x;
void* e;
type T;
identifier f;
@@
(
  *((T *)e)
|
  ((T *)x)[...]
|
  ((T*)x)->f
|
- (T*)
  e
)

Signed-off-by: simran singhal 
---
 drivers/iio/humidity/hts221_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/humidity/hts221_buffer.c 
b/drivers/iio/humidity/hts221_buffer.c
index 72ddcda..7d19a3d 100644
--- a/drivers/iio/humidity/hts221_buffer.c
+++ b/drivers/iio/humidity/hts221_buffer.c
@@ -41,7 +41,7 @@ static const struct iio_trigger_ops hts221_trigger_ops = {
 
 static irqreturn_t hts221_trigger_handler_thread(int irq, void *private)
 {
-   struct hts221_hw *hw = (struct hts221_hw *)private;
+   struct hts221_hw *hw = private;
u8 status;
int err;
 
-- 
2.7.4

[PATCH] iio: dac: ad5504: Remove unnecessary cast on void pointer

[simran singhal]

The following Coccinelle script was used to detect this:
@r@
expression x;
void* e;
type T;
identifier f;
@@
(
  *((T *)e)
|
  ((T *)x)[...]
|
  ((T*)x)->f
|
- (T*)
  e
)

Signed-off-by: simran singhal 
---
 drivers/iio/dac/ad5504.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index 22a027d..712d86b 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -223,7 +223,7 @@ static irqreturn_t ad5504_event_handler(int irq, void 
*private)
0,
IIO_EV_TYPE_THRESH,
IIO_EV_DIR_RISING),
-  iio_get_time_ns((struct iio_dev *)private));
+  iio_get_time_ns(private));
 
return IRQ_HANDLED;
 }
-- 
2.7.4

[PATCH] iio: common: ms_sensors: Remove unnecessary cast on void pointer

[simran singhal]

The following Coccinelle script was used to detect this:
@r@
expression x;
void* e;
type T;
identifier f;
@@
(
  *((T *)e)
|
  ((T *)x)[...]
|
  ((T*)x)->f
|
- (T*)
  e
)

Signed-off-by: simran singhal 
---
 drivers/iio/common/ms_sensors/ms_sensors_i2c.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/common/ms_sensors/ms_sensors_i2c.c 
b/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
index ecf7721..125b5ff 100644
--- a/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
+++ b/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
@@ -74,7 +74,7 @@ EXPORT_SYMBOL(ms_sensors_reset);
 int ms_sensors_read_prom_word(void *cli, int cmd, u16 *word)
 {
int ret;
-   struct i2c_client *client = (struct i2c_client *)cli;
+   struct i2c_client *client = cli;
 
ret = i2c_smbus_read_word_swapped(client, cmd);
if (ret < 0) {
@@ -107,7 +107,7 @@ int ms_sensors_convert_and_read(void *cli, u8 conv, u8 rd,
 {
int ret;
__be32 buf = 0;
-   struct i2c_client *client = (struct i2c_client *)cli;
+   struct i2c_client *client = cli;
 
/* Trigger conversion */
ret = i2c_smbus_write_byte(client, conv);
-- 
2.7.4

[PATCH] iio: adc: Remove unnecessary cast on void pointer

[simran singhal]

The following Coccinelle script was used to detect this:
@r@
expression x;
void* e;
type T;
identifier f;
@@
(
  *((T *)e)
|
  ((T *)x)[...]
|
  ((T*)x)->f
|
- (T*)
  e
)

Signed-off-by: simran singhal 
---
 drivers/iio/adc/exynos_adc.c  | 2 +-
 drivers/iio/adc/imx7d_adc.c   | 2 +-
 drivers/iio/adc/max1027.c | 2 +-
 drivers/iio/adc/rockchip_saradc.c | 2 +-
 drivers/iio/adc/sun4i-gpadc-iio.c | 2 +-
 drivers/iio/adc/vf610_adc.c   | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index ad1775b..6c5a7be 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -579,7 +579,7 @@ static int exynos_read_s3c64xx_ts(struct iio_dev 
*indio_dev, int *x, int *y)
 
 static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
 {
-   struct exynos_adc *info = (struct exynos_adc *)dev_id;
+   struct exynos_adc *info = dev_id;
u32 mask = info->data->mask;
 
/* Read value */
diff --git a/drivers/iio/adc/imx7d_adc.c b/drivers/iio/adc/imx7d_adc.c
index e2241ee..254b29a 100644
--- a/drivers/iio/adc/imx7d_adc.c
+++ b/drivers/iio/adc/imx7d_adc.c
@@ -365,7 +365,7 @@ static int imx7d_adc_read_data(struct imx7d_adc *info)
 
 static irqreturn_t imx7d_adc_isr(int irq, void *dev_id)
 {
-   struct imx7d_adc *info = (struct imx7d_adc *)dev_id;
+   struct imx7d_adc *info = dev_id;
int status;
 
status = readl(info->regs + IMX7D_REG_ADC_INT_STATUS);
diff --git a/drivers/iio/adc/max1027.c b/drivers/iio/adc/max1027.c
index 3b7c4f7..ebc7159 100644
--- a/drivers/iio/adc/max1027.c
+++ b/drivers/iio/adc/max1027.c
@@ -364,7 +364,7 @@ static int max1027_set_trigger_state(struct iio_trigger 
*trig, bool state)
 
 static irqreturn_t max1027_trigger_handler(int irq, void *private)
 {
-   struct iio_poll_func *pf = (struct iio_poll_func *)private;
+   struct iio_poll_func *pf = private;
struct iio_dev *indio_dev = pf->indio_dev;
struct max1027_state *st = iio_priv(indio_dev);
 
diff --git a/drivers/iio/adc/rockchip_saradc.c 
b/drivers/iio/adc/rockchip_saradc.c
index 85d7012..ae6d332 100644
--- a/drivers/iio/adc/rockchip_saradc.c
+++ b/drivers/iio/adc/rockchip_saradc.c
@@ -109,7 +109,7 @@ static int rockchip_saradc_read_raw(struct iio_dev 
*indio_dev,
 
 static irqreturn_t rockchip_saradc_isr(int irq, void *dev_id)
 {
-   struct rockchip_saradc *info = (struct rockchip_saradc *)dev_id;
+   struct rockchip_saradc *info = dev_id;
 
/* Read value */
info->last_val = readl_relaxed(info->regs + SARADC_DATA);
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c 
b/drivers/iio/adc/sun4i-gpadc-iio.c
index a8e134f..e531825 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -382,7 +382,7 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
 
 static int sun4i_gpadc_get_temp(void *data, int *temp)
 {
-   struct sun4i_gpadc_iio *info = (struct sun4i_gpadc_iio *)data;
+   struct sun4i_gpadc_iio *info = data;
int val, scale, offset;
 
if (sun4i_gpadc_temp_read(info->indio_dev, &val))
diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
index 228a003..01fc76f 100644
--- a/drivers/iio/adc/vf610_adc.c
+++ b/drivers/iio/adc/vf610_adc.c
@@ -584,7 +584,7 @@ static int vf610_adc_read_data(struct vf610_adc *info)
 
 static irqreturn_t vf610_adc_isr(int irq, void *dev_id)
 {
-   struct iio_dev *indio_dev = (struct iio_dev *)dev_id;
+   struct iio_dev *indio_dev = dev_id;
struct vf610_adc *info = iio_priv(indio_dev);
int coco;
 
-- 
2.7.4

[PATCH] net: ipv4: netfilter: Remove unused function nf_nat_need_gre()

[simran singhal]

The function nf_nat_need_gre() on being called, simply returns
back. The function doesn't have FIXME code around.
Hence, nf_nat_need_gre() and its calls have been removed.

Signed-off-by: simran singhal 
---
 net/ipv4/netfilter/nf_nat_pptp.c  | 2 --
 net/ipv4/netfilter/nf_nat_proto_gre.c | 6 --
 2 files changed, 8 deletions(-)

diff --git a/net/ipv4/netfilter/nf_nat_pptp.c b/net/ipv4/netfilter/nf_nat_pptp.c
index b3ca21b..747e737 100644
--- a/net/ipv4/netfilter/nf_nat_pptp.c
+++ b/net/ipv4/netfilter/nf_nat_pptp.c
@@ -282,8 +282,6 @@ pptp_inbound_pkt(struct sk_buff *skb,
 
 static int __init nf_nat_helper_pptp_init(void)
 {
-   nf_nat_need_gre();
-
BUG_ON(nf_nat_pptp_hook_outbound != NULL);
RCU_INIT_POINTER(nf_nat_pptp_hook_outbound, pptp_outbound_pkt);
 
diff --git a/net/ipv4/netfilter/nf_nat_proto_gre.c 
b/net/ipv4/netfilter/nf_nat_proto_gre.c
index edf0500..c020a4d 100644
--- a/net/ipv4/netfilter/nf_nat_proto_gre.c
+++ b/net/ipv4/netfilter/nf_nat_proto_gre.c
@@ -142,9 +142,3 @@ static void __exit nf_nat_proto_gre_fini(void)
 
 module_init(nf_nat_proto_gre_init);
 module_exit(nf_nat_proto_gre_fini);
-
-void nf_nat_need_gre(void)
-{
-   return;
-}
-EXPORT_SYMBOL_GPL(nf_nat_need_gre);
-- 
2.7.4

[PATCH] iio: proximity: as3935: constify attribute_group structures

[simran singhal]

Check for attribute_group structures that are only stored in the
attrs filed of iio_info structure. As the attrs field of iio_info
structures is constant, so these attribute_group structures can also be
declared constant.
Done using coccinelle:

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct attribute_group i@p = {...};

@ok1@
identifier r1.i;
position p;
struct iio_info x;
@@
x.attrs=&i@p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct attribute_group i={...};

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct attribute_group i;

File size before:
   textdata bss dec hex filename
   4037 288   0432510e5 drivers/iio/proximity/as3935.o

File size after:
   textdata bss dec hex filename
   4101 256   043571105 drivers/iio/proximity/as3935.o

Signed-off-by: simran singhal 
---
 drivers/iio/proximity/as3935.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/proximity/as3935.c b/drivers/iio/proximity/as3935.c
index 5656deb..1b8b472 100644
--- a/drivers/iio/proximity/as3935.c
+++ b/drivers/iio/proximity/as3935.c
@@ -155,7 +155,7 @@ static struct attribute *as3935_attributes[] = {
NULL,
 };
 
-static struct attribute_group as3935_attribute_group = {
+static const struct attribute_group as3935_attribute_group = {
.attrs = as3935_attributes,
 };
 
-- 
2.7.4

[PATCH] iio: light: bh1750: constify attribute_group structures

[simran singhal]

Check for attribute_group structures that are only stored in the
attrs filed of iio_info structure. As the attrs field of iio_info
structures is constant, so these attribute_group structures can also be
declared constant.
Done using coccinelle:

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct attribute_group i@p = {...};

@ok1@
identifier r1.i;
position p;
struct iio_info x;
@@
x.attrs=&i@p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct attribute_group i={...};

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct attribute_group i;

File size before:
   textdata bss dec hex filename
   2276 352   02628 a44 drivers/iio/light/bh1750.o

File size after:
   textdata bss dec hex filename
   2340 320   02660 a64 drivers/iio/light/bh1750.o

Signed-off-by: simran singhal 
---
 drivers/iio/light/bh1750.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/bh1750.c b/drivers/iio/light/bh1750.c
index b059466..6c61187 100644
--- a/drivers/iio/light/bh1750.c
+++ b/drivers/iio/light/bh1750.c
@@ -212,7 +212,7 @@ static struct attribute *bh1750_attributes[] = {
NULL,
 };
 
-static struct attribute_group bh1750_attribute_group = {
+static const struct attribute_group bh1750_attribute_group = {
.attrs = bh1750_attributes,
 };
 
-- 
2.7.4

[PATCH] iio: light: apds9960: constify attribute_group structures

[simran singhal]

Check for attribute_group structures that are only stored in the
attrs filed of iio_info structure. As the attrs field of iio_info
structures is constant, so these attribute_group structures can also be
declared constant.
Done using coccinelle:

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct attribute_group i@p = {...};

@ok1@
identifier r1.i;
position p;
struct iio_info x;
@@
x.attrs=&i@p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct attribute_group i={...};

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct attribute_group i;

File size before:
   textdata bss dec hex filename
   8503 488   08991231f drivers/iio/light/apds9960.o

File size after:
   textdata bss dec hex filename
   8567 424   08991231f drivers/iio/light/apds9960.o

Signed-off-by: simran singhal 
---
 drivers/iio/light/apds9960.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/apds9960.c b/drivers/iio/light/apds9960.c
index 90bc98d..c7af36d 100644
--- a/drivers/iio/light/apds9960.c
+++ b/drivers/iio/light/apds9960.c
@@ -343,7 +343,7 @@ static struct attribute *apds9960_attributes[] = {
NULL,
 };
 
-static struct attribute_group apds9960_attribute_group = {
+static const struct attribute_group apds9960_attribute_group = {
.attrs = apds9960_attributes,
 };
 
-- 
2.7.4

[PATCH] iio: humidity: hdc100x: constify attribute_group structures

[simran singhal]

Check for attribute_group structures that are only stored in the
attrs filed of iio_info structure. As the attrs field of iio_info
structures is constant, so these attribute_group structures can also be
declared constant.
Done using coccinelle:

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct attribute_group i@p = {...};

@ok1@
identifier r1.i;
position p;
struct iio_info x;
@@
x.attrs=&i@p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct attribute_group i={...};

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct attribute_group i;

File size before:
   textdata bss dec hex filename
   3459 488   03947 f6b drivers/iio/humidity/hdc100x.o

File size after:
   textdata bss dec hex filename
   3507 424   03931 f5b drivers/iio/humidity/hdc100x.o

Signed-off-by: simran singhal 
---
 drivers/iio/humidity/hdc100x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
index 265c34d..aa17115 100644
--- a/drivers/iio/humidity/hdc100x.c
+++ b/drivers/iio/humidity/hdc100x.c
@@ -79,7 +79,7 @@ static struct attribute *hdc100x_attributes[] = {
NULL
 };
 
-static struct attribute_group hdc100x_attribute_group = {
+static const struct attribute_group hdc100x_attribute_group = {
.attrs = hdc100x_attributes,
 };
 
-- 
2.7.4

[PATCH] iio: adc: hx711: constify attribute_group structures

[simran singhal]

Check for attribute_group structures that are only stored in the
attrs filed of iio_info structure. As the attrs field of iio_info
structures is constant, so these attribute_group structures can also be
declared constant.
Done using coccinelle:

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct attribute_group i@p = {...};

@ok1@
identifier r1.i;
position p;
struct iio_info x;
@@
x.attrs=&i@p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct attribute_group i={...};

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct attribute_group i;

File size before:
   textdata bss dec hex filename
   3042 480   03522 dc2 drivers/iio/adc/hx711.o

File size after:
   textdata bss dec hex filename
   3098 416   03514 dba drivers/iio/adc/hx711.o

Signed-off-by: simran singhal 
---
 drivers/iio/adc/hx711.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c
index 139639f..27005d8 100644
--- a/drivers/iio/adc/hx711.c
+++ b/drivers/iio/adc/hx711.c
@@ -369,7 +369,7 @@ static struct attribute *hx711_attributes[] = {
NULL,
 };
 
-static struct attribute_group hx711_attribute_group = {
+static const struct attribute_group hx711_attribute_group = {
.attrs = hx711_attributes,
 };
 
-- 
2.7.4

[PATCH] staging: iio: light: constify attribute_group structures

[simran singhal]

As the event_attrs field of iio_info structures is constant, so these
attribute_group structures can also be declared constant.

File size before:
   textdata bss dec hex filename
  150641528   0   1659240d0
drivers/staging/iio/light/tsl2x7x_core.o

File size after:
   textdata bss dec hex filename
  151921400   0   1659240d0
drivers/staging/iio/light/tsl2x7x_core.o

Signed-off-by: simran singhal 
---
 drivers/staging/iio/light/tsl2x7x_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/light/tsl2x7x_core.c 
b/drivers/staging/iio/light/tsl2x7x_core.c
index 0490c1d..9a0046a 100644
--- a/drivers/staging/iio/light/tsl2x7x_core.c
+++ b/drivers/staging/iio/light/tsl2x7x_core.c
@@ -1676,7 +1676,7 @@ static const struct attribute_group 
tsl2X7X_device_attr_group_tbl[] = {
},
 };
 
-static struct attribute_group tsl2X7X_event_attr_group_tbl[] = {
+static const struct attribute_group tsl2X7X_event_attr_group_tbl[] = {
[ALS] = {
.attrs = tsl2X7X_ALS_event_attrs,
.name = "events",
-- 
2.7.4

Re: [PATCH 1/3] iio: health: afe440x: Remove code in comments

[SIMRAN SINGHAL]

Please ignore this Patch. This Patch is wrong.

On Sat, Apr 1, 2017 at 12:20 AM, Joe Perches  wrote:
> On Sat, 2017-04-01 at 00:13 +0530, SIMRAN SINGHAL wrote:
>> On Sat, Apr 1, 2017 at 12:03 AM, Joe Perches  wrote:
>> > On Fri, 2017-03-31 at 22:16 +0530, simran singhal wrote:
>> > > Commenting out code is a bad idea.
>> > > As comments are for explaining what code is about.
>> >
>> > patch doesn't match commit message.
>>
>> In commit message I am clearly saying commenting out the code is a bad Idea.
>> As comments are for explaining what code is about.
>> And that's what I am doing in the patch deleting the commented codes.
>>
>> My subject also says "Remove code in comments"
>>
>> Than what is not matching?
>
> Read your own patch.
>
> You are removing #defines not comments.
>
> If the whole thing is in comments, then
> the #defines immediately above what you
> are removing should also be deleted.
>
>> >
>> > > Signed-off-by: simran singhal 
>> > > ---
>> > >  drivers/iio/health/afe440x.h | 58 
>> > > 
>> > >  1 file changed, 58 deletions(-)
>> > >
>> > > diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
>> > > index 1a0f247..71e2f0e 100644
>> > > --- a/drivers/iio/health/afe440x.h
>> > > +++ b/drivers/iio/health/afe440x.h
>> > > @@ -88,56 +88,11 @@
>> > >  #define AFE440X_CONTROL0_WRITE   0x0
>> > >  #define AFE440X_CONTROL0_READ0x1
>> > >
>> > > -#define AFE440X_INTENSITY_CHAN(_index, _mask)\
>> > > - {   \
>> > > - .type = IIO_INTENSITY,  \
>> > > - .channel = _index,  \
>> > > - .address = _index,  \
>> > > - .scan_index = _index,   \
>> > > - .scan_type = {  \
>> > > - .sign = 's',\
>> > > - .realbits = 24, \
>> > > - .storagebits = 32,  \
>> > > - .endianness = IIO_CPU,  \
>> > > - },  \
>> > > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |  \
>> > > - _mask,  \
>> > > - .indexed = true,\
>> > > - }
>> > > -
>> > > -#define AFE440X_CURRENT_CHAN(_index) \
>> > > - {   \
>> > > - .type = IIO_CURRENT,\
>> > > - .channel = _index,  \
>> > > - .address = _index,  \
>> > > - .scan_index = -1,   \
>> > > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |  \
>> > > - BIT(IIO_CHAN_INFO_SCALE),   \
>> > > - .indexed = true,\
>> > > - .output = true, \
>> > > - }
>> > > -
>> > >  struct afe440x_val_table {
>> > >   int integer;
>> > >   int fract;
>> > >  };
>> > >
>> > > -#define AFE440X_TABLE_ATTR(_name, _table)\
>> > > -static ssize_t _name ## _show(struct device *dev,\
>> > > -   struct device_attribute *attr, char *buf) \
>> > > -{\
>> > > - ssize_t len = 0;\
>> > > - int i;  \
>> > > - \
>> > > - for (i = 0; i < ARRAY_SIZE(_table); i++)\
>> > > - len += scnprintf(buf + len, PAGE_SIZE - len, "%d.%06u ", \

Re: [PATCH 0/3] iio: health: Clean up

[SIMRAN SINGHAL]

Please ignore this Patch. As this patch is completely wrong due to
my lack of understanding.

On Fri, Mar 31, 2017 at 10:16 PM, simran singhal
 wrote:
> This patch-series removes commented code in afe440x.h and
> removes unused functions in afe4404.c and afe4403.c.
>
> simran singhal (3):
>   iio: health: afe440x: Remove code in comments
>   iio: health: afe4404: Remove all unused functions
>   iio: health: afe4403: Remove all unused functions
>
>  drivers/iio/health/afe4403.c | 51 --
>  drivers/iio/health/afe4404.c | 51 --
>  drivers/iio/health/afe440x.h | 58 
> 
>  3 files changed, 160 deletions(-)
>
> --
> 2.7.4
>

Re: [PATCH 1/3] iio: health: afe440x: Remove code in comments

[SIMRAN SINGHAL]

On Sat, Apr 1, 2017 at 12:03 AM, Joe Perches  wrote:
> On Fri, 2017-03-31 at 22:16 +0530, simran singhal wrote:
>> Commenting out code is a bad idea.
>> As comments are for explaining what code is about.
>
> patch doesn't match commit message.

In commit message I am clearly saying commenting out the code is a bad Idea.
As comments are for explaining what code is about.
And that's what I am doing in the patch deleting the commented codes.

My subject also says "Remove code in comments"

Than what is not matching?

>
>> Signed-off-by: simran singhal 
>> ---
>>  drivers/iio/health/afe440x.h | 58 
>> 
>>  1 file changed, 58 deletions(-)
>>
>> diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
>> index 1a0f247..71e2f0e 100644
>> --- a/drivers/iio/health/afe440x.h
>> +++ b/drivers/iio/health/afe440x.h
>> @@ -88,56 +88,11 @@
>>  #define AFE440X_CONTROL0_WRITE   0x0
>>  #define AFE440X_CONTROL0_READ0x1
>>
>> -#define AFE440X_INTENSITY_CHAN(_index, _mask)\
>> - {   \
>> - .type = IIO_INTENSITY,  \
>> - .channel = _index,  \
>> - .address = _index,  \
>> - .scan_index = _index,   \
>> - .scan_type = {  \
>> - .sign = 's',\
>> - .realbits = 24, \
>> - .storagebits = 32,  \
>> - .endianness = IIO_CPU,  \
>> - },  \
>> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |  \
>> - _mask,  \
>> - .indexed = true,\
>> - }
>> -
>> -#define AFE440X_CURRENT_CHAN(_index) \
>> - {   \
>> - .type = IIO_CURRENT,\
>> - .channel = _index,  \
>> - .address = _index,  \
>> - .scan_index = -1,   \
>> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |  \
>> - BIT(IIO_CHAN_INFO_SCALE),   \
>> - .indexed = true,\
>> - .output = true, \
>> - }
>> -
>>  struct afe440x_val_table {
>>   int integer;
>>   int fract;
>>  };
>>
>> -#define AFE440X_TABLE_ATTR(_name, _table)\
>> -static ssize_t _name ## _show(struct device *dev,\
>> -   struct device_attribute *attr, char *buf) \
>> -{\
>> - ssize_t len = 0;\
>> - int i;  \
>> - \
>> - for (i = 0; i < ARRAY_SIZE(_table); i++)\
>> - len += scnprintf(buf + len, PAGE_SIZE - len, "%d.%06u ", \
>> -  _table[i].integer, \
>> -  _table[i].fract);  \
>> - \
>> - buf[len - 1] = '\n';\
>> - \
>> - return len; \
>> -}\
>>  static DEVICE_ATTR_RO(_name)
>>
>>  struct afe440x_attr {
>> @@ -147,17 +102,4 @@ struct afe440x_attr {
>>   unsigned int table_size;
>>  };
>>
>> -#define to_afe440x_attr(_dev_attr)   \
>> - container_of(_dev_attr, struct afe440x_attr, dev_attr)
>> -
>> -#define AFE440X_ATTR(_name, _field, _table)  \
>> - struct afe440x_attr afe440x_attr_##_name = {\
>> - .dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR),  \
>> -afe440x_show_register,   \
>> -afe440x_store_register), \
>> - .field = _field,\
>> - .val_table = _table,\
>> - .table_size = ARRAY_SIZE(_table),   \
>> - }
>> -
>>  #endif /* _AFE440X_H */

[PATCH 1/3] iio: health: afe440x: Remove code in comments

[simran singhal]

Commenting out code is a bad idea.
As comments are for explaining what code is about.

Signed-off-by: simran singhal 
---
 drivers/iio/health/afe440x.h | 58 
 1 file changed, 58 deletions(-)

diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
index 1a0f247..71e2f0e 100644
--- a/drivers/iio/health/afe440x.h
+++ b/drivers/iio/health/afe440x.h
@@ -88,56 +88,11 @@
 #define AFE440X_CONTROL0_WRITE 0x0
 #define AFE440X_CONTROL0_READ  0x1
 
-#define AFE440X_INTENSITY_CHAN(_index, _mask)  \
-   {   \
-   .type = IIO_INTENSITY,  \
-   .channel = _index,  \
-   .address = _index,  \
-   .scan_index = _index,   \
-   .scan_type = {  \
-   .sign = 's',\
-   .realbits = 24, \
-   .storagebits = 32,  \
-   .endianness = IIO_CPU,  \
-   },  \
-   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |  \
-   _mask,  \
-   .indexed = true,\
-   }
-
-#define AFE440X_CURRENT_CHAN(_index)   \
-   {   \
-   .type = IIO_CURRENT,\
-   .channel = _index,  \
-   .address = _index,  \
-   .scan_index = -1,   \
-   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |  \
-   BIT(IIO_CHAN_INFO_SCALE),   \
-   .indexed = true,\
-   .output = true, \
-   }
-
 struct afe440x_val_table {
int integer;
int fract;
 };
 
-#define AFE440X_TABLE_ATTR(_name, _table)  \
-static ssize_t _name ## _show(struct device *dev,  \
- struct device_attribute *attr, char *buf) \
-{  \
-   ssize_t len = 0;\
-   int i;  \
-   \
-   for (i = 0; i < ARRAY_SIZE(_table); i++)\
-   len += scnprintf(buf + len, PAGE_SIZE - len, "%d.%06u ", \
-_table[i].integer, \
-_table[i].fract);  \
-   \
-   buf[len - 1] = '\n';\
-   \
-   return len; \
-}  \
 static DEVICE_ATTR_RO(_name)
 
 struct afe440x_attr {
@@ -147,17 +102,4 @@ struct afe440x_attr {
unsigned int table_size;
 };
 
-#define to_afe440x_attr(_dev_attr) \
-   container_of(_dev_attr, struct afe440x_attr, dev_attr)
-
-#define AFE440X_ATTR(_name, _field, _table)\
-   struct afe440x_attr afe440x_attr_##_name = {\
-   .dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR),  \
-  afe440x_show_register,   \
-  afe440x_store_register), \
-   .field = _field,\
-   .val_table = _table,\
-   .table_size = ARRAY_SIZE(_table),   \
-   }
-
 #endif /* _AFE440X_H */
-- 
2.7.4

[PATCH 2/3] iio: health: afe4404: Remove all unused functions

[simran singhal]

The functions afe440x_show_register and afe440x_store_register are never
used anywhere in the kernel. So, remove their definitions.
Grepped to find occurences.

Signed-off-by: simran singhal 
---
 drivers/iio/health/afe4404.c | 51 
 1 file changed, 51 deletions(-)

diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
index 964f523..168991a 100644
--- a/drivers/iio/health/afe4404.c
+++ b/drivers/iio/health/afe4404.c
@@ -170,57 +170,6 @@ static const struct afe440x_val_table afe4404_cap_table[] 
= {
 };
 AFE440X_TABLE_ATTR(in_intensity_capacitance_available, afe4404_cap_table);
 
-static ssize_t afe440x_show_register(struct device *dev,
-struct device_attribute *attr,
-char *buf)
-{
-   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-   struct afe4404_data *afe = iio_priv(indio_dev);
-   struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
-   unsigned int reg_val;
-   int vals[2];
-   int ret;
-
-   ret = regmap_field_read(afe->fields[afe440x_attr->field], ®_val);
-   if (ret)
-   return ret;
-
-   if (reg_val >= afe440x_attr->table_size)
-   return -EINVAL;
-
-   vals[0] = afe440x_attr->val_table[reg_val].integer;
-   vals[1] = afe440x_attr->val_table[reg_val].fract;
-
-   return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals);
-}
-
-static ssize_t afe440x_store_register(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
-   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-   struct afe4404_data *afe = iio_priv(indio_dev);
-   struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
-   int val, integer, fract, ret;
-
-   ret = iio_str_to_fixpoint(buf, 10, &integer, &fract);
-   if (ret)
-   return ret;
-
-   for (val = 0; val < afe440x_attr->table_size; val++)
-   if (afe440x_attr->val_table[val].integer == integer &&
-   afe440x_attr->val_table[val].fract == fract)
-   break;
-   if (val == afe440x_attr->table_size)
-   return -EINVAL;
-
-   ret = regmap_field_write(afe->fields[afe440x_attr->field], val);
-   if (ret)
-   return ret;
-
-   return count;
-}
-
 static AFE440X_ATTR(in_intensity1_resistance, F_TIA_GAIN_SEP, 
afe4404_res_table);
 static AFE440X_ATTR(in_intensity1_capacitance, F_TIA_CF_SEP, 
afe4404_cap_table);
 
-- 
2.7.4

[PATCH 3/3] iio: health: afe4403: Remove all unused functions

[simran singhal]

The functions afe440x_show_register and afe440x_store_register are never
used anywhere in the kernel. So, remove their definitions.
Grepped to find occurences.

Signed-off-by: simran singhal 
---
 drivers/iio/health/afe4403.c | 51 
 1 file changed, 51 deletions(-)

diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
index 6bb23a4..3059014 100644
--- a/drivers/iio/health/afe4403.c
+++ b/drivers/iio/health/afe4403.c
@@ -136,57 +136,6 @@ static const struct afe440x_val_table afe4403_cap_table[] 
= {
 };
 AFE440X_TABLE_ATTR(in_intensity_capacitance_available, afe4403_cap_table);
 
-static ssize_t afe440x_show_register(struct device *dev,
-struct device_attribute *attr,
-char *buf)
-{
-   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-   struct afe4403_data *afe = iio_priv(indio_dev);
-   struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
-   unsigned int reg_val;
-   int vals[2];
-   int ret;
-
-   ret = regmap_field_read(afe->fields[afe440x_attr->field], ®_val);
-   if (ret)
-   return ret;
-
-   if (reg_val >= afe440x_attr->table_size)
-   return -EINVAL;
-
-   vals[0] = afe440x_attr->val_table[reg_val].integer;
-   vals[1] = afe440x_attr->val_table[reg_val].fract;
-
-   return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals);
-}
-
-static ssize_t afe440x_store_register(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
-   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-   struct afe4403_data *afe = iio_priv(indio_dev);
-   struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
-   int val, integer, fract, ret;
-
-   ret = iio_str_to_fixpoint(buf, 10, &integer, &fract);
-   if (ret)
-   return ret;
-
-   for (val = 0; val < afe440x_attr->table_size; val++)
-   if (afe440x_attr->val_table[val].integer == integer &&
-   afe440x_attr->val_table[val].fract == fract)
-   break;
-   if (val == afe440x_attr->table_size)
-   return -EINVAL;
-
-   ret = regmap_field_write(afe->fields[afe440x_attr->field], val);
-   if (ret)
-   return ret;
-
-   return count;
-}
-
 static AFE440X_ATTR(in_intensity1_resistance, F_RF_LED, afe4403_res_table);
 static AFE440X_ATTR(in_intensity1_capacitance, F_CF_LED, afe4403_cap_table);
 
-- 
2.7.4

[PATCH 0/3] iio: health: Clean up

[simran singhal]

This patch-series removes commented code in afe440x.h and 
removes unused functions in afe4404.c and afe4403.c.

simran singhal (3):
  iio: health: afe440x: Remove code in comments
  iio: health: afe4404: Remove all unused functions
  iio: health: afe4403: Remove all unused functions

 drivers/iio/health/afe4403.c | 51 --
 drivers/iio/health/afe4404.c | 51 --
 drivers/iio/health/afe440x.h | 58 
 3 files changed, 160 deletions(-)

-- 
2.7.4

[PATCH 0/3] staging: iio: Remove useless type conversion

[simran singhal]

This patch-series removes some type conversions like casting a pointer to a 
pointer of same type,
casting to the original type using addressof(&) operator etc.

simran singhal (3):
  staging: iio: accel: Remove useless type conversion
  staging: iio: frequency: Remove useless type conversion
  staging: iio: light: Remove useless type conversion

 drivers/staging/iio/accel/adis16201.c| 2 +-
 drivers/staging/iio/accel/adis16203.c| 2 +-
 drivers/staging/iio/accel/adis16209.c| 2 +-
 drivers/staging/iio/accel/adis16240.c| 6 +++---
 drivers/staging/iio/frequency/ad9832.c   | 2 +-
 drivers/staging/iio/light/tsl2x7x_core.c | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

-- 
2.7.4

[PATCH 2/3] staging: iio: frequency: Remove useless type conversion

[simran singhal]

Some type conversions like casting a pointer to a pointer of same type,
casting to the original type using addressof(&) operator etc. are not
needed. Therefore, remove them. Done using coccinelle:

@@
type t;
t *p;
t a;
@@
(
- (t)(a)
+ a
|
- (t *)(p)
+ p
|
- (t *)(&a)
+ &a
)

Signed-off-by: simran singhal 
---
 drivers/staging/iio/frequency/ad9832.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c 
b/drivers/staging/iio/frequency/ad9832.c
index 425b8ab..01bdf8e 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -119,7 +119,7 @@ struct ad9832_state {
 static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long 
fout)
 {
unsigned long long freqreg = (u64)fout *
-(u64)((u64)1L << AD9832_FREQ_BITS);
+(u64)1L << AD9832_FREQ_BITS;
do_div(freqreg, mclk);
return freqreg;
 }
-- 
2.7.4

[PATCH 3/3] staging: iio: light: Remove useless type conversion

[simran singhal]

Some type conversions like casting a pointer to a pointer of same type,
casting to the original type using addressof(&) operator etc. are not
needed. Therefore, remove them. Done using coccinelle:

@@
type t;
t *p;
t a;
@@
(
- (t)(a)
+ a
|
- (t *)(p)
+ p
|
- (t *)(&a)
+ &a
)

Signed-off-by: simran singhal 
---
 drivers/staging/iio/light/tsl2x7x_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/light/tsl2x7x_core.c 
b/drivers/staging/iio/light/tsl2x7x_core.c
index ea15bc1..0490c1d 100644
--- a/drivers/staging/iio/light/tsl2x7x_core.c
+++ b/drivers/staging/iio/light/tsl2x7x_core.c
@@ -624,7 +624,7 @@ static int tsl2x7x_als_calibrate(struct iio_dev *indio_dev)
dev_info(&chip->client->dev,
 "%s als_calibrate completed\n", chip->client->name);
 
-   return (int)gain_trim_val;
+   return gain_trim_val;
 }
 
 static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
-- 
2.7.4

[PATCH 1/3] staging: iio: accel: Remove useless type conversion

[simran singhal]

Some type conversions like casting a pointer to a pointer of same type,
casting to the original type using addressof(&) operator etc. are not
needed. Therefore, remove them. Done using coccinelle:

@@
type t;
t *p;
t a;
@@
(
- (t)(a)
+ a
|
- (t *)(p)
+ p
|
- (t *)(&a)
+ &a
)

Signed-off-by: simran singhal 
---
 drivers/staging/iio/accel/adis16201.c | 2 +-
 drivers/staging/iio/accel/adis16203.c | 2 +-
 drivers/staging/iio/accel/adis16209.c | 2 +-
 drivers/staging/iio/accel/adis16240.c | 6 +++---
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/accel/adis16201.c 
b/drivers/staging/iio/accel/adis16201.c
index fbc2406..b7381d3 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -228,7 +228,7 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
if (ret)
return ret;
val16 &= (1 << bits) - 1;
-   val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
+   val16 = val16 << (16 - bits) >> (16 - bits);
*val = val16;
return IIO_VAL_INT;
}
diff --git a/drivers/staging/iio/accel/adis16203.c 
b/drivers/staging/iio/accel/adis16203.c
index b59755a..25e5e52 100644
--- a/drivers/staging/iio/accel/adis16203.c
+++ b/drivers/staging/iio/accel/adis16203.c
@@ -211,7 +211,7 @@ static int adis16203_read_raw(struct iio_dev *indio_dev,
return ret;
}
val16 &= (1 << bits) - 1;
-   val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
+   val16 = val16 << (16 - bits) >> (16 - bits);
*val = val16;
mutex_unlock(&indio_dev->mlock);
return IIO_VAL_INT;
diff --git a/drivers/staging/iio/accel/adis16209.c 
b/drivers/staging/iio/accel/adis16209.c
index 52fa2e0..7aac310 100644
--- a/drivers/staging/iio/accel/adis16209.c
+++ b/drivers/staging/iio/accel/adis16209.c
@@ -259,7 +259,7 @@ static int adis16209_read_raw(struct iio_dev *indio_dev,
return ret;
}
val16 &= (1 << bits) - 1;
-   val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
+   val16 = val16 << (16 - bits) >> (16 - bits);
*val = val16;
return IIO_VAL_INT;
}
diff --git a/drivers/staging/iio/accel/adis16240.c 
b/drivers/staging/iio/accel/adis16240.c
index 6e3c95c..2c55b65 100644
--- a/drivers/staging/iio/accel/adis16240.c
+++ b/drivers/staging/iio/accel/adis16240.c
@@ -220,7 +220,7 @@ static ssize_t adis16240_spi_read_signed(struct device *dev,
if (val & ADIS16240_ERROR_ACTIVE)
adis_check_status(st);
 
-   val = (s16)(val << shift) >> shift;
+   val = val << shift >> shift;
return sprintf(buf, "%d\n", val);
 }
 
@@ -294,7 +294,7 @@ static int adis16240_read_raw(struct iio_dev *indio_dev,
return ret;
}
val16 &= (1 << bits) - 1;
-   val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
+   val16 = val16 << (16 - bits) >> (16 - bits);
*val = val16;
return IIO_VAL_INT;
case IIO_CHAN_INFO_PEAK:
@@ -305,7 +305,7 @@ static int adis16240_read_raw(struct iio_dev *indio_dev,
return ret;
}
val16 &= (1 << bits) - 1;
-   val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
+   val16 = val16 << (16 - bits) >> (16 - bits);
*val = val16;
return IIO_VAL_INT;
}
-- 
2.7.4

[PATCH v2] iio: gyro: adis16060: Change the name of function.

[simran singhal]

Change the name of function from adis16060_spi_write_than_read()
to adis16060_spi_write_then_read(). change "than" to "then" as
its time depended.

Signed-off-by: simran singhal 
---

 v2:
   -Change the subject.
   -Add signed-off-by.

 drivers/staging/iio/gyro/adis16060_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/gyro/adis16060_core.c 
b/drivers/staging/iio/gyro/adis16060_core.c
index 8115962..9675245 100644
--- a/drivers/staging/iio/gyro/adis16060_core.c
+++ b/drivers/staging/iio/gyro/adis16060_core.c
@@ -40,7 +40,7 @@ struct adis16060_state {
 
 static struct iio_dev *adis16060_iio_dev;
 
-static int adis16060_spi_write_than_read(struct iio_dev *indio_dev,
+static int adis16060_spi_write_then_read(struct iio_dev *indio_dev,
 u8 conf, u16 *val)
 {
int ret;
@@ -81,7 +81,7 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
 
switch (mask) {
case IIO_CHAN_INFO_RAW:
-   ret = adis16060_spi_write_than_read(indio_dev,
+   ret = adis16060_spi_write_then_read(indio_dev,
chan->address, &tval);
if (ret < 0)
return ret;
-- 
2.7.4

Re: [Outreachy kernel] [PATCH v4] staging: iio: ade7753: Replace mlock with driver private lock

[SIMRAN SINGHAL]

On Fri, Mar 31, 2017 at 1:18 AM, Jonathan Cameron
 wrote:
>
>
> On 30 March 2017 19:44:26 BST, SIMRAN SINGHAL  
> wrote:
>>On Fri, Mar 31, 2017 at 12:02 AM, Jonathan Cameron 
>>wrote:
>>> On 28/03/17 19:37, Alison Schofield wrote:
>>>> On Tue, Mar 28, 2017 at 10:55:17PM +0530, SIMRAN SINGHAL wrote:
>>>>> On Fri, Mar 24, 2017 at 12:51 AM, Alison Schofield
>> wrote:
>>>>>> On Fri, Mar 24, 2017 at 12:05:20AM +0530, simran singhal wrote:
>>>>>>> The IIO subsystem is redefining iio_dev->mlock to be used by
>>>>>>> the IIO core only for protecting device operating mode changes.
>>>>>>> ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
>>>>>>>
>>>>>>> In this driver, mlock was being used to protect hardware state
>>>>>>> changes.  Replace it with a lock in the devices global data.
>>>>>>
>>>>>> Hi Simran,
>>>>>>
>>>>>> Please post all revision histories below the --- not just the most
>>>>>> recent.
>>>>>>
>>>>> Sorry, will not repeat this.
>>>>>
>>>>>> Does this lock enforce the needed "atomicity" in the
>>write_frequency
>>>>>> function? I read Jonathans comment on a previous revision about
>>>>>> "ensuring the spi bus frequency and sampling frequency of the
>>device
>>>>>> are changed in an atomic fashion"
>>>>>>
>>>>>
>>>>> By introducing another lock I am protecting read_modify_write and
>>>>> in this way also protecting the designated register that we are
>>about
>>>>> to write.
>>>>
>>>> I see it protecting this path from being re-entered.  My uncertainty
>>>> is about other paths to read/write.
>>>>
>>>>>
>>>>>> Is it possible for another spi bus transaction (read or write) to
>>>>>> occur between the read and write in write_frequency?
>>>>>>
>>>>>
>>>>> Gargi has also come up with a solution.
>>>>>
>>https://groups.google.com/forum/#!topic/outreachy-kernel/kzE9CrI5Bd8
>>>>>
>>>>> Should I do like her as her's also seem correct or go ahead with
>>this.
>>>>
>>>> My suggestion would be to wait for feedback on Gargi's patch.
>>>> (See the Outreachy log about creating similar solutions.)
>>>>
>>>> We will not be able to close on this set of patches during the
>>>> Outreachy application window.  You can continue to push for closure
>>>> beyond the March 30th date as your time allows :)
>>>>
>>> It is a close choice between the two approaches. In some ways
>>> yours is easier to follow, but Gargi's is more elegant.
>>>
>>> Lets go with that one for consistency across similar drivers,
>>> but if you had been the original author and done it this way
>>> I certainly wouldn't bother asking you to change it!
>>
>>Yes, jonathan I am the original author.
>
> Sorry, I meant of the driver rather than this improvement.
>
By reading your pervious comment, I got what you mean!!
For consistency, I will do it in the same way Gargi did.

> Jonathan
>>
>>>
>>> So in conclusion both patches are good.
>>>
>>> Jonathan
>>>
>>>> Thanks,
>>>> alisons
>>>>
>>>>>
>>>>>> alisons
>>>>>>>
>>>>>>> Signed-off-by: simran singhal 
>>>>>>> ---
>>>>>>>
>>>>>>>  v4:
>>>>>>>-Add mutex_init
>>>>>>>
>>>>>>>  drivers/staging/iio/meter/ade7753.c | 7 +--
>>>>>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/staging/iio/meter/ade7753.c
>>b/drivers/staging/iio/meter/ade7753.c
>>>>>>> index b71fbd3..30aebaf 100644
>>>>>>> --- a/drivers/staging/iio/meter/ade7753.c
>>>>>>> +++ b/drivers/staging/iio/meter/ade7753.c
>>>>>>> @@ -80,11 +80,13 @@
>>>>>>>   * @us: actual spi_device
>>>>>>>   * @tx: transmit buffer
>>>>>>>   * @rx: receive buffer
>>>&g

[PATCH] iio: gyro: adis16060: Change the function's name

[simran singhal]

Change the name of function from adis16060_spi_write_than_read()
to adis16060_spi_write_then_read(). change "than" to "then" as
its time depended.
---
 drivers/staging/iio/gyro/adis16060_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/gyro/adis16060_core.c 
b/drivers/staging/iio/gyro/adis16060_core.c
index 8115962..9675245 100644
--- a/drivers/staging/iio/gyro/adis16060_core.c
+++ b/drivers/staging/iio/gyro/adis16060_core.c
@@ -40,7 +40,7 @@ struct adis16060_state {
 
 static struct iio_dev *adis16060_iio_dev;
 
-static int adis16060_spi_write_than_read(struct iio_dev *indio_dev,
+static int adis16060_spi_write_then_read(struct iio_dev *indio_dev,
 u8 conf, u16 *val)
 {
int ret;
@@ -81,7 +81,7 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
 
switch (mask) {
case IIO_CHAN_INFO_RAW:
-   ret = adis16060_spi_write_than_read(indio_dev,
+   ret = adis16060_spi_write_then_read(indio_dev,
chan->address, &tval);
if (ret < 0)
return ret;
-- 
2.7.4

Re: [Outreachy kernel] [PATCH v4] staging: iio: ade7753: Replace mlock with driver private lock

[SIMRAN SINGHAL]

On Fri, Mar 31, 2017 at 12:02 AM, Jonathan Cameron  wrote:
> On 28/03/17 19:37, Alison Schofield wrote:
>> On Tue, Mar 28, 2017 at 10:55:17PM +0530, SIMRAN SINGHAL wrote:
>>> On Fri, Mar 24, 2017 at 12:51 AM, Alison Schofield  
>>> wrote:
>>>> On Fri, Mar 24, 2017 at 12:05:20AM +0530, simran singhal wrote:
>>>>> The IIO subsystem is redefining iio_dev->mlock to be used by
>>>>> the IIO core only for protecting device operating mode changes.
>>>>> ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
>>>>>
>>>>> In this driver, mlock was being used to protect hardware state
>>>>> changes.  Replace it with a lock in the devices global data.
>>>>
>>>> Hi Simran,
>>>>
>>>> Please post all revision histories below the --- not just the most
>>>> recent.
>>>>
>>> Sorry, will not repeat this.
>>>
>>>> Does this lock enforce the needed "atomicity" in the write_frequency
>>>> function? I read Jonathans comment on a previous revision about
>>>> "ensuring the spi bus frequency and sampling frequency of the device
>>>> are changed in an atomic fashion"
>>>>
>>>
>>> By introducing another lock I am protecting read_modify_write and
>>> in this way also protecting the designated register that we are about
>>> to write.
>>
>> I see it protecting this path from being re-entered.  My uncertainty
>> is about other paths to read/write.
>>
>>>
>>>> Is it possible for another spi bus transaction (read or write) to
>>>> occur between the read and write in write_frequency?
>>>>
>>>
>>> Gargi has also come up with a solution.
>>> https://groups.google.com/forum/#!topic/outreachy-kernel/kzE9CrI5Bd8
>>>
>>> Should I do like her as her's also seem correct or go ahead with this.
>>
>> My suggestion would be to wait for feedback on Gargi's patch.
>> (See the Outreachy log about creating similar solutions.)
>>
>> We will not be able to close on this set of patches during the
>> Outreachy application window.  You can continue to push for closure
>> beyond the March 30th date as your time allows :)
>>
> It is a close choice between the two approaches. In some ways
> yours is easier to follow, but Gargi's is more elegant.
>
> Lets go with that one for consistency across similar drivers,
> but if you had been the original author and done it this way
> I certainly wouldn't bother asking you to change it!

Yes, jonathan I am the original author.

>
> So in conclusion both patches are good.
>
> Jonathan
>
>> Thanks,
>> alisons
>>
>>>
>>>> alisons
>>>>>
>>>>> Signed-off-by: simran singhal 
>>>>> ---
>>>>>
>>>>>  v4:
>>>>>-Add mutex_init
>>>>>
>>>>>  drivers/staging/iio/meter/ade7753.c | 7 +--
>>>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/staging/iio/meter/ade7753.c 
>>>>> b/drivers/staging/iio/meter/ade7753.c
>>>>> index b71fbd3..30aebaf 100644
>>>>> --- a/drivers/staging/iio/meter/ade7753.c
>>>>> +++ b/drivers/staging/iio/meter/ade7753.c
>>>>> @@ -80,11 +80,13 @@
>>>>>   * @us: actual spi_device
>>>>>   * @tx: transmit buffer
>>>>>   * @rx: receive buffer
>>>>> + * @lock:   protect sensor state
>>>>>   * @buf_lock:   mutex to protect tx and rx
>>>>>   **/
>>>>>  struct ade7753_state {
>>>>>   struct spi_device   *us;
>>>>>   struct mutexbuf_lock;
>>>>> + struct mutexlock;  /* protect sensor state */
>>>>>   u8  tx[ADE7753_MAX_TX] cacheline_aligned;
>>>>>   u8  rx[ADE7753_MAX_RX];
>>>>>  };
>>>>> @@ -484,7 +486,7 @@ static ssize_t ade7753_write_frequency(struct device 
>>>>> *dev,
>>>>>   if (!val)
>>>>>   return -EINVAL;
>>>>>
>>>>> - mutex_lock(&indio_dev->mlock);
>>>>> + mutex_lock(&st->lock);
>>>>>
>>>>>   t = 27900 / val;
>>>>>   if (t > 0)
>>>>> @@ -505,7 +507,7 @@ static ssize_t ade7753_write_frequency(struct device 
>>>>> *dev,
>>>>>   ret = ade7753_spi_write_reg_16(dev, ADE7753_MODE, reg);
>>>>>
>>>>>  out:
>>>>> - mutex_unlock(&indio_dev->mlock);
>>>>> + mutex_unlock(&st->lock);
>>>>>
>>>>>   return ret ? ret : len;
>>>>>  }
>>>>> @@ -581,6 +583,7 @@ static int ade7753_probe(struct spi_device *spi)
>>>>>   st = iio_priv(indio_dev);
>>>>>   st->us = spi;
>>>>>   mutex_init(&st->buf_lock);
>>>>> + mutex_init(&st->lock);
>>>>>
>>>>>   indio_dev->name = spi->dev.driver->name;
>>>>>   indio_dev->dev.parent = &spi->dev;
>>>>> --
>>>>> 2.7.4
>>>>>
>>>>> --
>>>>> You received this message because you are subscribed to the Google Groups 
>>>>> "outreachy-kernel" group.
>>>>> To unsubscribe from this group and stop receiving emails from it, send an 
>>>>> email to outreachy-kernel+unsubscr...@googlegroups.com.
>>>>> To post to this group, send email to outreachy-ker...@googlegroups.com.
>>>>> To view this discussion on the web visit 
>>>>> https://groups.google.com/d/msgid/outreachy-kernel/20170323183520.GA9871%40singhal-Inspiron-5558.
>>>>> For more options, visit https://groups.google.com/d/optout.
>

[PATCH v2] iio: light: lm3533-als: constify attribute_group structures

[simran singhal]

Check for attribute_group structures that are only stored in the
event_attrs filed of iio_info structure. As the event_attrs field of
iio_info structures is constant, so these attribute_group structures can
also be declared constant. Done using coccinelle:

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct attribute_group i@p = {...};

@ok1@
identifier r1.i;
position p;
struct iio_info x;
@@
x.event_attrs=&i@p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct attribute_group i={...};

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct attribute_group i;

As the attrs field of iio_info structures is also constant, so these
attribute_group structures can also be declared constant. Done manually.

File size before:
   textdata bss dec hex filename
   57982376   081741fee drivers/iio/light/lm3533-als.o

File size after:
   textdata bss dec hex filename
   59262248   081741fee drivers/iio/light/lm3533-als.o

Signed-off-by: simran singhal 
---
 v2:
   -make one more attribute_group const.
   -change the values of file size after.
   -change commit message.

 drivers/iio/light/lm3533-als.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
index f409c20..0443fd2 100644
--- a/drivers/iio/light/lm3533-als.c
+++ b/drivers/iio/light/lm3533-als.c
@@ -690,7 +690,7 @@ static struct attribute *lm3533_als_event_attributes[] = {
NULL
 };
 
-static struct attribute_group lm3533_als_event_attribute_group = {
+static const struct attribute_group lm3533_als_event_attribute_group = {
.attrs = lm3533_als_event_attributes
 };
 
@@ -714,7 +714,7 @@ static struct attribute *lm3533_als_attributes[] = {
NULL
 };
 
-static struct attribute_group lm3533_als_attribute_group = {
+static const struct attribute_group lm3533_als_attribute_group = {
.attrs = lm3533_als_attributes
 };
 
-- 
2.7.4

Re: [PATCH] iio: adc: ad799x: constify attribute_group structures

[SIMRAN SINGHAL]

On Thu, Mar 30, 2017 at 3:06 AM, Jonathan Cameron  wrote:
> On 29/03/17 22:28, SIMRAN SINGHAL wrote:
>> On Thu, Mar 30, 2017 at 2:43 AM, Jonathan Cameron  wrote:
>>> On 28/03/17 21:07, simran singhal wrote:
>>>> Check for attribute_group structures that are only stored in the
>>>> event_attrs filed of iio_info structure. As the event_attrs field of
>>>> iio_info structures is constant, so these attribute_group structures can
>>>> also be declared constant.
>>>> Done using coccinelle:
>>>>
>>>> @r1 disable optional_qualifier @
>>>> identifier i;
>>>> position p;
>>>> @@
>>>> static struct attribute_group i@p = {...};
>>>>
>>>> @ok1@
>>>> identifier r1.i;
>>>> position p;
>>>> struct iio_info x;
>>>> @@
>>>> x.event_attrs=&i@p;
>>>>
>>>> @bad@
>>>> position p!={r1.p,ok1.p};
>>>> identifier r1.i;
>>>> @@
>>>> i@p
>>>>
>>>> @depends on !bad disable optional_qualifier@
>>>> identifier r1.i;
>>>> @@
>>>> static
>>>> +const
>>>> struct attribute_group i={...};
>>>>
>>>> @depends on !bad disable optional_qualifier@
>>>> identifier r1.i;
>>>> @@
>>>> +const
>>>> struct attribute_group i;
>>>>
>>>> File size before:
>>>>textdata bss dec hex filename
>>>>   26051 464   0   265156793 drivers/iio/adc/ad799x.o
>>>>
>>>> File size after:
>>>>text  data bss dec hex filename
>>>>   26115   400   0   265156793 drivers/iio/adc/ad799x.o
>>>>
>>>> Signed-off-by: simran singhal 
>>> Applied to the togreg branch of iio.git and pushed out as testing
>>> for the autobuilders to play with it.
>>>
>>
>> But my tree is up-to-date and I also test it before sending.
> this is standard practice.  I build as well before pushing out, but
> I only do one or two configurations of one or two architectures.
> The autobuilders are a set of large servers that Intel run as a service
> to the kernel community.
>
> Typically they'll run a few hundred builds for dozens of processor 
> architectures
> + a much more extensive set of static tests than most kernel developers
> are set up to run.  Here things will almost certainly be fine, but
> I've been wrong before!
>
> The only thing is I tend to push out in the evening then go to bed before
> the results come in, then might not get to it for a day or so to push out
> as togreg.  This tree I am happy to rebase - that is I can drop or fixup
> patches with issues.  The togreg branch is the one that people often
> base new work on, so if I change the tree under them all sorts of nasty
> merge issues can occur.
>
> This is why most trees that ultimately go upstream are non rebasing -
> but a lot of people now have a testing branch which is deliberately
> unofficial - people shouldn't use it to base their trees on as it
> is temporary for build test purposes.
>
> Greg has one as well (I assume this is what it is for), he just tends
> to be quicker about dealing with whatever comes up than I am so
> doesn't explicitly mention it when he takes patches.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/log/?h=staging-testing
>
> So it's nothing to worry about. I just put that note in so people
> don't wonder why their patch isn't immediately present in the public
> togreg branch of iio.git.
>

Thanks for the explaination.

Simran

> Jonathan
>>
>>> Thanks,
>>>
>>> Jonathan
>>>> ---
>>>>  drivers/iio/adc/ad799x.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
>>>> index 9704090..22426ae 100644
>>>> --- a/drivers/iio/adc/ad799x.c
>>>> +++ b/drivers/iio/adc/ad799x.c
>>>> @@ -520,7 +520,7 @@ static struct attribute *ad799x_event_attributes[] = {
>>>>   NULL,
>>>>  };
>>>>
>>>> -static struct attribute_group ad799x_event_attrs_group = {
>>>> +static const struct attribute_group ad799x_event_attrs_group = {
>>>>   .attrs = ad799x_event_attributes,
>>>>  };
>>>>
>>>>
>>>
>

Re: [PATCH] iio: adc: ad799x: constify attribute_group structures

[SIMRAN SINGHAL]

On Thu, Mar 30, 2017 at 2:43 AM, Jonathan Cameron  wrote:
> On 28/03/17 21:07, simran singhal wrote:
>> Check for attribute_group structures that are only stored in the
>> event_attrs filed of iio_info structure. As the event_attrs field of
>> iio_info structures is constant, so these attribute_group structures can
>> also be declared constant.
>> Done using coccinelle:
>>
>> @r1 disable optional_qualifier @
>> identifier i;
>> position p;
>> @@
>> static struct attribute_group i@p = {...};
>>
>> @ok1@
>> identifier r1.i;
>> position p;
>> struct iio_info x;
>> @@
>> x.event_attrs=&i@p;
>>
>> @bad@
>> position p!={r1.p,ok1.p};
>> identifier r1.i;
>> @@
>> i@p
>>
>> @depends on !bad disable optional_qualifier@
>> identifier r1.i;
>> @@
>> static
>> +const
>> struct attribute_group i={...};
>>
>> @depends on !bad disable optional_qualifier@
>> identifier r1.i;
>> @@
>> +const
>> struct attribute_group i;
>>
>> File size before:
>>textdata bss dec hex filename
>>   26051 464   0   265156793 drivers/iio/adc/ad799x.o
>>
>> File size after:
>>text  data bss dec hex filename
>>   26115   400   0   265156793 drivers/iio/adc/ad799x.o
>>
>> Signed-off-by: simran singhal 
> Applied to the togreg branch of iio.git and pushed out as testing
> for the autobuilders to play with it.
>

But my tree is up-to-date and I also test it before sending.

> Thanks,
>
> Jonathan
>> ---
>>  drivers/iio/adc/ad799x.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
>> index 9704090..22426ae 100644
>> --- a/drivers/iio/adc/ad799x.c
>> +++ b/drivers/iio/adc/ad799x.c
>> @@ -520,7 +520,7 @@ static struct attribute *ad799x_event_attributes[] = {
>>   NULL,
>>  };
>>
>> -static struct attribute_group ad799x_event_attrs_group = {
>> +static const struct attribute_group ad799x_event_attrs_group = {
>>   .attrs = ad799x_event_attributes,
>>  };
>>
>>
>

Re: [PATCH] iio: adc: max1363: constify attribute_group structures

[SIMRAN SINGHAL]

On Thu, Mar 30, 2017 at 2:41 AM, Jonathan Cameron  wrote:
> On 28/03/17 21:15, simran singhal wrote:
>> Check for attribute_group structures that are only stored in the
>> event_attrs filed of iio_info structure. As the event_attrs field of
>> iio_info structures is constant, so these attribute_group structures can
>> also be declared constant. Done using coccinelle:
>>
>> @r1 disable optional_qualifier @
>> identifier i;
>> position p;
>> @@
>> static struct attribute_group i@p = {...};
>>
>> @ok1@
>> identifier r1.i;
>> position p;
>> struct iio_info x;
>> @@
>> x.event_attrs=&i@p;
>>
>> @bad@
>> position p!={r1.p,ok1.p};
>> identifier r1.i;
>> @@
>> i@p
>>
>> @depends on !bad disable optional_qualifier@
>> identifier r1.i;
>> @@
>> static
>> +const
>> struct attribute_group i={...};
>>
>> @depends on !bad disable optional_qualifier@
>> identifier r1.i;
>> @@
>> +const
>> struct attribute_group i;
>>
>> File size before:
>>textdata bss dec hex filename
>>   36951     448   0   373999217 drivers/iio/adc/max1363.o
>>
>> File size after:
>>text  data bss dec hex filename
>>   37015   384   0   373999217 drivers/iio/adc/max1363.o
>>
>> Signed-off-by: simran singhal 
> Applied to the togreg branch of iio.git and pushed out as testing for
> the autobuilders to play with it.
>

But my tree is up-to-date and I also test it before sending.

> Thanks,
>
> Jonathan
>> ---
>>  drivers/iio/adc/max1363.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
>> index c6c12fe..80eada4 100644
>> --- a/drivers/iio/adc/max1363.c
>> +++ b/drivers/iio/adc/max1363.c
>> @@ -1007,7 +1007,7 @@ static struct attribute *max1363_event_attributes[] = {
>>   NULL,
>>  };
>>
>> -static struct attribute_group max1363_event_attribute_group = {
>> +static const struct attribute_group max1363_event_attribute_group = {
>>   .attrs = max1363_event_attributes,
>>  };
>>
>>
>

Re: [PATCH] iio: dac: ad5504: constify attribute_group structures

[SIMRAN SINGHAL]

On Thu, Mar 30, 2017 at 2:39 AM, Jonathan Cameron  wrote:
> On 28/03/17 21:21, simran singhal wrote:
>> Check for attribute_group structures that are only stored in the
>> event_attrs filed of iio_info structure. As the event_attrs field of
>> iio_info structures is constant, so these attribute_group structures can
>> also be declared constant. Done using coccinelle:
>>
>> @r1 disable optional_qualifier @
>> identifier i;
>> position p;
>> @@
>> static struct attribute_group i@p = {...};
>>
>> @ok1@
>> identifier r1.i;
>> position p;
>> struct iio_info x;
>> @@
>> x.event_attrs=&i@p;
>>
>> @bad@
>> position p!={r1.p,ok1.p};
>> identifier r1.i;
>> @@
>> i@p
>>
>> @depends on !bad disable optional_qualifier@
>> identifier r1.i;
>> @@
>> static
>> +const
>> struct attribute_group i={...};
>>
>> @depends on !bad disable optional_qualifier@
>> identifier r1.i;
>> @@
>> +const
>> struct attribute_group i;
>>
>> File size before:
>>textdata bss dec hex filename
>>    3046 360   03406 d4e drivers/iio/dac/ad5504.o
>>
>> File size after:
>>text  data bss dec hex filename
>>3110   296   03406 d4e drivers/iio/dac/ad5504.o
>>
>> Signed-off-by: simran singhal 
> Applied to the togreg branch of iio.git and pushed out as testing
> for the autobuilders to play with it.
>
But my tree is up-to-date and I also test it before sending.

> Thanks,
>
> Jonathan
>> ---
>>  drivers/iio/dac/ad5504.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
>> index 788b3d6..22a027d 100644
>> --- a/drivers/iio/dac/ad5504.c
>> +++ b/drivers/iio/dac/ad5504.c
>> @@ -212,7 +212,7 @@ static struct attribute *ad5504_ev_attributes[] = {
>>   NULL,
>>  };
>>
>> -static struct attribute_group ad5504_ev_attribute_group = {
>> +static const struct attribute_group ad5504_ev_attribute_group = {
>>   .attrs = ad5504_ev_attributes,
>>  };
>>
>>
>

Re: [Outreachy kernel] [PATCH 1/4] iio: common: st_sensors: Replace ternary operator with min macro

[SIMRAN SINGHAL]

On Wed, Mar 29, 2017 at 9:46 PM, Daniel Baluta  wrote:
> On Wed, Mar 29, 2017 at 3:33 PM, simran singhal
>  wrote:
>> Use macro min() to get the minimum of two values for brevity and
>> readability.
>>
>> Signed-off-by: simran singhal 
>> ---
>>  drivers/iio/common/st_sensors/st_sensors_i2c.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/common/st_sensors/st_sensors_i2c.c 
>> b/drivers/iio/common/st_sensors/st_sensors_i2c.c
>> index c83df4d..7a68fdd 100644
>> --- a/drivers/iio/common/st_sensors/st_sensors_i2c.c
>> +++ b/drivers/iio/common/st_sensors/st_sensors_i2c.c
>> @@ -39,7 +39,7 @@ static int st_sensors_i2c_read_byte(struct 
>> st_sensor_transfer_buffer *tb,
>> *res_byte = err & 0xff;
>>
>>  st_accel_i2c_read_byte_error:
>> -   return err < 0 ? err : 0;
>> +   return min(err, 0);
>>  }
>
> Appreciate the brevity but this certainly doesn't make code
> easier to read.
>
> In linux kernel err < 0 signifies an error and be replacing
> comparison < 0 with min() we some hide the meaning of this.
>
Yes, you are right keeping the previous one will be more
meaningful.

Thanks.

> thanks,
> Daniel.

Re: [Outreachy kernel] [PATCH 4/4] iio: light: si1145: Replace ternary operator with min macro

[SIMRAN SINGHAL]

On Wed, Mar 29, 2017 at 8:22 PM, SIMRAN SINGHAL
 wrote:
> On Wed, Mar 29, 2017 at 6:23 PM, Lars-Peter Clausen  wrote:
>> On 03/29/2017 02:40 PM, Julia Lawall wrote:
>>>
>>>
>>> On Wed, 29 Mar 2017, simran singhal wrote:
>>>
>>>> Use macro min() to get the minimum of two values for brevity and
>>>> readability.
>>>>
>>>> Signed-off-by: simran singhal 
>>>> ---
>>>>  drivers/iio/light/si1145.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/iio/light/si1145.c b/drivers/iio/light/si1145.c
>>>> index 096034c..e7ad6fb 100644
>>>> --- a/drivers/iio/light/si1145.c
>>>> +++ b/drivers/iio/light/si1145.c
>>>> @@ -557,7 +557,7 @@ static int si1145_set_chlist(struct iio_dev 
>>>> *indio_dev, unsigned long scan_mask)
>>>>  data->scan_mask = scan_mask;
>>>>  ret = si1145_param_set(data, SI1145_PARAM_CHLIST, reg);
>>>>
>>>> -return ret < 0 ? ret : 0;
>>>> +return min(ret, 0);
>>>
>>> A similar change involving max was already rejected.  ret < 0 is a
>>> standard way of detecting an error, so perhaps leaving that explicitly
>>> present will be preferred.
>>
>> I think a more sensible thing to do here is to check whether ret/err can
>> actually take any positive values and if not, replace the whole thing with
>> just 'return ret;' or 'return some_fn()'. I'd expect that his can be done in
>> most of the cases.
>>

Lars, I will check it and resend it.

Re: [Outreachy kernel] [PATCH 4/4] iio: light: si1145: Replace ternary operator with min macro

[SIMRAN SINGHAL]

On Wed, Mar 29, 2017 at 6:23 PM, Lars-Peter Clausen  wrote:
> On 03/29/2017 02:40 PM, Julia Lawall wrote:
>>
>>
>> On Wed, 29 Mar 2017, simran singhal wrote:
>>
>>> Use macro min() to get the minimum of two values for brevity and
>>> readability.
>>>
>>> Signed-off-by: simran singhal 
>>> ---
>>>  drivers/iio/light/si1145.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/iio/light/si1145.c b/drivers/iio/light/si1145.c
>>> index 096034c..e7ad6fb 100644
>>> --- a/drivers/iio/light/si1145.c
>>> +++ b/drivers/iio/light/si1145.c
>>> @@ -557,7 +557,7 @@ static int si1145_set_chlist(struct iio_dev *indio_dev, 
>>> unsigned long scan_mask)
>>>  data->scan_mask = scan_mask;
>>>  ret = si1145_param_set(data, SI1145_PARAM_CHLIST, reg);
>>>
>>> -return ret < 0 ? ret : 0;
>>> +return min(ret, 0);
>>
>> A similar change involving max was already rejected.  ret < 0 is a
>> standard way of detecting an error, so perhaps leaving that explicitly
>> present will be preferred.
>
> I think a more sensible thing to do here is to check whether ret/err can
> actually take any positive values and if not, replace the whole thing with
> just 'return ret;' or 'return some_fn()'. I'd expect that his can be done in
> most of the cases.
>

[PATCH] iio: accel: bma180: Set up buffer timestamps for non-zero values

[simran singhal]

Use the iio_pollfunc_store_time parameter during triggered buffer set-up
to get valid timestamps.

Signed-off-by: simran singhal 
---
 drivers/iio/accel/bma180.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/accel/bma180.c b/drivers/iio/accel/bma180.c
index efc6773..4ae5c80 100644
--- a/drivers/iio/accel/bma180.c
+++ b/drivers/iio/accel/bma180.c
@@ -762,7 +762,7 @@ static int bma180_probe(struct i2c_client *client,
goto err_trigger_free;
}
 
-   ret = iio_triggered_buffer_setup(indio_dev, NULL,
+   ret = iio_triggered_buffer_setup(indio_dev, iio_pollfunc_store_time,
bma180_trigger_handler, NULL);
if (ret < 0) {
dev_err(&client->dev, "unable to setup iio triggered buffer\n");
-- 
2.7.4

[PATCH 3/4] iio: imu: st_lsm6dsx: Replace ternary operator with min macro

[simran singhal]

Use macro min() to get the minimum of two values for brevity and
readability.

Signed-off-by: simran singhal 
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c 
b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index e959825..e11653d 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -203,7 +203,7 @@ int st_lsm6dsx_update_watermark(struct st_lsm6dsx_sensor 
*sensor, u16 watermark)
 out:
mutex_unlock(&hw->lock);
 
-   return err < 0 ? err : 0;
+   return min(err, 0);
 }
 
 /**
-- 
2.7.4

[PATCH 4/4] iio: light: si1145: Replace ternary operator with min macro

[simran singhal]

Use macro min() to get the minimum of two values for brevity and
readability.

Signed-off-by: simran singhal 
---
 drivers/iio/light/si1145.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/si1145.c b/drivers/iio/light/si1145.c
index 096034c..e7ad6fb 100644
--- a/drivers/iio/light/si1145.c
+++ b/drivers/iio/light/si1145.c
@@ -557,7 +557,7 @@ static int si1145_set_chlist(struct iio_dev *indio_dev, 
unsigned long scan_mask)
data->scan_mask = scan_mask;
ret = si1145_param_set(data, SI1145_PARAM_CHLIST, reg);
 
-   return ret < 0 ? ret : 0;
+   return min(ret, 0);
 }
 
 static int si1145_measure(struct iio_dev *indio_dev,
-- 
2.7.4

[PATCH 2/4] iio: imu: st_lsm6dsx: Replace ternary operator with min macro

[simran singhal]

Use macro min() to get the minimum of two values for brevity and
readability.

Signed-off-by: simran singhal 
---
 drivers/iio/humidity/hts221_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/humidity/hts221_core.c 
b/drivers/iio/humidity/hts221_core.c
index 3f3ef4a1..f98240a 100644
--- a/drivers/iio/humidity/hts221_core.c
+++ b/drivers/iio/humidity/hts221_core.c
@@ -206,7 +206,7 @@ int hts221_config_drdy(struct hts221_hw *hw, bool enable)
err = hts221_write_with_mask(hw, HTS221_REG_CNTRL3_ADDR,
 HTS221_DRDY_MASK, val);
 
-   return err < 0 ? err : 0;
+   return min(err, 0);
 }
 
 static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
-- 
2.7.4

[PATCH 0/4] drivers: iio: Replace ternary operator with min macro

[simran singhal]

Use macro min() to get the minimum of two values for brevity and readability. 

simran singhal (4):
  iio: common: st_sensors: Replace ternary operator with min macro
  iio: imu: st_lsm6dsx: Replace ternary operator with min macro
  iio: imu: st_lsm6dsx: Replace ternary operator with min macro
  iio: light: si1145: Replace ternary operator with min macro

 drivers/iio/common/st_sensors/st_sensors_i2c.c | 2 +-
 drivers/iio/humidity/hts221_core.c | 2 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 2 +-
 drivers/iio/light/si1145.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.7.4

[PATCH 1/4] iio: common: st_sensors: Replace ternary operator with min macro

[simran singhal]

Use macro min() to get the minimum of two values for brevity and
readability.

Signed-off-by: simran singhal 
---
 drivers/iio/common/st_sensors/st_sensors_i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/common/st_sensors/st_sensors_i2c.c 
b/drivers/iio/common/st_sensors/st_sensors_i2c.c
index c83df4d..7a68fdd 100644
--- a/drivers/iio/common/st_sensors/st_sensors_i2c.c
+++ b/drivers/iio/common/st_sensors/st_sensors_i2c.c
@@ -39,7 +39,7 @@ static int st_sensors_i2c_read_byte(struct 
st_sensor_transfer_buffer *tb,
*res_byte = err & 0xff;
 
 st_accel_i2c_read_byte_error:
-   return err < 0 ? err : 0;
+   return min(err, 0);
 }
 
 static int st_sensors_i2c_read_multiple_byte(
-- 
2.7.4

Re: [PATCH v2] netfilter: Clean up tests if NULL returned on failure

[SIMRAN SINGHAL]

On Wed, Mar 29, 2017 at 2:19 PM, SIMRAN SINGHAL
 wrote:
> On Wed, Mar 29, 2017 at 12:25 PM, Jan Engelhardt  wrote:
>>
>> On Tuesday 2017-03-28 18:23, SIMRAN SINGHAL wrote:
>>>On Tue, Mar 28, 2017 at 7:24 PM, Jan Engelhardt  wrote:
>>>> On Tuesday 2017-03-28 15:13, simran singhal wrote:
>>>>
>>>>>Some functions like kmalloc/kzalloc return NULL on failure. When NULL
>>>>>represents failure, !x is commonly used.
>>>>>
>>>>>@@ -910,7 +910,7 @@ ip_vs_new_dest(struct ip_vs_service *svc, struct 
>>>>>ip_vs_dest_user_kern *udest,
>>>>>   }
>>>>>
>>>>>   dest = kzalloc(sizeof(struct ip_vs_dest), GFP_KERNEL);
>>>>>-  if (dest == NULL)
>>>>>+  if (!dest)
>>>>>   return -ENOMEM;
>>>>
>>>> This kind of transformation however is not cleanup anymore, it's really
>>>> bikeshedding and should be avoided. There are pro and cons for both
>>>> variants, and there is not really an overwhelming number of arguments
>>>> for either variant to justify the change.
>>>
>>>Sorry, but I didn't get what you are trying to convey. And particularly pros 
>>>and
>>>cons of both variants.
>>
>> The ==NULL/!=NULL part sort of ensures that the left side is a pointer, which
>> is lost when just using the variable and have it implicitly convert to bool.
>
> Thanks for the explaination
>
> But, according to me we should prefer != NULL over ==NULL according to
> coding style.

Sorry their is typing mistake in above.

But, according to me we should prefer !var over ( var ==NULL ) according to the
coding style

Re: [PATCH v2] netfilter: Clean up tests if NULL returned on failure

[SIMRAN SINGHAL]

On Wed, Mar 29, 2017 at 12:25 PM, Jan Engelhardt  wrote:
>
> On Tuesday 2017-03-28 18:23, SIMRAN SINGHAL wrote:
>>On Tue, Mar 28, 2017 at 7:24 PM, Jan Engelhardt  wrote:
>>> On Tuesday 2017-03-28 15:13, simran singhal wrote:
>>>
>>>>Some functions like kmalloc/kzalloc return NULL on failure. When NULL
>>>>represents failure, !x is commonly used.
>>>>
>>>>@@ -910,7 +910,7 @@ ip_vs_new_dest(struct ip_vs_service *svc, struct 
>>>>ip_vs_dest_user_kern *udest,
>>>>   }
>>>>
>>>>   dest = kzalloc(sizeof(struct ip_vs_dest), GFP_KERNEL);
>>>>-  if (dest == NULL)
>>>>+  if (!dest)
>>>>   return -ENOMEM;
>>>
>>> This kind of transformation however is not cleanup anymore, it's really
>>> bikeshedding and should be avoided. There are pro and cons for both
>>> variants, and there is not really an overwhelming number of arguments
>>> for either variant to justify the change.
>>
>>Sorry, but I didn't get what you are trying to convey. And particularly pros 
>>and
>>cons of both variants.
>
> The ==NULL/!=NULL part sort of ensures that the left side is a pointer, which
> is lost when just using the variable and have it implicitly convert to bool.

Thanks for the explaination

But, according to me we should prefer != NULL over ==NULL according to
coding style.

[PATCH] net: netfilter: Use list_{next/prev}_entry instead of list_entry

[simran singhal]

This patch replace list_entry with list_prev_entry as it makes the
code more clear to read.

Signed-off-by: simran singhal 
---
 net/netfilter/nf_tables_api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index b7645d7..a341eaf 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1895,7 +1895,7 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, 
struct net *net,
goto nla_put_failure;
 
if ((event != NFT_MSG_DELRULE) && (rule->list.prev != &chain->rules)) {
-   prule = list_entry(rule->list.prev, struct nft_rule, list);
+   prule = list_prev_entry(rule, list);
if (nla_put_be64(skb, NFTA_RULE_POSITION,
 cpu_to_be64(prule->handle),
 NFTA_RULE_PAD))
-- 
2.7.4

[PATCH] net: netfilter: Use seq_puts()/seq_putc() where possible

[simran singhal]

For string without format specifiers, use seq_puts(). For
seq_printf("\n"), use seq_putc('\n').

Signed-off-by: simran singhal 
---
 net/netfilter/ipvs/ip_vs_ctl.c  | 8 
 net/netfilter/nf_conntrack_expect.c | 4 ++--
 net/netfilter/nf_conntrack_standalone.c | 6 +++---
 net/netfilter/nf_log.c  | 4 ++--
 net/netfilter/nf_synproxy_core.c| 6 +++---
 net/netfilter/xt_recent.c   | 2 +-
 6 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index efe348a..c3423e3 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -2130,8 +2130,8 @@ static int ip_vs_stats_show(struct seq_file *seq, void *v)
 /*   01234567 01234567 01234567 0123456701234567 0123456701234567 
*/
seq_puts(seq,
 "   Total Incoming Outgoing Incoming 
Outgoing\n");
-   seq_printf(seq,
-  "   Conns  Packets  PacketsBytes
Bytes\n");
+   seq_puts(seq,
+"   Conns  Packets  PacketsBytes
Bytes\n");
 
ip_vs_copy_stats(&show, &net_ipvs(net)->tot_stats);
seq_printf(seq, "%8LX %8LX %8LX %16LX %16LX\n\n",
@@ -2178,8 +2178,8 @@ static int ip_vs_stats_percpu_show(struct seq_file *seq, 
void *v)
 /*   01234567 01234567 01234567 0123456701234567 0123456701234567 
*/
seq_puts(seq,
 "   Total Incoming Outgoing Incoming 
Outgoing\n");
-   seq_printf(seq,
-  "CPUConns  Packets  PacketsBytes
Bytes\n");
+   seq_puts(seq,
+"CPUConns  Packets  PacketsBytes
Bytes\n");
 
for_each_possible_cpu(i) {
struct ip_vs_cpu_stats *u = per_cpu_ptr(cpustats, i);
diff --git a/net/netfilter/nf_conntrack_expect.c 
b/net/netfilter/nf_conntrack_expect.c
index 4b2e1fb..74c384e 100644
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -549,7 +549,7 @@ static int exp_seq_show(struct seq_file *s, void *v)
seq_printf(s, "%ld ", timer_pending(&expect->timeout)
   ? (long)(expect->timeout.expires - jiffies)/HZ : 0);
else
-   seq_printf(s, "- ");
+   seq_puts(s, "- ");
seq_printf(s, "l3proto = %u proto=%u ",
   expect->tuple.src.l3num,
   expect->tuple.dst.protonum);
@@ -559,7 +559,7 @@ static int exp_seq_show(struct seq_file *s, void *v)
   expect->tuple.dst.protonum));
 
if (expect->flags & NF_CT_EXPECT_PERMANENT) {
-   seq_printf(s, "PERMANENT");
+   seq_puts(s, "PERMANENT");
delim = ",";
}
if (expect->flags & NF_CT_EXPECT_INACTIVE) {
diff --git a/net/netfilter/nf_conntrack_standalone.c 
b/net/netfilter/nf_conntrack_standalone.c
index 2256147..ccb5cb9 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -250,7 +250,7 @@ static int ct_seq_show(struct seq_file *s, void *v)
goto release;
 
if (!(test_bit(IPS_SEEN_REPLY_BIT, &ct->status)))
-   seq_printf(s, "[UNREPLIED] ");
+   seq_puts(s, "[UNREPLIED] ");
 
print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
l3proto, l4proto);
@@ -261,7 +261,7 @@ static int ct_seq_show(struct seq_file *s, void *v)
goto release;
 
if (test_bit(IPS_ASSURED_BIT, &ct->status))
-   seq_printf(s, "[ASSURED] ");
+   seq_puts(s, "[ASSURED] ");
 
if (seq_has_overflowed(s))
goto release;
@@ -350,7 +350,7 @@ static int ct_cpu_seq_show(struct seq_file *seq, void *v)
const struct ip_conntrack_stat *st = v;
 
if (v == SEQ_START_TOKEN) {
-   seq_printf(seq, "entries  searched found new invalid ignore 
delete delete_list insert insert_failed drop early_drop icmp_error  expect_new 
expect_create expect_delete search_restart\n");
+   seq_puts(seq, "entries  searched found new invalid ignore 
delete delete_list insert insert_failed drop early_drop icmp_error  expect_new 
expect_create expect_delete search_restart\n");
return 0;
}
 
diff --git a/net/netfilter/nf_log.c b/net/netfilter/nf_log.c
index 8d85a05..cc32727 100644
--- a/net/netfilter/nf_log.c
+++ b/net/netfilter/nf_log.c
@@ -376,13 +376,13 @@ static int seq_show(struct seq_file *s, void *v)
logger = nft_log_dereference(loggers[*pos][

[PATCH] iio: light: lm3533-als: constify attribute_group structures

[simran singhal]

Check for attribute_group structures that are only stored in the
event_attrs filed of iio_info structure. As the event_attrs field of
iio_info structures is constant, so these attribute_group structures can
also be declared constant. Done using coccinelle:

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct attribute_group i@p = {...};

@ok1@
identifier r1.i;
position p;
struct iio_info x;
@@
x.event_attrs=&i@p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct attribute_group i={...};

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct attribute_group i;

File size before:
   textdata bss dec hex filename
   57982376   081741fee drivers/iio/light/lm3533-als.o

File size after:
   textdata bss dec hex filename
   58622312   081741fee drivers/iio/light/lm3533-als.o

Signed-off-by: simran singhal 
---
 drivers/iio/light/lm3533-als.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
index f409c20..0bcc843 100644
--- a/drivers/iio/light/lm3533-als.c
+++ b/drivers/iio/light/lm3533-als.c
@@ -690,7 +690,7 @@ static struct attribute *lm3533_als_event_attributes[] = {
NULL
 };
 
-static struct attribute_group lm3533_als_event_attribute_group = {
+static const struct attribute_group lm3533_als_event_attribute_group = {
.attrs = lm3533_als_event_attributes
 };
 
-- 
2.7.4

[PATCH] iio: dac: ad5504: constify attribute_group structures

[simran singhal]

Check for attribute_group structures that are only stored in the
event_attrs filed of iio_info structure. As the event_attrs field of
iio_info structures is constant, so these attribute_group structures can
also be declared constant. Done using coccinelle:

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct attribute_group i@p = {...};

@ok1@
identifier r1.i;
position p;
struct iio_info x;
@@
x.event_attrs=&i@p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct attribute_group i={...};

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct attribute_group i;

File size before:
   textdata bss dec hex filename
   3046 360   03406 d4e drivers/iio/dac/ad5504.o

File size after:
   textdata bss dec hex filename
   3110 296   03406 d4e drivers/iio/dac/ad5504.o

Signed-off-by: simran singhal 
---
 drivers/iio/dac/ad5504.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index 788b3d6..22a027d 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -212,7 +212,7 @@ static struct attribute *ad5504_ev_attributes[] = {
NULL,
 };
 
-static struct attribute_group ad5504_ev_attribute_group = {
+static const struct attribute_group ad5504_ev_attribute_group = {
.attrs = ad5504_ev_attributes,
 };
 
-- 
2.7.4

[PATCH] iio: adc: max1363: constify attribute_group structures

[simran singhal]

Check for attribute_group structures that are only stored in the
event_attrs filed of iio_info structure. As the event_attrs field of
iio_info structures is constant, so these attribute_group structures can
also be declared constant. Done using coccinelle:

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct attribute_group i@p = {...};

@ok1@
identifier r1.i;
position p;
struct iio_info x;
@@
x.event_attrs=&i@p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct attribute_group i={...};

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct attribute_group i;

File size before:
   textdata bss dec hex filename
  36951 448   0   373999217 drivers/iio/adc/max1363.o

File size after:
   textdata bss dec hex filename
  37015 384   0   373999217 drivers/iio/adc/max1363.o

Signed-off-by: simran singhal 
---
 drivers/iio/adc/max1363.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
index c6c12fe..80eada4 100644
--- a/drivers/iio/adc/max1363.c
+++ b/drivers/iio/adc/max1363.c
@@ -1007,7 +1007,7 @@ static struct attribute *max1363_event_attributes[] = {
NULL,
 };
 
-static struct attribute_group max1363_event_attribute_group = {
+static const struct attribute_group max1363_event_attribute_group = {
.attrs = max1363_event_attributes,
 };
 
-- 
2.7.4

[PATCH] iio: adc: ad799x: constify attribute_group structures

[simran singhal]

Check for attribute_group structures that are only stored in the
event_attrs filed of iio_info structure. As the event_attrs field of
iio_info structures is constant, so these attribute_group structures can
also be declared constant.
Done using coccinelle:

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct attribute_group i@p = {...};

@ok1@
identifier r1.i;
position p;
struct iio_info x;
@@
x.event_attrs=&i@p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct attribute_group i={...};

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct attribute_group i;

File size before:
   textdata bss dec hex filename
  26051 464   0   265156793 drivers/iio/adc/ad799x.o

File size after:
   textdata bss dec hex filename
  26115 400   0   265156793 drivers/iio/adc/ad799x.o

Signed-off-by: simran singhal 
---
 drivers/iio/adc/ad799x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 9704090..22426ae 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -520,7 +520,7 @@ static struct attribute *ad799x_event_attributes[] = {
NULL,
 };
 
-static struct attribute_group ad799x_event_attrs_group = {
+static const struct attribute_group ad799x_event_attrs_group = {
.attrs = ad799x_event_attributes,
 };
 
-- 
2.7.4

[PATCH v2] net: Remove unnecessary cast on void pointer

[simran singhal]

The following Coccinelle script was used to detect this:
@r@
expression x;
void* e;
type T;
identifier f;
@@
(
  *((T *)e)
|
  ((T *)x)[...]
|
  ((T*)x)->f
|

- (T*)
  e
)

Unnecessary parantheses are also remove.

Signed-off-by: simran singhal 
---

 v2:
   -Remove unnecessary parantheses

 net/bridge/netfilter/ebtables.c |  2 +-
 net/ipv4/netfilter/arp_tables.c | 21 -
 net/ipv4/netfilter/ip_tables.c  | 20 
 net/ipv6/netfilter/ip6_tables.c | 20 
 net/netfilter/ipset/ip_set_bitmap_gen.h |  5 ++---
 net/netfilter/ipset/ip_set_core.c   |  2 +-
 net/netfilter/nf_conntrack_proto.c  |  2 +-
 net/netfilter/nft_set_hash.c|  2 +-
 net/netfilter/xt_hashlimit.c| 10 +-
 9 files changed, 35 insertions(+), 49 deletions(-)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 79b6991..bdc629e 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1713,7 +1713,7 @@ static int compat_copy_entry_to_user(struct ebt_entry *e, 
void __user **dstptr,
if (*size < sizeof(*ce))
return -EINVAL;
 
-   ce = (struct ebt_entry __user *)*dstptr;
+   ce = *dstptr;
if (copy_to_user(ce, e, sizeof(*ce)))
return -EFAULT;
 
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 6241a81..598e569 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -309,8 +309,7 @@ static int mark_source_chains(const struct xt_table_info 
*newinfo,
 */
for (hook = 0; hook < NF_ARP_NUMHOOKS; hook++) {
unsigned int pos = newinfo->hook_entry[hook];
-   struct arpt_entry *e
-   = (struct arpt_entry *)(entry0 + pos);
+   struct arpt_entry *e = entry0 + pos;
 
if (!(valid_hooks & (1 << hook)))
continue;
@@ -354,14 +353,12 @@ static int mark_source_chains(const struct xt_table_info 
*newinfo,
if (pos == oldpos)
goto next;
 
-   e = (struct arpt_entry *)
-   (entry0 + pos);
+   e = entry0 + pos;
} while (oldpos == pos + e->next_offset);
 
/* Move along one */
size = e->next_offset;
-   e = (struct arpt_entry *)
-   (entry0 + pos + size);
+   e = entry0 + pos + size;
if (pos + size >= newinfo->size)
return 0;
e->counters.pcnt = pos;
@@ -376,16 +373,14 @@ static int mark_source_chains(const struct xt_table_info 
*newinfo,
if (!xt_find_jump_offset(offsets, 
newpos,
 
newinfo->number))
return 0;
-   e = (struct arpt_entry *)
-   (entry0 + newpos);
+   e = entry0 + newpos;
} else {
/* ... this is a fallthru */
newpos = pos + e->next_offset;
if (newpos >= newinfo->size)
return 0;
}
-   e = (struct arpt_entry *)
-   (entry0 + newpos);
+   e = entry0 + newpos;
e->counters.pcnt = pos;
pos = newpos;
}
@@ -683,7 +678,7 @@ static int copy_entries_to_user(unsigned int total_size,
for (off = 0, num = 0; off < total_size; off += e->next_offset, num++){
const struct xt_entry_target *t;
 
-   e = (struct arpt_entry *)(loc_cpu_entry + off);
+   e = loc_cpu_entry + off;
if (copy_to_user(userptr + off, e, sizeof(*e))) {
ret = -EFAULT;
goto free_counters;
@@ -1130,7 +1125,7 @@ compat_copy_entry_from_user(struct compat_arpt_entry *e, 
void **dstptr,
int h;
 
origsize = *size;
-   de = (struct arpt_entry *)*dstptr;
+   de = *dstptr;
memcpy(de, e, sizeof(struct arpt_entry));
memcpy(&de->counters, &e->counters, sizeof(e->counters));
 
@@ -1324,7 +1319,7 @@ static int compat_copy

Re: [Outreachy kernel] [PATCH v4] staging: iio: ade7753: Replace mlock with driver private lock

[SIMRAN SINGHAL]

On Fri, Mar 24, 2017 at 12:51 AM, Alison Schofield  wrote:
> On Fri, Mar 24, 2017 at 12:05:20AM +0530, simran singhal wrote:
>> The IIO subsystem is redefining iio_dev->mlock to be used by
>> the IIO core only for protecting device operating mode changes.
>> ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
>>
>> In this driver, mlock was being used to protect hardware state
>> changes.  Replace it with a lock in the devices global data.
>
> Hi Simran,
>
> Please post all revision histories below the --- not just the most
> recent.
>
Sorry, will not repeat this.

> Does this lock enforce the needed "atomicity" in the write_frequency
> function? I read Jonathans comment on a previous revision about
> "ensuring the spi bus frequency and sampling frequency of the device
> are changed in an atomic fashion"
>

By introducing another lock I am protecting read_modify_write and
in this way also protecting the designated register that we are about
to write.

> Is it possible for another spi bus transaction (read or write) to
> occur between the read and write in write_frequency?
>

Gargi has also come up with a solution.
https://groups.google.com/forum/#!topic/outreachy-kernel/kzE9CrI5Bd8

Should I do like her as her's also seem correct or go ahead with this.

> alisons
>>
>> Signed-off-by: simran singhal 
>> ---
>>
>>  v4:
>>-Add mutex_init
>>
>>  drivers/staging/iio/meter/ade7753.c | 7 +--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/iio/meter/ade7753.c 
>> b/drivers/staging/iio/meter/ade7753.c
>> index b71fbd3..30aebaf 100644
>> --- a/drivers/staging/iio/meter/ade7753.c
>> +++ b/drivers/staging/iio/meter/ade7753.c
>> @@ -80,11 +80,13 @@
>>   * @us: actual spi_device
>>   * @tx: transmit buffer
>>   * @rx: receive buffer
>> + * @lock:   protect sensor state
>>   * @buf_lock:   mutex to protect tx and rx
>>   **/
>>  struct ade7753_state {
>>   struct spi_device   *us;
>>   struct mutexbuf_lock;
>> + struct mutexlock;  /* protect sensor state */
>>   u8  tx[ADE7753_MAX_TX] cacheline_aligned;
>>   u8  rx[ADE7753_MAX_RX];
>>  };
>> @@ -484,7 +486,7 @@ static ssize_t ade7753_write_frequency(struct device 
>> *dev,
>>   if (!val)
>>   return -EINVAL;
>>
>> - mutex_lock(&indio_dev->mlock);
>> + mutex_lock(&st->lock);
>>
>>   t = 27900 / val;
>>   if (t > 0)
>> @@ -505,7 +507,7 @@ static ssize_t ade7753_write_frequency(struct device 
>> *dev,
>>   ret = ade7753_spi_write_reg_16(dev, ADE7753_MODE, reg);
>>
>>  out:
>> - mutex_unlock(&indio_dev->mlock);
>> + mutex_unlock(&st->lock);
>>
>>   return ret ? ret : len;
>>  }
>> @@ -581,6 +583,7 @@ static int ade7753_probe(struct spi_device *spi)
>>   st = iio_priv(indio_dev);
>>   st->us = spi;
>>   mutex_init(&st->buf_lock);
>> + mutex_init(&st->lock);
>>
>>   indio_dev->name = spi->dev.driver->name;
>>   indio_dev->dev.parent = &spi->dev;
>> --
>> 2.7.4
>>
>> --
>> You received this message because you are subscribed to the Google Groups 
>> "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to outreachy-kernel+unsubscr...@googlegroups.com.
>> To post to this group, send email to outreachy-ker...@googlegroups.com.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/outreachy-kernel/20170323183520.GA9871%40singhal-Inspiron-5558.
>> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v2] netfilter: Clean up tests if NULL returned on failure

[SIMRAN SINGHAL]

On Tue, Mar 28, 2017 at 7:24 PM, Jan Engelhardt  wrote:
> On Tuesday 2017-03-28 15:13, simran singhal wrote:
>
>>Some functions like kmalloc/kzalloc return NULL on failure. When NULL
>>represents failure, !x is commonly used.
>>
>>@@ -910,7 +910,7 @@ ip_vs_new_dest(struct ip_vs_service *svc, struct 
>>ip_vs_dest_user_kern *udest,
>>   }
>>
>>   dest = kzalloc(sizeof(struct ip_vs_dest), GFP_KERNEL);
>>-  if (dest == NULL)
>>+  if (!dest)
>>   return -ENOMEM;
>
> This kind of transformation however is not cleanup anymore, it's really
> bikeshedding and should be avoided. There are pro and cons for both
> variants, and there is not really an overwhelming number of arguments
> for either variant to justify the change.

Sorry, but I didn't get what you are trying to convey. And particularly pros and
cons of both variants.

[PATCH] netfilter: ipset: Use max macro instead of ternary operator

[simran singhal]

This patch replaces ternary operator with macro max as it shorter and
thus increases code readability. Macro max return the maximum of the two
compared values.

Signed-off-by: simran singhal 
---
 net/netfilter/ipset/ip_set_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/ipset/ip_set_core.c 
b/net/netfilter/ipset/ip_set_core.c
index 10a3694..248fef7 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -571,7 +571,7 @@ ip_set_test(ip_set_id_t index, const struct sk_buff *skb,
}
 
/* Convert error codes to nomatch */
-   return (ret < 0 ? 0 : ret);
+   return max(0, ret);
 }
 EXPORT_SYMBOL_GPL(ip_set_test);
 
-- 
2.7.4

[PATCH v2] netfilter: Clean up tests if NULL returned on failure

[simran singhal]

Some functions like kmalloc/kzalloc return NULL on failure. When NULL
represents failure, !x is commonly used.

Signed-off-by: simran singhal 
---
 
 v2:
   -squash all the patches of the patch-set.
 
 net/netfilter/ipvs/ip_vs_ctl.c   |  4 ++--
 net/netfilter/ipvs/ip_vs_dh.c|  2 +-
 net/netfilter/ipvs/ip_vs_lblc.c  |  2 +-
 net/netfilter/ipvs/ip_vs_lblcr.c |  4 ++--
 net/netfilter/ipvs/ip_vs_sh.c|  2 +-
 net/netfilter/ipvs/ip_vs_wrr.c   |  2 +-
 net/netfilter/nf_conntrack_netlink.c |  2 +-
 net/netfilter/nf_conntrack_proto.c   |  2 +-
 net/netfilter/nf_nat_core.c  |  2 +-
 net/netfilter/nf_tables_api.c| 24 
 net/netfilter/nfnetlink.c|  2 +-
 net/netfilter/xt_TEE.c   |  2 +-
 12 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 5aeb0dd..efe348a 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -910,7 +910,7 @@ ip_vs_new_dest(struct ip_vs_service *svc, struct 
ip_vs_dest_user_kern *udest,
}
 
dest = kzalloc(sizeof(struct ip_vs_dest), GFP_KERNEL);
-   if (dest == NULL)
+   if (!dest)
return -ENOMEM;
 
dest->stats.cpustats = alloc_percpu(struct ip_vs_cpu_stats);
@@ -1228,7 +1228,7 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct 
ip_vs_service_user_kern *u,
 #endif
 
svc = kzalloc(sizeof(struct ip_vs_service), GFP_KERNEL);
-   if (svc == NULL) {
+   if (!svc) {
IP_VS_DBG(1, "%s(): no memory\n", __func__);
ret = -ENOMEM;
goto out_err;
diff --git a/net/netfilter/ipvs/ip_vs_dh.c b/net/netfilter/ipvs/ip_vs_dh.c
index 75f798f..22a2535 100644
--- a/net/netfilter/ipvs/ip_vs_dh.c
+++ b/net/netfilter/ipvs/ip_vs_dh.c
@@ -159,7 +159,7 @@ static int ip_vs_dh_init_svc(struct ip_vs_service *svc)
 
/* allocate the DH table for this service */
s = kzalloc(sizeof(struct ip_vs_dh_state), GFP_KERNEL);
-   if (s == NULL)
+   if (!s)
return -ENOMEM;
 
svc->sched_data = s;
diff --git a/net/netfilter/ipvs/ip_vs_lblc.c b/net/netfilter/ipvs/ip_vs_lblc.c
index 5824927..d7c6031 100644
--- a/net/netfilter/ipvs/ip_vs_lblc.c
+++ b/net/netfilter/ipvs/ip_vs_lblc.c
@@ -352,7 +352,7 @@ static int ip_vs_lblc_init_svc(struct ip_vs_service *svc)
 *Allocate the ip_vs_lblc_table for this service
 */
tbl = kmalloc(sizeof(*tbl), GFP_KERNEL);
-   if (tbl == NULL)
+   if (!tbl)
return -ENOMEM;
 
svc->sched_data = tbl;
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index 703f118..b0a9e1c 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -113,7 +113,7 @@ static void ip_vs_dest_set_insert(struct ip_vs_dest_set 
*set,
}
 
e = kmalloc(sizeof(*e), GFP_ATOMIC);
-   if (e == NULL)
+   if (!e)
return;
 
ip_vs_dest_hold(dest);
@@ -515,7 +515,7 @@ static int ip_vs_lblcr_init_svc(struct ip_vs_service *svc)
 *Allocate the ip_vs_lblcr_table for this service
 */
tbl = kmalloc(sizeof(*tbl), GFP_KERNEL);
-   if (tbl == NULL)
+   if (!tbl)
return -ENOMEM;
 
svc->sched_data = tbl;
diff --git a/net/netfilter/ipvs/ip_vs_sh.c b/net/netfilter/ipvs/ip_vs_sh.c
index 16aaac6..99f3c3e 100644
--- a/net/netfilter/ipvs/ip_vs_sh.c
+++ b/net/netfilter/ipvs/ip_vs_sh.c
@@ -235,7 +235,7 @@ static int ip_vs_sh_init_svc(struct ip_vs_service *svc)
 
/* allocate the SH table for this service */
s = kzalloc(sizeof(struct ip_vs_sh_state), GFP_KERNEL);
-   if (s == NULL)
+   if (!s)
return -ENOMEM;
 
svc->sched_data = s;
diff --git a/net/netfilter/ipvs/ip_vs_wrr.c b/net/netfilter/ipvs/ip_vs_wrr.c
index 17e6d44..0923e6c 100644
--- a/net/netfilter/ipvs/ip_vs_wrr.c
+++ b/net/netfilter/ipvs/ip_vs_wrr.c
@@ -116,7 +116,7 @@ static int ip_vs_wrr_init_svc(struct ip_vs_service *svc)
 *Allocate the mark variable for WRR scheduling
 */
mark = kmalloc(sizeof(struct ip_vs_wrr_mark), GFP_KERNEL);
-   if (mark == NULL)
+   if (!mark)
return -ENOMEM;
 
mark->cl = list_entry(&svc->destinations, struct ip_vs_dest, n_list);
diff --git a/net/netfilter/nf_conntrack_netlink.c 
b/net/netfilter/nf_conntrack_netlink.c
index 6806b5e..cdcf4c1 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -779,7 +779,7 @@ ctnetlink_alloc_filter(const struct nlattr * const cda[])
struct ctnetlink_filter *filter;
 
filter = kzalloc(sizeof(*filter), GFP_KERNEL);
-   if (filter == NULL)
+   if (!filter)
return ERR_PTR(-ENOMEM);
 
filter->mark.val = ntohl(nla_get_be32(cda[CTA_MARK]));
diff --git a

[PATCH] net: Remove unnecessary cast on void pointer

[simran singhal]

The following Coccinelle script was used to detect this:
@r@
expression x;
void* e;
type T;
identifier f;
@@
(
  *((T *)e)
|
  ((T *)x)[...]
|
  ((T*)x)->f
|

- (T*)
  e
)

Signed-off-by: simran singhal 
---
 net/bridge/netfilter/ebtables.c |  2 +-
 net/ipv4/netfilter/arp_tables.c | 20 
 net/ipv4/netfilter/ip_tables.c  | 20 
 net/ipv6/netfilter/ip6_tables.c | 20 
 net/netfilter/ipset/ip_set_bitmap_gen.h |  4 ++--
 net/netfilter/ipset/ip_set_core.c   |  2 +-
 net/netfilter/nf_conntrack_proto.c  |  2 +-
 net/netfilter/nft_set_hash.c|  2 +-
 net/netfilter/xt_hashlimit.c| 10 +-
 9 files changed, 35 insertions(+), 47 deletions(-)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 79b6991..bdc629e 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1713,7 +1713,7 @@ static int compat_copy_entry_to_user(struct ebt_entry *e, 
void __user **dstptr,
if (*size < sizeof(*ce))
return -EINVAL;
 
-   ce = (struct ebt_entry __user *)*dstptr;
+   ce = *dstptr;
if (copy_to_user(ce, e, sizeof(*ce)))
return -EFAULT;
 
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 6241a81..f046c12 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -310,7 +310,7 @@ static int mark_source_chains(const struct xt_table_info 
*newinfo,
for (hook = 0; hook < NF_ARP_NUMHOOKS; hook++) {
unsigned int pos = newinfo->hook_entry[hook];
struct arpt_entry *e
-   = (struct arpt_entry *)(entry0 + pos);
+   = (entry0 + pos);
 
if (!(valid_hooks & (1 << hook)))
continue;
@@ -354,14 +354,12 @@ static int mark_source_chains(const struct xt_table_info 
*newinfo,
if (pos == oldpos)
goto next;
 
-   e = (struct arpt_entry *)
-   (entry0 + pos);
+   e = (entry0 + pos);
} while (oldpos == pos + e->next_offset);
 
/* Move along one */
size = e->next_offset;
-   e = (struct arpt_entry *)
-   (entry0 + pos + size);
+   e = (entry0 + pos + size);
if (pos + size >= newinfo->size)
return 0;
e->counters.pcnt = pos;
@@ -376,16 +374,14 @@ static int mark_source_chains(const struct xt_table_info 
*newinfo,
if (!xt_find_jump_offset(offsets, 
newpos,
 
newinfo->number))
return 0;
-   e = (struct arpt_entry *)
-   (entry0 + newpos);
+   e = (entry0 + newpos);
} else {
/* ... this is a fallthru */
newpos = pos + e->next_offset;
if (newpos >= newinfo->size)
return 0;
}
-   e = (struct arpt_entry *)
-   (entry0 + newpos);
+   e = (entry0 + newpos);
e->counters.pcnt = pos;
pos = newpos;
}
@@ -683,7 +679,7 @@ static int copy_entries_to_user(unsigned int total_size,
for (off = 0, num = 0; off < total_size; off += e->next_offset, num++){
const struct xt_entry_target *t;
 
-   e = (struct arpt_entry *)(loc_cpu_entry + off);
+   e = (loc_cpu_entry + off);
if (copy_to_user(userptr + off, e, sizeof(*e))) {
ret = -EFAULT;
goto free_counters;
@@ -1130,7 +1126,7 @@ compat_copy_entry_from_user(struct compat_arpt_entry *e, 
void **dstptr,
int h;
 
origsize = *size;
-   de = (struct arpt_entry *)*dstptr;
+   de = *dstptr;
memcpy(de, e, sizeof(struct arpt_entry));
memcpy(&de->counters, &e->counters, sizeof(e->counters));
 
@@ -1324,7 +1320,7 @@ static int compat_copy_entry_to_user(struct arpt_entry 
*e, void __user **dstptr,
int ret;
 
origsize = *size;
- 

[PATCH v3] netfilter: Compress return logic

[simran singhal]

Simplify function returns by merging assignment and return into one
statement.

Signed-off-by: simran singhal 
---
 v3:
   -change commit message.
   -merge two patches into one.
 v2:
   -Change the subject of cover patch

 net/netfilter/ipset/ip_set_list_set.c | 5 +
 net/netfilter/ipvs/ip_vs_ftp.c| 5 +
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_list_set.c 
b/net/netfilter/ipset/ip_set_list_set.c
index 178d4eb..2fff6b5 100644
--- a/net/netfilter/ipset/ip_set_list_set.c
+++ b/net/netfilter/ipset/ip_set_list_set.c
@@ -453,7 +453,6 @@ static size_t
 list_set_memsize(const struct list_set *map, size_t dsize)
 {
struct set_elem *e;
-   size_t memsize;
u32 n = 0;
 
rcu_read_lock();
@@ -461,9 +460,7 @@ list_set_memsize(const struct list_set *map, size_t dsize)
n++;
rcu_read_unlock();
 
-   memsize = sizeof(*map) + n * dsize;
-
-   return memsize;
+   return (sizeof(*map) + n * dsize);
 }
 
 static int
diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index d30c327..c93c937 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -482,11 +482,8 @@ static struct pernet_operations ip_vs_ftp_ops = {
 
 static int __init ip_vs_ftp_init(void)
 {
-   int rv;
-
-   rv = register_pernet_subsys(&ip_vs_ftp_ops);
/* rcu_barrier() is called by netns on error */
-   return rv;
+   return register_pernet_subsys(&ip_vs_ftp_ops);
 }
 
 /*
-- 
2.7.4

Re: [PATCH v9] staging: iio: adis16060: Remove iio_dev mlock and refactor code

[SIMRAN SINGHAL]

On Sat, Mar 25, 2017 at 11:16 PM, Jonathan Cameron  wrote:
> On 23/03/17 09:20, simran singhal wrote:
>> The IIO subsystem is redefining iio_dev->mlock to be used by
>> the IIO core only for protecting device operating mode changes.
>> ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
>>
>> In this driver, mlock was being used to protect hardware state changes.
>>
>> In the driver, buf_lock protects both the adis16060_spi_write() and
>> adis16060_spi_read() functions and both are always called in pair.
>> First write, then read. Refactor the code to have one single function
>> adis16060_spi_write_then_read() protected by the buf_lock. This removes
>> the need for additional locking via mlock, so this locking is removed.
>>
>> Signed-off-by: simran singhal 
> Unfortunately I've now pushed out v8 as togreg which is a non rebasing tree.
> As such, could you post a separate patch, on top of that making the
> than -> then change?
>

ok, I will send the patch with than-> then change.

> Thanks,
>
> Jonathan
>> ---
>>
>>  v9:
>>-Change the name of function from adis16060_spi_write_than_read()
>> to adis16060_spi_write_then_read(). change "than" to "then" as
>> its time depended.
>>-Add mutex_unlock
> As Alison keeps mentioning, ideal is to keep all previous change logs
> here as it allows those who last looked at say v5 to know what changed
> in between without reading a lot of threads!
>>

Sorry, will not repeate this in future.

>>  drivers/staging/iio/gyro/adis16060_core.c | 35 
>> +--
>>  1 file changed, 10 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/staging/iio/gyro/adis16060_core.c 
>> b/drivers/staging/iio/gyro/adis16060_core.c
>> index c9d46e7..9675245 100644
>> --- a/drivers/staging/iio/gyro/adis16060_core.c
>> +++ b/drivers/staging/iio/gyro/adis16060_core.c
>> @@ -40,25 +40,20 @@ struct adis16060_state {
>>
>>  static struct iio_dev *adis16060_iio_dev;
>>
>> -static int adis16060_spi_write(struct iio_dev *indio_dev, u8 val)
>> +static int adis16060_spi_write_then_read(struct iio_dev *indio_dev,
>> +  u8 conf, u16 *val)
>>  {
>>   int ret;
>>   struct adis16060_state *st = iio_priv(indio_dev);
>>
>>   mutex_lock(&st->buf_lock);
>> - st->buf[2] = val; /* The last 8 bits clocked in are latched */
>> + st->buf[2] = conf; /* The last 8 bits clocked in are latched */
>>   ret = spi_write(st->us_w, st->buf, 3);
>> - mutex_unlock(&st->buf_lock);
>> -
>> - return ret;
>> -}
>> -
>> -static int adis16060_spi_read(struct iio_dev *indio_dev, u16 *val)
>> -{
>> - int ret;
>> - struct adis16060_state *st = iio_priv(indio_dev);
>>
>> - mutex_lock(&st->buf_lock);
>> + if (ret < 0) {
>> + mutex_unlock(&st->buf_lock);
>> + return ret;
>> + }
>>
>>   ret = spi_read(st->us_r, st->buf, 3);
>>
>> @@ -86,17 +81,11 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
>>
>>   switch (mask) {
>>   case IIO_CHAN_INFO_RAW:
>> - /* Take the iio_dev status lock */
>> - mutex_lock(&indio_dev->mlock);
>> - ret = adis16060_spi_write(indio_dev, chan->address);
>> + ret = adis16060_spi_write_then_read(indio_dev,
>> + chan->address, &tval);
>>   if (ret < 0)
>> - goto out_unlock;
>> + return ret;
>>
>> - ret = adis16060_spi_read(indio_dev, &tval);
>> - if (ret < 0)
>> - goto out_unlock;
>> -
>> - mutex_unlock(&indio_dev->mlock);
>>   *val = tval;
>>   return IIO_VAL_INT;
>>   case IIO_CHAN_INFO_OFFSET:
>> @@ -110,10 +99,6 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
>>   }
>>
>>   return -EINVAL;
>> -
>> -out_unlock:
>> - mutex_unlock(&indio_dev->mlock);
>> - return ret;
>>  }
>>
>>  static const struct iio_info adis16060_info = {
>>
>

[PATCH v4] staging: iio: ade7753: Replace mlock with driver private lock

[simran singhal]

The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.

In this driver, mlock was being used to protect hardware state
changes.  Replace it with a lock in the devices global data.

Signed-off-by: simran singhal 
---

 v4:
   -Add mutex_init

 drivers/staging/iio/meter/ade7753.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7753.c 
b/drivers/staging/iio/meter/ade7753.c
index b71fbd3..30aebaf 100644
--- a/drivers/staging/iio/meter/ade7753.c
+++ b/drivers/staging/iio/meter/ade7753.c
@@ -80,11 +80,13 @@
  * @us: actual spi_device
  * @tx: transmit buffer
  * @rx: receive buffer
+ * @lock:   protect sensor state
  * @buf_lock:   mutex to protect tx and rx
  **/
 struct ade7753_state {
struct spi_device   *us;
struct mutexbuf_lock;
+   struct mutexlock;  /* protect sensor state */
u8  tx[ADE7753_MAX_TX] cacheline_aligned;
u8  rx[ADE7753_MAX_RX];
 };
@@ -484,7 +486,7 @@ static ssize_t ade7753_write_frequency(struct device *dev,
if (!val)
return -EINVAL;
 
-   mutex_lock(&indio_dev->mlock);
+   mutex_lock(&st->lock);
 
t = 27900 / val;
if (t > 0)
@@ -505,7 +507,7 @@ static ssize_t ade7753_write_frequency(struct device *dev,
ret = ade7753_spi_write_reg_16(dev, ADE7753_MODE, reg);
 
 out:
-   mutex_unlock(&indio_dev->mlock);
+   mutex_unlock(&st->lock);
 
return ret ? ret : len;
 }
@@ -581,6 +583,7 @@ static int ade7753_probe(struct spi_device *spi)
st = iio_priv(indio_dev);
st->us = spi;
mutex_init(&st->buf_lock);
+   mutex_init(&st->lock);
 
indio_dev->name = spi->dev.driver->name;
indio_dev->dev.parent = &spi->dev;
-- 
2.7.4

Re: [PATCH v3 2/2] staging: iio: ade7753: Replace mlock with driver private lock

[SIMRAN SINGHAL]

On Thu, Mar 23, 2017 at 1:55 AM, Jonathan Cameron  wrote:
> On 21/03/17 18:03, simran singhal wrote:
>> The IIO subsystem is redefining iio_dev->mlock to be used by
>> the IIO core only for protecting device operating mode changes.
>> ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
>>
>> In this driver, mlock was being used to protect hardware state
>> changes.  Replace it with a lock in the devices global data.
>>
>> Signed-off-by: simran singhal 
> Mutex is not initialized...

Mutex is already initialized in ade7753_probe().

>> ---
>>  drivers/staging/iio/meter/ade7753.c | 6 --
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/iio/meter/ade7753.c 
>> b/drivers/staging/iio/meter/ade7753.c
>> index b71fbd3..9674e05 100644
>> --- a/drivers/staging/iio/meter/ade7753.c
>> +++ b/drivers/staging/iio/meter/ade7753.c
>> @@ -80,11 +80,13 @@
>>   * @us: actual spi_device
>>   * @tx: transmit buffer
>>   * @rx: receive buffer
>> + * @lock:   protect sensor state
>>   * @buf_lock:   mutex to protect tx and rx
>>   **/
>>  struct ade7753_state {
>>   struct spi_device   *us;
>>   struct mutexbuf_lock;
>> + struct mutexlock;  /* protect sensor state */
>>   u8  tx[ADE7753_MAX_TX] cacheline_aligned;
>>   u8  rx[ADE7753_MAX_RX];
>>  };
>> @@ -484,7 +486,7 @@ static ssize_t ade7753_write_frequency(struct device 
>> *dev,
>>   if (!val)
>>   return -EINVAL;
>>
>> - mutex_lock(&indio_dev->mlock);
>> + mutex_lock(&st->lock);
>>
>>   t = 27900 / val;
>>   if (t > 0)
>> @@ -505,7 +507,7 @@ static ssize_t ade7753_write_frequency(struct device 
>> *dev,
>>   ret = ade7753_spi_write_reg_16(dev, ADE7753_MODE, reg);
>>
>>  out:
>> - mutex_unlock(&indio_dev->mlock);
>> + mutex_unlock(&st->lock);
>>
>>   return ret ? ret : len;
>>  }
>>
>

[PATCH v9] staging: iio: adis16060: Remove iio_dev mlock and refactor code

[simran singhal]

The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.

In this driver, mlock was being used to protect hardware state changes.

In the driver, buf_lock protects both the adis16060_spi_write() and
adis16060_spi_read() functions and both are always called in pair.
First write, then read. Refactor the code to have one single function
adis16060_spi_write_then_read() protected by the buf_lock. This removes
the need for additional locking via mlock, so this locking is removed.

Signed-off-by: simran singhal 
---

 v9:
   -Change the name of function from adis16060_spi_write_than_read()
to adis16060_spi_write_then_read(). change "than" to "then" as
its time depended.
   -Add mutex_unlock
 
 drivers/staging/iio/gyro/adis16060_core.c | 35 +--
 1 file changed, 10 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/iio/gyro/adis16060_core.c 
b/drivers/staging/iio/gyro/adis16060_core.c
index c9d46e7..9675245 100644
--- a/drivers/staging/iio/gyro/adis16060_core.c
+++ b/drivers/staging/iio/gyro/adis16060_core.c
@@ -40,25 +40,20 @@ struct adis16060_state {
 
 static struct iio_dev *adis16060_iio_dev;
 
-static int adis16060_spi_write(struct iio_dev *indio_dev, u8 val)
+static int adis16060_spi_write_then_read(struct iio_dev *indio_dev,
+u8 conf, u16 *val)
 {
int ret;
struct adis16060_state *st = iio_priv(indio_dev);
 
mutex_lock(&st->buf_lock);
-   st->buf[2] = val; /* The last 8 bits clocked in are latched */
+   st->buf[2] = conf; /* The last 8 bits clocked in are latched */
ret = spi_write(st->us_w, st->buf, 3);
-   mutex_unlock(&st->buf_lock);
-
-   return ret;
-}
-
-static int adis16060_spi_read(struct iio_dev *indio_dev, u16 *val)
-{
-   int ret;
-   struct adis16060_state *st = iio_priv(indio_dev);
 
-   mutex_lock(&st->buf_lock);
+   if (ret < 0) {
+   mutex_unlock(&st->buf_lock);
+   return ret;
+   }
 
ret = spi_read(st->us_r, st->buf, 3);
 
@@ -86,17 +81,11 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
 
switch (mask) {
case IIO_CHAN_INFO_RAW:
-   /* Take the iio_dev status lock */
-   mutex_lock(&indio_dev->mlock);
-   ret = adis16060_spi_write(indio_dev, chan->address);
+   ret = adis16060_spi_write_then_read(indio_dev,
+   chan->address, &tval);
if (ret < 0)
-   goto out_unlock;
+   return ret;
 
-   ret = adis16060_spi_read(indio_dev, &tval);
-   if (ret < 0)
-   goto out_unlock;
-
-   mutex_unlock(&indio_dev->mlock);
*val = tval;
return IIO_VAL_INT;
case IIO_CHAN_INFO_OFFSET:
@@ -110,10 +99,6 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
}
 
return -EINVAL;
-
-out_unlock:
-   mutex_unlock(&indio_dev->mlock);
-   return ret;
 }
 
 static const struct iio_info adis16060_info = {
-- 
2.7.4

[PATCH v8] staging: adis16060: Remove iio_dev mlock and refactor code

[simran singhal]

The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.

In this driver, mlock was being used to protect hardware state changes.

In the driver, buf_lock protects both the adis16060_spi_write() and
adis16060_spi_read() functions and both are always called in
pair. First write, then read. Refactor the code to have
one single function adis16060_spi_write_than_read() and remove
the use of mlock as adis16060_read_raw() does not require an
iio_dev->mlock for reads.

Signed-off-by: simran singhal 
---

 v8:
   -change subject
   -change commit message
 v7:
   -Change subject
   -Remove lock from read_raw instead from
function adis16060_spi_write_than_read().
 v6:
   -Change commit message
   -Remove nested lock
 v5:
   -Rename val in adis16060_spi_write_than_read() to conf.
   -Rename val2 in adis16060_spi_write_than_read() to val.
   -Corrected Checkpatch issues.
   -Removed goto from adis16060_read_raw().
 v4:
   -Refactored code
   -change commit subject
   -change commit message
 v3:
   -Removed new lock to reuse the existing lock
 v2:
   -Fixed compilation error

 drivers/staging/iio/gyro/adis16060_core.c | 33 ---
 1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/iio/gyro/adis16060_core.c 
b/drivers/staging/iio/gyro/adis16060_core.c
index c9d46e7..e96fce5 100644
--- a/drivers/staging/iio/gyro/adis16060_core.c
+++ b/drivers/staging/iio/gyro/adis16060_core.c
@@ -40,25 +40,18 @@ struct adis16060_state {
 
 static struct iio_dev *adis16060_iio_dev;
 
-static int adis16060_spi_write(struct iio_dev *indio_dev, u8 val)
+static int adis16060_spi_write_than_read(struct iio_dev *indio_dev,
+u8 conf, u16 *val)
 {
int ret;
struct adis16060_state *st = iio_priv(indio_dev);
 
mutex_lock(&st->buf_lock);
-   st->buf[2] = val; /* The last 8 bits clocked in are latched */
+   st->buf[2] = conf; /* The last 8 bits clocked in are latched */
ret = spi_write(st->us_w, st->buf, 3);
-   mutex_unlock(&st->buf_lock);
-
-   return ret;
-}
-
-static int adis16060_spi_read(struct iio_dev *indio_dev, u16 *val)
-{
-   int ret;
-   struct adis16060_state *st = iio_priv(indio_dev);
 
-   mutex_lock(&st->buf_lock);
+   if (ret < 0)
+   return ret;
 
ret = spi_read(st->us_r, st->buf, 3);
 
@@ -86,17 +79,11 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
 
switch (mask) {
case IIO_CHAN_INFO_RAW:
-   /* Take the iio_dev status lock */
-   mutex_lock(&indio_dev->mlock);
-   ret = adis16060_spi_write(indio_dev, chan->address);
+   ret = adis16060_spi_write_than_read(indio_dev,
+   chan->address, &tval);
if (ret < 0)
-   goto out_unlock;
+   return ret;
 
-   ret = adis16060_spi_read(indio_dev, &tval);
-   if (ret < 0)
-   goto out_unlock;
-
-   mutex_unlock(&indio_dev->mlock);
*val = tval;
return IIO_VAL_INT;
case IIO_CHAN_INFO_OFFSET:
@@ -110,10 +97,6 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
}
 
return -EINVAL;
-
-out_unlock:
-   mutex_unlock(&indio_dev->mlock);
-   return ret;
 }
 
 static const struct iio_info adis16060_info = {
-- 
2.7.4

Re: [PATCH 0/5] netfilter: Clean up tests if NULL returned on failure

[SIMRAN SINGHAL]

On Wed, Mar 22, 2017 at 7:08 PM, Pablo Neira Ayuso  wrote:
> On Tue, Mar 21, 2017 at 02:14:34PM +0530, simran singhal wrote:
>> This patch series clean up tests if NULL returned on failure.
>
> $ git grep "== NULL" net/netfilter/ | wc -l
> 461
>
> This is cleaning up just some of them, we still seem to have quite a
> bit of them.
>
> Main problem with this changes is that it creates lots of work to
> people backporting patches and stable maintainers, I remember that
> they have complain about this.
>
> Either way, I would prefer you fix this coding style issue in one go,
> ie. one single patch, it will be a large one.

ok, I will send the single patch.

Re: [Outreachy kernel] [PATCH v6] staging: Use buf_lock instead of mlock and Refactor code

[SIMRAN SINGHAL]

On Tue, Mar 21, 2017 at 10:18 PM, Alison Schofield  wrote:
> On Mon, Mar 20, 2017 at 01:36:21AM +0530, simran singhal wrote:
>
> Hi Simran,
>
> I going to ask for a v7 without looking at the code ;)
> Subject line needs subsystem and driver.
> Subject and log message can be improved.
>
>> The IIO subsystem is redefining iio_dev->mlock to be used by
>> the IIO core only for protecting device operating mode changes.
>> ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
>>
>> In this driver, mlock was being used to protect hardware state
>> changes. Replace it with buf_lock in the devices global data.
>^^^ this was not done
>>
>> As buf_lock protects both the adis16060_spi_write() and
>> adis16060_spi_read() functions and both are always called in
>> pair. First write, then read. Thus, refactor the code to have
>> one single function adis16060_spi_write_than_read() which is
>> protected by the existing buf_lock.
> This was done.  So, you were able to obsolete the need for mlock
> by creating the paired function.

I am still using mlock but now locking it and performing both write and
read and than unlocking.

So, now have a single safe function.

>
>>
>> Removed nested locks as the function adis16060_read_raw call
>> a lock on &st->buf_lock and then calls the function
>> adis16060_spi_write which again tries to get hold
>> of the same lock.
>  this was not done.  Yes, you avoided nested locks through
> proper coding, but we don't want to give the impression in the
> log message that there was a pre-existing nested lock issue.
>
> I did checkpatch & compile it...but looked no further yet.
>
> alisons
>>
>> Signed-off-by: simran singhal 
>> ---
>>
>>  v6:
>>-Change commit message
>>-Remove nested lock
>>
>>  drivers/staging/iio/gyro/adis16060_core.c | 40 
>> ++-
>>  1 file changed, 13 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/staging/iio/gyro/adis16060_core.c 
>> b/drivers/staging/iio/gyro/adis16060_core.c
>> index c9d46e7..1c6de46 100644
>> --- a/drivers/staging/iio/gyro/adis16060_core.c
>> +++ b/drivers/staging/iio/gyro/adis16060_core.c
>> @@ -40,25 +40,17 @@ struct adis16060_state {
>>
>>  static struct iio_dev *adis16060_iio_dev;
>>
>> -static int adis16060_spi_write(struct iio_dev *indio_dev, u8 val)
>> +static int adis16060_spi_write_than_read(struct iio_dev *indio_dev,
>> +  u8 conf, u16 *val)
>>  {
>>   int ret;
>>   struct adis16060_state *st = iio_priv(indio_dev);
>>
>> - mutex_lock(&st->buf_lock);
>> - st->buf[2] = val; /* The last 8 bits clocked in are latched */
>> + st->buf[2] = conf; /* The last 8 bits clocked in are latched */
>>   ret = spi_write(st->us_w, st->buf, 3);
>> - mutex_unlock(&st->buf_lock);
>> -
>> - return ret;
>> -}
>> -
>> -static int adis16060_spi_read(struct iio_dev *indio_dev, u16 *val)
>> -{
>> - int ret;
>> - struct adis16060_state *st = iio_priv(indio_dev);
>>
>> - mutex_lock(&st->buf_lock);
>> + if (ret < 0)
>> + return ret;
>>
>>   ret = spi_read(st->us_r, st->buf, 3);
>>
>> @@ -69,8 +61,8 @@ static int adis16060_spi_read(struct iio_dev *indio_dev, 
>> u16 *val)
>>*/
>>   if (!ret)
>>   *val = ((st->buf[0] & 0x3) << 12) |
>> - (st->buf[1] << 4) |
>> - ((st->buf[2] >> 4) & 0xF);
>> +  (st->buf[1] << 4) |
>> +  ((st->buf[2] >> 4) & 0xF);
>>   mutex_unlock(&st->buf_lock);
>>
>>   return ret;
>> @@ -83,20 +75,18 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
>>  {
>>   u16 tval = 0;
>>   int ret;
>> + struct adis16060_state *st = iio_priv(indio_dev);
>>
>>   switch (mask) {
>>   case IIO_CHAN_INFO_RAW:
>>   /* Take the iio_dev status lock */
>> - mutex_lock(&indio_dev->mlock);
>> - ret = adis16060_spi_write(indio_dev, chan->address);
>> + mutex_lock(&st->buf_lock);
>> + ret = adis16060_spi_write_than_read(indio_dev,
>> + chan->address, &tval);
>> + mutex_unlock(&st->buf_lock);
>

[PATCH v3 1/2] staging: iio: ade7753: Remove trailing whitespaces

[simran singhal]

This patch removes trailing whitespaces in order to follow the Linux
coding style.

Signed-off-by: simran singhal 
---
 drivers/staging/iio/meter/ade7753.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7753.c 
b/drivers/staging/iio/meter/ade7753.c
index dfd8b71..b71fbd3 100644
--- a/drivers/staging/iio/meter/ade7753.c
+++ b/drivers/staging/iio/meter/ade7753.c
@@ -83,10 +83,10 @@
  * @buf_lock:   mutex to protect tx and rx
  **/
 struct ade7753_state {
-   struct spi_device   *us;
-   struct mutexbuf_lock;
-   u8  tx[ADE7753_MAX_TX] 
cacheline_aligned;
-   u8  rx[ADE7753_MAX_RX];
+   struct spi_device   *us;
+   struct mutexbuf_lock;
+   u8  tx[ADE7753_MAX_TX] cacheline_aligned;
+   u8  rx[ADE7753_MAX_RX];
 };
 
 static int ade7753_spi_write_reg_8(struct device *dev,
-- 
2.7.4

[PATCH v3 2/2] staging: iio: ade7753: Replace mlock with driver private lock

[simran singhal]

The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.

In this driver, mlock was being used to protect hardware state
changes.  Replace it with a lock in the devices global data.

Signed-off-by: simran singhal 
---
 drivers/staging/iio/meter/ade7753.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7753.c 
b/drivers/staging/iio/meter/ade7753.c
index b71fbd3..9674e05 100644
--- a/drivers/staging/iio/meter/ade7753.c
+++ b/drivers/staging/iio/meter/ade7753.c
@@ -80,11 +80,13 @@
  * @us: actual spi_device
  * @tx: transmit buffer
  * @rx: receive buffer
+ * @lock:   protect sensor state
  * @buf_lock:   mutex to protect tx and rx
  **/
 struct ade7753_state {
struct spi_device   *us;
struct mutexbuf_lock;
+   struct mutexlock;  /* protect sensor state */
u8  tx[ADE7753_MAX_TX] cacheline_aligned;
u8  rx[ADE7753_MAX_RX];
 };
@@ -484,7 +486,7 @@ static ssize_t ade7753_write_frequency(struct device *dev,
if (!val)
return -EINVAL;
 
-   mutex_lock(&indio_dev->mlock);
+   mutex_lock(&st->lock);
 
t = 27900 / val;
if (t > 0)
@@ -505,7 +507,7 @@ static ssize_t ade7753_write_frequency(struct device *dev,
ret = ade7753_spi_write_reg_16(dev, ADE7753_MODE, reg);
 
 out:
-   mutex_unlock(&indio_dev->mlock);
+   mutex_unlock(&st->lock);
 
return ret ? ret : len;
 }
-- 
2.7.4

[PATCH v3 0/2] Replace mlock with private lock and delete whitespaces

[simran singhal]

The patch series replaces mlock with a private lock for driver ad9834 and
Fix coding style issues related to white spaces.

v3:
  -Using new private "lock" instead of using "buf_lock"
   as it can cause deadlock.
  -Sending it as a series of two patches.

v2:
  -Using the existing buf_lock instead of lock.
   

simran singhal (2):
  staging: iio: ade7753: Remove trailing whitespaces
  staging: iio: ade7753: Replace mlock with driver private lock

 drivers/staging/iio/meter/ade7753.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

-- 
2.7.4

Re: [Outreachy kernel] [PATCH v6] staging: Use buf_lock instead of mlock and Refactor code

[SIMRAN SINGHAL]

On Tue, Mar 21, 2017 at 10:18 PM, Alison Schofield  wrote:
> On Mon, Mar 20, 2017 at 01:36:21AM +0530, simran singhal wrote:
>
> Hi Simran,
>
> I going to ask for a v7 without looking at the code ;)
> Subject line needs subsystem and driver.
> Subject and log message can be improved.

Hi Alison,
I have already sent v7 with changed subject.

>
>> The IIO subsystem is redefining iio_dev->mlock to be used by
>> the IIO core only for protecting device operating mode changes.
>> ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
>>
>> In this driver, mlock was being used to protect hardware state
>> changes. Replace it with buf_lock in the devices global data.
>^^^ this was not done
>>
>> As buf_lock protects both the adis16060_spi_write() and
>> adis16060_spi_read() functions and both are always called in
>> pair. First write, then read. Thus, refactor the code to have
>> one single function adis16060_spi_write_than_read() which is
>> protected by the existing buf_lock.
> This was done.  So, you were able to obsolete the need for mlock
> by creating the paired function.
>
>>
>> Removed nested locks as the function adis16060_read_raw call
>> a lock on &st->buf_lock and then calls the function
>> adis16060_spi_write which again tries to get hold
>> of the same lock.
>  this was not done.  Yes, you avoided nested locks through
> proper coding, but we don't want to give the impression in the
> log message that there was a pre-existing nested lock issue.
>
> I did checkpatch & compile it...but looked no further yet.
>
> alisons
>>
>> Signed-off-by: simran singhal 
>> ---
>>
>>  v6:
>>-Change commit message
>>-Remove nested lock
>>
>>  drivers/staging/iio/gyro/adis16060_core.c | 40 
>> ++-
>>  1 file changed, 13 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/staging/iio/gyro/adis16060_core.c 
>> b/drivers/staging/iio/gyro/adis16060_core.c
>> index c9d46e7..1c6de46 100644
>> --- a/drivers/staging/iio/gyro/adis16060_core.c
>> +++ b/drivers/staging/iio/gyro/adis16060_core.c
>> @@ -40,25 +40,17 @@ struct adis16060_state {
>>
>>  static struct iio_dev *adis16060_iio_dev;
>>
>> -static int adis16060_spi_write(struct iio_dev *indio_dev, u8 val)
>> +static int adis16060_spi_write_than_read(struct iio_dev *indio_dev,
>> +  u8 conf, u16 *val)
>>  {
>>   int ret;
>>   struct adis16060_state *st = iio_priv(indio_dev);
>>
>> - mutex_lock(&st->buf_lock);
>> - st->buf[2] = val; /* The last 8 bits clocked in are latched */
>> + st->buf[2] = conf; /* The last 8 bits clocked in are latched */
>>   ret = spi_write(st->us_w, st->buf, 3);
>> - mutex_unlock(&st->buf_lock);
>> -
>> - return ret;
>> -}
>> -
>> -static int adis16060_spi_read(struct iio_dev *indio_dev, u16 *val)
>> -{
>> - int ret;
>> - struct adis16060_state *st = iio_priv(indio_dev);
>>
>> - mutex_lock(&st->buf_lock);
>> + if (ret < 0)
>> + return ret;
>>
>>   ret = spi_read(st->us_r, st->buf, 3);
>>
>> @@ -69,8 +61,8 @@ static int adis16060_spi_read(struct iio_dev *indio_dev, 
>> u16 *val)
>>*/
>>   if (!ret)
>>   *val = ((st->buf[0] & 0x3) << 12) |
>> - (st->buf[1] << 4) |
>> - ((st->buf[2] >> 4) & 0xF);
>> +  (st->buf[1] << 4) |
>> +  ((st->buf[2] >> 4) & 0xF);
>>   mutex_unlock(&st->buf_lock);
>>
>>   return ret;
>> @@ -83,20 +75,18 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
>>  {
>>   u16 tval = 0;
>>   int ret;
>> + struct adis16060_state *st = iio_priv(indio_dev);
>>
>>   switch (mask) {
>>   case IIO_CHAN_INFO_RAW:
>>   /* Take the iio_dev status lock */
>> - mutex_lock(&indio_dev->mlock);
>> - ret = adis16060_spi_write(indio_dev, chan->address);
>> + mutex_lock(&st->buf_lock);
>> + ret = adis16060_spi_write_than_read(indio_dev,
>> + chan->address, &tval);
>> + mutex_unlock(&st->buf_lock);
>>   if (ret < 0)
>> - goto o

Re: [PATCH v5] staging: Use buf_lock instead of mlock and Refactor code

[SIMRAN SINGHAL]

On Tue, Mar 21, 2017 at 8:01 PM, kbuild test robot  wrote:
> Hi simran,
>
> [auto build test WARNING on iio/togreg]
> [also build test WARNING on v4.11-rc3 next-20170321]
> [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/simran-singhal/staging-Use-buf_lock-instead-of-mlock-and-Refactor-code/20170321-213956
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> config: x86_64-randconfig-x016-201712 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All warnings (new ones prefixed by >>):
>
I am not getting any error, and I have already sent the v7 of this patch
please check out that:
https://groups.google.com/forum/#!topic/outreachy-kernel/5uz6IpcOpMA


>In file included from include/uapi/linux/stddef.h:1:0,
> from include/linux/stddef.h:4,
> from include/uapi/linux/posix_types.h:4,
> from include/uapi/linux/types.h:13,
> from include/linux/types.h:5,
> from include/linux/list.h:4,
> from include/linux/module.h:9,
> from drivers/staging/iio/gyro/adis16060_core.c:9:
>drivers/staging/iio/gyro/adis16060_core.c: In function 
> 'adis16060_read_raw':
>include/linux/compiler.h:149:2: warning: this 'if' clause does not 
> guard... [-Wmisleading-indentation]
>  if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
>  ^
>include/linux/compiler.h:147:23: note: in expansion of macro '__trace_if'
> #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
>   ^~
>>> drivers/staging/iio/gyro/adis16060_core.c:89:3: note: in expansion of macro 
>>> 'if'
>   if (ret < 0)
>   ^~
>drivers/staging/iio/gyro/adis16060_core.c:91:4: note: ...this statement, 
> but the latter is misleadingly indented as if it is guarded by the 'if'
>return ret;
>^~
>
> vim +/if +89 drivers/staging/iio/gyro/adis16060_core.c
>
> e071f6b8 Barry Song   2010-10-27   3   *
> e071f6b8 Barry Song   2010-10-27   4   * Copyright 2010 Analog Devices 
> Inc.
> e071f6b8 Barry Song   2010-10-27   5   *
> e071f6b8 Barry Song   2010-10-27   6   * Licensed under the GPL-2 or 
> later.
> e071f6b8 Barry Song   2010-10-27   7   */
> e071f6b8 Barry Song   2010-10-27   8
> 45296236 Paul Gortmaker   2011-08-30  @9  #include 
> e071f6b8 Barry Song   2010-10-27  10  #include 
> e071f6b8 Barry Song   2010-10-27  11  #include 
> e071f6b8 Barry Song   2010-10-27  12  #include 
> e071f6b8 Barry Song   2010-10-27  13  #include 
> e071f6b8 Barry Song   2010-10-27  14  #include 
> e071f6b8 Barry Song   2010-10-27  15  #include 
> e071f6b8 Barry Song   2010-10-27  16  #include 
> 14f98326 Jonathan Cameron 2011-02-28  17
> 06458e27 Jonathan Cameron 2012-04-25  18  #include 
> 06458e27 Jonathan Cameron 2012-04-25  19  #include 
> e071f6b8 Barry Song   2010-10-27  20
> 14f98326 Jonathan Cameron 2011-02-28  21  #define ADIS16060_GYRO  
>   0x20 /* Measure Angular Rate (Gyro) */
> 14f98326 Jonathan Cameron 2011-02-28  22  #define ADIS16060_TEMP_OUT0x10 
> /* Measure Temperature */
> 14f98326 Jonathan Cameron 2011-02-28  23  #define ADIS16060_AIN2  
>   0x80 /* Measure AIN2 */
> 14f98326 Jonathan Cameron 2011-02-28  24  #define ADIS16060_AIN1  
>   0x40 /* Measure AIN1 */
> 14f98326 Jonathan Cameron 2011-02-28  25
> 14f98326 Jonathan Cameron 2011-02-28  26  /**
> 14f98326 Jonathan Cameron 2011-02-28  27   * struct adis16060_state - device 
> instance specific data
> 14f98326 Jonathan Cameron 2011-02-28  28   * @us_w: actual 
> spi_device to write config
> 14f98326 Jonathan Cameron 2011-02-28  29   * @us_r: actual 
> spi_device to read back data
> 25985edc Lucas De Marchi  2011-03-30  30   * @buf:  transmit or 
> receive buffer
> 14f98326 Jonathan Cameron 2011-02-28  31   * @buf_lock: mutex to 
> protect tx and rx
> 14f98326 Jonathan Cameron 2011-02-28  32   **/
> 14f98326 Jonathan Cameron 2011-02-28  33  struct adis16060_state {
> 14f98326 Jonathan Cameron 2011-02-28  34struct spi_device 
>   *us_w;
> 14f98326 Jonathan Cameron 2011-02-28  35struct spi_device 
>   *us_r;
> 14f98326 Jonathan Cameron 2011-02-28  36struct mutex  
>   buf_lock;
> 14f98326 J

[PATCH v2 1/2] netfilter: ipset: Compress return logic

[simran singhal]

Simplify function returns by merging assignment and return into one
command line.

Signed-off-by: simran singhal 
---

 --This is my contribution to the netfilter project of
   Outreachy Round 14.

 net/netfilter/ipset/ip_set_list_set.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_list_set.c 
b/net/netfilter/ipset/ip_set_list_set.c
index 178d4eb..2fff6b5 100644
--- a/net/netfilter/ipset/ip_set_list_set.c
+++ b/net/netfilter/ipset/ip_set_list_set.c
@@ -453,7 +453,6 @@ static size_t
 list_set_memsize(const struct list_set *map, size_t dsize)
 {
struct set_elem *e;
-   size_t memsize;
u32 n = 0;
 
rcu_read_lock();
@@ -461,9 +460,7 @@ list_set_memsize(const struct list_set *map, size_t dsize)
n++;
rcu_read_unlock();
 
-   memsize = sizeof(*map) + n * dsize;
-
-   return memsize;
+   return (sizeof(*map) + n * dsize);
 }
 
 static int
-- 
2.7.4

[PATCH v2 2/2] netfilter: ipvs: Compress return logic

[simran singhal]

Simplify function returns by merging assignment and return into
one command line.

Signed-off-by: simran singhal 
---

 --This is my contribution to the netfilter project of
   Outreachy Round 14.

 net/netfilter/ipvs/ip_vs_ftp.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index d30c327..c93c937 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -482,11 +482,8 @@ static struct pernet_operations ip_vs_ftp_ops = {
 
 static int __init ip_vs_ftp_init(void)
 {
-   int rv;
-
-   rv = register_pernet_subsys(&ip_vs_ftp_ops);
/* rcu_barrier() is called by netns on error */
-   return rv;
+   return register_pernet_subsys(&ip_vs_ftp_ops);
 }
 
 /*
-- 
2.7.4

[PATCH v2 0/2] netfilter: Compress return logic

[simran singhal]

This patch series Simplify function returns by merging
assignment and return into one command line.

v2:
  -Change the subject of cover patch

simran singhal (2):
  netfilter: ipset: Compress return logic
  netfilter: ipvs: Compress return logic

 net/netfilter/ipset/ip_set_list_set.c | 5 +
 net/netfilter/ipvs/ip_vs_ftp.c| 5 +
 2 files changed, 2 insertions(+), 8 deletions(-)

-- 
2.7.4

[PATCH 0/2] netfilter: ,net...@vger.kernel.org

[simran singhal]

This patch series Simplify function returns by merging 
assignment and return into one command line.

simran singhal (2):
  netfilter: ipset: Compress return logic
  netfilter: ipvs: Compress return logic

 net/netfilter/ipset/ip_set_list_set.c | 5 +
 net/netfilter/ipvs/ip_vs_ftp.c| 5 +
 2 files changed, 2 insertions(+), 8 deletions(-)

-- 
2.7.4

[PATCH 2/2] netfilter: ipvs: Compress return logic

[simran singhal]

Simplify function returns by merging assignment and return into
one command line.

Signed-off-by: simran singhal 
---

 --This is my contribution to the netfilter project of
   Outreachy Round 14.

 net/netfilter/ipvs/ip_vs_ftp.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index d30c327..c93c937 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -482,11 +482,8 @@ static struct pernet_operations ip_vs_ftp_ops = {
 
 static int __init ip_vs_ftp_init(void)
 {
-   int rv;
-
-   rv = register_pernet_subsys(&ip_vs_ftp_ops);
/* rcu_barrier() is called by netns on error */
-   return rv;
+   return register_pernet_subsys(&ip_vs_ftp_ops);
 }
 
 /*
-- 
2.7.4

[PATCH 1/2] netfilter: ipset: Compress return logic

[simran singhal]

Simplify function returns by merging assignment and return into one
command line.

Signed-off-by: simran singhal 
---
 
 --This is my contribution to the netfilter project of
   Outreachy Round 14.

 net/netfilter/ipset/ip_set_list_set.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_list_set.c 
b/net/netfilter/ipset/ip_set_list_set.c
index 178d4eb..2fff6b5 100644
--- a/net/netfilter/ipset/ip_set_list_set.c
+++ b/net/netfilter/ipset/ip_set_list_set.c
@@ -453,7 +453,6 @@ static size_t
 list_set_memsize(const struct list_set *map, size_t dsize)
 {
struct set_elem *e;
-   size_t memsize;
u32 n = 0;
 
rcu_read_lock();
@@ -461,9 +460,7 @@ list_set_memsize(const struct list_set *map, size_t dsize)
n++;
rcu_read_unlock();
 
-   memsize = sizeof(*map) + n * dsize;
-
-   return memsize;
+   return (sizeof(*map) + n * dsize);
 }
 
 static int
-- 
2.7.4

[PATCH 0/2] netfilter: Remove unnecessary cast on void pointer

[simran singhal]

This patch series remove unnecessary cast on void pointer.

simran singhal (2):
  netfilter: ipset: Remove unnecessary cast on void pointer
  netfilter: Remove unnecessary cast on void pointer

 net/netfilter/ipset/ip_set_bitmap_gen.h |  4 ++--
 net/netfilter/ipset/ip_set_core.c   |  2 +-
 net/netfilter/nf_conntrack_proto.c  |  2 +-
 net/netfilter/nft_set_hash.c|  2 +-
 net/netfilter/xt_hashlimit.c| 10 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

-- 
2.7.4

[PATCH 1/2] netfilter: ipset: Remove unnecessary cast on void pointer

[simran singhal]

The following Coccinelle script was used to detect this:
@r@
expression x;
void* e;
type T;
identifier f;
@@
(
  *((T *)e)
|
  ((T *)x)[...]
|
  ((T*)x)->f
|

- (T*)
  e
)

Signed-off-by: simran singhal 
---

 --This is my contribution to the netfilter project of
   Outreachy Round 14. 

 net/netfilter/ipset/ip_set_bitmap_gen.h | 4 ++--
 net/netfilter/ipset/ip_set_core.c   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_bitmap_gen.h 
b/net/netfilter/ipset/ip_set_bitmap_gen.h
index 6f09a99..d302d98 100644
--- a/net/netfilter/ipset/ip_set_bitmap_gen.h
+++ b/net/netfilter/ipset/ip_set_bitmap_gen.h
@@ -232,7 +232,7 @@ mtype_list(const struct ip_set *set,
if (!test_bit(id, map->members) ||
(SET_WITH_TIMEOUT(set) &&
 #ifdef IP_SET_BITMAP_STORED_TIMEOUT
-mtype_is_filled((const struct mtype_elem *)x) &&
+mtype_is_filled(x) &&
 #endif
 ip_set_timeout_expired(ext_timeout(x, set
continue;
@@ -249,7 +249,7 @@ mtype_list(const struct ip_set *set,
if (mtype_do_list(skb, map, id, set->dsize))
goto nla_put_failure;
if (ip_set_put_extensions(skb, set, x,
-   mtype_is_filled((const struct mtype_elem *)x)))
+   mtype_is_filled(x)))
goto nla_put_failure;
ipset_nest_end(skb, nested);
}
diff --git a/net/netfilter/ipset/ip_set_core.c 
b/net/netfilter/ipset/ip_set_core.c
index c296f9b..10a3694 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -1915,7 +1915,7 @@ ip_set_sockfn_get(struct sock *sk, int optval, void 
__user *user, int *len)
ret = -EFAULT;
goto done;
}
-   op = (unsigned int *)data;
+   op = data;
 
if (*op < IP_SET_OP_VERSION) {
/* Check the version at the beginning of operations */
-- 
2.7.4

[PATCH 2/2] netfilter: Remove unnecessary cast on void pointer

[simran singhal]

The following Coccinelle script was used to detect this:
@r@
expression x;
void* e;
type T;
identifier f;
@@
(
  *((T *)e)
|
  ((T *)x)[...]
|
  ((T*)x)->f
|

- (T*)
  e
)

Signed-off-by: simran singhal 
---

 --This is my contribution to the netfilter project of
   Outreachy Round 14. 

 net/netfilter/nf_conntrack_proto.c |  2 +-
 net/netfilter/nft_set_hash.c   |  2 +-
 net/netfilter/xt_hashlimit.c   | 10 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto.c 
b/net/netfilter/nf_conntrack_proto.c
index 48d8354..ec665c8 100644
--- a/net/netfilter/nf_conntrack_proto.c
+++ b/net/netfilter/nf_conntrack_proto.c
@@ -202,7 +202,7 @@ static int kill_l3proto(struct nf_conn *i, void *data)
 static int kill_l4proto(struct nf_conn *i, void *data)
 {
struct nf_conntrack_l4proto *l4proto;
-   l4proto = (struct nf_conntrack_l4proto *)data;
+   l4proto = data;
return nf_ct_protonum(i) == l4proto->l4proto &&
   nf_ct_l3num(i) == l4proto->l3proto;
 }
diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c
index 5f65272..8ec086b 100644
--- a/net/netfilter/nft_set_hash.c
+++ b/net/netfilter/nft_set_hash.c
@@ -352,7 +352,7 @@ static int nft_hash_init(const struct nft_set *set,
 
 static void nft_hash_elem_destroy(void *ptr, void *arg)
 {
-   nft_set_elem_destroy((const struct nft_set *)arg, ptr, true);
+   nft_set_elem_destroy(arg, ptr, true);
 }
 
 static void nft_hash_destroy(const struct nft_set *set)
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 2a6dfe8..762e187 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -119,7 +119,7 @@ static int
 cfg_copy(struct hashlimit_cfg2 *to, void *from, int revision)
 {
if (revision == 1) {
-   struct hashlimit_cfg1 *cfg = (struct hashlimit_cfg1 *)from;
+   struct hashlimit_cfg1 *cfg = from;
 
to->mode = cfg->mode;
to->avg = cfg->avg;
@@ -895,7 +895,7 @@ static void *dl_seq_start(struct seq_file *s, loff_t *pos)
 static void *dl_seq_next(struct seq_file *s, void *v, loff_t *pos)
 {
struct xt_hashlimit_htable *htable = s->private;
-   unsigned int *bucket = (unsigned int *)v;
+   unsigned int *bucket = v;
 
*pos = ++(*bucket);
if (*pos >= htable->cfg.size) {
@@ -909,7 +909,7 @@ static void dl_seq_stop(struct seq_file *s, void *v)
__releases(htable->lock)
 {
struct xt_hashlimit_htable *htable = s->private;
-   unsigned int *bucket = (unsigned int *)v;
+   unsigned int *bucket = v;
 
if (!IS_ERR(bucket))
kfree(bucket);
@@ -980,7 +980,7 @@ static int dl_seq_real_show(struct dsthash_ent *ent, 
u_int8_t family,
 static int dl_seq_show_v1(struct seq_file *s, void *v)
 {
struct xt_hashlimit_htable *htable = s->private;
-   unsigned int *bucket = (unsigned int *)v;
+   unsigned int *bucket = v;
struct dsthash_ent *ent;
 
if (!hlist_empty(&htable->hash[*bucket])) {
@@ -994,7 +994,7 @@ static int dl_seq_show_v1(struct seq_file *s, void *v)
 static int dl_seq_show(struct seq_file *s, void *v)
 {
struct xt_hashlimit_htable *htable = s->private;
-   unsigned int *bucket = (unsigned int *)v;
+   unsigned int *bucket = v;
struct dsthash_ent *ent;
 
if (!hlist_empty(&htable->hash[*bucket])) {
-- 
2.7.4

[PATCH 5/5] netfilter: xt_TEE: Clean up tests if NULL returned on failure

[simran singhal]

Some functions like kmalloc/kzalloc return NULL on failure. When NULL
represents failure, !x is commonly used.

This was done using Coccinelle:
@@
expression *e;
identifier l1;
@@
e = \(kmalloc\|kzalloc\|kcalloc\|devm_kzalloc\)(...);
...
- e == NULL
+ !e

Signed-off-by: simran singhal 
---

 --This is my contribution to the netfilter project of
   Outreachy Round 14.

 net/netfilter/xt_TEE.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/xt_TEE.c b/net/netfilter/xt_TEE.c
index 86b0580..5df3282 100644
--- a/net/netfilter/xt_TEE.c
+++ b/net/netfilter/xt_TEE.c
@@ -95,7 +95,7 @@ static int tee_tg_check(const struct xt_tgchk_param *par)
return -EINVAL;
 
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-   if (priv == NULL)
+   if (!priv)
return -ENOMEM;
 
priv->tginfo  = info;
-- 
2.7.4

[PATCH 2/5] netfilter: Clean up tests if NULL returned on failure

[simran singhal]

Some functions like kmalloc/kzalloc return NULL on failure. When NULL
represents failure, !x is commonly used.

This was done using Coccinelle:
@@
expression *e;
identifier l1;
@@
e = \(kmalloc\|kzalloc\|kcalloc\|devm_kzalloc\)(...);
...
- e == NULL
+ !e

Signed-off-by: simran singhal 
---

 --This is my contribution to the netfilter project of
   Outreachy Round 14.

 net/netfilter/nf_conntrack_netlink.c | 2 +-
 net/netfilter/nf_conntrack_proto.c   | 2 +-
 net/netfilter/nf_nat_core.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c 
b/net/netfilter/nf_conntrack_netlink.c
index 6806b5e..cdcf4c1 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -779,7 +779,7 @@ ctnetlink_alloc_filter(const struct nlattr * const cda[])
struct ctnetlink_filter *filter;
 
filter = kzalloc(sizeof(*filter), GFP_KERNEL);
-   if (filter == NULL)
+   if (!filter)
return ERR_PTR(-ENOMEM);
 
filter->mark.val = ntohl(nla_get_be32(cda[CTA_MARK]));
diff --git a/net/netfilter/nf_conntrack_proto.c 
b/net/netfilter/nf_conntrack_proto.c
index 2d6ee18..48d8354 100644
--- a/net/netfilter/nf_conntrack_proto.c
+++ b/net/netfilter/nf_conntrack_proto.c
@@ -358,7 +358,7 @@ int nf_ct_l4proto_register_one(struct nf_conntrack_l4proto 
*l4proto)
proto_array = kmalloc(MAX_NF_CT_PROTO *
  sizeof(struct nf_conntrack_l4proto *),
  GFP_KERNEL);
-   if (proto_array == NULL) {
+   if (!proto_array) {
ret = -ENOMEM;
goto out_unlock;
}
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 94b14c5..922bd5b 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -626,7 +626,7 @@ int nf_nat_l4proto_register(u8 l3proto, const struct 
nf_nat_l4proto *l4proto)
if (nf_nat_l4protos[l3proto] == NULL) {
l4protos = kmalloc(IPPROTO_MAX * sizeof(struct nf_nat_l4proto 
*),
   GFP_KERNEL);
-   if (l4protos == NULL) {
+   if (!l4protos) {
ret = -ENOMEM;
goto out;
}
-- 
2.7.4

[PATCH 1/5] netfilter: ipvs: Clean up tests if NULL returned on failure

[simran singhal]

Some functions like kmalloc/kzalloc return NULL on failure. When NULL
represents failure, !x is commonly used.

@@
expression *e;
identifier l1;
@@
e = \(kmalloc\|kzalloc\|kcalloc\|devm_kzalloc\)(...);
...
- e == NULL
+ !e

Signed-off-by: simran singhal 
---
 
 --This is my contribution to the netfilter project of
   Outreachy Round 14.
 
 net/netfilter/ipvs/ip_vs_ctl.c   | 4 ++--
 net/netfilter/ipvs/ip_vs_dh.c| 2 +-
 net/netfilter/ipvs/ip_vs_lblc.c  | 2 +-
 net/netfilter/ipvs/ip_vs_lblcr.c | 4 ++--
 net/netfilter/ipvs/ip_vs_sh.c| 2 +-
 net/netfilter/ipvs/ip_vs_wrr.c   | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 5aeb0dd..efe348a 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -910,7 +910,7 @@ ip_vs_new_dest(struct ip_vs_service *svc, struct 
ip_vs_dest_user_kern *udest,
}
 
dest = kzalloc(sizeof(struct ip_vs_dest), GFP_KERNEL);
-   if (dest == NULL)
+   if (!dest)
return -ENOMEM;
 
dest->stats.cpustats = alloc_percpu(struct ip_vs_cpu_stats);
@@ -1228,7 +1228,7 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct 
ip_vs_service_user_kern *u,
 #endif
 
svc = kzalloc(sizeof(struct ip_vs_service), GFP_KERNEL);
-   if (svc == NULL) {
+   if (!svc) {
IP_VS_DBG(1, "%s(): no memory\n", __func__);
ret = -ENOMEM;
goto out_err;
diff --git a/net/netfilter/ipvs/ip_vs_dh.c b/net/netfilter/ipvs/ip_vs_dh.c
index 75f798f..22a2535 100644
--- a/net/netfilter/ipvs/ip_vs_dh.c
+++ b/net/netfilter/ipvs/ip_vs_dh.c
@@ -159,7 +159,7 @@ static int ip_vs_dh_init_svc(struct ip_vs_service *svc)
 
/* allocate the DH table for this service */
s = kzalloc(sizeof(struct ip_vs_dh_state), GFP_KERNEL);
-   if (s == NULL)
+   if (!s)
return -ENOMEM;
 
svc->sched_data = s;
diff --git a/net/netfilter/ipvs/ip_vs_lblc.c b/net/netfilter/ipvs/ip_vs_lblc.c
index 5824927..d7c6031 100644
--- a/net/netfilter/ipvs/ip_vs_lblc.c
+++ b/net/netfilter/ipvs/ip_vs_lblc.c
@@ -352,7 +352,7 @@ static int ip_vs_lblc_init_svc(struct ip_vs_service *svc)
 *Allocate the ip_vs_lblc_table for this service
 */
tbl = kmalloc(sizeof(*tbl), GFP_KERNEL);
-   if (tbl == NULL)
+   if (!tbl)
return -ENOMEM;
 
svc->sched_data = tbl;
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index 703f118..b0a9e1c 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -113,7 +113,7 @@ static void ip_vs_dest_set_insert(struct ip_vs_dest_set 
*set,
}
 
e = kmalloc(sizeof(*e), GFP_ATOMIC);
-   if (e == NULL)
+   if (!e)
return;
 
ip_vs_dest_hold(dest);
@@ -515,7 +515,7 @@ static int ip_vs_lblcr_init_svc(struct ip_vs_service *svc)
 *Allocate the ip_vs_lblcr_table for this service
 */
tbl = kmalloc(sizeof(*tbl), GFP_KERNEL);
-   if (tbl == NULL)
+   if (!tbl)
return -ENOMEM;
 
svc->sched_data = tbl;
diff --git a/net/netfilter/ipvs/ip_vs_sh.c b/net/netfilter/ipvs/ip_vs_sh.c
index 16aaac6..99f3c3e 100644
--- a/net/netfilter/ipvs/ip_vs_sh.c
+++ b/net/netfilter/ipvs/ip_vs_sh.c
@@ -235,7 +235,7 @@ static int ip_vs_sh_init_svc(struct ip_vs_service *svc)
 
/* allocate the SH table for this service */
s = kzalloc(sizeof(struct ip_vs_sh_state), GFP_KERNEL);
-   if (s == NULL)
+   if (!s)
return -ENOMEM;
 
svc->sched_data = s;
diff --git a/net/netfilter/ipvs/ip_vs_wrr.c b/net/netfilter/ipvs/ip_vs_wrr.c
index 17e6d44..0923e6c 100644
--- a/net/netfilter/ipvs/ip_vs_wrr.c
+++ b/net/netfilter/ipvs/ip_vs_wrr.c
@@ -116,7 +116,7 @@ static int ip_vs_wrr_init_svc(struct ip_vs_service *svc)
 *Allocate the mark variable for WRR scheduling
 */
mark = kmalloc(sizeof(struct ip_vs_wrr_mark), GFP_KERNEL);
-   if (mark == NULL)
+   if (!mark)
return -ENOMEM;
 
mark->cl = list_entry(&svc->destinations, struct ip_vs_dest, n_list);
-- 
2.7.4

[PATCH 4/5] netfilter: nfnetlink: Clean up tests if NULL returned on failure

[simran singhal]

Some functions like kmalloc/kzalloc return NULL on failure. When NULL
represents failure, !x is commonly used.

This was done using Coccinelle:
@@
expression *e;
identifier l1;
@@
e = \(kmalloc\|kzalloc\|kcalloc\|devm_kzalloc\)(...);
...
- e == NULL
+ !e

Signed-off-by: simran singhal 
---

 --This is my contribution to the netfilter project of
   Outreachy Round 14.

 net/netfilter/nfnetlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 68eda920..c39f16c 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -232,7 +232,7 @@ static int nfnl_err_add(struct list_head *list, struct 
nlmsghdr *nlh, int err)
struct nfnl_err *nfnl_err;
 
nfnl_err = kmalloc(sizeof(struct nfnl_err), GFP_KERNEL);
-   if (nfnl_err == NULL)
+   if (!nfnl_err)
return -ENOMEM;
 
nfnl_err->nlh = nlh;
-- 
2.7.4

[PATCH 3/5] netfilter: nf_tables_api: Clean up tests if NULL returned on failure

[simran singhal]

Some functions like kmalloc/kzalloc return NULL on failure. When NULL
represents failure, !x is commonly used.

This was done using Coccinelle:
@@
expression *e;
identifier l1;
@@
e = \(kmalloc\|kzalloc\|kcalloc\|devm_kzalloc\)(...);
...
- e == NULL
+ !e

Signed-off-by: simran singhal 
---

 --This is my contribution to the netfilter project of
   Outreachy Round 14.

 net/netfilter/nf_tables_api.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 5e0ccfd..25a3951 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -117,7 +117,7 @@ static struct nft_trans *nft_trans_alloc_gfp(const struct 
nft_ctx *ctx,
struct nft_trans *trans;
 
trans = kzalloc(sizeof(struct nft_trans) + size, gfp);
-   if (trans == NULL)
+   if (!trans)
return NULL;
 
trans->msg_type = msg_type;
@@ -720,7 +720,7 @@ static int nf_tables_newtable(struct net *net, struct sock 
*nlsk,
 
err = -ENOMEM;
table = kzalloc(sizeof(*table), GFP_KERNEL);
-   if (table == NULL)
+   if (!table)
goto err2;
 
nla_strlcpy(table->name, name, NFT_TABLE_MAXNAMELEN);
@@ -1478,7 +1478,7 @@ static int nf_tables_newchain(struct net *net, struct 
sock *nlsk,
return err;
 
basechain = kzalloc(sizeof(*basechain), GFP_KERNEL);
-   if (basechain == NULL) {
+   if (!basechain) {
nft_chain_release_hook(&hook);
return -ENOMEM;
}
@@ -1526,7 +1526,7 @@ static int nf_tables_newchain(struct net *net, struct 
sock *nlsk,
basechain->policy = policy;
} else {
chain = kzalloc(sizeof(*chain), GFP_KERNEL);
-   if (chain == NULL)
+   if (!chain)
return -ENOMEM;
}
 
@@ -1800,7 +1800,7 @@ struct nft_expr *nft_expr_init(const struct nft_ctx *ctx,
 
err = -ENOMEM;
expr = kzalloc(info.ops->size, GFP_KERNEL);
-   if (expr == NULL)
+   if (!expr)
goto err2;
 
err = nf_tables_newexpr(ctx, &info, expr);
@@ -2214,7 +2214,7 @@ static int nf_tables_newrule(struct net *net, struct sock 
*nlsk,
 
err = -ENOMEM;
rule = kzalloc(sizeof(*rule) + size + usize, GFP_KERNEL);
-   if (rule == NULL)
+   if (!rule)
goto err1;
 
nft_activate_next(net, rule);
@@ -2810,7 +2810,7 @@ static int nf_tables_getset(struct net *net, struct sock 
*nlsk,
struct nft_ctx *ctx_dump;
 
ctx_dump = kmalloc(sizeof(*ctx_dump), GFP_KERNEL);
-   if (ctx_dump == NULL)
+   if (!ctx_dump)
return -ENOMEM;
 
*ctx_dump = ctx;
@@ -3014,7 +3014,7 @@ static int nf_tables_newset(struct net *net, struct sock 
*nlsk,
 
err = -ENOMEM;
set = kzalloc(sizeof(*set) + size + udlen, GFP_KERNEL);
-   if (set == NULL)
+   if (!set)
goto err1;
 
nla_strlcpy(name, nla[NFTA_SET_NAME], sizeof(set->name));
@@ -3543,7 +3543,7 @@ void *nft_set_elem_init(const struct nft_set *set,
void *elem;
 
elem = kzalloc(set->ops->elemsize + tmpl->len, gfp);
-   if (elem == NULL)
+   if (!elem)
return NULL;
 
ext = nft_set_elem_ext(set, elem);
@@ -3998,7 +3998,7 @@ struct nft_set_gc_batch *nft_set_gc_batch_alloc(const 
struct nft_set *set,
struct nft_set_gc_batch *gcb;
 
gcb = kzalloc(sizeof(*gcb), gfp);
-   if (gcb == NULL)
+   if (!gcb)
return gcb;
gcb->head.set = set;
return gcb;
@@ -4084,7 +4084,7 @@ static struct nft_object *nft_obj_init(const struct 
nft_object_type *type,
 
err = -ENOMEM;
obj = kzalloc(sizeof(struct nft_object) + type->size, GFP_KERNEL);
-   if (obj == NULL)
+   if (!obj)
goto err1;
 
err = type->init((const struct nlattr * const *)tb, obj);
@@ -5567,7 +5567,7 @@ static int __init nf_tables_module_init(void)
 
info = kmalloc(sizeof(struct nft_expr_info) * NFT_RULE_MAXEXPRS,
   GFP_KERNEL);
-   if (info == NULL) {
+   if (!info) {
err = -ENOMEM;
goto err1;
}
-- 
2.7.4

[PATCH 0/5] netfilter: Clean up tests if NULL returned on failure

[simran singhal]

This patch series clean up tests if NULL returned on failure.

simran singhal (5):
  netfilter: ipvs: Clean up tests if NULL returned on failure
  netfilter: Clean up tests if NULL returned on failure
  netfilter: nf_tables_api: Clean up tests if NULL returned on failure
  netfilter: nfnetlink: Clean up tests if NULL returned on failure
  netfilter: xt_TEE: Clean up tests if NULL returned on failure

 net/netfilter/ipvs/ip_vs_ctl.c   |  4 ++--
 net/netfilter/ipvs/ip_vs_dh.c|  2 +-
 net/netfilter/ipvs/ip_vs_lblc.c  |  2 +-
 net/netfilter/ipvs/ip_vs_lblcr.c |  4 ++--
 net/netfilter/ipvs/ip_vs_sh.c|  2 +-
 net/netfilter/ipvs/ip_vs_wrr.c   |  2 +-
 net/netfilter/nf_conntrack_netlink.c |  2 +-
 net/netfilter/nf_conntrack_proto.c   |  2 +-
 net/netfilter/nf_nat_core.c  |  2 +-
 net/netfilter/nf_tables_api.c| 24 
 net/netfilter/nfnetlink.c|  2 +-
 net/netfilter/xt_TEE.c   |  2 +-
 12 files changed, 25 insertions(+), 25 deletions(-)

-- 
2.7.4

[PATCH 1/2] staging: ade7754: Move header content to implementation file

[simran singhal]

The contents of ade7754.h are only used in ade7754.c.
Move the header contents to the implementation file,
and delete the header file.

Signed-off-by: simran singhal 
---
 drivers/staging/iio/meter/ade7754.c | 87 ++-
 drivers/staging/iio/meter/ade7754.h | 90 -
 2 files changed, 86 insertions(+), 91 deletions(-)
 delete mode 100644 drivers/staging/iio/meter/ade7754.h

diff --git a/drivers/staging/iio/meter/ade7754.c 
b/drivers/staging/iio/meter/ade7754.c
index 024463a..42f7b06 100644
--- a/drivers/staging/iio/meter/ade7754.c
+++ b/drivers/staging/iio/meter/ade7754.c
@@ -21,7 +21,92 @@
 #include 
 #include 
 #include "meter.h"
-#include "ade7754.h"
+
+#define ADE7754_AENERGY   0x01
+#define ADE7754_RAENERGY  0x02
+#define ADE7754_LAENERGY  0x03
+#define ADE7754_VAENERGY  0x04
+#define ADE7754_RVAENERGY 0x05
+#define ADE7754_LVAENERGY 0x06
+#define ADE7754_PERIOD0x07
+#define ADE7754_TEMP  0x08
+#define ADE7754_WFORM 0x09
+#define ADE7754_OPMODE0x0A
+#define ADE7754_MMODE 0x0B
+#define ADE7754_WAVMODE   0x0C
+#define ADE7754_WATMODE   0x0D
+#define ADE7754_VAMODE0x0E
+#define ADE7754_IRQEN 0x0F
+#define ADE7754_STATUS0x10
+#define ADE7754_RSTATUS   0x11
+#define ADE7754_ZXTOUT0x12
+#define ADE7754_LINCYC0x13
+#define ADE7754_SAGCYC0x14
+#define ADE7754_SAGLVL0x15
+#define ADE7754_VPEAK 0x16
+#define ADE7754_IPEAK 0x17
+#define ADE7754_GAIN  0x18
+#define ADE7754_AWG   0x19
+#define ADE7754_BWG   0x1A
+#define ADE7754_CWG   0x1B
+#define ADE7754_AVAG  0x1C
+#define ADE7754_BVAG  0x1D
+#define ADE7754_CVAG  0x1E
+#define ADE7754_APHCAL0x1F
+#define ADE7754_BPHCAL0x20
+#define ADE7754_CPHCAL0x21
+#define ADE7754_AAPOS 0x22
+#define ADE7754_BAPOS 0x23
+#define ADE7754_CAPOS 0x24
+#define ADE7754_CFNUM 0x25
+#define ADE7754_CFDEN 0x26
+#define ADE7754_WDIV  0x27
+#define ADE7754_VADIV 0x28
+#define ADE7754_AIRMS 0x29
+#define ADE7754_BIRMS 0x2A
+#define ADE7754_CIRMS 0x2B
+#define ADE7754_AVRMS 0x2C
+#define ADE7754_BVRMS 0x2D
+#define ADE7754_CVRMS 0x2E
+#define ADE7754_AIRMSOS   0x2F
+#define ADE7754_BIRMSOS   0x30
+#define ADE7754_CIRMSOS   0x31
+#define ADE7754_AVRMSOS   0x32
+#define ADE7754_BVRMSOS   0x33
+#define ADE7754_CVRMSOS   0x34
+#define ADE7754_AAPGAIN   0x35
+#define ADE7754_BAPGAIN   0x36
+#define ADE7754_CAPGAIN   0x37
+#define ADE7754_AVGAIN0x38
+#define ADE7754_BVGAIN0x39
+#define ADE7754_CVGAIN0x3A
+#define ADE7754_CHKSUM0x3E
+#define ADE7754_VERSION   0x3F
+
+#define ADE7754_READ_REG(a)a
+#define ADE7754_WRITE_REG(a) ((a) | 0x80)
+
+#define ADE7754_MAX_TX4
+#define ADE7754_MAX_RX4
+#define ADE7754_STARTUP_DELAY 1000
+
+#define ADE7754_SPI_SLOW   (u32)(300 * 1000)
+#define ADE7754_SPI_BURST  (u32)(1000 * 1000)
+#define ADE7754_SPI_FAST   (u32)(2000 * 1000)
+
+/**
+ * struct ade7754_state - device instance specific data
+ * @us:actual spi_device
+ * @buf_lock:  mutex to protect tx and rx
+ * @tx:transmit buffer
+ * @rx:receive buffer
+ **/
+struct ade7754_state {
+   struct spi_device   *us;
+   struct mutexbuf_lock;
+   u8  tx[ADE7754_MAX_TX] cacheline_aligned;
+   u8  rx[ADE7754_MAX_RX];
+};
 
 static int ade7754_spi_write_reg_8(struct device *dev, u8 reg_address, u8 val)
 {
diff --git a/drivers/staging/iio/meter/ade7754.h 
b/drivers/staging/iio/meter/ade7754.h
deleted file mode 100644
index 28f71c2..000
--- a/drivers/staging/iio/meter/ade7754.h
+++ /dev/null
@@ -1,90 +0,0 @@
-#ifndef _ADE7754_H
-#define _ADE7754_H
-
-#define ADE7754_AENERGY   0x01
-#define ADE7754_RAENERGY  0x02
-#define ADE7754_LAENERGY  0x03
-#define ADE7754_VAENERGY  0x04
-#define ADE7754_RVAENERGY 0x05
-#define ADE7754_LVAENERGY 0x06
-#define ADE7754_PERIOD0x07
-#define ADE7754_TEMP  0x08
-#define ADE7754_WFORM 0x09
-#define ADE7754_OPMODE0x0A
-#define ADE7754_MMODE 0x0B
-#define ADE7754_WAVMODE   0x0C
-#define ADE7754_WATMODE   0x0D
-#define ADE7754_VAMODE0x0E
-#define ADE7754_IRQEN 0x0F
-#define ADE7754_STATUS0x10
-#define ADE7754_RSTATUS   0x11
-#define ADE7754_ZXTOUT0x12
-#define ADE7754_LINCYC0x13
-#define ADE7754_SAGCYC0x14
-#define ADE7754_SAGLVL0x15
-#define ADE7754_VPEAK 0x16
-#define ADE7754_IPEAK 0x17
-#define ADE7754_GAIN  0x18
-#define ADE7754_AWG   0x19
-#define ADE7754_BWG   0x1A
-#define ADE7754_CWG   0x1B
-#define ADE7754_AVAG  0x1C
-#define ADE7754_BVAG  0x1D
-#define ADE7754_CVAG  0x1E
-#define ADE7754_APHCAL0x1F
-#define ADE7754_BPHCAL0x20
-#define ADE7754_CPHCAL0x21
-#define ADE7754_AAPOS 0x22
-#define ADE7754_BAPOS 0x23
-#define ADE7754_CAPOS 0x24
-#define AD

[PATCH 0/2] staging: iio: ade7754: Header file maintenance

[simran singhal]

Collapse header file into source, and clean up includes in 
implementation.

simran singhal (2):
  staging: ade7754: Move header content to implementation file
  staging: ade7754: Clean up #includes

 drivers/staging/iio/meter/ade7754.c | 98 ++---
 drivers/staging/iio/meter/ade7754.h | 90 --
 2 files changed, 91 insertions(+), 97 deletions(-)
 delete mode 100644 drivers/staging/iio/meter/ade7754.h

-- 
2.7.4

[PATCH 2/2] staging: ade7754: Clean up #includes

[simran singhal]

Alphabetize and separate kernel and subsystem headers.

Signed-off-by: simran singhal 
---
 drivers/staging/iio/meter/ade7754.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7754.c 
b/drivers/staging/iio/meter/ade7754.c
index 42f7b06..fea93d7 100644
--- a/drivers/staging/iio/meter/ade7754.c
+++ b/drivers/staging/iio/meter/ade7754.c
@@ -6,18 +6,17 @@
  * Licensed under the GPL-2 or later.
  */
 
-#include 
-#include 
 #include 
-#include 
 #include 
+#include 
+#include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
-#include 
-#include 
-
 #include 
 #include 
 #include "meter.h"
-- 
2.7.4

[PATCH v7] staging: adis16060_core: Replace mlock with buf_lock and refactor code

[simran singhal]

The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.

In this driver, mlock was being used to protect hardware state
changes. Replace it with buf_lock in the devices global data.

As buf_lock protects both the adis16060_spi_write() and
adis16060_spi_read() functions and both are always called in
pair. First write, then read. Thus, refactor the code to have
one single function adis16060_spi_write_than_read() which is
protected by the existing buf_lock.

Removed nested locks as the function adis16060_read_raw call
a lock on &st->buf_lock and then calls the function
adis16060_spi_write which again tries to get hold
of the same lock.

The locks in adis16060_read_raw are dropped and now have a
single safe function adis16060_spi_write_than_read
with the locks inside it being called.

Signed-off-by: simran singhal 
---

 v7:
   -Change subject
   -Remove lock from read_raw instead of from 
function adis16060_spi_write_than_read().
 
 drivers/staging/iio/gyro/adis16060_core.c | 38 +--
 1 file changed, 11 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/iio/gyro/adis16060_core.c 
b/drivers/staging/iio/gyro/adis16060_core.c
index c9d46e7..193587c 100644
--- a/drivers/staging/iio/gyro/adis16060_core.c
+++ b/drivers/staging/iio/gyro/adis16060_core.c
@@ -40,25 +40,18 @@ struct adis16060_state {
 
 static struct iio_dev *adis16060_iio_dev;
 
-static int adis16060_spi_write(struct iio_dev *indio_dev, u8 val)
+static int adis16060_spi_write_than_read(struct iio_dev *indio_dev,
+u8 conf, u16 *val)
 {
int ret;
struct adis16060_state *st = iio_priv(indio_dev);
 
mutex_lock(&st->buf_lock);
-   st->buf[2] = val; /* The last 8 bits clocked in are latched */
+   st->buf[2] = conf; /* The last 8 bits clocked in are latched */
ret = spi_write(st->us_w, st->buf, 3);
-   mutex_unlock(&st->buf_lock);
-
-   return ret;
-}
-
-static int adis16060_spi_read(struct iio_dev *indio_dev, u16 *val)
-{
-   int ret;
-   struct adis16060_state *st = iio_priv(indio_dev);
 
-   mutex_lock(&st->buf_lock);
+   if (ret < 0)
+   return ret;
 
ret = spi_read(st->us_r, st->buf, 3);
 
@@ -69,8 +62,8 @@ static int adis16060_spi_read(struct iio_dev *indio_dev, u16 
*val)
 */
if (!ret)
*val = ((st->buf[0] & 0x3) << 12) |
-   (st->buf[1] << 4) |
-   ((st->buf[2] >> 4) & 0xF);
+(st->buf[1] << 4) |
+((st->buf[2] >> 4) & 0xF);
mutex_unlock(&st->buf_lock);
 
return ret;
@@ -83,20 +76,15 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
 {
u16 tval = 0;
int ret;
+   struct adis16060_state *st = iio_priv(indio_dev);
 
switch (mask) {
case IIO_CHAN_INFO_RAW:
-   /* Take the iio_dev status lock */
-   mutex_lock(&indio_dev->mlock);
-   ret = adis16060_spi_write(indio_dev, chan->address);
+   ret = adis16060_spi_write_than_read(indio_dev,
+   chan->address, &tval);
if (ret < 0)
-   goto out_unlock;
+   return ret;
 
-   ret = adis16060_spi_read(indio_dev, &tval);
-   if (ret < 0)
-   goto out_unlock;
-
-   mutex_unlock(&indio_dev->mlock);
*val = tval;
return IIO_VAL_INT;
case IIO_CHAN_INFO_OFFSET:
@@ -110,10 +98,6 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
}
 
return -EINVAL;
-
-out_unlock:
-   mutex_unlock(&indio_dev->mlock);
-   return ret;
 }
 
 static const struct iio_info adis16060_info = {
-- 
2.7.4

Re: [Outreachy kernel] [PATCH v5] staging: Use buf_lock instead of mlock and Refactor code

[SIMRAN SINGHAL]

On Mon, Mar 20, 2017 at 2:13 AM, Jonathan Cameron  wrote:
> On 19/03/17 17:14, Gargi Sharma wrote:
>> On Sun, Mar 19, 2017 at 6:20 PM, simran singhal
>>  wrote:
>>> The IIO subsystem is redefining iio_dev->mlock to be used by
>>> the IIO core only for protecting device operating mode changes.
>>> ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
>>>
>>> In this driver, mlock was being used to protect hardware state
>>> changes. Replace it with buf_lock in the devices global data.
>>>
>>> As buf_lock protects both the adis16060_spi_write() and
>>> adis16060_spi_read() functions and both are always called in
>>> pair. First write, then read. Thus, refactor the code to have
>>> one single function adis16060_spi_write_than_read() which is
>>> protected by the existing buf_lock.
>>>
>>> Signed-off-by: simran singhal 
>>> ---
>>>
>>>  v5:
>>>-Rename val in adis16060_spi_write_than_read() to conf.
>>>-Rename val2 in adis16060_spi_write_than_read() to val.
>>>-Corrected Checkpatch issues.
>>>-Removed goto from adis16060_read_raw().
>>>
>>>
>>>  drivers/staging/iio/gyro/adis16060_core.c | 42 
>>> ---
>>>  1 file changed, 16 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/staging/iio/gyro/adis16060_core.c 
>>> b/drivers/staging/iio/gyro/adis16060_core.c
>>> index c9d46e7..0f12492 100644
>>> --- a/drivers/staging/iio/gyro/adis16060_core.c
>>> +++ b/drivers/staging/iio/gyro/adis16060_core.c
>>> @@ -40,25 +40,20 @@ struct adis16060_state {
>>>
>>>  static struct iio_dev *adis16060_iio_dev;
>>>
>>> -static int adis16060_spi_write(struct iio_dev *indio_dev, u8 val)
>>> +static int adis16060_spi_write_than_read(struct iio_dev *indio_dev,
>>> +u8 conf, u16 *val)
>>>  {
>>> int ret;
>>> struct adis16060_state *st = iio_priv(indio_dev);
>>>
>>> mutex_lock(&st->buf_lock);
>>> -   st->buf[2] = val; /* The last 8 bits clocked in are latched */
>>> +   st->buf[2] = conf; /* The last 8 bits clocked in are latched */
>>> ret = spi_write(st->us_w, st->buf, 3);
>>> -   mutex_unlock(&st->buf_lock);
>>>
>>> -   return ret;
>>> -}
>>> -
>>> -static int adis16060_spi_read(struct iio_dev *indio_dev, u16 *val)
>>> -{
>>> -   int ret;
>>> -   struct adis16060_state *st = iio_priv(indio_dev);
>>> -
>>> -   mutex_lock(&st->buf_lock);
>>> +   if (ret < 0) {
>>> +   mutex_unlock(&st->buf_lock);
>>> +   return ret;
>>> +   }
>>>
>>> ret = spi_read(st->us_r, st->buf, 3);
>>>
>>> @@ -69,8 +64,8 @@ static int adis16060_spi_read(struct iio_dev *indio_dev, 
>>> u16 *val)
>>>  */
>>> if (!ret)
>>> *val = ((st->buf[0] & 0x3) << 12) |
>>> -   (st->buf[1] << 4) |
>>> -   ((st->buf[2] >> 4) & 0xF);
>>> +(st->buf[1] << 4) |
>>> +((st->buf[2] >> 4) & 0xF);
>>> mutex_unlock(&st->buf_lock);
>>>
>>> return ret;
>>> @@ -83,20 +78,19 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
>>>  {
>>> u16 tval = 0;
>>> int ret;
>>> +   struct adis16060_state *st = iio_priv(indio_dev);
>>>
>>> switch (mask) {
>>> case IIO_CHAN_INFO_RAW:
>>> /* Take the iio_dev status lock */
>>> -   mutex_lock(&indio_dev->mlock);
>>> -   ret = adis16060_spi_write(indio_dev, chan->address);
>>> +   mutex_lock(&st->buf_lock);
>>> +   ret = adis16060_spi_write_than_read(indio_dev,
>>> +   chan->address, &tval);
>>> if (ret < 0)
>>> -   goto out_unlock;
>>> +   mutex_unlock(&st->buf_lock);
>>> +   return ret;
>>>
>>> -   ret = adis16060_spi_read(indio_dev, &tval);
>>> -

[PATCH v6] staging: Use buf_lock instead of mlock and Refactor code

[simran singhal]

The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.

In this driver, mlock was being used to protect hardware state
changes. Replace it with buf_lock in the devices global data.

As buf_lock protects both the adis16060_spi_write() and
adis16060_spi_read() functions and both are always called in
pair. First write, then read. Thus, refactor the code to have
one single function adis16060_spi_write_than_read() which is
protected by the existing buf_lock.

Removed nested locks as the function adis16060_read_raw call
a lock on &st->buf_lock and then calls the function
adis16060_spi_write which again tries to get hold
of the same lock.

Signed-off-by: simran singhal 
---

 v6:
   -Change commit message
   -Remove nested lock

 drivers/staging/iio/gyro/adis16060_core.c | 40 ++-
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/iio/gyro/adis16060_core.c 
b/drivers/staging/iio/gyro/adis16060_core.c
index c9d46e7..1c6de46 100644
--- a/drivers/staging/iio/gyro/adis16060_core.c
+++ b/drivers/staging/iio/gyro/adis16060_core.c
@@ -40,25 +40,17 @@ struct adis16060_state {
 
 static struct iio_dev *adis16060_iio_dev;
 
-static int adis16060_spi_write(struct iio_dev *indio_dev, u8 val)
+static int adis16060_spi_write_than_read(struct iio_dev *indio_dev,
+u8 conf, u16 *val)
 {
int ret;
struct adis16060_state *st = iio_priv(indio_dev);
 
-   mutex_lock(&st->buf_lock);
-   st->buf[2] = val; /* The last 8 bits clocked in are latched */
+   st->buf[2] = conf; /* The last 8 bits clocked in are latched */
ret = spi_write(st->us_w, st->buf, 3);
-   mutex_unlock(&st->buf_lock);
-
-   return ret;
-}
-
-static int adis16060_spi_read(struct iio_dev *indio_dev, u16 *val)
-{
-   int ret;
-   struct adis16060_state *st = iio_priv(indio_dev);
 
-   mutex_lock(&st->buf_lock);
+   if (ret < 0)
+   return ret;
 
ret = spi_read(st->us_r, st->buf, 3);
 
@@ -69,8 +61,8 @@ static int adis16060_spi_read(struct iio_dev *indio_dev, u16 
*val)
 */
if (!ret)
*val = ((st->buf[0] & 0x3) << 12) |
-   (st->buf[1] << 4) |
-   ((st->buf[2] >> 4) & 0xF);
+(st->buf[1] << 4) |
+((st->buf[2] >> 4) & 0xF);
mutex_unlock(&st->buf_lock);
 
return ret;
@@ -83,20 +75,18 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
 {
u16 tval = 0;
int ret;
+   struct adis16060_state *st = iio_priv(indio_dev);
 
switch (mask) {
case IIO_CHAN_INFO_RAW:
/* Take the iio_dev status lock */
-   mutex_lock(&indio_dev->mlock);
-   ret = adis16060_spi_write(indio_dev, chan->address);
+   mutex_lock(&st->buf_lock);
+   ret = adis16060_spi_write_than_read(indio_dev,
+   chan->address, &tval);
+   mutex_unlock(&st->buf_lock);
if (ret < 0)
-   goto out_unlock;
+   return ret;
 
-   ret = adis16060_spi_read(indio_dev, &tval);
-   if (ret < 0)
-   goto out_unlock;
-
-   mutex_unlock(&indio_dev->mlock);
*val = tval;
return IIO_VAL_INT;
case IIO_CHAN_INFO_OFFSET:
@@ -110,10 +100,6 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
}
 
return -EINVAL;
-
-out_unlock:
-   mutex_unlock(&indio_dev->mlock);
-   return ret;
 }
 
 static const struct iio_info adis16060_info = {
-- 
2.7.4

Re: [Outreachy kernel] [PATCH] staging: impedance-analyzer: ad5933: Remove unnecessary goto

[SIMRAN SINGHAL]

On Sun, Mar 19, 2017 at 11:26 PM, Julia Lawall  wrote:
>
>
> On Sun, 19 Mar 2017, simran singhal wrote:
>
>> Remove unnecessary goto.
>>
>> Signed-off-by: simran singhal 
>> ---
>>  drivers/staging/iio/impedance-analyzer/ad5933.c | 10 --
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c 
>> b/drivers/staging/iio/impedance-analyzer/ad5933.c
>> index 297665d..c9ed2ac 100644
>> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
>> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
>> @@ -663,12 +663,12 @@ static void ad5933_work(struct work_struct *work)
>>   ad5933_cmd(st, AD5933_CTRL_START_SWEEP);
>>   st->state = AD5933_CTRL_START_SWEEP;
>>   schedule_delayed_work(&st->work, st->poll_time_jiffies);
>> - goto out;
>> + mutex_unlock(&indio_dev->mlock);
>
> This is not correct.  The code at the end of the function implicitly does
> a return, which has disappeared in the change.  You could add a return to
> each place wher you have put a mutex_unlock, but I think that the code was
> better off as it was.  It's microscopically less error prone to have this
> cleanup code in only one place.
>
OOPS!! This is totally wrong.
Sorry for this.

> julia
>
>>   }
>>
>>   ret = ad5933_i2c_read(st->client, AD5933_REG_STATUS, 1, &status);
>>   if (ret)
>> - goto out;
>> + mutex_unlock(&indio_dev->mlock);
>>
>>   if (status & AD5933_STAT_DATA_VALID) {
>>   int scan_count = bitmap_weight(indio_dev->active_scan_mask,
>> @@ -678,7 +678,7 @@ static void ad5933_work(struct work_struct *work)
>>   AD5933_REG_REAL_DATA : AD5933_REG_IMAG_DATA,
>>   scan_count * 2, (u8 *)buf);
>>   if (ret)
>> - goto out;
>> + mutex_unlock(&indio_dev->mlock);
>>
>>   if (scan_count == 2) {
>>   val[0] = be16_to_cpu(buf[0]);
>> @@ -690,7 +690,7 @@ static void ad5933_work(struct work_struct *work)
>>   } else {
>>   /* no data available - try again later */
>>   schedule_delayed_work(&st->work, st->poll_time_jiffies);
>> - goto out;
>> + mutex_unlock(&indio_dev->mlock);
>>   }
>>
>>   if (status & AD5933_STAT_SWEEP_DONE) {
>> @@ -703,8 +703,6 @@ static void ad5933_work(struct work_struct *work)
>>   ad5933_cmd(st, AD5933_CTRL_INC_FREQ);
>>   schedule_delayed_work(&st->work, st->poll_time_jiffies);
>>   }
>> -out:
>> - mutex_unlock(&indio_dev->mlock);
>>  }
>>
>>  static int ad5933_probe(struct i2c_client *client,
>> --
>> 2.7.4
>>
>> --
>> You received this message because you are subscribed to the Google Groups 
>> "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to outreachy-kernel+unsubscr...@googlegroups.com.
>> To post to this group, send email to outreachy-ker...@googlegroups.com.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/outreachy-kernel/20170319175133.GA19710%40singhal-Inspiron-5558.
>> For more options, visit https://groups.google.com/d/optout.
>>

[PATCH] staging: impedance-analyzer: ad5933: Remove unnecessary goto

[simran singhal]

Remove unnecessary goto.

Signed-off-by: simran singhal 
---
 drivers/staging/iio/impedance-analyzer/ad5933.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c 
b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 297665d..c9ed2ac 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -663,12 +663,12 @@ static void ad5933_work(struct work_struct *work)
ad5933_cmd(st, AD5933_CTRL_START_SWEEP);
st->state = AD5933_CTRL_START_SWEEP;
schedule_delayed_work(&st->work, st->poll_time_jiffies);
-   goto out;
+   mutex_unlock(&indio_dev->mlock);
}
 
ret = ad5933_i2c_read(st->client, AD5933_REG_STATUS, 1, &status);
if (ret)
-   goto out;
+   mutex_unlock(&indio_dev->mlock);
 
if (status & AD5933_STAT_DATA_VALID) {
int scan_count = bitmap_weight(indio_dev->active_scan_mask,
@@ -678,7 +678,7 @@ static void ad5933_work(struct work_struct *work)
AD5933_REG_REAL_DATA : AD5933_REG_IMAG_DATA,
scan_count * 2, (u8 *)buf);
if (ret)
-   goto out;
+   mutex_unlock(&indio_dev->mlock);
 
if (scan_count == 2) {
val[0] = be16_to_cpu(buf[0]);
@@ -690,7 +690,7 @@ static void ad5933_work(struct work_struct *work)
} else {
/* no data available - try again later */
schedule_delayed_work(&st->work, st->poll_time_jiffies);
-   goto out;
+   mutex_unlock(&indio_dev->mlock);
}
 
if (status & AD5933_STAT_SWEEP_DONE) {
@@ -703,8 +703,6 @@ static void ad5933_work(struct work_struct *work)
ad5933_cmd(st, AD5933_CTRL_INC_FREQ);
schedule_delayed_work(&st->work, st->poll_time_jiffies);
}
-out:
-   mutex_unlock(&indio_dev->mlock);
 }
 
 static int ad5933_probe(struct i2c_client *client,
-- 
2.7.4

[PATCH v5] staging: Use buf_lock instead of mlock and Refactor code

[simran singhal]

The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.

In this driver, mlock was being used to protect hardware state
changes. Replace it with buf_lock in the devices global data.

As buf_lock protects both the adis16060_spi_write() and
adis16060_spi_read() functions and both are always called in
pair. First write, then read. Thus, refactor the code to have
one single function adis16060_spi_write_than_read() which is
protected by the existing buf_lock.

Signed-off-by: simran singhal 
---

 v5:
   -Rename val in adis16060_spi_write_than_read() to conf.
   -Rename val2 in adis16060_spi_write_than_read() to val.
   -Corrected Checkpatch issues.
   -Removed goto from adis16060_read_raw().


 drivers/staging/iio/gyro/adis16060_core.c | 42 ---
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/iio/gyro/adis16060_core.c 
b/drivers/staging/iio/gyro/adis16060_core.c
index c9d46e7..0f12492 100644
--- a/drivers/staging/iio/gyro/adis16060_core.c
+++ b/drivers/staging/iio/gyro/adis16060_core.c
@@ -40,25 +40,20 @@ struct adis16060_state {
 
 static struct iio_dev *adis16060_iio_dev;
 
-static int adis16060_spi_write(struct iio_dev *indio_dev, u8 val)
+static int adis16060_spi_write_than_read(struct iio_dev *indio_dev,
+u8 conf, u16 *val)
 {
int ret;
struct adis16060_state *st = iio_priv(indio_dev);
 
mutex_lock(&st->buf_lock);
-   st->buf[2] = val; /* The last 8 bits clocked in are latched */
+   st->buf[2] = conf; /* The last 8 bits clocked in are latched */
ret = spi_write(st->us_w, st->buf, 3);
-   mutex_unlock(&st->buf_lock);
 
-   return ret;
-}
-
-static int adis16060_spi_read(struct iio_dev *indio_dev, u16 *val)
-{
-   int ret;
-   struct adis16060_state *st = iio_priv(indio_dev);
-
-   mutex_lock(&st->buf_lock);
+   if (ret < 0) {
+   mutex_unlock(&st->buf_lock);
+   return ret;
+   }
 
ret = spi_read(st->us_r, st->buf, 3);
 
@@ -69,8 +64,8 @@ static int adis16060_spi_read(struct iio_dev *indio_dev, u16 
*val)
 */
if (!ret)
*val = ((st->buf[0] & 0x3) << 12) |
-   (st->buf[1] << 4) |
-   ((st->buf[2] >> 4) & 0xF);
+(st->buf[1] << 4) |
+((st->buf[2] >> 4) & 0xF);
mutex_unlock(&st->buf_lock);
 
return ret;
@@ -83,20 +78,19 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
 {
u16 tval = 0;
int ret;
+   struct adis16060_state *st = iio_priv(indio_dev);
 
switch (mask) {
case IIO_CHAN_INFO_RAW:
/* Take the iio_dev status lock */
-   mutex_lock(&indio_dev->mlock);
-   ret = adis16060_spi_write(indio_dev, chan->address);
+   mutex_lock(&st->buf_lock);
+   ret = adis16060_spi_write_than_read(indio_dev,
+   chan->address, &tval);
if (ret < 0)
-   goto out_unlock;
+   mutex_unlock(&st->buf_lock);
+   return ret;
 
-   ret = adis16060_spi_read(indio_dev, &tval);
-   if (ret < 0)
-   goto out_unlock;
-
-   mutex_unlock(&indio_dev->mlock);
+   mutex_unlock(&st->buf_lock);
*val = tval;
return IIO_VAL_INT;
case IIO_CHAN_INFO_OFFSET:
@@ -110,10 +104,6 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
}
 
return -EINVAL;
-
-out_unlock:
-   mutex_unlock(&indio_dev->mlock);
-   return ret;
 }
 
 static const struct iio_info adis16060_info = {
-- 
2.7.4

Re: [PATCH v4] staging: Use buf_lock instead of mlock and Refactor code

[SIMRAN SINGHAL]

On Sun, Mar 19, 2017 at 3:45 PM, Jonathan Cameron  wrote:
> On 18/03/17 18:44, simran singhal wrote:
>> The IIO subsystem is redefining iio_dev->mlock to be used by
>> the IIO core only for protecting device operating mode changes.
>> ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
>>
>> In this driver, mlock was being used to protect hardware state
>> changes. Replace it with buf_lock in the devices global data.
>>
>> As buf_lock protects both the adis16060_spi_write() and
>> adis16060_spi_read() functions and both are always called in
>> pair. First write, then read. Thus, refactor the code to have
>> one single function adis16060_spi_write_than_read() which is
>> protected by the existing buf_lock.
>>
>> Signed-off-by: simran singhal 
> Hi Simran,
>
> A couple of minor comments and opportunity to further improve
> the, now simpler, code flow.
>
> Thanks,
>
> Jonathan
>> ---
>>
>>  v4:
>>-Refactored code
>>-change commit subject
>>-change commit message
>>
>>  drivers/staging/iio/gyro/adis16060_core.c | 37 
>> ---
>>  1 file changed, 14 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/staging/iio/gyro/adis16060_core.c 
>> b/drivers/staging/iio/gyro/adis16060_core.c
>> index c9d46e7..39ddd55 100644
>> --- a/drivers/staging/iio/gyro/adis16060_core.c
>> +++ b/drivers/staging/iio/gyro/adis16060_core.c
>> @@ -40,7 +40,7 @@ struct adis16060_state {
>>
>>  static struct iio_dev *adis16060_iio_dev;
>>
>> -static int adis16060_spi_write(struct iio_dev *indio_dev, u8 val)
>> +static int adis16060_spi_write_than_read(struct iio_dev *indio_dev, u8 val, 
>> u16 *val2)
> val and val2 often have specific meanings in IIO.  I'd prefer these to be 
> renamed to
> val => conf (as it sets the configuration) and val2 => val.
>>  {
>>   int ret;
>>   struct adis16060_state *st = iio_priv(indio_dev);
>> @@ -48,17 +48,11 @@ static int adis16060_spi_write(struct iio_dev 
>> *indio_dev, u8 val)
>>   mutex_lock(&st->buf_lock);
>>   st->buf[2] = val; /* The last 8 bits clocked in are latched */
>>   ret = spi_write(st->us_w, st->buf, 3);
>> - mutex_unlock(&st->buf_lock);
>>
>> - return ret;
>> -}
>> -
>> -static int adis16060_spi_read(struct iio_dev *indio_dev, u16 *val)
>> -{
>> - int ret;
>> - struct adis16060_state *st = iio_priv(indio_dev);
>> -
>> - mutex_lock(&st->buf_lock);
>> + if (ret < 0) {
>> + mutex_unlock(&st->buf_lock);
>> + return ret;
>> + }
>>
>>   ret = spi_read(st->us_r, st->buf, 3);
>>
>> @@ -67,10 +61,10 @@ static int adis16060_spi_read(struct iio_dev *indio_dev, 
>> u16 *val)
>>* starts to place data MSB first on the DOUT line at
>>* the 6th falling edge of SCLK
>>*/
>> - if (!ret)
>> - *val = ((st->buf[0] & 0x3) << 12) |
>> - (st->buf[1] << 4) |
>> - ((st->buf[2] >> 4) & 0xF);
>> +if (!ret)
> Please run checkpatch.pl over your patches.  Looks like
> you ended up with spaces instead of a tab in the line above.

OOPS! My bad should have run checkpatch.pl
over patch.

Will correct all the checkpatch.pl issues and resend.

>> + *val2 = ((st->buf[0] & 0x3) << 12) |
>> +  (st->buf[1] << 4) |
>> +  ((st->buf[2] >> 4) & 0xF);
>>   mutex_unlock(&st->buf_lock);
>>
>>   return ret;
>> @@ -83,20 +77,17 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
>>  {
>>   u16 tval = 0;
>>   int ret;
>> + struct adis16060_state *st = iio_priv(indio_dev);
>>
>>   switch (mask) {
>>   case IIO_CHAN_INFO_RAW:
>>   /* Take the iio_dev status lock */
>> - mutex_lock(&indio_dev->mlock);
>> - ret = adis16060_spi_write(indio_dev, chan->address);
>> + mutex_lock(&st->buf_lock);
>> + ret = adis16060_spi_write_than_read(indio_dev, chan->address, 
>> &tval);
>>   if (ret < 0)
>>   goto out_unlock;
>>
>> - ret = adis16060_spi_read(indio_dev, &tval);
>> - if (ret < 0)
> This ugly goto construction arguably made sense when there were two of them.  
> Now
> you have only one just bring the mutex_unlock inline and return directly.

Will remove this and resend.

>> - goto out_unlock;
>> -
>> - mutex_unlock(&indio_dev->mlock);
>> + mutex_unlock(&st->buf_lock);
>>   *val = tval;
>>   return IIO_VAL_INT;
>>   case IIO_CHAN_INFO_OFFSET:
>> @@ -112,7 +103,7 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
>>   return -EINVAL;
>>
>>  out_unlock:
>> - mutex_unlock(&indio_dev->mlock);
>> + mutex_unlock(&st->buf_lock);
>>   return ret;
>>  }
>>
>>
>

Re: [Outreachy kernel] [PATCH v2] staging: iio: ade7753: replace mlock with driver private lock

[SIMRAN SINGHAL]

On Sat, Mar 18, 2017 at 11:51 PM, Jonathan Cameron  wrote:
> On 18/03/17 17:34, SIMRAN SINGHAL wrote:
>> On Thu, Mar 16, 2017 at 3:23 AM, Jonathan Cameron  wrote:
>>> On 13/03/17 19:53, Alison Schofield wrote:
>>>> On Mon, Mar 13, 2017 at 10:01:07PM +0530, simran singhal wrote:
>>>>> The IIO subsystem is redefining iio_dev->mlock to be used by
>>>>> the IIO core only for protecting device operating mode changes.
>>>>> ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
>>>>>
>>>>> In this driver, mlock was being used to protect hardware state
>>>>> changes.  Replace it with a lock in the devices global data.
>>>>>
>>>>> Fix some coding style issues related to white space also.
>>>>>
>>>>> Signed-off-by: simran singhal 
>>>>> ---
>>>>>
>>>>>  v2:
>>>>>-Removed new lock to reuse the existing lock
>>>> Simran,
>>>>
>>>> The good news is that you have 2 patches that have similar
>>>> challenges!  I'll suggest picking one, drive it to completion,
>>>> then do the other.
>>>>
>>>> This has the nested locking issue that Lars warned about.
>>>> Need to refactor to avoid.  Check back on his review comments.
>>>>
>>>> I suggest dropping those whitespace changes - they appear
>>>> out of place in this patch since you are no longer actually
>>>> touching those lines of code.
>>>>
>>>> alisons
>>> Also missing a mutex_init, though that may well become irrelevant
>>> with refactoring as suggested.
>>
>> Jonathon, what I found by looking at the codes of other drivers is that
>> we have to use mutex_init in probe function only.
>>
>> Is this correct?
> Yes, it only needs initializing once.
>>
So, no need of using mutex_init here as the function is not a
probe function.

Thanks
Simran

>> And this patch also suggest the same:-
>> https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=testing&id=388c8f18ff29fe95dbf72cb0a1bd8fbcd6f52f8f
>>
>>>>
>>>>
>>>>>
>>>>>  drivers/staging/iio/meter/ade7753.c | 12 ++--
>>>>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/drivers/staging/iio/meter/ade7753.c 
>>>>> b/drivers/staging/iio/meter/ade7753.c
>>>>> index dfd8b71..d88eaa3 100644
>>>>> --- a/drivers/staging/iio/meter/ade7753.c
>>>>> +++ b/drivers/staging/iio/meter/ade7753.c
>>>>> @@ -83,10 +83,10 @@
>>>>>   * @buf_lock:   mutex to protect tx and rx
>>>>>   **/
>>>>>  struct ade7753_state {
>>>>> -struct spi_device   *us;
>>>>> -struct mutexbuf_lock;
>>>>> -u8  tx[ADE7753_MAX_TX] 
>>>>> cacheline_aligned;
>>>>> -u8  rx[ADE7753_MAX_RX];
>>>>> +struct spi_device   *us;
>>>>> +struct mutexbuf_lock;
>>>>> +u8  tx[ADE7753_MAX_TX] cacheline_aligned;
>>>>> +u8  rx[ADE7753_MAX_RX];
>>>>>  };
>>>>>
>>>>>  static int ade7753_spi_write_reg_8(struct device *dev,
>>>>> @@ -484,7 +484,7 @@ static ssize_t ade7753_write_frequency(struct device 
>>>>> *dev,
>>>>>  if (!val)
>>>>>  return -EINVAL;
>>>>>
>>>>> -mutex_lock(&indio_dev->mlock);
>>>>> +mutex_lock(&st->buf_lock);
>>>>>
>>>>>  t = 27900 / val;
>>>>>  if (t > 0)
>>>>> @@ -505,7 +505,7 @@ static ssize_t ade7753_write_frequency(struct device 
>>>>> *dev,
>>>>>  ret = ade7753_spi_write_reg_16(dev, ADE7753_MODE, reg);
>>>>>
>>>>>  out:
>>>>> -mutex_unlock(&indio_dev->mlock);
>>>>> +mutex_unlock(&st->buf_lock);
>>>>>
>>>>>  return ret ? ret : len;
>>>>>  }
>>>>> --
>>>>> 2.7.4
>>>>>
>>>>> --
>>>>> You received this message because you are subscribed to the Google Groups 
>>>>> "outreachy-kernel" group.
>>>>> To unsubscribe from this group and stop receiving emails from it, send an 
>>>>> email to outreachy-kernel+unsubscr...@googlegroups.com.
>>>>> To post to this group, send email to outreachy-ker...@googlegroups.com.
>>>>> To view this discussion on the web visit 
>>>>> https://groups.google.com/d/msgid/outreachy-kernel/20170313163107.GA31496%40singhal-Inspiron-5558.
>>>>> For more options, visit https://groups.google.com/d/optout.
>>>> --
>>>> 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
>>>>
>>>
>