All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Heiko Stübner" <heiko@sntech.de>
To: "Linus Walleij" <linus.walleij@linaro.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Uwe Kleine-König" <ukleinek@kernel.org>,
	"William Breathitt Gray" <wbg@kernel.org>,
	"Sebastian Reichel" <sebastian.reichel@collabora.com>,
	"Kever Yang" <kever.yang@rock-chips.com>,
	"Nicolas Frattaroli" <nicolas.frattaroli@collabora.com>
Cc: linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-pwm@vger.kernel.org, linux-iio@vger.kernel.org,
	kernel@collabora.com, Jonas Karlman <jonas@kwiboo.se>,
	Detlev Casanova <detlev.casanova@collabora.com>,
	Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
Subject: Re: [PATCH 4/7] soc: rockchip: add mfpwm driver
Date: Sat, 31 May 2025 23:48:29 +0200	[thread overview]
Message-ID: <2188729.OBFZWjSADL@diego> (raw)
In-Reply-To: <20250408-rk3576-pwm-v1-4-a49286c2ca8e@collabora.com>

Am Dienstag, 8. April 2025, 14:32:16 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@collabora.com>

actually trying to compile this, led me to

aarch64-linux-gnu-ld: drivers/soc/rockchip/mfpwm.o: in function `mfpwm_reg_read':
/home/devel/hstuebner/00_git-repos/linux-rockchip/_build-arm64/../include/soc/rockchip/mfpwm.h:423: multiple definition of `mfpwm_reg_read'; drivers/pwm/pwm-rockchip-v4.o:/home/devel/hstuebner/00_git-repos/linux-rockchip/_build-arm64/../include/soc/rockchip/mfpwm.h:423: first defined here
aarch64-linux-gnu-ld: drivers/soc/rockchip/mfpwm.o: in function `mfpwm_reg_write':
/home/devel/hstuebner/00_git-repos/linux-rockchip/_build-arm64/../include/soc/rockchip/mfpwm.h:428: multiple definition of `mfpwm_reg_write'; drivers/pwm/pwm-rockchip-v4.o:/home/devel/hstuebner/00_git-repos/linux-rockchip/_build-arm64/../include/soc/rockchip/mfpwm.h:428: first defined here
make[3]: *** [../scripts/Makefile.vmlinux_o:72: vmlinux.o] Fehler 1


during the linking stage - with the driver as builtin


> +inline u32 mfpwm_reg_read(void __iomem *base, u32 reg)
> +{
> +	return readl(base + reg);
> +}
> +
> +inline void mfpwm_reg_write(void __iomem *base, u32 reg, u32 val)
> +{
> +	writel(val, base + reg);
> +}

making that a "static inline ..." solves that.


On a more general note, what is the differentiation to an MFD here?

Like you can already bind dt-nodes to MFD subdevices, and can implement
the exclusivity API thing on top of a general mfd device, to make sure only
one mfd-cell gets activated at one time.

Other than that, this looks like it reimplements MFDs?

Also handing around a regmap might be nicer, compared to readl/writel.


Heiko





WARNING: multiple messages have this Message-ID (diff)
From: "Heiko Stübner" <heiko@sntech.de>
To: "Linus Walleij" <linus.walleij@linaro.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Uwe Kleine-König" <ukleinek@kernel.org>,
	"William Breathitt Gray" <wbg@kernel.org>,
	"Sebastian Reichel" <sebastian.reichel@collabora.com>,
	"Kever Yang" <kever.yang@rock-chips.com>,
	"Nicolas Frattaroli" <nicolas.frattaroli@collabora.com>
Cc: linux-pwm@vger.kernel.org, linux-rockchip@lists.infradead.org,
	linux-iio@vger.kernel.org, Jonas Karlman <jonas@kwiboo.se>,
	linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org,
	Detlev Casanova <detlev.casanova@collabora.com>,
	kernel@collabora.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 4/7] soc: rockchip: add mfpwm driver
Date: Sat, 31 May 2025 23:48:29 +0200	[thread overview]
Message-ID: <2188729.OBFZWjSADL@diego> (raw)
In-Reply-To: <20250408-rk3576-pwm-v1-4-a49286c2ca8e@collabora.com>

Am Dienstag, 8. April 2025, 14:32:16 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@collabora.com>

actually trying to compile this, led me to

aarch64-linux-gnu-ld: drivers/soc/rockchip/mfpwm.o: in function `mfpwm_reg_read':
/home/devel/hstuebner/00_git-repos/linux-rockchip/_build-arm64/../include/soc/rockchip/mfpwm.h:423: multiple definition of `mfpwm_reg_read'; drivers/pwm/pwm-rockchip-v4.o:/home/devel/hstuebner/00_git-repos/linux-rockchip/_build-arm64/../include/soc/rockchip/mfpwm.h:423: first defined here
aarch64-linux-gnu-ld: drivers/soc/rockchip/mfpwm.o: in function `mfpwm_reg_write':
/home/devel/hstuebner/00_git-repos/linux-rockchip/_build-arm64/../include/soc/rockchip/mfpwm.h:428: multiple definition of `mfpwm_reg_write'; drivers/pwm/pwm-rockchip-v4.o:/home/devel/hstuebner/00_git-repos/linux-rockchip/_build-arm64/../include/soc/rockchip/mfpwm.h:428: first defined here
make[3]: *** [../scripts/Makefile.vmlinux_o:72: vmlinux.o] Fehler 1


during the linking stage - with the driver as builtin


> +inline u32 mfpwm_reg_read(void __iomem *base, u32 reg)
> +{
> +	return readl(base + reg);
> +}
> +
> +inline void mfpwm_reg_write(void __iomem *base, u32 reg, u32 val)
> +{
> +	writel(val, base + reg);
> +}

making that a "static inline ..." solves that.


On a more general note, what is the differentiation to an MFD here?

Like you can already bind dt-nodes to MFD subdevices, and can implement
the exclusivity API thing on top of a general mfd device, to make sure only
one mfd-cell gets activated at one time.

Other than that, this looks like it reimplements MFDs?

Also handing around a regmap might be nicer, compared to readl/writel.


Heiko




_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

  parent reply	other threads:[~2025-05-31 21:53 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-08 12:32 [PATCH 0/7] Add Rockchip RK3576 PWM Support Through MFPWM Nicolas Frattaroli
2025-04-08 12:32 ` Nicolas Frattaroli
2025-04-08 12:32 ` [PATCH 1/7] dt-bindings: pinctrl: rockchip: increase max amount of device functions Nicolas Frattaroli
2025-04-08 12:32   ` Nicolas Frattaroli
2025-04-08 16:08   ` Conor Dooley
2025-04-08 16:08     ` Conor Dooley
2025-04-08 17:27   ` Rob Herring
2025-04-08 17:27     ` Rob Herring
2025-05-31 12:59   ` Heiko Stübner
2025-05-31 12:59     ` Heiko Stübner
2025-04-08 12:32 ` [PATCH 2/7] dt-bindings: pwm: Add a new binding for rockchip,rk3576-pwm Nicolas Frattaroli
2025-04-08 12:32   ` Nicolas Frattaroli
2025-04-08 16:07   ` Conor Dooley
2025-04-08 16:07     ` Conor Dooley
2025-04-08 12:32 ` [PATCH 3/7] soc: rockchip: add utils header for things shared across drivers Nicolas Frattaroli
2025-04-08 12:32   ` Nicolas Frattaroli
2025-05-31 13:26   ` Heiko Stübner
2025-05-31 13:26     ` Heiko Stübner
2025-04-08 12:32 ` [PATCH 4/7] soc: rockchip: add mfpwm driver Nicolas Frattaroli
2025-04-08 12:32   ` Nicolas Frattaroli
2025-04-08 20:03   ` Heiko Stübner
2025-04-08 20:03     ` Heiko Stübner
2025-04-09 13:01     ` Nicolas Frattaroli
2025-04-09 13:01       ` Nicolas Frattaroli
2025-05-08  7:13       ` Damon Ding
2025-05-08  7:13         ` Damon Ding
2025-05-31 21:48   ` Heiko Stübner [this message]
2025-05-31 21:48     ` Heiko Stübner
2025-06-02 12:15     ` Nicolas Frattaroli
2025-06-02 12:15       ` Nicolas Frattaroli
2025-06-02 13:14       ` Heiko Stübner
2025-06-02 13:14         ` Heiko Stübner
2025-04-08 12:32 ` [PATCH 5/7] pwm: Add rockchip PWMv4 driver Nicolas Frattaroli
2025-04-08 12:32   ` Nicolas Frattaroli
2025-05-13 17:26   ` Uwe Kleine-König
2025-05-13 17:26     ` Uwe Kleine-König
2025-05-22 13:02     ` Nicolas Frattaroli
2025-05-22 13:02       ` Nicolas Frattaroli
2025-05-23 15:02       ` Uwe Kleine-König
2025-05-23 15:02         ` Uwe Kleine-König
2025-05-26  9:30         ` Nicolas Frattaroli
2025-05-26  9:30           ` Nicolas Frattaroli
2025-04-08 12:32 ` [PATCH 6/7] counter: Add rockchip-pwm-capture driver Nicolas Frattaroli
2025-04-08 12:32   ` Nicolas Frattaroli
2025-05-07  8:47   ` William Breathitt Gray
2025-05-07  8:47     ` William Breathitt Gray
2025-04-08 12:32 ` [PATCH 7/7] arm64: dts: rockchip: add PWM nodes to RK3576 SoC dtsi Nicolas Frattaroli
2025-04-08 12:32   ` Nicolas Frattaroli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2188729.OBFZWjSADL@diego \
    --to=heiko@sntech.de \
    --cc=conor+dt@kernel.org \
    --cc=detlev.casanova@collabora.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jonas@kwiboo.se \
    --cc=kernel@collabora.com \
    --cc=kever.yang@rock-chips.com \
    --cc=krzk+dt@kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=nicolas.frattaroli@collabora.com \
    --cc=robh@kernel.org \
    --cc=sebastian.reichel@collabora.com \
    --cc=ukleinek@kernel.org \
    --cc=wbg@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.