Fwd: [PATCH] hwmon: nct6775: Add missing voltage reading NCT6796D

Fwd: [PATCH] hwmon: nct6775: Add missing voltage reading NCT6796D

[Henk]

Retry with sendmail  hope this helps...

---

Noticed in the documentation that the NCT6796D actually has 16 analog
voltage inputs, where only 15 are configured.

See: https://www.nuvoton.com/resource-files/NCT6796D_Datasheet_V0_6.pdf
page 59-60:

8.6.2.1. Voltage Reading
NCT6796D has 16 voltage reading:
....
and the missing one in the driver:
VIN9: Voltage reading Bank 4, Index 8F

This patch adds support for in15 on the NCT6796D.
---
 drivers/hwmon/nct6775.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/hwmon/nct6775.c b/drivers/hwmon/nct6775.c
index 5bd15622a85f..385ef79d79c4 100644
--- a/drivers/hwmon/nct6775.c
+++ b/drivers/hwmon/nct6775.c
@@ -28,7 +28,7 @@
  * nct6792d    15      6       6       2+6    0xc910 0xc1    0x5ca3
  * nct6793d    15      6       6       2+6    0xd120 0xc1    0x5ca3
  * nct6795d    14      6       6       2+6    0xd350 0xc1    0x5ca3
- * nct6796d    14      7       7       2+6    0xd420 0xc1    0x5ca3
+ * nct6796d    16      7       7       2+6    0xd420 0xc1    0x5ca3
  * nct6797d    14      7       7       2+6    0xd450 0xc1    0x5ca3
  *                                           (0xd451)
  * nct6798d    14      7       7       2+6    0xd428 0xc1    0x5ca3
@@ -209,14 +209,14 @@ superio_exit(int ioreg)

 /* Common and NCT6775 specific data */

-/* Voltage min/max registers for nr=7..14 are in bank 5 */
+/* Voltage min/max registers for nr=7..15 are in bank 5 */

 static const u16 NCT6775_REG_IN_MAX[] = {
        0x2b, 0x2d, 0x2f, 0x31, 0x33, 0x35, 0x37, 0x554, 0x556, 0x558, 0x55a,
-       0x55c, 0x55e, 0x560, 0x562 };
+       0x55c, 0x55e, 0x560, 0x562, 0x564 };
 static const u16 NCT6775_REG_IN_MIN[] = {
        0x2c, 0x2e, 0x30, 0x32, 0x34, 0x36, 0x38, 0x555, 0x557, 0x559, 0x55b,
-       0x55d, 0x55f, 0x561, 0x563 };
+       0x55d, 0x55f, 0x561, 0x563, 0x565 };
 static const u16 NCT6775_REG_IN[] = {
        0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x550, 0x551, 0x552
 };
@@ -478,7 +478,7 @@ static const u16 NCT6776_REG_TEMP_CRIT[32] = {

 static const u16 NCT6779_REG_IN[] = {
        0x480, 0x481, 0x482, 0x483, 0x484, 0x485, 0x486, 0x487,
-       0x488, 0x489, 0x48a, 0x48b, 0x48c, 0x48d, 0x48e };
+       0x488, 0x489, 0x48a, 0x48b, 0x48c, 0x48d, 0x48e, 0x48f };

 static const u16 NCT6779_REG_ALARM[NUM_REG_ALARM] = {
        0x459, 0x45A, 0x45B, 0x568 };
@@ -1047,9 +1047,9 @@ div_from_reg(u8 reg)
  * Some of the voltage inputs have internal scaling, the tables below
  * contain 8 (the ADC LSB in mV) * scaling factor * 100
  */
