On Thu, Apr 28, 2011 at 11:46:37AM +0900, Magnus Damm wrote: > On Thu, Apr 28, 2011 at 11:22 AM, Paul Mundt <lethal@xxxxxxxxxxxx> wrote: > > On Thu, Apr 28, 2011 at 10:18:01AM +0900, Simon Horman wrote: > >> On Thu, Apr 28, 2011 at 10:06:38AM +0900, Magnus Damm wrote: > >> > On Thu, Apr 28, 2011 at 7:14 AM, Simon Horman <horms@xxxxxxxxxxxx> wrote: > >> > > I believe that on the sh73a0 and so far only the sh73a0 > >> > > denom needs to be doubled. > >> > > >> > Uhm, I don't think this patch is specific to any SoC type. It may of > >> > course be used on sh73a0 to adjust the denom value, but setting the > >> > I2C bus speed is something that can be used on any SoC. So I'd say > >> > that this is a fairly generic feature. > >> > >> I'm just saying that that I've observed the value being doubled for sh73a0. > >> > > The general rule of thumb is that whatever unusual behaviour is observed > > in the latest CPU we will see become the standard for future ones. > > This may also be an attempt to simply double the I2C bus speed on that > particular platform for that particular application. So in the end it > may have nothing to do with sh73a0. Actually, now when I think about > it, I recall hacking up a prototype to control the LCD backlight via > I2C on sh73a0 and AG5EVM, and I did not have to modify any part of the > I2C bus driver to get that going as expected. > Ok, that bit of information was missing from your patch. That's obviously a bit more dodgy. We don't want to have the default behaviour out of spec for some specific application. > > Abstracting the denom value seems reasonable as a cautionary measure, and > > it does do the NORMAL_SPEED as a default fallback so there is no adverse > > impact for normalized platforms. > > > > This really should be going in to include/linux/i2c/ or so though, no > > need to dump these tiny stubs in to include/linux/ directly. > > I guess I just followed what other i2c drivers do, but yes, putting > the header file in include/linux/i2c/ makes more sense. > Other i2c drivers put things in include/linux/i2c, too. There's also an include/linux/platform_data/ now as well for things that don't fit anywhere else. "This is the way we used to do it" tends to be a pretty pointless argument going forward. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
