* [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
2020-03-27 16:36 [PATCH 0/4] firmware: arm_scmi: Extend SMC/HVC to support multiple channels Sudeep Holla
@ 2020-03-27 16:36 ` Sudeep Holla
2020-04-01 1:12 ` Peng Fan
2020-04-01 9:14 ` Peng Fan
2020-03-27 16:36 ` [PATCH 2/4] firmware: arm_scmi: Drop empty stub for smc_mark_txdone Sudeep Holla
` (3 subsequent siblings)
4 siblings, 2 replies; 16+ messages in thread
From: Sudeep Holla @ 2020-03-27 16:36 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, Peng Fan
Cc: Sudeep Holla, f.fainelli, linux-imx
In order to support multiple SMC/HVC transport channels with associated
shared memory, it is better to maintain the mutex per channel instead of
existing global one.
Move the smc_mutex into the scmi_smc structure and also rename it to
shmem_lock which is more appropriate for it's use.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/arm_scmi/smc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
index 336168e40f49..6dc8a88cc91b 100644
--- a/drivers/firmware/arm_scmi/smc.c
+++ b/drivers/firmware/arm_scmi/smc.c
@@ -27,11 +27,10 @@
struct scmi_smc {
struct scmi_chan_info *cinfo;
struct scmi_shared_mem __iomem *shmem;
+ struct mutex shmem_lock;
u32 func_id;
};
-static DEFINE_MUTEX(smc_mutex);
-
static bool smc_chan_available(struct device *dev, int idx)
{
return true;
@@ -78,6 +77,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
scmi_info->func_id = func_id;
scmi_info->cinfo = cinfo;
+ mutex_init(&scmi_info->shmem_lock);
cinfo->transport_info = scmi_info;
return 0;
@@ -102,14 +102,14 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
struct scmi_smc *scmi_info = cinfo->transport_info;
struct arm_smccc_res res;
- mutex_lock(&smc_mutex);
+ mutex_lock(&scmi_info->shmem_lock);
shmem_tx_prepare(scmi_info->shmem, xfer);
arm_smccc_1_1_invoke(scmi_info->func_id, 0, 0, 0, 0, 0, 0, 0, &res);
scmi_rx_callback(scmi_info->cinfo, shmem_read_header(scmi_info->shmem));
- mutex_unlock(&smc_mutex);
+ mutex_unlock(&scmi_info->shmem_lock);
return res.a0;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* RE: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
2020-03-27 16:36 ` [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific Sudeep Holla
@ 2020-04-01 1:12 ` Peng Fan
2020-04-01 9:12 ` Sudeep Holla
2020-04-01 9:14 ` Peng Fan
1 sibling, 1 reply; 16+ messages in thread
From: Peng Fan @ 2020-04-01 1:12 UTC (permalink / raw)
To: Sudeep Holla, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org
Cc: f.fainelli@gmail.com, dl-linux-imx
Hi Sudeep,
> Subject: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
>
> In order to support multiple SMC/HVC transport channels with associated
> shared memory,
Does this mean each channel will have its own shared memory? Or
All channels share the same shared memory?
it is better to maintain the mutex per channel instead of
> existing global one.
If all channels shared the same memory, use per channel mutex lock
will not be able to prevent other channels accessing shared memory
at the same time.
Thanks,
Peng.
>
> Move the smc_mutex into the scmi_smc structure and also rename it to
> shmem_lock which is more appropriate for it's use.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/firmware/arm_scmi/smc.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/firmware/arm_scmi/smc.c
> b/drivers/firmware/arm_scmi/smc.c index 336168e40f49..6dc8a88cc91b
> 100644
> --- a/drivers/firmware/arm_scmi/smc.c
> +++ b/drivers/firmware/arm_scmi/smc.c
> @@ -27,11 +27,10 @@
> struct scmi_smc {
> struct scmi_chan_info *cinfo;
> struct scmi_shared_mem __iomem *shmem;
> + struct mutex shmem_lock;
> u32 func_id;
> };
>
> -static DEFINE_MUTEX(smc_mutex);
> -
> static bool smc_chan_available(struct device *dev, int idx) {
> return true;
> @@ -78,6 +77,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo,
> struct device *dev,
>
> scmi_info->func_id = func_id;
> scmi_info->cinfo = cinfo;
> + mutex_init(&scmi_info->shmem_lock);
> cinfo->transport_info = scmi_info;
>
> return 0;
> @@ -102,14 +102,14 @@ static int smc_send_message(struct
> scmi_chan_info *cinfo,
> struct scmi_smc *scmi_info = cinfo->transport_info;
> struct arm_smccc_res res;
>
> - mutex_lock(&smc_mutex);
> + mutex_lock(&scmi_info->shmem_lock);
>
> shmem_tx_prepare(scmi_info->shmem, xfer);
>
> arm_smccc_1_1_invoke(scmi_info->func_id, 0, 0, 0, 0, 0, 0, 0, &res);
> scmi_rx_callback(scmi_info->cinfo,
> shmem_read_header(scmi_info->shmem));
>
> - mutex_unlock(&smc_mutex);
> + mutex_unlock(&scmi_info->shmem_lock);
>
> return res.a0;
> }
> --
> 2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
2020-04-01 1:12 ` Peng Fan
@ 2020-04-01 9:12 ` Sudeep Holla
2020-04-01 9:14 ` Peng Fan
0 siblings, 1 reply; 16+ messages in thread
From: Sudeep Holla @ 2020-04-01 9:12 UTC (permalink / raw)
To: Peng Fan
Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
f.fainelli@gmail.com, Sudeep Holla, dl-linux-imx
On Wed, Apr 01, 2020 at 01:12:37AM +0000, Peng Fan wrote:
> Hi Sudeep,
>
> > Subject: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
> >
> > In order to support multiple SMC/HVC transport channels with associated
> > shared memory,
>
> Does this mean each channel will have its own shared memory? Or
> All channels share the same shared memory?
>
It depends on platform firmware and DT. If there is only one shmem at
the top level scmi node, all share that single channel. If some/all
protocols have their own channel, they there must be shmem entry in the
corresponding child node.
> it is better to maintain the mutex per channel instead of
> > existing global one.
>
> If all channels shared the same memory, use per channel mutex lock
> will not be able to prevent other channels accessing shared memory
> at the same time.
>
No we don't create channel per protocol. If they share, we just share
the channel pointer. Look at:
if (!info->desc->ops->chan_available(dev, idx)) {
cinfo = idr_find(idr, SCMI_PROTOCOL_BASE);
if (unlikely(!cinfo)) /* Possible only if platform has no Rx */
return -EINVAL;
goto idr_alloc;
}
If a protocol doesn't have a dedicated channel, we just assign the base
protocol channel to it. We don't call chan_setup at all on that channel.
Your patch assumed so but the core driver never did that.
Hope this clarifies you doubt.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 16+ messages in thread* RE: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
2020-04-01 9:12 ` Sudeep Holla
@ 2020-04-01 9:14 ` Peng Fan
2020-04-01 9:28 ` Sudeep Holla
0 siblings, 1 reply; 16+ messages in thread
From: Peng Fan @ 2020-04-01 9:14 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
f.fainelli@gmail.com, dl-linux-imx
> Subject: Re: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
>
> On Wed, Apr 01, 2020 at 01:12:37AM +0000, Peng Fan wrote:
> > Hi Sudeep,
> >
> > > Subject: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
> > >
> > > In order to support multiple SMC/HVC transport channels with
> > > associated shared memory,
> >
> > Does this mean each channel will have its own shared memory? Or All
> > channels share the same shared memory?
> >
>
> It depends on platform firmware and DT. If there is only one shmem at the top
> level scmi node, all share that single channel. If some/all protocols have their
> own channel, they there must be shmem entry in the corresponding child
> node.
>
> > it is better to maintain the mutex per channel instead of
> > > existing global one.
> >
> > If all channels shared the same memory, use per channel mutex lock
> > will not be able to prevent other channels accessing shared memory at
> > the same time.
> >
>
> No we don't create channel per protocol. If they share, we just share the
> channel pointer. Look at:
>
> if (!info->desc->ops->chan_available(dev, idx)) {
> cinfo = idr_find(idr, SCMI_PROTOCOL_BASE);
> if (unlikely(!cinfo)) /* Possible only if platform has no Rx */
> return -EINVAL;
> goto idr_alloc;
> }
>
> If a protocol doesn't have a dedicated channel, we just assign the base
> protocol channel to it. We don't call chan_setup at all on that channel.
> Your patch assumed so but the core driver never did that.
>
> Hope this clarifies you doubt.
Yes. Thanks for the explainaiton.
Thanks,
Peng.
>
> --
> Regards,
> Sudeep
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
2020-04-01 9:14 ` Peng Fan
@ 2020-04-01 9:28 ` Sudeep Holla
0 siblings, 0 replies; 16+ messages in thread
From: Sudeep Holla @ 2020-04-01 9:28 UTC (permalink / raw)
To: Peng Fan
Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
f.fainelli@gmail.com, dl-linux-imx
On Wed, Apr 01, 2020 at 09:14:36AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
> >
> > On Wed, Apr 01, 2020 at 01:12:37AM +0000, Peng Fan wrote:
> > > Hi Sudeep,
> > >
> > > > Subject: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
> > > >
> > > > In order to support multiple SMC/HVC transport channels with
> > > > associated shared memory,
> > >
> > > Does this mean each channel will have its own shared memory? Or All
> > > channels share the same shared memory?
> > >
> >
> > It depends on platform firmware and DT. If there is only one shmem at the top
> > level scmi node, all share that single channel. If some/all protocols have their
> > own channel, they there must be shmem entry in the corresponding child
> > node.
> >
> > > it is better to maintain the mutex per channel instead of
> > > > existing global one.
> > >
> > > If all channels shared the same memory, use per channel mutex lock
> > > will not be able to prevent other channels accessing shared memory at
> > > the same time.
> > >
> >
> > No we don't create channel per protocol. If they share, we just share the
> > channel pointer. Look at:
> >
> > if (!info->desc->ops->chan_available(dev, idx)) {
> > cinfo = idr_find(idr, SCMI_PROTOCOL_BASE);
> > if (unlikely(!cinfo)) /* Possible only if platform has no Rx */
> > return -EINVAL;
> > goto idr_alloc;
> > }
> >
> > If a protocol doesn't have a dedicated channel, we just assign the base
> > protocol channel to it. We don't call chan_setup at all on that channel.
> > Your patch assumed so but the core driver never did that.
> >
> > Hope this clarifies you doubt.
>
> Yes. Thanks for the explainaiton.
>
No worries, I should have seen this during initial review, just missed
few trivial things.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
2020-03-27 16:36 ` [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific Sudeep Holla
2020-04-01 1:12 ` Peng Fan
@ 2020-04-01 9:14 ` Peng Fan
1 sibling, 0 replies; 16+ messages in thread
From: Peng Fan @ 2020-04-01 9:14 UTC (permalink / raw)
To: Sudeep Holla, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org
Cc: f.fainelli@gmail.com, dl-linux-imx
> Subject: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
>
> In order to support multiple SMC/HVC transport channels with associated
> shared memory, it is better to maintain the mutex per channel instead of
> existing global one.
>
> Move the smc_mutex into the scmi_smc structure and also rename it to
> shmem_lock which is more appropriate for it's use.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/firmware/arm_scmi/smc.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/firmware/arm_scmi/smc.c
> b/drivers/firmware/arm_scmi/smc.c index 336168e40f49..6dc8a88cc91b
> 100644
> --- a/drivers/firmware/arm_scmi/smc.c
> +++ b/drivers/firmware/arm_scmi/smc.c
> @@ -27,11 +27,10 @@
> struct scmi_smc {
> struct scmi_chan_info *cinfo;
> struct scmi_shared_mem __iomem *shmem;
> + struct mutex shmem_lock;
> u32 func_id;
> };
>
> -static DEFINE_MUTEX(smc_mutex);
> -
> static bool smc_chan_available(struct device *dev, int idx) {
> return true;
> @@ -78,6 +77,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo,
> struct device *dev,
>
> scmi_info->func_id = func_id;
> scmi_info->cinfo = cinfo;
> + mutex_init(&scmi_info->shmem_lock);
> cinfo->transport_info = scmi_info;
>
> return 0;
> @@ -102,14 +102,14 @@ static int smc_send_message(struct
> scmi_chan_info *cinfo,
> struct scmi_smc *scmi_info = cinfo->transport_info;
> struct arm_smccc_res res;
>
> - mutex_lock(&smc_mutex);
> + mutex_lock(&scmi_info->shmem_lock);
>
> shmem_tx_prepare(scmi_info->shmem, xfer);
>
> arm_smccc_1_1_invoke(scmi_info->func_id, 0, 0, 0, 0, 0, 0, 0, &res);
> scmi_rx_callback(scmi_info->cinfo,
> shmem_read_header(scmi_info->shmem));
>
> - mutex_unlock(&smc_mutex);
> + mutex_unlock(&scmi_info->shmem_lock);
>
> return res.a0;
> }
> --
> 2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/4] firmware: arm_scmi: Drop empty stub for smc_mark_txdone
2020-03-27 16:36 [PATCH 0/4] firmware: arm_scmi: Extend SMC/HVC to support multiple channels Sudeep Holla
2020-03-27 16:36 ` [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific Sudeep Holla
@ 2020-03-27 16:36 ` Sudeep Holla
2020-04-01 1:15 ` Peng Fan
2020-03-27 16:36 ` [PATCH 3/4] firmware: arm_scmi: Check shmem property for channel availablity Sudeep Holla
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Sudeep Holla @ 2020-03-27 16:36 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, Peng Fan
Cc: Sudeep Holla, f.fainelli, linux-imx
The scmi protocol core driver check for non NULL mark_txdone before
invoking the same. There is no need to provide a empty stub. SMC/HVC
calls are synchronous and the call return indicates the completion.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/arm_scmi/smc.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
index 6dc8a88cc91b..dd4b54c29679 100644
--- a/drivers/firmware/arm_scmi/smc.c
+++ b/drivers/firmware/arm_scmi/smc.c
@@ -114,10 +114,6 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
return res.a0;
}
-static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret)
-{
-}
-
static void smc_fetch_response(struct scmi_chan_info *cinfo,
struct scmi_xfer *xfer)
{
@@ -139,7 +135,6 @@ static struct scmi_transport_ops scmi_smc_ops = {
.chan_setup = smc_chan_setup,
.chan_free = smc_chan_free,
.send_message = smc_send_message,
- .mark_txdone = smc_mark_txdone,
.fetch_response = smc_fetch_response,
.poll_done = smc_poll_done,
};
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* RE: [PATCH 2/4] firmware: arm_scmi: Drop empty stub for smc_mark_txdone
2020-03-27 16:36 ` [PATCH 2/4] firmware: arm_scmi: Drop empty stub for smc_mark_txdone Sudeep Holla
@ 2020-04-01 1:15 ` Peng Fan
0 siblings, 0 replies; 16+ messages in thread
From: Peng Fan @ 2020-04-01 1:15 UTC (permalink / raw)
To: Sudeep Holla, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org
Cc: f.fainelli@gmail.com, dl-linux-imx
> Subject: [PATCH 2/4] firmware: arm_scmi: Drop empty stub for
> smc_mark_txdone
>
> The scmi protocol core driver check for non NULL mark_txdone before
> invoking the same. There is no need to provide a empty stub. SMC/HVC calls
> are synchronous and the call return indicates the completion.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/firmware/arm_scmi/smc.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/firmware/arm_scmi/smc.c
> b/drivers/firmware/arm_scmi/smc.c index 6dc8a88cc91b..dd4b54c29679
> 100644
> --- a/drivers/firmware/arm_scmi/smc.c
> +++ b/drivers/firmware/arm_scmi/smc.c
> @@ -114,10 +114,6 @@ static int smc_send_message(struct scmi_chan_info
> *cinfo,
> return res.a0;
> }
>
> -static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret) -{ -}
> -
> static void smc_fetch_response(struct scmi_chan_info *cinfo,
> struct scmi_xfer *xfer)
> {
> @@ -139,7 +135,6 @@ static struct scmi_transport_ops scmi_smc_ops = {
> .chan_setup = smc_chan_setup,
> .chan_free = smc_chan_free,
> .send_message = smc_send_message,
> - .mark_txdone = smc_mark_txdone,
> .fetch_response = smc_fetch_response,
> .poll_done = smc_poll_done,
> };
> --
> 2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/4] firmware: arm_scmi: Check shmem property for channel availablity
2020-03-27 16:36 [PATCH 0/4] firmware: arm_scmi: Extend SMC/HVC to support multiple channels Sudeep Holla
2020-03-27 16:36 ` [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific Sudeep Holla
2020-03-27 16:36 ` [PATCH 2/4] firmware: arm_scmi: Drop empty stub for smc_mark_txdone Sudeep Holla
@ 2020-03-27 16:36 ` Sudeep Holla
2020-04-01 1:15 ` Peng Fan
2020-03-27 16:36 ` [PATCH 4/4] firmware: arm_scmi: Drop checking for shmem property in parent node Sudeep Holla
2020-03-31 13:53 ` [PATCH 0/4] firmware: arm_scmi: Extend SMC/HVC to support multiple channels Peng Fan
4 siblings, 1 reply; 16+ messages in thread
From: Sudeep Holla @ 2020-03-27 16:36 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, Peng Fan
Cc: Sudeep Holla, f.fainelli, linux-imx
Instead of declaring the channel availabilty unconditionally, let us
check for the presence of "shmem" property and return the channel
availablity accordingly.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/arm_scmi/smc.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
index dd4b54c29679..5929c668dc1d 100644
--- a/drivers/firmware/arm_scmi/smc.c
+++ b/drivers/firmware/arm_scmi/smc.c
@@ -33,6 +33,11 @@ struct scmi_smc {
static bool smc_chan_available(struct device *dev, int idx)
{
+ struct device_node *np = of_parse_phandle(dev->of_node, "shmem", 0);
+ if (!np)
+ return false;
+
+ of_node_put(np);
return true;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* RE: [PATCH 3/4] firmware: arm_scmi: Check shmem property for channel availablity
2020-03-27 16:36 ` [PATCH 3/4] firmware: arm_scmi: Check shmem property for channel availablity Sudeep Holla
@ 2020-04-01 1:15 ` Peng Fan
2020-04-01 9:05 ` Sudeep Holla
0 siblings, 1 reply; 16+ messages in thread
From: Peng Fan @ 2020-04-01 1:15 UTC (permalink / raw)
To: Sudeep Holla, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org
Cc: f.fainelli@gmail.com, dl-linux-imx
> Subject: [PATCH 3/4] firmware: arm_scmi: Check shmem property for channel
> availablity
>
> Instead of declaring the channel availabilty unconditionally, let us check for
> the presence of "shmem" property and return the channel availablity
> accordingly.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/firmware/arm_scmi/smc.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/firmware/arm_scmi/smc.c
> b/drivers/firmware/arm_scmi/smc.c index dd4b54c29679..5929c668dc1d
> 100644
> --- a/drivers/firmware/arm_scmi/smc.c
> +++ b/drivers/firmware/arm_scmi/smc.c
> @@ -33,6 +33,11 @@ struct scmi_smc {
>
> static bool smc_chan_available(struct device *dev, int idx) {
> + struct device_node *np = of_parse_phandle(dev->of_node, "shmem", 0);
> + if (!np)
> + return false;
> +
> + of_node_put(np);
> return true;
> }
This is global shared mem:)
Reviewed-by: Peng Fan <peng.fan@nxp.com>
>
> --
> 2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 3/4] firmware: arm_scmi: Check shmem property for channel availablity
2020-04-01 1:15 ` Peng Fan
@ 2020-04-01 9:05 ` Sudeep Holla
0 siblings, 0 replies; 16+ messages in thread
From: Sudeep Holla @ 2020-04-01 9:05 UTC (permalink / raw)
To: Peng Fan
Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
f.fainelli@gmail.com, Sudeep Holla, dl-linux-imx
On Wed, Apr 01, 2020 at 01:15:08AM +0000, Peng Fan wrote:
> > Subject: [PATCH 3/4] firmware: arm_scmi: Check shmem property for channel
> > availablity
> >
> > Instead of declaring the channel availabilty unconditionally, let us check for
> > the presence of "shmem" property and return the channel availablity
> > accordingly.
> >
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> > drivers/firmware/arm_scmi/smc.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/firmware/arm_scmi/smc.c
> > b/drivers/firmware/arm_scmi/smc.c index dd4b54c29679..5929c668dc1d
> > 100644
> > --- a/drivers/firmware/arm_scmi/smc.c
> > +++ b/drivers/firmware/arm_scmi/smc.c
> > @@ -33,6 +33,11 @@ struct scmi_smc {
> >
> > static bool smc_chan_available(struct device *dev, int idx) {
> > + struct device_node *np = of_parse_phandle(dev->of_node, "shmem", 0);
> > + if (!np)
> > + return false;
> > +
> > + of_node_put(np);
> > return true;
> > }
>
> This is global shared mem:)
>
No, the dev pointer is not the parent node here but the child devices
unless I am reading it wrong. But yes global for the base protocol :)
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
>
Thanks for this and other patches too.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/4] firmware: arm_scmi: Drop checking for shmem property in parent node
2020-03-27 16:36 [PATCH 0/4] firmware: arm_scmi: Extend SMC/HVC to support multiple channels Sudeep Holla
` (2 preceding siblings ...)
2020-03-27 16:36 ` [PATCH 3/4] firmware: arm_scmi: Check shmem property for channel availablity Sudeep Holla
@ 2020-03-27 16:36 ` Sudeep Holla
2020-04-01 1:19 ` Peng Fan
2020-03-31 13:53 ` [PATCH 0/4] firmware: arm_scmi: Extend SMC/HVC to support multiple channels Peng Fan
4 siblings, 1 reply; 16+ messages in thread
From: Sudeep Holla @ 2020-03-27 16:36 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, Peng Fan
Cc: Sudeep Holla, f.fainelli, linux-imx
The scmi protocol core driver checks for the channel availability
before evaluating the shmem property. If the individual protocols
don't have separate channel assigned to them, the channel alloted
for the BASE protocol is reused automatically.
Therefore there is no need to check for the shmem property in the
parent node if it is absent in the child protocol node.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/arm_scmi/smc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
index 5929c668dc1d..833e793b5391 100644
--- a/drivers/firmware/arm_scmi/smc.c
+++ b/drivers/firmware/arm_scmi/smc.c
@@ -60,8 +60,6 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
return -ENOMEM;
np = of_parse_phandle(cdev->of_node, "shmem", 0);
- if (!np)
- np = of_parse_phandle(dev->of_node, "shmem", 0);
ret = of_address_to_resource(np, 0, &res);
of_node_put(np);
if (ret) {
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* RE: [PATCH 4/4] firmware: arm_scmi: Drop checking for shmem property in parent node
2020-03-27 16:36 ` [PATCH 4/4] firmware: arm_scmi: Drop checking for shmem property in parent node Sudeep Holla
@ 2020-04-01 1:19 ` Peng Fan
0 siblings, 0 replies; 16+ messages in thread
From: Peng Fan @ 2020-04-01 1:19 UTC (permalink / raw)
To: Sudeep Holla, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org
Cc: f.fainelli@gmail.com, dl-linux-imx
> -----Original Message-----
> From: Sudeep Holla <sudeep.holla@arm.com>
> Sent: 2020年3月28日 0:37
> To: linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; Peng
> Fan <peng.fan@nxp.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>; f.fainelli@gmail.com;
> dl-linux-imx <linux-imx@nxp.com>
> Subject: [PATCH 4/4] firmware: arm_scmi: Drop checking for shmem property
> in parent node
>
> The scmi protocol core driver checks for the channel availability before
> evaluating the shmem property. If the individual protocols don't have separate
> channel assigned to them, the channel alloted for the BASE protocol is reused
> automatically.
>
> Therefore there is no need to check for the shmem property in the parent
> node if it is absent in the child protocol node.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/firmware/arm_scmi/smc.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/firmware/arm_scmi/smc.c
> b/drivers/firmware/arm_scmi/smc.c index 5929c668dc1d..833e793b5391
> 100644
> --- a/drivers/firmware/arm_scmi/smc.c
> +++ b/drivers/firmware/arm_scmi/smc.c
> @@ -60,8 +60,6 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo,
> struct device *dev,
> return -ENOMEM;
>
> np = of_parse_phandle(cdev->of_node, "shmem", 0);
> - if (!np)
> - np = of_parse_phandle(dev->of_node, "shmem", 0);
> ret = of_address_to_resource(np, 0, &res);
> of_node_put(np);
> if (ret) {
> --
> 2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 0/4] firmware: arm_scmi: Extend SMC/HVC to support multiple channels
2020-03-27 16:36 [PATCH 0/4] firmware: arm_scmi: Extend SMC/HVC to support multiple channels Sudeep Holla
` (3 preceding siblings ...)
2020-03-27 16:36 ` [PATCH 4/4] firmware: arm_scmi: Drop checking for shmem property in parent node Sudeep Holla
@ 2020-03-31 13:53 ` Peng Fan
2020-03-31 14:21 ` Sudeep Holla
4 siblings, 1 reply; 16+ messages in thread
From: Peng Fan @ 2020-03-31 13:53 UTC (permalink / raw)
To: Sudeep Holla, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org
Cc: f.fainelli@gmail.com, dl-linux-imx
> Subject: [PATCH 0/4] firmware: arm_scmi: Extend SMC/HVC to support
> multiple channels
>
> Hi,
>
> While merging SMC/HVC transport to SCMI, I did a quick hack to extend it to
> support multiple channels. I may be missing something obvious, if not the
> changes required look simple. Some of them could be merged into original
> patch, but since I am unable to test, I preferred to keep them separate, easy
> to revert if things break.
With patchset applied to your scmi tree, my test still work as before.
Tested-by: Peng Fan <peng.fan@nxp.com>
Thanks,
Peng.
>
> Regards,
> Sudeep
> --
>
> Sudeep Holla (4):
> firmware: arm_scmi: Make mutex channel specific
> firmware: arm_scmi: Drop empty stub for smc_mark_txdone
> firmware: arm_scmi: Check shmem property for channel availablity
> firmware: arm_scmi: Drop checking for shmem property in parent node
>
> drivers/firmware/arm_scmi/smc.c | 20 +++++++++-----------
> 1 file changed, 9 insertions(+), 11 deletions(-)
>
> --
> 2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 0/4] firmware: arm_scmi: Extend SMC/HVC to support multiple channels
2020-03-31 13:53 ` [PATCH 0/4] firmware: arm_scmi: Extend SMC/HVC to support multiple channels Peng Fan
@ 2020-03-31 14:21 ` Sudeep Holla
0 siblings, 0 replies; 16+ messages in thread
From: Sudeep Holla @ 2020-03-31 14:21 UTC (permalink / raw)
To: Peng Fan
Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
f.fainelli@gmail.com, dl-linux-imx, Sudeep Holla
Hi Peng,
On Tue, Mar 31, 2020 at 01:53:40PM +0000, Peng Fan wrote:
> > Subject: [PATCH 0/4] firmware: arm_scmi: Extend SMC/HVC to support
> > multiple channels
> >
> > Hi,
> >
> > While merging SMC/HVC transport to SCMI, I did a quick hack to extend it to
> > support multiple channels. I may be missing something obvious, if not the
> > changes required look simple. Some of them could be merged into original
> > patch, but since I am unable to test, I preferred to keep them separate, easy
> > to revert if things break.
>
> With patchset applied to your scmi tree, my test still work as before.
>
> Tested-by: Peng Fan <peng.fan@nxp.com>
>
Thanks for testing. If you can give it a quick review as it is very small
change, that would be great. I just want more eyes to look at it just to
make sure I am not missing something obvious.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 16+ messages in thread