-static const u16 scale_in[15] = {
+static const u16 scale_in[16] = {
        800, 800, 1600, 1600, 800, 800, 800, 1600, 1600, 800, 800, 800, 800,
-       800, 800
+       800, 800, 800
 };

 static inline long in_from_reg(u8 reg, u8 nr)
@@ -1143,7 +1143,7 @@ struct nct6775_data {
        /* Register values */
        u8 bank;                /* current register bank */
        u8 in_num;              /* number of in inputs we have */
-       u8 in[15][3];           /* [0]=in, [1]=in_max, [2]=in_min */
+       u8 in[16][3];           /* [0]=in, [1]=in_max, [2]=in_min */
        unsigned int rpm[NUM_FAN];
        u16 fan_min[NUM_FAN];
        u8 fan_pulses[NUM_FAN];
@@ -4200,7 +4200,7 @@ static int nct6775_probe(struct platform_device *pdev)
        case nct6796:
        case nct6797:
        case nct6798:
-               data->in_num = 15;
+               data->in_num = (data->kind == nct6796) ? 16 : 15;
                data->pwm_num = (data->kind == nct6796 ||
                                 data->kind == nct6797 ||
                                 data->kind == nct6798) ? 7 : 6;
--
2.26.2

Re: [PATCH] hwmon: nct6775: Add missing voltage reading NCT6796D

[Henk]

The nuvoton driver has quite some longstanding issues, as far as I can
tell the alarm bits and input registers are not properly configured
for a bunch of the chips.
And this patch only addresses a tiny proportion of the issues.

It would be helpful to get some meaningful feedback.

Is there at least any interest in solving this issue?

Op do 8 jul. 2021 om 02:42 schreef Henk <henk.vergonet@gmail.com>:
>
> Retry with sendmail  hope this helps...
>
> ---
>
> Noticed in the documentation that the NCT6796D actually has 16 analog
> voltage inputs, where only 15 are configured.
>
> See: https://www.nuvoton.com/resource-files/NCT6796D_Datasheet_V0_6.pdf
> page 59-60:
>
> 8.6.2.1. Voltage Reading
> NCT6796D has 16 voltage reading:
> ....
> and the missing one in the driver:
> VIN9: Voltage reading Bank 4, Index 8F
>
> This patch adds support for in15 on the NCT6796D.
> ---
>  drivers/hwmon/nct6775.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hwmon/nct6775.c b/drivers/hwmon/nct6775.c
> index 5bd15622a85f..385ef79d79c4 100644
> --- a/drivers/hwmon/nct6775.c
> +++ b/drivers/hwmon/nct6775.c
> @@ -28,7 +28,7 @@
>   * nct6792d    15      6       6       2+6    0xc910 0xc1    0x5ca3
>   * nct6793d    15      6       6       2+6    0xd120 0xc1    0x5ca3
>   * nct6795d    14      6       6       2+6    0xd350 0xc1    0x5ca3
> - * nct6796d    14      7       7       2+6    0xd420 0xc1    0x5ca3
> + * nct6796d    16      7       7       2+6    0xd420 0xc1    0x5ca3
>   * nct6797d    14      7       7       2+6    0xd450 0xc1    0x5ca3
>   *                                           (0xd451)
>   * nct6798d    14      7       7       2+6    0xd428 0xc1    0x5ca3
> @@ -209,14 +209,14 @@ superio_exit(int ioreg)
>
>  /* Common and NCT6775 specific data */
>
> -/* Voltage min/max registers for nr=7..14 are in bank 5 */
> +/* Voltage min/max registers for nr=7..15 are in bank 5 */
>
>  static const u16 NCT6775_REG_IN_MAX[] = {
>         0x2b, 0x2d, 0x2f, 0x31, 0x33, 0x35, 0x37, 0x554, 0x556, 0x558, 0x55a,
> -       0x55c, 0x55e, 0x560, 0x562 };
> +       0x55c, 0x55e, 0x560, 0x562, 0x564 };
>  static const u16 NCT6775_REG_IN_MIN[] = {
>         0x2c, 0x2e, 0x30, 0x32, 0x34, 0x36, 0x38, 0x555, 0x557, 0x559, 0x55b,
> -       0x55d, 0x55f, 0x561, 0x563 };
> +       0x55d, 0x55f, 0x561, 0x563, 0x565 };
>  static const u16 NCT6775_REG_IN[] = {
>         0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x550, 0x551, 0x552
>  };
> @@ -478,7 +478,7 @@ static const u16 NCT6776_REG_TEMP_CRIT[32] = {
>
>  static const u16 NCT6779_REG_IN[] = {
>         0x480, 0x481, 0x482, 0x483, 0x484, 0x485, 0x486, 0x487,
> -       0x488, 0x489, 0x48a, 0x48b, 0x48c, 0x48d, 0x48e };
> +       0x488, 0x489, 0x48a, 0x48b, 0x48c, 0x48d, 0x48e, 0x48f };
>
>  static const u16 NCT6779_REG_ALARM[NUM_REG_ALARM] = {
>         0x459, 0x45A, 0x45B, 0x568 };
> @@ -1047,9 +1047,9 @@ div_from_reg(u8 reg)
>   * Some of the voltage inputs have internal scaling, the tables below
>   * contain 8 (the ADC LSB in mV) * scaling factor * 100
>   */
> -static const u16 scale_in[15] = {
> +static const u16 scale_in[16] = {
>         800, 800, 1600, 1600, 800, 800, 800, 1600, 1600, 800, 800, 800, 800,
> -       800, 800
> +       800, 800, 800
>  };
>
>  static inline long in_from_reg(u8 reg, u8 nr)
> @@ -1143,7 +1143,7 @@ struct nct6775_data {
>         /* Register values */
>         u8 bank;                /* current register bank */
>         u8 in_num;              /* number of in inputs we have */
> -       u8 in[15][3];           /* [0]=in, [1]=in_max, [2]=in_min */
> +       u8 in[16][3];           /* [0]=in, [1]=in_max, [2]=in_min */
>         unsigned int rpm[NUM_FAN];
>         u16 fan_min[NUM_FAN];
>         u8 fan_pulses[NUM_FAN];
> @@ -4200,7 +4200,7 @@ static int nct6775_probe(struct platform_device *pdev)
>         case nct6796:
>         case nct6797:
>         case nct6798:
> -               data->in_num = 15;
> +               data->in_num = (data->kind == nct6796) ? 16 : 15;
>                 data->pwm_num = (data->kind == nct6796 ||
>                                  data->kind == nct6797 ||
>                                  data->kind == nct6798) ? 7 : 6;
> --
> 2.26.2

Re: [PATCH] hwmon: nct6775: Add missing voltage reading NCT6796D

[Guenter Roeck]

On 7/14/21 2:48 AM, Henk wrote:
> The nuvoton driver has quite some longstanding issues, as far as I can
> tell the alarm bits and input registers are not properly configured
> for a bunch of the chips.
> And this patch only addresses a tiny proportion of the issues.
> 
> It would be helpful to get some meaningful feedback.
> 
> Is there at least any interest in solving this issue?
> 

There is a lot of interest, but you'll have to do your part and send
patches in an acceptable form. So far you have not done that.

Guenter

Re: [PATCH] hwmon: nct6775: Add missing voltage reading NCT6796D

[Henk]

Can you be more specific?

Op wo 14 jul. 2021 om 13:00 schreef Guenter Roeck <linux@roeck-us.net>:
>
> On 7/14/21 2:48 AM, Henk wrote:
> > The nuvoton driver has quite some longstanding issues, as far as I can
> > tell the alarm bits and input registers are not properly configured
> > for a bunch of the chips.
> > And this patch only addresses a tiny proportion of the issues.
> >
> > It would be helpful to get some meaningful feedback.
> >
> > Is there at least any interest in solving this issue?
> >
>
> There is a lot of interest, but you'll have to do your part and send
> patches in an acceptable form. So far you have not done that.
>
> Guenter

Re: [PATCH] hwmon: nct6775: Add missing voltage reading NCT6796D

[Guenter Roeck]

On 7/14/21 7:28 AM, Henk wrote:
> Can you be more specific?
> 

Not a single one of your patch submissions followed the guidelines in
Documentation/process/submitting-patches.rst. Most common violations are:

- No or insufficient patch description
   See 'Describe your changes'
- Patch sent as attachment
   See 'No MIME, no links, no compression, no attachments.  Just plain text'
- Instead of sending a patch, sent pointer to github location

You could just set up git send-email and use it to send your patches,
as suggested in submitting-patches.rst. Not doing that has cost both
of us a lot of time, for no good reason. I don't know about your time,
but I am not getting paid for my work as Linux kernel maintainer,
and my time is limited.

You for sure have accomplished one thing: A new formletter.

<Formletter>
Please follow the guidelines in
     Documentation/hwmon/submitting-patches.rst
     Documentation/process/submitting-patches.rst
when submitting patches into the hardware monitoring subsystem.
Patches not following those guidelines will be ignored.
</Formletter>

Guenter

> Op wo 14 jul. 2021 om 13:00 schreef Guenter Roeck <linux@roeck-us.net>:
>>
>> On 7/14/21 2:48 AM, Henk wrote:
>>> The nuvoton driver has quite some longstanding issues, as far as I can
>>> tell the alarm bits and input registers are not properly configured
>>> for a bunch of the chips.
>>> And this patch only addresses a tiny proportion of the issues.
>>>
>>> It would be helpful to get some meaningful feedback.
>>>
>>> Is there at least any interest in solving this issue?
>>>
>>
>> There is a lot of interest, but you'll have to do your part and send
>> patches in an acceptable form. So far you have not done that.
>>
>> Guenter

Re: [PATCH] hwmon: nct6775: Add missing voltage reading NCT6796D

[Henk]

Op wo 14 jul. 2021 om 17:26 schreef Guenter Roeck <linux@roeck-us.net>:
>
> On 7/14/21 7:28 AM, Henk wrote:
> > Can you be more specific?
> >
>
> Not a single one of your patch submissions followed the guidelines in
> Documentation/process/submitting-patches.rst. Most common violations are:
>
> - No or insufficient patch description
>    See 'Describe your changes'
> - Patch sent as attachment
>    See 'No MIME, no links, no compression, no attachments.  Just plain text'
> - Instead of sending a patch, sent pointer to github location
>
> You could just set up git send-email and use it to send your patches,
> as suggested in submitting-patches.rst. Not doing that has cost both
> of us a lot of time, for no good reason. I don't know about your time,
> but I am not getting paid for my work as Linux kernel maintainer,
> and my time is limited.
>
> You for sure have accomplished one thing: A new formletter.
>
> <Formletter>
> Please follow the guidelines in
>      Documentation/hwmon/submitting-patches.rst
>      Documentation/process/submitting-patches.rst
> when submitting patches into the hardware monitoring subsystem.
> Patches not following those guidelines will be ignored.
> </Formletter>
>
> Guenter
>
Hi Guenter,

I am in similar position as you, my Linux work is unpaid and voluntary
and not subsidized as some of the other contributions are.

I am extremely grateful of the work that maintainers do and apologize
for the inconvenience that I may have caused.
Also I wish to thank you for the new form letter. I would have send a
pointer to my github location at first impulse however:

I forked torvalds/linux from github in the understanding that it would
allow me to create a proper documented pull request that could be
routed trough the proper channels.
However then i discovered after patching the file that I was not
allowed to create a pull request, which is quite annoying.
So I am looping in the Linuxfoundation in this mail in order to ask
them to please reconsider their current policy and allow the creation
of pull requests on gitgub/torvalds/linux . This would greatly reduce
the amount of overhead.
I would also argue that the github commenting system, the automated
build system, and the review system are quite good, and we have
definitely made some progress in that area since the invention of the
mailing-list.

I understand the master repositories should reside on kernel.org but
bringing in some of the feedback to the kernel via github could be
quite beneficial!

(Just occurred to me I can send a pointer to my own commit :
https://github.com/hvegh/linux/commit/00b3427269f731becbc10e9fe92046a6ad91eee8
 )


Kind regards,
Henk Vergonet