From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0FA42F8A140 for ; Thu, 16 Apr 2026 08:23:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=vf1NAEo8S9OwitHAGAehGzHb+5Xj0NkwKk+qLaje2K4=; b=aw5iIPzoT3b2dkHYwMgo7rh7om Ml07gTYYqndQrRb/z4dqoj+lKAwcO0yvoerjpBBU1QhxkvjMJ/sUDGIWb1ZiLKufIUR7x8cPzy/I6 dvg9wS+Oe1Vh9FEiOVkCNUWPh8bPht9s9poXHV02OWA4/8G3zkFtXzaV04y/KVZeyRPR5TTMpzuyc 1I3PHC2bV6jo4bf/V4wIJ7E+XfT7capY83pz6H4hBqWgo6BwHVsvI7NJoOxEWBhzTOk3+orvsSi5D l+aM7wAF4AxorBF61C7l9L7s6D+O4w8bLGAZY12WaJ3dlhKdOdTFr3tpIB6XzLrtwmTUiaig8gclw AIs7Yyiw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1wDI08-00000002Afd-0nuN; Thu, 16 Apr 2026 08:23:04 +0000 Received: from smtpout-04.galae.net ([185.171.202.116]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1wDI02-00000002Aet-3Dvc for linux-arm-kernel@lists.infradead.org; Thu, 16 Apr 2026 08:23:03 +0000 Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-04.galae.net (Postfix) with ESMTPS id 90103C5C3C5; Thu, 16 Apr 2026 08:23:34 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id 9605B5FDEB; Thu, 16 Apr 2026 08:22:56 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id EB1FD10459ECE; Thu, 16 Apr 2026 10:22:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1776327775; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:content-language:in-reply-to:references; bh=vf1NAEo8S9OwitHAGAehGzHb+5Xj0NkwKk+qLaje2K4=; b=IQOoASDjygSK/g57J/TKAASub60SdimIF6ESLFnep0UEYz75Noo9cFiE+IeunJPxCsUH6W PVLAv7DRh04P+0Pli7o7n/QeXE7ojTi2BVzWzgehw2bGWPal4dqx7oekXOgHs4ldQ4Rn8E l3jBqsY34iqG1U2AlGSipzQ945alsXEJmLzJaHOEo9c7FieaLiPSMH3mv9QBb/W72DnLUg 1B9bSOWckxHBmYXvyDKkNxvy66UYIicEXq8TUOPIE4dAZu8oPzbId01oQ7F71Itq6b6zan Ju5h9IKbufhA05Jk3mCng0d4mUulXbzE4QxLKTjEx5hrnRjA4H3F3FVmvjeZCg== Message-ID: <068e2f41-33fd-4627-826f-83eb0770cc3c@bootlin.com> Date: Thu, 16 Apr 2026 10:22:42 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 0/4] Introduce Allwinner H616 PWM controller To: Paul Kocialkowski Cc: =?UTF-8?Q?Uwe_Kleine-K=C3=B6nig?= , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , Philipp Zabel , Thomas Petazzoni , John Stultz , Joao Schim , linux-pwm@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org References: <20260305091959.2530374-1-richard.genoud@bootlin.com> From: Richard GENOUD Content-Language: en-US, fr Organization: Bootlin In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Last-TLS-Session-Version: TLSv1.3 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260416_012300_783147_88D740E1 X-CRM114-Status: GOOD ( 47.74 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Le 09/04/2026 à 19:16, Paul Kocialkowski a écrit : > Hi Richard, > > On Thu 05 Mar 26, 10:19, Richard Genoud wrote: >> Allwinner H616 PWM controller is quite different from the A10 one. > > As I've mentionned before, this PWM controller is not specific to the H616 > but also appears in other chips, so the name of the driver and registers > should not mention H616. > > After further investigation, I can see multiple versions of this new PWM IP > being used in different chips, starting with the R40/V40 (sun8iw11) in 2016. > > The latest downstream BSP driver has a list of the different generations: > https://github.com/radxa/allwinner-bsp/blob/cubie-aiot-v1.4.6/drivers/pwm/pwm-sunxi.c#L1901 > > We have a first generation called v100/v101 for the following chips: > H616, R328 and R40. A second generation is called v200 and brings slight > register layout differences for A133, D1/T113-S3 and V851. Subsequent > iterations (v201-5) are used in more recent chips like A527 and A733 and > seem register-compatible with v200 (from a quick look). > > So what I suggest here is to rename the driver "sun8i-pwm" and eventually add > a list of generations to the driver and different registers when needed, with > an appropriate suffix in their name. > > But since you're currently only dealing with H616, this work can be done later > when introducing support for more chips. ok, I'm fine with that :) > >> It can drive 6 PWM channels, and like for the A10, each channel has a >> bypass that permits to output a clock, bypassing the PWM logic, when >> enabled. >> >> But, the channels are paired 2 by 2, sharing a first set of >> MUX/prescaler/gate. >> Then, for each channel, there's another prescaler (that will be bypassed >> if the bypass is enabled for this channel). >> >> It looks like that: >> _____ ______ ________ >> OSC24M --->| | | | | | >> APB1 ----->| Mux |--->| Gate |--->| /div_m |-----> PWM_clock_src_xy >> |_____| |______| |________| >> ________ >> | | >> +->| /div_k |---> PWM_clock_x >> | |________| >> | ______ >> | | | >> +-->| Gate |----> PWM_bypass_clock_x >> | |______| >> PWM_clock_src_xy -----+ ________ >> | | | >> +->| /div_k |---> PWM_clock_y >> | |________| >> | ______ >> | | | >> +-->| Gate |----> PWM_bypass_clock_y >> |______| >> >> Where xy can be 0/1, 2/3, 4/5 >> >> PWM_clock_x/y serve for the PWM purpose. >> PWM_bypass_clock_x/y serve for the clock-provider purpose. >> The common clock framework has been used to manage those clocks. >> >> This PWM driver serves as a clock-provider for PWM_bypass_clocks. >> This is needed for example by the embedded AC300 PHY which clock comes >> from PMW5 pin (PB12). >> >> Usually, to get a clock from a PWM driver, we use the pwm-clock driver >> so that the PWM driver doesn't need to be a clk-provider itself. >> While this works in most cases, here it just doesn't. >> That's because the pwm-clock request a period from the PWM driver, >> without any clue that it actually wants a clock at a specific frequency, >> and not a PWM signal with duty cycle capability. > > From what I understand the pwm-clock driver will either assume a fixed rate > set in device-tree or deduce the rate from the pwm period. In any case it will > check that the pwm period (which it cannot change) is the same as the requested > clock period. > > So I agree that pwm-clock is unable to change the clock rate at runtime and will > just use whatever frequency the pwm is running at (which is typically set > in the device-tree consumer property). > >> So, the PWM driver doesn't know if it can use the bypass or not, it >> doesn't even have the real accurate frequency information (23809524 Hz >> instead of 24MHz) because PWM drivers only deal with periods. > > I agree that the driver needs to register as a proper clock provider in > addition to pwm. But what happens if the same PWM clock is requested both from > the clk side and the pwm side? The first to request it is the winner :) The other ones will receive a -EBUSY In h616_pwm_request() and h616_pwm_of_clk_get(), the channel mode is checked, and if it's free to use, it's set as either PWM or CLK mode so that it can't be requested a second time. > >> With pwm-clock, we loose a precious information along the way (that we >> actually want a clock and not a PWM signal). >> That's ok with simple PWM drivers that don't have multiple input clocks, >> but in this case, without this information, we can't know for sure which >> clock to use. >> And here, for instance, if we ask for a 24MHz clock, pwm-clock will >> requests 42ns (assigned-clocks doesn't help for that matter). The logic >> is to select the highest clock (100MHz) with no prescaler and a duty >> cycle value of 2/4 => we have 25MHz instead of 24MHz. >> And that's a perfectly fine choice for a PMW, because we still can >> change the duty cycle in the range [0-4]/4. >> But obviously for a clock, we don't care about the duty cycle, but more >> about the clock accuracy. >> >> And actually, this PWM is really a PWM AND a real clock when the bypass >> is set. > > Make sense to me. > >> This series is based onto v6.19-rc4 >> >> NB: checkpatch is not happy with patch 2, but it's a false positive. >> It doesn't detect that PWM_XY_SRC_MUX/GATE/DIV are structures, but as >> it's more readable like that, I prefer keeping it that way. >> >> NB2: for geopolitical reasons, I didn't re-use the old series that Paul >> was referring to. >> >> Changes since v3: >> - gather Acked-by/Tested-by >> - fix cast from pointer to integer of different size (kernel test robot >> with arc platform) >> - add devm_action for clk_hw_unregister_composite as suggested by Philipp >> - remove now unused pwm_remove as suggested by Philipp >> >> Changes since v2: >> - use U32_MAX instead of defining UINT32_MAX >> - add a comment on U32_MAX usage in clk_round_rate() >> - change clk_table_div_m (use macros) >> - fix formatting (double space, superfluous comma, extra line feed) >> - fix the parent clock order >> - simplify code by using scoped_guard() >> - add missing const in to_h616_pwm_chip() and rename to >> h616_pwm_from_chip() >> - add/remove missing/superflous error messages >> - rename cnt->period_ticks, duty_cnt->duty_ticks >> - fix PWM_PERIOD_MAX >> - add .remove() callback >> - fix DIV_ROUND_CLOSEST_ULL->DIV_ROUND_UP_ULL >> - add H616_ prefix >> - protect _reg in macros >> - switch to waveforms instead of apply/get_state >> - shrink struct h616_pwm_channel >> - rebase on v6.19-rc4 >> >> Changes since v1: >> - rebase onto v6.19-rc1 >> - add missing headers >> - remove MODULE_ALIAS (suggested by Krzysztof) >> - use sun4i-pwm binding instead of creating a new one (suggested by Krzysztof) >> - retrieve the parent clocks from the devicetree >> - switch num_parents to unsigned int >> >> Richard Genoud (4): >> dt-bindings: pwm: allwinner: add h616 pwm compatible >> pwm: sun50i: Add H616 PWM support >> arm64: dts: allwinner: h616: add PWM controller >> MAINTAINERS: Add entry on Allwinner H616 PWM driver >> >> .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 19 +- >> MAINTAINERS | 5 + >> .../arm64/boot/dts/allwinner/sun50i-h616.dtsi | 47 + >> drivers/pwm/Kconfig | 12 + >> drivers/pwm/Makefile | 1 + >> drivers/pwm/pwm-sun50i-h616.c | 936 ++++++++++++++++++ >> 6 files changed, 1019 insertions(+), 1 deletion(-) >> create mode 100644 drivers/pwm/pwm-sun50i-h616.c >> >> >> base-commit: 11439c4635edd669ae435eec308f4ab8a0804808 > Regards, Richard