Hi Biju, On Fri, Jul 2, 2021 at 12:05 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote: > Add DMA Controller driver for RZ/G2L SoC. > > Based on the work done by Chris Brandt for RZ/A DMA driver. > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> Thanks for your patch! > --- /dev/null > +++ b/drivers/dma/sh/rz-dmac.c > +static void rz_dmac_set_dmars_register(struct rz_dmac *dmac, int nr, > + u32 dmars) > +{ > + u32 dmars_offset = (nr / 2) * 4; > + u32 dmars32; > + > + dmars32 = rz_dmac_ext_readl(dmac, dmars_offset); > + if (nr % 2) { > + dmars32 &= 0x0000ffff; > + dmars32 |= dmars << 16; > + } else { > + dmars32 &= 0xffff0000; > + dmars32 |= dmars; > + } An alternative to Vinod's suggestion: shift = (nr %2) * 16; dmars32 &= ~(0xffff << shift); dmars32 |= dmars << shift; > + > + rz_dmac_ext_writel(dmac, dmars32, dmars_offset); > +} > +static int rz_dmac_chan_probe(struct rz_dmac *dmac, > + struct rz_dmac_chan *channel, > + unsigned int index) > +{ > + struct platform_device *pdev = to_platform_device(dmac->dev); > + struct rz_lmdesc *lmdesc; > + char pdev_irqname[5]; > + char *irqname; > + int ret; > + > + channel->index = index; > + channel->mid_rid = -EINVAL; > + > + /* Request the channel interrupt. */ > + sprintf(pdev_irqname, "ch%u", index); > + channel->irq = platform_get_irq_byname(pdev, pdev_irqname); > + if (channel->irq < 0) > + return -ENODEV; Please propagate the error in channel->irq, which might be -EPROBE_DEFER. > +static int rz_dmac_parse_of(struct device *dev, struct rz_dmac *dmac) > +{ > + struct device_node *np = dev->of_node; > + int ret; > + > + ret = of_property_read_u32(np, "dma-channels", &dmac->n_channels); > + if (ret < 0) { > + dev_err(dev, "unable to read dma-channels property\n"); > + return ret; > + } > + > + if (!dmac->n_channels || dmac->n_channels > RZ_DMAC_MAX_CHANNELS) { > + dev_err(dev, "invalid number of channels %u\n", dmac->n_channels); > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int rz_dmac_probe(struct platform_device *pdev) > +{ > + const char *irqname = "error"; > + struct dma_device *engine; > + struct rz_dmac *dmac; > + int channel_num; > + int ret, i; unsigned int i; > + int irq; > + > + dmac = devm_kzalloc(&pdev->dev, sizeof(*dmac), GFP_KERNEL); > + if (!dmac) > + return -ENOMEM; > + > + dmac->dev = &pdev->dev; > + platform_set_drvdata(pdev, dmac); > + > + ret = rz_dmac_parse_of(&pdev->dev, dmac); > + if (ret < 0) > + return ret; > + > + dmac->channels = devm_kcalloc(&pdev->dev, dmac->n_channels, > + sizeof(*dmac->channels), GFP_KERNEL); > + if (!dmac->channels) > + return -ENOMEM; > + > + /* Request resources */ > + dmac->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(dmac->base)) > + return PTR_ERR(dmac->base); > + > + dmac->ext_base = devm_platform_ioremap_resource(pdev, 1); > + if (IS_ERR(dmac->ext_base)) > + return PTR_ERR(dmac->ext_base); > + > + /* Register interrupt handler for error */ > + irq = platform_get_irq_byname(pdev, irqname); > + if (irq < 0) { > + dev_err(&pdev->dev, "no error IRQ specified\n"); > + return -ENODEV; I'd say "return dev_err_probe(&pdev->dev, irq, ..);", but platform_get_irq_byname() already prints an error message, so please just use "return irq;" to propagate the error, which could be -EPROBE_DEFER. > + } > +static int rz_dmac_remove(struct platform_device *pdev) > +{ > + struct rz_dmac *dmac = platform_get_drvdata(pdev); > + int i; unsigned int it; > + > + for (i = 0; i < dmac->n_channels; i++) { Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds