On 28/08/17 12:30, Mauro Carvalho Chehab wrote: > Em Mon, 28 Aug 2017 12:05:06 +0200 > Hans Verkuil <hverkuil@xxxxxxxxx> escreveu: > >> On 26/08/17 13:53, Mauro Carvalho Chehab wrote: >>> The documentation doesn't mention if vdev-centric hardware >>> control would have subdev API or not. >>> >>> Add a notice about that, reflecting the current status, where >>> three drivers use it, in order to support some subdev-specific >>> controls. >> >> I posted a patch removing v4l-subdevX support for cobalt. It's only used >> within Cisco, so this is safe to do and won't break any userspace support. > > OK. > >> atmel-isc is another driver that creates subdev nodes. Like cobalt, this >> is unnecessary. There are no sensors that use private controls. > > The question is not if the driver has private controls. Private controls > can be V4L2 device node oriented. > > The real question is if userspace applications use subdevs or not in > order to set something specific to a subdev, on a pipeline where > multiple subdevs could use the same control. > > E. g. even on a simple case where the driver would have something like: > > sensor -> processing -> DMA > > both "sensor" and "processing" could provide the same control > (bright, contrast, gain, or whatever). Only by exposing such > control via subdev is possible to pinpoint what part of the > hardware pipeline would be affected when such control is changed. In theory, yes. In practice this does not happen for any of the V4L2-centric drivers. Including for the three drivers under discussion. > >> This driver is not referenced anywhere (dts or board file) in the kernel. >> It is highly unlikely anyone would use v4l-subdevX nodes when there is no >> need to do so. My suggestion is to add a kernel option for this driver >> to enable v4l-subdevX support, but set it to 'default n'. Perhaps with >> a note in the Kconfig description and a message in the kernel log that >> this will be removed in the future. >> >> The final driver is rcar_drif that uses this to set the "I2S Enable" >> private control of the max2175 driver. >> >> I remember that there was a long discussion over this control. I still >> think that there is no need to mark this private. > > The problem with I2S is that a device may have multiple places > where I2S could be used. I don't know how the rcar-drif driver uses > it, but there are several vdev-centric boards that use I2S for audio. > > On several of the devices I worked with, the I2S can be enabled, in > runtime, if the audio signal would be directed to some digital output, > or it can be disabled if the audio signal would be directed to some > analog output. Thankfully, on those devices, I2S can be indirectly > controlled via either an ALSA mixer or via VIDIOC A/V routing > ioctls. Also, there's just one I2S bus on them. > > However, on a device that have multiple I2S bus, userspace should > be able to control each of them individually, as some parts of the > pipeline may require it enabled while others may require it > disabled. So, I strongly believe that this should be a subdev > control on such hardware. > > That's said, I don't know how rcar_drif uses it. If it has just > one I2S bus and it is used only for audio, then VIDIOC A/V routing > ioctls and/or an ALSA mixer could replace it. If not, then > it should be kept as-is and the driver would need to add support > for MC, in order for applications to identify the right > sub-devices that are associated with the pipelines where I2S > will be controlled. Ramesh, do applications using rcar_drif + max2175 have to manually enable the i2s? Shouldn't this be part of the device tree description instead? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
