Re: [PATCH 1/2] platform/x86: int3472: Rework regulator enable-time handling

[Hans de Goede <hansg@kernel.org>]

Hi Andy,

Thank you for your review.

On 25-Jul-25 4:49 PM, Andy Shevchenko wrote:
> On Fri, Jul 25, 2025 at 4:44 PM Hans de Goede <hansg@kernel.org> wrote:
>>
>> Instead of hardcoding the regulator enable-time for INT3472_GPIO_TYPE-
>> POWER_ENABLE and -HANDSHAKE, make int3472_get_con_id_and_polarity()
>> set the enable-time.
>>
>> This will allow overriding the enable time through quirks in
>> the int3472_gpio_map[].
> 
> ...
> 
>> + * @enable_time: Enable time in usec for GPIOs mapped to regulators
>>   * GPIO_ACTIVE_HIGH otherwise
> 
> Make it enable_time_us, it will immediately give a hint in the code
> without going to here to get units.
> 
> ...
> 
>>  struct int3472_gpio_map {
> 
>>         u8 type_to;
>>         bool polarity_low;
>>         const char *con_id;
>> +       unsigned int enable_time;
> 
> Wondering if this can be moved before con_id to save a few bytes on 64-bit.
> 
>>  };
> 
> ...
> 
>>  static const struct int3472_gpio_map int3472_gpio_map[] = {
>>         /* mt9m114 designs declare a powerdown pin which controls the regulators */
>> -       { "INT33F0", INT3472_GPIO_TYPE_POWERDOWN, INT3472_GPIO_TYPE_POWER_ENABLE, false, "vdd" },
>> +       { "INT33F0", INT3472_GPIO_TYPE_POWERDOWN, INT3472_GPIO_TYPE_POWER_ENABLE,
>> +         false, "vdd", GPIO_REGULATOR_ENABLE_TIME },
>>         /* ov7251 driver / DT-bindings expect "enable" as con_id for reset */
>>         { "INT347E", INT3472_GPIO_TYPE_RESET, INT3472_GPIO_TYPE_RESET, false, "enable" },
> 
> Can we actually either use a macro or at least C99 initializers?
> 
>>  };
> 
> ...
> 
>> +                                           unsigned int *enable_time)
> 
> _us
> 
> ...
> 
>> +       *enable_time = GPIO_REGULATOR_ENABLE_TIME;
> 
> _us
> _US

Ack,

I agree with all your remarks. I'll fix these for v2.

Regards,

Hans