| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CACRpkdYxRdToUM3JcEeNK_K87D5WDzzSLvVEbtqqdQEhz3k_Ow@mail.gmail.com>
Date: Tue, 12 Sep 2023 10:06:34 +0200
From: Linus Walleij <linus.walleij@...aro.org>
To: Sascha Hauer <s.hauer@...gutronix.de>,
Saravana Kannan <saravanak@...gle.com>
Cc: linux-rockchip@...ts.infradead.org,
Heiko Stuebner <heiko@...ech.de>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-gpio@...r.kernel.org,
kernel@...gutronix.de,
Quentin Schulz <quentin.schulz@...obroma-systems.com>,
Michael Riesch <michael.riesch@...fvision.net>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>
Subject: Re: [PATCH 1/3] pinctrl: rockchip: add support for io-domain dependency
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.
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@...gutronix.de> 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@...gutronix.de>
> ---
> 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
>
Powered by blists - more mailing lists