On Thu, Aug 11, 2011 at 09:26:05AM +0530, Inderpal Singh wrote:
> +static struct regulator_consumer_supply __initdata ldo7_consumer[] = {
> + REGULATOR_SUPPLY("avdd", "soc-audio"), /* Reatek ALC5625*/
> +};
Ick, no. The soc-audio device is a virtual device within Linux and is
being phased out, any driver adding new soc-audio devices will be
rejected. The CODEC driver should deal with its own power.
> +static struct regulator_init_data __initdata max8997_ldo3_data = {
> + .constraints = {
> + .name = "VMIPI_1.1V",
> + .min_uV = 1100000,
> + .max_uV = 1100000,
> + .apply_uV = 1,
> + .always_on = 1,
> + .valid_ops_mask = REGULATOR_CHANGE_STATUS,
The regulator is always enabled but the consumers can change its status?
> + .name = "DVDD_SWB_2.8V",
> + .min_uV = 2800000,
> + .max_uV = 2800000,
> + .apply_uV = 1,
> + .always_on = 1,
> + .valid_ops_mask = REGULATOR_CHANGE_VOLTAGE,
The regulator has a fixed voltage but the consumers can change it?
> +static struct i2c_board_info i2c0_devs[] __initdata = {
> +[I2C0_MAX8997] = {
> + I2C_BOARD_INFO("max8997", (0xCC >> 1)),
> + .platform_data = &origen_max8997_pdata,
> + },
> +};
Why are you assigning the array index? That's very unusual.
> +static void __init origen_power_init(void)
> +{
> + int gpio;
> + int irq_base = IRQ_GPIO_END + 1;
> +
> + origen_max8997_pdata.irq_base = irq_base;
Why is this not just set statically in the pdata?
> + gpio = EXYNOS4_GPX0(4);
> + s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));
> + s3c_gpio_setpull(gpio, S3C_GPIO_PULL_NONE);
A define for the GPIO would be a bit more common.
> + s3c_i2c0_set_platdata(NULL);
> + i2c0_devs[I2C0_MAX8997].irq = gpio_to_irq(EXYNOS4_GPX0(4));
There should be defines to do this statically.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
