lkml.org 
[lkml]   [2008]   [Jan]   [24]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
/
DateThu, 24 Jan 2008 17:19:20 +1100
FromStephen Rothwell <>
SubjectRe: [PATCH] UCC TDM driver for QE based MPC83xx platforms.
On Thu, 24 Jan 2008 10:16:42 +0530 (IST) Poonam_Aggrwal-b10812 <b10812@freescale.com> wrote:
>> +static int ucc_tdm_probe(struct of_device *ofdev,> +			 const struct of_device_id *match)> +{> +	struct device_node *np = ofdev->node;> +	struct resource res;> +	const unsigned int *prop;> +	u32 ucc_num, device_num, err, ret = 0;> +	struct device_node *np_tmp = NULL;

You don't need to initialise this.

> +	dma_addr_t physaddr;> +	void *tdm_buff;> +	struct ucc_tdm_info *ut_info;> +> +	prop = of_get_property(np, "device-id", NULL);

You should check for (prop == NULL).

> +	ucc_num = *prop - 1;> +	if ((ucc_num < 0) || (ucc_num > 7))
> +		return -ENODEV;
> +
> +	ut_info = &utdm_info[ucc_num];
> +	if (ut_info == NULL) {

This cannot be NULL as you have just taken the address of an array
element.

> +	tdm_ctrl[device_num]->ut_info = ut_info;> +> +	tdm_ctrl[device_num]->ut_info->uf_info.ucc_num = ucc_num;
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is the same as "ut_info".
> +	tdm_ctrl[device_num]->ut_info->uf_info.tdm_tx_clk =
> +			(char *) of_get_property(np, "fsl,tdm-tx-clk", NULL);
                                ^
We don't normall put spaces here.

> +	tdm_ctrl[device_num]->ut_info->uf_info.tdm_rx_clk =
> +			(char *) of_get_property(np, "fsl,tdm-rx-clk", NULL);
                                ^
Ditto. And later as well.

> +	tdm_ctrl[device_num]->ut_info->uf_info.irq =
> +					irq_of_parse_and_map(np, 0);> +	err = of_address_to_resource(np, 0, &res);> +	if (err) {> +		ret = EINVAL;

This should be -EINVAL or err.

> +		goto get_property_error;

You need to do something about unmapping the irq in the error path.

> +	tdm_ctrl[device_num]->uf_regs = of_iomap(np, 0);
> +> +	np_tmp = of_find_compatible_node(np_tmp, "slic", "legerity-slic");
> +	if (np_tmp != NULL)> +		tdm_ctrl[device_num]->leg_slic = 1;> +	else> +		tdm_ctrl[device_num]->leg_slic = 0;

	of_node_ut(np_tmp);

> +	tdm_buff = dma_alloc_coherent(NULL, 2 * NR_BUFS * SAMPLE_DEPTH *
> +				tdm_ctrl[device_num]->cfg_ctrl.active_num_ts,
> +					&physaddr, GFP_KERNEL);> +	if (!tdm_buff) {> +		printk(KERN_ERR "ucc-tdm: could not allocate buffer"
> +					"descriptors\n");
> +		ret = -ENOMEM;
> +		goto get_property_error;

You need to unmap the uf_regs in the error path.

> +get_property_error:> +	kfree(tdm_ctrl[device_num]);

Do you need to set "tdm_ctrl[device_num]" to NULL and decrement
num_tdm_devices?

> +	return ret;> +}> +> +static int ucc_tdm_remove(struct of_device *ofdev)
> +{> +	struct tdm_ctrl *tdm_c;> +	struct ucc_tdm_info *ut_info;> +	u32 ucc_num;> +> +	tdm_c = dev_get_drvdata(&(ofdev->dev));

	dev_set_drvdata(&of_dev->dev, NULL);

> +	ucc_num = tdm_c->ut_info->uf_info.ucc_num;> +	ut_info = &utdm_info[ucc_num];> +	tdm_stop(tdm_c);> +	tdm_deinit(tdm_c);> +> +	ucc_fast_free(tdm_c->uf_private);> +> +	dma_free_coherent(NULL, 2 * NR_BUFS * SAMPLE_DEPTH *
> +				tdm_c->cfg_ctrl.active_num_ts,> +				tdm_c->tdm_input_data,> +				tdm_c->dma_input_addr);> +

You need to unmap the uf_reg and the irq.

> +static struct of_device_id ucc_tdm_match[] = {

const, please.

> +	{> +	 .type = "tdm",> +	 .compatible = "fsl,ucc-tdm",> +	 }, {},

We euld normall format this like:

	{ .type = "tdm", .compatible = "fsl,ucc-tdm", },
	{},
> +static struct of_platform_driver ucc_tdm_driver = {

	.driver = {
> +	.name = DRV_NAME,

	},
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2008-01-24 07:21    [from the cache]
©2003-2008