westeri / meta-acpi

Yocto BSP layer for ACPI enabled boards
MIT License
13 stars 11 forks source link

Eds 5.0.0 #17

Closed htot closed 5 years ago

htot commented 5 years ago

Don't pull this right now. This is just to notify you of the patches I have left in my queue. Comments welcome as always.

htot commented 5 years ago

I need SoB tag. Otherwise (taking comment into consideration) looks fine to me!

@staroselskii Can I add your signed-off-by?

andy-shev commented 5 years ago

I can try to make a sensible commit message from Jarkko's e-mail:

I attached a hack to buildroot enabling the V_SHILD_SW domain. It is now put inside the MUX_I2C definition as a proof of concept as I didn't get yet a better idea. I think it should be enabled by combination #if MUX_THIS || MUX_THAT || MUX_SOMETHING. E.g. MUX_SPI itself shouldn't enable it if user uses only analogue inputs and ADC chip U3 doesn't need V_SHIELD_SW.

Otherwise, how does this patch look to you? Jarkko calls it a hack, but we don't have better.

I agree on Jarkko's proposal to make it under corresponding conditionals. Maybe we can ask Jarkko to clean up and submit it?

htot commented 5 years ago

Rethinking: I should drop the 2nd patch, because all it does is create a eds specialized version of acpi-table-load and creates a dependency on libgpiod. The 4th patch can also go, because it undoes the specialization.

So it all depends on if we accept the 3rd patch. If Jarkko would resubmit he can also put his tag. But I'm not sure it should be conditional. vshield-en is TRI_STATE_ALL right? Why would we want to have that in an undefined state after reboot?

andy-shev commented 5 years ago

But I'm not sure it should be conditional. vshield-en is TRI_STATE_ALL right? Why would we want to have that in an undefined state after reboot?

Good point. So, we may define it to 0 for cases when MUXes are not enabled. So, user will do manually whatever they wants.

htot commented 5 years ago

Is @jhnikula active on gh?

jhnikula commented 5 years ago

On Wed, Sep 04, 2019 at 07:50:31PM +0000, Ferry Toth wrote:

Is @jhnikula active on gh?

Semi-active I'd say :-)

staroselskii commented 5 years ago

I need SoB tag. Otherwise (taking comment into consideration) looks fine to me!

@staroselskii Can I add your signed-off-by?

Sure, but I don't believe I can force push in your branch. I'm fine with you modifying the commit and adding Signed-Off by.

andy-shev commented 5 years ago

@htot, There is a potential problem with the patch that tries to set TRI_STATE_ALL. Whenever we use it as GPIO hog the user will not be able to do anything with it in the shell. Only unloading ACPI table will help. So, I think we may not use ACPI to toggle TRI_STATE_ALL to any state, unfortunately. So, please remove that patch for now from the bundle. We need to discuss it separately.

UPDATE Promising idea is to enable it via I²C OperationRegion: https://patchwork.kernel.org/patch/4209471/. Anybody to try?

htot commented 5 years ago

@andy-shev I can confirm this behavior. The arduino.asli: enable V_SHILD_SW domain patch enables TRI_STATE_ALL on table load, but does not allow run time toggling. In the case I tested that worked well, but that may have been too simple.

The edison: acpi-tables: set TRI_STATE_ALL before loading tables patch also has a problem. It toggles too quickly after loading the table. I need to put a delay in there.

htot commented 5 years ago

I force pushed dropping enable V_SHILD_SW domain patches and fixing the first 2.

htot commented 5 years ago

One more fix.

htot commented 5 years ago

Commented PoC patch lives now here: arduino.asli: enable V_SHILD_SW domain

andy-shev commented 5 years ago

Since I have applied ASL file and only one patch left, which I think is not the best way we may do, I close this PR for now. I don't like the idea of applying hacks in a form of some sleep() calls.

htot commented 5 years ago

Thanks @andy-shev