Re: [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes

[Ezequiel Garcia]

On Fri, 2021-06-11 at 21:33 +0800, Dong Aisheng wrote:
> [...]
> 
> > > > +img_subsys: bus@58000000 {
> > > > +   compatible = "simple-bus";
> > > > +   #address-cells = <1>;
> > > > +   #size-cells = <1>;
> > > > +   ranges = <0x58000000 0x0 0x58000000 0x1000000>;
> > > > +
> > > > +   img_ipg_clk: clock-img-ipg {
> > > > +           compatible = "fixed-clock";
> > > > +           #clock-cells = <0>;
> > > > +           clock-frequency = <200000000>;
> > > > +           clock-output-names = "img_ipg_clk";
> > > > +   };
> > > > +
> > > > +   jpegdec: jpegdec@58400000 {
> > > 
> > > Node should be disabled by default.
> > > And enable it in board dts including LPCG.
> > 
> > At version v5 of this patch, the node was disabled by default, and I
> > received this feedback from Ezequiel Garcia:
> > 
> > "Pure memory-to-memory are typically not enabled per-board, but just
> > per-platform.
> > So you can drop the disabled status here."
> > 
> > So, in v6 I made it enabled by default.
> > 
> > Any strong reasons for enabled/disabled per platform?
> 
> AFAIK we usually only enable system basic features and let other
> user selectable features disabled by default in dts.
> Even for device LPCG clocks, if it's enabled by default and later
> enter runtime suspend if no users, it still consumes power.
> 

Well-written drivers shouldn't draw any power if not used.

And DT is about hardware-description, not about usage-description.
Which means, at the soc.dtsi level you disable devices that need
some board-level hardware thing to be enabled (e.g. a physical
connected, a regulator, etc.).

A pure memory-to-memory should be enabled by default, because
in practice you can't predict what the users a board will want
to use, nor the DT is the place for that.

Sticking to hardware description is the best way to get DT right :-)

Cheers,
Ezequiel