> Em Thu, 8 Jun 2017 09:42:43 +0000 > Ramesh Shanmugasundaram <ramesh.shanmugasundaram@xxxxxxxxxxxxxx> escreveu: > > > > Subject: Re: [PATCH v6 3/7] media: i2c: max2175: Add MAX2175 support > > > > > > Em Wed, 31 May 2017 09:44:53 +0100 > > > Ramesh Shanmugasundaram <ramesh.shanmugasundaram@xxxxxxxxxxxxxx> > escreveu: > > > > > > > +++ b/Documentation/media/v4l-drivers/max2175.rst > > > > @@ -0,0 +1,60 @@ > > > > +Maxim Integrated MAX2175 RF to bits tuner driver > > > > +================================================ > > > > + > > > > +The MAX2175 driver implements the following driver-specific > controls: > > > > + > > > > +``V4L2_CID_MAX2175_I2S_ENABLE`` > > > > +------------------------------- > > > > + Enable/Disable I2S output of the tuner. > > > > + > > > > +.. flat-table:: > > > > + :header-rows: 0 > > > > + :stub-columns: 0 > > > > + :widths: 1 4 > > > > + > > > > + * - ``(0)`` > > > > + - I2S output is disabled. > > > > + * - ``(1)`` > > > > + - I2S output is enabled. > > > > > > Hmm... There are other drivers at the subsystem that use I2S (for > > > audio - not for SDR - but I guess the issue is similar). > > > > > > On such drivers, the bridge driver controls it directly, being sure > > > that I2S is enabled when it is expecting some data coming from the I2S > bus. > > > > > > On some drivers, there are both I2S and A/D inputs at the bridge > chipset. > > > On such drivers, enabling/disabling I2S is done via VIDIOC_S_INPUT > > > (and optionally via ALSA mixer), being transparent to the user if > > > the stream comes from a tuner via I2S or from a directly connected A/D > input. > > > > > > I don't think it is a good idea to enable it via a control, as, if > > > the bridge driver is expecting data via I2S, disabling it will cause > > > timeouts at the videobuf handling. > > > > The MAX2175 device is exposed as a v4l2 subdev with tuner ops and can > interface with an SDR device. When the tuner is configured, the I2S output > is enabled by default. From an independent tuner device perspective, this > default behaviour is enough and this control may not be needed/used. > > > > However, for the use case here, the R-Car DRIF device acts as the main > SDR device and the Maxim MAX2175 provides a sub-dev interface with tuner > ops. > > > > +---------------------+ +---------------------+ > > | |-----SCK------->|CLK | > > | Master |-----SS-------->|SYNC DRIFn (slave) | > > | (MAX2175) |-----SD0------->|D0 | > > | |-----SD1------->|D1 | > > +---------------------+ +---------------------+ > > > > The DRIF device design is such that it involves separate register writes > to enable Rx on each of the data line. To keep both the data lines in sync > it expects the master device to enable output after both the data line Rx > are enabled. > > > > This level of control is exposed as a feature in the MAX2175 using this > control. When interfaced with DRIF this control is used to achieve the > desired functionality. When not interfaced with DRIF, the MAX2175 default > behaviour does not have to change because of DRIF and hence this I2S > control may be unused. Like MAX2175, DRIF is also an independent device > and can interface with a different third party tuner. > > > > Hence, this I2S enable/disable is exposed as a user control. The end > user application (knowing both these devices) is expected to use these > controls appropriately. Please let me know if I need to explain anything > in further detail. > > > The usecase is clear. That's exactly what other drivers with I2S do, > except that, on those other drivers, they pass I2S control info via > platform_data (they're not platform drivers). > > With those drivers, generic applications work as-is via the standard > video, radio or sdr devnodes, without knowing about I2S. > > The main difference here is that you're requiring an specialized > application for this device to work, Specialized application may be needed for this specific combination of devices (DRIF + MAX2175) only. For MAX2175 alone, a generic application itself would be enough. The MAX2175 will have I2S output enabled by default and the SDR device have to just enable/disable its own RX during .start_streaming/.stop_streaming. as a generic one won't be aware of > this device-specific control, and may end by exposing this "internal" > control to the end user. That is OK for embedded usage, but, as soon as > this is used on some non-embedded usecase (with is likely, as there are > several PC consumer products using other chips from Maxim), we'll have > problems. > > I guess the solution here is to make such control visible only via the > subdev interface. > Did you mean using v4l2_subdev_tuner_ops? It does not have a method as of today to support this. The video, audio subdev ops supports s_stream method. We may need to create one for tuner ops. Thanks, Ramesh