On Thu, Apr 28, 2011 at 12:25:30PM +0900, Magnus Damm wrote: > On Thu, Apr 28, 2011 at 12:11 PM, Paul Mundt <lethal@xxxxxxxxxxxx> wrote: > > 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. > > I suspect that I may have chosen my words poorly. By writing "This may > also be an attempt to simply double the I2C bus speed on that > particular platform for that particular application." I am not > referring to this patch. I am commenting on something Simon may see > when he is going though kernel source that has been mangled by people > doing integration. > > As for configuring the i2c bus speed, I wouldn't call it dodgy. It is > something that is fairly common but of course needs to be handled with > care. Other drivers handle this through module parameters, but since > we want to configure this with per-device instance granularity > platform data is a better fit. To clarify my position on this. I merely observed a different implementation of essentially the same code that doubles the value for the sh73a0. I am not entirely sure why it was doubled. Nor am I sure why it was implemented outside of platform code. However, I am sure that the exception that I noted is nowhere in upstream code. So I think it would be fine to merge Magnus's changes. If there is interest in resolving this sh73a0 anomaly then I guess enquires will need to be made to the people with the alternate implementation. That is probably a discussion best had off-list. -- 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
