[PATCH 2/7] spi-bcm2708: use pinconf to change pin mode

[Chris Boot]

On 28/05/12 07:10, Stephen Warren wrote:
> On 05/26/2012 03:19 PM, Chris Boot wrote:
>
>> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
>>   config SPI_BCM2708
>>   	tristate "BCM2708 SPI controller driver (SPI0)"
>>   	depends on MACH_BCM2708
>> +	select PINCONF
>> +	select PINCTRL_BCM2708
> The SPI driver is using PINCTRL; it is up to the architecture to provide
> the correct pinctrl driver.
>
> So, ARCH_BCM2708 should "select PINCTRL ; select PINCTRL_BCM2708". In
> turn, PINCTRL_BCM2708 should select this pinctrl features it needs;
> PINMUX and PINCONF.
>
> So finally, SPI_BCM2708 only interacts with the main PINCTRL API itself,
> so it should only "depends PINCTRL", or perhaps "select PINCTRL".

Yes, that makes more sense to me. Simon, do you want me to take the arch 
patch for this through my tree?

>> diff --git a/drivers/spi/spi-bcm2708.c b/drivers/spi/spi-bcm2708.c
>> @@ -468,13 +445,18 @@ static int __devinit bcm2708_spi_probe(struct platform_device *pdev)
>>   		return PTR_ERR(clk);
>>   	}
>>
>> -	bcm2708_init_pinmode();
>> +	pctl = pinctrl_get_select_default(&pdev->dev);
>> +	if (IS_ERR(pctl)) {
>> +		err = PTR_ERR(pctl);
>> +		dev_err(&pdev->dev, "could not set up pinctrl: %d\n", err);
>> +		goto out_clk_put;
>> +	}
> It'd be best to use devm_pincrl_get_select_default() here instead,
> because then you wouldn't have to:
>
> * call pinctrl_put() in the probe() error path, or in remove(), because
> the device management core will do that for you.
> * store the returned value in struct bcm2708_spi.

I completely forgot the devm_* functions existed at all! I could use 
those for the memory regions and kmalloc (in i2c) as well, among others. 
Thanks!

Cheers,
Chris

-- 
Chris Boot
bootc at bootc.net