ARM: S3C64XX: Fix clkdev device names for I2C clocks
Subsystems (3)
PATCH 1/1 ARM: S3C64XX: Fix clkdev device names for I2C clocks
When the S3C64xx CPUs were converted to clkdev mappings were added for the I2C controllers on them but there's a couple of issues with these mappings:
- We're randomly using a device name for controller 1 but not controller 0 which is odd.
- The controller type for controller 1 is s3c2440, not s3c2410 which is what the device is actually registered as so dev_name() based lookups fail.
The end result is that only controller 0 has a good mapping, though for some reason this did used to work.
Something in recent -next appears to have triggered this but I can't for the life of me figure out how it ever worked.
--- arch/arm/mach-s3c64xx/clock.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
arch/arm/mach-s3c64xx/clock.c +2 -1
@@ -138,12 +138,13 @@ static struct clk init_clocks_off[] = { .ctrlbit = S3C_CLKCON_PCLK_TSADC, }, { .name = "i2c", + .devname = "s3c2440-i2c.0", .parent = &clk_p, .enable = s3c64xx_pclk_ctrl, .ctrlbit = S3C_CLKCON_PCLK_IIC, }, { .name = "i2c", - .devname = "s3c2440-i2c.1", + .devname = "s3c2410-i2c.1", .parent = &clk_p, .enable = s3c64xx_pclk_ctrl, .ctrlbit = S3C6410_CLKCON_PCLK_I2C1,
Signature · 1 line
1.7.5.4
PATCH 1/1 ARM: S3C64XX: Fix clkdev device names for I2C clocks
Quoted 26 lines · Mark Brown
When the S3C64xx CPUs were converted to clkdev mappings were added for the I2C controllers on them but there's a couple of issues with these mappings:
- We're randomly using a device name for controller 1 but not controller 0 ?which is odd.
- The controller type for controller 1 is s3c2440, not s3c2410 which is ?what the device is actually registered as sodev_name()based lookups ?fail.The end result is that only controller 0 has a good mapping, though for some reason this did used to work.
Something in recent -next appears to have triggered this but I can't for the life of me figure out how it ever worked.
Signed-off-by Mark Brown <[email protected]>---
?arch/arm/mach-s3c64xx/clock.c | ? ?3 ++-
?1 files changed, 2 insertions(+), 1 deletions(-)arch/arm/mach-s3c64xx/clock.c +0
-0@@ -138,12 +138,13 @@ static struct clk init_clocks_off[] = { ? ? ? ? ? ? ? ?.ctrlbit ? ? ? ?= S3C_CLKCON_PCLK_TSADC,
arch/arm/mach-s3c64xx/clock.c static struct clk init_clocks_off[] = { · line 147
Quoted 10 lines
arch/arm/mach-s3c64xx/clock.c +2
-1? ? ? ?}, { ? ? ? ? ? ? ? ?.name ? ? ? ? ? = "i2c", + ? ? ? ? ? ? ? .devname ? ? ? ?= "s3c2440-i2c.0", ? ? ? ? ? ? ? ?.parent ? ? ? ? = &clk_p, ? ? ? ? ? ? ? ?.enable ? ? ? ? = s3c64xx_pclk_ctrl, ? ? ? ? ? ? ? ?.ctrlbit ? ? ? ?= S3C_CLKCON_PCLK_IIC, ? ? ? ?}, { ? ? ? ? ? ? ? ?.name ? ? ? ? ? = "i2c", - ? ? ? ? ? ? ? .devname ? ? ? ?= "s3c2440-i2c.1", + ? ? ? ? ? ? ? .devname ? ? ? ?= "s3c2410-i2c.1",
It's not matched with your description. and also not matched with codes at "arch/arm/mach-s3c64xx/s3c6410.c"
/* the i2c devices are directly compatible with s3c2440 */
s3c_i2c0_setname("s3c2440-i2c");
s3c_i2c1_setname("s3c2440-i2c");Thank you,
Kyungmin Park
Quoted 12 lines
arch/arm/mach-s3c64xx/clock.c +0
-1? ? ? ? ? ? ? ?.parent ? ? ? ? = &clk_p, ? ? ? ? ? ? ? ?.enable ? ? ? ? = s3c64xx_pclk_ctrl, ? ? ? ? ? ? ? ?.ctrlbit ? ? ? ?= S3C6410_CLKCON_PCLK_I2C1, --1.7.5.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/1 ARM: S3C64XX: Fix clkdev device names for I2C clocks
Quoted 6 lines · Kyungmin Park
It's not matched with your description. and also not matched with codes at "
arch/arm/mach-s3c64xx/s3c6410.c"/* the i2c devices are directly compatible with s3c2440 */ s3c_i2c0_setname("s3c2440-i2c"); s3c_i2c1_setname("s3c2440-i2c");
Oh, FFS. That's what's broken - the names that are actually appearing in the system are the names that the devices have in the relevant dev-i2c?.c files and obviously there's no reason one would expect to find code like the above since it's just a little abstruse to rewrite the devices like that.
Looking at the code I bet what broke it is the CPU revision identification changes - it's now responsible for making sure the fixups are run. Either it's not matching correctly or it's running too late to make the fixup before the device is registered.
Though the first part of the issue (the fact that I2C0 isn't using a device name and I2C1 is) remains an issue.
PATCH 1/1 ARM: S3C64XX: Fix clkdev device names for I2C clocks
Quoted 15 lines · Mark Brown
Quoted 6 lines · Kyungmin Park
It's not matched with your description. and also not matched with codes at "
arch/arm/mach-s3c64xx/s3c6410.c"? ? ? ?/* the i2c devices are directly compatible with s3c2440 */ ? ? ? ?
s3c_i2c0_setname("s3c2440-i2c");
? ? ? ?s3c_i2c1_setname("s3c2440-i2c");Oh, FFS. ?That's what's broken - the names that are actually appearing in the system are the names that the devices have in the relevant dev-i2c?.c files and obviously there's no reason one would expect to find code like the above since it's just a little abstruse to rewrite the devices like that.
Quoted 7 lines
Looking at the code I bet what broke it is the CPU revision identification changes - it's now responsible for making sure the fixups are run. ?Either it's not matching correctly or it's running too late to make the fixup before the device is registered.
Though the first part of the issue (the fact that I2C0 isn't using a device name and I2C1 is) remains an issue.
There are two instances of clock with name i2c registered for s3c64xx. One has a devname and the other does not. So this continued to work. The i2c0 clock instance did not get a devname because its instance id was -1 (prior to clkdev support).
If a devname is added to i2c0 clock instance for s3c64xx, it should be set to "s3c2440-i2c". If the devname is set to "s3c2440-i2c.0", then CONFIG_S3C_DEV_I2C1 will have to defined for s3c64xx, otherwise, as per arch/arm/plat-samsung/dev-i2c0.c, the platform device id for i2c0 instance will be -1 and the clock lookup for i2c0 will fail.
Regards,
Thomas.
PATCH 1/1 ARM: S3C64XX: Fix clkdev device names for I2C clocks
Quoted 4 lines · Thomas Abraham
There are two instances of clock with name i2c registered for s3c64xx. One has a devname and the other does not. So this continued to work. The i2c0 clock instance did not get a devname because its instance id was -1 (prior to clkdev support).
Not having devname will *work* - the problem is that it'll work too often and return the wrong clock whenever anything goes wrong.
Quoted 5 lines
If a devname is added to i2c0 clock instance for s3c64xx, it should be set to "s3c2440-i2c". If the devname is set to "s3c2440-i2c.0", then
CONFIG_S3C_DEV_I2C1will have to defined for s3c64xx, otherwise, as perarch/arm/plat-samsung/dev-i2c0.c, the platform device id for i2c0 instance will be -1 and the clock lookup for i2c0 will fail.
All of which is just asking for fragility; the ifdefs for renumbering the device are really unhelpful here - the system always has two I2C controllers and the numbering is going to change depending on which boards are built in so you can break things by enabling support for a second board.
Frankly I'm not convinced that all this dancing about with explicitly selecting which devices to enable is worth it; the space savings are extremely small and the complexity regularly causes issues that just shouldn't exist.