Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Biju Das <biju.das.jz@bp.renesas.com>
To:     Sasha Finkelstein <fnkl.kernel@gmail.com>,
        "thierry.reding@gmail.com" <thierry.reding@gmail.com>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "krzysztof.kozlowski+dt@linaro.org" 
        <krzysztof.kozlowski+dt@linaro.org>
CC:     "marcan@marcan.st" <marcan@marcan.st>,
        "sven@svenpeter.dev" <sven@svenpeter.dev>,
        "alyssa@rosenzweig.io" <alyssa@rosenzweig.io>,
        "asahi@lists.linux.dev" <asahi@lists.linux.dev>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-pwm@vger.kernel.org" <linux-pwm@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2 2/4] pwm: Add Apple PWM controller
Thread-Topic: [PATCH v2 2/4] pwm: Add Apple PWM controller
Thread-Index: AQHY7scaJddHyAtwqkGPO5YDCJjX564rwPsw
Date:   Wed, 2 Nov 2022 15:33:34 +0000
Message-ID: <OS0PR01MB59227A9E7B8A471E4D6B665786399@OS0PR01MB5922.jpnprd01.prod.outlook.com>
References: <20221102141513.49289-1-fnkl.kernel@gmail.com>
 <20221102141513.49289-3-fnkl.kernel@gmail.com>
In-Reply-To: <20221102141513.49289-3-fnkl.kernel@gmail.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?SLswX+JjwLxFalC9YbEtIf4YUYrUMHwexN8aifHClqrDNL0fh8MGeS0jqD6O?=
 =?us-ascii?Q?OVGXWyeghIMO0FwfBt1jphvrz2G6iYHTLdqfpSJOUWx1UbPGo0sw88yt4SGw?=
 =?us-ascii?Q?gKyXYYgrpavUC8GGjbNnqnmkdm9Y85plf5Pz3Kii8VOYJYn2vnwTyzzeqwgY?=
 =?us-ascii?Q?DFRkvoO8Z/LxQq0+UGXnh6LCG668UrXme+NTO0kZDRF8lmj2wtB/8PBZYCs+?=
 =?us-ascii?Q?C2KDc5cmCIeyy6wkM4YbNjXkSFPCn39HBxLPpKLrreViYHslEdwBjwh2qqaY?=
 =?us-ascii?Q?k7vieHztgMCE9aXBUfEmUIvM4Wr//7Jjj5s68KkwHwjmBlNAOPJHZSeS/ASS?=
 =?us-ascii?Q?vC0TmA6K46J/0Dv2ToivCy0zC3zca61uRSNoTSI60x4XZmAPk7JMQN7jwzzQ?=
 =?us-ascii?Q?04mNXC/wHXr3Y/lhajUf/Sm/R6DT4zkW+5J2CaP/jumc+lxUpRaNX4oLqzD0?=
 =?us-ascii?Q?y4QeYOMdIKfgM632ybvjq4B861pw3/mNDU0B0/LVY5oDfZiwhQFAeLBAdQg/?=
 =?us-ascii?Q?gC4nn+KNm4U6f1S3hSVb4xmxEjpVjI23PQQTCv6AO6V19l/lc4QcmuEdc06/?=
 =?us-ascii?Q?eNbqQz5+u+EDoLh/vH94qYpXviNuy3AQFdEAciOOFmrAzDaYWsncV51TuJCs?=
 =?us-ascii?Q?OuCgtKRPEEhXKZtjMh39MTmkyQF8aj7SZryqU63Z836J6esb0fbrLpSB+DnB?=
 =?us-ascii?Q?M7gFZbhR4FFU+oo/YkSYV4/S64g9VPtSI+wmvi8gGBf677QBtS5FG5fgoO01?=
 =?us-ascii?Q?5W7DyjnlzNTDwFXiTiKavpNOSMJ+D/EKC2F+ZM3lvP0LMKndEZQgvH8VIMN3?=
 =?us-ascii?Q?zjZpglgYfzGdoT2ylPQFAjjbiANzxaBsVz16ibWshb314KnqyhykkB9XDup0?=
 =?us-ascii?Q?Xy6ntkCvJ+vkEhfCqAKMphqcaCJA2oQhaQtPuo7JwXGx/1v+/DhrujCvn7MJ?=
 =?us-ascii?Q?H9QpybDZS7SzIp3c+vAFfZ46/i93Me+YD9VHD9aqAkMBWsAGmUqE0P94Pm5r?=
 =?us-ascii?Q?zUCTcH46bpBC+Vlvwsg9fDRM1U0FtK0zAZQYId6a9nendIz54rs+2A1mnbnL?=
 =?us-ascii?Q?T2Sxhwizty3LgdSSJLhVk6Zz0hcysSWc/griGj5MsUDvSJFHQckKoR1d7oid?=
 =?us-ascii?Q?K8X3Le4Z3innQ1PXw7yxX/xuCsOcyvuFiHR29Gsld9mCq+0wwOk1X6fR8g4U?=
 =?us-ascii?Q?LoQne4llwIjsHOHYBAAETLtVlvMQTHQygJoaTTcNQF8fDxANZW0vtUoidF80?=
 =?us-ascii?Q?+/efArCUekIGg1qzpNnqyzAKhxdKxmMohKAflCgNOiKzbeUJkVtiRMH5CmJ/?=
 =?us-ascii?Q?TyakX+4U8TMJAwy+ha7a7fNt7VGORipaEZkgqAjJQP2IcEi33aneDSEWmlmc?=
 =?us-ascii?Q?aZJjYRManNQh2hvJJ5F58x+RUtEakDPnz7Zd5wCnFfGYt/yqnMTEUcMlksT3?=
 =?us-ascii?Q?qT0/5qDm6D00sj+goCVspsnvAWcPd/sKqx1q1FwiDuCRxR5roKkEZTxCJ8Rn?=
 =?us-ascii?Q?gRzrfXzHhq80df23xEwFd1A4KaYM25ZB1bE2cvwnB+Uu9Z12TgQxg+95SGxl?=
 =?us-ascii?Q?ZWhUojfuhKsuOUx/MiZ0X/aroW9a07gcfgSorAWiR7B0UuvJc6bJWqW7moyA?=
 =?us-ascii?Q?mg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: bp.renesas.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: OS0PR01MB5922.jpnprd01.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3e690b8a-6269-4e24-bf05-08dabce79a84
X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Nov 2022 15:33:34.1736
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 53d82571-da19-47e4-9cb4-625a166a4a2a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 2A68bYuA/s5Zx9ArNuYli7k7+JT0qrEpBsV5HQVOYjoVORTDL1VWYLfSVdNevAXoG9hQIKXPAqLqBlvkueSyl+E17a0j5qMvpyuq3YLYKtw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: TY1PR01MB10850
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,
        SPF_HELO_PASS,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Sasha Finkelstein,

Thanks for the patch.

> Subject: [PATCH v2 2/4] pwm: Add Apple PWM controller
>=20
> Adds the Apple PWM controller driver.
>=20
> Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
> ---
>  drivers/pwm/Kconfig     |  12 ++++
>  drivers/pwm/Makefile    |   1 +
>  drivers/pwm/pwm-apple.c | 127
> ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 140 insertions(+)
>  create mode 100644 drivers/pwm/pwm-apple.c
>=20
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index
> 60d13a949bc5..c3be11468414 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.
>=20
> +config PWM_APPLE
> +	tristate "Apple SoC PWM support"
> +	depends on ARCH_APPLE || COMPILE_TEST
> +	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)		+=3D core.o
>  obj-$(CONFIG_PWM_SYSFS)		+=3D sysfs.o
>  obj-$(CONFIG_PWM_AB8500)	+=3D pwm-ab8500.o
> +obj-$(CONFIG_PWM_APPLE)		+=3D pwm-apple.o
>  obj-$(CONFIG_PWM_ATMEL)		+=3D pwm-atmel.o
>  obj-$(CONFIG_PWM_ATMEL_HLCDC_PWM)	+=3D pwm-atmel-hlcdc.o
>  obj-$(CONFIG_PWM_ATMEL_TCB)	+=3D pwm-atmel-tcb.o
> diff --git a/drivers/pwm/pwm-apple.c b/drivers/pwm/pwm-apple.c new
> file mode 100644 index 000000000000..7b2936346f4e
> --- /dev/null
> +++ b/drivers/pwm/pwm-apple.c
> @@ -0,0 +1,127 @@
> +// 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>
> +#include <linux/math64.h>

May be sort header alphabetically??

> +
> +#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 =3D container_of(chip, struct apple_pwm, chip);
> +	if (state->enabled) {
> +		on_cycles =3D mul_u64_u64_div_u64(fpwm->clkrate,
> +						state->duty_cycle, NSEC_PER_SEC);
> +		off_cycles =3D mul_u64_u64_div_u64(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 =3D container_of(chip, struct apple_pwm, chip);
> +
> +	ctrl =3D readl(fpwm->base + PWM_CONTROL);
> +	on_cycles =3D readl(fpwm->base + PWM_ON_CYCLES);
> +	off_cycles =3D readl(fpwm->base + PWM_OFF_CYCLES);
> +
> +	state->enabled =3D (ctrl & CTRL_ENABLE) && (ctrl &
> CTRL_OUTPUT_ENABLE);
> +	state->polarity =3D PWM_POLARITY_NORMAL;
> +	state->duty_cycle =3D div_u64(on_cycles, fpwm->clkrate) *
> NSEC_PER_SEC;
> +	state->period =3D div_u64(off_cycles + on_cycles, fpwm->clkrate) *
> +NSEC_PER_SEC; }
> +
> +static const struct pwm_ops apple_pwm_ops =3D {
> +	.apply =3D apple_pwm_apply,
> +	.get_state =3D apple_pwm_get_state,
> +	.owner =3D THIS_MODULE,
> +};
> +
> +static int apple_pwm_probe(struct platform_device *pdev) {
> +	struct apple_pwm *pwm;
> +	struct clk *clk;
> +	int ret;
> +
> +	pwm =3D devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
> +	if (!pwm)
> +		return -ENOMEM;
> +
> +	pwm->base =3D devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(pwm->base))
> +		return PTR_ERR(pwm->base);
> +
> +	platform_set_drvdata(pdev, pwm);
> +
> +	clk =3D devm_clk_get_enabled(&pdev->dev, NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);

What is your use case?=20

PWM is configured by bootloader intially??

Or

PWM is configured by Linux not by the bootloader?


Or

Driver needs to support both ??

Cheers,
Biju


> +
> +	pwm->clkrate =3D clk_get_rate(clk);
> +	pwm->chip.dev =3D &pdev->dev;
> +	pwm->chip.npwm =3D 1;
> +	pwm->chip.ops =3D &apple_pwm_ops;
> +
> +	ret =3D devm_pwmchip_add(&pdev->dev, &pwm->chip);
> +	return ret;
> +}
> +
> +static const struct of_device_id apple_pwm_of_match[] =3D {
> +	{ .compatible =3D "apple,s5l-fpwm" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, apple_pwm_of_match);
> +
> +static struct platform_driver apple_pwm_driver =3D {
> +	.probe =3D apple_pwm_probe,
> +	.driver =3D {
> +		.name =3D "apple-pwm",
> +		.owner =3D THIS_MODULE,
> +		.of_match_table =3D apple_pwm_of_match,
> +	},
> +};
> +module_platform_driver(apple_pwm_driver);
> +
> +MODULE_DESCRIPTION("Apple SoC PWM driver"); MODULE_LICENSE("Dual
> +MIT/GPL");
> --
> 2.37.3
>=20
>=20
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> https://jpn01.safelinks.protection.outlook.com/?url=3Dhttp%3A%2F%2Flists
> .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-
> kernel&amp;data=3D05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C0ff5a9275b954
> eff39ae08dabcde3c28%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C63802
> 9959918512569%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2lu
> MzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=3DzdoRENBcN
> FUDLdqC8wOspyvoOzRsTkBkS01Q8ff9okk%3D&amp;reserved=3D0