[PATCH 0/5] memory: renesas-rpc-if: Trivial fixes

[PATCH 0/5] memory: renesas-rpc-if: Trivial fixes

[Lad Prabhakar]

Hi All,

This patch series fixes trivial issues in RPC-IF driver.

Cheers,
Prabhakar

Lad Prabhakar (5):
  memory: renesas-rpc-if: Return correct value to the caller of
    rpcif_manual_xfer()
  memory: renesas-rpc-if: Make rpcif_enable/disable_rpm() as static
    inline
  memory: renesas-rpc-if: Export symbols as GPL
  memory: renesas-rpc-if: Avoid use of C++ style comments
  memory: renesas-rpc-if: Fix a reference leak in rpcif_probe()

 drivers/memory/renesas-rpc-if.c | 28 +++++++++-------------------
 include/memory/renesas-rpc-if.h | 19 ++++++++++++++-----
 2 files changed, 23 insertions(+), 24 deletions(-)

-- 
2.17.1

[PATCH 1/5] memory: renesas-rpc-if: Return correct value to the caller of rpcif_manual_xfer()

[Lad Prabhakar]

In the error path of rpcif_manual_xfer() the value of ret is overwritten
by value returned by reset_control_reset() function and thus returning
incorrect value to the caller.

This patch makes sure the correct value is returned to the caller of
rpcif_manual_xfer() by dropping the overwrite of ret in error path.
Also now we ignore the value returned by reset_control_reset() in the
error path and instead print a error message when it fails.

Fixes: ca7d8b980b67f ("memory: add Renesas RPC-IF driver")
Reported-by: Pavel Machek <pavel@denx.de>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Cc: stable@vger.kernel.org
---
 drivers/memory/renesas-rpc-if.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index f2a33a1af836..69f2e2b4cd50 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -508,7 +508,8 @@ int rpcif_manual_xfer(struct rpcif *rpc)
 	return ret;
 
 err_out:
-	ret = reset_control_reset(rpc->rstc);
+	if (reset_control_reset(rpc->rstc))
+		dev_err(rpc->dev, "Failed to reset HW\n");
 	rpcif_hw_init(rpc, rpc->bus_size == 2);
 	goto exit;
 }
-- 
2.17.1

[PATCH 2/5] memory: renesas-rpc-if: Make rpcif_enable/disable_rpm() as static inline

[Lad Prabhakar]

Define rpcif_enable_rpm() and rpcif_disable_rpm() as static
inline in the header instead of exporting it.

Suggested-by: Pavel Machek <pavel@denx.de>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/memory/renesas-rpc-if.c | 13 -------------
 include/memory/renesas-rpc-if.h | 13 +++++++++++--
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index 69f2e2b4cd50..c5b5691503d7 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -12,7 +12,6 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/of.h>
-#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/reset.h>
 
@@ -204,18 +203,6 @@ int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
 }
 EXPORT_SYMBOL(rpcif_sw_init);
 
