OMAP4: Clock: Correct the name of SLIMBUS interface clocks

PATCH New Jon Hunter 3 participants 7 messages Sep 16, 2011 – Oct 11, 2011

Subsystems (4)

Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP clock framework support

lore

Share & tools

lore

Jon Hunter

author

2011-09-16 17:48 UTC #0 lore raw

PATCH v2 6/6 OMAP4: Clock: Correct the name of SLIMBUS interface clocks

From: Jon Hunter <[email protected]>

Currently the interface clocks for the two SLIMBUS peripherals are named slimbus1_fck and slimbus2_fck. Rename these clocks to be slimbus1_ick and slimbus2_ick so it is clear that these are interface clocks and not functional clocks.

Signed-off-by Jon Hunter <[email protected]>

---
 arch/arm/mach-omap2/clock44xx_data.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

arch/arm/mach-omap2/clock44xx_data.c +6 -6

index eb2a345..d75ebdd 100644
@@ -2210,8 +2210,8 @@ static struct clk slimbus1_slimbus_clk = {
 	.recalc		= &followparent_recalc,
 };
 
-static struct clk slimbus1_fck = {
-	.name		= "slimbus1_fck",
+static struct clk slimbus1_ick = {
+	.name		= "slimbus1_ick",
 	.ops		= &clkops_omap2_dflt,
 	.enable_reg	= OMAP4430_CM1_ABE_SLIMBUS_CLKCTRL,
 	.enable_bit	= OMAP4430_MODULEMODE_SWCTRL,

@@ -2250,8 +2250,8 @@ static struct clk slimbus2_slimbus_clk = {
 	.recalc		= &followparent_recalc,
 };
 
-static struct clk slimbus2_fck = {
-	.name		= "slimbus2_fck",
+static struct clk slimbus2_ick = {
+	.name		= "slimbus2_ick",
 	.ops		= &clkops_omap2_dflt,
 	.enable_reg	= OMAP4430_CM_L4PER_SLIMBUS2_CLKCTRL,
 	.enable_bit	= OMAP4430_MODULEMODE_SWCTRL,

@@ -3272,11 +3272,11 @@ static struct omap_clk omap44xx_clks[] = {
 	CLK(NULL,	"slimbus1_fclk_0",		&slimbus1_fclk_0,	CK_443X),
 	CLK(NULL,	"slimbus1_fclk_2",		&slimbus1_fclk_2,	CK_443X),
 	CLK(NULL,	"slimbus1_slimbus_clk",		&slimbus1_slimbus_clk,	CK_443X),
-	CLK(NULL,	"slimbus1_fck",			&slimbus1_fck,	CK_443X),
+	CLK(NULL,	"slimbus1_ick",			&slimbus1_ick,	CK_443X),
 	CLK(NULL,	"slimbus2_fclk_1",		&slimbus2_fclk_1,	CK_443X),
 	CLK(NULL,	"slimbus2_fclk_0",		&slimbus2_fclk_0,	CK_443X),
 	CLK(NULL,	"slimbus2_slimbus_clk",		&slimbus2_slimbus_clk,	CK_443X),
-	CLK(NULL,	"slimbus2_fck",			&slimbus2_fck,	CK_443X),
+	CLK(NULL,	"slimbus2_ick",			&slimbus2_ick,	CK_443X),
 	CLK(NULL,	"smartreflex_core_fck",		&smartreflex_core_fck,	CK_443X),
 	CLK(NULL,	"smartreflex_iva_fck",		&smartreflex_iva_fck,	CK_443X),
 	CLK(NULL,	"smartreflex_mpu_fck",		&smartreflex_mpu_fck,	CK_443X),

Signature · 1 line

1.7.4.1

Paul Walmsley

maintainer

2011-10-07 01:42 UTC #1 lore raw

PATCH v2 6/6 OMAP4: Clock: Correct the name of SLIMBUS interface clocks

+ Beno?t

Quoted 8 lines · Jon Hunter

From: Jon Hunter <[email protected]>
Currently the interface clocks for the two SLIMBUS peripherals are named slimbus1_fck and slimbus2_fck. Rename these clocks to be slimbus1_ick and slimbus2_ick so it is clear that these are interface clocks and not functional clocks.
Signed-off-by Jon Hunter <[email protected]>

This one, I don't quite understand. We should probably be removing these MODULEMODE-only clocks from the OMAP4 tree, and using their parent clock as the main_clk. That would be a good cleanup for 3.3...

- Paul

Cousson, Benoit

2011-10-07 08:23 UTC #2 lore raw

PATCH v2 6/6 OMAP4: Clock: Correct the name of SLIMBUS interface clocks

Hi Paul & Jon,

Quoted 11 lines · Paul Walmsley

+ Beno?t
Quoted 7 lines · Jon Hunter
From: Jon Hunter<[email protected]>
Currently the interface clocks for the two SLIMBUS peripherals are named slimbus1_fck and slimbus2_fck. Rename these clocks to be slimbus1_ick and slimbus2_ick so it is clear that these are interface clocks and not functional clocks.

Quoted 5 lines

Quoted 1 line
Signed-off-by Jon Hunter<[email protected]>
This one, I don't quite understand. We should probably be removing these MODULEMODE-only clocks from the OMAP4 tree, and using their parent clock as the main_clk. That would be a good cleanup for 3.3...

Yes, but in order to remove that from the clock data we must ensure that the hwmod entry is there.
I kept a lot of legacy MODULEMODE clocks just because of missing hwmod / runtime_pm adaptation on some drivers.

In the case of slimbus, there is no main_clk but a bunch of optional clocks. It looks similar to the DSS case. So we should not use the parent clock as a main_clk.

We should probably promote one of the opt_clk as the main_clk. The slimbus_clk seems to be the good candidate for both instances.

static struct omap_hwmod_opt_clk slimbus1_opt_clks[] = {
        { .role = "fclk_1", .clk = "slimbus1_fclk_1" },
        { .role = "fclk_0", .clk = "slimbus1_fclk_0" },
        { .role = "fclk_2", .clk = "slimbus1_fclk_2" },
        { .role = "slimbus_clk", .clk = "slimbus1_slimbus_clk" },
};

static struct omap_hwmod_opt_clk slimbus2_opt_clks[] = {
        { .role = "fclk_1", .clk = "slimbus2_fclk_1" },
        { .role = "fclk_0", .clk = "slimbus2_fclk_0" },
        { .role = "slimbus_clk", .clk = "slimbus2_slimbus_clk" },
};

Jon,
Do you know if that one is indeed mandatory to use the slimbus IP?

Thanks,
Benoit

Jon Hunter

author

2011-10-07 22:46 UTC #3 lore raw

PATCH v2 6/6 OMAP4: Clock: Correct the name of SLIMBUS interface clocks

Hi Benoit,

Quoted 39 lines · Benoit

Hi Paul & Jon,
Quoted 11 lines · Paul Walmsley
+ Beno?t
Quoted 7 lines · Jon Hunter
From: Jon Hunter<[email protected]>
Currently the interface clocks for the two SLIMBUS peripherals are named slimbus1_fck and slimbus2_fck. Rename these clocks to be slimbus1_ick and slimbus2_ick so it is clear that these are interface clocks and not functional clocks.
Quoted 5 lines
Quoted 1 line
Signed-off-by Jon Hunter<[email protected]>
This one, I don't quite understand. We should probably be removing these MODULEMODE-only clocks from the OMAP4 tree, and using their parent clock as the main_clk. That would be a good cleanup for 3.3...
Yes, but in order to remove that from the clock data we must ensure that the hwmod entry is there.
I kept a lot of legacy MODULEMODE clocks just because of missing hwmod / runtime_pm adaptation on some drivers.
In the case of slimbus, there is no main_clk but a bunch of optional clocks. It looks similar to the DSS case. So we should not use the parent clock as a main_clk.
We should probably promote one of the opt_clk as the main_clk. The slimbus_clk seems to be the good candidate for both instances.
static struct omap_hwmod_opt_clk slimbus1_opt_clks[] = { { .role = "fclk_1", .clk = "slimbus1_fclk_1" }, { .role = "fclk_0", .clk = "slimbus1_fclk_0" }, { .role = "fclk_2", .clk = "slimbus1_fclk_2" }, { .role = "slimbus_clk", .clk = "slimbus1_slimbus_clk" }, };

Quoted 8 lines

static struct omap_hwmod_opt_clk slimbus2_opt_clks[] = { { .role = "fclk_1", .clk = "slimbus2_fclk_1" }, { .role = "fclk_0", .clk = "slimbus2_fclk_0" }, { .role = "slimbus_clk", .clk = "slimbus2_slimbus_clk" }, };
Jon,
Do you know if that one is indeed mandatory to use the slimbus IP?

Sorry, are you asking about the clocks I was re-naming or the slimbus_clk you are referring to above?

Looking at the clock tree tool, the slimbus_clk is the actual external slimbus clock that can be generated by OMAP or by an external device.

The TRM states ...

"Most of the SLIMbus module runs off two main clocks: the L4 interface clock for the data input and output registers, and the control and status control registers; and the SLIMbus clock, taken from the serial interface (CLK line) for the SLIMbus-side logic.

The SLIMbus controller operates as clock source component (active framer), which drives the SLIMbus clock line CLK, or as clock receiver component, which gets its clock from the same CLK line."

So, if you are operating as the clock source component on the bus then one of the optional clocks to drive the peripheral logic and if you are the clock receiver then you use slimbus_clk.

Cheers
Jon

Cousson, Benoit

2011-10-10 09:54 UTC #4 lore raw

PATCH v2 6/6 OMAP4: Clock: Correct the name of SLIMBUS interface clocks

Hi Jon,

Quoted 48 lines · Jon

Hi Benoit,
Quoted 45 lines · Benoit
Hi Paul& Jon,
Quoted 11 lines · Paul Walmsley
+ Beno?t
Quoted 7 lines · Jon Hunter
From: Jon Hunter<[email protected]>
Currently the interface clocks for the two SLIMBUS peripherals are named slimbus1_fck and slimbus2_fck. Rename these clocks to be slimbus1_ick and slimbus2_ick so it is clear that these are interface clocks and not functional clocks.
Quoted 5 lines
Quoted 1 line
Signed-off-by Jon Hunter<[email protected]>
This one, I don't quite understand. We should probably be removing these MODULEMODE-only clocks from the OMAP4 tree, and using their parent clock as the main_clk. That would be a good cleanup for 3.3...
Yes, but in order to remove that from the clock data we must ensure that the hwmod entry is there.
I kept a lot of legacy MODULEMODE clocks just because of missing hwmod / runtime_pm adaptation on some drivers.
In the case of slimbus, there is no main_clk but a bunch of optional clocks. It looks similar to the DSS case. So we should not use the parent clock as a main_clk.
We should probably promote one of the opt_clk as the main_clk. The slimbus_clk seems to be the good candidate for both instances.
static struct omap_hwmod_opt_clk slimbus1_opt_clks[] = { { .role = "fclk_1", .clk = "slimbus1_fclk_1" }, { .role = "fclk_0", .clk = "slimbus1_fclk_0" }, { .role = "fclk_2", .clk = "slimbus1_fclk_2" }, { .role = "slimbus_clk", .clk = "slimbus1_slimbus_clk" }, };
static struct omap_hwmod_opt_clk slimbus2_opt_clks[] = { { .role = "fclk_1", .clk = "slimbus2_fclk_1" }, { .role = "fclk_0", .clk = "slimbus2_fclk_0" }, { .role = "slimbus_clk", .clk = "slimbus2_slimbus_clk" }, };

Quoted 5 lines

Quoted 2 lines
Jon,
Do you know if that one is indeed mandatory to use the slimbus IP?
Sorry, are you asking about the clocks I was re-naming or the slimbus_clk you are referring to above?

The clocks you are renaming should not exist at all, so I was referring to the real clocks that are all listed as optional clocks.

Usually we do need to have a main_clk in order to access the IP registers (at least the sysconfig). But for some IPs, the iclk can be enough.
If the interface clock is enough, then potentially that main clock is not mandatory. But if one functional clock is mandatory, then we have to figured out which one from the various optional functional clocks can be used as the main_clk.

Quoted 10 lines

Looking at the clock tree tool, the slimbus_clk is the actual external slimbus clock that can be generated by OMAP or by an external device.
The TRM states ...
"Most of the SLIMbus module runs off two main clocks: the L4 interface clock for the data input and output registers, and the control and status control registers; and the SLIMbus clock, taken from the serial interface (CLK line) for the SLIMbus-side logic.

Quoted 7 lines

The SLIMbus controller operates as clock source component (active framer), which drives the SLIMbus clock line CLK, or as clock receiver component, which gets its clock from the same CLK line."
So, if you are operating as the clock source component on the bus then one of the optional clocks to drive the peripheral logic and if you are the clock receiver then you use slimbus_clk.

OK, so clearly, the slimbus_clk cannot be a main_clk. We have to check if the registers can be accessed only with the iclk.

Regards,
Benoit

Jon Hunter

author

2011-10-10 21:34 UTC #5 lore raw

PATCH v2 6/6 OMAP4: Clock: Correct the name of SLIMBUS interface clocks

Hi Benoit,

Quoted 53 lines · Benoit

Hi Jon,
Quoted 50 lines · Jon
Hi Benoit,
Quoted 47 lines · Benoit
Hi Paul& Jon,
Quoted 11 lines · Paul Walmsley
+ Beno?t
Quoted 7 lines · Jon Hunter
From: Jon Hunter<[email protected]>
Currently the interface clocks for the two SLIMBUS peripherals are named slimbus1_fck and slimbus2_fck. Rename these clocks to be slimbus1_ick and slimbus2_ick so it is clear that these are interface clocks and not functional clocks.
Quoted 7 lines
Quoted 1 line
Signed-off-by Jon Hunter<[email protected]>
This one, I don't quite understand. We should probably be removing these
MODULEMODE-only clocks from the OMAP4 tree, and using their parent clock
as the main_clk. That would be a good cleanup for 3.3...
Yes, but in order to remove that from the clock data we must ensure that the hwmod entry is there.
I kept a lot of legacy MODULEMODE clocks just because of missing hwmod / runtime_pm adaptation on some drivers.
In the case of slimbus, there is no main_clk but a bunch of optional clocks. It looks similar to the DSS case. So we should not use the parent clock as a main_clk.
We should probably promote one of the opt_clk as the main_clk. The slimbus_clk seems to be the good candidate for both instances.
static struct omap_hwmod_opt_clk slimbus1_opt_clks[] = { { .role = "fclk_1", .clk = "slimbus1_fclk_1" }, { .role = "fclk_0", .clk = "slimbus1_fclk_0" }, { .role = "fclk_2", .clk = "slimbus1_fclk_2" }, { .role = "slimbus_clk", .clk = "slimbus1_slimbus_clk" }, };
static struct omap_hwmod_opt_clk slimbus2_opt_clks[] = { { .role = "fclk_1", .clk = "slimbus2_fclk_1" }, { .role = "fclk_0", .clk = "slimbus2_fclk_0" }, { .role = "slimbus_clk", .clk = "slimbus2_slimbus_clk" }, };

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
Jon,
Do you know if that one is indeed mandatory to use the slimbus IP?
Sorry, are you asking about the clocks I was re-naming or the slimbus_clk you are referring to above?
The clocks you are renaming should not exist at all, so I was referring to the real clocks that are all listed as optional clocks.

These clocks are the interface clocks enabled via the module-mode. So you are saying you want to remove the interface clocks from the list of clocks?

Quoted 22 lines

Usually we do need to have a main_clk in order to access the IP registers (at least the sysconfig). But for some IPs, the iclk can be enough.
If the interface clock is enough, then potentially that main clock is not mandatory. But if one functional clock is mandatory, then we have to figured out which one from the various optional functional clocks can be used as the main_clk.
Quoted 14 lines
Looking at the clock tree tool, the slimbus_clk is the actual external slimbus clock that can be generated by OMAP or by an external device.
The TRM states ...
"Most of the SLIMbus module runs off two main clocks: the L4 interface clock for the data input and output registers, and the control and status control registers; and the SLIMbus clock, taken from the serial interface (CLK line) for the SLIMbus-side logic.
The SLIMbus controller operates as clock source component (active framer), which drives the SLIMbus clock line CLK, or as clock receiver component, which gets its clock from the same CLK line."

Quoted 6 lines

Quoted 3 lines
So, if you are operating as the clock source component on the bus then one of the optional clocks to drive the peripheral logic and if you are the clock receiver then you use slimbus_clk.
OK, so clearly, the slimbus_clk cannot be a main_clk. We have to check if the registers can be accessed only with the iclk.

I ran a quick test. If I just set the module mode to 0x2 (enabled), then I can access the registers. So no need to turn on any of the optional/functional clocks just the iclk.

Cheers
Jon

Cousson, Benoit

2011-10-11 11:30 UTC #6 lore raw

PATCH v2 6/6 OMAP4: Clock: Correct the name of SLIMBUS interface clocks

Quoted 62 lines · Jon

Hi Benoit,
Quoted 59 lines · Benoit
Hi Jon,
Quoted 55 lines · Jon
Hi Benoit,
Quoted 41 lines · Benoit
Hi Paul& Jon,
Quoted 11 lines · Paul Walmsley
+ Beno?t
Quoted 7 lines · Jon Hunter
From: Jon Hunter<[email protected]>
Currently the interface clocks for the two SLIMBUS peripherals are named slimbus1_fck and slimbus2_fck. Rename these clocks to be slimbus1_ick and slimbus2_ick so it is clear that these are interface clocks and not functional clocks.
Quoted 7 lines
Quoted 1 line
Signed-off-by Jon Hunter<[email protected]>
This one, I don't quite understand. We should probably be removing these
MODULEMODE-only clocks from the OMAP4 tree, and using their parent clock
as the main_clk. That would be a good cleanup for 3.3...
Yes, but in order to remove that from the clock data we must ensure that the hwmod entry is there.
I kept a lot of legacy MODULEMODE clocks just because of missing hwmod / runtime_pm adaptation on some drivers.
In the case of slimbus, there is no main_clk but a bunch of optional clocks. It looks similar to the DSS case. So we should not use the parent clock as a main_clk.
We should probably promote one of the opt_clk as the main_clk. The slimbus_clk seems to be the good candidate for both instances.
static struct omap_hwmod_opt_clk slimbus1_opt_clks[] = { { .role = "fclk_1", .clk = "slimbus1_fclk_1" }, { .role = "fclk_0", .clk = "slimbus1_fclk_0" }, { .role = "fclk_2", .clk = "slimbus1_fclk_2" }, { .role = "slimbus_clk", .clk = "slimbus1_slimbus_clk" }, };
Quoted 8 lines
static struct omap_hwmod_opt_clk slimbus2_opt_clks[] = { { .role = "fclk_1", .clk = "slimbus2_fclk_1" }, { .role = "fclk_0", .clk = "slimbus2_fclk_0" }, { .role = "slimbus_clk", .clk = "slimbus2_slimbus_clk" }, };
Jon,
Do you know if that one is indeed mandatory to use the slimbus IP?
Sorry, are you asking about the clocks I was re-naming or the slimbus_clk you are referring to above?

Quoted 6 lines

Quoted 2 lines
The clocks you are renaming should not exist at all, so I was referring to the real clocks that are all listed as optional clocks.
These clocks are the interface clocks enabled via the module-mode. So you are saying you want to remove the interface clocks from the list of clocks?

Yes or No, dependeing of which clock you are talking about.
The real interface clock is already handled thanks to the ocp_if structure.

/* l4_abe -> slimbus1 */
static struct omap_hwmod_ocp_if omap44xx_l4_abe__slimbus1 = {

       .master         = &omap44xx_l4_abe_hwmod,
       .slave          = &omap44xx_slimbus1_hwmod,
       .clk            = "ocp_abe_iclk",
       .addr           = omap44xx_slimbus1_addrs,
       .user           = OCP_USER_MPU,

};

And since the modulemodule is now handled by the hwmod core thanks to that:

       .prcm = {
               .omap4 = {
                       .clkctrl_offs = OMAP4_CM1_ABE_SLIMBUS_CLKCTRL_OFFSET,
                       .context_offs = OMAP4_RM_ABE_SLIMBUS_CONTEXT_OFFSET,
                       .modulemode   = MODULEMODE_SWCTRL,
               },
       },

There is no need to create a fake leaf clock to handle the module using the clock fmwk.

So we can now remove these two clock nodes from the clock44xx_data.c file:

static struct clk slimbus1_fck = {
	.name		= "slimbus1_fck",
	.ops		= &clkops_omap2_dflt,
	.enable_reg	= OMAP4430_CM1_ABE_SLIMBUS_CLKCTRL,
	.enable_bit	= OMAP4430_MODULEMODE_SWCTRL,
	.clkdm_name	= "abe_clkdm",
	.parent		= &ocp_abe_iclk,
	.recalc		= &followparent_recalc,
};

static struct clk slimbus2_fck = {
	.name		= "slimbus2_fck",
	.ops		= &clkops_omap2_dflt,
	.enable_reg	= OMAP4430_CM_L4PER_SLIMBUS2_CLKCTRL,
	.enable_bit	= OMAP4430_MODULEMODE_SWCTRL,
	.clkdm_name	= "l4_per_clkdm",
	.parent		= &l4_div_ck,
	.recalc		= &followparent_recalc,
};

Quoted 26 lines

Quoted 26 lines
Usually we do need to have a main_clk in order to access the IP registers (at least the sysconfig). But for some IPs, the iclk can be enough.
If the interface clock is enough, then potentially that main clock is not mandatory. But if one functional clock is mandatory, then we have to figured out which one from the various optional functional clocks can be used as the main_clk.
Quoted 17 lines
Looking at the clock tree tool, the slimbus_clk is the actual external slimbus clock that can be generated by OMAP or by an external device.
The TRM states ...
"Most of the SLIMbus module runs off two main clocks: the L4 interface clock for the data input and output registers, and the control and status control registers; and the SLIMbus clock, taken from the serial interface (CLK line) for the SLIMbus-side logic.
The SLIMbus controller operates as clock source component (active framer), which drives the SLIMbus clock line CLK, or as clock receiver component, which gets its clock from the same CLK line."
So, if you are operating as the clock source component on the bus then one of the optional clocks to drive the peripheral logic and if you are the clock receiver then you use slimbus_clk.

Quoted 6 lines

Quoted 2 lines
OK, so clearly, the slimbus_clk cannot be a main_clk. We have to check if the registers can be accessed only with the iclk.
I ran a quick test. If I just set the module mode to 0x2 (enabled), then I can access the registers. So no need to turn on any of the optional/functional clocks just the iclk.

That's good, so it looks like you will not have to populate the main_clk at all.

Regards,
Benoit