lsmod when all cros_ec are modules:
    lsmod | grep cros_ec
    cros_ec_sensors_ring    16384  0
    cros_ec_sensors        16384  2
    cros_ec_sensors_core    16384  2 cros_ec_sensors_ring,cros_ec_sensors
    industrialio_triggered_buffer    16384  1 cros_ec_sensors
    cros_ec_dev            20480  0
    cros_usbpd_charger     20480  1 cros_ec_dev
    cros_ec_ctl            24576  1 cros_ec_dev
    ...
    cros_ec_lpcs           16384  0
    cros_ec_pd_update      16384  1 cros_ec_dev
    cros_ec_core           16384  1 cros_ec_lpcs

At a glance, it looks to me like cros_ec_dev is indeed not using cros_ec_core directly, but it is going via cros_ec_proto for the actual device communication (EC_PROTO is not a module, it's set to 'y' even in Gwendal's patch)

ec_proto is then talking to the ec_dev - and it is this last guy who lives in cros_ec_core

Not yet sure what to make of it

cros_ec_proto is not a module, just a list of helper functions.

Given these functions are required, CONFIG_CROS_EC_PROTO will always be 'y' when cros ec stack is used, cros_ec_proto.c could be compiled as part of cros_ec_dev or cros_ec_core.

Per offline chat with Gwendal, we could make a ec_proto module or - as per comment #6 - have ec_proto be part of an existing module. This would enable saner dependency tracking.

Right now, the only connection between the usbpd_charger and cros_ec_core is via a function pointer in a struct - which is not tracked via the module dependency logic as that relies on symbols

Reproduced by just doing:

$ rmmod cros_ec_lpcs

while ssh'ed into the chromebook

[  254.943246] BUG: unable to handle kernel paging request at ffffffffc041f1e9
[  254.949382] IP: 0xffffffffc041f1e9
[  254.952319] PGD 217414067 P4D 217414067 PUD 217416067 PMD 42c444067 PTE 0
[  254.959334] Oops: 0010 [#1] PREEMPT SMP PTI
[  254.965790] gsmi: Log Shutdown Reason 0x03
[  254.968941] Modules linked in: cmac rfcomm uinput iio_trig_sysfs cros_ec_sensors_ring cros_ec_sensors cros_ec_sensors_core industriali]
[  255.039940] CPU: 0 PID: 100 Comm: kworker/u8:4 Tainted: G     U          4.14.43 #3
[  255.055366] Workqueue: cros_usb_pd_log cros_usb_pd_log_check [cros_usbpd_charger]
[  255.062463] task: ffff8f9e2b478000 task.stack: ffff9fc7806b4000
[  255.068320] RIP: 0010:0xffffffffc041f1e9
[  255.072419] RSP: 0018:ffff9fc7806b7d38 EFLAGS: 00010202
[  255.077947] RAX: 00000000000000f8 RBX: ffff8f9e0366de28 RCX: 0000000000000078
[  255.085210] RDX: 0000000000000000 RSI: ffff8f9de4421700 RDI: ffff8f9e0366de28
[  255.092235] RBP: ffff9fc7806b7d70 R08: 00000000014080c0 R09: 0000000000000020
[  255.099414] R10: 0000000000000020 R11: 000000000000000a R12: ffff8f9e0366de28
[  255.106546] R13: ffffffffc041f1e9 R14: ffff8f9de4421700 R15: ffff8f9e23e39000
[  255.113681] FS:  0000000000000000(0000) GS:ffff8f9e3ec00000(0000) knlGS:0000000000000000
[  255.122000] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  255.127634] CR2: ffffffffc041f1e9 CR3: 0000000217412001 CR4: 00000000003606f0
[  255.134765] Call Trace:
[  255.136445]  ? send_command+0x2c/0xd4
[  255.140599]  ? cros_ec_cmd_xfer+0xbc/0xd1
[  255.144727]  ? cros_ec_cmd_xfer_status+0x19/0x4c
[  255.149147]  ? ec_command.constprop.8+0x9c/0xcf [cros_usbpd_charger]
[  255.155937]  ? cros_usb_pd_log_check+0x59/0xb5 [cros_usbpd_charger]
[  255.161758]  ? __schedule+0x3d6/0x563
[  255.165792]  ? process_one_work+0x1b0/0x314
[  255.169078]  ? worker_thread+0x1cb/0x2c1
[  255.173180]  ? create_worker+0x1da/0x1da
[  255.177410]  ? kthread+0x156/0x15e
[  255.180358]  ? kthread_flush_work+0xea/0xea
[  255.184531]  ? ret_from_fork+0x35/0x40
[  255.188713] Code:  Bad RIP value.
[  255.191638] RIP: 0xffffffffc041f1e9 RSP: ffff9fc7806b7d38
[  255.197220] CR2: ffffffffc041f1e9
[  255.201159] ---[ end trace d0ed22bcea4065bc ]---
[  255.208202] Kernel panic - not syncing: Fatal exception
[  255.212731] Kernel Offset: 0x18c00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[  255.222851] gsmi: Log Shutdown Reason 0x02
[  255.229676] ACPI MEMORY or I/O RESET_REG.

Gerrit patch 1083753 fixes the cros_ec_core issue by making cros_ec_dev depend on cros_ec_core

I am still investigating the cros_ec_lpcs issue. Some data points:

When the ec_dev comes up, it sets its transfer function pointers to the lpcs provided cros_ec_cmd_xfer_lpc and cros_ec_pkt_xfer_lpc

When cros_ec_lpcs gets unloaded, the memory pages go away, and in turn this causes a Bad IP trap once one tries to use them again:

[    7.871806] cros_ec_lpcs GOOG0004:00: setting cmd_xfer = ffffffffc052831f and pkt_xfer = ffffffffc05281e9
<remove cros_ec_lpcs>
[  132.063240] using xfer_fxn = ffffffffc05281e9
[  132.068200] BUG: unable to handle kernel paging request at ffffffffc05281e9
[  132.075127] IP: 0xffffffffc05281e9

where ffffffffc05281e9 is cros_ec_pkt_xfer_lpc

I need to figure out a way to make the dependency of on cros_ec_lpcs explicit, since right now the reliance on function pointers hides the symbol import-export relationship

I sent out a patch to upstream Linux. Awaiting for feedback/review.

This bug has an owner, thus, it's been triaged. Changing status to "assigned".