RE: [PATCHv2 2/3] spi/spi-xilinx: Add clock support

[Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxxxxx>]

> -----Original Message-----
> From: Mark Brown [mailto:broonie@xxxxxxxxxx]
> Sent: Thursday, March 10, 2016 9:00 AM
> To: Shubhrajyoti Datta
> Cc: linux-spi@xxxxxxxxxxxxxxx; Soren Brinkmann;
> devicetree@xxxxxxxxxxxxxxx; Michal Simek; Shubhrajyoti Datta
> Subject: Re: [PATCHv2 2/3] spi/spi-xilinx: Add clock support
> 
> On Wed, Mar 09, 2016 at 02:17:21PM +0530, Shubhrajyoti Datta wrote:
> 
> > +       xspi->clk = devm_clk_get(&pdev->dev, NULL);
> > +       if (IS_ERR(xspi->clk)) {
> 
> As someone pointed out on the previous version of the series this will cause
> the driver to fail to probe with existing DTs.  We probably need to explicitly
> handle a -ENOENT as a "this clock will never appear" or something.
I got comments after I sent the v2.
I will update it with Lars comments.
> 
> This also requests a single nameless clock but someone pointed out on the
> previous version there are multiple clocks into the IP.  Even if you only want
> to add one clock right now the clock should probably be named so we can
> scale up

I will add a name.
.
> 
> > +	}
> > +	ret = clk_prepare_enable(xspi->clk);
> 
> Missing blank line here.
Will fix in next version
> 
> > +	if (ret)
> > +		dev_err(&pdev->dev, "Unable to enable clock.\n");
> > +
> 
> This isn't really checking the return code - if we failed to enable the clock we
> should be failing the probe, not just carrying on.
Will fix.
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html