The issue here is: how to tell kernel that the particular dell_battery_hook has to be bound with the primary battery? > plugged into the docking port on old E* series Latitudes: > https://dl.dell.com/manuals/all-products/esuprt_laptop/esuprt_latitude_laptop/latitude-e5420_user's%20guide_en-us.pdf > There are also separate tokens marked "Dock Battery", which control > charging behavior for batteries that are.. part of the dock? It's > unclear, and I don't have the hardware. IIRC both "Dock Battery" and "Slice Battery" means battery type connected to the connector on the bottom of the laptop. Same connector is used for connecting laptop to dock. As different models had different type of connector, probably also different name for battery were used. And there was also another "Media Bay" battery which was being connected to the slot where was normally either optical drive or second HDD. > So I'm pretty comfortable with the assumption that this charging > control interface will ignore all batteries other than the primary > battery. The SMBIOS code will ignore for sure. But would not the battery_hook_register() function hook the SMBIOS interface to the "wrong" battery? > Of course, more testing would be great, and I haven't personally > dealt with multiple batteries. > > > > > > > > > I have one Dell laptop which has an option to connect secondary > > > battery, but I do not have the compatible secondary battery to test it. > > How does it connect? Is the interface one of the types that I mentioned > above (slice & dock), or a different type? Via bottom connector. So it is either slice or dock, do not know exactly as I do not have that battery. > > > > > > > Has somebody Dell laptop with 2 or 3 batteries? It would be really good > > > to test this patch how it would behave... > > Agreed. > > > > > > > > > +} > > > > + > > > > +static void __exit dell_battery_exit(void) > > > > +{ > > > > + if (battery_supported_modes != 0) > > > > + battery_hook_unregister(&dell_battery_hook); > > > > +} > > > > + > > > > static int __init dell_init(void) > > > > { > > > > struct calling_interface_token *token; > > > > @@ -2219,6 +2504,7 @@ static int __init dell_init(void) > > > > touchpad_led_init(&platform_device->dev); > > > > > > > > kbd_led_init(&platform_device->dev); > > > > + dell_battery_init(&platform_device->dev); > > > > > > > > dell_laptop_dir = debugfs_create_dir("dell_laptop", NULL); > > > > debugfs_create_file("rfkill", 0444, dell_laptop_dir, NULL, > > > > @@ -2293,6 +2579,7 @@ static int __init dell_init(void) > > > > if (mute_led_registered) > > > > led_classdev_unregister(&mute_led_cdev); > > > > fail_led: > > > > + dell_battery_exit(); > > > > dell_cleanup_rfkill(); > > > > fail_rfkill: > > > > platform_device_del(platform_device); > > > > @@ -2311,6 +2598,7 @@ static void __exit dell_exit(void) > > > > if (quirks && quirks->touchpad_led) > > > > touchpad_led_exit(); > > > > kbd_led_exit(); > > > > + dell_battery_exit(); > > > > backlight_device_unregister(dell_backlight_device); > > > > if (micmute_led_registered) > > > > led_classdev_unregister(&micmute_led_cdev); > > > > diff --git a/drivers/platform/x86/dell/dell-smbios.h b/drivers/platform/x86/dell/dell-smbios.h > > > > index ea0cc38642a2..77baa15eb523 100644 > > > > --- a/drivers/platform/x86/dell/dell-smbios.h > > > > +++ b/drivers/platform/x86/dell/dell-smbios.h > > > > @@ -33,6 +33,13 @@ > > > > #define KBD_LED_AUTO_50_TOKEN 0x02EB > > > > #define KBD_LED_AUTO_75_TOKEN 0x02EC > > > > #define KBD_LED_AUTO_100_TOKEN 0x02F6 > > > > +#define BAT_PRI_AC_MODE_TOKEN 0x0341 > > > > +#define BAT_ADAPTIVE_MODE_TOKEN 0x0342 > > > > +#define BAT_CUSTOM_MODE_TOKEN 0x0343 > > > > +#define BAT_STANDARD_MODE_TOKEN 0x0346 > > > > +#define BAT_EXPRESS_MODE_TOKEN 0x0347 > > > > > > There is defined also tokenid 0x0348 which is per dell libsmbios's > > > token_list.csv documentation also for custom mode. It is marked as > > > deprecated, so seems that it is for old laptops, which do not support > > > tokenid 0x0343. > > > > > > What about defining '#define BAT_CUSTOM_OLD_MODE_TOKEN 0x0348', then > > > adding '{ BAT_CUSTOM_OLD_MODE_TOKEN, "custom" }' into battery_modes and > > > changing battery_get_supported_modes() to mask out custom_old bit when > > > the normal custom is supported? Seems that it would be OK, just the > > > charge_type_store() needs modification as written above. > > > > > > Just an idea. What do you think? > > > > > Generally, I'm nervous about touching stuff like that that I can't > personally test. I don't know if 0348 works the same way as 0343, and > the libsmbios code that added 0343 usage > (<https://github.com/dell/libsmbios/commit/0eab0085e8f0db3c2a0f8ae0146d3352c40d785a>) > never actually used 0348 previously. Or, it could be that 0348 was > deprecated due to buggy semantics or implementation. I just don't > know. > > If there's old code or documentation somewhere that shows how 0348 > worked, and it turns out to be similar to 0343, then I wouldn't be > opposed to adding BAT_CUSTOM_OLD_MODE_TOKEN and checking for it. > However, without any kind of example, I'd prefer to just ignore it > and only support custom mode charging on machines that have 0343. > My 10yo Latitude has 0343, so I suspect that we're targeting the vast > majority of (still working) hardware with 0343. Ok, I think that for now let it as is. It can be extended in the future after proper inspection. That mentioned dell commit does not contain any useful information about it. > > > > > +#define BAT_CUSTOM_CHARGE_START 0x0349 > > > > +#define BAT_CUSTOM_CHARGE_END 0x034A > > > > #define GLOBAL_MIC_MUTE_ENABLE 0x0364 > > > > #define GLOBAL_MIC_MUTE_DISABLE 0x0365 > > > > #define GLOBAL_MUTE_ENABLE 0x058C > > > > -- > > > > 2.39.2 > > > > > > > > > > > > > > > > > > > > -- > > > > I'm available for contract & employment work, see: > > > > https://spindle.queued.net/~dilinger/resume-tech.pdf > > > > -- > I'm available for contract & employment work, see: > https://spindle.queued.net/~dilinger/resume-tech.pdf
