diff for duplicates of <1461575235.17131.3.camel@linux.intel.com> diff --git a/a/1.txt b/N1/1.txt index 0a0f460..2dcfff7 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -15,19 +15,19 @@ On Fri, 2016-04-22 at 16:59 +0300, Jarkko Nikula wrote: > > > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com> > > --- -> > Since v3: -> > - use runtime PM rather than rpm in commit msg -> > - remove duplicated "(" in commit msg +> > ? Since v3: +> > ???- use runtime PM rather than rpm in commit msg +> > ???- remove duplicated "(" in commit msg > > -> > Since v2: -> > - s/clk/clock -> > - describe why use pm_runtime_put_noidle() +> > ? Since v2: +> > ???- s/clk/clock +> > ???- describe why use pm_runtime_put_noidle() > > -> > Since v1: -> > - fix commit msg: "not rely on rpm" rather than "rely on rpm" -> > - call i2c_dw_plat_prepare_clk after pm_rumtime_disable() -> > drivers/i2c/busses/i2c-designware-platdrv.c | 16 ++++++++++------ -> > 1 file changed, 10 insertions(+), 6 deletions(-) +> > ? Since v1: +> > ???- fix commit msg: "not rely on rpm" rather than "rely on rpm" +> > ???- call i2c_dw_plat_prepare_clk after pm_rumtime_disable() +> > ? drivers/i2c/busses/i2c-designware-platdrv.c | 16 ++++++++++------ +> > ? 1 file changed, 10 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c > > b/drivers/i2c/busses/i2c-designware-platdrv.c @@ -36,9 +36,9 @@ On Fri, 2016-04-22 at 16:59 +0300, Jarkko Nikula wrote: > > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c > > @@ -253,8 +253,11 @@ static int dw_i2c_plat_probe(struct > > platform_device *pdev) -> > } +> > ?? } > > -> > r = i2c_dw_probe(dev); +> > ?? r = i2c_dw_probe(dev); > > - if (r && !dev->pm_runtime_disabled) > > - pm_runtime_disable(&pdev->dev); > > + if (r) { @@ -47,21 +47,21 @@ On Fri, 2016-04-22 at 16:59 +0300, Jarkko Nikula wrote: > > + i2c_dw_plat_prepare_clk(dev, false); > > + } > > -> > return r; -> > } +> > ?? return r; +> > ? } > > @@ -264,15 +267,16 @@ static int dw_i2c_plat_remove(struct > > platform_device *pdev) -> > struct dw_i2c_dev *dev = platform_get_drvdata(pdev); +> > ?? struct dw_i2c_dev *dev = platform_get_drvdata(pdev); > > -> > pm_runtime_get_sync(&pdev->dev); +> > ?? pm_runtime_get_sync(&pdev->dev); > > + pm_runtime_dont_use_autosuspend(&pdev->dev); > > + if (!dev->pm_runtime_disabled) > > + pm_runtime_disable(&pdev->dev); > > + pm_runtime_put_noidle(&pdev->dev); > > -> > i2c_del_adapter(&dev->adapter); +> > ?? i2c_del_adapter(&dev->adapter); > > -> > i2c_dw_disable(dev); +> > ?? i2c_dw_disable(dev); > > > > - pm_runtime_dont_use_autosuspend(&pdev->dev); > > - pm_runtime_put_sync(&pdev->dev); @@ -69,45 +69,45 @@ On Fri, 2016-04-22 at 16:59 +0300, Jarkko Nikula wrote: > > - pm_runtime_disable(&pdev->dev); > > + i2c_dw_plat_prepare_clk(dev, false); > > -> This feels a bit an invasive change to me for unbalanced clock -> enable/disable and I noticed this changes semantics how +> This feels a bit an invasive change to me for unbalanced clock? +> enable/disable and I noticed this changes semantics how? > drivers/acpi/acpi_lpss.c devices are shutdown when removing the -> driver. +> driver.? > Although I didn't notice does it cause any regression. > > Before patch: > 1. drivers/base/dd.c: __device_release_driver() -> - pm_runtime_get_sync() -> -> acpi_device_set_power(D0) -> acpi_lpss_restore_ctx() -> dw_i2c_plat_resume() +> ????- pm_runtime_get_sync() +> ??????-> acpi_device_set_power(D0) +> ?????????acpi_lpss_restore_ctx() +> ?????????dw_i2c_plat_resume() > 2. dw_i2c_plat_remove() -> - pm_runtime_dont_use_autosuspend() -> pm_runtime_put_sync() -> -> dw_i2c_plat_suspend() -> acpi_lpss_save_ctx() -> acpi_device_set_power(D3) +> ????- pm_runtime_dont_use_autosuspend() +> ??????pm_runtime_put_sync() +> ??????-> dw_i2c_plat_suspend() +> ?????????acpi_lpss_save_ctx() +> ?????????acpi_device_set_power(D3) > 3. __device_release_driver() continue -> - dev->pm_domain->dismiss(dev) -> -> acpi_lpss_dismiss() ... -> acpi_device_set_power(D3) +> ????- dev->pm_domain->dismiss(dev) +> ??????-> acpi_lpss_dismiss() ... -> acpi_device_set_power(D3) > > After patch: > 1. drivers/base/dd.c: __device_release_driver() -> - pm_runtime_get_sync() -> -> acpi_device_set_power(D0) -> acpi_lpss_restore_ctx() -> dw_i2c_plat_resume() +> ? - pm_runtime_get_sync() +> ????-> acpi_device_set_power(D0) +> ???????acpi_lpss_restore_ctx() +> ???????dw_i2c_plat_resume() > 2. dw_i2c_plat_remove() -> - pm_runtime_dont_use_autosuspend() -> pm_runtime_put_noidle() -> * no device suspending and acpi_lpss_save_ctx() +> ????- pm_runtime_dont_use_autosuspend() +> ??????pm_runtime_put_noidle() +> ??????* no device suspending and acpi_lpss_save_ctx() > 3. __device_release_driver() continue -> - dev->pm_domain->dismiss(dev) -> -> acpi_lpss_dismiss() ... -> acpi_device_set_power(D3) -> * powers down here +> ????- dev->pm_domain->dismiss(dev) +> ????-> acpi_lpss_dismiss() ... -> acpi_device_set_power(D3) +> ??????* powers down here > -> So after patch there is no acpi_lpss_save_ctx() call but I don't see -> does it cause any issue here. Maybe it's better to track clock only. +> So after patch there is no acpi_lpss_save_ctx() call but I don't see? +> does it cause any issue here. Maybe it's better to track clock only.? > What you think Andy? Now it looks like two fixes in one patch. From the commit message I diff --git a/a/content_digest b/N1/content_digest index f8a00ac..aeb4700 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,15 +1,9 @@ "ref\01461314971-5944-1-git-send-email-jszhang@marvell.com\0" "ref\0571A2E43.9030109@linux.intel.com\0" - "From\0Andy Shevchenko <andriy.shevchenko@linux.intel.com>\0" - "Subject\0Re: [PATCH v4] i2c: designware-platdrv: fix unbalanced clk enable and prepare\0" + "From\0andriy.shevchenko@linux.intel.com (Andy Shevchenko)\0" + "Subject\0[PATCH v4] i2c: designware-platdrv: fix unbalanced clk enable and prepare\0" "Date\0Mon, 25 Apr 2016 12:07:15 +0300\0" - "To\0Jarkko Nikula <jarkko.nikula@linux.intel.com>" - Jisheng Zhang <jszhang@marvell.com> - mika.westerberg@linux.intel.com - " wsa@the-dreams.de\0" - "Cc\0linux-i2c@vger.kernel.org" - linux-kernel@vger.kernel.org - " linux-arm-kernel@lists.infradead.org\0" + "To\0linux-arm-kernel@lists.infradead.org\0" "\00:1\0" "b\0" "On Fri, 2016-04-22 at 16:59 +0300, Jarkko Nikula wrote:\n" @@ -29,19 +23,19 @@ "> > \n" "> > Signed-off-by: Jisheng Zhang <jszhang@marvell.com>\n" "> > ---\n" - "> > \302\240 Since v3:\n" - "> > \302\240\302\240\302\240- use runtime PM rather than rpm in commit msg\n" - "> > \302\240\302\240\302\240- remove duplicated \"(\" in commit msg\n" + "> > ? Since v3:\n" + "> > ???- use runtime PM rather than rpm in commit msg\n" + "> > ???- remove duplicated \"(\" in commit msg\n" "> > \n" - "> > \302\240 Since v2:\n" - "> > \302\240\302\240\302\240- s/clk/clock\n" - "> > \302\240\302\240\302\240- describe why use pm_runtime_put_noidle()\n" + "> > ? Since v2:\n" + "> > ???- s/clk/clock\n" + "> > ???- describe why use pm_runtime_put_noidle()\n" "> > \n" - "> > \302\240 Since v1:\n" - "> > \302\240\302\240\302\240- fix commit msg: \"not rely on rpm\" rather than \"rely on rpm\"\n" - "> > \302\240\302\240\302\240- call i2c_dw_plat_prepare_clk after pm_rumtime_disable()\n" - "> > \302\240 drivers/i2c/busses/i2c-designware-platdrv.c | 16 ++++++++++------\n" - "> > \302\240 1 file changed, 10 insertions(+), 6 deletions(-)\n" + "> > ? Since v1:\n" + "> > ???- fix commit msg: \"not rely on rpm\" rather than \"rely on rpm\"\n" + "> > ???- call i2c_dw_plat_prepare_clk after pm_rumtime_disable()\n" + "> > ? drivers/i2c/busses/i2c-designware-platdrv.c | 16 ++++++++++------\n" + "> > ? 1 file changed, 10 insertions(+), 6 deletions(-)\n" "> > \n" "> > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c\n" "> > b/drivers/i2c/busses/i2c-designware-platdrv.c\n" @@ -50,9 +44,9 @@ "> > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c\n" "> > @@ -253,8 +253,11 @@ static int dw_i2c_plat_probe(struct\n" "> > platform_device *pdev)\n" - "> > \302\240\302\240\t}\n" + "> > ??\t}\n" "> > \n" - "> > \302\240\302\240\tr = i2c_dw_probe(dev);\n" + "> > ??\tr = i2c_dw_probe(dev);\n" "> > -\tif (r && !dev->pm_runtime_disabled)\n" "> > -\t\tpm_runtime_disable(&pdev->dev);\n" "> > +\tif (r) {\n" @@ -61,21 +55,21 @@ "> > +\t\ti2c_dw_plat_prepare_clk(dev, false);\n" "> > +\t}\n" "> > \n" - "> > \302\240\302\240\treturn r;\n" - "> > \302\240 }\n" + "> > ??\treturn r;\n" + "> > ? }\n" "> > @@ -264,15 +267,16 @@ static int dw_i2c_plat_remove(struct\n" "> > platform_device *pdev)\n" - "> > \302\240\302\240\tstruct dw_i2c_dev *dev = platform_get_drvdata(pdev);\n" + "> > ??\tstruct dw_i2c_dev *dev = platform_get_drvdata(pdev);\n" "> > \n" - "> > \302\240\302\240\tpm_runtime_get_sync(&pdev->dev);\n" + "> > ??\tpm_runtime_get_sync(&pdev->dev);\n" "> > +\tpm_runtime_dont_use_autosuspend(&pdev->dev);\n" "> > +\tif (!dev->pm_runtime_disabled)\n" "> > +\t\tpm_runtime_disable(&pdev->dev);\n" "> > +\tpm_runtime_put_noidle(&pdev->dev);\n" "> > \n" - "> > \302\240\302\240\ti2c_del_adapter(&dev->adapter);\n" + "> > ??\ti2c_del_adapter(&dev->adapter);\n" "> > \n" - "> > \302\240\302\240\ti2c_dw_disable(dev);\n" + "> > ??\ti2c_dw_disable(dev);\n" "> > \n" "> > -\tpm_runtime_dont_use_autosuspend(&pdev->dev);\n" "> > -\tpm_runtime_put_sync(&pdev->dev);\n" @@ -83,45 +77,45 @@ "> > -\t\tpm_runtime_disable(&pdev->dev);\n" "> > +\ti2c_dw_plat_prepare_clk(dev, false);\n" "> > \n" - "> This feels a bit an invasive change to me for unbalanced clock\302\240\n" - "> enable/disable and I noticed this changes semantics how\302\240\n" + "> This feels a bit an invasive change to me for unbalanced clock?\n" + "> enable/disable and I noticed this changes semantics how?\n" "> drivers/acpi/acpi_lpss.c devices are shutdown when removing the\n" - "> driver.\302\240\n" + "> driver.?\n" "> Although I didn't notice does it cause any regression.\n" "> \n" "> Before patch:\n" "> 1. drivers/base/dd.c: __device_release_driver()\n" - "> \302\240\302\240\302\240\302\240- pm_runtime_get_sync()\n" - "> \302\240\302\240\302\240\302\240\302\240\302\240-> acpi_device_set_power(D0)\n" - "> \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240acpi_lpss_restore_ctx()\n" - "> \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240dw_i2c_plat_resume()\n" + "> ????- pm_runtime_get_sync()\n" + "> ??????-> acpi_device_set_power(D0)\n" + "> ?????????acpi_lpss_restore_ctx()\n" + "> ?????????dw_i2c_plat_resume()\n" "> 2. dw_i2c_plat_remove()\n" - "> \302\240\302\240\302\240\302\240- pm_runtime_dont_use_autosuspend()\n" - "> \302\240\302\240\302\240\302\240\302\240\302\240pm_runtime_put_sync()\n" - "> \302\240\302\240\302\240\302\240\302\240\302\240-> dw_i2c_plat_suspend()\n" - "> \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240acpi_lpss_save_ctx()\n" - "> \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240acpi_device_set_power(D3)\n" + "> ????- pm_runtime_dont_use_autosuspend()\n" + "> ??????pm_runtime_put_sync()\n" + "> ??????-> dw_i2c_plat_suspend()\n" + "> ?????????acpi_lpss_save_ctx()\n" + "> ?????????acpi_device_set_power(D3)\n" "> 3. __device_release_driver() continue\n" - "> \302\240\302\240\302\240\302\240- dev->pm_domain->dismiss(dev)\n" - "> \302\240\302\240\302\240\302\240\302\240\302\240-> acpi_lpss_dismiss() ... -> acpi_device_set_power(D3)\n" + "> ????- dev->pm_domain->dismiss(dev)\n" + "> ??????-> acpi_lpss_dismiss() ... -> acpi_device_set_power(D3)\n" "> \n" "> After patch:\n" "> 1. drivers/base/dd.c: __device_release_driver()\n" - "> \302\240 - pm_runtime_get_sync()\n" - "> \302\240\302\240\302\240\302\240-> acpi_device_set_power(D0)\n" - "> \302\240\302\240\302\240\302\240\302\240\302\240\302\240acpi_lpss_restore_ctx()\n" - "> \302\240\302\240\302\240\302\240\302\240\302\240\302\240dw_i2c_plat_resume()\n" + "> ? - pm_runtime_get_sync()\n" + "> ????-> acpi_device_set_power(D0)\n" + "> ???????acpi_lpss_restore_ctx()\n" + "> ???????dw_i2c_plat_resume()\n" "> 2. dw_i2c_plat_remove()\n" - "> \302\240\302\240\302\240\302\240- pm_runtime_dont_use_autosuspend()\n" - "> \302\240\302\240\302\240\302\240\302\240\302\240pm_runtime_put_noidle()\n" - "> \302\240\302\240\302\240\302\240\302\240\302\240* no device suspending and acpi_lpss_save_ctx()\n" + "> ????- pm_runtime_dont_use_autosuspend()\n" + "> ??????pm_runtime_put_noidle()\n" + "> ??????* no device suspending and acpi_lpss_save_ctx()\n" "> 3. __device_release_driver() continue\n" - "> \302\240\302\240\302\240\302\240- dev->pm_domain->dismiss(dev)\n" - "> \302\240\302\240\302\240\302\240-> acpi_lpss_dismiss() ... -> acpi_device_set_power(D3)\n" - "> \302\240\302\240\302\240\302\240\302\240\302\240* powers down here\n" + "> ????- dev->pm_domain->dismiss(dev)\n" + "> ????-> acpi_lpss_dismiss() ... -> acpi_device_set_power(D3)\n" + "> ??????* powers down here\n" "> \n" - "> So after patch there is no acpi_lpss_save_ctx() call but I don't see\302\240\n" - "> does it cause any issue here. Maybe it's better to track clock only.\302\240\n" + "> So after patch there is no acpi_lpss_save_ctx() call but I don't see?\n" + "> does it cause any issue here. Maybe it's better to track clock only.?\n" "> What you think Andy?\n" "\n" "Now it looks like two fixes in one patch. From the commit message I\n" @@ -133,4 +127,4 @@ "Andy Shevchenko <andriy.shevchenko@linux.intel.com>\n" Intel Finland Oy -f5d26b3597c09c640b955fcf569e6dfcd959841b5f8864f73eca7071ea346aac +72b606102bce9c9fc9f51158fe7e57da2b4dfb207dd491fe586a6baf50b0de4b
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.