| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 15 Sep 2011 12:52:59 +0530
From: "Munegowda, Keshava" <keshava_mgowda@...com>
To: "Cousson, Benoit" <b-cousson@...com>
Cc: "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Balbi, Felipe" <balbi@...com>, "Gadiyar, Anand" <gadiyar@...com>,
"sameo@...ux.intel.com" <sameo@...ux.intel.com>,
"parthab@...ia.ti.com" <parthab@...ia.ti.com>,
"tony@...mide.com" <tony@...mide.com>,
"Hilman, Kevin" <khilman@...com>,
"paul@...an.com" <paul@...an.com>,
"johnstul@...ibm.com" <johnstul@...ibm.com>,
"Sripathy, Vishwanath" <vishwanath.bs@...com>,
"Quadros, Roger" <rogerq@...com>
Subject: Re: [PATCH 1/5 v8] arm: omap: usb: ehci and ohci hwmod structures for omap4
On Thu, Sep 15, 2011 at 11:25 AM, Munegowda, Keshava
<keshava_mgowda@...com> wrote:
> On Wed, Sep 14, 2011 at 10:20 PM, Cousson, Benoit <b-cousson@...com> wrote:
>> Hi Keshava,
>>
>> On 8/25/2011 9:01 AM, Munegowda, Keshava wrote:
>>> From: Benoit Cousson<b-cousson@...com>
>>>
>>> Following 4 hwmod structures are added:
>>> UHH hwmod of usbhs with uhh base address and functional clock,
>>> EHCI hwmod with irq and base address,
>>> OHCI hwmod with irq and base address,
>>> TLL hwmod of usbhs with the TLL base address and irq.
>>>
>>> Signed-off-by: Benoit Cousson<b-cousson@...com>
>>
>> That version is really different compared to my original patch, so you should highlight the diff you introduced.
Since there are too many changes are done compare to your original
patch; i prefer keep a single patch,rather
keeping your original patch and my changes are another patch.
>>> Signed-off-by: Keshava Munegowda<keshava_mgowda@...com>
>>> ---
>>> arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 247 ++++++++++++++++++++++++++++
>>> 1 files changed, 247 insertions(+), 0 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..0bc01dd 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;
>>
>> None of the 3 last entries are master, and thus should not need any backward declaration.
yes, I will make this change.
>>
>>> /*
>>> * 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 = 0x0000,
>>> + .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),
>>
>> Minor, but it should be:
>> + .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 = "usbhs_uhh",
>>> + .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",
>>
>> In general, there is no name for unique entry. And if you need a name, you should find something relevant considering this is local to the hwmod.
>>
>>> + .pa_start = 0x4a064000,
>>> + .pa_end = 0x4a0647ff,
>>> + .flags = ADDR_TYPE_RT
>>> + },
>>> + {} /* Terminating Entry */
>>
>> That comment is useless. Paul added one space inside the terminator as well.
>>
>>> +};
>>> +
>>> +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 = "usbhs_uhh",
>>> + .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 = ARRAY_SIZE(omap44xx_usb_host_hs_slaves),
>>> + .masters = omap44xx_usb_host_hs_masters,
>>> + .masters_cnt = ARRAY_SIZE(omap44xx_usb_host_hs_masters),
>>> + .flags = HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY,
>>> + .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
>>> +};
>>> +
>>> +/* 'usbhs_ohci' class */
>>> +static struct omap_hwmod_class omap44xx_usbhs_ohci_hwmod_class = {
>>> + .name = "usbhs_ohci",
>>
>> In the context of devicetree and considering what Felipe did for the USB3, I'm wondering if you should define hwmods for ohci and ehci.
>> I'm not 100% sure it will apply here, but cannot you add the register entries inside the usb_host_hs hwmod and create the devices using that?
>>
>> There is no PRCM entry for them so they do not need to be omap_device.
you need , ehci and ohci as a different hwmods, because later we can
add the mux to ehc and ochi independently.
>>> +};
>>> +
>>> +static struct omap_hwmod_irq_info omap44xx_usbhs_ohci_irqs[] = {
>>> + { .name = "ohci-irq", .irq = 76 + OMAP44XX_IRQ_GIC_START },
>>
>> Same comment that before about address space name.
>>
>>> + { .irq = -1 } /* Terminating IRQ */
>>> +};
>>> +
>>> +static struct omap_hwmod_addr_space omap44xx_usbhs_ohci_addrs[] = {
>>> + {
>>> + .name = "ohci",
>>
>> Same comment than before.
>>
>>> + .pa_start = 0x4A064800,
>>> + .pa_end = 0x4A064BFF,
>>> + .flags = ADDR_MAP_ON_INIT
>>
>> Why do you need that flag?
This address space does not has sysconfig; so hwmod need not to map to
this address.
driver will to ioremap.
>>> + },
>>> + {}
>>> +};
>>> +
>>> +static struct omap_hwmod_ocp_if omap44xx_l4_cfg__usbhs_ohci = {
>>> + .master =&omap44xx_l4_cfg_hwmod,
>>> + .slave =&omap44xx_usbhs_ohci_hwmod,
>>> + .clk = "l4_div_ck",
>>> + .addr = omap44xx_usbhs_ohci_addrs,
>>> + .user = OCP_USER_MPU | OCP_USER_SDMA,
>>> +};
>>> +
>>> +static struct omap_hwmod_ocp_if *omap44xx_usbhs_ohci_slaves[] = {
>>> + &omap44xx_l4_cfg__usbhs_ohci,
>>> +};
>>> +
>>> +static struct omap_hwmod_ocp_if *omap44xx_usbhs_ohci_masters[] = {
>>> + &omap44xx_usb_host_hs__l3_main_2,
>>> +};
>>> +
>>> +static struct omap_hwmod omap44xx_usbhs_ohci_hwmod = {
>>> + .name = "usbhs_ohci",
>>> + .class =&omap44xx_usbhs_ohci_hwmod_class,
>>> + .clkdm_name = "l3_init_clkdm",
>>> + .mpu_irqs = omap44xx_usbhs_ohci_irqs,
>>> + .slaves = omap44xx_usbhs_ohci_slaves,
>>> + .slaves_cnt = ARRAY_SIZE(omap44xx_usbhs_ohci_slaves),
>>> + .masters = omap44xx_usbhs_ohci_masters,
>>> + .masters_cnt = ARRAY_SIZE(omap44xx_usbhs_ohci_masters),
>>
>> Assuming you need hwmod entries for that, these master entries are useless and probably wrong.
>> The TRM just documents 2 ports for usb_host_hs and 1 port for usb_tll_hs.
ports are not specific to usbhs or tll, port 1 and port2 can be used
in phy or tll modes.
>> You will still need the slave for the address space.
>>
>>> + .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
>>> + .flags = HWMOD_INIT_NO_RESET | HWMOD_NO_IDLEST,
>>> +};
>>> +
>>> +/* 'usbhs_ehci' class */
>>> +static struct omap_hwmod_class omap44xx_usbhs_ehci_hwmod_class = {
>>> + .name = "usbhs_ehci",
>>> +};
>>> +
>>> +static struct omap_hwmod_irq_info omap44xx_usbhs_ehci_irqs[] = {
>>> + { .name = "ehci-irq", .irq = 77 + OMAP44XX_IRQ_GIC_START },
>>> + { .irq = -1 } /* Terminating IRQ */
>>> +};
>>> +
>>> +static struct omap_hwmod_addr_space omap44xx_usbhs_ehci_addrs[] = {
>>> + {
>>> + .name = "ehci",
>>> + .pa_start = 0x4A064C00,
>>> + .pa_end = 0x4A064FFF,
>>> + .flags = ADDR_MAP_ON_INIT
>>> + },
>>> + {} /* Terminating Entry */
>>> +};
>>> +
>>> +static struct omap_hwmod_ocp_if omap44xx_l4_cfg__usbhs_ehci = {
>>> + .master =&omap44xx_l4_cfg_hwmod,
>>> + .slave =&omap44xx_usbhs_ehci_hwmod,
>>> + .clk = "l4_div_ck",
>>> + .addr = omap44xx_usbhs_ehci_addrs,
>>> + .user = OCP_USER_MPU | OCP_USER_SDMA,
>>> +};
>>> +
>>> +static struct omap_hwmod_ocp_if *omap44xx_usbhs_ehci_slaves[] = {
>>> + &omap44xx_l4_cfg__usbhs_ehci,
>>> +};
>>> +
>>> +static struct omap_hwmod_ocp_if *omap44xx_usbhs_ehci_masters[] = {
>>> + &omap44xx_usb_host_hs__l3_main_2,
>>> +};
>>> +
>>> +
>>> +static struct omap_hwmod omap44xx_usbhs_ehci_hwmod = {
>>> + .name = "usbhs_ehci",
>>> + .class =&omap44xx_usbhs_ehci_hwmod_class,
>>> + .clkdm_name = "l3_init_clkdm",
>>> + .mpu_irqs = omap44xx_usbhs_ehci_irqs,
>>> + .slaves = omap44xx_usbhs_ehci_slaves,
>>> + .slaves_cnt = ARRAY_SIZE(omap44xx_usbhs_ehci_slaves),
>>> + .masters = omap44xx_usbhs_ehci_masters,
>>> + .masters_cnt = ARRAY_SIZE(omap44xx_usbhs_ehci_masters),
>>> + .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
>>> + .flags = HWMOD_INIT_NO_RESET | HWMOD_NO_IDLEST,
>>> +};
>>> +
>>> +/*
>>> + * 'usb_tll_hs' class
>>> + * usb_tll_hs module is the adapter on the usb_host_hs ports
>>> + */
>>> +static struct omap_hwmod_class_sysconfig omap44xx_usb_tll_hs_sysc = {
>>> + .rev_offs = 0x0000,
>>> + .sysc_offs = 0x0010,
>>> + .syss_offs = 0x0014,
>>> + .sysc_flags = (SYSC_HAS_AUTOIDLE | SYSC_HAS_SIDLEMODE |
>>> + SYSC_HAS_SOFTRESET | SYSC_HAS_ENAWAKEUP |
>>> + SYSC_HAS_CLOCKACTIVITY),
>>> + .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
>>> + .sysc_fields =&omap_hwmod_sysc_type1,
>>> +};
>>> +
>>> +static struct omap_hwmod_class omap44xx_usb_tll_hs_hwmod_class = {
>>> + .name = "usbhs_tll",
>>> + .sysc =&omap44xx_usb_tll_hs_sysc,
>>> +};
>>> +
>>> +static struct omap_hwmod_irq_info omap44xx_usb_tll_hs_irqs[] = {
>>> + { .name = "tll-irq", .irq = 78 + OMAP44XX_IRQ_GIC_START },
>>> + { .irq = -1 } /* Terminating IRQ */
>>> +};
>>> +
>>> +static struct omap_hwmod_addr_space omap44xx_usb_tll_hs_addrs[] = {
>>> + {
>>> + .name = "tll",
>>> + .pa_start = 0x4a062000,
>>> + .pa_end = 0x4a063fff,
>>> + .flags = ADDR_TYPE_RT
>>> + },
>>> + {} /* Terminating Entry */
>>> +};
>>> +
>>> +static struct omap_hwmod_ocp_if omap44xx_l4_cfg__usb_tll_hs = {
>>> + .master =&omap44xx_l4_cfg_hwmod,
>>> + .slave =&omap44xx_usb_tll_hs_hwmod,
>>> + .clk = "l4_div_ck",
>>> + .addr = omap44xx_usb_tll_hs_addrs,
>>> + .user = OCP_USER_MPU | OCP_USER_SDMA,
>>> +};
>>> +
>>> +static struct omap_hwmod_ocp_if *omap44xx_usb_tll_hs_slaves[] = {
>>> + &omap44xx_l4_cfg__usb_tll_hs,
>>> +};
>>> +
>>> +static struct omap_hwmod omap44xx_usb_tll_hs_hwmod = {
>>> + .name = "usbhs_tll",
>>> + .class =&omap44xx_usb_tll_hs_hwmod_class,
>>> + .clkdm_name = "l3_init_clkdm",
>>> + .mpu_irqs = omap44xx_usb_tll_hs_irqs,
>>> + .main_clk = "usb_tll_hs_ick",
>>> + .prcm = {
>>> + .omap4 = {
>>> + .clkctrl_offs = OMAP4_CM_L3INIT_USB_TLL_CLKCTRL_OFFSET,
>>> + .context_offs = OMAP4_RM_L3INIT_USB_TLL_CONTEXT_OFFSET,
>>> + .modulemode = MODULEMODE_HWCTRL,
>>> + },
>>> + },
>>> + .slaves = omap44xx_usb_tll_hs_slaves,
>>> + .slaves_cnt = ARRAY_SIZE(omap44xx_usb_tll_hs_slaves),
>>> + .flags = HWMOD_SWSUP_SIDLE,
>>> + .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
>>> +};
>>> +
>>> static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
>>>
>>> /* dmm class */
>>> @@ -5482,6 +5725,10 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
>>> &omap44xx_wd_timer2_hwmod,
>>> &omap44xx_wd_timer3_hwmod,
>>>
>>> + &omap44xx_usb_host_hs_hwmod,
>>> + &omap44xx_usbhs_ohci_hwmod,
>>> + &omap44xx_usbhs_ehci_hwmod,
>>> + &omap44xx_usb_tll_hs_hwmod,
>>
>> This list is ordered, so it should be something like that:
yes, i will do this.
>>
>> &omap44xx_uart3_hwmod,
>> &omap44xx_uart4_hwmod,
>>
>> + /* usb_host_hs class */
>> + &omap44xx_usb_host_hs_hwmod,
>> +
>> /* usb_otg_hs class */
>> &omap44xx_usb_otg_hs_hwmod,
>>
>> + /* usb_tll_hs class */
>> + &omap44xx_usb_tll_hs_hwmod,
>> +
>> /* wd_timer class */
>> &omap44xx_wd_timer2_hwmod,
>> &omap44xx_wd_timer3_hwmod,
>>
>> Regards,
>> Benoit
>
>
> Thanks benoit,
> I make the changes as per your comments ! I will send the new set of
> patches today only.
>
>
> regards
> keshava
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists