All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Adam Ford <aford173@gmail.com>
Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	linux-media <linux-media@vger.kernel.org>,
	Benjamin Gaignard <benjamin.gaignard@collabora.com>,
	Chris Healy <cphealy@gmail.com>,
	Adam Ford-BE <aford@beaconembedded.com>,
	Nicolas Dufresne <nicolas@ndufresne.ca>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Lucas Stach <l.stach@pengutronix.de>,
	"open list:HANTRO VPU CODEC DRIVER"
	<linux-rockchip@lists.infradead.org>,
	devicetree <devicetree@vger.kernel.org>,
	arm-soc <linux-arm-kernel@lists.infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"open list:STAGING SUBSYSTEM" <linux-staging@lists.linux.dev>
Subject: Re: [PATCH 04/10] dt-bindings: media: nxp,imx8mq-vpu: Support split G1 and G2 nodes with vpu-blk-ctrl
Date: Fri, 10 Dec 2021 09:36:23 -0600	[thread overview]
Message-ID: <YbNz94G3vwbHCMdB@robh.at.kernel.org> (raw)
In-Reply-To: <CAHCN7xKrHSSsqS9DNL1tMH1Ctpz16FsSgcVbSHXzUWF98v738Q@mail.gmail.com>

On Thu, Dec 09, 2021 at 05:36:04AM -0600, Adam Ford wrote:
> On Thu, Dec 9, 2021 at 4:26 AM Ezequiel Garcia
> <ezequiel@vanguardiasur.com.ar> wrote:
> >
> > Hi,
> >
> > Thanks for the patch.
> >
> > On Wed, Dec 08, 2021 at 04:50:23PM -0600, Adam Ford wrote:
> > > The G1 and G2 are separate decoder blocks that are enabled by the
> > > vpu-blk-ctrl power-domain controller, which now has a proper driver.
> > > Update the bindings to support separate nodes for the G1 and G2
> > > decoders using the proper driver or the older unified node with
> > > the legacy controls.
> > >
> > > To be compatible with older DT the driver, mark certain items as
> > > deprecated and retain the backwards compatible example.
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > > ---
> > >  .../bindings/media/nxp,imx8mq-vpu.yaml        | 83 ++++++++++++++-----
> > >  1 file changed, 64 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > > index 762be3f96ce9..eeb7bd6281f9 100644
> > > --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > > @@ -15,29 +15,39 @@ description:
> > >
> > >  properties:
> > >    compatible:
> > > -    const: nxp,imx8mq-vpu
> > > +    oneOf:
> > > +      - const: nxp,imx8mq-vpu
> > > +        deprecated: true
> > > +      - const: nxp,imx8mq-vpu-g1
> > > +      - const: nxp,imx8mq-vpu-g2
> > >
> > >    reg:
> > > +    minItems: 1
> > >      maxItems: 3
> >
> > Is it really useful to keep the deprecated binding nxp,imx8mq-vpu
> > as something supported by the binding file?
> 
> Since I was told that the driver needed to be backwards compatible, i
> wanted to make sure that any attempts to build the old device tree
> would not fail

I'm not convinced changing the binding at all is correct. 'The driver 
structure is changing and I want the binding to align with it' is not a 
reason. Are G1 and G2 actually separate, independent blocks where we 
could have 1 or both of them? And what about other platforms using this 
block?

Even if the driver handles the old binding, a new dtb with an old kernel 
is broken. It's up to the platform to care or not, but you have to 
highlight that.


> > In other words, can we drop the deprecated binding from this file,
> > while keeping the support in the driver for legacy device-trees?
> 
> I was trying to represent both the old driver binding and the new one
> at the same time.  I thought that's what I was told to do.

I don't care so much if we have a schema for old binding. I'd rather 
have warnings if the binding has not been updated. Eventually I want to 
be able to test for compatibility by testing DTs with different schema 
versions. We've got to get to 0 warnings first though...

Rob

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Adam Ford <aford173@gmail.com>
Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	linux-media <linux-media@vger.kernel.org>,
	Benjamin Gaignard <benjamin.gaignard@collabora.com>,
	Chris Healy <cphealy@gmail.com>,
	Adam Ford-BE <aford@beaconembedded.com>,
	Nicolas Dufresne <nicolas@ndufresne.ca>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Lucas Stach <l.stach@pengutronix.de>,
	"open list:HANTRO VPU CODEC DRIVER"
	<linux-rockchip@lists.infradead.org>,
	devicetree <devicetree@vger.kernel.org>,
	arm-soc <linux-arm-kernel@lists.infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"open list:STAGING SUBSYSTEM" <linux-staging@lists.linux.dev>
Subject: Re: [PATCH 04/10] dt-bindings: media: nxp,imx8mq-vpu: Support split G1 and G2 nodes with vpu-blk-ctrl
Date: Fri, 10 Dec 2021 09:36:23 -0600	[thread overview]
Message-ID: <YbNz94G3vwbHCMdB@robh.at.kernel.org> (raw)
In-Reply-To: <CAHCN7xKrHSSsqS9DNL1tMH1Ctpz16FsSgcVbSHXzUWF98v738Q@mail.gmail.com>

On Thu, Dec 09, 2021 at 05:36:04AM -0600, Adam Ford wrote:
> On Thu, Dec 9, 2021 at 4:26 AM Ezequiel Garcia
> <ezequiel@vanguardiasur.com.ar> wrote:
> >
> > Hi,
> >
> > Thanks for the patch.
> >
> > On Wed, Dec 08, 2021 at 04:50:23PM -0600, Adam Ford wrote:
> > > The G1 and G2 are separate decoder blocks that are enabled by the
> > > vpu-blk-ctrl power-domain controller, which now has a proper driver.
> > > Update the bindings to support separate nodes for the G1 and G2
> > > decoders using the proper driver or the older unified node with
> > > the legacy controls.
> > >
> > > To be compatible with older DT the driver, mark certain items as
> > > deprecated and retain the backwards compatible example.
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > > ---
> > >  .../bindings/media/nxp,imx8mq-vpu.yaml        | 83 ++++++++++++++-----
> > >  1 file changed, 64 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > > index 762be3f96ce9..eeb7bd6281f9 100644
> > > --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > > @@ -15,29 +15,39 @@ description:
> > >
> > >  properties:
> > >    compatible:
> > > -    const: nxp,imx8mq-vpu
> > > +    oneOf:
> > > +      - const: nxp,imx8mq-vpu
> > > +        deprecated: true
> > > +      - const: nxp,imx8mq-vpu-g1
> > > +      - const: nxp,imx8mq-vpu-g2
> > >
> > >    reg:
> > > +    minItems: 1
> > >      maxItems: 3
> >
> > Is it really useful to keep the deprecated binding nxp,imx8mq-vpu
> > as something supported by the binding file?
> 
> Since I was told that the driver needed to be backwards compatible, i
> wanted to make sure that any attempts to build the old device tree
> would not fail

I'm not convinced changing the binding at all is correct. 'The driver 
structure is changing and I want the binding to align with it' is not a 
reason. Are G1 and G2 actually separate, independent blocks where we 
could have 1 or both of them? And what about other platforms using this 
block?

Even if the driver handles the old binding, a new dtb with an old kernel 
is broken. It's up to the platform to care or not, but you have to 
highlight that.


> > In other words, can we drop the deprecated binding from this file,
> > while keeping the support in the driver for legacy device-trees?
> 
> I was trying to represent both the old driver binding and the new one
> at the same time.  I thought that's what I was told to do.

I don't care so much if we have a schema for old binding. I'd rather 
have warnings if the binding has not been updated. Eventually I want to 
be able to test for compatibility by testing DTs with different schema 
versions. We've got to get to 0 warnings first though...