-void rpcif_enable_rpm(struct rpcif *rpc)
-{
-	pm_runtime_enable(rpc->dev);
-}
-EXPORT_SYMBOL(rpcif_enable_rpm);
-
-void rpcif_disable_rpm(struct rpcif *rpc)
-{
-	pm_runtime_put_sync(rpc->dev);
-}
-EXPORT_SYMBOL(rpcif_disable_rpm);
-
 void rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
 {
 	u32 dummy;
diff --git a/include/memory/renesas-rpc-if.h b/include/memory/renesas-rpc-if.h
index 9ad136682c47..b8c7cc63065f 100644
--- a/include/memory/renesas-rpc-if.h
+++ b/include/memory/renesas-rpc-if.h
@@ -10,6 +10,7 @@
 #ifndef __RENESAS_RPC_IF_H
 #define __RENESAS_RPC_IF_H
 
+#include <linux/pm_runtime.h>
 #include <linux/types.h>
 
 enum rpcif_data_dir {
@@ -77,11 +78,19 @@ struct	rpcif {
 
 int  rpcif_sw_init(struct rpcif *rpc, struct device *dev);
 void rpcif_hw_init(struct rpcif *rpc, bool hyperflash);
-void rpcif_enable_rpm(struct rpcif *rpc);
-void rpcif_disable_rpm(struct rpcif *rpc);
 void rpcif_prepare(struct rpcif *rpc, const struct rpcif_op *op, u64 *offs,
 		   size_t *len);
 int rpcif_manual_xfer(struct rpcif *rpc);
 ssize_t rpcif_dirmap_read(struct rpcif *rpc, u64 offs, size_t len, void *buf);
 
+static inline void rpcif_enable_rpm(struct rpcif *rpc)
+{
+	pm_runtime_enable(rpc->dev);
+}
+
+static inline void rpcif_disable_rpm(struct rpcif *rpc)
+{
+	pm_runtime_put_sync(rpc->dev);
+}
+
 #endif // __RENESAS_RPC_IF_H
-- 
2.17.1

[PATCH 3/5] memory: renesas-rpc-if: Export symbols as GPL

[Lad Prabhakar]

Renesas RPC-IF driver is licensed under GPL2.0, to be in sync export the
symbols as GPL.

Suggested-by: Pavel Machek <pavel@denx.de>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/memory/renesas-rpc-if.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index c5b5691503d7..f5cbc762fda7 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -201,7 +201,7 @@ int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
 
 	return PTR_ERR_OR_ZERO(rpc->rstc);
 }
-EXPORT_SYMBOL(rpcif_sw_init);
+EXPORT_SYMBOL_GPL(rpcif_sw_init);
 
 void rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
 {
@@ -249,7 +249,7 @@ void rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
 
 	rpc->bus_size = hyperflash ? 2 : 1;
 }
-EXPORT_SYMBOL(rpcif_hw_init);
+EXPORT_SYMBOL_GPL(rpcif_hw_init);
 
 static int wait_msg_xfer_end(struct rpcif *rpc)
 {
@@ -358,7 +358,7 @@ void rpcif_prepare(struct rpcif *rpc, const struct rpcif_op *op, u64 *offs,
 			RPCIF_SMENR_SPIDB(rpcif_bit_size(op->data.buswidth));
 	}
 }
-EXPORT_SYMBOL(rpcif_prepare);
+EXPORT_SYMBOL_GPL(rpcif_prepare);
 
 int rpcif_manual_xfer(struct rpcif *rpc)
 {
@@ -500,7 +500,7 @@ int rpcif_manual_xfer(struct rpcif *rpc)
 	rpcif_hw_init(rpc, rpc->bus_size == 2);
 	goto exit;
 }
-EXPORT_SYMBOL(rpcif_manual_xfer);
+EXPORT_SYMBOL_GPL(rpcif_manual_xfer);
 
 ssize_t rpcif_dirmap_read(struct rpcif *rpc, u64 offs, size_t len, void *buf)
 {
@@ -529,7 +529,7 @@ ssize_t rpcif_dirmap_read(struct rpcif *rpc, u64 offs, size_t len, void *buf)
 
 	return len;
 }
-EXPORT_SYMBOL(rpcif_dirmap_read);
+EXPORT_SYMBOL_GPL(rpcif_dirmap_read);
 
 static int rpcif_probe(struct platform_device *pdev)
 {
-- 
2.17.1

[PATCH 4/5] memory: renesas-rpc-if: Avoid use of C++ style comments

[Lad Prabhakar]

Replace C++ style comment with C style.

While at it also replace the tab with a space between struct and
struct name.

Suggested-by: Pavel Machek <pavel@denx.de>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 include/memory/renesas-rpc-if.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/memory/renesas-rpc-if.h b/include/memory/renesas-rpc-if.h
index b8c7cc63065f..30ea6bd969b4 100644
--- a/include/memory/renesas-rpc-if.h
+++ b/include/memory/renesas-rpc-if.h
@@ -19,7 +19,7 @@ enum rpcif_data_dir {
 	RPCIF_DATA_OUT,
 };
 
-struct	rpcif_op {
+struct rpcif_op {
 	struct {
 		u8 buswidth;
 		u8 opcode;
@@ -57,7 +57,7 @@ struct	rpcif_op {
 	} data;
 };
 
-struct	rpcif {
+struct rpcif {
 	struct device *dev;
 	void __iomem *dirmap;
 	struct regmap *regmap;
@@ -93,4 +93,4 @@ static inline void rpcif_disable_rpm(struct rpcif *rpc)
 	pm_runtime_put_sync(rpc->dev);
 }
 
-#endif // __RENESAS_RPC_IF_H
+#endif /* __RENESAS_RPC_IF_H */
-- 
2.17.1

[PATCH 5/5] memory: renesas-rpc-if: Fix a reference leak in rpcif_probe()

[Lad Prabhakar]

Release the node reference by calling of_node_put(flash) in the probe.

Fixes: ca7d8b980b67f ("memory: add Renesas RPC-IF driver")
Reported-by: Pavel Machek <pavel@denx.de>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Cc: stable@vger.kernel.org
---
 drivers/memory/renesas-rpc-if.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index f5cbc762fda7..99633986ffda 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -548,9 +548,11 @@ static int rpcif_probe(struct platform_device *pdev)
 	} else if (of_device_is_compatible(flash, "cfi-flash")) {
 		name = "rpc-if-hyperflash";
 	} else	{
+		of_node_put(flash);
 		dev_warn(&pdev->dev, "unknown flash type\n");
 		return -ENODEV;
 	}
+	of_node_put(flash);
 
 	vdev = platform_device_alloc(name, pdev->id);
 	if (!vdev)
-- 
2.17.1

Re: [PATCH 0/5] memory: renesas-rpc-if: Trivial fixes

[Lad, Prabhakar]

Hi Sergei,

On Tue, Nov 24, 2020 at 11:26 AM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
>
> Hi All,
>
> This patch series fixes trivial issues in RPC-IF driver.
>
> Cheers,
> Prabhakar
>
> Lad Prabhakar (5):
>   memory: renesas-rpc-if: Return correct value to the caller of
>     rpcif_manual_xfer()
>   memory: renesas-rpc-if: Make rpcif_enable/disable_rpm() as static
>     inline
>   memory: renesas-rpc-if: Export symbols as GPL
>   memory: renesas-rpc-if: Avoid use of C++ style comments
>   memory: renesas-rpc-if: Fix a reference leak in rpcif_probe()
>
Patches sent to sergei.shtylyov@cogentembedded.com have bounced back
so including gmail address (patchwork [1]).

[1] https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=390163

Cheers,
Prabhakar


>  drivers/memory/renesas-rpc-if.c | 28 +++++++++-------------------
>  include/memory/renesas-rpc-if.h | 19 ++++++++++++++-----
>  2 files changed, 23 insertions(+), 24 deletions(-)
>
> --
> 2.17.1
>

Re: [PATCH 2/5] memory: renesas-rpc-if: Make rpcif_enable/disable_rpm() as static inline

[Geert Uytterhoeven]

Hi Prabhakar,

On Tue, Nov 24, 2020 at 12:27 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Define rpcif_enable_rpm() and rpcif_disable_rpm() as static
> inline in the header instead of exporting it.
>
> Suggested-by: Pavel Machek <pavel@denx.de>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch, which is an improvement.

> --- a/include/memory/renesas-rpc-if.h
> +++ b/include/memory/renesas-rpc-if.h
> @@ -10,6 +10,7 @@
>  #ifndef __RENESAS_RPC_IF_H
>  #define __RENESAS_RPC_IF_H
>
> +#include <linux/pm_runtime.h>
>  #include <linux/types.h>
>
>  enum rpcif_data_dir {
> @@ -77,11 +78,19 @@ struct      rpcif {
>
>  int  rpcif_sw_init(struct rpcif *rpc, struct device *dev);
>  void rpcif_hw_init(struct rpcif *rpc, bool hyperflash);
> -void rpcif_enable_rpm(struct rpcif *rpc);
> -void rpcif_disable_rpm(struct rpcif *rpc);
>  void rpcif_prepare(struct rpcif *rpc, const struct rpcif_op *op, u64 *offs,
>                    size_t *len);
>  int rpcif_manual_xfer(struct rpcif *rpc);
>  ssize_t rpcif_dirmap_read(struct rpcif *rpc, u64 offs, size_t len, void *buf);
>
> +static inline void rpcif_enable_rpm(struct rpcif *rpc)
> +{
> +       pm_runtime_enable(rpc->dev);
> +}
> +
> +static inline void rpcif_disable_rpm(struct rpcif *rpc)
> +{
> +       pm_runtime_put_sync(rpc->dev);

Looking at how this is used, this should call pm_runtime_disable()
instead.

And probably this should be moved inside the core RPC-IF driver:
  1. pm_runtime_enable() could be called from rpcif_sw_init(),
  2. pm_runtime_put_sync() can be called from a new rpc_sw_deinit()
     function, to be called by the SPI and MTD drivers on probe failure
     and on remove.

> +}

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/5] memory: renesas-rpc-if: Make rpcif_enable/disable_rpm() as static inline

[Sergei Shtylyov]

Hello!

On 11/24/20 2:25 PM, Lad Prabhakar wrote:

> Define rpcif_enable_rpm() and rpcif_disable_rpm() as static

   Not sure why I didn't do it this way myself...

> inline in the header instead of exporting it.

   s/it/them/.

> Suggested-by: Pavel Machek <pavel@denx.de>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  drivers/memory/renesas-rpc-if.c | 13 -------------
>  include/memory/renesas-rpc-if.h | 13 +++++++++++--
>  2 files changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
> index 69f2e2b4cd50..c5b5691503d7 100644
> --- a/drivers/memory/renesas-rpc-if.c
> +++ b/drivers/memory/renesas-rpc-if.c
[...]
> @@ -204,18 +203,6 @@ int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
>  }
>  EXPORT_SYMBOL(rpcif_sw_init);
>  
> -void rpcif_enable_rpm(struct rpcif *rpc)
> -{
> -	pm_runtime_enable(rpc->dev);
> -}
> -EXPORT_SYMBOL(rpcif_enable_rpm);
> -
> -void rpcif_disable_rpm(struct rpcif *rpc)
> -{
> -	pm_runtime_put_sync(rpc->dev);

   Ugh... sorry for this blunder (that went unnoticed till now). Mind fixing
it to pm_runtime_disable() (before this patch)?

> -}
> -EXPORT_SYMBOL(rpcif_disable_rpm);
> -
>  void rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
>  {
>  	u32 dummy;
[...]

MBR, Sergei

Re: [PATCH 0/5] memory: renesas-rpc-if: Trivial fixes

[Sergei Shtylyov]

On 11/24/20 2:34 PM, Lad, Prabhakar wrote:

[...]
>> This patch series fixes trivial issues in RPC-IF driver.
>>
>> Cheers,
>> Prabhakar
>>
>> Lad Prabhakar (5):
>>   memory: renesas-rpc-if: Return correct value to the caller of
>>     rpcif_manual_xfer()
>>   memory: renesas-rpc-if: Make rpcif_enable/disable_rpm() as static
>>     inline
>>   memory: renesas-rpc-if: Export symbols as GPL
>>   memory: renesas-rpc-if: Avoid use of C++ style comments
>>   memory: renesas-rpc-if: Fix a reference leak in rpcif_probe()
>>
> Patches sent to sergei.shtylyov@cogentembedded.com have bounced back
> so including gmail address (patchwork [1]).

   Sorry, I got laid off by Cogent last May. Thanks for CCing my gmail address...

> [1] https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=390163
> 
> Cheers,
> Prabhakar

[...]

MBR, Sergei

Re: [PATCH 4/5] memory: renesas-rpc-if: Avoid use of C++ style comments

[Sergei Shtylyov]

On 11/24/20 2:25 PM, Lad Prabhakar wrote:

> Replace C++ style comment with C style.

   Thanks, I've overlooked this, and the header files should use C style comment,
not C++.
 
> While at it also replace the tab with a space between struct and
> struct name.

   No connection between these 2 changes, so there should be 2 patches, not 1.
Also, I'd like to ask you that they're left intact (unless it causes problems
for you).

> Suggested-by: Pavel Machek <pavel@denx.de>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[...]

> diff --git a/include/memory/renesas-rpc-if.h b/include/memory/renesas-rpc-if.h
> index b8c7cc63065f..30ea6bd969b4 100644
> --- a/include/memory/renesas-rpc-if.h
> +++ b/include/memory/renesas-rpc-if.h
> @@ -19,7 +19,7 @@ enum rpcif_data_dir {
>  	RPCIF_DATA_OUT,
>  };
>  
> -struct	rpcif_op {
> +struct rpcif_op {
>  	struct {
>  		u8 buswidth;
>  		u8 opcode;
> @@ -57,7 +57,7 @@ struct	rpcif_op {
>  	} data;
>  };
>  
> -struct	rpcif {
> +struct rpcif {
>  	struct device *dev;
>  	void __iomem *dirmap;
>  	struct regmap *regmap;
> @@ -93,4 +93,4 @@ static inline void rpcif_disable_rpm(struct rpcif *rpc)
>  	pm_runtime_put_sync(rpc->dev);
>  }
>  
> -#endif // __RENESAS_RPC_IF_H
> +#endif /* __RENESAS_RPC_IF_H */

MBR, Sergei

Re: [PATCH 1/5] memory: renesas-rpc-if: Return correct value to the caller of rpcif_manual_xfer()

[Sergei Shtylyov]

Hello!

On 24.11.2020 14:25, Lad Prabhakar wrote:

> In the error path of rpcif_manual_xfer() the value of ret is overwritten
> by value returned by reset_control_reset() function and thus returning
> incorrect value to the caller.
> 
> This patch makes sure the correct value is returned to the caller of
> rpcif_manual_xfer() by dropping the overwrite of ret in error path.
> Also now we ignore the value returned by reset_control_reset() in the
> error path and instead print a error message when it fails.
> 
> Fixes: ca7d8b980b67f ("memory: add Renesas RPC-IF driver")
> Reported-by: Pavel Machek <pavel@denx.de>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Cc: stable@vger.kernel.org

Reviewed-by: Sergei Shtylyov <sergei.shtylyov@gmail.com>

MBR, Sergei

Re: [PATCH 3/5] memory: renesas-rpc-if: Export symbols as GPL

[Sergei Shtylyov]

On 24.11.2020 14:25, Lad Prabhakar wrote:

> Renesas RPC-IF driver is licensed under GPL2.0, to be in sync export the
> symbols as GPL.

    Didn't know there's a connection...

> Suggested-by: Pavel Machek <pavel@denx.de>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Sergei Shtylyov <sergei.shtylyov@gmail.com>

[...]

MBR, Sergei

Re: [PATCH 5/5] memory: renesas-rpc-if: Fix a reference leak in rpcif_probe()

[Sergei Shtylyov]

On 11/24/20 2:25 PM, Lad Prabhakar wrote:

> Release the node reference by calling of_node_put(flash) in the probe.

   Sorry about missing this...

> Fixes: ca7d8b980b67f ("memory: add Renesas RPC-IF driver")
> Reported-by: Pavel Machek <pavel@denx.de>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Cc: stable@vger.kernel.org

Reviewed-by: Sergei Shtylyov <sergei.shtylyov@gmail.com>

[...]

MBR, Sergei

Re: [PATCH 0/5] memory: renesas-rpc-if: Trivial fixes

[Lad, Prabhakar]

On Tue, Nov 24, 2020 at 6:25 PM Sergei Shtylyov
<sergei.shtylyov@gmail.com> wrote:
>
> On 11/24/20 2:34 PM, Lad, Prabhakar wrote:
>
> [...]
> >> This patch series fixes trivial issues in RPC-IF driver.
> >>
> >> Cheers,
> >> Prabhakar
> >>
> >> Lad Prabhakar (5):
> >>   memory: renesas-rpc-if: Return correct value to the caller of
> >>     rpcif_manual_xfer()
> >>   memory: renesas-rpc-if: Make rpcif_enable/disable_rpm() as static
> >>     inline
> >>   memory: renesas-rpc-if: Export symbols as GPL
> >>   memory: renesas-rpc-if: Avoid use of C++ style comments
> >>   memory: renesas-rpc-if: Fix a reference leak in rpcif_probe()
> >>
> > Patches sent to sergei.shtylyov@cogentembedded.com have bounced back
> > so including gmail address (patchwork [1]).
>
>    Sorry, I got laid off by Cogent last May. Thanks for CCing my gmail address...
>
Sorry to hear that.

Thank you for the review. I'll fix the review comments for patch 2/2
and post a v2.

Cheers,
Prabhakar

Re: [PATCH 2/5] memory: renesas-rpc-if: Make rpcif_enable/disable_rpm() as static inline

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Tue, Nov 24, 2020 at 3:43 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, Nov 24, 2020 at 12:27 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > Define rpcif_enable_rpm() and rpcif_disable_rpm() as static
> > inline in the header instead of exporting it.
> >
> > Suggested-by: Pavel Machek <pavel@denx.de>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch, which is an improvement.
>
> > --- a/include/memory/renesas-rpc-if.h
> > +++ b/include/memory/renesas-rpc-if.h
> > @@ -10,6 +10,7 @@
> >  #ifndef __RENESAS_RPC_IF_H
> >  #define __RENESAS_RPC_IF_H
> >
> > +#include <linux/pm_runtime.h>
> >  #include <linux/types.h>
> >
> >  enum rpcif_data_dir {
> > @@ -77,11 +78,19 @@ struct      rpcif {
> >
> >  int  rpcif_sw_init(struct rpcif *rpc, struct device *dev);
> >  void rpcif_hw_init(struct rpcif *rpc, bool hyperflash);
> > -void rpcif_enable_rpm(struct rpcif *rpc);
> > -void rpcif_disable_rpm(struct rpcif *rpc);
> >  void rpcif_prepare(struct rpcif *rpc, const struct rpcif_op *op, u64 *offs,
> >                    size_t *len);
> >  int rpcif_manual_xfer(struct rpcif *rpc);
> >  ssize_t rpcif_dirmap_read(struct rpcif *rpc, u64 offs, size_t len, void *buf);
> >
> > +static inline void rpcif_enable_rpm(struct rpcif *rpc)
> > +{
> > +       pm_runtime_enable(rpc->dev);
> > +}
> > +
> > +static inline void rpcif_disable_rpm(struct rpcif *rpc)
> > +{
> > +       pm_runtime_put_sync(rpc->dev);
>
> Looking at how this is used, this should call pm_runtime_disable()
> instead.
>
> And probably this should be moved inside the core RPC-IF driver:
>   1. pm_runtime_enable() could be called from rpcif_sw_init(),
>   2. pm_runtime_put_sync() can be called from a new rpc_sw_deinit()
>      function, to be called by the SPI and MTD drivers on probe failure
>      and on remove.
>
Totally agree.

Sergei are you OK with the above suggestions ?

Cheers,
Prabhakar