On Thu, 2016-04-21 at 09:32:44 -0700, Appana Durga Kedareswara Rao wrote:
> Hi Soren,
>
> > -----Original Message-----
> > From: Sören Brinkmann [mailto:soren.brinkm...@xilinx.com]
> > Sent: Thursday, April 21, 2016 9:52 PM
> > To: Appana Durga Kedareswara Rao <appa...@xilinx.com>
> > Cc: robh...@kernel.org; pawel.m...@arm.com; mark.rutl...@arm.com;
> > ijc+devicet...@hellion.org.uk; ga...@codeaurora.org; Michal Simek
> > <mich...@xilinx.com>; vinod.k...@intel.com; dan.j.willi...@intel.com;
> > Appana Durga Kedareswara Rao <appa...@xilinx.com>;
> > moritz.fisc...@ettus.com; laurent.pinch...@ideasonboard.com;
> > l...@debethencourt.com; Anirudha Sarangi <anir...@xilinx.com>; Punnaiah
> > Choudary Kalluri <punn...@xilinx.com>; Shubhrajyoti Datta
> > <shubh...@xilinx.com>; devicet...@vger.kernel.org; linux-arm-
> > ker...@lists.infradead.org; linux-kernel@vger.kernel.org;
> > dmaeng...@vger.kernel.org
> > Subject: Re: [PATCH v3 3/3] dmaengine: vdma: Add clock support
> >
> > On Thu, 2016-04-21 at 16:08:38 +0530, Kedareswara rao Appana wrote:
[...]
> > > @@ -1757,6 +1767,200 @@ static void xilinx_dma_chan_remove(struct
> > xilinx_dma_chan *chan)
> > > list_del(&chan->common.device_node);
> > > }
> > >
> > > +static int axidma_clk_init(struct platform_device *pdev, struct clk
> > > **axi_clk,
> > > + struct clk **tx_clk, struct clk **rx_clk,
> > > + struct clk **sg_clk, struct clk **tmp_clk) {
> > > + int err;
> > > +
> > > + *tmp_clk = NULL;
> > > +
> > > + *axi_clk = devm_clk_get(&pdev->dev, "s_axi_lite_aclk");
> > > + if (IS_ERR(*axi_clk)) {
> > > + err = PTR_ERR(*axi_clk);
> > > + dev_err(&pdev->dev, "failed to get axi_aclk (%u)\n", err);
> > > + return err;
> > > + }
> > > +
> > > + *tx_clk = devm_clk_get(&pdev->dev, "m_axi_mm2s_aclk");
> > > + if (IS_ERR(*tx_clk))
> > > + *tx_clk = NULL;
> > > +
> > > + *rx_clk = devm_clk_get(&pdev->dev, "m_axi_s2mm_aclk");
> > > + if (IS_ERR(*rx_clk))
> > > + *rx_clk = NULL;
> > > +
> > > + *sg_clk = devm_clk_get(&pdev->dev, "m_axi_sg_aclk");
> > > + if (IS_ERR(*sg_clk))
> > > + *sg_clk = NULL;
> > > +
> > > +
> > > + err = clk_prepare_enable(*axi_clk);
> >
> > Should this be called if you know that the pointer might be NULL?
>
> It is a mandatory clock and if this clk is not there in DT I am already
> returning error...
> I didn't get your question could you please elaborate???
But for all the optional clocks. They could all be NULL and you're calling
clk_prepare_enable with a NULL pointer. That function is nice enough to
do a NULL check for you, but I wonder whether these calls should happen at
all when you already know that the pointer is not a valid clock.
Sören