Rob

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Adam Ford <aford173@gmail.com>
Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	linux-media <linux-media@vger.kernel.org>,
	Benjamin Gaignard <benjamin.gaignard@collabora.com>,
	Chris Healy <cphealy@gmail.com>,
	Adam Ford-BE <aford@beaconembedded.com>,
	Nicolas Dufresne <nicolas@ndufresne.ca>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Lucas Stach <l.stach@pengutronix.de>,
	"open list:HANTRO VPU CODEC DRIVER"
	<linux-rockchip@lists.infradead.org>,
	devicetree <devicetree@vger.kernel.org>,
	arm-soc <linux-arm-kernel@lists.infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"open list:STAGING SUBSYSTEM" <linux-staging@lists.linux.dev>
Subject: Re: [PATCH 04/10] dt-bindings: media: nxp,imx8mq-vpu: Support split G1 and G2 nodes with vpu-blk-ctrl
Date: Fri, 10 Dec 2021 09:36:23 -0600	[thread overview]
Message-ID: <YbNz94G3vwbHCMdB@robh.at.kernel.org> (raw)
In-Reply-To: <CAHCN7xKrHSSsqS9DNL1tMH1Ctpz16FsSgcVbSHXzUWF98v738Q@mail.gmail.com>

On Thu, Dec 09, 2021 at 05:36:04AM -0600, Adam Ford wrote:
> On Thu, Dec 9, 2021 at 4:26 AM Ezequiel Garcia
> <ezequiel@vanguardiasur.com.ar> wrote:
> >
> > Hi,
> >
> > Thanks for the patch.
> >
> > On Wed, Dec 08, 2021 at 04:50:23PM -0600, Adam Ford wrote:
> > > The G1 and G2 are separate decoder blocks that are enabled by the
> > > vpu-blk-ctrl power-domain controller, which now has a proper driver.
> > > Update the bindings to support separate nodes for the G1 and G2
> > > decoders using the proper driver or the older unified node with
> > > the legacy controls.
> > >
> > > To be compatible with older DT the driver, mark certain items as
> > > deprecated and retain the backwards compatible example.
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > > ---
> > >  .../bindings/media/nxp,imx8mq-vpu.yaml        | 83 ++++++++++++++-----
> > >  1 file changed, 64 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > > index 762be3f96ce9..eeb7bd6281f9 100644
> > > --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > > @@ -15,29 +15,39 @@ description:
> > >
> > >  properties:
> > >    compatible:
> > > -    const: nxp,imx8mq-vpu
> > > +    oneOf:
> > > +      - const: nxp,imx8mq-vpu
> > > +        deprecated: true
> > > +      - const: nxp,imx8mq-vpu-g1
> > > +      - const: nxp,imx8mq-vpu-g2
> > >
> > >    reg:
> > > +    minItems: 1
> > >      maxItems: 3
> >
> > Is it really useful to keep the deprecated binding nxp,imx8mq-vpu
> > as something supported by the binding file?
> 
> Since I was told that the driver needed to be backwards compatible, i
> wanted to make sure that any attempts to build the old device tree
> would not fail

I'm not convinced changing the binding at all is correct. 'The driver 
structure is changing and I want the binding to align with it' is not a 
reason. Are G1 and G2 actually separate, independent blocks where we 
could have 1 or both of them? And what about other platforms using this 
block?

Even if the driver handles the old binding, a new dtb with an old kernel 
is broken. It's up to the platform to care or not, but you have to 
highlight that.


> > In other words, can we drop the deprecated binding from this file,
> > while keeping the support in the driver for legacy device-trees?
> 
> I was trying to represent both the old driver binding and the new one
> at the same time.  I thought that's what I was told to do.

I don't care so much if we have a schema for old binding. I'd rather 
have warnings if the binding has not been updated. Eventually I want to 
be able to test for compatibility by testing DTs with different schema 
versions. We've got to get to 0 warnings first though...

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-12-10 15:36 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-08 22:50 [PATCH 00/10] media: hantro: imx8mq/imx8mm: Let VPU decoders get controlled by vpu-blk-ctrl Adam Ford
2021-12-08 22:50 ` Adam Ford
2021-12-08 22:50 ` Adam Ford
2021-12-08 22:50 ` [PATCH 01/10] dt-bindings: power: imx8mq: add defines for VPU blk-ctrl domains Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-08 22:50 ` [PATCH 02/10] dt-bindings: soc: add binding for i.MX8MQ VPU blk-ctrl Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-08 22:50 ` [PATCH 03/10] soc: imx: imx8m-blk-ctrl: add " Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-08 22:50 ` [PATCH 04/10] dt-bindings: media: nxp,imx8mq-vpu: Support split G1 and G2 nodes with vpu-blk-ctrl Adam Ford
2021-12-08 22:50   ` [PATCH 04/10] dt-bindings: media: nxp, imx8mq-vpu: " Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-09  3:08   ` [PATCH 04/10] dt-bindings: media: nxp,imx8mq-vpu: " Rob Herring
2021-12-09  3:08     ` [PATCH 04/10] dt-bindings: media: nxp, imx8mq-vpu: " Rob Herring
2021-12-09  3:08     ` Rob Herring
2021-12-09 10:26   ` [PATCH 04/10] dt-bindings: media: nxp,imx8mq-vpu: " Ezequiel Garcia
2021-12-09 10:26     ` Ezequiel Garcia
2021-12-09 10:26     ` Ezequiel Garcia
2021-12-09 11:36     ` Adam Ford
2021-12-09 11:36       ` Adam Ford
2021-12-09 11:36       ` Adam Ford
2021-12-10 15:36       ` Rob Herring [this message]
2021-12-10 15:36         ` Rob Herring
2021-12-10 15:36         ` Rob Herring
2021-12-10 15:58         ` Lucas Stach
2021-12-10 15:58           ` Lucas Stach
2021-12-10 15:58           ` Lucas Stach
2021-12-10 17:02           ` Adam Ford
2021-12-10 17:02             ` Adam Ford
2021-12-10 17:02             ` Adam Ford
2021-12-10 18:32             ` Ezequiel Garcia
2021-12-10 18:32               ` Ezequiel Garcia
2021-12-10 18:32               ` Ezequiel Garcia
2021-12-08 22:50 ` [PATCH 05/10] media: hantro: Allow i.MX8MQ G1 and G2 to run independently Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-08 22:50 ` [PATCH 06/10] arm64: dts: imx8mq: Enable both G1 and G2 VPU's with vpu-blk-ctrl Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-08 22:50 ` [PATCH 07/10] arm64: dts: imx8mm: Fix VPU Hanging Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-08 22:50 ` [PATCH 08/10] dt-bindings: media: nxp,imx8mq-vpu: Enable support for i.MX8M Mini Adam Ford
2021-12-08 22:50   ` [PATCH 08/10] dt-bindings: media: nxp, imx8mq-vpu: " Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-15 16:53   ` [PATCH 08/10] dt-bindings: media: nxp,imx8mq-vpu: " Rob Herring
2021-12-15 16:53     ` [PATCH 08/10] dt-bindings: media: nxp, imx8mq-vpu: " Rob Herring
2021-12-15 16:53     ` Rob Herring
2021-12-08 22:50 ` [PATCH 09/10] media: hantro: Add support for i.MX8MM Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-08 22:50 ` [PATCH 10/10] arm64: dts: imx8mm: Enable Hantro G1 and G2 video decoders Adam Ford
2021-12-08 22:50   ` Adam Ford
2021-12-08 22:50   ` Adam Ford

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=YbNz94G3vwbHCMdB@robh.at.kernel.org \
    --to=robh@kernel.org \
    --cc=aford173@gmail.com \
    --cc=aford@beaconembedded.com \
    --cc=benjamin.gaignard@collabora.com \
    --cc=cphealy@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=ezequiel@vanguardiasur.com.ar \
    --cc=festevam@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel@pengutronix.de \
    --cc=l.stach@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=mchehab@kernel.org \
    --cc=nicolas@ndufresne.ca \
    --cc=p.zabel@pengutronix.de \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.