| Message ID | 20230715152931.1307087-1-ahmad@khalifa.ws (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hwmon: (nct6775) Change labels for nct6799 | expand |
On 7/15/23 08:29, Ahmad Khalifa wrote: > nct6799d-r and nct6796d-s seem to be revisions where the former > appears on ASUS boards and the latter on ASRock. > > The datasheet for nct6796d-s (dated 2022) has several updates > over the datasheet for nct6796d (dated 2017) so we know it's > closer to the nct6799d-r (though have some minor diversions). > > Since both will be detected by the driver anyway due to the > chipid mask, they should be labeled together for dmesg msg. > > Signed-off-by: Ahmad Khalifa <ahmad@khalifa.ws> > --- > drivers/hwmon/nct6775-core.c | 5 ++++- > drivers/hwmon/nct6775-platform.c | 2 +- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c > index 236dc97f4d22..7163a2473fa0 100644 > --- a/drivers/hwmon/nct6775-core.c > +++ b/drivers/hwmon/nct6775-core.c > @@ -33,10 +33,13 @@ > * (0xd451) > * nct6798d 14 7 7 2+6 0xd428 0xc1 0x5ca3 > * (0xd429) > - * nct6799d 14 7 7 2+6 0xd802 0xc1 0x5ca3 > + * nct6796d-s 18 7 7 6+2 0xd801 0xc1 0x5ca3 (*) > + * nct6799d-r 18 7 7 6+2 0xd802 0xc1 0x5ca3 > * > * #temp lists the number of monitored temperature sources (first value) plus > * the number of directly connectable temperature sensors (second value). > + * > + * (*) nct6799d-r based on nct6796d-s and both quite different to nct6796d We should not say "based on" without evidence (that is, someone from Nuvoton confirming that this is the case), and "both quite different to nct6796d" is just confusing. nct6799d is _expected_ to be different to nct6796d. This should say something like "nct6796d-s is similar to nct6799d-r with minor differences which do not affect the driver". Apparently there are also NCT6796D-E (Nuvoton's web site) and NCT6796D-R (LibreHardwareMonitor). Do you know anything about those chips ? Based on the chip ID, NCT6796-R seems to be a variant of NCT6798D with chip ID 0xd42a. Any chance you can share the datasheet for NCT6796D-S ? > */ > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > diff --git a/drivers/hwmon/nct6775-platform.c b/drivers/hwmon/nct6775-platform.c > index a409d7a0b813..e8298befb77f 100644 > --- a/drivers/hwmon/nct6775-platform.c > +++ b/drivers/hwmon/nct6775-platform.c > @@ -35,7 +35,7 @@ static const char * const nct6775_sio_names[] __initconst = { > "NCT6796D", > "NCT6797D", > "NCT6798D", > - "NCT6799D", > + "NCT6799D-R/NCT6796D-S", Alphabetic order, please ("NCT6796D-S/NCT6799D-R"). Thanks, Guenter > }; > > static unsigned short force_id; > > base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 > prerequisite-patch-id: 36e3467bd9ea72cb3ad2bef638a8389a9537d111
On 15/07/2023 17:09, Guenter Roeck wrote: > On 7/15/23 08:29, Ahmad Khalifa wrote: >> * #temp lists the number of monitored temperature sources (first >> value) plus >> * the number of directly connectable temperature sensors (second >> value). >> + * >> + * (*) nct6799d-r based on nct6796d-s and both quite different to >> nct6796d > > We should not say "based on" without evidence (that is, someone from > Nuvoton confirming > that this is the case), and "both quite different to nct6796d" is just > confusing. > nct6799d is _expected_ to be different to nct6796d. This should say > something like > "nct6796d-s is similar to nct6799d-r with minor differences which do not > affect the > driver". It was Nuvoton support that told me that NCT6799D-R was a customer- specific chip, but "very similar" to 6796d-S. The point here was to specify the relevant chip ids and to say that the driver treats both the same way, so I can rephrase it to say "driver treats both the same way" maybe? However, I'm somewhat sure the temperature sources on 6799d-R use one of the reserved values when setup through the bios. > Apparently there are also NCT6796D-E (Nuvoton's web site) and NCT6796D-R > (LibreHardwareMonitor). Do you know anything about those chips ? Based > on the > chip ID, NCT6796-R seems to be a variant of NCT6798D with chip ID 0xd42a. Do you know which board had the NCT6796-R, it seems the R suffix is "customer-specific"? > Any chance you can share the datasheet for NCT6796D-S ? Of course: https://khalifa.ws/randomfiles/nuvoton/NCT6796D-S_Datasheet_V0_6.pdf >> + "NCT6799D-R/NCT6796D-S", > > Alphabetic order, please ("NCT6796D-S/NCT6799D-R"). Will update.
On 7/15/23 09:39, Ahmad Khalifa wrote: > On 15/07/2023 17:09, Guenter Roeck wrote: >> On 7/15/23 08:29, Ahmad Khalifa wrote: >>> * #temp lists the number of monitored temperature sources (first value) plus >>> * the number of directly connectable temperature sensors (second value). >>> + * >>> + * (*) nct6799d-r based on nct6796d-s and both quite different to nct6796d >> >> We should not say "based on" without evidence (that is, someone from Nuvoton confirming >> that this is the case), and "both quite different to nct6796d" is just confusing. >> nct6799d is _expected_ to be different to nct6796d. This should say something like >> "nct6796d-s is similar to nct6799d-r with minor differences which do not affect the >> driver". > > It was Nuvoton support that told me that NCT6799D-R was a customer- > specific chip, but "very similar" to 6796d-S. > > The point here was to specify the relevant chip ids and to say that the > driver treats both the same way, so I can rephrase it to say "driver > treats both the same way" maybe? > It should say that the two chips are similar and that the driver does not treat them differently. I'll leave the exact wording to you. > However, I'm somewhat sure the temperature sources on 6799d-R use one of > the reserved values when setup through the bios. > >> Apparently there are also NCT6796D-E (Nuvoton's web site) and NCT6796D-R >> (LibreHardwareMonitor). Do you know anything about those chips ? Based on the >> chip ID, NCT6796-R seems to be a variant of NCT6798D with chip ID 0xd42a. > > Do you know which board had the NCT6796-R, it seems the R suffix is > "customer-specific"? > From LibreHardwareMonitor commit log: Add initial NCT6796D-R support This is used by Asrock X570 Pro4 >> Any chance you can share the datasheet for NCT6796D-S ? > > Of course: > https://khalifa.ws/randomfiles/nuvoton/NCT6796D-S_Datasheet_V0_6.pdf > Thanks! Guenter
diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c index 236dc97f4d22..7163a2473fa0 100644 --- a/drivers/hwmon/nct6775-core.c +++ b/drivers/hwmon/nct6775-core.c @@ -33,10 +33,13 @@ * (0xd451) * nct6798d 14 7 7 2+6 0xd428 0xc1 0x5ca3 * (0xd429) - * nct6799d 14 7 7 2+6 0xd802 0xc1 0x5ca3 + * nct6796d-s 18 7 7 6+2 0xd801 0xc1 0x5ca3 (*) + * nct6799d-r 18 7 7 6+2 0xd802 0xc1 0x5ca3 * * #temp lists the number of monitored temperature sources (first value) plus * the number of directly connectable temperature sensors (second value). + * + * (*) nct6799d-r based on nct6796d-s and both quite different to nct6796d */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt diff --git a/drivers/hwmon/nct6775-platform.c b/drivers/hwmon/nct6775-platform.c index a409d7a0b813..e8298befb77f 100644 --- a/drivers/hwmon/nct6775-platform.c +++ b/drivers/hwmon/nct6775-platform.c @@ -35,7 +35,7 @@ static const char * const nct6775_sio_names[] __initconst = { "NCT6796D", "NCT6797D", "NCT6798D", - "NCT6799D", + "NCT6799D-R/NCT6796D-S", }; static unsigned short force_id;
nct6799d-r and nct6796d-s seem to be revisions where the former appears on ASUS boards and the latter on ASRock. The datasheet for nct6796d-s (dated 2022) has several updates over the datasheet for nct6796d (dated 2017) so we know it's closer to the nct6799d-r (though have some minor diversions). Since both will be detected by the driver anyway due to the chipid mask, they should be labeled together for dmesg msg. Signed-off-by: Ahmad Khalifa <ahmad@khalifa.ws> --- drivers/hwmon/nct6775-core.c | 5 ++++- drivers/hwmon/nct6775-platform.c | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 prerequisite-patch-id: 36e3467bd9ea72cb3ad2bef638a8389a9537d111