Re: EP93xx PIO IDE driver proposal

[Ryan Mallon <ryan@bluewatersys.com>]

H Hartley Sweeten wrote:
> On Thursday, May 07, 2009 3:51 PM, Ryan Mallon wrote:
>>>> You can (and should) use platform_driver_probe and still have
>>>> the module be loadable. For the gpio settings, I think that
>>>> probably the best approach is to use gpio_request for each of
>>>> the gpio pins so that gpio_lib knows that they are in use and
>>>> sysfs/debugfs will correctly show they are assigned to ide,
>>>> and then call ep93xx_ide_on_gpio, so you will have something
>>>> like in your init code:
>>> Hmmm...
>>>
>>> How about doing all the request/free's in the ep93xx_ide_on_gpio()
>>> function?  That way the platform code can initially call it and set
>>> all the pins into gpio mode (freeing them).  Then later when the
>>> IDE driver is loaded it can call the function to try and put the
>>> pins into IDE mode (requesting them).  If any of the pins are
>>> unavailable the ep93xx_ide_on_gpio() function can return the
>>> necessary error code preventing the IDE driver from loading.  When
>>> the driver is unloaded it just needs to call the core to free
>>> the gpio's.
>>>
>>> Something like:
>>>
>>> int ep93xx_ide_on_gpio(int enable)
>>> {
>>> 	u32 reg;
>>> 	int err;
>>> 	int i;
>>>
>>> 	reg = __raw_read(EP93XX_SYSCON_DEVICE_CONFIG);
>>>
>>> 	if (enable) {
>>> 		for (i = 0; i < 8; i++) {
>>> 			err = gpio_request(EP93XX_GPIO_LINE_E(i), "ep93xx_ide");
>>> 			if (err)
>>> 				goto fail_gpio_e;
>>> 			err = gpio_request(EP93XX_GPIO_LINE_F(i), "ep93xx_ide");
>>> 			if (err)
>>> 				goto fail_gpio_f;
>>> 			err = gpio_request(EP93XX_GPIO_LINE_G(i), "ep93xx_ide");
>>> 			if (err)
>>> 				goto fail_gpio_g;
>>> 		}
>>> 		reg &= ~(EP93XX_SYSCON_DEVICE_CONFIG_EONIDE |
>>> 			EP93XX_SYSCON_DEVICE_CONFIG_GONIDE |
>>> 			EP93XX_SYSCON_DEVICE_CONFIG_HONIDE);
>>> 	} else {
>>> 		for (i = 0; i < 8; i++) {
>>> 			gpio_free(EP93XX_GPIO_LINE_E(i));
>>> 			gpio_free(EP93XX_GPIO_LINE_F(i));
>>> 			gpio_free(EP93XX_GPIO_LINE_G(i));
>>> 		}
>>> 		reg |= (EP93XX_SYSCON_DEVICE_CONFIG_EONIDE |
>>> 			EP93XX_SYSCON_DEVICE_CONFIG_GONIDE |
>>> 			EP93XX_SYSCON_DEVICE_CONFIG_HONIDE);
>>> 	}
>>> 	
>>> 	__raw_writel(0xaa, EP93XX_SYSCON_SWLOCK);
>>> 	__raw_writel(reg, EP93XX_SYSCON_DEVICE_CONFIG);	
>> Is this going to be modified later to use the ep93xx_syscon_ functions
>> once they are merged?
> 
> Yes, hopefully that occurs sometime soon.  Still no responses on it.
> 
>>> 	return 0;
>>>
>>> fail_gpio_g:
>>> 	free_gpio(EP93XX_GPIO_LINE_F(i);
>>> fail_gpio_f:
>>> 	free_gpio(EP93XX_GPIO_LINE_E(i);
>>> fail_gpio_e:
>>> 	for ( ; i >= 0; --i) {
>>> 		free_gpio(EP93XX_GPIO_LINE_E(i);
>>> 		free_gpio(EP93XX_GPIO_LINE_F(i);
>>> 		free_gpio(EP93XX_GPIO_LINE_G(i);
>>> 	}
>>> 	return err;
>>> }
>> Yeah, that makes sense. Keeps the driver nice and clean that way too.
>> The ep93xx_ide_on_gpio function for core.c should be posted as a
>> separate patch though.
>>
> 
> I agree.  Actually that patch needs to go in now so that the port E/F/G
> pins will work for normal GPIO.  The syscon register defaults at reset with
> the bits cleared so the boot state has them set for IDE mode.  That drove
> me nuts for quite a while...
> 
> If wanted I will put together the necessary patch for this.

That would be good. I think that we should default all of the pins to
gpio mode on boot, and then drivers should call into core.c to put pins
in alternative function mode where necessary.

> This function will hopefully get a bit cleaner later.  I've been messing
> with an addition to the GPIOLIB API to add support for "ports".  When/if
> that goes in all the for() loops go away and you just need to do a
> gpio_port_request(...) to grab the pins and gpio_port_free(...) to put
> them back.

It would be nice if gpiolib had some understanding of alternative
function modes for gpios. However I'm not sure that it can be done in a
generic way that will suit all architectures/chips.

> Ahh.. If only the days were longer....

Agreed :-).

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

       Ryan Mallon                              Unit 5, Amuri Park
       Phone: +64 3 3779127                     404 Barbadoes St
       Fax:   +64 3 3779135                     PO Box 13 889
       Email: ryan@bluewatersys.com             Christchurch, 8013
       Web:   http://www.bluewatersys.com       New Zealand
       Freecall Australia  1800 148 751         USA 1800 261 2934