Re: [PATCH 3/7] net: phy: sync phy_{write,read,modify}_mmd_indirect with linux

[Marco Felsch <m.felsch@pengutronix.de>]

On 23-07-10, Ahmad Fatoum wrote:
> On 10.07.23 13:01, Marco Felsch wrote:
> > On 23-07-10, Ahmad Fatoum wrote:
> >> Hello Marco,
> >>
> >> On 10.07.23 12:49, Marco Felsch wrote:
> >>> On 23-07-10, Ahmad Fatoum wrote:
> >>>> On 10.07.23 08:36, Marco Felsch wrote:
> >>>>> Sync the barebox phy_{write,read,modify}_mmd_indirect API with the Linux
> >>>>> phy_{write,read,modify}_mmd API to make it easier and less error prone
> >>>>> to port phy drivers. This also fixes the r8169 driver since this driver
> >>>>> did not adapt the parameters while porting from Linux.
> >>>>>
> >>>>> The sync also aligns the function parameter `prtad` as well as the
> >>>>> function documentation.
> >>>>
> >>>> This breaks out of tree board code in a subtle and annoying manner.
> >>>
> >>> Do we really need to care about out all-of-tree drivers/boards? If so,
> >>> we need to ensure to not break any public function.
> >>
> >> Out-of-tree drivers not too much probably, but out-of-tree boards
> >> are unfortunately the norm. Breaking the build can be justified,
> >> but silent breaking by swapping arguments is not nice.
> > 
> > And how do you ensure that a public API is only used by drivers? Don't
> > get me wrong I got your point but if you update an out-of-tree board to
> > a newer barebox version it may happen that something break. Therefore
> > manufacturers should upstream there code to reduce the maintenance
> > effort to a minimum :) I know we don't live in an ideal world :/
> 
> Some breakage is ultimately unavoidable, but we can try to not needlessly
> introduce silent breakage. Here we have a nice way out: We import the
> Linux function with the Linux name and use that for upstream code. The old
> function is marked deprecated and removed later on. Do you see an issue
> with that?

The only problem I see is, that the Linux API does support C45 and C22
and we don't. As said, we can implement it and make the C45 support a
stub. From user perspective I would assume that the new API is working
for both classes (like Linux). Therefore I kept the _indirect suffix
since this makes it clear from API pov.

Regards,
  Marco