[PATCH 01/17] miiphy: Introduce mdio_init()

[Marek Vasut]

On 2/4/25 7:17 PM, Paul Barker wrote:
> On 27/01/2025 13:28, Paul Barker wrote:
>> On 27/01/2025 11:30, Marek Vasut wrote:
>>> On 1/27/25 11:32 AM, Paul Barker wrote:
>>>> Hi Marek,
>>>>
>>>> On 25/01/2025 12:56, Marek Vasut wrote:
>>>>> On 1/21/25 3:38 PM, Paul Barker wrote:
>>>>>> On 18/01/2025 06:53, Marek Vasut wrote:
>>>>>>> Introduce mdio_init() split off from mdio_alloc(), which is used
>>>>>>> to initialize already allocated struct mii_dev.
>>>>>>>
>>>>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas at mailbox.org>
>>>>>>
>>>>>> Replying here for the whole patch series...
>>>>>>
>>>>>> I like where we get to at the end of this series, but I don't like the
>>>>>> intermediate state where the bb_miiphy_buses arrays still exist but are
>>>>>> unused. I also think we should introduce a new ops struct so we're not
>>>>>> duplicating function pointers in each instance of struct bb_miiphy_bus.
>>>>>>
>>>>>> I would prefer the following order of changes, let me know if you think
>>>>>> this would be cleaner:
>>>>>>
>>>>>> 1) Introduce mdio_init().
>>>>>>
>>>>>> 2) Add mii member to struct bb_miiphy_bus.
>>>>>>
>>>>>> 3) Modify each driver to use the mii member of the appropriate struct
>>>>>>       bb_miiphy_bus instance, calling mdio_init() instead of mdio_alloc().
>>>>>>       At this point we're still using the instances in the statically
>>>>>>       allocated bb_miiphy_buses array.
>>>>>
>>>>> Those instances might be rodata , so using them as read-write storage
>>>>> does not necessarily work , does it ?
>>>>
>>>> None of the instances in the current U-Boot code are marked as const,
>>>> and the name field is already modified at runtime by each driver. So I
>>>> don't think we need to worry about them being rodata.
>>> Can the compiler not decide to place the structure into rodata if it is
>>> never written ?
>>
>> To my understanding the compiler cannot place anything in a different
>> section such as rodata unless this is explicitly requested with an
>> attribute (i.e. `__attribute__(( section(".rodata") ))` or a macro call
>> which resolves to this).
> 
> Hi Marek,
> 
> To follow up after our brief converstation at FOSDEM:
> 
> The important thing is where we arrive at the end of this series, the
> duplication we have in the intermediate state is unfortunate but it
> shouldn't be a blocker. If no one else is complaining, and it'd be time
> consuming to re-order and re-factor things, then please ignore my
> comments above and let's move ahead with your current proposal. I should
> have time this week to review the patches in more detail.
I did have one more look at this, but after a bit, I found out that this 
does not make the series easier, rather the opposite. Before 3) the 
driver does a mix of static and dynamic allocation of mdio and mii_phy 
structures , 3) would move the driver to static only allocation and 
remove the existing dynamic allocation, 11) would re-add the dynamic 
allocation . I'll be sending slightly reordered V2 now.