Re: [PATCH 1/1] drivers: android: Cleanup warnings
On Sun, Dec 17, 2017 at 03:07:55AM +0530, Harsh Shandilya wrote: > Ran checkpatch across the entire drivers/android > directory and fixed all relevant warnings. Summary > of changes done: > > -> Convert all symbolic permissions into their > octal equivalents. > -> Use "%s", __func__ in logging macros where the > function name was previously hard-coded. > -> Add a blank line to separate declarations from > statements according to the kernel coding style > guidelines. > -> Fix linelength issues wherever possible. > -> Fix-up a commented out statement to use // in place > of /* */ to silence checkpatch. > > Most line-length warnings were ignored in favor > of code readability. > > Cc: Greg Kroah-Hartman > Cc: Todd Kjos > Cc: Martijn Coenen > Cc: de...@driverdev.osuosl.org > Cc: linux-ker...@vger.kernel.org > Signed-off-by: Harsh Shandilya > --- Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - Your patch did many different things all at once, making it difficult to review. All Linux kernel patches need to only do one thing at a time. If you need to do multiple things (such as clean up all coding style issues in a file/driver), do it in a sequence of patches, each one doing only one thing. This will make it easier to review the patches to ensure that they are correct, and to help alleviate any merge issues that larger patches can cause. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] Remove checkpatch warnings
This patchset removes some warnings generated by checkpatch for cleanup of the rtl8723bs driver. Also some additional cleanups are introduced in the *[1/4] and *[3/4] patches to make the code according to the kernel coding style. Shreeya Patel (4): Staging: rtl8723bs: Replace true with x and false with !x Staging: rtl8723bs: Change names to conform to the kernel code Staging: rtl8723bs: Change condition to assignment Staging: rtl8723bs: Use !x instead of NULL comparison drivers/staging/rtl8723bs/hal/sdio_ops.c | 722 +++ 1 file changed, 360 insertions(+), 362 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] Staging: rtl8723bs: Replace true with x and false with !x
Replace true and false keywords with "x" and "!x"
respectively to follow the kernel coding style.
Signed-off-by: Shreeya Patel
---
drivers/staging/rtl8723bs/hal/sdio_ops.c | 30 ++
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/rtl8723bs/hal/sdio_ops.c
b/drivers/staging/rtl8723bs/hal/sdio_ops.c
index 93ac083..aa52c31 100644
--- a/drivers/staging/rtl8723bs/hal/sdio_ops.c
+++ b/drivers/staging/rtl8723bs/hal/sdio_ops.c
@@ -191,8 +191,8 @@ static u32 sdio_read32(struct intf_hdl *pintfhdl, u32 addr)
rtw_hal_get_hwreg(padapter, HW_VAR_APFM_ON_MAC, &bMacPwrCtrlOn);
if (
((deviceId == WLAN_IOREG_DEVICE_ID) && (offset < 0x100)) ||
- (false == bMacPwrCtrlOn) ||
- (true == adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
+ (!bMacPwrCtrlOn) ||
+ (adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
) {
err = sd_cmd52_read(pintfhdl, ftaddr, 4, (u8 *)&le_tmp);
#ifdef SDIO_DEBUG_IO
@@ -248,8 +248,8 @@ static s32 sdio_readN(struct intf_hdl *pintfhdl, u32 addr,
u32 cnt, u8 *pbuf)
rtw_hal_get_hwreg(padapter, HW_VAR_APFM_ON_MAC, &bMacPwrCtrlOn);
if (
((deviceId == WLAN_IOREG_DEVICE_ID) && (offset < 0x100)) ||
- (false == bMacPwrCtrlOn) ||
- (true == adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
+ (!bMacPwrCtrlOn) ||
+ (adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
)
return sd_cmd52_read(pintfhdl, ftaddr, cnt, pbuf);
@@ -352,8 +352,8 @@ static s32 sdio_writeN(struct intf_hdl *pintfhdl, u32 addr,
u32 cnt, u8 *pbuf)
rtw_hal_get_hwreg(padapter, HW_VAR_APFM_ON_MAC, &bMacPwrCtrlOn);
if (
((deviceId == WLAN_IOREG_DEVICE_ID) && (offset < 0x100)) ||
- (false == bMacPwrCtrlOn) ||
- (true == adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
+ (!bMacPwrCtrlOn) ||
+ (adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
)
return sd_cmd52_write(pintfhdl, ftaddr, cnt, pbuf);
@@ -513,7 +513,7 @@ static u32 sdio_write_port(
padapter = pintfhdl->padapter;
psdio = &adapter_to_dvobj(padapter)->intf_data;
- if (padapter->hw_init_completed == false) {
+ if (!padapter->hw_init_completed) {
DBG_871X("%s [addr = 0x%x cnt =%d] padapter->hw_init_completed
== false\n", __func__, addr, cnt);
return _FAIL;
}
@@ -577,7 +577,7 @@ static s32 _sdio_local_read(
HalSdioGetCmdAddr8723BSdio(padapter, SDIO_LOCAL_DEVICE_ID, addr, &addr);
rtw_hal_get_hwreg(padapter, HW_VAR_APFM_ON_MAC, &bMacPwrCtrlOn);
- if (false == bMacPwrCtrlOn)
+ if (!bMacPwrCtrlOn)
return _sd_cmd52_read(pintfhdl, addr, cnt, pbuf);
n = RND4(cnt);
@@ -616,8 +616,8 @@ s32 sdio_local_read(
rtw_hal_get_hwreg(padapter, HW_VAR_APFM_ON_MAC, &bMacPwrCtrlOn);
if (
- (false == bMacPwrCtrlOn) ||
- (true == adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
+ (!bMacPwrCtrlOn) ||
+ (adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
)
return sd_cmd52_read(pintfhdl, addr, cnt, pbuf);
@@ -662,8 +662,8 @@ s32 sdio_local_write(
rtw_hal_get_hwreg(padapter, HW_VAR_APFM_ON_MAC, &bMacPwrCtrlOn);
if (
- (false == bMacPwrCtrlOn) ||
- (true == adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
+ (!bMacPwrCtrlOn) ||
+ (adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
)
return sd_cmd52_write(pintfhdl, addr, cnt, pbuf);
@@ -843,8 +843,7 @@ void ClearInterrupt8723BSdio(struct adapter *padapter)
struct hal_com_data *pHalData;
u8 *clear;
-
- if (true == padapter->bSurpriseRemoved)
+ if (padapter->bSurpriseRemoved)
return;
pHalData = GET_HAL_DATA(padapter);
@@ -1161,8 +1160,7 @@ void sd_int_hdl(struct adapter *padapter)
if (
- (padapter->bDriverStopped == true) ||
- (padapter->bSurpriseRemoved == true)
+ (padapter->bDriverStopped) || (padapter->bSurpriseRemoved)
)
return;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] Staging: rtl8723bs: Change names to conform to the kernel code
Change names of some variables and functions to conform
to the kernel coding style.
Signed-off-by: Shreeya Patel
---
drivers/staging/rtl8723bs/hal/sdio_ops.c | 714 +++
1 file changed, 357 insertions(+), 357 deletions(-)
diff --git a/drivers/staging/rtl8723bs/hal/sdio_ops.c
b/drivers/staging/rtl8723bs/hal/sdio_ops.c
index aa52c31..00b20c0 100644
--- a/drivers/staging/rtl8723bs/hal/sdio_ops.c
+++ b/drivers/staging/rtl8723bs/hal/sdio_ops.c
@@ -28,35 +28,35 @@
/* Creadted by Roger, 2011.01.31. */
/* */
static void HalSdioGetCmdAddr8723BSdio(
- struct adapter *padapter,
- u8 DeviceID,
- u32 Addr,
- u32 *pCmdAddr
+ struct adapter *adapter,
+ u8 device_id,
+ u32 addr,
+ u32 *cmdaddr
)
{
- switch (DeviceID) {
+ switch (device_id) {
case SDIO_LOCAL_DEVICE_ID:
- *pCmdAddr = ((SDIO_LOCAL_DEVICE_ID << 13) | (Addr &
SDIO_LOCAL_MSK));
+ *cmdaddr = ((SDIO_LOCAL_DEVICE_ID << 13) | (addr &
SDIO_LOCAL_MSK));
break;
case WLAN_IOREG_DEVICE_ID:
- *pCmdAddr = ((WLAN_IOREG_DEVICE_ID << 13) | (Addr &
WLAN_IOREG_MSK));
+ *cmdaddr = ((WLAN_IOREG_DEVICE_ID << 13) | (addr &
WLAN_IOREG_MSK));
break;
case WLAN_TX_HIQ_DEVICE_ID:
- *pCmdAddr = ((WLAN_TX_HIQ_DEVICE_ID << 13) | (Addr &
WLAN_FIFO_MSK));
+ *cmdaddr = ((WLAN_TX_HIQ_DEVICE_ID << 13) | (addr &
WLAN_FIFO_MSK));
break;
case WLAN_TX_MIQ_DEVICE_ID:
- *pCmdAddr = ((WLAN_TX_MIQ_DEVICE_ID << 13) | (Addr &
WLAN_FIFO_MSK));
+ *cmdaddr = ((WLAN_TX_MIQ_DEVICE_ID << 13) | (addr &
WLAN_FIFO_MSK));
break;
case WLAN_TX_LOQ_DEVICE_ID:
- *pCmdAddr = ((WLAN_TX_LOQ_DEVICE_ID << 13) | (Addr &
WLAN_FIFO_MSK));
+ *cmdaddr = ((WLAN_TX_LOQ_DEVICE_ID << 13) | (addr &
WLAN_FIFO_MSK));
break;
case WLAN_RX0FF_DEVICE_ID:
- *pCmdAddr = ((WLAN_RX0FF_DEVICE_ID << 13) | (Addr &
WLAN_RX0FF_MSK));
+ *cmdaddr = ((WLAN_RX0FF_DEVICE_ID << 13) | (addr &
WLAN_RX0FF_MSK));
break;
default:
@@ -66,64 +66,64 @@ static void HalSdioGetCmdAddr8723BSdio(
static u8 get_deviceid(u32 addr)
{
- u8 devideId;
- u16 pseudoId;
+ u8 devide_id;
+ u16 pseudo_id;
- pseudoId = (u16)(addr >> 16);
- switch (pseudoId) {
+ pseudo_id = (u16)(addr >> 16);
+ switch (pseudo_id) {
case 0x1025:
- devideId = SDIO_LOCAL_DEVICE_ID;
+ devide_id = SDIO_LOCAL_DEVICE_ID;
break;
case 0x1026:
- devideId = WLAN_IOREG_DEVICE_ID;
+ devide_id = WLAN_IOREG_DEVICE_ID;
break;
/* case 0x1027: */
-/* devideId = SDIO_FIRMWARE_FIFO; */
+/* devide_id = SDIO_FIRMWARE_FIFO; */
/* break; */
case 0x1031:
- devideId = WLAN_TX_HIQ_DEVICE_ID;
+ devide_id = WLAN_TX_HIQ_DEVICE_ID;
break;
case 0x1032:
- devideId = WLAN_TX_MIQ_DEVICE_ID;
+ devide_id = WLAN_TX_MIQ_DEVICE_ID;
break;
case 0x1033:
- devideId = WLAN_TX_LOQ_DEVICE_ID;
+ devide_id = WLAN_TX_LOQ_DEVICE_ID;
break;
case 0x1034:
- devideId = WLAN_RX0FF_DEVICE_ID;
+ devide_id = WLAN_RX0FF_DEVICE_ID;
break;
default:
-/* devideId = (u8)((addr >> 13) & 0xF); */
- devideId = WLAN_IOREG_DEVICE_ID;
+/* devide_id = (u8)((addr >> 13) & 0xF); */
+ devide_id = WLAN_IOREG_DEVICE_ID;
break;
}
- return devideId;
+ return devide_id;
}
/*
* Ref:
*HalSdioGetCmdAddr8723BSdio()
*/
-static u32 _cvrt2ftaddr(const u32 addr, u8 *pdeviceId, u16 *poffset)
+static u32 _cvrt2ftaddr(const u32 addr, u8 *pdevice_id, u16 *poffset)
{
- u8 deviceId;
+ u8 device_id;
u16 offset;
u32 ftaddr;
- deviceId = get_deviceid(addr);
+ device_id = get_deviceid(addr);
offset = 0;
- switch (deviceId) {
+ switch (device_id) {
case SDIO_LOCAL_DEVICE_ID:
offset = addr & SDIO_LOCAL_MSK;
break;
@@ -140,44 +140,44 @@ static u32 _cvrt2ftaddr(const u32 addr, u8 *pdeviceId,
u16 *poffset)
case WLAN_IOREG_DEVICE_ID:
default:
- deviceId = WLAN_IOREG_DEVICE_ID;
+ device_id = WLAN_IOREG_DEVICE_ID;
offset = addr & WLAN_IOREG_MSK;
break;
}
- ftaddr = (deviceId << 13) | offset;
+ ftaddr = (device_id << 13) | offset;
- if (pdeviceId)
- *pdeviceId = deviceId;
[PATCH 3/4] Staging: rtl8723bs: Change condition to assignment
Change the conditional operator to assignment as it is
not a conditional statement.
Signed-off-by: Shreeya Patel
---
drivers/staging/rtl8723bs/hal/sdio_ops.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8723bs/hal/sdio_ops.c
b/drivers/staging/rtl8723bs/hal/sdio_ops.c
index 00b20c0..314b31f 100644
--- a/drivers/staging/rtl8723bs/hal/sdio_ops.c
+++ b/drivers/staging/rtl8723bs/hal/sdio_ops.c
@@ -460,7 +460,7 @@ static u32 sdio_read_port(
if (mem == NULL) {
DBG_8192C(KERN_WARNING "%s: allocate memory %d bytes
fail!\n", __func__, cnt);
mem = oldmem;
- oldmem == NULL;
+ oldmem = NULL;
}
#else
/* in this case, caller should gurante the buffer is big
enough */
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] Staging: rtl8723bs: Use !x instead of NULL comparison
If "x" is compared to NULL, use "!x" instead of it, so
as to follow the kernel coding style.
Signed-off-by: Shreeya Patel
---
drivers/staging/rtl8723bs/hal/sdio_ops.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/rtl8723bs/hal/sdio_ops.c
b/drivers/staging/rtl8723bs/hal/sdio_ops.c
index 314b31f..08e6a7b 100644
--- a/drivers/staging/rtl8723bs/hal/sdio_ops.c
+++ b/drivers/staging/rtl8723bs/hal/sdio_ops.c
@@ -215,7 +215,7 @@ static u32 sdio_read32(struct intf_hdl *intfhdl, u32 addr)
u8 *tmpbuf;
tmpbuf = rtw_malloc(8);
- if (NULL == tmpbuf) {
+ if (!tmpbuf) {
DBG_8192C(KERN_ERR "%s: Allocate memory FAIL!(size =8)
addr = 0x%x\n", __func__, addr);
return SDIO_ERR_VAL32;
}
@@ -264,7 +264,7 @@ static s32 sdio_readN(struct intf_hdl *intfhdl, u32 addr,
u32 cnt, u8 *buf)
ftaddr &= ~(u16)0x3;
n = cnt + shift;
tmpbuf = rtw_malloc(n);
- if (NULL == tmpbuf)
+ if (!tmpbuf)
return -1;
err = sd_read(intfhdl, ftaddr, n, tmpbuf);
@@ -367,7 +367,7 @@ static s32 sdio_writeN(struct intf_hdl *intfhdl, u32 addr,
u32 cnt, u8 *buf)
ftaddr &= ~(u16)0x3;
n = cnt + shift;
tmpbuf = rtw_malloc(n);
- if (NULL == tmpbuf)
+ if (!tmpbuf)
return -1;
err = sd_read(intfhdl, ftaddr, 4, tmpbuf);
if (err) {
@@ -457,7 +457,7 @@ static u32 sdio_read_port(
#ifdef SDIO_DYNAMIC_ALLOC_MEM
oldmem = mem;
mem = rtw_malloc(cnt);
- if (mem == NULL) {
+ if (!mem) {
DBG_8192C(KERN_WARNING "%s: allocate memory %d bytes
fail!\n", __func__, cnt);
mem = oldmem;
oldmem = NULL;
@@ -745,7 +745,7 @@ static s32 ReadInterrupt8723BSdio(struct adapter *adapter,
u32 *phisr)
u8 val8, hisr_len;
- if (phisr == NULL)
+ if (!phisr)
return false;
himr = GET_HAL_DATA(adapter)->sdio_himr;
@@ -969,13 +969,13 @@ static struct recv_buf *sd_recv_rxfifo(struct adapter
*adapter, u32 size)
/* 3 1. alloc recvbuf */
recv_priv = &adapter->recvpriv;
recvbuf = rtw_dequeue_recvbuf(&recv_priv->free_recv_buf_queue);
- if (recvbuf == NULL) {
+ if (!recvbuf) {
DBG_871X_LEVEL(_drv_err_, "%s: alloc recvbuf FAIL!\n",
__func__);
return NULL;
}
/* 3 2. alloc skb */
- if (recvbuf->pskb == NULL) {
+ if (!recvbuf->pskb) {
SIZE_PTR tmpaddr = 0;
SIZE_PTR alignment = 0;
@@ -989,7 +989,7 @@ static struct recv_buf *sd_recv_rxfifo(struct adapter
*adapter, u32 size)
skb_reserve(recvbuf->pskb, (RECVBUFF_ALIGN_SZ -
alignment));
}
- if (recvbuf->pskb == NULL) {
+ if (!recvbuf->pskb) {
DBG_871X("%s: alloc_skb fail! read =%d\n", __func__,
readsize);
return NULL;
}
@@ -1247,7 +1247,7 @@ u8 RecvOnePkt(struct adapter *adapter, u32 size)
DBG_871X("+%s: size: %d+\n", __func__, size);
- if (adapter == NULL) {
+ if (!adapter) {
DBG_871X(KERN_ERR "%s: adapter is NULL!\n", __func__);
return false;
}
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] Staging: rtl8723bs: Replace true with x and false with !x
On Sun, 2017-12-17 at 01:45 -0800, Joe Perches wrote: > On Sun, 2017-12-17 at 15:07 +0530, Shreeya Patel wrote: > > > > Replace true and false keywords with "x" and "!x" > > respectively to follow the kernel coding style. > [] > > > > diff --git a/drivers/staging/rtl8723bs/hal/sdio_ops.c > > b/drivers/staging/rtl8723bs/hal/sdio_ops.c > [] > > > > @@ -191,8 +191,8 @@ static u32 sdio_read32(struct intf_hdl > > *pintfhdl, u32 addr) > > rtw_hal_get_hwreg(padapter, HW_VAR_APFM_ON_MAC, > > &bMacPwrCtrlOn); > > if ( > > ((deviceId == WLAN_IOREG_DEVICE_ID) && (offset < > > 0x100)) || > > - (false == bMacPwrCtrlOn) || > > - (true == adapter_to_pwrctl(padapter)- > > >bFwCurrentInPSMode) > > + (!bMacPwrCtrlOn) || > When you do these sorts of conversions can you > please remove the unnecessary parentheses too? > Yes, surely I'll keep this in mind. Thank you ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] Staging: rtl8723bs: Replace true with x and false with !x
On Sun, 2017-12-17 at 15:07 +0530, Shreeya Patel wrote: > Replace true and false keywords with "x" and "!x" > respectively to follow the kernel coding style. [] > diff --git a/drivers/staging/rtl8723bs/hal/sdio_ops.c > b/drivers/staging/rtl8723bs/hal/sdio_ops.c [] > @@ -191,8 +191,8 @@ static u32 sdio_read32(struct intf_hdl *pintfhdl, u32 > addr) > rtw_hal_get_hwreg(padapter, HW_VAR_APFM_ON_MAC, &bMacPwrCtrlOn); > if ( > ((deviceId == WLAN_IOREG_DEVICE_ID) && (offset < 0x100)) || > - (false == bMacPwrCtrlOn) || > - (true == adapter_to_pwrctl(padapter)->bFwCurrentInPSMode) > + (!bMacPwrCtrlOn) || When you do these sorts of conversions can you please remove the unnecessary parentheses too? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] hyperv: make HYPERV a menuconfig to ease disabling it all
Hello, On Sat, Dec 16, 2017 at 6:51 PM, Stephen Hemminger wrote: > It makes sense to organize the config if you dont break old configs. > It would be more logical to group and treat all para-virtualized guest > support in same way. Hyper-V should be next to KVM and Xen. I agree, I can try to work on that too, as it would nicely complement the menuconfigifcation. -- Vincent Legoll ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] staging: pi433: Rename enum modShaping in rf69_enum.h
Am 04.12.2017 um 21:18 schrieb Dan Carpenter:
On Mon, Dec 04, 2017 at 08:59:35PM +0200, Marcus Wolf wrote:
Am 04.12.2017 um 12:33 schrieb Dan Carpenter:
On Sun, Dec 03, 2017 at 04:17:26PM +0100, Simon Sandström wrote:
diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
index 34ff0d4807bd..bcfe29840889 100644
--- a/drivers/staging/pi433/pi433_if.h
+++ b/drivers/staging/pi433/pi433_if.h
@@ -63,7 +63,7 @@ struct pi433_tx_cfg {
__u16 bit_rate;
__u32 dev_frequency;
enum modulation modulation;
- enum modShaping modShaping;
+ enum mod_shapingmod_shaping;
I looked at how mod_shaping is set and the only place is in the ioctl:
789 case PI433_IOC_WR_TX_CFG:
790 if (copy_from_user(&instance->tx_cfg, argp,
791 sizeof(struct pi433_tx_cfg)))
792 return -EFAULT;
793 break;
We just write over the whole config. Including important things like
rx_cfg.fixed_message_length. There is no locking so when we do things
like:
385 /* fixed or unlimited length? */
386 if (dev->rx_cfg.fixed_message_length != 0)
387 {
388 if (dev->rx_cfg.fixed_message_length >
dev->rx_buffer_size)
^^
check
389 {
390 retval = -1;
391 goto abort;
392 }
393 bytes_total = dev->rx_cfg.fixed_message_length;
^
set this in the ioctl after the check but before this line and it looks
like a security problem.
394 dev_dbg(dev->dev,"rx: msg len set to %d by fixed
length", bytes_total);
395 }
Anyway, I guess this patch is fine.
regards,
dan carpenter
Hi Dan,
you are mixing rx and tx. The part from IOCTL is copied from the tx-part,
the lower part is dealing with rx.
With rx there should be no problem, since IOCTL is blocked, as long as an rx
operation is going on.
With tx, I also expect no problems, since instance->tx_cfg is never used to
configure the rf69. Everytime, you pass in new data via write() a copy of
tx_cfg is made. Transmission is done, using the copy of the tx_cfg, never by
using instance->tx_cfg.
But maybe I didn't got your point and misunderstand your intention.
No. You're right. I mixed up rx and tx. But the ioctl interface still
seems really horrible. We generally frown on adding new ioctls at all,
but in this case to just write over the whole struct with no locking
seems really bad.
regards,
dan carpenter
Hi Dan,
unexpectetly I was into the driver code today, because a customer asked
for an enhancment. In doing so, I also had a look at the points we
discussed above.
Since both - the tx_cfg and the rx_cfg buffer belong to the instance, in
order to get into trouble, you need to use the same file descriptor. If
an other app is changing its config, it doesn't touch the current app.
So for RX: If a programm has called read(), it won't be able to
succesfully call ioctl any more, because it is blocked:
case PI433_IOC_WR_RX_CFG:
mutex_lock(&device->rx_lock);
/* during pendig read request, change of config not
allowed */
if (device->rx_active) {
mutex_unlock(&device->rx_lock);
return -EAGAIN;
}
For TX in fact there is a little risk:
If a programm is using two tasks and passes the descriptor to both
tasks, one is using the ioctl() and one is using write() and they are
not synchronised, it might happen, that the ioctl is in the middle of
the update the tx_cfg, while the write() is in the middle of copying the
tx_cfg to the kernel fifo.
On one hand, that might be an "open point" at the driver, on the other
hand no one will do such a programm architecture. Even if the driver
will prevent a broken tx_cfg by mutex, the programm will never know,
what it gets, if it issues ioctl() and write() unsynchronised from
different tasks.
For fixing the driver, it might help to lock the write to the tx_cfg in
ioctl() with the tx_fifo_lock, since write() is only copying the tx_cfg
if it has the tx_fifo_lock.
I am not 100% sure. Maybe you (or someone else) want to crosscheck?
Regards,
Marcus
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/2] staging: greybus: Fix alignments
This patch set fixes the alignments errors found by checkpatch.pl Kamal Heib (2): staging: greybus: arche-apb-ctrl.c: Fix alignment should match open parenthesis staging: greybus: arche-platform.c: Fix alignment should match open parenthesis drivers/staging/greybus/arche-apb-ctrl.c | 39 drivers/staging/greybus/arche-platform.c | 21 + 2 files changed, 32 insertions(+), 28 deletions(-) -- 2.14.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: greybus: arche-apb-ctrl.c: Fix alignment should match open parenthesis
Fix "alignment should match open parenthesis" checkpatch.pl error.
Signed-off-by: Kamal Heib
---
drivers/staging/greybus/arche-apb-ctrl.c | 39
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/greybus/arche-apb-ctrl.c
b/drivers/staging/greybus/arche-apb-ctrl.c
index b0c66112eb18..cc8d6fc831b4 100644
--- a/drivers/staging/greybus/arche-apb-ctrl.c
+++ b/drivers/staging/greybus/arche-apb-ctrl.c
@@ -72,14 +72,14 @@ static int coldboot_seq(struct platform_device *pdev)
int ret;
if (apb->init_disabled ||
- apb->state == ARCHE_PLATFORM_STATE_ACTIVE)
+ apb->state == ARCHE_PLATFORM_STATE_ACTIVE)
return 0;
/* Hold APB in reset state */
assert_reset(apb->resetn_gpio);
if (apb->state == ARCHE_PLATFORM_STATE_FW_FLASHING &&
- gpio_is_valid(apb->spi_en_gpio))
+ gpio_is_valid(apb->spi_en_gpio))
devm_gpio_free(dev, apb->spi_en_gpio);
/* Enable power to APB */
@@ -122,7 +122,7 @@ static int fw_flashing_seq(struct platform_device *pdev)
int ret;
if (apb->init_disabled ||
- apb->state == ARCHE_PLATFORM_STATE_FW_FLASHING)
+ apb->state == ARCHE_PLATFORM_STATE_FW_FLASHING)
return 0;
ret = regulator_enable(apb->vcore);
@@ -146,7 +146,7 @@ static int fw_flashing_seq(struct platform_device *pdev)
flags = GPIOF_OUT_INIT_LOW;
ret = devm_gpio_request_one(dev, apb->spi_en_gpio,
- flags, "apb_spi_en");
+ flags, "apb_spi_en");
if (ret) {
dev_err(dev, "Failed requesting SPI bus en gpio %d\n",
apb->spi_en_gpio);
@@ -174,11 +174,11 @@ static int standby_boot_seq(struct platform_device *pdev)
* then we do not want to change the state
*/
if (apb->state == ARCHE_PLATFORM_STATE_STANDBY ||
- apb->state == ARCHE_PLATFORM_STATE_OFF)
+ apb->state == ARCHE_PLATFORM_STATE_OFF)
return 0;
if (apb->state == ARCHE_PLATFORM_STATE_FW_FLASHING &&
- gpio_is_valid(apb->spi_en_gpio))
+ gpio_is_valid(apb->spi_en_gpio))
devm_gpio_free(dev, apb->spi_en_gpio);
/*
@@ -203,7 +203,7 @@ static void poweroff_seq(struct platform_device *pdev)
return;
if (apb->state == ARCHE_PLATFORM_STATE_FW_FLASHING &&
- gpio_is_valid(apb->spi_en_gpio))
+ gpio_is_valid(apb->spi_en_gpio))
devm_gpio_free(dev, apb->spi_en_gpio);
/* disable the clock */
@@ -251,7 +251,8 @@ void apb_ctrl_poweroff(struct device *dev)
}
static ssize_t state_store(struct device *dev,
- struct device_attribute *attr, const char *buf, size_t count)
+ struct device_attribute *attr,
+ const char *buf, size_t count)
{
struct platform_device *pdev = to_platform_device(dev);
struct arche_apb_ctrl_drvdata *apb = platform_get_drvdata(pdev);
@@ -297,7 +298,7 @@ static ssize_t state_store(struct device *dev,
}
static ssize_t state_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+ struct device_attribute *attr, char *buf)
{
struct arche_apb_ctrl_drvdata *apb = dev_get_drvdata(dev);
@@ -319,7 +320,7 @@ static ssize_t state_show(struct device *dev,
static DEVICE_ATTR_RW(state);
static int apb_ctrl_get_devtree_data(struct platform_device *pdev,
- struct arche_apb_ctrl_drvdata *apb)
+struct arche_apb_ctrl_drvdata *apb)
{
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
@@ -331,10 +332,10 @@ static int apb_ctrl_get_devtree_data(struct
platform_device *pdev,
return apb->resetn_gpio;
}
ret = devm_gpio_request_one(dev, apb->resetn_gpio,
- GPIOF_OUT_INIT_LOW, "apb-reset");
+ GPIOF_OUT_INIT_LOW, "apb-reset");
if (ret) {
dev_err(dev, "Failed requesting reset gpio %d\n",
- apb->resetn_gpio);
+ apb->resetn_gpio);
return ret;
}
@@ -344,10 +345,10 @@ static int apb_ctrl_get_devtree_data(struct
platform_device *pdev,
return apb->boot_ret_gpio;
}
ret = devm_gpio_request_one(dev, apb->boot_ret_gpio,
- GPIOF_OUT_INIT_LOW, "boot retention");
+ GPIOF_OUT_INIT_LOW, "boot retention");
if (ret) {
dev_err(dev, "Failed requesting bootret gpio %d\n",
- apb->boot_ret_gpio);
+
[PATCH 2/2] staging: greybus: arche-platform.c: Fix alignment should match open parenthesis
Fix "alignment should match open parenthesis" checkpatch.pl error.
Signed-off-by: Kamal Heib
---
drivers/staging/greybus/arche-platform.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/greybus/arche-platform.c
b/drivers/staging/greybus/arche-platform.c
index ace4eb365c0e..83254a72a7bb 100644
--- a/drivers/staging/greybus/arche-platform.c
+++ b/drivers/staging/greybus/arche-platform.c
@@ -233,7 +233,7 @@ arche_platform_coldboot_seq(struct arche_platform_drvdata
*arche_pdata)
ret = clk_prepare_enable(arche_pdata->svc_ref_clk);
if (ret) {
dev_err(arche_pdata->dev, "failed to enable svc_ref_clk: %d\n",
- ret);
+ ret);
return ret;
}
@@ -269,7 +269,7 @@ arche_platform_fw_flashing_seq(struct
arche_platform_drvdata *arche_pdata)
ret = clk_prepare_enable(arche_pdata->svc_ref_clk);
if (ret) {
dev_err(arche_pdata->dev, "failed to enable svc_ref_clk: %d\n",
- ret);
+ ret);
return ret;
}
@@ -312,7 +312,8 @@ arche_platform_poweroff_seq(struct arche_platform_drvdata
*arche_pdata)
}
static ssize_t state_store(struct device *dev,
- struct device_attribute *attr, const char *buf, size_t count)
+ struct device_attribute *attr,
+ const char *buf, size_t count)
{
struct platform_device *pdev = to_platform_device(dev);
struct arche_platform_drvdata *arche_pdata = platform_get_drvdata(pdev);
@@ -376,7 +377,7 @@ static ssize_t state_store(struct device *dev,
}
static ssize_t state_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+ struct device_attribute *attr, char *buf)
{
struct arche_platform_drvdata *arche_pdata = dev_get_drvdata(dev);
@@ -443,7 +444,7 @@ static int arche_platform_probe(struct platform_device
*pdev)
/* setup svc reset gpio */
arche_pdata->is_reset_act_hi = of_property_read_bool(np,
- "svc,reset-active-high");
+
"svc,reset-active-high");
arche_pdata->svc_reset_gpio = of_get_named_gpio(np,
"svc,reset-gpio",
0);
@@ -457,7 +458,7 @@ static int arche_platform_probe(struct platform_device
*pdev)
return ret;
}
ret = gpio_direction_output(arche_pdata->svc_reset_gpio,
- arche_pdata->is_reset_act_hi);
+ arche_pdata->is_reset_act_hi);
if (ret) {
dev_err(dev, "failed to set svc-reset gpio dir:%d\n", ret);
return ret;
@@ -465,7 +466,8 @@ static int arche_platform_probe(struct platform_device
*pdev)
arche_platform_set_state(arche_pdata, ARCHE_PLATFORM_STATE_OFF);
arche_pdata->svc_sysboot_gpio = of_get_named_gpio(np,
- "svc,sysboot-gpio", 0);
+ "svc,sysboot-gpio",
+ 0);
if (arche_pdata->svc_sysboot_gpio < 0) {
dev_err(dev, "failed to get sysboot gpio\n");
return arche_pdata->svc_sysboot_gpio;
@@ -483,7 +485,8 @@ static int arche_platform_probe(struct platform_device
*pdev)
/* setup the clock request gpio first */
arche_pdata->svc_refclk_req = of_get_named_gpio(np,
- "svc,refclk-req-gpio", 0);
+ "svc,refclk-req-gpio",
+ 0);
if (arche_pdata->svc_refclk_req < 0) {
dev_err(dev, "failed to get svc clock-req gpio\n");
return arche_pdata->svc_refclk_req;
@@ -525,7 +528,7 @@ static int arche_platform_probe(struct platform_device
*pdev)
"wake detect");
if (ret) {
dev_err(dev, "Failed requesting wake_detect gpio %d\n",
- arche_pdata->wake_detect_gpio);
+ arche_pdata->wake_detect_gpio);
return ret;
}
--
2.14.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 0/4] NVIDIA Tegra video decoder driver
On 12.12.2017 03:26, Dmitry Osipenko wrote: > VDE driver provides accelerated video decoding to NVIDIA Tegra SoC's, > it is a result of reverse-engineering efforts. Driver has been tested on > Toshiba AC100 and Acer A500, it should work on any Tegra20 device. > > In userspace this driver is utilized by libvdpau-tegra [0] that implements > VDPAU interface, so any video player that supports VDPAU can provide > accelerated video decoding on Tegra20 on Linux. > > [0] https://github.com/grate-driver/libvdpau-tegra Thierry, driver has been approved by media maintainers and should appear in 4.16 (it is already in -next). Please schedule the DT patches for 4.16, thanks. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv2 0/6] staging: pi433: pi433_if.c various codestyle fixes
On 13/12/17, Oliver Graute wrote: > rearranged the patches and summarized them > > Oliver Graute (6): > staging: pi433: pi433_if.c codestyle space required > staging: pi433: pi433_if.c style fix wrong placed brace > staging: pi433: pi433_if.c style open brace > staging: pi433: pi433_if.c space required in for loop > staging: pi433: pi433_if.c style fix forbidden spaces > staging: pi433: pi433_if.c style fix of space prohibited these changes are no longer necessary because they already fixed by Vincent patch series. Best Regards, Oliver ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vchiq_arm: fix open brace placement errors
Fix checkpatch errors relating to open brace placement for enums and
function definitions.
Signed-off-by: Tara Null
---
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 9 +
1 file changed, 5 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 ecff92bae200..5d28fff46557 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -66,8 +66,7 @@ struct vchiq_openack_payload {
short version;
};
-enum
-{
+enum {
QMFLAGS_IS_BLOCKING = (1 << 0),
QMFLAGS_NO_MUTEX_LOCK = (1 << 1),
QMFLAGS_NO_MUTEX_UNLOCK = (1 << 2)
@@ -212,7 +211,8 @@ find_service_by_port(VCHIQ_STATE_T *state, int localport)
VCHIQ_SERVICE_T *
find_service_for_instance(VCHIQ_INSTANCE_T instance,
- VCHIQ_SERVICE_HANDLE_T handle) {
+ VCHIQ_SERVICE_HANDLE_T handle)
+{
VCHIQ_SERVICE_T *service;
spin_lock(&service_spinlock);
@@ -235,7 +235,8 @@ find_service_for_instance(VCHIQ_INSTANCE_T instance,
VCHIQ_SERVICE_T *
find_closed_service_for_instance(VCHIQ_INSTANCE_T instance,
- VCHIQ_SERVICE_HANDLE_T handle) {
+ VCHIQ_SERVICE_HANDLE_T handle)
+{
VCHIQ_SERVICE_T *service;
spin_lock(&service_spinlock);
--
2.15.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
