Re: [PATCH v3 2/2] pinctrl: samsung: support a bus clock

From: Tudor Ambarus <tudor.ambarus@linaro.org>
To: "André Draszik" <andre.draszik@linaro.org>,
	"Krzysztof Kozlowski" <krzk@kernel.org>,
	"Sylwester Nawrocki" <s.nawrocki@samsung.com>,
	"Alim Akhtar" <alim.akhtar@samsung.com>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Rob Herring" <robh@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Tomasz Figa" <tomasz.figa@gmail.com>,
	"Peter Griffin" <peter.griffin@linaro.org>
Cc: Will McVicker <willmcvicker@google.com>,
	Sam Protsenko <semen.protsenko@linaro.org>,
	kernel-team@android.com, linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 2/2] pinctrl: samsung: support a bus clock
Date: Thu, 2 May 2024 08:41:06 +0100	[thread overview]
Message-ID: <ea6f17d7-49bf-4a1e-ba3b-757e29221590@linaro.org> (raw)
In-Reply-To: <20240426-samsung-pinctrl-busclock-v3-2-adb8664b8a7e@linaro.org>

Hi, André!

On 4/26/24 14:25, André Draszik wrote:
> @@ -200,6 +235,14 @@ static int exynos_irq_request_resources(struct irq_data *irqd)
>  	shift = irqd->hwirq * bank_type->fld_width[PINCFG_TYPE_FUNC];
>  	mask = (1 << bank_type->fld_width[PINCFG_TYPE_FUNC]) - 1;
>  
> +	ret = clk_enable(bank->drvdata->pclk);
> +	if (ret) {
> +		dev_err(bank->gpio_chip.parent,
> +			"unable to enable clock for configuring pin %s-%lu\n",
> +			bank->name, irqd->hwirq);
> +		return ret;

here we return an error
> +	}
> +
>  	raw_spin_lock_irqsave(&bank->slock, flags);
>  
>  	con = readl(bank->pctl_base + reg_con);
> @@ -209,6 +252,8 @@ static int exynos_irq_request_resources(struct irq_data *irqd)
>  
>  	raw_spin_unlock_irqrestore(&bank->slock, flags);
>  
> +	clk_disable(bank->drvdata->pclk);
> +
>  	return 0;
>  }
>  
> @@ -223,6 +268,13 @@ static void exynos_irq_release_resources(struct irq_data *irqd)
>  	shift = irqd->hwirq * bank_type->fld_width[PINCFG_TYPE_FUNC];
>  	mask = (1 << bank_type->fld_width[PINCFG_TYPE_FUNC]) - 1;
>  
> +	if (clk_enable(bank->drvdata->pclk)) {
> +		dev_err(bank->gpio_chip.parent,
> +			"unable to enable clock for deconfiguring pin %s-%lu\n",
> +			bank->name, irqd->hwirq);
> +		return;

but here we just print an error. I guess that for consistency reasons it
would be good to follow up with a patch and change the return types of
these methods and return the error too when the clock enable fails.

Cheers,
ta

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel