Message ID | 20201211155708.154710-1-paul.kocialkowski@bootlin.com |
---|---|
Headers | show |
Series | Allwinner MIPI CSI-2 support for A31/V3s/A83T | expand |
On Fri, Dec 11, 2020 at 04:57:00PM +0100, Paul Kocialkowski wrote: > The A31 CSI controller supports a MIPI CSI-2 bridge input, which has > its own dedicated port in the fwnode graph. > > Support for this input is added with this change: > - two pads are defined for the media entity instead of one > and only one needs to be connected at a time; > - the pads currently match the fwnode graph representation; > - links are created between our pads and the subdevs for each > interface and are no longer immutable so that userspace can select > which interface to use in case both are bound to a subdev; > - fwnode endpoints are parsed and stored for each interface; > - the active subdev (and fwnode endpoint) is retrieved when validating > the media link at stream on time and cleared at stream off; > - an error is raised if both links are active at the same time; > - the MIPI interface bit is set if the MIPI CSI-2 bridge endpoint is > active. > > In the future, the media entity representation might evolve to: > - distinguish the internal parallel bridge and data formatter; > - represent each of the 4 internal channels that can exist between > the parallel bridge (for BT656 time-multiplex) and MIPI CSI-2 > (internal channels can be mapped to virtual channels); > - connect the controller's output to the ISP instead of its > DMA engine. > > Finally note that the MIPI CSI-2 bridges should not be linked in > the fwnode graph unless they have a sensor subdev attached. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 123 ++++++++++++++---- > .../platform/sunxi/sun6i-csi/sun6i_csi.h | 3 - > .../platform/sunxi/sun6i-csi/sun6i_video.c | 53 ++++---- > .../platform/sunxi/sun6i-csi/sun6i_video.h | 7 +- > 4 files changed, 135 insertions(+), 51 deletions(-) > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c > index f1150de94e98..481181038e1e 100644 > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c > @@ -52,15 +52,16 @@ bool sun6i_csi_is_format_supported(struct sun6i_csi *csi, > u32 pixformat, u32 mbus_code) > { > struct sun6i_csi_dev *sdev = sun6i_csi_to_dev(csi); > + struct v4l2_fwnode_endpoint *endpoint = sdev->csi.video.source_endpoint; > > /* > * Some video receivers have the ability to be compatible with > * 8bit and 16bit bus width. > * Identify the media bus format from device tree. > */ > - if ((sdev->csi.v4l2_ep.bus_type == V4L2_MBUS_PARALLEL > - || sdev->csi.v4l2_ep.bus_type == V4L2_MBUS_BT656) > - && sdev->csi.v4l2_ep.bus.parallel.bus_width == 16) { > + if ((endpoint->bus_type == V4L2_MBUS_PARALLEL > + || endpoint->bus_type == V4L2_MBUS_BT656) > + && endpoint->bus.parallel.bus_width == 16) { The operators should be at the end of the previous line, not at the beginning Maxime
On Fri, Dec 11, 2020 at 04:57:02PM +0100, Paul Kocialkowski wrote: > +#define sun6i_mipi_csi2_subdev_video(subdev) \ > + container_of(subdev, struct sun6i_mipi_csi2_video, subdev) > + > +#define sun6i_mipi_csi2_video_dev(video) \ > + container_of(video, struct sun6i_mipi_csi2_dev, video) Isn't it a bit unsafe? The second subdev and video here is not the variable passed in the macro but the field in the structure, so any attempt at using those two macros with anything but a variable named subdev or video will result in a compilation issue? Maxime
Hi, On Mon 14 Dec 20, 12:39, Maxime Ripard wrote: > On Fri, Dec 11, 2020 at 04:57:02PM +0100, Paul Kocialkowski wrote: > > +#define sun6i_mipi_csi2_subdev_video(subdev) \ > > + container_of(subdev, struct sun6i_mipi_csi2_video, subdev) > > + > > +#define sun6i_mipi_csi2_video_dev(video) \ > > + container_of(video, struct sun6i_mipi_csi2_dev, video) > > Isn't it a bit unsafe? > > The second subdev and video here is not the variable passed in the macro > but the field in the structure, so any attempt at using those two macros > with anything but a variable named subdev or video will result in a > compilation issue? Yep you're totally right. Will fix in the next revision! Cheers, Paul -- Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com