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
