From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_2 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 15517C433E1 for ; Thu, 13 Aug 2020 09:59:58 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D343F206A4 for ; Thu, 13 Aug 2020 09:59:57 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="b4JtEaJ3"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="XfTzQarb" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D343F206A4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=+0Z6sPDAGx/Tpce/vo5frxfF0smMhj2JtVMRyF/xpXM=; b=b4JtEaJ3/giRodheuYo2eBZ9n jlIipJOFTwGOeAi7iEyD5IIa3ts+N9mHnHSeoESiIGkF6TqLZ47LV+8eNeEw87P7/zp6ao3z8B7PC x/fKrK4UHvry5MZoKYysMvLdx3jXOa8bhYubIwO1x3gh1V1AUjDVNEP48n2sP8KWQstQb1p1P2yA6 ADY1l4Tbd2qOZG+kwkUGibMi2XEWVPScdtwCP3p7W6yFzeeJChPOXMfnDNs7g/GO8z2jNfFo0SHOV 015MOGyEZLO+lalNXq40sL71p59EQ9Yf9QNVeS5kJJiHYK6fHK+Ger8h0j17hTcUT0Ldz0Il9WV3+ h6tLcsCXQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k6A0J-0007xx-Os; Thu, 13 Aug 2020 09:58:35 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1k6A0H-0007xO-0a for linux-arm-kernel@lists.infradead.org; Thu, 13 Aug 2020 09:58:34 +0000 Received: from coco.lan (ip5f5ad5c5.dynamic.kabel-deutschland.de [95.90.213.197]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id C8385206A4; Thu, 13 Aug 2020 09:58:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1597312712; bh=9RgQzuhqlRmtPEcH71/uI/W3vda4Hj2yojosYlaMzQc=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=XfTzQarbNsyDPeTzjsFu2xl1etxDXCqda8LvDEXpdjegidj0Xgsr2kj27WVGm6slM o8qXDSNIz8t27fK7IrRxoLEY5xrWvi+vTNigaxxqamJKt4vRAGMO9Ag7XojLkptG8B OIGlea/1wskhl5ANMK5MBT5wraJ2/dxIfgFnj16o= Date: Thu, 13 Aug 2020 11:58:23 +0200 From: Mauro Carvalho Chehab To: Lee Jones Subject: Re: [PATCH 00/44] SPMI patches needed by Hikey 970 Message-ID: <20200813115823.70f9016a@coco.lan> In-Reply-To: <20200813075827.GH4354@dell> References: <20200813075827.GH4354@dell> X-Mailer: Claws Mail 3.17.5 (GTK+ 2.24.32; x86_64-redhat-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200813_055833_199545_BA396B40 X-CRM114-Status: GOOD ( 50.16 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devel@driverdev.osuosl.org, devicetree@vger.kernel.org, Rob Herring , Stephen Boyd , Greg Kroah-Hartman , Mark Brown , Jonathan Cameron , linuxarm@huawei.com, Wei Xu , linux-kernel@vger.kernel.org, Rob Herring , linux-arm-msm@vger.kernel.org, mauro.chehab@huawei.com, "David S. Miller" , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Lee, Em Thu, 13 Aug 2020 08:58:27 +0100 Lee Jones escreveu: > On Wed, 12 Aug 2020, Mauro Carvalho Chehab wrote: > > > Hi Greg, > > > > This patch series is part of a work I'm doing in order to be able to support > > a HiKey 970 board that I recently got on my hands. > > > > I received some freedback from Mark and from Jonathan on a first > > attempt I made to upstream this. > > > > I'm opting to submit it via staging, because I had to start from the > > patch that originally added this driver on a 4.9 Kernel tree: > > > > https://github.com/96boards-hikey/linux/tree/hikey970-v4.9 > > > > In order to preserve the original SOB from the driver's author. > > > > The patches following it are on the standard way: one patch per > > logical change. > > > > This is part of a bigger work whose goal is to have upstream support > > for USB and DRM/KMS on such boards. > > > > I suspect that, maybe except for the DT part, those 3 specific drivers > > are more or less ready to be moved from staging, but the other > > drivers that are also part of this attempt aren't ready. Specially the > > DRM driver has some bugs that came from the OOT version. > > > > So, my current plan is to submit those drivers to staging for 5.9 > > and move the ones that are ok out of staging on Kernel 5.10. > > What a mess. This is no way to upstream a new driver. > > Firstly, could you please add versioning to your submissions. I know > this at least version 2. Were there previous submissions? Is this > the latest? Yeah, that's the second attempt. The first one was: https://lore.kernel.org/lkml/176043f329dfa9889f014feec04e7e1553077873.1597160086.git.mchehab+huawei@kernel.org/T/#u I was in doubt about adding a v2 in this specific case or not, since I ended submitting it to the staging tree. > Secondly and more importantly, you have submitted what looks like a > new driver (bearing in mind that I'm only concerning myself with the > MFD related changes), then in the same submission you are adding and > removing large chunks. Please just submit the new driver, on its own > as a single patch, complete with its associated Makefile and Kconfig > changes. I can't do like that because I'm not the author of the original patch that added the driver. The original patch came from the 96board's android-kernel based 4.9 tree: https://github.com/96boards-hikey/linux/tree/hikey970-v4.9 > What are your reasons for submitting this via Staging? The main reason is to preserve both the patch authorship and its history. After the original patch, I wrote several incremental changes cleaning up the original driver and stripping parts of it that aren't needed. By preserving the history, if someone wants to restore some removed functionality, it is just a matter of reverting a patch. For example, the original driver had its own sysfs interface for debugging the regulator driver. This is not needed for it to work. Also, the right interface for such things is via configfs. Yet, someone could think on restoring such feat and start from the existing code, instead of coming with something else from scratch. > Is it not ready yet? >From my side, I believe that, after my changes, the code now meets upstream requirements, maybe except for DT (and the parsing code). There are a few things at the DT properties on this driver that could be named on a different (more standard way). Yet, I'm not a regular contributor for mfd/regulator/spmi. So, I may have missed something. > Are the resultant components not at a high enough level of > quality or enablement to go straight into the subsystems, which is > more typical? From an MFD perspective, I would be reviewing the > driver as a whole when (if) it moves from Staging into MFD anyway, so > why are you jumping through hoops with this additional, seemingly > superfluous step? I'm OK if this gets reviewed by MFD people only after moving it out of staging. Assuming that this would be merged for Kernel 5.10, I'll likely send a patch moving it out of staging for 5.11. Then, you can do a comprehensive review. > Finally, the subject of authorship is often a contentious one, but > this is a problem you need to work out with the original author, not > something that should require special handing by upstream. You have a > couple of choices, but bear in mind that upstreaming a non-suitable > driver then bringing it up to standard is not one of them. > > 1. Keep the original author's authorship and SoB, make your changes > and get them to review to ensure they are still happy about being > associated with the resultant code. Ensure you mention all of the > changes you make in the commit message and follow-up by adding your > own SoB. > > 2. This is the contentious bit. If you've made enough changes, there > is an argument for you to adopt authorship. You should discuss > with the original author whether they are happy for you to retain > their SoB. My suggestion is always try to keep the SoB as a bare > minimum to preserve patch history and out of pure courtesy. It is not only the above. Both the original author and anyone touching the code should comply with applicable internal policies. >From my experience, dealing with such things takes a lot more of time then coding, as it require talking with legal departments on different continents, and with developers and with their bosses in order to be able to do things like that. This can also be a very frustrating process. During almost 20 years of being the media maintainer, I've seen several cases where trying to enforce a folded initial patch caused devs to receive NACKS, preventing them so submit otherwise good stuff. So, at the media subsystem, I'm perfectly fine if someone starts from the original OOT driver, preserving its original authorships. We're also dealing there with the patches sent to drivers/staging/media. I'm not saying that other subsystem maintainers should do the same. Dealing with staging is time consuming, and I completely understand that most maintainers prefer to stay out of it ;-) - Since when staging tree started, if someone has to start from the original patch, such things can be merged at staging. Then, incremental patches are applied at the top until it reaches what's required to be promoted. That's said, there's no hush to have those drivers out of staging. My end goal is to have DRM/KMS and USB support for Hikey 970. The patchsets I have so far are at: https://github.com/mchehab/linux/commits/hikey970/to_upstream-2.0-v1.1 (this branch has the v1 of my patchset) Porting this driver is part of such effort. While this driver is on a good situation, the other ones may require some time to mature. The DRM/KMS driver for example, is not ready to be merged outside staging, as it carries several bugs that came from the original driver and are present at the official tree at 96boards. For example, there is a a very dirty hack that enforces the HDMI chipset to only work with a limited set of resolutions that are known to work: https://github.com/96boards-hikey/linux/blob/hikey970-v4.9/drivers/gpu/drm/hisilicon/kirin9xx/hdmi/adv7535.c#L869 It also has problems reading the frequencies via EDID interface. Due to that, the driver fakes an EDID table: https://github.com/96boards-hikey/linux/blob/hikey970-v4.9/drivers/gpu/drm/hisilicon/kirin9xx/hdmi/adv7535.c#L463 It sounds to me that some clocks are not properly set for a random resolution, but fixing it is not trivial and requires deep knowledge about how the display registers should be tuned to better support resolutions. The current settings cause underflows with 1080p, which in turn makes the display driver to (silently) stop working. So, in summary, I believe that some drivers from my port will require being at staging for a while. While I was planning to do that on my next patch submission, placing the PM drivers there won't make much difference from my side, as I'll need to be submitting patches anyway moving drivers out of staging as they become ready. Thanks, Mauro _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel