[PATCH V2 2/4] pinctrl: Add SPEAr pinctrl drivers

[viresh.kumar@st.com (Viresh Kumar)]

On 4/5/2012 3:41 AM, Stephen Warren wrote:
> On 04/04/2012 05:35 AM, Viresh Kumar wrote:
>> +int __devinit spear_pinctrl_probe(struct platform_device *pdev,
>> +		struct spear_pinctrl_machdata *machdata)
>> +{
> ...
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> ...
>> +	pmx->vbase = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> 
> If this driver is DT-only, you can replace those two calls with of_iomap().

Ya. This is DT-only. Will do that.

>> +	platform_set_drvdata(pdev, pmx);
> 
> This should probably before the pinctrl_register() call, just in case
> pinctrl_register() starts calling the pinctrl driver ops immediately,
> and the drvdata is needed by the callbacks.

I don't use platform_get_drvdata in ops. Only used in remove routine.
Still would move it up.

> (Yes, I should fix up the Tegra driver for both of those)

:)

>> +	dev_info(&pdev->dev, "Registered with virtual address 0x%08x, physical address 0x%08x\n",
>> +			(u32)pmx->vbase, res->start);
> 
> Is that useful?

It does. Can easily check from boot prints that everything worked as planned.

>> +int __devexit spear_pinctrl_remove(struct platform_device *pdev)
>> +{
>> +	struct spear_pmx *pmx = platform_get_drvdata(pdev);
>> +
>> +	platform_set_drvdata(pdev, NULL);
> 
> You don't need to do that; nothing should be touching the drvdata once
> the driver has been removed, so the value doesn't matter.

Why do most of the drivers do this? What can get called for them?

-- 
viresh