linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Biju Das <biju.das.jz@bp.renesas.com>
Cc: Vinod Koul <vkoul@kernel.org>,
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	Chris Paterson <Chris.Paterson2@renesas.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	dmaengine <dmaengine@vger.kernel.org>,
	Chris Brandt <chris.brandt@renesas.com>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Subject: Re: [PATCH v3 2/4] drivers: dma: sh: Add DMAC driver for RZ/G2L SoC
Date: Wed, 14 Jul 2021 10:09:28 +0200	[thread overview]
Message-ID: <CAMuHMdV8VFoUC_Od9F=4On6=tZwr-qN4s4g+=_QcHQTrxrvQJg@mail.gmail.com> (raw)
In-Reply-To: <20210702100527.28251-3-biju.das.jz@bp.renesas.com>

Hi Biju,

On Fri, Jul 2, 2021 at 12:05 PM Biju Das <biju.das.jz@bp.renesas.com> 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@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

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@linux-m68k.org

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

  parent reply	other threads:[~2021-07-14  8:09 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-02 10:05 [PATCH v3 0/4] Add RZ/G2L DMAC support Biju Das
2021-07-02 10:05 ` [PATCH v3 1/4] dt-bindings: dma: Document RZ/G2L bindings Biju Das
2021-07-02 21:37   ` Rob Herring
2021-07-02 10:05 ` [PATCH v3 2/4] drivers: dma: sh: Add DMAC driver for RZ/G2L SoC Biju Das
2021-07-14  6:15   ` Vinod Koul
2021-07-15 12:25     ` Biju Das
2021-07-14  8:09   ` Geert Uytterhoeven [this message]
2021-07-15 12:27     ` Biju Das
2021-07-02 10:05 ` [PATCH v3 3/4] arm64: dts: renesas: r9a07g044: Add DMAC support Biju Das
2021-07-02 10:05 ` [PATCH v3 4/4] arm64: defconfig: Enable DMA controller for RZ/G2L SoC's Biju Das

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMuHMdV8VFoUC_Od9F=4On6=tZwr-qN4s4g+=_QcHQTrxrvQJg@mail.gmail.com' \
    --to=geert@linux-m68k.org \
    --cc=Chris.Paterson2@renesas.com \
    --cc=biju.das.jz@bp.renesas.com \
    --cc=chris.brandt@renesas.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=geert+renesas@glider.be \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).