Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1532015

Re: [PATCH 0/2] media protect enable and disable source handler paths

From Mauro Carvalho Chehab <mchehab@s-opensource.com>
Newsgroups linux.kernel
Subject Re: [PATCH 0/2] media protect enable and disable source handler paths
Date 2016-11-29 10:20 +0100
Message-ID <sIMJX-6yn-1@gated-at.bofh.it> (permalink)
References <sIGbv-1YA-1@gated-at.bofh.it>
Organization Samsung

Show all headers | View raw


Em Mon, 28 Nov 2016 19:15:12 -0700
Shuah Khan <shuahkh@osg.samsung.com> escreveu:

> These two patches fix enable and disable source handler paths. These
> aren't dependent patches, grouped because they fix similar problems.

Those two patches should be fold, as applying just the first patch
would cause au0828 to try to double lock.

> 
> This work is triggered by a review comment from Mauro Chehab on a
> snd_usb_audio patch about protecting the enable and disabel handler
> path in it.
> 
> Ran tests to make sure enable and disable handler paths work. When
> digital stream is active, analog app finds the tuner busy and vice
> versa. Also ran the Sakari's unbind while video stream is active test.

Sorry, but your patches descriptions don't make things clear:

- It doesn't present any OOPS or logs that would help to
  understand what you're trying to fix;

- From what I understood, you're moving the lock out of
  enable/disable handlers, and letting their callers to do
  the locks themselves. Why? Are there any condition where it
  won't need to be locked?

- It is not touching documentation. If now the callbacks should
  not implement locks, this should be explicitly described.

Btw, I think it is a bad idea to let the callers to handle
the locks. The best would be, instead, to change the code in
some other way to avoid it, if possible. If not possible at all,
clearly describe why it is not possible and insert some comments
inside the code, to avoid some cleanup patch to mess up with this.

Regards

Thanks,
Mauro

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 0/2] media protect enable and disable source handler paths Shuah Khan <shuahkh@osg.samsung.com> - 2016-11-29 03:20 +0100
  [PATCH 2/2] media: protect enable and disable source handler checks and calls Shuah Khan <shuahkh@osg.samsung.com> - 2016-11-29 03:20 +0100
    Re: [PATCH 2/2] media: protect enable and disable source handler  checks and calls Sakari Ailus <sakari.ailus@iki.fi> - 2016-11-29 10:30 +0100
      Re: [PATCH 2/2] media: protect enable and disable source handler  checks and calls Shuah Khan <shuahkh@osg.samsung.com> - 2016-11-29 18:50 +0100
        Re: [PATCH 2/2] media: protect enable and disable source handler  checks and calls Sakari Ailus <sakari.ailus@iki.fi> - 2016-12-01 15:00 +0100
          Re: [PATCH 2/2] media: protect enable and disable source handler  checks and calls Shuah Khan <shuahkh@osg.samsung.com> - 2016-12-01 18:00 +0100
  [PATCH 1/2] media: au0828 fix to protect enable/disable source set and clear Shuah Khan <shuahkh@osg.samsung.com> - 2016-11-29 03:20 +0100
  Re: [PATCH 0/2] media protect enable and disable source handler  paths Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2016-11-29 10:20 +0100
    Re: [PATCH 0/2] media protect enable and disable source handler paths Shuah Khan <shuahkh@osg.samsung.com> - 2016-11-29 18:10 +0100
      Re: [PATCH 0/2] media protect enable and disable source handler  paths Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2016-11-29 18:20 +0100

csiph-web