Hi
,[..]
+UCLASS_DRIVER(generic_phy) = {
.id = UCLASS_PHY,I would like the uclass name to match the header file and the uclass name. So if you are calling this generic_phy, then the uclass should be named this too. Same with the directory drivers/phy. If you want to rename it all to 'phy' then that is fine too.
IMO 'phy' would be the best option. Unfortunately there are already tons of functions starting with 'phy_' and they're used for the ethernet phy. So I would propose to use 'phy' everywhere except for the API where 'generic_phy_' can be used to prefix the functions. What do you think ?
Sounds good. That would be easy to clean up later once Ethernet Phy is done.
.name = "generic_phy",+}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 8c92d0b..9d34a32 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -83,6 +83,7 @@ enum uclass_id { UCLASS_VIDEO, /* Video or LCD device */ UCLASS_VIDEO_BRIDGE, /* Video bridge, e.g. DisplayPort to LVDS */ UCLASS_VIDEO_CONSOLE, /* Text console driver for video device */
UCLASS_PHY, /* generic PHY device */Physical layer device? Can you make your comment a bit more useful?
UCLASS_COUNT, UCLASS_INVALID = -1,diff --git a/include/generic-phy.h b/include/generic-phy.h new file mode 100644 index 0000000..24475f0 --- /dev/null +++ b/include/generic-phy.h @@ -0,0 +1,36 @@ +/*
- Copyright (C) 2017 Texas Instruments Incorporated -
- Written by Jean-Jacques Hiblot jjhiblot@ti.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#ifndef __GENERIC_PHY_H +#define __GENERIC_PHY_H
+/*
- struct udevice_ops - set of function pointers for phy operations
- @init: operation to be performed for initializing phy (optionnal)
optional
Can you please put these comments in front of each function in struct generic_phy_ops as is done with other uclasses? Your description should explain what the operation does. For example, what happens for init()? Since the phy has already been probed, what should you not do in probe() but do in init()?
I'll work on documenting this. Too bad that linux also lacks this documentation. The core idea is that probe() does not do much hardware-wise, it sets up everyting (irqs, mapping, clocks) but the actual initialization of the hardware is done in init().
OK sounds good.
Regards, Simon