[PATCH] staging: fsl-mc: fix coding stye errors

2017-01-17 Thread Dhananjay Balan
Split line at boolean operator.

Error was reported by checkpatch.pl as
WARNING: Avoid multiple line dereference - prefer 'mc_msi_domain->host_data'

Signed-off-by: Dhananjay Balan 
---
 drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c 
b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
index 6b1cd57..6b19a21 100644
--- a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
+++ b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
@@ -94,8 +94,8 @@ int __init its_fsl_mc_msi_init(void)
continue;
}
 
-   WARN_ON(mc_msi_domain->
-   host_data != &its_fsl_mc_msi_domain_info);
+   WARN_ON(mc_msi_domain->host_data !=
+   &its_fsl_mc_msi_domain_info);
 
pr_info("fsl-mc MSI: %s domain created\n", np->full_name);
}
-- 
2.7.4

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


Re: [PATCH] staging: fsl-mc: fix coding stye errors

2017-01-17 Thread Dan Carpenter
On Tue, Jan 17, 2017 at 09:11:28AM +0100, Dhananjay Balan wrote:
> Split line at boolean operator.
> 
> Error was reported by checkpatch.pl as
> WARNING: Avoid multiple line dereference - prefer 'mc_msi_domain->host_data'
> 
> Signed-off-by: Dhananjay Balan 
> ---
>  drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c 
> b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> index 6b1cd57..6b19a21 100644
> --- a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> +++ b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> @@ -94,8 +94,8 @@ int __init its_fsl_mc_msi_init(void)
>   continue;
>   }
>  
> - WARN_ON(mc_msi_domain->
> - host_data != &its_fsl_mc_msi_domain_info);
> + WARN_ON(mc_msi_domain->host_data !=
> + &its_fsl_mc_msi_domain_info);

Better to line it up like this:

WARN_ON(mc_msi_domain->host_data !=
&its_fsl_mc_msi_domain_info);

regards,
dan carpenter

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


[PATCH v2 07/15] staging: iio: isl29028: move failure logging into isl29028_set_als_scale()

2017-01-17 Thread Brian Masney
When isl29028_set_als_scale() fails, it was up to both callers to log
the failure message. This patch moves the logging into
isl29028_set_als_scale() to reduce the overall amount of code in the
driver.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/isl29028.c | 27 ++-
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index bd85ccb..8e7b3db 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -123,11 +123,22 @@ static int isl29028_enable_proximity(struct isl29028_chip 
*chip, bool enable)
 
 static int isl29028_set_als_scale(struct isl29028_chip *chip, int lux_scale)
 {
+   struct device *dev = regmap_get_device(chip->regmap);
int val = (lux_scale == 2000) ? ISL29028_CONF_ALS_RANGE_HIGH_LUX :
ISL29028_CONF_ALS_RANGE_LOW_LUX;
+   int ret;
+
+   ret = regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
+ISL29028_CONF_ALS_RANGE_MASK, val);
+   if (ret < 0) {
+   dev_err(dev, "%s(): Error %d setting the ALS scale\n", __func__,
+   ret);
+   return ret;
+   }
 
-   return regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
- ISL29028_CONF_ALS_RANGE_MASK, val);
+   chip->lux_scale = lux_scale;
+
+   return ret;
 }
 
 static int isl29028_set_als_ir_mode(struct isl29028_chip *chip,
@@ -318,13 +329,6 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
}
 
ret = isl29028_set_als_scale(chip, val);
-   if (ret < 0) {
-   dev_err(dev,
-   "Setting lux scale fail with error %d\n", ret);
-   break;
-   }
-
-   chip->lux_scale = val;
break;
default:
dev_err(dev, "Unsupported channel type\n");
@@ -443,10 +447,7 @@ static int isl29028_chip_init_and_power_on(struct 
isl29028_chip *chip)
if (ret < 0)
return ret;
 
-   ret = isl29028_set_als_scale(chip, chip->lux_scale);
-   if (ret < 0)
-   dev_err(dev, "setting als scale failed, err = %d\n", ret);
-   return ret;
+   return isl29028_set_als_scale(chip, chip->lux_scale);
 }
 
 static bool isl29028_is_volatile_reg(struct device *dev, unsigned int reg)
-- 
2.9.3

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


[PATCH v2 03/15] staging: iio: isl29028: combine isl29028_proxim_get() and isl29028_read_proxim()

2017-01-17 Thread Brian Masney
isl29028_proxim_get() checks to see if the promixity needs to be
enabled on the chip and then calls isl29028_read_proxim(). There
are no other callers of isl29028_read_proxim(). The naming between
these two functions can be confusing so this patch combines the
two to avoid the confusion.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/isl29028.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index 74eb736..a13c8db 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -201,6 +201,13 @@ static int isl29028_read_proxim(struct isl29028_chip 
*chip, int *prox)
unsigned int data;
int ret;
 
+   if (!chip->enable_prox) {
+   ret = isl29028_enable_proximity(chip, true);
+   if (ret < 0)
+   return ret;
+   chip->enable_prox = true;
+   }
+
ret = regmap_read(chip->regmap, ISL29028_REG_PROX_DATA, &data);
if (ret < 0) {
dev_err(dev, "Error in reading register %d, error %d\n",
@@ -211,19 +218,6 @@ static int isl29028_read_proxim(struct isl29028_chip 
*chip, int *prox)
return 0;
 }
 
-static int isl29028_proxim_get(struct isl29028_chip *chip, int *prox_data)
-{
-   int ret;
-
-   if (!chip->enable_prox) {
-   ret = isl29028_enable_proximity(chip, true);
-   if (ret < 0)
-   return ret;
-   chip->enable_prox = true;
-   }
-   return isl29028_read_proxim(chip, prox_data);
-}
-
 static int isl29028_als_get(struct isl29028_chip *chip, int *als_data)
 {
struct device *dev = regmap_get_device(chip->regmap);
@@ -349,7 +343,7 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
ret = isl29028_ir_get(chip, val);
break;
case IIO_PROXIMITY:
-   ret = isl29028_proxim_get(chip, val);
+   ret = isl29028_read_proxim(chip, val);
break;
default:
break;
-- 
2.9.3

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


[PATCH v2 00/15] staging cleanups

2017-01-17 Thread Brian Masney
This patch set contains more of my cleanups to the isl29028 light driver
in preparation for moving the driver out of staging. The main feature
introduced by this patch set is support for runtime power management
and autosuspend after two seconds. The rest of the patches are minor
code improvements, mostly style.

The ALS and IR portion of the sensor was tested using an ISL29028 hooked
up to a Raspberry Pi 2. In my v1 submission cover letter, I mention that
I am not able to use the proximity sensing function on my sensor. (This
is before any of my code changes.) I'm going to look into that issue
next. I suspect that it is a hardware issue on my local setup. I verified
with a visible light LED that the IRDR pin continues to be driven as
expected when a proximity reading is requested and is turned off after
two seconds of inactivity. The pin is driven the next time a proximity
reading is requested.

Datasheet:
http://www.intersil.com/content/dam/Intersil/documents/isl2/isl29028.pdf

Changes since version 1:
- Suggestions from Jonathan Cameron
  - In v1 patch #4 (#15 in this series), remove suspended flag. Added
support for runtime power management support and autosuspend.
  - Dropped v1 patch #19 (remove legacy device tree binding)
  - Dropped v1 patch #11 (made column alignment in isl29028_channels
consistent)
  - In v1 patches #13 and #14, if there is an error, return inside
the error check.
  - It was suggested to drop the v1 #2 patch that removes the enable
flag from isl29028_enable_proximity(). I think that we can apply
that patch now that the driver has runtime power management support.
- Suggestions from Dan Carpenter
  - Dropped v1 patch #12 (fix comparison between signed and unsigned
integers)

Brian Masney (15):
  staging: iio: isl29028: made alignment of variables in struct
isl29028_chip consistent
  staging: iio: isl29028: fix alignment of function arguments
  staging: iio: isl29028: combine isl29028_proxim_get() and
isl29028_read_proxim()
  staging: iio: isl29028: change newlines to improve readability
  staging: iio: isl29028: remove unused define ISL29028_DEV_ATTR
  staging: iio: isl29028: move failure logging into
isl29028_set_proxim_sampling()
  staging: iio: isl29028: move failure logging into
isl29028_set_als_scale()
  staging: iio: isl29028: made error messages consistent
  staging: iio: isl29028: remove unnecessary error logging in
isl29028_chip_init_and_power_on()
  staging: iio: isl29028: remove out of memory log message
  staging: iio: isl29028: remove unnecessary parenthesis
  staging: iio: isl29028: remove enable flag from
isl29028_enable_proximity()
  staging: iio: isl29028: only set proximity sampling rate when
proximity is enabled
  staging: iio: isl29028: only set ALS scale when ALS sensing is enabled
  staging: iio: isl29028: add runtime power management support

 drivers/staging/iio/light/isl29028.c | 323 ---
 1 file changed, 221 insertions(+), 102 deletions(-)

-- 
2.9.3

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


[PATCH v2 02/15] staging: iio: isl29028: fix alignment of function arguments

2017-01-17 Thread Brian Masney
Two separate calls to regmap_update_bits() in isl29028_set_als_scale()
and isl29028_set_als_ir_mode() did not have their function arguments
on the next line aligned correctly to the open parenthesis. This patch
corrects the alignment.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/isl29028.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index 4cce663..74eb736 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -116,7 +116,7 @@ static int isl29028_set_als_scale(struct isl29028_chip 
*chip, int lux_scale)
ISL29028_CONF_ALS_RANGE_LOW_LUX;
 
return regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
-   ISL29028_CONF_ALS_RANGE_MASK, val);
+ ISL29028_CONF_ALS_RANGE_MASK, val);
 }
 
 static int isl29028_set_als_ir_mode(struct isl29028_chip *chip,
@@ -148,7 +148,8 @@ static int isl29028_set_als_ir_mode(struct isl29028_chip 
*chip,
 
case ISL29028_MODE_NONE:
return regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
-   ISL29028_CONF_ALS_EN_MASK, ISL29028_CONF_ALS_DIS);
+ ISL29028_CONF_ALS_EN_MASK,
+ ISL29028_CONF_ALS_DIS);
}
 
if (ret < 0)
-- 
2.9.3

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


[PATCH v2 09/15] staging: iio: isl29028: remove unnecessary error logging in isl29028_chip_init_and_power_on()

2017-01-17 Thread Brian Masney
If the call to isl29028_chip_init_and_power_on() in isl29028_probe()
fails, then isl29028_probe() will log an error message. All of the
error paths in that call path already have error logging in place. This
patch removes the unnecessary logging.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/isl29028.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index cffecf9..be1fc4a 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -528,10 +528,8 @@ static int isl29028_probe(struct i2c_client *client,
}
 
ret = isl29028_chip_init_and_power_on(chip);
-   if (ret < 0) {
-   dev_err(&client->dev, "chip initialization failed: %d\n", ret);
+   if (ret < 0)
return ret;
-   }
 
indio_dev->info = &isl29028_info;
indio_dev->channels = isl29028_channels;
-- 
2.9.3

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


[PATCH v2 12/15] staging: iio: isl29028: remove enable flag from isl29028_enable_proximity()

2017-01-17 Thread Brian Masney
isl29028_enable_proximity() has a boolean argument named enable. This
function is only called once and the enable flag is set to true in that
call. This patch removes the enable parameter from that function.

Signed-off-by: Brian Masney 
---
The device gets runtime power management support in the next patch in
this set and autosuspends after two seconds of inactivity. Once the
device suspends, the pin that drives the external LED for proximity
sensing will only go high the next time that the user asks for a
reading from the proximity sensor.

This patch also sets the stage for additional cleanups prior to the
introduction of runtime power management support.

 drivers/staging/iio/light/isl29028.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index bc9c01d..f1b3651 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -103,15 +103,13 @@ static int isl29028_set_proxim_sampling(struct 
isl29028_chip *chip,
return ret;
 }
 
-static int isl29028_enable_proximity(struct isl29028_chip *chip, bool enable)
+static int isl29028_enable_proximity(struct isl29028_chip *chip)
 {
int ret;
-   int val = 0;
 
-   if (enable)
-   val = ISL29028_CONF_PROX_EN;
ret = regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
-ISL29028_CONF_PROX_EN_MASK, val);
+ISL29028_CONF_PROX_EN_MASK,
+ISL29028_CONF_PROX_EN);
if (ret < 0)
return ret;
 
@@ -225,7 +223,7 @@ static int isl29028_read_proxim(struct isl29028_chip *chip, 
int *prox)
int ret;
 
if (!chip->enable_prox) {
-   ret = isl29028_enable_proximity(chip, true);
+   ret = isl29028_enable_proximity(chip);
if (ret < 0)
return ret;
 
-- 
2.9.3

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


[PATCH v2 08/15] staging: iio: isl29028: made error messages consistent

2017-01-17 Thread Brian Masney
The wording and style of the different error messages was not
consistent. This patch makes the wording and style consistent
throughout the driver.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/isl29028.c | 56 
 1 file changed, 32 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index 8e7b3db..cffecf9 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -200,14 +200,16 @@ static int isl29028_read_als_ir(struct isl29028_chip 
*chip, int *als_ir)
ret = regmap_read(chip->regmap, ISL29028_REG_ALSIR_L, &lsb);
if (ret < 0) {
dev_err(dev,
-   "Error in reading register ALSIR_L err %d\n", ret);
+   "%s(): Error %d reading register ALSIR_L\n",
+   __func__, ret);
return ret;
}
 
ret = regmap_read(chip->regmap, ISL29028_REG_ALSIR_U, &msb);
if (ret < 0) {
dev_err(dev,
-   "Error in reading register ALSIR_U err %d\n", ret);
+   "%s(): Error %d reading register ALSIR_U\n",
+   __func__, ret);
return ret;
}
 
@@ -232,8 +234,8 @@ static int isl29028_read_proxim(struct isl29028_chip *chip, 
int *prox)
 
