| 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: <20240824184950.gzsgdawt2ujjt6ky@subgroup>
Date: Sat, 24 Aug 2024 13:49:50 -0500
From: Nishanth Menon <nm@...com>
To: Kousik Sanagavarapu <five231003@...il.com>
CC: Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Santosh Shilimkar
<ssantosh@...nel.org>,
Nathan Chancellor <nathan@...nel.org>,
Julia Lawall
<julia.lawall@...ia.fr>,
Shuah Khan <skhan@...uxfoundation.org>,
Javier
Carrasco <javier.carrasco.cruz@...il.com>,
<linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3 1/4] soc: ti: pruss: factor out memories setup
On 10:44-20240707, Kousik Sanagavarapu wrote:
> Factor out memories setup code from probe() into a new function
> pruss_of_setup_memories(). This sets the stage for introducing auto
> cleanup of the device node (done in the subsequent patch), since the
> clean up depends on the scope of the pointer and factoring out
> code into a seperate function obviously limits the scope of the various
typo s/seperate/separate - use --codespell with checkpatch to catch :)
A follow on patch has the same problem as well.
> variables used in that function.
>
> Apart from the above, this change also has the advantage of making the
> code look more neat.
>
> While at it, use dev_err_probe() instead of plain dev_err() as this new
> function is called by the probe().
>
> Signed-off-by: Kousik Sanagavarapu <five231003@...il.com>
> ---
> drivers/soc/ti/pruss.c | 111 ++++++++++++++++++++++-------------------
> 1 file changed, 61 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
> index 24a42e0b645c..a3c55a291b0b 100644
> --- a/drivers/soc/ti/pruss.c
> +++ b/drivers/soc/ti/pruss.c
> @@ -415,6 +415,63 @@ static int pruss_clk_init(struct pruss *pruss, struct device_node *cfg_node)
> return ret;
> }
>
> +static int pruss_of_setup_memories(struct device *dev, struct pruss *pruss)
> +{
> + struct device_node *np = dev_of_node(dev);
> + struct device_node *child;
> + const struct pruss_private_data *data = of_device_get_match_data(dev);
> + const char *mem_names[PRUSS_MEM_MAX] = { "dram0", "dram1", "shrdram2" };
> + int i;
> +
> + child = of_get_child_by_name(np, "memories");
> + if (!child)
> + return dev_err_probe(dev, -ENODEV,
> + "%pOF is missing its 'memories' node\n",
> + child);
> +
> + for (i = 0; i < PRUSS_MEM_MAX; i++) {
> + struct resource res;
> + int index;
> +
> + /*
> + * On AM437x one of two PRUSS units don't contain Shared RAM,
> + * skip it
> + */
> + if (data && data->has_no_sharedram && i == PRUSS_MEM_SHRD_RAM2)
> + continue;
> +
> + index = of_property_match_string(child, "reg-names",
> + mem_names[i]);
> + if (index < 0) {
> + of_node_put(child);
> + return index;
> + }
> +
> + if (of_address_to_resource(child, index, &res)) {
> + of_node_put(child);
> + return -EINVAL;
> + }
> +
> + pruss->mem_regions[i].va = devm_ioremap(dev, res.start,
> + resource_size(&res));
> + if (!pruss->mem_regions[i].va) {
> + of_node_put(child);
> + return dev_err_probe(dev, -ENOMEM,
> + "failed to parse and map memory resource %d %s\n",
> + i, mem_names[i]);
> + }
> + pruss->mem_regions[i].pa = res.start;
> + pruss->mem_regions[i].size = resource_size(&res);
> +
> + dev_dbg(dev, "memory %8s: pa %pa size 0x%zx va %pK\n",
> + mem_names[i], &pruss->mem_regions[i].pa,
> + pruss->mem_regions[i].size, pruss->mem_regions[i].va);
> + }
> + of_node_put(child);
> +
> + return 0;
> +}
> +
> static struct regmap_config regmap_conf = {
> .reg_bits = 32,
> .val_bits = 32,
> @@ -471,15 +528,8 @@ static int pruss_cfg_of_init(struct device *dev, struct pruss *pruss)
> static int pruss_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> - struct device_node *np = dev_of_node(dev);
> - struct device_node *child;
> struct pruss *pruss;
> - struct resource res;
> - int ret, i, index;
> - const struct pruss_private_data *data;
> - const char *mem_names[PRUSS_MEM_MAX] = { "dram0", "dram1", "shrdram2" };
> -
> - data = of_device_get_match_data(&pdev->dev);
> + int ret;
>
> ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> if (ret) {
> @@ -494,48 +544,9 @@ static int pruss_probe(struct platform_device *pdev)
> pruss->dev = dev;
> mutex_init(&pruss->lock);
>
> - child = of_get_child_by_name(np, "memories");
> - if (!child) {
> - dev_err(dev, "%pOF is missing its 'memories' node\n", child);
> - return -ENODEV;
> - }
> -
> - for (i = 0; i < PRUSS_MEM_MAX; i++) {
> - /*
> - * On AM437x one of two PRUSS units don't contain Shared RAM,
> - * skip it
> - */
> - if (data && data->has_no_sharedram && i == PRUSS_MEM_SHRD_RAM2)
> - continue;
> -
> - index = of_property_match_string(child, "reg-names",
> - mem_names[i]);
> - if (index < 0) {
> - of_node_put(child);
> - return index;
> - }
> -
> - if (of_address_to_resource(child, index, &res)) {
> - of_node_put(child);
> - return -EINVAL;
> - }
> -
> - pruss->mem_regions[i].va = devm_ioremap(dev, res.start,
> - resource_size(&res));
> - if (!pruss->mem_regions[i].va) {
> - dev_err(dev, "failed to parse and map memory resource %d %s\n",
> - i, mem_names[i]);
> - of_node_put(child);
> - return -ENOMEM;
> - }
> - pruss->mem_regions[i].pa = res.start;
> - pruss->mem_regions[i].size = resource_size(&res);
> -
> - dev_dbg(dev, "memory %8s: pa %pa size 0x%zx va %pK\n",
> - mem_names[i], &pruss->mem_regions[i].pa,
> - pruss->mem_regions[i].size, pruss->mem_regions[i].va);
> - }
> - of_node_put(child);
> + ret = pruss_of_setup_memories(dev, pruss);
> + if (ret < 0)
> + goto rpm_put;
Why? We have not called pm_runtime_enable at this point.
>
> platform_set_drvdata(pdev, pruss);
>
> --
> 2.45.2.561.g66ac6e4bcd
>
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
Powered by blists - more mailing lists