diff mbox series

hwmon: corsair-psu: update supported devices

Message ID X7+T4aZSUuzfsf7H@monster.powergraphx.local (mailing list archive)
State Accepted
Headers show
Series hwmon: corsair-psu: update supported devices | expand

Commit Message

Wilken Gottwalt Nov. 26, 2020, 11:40 a.m. UTC
Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
AX1500i and AX1600i. The first 3 power supplies are supported through
the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
converter especially made for the COM ports of these power supplies.
There are 3 known revisions of these adapters. The AX1500i power supply
has revision 3 built into the case and AX1600i is the only one in that
series, which has an unique usb hid id like the RM/RX series.

The patch also changes the usb hid ids to use upper case letters to be
consistent with the rest of the hex numbers in the driver and updates
the hwmon documentation.

This patch adds:
- hwmon/corsair-psu documentation update
- corsair-psu driver update

Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
---
 Documentation/hwmon/corsair-psu.rst | 10 +++++++++
 drivers/hwmon/Kconfig               |  7 +++---
 drivers/hwmon/corsair-psu.c         | 33 +++++++++++++++++++----------
 3 files changed, 36 insertions(+), 14 deletions(-)

Comments

Guenter Roeck Nov. 27, 2020, 3:39 p.m. UTC | #1
On Thu, Nov 26, 2020 at 12:40:16PM +0100, Wilken Gottwalt wrote:
> Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
> AX1500i and AX1600i. The first 3 power supplies are supported through
> the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
> converter especially made for the COM ports of these power supplies.
> There are 3 known revisions of these adapters. The AX1500i power supply
> has revision 3 built into the case and AX1600i is the only one in that
> series, which has an unique usb hid id like the RM/RX series.
> 
> The patch also changes the usb hid ids to use upper case letters to be
> consistent with the rest of the hex numbers in the driver and updates
> the hwmon documentation.
> 
> This patch adds:
> - hwmon/corsair-psu documentation update
> - corsair-psu driver update
> 
> Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>

Applied.

Thanks,
Guenter

