| Message ID | 20221028165215.43662-3-fnkl.kernel@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PWM and keyboard backlight driver for ARM Macs | expand |
On 28/10/2022 12:52, Sasha Finkelstein wrote: > Adds the Apple PWM controller driver. > > Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com> > --- > drivers/pwm/Kconfig | 12 ++++ > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-apple.c | 124 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 137 insertions(+) > create mode 100644 drivers/pwm/pwm-apple.c > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > index 60d13a949bc5..ec6acb368073 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -51,6 +51,18 @@ config PWM_AB8500 > To compile this driver as a module, choose M here: the module > will be called pwm-ab8500. > > +config PWM_APPLE > + tristate "Apple SoC PWM support" > + depends on ARCH_APPLE || (COMPILE_TEST && 64BIT) Why this code cannot be build on 32-bit? > + help > + Generic PWM framework driver for PWM controller present on > + Apple SoCs > + Best regards, Krzysztof
On Fri, 28 Oct 2022 at 20:54, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 28/10/2022 12:52, Sasha Finkelstein wrote: > > +config PWM_APPLE > > + tristate "Apple SoC PWM support" > > + depends on ARCH_APPLE || (COMPILE_TEST && 64BIT) > > Why this code cannot be build on 32-bit? It uses 64-bit divisions, which causes it to fail to build on 32-bit mips. It should not be a problem, since this hardware is only present on 64-bit SoCs.
On 28/10/2022 14:51, Sasha Finkelstein wrote: > On Fri, 28 Oct 2022 at 20:54, Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 28/10/2022 12:52, Sasha Finkelstein wrote: >>> +config PWM_APPLE >>> + tristate "Apple SoC PWM support" >>> + depends on ARCH_APPLE || (COMPILE_TEST && 64BIT) >> >> Why this code cannot be build on 32-bit? > It uses 64-bit divisions, which causes it to fail to build on 32-bit > mips. It should not be a > problem, since this hardware is only present on 64-bit SoCs. Does not matter, code should be portable and buildable on 32-bit. If it does not build then your code is not correct. You need to investigate the failure. Maybe using do_div would solve (or other macros from div64.h) Best regards, Krzysztof
On 29/10/2022 04.49, Krzysztof Kozlowski wrote: > On 28/10/2022 14:51, Sasha Finkelstein wrote: >> On Fri, 28 Oct 2022 at 20:54, Krzysztof Kozlowski >> <krzysztof.kozlowski@linaro.org> wrote: >>> >>> On 28/10/2022 12:52, Sasha Finkelstein wrote: >>>> +config PWM_APPLE >>>> + tristate "Apple SoC PWM support" >>>> + depends on ARCH_APPLE || (COMPILE_TEST && 64BIT) >>> >>> Why this code cannot be build on 32-bit? >> It uses 64-bit divisions, which causes it to fail to build on 32-bit >> mips. It should not be a >> problem, since this hardware is only present on 64-bit SoCs. > > Does not matter, code should be portable and buildable on 32-bit. If it > does not build then your code is not correct. This statement does not apply in general. There are plenty of drivers which cannot reasonably build for 32-bit, and make no sense because no 32-bit hardware exists that could use them. Examples include anything that accesses 64-bit registers on 64-bit SoCs the normal way, and further anything that touches CPU stuff like system registers. In *this* case, if the only issue is some 64-bit math, then yes, it should be made to build on 32-bit (especially since this is likely to also work for older 32-bit Apple SoCs). But the (COMPILE_TEST && 64BIT) pattern is definitely valid in other cases, and I've been adding it lately to shut up the kernel test bot since it makes no sense to compile test a whole pile of our drivers on 32-bit architectures - they fundamentally can't compile without adding pointless hypothetical broken fluff to the driver like split MMIO accesses (which often can't work on real hardware), and it serves no purpose. - Hector
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index 60d13a949bc5..ec6acb368073 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -51,6 +51,18 @@ config PWM_AB8500 To compile this driver as a module, choose M here: the module will be called pwm-ab8500. +config PWM_APPLE + tristate "Apple SoC PWM support" + depends on ARCH_APPLE || (COMPILE_TEST && 64BIT) + help + Generic PWM framework driver for PWM controller present on + Apple SoCs + + Say Y here if you have an ARM Apple laptop, otherwise say N + + To compile this driver as a module, choose M here: the module + will be called pwm-apple. + config PWM_ATMEL tristate "Atmel PWM support" depends on ARCH_AT91 || COMPILE_TEST diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile index 7bf1a29f02b8..19899b912e00 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -2,6 +2,7 @@ obj-$(CONFIG_PWM) += core.o obj-$(CONFIG_PWM_SYSFS) += sysfs.o obj-$(CONFIG_PWM_AB8500) += pwm-ab8500.o +obj-$(CONFIG_PWM_APPLE) += pwm-apple.o obj-$(CONFIG_PWM_ATMEL) += pwm-atmel.o obj-$(CONFIG_PWM_ATMEL_HLCDC_PWM) += pwm-atmel-hlcdc.o obj-$(CONFIG_PWM_ATMEL_TCB) += pwm-atmel-tcb.o diff --git a/drivers/pwm/pwm-apple.c b/drivers/pwm/pwm-apple.c new file mode 100644 index 000000000000..4cfa9bf435dc --- /dev/null +++ b/drivers/pwm/pwm-apple.c @@ -0,0 +1,124 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT +/* + * Driver for the Apple SoC PWM controller + * + * Copyright The Asahi Linux Contributors + */ + +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/pwm.h> +#include <linux/io.h> +#include <linux/clk.h> +#include <linux/pm_runtime.h> + +#define PWM_CONTROL 0x00 +#define PWM_ON_CYCLES 0x1c +#define PWM_OFF_CYCLES 0x18 + +#define CTRL_ENABLE BIT(0) +#define CTRL_MODE BIT(2) +#define CTRL_UPDATE BIT(5) +#define CTRL_TRIGGER BIT(9) +#define CTRL_INVERT BIT(10) +#define CTRL_OUTPUT_ENABLE BIT(14) + +struct apple_pwm { + struct pwm_chip chip; + void __iomem *base; + u64 clkrate; +}; + +static int apple_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) +{ + struct apple_pwm *fpwm; + u64 on_cycles, off_cycles; + + fpwm = container_of(chip, struct apple_pwm, chip); + if (state->enabled) { + on_cycles = fpwm->clkrate * state->duty_cycle / NSEC_PER_SEC; + off_cycles = (fpwm->clkrate * state->period / NSEC_PER_SEC) - on_cycles; + writel(on_cycles, fpwm->base + PWM_ON_CYCLES); + writel(off_cycles, fpwm->base + PWM_OFF_CYCLES); + writel(CTRL_ENABLE | CTRL_OUTPUT_ENABLE | CTRL_UPDATE, + fpwm->base + PWM_CONTROL); + } else { + writel(0, fpwm->base + PWM_CONTROL); + } + return 0; +} + +static void apple_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, + struct pwm_state *state) +{ + struct apple_pwm *fpwm; + u32 on_cycles, off_cycles, ctrl; + + fpwm = container_of(chip, struct apple_pwm, chip); + + ctrl = readl(fpwm->base + PWM_CONTROL); + on_cycles = readl(fpwm->base + PWM_ON_CYCLES); + off_cycles = readl(fpwm->base + PWM_OFF_CYCLES); + + state->enabled = (ctrl & CTRL_ENABLE) && (ctrl & CTRL_OUTPUT_ENABLE); + state->polarity = PWM_POLARITY_NORMAL; + state->duty_cycle = on_cycles / fpwm->clkrate * NSEC_PER_SEC; + state->period = (off_cycles + on_cycles) / fpwm->clkrate * NSEC_PER_SEC; +} + +static const struct pwm_ops apple_pwm_ops = { + .apply = apple_pwm_apply, + .get_state = apple_pwm_get_state, + .owner = THIS_MODULE, +}; + +static int apple_pwm_probe(struct platform_device *pdev) +{ + struct apple_pwm *pwm; + struct clk *clk; + int ret; + + pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL); + if (!pwm) + return -ENOMEM; + + pwm->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(pwm->base)) + return PTR_ERR(pwm->base); + + platform_set_drvdata(pdev, pwm); + + clk = devm_clk_get_enabled(&pdev->dev, NULL); + if (IS_ERR(clk)) + return PTR_ERR(clk); + + pwm->clkrate = clk_get_rate(clk); + pwm->chip.dev = &pdev->dev; + pwm->chip.npwm = 1; + pwm->chip.ops = &apple_pwm_ops; + + ret = devm_pwmchip_add(&pdev->dev, &pwm->chip); + return ret; +} + +static const struct of_device_id apple_pwm_of_match[] = { + { .compatible = "apple,s5l-fpwm" }, + {} +}; +MODULE_DEVICE_TABLE(of, apple_pwm_of_match); + +static struct platform_driver apple_pwm_driver = { + .probe = apple_pwm_probe, + .driver = { + .name = "apple-pwm", + .owner = THIS_MODULE, + .of_match_table = apple_pwm_of_match, + }, +}; +module_platform_driver(apple_pwm_driver); + +MODULE_DESCRIPTION("Apple SoC PWM driver"); +MODULE_LICENSE("Dual MIT/GPL");
Adds the Apple PWM controller driver. Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com> --- drivers/pwm/Kconfig | 12 ++++ drivers/pwm/Makefile | 1 + drivers/pwm/pwm-apple.c | 124 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+) create mode 100644 drivers/pwm/pwm-apple.c