Re: [PATCH v4 3/8] wireless: wl1271: add platform driver to get board data

[Felipe Balbi <felipe.balbi@nokia.com>]

hi,

On Wed, Aug 11, 2010 at 08:42:18PM +0200, ext DebBarma, Tarun Kanti wrote:
>> @@ -182,10 +186,84 @@ static struct wl1271_if_operations sdio_ops = {
>>  	.disable_irq	= wl1271_sdio_disable_interrupts
>>  };
>>
>> +static int wl1271_plat_probe(struct platform_device *pdev)
>> +{
>> +	struct wl12xx_platform_data *pdata;
>> +	struct platform_driver *pdriver;
>> +	struct wl12xx_plat_instance *pinstance;
>> +
>What about checking for pdev here before extracting pdata?

why ? if probe is called pdev is guaranteed to be valid, no ?

>> +	pdata = pdev->dev.platform_data;
>> +	if (!pdata) {
>> +		wl1271_error("no platform data");
>> +		return -ENODEV;
>> +	}
>> +
>> +	pdriver = container_of(pdev->dev.driver, struct platform_driver,
>> +								driver);

but you shouldn't fiddle with the driver structure here. What are you 
actually trying to achieve here ? What's pinstance supposed to do ? are 
you trying to handle cases where you might have several wl12xx chips on 
the same board ? If that's the case you should have several platform 
devices, one for each chip. They'll only have different ids.

-- 
balbi

DefectiveByDesign.org