On Friday 23 May 2008, Grant Likely wrote: > On Thu, May 22, 2008 at 8:15 PM, David Brownell <[EMAIL PROTECTED]> wrote: > > On Friday 16 May 2008, you wrote: > >> + prop = of_get_property(nc, "max-speed", &len); > >> + if (prop && len >= sizeof(*prop)) > >> + spi->max_speed_hz = *prop; > >> + else > >> + spi->max_speed_hz = 100000; > > > > This isn't I2C; I suggest a default more appropriate to SPI! > > Maybe 10 MHz, rather than 100 KHz; or if you want folk to use > > this *a lot* then maybe 1 MHz. I'd consider it a bug to have > > folk rely on this very much, though. > > Yeah, I thought it a little stinky when I wrote it, but I wanted to > put *something* in for the case where the driver sets it's own value > for max_speed and it can be omitted from the device tree.
That is, this is just so the spi_setup() from spi_new_device() doesn't fail? Thing is, drivers playing with max_speed are rare. They just can't know the board-specific factors involved in making some speed fail, even when the chip might handle it on other boards. (Factors like lower voltage and higher capacitance tend to reduce the speeds that will work.) The only driver I know which mucks with max_speed_hz is the MMC-over-SPI driver, and that's because the cards themselves report various ranges that can work ... and even there, the driver remembers the board-specific maximum (which may be less than the card can handle). > Maybe it > would just be better to leave it as 0 if the max-speed property is > non-existent. Which would usually cause spi_setup() to fail, and thus cause the device not to be listed ... > What do you think? Technically, spi_setup() with max_speed set to 0 isn't what I'd call well specified ... the issue rarely came up before. "Max" of zero basically means "off", and there's not really any such state in the spi_device lifecycle. Those are good reasons to avoid such boundary cases; also, it's easy to oops in divider calculations when zero is used. Instead, board init code has set up nonzero speeds, so the spi_new_device() call to spi_setup() hasn't had ar eason to fail because max_speed was illegal/unsupportable. If it's worth avoiding, that may mean it's worth defaulting it in core code (e.g. spi_new_device) not OF platform glue... but I basically think of that speed as a value that board setup code *must* provide. - Dave ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ spi-devel-general mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/spi-devel-general
