Re: [PATCH net-next v2 3/4] net: phy: air_phy_lib: Factorize BuckPBus register accessors

[Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>]

Hi Andrew,

On Thu, 2026-05-07 at 23:36 +0200, Andrew Lunn wrote:
> On Thu, May 07, 2026 at 02:11:54PM +0200, Louis-Alexis Eyraud wrote:
> > Hi Andrew,
> > 
> > On Thu, 2026-03-26 at 13:30 +0100, Andrew Lunn wrote:
> > > > @@ -480,8 +287,8 @@ static int en8811h_wait_mcu_ready(struct
> > > > phy_device *phydev)
> > > >  {
> > > >  	int ret, reg_value;
> > > >  
> > > > -	ret = air_buckpbus_reg_write(phydev,
> > > > EN8811H_FW_CTRL_1,
> > > > -				    
> > > > EN8811H_FW_CTRL_1_FINISH);
> > > > +	ret = air_phy_buckpbus_reg_write(phydev,
> > > > EN8811H_FW_CTRL_1,
> > > > +					
> > > > EN8811H_FW_CTRL_1_FINISH);
> > > 
> > > Is a rename required? Is the namespace air_buckpbus_ used
> > > somewhere
> > > else?
> > > 
> > > 	Andrew
> > Sorry for the delay.
> > 
> > The air_buckpbus_ namespace is only used in the air_en8811h driver.
> > It seemed better to me that in the new air_phy_lib, all functions
> > (the
> > buckpbus accessors and air_phy_read/write_page functions) started
> > with
> > the same prefix. That is the reason I renamed them, even if not
> > required.
> > 
> > As an alternative, to avoid renaming those buckpbus function calls
> > on
> > air_en8811h driver and reduce this patch changes, I can add macros
> > at
> > the beginning of the file such as:
> > ```
> > #define air_buckpbus_reg_write(_phydev, _pbus_address, _pbus_data)
> > \
> > 	air_phy_buckpbus_reg_write(_phydev, _pbus_address,
> > _pbus_data)
> 
> No don't do this.
> 
> If you want to rename them, rename them. But do it in a patch which
> only contains a rename. That is easier to review, and more obviously
> correct.
I'll add in v3 a new patch to rename the functions and the calls in
air_en8811h.c then.

Best regards,
Louis-Alexis

> 
> 	Andrew