From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eugeniy Paltsev Date: Mon, 11 Dec 2017 16:37:20 +0000 Subject: [U-Boot] [PATCH v6 2/2] DW SPI: Get clock value from Device Tree In-Reply-To: References: <20171211161855.14958-1-Eugeniy.Paltsev@synopsys.com> <20171211161855.14958-3-Eugeniy.Paltsev@synopsys.com> Message-ID: <1513010240.2351.2.camel@synopsys.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On Mon, 2017-12-11 at 17:21 +0100, Marek Vasut wrote: > On 12/11/2017 05:18 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) fon'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 > > --- > >  drivers/spi/designware_spi.c | 43 +++++++++++++++++++++++++++++++++++++++++-- > >  1 file changed, 41 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c > > index 5aa507b..a037376 100644 > > --- a/drivers/spi/designware_spi.c > > +++ b/drivers/spi/designware_spi.c > > @@ -11,6 +11,7 @@ > >   */ > >   > >  #include > > +#include > >  #include > >  #include > >  #include > > @@ -18,7 +19,6 @@ > >  #include > >  #include > >  #include > > -#include > >   > >  DECLARE_GLOBAL_DATA_PTR; > >   > > @@ -94,6 +94,8 @@ struct dw_spi_priv { > >   void __iomem *regs; > >   unsigned int freq; /* Default frequency */ > >   unsigned int mode; > > + struct clk clk; > > + unsigned long bus_clk_rate; > >   > >   int bits_per_word; > >   u8 cs; /* chip select pin */ > > @@ -176,14 +178,51 @@ static void spi_hw_init(struct dw_spi_priv *priv) > >   debug("%s: fifo_len=%d\n", __func__, priv->fifo_len); > >  } > >   > > +/* > > + * 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 dw_spi_priv *priv = dev_get_priv(bus); > > + int ret; > > + > > + ret = clk_get_by_index(bus, 0, &priv->clk); > > + if (ret) > > + return -EINVAL; > > if (ret) >  return ret; Ok. > > > + ret = clk_enable(&priv->clk); > > + if (ret && ret != -ENOSYS && ret != -ENOSYS && ret != -ENOTSUPP) > > You have ENOSYS twice in there, but you can do just if (ret) return ret > again. The idea is to skip error if error code is -ENOSYS or -ENOTSUPP as it is OK situation: some clock drivers don't implement .enable/.disable callbacks so clk_enable returns -ENOSYS. Also some clock drivers implement .enable/.disable callbacks not for all clock IDs and return -ENOSYS or -ENOTSUPP for others. But definitely I should remove second ENOSYS here. > > + return ret; > > + > > + *rate = clk_get_rate(&priv->clk); > > + if (!*rate) { > > + clk_disable(&priv->clk); > > You didn't do clk_free() here, just implement a failpath (ie. goto > err_rate). > > > + return -EINVAL; > > + } > > + > > + debug("%s: get spi controller clk via device tree: %lu Hz\n", > > +       __func__, *rate); > > + > > + clk_free(&priv->clk); > > If anyone accesses priv->clk outside of this function, the code will > crash, so remove this. > > > + return 0; > > err_rate: >  clk_free() ... Sure, thanks. > > > +} > > + > >  static int dw_spi_probe(struct udevice *bus) > >  { > >   struct dw_spi_platdata *plat = dev_get_platdata(bus); > >   struct dw_spi_priv *priv = dev_get_priv(bus); > > + int ret; > >   > >   priv->regs = plat->regs; > >   priv->freq = plat->frequency; > >   > > + ret = dw_spi_get_clk(bus, &priv->bus_clk_rate); > > + if (ret) > > + return ret; > > + > >   /* Currently only bits_per_word == 8 supported */ > >   priv->bits_per_word = 8; > >   > > @@ -369,7 +408,7 @@ static int dw_spi_set_speed(struct udevice *bus, uint speed) > >   spi_enable_chip(priv, 0); > >   > >   /* clk_div doesn't support odd number */ > > - clk_div = cm_get_spi_controller_clk_hz() / speed; > > + clk_div = priv->bus_clk_rate / speed; > >   clk_div = (clk_div + 1) & 0xfffe; > >   dw_writel(priv, DW_SPI_BAUDR, clk_div); > >   > > > > --  Eugeniy Paltsev