> ---
>  Documentation/hwmon/corsair-psu.rst | 10 +++++++++
>  drivers/hwmon/Kconfig               |  7 +++---
>  drivers/hwmon/corsair-psu.c         | 33 +++++++++++++++++++----------
>  3 files changed, 36 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/hwmon/corsair-psu.rst b/Documentation/hwmon/corsair-psu.rst
> index 396b95c9a76a..6227e9046d73 100644
> --- a/Documentation/hwmon/corsair-psu.rst
> +++ b/Documentation/hwmon/corsair-psu.rst
> @@ -7,6 +7,16 @@ Supported devices:
>  
>  * Corsair Power Supplies
>  
> +  Corsair AX760i (by Corsair Link USB Dongle)
> +
> +  Corsair AX860i (by Corsair Link USB Dongle)
> +
> +  Corsair AX1200i (by Corsair Link USB Dongle)
> +
> +  Corsair AX1500i (by builtin Corsair Link USB Dongle)
> +
> +  Corsair AX1600i
> +
>    Corsair HX550i
>  
>    Corsair HX650i
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 716df51edc87..3c059fc23cd6 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -453,11 +453,12 @@ config SENSORS_CORSAIR_PSU
>  	tristate "Corsair PSU HID controller"
>  	depends on HID
>  	help
> -	  If you say yes here you get support for Corsair PSUs with a HID
> +	  If you say yes here you get support for Corsair PSUs with an USB HID
>  	  interface.
>  	  Currently this driver supports the (RM/HX)550i, (RM/HX)650i,
> -	  (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i and HX1200i power supplies
> -	  by Corsair.
> +	  (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i, HX1200i and AX1600i power
> +	  supplies by Corsair. The AX760i, AX860i, AX1200i and AX1500i
> +	  power supplies are supported through the Corsair Link USB Dongle.
>  
>  	  This driver can also be built as a module. If so, the module
>  	  will be called corsair-psu.
> diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> index 99494056f4bd..0146dda3e2c3 100644
> --- a/drivers/hwmon/corsair-psu.c
> +++ b/drivers/hwmon/corsair-psu.c
> @@ -571,17 +571,28 @@ static int corsairpsu_raw_event(struct hid_device *hdev, struct hid_report *repo
>  }
>  
>  static const struct hid_device_id corsairpsu_idtable[] = {
> -	{ HID_USB_DEVICE(0x1b1c, 0x1c03) }, /* Corsair HX550i */
> -	{ HID_USB_DEVICE(0x1b1c, 0x1c04) }, /* Corsair HX650i */
> -	{ HID_USB_DEVICE(0x1b1c, 0x1c05) }, /* Corsair HX750i */
> -	{ HID_USB_DEVICE(0x1b1c, 0x1c06) }, /* Corsair HX850i */
> -	{ HID_USB_DEVICE(0x1b1c, 0x1c07) }, /* Corsair HX1000i */
> -	{ HID_USB_DEVICE(0x1b1c, 0x1c08) }, /* Corsair HX1200i */
> -	{ HID_USB_DEVICE(0x1b1c, 0x1c09) }, /* Corsair RM550i */
> -	{ HID_USB_DEVICE(0x1b1c, 0x1c0a) }, /* Corsair RM650i */
> -	{ HID_USB_DEVICE(0x1b1c, 0x1c0b) }, /* Corsair RM750i */
> -	{ HID_USB_DEVICE(0x1b1c, 0x1c0c) }, /* Corsair RM850i */
> -	{ HID_USB_DEVICE(0x1b1c, 0x1c0d) }, /* Corsair RM1000i */
> +	/*
> +	 * The Corsair USB/COM Dongles appear in at least 3 different revisions, where rev 1 and 2
> +	 * are commonly used with the AX760i, AX860i and AX1200i, while rev3 is rarely seen with
> +	 * these PSUs. Rev3 is also build into the AX1500i, while the AX1600i is the first PSU of
> +	 * this series which has an unique usb hid id. Though, the actual device name is part of
> +	 * the HID message protocol, so it doesn't matter which dongle is connected.
> +	 */
> +	{ HID_USB_DEVICE(0x1B1C, 0x1C00) }, /* Corsair Link USB/COM Dongle rev1 */
> +	{ HID_USB_DEVICE(0x1B1C, 0x1C01) }, /* Corsair Link USB/COM Dongle rev2 */
> +	{ HID_USB_DEVICE(0x1B1C, 0x1C02) }, /* Corsair Link USB/COM Dongle rev3 (AX1500i) */
> +	{ HID_USB_DEVICE(0x1B1C, 0x1C03) }, /* Corsair HX550i */
> +	{ HID_USB_DEVICE(0x1B1C, 0x1C04) }, /* Corsair HX650i */
> +	{ HID_USB_DEVICE(0x1B1C, 0x1C05) }, /* Corsair HX750i */
> +	{ HID_USB_DEVICE(0x1B1C, 0x1C06) }, /* Corsair HX850i */
> +	{ HID_USB_DEVICE(0x1B1C, 0x1C07) }, /* Corsair HX1000i */
> +	{ HID_USB_DEVICE(0x1B1C, 0x1C08) }, /* Corsair HX1200i */
> +	{ HID_USB_DEVICE(0x1B1C, 0x1C09) }, /* Corsair RM550i */
> +	{ HID_USB_DEVICE(0x1B1C, 0x1C0A) }, /* Corsair RM650i */
> +	{ HID_USB_DEVICE(0x1B1C, 0x1C0B) }, /* Corsair RM750i */
> +	{ HID_USB_DEVICE(0x1B1C, 0x1C0C) }, /* Corsair RM850i */
> +	{ HID_USB_DEVICE(0x1B1C, 0x1C0D) }, /* Corsair RM1000i */
> +	{ HID_USB_DEVICE(0x1B1C, 0x1C11) }, /* Corsair AX1600i */
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(hid, corsairpsu_idtable);
Jonas Malaco Nov. 28, 2020, 5:37 a.m. UTC | #2
On Thu, Nov 26, 2020 at 8:43 AM Wilken Gottwalt
<wilken.gottwalt@posteo.net> wrote:
>
> Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
> AX1500i and AX1600i. The first 3 power supplies are supported through
> the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
> converter especially made for the COM ports of these power supplies.
> There are 3 known revisions of these adapters. The AX1500i power supply
> has revision 3 built into the case and AX1600i is the only one in that
> series, which has an unique usb hid id like the RM/RX series.

Can I ask what AXi power supplies were tested?

I ask because, based on the user-space implementations I am aware of,
the AXi dongle protocol appears to be different from the RMi/HXi series.

AXi dongle:
 - https://github.com/ka87/cpsumon

RMi/HXi:
 - https://github.com/jonasmalacofilho/liquidctl
 - https://github.com/audiohacked/OpenCorsairLink
 - https://github.com/notaz/corsairmi

One additional concern is that the non-HID AXi dongles may only have bulk
USB endpoints, and this is a HID driver.[1]

Thanks,
Jonas

[1] https://github.com/ka87/cpsumon/issues/4


>
> The patch also changes the usb hid ids to use upper case letters to be
> consistent with the rest of the hex numbers in the driver and updates
> the hwmon documentation.
>
> This patch adds:
> - hwmon/corsair-psu documentation update
> - corsair-psu driver update
>
> Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> ---
>  Documentation/hwmon/corsair-psu.rst | 10 +++++++++
>  drivers/hwmon/Kconfig               |  7 +++---
>  drivers/hwmon/corsair-psu.c         | 33 +++++++++++++++++++----------
>  3 files changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/hwmon/corsair-psu.rst b/Documentation/hwmon/corsair-psu.rst
> index 396b95c9a76a..6227e9046d73 100644
> --- a/Documentation/hwmon/corsair-psu.rst
> +++ b/Documentation/hwmon/corsair-psu.rst
> @@ -7,6 +7,16 @@ Supported devices:
>
>  * Corsair Power Supplies
>
> +  Corsair AX760i (by Corsair Link USB Dongle)
> +
> +  Corsair AX860i (by Corsair Link USB Dongle)
> +
> +  Corsair AX1200i (by Corsair Link USB Dongle)
> +
> +  Corsair AX1500i (by builtin Corsair Link USB Dongle)
> +
> +  Corsair AX1600i
> +
>    Corsair HX550i
>
>    Corsair HX650i
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 716df51edc87..3c059fc23cd6 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -453,11 +453,12 @@ config SENSORS_CORSAIR_PSU
>         tristate "Corsair PSU HID controller"
>         depends on HID
>         help
> -         If you say yes here you get support for Corsair PSUs with a HID
> +         If you say yes here you get support for Corsair PSUs with an USB HID
>           interface.
>           Currently this driver supports the (RM/HX)550i, (RM/HX)650i,
> -         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i and HX1200i power supplies
> -         by Corsair.
> +         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i, HX1200i and AX1600i power
> +         supplies by Corsair. The AX760i, AX860i, AX1200i and AX1500i
> +         power supplies are supported through the Corsair Link USB Dongle.
>
>           This driver can also be built as a module. If so, the module
>           will be called corsair-psu.
> diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> index 99494056f4bd..0146dda3e2c3 100644
> --- a/drivers/hwmon/corsair-psu.c
> +++ b/drivers/hwmon/corsair-psu.c
> @@ -571,17 +571,28 @@ static int corsairpsu_raw_event(struct hid_device *hdev, struct hid_report *repo
>  }
>
>  static const struct hid_device_id corsairpsu_idtable[] = {
> -       { HID_USB_DEVICE(0x1b1c, 0x1c03) }, /* Corsair HX550i */
> -       { HID_USB_DEVICE(0x1b1c, 0x1c04) }, /* Corsair HX650i */
> -       { HID_USB_DEVICE(0x1b1c, 0x1c05) }, /* Corsair HX750i */
> -       { HID_USB_DEVICE(0x1b1c, 0x1c06) }, /* Corsair HX850i */
> -       { HID_USB_DEVICE(0x1b1c, 0x1c07) }, /* Corsair HX1000i */
> -       { HID_USB_DEVICE(0x1b1c, 0x1c08) }, /* Corsair HX1200i */
> -       { HID_USB_DEVICE(0x1b1c, 0x1c09) }, /* Corsair RM550i */
> -       { HID_USB_DEVICE(0x1b1c, 0x1c0a) }, /* Corsair RM650i */
> -       { HID_USB_DEVICE(0x1b1c, 0x1c0b) }, /* Corsair RM750i */
> -       { HID_USB_DEVICE(0x1b1c, 0x1c0c) }, /* Corsair RM850i */
> -       { HID_USB_DEVICE(0x1b1c, 0x1c0d) }, /* Corsair RM1000i */
> +       /*
> +        * The Corsair USB/COM Dongles appear in at least 3 different revisions, where rev 1 and 2
> +        * are commonly used with the AX760i, AX860i and AX1200i, while rev3 is rarely seen with
> +        * these PSUs. Rev3 is also build into the AX1500i, while the AX1600i is the first PSU of
> +        * this series which has an unique usb hid id. Though, the actual device name is part of
> +        * the HID message protocol, so it doesn't matter which dongle is connected.
> +        */
> +       { HID_USB_DEVICE(0x1B1C, 0x1C00) }, /* Corsair Link USB/COM Dongle rev1 */
> +       { HID_USB_DEVICE(0x1B1C, 0x1C01) }, /* Corsair Link USB/COM Dongle rev2 */
> +       { HID_USB_DEVICE(0x1B1C, 0x1C02) }, /* Corsair Link USB/COM Dongle rev3 (AX1500i) */
> +       { HID_USB_DEVICE(0x1B1C, 0x1C03) }, /* Corsair HX550i */
> +       { HID_USB_DEVICE(0x1B1C, 0x1C04) }, /* Corsair HX650i */
> +       { HID_USB_DEVICE(0x1B1C, 0x1C05) }, /* Corsair HX750i */
> +       { HID_USB_DEVICE(0x1B1C, 0x1C06) }, /* Corsair HX850i */
> +       { HID_USB_DEVICE(0x1B1C, 0x1C07) }, /* Corsair HX1000i */
> +       { HID_USB_DEVICE(0x1B1C, 0x1C08) }, /* Corsair HX1200i */
> +       { HID_USB_DEVICE(0x1B1C, 0x1C09) }, /* Corsair RM550i */
> +       { HID_USB_DEVICE(0x1B1C, 0x1C0A) }, /* Corsair RM650i */
> +       { HID_USB_DEVICE(0x1B1C, 0x1C0B) }, /* Corsair RM750i */
> +       { HID_USB_DEVICE(0x1B1C, 0x1C0C) }, /* Corsair RM850i */
> +       { HID_USB_DEVICE(0x1B1C, 0x1C0D) }, /* Corsair RM1000i */
> +       { HID_USB_DEVICE(0x1B1C, 0x1C11) }, /* Corsair AX1600i */
>         { },
>  };
>  MODULE_DEVICE_TABLE(hid, corsairpsu_idtable);
> --
> 2.29.2
>
Wilken Gottwalt Nov. 28, 2020, 10:35 a.m. UTC | #3
On Sat, 28 Nov 2020 02:37:38 -0300
Jonas Malaco <jonas@protocubo.io> wrote:

> On Thu, Nov 26, 2020 at 8:43 AM Wilken Gottwalt
> <wilken.gottwalt@posteo.net> wrote:
> >
> > Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
> > AX1500i and AX1600i. The first 3 power supplies are supported through
> > the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
> > converter especially made for the COM ports of these power supplies.
> > There are 3 known revisions of these adapters. The AX1500i power supply
> > has revision 3 built into the case and AX1600i is the only one in that
> > series, which has an unique usb hid id like the RM/RX series.
> 
> Can I ask what AXi power supplies were tested?
> 
> I ask because, based on the user-space implementations I am aware of,
> the AXi dongle protocol appears to be different from the RMi/HXi series.

I was not able to test this against the AX power supplies, they are really
hard to find (and are far to expensive). But I went through all these tools
and stuck to the most common commands, which all 3 series support. Not every
series supports all commands (there also seem to be different firmwares in
the micro-conrollers). But this is fine, some sensors will show up as N/A.
Even my HX850i does not support all commands covered in this driver.

> AXi dongle:
>  - https://github.com/ka87/cpsumon

This tool made me to consider including the AX series, because it uses some
of the same commands on the AX760i, AX860i, AX1200i and AX1500i. But it is
a usb-serial tool only. But it was nice to know, that the commands are mostly
the same. I left out all the commands for configuring, PCIe power rails, 
efficiency and others which do not really belong into hwmon.

> RMi/HXi:
>  - https://github.com/jonasmalacofilho/liquidctl
>  - https://github.com/audiohacked/OpenCorsairLink

This tool made me include the AX series, because it uses the rmi protocol
component for the rmi driver (RM/HX series) and the corsair dongles.

>  - https://github.com/notaz/corsairmi

This one covers only some HX/RM PSUs, but is uses the rawhid access which
made me looking up the actual usb chips/bridges Corsair uses.

> 
> One additional concern is that the non-HID AXi dongles may only have bulk
> USB endpoints, and this is a HID driver.[1]

You are right, in the case of the dongles it could be different. But I did
some research on Corsair usb driven devices and they really like to stick to
the cp210x, which is an usb hid bridge. The commit
b9326057a3d8447f5d2e74a7b521ccf21add2ec0 actually covers two Corsair USB
dongles as a cp210x device. So it is very likely that all Corsair PSUs with
such an interface/dongle use usb hid. But I'm completely open to get proven
wrong. Actually I really would like to see this tested by people who have
access to the more rare devices.

> Thanks,
> Jonas
> 
> [1] https://github.com/ka87/cpsumon/issues/4

Yes ... that one. The last revision of the dongle could indeed be a problem.
But I'm not really sure what is described here. The last commenter is actually
the one who provided the cp210x patch mentioned up there. The problem here is,
the AX1500i has both connectors, USB and that other one. I call it the other
one because it is the only PSU where it is labeled I2C COMM instead of COMM
only. But at the same time this tools uses the same commands for this PSU.

So, only some real hardware tests will show.

Greetings,
Wilken

> >
> > The patch also changes the usb hid ids to use upper case letters to be
> > consistent with the rest of the hex numbers in the driver and updates
> > the hwmon documentation.
> >
> > This patch adds:
> > - hwmon/corsair-psu documentation update
> > - corsair-psu driver update
> >
> > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> > ---
> >  Documentation/hwmon/corsair-psu.rst | 10 +++++++++
> >  drivers/hwmon/Kconfig               |  7 +++---
> >  drivers/hwmon/corsair-psu.c         | 33 +++++++++++++++++++----------
> >  3 files changed, 36 insertions(+), 14 deletions(-)
> >
> > diff --git a/Documentation/hwmon/corsair-psu.rst b/Documentation/hwmon/corsair-psu.rst
> > index 396b95c9a76a..6227e9046d73 100644
> > --- a/Documentation/hwmon/corsair-psu.rst
> > +++ b/Documentation/hwmon/corsair-psu.rst
> > @@ -7,6 +7,16 @@ Supported devices:
> >
> >  * Corsair Power Supplies
> >
> > +  Corsair AX760i (by Corsair Link USB Dongle)
> > +
> > +  Corsair AX860i (by Corsair Link USB Dongle)
> > +
> > +  Corsair AX1200i (by Corsair Link USB Dongle)
> > +
> > +  Corsair AX1500i (by builtin Corsair Link USB Dongle)
> > +
> > +  Corsair AX1600i
> > +
> >    Corsair HX550i
> >
> >    Corsair HX650i
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 716df51edc87..3c059fc23cd6 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -453,11 +453,12 @@ config SENSORS_CORSAIR_PSU
> >         tristate "Corsair PSU HID controller"
> >         depends on HID
> >         help
> > -         If you say yes here you get support for Corsair PSUs with a HID
> > +         If you say yes here you get support for Corsair PSUs with an USB HID
> >           interface.
> >           Currently this driver supports the (RM/HX)550i, (RM/HX)650i,
> > -         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i and HX1200i power supplies
> > -         by Corsair.
> > +         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i, HX1200i and AX1600i power
> > +         supplies by Corsair. The AX760i, AX860i, AX1200i and AX1500i
> > +         power supplies are supported through the Corsair Link USB Dongle.
> >
> >           This driver can also be built as a module. If so, the module
> >           will be called corsair-psu.
> > diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> > index 99494056f4bd..0146dda3e2c3 100644
> > --- a/drivers/hwmon/corsair-psu.c
> > +++ b/drivers/hwmon/corsair-psu.c
> > @@ -571,17 +571,28 @@ static int corsairpsu_raw_event(struct hid_device *hdev, struct
> > hid_report *repo }
> >
> >  static const struct hid_device_id corsairpsu_idtable[] = {
> > -       { HID_USB_DEVICE(0x1b1c, 0x1c03) }, /* Corsair HX550i */
> > -       { HID_USB_DEVICE(0x1b1c, 0x1c04) }, /* Corsair HX650i */
> > -       { HID_USB_DEVICE(0x1b1c, 0x1c05) }, /* Corsair HX750i */
> > -       { HID_USB_DEVICE(0x1b1c, 0x1c06) }, /* Corsair HX850i */
> > -       { HID_USB_DEVICE(0x1b1c, 0x1c07) }, /* Corsair HX1000i */
> > -       { HID_USB_DEVICE(0x1b1c, 0x1c08) }, /* Corsair HX1200i */
> > -       { HID_USB_DEVICE(0x1b1c, 0x1c09) }, /* Corsair RM550i */
> > -       { HID_USB_DEVICE(0x1b1c, 0x1c0a) }, /* Corsair RM650i */
> > -       { HID_USB_DEVICE(0x1b1c, 0x1c0b) }, /* Corsair RM750i */
> > -       { HID_USB_DEVICE(0x1b1c, 0x1c0c) }, /* Corsair RM850i */
> > -       { HID_USB_DEVICE(0x1b1c, 0x1c0d) }, /* Corsair RM1000i */
> > +       /*
> > +        * The Corsair USB/COM Dongles appear in at least 3 different revisions, where rev 1
> > and 2
> > +        * are commonly used with the AX760i, AX860i and AX1200i, while rev3 is rarely seen with
> > +        * these PSUs. Rev3 is also build into the AX1500i, while the AX1600i is the first PSU
> > of
> > +        * this series which has an unique usb hid id. Though, the actual device name is part of
> > +        * the HID message protocol, so it doesn't matter which dongle is connected.
> > +        */
> > +       { HID_USB_DEVICE(0x1B1C, 0x1C00) }, /* Corsair Link USB/COM Dongle rev1 */
> > +       { HID_USB_DEVICE(0x1B1C, 0x1C01) }, /* Corsair Link USB/COM Dongle rev2 */
> > +       { HID_USB_DEVICE(0x1B1C, 0x1C02) }, /* Corsair Link USB/COM Dongle rev3 (AX1500i) */
> > +       { HID_USB_DEVICE(0x1B1C, 0x1C03) }, /* Corsair HX550i */
> > +       { HID_USB_DEVICE(0x1B1C, 0x1C04) }, /* Corsair HX650i */
> > +       { HID_USB_DEVICE(0x1B1C, 0x1C05) }, /* Corsair HX750i */
> > +       { HID_USB_DEVICE(0x1B1C, 0x1C06) }, /* Corsair HX850i */
> > +       { HID_USB_DEVICE(0x1B1C, 0x1C07) }, /* Corsair HX1000i */
> > +       { HID_USB_DEVICE(0x1B1C, 0x1C08) }, /* Corsair HX1200i */
> > +       { HID_USB_DEVICE(0x1B1C, 0x1C09) }, /* Corsair RM550i */
> > +       { HID_USB_DEVICE(0x1B1C, 0x1C0A) }, /* Corsair RM650i */
> > +       { HID_USB_DEVICE(0x1B1C, 0x1C0B) }, /* Corsair RM750i */
> > +       { HID_USB_DEVICE(0x1B1C, 0x1C0C) }, /* Corsair RM850i */
> > +       { HID_USB_DEVICE(0x1B1C, 0x1C0D) }, /* Corsair RM1000i */
> > +       { HID_USB_DEVICE(0x1B1C, 0x1C11) }, /* Corsair AX1600i */
> >         { },
> >  };
> >  MODULE_DEVICE_TABLE(hid, corsairpsu_idtable);
> > --
> > 2.29.2
> >
Jonas Malaco Nov. 28, 2020, 8:21 p.m. UTC | #4
On Sat, Nov 28, 2020 at 7:35 AM Wilken Gottwalt
<wilken.gottwalt@posteo.net> wrote:
>
> On Sat, 28 Nov 2020 02:37:38 -0300
> Jonas Malaco <jonas@protocubo.io> wrote:
>
> > On Thu, Nov 26, 2020 at 8:43 AM Wilken Gottwalt
> > <wilken.gottwalt@posteo.net> wrote:
> > >
> > > Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
> > > AX1500i and AX1600i. The first 3 power supplies are supported through
> > > the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
> > > converter especially made for the COM ports of these power supplies.
> > > There are 3 known revisions of these adapters. The AX1500i power supply
> > > has revision 3 built into the case and AX1600i is the only one in that
> > > series, which has an unique usb hid id like the RM/RX series.
> >
> > Can I ask what AXi power supplies were tested?
> >
> > I ask because, based on the user-space implementations I am aware of,
> > the AXi dongle protocol appears to be different from the RMi/HXi series.
>
> I was not able to test this against the AX power supplies, they are really
> hard to find (and are far to expensive). But I went through all these tools
> and stuck to the most common commands, which all 3 series support. Not every
> series supports all commands (there also seem to be different firmwares in
> the micro-conrollers). But this is fine, some sensors will show up as N/A.
> Even my HX850i does not support all commands covered in this driver.

I think the similarities come from all using wrappers over the PMBus
interface to the voltage controller.  But I am not sure the wrapping
protocols are identical.

For example, cpsumon shows significantly more things going on during a
read than what is needed for the RMi/HXi series.[1]

[1] https://github.com/ka87/cpsumon/blob/fd639684d7f9/libcpsumon/src/cpsumon.c#L213-L231


>
> > AXi dongle:
> >  - https://github.com/ka87/cpsumon
>
> This tool made me to consider including the AX series, because it uses some
> of the same commands on the AX760i, AX860i, AX1200i and AX1500i. But it is
> a usb-serial tool only. But it was nice to know, that the commands are mostly
> the same. I left out all the commands for configuring, PCIe power rails,
> efficiency and others which do not really belong into hwmon.
>
> > RMi/HXi:
> >  - https://github.com/jonasmalacofilho/liquidctl
> >  - https://github.com/audiohacked/OpenCorsairLink
>
> This tool made me include the AX series, because it uses the rmi protocol
> component for the rmi driver (RM/HX series) and the corsair dongles.

The corsairlink_driver_dongle has no implementations for reading sensor
data (compare that with the corsairlink_driver_rmi).[2][3]  There is
also no code that actually tries to read (write) from (to) the device
using that dongle driver.[4]

I also looked at a few of the issues, and all of the ones I read
mentioned AXi support being under development, and the hypothesis of the
AXi series being compatible with the RMi/HXi code still remaining to be
confirmed.

[2] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/dongle.c#L33-L39
[3] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/rmi.c#L33-L57
[4] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/main.c#L106


>
> >  - https://github.com/notaz/corsairmi
>
> This one covers only some HX/RM PSUs, but is uses the rawhid access which
> made me looking up the actual usb chips/bridges Corsair uses.
>
> >
> > One additional concern is that the non-HID AXi dongles may only have bulk
> > USB endpoints, and this is a HID driver.[1]
>
> You are right, in the case of the dongles it could be different. But I did
> some research on Corsair usb driven devices and they really like to stick to
> the cp210x, which is an usb hid bridge. The commit
> b9326057a3d8447f5d2e74a7b521ccf21add2ec0 actually covers two Corsair USB
> dongles as a cp210x device. So it is very likely that all Corsair PSUs with
> such an interface/dongle use usb hid. But I'm completely open to get proven
> wrong. Actually I really would like to see this tested by people who have
> access to the more rare devices.

I could be wrong (and I am sorry for the noise if that is the case), but
as far as I can see the cp210x does not create a HID device.

Thanks again,
Jonas


>
> > Thanks,
> > Jonas
> >
> > [1] https://github.com/ka87/cpsumon/issues/4
>
> Yes ... that one. The last revision of the dongle could indeed be a problem.
> But I'm not really sure what is described here. The last commenter is actually
> the one who provided the cp210x patch mentioned up there. The problem here is,
> the AX1500i has both connectors, USB and that other one. I call it the other
> one because it is the only PSU where it is labeled I2C COMM instead of COMM
> only. But at the same time this tools uses the same commands for this PSU.
>
> So, only some real hardware tests will show.
>
> Greetings,
> Wilken
>
> > >
> > > The patch also changes the usb hid ids to use upper case letters to be
> > > consistent with the rest of the hex numbers in the driver and updates
> > > the hwmon documentation.
> > >
> > > This patch adds:
> > > - hwmon/corsair-psu documentation update
> > > - corsair-psu driver update
> > >
> > > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> > > ---
> > >  Documentation/hwmon/corsair-psu.rst | 10 +++++++++
> > >  drivers/hwmon/Kconfig               |  7 +++---
> > >  drivers/hwmon/corsair-psu.c         | 33 +++++++++++++++++++----------
> > >  3 files changed, 36 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/Documentation/hwmon/corsair-psu.rst b/Documentation/hwmon/corsair-psu.rst
> > > index 396b95c9a76a..6227e9046d73 100644
> > > --- a/Documentation/hwmon/corsair-psu.rst
> > > +++ b/Documentation/hwmon/corsair-psu.rst
> > > @@ -7,6 +7,16 @@ Supported devices:
> > >
> > >  * Corsair Power Supplies
> > >
> > > +  Corsair AX760i (by Corsair Link USB Dongle)
> > > +
> > > +  Corsair AX860i (by Corsair Link USB Dongle)
> > > +
> > > +  Corsair AX1200i (by Corsair Link USB Dongle)
> > > +
> > > +  Corsair AX1500i (by builtin Corsair Link USB Dongle)
> > > +
> > > +  Corsair AX1600i
> > > +
> > >    Corsair HX550i
> > >
> > >    Corsair HX650i
> > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > > index 716df51edc87..3c059fc23cd6 100644
> > > --- a/drivers/hwmon/Kconfig
> > > +++ b/drivers/hwmon/Kconfig
> > > @@ -453,11 +453,12 @@ config SENSORS_CORSAIR_PSU
> > >         tristate "Corsair PSU HID controller"
> > >         depends on HID
> > >         help
> > > -         If you say yes here you get support for Corsair PSUs with a HID
> > > +         If you say yes here you get support for Corsair PSUs with an USB HID
> > >           interface.
> > >           Currently this driver supports the (RM/HX)550i, (RM/HX)650i,
> > > -         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i and HX1200i power supplies
> > > -         by Corsair.
> > > +         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i, HX1200i and AX1600i power
> > > +         supplies by Corsair. The AX760i, AX860i, AX1200i and AX1500i
> > > +         power supplies are supported through the Corsair Link USB Dongle.
> > >
> > >           This driver can also be built as a module. If so, the module
> > >           will be called corsair-psu.
> > > diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> > > index 99494056f4bd..0146dda3e2c3 100644
> > > --- a/drivers/hwmon/corsair-psu.c
> > > +++ b/drivers/hwmon/corsair-psu.c
> > > @@ -571,17 +571,28 @@ static int corsairpsu_raw_event(struct hid_device *hdev, struct
> > > hid_report *repo }
> > >
> > >  static const struct hid_device_id corsairpsu_idtable[] = {
> > > -       { HID_USB_DEVICE(0x1b1c, 0x1c03) }, /* Corsair HX550i */
> > > -       { HID_USB_DEVICE(0x1b1c, 0x1c04) }, /* Corsair HX650i */
> > > -       { HID_USB_DEVICE(0x1b1c, 0x1c05) }, /* Corsair HX750i */
> > > -       { HID_USB_DEVICE(0x1b1c, 0x1c06) }, /* Corsair HX850i */
> > > -       { HID_USB_DEVICE(0x1b1c, 0x1c07) }, /* Corsair HX1000i */
> > > -       { HID_USB_DEVICE(0x1b1c, 0x1c08) }, /* Corsair HX1200i */
> > > -       { HID_USB_DEVICE(0x1b1c, 0x1c09) }, /* Corsair RM550i */
> > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0a) }, /* Corsair RM650i */
> > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0b) }, /* Corsair RM750i */
> > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0c) }, /* Corsair RM850i */
> > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0d) }, /* Corsair RM1000i */
> > > +       /*
> > > +        * The Corsair USB/COM Dongles appear in at least 3 different revisions, where rev 1
> > > and 2
> > > +        * are commonly used with the AX760i, AX860i and AX1200i, while rev3 is rarely seen with
> > > +        * these PSUs. Rev3 is also build into the AX1500i, while the AX1600i is the first PSU
> > > of
> > > +        * this series which has an unique usb hid id. Though, the actual device name is part of
> > > +        * the HID message protocol, so it doesn't matter which dongle is connected.
> > > +        */
> > > +       { HID_USB_DEVICE(0x1B1C, 0x1C00) }, /* Corsair Link USB/COM Dongle rev1 */
> > > +       { HID_USB_DEVICE(0x1B1C, 0x1C01) }, /* Corsair Link USB/COM Dongle rev2 */
> > > +       { HID_USB_DEVICE(0x1B1C, 0x1C02) }, /* Corsair Link USB/COM Dongle rev3 (AX1500i) */
> > > +       { HID_USB_DEVICE(0x1B1C, 0x1C03) }, /* Corsair HX550i */
> > > +       { HID_USB_DEVICE(0x1B1C, 0x1C04) }, /* Corsair HX650i */
> > > +       { HID_USB_DEVICE(0x1B1C, 0x1C05) }, /* Corsair HX750i */
> > > +       { HID_USB_DEVICE(0x1B1C, 0x1C06) }, /* Corsair HX850i */
> > > +       { HID_USB_DEVICE(0x1B1C, 0x1C07) }, /* Corsair HX1000i */
> > > +       { HID_USB_DEVICE(0x1B1C, 0x1C08) }, /* Corsair HX1200i */
> > > +       { HID_USB_DEVICE(0x1B1C, 0x1C09) }, /* Corsair RM550i */
> > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0A) }, /* Corsair RM650i */
> > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0B) }, /* Corsair RM750i */
> > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0C) }, /* Corsair RM850i */
> > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0D) }, /* Corsair RM1000i */
> > > +       { HID_USB_DEVICE(0x1B1C, 0x1C11) }, /* Corsair AX1600i */
> > >         { },
> > >  };
> > >  MODULE_DEVICE_TABLE(hid, corsairpsu_idtable);
> > > --
> > > 2.29.2
> > >
>
Wilken Gottwalt Nov. 29, 2020, 6:36 a.m. UTC | #5
On Sat, 28 Nov 2020 17:21:40 -0300
Jonas Malaco <jonas@protocubo.io> wrote:

> On Sat, Nov 28, 2020 at 7:35 AM Wilken Gottwalt
> <wilken.gottwalt@posteo.net> wrote:
> >
> > On Sat, 28 Nov 2020 02:37:38 -0300
> > Jonas Malaco <jonas@protocubo.io> wrote:
> >
> > > On Thu, Nov 26, 2020 at 8:43 AM Wilken Gottwalt
> > > <wilken.gottwalt@posteo.net> wrote:
> > > >
> > > > Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
> > > > AX1500i and AX1600i. The first 3 power supplies are supported through
> > > > the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
> > > > converter especially made for the COM ports of these power supplies.
> > > > There are 3 known revisions of these adapters. The AX1500i power supply
> > > > has revision 3 built into the case and AX1600i is the only one in that
> > > > series, which has an unique usb hid id like the RM/RX series.
> > >
> > > Can I ask what AXi power supplies were tested?
> > >
> > > I ask because, based on the user-space implementations I am aware of,
> > > the AXi dongle protocol appears to be different from the RMi/HXi series.
> >
> > I was not able to test this against the AX power supplies, they are really
> > hard to find (and are far to expensive). But I went through all these tools
> > and stuck to the most common commands, which all 3 series support. Not every
> > series supports all commands (there also seem to be different firmwares in
> > the micro-conrollers). But this is fine, some sensors will show up as N/A.
> > Even my HX850i does not support all commands covered in this driver.
> 
> I think the similarities come from all using wrappers over the PMBus
> interface to the voltage controller.  But I am not sure the wrapping
> protocols are identical.
> 
> For example, cpsumon shows significantly more things going on during a
> read than what is needed for the RMi/HXi series.[1]
> 
> [1] https://github.com/ka87/cpsumon/blob/fd639684d7f9/libcpsumon/src/cpsumon.c#L213-L231
> 
> 
> >
> > > AXi dongle:
> > >  - https://github.com/ka87/cpsumon
> >
> > This tool made me to consider including the AX series, because it uses some
> > of the same commands on the AX760i, AX860i, AX1200i and AX1500i. But it is
> > a usb-serial tool only. But it was nice to know, that the commands are mostly
> > the same. I left out all the commands for configuring, PCIe power rails,
> > efficiency and others which do not really belong into hwmon.
> >
> > > RMi/HXi:
> > >  - https://github.com/jonasmalacofilho/liquidctl
> > >  - https://github.com/audiohacked/OpenCorsairLink
> >
> > This tool made me include the AX series, because it uses the rmi protocol
> > component for the rmi driver (RM/HX series) and the corsair dongles.
> 
> The corsairlink_driver_dongle has no implementations for reading sensor
> data (compare that with the corsairlink_driver_rmi).[2][3]  There is
> also no code that actually tries to read (write) from (to) the device
> using that dongle driver.[4]
> 
> I also looked at a few of the issues, and all of the ones I read
> mentioned AXi support being under development, and the hypothesis of the
> AXi series being compatible with the RMi/HXi code still remaining to be
> confirmed.
> 
> [2] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/dongle.c#L33-L39
> [3] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/rmi.c#L33-L57
> [4] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/main.c#L106
> 
> 
> >
> > >  - https://github.com/notaz/corsairmi
> >
> > This one covers only some HX/RM PSUs, but is uses the rawhid access which
> > made me looking up the actual usb chips/bridges Corsair uses.
> >
> > >
> > > One additional concern is that the non-HID AXi dongles may only have bulk
> > > USB endpoints, and this is a HID driver.[1]
> >
> > You are right, in the case of the dongles it could be different. But I did
> > some research on Corsair usb driven devices and they really like to stick to
> > the cp210x, which is an usb hid bridge. The commit
> > b9326057a3d8447f5d2e74a7b521ccf21add2ec0 actually covers two Corsair USB
> > dongles as a cp210x device. So it is very likely that all Corsair PSUs with
> > such an interface/dongle use usb hid. But I'm completely open to get proven
> > wrong. Actually I really would like to see this tested by people who have
> > access to the more rare devices.
> 
> I could be wrong (and I am sorry for the noise if that is the case), but
> as far as I can see the cp210x does not create a HID device.

No no, this is fine. It really helps if some more people are looking into this.
I wish I had access to at least one of the later models (AX1500i/AX1600i), I
make mistakes from time to time. And it really doesn't help that Corsair changes
single devices in the same product line by firmware update. The AX1600i seems to
be the only one, which uses exactly the same protocol like the RM/HX series, but
is missing the actual usb hid part. But there seems to be a firmware where the
usb hid part was available for a short time. So, what to do? Remove the AXi part
completely or keep only the AX1600i?

Guenter, what would you suggest?

> Thanks again,
> Jonas
> 
> 
> >
> > > Thanks,
> > > Jonas
> > >
> > > [1] https://github.com/ka87/cpsumon/issues/4
> >
> > Yes ... that one. The last revision of the dongle could indeed be a problem.
> > But I'm not really sure what is described here. The last commenter is actually
> > the one who provided the cp210x patch mentioned up there. The problem here is,
> > the AX1500i has both connectors, USB and that other one. I call it the other
> > one because it is the only PSU where it is labeled I2C COMM instead of COMM
> > only. But at the same time this tools uses the same commands for this PSU.
> >
> > So, only some real hardware tests will show.
> >
> > Greetings,
> > Wilken
> >
> > > >
> > > > The patch also changes the usb hid ids to use upper case letters to be
> > > > consistent with the rest of the hex numbers in the driver and updates
> > > > the hwmon documentation.
> > > >
> > > > This patch adds:
> > > > - hwmon/corsair-psu documentation update
> > > > - corsair-psu driver update
> > > >
> > > > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> > > > ---
> > > >  Documentation/hwmon/corsair-psu.rst | 10 +++++++++
> > > >  drivers/hwmon/Kconfig               |  7 +++---
> > > >  drivers/hwmon/corsair-psu.c         | 33 +++++++++++++++++++----------
> > > >  3 files changed, 36 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/Documentation/hwmon/corsair-psu.rst b/Documentation/hwmon/corsair-psu.rst
> > > > index 396b95c9a76a..6227e9046d73 100644
> > > > --- a/Documentation/hwmon/corsair-psu.rst
> > > > +++ b/Documentation/hwmon/corsair-psu.rst
> > > > @@ -7,6 +7,16 @@ Supported devices:
> > > >
> > > >  * Corsair Power Supplies
> > > >
> > > > +  Corsair AX760i (by Corsair Link USB Dongle)
> > > > +
> > > > +  Corsair AX860i (by Corsair Link USB Dongle)
> > > > +
> > > > +  Corsair AX1200i (by Corsair Link USB Dongle)
> > > > +
> > > > +  Corsair AX1500i (by builtin Corsair Link USB Dongle)
> > > > +
> > > > +  Corsair AX1600i
> > > > +
> > > >    Corsair HX550i
> > > >
> > > >    Corsair HX650i
> > > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > > > index 716df51edc87..3c059fc23cd6 100644
> > > > --- a/drivers/hwmon/Kconfig
> > > > +++ b/drivers/hwmon/Kconfig
> > > > @@ -453,11 +453,12 @@ config SENSORS_CORSAIR_PSU
> > > >         tristate "Corsair PSU HID controller"
> > > >         depends on HID
> > > >         help
> > > > -         If you say yes here you get support for Corsair PSUs with a HID
> > > > +         If you say yes here you get support for Corsair PSUs with an USB HID
> > > >           interface.
> > > >           Currently this driver supports the (RM/HX)550i, (RM/HX)650i,
> > > > -         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i and HX1200i power supplies
> > > > -         by Corsair.
> > > > +         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i, HX1200i and AX1600i power
> > > > +         supplies by Corsair. The AX760i, AX860i, AX1200i and AX1500i
> > > > +         power supplies are supported through the Corsair Link USB Dongle.
> > > >
> > > >           This driver can also be built as a module. If so, the module
> > > >           will be called corsair-psu.
> > > > diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> > > > index 99494056f4bd..0146dda3e2c3 100644
> > > > --- a/drivers/hwmon/corsair-psu.c
> > > > +++ b/drivers/hwmon/corsair-psu.c
> > > > @@ -571,17 +571,28 @@ static int corsairpsu_raw_event(struct hid_device *hdev, struct
> > > > hid_report *repo }
> > > >
> > > >  static const struct hid_device_id corsairpsu_idtable[] = {
> > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c03) }, /* Corsair HX550i */
> > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c04) }, /* Corsair HX650i */
> > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c05) }, /* Corsair HX750i */
> > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c06) }, /* Corsair HX850i */
> > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c07) }, /* Corsair HX1000i */
> > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c08) }, /* Corsair HX1200i */
> > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c09) }, /* Corsair RM550i */
> > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0a) }, /* Corsair RM650i */
> > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0b) }, /* Corsair RM750i */
> > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0c) }, /* Corsair RM850i */
> > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0d) }, /* Corsair RM1000i */
> > > > +       /*
> > > > +        * The Corsair USB/COM Dongles appear in at least 3 different revisions, where rev 1
> > > > and 2
> > > > +        * are commonly used with the AX760i, AX860i and AX1200i, while rev3 is rarely seen
> > > > with
> > > > +        * these PSUs. Rev3 is also build into the AX1500i, while the AX1600i is the first
> > > > PSU of
> > > > +        * this series which has an unique usb hid id. Though, the actual device name is
> > > > part of
> > > > +        * the HID message protocol, so it doesn't matter which dongle is connected.
> > > > +        */
> > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C00) }, /* Corsair Link USB/COM Dongle rev1 */
> > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C01) }, /* Corsair Link USB/COM Dongle rev2 */
> > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C02) }, /* Corsair Link USB/COM Dongle rev3 (AX1500i) */
> > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C03) }, /* Corsair HX550i */
> > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C04) }, /* Corsair HX650i */
> > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C05) }, /* Corsair HX750i */
> > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C06) }, /* Corsair HX850i */
> > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C07) }, /* Corsair HX1000i */
> > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C08) }, /* Corsair HX1200i */
> > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C09) }, /* Corsair RM550i */
> > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0A) }, /* Corsair RM650i */
> > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0B) }, /* Corsair RM750i */
> > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0C) }, /* Corsair RM850i */
> > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0D) }, /* Corsair RM1000i */
> > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C11) }, /* Corsair AX1600i */
> > > >         { },
> > > >  };
> > > >  MODULE_DEVICE_TABLE(hid, corsairpsu_idtable);
> > > > --
> > > > 2.29.2
> > > >
> >
Guenter Roeck Nov. 29, 2020, 1 p.m. UTC | #6
On Sun, Nov 29, 2020 at 07:36:18AM +0100, Wilken Gottwalt wrote:
> On Sat, 28 Nov 2020 17:21:40 -0300
> Jonas Malaco <jonas@protocubo.io> wrote:
> 
> > On Sat, Nov 28, 2020 at 7:35 AM Wilken Gottwalt
> > <wilken.gottwalt@posteo.net> wrote:
> > >
> > > On Sat, 28 Nov 2020 02:37:38 -0300
> > > Jonas Malaco <jonas@protocubo.io> wrote:
> > >
> > > > On Thu, Nov 26, 2020 at 8:43 AM Wilken Gottwalt
> > > > <wilken.gottwalt@posteo.net> wrote:
> > > > >
> > > > > Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
> > > > > AX1500i and AX1600i. The first 3 power supplies are supported through
> > > > > the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
> > > > > converter especially made for the COM ports of these power supplies.
> > > > > There are 3 known revisions of these adapters. The AX1500i power supply
> > > > > has revision 3 built into the case and AX1600i is the only one in that
> > > > > series, which has an unique usb hid id like the RM/RX series.
> > > >
> > > > Can I ask what AXi power supplies were tested?
> > > >
> > > > I ask because, based on the user-space implementations I am aware of,
> > > > the AXi dongle protocol appears to be different from the RMi/HXi series.
> > >
> > > I was not able to test this against the AX power supplies, they are really
> > > hard to find (and are far to expensive). But I went through all these tools
> > > and stuck to the most common commands, which all 3 series support. Not every
> > > series supports all commands (there also seem to be different firmwares in
> > > the micro-conrollers). But this is fine, some sensors will show up as N/A.
> > > Even my HX850i does not support all commands covered in this driver.
> > 
> > I think the similarities come from all using wrappers over the PMBus
> > interface to the voltage controller.  But I am not sure the wrapping
> > protocols are identical.
> > 
> > For example, cpsumon shows significantly more things going on during a
> > read than what is needed for the RMi/HXi series.[1]
> > 
> > [1] https://github.com/ka87/cpsumon/blob/fd639684d7f9/libcpsumon/src/cpsumon.c#L213-L231
> > 
> > 
> > >
> > > > AXi dongle:
> > > >  - https://github.com/ka87/cpsumon
> > >
> > > This tool made me to consider including the AX series, because it uses some
> > > of the same commands on the AX760i, AX860i, AX1200i and AX1500i. But it is
> > > a usb-serial tool only. But it was nice to know, that the commands are mostly
> > > the same. I left out all the commands for configuring, PCIe power rails,
> > > efficiency and others which do not really belong into hwmon.
> > >
> > > > RMi/HXi:
> > > >  - https://github.com/jonasmalacofilho/liquidctl
> > > >  - https://github.com/audiohacked/OpenCorsairLink
> > >
> > > This tool made me include the AX series, because it uses the rmi protocol
> > > component for the rmi driver (RM/HX series) and the corsair dongles.
> > 
> > The corsairlink_driver_dongle has no implementations for reading sensor
> > data (compare that with the corsairlink_driver_rmi).[2][3]  There is
> > also no code that actually tries to read (write) from (to) the device
> > using that dongle driver.[4]
> > 
> > I also looked at a few of the issues, and all of the ones I read
> > mentioned AXi support being under development, and the hypothesis of the
> > AXi series being compatible with the RMi/HXi code still remaining to be
> > confirmed.
> > 
> > [2] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/dongle.c#L33-L39
> > [3] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/rmi.c#L33-L57
> > [4] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/main.c#L106
> > 
> > 
> > >
> > > >  - https://github.com/notaz/corsairmi
> > >
> > > This one covers only some HX/RM PSUs, but is uses the rawhid access which
> > > made me looking up the actual usb chips/bridges Corsair uses.
> > >
> > > >
> > > > One additional concern is that the non-HID AXi dongles may only have bulk
> > > > USB endpoints, and this is a HID driver.[1]
> > >
> > > You are right, in the case of the dongles it could be different. But I did
> > > some research on Corsair usb driven devices and they really like to stick to
> > > the cp210x, which is an usb hid bridge. The commit
> > > b9326057a3d8447f5d2e74a7b521ccf21add2ec0 actually covers two Corsair USB
> > > dongles as a cp210x device. So it is very likely that all Corsair PSUs with
> > > such an interface/dongle use usb hid. But I'm completely open to get proven
> > > wrong. Actually I really would like to see this tested by people who have
> > > access to the more rare devices.
> > 
> > I could be wrong (and I am sorry for the noise if that is the case), but
> > as far as I can see the cp210x does not create a HID device.
> 
> No no, this is fine. It really helps if some more people are looking into this.
> I wish I had access to at least one of the later models (AX1500i/AX1600i), I
> make mistakes from time to time. And it really doesn't help that Corsair changes
> single devices in the same product line by firmware update. The AX1600i seems to
> be the only one, which uses exactly the same protocol like the RM/HX series, but
> is missing the actual usb hid part. But there seems to be a firmware where the
> usb hid part was available for a short time. So, what to do? Remove the AXi part
> completely or keep only the AX1600i?
> 
> Guenter, what would you suggest?
> 
I'd suggest to remove it completely, and explain the reason. Anything else will
just create trouble with people demanding to know why their power supply is not
supported even though it is listed as supported. And, believe me, you don't want
to be on the receiving side of those complaints. The Internet is much less
friendly nowadays than it used to be.

Guenter

> > Thanks again,
> > Jonas
> > 
> > 
> > >
> > > > Thanks,
> > > > Jonas
> > > >
> > > > [1] https://github.com/ka87/cpsumon/issues/4
> > >
> > > Yes ... that one. The last revision of the dongle could indeed be a problem.
> > > But I'm not really sure what is described here. The last commenter is actually
> > > the one who provided the cp210x patch mentioned up there. The problem here is,
> > > the AX1500i has both connectors, USB and that other one. I call it the other
> > > one because it is the only PSU where it is labeled I2C COMM instead of COMM
> > > only. But at the same time this tools uses the same commands for this PSU.
> > >
> > > So, only some real hardware tests will show.
> > >
> > > Greetings,
> > > Wilken
> > >
> > > > >
> > > > > The patch also changes the usb hid ids to use upper case letters to be
> > > > > consistent with the rest of the hex numbers in the driver and updates
> > > > > the hwmon documentation.
> > > > >
> > > > > This patch adds:
> > > > > - hwmon/corsair-psu documentation update
> > > > > - corsair-psu driver update
> > > > >
> > > > > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> > > > > ---
> > > > >  Documentation/hwmon/corsair-psu.rst | 10 +++++++++
> > > > >  drivers/hwmon/Kconfig               |  7 +++---
> > > > >  drivers/hwmon/corsair-psu.c         | 33 +++++++++++++++++++----------
> > > > >  3 files changed, 36 insertions(+), 14 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/hwmon/corsair-psu.rst b/Documentation/hwmon/corsair-psu.rst
> > > > > index 396b95c9a76a..6227e9046d73 100644
> > > > > --- a/Documentation/hwmon/corsair-psu.rst
> > > > > +++ b/Documentation/hwmon/corsair-psu.rst
> > > > > @@ -7,6 +7,16 @@ Supported devices:
> > > > >
> > > > >  * Corsair Power Supplies
> > > > >
> > > > > +  Corsair AX760i (by Corsair Link USB Dongle)
> > > > > +
> > > > > +  Corsair AX860i (by Corsair Link USB Dongle)
> > > > > +
> > > > > +  Corsair AX1200i (by Corsair Link USB Dongle)
> > > > > +
> > > > > +  Corsair AX1500i (by builtin Corsair Link USB Dongle)
> > > > > +
> > > > > +  Corsair AX1600i
> > > > > +
> > > > >    Corsair HX550i
> > > > >
> > > > >    Corsair HX650i
> > > > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > > > > index 716df51edc87..3c059fc23cd6 100644
> > > > > --- a/drivers/hwmon/Kconfig
> > > > > +++ b/drivers/hwmon/Kconfig
> > > > > @@ -453,11 +453,12 @@ config SENSORS_CORSAIR_PSU
> > > > >         tristate "Corsair PSU HID controller"
> > > > >         depends on HID
> > > > >         help
> > > > > -         If you say yes here you get support for Corsair PSUs with a HID
> > > > > +         If you say yes here you get support for Corsair PSUs with an USB HID
> > > > >           interface.
> > > > >           Currently this driver supports the (RM/HX)550i, (RM/HX)650i,
> > > > > -         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i and HX1200i power supplies
> > > > > -         by Corsair.
> > > > > +         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i, HX1200i and AX1600i power
> > > > > +         supplies by Corsair. The AX760i, AX860i, AX1200i and AX1500i
> > > > > +         power supplies are supported through the Corsair Link USB Dongle.
> > > > >
> > > > >           This driver can also be built as a module. If so, the module
> > > > >           will be called corsair-psu.
> > > > > diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> > > > > index 99494056f4bd..0146dda3e2c3 100644
> > > > > --- a/drivers/hwmon/corsair-psu.c
> > > > > +++ b/drivers/hwmon/corsair-psu.c
> > > > > @@ -571,17 +571,28 @@ static int corsairpsu_raw_event(struct hid_device *hdev, struct
> > > > > hid_report *repo }
> > > > >
> > > > >  static const struct hid_device_id corsairpsu_idtable[] = {
> > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c03) }, /* Corsair HX550i */
> > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c04) }, /* Corsair HX650i */
> > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c05) }, /* Corsair HX750i */
> > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c06) }, /* Corsair HX850i */
> > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c07) }, /* Corsair HX1000i */
> > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c08) }, /* Corsair HX1200i */
> > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c09) }, /* Corsair RM550i */
> > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0a) }, /* Corsair RM650i */
> > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0b) }, /* Corsair RM750i */
> > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0c) }, /* Corsair RM850i */
> > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0d) }, /* Corsair RM1000i */
> > > > > +       /*
> > > > > +        * The Corsair USB/COM Dongles appear in at least 3 different revisions, where rev 1
> > > > > and 2
> > > > > +        * are commonly used with the AX760i, AX860i and AX1200i, while rev3 is rarely seen
> > > > > with
> > > > > +        * these PSUs. Rev3 is also build into the AX1500i, while the AX1600i is the first
> > > > > PSU of
> > > > > +        * this series which has an unique usb hid id. Though, the actual device name is
> > > > > part of
> > > > > +        * the HID message protocol, so it doesn't matter which dongle is connected.
> > > > > +        */
> > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C00) }, /* Corsair Link USB/COM Dongle rev1 */
> > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C01) }, /* Corsair Link USB/COM Dongle rev2 */
> > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C02) }, /* Corsair Link USB/COM Dongle rev3 (AX1500i) */
> > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C03) }, /* Corsair HX550i */
> > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C04) }, /* Corsair HX650i */
> > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C05) }, /* Corsair HX750i */
> > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C06) }, /* Corsair HX850i */
> > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C07) }, /* Corsair HX1000i */
> > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C08) }, /* Corsair HX1200i */
> > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C09) }, /* Corsair RM550i */
> > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0A) }, /* Corsair RM650i */
> > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0B) }, /* Corsair RM750i */
> > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0C) }, /* Corsair RM850i */
> > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0D) }, /* Corsair RM1000i */
> > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C11) }, /* Corsair AX1600i */
> > > > >         { },
> > > > >  };
> > > > >  MODULE_DEVICE_TABLE(hid, corsairpsu_idtable);
> > > > > --
> > > > > 2.29.2
> > > > >
> > >
>
Wilken Gottwalt Nov. 29, 2020, 3:54 p.m. UTC | #7
On Sun, 29 Nov 2020 05:00:49 -0800
Guenter Roeck <linux@roeck-us.net> wrote:

