Hi Igor,
On 15.12.2011 14:40, Igor Grinberg wrote:
>> + r = gpio_request_array(mcx_dss_gpios, ARRAY_SIZE(mcx_dss_gpios));
>> + if (r) {
>> + pr_err("failed to get DSS control GPIOs\n");
>> + return;
>> + }
>> +
>> + omap_mux_init_gpio(LCD_BKLIGHT_EN, OMAP_PIN_OUTPUT);
>> + omap_mux_init_gpio(LCD_LVL_SFHT_BUF_ENn, OMAP_PIN_OUTPUT);
>> + omap_mux_init_gpio(LCD_PWR_ENn, OMAP_PIN_OUTPUT);
>> + omap_mux_init_gpio(HDMI_TRCVR_PDn, OMAP_PIN_OUTPUT);
>
> Shouldn't you mux the pins, before you access the GPIO
> (e.g. before the gpio_request_array()).
> Are there any safety problems?
No, there are no problems. I will move mux settings above
gpio_request_array() call.
>> +static struct omap2_hsmmc_info mmc[] = {
>> + {
>> + .mmc = 1,
>> + .caps = MMC_CAP_4_BIT_DATA,
>> + .gpio_cd = SD_CARD_CD,
>> + .gpio_wp = SD_CARD_WP,
>> + .ocr_mask = MMC_VDD_32_33 | MMC_VDD_33_34 |
>> + MMC_VDD_165_195,
>
> The ocr_mask will be overridden, by the following patch:
> -----------------
> commit e89715a7e48d505f42813a4e3ee0f0efb49832ba
> Author: Abhilash K V <abhilash.kv@xxxxxx>
> Date: Fri Dec 9 12:27:36 2011 -0800
>
> ARM: OMAP: hsmmc: Support for AM3517 MMC1 voltages
> --------------
>
> in Tony's hsmmc branch.
>
> IMO it should be fixed, by adding a check if the ocr_mask is
> already set...
> I can't send a patch for this right now...
Well, I think I should just drop the .ocr_mask field then. Everything
works fine for me with the above mentioned patch.
>> +#ifdef CONFIG_OMAP_MUX
>> +static struct omap_board_mux board_mux[] __initdata = {
>> + OMAP3_MUX(CHASSIS_DMAREQ3, OMAP_MUX_MODE0 | OMAP_PIN_INPUT_PULLDOWN),
>> + OMAP3_MUX(UART1_TX, OMAP_MUX_MODE0 | OMAP_PIN_OUTPUT |
>> + OMAP_PULL_ENA | OMAP_PULL_UP),
>> + OMAP3_MUX(UART1_RX, OMAP_MUX_MODE0 | OMAP_PIN_INPUT),
>> + OMAP3_MUX(UART1_RTS, OMAP_MUX_MODE0 | OMAP_PIN_INPUT),
>> + OMAP3_MUX(UART1_CTS, OMAP_MUX_MODE0 | OMAP_PIN_OUTPUT |
>> + OMAP_PULL_ENA | OMAP_PULL_UP),
>
> Hmm... pullup for output? Is this needed for kind of safety?
This is a mistake indeed. Will remove the pullups.
>> +static void __init mcx_init(void)
>> +{
>> + int err;
>> +
>> + omap3_mux_init(board_mux, OMAP_PACKAGE_CBB);
>> + mcx_i2c_init();
>> + platform_add_devices(mcx_devices, ARRAY_SIZE(mcx_devices));
>> + omap_serial_init();
>
> Shouldn't this one be before the mcx_i2c_init() call?
Well, I think I've taken this order from some other board init... I
think the idea was to bring up regulator chip earlier. But I can move
serial up with no problem.
>> + mcx_display_init();
>> +
>> + /* Configure EHCI ports */
>> + err = gpio_request_one(USB_HOST_PWR_EN, GPIOF_OUT_INIT_HIGH,
>> + "USB_HOST_PWR_EN");
>> + if (err)
>> + pr_warn("Failed to request USB host power enable GPIO\n");
>
> empty line here will improve the readability.
Ok.
Regards, Ilya.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
