RE: [PATCH 02/14] dspbridge: deh: trivial cleanups

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Fernando,

Thanks for forwarding the thread.

In wait_for_timer(), should return be used inside the loop?  It will result in omap_dm_timer_disable(timer) not being called.  I believe it should be called in all outcomes.

> +	for (c = 0; c < GPTIMER_IRQ_WAIT_MAX_CNT; c++)
> +		if ((omap_dm_timer_read_status(timer) & GPTIMER_IRQ_OVERFLOW))
> +			return;
> +
> +	pr_err("%s: GPTimer interrupt failed\n", __func__);
> 
>  	omap_dm_timer_disable(timer);
>  }

Best Regards,
 
Cris

-----Original Message-----
From: Guzman Lugo, Fernando 
Sent: Monday, May 17, 2010 2:09 PM
To: Felipe Contreras; linux-omap; Liu, Stanley; Jansson, Cris
Cc: Ramirez Luna, Omar
Subject: RE: [PATCH 02/14] dspbridge: deh: trivial cleanups



+Cris
+Stanley
Loop in DSP guys in case they would have something to add.

> -----Original Message-----
> From: Felipe Contreras [mailto:felipe.contreras@xxxxxxxxx]
> Sent: Sunday, May 16, 2010 10:46 AM
> To: linux-omap
> Cc: Ramirez Luna, Omar; Guzman Lugo, Fernando; Felipe Contreras
> Subject: [PATCH 02/14] dspbridge: deh: trivial cleanups
> 
> No functional changes.
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
> ---
>  drivers/dsp/bridge/core/ue_deh.c |  112 ++++++++++++++-------------------
> -----
>  1 files changed, 41 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/dsp/bridge/core/ue_deh.c
> b/drivers/dsp/bridge/core/ue_deh.c
> index 48c11e9..605cec7 100644
> --- a/drivers/dsp/bridge/core/ue_deh.c
> +++ b/drivers/dsp/bridge/core/ue_deh.c
> @@ -60,11 +60,6 @@
>  /* Max time to check for GP Timer IRQ */
>  #define GPTIMER_IRQ_WAIT_MAX_CNT       1000
> 
> -static struct hw_mmu_map_attrs_t map_attrs = { HW_LITTLE_ENDIAN,
> -	HW_ELEM_SIZE16BIT,
> -	HW_MMU_CPUES
> -};
> -
>  static void *dummy_va_addr;
> 
>  static struct omap_dm_timer *timer;
> @@ -72,7 +67,7 @@ static struct omap_dm_timer *timer;
>  dsp_status bridge_deh_create(struct deh_mgr **ret_deh_mgr,
>  		struct dev_object *hdev_obj)
>  {
> -	dsp_status status = DSP_SOK;
> +	dsp_status status;
>  	struct deh_mgr *deh_mgr;
>  	struct bridge_dev_context *hbridge_context = NULL;
> 
> @@ -81,23 +76,20 @@ dsp_status bridge_deh_create(struct deh_mgr
> **ret_deh_mgr,
>  	 *  the base image. */
>  	/* Get WMD context info. */
>  	dev_get_bridge_context(hdev_obj, &hbridge_context);
> -	DBC_ASSERT(hbridge_context);
> -	dummy_va_addr = NULL;
>  	/* Allocate IO manager object: */
>  	deh_mgr = kzalloc(sizeof(struct deh_mgr), GFP_KERNEL);
>  	if (!deh_mgr) {
>  		status = -ENOMEM;
> -		goto leave;
> +		goto err;
>  	}
> 
>  	/* Create an NTFY object to manage notifications */
>  	deh_mgr->ntfy_obj = kmalloc(sizeof(struct ntfy_object), GFP_KERNEL);
> -	if (deh_mgr->ntfy_obj) {
> -		ntfy_init(deh_mgr->ntfy_obj);
> -	} else {
> +	if (!deh_mgr->ntfy_obj) {
>  		status = -ENOMEM;
>  		goto err;
>  	}
> +	ntfy_init(deh_mgr->ntfy_obj);
> 
>  	/* Create a MMUfault DPC */
>  	tasklet_init(&deh_mgr->dpc_tasklet, mmu_fault_dpc, (u32) deh_mgr);
> @@ -110,32 +102,25 @@ dsp_status bridge_deh_create(struct deh_mgr
> **ret_deh_mgr,
>  	deh_mgr->err_info.dw_val3 = 0L;
> 
>  	/* Install ISR function for DSP MMU fault */
> -	if ((request_irq(INT_DSP_MMU_IRQ, mmu_fault_isr, 0,
> -					"DspBridge\tiommu fault",
> -					(void *)deh_mgr)) == 0)
> -		status = DSP_SOK;
> -	else
> -		status = -EPERM;
> +	status = request_irq(INT_DSP_MMU_IRQ, mmu_fault_isr, 0,
> +			"DspBridge\tiommu fault", deh_mgr);
> +	if (status < 0)
> +		goto err;
> 
> -err:
> -	if (DSP_FAILED(status)) {
> -		/* If create failed, cleanup */
> -		bridge_deh_destroy(deh_mgr);
> -		deh_mgr = NULL;
> -	} else {
> -		timer = omap_dm_timer_request_specific(
> -					GPTIMER_FOR_DSP_MMU_FAULT);
> -		if (timer) {
> -			omap_dm_timer_disable(timer);
> -		} else {
> -			pr_err("%s: GPTimer not available\n", __func__);
> -			return -ENODEV;
> -		}
> +	timer = omap_dm_timer_request_specific(GPTIMER_FOR_DSP_MMU_FAULT);
> +	if (!timer) {
> +		pr_err("%s: GPTimer not available\n", __func__);
> +		status = -ENODEV;
> +		goto err;
>  	}
> +	omap_dm_timer_disable(timer);
> 
> -leave:
>  	*ret_deh_mgr = deh_mgr;
> +	return 0;
> 
> +err:
> +	bridge_deh_destroy(deh_mgr);
> +	*ret_deh_mgr = NULL;
>  	return status;
>  }
> 
> @@ -164,36 +149,31 @@ dsp_status bridge_deh_destroy(struct deh_mgr
> *deh_mgr)
>  	omap_dm_timer_free(timer);
>  	timer = NULL;
> 
> -	return DSP_SOK;
> +	return 0;
>  }
> 
>  dsp_status bridge_deh_register_notify(struct deh_mgr *deh_mgr, u32
> event_mask,
>  		u32 notify_type,
>  		struct dsp_notification *hnotification)
>  {
> -	dsp_status status = DSP_SOK;
> -
>  	if (!deh_mgr)
>  		return -EFAULT;
> 
>  	if (event_mask)
> -		status = ntfy_register(deh_mgr->ntfy_obj, hnotification,
> -					event_mask, notify_type);
> +		return ntfy_register(deh_mgr->ntfy_obj, hnotification,
> +				event_mask, notify_type);
>  	else
> -		status = ntfy_unregister(deh_mgr->ntfy_obj, hnotification);
> -
> -	return status;
> +		return ntfy_unregister(deh_mgr->ntfy_obj, hnotification);
>  }
> 
>  static void wait_for_timer(void)
>  {
> -	u32 cnt = 0;
> +	int c;
> 
>  	omap_dm_timer_enable(timer);
> 
>  	/* Enable overflow interrupt */
> -	omap_dm_timer_set_int_enable(timer,
> -			GPTIMER_IRQ_OVERFLOW);
> +	omap_dm_timer_set_int_enable(timer, GPTIMER_IRQ_OVERFLOW);
>  	/*
>  	 * Set counter value to overflow counter after
>  	 * one tick and start timer.
> @@ -204,15 +184,11 @@ static void wait_for_timer(void)
>  	udelay(80);
> 
>  	/* Check interrupt status and wait for interrupt */
> -	cnt = 0;
> -	while (!(omap_dm_timer_read_status(timer) &
> -				GPTIMER_IRQ_OVERFLOW)) {
> -		if (cnt++ >= GPTIMER_IRQ_WAIT_MAX_CNT) {
> -			pr_err("%s: GPTimer interrupt failed\n",
> -					__func__);
> -			break;
> -		}
> -	}
> +	for (c = 0; c < GPTIMER_IRQ_WAIT_MAX_CNT; c++)
> +		if ((omap_dm_timer_read_status(timer) & GPTIMER_IRQ_OVERFLOW))
> +			return;
> +
> +	pr_err("%s: GPTimer interrupt failed\n", __func__);
> 
>  	omap_dm_timer_disable(timer);
>  }
> @@ -220,16 +196,19 @@ static void wait_for_timer(void)
>  void bridge_deh_notify(struct deh_mgr *deh_mgr, u32 ulEventMask, u32
> dwErrInfo)
>  {
>  	struct bridge_dev_context *dev_context;
> -	dsp_status status = DSP_SOK;
>  	u32 hw_mmu_max_tlb_count = 31;
>  	struct cfg_hostres *resources;
> -	hw_status hw_status_obj;
> +	struct hw_mmu_map_attrs_t map_attrs = {
> +		.endianism = HW_LITTLE_ENDIAN,
> +		.element_size = HW_ELEM_SIZE16BIT,
> +		.mixed_size = HW_MMU_CPUES,
> +	};
> 
>  	if (!deh_mgr)
>  		return;
> 
>  	dev_info(bridge, "%s: device exception\n", __func__);
> -	dev_context = (struct bridge_dev_context *)deh_mgr->hbridge_context;
> +	dev_context = deh_mgr->hbridge_context;
>  	resources = dev_context->resources;
> 
>  	switch (ulEventMask) {
> @@ -256,8 +235,6 @@ void bridge_deh_notify(struct deh_mgr *deh_mgr, u32
> ulEventMask, u32 dwErrInfo)
>  			(unsigned int) deh_mgr->err_info.dw_val2,
>  			(unsigned int) fault_addr);
>  		dummy_va_addr = (void*)__get_free_page(GFP_ATOMIC);
> -		dev_context = (struct bridge_dev_context *)
> -			deh_mgr->hbridge_context;
> 
>  		print_dsp_trace_buffer(dev_context);
>  		dump_dl_modules(dev_context);
> @@ -272,20 +249,16 @@ void bridge_deh_notify(struct deh_mgr *deh_mgr, u32
> ulEventMask, u32 dwErrInfo)
>  			dev_context->num_tlb_entries =
>  				dev_context->fixed_tlb_entries;
>  		}
> -		if (DSP_SUCCEEDED(status)) {
> -			hw_status_obj =
> -				hw_mmu_tlb_add(resources->dw_dmmu_base,
> -						virt_to_phys(dummy_va_addr),
> fault_addr,
> -						HW_PAGE_SIZE4KB, 1,
> -						&map_attrs, HW_SET, HW_SET);
> -		}
> +		hw_mmu_tlb_add(resources->dw_dmmu_base,
> +				virt_to_phys(dummy_va_addr), fault_addr,
> +				HW_PAGE_SIZE4KB, 1,
> +				&map_attrs, HW_SET, HW_SET);
> 
>  		wait_for_timer();
> -
>  		/* Clear MMU interrupt */
>  		hw_mmu_event_ack(resources->dw_dmmu_base,
>  				HW_MMU_TRANSLATION_FAULT);
> -		dump_dsp_stack(deh_mgr->hbridge_context);
> +		dump_dsp_stack(dev_context);
>  		break;
>  #ifdef CONFIG_BRIDGE_NTFY_PWRERR
>  	case DSP_PWRERROR:
> @@ -334,9 +307,6 @@ void bridge_deh_notify(struct deh_mgr *deh_mgr, u32
> ulEventMask, u32 dwErrInfo)
>  dsp_status bridge_deh_get_info(struct deh_mgr *deh_mgr,
>  		struct dsp_errorinfo *pErrInfo)
>  {
> -	DBC_REQUIRE(deh_mgr);
> -	DBC_REQUIRE(pErrInfo);
> -
>  	if (!deh_mgr)
>  		return -EFAULT;
> 
> @@ -346,7 +316,7 @@ dsp_status bridge_deh_get_info(struct deh_mgr
> *deh_mgr,
>  	pErrInfo->dw_val2 = deh_mgr->err_info.dw_val2;
>  	pErrInfo->dw_val3 = deh_mgr->err_info.dw_val3;
> 
> -	return DSP_SOK;
> +	return 0;
>  }
> 
>  void bridge_deh_release_dummy_mem(void)
> --
> 1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux