Re: [PATCH v2] staging: vchiq_arm: Clear VLA warning

[Stefan Wahren]

Hi Tobin,

> "Tobin C. Harding"  hat am 12. März 2018 um 06:46 geschrieben:
> 
> 
> On Mon, Mar 12, 2018 at 12:37:53PM +1100, Tobin C. Harding wrote:
> > The kernel would like to have all stack VLA usage removed[1].  The array
> > here is fixed (declared with a const variable) but it appears like a VLA
> > to the compiler.  Also, currently we are putting 768 bytes on the
> > stack.  This function is only called on the error path so performance is
> > not critical, let's just allocate the memory instead of using the
> > stack.  This saves stack space and removes the VLA build warning.
> > 
> > kmalloc a buffer for dumping state instead of using the stack.
> > 
> > [1]: https://lkml.org/lkml/2018/3/7/621
> > 
> > Signed-off-by: Tobin C. Harding 
> > ---
> 
> Drop this please, leaks memory.

except from the leak, did you test this patch on a RPi?

Thanks
Stefan

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

Re: [PATCH v2] staging: vchiq_arm: Clear VLA warning

[Tobin C. Harding]

On Mon, Mar 12, 2018 at 12:37:53PM +1100, Tobin C. Harding wrote:
> The kernel would like to have all stack VLA usage removed[1].  The array
> here is fixed (declared with a const variable) but it appears like a VLA
> to the compiler.  Also, currently we are putting 768 bytes on the
> stack.  This function is only called on the error path so performance is
> not critical, let's just allocate the memory instead of using the
> stack.  This saves stack space and removes the VLA build warning.
> 
> kmalloc a buffer for dumping state instead of using the stack.
> 
> [1]: https://lkml.org/lkml/2018/3/7/621
> 
> Signed-off-by: Tobin C. Harding 
> ---

Drop this please, leaks memory.

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

[PATCH char-misc 1/1] Drivers: hv: vmbus: Add hooks for per-CPU IRQ

[Michael Kelley]

Add hooks to enable/disable a per-CPU IRQ for VMbus. These hooks
are in the architecture independent setup and shutdown paths for
Hyper-V.  They are being added as staging for upcoming code for
Linux guests on Hyper-V on ARM64.  The x86/x64 implementation
is null because VMbus interrupts on x86/x64 don't use an IRQ.

Signed-off-by: Michael Kelley 
---
 arch/x86/include/asm/mshyperv.h | 4 
 drivers/hv/hv.c | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index e73c4d0..b73b839 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -168,6 +168,10 @@ void hyperv_vector_handler(struct pt_regs *regs);
 void hv_setup_vmbus_irq(void (*handler)(void));
 void hv_remove_vmbus_irq(void);
 
+/* On x86/x64, there isn't a real IRQ to be enabled/disabled */
+static inline void hv_enable_vmbus_irq(void) {}
+static inline void hv_disable_vmbus_irq(void) {}
+
 void hv_setup_kexec_handler(void (*handler)(void));
 void hv_remove_kexec_handler(void);
 void hv_setup_crash_handler(void (*handler)(struct pt_regs *regs));
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index b1f6793..6a3ea4e 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -299,6 +299,7 @@ int hv_synic_init(unsigned int cpu)
hv_set_siefp(siefp.as_uint64);
 
/* Setup the shared SINT. */
+   hv_enable_vmbus_irq();
hv_get_synint_state(HV_X64_MSR_SINT0 + VMBUS_MESSAGE_SINT,
shared_sint.as_uint64);
 
@@ -433,6 +434,7 @@ int hv_synic_cleanup(unsigned int cpu)
hv_get_synic_state(sctrl.as_uint64);
sctrl.enable = 0;
hv_set_synic_state(sctrl.as_uint64);
+   hv_disable_vmbus_irq();
 
return 0;
 }
-- 
2.7.4

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

[PATCH v2] staging: vchiq_arm: Clear VLA warning

[Tobin C. Harding]

The kernel would like to have all stack VLA usage removed[1].  The array
here is fixed (declared with a const variable) but it appears like a VLA
to the compiler.  Also, currently we are putting 768 bytes on the
stack.  This function is only called on the error path so performance is
not critical, let's just allocate the memory instead of using the
stack.  This saves stack space and removes the VLA build warning.

kmalloc a buffer for dumping state instead of using the stack.

[1]: https://lkml.org/lkml/2018/3/7/621

Signed-off-by: Tobin C. Harding 
---

v1 of this patch already merged into staging-testing branch of Greg's
staging tree.  This patch depends on v1 being removed, can re-do this
one on top of tip of staging-testing if required.

v2:
 - Use kmalloc() instead of the stack

Patch is untested.  

 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 6 +-
 1 file changed, 5 insertions(+), 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 f5cefda49b22..408ea73f8da7 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -3455,11 +3455,15 @@ vchiq_dump_service_use_state(VCHIQ_STATE_T *state)
int fourcc;
int clientid;
int use_count;
-   } service_data[local_max_services];
+   } *service_data;
 
if (!arm_state)
return;
 
+   service_data = kmalloc_array(local_max_services, sizeof(*service_data), 
GFP_KERNEL);
+   if (!service_data)
+   return;
+
read_lock_bh(_state->susp_res_lock);
vc_suspend_state = arm_state->vc_suspend_state;
vc_resume_state  = arm_state->vc_resume_state;
-- 
2.7.4

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

Re: [PATCH 10/12] staging: iio: tsl2x7x: make logging consistent and correct newlines

[Jonathan Cameron]

On Sat, 10 Mar 2018 14:52:40 +
Jonathan Cameron  wrote:

> On Sat,  3 Mar 2018 20:49:40 -0500
> Brian Masney  wrote:
> 
> > This patch updates all of the logging commands so that they are
> > consistent with the other messages, includes __func__ in the message,
> > and all of the messages include newlines.
> > 
> > This patch also removes some debug log messages from tsl2x7x_prox_cal().
> > 
> > Signed-off-by: Brian Masney   
> Hmm. I nearly fixed this one up but there are a few too many elements I don't
> agree with.

I knew there was something I was forgetting.  There is no point in
putting function names in dynamic debug calls, you can turn that
on if you want it for all of them anyway.

