Re: EP93xx PIO IDE driver proposal

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ryan Mallon escreveu:
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:

As for the platform_driver_probe, I will fix that then.

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.

I was hoping for that patch to be merged to use the functions provided to write Syscon locked registers.
In the meanwhile, as it isn't in the mainline kernel yet, I used this form.

	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.

Hartley, will you handle this then, or do you want me to submit a separate patch for the renewed 'ep93xx_ide_on_gpio' function?
I can do this and test it already with the IDE patch to see if it's OK.

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



--
************************************************************************

   João Ramos        <joao.ramos@xxxxxxx>

   INOV INESC Inovação - ESTG Leiria
   Escola Superior de Tecnologia e Gestão de Leiria
   Edíficio C1, Campus 2
   Morro do Lena, Alto do Vieiro
   Leiria
   2411-901 Leiria
   Portugal

   Tel: +351244843424
   Fax: +351244843424

************************************************************************

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux