linux-kernel - Re: [PATCH v4 4/8] drm/mediatek: add support for Mediatek SoC MT2701

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1469671655.11841.85.camel@mtksdaap41>
Date:	Thu, 28 Jul 2016 10:07:35 +0800
From:	CK Hu <ck.hu@...iatek.com>
To:	YT Shen <yt.shen@...iatek.com>
CC:	Philipp Zabel <p.zabel@...gutronix.de>,
	<dri-devel@...ts.freedesktop.org>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	"Mark Rutland" <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	"Russell King" <linux@....linux.org.uk>,
	David Airlie <airlied@...ux.ie>,
	"Matthias Brugger" <matthias.bgg@...il.com>,
	Mao Huang <littlecvr@...omium.org>,
	"Bibby Hsieh" <bibby.hsieh@...iatek.com>,
	<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>,
	<linux-mediatek@...ts.infradead.org>,
	<srv_heupstream@...iatek.com>,
	"Sascha Hauer" <kernel@...gutronix.de>,
	<yingjoe.chen@...iatek.com>, <emil.l.velikov@...il.com>,
	<thierry.reding@...il.com>
Subject: Re: [PATCH v4 4/8] drm/mediatek: add support for Mediatek SoC MT2701

Hi, YT:

On Wed, 2016-07-27 at 12:03 +0200, Philipp Zabel wrote:
> Am Dienstag, den 26.07.2016, 18:42 +0800 schrieb YT Shen:
> > Hi CK,
> > 
> > On Wed, 2016-07-20 at 14:53 +0800, CK Hu wrote:
> > > Hi, YT:
> > > 
> > > Some comments inline.
> > > 
> > > On Fri, 2016-07-15 at 18:07 +0800, YT Shen wrote:
> > > > This patch add support for the Mediatek MT2701 DISP subsystem.
> > > > There is only one OVL engine in MT2701.
> > > > 
> > > > Signed-off-by: YT Shen <yt.shen@...iatek.com>
> > > > ---
> > > >  drivers/gpu/drm/mediatek/mtk_disp_ovl.c     |    6 ++++
> > > >  drivers/gpu/drm/mediatek/mtk_disp_rdma.c    |    6 ++++
> > > >  drivers/gpu/drm/mediatek/mtk_drm_ddp.c      |   41 +++++++++++++++++++++++++++
> > > >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |    7 +++++
> > > >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |    1 +
> > > >  drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   31 ++++++++++++++++++++
> > > >  6 files changed, 92 insertions(+)
> > > > 
> > > 
> > > [snip...]
> > > 
> > > >  
> > > >  static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
> > > > +	{ .compatible = "mediatek,mt2701-disp-ovl",   .data = (void *)MTK_DISP_OVL },
> > > >  	{ .compatible = "mediatek,mt8173-disp-ovl",   .data = (void *)MTK_DISP_OVL },
> > > > +	{ .compatible = "mediatek,mt2701-disp-rdma",  .data = (void *)MTK_DISP_RDMA },
> > > >  	{ .compatible = "mediatek,mt8173-disp-rdma",  .data = (void *)MTK_DISP_RDMA },
> > > > +	{ .compatible = "mediatek,mt2701-disp-wdma",  .data = (void *)MTK_DISP_WDMA },
> > > 
> > > Is WDMA different from MT8173 to MT2701. If they are the same, you need
> > > not to add compatible of 'mediatek,mt2701-disp-wdma' because use
> > > 'mediatek,mt8173-disp-wdma' is enough.
> > Yes, the hardware has differences, so we need add new compatible
> > 'mt2701-disp-wdma'
> 
> It would be nice if you could add a short paragraph to the patch
> description about what the hardware differences are (probably just
> changed register layout in most cases).

If you don't want to add description, you may separate the modification
of OVL, RDMA, WDMA, PWM to different patch. And you send patch of OVL
and RDMA first because you have related code modification for these two.
You temporarily treat WDMA and PWM as MT8173 and it works. And one day
you need to modify code of WDMA and PWM for MT2701, then you send the
patch of WDMA and PWM and you need not to add any description because we
can find out the difference from source code.

> 
> regards
> Philipp
> 

Regards,
CK