Re: [PATCH v4 4/4] arm64: dts: add description for solidrun cn9131 solidwan board

[Josua Mayer <josua@solid-run.com>]

Am 02.05.24 um 17:20 schrieb Rob Herring:
> On Thu, May 2, 2024 at 7:32 AM Josua Mayer <josua@solid-run.com> wrote:
>> Add description for the SolidRun CN9131 SolidWAN, based on CN9130 SoM
>> with an extra communication  processor on the carrier board.
>>
>> This board differentiates itself from CN9130 Clearfog by providing
>> additional SoC native network interfaces and pci buses:
>> 2x 10Gbps SFP+
>> 4x 1Gbps RJ45
>> 1x miniPCI-E
>> 1x m.2 b-key with sata, usb-2.0 and usb-3.0
>> 1x m.2 m-key with pcie and usb-2.0
>> 1x m.2 b-key with pcie, usb-2.0, usb-3.0 and 2x sim slots
>> 1x mpcie with pcie only
>> 2x type-a usb-2.0/3.0
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
cut
>> +       /* Type-A port on J53 */
>> +       reg_usb_a_vbus0: regulator-usb-a-vbus0 {
>> +               compatible = "regulator-fixed";
>> +               pinctrl-0 = <&cp0_reg_usb_a_vbus0_pins>;
>> +               pinctrl-names = "default";
>> +               regulator-name = "vbus0";
>> +               regulator-min-microvolt = <5000000>;
>> +               regulator-max-microvolt = <5000000>;
>> +               regulator-oc-protection-microamp = <1000000>;
>> +               gpio = <&cp0_gpio1 27 GPIO_ACTIVE_HIGH>;
> "gpio" is deprecated.
Thank you for pointing this out.
I did actually think about it while reading the bindings,
and failed to figure this out. "gpio" occured to me as more correct:

  gpio:
    description: gpio to use for enable control
    maxItems: 1

  gpios:
    maxItems: 1

I could of course have checked git log, and found
regulator: dt-bindings: fixed-regulator:allow gpios property
by Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
with the explanation:

"'gpios' is in general preferred"

>
>> +               enable-active-high;
>> +               regulator-always-on;
>> +       };
>> +
>> +       reg_usb_a_vbus1: regulator-usb-a-vbus1 {
>> +               compatible = "regulator-fixed";
>> +               pinctrl-0 = <&cp0_reg_usb_a_vbus1_pins>;
>> +               pinctrl-names = "default";
>> +               regulator-name = "vbus1";
>> +               regulator-min-microvolt = <5000000>;
>> +               regulator-max-microvolt = <5000000>;
>> +               regulator-oc-protection-microamp = <1000000>;
>> +               gpio = <&cp0_gpio1 28 GPIO_ACTIVE_HIGH>;
>> +               enable-active-high;
>> +               regulator-always-on;
>> +       };
>> +
>> +       sfp0: sfp-0 {
>> +               compatible = "sff,sfp";
>> +               pinctrl-0 = <&cp0_sfp0_pins>;
>> +               pinctrl-names = "default";
>> +               i2c-bus = <&cp0_i2c1>;
>> +               los-gpio = <&cp0_gpio2 2 GPIO_ACTIVE_HIGH>;
>> +               mod-def0-gpio = <&cp0_gpio2 0 GPIO_ACTIVE_LOW>;
>> +               tx-disable-gpio = <&cp0_gpio2 1 GPIO_ACTIVE_HIGH>;
>> +               tx-fault-gpio = <&cp0_gpio1 31 GPIO_ACTIVE_HIGH>;
> As is "-gpio" suffix.  These are all pointed out with 'dtbs_check'
> which I sent a report on v3. I haven't checked what else from that you
> ignored... I don't expect warnings inherited from the SoC .dtsi to be
> fixed in this series, but certainly the board level ones. Yes, it's
> hard to pick out those, but that's the Marvell folks fault for not
> fixing SoC level warnings.
You are right, they were clearly pointed out by your bot
and I missed them. They will be corrected in v5.


Thanks
- Josua Mayer

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel