From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sakari Ailus Subject: Re: [PATCH v4 01/14] dt-bindings: Add binding for MT2712 MIPI-CSI2 Date: Mon, 26 Aug 2019 17:05:36 +0300 Message-ID: <20190826140536.GE826@valkosipuli.retiisi.org.uk> References: <1559643115-15124-1-git-send-email-stu.hsieh@mediatek.com> <1559643115-15124-2-git-send-email-stu.hsieh@mediatek.com> <1560134057.28527.5.camel@mtksdaap41> <1560153073.31854.6.camel@mtksdaap41> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Stu Hsieh Cc: CK Hu , Tomasz Figa , Mauro Carvalho Chehab , Rob Herring , Mark Rutland , Matthias Brugger , Linux Media Mailing List , devicetree@vger.kernel.org, Linux Kernel Mailing List , "list@263.net:IOMMU DRIVERS , Joerg Roedel ," , "moderated list:ARM/Mediatek SoC support" , srv_heupstream List-Id: linux-mediatek@lists.infradead.org On Mon, Jun 10, 2019 at 04:58:02PM +0900, Tomasz Figa wrote: > On Mon, Jun 10, 2019 at 4:51 PM CK Hu wrote: > > > > Hi, Tomasz: > > > > On Mon, 2019-06-10 at 12:32 +0900, Tomasz Figa wrote: > > > Hi CK, Stu, > > > > > > On Mon, Jun 10, 2019 at 11:34 AM CK Hu wrote: > > > > > > > > Hi, Stu: > > > > > > > > "mediatek,mt2712-mipicsi" and "mediatek,mt2712-mipicsi-common" have many > > > > common part with "mediatek,mt8183-seninf", and I've a discussion in [1], > > > > so I would like these two to be merged together. > > > > > > > > [1] https://patchwork.kernel.org/patch/10979131/ > > > > > > > > > > Thanks CK for spotting this. > > > > > > I also noticed that the driver in fact handles two hardware blocks at > > > the same time - SenInf and CamSV. Unless the architecture is very > > > different from MT8183, I'd suggest splitting it. > > > > > > On a general note, the MT8183 SenInf driver has received several > > > rounds of review comments already, but I couldn't find any comments > > > posted for this one. > > > > > > Given the two aspects above and also based on my quick look at code > > > added by this series, I'd recommend adding MT2712 support on top of > > > the MT8183 series. > > > > In [1], "mediatek,mt8183-seninf" use one device to control multiple csi > > instance, so it duplicate many register definition. In [2], one > > "mediatek,mt2712-mipicsi" device control one csi instance, so there are > > multiple device and the register definition does not duplicate. > > I guess we didn't catch that in the review yet. It should be fixed. > > > You > > recommend adding MT2712 support on top of the MT8183 series, do you mean > > that "mediatek,mt2712-mipicsi" should use one device to control multiple > > csi instance and duplicate the register setting? > > There are some aspects of MT8183 series that are done better than the > MT2712 series, but apparently there are also some better aspects in > MT2712. We should take the best aspects of both series. :) Stu: Are the two devices similar enough or not; does this look like a feasible approach to you? -- Sakari Ailus