| Message ID | 20200805101838.180134-1-chee.hong.ang@intel.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [v1] spi: cadence_qspi: Probe fail if QSPI clock is not set | expand |
Hi, On 05/08/20 3:48 pm, Chee Hong Ang wrote: > If the QSPI clock is not set (read as 0), QSPI driver probe shall fail > and prevent further QSPI access. > > Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com> > --- > drivers/spi/cadence_qspi.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > index 1e85749209..3bb5c7031d 100644 > --- a/drivers/spi/cadence_qspi.c > +++ b/drivers/spi/cadence_qspi.c > @@ -170,6 +170,9 @@ static int cadence_spi_probe(struct udevice *bus) > struct clk clk; > int ret; > > + if (!CONFIG_CQSPI_REF_CLK) > + return -ENODEV; > + This seems broken to me... Most platforms have moved away from passing clk freq via CONFIG_ and are using more scalable DT or platdata approach where freq is obtained via clk_get_*() APIs... See few lines below: if (plat->ref_clk_hz == 0) { ret = clk_get_by_index(bus, 0, &clk); if (ret) { #ifdef CONFIG_CQSPI_REF_CLK plat->ref_clk_hz = CONFIG_CQSPI_REF_CLK; #else return ret; #endif } else { plat->ref_clk_hz = clk_get_rate(&clk); clk_free(&clk); if (IS_ERR_VALUE(plat->ref_clk_hz)) return plat->ref_clk_hz; } } So there is no need to make CONFIG_CQSPI_REF_CLK mandatory. Probe may fail only when CONFIG_CQSPI_REF_CLK is undefined and there is no reference to a valid clk device either. > priv->regbase = plat->regbase; > priv->ahbbase = plat->ahbbase; > > Regards Vignesh
> Hi, > > On 05/08/20 3:48 pm, Chee Hong Ang wrote: > > If the QSPI clock is not set (read as 0), QSPI driver probe shall fail > > and prevent further QSPI access. > > > > Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com> > > --- > > drivers/spi/cadence_qspi.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > > index 1e85749209..3bb5c7031d 100644 > > --- a/drivers/spi/cadence_qspi.c > > +++ b/drivers/spi/cadence_qspi.c > > @@ -170,6 +170,9 @@ static int cadence_spi_probe(struct udevice *bus) > > struct clk clk; > > int ret; > > > > + if (!CONFIG_CQSPI_REF_CLK) > > + return -ENODEV; > > + > > This seems broken to me... > > Most platforms have moved away from passing clk freq via CONFIG_ and are > using more scalable DT or platdata approach where freq is obtained via I think the proper way to resolve this is to change our clock driver to DM. Thanks. > clk_get_*() APIs... See few lines below: > > if (plat->ref_clk_hz == 0) { > ret = clk_get_by_index(bus, 0, &clk); > if (ret) { > #ifdef CONFIG_CQSPI_REF_CLK > plat->ref_clk_hz = CONFIG_CQSPI_REF_CLK; #else > return ret; > #endif > } else { > plat->ref_clk_hz = clk_get_rate(&clk); > clk_free(&clk); > if (IS_ERR_VALUE(plat->ref_clk_hz)) > return plat->ref_clk_hz; > } > } > > So there is no need to make CONFIG_CQSPI_REF_CLK mandatory. > Probe may fail only when CONFIG_CQSPI_REF_CLK is undefined and there is > no reference to a valid clk device either. > > > > priv->regbase = plat->regbase; > > priv->ahbbase = plat->ahbbase; > > > > > > Regards > Vignesh
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 1e85749209..3bb5c7031d 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -170,6 +170,9 @@ static int cadence_spi_probe(struct udevice *bus) struct clk clk; int ret; + if (!CONFIG_CQSPI_REF_CLK) + return -ENODEV; + priv->regbase = plat->regbase; priv->ahbbase = plat->ahbbase;
If the QSPI clock is not set (read as 0), QSPI driver probe shall fail and prevent further QSPI access. Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com> --- drivers/spi/cadence_qspi.c | 3 +++ 1 file changed, 3 insertions(+)