diff mbox series

[v3,02/18] pinctrl: pinctrl-single: remove dead code in suspend() and resume() callbacks

Message ID 20240102-j7200-pcie-s2r-v3-2-5c2e4a3fac1f@bootlin.com (mailing list archive)
State New, archived
Headers show
Series Add suspend to ram support for PCIe on J7200 | expand

Commit Message

Thomas Richard Feb. 15, 2024, 3:17 p.m. UTC
No need to check the pointer returned by platform_get_drvdata(), as
platform_set_drvdata() is called during the probe.

Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
---
 drivers/pinctrl/pinctrl-single.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

Comments

Andy Shevchenko Feb. 15, 2024, 3:27 p.m. UTC | #1
On Thu, Feb 15, 2024 at 04:17:47PM +0100, Thomas Richard wrote:
> No need to check the pointer returned by platform_get_drvdata(), as
> platform_set_drvdata() is called during the probe.

This patch should go _after_ the next one, otherwise the commit message doesn't
tell full story and the code change bring a potential regression.
Thomas Richard Feb. 16, 2024, 7:59 a.m. UTC | #2
On 2/15/24 16:27, Andy Shevchenko wrote:
> On Thu, Feb 15, 2024 at 04:17:47PM +0100, Thomas Richard wrote:
>> No need to check the pointer returned by platform_get_drvdata(), as
>> platform_set_drvdata() is called during the probe.
> 
> This patch should go _after_ the next one, otherwise the commit message doesn't
> tell full story and the code change bring a potential regression.
> 

Hello Andy,

I'm ok to move this patch after the next one.
But for my understanding, could you explain me why changing the order is
important in this case ?

Regards,
Andy Shevchenko Feb. 16, 2024, 3:08 p.m. UTC | #3
On Fri, Feb 16, 2024 at 08:59:47AM +0100, Thomas Richard wrote:
> On 2/15/24 16:27, Andy Shevchenko wrote:
> > On Thu, Feb 15, 2024 at 04:17:47PM +0100, Thomas Richard wrote:
> >> No need to check the pointer returned by platform_get_drvdata(), as
> >> platform_set_drvdata() is called during the probe.
> > 
> > This patch should go _after_ the next one, otherwise the commit message doesn't
> > tell full story and the code change bring a potential regression.
> 
> Hello Andy,
> 
> I'm ok to move this patch after the next one.
> But for my understanding, could you explain me why changing the order is
> important in this case ?

Old PM calls obviously can be called in different circumstances and these
checks are important.

Just squash these two patches to avoid additional churn and we are done.
Thomas Richard Feb. 21, 2024, 11:01 a.m. UTC | #4
On 2/16/24 16:08, Andy Shevchenko wrote:
> On Fri, Feb 16, 2024 at 08:59:47AM +0100, Thomas Richard wrote:
>> On 2/15/24 16:27, Andy Shevchenko wrote:
>>> On Thu, Feb 15, 2024 at 04:17:47PM +0100, Thomas Richard wrote:
>>>> No need to check the pointer returned by platform_get_drvdata(), as
>>>> platform_set_drvdata() is called during the probe.
>>>
>>> This patch should go _after_ the next one, otherwise the commit message doesn't
>>> tell full story and the code change bring a potential regression.
>>
>> Hello Andy,
>>
>> I'm ok to move this patch after the next one.
>> But for my understanding, could you explain me why changing the order is
>> important in this case ?
> 
> Old PM calls obviously can be called in different circumstances and these
> checks are important.
> 
> Just squash these two patches to avoid additional churn and we are done.

You mean invert the order instead of squash.
Andy Shevchenko Feb. 21, 2024, 1:13 p.m. UTC | #5
On Wed, Feb 21, 2024 at 12:01:43PM +0100, Thomas Richard wrote:
> On 2/16/24 16:08, Andy Shevchenko wrote:
> > On Fri, Feb 16, 2024 at 08:59:47AM +0100, Thomas Richard wrote:
> >> On 2/15/24 16:27, Andy Shevchenko wrote:
> >>> On Thu, Feb 15, 2024 at 04:17:47PM +0100, Thomas Richard wrote:
> >>>> No need to check the pointer returned by platform_get_drvdata(), as
> >>>> platform_set_drvdata() is called during the probe.
> >>>
> >>> This patch should go _after_ the next one, otherwise the commit message doesn't
> >>> tell full story and the code change bring a potential regression.
> >>
> >> Hello Andy,
> >>
> >> I'm ok to move this patch after the next one.
> >> But for my understanding, could you explain me why changing the order is
> >> important in this case ?
> > 
> > Old PM calls obviously can be called in different circumstances and these
> > checks are important.
> > 
> > Just squash these two patches to avoid additional churn and we are done.
> 
> You mean invert the order instead of squash.

Either would work, but see how much churn in terms of changing just changed
lines it adds.
Thomas Richard Feb. 21, 2024, 2:19 p.m. UTC | #6
On 2/21/24 14:13, Andy Shevchenko wrote:
> On Wed, Feb 21, 2024 at 12:01:43PM +0100, Thomas Richard wrote:
>> On 2/16/24 16:08, Andy Shevchenko wrote:
>>> On Fri, Feb 16, 2024 at 08:59:47AM +0100, Thomas Richard wrote:
>>>> On 2/15/24 16:27, Andy Shevchenko wrote:
>>>>> On Thu, Feb 15, 2024 at 04:17:47PM +0100, Thomas Richard wrote:
>>>>>> No need to check the pointer returned by platform_get_drvdata(), as
>>>>>> platform_set_drvdata() is called during the probe.
>>>>>
>>>>> This patch should go _after_ the next one, otherwise the commit message doesn't
>>>>> tell full story and the code change bring a potential regression.
>>>>
>>>> Hello Andy,
>>>>
>>>> I'm ok to move this patch after the next one.
>>>> But for my understanding, could you explain me why changing the order is
>>>> important in this case ?
>>>
>>> Old PM calls obviously can be called in different circumstances and these
>>> checks are important.
>>>
>>> Just squash these two patches to avoid additional churn and we are done.
>>
>> You mean invert the order instead of squash.
> 
> Either would work, but see how much churn in terms of changing just changed
> lines it adds.

OK thanks.

I'll squash the two patches. And I'll add a comment which explains that
I dropped some dead code.

Regards,
diff mbox series

Patch

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 19cc0db771a5..02eabd28d46e 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1693,11 +1693,7 @@  static void pcs_restore_context(struct pcs_device *pcs)
 static int pinctrl_single_suspend(struct platform_device *pdev,
 					pm_message_t state)
 {
-	struct pcs_device *pcs;
-
-	pcs = platform_get_drvdata(pdev);
-	if (!pcs)
-		return -EINVAL;
+	struct pcs_device *pcs = platform_get_drvdata(pdev);
 
 	if (pcs->flags & PCS_CONTEXT_LOSS_OFF) {
 		int ret;
@@ -1712,11 +1708,7 @@  static int pinctrl_single_suspend(struct platform_device *pdev,
 
 static int pinctrl_single_resume(struct platform_device *pdev)
 {
-	struct pcs_device *pcs;
-
-	pcs = platform_get_drvdata(pdev);
-	if (!pcs)
-		return -EINVAL;
+	struct pcs_device *pcs = platform_get_drvdata(pdev);
 
 	if (pcs->flags & PCS_CONTEXT_LOSS_OFF)
 		pcs_restore_context(pcs);