| 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: <6246542f-6059-4bf8-91f7-6de713707711@wanadoo.fr>
Date: Sun, 11 Aug 2024 08:00:05 +0200
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Inochi Amaoto <inochiama@...look.com>, Vinod Koul <vkoul@...nel.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Chen Wang <unicorn_wang@...look.com>,
Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
<palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>
Cc: dmaengine@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v11 3/3] dmaengine: add driver for Sophgo CV18XX/SG200X
dmamux
Le 11/08/2024 à 07:16, Inochi Amaoto a écrit :
> Sophgo CV18XX/SG200X use DW AXI CORE with a multiplexer for remapping
> its request lines. The multiplexer supports at most 8 request lines.
>
> Add driver for Sophgo CV18XX/SG200X DMA multiplexer.
>
> Signed-off-by: Inochi Amaoto <inochiama@...look.com>
> ---
Hi,
...
> +static void *cv1800_dmamux_route_allocate(struct of_phandle_args *dma_spec,
> + struct of_dma *ofdma)
> +{
> + struct platform_device *pdev = of_find_device_by_node(ofdma->of_node);
> + struct cv1800_dmamux_data *dmamux = platform_get_drvdata(pdev);
> + struct cv1800_dmamux_map *map;
> + struct llist_node *node;
> + unsigned long flags;
> + unsigned int chid, devid, cpuid;
> + int ret;
> +
> + if (dma_spec->args_count != DMAMUX_NCELLS) {
> + dev_err(&pdev->dev, "invalid number of dma mux args\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + devid = dma_spec->args[0];
> + cpuid = dma_spec->args[1];
> + dma_spec->args_count = 1;
> +
> + if (devid > MAX_DMA_MAPPING_ID) {
> + dev_err(&pdev->dev, "invalid device id: %u\n", devid);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + if (cpuid > MAX_DMA_CPU_ID) {
> + dev_err(&pdev->dev, "invalid cpu id: %u\n", cpuid);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + dma_spec->np = of_parse_phandle(ofdma->of_node, "dma-masters", 0);
> + if (!dma_spec->np) {
> + dev_err(&pdev->dev, "can't get dma master\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + spin_lock_irqsave(&dmamux->lock, flags);
> +
> + if (test_bit(devid, dmamux->mapped_peripherals)) {
> + llist_for_each_entry(map, dmamux->reserve_maps.first, node) {
> + if (map->peripheral == devid && map->cpu == cpuid)
> + goto found;
> + }
> +
> + ret = -EINVAL;
> + goto failed;
> + } else {
> + node = llist_del_first(&dmamux->free_maps);
> + if (!node) {
> + ret = -ENODEV;
> + goto failed;
> + }
> +
> + map = llist_entry(node, struct cv1800_dmamux_map, node);
> + llist_add(&map->node, &dmamux->reserve_maps);
> + set_bit(devid, dmamux->mapped_peripherals);
> + }
> +
> +found:
> + chid = map->channel;
> + map->peripheral = devid;
> + map->cpu = cpuid;
> +
> + regmap_set_bits(dmamux->regmap,
> + DMAMUX_CH_REG(chid),
> + DMAMUX_CH_SET(chid, devid));
> +
> + regmap_update_bits(dmamux->regmap, CV1800_SDMA_DMA_INT_MUX,
> + DMAMUX_INT_CH_MASK(chid, cpuid),
> + DMAMUX_INT_CH_BIT(chid, cpuid));
> +
> + spin_unlock_irqrestore(&dmamux->lock, flags);
> +
> + dma_spec->args[0] = chid;
> +
> + dev_dbg(&pdev->dev, "register channel %u for req %u (cpu %u)\n",
> + chid, devid, cpuid);
> +
> + return map;
> +
> +failed:
> + spin_unlock_irqrestore(&dmamux->lock, flags);
> + of_node_put(dma_spec->np);
> + dev_err(&pdev->dev, "errno %d\n", ret);
> + return ERR_PTR(ret);
> +
Nitpick: Unneeded empty new line.
> +}
> +
> +static int cv1800_dmamux_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *mux_node = dev->of_node;
> + struct cv1800_dmamux_data *data;
> + struct cv1800_dmamux_map *tmp;
> + struct device *parent = dev->parent;
> + struct regmap *regmap = NULL;
> + unsigned int i;
> +
> + if (!parent)
> + return -ENODEV;
> +
> + regmap = device_node_to_regmap(parent->of_node);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + data = devm_kmalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + spin_lock_init(&data->lock);
> + init_llist_head(&data->free_maps);
Why init free_maps and not reserve_maps?
'data' is not zeroed, so it should be needed, IMHO.
Same for mapped_peripherals. It is not initialized.
I think that using devm_kzalloc() above is needed. (and
init_llist_head(&data->free_maps) could then be removed, if you want)
Just my 2c.
> +
> + for (i = 0; i <= MAX_DMA_CH_ID; i++) {
> + tmp = devm_kmalloc(dev, sizeof(*tmp), GFP_KERNEL);
> + if (!tmp) {
> + /* It is OK for not allocating all channel */
> + dev_warn(dev, "can not allocate channel %u\n", i);
> + continue;
> + }
> +
> + init_llist_node(&tmp->node);
> + tmp->channel = i;
> + llist_add(&tmp->node, &data->free_maps);
> + }
> +
> + /* if no channel is allocated, the probe must fail */
> + if (llist_empty(&data->free_maps))
> + return -ENOMEM;
> +
> + data->regmap = regmap;
> + data->dmarouter.dev = dev;
> + data->dmarouter.route_free = cv1800_dmamux_free;
> +
> + platform_set_drvdata(pdev, data);
> +
> + return of_dma_router_register(mux_node,
> + cv1800_dmamux_route_allocate,
> + &data->dmarouter);
> +}
...
CJ
Powered by blists - more mailing lists