[PATCH v5 11/17] ARM: sa1100: make collie use new locomo drivers
Russell King - ARM Linux
linux at arm.linux.org.uk
Sun Jun 14 08:06:48 PDT 2015
On Mon, Jun 08, 2015 at 11:56:42PM +0300, Dmitry Eremin-Solenikov wrote:
> +static struct gpio collie_uart_gpio[] = {
> + { COLLIE_GPIO_CTS, GPIOF_IN, "CTS" },
> + { COLLIE_GPIO_RTS, GPIOF_OUT_INIT_LOW, "RTS" },
> + { COLLIE_GPIO_DTR, GPIOF_OUT_INIT_LOW, "DTR" },
> + { COLLIE_GPIO_DSR, GPIOF_IN, "DSR" },
> +};
These should probably be given a better name rather than just "CTS" -
maybe something that gives a clue as to what UART they refer to?
> static void collie_uart_set_mctrl(struct uart_port *port, u_int mctrl)
> {
> - if (mctrl & TIOCM_RTS)
> - locomo_gpio_write(&collie_locomo_device.dev, LOCOMO_GPIO_RTS, 0);
> - else
> - locomo_gpio_write(&collie_locomo_device.dev, LOCOMO_GPIO_RTS, 1);
> -
> - if (mctrl & TIOCM_DTR)
> - locomo_gpio_write(&collie_locomo_device.dev, LOCOMO_GPIO_DTR, 0);
> - else
> - locomo_gpio_write(&collie_locomo_device.dev, LOCOMO_GPIO_DTR, 1);
> + if (!collie_uart_gpio_ok) {
> + int rc = gpio_request_array(collie_uart_gpio,
> + ARRAY_SIZE(collie_uart_gpio));
> + if (rc)
> + pr_err("collie_uart_set_mctrl: gpio request %d\n", rc);
> + else
> + collie_uart_gpio_ok = true;
> + }
> +
> + if (collie_uart_gpio_ok) {
This seems to be a repeated chunk of code. Maybe:
static bool collie_mctrl_present(void)
{
static bool collie_uart_mctrl_claimed;
if (!collie_uart_mctrl_claimed) {
int rc = gpio_request_array(collie_uart_gpio,
ARRAY_SIZE(collie_uart_gpio));
if (rc)
pr_err("%s: gpio_request_array() failed: %d\n",
__func__, rc);
else
collie_uart_mctrl_claimed = true;
}
return collie_uart_mctrl_claimed;
}
static void collie_uart_set_mctrl(struct uart_port *port, u_int mctrl)
{
if (collie_mctrl_present()) {
> + gpio_set_value(COLLIE_GPIO_RTS, !(mctrl & TIOCM_RTS));
> + gpio_set_value(COLLIE_GPIO_DTR, !(mctrl & TIOCM_DTR));
> + }
> }
>
> static u_int collie_uart_get_mctrl(struct uart_port *port)
> {
> int ret = TIOCM_CD;
> - unsigned int r;
>
> - r = locomo_gpio_read_output(&collie_locomo_device.dev, LOCOMO_GPIO_CTS & LOCOMO_GPIO_DSR);
> - if (r == -ENODEV)
> + if (!collie_uart_gpio_ok) {
> + int rc = gpio_request_array(collie_uart_gpio,
> + ARRAY_SIZE(collie_uart_gpio));
> + if (rc)
> + pr_err("collie_uart_get_mctrl: gpio request %d\n", rc);
> + else
> + collie_uart_gpio_ok = true;
> + }
> +
> + if (!collie_uart_gpio_ok)
> return ret;
> - if (r & LOCOMO_GPIO_CTS)
> +
> + if (gpio_get_value(COLLIE_GPIO_CTS))
> ret |= TIOCM_CTS;
> - if (r & LOCOMO_GPIO_DSR)
> + if (gpio_get_value(COLLIE_GPIO_DSR))
> ret |= TIOCM_DSR;
>
> return ret;
And this would become:
int TIOCM_CD;
if (collie_mctrl_present()) {
if (gpio_get_value(COLLIE_GPIO_CTS))
ret |= TIOCM_CTS;
if (gpio_get_value(COLLIE_GPIO_DSR))
ret |= TIOCM_DSR;
}
return ret;
which kind'a looks neater, and avoids duplicating the GPIO claiming.
> @@ -191,33 +216,35 @@ static struct sa1100_port_fns collie_port_fns __initdata = {
> .get_mctrl = collie_uart_get_mctrl,
> };
>
> -static int collie_uart_probe(struct locomo_dev *dev)
> -{
> - return 0;
> -}
> -
> -static int collie_uart_remove(struct locomo_dev *dev)
> -{
> - return 0;
> -}
> +static struct regulator_consumer_supply collie_amp_on_consumer_supplies[] = {
> + REGULATOR_SUPPLY("VCC", "1-004e"),
> +};
>
> -static struct locomo_driver collie_uart_driver = {
> - .drv = {
> - .name = "collie_uart",
> +static struct regulator_init_data collie_amp_on_init_data = {
> + .constraints = {
> + .name = "AMP_ON",
> + .valid_ops_mask = REGULATOR_CHANGE_STATUS,
> },
> - .devid = LOCOMO_DEVID_UART,
> - .probe = collie_uart_probe,
> - .remove = collie_uart_remove,
> + .consumer_supplies = collie_amp_on_consumer_supplies,
> + .num_consumer_supplies = ARRAY_SIZE(collie_amp_on_consumer_supplies),
> };
>
> -static int __init collie_uart_init(void)
> -{
> - return locomo_driver_register(&collie_uart_driver);
> -}
> -device_initcall(collie_uart_init);
> -
> -#endif
> +static struct fixed_voltage_config collie_amp_on_data = {
> + .supply_name = "amp_on",
> + .microvolts = 3300000,
> + .gpio = COLLIE_GPIO_AMP2_ON,
> + .startup_delay = 5,
> + .enable_high = 1,
> + .init_data = &collie_amp_on_init_data,
> +};
>
> +static struct platform_device collie_amp_on_device = {
> + .name = "reg-fixed-voltage",
> + .id = -1,
> + .dev = {
> + .platform_data = &collie_amp_on_data,
> + },
> +};
>
> static struct resource locomo_resources[] = {
> [0] = DEFINE_RES_MEM(0x40000000, SZ_8K),
> @@ -225,14 +252,15 @@ static struct resource locomo_resources[] = {
> };
>
> static struct locomo_platform_data locomo_info = {
> - .irq_base = IRQ_BOARD_START,
> + .gpio_base = COLLIE_LOCOMO_GPIO_BASE,
> + .comadj = 128,
Using spaces for what looks like pointless indentation. Please either
align the = signs using tabs, or don't bother at all. As the rest of
the file's style is to align the = signs using tabs, please remain
consistent with the rest of the file unless you are intending to
reformat it - in which case, the reformatting should happen as the
very first patch.
> };
>
> -struct platform_device collie_locomo_device = {
> +static struct platform_device collie_locomo_device = {
> .name = "locomo",
> .id = 0,
> .dev = {
> - .platform_data = &locomo_info,
> + .platform_data = &locomo_info,
You seem to replace a tab with two spaces here...
> },
> .num_resources = ARRAY_SIZE(locomo_resources),
> .resource = locomo_resources,
> @@ -270,7 +298,55 @@ static struct platform_device collie_gpio_keys_device = {
> },
> };
>
> +static int collie_mmc_init(struct device *dev,
> + irqreturn_t (*isr)(int, void*), void *mmc)
> +{
> + int ret;
> +
> + ret = gpio_request(COLLIE_GPIO_CARD_POWER, "MMC power");
> + if (!ret)
> + ret = gpio_direction_output(COLLIE_GPIO_CARD_POWER, 0);
> + if (ret)
> + gpio_free(COLLIE_GPIO_CARD_POWER);
> + return ret;
Why not use gpio_request_one() here? This whole function could be
collapsed to just:
return gpio_request_one(COLLIE_GPIO_CARD_POWER, GPIOF_OUT_INIT_LOW,
"MMC power");
> +}
> +
> +static void collie_mmc_exit(struct device *dev, void *mmc)
> +{
> + gpio_free(COLLIE_GPIO_CARD_POWER);
> +}
> +
> +static void collie_mmc_setpower(struct device *dev, unsigned int mask)
> +{
> + gpio_set_value(COLLIE_GPIO_CARD_POWER, !!mask);
> +}
> +
> +static struct mmc_spi_platform_data collie_mmc_data = {
> + .init = collie_mmc_init,
> + .exit = collie_mmc_exit,
> + .setpower = collie_mmc_setpower,
> + .detect_delay = 200,
> + .powerup_msecs = 200,
> + .ocr_mask = MMC_VDD_32_33 | MMC_VDD_33_34,
> + .flags = MMC_SPI_USE_CD_GPIO | MMC_SPI_USE_RO_GPIO,
> + .cd_gpio = COLLIE_GPIO_CARD_DETECT,
> + .ro_gpio = COLLIE_GPIO_CARD_RO,
> + .caps2 = MMC_CAP2_RO_ACTIVE_HIGH,
> +};
> +
> +static struct spi_board_info collie_spi_board_info[] __initdata = {
> + {
> + .modalias = "mmc_spi",
> + .platform_data = &collie_mmc_data,
> + .max_speed_hz = 25000000,
> + .bus_num = 0,
> + .chip_select = 0,
> + .mode = SPI_MODE_0,
> + },
> +};
> +
> static struct platform_device *devices[] __initdata = {
> + &collie_amp_on_device,
> &collie_locomo_device,
> &colliescoop_device,
> &collie_power_device,
> @@ -347,10 +423,39 @@ static struct sa1100fb_mach_info collie_lcd_info = {
>
> .lccr0 = LCCR0_Color | LCCR0_Sngl | LCCR0_Act,
> .lccr3 = LCCR3_OutEnH | LCCR3_PixRsEdg | LCCR3_ACBsDiv(2),
> +};
>
> -#ifdef CONFIG_BACKLIGHT_LOCOMO
> - .lcd_power = locomolcd_power
> -#endif
> +static struct iio_map locomo_iio_map[] = {
> + {
> + .consumer_dev_name = "locomo-lcd.0",
> + .consumer_channel = "comadj",
> + .adc_channel_label = "CH0",
> + },
> + { }
> +};
> +
> +static struct i2c_board_info locomo_i2c_devs[] __initdata = {
> + {
> + I2C_BOARD_INFO("m62332", 0x4e),
> + .platform_data = locomo_iio_map,
> + },
> +};
> +
> +static struct gpiod_lookup_table collie_bl_gpios_table = {
> + .dev_id = "locomo-backlight.0",
> + .table = {
> + GPIO_LOOKUP("locomo-gpio", 9, "flvr", GPIO_ACTIVE_HIGH),
> + { },
> + },
> +}, collie_lcd_gpios_table = {
Please don't do this. Please phrase this instead as:
};
static struct gpiod_lookup_table collie_lcd_gpios_table = {
> + .dev_id = "locomo-lcd.0",
> + .table = {
> + GPIO_LOOKUP("locomo-gpio", 4, "VSHA", GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP("locomo-gpio", 5, "VSHD", GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP("locomo-gpio", 6, "Vee", GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP("locomo-gpio", 7, "MOD", GPIO_ACTIVE_HIGH),
> + { },
> + },
> };
...
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
More information about the linux-arm-kernel
mailing list