Re: [PATCH RFC 0/4] hwmon: Add fan tachometer support for Raspberry Pi RP1

[Guenter Roeck <linux@roeck-us.net>]

On 10/23/26 03:15, Andrea della Porta wrote:
> Hi,
> 
> This patch series adds fan tachometer / RPM monitoring support for the
> Raspberry Pi RP1 southbridge found on the Raspberry Pi 5.
> Please note that the patchset in [1] is a prerequisite for this one.
> 
> On the Raspberry Pi RP1, the active cooling fan speed is controlled via
> the RP1 PWM controller. In addition to generating PWM signals, the RP1
> monitors fan tachometer pulses. The pulse measurement and RPM calculation
> are performed autonomously by the RP1's onboard Cortex-M microcontroller
> core, which periodically writes the computed RPM value directly into
> dedicated MMIO registers located within the RP1 PWM hardware block.
> 
> During earlier discussions, Uwe Kleine-König explicitly indicated a
> preference to keep the core PWM controller driver (drivers/pwm/pwm-rp1.c)
> focused strictly on PWM duties, rather than bloating it by registering an
> hwmon device directly from within the PWM driver (see [2]).
> 
> Because the Linux PWM subsystem currently lacks a generic framework or
> consumer API for reporting hardware-calculated tachometer readings, this
> series implements a standalone hwmon driver (rp1-fan) using the following
> structure:
> 
> -  The PWM driver exposes and exports rp1_pwm_read_tachometer() within a
>     dedicated symbol namespace ('RP1_PWM') to read the RPM value from
>     the MMIO block.
> -  A companion DT binding (raspberrypi,rp1-fan.yaml) introduces a vendor
>     property, 'raspberrypi,pwm-tachometer', to link the fan node to the
>     PWM controller node without overloading or abusing the standard 'pwms'
>     property.
> -  The rp1-fan hwmon driver parses this phandle, establishes a device link
>     for proper runtime and unbind dependency management, and registers the
>     standard hwmon 'fan1_input' sysfs attribute.
> 
> An alternative architectural approach to the DT phandle + exported helper
> method would be to model the tachometer as a child device using the
> auxiliary bus framework (drivers/base/auxiliary.c). In that model, the RP1
> PWM driver would instantiate an auxiliary device for the tachometer, and
> the hwmon driver would bind to that auxiliary bus device without requiring
> a separate DT node.
> 
> However, adopting the auxiliary bus would require a more significant
> rework and refactoring of the existing RP1 PWM driver.
> 
> I'd appreciate any feedback on whether the current companion driver with
> a dedicated DT property is acceptable, or if an auxiliary bus-based
> refactoring of the PWM driver is the preferred upstream path.
> 
> Patch Breakdown
> ---------------
> Patch 1: pwm: rp1: Export helper to read fan tachometer RPM
> Patch 2: dt-bindings: hwmon: Add Raspberry Pi RP1 fan binding
> Patch 3: hwmon: Add driver for Raspberry Pi RP1 fan tachometer

This seems conceptually wrong.

Have you considered implementing the hwmon driver as auxiliary driver
and accessing the register in question through regmap ?

> Patch 4: arm64: dts: broadcom: bcm2712-rpi-5-b: Add RP1 fan tachometer node
> 
> Thanks,
> Andrea
> 
> 
> LINKS:
> [1] - https://lore.kernel.org/all/cover.1790686331.git.andrea.porta@suse.com/
> [2] - https://lore.kernel.org/all/adLTwOTbkJ0VQXy6@monoceros/
> 
> Andrea della Porta (4):
>    pwm: rp1: Export helper to read fan tachometer RPM
>    dt-bindings: hwmon: Add Raspberry Pi RP1 fan binding
>    hwmon: Add driver for Raspberry Pi RP1 fan tachometer
>    arm64: dts: broadcom: bcm2712-rpi-5-b: Add RP1 fan tachometer node
> 
>   .../bindings/hwmon/raspberrypi,rp1-fan.yaml   |  39 +++++
>   Documentation/hwmon/index.rst                 |   1 +
>   Documentation/hwmon/rp1-fan.rst               |  41 ++++++
>   MAINTAINERS                                   |   1 +
>   .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     |   5 +
>   drivers/hwmon/Kconfig                         |  11 ++
>   drivers/hwmon/Makefile                        |   1 +
>   drivers/hwmon/rp1-hwmon.c                     | 136 ++++++++++++++++++
>   drivers/pwm/pwm-rp1.c                         |  27 ++++

Either case, there is no such driver in mainline or linux-next.

Guenter

>   include/soc/bcm2835/raspberrypi-pwm-rp1.h     |  10 ++
>   10 files changed, 272 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-fan.yaml
>   create mode 100644 Documentation/hwmon/rp1-fan.rst
>   create mode 100644 drivers/hwmon/rp1-hwmon.c
>   create mode 100644 include/soc/bcm2835/raspberrypi-pwm-rp1.h
>