a problem about the EC driver

a problem about the EC driver

[Zhao Yakui]

Hi, Alexey
    I know that you are very familiar with the EC driver and you submit
a lot patches for EC. Now I have a problem about the EC and want to get
help from you.
   a. In the March of 2007 the patch of
33c7a0738c5f753a7d94fd3b2ec7d84e79a141a8 was submitted by you. In this
patch the Fake ECDT workaround is no longer used, which means that OSPM
won't parse EC in DSDT when there is no ECDT table. 
   b. In the August of 2007 the patch of
cd8c93a4e04dce8f00d1ef3a476aac8bd65ae40b was submitted. In this patch
OSPM will parse EC in DSDT when there is no ECDT table. It seems that
many laptops without ECDT table will be affected. Is there a potential
problem for the laptops without ECDT table? 
   c. In the November of 2007 the patch of
5870a8cd23181703cc76f88f630372f8602c7648 was submitted. In this patch
_INI check is added for EC device in DSDT table, which means that in the
function of acpi_ec_ecdt_probe OSPM won't install EC handler when no
_INI  method is found under the scope of EC in DSDT. It seems that a
regression is caused by this patch. Please refer to the bug 9730. In the
bug of 9730 there is no _INI method for EC device in DSDT table and OSPM
won't install EC handler. But the _CRS method of EC device will set the
EC flag. So some devices can't access the Operation Region defined in EC
device in the course of Linux ACPI device scan and the error messages
are reported. 
    According to the description of this patch it seems that the _EC
device is related to the _INI method.  But according to ACPI spec
whether the operation region can be accessed is related to the _REG
method. 
   d. In the Jan of 2008 the patch of
c04209a7948b95e8c52084e8595e74e9428653d3 was submitted. In this patch
the function of boot_ec_enable is added , which means that OSPM will
install EC handler for boot_ec device before acpi_bus_scan. 
   
   I don't understand why the flowchart of boot EC is changed very
frequently. Will you please give me detailed info about the four
patches? Maybe it is helpful for us to understand the EC driver.
   

Thanks.

   
     

    

Re: a problem about the EC driver

[Alexey Starikovskiy]

Hi Zhao,

All patches have relevant bugzilla entries, so you could read them for full
understanding.
Summary:
acpi_ec_probe/boot_ec_enable are workarounds for same situation then
EC driver should be inited early, but ECDT was not provided. Half of this
workaround was enabled earlier with fake ECDT.

Having a kernel parameter to enable a workaround is not good, as it requires too
much knowledge from the users, while the problem seem to be not one or two notebooks,
but whole MSI and ASUS lines.

Spec only covers case of ECDT for early init of EC, so whole series of patches do not 
have anything to do with spec, that should be clear.

EC could be used in either _INI methods for some devices at device init stage, 
or in _STA methods, then devices are scanned and attached to drivers.

It was found, that in absence of ECDT, if some _INI requires presence of
EC op region, there will be empty _INI method in EC scope.
It was also found that many EC will fail to initialize before all _INI methods
are done. This is why the registration of EC op region is done only if _INI method
is present in EC scope.

For _STA methods during device scan it is completely safe to enable EC op region
before, just because this is only a priority of driver assignment. We could be
very smart and add several priority passes to scan, but enabling EC before scan
does exactly what we need.

>    I don't understand why the flowchart of boot EC is changed very
> frequently.
Why it is my problem, if don't understand something in the kernel?
> Will you please give me detailed info about the four
> patches? Maybe it is helpful for us to understand the EC driver.
Maybe it is not, who is "us" in this sentence?

Regards,
Alex.


