Re: [PATCH 3/4] clk: renesas: r9a06g032: document structs

From: Ralph Siemsen <ralph.siemsen@linaro.org>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: linux-renesas-soc@vger.kernel.org, linux-clk@vger.kernel.org,
	Stephen Boyd <sboyd@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>
Subject: Re: [PATCH 3/4] clk: renesas: r9a06g032: document structs
Date: Wed, 1 Mar 2023 14:03:18 -0500	[thread overview]
Message-ID: <CANp-EDYOeZ3GWNQFpoLgRXj+6-O1Hk+DSr1hTGd=OhX=T9iHhQ@mail.gmail.com> (raw)
In-Reply-To: <20230301122430.4317530f@xps-13>

Hi Miquèl,

Thanks for reviewing. A few comments below.

On Wed, Mar 1, 2023 at 6:24 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> > +/**
> > + * struct r9a06g032_gate - clock gate control bits
> > + * @gate:   bit which enables/disables the clock
>
> Is this really a bit? I see below you explain what each field means
> in terms of offset/bitmask, so maybe we could be vague here,
> something like:
>
> "configuration to enable/disable the clock"

I'll drop the "bit" phrasing from all of them, to avoid confusion.

> > + * @reset:  bit which controls module reset (active low)
>
> "clock module reset" ?

Sounds good!

> > + * @ready:  bit which indicates device is ready for access
>
> "the clock is stacle and the device fed should be ready for access"
> (not sure about this one)

This one controls whether read/write requests are forwarded to the
device. I believe this is to prevent the AXI bus from hanging the
module clock is not enabled. I tried to explain that here:

> > + * Modules may also need to signal when the are @ready to
> > + * handle requests (read/writes) from the NoC interconnect.

I'll see if I can come up with a better description for both @ready and @midle.

> > + * @type:   see enum gate_type
> > + * @index:  the ID of this clock element
> > + * @source: the ID+1 of the parent clock element.
> > + *          Root clock uses ID of ~0 (PARENT_ID);
> > + * @gate:   describes the bits which control clock gate
>
> I would just refer to the structure above (like @type).
>
> No description of the following fields? :-) :-) It's okay, but while
> you're at it...

Ack, I will add the others too.

Ralph