Re: [PATCH v2] platform/x86: int3472: Evaluate device's _DSM method to control imaging clock

[Hans de Goede <hdegoede@redhat.com>]

HI,

On 5/31/23 19:56, Andy Shevchenko wrote:
> On Wed, May 31, 2023 at 4:44 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> From: Bingbu Cao <bingbu.cao@intel.com>
>>
>> On some platforms, the imaging clock should be controlled by evaluating
>> specific clock device's _DSM method instead of setting gpio, so this
>> change register clock if no gpio based clock and then use the _DSM method
>> to enable and disable clock.
> 
> ...
> 
>> +       if (clk->ena_gpio)
>> +               gpiod_set_value_cansleep(clk->ena_gpio, 1);
>> +       else
>> +               skl_int3472_enable_clk_acpi_method(clk, 1);
> 
> Looking at this, can we avoid duplicative validation of the GPIO?
> Perhaps skl_int3472_enable_clk_acpi_method() can have embedded another
> check so they won't be called together?
> 
> ...
> 
>> +       if (clk->ena_gpio)
>> +               gpiod_set_value_cansleep(clk->ena_gpio, 0);
>> +       else
>> +               skl_int3472_enable_clk_acpi_method(clk, 0);
> 
> Ditto.

Ack, I've squashed a fix for this into this patch while merging it into
my review-hans branch.

Regards,

Hans