LKML Archive on
help / color / mirror / Atom feed
From: Alex Elder <>
To: Johan Hovold <>
Cc: Matthew Wilcox <>,
	Alex Elder <>,,,,
	"Fabio M. De Francesco" <>
Subject: Re: [greybus-dev] [PATCH v4] staging: greybus: Convert uart.c from IDR to XArray
Date: Tue, 31 Aug 2021 05:42:20 -0500	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <YS3jSsGSs0yAw/>

On 8/31/21 3:07 AM, Johan Hovold wrote:
> 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.

This is true.  But it's separate from my reason for doing it,
and unrelated to the suggested change.

>> - 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. ]

Yes, yours is a general comment.  But I would characterize
this as Fabio "scratching an itch" rather than "inventing
new work."  The strained community needs more helpers, and
they don't suddenly appear fully-formed; they need to be
cultivated.  There's a balance to strike between "I see
you need a little guidance here" and "go away and come
back when you know how to do it right."

In any case, I don't disagree with your general point, but
we seem to view this particular instance differently.

>> - 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.

Testing Greybus code is a problem.  *That* would be something useful
for people to help fix.


> Johan

  reply	other threads:[~2021-08-31 10:42 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-29  9:22 Fabio M. De Francesco
2021-08-30  9:12 ` Johan Hovold
2021-08-30 11:10   ` Fabio M. De Francesco
2021-08-30 11:52     ` Johan Hovold
2021-08-30 12:16       ` Matthew Wilcox
2021-08-30 12:33         ` Johan Hovold
2021-08-30 13:16           ` Fabio M. De Francesco
2021-08-30 13:20           ` [greybus-dev] " Alex Elder
2021-08-31  8:07             ` Johan Hovold
2021-08-31 10:42               ` Alex Elder [this message]
2021-08-31 11:51                 ` Johan Hovold
2021-08-31 11:50               ` Fabio M. De Francesco
2021-08-31 12:18                 ` Johan Hovold
2021-09-01 12:09                 ` Alex Elder
2021-09-01 13:56                   ` Fabio M. De Francesco
2021-09-01 14:29                     ` Matthew Wilcox
2021-09-01 15:39                       ` Fabio M. De Francesco
2021-08-30 13:31           ` Matthew Wilcox
2021-08-31  8:16             ` Johan Hovold

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \
    --subject='Re: [greybus-dev] [PATCH v4] staging: greybus: Convert uart.c from IDR to XArray' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).