Hi Ramiro,
Thank you for the patch.
On Wednesday 14 Dec 2016 17:18:24 Ramiro Oliveira wrote:
> Add a DT property to control an optional external reset line
>
> Signed-off-by: Ramiro Oliveira <roliveir@xxxxxxxxxxxx>
> ---
> drivers/dma/xilinx/xilinx_dma.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/dma/xilinx/xilinx_dma.c
> b/drivers/dma/xilinx/xilinx_dma.c index 5c9f11b..b845224 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -46,6 +46,7 @@
> #include <linux/slab.h>
> #include <linux/clk.h>
> #include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/reset.h>
I had neatly sorted the header alphabetically until someone added clk.h and
io-64-nonatomic-lo-hi.h :-( Could you please move reset.h just before slab.h ?
>
> #include "../dmaengine.h"
>
> @@ -409,6 +410,7 @@ struct xilinx_dma_device {
> struct clk *rxs_clk;
> u32 nr_channels;
> u32 chan_id;
> + struct reset_control *rst;
> };
>
> /* Macros */
> @@ -2543,6 +2545,27 @@ static int xilinx_dma_probe(struct platform_device
> *pdev) if (IS_ERR(xdev->regs))
> return PTR_ERR(xdev->regs);
>
> + xdev->rst = devm_reset_control_get_optional(&pdev->dev, "reset");
devm_reset_control_get_optional() is deprecated as explained in linux/reset.h,
you should use devm_reset_control_get_optional_exclusive() or
devm_reset_control_get_optional_shared() instead, as applicable.
This being said, I'm wondering why the optional versions of those functions
exist, as they do exactly the same as the non-optional versions. The API feels
wrong, it should have been modelled like the GPIO API. Feel free to fix it if
you want :-) But that's out of scope for this patch.
> + if (IS_ERR(xdev->rst)) {
> + err = PTR_ERR(xdev->rst);
> + switch (err) {
> + case -ENOENT:
If you drop the name as proposed in the review of patch 1/2 you don't have to
check for -ENOENT.
> + case -ENOTSUPP:
> + xdev->rst = NULL;
> + break;
Wrong indentation.
You need to handle -EPROBE_DEFER and defer probing of the xilinx_dma device.
> + default:
> + dev_err(xdev->dev, "error getting reset %d\n", err);
> + return err;
Wrong indentation.
> + }
> + } else {
> + err = reset_control_deassert(xdev->rst);
> + if (err) {
> + dev_err(xdev->dev, "failed to deassert reset: %d\n",
> + err);
Wrong indentation.
> + return err;
> + }
> + }
> +
> /* Retrieve the DMA engine properties from the device tree */
> xdev->has_sg = of_property_read_bool(node, "xlnx,include-sg");
> if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA)
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
