[PATCH v2 12/32] board: dragonboard410c: upstream DT compat

[Caleb Connolly]

On 02/01/2024 12:54, Sumit Garg wrote:
> On Tue, 2 Jan 2024 at 14:43, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>>
>>
>>
>> On 26/12/2023 10:30, Sumit Garg wrote:
>>> On Fri, 22 Dec 2023 at 21:43, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>>>>
>>>> Hi Sumit,
>>>>
>>>> [...]
>>>>>> -       if (init == USB_INIT_HOST) {
>>>>>> -               /* Start USB Hub */
>>>>>> -               dm_gpio_set_dir_flags(&hub_reset,
>>>>>> -                                     GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
>>>>>> -               mdelay(100);
>>>>>> -               /* Switch usb to host connectors */
>>>>>> -               dm_gpio_set_dir_flags(&usb_sel,
>>>>>> -                                     GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
>>>>>> -               mdelay(100);
>>>>>> -       } else { /* Device */
>>>>>> -               /* Disable hub */
>>>>>> -               dm_gpio_set_dir_flags(&hub_reset, GPIOD_IS_OUT);
>>>>>> -               /* Switch back to device connector */
>>>>>> -               dm_gpio_set_dir_flags(&usb_sel, GPIOD_IS_OUT);
>>>>>> +       /* Select "default" or "device" pinctrl */
>>>>>> +       switch (init) {
>>>>>> +       case USB_INIT_HOST:
>>>>>> +               pinctrl_select_state(usb, "default");
>>>>>> +               break;
>>>>>> +       case USB_INIT_DEVICE:
>>>>>> +               pinctrl_select_state(usb, "device");
>>>>>> +               break;
>>>>>> +       default:
>>>>>> +               debug("Unknown usb_init_type %d\n", init);
>>>>>> +               break;
>>>>>
>>>>> Can this pinctrl configuration move to the corresponding USB driver instead?
>>>>
>>>> Possibly, this is definitely something where DT is currently lacking,
>>>> similar discussions in Linux resulted in the USB onboard_hub driver, it
>>>> would be nice to add support for that U-Boot at some point.
>>>>
>>>> I don't think putting the pinctrl code directly into the USB driver is
>>>> the right way to go, as it definitely wouldn't be accepted in upstream
>>>> DT bindings.
>>>
>>> As discussed in the other thread,
>>> arch/arm/mach-snapdragon/board_apq8016.c would be a better place for
>>> the time being.
>>
>> I would instead prefer this go in mach-snapdragon/board.c and include a
>> conditional to check for a named pinctrl on the USB device, using
>> "uclass_find_device_by_seq()" to fetch the device would then make this
>> properly generic.
> 
> Yes, that can be a further step and I don't expect that to be part of
> this series. But since this patch is moving stuff, we should move SoC
> specific bits to mach-snapdragon/board_apq8016.c rather than its
> opposite to the db410c specific file.

The reason I moved this board code to the board directory is because 
with this series I'm enforcing that all mach-snapdragon code be generic 
and built for all Qualcomm platforms. I also did it because while I 
don't want to break support for db410c, I don't plan to fix it, so this 
works as a temporary solution until someone who does care about the 
board (like you) wants to fix it up.
> 
>>
>> This is really not relevant to this patch series, but is a change that
>> makes sense when introducing support for new boards, let's limit
>> discussion to the other thread [1].
> 
> But why? The USB stuff we are discussing here is relevant to existing
> board support, right?

Right, and the existing board is supported with this series. You're 
suggesting that I spend significantly more time on this series to make 
supporting your board (which is NOT yet upstream) easier.

My cover letter is very explicit that this series lays the groundwork 
such that all future Qualcomm boards can be supported in a fully generic 
way, with no additional board code. I have support for 3 new Qualcomm 
SoCs and ~6 devices based on this series, and none of that requires 
adding new board code because I took the time to come up with proper 
solutions by fixing drivers or DT.

I carved out exceptions for the db410c and db820c because I cannot test 
all the usecases they support, and finding proper solutions to allow for 
dropping their board code is a whole lot of work for boards that - 
unfortunately - I'm not able to prioritize.

The fact that you are working on a board based on db410c is great, it 
means that you can put the effort in to find a proper generic solution, 
and ideally we can drop board/qualcomm/dragonboard410c entirely!

I'm *more than* happy to discuss ideas and figure out the right 
solutions to the remaining board code with you. There are absolutely 
areas where U-Boot is missing flexibility, and I see this as a good 
opportunity for us to fix some of that.

It's clear from other threads that restricting the amount of mach/board 
code in U-Boot is a good way forward, and I would very much like for 
Qualcomm to lead the way here. There is no good reason why we shouldn't 
be able to have a single U-Boot binary for all of our supported boards, 
and it makes testing *so* much easier.

> 
> -Sumit
> 
>>
>> [1]:
>> https://lore.kernel.org/u-boot/f805111d-106e-4904-ae9d-e63596f7db4e@linaro.org/
>>>
>>> -Sumit
>>>
>>>>>
>>>>> -Sumit
>>>>>
>>>> --
>>>> // Caleb (they/them)
>>
>> --
>> // Caleb (they/them)

-- 
// Caleb (they/them)