linux-kernel - Re: [PATCH v2 4/7] soc: rockchip: add mfpwm driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8203440.zQ0Gbyo6oJ@diego>
Date: Wed, 09 Jul 2025 09:22:57 +0200
From: Heiko Stübner <heiko@...ech.de>
To: Linus Walleij <linus.walleij@...aro.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Uwe Kleine-König <ukleinek@...nel.org>,
 William Breathitt Gray <wbg@...nel.org>,
 Sebastian Reichel <sebastian.reichel@...labora.com>,
 Kever Yang <kever.yang@...k-chips.com>, Yury Norov <yury.norov@...il.com>,
 Rasmus Villemoes <linux@...musvillemoes.dk>,
 Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Dave Ertman <david.m.ertman@...el.com>, Ira Weiny <ira.weiny@...el.com>,
 Leon Romanovsky <leon@...nel.org>, Lee Jones <lee@...nel.org>,
 linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
 linux-kernel@...r.kernel.org, linux-pwm@...r.kernel.org,
 linux-iio@...r.kernel.org, kernel@...labora.com,
 Jonas Karlman <jonas@...boo.se>,
 Detlev Casanova <detlev.casanova@...labora.com>,
 Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
Subject: Re: [PATCH v2 4/7] soc: rockchip: add mfpwm driver

Hi Nicolas,

Am Montag, 2. Juni 2025, 18:19:15 Mitteleuropäische Sommerzeit schrieb Nicolas Frattaroli:
> With the Rockchip RK3576, the PWM IP used by Rockchip has changed
> substantially. Looking at both the downstream pwm-rockchip driver as
> well as the mainline pwm-rockchip driver made it clear that with all its
> additional features and its differences from previous IP revisions, it
> is best supported in a new driver.
> 
> This brings us to the question as to what such a new driver should be.
> To me, it soon became clear that it should actually be several new
> drivers, most prominently when Uwe Kleine-König let me know that I
> should not implement the pwm subsystem's capture callback, but instead
> write a counter driver for this functionality.
> 
> Combined with the other as-of-yet unimplemented functionality of this
> new IP, it became apparent that it needs to be spread across several
> subsystems.
> 
> For this reason, we add a new platform bus based driver, called mfpwm
> (short for "Multi-function PWM"). This "parent" driver makes sure that
> only one device function driver is using the device at a time, and is in
> charge of registering the platform bus devices for the individual device
> functions offered by the device.
> 
> An acquire/release pattern is used to guarantee that device function
> drivers don't step on each other's toes.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
> ---

> +/**
> + * mfpwm_register_subdev - register a single mfpwm_func
> + * @mfpwm: pointer to the parent &struct rockchip_mfpwm
> + * @target: pointer to where the &struct platform_device pointer should be
> + *          stored, usually a member of @mfpwm
> + * @name: sub-device name string
> + *
> + * Allocate a single &struct mfpwm_func, fill its members with appropriate data,
> + * and register a new platform device, saving its pointer to @target. The
> + * allocation is devres tracked, so will be automatically freed on mfpwm remove.
> + *
> + * Returns: 0 on success, negative errno on error
> + */
> +static int mfpwm_register_subdev(struct rockchip_mfpwm *mfpwm,
> +				 struct platform_device **target,
> +				 const char *name)
> +{
> +	struct rockchip_mfpwm_func *func;
> +	struct platform_device *child;
> +
> +	func = devm_kzalloc(&mfpwm->pdev->dev, sizeof(*func), GFP_KERNEL);
> +	if (IS_ERR(func))
> +		return PTR_ERR(func);
> +	func->irq = mfpwm->irq;
> +	func->parent = mfpwm;
> +	func->id = atomic_inc_return(&subdev_id);
> +	func->base = mfpwm->base;
> +	func->core = mfpwm->chosen_clk;
> +	child = platform_device_register_data(&mfpwm->pdev->dev, name, func->id,
> +					      func, sizeof(*func));
> +
> +	if (IS_ERR(child))
> +		return PTR_ERR(child);
> +
> +	*target = child;
> +
> +	return 0;
> +}
> +
> +static int mfpwm_register_subdevs(struct rockchip_mfpwm *mfpwm)
> +{
> +	int ret;
> +
> +	ret = mfpwm_register_subdev(mfpwm, &mfpwm->pwm_dev, "pwm-rockchip-v4");
> +	if (ret)
> +		return ret;
> +
> +	ret = mfpwm_register_subdev(mfpwm, &mfpwm->counter_dev,
> +				    "rockchip-pwm-capture");
> +	if (ret)
> +		goto err_unreg_pwm_dev;
> +
> +	return 0;
> +
> +err_unreg_pwm_dev:
> +	platform_device_unregister(mfpwm->pwm_dev);
> +
> +	return ret;
> +}

I still had this lingering feeling that this _is_ a MFD just with added
sprinkles, so asked Lee on IRC about it:

	<lag> Looks like an MFD to me
	<lag> Yes, you can use an MFD core driver to control state / manage single-use resources

So, citing Jean Luc Picard, "Make it so" ... please :-)

Heiko