[dwmac] split dwmac into two devices
Bug:73278
Test:vim3 check for panic
device-enumeration-test
Change-Id: I89424effc8462f9faec6b7dcc864da9d156c5c16
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/512776
Fuchsia-Auto-Submit: Ruby Zhuang <rdzhuang@google.com>
Reviewed-by: James Robinson <jamesr@google.com>
Reviewed-by: Alex Legg <alexlegg@google.com>
Reviewed-by: Adrian Danis <adanis@google.com>
Reviewed-by: Suraj Malhotra <surajmalhotra@google.com>
Commit-Queue: Ruby Zhuang <rdzhuang@google.com>
diff --git a/boards/kernel_cmdline/BUILD.gn b/boards/kernel_cmdline/BUILD.gn
index b04867d..9d877d4 100644
--- a/boards/kernel_cmdline/BUILD.gn
+++ b/boards/kernel_cmdline/BUILD.gn
@@ -76,6 +76,6 @@
kernel_cmdline("vim3") {
args = [
# Prefer using the built-in NIC to the CDC-ether interface.
- "netsvc.interface=/dev/dwmac/Designware-MAC/ethernet",
+ "netsvc.interface=/dev/dwmac/dwmac/Designware-MAC/ethernet",
]
}
diff --git a/src/connectivity/ethernet/drivers/dwmac/dwmac.cc b/src/connectivity/ethernet/drivers/dwmac/dwmac.cc
index 34f9465..8bbbf5e 100644
--- a/src/connectivity/ethernet/drivers/dwmac/dwmac.cc
+++ b/src/connectivity/ethernet/drivers/dwmac/dwmac.cc
@@ -92,13 +92,21 @@
int ret = thrd_create_with_name(&thread_, thunk, this, "mac-thread");
ZX_DEBUG_ASSERT(ret == thrd_success);
- zx_status_t status = DdkAdd("Designware-MAC");
+ fbl::AllocChecker ac;
+ std::unique_ptr<EthPhyFunction> phy_function(new (&ac) EthPhyFunction(zxdev(), this));
+ if (!ac.check()) {
+ DdkAsyncRemove();
+ return ZX_ERR_NO_MEMORY;
+ }
+ auto status = phy_function->DdkAdd("Designware-MAC");
if (status != ZX_OK) {
zxlogf(ERROR, "dwmac: Could not create eth device: %d", status);
+ DdkAsyncRemove();
return status;
} else {
zxlogf(INFO, "dwmac: Added dwMac device");
}
+ phy_function_ = phy_function.release();
return status;
}
@@ -207,6 +215,12 @@
sync_completion_reset(&mac_device->cb_registered_signal_);
+ status = mac_device->DdkAdd("dwmac", DEVICE_ADD_NON_BINDABLE);
+ if (status != ZX_OK) {
+ zxlogf(ERROR, "DWMac DdkAdd failed %d", status);
+ return status;
+ }
+
// Populate board specific information
eth_dev_metadata_t phy_info;
size_t actual;
@@ -223,25 +237,20 @@
{BIND_PLATFORM_DEV_DID, 0, phy_info.did},
};
- device_add_args_t phy_device_args = {};
- phy_device_args.version = DEVICE_ADD_ARGS_VERSION;
- phy_device_args.name = "eth_phy";
- phy_device_args.ops = &mac_device->ddk_device_proto_,
- phy_device_args.proto_id = ZX_PROTOCOL_ETH_MAC;
- phy_device_args.props = props;
- phy_device_args.prop_count = countof(props);
- phy_device_args.ctx = mac_device.get();
- phy_device_args.proto_ops = &mac_device->eth_mac_protocol_ops_;
-
+ fbl::AllocChecker ac;
+ std::unique_ptr<EthMacFunction> mac_function(
+ new (&ac) EthMacFunction(mac_device->zxdev(), mac_device.get()));
+ if (!ac.check()) {
+ return ZX_ERR_NO_MEMORY;
+ }
// TODO(braval): use proper device pointer, depending on how
// many PHY devices we have to load, from the metadata.
- zx_device_t* dev;
- status = device_add(device, &phy_device_args, &dev);
+ status = mac_function->DdkAdd(ddk::DeviceAddArgs("eth_phy").set_props(props));
if (status != ZX_OK) {
- zxlogf(ERROR, "dwmac: Could not create phy device: %d", status);
-
+ zxlogf(ERROR, "DdkAdd for Eth Mac Function failed %d", status);
return status;
}
+ mac_device->mac_function_ = mac_function.release();
auto worker_thunk = [](void* arg) -> int {
return reinterpret_cast<DWMacDevice*>(arg)->WorkerThread();
@@ -354,7 +363,9 @@
}
DWMacDevice::DWMacDevice(zx_device_t* device, ddk::PDev pdev, ddk::EthBoardProtocolClient eth_board)
- : ddk::Device<DWMacDevice, ddk::Unbindable>(device), pdev_(pdev), eth_board_(eth_board) {}
+ : ddk::Device<DWMacDevice, ddk::Unbindable, ddk::Suspendable>(device),
+ pdev_(pdev),
+ eth_board_(eth_board) {}
void DWMacDevice::ReleaseBuffers() {
// Unpin the memory used for the dma buffers
@@ -368,6 +379,8 @@
void DWMacDevice::DdkRelease() {
zxlogf(INFO, "Ethernet release...");
+ ZX_ASSERT(phy_function_ == nullptr);
+ ZX_ASSERT(mac_function_ == nullptr);
delete this;
}
@@ -377,6 +390,13 @@
txn.Reply();
}
+void DWMacDevice::DdkSuspend(ddk::SuspendTxn txn) {
+ zxlogf(INFO, "Ethernet DdkSuspend");
+ // We do not distinguish between states, just completely shutdown.
+ ShutDown();
+ txn.Reply(ZX_OK, txn.requested_state());
+}
+
zx_status_t DWMacDevice::ShutDown() {
if (running_.load()) {
running_.store(false);
diff --git a/src/connectivity/ethernet/drivers/dwmac/dwmac.h b/src/connectivity/ethernet/drivers/dwmac/dwmac.h
index e3b530a..385ae82 100644
--- a/src/connectivity/ethernet/drivers/dwmac/dwmac.h
+++ b/src/connectivity/ethernet/drivers/dwmac/dwmac.h
@@ -92,9 +92,10 @@
namespace eth {
-class DWMacDevice : public ddk::Device<DWMacDevice, ddk::Unbindable>,
- public ddk::EthernetImplProtocol<DWMacDevice, ddk::base_protocol>,
- public ddk::EthMacProtocol<DWMacDevice> {
+class EthPhyFunction;
+class EthMacFunction;
+
+class DWMacDevice : public ddk::Device<DWMacDevice, ddk::Unbindable, ddk::Suspendable> {
public:
DWMacDevice(zx_device_t* device, ddk::PDev pdev, ddk::EthBoardProtocolClient eth_board);
@@ -102,6 +103,7 @@
void DdkRelease();
void DdkUnbind(ddk::UnbindTxn txn);
+ void DdkSuspend(ddk::SuspendTxn txn);
// ZX_PROTOCOL_ETHERNET_IMPL ops.
zx_status_t EthernetImplQuery(uint32_t options, ethernet_info_t* info);
@@ -120,6 +122,9 @@
zx_status_t EthMacRegisterCallbacks(const eth_mac_callbacks_t* callbacks);
private:
+ friend class EthMacFunction;
+ friend class EthPhyFunction;
+
zx_status_t InitBuffers();
zx_status_t InitDevice();
zx_status_t DeInitDevice() __TA_REQUIRES(lock_);
@@ -194,6 +199,90 @@
// Callbacks registered signal.
sync_completion_t cb_registered_signal_;
+
+ EthPhyFunction* phy_function_;
+ EthMacFunction* mac_function_;
+};
+
+using EthPhyFunctionType = ddk::Device<EthPhyFunction, ddk::Unbindable, ddk::Suspendable>;
+class EthPhyFunction : public EthPhyFunctionType,
+ public ddk::EthernetImplProtocol<EthPhyFunction, ddk::base_protocol> {
+ public:
+ explicit EthPhyFunction(zx_device_t* device, DWMacDevice* peripheral)
+ : EthPhyFunctionType(device), device_(peripheral) {}
+
+ void DdkUnbind(ddk::UnbindTxn txn) {
+ device_->phy_function_ = nullptr;
+ device_ = nullptr;
+ txn.Reply();
+ }
+ void DdkSuspend(ddk::SuspendTxn txn) {
+ device_->phy_function_ = nullptr;
+ device_ = nullptr;
+ txn.Reply(ZX_OK, txn.requested_state());
+ }
+ void DdkRelease() {
+ ZX_ASSERT(device_ == nullptr);
+ delete this;
+ }
+
+ // ZX_PROTOCOL_ETHERNET_IMPL ops.
+ zx_status_t EthernetImplQuery(uint32_t options, ethernet_info_t* info) {
+ return device_->EthernetImplQuery(options, info);
+ }
+ void EthernetImplStop() { device_->EthernetImplStop(); }
+ zx_status_t EthernetImplStart(const ethernet_ifc_protocol_t* ifc) {
+ return device_->EthernetImplStart(ifc);
+ }
+ void EthernetImplQueueTx(uint32_t options, ethernet_netbuf_t* netbuf,
+ ethernet_impl_queue_tx_callback completion_cb, void* cookie) {
+ device_->EthernetImplQueueTx(options, netbuf, completion_cb, cookie);
+ }
+ zx_status_t EthernetImplSetParam(uint32_t param, int32_t value, const uint8_t* data,
+ size_t data_size) {
+ return device_->EthernetImplSetParam(param, value, data, data_size);
+ }
+ void EthernetImplGetBti(zx::bti* bti) { device_->EthernetImplGetBti(bti); }
+
+ private:
+ DWMacDevice* device_;
+};
+
+using EthMacFunctionType = ddk::Device<EthMacFunction, ddk::Unbindable, ddk::Suspendable>;
+class EthMacFunction : public EthMacFunctionType,
+ public ddk::EthMacProtocol<EthMacFunction, ddk::base_protocol> {
+ public:
+ explicit EthMacFunction(zx_device_t* device, DWMacDevice* peripheral)
+ : EthMacFunctionType(device), device_(peripheral) {}
+
+ void DdkUnbind(ddk::UnbindTxn txn) {
+ device_->mac_function_ = nullptr;
+ device_ = nullptr;
+ txn.Reply();
+ }
+ void DdkSuspend(ddk::SuspendTxn txn) {
+ device_->mac_function_ = nullptr;
+ device_ = nullptr;
+ txn.Reply(ZX_OK, txn.requested_state());
+ }
+ void DdkRelease() {
+ ZX_ASSERT(device_ == nullptr);
+ delete this;
+ }
+
+ // ZX_PROTOCOL_ETH_MAC ops.
+ zx_status_t EthMacMdioWrite(uint32_t reg, uint32_t val) {
+ return device_->EthMacMdioWrite(reg, val);
+ }
+ zx_status_t EthMacMdioRead(uint32_t reg, uint32_t* val) {
+ return device_->EthMacMdioRead(reg, val);
+ }
+ zx_status_t EthMacRegisterCallbacks(const eth_mac_callbacks_t* callbacks) {
+ return device_->EthMacRegisterCallbacks(callbacks);
+ }
+
+ private:
+ DWMacDevice* device_;
};
} // namespace eth
diff --git a/zircon/system/utest/device-enumeration/main.cc b/zircon/system/utest/device-enumeration/main.cc
index 37b73b2..ffcf287 100644
--- a/zircon/system/utest/device-enumeration/main.cc
+++ b/zircon/system/utest/device-enumeration/main.cc
@@ -174,7 +174,7 @@
"aml_emmc/aml-sd-emmc/sdmmc/sdmmc-mmc/user/block/part-006/block",
// "sys/platform/05:00:3/aml-uart/serial/bt-transport-uart/bcm-hci",
"sys/platform/05:00:2/aml-i2c/i2c/i2c-1-81/rtc", "sys/platform/00:00:2/xhci/usb-bus",
- "mali/aml-gpu", "dwmac/eth_phy/phy_null_device", "dwmac/Designware-MAC/ethernet",
+ "mali/aml-gpu", "dwmac/dwmac/eth_phy/phy_null_device", "dwmac/dwmac/Designware-MAC/ethernet",
"display/vim2-display/display-controller", "vim-thermal/vim-thermal", "gpio-light/gpio-light",
"wifi/brcmfmac-wlanphy", "aml-sdio/aml-sd-emmc/sdmmc",
"aml-sdio/aml-sd-emmc/sdmmc/sdmmc-sdio", "aml-sdio/aml-sd-emmc/sdmmc/sdmmc-sdio/sdmmc-sdio-1",
@@ -194,8 +194,8 @@
"sys/platform/05:00:14/clocks",
"sys/platform/05:00:2/aml-i2c",
"sys/platform/05:00:2/aml-i2c/i2c/i2c-0-81/rtc",
- "dwmac/eth_phy/phy_null_device",
- "dwmac/Designware-MAC/ethernet",
+ "dwmac/dwmac/eth_phy/phy_null_device",
+ "dwmac/dwmac/Designware-MAC/ethernet",
"aml_sd/aml-sd-emmc",
"aml_sdio/aml-sd-emmc/sdmmc/sdmmc-sdio/sdmmc-sdio-1",
"aml_sdio/aml-sd-emmc/sdmmc/sdmmc-sdio/sdmmc-sdio-2",
@@ -311,9 +311,15 @@
TEST_F(DeviceEnumerationTest, NelsonTest) {
static const char* kDevicePaths[] = {
- "sys/platform/nelson", "sys/platform/05:03:1/aml-axg-gpio", "nelson-buttons/hid-buttons",
- "class/bt-transport/000", "class/bt-hci/000", "sys/platform/05:00:2/aml-i2c", "mali/aml-gpu",
- "sys/platform/05:0a:21/nelson-usb-phy", "nelson-audio-i2s-out",
+ "sys/platform/nelson",
+ "sys/platform/05:03:1/aml-axg-gpio",
+ "nelson-buttons/hid-buttons",
+ "class/bt-transport/000",
+ "class/bt-hci/000",
+ "sys/platform/05:00:2/aml-i2c",
+ "mali/aml-gpu",
+ "sys/platform/05:0a:21/nelson-usb-phy",
+ "nelson-audio-i2s-out",
"sys/platform/05:05:13/nelson-audio-pdm-in",
"sys/platform/00:00:29", // registers device
@@ -324,8 +330,11 @@
// so we can't just test that one of them is bound. That is why the
// following test is disabled.
// "sys/platform/03:03:5/gt92xx HidDevice/hid-device-000",
- "backlight/ti-lp8556", "sys/platform/05:00:10/aml-canvas", "tee/optee",
- "sys/platform/00:00:f/fallback-rtc", "nelson-emmc/aml-sd-emmc/sdmmc/sdmmc-mmc/boot1/block",
+ "backlight/ti-lp8556",
+ "sys/platform/05:00:10/aml-canvas",
+ "tee/optee",
+ "sys/platform/00:00:f/fallback-rtc",
+ "nelson-emmc/aml-sd-emmc/sdmmc/sdmmc-mmc/boot1/block",
"nelson-emmc/aml-sd-emmc/sdmmc/sdmmc-mmc/boot2/block",
"nelson-emmc/aml-sd-emmc/sdmmc/sdmmc-mmc/rpmb",
"nelson-emmc/aml-sd-emmc/sdmmc/sdmmc-mmc/user/block/part-000/block",
@@ -346,20 +355,38 @@
"nelson-emmc/aml-sd-emmc/sdmmc/sdmmc-mmc/user/block/part-015/block",
"nelson-emmc/aml-sd-emmc/sdmmc/sdmmc-mmc/user/block/part-016/block",
"nelson-emmc/aml-sd-emmc/sdmmc/sdmmc-mmc/user/block/part-017/block",
- "tcs3400-light/tcs-3400/hid-device-000", "aml-nna", "sys/platform/05:05:22/clocks",
- "aml-thermal-pll/thermal", "class/thermal/000",
+ "tcs3400-light/tcs-3400/hid-device-000",
+ "aml-nna",
+ "sys/platform/05:05:22/clocks",
+ "aml-thermal-pll/thermal",
+ "class/thermal/000",
// "sys/platform/05:03:1e/cpu",
- "aml-secure-mem/aml-securemem", "class/pwm/000", "class/pwm/001", "class/pwm/002",
- "class/pwm/003", "class/pwm/004", "class/pwm/005", "class/pwm/006", "class/pwm/007",
- "class/pwm/008", "class/pwm/009", "aml-sdio/aml-sd-emmc/sdmmc",
- "aml-sdio/aml-sd-emmc/sdmmc/sdmmc-sdio", "aml-sdio/aml-sd-emmc/sdmmc/sdmmc-sdio/sdmmc-sdio-1",
- "aml-sdio/aml-sd-emmc/sdmmc/sdmmc-sdio/sdmmc-sdio-2", "sys/platform/00:00:1e/dw-dsi",
- "display/amlogic-display/display-controller", "class/dsi-base/000", "ti-ina231-mlb/ti-ina231",
- "ti-ina231-speakers/ti-ina231", "sys/platform/05:00:2/aml-i2c/i2c/i2c-0-112/shtv3",
+ "aml-secure-mem/aml-securemem",
+ "class/pwm/000",
+ "class/pwm/001",
+ "class/pwm/002",
+ "class/pwm/003",
+ "class/pwm/004",
+ "class/pwm/005",
+ "class/pwm/006",
+ "class/pwm/007",
+ "class/pwm/008",
+ "class/pwm/009",
+ "aml-sdio/aml-sd-emmc/sdmmc",
+ "aml-sdio/aml-sd-emmc/sdmmc/sdmmc-sdio",
+ "aml-sdio/aml-sd-emmc/sdmmc/sdmmc-sdio/sdmmc-sdio-1",
+ "aml-sdio/aml-sd-emmc/sdmmc/sdmmc-sdio/sdmmc-sdio-2",
+ "sys/platform/00:00:1e/dw-dsi",
+ "display/amlogic-display/display-controller",
+ "class/dsi-base/000",
+ "ti-ina231-mlb/ti-ina231",
+ "ti-ina231-speakers/ti-ina231",
+ "sys/platform/05:00:2/aml-i2c/i2c/i2c-0-112/shtv3",
"gt6853-touch/gt6853",
// Amber LED.
- "gpio-light", "class/light/000",
+ "gpio-light",
+ "class/light/000",
"spi/aml-spi-1/spi/spi-1-0",
"selina/selina",