> On Sun, Nov 29, 2020 at 07:36:18AM +0100, Wilken Gottwalt wrote:
> > On Sat, 28 Nov 2020 17:21:40 -0300
> > Jonas Malaco <jonas@protocubo.io> wrote:
> > 
> > > On Sat, Nov 28, 2020 at 7:35 AM Wilken Gottwalt
> > > <wilken.gottwalt@posteo.net> wrote:
> > > >
> > > > On Sat, 28 Nov 2020 02:37:38 -0300
> > > > Jonas Malaco <jonas@protocubo.io> wrote:
> > > >
> > > > > On Thu, Nov 26, 2020 at 8:43 AM Wilken Gottwalt
> > > > > <wilken.gottwalt@posteo.net> wrote:
> > > > > >
> > > > > > Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
> > > > > > AX1500i and AX1600i. The first 3 power supplies are supported through
> > > > > > the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
> > > > > > converter especially made for the COM ports of these power supplies.
> > > > > > There are 3 known revisions of these adapters. The AX1500i power supply
> > > > > > has revision 3 built into the case and AX1600i is the only one in that
> > > > > > series, which has an unique usb hid id like the RM/RX series.
> > > > >
> > > > > Can I ask what AXi power supplies were tested?
> > > > >
> > > > > I ask because, based on the user-space implementations I am aware of,
> > > > > the AXi dongle protocol appears to be different from the RMi/HXi series.
> > > >
> > > > I was not able to test this against the AX power supplies, they are really
> > > > hard to find (and are far to expensive). But I went through all these tools
> > > > and stuck to the most common commands, which all 3 series support. Not every
> > > > series supports all commands (there also seem to be different firmwares in
> > > > the micro-conrollers). But this is fine, some sensors will show up as N/A.
> > > > Even my HX850i does not support all commands covered in this driver.
> > > 
> > > I think the similarities come from all using wrappers over the PMBus
> > > interface to the voltage controller.  But I am not sure the wrapping
> > > protocols are identical.
> > > 
> > > For example, cpsumon shows significantly more things going on during a
> > > read than what is needed for the RMi/HXi series.[1]
> > > 
> > > [1] https://github.com/ka87/cpsumon/blob/fd639684d7f9/libcpsumon/src/cpsumon.c#L213-L231
> > > 
> > > 
> > > >
> > > > > AXi dongle:
> > > > >  - https://github.com/ka87/cpsumon
> > > >
> > > > This tool made me to consider including the AX series, because it uses some
> > > > of the same commands on the AX760i, AX860i, AX1200i and AX1500i. But it is
> > > > a usb-serial tool only. But it was nice to know, that the commands are mostly
> > > > the same. I left out all the commands for configuring, PCIe power rails,
> > > > efficiency and others which do not really belong into hwmon.
> > > >
> > > > > RMi/HXi:
> > > > >  - https://github.com/jonasmalacofilho/liquidctl
> > > > >  - https://github.com/audiohacked/OpenCorsairLink
> > > >
> > > > This tool made me include the AX series, because it uses the rmi protocol
> > > > component for the rmi driver (RM/HX series) and the corsair dongles.
> > > 
> > > The corsairlink_driver_dongle has no implementations for reading sensor
> > > data (compare that with the corsairlink_driver_rmi).[2][3]  There is
> > > also no code that actually tries to read (write) from (to) the device
> > > using that dongle driver.[4]
> > > 
> > > I also looked at a few of the issues, and all of the ones I read
> > > mentioned AXi support being under development, and the hypothesis of the
> > > AXi series being compatible with the RMi/HXi code still remaining to be
> > > confirmed.
> > > 
> > > [2] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/dongle.c#L33-L39
> > > [3] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/rmi.c#L33-L57
> > > [4] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/main.c#L106
> > > 
> > > 
> > > >
> > > > >  - https://github.com/notaz/corsairmi
> > > >
> > > > This one covers only some HX/RM PSUs, but is uses the rawhid access which
> > > > made me looking up the actual usb chips/bridges Corsair uses.
> > > >
> > > > >
> > > > > One additional concern is that the non-HID AXi dongles may only have bulk
> > > > > USB endpoints, and this is a HID driver.[1]
> > > >
> > > > You are right, in the case of the dongles it could be different. But I did
> > > > some research on Corsair usb driven devices and they really like to stick to
> > > > the cp210x, which is an usb hid bridge. The commit
> > > > b9326057a3d8447f5d2e74a7b521ccf21add2ec0 actually covers two Corsair USB
> > > > dongles as a cp210x device. So it is very likely that all Corsair PSUs with
> > > > such an interface/dongle use usb hid. But I'm completely open to get proven
> > > > wrong. Actually I really would like to see this tested by people who have
> > > > access to the more rare devices.
> > > 
> > > I could be wrong (and I am sorry for the noise if that is the case), but
> > > as far as I can see the cp210x does not create a HID device.
> > 
> > No no, this is fine. It really helps if some more people are looking into this.
> > I wish I had access to at least one of the later models (AX1500i/AX1600i), I
> > make mistakes from time to time. And it really doesn't help that Corsair changes
> > single devices in the same product line by firmware update. The AX1600i seems to
> > be the only one, which uses exactly the same protocol like the RM/HX series, but
> > is missing the actual usb hid part. But there seems to be a firmware where the
> > usb hid part was available for a short time. So, what to do? Remove the AXi part
> > completely or keep only the AX1600i?
> > 
> > Guenter, what would you suggest?
> > 
> I'd suggest to remove it completely, and explain the reason. Anything else will
> just create trouble with people demanding to know why their power supply is not
> supported even though it is listed as supported. And, believe me, you don't want
> to be on the receiving side of those complaints. The Internet is much less
> friendly nowadays than it used to be.

So how is the procedure for this? Just revert it and make a common patch out of
it with a proper explanation?

And yeah, I know exactly what you mean. I remember very well how the "internet"
got it first ugly hit in the 90s with the upcoming of Flash ... and then the
"social media". :D Thanks for your help.

greetings,
Wilken

> Guenter
> 
> > > Thanks again,
> > > Jonas
> > > 
> > > 
> > > >
> > > > > Thanks,
> > > > > Jonas
> > > > >
> > > > > [1] https://github.com/ka87/cpsumon/issues/4
> > > >
> > > > Yes ... that one. The last revision of the dongle could indeed be a problem.
> > > > But I'm not really sure what is described here. The last commenter is actually
> > > > the one who provided the cp210x patch mentioned up there. The problem here is,
> > > > the AX1500i has both connectors, USB and that other one. I call it the other
> > > > one because it is the only PSU where it is labeled I2C COMM instead of COMM
> > > > only. But at the same time this tools uses the same commands for this PSU.
> > > >
> > > > So, only some real hardware tests will show.
> > > >
> > > > Greetings,
> > > > Wilken
> > > >
> > > > > >
> > > > > > The patch also changes the usb hid ids to use upper case letters to be
> > > > > > consistent with the rest of the hex numbers in the driver and updates
> > > > > > the hwmon documentation.
> > > > > >
> > > > > > This patch adds:
> > > > > > - hwmon/corsair-psu documentation update
> > > > > > - corsair-psu driver update
> > > > > >
> > > > > > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> > > > > > ---
> > > > > >  Documentation/hwmon/corsair-psu.rst | 10 +++++++++
> > > > > >  drivers/hwmon/Kconfig               |  7 +++---
> > > > > >  drivers/hwmon/corsair-psu.c         | 33 +++++++++++++++++++----------
> > > > > >  3 files changed, 36 insertions(+), 14 deletions(-)
> > > > > >
> > > > > > diff --git a/Documentation/hwmon/corsair-psu.rst b/Documentation/hwmon/corsair-psu.rst
> > > > > > index 396b95c9a76a..6227e9046d73 100644
> > > > > > --- a/Documentation/hwmon/corsair-psu.rst
> > > > > > +++ b/Documentation/hwmon/corsair-psu.rst
> > > > > > @@ -7,6 +7,16 @@ Supported devices:
> > > > > >
> > > > > >  * Corsair Power Supplies
> > > > > >
> > > > > > +  Corsair AX760i (by Corsair Link USB Dongle)
> > > > > > +
> > > > > > +  Corsair AX860i (by Corsair Link USB Dongle)
> > > > > > +
> > > > > > +  Corsair AX1200i (by Corsair Link USB Dongle)
> > > > > > +
> > > > > > +  Corsair AX1500i (by builtin Corsair Link USB Dongle)
> > > > > > +
> > > > > > +  Corsair AX1600i
> > > > > > +
> > > > > >    Corsair HX550i
> > > > > >
> > > > > >    Corsair HX650i
> > > > > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > > > > > index 716df51edc87..3c059fc23cd6 100644
> > > > > > --- a/drivers/hwmon/Kconfig
> > > > > > +++ b/drivers/hwmon/Kconfig
> > > > > > @@ -453,11 +453,12 @@ config SENSORS_CORSAIR_PSU
> > > > > >         tristate "Corsair PSU HID controller"
> > > > > >         depends on HID
> > > > > >         help
> > > > > > -         If you say yes here you get support for Corsair PSUs with a HID
> > > > > > +         If you say yes here you get support for Corsair PSUs with an USB HID
> > > > > >           interface.
> > > > > >           Currently this driver supports the (RM/HX)550i, (RM/HX)650i,
> > > > > > -         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i and HX1200i power supplies
> > > > > > -         by Corsair.
> > > > > > +         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i, HX1200i and AX1600i power
> > > > > > +         supplies by Corsair. The AX760i, AX860i, AX1200i and AX1500i
> > > > > > +         power supplies are supported through the Corsair Link USB Dongle.
> > > > > >
> > > > > >           This driver can also be built as a module. If so, the module
> > > > > >           will be called corsair-psu.
> > > > > > diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> > > > > > index 99494056f4bd..0146dda3e2c3 100644
> > > > > > --- a/drivers/hwmon/corsair-psu.c
> > > > > > +++ b/drivers/hwmon/corsair-psu.c
> > > > > > @@ -571,17 +571,28 @@ static int corsairpsu_raw_event(struct hid_device *hdev, struct
> > > > > > hid_report *repo }
> > > > > >
> > > > > >  static const struct hid_device_id corsairpsu_idtable[] = {
> > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c03) }, /* Corsair HX550i */
> > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c04) }, /* Corsair HX650i */
> > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c05) }, /* Corsair HX750i */
> > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c06) }, /* Corsair HX850i */
> > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c07) }, /* Corsair HX1000i */
> > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c08) }, /* Corsair HX1200i */
> > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c09) }, /* Corsair RM550i */
> > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0a) }, /* Corsair RM650i */
> > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0b) }, /* Corsair RM750i */
> > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0c) }, /* Corsair RM850i */
> > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0d) }, /* Corsair RM1000i */
> > > > > > +       /*
> > > > > > +        * The Corsair USB/COM Dongles appear in at least 3 different revisions, where
> > > > > > rev 1 and 2
> > > > > > +        * are commonly used with the AX760i, AX860i and AX1200i, while rev3 is rarely
> > > > > > seen with
> > > > > > +        * these PSUs. Rev3 is also build into the AX1500i, while the AX1600i is the
> > > > > > first PSU of
> > > > > > +        * this series which has an unique usb hid id. Though, the actual device name is
> > > > > > part of
> > > > > > +        * the HID message protocol, so it doesn't matter which dongle is connected.
> > > > > > +        */
> > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C00) }, /* Corsair Link USB/COM Dongle rev1 */
> > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C01) }, /* Corsair Link USB/COM Dongle rev2 */
> > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C02) }, /* Corsair Link USB/COM Dongle rev3
> > > > > > (AX1500i) */
> > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C03) }, /* Corsair HX550i */
> > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C04) }, /* Corsair HX650i */
> > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C05) }, /* Corsair HX750i */
> > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C06) }, /* Corsair HX850i */
> > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C07) }, /* Corsair HX1000i */
> > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C08) }, /* Corsair HX1200i */
> > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C09) }, /* Corsair RM550i */
> > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0A) }, /* Corsair RM650i */
> > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0B) }, /* Corsair RM750i */
> > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0C) }, /* Corsair RM850i */
> > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0D) }, /* Corsair RM1000i */
> > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C11) }, /* Corsair AX1600i */
> > > > > >         { },
> > > > > >  };
> > > > > >  MODULE_DEVICE_TABLE(hid, corsairpsu_idtable);
> > > > > > --
> > > > > > 2.29.2
> > > > > >
> > > >
> >
Guenter Roeck Nov. 29, 2020, 9:59 p.m. UTC | #8
On Sun, Nov 29, 2020 at 04:54:43PM +0100, Wilken Gottwalt wrote:
> On Sun, 29 Nov 2020 05:00:49 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
> 
> > On Sun, Nov 29, 2020 at 07:36:18AM +0100, Wilken Gottwalt wrote:
> > > On Sat, 28 Nov 2020 17:21:40 -0300
> > > Jonas Malaco <jonas@protocubo.io> wrote:
> > > 
> > > > On Sat, Nov 28, 2020 at 7:35 AM Wilken Gottwalt
> > > > <wilken.gottwalt@posteo.net> wrote:
> > > > >
> > > > > On Sat, 28 Nov 2020 02:37:38 -0300
> > > > > Jonas Malaco <jonas@protocubo.io> wrote:
> > > > >
> > > > > > On Thu, Nov 26, 2020 at 8:43 AM Wilken Gottwalt
> > > > > > <wilken.gottwalt@posteo.net> wrote:
> > > > > > >
> > > > > > > Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
> > > > > > > AX1500i and AX1600i. The first 3 power supplies are supported through
> > > > > > > the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
> > > > > > > converter especially made for the COM ports of these power supplies.
> > > > > > > There are 3 known revisions of these adapters. The AX1500i power supply
> > > > > > > has revision 3 built into the case and AX1600i is the only one in that
> > > > > > > series, which has an unique usb hid id like the RM/RX series.
> > > > > >
> > > > > > Can I ask what AXi power supplies were tested?
> > > > > >
> > > > > > I ask because, based on the user-space implementations I am aware of,
> > > > > > the AXi dongle protocol appears to be different from the RMi/HXi series.
> > > > >
> > > > > I was not able to test this against the AX power supplies, they are really
> > > > > hard to find (and are far to expensive). But I went through all these tools
> > > > > and stuck to the most common commands, which all 3 series support. Not every
> > > > > series supports all commands (there also seem to be different firmwares in
> > > > > the micro-conrollers). But this is fine, some sensors will show up as N/A.
> > > > > Even my HX850i does not support all commands covered in this driver.
> > > > 
> > > > I think the similarities come from all using wrappers over the PMBus
> > > > interface to the voltage controller.  But I am not sure the wrapping
> > > > protocols are identical.
> > > > 
> > > > For example, cpsumon shows significantly more things going on during a
> > > > read than what is needed for the RMi/HXi series.[1]
> > > > 
> > > > [1] https://github.com/ka87/cpsumon/blob/fd639684d7f9/libcpsumon/src/cpsumon.c#L213-L231
> > > > 
> > > > 
> > > > >
> > > > > > AXi dongle:
> > > > > >  - https://github.com/ka87/cpsumon
> > > > >
> > > > > This tool made me to consider including the AX series, because it uses some
> > > > > of the same commands on the AX760i, AX860i, AX1200i and AX1500i. But it is
> > > > > a usb-serial tool only. But it was nice to know, that the commands are mostly
> > > > > the same. I left out all the commands for configuring, PCIe power rails,
> > > > > efficiency and others which do not really belong into hwmon.
> > > > >
> > > > > > RMi/HXi:
> > > > > >  - https://github.com/jonasmalacofilho/liquidctl
> > > > > >  - https://github.com/audiohacked/OpenCorsairLink
> > > > >
> > > > > This tool made me include the AX series, because it uses the rmi protocol
> > > > > component for the rmi driver (RM/HX series) and the corsair dongles.
> > > > 
> > > > The corsairlink_driver_dongle has no implementations for reading sensor
> > > > data (compare that with the corsairlink_driver_rmi).[2][3]  There is
> > > > also no code that actually tries to read (write) from (to) the device
> > > > using that dongle driver.[4]
> > > > 
> > > > I also looked at a few of the issues, and all of the ones I read
> > > > mentioned AXi support being under development, and the hypothesis of the
> > > > AXi series being compatible with the RMi/HXi code still remaining to be
> > > > confirmed.
> > > > 
> > > > [2] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/dongle.c#L33-L39
> > > > [3] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/rmi.c#L33-L57
> > > > [4] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/main.c#L106
> > > > 
> > > > 
> > > > >
> > > > > >  - https://github.com/notaz/corsairmi
> > > > >
> > > > > This one covers only some HX/RM PSUs, but is uses the rawhid access which
> > > > > made me looking up the actual usb chips/bridges Corsair uses.
> > > > >
> > > > > >
> > > > > > One additional concern is that the non-HID AXi dongles may only have bulk
> > > > > > USB endpoints, and this is a HID driver.[1]
> > > > >
> > > > > You are right, in the case of the dongles it could be different. But I did
> > > > > some research on Corsair usb driven devices and they really like to stick to
> > > > > the cp210x, which is an usb hid bridge. The commit
> > > > > b9326057a3d8447f5d2e74a7b521ccf21add2ec0 actually covers two Corsair USB
> > > > > dongles as a cp210x device. So it is very likely that all Corsair PSUs with
> > > > > such an interface/dongle use usb hid. But I'm completely open to get proven
> > > > > wrong. Actually I really would like to see this tested by people who have
> > > > > access to the more rare devices.
> > > > 
> > > > I could be wrong (and I am sorry for the noise if that is the case), but
> > > > as far as I can see the cp210x does not create a HID device.
> > > 
> > > No no, this is fine. It really helps if some more people are looking into this.
> > > I wish I had access to at least one of the later models (AX1500i/AX1600i), I
> > > make mistakes from time to time. And it really doesn't help that Corsair changes
> > > single devices in the same product line by firmware update. The AX1600i seems to
> > > be the only one, which uses exactly the same protocol like the RM/HX series, but
> > > is missing the actual usb hid part. But there seems to be a firmware where the
> > > usb hid part was available for a short time. So, what to do? Remove the AXi part
> > > completely or keep only the AX1600i?
> > > 
> > > Guenter, what would you suggest?
> > > 
> > I'd suggest to remove it completely, and explain the reason. Anything else will
> > just create trouble with people demanding to know why their power supply is not
> > supported even though it is listed as supported. And, believe me, you don't want
> > to be on the receiving side of those complaints. The Internet is much less
> > friendly nowadays than it used to be.
> 
> So how is the procedure for this? Just revert it and make a common patch out of
> it with a proper explanation?
> 
> And yeah, I know exactly what you mean. I remember very well how the "internet"
> got it first ugly hit in the 90s with the upcoming of Flash ... and then the
> "social media". :D Thanks for your help.
> 

