* [PATCH 0/2] gpio-rochchip @ 2022-03-03 6:22 ` Jianqun Xu 0 siblings, 0 replies; 15+ messages in thread From: Jianqun Xu @ 2022-03-03 6:22 UTC (permalink / raw) To: heiko; +Cc: linus.walleij, linux-rockchip, linux-gpio, Jianqun Xu The first patch fixes gpio driver to support works without cru, and the second patch try to support different dt node format. Jianqun Xu (2): gpio: rockchip: make gpio work without cru module gpio: rockchip: get pinctrl node from 'gpio-ranges' property drivers/gpio/gpio-rockchip.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/2] gpio-rochchip @ 2022-03-03 6:22 ` Jianqun Xu 0 siblings, 0 replies; 15+ messages in thread From: Jianqun Xu @ 2022-03-03 6:22 UTC (permalink / raw) To: heiko; +Cc: linus.walleij, linux-rockchip, linux-gpio, Jianqun Xu The first patch fixes gpio driver to support works without cru, and the second patch try to support different dt node format. Jianqun Xu (2): gpio: rockchip: make gpio work without cru module gpio: rockchip: get pinctrl node from 'gpio-ranges' property drivers/gpio/gpio-rockchip.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) -- 2.25.1 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] gpio: rockchip: make gpio work without cru module 2022-03-03 6:22 ` Jianqun Xu @ 2022-03-03 6:22 ` Jianqun Xu -1 siblings, 0 replies; 15+ messages in thread From: Jianqun Xu @ 2022-03-03 6:22 UTC (permalink / raw) To: heiko; +Cc: linus.walleij, linux-rockchip, linux-gpio, 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. 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"); + } 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; -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 1/2] gpio: rockchip: make gpio work without cru module @ 2022-03-03 6:22 ` Jianqun Xu 0 siblings, 0 replies; 15+ messages in thread From: Jianqun Xu @ 2022-03-03 6:22 UTC (permalink / raw) To: heiko; +Cc: linus.walleij, linux-rockchip, linux-gpio, 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. 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"); + } 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; -- 2.25.1 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] gpio: rockchip: make gpio work without cru module 2022-03-03 6:22 ` Jianqun Xu @ 2022-03-03 11:28 ` Heiko Stübner -1 siblings, 0 replies; 15+ messages in thread From: Heiko Stübner @ 2022-03-03 11:28 UTC (permalink / raw) To: Jianqun Xu; +Cc: linus.walleij, linux-rockchip, linux-gpio, Jianqun Xu 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; > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] gpio: rockchip: make gpio work without cru module @ 2022-03-03 11:28 ` Heiko Stübner 0 siblings, 0 replies; 15+ messages in thread From: Heiko Stübner @ 2022-03-03 11:28 UTC (permalink / raw) To: Jianqun Xu; +Cc: linus.walleij, linux-rockchip, linux-gpio, Jianqun Xu 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; > _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Re: [PATCH 1/2] gpio: rockchip: make gpio work without cru module 2022-03-03 11:28 ` Heiko Stübner (?) @ 2022-03-04 3:00 ` jay.xu -1 siblings, 0 replies; 15+ messages in thread From: jay.xu @ 2022-03-04 3:00 UTC (permalink / raw) To: Heiko Stübner Cc: linus.walleij, open list:ARM/Rockchip SoC..., linux-gpio 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; >> > > > > _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/2] gpio: rockchip: get pinctrl node from 'gpio-ranges' property 2022-03-03 6:22 ` Jianqun Xu @ 2022-03-03 6:22 ` Jianqun Xu -1 siblings, 0 replies; 15+ messages in thread From: Jianqun Xu @ 2022-03-03 6:22 UTC (permalink / raw) To: heiko; +Cc: linus.walleij, linux-rockchip, linux-gpio, Jianqun Xu The dt nodes for rockchip soc designes as pinctrl: pinctrl { gpio { gpio-ranges = <&pinctrl xxx>; }; }; Currently, we get the pinctrl dt node from parent of gpio, this patch try to get pinctrl dt node from 'gpio-ranges' property. After this patch, the dt nodes possible to be gpio { gpio-ranges = <&pinctrl xxx>; }; pinctrl: pinctrl { }; then the gpio driver could register as platform device itself, but not populate from pinctrl driver. Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com> --- drivers/gpio/gpio-rockchip.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c index 1da0324445cc..46c54dff92db 100644 --- a/drivers/gpio/gpio-rockchip.c +++ b/drivers/gpio/gpio-rockchip.c @@ -690,6 +690,9 @@ rockchip_gpio_find_bank(struct pinctrl_dev *pctldev, int id) int i, found = 0; info = pinctrl_dev_get_drvdata(pctldev); + if (!info) + return NULL; + bank = info->ctrl->pin_banks; for (i = 0; i < info->ctrl->nr_banks; i++, bank++) { if (bank->bank_num == id) { @@ -705,15 +708,16 @@ static int rockchip_gpio_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct device_node *np = dev->of_node; - struct device_node *pctlnp = of_get_parent(np); + struct device_node *pctlnp = NULL; struct pinctrl_dev *pctldev = NULL; struct rockchip_pin_bank *bank = NULL; struct rockchip_pin_output_deferred *cfg; static int gpio; int id, ret; - if (!np || !pctlnp) - return -ENODEV; + pctlnp = of_parse_phandle(np, "gpio-ranges", 0); + if (!pctlnp) + pctlnp = of_get_parent(np); pctldev = of_pinctrl_get(pctlnp); if (!pctldev) -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] gpio: rockchip: get pinctrl node from 'gpio-ranges' property @ 2022-03-03 6:22 ` Jianqun Xu 0 siblings, 0 replies; 15+ messages in thread From: Jianqun Xu @ 2022-03-03 6:22 UTC (permalink / raw) To: heiko; +Cc: linus.walleij, linux-rockchip, linux-gpio, Jianqun Xu The dt nodes for rockchip soc designes as pinctrl: pinctrl { gpio { gpio-ranges = <&pinctrl xxx>; }; }; Currently, we get the pinctrl dt node from parent of gpio, this patch try to get pinctrl dt node from 'gpio-ranges' property. After this patch, the dt nodes possible to be gpio { gpio-ranges = <&pinctrl xxx>; }; pinctrl: pinctrl { }; then the gpio driver could register as platform device itself, but not populate from pinctrl driver. Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com> --- drivers/gpio/gpio-rockchip.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c index 1da0324445cc..46c54dff92db 100644 --- a/drivers/gpio/gpio-rockchip.c +++ b/drivers/gpio/gpio-rockchip.c @@ -690,6 +690,9 @@ rockchip_gpio_find_bank(struct pinctrl_dev *pctldev, int id) int i, found = 0; info = pinctrl_dev_get_drvdata(pctldev); + if (!info) + return NULL; + bank = info->ctrl->pin_banks; for (i = 0; i < info->ctrl->nr_banks; i++, bank++) { if (bank->bank_num == id) { @@ -705,15 +708,16 @@ static int rockchip_gpio_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct device_node *np = dev->of_node; - struct device_node *pctlnp = of_get_parent(np); + struct device_node *pctlnp = NULL; struct pinctrl_dev *pctldev = NULL; struct rockchip_pin_bank *bank = NULL; struct rockchip_pin_output_deferred *cfg; static int gpio; int id, ret; - if (!np || !pctlnp) - return -ENODEV; + pctlnp = of_parse_phandle(np, "gpio-ranges", 0); + if (!pctlnp) + pctlnp = of_get_parent(np); pctldev = of_pinctrl_get(pctlnp); if (!pctldev) -- 2.25.1 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] gpio: rockchip: get pinctrl node from 'gpio-ranges' property 2022-03-03 6:22 ` Jianqun Xu @ 2022-03-03 11:40 ` Heiko Stübner -1 siblings, 0 replies; 15+ messages in thread From: Heiko Stübner @ 2022-03-03 11:40 UTC (permalink / raw) To: Jianqun Xu; +Cc: linus.walleij, linux-rockchip, linux-gpio, Jianqun Xu Hi Jianqun, Am Donnerstag, 3. März 2022, 07:22:11 CET schrieb Jianqun Xu: > The dt nodes for rockchip soc designes as > > pinctrl: pinctrl { > gpio { > gpio-ranges = <&pinctrl xxx>; > }; > }; > > Currently, we get the pinctrl dt node from parent of gpio, this patch > try to get pinctrl dt node from 'gpio-ranges' property. > > After this patch, the dt nodes possible to be > > gpio { > gpio-ranges = <&pinctrl xxx>; > }; > > pinctrl: pinctrl { > > }; > > then the gpio driver could register as platform device itself, but not > populate from pinctrl driver. The change looks interesting, as it would solve that long-standing design-issue I "created" back in 2013 ;-) . Though you need to keep some things in mind: (1) Such a change should be reflected in the devicetree binding as it involves a different form of nodes and introduces. Looking at the binding description, using gpio-ranges to map to specific pinctrl pins really seems to be a valid use for this. (2) Keep things backwards compatible. Old devicetrees should stay working with new kernel versions A common pattern is to try the new approach and if that fails try the "deprecated" method, which should be nicely doable when looking at the code change below. Heiko > Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com> > --- > drivers/gpio/gpio-rockchip.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c > index 1da0324445cc..46c54dff92db 100644 > --- a/drivers/gpio/gpio-rockchip.c > +++ b/drivers/gpio/gpio-rockchip.c > @@ -690,6 +690,9 @@ rockchip_gpio_find_bank(struct pinctrl_dev *pctldev, int id) > int i, found = 0; > > info = pinctrl_dev_get_drvdata(pctldev); > + if (!info) > + return NULL; > + > bank = info->ctrl->pin_banks; > for (i = 0; i < info->ctrl->nr_banks; i++, bank++) { > if (bank->bank_num == id) { > @@ -705,15 +708,16 @@ static int rockchip_gpio_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct device_node *np = dev->of_node; > - struct device_node *pctlnp = of_get_parent(np); > + struct device_node *pctlnp = NULL; > struct pinctrl_dev *pctldev = NULL; > struct rockchip_pin_bank *bank = NULL; > struct rockchip_pin_output_deferred *cfg; > static int gpio; > int id, ret; > > - if (!np || !pctlnp) > - return -ENODEV; > + pctlnp = of_parse_phandle(np, "gpio-ranges", 0); > + if (!pctlnp) > + pctlnp = of_get_parent(np); > > pctldev = of_pinctrl_get(pctlnp); > if (!pctldev) > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] gpio: rockchip: get pinctrl node from 'gpio-ranges' property @ 2022-03-03 11:40 ` Heiko Stübner 0 siblings, 0 replies; 15+ messages in thread From: Heiko Stübner @ 2022-03-03 11:40 UTC (permalink / raw) To: Jianqun Xu; +Cc: linus.walleij, linux-rockchip, linux-gpio, Jianqun Xu Hi Jianqun, Am Donnerstag, 3. März 2022, 07:22:11 CET schrieb Jianqun Xu: > The dt nodes for rockchip soc designes as > > pinctrl: pinctrl { > gpio { > gpio-ranges = <&pinctrl xxx>; > }; > }; > > Currently, we get the pinctrl dt node from parent of gpio, this patch > try to get pinctrl dt node from 'gpio-ranges' property. > > After this patch, the dt nodes possible to be > > gpio { > gpio-ranges = <&pinctrl xxx>; > }; > > pinctrl: pinctrl { > > }; > > then the gpio driver could register as platform device itself, but not > populate from pinctrl driver. The change looks interesting, as it would solve that long-standing design-issue I "created" back in 2013 ;-) . Though you need to keep some things in mind: (1) Such a change should be reflected in the devicetree binding as it involves a different form of nodes and introduces. Looking at the binding description, using gpio-ranges to map to specific pinctrl pins really seems to be a valid use for this. (2) Keep things backwards compatible. Old devicetrees should stay working with new kernel versions A common pattern is to try the new approach and if that fails try the "deprecated" method, which should be nicely doable when looking at the code change below. Heiko > Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com> > --- > drivers/gpio/gpio-rockchip.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c > index 1da0324445cc..46c54dff92db 100644 > --- a/drivers/gpio/gpio-rockchip.c > +++ b/drivers/gpio/gpio-rockchip.c > @@ -690,6 +690,9 @@ rockchip_gpio_find_bank(struct pinctrl_dev *pctldev, int id) > int i, found = 0; > > info = pinctrl_dev_get_drvdata(pctldev); > + if (!info) > + return NULL; > + > bank = info->ctrl->pin_banks; > for (i = 0; i < info->ctrl->nr_banks; i++, bank++) { > if (bank->bank_num == id) { > @@ -705,15 +708,16 @@ static int rockchip_gpio_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct device_node *np = dev->of_node; > - struct device_node *pctlnp = of_get_parent(np); > + struct device_node *pctlnp = NULL; > struct pinctrl_dev *pctldev = NULL; > struct rockchip_pin_bank *bank = NULL; > struct rockchip_pin_output_deferred *cfg; > static int gpio; > int id, ret; > > - if (!np || !pctlnp) > - return -ENODEV; > + pctlnp = of_parse_phandle(np, "gpio-ranges", 0); > + if (!pctlnp) > + pctlnp = of_get_parent(np); > > pctldev = of_pinctrl_get(pctlnp); > if (!pctldev) > _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Re: [PATCH 2/2] gpio: rockchip: get pinctrl node from 'gpio-ranges' property 2022-03-03 11:40 ` Heiko Stübner (?) @ 2022-03-04 3:02 ` jay.xu -1 siblings, 0 replies; 15+ messages in thread From: jay.xu @ 2022-03-04 3:02 UTC (permalink / raw) To: Heiko Stübner Cc: linus.walleij, open list:ARM/Rockchip SoC..., linux-gpio Hi Heiko: -------------- jay.xu@rock-chips.com >Hi Jianqun, > >Am Donnerstag, 3. März 2022, 07:22:11 CET schrieb Jianqun Xu: >> The dt nodes for rockchip soc designes as >> >> pinctrl: pinctrl { >> gpio { >> gpio-ranges = <&pinctrl xxx>; >> }; >> }; >> >> Currently, we get the pinctrl dt node from parent of gpio, this patch >> try to get pinctrl dt node from 'gpio-ranges' property. >> >> After this patch, the dt nodes possible to be >> >> gpio { >> gpio-ranges = <&pinctrl xxx>; >> }; >> >> pinctrl: pinctrl { >> >> }; >> >> then the gpio driver could register as platform device itself, but not >> populate from pinctrl driver. > >The change looks interesting, as it would solve that long-standing >design-issue I "created" back in 2013 ;-) . > >Though you need to keep some things in mind: > >(1) Such a change should be reflected in the devicetree binding > as it involves a different form of nodes and introduces. > > Looking at the binding description, using gpio-ranges to map > to specific pinctrl pins really seems to be a valid use for this. > thanks, i will add the fix in next patchset by adding another patch. > >(2) Keep things backwards compatible. > Old devicetrees should stay working with new kernel versions > > A common pattern is to try the new approach and if that fails > try the "deprecated" method, which should be nicely doable > when looking at the code change below. > okay, i will test this case. > >Heiko > >> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com> >> --- >> drivers/gpio/gpio-rockchip.c | 10 +++++++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c >> index 1da0324445cc..46c54dff92db 100644 >> --- a/drivers/gpio/gpio-rockchip.c >> +++ b/drivers/gpio/gpio-rockchip.c >> @@ -690,6 +690,9 @@ rockchip_gpio_find_bank(struct pinctrl_dev *pctldev, int id) >> int i, found = 0; >> >> info = pinctrl_dev_get_drvdata(pctldev); >> + if (!info) >> + return NULL; >> + >> bank = info->ctrl->pin_banks; >> for (i = 0; i < info->ctrl->nr_banks; i++, bank++) { >> if (bank->bank_num == id) { >> @@ -705,15 +708,16 @@ static int rockchip_gpio_probe(struct platform_device *pdev) >> { >> struct device *dev = &pdev->dev; >> struct device_node *np = dev->of_node; >> - struct device_node *pctlnp = of_get_parent(np); >> + struct device_node *pctlnp = NULL; >> struct pinctrl_dev *pctldev = NULL; >> struct rockchip_pin_bank *bank = NULL; >> struct rockchip_pin_output_deferred *cfg; >> static int gpio; >> int id, ret; >> >> - if (!np || !pctlnp) >> - return -ENODEV; >> + pctlnp = of_parse_phandle(np, "gpio-ranges", 0); >> + if (!pctlnp) >> + pctlnp = of_get_parent(np); >> >> pctldev = of_pinctrl_get(pctlnp); >> if (!pctldev) >> > > > > _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Re: [PATCH 2/2] gpio: rockchip: get pinctrl node from 'gpio-ranges' property 2022-03-03 11:40 ` Heiko Stübner (?) (?) @ 2022-03-07 8:08 ` jay.xu -1 siblings, 0 replies; 15+ messages in thread From: jay.xu @ 2022-03-07 8:08 UTC (permalink / raw) To: Heiko Stübner Cc: linus.walleij, open list:ARM/Rockchip SoC..., linux-gpio -------------- jay.xu@rock-chips.com >Hi Jianqun, > >Am Donnerstag, 3. März 2022, 07:22:11 CET schrieb Jianqun Xu: >> The dt nodes for rockchip soc designes as >> >> pinctrl: pinctrl { >> gpio { >> gpio-ranges = <&pinctrl xxx>; >> }; >> }; >> >> Currently, we get the pinctrl dt node from parent of gpio, this patch >> try to get pinctrl dt node from 'gpio-ranges' property. >> >> After this patch, the dt nodes possible to be >> >> gpio { >> gpio-ranges = <&pinctrl xxx>; >> }; >> >> pinctrl: pinctrl { >> >> }; >> >> then the gpio driver could register as platform device itself, but not >> populate from pinctrl driver. > >The change looks interesting, as it would solve that long-standing >design-issue I "created" back in 2013 ;-) . > >Though you need to keep some things in mind: > >(1) Such a change should be reflected in the devicetree binding > as it involves a different form of nodes and introduces. > > Looking at the binding description, using gpio-ranges to map > to specific pinctrl pins really seems to be a valid use for this. > > >(2) Keep things backwards compatible. > Old devicetrees should stay working with new kernel versions > > A common pattern is to try the new approach and if that fails > try the "deprecated" method, which should be nicely doable > when looking at the code change below. > Hi heiko could i get pinctrl device from name ? like this: pctldev = get_pinctrl_dev_from_devname(ROCKCHIP_PINCTRL_DEV_NAME); the ROCKCHIP_PINCTRL_DEV_NAME is a const string from pinctrl head file. > >Heiko > >> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com> >> --- >> drivers/gpio/gpio-rockchip.c | 10 +++++++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c >> index 1da0324445cc..46c54dff92db 100644 >> --- a/drivers/gpio/gpio-rockchip.c >> +++ b/drivers/gpio/gpio-rockchip.c >> @@ -690,6 +690,9 @@ rockchip_gpio_find_bank(struct pinctrl_dev *pctldev, int id) >> int i, found = 0; >> >> info = pinctrl_dev_get_drvdata(pctldev); >> + if (!info) >> + return NULL; >> + >> bank = info->ctrl->pin_banks; >> for (i = 0; i < info->ctrl->nr_banks; i++, bank++) { >> if (bank->bank_num == id) { >> @@ -705,15 +708,16 @@ static int rockchip_gpio_probe(struct platform_device *pdev) >> { >> struct device *dev = &pdev->dev; >> struct device_node *np = dev->of_node; >> - struct device_node *pctlnp = of_get_parent(np); >> + struct device_node *pctlnp = NULL; >> struct pinctrl_dev *pctldev = NULL; >> struct rockchip_pin_bank *bank = NULL; >> struct rockchip_pin_output_deferred *cfg; >> static int gpio; >> int id, ret; >> >> - if (!np || !pctlnp) >> - return -ENODEV; >> + pctlnp = of_parse_phandle(np, "gpio-ranges", 0); >> + if (!pctlnp) >> + pctlnp = of_get_parent(np); >> >> pctldev = of_pinctrl_get(pctlnp); >> if (!pctldev) >> > > > > _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] gpio: rockchip: get pinctrl node from 'gpio-ranges' property 2022-03-03 6:22 ` Jianqun Xu @ 2022-03-15 1:15 ` Linus Walleij -1 siblings, 0 replies; 15+ messages in thread From: Linus Walleij @ 2022-03-15 1:15 UTC (permalink / raw) To: Jianqun Xu; +Cc: heiko, linux-rockchip, linux-gpio On Thu, Mar 3, 2022 at 7:22 AM Jianqun Xu <jay.xu@rock-chips.com> wrote: > The dt nodes for rockchip soc designes as > > pinctrl: pinctrl { > gpio { > gpio-ranges = <&pinctrl xxx>; > }; > }; > > Currently, we get the pinctrl dt node from parent of gpio, this patch > try to get pinctrl dt node from 'gpio-ranges' property. > > After this patch, the dt nodes possible to be > > gpio { > gpio-ranges = <&pinctrl xxx>; > }; > > pinctrl: pinctrl { > > }; > > then the gpio driver could register as platform device itself, but not > populate from pinctrl driver. > > Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com> (...) > - if (!np || !pctlnp) > - return -ENODEV; > + pctlnp = of_parse_phandle(np, "gpio-ranges", 0); > + if (!pctlnp) > + pctlnp = of_get_parent(np); GPIO ranges can be several and point to several different pin controllers. I understand it does not in your case, so I guess it is fine if Heiko is fine with it. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] gpio: rockchip: get pinctrl node from 'gpio-ranges' property @ 2022-03-15 1:15 ` Linus Walleij 0 siblings, 0 replies; 15+ messages in thread From: Linus Walleij @ 2022-03-15 1:15 UTC (permalink / raw) To: Jianqun Xu; +Cc: heiko, linux-rockchip, linux-gpio On Thu, Mar 3, 2022 at 7:22 AM Jianqun Xu <jay.xu@rock-chips.com> wrote: > The dt nodes for rockchip soc designes as > > pinctrl: pinctrl { > gpio { > gpio-ranges = <&pinctrl xxx>; > }; > }; > > Currently, we get the pinctrl dt node from parent of gpio, this patch > try to get pinctrl dt node from 'gpio-ranges' property. > > After this patch, the dt nodes possible to be > > gpio { > gpio-ranges = <&pinctrl xxx>; > }; > > pinctrl: pinctrl { > > }; > > then the gpio driver could register as platform device itself, but not > populate from pinctrl driver. > > Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com> (...) > - if (!np || !pctlnp) > - return -ENODEV; > + pctlnp = of_parse_phandle(np, "gpio-ranges", 0); > + if (!pctlnp) > + pctlnp = of_get_parent(np); GPIO ranges can be several and point to several different pin controllers. I understand it does not in your case, so I guess it is fine if Heiko is fine with it. Yours, Linus Walleij _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-03-15 1:15 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-03-03 6:22 [PATCH 0/2] gpio-rochchip Jianqun Xu 2022-03-03 6:22 ` Jianqun Xu 2022-03-03 6:22 ` [PATCH 1/2] gpio: rockchip: make gpio work without cru module Jianqun Xu 2022-03-03 6:22 ` Jianqun Xu 2022-03-03 11:28 ` Heiko Stübner 2022-03-03 11:28 ` Heiko Stübner 2022-03-04 3:00 ` jay.xu 2022-03-03 6:22 ` [PATCH 2/2] gpio: rockchip: get pinctrl node from 'gpio-ranges' property Jianqun Xu 2022-03-03 6:22 ` Jianqun Xu 2022-03-03 11:40 ` Heiko Stübner 2022-03-03 11:40 ` Heiko Stübner 2022-03-04 3:02 ` jay.xu 2022-03-07 8:08 ` jay.xu 2022-03-15 1:15 ` Linus Walleij 2022-03-15 1:15 ` Linus Walleij
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.