linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* RE: [PATCH 1/2] firmware: arm_scmi: mailbox: share shmem for protocols
       [not found] ` <20200206142204.GB3383@bogus>
@ 2020-02-07  2:08   ` Peng Fan
  0 siblings, 0 replies; 6+ messages in thread
From: Peng Fan @ 2020-02-07  2:08 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: viresh.kumar, f.fainelli, dl-linux-imx,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

> Subject: Re: [PATCH 1/2] firmware: arm_scmi: mailbox: share shmem for
> protocols
> 
> On Thu, Feb 06, 2020 at 08:57:25PM +0800, peng.fan@nxp.com wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > When shmem property of protocol is not specificed, let it use its
> > parent's shmem property.
> >
> 

+arm list

> Why do you even need this ? If shmem property of protocol is not specified
> then why is the mailbox property ? Either you need both or none. I don't see
> the point of this patch. I am interested to know how are you even hitting this
> case.

Without this patch, I need add shmem property in each protocol node.
With this patch, I only need to add shmem property in scmi node.

In mailbox_chan_setup, cdev is protocol device, input parameter device
is the parent, saying scmi device.

> 
> If you don't have mailbox property, then chan_available is false and hence we
> don't call chan_setup.

chan_avilable only parse mbox properties, not shmem.

Regards,
Peng.

> 
> --
> Regards,
> Sudeep

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH 2/2] firmware: arm_scmi: mark channel free when init
       [not found]   ` <20200206143337.GC3383@bogus>
@ 2020-02-07  2:16     ` Peng Fan
  2020-02-07 10:40       ` Sudeep Holla
  0 siblings, 1 reply; 6+ messages in thread
From: Peng Fan @ 2020-02-07  2:16 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: viresh.kumar, f.fainelli, dl-linux-imx,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel


> Subject: Re: [PATCH 2/2] firmware: arm_scmi: mark channel free when init
> 
> On Thu, Feb 06, 2020 at 08:57:26PM +0800, peng.fan@nxp.com wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > The firmware itself might not mark channel free, so let's explicitly
> > mark it free when do initialization.
> >
> > Also move struct scmi_shared_mem to common.h
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/firmware/arm_scmi/common.h  | 19 +++++++++++++++++--
> > drivers/firmware/arm_scmi/mailbox.c |  2 ++
> >  drivers/firmware/arm_scmi/shmem.c   | 18 ------------------
> >  3 files changed, 19 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/firmware/arm_scmi/common.h
> > b/drivers/firmware/arm_scmi/common.h
> > index fd091a4ccbff..5df262a564a4 100644
> > --- a/drivers/firmware/arm_scmi/common.h
> > +++ b/drivers/firmware/arm_scmi/common.h
> > @@ -211,8 +211,23 @@ extern const struct scmi_desc scmi_mailbox_desc;
> > void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr);
> > void scmi_free_channel(struct scmi_chan_info *cinfo, struct idr *idr,
> > int id);
> >
> > -/* shmem related declarations */
> > -struct scmi_shared_mem;
> > +/*
> > + * SCMI specification requires all parameters, message headers,
> > +return
> > + * arguments or any protocol data to be expressed in little endian
> > + * format only.
> > + */
> > +struct scmi_shared_mem {
> > +	__le32 reserved;
> > +	__le32 channel_status;
> > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR	BIT(1)
> > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE	BIT(0)
> > +	__le32 reserved1[2];
> > +	__le32 flags;
> > +#define SCMI_SHMEM_FLAG_INTR_ENABLED	BIT(0)
> > +	__le32 length;
> > +	__le32 msg_header;
> > +	u8 msg_payload[0];
> > +};
> >
> >  void shmem_tx_prepare(struct scmi_shared_mem __iomem *shmem,
> >  		      struct scmi_xfer *xfer);
> > diff --git a/drivers/firmware/arm_scmi/mailbox.c
> > b/drivers/firmware/arm_scmi/mailbox.c
> > index 68ed58e2a47a..2d34bf6e94e2 100644
> > --- a/drivers/firmware/arm_scmi/mailbox.c
> > +++ b/drivers/firmware/arm_scmi/mailbox.c
> > @@ -104,6 +104,8 @@ static int mailbox_chan_setup(struct
> scmi_chan_info *cinfo, struct device *dev,
> >  	cinfo->transport_info = smbox;
> >  	smbox->cinfo = cinfo;
> >
> > +	iowrite32(BIT(0), &smbox->shmem->channel_status);
> > +
> 

+arm list

> If we need this then we may need to put this as a function in shmem.c I am
> still not convinced if we can do this unconditionally, i.e. will that affect Rx
> channel if there's notification pending before we initialise. But we can deal
> with that later.

Per understanding, channel is specific to an agent, it could not be shared.
So the shmem binded to the channel will not be used by others.

Since this is the initialization process, the firmware might not init the shmem.

The shmem.c shmem_tx_prepare will spin until channel free, so I did the patch.
Otherwise it might spin forever.

I'll add a check as following
if (tx)
 iowrite32(BIT(0), &smbox->shmem->channel_status);

I not find a good place to put this in shmem.c (:

> 
> Also what about error fields ? I would rather clear it to 0, not just BIT(0)

Tx channel error should also be cleared, fix in v2.

Thanks,
Peng

> 
> --
> Regards,
> Sudeep

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] firmware: arm_scmi: mark channel free when init
  2020-02-07  2:16     ` [PATCH 2/2] firmware: arm_scmi: mark channel free when init Peng Fan
@ 2020-02-07 10:40       ` Sudeep Holla
  2020-02-07 10:44         ` Peng Fan
  2020-02-09 13:19         ` Peng Fan
  0 siblings, 2 replies; 6+ messages in thread
From: Sudeep Holla @ 2020-02-07 10:40 UTC (permalink / raw)
  To: Peng Fan
  Cc: viresh.kumar, f.fainelli, dl-linux-imx,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel, Sudeep Holla

On Fri, Feb 07, 2020 at 02:16:04AM +0000, Peng Fan wrote:
>
> > Subject: Re: [PATCH 2/2] firmware: arm_scmi: mark channel free when init
> >
> > On Thu, Feb 06, 2020 at 08:57:26PM +0800, peng.fan@nxp.com wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > The firmware itself might not mark channel free, so let's explicitly
> > > mark it free when do initialization.
> > >
> > > Also move struct scmi_shared_mem to common.h
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >  drivers/firmware/arm_scmi/common.h  | 19 +++++++++++++++++--
> > > drivers/firmware/arm_scmi/mailbox.c |  2 ++
> > >  drivers/firmware/arm_scmi/shmem.c   | 18 ------------------
> > >  3 files changed, 19 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/drivers/firmware/arm_scmi/common.h
> > > b/drivers/firmware/arm_scmi/common.h
> > > index fd091a4ccbff..5df262a564a4 100644
> > > --- a/drivers/firmware/arm_scmi/common.h
> > > +++ b/drivers/firmware/arm_scmi/common.h
> > > @@ -211,8 +211,23 @@ extern const struct scmi_desc scmi_mailbox_desc;
> > > void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr);
> > > void scmi_free_channel(struct scmi_chan_info *cinfo, struct idr *idr,
> > > int id);
> > >
> > > -/* shmem related declarations */
> > > -struct scmi_shared_mem;
> > > +/*
> > > + * SCMI specification requires all parameters, message headers,
> > > +return
> > > + * arguments or any protocol data to be expressed in little endian
> > > + * format only.
> > > + */
> > > +struct scmi_shared_mem {
> > > +	__le32 reserved;
> > > +	__le32 channel_status;
> > > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR	BIT(1)
> > > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE	BIT(0)
> > > +	__le32 reserved1[2];
> > > +	__le32 flags;
> > > +#define SCMI_SHMEM_FLAG_INTR_ENABLED	BIT(0)
> > > +	__le32 length;
> > > +	__le32 msg_header;
> > > +	u8 msg_payload[0];
> > > +};
> > >
> > >  void shmem_tx_prepare(struct scmi_shared_mem __iomem *shmem,
> > >  		      struct scmi_xfer *xfer);
> > > diff --git a/drivers/firmware/arm_scmi/mailbox.c
> > > b/drivers/firmware/arm_scmi/mailbox.c
> > > index 68ed58e2a47a..2d34bf6e94e2 100644
> > > --- a/drivers/firmware/arm_scmi/mailbox.c
> > > +++ b/drivers/firmware/arm_scmi/mailbox.c
> > > @@ -104,6 +104,8 @@ static int mailbox_chan_setup(struct
> > scmi_chan_info *cinfo, struct device *dev,
> > >  	cinfo->transport_info = smbox;
> > >  	smbox->cinfo = cinfo;
> > >
> > > +	iowrite32(BIT(0), &smbox->shmem->channel_status);
> > > +
> >
>
> +arm list
>
> > If we need this then we may need to put this as a function in shmem.c I am
> > still not convinced if we can do this unconditionally, i.e. will that affect Rx
> > channel if there's notification pending before we initialise. But we can deal
> > with that later.
>
> Per understanding, channel is specific to an agent, it could not be shared.
> So the shmem binded to the channel will not be used by others.
>

Yes

> Since this is the initialization process, the firmware might not init the shmem.
>

But, is there any particular reason for firmware not to ? It means platform
holds the channel and needs to release it to agent(OSPM) in this case after
initialisation.

> The shmem.c shmem_tx_prepare will spin until channel free, so I did the patch.
> Otherwise it might spin forever.
>

Yes I guessed that to be reason.

> I'll add a check as following
> if (tx)
>  iowrite32(BIT(0), &smbox->shmem->channel_status);
>

Not yet, I need answers to above query.

> I not find a good place to put this in shmem.c (:
>

Least of the problem :), let us first agree if we have to have it.

> >
> > Also what about error fields ? I would rather clear it to 0, not just BIT(0)
>
> Tx channel error should also be cleared, fix in v2.
>

OK but wait for a while before you post for the discussion to end.

--
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH 2/2] firmware: arm_scmi: mark channel free when init
  2020-02-07 10:40       ` Sudeep Holla
@ 2020-02-07 10:44         ` Peng Fan
  2020-02-09 13:19         ` Peng Fan
  1 sibling, 0 replies; 6+ messages in thread
From: Peng Fan @ 2020-02-07 10:44 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: viresh.kumar, f.fainelli, dl-linux-imx,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

> Subject: Re: [PATCH 2/2] firmware: arm_scmi: mark channel free when init
> 
> On Fri, Feb 07, 2020 at 02:16:04AM +0000, Peng Fan wrote:
> >
> > > Subject: Re: [PATCH 2/2] firmware: arm_scmi: mark channel free when
> > > init
> > >
> > > On Thu, Feb 06, 2020 at 08:57:26PM +0800, peng.fan@nxp.com wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > The firmware itself might not mark channel free, so let's
> > > > explicitly mark it free when do initialization.
> > > >
> > > > Also move struct scmi_shared_mem to common.h
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > ---
> > > >  drivers/firmware/arm_scmi/common.h  | 19 +++++++++++++++++--
> > > > drivers/firmware/arm_scmi/mailbox.c |  2 ++
> > > >  drivers/firmware/arm_scmi/shmem.c   | 18 ------------------
> > > >  3 files changed, 19 insertions(+), 20 deletions(-)
> > > >
> > > > diff --git a/drivers/firmware/arm_scmi/common.h
> > > > b/drivers/firmware/arm_scmi/common.h
> > > > index fd091a4ccbff..5df262a564a4 100644
> > > > --- a/drivers/firmware/arm_scmi/common.h
> > > > +++ b/drivers/firmware/arm_scmi/common.h
> > > > @@ -211,8 +211,23 @@ extern const struct scmi_desc
> > > > scmi_mailbox_desc; void scmi_rx_callback(struct scmi_chan_info
> > > > *cinfo, u32 msg_hdr); void scmi_free_channel(struct scmi_chan_info
> > > > *cinfo, struct idr *idr, int id);
> > > >
> > > > -/* shmem related declarations */
> > > > -struct scmi_shared_mem;
> > > > +/*
> > > > + * SCMI specification requires all parameters, message headers,
> > > > +return
> > > > + * arguments or any protocol data to be expressed in little
> > > > +endian
> > > > + * format only.
> > > > + */
> > > > +struct scmi_shared_mem {
> > > > +	__le32 reserved;
> > > > +	__le32 channel_status;
> > > > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR	BIT(1)
> > > > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE	BIT(0)
> > > > +	__le32 reserved1[2];
> > > > +	__le32 flags;
> > > > +#define SCMI_SHMEM_FLAG_INTR_ENABLED	BIT(0)
> > > > +	__le32 length;
> > > > +	__le32 msg_header;
> > > > +	u8 msg_payload[0];
> > > > +};
> > > >
> > > >  void shmem_tx_prepare(struct scmi_shared_mem __iomem *shmem,
> > > >  		      struct scmi_xfer *xfer);
> > > > diff --git a/drivers/firmware/arm_scmi/mailbox.c
> > > > b/drivers/firmware/arm_scmi/mailbox.c
> > > > index 68ed58e2a47a..2d34bf6e94e2 100644
> > > > --- a/drivers/firmware/arm_scmi/mailbox.c
> > > > +++ b/drivers/firmware/arm_scmi/mailbox.c
> > > > @@ -104,6 +104,8 @@ static int mailbox_chan_setup(struct
> > > scmi_chan_info *cinfo, struct device *dev,
> > > >  	cinfo->transport_info = smbox;
> > > >  	smbox->cinfo = cinfo;
> > > >
> > > > +	iowrite32(BIT(0), &smbox->shmem->channel_status);
> > > > +
> > >
> >
> > +arm list
> >
> > > If we need this then we may need to put this as a function in
> > > shmem.c I am still not convinced if we can do this unconditionally,
> > > i.e. will that affect Rx channel if there's notification pending
> > > before we initialise. But we can deal with that later.
> >
> > Per understanding, channel is specific to an agent, it could not be shared.
> > So the shmem binded to the channel will not be used by others.
> >
> 
> Yes
> 
> > Since this is the initialization process, the firmware might not init the
> shmem.
> >
> 
> But, is there any particular reason for firmware not to ? It means platform
> holds the channel and needs to release it to agent(OSPM) in this case after
> initialisation.

It is just my ATF not initialize the shmem area the leave the area with random
data.

So I think that some released buggy firmware might also has similar issue.

To support buggy firmware and bug-fixed firmware, I think it might be helpful
to init shmem in Linux side.

Thanks,
Peng.

> 
> > The shmem.c shmem_tx_prepare will spin until channel free, so I did the
> patch.
> > Otherwise it might spin forever.
> >
> 
> Yes I guessed that to be reason.
> 
> > I'll add a check as following
> > if (tx)
> >  iowrite32(BIT(0), &smbox->shmem->channel_status);
> >
> 
> Not yet, I need answers to above query.
> 
> > I not find a good place to put this in shmem.c (:
> >
> 
> Least of the problem :), let us first agree if we have to have it.
> 
> > >
> > > Also what about error fields ? I would rather clear it to 0, not
> > > just BIT(0)
> >
> > Tx channel error should also be cleared, fix in v2.
> >
> 
> OK but wait for a while before you post for the discussion to end.
> 
> --
> Regards,
> Sudeep

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH 2/2] firmware: arm_scmi: mark channel free when init
  2020-02-07 10:40       ` Sudeep Holla
  2020-02-07 10:44         ` Peng Fan
@ 2020-02-09 13:19         ` Peng Fan
  1 sibling, 0 replies; 6+ messages in thread
From: Peng Fan @ 2020-02-09 13:19 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: viresh.kumar, f.fainelli, dl-linux-imx,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

Hi Sudeep

> Subject: Re: [PATCH 2/2] firmware: arm_scmi: mark channel free when init
> 
> On Fri, Feb 07, 2020 at 02:16:04AM +0000, Peng Fan wrote:
> >
> > > Subject: Re: [PATCH 2/2] firmware: arm_scmi: mark channel free when
> > > init
> > >
> > > On Thu, Feb 06, 2020 at 08:57:26PM +0800, peng.fan@nxp.com wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > The firmware itself might not mark channel free, so let's
> > > > explicitly mark it free when do initialization.
> > > >
> > > > Also move struct scmi_shared_mem to common.h
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > ---
> > > >  drivers/firmware/arm_scmi/common.h  | 19 +++++++++++++++++--
> > > > drivers/firmware/arm_scmi/mailbox.c |  2 ++
> > > >  drivers/firmware/arm_scmi/shmem.c   | 18 ------------------
> > > >  3 files changed, 19 insertions(+), 20 deletions(-)
> > > >
> > > > diff --git a/drivers/firmware/arm_scmi/common.h
> > > > b/drivers/firmware/arm_scmi/common.h
> > > > index fd091a4ccbff..5df262a564a4 100644
> > > > --- a/drivers/firmware/arm_scmi/common.h
> > > > +++ b/drivers/firmware/arm_scmi/common.h
> > > > @@ -211,8 +211,23 @@ extern const struct scmi_desc
> > > > scmi_mailbox_desc; void scmi_rx_callback(struct scmi_chan_info
> > > > *cinfo, u32 msg_hdr); void scmi_free_channel(struct scmi_chan_info
> > > > *cinfo, struct idr *idr, int id);
> > > >
> > > > -/* shmem related declarations */
> > > > -struct scmi_shared_mem;
> > > > +/*
> > > > + * SCMI specification requires all parameters, message headers,
> > > > +return
> > > > + * arguments or any protocol data to be expressed in little
> > > > +endian
> > > > + * format only.
> > > > + */
> > > > +struct scmi_shared_mem {
> > > > +	__le32 reserved;
> > > > +	__le32 channel_status;
> > > > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR	BIT(1)
> > > > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE	BIT(0)
> > > > +	__le32 reserved1[2];
> > > > +	__le32 flags;
> > > > +#define SCMI_SHMEM_FLAG_INTR_ENABLED	BIT(0)
> > > > +	__le32 length;
> > > > +	__le32 msg_header;
> > > > +	u8 msg_payload[0];
> > > > +};
> > > >
> > > >  void shmem_tx_prepare(struct scmi_shared_mem __iomem *shmem,
> > > >  		      struct scmi_xfer *xfer);
> > > > diff --git a/drivers/firmware/arm_scmi/mailbox.c
> > > > b/drivers/firmware/arm_scmi/mailbox.c
> > > > index 68ed58e2a47a..2d34bf6e94e2 100644
> > > > --- a/drivers/firmware/arm_scmi/mailbox.c
> > > > +++ b/drivers/firmware/arm_scmi/mailbox.c
> > > > @@ -104,6 +104,8 @@ static int mailbox_chan_setup(struct
> > > scmi_chan_info *cinfo, struct device *dev,
> > > >  	cinfo->transport_info = smbox;
> > > >  	smbox->cinfo = cinfo;
> > > >
> > > > +	iowrite32(BIT(0), &smbox->shmem->channel_status);
> > > > +
> > >
> >
> > +arm list
> >
> > > If we need this then we may need to put this as a function in
> > > shmem.c I am still not convinced if we can do this unconditionally,
> > > i.e. will that affect Rx channel if there's notification pending
> > > before we initialise. But we can deal with that later.
> >
> > Per understanding, channel is specific to an agent, it could not be shared.
> > So the shmem binded to the channel will not be used by others.
> >
> 
> Yes
> 
> > Since this is the initialization process, the firmware might not init the
> shmem.
> >
> 
> But, is there any particular reason for firmware not to ? It means platform
> holds the channel and needs to release it to agent(OSPM) in this case after
> initialisation.
> 
> > The shmem.c shmem_tx_prepare will spin until channel free, so I did the
> patch.
> > Otherwise it might spin forever.
> >
> 
> Yes I guessed that to be reason.
> 
> > I'll add a check as following
> > if (tx)
> >  iowrite32(BIT(0), &smbox->shmem->channel_status);
> >
> 
> Not yet, I need answers to above query.

After recheck the SCMI 2.0 spec,

Shared memory area:
This is an area of memory that is shared between the caller and the callee.
At any point in time, the shared memory is owned by the caller or the callee.
The ownership is reflected by a channel status word in the shared memory area.

For TX shared memory, it not explains who should initialize the channel status,
callee or the caller.

In my case, I not let firmware initialize the shared memory, so it might contain
garbage data and not able to wait channel free.

So I think add a check of tx and set channel to free when scmi driver probe
, this is valid.

Or update spec to explicitly show who should initialize channel?

Thanks,
Peng.

> 
> > I not find a good place to put this in shmem.c (:
> >
> 
> Least of the problem :), let us first agree if we have to have it.
> 
> > >
> > > Also what about error fields ? I would rather clear it to 0, not
> > > just BIT(0)
> >
> > Tx channel error should also be cleared, fix in v2.
> >
> 
> OK but wait for a while before you post for the discussion to end.
> 
> --
> Regards,
> Sudeep

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] firmware: arm_scmi: mark channel free when init
  2020-02-06 12:59 [PATCH 1/2] firmware: arm_scmi: mailbox: share shmem for protocols peng.fan
@ 2020-02-06 12:59 ` peng.fan
  0 siblings, 0 replies; 6+ messages in thread
