| 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: Mon, 19 Sep 2011 22:46:25 +0200
From: "Cousson, Benoit" <b-cousson@...com>
To: "Munegowda, Keshava" <keshava_mgowda@...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>
Subject: Re: [PATCH 1/5 v9] arm: omap: usb: ehci and ohci hwmod structures
for omap4
Keshava,
I've just replied to your previous v8 version about the comments you
didn't take into account.
I just have one minor nit on that one I missed previously, plus a couple
of clarifications.
On 9/15/2011 3:22 PM, 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>
> Signed-off-by: Keshava Munegowda<keshava_mgowda@...com>
> ---
> arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 249 +++++++++++++++++++++++++++-
> 1 files changed, 248 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..f06efa6 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,244 @@ 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),
> + .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",
> + .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 = "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,
Why do you need these flags? These flags are reserved for non-standard
behavior / HW bugs. I know that this IP is full of various bugs, but it
will be nice to add some explanation in the changelog and a small
comment here as well.
> + .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
> +};
> +
> +/* 'usbhs_ohci' class */
> +static struct omap_hwmod_class omap44xx_usbhs_ohci_hwmod_class = {
> + .name = "usbhs_ohci",
> +};
> +
> +static struct omap_hwmod_irq_info omap44xx_usbhs_ohci_irqs[] = {
> + { .name = "ohci-irq", .irq = 76 + OMAP44XX_IRQ_GIC_START },
> + { .irq = -1 }
> +};
> +
> +static struct omap_hwmod_addr_space omap44xx_usbhs_ohci_addrs[] = {
> + {
> + .name = "ohci",
> + .pa_start = 0x4A064800,
> + .pa_end = 0x4A064BFF,
Nit: Please use lower case for hexa value. This is applicable for the
whole patch.
> + .flags = ADDR_MAP_ON_INIT
> + },
> + {}
> +};
> +
> +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),
> + .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
> + .flags = HWMOD_INIT_NO_RESET | HWMOD_NO_IDLEST,
Same question about these flags. Why do you need these? Please add some
explanation.
That comment is applicable for the remaining flags. These flags are
probably all needed for that kind of IP, but the least you should do is
to add some explanation.
Thanks,
Benoit
--
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