Re: [PATCH 4/4] usb: ohci-at91: Add USB PHY functionality

[<Sergiu.Moga@microchip.com>]

On 03.01.2023 01:33, Marek Vasut wrote:
> On 12/23/22 13:34, Sergiu Moga wrote:
>> Add the ability to enable/disable whatever USB PHY's are
>> passed to the AT91 OHCI driver through DT.
>>
>> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
>> Tested-by: Mihai Sain <mihai.sain@microchip.com>
>> ---
>>   drivers/usb/host/ohci-at91.c | 31 +++++++++++++++++++++++++++++++
>>   1 file changed, 31 insertions(+)
>>
>> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
>> index 5cf8f283e5..217f31b402 100644
>> --- a/drivers/usb/host/ohci-at91.c
>> +++ b/drivers/usb/host/ohci-at91.c
>> @@ -74,6 +74,10 @@ int usb_cpu_init_fail(void)
>>   #include <usb.h>
>>   #include "ohci.h"
>>
>> +#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB)
>> +#include <generic-phy.h>
>> +#endif
>> +
>>   #define AT91_MAX_USBH_PORTS        3
>>
>>   #define at91_for_each_port(index)   \
>> @@ -91,6 +95,10 @@ struct ohci_at91_priv {
>>       struct clk *fclk;
>>       struct clk *hclk;
>>       bool clocked;
>> +
>> +#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB)
>> +     struct phy phy[AT91_MAX_USBH_PORTS];
>> +#endif
>>   };
>>
>>   static void at91_start_clock(struct ohci_at91_priv *ohci_at91)
>> @@ -98,6 +106,13 @@ static void at91_start_clock(struct ohci_at91_priv 
>> *ohci_at91)
>>       if (ohci_at91->clocked)
>>               return;
>>
>> +#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB)
> 
> Use plain:
> 
> if (CONFIG_IS_ENABLED(...)) { ... }
> 
> instead of the #if ... , the compiler would optimize the code out 
> correctly.
> 


The build system complains that the generic_phy_* methods are not 
present and, if possible, I would like not to have to enable the PHY 
related CONFIGs on the boards that do not need it only to avoid these 
warnings.


>> +     int i;
>> +
>> +     at91_for_each_port(i)
>> +             generic_phy_power_on(&ohci_at91->phy[i]);
> 
> Look at generic_phy_get_bulk() and generic_phy_power_on_bulk() and co.
> those should get rid of this loop altogether.
> 
> [...]