Skip to contentSkip to conversation

i2c: i2c-sh_mobile bus speed platform data

PATCH dong chen 1 participant 1 message May 4, 2011

Subsystems (2)
lore
Share & tools
lore

dong chen

author
#0 lore raw

Magnus Damm <[email protected]>编写:

Quoted 25 lines · Paul Mundt
Quoted 17 lines · Magnus Damm
Quoted 11 lines · Paul Mundt
Quoted 10 lines · Simon Horman
Quoted 8 lines · Magnus Damm
Quoted 2 lines · Simon Horman

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.

Quoted 4 lines
Quoted 2 lines

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.

Quoted 7 lines

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.

Quoted 3 lines

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.

Quoted 7 lines
Quoted 7 lines
Quoted 6 lines

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.

Quoted 7 lines
Quoted 3 lines

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.

I looks like I have been unclear in my writing once again. I'm not arguing that the header should be kept in include/linux/.

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html