Patch Set 2:

(1 comment)

Patch Set 2:

(1 comment)

Hmm, this makes me want to attack the power resource problem again...
With the devtree alias patches in, expressing power resource dependencies is even cleaner, e.g.,:

```
    chip drivers/generic/power_resource/
        register "on" = "{{GPIO_L(GPP_A1), 50},
                          {GPIO_L(GPIO_B2), 1},}"
        register "off" = "{{GPIO_L(GPP_B2), 1},
                           {GPIO(GPP_A1), 10},}"
        # or even the current interface in generic/2c with the stop_gpio, reset_gpio, etc.
        # but I kind of like this explicit sequencing you get here.
        device generic 0 alias i2c1_power_res on end
    end
    chip drivers/i2c/generic
        use i2c1_power_res as power_resource;
        device i2c 1f on end
    end
    chip drivers/spi/generic
        use i2c1_power_res as power_resource;
        device spi 0 on end
    end
```

Each "resource" in the 'on' and 'off' list (i.e., GPIOs) can emit enable/disable methods that use reference counting to keep track of when it's safe to actually assert/deassert the pins.

WDYT? If you don't want to tackle that right now, factoring out the power resource fields into a common `struct power_resource_config` or similar would be better than copy-pasting these fields around into different drivers.

We might want to eventually do something like you recommended i.e. having a separate power resource device especially if multiple devices have to share power resource. Last week you mentioned that this might probably be a use case for the camera device.

One thing that we will probably have to think about some more is -- if a driver wants to expose the GPIOs in _CRS in addition to generating the power resource, then it will require both on/off as well as reset/enable GPIOs to be provided by mainboard in the devicetree entry. It might be helpful to write these thoughts in a doc/bug so that we can capture all scenarios.

About adding a power_resource structure - I think that can be a quick way forward right now especially if Karthik is looking to unblock the mainboard CLs. However, if you plan to refactor and add a new power_resource driver, do you want to wait until then to decide what the structure should really look like?

Just a strawman for the moment 😊

There are other considerations too, like ensuring that _PR0/_PR3 get emitted too. I have some thoughts, I should collect them into a doc.

View Change

1 comment:

• File src/drivers/usb/acpi/chip.h:

  • Patch Set #2, Line 47:

    /* Disable reset and enable GPIO export in _CRS */
    	bool disable_gpio_export_in_crs;
    

    I think we should skip this. […]

    Thanks for the background, I had been wondering what the deal was with leaving the GPIOs out of _CRS.

To view, visit change 46713. To unsubscribe, or for help writing mail filters, visit settings.