| Message ID | 1428002603-21892-5-git-send-email-jonathar@broadcom.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Le 02/04/2015 12:23, Jonathan Richardson a écrit : > The driver wasn't setting the SPBR (serial clock baud rate) which caused > it to run at the slowest speed possible. The driver now calculates the > SPBR based on the reference clock frequency resulting in much faster > SPI transfers. > > Signed-off-by: Jonathan Richardson <jonathar@broadcom.com> > --- [snip] > + data->clk = devm_clk_get(dev, "mspi_clk"); > + if (!IS_ERR(data->clk)) { > + int ret = clk_prepare_enable(data->clk); > + > + if (ret < 0) { > + dev_err(dev, "failed to enable clock: %d\n", ret); > + return 0; > + } > + > + /* Calculate SPBR if clock-frequency provided. */ > + if (of_property_read_u32(dev->of_node, "clock-frequency", > + &desired_rate) >= 0) { > + u32 spbr = clk_get_rate(data->clk) / (2 * desired_rate); Usually, specifying a "clock-frequency" property is done when there is no clock provider available, yet we take this code path only if we could find a "mspi_clk" which sounds a litle weird. Once there is a proper "mspi_clk" clock, I would make it mandatory for the clock provider to be able to provide the rate as well? -- Florian -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Apr 04, 2015 at 12:12:59PM -0700, Florian Fainelli wrote: > Le 02/04/2015 12:23, Jonathan Richardson a écrit : > > + /* Calculate SPBR if clock-frequency provided. */ > > + if (of_property_read_u32(dev->of_node, "clock-frequency", > > + &desired_rate) >= 0) { > > + u32 spbr = clk_get_rate(data->clk) / (2 * desired_rate); > Usually, specifying a "clock-frequency" property is done when there is > no clock provider available, yet we take this code path only if we could > find a "mspi_clk" which sounds a litle weird. > Once there is a proper "mspi_clk" clock, I would make it mandatory for > the clock provider to be able to provide the rate as well? As far as I can tell it's already mandatory if the property is present - it's taking the clock presented to the block and then using an internal divider to bring that down to the desired rate. We are missing error handling though.
On 15-04-06 02:46 AM, Mark Brown wrote: > On Sat, Apr 04, 2015 at 12:12:59PM -0700, Florian Fainelli wrote: >> Le 02/04/2015 12:23, Jonathan Richardson a écrit : > >>> + /* Calculate SPBR if clock-frequency provided. */ >>> + if (of_property_read_u32(dev->of_node, "clock-frequency", >>> + &desired_rate) >= 0) { >>> + u32 spbr = clk_get_rate(data->clk) / (2 * desired_rate); > >> Usually, specifying a "clock-frequency" property is done when there is >> no clock provider available, yet we take this code path only if we could >> find a "mspi_clk" which sounds a litle weird. > >> Once there is a proper "mspi_clk" clock, I would make it mandatory for >> the clock provider to be able to provide the rate as well? > > As far as I can tell it's already mandatory if the property is present - > it's taking the clock presented to the block and then using an internal > divider to bring that down to the desired rate. > > We are missing error handling though. > Thanks for the review. Yes that's correct. I also tried to make it backwards compatible with the current version of the driver where you can ignore configuring the frequency. The result being ref clock frequency / 2 * 255. If you provide the clock then it will enable/prepare it. If you also provide clock-frequency then it will configure the SPBR. If you don't provide anything then it works as before - it assumes the clock is already enabled and uses the h/w default SPBR. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/spi/spi-bcm-mspi.c b/drivers/spi/spi-bcm-mspi.c index df27449..5617b5b 100644 --- a/drivers/spi/spi-bcm-mspi.c +++ b/drivers/spi/spi-bcm-mspi.c @@ -18,10 +18,15 @@ #include <linux/bcma/bcma.h> #include <linux/spi/spi.h> #include <linux/of.h> +#include <linux/clk.h> #include "spi-bcm-mspi.h" #define BCM_MSPI_MAX_SPI_BAUD 13500000 /* 216 MHz? */ +#define SPBR_MIN 8U +#define SPBR_MAX 255U +#define MSPI_SPCR0_LSB_OFFSET 0x200 +#define MSPI_SPCR0_LSB_SHIFT 0 /* The longest observed required wait was 19 ms */ #define BCM_MSPI_SPE_TIMEOUT_MS 80 @@ -33,7 +38,9 @@ struct bcm_mspi { void __iomem *base; struct spi_master *master; + struct clk *clk; size_t read_offset; + u32 spbr; void (*mspi_write)(struct bcm_mspi *mspi, u16 offset, u32 value); u32 (*mspi_read)(struct bcm_mspi *mspi, u16 offset); @@ -45,6 +52,15 @@ static inline unsigned int bcm_mspi_calc_timeout(size_t len) return (len * 9000 / BCM_MSPI_MAX_SPI_BAUD * 110 / 100) + 1; } +static void bcm_mspi_hw_init(struct bcm_mspi *mspi) +{ + /* Set SPBR (serial clock baud rate). */ + if (mspi->spbr) { + mspi->mspi_write(mspi, MSPI_SPCR0_LSB_OFFSET, + mspi->spbr << MSPI_SPCR0_LSB_SHIFT); + } +} + static int bcm_mspi_wait(struct bcm_mspi *mspi, unsigned int timeout_ms) { unsigned long deadline; @@ -221,6 +237,7 @@ static struct bcm_mspi *bcm_mspi_init(struct device *dev) { struct bcm_mspi *data; struct spi_master *master; + u32 desired_rate; master = spi_alloc_master(dev, sizeof(*data)); if (!master) { @@ -235,6 +252,31 @@ static struct bcm_mspi *bcm_mspi_init(struct device *dev) master->dev.of_node = dev->of_node; master->transfer_one = bcm_mspi_transfer_one; + /* + * Enable clock if provided. The frequency can be changed by setting + * SPBR (serial clock baud rate) based on the desired 'clock-frequency'. + * + * Baud rate is calculated as: mspi_clk / (2 * SPBR) where SPBR is a + * value between 1-255. If not set then it is left at the h/w default. + */ + data->clk = devm_clk_get(dev, "mspi_clk"); + if (!IS_ERR(data->clk)) { + int ret = clk_prepare_enable(data->clk); + + if (ret < 0) { + dev_err(dev, "failed to enable clock: %d\n", ret); + return 0; + } + + /* Calculate SPBR if clock-frequency provided. */ + if (of_property_read_u32(dev->of_node, "clock-frequency", + &desired_rate) >= 0) { + u32 spbr = clk_get_rate(data->clk) / (2 * desired_rate); + + data->spbr = clamp_val(spbr, SPBR_MIN, SPBR_MAX); + } + } + return data; } @@ -286,6 +328,9 @@ static int bcm_mspi_probe(struct platform_device *pdev) data->mspi_write = bcm_mspi_write; platform_set_drvdata(pdev, data); + /* Initialize SPI controller. */ + bcm_mspi_hw_init(data); + err = devm_spi_register_master(dev, data->master); if (err) goto out; @@ -360,6 +405,9 @@ static int bcm_mspi_bcma_probe(struct bcma_device *core) data->core = core; bcma_set_drvdata(core, data); + /* Initialize SPI controller. */ + bcm_mspi_hw_init(data); + err = devm_spi_register_master(&core->dev, data->master); if (err) { spi_master_put(data->master);
The driver wasn't setting the SPBR (serial clock baud rate) which caused it to run at the slowest speed possible. The driver now calculates the SPBR based on the reference clock frequency resulting in much faster SPI transfers. Signed-off-by: Jonathan Richardson <jonathar@broadcom.com> --- drivers/spi/spi-bcm-mspi.c | 48 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)