From: peng.fan @ 2020-02-06 12:59 UTC (permalink / raw)
  To: sudeep.holla
  Cc: viresh.kumar, f.fainelli, linux-imx, linux-arm-kernel,
	linux-kernel, Peng Fan

From: Peng Fan <peng.fan@nxp.com>

The firmware itself might not mark channel free, so let's explicitly 
mark it free when do initialization.

Also move struct scmi_shared_mem to common.h

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/arm_scmi/common.h  | 19 +++++++++++++++++--
 drivers/firmware/arm_scmi/mailbox.c |  2 ++
 drivers/firmware/arm_scmi/shmem.c   | 18 ------------------
 3 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index fd091a4ccbff..5df262a564a4 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -211,8 +211,23 @@ extern const struct scmi_desc scmi_mailbox_desc;
 void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr);
 void scmi_free_channel(struct scmi_chan_info *cinfo, struct idr *idr, int id);
 
-/* shmem related declarations */
-struct scmi_shared_mem;
+/*
+ * SCMI specification requires all parameters, message headers, return
+ * arguments or any protocol data to be expressed in little endian
+ * format only.
+ */
+struct scmi_shared_mem {
+	__le32 reserved;
+	__le32 channel_status;
+#define SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR	BIT(1)
+#define SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE	BIT(0)
+	__le32 reserved1[2];
+	__le32 flags;
+#define SCMI_SHMEM_FLAG_INTR_ENABLED	BIT(0)
+	__le32 length;
+	__le32 msg_header;
+	u8 msg_payload[0];
+};
 
 void shmem_tx_prepare(struct scmi_shared_mem __iomem *shmem,
 		      struct scmi_xfer *xfer);
diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c
index 68ed58e2a47a..2d34bf6e94e2 100644
--- a/drivers/firmware/arm_scmi/mailbox.c
+++ b/drivers/firmware/arm_scmi/mailbox.c
@@ -104,6 +104,8 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
 	cinfo->transport_info = smbox;
 	smbox->cinfo = cinfo;
 
+	iowrite32(BIT(0), &smbox->shmem->channel_status);
+
 	return 0;
 }
 
diff --git a/drivers/firmware/arm_scmi/shmem.c b/drivers/firmware/arm_scmi/shmem.c
index 02c6c7540f42..c9e334654ba3 100644
--- a/drivers/firmware/arm_scmi/shmem.c
+++ b/drivers/firmware/arm_scmi/shmem.c
@@ -11,24 +11,6 @@
 
 #include "common.h"
 
-/*
- * SCMI specification requires all parameters, message headers, return
- * arguments or any protocol data to be expressed in little endian
- * format only.
- */
-struct scmi_shared_mem {
-	__le32 reserved;
-	__le32 channel_status;
-#define SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR	BIT(1)
-#define SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE	BIT(0)
-	__le32 reserved1[2];
-	__le32 flags;
-#define SCMI_SHMEM_FLAG_INTR_ENABLED	BIT(0)
-	__le32 length;
-	__le32 msg_header;
-	u8 msg_payload[0];
-};
-
 void shmem_tx_prepare(struct scmi_shared_mem __iomem *shmem,
 		      struct scmi_xfer *xfer)
 {
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-02-09 13:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1580993846-17712-1-git-send-email-peng.fan@nxp.com>
     [not found] ` <20200206142204.GB3383@bogus>
2020-02-07  2:08   ` [PATCH 1/2] firmware: arm_scmi: mailbox: share shmem for protocols Peng Fan
     [not found] ` <1580993846-17712-2-git-send-email-peng.fan@nxp.com>
     [not found]   ` <20200206143337.GC3383@bogus>
2020-02-07  2:16     ` [PATCH 2/2] firmware: arm_scmi: mark channel free when init Peng Fan
2020-02-07 10:40       ` Sudeep Holla
2020-02-07 10:44         ` Peng Fan
2020-02-09 13:19         ` Peng Fan
2020-02-06 12:59 [PATCH 1/2] firmware: arm_scmi: mailbox: share shmem for protocols peng.fan
2020-02-06 12:59 ` [PATCH 2/2] firmware: arm_scmi: mark channel free when init peng.fan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).