Can I just drop your previous patch and you send me another - more restrictive -
one ?

Thanks,
Guenter

> greetings,
> Wilken
> 
> > Guenter
> > 
> > > > Thanks again,
> > > > Jonas
> > > > 
> > > > 
> > > > >
> > > > > > Thanks,
> > > > > > Jonas
> > > > > >
> > > > > > [1] https://github.com/ka87/cpsumon/issues/4
> > > > >
> > > > > Yes ... that one. The last revision of the dongle could indeed be a problem.
> > > > > But I'm not really sure what is described here. The last commenter is actually
> > > > > the one who provided the cp210x patch mentioned up there. The problem here is,
> > > > > the AX1500i has both connectors, USB and that other one. I call it the other
> > > > > one because it is the only PSU where it is labeled I2C COMM instead of COMM
> > > > > only. But at the same time this tools uses the same commands for this PSU.
> > > > >
> > > > > So, only some real hardware tests will show.
> > > > >
> > > > > Greetings,
> > > > > Wilken
> > > > >
> > > > > > >
> > > > > > > The patch also changes the usb hid ids to use upper case letters to be
> > > > > > > consistent with the rest of the hex numbers in the driver and updates
> > > > > > > the hwmon documentation.
> > > > > > >
> > > > > > > This patch adds:
> > > > > > > - hwmon/corsair-psu documentation update
> > > > > > > - corsair-psu driver update
> > > > > > >
> > > > > > > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> > > > > > > ---
> > > > > > >  Documentation/hwmon/corsair-psu.rst | 10 +++++++++
> > > > > > >  drivers/hwmon/Kconfig               |  7 +++---
> > > > > > >  drivers/hwmon/corsair-psu.c         | 33 +++++++++++++++++++----------
> > > > > > >  3 files changed, 36 insertions(+), 14 deletions(-)
> > > > > > >
> > > > > > > diff --git a/Documentation/hwmon/corsair-psu.rst b/Documentation/hwmon/corsair-psu.rst
> > > > > > > index 396b95c9a76a..6227e9046d73 100644
> > > > > > > --- a/Documentation/hwmon/corsair-psu.rst
> > > > > > > +++ b/Documentation/hwmon/corsair-psu.rst
> > > > > > > @@ -7,6 +7,16 @@ Supported devices:
> > > > > > >
> > > > > > >  * Corsair Power Supplies
> > > > > > >
> > > > > > > +  Corsair AX760i (by Corsair Link USB Dongle)
> > > > > > > +
> > > > > > > +  Corsair AX860i (by Corsair Link USB Dongle)
> > > > > > > +
> > > > > > > +  Corsair AX1200i (by Corsair Link USB Dongle)
> > > > > > > +
> > > > > > > +  Corsair AX1500i (by builtin Corsair Link USB Dongle)
> > > > > > > +
> > > > > > > +  Corsair AX1600i
> > > > > > > +
> > > > > > >    Corsair HX550i
> > > > > > >
> > > > > > >    Corsair HX650i
> > > > > > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > > > > > > index 716df51edc87..3c059fc23cd6 100644
> > > > > > > --- a/drivers/hwmon/Kconfig
> > > > > > > +++ b/drivers/hwmon/Kconfig
> > > > > > > @@ -453,11 +453,12 @@ config SENSORS_CORSAIR_PSU
> > > > > > >         tristate "Corsair PSU HID controller"
> > > > > > >         depends on HID
> > > > > > >         help
> > > > > > > -         If you say yes here you get support for Corsair PSUs with a HID
> > > > > > > +         If you say yes here you get support for Corsair PSUs with an USB HID
> > > > > > >           interface.
> > > > > > >           Currently this driver supports the (RM/HX)550i, (RM/HX)650i,
> > > > > > > -         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i and HX1200i power supplies
> > > > > > > -         by Corsair.
> > > > > > > +         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i, HX1200i and AX1600i power
> > > > > > > +         supplies by Corsair. The AX760i, AX860i, AX1200i and AX1500i
> > > > > > > +         power supplies are supported through the Corsair Link USB Dongle.
> > > > > > >
> > > > > > >           This driver can also be built as a module. If so, the module
> > > > > > >           will be called corsair-psu.
> > > > > > > diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> > > > > > > index 99494056f4bd..0146dda3e2c3 100644
> > > > > > > --- a/drivers/hwmon/corsair-psu.c
> > > > > > > +++ b/drivers/hwmon/corsair-psu.c
> > > > > > > @@ -571,17 +571,28 @@ static int corsairpsu_raw_event(struct hid_device *hdev, struct
> > > > > > > hid_report *repo }
> > > > > > >
> > > > > > >  static const struct hid_device_id corsairpsu_idtable[] = {
> > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c03) }, /* Corsair HX550i */
> > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c04) }, /* Corsair HX650i */
> > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c05) }, /* Corsair HX750i */
> > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c06) }, /* Corsair HX850i */
> > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c07) }, /* Corsair HX1000i */
> > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c08) }, /* Corsair HX1200i */
> > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c09) }, /* Corsair RM550i */
> > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0a) }, /* Corsair RM650i */
> > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0b) }, /* Corsair RM750i */
> > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0c) }, /* Corsair RM850i */
> > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0d) }, /* Corsair RM1000i */
> > > > > > > +       /*
> > > > > > > +        * The Corsair USB/COM Dongles appear in at least 3 different revisions, where
> > > > > > > rev 1 and 2
> > > > > > > +        * are commonly used with the AX760i, AX860i and AX1200i, while rev3 is rarely
> > > > > > > seen with
> > > > > > > +        * these PSUs. Rev3 is also build into the AX1500i, while the AX1600i is the
> > > > > > > first PSU of
> > > > > > > +        * this series which has an unique usb hid id. Though, the actual device name is
> > > > > > > part of
> > > > > > > +        * the HID message protocol, so it doesn't matter which dongle is connected.
> > > > > > > +        */
> > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C00) }, /* Corsair Link USB/COM Dongle rev1 */
> > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C01) }, /* Corsair Link USB/COM Dongle rev2 */
> > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C02) }, /* Corsair Link USB/COM Dongle rev3
> > > > > > > (AX1500i) */
> > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C03) }, /* Corsair HX550i */
> > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C04) }, /* Corsair HX650i */
> > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C05) }, /* Corsair HX750i */
> > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C06) }, /* Corsair HX850i */
> > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C07) }, /* Corsair HX1000i */
> > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C08) }, /* Corsair HX1200i */
> > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C09) }, /* Corsair RM550i */
> > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0A) }, /* Corsair RM650i */
> > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0B) }, /* Corsair RM750i */
> > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0C) }, /* Corsair RM850i */
> > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0D) }, /* Corsair RM1000i */
> > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C11) }, /* Corsair AX1600i */
> > > > > > >         { },
> > > > > > >  };
> > > > > > >  MODULE_DEVICE_TABLE(hid, corsairpsu_idtable);
> > > > > > > --
> > > > > > > 2.29.2
> > > > > > >
> > > > >
> > > 
>
Wilken Gottwalt Nov. 30, 2020, 5:22 a.m. UTC | #9
On Sun, 29 Nov 2020 13:59:33 -0800
Guenter Roeck <linux@roeck-us.net> wrote:

> On Sun, Nov 29, 2020 at 04:54:43PM +0100, Wilken Gottwalt wrote:
> > On Sun, 29 Nov 2020 05:00:49 -0800
> > Guenter Roeck <linux@roeck-us.net> wrote:
> > 
> > > On Sun, Nov 29, 2020 at 07:36:18AM +0100, Wilken Gottwalt wrote:
> > > > On Sat, 28 Nov 2020 17:21:40 -0300
> > > > Jonas Malaco <jonas@protocubo.io> wrote:
> > > > 
> > > > > On Sat, Nov 28, 2020 at 7:35 AM Wilken Gottwalt
> > > > > <wilken.gottwalt@posteo.net> wrote:
> > > > > >
> > > > > > On Sat, 28 Nov 2020 02:37:38 -0300
> > > > > > Jonas Malaco <jonas@protocubo.io> wrote:
> > > > > >
> > > > > > > On Thu, Nov 26, 2020 at 8:43 AM Wilken Gottwalt
> > > > > > > <wilken.gottwalt@posteo.net> wrote:
> > > > > > > >
> > > > > > > > Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
> > > > > > > > AX1500i and AX1600i. The first 3 power supplies are supported through
> > > > > > > > the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
> > > > > > > > converter especially made for the COM ports of these power supplies.
> > > > > > > > There are 3 known revisions of these adapters. The AX1500i power supply
> > > > > > > > has revision 3 built into the case and AX1600i is the only one in that
> > > > > > > > series, which has an unique usb hid id like the RM/RX series.
> > > > > > >
> > > > > > > Can I ask what AXi power supplies were tested?
> > > > > > >
> > > > > > > I ask because, based on the user-space implementations I am aware of,
> > > > > > > the AXi dongle protocol appears to be different from the RMi/HXi series.
> > > > > >
> > > > > > I was not able to test this against the AX power supplies, they are really
> > > > > > hard to find (and are far to expensive). But I went through all these tools
> > > > > > and stuck to the most common commands, which all 3 series support. Not every
> > > > > > series supports all commands (there also seem to be different firmwares in
> > > > > > the micro-conrollers). But this is fine, some sensors will show up as N/A.
> > > > > > Even my HX850i does not support all commands covered in this driver.
> > > > > 
> > > > > I think the similarities come from all using wrappers over the PMBus
> > > > > interface to the voltage controller.  But I am not sure the wrapping
> > > > > protocols are identical.
> > > > > 
> > > > > For example, cpsumon shows significantly more things going on during a
> > > > > read than what is needed for the RMi/HXi series.[1]
> > > > > 
> > > > > [1] https://github.com/ka87/cpsumon/blob/fd639684d7f9/libcpsumon/src/cpsumon.c#L213-L231
> > > > > 
> > > > > 
> > > > > >
> > > > > > > AXi dongle:
> > > > > > >  - https://github.com/ka87/cpsumon
> > > > > >
> > > > > > This tool made me to consider including the AX series, because it uses some
> > > > > > of the same commands on the AX760i, AX860i, AX1200i and AX1500i. But it is
> > > > > > a usb-serial tool only. But it was nice to know, that the commands are mostly
> > > > > > the same. I left out all the commands for configuring, PCIe power rails,
> > > > > > efficiency and others which do not really belong into hwmon.
> > > > > >
> > > > > > > RMi/HXi:
> > > > > > >  - https://github.com/jonasmalacofilho/liquidctl
> > > > > > >  - https://github.com/audiohacked/OpenCorsairLink
> > > > > >
> > > > > > This tool made me include the AX series, because it uses the rmi protocol
> > > > > > component for the rmi driver (RM/HX series) and the corsair dongles.
> > > > > 
> > > > > The corsairlink_driver_dongle has no implementations for reading sensor
> > > > > data (compare that with the corsairlink_driver_rmi).[2][3]  There is
> > > > > also no code that actually tries to read (write) from (to) the device
> > > > > using that dongle driver.[4]
> > > > > 
> > > > > I also looked at a few of the issues, and all of the ones I read
> > > > > mentioned AXi support being under development, and the hypothesis of the
> > > > > AXi series being compatible with the RMi/HXi code still remaining to be
> > > > > confirmed.
> > > > > 
> > > > > [2]
> > > > > https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/dongle.c#L33-L39
> > > > > [3]
> > > > > https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/rmi.c#L33-L57
> > > > > [4] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/main.c#L106
> > > > > 
> > > > > 
> > > > > >
> > > > > > >  - https://github.com/notaz/corsairmi
> > > > > >
> > > > > > This one covers only some HX/RM PSUs, but is uses the rawhid access which
> > > > > > made me looking up the actual usb chips/bridges Corsair uses.
> > > > > >
> > > > > > >
> > > > > > > One additional concern is that the non-HID AXi dongles may only have bulk
> > > > > > > USB endpoints, and this is a HID driver.[1]
> > > > > >
> > > > > > You are right, in the case of the dongles it could be different. But I did
> > > > > > some research on Corsair usb driven devices and they really like to stick to
> > > > > > the cp210x, which is an usb hid bridge. The commit
> > > > > > b9326057a3d8447f5d2e74a7b521ccf21add2ec0 actually covers two Corsair USB
> > > > > > dongles as a cp210x device. So it is very likely that all Corsair PSUs with
> > > > > > such an interface/dongle use usb hid. But I'm completely open to get proven
> > > > > > wrong. Actually I really would like to see this tested by people who have
> > > > > > access to the more rare devices.
> > > > > 
> > > > > I could be wrong (and I am sorry for the noise if that is the case), but
> > > > > as far as I can see the cp210x does not create a HID device.
> > > > 
> > > > No no, this is fine. It really helps if some more people are looking into this.
> > > > I wish I had access to at least one of the later models (AX1500i/AX1600i), I
> > > > make mistakes from time to time. And it really doesn't help that Corsair changes
> > > > single devices in the same product line by firmware update. The AX1600i seems to
> > > > be the only one, which uses exactly the same protocol like the RM/HX series, but
> > > > is missing the actual usb hid part. But there seems to be a firmware where the
> > > > usb hid part was available for a short time. So, what to do? Remove the AXi part
> > > > completely or keep only the AX1600i?
> > > > 
> > > > Guenter, what would you suggest?
> > > > 
> > > I'd suggest to remove it completely, and explain the reason. Anything else will
> > > just create trouble with people demanding to know why their power supply is not
> > > supported even though it is listed as supported. And, believe me, you don't want
> > > to be on the receiving side of those complaints. The Internet is much less
> > > friendly nowadays than it used to be.
> > 
> > So how is the procedure for this? Just revert it and make a common patch out of
> > it with a proper explanation?
> > 
> > And yeah, I know exactly what you mean. I remember very well how the "internet"
> > got it first ugly hit in the 90s with the upcoming of Flash ... and then the
> > "social media". :D Thanks for your help.
> > 
> 
> Can I just drop your previous patch and you send me another - more restrictive -
> one ?

Yeah, dropping is fine. There is no need for another patch after that, this was
a patch only dealing with the AXi series.

greetings,
Wilken

