Re: [PATCH RESEND v10 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



Hi Lee,

> On Apr 17, 2024, at 8:14 AM, Lee Jones <lee@xxxxxxxxxx> wrote:
> 
> On Tue, 16 Apr 2024, James Ogletree wrote:
> 
>>> On Apr 16, 2024, at 7:35 AM, Lee Jones <lee@xxxxxxxxxx> wrote:
>>> 
>>> On Fri, 12 Apr 2024, James Ogletree wrote:
>>>> All feedback to date has been addressed
>>> 
>>> There are still MFD review comments left outstanding.
>> 
>> Hi Lee,
>> 
>> As best as I can tell, the only review comment in v10 is from you,
>> saying that the MFD part looks okay. Then in this resent series, you
>> commented that the MFD part had not been updated. Indeed, this is
>> because there were no comments to address. Please correct me if I
>> interpreted your comment incorrectly.
>> 
>> I have also gone back over v8 and v9 looking for outstanding
>> comments, and all of the review comments appear to be applied.
>> Would you kindly link me to the email with the missing review
>> comments if I am mistaken?
> 
> At the very least, you still have this in here:
> 
>    /* If write never occurred, don't bother polling for ACK */
> 
> After realising that I'd commented on this before, I stopped the
> re-review.

It looks like I took some liberties here without communicating them,
so my sincere apologies.

In V8 the line was:

"If we never wrote, don't bother checking for ACK”

In that review, you provided an improved phrasing:

"If the write never took place, no need to check for the ACK”

My takeaway from your phrasing was that the use of the first-person
pronoun “we" was improper. So I rewrote it in the third-person, but
still in my own words, resulting in its current form:

"If write never occurred, don't bother polling for ACK”

Is it that the phrase “don’t bother” is too idiomatic? If so I will apply
your phrasing verbatim in V11.

Best,
James






[Index of Archives]     [Pulseaudio]     [Linux Audio Users]     [ALSA Devel]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux