hvdcp_max_current not in effect?

[tomty89]

In b413135, support for HVDCP charger has been added to the driver phy_msm_usb. Max current drawn from HVDCP charger has been set to 1800mA in the commit as well. I can confirmed that it has been applied to the kernel I am using:

u0_a210@F5122:/ $ cat /sys/module/phy_msm_usb/parameters/hvdcp_max_current
1800

However, when I check the following uevent file while I am charging my Xperia X, I see that the max current is not 1800mA but 1500mA:

u0_a210@F5122:/ $ cat /sys/class/power_supply/usb/uevent
POWER_SUPPLY_NAME=usb
POWER_SUPPLY_HEALTH=Good
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_ONLINE=1
POWER_SUPPLY_VOLTAGE_MAX=0
POWER_SUPPLY_CURRENT_MAX=1500000
POWER_SUPPLY_INPUT_CURRENT_MAX=0
POWER_SUPPLY_SCOPE=Device
POWER_SUPPLY_TYPE=USB_HVDCP
POWER_SUPPLY_VOLTAGE_NOW=9038620
POWER_SUPPLY_DP_DM=1
POWER_SUPPLY_USB_OTG=0
POWER_SUPPLY_SUB_TYPE=0
POWER_SUPPLY_CHARGER_TYPE=USB_HVDCP_CHARGER

Does that mean some necessary changes are missing for proper support for HVDCP?

Btw, when I check the history of the phy_msm_usb driver, a recent change (867d794) is that some hacks have been added for Suzu (which is Xperia X, the model I have here, IIRC). So what so special about Suzu that makes it need hacks?

[tomty89]

I can see that hvdcp_max_current is only used in an msm_otg_notify_charger called in otg_power_set_property_usb. (Note: compare this with where/how dcp_max_current is used)

The thing is, it appears to me that, otg_power_set_property_usb is a method that is only used when we write to corresponding sysfs file (/sys/class/power_supply/usb/type in this case), but not when the driver detects the actual psy->type. (In fact, I don't see it does that at all other than in otg_power_get_property_usb, which is probably used only for sysfs files as well.)

If it is true, it means that current_max will only be set to 1800mA if we explicitly write USB_HVDCP (yes, even when it is already the value) to type after we connect a phone to an HVDCP charger.

Worsestill, the test cannot be done before we list POWER_SUPPLY_PROP_TYPE (i.e. /sys/class/power_supply/usb/type) as writeable. According to the comment, it is supposed to be a means for us to override the charger type, yet it has never been made writeable.

What do you think? @kholk @humberos

[kholk]

Well, first of all the simplest question. The extensions for suzu on USB are interface tunings, to ensure max bandwidth can be reached and, more important things, usb host to work and maximum compatibility with dumb slow USB devices and controllers (respectively in host and client mode).

About the hvdcp amperage, the charger type sysfs is not writeable by design: it is only an informative sysfs and ONLY drivers are allowed to write to it because ONLY drivers can choose the paths to follow. For setting up type and max amperage, the smbcharger driver (in our case) and fuel gauge driver needs to send commands to the hardware to tell it to limit the input to that number and this is obviously done because heat constraints in a so much small environment (small board, small phone, small dissipation area).

In any case, let me make something clear to you. You may (or may not) know how HVDCP chargers do work: instead of the usual 5V output of normal chargers, those ones are variable. You have a 9V tension at charging startup and then (sometimes, I don't understand when since I haven't got those hvdcp chargers and couldn't analyze them) it goes down to 5V.

Now, remember the fundamental Ohm's law.... I=V/R.... And also remember that W=V*I (let's make it simple). First, without mentioning all the formulas, to not complicate things too much, what complicates things about carrying current is the wire gauge and length. It is mostly relative (but not only, and, again, I'm keeping it simple) to the amperage that you carry: let's take a fixed wire gauge of, for example, 24AWG. On that (pair of) cable you will have a certain current dispersion per every centimeter of length: this comes different when you apply a higher voltage (that's one of the reasons why you have 220/120VAC at mains!!) so it is much simpler to carry 20W as 9V, 2.2A, instead of 5V, 4A (and lets not even talk about designing a psu capable of delivering that amperage at 5V!!).

Now that the theory is done and you have very basic informations, how many watts are being carried to your phone at 9V, 1500mA? How many watts are being carried to your phone at 5V, 1500mA?

And how many watts are being carried to your phone at 5V, - >1800mA<-?

Do you understand what I'm trying to say? :)

[tomty89]

@kholk ugh, I might not know very well about how HVDCP works or the physics of it, but I am not sure how much it is relevant to what I was pointing out.

The problem here is that I see a commit that tried to introduce a module parameter (hvdcp_max_current) that should be used to set as a certain limit (current_max) upon the connection of the targeted type of device (HVDCP charger), but it failed to do so because it is implemented in wrong way programmatically. It has been partially done so the module param will only be used when the type sysfs file is written by user.

As for whether type should be writeable, I don't have a stand here, but it appears that the original writer does:

    case POWER_SUPPLY_PROP_TYPE:
        psy->type = val->intval;

        /*
         * If charger detection is done by the USB driver,
         * motg->chg_type is already assigned in the
         * charger detection work.
         *
         * There is a possibility of overriding the
         * actual charger type with power supply type
         * charger. For example USB PROPRIETARY charger
         * does not exist in power supply enum and it
         * gets overridden as DCP.
         */
        ...
        ...

If we don't think POWER_SUPPLY_PROP_TYPE should be writeable, this whole "case" for it here is pointless to exist. We only need to have a case for it in the get_property method.

In any case, it also shows why msm_otg_notify_charger(motg, hvdcp_max_current) should not (only) be called in the set_property method.

[kholk]

Well then I'm sorry, I have misunderstood the question.

You have a valid point in this and the hvdcp thing should be managed not only by QC's charger drivers, but also by the proprietary Qnovo one, which hasn't got a real kernel driver and I'm not sure it is implemented in userspace (ugh, yes, it's horrible).

Let me check a lil bit how things are here, hold on