Jonathan
> 
> > ---
> >  drivers/staging/iio/light/tsl2x7x.c | 58 
> > -
> >  1 file changed, 31 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> > b/drivers/staging/iio/light/tsl2x7x.c
> > index da7a4e025083..fb91c46c8747 100644
> > --- a/drivers/staging/iio/light/tsl2x7x.c
> > +++ b/drivers/staging/iio/light/tsl2x7x.c
> > @@ -378,7 +378,8 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
> > ret = i2c_smbus_read_byte_data(chip->client, reg);
> > if (ret < 0) {
> > dev_err(>client->dev,
> > -   "failed to read. err=%x\n", ret);
> > +   "%s: failed to read from register %x: %d\n",
> > +   __func__, reg, ret);
> > goto out_unlock;
> > }
> >  
> > @@ -424,7 +425,9 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
> >  
> > /* note: lux is 31 bit max at this point */
> > if (ch1lux > ch0lux) {
> > -   dev_dbg(>client->dev, "ch1lux > ch0lux-return last 
> > value\n");
> > +   dev_dbg(>client->dev,
> > +   "%s: ch1lux > ch0lux; returning last value\n",
> > +   __func__);
> > ret = chip->als_cur_info.lux;
> > goto out_unlock;
> > }
> > @@ -600,7 +603,8 @@ static int tsl2x7x_als_calibrate(struct iio_dev 
> > *indio_dev)
> >  
> > chip->settings.als_gain_trim = ret;
> > dev_info(>client->dev,
> > -"%s als_calibrate completed\n", chip->client->name);
> > +"%s: %s ALS calibration successfully completed\n",
> > +__func__, chip->client->name);  
> I'm happy with having function name in errors, but it's just noise in info
> prints.
> 
> Mind you this dev_info should go or become dev_dbg - it is superflous
> detail to put in the kernel logs.
> 
> >  
> > return ret;
> >  }
> > @@ -644,7 +648,8 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
> > /* and make sure we're not already on */
> > if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_WORKING) {
> > /* if forcing a register update - turn off, then on */
> > -   dev_info(>client->dev, "device is already enabled\n");
> > +   dev_info(>client->dev, "%s: device is already enabled\n",
> > +__func__);  
> Again noise.  dev_dbg or drop entirely as it doesn't need to be logged.
> > return -EINVAL;
> > }
> >  
> > @@ -681,12 +686,14 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
> >  */
> > for (i = 0, dev_reg = chip->tsl2x7x_config;
> > i < TSL2X7X_MAX_CONFIG_REG; i++) {
> > -   ret = i2c_smbus_write_byte_data(chip->client,
> > -   TSL2X7X_CMD_REG + i,
> > +   int reg = TSL2X7X_CMD_REG + i;
> > +
> > +   ret = i2c_smbus_write_byte_data(chip->client, reg,
> > *dev_reg++);
> > if (ret < 0) {
> > dev_err(>client->dev,
> > -   "failed on write to reg %d.\n", i);
> > +   "%s: failed to write to register %x: %d\n",
> > +   __func__, reg, ret);
> > return ret;
> > }
> > }
> > @@ -708,7 +715,8 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
> > chip->tsl2x7x_chip_status = TSL2X7X_CHIP_WORKING;
> >  
> > if (chip->settings.interrupts_en != 0) {
> > -   dev_info(>client->dev, "Setting Up Interrupt(s)\n");
> > +   dev_info(>client->dev, "%s: Setting up interrupt(s)\n",
> > +__func__);
> >  
> > reg_val = TSL2X7X_CNTL_PWR_ON | TSL2X7X_CNTL_ADC_ENBL;
> > if (chip->settings.interrupts_en == 0x20 ||
> > @@ -819,8 +827,8 @@ static int tsl2x7x_prox_cal(struct iio_dev *indio_dev)
> >  
> > if (chip->settings.prox_max_samples_cal > MAX_SAMPLES_CAL) {
> > dev_err(>client->dev,
> > -   "max prox samples cal is too big: %d\n",
> > -   chip->settings.prox_max_samples_cal);
> > +   

[PATCH] staging: ks7010: replace custom rotation operations in favour of the kernel ones

[Sergio Paracuellos]

This patch replaces custom ROR32 and ROL32 macros for the ones included in
bitops header of the linux kernel.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/ks7010/michael_mic.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/ks7010/michael_mic.c 
b/drivers/staging/ks7010/michael_mic.c
index 80497ef..1df4366 100644
--- a/drivers/staging/ks7010/michael_mic.c
+++ b/drivers/staging/ks7010/michael_mic.c
@@ -11,11 +11,9 @@
 
 #include 
 #include 
+#include 
 #include "michael_mic.h"
 
-// Rotation functions on 32 bit values
-#define ROL32(A, n)(((A) << (n)) | (((A) >> (32 - (n))) & ((1UL << (n)) - 
1)))
-#define ROR32(A, n)ROL32((A), 32 - (n))
 // Convert from Byte[] to UInt32 in a portable way
 #define getUInt32(A, B)((uint32_t)(A[B + 0] << 0) \
+ (A[B + 1] << 8) + (A[B + 2] << 16) + (A[B + 3] << 24))
@@ -50,13 +48,13 @@ void MichaelInitializeFunction(struct michael_mic_t *Mic, 
uint8_t *key)
 
 #define MichaelBlockFunction(L, R) \
 do {   \
-   R ^= ROL32(L, 17);  \
+   R ^= rol32(L, 17);  \
L += R; \
R ^= ((L & 0xff00ff00) >> 8) | ((L & 0x00ff00ff) << 8); \
L += R; \
-   R ^= ROL32(L, 3);   \
+   R ^= rol32(L, 3);   \
L += R; \
-   R ^= ROR32(L, 2);   \
+   R ^= ror32(L, 2);   \
L += R; \
 } while (0)
 
-- 
2.7.4

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