Re: [PATCH 2/4] usb: ohci-at91: Enable OHCI functionality and register into DM

[<Sergiu.Moga@microchip.com>]

On 03.01.2023 16:34, Marek Vasut wrote:
> On 1/3/23 13:11, Sergiu.Moga@microchip.com wrote:
>> On 03.01.2023 01:26, Marek Vasut wrote:
>>> On 12/23/22 13:34, Sergiu Moga wrote:
>>>
>>> [...]
>>>
>>>> diff --git a/drivers/usb/host/ohci-at91.c 
>>>> b/drivers/usb/host/ohci-at91.c
>>>> index 9b955c1bd6..9ae55c6e5d 100644
>>>> --- a/drivers/usb/host/ohci-at91.c
>>>> +++ b/drivers/usb/host/ohci-at91.c
>>>> @@ -5,6 +5,9 @@
>>>>     */
>>>>
>>>>    #include <common.h>
>>>> +
>>>> +#if !(CONFIG_IS_ENABLED(DM_USB))
>>>> +
>>>>    #include <asm/arch/clk.h>
>>>>
>>>>    int usb_cpu_init(void)
>>>> @@ -62,3 +65,183 @@ int usb_cpu_init_fail(void)
>>>>    {
>>>>        return usb_cpu_stop();
>>>>    }
>>>
>>> Would it be possible to just remove the non-DM functionality ?
>>>
>>
>> Unfortunately, the other older boards would not build and work anymore
>> if I were to remove this.
> 
> Any chance they can be converted to DM ?
> 

Not at the moment, no.

>>>> +#elif CONFIG_IS_ENABLED(DM_GPIO)
>>>
>>> I think you want plain '#else' here, and then make the driver select
>>> DM_GPIO in case DM_USB is enabled in Kconfig entry.
>>>
>>>> +#include <clk.h>
>>>> +#include <dm.h>
>>>> +#include <asm/gpio.h>
>>>> +#include <usb.h>
>>>> +#include "ohci.h"
>>>> +
>>>> +#define AT91_MAX_USBH_PORTS        3
>>>>
>>>> +#define at91_for_each_port(index)    \
>>>> +             for ((index) = 0; (index) < AT91_MAX_USBH_PORTS; 
>>>> (index)++)
>>>> +
>>>> +struct at91_usbh_data {
>>>> +     enum usb_init_type init_type;
>>>> +     struct gpio_desc vbus_pin[AT91_MAX_USBH_PORTS];
>>>
>>> drivers/usb/host/ehci-generic.c:        ret =
>>> device_get_supply_regulator(dev, "vbus-supply",
>>>
>>> I wonder if we can instead use regulator for vbus-supply here too ?
>>>
>>
>> I would rather keep the same DT properties as those in Linux and have
>> them be in concordance with our USB DT binding, which specifies the
>> atmel,vbus-gpio property and no vbus-supply.
> 
> Oh well, so yes, we're stuck with this GPIO stuff.
> 
> [...]

Unfortunately yes...