> Thanks,
> Guenter
> 
> > greetings,
> > Wilken
> > 
> > > Guenter
> > > 
> > > > > Thanks again,
> > > > > Jonas
> > > > > 
> > > > > 
> > > > > >
> > > > > > > Thanks,
> > > > > > > Jonas
> > > > > > >
> > > > > > > [1] https://github.com/ka87/cpsumon/issues/4
> > > > > >
> > > > > > Yes ... that one. The last revision of the dongle could indeed be a problem.
> > > > > > But I'm not really sure what is described here. The last commenter is actually
> > > > > > the one who provided the cp210x patch mentioned up there. The problem here is,
> > > > > > the AX1500i has both connectors, USB and that other one. I call it the other
> > > > > > one because it is the only PSU where it is labeled I2C COMM instead of COMM
> > > > > > only. But at the same time this tools uses the same commands for this PSU.
> > > > > >
> > > > > > So, only some real hardware tests will show.
> > > > > >
> > > > > > Greetings,
> > > > > > Wilken
> > > > > >
> > > > > > > >
> > > > > > > > The patch also changes the usb hid ids to use upper case letters to be
> > > > > > > > consistent with the rest of the hex numbers in the driver and updates
> > > > > > > > the hwmon documentation.
> > > > > > > >
> > > > > > > > This patch adds:
> > > > > > > > - hwmon/corsair-psu documentation update
> > > > > > > > - corsair-psu driver update
> > > > > > > >
> > > > > > > > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> > > > > > > > ---
> > > > > > > >  Documentation/hwmon/corsair-psu.rst | 10 +++++++++
> > > > > > > >  drivers/hwmon/Kconfig               |  7 +++---
> > > > > > > >  drivers/hwmon/corsair-psu.c         | 33 +++++++++++++++++++----------
> > > > > > > >  3 files changed, 36 insertions(+), 14 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/Documentation/hwmon/corsair-psu.rst
> > > > > > > > b/Documentation/hwmon/corsair-psu.rst index 396b95c9a76a..6227e9046d73 100644
> > > > > > > > --- a/Documentation/hwmon/corsair-psu.rst
> > > > > > > > +++ b/Documentation/hwmon/corsair-psu.rst
> > > > > > > > @@ -7,6 +7,16 @@ Supported devices:
> > > > > > > >
> > > > > > > >  * Corsair Power Supplies
> > > > > > > >
> > > > > > > > +  Corsair AX760i (by Corsair Link USB Dongle)
> > > > > > > > +
> > > > > > > > +  Corsair AX860i (by Corsair Link USB Dongle)
> > > > > > > > +
> > > > > > > > +  Corsair AX1200i (by Corsair Link USB Dongle)
> > > > > > > > +
> > > > > > > > +  Corsair AX1500i (by builtin Corsair Link USB Dongle)
> > > > > > > > +
> > > > > > > > +  Corsair AX1600i
> > > > > > > > +
> > > > > > > >    Corsair HX550i
> > > > > > > >
> > > > > > > >    Corsair HX650i
> > > > > > > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > > > > > > > index 716df51edc87..3c059fc23cd6 100644
> > > > > > > > --- a/drivers/hwmon/Kconfig
> > > > > > > > +++ b/drivers/hwmon/Kconfig
> > > > > > > > @@ -453,11 +453,12 @@ config SENSORS_CORSAIR_PSU
> > > > > > > >         tristate "Corsair PSU HID controller"
> > > > > > > >         depends on HID
> > > > > > > >         help
> > > > > > > > -         If you say yes here you get support for Corsair PSUs with a HID
> > > > > > > > +         If you say yes here you get support for Corsair PSUs with an USB HID
> > > > > > > >           interface.
> > > > > > > >           Currently this driver supports the (RM/HX)550i, (RM/HX)650i,
> > > > > > > > -         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i and HX1200i power supplies
> > > > > > > > -         by Corsair.
> > > > > > > > +         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i, HX1200i and AX1600i power
> > > > > > > > +         supplies by Corsair. The AX760i, AX860i, AX1200i and AX1500i
> > > > > > > > +         power supplies are supported through the Corsair Link USB Dongle.
> > > > > > > >
> > > > > > > >           This driver can also be built as a module. If so, the module
> > > > > > > >           will be called corsair-psu.
> > > > > > > > diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> > > > > > > > index 99494056f4bd..0146dda3e2c3 100644
> > > > > > > > --- a/drivers/hwmon/corsair-psu.c
> > > > > > > > +++ b/drivers/hwmon/corsair-psu.c
> > > > > > > > @@ -571,17 +571,28 @@ static int corsairpsu_raw_event(struct hid_device *hdev,
> > > > > > > > struct hid_report *repo }
> > > > > > > >
> > > > > > > >  static const struct hid_device_id corsairpsu_idtable[] = {
> > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c03) }, /* Corsair HX550i */
> > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c04) }, /* Corsair HX650i */
> > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c05) }, /* Corsair HX750i */
> > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c06) }, /* Corsair HX850i */
> > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c07) }, /* Corsair HX1000i */
> > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c08) }, /* Corsair HX1200i */
> > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c09) }, /* Corsair RM550i */
> > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0a) }, /* Corsair RM650i */
> > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0b) }, /* Corsair RM750i */
> > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0c) }, /* Corsair RM850i */
> > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0d) }, /* Corsair RM1000i */
> > > > > > > > +       /*
> > > > > > > > +        * The Corsair USB/COM Dongles appear in at least 3 different revisions,
> > > > > > > > where rev 1 and 2
> > > > > > > > +        * are commonly used with the AX760i, AX860i and AX1200i, while rev3 is
> > > > > > > > rarely seen with
> > > > > > > > +        * these PSUs. Rev3 is also build into the AX1500i, while the AX1600i is the
> > > > > > > > first PSU of
> > > > > > > > +        * this series which has an unique usb hid id. Though, the actual device
> > > > > > > > name is part of
> > > > > > > > +        * the HID message protocol, so it doesn't matter which dongle is connected.
> > > > > > > > +        */
> > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C00) }, /* Corsair Link USB/COM Dongle rev1 */
> > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C01) }, /* Corsair Link USB/COM Dongle rev2 */
> > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C02) }, /* Corsair Link USB/COM Dongle rev3
> > > > > > > > (AX1500i) */
> > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C03) }, /* Corsair HX550i */
> > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C04) }, /* Corsair HX650i */
> > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C05) }, /* Corsair HX750i */
> > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C06) }, /* Corsair HX850i */
> > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C07) }, /* Corsair HX1000i */
> > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C08) }, /* Corsair HX1200i */
> > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C09) }, /* Corsair RM550i */
> > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0A) }, /* Corsair RM650i */
> > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0B) }, /* Corsair RM750i */
> > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0C) }, /* Corsair RM850i */
> > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0D) }, /* Corsair RM1000i */
> > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C11) }, /* Corsair AX1600i */
> > > > > > > >         { },
> > > > > > > >  };
> > > > > > > >  MODULE_DEVICE_TABLE(hid, corsairpsu_idtable);
> > > > > > > > --
> > > > > > > > 2.29.2
> > > > > > > >
> > > > > >
> > > > 
> >
Jonas Malaco Nov. 30, 2020, 10:53 a.m. UTC | #10
On Mon, Nov 30, 2020 at 2:22 AM Wilken Gottwalt
<wilken.gottwalt@posteo.net> wrote:
>
> On Sun, 29 Nov 2020 13:59:33 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
>
> > On Sun, Nov 29, 2020 at 04:54:43PM +0100, Wilken Gottwalt wrote:
> > > On Sun, 29 Nov 2020 05:00:49 -0800
> > > Guenter Roeck <linux@roeck-us.net> wrote:
> > >
> > > > On Sun, Nov 29, 2020 at 07:36:18AM +0100, Wilken Gottwalt wrote:
> > > > > On Sat, 28 Nov 2020 17:21:40 -0300
> > > > > Jonas Malaco <jonas@protocubo.io> wrote:
> > > > >
> > > > > > On Sat, Nov 28, 2020 at 7:35 AM Wilken Gottwalt
> > > > > > <wilken.gottwalt@posteo.net> wrote:
> > > > > > >
> > > > > > > On Sat, 28 Nov 2020 02:37:38 -0300
> > > > > > > Jonas Malaco <jonas@protocubo.io> wrote:
> > > > > > >
> > > > > > > > On Thu, Nov 26, 2020 at 8:43 AM Wilken Gottwalt
> > > > > > > > <wilken.gottwalt@posteo.net> wrote:
> > > > > > > > >
> > > > > > > > > Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
> > > > > > > > > AX1500i and AX1600i. The first 3 power supplies are supported through
> > > > > > > > > the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
> > > > > > > > > converter especially made for the COM ports of these power supplies.
> > > > > > > > > There are 3 known revisions of these adapters. The AX1500i power supply
> > > > > > > > > has revision 3 built into the case and AX1600i is the only one in that
> > > > > > > > > series, which has an unique usb hid id like the RM/RX series.
> > > > > > > >
> > > > > > > > Can I ask what AXi power supplies were tested?
> > > > > > > >
> > > > > > > > I ask because, based on the user-space implementations I am aware of,
> > > > > > > > the AXi dongle protocol appears to be different from the RMi/HXi series.
> > > > > > >
> > > > > > > I was not able to test this against the AX power supplies, they are really
> > > > > > > hard to find (and are far to expensive). But I went through all these tools
> > > > > > > and stuck to the most common commands, which all 3 series support. Not every
> > > > > > > series supports all commands (there also seem to be different firmwares in
> > > > > > > the micro-conrollers). But this is fine, some sensors will show up as N/A.
> > > > > > > Even my HX850i does not support all commands covered in this driver.
> > > > > >
> > > > > > I think the similarities come from all using wrappers over the PMBus
> > > > > > interface to the voltage controller.  But I am not sure the wrapping
> > > > > > protocols are identical.
> > > > > >
> > > > > > For example, cpsumon shows significantly more things going on during a
> > > > > > read than what is needed for the RMi/HXi series.[1]
> > > > > >
> > > > > > [1] https://github.com/ka87/cpsumon/blob/fd639684d7f9/libcpsumon/src/cpsumon.c#L213-L231
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > > AXi dongle:
> > > > > > > >  - https://github.com/ka87/cpsumon
> > > > > > >
> > > > > > > This tool made me to consider including the AX series, because it uses some
> > > > > > > of the same commands on the AX760i, AX860i, AX1200i and AX1500i. But it is
> > > > > > > a usb-serial tool only. But it was nice to know, that the commands are mostly
> > > > > > > the same. I left out all the commands for configuring, PCIe power rails,
> > > > > > > efficiency and others which do not really belong into hwmon.
> > > > > > >
> > > > > > > > RMi/HXi:
> > > > > > > >  - https://github.com/jonasmalacofilho/liquidctl
> > > > > > > >  - https://github.com/audiohacked/OpenCorsairLink
> > > > > > >
> > > > > > > This tool made me include the AX series, because it uses the rmi protocol
> > > > > > > component for the rmi driver (RM/HX series) and the corsair dongles.
> > > > > >
> > > > > > The corsairlink_driver_dongle has no implementations for reading sensor
> > > > > > data (compare that with the corsairlink_driver_rmi).[2][3]  There is
> > > > > > also no code that actually tries to read (write) from (to) the device
> > > > > > using that dongle driver.[4]
> > > > > >
> > > > > > I also looked at a few of the issues, and all of the ones I read
> > > > > > mentioned AXi support being under development, and the hypothesis of the
> > > > > > AXi series being compatible with the RMi/HXi code still remaining to be
> > > > > > confirmed.
> > > > > >
> > > > > > [2]
> > > > > > https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/dongle.c#L33-L39
> > > > > > [3]
> > > > > > https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/rmi.c#L33-L57
> > > > > > [4] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/main.c#L106
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > >  - https://github.com/notaz/corsairmi
> > > > > > >
> > > > > > > This one covers only some HX/RM PSUs, but is uses the rawhid access which
> > > > > > > made me looking up the actual usb chips/bridges Corsair uses.
> > > > > > >
> > > > > > > >
> > > > > > > > One additional concern is that the non-HID AXi dongles may only have bulk
> > > > > > > > USB endpoints, and this is a HID driver.[1]
> > > > > > >
> > > > > > > You are right, in the case of the dongles it could be different. But I did
> > > > > > > some research on Corsair usb driven devices and they really like to stick to
> > > > > > > the cp210x, which is an usb hid bridge. The commit
> > > > > > > b9326057a3d8447f5d2e74a7b521ccf21add2ec0 actually covers two Corsair USB
> > > > > > > dongles as a cp210x device. So it is very likely that all Corsair PSUs with
> > > > > > > such an interface/dongle use usb hid. But I'm completely open to get proven
> > > > > > > wrong. Actually I really would like to see this tested by people who have
> > > > > > > access to the more rare devices.
> > > > > >
> > > > > > I could be wrong (and I am sorry for the noise if that is the case), but
> > > > > > as far as I can see the cp210x does not create a HID device.
> > > > >
> > > > > No no, this is fine. It really helps if some more people are looking into this.
> > > > > I wish I had access to at least one of the later models (AX1500i/AX1600i), I
> > > > > make mistakes from time to time. And it really doesn't help that Corsair changes
> > > > > single devices in the same product line by firmware update. The AX1600i seems to
> > > > > be the only one, which uses exactly the same protocol like the RM/HX series, but
> > > > > is missing the actual usb hid part. But there seems to be a firmware where the
> > > > > usb hid part was available for a short time. So, what to do? Remove the AXi part
> > > > > completely or keep only the AX1600i?
> > > > >
> > > > > Guenter, what would you suggest?
> > > > >
> > > > I'd suggest to remove it completely, and explain the reason. Anything else will
> > > > just create trouble with people demanding to know why their power supply is not
> > > > supported even though it is listed as supported. And, believe me, you don't want
> > > > to be on the receiving side of those complaints. The Internet is much less
> > > > friendly nowadays than it used to be.
> > >
> > > So how is the procedure for this? Just revert it and make a common patch out of
> > > it with a proper explanation?
> > >
> > > And yeah, I know exactly what you mean. I remember very well how the "internet"
> > > got it first ugly hit in the 90s with the upcoming of Flash ... and then the
> > > "social media". :D Thanks for your help.
> > >
> >
> > Can I just drop your previous patch and you send me another - more restrictive -
> > one ?
>
> Yeah, dropping is fine. There is no need for another patch after that, this was
> a patch only dealing with the AXi series.

And I'm sorry for throwing cold water at the patch.

Hopefully we'll find someone with one of these AXi PSUs once the RMi/HXi
driver is widely available on distros.

Thanks,
Jonas

> greetings,
> Wilken
>
> > Thanks,
> > Guenter
> >
> > > greetings,
> > > Wilken
> > >
> > > > Guenter
> > > >
> > > > > > Thanks again,
> > > > > > Jonas
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Jonas
> > > > > > > >
> > > > > > > > [1] https://github.com/ka87/cpsumon/issues/4
> > > > > > >
> > > > > > > Yes ... that one. The last revision of the dongle could indeed be a problem.
> > > > > > > But I'm not really sure what is described here. The last commenter is actually
> > > > > > > the one who provided the cp210x patch mentioned up there. The problem here is,
> > > > > > > the AX1500i has both connectors, USB and that other one. I call it the other
> > > > > > > one because it is the only PSU where it is labeled I2C COMM instead of COMM
> > > > > > > only. But at the same time this tools uses the same commands for this PSU.
> > > > > > >
> > > > > > > So, only some real hardware tests will show.
> > > > > > >
> > > > > > > Greetings,
> > > > > > > Wilken
> > > > > > >
> > > > > > > > >
> > > > > > > > > The patch also changes the usb hid ids to use upper case letters to be
> > > > > > > > > consistent with the rest of the hex numbers in the driver and updates
> > > > > > > > > the hwmon documentation.
> > > > > > > > >
> > > > > > > > > This patch adds:
> > > > > > > > > - hwmon/corsair-psu documentation update
> > > > > > > > > - corsair-psu driver update
> > > > > > > > >
> > > > > > > > > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> > > > > > > > > ---
> > > > > > > > >  Documentation/hwmon/corsair-psu.rst | 10 +++++++++
> > > > > > > > >  drivers/hwmon/Kconfig               |  7 +++---
> > > > > > > > >  drivers/hwmon/corsair-psu.c         | 33 +++++++++++++++++++----------
> > > > > > > > >  3 files changed, 36 insertions(+), 14 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git a/Documentation/hwmon/corsair-psu.rst
> > > > > > > > > b/Documentation/hwmon/corsair-psu.rst index 396b95c9a76a..6227e9046d73 100644
> > > > > > > > > --- a/Documentation/hwmon/corsair-psu.rst
> > > > > > > > > +++ b/Documentation/hwmon/corsair-psu.rst
> > > > > > > > > @@ -7,6 +7,16 @@ Supported devices:
> > > > > > > > >
> > > > > > > > >  * Corsair Power Supplies
> > > > > > > > >
> > > > > > > > > +  Corsair AX760i (by Corsair Link USB Dongle)
> > > > > > > > > +
> > > > > > > > > +  Corsair AX860i (by Corsair Link USB Dongle)
> > > > > > > > > +
> > > > > > > > > +  Corsair AX1200i (by Corsair Link USB Dongle)
> > > > > > > > > +
> > > > > > > > > +  Corsair AX1500i (by builtin Corsair Link USB Dongle)
> > > > > > > > > +
> > > > > > > > > +  Corsair AX1600i
> > > > > > > > > +
> > > > > > > > >    Corsair HX550i
> > > > > > > > >
> > > > > > > > >    Corsair HX650i
> > > > > > > > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > > > > > > > > index 716df51edc87..3c059fc23cd6 100644
> > > > > > > > > --- a/drivers/hwmon/Kconfig
> > > > > > > > > +++ b/drivers/hwmon/Kconfig
> > > > > > > > > @@ -453,11 +453,12 @@ config SENSORS_CORSAIR_PSU
> > > > > > > > >         tristate "Corsair PSU HID controller"
> > > > > > > > >         depends on HID
> > > > > > > > >         help
> > > > > > > > > -         If you say yes here you get support for Corsair PSUs with a HID
> > > > > > > > > +         If you say yes here you get support for Corsair PSUs with an USB HID
> > > > > > > > >           interface.
> > > > > > > > >           Currently this driver supports the (RM/HX)550i, (RM/HX)650i,
> > > > > > > > > -         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i and HX1200i power supplies
> > > > > > > > > -         by Corsair.
> > > > > > > > > +         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i, HX1200i and AX1600i power
> > > > > > > > > +         supplies by Corsair. The AX760i, AX860i, AX1200i and AX1500i
> > > > > > > > > +         power supplies are supported through the Corsair Link USB Dongle.
> > > > > > > > >
> > > > > > > > >           This driver can also be built as a module. If so, the module
> > > > > > > > >           will be called corsair-psu.
> > > > > > > > > diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> > > > > > > > > index 99494056f4bd..0146dda3e2c3 100644
> > > > > > > > > --- a/drivers/hwmon/corsair-psu.c
> > > > > > > > > +++ b/drivers/hwmon/corsair-psu.c
> > > > > > > > > @@ -571,17 +571,28 @@ static int corsairpsu_raw_event(struct hid_device *hdev,
> > > > > > > > > struct hid_report *repo }
> > > > > > > > >
> > > > > > > > >  static const struct hid_device_id corsairpsu_idtable[] = {
> > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c03) }, /* Corsair HX550i */
> > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c04) }, /* Corsair HX650i */
> > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c05) }, /* Corsair HX750i */
> > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c06) }, /* Corsair HX850i */
> > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c07) }, /* Corsair HX1000i */
> > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c08) }, /* Corsair HX1200i */
> > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c09) }, /* Corsair RM550i */
> > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0a) }, /* Corsair RM650i */
> > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0b) }, /* Corsair RM750i */
> > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0c) }, /* Corsair RM850i */
> > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0d) }, /* Corsair RM1000i */
> > > > > > > > > +       /*
> > > > > > > > > +        * The Corsair USB/COM Dongles appear in at least 3 different revisions,
> > > > > > > > > where rev 1 and 2
> > > > > > > > > +        * are commonly used with the AX760i, AX860i and AX1200i, while rev3 is
> > > > > > > > > rarely seen with
> > > > > > > > > +        * these PSUs. Rev3 is also build into the AX1500i, while the AX1600i is the
> > > > > > > > > first PSU of
> > > > > > > > > +        * this series which has an unique usb hid id. Though, the actual device
> > > > > > > > > name is part of
> > > > > > > > > +        * the HID message protocol, so it doesn't matter which dongle is connected.
> > > > > > > > > +        */
> > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C00) }, /* Corsair Link USB/COM Dongle rev1 */
> > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C01) }, /* Corsair Link USB/COM Dongle rev2 */
> > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C02) }, /* Corsair Link USB/COM Dongle rev3
> > > > > > > > > (AX1500i) */
> > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C03) }, /* Corsair HX550i */
> > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C04) }, /* Corsair HX650i */
> > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C05) }, /* Corsair HX750i */
> > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C06) }, /* Corsair HX850i */
> > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C07) }, /* Corsair HX1000i */
> > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C08) }, /* Corsair HX1200i */
> > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C09) }, /* Corsair RM550i */
> > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0A) }, /* Corsair RM650i */
> > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0B) }, /* Corsair RM750i */
> > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0C) }, /* Corsair RM850i */
> > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0D) }, /* Corsair RM1000i */
> > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C11) }, /* Corsair AX1600i */
> > > > > > > > >         { },
> > > > > > > > >  };
> > > > > > > > >  MODULE_DEVICE_TABLE(hid, corsairpsu_idtable);
> > > > > > > > > --
> > > > > > > > > 2.29.2
> > > > > > > > >
> > > > > > >
> > > > >
> > >
>
Thomas Backlund Nov. 30, 2020, 12:43 p.m. UTC | #11
Den 28.11.2020 kl. 12:35, skrev Wilken Gottwalt:
> On Sat, 28 Nov 2020 02:37:38 -0300
> Jonas Malaco <jonas@protocubo.io> wrote:
> 
>> On Thu, Nov 26, 2020 at 8:43 AM Wilken Gottwalt
>> <wilken.gottwalt@posteo.net> wrote:
>>>
>>> Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
>>> AX1500i and AX1600i. The first 3 power supplies are supported through
>>> the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
>>> converter especially made for the COM ports of these power supplies.
>>> There are 3 known revisions of these adapters. The AX1500i power supply
>>> has revision 3 built into the case and AX1600i is the only one in that
>>> series, which has an unique usb hid id like the RM/RX series.
>>
>> Can I ask what AXi power supplies were tested?
>>
>> I ask because, based on the user-space implementations I am aware of,
>> the AXi dongle protocol appears to be different from the RMi/HXi series.
> 
> I was not able to test this against the AX power supplies, they are really
> hard to find (and are far to expensive). But I went through all these tools
> and stuck to the most common commands, which all 3 series support. Not every
> series supports all commands (there also seem to be different firmwares in
> the micro-conrollers). But this is fine, some sensors will show up as N/A.
> Even my HX850i does not support all commands covered in this driver.
> 

What kind of tests do you want / need ?

I have an AX860i here.

--
Regards

Thomas Backlund
Wilken Gottwalt Nov. 30, 2020, 2:46 p.m. UTC | #12
On Mon, 30 Nov 2020 07:53:48 -0300
Jonas Malaco <jonas@protocubo.io> wrote:

> On Mon, Nov 30, 2020 at 2:22 AM Wilken Gottwalt
> <wilken.gottwalt@posteo.net> wrote:
> >
> > On Sun, 29 Nov 2020 13:59:33 -0800
> > Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > > On Sun, Nov 29, 2020 at 04:54:43PM +0100, Wilken Gottwalt wrote:
> > > > On Sun, 29 Nov 2020 05:00:49 -0800
> > > > Guenter Roeck <linux@roeck-us.net> wrote:
> > > >
> > > > > On Sun, Nov 29, 2020 at 07:36:18AM +0100, Wilken Gottwalt wrote:
> > > > > > On Sat, 28 Nov 2020 17:21:40 -0300
> > > > > > Jonas Malaco <jonas@protocubo.io> wrote:
> > > > > >
> > > > > > > On Sat, Nov 28, 2020 at 7:35 AM Wilken Gottwalt
> > > > > > > <wilken.gottwalt@posteo.net> wrote:
> > > > > > > >
> > > > > > > > On Sat, 28 Nov 2020 02:37:38 -0300
> > > > > > > > Jonas Malaco <jonas@protocubo.io> wrote:
> > > > > > > >
> > > > > > > > > On Thu, Nov 26, 2020 at 8:43 AM Wilken Gottwalt
> > > > > > > > > <wilken.gottwalt@posteo.net> wrote:
> > > > > > > > > >
> > > > > > > > > > Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
> > > > > > > > > > AX1500i and AX1600i. The first 3 power supplies are supported through
> > > > > > > > > > the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
> > > > > > > > > > converter especially made for the COM ports of these power supplies.
> > > > > > > > > > There are 3 known revisions of these adapters. The AX1500i power supply
> > > > > > > > > > has revision 3 built into the case and AX1600i is the only one in that
> > > > > > > > > > series, which has an unique usb hid id like the RM/RX series.
> > > > > > > > >
> > > > > > > > > Can I ask what AXi power supplies were tested?
> > > > > > > > >
> > > > > > > > > I ask because, based on the user-space implementations I am aware of,
> > > > > > > > > the AXi dongle protocol appears to be different from the RMi/HXi series.
> > > > > > > >
> > > > > > > > I was not able to test this against the AX power supplies, they are really
> > > > > > > > hard to find (and are far to expensive). But I went through all these tools
> > > > > > > > and stuck to the most common commands, which all 3 series support. Not every
> > > > > > > > series supports all commands (there also seem to be different firmwares in
> > > > > > > > the micro-conrollers). But this is fine, some sensors will show up as N/A.
> > > > > > > > Even my HX850i does not support all commands covered in this driver.
> > > > > > >
> > > > > > > I think the similarities come from all using wrappers over the PMBus
> > > > > > > interface to the voltage controller.  But I am not sure the wrapping
> > > > > > > protocols are identical.
> > > > > > >
> > > > > > > For example, cpsumon shows significantly more things going on during a
> > > > > > > read than what is needed for the RMi/HXi series.[1]
> > > > > > >
> > > > > > > [1]
> > > > > > > https://github.com/ka87/cpsumon/blob/fd639684d7f9/libcpsumon/src/cpsumon.c#L213-L231
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > > AXi dongle:
> > > > > > > > >  - https://github.com/ka87/cpsumon
> > > > > > > >
> > > > > > > > This tool made me to consider including the AX series, because it uses some
> > > > > > > > of the same commands on the AX760i, AX860i, AX1200i and AX1500i. But it is
> > > > > > > > a usb-serial tool only. But it was nice to know, that the commands are mostly
> > > > > > > > the same. I left out all the commands for configuring, PCIe power rails,
> > > > > > > > efficiency and others which do not really belong into hwmon.
> > > > > > > >
> > > > > > > > > RMi/HXi:
> > > > > > > > >  - https://github.com/jonasmalacofilho/liquidctl
> > > > > > > > >  - https://github.com/audiohacked/OpenCorsairLink
> > > > > > > >
> > > > > > > > This tool made me include the AX series, because it uses the rmi protocol
> > > > > > > > component for the rmi driver (RM/HX series) and the corsair dongles.
> > > > > > >
> > > > > > > The corsairlink_driver_dongle has no implementations for reading sensor
> > > > > > > data (compare that with the corsairlink_driver_rmi).[2][3]  There is
> > > > > > > also no code that actually tries to read (write) from (to) the device
> > > > > > > using that dongle driver.[4]
> > > > > > >
> > > > > > > I also looked at a few of the issues, and all of the ones I read
> > > > > > > mentioned AXi support being under development, and the hypothesis of the
> > > > > > > AXi series being compatible with the RMi/HXi code still remaining to be
> > > > > > > confirmed.
> > > > > > >
> > > > > > > [2]
> > > > > > > https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/dongle.c#L33-L39
> > > > > > > [3]
> > > > > > > https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/drivers/rmi.c#L33-L57
> > > > > > > [4] https://github.com/audiohacked/OpenCorsairLink/blob/61d336a61b85/main.c#L106
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > >  - https://github.com/notaz/corsairmi
> > > > > > > >
> > > > > > > > This one covers only some HX/RM PSUs, but is uses the rawhid access which
> > > > > > > > made me looking up the actual usb chips/bridges Corsair uses.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > One additional concern is that the non-HID AXi dongles may only have bulk
> > > > > > > > > USB endpoints, and this is a HID driver.[1]
> > > > > > > >
> > > > > > > > You are right, in the case of the dongles it could be different. But I did
> > > > > > > > some research on Corsair usb driven devices and they really like to stick to
> > > > > > > > the cp210x, which is an usb hid bridge. The commit
> > > > > > > > b9326057a3d8447f5d2e74a7b521ccf21add2ec0 actually covers two Corsair USB
> > > > > > > > dongles as a cp210x device. So it is very likely that all Corsair PSUs with
> > > > > > > > such an interface/dongle use usb hid. But I'm completely open to get proven
> > > > > > > > wrong. Actually I really would like to see this tested by people who have
> > > > > > > > access to the more rare devices.
> > > > > > >
> > > > > > > I could be wrong (and I am sorry for the noise if that is the case), but
> > > > > > > as far as I can see the cp210x does not create a HID device.
> > > > > >
> > > > > > No no, this is fine. It really helps if some more people are looking into this.
> > > > > > I wish I had access to at least one of the later models (AX1500i/AX1600i), I
> > > > > > make mistakes from time to time. And it really doesn't help that Corsair changes
> > > > > > single devices in the same product line by firmware update. The AX1600i seems to
> > > > > > be the only one, which uses exactly the same protocol like the RM/HX series, but
> > > > > > is missing the actual usb hid part. But there seems to be a firmware where the
> > > > > > usb hid part was available for a short time. So, what to do? Remove the AXi part
> > > > > > completely or keep only the AX1600i?
> > > > > >
> > > > > > Guenter, what would you suggest?
> > > > > >
> > > > > I'd suggest to remove it completely, and explain the reason. Anything else will
> > > > > just create trouble with people demanding to know why their power supply is not
> > > > > supported even though it is listed as supported. And, believe me, you don't want
> > > > > to be on the receiving side of those complaints. The Internet is much less
> > > > > friendly nowadays than it used to be.
> > > >
> > > > So how is the procedure for this? Just revert it and make a common patch out of
> > > > it with a proper explanation?
> > > >
> > > > And yeah, I know exactly what you mean. I remember very well how the "internet"
> > > > got it first ugly hit in the 90s with the upcoming of Flash ... and then the
> > > > "social media". :D Thanks for your help.
> > > >
> > >
> > > Can I just drop your previous patch and you send me another - more restrictive -
> > > one ?
> >
> > Yeah, dropping is fine. There is no need for another patch after that, this was
> > a patch only dealing with the AXi series.
> 
> And I'm sorry for throwing cold water at the patch.
> 
> Hopefully we'll find someone with one of these AXi PSUs once the RMi/HXi
> driver is widely available on distros.

This is absolutely fine. If a patch is nonsense, and if the AXi series has no
usb hid support, than the patch is nonsense. So reverting is the right option.

greetings,
Wilken

> Thanks,
> Jonas
> 
> > greetings,
> > Wilken
> >
> > > Thanks,
> > > Guenter
> > >
> > > > greetings,
> > > > Wilken
> > > >
> > > > > Guenter
> > > > >
> > > > > > > Thanks again,
> > > > > > > Jonas
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > > Thanks,
> > > > > > > > > Jonas
> > > > > > > > >
> > > > > > > > > [1] https://github.com/ka87/cpsumon/issues/4
> > > > > > > >
> > > > > > > > Yes ... that one. The last revision of the dongle could indeed be a problem.
> > > > > > > > But I'm not really sure what is described here. The last commenter is actually
> > > > > > > > the one who provided the cp210x patch mentioned up there. The problem here is,
> > > > > > > > the AX1500i has both connectors, USB and that other one. I call it the other
> > > > > > > > one because it is the only PSU where it is labeled I2C COMM instead of COMM
> > > > > > > > only. But at the same time this tools uses the same commands for this PSU.
> > > > > > > >
> > > > > > > > So, only some real hardware tests will show.
> > > > > > > >
> > > > > > > > Greetings,
> > > > > > > > Wilken
> > > > > > > >
> > > > > > > > > >
> > > > > > > > > > The patch also changes the usb hid ids to use upper case letters to be
> > > > > > > > > > consistent with the rest of the hex numbers in the driver and updates
> > > > > > > > > > the hwmon documentation.
> > > > > > > > > >
> > > > > > > > > > This patch adds:
> > > > > > > > > > - hwmon/corsair-psu documentation update
> > > > > > > > > > - corsair-psu driver update
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> > > > > > > > > > ---
> > > > > > > > > >  Documentation/hwmon/corsair-psu.rst | 10 +++++++++
> > > > > > > > > >  drivers/hwmon/Kconfig               |  7 +++---
> > > > > > > > > >  drivers/hwmon/corsair-psu.c         | 33 +++++++++++++++++++----------
> > > > > > > > > >  3 files changed, 36 insertions(+), 14 deletions(-)
> > > > > > > > > >
> > > > > > > > > > diff --git a/Documentation/hwmon/corsair-psu.rst
> > > > > > > > > > b/Documentation/hwmon/corsair-psu.rst index 396b95c9a76a..6227e9046d73 100644
> > > > > > > > > > --- a/Documentation/hwmon/corsair-psu.rst
> > > > > > > > > > +++ b/Documentation/hwmon/corsair-psu.rst
> > > > > > > > > > @@ -7,6 +7,16 @@ Supported devices:
> > > > > > > > > >
> > > > > > > > > >  * Corsair Power Supplies
> > > > > > > > > >
> > > > > > > > > > +  Corsair AX760i (by Corsair Link USB Dongle)
> > > > > > > > > > +
> > > > > > > > > > +  Corsair AX860i (by Corsair Link USB Dongle)
> > > > > > > > > > +
> > > > > > > > > > +  Corsair AX1200i (by Corsair Link USB Dongle)
> > > > > > > > > > +
> > > > > > > > > > +  Corsair AX1500i (by builtin Corsair Link USB Dongle)
> > > > > > > > > > +
> > > > > > > > > > +  Corsair AX1600i
> > > > > > > > > > +
> > > > > > > > > >    Corsair HX550i
> > > > > > > > > >
> > > > > > > > > >    Corsair HX650i
> > > > > > > > > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > > > > > > > > > index 716df51edc87..3c059fc23cd6 100644
> > > > > > > > > > --- a/drivers/hwmon/Kconfig
> > > > > > > > > > +++ b/drivers/hwmon/Kconfig
> > > > > > > > > > @@ -453,11 +453,12 @@ config SENSORS_CORSAIR_PSU
> > > > > > > > > >         tristate "Corsair PSU HID controller"
> > > > > > > > > >         depends on HID
> > > > > > > > > >         help
> > > > > > > > > > -         If you say yes here you get support for Corsair PSUs with a HID
> > > > > > > > > > +         If you say yes here you get support for Corsair PSUs with an USB HID
> > > > > > > > > >           interface.
> > > > > > > > > >           Currently this driver supports the (RM/HX)550i, (RM/HX)650i,
> > > > > > > > > > -         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i and HX1200i power supplies
> > > > > > > > > > -         by Corsair.
> > > > > > > > > > +         (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i, HX1200i and AX1600i power
> > > > > > > > > > +         supplies by Corsair. The AX760i, AX860i, AX1200i and AX1500i
> > > > > > > > > > +         power supplies are supported through the Corsair Link USB Dongle.
> > > > > > > > > >
> > > > > > > > > >           This driver can also be built as a module. If so, the module
> > > > > > > > > >           will be called corsair-psu.
> > > > > > > > > > diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> > > > > > > > > > index 99494056f4bd..0146dda3e2c3 100644
> > > > > > > > > > --- a/drivers/hwmon/corsair-psu.c
> > > > > > > > > > +++ b/drivers/hwmon/corsair-psu.c
> > > > > > > > > > @@ -571,17 +571,28 @@ static int corsairpsu_raw_event(struct hid_device *hdev,
> > > > > > > > > > struct hid_report *repo }
> > > > > > > > > >
> > > > > > > > > >  static const struct hid_device_id corsairpsu_idtable[] = {
> > > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c03) }, /* Corsair HX550i */
> > > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c04) }, /* Corsair HX650i */
> > > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c05) }, /* Corsair HX750i */
> > > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c06) }, /* Corsair HX850i */
> > > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c07) }, /* Corsair HX1000i */
> > > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c08) }, /* Corsair HX1200i */
> > > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c09) }, /* Corsair RM550i */
> > > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0a) }, /* Corsair RM650i */
> > > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0b) }, /* Corsair RM750i */
> > > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0c) }, /* Corsair RM850i */
> > > > > > > > > > -       { HID_USB_DEVICE(0x1b1c, 0x1c0d) }, /* Corsair RM1000i */
> > > > > > > > > > +       /*
> > > > > > > > > > +        * The Corsair USB/COM Dongles appear in at least 3 different revisions,
> > > > > > > > > > where rev 1 and 2
> > > > > > > > > > +        * are commonly used with the AX760i, AX860i and AX1200i, while rev3 is
> > > > > > > > > > rarely seen with
> > > > > > > > > > +        * these PSUs. Rev3 is also build into the AX1500i, while the AX1600i
> > > > > > > > > > is the first PSU of
> > > > > > > > > > +        * this series which has an unique usb hid id. Though, the actual device
> > > > > > > > > > name is part of
> > > > > > > > > > +        * the HID message protocol, so it doesn't matter which dongle is
> > > > > > > > > > connected.
> > > > > > > > > > +        */
> > > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C00) }, /* Corsair Link USB/COM Dongle rev1
> > > > > > > > > > */
> > > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C01) }, /* Corsair Link USB/COM Dongle rev2
> > > > > > > > > > */
> > > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C02) }, /* Corsair Link USB/COM Dongle rev3
> > > > > > > > > > (AX1500i) */
> > > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C03) }, /* Corsair HX550i */
> > > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C04) }, /* Corsair HX650i */
> > > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C05) }, /* Corsair HX750i */
> > > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C06) }, /* Corsair HX850i */
> > > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C07) }, /* Corsair HX1000i */
> > > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C08) }, /* Corsair HX1200i */
> > > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C09) }, /* Corsair RM550i */
> > > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0A) }, /* Corsair RM650i */
> > > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0B) }, /* Corsair RM750i */
> > > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0C) }, /* Corsair RM850i */
> > > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C0D) }, /* Corsair RM1000i */
> > > > > > > > > > +       { HID_USB_DEVICE(0x1B1C, 0x1C11) }, /* Corsair AX1600i */
> > > > > > > > > >         { },
> > > > > > > > > >  };
> > > > > > > > > >  MODULE_DEVICE_TABLE(hid, corsairpsu_idtable);
> > > > > > > > > > --
> > > > > > > > > > 2.29.2
> > > > > > > > > >
> > > > > > > >
> > > > > >
> > > >
> >
Wilken Gottwalt Nov. 30, 2020, 2:49 p.m. UTC | #13
On Mon, 30 Nov 2020 14:43:44 +0200
Thomas Backlund <tmb@iki.fi> wrote:

> Den 28.11.2020 kl. 12:35, skrev Wilken Gottwalt:
> > On Sat, 28 Nov 2020 02:37:38 -0300
> > Jonas Malaco <jonas@protocubo.io> wrote:
> > 
> >> On Thu, Nov 26, 2020 at 8:43 AM Wilken Gottwalt
> >> <wilken.gottwalt@posteo.net> wrote:
> >>>
> >>> Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
> >>> AX1500i and AX1600i. The first 3 power supplies are supported through
> >>> the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
> >>> converter especially made for the COM ports of these power supplies.
> >>> There are 3 known revisions of these adapters. The AX1500i power supply
> >>> has revision 3 built into the case and AX1600i is the only one in that
> >>> series, which has an unique usb hid id like the RM/RX series.
> >>
> >> Can I ask what AXi power supplies were tested?
> >>
> >> I ask because, based on the user-space implementations I am aware of,
> >> the AXi dongle protocol appears to be different from the RMi/HXi series.
> > 
> > I was not able to test this against the AX power supplies, they are really
> > hard to find (and are far to expensive). But I went through all these tools
> > and stuck to the most common commands, which all 3 series support. Not every
> > series supports all commands (there also seem to be different firmwares in
> > the micro-conrollers). But this is fine, some sensors will show up as N/A.
> > Even my HX850i does not support all commands covered in this driver.
> > 
> 
> What kind of tests do you want / need ?
> 
> I have an AX860i here.

Oh nice. Lets start with an usb info dump. Can you give me the full dump of
lsusb -vd <device> of the Corsair USB dongle?

greetings,
Wilken

> --
> Regards
> 
> Thomas Backlund
>
Guenter Roeck Nov. 30, 2020, 2:50 p.m. UTC | #14
On Mon, Nov 30, 2020 at 06:22:30AM +0100, Wilken Gottwalt wrote:
[ ... ]
> > 
> > Can I just drop your previous patch and you send me another - more restrictive -
> > one ?
> 
> Yeah, dropping is fine. There is no need for another patch after that, this was
> a patch only dealing with the AXi series.
> 
Done.

Thanks,
Guenter
Thomas Backlund Nov. 30, 2020, 4:25 p.m. UTC | #15
Den 30.11.2020 kl. 16:49, skrev Wilken Gottwalt:
> On Mon, 30 Nov 2020 14:43:44 +0200
> Thomas Backlund <tmb@iki.fi> wrote:
>
>> Den 28.11.2020 kl. 12:35, skrev Wilken Gottwalt:
>>> On Sat, 28 Nov 2020 02:37:38 -0300
>>> Jonas Malaco <jonas@protocubo.io> wrote:
>>>
>>>> On Thu, Nov 26, 2020 at 8:43 AM Wilken Gottwalt
>>>> <wilken.gottwalt@posteo.net> wrote:
>>>>> Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
>>>>> AX1500i and AX1600i. The first 3 power supplies are supported through
>>>>> the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
>>>>> converter especially made for the COM ports of these power supplies.
>>>>> There are 3 known revisions of these adapters. The AX1500i power supply
>>>>> has revision 3 built into the case and AX1600i is the only one in that
>>>>> series, which has an unique usb hid id like the RM/RX series.
>>>> Can I ask what AXi power supplies were tested?
>>>>
>>>> I ask because, based on the user-space implementations I am aware of,
>>>> the AXi dongle protocol appears to be different from the RMi/HXi series.
>>> I was not able to test this against the AX power supplies, they are really
>>> hard to find (and are far to expensive). But I went through all these tools
>>> and stuck to the most common commands, which all 3 series support. Not every
>>> series supports all commands (there also seem to be different firmwares in
>>> the micro-conrollers). But this is fine, some sensors will show up as N/A.
>>> Even my HX850i does not support all commands covered in this driver.
>>>
>> What kind of tests do you want / need ?
>>
>> I have an AX860i here.
> Oh nice. Lets start with an usb info dump. Can you give me the full dump of
> lsusb -vd <device> of the Corsair USB dongle?

$ lsusb  -vd  1b1c:1c00

Bus 011 Device 005: ID 1b1c:1c00 Corsair Controller for Corsair Link
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               1.10
   bDeviceClass            0
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        64
   idVendor           0x1b1c Corsair
   idProduct          0x1c00 Controller for Corsair Link
   bcdDevice            1.00
   iManufacturer           1 Silicon Labs
   iProduct                2 Corsair Link TM USB Dongle
   iSerial                 3 R7480347
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength       0x0020
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0x80
       (Bus Powered)
     MaxPower              100mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           2
       bInterfaceClass       255 Vendor Specific Class
       bInterfaceSubClass      0
       bInterfaceProtocol      0
       iInterface              2 Corsair Link TM USB Dongle
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0040  1x 64 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x01  EP 1 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0040  1x 64 bytes
         bInterval               0
can't get debug descriptor: Resource temporarily unavailable
Device Status:     0x0000
   (Bus Powered)

--

Thomas
Wilken Gottwalt Dec. 1, 2020, 7:24 a.m. UTC | #16
On Mon, 30 Nov 2020 18:25:30 +0200
Backlund Thomas <tmb@iki.fi> wrote:

> Den 30.11.2020 kl. 16:49, skrev Wilken Gottwalt:
> > On Mon, 30 Nov 2020 14:43:44 +0200
> > Thomas Backlund <tmb@iki.fi> wrote:
> >
> >> Den 28.11.2020 kl. 12:35, skrev Wilken Gottwalt:
> >>> On Sat, 28 Nov 2020 02:37:38 -0300
> >>> Jonas Malaco <jonas@protocubo.io> wrote:
> >>>
> >>>> On Thu, Nov 26, 2020 at 8:43 AM Wilken Gottwalt
> >>>> <wilken.gottwalt@posteo.net> wrote:
> >>>>> Adds support for another Corsair PSUs series: AX760i, AX860i, AX1200i,
> >>>>> AX1500i and AX1600i. The first 3 power supplies are supported through
> >>>>> the Corsair Link USB Dongle which is some kind of USB/Serial/TTL
> >>>>> converter especially made for the COM ports of these power supplies.
> >>>>> There are 3 known revisions of these adapters. The AX1500i power supply
> >>>>> has revision 3 built into the case and AX1600i is the only one in that
> >>>>> series, which has an unique usb hid id like the RM/RX series.
> >>>> Can I ask what AXi power supplies were tested?
> >>>>
> >>>> I ask because, based on the user-space implementations I am aware of,
> >>>> the AXi dongle protocol appears to be different from the RMi/HXi series.
> >>> I was not able to test this against the AX power supplies, they are really
> >>> hard to find (and are far to expensive). But I went through all these tools
> >>> and stuck to the most common commands, which all 3 series support. Not every
> >>> series supports all commands (there also seem to be different firmwares in
> >>> the micro-conrollers). But this is fine, some sensors will show up as N/A.
> >>> Even my HX850i does not support all commands covered in this driver.
> >>>
> >> What kind of tests do you want / need ?
> >>
> >> I have an AX860i here.
> > Oh nice. Lets start with an usb info dump. Can you give me the full dump of
> > lsusb -vd <device> of the Corsair USB dongle?
> 
> $ lsusb  -vd  1b1c:1c00
> 
> Bus 011 Device 005: ID 1b1c:1c00 Corsair Controller for Corsair Link
> Device Descriptor:
>    bLength                18
>    bDescriptorType         1
>    bcdUSB               1.10
>    bDeviceClass            0
>    bDeviceSubClass         0
>    bDeviceProtocol         0
>    bMaxPacketSize0        64
>    idVendor           0x1b1c Corsair
>    idProduct          0x1c00 Controller for Corsair Link
>    bcdDevice            1.00
>    iManufacturer           1 Silicon Labs
>    iProduct                2 Corsair Link TM USB Dongle
>    iSerial                 3 R7480347
>    bNumConfigurations      1
>    Configuration Descriptor:
>      bLength                 9
>      bDescriptorType         2
>      wTotalLength       0x0020
>      bNumInterfaces          1
>      bConfigurationValue     1
>      iConfiguration          0
>      bmAttributes         0x80
>        (Bus Powered)
>      MaxPower              100mA
>      Interface Descriptor:
>        bLength                 9
>        bDescriptorType         4
>        bInterfaceNumber        0
>        bAlternateSetting       0
>        bNumEndpoints           2
>        bInterfaceClass       255 Vendor Specific Class
>        bInterfaceSubClass      0
>        bInterfaceProtocol      0
>        iInterface              2 Corsair Link TM USB Dongle
>        Endpoint Descriptor:
>          bLength                 7
>          bDescriptorType         5
>          bEndpointAddress     0x81  EP 1 IN
>          bmAttributes            2
>            Transfer Type            Bulk
>            Synch Type               None
>            Usage Type               Data
>          wMaxPacketSize     0x0040  1x 64 bytes
>          bInterval               0
>        Endpoint Descriptor:
>          bLength                 7
>          bDescriptorType         5
>          bEndpointAddress     0x01  EP 1 OUT
>          bmAttributes            2
>            Transfer Type            Bulk
>            Synch Type               None
>            Usage Type               Data
>          wMaxPacketSize     0x0040  1x 64 bytes
>          bInterval               0
> can't get debug descriptor: Resource temporarily unavailable
> Device Status:     0x0000
>    (Bus Powered)

Thank you. Hmm yeah, this dongle has no usb hid part. So this driver will not
work with this dongle for sure. I already have an idea how to support all 3
series, though it involves a lot of testing. I would provide a github repo
with tools and test kernel modules if you are interested. But I don't know
yet how long it will take, I work on other stuff which has a higher prio.

greetings,
Wilken

> --
> 
> Thomas
> 
>
Thomas Backlund Dec. 1, 2020, 3:20 p.m. UTC | #17
Den 01.12.2020 kl. 09:24, skrev Wilken Gottwalt:

> Thank you. Hmm yeah, this dongle has no usb hid part. So this driver will not
> work with this dongle for sure. I already have an idea how to support all 3
> series, though it involves a lot of testing. I would provide a github repo
> with tools and test kernel modules if you are interested. But I don't know
> yet how long it will take, I work on other stuff which has a higher prio.
>

Yes, I'm interested...

Just drop me a note when you have something to test.


--

Thomas
diff mbox series

Patch

diff --git a/Documentation/hwmon/corsair-psu.rst b/Documentation/hwmon/corsair-psu.rst
index 396b95c9a76a..6227e9046d73 100644
--- a/Documentation/hwmon/corsair-psu.rst
+++ b/Documentation/hwmon/corsair-psu.rst
@@ -7,6 +7,16 @@  Supported devices:
 
 * Corsair Power Supplies
 
+  Corsair AX760i (by Corsair Link USB Dongle)
+
+  Corsair AX860i (by Corsair Link USB Dongle)
+
+  Corsair AX1200i (by Corsair Link USB Dongle)
+
+  Corsair AX1500i (by builtin Corsair Link USB Dongle)
+
+  Corsair AX1600i
+
   Corsair HX550i
 
   Corsair HX650i
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 716df51edc87..3c059fc23cd6 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -453,11 +453,12 @@  config SENSORS_CORSAIR_PSU
 	tristate "Corsair PSU HID controller"
 	depends on HID
 	help
-	  If you say yes here you get support for Corsair PSUs with a HID
+	  If you say yes here you get support for Corsair PSUs with an USB HID
 	  interface.
 	  Currently this driver supports the (RM/HX)550i, (RM/HX)650i,
-	  (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i and HX1200i power supplies
-	  by Corsair.
+	  (RM/HX)750i, (RM/HX)850i, (RM/HX)1000i, HX1200i and AX1600i power
+	  supplies by Corsair. The AX760i, AX860i, AX1200i and AX1500i
+	  power supplies are supported through the Corsair Link USB Dongle.
 
 	  This driver can also be built as a module. If so, the module
 	  will be called corsair-psu.
diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
index 99494056f4bd..0146dda3e2c3 100644
--- a/drivers/hwmon/corsair-psu.c
+++ b/drivers/hwmon/corsair-psu.c
@@ -571,17 +571,28 @@  static int corsairpsu_raw_event(struct hid_device *hdev, struct hid_report *repo
 }
 
 static const struct hid_device_id corsairpsu_idtable[] = {
-	{ HID_USB_DEVICE(0x1b1c, 0x1c03) }, /* Corsair HX550i */
-	{ HID_USB_DEVICE(0x1b1c, 0x1c04) }, /* Corsair HX650i */
-	{ HID_USB_DEVICE(0x1b1c, 0x1c05) }, /* Corsair HX750i */
-	{ HID_USB_DEVICE(0x1b1c, 0x1c06) }, /* Corsair HX850i */
-	{ HID_USB_DEVICE(0x1b1c, 0x1c07) }, /* Corsair HX1000i */
-	{ HID_USB_DEVICE(0x1b1c, 0x1c08) }, /* Corsair HX1200i */
-	{ HID_USB_DEVICE(0x1b1c, 0x1c09) }, /* Corsair RM550i */
-	{ HID_USB_DEVICE(0x1b1c, 0x1c0a) }, /* Corsair RM650i */
-	{ HID_USB_DEVICE(0x1b1c, 0x1c0b) }, /* Corsair RM750i */
-	{ HID_USB_DEVICE(0x1b1c, 0x1c0c) }, /* Corsair RM850i */
-	{ HID_USB_DEVICE(0x1b1c, 0x1c0d) }, /* Corsair RM1000i */
+	/*
+	 * The Corsair USB/COM Dongles appear in at least 3 different revisions, where rev 1 and 2
+	 * are commonly used with the AX760i, AX860i and AX1200i, while rev3 is rarely seen with
+	 * these PSUs. Rev3 is also build into the AX1500i, while the AX1600i is the first PSU of
+	 * this series which has an unique usb hid id. Though, the actual device name is part of
+	 * the HID message protocol, so it doesn't matter which dongle is connected.
+	 */
+	{ HID_USB_DEVICE(0x1B1C, 0x1C00) }, /* Corsair Link USB/COM Dongle rev1 */
+	{ HID_USB_DEVICE(0x1B1C, 0x1C01) }, /* Corsair Link USB/COM Dongle rev2 */
+	{ HID_USB_DEVICE(0x1B1C, 0x1C02) }, /* Corsair Link USB/COM Dongle rev3 (AX1500i) */
+	{ HID_USB_DEVICE(0x1B1C, 0x1C03) }, /* Corsair HX550i */
+	{ HID_USB_DEVICE(0x1B1C, 0x1C04) }, /* Corsair HX650i */
+	{ HID_USB_DEVICE(0x1B1C, 0x1C05) }, /* Corsair HX750i */
+	{ HID_USB_DEVICE(0x1B1C, 0x1C06) }, /* Corsair HX850i */
+	{ HID_USB_DEVICE(0x1B1C, 0x1C07) }, /* Corsair HX1000i */
+	{ HID_USB_DEVICE(0x1B1C, 0x1C08) }, /* Corsair HX1200i */
+	{ HID_USB_DEVICE(0x1B1C, 0x1C09) }, /* Corsair RM550i */
+	{ HID_USB_DEVICE(0x1B1C, 0x1C0A) }, /* Corsair RM650i */
+	{ HID_USB_DEVICE(0x1B1C, 0x1C0B) }, /* Corsair RM750i */
+	{ HID_USB_DEVICE(0x1B1C, 0x1C0C) }, /* Corsair RM850i */
+	{ HID_USB_DEVICE(0x1B1C, 0x1C0D) }, /* Corsair RM1000i */
+	{ HID_USB_DEVICE(0x1B1C, 0x1C11) }, /* Corsair AX1600i */
 	{ },
 };
 MODULE_DEVICE_TABLE(hid, corsairpsu_idtable);