| Message ID | 20241111023412.3466161-3-ye.zhang@rock-chips.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: rockchip: Update the GPIO driver | expand |
On Mon, Nov 11, 2024 at 10:34:10AM +0800, Ye Zhang wrote: > Have a list of valid IDs and default to -ENODEV. ... > - /* If not gpio v2, that is default to v1. */ > - if (id == GPIO_TYPE_V2 || id == GPIO_TYPE_V2_1) { > + switch (id) { > + case GPIO_TYPE_V1: If you leave the V2 case first... > + bank->gpio_regs = &gpio_regs_v1; > + bank->gpio_type = GPIO_TYPE_V1; > + break; > + case GPIO_TYPE_V2: > + case GPIO_TYPE_V2_1: ...and the v1 case last, the whole diff will be much more understandable and reviewable. > bank->gpio_regs = &gpio_regs_v2; > bank->gpio_type = GPIO_TYPE_V2; > bank->db_clk = of_clk_get(bank->of_node, 1); > @@ -677,9 +682,10 @@ static int rockchip_get_bank_data(struct rockchip_pin_bank *bank) > clk_disable_unprepare(bank->clk); > return -EINVAL; > } > - } else { > - bank->gpio_regs = &gpio_regs_v1; > - bank->gpio_type = GPIO_TYPE_V1; > + break; > + default: > + dev_err(bank->dev, "cannot get the version ID\n"); > + return -ENODEV; > }
Hi, On Mon, Nov 11, 2024 at 10:34:10AM +0800, Ye Zhang wrote: > Have a list of valid IDs and default to -ENODEV. > > Signed-off-by: Ye Zhang <ye.zhang@rock-chips.com> > --- > drivers/gpio/gpio-rockchip.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c > index 71672d654491..f05b92e0e977 100644 > --- a/drivers/gpio/gpio-rockchip.c > +++ b/drivers/gpio/gpio-rockchip.c > @@ -667,8 +667,13 @@ static int rockchip_get_bank_data(struct rockchip_pin_bank *bank) > clk_prepare_enable(bank->clk); > id = readl(bank->reg_base + gpio_regs_v2.version_id); > > - /* If not gpio v2, that is default to v1. */ > - if (id == GPIO_TYPE_V2 || id == GPIO_TYPE_V2_1) { > + switch (id) { > + case GPIO_TYPE_V1: > + bank->gpio_regs = &gpio_regs_v1; > + bank->gpio_type = GPIO_TYPE_V1; > + break; > + case GPIO_TYPE_V2: > + case GPIO_TYPE_V2_1: > bank->gpio_regs = &gpio_regs_v2; > bank->gpio_type = GPIO_TYPE_V2; > bank->db_clk = of_clk_get(bank->of_node, 1); > @@ -677,9 +682,10 @@ static int rockchip_get_bank_data(struct rockchip_pin_bank *bank) > clk_disable_unprepare(bank->clk); > return -EINVAL; > } > - } else { > - bank->gpio_regs = &gpio_regs_v1; > - bank->gpio_type = GPIO_TYPE_V1; > + break; > + default: > + dev_err(bank->dev, "cannot get the version ID\n"); I think this would be a better error message: dev_err(bank->dev, "unsupported version ID: 0x%08x\n", id); But it can be improved later on. Just like the next patch I think this should go into the 6.13 kernel as soon as possible to fix handling of the Rockchip RK3576, which currently initializes its GPIO controller using the v1 register layer instead of the v2 register layout. Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > + return -ENODEV; > } > > return 0; > -- > 2.34.1 > >
diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c index 71672d654491..f05b92e0e977 100644 --- a/drivers/gpio/gpio-rockchip.c +++ b/drivers/gpio/gpio-rockchip.c @@ -667,8 +667,13 @@ static int rockchip_get_bank_data(struct rockchip_pin_bank *bank) clk_prepare_enable(bank->clk); id = readl(bank->reg_base + gpio_regs_v2.version_id); - /* If not gpio v2, that is default to v1. */ - if (id == GPIO_TYPE_V2 || id == GPIO_TYPE_V2_1) { + switch (id) { + case GPIO_TYPE_V1: + bank->gpio_regs = &gpio_regs_v1; + bank->gpio_type = GPIO_TYPE_V1; + break; + case GPIO_TYPE_V2: + case GPIO_TYPE_V2_1: bank->gpio_regs = &gpio_regs_v2; bank->gpio_type = GPIO_TYPE_V2; bank->db_clk = of_clk_get(bank->of_node, 1); @@ -677,9 +682,10 @@ static int rockchip_get_bank_data(struct rockchip_pin_bank *bank) clk_disable_unprepare(bank->clk); return -EINVAL; } - } else { - bank->gpio_regs = &gpio_regs_v1; - bank->gpio_type = GPIO_TYPE_V1; + break; + default: + dev_err(bank->dev, "cannot get the version ID\n"); + return -ENODEV; } return 0;
Have a list of valid IDs and default to -ENODEV. Signed-off-by: Ye Zhang <ye.zhang@rock-chips.com> --- drivers/gpio/gpio-rockchip.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)