Re: [PATCH] media: dvbsky: use a single mutex and state buffers for all R/W ops

[Sean Young]

On Sat, Jun 15, 2019 at 10:47:28AM +0300, Andrei Koshkosh wrote:
> ---
>  drivers/media/usb/dvb-usb-v2/dvbsky.c | 18 ++
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 

Missing signed-off-by. Please read:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

> diff --git a/drivers/media/usb/dvb-usb-v2/dvbsky.c 
> b/drivers/media/usb/dvb-usb-v2/dvbsky.c
> index c41e10b..0b5da891 100644
> --- a/drivers/media/usb/dvb-usb-v2/dvbsky.c
> +++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c
> @@ -22,7 +22,7 @@ MODULE_PARM_DESC(disable_rc, "Disable inbuilt IR 
> receiver.");
>  DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
>  
>  struct dvbsky_state {
> - struct mutex stream_mutex;
> + //struct mutex stream_mutex;

Please remove rather comment out.


>   u8 ibuf[DVBSKY_BUF_LEN];
>   u8 obuf[DVBSKY_BUF_LEN];
>   u8 last_lock;
> @@ -61,16 +61,18 @@ static int dvbsky_stream_ctrl(struct dvb_usb_device *d, 
> u8 onoff)
>  {
>   struct dvbsky_state *state = d_to_priv(d);
>   int ret;
> - u8 obuf_pre[3] = { 0x37, 0, 0 };
> - u8 obuf_post[3] = { 0x36, 3, 0 };
> + static u8 obuf_pre[3] = { 0x37, 0, 0 };
> + static u8 obuf_post[3] = { 0x36, 3, 0 };

Can these be const?
>
> - mutex_lock(>stream_mutex);
> - ret = dvbsky_usb_generic_rw(d, obuf_pre, 3, NULL, 0);
> + mutex_lock(>usb_mutex);
> + memcpy(state->obuf, obuf_pre, 3);
> + ret = dvb_usbv2_generic_write_locked(d, state->obuf, 3);
>   if (!ret && onoff) {
>   msleep(20);
> - ret = dvbsky_usb_generic_rw(d, obuf_post, 3, NULL, 0);
> + memcpy(state->obuf, obuf_post, 3);
> + ret = dvb_usbv2_generic_write_locked(d, state->obuf, 3);
>   }
> - mutex_unlock(>stream_mutex);
> + mutex_unlock(>usb_mutex);
>   return ret;
>  }
>  
> @@ -599,7 +601,7 @@ static int dvbsky_init(struct dvb_usb_device *d)
>   if (ret)
>   return ret;
>   */
> - mutex_init(>stream_mutex);
> + //mutex_init(>stream_mutex);

Remove.

>  
>   state->last_lock = 0;
>  
> -- 
> 2.7.4

Thanks,
Sean

[PATCH] media: dvbsky: use a single mutex and state buffers for all R/W ops

[Andrei Koshkosh]

---
 drivers/media/usb/dvb-usb-v2/dvbsky.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/dvbsky.c 
b/drivers/media/usb/dvb-usb-v2/dvbsky.c
index c41e10b..0b5da891 100644
--- a/drivers/media/usb/dvb-usb-v2/dvbsky.c
+++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c
@@ -22,7 +22,7 @@ MODULE_PARM_DESC(disable_rc, "Disable inbuilt IR receiver.");
 DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
 
 struct dvbsky_state {
-   struct mutex stream_mutex;
+   //struct mutex stream_mutex;
u8 ibuf[DVBSKY_BUF_LEN];
u8 obuf[DVBSKY_BUF_LEN];
u8 last_lock;
@@ -61,16 +61,18 @@ static int dvbsky_stream_ctrl(struct dvb_usb_device *d, u8 
onoff)
 {
struct dvbsky_state *state = d_to_priv(d);
int ret;
-   u8 obuf_pre[3] = { 0x37, 0, 0 };
-   u8 obuf_post[3] = { 0x36, 3, 0 };
+   static u8 obuf_pre[3] = { 0x37, 0, 0 };
+   static u8 obuf_post[3] = { 0x36, 3, 0 };
 
-   mutex_lock(>stream_mutex);
-   ret = dvbsky_usb_generic_rw(d, obuf_pre, 3, NULL, 0);
+   mutex_lock(>usb_mutex);
+   memcpy(state->obuf, obuf_pre, 3);
+   ret = dvb_usbv2_generic_write_locked(d, state->obuf, 3);
if (!ret && onoff) {
msleep(20);
-   ret = dvbsky_usb_generic_rw(d, obuf_post, 3, NULL, 0);
+   memcpy(state->obuf, obuf_post, 3);
+   ret = dvb_usbv2_generic_write_locked(d, state->obuf, 3);
}
-   mutex_unlock(>stream_mutex);
+   mutex_unlock(>usb_mutex);
return ret;
 }
 
@@ -599,7 +601,7 @@ static int dvbsky_init(struct dvb_usb_device *d)
if (ret)
return ret;
*/
-   mutex_init(>stream_mutex);
+   //mutex_init(>stream_mutex);
 
state->last_lock = 0;
 
-- 
2.7.4