Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755008AbcJ3XC7 (ORCPT ); Sun, 30 Oct 2016 19:02:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49710 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752800AbcJ3XC5 (ORCPT ); Sun, 30 Oct 2016 19:02:57 -0400 From: Jes Sorensen To: John Heenan Cc: Kalle Valo , linux-wireless@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] rtl8xxxu: Fix for bogus data used to determine macpower References: <20161030102112.GA5789@cube> Date: Sun, 30 Oct 2016 19:02:56 -0400 In-Reply-To: (John Heenan's message of "Sun, 30 Oct 2016 23:56:59 +1000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Sun, 30 Oct 2016 23:02:57 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2905 Lines: 66 John Heenan writes: > Thanks for your reply. > > The code was tested on a Cube i9 which has an internal rtl8723bu. > > No other devices were tested. > > I am happy to accept in an ideal context hard coding macpower is > undesirable, the comment is undesirable and it is wrong to assume the > issue is not unique to the rtl8723bu. > > Your reply is idealistic. What can I do now? I should of course have > factored out other untested devices in my patches. The apparent > concern you have with process over outcome is a useful lesson. > > We are not in an ideal situation. The comment is of course relevant > and useful to starting a process to fixing a real bug I do not have > sufficient information to refine any further for and others do. In the > circumstances nothing really more can be expected. Well you should start by reporting the issue and either providing a patch that only affects 8723bu, or work on a generic solution. I appreciate patches, but I do not appreciate patches that will make something work for one person and break for everyone else - I spent a lot of time making sure the driver works across the different devices. The comment violates all Linux standards - first rule when modifying code is to respect the style of the code you are dealing with. Code is 80 characters wide, and comments are /* */ never the ugly C++ crap. > My patch cover letter, [PATCH 0/2] provides evidence of a mess with > regard to determining macpower for the rtl8723bu and what is > subsequently required. This is important. > > The kernel driver code is very poorly documented and there is not a > single source reference to device documentation. For example macpower > is noting more than a setting that is true or false according to > whether a read of a particular register return 0xef or not. Such value > was never obtained so a full init sequence was never performed. The kernel driver is documented with the information I have - there is NO device documentation because Realtek refuses to provide any. I have written the driver based on what I have retried by reading the vendor drivers. If you can provide better documentation, I certainly would love to get it. > It would be helpful if you could provide a link to device references. > As it is, how am I supposed to revise the patch without relevant > information? Look at the USB device table, it shows you which devices are supported. > My patch code works with the Cube i9, as is, despite a lack of > adequate information. Before it did not. That is a powerful statement The driver works with a lot of different devices in itself that is a powerful statement! Yes I want to see it work with as many devices as possible, but just moving things around without balancing it and not explaining why is not a fix. If we move more of the init sequence to _start() you also have to move matching pieces to _stop(). Jes