Re: [PATCH 1/2] i2c-s3c2410: Rework device type handling

[Karol Lewandowski <k.lewandowsk@samsung.com>]

On 23.04.2012 20:20, Wolfram Sang wrote:

Hi!

>> -static inline int s3c24xx_i2c_is2440(struct s3c24xx_i2c *i2c)
>> +static inline unsigned int s3c24xx_get_device_quirks(struct platform_device *pdev)
>>  {
>> -	struct platform_device *pdev = to_platform_device(i2c->dev);
>> -	enum s3c24xx_i2c_type type;
>> -
>> -#ifdef CONFIG_OF
>> -	if (i2c->dev->of_node)
>> -		return of_device_is_compatible(i2c->dev->of_node,
>> -				"samsung,s3c2440-i2c");
>> -#endif
>> +	if (pdev->dev.of_node) {
>> +		const struct of_device_id *match;
>> +		match = of_match_node(&s3c24xx_i2c_match, pdev->dev.of_node);
> 
> I'd appreciate a comment explaining why match can't be NULL here (as to
> my understanding, it can't). Or just check for it, but this way it looks
> a bit fishy and people (hopefully ;)) will ask about it.


My understanding is that it can't be null for exactly same reason why
platform_get_device_id(pdev) can't be null either (see below).

I.e. prerequisite for this code to be run at all (as it's called from
driver's .probe()) is to be already matched against very same match
table.

As far I can see the only possibility for it to fail is to have
dev.of_node pointing to device tree node and NOT being instantiated
from DT description...

>> +		return (unsigned int)match->data;
>> +	}
>>  
>> -	type = platform_get_device_id(pdev)->driver_data;
>> -	return type == TYPE_S3C2440;
>> +	return platform_get_device_id(pdev)->driver_data;
>>  }


Regards,

-- 

Karol Lewandowski | Samsung Poland R&D Center | Linux/Platform