Re: [PATCH 3/5] gpio: mvebu: add pwm support for Armada 8K/7K

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
To: Baruch Siach <baruch@tkos.co.il>
Cc: "Andrew Lunn" <andrew@lunn.ch>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Lee Jones" <lee.jones@linaro.org>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Jason Cooper" <jason@lakedaemon.net>,
	"Gregory Clement" <gregory.clement@bootlin.com>,
	"Sebastian Hesselbarth" <sebastian.hesselbarth@gmail.com>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	"Chris Packham" <chris.packham@alliedtelesis.co.nz>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Ralph Sennhauser" <ralph.sennhauser@gmail.com>,
	linux-pwm@vger.kernel.org,
	linux-gpio <linux-gpio@vger.kernel.org>,
	arm-soc <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 3/5] gpio: mvebu: add pwm support for Armada 8K/7K
Date: Tue, 1 Dec 2020 19:16:44 +0100	[thread overview]
Message-ID: <CAMpxmJXKDhB1fOGaY_+bmZ=4X6du0n6aoYVTMKnGXvTd8PdUNg@mail.gmail.com> (raw)
In-Reply-To: <877dqhfogp.fsf@tarshish>

On Thu, Nov 19, 2020 at 2:47 PM Baruch Siach <baruch@tkos.co.il> wrote:
>
> Hi Andrew,
>
> On Thu, Nov 19 2020, Andrew Lunn wrote:
> >> >> @@ -1200,6 +1235,13 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
> >> >>
> >> >>   devm_gpiochip_add_data(&pdev->dev, &mvchip->chip, mvchip);
> >> >>
> >> >> + /* Some MVEBU SoCs have simple PWM support for GPIO lines */
> >> >> + if (IS_ENABLED(CONFIG_PWM)) {
> >> >> +         err = mvebu_pwm_probe(pdev, mvchip, id);
> >> >> +         if (err)
> >> >> +                 return err;
> >> >> + }
> >> >> +
> >> >
> >> > The existing error handling looks odd here. Why is there no goto
> >> > err_domain when probing the PWMs fails? I wonder if this a bug from me
> >> > from a long time again?
> >>
> >> What would you release under the err_domain label? As far as I can see
> >> all resources are allocated using devres, and released automatically on
> >> failure exit.
> >
> > The IRQ domain is still registers. So once the memory is automatically
> > freed, don't we have a potential use after free?
>
> This patch moves PWM registration before IRQ domain registration for
> another reason as mentioned in the commit log. So this might
> incidentally fix the bug.
>
> Would you prefer a separate patch for that with a 'Fixes:
> 757642f9a584e8' tag?
>
> baruch
>

Baruch: does this series conflict with the fix you sent? I'm thinking
about how to take it through the next and fixes trees.

Bartosz