Zhao Yakui wrote:
> Hi, Alexey
>     I know that you are very familiar with the EC driver and you submit
> a lot patches for EC. Now I have a problem about the EC and want to get
> help from you.
>    a. In the March of 2007 the patch of
> 33c7a0738c5f753a7d94fd3b2ec7d84e79a141a8 was submitted by you. In this
> patch the Fake ECDT workaround is no longer used, which means that OSPM
> won't parse EC in DSDT when there is no ECDT table. 
>    b. In the August of 2007 the patch of
> cd8c93a4e04dce8f00d1ef3a476aac8bd65ae40b was submitted. In this patch
> OSPM will parse EC in DSDT when there is no ECDT table. It seems that
> many laptops without ECDT table will be affected. Is there a potential
> problem for the laptops without ECDT table? 
>    c. In the November of 2007 the patch of
> 5870a8cd23181703cc76f88f630372f8602c7648 was submitted. In this patch
> _INI check is added for EC device in DSDT table, which means that in the
> function of acpi_ec_ecdt_probe OSPM won't install EC handler when no
> _INI  method is found under the scope of EC in DSDT. It seems that a
> regression is caused by this patch. Please refer to the bug 9730. In the
> bug of 9730 there is no _INI method for EC device in DSDT table and OSPM
> won't install EC handler. But the _CRS method of EC device will set the
> EC flag. So some devices can't access the Operation Region defined in EC
> device in the course of Linux ACPI device scan and the error messages
> are reported. 
>     According to the description of this patch it seems that the _EC
> device is related to the _INI method.  But according to ACPI spec
> whether the operation region can be accessed is related to the _REG
> method. 
>    d. In the Jan of 2008 the patch of
> c04209a7948b95e8c52084e8595e74e9428653d3 was submitted. In this patch
> the function of boot_ec_enable is added , which means that OSPM will
> install EC handler for boot_ec device before acpi_bus_scan. 
>    
>    I don't understand why the flowchart of boot EC is changed very
> frequently. Will you please give me detailed info about the four
> patches? Maybe it is helpful for us to understand the EC driver.
>    
> 
> Thanks.
> 
>    
>      
> 
>     
> 

Re: a problem about the EC driver

[Zhao Yakui]

On Mon, 2008-01-21 at 13:00 +0300, Alexey Starikovskiy wrote:
Thanks for the response.
> Hi Zhao,
> 
> All patches have relevant bugzilla entries, so you could read them for full
> understanding.
> Summary:
> acpi_ec_probe/boot_ec_enable are workarounds for same situation then
> EC driver should be inited early, but ECDT was not provided. Half of this
> workaround was enabled earlier with fake ECDT.
> 
> Having a kernel parameter to enable a workaround is not good, as it requires too
> much knowledge from the users, while the problem seem to be not one or two notebooks,
> but whole MSI and ASUS lines.
> 
> Spec only covers case of ECDT for early init of EC, so whole series of patches do not 
> have anything to do with spec, that should be clear.
The spec only emphasizes that OSPM should make EC operation region
accessed via EC described in ECDT table. It doesn't cover the EC defined
in DSDT. Maybe it is better to use the similar flowchart.
> 
> EC could be used in either _INI methods for some devices at device init stage, 
> or in _STA methods, then devices are scanned and attached to drivers.
Whether EC operation region can be accessed is only related to the _REG
method. After the _REG method for EC is called, it means that AML can
access the data fields in EC region. According to spec the _REG method
should be called before calling other methods including _INI. The spec
doesn't cover whether the _STA method is also included.   

> It was found, that in absence of ECDT, if some _INI requires presence of
> EC op region, there will be empty _INI method in EC scope.
> It was also found that many EC will fail to initialize before all _INI methods
> are done. This is why the registration of EC op region is done only if _INI method
> is present in EC scope.
If EC fails to initialize before all _INI methods, EC still can't be
initialized successfully even when there is _INI method under its scope.
So I think that it is unnecessary to not initialize EC even when there
is no _INI method under the scope of EC.
> 
> For _STA methods during device scan it is completely safe to enable EC op region
> before, just because this is only a priority of driver assignment. We could be
> very smart and add several priority passes to scan, but enabling EC before scan
> does exactly what we need.
OK. This can solve the problem. But maybe it is better to enable boot EC
after the ACPI full initialization.  
> 
> >    I don't understand why the flowchart of boot EC is changed very
> > frequently.
> Why it is my problem, if don't understand something in the kernel?
> > Will you please give me detailed info about the four
> > patches? Maybe it is helpful for us to understand the EC driver.
> Maybe it is not, who is "us" in this sentence?
> 
> Regards,
> Alex.
> 
> 
> Zhao Yakui wrote:
> > Hi, Alexey
> >     I know that you are very familiar with the EC driver and you submit
> > a lot patches for EC. Now I have a problem about the EC and want to get
> > help from you.
> >    a. In the March of 2007 the patch of
> > 33c7a0738c5f753a7d94fd3b2ec7d84e79a141a8 was submitted by you. In this
> > patch the Fake ECDT workaround is no longer used, which means that OSPM
> > won't parse EC in DSDT when there is no ECDT table. 
> >    b. In the August of 2007 the patch of
> > cd8c93a4e04dce8f00d1ef3a476aac8bd65ae40b was submitted. In this patch
> > OSPM will parse EC in DSDT when there is no ECDT table. It seems that
> > many laptops without ECDT table will be affected. Is there a potential
> > problem for the laptops without ECDT table? 
> >    c. In the November of 2007 the patch of
> > 5870a8cd23181703cc76f88f630372f8602c7648 was submitted. In this patch
> > _INI check is added for EC device in DSDT table, which means that in the
> > function of acpi_ec_ecdt_probe OSPM won't install EC handler when no
> > _INI  method is found under the scope of EC in DSDT. It seems that a
> > regression is caused by this patch. Please refer to the bug 9730. In the
> > bug of 9730 there is no _INI method for EC device in DSDT table and OSPM
> > won't install EC handler. But the _CRS method of EC device will set the
> > EC flag. So some devices can't access the Operation Region defined in EC
> > device in the course of Linux ACPI device scan and the error messages
> > are reported. 
> >     According to the description of this patch it seems that the _EC
> > device is related to the _INI method.  But according to ACPI spec
> > whether the operation region can be accessed is related to the _REG
> > method. 
> >    d. In the Jan of 2008 the patch of
> > c04209a7948b95e8c52084e8595e74e9428653d3 was submitted. In this patch
> > the function of boot_ec_enable is added , which means that OSPM will
> > install EC handler for boot_ec device before acpi_bus_scan. 
> >    
> >    I don't understand why the flowchart of boot EC is changed very
> > frequently. Will you please give me detailed info about the four
> > patches? Maybe it is helpful for us to understand the EC driver.
> >    
> > 
> > Thanks.
> > 
> >    
> >      
> > 
> >     
> > 
> 

