2022-08-24 09:45:17

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v10 0/4] Microchip soft ip corePWM driver

Hey Uwe, all,

6.0-rc1 has rolled around so here is the ~promised v8~~v9~ v10.
The pre 6.0-rc1 cover letter/series is here:
https://lore.kernel.org/linux-pwm/[email protected]
I'll take the dts change myself once the rest is merged.

There is one change here that is not directly from your feedback, I
added a test for invalid PERIOD_STEPS values, in which case we abort if
the period is locked and cannot be fixed. Hopefully the rounding is not
ruined..

Thanks,
Conor.

Changes since v9:
- added the missing unlock that Dan reported

Changes since v8:
- fixed a(nother) raw 64 bit division (& built it for riscv32!)
- added a check to make sure we don't try to sleep for 0 us

Changes since v7:
- rebased on 6.0-rc1
- reworded comments you highlighted in v7
- fixed the overkill sleeping
- removed the unused variables in calc_duty
- added some extra comments to explain behaviours you questioned in v7
- make the mutexes un-interruptible
- fixed added the 1s you suggested for the if(period_locked) logic
- added setup of the channel_enabled shadowing
- fixed the period reporting for the negedge == posedge case in
get_state() I had to add the enabled check, as otherwise it broke
setting the period for the first time out of reset.
- added a test for invalid PERIOD_STEPS values, in which case we abort
if we cannot fix the period

Changes from v6:
- Dropped an unused variable that I'd missed
- Actually check the return values of the mutex lock()s
- Re-rebased on -next for the MAINTAINERS patch (again...)

Changes from v5:
- switched to a mutex b/c we must sleep with the lock taken
- simplified the locking in apply() and added locking to get_state()
- reworked apply() as requested
- removed the loop in the period calculation (thanks Uwe!)
- add a copy of the enable registers in the driver to save on reads.
- remove the second (useless) write to sync_update
- added some missing rounding in get_state()
- couple other minor cleanups as requested in:
https://lore.kernel.org/linux-riscv/[email protected]/

Changes from v4:
- dropped some accidentally added files

Conor Dooley (4):
dt-bindings: pwm: fix microchip corePWM's pwm-cells
riscv: dts: fix the icicle's #pwm-cells
pwm: add microchip soft ip corePWM driver
MAINTAINERS: add pwm to PolarFire SoC entry

.../bindings/pwm/microchip,corepwm.yaml | 4 +-
MAINTAINERS | 1 +
.../dts/microchip/mpfs-icicle-kit-fabric.dtsi | 2 +-
drivers/pwm/Kconfig | 10 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-microchip-core.c | 402 ++++++++++++++++++
6 files changed, 418 insertions(+), 2 deletions(-)
create mode 100644 drivers/pwm/pwm-microchip-core.c


base-commit: 568035b01cfb107af8d2e4bd2fb9aea22cf5b868
--
2.36.1


2022-08-24 09:47:24

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v10 2/4] riscv: dts: fix the icicle's #pwm-cells

\#pwm-cells for the Icicle kit's fabric PWM was incorrectly set to 2 &
blindly overridden by the (out of tree) driver anyway. The core can
support inverted operation, so update the entry to correctly report its
capabilities.

Fixes: 72560c6559b8 ("riscv: dts: microchip: add fpga fabric section to icicle kit")
Signed-off-by: Conor Dooley <[email protected]>
---
arch/riscv/boot/dts/microchip/mpfs-icicle-kit-fabric.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/boot/dts/microchip/mpfs-icicle-kit-fabric.dtsi b/arch/riscv/boot/dts/microchip/mpfs-icicle-kit-fabric.dtsi
index 0d28858b83f2..e09a13aef268 100644
--- a/arch/riscv/boot/dts/microchip/mpfs-icicle-kit-fabric.dtsi
+++ b/arch/riscv/boot/dts/microchip/mpfs-icicle-kit-fabric.dtsi
@@ -8,7 +8,7 @@ core_pwm0: pwm@41000000 {
compatible = "microchip,corepwm-rtl-v4";
reg = <0x0 0x41000000 0x0 0xF0>;
microchip,sync-update-mask = /bits/ 32 <0>;
- #pwm-cells = <2>;
+ #pwm-cells = <3>;
clocks = <&fabric_clk3>;
status = "disabled";
};
--
2.36.1

2022-09-14 20:13:05

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH v10 2/4] riscv: dts: fix the icicle's #pwm-cells

Hello,

On Wed, Aug 24, 2022 at 10:12:13AM +0100, Conor Dooley wrote:
> \#pwm-cells for the Icicle kit's fabric PWM was incorrectly set to 2 &
> blindly overridden by the (out of tree) driver anyway. The core can
> support inverted operation, so update the entry to correctly report its
> capabilities.
>
> Fixes: 72560c6559b8 ("riscv: dts: microchip: add fpga fabric section to icicle kit")
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> arch/riscv/boot/dts/microchip/mpfs-icicle-kit-fabric.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/boot/dts/microchip/mpfs-icicle-kit-fabric.dtsi b/arch/riscv/boot/dts/microchip/mpfs-icicle-kit-fabric.dtsi
> index 0d28858b83f2..e09a13aef268 100644
> --- a/arch/riscv/boot/dts/microchip/mpfs-icicle-kit-fabric.dtsi
> +++ b/arch/riscv/boot/dts/microchip/mpfs-icicle-kit-fabric.dtsi
> @@ -8,7 +8,7 @@ core_pwm0: pwm@41000000 {
> compatible = "microchip,corepwm-rtl-v4";
> reg = <0x0 0x41000000 0x0 0xF0>;
> microchip,sync-update-mask = /bits/ 32 <0>;
> - #pwm-cells = <2>;
> + #pwm-cells = <3>;
> clocks = <&fabric_clk3>;
> status = "disabled";
> };

there are no phandles on that PWM, so there is nothing that needs a
followup adaption.

Reviewed-by: Uwe Kleine-K?nig <[email protected]>

Best regards
Uwe

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


Attachments:
(No filename) (1.50 kB)
signature.asc (499.00 B)
Download all attachments

2022-09-15 07:37:14

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v10 2/4] riscv: dts: fix the icicle's #pwm-cells

On 14/09/2022 20:59, Uwe Kleine-König wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> there are no phandles on that PWM, so there is nothing that needs a
> followup adaption.

> Reviewed-by: Uwe Kleine-König <[email protected]>

Hey Uwe,
Thanks for checking the pwm consumers :)

I assume you going to take the corresponding binding change via
either pwm-fixes or pwm-for-next, I think you can take the dts too, I
don't think the dependency I previously thought existed exists.
For that purpose:
Acked-by: Conor Dooley <[email protected]>

Thanks,
Conor.