On Tue, 2 Jan 2024 at 14:43, Caleb Connolly <caleb.conno...@linaro.org> wrote:
>
>
>
> On 26/12/2023 10:30, Sumit Garg wrote:
> > On Fri, 22 Dec 2023 at 21:43, Caleb Connolly <caleb.conno...@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.
>
> 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?
-Sumit
>
> [1]:
> https://lore.kernel.org/u-boot/f805111d-106e-4904-ae9d-e63596f7d...@linaro.org/
> >
> > -Sumit
> >
> >>>
> >>> -Sumit
> >>>
> >> --
> >> // Caleb (they/them)
>
> --
> // Caleb (they/them)