ret = regmap_read(chip->regmap, ISL29028_REG_PROX_DATA, &data);
if (ret < 0) {
-   dev_err(dev, "Error in reading register %d, error %d\n",
-   ISL29028_REG_PROX_DATA, ret);
+   dev_err(dev, "%s(): Error %d reading register PROX_DATA\n",
+   __func__, ret);
return ret;
}
 
@@ -250,7 +252,8 @@ static int isl29028_als_get(struct isl29028_chip *chip, int 
*als_data)
 
ret = isl29028_set_als_ir_mode(chip, ISL29028_MODE_ALS);
if (ret < 0) {
-   dev_err(dev, "Error in enabling ALS mode err %d\n", ret);
+   dev_err(dev, "%s(): Error %d enabling ALS mode\n", __func__,
+   ret);
return ret;
}
 
@@ -280,7 +283,8 @@ static int isl29028_ir_get(struct isl29028_chip *chip, int 
*ir_data)
 
ret = isl29028_set_als_ir_mode(chip, ISL29028_MODE_IR);
if (ret < 0) {
-   dev_err(dev, "Error in enabling IR mode err %d\n", ret);
+   dev_err(dev, "%s(): Error %d enabling IR mode\n", __func__,
+   ret);
return ret;
}
 
@@ -301,14 +305,15 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
case IIO_PROXIMITY:
if (mask != IIO_CHAN_INFO_SAMP_FREQ) {
dev_err(dev,
-   "proximity: mask value 0x%08lx not supported\n",
-   mask);
+   "%s(): proximity: Mask value 0x%08lx is not 
supported\n",
+   __func__, mask);
break;
}
 
if (val < 1 || val > 100) {
dev_err(dev,
-   "Samp_freq %d is not in range[1:100]\n", val);
+   "%s(): proximity: Sampling frequency %d is not 
in the range [1:100]\n",
+   __func__, val);
break;
}
 
@@ -317,21 +322,23 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
case IIO_LIGHT:
if (mask != IIO_CHAN_INFO_SCALE) {
dev_err(dev,
-   "light: mask value 0x%08lx not supported\n",
-   mask);
+   "%s(): light: Mask value 0x%08lx is not 
supported\n",
+   __func__, mask);
break;
}
 
if ((val != 125) && (val != 2000)) {
dev_err(dev,
-   "lux scale %d is invalid [125, 2000]\n", val);
+   "%s(): light: Lux scale %d is not in the set 
{125, 2000}\n",
+   __func__, val);
break;
}
 
ret = isl29028_set_als_scale(chip, val);
break;
default:
-   dev_err(dev, "Unsupported channel type\n");
+   dev_err(dev, "%s(): Unsupported channel type %x\n",
+   __func__, chan->type);
break;
}
 
@@ -385,7 +392,8 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
ret = IIO_VAL_INT;
break;
default:
-   dev_err(dev, "mask value 0x%08lx not supported\n", mask);
+   dev_err(dev, "%s(): mask value 0x%08lx is not supported\n",
+   __func__, mask);
break;
}
 
@@ -4

[PATCH v2 06/15] staging: iio: isl29028: move failure logging into isl29028_set_proxim_sampling()

2017-01-17 Thread Brian Masney
When isl29028_set_proxim_sampling() fails, it was up to both callers to
log the failure message. This patch moves the logging into
isl29028_set_proxim_sampling() to reduce the overall amount of code in
the driver.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/isl29028.c | 31 ---
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index 7074e62..bd85ccb 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -78,18 +78,29 @@ struct isl29028_chip {
 static int isl29028_set_proxim_sampling(struct isl29028_chip *chip,
unsigned int sampling)
 {
+   struct device *dev = regmap_get_device(chip->regmap);
static unsigned int prox_period[] = {800, 400, 200, 100, 75, 50, 12, 0};
-   int sel;
unsigned int period = DIV_ROUND_UP(1000, sampling);
+   int sel, ret;
 
for (sel = 0; sel < ARRAY_SIZE(prox_period); ++sel) {
if (period >= prox_period[sel])
break;
}
 
-   return regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
- ISL29028_CONF_PROX_SLP_MASK,
- sel << ISL29028_CONF_PROX_SLP_SH);
+   ret = regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
+ISL29028_CONF_PROX_SLP_MASK,
+sel << ISL29028_CONF_PROX_SLP_SH);
+
+   if (ret < 0) {
+   dev_err(dev, "%s(): Error %d setting the proximity sampling\n",
+   __func__, ret);
+   return ret;
+   }
+
+   chip->prox_sampling = sampling;
+
+   return ret;
 }
 
 static int isl29028_enable_proximity(struct isl29028_chip *chip, bool enable)
@@ -291,14 +302,6 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
}
 
ret = isl29028_set_proxim_sampling(chip, val);
-   if (ret < 0) {
-   dev_err(dev,
-   "Setting proximity samp_freq fail, err %d\n",
-   ret);
-   break;
-   }
-
-   chip->prox_sampling = val;
break;
case IIO_LIGHT:
if (mask != IIO_CHAN_INFO_SCALE) {
@@ -437,10 +440,8 @@ static int isl29028_chip_init_and_power_on(struct 
isl29028_chip *chip)
}
 
ret = isl29028_set_proxim_sampling(chip, chip->prox_sampling);
-   if (ret < 0) {
-   dev_err(dev, "setting the proximity, err = %d\n", ret);
+   if (ret < 0)
return ret;
-   }
 
ret = isl29028_set_als_scale(chip, chip->lux_scale);
if (ret < 0)
-- 
2.9.3

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


[PATCH v2 01/15] staging: iio: isl29028: made alignment of variables in struct isl29028_chip consistent

2017-01-17 Thread Brian Masney
The alignment of the variables in the struct isl29028_chip is not
consistent. This changes all of the variables to use consistent
alignment to improve the code readability.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/isl29028.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index 1de81f5..4cce663 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -67,13 +67,13 @@ enum isl29028_als_ir_mode {
 };
 
 struct isl29028_chip {
-   struct mutexlock;
-   struct regmap   *regmap;
+   struct mutexlock;
+   struct regmap   *regmap;
 
-   unsigned intprox_sampling;
-   boolenable_prox;
+   unsigned intprox_sampling;
+   boolenable_prox;
 
-   int lux_scale;
+   int lux_scale;
enum isl29028_als_ir_mode   als_ir_mode;
 };
 
-- 
2.9.3

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


[PATCH v2 11/15] staging: iio: isl29028: remove unnecessary parenthesis

2017-01-17 Thread Brian Masney
isl29028_write_raw() contains unnecessary parenthesis when checking to
see if the passed in lux scale is valid. This patch removes the
unnecessary parenthesis.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/isl29028.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index e93077b..bc9c01d 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -327,7 +327,7 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
break;
}
 
-   if ((val != 125) && (val != 2000)) {
+   if (val != 125 && val != 2000) {
dev_err(dev,
"%s(): light: Lux scale %d is not in the set 
{125, 2000}\n",
__func__, val);
-- 
2.9.3

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


[PATCH v2 14/15] staging: iio: isl29028: only set ALS scale when ALS/IR sensing is enabled

2017-01-17 Thread Brian Masney
isl29028_chip_init_and_power_on() calls isl29028_set_als_scale() and
this is not needed until the user actually needs to take a reading from
the ALS/IR sensor. This patch moves the isl29028_set_als_scale() call
from isl29028_chip_init_and_power_on() to isl29028_set_als_ir_mode().
This sets the stage for faster resume times from runtime power
management if the user is only querying the proximity sensor.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/isl29028.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index fa58d08..598a5a5 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -146,11 +146,15 @@ static int isl29028_set_als_scale(struct isl29028_chip 
*chip, int lux_scale)
 static int isl29028_set_als_ir_mode(struct isl29028_chip *chip,
enum isl29028_als_ir_mode mode)
 {
-   int ret = 0;
+   int ret;
 
if (chip->als_ir_mode == mode)
return 0;
 
+   ret = isl29028_set_als_scale(chip, chip->lux_scale);
+   if (ret < 0)
+   return ret;
+
switch (mode) {
case ISL29028_MODE_ALS:
ret = regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
@@ -453,7 +457,7 @@ static int isl29028_chip_init_and_power_on(struct 
isl29028_chip *chip)
return ret;
}
 
-   return isl29028_set_als_scale(chip, chip->lux_scale);
+   return ret;
 }
 
 static bool isl29028_is_volatile_reg(struct device *dev, unsigned int reg)
-- 
2.9.3

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


[PATCH v2 15/15] staging: iio: isl29028: add runtime power management support

2017-01-17 Thread Brian Masney
This patch adds runtime power management support to the isl29028 driver.
It defaults to powering off the device after two seconds of inactivity.

isl29028_chip_init_and_power_on() currently only zeros the CONFIGURE
register on the chip, which will cause the chip to turn off. This patch
also renames that function to isl29028_clear_configure_reg() since it is
now used in several places.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/isl29028.c | 118 ---
 1 file changed, 110 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index 598a5a5..a3264f7 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define ISL29028_CONV_TIME_MS  100
 
@@ -60,6 +61,8 @@
 
 #define ISL29028_NUM_REGS  (ISL29028_REG_TEST2_MODE + 1)
 
+#define ISL29028_POWER_OFF_DELAY_MS2000
+
 enum isl29028_als_ir_mode {
ISL29028_MODE_NONE = 0,
ISL29028_MODE_ALS,
@@ -297,6 +300,23 @@ static int isl29028_ir_get(struct isl29028_chip *chip, int 
*ir_data)
return isl29028_read_als_ir(chip, ir_data);
 }
 
+static int isl29028_set_pm_runtime_busy(struct isl29028_chip *chip, bool on)
+{
+   struct device *dev = regmap_get_device(chip->regmap);
+   int ret;
+
+   if (on) {
+   ret = pm_runtime_get_sync(dev);
+   if (ret < 0)
+   pm_runtime_put_noidle(dev);
+   } else {
+   pm_runtime_mark_last_busy(dev);
+   ret = pm_runtime_put_autosuspend(dev);
+   }
+
+   return ret;
+}
+
 /* Channel IO */
 static int isl29028_write_raw(struct iio_dev *indio_dev,
  struct iio_chan_spec const *chan,
@@ -304,9 +324,15 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 {
struct isl29028_chip *chip = iio_priv(indio_dev);
struct device *dev = regmap_get_device(chip->regmap);
-   int ret = -EINVAL;
+   int ret;
+
+   ret = isl29028_set_pm_runtime_busy(chip, true);
+   if (ret < 0)
+   return ret;
 
mutex_lock(&chip->lock);
+
+   ret = -EINVAL;
switch (chan->type) {
case IIO_PROXIMITY:
if (mask != IIO_CHAN_INFO_SAMP_FREQ) {
@@ -350,6 +376,13 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 
mutex_unlock(&chip->lock);
 
+   if (ret < 0)
+   return ret;
+
+   ret = isl29028_set_pm_runtime_busy(chip, false);
+   if (ret < 0)
+   return ret;
+
return ret;
 }
 
@@ -359,9 +392,15 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
 {
struct isl29028_chip *chip = iio_priv(indio_dev);
struct device *dev = regmap_get_device(chip->regmap);
-   int ret = -EINVAL;
+   int ret, pm_ret;
+
+   ret = isl29028_set_pm_runtime_busy(chip, true);
+   if (ret < 0)
+   return ret;
 
mutex_lock(&chip->lock);
+
+   ret = -EINVAL;
switch (mask) {
case IIO_CHAN_INFO_RAW:
case IIO_CHAN_INFO_PROCESSED:
@@ -405,6 +444,18 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
 
mutex_unlock(&chip->lock);
 
+   if (ret < 0)
+   return ret;
+
+   /**
+* Preserve the ret variable if the call to
+* isl29028_set_pm_runtime_busy() is successful so the reading
+* (if applicable) is returned to user space.
+*/
+   pm_ret = isl29028_set_pm_runtime_busy(chip, false);
+   if (pm_ret < 0)
+   return pm_ret;
+
return ret;
 }
 
@@ -445,17 +496,18 @@ static const struct iio_info isl29028_info = {
.write_raw = isl29028_write_raw,
 };
 
-static int isl29028_chip_init_and_power_on(struct isl29028_chip *chip)
+static int isl29028_clear_configure_reg(struct isl29028_chip *chip)
 {
struct device *dev = regmap_get_device(chip->regmap);
int ret;
 
ret = regmap_write(chip->regmap, ISL29028_REG_CONFIGURE, 0x0);
-   if (ret < 0) {
+   if (ret < 0)
dev_err(dev, "%s(): Error %d clearing the CONFIGURE register\n",
__func__, ret);
-   return ret;
-   }
+
+   chip->als_ir_mode = ISL29028_MODE_NONE;
+   chip->enable_prox = false;
 
return ret;
 }
@@ -509,7 +561,6 @@ static int isl29028_probe(struct i2c_client *client,
chip->enable_prox  = false;
chip->prox_sampling = 20;
chip->lux_scale = 2000;
-   chip->als_ir_mode = ISL29028_MODE_NONE;
 
ret = regmap_write(chip->regmap, ISL29028_REG_TEST1_MODE, 0x0);
if (ret < 0) {
@@ -527,7 +578,7 @@ static int isl29028_probe(struct i2c_client *client,
return ret;
}
 
-   ret = isl29028_chip_init_and_power_on(chip);
+   ret = isl29028_clear_configure_reg(chip);
if (re

[PATCH v2 13/15] staging: iio: isl29028: only set proximity sampling rate when proximity is enabled

2017-01-17 Thread Brian Masney
isl29028_chip_init_and_power_on() calls isl29028_set_proxim_sampling()
and this is not needed until the user actually needs to take a proximity
reading. This patch moves the isl29028_set_proxim_sampling() call from
isl29028_chip_init_and_power_on() to isl29028_enable_proximity().
This sets the stage for faster resume times from the runtime power
management if the user is only querying the ALS/IR sensor.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/isl29028.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index f1b3651..fa58d08 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -107,6 +107,10 @@ static int isl29028_enable_proximity(struct isl29028_chip 
*chip)
 {
int ret;
 
+   ret = isl29028_set_proxim_sampling(chip, chip->prox_sampling);
+   if (ret < 0)
+   return ret;
+
ret = regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
 ISL29028_CONF_PROX_EN_MASK,
 ISL29028_CONF_PROX_EN);
@@ -449,10 +453,6 @@ static int isl29028_chip_init_and_power_on(struct 
isl29028_chip *chip)
return ret;
}
 
-   ret = isl29028_set_proxim_sampling(chip, chip->prox_sampling);
-   if (ret < 0)
-   return ret;
-
return isl29028_set_als_scale(chip, chip->lux_scale);
 }
 
-- 
2.9.3

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


[PATCH v2 10/15] staging: iio: isl29028: remove out of memory log message

2017-01-17 Thread Brian Masney
If the call to devm_iio_device_alloc() fails, then isl29028_probe()
logs a message saying that memory cannot be allocated. The user's system
most likely has larger issues at this point. This patch removes that
error message since the error code is passed on and the message is not
necessary.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/isl29028.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index be1fc4a..e93077b 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -488,10 +488,8 @@ static int isl29028_probe(struct i2c_client *client,
int ret;
 
indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
-   if (!indio_dev) {
-   dev_err(&client->dev, "iio allocation fails\n");
+   if (!indio_dev)
return -ENOMEM;
-   }
 
chip = iio_priv(indio_dev);
 
-- 
2.9.3

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


Re: [PATCH] staging: fsl-mc: fix coding stye errors

2017-01-17 Thread Joe Perches
On Tue, 2017-01-17 at 11:27 +0300, Dan Carpenter wrote:
> On Tue, Jan 17, 2017 at 09:11:28AM +0100, Dhananjay Balan wrote:
> > Split line at boolean operator.
> > 
> > Error was reported by checkpatch.pl as
> > WARNING: Avoid multiple line dereference - prefer 'mc_msi_domain->host_data'
[]
> > diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c 
> > b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
[]
> > @@ -94,8 +94,8 @@ int __init its_fsl_mc_msi_init(void)
> > continue;
> > }
> >  
> > -   WARN_ON(mc_msi_domain->
> > -   host_data != &its_fsl_mc_msi_domain_info);
> > +   WARN_ON(mc_msi_domain->host_data !=
> > +   &its_fsl_mc_msi_domain_info);
> 
> Better to line it up like this:
> 
>   WARN_ON(mc_msi_domain->host_data !=
>   &its_fsl_mc_msi_domain_info);

At 81 chars, it might be better on a single line instead.

WARN_ON(mc_msi_domain->host_data != 
&its_fsl_mc_msi_domain_info);

It also might be better to shorten some of the identifiers.

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


[PATCH v2 04/15] staging: iio: isl29028: change newlines to improve readability

2017-01-17 Thread Brian Masney
Add and remove newlines to improve code readability in preparation for
moving the driver out of staging.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/isl29028.c | 33 -
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index a13c8db..d05d761 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -69,10 +69,8 @@ enum isl29028_als_ir_mode {
 struct isl29028_chip {
struct mutexlock;
struct regmap   *regmap;
-
unsigned intprox_sampling;
boolenable_prox;
-
int lux_scale;
enum isl29028_als_ir_mode   als_ir_mode;
 };
@@ -88,6 +86,7 @@ static int isl29028_set_proxim_sampling(struct isl29028_chip 
*chip,
if (period >= prox_period[sel])
break;
}
+
return regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
  ISL29028_CONF_PROX_SLP_MASK,
  sel << ISL29028_CONF_PROX_SLP_SH);
@@ -107,6 +106,7 @@ static int isl29028_enable_proximity(struct isl29028_chip 
*chip, bool enable)
 
/* Wait for conversion to be complete for first sample */
mdelay(DIV_ROUND_UP(1000, chip->prox_sampling));
+
return 0;
 }
 
@@ -139,13 +139,11 @@ static int isl29028_set_als_ir_mode(struct isl29028_chip 
*chip,
 ISL29028_CONF_ALS_RANGE_MASK,
 ISL29028_CONF_ALS_RANGE_HIGH_LUX);
break;
-
case ISL29028_MODE_IR:
ret = regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
 ISL29028_CONF_ALS_IR_MODE_MASK,
 ISL29028_CONF_ALS_IR_MODE_IR);
break;
-
case ISL29028_MODE_NONE:
return regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
  ISL29028_CONF_ALS_EN_MASK,
@@ -192,6 +190,7 @@ static int isl29028_read_als_ir(struct isl29028_chip *chip, 
int *als_ir)
}
 
*als_ir = ((msb & 0xF) << 8) | (lsb & 0xFF);
+
return 0;
 }
 
@@ -205,6 +204,7 @@ static int isl29028_read_proxim(struct isl29028_chip *chip, 
int *prox)
ret = isl29028_enable_proximity(chip, true);
if (ret < 0)
return ret;
+
chip->enable_prox = true;
}
 
@@ -214,7 +214,9 @@ static int isl29028_read_proxim(struct isl29028_chip *chip, 
int *prox)
ISL29028_REG_PROX_DATA, ret);
return ret;
}
+
*prox = data;
+
return 0;
 }
 
@@ -245,6 +247,7 @@ static int isl29028_als_get(struct isl29028_chip *chip, int 
*als_data)
als_ir_data = (als_ir_data * 49) / 100;
 
*als_data = als_ir_data;
+
return 0;
 }
 
@@ -258,6 +261,7 @@ static int isl29028_ir_get(struct isl29028_chip *chip, int 
*ir_data)
dev_err(dev, "Error in enabling IR mode err %d\n", ret);
return ret;
}
+
return isl29028_read_als_ir(chip, ir_data);
 }
 
@@ -279,11 +283,13 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
mask);
break;
}
+
if (val < 1 || val > 100) {
dev_err(dev,
"Samp_freq %d is not in range[1:100]\n", val);
break;
}
+
ret = isl29028_set_proxim_sampling(chip, val);
if (ret < 0) {
dev_err(dev,
@@ -291,9 +297,9 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
ret);
break;
}
+
chip->prox_sampling = val;
break;
-
case IIO_LIGHT:
if (mask != IIO_CHAN_INFO_SCALE) {
dev_err(dev,
@@ -301,25 +307,29 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
mask);
break;
}
+
if ((val != 125) && (val != 2000)) {
dev_err(dev,
"lux scale %d is invalid [125, 2000]\n", val);
break;
}
+
ret = isl29028_set_als_scale(chip, val);
if (ret < 0) {
dev_err(dev,
"Setting lux scale fail with error %d\n", ret);
break;
}
+
chip->lux_scale = val;
break;
-
default:
dev_err(dev, "Unsupported chan

[PATCH v2 05/15] staging: iio: isl29028: remove unused define ISL29028_DEV_ATTR

2017-01-17 Thread Brian Masney
The #define ISL29028_DEV_ATTR was not used so this patch removes the
unnecessary code.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/isl29028.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index d05d761..7074e62 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -391,7 +391,6 @@ static 
IIO_CONST_ATTR(in_proximity_sampling_frequency_available,
"1 3 5 10 13 20 83 100");
 static IIO_CONST_ATTR(in_illuminance_scale_available, "125 2000");
 
-#define ISL29028_DEV_ATTR(name) (&iio_dev_attr_##name.dev_attr.attr)
 #define ISL29028_CONST_ATTR(name) (&iio_const_attr_##name.dev_attr.attr)
 static struct attribute *isl29028_attributes[] = {
ISL29028_CONST_ATTR(in_proximity_sampling_frequency_available),
-- 
2.9.3

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


Re: [PATCH RFC] hv_utils: implement Hyper-V PTP source

2017-01-17 Thread Vitaly Kuznetsov
Thomas Gleixner  writes:

> On Fri, 13 Jan 2017, Vitaly Kuznetsov wrote:
>> With TimeSync version 4 protocol support we started updating system time
>> continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
>> there is a time sample from the host which triggers do_settimeofday[64]().
>> While the time from the host is very accurate such adjustments may cause
>> issues:
>> - Time is jumping forward and backward, some applications may misbehave.
>> - In case an NTP server runs in parallel and uses something else for time
>>   sync (network, PTP,...) system time will never converge.
>> - Systemd starts annoying you by printing "Time has been changed" every 5
>>   seconds to the system log.
>> 
>> Instead of doing in-kernel time adjustments offload the work to an
>> NTP client by exposing TimeSync messages as a PTP device. Users my now
>> decide what they want to use as a source.
>> 
>> I tested the solution with chrony, the config was:
>> 
>>  refclock PHC /dev/ptp0 poll 3 precision 1e-9
>> 
>> The result I'm seeing is accurate enough, the time delta between the guest
>> and the host is almost always within [-10us, +10us], the in-kernel solution
>> was giving us comparable results.
>> 
>> I also tried implementing PPS device instead of PTP by using not currently
>> used Hyper-V synthetic timers (we use only one of four for clockevent) but
>> with PPS source only chrony wasn't able to give me the required accuracy,
>> the delta often more that 100us.
>
> Makes sense. The PTP based solution is really nice!
>
>>  static void hv_set_host_time(struct work_struct *work)
>>  {
>>  struct adj_time_work*wrk;
>> -s64 host_tns;
>>  u64 newtime;
>>  struct timespec64 host_ts;
>
> Just a nitpick. Ordering variables in reverse fir tree (length) order:
>
>   struct adj_time_work *wrk;
>   struct timespec64 host_ts;
>   u64 newtime;
>
> makes is simpler to parse
>
>> +
>> +static struct {
>> +u64 host_time;
>> +u64 ref_time;
>> +spinlock_t lock;
>> +} host_ts;
>
> Another formatting nit. If you arrange the members in tabular fashion it
> becomes simpler to parse:
>
> static struct {
>   u64 host_time;
>   u64 ref_time;
>   spinlock_t  lock;
> } host_ts;
>
> Also the struct might do with some comment explaning that it is the storage
> for the PTP machinery,
>
>> +static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 adj_flags)
>>  {
>> +unsigned long flags;
>>  
>>  /*
>>   * This check is safe since we are executing in the
>>   * interrupt context and time synch messages arre always
>>   * delivered on the same CPU.
>>   */
>> -if (work_pending(&wrk.work))
>> -return;
>> +if (adj_flags & ICTIMESYNCFLAG_SYNC) {
>> +if (work_pending(&wrk.work))
>> +return;
>>  
>> -wrk.host_time = hosttime;
>> -wrk.ref_time = reftime;
>> -wrk.flags = flags;
>> -if ((flags & (ICTIMESYNCFLAG_SYNC | ICTIMESYNCFLAG_SAMPLE)) != 0) {
>> +wrk.host_time = hosttime;
>> +wrk.ref_time = reftime;
>> +wrk.flags = adj_flags;
>>  schedule_work(&wrk.work);
>> +} else {
>> +spin_lock_irqsave(&host_ts.lock, flags);
>> +host_ts.host_time = hosttime;
>> +
>> +if (ts_srv_version <= TS_VERSION_3)
>> +rdmsrl(HV_X64_MSR_TIME_REF_COUNT, host_ts.ref_time);
>
> I'm confused here. The reftime / hosttime pair is accurate at sampling time
> on the host. So why reading the MSR here? I'm certainly missing something,
> but then this wants to have a comment like the other one in
> get_timeadj_latency().

Old TimeSync (vesion < 4.0) protocol messages don't specify any
reference time so we're 'faking' it by saving the reference time when
the message was received on the guest. This, of course, reduces the
precision of the device as we have a delta between the time sample
generation and its reception on the guest but there is no way we can
calculate this delta. I'll put a comment.

>
>> +else
>> +host_ts.ref_time = reftime;
>> +spin_unlock_irqrestore(&host_ts.lock, flags);
>>  }
>>  }
>
> Other than that: Nice work!
>

Thanks, I'll incorporate all your feedback and post non-RFC version.

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


Re: [PATCH] staging: fsl-mc: fix coding stye errors

2017-01-17 Thread Dan Carpenter
On Tue, Jan 17, 2017 at 12:47:30AM -0800, Joe Perches wrote:
> On Tue, 2017-01-17 at 11:27 +0300, Dan Carpenter wrote:
> > On Tue, Jan 17, 2017 at 09:11:28AM +0100, Dhananjay Balan wrote:
> > > Split line at boolean operator.
> > > 
> > > Error was reported by checkpatch.pl as
> > > WARNING: Avoid multiple line dereference - prefer 
> > > 'mc_msi_domain->host_data'
> []
> > > diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c 
> > > b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> []
> > > @@ -94,8 +94,8 @@ int __init its_fsl_mc_msi_init(void)
> > >   continue;
> > >   }
> > >  
> > > - WARN_ON(mc_msi_domain->
> > > - host_data != &its_fsl_mc_msi_domain_info);
> > > + WARN_ON(mc_msi_domain->host_data !=
> > > + &its_fsl_mc_msi_domain_info);
> > 
> > Better to line it up like this:
> > 
> > WARN_ON(mc_msi_domain->host_data !=
> > &its_fsl_mc_msi_domain_info);
> 
> At 81 chars, it might be better on a single line instead.
> 

Yeah.  Except you can't fight checkpatch.pl...  In the long run it will
just wear you down for no good reason.

>   WARN_ON(mc_msi_domain->host_data != 
> &its_fsl_mc_msi_domain_info);
> 
> It also might be better to shorten some of the identifiers.

Yeah.  Shorter names are a great idea.  This driver went a bit crazy
with all the prefixes.  It's a static variable.

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


[PATCH v2 1/4] staging: greybus: audio: Avoid less than zero check for le32 variable

2017-01-17 Thread Vaibhav Agarwal
mixer control->info call back function checks for -ve values to rebase
min and max values. However, le32 variable is used to fetch values from
GB module FW. Thus -ve value checking is not required. Fix this!!

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_topology.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index 3001a4971c06..a8b63a8b2bb0 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -361,8 +361,8 @@ static int gbcodec_mixer_dapm_ctl_info(struct snd_kcontrol 
*kcontrol,
info = (struct gb_audio_ctl_elem_info *)data->info;
 
/* update uinfo */
-   platform_max = info->value.integer.max;
-   platform_min = info->value.integer.min;
+   platform_max = le32_to_cpu(info->value.integer.max);
+   platform_min = le32_to_cpu(info->value.integer.min);
 
if (platform_max == 1 &&
!strnstr(kcontrol->id.name, " Volume", NAME_SIZE))
@@ -371,12 +371,8 @@ static int gbcodec_mixer_dapm_ctl_info(struct snd_kcontrol 
*kcontrol,
uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
 
uinfo->count = data->vcount;
-   uinfo->value.integer.min = 0;
-   if (info->value.integer.min < 0 &&
-   (uinfo->type == SNDRV_CTL_ELEM_TYPE_INTEGER))
-   uinfo->value.integer.max = platform_max - platform_min;
-   else
-   uinfo->value.integer.max = platform_max;
+   uinfo->value.integer.min = platform_min;
+   uinfo->value.integer.max = platform_max;
 
return 0;
 }
-- 
2.7.4

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


[PATCH v2 0/4] Cleanup greybus audio driver

2017-01-17 Thread Vaibhav Agarwal
This patch series include following cleanup changes in GB Audio driver.
- Avoid unnecessary checks for le32 variables
- Initialize sig_bits before configuring hw_params
- Remove junk codec register mapping
- Ensure proper byte ordering

Next task is to enable build for GB codec driver. However this requires
pushing some changes in ASoC framework. Possibly in another two weeks
(based on my freetime), I'll try to submit those changes to ASoC mailing
list. And once the same are accepted there, I'll share relevant patches
for GB Audio codec driver as well.

Changes from v1:
- Include review comments from Dan


Vaibhav Agarwal (4):
  staging: greybus: audio: Avoid less than zero check for le32 variable
  staging: greybus: audio: Initialize sig_bits before configuring
hwparams
  staging: greybus: audio: Cleanup junk codec registers
  staging: greybus: audio: Ensure proper byte order

 drivers/staging/greybus/audio_codec.c|  46 +++---
 drivers/staging/greybus/audio_module.c   |   2 +-
 drivers/staging/greybus/audio_topology.c | 100 ---
 3 files changed, 61 insertions(+), 87 deletions(-)

-- 
2.7.4

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


[PATCH v2 3/4] staging: greybus: audio: Cleanup junk codec registers

2017-01-17 Thread Vaibhav Agarwal
From: Vaibhav Agarwal 

Dummy codec register were initially added while populating dummy codec
mixer controls until module topology parser was available. Now, these
dummy registers are nowhere used and thus can be safely removed.

Since ASoC framework requires a valid callback for both read & write
register APIS, currently empty placeholders are kept to avoid panic.

Later, register mapping logic can be defined:
1. Assuming fixed number of maximum modules connected and register bits
corresponds to basic info of each module OR
2. With a logic to dynamically grow register_cache_size based on codec
modules added/removed.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_codec.c | 39 ++-
 1 file changed, 2 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index b9d66278ff87..30941f9e380d 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -1026,47 +1026,16 @@ static int gbcodec_remove(struct snd_soc_codec *codec)
return 0;
 }
 
-static u8 gbcodec_reg[GBCODEC_REG_COUNT] = {
-   [GBCODEC_CTL_REG] = GBCODEC_CTL_REG_DEFAULT,
-   [GBCODEC_MUTE_REG] = GBCODEC_MUTE_REG_DEFAULT,
-   [GBCODEC_PB_LVOL_REG] = GBCODEC_PB_VOL_REG_DEFAULT,
-   [GBCODEC_PB_RVOL_REG] = GBCODEC_PB_VOL_REG_DEFAULT,
-   [GBCODEC_CAP_LVOL_REG] = GBCODEC_CAP_VOL_REG_DEFAULT,
-   [GBCODEC_CAP_RVOL_REG] = GBCODEC_CAP_VOL_REG_DEFAULT,
-   [GBCODEC_APB1_MUX_REG] = GBCODEC_APB1_MUX_REG_DEFAULT,
-   [GBCODEC_APB2_MUX_REG] = GBCODEC_APB2_MUX_REG_DEFAULT,
-};
-
 static int gbcodec_write(struct snd_soc_codec *codec, unsigned int reg,
 unsigned int value)
 {
-   int ret = 0;
-
-   if (reg == SND_SOC_NOPM)
-   return 0;
-
-   BUG_ON(reg >= GBCODEC_REG_COUNT);
-
-   gbcodec_reg[reg] = value;
-   dev_dbg(codec->dev, "reg[%d] = 0x%x\n", reg, value);
-
-   return ret;
+   return 0;
 }
 
 static unsigned int gbcodec_read(struct snd_soc_codec *codec,
 unsigned int reg)
 {
-   unsigned int val = 0;
-
-   if (reg == SND_SOC_NOPM)
-   return 0;
-
-   BUG_ON(reg >= GBCODEC_REG_COUNT);
-
-   val = gbcodec_reg[reg];
-   dev_dbg(codec->dev, "reg[%d] = 0x%x\n", reg, val);
-
-   return val;
+   return 0;
 }
 
 static struct snd_soc_codec_driver soc_codec_dev_gbaudio = {
@@ -1076,10 +1045,6 @@ static struct snd_soc_codec_driver soc_codec_dev_gbaudio 
= {
.read = gbcodec_read,
.write = gbcodec_write,
 
-   .reg_cache_size = GBCODEC_REG_COUNT,
-   .reg_cache_default = gbcodec_reg_defaults,
-   .reg_word_size = 1,
-
.idle_bias_off = true,
.ignore_pmdown_time = 1,
 };
-- 
2.7.4

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


[PATCH v2 2/4] staging: greybus: audio: Initialize sig_bits before configuring hwparams

2017-01-17 Thread Vaibhav Agarwal
From: Vaibhav Agarwal 

Uninitialized variable sig_bits was used while configuring stream params
for codec module. These params are used to configure PCM settings for
APBridgeA.

Usually, this is dependent on codec capability and thus populated via
codec dai_driver definition. In our case, it is fixed to 16 based on the
data format, container supported.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_codec.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index f8862c6d7102..b9d66278ff87 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -496,6 +496,11 @@ static int gbcodec_hw_params(struct snd_pcm_substream 
*substream,
 
gb_pm_runtime_put_noidle(bundle);
 
+   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+   sig_bits = dai->driver->playback.sig_bits;
+   else
+   sig_bits = dai->driver->capture.sig_bits;
+
params->state = GBAUDIO_CODEC_HWPARAMS;
params->format = format;
params->rate = rate;
@@ -689,6 +694,7 @@ static struct snd_soc_dai_driver gbaudio_dai[] = {
.rate_min = 48000,
.channels_min = 1,
.channels_max = 2,
+   .sig_bits = 16,
},
.capture = {
.stream_name = "I2S 0 Capture",
@@ -698,6 +704,7 @@ static struct snd_soc_dai_driver gbaudio_dai[] = {
.rate_min = 48000,
.channels_min = 1,
.channels_max = 2,
+   .sig_bits = 16,
},
.ops = &gbcodec_dai_ops,
},
-- 
2.7.4

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


[PATCH v2 4/4] staging: greybus: audio: Ensure proper byte order

2017-01-17 Thread Vaibhav Agarwal
From: Vaibhav Agarwal 

Proper byte order was completely disregarded for multi byte data shared
between AP and module (and APB1). Fix this.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_module.c   |  2 +-
 drivers/staging/greybus/audio_topology.c | 88 +---
 2 files changed, 48 insertions(+), 42 deletions(-)

diff --git a/drivers/staging/greybus/audio_module.c 
b/drivers/staging/greybus/audio_module.c
index 17a9948b1ba1..094c3be79b33 100644
--- a/drivers/staging/greybus/audio_module.c
+++ b/drivers/staging/greybus/audio_module.c
@@ -134,7 +134,7 @@ static int gbaudio_request_stream(struct 
gbaudio_module_info *module,
  struct gb_audio_streaming_event_request *req)
 {
dev_warn(module->dev, "Audio Event received: cport: %u, event: %u\n",
-req->data_cport, req->event);
+le16_to_cpu(req->data_cport), req->event);
 
return 0;
 }
diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index a8b63a8b2bb0..4002a57977bd 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -141,13 +141,14 @@ static const char **gb_generate_enum_strings(struct 
gbaudio_module_info *gb,
 {
const char **strings;
int i;
+   unsigned int items;
__u8 *data;
 
-   strings = devm_kzalloc(gb->dev, sizeof(char *) * gbenum->items,
-  GFP_KERNEL);
+   items = le32_to_cpu(gbenum->items);
+   strings = devm_kzalloc(gb->dev, sizeof(char *) * items, GFP_KERNEL);
data = gbenum->names;
 
-   for (i = 0; i < gbenum->items; i++) {
+   for (i = 0; i < items; i++) {
strings[i] = (const char *)data;
while (*data != '\0')
data++;
@@ -185,11 +186,11 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
switch (info->type) {
case GB_AUDIO_CTL_ELEM_TYPE_BOOLEAN:
case GB_AUDIO_CTL_ELEM_TYPE_INTEGER:
-   uinfo->value.integer.min = info->value.integer.min;
-   uinfo->value.integer.max = info->value.integer.max;
+   uinfo->value.integer.min = le32_to_cpu(info->value.integer.min);
+   uinfo->value.integer.max = le32_to_cpu(info->value.integer.max);
break;
case GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED:
-   max = info->value.enumerated.items;
+   max = le32_to_cpu(info->value.enumerated.items);
uinfo->value.enumerated.items = max;
if (uinfo->value.enumerated.item > max - 1)
uinfo->value.enumerated.item = max - 1;
@@ -249,17 +250,17 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
case GB_AUDIO_CTL_ELEM_TYPE_BOOLEAN:
case GB_AUDIO_CTL_ELEM_TYPE_INTEGER:
ucontrol->value.integer.value[0] =
-   gbvalue.value.integer_value[0];
+   le32_to_cpu(gbvalue.value.integer_value[0]);
if (data->vcount == 2)
ucontrol->value.integer.value[1] =
-   gbvalue.value.integer_value[1];
+   le32_to_cpu(gbvalue.value.integer_value[1]);
break;
case GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED:
ucontrol->value.enumerated.item[0] =
-   gbvalue.value.enumerated_item[0];
+   le32_to_cpu(gbvalue.value.enumerated_item[0]);
if (data->vcount == 2)
ucontrol->value.enumerated.item[1] =
-   gbvalue.value.enumerated_item[1];
+   le32_to_cpu(gbvalue.value.enumerated_item[1]);
break;
default:
dev_err(codec->dev, "Invalid type: %d for %s:kcontrol\n",
@@ -296,17 +297,17 @@ static int gbcodec_mixer_ctl_put(struct snd_kcontrol 
*kcontrol,
case GB_AUDIO_CTL_ELEM_TYPE_BOOLEAN:
case GB_AUDIO_CTL_ELEM_TYPE_INTEGER:
gbvalue.value.integer_value[0] =
-   ucontrol->value.integer.value[0];
+   cpu_to_le32(ucontrol->value.integer.value[0]);
if (data->vcount == 2)
gbvalue.value.integer_value[1] =
-   ucontrol->value.integer.value[1];
+   cpu_to_le32(ucontrol->value.integer.value[1]);
break;
case GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED:
gbvalue.value.enumerated_item[0] =
-   ucontrol->value.enumerated.item[0];
+   cpu_to_le32(ucontrol->value.enumerated.item[0]);
if (data->vcount == 2)
gbvalue.value.enumerated_item[1] =
-   ucontrol->value.enumerated.item[1];
+   cpu_to_

[PATCH v3 1/2] hv_util: switch to using timespec64

2017-01-17 Thread Vitaly Kuznetsov
do_settimeofday() is deprecated, use do_settimeofday64() instead.

Signed-off-by: Vitaly Kuznetsov 
Acked-by: John Stultz 
---
 drivers/hv/hv_util.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index e770774..94719eb 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -184,7 +184,7 @@ static void hv_set_host_time(struct work_struct *work)
struct adj_time_work*wrk;
s64 host_tns;
u64 newtime;
-   struct timespec host_ts;
+   struct timespec64 host_ts;
 
wrk = container_of(work, struct adj_time_work, work);
 
@@ -203,9 +203,9 @@ static void hv_set_host_time(struct work_struct *work)
newtime += (current_tick - wrk->ref_time);
}
host_tns = (newtime - WLTIMEDELTA) * 100;
-   host_ts = ns_to_timespec(host_tns);
+   host_ts = ns_to_timespec64(host_tns);
 
-   do_settimeofday(&host_ts);
+   do_settimeofday64(&host_ts);
 }
 
 /*
-- 
2.9.3

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


[PATCH v3 2/2] hv_utils: implement Hyper-V PTP source

2017-01-17 Thread Vitaly Kuznetsov
With TimeSync version 4 protocol support we started updating system time
continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
there is a time sample from the host which triggers do_settimeofday[64]().
While the time from the host is very accurate such adjustments may cause
issues:
- Time is jumping forward and backward, some applications may misbehave.
- In case an NTP server runs in parallel and uses something else for time
  sync (network, PTP,...) system time will never converge.
- Systemd starts annoying you by printing "Time has been changed" every 5
  seconds to the system log.

Instead of doing in-kernel time adjustments offload the work to an
NTP client by exposing TimeSync messages as a PTP device. Users may now
decide what they want to use as a source.

I tested the solution with chrony, the config was:

 refclock PHC /dev/ptp0 poll 3 precision 1e-9

The result I'm seeing is accurate enough, the time delta between the guest
and the host is almost always within [-10us, +10us], the in-kernel solution
was giving us comparable results.

I also tried implementing PPS device instead of PTP by using not currently
used Hyper-V synthetic timers (we use only one of four for clockevent) but
with PPS source only chrony wasn't able to give me the required accuracy,
the delta often more that 100us.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/hv_util.c | 140 ++-
 1 file changed, 115 insertions(+), 25 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 94719eb..e49c5f3 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "hyperv_vmbus.h"
 
@@ -179,31 +180,35 @@ struct adj_time_work {
u8  flags;
 };
 
+static inline u64 get_timeadj_latency(u64 ref_time)
+{
+   u64 current_tick;
+
+   if (ts_srv_version <= TS_VERSION_3)
+   return 0;
+
+   /*
+* Some latency has been introduced since Hyper-V generated
+* its time sample. Take that latency into account before
+* using TSC reference time sample from Hyper-V.
+*
+* This sample is given by TimeSync v4 and above hosts.
+*/
+
+   rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
+   return current_tick - ref_time;
+}
+
 static void hv_set_host_time(struct work_struct *work)
 {
struct adj_time_work*wrk;
-   s64 host_tns;
-   u64 newtime;
struct timespec64 host_ts;
+   u64 newtime;
 
wrk = container_of(work, struct adj_time_work, work);
 
-   newtime = wrk->host_time;
-   if (ts_srv_version > TS_VERSION_3) {
-   /*
-* Some latency has been introduced since Hyper-V generated
-* its time sample. Take that latency into account before
-* using TSC reference time sample from Hyper-V.
-*
-* This sample is given by TimeSync v4 and above hosts.
-*/
-   u64 current_tick;
-
-   rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
-   newtime += (current_tick - wrk->ref_time);
-   }
-   host_tns = (newtime - WLTIMEDELTA) * 100;
-   host_ts = ns_to_timespec64(host_tns);
+   newtime = wrk->host_time + get_timeadj_latency(wrk->ref_time);
+   host_ts = ns_to_timespec64((newtime - WLTIMEDELTA) * 100);
 
do_settimeofday64(&host_ts);
 }
@@ -222,22 +227,52 @@ static void hv_set_host_time(struct work_struct *work)
  * to discipline the clock.
  */
 static struct adj_time_work  wrk;
-static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 flags)
+
+/*
+ * The last time sample, received from the host. PTP device responds to
+ * requests by using this data and the current partition-wide time reference
+ * count.
+ */
+static struct {
+   u64 host_time;
+   u64 ref_time;
+   spinlock_t  lock;
+} host_ts;
+
+static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 adj_flags)
 {
+   unsigned long flags;
 
/*
 * This check is safe since we are executing in the
 * interrupt context and time synch messages arre always
 * delivered on the same CPU.
 */
-   if (work_pending(&wrk.work))
-   return;
+   if (adj_flags & ICTIMESYNCFLAG_SYNC) {
+   if (work_pending(&wrk.work))
+   return;
 
-   wrk.host_time = hosttime;
-   wrk.ref_time = reftime;
-   wrk.flags = flags;
-   if ((flags & (ICTIMESYNCFLAG_SYNC | ICTIMESYNCFLAG_SAMPLE)) != 0) {
+   wrk.host_time = hosttime;
+   wrk.ref_time = reftime;
+   wrk.flags = adj_flags;
schedule_work(&wrk.work);
+   } else {
+   spin_lock_irqsave(&host_ts.lock, flags);
+   host_ts.host_time = hosttime;
+
+   /*
+* Prior t

[PATCH v3 0/2] hv_util: adjust system time smoothly

2017-01-17 Thread Vitaly Kuznetsov
With TimeSync version 4 protocol support we started updating system time
continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
there is a time sample from the host which triggers do_settimeofday[64]().
While the time from the host is very accurate such adjustments may cause
issues:
- Time is jumping forward and backward, some applications may misbehave.
- In case an NTP server runs in parallel and uses something else for time
  sync (network, PTP,...) system time will never converge.
- Systemd starts annoying you by printing "Time has been changed" every 5
  seconds to the system log.

Instead of doing in-kernel time adjustments offload the work to an
NTP client by exposing TimeSync messages as a PTP device. Users may now
decide what they want to use as a source.

Changes since v2:
- Implement Hyper-V PTP device instead of doint in-kernel time sync.

Changes since "[PATCH RFC] hv_utils: implement Hyper-V PTP source":
- Richard Cochran: implement .adjfreq, .adjtime, .settime64 returning
  -EOPNOTSUPP.
- Olaf Hering: change IS_ERR->IS_ERR_OR_NULL as CONFIG_PTP_1588_CLOCK
  can be disabled.
- Thomas Gleixner: formatting fixes, comments added.

Vitaly Kuznetsov (2):
  hv_util: switch to using timespec64
  hv_utils: implement Hyper-V PTP source

 drivers/hv/hv_util.c | 142 +--
 1 file changed, 116 insertions(+), 26 deletions(-)

-- 
2.9.3

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


Re: [PATCH v3 0/2] hv_util: adjust system time smoothly

2017-01-17 Thread Stephen Hemminger
On Tue, 17 Jan 2017 16:27:17 +0100
Vitaly Kuznetsov  wrote:

> With TimeSync version 4 protocol support we started updating system time
> continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
> there is a time sample from the host which triggers do_settimeofday[64]().
> While the time from the host is very accurate such adjustments may cause
> issues:
> - Time is jumping forward and backward, some applications may misbehave.
> - In case an NTP server runs in parallel and uses something else for time
>   sync (network, PTP,...) system time will never converge.
> - Systemd starts annoying you by printing "Time has been changed" every 5
>   seconds to the system log.
> 
> Instead of doing in-kernel time adjustments offload the work to an
> NTP client by exposing TimeSync messages as a PTP device. Users may now
> decide what they want to use as a source.
> 
> Changes since v2:
> - Implement Hyper-V PTP device instead of doint in-kernel time sync.
> 
> Changes since "[PATCH RFC] hv_utils: implement Hyper-V PTP source":
> - Richard Cochran: implement .adjfreq, .adjtime, .settime64 returning
>   -EOPNOTSUPP.
> - Olaf Hering: change IS_ERR->IS_ERR_OR_NULL as CONFIG_PTP_1588_CLOCK
>   can be disabled.
> - Thomas Gleixner: formatting fixes, comments added.
> 
> Vitaly Kuznetsov (2):
>   hv_util: switch to using timespec64
>   hv_utils: implement Hyper-V PTP source
> 
>  drivers/hv/hv_util.c | 142 
> +--
>  1 file changed, 116 insertions(+), 26 deletions(-)
> 

It would be good to update Documentation files to describe any configuration 
needed.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3 2/2] hv_utils: implement Hyper-V PTP source

2017-01-17 Thread Stephen Hemminger
On Tue, 17 Jan 2017 16:27:19 +0100
Vitaly Kuznetsov  wrote:

> With TimeSync version 4 protocol support we started updating system time
> continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
> there is a time sample from the host which triggers do_settimeofday[64]().
> While the time from the host is very accurate such adjustments may cause
> issues:
> - Time is jumping forward and backward, some applications may misbehave.
> - In case an NTP server runs in parallel and uses something else for time
>   sync (network, PTP,...) system time will never converge.
> - Systemd starts annoying you by printing "Time has been changed" every 5
>   seconds to the system log.
> 
> Instead of doing in-kernel time adjustments offload the work to an
> NTP client by exposing TimeSync messages as a PTP device. Users may now
> decide what they want to use as a source.
> 
> I tested the solution with chrony, the config was:
> 
>  refclock PHC /dev/ptp0 poll 3 precision 1e-9
> 
> The result I'm seeing is accurate enough, the time delta between the guest
> and the host is almost always within [-10us, +10us], the in-kernel solution
> was giving us comparable results.
> 
> I also tried implementing PPS device instead of PTP by using not currently
> used Hyper-V synthetic timers (we use only one of four for clockevent) but
> with PPS source only chrony wasn't able to give me the required accuracy,
> the delta often more that 100us.
> 
> Signed-off-by: Vitaly Kuznetsov 

Looks good. Minor style comments.

> ---
>  drivers/hv/hv_util.c | 140 
> ++-
>  1 file changed, 115 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index 94719eb..e49c5f3 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c

 
> +static inline u64 get_timeadj_latency(u64 ref_time)

inline not necessary on static functions. GCC inlines anyway

> +{
> + u64 current_tick;
> +
> + if (ts_srv_version <= TS_VERSION_3)
> + return 0;
> +
> + /*
> +  * Some latency has been introduced since Hyper-V generated
> +  * its time sample. Take that latency into account before
> +  * using TSC reference time sample from Hyper-V.
> +  *
> +  * This sample is given by TimeSync v4 and above hosts.
> +  */
> +
> + rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);

Personal preference is not to add blank line between comment
and associated code.

...

> +
> +struct ptp_clock_info ptp_hyperv_info = {

This could be static?
Could it be const?

> + .name   = "hyperv",
> + .enable = hv_ptp_enable,
> + .adjtime= hv_ptp_adjtime,
> + .adjfreq= hv_ptp_adjfreq,
> + .gettime64  = hv_ptp_gettime,
> + .settime64  = hv_ptp_settime,
> + .owner  = THIS_MODULE,
> +};
> +
> +static struct ptp_clock *hv_ptp_clock;
> +
>  static int hv_timesync_init(struct hv_util_service *srv)
>  {
>   INIT_WORK(&wrk.work, hv_set_host_time);
> +
> + hv_ptp_clock = ptp_clock_register(&ptp_hyperv_info, NULL);
> + if (IS_ERR_OR_NULL(hv_ptp_clock)) {
> + pr_err("cannot register PTP clock: %ld\n",
> +PTR_ERR(hv_ptp_clock));

Why not return error to init routine in case of failure.

> + hv_ptp_clock = NULL;

Why not return error to init routine?  Rather than having user
scan log.

> + }
> +
>   return 0;
>  }
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3 2/2] hv_utils: implement Hyper-V PTP source

2017-01-17 Thread Vitaly Kuznetsov
Stephen Hemminger  writes:

> On Tue, 17 Jan 2017 16:27:19 +0100
> Vitaly Kuznetsov  wrote:
>
>> With TimeSync version 4 protocol support we started updating system time
>> continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
>> there is a time sample from the host which triggers do_settimeofday[64]().
>> While the time from the host is very accurate such adjustments may cause
>> issues:
>> - Time is jumping forward and backward, some applications may misbehave.
>> - In case an NTP server runs in parallel and uses something else for time
>>   sync (network, PTP,...) system time will never converge.
>> - Systemd starts annoying you by printing "Time has been changed" every 5
>>   seconds to the system log.
>> 
>> Instead of doing in-kernel time adjustments offload the work to an
>> NTP client by exposing TimeSync messages as a PTP device. Users may now
>> decide what they want to use as a source.
>> 
>> I tested the solution with chrony, the config was:
>> 
>>  refclock PHC /dev/ptp0 poll 3 precision 1e-9
>> 
>> The result I'm seeing is accurate enough, the time delta between the guest
>> and the host is almost always within [-10us, +10us], the in-kernel solution
>> was giving us comparable results.
>> 
>> I also tried implementing PPS device instead of PTP by using not currently
>> used Hyper-V synthetic timers (we use only one of four for clockevent) but
>> with PPS source only chrony wasn't able to give me the required accuracy,
>> the delta often more that 100us.
>> 
>> Signed-off-by: Vitaly Kuznetsov 
>
> Looks good. Minor style comments.
>
>> ---
>>  drivers/hv/hv_util.c | 140 
>> ++-
>>  1 file changed, 115 insertions(+), 25 deletions(-)
>> 
>> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
>> index 94719eb..e49c5f3 100644
>> --- a/drivers/hv/hv_util.c
>> +++ b/drivers/hv/hv_util.c
>
>> +static inline u64 get_timeadj_latency(u64 ref_time)
>
> inline not necessary on static functions. GCC inlines anyway
>

Even when we have multiple call sites? Interesting...

>> +{
>> +u64 current_tick;
>> +
>> +if (ts_srv_version <= TS_VERSION_3)
>> +return 0;
>> +
>> +/*
>> + * Some latency has been introduced since Hyper-V generated
>> + * its time sample. Take that latency into account before
>> + * using TSC reference time sample from Hyper-V.
>> + *
>> + * This sample is given by TimeSync v4 and above hosts.
>> + */
>> +
>> +rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
>
> Personal preference is not to add blank line between comment
> and associated code.
>
> ...
>

Ok.

>> +
>> +struct ptp_clock_info ptp_hyperv_info = {
>
> This could be static?
> Could it be const?
>

Could be both I think.

>> +.name   = "hyperv",
>> +.enable = hv_ptp_enable,
>> +.adjtime= hv_ptp_adjtime,
>> +.adjfreq= hv_ptp_adjfreq,
>> +.gettime64  = hv_ptp_gettime,
>> +.settime64  = hv_ptp_settime,
>> +.owner  = THIS_MODULE,
>> +};
>> +
>> +static struct ptp_clock *hv_ptp_clock;
>> +
>>  static int hv_timesync_init(struct hv_util_service *srv)
>>  {
>>  INIT_WORK(&wrk.work, hv_set_host_time);
>> +
>> +hv_ptp_clock = ptp_clock_register(&ptp_hyperv_info, NULL);
>> +if (IS_ERR_OR_NULL(hv_ptp_clock)) {
>> +pr_err("cannot register PTP clock: %ld\n",
>> +   PTR_ERR(hv_ptp_clock));
>
> Why not return error to init routine in case of failure.
>
>> +hv_ptp_clock = NULL;
>
> Why not return error to init routine?  Rather than having user
> scan log.
>

The idea here was to not depend on CONFIG_PTP_1588_CLOCK. In case
CONFIG_PTP_1588_CLOCK is disabled ptp_clock_register() will return NULL
but the Hyper-V timesync driver remains functional - it still handles
the ICTIMESYNCFLAG_SYNC case, just the ptp device will be missing.
We can:
1) Put PTP-related code under #ifdef CONFIG_PTP_1588_CLOCK
2) Handle errors and NULL returned from ptp_clock_register() differently,
   fail init in case we get an error and continue in case we see NULL.
3) Leave things as they are.
4) Always require CONFIG_PTP_1588_CLOCK.

My personal preference would be 2 or 3. What do you think?

>> +}
>> +
>>  return 0;
>>  }

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


Re: [PATCH v3 0/2] hv_util: adjust system time smoothly

2017-01-17 Thread Vitaly Kuznetsov
Stephen Hemminger  writes:

> On Tue, 17 Jan 2017 16:27:17 +0100
> Vitaly Kuznetsov  wrote:
>
>> With TimeSync version 4 protocol support we started updating system time
>> continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
>> there is a time sample from the host which triggers do_settimeofday[64]().
>> While the time from the host is very accurate such adjustments may cause
>> issues:
>> - Time is jumping forward and backward, some applications may misbehave.
>> - In case an NTP server runs in parallel and uses something else for time
>>   sync (network, PTP,...) system time will never converge.
>> - Systemd starts annoying you by printing "Time has been changed" every 5
>>   seconds to the system log.
>> 
>> Instead of doing in-kernel time adjustments offload the work to an
>> NTP client by exposing TimeSync messages as a PTP device. Users may now
>> decide what they want to use as a source.
>> 
>> Changes since v2:
>> - Implement Hyper-V PTP device instead of doint in-kernel time sync.
>> 
>> Changes since "[PATCH RFC] hv_utils: implement Hyper-V PTP source":
>> - Richard Cochran: implement .adjfreq, .adjtime, .settime64 returning
>>   -EOPNOTSUPP.
>> - Olaf Hering: change IS_ERR->IS_ERR_OR_NULL as CONFIG_PTP_1588_CLOCK
>>   can be disabled.
>> - Thomas Gleixner: formatting fixes, comments added.
>> 
>> Vitaly Kuznetsov (2):
>>   hv_util: switch to using timespec64
>>   hv_utils: implement Hyper-V PTP source
>> 
>>  drivers/hv/hv_util.c | 142 
>> +--
>>  1 file changed, 116 insertions(+), 26 deletions(-)
>> 
>
> It would be good to update Documentation files to describe any configuration 
> needed.

This is just a PTP device, not any different for other PTP devices so
users will be reading their NTP server docs to figure out how to add a
PTP reference clock.

Or do you have any particular idea where to put an example?

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


[PATCH] drivers: staging: Fix "space required after that ','" errors

2017-01-17 Thread simran singhal
Fix checkpatch.pl "space required after that ','" errors

Signed-off-by: simran singhal 

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c 
b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
index d7d85b3..6380b41 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
@@ -3177,7 +3177,7 @@ int ieee80211_wpa_supplicant_ioctl(struct 
ieee80211_device *ieee, struct iw_poin
break;

default:
-   printk("Unknown WPA supplicant request: %d\n",param->cmd);
+   printk("Unknown WPA supplicant request: %d\n", param->cmd);
ret = -EOPNOTSUPP;
break;
}
---
 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c 
b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
index d7d85b3..6380b41 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
@@ -3177,7 +3177,7 @@ int ieee80211_wpa_supplicant_ioctl(struct 
ieee80211_device *ieee, struct iw_poin
break;
 
default:
-   printk("Unknown WPA supplicant request: %d\n",param->cmd);
+   printk("Unknown WPA supplicant request: %d\n", param->cmd);
ret = -EOPNOTSUPP;
break;
}
-- 
2.7.4

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


Re: [PATCH v2 3/4] staging: greybus: audio: Cleanup junk codec registers

2017-01-17 Thread Mark Greer
On Tue, Jan 17, 2017 at 08:19:29PM +0530, Vaibhav Agarwal wrote:
> From: Vaibhav Agarwal 
> 
> Dummy codec register were initially added while populating dummy codec
> mixer controls until module topology parser was available. Now, these
> dummy registers are nowhere used and thus can be safely removed.
> 
> Since ASoC framework requires a valid callback for both read & write
> register APIS, currently empty placeholders are kept to avoid panic.
> 
> Later, register mapping logic can be defined:
> 1. Assuming fixed number of maximum modules connected and register bits
> corresponds to basic info of each module OR
> 2. With a logic to dynamically grow register_cache_size based on codec
> modules added/removed.
> 
> Signed-off-by: Vaibhav Agarwal 
> ---
>  drivers/staging/greybus/audio_codec.c | 39 
> ++-
>  1 file changed, 2 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/staging/greybus/audio_codec.c 
> b/drivers/staging/greybus/audio_codec.c
> index b9d66278ff87..30941f9e380d 100644
> --- a/drivers/staging/greybus/audio_codec.c
> +++ b/drivers/staging/greybus/audio_codec.c
> @@ -1026,47 +1026,16 @@ static int gbcodec_remove(struct snd_soc_codec *codec)
>   return 0;
>  }
>  
> -static u8 gbcodec_reg[GBCODEC_REG_COUNT] = {
> - [GBCODEC_CTL_REG] = GBCODEC_CTL_REG_DEFAULT,
> - [GBCODEC_MUTE_REG] = GBCODEC_MUTE_REG_DEFAULT,
> - [GBCODEC_PB_LVOL_REG] = GBCODEC_PB_VOL_REG_DEFAULT,
> - [GBCODEC_PB_RVOL_REG] = GBCODEC_PB_VOL_REG_DEFAULT,
> - [GBCODEC_CAP_LVOL_REG] = GBCODEC_CAP_VOL_REG_DEFAULT,
> - [GBCODEC_CAP_RVOL_REG] = GBCODEC_CAP_VOL_REG_DEFAULT,
> - [GBCODEC_APB1_MUX_REG] = GBCODEC_APB1_MUX_REG_DEFAULT,
> - [GBCODEC_APB2_MUX_REG] = GBCODEC_APB2_MUX_REG_DEFAULT,
> -};
> -

gbcodec_reg_defaults[] and the definitions of 'GBCODEC_CTL_REG_DEFAULT',
etc. can be removed now too, can't they?

The rest looks good.

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


Re: [PATCH v2 2/4] staging: greybus: audio: Initialize sig_bits before configuring hwparams

2017-01-17 Thread Mark Greer
On Tue, Jan 17, 2017 at 08:19:28PM +0530, Vaibhav Agarwal wrote:
> From: Vaibhav Agarwal 
> 
> Uninitialized variable sig_bits was used while configuring stream params
> for codec module. These params are used to configure PCM settings for
> APBridgeA.
> 
> Usually, this is dependent on codec capability and thus populated via
> codec dai_driver definition. In our case, it is fixed to 16 based on the
> data format, container supported.
> 
> Signed-off-by: Vaibhav Agarwal 
> ---

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


Re: [PATCH v2 1/4] staging: greybus: audio: Avoid less than zero check for le32 variable

2017-01-17 Thread Mark Greer
Hi Vaibhav.

On Tue, Jan 17, 2017 at 08:19:27PM +0530, Vaibhav Agarwal wrote:
> mixer control->info call back function checks for -ve values to rebase
> min and max values. However, le32 variable is used to fetch values from
> GB module FW. Thus -ve value checking is not required. Fix this!!

nit: Please spell out "negative" so it is as easy as possible for the
 casual observer to understand what you're saying.

> Signed-off-by: Vaibhav Agarwal 
> ---
>  drivers/staging/greybus/audio_topology.c | 12 
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/greybus/audio_topology.c 
> b/drivers/staging/greybus/audio_topology.c
> index 3001a4971c06..a8b63a8b2bb0 100644
> --- a/drivers/staging/greybus/audio_topology.c
> +++ b/drivers/staging/greybus/audio_topology.c
> @@ -361,8 +361,8 @@ static int gbcodec_mixer_dapm_ctl_info(struct 
> snd_kcontrol *kcontrol,
>   info = (struct gb_audio_ctl_elem_info *)data->info;
>  
>   /* update uinfo */
> - platform_max = info->value.integer.max;
> - platform_min = info->value.integer.min;
> + platform_max = le32_to_cpu(info->value.integer.max);
> + platform_min = le32_to_cpu(info->value.integer.min);

Should this piece be in patch 4/4?  It seems out of place in this patch.

>   if (platform_max == 1 &&
>   !strnstr(kcontrol->id.name, " Volume", NAME_SIZE))
> @@ -371,12 +371,8 @@ static int gbcodec_mixer_dapm_ctl_info(struct 
> snd_kcontrol *kcontrol,
>   uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
>  
>   uinfo->count = data->vcount;
> - uinfo->value.integer.min = 0;
> - if (info->value.integer.min < 0 &&
> - (uinfo->type == SNDRV_CTL_ELEM_TYPE_INTEGER))
> - uinfo->value.integer.max = platform_max - platform_min;
> - else
> - uinfo->value.integer.max = platform_max;
> + uinfo->value.integer.min = platform_min;
> + uinfo->value.integer.max = platform_max;

This part looks good.

Thanks,

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


Re: [PATCH v3 1/2] hv_util: switch to using timespec64

2017-01-17 Thread Thomas Gleixner
On Tue, 17 Jan 2017, Vitaly Kuznetsov wrote:

> do_settimeofday() is deprecated, use do_settimeofday64() instead.
> 
> Signed-off-by: Vitaly Kuznetsov 
> Acked-by: John Stultz 

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


Re: [PATCH v2 4/4] staging: greybus: audio: Ensure proper byte order

2017-01-17 Thread Mark Greer
On Tue, Jan 17, 2017 at 08:19:30PM +0530, Vaibhav Agarwal wrote:
> From: Vaibhav Agarwal 
> 
> Proper byte order was completely disregarded for multi byte data shared
> between AP and module (and APB1). Fix this.
> 
> Signed-off-by: Vaibhav Agarwal 
> ---

Hi Vaibhav.

I think you got them all except for this one in
audio_topology.c:gbcodec_mixer_dapm_ctl_put():

>>> max = info->value.integer.max;  <<<
mask = (1 << fls(max)) - 1;
val = ucontrol->value.integer.value[0] & mask;
connect = !!val;

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


[PATCH] Fix coding style errors "open brace go on the same line"

2017-01-17 Thread Adrien Descamps
Open braces for enum, union and struct go on the same line.

Signed-off-by: Adrien Descamps 
Reviewed-by: Eric Anholt 
---
 .../interface/vchi/connections/connection.h|  3 +--
 .../interface/vchi/message_drivers/message.h   |  9 +++--
 drivers/staging/vc04_services/interface/vchi/vchi.h| 18 ++
 .../staging/vc04_services/interface/vchi/vchi_common.h |  9 +++--
 .../vc04_services/interface/vchiq_arm/vchiq_debugfs.h  |  3 +--
 5 files changed, 14 insertions(+), 28 deletions(-)

diff --git 
a/drivers/staging/vc04_services/interface/vchi/connections/connection.h 
b/drivers/staging/vc04_services/interface/vchi/connections/connection.h
index fef6ac3..e793cdf 100644
--- a/drivers/staging/vc04_services/interface/vchi/connections/connection.h
+++ b/drivers/staging/vc04_services/interface/vchi/connections/connection.h
@@ -217,8 +217,7 @@ typedef void   
(*VCHI_BUFFER_FREE)(VCHI_CONNECTION_SERVICE_HANDLE_T service_hand
  System driver struct
  */
 
-struct opaque_vchi_connection_api_t
-{
+struct opaque_vchi_connection_api_t {
// Routine to init the connection
VCHI_CONNECTION_INIT_T  init;
 
diff --git 
a/drivers/staging/vc04_services/interface/vchi/message_drivers/message.h 
b/drivers/staging/vc04_services/interface/vchi/message_drivers/message.h
index 8b3f767..a7740a4 100644
--- a/drivers/staging/vc04_services/interface/vchi/message_drivers/message.h
+++ b/drivers/staging/vc04_services/interface/vchi/message_drivers/message.h
@@ -53,14 +53,12 @@ typedef enum message_event_type {
MESSAGE_EVENT_MSG_DISCARDED
 } MESSAGE_EVENT_TYPE_T;
 
-typedef enum vchi_msg_flags
-{
+typedef enum vchi_msg_flags {
VCHI_MSG_FLAGS_NONE  = 0x0,
VCHI_MSG_FLAGS_TERMINATE_DMA = 0x1
 } VCHI_MSG_FLAGS_T;
 
-typedef enum message_tx_channel
-{
+typedef enum message_tx_channel {
MESSAGE_TX_CHANNEL_MESSAGE   = 0,
MESSAGE_TX_CHANNEL_BULK  = 1 // drivers may provide multiple 
bulk channels, from 1 upwards
 } MESSAGE_TX_CHANNEL_T;
@@ -69,8 +67,7 @@ typedef enum message_tx_channel
 #define MESSAGE_TX_CHANNEL_BULK_PREV(c) 
(MESSAGE_TX_CHANNEL_BULK+((c)-MESSAGE_TX_CHANNEL_BULK+VCHI_MAX_BULK_TX_CHANNELS_PER_CONNECTION-1)%VCHI_MAX_BULK_TX_CHANNELS_PER_CONNECTION)
 #define MESSAGE_TX_CHANNEL_BULK_NEXT(c) 
(MESSAGE_TX_CHANNEL_BULK+((c)-MESSAGE_TX_CHANNEL_BULK+1)%VCHI_MAX_BULK_TX_CHANNELS_PER_CONNECTION)
 
-typedef enum message_rx_channel
-{
+typedef enum message_rx_channel {
MESSAGE_RX_CHANNEL_MESSAGE   = 0,
MESSAGE_RX_CHANNEL_BULK  = 1 // drivers may provide multiple 
bulk channels, from 1 upwards
 } MESSAGE_RX_CHANNEL_T;
diff --git a/drivers/staging/vc04_services/interface/vchi/vchi.h 
b/drivers/staging/vc04_services/interface/vchi/vchi.h
index d693728..44169d6 100644
--- a/drivers/staging/vc04_services/interface/vchi/vchi.h
+++ b/drivers/staging/vc04_services/interface/vchi/vchi.h
@@ -61,8 +61,7 @@ struct vchi_version {
 #define VCHI_VERSION(v_) { v_, v_ }
 #define VCHI_VERSION_EX(v_, m_) { v_, m_ }
 
-typedef enum
-{
+typedef enum {
VCHI_VEC_POINTER,
VCHI_VEC_HANDLE,
VCHI_VEC_LIST
@@ -71,26 +70,22 @@ typedef enum
 typedef struct vchi_msg_vector_ex {
 
VCHI_MSG_VECTOR_TYPE_T type;
-   union
-   {
+   union {
   // a memory handle
-  struct
-  {
+  struct {
  VCHI_MEM_HANDLE_T handle;
  uint32_t offset;
  int32_t vec_len;
   } handle;
 
   // an ordinary data pointer
-  struct
-  {
+  struct {
  const void *vec_base;
  int32_t vec_len;
   } ptr;
 
   // a nested vector list
-  struct
-  {
+  struct {
  struct vchi_msg_vector_ex *vec;
  uint32_t vec_len;
   } list;
@@ -114,8 +109,7 @@ struct opaque_vchi_service_t;
 
 // Descriptor for a held message. Allocated by client, initialised by 
vchi_msg_hold,
 // vchi_msg_iter_hold or vchi_msg_iter_hold_next. Fields are for internal VCHI 
use only.
-typedef struct
-{
+typedef struct {
struct opaque_vchi_service_t *service;
void *message;
 } VCHI_HELD_MSG_T;
diff --git a/drivers/staging/vc04_services/interface/vchi/vchi_common.h 
b/drivers/staging/vc04_services/interface/vchi/vchi_common.h
index d535a72..3f7d843 100644
--- a/drivers/staging/vc04_services/interface/vchi/vchi_common.h
+++ b/drivers/staging/vc04_services/interface/vchi/vchi_common.h
@@ -36,8 +36,7 @@
 
 
 //flags used when sending messages (must be bitmapped)
-typedef enum
-{
+typedef enum {
VCHI_FLAGS_NONE  = 0x0,
VCHI_FLAGS_BLOCK_UNTIL_OP_COMPLETE   = 0x1,   // waits for message to be 
received, or sent (NB. not the same as being seen on other side)
VCHI_FLAGS_CALLBACK_WHEN_OP_COMPLETE = 0x2,   // run a callback when 
message sent
@@ -62,8 +61,7 @@ typedef enum {
 } VCHI_CRC_CONTROL_T;
 
 //callback reasons when an event occurs on a service
-

Re: [PATCH v3 2/2] hv_utils: implement Hyper-V PTP source

2017-01-17 Thread Thomas Gleixner
On Tue, 17 Jan 2017, Vitaly Kuznetsov wrote:
> Stephen Hemminger  writes:
> >>  static int hv_timesync_init(struct hv_util_service *srv)
> >>  {
> >>INIT_WORK(&wrk.work, hv_set_host_time);
> >> +
> >> +  hv_ptp_clock = ptp_clock_register(&ptp_hyperv_info, NULL);
> >> +  if (IS_ERR_OR_NULL(hv_ptp_clock)) {
> >> +  pr_err("cannot register PTP clock: %ld\n",
> >> + PTR_ERR(hv_ptp_clock));
> >
> > Why not return error to init routine in case of failure.
> >
> >> +  hv_ptp_clock = NULL;
> >
> > Why not return error to init routine?  Rather than having user
> > scan log.
> >
> 
> The idea here was to not depend on CONFIG_PTP_1588_CLOCK. In case
> CONFIG_PTP_1588_CLOCK is disabled ptp_clock_register() will return NULL
> but the Hyper-V timesync driver remains functional - it still handles
> the ICTIMESYNCFLAG_SYNC case, just the ptp device will be missing.
> We can:
> 1) Put PTP-related code under #ifdef CONFIG_PTP_1588_CLOCK
> 2) Handle errors and NULL returned from ptp_clock_register() differently,
>fail init in case we get an error and continue in case we see NULL.
> 3) Leave things as they are.
> 4) Always require CONFIG_PTP_1588_CLOCK.
> 
> My personal preference would be 2 or 3. What do you think?

Keep the current implementation and add a comment explaining the logic of
keeping the driver functional for the the ICTIMESYNC case.

Thanks,

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


[PATCH 1/2] staging: rtl8188eu: fix type of wpa_ielen in rtw_get_cipher_info

2017-01-17 Thread Pierre-Yves Kerbrat
wpa_ielen was declared u32 when we actually use an int

Fix sparse (-Wtypesign) issues:
drivers/staging/rtl8188eu/core/rtw_ieee80211.c:1008:60: warning:
incorrect type in argument 2 (different signedness)
drivers/staging/rtl8188eu/core/rtw_ieee80211.c:1008:60:expected int
*wpa_ie_len
drivers/staging/rtl8188eu/core/rtw_ieee80211.c:1008:60:got unsigned
int *
drivers/staging/rtl8188eu/core/rtw_ieee80211.c:1021:69: warning:
incorrect type in argument 2 (different signedness)

Signed-off-by: Pierre-Yves Kerbrat 
---
 drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c 
b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
index 3e7a1767607e..b074de5e93f8 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
@@ -1000,7 +1000,7 @@ int ieee80211_get_hdrlen(u16 fc)
 
 static int rtw_get_cipher_info(struct wlan_network *pnetwork)
 {
-   u32 wpa_ielen;
+   int wpa_ielen;
unsigned char *pbuf;
int group_cipher = 0, pairwise_cipher = 0, is8021x = 0;
int ret = _FAIL;
-- 
2.11.0

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


[PATCH 2/2] staging: rtl8188eu: fix type sign of len in rtw_get_bcn_info

2017-01-17 Thread Pierre-Yves Kerbrat
len was declared unsigned int where we use an int

Fix sparse (-Wtypesign) issues:
drivers/staging/rtl8188eu/core/rtw_ieee80211.c:1081:88: warning:
incorrect type in argument 3 (different signedness)
drivers/staging/rtl8188eu/core/rtw_ieee80211.c:1081:88:expected int
*len
drivers/staging/rtl8188eu/core/rtw_ieee80211.c:1081:88:got unsigned
int *
drivers/staging/rtl8188eu/core/rtw_ieee80211.c:1091:86: warning:
incorrect type in argument 3 (different signedness)
drivers/staging/rtl8188eu/core/rtw_ieee80211.c:1091:86:expected int
*len
drivers/staging/rtl8188eu/core/rtw_ieee80211.c:1091:86:got unsigned
int *

Signed-off-by: Pierre-Yves Kerbrat 
---
 drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c 
b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
index b074de5e93f8..d1cd34011602 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
@@ -1045,7 +1045,7 @@ void rtw_get_bcn_info(struct wlan_network *pnetwork)
__le16 le_tmp;
u16 wpa_len = 0, rsn_len = 0;
struct HT_info_element *pht_info = NULL;
-   unsigned intlen;
+   int len;
unsigned char   *p;
 
memcpy(&le_tmp, rtw_get_capability_from_ie(pnetwork->network.IEs), 2);
-- 
2.11.0

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


[PATCH 0/2] staging: rtl8188eu: fix typesign issues

2017-01-17 Thread Pierre-Yves Kerbrat
This patch fixes 2 issues detected with sparse (-Wtypesign)

Pierre-Yves Kerbrat (2):
  staging: rtl8188eu: fix type of wpa_ielen in rtw_get_cipher_info
  staging: rtl8188eu: fix type sign of len in rtw_get_bcn_info

 drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.11.0

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


[PATCH 2/5] staging: vchiq_arm: Fix unlocked access to dequeue_pending

2017-01-17 Thread Stefan Wahren
From: Phil Elwell 

The dequeue_pending flag wasn't protected by a spinlock in the
service_callback. So fix this to make it safe.

Signed-off-by: Phil Elwell 
Signed-off-by: Stefan Wahren 
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |   13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

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 0525211..4f024fa 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -279,6 +279,7 @@ struct vchiq_instance_struct {
USER_SERVICE_T *user_service;
VCHIQ_SERVICE_T *service;
VCHIQ_INSTANCE_T instance;
+   bool skip_completion = false;
DEBUG_INITIALISE(g_state.local)
 
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
@@ -345,9 +346,6 @@ struct vchiq_instance_struct {
user_service->msg_queue[user_service->msg_insert &
(MSG_QUEUE_SIZE - 1)] = header;
user_service->msg_insert++;
-   spin_unlock(&msg_queue_spinlock);
-
-   up(&user_service->insert_event);
 
/* If there is a thread waiting in DEQUEUE_MESSAGE, or if
** there is a MESSAGE_AVAILABLE in the completion queue then
@@ -356,15 +354,20 @@ struct vchiq_instance_struct {
if (((user_service->message_available_pos -
instance->completion_remove) >= 0) ||
user_service->dequeue_pending) {
-   DEBUG_TRACE(SERVICE_CALLBACK_LINE);
user_service->dequeue_pending = 0;
-   return VCHIQ_SUCCESS;
+   skip_completion = true;
}
 
+   spin_unlock(&msg_queue_spinlock);
+   up(&user_service->insert_event);
+
header = NULL;
}
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
 
+   if (skip_completion)
+   return VCHIQ_SUCCESS;
+
return add_completion(instance, reason, header, user_service,
bulk_userdata);
 }
-- 
1.7.9.5

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


[PATCH 0/5] staging: vc04_services: Upstream fixes

2017-01-17 Thread Stefan Wahren
This patch series contain several fixes from the Foundation kernel
and based on this pull request [1] from Phil Elwell. Since the
original commits wasn't ready for mainline submission i reworked them.

[1] - https://github.com/anholt/linux/pull/63

Phil Elwell (5):
  staging: vchiq_core: Reduce the memdump size
  staging: vchiq_arm: Fix unlocked access to dequeue_pending
  staging: vchiq_arm: Service callbacks must not fail
  staging: vc04_services: Fix messages appearing twice
  staging: vchiq_arm: Avoid premature message stalls

 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |   64 
 .../vc04_services/interface/vchiq_arm/vchiq_core.c |   32 +++---
 2 files changed, 61 insertions(+), 35 deletions(-)

-- 
1.7.9.5

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


[PATCH 5/5] staging: vchiq_arm: Avoid premature message stalls

2017-01-17 Thread Stefan Wahren
From: Phil Elwell 

The constants MAX_COMPLETIONS and MSG_QUEUE_SIZE control
the number of messages that can be outstanding to each client
before the system as a whole stalls. If the numbers are too
small then unnecessary thread switching will occur while
waiting for a (potentially low priority) client thread to
consume some data; badly written clients can even lead to
deadlock.

For services that carry many short messages, 16 messages can
represent a very small amount of data. Since the resources
are small - 16 bytes for a completion, 4 bytes for a message
pointer - increase the limits so they are unlikely to be hit
except in exceptional circumstances.

Signed-off-by: Phil Elwell 
Signed-off-by: Stefan Wahren 
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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 b02dc4b..1dc8627 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -64,10 +64,10 @@
 #define VCHIQ_MINOR 0
 
 /* Some per-instance constants */
-#define MAX_COMPLETIONS 16
+#define MAX_COMPLETIONS 128
 #define MAX_SERVICES 64
 #define MAX_ELEMENTS 8
-#define MSG_QUEUE_SIZE 64
+#define MSG_QUEUE_SIZE 128
 
 #define KEEPALIVE_VER 1
 #define KEEPALIVE_VER_MIN KEEPALIVE_VER
-- 
1.7.9.5

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


[PATCH 3/5] staging: vchiq_arm: Service callbacks must not fail

2017-01-17 Thread Stefan Wahren
From: Phil Elwell 

Service callbacks are not allowed to return an error. The internal
callback that delivers events and messages to user tasks does not
enqueue them if the service is closing, but this is not an error
and should not be reported as such.

Signed-off-by: Phil Elwell 
Signed-off-by: Stefan Wahren 
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 4f024fa..72945f9 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -224,7 +224,7 @@ struct vchiq_instance_struct {
} else if (instance->closing) {
vchiq_log_info(vchiq_arm_log_level,
"service_callback closing");
-   return VCHIQ_ERROR;
+   return VCHIQ_SUCCESS;
}
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
}
-- 
1.7.9.5

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


[PATCH 4/5] staging: vc04_services: Fix messages appearing twice

2017-01-17 Thread Stefan Wahren
From: Phil Elwell 

An issue was observed when flushing openmax components
which generate a large number of messages returning
buffers to host.

We occasionally found a duplicate message from 16
messages prior, resulting in a buffer returned twice.

So fix the issue by adding more memory barriers.

Signed-off-by: Phil Elwell 
Signed-off-by: Stefan Wahren 
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |   45 
 .../vc04_services/interface/vchiq_arm/vchiq_core.c |   24 ---
 2 files changed, 46 insertions(+), 23 deletions(-)

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 72945f9..b02dc4b 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -208,10 +208,11 @@ struct vchiq_instance_struct {
void *bulk_userdata)
 {
VCHIQ_COMPLETION_DATA_T *completion;
+   int insert;
DEBUG_INITIALISE(g_state.local)
 
-   while (instance->completion_insert ==
-   (instance->completion_remove + MAX_COMPLETIONS)) {
+   insert = instance->completion_insert;
+   while ((insert - instance->completion_remove) >= MAX_COMPLETIONS) {
/* Out of space - wait for the client */
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
vchiq_log_trace(vchiq_arm_log_level,
@@ -229,9 +230,7 @@ struct vchiq_instance_struct {
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
}
 
-   completion =
-&instance->completions[instance->completion_insert &
-(MAX_COMPLETIONS - 1)];
+   completion = &instance->completions[insert & (MAX_COMPLETIONS - 1)];
 
completion->header = header;
completion->reason = reason;
@@ -252,9 +251,10 @@ struct vchiq_instance_struct {
wmb();
 
if (reason == VCHIQ_MESSAGE_AVAILABLE)
-   user_service->message_available_pos =
-   instance->completion_insert;
-   instance->completion_insert++;
+   user_service->message_available_pos = insert;
+
+   insert++;
+   instance->completion_insert = insert;
 
up(&instance->insert_event);
 
@@ -895,24 +895,27 @@ struct vchiq_io_copy_callback_context {
}
DEBUG_TRACE(AWAIT_COMPLETION_LINE);
 
-   /* A read memory barrier is needed to stop prefetch of a stale
-   ** completion record
-   */
-   rmb();
-
if (ret == 0) {
int msgbufcount = args.msgbufcount;
+   int remove = instance->completion_remove;
+
for (ret = 0; ret < args.count; ret++) {
VCHIQ_COMPLETION_DATA_T *completion;
VCHIQ_SERVICE_T *service;
USER_SERVICE_T *user_service;
VCHIQ_HEADER_T *header;
-   if (instance->completion_remove ==
-   instance->completion_insert)
+
+   if (remove == instance->completion_insert)
break;
+
completion = &instance->completions[
-   instance->completion_remove &
-   (MAX_COMPLETIONS - 1)];
+   remove & (MAX_COMPLETIONS - 1)];
+
+   /*
+* A read memory barrier is needed to stop
+* prefetch of a stale completion record
+*/
+   rmb();
 
service = completion->service_userdata;
user_service = service->base.userdata;
@@ -987,7 +990,13 @@ struct vchiq_io_copy_callback_context {
break;
}
 
-   instance->completion_remove++;
+   /*
+* Ensure that the above copy has completed
+* before advancing the remove pointer.
+*/
+   mb();
+   remove++;
+   instance->completion_remove = remove;
}
 
if (msgbufcount != args.msgbufcount) {
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 9867e64..d587097 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -607,15 +607,17 @@ s

[PATCH 1/5] staging: vchiq_core: Reduce the memdump size

2017-01-17 Thread Stefan Wahren
From: Phil Elwell 

This reduces the memory dump size to a sufficient value of 16 bytes.

Signed-off-by: Phil Elwell 
Signed-off-by: Stefan Wahren 
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index fcdfd66..9867e64 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -920,7 +920,7 @@ static const char *msg_type_str(unsigned int msg_type)
 VCHIQ_LOG_INFO))
vchiq_log_dump_mem("Sent", 0,
   header->data,
-  min((size_t)64,
+  min((size_t)16,
   (size_t)callback_result));
 
spin_lock("a_spinlock);
@@ -1073,7 +1073,7 @@ static const char *msg_type_str(unsigned int msg_type)
 VCHIQ_LOG_INFO))
vchiq_log_dump_mem("Sent", 0,
   header->data,
-  min((size_t)64,
+  min((size_t)16,
   (size_t)callback_result));
 
VCHIQ_SERVICE_STATS_INC(service, ctrl_tx_count);
@@ -1734,7 +1734,7 @@ static const char *msg_type_str(unsigned int msg_type)
remoteport, localport, size);
if (size > 0)
vchiq_log_dump_mem("Rcvd", 0, header->data,
-   min(64, size));
+   min(16, size));
}
 
if (((unsigned long)header & VCHIQ_SLOT_MASK) +
@@ -2191,7 +2191,7 @@ static const char *msg_type_str(unsigned int msg_type)
remoteport, localport, size);
if (size > 0)
vchiq_log_dump_mem("Rcvd", 0, header->data,
-   min(64, size));
+   min(16, size));
}
 
switch (type) {
-- 
1.7.9.5

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


[PATCH] staging: fbtft: replace decimal with 4-digit octal permissions

2017-01-17 Thread Stefano Manni
Following error detected by checkpatch.pl:

ERROR: Use 4 digit octal (0777) not decimal permissions

Signed-off-by: Stefano Manni 
---
 drivers/staging/fbtft/flexfb.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
index ded1071..7d2ffbb 100644
--- a/drivers/staging/fbtft/flexfb.c
+++ b/drivers/staging/fbtft/flexfb.c
@@ -27,40 +27,40 @@
 #define DRVNAME"flexfb"
 
 static char *chip;
-module_param(chip, charp, 0);
+module_param(chip, charp, );
 MODULE_PARM_DESC(chip, "LCD controller");
 
 static unsigned int width;
-module_param(width, uint, 0);
+module_param(width, uint, );
 MODULE_PARM_DESC(width, "Display width");
 
 static unsigned int height;
-module_param(height, uint, 0);
+module_param(height, uint, );
 MODULE_PARM_DESC(height, "Display height");
 
 static s16 init[512];
 static int init_num;
-module_param_array(init, short, &init_num, 0);
+module_param_array(init, short, &init_num, );
 MODULE_PARM_DESC(init, "Init sequence");
 
 static unsigned int setaddrwin;
-module_param(setaddrwin, uint, 0);
+module_param(setaddrwin, uint, );
 MODULE_PARM_DESC(setaddrwin, "Which set_addr_win() implementation to use");
 
 static unsigned int buswidth = 8;
-module_param(buswidth, uint, 0);
+module_param(buswidth, uint, );
 MODULE_PARM_DESC(buswidth, "Width of databus (default: 8)");
 
 static unsigned int regwidth = 8;
-module_param(regwidth, uint, 0);
+module_param(regwidth, uint, );
 MODULE_PARM_DESC(regwidth, "Width of controller register (default: 8)");
 
 static bool nobacklight;
-module_param(nobacklight, bool, 0);
+module_param(nobacklight, bool, );
 MODULE_PARM_DESC(nobacklight, "Turn off backlight functionality.");
 
 static bool latched;
-module_param(latched, bool, 0);
+module_param(latched, bool, );
 MODULE_PARM_DESC(latched, "Use with latched 16-bit databus");
 
 static s16 *initp;
-- 
2.5.5

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


Re: [PATCH] staging: fbtft: replace decimal with 4-digit octal permissions

2017-01-17 Thread Andy Shevchenko
On Wed, Jan 18, 2017 at 1:38 AM, Stefano Manni  wrote:
> Following error detected by checkpatch.pl:
>
> ERROR: Use 4 digit octal (0777) not decimal permissions
>
> Signed-off-by: Stefano Manni 
> ---
>  drivers/staging/fbtft/flexfb.c | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
> index ded1071..7d2ffbb 100644
> --- a/drivers/staging/fbtft/flexfb.c
> +++ b/drivers/staging/fbtft/flexfb.c
> @@ -27,40 +27,40 @@
>  #define DRVNAME"flexfb"
>
>  static char *chip;
> -module_param(chip, charp, 0);
> +module_param(chip, charp, );
>  MODULE_PARM_DESC(chip, "LCD controller");

Perhaps make them readable back?
0400

-- 
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] drivers: staging: rtl8188eu: include: statements using pointers enclosed in parantheses

2017-01-17 Thread Kartikey Singh
Macros with complex values enclosed in parentheses

Signed-off-by: Kartikey Singh 
---
 drivers/staging/rtl8188eu/include/wifi.h | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/rtl8188eu/include/wifi.h 
b/drivers/staging/rtl8188eu/include/wifi.h
index 5630dcb..396b89d 100644
--- a/drivers/staging/rtl8188eu/include/wifi.h
+++ b/drivers/staging/rtl8188eu/include/wifi.h
@@ -150,7 +150,7 @@ enum WIFI_REG_DOMAIN {
 #define _ORDER_BIT(15)
 
 #define SetToDs(pbuf)  \
-   *(__le16 *)(pbuf) |= cpu_to_le16(_TO_DS_)
+   (*(__le16 *)(pbuf) |= cpu_to_le16(_TO_DS_))
 
 #define GetToDs(pbuf)  (((*(__le16 *)(pbuf)) & cpu_to_le16(_TO_DS_)) != 0)
 
@@ -158,26 +158,26 @@ enum WIFI_REG_DOMAIN {
*(__le16 *)(pbuf) &= (~cpu_to_le16(_TO_DS_))
 
 #define SetFrDs(pbuf)  \
-   *(__le16 *)(pbuf) |= cpu_to_le16(_FROM_DS_)
+   (*(__le16 *)(pbuf) |= cpu_to_le16(_FROM_DS_))
 
 #define GetFrDs(pbuf)  (((*(__le16 *)(pbuf)) & cpu_to_le16(_FROM_DS_)) != 0)
 
 #define ClearFrDs(pbuf)\
-   *(__le16 *)(pbuf) &= (~cpu_to_le16(_FROM_DS_))
+   (*(__le16 *)(pbuf) &= (~cpu_to_le16(_FROM_DS_)))
 
 #define get_tofr_ds(pframe)((GetToDs(pframe) << 1) | GetFrDs(pframe))
 
 
 #define SetMFrag(pbuf) \
-   *(__le16 *)(pbuf) |= cpu_to_le16(_MORE_FRAG_)
+   (*(__le16 *)(pbuf) |= cpu_to_le16(_MORE_FRAG_))
 
 #define GetMFrag(pbuf) (((*(__le16 *)(pbuf)) & cpu_to_le16(_MORE_FRAG_)) != 0)
 
 #define ClearMFrag(pbuf)   \
-   *(__le16 *)(pbuf) &= (~cpu_to_le16(_MORE_FRAG_))
+   (*(__le16 *)(pbuf) &= (~cpu_to_le16(_MORE_FRAG_)))
 
 #define SetRetry(pbuf) \
-   *(__le16 *)(pbuf) |= cpu_to_le16(_RETRY_)
+   (*(__le16 *)(pbuf) |= cpu_to_le16(_RETRY_))
 
 #define GetRetry(pbuf) (((*(__le16 *)(pbuf)) & cpu_to_le16(_RETRY_)) != 0)
 
@@ -185,23 +185,23 @@ enum WIFI_REG_DOMAIN {
*(__le16 *)(pbuf) &= (~cpu_to_le16(_RETRY_))
 
 #define SetPwrMgt(pbuf)\
-   *(__le16 *)(pbuf) |= cpu_to_le16(_PWRMGT_)
+   (*(__le16 *)(pbuf) |= cpu_to_le16(_PWRMGT_))
 
 #define GetPwrMgt(pbuf)(((*(__le16 *)(pbuf)) & cpu_to_le16(_PWRMGT_)) 
!= 0)
 
 #define ClearPwrMgt(pbuf)  \
-   *(__le16 *)(pbuf) &= (~cpu_to_le16(_PWRMGT_))
+   (*(__le16 *)(pbuf) &= (~cpu_to_le16(_PWRMGT_)))
 
 #define SetMData(pbuf) \
-   *(__le16 *)(pbuf) |= cpu_to_le16(_MORE_DATA_)
+   (*(__le16 *)(pbuf) |= cpu_to_le16(_MORE_DATA_))
 
 #define GetMData(pbuf) (((*(__le16 *)(pbuf)) & cpu_to_le16(_MORE_DATA_)) != 0)
 
 #define ClearMData(pbuf)   \
-   *(__le16 *)(pbuf) &= (~cpu_to_le16(_MORE_DATA_))
+   (*(__le16 *)(pbuf) &= (~cpu_to_le16(_MORE_DATA_)))
 
 #define SetPrivacy(pbuf)   \
-   *(__le16 *)(pbuf) |= cpu_to_le16(_PRIVACY_)
+   (*(__le16 *)(pbuf) |= cpu_to_le16(_PRIVACY_))
 
 #define GetPrivacy(pbuf)   \
(((*(__le16 *)(pbuf)) & cpu_to_le16(_PRIVACY_)) != 0)
@@ -240,15 +240,15 @@ enum WIFI_REG_DOMAIN {
 
 
 #define SetPriority(pbuf, tid) \
-   *(__le16 *)(pbuf) |= cpu_to_le16(tid & 0xf)
+   (*(__le16 *)(pbuf) |= cpu_to_le16(tid & 0xf))
 
 #define GetPriority(pbuf)  ((le16_to_cpu(*(__le16 *)(pbuf))) & 0xf)
 
 #define SetEOSP(pbuf, eosp)\
-   *(__le16 *)(pbuf) |= cpu_to_le16((eosp & 1) << 4)
+   (*(__le16 *)(pbuf) |= cpu_to_le16((eosp & 1) << 4))
 
 #define SetAckpolicy(pbuf, ack)\
-   *(__le16 *)(pbuf) |= cpu_to_le16((ack & 3) << 5)
+   (*(__le16 *)(pbuf) |= cpu_to_le16((ack & 3) << 5))
 
 #define GetAckpolicy(pbuf) (((le16_to_cpu(*(__le16 *)pbuf)) >> 5) & 0x3)
 
-- 
2.9.3

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


[PATCH] iio: trigger: free trigger resource correctly

2017-01-17 Thread Alison Schofield
Using iio_trigger_put() to free a trigger leads to release of
a resource we never held.  Replace with iio_trigger_free().

Signed-off-by: Alison Schofield 
---
Patches to use devm_* funcs are ready to follow this for
the interrupt & bfin-timer triggers.

 drivers/iio/trigger/iio-trig-interrupt.c  | 4 ++--
 drivers/iio/trigger/iio-trig-sysfs.c  | 2 +-
 drivers/staging/iio/trigger/iio-trig-bfin-timer.c | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/trigger/iio-trig-interrupt.c 
b/drivers/iio/trigger/iio-trig-interrupt.c
index 572bc6f..b18e50d 100644
--- a/drivers/iio/trigger/iio-trig-interrupt.c
+++ b/drivers/iio/trigger/iio-trig-interrupt.c
@@ -84,7 +84,7 @@ static int iio_interrupt_trigger_probe(struct platform_device 
*pdev)
 error_free_trig_info:
kfree(trig_info);
 error_put_trigger:
-   iio_trigger_put(trig);
+   iio_trigger_free(trig);
 error_ret:
return ret;
 }
@@ -99,7 +99,7 @@ static int iio_interrupt_trigger_remove(struct 
platform_device *pdev)
iio_trigger_unregister(trig);
free_irq(trig_info->irq, trig);
kfree(trig_info);
-   iio_trigger_put(trig);
+   iio_trigger_free(trig);
 
return 0;
 }
diff --git a/drivers/iio/trigger/iio-trig-sysfs.c 
b/drivers/iio/trigger/iio-trig-sysfs.c
index 3dfab2b..202e8b8 100644
--- a/drivers/iio/trigger/iio-trig-sysfs.c
+++ b/drivers/iio/trigger/iio-trig-sysfs.c
@@ -174,7 +174,7 @@ static int iio_sysfs_trigger_probe(int id)
return 0;
 
 out2:
-   iio_trigger_put(t->trig);
+   iio_trigger_free(t->trig);
 free_t:
kfree(t);
 out1:
diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c 
b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
index 9658f20..4e0b4ee 100644
--- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
+++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
@@ -260,7 +260,7 @@ static int iio_bfin_tmr_trigger_probe(struct 
platform_device *pdev)
 out1:
iio_trigger_unregister(st->trig);
 out:
-   iio_trigger_put(st->trig);
+   iio_trigger_free(st->trig);
return ret;
 }
 
@@ -273,7 +273,7 @@ static int iio_bfin_tmr_trigger_remove(struct 
platform_device *pdev)
peripheral_free(st->t->pin);
free_irq(st->irq, st);
iio_trigger_unregister(st->trig);
-   iio_trigger_put(st->trig);
+   iio_trigger_free(st->trig);
 
return 0;
 }
-- 
2.1.4

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