Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Sender: james@equiv.tech
Date:   Fri, 5 May 2023 07:39:55 -0700
From:   James Seo <james@equiv.tech>
To:     Guenter Roeck <linux@roeck-us.net>
Cc:     James Seo <james@equiv.tech>, Jean Delvare <jdelvare@suse.com>,
        Jonathan Corbet <corbet@lwn.net>, linux-hwmon@vger.kernel.org,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC 02/11] hwmon: (core) Rename last parameter of
 devm_hwmon_register_with_info()
Message-ID: <ZFUVO4ECF0Xlcmx8@equiv.tech>
References: <20230504075752.1320967-1-james@equiv.tech>
 <20230504075752.1320967-3-james@equiv.tech>
 <ff6efe53-2173-1372-a1ca-c0928cf8a160@roeck-us.net>
 <ZFUBZyG5eoz2TpZX@equiv.tech>
 <ec28c2c2-4623-0d92-00bc-b8cd53c965b2@roeck-us.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <ec28c2c2-4623-0d92-00bc-b8cd53c965b2@roeck-us.net>
Precedence: bulk

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