2022-04-26 13:02:18

by Zev Weiss

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] hwmon: (nct6775) Convert to regmap, add i2c support

[Adding korg webmaster re: list infrastructure]

On Tue, Apr 26, 2022 at 12:18:42AM PDT, Zev Weiss wrote:
>Hello,
>
>This is v3 of my effort to add i2c support to the nct6775 hwmon
>driver.
>
>Changes since v2 [0]:
> ...
> - Renamed drivers and Kconfig symbols to keep existing platform
> driver as "nct6775" (SENSORS_NCT6775) and the core module as
> "nct6775-core" (SENSORS_NCT6775_CORE) [Guenter]

Unfortunately while this was a simple enough change to make (a few 'git
mv' commands and a handful of actual text changes), it ballooned the
size of the diff for patch 5 to the point that vger bounced it for
exceeding the 100K message-size limit. As far as I can tell it looks
like it went through elsewhere, but does leave a bit of a gap in the
public list archives -- please let me know if there's anything I should
try in terms of re-sending it. (The only combination of 'git
format-patch' flags I've been able to find that gets it back down to
approximately its previous size is '-B -D', which isn't so useful for
actually applying.)

I'm not sure how critical a limit that 100K is, or if it's something we
might consider raising a bit?


Thanks,
Zev


2022-04-26 14:05:25

by Joel Stanley

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] hwmon: (nct6775) Convert to regmap, add i2c support

On Tue, 26 Apr 2022 at 08:29, Zev Weiss <[email protected]> wrote:
>
> [Adding korg webmaster re: list infrastructure]
>
> On Tue, Apr 26, 2022 at 12:18:42AM PDT, Zev Weiss wrote:
> >Hello,
> >
> >This is v3 of my effort to add i2c support to the nct6775 hwmon
> >driver.
> >
> >Changes since v2 [0]:
> > ...
> > - Renamed drivers and Kconfig symbols to keep existing platform
> > driver as "nct6775" (SENSORS_NCT6775) and the core module as
> > "nct6775-core" (SENSORS_NCT6775_CORE) [Guenter]
>
> Unfortunately while this was a simple enough change to make (a few 'git
> mv' commands and a handful of actual text changes), it ballooned the
> size of the diff for patch 5 to the point that vger bounced it for
> exceeding the 100K message-size limit. As far as I can tell it looks
> like it went through elsewhere, but does leave a bit of a gap in the
> public list archives -- please let me know if there's anything I should
> try in terms of re-sending it. (The only combination of 'git
> format-patch' flags I've been able to find that gets it back down to
> approximately its previous size is '-B -D', which isn't so useful for
> actually applying.)

FWIW, I moderated it through to the openbmc list, which is on lore:

https://lore.kernel.org/openbmc/[email protected]/

So the series can be fetched with eg. b4.

Aside from the mega-diff in patch 5 the changes look good to me (If
you can think of a way that makes patch 5 easier to review then let me
know).

Reviewed-by: Joel Stanley <[email protected]>

>
> I'm not sure how critical a limit that 100K is, or if it's something we
> might consider raising a bit?
>
>
> Thanks,
> Zev
>

2022-04-27 10:08:50

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] hwmon: (nct6775) Convert to regmap, add i2c support

On 4/26/22 01:29, Zev Weiss wrote:
> [Adding korg webmaster re: list infrastructure]
>
> On Tue, Apr 26, 2022 at 12:18:42AM PDT, Zev Weiss wrote:
>> Hello,
>>
>> This is v3 of my effort to add i2c support to the nct6775 hwmon
>> driver.
>>
>> Changes since v2 [0]:
>> ...
>> - Renamed drivers and Kconfig symbols to keep existing platform
>>   driver as "nct6775" (SENSORS_NCT6775) and the core module as
>>   "nct6775-core" (SENSORS_NCT6775_CORE) [Guenter]
>
> Unfortunately while this was a simple enough change to make (a few 'git mv' commands and a handful of actual text changes), it ballooned the size of the diff for patch 5 to the point that vger bounced it for exceeding the 100K message-size limit.  As far as I can tell it looks like it went through elsewhere, but does leave a bit of a gap in the public list archives -- please let me know if there's anything I should try in terms of re-sending it.  (The only combination of 'git format-patch' flags I've been able to find that gets it back down to approximately its previous size is '-B -D', which isn't so useful for actually applying.)
>
> I'm not sure how critical a limit that 100K is, or if it's something we might consider raising a bit?
>

You could split it up further. For example, you could introduce
the include file first. Also, please run checkpatch --strict on
your patches. I don't care about commenting the mutex, but there
should be no double empty lines. Also, while you are at it,
it would be great if you can add another patch to fix the

WARNING: Symbolic permissions 'S_IWUSR | S_IRUGO' are not preferred. Consider using octal permissions '0644'.

warnings.

Anyway, I wondered what changed... oh, I see. You didn't rename
nct6775.c. I didn't suggest that. The new file name was fine.
I only asked you to change the Kconfig symbols, not the file names.
It is ok and actually desirable to change the file names.
The platform driver module can and should still be named nct6775;
that can be handled in the Makefile with something like

nct6775-objs := nct6775-platform.o
obj-$(CONFIG_SENSORS_NCT6775) += nct6775.o

Guenter