[PATCH 0/3] staging: wilc1000

[PATCH 0/3] staging: wilc1000

[Ioannis Valasakis]

This is a patchset that fixes all the source related issues 
that were reported by checkpatch.

Ioannis Valasakis (3):
  staging: wilc1000: replace udelay with usleep_range
  staging: wilc1000: prefer 'help' in KConfig
  staging: wilc1000: Change struct members from bool to u8

 drivers/staging/wilc1000/Kconfig              |  8 ++++----
 drivers/staging/wilc1000/coreconfigurator.h   |  4 ++--
 drivers/staging/wilc1000/host_interface.h     |  8 ++++----
 drivers/staging/wilc1000/wilc_wfi_netdevice.h | 10 +++++-----
 drivers/staging/wilc1000/wilc_wlan.c          |  2 +-
 5 files changed, 16 insertions(+), 16 deletions(-)

-- 
2.19.1

[PATCH 1/3] staging: wilc1000: replace udelay with usleep_range

[Ioannis Valasakis]

Fix warning and replace a udelay inside a non-atomic context and
it can be safely replace by a usleep_range(t, t + delta), where delta is
20% of t, as that is between 10 and 20 microseconds.
Reported by checkpatch.

Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
---
 drivers/staging/wilc1000/wilc_wlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 8b184aa30d25..661c594b0a3c 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -436,7 +436,7 @@ void chip_wakeup(struct wilc *wilc)
 		} while (wilc_get_chipid(wilc, true) == 0);
 	} else if ((wilc->io_type & 0x1) == HIF_SDIO) {
 		wilc->hif_func->hif_write_reg(wilc, 0xfa, 1);
-		udelay(200);
+		usleep_range(200, 210);
 		wilc->hif_func->hif_read_reg(wilc, 0xf0, &reg);
 		do {
 			wilc->hif_func->hif_write_reg(wilc, 0xf0,
-- 
2.19.1

[PATCH 2/3] staging: wilc1000: prefer 'help' in KConfig

[Ioannis Valasakis]

Fix for a style warning using the help without the
deprecated help included between dashes.
Reported by checkpatch.

Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
---
 drivers/staging/wilc1000/Kconfig | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/wilc1000/Kconfig b/drivers/staging/wilc1000/Kconfig
index 73f7fefd3bc3..f9d3ad41c862 100644
--- a/drivers/staging/wilc1000/Kconfig
+++ b/drivers/staging/wilc1000/Kconfig
@@ -1,13 +1,13 @@
 config WILC1000
 	tristate
-	---help---
+	help
 	  This module only support IEEE 802.11n WiFi.
 
 config WILC1000_SDIO
 	tristate "Atmel WILC1000 SDIO (WiFi only)"
 	depends on CFG80211 && INET && MMC
 	select WILC1000
-	---help---
+	help
 	  This module adds support for the SDIO interface of adapters using
 	  WILC1000 chipset. The Atmel WILC1000 SDIO is a full speed interface.
 	  It meets SDIO card specification version 2.0. The interface supports
@@ -21,7 +21,7 @@ config WILC1000_SPI
 	tristate "Atmel WILC1000 SPI (WiFi only)"
 	depends on CFG80211 && INET && SPI
 	select WILC1000
-	---help---
+	help
 	  This module adds support for the SPI interface of adapters using
 	  WILC1000 chipset. The Atmel WILC1000 has a Serial Peripheral
 	  Interface (SPI) that operates as a SPI slave. This SPI interface can
@@ -34,7 +34,7 @@ config WILC1000_HW_OOB_INTR
 	bool "WILC1000 out of band interrupt"
 	depends on WILC1000_SDIO
 	default n
-	---help---
+	help
 	  This option enables out-of-band interrupt support for the WILC1000
 	  chipset. This OOB interrupt is intended to provide a faster interrupt
 	  mechanism for SDIO host controllers that don't support SDIO interrupt.
-- 
2.19.1

[PATCH 3/3] staging: wilc1000: Change struct members from bool to u8

[Ioannis Valasakis]

Recent versions of checkpatch have a new warning based on a documented
preference of Linus to not use bool in structures due to wasted space and
the size of bool is implementation dependent.  For more information, see
the email thread at https://lkml.org/lkml/2017/11/21/384.

Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
---
 drivers/staging/wilc1000/coreconfigurator.h   |  4 ++--
 drivers/staging/wilc1000/host_interface.h     |  8 ++++----
 drivers/staging/wilc1000/wilc_wfi_netdevice.h | 10 +++++-----
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/wilc1000/coreconfigurator.h b/drivers/staging/wilc1000/coreconfigurator.h
index b62acb447383..dbea5144395e 100644
--- a/drivers/staging/wilc1000/coreconfigurator.h
+++ b/drivers/staging/wilc1000/coreconfigurator.h
@@ -30,7 +30,7 @@
 #define MAKE_WORD32(lsw, msw) ((((u32)(msw) << 16) & 0xFFFF0000) | (lsw))
 
 struct rssi_history_buffer {
-	bool full;
+	u8 full;
 	u8 index;
 	s8 samples[NUM_RSSI];
 };
@@ -46,7 +46,7 @@ struct network_info {
 	u8 ch;
 	unsigned long time_scan_cached;
 	unsigned long time_scan;
-	bool new_network;
+	u8 new_network;
 	u8 found;
 	u32 tsf_lo;
 	u8 *ies;
diff --git a/drivers/staging/wilc1000/host_interface.h b/drivers/staging/wilc1000/host_interface.h
index 84866a62a4d4..f76dc682f882 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -217,7 +217,7 @@ struct user_conn_req {
 	u8 *ies;
 	size_t ies_len;
 	wilc_connect_result conn_result;
-	bool ht_capable;
+	u8 ht_capable;
 	void *arg;
 };
 
@@ -252,7 +252,7 @@ struct remain_ch {
 };
 
 struct reg_frame {
-	bool reg;
+	u8 reg;
 	u16 frame_type;
 	u8 reg_id;
 };
@@ -282,7 +282,7 @@ struct host_if_drv {
 	struct timer_list remain_on_ch_timer;
 	struct wilc_vif *remain_on_ch_timer_vif;
 
-	bool ifc_up;
+	u8 ifc_up;
 	int driver_handler_id;
 };
 
@@ -291,7 +291,7 @@ struct add_sta_param {
 	u16 aid;
 	u8 rates_len;
 	const u8 *rates;
-	bool ht_supported;
+	u8 ht_supported;
 	struct ieee80211_ht_cap ht_capa;
 	u16 flags_mask;
 	u16 flags_set;
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index b7eee772f3fe..347b35ceead4 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -75,7 +75,7 @@ struct wilc_priv {
 	struct wilc_wfi_p2p_listen_params remain_on_ch_params;
 	u64 tx_cookie;
 
-	bool cfg_scanning;
+	u8 cfg_scanning;
 	u32 rcvd_ch_cnt;
 
 	u8 associated_bss[ETH_ALEN];
@@ -93,13 +93,13 @@ struct wilc_priv {
 	u8 wilc_groupkey;
 	/* mutexes */
 	struct mutex scan_req_lock;
-	bool p2p_listen_state;
+	u8 p2p_listen_state;
 
 };
 
 struct frame_reg {
 	u16 type;
-	bool reg;
+	u8 reg;
 };
 
 struct wilc_vif {
@@ -123,7 +123,7 @@ struct wilc {
 	int io_type;
 	int mac_status;
 	struct gpio_desc *gpio_irq;
-	bool initialized;
+	u8 initialized;
 	int dev_irq_num;
 	int close;
 	u8 vif_num;
@@ -163,7 +163,7 @@ struct wilc {
 	const struct firmware *firmware;
 
 	struct device *dev;
-	bool suspend_event;
+	u8 suspend_event;
 
 	struct rf_info dummy_statistics;
 };
-- 
2.19.1

Re: [Outreachy kernel] [PATCH 1/3] staging: wilc1000: replace udelay with usleep_range

[Julia Lawall]

On Wed, 10 Oct 2018, Ioannis Valasakis wrote:

> Fix warning and replace a udelay inside a non-atomic context and
> it can be safely replace by a usleep_range(t, t + delta), where delta is
> 20% of t, as that is between 10 and 20 microseconds.
> Reported by checkpatch.

It looks like your delta is 5%?

julia

>
> Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
> ---
>  drivers/staging/wilc1000/wilc_wlan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
> index 8b184aa30d25..661c594b0a3c 100644
> --- a/drivers/staging/wilc1000/wilc_wlan.c
> +++ b/drivers/staging/wilc1000/wilc_wlan.c
> @@ -436,7 +436,7 @@ void chip_wakeup(struct wilc *wilc)
>  		} while (wilc_get_chipid(wilc, true) == 0);
>  	} else if ((wilc->io_type & 0x1) == HIF_SDIO) {
>  		wilc->hif_func->hif_write_reg(wilc, 0xfa, 1);
> -		udelay(200);
> +		usleep_range(200, 210);
>  		wilc->hif_func->hif_read_reg(wilc, 0xf0, &reg);
>  		do {
>  			wilc->hif_func->hif_write_reg(wilc, 0xf0,
> --
> 2.19.1
>
>
> --
> 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+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/6eca799b032fdd5f674f55087c926b1957e22e5c.1539182155.git.code%40wizofe.uk.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH 3/3] staging: wilc1000: Change struct members from bool to u8

[Julia Lawall]

On Wed, 10 Oct 2018, Ioannis Valasakis wrote:

> Recent versions of checkpatch have a new warning based on a documented
> preference of Linus to not use bool in structures due to wasted space and
> the size of bool is implementation dependent.  For more information, see
> the email thread at https://lkml.org/lkml/2017/11/21/384.

I can see Linus's point of view, but it seems sad anyway, as it allows all
sorts of values for somthing that should only be true or false.

julia

>
> Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
> ---
>  drivers/staging/wilc1000/coreconfigurator.h   |  4 ++--
>  drivers/staging/wilc1000/host_interface.h     |  8 ++++----
>  drivers/staging/wilc1000/wilc_wfi_netdevice.h | 10 +++++-----
>  3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/coreconfigurator.h b/drivers/staging/wilc1000/coreconfigurator.h
> index b62acb447383..dbea5144395e 100644
> --- a/drivers/staging/wilc1000/coreconfigurator.h
> +++ b/drivers/staging/wilc1000/coreconfigurator.h
> @@ -30,7 +30,7 @@
>  #define MAKE_WORD32(lsw, msw) ((((u32)(msw) << 16) & 0xFFFF0000) | (lsw))
>
>  struct rssi_history_buffer {
> -	bool full;
> +	u8 full;
>  	u8 index;
>  	s8 samples[NUM_RSSI];
>  };
> @@ -46,7 +46,7 @@ struct network_info {
>  	u8 ch;
>  	unsigned long time_scan_cached;
>  	unsigned long time_scan;
> -	bool new_network;
> +	u8 new_network;
>  	u8 found;
>  	u32 tsf_lo;
>  	u8 *ies;
> diff --git a/drivers/staging/wilc1000/host_interface.h b/drivers/staging/wilc1000/host_interface.h
> index 84866a62a4d4..f76dc682f882 100644
> --- a/drivers/staging/wilc1000/host_interface.h
> +++ b/drivers/staging/wilc1000/host_interface.h
> @@ -217,7 +217,7 @@ struct user_conn_req {
>  	u8 *ies;
>  	size_t ies_len;
>  	wilc_connect_result conn_result;
> -	bool ht_capable;
> +	u8 ht_capable;
>  	void *arg;
>  };
>
> @@ -252,7 +252,7 @@ struct remain_ch {
>  };
>
>  struct reg_frame {
> -	bool reg;
> +	u8 reg;
>  	u16 frame_type;
>  	u8 reg_id;
>  };
> @@ -282,7 +282,7 @@ struct host_if_drv {
>  	struct timer_list remain_on_ch_timer;
>  	struct wilc_vif *remain_on_ch_timer_vif;
>
> -	bool ifc_up;
> +	u8 ifc_up;
>  	int driver_handler_id;
>  };
>
> @@ -291,7 +291,7 @@ struct add_sta_param {
>  	u16 aid;
>  	u8 rates_len;
>  	const u8 *rates;
> -	bool ht_supported;
> +	u8 ht_supported;
>  	struct ieee80211_ht_cap ht_capa;
>  	u16 flags_mask;
>  	u16 flags_set;
> diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> index b7eee772f3fe..347b35ceead4 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> +++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> @@ -75,7 +75,7 @@ struct wilc_priv {
>  	struct wilc_wfi_p2p_listen_params remain_on_ch_params;
>  	u64 tx_cookie;
>
> -	bool cfg_scanning;
> +	u8 cfg_scanning;
>  	u32 rcvd_ch_cnt;
>
>  	u8 associated_bss[ETH_ALEN];
> @@ -93,13 +93,13 @@ struct wilc_priv {
>  	u8 wilc_groupkey;
>  	/* mutexes */
>  	struct mutex scan_req_lock;
> -	bool p2p_listen_state;
> +	u8 p2p_listen_state;
>
>  };
>
>  struct frame_reg {
>  	u16 type;
> -	bool reg;
> +	u8 reg;
>  };
>
>  struct wilc_vif {
> @@ -123,7 +123,7 @@ struct wilc {
>  	int io_type;
>  	int mac_status;
>  	struct gpio_desc *gpio_irq;
> -	bool initialized;
> +	u8 initialized;
>  	int dev_irq_num;
>  	int close;
>  	u8 vif_num;
> @@ -163,7 +163,7 @@ struct wilc {
>  	const struct firmware *firmware;
>
>  	struct device *dev;
> -	bool suspend_event;
> +	u8 suspend_event;
>
>  	struct rf_info dummy_statistics;
>  };
> --
> 2.19.1
>
>
> --
> 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+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/e409baf10c6cc043344f1a63712d661611953973.1539182155.git.code%40wizofe.uk.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH 1/3] staging: wilc1000: replace udelay with usleep_range

[Ioannis Valasakis]

On Wed, Oct 10, 2018 at 04:54:38PM +0200, Julia Lawall wrote:
> 
> 
> On Wed, 10 Oct 2018, Ioannis Valasakis wrote:
> 
> > Fix warning and replace a udelay inside a non-atomic context and
> > it can be safely replace by a usleep_range(t, t + delta), where delta is
> > 20% of t, as that is between 10 and 20 microseconds.
> > Reported by checkpatch.
> 
> It looks like your delta is 5%?
> 
> julia

Indeed. Thanks for the quick reply, I am going to submit a new version.

> 
> >
> > Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
> > ---
> >  drivers/staging/wilc1000/wilc_wlan.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
> > index 8b184aa30d25..661c594b0a3c 100644
> > --- a/drivers/staging/wilc1000/wilc_wlan.c
> > +++ b/drivers/staging/wilc1000/wilc_wlan.c
> > @@ -436,7 +436,7 @@ void chip_wakeup(struct wilc *wilc)
> >  		} while (wilc_get_chipid(wilc, true) == 0);
> >  	} else if ((wilc->io_type & 0x1) == HIF_SDIO) {
> >  		wilc->hif_func->hif_write_reg(wilc, 0xfa, 1);
> > -		udelay(200);
> > +		usleep_range(200, 210);
> >  		wilc->hif_func->hif_read_reg(wilc, 0xf0, &reg);
> >  		do {
> >  			wilc->hif_func->hif_write_reg(wilc, 0xf0,
> > --
> > 2.19.1
> >
> >
> > --
> > 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+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/6eca799b032fdd5f674f55087c926b1957e22e5c.1539182155.git.code%40wizofe.uk.
> > For more options, visit https://groups.google.com/d/optout.
> >

Re: [Outreachy kernel] [PATCH 1/3] staging: wilc1000: replace udelay with usleep_range

[Julia Lawall]

On Wed, 10 Oct 2018, Ioannis Valasakis wrote:

> On Wed, Oct 10, 2018 at 04:54:38PM +0200, Julia Lawall wrote:
> >
> >
> > On Wed, 10 Oct 2018, Ioannis Valasakis wrote:
> >
> > > Fix warning and replace a udelay inside a non-atomic context and
> > > it can be safely replace by a usleep_range(t, t + delta), where delta is
> > > 20% of t, as that is between 10 and 20 microseconds.
> > > Reported by checkpatch.
> >
> > It looks like your delta is 5%?
> >
> > julia
>
> Indeed. Thanks for the quick reply, I am going to submit a new version.

Did you see the 20% rule somewhere?  Theer was anther patch that Greg
rejected, as he considered that one would need the hardware to find the
correct range.

julia

>
> >
> > >
> > > Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
> > > ---
> > >  drivers/staging/wilc1000/wilc_wlan.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
> > > index 8b184aa30d25..661c594b0a3c 100644
> > > --- a/drivers/staging/wilc1000/wilc_wlan.c
> > > +++ b/drivers/staging/wilc1000/wilc_wlan.c
> > > @@ -436,7 +436,7 @@ void chip_wakeup(struct wilc *wilc)
> > >  		} while (wilc_get_chipid(wilc, true) == 0);
> > >  	} else if ((wilc->io_type & 0x1) == HIF_SDIO) {
> > >  		wilc->hif_func->hif_write_reg(wilc, 0xfa, 1);
> > > -		udelay(200);
> > > +		usleep_range(200, 210);
> > >  		wilc->hif_func->hif_read_reg(wilc, 0xf0, &reg);
> > >  		do {
> > >  			wilc->hif_func->hif_write_reg(wilc, 0xf0,
> > > --
> > > 2.19.1
> > >
> > >
> > > --
> > > 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+unsubscribe@googlegroups.com.
> > > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/6eca799b032fdd5f674f55087c926b1957e22e5c.1539182155.git.code%40wizofe.uk.
> > > For more options, visit https://groups.google.com/d/optout.
> > >
>
>

Re: [Outreachy kernel] [PATCH 1/3] staging: wilc1000: replace udelay with usleep_range

[Ioannis Valasakis]

On Wed, Oct 10, 2018 at 05:02:25PM +0200, Julia Lawall wrote:
> 
> 
> On Wed, 10 Oct 2018, Ioannis Valasakis wrote:
> 
> > On Wed, Oct 10, 2018 at 04:54:38PM +0200, Julia Lawall wrote:
> > >
> > >
> > > On Wed, 10 Oct 2018, Ioannis Valasakis wrote:
> > >
> > > > Fix warning and replace a udelay inside a non-atomic context and
> > > > it can be safely replace by a usleep_range(t, t + delta), where delta is
> > > > 20% of t, as that is between 10 and 20 microseconds.
> > > > Reported by checkpatch.
> > >
> > > It looks like your delta is 5%?
> > >
> > > julia
> >
> > Indeed. Thanks for the quick reply, I am going to submit a new version.
> 
> Did you see the 20% rule somewhere?  Theer was anther patch that Greg
> rejected, as he considered that one would need the hardware to find the
> correct range.
> 
> julia

There's no rule but I was going through old approved commits 
and this is where I saw that 10-20 microseconds allowance.

I don't have the hardware indeed but I hope I can get some 
insight form the cc:ed developers from Microchip.
> 
> >
> > >
> > > >
> > > > Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
> > > > ---
> > > >  drivers/staging/wilc1000/wilc_wlan.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
> > > > index 8b184aa30d25..661c594b0a3c 100644
> > > > --- a/drivers/staging/wilc1000/wilc_wlan.c
> > > > +++ b/drivers/staging/wilc1000/wilc_wlan.c
> > > > @@ -436,7 +436,7 @@ void chip_wakeup(struct wilc *wilc)
> > > >  		} while (wilc_get_chipid(wilc, true) == 0);
> > > >  	} else if ((wilc->io_type & 0x1) == HIF_SDIO) {
> > > >  		wilc->hif_func->hif_write_reg(wilc, 0xfa, 1);
> > > > -		udelay(200);
> > > > +		usleep_range(200, 210);
> > > >  		wilc->hif_func->hif_read_reg(wilc, 0xf0, &reg);
> > > >  		do {
> > > >  			wilc->hif_func->hif_write_reg(wilc, 0xf0,
> > > > --
> > > > 2.19.1
> > > >
> > > >
> > > > --
> > > > 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+unsubscribe@googlegroups.com.
> > > > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > > > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/6eca799b032fdd5f674f55087c926b1957e22e5c.1539182155.git.code%40wizofe.uk.
> > > > For more options, visit https://groups.google.com/d/optout.
> > > >
> >
> >

Re: [Outreachy kernel] [PATCH 3/3] staging: wilc1000: Change struct members from bool to u8

[Himanshu Jha]

On Wed, Oct 10, 2018 at 03:48:21PM +0100, Ioannis Valasakis wrote:
> Recent versions of checkpatch have a new warning based on a documented
> preference of Linus to not use bool in structures due to wasted space and
> the size of bool is implementation dependent.  For more information, see
> the email thread at https://lkml.org/lkml/2017/11/21/384.
> 
> Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
> ---
>  drivers/staging/wilc1000/coreconfigurator.h   |  4 ++--
>  drivers/staging/wilc1000/host_interface.h     |  8 ++++----
>  drivers/staging/wilc1000/wilc_wfi_netdevice.h | 10 +++++-----
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/coreconfigurator.h b/drivers/staging/wilc1000/coreconfigurator.h
> index b62acb447383..dbea5144395e 100644
> --- a/drivers/staging/wilc1000/coreconfigurator.h
> +++ b/drivers/staging/wilc1000/coreconfigurator.h
> @@ -30,7 +30,7 @@
>  #define MAKE_WORD32(lsw, msw) ((((u32)(msw) << 16) & 0xFFFF0000) | (lsw))
>  
>  struct rssi_history_buffer {
> -	bool full;
> +	u8 full;

The disccussion was not about "u8(usigned char) Vs bool" object
representation but rather about "bitfields Vs bool".

It is true that the sizeof(__Bool) is implementation defined can be
1-4 depending upon the implementation.

Indeed sizeof(usigned char) is *guaranteed* to be 1!
https://port70.net/~nsz/c/c11/n1570.html#6.5.3.4p4

But would be worthwhile to change it to usigned/signed int full : 1
instead since we only care about true/false ? 


-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

Re: [PATCH 3/3] staging: wilc1000: Change struct members from bool to u8

[Greg KH]

On Wed, Oct 10, 2018 at 03:48:21PM +0100, Ioannis Valasakis wrote:
> Recent versions of checkpatch have a new warning based on a documented
> preference of Linus to not use bool in structures due to wasted space and
> the size of bool is implementation dependent.  For more information, see
> the email thread at https://lkml.org/lkml/2017/11/21/384.
> 
> Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
> ---
>  drivers/staging/wilc1000/coreconfigurator.h   |  4 ++--
>  drivers/staging/wilc1000/host_interface.h     |  8 ++++----
>  drivers/staging/wilc1000/wilc_wfi_netdevice.h | 10 +++++-----
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/coreconfigurator.h b/drivers/staging/wilc1000/coreconfigurator.h
> index b62acb447383..dbea5144395e 100644
> --- a/drivers/staging/wilc1000/coreconfigurator.h
> +++ b/drivers/staging/wilc1000/coreconfigurator.h
> @@ -30,7 +30,7 @@
>  #define MAKE_WORD32(lsw, msw) ((((u32)(msw) << 16) & 0xFFFF0000) | (lsw))
>  
>  struct rssi_history_buffer {
> -	bool full;
> +	u8 full;

No, this reduces the information about the field.  There's no wasted
space here at all, the end result is the same.  The issue is, as
Himanshu points out, when using bool instead of a bitfield.

So this patch isn't needed, sorry.

greg k-h

Re: [PATCH 1/3] staging: wilc1000: replace udelay with usleep_range

[Greg KH]

On Wed, Oct 10, 2018 at 03:47:20PM +0100, Ioannis Valasakis wrote:
> Fix warning and replace a udelay inside a non-atomic context and
> it can be safely replace by a usleep_range(t, t + delta), where delta is
> 20% of t, as that is between 10 and 20 microseconds.
> Reported by checkpatch.
> 
> Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
> ---
>  drivers/staging/wilc1000/wilc_wlan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
> index 8b184aa30d25..661c594b0a3c 100644
> --- a/drivers/staging/wilc1000/wilc_wlan.c
> +++ b/drivers/staging/wilc1000/wilc_wlan.c
> @@ -436,7 +436,7 @@ void chip_wakeup(struct wilc *wilc)
>  		} while (wilc_get_chipid(wilc, true) == 0);
>  	} else if ((wilc->io_type & 0x1) == HIF_SDIO) {
>  		wilc->hif_func->hif_write_reg(wilc, 0xfa, 1);
> -		udelay(200);
> +		usleep_range(200, 210);

Unless you have the hardware for this device, and have tested it, I
can't take this patch, sorry.  The 20% number is totally randomly
picked, which isn't ok.

The _range stuff is primarily for when scheduling timers that are called
a lot, which is not the case here.

thanks,

greg k-h

Re: [PATCH 3/3] staging: wilc1000: Change struct members from bool to u8

[Ioannis Valasakis]

On Wed, Oct 10, 2018 at 08:32:10PM +0200, Greg KH wrote:
> On Wed, Oct 10, 2018 at 03:48:21PM +0100, Ioannis Valasakis wrote:
> > Recent versions of checkpatch have a new warning based on a documented
> > preference of Linus to not use bool in structures due to wasted space and
> > the size of bool is implementation dependent.  For more information, see
> > the email thread at https://lkml.org/lkml/2017/11/21/384.
> > 
> > Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
> > ---
> >  drivers/staging/wilc1000/coreconfigurator.h   |  4 ++--
> >  drivers/staging/wilc1000/host_interface.h     |  8 ++++----
> >  drivers/staging/wilc1000/wilc_wfi_netdevice.h | 10 +++++-----
> >  3 files changed, 11 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/staging/wilc1000/coreconfigurator.h b/drivers/staging/wilc1000/coreconfigurator.h
> > index b62acb447383..dbea5144395e 100644
> > --- a/drivers/staging/wilc1000/coreconfigurator.h
> > +++ b/drivers/staging/wilc1000/coreconfigurator.h
> > @@ -30,7 +30,7 @@
> >  #define MAKE_WORD32(lsw, msw) ((((u32)(msw) << 16) & 0xFFFF0000) | (lsw))
> >  
> >  struct rssi_history_buffer {
> > -	bool full;
> > +	u8 full;
> 
> No, this reduces the information about the field.  There's no wasted
> space here at all, the end result is the same.  The issue is, as
> Himanshu points out, when using bool instead of a bitfield.
> 
> So this patch isn't needed, sorry.
> 
> greg k-h

Noted. Learning something every day. Thanks greg.

ioannis