Hi Alper,
On Wed, 29 Dec 2021 at 15:01, Alper Nebi Yasak alpernebiyasak@gmail.com wrote:
On 28/12/2021 11:34, Simon Glass wrote:
Should add comments for the struct
I can do that and send as a v3.
Also I wonder if a simple fixed-length array might be possible instead of the linked list?
I think it's possible, my first prototype was something like:
#define MAX_PHYS 16
struct phy_uc_priv { int power_on_count[MAX_PHYS]; int init_count[MAX_PHYS]; };
uc_priv = dev_get_uclass_priv(phy->dev); uc_priv->power_on_count[phy->id]; uc_priv->init_count[phy->id];
UCLASS_DRIVER(phy) = { ... .per_device_auto = sizeof(struct phy_uc_priv), }
But I rewrote as a linked list because I couldn't decide on a reasonable value for that MAX_PHYS. Best guess I have is from drivers/phy/cadence/phy-cadence-sierra.c which defines SIERRA_MAX_LANES as 16.
You could make it a CONFIG.
I was more thinking of calculating the size and then allocating it in one hit, for simplicity. But given that you have something more flexible and it seems to be needed, let's leave it as it is.
As array-of-structs, I guess things would be roughly:
#define MAX_PHYS 16
struct phy_counts { int power_on_count; int init_count; };
counts = dev_get_uclass_priv(phy->dev)[phy->id]; counts->power_on_count; counts->init_count;
UCLASS_DRIVER(phy) = { ... .per_device_auto = sizeof(struct phy_counts[MAX_PHYS]), }
Should I change to that?
I really like driver model doing the allocation. But I think if you did that, it would be better to have a struct that included the array, so we can have a simple sizeof(struct xxx) there.
Regards, Simon