public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
To: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	Rob Herring <robh@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Heiko Stuebner <heiko@sntech.de>,
	devicetree <devicetree@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 3/3] arm64: dts: rockchip: Add Hantro encoder node to rk356x
Date: Fri, 13 May 2022 18:36:30 +0200	[thread overview]
Message-ID: <42373378.kc9IRrpb2S@archbook> (raw)
In-Reply-To: <20220513152358.GA334264-robh@kernel.org>

Hi Rob,

On Freitag, 13. Mai 2022 17:23:58 CEST Rob Herring wrote:
> On Fri, May 13, 2022 at 10:07:51AM -0300, Ezequiel Garcia wrote:
> > [...]
> > 
> > I didn't ignore it, I just didn't reply to it. You think this is about
> > changing a dt-binding, but you are actually introducing a new dt-binding
> > since you are adding a new compatible string.
> > 
> > You are doing so by extending an existing dt-binding.
> > 
> > I am explaining you the _existing_ dt-binding models the (incorrect) idea
> > of a combined decoder and encoder. Since your device is encoder-only
> > and has a single interrupt line, you should omit the interrupt-names,
> > because it doesn't not add anything.
> 
> While in general I agree on single entries don't need -names, given just 
> 'vdpu' is already allowed I would go with symmetry here and allow it for 
> the encoder.
> 
> If you wanted to mark 'vdpu' alone as deprecated, then that would be 
> fine too. No need for an if/then schema to disallow interrupt-names 
> either. Eventually, I plan to (optionally) remove deprecated schemas 
> from validation and that would have the effect of requiring 
> interrupt-names to have 2 entries here.
> 
> Rob

After some discussion in the #linux-media IRC channel on OFTC, we've
come to the conclusion that moving forward with a separate binding for
encoder-only instances would probably be a wise move, as the VPU binding
assumes we'll always have a decoder, and breaking this assumption will
just make it more complex the longer we go on not separating these.

While I was writing such a separate binding for v4 of my series, I
discovered that this makes things quite a bit simpler. Especially
looking into the near future, there will be between one to two
additional compatibles that will be added to this binding for the
RK3588, as it has no less than 5 instances capable of encoding
JPEG, and I believe at least one of them is capable of other
formats as well.

Regards,
Nicolas Frattaroli



      reply	other threads:[~2022-05-13 16:36 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-08 20:25 [PATCH v2 0/3] Enable JPEG Encoder on RK3566/RK3568 Nicolas Frattaroli
2022-05-08 20:25 ` [PATCH v2 1/3] dt-bindings: media: rockchip-vpu: Add RK3568 VEPU compatible Nicolas Frattaroli
2022-05-09  7:25   ` Krzysztof Kozlowski
2022-05-09  9:24     ` Nicolas Frattaroli
2022-05-09 10:41       ` Krzysztof Kozlowski
2022-05-08 20:25 ` [PATCH v2 2/3] media: hantro: Add support for RK356x encoder Nicolas Frattaroli
2022-05-08 20:25 ` [PATCH v2 3/3] arm64: dts: rockchip: Add Hantro encoder node to rk356x Nicolas Frattaroli
2022-05-09 14:17   ` Ezequiel Garcia
2022-05-10 15:27     ` Nicolas Frattaroli
2022-05-12 14:16       ` Ezequiel Garcia
2022-05-12 20:00         ` Nicolas Frattaroli
2022-05-12 21:33           ` Ezequiel Garcia
2022-05-13  6:23             ` Nicolas Frattaroli
2022-05-13 13:07               ` Ezequiel Garcia
2022-05-13 14:44                 ` Nicolas Frattaroli
2022-05-13 15:23                 ` Rob Herring
2022-05-13 16:36                   ` Nicolas Frattaroli [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=42373378.kc9IRrpb2S@archbook \
    --to=frattaroli.nicolas@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=ezequiel@vanguardiasur.com.ar \
    --cc=heiko@sntech.de \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox