Re: [patch] staging: speakup: safely close tty

2017-07-11 Thread Okash Khawaja 
Hi,

On Mon, Jul 10, 2017 at 12:55:44PM +0100, Alan Cox wrote:
> On Fri, 7 Jul 2017 20:13:01 +0100
> Okash Khawaja  wrote:
> 
> > Speakup opens tty using tty_open_by_driver. When closing, it calls
> > tty_ldisc_release but doesn't close and remove the tty itself. As a
> > result, that tty cannot then be opened from user space. This patch calls
> > tty_release_struct which ensures that tty is safely removed and freed
> > up. It also calls tty_ldisc_release, so speakup doesn't need to call it.
> > 
> > This patch also unregisters N_SPEAKUP. It is registered when a speakup
> > module is loaded.
> 
> What happens if after you register it someone assigns that ldisc to
> another tty as well ?
> 
> You should register the ldisc when the relevant module is initialized and
> release it only when the module is unloaded. That way the module ref
> counts will handle cases where someone uses the ldisc with something else.
Sorry if I misunderstood it. That's what we do here.
spk_ttyio_initialise_ldisc is called separately for each module (e.g.
speakup_apollo, speakup_ltlk etc) when it is loaded. spk_ttyio_release
is also called separately for each module when it is unloaded. The ldisc
stays around until the last of the modules is unloaded.

> 
> I'd also btw strongly recommend putting the ldisc and the speakup tty
> driver as different modules.
Sure, that makes sense. I will do that following these patches.

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

[PATCH] staging: bcm2835-audio: remove more than 80 char error

2017-07-11 Thread Karuna Grewal 
remove the more than 80 character error as pointed out by checkpatch by
spliting the statements at the separators in the statements .

Signed-off-by: Karuna Grewal 
---
 .../vc04_services/bcm2835-audio/bcm2835-ctl.c  | 53 ++
 1 file changed, 35 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c 
b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
index 30d310b..8277c6f 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
@@ -107,7 +107,7 @@ static int snd_bcm2835_ctl_get(struct snd_kcontrol 
*kcontrol,
 }
 
 static int snd_bcm2835_ctl_put(struct snd_kcontrol *kcontrol,
-   struct snd_ctl_elem_value *ucontrol)
+  struct snd_ctl_elem_value *ucontrol)
 {
struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol);
int changed = 0;
@@ -116,14 +116,19 @@ static int snd_bcm2835_ctl_put(struct snd_kcontrol 
*kcontrol,
return -EINTR;
 
if (kcontrol->private_value == PCM_PLAYBACK_VOLUME) {
-   audio_info("Volume change attempted.. volume = %d new_volume = 
%d\n", chip->volume, (int)ucontrol->value.integer.value[0]);
+   audio_info(
+   "Volume change attempted.. volume = %d new_volume = 
%d\n",
+   chip->volume, (int)ucontrol->value.integer.value[0]);
if (chip->mute == CTRL_VOL_MUTE) {
-   /* changed = toggle_mute(chip, CTRL_VOL_UNMUTE); */
-   changed = 1; /* should return 0 to signify no change 
but the mixer takes this as the opposite sign (no idea why) */
+   /* changed = toggle_mute(chip, CTRL_VOL_UNMUTE);*/
+   /* should return 0*/
+   changed = 1; //mixers takes opposite sign that's why 1
goto unlock;
}
-   if (changed || (ucontrol->value.integer.value[0] != 
chip2alsa(chip->volume))) {
-   chip->volume = 
alsa2chip(ucontrol->value.integer.value[0]);
+   if (changed || (ucontrol->value.integer.value[0] != chip2alsa(
+   chip->volume))) {
+   chip->volume =
+   alsa2chip(ucontrol->value.integer.value[0]);
changed = 1;
}
 
@@ -154,7 +159,9 @@ static struct snd_kcontrol_new snd_bcm2835_ctl[] = {
.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
.name = "PCM Playback Volume",
.index = 0,
-   .access = SNDRV_CTL_ELEM_ACCESS_READWRITE | 
SNDRV_CTL_ELEM_ACCESS_TLV_READ,
+   .access =
+SNDRV_CTL_ELEM_ACCESS_READWRITE |
+ SNDRV_CTL_ELEM_ACCESS_TLV_READ,
.private_value = PCM_PLAYBACK_VOLUME,
.info = snd_bcm2835_ctl_info,
.get = snd_bcm2835_ctl_get,
@@ -186,15 +193,17 @@ static struct snd_kcontrol_new snd_bcm2835_ctl[] = {
},
 };
 
-static int snd_bcm2835_spdif_default_info(struct snd_kcontrol *kcontrol,
-   struct snd_ctl_elem_info *uinfo)
+static int snd_bcm2835_spdif_default_info(
+   struct snd_kcontrol *kcontrol,
+   struct snd_ctl_elem_info *uinfo)
 {
uinfo->type = SNDRV_CTL_ELEM_TYPE_IEC958;
uinfo->count = 1;
return 0;
 }
 
-static int snd_bcm2835_spdif_default_get(struct snd_kcontrol *kcontrol,
+static int snd_bcm2835_spdif_default_get(
+   struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)
 {
struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol);
@@ -211,7 +220,8 @@ static int snd_bcm2835_spdif_default_get(struct 
snd_kcontrol *kcontrol,
return 0;
 }
 
-static int snd_bcm2835_spdif_default_put(struct snd_kcontrol *kcontrol,
+static int snd_bcm2835_spdif_default_put(
+   struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)
 {
struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol);
@@ -222,7 +232,8 @@ static int snd_bcm2835_spdif_default_put(struct 
snd_kcontrol *kcontrol,
return -EINTR;
 
for (i = 0; i < 4; i++)
-   val |= (unsigned int)ucontrol->value.iec958.status[i] << (i * 
8);
+   val |=
+(unsigned int)ucontrol->value.iec958.status[i] << (i * 8);
 
change = val != chip->spdif_status;
chip->spdif_status = val;
@@ -231,7 +242,8 @@ static int snd_bcm2835_spdif_default_put(struct 
snd_kcontrol *kcontrol,
return change;
 }
 
-static int snd_bcm2835_spdif_mask_info(struct snd_kcontrol *kcontrol,
+static int snd_bcm2835_spdif_mask_info(
+   struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo)
 {
uinfo->type = SNDRV_CTL_ELEM_TYPE_IEC958;
@@ -239,7 +251,8 @@ static int snd_bcm2835_spdif_mask_info(struct

Re: [PATCH 00/10] Fix alignment issues in staging/ccree

2017-07-11 Thread Gilad Ben-Yossef 
On Mon, Jul 3, 2017 at 3:28 PM, Simon Sandström  wrote:
> On Mon, Jul 03, 2017 at 10:19:31AM +0300, Gilad Ben-Yossef wrote:
>> but for the few cases where its a complex expression that can be
>> broken down like this one:
>>
>> WARNING: line over 80 characters
>> #93: FILE: drivers/staging/ccree/ssi_buffer_mgr.c:437:
>> + (AES_BLOCK_SIZE + areq_ctx->ccm_hdr_size),
>>
>> It would be great if you fix those.
>
> Do you mean to fix them by just breaking the line after the ternary
> operator? Is that OK according to the code style rules?

Yes, it is.

>
> If not, then the two warnings in ssi_aead_handle_config_buf can be
> fixed by simply having a local variable:
>
> const unsigned int buflen = AES_BLOCK_SIZE + areq_ctx->ccm_hdr_size;
>
> but I'm not sure if the same can be done in
> ssi_buffer_mgr_map_hash_request_update for (update_data_len - *curr_buf_cnt)
> as it's not always the case that update_data_len is greater than
> *curr_buf_cnt. Even if it's safe to always calculate it I'm not
> entierly sure what to call the variable.

No need not. I simply meant to break this:

 (update_data_len - *curr_buf_cnt)

To this:

 (update_data_len -
   *curr_buf_cnt)

Assuming it made sense.

> Other then those two functions I don't think that there are any more
> lines that can be fixed without renaming functions / variables or
> by rewriting them to decrease the indentation depth, as you wrote.

No, not in this patch.

Thanks!
Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Stagingdriver cctree: Fix for checkpatch warning

2017-07-11 Thread Gilad Ben-Yossef 
Hello Philip,

Thank your patch.

Your patch subject line is not descriptive and not formatted well.

A better subject would be something like:

staging: ccree: move comment to fit coding style

Thanks,
Gilad

On Fri, Jun 30, 2017 at 7:32 AM,   wrote:
> From: Bincy K Philip 
>
> Trivial fix for Line over 80 characters
>
> Moved the comment to top of the definition
>
> Signed-off-by: Bincy K Philip 
> ---
>  drivers/staging/ccree/cc_hw_queue_defs.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/ccree/cc_hw_queue_defs.h 
> b/drivers/staging/ccree/cc_hw_queue_defs.h
> index aaa56c8..a18e6c9 100644
> --- a/drivers/staging/ccree/cc_hw_queue_defs.h
> +++ b/drivers/staging/ccree/cc_hw_queue_defs.h
> @@ -27,7 +27,8 @@
>  
> **/
>
>  #define HW_DESC_SIZE_WORDS 6
> -#define HW_QUEUE_SLOTS_MAX  15 /* Max. available slots in HW 
> queue */
> +/* Define max. available slots in HW queue */
> +#define HW_QUEUE_SLOTS_MAX  15
>
>  #define CC_REG_NAME(word, name) DX_DSCRPTR_QUEUE_WORD ## word ## _ ## name
>
> --
> 1.8.3.1
>



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: bcm2835-audio: remove more than 80 char error

2017-07-11 Thread Dan Carpenter 
On Tue, Jul 11, 2017 at 04:59:14PM +0530, Karuna Grewal wrote:
> remove the more than 80 character error as pointed out by checkpatch by
> spliting the statements at the separators in the statements .
> 

Don't indent your commit log.

> - if (changed || (ucontrol->value.integer.value[0] != 
> chip2alsa(chip->volume))) {
> - chip->volume = 
> alsa2chip(ucontrol->value.integer.value[0]);
> + if (changed || (ucontrol->value.integer.value[0] != chip2alsa(
> + chip->volume))) {
> + chip->volume =
> + alsa2chip(ucontrol->value.integer.value[0]);

Don't indent like that.

> @@ -154,7 +159,9 @@ static struct snd_kcontrol_new snd_bcm2835_ctl[] = {
>   .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
>   .name = "PCM Playback Volume",
>   .index = 0,
> - .access = SNDRV_CTL_ELEM_ACCESS_READWRITE | 
> SNDRV_CTL_ELEM_ACCESS_TLV_READ,
> + .access =
> +  SNDRV_CTL_ELEM_ACCESS_READWRITE |
> +   SNDRV_CTL_ELEM_ACCESS_TLV_READ,


Of this.  Just go over 80 characters if that's more readable than
breaking the line up.

regards,
dan carpenter

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

Re: [PATCH] staging: bcm2835-audio: remove more than 80 char error

2017-07-11 Thread Joe Perches 
On Tue, 2017-07-11 at 16:59 +0530, Karuna Grewal wrote:
> remove the more than 80 character error as pointed out by checkpatch by
> spliting the statements at the separators in the statements .

Not every 80 column warning needs fixing.

Your selection of separators is poor.  For instance:

> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c 
> b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
[]
> @@ -116,14 +116,19 @@ static int snd_bcm2835_ctl_put(struct snd_kcontrol 
> *kcontrol,
>   return -EINTR;
>  
>   if (kcontrol->private_value == PCM_PLAYBACK_VOLUME) {
> - audio_info("Volume change attempted.. volume = %d new_volume = 
> %d\n", chip->volume, (int)ucontrol->value.integer.value[0]);
> + audio_info(
> + "Volume change attempted.. volume = %d new_volume = 
> %d\n",
> + chip->volume, (int)ucontrol->value.integer.value[0]);

Better:

audio_info("Volume change attempted.. volume = %d new_volume = 
%d\n",
   chip->volume, (int)ucontrol->value.integer.value[0]);

[]

> - if (changed || (ucontrol->value.integer.value[0] != 
> chip2alsa(chip->volume))) {
> - chip->volume = 
> alsa2chip(ucontrol->value.integer.value[0]);
> + if (changed || (ucontrol->value.integer.value[0] != chip2alsa(
> + chip->volume))) {
> + chip->volume =
> + alsa2chip(ucontrol->value.integer.value[0]);

Better:
if (changed ||
ucontrol->value.integer.value[0] != chip2alsa(chip->volume) 
{
chip->volume = 
alsa2chip(ucontrol->value.integer.value[0]);
changed = 1;
}

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

Re: [PATCH] staging: bcm2835-audio: replace BUG_ON with WARN_ON

2017-07-11 Thread Greg KH 
On Tue, Jul 11, 2017 at 04:54:24PM +0530, karuna grewal wrote:
> replace BUG_ON with WARN_ON as pointed out by checkpatch
> 
> Signed-off-by: Karuna Grewal 

I can not accept patches sent in html format...

> ---
>  drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c b/
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
> index 5ff4b1c..30d310b 100644
> --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
> +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
> @@ -93,7 +93,7 @@ static int snd_bcm2835_ctl_get(struct snd_kcontrol 
> *kcontrol,
>         if (mutex_lock_interruptible(&chip->audio_mutex))
>                 return -EINTR;
> 
> -       BUG_ON(!chip && !(chip->avail_substreams & AVAIL_SUBSTREAMS_MASK));
> +       WARN_ON(!chip && !(chip->avail_substreams & AVAIL_SUBSTREAMS_MASK));

Why will this make things better?

thanks,

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

[PATCH v2] [media] staging/imx: remove confusing IS_ERR_OR_NULL usage

2017-07-11 Thread Arnd Bergmann 
While looking at a compiler warning, I noticed the use of
IS_ERR_OR_NULL, which is generally a sign of a bad API design
and should be avoided.

In this driver, this is fairly easy, we can simply stop storing
error pointers in persistent structures, and change the two
functions that might return either a NULL pointer or an error
code to consistently return error pointers when failing.

of_parse_subdev() now separates the error code and the pointer
it looks up, to clarify the interface. There are two cases
where this function originally returns 'NULL', and I have
changed that to '0' for success to keep the current behavior,
though returning an error would also make sense there.

Fixes: e130291212df ("[media] media: Add i.MX media core driver")
Signed-off-by: Arnd Bergmann 
---
v2: fix type mismatch
v3: rework of_parse_subdev() as well.
---
 drivers/staging/media/imx/imx-ic-prpencvf.c | 41 ---
 drivers/staging/media/imx/imx-media-csi.c   | 30 ++---
 drivers/staging/media/imx/imx-media-dev.c   |  4 +--
 drivers/staging/media/imx/imx-media-of.c| 50 -
 drivers/staging/media/imx/imx-media-vdic.c  | 37 +++--
 5 files changed, 90 insertions(+), 72 deletions(-)

diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c 
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index ed363fe3b3d0..7a9d9f32f989 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -134,19 +134,19 @@ static inline struct prp_priv *sd_to_priv(struct 
v4l2_subdev *sd)
 
 static void prp_put_ipu_resources(struct prp_priv *priv)
 {
-   if (!IS_ERR_OR_NULL(priv->ic))
+   if (priv->ic)
ipu_ic_put(priv->ic);
priv->ic = NULL;
 
-   if (!IS_ERR_OR_NULL(priv->out_ch))
+   if (priv->out_ch)
ipu_idmac_put(priv->out_ch);
priv->out_ch = NULL;
 
-   if (!IS_ERR_OR_NULL(priv->rot_in_ch))
+   if (priv->rot_in_ch)
ipu_idmac_put(priv->rot_in_ch);
priv->rot_in_ch = NULL;
 
-   if (!IS_ERR_OR_NULL(priv->rot_out_ch))
+   if (priv->rot_out_ch)
ipu_idmac_put(priv->rot_out_ch);
priv->rot_out_ch = NULL;
 }
@@ -154,43 +154,46 @@ static void prp_put_ipu_resources(struct prp_priv *priv)
 static int prp_get_ipu_resources(struct prp_priv *priv)
 {
struct imx_ic_priv *ic_priv = priv->ic_priv;
+   struct ipu_ic *ic;
+   struct ipuv3_channel *out_ch, *rot_in_ch, *rot_out_ch;
int ret, task = ic_priv->task_id;
 
priv->ipu = priv->md->ipu[ic_priv->ipu_id];
 
-   priv->ic = ipu_ic_get(priv->ipu, task);
-   if (IS_ERR(priv->ic)) {
+   ic = ipu_ic_get(priv->ipu, task);
+   if (IS_ERR(ic)) {
v4l2_err(&ic_priv->sd, "failed to get IC\n");
-   ret = PTR_ERR(priv->ic);
+   ret = PTR_ERR(ic);
goto out;
}
+   priv->ic = ic;
 
-   priv->out_ch = ipu_idmac_get(priv->ipu,
-prp_channel[task].out_ch);
-   if (IS_ERR(priv->out_ch)) {
+   out_ch = ipu_idmac_get(priv->ipu, prp_channel[task].out_ch);
+   if (IS_ERR(out_ch)) {
v4l2_err(&ic_priv->sd, "could not get IDMAC channel %u\n",
 prp_channel[task].out_ch);
-   ret = PTR_ERR(priv->out_ch);
+   ret = PTR_ERR(out_ch);
goto out;
}
+   priv->out_ch = out_ch;
 
-   priv->rot_in_ch = ipu_idmac_get(priv->ipu,
-   prp_channel[task].rot_in_ch);
-   if (IS_ERR(priv->rot_in_ch)) {
+   rot_in_ch = ipu_idmac_get(priv->ipu, prp_channel[task].rot_in_ch);
+   if (IS_ERR(rot_in_ch)) {
v4l2_err(&ic_priv->sd, "could not get IDMAC channel %u\n",
 prp_channel[task].rot_in_ch);
-   ret = PTR_ERR(priv->rot_in_ch);
+   ret = PTR_ERR(rot_in_ch);
goto out;
}
+   priv->rot_in_ch = rot_in_ch;
 
-   priv->rot_out_ch = ipu_idmac_get(priv->ipu,
-prp_channel[task].rot_out_ch);
-   if (IS_ERR(priv->rot_out_ch)) {
+   rot_out_ch = ipu_idmac_get(priv->ipu, prp_channel[task].rot_out_ch);
+   if (IS_ERR(rot_out_ch)) {
v4l2_err(&ic_priv->sd, "could not get IDMAC channel %u\n",
 prp_channel[task].rot_out_ch);
-   ret = PTR_ERR(priv->rot_out_ch);
+   ret = PTR_ERR(rot_out_ch);
goto out;
}
+   priv->rot_out_ch = rot_out_ch;
 
return 0;
 out:
diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index a2d26693912e..17fd1e61dd5d 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -122,11 +122,11 @@ static inline struct csi_priv *sd_to_dev(struct 
v4l2_subdev *sdev)
 
 static void csi_i

Re: [PATCH] [media] staging/imx: remove confusing IS_ERR_OR_NULL usage

2017-07-11 Thread Arnd Bergmann 
On Thu, Jun 29, 2017 at 11:13 AM, Philipp Zabel  wrote:

>> @@ -134,23 +134,26 @@ static void csi_idmac_put_ipu_resources(struct 
>> csi_priv *priv)
>>  static int csi_idmac_get_ipu_resources(struct csi_priv *priv)
>>  {
>>   int ch_num, ret;
>> + struct ipu_smfc *smfc, *idmac_ch;
>
> This should be
>
> +   struct ipuv3_channel *idmac_ch;
> +   struct ipu_smfc *smfc;
>
> instead.

Fixed in v2 now.

>
> ... this changes behaviour:
>
> imx-media: imx_media_of_parse failed with -17
> imx-media: probe of capture-subsystem failed with error -17
>
> We must continue to return NULL here if imxsd == -EEXIST:
>
> -   return imxsd;
> +   return PTR_ERR(imxsd) == -EEXIST ? NULL : imxsd;
>
> or change the code where of_parse_subdev is called (from
> imx_media_of_parse, and recursively from of_parse_subdev) to not handle
> the -EEXIST return value as an error.
>
> With those fixed,
>
> Reviewed-by: Philipp Zabel 
> Tested-by: Philipp Zabel 

I thought about it some more and tried to find a better solution for this
function, which is now a bit different, so I did not add your tags.

Can you have another look at v2? This time, of_parse_subdev separates
the return code from the pointer, which seems less confusing in a function
like that. There are in fact two cases where we return NULL and it's
not clear if the caller should treat that as success or failure. I've left
the current behavior the same but added comments there.

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

Re: [PATCH 0/5] staging: ccree: fix checkpatch errors

2017-07-11 Thread Gilad Ben-Yossef 
On Mon, Jul 10, 2017 at 12:10 AM,   wrote:
> From: Tyler Olivieri 
>
> This patchset fixes several checkpatch errors and warnings in /staging/ccree:
>
> ERROR: that open brace { should be on the previous line
> ERROR: open brace '{' following function declarations go on the next line
> WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
> ERROR: do not use assignment in if condition
> ERROR: switch and case should be at the same indent
> WARNING: Statements terminations use 1 semicolon
>
> This is also a submission for the eudyptula challenge.
>
> Tyler Olivieri (5):
>   staging: ccree: remove redudant semicolons
>   staging: ccree: fix placement of curly braces
>   staging: ccree: remove assignement in conditional
>   staging: ccree: export symbol immediately following function
>   staging: ccree: fix switch case indentation
>
>  drivers/staging/ccree/ssi_buffer_mgr.c | 14 ++
>  drivers/staging/ccree/ssi_cipher.c |  6 ++-
>  drivers/staging/ccree/ssi_driver.c |  5 +-
>  drivers/staging/ccree/ssi_fips.c   |  2 -
>  drivers/staging/ccree/ssi_fips_ll.c| 85 
> +++---
>  drivers/staging/ccree/ssi_hash.c   | 33 +++--
>  drivers/staging/ccree/ssi_sysfs.c  |  3 +-
>  7 files changed, 57 insertions(+), 91 deletions(-)
>
> --
> 2.9.4
>


Looks good to me.

Acked-by: Gilad Ben-Yossef 

Thanks!

Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: ccree: Use __func__ instead of function name

2017-07-11 Thread Gilad Ben-Yossef 
Hello Karthik ,

Thank you for the patch.

On Thu, Jun 29, 2017 at 8:08 PM,   wrote:
> From: Karthik Tummala 
>
> Fixed following checkpatch.pl warning:
> WARNING: Prefer using '"%s...", __func__' to using
> the function's name, in a string
>
> It is prefered to use '%s & __func__' instead of function
> name for logging.
>
> Signed-off-by: Karthik Tummala 
> ---
> Changes for v2:
> v1 was a patch series, which consisted of two patches in which
> second one was already submitted by Gilad Ben-Yossef, so dropped
> that one.
>
> Patch generated on staging-testing as suggested by Greg-K H.
> ---
>  drivers/staging/ccree/ssi_aead.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/ccree/ssi_aead.c 
> b/drivers/staging/ccree/ssi_aead.c
> index 1fc0b05..1168161 100644
> --- a/drivers/staging/ccree/ssi_aead.c
> +++ b/drivers/staging/ccree/ssi_aead.c
> @@ -1886,7 +1886,7 @@ static int config_gcm_context(struct aead_request *req)
> (req->cryptlen - ctx->authsize);
> __be32 counter = cpu_to_be32(2);
>
> -   SSI_LOG_DEBUG("config_gcm_context() cryptlen = %d, req->assoclen = %d 
> ctx->authsize = %d\n", cryptlen, req->assoclen, ctx->authsize);
> +   SSI_LOG_DEBUG("%s() cryptlen = %d, req->assoclen = %d ctx->authsize = 
> %d\n", __func__, cryptlen, req->assoclen, ctx->authsize);
>
> memset(req_ctx->hkey, 0, AES_BLOCK_SIZE);
>
> @@ -2198,7 +2198,7 @@ static int ssi_rfc4106_gcm_setkey(struct crypto_aead 
> *tfm, const u8 *key, unsign
> struct ssi_aead_ctx *ctx = crypto_aead_ctx(tfm);
> int rc = 0;
>
> -   SSI_LOG_DEBUG("ssi_rfc4106_gcm_setkey()  keylen %d, key %p\n", 
> keylen, key);
> +   SSI_LOG_DEBUG("%s()  keylen %d, key %p\n", __func__, keylen, key);
>
> if (keylen < 4)
> return -EINVAL;
> @@ -2216,7 +2216,7 @@ static int ssi_rfc4543_gcm_setkey(struct crypto_aead 
> *tfm, const u8 *key, unsign
> struct ssi_aead_ctx *ctx = crypto_aead_ctx(tfm);
> int rc = 0;
>
> -   SSI_LOG_DEBUG("ssi_rfc4543_gcm_setkey()  keylen %d, key %p\n", 
> keylen, key);
> +   SSI_LOG_DEBUG("%s()  keylen %d, key %p\n", __func__, keylen, key);
>
> if (keylen < 4)
> return -EINVAL;
> --
> 1.9.1
>

Acked-by: Gilad Ben-Yossef 

Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/5] staging: ccree: fix checkpatch errors

2017-07-11 Thread Gilad Ben-Yossef 
Tyler,


On Tue, Jul 11, 2017 at 4:38 PM, Gilad Ben-Yossef  wrote:
> On Mon, Jul 10, 2017 at 12:10 AM,   wrote:
>> From: Tyler Olivieri 
>>
>> This patchset fixes several checkpatch errors and warnings in /staging/ccree:

You've messed Greg's email address, so my ACK bounced.

The content is good, but I don't know if Greg saw the patch set.
You might want to resend it.

Gilad.




-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: ccree: remove unnecessary cast on kmalloc

2017-07-11 Thread Gilad Ben-Yossef 
On Sun, Jul 9, 2017 at 8:43 AM, Gustavo A. R. Silva
 wrote:
> The assignment operator implicitly converts a void pointer to the type of the
> pointer it is assigned to.
>
> This issue was detected using Coccinelle and the following semantic patch:
>
> @@
> expression * e;
> expression arg1, arg2;
> type T;
> @@
>
> - e=(T*)
> + e=
> kmalloc(arg1, arg2);
>
> Signed-off-by: Gustavo A. R. Silva 

For both patches:

Acked-by: Gilad Ben-Yossef 

> ---
>  drivers/staging/ccree/ssi_buffer_mgr.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/ccree/ssi_buffer_mgr.c 
> b/drivers/staging/ccree/ssi_buffer_mgr.c
> index b35871e..18a8694 100644
> --- a/drivers/staging/ccree/ssi_buffer_mgr.c
> +++ b/drivers/staging/ccree/ssi_buffer_mgr.c
> @@ -1725,8 +1725,7 @@ int ssi_buffer_mgr_init(struct ssi_drvdata *drvdata)
> struct buff_mgr_handle *buff_mgr_handle;
> struct device *dev = &drvdata->plat_dev->dev;
>
> -   buff_mgr_handle = (struct buff_mgr_handle *)
> -   kmalloc(sizeof(struct buff_mgr_handle), GFP_KERNEL);
> +   buff_mgr_handle = kmalloc(sizeof(struct buff_mgr_handle), GFP_KERNEL);
> if (!buff_mgr_handle)
> return -ENOMEM;
>
> --
> 2.5.0
>

Thanks,
Gilad



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: ccree: move FIPS support to kernel infrastructure

2017-07-11 Thread Gilad Ben-Yossef 
The ccree driver had its own FIPS support, complete with
a test harness comparable to crypto testmgr and an
implementation which disables crypto functionality on
FIPS test error detection, either in Linux or from TEE.

This patch removes the duplication, while reimplementing
the handling of TEE reported FIPS errors according to the
kernel policy of inducing a panic in such an event.

Signed-off-by: Gilad Ben-Yossef 
---

Note: this patch is based on top of Tyler Olivieri patch
entitled "staging: ccree: fix switch case indentation".

 drivers/staging/ccree/Kconfig   |9 -
 drivers/staging/ccree/Makefile  |2 +-
 drivers/staging/ccree/ssi_aead.c|6 -
 drivers/staging/ccree/ssi_cipher.c  |   30 +-
 drivers/staging/ccree/ssi_driver.c  |8 +-
 drivers/staging/ccree/ssi_driver.h  |1 -
 drivers/staging/ccree/ssi_fips.c|  121 ++-
 drivers/staging/ccree/ssi_fips.h|   58 +-
 drivers/staging/ccree/ssi_fips_data.h   |  306 --
 drivers/staging/ccree/ssi_fips_ext.c|   92 --
 drivers/staging/ccree/ssi_fips_ll.c | 1620 ---
 drivers/staging/ccree/ssi_fips_local.c  |  357 ---
 drivers/staging/ccree/ssi_fips_local.h  |   67 --
 drivers/staging/ccree/ssi_hash.c|   21 -
 drivers/staging/ccree/ssi_request_mgr.c |2 -
 15 files changed, 130 insertions(+), 2570 deletions(-)
 delete mode 100644 drivers/staging/ccree/ssi_fips_data.h
 delete mode 100644 drivers/staging/ccree/ssi_fips_ext.c
 delete mode 100644 drivers/staging/ccree/ssi_fips_ll.c
 delete mode 100644 drivers/staging/ccree/ssi_fips_local.c
 delete mode 100644 drivers/staging/ccree/ssi_fips_local.h

diff --git a/drivers/staging/ccree/Kconfig b/drivers/staging/ccree/Kconfig
index 36a87c6..0b3092b 100644
--- a/drivers/staging/ccree/Kconfig
+++ b/drivers/staging/ccree/Kconfig
@@ -23,12 +23,3 @@ config CRYPTO_DEV_CCREE
  Choose this if you wish to use hardware acceleration of
  cryptographic operations on the system REE.
  If unsure say Y.
-
-config CCREE_FIPS_SUPPORT
-   bool "Turn on CryptoCell 7XX REE FIPS mode support"
-   depends on CRYPTO_DEV_CCREE
-   default n
-   help
- Say 'Y' to enable support for FIPS compliant mode by the
- CCREE driver.
- If unsure say N.
diff --git a/drivers/staging/ccree/Makefile b/drivers/staging/ccree/Makefile
index 318c2b3..ae702f3 100644
--- a/drivers/staging/ccree/Makefile
+++ b/drivers/staging/ccree/Makefile
@@ -1,3 +1,3 @@
 obj-$(CONFIG_CRYPTO_DEV_CCREE) := ccree.o
 ccree-y := ssi_driver.o ssi_sysfs.o ssi_buffer_mgr.o ssi_request_mgr.o 
ssi_cipher.o ssi_hash.o ssi_aead.o ssi_ivgen.o ssi_sram_mgr.o ssi_pm.o
-ccree-$(CCREE_FIPS_SUPPORT) += ssi_fips.o ssi_fips_ll.o ssi_fips_ext.o 
ssi_fips_local.o
+ccree-$(CONFIG_CRYPTO_FIPS) += ssi_fips.o
diff --git a/drivers/staging/ccree/ssi_aead.c b/drivers/staging/ccree/ssi_aead.c
index 1168161..3a10c31 100644
--- a/drivers/staging/ccree/ssi_aead.c
+++ b/drivers/staging/ccree/ssi_aead.c
@@ -36,7 +36,6 @@
 #include "ssi_hash.h"
 #include "ssi_sysfs.h"
 #include "ssi_sram_mgr.h"
-#include "ssi_fips_local.h"
 
 #define template_aead  template_u.aead
 
@@ -146,8 +145,6 @@ static int ssi_aead_init(struct crypto_aead *tfm)
container_of(alg, struct ssi_crypto_alg, aead_alg);
SSI_LOG_DEBUG("Initializing context @%p for %s\n", ctx, 
crypto_tfm_alg_name(&(tfm->base)));
 
-   CHECK_AND_RETURN_UPON_FIPS_ERROR();
-
/* Initialize modes in instance */
ctx->cipher_mode = ssi_alg->cipher_mode;
ctx->flow_mode = ssi_alg->flow_mode;
@@ -538,7 +535,6 @@ ssi_aead_setkey(struct crypto_aead *tfm, const u8 *key, 
unsigned int keylen)
SSI_LOG_DEBUG("Setting key in context @%p for %s. key=%p keylen=%u\n",
ctx, crypto_tfm_alg_name(crypto_aead_tfm(tfm)), key, keylen);
 
-   CHECK_AND_RETURN_UPON_FIPS_ERROR();
/* STAT_PHASE_0: Init and sanity checks */
 
if (ctx->auth_mode != DRV_HASH_NULL) { /* authenc() alg. */
@@ -654,7 +650,6 @@ static int ssi_aead_setauthsize(
 {
struct ssi_aead_ctx *ctx = crypto_aead_ctx(authenc);
 
-   CHECK_AND_RETURN_UPON_FIPS_ERROR();
/* Unsupported auth. sizes */
if ((authsize == 0) ||
(authsize > crypto_aead_maxauthsize(authenc))) {
@@ -1946,7 +1941,6 @@ static int ssi_aead_process(struct aead_request *req, 
enum drv_crypto_direction
SSI_LOG_DEBUG("%s context=%p req=%p iv=%p src=%p src_ofs=%d dst=%p 
dst_ofs=%d cryptolen=%d\n",
((direct == DRV_CRYPTO_DIRECTION_ENCRYPT) ? "Encrypt" : 
"Decrypt"), ctx, req, req->iv,
sg_virt(req->src), req->src->offset, sg_virt(req->dst), 
req->dst->offset, req->cryptlen);
-   CHECK_AND_RETURN_UPON_FIPS_ERROR();
 
/* STAT_PHASE_0: Init and sanity checks */
 
diff --git a/drivers/staging/ccree/ssi_cipher.c 
b/drivers/staging/ccree/ssi_cipher.c
index 4ef0c9b..af16bd0 100644
--- a/drivers/staging/c

Re: [PATCH 1/2] staging: atomisp2: hmm: Fixed comment style

2017-07-11 Thread Sakari Ailus 
On Mon, Jul 10, 2017 at 08:56:20PM +0200, Philipp Guendisch wrote:
> This patch fixed comment style. Semantic should not be affected.
> There are also two warnings left about too long lines, which
> reduce readability if changed.
> 
> Signed-off-by: Philipp Guendisch 
> Signed-off-by: Chris Baller 

Hi Philipp,

The patches don't apply. Recently there have been many atomisp patches
being posted and I've applied them to the atomisp branch here:

https://git.linuxtv.org/sailus/media_tree.git/log/?h=atomisp>

Could you rebase yours on top of that, please?

-- 
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 00/10] Fix alignment issues in staging/ccree

2017-07-11 Thread Greg KH 
On Sun, Jul 02, 2017 at 01:25:45AM +0200, Simon Sandström wrote:
> Fixes a total of 195 alignment issues in staging/ccree reported by
> checkpatch.pl. Adds a few "line over 80 characters" warnings as a
> result of the realignments, but I could try to get rid of them in the
> same patchset if needed.

Not all of these applied, some did, if you could rebase the remaining
against my staging-testing branch at the moment, and resend, that would
be great.

thanks,

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

Re: [PATCH] drivers/staging/wilc1000: fix sparse warning: right shift by bigger than source value

2017-07-11 Thread Greg Kroah-Hartman 
On Mon, Jul 10, 2017 at 04:57:31PM +0800, Rui Teng wrote:
> This patch sets memory to zero directly to avoid unnecessary shift and
> bitwise operations on bool type, which can fix a sparse warning and also
> improve performance.

It does?  How did you measure the performance impact?  What was now
faster?

thanks,

greg k-h


> 
> Signed-off-by: Rui Teng 
> ---
>  drivers/staging/wilc1000/host_interface.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/host_interface.c 
> b/drivers/staging/wilc1000/host_interface.c
> index 2568dfc15181..036c5c19a016 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -2416,10 +2416,9 @@ static void Handle_SetMulticastFilter(struct wilc_vif 
> *vif,
>   goto ERRORHANDLER;
>  
>   pu8CurrByte = wid.val;
> - *pu8CurrByte++ = (strHostIfSetMulti->enabled & 0xFF);
> - *pu8CurrByte++ = ((strHostIfSetMulti->enabled >> 8) & 0xFF);
> - *pu8CurrByte++ = ((strHostIfSetMulti->enabled >> 16) & 0xFF);
> - *pu8CurrByte++ = ((strHostIfSetMulti->enabled >> 24) & 0xFF);
> + memset(pu8CurrByte, 0, 4);
> + *pu8CurrByte = (strHostIfSetMulti->enabled & 0xFF);
> + pu8CurrByte += 4;

Are you sure enabled isn't larger than 8 bits?

thanks,

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

Re: [PATCH 8/8] Staging: lustre :lustre: include :lustre_compat.h: Prefer using the BIT macro

2017-07-11 Thread Greg KH 
On Thu, Jul 06, 2017 at 12:43:15PM +0530, Jaya Durga wrote:
> Replace all instances of (1 << 27) with BIT(27) to fix
> checkpatch check messages
> 
> Signed-off-by: Jaya Durga 
> ---
>  drivers/staging/lustre/lustre/include/lustre_compat.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/lustre/lustre/include/lustre_compat.h 
> b/drivers/staging/lustre/lustre/include/lustre_compat.h
> index da9ce19..686a251 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_compat.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_compat.h
> @@ -43,7 +43,7 @@
>   * set ATTR_BLOCKS to a high value to avoid any risk of collision with other
>   * ATTR_* attributes (see bug 13828)
>   */
> -#define ATTR_BLOCKS(1 << 27)
> +#define ATTR_BLOCKSBIT(27)

Isn't this used in lustre's userspace code?  If so, you can't use the
BIT() macro there :(

Please check before you redo this.

thanks,

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

Re: [PATCH 2/6] Staging: rtl8712 : os_intfs.c: use octal permission representation

2017-07-11 Thread Greg KH 
On Fri, Jun 30, 2017 at 11:28:13AM +0530, Jaya Durga wrote:
> Fix checkpatch.pl Warning: Symbolic permissions 'S_IRUGO | S_IWUSR' are not 
> preferred.
> Consider using octal permissions '0644'.
> 
> Signed-off-by: Jaya Durga 
> ---
>  drivers/staging/rtl8712/os_intfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Where are the other 5 patches in this series?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/6] Staging: rtl8712 : os_intfs.c: use octal permission representation

2017-07-11 Thread Greg KH 
On Tue, Jul 04, 2017 at 11:31:15AM +0530, Jaya Durga wrote:
> Fix checkpatch.pl Warning: Symbolic permissions 'S_IRUGO | S_IWUSR' are not 
> preferred.
> Consider using octal permissions '0644'.
> 
> Signed-off-by: Jaya Durga 
> ---
>  drivers/staging/rtl8712/os_intfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Did something change from the previous version of this patch?  If so,
what?

And again, where are the rest of the patches in this series?

thanks,

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

Re: [PATCH 7/7] Staging: rtl8712 : rtl871x_io.c:fix coding style of kmalloc usage

2017-07-11 Thread Greg KH 
On Tue, Jul 04, 2017 at 04:11:31PM +0530, Jaya Durga wrote:
> CHECK: multiple assignments should be avoided
> CHECK: Prefer kmalloc(sizeof(*pintf_hdl->pintfpriv)...) 
> over kmalloc(sizeof(struct intf_priv)...)
> 
> Signed-off-by: Jaya Durga 
> ---
>  drivers/staging/rtl8712/rtl871x_io.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Where are the other 6 patches in this series?  I can't apply just one of
them :(

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

Re: [PATCH 390/390] staging:rtl8723bs:core:rtw_btcoex: Fixed checkpatch.pl warning on 'Comparisons should place the constant on the right side of the test'.

2017-07-11 Thread Greg KH 
On Wed, Jul 05, 2017 at 12:49:52PM +0530, Shreeya Patel wrote:
> Fixed a coding style issue.
> 
> Signed-off-by: Shreeya Patel 
> ---
>  drivers/staging/rtl8723bs/core/rtw_btcoex.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Where are the 329 other patches in this series?  I can't take just one
of them, right?

And your subject is _very_ long :(

thanks,

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

Re: [PATCH] staging: lustre: mgc: Fix incorrect type in assignment

2017-07-11 Thread Greg Kroah-Hartman 
On Sat, Jul 01, 2017 at 05:26:52PM +0300, Kamal Heib wrote:
> Fix the following sparse warnings:
> mgc_request.c:68:25: warning: incorrect type in assignment (different base 
> types)
> mgc_request.c:68:25:expected unsigned long long [unsigned] [long] [long 
> long] 
> mgc_request.c:68:25:got restricted __le64 [usertype] 
> mgc_request.c:82:25: warning: incorrect type in assignment (different base 
> types)
> mgc_request.c:82:25:expected unsigned long long [unsigned] [long] [long 
> long] 
> mgc_request.c:82:25:got restricted __le64 [usertype] 
> 
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Kamal Heib 
> ---
>  drivers/staging/lustre/lustre/mgc/mgc_request.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c 
> b/drivers/staging/lustre/lustre/mgc/mgc_request.c
> index eee0b667a33c..93819bb84ef8 100644
> --- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
> +++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
> @@ -65,7 +65,7 @@ static int mgc_name2resid(char *name, int len, struct 
> ldlm_res_id *res_id,
>  
>   /* Always use the same endianness for the resid */
>   memset(res_id, 0, sizeof(*res_id));
> - res_id->name[0] = cpu_to_le64(resname);
> + res_id->name[0] = resname;

Really?  What about that comment right above?  Are you _sure_ this patch
is correct?

thanks,

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

Re: [PATCH 1/4] Staging: Luster: Clean up line over 80Char in lib-lnet.h

2017-07-11 Thread Greg Kroah-Hartman 
On Fri, Jul 07, 2017 at 01:46:42AM +, Craig Inches wrote:
> This patch fixes a warning generated by checkpatch for
> a line over 80 characters.
> 
> Signed-off-by: Craig Inches 
> ---
>  drivers/staging/lustre/include/linux/lnet/lib-lnet.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Your subject line mispelled the filesystem's name :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/4] Staging: Lustre Fixing multiline block comments in lnetst.h

2017-07-11 Thread Greg Kroah-Hartman 
On Fri, Jul 07, 2017 at 01:47:04AM +, Craig Inches wrote:
> This fixes multiple block statements found not to match
> style as per checkpatch
> 
> Signed-off-by: Craig Inches 
> ---
>  drivers/staging/lustre/include/linux/lnet/lnetst.h | 129 
> +
>  1 file changed, 81 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/staging/lustre/include/linux/lnet/lnetst.h 
> b/drivers/staging/lustre/include/linux/lnet/lnetst.h
> index ea736f8d5231..a4f9ff01d458 100644
> --- a/drivers/staging/lustre/include/linux/lnet/lnetst.h
> +++ b/drivers/staging/lustre/include/linux/lnet/lnetst.h
> @@ -54,7 +54,8 @@
>  #define LSTIO_GROUP_ADD  0xC10   /* add group */
>  #define LSTIO_GROUP_LIST 0xC11   /* list all groups in session */
>  #define LSTIO_GROUP_INFO 0xC12   /* query default information of
> -  * specified group */
> +  * specified group
> +  */
>  #define LSTIO_GROUP_DEL  0xC13   /* delete group */
>  #define LSTIO_NODES_ADD  0xC14   /* add nodes to specified group 
> */
>  #define LSTIO_GROUP_UPDATE  0xC15/* update group */
> @@ -102,27 +103,32 @@ struct lstcon_test_ent {
>   int tse_type;   /* test type */
>   int tse_loop;   /* loop count */
>   int tse_concur; /* concurrency of test */
> -};   /*** test summary entry, for
> -  *** list_batch command */
> +};   /* test summary entry, for
> +  * list_batch command
> +  */

That's odd, what was the *** stuff for?

I'd like to get a lustre maintainer's ack for all of these before I
apply them...

thanks,

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

Re: [PATCH 04/25] staging: unisys: visorbus: visorbus_main.c: fixed comment formatting issues

2017-07-11 Thread Greg KH 
On Fri, Jun 30, 2017 at 03:43:05PM -0400, David Kershner wrote:
> From: Sameer Wadgaonkar 
> 
> Removed comments from the right side of the lines.

You also reformattted a few of them.  Not a big deal, but don't lie in
changelogs :)

thanks,

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

Re: [PATCH] staging: ks7010: fix styling WARNINGs

2017-07-11 Thread Greg KH 
On Fri, Jun 30, 2017 at 11:39:27AM -0700, Mark Rogers wrote:
> Thank you for your feedback. I guess when making this patch I had the
> preferred coding style in mind, but didn't ask myself if making the code
> conform to it would truly improve readability.
> 
> I agree with all of your comments. Do you think the best course of
> action is to create a new patch with this change alone and forget the
> rest?
> 
> -   DPRINTK(1, "ks7010_sdio_remove()\n");
> +   DPRINTK(1, "%s()\n", __func__);

Lines like this, that are just "here I am!" comments, should all be
deleted anyway, we have ftrace in the kernel, people always seem to
forget about it...

thanks,

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

[PATCH][V2] staging: fbtft: make const array gamma_par_mask static

2017-07-11 Thread Colin King 
From: Colin Ian King 

Don't populate array gamma_par_mask on the stack but instead make it
static.  Makes the object code smaller by 148 bytes:

Before:
   textdata bss dec hex filename
   29931104   040971001 drivers/staging/fbtft/fb_st7789v.o

After:
   textdata bss dec hex filename
   27571192   03949 f6d drivers/staging/fbtft/fb_st7789v.o

Signed-off-by: Colin Ian King 
---
 drivers/staging/fbtft/fb_st7789v.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/fbtft/fb_st7789v.c 
b/drivers/staging/fbtft/fb_st7789v.c
index 8935a97ec048..a5d7c87557f8 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -189,7 +189,7 @@ static int set_gamma(struct fbtft_par *par, u32 *curves)
 * The masks are the same for both positive and negative voltage
 * gamma curves.
 */
-   const u8 gamma_par_mask[] = {
+   static const u8 gamma_par_mask[] = {
0xFF, /* V63[3:0], V0[3:0]*/
0x3F, /* V1[5:0] */
0x3F, /* V2[5:0] */
-- 
2.11.0

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

[PATCH] Clean up lirc zilog error codes

2017-07-11 Thread Yves Lemée 
According the coding style guidelines, the ENOSYS error code must be returned
in case of a non existent system call. This code has been replaced with
the ENOTTY error code indicating, a missing functionality.

Signed-off-by: Yves Lemée 
---
 drivers/staging/media/lirc/lirc_zilog.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index 015e41bd036e..26dd32d5b5b2 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -1249,7 +1249,7 @@ static long ioctl(struct file *filep, unsigned int cmd, 
unsigned long arg)
break;
case LIRC_GET_REC_MODE:
if (!(features & LIRC_CAN_REC_MASK))
-   return -ENOSYS;
+   return -ENOTTY;
 
result = put_user(LIRC_REC2MODE
  (features & LIRC_CAN_REC_MASK),
@@ -1257,21 +1257,21 @@ static long ioctl(struct file *filep, unsigned int cmd, 
unsigned long arg)
break;
case LIRC_SET_REC_MODE:
if (!(features & LIRC_CAN_REC_MASK))
-   return -ENOSYS;
+   return -ENOTTY;
 
result = get_user(mode, uptr);
if (!result && !(LIRC_MODE2REC(mode) & features))
-   result = -EINVAL;
+   result = -ENOTTY;
break;
case LIRC_GET_SEND_MODE:
if (!(features & LIRC_CAN_SEND_MASK))
-   return -ENOSYS;
+   return -ENOTTY;
 
result = put_user(LIRC_MODE_PULSE, uptr);
break;
case LIRC_SET_SEND_MODE:
if (!(features & LIRC_CAN_SEND_MASK))
-   return -ENOSYS;
+   return -ENOTTY;
 
result = get_user(mode, uptr);
if (!result && mode != LIRC_MODE_PULSE)
-- 
2.13.2

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

Re: [PATCH] Staging: android/ion: fix sparse warnings

2017-07-11 Thread Greg KH 
On Wed, Jul 05, 2017 at 12:12:24AM +, Joseph Wright wrote:
> ion_carveout_heap.c:115:17: warning: symbol 'ion_carveout_heap_create' \
>   was not declared. Should it be static?
> ion_chunk_heap.c:120:17: warning: symbol 'ion_chunk_heap_create' \
>   was not declared. Should it be static?
> ion_cma_heap.c:109:5: warning: symbol '__ion_add_cma_heaps' \
>   was not declared. Should it be static?

You are doing two different types of things here, please break this up
into multiple patches.

thanks,

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

Re: [PATCH] staging: unisys: visorbus: fix function open braces

2017-07-11 Thread Greg Kroah-Hartman 
On Mon, Jul 10, 2017 at 11:48:26PM -0400, Mitchell Tasman wrote:
> Resolve multiple checkpatch errors by relocating open braces
> following function definitions to the next line.
> 
> Signed-off-by: Mitchell Tasman 
> ---
>  drivers/staging/unisys/visorbus/visorbus_main.c | 18 --
>  1 file changed, 12 insertions(+), 6 deletions(-)

Doesn't apply on top of the other unisys patches that were recently
posted :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: wlan-ng: Use little-endian type

2017-07-11 Thread Aviv Palivoda 
Fix the following sparse warning:
drivers/staging//wlan-ng/prism2sta.c:1691:20: warning: incorrect type in 
assignment (different base types)

(a) Change struct hfa384x_authenticate_station_data status member type to 
__le16.
(b) All assignment to status are converted to little-endian prior to assignment.

Signed-off-by: Aviv Palivoda 
---
 drivers/staging/wlan-ng/hfa384x.h   |  2 +-
 drivers/staging/wlan-ng/prism2sta.c | 17 -
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/wlan-ng/hfa384x.h 
b/drivers/staging/wlan-ng/hfa384x.h
index 018db22..fa8cb4a 100644
--- a/drivers/staging/wlan-ng/hfa384x.h
+++ b/drivers/staging/wlan-ng/hfa384x.h
@@ -413,7 +413,7 @@ struct hfa384x_join_request_data {
 /*-- Configuration Record: authenticateStation (data portion only) --*/
 struct hfa384x_authenticate_station_data {
u8 address[ETH_ALEN];
-   u16 status;
+   __le16 status;
u16 algorithm;
 } __packed;
 
diff --git a/drivers/staging/wlan-ng/prism2sta.c 
b/drivers/staging/wlan-ng/prism2sta.c
index e16da34..250af0d 100644
--- a/drivers/staging/wlan-ng/prism2sta.c
+++ b/drivers/staging/wlan-ng/prism2sta.c
@@ -1561,7 +1561,7 @@ static void prism2sta_inf_authreq_defer(struct wlandevice 
*wlandev,
 */
 
ether_addr_copy(rec.address, inf->info.authreq.sta_addr);
-   rec.status = P80211ENUM_status_unspec_failure;
+   rec.status = cpu_to_le16(P80211ENUM_status_unspec_failure);
 
/*
 * Authenticate based on the access mode.
@@ -1578,7 +1578,7 @@ static void prism2sta_inf_authreq_defer(struct wlandevice 
*wlandev,
for (i = 0; i < hw->authlist.cnt; i++)
if (ether_addr_equal(rec.address,
 hw->authlist.addr[i])) {
-   rec.status = P80211ENUM_status_successful;
+   rec.status = 
cpu_to_le16(P80211ENUM_status_successful);
break;
}
 
@@ -1590,7 +1590,7 @@ static void prism2sta_inf_authreq_defer(struct wlandevice 
*wlandev,
 * Allow all authentications.
 */
 
-   rec.status = P80211ENUM_status_successful;
+   rec.status = cpu_to_le16(P80211ENUM_status_successful);
break;
 
case WLAN_ACCESS_ALLOW:
@@ -1615,7 +1615,7 @@ static void prism2sta_inf_authreq_defer(struct wlandevice 
*wlandev,
 
for (i = 0; i < cnt; i++, addr += ETH_ALEN)
if (ether_addr_equal(rec.address, addr)) {
-   rec.status = P80211ENUM_status_successful;
+   rec.status = 
cpu_to_le16(P80211ENUM_status_successful);
break;
}
 
@@ -1641,11 +1641,11 @@ static void prism2sta_inf_authreq_defer(struct 
wlandevice *wlandev,
addr = hw->deny.addr1[0];
}
 
-   rec.status = P80211ENUM_status_successful;
+   rec.status = cpu_to_le16(P80211ENUM_status_successful);
 
for (i = 0; i < cnt; i++, addr += ETH_ALEN)
if (ether_addr_equal(rec.address, addr)) {
-   rec.status = P80211ENUM_status_unspec_failure;
+   rec.status = 
cpu_to_le16(P80211ENUM_status_unspec_failure);
break;
}
 
@@ -1663,7 +1663,7 @@ static void prism2sta_inf_authreq_defer(struct wlandevice 
*wlandev,
 
added = 0;
 
-   if (rec.status == P80211ENUM_status_successful) {
+   if (rec.status == cpu_to_le16(P80211ENUM_status_successful)) {
for (i = 0; i < hw->authlist.cnt; i++)
if (ether_addr_equal(rec.address,
 hw->authlist.addr[i]))
@@ -1671,7 +1671,7 @@ static void prism2sta_inf_authreq_defer(struct wlandevice 
*wlandev,
 
if (i >= hw->authlist.cnt) {
if (hw->authlist.cnt >= WLAN_AUTH_MAX) {
-   rec.status = P80211ENUM_status_ap_full;
+   rec.status = 
cpu_to_le16(P80211ENUM_status_ap_full);
} else {
ether_addr_copy(
hw->authlist.addr[hw->authlist.cnt],
@@ -1688,7 +1688,6 @@ static void prism2sta_inf_authreq_defer(struct wlandevice 
*wlandev,
 * it was added.
 */
 
-   rec.status = cpu_to_le16(rec.status);
rec.algorithm = inf->info.authreq.algorithm;
 
result = hfa384x_drvr_setconfig(hw, HFA384x_RID_AUTHENTICATESTA,
-- 
2.7.4

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

[PATCH] staging: vchiq_arm: fix error codes in probe

2017-07-11 Thread Dan Carpenter 
If vchiq_debugfs_init() fails, then we accidentally return a valid
pointer casted to int on error.  This code is simpler if we get rid of
the "ptr_err" variable and just use "err" throughout.

Signed-off-by: Dan Carpenter 

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 030bec855d86..314ffac50bb8 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -3391,7 +3391,6 @@ static int vchiq_probe(struct platform_device *pdev)
struct device_node *fw_node;
struct rpi_firmware *fw;
int err;
-   void *ptr_err;
 
fw_node = of_parse_phandle(pdev->dev.of_node, "firmware", 0);
if (!fw_node) {
@@ -3427,14 +3426,14 @@ static int vchiq_probe(struct platform_device *pdev)
 
/* create sysfs entries */
vchiq_class = class_create(THIS_MODULE, DEVICE_NAME);
-   ptr_err = vchiq_class;
-   if (IS_ERR(ptr_err))
+   err = PTR_ERR(vchiq_class);
+   if (IS_ERR(vchiq_class))
goto failed_class_create;
 
vchiq_dev = device_create(vchiq_class, NULL,
vchiq_devid, NULL, "vchiq");
-   ptr_err = vchiq_dev;
-   if (IS_ERR(ptr_err))
+   err = PTR_ERR(vchiq_dev);
+   if (IS_ERR(vchiq_dev))
goto failed_device_create;
 
/* create debugfs entries */
@@ -3455,7 +3454,6 @@ static int vchiq_probe(struct platform_device *pdev)
class_destroy(vchiq_class);
 failed_class_create:
cdev_del(&vchiq_cdev);
-   err = PTR_ERR(ptr_err);
 failed_cdev_add:
unregister_chrdev_region(vchiq_devid, 1);
 failed_platform_init:
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH RESEND] staging: sm750fb: avoid conflicting vesafb

2017-07-11 Thread Sudip Mukherjee 
Hi Greg,

On Fri, Jun 30, 2017 at 09:57:43PM +0100, Sudip Mukherjee wrote:
> From: Teddy Wang 
> 
> If vesafb is enabled in the config then /dev/fb0 is created by vesa
> and this sm750 driver gets fb1, fb2. But we need to be fb0 and fb1 to
> effectively work with xorg.
> So if it has been alloted fb1, then try to remove the other fb0.
> 
> Cc:  # v4.4+
> Signed-off-by: Teddy Wang 
> Signed-off-by: Sudip Mukherjee 
> ---
> 
> In the previous send, why #ifdef is used was asked.
> https://lkml.org/lkml/2017/6/25/57
> 
> Answered at: https://lkml.org/lkml/2017/6/25/69
> Also pasting here for reference.
> 
> 'Did a quick research into "why".
> The patch d8801e4df91e ("x86/PCI: Set IORESOURCE_ROM_SHADOW only for the
> default VGA device") has started setting IORESOURCE_ROM_SHADOW in flags
> for a default VGA device and that is being done only for x86.
> And so, we will need that #ifdef to check IORESOURCE_ROM_SHADOW as that
> needs to be checked only for a x86 and not for other arch.'

A gentle ping.

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

Re: [PATCH] drivers/staging/wilc1000: fix sparse warning: right shift by bigger than source value

2017-07-11 Thread Rui Teng 

On 12/07/2017 1:04 AM, Greg Kroah-Hartman wrote:

On Mon, Jul 10, 2017 at 04:57:31PM +0800, Rui Teng wrote:

This patch sets memory to zero directly to avoid unnecessary shift and
bitwise operations on bool type, which can fix a sparse warning and also
improve performance.


It does?  How did you measure the performance impact?  What was now
faster?


It can avoid 3 times right shift and 3 times bitwise operations.
And once memory set should also faster than 4 times copy operations.
And add number 4 once should also faster than 4 times plus plus.



thanks,

greg k-h




Signed-off-by: Rui Teng 
---
 drivers/staging/wilc1000/host_interface.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 2568dfc15181..036c5c19a016 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -2416,10 +2416,9 @@ static void Handle_SetMulticastFilter(struct wilc_vif 
*vif,
goto ERRORHANDLER;

pu8CurrByte = wid.val;
-   *pu8CurrByte++ = (strHostIfSetMulti->enabled & 0xFF);
-   *pu8CurrByte++ = ((strHostIfSetMulti->enabled >> 8) & 0xFF);
-   *pu8CurrByte++ = ((strHostIfSetMulti->enabled >> 16) & 0xFF);
-   *pu8CurrByte++ = ((strHostIfSetMulti->enabled >> 24) & 0xFF);
+   memset(pu8CurrByte, 0, 4);
+   *pu8CurrByte = (strHostIfSetMulti->enabled & 0xFF);
+   pu8CurrByte += 4;


Are you sure enabled isn't larger than 8 bits?


The size of bool may larger than 8 bits. But when we assign any value
to bool type, the value of bool type will only be 1 or 0.
For example, the following output will be 1 other than 0x100.
bool b = 0x100;
printf("b: %d\n", b);

Or I think it should change 'enabled' type from bool to u32, to make 
sure that the right shift operation can take effect.




thanks,

greg k-h



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

[PATCH v2 1/2] Staging: android/ion: fix sparse warnings

2017-07-11 Thread Joseph Wright 
Declare functions to fix sparse warnings:

ion_carveout_heap.c:115:17: warning: symbol 'ion_carveout_heap_create' \
was not declared. Should it be static?
ion_chunk_heap.c:120:17: warning: symbol 'ion_chunk_heap_create' \
was not declared. Should it be static?

Signed-off-by: Joseph Wright 
---
Changes in v2:
  - Split into multiple patches

 drivers/staging/android/ion/ion.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/android/ion/ion.h 
b/drivers/staging/android/ion/ion.h
index fa9ed81..fda1e91 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -358,4 +358,8 @@ long ion_ioctl(struct file *filp, unsigned int cmd, 
unsigned long arg);
 
 int ion_query_heaps(struct ion_heap_query *query);
 
+struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap *heap_data);
+
+struct ion_heap *ion_carveout_heap_create(struct ion_platform_heap *heap_data);
+
 #endif /* _ION_H */
-- 
2.9.3

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

[PATCH v2 0/2] Staging: android/ion: fix sparse warnings

2017-07-11 Thread Joseph Wright 
Split sparse warning fixes into multiple patches.

Joseph Wright (2):
  Staging: android/ion: fix sparse warnings
  Staging: android/ion: fix sparse warning

 drivers/staging/android/ion/ion.h  | 4 
 drivers/staging/android/ion/ion_cma_heap.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

-- 
2.9.3

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

[PATCH v2 2/2] Staging: android/ion: fix sparse warning

2017-07-11 Thread Joseph Wright 
Declare private function static to fix sparse warning:

ion_cma_heap.c:109:5: warning: symbol '__ion_add_cma_heaps' \
was not declared. Should it be static?

Signed-off-by: Joseph Wright 
---
Changes in v2:
  - Split into multiple patches

 drivers/staging/android/ion/ion_cma_heap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion_cma_heap.c 
b/drivers/staging/android/ion/ion_cma_heap.c
index a0949bc..c6db9b7 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -106,7 +106,7 @@ static struct ion_heap *__ion_cma_heap_create(struct cma 
*cma)
return &cma_heap->heap;
 }
 
-int __ion_add_cma_heaps(struct cma *cma, void *data)
+static int __ion_add_cma_heaps(struct cma *cma, void *data)
 {
struct ion_heap *heap;
 
-- 
2.9.3

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

Re: [PATCH 3/4] Staging: Lustre Fixing multiline block comments in lnetst.h

2017-07-11 Thread Dilger, Andreas 
On Jul 11, 2017, at 11:14, Greg Kroah-Hartman  
wrote:
> 
> On Fri, Jul 07, 2017 at 01:47:04AM +, Craig Inches wrote:
>> This fixes multiple block statements found not to match
>> style as per checkpatch
>> 
>> Signed-off-by: Craig Inches 
>> ---
>> drivers/staging/lustre/include/linux/lnet/lnetst.h | 129 
>> +
>> 1 file changed, 81 insertions(+), 48 deletions(-)
>> 
>> diff --git a/drivers/staging/lustre/include/linux/lnet/lnetst.h 
>> b/drivers/staging/lustre/include/linux/lnet/lnetst.h
>> index ea736f8d5231..a4f9ff01d458 100644
>> --- a/drivers/staging/lustre/include/linux/lnet/lnetst.h
>> +++ b/drivers/staging/lustre/include/linux/lnet/lnetst.h
>> @@ -54,7 +54,8 @@
>> #define LSTIO_GROUP_ADD  0xC10   /* add group */
>> #define LSTIO_GROUP_LIST 0xC11   /* list all groups in session */
>> #define LSTIO_GROUP_INFO 0xC12   /* query default information of
>> - * specified group */
>> + * specified group
>> + */
>> #define LSTIO_GROUP_DEL  0xC13   /* delete group */
>> #define LSTIO_NODES_ADD  0xC14   /* add nodes to specified group 
>> */
>> #define LSTIO_GROUP_UPDATE  0xC15/* update group */
>> @@ -102,27 +103,32 @@ struct lstcon_test_ent {
>>  int tse_type;   /* test type */
>>  int tse_loop;   /* loop count */
>>  int tse_concur; /* concurrency of test */
>> -};  /*** test summary entry, for
>> - *** list_batch command */
>> +};  /* test summary entry, for
>> + * list_batch command
>> + */
> 
> That's odd, what was the *** stuff for?
> 
> I'd like to get a lustre maintainer's ack for all of these before I
> apply them...

It's nothing that I know about.  We used DOxygen to comment the code and 
generate docs,
but the "***" isn't any markup that I'm familiar with.

Reviewed-by: Andreas Dilger 

Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel Corporation







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

Re: [PATCH 8/8] Staging: lustre :lustre: include :lustre_compat.h: Prefer using the BIT macro

2017-07-11 Thread Dilger, Andreas 

> On Jul 11, 2017, at 11:08, Greg KH  wrote:
> 
> On Thu, Jul 06, 2017 at 12:43:15PM +0530, Jaya Durga wrote:
>> Replace all instances of (1 << 27) with BIT(27) to fix
>> checkpatch check messages
>> 
>> Signed-off-by: Jaya Durga 
>> ---
>> drivers/staging/lustre/lustre/include/lustre_compat.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/staging/lustre/lustre/include/lustre_compat.h 
>> b/drivers/staging/lustre/lustre/include/lustre_compat.h
>> index da9ce19..686a251 100644
>> --- a/drivers/staging/lustre/lustre/include/lustre_compat.h
>> +++ b/drivers/staging/lustre/lustre/include/lustre_compat.h
>> @@ -43,7 +43,7 @@
>>  * set ATTR_BLOCKS to a high value to avoid any risk of collision with other
>>  * ATTR_* attributes (see bug 13828)
>>  */
>> -#define ATTR_BLOCKS(1 << 27)
>> +#define ATTR_BLOCKSBIT(27)
> 
> Isn't this used in lustre's userspace code?  If so, you can't use the
> BIT() macro there :(
> 
> Please check before you redo this.

The "lustre_compat.h" header was previously used for compatibility between 
different kernel
versions, which is why it is now basically empty.  It isn't used for userspace 
interfaces
as other "compat" headers are in the kernel.

Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel Corporation







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

Re: [PATCH v2 1/2] Staging: android/ion: fix sparse warnings

2017-07-11 Thread Frans Klaver 
Hi,

please consider changing your subject to something like

staging: android/ion: declare two functions

Perhaps you can make it more on-topic. It's more useful than "fix
sparse warning"

On Wed, Jul 12, 2017 at 6:51 AM, Joseph Wright  wrote:
> Declare functions to fix sparse warnings:
>
> ion_carveout_heap.c:115:17: warning: symbol 'ion_carveout_heap_create' \
> was not declared. Should it be static?
> ion_chunk_heap.c:120:17: warning: symbol 'ion_chunk_heap_create' \
> was not declared. Should it be static?

And then explain why declaring them is preferred over making them static.

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

Re: [PATCH v2 2/2] Staging: android/ion: fix sparse warning

2017-07-11 Thread Frans Klaver 
Hi,

Again, your subject is too generic.


On Wed, Jul 12, 2017 at 6:51 AM, Joseph Wright  wrote:
> Declare private function static to fix sparse warning:
>
> ion_cma_heap.c:109:5: warning: symbol '__ion_add_cma_heaps' \
> was not declared. Should it be static?
>
> Signed-off-by: Joseph Wright 
> ---
> Changes in v2:
>   - Split into multiple patches
>
>  drivers/staging/android/ion/ion_cma_heap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/android/ion/ion_cma_heap.c 
> b/drivers/staging/android/ion/ion_cma_heap.c
> index a0949bc..c6db9b7 100644
> --- a/drivers/staging/android/ion/ion_cma_heap.c
> +++ b/drivers/staging/android/ion/ion_cma_heap.c
> @@ -106,7 +106,7 @@ static struct ion_heap *__ion_cma_heap_create(struct cma 
> *cma)
> return &cma_heap->heap;
>  }
>
> -int __ion_add_cma_heaps(struct cma *cma, void *data)
> +static int __ion_add_cma_heaps(struct cma *cma, void *data)
>  {
> struct ion_heap *heap;
>
> --
> 2.9.3
>
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: wlan-ng: Use little-endian type

2017-07-11 Thread Frans Klaver 
On Tue, Jul 11, 2017 at 9:51 PM, Aviv Palivoda  wrote:
> Fix the following sparse warning:
> drivers/staging//wlan-ng/prism2sta.c:1691:20: warning: incorrect type in 
> assignment (different base types)
>
> (a) Change struct hfa384x_authenticate_station_data status member type to 
> __le16.
> (b) All assignment to status are converted to little-endian prior to 
> assignment.

Why is this the right thing to do?

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

Re: [PATCH] Clean up lirc zilog error codes

2017-07-11 Thread Frans Klaver 
On Tue, Jul 11, 2017 at 7:57 PM, Yves Lemée  wrote:
> According the coding style guidelines, the ENOSYS error code must be returned
> in case of a non existent system call. This code has been replaced with
> the ENOTTY error code indicating, a missing functionality.
>
> Signed-off-by: Yves Lemée 

Your subject line is not according to the patch submission guidelines.

Also, on a nit-picking note, that comma after 'indicating' is rather
oddly placed. Please move or remove it.

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

Re: [PATCH RESEND] staging: sm750fb: avoid conflicting vesafb

2017-07-11 Thread Greg Kroah-Hartman 
On Tue, Jul 11, 2017 at 10:03:01PM +0100, Sudip Mukherjee wrote:
> Hi Greg,
> 
> On Fri, Jun 30, 2017 at 09:57:43PM +0100, Sudip Mukherjee wrote:
> > From: Teddy Wang 
> > 
> > If vesafb is enabled in the config then /dev/fb0 is created by vesa
> > and this sm750 driver gets fb1, fb2. But we need to be fb0 and fb1 to
> > effectively work with xorg.
> > So if it has been alloted fb1, then try to remove the other fb0.
> > 
> > Cc:  # v4.4+
> > Signed-off-by: Teddy Wang 
> > Signed-off-by: Sudip Mukherjee 
> > ---
> > 
> > In the previous send, why #ifdef is used was asked.
> > https://lkml.org/lkml/2017/6/25/57
> > 
> > Answered at: https://lkml.org/lkml/2017/6/25/69
> > Also pasting here for reference.
> > 
> > 'Did a quick research into "why".
> > The patch d8801e4df91e ("x86/PCI: Set IORESOURCE_ROM_SHADOW only for the
> > default VGA device") has started setting IORESOURCE_ROM_SHADOW in flags
> > for a default VGA device and that is being done only for x86.
> > And so, we will need that #ifdef to check IORESOURCE_ROM_SHADOW as that
> > needs to be checked only for a x86 and not for other arch.'
> 
> A gentle ping.

It's in the middle of the merge window, I can't do anything with patches
right now, you know better than this...

I'll get to it after 4.13-rc1 is out, relax please.

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

Re: [PATCH] drivers/staging/wilc1000: fix sparse warning: right shift by bigger than source value

2017-07-11 Thread Greg Kroah-Hartman 
On Wed, Jul 12, 2017 at 10:23:02AM +0800, Rui Teng wrote:
> On 12/07/2017 1:04 AM, Greg Kroah-Hartman wrote:
> > On Mon, Jul 10, 2017 at 04:57:31PM +0800, Rui Teng wrote:
> > > This patch sets memory to zero directly to avoid unnecessary shift and
> > > bitwise operations on bool type, which can fix a sparse warning and also
> > > improve performance.
> > 
> > It does?  How did you measure the performance impact?  What was now
> > faster?
> 
> It can avoid 3 times right shift and 3 times bitwise operations.
> And once memory set should also faster than 4 times copy operations.
> And add number 4 once should also faster than 4 times plus plus.

And did you test this to prove that this does matter and is noticable?
How do you know that gcc doesn't just optimize it all away?  Is this on
a code path that actually matters?

Don't ever say "improve performance" without actually being able to
prove it please.

> > thanks,
> > 
> > greg k-h
> > 
> > 
> > > 
> > > Signed-off-by: Rui Teng 
> > > ---
> > >  drivers/staging/wilc1000/host_interface.c | 7 +++
> > >  1 file changed, 3 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/staging/wilc1000/host_interface.c 
> > > b/drivers/staging/wilc1000/host_interface.c
> > > index 2568dfc15181..036c5c19a016 100644
> > > --- a/drivers/staging/wilc1000/host_interface.c
> > > +++ b/drivers/staging/wilc1000/host_interface.c
> > > @@ -2416,10 +2416,9 @@ static void Handle_SetMulticastFilter(struct 
> > > wilc_vif *vif,
> > >   goto ERRORHANDLER;
> > > 
> > >   pu8CurrByte = wid.val;
> > > - *pu8CurrByte++ = (strHostIfSetMulti->enabled & 0xFF);
> > > - *pu8CurrByte++ = ((strHostIfSetMulti->enabled >> 8) & 0xFF);
> > > - *pu8CurrByte++ = ((strHostIfSetMulti->enabled >> 16) & 0xFF);
> > > - *pu8CurrByte++ = ((strHostIfSetMulti->enabled >> 24) & 0xFF);
> > > + memset(pu8CurrByte, 0, 4);
> > > + *pu8CurrByte = (strHostIfSetMulti->enabled & 0xFF);
> > > + pu8CurrByte += 4;
> > 
> > Are you sure enabled isn't larger than 8 bits?
> 
> The size of bool may larger than 8 bits. But when we assign any value
> to bool type, the value of bool type will only be 1 or 0.
> For example, the following output will be 1 other than 0x100.
>   bool b = 0x100;
>   printf("b: %d\n", b);
> 
> Or I think it should change 'enabled' type from bool to u32, to make sure
> that the right shift operation can take effect.

I think we should ask the authors of this code what is expected here as
it is quite odd...

thanks,

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