On Tue, Sep 12, 2023 at 4:07 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > > Top posting to bring Saravana Kannan into this discussion. > > This looks like a big hack to me, Saravana has been working > tirelessly to make the device tree probe order "sort itself out" > and I am pretty sure this issue needs to be fixed at the DT > core level and not in a driver. We could merge all the IO domain stuff into the pinctrl node/driver, like is done for Allwinner? Maybe that would simplify things a bit? IIRC on Allwinner SoCs the PMIC pins don't have a separate power rail, or if they do they almost certainly use the default I/O rail that is always on, and so we omit it to work around the dependency cycle. ChenYu > Saravana, can you advice how we should handle this properly? > > Yours, > Linus Walleij > > On Mon, Sep 4, 2023 at 1:58 PM Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> wrote: > > > > On some Rockchip SoCs, some SoC pins are split in what are called IO > > domains. > > > > An IO domain is supplied power externally, by regulators from a PMIC for > > example. This external power supply is then used by the IO domain as > > "supply" for the IO pins if they are outputs. > > > > Each IO domain can configure which voltage the IO pins will be operating > > on (1.8V or 3.3V). > > > > There already exists an IO domain driver for Rockchip SoCs[1]. This > > driver allows to explicit the relationship between the external power > > supplies and IO domains[2]. This makes sure the regulators are enabled > > by the Linux kernel so the IO domains are supplied with power and > > correctly configured as per the supplied voltage. > > This driver is a regulator consumer and does not offer any other > > interface for device dependency. > > > > However, IO pins belonging to an IO domain need to have this IO domain > > configured correctly before they are being used otherwise they do not > > operate correctly. > > > > We currently do not have any knowledge about which pin is on which IO > > domain, so we assume that all pins are on some IO domain and defer > > probing of the pin consumers until the IO domain driver has been probed. > > Some pins however are needed to access the regulators driving an IO > > domain. Deferring probe for them as well would introduce a cyclic > > dependency. To break out of this dependency a pin group can be supplied > > a rockchip,io-domain-boot-on property. Probe won't be deferred for pin > > groups with this property. rockchip,io-domain-boot-on should be added > > to all pin groups needed to access the PMIC driving the IO domains. > > > > [1] drivers/soc/rockchip/io-domain.c > > [2] Documentation/devicetree/bindings/power/rockchip-io-domain.yaml > > > > Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> > > --- > > drivers/pinctrl/pinctrl-rockchip.c | 64 ++++++++++++++++++++++++++++++ > > drivers/pinctrl/pinctrl-rockchip.h | 3 ++ > > 2 files changed, 67 insertions(+) > > > > diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c > > index 0276b52f37168..663bd9d6840a5 100644 > > --- a/drivers/pinctrl/pinctrl-rockchip.c > > +++ b/drivers/pinctrl/pinctrl-rockchip.c > > @@ -24,6 +24,8 @@ > > #include <linux/of_address.h> > > #include <linux/of_device.h> > > #include <linux/of_irq.h> > > +#include <linux/of_platform.h> > > +#include <linux/platform_device.h> > > #include <linux/pinctrl/machine.h> > > #include <linux/pinctrl/pinconf.h> > > #include <linux/pinctrl/pinctrl.h> > > @@ -2678,6 +2680,43 @@ static int rockchip_pmx_get_groups(struct pinctrl_dev *pctldev, > > return 0; > > } > > > > +static int rockchip_pmx_check_io_domain(struct rockchip_pinctrl *info, unsigned group) > > +{ > > + struct platform_device *pdev; > > + int i; > > + > > + if (!info->io_domains) > > + return 0; > > + > > + if (info->groups[group].io_domain_skip) > > + return 0; > > + > > + for (i = 0; i < info->num_io_domains; i++) { > > + if (!info->io_domains[i]) > > + continue; > > + > > + pdev = of_find_device_by_node(info->io_domains[i]); > > + if (!pdev) { > > + dev_err(info->dev, "couldn't find IO domain device\n"); > > + return -ENODEV; > > + } > > + > > + if (!platform_get_drvdata(pdev)) { > > + dev_err(info->dev, "IO domain device is not probed yet, deferring...(%s)", > > + info->groups[group].name); > > + return -EPROBE_DEFER; > > + } > > + > > + of_node_put(info->io_domains[i]); > > + info->io_domains[i] = NULL; > > + } > > + > > + devm_kfree(info->dev, info->io_domains); > > + info->io_domains = NULL; > > + > > + return 0; > > +} > > + > > static int rockchip_pmx_set(struct pinctrl_dev *pctldev, unsigned selector, > > unsigned group) > > { > > @@ -2691,6 +2730,10 @@ static int rockchip_pmx_set(struct pinctrl_dev *pctldev, unsigned selector, > > dev_dbg(dev, "enable function %s group %s\n", > > info->functions[selector].name, info->groups[group].name); > > > > + ret = rockchip_pmx_check_io_domain(info, group); > > + if (ret) > > + return ret; > > + > > /* > > * for each pin in the pin group selected, program the corresponding > > * pin function number in the config register. > > @@ -3019,6 +3062,8 @@ static int rockchip_pinctrl_parse_groups(struct device_node *np, > > if (!size || size % 4) > > return dev_err_probe(dev, -EINVAL, "wrong pins number or pins and configs should be by 4\n"); > > > > + grp->io_domain_skip = of_property_read_bool(np, "rockchip,io-domain-boot-on"); > > + > > grp->npins = size / 4; > > > > grp->pins = devm_kcalloc(dev, grp->npins, sizeof(*grp->pins), GFP_KERNEL); > > @@ -3417,6 +3462,22 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev) > > return PTR_ERR(info->regmap_pmu); > > } > > > > + info->num_io_domains = of_property_count_u32_elems(np, "rockchip,io-domains"); > > + if (info->num_io_domains) { > > + int i; > > + > > + info->io_domains = devm_kmalloc_array(dev, info->num_io_domains, > > + sizeof(*info->io_domains), GFP_KERNEL); > > + if (!info->io_domains) > > + return -ENOMEM; > > + > > + for (i = 0; i < info->num_io_domains; i++) { > > + info->io_domains[i] = of_parse_phandle(np, "rockchip,io-domains", 0); > > + if (!info->io_domains[i]) > > + return -EINVAL; > > + } > > + } > > + > > ret = rockchip_pinctrl_register(pdev, info); > > if (ret) > > return ret; > > @@ -3439,6 +3500,9 @@ static int rockchip_pinctrl_remove(struct platform_device *pdev) > > > > of_platform_depopulate(&pdev->dev); > > > > + for (i = 0; i < info->num_io_domains; i++) > > + of_node_put(info->io_domains[i]); > > + > > for (i = 0; i < info->ctrl->nr_banks; i++) { > > bank = &info->ctrl->pin_banks[i]; > > > > diff --git a/drivers/pinctrl/pinctrl-rockchip.h b/drivers/pinctrl/pinctrl-rockchip.h > > index 4759f336941ef..d2ac79b0a7bc4 100644 > > --- a/drivers/pinctrl/pinctrl-rockchip.h > > +++ b/drivers/pinctrl/pinctrl-rockchip.h > > @@ -435,6 +435,7 @@ struct rockchip_pin_group { > > unsigned int npins; > > unsigned int *pins; > > struct rockchip_pin_config *data; > > + bool io_domain_skip; > > }; > > > > /** > > @@ -462,6 +463,8 @@ struct rockchip_pinctrl { > > unsigned int ngroups; > > struct rockchip_pmx_func *functions; > > unsigned int nfunctions; > > + struct device_node **io_domains; > > + int num_io_domains; > > }; > > > > #endif > > -- > > 2.39.2 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel