From mboxrd@z Thu Jan 1 00:00:00 1970 From: Cyrille Pitchen Subject: Re: [PATCH v1 4/5] mtd: m25p80: Check if the spi flash device has pm support Date: Mon, 6 Feb 2017 12:01:27 +0100 Message-ID: <9108d13d-0f06-172f-5e8d-91ff80fdd510@atmel.com> References: <1486164676-12912-1-git-send-email-kdasu.kdev@gmail.com> <1486164676-12912-5-git-send-email-kdasu.kdev@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit Cc: , , To: Kamal Dasu , , , Return-path: In-Reply-To: <1486164676-12912-5-git-send-email-kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Sender: linux-spi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Hi Kamal, Le 04/02/2017 à 00:31, Kamal Dasu a écrit : > Call the spi_nor_rescan() only if the controller driver needs this > support. This way SoCs that need this feature can use it. > > Signed-off-by: Kamal Dasu > --- > drivers/mtd/devices/m25p80.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c > index 4528e33..ffdec60 100644 > --- a/drivers/mtd/devices/m25p80.c > +++ b/drivers/mtd/devices/m25p80.c > @@ -328,8 +328,13 @@ static int m25p_suspend(struct device *dev) > static int m25p_resume(struct device *dev) > { > struct m25p *flash = dev_get_drvdata(dev); > + struct spi_device *spi = flash->spi; > + int ret = 0; > + > + if (spi_flash_pm_supported(spi)) > + ret = spi_nor_pm_rescan(&flash->spi_nor, NULL); > Why don't you squash patch 2 into this one? Patch 2 suggests that spi_nor_pm_rescan() could safely be called in any case but now this patch suggests that calling that function is not so safe. I see two cases: 1/ either calling spi_nor_pm_rescan() is safe in any case, then patches 3, 4 and 5 are needless. 2/ or calling spi_nor_pm_rescan() has unwanted side effects in some cases, then patch 2 should be squashed into this patch: patch 2 will never be taken alone in the spi-nor tree if it may introduce bugs or regressions. If something has to be done in the SPI sub-system, I guess it should be done first: for sure applying patches 3 and 5 would not create any issue. Then patches 2 and 4 squashed into a single patch may be safely applied after. However, patches 3 and 5 still need to be discussed first. Best regards, Cyrille > - return spi_nor_pm_rescan(&flash->spi-nor, NULL); > + return ret; > } > #endif > static SIMPLE_DEV_PM_OPS(m25p_pm_ops, m25p_suspend, m25p_resume); > -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtpout.microchip.com ([198.175.253.82] helo=email.microchip.com) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1cah3O-0006iD-PZ for linux-mtd@lists.infradead.org; Mon, 06 Feb 2017 11:01:56 +0000 Subject: Re: [PATCH v1 4/5] mtd: m25p80: Check if the spi flash device has pm support To: Kamal Dasu , , , References: <1486164676-12912-1-git-send-email-kdasu.kdev@gmail.com> <1486164676-12912-5-git-send-email-kdasu.kdev@gmail.com> CC: , , From: Cyrille Pitchen Message-ID: <9108d13d-0f06-172f-5e8d-91ff80fdd510@atmel.com> Date: Mon, 6 Feb 2017 12:01:27 +0100 MIME-Version: 1.0 In-Reply-To: <1486164676-12912-5-git-send-email-kdasu.kdev@gmail.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Hi Kamal, Le 04/02/2017 à 00:31, Kamal Dasu a écrit : > Call the spi_nor_rescan() only if the controller driver needs this > support. This way SoCs that need this feature can use it. > > Signed-off-by: Kamal Dasu > --- > drivers/mtd/devices/m25p80.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c > index 4528e33..ffdec60 100644 > --- a/drivers/mtd/devices/m25p80.c > +++ b/drivers/mtd/devices/m25p80.c > @@ -328,8 +328,13 @@ static int m25p_suspend(struct device *dev) > static int m25p_resume(struct device *dev) > { > struct m25p *flash = dev_get_drvdata(dev); > + struct spi_device *spi = flash->spi; > + int ret = 0; > + > + if (spi_flash_pm_supported(spi)) > + ret = spi_nor_pm_rescan(&flash->spi_nor, NULL); > Why don't you squash patch 2 into this one? Patch 2 suggests that spi_nor_pm_rescan() could safely be called in any case but now this patch suggests that calling that function is not so safe. I see two cases: 1/ either calling spi_nor_pm_rescan() is safe in any case, then patches 3, 4 and 5 are needless. 2/ or calling spi_nor_pm_rescan() has unwanted side effects in some cases, then patch 2 should be squashed into this patch: patch 2 will never be taken alone in the spi-nor tree if it may introduce bugs or regressions. If something has to be done in the SPI sub-system, I guess it should be done first: for sure applying patches 3 and 5 would not create any issue. Then patches 2 and 4 squashed into a single patch may be safely applied after. However, patches 3 and 5 still need to be discussed first. Best regards, Cyrille > - return spi_nor_pm_rescan(&flash->spi-nor, NULL); > + return ret; > } > #endif > static SIMPLE_DEV_PM_OPS(m25p_pm_ops, m25p_suspend, m25p_resume); >