Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932497AbdLGNXL (ORCPT ); Thu, 7 Dec 2017 08:23:11 -0500 Received: from mail-wr0-f172.google.com ([209.85.128.172]:37066 "EHLO mail-wr0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932451AbdLGNXJ (ORCPT ); Thu, 7 Dec 2017 08:23:09 -0500 X-Google-Smtp-Source: AGs4zMYO7XHY+gsU1Vu84bNd25Fx+ShKNa2vKpbY0mkmn1n9K2EU6pgZKVUzS5Y5IEgmWAgLXvEuKlmkYzsbiJdp0jU= MIME-Version: 1.0 In-Reply-To: References: From: Philippe Ombredanne Date: Thu, 7 Dec 2017 14:22:27 +0100 Message-ID: Subject: Re: [PATCH v9 4/4] [media] platform: Add Synopsys DesignWare HDMI RX Controller Driver To: Jose Abreu Cc: Hans Verkuil , Linux Media Mailing List , LKML , Joao Pinto , Mauro Carvalho Chehab , Hans Verkuil , Sylwester Nawrocki , Sakari Ailus Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id vB7DNuF4001837 Content-Length: 5381 Lines: 125 Jose, On Thu, Dec 7, 2017 at 1:33 PM, Hans Verkuil wrote: > Hi Jose, > > Some (small) comments below: > > On 12/07/17 10:47, Jose Abreu wrote: >> This is an initial submission for the Synopsys DesignWare HDMI RX >> Controller Driver. This driver interacts with a phy driver so that >> a communication between them is created and a video pipeline is >> configured. >> >> The controller + phy pipeline can then be integrated into a fully >> featured system that can be able to receive video up to 4k@60Hz >> with deep color 48bit RGB, depending on the platform. Although, >> this initial version does not yet handle deep color modes. >> >> This driver was implemented as a standard V4L2 subdevice and its >> main features are: >> - Internal state machine that reconfigures phy until the >> video is not stable >> - JTAG communication with phy >> - Inter-module communication with phy driver >> - Debug write/read ioctls >> >> Some notes: >> - RX sense controller (cable connection/disconnection) must >> be handled by the platform wrapper as this is not integrated >> into the controller RTL >> - The same goes for EDID ROM's >> - ZCAL calibration is needed only in FPGA platforms, in ASIC >> this is not needed >> - The state machine is not an ideal solution as it creates a >> kthread but it is needed because some sources might not be >> very stable at sending the video (i.e. we must react >> accordingly). >> >> Signed-off-by: Jose Abreu >> Cc: Joao Pinto >> Cc: Mauro Carvalho Chehab >> Cc: Hans Verkuil >> Cc: Sylwester Nawrocki >> Cc: Sakari Ailus [] >> --- /dev/null >> +++ b/drivers/media/platform/dwc/dw-hdmi-rx.h >> @@ -0,0 +1,441 @@ >> +/* >> + * Synopsys Designware HDMI Receiver controller driver >> + * >> + * This Synopsys dw-hdmi-rx software and associated documentation >> + * (hereinafter the "Software") is an unsupported proprietary work of >> + * Synopsys, Inc. unless otherwise expressly agreed to in writing between >> + * Synopsys and you. The Software IS NOT an item of Licensed Software or a >> + * Licensed Product under any End User Software License Agreement or >> + * Agreement for Licensed Products with Synopsys or any supplement thereto. >> + * Synopsys is a registered trademark of Synopsys, Inc. Other names included >> + * in the SOFTWARE may be the trademarks of their respective owners. >> + * >> + * The contents of this file are dual-licensed; you may select either version 2 >> + * of the GNU General Public License (“GPL”) or the MIT license (“MIT”). >> + * >> + * Copyright (c) 2017 Synopsys, Inc. and/or its affiliates. >> + * >> + * THIS SOFTWARE IS PROVIDED "AS IS" WITHOUT WARRANTY OF ANY KIND, EXPRESS OR >> + * IMPLIED, INCLUDING, BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE, AND NONINFRINGEMENT. IN NO EVENT SHALL THE >> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR >> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT, OR OTHERWISE >> + * ARISING FROM, OUT OF, OR IN CONNECTION WITH THE SOFTWARE THE USE OR >> + * OTHER DEALINGS IN THE SOFTWARE. >> + */ My heart bleeds when I see such a long legalese crap obstructing your otherwise fine code contributions. Would you be kind enough to consider using the new SPDX ids as documented by Thomas and commented and agreed by Linus, Greg and other maintainers? This has the same effect, but is much more concise and readable. This could come out this way in this example: >> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */ >> +/* >> + * Synopsys Designware HDMI Receiver controller driver >> + * >> + * Copyright (c) 2017 Synopsys, Inc. and/or its affiliates. >> + * >> + */ Or even better using C++ /// style comments if this works for your code. These are preferred by Linus for such things (and requested for .c files SPDX ids vs. .h includes): >> +// SPDX-License-Identifier: (GPL-2.0 OR MIT) >> +// Copyright (c) 2017 Synopsys, Inc. and/or its affiliates. >> +// Synopsys Designware HDMI Receiver controller driver Think of it this way: unless you are a legalese lover or a lawyer-who-codes and confused the kernel with a licensing contract draft, you now have improved the signal/noise ratio of your code quite nicely by removing about 20 lines of comments. And if someone ever prints your code, you will have saved a tree or two and be a good earth citizen. And even better, you can now grep your code for licenses, unambiguously. If you could do this that would be really nice: we already have tagged ~13K files with SPDX and they are now in Linus's tree. We still have roughly 60K files to do... so every little help that would avoid piling up more work for us with new and innovative legalese boilerplate would really be much appreciated. Extra bonus: you can also do this for all your past, present and future contributions.... and spread the good word at your company so that everyone does the same: with this small thing you will earn at least 10 or more extra good karma points and my eternal gratitude. Thank you for your kind consideration, your friendly kernel licensing janitor! -- Cordially Philippe Ombredanne