On 10/01/2013 02:43 PM, Thierry Reding wrote:
> On Tue, Oct 01, 2013 at 12:31:04PM -0600, Stephen Warren wrote:
>> On 09/23/2013 03:41 PM, Thierry Reding wrote:
>>> The GPIO API defines 0 as being a valid GPIO number, so this
>>> field needs to be initialized explicitly.
>>
>>> static void __init smdkv210_map_io(void)
>>
>>> @@ -70,6 +70,7 @@ static struct samsung_bl_drvdata
>>> samsung_dfl_bl_data __initdata = { .max_brightness = 255,
>>> .dft_brightness = 255, .pwm_period_ns = 78770, + .enable_gpio
>>> = -1, .init = samsung_bl_init, .exit =
>>> samsung_bl_exit, }, @@ -121,6 +122,10 @@ void __init
>>> samsung_bl_set(struct samsung_bl_gpio_info *gpio_info,
>>> samsung_bl_data->lth_brightness = bl_data->lth_brightness; if
>>> (bl_data->pwm_period_ns) samsung_bl_data->pwm_period_ns =
>>> bl_data->pwm_period_ns; + if (bl_data->enable_gpio) +
>>> samsung_bl_data->enable_gpio = bl_data->enable_gpio; + if
>>> (bl_data->enable_gpio_flags) +
>>> samsung_bl_data->enable_gpio_flags =
>>> bl_data->enable_gpio_flags;
>>
>> Won't this cause the core pwm_bl driver to request/manipulate the
>> GPIO, whereas this driver already does that inside the
>> samsung_bl_init/exit callbacks? I think you either need to adjust
>> those callbacks, or not set the new standard GPIO property in
>> samsung_bl_data.
>
> I don't think so. The samsung_bl_data is a copy of
> samsung_dfl_bl_data augmented by board-specific settings. So in
> fact copying these values here is essential to allow boards to
> override the enable_gpio and flags fields. Currently no board sets
> the enable_gpio to a valid GPIO so it's all still handled by the
> callbacks only.
Oh yes, you're right. I was confusing the new enable_gpio field in
pwm_bl's platform data with some other field in a custom data structure.
One minor point though:
>>> + if (bl_data->enable_gpio) + samsung_bl_data->enable_gpio =
>>> bl_data->enable_gpio;
That assumes that enable_gpio==0 means "none", whereas you've gone to
great pains in the rest of the series to allow 0 to be a valid GPIO
ID. right now, the default value of samsung_bl_data->enable_gpio is
-1, and if !bl_data->enable_gpio, that value won't be propagated across.
--
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
