Re: [PATCH 1/2] gpio: rockchip: make gpio work without cru module

From: "Heiko Stübner" <heiko@sntech.de>
To: Jianqun Xu <jay.xu@rock-chips.com>
Cc: linus.walleij@linaro.org, linux-rockchip@lists.infradead.org,
	linux-gpio@vger.kernel.org, Jianqun Xu <jay.xu@rock-chips.com>
Subject: Re: [PATCH 1/2] gpio: rockchip: make gpio work without cru module
Date: Thu, 03 Mar 2022 12:28:10 +0100	[thread overview]
Message-ID: <8576445.5iEe77X0Xf@diego> (raw)
In-Reply-To: <20220303062211.1378883-2-jay.xu@rock-chips.com>

Hi Jianqun,

Am Donnerstag, 3. März 2022, 07:22:10 CET schrieb Jianqun Xu:
> In some case the system may has no builtin cru module, the gpio driver
> will fail to get periph clock and debounce clock.

can you elaborate a bit on what these cases are?

> On rockchip SoCs, the pclk and dbg clk are default to be enabled and
> ungated, the gpio possible to work without cru module.
> 
> This patch makes gpio work fine without cru module.
> 
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> ---
>  drivers/gpio/gpio-rockchip.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
> index a4c4e4584f5b..1da0324445cc 100644
> --- a/drivers/gpio/gpio-rockchip.c
> +++ b/drivers/gpio/gpio-rockchip.c
> @@ -195,6 +195,9 @@ static int rockchip_gpio_set_debounce(struct gpio_chip *gc,
>  	unsigned int cur_div_reg;
>  	u64 div;
>  
> +	if (!bank->db_clk)
> +		return -ENOENT;
> +
>  	if (bank->gpio_type == GPIO_TYPE_V2 && !IS_ERR(bank->db_clk)) {
>  		div_debounce_support = true;
>  		freq = clk_get_rate(bank->db_clk);
> @@ -654,8 +657,10 @@ static int rockchip_get_bank_data(struct rockchip_pin_bank *bank)
>  		return -EINVAL;
>  
>  	bank->clk = of_clk_get(bank->of_node, 0);
> -	if (IS_ERR(bank->clk))
> -		return PTR_ERR(bank->clk);
> +	if (IS_ERR(bank->clk)) {
> +		bank->clk = NULL;
> +		dev_warn(bank->dev, "works without clk pm\n");

I'd definitly expect a more sensitive handling here (and below).

I.e. the change right now, simply disables all error handling.
But I do expect a handling difference between:
- clock described in DT, but not available - should fail
- clock not described in DT - can be allowed to go to NULL

Heiko

> +	}
>  
>  	clk_prepare_enable(bank->clk);
>  	id = readl(bank->reg_base + gpio_regs_v2.version_id);
> @@ -666,9 +671,8 @@ static int rockchip_get_bank_data(struct rockchip_pin_bank *bank)
>  		bank->gpio_type = GPIO_TYPE_V2;
>  		bank->db_clk = of_clk_get(bank->of_node, 1);
>  		if (IS_ERR(bank->db_clk)) {
> -			dev_err(bank->dev, "cannot find debounce clk\n");
> -			clk_disable_unprepare(bank->clk);
> -			return -EINVAL;
> +			bank->db_clk = NULL;
> +			dev_warn(bank->dev, "works without debounce clk pm\n");
>  		}
>  	} else {
>  		bank->gpio_regs = &gpio_regs_v1;
> 