lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Mon, 16 Jan 2017 13:41:28 -0200
From:   Bruno Herrera <bruherrera@...il.com>
To:     Alexandre Torgue <alexandre.torgue@...com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Russell King <linux@...linux.org.uk>,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [v2 2/3] ARM: dts: STM32 Add USB FS host mode support

On Mon, Jan 16, 2017 at 9:47 AM, Alexandre Torgue
<alexandre.torgue@...com> wrote:
>
>
> On 01/16/2017 11:26 AM, Bruno Herrera wrote:
>>
>> Hi Alex,
>>
>> On Mon, Jan 16, 2017 at 6:57 AM, Alexandre Torgue
>> <alexandre.torgue@...com> wrote:
>>>
>>> Hi Bruno,
>>>
>>> On 01/16/2017 03:09 AM, Bruno Herrera wrote:
>>>>
>>>>
>>>> This patch adds the USB pins and nodes for USB HS/FS cores working at FS
>>>> speed,
>>>> using embedded PHY.
>>>>
>>>> Signed-off-by: Bruno Herrera <bruherrera@...il.com>
>>>
>>>
>>>
>>> Sorry, but what is patch 1 & pacth 3 status ?
>>
>>
>> My bad, I'll add the status of the patch series version 3.
>>>
>>>
>>> For this one, can split it in 3 patches (one patch for SOC and one for
>>> each
>>> board) please.
>>>
>>
>> No problem.
>>>
>>>
>>>
>>>> ---
>>>>  arch/arm/boot/dts/stm32f429-disco.dts | 30
>>>> ++++++++++++++++++++++++++++++
>>>>  arch/arm/boot/dts/stm32f429.dtsi      | 35
>>>> ++++++++++++++++++++++++++++++++++-
>>>>  arch/arm/boot/dts/stm32f469-disco.dts | 30
>>>> ++++++++++++++++++++++++++++++
>>>>  3 files changed, 94 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/stm32f429-disco.dts
>>>> b/arch/arm/boot/dts/stm32f429-disco.dts
>>>> index 7d0415e..374c5ed 100644
>>>> --- a/arch/arm/boot/dts/stm32f429-disco.dts
>>>> +++ b/arch/arm/boot/dts/stm32f429-disco.dts
>>>> @@ -88,6 +88,16 @@
>>>>                         gpios = <&gpioa 0 0>;
>>>>                 };
>>>>         };
>>>> +
>>>> +       /* This turns on vbus for otg for host mode (dwc2) */
>>>> +       vcc5v_otg: vcc5v-otg-regulator {
>>>> +               compatible = "regulator-fixed";
>>>> +               gpio = <&gpioc 4 0>;
>>>> +               pinctrl-names = "default";
>>>> +               pinctrl-0 = <&usbotg_pwren_h>;
>>>> +               regulator-name = "vcc5_host1";
>>>> +               regulator-always-on;
>>>> +       };
>>>>  };
>>>>
>>>>  &clk_hse {
>>>> @@ -99,3 +109,23 @@
>>>>         pinctrl-names = "default";
>>>>         status = "okay";
>>>>  };
>>>> +
>>>> +&usbotg_hs {
>>>> +       compatible = "st,stm32-fsotg", "snps,dwc2";
>>>> +       dr_mode = "host";
>>>> +       pinctrl-0 = <&usbotg_fs_pins_b>;
>>>> +       pinctrl-names = "default";
>>>> +       status = "okay";
>>>> +};
>>>> +
>>>> +&pinctrl {
>>>> +       usb-host {
>>>> +               usbotg_pwren_h: usbotg-pwren-h {
>>>> +                       pins {
>>>> +                               pinmux = <STM32F429_PC4_FUNC_GPIO>;
>>>> +                               bias-disable;
>>>> +                               drive-push-pull;
>>>> +                       };
>>>> +               };
>>>> +       };
>>>> +};
>>>
>>>
>>>
>>> Pinctrl muxing has to be defined/declared in stm32f429.dtsi
>>>
>> This is board specific logic and it vary from board to board, should
>> it be defined here?
>
>
> Pinmuxing definition is a SOC part (as it is a possibility offered by SOC).
> Pinmuxing choice is board specific.
>
> Regarding your code, it should not boot. Ex for disco:
>
>  +               gpio = <&gpiob 2 0>;
>>>> +               pinctrl-names = "default";
>>>> +               pinctrl-0 = <&usbotg_pwren_h>;
>
> +
>
>   usb-host {
>>>> +               usbotg_pwren_h: usbotg-pwren-h {
>>>> +                       pins {
>>>> +                               pinmux = <STM32F429_PB2_FUNC_GPIO>;
>
> Indeed, you are declaring two time the pin PB2 (one time through pinctrl and
> one other time through gpiolib). in strict mode you can't request 2 times
> the same Pin.
> I assume that your driver want controls this GPIO (request/set direction /
> set, get value ...). in this case you only need to declare this part:
>
> gpio = <&gpiob 2 0>;
>
> The GPIO lib will deal with pinctrl framework for you.
> And in this case, yes gpio declaration is board specific so this part will
> be in board file.
>
> Let me know, if I'm not enough clear.

Thats very clear! Thanks for bringing.
I'll retest without the pinctrl.

br,
Bruno

>
> Regards
> Alex
>
>
>
>
>
>>>
>>>
>>>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>>>> b/arch/arm/boot/dts/stm32f429.dtsi
>>>> index e4dae0e..bc07aa8 100644
>>>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>>>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>>>> @@ -206,7 +206,7 @@
>>>>                         reg = <0x40007000 0x400>;
>>>>                 };
>>>>
>>>> -               pin-controller {
>>>> +               pinctrl: pin-controller {
>>>>                         #address-cells = <1>;
>>>>                         #size-cells = <1>;
>>>>                         compatible = "st,stm32f429-pinctrl";
>>>> @@ -316,6 +316,30 @@
>>>>                                 };
>>>>                         };
>>>>
>>>> +                       usbotg_fs_pins_a: usbotg_fs@0 {
>>>> +                               pins {
>>>> +                                       pinmux =
>>>> <STM32F429_PA10_FUNC_OTG_FS_ID>,
>>>> +
>>>> <STM32F429_PA11_FUNC_OTG_FS_DM>,
>>>> +
>>>> <STM32F429_PA12_FUNC_OTG_FS_DP>;
>>>> +                                       bias-disable;
>>>> +                                       drive-push-pull;
>>>> +                                       slew-rate = <2>;
>>>> +                               };
>>>> +                       };
>>>> +
>>>> +                       usbotg_fs_pins_b: usbotg_fs@1 {
>>>> +                               pins {
>>>> +                                       pinmux =
>>>> <STM32F429_PB12_FUNC_OTG_HS_ID>,
>>>> +
>>>> <STM32F429_PB14_FUNC_OTG_HS_DM>,
>>>> +
>>>> <STM32F429_PB15_FUNC_OTG_HS_DP>;
>>>> +                                       bias-disable;
>>>> +                                       drive-push-pull;
>>>> +                                       slew-rate = <2>;
>>>> +                               };
>>>> +                       };
>>>> +
>>>> +
>>>> +
>>>>                         usbotg_hs_pins_a: usbotg_hs@0 {
>>>>                                 pins {
>>>>                                         pinmux =
>>>> <STM32F429_PH4_FUNC_OTG_HS_ULPI_NXT>,
>>>> @@ -420,6 +444,15 @@
>>>>                         status = "disabled";
>>>>                 };
>>>>
>>>> +               usbotg_fs: usb@...00000 {
>>>> +                       compatible = "st,stm32f4xx-fsotg", "snps,dwc2";
>>>> +                       reg = <0x50000000 0x40000>;
>>>> +                       interrupts = <67>;
>>>> +                       clocks = <&rcc 0 39>;
>>>> +                       clock-names = "otg";
>>>> +                       status = "disabled";
>>>> +               };
>>>> +
>>>>                 rng: rng@...60800 {
>>>>                         compatible = "st,stm32-rng";
>>>>                         reg = <0x50060800 0x400>;
>>>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts
>>>> b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> index 8877c00..8ae6763 100644
>>>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>>>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> @@ -68,6 +68,17 @@
>>>>         soc {
>>>>                 dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>>         };
>>>> +
>>>> +       /* This turns on vbus for otg for host mode (dwc2) */
>>>> +       vcc5v_otg: vcc5v-otg-regulator {
>>>> +               compatible = "regulator-fixed";
>>>> +               enable-active-high;
>>>> +               gpio = <&gpiob 2 0>;
>>>> +               pinctrl-names = "default";
>>>> +               pinctrl-0 = <&usbotg_pwren_h>;
>>>> +               regulator-name = "vcc5_host1";
>>>> +               regulator-always-on;
>>>> +       };
>>>>  };
>>>>
>>>>  &rcc {
>>>> @@ -81,3 +92,22 @@
>>>>  &usart3 {
>>>>         status = "okay";
>>>>  };
>>>> +
>>>> +&usbotg_fs {
>>>> +       dr_mode = "host";
>>>> +       pinctrl-0 = <&usbotg_fs_pins_a>;
>>>> +       pinctrl-names = "default";
>>>> +       status = "okay";
>>>> +};
>>>> +
>>>> +&pinctrl {
>>>> +       usb-host {
>>>> +               usbotg_pwren_h: usbotg-pwren-h {
>>>> +                       pins {
>>>> +                               pinmux = <STM32F429_PB2_FUNC_GPIO>;
>>>> +                               bias-disable;
>>>> +                               drive-push-pull;
>>>> +                       };
>>>> +               };
>>>> +       };
>>>> +};
>>>
>>>
>>> Same. Note that if you have 2 configuration for one feature (like it is
>>> here
>>> for "usbotg_pwren_h"), you could index it. Not that I'm adding a
>>> dedidacted
>>> pinctroller for stm32f469.
>>>
>> Sorry, but I dont know what you mean by index here.
>> The usbotg_pwren_h (VBUS ENABLE) is attached in different port/pins
>> for each board.
>>
>> Br.,
>>
>>
>>> Br
>>> Alex
>>>>
>>>>
>>>>
>>>
>>>
>

Powered by blists - more mailing lists