-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Somebody in the thread at some point said: | On Tuesday 22 July 2008 13:15:07 Andy Green wrote: |> These patches try to do something about the random non-charging |> behaviour getting reported, along with some incorrect behaviours |> of the charging trigger stuff Holger worked on. | | cool. I have some minor comments and one question. | | pcf50633_usb_curlim_set gets called from gta02 code in return to a usb plug | action? So we enable charging when we know that a USB cable is inserted and | can take the power we want, otherwise go to idle? Sounds sane.
mach-gta02.c doesn't directly deal with pcf50633_charge_enable or charger enable after this patch at all. The only thing it glues together now is the USB enumeration event to pcf50633 world, but it does that by calling an opaque export from pcf50633 which then does the pcf50633_charge_enable call on that side of the fence. So pcf50633 charger got a little bit more selfcontained and mach-gta02.c needed to know less detail about the PMU. | minor comments: | - I think we call the callbacks twice now. Once in set_cur_limit once in the | enable charging.. Ha you're right. It's harmless but it's wrong to do it twice. I'll see how safe that patient is to operate on further. | - To increase readability of the patch you might want to do the pdata change | as a separate patch? Too late, but good advice. It doesn't affect the code just the patch beauty level. I find unless I layer them while I do them often stuff gets mingled in single patch stanzas and then it is hard to pick them apart. And when I am sweating about what I am doing, I don't even know the patch is viable until it works much later so beautification seems a double waste. - -Andy -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkiGFdIACgkQOjLpvpq7dMqL6ACfecaCsdVvmNMY40k5g+0xeSJh 1soAoIX9wsqdk5GhohrQVaYBSLnQmQDW =O19A -----END PGP SIGNATURE-----
