Hi Jagan, Thanks for respond, my comments are given inline.
On Wed, 2017-10-11 at 16:36 +0530, Jagan Teki wrote:
On Tue, Sep 26, 2017 at 6:40 PM, Eugeniy Paltsev Eugeniy.Paltsev@synopsys.com wrote:
Add option to set spi controller clock frequency via device tree using standard clock bindings. Old way of setting spi controller clock frequency (via implementation of 'cm_get_spi_controller_clk_hz' function in platform specific code) remains supported. Signed-off-by: Eugeniy Paltsev Eugeniy.Paltsev@synopsys.com
diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c index 5aa507b..c70697e 100644 --- a/drivers/spi/designware_spi.c +++ b/drivers/spi/designware_spi.c @@ -11,6 +11,7 @@ +/*
- cm_get_spi_controller_clk_hz function is old way to set spi controller
- frequency. If it isn't implemented and spi controller frequency isn't set via
- device tree we will get into next default function.
- */
+__weak unsigned int cm_get_spi_controller_clk_hz(void) +{ + error("SPI clock is defined neither via device tree nor via cm_get_spi_controller_clk_hz!");
+ return 0; +}
+static int dw_spi_get_clk(struct udevice *bus) +{ + struct dw_spi_priv *priv = dev_get_priv(bus);
+ /* Firstly try to get clock frequency from device tree */ + if (!dw_spi_of_get_clk(bus)) + return 0;
+ /* In case of failure rollback to cm_get_spi_controller_clk_hz */ + priv->bus_clk_rate = cm_get_spi_controller_clk_hz();
Why weak function, we can just return with error message since weak does the same job
I used weak function for backward compatibility with the previous driver version: The previous driver version used cm_get_spi_controller_clk_hz() function which is set outside of designware_spi.c You can look at arch/arm/mach-socfpga/clock_manager_arria10.c arch/arm/mach-socfpga/clock_manager_gen5.c ... as examples.
So I define cm_get_spi_controller_clk_hz as weak in this driver to handle both use cases: 1) We set frequency via device tree and cm_get_spi_controller_clk_hz is NOT defined outside of designware_spi.c (new way) 2) We set frequency using cm_get_spi_controller_clk_hz, which is defined outside of designware_spi.c (old way)
I guess it is OK? -- Eugeniy Paltsev