Message ID | 20240619153559.1647957-2-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
---|---|
State | Superseded |
Headers |
Received: from ny.mirrors.kernel.org ([147.75.199.223]) by linuxtv.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from <linux-media+bounces-13738-patchwork=linuxtv.org@vger.kernel.org>) id 1sJxMt-0005W3-0D for patchwork@linuxtv.org; Wed, 19 Jun 2024 15:37:03 +0000 Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 85E0F1C21AE6 for <patchwork@linuxtv.org>; Wed, 19 Jun 2024 15:37:01 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 77355152798; Wed, 19 Jun 2024 15:36:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=ragnatech.se header.i=@ragnatech.se header.b="Zqx/Jikj"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="ro2EA02L" X-Original-To: linux-media@vger.kernel.org Received: from fhigh7-smtp.messagingengine.com (fhigh7-smtp.messagingengine.com [103.168.172.158]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 899C221345; Wed, 19 Jun 2024 15:36:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.158 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718811395; cv=none; b=R0y4rs9zwhrYJ0it5OSKwhiDrub5jtKoikiRsts0Pe+kkj1jMoFKTLMPfgdWFN8Gc2bS7+MEX7PP+fwiykki3c8ljhmqyeiJ18r3doSFCk1A/u4nfE3IGKBJqqzlQlb56N64wFgkE1qgCv5+tP5HrbcYfwBwWvji30TY5DO3Elo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718811395; c=relaxed/simple; bh=KJFj5e+o7VpD9NNr3CchM8mAH05uZxhtzX7TKcYvdnY=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=l75xiomvkesuhTsKSYOBeOM7T3eOyzRXjWIXNHXmYwiAyiizJKBD3kwcHBlJdenyO81fwWWEQCk4KbDbvcJtcHVi2Iu9FUQ5ffQniQo1GfW42s2PzkZLWJOpN0Dyypopcbj5Lm/4SwWFSsXAAQ0nuaU0SIvY1A6XPEmdhsZBj7g= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=ragnatech.se; spf=pass smtp.mailfrom=ragnatech.se; dkim=pass (2048-bit key) header.d=ragnatech.se header.i=@ragnatech.se header.b=Zqx/Jikj; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=ro2EA02L; arc=none smtp.client-ip=103.168.172.158 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=ragnatech.se Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ragnatech.se Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailfhigh.nyi.internal (Postfix) with ESMTP id A8A1F11401E0; Wed, 19 Jun 2024 11:36:31 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 19 Jun 2024 11:36:31 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ragnatech.se; h= cc:cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1718811391; x=1718897791; bh=Nu/e/5QrvdFIKUx7T4lRBSEMjBYYsMhdUkhwPu9DOTg=; b= Zqx/JikjMn4FiVZgrNhn9kHTpxtvg4YNvJjphCadwNg7hc4SVArtuxX6lN5gSR3H g7jzktao1VFBg+SeStnZnUEPU4FIuHiiA3J5f1pmuQtdqRWg+W7fE1IbkJLdLDGN Xif5n67JWrlmM5/mIwkGnlaxmqpIToVZF8ipcnh8PRq/VE5HT+5YZ4gJzSyDBHin sHKqtOYM1Ruf9f170e2oC0osw5wy2yE9wWnrOM1DnzkWh5GeXbXSUJNuaYGM3XAO uX/GDHdUN9IQesi/MzAiqmqznfqso3lYCF0uWIve+NQmeAj0oI9oQbmpTE3hDrLU cAXSQsIcclf9gBa7p5Aw7g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1718811391; x= 1718897791; bh=Nu/e/5QrvdFIKUx7T4lRBSEMjBYYsMhdUkhwPu9DOTg=; b=r o2EA02L4qywVne8D9OEgyf4kZ4hIVZPc2h1FZggW1ksDtVfE8HlxpjP8jXRmjSIb 3zWs/ff9qzIX+VgWbJpVF7sh/LWzWi5zFjMRLrNaiiYtHYj53CDH/nEkdEZjMhNy TrI4iq8SHkGhMQ1YhZPjTYF8EPq4SliYwwL0oZlqZM4Csx9eaTkV0b9iNy8AXhdj TUpMtR/Euk19j9GIy4bOyG4vHHpABh0Vdp0ryJYvS+XhikMl5DhGvVmQB1CQ1S+K P+OrnyVt77EdDAj6o6uJYij2Nco7ndeo4TUrFsS2AhOMdT1cKhtrWkJ8R4YM1s2L jNPaiGHO6VSa3mz2ftyWA== X-ME-Sender: <xms:__pyZmZxcY5N2ki3pO995wpJQ8SxIRWlOn_8js5aTlHax6vAnyrFjg> <xme:__pyZpYILdhHmnAwoBfWall6UCzrtBE01wDOluBdLtbmGelsagWyyZgty9wjrPCta -3k8SwOEvrxykppDjI> X-ME-Received: <xmr:__pyZg_Ei3as35Yws7XmM-x129zPJijkcwslbRUz4WVCfi80_EG_zmwlOFwsAGkI_w37ABQNYhlLP1FjSDwTBoVSAw> X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrfeeftddgkeekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvvefufffkofgjfhggtgfgsehtkeertdertdejnecuhfhrohhmpefpihhk lhgrshcuufpnuggvrhhluhhnugcuoehnihhklhgrshdrshhouggvrhhluhhnugdorhgvnh gvshgrshesrhgrghhnrghtvggthhdrshgvqeenucggtffrrghtthgvrhhnpeehiefgueev uedtfefhheegkeevtdelueeukeevfeduhefhhfejfffggeffleefgeenucevlhhushhtvg hrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehnihhklhgrshdrshhouggv rhhluhhnugesrhgrghhnrghtvggthhdrshgv X-ME-Proxy: <xmx:__pyZopgXxACe-nOzEuVJS8M822E7Bz3Jv5XdbjDS84t3wLVsEXV-A> <xmx:__pyZhrb05DO83Rxzo1TSmJGYj_T7ztGvClbuAzjPvnFBwZCwQ58Nw> <xmx:__pyZmRC7Al5KMtGElGJo30ak6K3UaFMmxWqhkJBqITdULVqO6Mjxg> <xmx:__pyZhquC9wsqYaTBSDEymobFde_inGaDtOZ613HfR0XL05IqZdaFw> <xmx:__pyZuhnrwW3QVo7DB0cGSxN3pytrTv2-GIJRgfZqM6MJsO7VSUDZYTm> Feedback-ID: i80c9496c:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 19 Jun 2024 11:36:31 -0400 (EDT) From: =?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund+renesas@ragnatech.se> To: Mauro Carvalho Chehab <mchehab@kernel.org>, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, Geert Uytterhoeven <geert+renesas@glider.be>, linux-media@vger.kernel.org, devicetree@vger.kernel.org Cc: linux-renesas-soc@vger.kernel.org, =?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund+renesas@ragnatech.se> Subject: [PATCH v3 1/2] dt-bindings: media: renesas,vin: Add binding for V4M Date: Wed, 19 Jun 2024 17:35:58 +0200 Message-ID: <20240619153559.1647957-2-niklas.soderlund+renesas@ragnatech.se> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240619153559.1647957-1-niklas.soderlund+renesas@ragnatech.se> References: <20240619153559.1647957-1-niklas.soderlund+renesas@ragnatech.se> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: <linux-media.vger.kernel.org> List-Subscribe: <mailto:linux-media+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-media+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-LSpam-Score: -2.6 (--) X-LSpam-Report: No, score=-2.6 required=5.0 tests=ARC_SIGNED=0.001,ARC_VALID=-0.1,BAYES_00=-1.9,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,DMARC_PASS=-0.001,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1,SPF_HELO_NONE=0.001,SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no |
Series |
rcar-vin: Add support for R-Car V4M
|
|
Commit Message
Niklas Söderlund
June 19, 2024, 3:35 p.m. UTC
Document support for the VIN module in the Renesas V4M (r8a779h0) SoC. Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> --- Documentation/devicetree/bindings/media/renesas,vin.yaml | 1 + 1 file changed, 1 insertion(+)
Comments
On Wed, Jun 19, 2024 at 05:35:58PM +0200, Niklas Söderlund wrote: > Document support for the VIN module in the Renesas V4M (r8a779h0) SoC. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Didn't we just have a conversation about this, yet nothing has changed? NAK. Either you need a fallback or to explain why a fallback is not suitable _in this patch_. Thanks, Conor. > --- > Documentation/devicetree/bindings/media/renesas,vin.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml > index 5539d0f8e74d..168cb02f8abe 100644 > --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml > +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml > @@ -54,6 +54,7 @@ properties: > - renesas,vin-r8a77995 # R-Car D3 > - renesas,vin-r8a779a0 # R-Car V3U > - renesas,vin-r8a779g0 # R-Car V4H > + - renesas,vin-r8a779h0 # R-Car V4M > > reg: > maxItems: 1 > -- > 2.45.2 >
Hi Conor, On 2024-06-19 18:33:37 +0100, Conor Dooley wrote: > On Wed, Jun 19, 2024 at 05:35:58PM +0200, Niklas Söderlund wrote: > > Document support for the VIN module in the Renesas V4M (r8a779h0) SoC. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Didn't we just have a conversation about this, yet nothing has changed? > NAK. Either you need a fallback or to explain why a fallback is not > suitable _in this patch_. Sorry, I'm confused from the conclusion of our conversation in v2. I did add an explanation to why not fallback is used, but I added it to patch 2/2 which adds the compatible to the driver. It was my understanding that a SoC specific compatible was needed in either case so, at lest to me, made more sens to explain why in the driver patch the reason go into detail about the register differences between the two. Sorry if I misunderstood. I can add the same explanation to both patches, would this help explain why only a SoC specific value is added? The datasheet for the two SoCs have small nuances around the Pre-Clip registers ELPrC and EPPrC in three use-cases, interlaced images, embedded data and RAW8 input. On V4H the values written to the registers are based on odd numbers while on V4M they are even numbers, based on the input image size. No board that uses these SoCs which also have the external peripherals to test these nuances exists. Most likely this is an issue in the datasheet, but to make this easy to address in the future do not add a common Gen4 fallback compatible. Instead uses SoC specific compatibles for both SoCs. This is what was done for Gen3 SoCs, which also had similar nuances in the register documentation. > > Thanks, > Conor. > > > --- > > Documentation/devicetree/bindings/media/renesas,vin.yaml | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml > > index 5539d0f8e74d..168cb02f8abe 100644 > > --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml > > +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml > > @@ -54,6 +54,7 @@ properties: > > - renesas,vin-r8a77995 # R-Car D3 > > - renesas,vin-r8a779a0 # R-Car V3U > > - renesas,vin-r8a779g0 # R-Car V4H > > + - renesas,vin-r8a779h0 # R-Car V4M > > > > reg: > > maxItems: 1 > > -- > > 2.45.2 > >
Hello again. On 2024-06-19 20:56:11 +0200, Niklas Söderlund wrote: > Hi Conor, > > On 2024-06-19 18:33:37 +0100, Conor Dooley wrote: > > On Wed, Jun 19, 2024 at 05:35:58PM +0200, Niklas Söderlund wrote: > > > Document support for the VIN module in the Renesas V4M (r8a779h0) SoC. > > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > Didn't we just have a conversation about this, yet nothing has changed? > > NAK. Either you need a fallback or to explain why a fallback is not > > suitable _in this patch_. > > Sorry, I'm confused from the conclusion of our conversation in v2. I did > add an explanation to why not fallback is used, but I added it to patch > 2/2 which adds the compatible to the driver. > > It was my understanding that a SoC specific compatible was needed in > either case so, at lest to me, made more sens to explain why in the > driver patch the reason go into detail about the register differences > between the two. Sorry if I misunderstood. I can add the same > explanation to both patches, would this help explain why only a SoC > specific value is added? > > The datasheet for the two SoCs have small nuances around the Pre-Clip > registers ELPrC and EPPrC in three use-cases, interlaced images, > embedded data and RAW8 input. On V4H the values written to the registers > are based on odd numbers while on V4M they are even numbers, based on > the input image size. > > No board that uses these SoCs which also have the external peripherals > to test these nuances exists. Most likely this is an issue in the > datasheet, but to make this easy to address in the future do not add a > common Gen4 fallback compatible. Instead uses SoC specific compatibles > for both SoCs. This is what was done for Gen3 SoCs, which also had > similar nuances in the register documentation. After have read thru v1 and v2 comments a few more times I think I might have spotted what I got wrong. If so I apologies for wasting your time reviewing this. I'm really trying to understand what I got wrong and address the review feedback. Is what you are asking for with a fallback something like this? --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml @@ -53,7 +53,11 @@ properties: - renesas,vin-r8a77990 # R-Car E3 - renesas,vin-r8a77995 # R-Car D3 - renesas,vin-r8a779a0 # R-Car V3U + - items: + - enum: - renesas,vin-r8a779g0 # R-Car V4H + - renesas,vin-r8a779h0 # R-Car V4M + - const: renesas,rcar-gen4-vin # Generic R-Car Gen4 If so I can see that working as I could still fix any issues that come from differences between V4H and V4M if needed. If so do you think it best to add this in two different patches? One to add the renesas,rcar-gen4-vin fallback (which will also need DTS updates to fix warnings from exciting users of V4H not listing the gen4 fallback) and one to add V4M? Apologies again for the confusion. > > > > > Thanks, > > Conor. > > > > > --- > > > Documentation/devicetree/bindings/media/renesas,vin.yaml | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml > > > index 5539d0f8e74d..168cb02f8abe 100644 > > > --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml > > > +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml > > > @@ -54,6 +54,7 @@ properties: > > > - renesas,vin-r8a77995 # R-Car D3 > > > - renesas,vin-r8a779a0 # R-Car V3U > > > - renesas,vin-r8a779g0 # R-Car V4H > > > + - renesas,vin-r8a779h0 # R-Car V4M > > > > > > reg: > > > maxItems: 1 > > > -- > > > 2.45.2 > > > > > > > -- > Kind Regards, > Niklas Söderlund
On Wed, Jun 19, 2024 at 10:43:21PM +0200, Niklas Söderlund wrote: > Hello again. > > On 2024-06-19 20:56:11 +0200, Niklas Söderlund wrote: > > Hi Conor, > > > > On 2024-06-19 18:33:37 +0100, Conor Dooley wrote: > > > On Wed, Jun 19, 2024 at 05:35:58PM +0200, Niklas Söderlund wrote: > > > > Document support for the VIN module in the Renesas V4M (r8a779h0) SoC. > > > > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > > Didn't we just have a conversation about this, yet nothing has changed? > > > NAK. Either you need a fallback or to explain why a fallback is not > > > suitable _in this patch_. > > > > Sorry, I'm confused from the conclusion of our conversation in v2. I did > > add an explanation to why not fallback is used, but I added it to patch > > 2/2 which adds the compatible to the driver. If you're unsure at all just ask, better that than send a new version. > > > > It was my understanding that a SoC specific compatible was needed in > > either case so, at lest to me, made more sens to explain why in the > > driver patch the reason go into detail about the register differences > > between the two. Sorry if I misunderstood. I can add the same > > explanation to both patches, would this help explain why only a SoC > > specific value is added? > > > > The datasheet for the two SoCs have small nuances around the Pre-Clip > > registers ELPrC and EPPrC in three use-cases, interlaced images, > > embedded data and RAW8 input. On V4H the values written to the registers > > are based on odd numbers while on V4M they are even numbers, based on > > the input image size. > > > > No board that uses these SoCs which also have the external peripherals > > to test these nuances exists. Most likely this is an issue in the > > datasheet, but to make this easy to address in the future do not add a > > common Gen4 fallback compatible. Instead uses SoC specific compatibles > > for both SoCs. This is what was done for Gen3 SoCs, which also had > > similar nuances in the register documentation. > > After have read thru v1 and v2 comments a few more times I think I might > have spotted what I got wrong. If so I apologies for wasting your time > reviewing this. I'm really trying to understand what I got wrong and > address the review feedback. > > Is what you are asking for with a fallback something like this? > > --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml > +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml > @@ -53,7 +53,11 @@ properties: > - renesas,vin-r8a77990 # R-Car E3 > - renesas,vin-r8a77995 # R-Car D3 > - renesas,vin-r8a779a0 # R-Car V3U > + - items: > + - enum: > - renesas,vin-r8a779g0 # R-Car V4H > + - renesas,vin-r8a779h0 # R-Car V4M > + - const: renesas,rcar-gen4-vin # Generic R-Car Gen4 > > If so I can see that working as I could still fix any issues that come > from differences between V4H and V4M if needed. If so do you think it > best to add this in two different patches? One to add the > renesas,rcar-gen4-vin fallback (which will also need DTS updates to fix > warnings from exciting users of V4H not listing the gen4 fallback) and > one to add V4M? I would just do: diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml index 5539d0f8e74d..22bbad42fc03 100644 --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml @@ -54,6 +54,9 @@ properties: - renesas,vin-r8a77995 # R-Car D3 - renesas,vin-r8a779a0 # R-Car V3U - renesas,vin-r8a779g0 # R-Car V4H + - items: + - const: renesas,vin-r8a779h0 # R-Car V4L2 + - const: renesas,vin-r8a779g0 # R-Car V4H reg: maxItems: 1 Which requires no driver or dts changes. That could become: - items: - enum: - renesas,vin-r8a779h0 # R-Car V4L2 - renesas,vin-r8a779i0 # R-Car R4P17 - const: renesas,vin-r8a779g0 # R-Car V4H if there's another compatible device in the future. > Apologies again for the confusion. dw about it
On 2024-06-20 17:27:00 +0100, Conor Dooley wrote: > > + - items: > > + - enum: > > - renesas,vin-r8a779g0 # R-Car V4H > > + - renesas,vin-r8a779h0 # R-Car V4M > > + - const: renesas,rcar-gen4-vin # Generic R-Car Gen4 > > > > If so I can see that working as I could still fix any issues that come > > from differences between V4H and V4M if needed. If so do you think it > > best to add this in two different patches? One to add the > > renesas,rcar-gen4-vin fallback (which will also need DTS updates to fix > > warnings from exciting users of V4H not listing the gen4 fallback) and > > one to add V4M? > > > I would just do: > diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml > index 5539d0f8e74d..22bbad42fc03 100644 > --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml > +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml > @@ -54,6 +54,9 @@ properties: > - renesas,vin-r8a77995 # R-Car D3 > - renesas,vin-r8a779a0 # R-Car V3U > - renesas,vin-r8a779g0 # R-Car V4H > + - items: > + - const: renesas,vin-r8a779h0 # R-Car V4L2 > + - const: renesas,vin-r8a779g0 # R-Car V4H @Geert: What do you think about this? This would be a first use-case for compatibles crossing SoC DTS files that I know of. I'm a bit uneasy going down this road. Would this not also effect the existing users of renesas,vin-r8a779g0 which would now need something similar to what you propose below with a list of SoC compatibles and a fallback. > > reg: > maxItems: 1 > > Which requires no driver or dts changes. That could become: > - items: > - enum: > - renesas,vin-r8a779h0 # R-Car V4L2 > - renesas,vin-r8a779i0 # R-Car R4P17 > - const: renesas,vin-r8a779g0 # R-Car V4H FWIW, on Gen2 where fallback es where useful compared to Gen3 we did this with "renesas,rcar-gen2-vin". > > if there's another compatible device in the future. > > > Apologies again for the confusion. > > dw about it
Hi Niklas, On Thu, Jun 20, 2024 at 7:22 PM Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> wrote: > On 2024-06-20 17:27:00 +0100, Conor Dooley wrote: > > > + - items: > > > + - enum: > > > - renesas,vin-r8a779g0 # R-Car V4H > > > + - renesas,vin-r8a779h0 # R-Car V4M > > > + - const: renesas,rcar-gen4-vin # Generic R-Car Gen4 > > > > > > If so I can see that working as I could still fix any issues that come > > > from differences between V4H and V4M if needed. If so do you think it > > > best to add this in two different patches? One to add the > > > renesas,rcar-gen4-vin fallback (which will also need DTS updates to fix > > > warnings from exciting users of V4H not listing the gen4 fallback) and > > > one to add V4M? > > > > > > I would just do: > > diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml > > index 5539d0f8e74d..22bbad42fc03 100644 > > --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml > > +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml > > @@ -54,6 +54,9 @@ properties: > > - renesas,vin-r8a77995 # R-Car D3 > > - renesas,vin-r8a779a0 # R-Car V3U > > - renesas,vin-r8a779g0 # R-Car V4H > > + - items: > > + - const: renesas,vin-r8a779h0 # R-Car V4L2 > > + - const: renesas,vin-r8a779g0 # R-Car V4H > > @Geert: What do you think about this? This would be a first use-case for > compatibles crossing SoC DTS files that I know of. I'm a bit uneasy > going down this road. Me too ;-) > Would this not also effect the existing users of renesas,vin-r8a779g0 > which would now need something similar to what you propose below with a > list of SoC compatibles and a fallback. > > > > > reg: > > maxItems: 1 > > > > Which requires no driver or dts changes. That could become: > > - items: > > - enum: > > - renesas,vin-r8a779h0 # R-Car V4L2 > > - renesas,vin-r8a779i0 # R-Car R4P17 > > - const: renesas,vin-r8a779g0 # R-Car V4H > > FWIW, on Gen2 where fallback es where useful compared to Gen3 we did > this with "renesas,rcar-gen2-vin". We do know there are differences (albeit probably small) among the R-Car Gen4 VIN implementations, so I am reluctant to add a family-specific compatible value. Typically we only use a family-specific compatible value if the IP cores are known (or better, assumed ;-) to be identical. And sometimes our assumptions turn out to be wrong... See slides 25-33 (last two for the numbers) of my talk at ER2019 https://embedded-recipes.org/2019/talks/herd-your-socs-become-a-matchmaker/ Gr{oetje,eeting}s, Geert
Hi Conor, On 2024-06-21 09:21:24 +0200, Geert Uytterhoeven wrote: > Hi Niklas, > > On Thu, Jun 20, 2024 at 7:22 PM Niklas Söderlund > <niklas.soderlund+renesas@ragnatech.se> wrote: > > On 2024-06-20 17:27:00 +0100, Conor Dooley wrote: > > > > + - items: > > > > + - enum: > > > > - renesas,vin-r8a779g0 # R-Car V4H > > > > + - renesas,vin-r8a779h0 # R-Car V4M > > > > + - const: renesas,rcar-gen4-vin # Generic R-Car Gen4 > > > > > > > > If so I can see that working as I could still fix any issues that come > > > > from differences between V4H and V4M if needed. If so do you think it > > > > best to add this in two different patches? One to add the > > > > renesas,rcar-gen4-vin fallback (which will also need DTS updates to fix > > > > warnings from exciting users of V4H not listing the gen4 fallback) and > > > > one to add V4M? > > > > > > > > > I would just do: > > > diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml > > > index 5539d0f8e74d..22bbad42fc03 100644 > > > --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml > > > +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml > > > @@ -54,6 +54,9 @@ properties: > > > - renesas,vin-r8a77995 # R-Car D3 > > > - renesas,vin-r8a779a0 # R-Car V3U > > > - renesas,vin-r8a779g0 # R-Car V4H > > > + - items: > > > + - const: renesas,vin-r8a779h0 # R-Car V4L2 > > > + - const: renesas,vin-r8a779g0 # R-Car V4H > > > > @Geert: What do you think about this? This would be a first use-case for > > compatibles crossing SoC DTS files that I know of. I'm a bit uneasy > > going down this road. > > Me too ;-) > > > Would this not also effect the existing users of renesas,vin-r8a779g0 > > which would now need something similar to what you propose below with a > > list of SoC compatibles and a fallback. > > > > > > > > reg: > > > maxItems: 1 > > > > > > Which requires no driver or dts changes. That could become: > > > - items: > > > - enum: > > > - renesas,vin-r8a779h0 # R-Car V4L2 > > > - renesas,vin-r8a779i0 # R-Car R4P17 > > > - const: renesas,vin-r8a779g0 # R-Car V4H > > > > FWIW, on Gen2 where fallback es where useful compared to Gen3 we did > > this with "renesas,rcar-gen2-vin". > > We do know there are differences (albeit probably small) among the R-Car > Gen4 VIN implementations, so I am reluctant to add a family-specific > compatible value. Typically we only use a family-specific compatible > value if the IP cores are known (or better, assumed ;-) to be identical. > > And sometimes our assumptions turn out to be wrong... > See slides 25-33 (last two for the numbers) of my talk at ER2019 > https://embedded-recipes.org/2019/talks/herd-your-socs-become-a-matchmaker/ Do Geert's slides help to explain the R-Car perspective on why a family-specific fallback compatible might not be desirable, and why the SoC specific one is proposed?
On Mon, Jun 24, 2024 at 11:20:29AM +0200, Niklas Söderlund wrote: > Hi Conor, > > On 2024-06-21 09:21:24 +0200, Geert Uytterhoeven wrote: > > Hi Niklas, > > > > On Thu, Jun 20, 2024 at 7:22 PM Niklas Söderlund > > <niklas.soderlund+renesas@ragnatech.se> wrote: > > > On 2024-06-20 17:27:00 +0100, Conor Dooley wrote: > > > > > + - items: > > > > > + - enum: > > > > > - renesas,vin-r8a779g0 # R-Car V4H > > > > > + - renesas,vin-r8a779h0 # R-Car V4M > > > > > + - const: renesas,rcar-gen4-vin # Generic R-Car Gen4 > > > > > > > > > > If so I can see that working as I could still fix any issues that come > > > > > from differences between V4H and V4M if needed. If so do you think it > > > > > best to add this in two different patches? One to add the > > > > > renesas,rcar-gen4-vin fallback (which will also need DTS updates to fix > > > > > warnings from exciting users of V4H not listing the gen4 fallback) and > > > > > one to add V4M? > > > > > > > > > > > > I would just do: > > > > diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml > > > > index 5539d0f8e74d..22bbad42fc03 100644 > > > > --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml > > > > +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml > > > > @@ -54,6 +54,9 @@ properties: > > > > - renesas,vin-r8a77995 # R-Car D3 > > > > - renesas,vin-r8a779a0 # R-Car V3U > > > > - renesas,vin-r8a779g0 # R-Car V4H > > > > + - items: > > > > + - const: renesas,vin-r8a779h0 # R-Car V4L2 > > > > + - const: renesas,vin-r8a779g0 # R-Car V4H > > > > > > @Geert: What do you think about this? This would be a first use-case for > > > compatibles crossing SoC DTS files that I know of. I'm a bit uneasy > > > going down this road. > > > > Me too ;-) > > > > > Would this not also effect the existing users of renesas,vin-r8a779g0 > > > which would now need something similar to what you propose below with a > > > list of SoC compatibles and a fallback. > > > > > > > > > > > reg: > > > > maxItems: 1 > > > > > > > > Which requires no driver or dts changes. That could become: > > > > - items: > > > > - enum: > > > > - renesas,vin-r8a779h0 # R-Car V4L2 > > > > - renesas,vin-r8a779i0 # R-Car R4P17 > > > > - const: renesas,vin-r8a779g0 # R-Car V4H > > > > > > FWIW, on Gen2 where fallback es where useful compared to Gen3 we did > > > this with "renesas,rcar-gen2-vin". > > > > We do know there are differences (albeit probably small) among the R-Car > > Gen4 VIN implementations, so I am reluctant to add a family-specific > > compatible value. Typically we only use a family-specific compatible > > value if the IP cores are known (or better, assumed ;-) to be identical. > > > > And sometimes our assumptions turn out to be wrong... > > See slides 25-33 (last two for the numbers) of my talk at ER2019 > > https://embedded-recipes.org/2019/talks/herd-your-socs-become-a-matchmaker/ > > Do Geert's slides help to explain the R-Car perspective on why a > family-specific fallback compatible might not be desirable, and why the > SoC specific one is proposed? IIRC, it was you that wanted to use a "family-specific" fallback, I don't understand what you want from me. If you look back at even the context in this email, you can see you suggesting one and my counter point. Confused, Conor.
On 2024-06-24 11:36:40 +0100, Conor Dooley wrote: > On Mon, Jun 24, 2024 at 11:20:29AM +0200, Niklas Söderlund wrote: > > Hi Conor, > > > > On 2024-06-21 09:21:24 +0200, Geert Uytterhoeven wrote: > > > Hi Niklas, > > > > > > On Thu, Jun 20, 2024 at 7:22 PM Niklas Söderlund > > > <niklas.soderlund+renesas@ragnatech.se> wrote: > > > > On 2024-06-20 17:27:00 +0100, Conor Dooley wrote: > > > > > > + - items: > > > > > > + - enum: > > > > > > - renesas,vin-r8a779g0 # R-Car V4H > > > > > > + - renesas,vin-r8a779h0 # R-Car V4M > > > > > > + - const: renesas,rcar-gen4-vin # Generic R-Car Gen4 > > > > > > > > > > > > If so I can see that working as I could still fix any issues that come > > > > > > from differences between V4H and V4M if needed. If so do you think it > > > > > > best to add this in two different patches? One to add the > > > > > > renesas,rcar-gen4-vin fallback (which will also need DTS updates to fix > > > > > > warnings from exciting users of V4H not listing the gen4 fallback) and > > > > > > one to add V4M? > > > > > > > > > > > > > > > I would just do: > > > > > diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml > > > > > index 5539d0f8e74d..22bbad42fc03 100644 > > > > > --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml > > > > > +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml > > > > > @@ -54,6 +54,9 @@ properties: > > > > > - renesas,vin-r8a77995 # R-Car D3 > > > > > - renesas,vin-r8a779a0 # R-Car V3U > > > > > - renesas,vin-r8a779g0 # R-Car V4H > > > > > + - items: > > > > > + - const: renesas,vin-r8a779h0 # R-Car V4L2 > > > > > + - const: renesas,vin-r8a779g0 # R-Car V4H > > > > > > > > @Geert: What do you think about this? This would be a first use-case for > > > > compatibles crossing SoC DTS files that I know of. I'm a bit uneasy > > > > going down this road. > > > > > > Me too ;-) > > > > > > > Would this not also effect the existing users of renesas,vin-r8a779g0 > > > > which would now need something similar to what you propose below with a > > > > list of SoC compatibles and a fallback. > > > > > > > > > > > > > > reg: > > > > > maxItems: 1 > > > > > > > > > > Which requires no driver or dts changes. That could become: > > > > > - items: > > > > > - enum: > > > > > - renesas,vin-r8a779h0 # R-Car V4L2 > > > > > - renesas,vin-r8a779i0 # R-Car R4P17 > > > > > - const: renesas,vin-r8a779g0 # R-Car V4H > > > > > > > > FWIW, on Gen2 where fallback es where useful compared to Gen3 we did > > > > this with "renesas,rcar-gen2-vin". > > > > > > We do know there are differences (albeit probably small) among the R-Car > > > Gen4 VIN implementations, so I am reluctant to add a family-specific > > > compatible value. Typically we only use a family-specific compatible > > > value if the IP cores are known (or better, assumed ;-) to be identical. > > > > > > And sometimes our assumptions turn out to be wrong... > > > See slides 25-33 (last two for the numbers) of my talk at ER2019 > > > https://embedded-recipes.org/2019/talks/herd-your-socs-become-a-matchmaker/ > > > > Do Geert's slides help to explain the R-Car perspective on why a > > family-specific fallback compatible might not be desirable, and why the > > SoC specific one is proposed? > > IIRC, it was you that wanted to use a "family-specific" fallback, I > don't understand what you want from me. If you look back at even the > context in this email, you can see you suggesting one and my counter > point. Sorry that I'm spreading my confusion around and taking up your time. I'm trying to understand if Geert's reply helped outline why a single SoC specific compatible is being used here, if so I was hoping a revised commit message would make this solution acceptable. If not I will try to summaries the issue and the different proposals so we can find a design that works and address some of the confusion before sending a new version.
On Mon, Jun 24, 2024 at 02:50:51PM +0200, Niklas Söderlund wrote: > On 2024-06-24 11:36:40 +0100, Conor Dooley wrote: > > On Mon, Jun 24, 2024 at 11:20:29AM +0200, Niklas Söderlund wrote: > > > Hi Conor, > > > > > > On 2024-06-21 09:21:24 +0200, Geert Uytterhoeven wrote: > > > > Hi Niklas, > > > > > > > > On Thu, Jun 20, 2024 at 7:22 PM Niklas Söderlund > > > > <niklas.soderlund+renesas@ragnatech.se> wrote: > > > > > On 2024-06-20 17:27:00 +0100, Conor Dooley wrote: > > > > > > > + - items: > > > > > > > + - enum: > > > > > > > - renesas,vin-r8a779g0 # R-Car V4H > > > > > > > + - renesas,vin-r8a779h0 # R-Car V4M > > > > > > > + - const: renesas,rcar-gen4-vin # Generic R-Car Gen4 > > > > > > > > > > > > > > If so I can see that working as I could still fix any issues that come > > > > > > > from differences between V4H and V4M if needed. If so do you think it > > > > > > > best to add this in two different patches? One to add the > > > > > > > renesas,rcar-gen4-vin fallback (which will also need DTS updates to fix > > > > > > > warnings from exciting users of V4H not listing the gen4 fallback) and > > > > > > > one to add V4M? > > > > > > > > > > > > > > > > > > I would just do: > > > > > > diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml > > > > > > index 5539d0f8e74d..22bbad42fc03 100644 > > > > > > --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml > > > > > > +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml > > > > > > @@ -54,6 +54,9 @@ properties: > > > > > > - renesas,vin-r8a77995 # R-Car D3 > > > > > > - renesas,vin-r8a779a0 # R-Car V3U > > > > > > - renesas,vin-r8a779g0 # R-Car V4H > > > > > > + - items: > > > > > > + - const: renesas,vin-r8a779h0 # R-Car V4L2 > > > > > > + - const: renesas,vin-r8a779g0 # R-Car V4H > > > > > > > > > > @Geert: What do you think about this? This would be a first use-case for > > > > > compatibles crossing SoC DTS files that I know of. I'm a bit uneasy > > > > > going down this road. > > > > > > > > Me too ;-) > > > > > > > > > Would this not also effect the existing users of renesas,vin-r8a779g0 > > > > > which would now need something similar to what you propose below with a > > > > > list of SoC compatibles and a fallback. > > > > > > > > > > > > > > > > > reg: > > > > > > maxItems: 1 > > > > > > > > > > > > Which requires no driver or dts changes. That could become: > > > > > > - items: > > > > > > - enum: > > > > > > - renesas,vin-r8a779h0 # R-Car V4L2 > > > > > > - renesas,vin-r8a779i0 # R-Car R4P17 > > > > > > - const: renesas,vin-r8a779g0 # R-Car V4H > > > > > > > > > > FWIW, on Gen2 where fallback es where useful compared to Gen3 we did > > > > > this with "renesas,rcar-gen2-vin". > > > > > > > > We do know there are differences (albeit probably small) among the R-Car > > > > Gen4 VIN implementations, so I am reluctant to add a family-specific > > > > compatible value. Typically we only use a family-specific compatible > > > > value if the IP cores are known (or better, assumed ;-) to be identical. > > > > > > > > And sometimes our assumptions turn out to be wrong... > > > > See slides 25-33 (last two for the numbers) of my talk at ER2019 > > > > https://embedded-recipes.org/2019/talks/herd-your-socs-become-a-matchmaker/ > > > > > > Do Geert's slides help to explain the R-Car perspective on why a > > > family-specific fallback compatible might not be desirable, and why the > > > SoC specific one is proposed? > > > > IIRC, it was you that wanted to use a "family-specific" fallback, I > > don't understand what you want from me. If you look back at even the > > context in this email, you can see you suggesting one and my counter > > point. > > Sorry that I'm spreading my confusion around and taking up your time. I don't care if non-native speakers of English say confusing things, don't worry about that. > I'm trying to understand if Geert's reply helped outline why a single > SoC specific compatible is being used here, if so I was hoping a revised > commit message would make this solution acceptable. > > If not I will try to summaries the issue and the different proposals so > we can find a design that works and address some of the confusion before > sending a new version. These devices look, for all intents and purposes, to be compatible. If they're not, *say* what is not compatible about them. Don't just say "ohh there might be, but they're small", say exactly what - because your driver makes them look compatible. It looks compatible with the a0 as well... The vibe that comes across here is of being "afraid" of having fallback compatibles that reference other SoCs - which is totally normal for other vendors, not that there are any differences in programming model between the VIN instances on these devices. Thanks, Conor.
diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml index 5539d0f8e74d..168cb02f8abe 100644 --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml @@ -54,6 +54,7 @@ properties: - renesas,vin-r8a77995 # R-Car D3 - renesas,vin-r8a779a0 # R-Car V3U - renesas,vin-r8a779g0 # R-Car V4H + - renesas,vin-r8a779h0 # R-Car V4M reg: maxItems: 1