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

[Marek Vasut <marex@nabladev.com>]

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.

[...]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy