Re: [PATCH 24/29] drivers: convert iblock_req.pending from atomic_t to refcount_t
Hi Elena, On Mon, 2017-03-06 at 16:21 +0200, Elena Reshetova wrote: > refcount_t type and corresponding API should be > used instead of atomic_t when the variable is used as > a reference counter. This allows to avoid accidental > refcounter overflows that might lead to use-after-free > situations. > > Signed-off-by: Elena Reshetova > Signed-off-by: Hans Liljestrand > Signed-off-by: Kees Cook > Signed-off-by: David Windsor > --- > drivers/target/target_core_iblock.c | 12 ++-- > drivers/target/target_core_iblock.h | 3 ++- > 2 files changed, 8 insertions(+), 7 deletions(-) For the target_core_iblock part: Acked-by: Nicholas Bellinger ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] storvsc: workaround for virtual DVD SCSI version
On 03/07/2017 06:15 PM, Stephen Hemminger wrote: > Hyper-V host emulation of SCSI for virtual DVD device reports SCSI > version 0 (UNKNOWN) but is still capable of supporting REPORTLUN. > > Without this patch, a GEN2 Linux guest on Hyper-V will not boot 4.11 > successfully with virtual DVD ROM device. What happens is that the > SCSI scan process falls back to doing sequential probing by INQUIRY. > But the storvsc driver has a previous workaround that masks/blocks all > errors reports from INQUIRY (or MODE_SENSE) commands. This workaround > causes the scan to then populate a full set of bogus LUN's on the > target and then sends kernel spinning off into a death spiral doing > block reads on the non-existent LUNs. > > By setting the correct blacklist flags, the target with the > DVD device is scanned with REPORTLUN and that works correctly. > > Patch needs to go in current 4.11, it is safe but not necessary > in older kernels. > > Signed-off-by: Stephen Hemminger > --- > drivers/scsi/storvsc_drv.c | 27 +-- > 1 file changed, 17 insertions(+), 10 deletions(-) > > PS: The error handling does need to be fixed (have patches pending) > but that is interrelated with hotplug and can wait. > Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 10/12] staging: ks7010: add variables key, key_index
On Wed, Mar 08, 2017 at 08:54:36AM +0300, Dan Carpenter wrote:
> On Wed, Mar 08, 2017 at 02:36:55PM +1100, Tobin C. Harding wrote:
> > diff --git a/drivers/staging/ks7010/ks_hostif.c
> > b/drivers/staging/ks7010/ks_hostif.c
> > index 7dc0d99..7ff5345 100644
> > --- a/drivers/staging/ks7010/ks_hostif.c
> > +++ b/drivers/staging/ks7010/ks_hostif.c
> > @@ -318,6 +318,8 @@ int hostif_data_indication_wpa(struct ks_wlan_private
> > *priv, unsigned int auth_t
> > struct mic_failure_t *mic_failure;
> > struct mihcael_mic_t mihcael_mic;
> > union iwreq_data wrqu;
> > + unsigned int key_index = auth_type - 1;
> > + struct wpa_key_t *key = priv->wpa.key[key_index];
> >
> > eth_hdr = (struct ether_hdr *)(priv->rxp);
> > eth_proto = ntohs(eth_hdr->h_proto);
> > @@ -340,7 +342,7 @@ int hostif_data_indication_wpa(struct ks_wlan_private
> > *priv, unsigned int auth_t
> > || (auth_type == TYPE_GMK2
> > && priv->wpa.group_suite ==
> > IW_AUTH_CIPHER_TKIP))
> > - && priv->wpa.key[auth_type - 1].key_len) {
> > + && key.key_len) {
>
> This won't compile. It should be key->key_len.
>
> It's very annoying for me to review compile errors because first of all
> I should never have to. And secondly I have to verify that I'm not just
> misreading the code so I have to leave my email client and it's time
> consuming.
Sloppy work by me, sincere apologies. I'll wait overnight as
previously suggested before resubmitting.
Again, sorry to waste your time.
thanks,
Tobin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: wilc1000: Fix sparse warnings incorrect type assignment
I think this change is buggy. On Tue, Mar 07, 2017 at 10:36:53PM +0100, Andrea Ghittino wrote: > Fixed sparse warnings related to the conversion of le16 and le32 to u16 and > u32, during the update of internal structures > > Fixed sparse warnings: > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2011:52: warning: > incorrect type in assignment (different base types) > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2011:52:expected > unsigned short [unsigned] [assigned] [usertype] ht_ext_params > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2011:52:got restricted > __le16 const [usertype] extended_ht_cap_info > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2012:51: warning: > incorrect type in assignment (different base types) > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2012:51:expected > unsigned int [unsigned] [assigned] [usertype] ht_tx_bf_cap > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2012:51:got restricted > __le32 const [usertype] tx_BF_cap_info > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2078:51: warning: > incorrect type in assignment (different base types) > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2078:51:expected > unsigned short [unsigned] [assigned] [usertype] ht_capa_info > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2078:51:got restricted > __le16 const [usertype] cap_info > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2083:52: warning: > incorrect type in assignment (different base types) > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2083:52:expected > unsigned short [unsigned] [assigned] [usertype] ht_ext_params > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2083:52:got restricted > __le16 const [usertype] extended_ht_cap_info > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2084:51: warning: > incorrect type in assignment (different base types) > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2084:51:expected > unsigned int [unsigned] [assigned] [usertype] ht_tx_bf_cap > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2084:51:got restricted > __le32 const [usertype] tx_BF_cap_info > What I want in a changelog is: "We're copying data that's little endian from (some place) to where ever and then we (whatever) do math using the variable so it needs to be CPU endian. Presumably this wasn't caught in testing because it was only used on x86 or other little endian systems." In this case we're copying little endian data and then sending it directly back to some place which requires little endian data so converting it is a bug. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] Staging: comedi: comedi_fops: Fix "out of minor numbers for board device files"
On Sun, Mar 05, 2017 at 03:22:33AM +0800, Cheah Kok Cheong wrote: > If comedi module is loaded with the following max allowed parameter > [comedi_num_legacy_minors=48], subsequent loading of an auto-configured > device will fail. Don't set comedi_num_legacy_minors=48, then? This doesn't seem like the right fix at all. Why only allow one auto configured board? Why not 5 or 10? regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 08/12] staging: ks7010: fix spelling of Michael MIC
On Wed, Mar 08, 2017 at 02:36:53PM +1100, Tobin C. Harding wrote: > Various symbols are named 'michel*'. The keyed hash function name is > spelled 'Michael'. > > Rename symbols michel -> michael. > > Signed-off-by: Tobin C. Harding > --- > drivers/staging/ks7010/ks_hostif.c | 16 > drivers/staging/ks7010/michael_mic.c | 8 > drivers/staging/ks7010/michael_mic.h | 4 ++-- > 3 files changed, 14 insertions(+), 14 deletions(-) > > diff --git a/drivers/staging/ks7010/ks_hostif.c > b/drivers/staging/ks7010/ks_hostif.c > index edeefea..4256b10 100644 > --- a/drivers/staging/ks7010/ks_hostif.c > +++ b/drivers/staging/ks7010/ks_hostif.c > @@ -316,7 +316,7 @@ int hostif_data_indication_wpa(struct ks_wlan_private > *priv, unsigned int auth_t > char buf[128]; > unsigned long now; > struct mic_failure_t *mic_failure; > - struct michel_mic_t michel_mic; > + struct mihcael_mic_t mihcael_mic; Nope. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 10/12] staging: ks7010: add variables key, key_index
On Wed, Mar 08, 2017 at 02:36:55PM +1100, Tobin C. Harding wrote:
> diff --git a/drivers/staging/ks7010/ks_hostif.c
> b/drivers/staging/ks7010/ks_hostif.c
> index 7dc0d99..7ff5345 100644
> --- a/drivers/staging/ks7010/ks_hostif.c
> +++ b/drivers/staging/ks7010/ks_hostif.c
> @@ -318,6 +318,8 @@ int hostif_data_indication_wpa(struct ks_wlan_private
> *priv, unsigned int auth_t
> struct mic_failure_t *mic_failure;
> struct mihcael_mic_t mihcael_mic;
> union iwreq_data wrqu;
> + unsigned int key_index = auth_type - 1;
> + struct wpa_key_t *key = priv->wpa.key[key_index];
>
> eth_hdr = (struct ether_hdr *)(priv->rxp);
> eth_proto = ntohs(eth_hdr->h_proto);
> @@ -340,7 +342,7 @@ int hostif_data_indication_wpa(struct ks_wlan_private
> *priv, unsigned int auth_t
> || (auth_type == TYPE_GMK2
> && priv->wpa.group_suite ==
> IW_AUTH_CIPHER_TKIP))
> - && priv->wpa.key[auth_type - 1].key_len) {
> + && key.key_len) {
This won't compile. It should be key->key_len.
It's very annoying for me to review compile errors because first of all
I should never have to. And secondly I have to verify that I'm not just
misreading the code so I have to leave my email client and it's time
consuming.
Please be a lot more careful.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: cb_pcidas64: move loop invariant
On Wed, Mar 08, 2017 at 12:26:27AM +0300, Dan Carpenter wrote: > On Wed, Mar 08, 2017 at 08:18:50AM +1100, Tobin C. Harding wrote: > > On Tue, Mar 07, 2017 at 08:03:16PM +0100, Greg Kroah-Hartman wrote: > > > On Tue, Mar 07, 2017 at 01:29:35PM +1100, Tobin C. Harding wrote: > > > > Loop invariant is inside the loop so code checks invariant on each > > > > iteration of the loop. Invariant can be moved outside of the loop so > > > > it is only checked once. > > > > > > > > Move loop invariant outside of for loop. > > > > > > But does it really matter? Does this fix an issue? Make something > > > faster in a _measurable_ way? Did you test this on the hardware? > > > > Good points, patch is untested and does not fix a particular > > issue. Lesson learned. > > You've learned the wrong lesson. The right lesson is to sell your patch > better. If you had said, "This improves readability and makes things > more uniform" then it would probably have been accepted. But you > sold it as a speed up and it obviously isn't. Thanks Dan, I appreciate you taking the time to explain. thanks, Tobin ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 12/12] staging: ks7010: fix checkpatch line over 80
Checkpatch emits WARNING: line over 80 characters. Modify the comment without loss of meaning to reduce the length of the line. Clear two warnings with single change. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks_hostif.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 940ab64..47a4034 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -370,7 +370,7 @@ int hostif_data_indication_wpa(struct ks_wlan_private *priv, mic_failure->failure = 2; mic_failure->counter = (u16)((now - mic_failure->last_failure_time) / HZ); - if (!mic_failure->counter) /* mic_failure counter value range 1-60 */ + if (!mic_failure->counter) /* range 1-60 */ mic_failure->counter = 1; } priv->wpa.mic_failure.last_failure_time = now; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 11/12] staging: ks7010: refactor, improve code layout
Function hostif_data_indication_wpa() has various sites that can be
improved with whitespace refactoring. In particular checkpatch emits
WARNING: Avoid multiple line dereference. Also how long lines are
broken up may be modified to enhance readability. Checkpatch also
emits logical line continuation warnings. These can also be fixed
changing only whitespace.
Refactor code making whitespace only changes. Do not change the
program logic.
Signed-off-by: Tobin C. Harding
---
drivers/staging/ks7010/ks_hostif.c | 63 --
1 file changed, 27 insertions(+), 36 deletions(-)
diff --git a/drivers/staging/ks7010/ks_hostif.c
b/drivers/staging/ks7010/ks_hostif.c
index 7ff5345..940ab64 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -308,7 +308,8 @@ int get_ap_information(struct ks_wlan_private *priv, struct
ap_info_t *ap_info,
}
static
-int hostif_data_indication_wpa(struct ks_wlan_private *priv, unsigned int
auth_type)
+int hostif_data_indication_wpa(struct ks_wlan_private *priv,
+ unsigned int auth_type)
{
struct ether_hdr *eth_hdr;
unsigned short eth_proto;
@@ -319,7 +320,7 @@ int hostif_data_indication_wpa(struct ks_wlan_private
*priv, unsigned int auth_t
struct mihcael_mic_t mihcael_mic;
union iwreq_data wrqu;
unsigned int key_index = auth_type - 1;
- struct wpa_key_t *key = priv->wpa.key[key_index];
+ struct wpa_key_t *key = &priv->wpa.key[key_index];
eth_hdr = (struct ether_hdr *)(priv->rxp);
eth_proto = ntohs(eth_hdr->h_proto);
@@ -333,34 +334,32 @@ int hostif_data_indication_wpa(struct ks_wlan_private
*priv, unsigned int auth_t
priv->nstats.rx_errors++;
return -EINVAL;
}
- if (((auth_type == TYPE_PMK1
- && priv->wpa.pairwise_suite ==
- IW_AUTH_CIPHER_TKIP) || (auth_type == TYPE_GMK1
- && priv->wpa.
- group_suite ==
- IW_AUTH_CIPHER_TKIP)
- || (auth_type == TYPE_GMK2
- && priv->wpa.group_suite ==
- IW_AUTH_CIPHER_TKIP))
- && key.key_len) {
+ if (((auth_type == TYPE_PMK1 &&
+ priv->wpa.pairwise_suite == IW_AUTH_CIPHER_TKIP) ||
+(auth_type == TYPE_GMK1 &&
+ priv->wpa.group_suite == IW_AUTH_CIPHER_TKIP) ||
+(auth_type == TYPE_GMK2 &&
+ priv->wpa.group_suite == IW_AUTH_CIPHER_TKIP)) &&
+ key->key_len) {
DPRINTK(4, "TKIP: protocol=%04X: size=%u\n",
eth_proto, priv->rx_size);
/* MIC save */
memcpy(&recv_mic_buf[0],
- (priv->rxp) + ((priv->rx_size) - 8), 8);
+ (priv->rxp) + ((priv->rx_size) - 8), 8);
priv->rx_size = priv->rx_size - 8;
if (auth_type > 0 && auth_type < 4) { /* auth_type check */
- MichaelMICFunction(&mihcael_mic, (u8 *)key.rx_mic_key,
(u8 *)priv->rxp, (int)priv->rx_size, (u8)0, /* priority */
- (u8 *)mihcael_mic.Result);
+ MichaelMICFunction(&mihcael_mic,
+ (u8 *)key->rx_mic_key,
+ (u8 *)priv->rxp, (int)priv->rx_size,
+ (u8)0, /* priority */
+ (u8 *)mihcael_mic.Result);
}
if (memcmp(mihcael_mic.Result, recv_mic_buf, 8)) {
now = jiffies;
mic_failure = &priv->wpa.mic_failure;
/* MIC FAILURE */
if (mic_failure->last_failure_time &&
- (now -
- mic_failure->last_failure_time) /
- HZ >= 60) {
+ (now - mic_failure->last_failure_time) / HZ >= 60) {
mic_failure->failure = 0;
}
DPRINTK(4, "MIC FAILURE\n");
@@ -370,31 +369,23 @@ int hostif_data_indication_wpa(struct ks_wlan_private
*priv, unsigned int auth_t
} else if (mic_failure->failure == 1) {
mic_failure->failure = 2;
mic_failure->counter =
- (u16)((now -
- mic_failure->
- last_failure_time)
- / HZ);
+
[PATCH 10/12] staging: ks7010: add variables key, key_index
'auth_type - 1' is used as an index into a key table. Adding a variable
appropriately named simplifies the code and adds meaning when
reading. Adding a pointer variable of type struct *kpa_key_t adds to
readability by removing the table access each time the key is used.
The key index is used to create a string so having it named adds
additional meaning when creating the string.
Declare variable 'key_index' and define it at declaration time. Declare
a pointer variable 'key' and define it to point to the correct key in
the key table.
Signed-off-by: Tobin C. Harding
---
drivers/staging/ks7010/ks_hostif.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/ks7010/ks_hostif.c
b/drivers/staging/ks7010/ks_hostif.c
index 7dc0d99..7ff5345 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -318,6 +318,8 @@ int hostif_data_indication_wpa(struct ks_wlan_private
*priv, unsigned int auth_t
struct mic_failure_t *mic_failure;
struct mihcael_mic_t mihcael_mic;
union iwreq_data wrqu;
+ unsigned int key_index = auth_type - 1;
+ struct wpa_key_t *key = priv->wpa.key[key_index];
eth_hdr = (struct ether_hdr *)(priv->rxp);
eth_proto = ntohs(eth_hdr->h_proto);
@@ -340,7 +342,7 @@ int hostif_data_indication_wpa(struct ks_wlan_private
*priv, unsigned int auth_t
|| (auth_type == TYPE_GMK2
&& priv->wpa.group_suite ==
IW_AUTH_CIPHER_TKIP))
- && priv->wpa.key[auth_type - 1].key_len) {
+ && key.key_len) {
DPRINTK(4, "TKIP: protocol=%04X: size=%u\n",
eth_proto, priv->rx_size);
/* MIC save */
@@ -348,7 +350,7 @@ int hostif_data_indication_wpa(struct ks_wlan_private
*priv, unsigned int auth_t
(priv->rxp) + ((priv->rx_size) - 8), 8);
priv->rx_size = priv->rx_size - 8;
if (auth_type > 0 && auth_type < 4) { /* auth_type check */
- MichaelMICFunction(&mihcael_mic, (u8
*)priv->wpa.key[auth_type - 1].rx_mic_key, (u8 *)priv->rxp, (int)priv->rx_size,
(u8)0, /* priority */
+ MichaelMICFunction(&mihcael_mic, (u8 *)key.rx_mic_key,
(u8 *)priv->rxp, (int)priv->rx_size, (u8)0, /* priority */
(u8 *)mihcael_mic.Result);
}
if (memcmp(mihcael_mic.Result, recv_mic_buf, 8)) {
@@ -382,7 +384,7 @@ int hostif_data_indication_wpa(struct ks_wlan_private
*priv, unsigned int auth_t
sprintf(wrqu_buf,
"MLME-MICHAELMICFAILURE.indication(keyid=%d
%scast addr="
"%pM)",
- auth_type - 1,
+ key_index,
eth_hdr->
h_dest[0] & 0x01 ? "broad" :
"uni", eth_hdr->h_source);
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 09/12] staging: ks7010: add meaningful buffer names
Function contains a buffer named 'buf' and another named
RecvMIC. Checkpatch emits WARNING: Avoid CamelCase on the symbol name
RecvMIC. Since there are two buffers and we need to rename one of them
let's give both of them new names in order to add meaning to code and
simplify reading.
Change buffer name RecvMIC -> recv_mic_buf.
Change buffer name buf -> wrqu_buf.
Signed-off-by: Tobin C. Harding
---
drivers/staging/ks7010/ks_hostif.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/ks7010/ks_hostif.c
b/drivers/staging/ks7010/ks_hostif.c
index 4256b10..7dc0d99 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -312,8 +312,8 @@ int hostif_data_indication_wpa(struct ks_wlan_private
*priv, unsigned int auth_t
{
struct ether_hdr *eth_hdr;
unsigned short eth_proto;
- unsigned char RecvMIC[8];
- char buf[128];
+ unsigned char recv_mic_buf[8];
+ char wrqu_buf[128];
unsigned long now;
struct mic_failure_t *mic_failure;
struct mihcael_mic_t mihcael_mic;
@@ -344,14 +344,14 @@ int hostif_data_indication_wpa(struct ks_wlan_private
*priv, unsigned int auth_t
DPRINTK(4, "TKIP: protocol=%04X: size=%u\n",
eth_proto, priv->rx_size);
/* MIC save */
- memcpy(&RecvMIC[0],
+ memcpy(&recv_mic_buf[0],
(priv->rxp) + ((priv->rx_size) - 8), 8);
priv->rx_size = priv->rx_size - 8;
if (auth_type > 0 && auth_type < 4) { /* auth_type check */
MichaelMICFunction(&mihcael_mic, (u8
*)priv->wpa.key[auth_type - 1].rx_mic_key, (u8 *)priv->rxp, (int)priv->rx_size,
(u8)0, /* priority */
(u8 *)mihcael_mic.Result);
}
- if (memcmp(mihcael_mic.Result, RecvMIC, 8)) {
+ if (memcmp(mihcael_mic.Result, recv_mic_buf, 8)) {
now = jiffies;
mic_failure = &priv->wpa.mic_failure;
/* MIC FAILURE */
@@ -379,7 +379,7 @@ int hostif_data_indication_wpa(struct ks_wlan_private
*priv, unsigned int auth_t
priv->wpa.mic_failure.
last_failure_time = now;
/* needed parameters: count, keyid, key type, TSC */
- sprintf(buf,
+ sprintf(wrqu_buf,
"MLME-MICHAELMICFAILURE.indication(keyid=%d
%scast addr="
"%pM)",
auth_type - 1,
@@ -387,12 +387,12 @@ int hostif_data_indication_wpa(struct ks_wlan_private
*priv, unsigned int auth_t
h_dest[0] & 0x01 ? "broad" :
"uni", eth_hdr->h_source);
memset(&wrqu, 0, sizeof(wrqu));
- wrqu.data.length = strlen(buf);
+ wrqu.data.length = strlen(wrqu_buf);
DPRINTK(4,
"IWEVENT:MICHAELMICFAILURE\n");
wireless_send_event(priv->net_dev,
IWEVCUSTOM, &wrqu,
- buf);
+ wrqu_buf);
return -EINVAL;
}
}
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 08/12] staging: ks7010: fix spelling of Michael MIC
Various symbols are named 'michel*'. The keyed hash function name is
spelled 'Michael'.
Rename symbols michel -> michael.
Signed-off-by: Tobin C. Harding
---
drivers/staging/ks7010/ks_hostif.c | 16
drivers/staging/ks7010/michael_mic.c | 8
drivers/staging/ks7010/michael_mic.h | 4 ++--
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/ks7010/ks_hostif.c
b/drivers/staging/ks7010/ks_hostif.c
index edeefea..4256b10 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -316,7 +316,7 @@ int hostif_data_indication_wpa(struct ks_wlan_private
*priv, unsigned int auth_t
char buf[128];
unsigned long now;
struct mic_failure_t *mic_failure;
- struct michel_mic_t michel_mic;
+ struct mihcael_mic_t mihcael_mic;
union iwreq_data wrqu;
eth_hdr = (struct ether_hdr *)(priv->rxp);
@@ -348,10 +348,10 @@ int hostif_data_indication_wpa(struct ks_wlan_private
*priv, unsigned int auth_t
(priv->rxp) + ((priv->rx_size) - 8), 8);
priv->rx_size = priv->rx_size - 8;
if (auth_type > 0 && auth_type < 4) { /* auth_type check */
- MichaelMICFunction(&michel_mic, (u8
*)priv->wpa.key[auth_type - 1].rx_mic_key, (u8 *)priv->rxp, (int)priv->rx_size,
(u8)0, /* priority */
- (u8 *)michel_mic.Result);
+ MichaelMICFunction(&mihcael_mic, (u8
*)priv->wpa.key[auth_type - 1].rx_mic_key, (u8 *)priv->rxp, (int)priv->rx_size,
(u8)0, /* priority */
+ (u8 *)mihcael_mic.Result);
}
- if (memcmp(michel_mic.Result, RecvMIC, 8)) {
+ if (memcmp(mihcael_mic.Result, RecvMIC, 8)) {
now = jiffies;
mic_failure = &priv->wpa.mic_failure;
/* MIC FAILURE */
@@ -1128,7 +1128,7 @@ int hostif_data_request(struct ks_wlan_private *priv,
struct sk_buff *packet)
int result = 0;
unsigned short eth_proto;
struct ether_hdr *eth_hdr;
- struct michel_mic_t michel_mic;
+ struct mihcael_mic_t mihcael_mic;
unsigned short keyinfo = 0;
struct ieee802_1x_hdr *aa1x_hdr;
struct wpa_eapol_key *eap_key;
@@ -1236,10 +1236,10 @@ int hostif_data_request(struct ks_wlan_private *priv,
struct sk_buff *packet)
pp->auth_type = cpu_to_le16((u16)TYPE_AUTH);/* no
encryption */
} else {
if (priv->wpa.pairwise_suite == IW_AUTH_CIPHER_TKIP) {
- MichaelMICFunction(&michel_mic, (u8
*)priv->wpa.key[0].tx_mic_key, (u8 *)&pp->data[0], (int)packet_len, (u8)0, /*
priority */
- (u8 *)michel_mic.
+ MichaelMICFunction(&mihcael_mic, (u8
*)priv->wpa.key[0].tx_mic_key, (u8 *)&pp->data[0], (int)packet_len, (u8)0, /*
priority */
+ (u8 *)mihcael_mic.
Result);
- memcpy(p, michel_mic.Result, 8);
+ memcpy(p, mihcael_mic.Result, 8);
length += 8;
packet_len += 8;
p += 8;
diff --git a/drivers/staging/ks7010/michael_mic.c
b/drivers/staging/ks7010/michael_mic.c
index f6e70fa..5b98c0a 100644
--- a/drivers/staging/ks7010/michael_mic.c
+++ b/drivers/staging/ks7010/michael_mic.c
@@ -38,7 +38,7 @@ do { \
} while (0)
static
-void MichaelInitializeFunction(struct michel_mic_t *Mic, uint8_t *key)
+void MichaelInitializeFunction(struct mihcael_mic_t *Mic, uint8_t *key)
{
// Set the key
Mic->K0 = getUInt32(key, 0);
@@ -61,7 +61,7 @@ do {
\
} while (0)
static
-void MichaelAppend(struct michel_mic_t *Mic, uint8_t *src, int nBytes)
+void MichaelAppend(struct mihcael_mic_t *Mic, uint8_t *src, int nBytes)
{
int addlen;
@@ -96,7 +96,7 @@ void MichaelAppend(struct michel_mic_t *Mic, uint8_t *src,
int nBytes)
}
static
-void MichaelGetMIC(struct michel_mic_t *Mic, uint8_t *dst)
+void MichaelGetMIC(struct mihcael_mic_t *Mic, uint8_t *dst)
{
u8 *data = Mic->M;
@@ -125,7 +125,7 @@ void MichaelGetMIC(struct michel_mic_t *Mic, uint8_t *dst)
MichaelClear(Mic);
}
-void MichaelMICFunction(struct michel_mic_t *Mic, u8 *Key,
+void MichaelMICFunction(struct mihcael_mic_t *Mic, u8 *Key,
u8 *Data, int Len, u8 priority,
u8 *Result)
{
diff --git a/drivers/staging/ks7010/michael_mic.h
b/drivers/staging/ks7010/michael_mic.h
index 248f849..51ea2eb 100644
--- a/drivers/staging/ks7010/michael_mic.h
+++ b/drivers/stag
[PATCH 06/12] staging: ks7010: factor out WPA code
Function hostif_data_indication() is approx 200 lines long. Function
has many local variables. WPA code is contained and may be factored
out into a separate function. This will reduce the length and
complexity of hostif_data_indication(). At times within the WPA code
errors result in the function returning. In order to maintain this
behaviour new function should return a status integer.
Factor out WPA code into separate function. Add only code needed to
get compilation to pass, including modifying return statements. Make
no other code changes, program logic is unchanged.
Signed-off-by: Tobin C. Harding
---
drivers/staging/ks7010/ks_hostif.c | 175 -
1 file changed, 95 insertions(+), 80 deletions(-)
diff --git a/drivers/staging/ks7010/ks_hostif.c
b/drivers/staging/ks7010/ks_hostif.c
index b75ef1d6..1da7b5a 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -308,23 +308,107 @@ int get_ap_information(struct ks_wlan_private *priv,
struct ap_info_t *ap_info,
}
static
+int hostif_data_indication_wpa(struct ks_wlan_private *priv, unsigned int
auth_type)
+{
+ struct ether_hdr *eth_hdr;
+ unsigned short eth_proto;
+ unsigned char RecvMIC[8];
+ char buf[128];
+ unsigned long now;
+ struct mic_failure_t *mic_failure;
+ struct michel_mic_t michel_mic;
+ union iwreq_data wrqu;
+
+ eth_hdr = (struct ether_hdr *)(priv->rxp);
+ eth_proto = ntohs(eth_hdr->h_proto);
+
+ if (memcmp(ð_hdr->h_source[0], &priv->eth_addr[0], ETH_ALEN)) {
/* source address check */
+ if (eth_hdr->h_dest_snap != eth_hdr->h_source_snap) {
+ DPRINTK(1, "invalid data format\n");
+ priv->nstats.rx_errors++;
+ return -EINVAL;
+ }
+ if (((auth_type == TYPE_PMK1
+ && priv->wpa.pairwise_suite ==
+ IW_AUTH_CIPHER_TKIP) || (auth_type ==
TYPE_GMK1
+ && priv->wpa.
+ group_suite ==
+
IW_AUTH_CIPHER_TKIP)
+ || (auth_type == TYPE_GMK2
+ && priv->wpa.group_suite ==
+ IW_AUTH_CIPHER_TKIP))
+ && priv->wpa.key[auth_type - 1].key_len) {
+ DPRINTK(4, "TKIP: protocol=%04X: size=%u\n",
+ eth_proto, priv->rx_size);
+ /* MIC save */
+ memcpy(&RecvMIC[0],
+ (priv->rxp) + ((priv->rx_size) - 8), 8);
+ priv->rx_size = priv->rx_size - 8;
+ if (auth_type > 0 && auth_type < 4) { /* auth_type
check */
+ MichaelMICFunction(&michel_mic, (u8
*)priv->wpa.key[auth_type - 1].rx_mic_key, (u8 *)priv->rxp, (int)priv->rx_size,
(u8)0, /* priority */
+ (u8 *)michel_mic.Result);
+ }
+ if (memcmp(michel_mic.Result, RecvMIC, 8)) {
+ now = jiffies;
+ mic_failure = &priv->wpa.mic_failure;
+ /* MIC FAILURE */
+ if (mic_failure->last_failure_time &&
+ (now -
+ mic_failure->last_failure_time)
/
+ HZ >= 60) {
+ mic_failure->failure = 0;
+ }
+ DPRINTK(4, "MIC FAILURE\n");
+ if (mic_failure->failure == 0) {
+ mic_failure->failure = 1;
+ mic_failure->counter = 0;
+ } else if (mic_failure->failure == 1) {
+ mic_failure->failure = 2;
+ mic_failure->counter =
+ (u16)((now -
+ mic_failure->
+
last_failure_time)
+ / HZ);
+ if (!mic_failure->counter) /*
mic_failure counter value range 1-60 */
+ mic_failure->counter =
+ 1;
+ }
+ priv->wpa.mic_failure.
+ last_failure_t
[PATCH 07/12] staging: ks7010: reduce level of indentation
Indentation may be reduced by inverting if statement conditional and
returning.
Invert conditional. Return if new conditional evaluates to true. Do
not alter program logic. Reduce by one the level of indentation in
subsequent code.
Signed-off-by: Tobin C. Harding
---
drivers/staging/ks7010/ks_hostif.c | 134 +++--
1 file changed, 68 insertions(+), 66 deletions(-)
diff --git a/drivers/staging/ks7010/ks_hostif.c
b/drivers/staging/ks7010/ks_hostif.c
index 1da7b5a..edeefea 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -322,76 +322,78 @@ int hostif_data_indication_wpa(struct ks_wlan_private
*priv, unsigned int auth_t
eth_hdr = (struct ether_hdr *)(priv->rxp);
eth_proto = ntohs(eth_hdr->h_proto);
- if (memcmp(ð_hdr->h_source[0], &priv->eth_addr[0], ETH_ALEN)) {
/* source address check */
- if (eth_hdr->h_dest_snap != eth_hdr->h_source_snap) {
- DPRINTK(1, "invalid data format\n");
- priv->nstats.rx_errors++;
- return -EINVAL;
+ /* source address check */
+ if (!memcmp(ð_hdr->h_source[0], &priv->eth_addr[0], ETH_ALEN))
+ return 0;
+
+ if (eth_hdr->h_dest_snap != eth_hdr->h_source_snap) {
+ DPRINTK(1, "invalid data format\n");
+ priv->nstats.rx_errors++;
+ return -EINVAL;
+ }
+ if (((auth_type == TYPE_PMK1
+ && priv->wpa.pairwise_suite ==
+ IW_AUTH_CIPHER_TKIP) || (auth_type == TYPE_GMK1
+ && priv->wpa.
+ group_suite ==
+ IW_AUTH_CIPHER_TKIP)
+ || (auth_type == TYPE_GMK2
+ && priv->wpa.group_suite ==
+ IW_AUTH_CIPHER_TKIP))
+ && priv->wpa.key[auth_type - 1].key_len) {
+ DPRINTK(4, "TKIP: protocol=%04X: size=%u\n",
+ eth_proto, priv->rx_size);
+ /* MIC save */
+ memcpy(&RecvMIC[0],
+ (priv->rxp) + ((priv->rx_size) - 8), 8);
+ priv->rx_size = priv->rx_size - 8;
+ if (auth_type > 0 && auth_type < 4) { /* auth_type check */
+ MichaelMICFunction(&michel_mic, (u8
*)priv->wpa.key[auth_type - 1].rx_mic_key, (u8 *)priv->rxp, (int)priv->rx_size,
(u8)0, /* priority */
+ (u8 *)michel_mic.Result);
}
- if (((auth_type == TYPE_PMK1
- && priv->wpa.pairwise_suite ==
- IW_AUTH_CIPHER_TKIP) || (auth_type ==
TYPE_GMK1
- && priv->wpa.
- group_suite ==
-
IW_AUTH_CIPHER_TKIP)
- || (auth_type == TYPE_GMK2
- && priv->wpa.group_suite ==
- IW_AUTH_CIPHER_TKIP))
- && priv->wpa.key[auth_type - 1].key_len) {
- DPRINTK(4, "TKIP: protocol=%04X: size=%u\n",
- eth_proto, priv->rx_size);
- /* MIC save */
- memcpy(&RecvMIC[0],
- (priv->rxp) + ((priv->rx_size) - 8), 8);
- priv->rx_size = priv->rx_size - 8;
- if (auth_type > 0 && auth_type < 4) { /* auth_type
check */
- MichaelMICFunction(&michel_mic, (u8
*)priv->wpa.key[auth_type - 1].rx_mic_key, (u8 *)priv->rxp, (int)priv->rx_size,
(u8)0, /* priority */
- (u8 *)michel_mic.Result);
+ if (memcmp(michel_mic.Result, RecvMIC, 8)) {
+ now = jiffies;
+ mic_failure = &priv->wpa.mic_failure;
+ /* MIC FAILURE */
+ if (mic_failure->last_failure_time &&
+ (now -
+ mic_failure->last_failure_time) /
+ HZ >= 60) {
+ mic_failure->failure = 0;
}
- if (memcmp(michel_mic.Result, RecvMIC, 8)) {
- now = jiffies;
- mic_failure = &priv->wpa.mic_failure;
- /* MIC FAILURE */
- if (mic_failure->last_failure_time &&
- (now -
-
[PATCH 05/12] staging: ks7010: remove unnecessary parenthesis
Checkpatch emits CHECK: Unnecessary parentheses.
Remove unnecessary parentheses.
Signed-off-by: Tobin C. Harding
---
drivers/staging/ks7010/ks_hostif.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/ks7010/ks_hostif.c
b/drivers/staging/ks7010/ks_hostif.c
index a354e34e..b75ef1d6 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -36,7 +36,7 @@ inline u8 get_BYTE(struct ks_wlan_private *priv)
{
u8 data;
- data = *(priv->rxp)++;
+ data = *priv->rxp++;
/* length check in advance ! */
--(priv->rx_size);
return data;
@@ -192,7 +192,7 @@ int get_current_ap(struct ks_wlan_private *priv, struct
link_ap_info_t *ap_info)
DPRINTK(4, "\nLink AP\n");
DPRINTK(4, "bssid=%02X:%02X:%02X:%02X:%02X:%02X\n \
essid=%s\nrate_set=%02X,%02X,%02X,%02X,%02X,%02X,%02X,%02X\n
channel=%d\n \
- rssi=%d\nsq=%d\ncapability=%04X\n", ap->bssid[0], ap->bssid[1],
ap->bssid[2], ap->bssid[3], ap->bssid[4], ap->bssid[5], &(ap->ssid.body[0]),
ap->rate_set.body[0], ap->rate_set.body[1], ap->rate_set.body[2],
ap->rate_set.body[3], ap->rate_set.body[4], ap->rate_set.body[5],
ap->rate_set.body[6], ap->rate_set.body[7], ap->channel, ap->rssi, ap->sq,
ap->capability);
+ rssi=%d\nsq=%d\ncapability=%04X\n", ap->bssid[0], ap->bssid[1],
ap->bssid[2], ap->bssid[3], ap->bssid[4], ap->bssid[5], &ap->ssid.body[0],
ap->rate_set.body[0], ap->rate_set.body[1], ap->rate_set.body[2],
ap->rate_set.body[3], ap->rate_set.body[4], ap->rate_set.body[5],
ap->rate_set.body[6], ap->rate_set.body[7], ap->channel, ap->rssi, ap->sq,
ap->capability);
DPRINTK(4, "\nLink AP\nrsn.mode=%d\nrsn.size=%d\n",
ap_info->rsn_mode, ap_info->rsn.size);
DPRINTK(4, "\next_rate_set_size=%d\nrate_set_size=%d\n",
@@ -239,19 +239,19 @@ int get_ap_information(struct ks_wlan_private *priv,
struct ap_info_t *ap_info,
*(bp + 1));
ap->ssid.size = SSID_MAX_SIZE;
}
- memcpy(&(ap->ssid.body[0]), bp + 2, ap->ssid.size);
+ memcpy(&ap->ssid.body[0], bp + 2, ap->ssid.size);
break;
case 1: /* rate */
case 50:/* ext rate */
if ((*(bp + 1) + ap->rate_set.size) <=
RATE_SET_MAX_SIZE) {
- memcpy(&(ap->rate_set.body[ap->rate_set.size]),
+ memcpy(&ap->rate_set.body[ap->rate_set.size],
bp + 2, *(bp + 1));
ap->rate_set.size += *(bp + 1);
} else {
DPRINTK(1, "size over :: rate size=%d\n",
(*(bp + 1) + ap->rate_set.size));
- memcpy(&(ap->rate_set.body[ap->rate_set.size]),
+ memcpy(&ap->rate_set.body[ap->rate_set.size],
bp + 2,
RATE_SET_MAX_SIZE - ap->rate_set.size);
ap->rate_set.size +=
@@ -269,7 +269,7 @@ int get_ap_information(struct ks_wlan_private *priv, struct
ap_info_t *ap_info,
*(bp + 1));
ap->rsn_ie.size = RSN_IE_BODY_MAX;
}
- memcpy(&(ap->rsn_ie.body[0]), bp + 2, ap->rsn_ie.size);
+ memcpy(&ap->rsn_ie.body[0], bp + 2, ap->rsn_ie.size);
break;
case 221: /* WPA */
if (!memcmp(bp + 2, "\x00\x50\xf2\x01", 4)) { /* WPA
OUI check */
@@ -282,7 +282,7 @@ int get_ap_information(struct ks_wlan_private *priv, struct
ap_info_t *ap_info,
*(bp + 1));
ap->wpa_ie.size = RSN_IE_BODY_MAX;
}
- memcpy(&(ap->wpa_ie.body[0]), bp + 2,
+ memcpy(&ap->wpa_ie.body[0], bp + 2,
ap->wpa_ie.size);
}
break;
@@ -828,8 +828,8 @@ void hostif_scan_indication(struct ks_wlan_private *priv)
if (priv->scan_ind_count != 0) {
for (i = 0; i < priv->aplist.size; i++) { /* bssid check
*/
if (!memcmp
- (&(ap_info->bssid[0]),
-&(priv->aplist.ap[i].bssid[0]), ETH_ALEN)) {
+ (&ap_info->bssid[0],
+&priv->aplist.ap[i].bssid[0], ETH_ALEN)) {
if (ap_info->frame_type ==
[PATCH 04/12] staging: ks7010: fix spelling mistake
Comment contains misspelled work 'Adress'.
Correct spelling: Adress -> Address
Signed-off-by: Tobin C. Harding
---
drivers/staging/ks7010/ks_hostif.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/ks7010/ks_hostif.h
b/drivers/staging/ks7010/ks_hostif.h
index 2d596a8..e21cfe3f 100644
--- a/drivers/staging/ks7010/ks_hostif.h
+++ b/drivers/staging/ks7010/ks_hostif.h
@@ -110,9 +110,9 @@ struct channel_list_t {
#defineDOT11_OPERATION_RATE_SET 0x0100/* rate set */
#define LOCAL_AP_SEARCH_INTEAVAL 0xF1010100 /* AP search interval
(R/W) */
-#define LOCAL_CURRENTADDRESS 0xF1050100 /* MAC Adress change
(W) */
-#define LOCAL_MULTICAST_ADDRESS 0xF1060100 /* Multicast Adress (W)
*/
-#define LOCAL_MULTICAST_FILTER0xF1060200 /* Multicast Adress
Filter enable/disable (W) */
+#define LOCAL_CURRENTADDRESS 0xF1050100 /* MAC Address change
(W) */
+#define LOCAL_MULTICAST_ADDRESS 0xF1060100 /* Multicast Address
(W) */
+#define LOCAL_MULTICAST_FILTER0xF1060200 /* Multicast Address
Filter enable/disable (W) */
#define LOCAL_SEARCHED_AP_LIST0xF1030100 /* AP list (R) */
#define LOCAL_LINK_AP_STATUS 0xF1040100 /* Link AP status (R) */
#defineLOCAL_PACKET_STATISTICS 0xF1020100/* tx,rx
packets statistics */
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 03/12] staging: ks7010: replace C types with kernel types
Checkpatch emits CHECK prefer kernel type. Source and header file use
C standard types uintN_t.
Replace C standard types with kernel types. uintN_t -> uN
Signed-off-by: Tobin C. Harding
---
drivers/staging/ks7010/ks_hostif.c | 250 +++---
drivers/staging/ks7010/ks_hostif.h | 304 ++---
2 files changed, 277 insertions(+), 277 deletions(-)
diff --git a/drivers/staging/ks7010/ks_hostif.c
b/drivers/staging/ks7010/ks_hostif.c
index a6c5c53..a354e34e 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -379,8 +379,8 @@ void hostif_data_indication(struct ks_wlan_private *priv)
(priv->rxp) + ((priv->rx_size) - 8), 8);
priv->rx_size = priv->rx_size - 8;
if (auth_type > 0 && auth_type < 4) { /*
auth_type check */
- MichaelMICFunction(&michel_mic,
(uint8_t *)priv->wpa.key[auth_type - 1].rx_mic_key, (uint8_t *)priv->rxp,
(int)priv->rx_size, (uint8_t)0, /* priority */
- (uint8_t
*)michel_mic.Result);
+ MichaelMICFunction(&michel_mic, (u8
*)priv->wpa.key[auth_type - 1].rx_mic_key, (u8 *)priv->rxp, (int)priv->rx_size,
(u8)0, /* priority */
+ (u8
*)michel_mic.Result);
}
if (memcmp(michel_mic.Result, RecvMIC, 8)) {
now = jiffies;
@@ -399,7 +399,7 @@ void hostif_data_indication(struct ks_wlan_private *priv)
} else if (mic_failure->failure == 1) {
mic_failure->failure = 2;
mic_failure->counter =
- (uint16_t)((now -
+ (u16)((now -
mic_failure->
last_failure_time)
/ HZ);
@@ -507,10 +507,10 @@ static
void hostif_mib_get_confirm(struct ks_wlan_private *priv)
{
struct net_device *dev = priv->net_dev;
- uint32_t mib_status;
- uint32_t mib_attribute;
- uint16_t mib_val_size;
- uint16_t mib_val_type;
+ u32 mib_status;
+ u32 mib_attribute;
+ u16 mib_val_size;
+ u16 mib_val_type;
DPRINTK(3, "\n");
@@ -587,8 +587,8 @@ void hostif_mib_get_confirm(struct ks_wlan_private *priv)
static
void hostif_mib_set_confirm(struct ks_wlan_private *priv)
{
- uint32_t mib_status;/* +04 MIB Status */
- uint32_t mib_attribute; /* +08 MIB attribute */
+ u32 mib_status; /* +04 MIB Status */
+ u32 mib_attribute; /* +08 MIB attribute */
DPRINTK(3, "\n");
@@ -902,7 +902,7 @@ void hostif_ps_adhoc_set_confirm(struct ks_wlan_private
*priv)
static
void hostif_infrastructure_set_confirm(struct ks_wlan_private *priv)
{
- uint16_t result_code;
+ u16 result_code;
DPRINTK(3, "\n");
result_code = get_WORD(priv);
@@ -1216,37 +1216,37 @@ int hostif_data_request(struct ks_wlan_private *priv,
struct sk_buff *packet)
&& !(priv->wpa.key[1].key_len)
&& !(priv->wpa.key[2].key_len)
&& !(priv->wpa.key[3].key_len)) {
- pp->auth_type = cpu_to_le16((uint16_t)TYPE_AUTH);
/* no encryption */
+ pp->auth_type = cpu_to_le16((u16)TYPE_AUTH);/* no
encryption */
} else {
if (priv->wpa.pairwise_suite == IW_AUTH_CIPHER_TKIP) {
- MichaelMICFunction(&michel_mic, (uint8_t
*)priv->wpa.key[0].tx_mic_key, (uint8_t *)&pp->data[0], (int)packet_len,
(uint8_t)0, /* priority */
- (uint8_t *)michel_mic.
+ MichaelMICFunction(&michel_mic, (u8
*)priv->wpa.key[0].tx_mic_key, (u8 *)&pp->data[0], (int)packet_len, (u8)0, /*
priority */
+ (u8 *)michel_mic.
Result);
memcpy(p, michel_mic.Result, 8);
length += 8;
packet_len += 8;
p += 8;
pp->auth_type =
- cpu_to_le16((uint16_t)TYPE_DATA);
+ cpu_to_le16((u16)TYPE_DATA);
} else if (priv->wpa.pairwise_suite ==
IW_AUTH_CIPHER_CCMP) {
[PATCH 02/12] staging: ks7010: fix checkpatch CONSTANT_COMPARISON
Checkpatch emits WARNING: Comparisons should place the constant on the
right side of the test.
Move comparison constant to the right side of the test.
Signed-off-by: Tobin C. Harding
---
drivers/staging/ks7010/ks_hostif.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/ks7010/ks_hostif.c
b/drivers/staging/ks7010/ks_hostif.c
index 4fe922e..a6c5c53 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -2596,13 +2596,13 @@ void hostif_sme_task(unsigned long dev)
DPRINTK(3, "\n");
if (priv->dev_state >= DEVICE_STATE_BOOT) {
- if (0 < cnt_smeqbody(priv)
+ if (cnt_smeqbody(priv) > 0
&& priv->dev_state >= DEVICE_STATE_BOOT) {
hostif_sme_execute(priv,
priv->sme_i.event_buff[priv->sme_i.
qhead]);
inc_smeqhead(priv);
- if (0 < cnt_smeqbody(priv))
+ if (cnt_smeqbody(priv) > 0)
tasklet_schedule(&priv->sme_task);
}
}
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 00/12] refactor hostif_data_indication()
Checkpatch emits a bunch of checkpatch warnings, errors and checks when parsing ks_hostif.c and ks_hostif.h The First five patches of this set do general clean up, fixing various warnings along the way. Patch #1 fixes checkpatch warnings/errors relating to misplaced spaces. Patch #2, #3, #4 fix a single checkpatch warning per patch. If necessary a patch may touch both the header and the source file in order to group patches by warning type. Starting at patch #6 effort is applied to cleaning up the function hostif_data_indication(). Patch #6 refactors WPA code into a separate function, this is a cut and paste with additional code added to pass compilation. Patch #7 reduces the level of indentation in the new function. Patch #8 fixes a spelling mistake in various symbols. Patch #9 renames two buffers with more meaningful names. Patch #10 adds two variables to reduce complex access of deeply nested data structures. Patch #11 does whitespace refactor taking advantage of the changes made in the previous patches. Patch #12 removes a line over 80 warning. Series compiles after each commit. Program logic is not modified at any stage of the series. Further testing has not been carried out. Tobin C. Harding (12): staging: ks7010: fix checkpatch whitespace warnings staging: ks7010: fix checkpatch CONSTANT_COMPARISON staging: ks7010: replace C types with kernel types staging: ks7010: fix spelling mistake staging: ks7010: remove unnecessary parenthesis staging: ks7010: factor out WPA code staging: ks7010: reduce level of indentation staging: ks7010: fix spelling of Michael MIC staging: ks7010: add meaningful buffer names staging: ks7010: add variables key, key_index staging: ks7010: refactor, improve code layout staging: ks7010: fix checkpatch line over 80 drivers/staging/ks7010/ks_hostif.c | 448 ++- drivers/staging/ks7010/ks_hostif.h | 352 +-- drivers/staging/ks7010/michael_mic.c | 8 +- drivers/staging/ks7010/michael_mic.h | 4 +- 4 files changed, 410 insertions(+), 402 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/12] staging: ks7010: fix checkpatch whitespace warnings
Checkpatch emits various warnings/errors pointing to misplaced
spaces.
- trailing whitespace
- please, no spaces at the start of a line
- please, no space before tabs
- Unnecessary space before function pointer arguments
- unnecessary whitespace before a quoted newline
- code indent should use tabs where possible
Remove all undesirable whitespace.
Signed-off-by: Tobin C. Harding
---
drivers/staging/ks7010/ks_hostif.c | 2 --
drivers/staging/ks7010/ks_hostif.h | 50 +++---
2 files changed, 25 insertions(+), 27 deletions(-)
diff --git a/drivers/staging/ks7010/ks_hostif.c
b/drivers/staging/ks7010/ks_hostif.c
index ae7cf3f..4fe922e 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -2617,14 +2617,12 @@ void hostif_sme_enqueue(struct ks_wlan_private *priv,
unsigned short event)
if (cnt_smeqbody(priv) < (SME_EVENT_BUFF_SIZE - 1)) {
priv->sme_i.event_buff[priv->sme_i.qtail] = event;
inc_smeqtail(priv);
- //DPRINTK(3,"inc_smeqtail \n");
#ifdef KS_WLAN_DEBUG
if (priv->sme_i.max_event_count < cnt_smeqbody(priv))
priv->sme_i.max_event_count = cnt_smeqbody(priv);
#endif /* KS_WLAN_DEBUG */
} else {
/* in case of buffer overflow */
- //DPRINTK(2,"sme queue buffer overflow\n");
netdev_err(priv->net_dev, "sme queue buffer overflow\n");
}
diff --git a/drivers/staging/ks7010/ks_hostif.h
b/drivers/staging/ks7010/ks_hostif.h
index 61c2f96..1d57bf0 100644
--- a/drivers/staging/ks7010/ks_hostif.h
+++ b/drivers/staging/ks7010/ks_hostif.h
@@ -1,6 +1,6 @@
/*
* Driver for KeyStream wireless LAN
- *
+ *
* Copyright (c) 2005-2008 KeyStream Corp.
* Copyright (C) 2009 Renesas Technology Corp.
*
@@ -128,7 +128,7 @@ struct channel_list_t {
#define DOT11_PMK_TSC 0x55010100 /* PMK_TSC (W) */
#define DOT11_GMK1_TSC0x55010101 /* GMK1_TSC (W) */
#define DOT11_GMK2_TSC0x55010102 /* GMK2_TSC (W) */
-#define DOT11_GMK3_TSC 0x55010103/* GMK3_TSC */
+#define DOT11_GMK3_TSC0x55010103 /* GMK3_TSC */
#define LOCAL_PMK 0x58010100 /* Pairwise Master Key
cache (W) */
#define LOCAL_REGION 0xF10A0100 /* Region setting */
@@ -360,7 +360,7 @@ struct hostif_ps_adhoc_set_request_t {
#define CTS_MODE_TRUE 1
uint16_t channel;
struct rate_set16_t rate_set;
- uint16_t capability;/* bit5:preamble bit6:pbcc pbcc not supported
always 0
+ uint16_t capability;/* bit5:preamble bit6:pbcc pbcc not supported
always 0
* bit10:ShortSlotTime bit13:DSSS-OFDM
DSSS-OFDM not supported always 0 */
uint16_t scan_type;
} __packed;
@@ -376,7 +376,7 @@ struct hostif_infrastructure_set_request_t {
uint16_t cts_mode;
struct rate_set16_t rate_set;
struct ssid_t ssid;
- uint16_t capability;/* bit5:preamble bit6:pbcc pbcc not supported
always 0
+ uint16_t capability;/* bit5:preamble bit6:pbcc pbcc not supported
always 0
* bit10:ShortSlotTime bit13:DSSS-OFDM
DSSS-OFDM not supported always 0 */
uint16_t beacon_lost_count;
uint16_t auth_type;
@@ -392,7 +392,7 @@ struct hostif_infrastructure_set2_request_t {
uint16_t cts_mode;
struct rate_set16_t rate_set;
struct ssid_t ssid;
- uint16_t capability;/* bit5:preamble bit6:pbcc pbcc not supported
always 0
+ uint16_t capability;/* bit5:preamble bit6:pbcc pbcc not supported
always 0
* bit10:ShortSlotTime bit13:DSSS-OFDM
DSSS-OFDM not supported always 0 */
uint16_t beacon_lost_count;
uint16_t auth_type;
@@ -415,7 +415,7 @@ struct hostif_adhoc_set_request_t {
uint16_t channel;
struct rate_set16_t rate_set;
struct ssid_t ssid;
- uint16_t capability;/* bit5:preamble bit6:pbcc pbcc not supported
always 0
+ uint16_t capability;/* bit5:preamble bit6:pbcc pbcc not supported
always 0
* bit10:ShortSlotTime bit13:DSSS-OFDM
DSSS-OFDM not supported always 0 */
uint16_t scan_type;
} __packed;
@@ -427,7 +427,7 @@ struct hostif_adhoc_set2_request_t {
uint16_t reserved;
struct rate_set16_t rate_set;
struct ssid_t ssid;
- uint16_t capability;/* bit5:preamble bit6:pbcc pbcc not supported
always 0
+ uint16_t capability;/* bit5:preamble bit6:pbcc pbcc not supported
always 0
* bit10:ShortSlotTime bit13:DSSS-OFDM
DSSS-OFDM not supported always 0 */
uint16_t scan_type;
struct channel_list_t channel_list;
@@ -568,19 +568,19 @@ struct hostif_mic_failure_confirm_t {
#define TX_RATE_48M(uint8_t
[PATCH V5 2/2] staging: vchiq_arm: Disable ability to dump memory by default
vc04_services has an ioctl interface to dump arbitrary memory
to a custom debug log. This is typically only needed by
diagnostic tools, and can potentially be a security issue
if the devtmpfs node doesn't have adequate permissions set.
Since the ability to dump memory still has debugging value,
create a new build configuration and disable the feature
by default.
Signed-off-by: Michael Zoran
---
drivers/staging/vc04_services/Kconfig | 12
.../staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 14 ++
2 files changed, 26 insertions(+)
diff --git a/drivers/staging/vc04_services/Kconfig
b/drivers/staging/vc04_services/Kconfig
index eb52cec18c87..b9f316603308 100644
--- a/drivers/staging/vc04_services/Kconfig
+++ b/drivers/staging/vc04_services/Kconfig
@@ -18,6 +18,18 @@ config BCM2835_VCHIQ
Defaults to Y when the Broadcom Videocore services
are included in the build, N otherwise.
+if BCM2835_VCHIQ
+
+config BCM2835_VCHIQ_SUPPORT_MEMDUMP
+ bool "Support dumping memory contents to debug log"
+ help
+ BCM2835 VCHIQ supports the ability to dump the
+ contents of memory to the debug log. This
+ is typically only needed by diagnostic tools used
+ to debug issues with VideoCore.
+
+endif
+
source "drivers/staging/vc04_services/bcm2835-audio/Kconfig"
source "drivers/staging/vc04_services/bcm2835-camera/Kconfig"
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index ca6ab47fba6f..ff96e71cfa8b 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -195,8 +195,10 @@ static const char *const ioctl_names[] = {
vchiq_static_assert(ARRAY_SIZE(ioctl_names) ==
(VCHIQ_IOC_MAX + 1));
+#if defined(CONFIG_BCM2835_VCHIQ_SUPPORT_MEMDUMP)
static void
dump_phys_mem(void *virt_addr, u32 num_bytes);
+#endif
/
*
@@ -1159,6 +1161,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
args.handle, args.option, args.value);
} break;
+#if defined(CONFIG_BCM2835_VCHIQ_SUPPORT_MEMDUMP)
case VCHIQ_IOC_DUMP_PHYS_MEM: {
VCHIQ_DUMP_MEM_T args;
@@ -1170,6 +1173,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
}
dump_phys_mem(args.virt_addr, args.num_bytes);
} break;
+#endif
case VCHIQ_IOC_LIB_VERSION: {
unsigned int lib_version = (unsigned int)arg;
@@ -1650,6 +1654,8 @@ vchiq_compat_ioctl_get_config(struct file *file,
return vchiq_ioctl(file, VCHIQ_IOC_GET_CONFIG, (unsigned long)args);
}
+#if defined(CONFIG_BCM2835_VCHIQ_SUPPORT_MEMDUMP)
+
struct vchiq_dump_mem32 {
compat_uptr_t virt_addr;
u32 num_bytes;
@@ -1682,6 +1688,8 @@ vchiq_compat_ioctl_dump_phys_mem(struct file *file,
return vchiq_ioctl(file, VCHIQ_IOC_DUMP_PHYS_MEM, (unsigned long)args);
}
+#endif
+
static long
vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
@@ -1699,8 +1707,10 @@ vchiq_compat_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
return vchiq_compat_ioctl_dequeue_message(file, cmd, arg);
case VCHIQ_IOC_GET_CONFIG32:
return vchiq_compat_ioctl_get_config(file, cmd, arg);
+#if defined(CONFIG_BCM2835_VCHIQ_SUPPORT_MEMDUMP)
case VCHIQ_IOC_DUMP_PHYS_MEM32:
return vchiq_compat_ioctl_dump_phys_mem(file, cmd, arg);
+#endif
default:
return vchiq_ioctl(file, cmd, arg);
}
@@ -2044,6 +2054,8 @@ vchiq_dump_platform_service_state(void *dump_context,
VCHIQ_SERVICE_T *service)
*
***/
+#if defined(CONFIG_BCM2835_VCHIQ_SUPPORT_MEMDUMP)
+
static void
dump_phys_mem(void *virt_addr, u32 num_bytes)
{
@@ -2126,6 +2138,8 @@ dump_phys_mem(void *virt_addr, u32 num_bytes)
kfree(pages);
}
+#endif
+
/
*
* vchiq_read
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V5 0/2] staging: vchiq_arm: Add compatibility wrappers for ioctls
Here is V5 of the changes to add 32 compatibility to vc04_services as required by 32 bit only operating systems such as Raspbian which is the preferred system for the Raspberry PI. Changes: V1 - Complete rewrite of the ioctl code. V2 - Rewrite of only ioctls that change between 32 bit and 64 bit. V3 - Minor changes. V4 - Abandon cleaning up the exising code and completely write the wrappers on top of the native ioctls. No existing lines are changed. V5 - Add additional comments to clarify the assumptions of the await completion ioctl. Hard code some return values to clarify what the return values are. Remove the ability to dump memory to the debug log and create a new build configuration option to reenable it for support of existing closed source diagnostic tools. Michael Zoran (2): staging: vchiq_arm: Add compatibility wrappers for ioctls staging: vchiq_arm: Disable ability to dump memory by default drivers/staging/vc04_services/Kconfig | 12 + .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 497 + 2 files changed, 509 insertions(+) -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V5 1/2] staging: vchiq_arm: Add compatibility wrappers for ioctls
This patch adds compatibility wrappers for the ioctls
exposed by vchiq/vc04_services. The compat ioctls are
completely implemented on top of the native ioctls. No
existing lines are modified.
While the ideal approach would be to cleanup the existing
code, this path is simplier and easier to review. While
it does have a small runtime performance penality vs
seperating the existing code into wrapper+worker functions,
the penality is small since only the metadata is copied
back onto the 32 bit user mode stack.
The on top of approach is the approach used by several
existing performance critical subsystems of Linux such
as the DRM 3D graphics subsystem.
Testing:
1. A 32 bit chroot was created on a RPI 3 and vchiq_test
was built for armhf. The usual tests were run such as
vchiq_test -f 10 and vchiq_test -p.
2. This patch was copied onto the shipping version of
the Linux kernel used for the RPI and that kernel was
built for arm64. That kernel was used to boot Raspbian.
Many of the builtin features are now functional such
as the "hello_pi" examples, and minecraft_pi.
Signed-off-by: Michael Zoran
---
.../vc04_services/interface/vchiq_arm/vchiq_arm.c | 483 +
1 file changed, 483 insertions(+)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index cc4cb168f483..ca6ab47fba6f 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -48,6 +48,7 @@
#include
#include
#include
+#include
#include
#include "vchiq_core.h"
@@ -1228,6 +1229,485 @@ vchiq_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
return ret;
}
+#if defined(CONFIG_COMPAT)
+
+struct vchiq_service_params32 {
+ int fourcc;
+ compat_uptr_t callback;
+ compat_uptr_t userdata;
+ short version; /* Increment for non-trivial changes */
+ short version_min; /* Update for incompatible changes */
+};
+
+struct vchiq_create_service32 {
+ struct vchiq_service_params32 params;
+ int is_open;
+ int is_vchi;
+ unsigned int handle; /* OUT */
+};
+
+#define VCHIQ_IOC_CREATE_SERVICE32 \
+ _IOWR(VCHIQ_IOC_MAGIC, 2, struct vchiq_create_service32)
+
+static long
+vchiq_compat_ioctl_create_service(
+ struct file *file,
+ unsigned int cmd,
+ unsigned long arg)
+{
+ VCHIQ_CREATE_SERVICE_T __user *args;
+ struct vchiq_create_service32 __user *ptrargs32 =
+ (struct vchiq_create_service32 __user *)arg;
+ struct vchiq_create_service32 args32;
+ long ret;
+
+ args = compat_alloc_user_space(sizeof(*args));
+ if (!args)
+ return -EFAULT;
+
+ if (copy_from_user(&args32,
+ (struct vchiq_create_service32 __user *)arg,
+ sizeof(args32)))
+ return -EFAULT;
+
+ if (put_user(args32.params.fourcc, &args->params.fourcc) ||
+ put_user(compat_ptr(args32.params.callback),
+&args->params.callback) ||
+ put_user(compat_ptr(args32.params.userdata),
+&args->params.userdata) ||
+ put_user(args32.params.version, &args->params.version) ||
+ put_user(args32.params.version_min,
+&args->params.version_min) ||
+ put_user(args32.is_open, &args->is_open) ||
+ put_user(args32.is_vchi, &args->is_vchi) ||
+ put_user(args32.handle, &args->handle))
+ return -EFAULT;
+
+ ret = vchiq_ioctl(file, VCHIQ_IOC_CREATE_SERVICE, (unsigned long)args);
+
+ if (ret < 0)
+ return ret;
+
+ if (get_user(args32.handle, &args->handle))
+ return -EFAULT;
+
+ if (copy_to_user(&ptrargs32->handle,
+&args32.handle,
+sizeof(args32.handle)))
+ return -EFAULT;
+
+ return 0;
+}
+
+struct vchiq_element32 {
+ compat_uptr_t data;
+ unsigned int size;
+};
+
+struct vchiq_queue_message32 {
+ unsigned int handle;
+ unsigned int count;
+ compat_uptr_t elements;
+};
+
+#define VCHIQ_IOC_QUEUE_MESSAGE32 \
+ _IOW(VCHIQ_IOC_MAGIC, 4, struct vchiq_queue_message32)
+
+static long
+vchiq_compat_ioctl_queue_message(struct file *file,
+unsigned int cmd,
+unsigned long arg)
+{
+ VCHIQ_QUEUE_MESSAGE_T *args;
+ VCHIQ_ELEMENT_T *elements;
+ struct vchiq_queue_message32 args32;
+ unsigned int count;
+
+ if (copy_from_user(&args32,
+ (struct vchiq_queue_message32 __user *)arg,
+ sizeof(args32)))
+ return -EFAULT;
+
+ args = compat_alloc_user_space(sizeof(*args) +
+ (sizeof(*elements) * MAX_ELEMENTS));
+
+ if (
Re: [PATCH] storvsc: workaround for virtual DVD SCSI version
> "Stephen" == Stephen Hemminger writes: Stephen, Stephen> Hyper-V host emulation of SCSI for virtual DVD device reports Stephen> SCSI version 0 (UNKNOWN) but is still capable of supporting Stephen> REPORTLUN. Stephen> Without this patch, a GEN2 Linux guest on Hyper-V will not boot Stephen> 4.11 successfully with virtual DVD ROM device. What happens is Stephen> that the SCSI scan process falls back to doing sequential Stephen> probing by INQUIRY. But the storvsc driver has a previous Stephen> workaround that masks/blocks all errors reports from INQUIRY Stephen> (or MODE_SENSE) commands. This workaround causes the scan to Stephen> then populate a full set of bogus LUN's on the target and then Stephen> sends kernel spinning off into a death spiral doing block reads Stephen> on the non-existent LUNs. Stephen> By setting the correct blacklist flags, the target with the DVD Stephen> device is scanned with REPORTLUN and that works correctly. Applied to 4.11/scsi-fixes. Thank you! -- Martin K. Petersen Oracle Linux Engineering ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: rtl8188eu: wrap macro parameters in parentheses
On Tue, Mar 07, 2017 at 11:06:48PM +0300, Dan Carpenter wrote:
> On Sat, Mar 04, 2017 at 11:05:58PM +0100, Sebastian Haas wrote:
> > diff --git a/drivers/staging/rtl8188eu/include/rtl8188e_hal.h
> > b/drivers/staging/rtl8188eu/include/rtl8188e_hal.h
> > index 9330361..ca9db14 100644
> > --- a/drivers/staging/rtl8188eu/include/rtl8188e_hal.h
> > +++ b/drivers/staging/rtl8188eu/include/rtl8188e_hal.h
> > @@ -146,7 +146,7 @@ struct txpowerinfo24g {
> > #define EFUSE_REAL_CONTENT_LEN 512
> > #define EFUSE_MAX_SECTION 16
> > #define EFUSE_IC_ID_OFFSET 506 /* For some inferior IC purpose*/
> > -#define AVAILABLE_EFUSE_ADDR(addr) (addr < EFUSE_REAL_CONTENT_LEN)
> > +#define AVAILABLE_EFUSE_ADDR(addr) ((addr) < EFUSE_REAL_CONTENT_LEN)
>
> This one is never used. I feel like you're just randomly adding
> parenthesis instead of taking your time to think about things.
>
Not randomly, but maybe following too blindly the checkpatch findings.
> > --- a/drivers/staging/rtl8188eu/include/rtw_ioctl.h
> > +++ b/drivers/staging/rtl8188eu/include/rtw_ioctl.h
> > @@ -59,7 +59,7 @@
> > #define OID_MP_SEG40xFF011100
> >
> > #define DEBUG_OID(dbg, str)
> > \
> > - if ((!dbg)) { \
> > + if ((!(dbg))) { \
>
> Ugh...
>
> > #define rtw_get_ips_mode_req(pwrctrlpriv) \
> > - (pwrctrlpriv)->ips_mode_req
> > + ((pwrctrlpriv)->ips_mode_req)
>
> Do you really think this is an issue? I know I should be in favour of
> defensive coding and all that, but I really feel like the focus should
> be on real issues, cleaning things up and deleting as much of this code
> as we can instead of just adding parenthesis.
You are right, I have not checked if the code is still in use and worth
to rewrite it. I'll try to come up with a better patch.
thanks,
sebastian
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] storvsc: workaround for virtual DVD SCSI version
Thanks Stephen, this looks good to me: Reviewed-by: Christoph Hellwig ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: unisys: fix sparse warnings
On Tue, Mar 07, 2017 at 11:17:36PM +0300, Dan Carpenter wrote: > On Sat, Mar 04, 2017 at 06:20:50PM +0100, Andrea Ghittino wrote: > > Sparse generates two warnings related to incorrect type in assignment. > > This patch changes the types in the struct defined in unisys > > Can you post the Sparse warning? Otherwise when I'm reviewing this > code I have to re-run Sparse to see what you're talking about. > > regards, > dan carpenter Here you can find sparse warnings: drivers/staging/unisys/visornic/visornic_main.c:844:50: warning: incorrect type in assignment (different base types) drivers/staging/unisys/visornic/visornic_main.c:844:50:expected unsigned short [unsigned] [usertype] protocol drivers/staging/unisys/visornic/visornic_main.c:844:50:got restricted __be16 [usertype] protocol drivers/staging/unisys/visornic/visornic_main.c:855:46: warning: incorrect type in assignment (different base types) drivers/staging/unisys/visornic/visornic_main.c:855:46:expected unsigned int [unsigned] [usertype] csum drivers/staging/unisys/visornic/visornic_main.c:855:46:got restricted __wsum [usertype] csum andrea ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: wilc1000: Fix sparse warnings incorrect type assignment
Fixed sparse warnings related to the conversion of le16 and le32 to u16 and
u32, during the update of internal structures
Fixed sparse warnings:
drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2011:52: warning: incorrect
type in assignment (different base types)
drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2011:52:expected
unsigned short [unsigned] [assigned] [usertype] ht_ext_params
drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2011:52:got restricted
__le16 const [usertype] extended_ht_cap_info
drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2012:51: warning: incorrect
type in assignment (different base types)
drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2012:51:expected
unsigned int [unsigned] [assigned] [usertype] ht_tx_bf_cap
drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2012:51:got restricted
__le32 const [usertype] tx_BF_cap_info
drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2078:51: warning: incorrect
type in assignment (different base types)
drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2078:51:expected
unsigned short [unsigned] [assigned] [usertype] ht_capa_info
drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2078:51:got restricted
__le16 const [usertype] cap_info
drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2083:52: warning: incorrect
type in assignment (different base types)
drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2083:52:expected
unsigned short [unsigned] [assigned] [usertype] ht_ext_params
drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2083:52:got restricted
__le16 const [usertype] extended_ht_cap_info
drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2084:51: warning: incorrect
type in assignment (different base types)
drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2084:51:expected
unsigned int [unsigned] [assigned] [usertype] ht_tx_bf_cap
drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2084:51:got restricted
__le32 const [usertype] tx_BF_cap_info
Signed-off-by: Andrea Ghittino
---
changelog:
v2) Changed patch desacription
Compile tested only
wilc_wfi_cfgoperations.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 7961d1c..d3b5ac8 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -2003,13 +2003,13 @@ static int add_station(struct wiphy *wiphy, struct
net_device *dev,
strStaParams.ht_supported = false;
} else {
strStaParams.ht_supported = true;
- strStaParams.ht_capa_info = params->ht_capa->cap_info;
+ strStaParams.ht_capa_info =
le16_to_cpu(params->ht_capa->cap_info);
strStaParams.ht_ampdu_params =
params->ht_capa->ampdu_params_info;
memcpy(strStaParams.ht_supp_mcs_set,
¶ms->ht_capa->mcs,
WILC_SUPP_MCS_SET_SIZE);
- strStaParams.ht_ext_params =
params->ht_capa->extended_ht_cap_info;
- strStaParams.ht_tx_bf_cap =
params->ht_capa->tx_BF_cap_info;
+ strStaParams.ht_ext_params =
le16_to_cpu(params->ht_capa->extended_ht_cap_info);
+ strStaParams.ht_tx_bf_cap =
le32_to_cpu(params->ht_capa->tx_BF_cap_info);
strStaParams.ht_ante_sel =
params->ht_capa->antenna_selection_info;
}
@@ -2075,13 +2075,13 @@ static int change_station(struct wiphy *wiphy, struct
net_device *dev,
strStaParams.ht_supported = false;
} else {
strStaParams.ht_supported = true;
- strStaParams.ht_capa_info = params->ht_capa->cap_info;
+ strStaParams.ht_capa_info =
le16_to_cpu(params->ht_capa->cap_info);
strStaParams.ht_ampdu_params =
params->ht_capa->ampdu_params_info;
memcpy(strStaParams.ht_supp_mcs_set,
¶ms->ht_capa->mcs,
WILC_SUPP_MCS_SET_SIZE);
- strStaParams.ht_ext_params =
params->ht_capa->extended_ht_cap_info;
- strStaParams.ht_tx_bf_cap =
params->ht_capa->tx_BF_cap_info;
+ strStaParams.ht_ext_params =
le16_to_cpu(params->ht_capa->extended_ht_cap_info);
+ strStaParams.ht_tx_bf_cap =
le32_to_cpu(params->ht_capa->tx_BF_cap_info);
strStaParams.ht_ante_sel =
params->ht_capa->antenna_selection_info;
}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: cb_pcidas64: move loop invariant
On Wed, Mar 08, 2017 at 08:18:50AM +1100, Tobin C. Harding wrote: > On Tue, Mar 07, 2017 at 08:03:16PM +0100, Greg Kroah-Hartman wrote: > > On Tue, Mar 07, 2017 at 01:29:35PM +1100, Tobin C. Harding wrote: > > > Loop invariant is inside the loop so code checks invariant on each > > > iteration of the loop. Invariant can be moved outside of the loop so > > > it is only checked once. > > > > > > Move loop invariant outside of for loop. > > > > But does it really matter? Does this fix an issue? Make something > > faster in a _measurable_ way? Did you test this on the hardware? > > Good points, patch is untested and does not fix a particular > issue. Lesson learned. You've learned the wrong lesson. The right lesson is to sell your patch better. If you had said, "This improves readability and makes things more uniform" then it would probably have been accepted. But you sold it as a speed up and it obviously isn't. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] staging: dgnc: audit goto's in dgnc_driver
On Wed, Mar 08, 2017 at 08:06:31AM +1100, Tobin C. Harding wrote: > On Tue, Mar 07, 2017 at 08:03:51PM +0300, Dan Carpenter wrote: > > On Tue, Mar 07, 2017 at 05:33:06PM +1100, Tobin C. Harding wrote: > > > @@ -419,17 +411,14 @@ static int dgnc_init_one(struct pci_dev *pdev, > > > const struct pci_device_id *ent) > > > brd->dpastatus = BD_RUNNING; > > > > > > dgnc_board[dgnc_num_boards++] = brd; > > > - > > > return 0; > > > > > > > There's nothing wrong with putting a blank before a return 0. The blank > > sort of makes it stand out nicely. > > I thought this one would get a comment :) The reasoning was to be > uniform across the whole directory. Originally some returns were > preceded by a new line while others were not. I picked one and went for > uniformity. Is this level of uniformity too much? Is it better to be > less pedantic and have less code churn? > It's actually already uniform. If the function ends in "return 0;" there is a blank line. If the "return 0;" is in the middle there isn't. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: cb_pcidas64: move loop invariant
On Tue, Mar 07, 2017 at 08:03:16PM +0100, Greg Kroah-Hartman wrote: > On Tue, Mar 07, 2017 at 01:29:35PM +1100, Tobin C. Harding wrote: > > Loop invariant is inside the loop so code checks invariant on each > > iteration of the loop. Invariant can be moved outside of the loop so > > it is only checked once. > > > > Move loop invariant outside of for loop. > > But does it really matter? Does this fix an issue? Make something > faster in a _measurable_ way? Did you test this on the hardware? Good points, patch is untested and does not fix a particular issue. Lesson learned. thanks, Tobin. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging: ks7010: add additional goto target
On Wed, Mar 08, 2017 at 08:10:44AM +1100, Tobin C. Harding wrote:
> On Tue, Mar 07, 2017 at 08:19:03PM +0300, Dan Carpenter wrote:
> > On Tue, Mar 07, 2017 at 09:31:16AM +1100, Tobin C. Harding wrote:
> > @@ -610,12 +610,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
> > > if (atomic_read(&priv->psstatus.status) ==
> > > PS_SNOOZE) {
> > > if (cnt_txqbody(priv)) {
> > > ks_wlan_hw_wakeup_request(priv);
> > > - queue_delayed_work
> > > - (priv->ks_wlan_hw.
> > > - ks7010sdio_wq,
> > > -
> > > &priv->ks_wlan_hw.
> > > - rw_wq, 1);
> > > - return;
> > > + goto intr_out_with_delay;
> >
> >
> > The original code is crap but this isn't really an improvement. We've
> > just used a goto to split the code up randomly so it's hard to read.
>
> Righto, let's drop this one and I will come back to it when I
> understand the module enough to remove the ifdef 0
We traditionally remove ifdef 0 code without even thinking about it.
It's in git history if anyone wants to revive it.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging: ks7010: add additional goto target
On Tue, Mar 07, 2017 at 08:19:03PM +0300, Dan Carpenter wrote:
> On Tue, Mar 07, 2017 at 09:31:16AM +1100, Tobin C. Harding wrote:
> @@ -610,12 +610,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
> > if (atomic_read(&priv->psstatus.status) ==
> > PS_SNOOZE) {
> > if (cnt_txqbody(priv)) {
> > ks_wlan_hw_wakeup_request(priv);
> > - queue_delayed_work
> > - (priv->ks_wlan_hw.
> > - ks7010sdio_wq,
> > -
> > &priv->ks_wlan_hw.
> > - rw_wq, 1);
> > - return;
> > + goto intr_out_with_delay;
>
>
> The original code is crap but this isn't really an improvement. We've
> just used a goto to split the code up randomly so it's hard to read.
Righto, let's drop this one and I will come back to it when I
understand the module enough to remove the ifdef 0
thanks,
Tobin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] staging: dgnc: audit goto's in dgnc_tty
On Tue, Mar 07, 2017 at 08:08:44PM +0300, Dan Carpenter wrote:
> On Tue, Mar 07, 2017 at 05:33:08PM +1100, Tobin C. Harding wrote:
> > @@ -1668,20 +1669,20 @@ static int dgnc_tty_tiocmget(struct tty_struct *tty)
> > {
> > struct channel_t *ch;
> > struct un_t *un;
> > - int result = -EIO;
> > + int rc = -EIO;
> > unsigned char mstat = 0;
> > unsigned long flags;
> >
> > if (!tty || tty->magic != TTY_MAGIC)
> > - return result;
> > + return rc;
> >
>
> It's better to just do "return -EIO;" That way all the information is
> on one line, and you don't need to scroll back to find out what it's
> returning.
>
> > @@ -1987,20 +1990,21 @@ static int dgnc_tty_digigeta(struct tty_struct *tty,
> > struct un_t *un;
> > struct digi_t tmp;
> > unsigned long flags;
> > + int rc = -EFAULT;
> >
> > if (!retinfo)
> > - return -EFAULT;
> > + return rc;
>
> The original is better here.
Awesome, thanks. Will include in v3.
thanks,
Tobin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: dgnc: audit goto's in dgnc_mgmt
On Tue, Mar 07, 2017 at 08:05:10PM +0300, Dan Carpenter wrote:
> On Tue, Mar 07, 2017 at 05:33:07PM +1100, Tobin C. Harding wrote:
> > + /* Only allow 1 open at a time on mgmt device */
> > + if (dgnc_mgmt_in_use[minor]) {
> > + rc = -EBUSY;
> > + goto out;
> > + }
> > + dgnc_mgmt_in_use[minor]++;
> >
> > +out:
>
> Better to use "unlock" instead of "out". "out" is very ambigous.
Noted. Will include in v3.
thanks,
Tobin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] staging: dgnc: audit goto's in dgnc_driver
On Tue, Mar 07, 2017 at 08:03:51PM +0300, Dan Carpenter wrote: > On Tue, Mar 07, 2017 at 05:33:06PM +1100, Tobin C. Harding wrote: > > @@ -419,17 +411,14 @@ static int dgnc_init_one(struct pci_dev *pdev, const > > struct pci_device_id *ent) > > brd->dpastatus = BD_RUNNING; > > > > dgnc_board[dgnc_num_boards++] = brd; > > - > > return 0; > > > > There's nothing wrong with putting a blank before a return 0. The blank > sort of makes it stand out nicely. I thought this one would get a comment :) The reasoning was to be uniform across the whole directory. Originally some returns were preceded by a new line while others were not. I picked one and went for uniformity. Is this level of uniformity too much? Is it better to be less pedantic and have less code churn? thanks, Tobin. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: unisys: fix sparse warnings
On Sat, Mar 04, 2017 at 06:20:50PM +0100, Andrea Ghittino wrote: > Sparse generates two warnings related to incorrect type in assignment. > This patch changes the types in the struct defined in unisys Can you post the Sparse warning? Otherwise when I'm reviewing this code I have to re-run Sparse to see what you're talking about. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: rtl8188eu: wrap macro parameters in parentheses
On Sat, Mar 04, 2017 at 11:05:58PM +0100, Sebastian Haas wrote:
> diff --git a/drivers/staging/rtl8188eu/include/rtl8188e_hal.h
> b/drivers/staging/rtl8188eu/include/rtl8188e_hal.h
> index 9330361..ca9db14 100644
> --- a/drivers/staging/rtl8188eu/include/rtl8188e_hal.h
> +++ b/drivers/staging/rtl8188eu/include/rtl8188e_hal.h
> @@ -146,7 +146,7 @@ struct txpowerinfo24g {
> #define EFUSE_REAL_CONTENT_LEN 512
> #define EFUSE_MAX_SECTION16
> #define EFUSE_IC_ID_OFFSET 506 /* For some inferior IC purpose*/
> -#define AVAILABLE_EFUSE_ADDR(addr) (addr < EFUSE_REAL_CONTENT_LEN)
> +#define AVAILABLE_EFUSE_ADDR(addr) ((addr) < EFUSE_REAL_CONTENT_LEN)
This one is never used. I feel like you're just randomly adding
parenthesis instead of taking your time to think about things.
> --- a/drivers/staging/rtl8188eu/include/rtw_ioctl.h
> +++ b/drivers/staging/rtl8188eu/include/rtw_ioctl.h
> @@ -59,7 +59,7 @@
> #define OID_MP_SEG4 0xFF011100
>
> #define DEBUG_OID(dbg, str) \
> - if ((!dbg)) { \
> + if ((!(dbg))) { \
Ugh...
> #define rtw_get_ips_mode_req(pwrctrlpriv) \
> - (pwrctrlpriv)->ips_mode_req
> + ((pwrctrlpriv)->ips_mode_req)
Do you really think this is an issue? I know I should be in favour of
defensive coding and all that, but I really feel like the focus should
be on real issues, cleaning things up and deleting as much of this code
as we can instead of just adding parenthesis.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 17/29] drivers, pci: convert hv_pci_dev.refs from atomic_t to refcount_t
On Mon, 6 Mar 2017 15:38:29 -0600 Bjorn Helgaas wrote: > [+cc Hyper-V folks, -cc others] > > On Mon, Mar 06, 2017 at 04:21:04PM +0200, Elena Reshetova wrote: > > refcount_t type and corresponding API should be > > used instead of atomic_t when the variable is used as > > a reference counter. This allows to avoid accidental > > refcounter overflows that might lead to use-after-free > > situations. > > > > Signed-off-by: Elena Reshetova > > Signed-off-by: Hans Liljestrand > > Signed-off-by: Kees Cook > > Signed-off-by: David Windsor Reviewed-by: Stephen Hemminger ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] Staging: comedi: comedi_fops: Change comedi_num_legacy_minors type
On Sun, Mar 05, 2017 at 03:22:32AM +0800, Cheah Kok Cheong wrote: > Change to unsigned to allow removal of negative value check in > init section. Why? > Use smaller data type since the max possible value currently is 48. Does it matter? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 10/29] drivers, md: convert stripe_head.count from atomic_t to refcount_t
On Mon, Mar 06, 2017 at 04:20:57PM +0200, Elena Reshetova wrote: > refcount_t type and corresponding API should be > used instead of atomic_t when the variable is used as > a reference counter. This allows to avoid accidental > refcounter overflows that might lead to use-after-free > situations. > > Signed-off-by: Elena Reshetova > Signed-off-by: Hans Liljestrand > Signed-off-by: Kees Cook > Signed-off-by: David Windsor > --- > drivers/md/raid5-cache.c | 8 +++--- > drivers/md/raid5.c | 66 > > drivers/md/raid5.h | 3 ++- > 3 files changed, 39 insertions(+), 38 deletions(-) > > diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c > index 3f307be..6c05e12 100644 > --- a/drivers/md/raid5-cache.c > +++ b/drivers/md/raid5-cache.c snip > sh->check_state, sh->reconstruct_state); > > analyse_stripe(sh, &s); > @@ -4924,7 +4924,7 @@ static void activate_bit_delay(struct r5conf *conf, > struct stripe_head *sh = list_entry(head.next, struct > stripe_head, lru); > int hash; > list_del_init(&sh->lru); > - atomic_inc(&sh->count); > + refcount_inc(&sh->count); > hash = sh->hash_lock_index; > __release_stripe(conf, sh, &temp_inactive_list[hash]); > } > @@ -5240,7 +5240,7 @@ static struct stripe_head *__get_priority_stripe(struct > r5conf *conf, int group) > sh->group = NULL; > } > list_del_init(&sh->lru); > - BUG_ON(atomic_inc_return(&sh->count) != 1); > + BUG_ON(refcount_inc_not_zero(&sh->count)); This changes the behavior. refcount_inc_not_zero doesn't inc if original value is 0 Thanks, Shaohua ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 3/5] pci: set msi_domain_ops as __ro_after_init
On Thu, Feb 16, 2017 at 03:38:05PM +0100, Thomas Gleixner wrote:
> On Thu, 16 Feb 2017, Bjorn Helgaas wrote:
> > On Wed, Feb 15, 2017 at 10:16:32PM +0100, Thomas Gleixner wrote:
> >
> > > I think I suggested to Jiang to do that 'update with default functions' to
> > >
> > > - avoid exporting the world and some more
> > >
> > > - have the flexibility to add new functions to the ops w/o updating a
> > > gazillion of existing usage sites, which has saved us lots of chaising
> > > in
> > > the last years
> > >
> > > - avoid the if (ops->ptr) ops->ptr(); else default_fn(); constructs all
> > > over the place.
> > >
> > > I admit I did not think about the fact that this makes the structs non
> > > const.
> > >
> > > Mopping that up by exporting the default functions and setting all the
> > > function pointers is tedious and requires a full tree sweep when we add
> > > new
> > > stuff. There's also code shared between PCI/platform/DT based stuff, so
> > > that becomes interesting.
> >
> > It's legal to initialize a field multiple times, and the last one
> > takes precedence, so doing this might at least avoid the full tree
> > sweeps:
> >
> > static struct msi_domain_ops vmd_msi_domain_ops = {
> > MSI_DOMAIN_DEFAULT_OPS,
> > .get_hwirq = vmd_get_hwirq,
> > };
> >
> > The functions referenced by MSI_DOMAIN_DEFAULT_OPS would still have to
> > be exported, though.
>
> Hmm, that'd work. Though it will fall apart for those pieces where we share
> code across backends. But I did not yet go through all the places and check
> them.
>
> > > Doing the if (ops->ptr) ops->ptr() else default_fn(); dance should be
> > > simpler to pull off. There are not that many sites to look at, but then we
> > > have some of the GICv3 code using the domain ops out of core.
> > >
> > > For now doing the __ro_after_init is definitely the simplest and fastest
> > > solution to tighten these statically allocated structures.
> >
> > I'm OK with __ro_after_init, at least as an interim solution.
> >
> > I do think it would be good to audit all the uses of
> > MSI_FLAG_USE_DEF_DOM_OPS and MSI_FLAG_USE_DEF_CHIP_OPS, since they
> > seem to be the primary indicator of when the struct might be modified.
> > I suspect we could add __ro_after_init to more than just pci-hyperv.c,
> > vmd.c, and msi.c
>
> Agreed. I have it on my radar.
This seems like a worthwhile change, so I don't want to just drop this
patch. But if we're going to do something, I'd like to do it
everywhere that it makes sense, all at the same time.
It looks like the v2 series was split up by subsystem, which is fine
with me. I'll happily apply the PCI parts (or ack them, since it
might make sense to apply all of them via the same non-PCI tree).
But I *would* like to include the following users of
MSI_FLAG_USE_DEF_DOM_OPS and MSI_FLAG_USE_DEF_CHIP_OPS at the same
time (or explain why __ro_after_init won't work for them):
pci-xgene-msi.c
pcie-altera-msi.c
pcie-iproc-msi.c
pcie-xilinx-nwl.c
Bjorn
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t
On Mon, Mar 06, 2017 at 04:20:55PM +0200, Elena Reshetova wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
Looks good. Let me know how do you want to route the patch to upstream.
> Signed-off-by: Elena Reshetova
> Signed-off-by: Hans Liljestrand
> Signed-off-by: Kees Cook
> Signed-off-by: David Windsor
> ---
> drivers/md/md.c | 6 +++---
> drivers/md/md.h | 3 ++-
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 985374f..94c8ebf 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -449,7 +449,7 @@ EXPORT_SYMBOL(md_unplug);
>
> static inline struct mddev *mddev_get(struct mddev *mddev)
> {
> - atomic_inc(&mddev->active);
> + refcount_inc(&mddev->active);
> return mddev;
> }
>
> @@ -459,7 +459,7 @@ static void mddev_put(struct mddev *mddev)
> {
> struct bio_set *bs = NULL;
>
> - if (!atomic_dec_and_lock(&mddev->active, &all_mddevs_lock))
> + if (!refcount_dec_and_lock(&mddev->active, &all_mddevs_lock))
> return;
> if (!mddev->raid_disks && list_empty(&mddev->disks) &&
> mddev->ctime == 0 && !mddev->hold_active) {
> @@ -495,7 +495,7 @@ void mddev_init(struct mddev *mddev)
> INIT_LIST_HEAD(&mddev->all_mddevs);
> setup_timer(&mddev->safemode_timer, md_safemode_timeout,
> (unsigned long) mddev);
> - atomic_set(&mddev->active, 1);
> + refcount_set(&mddev->active, 1);
> atomic_set(&mddev->openers, 0);
> atomic_set(&mddev->active_io, 0);
> spin_lock_init(&mddev->lock);
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index b8859cb..4811663 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -360,7 +361,7 @@ struct mddev {
>*/
> struct mutexopen_mutex;
> struct mutexreconfig_mutex;
> - atomic_tactive; /* general refcount */
> + refcount_t active; /* general refcount */
> atomic_topeners;/* number of active
> opens */
>
> int changed;/* True if we might
> need to
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: cb_pcidas64: move loop invariant
On Tue, Mar 07, 2017 at 01:29:35PM +1100, Tobin C. Harding wrote: > Loop invariant is inside the loop so code checks invariant on each > iteration of the loop. Invariant can be moved outside of the loop so > it is only checked once. > > Move loop invariant outside of for loop. But does it really matter? Does this fix an issue? Make something faster in a _measurable_ way? Did you test this on the hardware? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/2] Staging: comedi: comedi_fops: Fix "out of minor numbers for board device files"
If comedi module is loaded with the following max allowed parameter
[comedi_num_legacy_minors=48], subsequent loading of an auto-configured
device will fail at auto-configuration. If there's no fall back in
place then module loading will fail.
In this case, a default to auto-configure comedi_test module failed
to auto-configure with the following messages. It loaded but fell back
to unconfigured mode.
comedi_test comedi_testd: ran out of minor numbers for board device files
comedi_test comedi_testd: driver 'comedi_test' could not create device.
comedi_test: unable to auto-configure device
This is due to changes in commit 38b9722a4414
("staging: comedi: avoid releasing legacy minors automatically") which
will not allocate a minor number when comedi_num_legacy_minors equals
COMEDI_NUM_BOARD_MINORS. COMEDI_NUM_BOARD_MINORS is defined to be
0x30 which is 48.
This goes for a simple fix which limit comedi_num_legacy_minors to 47
instead of tinkering with comedi_alloc_board_minor() and
comedi_release_hardware_device().
Fix: commit 38b9722a4414 ("staging: comedi: avoid releasing legacy minors
automatically")
Signed-off-by: Cheah Kok Cheong
---
V2:
-Amend commit log to specify that comedi_test module failed
to auto-configure and fell back to unconfigured mode.
For other devices with no fall back, module loading will fail.
drivers/staging/comedi/comedi_fops.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 354d264..339854f 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2857,9 +2857,9 @@ static int __init comedi_init(void)
pr_info("version " COMEDI_RELEASE " - http://www.comedi.org\n";);
- if (comedi_num_legacy_minors > COMEDI_NUM_BOARD_MINORS) {
+ if (comedi_num_legacy_minors >= COMEDI_NUM_BOARD_MINORS) {
pr_err("invalid value for module parameter
\"comedi_num_legacy_minors\". Valid values are 0 through %i.\n",
- COMEDI_NUM_BOARD_MINORS);
+ COMEDI_NUM_BOARD_MINORS - 1);
return -EINVAL;
}
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/2] Staging: comedi: comedi_fops: Change comedi_num_legacy_minors type
Change to unsigned to allow removal of negative value check in
init section. Use smaller data type since the max possible
value currently is 48.
Signed-off-by: Cheah Kok Cheong
---
V2:
-No changes.
drivers/staging/comedi/comedi_fops.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 57e8599..354d264 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -76,8 +76,8 @@ struct comedi_file {
#define COMEDI_NUM_SUBDEVICE_MINORS\
(COMEDI_NUM_MINORS - COMEDI_NUM_BOARD_MINORS)
-static int comedi_num_legacy_minors;
-module_param(comedi_num_legacy_minors, int, 0444);
+static unsigned short comedi_num_legacy_minors;
+module_param(comedi_num_legacy_minors, ushort, 0444);
MODULE_PARM_DESC(comedi_num_legacy_minors,
"number of comedi minor devices to reserve for
non-auto-configured devices (default 0)"
);
@@ -2857,8 +2857,7 @@ static int __init comedi_init(void)
pr_info("version " COMEDI_RELEASE " - http://www.comedi.org\n";);
- if (comedi_num_legacy_minors < 0 ||
- comedi_num_legacy_minors > COMEDI_NUM_BOARD_MINORS) {
+ if (comedi_num_legacy_minors > COMEDI_NUM_BOARD_MINORS) {
pr_err("invalid value for module parameter
\"comedi_num_legacy_minors\". Valid values are 0 through %i.\n",
COMEDI_NUM_BOARD_MINORS);
return -EINVAL;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging: sm750fb: Alignment should match open parenthesis
On Sun, Mar 05, 2017 at 04:54:50PM +0530, Arushi Singhal wrote: > @@ -147,18 +146,18 @@ static int programModeRegisters(mode_parameter_t > *pModeParam, > PANEL_HORIZONTAL_SYNC_START_MASK)); > > poke32(PANEL_VERTICAL_TOTAL, > - (((pModeParam->vertical_total - 1) << > - PANEL_VERTICAL_TOTAL_TOTAL_SHIFT) & > - PANEL_VERTICAL_TOTAL_TOTAL_MASK) | > - ((pModeParam->vertical_display_end - 1) & > - PANEL_VERTICAL_TOTAL_DISPLAY_END_MASK)); > +(((pModeParam->vertical_total - 1) << > + PANEL_VERTICAL_TOTAL_TOTAL_SHIFT) & > + PANEL_VERTICAL_TOTAL_TOTAL_MASK) | > +((pModeParam->vertical_display_end - 1) & > + ANEL_VERTICAL_TOTAL_DISPLAY_END_MASK)); Missing P in PANEL means this won't compile. > > poke32(PANEL_VERTICAL_SYNC, > - ((pModeParam->vertical_sync_height << > - PANEL_VERTICAL_SYNC_HEIGHT_SHIFT) & > - PANEL_VERTICAL_SYNC_HEIGHT_MASK) | > - ((pModeParam->vertical_sync_start - 1) & > - PANEL_VERTICAL_SYNC_START_MASK)); > +((pModeParam->vertical_sync_height << > + PANEL_VERTICAL_SYNC_HEIGHT_SHIFT) & > + PANEL_VERTICAL_SYNC_HEIGHT_MASK) | > +((pModeParam->vertical_sync_start - 1) & > + PANEL_VERTICAL_SYNC_START_MASK)); > > tmp = DISPLAY_CTRL_TIMING | DISPLAY_CTRL_PLANE; > if (pModeParam->vertical_sync_polarity) regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: sm750fb: fixes add blank line after function/struct/union/enum declarations
On Sun, Mar 05, 2017 at 04:54:48PM +0530, Arushi Singhal wrote:
> This patch fixes the warnings reported by checkpatch.pl
> for please use a blank line after function/struct/union/enum
> declarations.
>
> Signed-off-by: Arushi Singhal
> ---
> drivers/staging/sm750fb/ddk750_display.h | 1 +
> drivers/staging/sm750fb/ddk750_mode.h| 2 ++
> drivers/staging/sm750fb/ddk750_power.h | 1 +
> drivers/staging/sm750fb/sm750_cursor.c | 3 +++
> 4 files changed, 7 insertions(+)
>
> diff --git a/drivers/staging/sm750fb/ddk750_display.h
> b/drivers/staging/sm750fb/ddk750_display.h
> index e2a3f84ca4c5..8bf22e4f0d8b 100644
> --- a/drivers/staging/sm750fb/ddk750_display.h
> +++ b/drivers/staging/sm750fb/ddk750_display.h
> @@ -100,6 +100,7 @@ typedef enum _disp_output_t {
> do_CRT_PRI = CRT_2_PRI | PRI_TP_ON | DPMS_ON | DAC_ON,
> do_CRT_SEC = CRT_2_SEC | SEC_TP_ON | DPMS_ON | DAC_ON,
> }
> +
> disp_output_t;
Nah... This is a typedef and this change makes it even worse. It
should be on the same line as the }.
} disp_output_t;
But actually this typedef is not allowed.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: wlan-ng: hfa384x.h: fixed a newline coding style issue
On Tue, Mar 07, 2017 at 12:36:46PM -0500, Mark Stenglein wrote: > All, > > Thanks for the feedback. Trying to introduce myself and in retrospect > this seems to be a fairly non-productive way to go about it. Apologies > for any time lost. > It's fine. I'm just rushing through patches as fast as possible. It doesn't take any time. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: wlan-ng: hfa384x.h: fixed a newline coding style issue
All,
Thanks for the feedback. Trying to introduce myself and in retrospect
this seems to be a fairly non-productive way to go about it. Apologies
for any time lost.
Best,
Mark Stenglein
On Tue, Mar 07, 2017 at 08:31:13PM +0300, Dan Carpenter wrote:
> On Sun, Mar 05, 2017 at 09:09:12PM -0500, Mark Stenglein wrote:
> > Fixed a coding style issue.
> >
> > Signed-off-by: Mark Stenglein
> > ---
> > drivers/staging/wlan-ng/hfa384x.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/staging/wlan-ng/hfa384x.h
> > b/drivers/staging/wlan-ng/hfa384x.h
> > index 5f1851c85f12..f19984747b1e 100644
> > --- a/drivers/staging/wlan-ng/hfa384x.h
> > +++ b/drivers/staging/wlan-ng/hfa384x.h
> > @@ -1175,6 +1175,7 @@ struct hfa384x_usbctlx {
> > enum ctlx_state state; /* Tracks running state */
> >
> > struct completion done;
> > +
> > volatile int reapable; /* Food for the reaper task */
>
> No blank line needed. The original is OK. (Except for the volatile is
> wrong obviously).
>
> regards,
> dan carpenter
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: rtl8192u: clean up some white space issues
This patch fixes two coding style errors, reported by the checkpatch
script.
Signed-off-by: Elia Geretto
---
drivers/staging/rtl8192u/r8192U_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8192u/r8192U_core.c
b/drivers/staging/rtl8192u/r8192U_core.c
index b631990..b61ffa3 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -269,7 +269,7 @@ int write_nic_byte_E(struct net_device *dev, int indx, u8
data)
indx | 0xfe00, 0, usbdata, 1, HZ / 2);
kfree(usbdata);
- if (status < 0){
+ if (status < 0) {
netdev_err(dev, "write_nic_byte_E TimeOut! status: %d\n",
status);
return status;
@@ -2519,7 +2519,7 @@ static int rtl8192_read_eeprom_info(struct net_device
*dev)
for (i = 0; i < 3; i++) {
if (bLoad_From_EEPOM) {
ret = eprom_read(dev,
(EEPROM_TxPwIndex_OFDM_24G + i) >> 1);
- if ( ret < 0)
+ if (ret < 0)
return ret;
if (((EEPROM_TxPwIndex_OFDM_24G + i) %
2) == 0)
tmpValue = (u16)ret & 0x00ff;
--
2.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: wlan-ng: hfa384x.h: fixed a newline coding style issue
On Sun, Mar 05, 2017 at 09:09:12PM -0500, Mark Stenglein wrote:
> Fixed a coding style issue.
>
> Signed-off-by: Mark Stenglein
> ---
> drivers/staging/wlan-ng/hfa384x.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/wlan-ng/hfa384x.h
> b/drivers/staging/wlan-ng/hfa384x.h
> index 5f1851c85f12..f19984747b1e 100644
> --- a/drivers/staging/wlan-ng/hfa384x.h
> +++ b/drivers/staging/wlan-ng/hfa384x.h
> @@ -1175,6 +1175,7 @@ struct hfa384x_usbctlx {
> enum ctlx_state state; /* Tracks running state */
>
> struct completion done;
> +
> volatile int reapable; /* Food for the reaper task */
No blank line needed. The original is OK. (Except for the volatile is
wrong obviously).
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Fix coding style errors
Resend with a better subject. [PATCH] Staging: rtl8192u: clean up some white space issues regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: Fix sparse warnings incorrect type assignment
On Tue, Mar 07, 2017 at 08:23:04PM +0300, Dan Carpenter wrote: > This commit doesn't tell me if you though about what you are doing at > all. We get so many commits where people just randomly do random endian > things... Did you think about this? Could you resend with a better > commit message? > The commit message should say what the user visible effects of this bug fix are going to be. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: Fix sparse warnings incorrect type assignment
On Mon, Mar 06, 2017 at 10:11:21PM +0100, Andrea Ghittino wrote: > Fixed sparse warnings > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2011:52: warning: > incorrect type in assignment (different base types) > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2011:52:expected > unsigned short [unsigned] [assigned] [usertype] ht_ext_params > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2011:52:got restricted > __le16 const [usertype] extended_ht_cap_info > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2012:51: warning: > incorrect type in assignment (different base types) > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2012:51:expected > unsigned int [unsigned] [assigned] [usertype] ht_tx_bf_cap > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2012:51:got restricted > __le32 const [usertype] tx_BF_cap_info > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2078:51: warning: > incorrect type in assignment (different base types) > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2078:51:expected > unsigned short [unsigned] [assigned] [usertype] ht_capa_info > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2078:51:got restricted > __le16 const [usertype] cap_info > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2083:52: warning: > incorrect type in assignment (different base types) > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2083:52:expected > unsigned short [unsigned] [assigned] [usertype] ht_ext_params > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2083:52:got restricted > __le16 const [usertype] extended_ht_cap_info > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2084:51: warning: > incorrect type in assignment (different base types) > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2084:51:expected > unsigned int [unsigned] [assigned] [usertype] ht_tx_bf_cap > drivers/staging/wilc1000//wilc_wfi_cfgoperations.c:2084:51:got restricted > __le32 const [usertype] tx_BF_cap_info > > > Signed-off-by: Andrea Ghittino > --- > Compile tested only This commit doesn't tell me if you though about what you are doing at all. We get so many commits where people just randomly do random endian things... Did you think about this? Could you resend with a better commit message? regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] staging: dgnc: audit goto's in dgnc_tty
On Tue, Mar 07, 2017 at 05:33:08PM +1100, Tobin C. Harding wrote:
> @@ -1375,7 +1378,7 @@ static int dgnc_tty_chars_in_buffer(struct tty_struct
> *tty)
> ushort thead;
> ushort ttail;
> uint tmask;
> - uint chars = 0;
> + uint chars;
> unsigned long flags;
>
> if (!tty)
> @@ -1397,16 +1400,14 @@ static int dgnc_tty_chars_in_buffer(struct tty_struct
> *tty)
>
> spin_unlock_irqrestore(&ch->ch_lock, flags);
>
> - if (ttail == thead) {
> + if (ttail == thead)
> chars = 0;
> - } else {
> - if (thead >= ttail)
> - chars = thead - ttail;
> - else
> - chars = thead - ttail + WQUEUESIZE;
> - }
> + else if (thead > ttail)
> + chars = thead - ttail;
> + else
> + chars = thead - ttail + WQUEUESIZE;
>
> - return chars;
> + return (int)chars;
This cast means something suspicious is going on. Is there really
something suspicous going on? Can we fix the types? Can we just remove
the cast?
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging: ks7010: add additional goto target
On Tue, Mar 07, 2017 at 09:31:16AM +1100, Tobin C. Harding wrote:
@@ -610,12 +610,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
> if (atomic_read(&priv->psstatus.status) ==
> PS_SNOOZE) {
> if (cnt_txqbody(priv)) {
> ks_wlan_hw_wakeup_request(priv);
> - queue_delayed_work
> - (priv->ks_wlan_hw.
> - ks7010sdio_wq,
> -
> &priv->ks_wlan_hw.
> - rw_wq, 1);
> - return;
> + goto intr_out_with_delay;
The original code is crap but this isn't really an improvement. We've
just used a goto to split the code up randomly so it's hard to read.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] storvsc: workaround for virtual DVD SCSI version
> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Tuesday, March 7, 2017 9:16 AM
> To: KY Srinivasan ; Haiyang Zhang
> ; Long Li ;
> martin.peter...@oracle.com; h...@lst.de; h...@suse.de
> Cc: linux-s...@vger.kernel.org; linux-ker...@vger.kernel.org;
> de...@linuxdriverproject.org; Stephen Hemminger
>
> Subject: [PATCH] storvsc: workaround for virtual DVD SCSI version
>
> Hyper-V host emulation of SCSI for virtual DVD device reports SCSI
> version 0 (UNKNOWN) but is still capable of supporting REPORTLUN.
>
> Without this patch, a GEN2 Linux guest on Hyper-V will not boot 4.11
> successfully with virtual DVD ROM device. What happens is that the
> SCSI scan process falls back to doing sequential probing by INQUIRY.
> But the storvsc driver has a previous workaround that masks/blocks all
> errors reports from INQUIRY (or MODE_SENSE) commands. This workaround
> causes the scan to then populate a full set of bogus LUN's on the
> target and then sends kernel spinning off into a death spiral doing
> block reads on the non-existent LUNs.
>
> By setting the correct blacklist flags, the target with the
> DVD device is scanned with REPORTLUN and that works correctly.
>
> Patch needs to go in current 4.11, it is safe but not necessary
> in older kernels.
>
> Signed-off-by: Stephen Hemminger
Reviewed-by: K. Y. Srinivasan
> ---
> drivers/scsi/storvsc_drv.c | 27 +--
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> PS: The error handling does need to be fixed (have patches pending)
> but that is interrelated with hotplug and can wait.
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 638e5f427c90..19973e874830 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -400,8 +400,6 @@
> MODULE_PARM_DESC(storvsc_vcpus_per_sub_channel, "Ratio of VCPUs to
> subchannels")
> */
> static int storvsc_timeout = 180;
>
> -static int msft_blist_flags = BLIST_TRY_VPD_PAGES;
> -
> #if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
> static struct scsi_transport_template *fc_transport_template;
> #endif
> @@ -1383,6 +1381,22 @@ static int storvsc_do_io(struct hv_device *device,
> return ret;
> }
>
> +static int storvsc_device_alloc(struct scsi_device *sdevice)
> +{
> + /*
> + * Set blist flag to permit the reading of the VPD pages even when
> + * the target may claim SPC-2 compliance. MSFT targets currently
> + * claim SPC-2 compliance while they implement post SPC-2 features.
> + * With this flag we can correctly handle WRITE_SAME_16 issues.
> + *
> + * Hypervisor reports SCSI_UNKNOWN type for DVD ROM device but
> + * still supports REPORT LUN.
> + */
> + sdevice->sdev_bflags = BLIST_REPORTLUN2 |
> BLIST_TRY_VPD_PAGES;
> +
> + return 0;
> +}
> +
> static int storvsc_device_configure(struct scsi_device *sdevice)
> {
>
> @@ -1396,14 +1410,6 @@ static int storvsc_device_configure(struct
> scsi_device *sdevice)
> sdevice->no_write_same = 1;
>
> /*
> - * Add blist flags to permit the reading of the VPD pages even when
> - * the target may claim SPC-2 compliance. MSFT targets currently
> - * claim SPC-2 compliance while they implement post SPC-2 features.
> - * With this patch we can correctly handle WRITE_SAME_16 issues.
> - */
> - sdevice->sdev_bflags |= msft_blist_flags;
> -
> - /*
>* If the host is WIN8 or WIN8 R2, claim conformance to SPC-3
>* if the device is a MSFT virtual device. If the host is
>* WIN10 or newer, allow write_same.
> @@ -1661,6 +1667,7 @@ static struct scsi_host_template scsi_driver = {
> .eh_host_reset_handler =storvsc_host_reset_handler,
> .proc_name ="storvsc_host",
> .eh_timed_out = storvsc_eh_timed_out,
> + .slave_alloc = storvsc_device_alloc,
> .slave_configure = storvsc_device_configure,
> .cmd_per_lun = 255,
> .this_id = -1,
> --
> 2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] storvsc: workaround for virtual DVD SCSI version
Hyper-V host emulation of SCSI for virtual DVD device reports SCSI
version 0 (UNKNOWN) but is still capable of supporting REPORTLUN.
Without this patch, a GEN2 Linux guest on Hyper-V will not boot 4.11
successfully with virtual DVD ROM device. What happens is that the
SCSI scan process falls back to doing sequential probing by INQUIRY.
But the storvsc driver has a previous workaround that masks/blocks all
errors reports from INQUIRY (or MODE_SENSE) commands. This workaround
causes the scan to then populate a full set of bogus LUN's on the
target and then sends kernel spinning off into a death spiral doing
block reads on the non-existent LUNs.
By setting the correct blacklist flags, the target with the
DVD device is scanned with REPORTLUN and that works correctly.
Patch needs to go in current 4.11, it is safe but not necessary
in older kernels.
Signed-off-by: Stephen Hemminger
---
drivers/scsi/storvsc_drv.c | 27 +--
1 file changed, 17 insertions(+), 10 deletions(-)
PS: The error handling does need to be fixed (have patches pending)
but that is interrelated with hotplug and can wait.
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 638e5f427c90..19973e874830 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -400,8 +400,6 @@ MODULE_PARM_DESC(storvsc_vcpus_per_sub_channel, "Ratio of
VCPUs to subchannels")
*/
static int storvsc_timeout = 180;
-static int msft_blist_flags = BLIST_TRY_VPD_PAGES;
-
#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
static struct scsi_transport_template *fc_transport_template;
#endif
@@ -1383,6 +1381,22 @@ static int storvsc_do_io(struct hv_device *device,
return ret;
}
+static int storvsc_device_alloc(struct scsi_device *sdevice)
+{
+ /*
+* Set blist flag to permit the reading of the VPD pages even when
+* the target may claim SPC-2 compliance. MSFT targets currently
+* claim SPC-2 compliance while they implement post SPC-2 features.
+* With this flag we can correctly handle WRITE_SAME_16 issues.
+*
+* Hypervisor reports SCSI_UNKNOWN type for DVD ROM device but
+* still supports REPORT LUN.
+*/
+ sdevice->sdev_bflags = BLIST_REPORTLUN2 | BLIST_TRY_VPD_PAGES;
+
+ return 0;
+}
+
static int storvsc_device_configure(struct scsi_device *sdevice)
{
@@ -1396,14 +1410,6 @@ static int storvsc_device_configure(struct scsi_device
*sdevice)
sdevice->no_write_same = 1;
/*
-* Add blist flags to permit the reading of the VPD pages even when
-* the target may claim SPC-2 compliance. MSFT targets currently
-* claim SPC-2 compliance while they implement post SPC-2 features.
-* With this patch we can correctly handle WRITE_SAME_16 issues.
-*/
- sdevice->sdev_bflags |= msft_blist_flags;
-
- /*
* If the host is WIN8 or WIN8 R2, claim conformance to SPC-3
* if the device is a MSFT virtual device. If the host is
* WIN10 or newer, allow write_same.
@@ -1661,6 +1667,7 @@ static struct scsi_host_template scsi_driver = {
.eh_host_reset_handler =storvsc_host_reset_handler,
.proc_name ="storvsc_host",
.eh_timed_out = storvsc_eh_timed_out,
+ .slave_alloc = storvsc_device_alloc,
.slave_configure = storvsc_device_configure,
.cmd_per_lun = 255,
.this_id = -1,
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192e: fix coding style issue, improve error handling
On Tue, Mar 07, 2017 at 12:39:40PM +0530, Suniel Mahesh wrote: > Fix coding style issue and comments in rtl_core.c > Return -ENOMEM, if it is out of memory > Pointer comparison with NULL replaced by logical NOT > Split it up into 3 commits. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/4] staging: dgnc: remove goto task from TODO list
Better to always wait overnight before sending a v2. ;) regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] staging: dgnc: audit goto's in dgnc_tty
On Tue, Mar 07, 2017 at 05:33:08PM +1100, Tobin C. Harding wrote:
> @@ -1668,20 +1669,20 @@ static int dgnc_tty_tiocmget(struct tty_struct *tty)
> {
> struct channel_t *ch;
> struct un_t *un;
> - int result = -EIO;
> + int rc = -EIO;
> unsigned char mstat = 0;
> unsigned long flags;
>
> if (!tty || tty->magic != TTY_MAGIC)
> - return result;
> + return rc;
>
It's better to just do "return -EIO;" That way all the information is
on one line, and you don't need to scroll back to find out what it's
returning.
> @@ -1987,20 +1990,21 @@ static int dgnc_tty_digigeta(struct tty_struct *tty,
> struct un_t *un;
> struct digi_t tmp;
> unsigned long flags;
> + int rc = -EFAULT;
>
> if (!retinfo)
> - return -EFAULT;
> + return rc;
The original is better here.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: dgnc: audit goto's in dgnc_mgmt
On Tue, Mar 07, 2017 at 05:33:07PM +1100, Tobin C. Harding wrote:
> + /* Only allow 1 open at a time on mgmt device */
> + if (dgnc_mgmt_in_use[minor]) {
> + rc = -EBUSY;
> + goto out;
> + }
> + dgnc_mgmt_in_use[minor]++;
>
> +out:
Better to use "unlock" instead of "out". "out" is very ambigous.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] staging: dgnc: audit goto's in dgnc_driver
On Tue, Mar 07, 2017 at 05:33:06PM +1100, Tobin C. Harding wrote: > @@ -419,17 +411,14 @@ static int dgnc_init_one(struct pci_dev *pdev, const > struct pci_device_id *ent) > brd->dpastatus = BD_RUNNING; > > dgnc_board[dgnc_num_boards++] = brd; > - > return 0; > There's nothing wrong with putting a blank before a return 0. The blank sort of makes it stand out nicely. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[staging:staging-next 261/261] drivers/staging/built-in.o:(.bss+0x109d0): multiple definition of `dbg_level'
Hi Greg, First bad commit (maybe != root cause): tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git staging-next head: 25016567be26887232aa3f6fd0d0116356691cc3 commit: 25016567be26887232aa3f6fd0d0116356691cc3 [261/261] staging: atomisp: fix include Makefile mess config: i386-allyesconfig (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: git checkout 25016567be26887232aa3f6fd0d0116356691cc3 # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): >> drivers/staging/built-in.o:(.bss+0x109d0): multiple definition of `dbg_level' drivers/rapidio/built-in.o:(.bss+0x16c): first defined here --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v2 08/32] x86: Use PAGE_KERNEL protection for ioremap of memory page
On Thu, Mar 02, 2017 at 10:13:32AM -0500, Brijesh Singh wrote:
> From: Tom Lendacky
>
> In order for memory pages to be properly mapped when SEV is active, we
> need to use the PAGE_KERNEL protection attribute as the base protection.
> This will insure that memory mapping of, e.g. ACPI tables, receives the
> proper mapping attributes.
>
> Signed-off-by: Tom Lendacky
> ---
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index c400ab5..481c999 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -151,7 +151,15 @@ static void __iomem *__ioremap_caller(resource_size_t
> phys_addr,
> pcm = new_pcm;
> }
>
> + /*
> +* If the page being mapped is in memory and SEV is active then
> +* make sure the memory encryption attribute is enabled in the
> +* resulting mapping.
> +*/
> prot = PAGE_KERNEL_IO;
> + if (sev_active() && page_is_mem(pfn))
Hmm, a resource tree walk per ioremap call. This could get expensive for
ioremap-heavy workloads.
__ioremap_caller() gets called here during boot 55 times so not a whole
lot but I wouldn't be surprised if there were some nasty use cases which
ioremap a lot.
...
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 9b5f044..db56ba3 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -518,6 +518,46 @@ int __weak page_is_ram(unsigned long pfn)
> }
> EXPORT_SYMBOL_GPL(page_is_ram);
>
> +/*
> + * This function returns true if the target memory is marked as
> + * IORESOURCE_MEM and IORESOUCE_BUSY and described as other than
> + * IORES_DESC_NONE (e.g. IORES_DESC_ACPI_TABLES).
> + */
> +static int walk_mem_range(unsigned long start_pfn, unsigned long nr_pages)
> +{
> + struct resource res;
> + unsigned long pfn, end_pfn;
> + u64 orig_end;
> + int ret = -1;
> +
> + res.start = (u64) start_pfn << PAGE_SHIFT;
> + res.end = ((u64)(start_pfn + nr_pages) << PAGE_SHIFT) - 1;
> + res.flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> + orig_end = res.end;
> + while ((res.start < res.end) &&
> + (find_next_iomem_res(&res, IORES_DESC_NONE, true) >= 0)) {
> + pfn = (res.start + PAGE_SIZE - 1) >> PAGE_SHIFT;
> + end_pfn = (res.end + 1) >> PAGE_SHIFT;
> + if (end_pfn > pfn)
> + ret = (res.desc != IORES_DESC_NONE) ? 1 : 0;
> + if (ret)
> + break;
> + res.start = res.end + 1;
> + res.end = orig_end;
> + }
> + return ret;
> +}
So the relevant difference between this one and walk_system_ram_range()
is this:
- ret = (*func)(pfn, end_pfn - pfn, arg);
+ ret = (res.desc != IORES_DESC_NONE) ? 1 : 0;
so it seems to me you can have your own *func() pointer which does that
IORES_DESC_NONE comparison. And then you can define your own workhorse
__walk_memory_range() which gets called by both walk_mem_range() and
walk_system_ram_range() instead of almost duplicating them.
And looking at walk_system_ram_res(), that one looks similar too except
the pfn computation. But AFAICT the pfn/end_pfn things are computed from
res.start and res.end so it looks to me like all those three functions
are crying for unification...
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284
(AG Nürnberg)
--
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 13/29] drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to refcount_t
> Hi Elena,
>
> On Mon, Mar 06, 2017 at 04:21:00PM +0200, Elena Reshetova wrote:
> > refcount_t type and corresponding API should be
> > used instead of atomic_t when the variable is used as
> > a reference counter. This allows to avoid accidental
> > refcounter overflows that might lead to use-after-free
> > situations.
> >
> > Signed-off-by: Elena Reshetova
> > Signed-off-by: Hans Liljestrand
> > Signed-off-by: Kees Cook
> > Signed-off-by: David Windsor
> > ---
> > drivers/media/v4l2-core/videobuf2-memops.c | 6 +++---
> > include/media/videobuf2-memops.h | 3 ++-
> > 2 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/videobuf2-memops.c
> > b/drivers/media/v4l2-
> core/videobuf2-memops.c
> > index 1cd322e..4bb8424 100644
> > --- a/drivers/media/v4l2-core/videobuf2-memops.c
> > +++ b/drivers/media/v4l2-core/videobuf2-memops.c
> > @@ -96,10 +96,10 @@ static void vb2_common_vm_open(struct
> vm_area_struct *vma)
> > struct vb2_vmarea_handler *h = vma->vm_private_data;
> >
> > pr_debug("%s: %p, refcount: %d, vma: %08lx-%08lx\n",
> > - __func__, h, atomic_read(h->refcount), vma->vm_start,
> > + __func__, h, refcount_read(h->refcount), vma->vm_start,
> >vma->vm_end);
> >
> > - atomic_inc(h->refcount);
> > + refcount_inc(h->refcount);
> > }
> >
> > /**
> > @@ -114,7 +114,7 @@ static void vb2_common_vm_close(struct
> vm_area_struct *vma)
> > struct vb2_vmarea_handler *h = vma->vm_private_data;
> >
> > pr_debug("%s: %p, refcount: %d, vma: %08lx-%08lx\n",
> > - __func__, h, atomic_read(h->refcount), vma->vm_start,
> > + __func__, h, refcount_read(h->refcount), vma->vm_start,
> >vma->vm_end);
> >
> > h->put(h->arg);
> > diff --git a/include/media/videobuf2-memops.h b/include/media/videobuf2-
> memops.h
> > index 36565c7a..a6ed091 100644
> > --- a/include/media/videobuf2-memops.h
> > +++ b/include/media/videobuf2-memops.h
> > @@ -16,6 +16,7 @@
> >
> > #include
> > #include
> > +#include
> >
> > /**
> > * struct vb2_vmarea_handler - common vma refcount tracking handler
> > @@ -25,7 +26,7 @@
> > * @arg: argument for @put callback
> > */
> > struct vb2_vmarea_handler {
> > - atomic_t*refcount;
> > + refcount_t *refcount;
>
> This is a pointer to refcount, not refcount itself. The refcount is part of
> a memory type specific struct, the types that you change in the following
> three patches. I guess it would still compile and work as separate patches
> but you'd sure get warnings at least.
Actually it doesn't compile without this patch if I remember it correctly back
in past when I was initially splitting the patches per variable.
>
> How about merging this and the three following patches that change the memop
> refcount types?
Sounds good!
Best Regards,
Elena.
>
> > void(*put)(void *arg);
> > void*arg;
> > };
>
> --
> Kind regards,
>
> Sakari Ailus
> e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 12/29] drivers, media: convert s2255_dev.num_channels from atomic_t to refcount_t
> Hi Elena, > > On Mon, Mar 06, 2017 at 04:20:59PM +0200, Elena Reshetova wrote: > > refcount_t type and corresponding API should be > > used instead of atomic_t when the variable is used as > > a reference counter. This allows to avoid accidental > > refcounter overflows that might lead to use-after-free > > situations. > > > > Signed-off-by: Elena Reshetova > > Signed-off-by: Hans Liljestrand > > Signed-off-by: Kees Cook > > Signed-off-by: David Windsor > > --- > ... > > @@ -1688,7 +1689,7 @@ static int s2255_probe_v4l(struct s2255_dev *dev) > > "failed to register > video device!\n"); > > break; > > } > > - atomic_inc(&dev->num_channels); > > + refcount_set(&dev->num_channels, 1); > > That's not right. The loop runs four iterations and the value after the > loop should be indeed the number of channels. Oh, yes, I was blind here, sorry. The problem why it cannot be left as inc is because it would do increment from zero here, which is not allowed by refcount_t interface. > atomic_t isn't really used for reference counting here, but storing out how > many "channels" there are per hardware device, with maximum number of four > (4). I'd leave this driver using atomic_t. Yes, sounds like the best thing to do. I will drop this patch. Thank you for reviews! Best Regards, Elena. > > > v4l2_info(&dev->v4l2_dev, "V4L2 device registered > as %s\n", > > video_device_node_name(&vc- > >vdev)); > > > > -- > Kind regards, > > Sakari Ailus > e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] staging: unisys: fix sparse warnings
> -Original Message-
> From: Andrea Ghittino [mailto:aghitt...@gmail.com]
> Sent: Saturday, March 4, 2017 12:21 PM
> To: de...@driverdev.osuosl.org; Kershner, David A
> ; gre...@linuxfoundation.org; *S-Par-
> Maintainer ; linux-ker...@vger.kernel.org
> Subject: [PATCH] staging: unisys: fix sparse warnings
>
> Sparse generates two warnings related to incorrect type in assignment.
> This patch changes the types in the struct defined in unisys
>
> Signed-off-by: Andrea Ghittino
Acked-by: David Kershner
Tested it on s-Par and no problems.
> ---
> Compile tested only
>
> iochannel.h |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/unisys/include/iochannel.h
> b/drivers/staging/unisys/include/iochannel.h
> index 54f4900..41e5b4e 100644
> --- a/drivers/staging/unisys/include/iochannel.h
> +++ b/drivers/staging/unisys/include/iochannel.h
> @@ -308,8 +308,8 @@ struct net_pkt_xmt {
> u8 valid; /* 1 = struct is valid - else ignore */
> u8 hrawoffv;/* 1 = hwrafoff is valid */
> u8 nhrawoffv; /* 1 = nhwrafoff is valid */
> - u16 protocol; /* specifies packet protocol */
> - u32 csum; /* value used to set skb->csum at IOPart */
> + __be16 protocol;/* specifies packet protocol */
> + __wsum csum;/* value used to set skb->csum at IOPart */
> u32 hrawoff;/* value used to set skb->h.raw at IOPart */
> /* hrawoff points to the start of the TRANSPORT LAYER
> HEADER */
> u32 nhrawoff; /* value used to set skb->nh.raw at IOPart */
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v2 07/32] x86/efi: Access EFI data as encrypted when SEV is active
On Thu, Mar 02, 2017 at 10:13:21AM -0500, Brijesh Singh wrote:
> From: Tom Lendacky
>
> EFI data is encrypted when the kernel is run under SEV. Update the
> page table references to be sure the EFI memory areas are accessed
> encrypted.
>
> Signed-off-by: Tom Lendacky
> Signed-off-by: Brijesh Singh
This SOB chain looks good.
> ---
> arch/x86/platform/efi/efi_64.c | 15 ++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> index 2d8674d..9a76ed8 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -45,6 +45,7 @@
> #include
> #include
> #include
> +#include
>
> /*
> * We allocate runtime services regions bottom-up, starting from -4G, i.e.
> @@ -286,7 +287,10 @@ int __init efi_setup_page_tables(unsigned long
> pa_memmap, unsigned num_pages)
>* as trim_bios_range() will reserve the first page and isolate it away
>* from memory allocators anyway.
>*/
> - if (kernel_map_pages_in_pgd(pgd, 0x0, 0x0, 1, _PAGE_RW)) {
> + pf = _PAGE_RW;
> + if (sev_active())
> + pf |= _PAGE_ENC;
> + if (kernel_map_pages_in_pgd(pgd, 0x0, 0x0, 1, pf)) {
> pr_err("Failed to create 1:1 mapping for the first page!\n");
> return 1;
> }
> @@ -329,6 +333,9 @@ static void __init __map_region(efi_memory_desc_t *md,
> u64 va)
> if (!(md->attribute & EFI_MEMORY_WB))
> flags |= _PAGE_PCD;
>
> + if (sev_active())
> + flags |= _PAGE_ENC;
> +
So I'm wondering if we could avoid this sprinkling of _PAGE_ENC in the
EFI code by defining something like __supported_pte_mask but called
__efi_base_page_flags or so which has _PAGE_ENC cleared in the SME case,
i.e., when baremetal and has it set in the SEV case.
Then we could simply OR in __efi_base_page_flags which the SME/SEV code
will set appropriately early enough.
Hmm.
Matt, what do you think?
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284
(AG Nürnberg)
--
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v2 02/32] x86: Secure Encrypted Virtualization (SEV) support
On Thu, Mar 02, 2017 at 10:12:20AM -0500, Brijesh Singh wrote: > From: Tom Lendacky > > Provide support for Secure Encyrpted Virtualization (SEV). This initial > support defines a flag that is used by the kernel to determine if it is > running with SEV active. > > Signed-off-by: Tom Lendacky Btw, you need to add your Signed-off-by here after Tom's to denote that you're handing that patch forward. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v2 05/32] x86: Use encrypted access of BOOT related data with SEV
On Thu, Mar 02, 2017 at 10:12:59AM -0500, Brijesh Singh wrote: > From: Tom Lendacky > > When Secure Encrypted Virtualization (SEV) is active, BOOT data (such as > EFI related data, setup data) is encrypted and needs to be accessed as > such when mapped. Update the architecture override in early_memremap to > keep the encryption attribute when mapping this data. This could also explain why persistent memory needs to be accessed decrypted with SEV. In general, what the difference in that aspect is in respect to SME. And I'd write that in the comment over the function. And not say "E820 areas are checked in making this determination." because that is visible but say *why* we need to check those ranges and determine access depending on their type. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 11/29] drivers, media: convert cx88_core.refcount from atomic_t to refcount_t
On 3/7/2017 10:52 AM, Reshetova, Elena wrote:
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
[...]
diff --git a/drivers/media/pci/cx88/cx88.h b/drivers/media/pci/cx88/cx88.h
index 115414c..16c1313 100644
--- a/drivers/media/pci/cx88/cx88.h
+++ b/drivers/media/pci/cx88/cx88.h
[...]
@@ -339,7 +340,7 @@ struct cx8802_dev;
struct cx88_core {
struct list_head devlist;
- atomic_t refcount;
+ refcount_t refcount;
Could you please keep the name aligned with above and below?
You mean "not aligned" to devlist, but with a shift like it was before?
I mean aligned, like it was before. :-)
Sure, will fix. Is the patch ok otherwise?
I haven't noticed anything else...
Best Regards,
Elena.
[...]
MBR, Sergei
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe
On Mar 6, 2017, at 08:20, James Simmons wrote:
>
>>
>> Doubly linked lists which are iterated using list_empty
>> and list_entry macros have been replaced with list_for_each_entry_safe
>> macro.
>> This makes the iteration simpler and more readable.
>>
>> This patch replaces the while loop containing list_empty and list_entry
>> with list_for_each_entry_safe.
>>
>> This was done with Coccinelle.
>>
>> @@
>> expression E1;
>> identifier I1, I2;
>> type T;
>> iterator name list_for_each_entry_safe;
>> @@
>>
>> T *I1;
>> + T *tmp;
>> ...
>> - while (list_empty(&E1) == 0)
>> + list_for_each_entry_safe (I1, tmp, &E1, I2)
>> {
>> ...when != T *I1;
>> - I1 = list_entry(E1.next, T, I2);
>> ...
>> }
>>
>> Signed-off-by: simran singhal
>
> NAK!!
>
> This change was reverted in commit
>
> cd15dd6ef4ea11df87f717b8b1b83aaa738ec8af
>
> Doing these while (list_empty(..)) to list_for_entry...
> are not simple changes and have broken things in lustre
> before. Unless you really understand the state machine of
> the lustre code I don't recommend these kinds of change
> for lustre.
It may be useful to add a comment to these cases where the while() loop cannot
be
replaced by list_for_each_entry_safe() (with details of why that is the case) to
avoid such optimization attempts again in the future.
Cheers, Andreas
>
>> ---
>> drivers/staging/lustre/lustre/osc/osc_page.c | 11 ---
>> 1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/staging/lustre/lustre/osc/osc_page.c
>> b/drivers/staging/lustre/lustre/osc/osc_page.c
>> index ed8a0dc..e8b974f 100644
>> --- a/drivers/staging/lustre/lustre/osc/osc_page.c
>> +++ b/drivers/staging/lustre/lustre/osc/osc_page.c
>> @@ -542,6 +542,7 @@ long osc_lru_shrink(const struct lu_env *env, struct
>> client_obd *cli,
>> struct cl_object *clobj = NULL;
>> struct cl_page **pvec;
>> struct osc_page *opg;
>> +struct osc_page *tmp;
>> int maxscan = 0;
>> long count = 0;
>> int index = 0;
>> @@ -572,7 +573,7 @@ long osc_lru_shrink(const struct lu_env *env, struct
>> client_obd *cli,
>> if (force)
>> cli->cl_lru_reclaim++;
>> maxscan = min(target << 1, atomic_long_read(&cli->cl_lru_in_list));
>> -while (!list_empty(&cli->cl_lru_list)) {
>> +list_for_each_entry_safe(opg, tmp, &cli->cl_lru_list, ops_lru) {
>> struct cl_page *page;
>> bool will_free = false;
>>
>> @@ -582,8 +583,6 @@ long osc_lru_shrink(const struct lu_env *env, struct
>> client_obd *cli,
>> if (--maxscan < 0)
>> break;
>>
>> -opg = list_entry(cli->cl_lru_list.next, struct osc_page,
>> - ops_lru);
>> page = opg->ops_cl.cpl_page;
>> if (lru_page_busy(cli, page)) {
>> list_move_tail(&opg->ops_lru, &cli->cl_lru_list);
>> @@ -1043,6 +1042,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker
>> *sk,
>> {
>> struct client_obd *stop_anchor = NULL;
>> struct client_obd *cli;
>> +struct client_obd *tmp;
>> struct lu_env *env;
>> long shrank = 0;
>> u16 refcheck;
>> @@ -1059,10 +1059,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker
>> *sk,
>> return SHRINK_STOP;
>>
>> spin_lock(&osc_shrink_lock);
>> -while (!list_empty(&osc_shrink_list)) {
>> -cli = list_entry(osc_shrink_list.next, struct client_obd,
>> - cl_shrink_list);
>> -
>> +list_for_each_entry_safe(cli, tmp, &osc_shrink_list, cl_shrink_list) {
>> if (!stop_anchor)
>> stop_anchor = cli;
>> else if (cli == stop_anchor)
>> --
>> 2.7.4
Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel Corporation
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/4] staging: dgnc: audit goto's in dgnc_tty
TODO file requests fix up of error handling.
Audit dgnc_mgmt.c and fix all return paths to be uniform and inline
with kernel coding style.
Signed-off-by: Tobin C. Harding
---
drivers/staging/dgnc/dgnc_tty.c | 219
1 file changed, 112 insertions(+), 107 deletions(-)
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index c3b8fc5..4398ace 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -143,7 +143,6 @@ int dgnc_tty_register(struct dgnc_board *brd)
TTY_DRIVER_REAL_RAW |
TTY_DRIVER_DYNAMIC_DEV |
TTY_DRIVER_HARDWARE_BREAK);
-
if (IS_ERR(brd->serial_driver))
return PTR_ERR(brd->serial_driver);
@@ -181,7 +180,6 @@ int dgnc_tty_register(struct dgnc_board *brd)
TTY_DRIVER_REAL_RAW |
TTY_DRIVER_DYNAMIC_DEV |
TTY_DRIVER_HARDWARE_BREAK);
-
if (IS_ERR(brd->print_driver)) {
rc = PTR_ERR(brd->print_driver);
goto unregister_serial_driver;
@@ -220,7 +218,6 @@ int dgnc_tty_register(struct dgnc_board *brd)
tty_unregister_driver(brd->serial_driver);
free_serial_driver:
put_tty_driver(brd->serial_driver);
-
return rc;
}
@@ -241,6 +238,7 @@ void dgnc_tty_unregister(struct dgnc_board *brd)
int dgnc_tty_init(struct dgnc_board *brd)
{
int i;
+ int rc;
void __iomem *vaddr;
struct channel_t *ch;
@@ -260,8 +258,10 @@ int dgnc_tty_init(struct dgnc_board *brd)
*/
brd->channels[i] = kzalloc(sizeof(*brd->channels[i]),
GFP_KERNEL);
- if (!brd->channels[i])
+ if (!brd->channels[i]) {
+ rc = -ENOMEM;
goto err_free_channels;
+ }
}
ch = brd->channels[0];
@@ -319,7 +319,7 @@ int dgnc_tty_init(struct dgnc_board *brd)
kfree(brd->channels[i]);
brd->channels[i] = NULL;
}
- return -ENOMEM;
+ return rc;
}
/*
@@ -914,7 +914,6 @@ static int dgnc_tty_open(struct tty_struct *tty, struct
file *file)
*/
rc = wait_event_interruptible(brd->state_wait,
(brd->state & BOARD_READY));
-
if (rc)
return rc;
@@ -922,14 +921,14 @@ static int dgnc_tty_open(struct tty_struct *tty, struct
file *file)
/* If opened device is greater than our number of ports, bail. */
if (PORT_NUM(minor) >= brd->nasync) {
- spin_unlock_irqrestore(&brd->bd_lock, flags);
- return -ENXIO;
+ rc = -ENXIO;
+ goto err_brd_unlock;
}
ch = brd->channels[PORT_NUM(minor)];
if (!ch) {
- spin_unlock_irqrestore(&brd->bd_lock, flags);
- return -ENXIO;
+ rc = -ENXIO;
+ goto err_brd_unlock;
}
/* Drop board lock */
@@ -946,8 +945,8 @@ static int dgnc_tty_open(struct tty_struct *tty, struct
file *file)
un = &brd->channels[PORT_NUM(minor)]->ch_pun;
un->un_type = DGNC_PRINT;
} else {
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- return -ENXIO;
+ rc = -ENXIO;
+ goto err_ch_unlock;
}
/*
@@ -959,7 +958,6 @@ static int dgnc_tty_open(struct tty_struct *tty, struct
file *file)
rc = wait_event_interruptible(ch->ch_flags_wait,
((ch->ch_flags & CH_OPENING) == 0));
-
/* If ret is non-zero, user ctrl-c'ed us */
if (rc)
return -EINTR;
@@ -975,7 +973,6 @@ static int dgnc_tty_open(struct tty_struct *tty, struct
file *file)
ch->ch_flags_wait,
(((ch->ch_tun.un_flags |
ch->ch_pun.un_flags) & UN_CLOSING) == 0));
-
/* If ret is non-zero, user ctrl-c'ed us */
if (rc)
return -EINTR;
@@ -1014,7 +1011,6 @@ static int dgnc_tty_open(struct tty_struct *tty, struct
file *file)
kfree(ch->ch_rqueue);
kfree(ch->ch_equeue);
kfree(ch->ch_wqueue);
-
return -ENOMEM;
}
@@ -1082,6 +1078,14 @@ static int dgnc_tty_open(struct tty_struct *tty, struct
file *file)
spin_unlock_irqrestore(&ch->ch_lock, flags);
return rc;
+
+err_brd_unlock:
+ spin_unlock_irqrestore(&brd->bd_lock, flags);
+ return rc;
+err_ch_unlock:
+ spin_unlock_irqrestore(&ch->ch_lock, flags);
+ return rc;
+
}
/*
@@ -1093,7 +1097,7 @@ static int dgnc_blo
[PATCH v2 4/4] staging: dgnc: remove item from TODO list
TODO file contains task to verify and correct function return sites. Need to check for and implement correct usage of goto's when there is work to be done before returning. Remove task from TODO list after already having completed audit of directory drivers/staging/dgnc. Signed-off-by: Tobin C. Harding --- drivers/staging/dgnc/TODO | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/dgnc/TODO b/drivers/staging/dgnc/TODO index e26d1d6..d3806be 100644 --- a/drivers/staging/dgnc/TODO +++ b/drivers/staging/dgnc/TODO @@ -1,7 +1,6 @@ * remove unnecessary comments * remove unnecessary error messages. Example kzalloc() has its own error message. Adding an extra one is useless. -* use goto statements for error handling when appropriate * there is a lot of unnecessary code in the driver. It was originally a standalone driver. Remove unneeded code. -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/4] staging: dgnc: audit goto's in dgnc_mgmt
TODO file requests fix up of error handling.
Audit dgnc_mgmt.c and fix all return paths to be uniform and inline
with kernel coding style.
Signed-off-by: Tobin C. Harding
---
drivers/staging/dgnc/dgnc_mgmt.c | 37 -
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/dgnc/dgnc_mgmt.c b/drivers/staging/dgnc/dgnc_mgmt.c
index 9d9b15d..9e984eb 100644
--- a/drivers/staging/dgnc/dgnc_mgmt.c
+++ b/drivers/staging/dgnc/dgnc_mgmt.c
@@ -42,25 +42,25 @@ int dgnc_mgmt_open(struct inode *inode, struct file *file)
{
unsigned long flags;
unsigned int minor = iminor(inode);
+ int rc = 0;
spin_lock_irqsave(&dgnc_global_lock, flags);
/* mgmt device */
- if (minor < MAXMGMTDEVICES) {
- /* Only allow 1 open at a time on mgmt device */
- if (dgnc_mgmt_in_use[minor]) {
- spin_unlock_irqrestore(&dgnc_global_lock, flags);
- return -EBUSY;
- }
- dgnc_mgmt_in_use[minor]++;
- } else {
- spin_unlock_irqrestore(&dgnc_global_lock, flags);
- return -ENXIO;
+ if (minor >= MAXMGMTDEVICES) {
+ rc = -ENXIO;
+ goto out;
}
+ /* Only allow 1 open at a time on mgmt device */
+ if (dgnc_mgmt_in_use[minor]) {
+ rc = -EBUSY;
+ goto out;
+ }
+ dgnc_mgmt_in_use[minor]++;
+out:
spin_unlock_irqrestore(&dgnc_global_lock, flags);
-
- return 0;
+ return rc;
}
/*
@@ -72,17 +72,20 @@ int dgnc_mgmt_close(struct inode *inode, struct file *file)
{
unsigned long flags;
unsigned int minor = iminor(inode);
+ int rc = 0;
spin_lock_irqsave(&dgnc_global_lock, flags);
/* mgmt device */
- if (minor < MAXMGMTDEVICES) {
- if (dgnc_mgmt_in_use[minor])
- dgnc_mgmt_in_use[minor] = 0;
+ if (minor >= MAXMGMTDEVICES) {
+ rc = -ENXIO;
+ goto out;
}
- spin_unlock_irqrestore(&dgnc_global_lock, flags);
+ dgnc_mgmt_in_use[minor] = 0;
- return 0;
+out:
+ spin_unlock_irqrestore(&dgnc_global_lock, flags);
+ return rc;
}
/*
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/4] staging: dgnc: remove goto task from TODO list
TODO file contains task to verify and correct function return sites. Need to check for and implement correct usage of goto's when there is work to be done before returning. Patch series is broken up by file to ease review. Perhaps a single patch would have been more appropriate. Changes to dgnc_mgmt are more stylistic than all other changes. Perhaps if review ok's those changes patch set could be rolled into one patch. Happy to do so if deemed appropriate. In order to make all return sites uniform the following format was chosen 1. Use variable name 'rc' throughout. 2. No space after function call when checking return value rc = fn(foo); if (rc) return rc; 3. If multiple returns occur at start of function with same error code, define and declare rc in single statement and return rc int rc = -ENOMEM; ... if (conditional) return rc; ... if (other-conditional) return rc; ... v1 -> v2: - remove out of place white space change from patch 01 of series Tobin C. Harding (4): staging: dgnc: audit goto's in dgnc_driver staging: dgnc: audit goto's in dgnc_mgmt staging: dgnc: audit goto's in dgnc_tty staging: dgnc: remove item from TODO list drivers/staging/dgnc/TODO | 1 - drivers/staging/dgnc/dgnc_driver.c | 23 +--- drivers/staging/dgnc/dgnc_mgmt.c | 37 --- drivers/staging/dgnc/dgnc_tty.c| 219 +++-- 4 files changed, 137 insertions(+), 143 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/4] staging: dgnc: audit goto's in dgnc_driver
TODO file requests fix up of error handling.
Audit dgnc_driver.c and fix all return paths to be uniform and inline
with kernel coding style.
Signed-off-by: Tobin C. Harding
---
v1 -> v2:
- remove out of place white space change
drivers/staging/dgnc/dgnc_driver.c | 23 +--
1 file changed, 5 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/dgnc/dgnc_driver.c
b/drivers/staging/dgnc/dgnc_driver.c
index 5381dbd..e970d2d 100644
--- a/drivers/staging/dgnc/dgnc_driver.c
+++ b/drivers/staging/dgnc/dgnc_driver.c
@@ -98,13 +98,11 @@ static const struct board_id dgnc_ids[] = {
static int dgnc_do_remap(struct dgnc_board *brd)
{
- int rc = 0;
-
brd->re_map_membase = ioremap(brd->membase, 0x1000);
if (!brd->re_map_membase)
- rc = -ENOMEM;
+ return -ENOMEM;
- return rc;
+ return 0;
}
/*
@@ -198,7 +196,6 @@ static struct dgnc_board *dgnc_found_board(struct pci_dev
*pdev, int id)
brd->bd_dividend = 921600;
rc = dgnc_do_remap(brd);
-
if (rc < 0)
goto failed;
@@ -283,27 +280,23 @@ static struct dgnc_board *dgnc_found_board(struct pci_dev
*pdev, int id)
failed:
kfree(brd);
-
return ERR_PTR(rc);
}
static int dgnc_request_irq(struct dgnc_board *brd)
{
- int rc = 0;
-
if (brd->irq) {
- rc = request_irq(brd->irq, brd->bd_ops->intr,
+ int rc = request_irq(brd->irq, brd->bd_ops->intr,
IRQF_SHARED, "DGNC", brd);
-
if (rc) {
dev_err(&brd->pdev->dev,
"Failed to hook IRQ %d\n", brd->irq);
brd->state = BOARD_FAILED;
brd->dpastatus = BD_NOFEP;
- rc = -ENODEV;
+ return -ENODEV;
}
}
- return rc;
+ return 0;
}
static void dgnc_free_irq(struct dgnc_board *brd)
@@ -387,7 +380,6 @@ static int dgnc_init_one(struct pci_dev *pdev, const struct
pci_device_id *ent)
/* wake up and enable device */
rc = pci_enable_device(pdev);
-
if (rc)
return -EIO;
@@ -419,17 +411,14 @@ static int dgnc_init_one(struct pci_dev *pdev, const
struct pci_device_id *ent)
brd->dpastatus = BD_RUNNING;
dgnc_board[dgnc_num_boards++] = brd;
-
return 0;
free_irq:
dgnc_free_irq(brd);
unregister_tty:
dgnc_tty_unregister(brd);
-
failed:
kfree(brd);
-
return rc;
}
@@ -488,7 +477,6 @@ static int dgnc_start(void)
spin_unlock_irqrestore(&dgnc_poll_lock, flags);
add_timer(&dgnc_poll_timer);
-
return 0;
failed_device:
@@ -597,7 +585,6 @@ static int __init dgnc_init_module(void)
/* Initialize global stuff */
rc = dgnc_start();
-
if (rc < 0)
return rc;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] staging: dgnc: audit goto's in dgnc_driver
On Tue, Mar 07, 2017 at 10:28:17AM +0100, Greg Kroah-Hartman wrote:
> On Tue, Mar 07, 2017 at 07:57:40PM +1100, Tobin C. Harding wrote:
> > On Tue, Mar 07, 2017 at 09:42:53AM +0100, Greg Kroah-Hartman wrote:
> > > On Tue, Mar 07, 2017 at 05:33:06PM +1100, Tobin C. Harding wrote:
> > > > TODO file requests fix up of error handling.
> > > >
> > > > Audit dgnc_driver.c and fix all return paths to be uniform and inline
> > > > with kernel coding style.
> > > >
> > > > Signed-off-by: Tobin C. Harding
> > > > ---
> > > > drivers/staging/dgnc/dgnc_driver.c | 27 +++
> > > > 1 file changed, 7 insertions(+), 20 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/dgnc/dgnc_driver.c
> > > > b/drivers/staging/dgnc/dgnc_driver.c
> > > > index 5381dbd..8075fff 100644
> > > > --- a/drivers/staging/dgnc/dgnc_driver.c
> > > > +++ b/drivers/staging/dgnc/dgnc_driver.c
> > > > @@ -98,13 +98,11 @@ static const struct board_id dgnc_ids[] = {
> > > >
> > > > static int dgnc_do_remap(struct dgnc_board *brd)
> > > > {
> > > > - int rc = 0;
> > > > -
> > > > brd->re_map_membase = ioremap(brd->membase, 0x1000);
> > > > if (!brd->re_map_membase)
> > > > - rc = -ENOMEM;
> > > > + return -ENOMEM;
> > > >
> > > > - return rc;
> > > > + return 0;
> > > > }
> > > >
> > > > /*
> > > > @@ -198,7 +196,6 @@ static struct dgnc_board *dgnc_found_board(struct
> > > > pci_dev *pdev, int id)
> > > > brd->bd_dividend = 921600;
> > > >
> > > > rc = dgnc_do_remap(brd);
> > > > -
> > > > if (rc < 0)
> > > > goto failed;
> > > >
> > > > @@ -283,27 +280,23 @@ static struct dgnc_board *dgnc_found_board(struct
> > > > pci_dev *pdev, int id)
> > > >
> > > > failed:
> > > > kfree(brd);
> > > > -
> > > > return ERR_PTR(rc);
> > > > }
> > > >
> > > > static int dgnc_request_irq(struct dgnc_board *brd)
> > > > {
> > > > - int rc = 0;
> > > > -
> > > > if (brd->irq) {
> > > > - rc = request_irq(brd->irq, brd->bd_ops->intr,
> > > > + int rc = request_irq(brd->irq, brd->bd_ops->intr,
> > > > IRQF_SHARED, "DGNC", brd);
> > > > -
> > > > if (rc) {
> > > > - dev_err(&brd->pdev->dev,
> > > > - "Failed to hook IRQ %d\n", brd->irq);
> > > > + dev_err(&brd->pdev->dev, "Failed to hook IRQ
> > > > %d\n",
> > > > + brd->irq);
> > >
> > > Why change these two lines?
> >
> > change based on information in Documentation/process/coding-style.rst
> >
> > ...Descendants are always substantially shorter than the parent and
> > are placed substantially to the right...
> >
> > Is my interpretation wrong? I read that as meaning the amendment in question
> > was preferable to the original?
> >
> > Or is it a case that neither way is definitively better so better not
> > to touch it?
>
> It's fine to change it, as it is something that could be done, BUT not
> as part of a patch that is doing something totally different, right?
>
> Remember, only one thing at a time please.
Oh, of course. Too trigger happy. thanks for pointing it out. Will re-submit.
thanks,
Tobin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] staging: dgnc: audit goto's in dgnc_driver
On Tue, Mar 07, 2017 at 07:57:40PM +1100, Tobin C. Harding wrote:
> On Tue, Mar 07, 2017 at 09:42:53AM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Mar 07, 2017 at 05:33:06PM +1100, Tobin C. Harding wrote:
> > > TODO file requests fix up of error handling.
> > >
> > > Audit dgnc_driver.c and fix all return paths to be uniform and inline
> > > with kernel coding style.
> > >
> > > Signed-off-by: Tobin C. Harding
> > > ---
> > > drivers/staging/dgnc/dgnc_driver.c | 27 +++
> > > 1 file changed, 7 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/drivers/staging/dgnc/dgnc_driver.c
> > > b/drivers/staging/dgnc/dgnc_driver.c
> > > index 5381dbd..8075fff 100644
> > > --- a/drivers/staging/dgnc/dgnc_driver.c
> > > +++ b/drivers/staging/dgnc/dgnc_driver.c
> > > @@ -98,13 +98,11 @@ static const struct board_id dgnc_ids[] = {
> > >
> > > static int dgnc_do_remap(struct dgnc_board *brd)
> > > {
> > > - int rc = 0;
> > > -
> > > brd->re_map_membase = ioremap(brd->membase, 0x1000);
> > > if (!brd->re_map_membase)
> > > - rc = -ENOMEM;
> > > + return -ENOMEM;
> > >
> > > - return rc;
> > > + return 0;
> > > }
> > >
> > > /*
> > > @@ -198,7 +196,6 @@ static struct dgnc_board *dgnc_found_board(struct
> > > pci_dev *pdev, int id)
> > > brd->bd_dividend = 921600;
> > >
> > > rc = dgnc_do_remap(brd);
> > > -
> > > if (rc < 0)
> > > goto failed;
> > >
> > > @@ -283,27 +280,23 @@ static struct dgnc_board *dgnc_found_board(struct
> > > pci_dev *pdev, int id)
> > >
> > > failed:
> > > kfree(brd);
> > > -
> > > return ERR_PTR(rc);
> > > }
> > >
> > > static int dgnc_request_irq(struct dgnc_board *brd)
> > > {
> > > - int rc = 0;
> > > -
> > > if (brd->irq) {
> > > - rc = request_irq(brd->irq, brd->bd_ops->intr,
> > > + int rc = request_irq(brd->irq, brd->bd_ops->intr,
> > >IRQF_SHARED, "DGNC", brd);
> > > -
> > > if (rc) {
> > > - dev_err(&brd->pdev->dev,
> > > - "Failed to hook IRQ %d\n", brd->irq);
> > > + dev_err(&brd->pdev->dev, "Failed to hook IRQ %d\n",
> > > + brd->irq);
> >
> > Why change these two lines?
>
> change based on information in Documentation/process/coding-style.rst
>
> ...Descendants are always substantially shorter than the parent and
> are placed substantially to the right...
>
> Is my interpretation wrong? I read that as meaning the amendment in question
> was preferable to the original?
>
> Or is it a case that neither way is definitively better so better not
> to touch it?
It's fine to change it, as it is something that could be done, BUT not
as part of a patch that is doing something totally different, right?
Remember, only one thing at a time please.
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: ks7010: remove one level of indentation
Hi Tobin,
[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.11-rc1]
[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/Tobin-C-Harding/clean-up-ks_sdio_interrupt/20170307-162643
config: x86_64-randconfig-x018-201710 (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 errors (new ones prefixed by >>):
In file included from include/asm-generic/int-ll64.h:10:0,
from include/uapi/asm-generic/types.h:6,
from arch/x86/include/uapi/asm/types.h:4,
from include/uapi/linux/types.h:4,
from include/linux/types.h:5,
from include/linux/firmware.h:4,
from drivers/staging/ks7010/ks7010_sdio.c:13:
>> include/uapi/asm-generic/int-ll64.h:19:1: error: expected '=', ',', ';',
>> 'asm' or '__attribute__' before 'typedef'
typedef __signed__ char __s8;
^~~
In file included from include/linux/firmware.h:4:0,
from drivers/staging/ks7010/ks7010_sdio.c:13:
>> include/linux/types.h:98:10: error: unknown type name '__s8'
typedef __s8 int8_t;
^~~~
In file included from include/linux/quota.h:42:0,
from include/linux/fs.h:214,
from include/linux/net.h:28,
from include/linux/skbuff.h:29,
from include/linux/if_ether.h:23,
from include/uapi/linux/ethtool.h:18,
from include/linux/ethtool.h:17,
from include/linux/netdevice.h:42,
from drivers/staging/ks7010/ks_wlan.h:24,
from drivers/staging/ks7010/ks7010_sdio.c:19:
>> include/uapi/linux/dqblk_xfs.h:53:2: error: unknown type name '__s8'
__s8 d_version; /* version of this structure */
^~~~
include/uapi/linux/dqblk_xfs.h:54:2: error: unknown type name '__s8'
__s8 d_flags; /* FS_{USER,PROJ,GROUP}_QUOTA */
^~~~
include/uapi/linux/dqblk_xfs.h:155:2: error: unknown type name '__s8'
__s8 qs_version; /* version number for future changes */
^~~~
include/uapi/linux/dqblk_xfs.h:157:2: error: unknown type name '__s8'
__s8 qs_pad; /* unused */
^~~~
include/uapi/linux/dqblk_xfs.h:199:2: error: unknown type name '__s8'
__s8 qs_version; /* version for future changes */
^~~~
In file included from include/linux/ethtool.h:17:0,
from include/linux/netdevice.h:42,
from drivers/staging/ks7010/ks_wlan.h:24,
from drivers/staging/ks7010/ks7010_sdio.c:19:
>> include/uapi/linux/ethtool.h:1756:2: error: unknown type name '__s8'
__s8 link_mode_masks_nwords;
^~~~
In file included from include/linux/netdevice.h:53:0,
from drivers/staging/ks7010/ks_wlan.h:24,
from drivers/staging/ks7010/ks7010_sdio.c:19:
>> include/uapi/linux/if_bonding.h:106:2: error: unknown type name '__s8'
__s8 link;
^~~~
include/uapi/linux/if_bonding.h:107:2: error: unknown type name '__s8'
__s8 state;
^~~~
vim +19 include/uapi/asm-generic/int-ll64.h
8a1ab315 David Howells 2012-10-04 3 *
8a1ab315 David Howells 2012-10-04 4 * Integer declarations for
architectures which use "long long"
8a1ab315 David Howells 2012-10-04 5 * for 64-bit types.
8a1ab315 David Howells 2012-10-04 6 */
8a1ab315 David Howells 2012-10-04 7
8a1ab315 David Howells 2012-10-04 8 #ifndef _UAPI_ASM_GENERIC_INT_LL64_H
8a1ab315 David Howells 2012-10-04 9 #define _UAPI_ASM_GENERIC_INT_LL64_H
8a1ab315 David Howells 2012-10-04 10
8a1ab315 David Howells 2012-10-04 11 #include
8a1ab315 David Howells 2012-10-04 12
8a1ab315 David Howells 2012-10-04 13 #ifndef __ASSEMBLY__
8a1ab315 David Howells 2012-10-04 14 /*
8a1ab315 David Howells 2012-10-04 15 * __xx is ok: it doesn't pollute the
POSIX namespace. Use these in the
8a1ab315 David Howells 2012-10-04 16 * header files exported to user space
8a1ab315 David Howells 2012-10-04 17 */
8a1ab315 David Howells 2012-10-04 18
8a1ab315 David Howells 2012-10-04 @19 typedef __signed__ char __s8;
8a1ab315 David Howells 2012-10-04 20 typedef unsigned char __u8;
8a1ab315 David Howells 2012-10-04 21
8a1ab315 David Howells 2012-10-04 22 typedef __signed__ short __s16;
8a1ab315 David Howells 2012-10-04 23 typedef unsigned short __u16;
8a1ab315 David Howells 2012-10-04 24
8a1ab315 David Howells 2012-10-04 25 typedef __signed__ int __s32;
8a1ab315 David Howells 2012-10-04 26 typedef un
Re: [PATCH 1/4] staging: dgnc: audit goto's in dgnc_driver
On Tue, Mar 07, 2017 at 09:42:53AM +0100, Greg Kroah-Hartman wrote:
> On Tue, Mar 07, 2017 at 05:33:06PM +1100, Tobin C. Harding wrote:
> > TODO file requests fix up of error handling.
> >
> > Audit dgnc_driver.c and fix all return paths to be uniform and inline
> > with kernel coding style.
> >
> > Signed-off-by: Tobin C. Harding
> > ---
> > drivers/staging/dgnc/dgnc_driver.c | 27 +++
> > 1 file changed, 7 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/staging/dgnc/dgnc_driver.c
> > b/drivers/staging/dgnc/dgnc_driver.c
> > index 5381dbd..8075fff 100644
> > --- a/drivers/staging/dgnc/dgnc_driver.c
> > +++ b/drivers/staging/dgnc/dgnc_driver.c
> > @@ -98,13 +98,11 @@ static const struct board_id dgnc_ids[] = {
> >
> > static int dgnc_do_remap(struct dgnc_board *brd)
> > {
> > - int rc = 0;
> > -
> > brd->re_map_membase = ioremap(brd->membase, 0x1000);
> > if (!brd->re_map_membase)
> > - rc = -ENOMEM;
> > + return -ENOMEM;
> >
> > - return rc;
> > + return 0;
> > }
> >
> > /*
> > @@ -198,7 +196,6 @@ static struct dgnc_board *dgnc_found_board(struct
> > pci_dev *pdev, int id)
> > brd->bd_dividend = 921600;
> >
> > rc = dgnc_do_remap(brd);
> > -
> > if (rc < 0)
> > goto failed;
> >
> > @@ -283,27 +280,23 @@ static struct dgnc_board *dgnc_found_board(struct
> > pci_dev *pdev, int id)
> >
> > failed:
> > kfree(brd);
> > -
> > return ERR_PTR(rc);
> > }
> >
> > static int dgnc_request_irq(struct dgnc_board *brd)
> > {
> > - int rc = 0;
> > -
> > if (brd->irq) {
> > - rc = request_irq(brd->irq, brd->bd_ops->intr,
> > + int rc = request_irq(brd->irq, brd->bd_ops->intr,
> > IRQF_SHARED, "DGNC", brd);
> > -
> > if (rc) {
> > - dev_err(&brd->pdev->dev,
> > - "Failed to hook IRQ %d\n", brd->irq);
> > + dev_err(&brd->pdev->dev, "Failed to hook IRQ %d\n",
> > + brd->irq);
>
> Why change these two lines?
change based on information in Documentation/process/coding-style.rst
...Descendants are always substantially shorter than the parent and
are placed substantially to the right...
Is my interpretation wrong? I read that as meaning the amendment in question
was preferable to the original?
Or is it a case that neither way is definitively better so better not
to touch it?
thanks for the review,
Tobin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 13/29] drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to refcount_t
Hi Elena,
On Mon, Mar 06, 2017 at 04:21:00PM +0200, Elena Reshetova wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
>
> Signed-off-by: Elena Reshetova
> Signed-off-by: Hans Liljestrand
> Signed-off-by: Kees Cook
> Signed-off-by: David Windsor
> ---
> drivers/media/v4l2-core/videobuf2-memops.c | 6 +++---
> include/media/videobuf2-memops.h | 3 ++-
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-memops.c
> b/drivers/media/v4l2-core/videobuf2-memops.c
> index 1cd322e..4bb8424 100644
> --- a/drivers/media/v4l2-core/videobuf2-memops.c
> +++ b/drivers/media/v4l2-core/videobuf2-memops.c
> @@ -96,10 +96,10 @@ static void vb2_common_vm_open(struct vm_area_struct *vma)
> struct vb2_vmarea_handler *h = vma->vm_private_data;
>
> pr_debug("%s: %p, refcount: %d, vma: %08lx-%08lx\n",
> -__func__, h, atomic_read(h->refcount), vma->vm_start,
> +__func__, h, refcount_read(h->refcount), vma->vm_start,
> vma->vm_end);
>
> - atomic_inc(h->refcount);
> + refcount_inc(h->refcount);
> }
>
> /**
> @@ -114,7 +114,7 @@ static void vb2_common_vm_close(struct vm_area_struct
> *vma)
> struct vb2_vmarea_handler *h = vma->vm_private_data;
>
> pr_debug("%s: %p, refcount: %d, vma: %08lx-%08lx\n",
> -__func__, h, atomic_read(h->refcount), vma->vm_start,
> +__func__, h, refcount_read(h->refcount), vma->vm_start,
> vma->vm_end);
>
> h->put(h->arg);
> diff --git a/include/media/videobuf2-memops.h
> b/include/media/videobuf2-memops.h
> index 36565c7a..a6ed091 100644
> --- a/include/media/videobuf2-memops.h
> +++ b/include/media/videobuf2-memops.h
> @@ -16,6 +16,7 @@
>
> #include
> #include
> +#include
>
> /**
> * struct vb2_vmarea_handler - common vma refcount tracking handler
> @@ -25,7 +26,7 @@
> * @arg: argument for @put callback
> */
> struct vb2_vmarea_handler {
> - atomic_t*refcount;
> + refcount_t *refcount;
This is a pointer to refcount, not refcount itself. The refcount is part of
a memory type specific struct, the types that you change in the following
three patches. I guess it would still compile and work as separate patches
but you'd sure get warnings at least.
How about merging this and the three following patches that change the memop
refcount types?
> void(*put)(void *arg);
> void*arg;
> };
--
Kind regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] staging: dgnc: audit goto's in dgnc_driver
On Tue, Mar 07, 2017 at 05:33:06PM +1100, Tobin C. Harding wrote:
> TODO file requests fix up of error handling.
>
> Audit dgnc_driver.c and fix all return paths to be uniform and inline
> with kernel coding style.
>
> Signed-off-by: Tobin C. Harding
> ---
> drivers/staging/dgnc/dgnc_driver.c | 27 +++
> 1 file changed, 7 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/staging/dgnc/dgnc_driver.c
> b/drivers/staging/dgnc/dgnc_driver.c
> index 5381dbd..8075fff 100644
> --- a/drivers/staging/dgnc/dgnc_driver.c
> +++ b/drivers/staging/dgnc/dgnc_driver.c
> @@ -98,13 +98,11 @@ static const struct board_id dgnc_ids[] = {
>
> static int dgnc_do_remap(struct dgnc_board *brd)
> {
> - int rc = 0;
> -
> brd->re_map_membase = ioremap(brd->membase, 0x1000);
> if (!brd->re_map_membase)
> - rc = -ENOMEM;
> + return -ENOMEM;
>
> - return rc;
> + return 0;
> }
>
> /*
> @@ -198,7 +196,6 @@ static struct dgnc_board *dgnc_found_board(struct pci_dev
> *pdev, int id)
> brd->bd_dividend = 921600;
>
> rc = dgnc_do_remap(brd);
> -
> if (rc < 0)
> goto failed;
>
> @@ -283,27 +280,23 @@ static struct dgnc_board *dgnc_found_board(struct
> pci_dev *pdev, int id)
>
> failed:
> kfree(brd);
> -
> return ERR_PTR(rc);
> }
>
> static int dgnc_request_irq(struct dgnc_board *brd)
> {
> - int rc = 0;
> -
> if (brd->irq) {
> - rc = request_irq(brd->irq, brd->bd_ops->intr,
> + int rc = request_irq(brd->irq, brd->bd_ops->intr,
>IRQF_SHARED, "DGNC", brd);
> -
> if (rc) {
> - dev_err(&brd->pdev->dev,
> - "Failed to hook IRQ %d\n", brd->irq);
> + dev_err(&brd->pdev->dev, "Failed to hook IRQ %d\n",
> + brd->irq);
Why change these two lines?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 12/29] drivers, media: convert s2255_dev.num_channels from atomic_t to refcount_t
Hi Elena, On Mon, Mar 06, 2017 at 04:20:59PM +0200, Elena Reshetova wrote: > refcount_t type and corresponding API should be > used instead of atomic_t when the variable is used as > a reference counter. This allows to avoid accidental > refcounter overflows that might lead to use-after-free > situations. > > Signed-off-by: Elena Reshetova > Signed-off-by: Hans Liljestrand > Signed-off-by: Kees Cook > Signed-off-by: David Windsor > --- ... > @@ -1688,7 +1689,7 @@ static int s2255_probe_v4l(struct s2255_dev *dev) > "failed to register video device!\n"); > break; > } > - atomic_inc(&dev->num_channels); > + refcount_set(&dev->num_channels, 1); That's not right. The loop runs four iterations and the value after the loop should be indeed the number of channels. atomic_t isn't really used for reference counting here, but storing out how many "channels" there are per hardware device, with maximum number of four (4). I'd leave this driver using atomic_t. > v4l2_info(&dev->v4l2_dev, "V4L2 device registered as %s\n", > video_device_node_name(&vc->vdev)); > -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 11/29] drivers, media: convert cx88_core.refcount from atomic_t to refcount_t
On Mon, Mar 06, 2017 at 04:20:58PM +0200, Elena Reshetova wrote: > refcount_t type and corresponding API should be > used instead of atomic_t when the variable is used as > a reference counter. This allows to avoid accidental > refcounter overflows that might lead to use-after-free > situations. > > Signed-off-by: Elena Reshetova > Signed-off-by: Hans Liljestrand > Signed-off-by: Kees Cook > Signed-off-by: David Windsor Acked-by: Sakari Ailus -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
