[PATCH v2 0/2] mailbox: pcc: export the PCC subspace type

[PATCH v2 0/2] mailbox: pcc: export the PCC subspace type

[Huisong Li]

Currently, it seems that the subspace type in all drivers used PCC is fixed
and not obtained from their platform.
And PCCT is a best natural way to export it.

v1->v2:
 - add one patch to use PCC subspace type in kunpeng_hccs

Huisong Li (2):
  mailbox: pcc: export the PCC subspace type
  soc: kunpeng_hccs: add the check for PCC subspace type

 drivers/mailbox/pcc.c                | 10 ++++------
 drivers/soc/hisilicon/kunpeng_hccs.c | 17 +++++++++++++++++
 include/acpi/pcc.h                   |  1 +
 3 files changed, 22 insertions(+), 6 deletions(-)

-- 
2.33.0

[PATCH v2 1/2] mailbox: pcc: export the PCC subspace type

[Huisong Li]

As stated in APCI spec, the size of the subspace shared memory region may
be different for different types. So it is useful for driver to fill PCC
communication space. But the driver used this PCC channel doesn't know what
is the subspace type of the channel.

So export the PCC subspace type by requesting PCC channel.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/mailbox/pcc.c | 10 ++++------
 include/acpi/pcc.h    |  1 +
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 94885e411085..9742cc7837bd 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -91,7 +91,6 @@ struct pcc_chan_reg {
  * @cmd_update: PCC register bundle for the command complete update register
  * @error: PCC register bundle for the error status register
  * @plat_irq: platform interrupt
- * @type: PCC subspace type
  * @plat_irq_flags: platform interrupt flags
  * @chan_in_use: this flag is used just to check if the interrupt needs
  *		handling when it is shared. Since only one transfer can occur
@@ -108,7 +107,6 @@ struct pcc_chan_info {
 	struct pcc_chan_reg cmd_update;
 	struct pcc_chan_reg error;
 	int plat_irq;
-	u8 type;
 	unsigned int plat_irq_flags;
 	bool chan_in_use;
 };
@@ -263,7 +261,7 @@ static bool pcc_mbox_cmd_complete_check(struct pcc_chan_info *pchan)
 	 * bit 0 indicates that Platform is sending a notification and OSPM
 	 * needs to respond this interrupt to process this command.
 	 */
-	if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
+	if (pchan->chan.type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
 		return !val;
 
 	return !!val;
@@ -284,7 +282,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 	int ret;
 
 	pchan = chan->con_priv;
-	if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE &&
+	if (pchan->chan.type == ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE &&
 	    !pchan->chan_in_use)
 		return IRQ_NONE;
 
@@ -312,7 +310,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 	 *
 	 * The PCC master subspace channel clears chan_in_use to free channel.
 	 */
-	if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
+	if (pchan->chan.type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
 		pcc_send_data(chan, NULL);
 	pchan->chan_in_use = false;
 
@@ -766,7 +764,7 @@ static int pcc_mbox_probe(struct platform_device *pdev)
 
 		pcc_parse_subspace_shmem(pchan, pcct_entry);
 
-		pchan->type = pcct_entry->type;
+		pchan->chan.type = pcct_entry->type;
 		pcct_entry = (struct acpi_subtable_header *)
 			((unsigned long) pcct_entry + pcct_entry->length);
 	}
diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
index 73e806fe7ce7..eddf9f5dfba3 100644
--- a/include/acpi/pcc.h
+++ b/include/acpi/pcc.h
@@ -16,6 +16,7 @@ struct pcc_mbox_chan {
 	u32 latency;
 	u32 max_access_rate;
 	u16 min_turnaround_time;
+	u8 type;
 };
 
 #define MAX_PCC_SUBSPACES	256
-- 
2.33.0

[PATCH v2 2/2] soc: kunpeng_hccs: add the check for PCC subspace type

[Huisong Li]

Currently, HCCS driver directly uses Generic Communications Channel Shared
Memory Region which is used in type0/1/2 to communicate with platform,
but actually doesn't support type3/4/5.
So this patch adds the check for PCC subspace type.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/soc/hisilicon/kunpeng_hccs.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c
index f3810d9d1caa..4ba3bfd45a01 100644
--- a/drivers/soc/hisilicon/kunpeng_hccs.c
+++ b/drivers/soc/hisilicon/kunpeng_hccs.c
@@ -174,6 +174,19 @@ static int hccs_register_pcc_channel(struct hccs_dev *hdev)
 	return rc;
 }
 
+static int hccs_check_pcc_info(struct hccs_dev *hdev)
+{
+	struct pcc_mbox_chan *pcc_chan = hdev->cl_info.pcc_chan;
+
+	if (pcc_chan->type >= ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE) {
+		dev_err(hdev->dev, "unsupport for subspace type%u.\n",
+			pcc_chan->type);
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
 static int hccs_check_chan_cmd_complete(struct hccs_dev *hdev)
 {
 	struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
@@ -1224,6 +1237,10 @@ static int hccs_probe(struct platform_device *pdev)
 	if (rc)
 		return rc;
 
+	rc = hccs_check_pcc_info(hdev);
+	if (rc)
+		goto unregister_pcc_chan;
+
 	rc = hccs_get_dev_caps(hdev);
 	if (rc)
 		goto unregister_pcc_chan;
-- 
2.33.0

Re: [PATCH v2 2/2] soc: kunpeng_hccs: add the check for PCC subspace type

[Sudeep Holla]

On Wed, Sep 20, 2023 at 02:47:03PM +0800, Huisong Li wrote:
> Currently, HCCS driver directly uses Generic Communications Channel Shared
> Memory Region which is used in type0/1/2 to communicate with platform,
> but actually doesn't support type3/4/5.
> So this patch adds the check for PCC subspace type.
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>  drivers/soc/hisilicon/kunpeng_hccs.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c
> index f3810d9d1caa..4ba3bfd45a01 100644
> --- a/drivers/soc/hisilicon/kunpeng_hccs.c
> +++ b/drivers/soc/hisilicon/kunpeng_hccs.c
> @@ -174,6 +174,19 @@ static int hccs_register_pcc_channel(struct hccs_dev *hdev)
>  	return rc;
>  }
>  
> +static int hccs_check_pcc_info(struct hccs_dev *hdev)
> +{
> +	struct pcc_mbox_chan *pcc_chan = hdev->cl_info.pcc_chan;
> +
> +	if (pcc_chan->type >= ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE) {
> +		dev_err(hdev->dev, "unsupport for subspace type%u.\n",
> +			pcc_chan->type);
> +		return -EOPNOTSUPP;
> +	}
 
Is this the only use of the PCC type information you have or do you plan to
use it for something other than the validation.

Just for sake of argument, I can say all users of PCC must then do the
similar validation. I don't know where to draw the line here.

Ideally I would expect the driver to make this transparent and give error
during transmit if not supported.

The driver must be able to work with different PCC type to support variety
of platforms TBH. What is the issue exactly here ? Is this to prevent the
use of Type 4 ? I think we must do something better but I don't know what
that is yet.

-- 
Regards,
Sudeep

Re: [PATCH v2 2/2] soc: kunpeng_hccs: add the check for PCC subspace type

[lihuisong (C)]

在 2023/9/20 22:11, Sudeep Holla 写道:
> On Wed, Sep 20, 2023 at 02:47:03PM +0800, Huisong Li wrote:
>> Currently, HCCS driver directly uses Generic Communications Channel Shared
>> Memory Region which is used in type0/1/2 to communicate with platform,
>> but actually doesn't support type3/4/5.
>> So this patch adds the check for PCC subspace type.
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>>   drivers/soc/hisilicon/kunpeng_hccs.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c
>> index f3810d9d1caa..4ba3bfd45a01 100644
>> --- a/drivers/soc/hisilicon/kunpeng_hccs.c
>> +++ b/drivers/soc/hisilicon/kunpeng_hccs.c
>> @@ -174,6 +174,19 @@ static int hccs_register_pcc_channel(struct hccs_dev *hdev)
>>   	return rc;
>>   }
>>   
>> +static int hccs_check_pcc_info(struct hccs_dev *hdev)
>> +{
>> +	struct pcc_mbox_chan *pcc_chan = hdev->cl_info.pcc_chan;
>> +
>> +	if (pcc_chan->type >= ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE) {
>> +		dev_err(hdev->dev, "unsupport for subspace type%u.\n",
>> +			pcc_chan->type);
>> +		return -EOPNOTSUPP;
>> +	}
>   
> Is this the only use of the PCC type information you have or do you plan to
> use it for something other than the validation.
Yeah, it is just validation now. we want to plan this driver can support 
more types.
>
> Just for sake of argument, I can say all users of PCC must then do the
> similar validation. I don't know where to draw the line here.

If export PCC type, it is good for the user of PCC to be more universal 
and more compatible.

>
> Ideally I would expect the driver to make this transparent and give error
> during transmit if not supported.
I understand you.
I just check this type only once during the initializing phase.
Otherwise, every once need to verify it when send PCC command.
>
> The driver must be able to work with different PCC type to support variety
> of platforms TBH. What is the issue exactly here ? Is this to prevent the
Agree more with you.
IMO, the user of PCC has the ability to support variety of platforms if 
they can get PCC type.
In this case, to prevent type 4 is necessary if driver cannot act as a 
slave.
on the other hand, If one driver acts as a slave, platform must supply 
slave subspace for them.
> use of Type 4 ? I think we must do something better but I don't know what
> that is yet.

Yes, we can try to do it better. I have a concern, like below.

You know that the use of PCC can use polling mode and interrupt mode to 
communicate with platform.
I'm not sure if the obtaining of the polling mode and interrupt mode is 
an issue to prevent driver to be more universal.
But this driver can know if they support interrupt mode based on struct 
mbox_chan::struct mbox_controller::txdone_irq after requesting PCC channel.
Because I'm not sure it's a better way.
You know that drivers used interrupt mode need to fill the rx_callback 
function into mbx_client when register PCC channel.
And drivers used polling mode don't do it.
If we use this way, drivers that both support the two mode have to 
modify the rx_callback pointer after requesting PCC channel.
>

Re: [PATCH v2 2/2] soc: kunpeng_hccs: add the check for PCC subspace type

[lihuisong (C)]

Hi Sudeep,

在 2023/9/21 10:27, lihuisong (C) 写道:
>
> 在 2023/9/20 22:11, Sudeep Holla 写道:
>> On Wed, Sep 20, 2023 at 02:47:03PM +0800, Huisong Li wrote:
>>> Currently, HCCS driver directly uses Generic Communications Channel 
>>> Shared
>>> Memory Region which is used in type0/1/2 to communicate with platform,
>>> but actually doesn't support type3/4/5.
>>> So this patch adds the check for PCC subspace type.
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> ---
>>>   drivers/soc/hisilicon/kunpeng_hccs.c | 17 +++++++++++++++++
>>>   1 file changed, 17 insertions(+)
>>>
>>> diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c 
>>> b/drivers/soc/hisilicon/kunpeng_hccs.c
>>> index f3810d9d1caa..4ba3bfd45a01 100644
>>> --- a/drivers/soc/hisilicon/kunpeng_hccs.c
>>> +++ b/drivers/soc/hisilicon/kunpeng_hccs.c
>>> @@ -174,6 +174,19 @@ static int hccs_register_pcc_channel(struct 
>>> hccs_dev *hdev)
>>>       return rc;
>>>   }
>>>   +static int hccs_check_pcc_info(struct hccs_dev *hdev)
>>> +{
>>> +    struct pcc_mbox_chan *pcc_chan = hdev->cl_info.pcc_chan;
>>> +
>>> +    if (pcc_chan->type >= ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE) {
>>> +        dev_err(hdev->dev, "unsupport for subspace type%u.\n",
>>> +            pcc_chan->type);
>>> +        return -EOPNOTSUPP;
>>> +    }
>>   Is this the only use of the PCC type information you have or do you 
>> plan to
>> use it for something other than the validation.
> Yeah, it is just validation now. we want to plan this driver can 
> support more types.
>>
>> Just for sake of argument, I can say all users of PCC must then do the
>> similar validation. I don't know where to draw the line here.
>
> If export PCC type, it is good for the user of PCC to be more 
> universal and more compatible.
>
>>
>> Ideally I would expect the driver to make this transparent and give 
>> error
>> during transmit if not supported.
> I understand you.
> I just check this type only once during the initializing phase.
> Otherwise, every once need to verify it when send PCC command.
>>
>> The driver must be able to work with different PCC type to support 
>> variety
>> of platforms TBH. What is the issue exactly here ? Is this to prevent 
>> the
> Agree more with you.
> IMO, the user of PCC has the ability to support variety of platforms 
> if they can get PCC type.
> In this case, to prevent type 4 is necessary if driver cannot act as a 
> slave.
> on the other hand, If one driver acts as a slave, platform must supply 
> slave subspace for them.
>> use of Type 4 ? I think we must do something better but I don't know 
>> what
>> that is yet.
>
> Yes, we can try to do it better. I have a concern, like below.
>
> You know that the use of PCC can use polling mode and interrupt mode 
> to communicate with platform.
> I'm not sure if the obtaining of the polling mode and interrupt mode 
> is an issue to prevent driver to be more universal.
> But this driver can know if they support interrupt mode based on 
> struct mbox_chan::struct mbox_controller::txdone_irq after requesting 
> PCC channel.
> Because I'm not sure it's a better way.
> You know that drivers used interrupt mode need to fill the rx_callback 
> function into mbx_client when register PCC channel.
> And drivers used polling mode don't do it.
> If we use this way, drivers that both support the two mode have to 
> modify the rx_callback pointer after requesting PCC channel.
What do you feeling about the comments montioned above?
Maybe we still need to export the information whether the platform 
support interrupt before requesting PCC channel.
Looking forward your reply.
>>
> .

Re: [PATCH v2 2/2] soc: kunpeng_hccs: add the check for PCC subspace type

[Sudeep Holla]

On Thu, Sep 21, 2023 at 10:27:36AM +0800, lihuisong (C) wrote:
> 
> 在 2023/9/20 22:11, Sudeep Holla 写道:
> > On Wed, Sep 20, 2023 at 02:47:03PM +0800, Huisong Li wrote:
> > > Currently, HCCS driver directly uses Generic Communications Channel Shared
> > > Memory Region which is used in type0/1/2 to communicate with platform,
> > > but actually doesn't support type3/4/5.
> > > So this patch adds the check for PCC subspace type.
> > > 
> > > Signed-off-by: Huisong Li <lihuisong@huawei.com>
> > > ---
> > >   drivers/soc/hisilicon/kunpeng_hccs.c | 17 +++++++++++++++++
> > >   1 file changed, 17 insertions(+)
> > > 
> > > diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c
> > > index f3810d9d1caa..4ba3bfd45a01 100644
> > > --- a/drivers/soc/hisilicon/kunpeng_hccs.c
> > > +++ b/drivers/soc/hisilicon/kunpeng_hccs.c
> > > @@ -174,6 +174,19 @@ static int hccs_register_pcc_channel(struct hccs_dev *hdev)
> > >   	return rc;
> > >   }
> > > +static int hccs_check_pcc_info(struct hccs_dev *hdev)
> > > +{
> > > +	struct pcc_mbox_chan *pcc_chan = hdev->cl_info.pcc_chan;
> > > +
> > > +	if (pcc_chan->type >= ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE) {
> > > +		dev_err(hdev->dev, "unsupport for subspace type%u.\n",
> > > +			pcc_chan->type);
> > > +		return -EOPNOTSUPP;
> > > +	}
> > Is this the only use of the PCC type information you have or do you plan to
> > use it for something other than the validation.
> Yeah, it is just validation now. we want to plan this driver can support
> more types.

OK

> > 
> > Just for sake of argument, I can say all users of PCC must then do the
> > similar validation. I don't know where to draw the line here.
> 
> If export PCC type, it is good for the user of PCC to be more universal and
> more compatible.
>

I don't think it is a good idea to just export raw ACPI PCC type to the
client drivers. What if the driver in the future needs to work on a DT
platform as well and need to work with non-PCC mailbox channel ?

Also pushing the client PCC drivers to have more ACPI knowledge to understand
what each type means is also something I prefer to avoid. The real information
you want is whether this is an initiator(previously known master) channel or
responder(previously known as slave) channel. It boils down to unidirectional
vs bidirectional and what direction in bidirectional channels.

It would be good if mbox framework can be taught that, or atleast make an
attempt to see what people think about it. Just patching it the way you are
proposing is just going to hide real issue here.

> > 
> > Ideally I would expect the driver to make this transparent and give error
> > during transmit if not supported.
> I understand you.
> I just check this type only once during the initializing phase.
> Otherwise, every once need to verify it when send PCC command.

Agreed, but refer above for my concern on the solution proposed.

> >
> > The driver must be able to work with different PCC type to support variety
> > of platforms TBH. What is the issue exactly here ? Is this to prevent the
> Agree more with you.
> IMO, the user of PCC has the ability to support variety of platforms if they
> can get PCC type.
> In this case, to prevent type 4 is necessary if driver cannot act as a
> slave.
> on the other hand, If one driver acts as a slave, platform must supply slave
> subspace for them.
> > use of Type 4 ? I think we must do something better but I don't know what
> > that is yet.
> 
> Yes, we can try to do it better. I have a concern, like below.
>

Thanks for agreeing.

> You know that the use of PCC can use polling mode and interrupt mode to
> communicate with platform.
> I'm not sure if the obtaining of the polling mode and interrupt mode is an
> issue to prevent driver to be more universal.

Again this must be addressed at mailbox API level.

> But this driver can know if they support interrupt mode based on struct
> mbox_chan::struct mbox_controller::txdone_irq after requesting PCC channel.

Agreed, I was about to ask the same.

> Because I'm not sure it's a better way.

Again agreed.

> You know that drivers used interrupt mode need to fill the rx_callback
> function into mbx_client when register PCC channel.
> And drivers used polling mode don't do it.
> If we use this way, drivers that both support the two mode have to modify
> the rx_callback pointer after requesting PCC channel.

I am not sure if I followed all the details here. May be a rough hack helps
to understand what you are proposing ? Ofcourse not for merging, just to
understand the issue better.

--
Regards,
Sudeep

Re: [PATCH v2 2/2] soc: kunpeng_hccs: add the check for PCC subspace type

[lihuisong (C)]

在 2023/9/29 19:27, Sudeep Holla 写道:
> On Thu, Sep 21, 2023 at 10:27:36AM +0800, lihuisong (C) wrote:
>> 在 2023/9/20 22:11, Sudeep Holla 写道:
>>> On Wed, Sep 20, 2023 at 02:47:03PM +0800, Huisong Li wrote:
>>>> Currently, HCCS driver directly uses Generic Communications Channel Shared
>>>> Memory Region which is used in type0/1/2 to communicate with platform,
>>>> but actually doesn't support type3/4/5.
>>>> So this patch adds the check for PCC subspace type.
>>>>
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>> ---
>>>>    drivers/soc/hisilicon/kunpeng_hccs.c | 17 +++++++++++++++++
>>>>    1 file changed, 17 insertions(+)
>>>>
>>>> diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c
>>>> index f3810d9d1caa..4ba3bfd45a01 100644
>>>> --- a/drivers/soc/hisilicon/kunpeng_hccs.c
>>>> +++ b/drivers/soc/hisilicon/kunpeng_hccs.c
>>>> @@ -174,6 +174,19 @@ static int hccs_register_pcc_channel(struct hccs_dev *hdev)
>>>>    	return rc;
>>>>    }
>>>> +static int hccs_check_pcc_info(struct hccs_dev *hdev)
>>>> +{
>>>> +	struct pcc_mbox_chan *pcc_chan = hdev->cl_info.pcc_chan;
>>>> +
>>>> +	if (pcc_chan->type >= ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE) {
>>>> +		dev_err(hdev->dev, "unsupport for subspace type%u.\n",
>>>> +			pcc_chan->type);
>>>> +		return -EOPNOTSUPP;
>>>> +	}
>>> Is this the only use of the PCC type information you have or do you plan to
>>> use it for something other than the validation.
>> Yeah, it is just validation now. we want to plan this driver can support
>> more types.
> OK
>
>>> Just for sake of argument, I can say all users of PCC must then do the
>>> similar validation. I don't know where to draw the line here.
>> If export PCC type, it is good for the user of PCC to be more universal and
>> more compatible.
>>
> I don't think it is a good idea to just export raw ACPI PCC type to the
I understand you.
But the format and size of type0-2, type3/4 and type5 are different.
And all PCC client drivers need to know which PCC type they used and how 
to fill
the shared memory region header.
Please notice that the type used by the client driver is fixed and 
agreed with the platform in advance.
> client drivers. What if the driver in the future needs to work on a DT
> platform as well and need to work with non-PCC mailbox channel ?
It's also an alternative path.
>
> Also pushing the client PCC drivers to have more ACPI knowledge to understand
> what each type means is also something I prefer to avoid. The real information
Yeah, it is great that the PCC client drivers don't need to know which 
PCC type is used.
But it is inevitable according to above mentioned.
> you want is whether this is an initiator(previously known master) channel or
> responder(previously known as slave) channel. It boils down to unidirectional
> vs bidirectional and what direction in bidirectional channels.
not just for this.
>
> It would be good if mbox framework can be taught that, or atleast make an
> attempt to see what people think about it. Just patching it the way you are
> proposing is just going to hide real issue here.
>
>>> Ideally I would expect the driver to make this transparent and give error
>>> during transmit if not supported.
>> I understand you.
>> I just check this type only once during the initializing phase.
>> Otherwise, every once need to verify it when send PCC command.
> Agreed, but refer above for my concern on the solution proposed.
>
>>> The driver must be able to work with different PCC type to support variety
>>> of platforms TBH. What is the issue exactly here ? Is this to prevent the
>> Agree more with you.
>> IMO, the user of PCC has the ability to support variety of platforms if they
>> can get PCC type.
>> In this case, to prevent type 4 is necessary if driver cannot act as a
>> slave.
>> on the other hand, If one driver acts as a slave, platform must supply slave
>> subspace for them.
>>> use of Type 4 ? I think we must do something better but I don't know what
>>> that is yet.
>> Yes, we can try to do it better. I have a concern, like below.
>>
> Thanks for agreeing.
>
>> You know that the use of PCC can use polling mode and interrupt mode to
>> communicate with platform.
>> I'm not sure if the obtaining of the polling mode and interrupt mode is an
>> issue to prevent driver to be more universal.
> Again this must be addressed at mailbox API level.
Maybe it is ok.
The harder thing to deal with is that the client driver needs to decide 
if set rx_callback for the polling or
interrupt mode before requesting PCC channel.
because driver doesn't know whether platform supports interrupt.
If we fully put PCC type and interrupt mode we discussed to mailbox API 
or PCC driver, there are possibly
many works should be done. I have no good idea to do it yet.
>
>> But this driver can know if they support interrupt mode based on struct
>> mbox_chan::struct mbox_controller::txdone_irq after requesting PCC channel.
> Agreed, I was about to ask the same.
>
>> Because I'm not sure it's a better way.
> Again agreed.
>
>> You know that drivers used interrupt mode need to fill the rx_callback
>> function into mbx_client when register PCC channel.
>> And drivers used polling mode don't do it.
>> If we use this way, drivers that both support the two mode have to modify
>> the rx_callback pointer after requesting PCC channel.
> I am not sure if I followed all the details here. May be a rough hack helps
> to understand what you are proposing ? Ofcourse not for merging, just to
> understand the issue better.
please see the above reply😁
>
> --
> Regards,
> Sudeep
>
> .