On 09/24/2014 05:26 AM, Hans Verkuil wrote: > Hi Shuah, > > Here is my review... > > On 09/22/2014 05:00 PM, Shuah Khan wrote: >> Add media token device resource framework to allow sharing >> resources such as tuner, dma, audio etc. across media drivers >> and non-media sound drivers that control media hardware. The >> Media token resource is created at the main struct device that >> is common to all drivers that claim various pieces of the main >> media device, which allows them to find the resource using the >> main struct device. As an example, digital, analog, and >> snd-usb-audio drivers can use the media token resource API >> using the main struct device for the interface the media device >> is attached to. >> >> The media token resource contains token for tuner, dma, and >> audio. > > Why dma and audio? Neither is being used in this patch series. I > would leave them out until you actually show how they are used in a > driver. Yeah I can remove these. I am not using them in this series. >> +static int __media_get_tkn(struct media_tkn *tkn, >> + enum media_tkn_mode mode, bool exclusive) >> +{ >> + int rc = 0; >> + >> + spin_lock(&tkn->lock); >> + if (tkn->is_exclusive) >> + rc = -EBUSY; >> + else if (tkn->owners && ((mode != tkn->mode) || exclusive)) >> + rc = -EBUSY; >> + else { >> + if (tkn->owners < INT_MAX) >> + tkn->owners++; >> + else >> + tkn->owners = 1; > > Somewhat weird. Can owners ever become INT_MAX? > I didn't have this at first. I was testing with tvtime and noticed the count going up to 40k+ while it is streaming. The count kept going up. So I figured I might as well add the check to cover for cases where application like tvtime run for a very longtime like a couple of hours. thanks, -- Shuah -- Shuah Khan Sr. Linux Kernel Developer Samsung Research America (Silicon Valley) shuahkh@xxxxxxxxxxxxxxx | (970) 217-8978 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html