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+rene...@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. Thanks, -- Paul Barker
OpenPGP_0x27F4B3459F002257.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
