From mboxrd@z Thu Jan  1 00:00:00 1970
From: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Date: Sat, 9 Dec 2017 15:23:33 +0000
Subject: [U-Boot] [uboot-snps-arc] Re: [PATCH v5 2/2] DW SPI: Get clock
 value from Device Tree
In-Reply-To: <50cc22a6-fa87-c830-09ed-9146875899f1@denx.de>
References: <20171114153345.8655-1-Eugeniy.Paltsev@synopsys.com>
 <20171114153345.8655-3-Eugeniy.Paltsev@synopsys.com>
 <50cc22a6-fa87-c830-09ed-9146875899f1@denx.de>
Message-ID: <1512833013.2699.74.camel@synopsys.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
To: u-boot@lists.denx.de

On Wed, 2017-11-15 at 10:24 +0100, Marek Vasut wrote:
> On 11/14/2017 04:33 PM, Eugeniy Paltsev wrote:
> > Add option to set spi controller clock frequency via device tree
> > using standard clock bindings.
> > 
> > Define dw_spi_get_clk function as 'weak' as some targets
> > (like SOCFPGA_GEN5 and SOCFPGA_ARRIA10) don't use standard clock API
> > and implement dw_spi_get_clk their own way in their clock manager.
> > 
> > Get rid of clock_manager.h include as we don't use
> > cm_get_spi_controller_clk_hz function anymore. (we use redefined
> > dw_spi_get_clk in SOCFPGA clock managers instead)
> > 
> > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> > ---
> >  drivers/spi/designware_spi.c | 42 ++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 40 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
> > index 6cc4f51..470a3a7 100644
> > --- a/drivers/spi/designware_spi.c
> > +++ b/drivers/spi/designware_spi.c
> > @@ -12,6 +12,7 @@
> >  
> > +/*
> > + * We define dw_spi_get_clk function as 'weak' as some targets
> > + * (like SOCFPGA_GEN5 and SOCFPGA_ARRIA10) fon't use standard clock API
> > + * and implement dw_spi_get_clk their own way in their clock manager.
> > + */
> > +__weak int dw_spi_get_clk(struct udevice *bus, ulong *rate)
> > +{
> > +	struct clk clk;
> > +	int ret;
> > +
> > +	ret = clk_get_by_index(bus, 0, &clk);
> > +	if (ret)
> > +		return -EINVAL;
> > +
> > +	ret = clk_enable(&clk);
> > +	if (ret && ret != -ENOSYS)
> > +		return ret;
> > +
> > +	*rate = clk_get_rate(&clk);
> > +	if (!*rate) {
> > +		clk_disable(&clk);
> > +		return -EINVAL;
> > +	}
> > +
> > +	debug("%s: get spi controller clk via device tree: %lu Hz\n",
> > +	      __func__, *rate);
> > +
> > +	clk_free(&clk);
> 
> You probably want to retain the handle to these clock in the private
> data, since otherwise you won't be able to turn the clock off in
> .remove() callback of the driver (if/once it's implemented)

No, .remove() callback isn't implemented in this driver, so it isn't necessary.

> Otherwise the patch look good, thanks !
> 
> 
-- 
 Eugeniy Paltsev