From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de> To: Sean Young <sean@mess.org> Cc: linux-media@vger.kernel.org, linux-pwm@vger.kernel.org, Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>, Thierry Reding <thierry.reding@gmail.com>, Florian Fainelli <florian.fainelli@broadcom.com>, Ray Jui <rjui@broadcom.com>, Scott Branden <sbranden@broadcom.com>, Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v9 5/6] pwm: bcm2835: Allow PWM driver to be used in atomic context Date: Mon, 18 Dec 2023 10:31:36 +0100 [thread overview] Message-ID: <eicw7ppqj5dubskhmeh7iwdaoixv27qw2zqaljkddt2rwosogt@6aftnwt6p5ek> (raw) In-Reply-To: <5249bb5d6c067692e4cd09573ced2df58966693b.1702890244.git.sean@mess.org> [-- Attachment #1.1: Type: text/plain, Size: 2074 bytes --] Hello Sean, On Mon, Dec 18, 2023 at 09:06:46AM +0000, Sean Young wrote: > @@ -151,16 +146,40 @@ static int bcm2835_pwm_probe(struct platform_device *pdev) > return dev_err_probe(&pdev->dev, PTR_ERR(pc->clk), > "clock not found\n"); > > + ret = clk_rate_exclusive_get(pc->clk); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, > + "fail to get exclusive rate\n"); If Maxime didn't object to my series at https://lore.kernel.org/linux-clk/cover.1702400947.git.u.kleine-koenig@pengutronix.de/, I'd ask you to not do error checking here. With the objection I hesitate to ask for that and just mention it. > + pc->rate = clk_get_rate(pc->clk); > + if (!pc->rate) { > + clk_rate_exclusive_put(pc->clk); > + return dev_err_probe(&pdev->dev, -EINVAL, > + "failed to get clock rate\n"); > + } > + > pc->chip.dev = &pdev->dev; > pc->chip.ops = &bcm2835_pwm_ops; > + pc->chip.atomic = true; > pc->chip.npwm = 2; > > platform_set_drvdata(pdev, pc); > > ret = devm_pwmchip_add(&pdev->dev, &pc->chip); > - if (ret < 0) > + if (ret < 0) { > + clk_rate_exclusive_put(pc->clk); > return dev_err_probe(&pdev->dev, ret, > "failed to add pwmchip\n"); > + } > + > + return 0; > +} > + > +static int bcm2835_pwm_remove(struct platform_device *pdev) > +{ > + struct bcm2835_pwm *pc = platform_get_drvdata(pdev); > + > + clk_rate_exclusive_put(pc->clk); The ugly thing here is that now clk_rate_exclusive_put() happens before pwmchip_remove(). Maybe register a devm cleanup which also gets rid of the two clk_rate_exclusive_put() in probe's error path? > return 0; > } > @@ -197,6 +216,7 @@ static struct platform_driver bcm2835_pwm_driver = { > .pm = pm_ptr(&bcm2835_pwm_pm_ops), > }, > .probe = bcm2835_pwm_probe, > + .remove = bcm2835_pwm_remove, Please use .remove_new Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de> To: Sean Young <sean@mess.org> Cc: linux-media@vger.kernel.org, linux-pwm@vger.kernel.org, Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>, Thierry Reding <thierry.reding@gmail.com>, Florian Fainelli <florian.fainelli@broadcom.com>, Ray Jui <rjui@broadcom.com>, Scott Branden <sbranden@broadcom.com>, Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v9 5/6] pwm: bcm2835: Allow PWM driver to be used in atomic context Date: Mon, 18 Dec 2023 10:31:36 +0100 [thread overview] Message-ID: <eicw7ppqj5dubskhmeh7iwdaoixv27qw2zqaljkddt2rwosogt@6aftnwt6p5ek> (raw) In-Reply-To: <5249bb5d6c067692e4cd09573ced2df58966693b.1702890244.git.sean@mess.org> [-- Attachment #1: Type: text/plain, Size: 2074 bytes --] Hello Sean, On Mon, Dec 18, 2023 at 09:06:46AM +0000, Sean Young wrote: > @@ -151,16 +146,40 @@ static int bcm2835_pwm_probe(struct platform_device *pdev) > return dev_err_probe(&pdev->dev, PTR_ERR(pc->clk), > "clock not found\n"); > > + ret = clk_rate_exclusive_get(pc->clk); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, > + "fail to get exclusive rate\n"); If Maxime didn't object to my series at https://lore.kernel.org/linux-clk/cover.1702400947.git.u.kleine-koenig@pengutronix.de/, I'd ask you to not do error checking here. With the objection I hesitate to ask for that and just mention it. > + pc->rate = clk_get_rate(pc->clk); > + if (!pc->rate) { > + clk_rate_exclusive_put(pc->clk); > + return dev_err_probe(&pdev->dev, -EINVAL, > + "failed to get clock rate\n"); > + } > + > pc->chip.dev = &pdev->dev; > pc->chip.ops = &bcm2835_pwm_ops; > + pc->chip.atomic = true; > pc->chip.npwm = 2; > > platform_set_drvdata(pdev, pc); > > ret = devm_pwmchip_add(&pdev->dev, &pc->chip); > - if (ret < 0) > + if (ret < 0) { > + clk_rate_exclusive_put(pc->clk); > return dev_err_probe(&pdev->dev, ret, > "failed to add pwmchip\n"); > + } > + > + return 0; > +} > + > +static int bcm2835_pwm_remove(struct platform_device *pdev) > +{ > + struct bcm2835_pwm *pc = platform_get_drvdata(pdev); > + > + clk_rate_exclusive_put(pc->clk); The ugly thing here is that now clk_rate_exclusive_put() happens before pwmchip_remove(). Maybe register a devm cleanup which also gets rid of the two clk_rate_exclusive_put() in probe's error path? > return 0; > } > @@ -197,6 +216,7 @@ static struct platform_driver bcm2835_pwm_driver = { > .pm = pm_ptr(&bcm2835_pwm_pm_ops), > }, > .probe = bcm2835_pwm_probe, > + .remove = bcm2835_pwm_remove, Please use .remove_new Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2023-12-18 9:32 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-12-18 9:06 [PATCH v9 0/6] Improve pwm-ir-tx precision Sean Young 2023-12-18 9:06 ` [PATCH v9 1/6] pwm: Rename pwm_apply_state() to pwm_apply_might_sleep() Sean Young 2023-12-18 9:06 ` Sean Young 2023-12-18 9:06 ` Sean Young 2023-12-18 9:06 ` [PATCH v9 2/6] pwm: Replace ENOTSUPP with EOPNOTSUPP Sean Young 2023-12-18 9:06 ` [PATCH v9 3/6] pwm: renesas: Remove unused include Sean Young 2023-12-18 9:06 ` [PATCH v9 4/6] pwm: Make it possible to apply PWM changes in atomic context Sean Young 2023-12-18 9:06 ` [PATCH v9 5/6] pwm: bcm2835: Allow PWM driver to be used " Sean Young 2023-12-18 9:06 ` Sean Young 2023-12-18 9:31 ` Uwe Kleine-König [this message] 2023-12-18 9:31 ` Uwe Kleine-König 2023-12-19 17:04 ` Sean Young 2023-12-19 17:04 ` Sean Young 2023-12-18 9:06 ` [PATCH v9 6/6] media: pwm-ir-tx: Trigger edges from hrtimer interrupt context Sean Young
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=eicw7ppqj5dubskhmeh7iwdaoixv27qw2zqaljkddt2rwosogt@6aftnwt6p5ek \ --to=u.kleine-koenig@pengutronix.de \ --cc=bcm-kernel-feedback-list@broadcom.com \ --cc=florian.fainelli@broadcom.com \ --cc=ivo.g.dimitrov.75@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=linux-pwm@vger.kernel.org \ --cc=linux-rpi-kernel@lists.infradead.org \ --cc=rjui@broadcom.com \ --cc=sbranden@broadcom.com \ --cc=sean@mess.org \ --cc=thierry.reding@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.