Hi Miquèl, Thanks for reviewing. A few comments below. On Wed, Mar 1, 2023 at 6:24 AM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> 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
