From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eugeniy Paltsev Date: Tue, 12 Dec 2017 14:30:07 +0000 Subject: [U-Boot] [PATCH v6 2/2] DW SPI: Get clock value from Device Tree In-Reply-To: <6d265b43-65ab-1ec7-1782-639e78e912fa@denx.de> References: <20171211161855.14958-1-Eugeniy.Paltsev@synopsys.com> <20171211161855.14958-3-Eugeniy.Paltsev@synopsys.com> <1513010240.2351.2.camel@synopsys.com> <6d265b43-65ab-1ec7-1782-639e78e912fa@denx.de> Message-ID: <1513089007.2351.5.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:41 +0100, Marek Vasut wrote: > On 12/11/2017 05:37 PM, Eugeniy Paltsev wrote: > > 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. > > +CC Simon, that's somewhat iffy ... Looks like it is quite common approach, I can see it in some uboot drivers: For example: drivers/mmc/zynq_sdhci.c --------------->8------------ ret = clk_enable(&clk); if (ret && ret != -ENOSYS) { dev_err(dev, "failed to enable clock\n"); return ret; } --------------->8------------ --  Eugeniy Paltsev