Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1506721

Re: [PATCH v3 1/1] x86/platform/intel-mid: Retrofit pci_platform_pm_ops ->get_state hook

Show all headers | View raw

On Sun, 2016-10-23 at 16:57 +0200, Lukas Wunner wrote:
> On Sun, Oct 23, 2016 at 01:37:55PM +0100, Bryan O'Donoghue wrote:
> > 
> The usage of a mutex in mid_pwr_set_power_state() actually seems
> questionable since this is called with interrupts disabled:

> pci_pm_resume_noirq
>   pci_pm_default_resume_early
>     pci_power_up
>       platform_pci_set_power_state
>         mid_pci_set_power_state
>           intel_mid_pci_set_power_state
>             mid_pwr_set_power_state


That was my other question then - though I assume the mutex is put in
place to future-proof the code.

I'm just wondering out loud - considering we have the case where we update a register and then spin waiting for a command completion - is it in fact logically valid to have a concurrent reader read out the power state - when another writer is executing mid_pwr_wait() - for example.

/* Wait 500ms that the latest PWRMU command finished */
static int mid_pwr_wait(struct mid_pwr *pwr)
{
        unsigned int count = 500000;
        bool busy;

        do {
                busy = mid_pwr_is_busy(pwr);
                if (!busy)
                        return 0;
                udelay(1);
        } while (--count);

        return -EBUSY;
}

static int mid_pwr_wait_for_cmd(struct mid_pwr *pwr, u8 cmd)
{
        writel(PM_CMD_CMD(cmd) | PM_CMD_CM_IMMEDIATE, pwr->regs +
PM_CMD);
        return mid_pwr_wait(pwr);
}

static int __update_power_state(struct mid_pwr *pwr, int reg, int bit,
int new)
{

<snip>
        /* Update the power state */
        mid_pwr_set_state(pwr, reg, (power & ~(3 << bit)) | (new <<
bit));

        /* Send command to SCU */
        ret = mid_pwr_wait_for_cmd(pwr, CMD_SET_CFG);
        if (ret)
                return ret;
<snip>
}

anyway...

I've tested your patch and it looks good. We can otherwise defer to
andy on the usage of the mutex.

Tested-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>

---
bod

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v3 0/1] x86/platform/intel-mid: Retrofit pci_platform_pm_ops Lukas Wunner <lukas@wunner.de> - 2016-10-23 14:00 +0200
  [PATCH v3 1/1] x86/platform/intel-mid: Retrofit pci_platform_pm_ops  ->get_state hook Lukas Wunner <lukas@wunner.de> - 2016-10-23 14:00 +0200
    Re: [PATCH v3 1/1] x86/platform/intel-mid: Retrofit  pci_platform_pm_ops ->get_state hook Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2016-10-23 14:40 +0200
      Re: [PATCH v3 1/1] x86/platform/intel-mid: Retrofit  pci_platform_pm_ops ->get_state hook Lukas Wunner <lukas@wunner.de> - 2016-10-23 17:00 +0200
        Re: [PATCH v3 1/1] x86/platform/intel-mid: Retrofit  pci_platform_pm_ops ->get_state hook Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2016-10-23 18:20 +0200
        Re: [PATCH v3 1/1] x86/platform/intel-mid: Retrofit  pci_platform_pm_ops ->get_state hook Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-10-24 11:20 +0200
          Re: [PATCH v3 1/1] x86/platform/intel-mid: Retrofit  pci_platform_pm_ops ->get_state hook Lukas Wunner <lukas@wunner.de> - 2016-10-24 12:10 +0200
            Re: [PATCH v3 1/1] x86/platform/intel-mid: Retrofit  pci_platform_pm_ops ->get_state hook Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-10-24 13:10 +0200
              Re: [PATCH v3 1/1] x86/platform/intel-mid: Retrofit  pci_platform_pm_ops ->get_state hook Lukas Wunner <lukas@wunner.de> - 2016-10-25 08:20 +0200
                Re: [PATCH v3 1/1] x86/platform/intel-mid: Retrofit  pci_platform_pm_ops ->get_state hook Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2016-10-26 16:00 +0200
                Re: [PATCH v3 1/1] x86/platform/intel-mid: Retrofit  pci_platform_pm_ops ->get_state hook Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-10-26 17:10 +0200

csiph-web