Re: [PATCH 01/10] GPIO: gpio-generic: Move initialization up to postcore

[Janusz Krzysztofik]

(adding Greg Kroah-Hartman and linux-serial@xxxxxxxxxxxxxxx to Cc:)

On Monday 12 of December 2011 at 19:04:56, Tony Lindgren wrote:
> * Janusz Krzysztofik <jkrzyszt@xxxxxxxxxxxx> [111211 11:41]:
> > This will allow boards with custom memory mapped GPIO ports to set up
> > and use those port pins while initializing devices from arch init.
> 
> Care to explain a bit more why you need to initialize those devices
> early on?

Let me try.

>From patch 2/10 "ARM: OMAP1: ams-delta: Convert latches to 
basic_mmio_gpio":

> @@ -307,8 +487,8 @@ static void __init ams_delta_init(void)
>  	omap_serial_init();
>  	omap_register_i2c_bus(1, 100, NULL, 0);
>  
> -	/* Clear latch2 (NAND, LCD, modem enable) */
> -	ams_delta_latch2_write(~0, 0);
> +	platform_add_devices(_latch_devices, ARRAY_SIZE(_latch_devices));
> +	BUG_ON(gpio_request_array(_latch_gpios, ARRAY_SIZE(_latch_gpios)));
>  
>  	omap1_usb_init(&ams_delta_usb_config);
>  	omap1_set_camera_info(&ams_delta_camera_platform_data);

Here I'm accessing the latches from ams_delta_init() (.init_machine 
hook) with gpio_request_array() in order to:
a) clear their contents,
b) avoid accessing them, from ams_delta_latch_write(), never requested.

I could imagine clearing their contents with something like

	*(volatile __u8 *) AMS_DELTA_LATCH2_VIRT = 0;
	*(volatile __u16 *) AMS_DELTA_LATCH2_VIRT = 0;

instead, i.e., the way the old code used to, than accessing those GPIO 
pins not requested, until they are finally requested by drivers updated 
to use gpiolib with successive patche. Would this be acceptable?

However,
>From patch 6/10 "ARM: OMAP1: ams-delta: Use GPIO API in modem setup":

> @@ -482,6 +472,12 @@ static void __init ams_delta_init(void)
>  	omap_writew(omap_readw(ARM_RSTCT1) | 0x0004, ARM_RSTCT1);
>  }
>  
> +static void _modem_pm(struct uart_port *port, unsigned int state, unsigned old)
> +{
> +	if (state != old)
> +		gpio_set_value(AMS_DELTA_GPIO_PIN_MODEM_NRESET, state == 0);
> +}
> +
>  static struct plat_serial8250_port ams_delta_modem_ports[] = {
>  	{
>  		.membase	= IOMEM(_MODEM_VIRT),
> @@ -492,6 +488,7 @@ static struct plat_serial8250_port ams_delta_modem_ports[] = {
>  		.iotype		= UPIO_MEM,
>  		.regshift	= 1,
>  		.uartclk	= BASE_BAUD * 16,
> +		.pm		= _modem_pm,
>  	},
>  	{ },
>  };
> @@ -504,6 +501,24 @@ static struct platform_device ams_delta_modem_device = {
>  	},
>  };
>  
> +static struct gpio _modem_gpios[] __initconst = {
> +	{
> +		.gpio	= AMS_DELTA_GPIO_PIN_MODEM_IRQ,
> +		.flags	= GPIOF_DIR_IN,
> +		.label	= "modem_irq",
> +	},
> +	{
> +		.gpio	= AMS_DELTA_GPIO_PIN_MODEM_NRESET,
> +		.flags	= GPIOF_OUT_INIT_HIGH,
> +		.label	= "modem_nreset",
> +	},
> +	{
> +		.gpio	= AMS_DELTA_GPIO_PIN_MODEM_CODEC,
> +		.flags	= GPIOF_OUT_INIT_HIGH,
> +		.label	= "modem_codec",
> +	},
> +};
> +
>  static int __init ams_delta_modem_init(void)
>  {
>  	int err;
> @@ -515,16 +530,11 @@ static int __init ams_delta_modem_init(void)
>  	ams_delta_modem_ports[0].irq =
>  			gpio_to_irq(AMS_DELTA_GPIO_PIN_MODEM_IRQ);
>  
> -	err = gpio_request(AMS_DELTA_GPIO_PIN_MODEM_IRQ, "modem");
> +	err = gpio_request_array(_modem_gpios, ARRAY_SIZE(_modem_gpios));
>  	if (err) {
> -		pr_err("Couldn't request gpio pin for modem\n");
> +		pr_err("Couldn't request gpio pins for modem\n");
>  		return err;
>  	}
> -	gpio_direction_input(AMS_DELTA_GPIO_PIN_MODEM_IRQ);
> -
> -	ams_delta_latch2_write(
> -		AMS_DELTA_LATCH2_MODEM_NRESET | AMS_DELTA_LATCH2_MODEM_CODEC,
> -		AMS_DELTA_LATCH2_MODEM_NRESET | AMS_DELTA_LATCH2_MODEM_CODEC);
>  
>  	return platform_device_register(&ams_delta_modem_device);
>  }

Before the change, only one GPIO pin, that IRQ, driven with omap_gpio 
driver which is registered from postcore, was gpio_request()ed. Now, two 
more, gpio-generic driven pins, are also requested _and_ initialized in 
order to make the modem accessible. The ams_delta_modem_init() is 
installed with arch_initcall().

I could imagine initializing those modem pins the old way I've suggested 
above, but I can see no good solution to delegate calling of that 
gpio_request*() to the 8250 driver. For me, the only hook passed to the 
driver with platform_data that can be suitable is the .pm hook. However, 
when I tried to remove powering up the modem (rising up 
AMS_DELTA_GPIO_PIN_MODEM_NRESET) from the arch init and do this only 
from that .pm hook, the device was not detected, so I wonder if and when 
that hook is called, and if it is before probe, then perhaps powering 
up the modem chip from there is too late for the device to get up before 
being examined. But then, maybe if we initialize the pin the old way 
from arch init, it would be enough for the device to be detected, while 
consecutive open() response times may not be so critical. Perhaps Greg 
or another serial guru can shed some more light on this.

But this way or another, we are talking about dirty hacks here IMHO.

Thanks,
Janusz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/