| tmlind | Wizzup: so the MASTER_USBHOSTHS corresponds to ohci/echi, so usbhshost in omap4-l4.dtsi, seems that on low battery mdm6600 shuts down, and the usb phy is on the mdm6600 causing invalid access to the phy, or at least that's my guess | 06:31 |
|---|---|---|
| tmlind | seems like usbhs_omap_remove() or similar should get called to remove the related phys at that point | 06:34 |
| tmlind | freemangordon: to me it seems that usb gadget framework should add/remove a vbus regulator as needed telling the max current, then chargers could try to get it and check the max current after connecting.. somehow the regulator should be drivers/phy instance specific though.. just trying to think of ideas how it could be done | 06:38 |
| freemangordon | tmlind: and what about just adding second notifier that is dedicated to chargers? | 07:58 |
| freemangordon | hmm, wait. don;t we already have a regulator for vbus? | 07:59 |
| freemangordon | so, what about gadget asking phy for the regulator and setting the max current on it> | 08:00 |
| freemangordon | ? | 08:00 |
| freemangordon | phy can do as well, if needed | 08:01 |
| freemangordon | and ti is needed, in host mode | 08:01 |
| freemangordon | so, phy core can register some "usb-charger" regulator and this can be used by chargers to limit the current and gadgets or phy itself to set the current limit | 08:06 |
| freemangordon | do you like that? | 08:07 |
| freemangordon | I just wonder if it should be only one, global, regulator or per phy | 08:38 |
| freemangordon | ah, you said it should not e global | 08:53 |
| freemangordon | well, phy_create() can create a dummy regulator based on device name or some other string (phy label?) | 08:58 |
| tmlind | yeah that vbus regulator could be common to the phy framework maybe, then gadget would somehow set max value, then charger could query it.. | 10:16 |
| tmlind | or how about add phy_set_charge_current() and phy_get_charge_current() ? | 10:17 |
| freemangordon | we already have that | 10:17 |
| tmlind | oh we do? | 10:17 |
| freemangordon | usb_phy_get_charger_current | 10:17 |
| freemangordon | and usb_phy_set_charger_current( | 10:17 |
| freemangordon | I was about to add regulator in usb_phy struct | 10:18 |
| tmlind | ok but that's specific to the old drivers/usb/phy and won't work with drivers/phy? | 10:18 |
| freemangordon | is it? | 10:18 |
| tmlind | i think so, it's a different old framework for drivers/usb/phy | 10:18 |
| freemangordon | cpcap phy calls usb_add_phy_dev() | 10:19 |
| freemangordon | is this obsolete too? | 10:19 |
| tmlind | oh not sure | 10:19 |
| tmlind | it might be legacy needed for musb still | 10:19 |
| freemangordon | what I see is that usb_phy is used all over the place | 10:19 |
| tmlind | ok maybe it's still the way to go | 10:20 |
| freemangordon | so, what I think is: | 10:20 |
| tmlind | vbus_get_charge_current(phy) vbus_set_charge_current(phy) ? | 10:20 |
| freemangordon | add regulator in usb_phy and create it in usb_charger_init() | 10:20 |
| tmlind | so how do you map a phy instance to that particular vbus regulator? | 10:21 |
| freemangordon | usb_charger_init is called by usb_add_phy_dev and usb_add_phy_dev | 10:21 |
| tmlind | or vbus supply.. | 10:21 |
| freemangordon | vbus supply is another regulator | 10:21 |
| freemangordon | this one is $device_name-charger or something | 10:22 |
| freemangordon | and is a member of phy struct | 10:22 |
| freemangordon | so chargers can either use regulator framework to register notigier on it | 10:22 |
| freemangordon | or we can have usb_charger_notifier_register() function | 10:22 |
| tmlind | hmm so looks like there's common vbus-supply devicetree property | 10:23 |
| freemangordon | and in DTS we set the phy on the charger | 10:23 |
| freemangordon | there is | 10:23 |
| freemangordon | but it is not what we need | 10:23 |
| tmlind | ok | 10:23 |
| freemangordon | IIUC | 10:23 |
| freemangordon | maybe I am wrong | 10:23 |
| freemangordon | but, this supply is the one that exists in the device, no? | 10:24 |
| freemangordon | why we want some dummy regulator used to carry the min/max current | 10:24 |
| tmlind | Documentation/devicetree/bindings/connector/usb-connector.yaml | 10:24 |
| tmlind | documents vbus-supply | 10:24 |
| freemangordon | ok, but this is the internal supply, no? | 10:24 |
| * freemangordon reads the docs | 10:25 | |
| tmlind | compatible = "usb-b-connector" | 10:25 |
| freemangordon | hmm | 10:25 |
| tmlind | not sure what is handling it, but cpcap-charger could query it | 10:25 |
| freemangordon | I dont think anyone uses that, lemme grep | 10:25 |
| tmlind | cpcap-charger must not try to claim it on init though.. loading the phy should be optional, otherwise assume 500mA max current if regulator not available | 10:26 |
| freemangordon | sure | 10:26 |
| freemangordon | the same as in wm831x driver | 10:27 |
| tmlind | ok | 10:27 |
| freemangordon | so, if devm_usb_get_phy_by_phandle() fails, we just assume 500mA | 10:27 |
| tmlind | looks like we may just need to add the usb-b-connector node as a child of the usb or the phy | 10:27 |
| Wizzup | tmlind: I see, it clearly was proceeded by an empty battery warning, but it looks like what triggered it was the modem dropping off? | 10:28 |
| freemangordon | who is going to set that? | 10:28 |
| Wizzup | oh, you wrote that too. | 10:28 |
| freemangordon | tmlind: hmm, this is usb-conn-gpio.c | 10:29 |
| freemangordon | so this vbus-supply is specific to this driver, no? | 10:30 |
| freemangordon | tmlind: BTW, did you look at https://www.mail-archive.com/linux-usb@vger.kernel.org/msg93172.html | 10:33 |
| freemangordon | we still have that and I see no signs of it being obsolete | 10:33 |
| freemangordon | but what do I know :) | 10:33 |
| freemangordon | still, with soma small fixes this can be made to use a dummy regulator | 10:34 |
| freemangordon | and everything will start working | 10:34 |
| freemangordon | the real issue right now is that code currently uses the same atomic notifier as what gadget network uses to report VBUS state | 10:35 |
| freemangordon | s/gadget network/gadget framework | 10:35 |
| freemangordon | so my idea is to fix that by doing regulator_get() (that creates dummy regulator) in usb_charger_init() | 10:38 |
| freemangordon | and then usb_charger_notifier_register() registering notifier on that regulator to be used by charger | 10:39 |
| freemangordon | or, simply adding second notifier to the phy | 10:40 |
| tmlind | seems like there are multiple drivers setting up a usb connector: git grep -C20 usb-b-connector arch/arm*/boot/dts | 10:40 |
| freemangordon | sure | 10:40 |
| freemangordon | but this is gpio based charger detection | 10:40 |
| freemangordon | IIUC | 10:40 |
| tmlind | well adding gpio support for connect and vbus to cpcap would be trivial, it has all kind of gpios anyways | 10:43 |
| tmlind | we're just not using them right now | 10:43 |
| * freemangordon is looking in usb-conn-gpio.c | 10:43 | |
| tmlind | seems like to use that we should add drivers/gpio/gpio-cpcap.c, the gpio code for cpcap is there in the v3.0.8 android kernel | 10:43 |
| freemangordon | tmlind: umm, I don;t get that, like, how we are goping to detect ACA by using gpios? | 10:44 |
| freemangordon | or even a simple charger? | 10:44 |
| freemangordon | for ACA we have to do voltage measurement | 10:44 |
| freemangordon | android kernel uses switch driver for charger detection == extcon | 10:45 |
| freemangordon | maybe I don;t understand the idea | 10:45 |
| tmlind | hmm what's aca? | 10:47 |
| tmlind | some regulator somewhere to query would be still needed it seems | 10:48 |
| tmlind | bbl, family | 10:48 |
| freemangordon | tmlind: "Accessory Charger Adaptor" | 10:50 |
| freemangordon | tmlind: ok, I will create some small patch to have something to discuss :) | 10:51 |
| freemangordon | tmlind: this https://pastebin.com/WfVwFwYL | 11:42 |
| sicelo | freemangordon: regarding my kernel size issue ... i build a kernel with plain omap2plus_defconfig. kernel size was exactly 5MB (i.e. 500kB less than before). the irq problem still happens. tbh, i doubt it's related to kernel size. plus, the only one with missing irq is musb. other drivers don't have the issue | 15:32 |
| freemangordon | hmm | 15:32 |
| freemangordon | I still think it is DTB related, but yeah, maybe it is not the size that bugs it | 15:33 |
| freemangordon | uvos: tmlind: https://pastebin.com/S7wusWBf | 15:38 |
| Wizzup | sicelo: is it always attached? | 15:39 |
| Wizzup | (dtb) | 15:39 |
| Wizzup | maybe you need to adjust the u-boot load addrs | 15:40 |
| freemangordon | uvos: tmlind: this is the patch https://pastebin.com/xHpWzDth | 15:40 |
| sicelo | Wizzup: yes i'm using attached dtb. Not sure how to make u-boot use a standalone dtb yet on n900, although that'd be a nice thing to have | 15:43 |
| freemangordon | oh, and as a side effect now we have stable usb ethernet even when the battery is full :D | 15:49 |
| freemangordon | Wizzup: ^^^ | 15:49 |
| Wizzup | freemangordon: that is so lovely, I'm so happy about that | 15:49 |
| freemangordon | on the cons side - dumb chargers does not work :p | 15:49 |
| Wizzup | really | 15:49 |
| Wizzup | heh | 15:49 |
| sicelo | dumb charger being? | 15:50 |
| freemangordon | wall charger :) | 15:50 |
| * freemangordon hides | 15:50 | |
| sicelo | hehe | 15:50 |
| sicelo | does not work - doesn't charge the device now? | 15:50 |
| freemangordon | sure | 15:50 |
| freemangordon | we need one more driver | 15:51 |
| freemangordon | cpcap-extcon | 15:51 |
| freemangordon | that detects wall chargers | 15:51 |
| freemangordon | but, I want tmlind to confirm what I have done so far is correct | 15:51 |
| freemangordon | as writing extcon driver from scratch is not a fun and if I am doing something wrong will be in vain | 15:52 |
| freemangordon | which I want to avoid | 15:52 |
| uvos | sicelo: so if you compile dtb and then decompile it again with dtc is the irq there? | 15:53 |
| freemangordon | yes | 15:53 |
| uvos | ok | 15:53 |
| uvos | what about after attaching it, pull it back out of the uboot image | 15:54 |
| uvos | maybe it gets messed up there | 15:54 |
| sicelo | maybe ... but the question would then be - why it it's fine in 5.18.x, and not fine in 5.19.1 :-) | 15:55 |
| sicelo | but yes, let me see if can find how to 'detach' it from uImage | 15:55 |
| uvos | sicelo: no idea just thinking of steps, clearly somehow it gets ruined on the way to ram | 15:56 |
| uvos | so some step in the process must be doing it | 15:56 |
| freemangordon | uvos: attaching it to uImage is simply doing cat, so can't be this | 15:56 |
| uvos | freemangordon: ok | 15:57 |
| uvos | maybe sicelo has a broken flash cell in a inauspicious place | 15:57 |
| freemangordon | I'd rather bet on u-boot allocating no enough RAM, or not reading the full uImage | 15:57 |
| freemangordon | or not reading the full attached DTB | 15:57 |
| freemangordon | Pali: ^^^ | 15:57 |
| freemangordon | any idea? | 15:57 |
| uvos | sure maybe | 15:57 |
| Pali | u-boot should be able to load separate zImage and separate DTB without any issue | 15:59 |
| uvos | not really a reason to ignore it breaking uimage | 15:59 |
| freemangordon | Pali: yeah, the point is that it seems it somehow breaks the attached DTB | 16:00 |
| freemangordon | either u-boot or kernel | 16:00 |
| Pali | You can enable CONFIG_DEBUG_LL and kernel starts printing debug info prior reading attached dtb to serial console | 16:01 |
| Pali | serial console is available also in u-boot | 16:02 |
| Pali | IIRC it prints attached dtb address and size | 16:02 |
| Pali | so you can verify if the issue is in kernel code | 16:02 |
| freemangordon | sicelo: ^^^ | 16:02 |
| freemangordon | that should help | 16:02 |
| freemangordon | Pali: is ther some size limit on DTB? | 16:02 |
| sicelo | the CONFIG_DEBUG_LL should be enabled in u-boot? or kernel? | 16:03 |
| freemangordon | u-boot | 16:03 |
| Pali | In past there was a DTB limit. And n900 dtb file triggered that limit, so it was increased | 16:03 |
| Pali | in kernel | 16:03 |
| freemangordon | maybe it triggered it again? | 16:03 |
| Pali | it is part of zImage wrapper | 16:03 |
| freemangordon | do you remember file was that limit in? | 16:04 |
| Pali | it was in zImage wrapper, so some in arch/arm/boot/ | 16:05 |
| freemangordon | ok, thanks | 16:05 |
| Pali | Also enable CONFIG_DEBUG_OMAP3UART3 | 16:05 |
| sicelo | unfortunately i don't have access to good/reliable serial. but let me first try with u-boot that has serial enabled, although i recall this just loses connectivity as soon as kernel starts | 16:07 |
| Pali | right, this would not work. kernel prints only to UART, not to usb | 16:08 |
| Pali | but you can test it in qemu | 16:08 |
| Pali | qemu has working UART nad kernel CONFIG_DEBUG_LL can print to it | 16:09 |
| sicelo | standard `qemu-system-arm`, or a special/n900 one? | 16:10 |
| Pali | special n900 one | 16:10 |
| Pali | linaro's qemu-system-arm | 16:10 |
| Pali | https://u-boot.readthedocs.io/en/latest/board/nokia/rx51.html#run-in-qemu | 16:11 |
| sicelo | thanks very much | 16:14 |
| sicelo | Pali: u-boot should be able to load separate zImage and separate DTB without any issue <--- how would one know which memory address to use for the dtb blob? | 16:16 |
| Pali | you can use any unused address | 16:17 |
| Pali | for example ${initrdaddr} if you are not going to use initrd | 16:17 |
| Pali | e.g.: load mmc ${mmcnum}:${mmcpart} ${kernaddr} zImage && load mmc ${mmcnum}:${mmcpart} ${initrdaddr} dtb && bootz ${kernaddr} - ${initrdaddr} | 16:17 |
| Pali | it boots zImage (not uImage) | 16:20 |
| sicelo | thank you. i'll give this a try. charging the device for now | 16:22 |
| freemangordon | uvos: could you please test the ^^^ patch, to see if it still crashes the device with 30mA limit | 16:33 |
| uvos | freemangordon: yes | 16:33 |
| uvos | i will | 16:33 |
| freemangordon | ok, thanks | 16:33 |
| freemangordon | that'd confirm I am on the right track | 16:34 |
| uvos | i mean the implementation looks fine | 16:34 |
| uvos | i just dont know if its the right way to do it | 16:34 |
| uvos | so cant really comment on that | 16:35 |
| freemangordon | sure, but if it fixes the issue... | 16:35 |
| freemangordon | hmm... | 16:35 |
| freemangordon | my a33 supports OTG | 16:36 |
| freemangordon | I can test there | 16:36 |
| uvos | freemangordon: reducing the current in sysfs | 16:36 |
| uvos | also fixes the issue atm | 16:36 |
| uvos | so i see no reason why the patch would not alos | 16:36 |
| uvos | but i will test | 16:36 |
| uvos | the device trying to pull 500mA just trippes the over current protection in xt1602 causing it to resett | 16:37 |
| freemangordon | sure, but there might be some transitions or who knows what that'd break it still | 16:41 |
| freemangordon | tablet seems to provide 500 mA | 17:21 |
Generated by irclog2html.py 2.17.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!