Re: [PATCH 1/3] dt-bindings: mtk-jpeg: Add binding for MT8195 JPG

From: kyrie.wu <kyrie.wu@mediatek.com>
To: Tzung-Bi Shih <tzungbi@google.com>
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Rick Chang <rick.chang@mediatek.com>,
	Bin Liu <bin.liu@mediatek.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Tzung-Bi Shih <tzungbi@chromium.org>,
	<Project_Global_Chrome_Upstream_Group@mediatek.com>,
	<linux-media@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-mediatek@lists.infradead.org>,
	Tomasz Figa <tfiga@chromium.org>, <xia.jiang@mediatek.com>,
	<maoguang.meng@mediatek.com>, <kyrie.wu@mediatek.com>,
	<srv_heupstream@mediatek.com>
Subject: Re: [PATCH 1/3] dt-bindings: mtk-jpeg: Add binding for MT8195 JPG
Date: Mon, 5 Jul 2021 18:02:49 +0800	[thread overview]
Message-ID: <1625479369.1462.5.camel@mhfsdcap03> (raw)
In-Reply-To: <CA+Px+wUXK2t42S0HgcOj7_kbkSskHGris5LBquTS3d+hi8kCaQ@mail.gmail.com>

On Mon, 2021-07-05 at 16:09 +0800, Tzung-Bi Shih wrote:
> On Wed, Jun 30, 2021 at 3:56 PM kyrie.wu <kyrie.wu@mediatek.com> wrote:
> > Mt8195 has two hardwares, "mediatek,mt8195-jpgenc0" for hw0 and
> > "mediatek,mt8195-jpgenc1" for HW1. These two nodes will register
> > hardware interrupt, initialize clock, power domain, remap register base
> > addr and other operations. But the device node is not registered.
> > "mediatek,mt8195-jpgenc" will register the device node to represent jpeg
> > encode device. Then the component framework is used to manage the above
> > two hardwares.
> Please don't top-posting.  Inline your replies so that people can
> easily follow the discussion.
> 
> I still don't quite understand why it needs to introduce 2 compatible
> strings.  If hw0 and hw1 are different from interrupts, clocks, power
> domain, and etc, couldn't you use the same compatible string (e.g.
> "mt8195-jpgenc") and provide them different DT attributes?
Dear Tzung-Bi,

As you replied, I can use the same compatible string (e.g
"mt8195-jpgenc") and provide resources with different DT attributes.
But there are the following problems:
The following description takes the resource of clock as an example:
1. If different DT attributes are used to represent different clock
information,the structure array (e.g.struct clkinfo) can be used in the
driver to save the corresponding information, which will increase the
complexity of the software;
2. When using hw0, you only need to turn on clock0, but not clock1,
which can reduce power consumption. When the driver uses one of the
clocks, it needs to extract each structure from structure array
(e.g.struct clkinfo) and compare the attributes to confirming which one
to use.
The software architecture is as follows:
if (...) {
[...]
} else if (...) {
[...]
} 
......
At present, there are only two HWS, and the software architecture is not
complex. With the increase of the number of hardware, the software
architecture will become more and more complex.
To sum up, using different compatibles to represent different HW
can reduce power consumption, decrease code complexity
and enhance code readability.
> 
> > On Fri, 2021-06-25 at 17:18 +0800, Tzung-Bi Shih wrote:
> > > On Wed, Jun 23, 2021 at 2:06 PM kyrie.wu <kyrie.wu@mediatek.com> wrote:
> > > >  Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > Note: the patch won't apply after [1].
> > >
> > > [1]: https://lore.kernel.org/patchwork/patch/1445298/
> > >
> > > >  Required properties:
> > > >  - compatible : "mediatek,mt2701-jpgenc"
> > > > +- compatible : "mediatek,mt8195-jpgenc"
> > > > +- compatible : "mediatek,mt8195-jpgenc0"
> > > > +- compatible : "mediatek,mt8195-jpgenc1"
> > > Why it needs 3 compatible strings?

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