diff mbox series

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

Message ID 20220303062211.1378883-2-jay.xu@rock-chips.com (mailing list archive)
State New
Headers show
Series gpio-rochchip | expand

Commit Message

jay.xu@rock-chips.com March 3, 2022, 6:22 a.m. UTC
In some case the system may has no builtin cru module, the gpio driver
will fail to get periph clock and debounce clock.

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(-)

Comments

Heiko Stübner March 3, 2022, 11:28 a.m. UTC | #1
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;
>
jay.xu@rock-chips.com March 4, 2022, 3 a.m. UTC | #2
Hi Heiko:

--------------
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). 
A often case is when we prepare driver for a new SoC, and need to test it on FPGA or ZEBU,
they have no cru module works, that will case gpio driver fail, I need to do wip fix

Another case is when a product needs to cut kernel as mini as possible, then the cru maybe cut and move to
other place, like uboot, the gpio will fail since get clock fail.

We have keep pclk and dbg clk (if have) to always on currently, is it possible to not handle clock for gpio ?

>
>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;
>>
>
>
>
>
diff mbox series

Patch

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");
+	}
 
 	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;