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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 6, 2023 at 12:23 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> On 5/31/23 19:56, Andy Shevchenko wrote:
> > On Wed, May 31, 2023 at 4:44 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:

...

> >> +       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.

Now you have two different checks for the same, I would suggest that
you switch to check clk->cl in the skl_int3472_enable_clk()
as it's done in the skl_int3472_register_dsm_clock() instead of GPIO.
Other way around is also possible but it seems to me that checking for
clock existence has better guarantees than just checking for GPIO
availability.

-- 
With Best Regards,
Andy Shevchenko




[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux