Received: by 10.192.165.148 with SMTP id m20csp1846930imm; Thu, 26 Apr 2018 03:12:03 -0700 (PDT) X-Google-Smtp-Source: AIpwx48GEyHVyZGXz1ES8qfSAf3k2Wb1wxN/efiyeqGr5rncvO+qIebMA3nAX3kmEquGUIMogWW9 X-Received: by 2002:a17:902:bf41:: with SMTP id u1-v6mr28987284pls.257.1524737523881; Thu, 26 Apr 2018 03:12:03 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1524737523; cv=none; d=google.com; s=arc-20160816; b=tCnt4+MQbTmZCEwbxayOBghv2OX1hXjJ05GdA+OI8Lh/4ODm6MjsP/iaZWgW1IDo0Z UvwsE3LZiABTI6CLBspR2fKrTnjOd2afhECtbEI+CB82iXZEjzgzGb8x0md0N6lyJ7sp tM4P2yAihpPurkzjSmx22oVVekB1EKSPKpS1SMzKZMP50M/0Vbhw2MpRzvAHpa3F07W6 zwQ7NPYxp1/5a6LnWza0UX9fnSy7Vrwl0Buc9hkQLigtYRcNayAoopUTnOW8nUulBD2+ /2wHKLbOcbDcB1LMtEWhVr9kroMvAqyoBOiuReA59z68HT6h6eVZ8S+ciW4w1lplJ/OS i2wg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:to:references:message-id :content-transfer-encoding:cc:date:in-reply-to:from:subject :mime-version:dkim-signature:arc-authentication-results; bh=4GRgm9B9IE3SqabkrzxaIUAQ7vkAx+RX5hUwKd1EQbE=; b=ly6cq0iBGbDvL1y2xXr/DabmiRjfu03hEJrRegS7pW72YzYmGxX3aMPNWenYeqlMl3 YRrSm/aWg44uRjOIhyysK08jokkMlt0GSd7/qElLo9ue9PelxT81TEG/2+EXjPqgkuLL kC+D6iSJZiEZMLrRdKlFyfUUqRQ6XnX0cKpxk5bYwQnSfkZC0/DhnK64bi/ui2xiEsnq lgly12lOT42YIjjtxj9AbG8qR74Oe0IU8mpDBkOMTbNqAND0gpdRFbgZPPXLuBUmg7oT 9Nw8CFLOzAj0G2xnbjFSnrlSG/Ep7l5SWDr+uOl9ZjcNmkk81QDSym+XreWrN1yTDzX1 WM8A== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@goldelico.com header.s=strato-dkim-0002 header.b=IA9R3jBX; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id t5si9705303pgp.594.2018.04.26.03.11.49; Thu, 26 Apr 2018 03:12:03 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=fail header.i=@goldelico.com header.s=strato-dkim-0002 header.b=IA9R3jBX; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754842AbeDZKKQ (ORCPT + 99 others); Thu, 26 Apr 2018 06:10:16 -0400 Received: from mo4-p01-ob.smtp.rzone.de ([81.169.146.167]:31116 "EHLO mo4-p01-ob.smtp.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753879AbeDZKKN (ORCPT ); Thu, 26 Apr 2018 06:10:13 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1524737411; s=strato-dkim-0002; d=goldelico.com; h=To:References:Message-Id:Content-Transfer-Encoding:Cc:Date: In-Reply-To:From:Subject:Content-Type:X-RZG-CLASS-ID:X-RZG-AUTH:From: Subject:Sender; bh=4GRgm9B9IE3SqabkrzxaIUAQ7vkAx+RX5hUwKd1EQbE=; b=IA9R3jBXA60z3Utx9Xo3F7SjP//7ABy+Q5voxTBxqv6MugoIKmyfe5FJoa3WrDMEm1 irupW+a/9U/X52jKBsAtqZe2o7zQjizkLu8BXKxL22T7AmfbHrOFK09CeJqbmCj11I2h qgXFk3oZWQexHrx6tk+mc1K7yJnEgldhk5/SRybVFB1/TS34jG0Im/AZkgALl0duJN3C 47ZokKmLWyDmpFsTxejNUJSCpu4wVlHGqRtJSadLetPPtbAK0WsFWzIZky8phVCugn1D KP+HB4+kbjNHj7WALqyk5M89MLGiqbCha0xLb7Ktv32vPWaxlZ2aFR/zGyYwz5rOszIB 5Y2w== X-RZG-AUTH: :JGIXVUS7cutRB/49FwqZ7WcJeFKiMgPgp8VKxflSZ1P34KBj4Qpw87WisdN1GTaY X-RZG-CLASS-ID: mo00 Received: from [192.168.2.107] by smtp.strato.de (RZmta 43.5 DYNA|AUTH) with ESMTPSA id z0512cu3QAA33Gp (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (curve secp521r1 with 521 ECDH bits, eq. 15360 bits RSA)) (Client did not present a certificate); Thu, 26 Apr 2018 12:10:03 +0200 (CEST) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\)) Subject: Re: [PATCH 0/7] gnss: add new GNSS subsystem From: "H. Nikolaus Schaller" In-Reply-To: <20180425081120.GL4615@localhost> Date: Thu, 26 Apr 2018 12:10:02 +0200 Cc: Greg Kroah-Hartman , Rob Herring , Mark Rutland , Andreas Kemnade , Arnd Bergmann , Pavel Machek , Linux Kernel Mailing List , Discussions about the Letux Kernel , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" Content-Transfer-Encoding: quoted-printable Message-Id: <7A11E17A-64B7-4B39-87EC-D6A52773850B@goldelico.com> References: <20180424163458.11947-1-johan@kernel.org> <31CF06C6-D6ED-4930-8D81-12256A518059@goldelico.com> <20180424175050.GG4615@localhost> <8F4FAF5B-AAA9-4D46-A022-99B96C74ABFA@goldelico.com> <20180425081120.GL4615@localhost> To: Johan Hovold X-Mailer: Apple Mail (2.3124) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Johan, > Am 25.04.2018 um 10:11 schrieb Johan Hovold : >=20 > On Tue, Apr 24, 2018 at 09:44:08PM +0200, H. Nikolaus Schaller wrote: >=20 >>> Am 24.04.2018 um 19:50 schrieb Johan Hovold : >=20 >>> I think it should be done the other way round (if I understand you >>> correctly), that is, by adding support for configurations were = WAKEUP is >>> left not connected to the sirf driver instead. >>=20 >> Hm. Yes, the w2sg00x4 is a Sirf based chip. >>=20 >>> I had that use-case in mind when implementing >>=20 >> s/implementing/reinventing/ >>=20 >>> the driver, and some ideas of how it should be >>> done, but did not get around to actually implement it yet. >>=20 >> What do you need ideas for? We have that function working and >> submitted year after year, but it was always rejected for API >> reasons. >>=20 >> You could have simply reused what we have proposed [1] and just >> adapt it to the new API instead of writing a new driver (which >> is missing some features for us). >=20 > Your code was broken or needed to be updated in several ways as I > pointed out in the thread you refer to. No, it was not broken (it is in daily use and no practical problems are known), but surely does not cover some corner-cases (e.g. races). I said we should focus firstly on the "abstraction level". So I postponed these code issues from discussion. In the end you announced to provide a new kernel API which would make most of the code (all tty stuff) and related problems go away anyways. Therefore I simply did wait for this API to come, before fixing the remaining issues and submitting a new v6. > It also did not support all those systems that use the same family of > chips, but which has the WAKEUP signal connected. That is simply wrong. It supports them. It just does not make use of the WAKEUP signal, but uses a different mechanism to detect the power state of the chip. So our driver is more complete because it gracefully handles a WAKEUP line (by ignoring it) while yours doesn't handle the opposite case of a missing WAKEUP. >=20 >> "proof-of-concept" is misleading if you expect this to become >> *the* Sirf driver and we are just invited to add some features >> to that. Making our own work and proposals completely obsolete. >>=20 >> What I find really strange and foul play is that we are in the >> review process and then comes a hidden counter-proposal by the >> reviewer. >=20 > Dude, in the very same thread you refer to above, after being asked to > reiterate your proposal to find and appropriate abstraction level you > reply: >=20 > "Yes, please feel free to write patches that implement it that > way." When looking back, I understand that as that I suggested to add a = "proper abstraction level" (i.e. a gps framework) so that *we* can make use of it. I never meant that you should completely replace our driver by an = incomplete solution. >=20 > Now I've done just that for you, and then you whine about that too. At the end of the thread (which I take as the result of v5 review) you = said: "Yeah, I think this is a dead end. We need some kind of gps framework with drivers that can implement the device specific bits. I may have some time to look at little closer at it this week." And I answered and laid down my plans for a v6: "Ok, that would be fine if we can get that! For a minimal set of API I think something like this (following hci_dev) = would suffice: ... If that would wrap all creation of some /dev/ttyGPS0 (or however it is = called), it would fit our needs for a driver and user-space for our system. And I would be happy to get rid of creating and registering a = /dev/ttyGPS0 in the w2sg0004 driver. Then, the driver will not need to be touched if the GPS framework is = improved in some far future (e.g. to provide some additional ioctl for getting = kalman-filtered position+heading by doing sensor fusion with some iio-based = accelerometer/gyro).=20 So I am looking forward to some framework for review and integration = testing." I think it was clear and understandable what I expected (a new framework = instead of creating ttys in the driver) and what I did not (a replacement of our w2sg0004 proposal with a variant with missing features). The problem I have with this sort of review process is that I now have = to reintegrate the missing pieces of handling the non-WAKEUP case and LNA control and rfkill into your code. Instead of stripping down our code and adjusting to your API. Alternatively I have to wait until you present some code of which you think it works. Would you also test it? Probably no. Because of lack of = hardware. So I have to test your new code in an area where I have well-hung code = being in daily use. And your new code may still be incomplete wrt. what our driver provides = to users. I hope that you understand that this is now more work for me in any = case... So in the end I will have: * a new but still incomplete driver that is a regression compared to our = old driver * even more testing work in addition 5 years urging for acceptance * no appreciation because you sign off and author everything Is this a good deal or a reason to contribute to our common project? If you want an example how to get rid of volunteers contributing, this = may be one... >=20 > SiRF is a very common chip and I wanted to make sure that the common > setup with WAKEUP connected was supported from the start. I'll get to > your configuration in time too. As said, our driver would also work if the WAKEUP line exists, and is described in DT, but is broken. So we will probably stay with our out-of the kernel driver for the time = being. BR, Nikolaus > [1]: https://patchwork.ozlabs.org/cover/843392/