Re: [PATCH v8 11/11] pwm: Add Raspberry Pi Firmware based PWM bus

[Uwe Kleine-König]

Hello Nicolas,

On Fri, Mar 12, 2021 at 01:24:54PM +0100, Nicolas Saenz Julienne wrote:
> Adds support to control the PWM bus available in official Raspberry Pi
> PoE HAT. Only RPi's co-processor has access to it, so commands have to
> be sent through RPi's firmware mailbox interface.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@xxxxxxx>
> 
> ---
> 
> Changes since v7:
>  - Remove unwarranted RPI_PWM_DEF_DUTY_REG usage
> 
>  Changes since v6:
> - Use %pe
> - Round divisions properly
> - Use dev_err_probe()
> - Pass check_patch
> 
> Changes since v3:
>  - Rename compatible string to be more explicit WRT to bus's limitations
> 
> Changes since v2:
>  - Use devm_rpi_firmware_get()
>  - Rename driver
>  - Small cleanups
> 
> Changes since v1:
>  - Use default pwm bindings and get rid of xlate() function
>  - Correct spelling errors
>  - Correct apply() function
>  - Round values
>  - Fix divisions in arm32 mode
>  - Small cleanups
> 
>  drivers/pwm/Kconfig               |   9 ++
>  drivers/pwm/Makefile              |   1 +
>  drivers/pwm/pwm-raspberrypi-poe.c | 206 ++++++++++++++++++++++++++++++
>  3 files changed, 216 insertions(+)
>  create mode 100644 drivers/pwm/pwm-raspberrypi-poe.c
> 
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index a7a7a9f26aef..d3371ac7b871 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -431,6 +431,15 @@ config PWM_PXA
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called pwm-pxa.
>  
> +config PWM_RASPBERRYPI_POE
> +	tristate "Raspberry Pi Firwmware PoE Hat PWM support"
> +	# Make sure not 'y' when RASPBERRYPI_FIRMWARE is 'm'. This can only
> +	# happen when COMPILE_TEST=y, hence the added !RASPBERRYPI_FIRMWARE.
> +	depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE)
> +	help
> +	  Enable Raspberry Pi firmware controller PWM bus used to control the
> +	  official RPI PoE hat
> +
>  config PWM_RCAR
>  	tristate "Renesas R-Car PWM support"
>  	depends on ARCH_RENESAS || COMPILE_TEST
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index 4e35a55fa7b6..d3879619bd76 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -39,6 +39,7 @@ obj-$(CONFIG_PWM_NTXEC)		+= pwm-ntxec.o
>  obj-$(CONFIG_PWM_OMAP_DMTIMER)	+= pwm-omap-dmtimer.o
>  obj-$(CONFIG_PWM_PCA9685)	+= pwm-pca9685.o
>  obj-$(CONFIG_PWM_PXA)		+= pwm-pxa.o
> +obj-$(CONFIG_PWM_RASPBERRYPI_POE)	+= pwm-raspberrypi-poe.o
>  obj-$(CONFIG_PWM_RCAR)		+= pwm-rcar.o
>  obj-$(CONFIG_PWM_RENESAS_TPU)	+= pwm-renesas-tpu.o
>  obj-$(CONFIG_PWM_ROCKCHIP)	+= pwm-rockchip.o
> diff --git a/drivers/pwm/pwm-raspberrypi-poe.c b/drivers/pwm/pwm-raspberrypi-poe.c
> new file mode 100644
> index 000000000000..71ade5e55069
> --- /dev/null
> +++ b/drivers/pwm/pwm-raspberrypi-poe.c
> @@ -0,0 +1,206 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2020 Nicolas Saenz Julienne <nsaenzjulienne@xxxxxxx>

2021?

> + * For more information on Raspberry Pi's PoE hat see:
> + * https://www.raspberrypi.org/products/poe-hat/

Out of personal interest: Is this hat also able to power a RPi CM4?

> + * Limitations:
> + *  - No disable bit, so a disabled PWM is simulated by duty_cycle 0
> + *  - Only normal polarity
> + *  - Fixed 12.5 kHz period
> + *
> + * The current period is completed when HW is reconfigured.
> + */

Other than that as mentioned in the previous round: This looks good,

Reviewed-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>

What is your thought about how to get this series merged? At least
input, staging, armsoc, clk, reset anf firmware are touched. Do you
prepare a branch for merging in the relevant trees (once you have all
the necessary Acks)?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature