在 2023/5/18 16:38, Sudeep Holla 写道:
On Thu, May 18, 2023 at 04:24:36PM +0800, lihuisong (C) wrote:
在 2023/5/17 21:16, Sudeep Holla 写道:
On Wed, May 17, 2023 at 07:35:25PM +0800, lihuisong (C) wrote:
在 2023/5/17 17:30, Sudeep Holla 写道:
On Wed, May 17, 2023 at 03:16:12PM +0800, lihuisong (C) wrote:
[...]
No. I want to use this flag to make compability between different platforms.
This driver only use PCC OpRegion to access to the channel if platform
support use PCC OpRegion.
What do you mean by that ? It is not correct. If there is a PCC Opregion,
then you need to make it work with drivers/acpi/acpi_pcc.c
You need to have all the other details in the firmware(ASL). By looking
at the driver, it has no connection to PCC Opregion IMO unless I am missing
something.
Driver just needs to call these APIs, such as acpi_evaluate_integer(), if
want to use PCC OpRegion.
OK, please provide examples. I am definitely lost as it doesn't match with
my understanding of how PCC Opregions are/can be used.
I know that. I have tested PCC OpRegion before.
Cool, examples please.
You've completely misunderstood what I said.😅
Hmm, may be but I need examples.
As you said below, the driver works just for PCC not PCC Opregion for now.
not sure if we need to discuss how PCC Opregion is used here.
Good let us drop the idea of using PCC Opregion with this driver for now.
I mean that this driver plans to support both PCC and PCC OpRegion.
For example,
Platform A: this driver use PCC (as the current implementation)
Good, then just keep what it needs in the implementation nothing more
until you add support for something you have described below(not that
I agree, just want you to make progress here based on what is actually
required today)
Agreed.
Platform B: this driver use PCC OpRegion (Currently, this patch does not
implement it, but it may be available in the future.)
Then let us discuss that in the future, don't add unnecessary complexity
for some future use case today. You can always add it when you introduce
that feature or support in the future.
Yes. We just need to focus on the current.
If there are any usage problems with PCC OpRegion in the future, we can
discuss that later.
Agreed.
My original full scheme is as follows:
-->
dev_flags = get_device_flags(); // to know if use PCC OpRegion
if (USE_PCC_OPREGION_B in dev_flags is 0) {
chan_id = get_pcc_chan_id();
init_mbox_client();
pcc_mbox_request_channel(cl, chan_id)
} else {
/* we need to return unsupport now because of no this feature in this
driver. */
do_nothing();
}
void get_some_info(...) {
if (USE_PCC_OPREGION_B in dev_flags is 0)
pcc_cmd_send(); // use PCC to communicate with Platform
else
acpi_evaluate_object(); // will be used in future.
}
As described in the pseudocode above,
it is necessary to put "dev_flags" in this current driver first in case of
the version driver runs on the platform which just use PCC Opregion.
No, you can't randomly define dev_flags just to assist your driver
implementation. If you need it, you need to get the spec updated. We
will not add anything unless that happens.
Note that I don't agree with the flags at all but if you convince and get
them added to spec, I won't object.
Ok，let us drop it.
Note:
This driver selects only one of them (PCC and PCC OpRegion) to communicate
with firmware on one platform.
Let us keep it simple(KISS). The driver works just for PCC not PCC Opregion
for now.
ok.
Good
We use one bit in device-flags to know which one this driver will use.
NACK again just to re-iterate my point if you have not yet accepted that
fact.
Above is our plan. Do you still think we shouldn't add this device-flags?
please let me know.
Correct, no device flags as I see no use for it with your PCC only use case
for now, right ?
Yes, it can still work well.
As for PCC Opregion way on other platform, I think of other way.
I'm not sure if you can understand what I mean by saing that.
If you're not confused about this now, can you reply to my last email
again?😁
The example you had IIRC is use of System Memory Opregion to demonstrate
some _DSM. That has nothing to do with PCC Opregion.
Yes, it doesn't matter.
I just want to have a way to get device-flags which contains many bits(every
bits can be used to as one feature for expanding), rigtht?
Get it through the spec, we don't allow random additions for some
implementations like this.
Get it.
Commit 77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for
the PCC Type 3 subtype") has the example in the commit message. IIRC,
Your example is very useful to the user.
you have even fixed couple of bugs in that driver. That is the reason
why I don't understand how you think this driver and that can or must
Understand you, Sudeep.
At that time, I tested it by a simple demo driver on the platform supported
type3.
OK
This driver will support multiple platforms.
On some platforms, we can only use PCC with polling way.
And we will add PCC Opregion way for others platforms.
Again when you do please post the patch with the ASL snippet as I am
very much interested in understanding how you would make that work.
All right.
What's more, every platform just use one of them(PCC and PCC Opregion).
OK
work together. At least I fail to see how ATM(examples please, by that
I mean ASL snippet for PCC vs PCC Opregion usage to work with this driver)
ok!
For PCC, ASL snippet is little.
I will add ASL snippet when this driver addes PCC Opregion way.
Sounds like a plan to make progress at-least for now.
