Re: [PATCH 04/22] media: Move v4l2_fwnode_parse_link from v4l2 to driver base

[Steve Longerbeam]

On 8/14/19 4:15 PM, Russell King - ARM Linux admin wrote:
> On Wed, Aug 14, 2019 at 04:00:30PM -0700, Steve Longerbeam wrote:
> > On 8/14/19 3:04 PM, Russell King - ARM Linux admin wrote:
> > > On Wed, Aug 14, 2019 at 12:04:41PM -0700, Steve Longerbeam wrote:
> > > > On 8/14/19 3:30 AM, Russell King - ARM Linux admin wrote:
> > > > > On Tue, Aug 06, 2019 at 09:53:41AM -0700, Steve Longerbeam wrote:
> > > > > > The full patchset doesn't seem to be up yet, but see [1] for the cover
> > > > > > letter.
> > > > > Was the entire series copied to the mailing lists, or just selected
> > > > > patches?  I only saw 4, 9, 11 and 13-22 via lakml.
> > > > The whole series was posted to the linux-media ML, see [1]. At the time,
> > > > none of the linux-media ML archives had the whole series.
> > > >
> > > > > In the absence of the other patches, will this solve imx-media binding
> > > > > the internal subdevs of sensor devices to the CSI2 interface?
> > > > "internal subdevs of sensor devices" ?? That doesn't make any sense.
> > > Sorry, but it makes complete sense when you consider that sensor
> > > devices may have more than one subdev, but there should be only one
> > > that is the "output" to whatever the camera is attached to.  The
> > > other subdevs are internal to the sensor.
> > Ah, thanks for the clarification. Yes, by "internal subdevs" I understand
> > what you mean now. The adv748x and smiapp are examples.
> >
> > > subdevs are not purely the remit of SoC drivers.
> > So there is no binding of internal subdevs to the receiver CSI-2. The
> > receiver CSI-2 subdev will create media links to the subdev that has an
> > externally exposed fwnode endpoint that connects with the CSI-2 sink pad.
> Maybe - with 5.2, I get:
>
> - entity 15: imx6-mipi-csi2 (5 pads, 6 links)
>               type V4L2 subdev subtype Unknown flags 0
>               device node name /dev/v4l-subdev2
>          pad0: Sink
> ...
>                  <- "imx219 0-0010":0 []
>                  <- "imx219 pixel 0-0010":0 []
>
> Adding some debug in gives:
>
> [   11.963362] imx-media: imx_media_create_of_links() for imx6-mipi-csi2
> [   11.963396] imx-media: create_of_link(): /soc/aips-bus@2000000/iomuxc-gpr@20e0000/ipu1_csi0_mux
> [   11.963422] imx-media: create_of_link(): /soc/ipu@2400000
> [   11.963450] imx-media: create_of_link(): /soc/ipu@2800000
> [   11.963478] imx-media: create_of_link(): /soc/aips-bus@2000000/iomuxc-gpr@20e0000/ipu2_csi1_mux
> [   11.963489] imx-media: imx6-mipi-csi2:4 -> ipu2_csi1_mux:0
> [   11.963522] imx-media: create_of_link(): /soc/aips-bus@2100000/i2c@21a0000/camera@10
> [   11.963533] imx-media: imx219 0-0010:0 -> imx6-mipi-csi2:0
> [   11.963549] imx-media: imx_media_create_of_links() for imx219 pixel 0-0010
> [   11.963577] imx-media: create_of_link(): /soc/aips-bus@2100000/mipi@21dc000
> [   11.963587] imx-media: imx219 pixel 0-0010:0 -> imx6-mipi-csi2:0
> [   11.963602] imx-media: imx_media_create_of_links() for imx219 0-0010
>
> Note that it's not created by imx6-mipi-csi2, but by imx-media delving
> around in the imx219 subdevs.
>
>  From what I can see, smiapp does the same thing that I do in imx219 -
> sets the subdev->dev member to point at the struct device, which then
> means that v4l2_device_register_subdev() will associate the same fwnode
> with both "imx219 pixel 0-0010" and "imx219 0-0010".

Ok, understood.

I realize imx_media_create_of_link() is a bit intrusive, and that's one 
of the things I'm trying to get rid of in this patchset. Unfortunately 
it's there for a reason which is described in patch 0021. But to explain 
here, the imx6-mipi-csi2 receiver outputs its four virtual channels on 
four separate source pads, and those connect to four different 
subdevices (video mux's and CSI's), and the problem is that only the 
first subdev that adds imx6-mipi-csi2 to its notifier asd list will get 
a notifier bind() callback (where links can be created to 
imx6-mipi-csi2) -- the other subdevs don't contain it in their asd lists 
so they never create the links to imx6-mipi-csi2. So until the 
requirement in v4l2-async that no notifiers can contain the same asd in 
its list is relaxed, this function will have to remain, but I can make 
it less intrusive (only create the missing links from imx6-mipi-csi2). 
I'm not able to find a cleaner workaround at the moment.

Steve