Re: [PATCH v3 2/3] acpi: dptf_power: Add DPTF power participant

["Rafael J. Wysocki" <rafael@kernel.org>]

On Thu, Jun 23, 2016 at 11:42 PM, Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
> This driver adds support for Dynamic Platform and Thermal Framework
> (DPTF) Platform Power Participant device support.
> This participant is responsible for exposing platform telemetry such as
> platform Power, battery Information such as state of Charge, estimated
> maximum sustainable power (PMax), SMART battery spec information.
>
> This driver is implemented as a platform driver for INT3407 and presented
> as power_supply device. Since this has common features with the ACPI
> battery, existing interface provide by battery_common driver are reused
> to present as a battery power supply device.
>
> When both CONFIG_ACPI_BATTERY and CONFIG_DPTF_POWER are defined and
> platform has support for INT3407, then dptf power registration is
> delayed for 100ms. In 100 ms, if there is no ACPI battery is registered
> then dptf power will be registered. Since both can be modules and
> battery driver loads in async thread, there can be race even if we
> specify loading order for initialization.

First, does the waiting actually help, though?

Second, to my eyes, even if acpi_battery load first, the dptf_power
thing will still bind to the same device, but via the platform bus
instead of the ACPI bus type.  I don't see anything preventing that
from happening in the patch, but maybe I missed something?

Also please put the part of the changelog below into a comment in
dptf_power.c (if you don't want to add a separate doc for that).

> There are two types of objects in INT3407:
> - Same as ACPI Battery objects: _BST and _BIX. These are exposed as
> standard power supply attributes.
> - Specific to INT3407, which are related to platform power
> There are some objects, for which it doesn't make sense to enhance
> power_supply class and add attributes there. They are represented as
> sysfs attribute under acpi device. Here the bid for INT3407 is TPWR
> in the following example.
> /sys/class/power_supply/TPWR
> ├── alarm
> ├── capacity
> ├── capacity_level
> ├── cycle_count
> ├── device -> ../../../INT3407:00
> │   ├── dptf_power
> │   │   ├── adapter_rating
> │   │   ├── battery_steady_power
> │   │   ├── charger_type
> │   │   ├── max_platform_power
> │   │   ├── platform_power_source
> │   │   ├── power_sampling_period
>
> For example representing AC/adapter properties as a power_supply
> properties will not make sense for a battery device. In some case
> when there is an existing property, the meaning is different.
> For example charger_type has a equivalent power_supply property,
> which has different symantics than INT3407.
>
> ACPI methods description used in this driver:
> PSOC: Platform Battery State Of Charge as a percentage.
> PMAX: Maximum platform power that can be supported by the battery in mW.
> PSRC: System charge source,
>         0x00 = DC
>         0x01 = AC
>         0x02 = USB
>         0x03 = Wireless Charger
> ARTG: Adapter rating in mW (Maximum Adapter power) Must be 0 if no AC
>         Adaptor is plugged in.
> CTYP: Charger Type,
>         Traditional : 0x01
>         Hybrid: 0x02
>         NVDC: 0x03
> PBSS: Returns max sustained power for battery in milliWatts.
> DPSP: Sets the polling interval in 10ths of seconds. A value of 0 tells
>         the driver to use event notification for PMAX and PBSS
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html