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 <eugeniy.palt...@synopsys.com>
> > > > ---
> > > > 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 <common.h>
> > > > +#include <clk.h>
> > > > #include <dm.h>
> > > > #include <errno.h>
> > > > #include <malloc.h>
> > > > @@ -18,7 +19,6 @@
> > > > #include <fdtdec.h>
> > > > #include <linux/compat.h>
> > > > #include <asm/io.h>
> > > > -#include <asm/arch/clock_manager.h>
> > > >
> > > > 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