Re: a problem about the EC driver

[Alexey Starikovskiy]

Zhao Yakui wrote:
> On Mon, 2008-01-21 at 13:00 +0300, Alexey Starikovskiy wrote:
> Thanks for the response.
>> Hi Zhao,
>>
>> All patches have relevant bugzilla entries, so you could read them for full
>> understanding.
>> Summary:
>> acpi_ec_probe/boot_ec_enable are workarounds for same situation then
>> EC driver should be inited early, but ECDT was not provided. Half of this
>> workaround was enabled earlier with fake ECDT.
>>
>> Having a kernel parameter to enable a workaround is not good, as it requires too
>> much knowledge from the users, while the problem seem to be not one or two notebooks,
>> but whole MSI and ASUS lines.
>>
>> Spec only covers case of ECDT for early init of EC, so whole series of patches do not 
>> have anything to do with spec, that should be clear.
> The spec only emphasizes that OSPM should make EC operation region
> accessed via EC described in ECDT table. It doesn't cover the EC defined
> in DSDT. Maybe it is better to use the similar flowchart.
What do you mean by "similar flowchart"? You are certainly welcome to write your own debug patches
and ask reporters of all mentioned bugs to test them, if you have something better in your mind. 
>> EC could be used in either _INI methods for some devices at device init stage, 
>> or in _STA methods, then devices are scanned and attached to drivers.
> Whether EC operation region can be accessed is only related to the _REG
> method. After the _REG method for EC is called, it means that AML can
> access the data fields in EC region. According to spec the _REG method
> should be called before calling other methods including _INI. The spec
> doesn't cover whether the _STA method is also included.   
I'm not talking about calling _INI or _STA method of EC itself, I'm talking
about _other_ devices, which fail initialization due to us following the spec.
> 
>> It was found, that in absence of ECDT, if some _INI requires presence of
>> EC op region, there will be empty _INI method in EC scope.
>> It was also found that many EC will fail to initialize before all _INI methods
>> are done. This is why the registration of EC op region is done only if _INI method
>> is present in EC scope.
> If EC fails to initialize before all _INI methods, EC still can't be
> initialized successfully even when there is _INI method under its scope.
Well, I have report, that it works. Do you have another report, or is it your thoughts?
> So I think that it is unnecessary to not initialize EC even when there
> is no _INI method under the scope of EC.
There are too many negatives in one sentence. Please do a patch.
>> For _STA methods during device scan it is completely safe to enable EC op region
>> before, just because this is only a priority of driver assignment. We could be
>> very smart and add several priority passes to scan, but enabling EC before scan
>> does exactly what we need.
> OK. This can solve the problem. But maybe it is better to enable boot EC
> after the ACPI full initialization.

IMHO, you still don't understand why it is all needed... If EC is enabled after full ACPI init,
all bug reporters will have broken ACPI subsystems, just because all of their ACPI drivers will not be 
found or functioning. 
  
Write a patch, test it. It is that easy.

Regards,
Alex.