From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DBFF7C2D0A3 for ; Wed, 4 Nov 2020 18:57:34 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 54A7D2087C for ; Wed, 4 Nov 2020 18:57:34 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="dqspqsIa"; dkim=temperror (0-bit key) header.d=cerno.tech header.i=@cerno.tech header.b="tr1DljGP"; dkim=temperror (0-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="anMjAzC9" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 54A7D2087C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=cerno.tech Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type:Cc: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=wo+k5oRNPsndnyERl8OIMHqtS0863Jl0PpuT5RFHiiU=; b=dqspqsIa/MOK5IKJvWmi3S6H6 rBu3XaPyCx/crdAaJpDqFcSzmF7wRJ/4wRDjK7LKs0j1KbwFFIEQeJI29buEqfKsALlJvoJJOzhda s/XBOqU5M6D5JAaqdjyyrLwiJMHSgUBocebEPSzBTjADKhbNkZwWQUADA1FymDCYTpRvLxcxekXAk LLbhxzivQAXyjkhgvlHpusG0ox7ne5icCX6llVCL9vQ84iLw5JU84mb4RoCzqXEarhUac+10b4mrp 7u9hCtkoSEdxUAb6mSeMtxnAFt/diJXs9j71ZLWB/Uc6lb/oSOKhwX+rK0pAe+oD1bpNkYMUelI0N jI/Cgiqfw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kaNxv-0002Pn-SY; Wed, 04 Nov 2020 18:57:03 +0000 Received: from new4-smtp.messagingengine.com ([66.111.4.230]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kaNxo-0002Mk-Lx for linux-arm-kernel@lists.infradead.org; Wed, 04 Nov 2020 18:56:57 +0000 Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailnew.nyi.internal (Postfix) with ESMTP id 3E3C2580791; Wed, 4 Nov 2020 13:56:55 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute6.internal (MEProxy); Wed, 04 Nov 2020 13:56:55 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cerno.tech; h= date:from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm1; bh=OVc/96aODvJeJoiBmnUW9Y0oJ7/ fFA1GzRqunAxPing=; b=tr1DljGPY9MWd7f90ap+INggtuV4FGeMUU+fcZ0zoIN vF8JS4egEUyq61VRW2eRlAhIaa3F2Y8OITQlcc3ddrK2Q5ANgoKh+GuQiUxLSlJj OJj/8aL8x1z1qUpppGGLMzL6Dqa5QllobEh+OtCW4cu0kwQW4L5WTEEtOj3pjkmF bWAdBYQiBx3coHKEmdhQvQlfLE4WGkI0/6+Sivx73NRY4sG0BhN5gFnHqp+ylBWX 2QWMbY2TkA/dEVo3Nhia8dGAxo1hxCmGzVOnpvD9W+yB4XFu3TKHA2L68TJM6Q8v jrAxjivO+hQ0ne/T3cHCYYs+Wd80b/8guk6u52gNexQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=OVc/96 aODvJeJoiBmnUW9Y0oJ7/fFA1GzRqunAxPing=; b=anMjAzC9sweKpPacv/4IgD WKVDXQJpL4KrjsMH3GnGsJsQr897zB5gs6ioYdraAtq77CqRPHj25uha13q1kp85 OB64S87MejQlWhbiL+DoZcSfMmG9aMpk39fewdmcRzlOuI3seGEa6UrlIuu4/4ts WKYOiiXGSgk0eshuamrIKtNOE9YcoR5IQqEWY1fc3pkAiOusIRau0/AdQpqmwOWU vbChV6ooLdEmTkh2NzeQGhpZ6X6968VsFgeS6q33bMr1H+S7yP4KGTKOgM1038d0 SOIuVeeGGgbIGOf4yKDGGLg/31NXudsXiTMzf7xBkvBEOjjfv9kK1CNuQ1/NzIZA == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedruddthedguddvtdcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvuffkfhggtggujgesghdtreertddtvdenucfhrhhomhepofgrgihi mhgvucftihhprghrugcuoehmrgigihhmvgestggvrhhnohdrthgvtghhqeenucggtffrrg htthgvrhhnpeelkeeghefhuddtleejgfeljeffheffgfeijefhgfeufefhtdevteegheei heegudenucfkphepledtrdekledrieekrdejieenucevlhhushhtvghrufhiiigvpedtne curfgrrhgrmhepmhgrihhlfhhrohhmpehmrgigihhmvgestggvrhhnohdrthgvtghh X-ME-Proxy: Received: from localhost (lfbn-tou-1-1502-76.w90-89.abo.wanadoo.fr [90.89.68.76]) by mail.messagingengine.com (Postfix) with ESMTPA id 24349328038E; Wed, 4 Nov 2020 13:56:51 -0500 (EST) Date: Wed, 4 Nov 2020 19:56:50 +0100 From: Maxime Ripard To: Paul Kocialkowski Subject: Re: [PATCH 08/14] media: sunxi: Add support for the A31 MIPI CSI-2 controller Message-ID: <20201104185650.ii7dlekjtfar2xpp@gilmour.lan> References: <20201023174546.504028-1-paul.kocialkowski@bootlin.com> <20201023174546.504028-9-paul.kocialkowski@bootlin.com> <20201026165407.rrq6ccsexcsub5bm@gilmour.lan> <20201104113458.GC287014@aptenodytes> MIME-Version: 1.0 In-Reply-To: <20201104113458.GC287014@aptenodytes> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201104_135656_836457_8151E75A X-CRM114-Status: GOOD ( 31.36 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devel@driverdev.osuosl.org, devicetree@vger.kernel.org, Philipp Zabel , Kishon Vijay Abraham I , Thomas Petazzoni , Greg Kroah-Hartman , Helen Koike , linux-kernel@vger.kernel.org, Chen-Yu Tsai , Hans Verkuil , linux-sunxi@googlegroups.com, Rob Herring , Vinod Koul , Yong Deng , Sakari Ailus , Hans Verkuil , Mauro Carvalho Chehab , kevin.lhopital@hotmail.com, linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org Content-Type: multipart/mixed; boundary="===============1631453338628574513==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============1631453338628574513== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ivpywtswxref2r4j" Content-Disposition: inline --ivpywtswxref2r4j Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 04, 2020 at 12:34:58PM +0100, Paul Kocialkowski wrote: > > > + regmap_write(regmap, SUN6I_MIPI_CSI2_CFG_REG, > > > + SUN6I_MIPI_CSI2_CFG_CHANNEL_MODE(1) | > > > + SUN6I_MIPI_CSI2_CFG_LANE_COUNT(lanes_count)); > >=20 > > It's not really clear what the channel is here? The number of virtual > > channels? Something else? >=20 > That's somewhat described in the controller documentation. Channels refer= s to > physical channels of the controller, which can be used to redirect data > matching either a specific data type, a specific virtual channel, or both. > There's a somewhat similar concept of channels in the CSI controller too. >=20 > We're currently only using one... >=20 > > > + regmap_write(regmap, SUN6I_MIPI_CSI2_VCDT_RX_REG, > > > + SUN6I_MIPI_CSI2_VCDT_RX_CH_VC(3, 3) | > > > + SUN6I_MIPI_CSI2_VCDT_RX_CH_VC(2, 2) | > > > + SUN6I_MIPI_CSI2_VCDT_RX_CH_VC(1, 1) | > > > + SUN6I_MIPI_CSI2_VCDT_RX_CH_VC(0, 0) | > > > + SUN6I_MIPI_CSI2_VCDT_RX_CH_DT(0, data_type)); >=20 > ... but it's safer to configure them all to virtual channel numbers so we= don't > end up with multiple channels matching virtual channel 0. >=20 > I'll add a comment about that. Maybe we should have pads for all of them then, even if we don't support changing anything? > > > +static const struct v4l2_subdev_pad_ops sun6i_mipi_csi2_subdev_pad_o= ps =3D { > > > + .enum_mbus_code =3D sun6i_mipi_csi2_enum_mbus_code, > > > + .get_fmt =3D sun6i_mipi_csi2_get_fmt, > > > + .set_fmt =3D sun6i_mipi_csi2_set_fmt, > > > + .enum_frame_size =3D sun6i_mipi_csi2_enum_frame_size, > > > + .enum_frame_interval =3D sun6i_mipi_csi2_enum_frame_interval, > > > +}; > > > + > > > +/* Subdev */ > > > + > > > +static const struct v4l2_subdev_ops sun6i_mipi_csi2_subdev_ops =3D { > > > + .core =3D &sun6i_mipi_csi2_subdev_core_ops, > > > + .video =3D &sun6i_mipi_csi2_subdev_video_ops, > > > + .pad =3D &sun6i_mipi_csi2_subdev_pad_ops, > > > +}; > > > + > > > +/* Notifier */ > > > + > > > +static int sun6i_mipi_csi2_notifier_bound(struct v4l2_async_notifier= *notifier, > > > + struct v4l2_subdev *remote_subdev, > > > + struct v4l2_async_subdev *remote_subdev_async) > > > +{ > > > + struct v4l2_subdev *subdev =3D notifier->sd; > > > + struct sun6i_mipi_csi2_video *video =3D > > > + sun6i_mipi_csi2_subdev_video(subdev); > > > + struct sun6i_mipi_csi2_dev *cdev =3D sun6i_mipi_csi2_video_dev(vide= o); > > > + int source_pad; > > > + int ret; > > > + > > > + source_pad =3D media_entity_get_fwnode_pad(&remote_subdev->entity, > > > + remote_subdev->fwnode, > > > + MEDIA_PAD_FL_SOURCE); > > > + if (source_pad < 0) > > > + return source_pad; > > > + > > > + ret =3D media_create_pad_link(&remote_subdev->entity, source_pad, > > > + &subdev->entity, 0, > > > + MEDIA_LNK_FL_ENABLED | > > > + MEDIA_LNK_FL_IMMUTABLE); > > > + if (ret) { > > > + dev_err(cdev->dev, "failed to create %s:%u -> %s:%u link\n", > > > + remote_subdev->entity.name, source_pad, > > > + subdev->entity.name, 0); > > > + return ret; > > > + } > > > + > > > + video->remote_subdev =3D remote_subdev; > > > + video->remote_pad_index =3D source_pad; > > > + > > > + return 0; > > > +} > > > + > > > +static const struct v4l2_async_notifier_operations sun6i_mipi_csi2_n= otifier_ops =3D { > > > + .bound =3D sun6i_mipi_csi2_notifier_bound, > > > +}; > > > + > > > +/* Media Entity */ > > > + > > > +static int sun6i_mipi_csi2_link_validate(struct media_link *link) > > > +{ > > > + struct v4l2_subdev *subdev =3D > > > + container_of(link->sink->entity, struct v4l2_subdev, entity); > > > + struct sun6i_mipi_csi2_video *video =3D > > > + sun6i_mipi_csi2_subdev_video(subdev); > > > + struct v4l2_subdev *remote_subdev; > > > + struct v4l2_subdev_format format =3D { 0 }; > > > + int ret; > > > + > > > + if (!is_media_entity_v4l2_subdev(link->source->entity)) > > > + return -EINVAL; > > > + > > > + remote_subdev =3D media_entity_to_v4l2_subdev(link->source->entity); > > > + > > > + format.which =3D V4L2_SUBDEV_FORMAT_ACTIVE; > > > + format.pad =3D link->source->index; > > > + > > > + ret =3D v4l2_subdev_call(remote_subdev, pad, get_fmt, NULL, &format= ); > > > + if (ret) > > > + return ret; > > > + > > > + video->mbus_code =3D format.format.code; > > > + > > > + return 0; > > > +} > >=20 > > I'm not really sure what you're trying to validate here? >=20 > The whole purpose is to retreive video->mbus_code from the subdev, like i= t's > done in the sun6i-csi driver. Maybe there is a more appropriate op to do = it? I'm not sure why you need to do that in the link_validate though? You just need to init the pad format, and then you'll have a get_fmt/set_fmt for your pads. > > > + cdev->regmap =3D devm_regmap_init_mmio_clk(&pdev->dev, "bus", io_ba= se, > > > + &sun6i_mipi_csi2_regmap_config); > > > + if (IS_ERR(cdev->regmap)) { > > > + dev_err(&pdev->dev, "failed to init register map\n"); > > > + return PTR_ERR(cdev->regmap); > > > + } > >=20 > > Yeah, so that won't work. regmap expects to have access to those > > registers when you enable that clock, but that won't happen since the > > reset line can be disabled. You would be better off using runtime_pm > > here. >=20 > I don't understand what you mean here or what the problem could be. > Here we're just initializing regmap and while this is done before the > registers are available for I/O, I don't see why it would cause any > issue at this point. The regmap here is supposed to take care of the resources, except it only does it for some of the resources here, which kind of breaks the expectations. And it doesn't allow you to have the reset / clock sequence properly done. > The exact same thing is done in the CSI driver. That's not an argument though, is it? :) Maxime --ivpywtswxref2r4j Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCX6L5cQAKCRDj7w1vZxhR xapUAPkBCQbEYpxpQRc2dQrlYckdrRucLdZjrw4DyHXeRpWXKwD8D9vPu/MJrl2p 3cE184xWD5PpdHRseodrz4F6IAC7tA4= =egrI -----END PGP SIGNATURE----- --ivpywtswxref2r4j-- --===============1631453338628574513== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============1631453338628574513==--