On Sun, Aug 11, 2024 at 08:00:05AM GMT, Christophe JAILLET wrote: > 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@xxxxxxxxxxx> > > --- > > 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. > Thanks, I missed these two field. Although I got no error when testing, it is necessary to fix this. I think devm_kzalloc can only zero "mapped_peripherals". It will be good to add missing init_llist_head for "reserve_maps". Regards, Inochi > > + > > + 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 > > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-riscv