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 X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7DE39C43381 for ; Wed, 10 Mar 2021 11:51:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 38A7964FF7 for ; Wed, 10 Mar 2021 11:51:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231916AbhCJLvB (ORCPT ); Wed, 10 Mar 2021 06:51:01 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53670 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231382AbhCJLux (ORCPT ); Wed, 10 Mar 2021 06:50:53 -0500 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9CE47C06174A for ; Wed, 10 Mar 2021 03:50:53 -0800 (PST) Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lJxMS-0000dy-H8; Wed, 10 Mar 2021 12:50:44 +0100 Received: from ukl by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1lJxMQ-0001h5-O7; Wed, 10 Mar 2021 12:50:42 +0100 Date: Wed, 10 Mar 2021 12:50:41 +0100 From: Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= To: Nicolas Saenz Julienne Cc: f.fainelli@gmail.com, linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org, bcm-kernel-feedback-list@broadcom.com, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, wahrenst@gmx.net, linux-input@vger.kernel.org, dmitry.torokhov@gmail.com, gregkh@linuxfoundation.org, devel@driverdev.osuosl.org, p.zabel@pengutronix.de, linux-gpio@vger.kernel.org, linus.walleij@linaro.org, linux-clk@vger.kernel.org, sboyd@kernel.org, linux-rpi-kernel@lists.infradead.org, bgolaszewski@baylibre.com, andy.shevchenko@gmail.com Subject: Re: [PATCH v7 11/11] pwm: Add Raspberry Pi Firmware based PWM bus Message-ID: <20210310115041.s7tzvgdpksws6yss@pengutronix.de> References: <20210118123244.13669-1-nsaenzjulienne@suse.de> <20210118123244.13669-12-nsaenzjulienne@suse.de> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ul6qbztvwf7nowqu" Content-Disposition: inline In-Reply-To: <20210118123244.13669-12-nsaenzjulienne@suse.de> X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-gpio@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org --ul6qbztvwf7nowqu Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hello Nicolas, On Mon, Jan 18, 2021 at 01:32:44PM +0100, Nicolas Saenz Julienne wrote: > diff --git a/drivers/pwm/pwm-raspberrypi-poe.c b/drivers/pwm/pwm-raspberr= ypi-poe.c > new file mode 100644 > index 000000000000..ca845e8fabe6 > --- /dev/null > +++ b/drivers/pwm/pwm-raspberrypi-poe.c > @@ -0,0 +1,220 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright 2020 Nicolas Saenz Julienne > + * For more information on Raspberry Pi's PoE hat see: > + * https://www.raspberrypi.org/products/poe-hat/ > + * > + * Limitations: > + * - No disable bit, so a disabled PWM is simulated by duty_cycle 0 > + * - Only normal polarity > + * - Fixed 12.5 kHz period > + * > + * The current period is completed when HW is reconfigured. nice. > + */ > + > [...] > +static int raspberrypi_pwm_apply(struct pwm_chip *chip, struct pwm_devic= e *pwm, > + const struct pwm_state *state) > +{ > + struct raspberrypi_pwm *rpipwm =3D raspberrypi_pwm_from_chip(chip); > + unsigned int duty_cycle; > + int ret; > + > + if (state->period < RPI_PWM_PERIOD_NS || > + state->polarity !=3D PWM_POLARITY_NORMAL) > + return -EINVAL; > + > + if (!state->enabled) > + duty_cycle =3D 0; > + else if (state->duty_cycle < RPI_PWM_PERIOD_NS) > + duty_cycle =3D DIV_ROUND_DOWN_ULL(state->duty_cycle * RPI_PWM_MAX_DUTY, > + RPI_PWM_PERIOD_NS); > + else > + duty_cycle =3D RPI_PWM_MAX_DUTY; > + > + if (duty_cycle =3D=3D rpipwm->duty_cycle) > + return 0; > + > + ret =3D raspberrypi_pwm_set_property(rpipwm->firmware, RPI_PWM_CUR_DUTY= _REG, > + duty_cycle); > + if (ret) { > + dev_err(chip->dev, "Failed to set duty cycle: %pe\n", > + ERR_PTR(ret)); > + return ret; > + } > + > + /* > + * This sets the default duty cycle after resetting the board, we > + * updated it every time to mimic Raspberry Pi's downstream's driver > + * behaviour. > + */ > + ret =3D raspberrypi_pwm_set_property(rpipwm->firmware, RPI_PWM_DEF_DUTY= _REG, > + duty_cycle); > + if (ret) { > + dev_err(chip->dev, "Failed to set default duty cycle: %pe\n", > + ERR_PTR(ret)); > + return ret; This only has an effect for the next reboot, right? If so I wonder if it is a good idea in general. (Think: The current PWM setting enables a motor that makes a self-driving car move at 100 km/h. Consider the rpi crashes, do I want to car to pick up driving 100 km/h at power up even before Linux is up again?) And if we agree it's a good idea: Should raspberrypi_pwm_apply return 0 if setting the duty cycle succeeded and only setting the default didn't? Other than that the patch looks fine. Best regards Uwe --=20 Pengutronix e.K. | Uwe Kleine-K=F6nig | Industrial Linux Solutions | https://www.pengutronix.de/ | --ul6qbztvwf7nowqu Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEfnIqFpAYrP8+dKQLwfwUeK3K7AkFAmBIso0ACgkQwfwUeK3K 7An3RQgAk7LiLPojRn3qgp/eEHGcY24aQQQnYHXFzwvNNsqfY9q1T6NOjwiJlIBj owBPtq8IteT+V4qhiiQuB4MLMbNeaBZ+iR4l7OVDwzwuPrHtZuGFxz7vZumxdIET Eqq1G++2nk48ZFJOUnKeWM733IgWZQwaM0XEr04i58ZjnoJ9mZo7g4nC2c8O6F6A HQshnIGC2hEIZbpmpQrBOMI92Uh0pt03ScuCXM4o/YUuKxb8oeygwt963lrJwhXp Vb1/aPD/lELD/kAAeOa3cje1nAdfEJPzH0kjIMfXNTkX8TeEEA0klNEEOXkVxrTP 0c+bK2SaxSe/2i1555OooX7x1H2VCg== =540n -----END PGP SIGNATURE----- --ul6qbztvwf7nowqu-- 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 X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0D74AC433E0 for ; Wed, 10 Mar 2021 11:50:57 +0000 (UTC) Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9988764F76 for ; Wed, 10 Mar 2021 11:50:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9988764F76 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=driverdev-devel-bounces@linuxdriverproject.org Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 59FC44314A; Wed, 10 Mar 2021 11:50:56 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 858tGgQh8fCC; Wed, 10 Mar 2021 11:50:55 +0000 (UTC) Received: from ash.osuosl.org (ash.osuosl.org [140.211.166.34]) by smtp2.osuosl.org (Postfix) with ESMTP id 2641643150; Wed, 10 Mar 2021 11:50:55 +0000 (UTC) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by ash.osuosl.org (Postfix) with ESMTP id 784731BF3C6 for ; Wed, 10 Mar 2021 11:50:53 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 73CBD6F7A7 for ; Wed, 10 Mar 2021 11:50:53 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id mMNodq3_PhFY for ; Wed, 10 Mar 2021 11:50:52 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [85.220.165.71]) by smtp3.osuosl.org (Postfix) with ESMTPS id 122096F710 for ; Wed, 10 Mar 2021 11:50:51 +0000 (UTC) Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lJxMS-0000dy-H8; Wed, 10 Mar 2021 12:50:44 +0100 Received: from ukl by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1lJxMQ-0001h5-O7; Wed, 10 Mar 2021 12:50:42 +0100 Date: Wed, 10 Mar 2021 12:50:41 +0100 From: Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= To: Nicolas Saenz Julienne Subject: Re: [PATCH v7 11/11] pwm: Add Raspberry Pi Firmware based PWM bus Message-ID: <20210310115041.s7tzvgdpksws6yss@pengutronix.de> References: <20210118123244.13669-1-nsaenzjulienne@suse.de> <20210118123244.13669-12-nsaenzjulienne@suse.de> MIME-Version: 1.0 In-Reply-To: <20210118123244.13669-12-nsaenzjulienne@suse.de> X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: devel@driverdev.osuosl.org X-BeenThere: driverdev-devel@linuxdriverproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux Driver Project Developer List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devel@driverdev.osuosl.org, linux-pwm@vger.kernel.org, f.fainelli@gmail.com, devicetree@vger.kernel.org, sboyd@kernel.org, gregkh@linuxfoundation.org, linus.walleij@linaro.org, dmitry.torokhov@gmail.com, linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, andy.shevchenko@gmail.com, bcm-kernel-feedback-list@broadcom.com, wahrenst@gmx.net, p.zabel@pengutronix.de, linux-input@vger.kernel.org, bgolaszewski@baylibre.com, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rpi-kernel@lists.infradead.org Content-Type: multipart/mixed; boundary="===============1254284706311298061==" Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" --===============1254284706311298061== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ul6qbztvwf7nowqu" Content-Disposition: inline --ul6qbztvwf7nowqu Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hello Nicolas, On Mon, Jan 18, 2021 at 01:32:44PM +0100, Nicolas Saenz Julienne wrote: > diff --git a/drivers/pwm/pwm-raspberrypi-poe.c b/drivers/pwm/pwm-raspberr= ypi-poe.c > new file mode 100644 > index 000000000000..ca845e8fabe6 > --- /dev/null > +++ b/drivers/pwm/pwm-raspberrypi-poe.c > @@ -0,0 +1,220 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright 2020 Nicolas Saenz Julienne > + * For more information on Raspberry Pi's PoE hat see: > + * https://www.raspberrypi.org/products/poe-hat/ > + * > + * Limitations: > + * - No disable bit, so a disabled PWM is simulated by duty_cycle 0 > + * - Only normal polarity > + * - Fixed 12.5 kHz period > + * > + * The current period is completed when HW is reconfigured. nice. > + */ > + > [...] > +static int raspberrypi_pwm_apply(struct pwm_chip *chip, struct pwm_devic= e *pwm, > + const struct pwm_state *state) > +{ > + struct raspberrypi_pwm *rpipwm =3D raspberrypi_pwm_from_chip(chip); > + unsigned int duty_cycle; > + int ret; > + > + if (state->period < RPI_PWM_PERIOD_NS || > + state->polarity !=3D PWM_POLARITY_NORMAL) > + return -EINVAL; > + > + if (!state->enabled) > + duty_cycle =3D 0; > + else if (state->duty_cycle < RPI_PWM_PERIOD_NS) > + duty_cycle =3D DIV_ROUND_DOWN_ULL(state->duty_cycle * RPI_PWM_MAX_DUTY, > + RPI_PWM_PERIOD_NS); > + else > + duty_cycle =3D RPI_PWM_MAX_DUTY; > + > + if (duty_cycle =3D=3D rpipwm->duty_cycle) > + return 0; > + > + ret =3D raspberrypi_pwm_set_property(rpipwm->firmware, RPI_PWM_CUR_DUTY= _REG, > + duty_cycle); > + if (ret) { > + dev_err(chip->dev, "Failed to set duty cycle: %pe\n", > + ERR_PTR(ret)); > + return ret; > + } > + > + /* > + * This sets the default duty cycle after resetting the board, we > + * updated it every time to mimic Raspberry Pi's downstream's driver > + * behaviour. > + */ > + ret =3D raspberrypi_pwm_set_property(rpipwm->firmware, RPI_PWM_DEF_DUTY= _REG, > + duty_cycle); > + if (ret) { > + dev_err(chip->dev, "Failed to set default duty cycle: %pe\n", > + ERR_PTR(ret)); > + return ret; This only has an effect for the next reboot, right? If so I wonder if it is a good idea in general. (Think: The current PWM setting enables a motor that makes a self-driving car move at 100 km/h. Consider the rpi crashes, do I want to car to pick up driving 100 km/h at power up even before Linux is up again?) And if we agree it's a good idea: Should raspberrypi_pwm_apply return 0 if setting the duty cycle succeeded and only setting the default didn't? Other than that the patch looks fine. Best regards Uwe --=20 Pengutronix e.K. | Uwe Kleine-K=F6nig | Industrial Linux Solutions | https://www.pengutronix.de/ | --ul6qbztvwf7nowqu Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEfnIqFpAYrP8+dKQLwfwUeK3K7AkFAmBIso0ACgkQwfwUeK3K 7An3RQgAk7LiLPojRn3qgp/eEHGcY24aQQQnYHXFzwvNNsqfY9q1T6NOjwiJlIBj owBPtq8IteT+V4qhiiQuB4MLMbNeaBZ+iR4l7OVDwzwuPrHtZuGFxz7vZumxdIET Eqq1G++2nk48ZFJOUnKeWM733IgWZQwaM0XEr04i58ZjnoJ9mZo7g4nC2c8O6F6A HQshnIGC2hEIZbpmpQrBOMI92Uh0pt03ScuCXM4o/YUuKxb8oeygwt963lrJwhXp Vb1/aPD/lELD/kAAeOa3cje1nAdfEJPzH0kjIMfXNTkX8TeEEA0klNEEOXkVxrTP 0c+bK2SaxSe/2i1555OooX7x1H2VCg== =540n -----END PGP SIGNATURE----- --ul6qbztvwf7nowqu-- --===============1254284706311298061== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel --===============1254284706311298061==-- 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 X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0BC67C433E0 for ; Wed, 10 Mar 2021 11:52:46 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4E3D764F76 for ; Wed, 10 Mar 2021 11:52:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4E3D764F76 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Type: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Reply-To:Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date :Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=pIdlqvmPM8IfRhkKsVHjtY58o5V+cbgX9lqxKoNW8EY=; b=fDyCjVMBPCpxEtFJHDXw2XpYZ yc8LejFXwZ7n0Eqi6dNeFIyUAI9AA42xiMSoZvqBas/zj32S6pWxitONbRjpimGbq4eqADl95uv6c YyuRtfPMdCOrXEtUcmRfnP2TpJV4uuNK92qkKcaV5Mk9dbRg9OB/rI594+gkUGU/BHbOap43r+LEj ScU5Zmvm1BrLttmg52KtyNzAEOEILKHVNtnPsI1oHuzxmkpzXUPqor3CzyWrRL6+mpxnFePMoT+LI TZ1HkvM/XIl373NZ+ovjJaWgk7hj+eK6YTqVgb4JsjsF1f62hlpR75OTKK0qKgrqktsGCq6nE9y1R t+KMVZdAw==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lJxMy-006lHv-9K; Wed, 10 Mar 2021 11:51:16 +0000 Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lJxMc-006lDh-EP for linux-arm-kernel@lists.infradead.org; Wed, 10 Mar 2021 11:51:00 +0000 Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lJxMS-0000dy-H8; Wed, 10 Mar 2021 12:50:44 +0100 Received: from ukl by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1lJxMQ-0001h5-O7; Wed, 10 Mar 2021 12:50:42 +0100 Date: Wed, 10 Mar 2021 12:50:41 +0100 From: Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= To: Nicolas Saenz Julienne Cc: f.fainelli@gmail.com, linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org, bcm-kernel-feedback-list@broadcom.com, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, wahrenst@gmx.net, linux-input@vger.kernel.org, dmitry.torokhov@gmail.com, gregkh@linuxfoundation.org, devel@driverdev.osuosl.org, p.zabel@pengutronix.de, linux-gpio@vger.kernel.org, linus.walleij@linaro.org, linux-clk@vger.kernel.org, sboyd@kernel.org, linux-rpi-kernel@lists.infradead.org, bgolaszewski@baylibre.com, andy.shevchenko@gmail.com Subject: Re: [PATCH v7 11/11] pwm: Add Raspberry Pi Firmware based PWM bus Message-ID: <20210310115041.s7tzvgdpksws6yss@pengutronix.de> References: <20210118123244.13669-1-nsaenzjulienne@suse.de> <20210118123244.13669-12-nsaenzjulienne@suse.de> MIME-Version: 1.0 In-Reply-To: <20210118123244.13669-12-nsaenzjulienne@suse.de> X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-arm-kernel@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210310_115055_546660_DF5F4650 X-CRM114-Status: GOOD ( 26.60 ) 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: , Content-Type: multipart/mixed; boundary="===============0589015465687274722==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============0589015465687274722== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ul6qbztvwf7nowqu" Content-Disposition: inline --ul6qbztvwf7nowqu Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hello Nicolas, On Mon, Jan 18, 2021 at 01:32:44PM +0100, Nicolas Saenz Julienne wrote: > diff --git a/drivers/pwm/pwm-raspberrypi-poe.c b/drivers/pwm/pwm-raspberr= ypi-poe.c > new file mode 100644 > index 000000000000..ca845e8fabe6 > --- /dev/null > +++ b/drivers/pwm/pwm-raspberrypi-poe.c > @@ -0,0 +1,220 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright 2020 Nicolas Saenz Julienne > + * For more information on Raspberry Pi's PoE hat see: > + * https://www.raspberrypi.org/products/poe-hat/ > + * > + * Limitations: > + * - No disable bit, so a disabled PWM is simulated by duty_cycle 0 > + * - Only normal polarity > + * - Fixed 12.5 kHz period > + * > + * The current period is completed when HW is reconfigured. nice. > + */ > + > [...] > +static int raspberrypi_pwm_apply(struct pwm_chip *chip, struct pwm_devic= e *pwm, > + const struct pwm_state *state) > +{ > + struct raspberrypi_pwm *rpipwm =3D raspberrypi_pwm_from_chip(chip); > + unsigned int duty_cycle; > + int ret; > + > + if (state->period < RPI_PWM_PERIOD_NS || > + state->polarity !=3D PWM_POLARITY_NORMAL) > + return -EINVAL; > + > + if (!state->enabled) > + duty_cycle =3D 0; > + else if (state->duty_cycle < RPI_PWM_PERIOD_NS) > + duty_cycle =3D DIV_ROUND_DOWN_ULL(state->duty_cycle * RPI_PWM_MAX_DUTY, > + RPI_PWM_PERIOD_NS); > + else > + duty_cycle =3D RPI_PWM_MAX_DUTY; > + > + if (duty_cycle =3D=3D rpipwm->duty_cycle) > + return 0; > + > + ret =3D raspberrypi_pwm_set_property(rpipwm->firmware, RPI_PWM_CUR_DUTY= _REG, > + duty_cycle); > + if (ret) { > + dev_err(chip->dev, "Failed to set duty cycle: %pe\n", > + ERR_PTR(ret)); > + return ret; > + } > + > + /* > + * This sets the default duty cycle after resetting the board, we > + * updated it every time to mimic Raspberry Pi's downstream's driver > + * behaviour. > + */ > + ret =3D raspberrypi_pwm_set_property(rpipwm->firmware, RPI_PWM_DEF_DUTY= _REG, > + duty_cycle); > + if (ret) { > + dev_err(chip->dev, "Failed to set default duty cycle: %pe\n", > + ERR_PTR(ret)); > + return ret; This only has an effect for the next reboot, right? If so I wonder if it is a good idea in general. (Think: The current PWM setting enables a motor that makes a self-driving car move at 100 km/h. Consider the rpi crashes, do I want to car to pick up driving 100 km/h at power up even before Linux is up again?) And if we agree it's a good idea: Should raspberrypi_pwm_apply return 0 if setting the duty cycle succeeded and only setting the default didn't? Other than that the patch looks fine. Best regards Uwe --=20 Pengutronix e.K. | Uwe Kleine-K=F6nig | Industrial Linux Solutions | https://www.pengutronix.de/ | --ul6qbztvwf7nowqu Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEfnIqFpAYrP8+dKQLwfwUeK3K7AkFAmBIso0ACgkQwfwUeK3K 7An3RQgAk7LiLPojRn3qgp/eEHGcY24aQQQnYHXFzwvNNsqfY9q1T6NOjwiJlIBj owBPtq8IteT+V4qhiiQuB4MLMbNeaBZ+iR4l7OVDwzwuPrHtZuGFxz7vZumxdIET Eqq1G++2nk48ZFJOUnKeWM733IgWZQwaM0XEr04i58ZjnoJ9mZo7g4nC2c8O6F6A HQshnIGC2hEIZbpmpQrBOMI92Uh0pt03ScuCXM4o/YUuKxb8oeygwt963lrJwhXp Vb1/aPD/lELD/kAAeOa3cje1nAdfEJPzH0kjIMfXNTkX8TeEEA0klNEEOXkVxrTP 0c+bK2SaxSe/2i1555OooX7x1H2VCg== =540n -----END PGP SIGNATURE----- --ul6qbztvwf7nowqu-- --===============0589015465687274722== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============0589015465687274722==--