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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html