[v2,0/7] Drivers to support the MCU on QNAP NAS devices

This implements a set of drivers for the MCU used on QNAP NAS devices.

Of course no documentation for the serial protocol is available, so
thankfully QNAP has a tool on their rescue-inird to talk to the MCU and
I found interceptty [0] to listen to what goes over the serial connection.

In general it looks like there are two different generations in general,
an "EC" device and now this "MCU" - referenced in the strings of the
userspace handlers for those devices.

For the MCU "SPEC3" and "SPEC4" are listed which is configured in
the model.conf of the device. When setting the value from SPEC4 to
SPEC3 on my TS433, the supported commands change, but the command
interface stays the same and especially the version command is the
same.

The binding also does not expose any interals of the device that
might change, so hopefully there shouldn't be big roadblocks to
support different devices, apart from possibly adapting the commands.

changes in v2:
binding:
- rename to qnap,ts433-mcu.yaml (Krzysztof)
- drop "preserve formatting" indicator (Krzysztof)
- add Krzysztof's Review tag

mfd:
- fix checkpatch --strict CHECKs
- add a MAINTAINERS entry for all qnap-mcu-parts

hwmon:
address Guenter's review comments:
- fix checkpatch strict warnings
  I've kept the devm_thermal_of_cooling_device_register alignment,
  because that line is so long that aligning to the "(" would make
  things way too long and unreadable
- add hwmon documentation
- spelling corrections
- report actual pwm value, not last-set one
- make some cmd arrays static
- drop pwm_enable as the pwm-mode is not controllable
- actually handle error returns from mcu commands
- fix calculation of fan-rpm (I read my notes wrong)
- fix temperature calculation to return millicelsius as expected
- only bail at obviously wrong pwm values, but clamp to min,max
- only register cooling-device if cooling-levels are available


[0] https://github.com/geoffmeyers/interceptty

Heiko Stuebner (7):
  dt-bindings: mfd: add binding for qnap,ts433-mcu devices
  mfd: add base driver for qnap-mcu devices
  leds: add driver for LEDs from qnap-mcu devices
  Input: add driver for the input part of qnap-mcu devices
  hwmon: add driver for the hwmon parts of qnap-mcu devices
  arm64: dts: rockchip: hook up the MCU on the QNAP TS433
  arm64: dts: rockchip: set hdd led labels on qnap-ts433

 .../bindings/mfd/qnap,ts433-mcu.yaml          |  43 ++
 Documentation/hwmon/index.rst                 |   1 +
 Documentation/hwmon/qnap-mcu-hwmon.rst        |  27 ++
 MAINTAINERS                                   |   9 +
 .../boot/dts/rockchip/rk3568-qnap-ts433.dts   |  58 +++
 drivers/hwmon/Kconfig                         |  12 +
 drivers/hwmon/Makefile                        |   1 +
 drivers/hwmon/qnap-mcu-hwmon.c                | 375 ++++++++++++++++++
 drivers/input/misc/Kconfig                    |  12 +
 drivers/input/misc/Makefile                   |   1 +
 drivers/input/misc/qnap-mcu-input.c           | 156 ++++++++
 drivers/leds/Kconfig                          |  11 +
 drivers/leds/Makefile                         |   1 +
 drivers/leds/leds-qnap-mcu.c                  | 247 ++++++++++++
 drivers/mfd/Kconfig                           |  10 +
 drivers/mfd/Makefile                          |   2 +
 drivers/mfd/qnap-mcu.c                        | 358 +++++++++++++++++
 include/linux/mfd/qnap-mcu.h                  |  28 ++
 18 files changed, 1352 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/qnap,ts433-mcu.yaml
 create mode 100644 Documentation/hwmon/qnap-mcu-hwmon.rst
 create mode 100644 drivers/hwmon/qnap-mcu-hwmon.c
 create mode 100644 drivers/input/misc/qnap-mcu-input.c
 create mode 100644 drivers/leds/leds-qnap-mcu.c
 create mode 100644 drivers/mfd/qnap-mcu.c
 create mode 100644 include/linux/mfd/qnap-mcu.h

Hi Florian,

Am Montag, 29. Juli 2024, 08:24:26 CEST schrieb Florian Eckert:
> > +static int qnap_mcu_register_err_led(struct device *dev, struct
> > qnap_mcu *mcu, int num)
> > +{
> > +	struct qnap_mcu_err_led *err_led;
> > +	char tmp_buf[LED_MAX_NAME_SIZE];
> > +	int ret;
> > +
> > +	err_led = devm_kzalloc(dev, sizeof(*err_led), GFP_KERNEL);
> > +	if (!err_led)
> > +		return -ENOMEM;
> > +
> > +	err_led->mcu = mcu;
> > +	err_led->num = num;
> > +	err_led->mode = QNAP_MCU_ERR_LED_OFF;
> > +
> > +	snprintf(tmp_buf, LED_MAX_NAME_SIZE, "hdd%d:red:status", num + 1);
> > +	err_led->cdev.name = tmp_buf;
> 
> Should not the memory have to be allocated here via 'kzalloc' for 
> 'err_led->cdev.name'?
> After leaving the function, tmp_buf is no longer on the stack?

Reading the led_classdev_register thing, cdev->name is used only for
creating the final-name for the LED and thus copied into yet another
temporary buffer [0] .

And cdev->name is not accessed anymore outside the register function.

But thinking more about that, you're still right, because after registering
cdev->name is in a bad state, pointing to something no valid anymore.
So if the LED core changes behaviour in the future, this will cause breakage.

So I'll change that.


Thanks for catching that
Heiko



[0] https://elixir.bootlin.com/linux/v6.10.1/source/drivers/leds/led-class.c#L500
https://elixir.bootlin.com/linux/v6.10.1/source/drivers/leds/led-class.c#L503