From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A85F43FC2 for ; Tue, 31 Aug 2021 08:07:46 +0000 (UTC) Received: by mail.kernel.org (Postfix) with ESMTPSA id 34B64603E7; Tue, 31 Aug 2021 08:07:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1630397266; bh=z7P55UZRnlAxsdqAANQXJIQgoIdMDN7PXKO0gr+rm5M=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=q2vdWeLhaBBq6sEVc+B7bSwAyRp14UvYy8+aptxG9qQeEaexhWxicYWqcobw7kMX+ bj0yqnoB6ECi5yqLdEP7Baz+WzV1Km2pbmFaC7VO5BuDwp7A4Dk43tqqUZ8O/MA8Bd MaO2Ny3od4cBnW1L+hq5uJC840+68lG07ujDXV6FvuvVkGPwqwYB1ojYsy1H/w2tNi r1kmMKP2oKeDKs9ZSlmY/q3Mr+Po4nbanzvWf7+QKOrHLfZjsFI7bhtJRQpiwLkQMT RJ6vcg94nwwM8sSKTGyinTt0ISTQwKiAk95Gn0CBOZ0ivYMstet2jArn/uQffncUCZ CwF87DBWP/nZA== Received: from johan by xi.lan with local (Exim 4.94.2) (envelope-from ) id 1mKyny-0007AS-8H; Tue, 31 Aug 2021 10:07:39 +0200 Date: Tue, 31 Aug 2021 10:07:38 +0200 From: Johan Hovold To: Alex Elder Cc: Matthew Wilcox , Alex Elder , linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org, greybus-dev@lists.linaro.org, "Fabio M. De Francesco" Subject: Re: [greybus-dev] [PATCH v4] staging: greybus: Convert uart.c from IDR to XArray Message-ID: References: <20210829092250.25379-1-fmdefrancesco@gmail.com> <2879439.WEJLM9RBEh@localhost.localdomain> Precedence: bulk X-Mailing-List: linux-staging@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Mon, Aug 30, 2021 at 08:20:25AM -0500, Alex Elder wrote: > I have been offering review feedback on this patch for three reasons: > > - First, because I think the intended change does no real harm to the > Greybus code, and in a small way actually simplifies it. You leave out that we've already seen three versions of the patch that broke things in various ways and that there was still work to be done with respect to the commit message and verifying the locking. That's all real costs that someone needs to bear. > - Because I wanted to encourage Fabio's efforts to be part of the > Linux contributor community. Helping new contributers that for example have run into a bug or need some assistance adding a new feature that they themselves have use for is one thing. I'm not so sure we're helping either newcomers or Linux long term by inventing work for an already strained community however. [ This is more of a general comment and of course in no way a critique against Fabio or a claim that the XArray conversions are pointless. ] > - Because I suspected that Matthew's long-term intention was to > replace IDR/IDA use with XArray, so this would represent an early > conversion. That could be a valid motivation for the change indeed (since the efficiency arguments are irrelevant in this case), but I could not find any indications that there has been an agreement to deprecate the current interfaces. Last time around I think there was even push-back to convert IDR/IDA to use XArray internally instead (but I may misremember). > The Greybus code is generally good, but that doesn't mean it can't > evolve. It gets very little patch traffic, so I don't consider small > changes like this "useless churn." The Greybus code is held up as an > example of Linux kernel code that can be safely modified, and I think > it's actively promoted as a possible source of new developer tasks > (e.g. for Outreachy). Since most people can't really test their changes to Greybus perhaps it isn't the best example of code that can be safely modified. But yeah, parts of it are still in staging and, yes, staging has been promoted as place were some churn is accepted. Johan