Re: [PATCH v3 2/9] phy: stm32: Add support for ST STM32MP25 USB2-FEMTO PHY

[Marek Vasut]

On 8/27/26 6:00 PM, Vinod Koul wrote:

[...]

> > +static int stm32_usb2phy_enable(struct stm32_usb2phy *phy_dev)
> > +{
> > +	const struct stm32mp2_usb2phy_hw_data *phy_data = phy_dev->hw_data;
> > +	unsigned int refsel;
> > +	unsigned long rate;
> > +	int ret;
> > +
> > +	/* Check if a phy is already init or clkpll in use */
> > +	if (atomic_inc_return(&phy_dev->en_refcnt) > 1)
> > +		return 0;
> > +
> > +	rate = clk_get_rate(phy_dev->phyref);
> > +	if (rate == 19200000)
> > +		refsel = 0;
> > +	else if (rate == 20000000)
> > +		refsel = 1;
> > +	else if (rate == 24000000)
> > +		refsel = 2;
> > +	else
> > +		return -EINVAL;
>
> Would a switch not be better here?

I think here it is a matter of taste, switch would use a few more lines, 
so would a look up table. If you insist on a switch, I'll turn this into 
a switch.

[...]

> > +static int stm32_usb2phy2_init(struct phy *phy)
> > +{
> > +	struct stm32_usb2phy *phy_dev = phy_get_drvdata(phy);
> > +	int ret;
> > +
> > +	ret = stm32_usb2phy_enable(phy_dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (phy_dev->mode != PHY_MODE_INVALID) {
> > +		ret = stm32_usb2phy_set_mode(phy, phy_dev->mode, USB_ROLE_NONE);
> > +		if (ret) {
> > +			stm32_usb2phy_disable(phy_dev);
> > +			return ret;
> > +		}
> > +	}
>
> so what is the diff b/w these two phys that we need to check invalid
> only here?
For this particular case -- One PHY is connected to host-only EHCI/OHCI 
controller, so it does not do mode set. The other PHY is connected to 
DWC3 DRD controller which can do either Host/Peripheral and does mode 
set, but that can only be done if a valid mode is already configured. 
Hence the conditional here.

[...]