Re: [RFC 02/11] hwmon: (core) Rename last parameter of devm_hwmon_register_with_info()

[James Seo]

On Fri, May 05, 2023 at 06:30:53AM -0700, Guenter Roeck wrote:
> On 5/5/23 06:15, James Seo wrote:
>> On Thu, May 04, 2023 at 08:29:57AM -0700, Guenter Roeck wrote:
>> 
>> Hello,
>> 
>> Of course arbitrarily changing variable names is pointless. But this
>> patch fulfills the intent of 848ba0a2f20dc121a3ef5272a24641d2bd963d8b,
>> which makes this change for devm_hwmon_device_register_with_info() in
>> hwmon-kernel-api.txt and in hwmon.h - but not in hwmon.c. The same
>> commit makes the same change for hwmon_device_register_with_info() in
>> all three files, so it obviously should have been in tree already.
>> 
>> The other reason for this patch is that for the purpose of generating
>> function documentation from kerneldocs, it is not feasible to call
>> this parameter "extra_groups" in the kerneldoc and still call it
>> "groups" in the function itself. Doing so results in the lines
>> "const struct attribute_group **groups / undescribed" and no mention
>> of "extra_groups" in the generated document. Leaving things as is, so
>> that [devm_]hwmon_device_register_with_info() have different names
>> for this parameter, is potentially confusing and more noticeable to
>> to the eye than I would like once rendered.
>> 
>> Is this good enough to proceed? And as a subsystem maintainer, is
>> there anything else, specifically or in general, that you would like
> 
> Marginally. That should have been explained in more detail in the
> description.

OK, I will add more detail.

> 
>> to see addressed?
>> 
> 
> I don't know. There are changes which seem to be more based on POV
> than real improvement (such as the removal of the credit from the
> PMBus document). I'll have to verify each and every reference to determine
> if the change is appropriate. Also, the changes are substantial.

Yes, sorry. At some point comparing a local make htmldocs build to
docs.kernel.org became much easier to follow than slogging through
diffs, and some of the markup only makes sense once rendered next to
the automatic cross-references that Sphinx adds.

> It will take a lot of time for me to review, and right now I do not have
> that time. I have a hard time keeping up with code reviews.
> 
> Guenter
> 

No rush. Your time is always appreciated. As I have gathered some
feedback from Bagas, I will submit the smaller changes as a PATCH
series in a day or two and an updated RFC series that you can
tackle at your leisure.

James