Re: [PATCH 1/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap4
Hello Ming, On Wed, 28 Sep 2011, Ming Lei wrote: On Fri, Sep 23, 2011 at 7:31 AM, Paul Walmsley p...@pwsan.com wrote: The idea of the main_clk was not intended to be PRCM or OCP or even OMAP-specific. It's just intended to represent a clock that is used to drive the register logic inside the IP block. Therefore it must be enabled before any register access may occur. Even if clock gating is handled by some higher-level interface (e.g., at the IP block level), the main_clk has a rate, so it also implies an upper limit on how quickly register operations can occur. I suppose that all of the IP block's clocks could be optional clocks, but we know that every IP block with registers requires at least one clock to work, and that should be the main_clk. I am a bit confused about you comment on main_clk. From hwmod related source code, main_clk is the function clock of one module(hwmod), such as: on omap4, for uart3, main_clk is uart3_fck. But from[1] and [2] of omap4 PRM, we can find that interface clock is required to provide register access instead of function clock. As far as I know, the two cases that you cite are basically documentation bugs in the TRM. In my experience, for IP blocks on OMAPs, a functional clock has to be enabled in order for register accesses to succeed. It's possible there may be a few odd IP blocks that behave differently, but I can't think of any off the top of my head. There are three cases that I've come across that might be interesting to you. The first involves IP blocks that use their interface clock as their functional clock, such as the MAILBOXES IP block. In this case, there is no separate functional clock that is needed for register accesses to complete, and therefore the main_clk should be the interface clock. The second case involves IP blocks that can receive functional clocks from an off-chip source (i.e., not via a PRCM-controlled clock), such as the McBSP IP block. For these IP blocks, it could be that register accesses might still succeed even if the module's PRCM-provided functional clock is off. I haven't tested this personally. The third case involves IP blocks with multiple functional clocks, such as the OMAP3 USBHOST subsystem. What we found on OMAP3 was that only one of these two clocks needs to be enabled for register accesses to succeed. Some functional clocks might control parts of an IP block that have nothing to do with register access. This is a bit conflictive with what you description, so could you give a further comments about main_clk, function clock and interface clock? I don't know why there are these documentation discrepancies. I can guess, but probably I'd better not :-) Hope the above helped. [1], 23.3.4.2 Clock Configuration Each UART uses a 48-MHz functional clock for its logic and to generate external interface signals. Each UART uses an interface clock for register accesses. [2], 3.1.1.1.1 Module Interface and Functional Clocks The interface clocks have the following characteristics: • They ensure proper communication between any module/subsystem and the interconnect. • In most cases, they supply the system interconnect interface and registers of the module. regards, - Paul
Re: [PATCH 1/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap4
Hi Paul, On Fri, Sep 23, 2011 at 7:31 AM, Paul Walmsley p...@pwsan.com wrote: The idea of the main_clk was not intended to be PRCM or OCP or even OMAP-specific. It's just intended to represent a clock that is used to drive the register logic inside the IP block. Therefore it must be enabled before any register access may occur. Even if clock gating is handled by some higher-level interface (e.g., at the IP block level), the main_clk has a rate, so it also implies an upper limit on how quickly register operations can occur. I suppose that all of the IP block's clocks could be optional clocks, but we know that every IP block with registers requires at least one clock to work, and that should be the main_clk. I am a bit confused about you comment on main_clk. From hwmod related source code, main_clk is the function clock of one module(hwmod), such as: on omap4, for uart3, main_clk is uart3_fck. But from[1] and [2] of omap4 PRM, we can find that interface clock is required to provide register access instead of function clock. This is a bit conflictive with what you description, so could you give a further comments about main_clk, function clock and interface clock? [1], 23.3.4.2 Clock Configuration Each UART uses a 48-MHz functional clock for its logic and to generate external interface signals. Each UART uses an interface clock for register accesses. [2], 3.1.1.1.1 Module Interface and Functional Clocks The interface clocks have the following characteristics: • They ensure proper communication between any module/subsystem and the interconnect. • In most cases, they supply the system interconnect interface and registers of the module. thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap4
Hi Paul, On 9/23/2011 1:31 AM, Paul Walmsley wrote: Hi On Thu, 22 Sep 2011, Cousson, Benoit wrote: ADDR_TYPE_RT is mainly use for sysconfig access inside hwmod core today, so why should we use it in this case? Just for consistency, since the flag isn't defined to be SYSCONFIG-specific. That way, if there's another need -- other than SYSCONFIG access -- to identify the chunk of address space that's used for register access, we won't have to dig through and possibly repatch the hwmod data, just because some people didn't want to follow the rule. If the flag was simply defined to be SYSCONFIG-specific, then you're right, it wouldn't be needed. To be honest, I've been confused with that flag for some time :-) * ADDR_TYPE_RT: Address space contains module register target data. Maybe that's my English, but what does register target data mean exactly? The name comes from the L3 section of the 34xx TRM - see for example section 9.1.1 Terminology of the rev ZR TRM. The L3 has several address space sections, and whoever wrote that text -- Sonics? -- called the one with the L3's own internal registers the register target. And I was looking for a name that I did not have to make up, so I personally wouldn't have to defend the name ;-) OK, so the registers target is not even the IP itself but the 4k memory space before every L3/L4 IPs? What was the intent? Providing a way to identify some register vs memory space? As you suggest, the original impetus for the flag was to identify which chunk of address space needs to be mapped by the hwmod code for SYSCONFIG accesses to work. On current OMAPs, this seems to be the same thing as identifying the IP block's primary register area for every module with SYS* and REVISION registers. And I probably thought at the time that specifying the IP block's main register mapping seemed more useful and generally applicable than designating where the SYSCONFIG register was. Hence the current definition. I think we are lucky that the sysconfig is always in the first memory region in the case of multiple address spaces like for the usb_host. This is clearly something we will have to enforce in the future, especially in the strongly order DT world :-) Regarding main_clk, I do not think that some internal IPs like ohci/ehci should have a main_clk, since this is not visible at that level. Do you not agree that every IP block that contains sequential logic on current and foreseeable future SoCs must have some clock signal to drive that logic? Yes I do. My point was that some time we cannot necessarily identify this clock or this clock is not relevant due to the container around it. The idea of the main_clk was not intended to be PRCM or OCP or even OMAP-specific. It's just intended to represent a clock that is used to drive the register logic inside the IP block. Therefore it must be enabled before any register access may occur. Even if clock gating is handled by some higher-level interface (e.g., at the IP block level), the main_clk has a rate, so it also implies an upper limit on how quickly register operations can occur. I suppose that all of the IP block's clocks could be optional clocks, but we know that every IP block with registers requires at least one clock to work, and that should be the main_clk. OK, I'm too PRCM centric... A clock that is not clearly represented by the PRCM should not necessarily exist in the current hwmod representation for my PRCM centric point of view :-). And in this case, enabling the clock might not be enough since this is the modulemode that does matter in OMAP4. That's why I'd rather not put any clock instead of a clock that will anyway not be enough to do anything useful due to the dependency with the container. Exposing too many fake nodes might lead to false interpretation and some increase of the apparent complexity wrt the real HW capacity. Bottom-line, I'm not opposed to add such information, but I'd rather populate the real HW information available and not extrapolate too much if this does not bring some real advantage. Regards, Benoit -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap4
Hi On Fri, 23 Sep 2011, Munegowda, Keshava wrote: Paul Walmsley p...@pwsan.com wrote: So I'd suggest one of two approaches: 1. If the pin muxing can follow the PM runtime status of the UHH IP block, then the pin mux data should be associated with the UHH hwmod. No, the mux is applicable only to ehci and ohci , where as sysconfig is applicable to uhh ( usb_host_hs class). My point is that, as far as I can tell, I/O pad wakeup (caused by USB remote wakeup) is only going to matter when the entire UHH IP block has its clocks cut. Otherwise, while the UHH is clocked, the EHCI and/or OHCI IP blocks will also be clocked, so no I/O pad wakeup will be needed. Do you agree? 2. If the pin muxing must follow the EHCI/OHCI IP block PM runtime status, then drivers/mfd/omap-usb-host.c is what should be handling the remuxing. omap-usb-host.c can set the dev_pm_ops of the EHCI/OHCI platform_devices to point to functions either in arch/arm/mach-omap2/usb-host.c, or local functions that call into mach-omap2/usb-host.c functions to handle pin remuxing. (Those function pointers should be provided to the MFD driver in some clean way, like via platform_data.) The dev_pm_ops of ehci should exist in /drivers/usb/host/ehci-omap.c and dev_pm_ops of phci should exist in /drivers/usb/host/ohci-omap3.c. In the existing design, the omap-usb-host.c host can not access the platform driver of ehci and ohci. But, through function pointers it is possible to send the platform data to ehci and ohci drivers to handle pin remuxing; but we will not able to use tero's patches; and it will prevent our future design activity for remote wakeup of ehci and ohci. Could you please explain in more detail why the dynamic remuxing can't be done when the UHH enters or exits idle? - Paul
Re: [PATCH 1/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap4
So I'd suggest one of two approaches: 1. If the pin muxing can follow the PM runtime status of the UHH IP block, then the pin mux data should be associated with the UHH hwmod. No, the mux is applicable only to ehci and ohci , where as sysconfig is applicable to uhh ( usb_host_hs class). 2. If the pin muxing must follow the EHCI/OHCI IP block PM runtime status, then drivers/mfd/omap-usb-host.c is what should be handling the remuxing. omap-usb-host.c can set the dev_pm_ops of the EHCI/OHCI platform_devices to point to functions either in arch/arm/mach-omap2/usb-host.c, or local functions that call into mach-omap2/usb-host.c functions to handle pin remuxing. (Those function pointers should be provided to the MFD driver in some clean way, like via platform_data.) The dev_pm_ops of ehci should exist in /drivers/usb/host/ehci-omap.c and dev_pm_ops of phci should exist in /drivers/usb/host/ohci-omap3.c. In the existing design, the omap-usb-host.c host can not access the platform driver of ehci and ohci. But, through function pointers it is possible to send the platform data to ehci and ohci drivers to handle pin remuxing; but we will not able to use tero's patches; and it will prevent our future design activity for remote wakeup of ehci and ohci. please let me know if you have thoughts on this. Regards Keshava -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap4
From: Benoit Cousson b-cous...@ti.com Following 4 hwmod structures are added 1. usb_host_hs hwmod of usbhs with uhh base address and functional clock, 2. usb_ehci_hs hwmod with irq and base address, 3. usb_ohci_hs hwmod with irq and base address, - The ehci and ohci hwmods does not require functional clock because usb_host_hs has functional clock which is sufficient to access ehci and ohci address space. - The usb_ehci_hs and usb_ohci_hs should be two separate hwmods which should not be combined. This is needed because ehci and ohci will have separate dedicated ports in omap4 there is a clock per port.We should be able to configure the IO-Wakeup capability of pins specific to EHCI OHCI separately and depending on the I/O wakeup event the only port clocks corresponding to the wakeup source will be enabled internally by the usb host driver. 4. usb_tll_hs hwmod of usbhs with the TLL base address and irq. Signed-off-by: Benoit Cousson b-cous...@ti.com - The initial version had only two hwmods,usb_host_hs and usb_tll_hs. These two hwmods are reorganized as above four hwmods, because there will be dedicated ports for ehci and ohci; and each port will have I/O mux, which will be initialized per hwmod in future. - migrated these hwmod structures to Kevin's pm branch: git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git . Signed-off-by: Keshava Munegowda keshava_mgo...@ti.com Reviewed-by: Partha Basak part...@india.ti.com --- arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 250 +++- 1 files changed, 249 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c index 6201422..5e8a640 100644 --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c @@ -68,6 +68,10 @@ static struct omap_hwmod omap44xx_mmc2_hwmod; static struct omap_hwmod omap44xx_mpu_hwmod; static struct omap_hwmod omap44xx_mpu_private_hwmod; static struct omap_hwmod omap44xx_usb_otg_hs_hwmod; +static struct omap_hwmod omap44xx_usb_host_hs_hwmod; +static struct omap_hwmod omap44xx_usbhs_ohci_hwmod; +static struct omap_hwmod omap44xx_usbhs_ehci_hwmod; +static struct omap_hwmod omap44xx_usb_tll_hs_hwmod; /* * Interconnects omap_hwmod structures @@ -5336,6 +5340,245 @@ static struct omap_hwmod omap44xx_wd_timer3_hwmod = { .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430), }; +/* + * 'usb_host_hs' class + * high-speed multi-port usb host controller + */ +static struct omap_hwmod_ocp_if omap44xx_usb_host_hs__l3_main_2 = { + .master = omap44xx_usb_host_hs_hwmod, + .slave = omap44xx_l3_main_2_hwmod, + .clk= l3_div_ck, + .user = OCP_USER_MPU | OCP_USER_SDMA, +}; + +static struct omap_hwmod_class_sysconfig omap44xx_usb_host_hs_sysc = { + .rev_offs = 0x, + .sysc_offs = 0x0010, + .syss_offs = 0x0014, + .sysc_flags = (SYSC_HAS_MIDLEMODE | SYSC_HAS_SIDLEMODE), + .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART | + SIDLE_SMART_WKUP | MSTANDBY_FORCE | MSTANDBY_NO | + MSTANDBY_SMART | MSTANDBY_SMART_WKUP), + .sysc_fields= omap_hwmod_sysc_type2, +}; + +static struct omap_hwmod_class omap44xx_usb_host_hs_hwmod_class = { + .name = usb_host_hs, + .sysc = omap44xx_usb_host_hs_sysc, +}; + +static struct omap_hwmod_ocp_if *omap44xx_usb_host_hs_masters[] = { + omap44xx_usb_host_hs__l3_main_2, +}; + +static struct omap_hwmod_addr_space omap44xx_usb_host_hs_addrs[] = { + { + .name = uhh, + .pa_start = 0x4a064000, + .pa_end = 0x4a0647ff, + .flags = ADDR_TYPE_RT + }, + {} +}; + +static struct omap_hwmod_ocp_if omap44xx_l4_cfg__usb_host_hs = { + .master = omap44xx_l4_cfg_hwmod, + .slave = omap44xx_usb_host_hs_hwmod, + .clk= l4_div_ck, + .addr = omap44xx_usb_host_hs_addrs, + .user = OCP_USER_MPU | OCP_USER_SDMA, +}; + +static struct omap_hwmod_ocp_if *omap44xx_usb_host_hs_slaves[] = { + omap44xx_l4_cfg__usb_host_hs, +}; + +static struct omap_hwmod omap44xx_usb_host_hs_hwmod = { + .name = usb_host_hs, + .class = omap44xx_usb_host_hs_hwmod_class, + .clkdm_name = l3_init_clkdm, + .main_clk = usb_host_hs_fck, + .prcm = { + .omap4 = { + .clkctrl_offs = OMAP4_CM_L3INIT_USB_HOST_CLKCTRL_OFFSET, + .context_offs = OMAP4_RM_L3INIT_USB_HOST_CONTEXT_OFFSET, + .modulemode = MODULEMODE_SWCTRL, + }, + }, + .slaves = omap44xx_usb_host_hs_slaves, + .slaves_cnt =
[PATCH 1/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap4
From: Benoit Cousson b-cous...@ti.com Following 4 hwmod structures are added 1. usb_host_hs hwmod of usbhs with uhh base address and functional clock, 2. usb_ehci_hs hwmod with irq and base address, 3. usb_ohci_hs hwmod with irq and base address, - The ehci and ohci hwmods does not require functional clock because usb_host_hs has functional clock which is sufficient to access ehci and ohci address space. - The usb_ehci_hs and usb_ohci_hs should be two separate hwmods which should not be combined. This is needed because ehci and ohci will have separate dedicated ports in omap4 there is a clock per port.We should be able to configure the IO-Wakeup capability of pins specific to EHCI OHCI separately and depending on the I/O wakeup event the only port clocks corresponding to the wakeup source will be enabled internally by the usb host driver. 4. usb_tll_hs hwmod of usbhs with the TLL base address and irq. Signed-off-by: Benoit Cousson b-cous...@ti.com - The initial version had only two hwmods,usb_host_hs and usb_tll_hs. These two hwmods are reorganized as above four hwmods, because there will be dedicated ports for ehci and ohci; and each port will have I/O mux, which will be initialized per hwmod in future. - migrated these hwmod structures to Kevin's pm branch: git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git . Signed-off-by: Keshava Munegowda keshava_mgo...@ti.com Reviewed-by: Partha Basak part...@india.ti.com --- arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 250 +++- 1 files changed, 249 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c index 6201422..5e8a640 100644 --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c @@ -68,6 +68,10 @@ static struct omap_hwmod omap44xx_mmc2_hwmod; static struct omap_hwmod omap44xx_mpu_hwmod; static struct omap_hwmod omap44xx_mpu_private_hwmod; static struct omap_hwmod omap44xx_usb_otg_hs_hwmod; +static struct omap_hwmod omap44xx_usb_host_hs_hwmod; +static struct omap_hwmod omap44xx_usbhs_ohci_hwmod; +static struct omap_hwmod omap44xx_usbhs_ehci_hwmod; +static struct omap_hwmod omap44xx_usb_tll_hs_hwmod; /* * Interconnects omap_hwmod structures @@ -5336,6 +5340,245 @@ static struct omap_hwmod omap44xx_wd_timer3_hwmod = { .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430), }; +/* + * 'usb_host_hs' class + * high-speed multi-port usb host controller + */ +static struct omap_hwmod_ocp_if omap44xx_usb_host_hs__l3_main_2 = { + .master = omap44xx_usb_host_hs_hwmod, + .slave = omap44xx_l3_main_2_hwmod, + .clk= l3_div_ck, + .user = OCP_USER_MPU | OCP_USER_SDMA, +}; + +static struct omap_hwmod_class_sysconfig omap44xx_usb_host_hs_sysc = { + .rev_offs = 0x, + .sysc_offs = 0x0010, + .syss_offs = 0x0014, + .sysc_flags = (SYSC_HAS_MIDLEMODE | SYSC_HAS_SIDLEMODE), + .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART | + SIDLE_SMART_WKUP | MSTANDBY_FORCE | MSTANDBY_NO | + MSTANDBY_SMART | MSTANDBY_SMART_WKUP), + .sysc_fields= omap_hwmod_sysc_type2, +}; + +static struct omap_hwmod_class omap44xx_usb_host_hs_hwmod_class = { + .name = usb_host_hs, + .sysc = omap44xx_usb_host_hs_sysc, +}; + +static struct omap_hwmod_ocp_if *omap44xx_usb_host_hs_masters[] = { + omap44xx_usb_host_hs__l3_main_2, +}; + +static struct omap_hwmod_addr_space omap44xx_usb_host_hs_addrs[] = { + { + .name = uhh, + .pa_start = 0x4a064000, + .pa_end = 0x4a0647ff, + .flags = ADDR_TYPE_RT + }, + {} +}; + +static struct omap_hwmod_ocp_if omap44xx_l4_cfg__usb_host_hs = { + .master = omap44xx_l4_cfg_hwmod, + .slave = omap44xx_usb_host_hs_hwmod, + .clk= l4_div_ck, + .addr = omap44xx_usb_host_hs_addrs, + .user = OCP_USER_MPU | OCP_USER_SDMA, +}; + +static struct omap_hwmod_ocp_if *omap44xx_usb_host_hs_slaves[] = { + omap44xx_l4_cfg__usb_host_hs, +}; + +static struct omap_hwmod omap44xx_usb_host_hs_hwmod = { + .name = usb_host_hs, + .class = omap44xx_usb_host_hs_hwmod_class, + .clkdm_name = l3_init_clkdm, + .main_clk = usb_host_hs_fck, + .prcm = { + .omap4 = { + .clkctrl_offs = OMAP4_CM_L3INIT_USB_HOST_CLKCTRL_OFFSET, + .context_offs = OMAP4_RM_L3INIT_USB_HOST_CONTEXT_OFFSET, + .modulemode = MODULEMODE_SWCTRL, + }, + }, + .slaves = omap44xx_usb_host_hs_slaves, + .slaves_cnt =
Re: [PATCH 1/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap4
Hi On Thu, 22 Sep 2011, Keshava Munegowda wrote: Following 4 hwmod structures are added 1. usb_host_hs hwmod of usbhs with uhh base address and functional clock, 2. usb_ehci_hs hwmod with irq and base address, 3. usb_ohci_hs hwmod with irq and base address, - The ehci and ohci hwmods does not require functional clock because usb_host_hs has functional clock which is sufficient to access ehci and ohci address space. - The usb_ehci_hs and usb_ohci_hs should be two separate hwmods which should not be combined. This is needed because ehci and ohci will have separate dedicated ports in omap4 there is a clock per port.We should be able to configure the IO-Wakeup capability of pins specific to EHCI OHCI separately and depending on the I/O wakeup event the only port clocks corresponding to the wakeup source will be enabled internally by the usb host driver. 4. usb_tll_hs hwmod of usbhs with the TLL base address and irq. Many of the same issues with the OMAP3 data (missing main_clks, missing ADDR_TYPE_RT, etc.) exist with this patch also. - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap4
Hi Paul, On 9/22/2011 8:14 PM, Paul Walmsley wrote: Hi On Thu, 22 Sep 2011, Keshava Munegowda wrote: Following 4 hwmod structures are added 1. usb_host_hs hwmod of usbhs with uhh base address and functional clock, 2. usb_ehci_hs hwmod with irq and base address, 3. usb_ohci_hs hwmod with irq and base address, - The ehci and ohci hwmods does not require functional clock because usb_host_hs has functional clock which is sufficient to access ehci and ohci address space. - The usb_ehci_hs and usb_ohci_hs should be two separate hwmods which should not be combined. This is needed because ehci and ohci will have separate dedicated ports in omap4 there is a clock per port.We should be able to configure the IO-Wakeup capability of pins specific to EHCI OHCI separately and depending on the I/O wakeup event the only port clocks corresponding to the wakeup source will be enabled internally by the usb host driver. 4. usb_tll_hs hwmod of usbhs with the TLL base address and irq. Many of the same issues with the OMAP3 data (missing main_clks, missing ADDR_TYPE_RT, etc.) exist with this patch also. ADDR_TYPE_RT is mainly use for sysconfig access inside hwmod core today, so why should we use it in this case? To be honest, I've been confused with that flag for some time :-) * ADDR_TYPE_RT: Address space contains module register target data. Maybe that's my English, but what does register target data mean exactly? What was the intent? Providing a way to identify some register vs memory space? Regarding main_clk, I do not think that some internal IPs like ohci/ehci should have a main_clk, since this is not visible at that level. These blocks do not have any PRCM / OCP connection. In fact they should not even be hwmod in theory, these are just some internal IPs inside the usb_host controller. These are hwmods just because of the dynamic mux support needed by these IPs. That was one of the comments I made on these, and Keshava explained me the rational and added it in the changelog. Hopefully, we will not have that limitation once we will have migrated that to DT :-) Regards, Benoit -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap4
Hi On Thu, 22 Sep 2011, Cousson, Benoit wrote: ADDR_TYPE_RT is mainly use for sysconfig access inside hwmod core today, so why should we use it in this case? Just for consistency, since the flag isn't defined to be SYSCONFIG-specific. That way, if there's another need -- other than SYSCONFIG access -- to identify the chunk of address space that's used for register access, we won't have to dig through and possibly repatch the hwmod data, just because some people didn't want to follow the rule. If the flag was simply defined to be SYSCONFIG-specific, then you're right, it wouldn't be needed. To be honest, I've been confused with that flag for some time :-) * ADDR_TYPE_RT: Address space contains module register target data. Maybe that's my English, but what does register target data mean exactly? The name comes from the L3 section of the 34xx TRM - see for example section 9.1.1 Terminology of the rev ZR TRM. The L3 has several address space sections, and whoever wrote that text -- Sonics? -- called the one with the L3's own internal registers the register target. And I was looking for a name that I did not have to make up, so I personally wouldn't have to defend the name ;-) What was the intent? Providing a way to identify some register vs memory space? As you suggest, the original impetus for the flag was to identify which chunk of address space needs to be mapped by the hwmod code for SYSCONFIG accesses to work. On current OMAPs, this seems to be the same thing as identifying the IP block's primary register area for every module with SYS* and REVISION registers. And I probably thought at the time that specifying the IP block's main register mapping seemed more useful and generally applicable than designating where the SYSCONFIG register was. Hence the current definition. Regarding main_clk, I do not think that some internal IPs like ohci/ehci should have a main_clk, since this is not visible at that level. Do you not agree that every IP block that contains sequential logic on current and foreseeable future SoCs must have some clock signal to drive that logic? The idea of the main_clk was not intended to be PRCM or OCP or even OMAP-specific. It's just intended to represent a clock that is used to drive the register logic inside the IP block. Therefore it must be enabled before any register access may occur. Even if clock gating is handled by some higher-level interface (e.g., at the IP block level), the main_clk has a rate, so it also implies an upper limit on how quickly register operations can occur. I suppose that all of the IP block's clocks could be optional clocks, but we know that every IP block with registers requires at least one clock to work, and that should be the main_clk. These blocks do not have any PRCM / OCP connection. In fact they should not even be hwmod in theory, these are just some internal IPs inside the usb_host controller. After looking at the OMAP USB host controller drivers, particularly drivers/mfd/omap-usb-host.c, I completely agree: it's not just theory: OMAP shouldn't have EHCI and OHCI hwmods in practice, either :-) These are hwmods just because of the dynamic mux support needed by these IPs. That was one of the comments I made on these, and Keshava explained me the rational and added it in the changelog. On OMAP, since the EHCI and OHCI IP blocks are integrated into the UHH IP block, an MFD driver creates the EHCI and OHCI platform_devices (see drivers/mfd/omap-usb-host.c:omap_usbhs_alloc_children()). On OMAP, the EHCI and OHCI aren't hwmod-backed devices, and so they shouldn't be using the hwmod dynamic pin remuxing code directly. So I'd suggest one of two approaches: 1. If the pin muxing can follow the PM runtime status of the UHH IP block, then the pin mux data should be associated with the UHH hwmod. 2. If the pin muxing must follow the EHCI/OHCI IP block PM runtime status, then drivers/mfd/omap-usb-host.c is what should be handling the remuxing. omap-usb-host.c can set the dev_pm_ops of the EHCI/OHCI platform_devices to point to functions either in arch/arm/mach-omap2/usb-host.c, or local functions that call into mach-omap2/usb-host.c functions to handle pin remuxing. (Those function pointers should be provided to the MFD driver in some clean way, like via platform_data.) Hopefully, we will not have that limitation once we will have migrated that to DT :-) Even if the data is coming from some other source, if the code still relies on main_clk, then it will need to be present. But why do you perceive that specifying a main_clk is a limitation? Or, put differently: what problem is caused by specifying a main_clk here? - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap4
Hi Keshava so based on a closer look at how the EHCI/OHCI IP blocks are actually implemented on OMAP, they should not have hwmod entries associated with them. Please take a look at http://marc.info/?l=linux-omapm=131673433121673w=2 - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap4
Hi On Thu, 22 Sep 2011, Paul Walmsley wrote: On Thu, 22 Sep 2011, Cousson, Benoit wrote: ADDR_TYPE_RT is mainly use for sysconfig access inside hwmod core today, so why should we use it in this case? Just for consistency, since the flag isn't defined to be SYSCONFIG-specific. That way, if there's another need -- other than SYSCONFIG access -- to identify the chunk of address space that's used for register access, we won't have to dig through and possibly repatch the hwmod data, just because some people didn't want to follow the rule. If the flag was simply defined to be SYSCONFIG-specific, then you're right, it wouldn't be needed. To be honest, I've been confused with that flag for some time :-) * ADDR_TYPE_RT: Address space contains module register target data. Maybe that's my English, but what does register target data mean exactly? The name comes from the L3 section of the 34xx TRM - see for example section 9.1.1 Terminology of the rev ZR TRM. The L3 has several address space sections, and whoever wrote that text -- Sonics? -- called the one with the L3's own internal registers the register target. And I was looking for a name that I did not have to make up, so I personally wouldn't have to defend the name ;-) What was the intent? Providing a way to identify some register vs memory space? As you suggest, the original impetus for the flag was to identify which chunk of address space needs to be mapped by the hwmod code for SYSCONFIG accesses to work. On current OMAPs, this seems to be the same thing as identifying the IP block's primary register area for every module with SYS* and REVISION registers. And I probably thought at the time that specifying the IP block's main register mapping seemed more useful and generally applicable than designating where the SYSCONFIG register was. Hence the current definition. Hopefully this should go without saying, but if you think there's a good rationale for renaming this flag to indicate that an address space contains the SYSCONFIG registers, or maybe just changing the documentation for the flag to indicate that, that's fine with me, just send a patch. - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html