public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Sam Edwards <cfsworks@gmail.com>
To: Andre Przywara <andre.przywara@arm.com>
Cc: u-boot@lists.denx.de, Jagan Teki <jagan@amarulasolutions.com>,
	Marek Vasut <marex@denx.de>
Subject: Re: [PATCH 2/2] usb: musb-new: sunxi: clarify the purpose of SRAM initialization
Date: Fri, 9 Jun 2023 13:16:00 -0600	[thread overview]
Message-ID: <8a921ccb-cdc3-80a0-d155-708e908fd712@gmail.com> (raw)
In-Reply-To: <20230609111330.1f3f59a9@donnerap.cambridge.arm.com>

Hi Andre,

I've applied most of this feedback (most of which comes as a relief; I 
dislike inventing names for mystery bits) in preparation to send a v2, 
but had two questions:

On 6/9/23 04:13, Andre Przywara wrote:
>> The new comments and function name are not from any official source,
>> but are updated to mirror how the Linux kernel sources treat this
>> mystery magic bit. If we wanted to confirm that this speculation is
>> correct, we could verify that SRAM-D is inaccessible whenever the
>> bit is set, and then try clearing it again while the MUSB is in use
>> to see what debris gets left behind in SRAM-D.
>>
>> This cleanup also adds a TODO comment about runtime discovery
>> of the SYSCON base, per discussion with Andre.
> 
> thanks for sending this, looks good. Some stylistic comments below.

I take it that this (in combination with your review on 1/2) means you 
concur with my speculated purpose of the mystery bit. If so, I'd like to 
rephrase the above paragraph in the commit message to:

"""
The new comments and function name are not from any official source,
but are updated to mirror how the Linux kernel sources treat this
mystery magic bit. This also reflects what's been observed on actual
hardware: SRAM-D is inaccessible by the CPU when the bit is set, and
the MUSB unit crashes when this bit is cleared while USB is in use,
leaving behind debris in SRAM-D from its use as a "scratch space."
"""

Does this accurately reflect what you've seen, particularly (especially) 
that last line, or should I end the commentary at "is in use."?

> I think we should use "non-net" commenting style, with the "/*" on a line
> on its own.

This seems to be an obscure term of art, and searching "non-net comment" 
and "non-net style" on Google isn't finding me any style guide or set of 
rules to check against. (Amusingly: if I search for the "net" style, I 
get a lot of .NET suggestions.)

The main thing I'm trying to figure out is if it also demands */ on its 
own line, which I would intuitively think makes sense (it does look 
better to me that way), but I'm unsure if your lack of critique at the 
closing side of my multiline comments means you would prefer:
/*
  * A block of text that's long enough to become a
  * multiline comment and ends up looking like this */

Thanks for your quick and thorough feedback,
Sam

  reply	other threads:[~2023-06-09 19:16 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-07 23:16 [PATCH 0/2] sunxi, usb: Clean up SRAM initialization code Sam Edwards
2023-06-07 23:16 ` [PATCH 1/2] usb: musb-new: sunxi: only perform SRAM initialization when necessary Sam Edwards
2023-06-08 12:03   ` Andre Przywara
2023-06-09 10:00   ` Andre Przywara
2023-06-07 23:16 ` [PATCH 2/2] usb: musb-new: sunxi: clarify the purpose of SRAM initialization Sam Edwards
2023-06-09 10:13   ` Andre Przywara
2023-06-09 19:16     ` Sam Edwards [this message]
2023-06-09 19:40       ` Andre Przywara

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=8a921ccb-cdc3-80a0-d155-708e908fd712@gmail.com \
    --to=cfsworks@gmail.com \
    --cc=andre.przywara@arm.com \
    --cc=jagan@amarulasolutions.com \
    --cc=marex@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox