Re: [PATCH v7 4/4] arm64: dts: ti: hummingboard-t: add overlays for m.2 pci-e and usb-3

Re: [PATCH v7 4/4] arm64: dts: ti: hummingboard-t: add overlays for m.2 pci-e and usb-3

[Geert Uytterhoeven]

Hi Josua,

On Mon, Feb 19, 2024 at 4:05 PM Josua Mayer <josua@solid-run.com> wrote:
> HummingBoard-T features two M.2 connectors labeled "M1" and "M2".
> The single SerDes lane of the SoC can be routed to either M1 pci-e
> signals, or M2 usb-3 signals by a gpio-controlled mux.
>
> Add overlays for each configuration.
>
> Signed-off-by: Josua Mayer <josua@solid-run.com>

Thanks for your patch, which is now commit bbef42084cc170cb ("arm64:
dts: ti: hummingboard-t: add overlays for m.2 pci-e and usb-3") in v6.9.

> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-am642-hummingboard-t-usb3.dtso
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2023 Josua Mayer <josua@solid-run.com>
> + *
> + * Overlay for SolidRun AM642 HummingBoard-T to enable USB-3.1.
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +#include <dt-bindings/phy/phy.h>
> +
> +#include "k3-serdes.h"
> +
> +&serdes0 {
> +       #address-cells = <1>;
> +       #size-cells = <0>;
> +
> +       serdes0_link: phy@0 {
> +               reg = <0>;
> +               cdns,num-lanes = <1>;
> +               cdns,phy-type = <PHY_TYPE_USB3>;
> +               #phy-cells = <0>;
> +               resets = <&serdes_wiz0 1>;
> +       };
> +};
> +
> +&serdes_ln_ctrl {
> +       idle-states = <AM64_SERDES0_LANE0_USB>;
> +};
> +
> +&serdes_mux {
> +       idle-state = <0>;
> +};
> +
> +&usbss0 {
> +       /delete-property/ ti,usb2-only;

/delete-property/ (and /delete-node/) to delete something in the base DTS
does not work.

> +};
> +
> +&usb0 {
> +       maximum-speed = "super-speed";
> +       phys = <&serdes0_link>;
> +       phy-names = "cdns3,usb3-phy";
> +};

You can run

    dtx_diff --color arch/arm64/boot/dts/ti/k3-am642-hummingboard-t{,-usb3}.dtb

to verify.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v7 4/4] arm64: dts: ti: hummingboard-t: add overlays for m.2 pci-e and usb-3

[Vignesh Raghavendra]

On 25/10/24 19:27, Geert Uytterhoeven wrote:
> Hi Josua,
> 
> On Mon, Feb 19, 2024 at 4:05 PM Josua Mayer <josua@solid-run.com> wrote:
>> HummingBoard-T features two M.2 connectors labeled "M1" and "M2".
>> The single SerDes lane of the SoC can be routed to either M1 pci-e
>> signals, or M2 usb-3 signals by a gpio-controlled mux.
>>
>> Add overlays for each configuration.
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
> 
> Thanks for your patch, which is now commit bbef42084cc170cb ("arm64:
> dts: ti: hummingboard-t: add overlays for m.2 pci-e and usb-3") in v6.9.
> 
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/ti/k3-am642-hummingboard-t-usb3.dtso
>> @@ -0,0 +1,44 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2023 Josua Mayer <josua@solid-run.com>
>> + *
>> + * Overlay for SolidRun AM642 HummingBoard-T to enable USB-3.1.
>> + */
>> +
>> +/dts-v1/;
>> +/plugin/;
>> +
>> +#include <dt-bindings/phy/phy.h>
>> +
>> +#include "k3-serdes.h"
>> +
>> +&serdes0 {
>> +       #address-cells = <1>;
>> +       #size-cells = <0>;
>> +
>> +       serdes0_link: phy@0 {
>> +               reg = <0>;
>> +               cdns,num-lanes = <1>;
>> +               cdns,phy-type = <PHY_TYPE_USB3>;
>> +               #phy-cells = <0>;
>> +               resets = <&serdes_wiz0 1>;
>> +       };
>> +};
>> +
>> +&serdes_ln_ctrl {
>> +       idle-states = <AM64_SERDES0_LANE0_USB>;
>> +};
>> +
>> +&serdes_mux {
>> +       idle-state = <0>;
>> +};
>> +
>> +&usbss0 {
>> +       /delete-property/ ti,usb2-only;
> 
> /delete-property/ (and /delete-node/) to delete something in the base DTS
> does not work.

Geert,

Thanks for the catching

Joshua,

This overlay is pretty useless in light of above issue.  I intend to
just drop this file unless you convince me otherwise?


-- 
Regards
Vignesh

Re: [PATCH v7 4/4] arm64: dts: ti: hummingboard-t: add overlays for m.2 pci-e and usb-3

[Josua Mayer]

Hi Geert, Vignesh,

Am 28.10.24 um 16:31 schrieb Vignesh Raghavendra:
>
> On 25/10/24 19:27, Geert Uytterhoeven wrote:
>> Hi Josua,
>>
>> On Mon, Feb 19, 2024 at 4:05 PM Josua Mayer <josua@solid-run.com> wrote:
>>> HummingBoard-T features two M.2 connectors labeled "M1" and "M2".
>>> The single SerDes lane of the SoC can be routed to either M1 pci-e
>>> signals, or M2 usb-3 signals by a gpio-controlled mux.
>>>
>>> Add overlays for each configuration.
>>>
>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>> Thanks for your patch, which is now commit bbef42084cc170cb ("arm64:
>> dts: ti: hummingboard-t: add overlays for m.2 pci-e and usb-3") in v6.9.
>>
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/ti/k3-am642-hummingboard-t-usb3.dtso
>>> @@ -0,0 +1,44 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Copyright (C) 2023 Josua Mayer <josua@solid-run.com>
>>> + *
>>> + * Overlay for SolidRun AM642 HummingBoard-T to enable USB-3.1.
>>> + */
>>> +
>>> +/dts-v1/;
>>> +/plugin/;
>>> +
>>> +#include <dt-bindings/phy/phy.h>
>>> +
>>> +#include "k3-serdes.h"
>>> +
>>> +&serdes0 {
>>> +       #address-cells = <1>;
>>> +       #size-cells = <0>;
>>> +
>>> +       serdes0_link: phy@0 {
>>> +               reg = <0>;
>>> +               cdns,num-lanes = <1>;
>>> +               cdns,phy-type = <PHY_TYPE_USB3>;
>>> +               #phy-cells = <0>;
>>> +               resets = <&serdes_wiz0 1>;
>>> +       };
>>> +};
>>> +
>>> +&serdes_ln_ctrl {
>>> +       idle-states = <AM64_SERDES0_LANE0_USB>;
>>> +};
>>> +
>>> +&serdes_mux {
>>> +       idle-state = <0>;
>>> +};
>>> +
>>> +&usbss0 {
>>> +       /delete-property/ ti,usb2-only;
>> /delete-property/ (and /delete-node/) to delete something in the base DTS
>> does not work.

My understanding is that flags are equivalent to boolean, i.e:

ti,usb2-only = <true>;
ti,usb2-only;

are equivalent.

If so, can we assign <false> within the overlay?

> Geert,
>
> Thanks for the catching
Excellent spotting indeed.
I noticed this in passing about a week ago when I requested
Debian enable the necessary drivers in their distro kernel
(without understanding the root cause).
>
> Joshua,
>
> This overlay is pretty useless in light of above issue.  I intend to
> just drop this file unless you convince me otherwise?
>
I would really prefer to fix it, or somehow replace with equivalent functionality.

My original proposal was having the board a dtsi, and the pci and usb3
variants as standalone dts.

Re: [PATCH v7 4/4] arm64: dts: ti: hummingboard-t: add overlays for m.2 pci-e and usb-3

[Geert Uytterhoeven]

Hi Josua,

On Mon, Oct 28, 2024 at 6:19 PM Josua Mayer <josua@solid-run.com> wrote:
> Am 28.10.24 um 16:31 schrieb Vignesh Raghavendra:
> > On 25/10/24 19:27, Geert Uytterhoeven wrote:
> >> On Mon, Feb 19, 2024 at 4:05 PM Josua Mayer <josua@solid-run.com> wrote:
> >>> HummingBoard-T features two M.2 connectors labeled "M1" and "M2".
> >>> The single SerDes lane of the SoC can be routed to either M1 pci-e
> >>> signals, or M2 usb-3 signals by a gpio-controlled mux.
> >>>
> >>> Add overlays for each configuration.
> >>>
> >>> Signed-off-by: Josua Mayer <josua@solid-run.com>
> >> Thanks for your patch, which is now commit bbef42084cc170cb ("arm64:
> >> dts: ti: hummingboard-t: add overlays for m.2 pci-e and usb-3") in v6.9.
> >>
> >>> --- /dev/null
> >>> +++ b/arch/arm64/boot/dts/ti/k3-am642-hummingboard-t-usb3.dtso
> >>> @@ -0,0 +1,44 @@
> >>> +// SPDX-License-Identifier: GPL-2.0+
> >>> +/*
> >>> + * Copyright (C) 2023 Josua Mayer <josua@solid-run.com>
> >>> + *
> >>> + * Overlay for SolidRun AM642 HummingBoard-T to enable USB-3.1.
> >>> + */
> >>> +
> >>> +/dts-v1/;
> >>> +/plugin/;
> >>> +
> >>> +#include <dt-bindings/phy/phy.h>
> >>> +
> >>> +#include "k3-serdes.h"
> >>> +
> >>> +&serdes0 {
> >>> +       #address-cells = <1>;
> >>> +       #size-cells = <0>;
> >>> +
> >>> +       serdes0_link: phy@0 {
> >>> +               reg = <0>;
> >>> +               cdns,num-lanes = <1>;
> >>> +               cdns,phy-type = <PHY_TYPE_USB3>;
> >>> +               #phy-cells = <0>;
> >>> +               resets = <&serdes_wiz0 1>;
> >>> +       };
> >>> +};
> >>> +
> >>> +&serdes_ln_ctrl {
> >>> +       idle-states = <AM64_SERDES0_LANE0_USB>;
> >>> +};
> >>> +
> >>> +&serdes_mux {
> >>> +       idle-state = <0>;
> >>> +};
> >>> +
> >>> +&usbss0 {
> >>> +       /delete-property/ ti,usb2-only;
> >> /delete-property/ (and /delete-node/) to delete something in the base DTS
> >> does not work.
>
> My understanding is that flags are equivalent to boolean, i.e:
>
> ti,usb2-only = <true>;
> ti,usb2-only;
>
> are equivalent.
>
> If so, can we assign <false> within the overlay?

Unfortunately not. My first thought was "it depends on the actual code
in the driver", but that turns out to be wrong:

    static inline bool of_property_read_bool(const struct device_node *np,
                                             const char *propname)
    {
            const struct property *prop = of_find_property(np, propname, NULL);

            return prop ? true : false;
    }

    static inline bool of_property_present(const struct device_node
*np, const char *propname)
    {
            return of_property_read_bool(np, propname);
    }

So both methods just check if the property is present, and do not use
its value, when present (i.e. the former does not "read" the bool).

drivers/usb/cdns3/cdns3-ti.c uses device_property_read_bool:

    static inline bool device_property_read_bool(const struct device *dev,
                                                 const char *propname)
    {
            return device_property_present(dev, propname);
    }

so (at least for DT) that should map to the above.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v7 4/4] arm64: dts: ti: hummingboard-t: add overlays for m.2 pci-e and usb-3

[Vignesh Raghavendra]

On 28/10/24 22:49, Josua Mayer wrote:
> Hi Geert, Vignesh,
> 
> Am 28.10.24 um 16:31 schrieb Vignesh Raghavendra:
>>
>> On 25/10/24 19:27, Geert Uytterhoeven wrote:
>>> Hi Josua,
>>>
>>> On Mon, Feb 19, 2024 at 4:05 PM Josua Mayer <josua@solid-run.com> wrote:
>>>> HummingBoard-T features two M.2 connectors labeled "M1" and "M2".
>>>> The single SerDes lane of the SoC can be routed to either M1 pci-e
>>>> signals, or M2 usb-3 signals by a gpio-controlled mux.
>>>>
>>>> Add overlays for each configuration.
>>>>
>>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>>> Thanks for your patch, which is now commit bbef42084cc170cb ("arm64:
>>> dts: ti: hummingboard-t: add overlays for m.2 pci-e and usb-3") in v6.9.
>>>
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/ti/k3-am642-hummingboard-t-usb3.dtso
>>>> @@ -0,0 +1,44 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>> +/*
>>>> + * Copyright (C) 2023 Josua Mayer <josua@solid-run.com>
>>>> + *
>>>> + * Overlay for SolidRun AM642 HummingBoard-T to enable USB-3.1.
>>>> + */
>>>> +
>>>> +/dts-v1/;
>>>> +/plugin/;
>>>> +
>>>> +#include <dt-bindings/phy/phy.h>
>>>> +
>>>> +#include "k3-serdes.h"
>>>> +
>>>> +&serdes0 {
>>>> +       #address-cells = <1>;
>>>> +       #size-cells = <0>;
>>>> +
>>>> +       serdes0_link: phy@0 {
>>>> +               reg = <0>;
>>>> +               cdns,num-lanes = <1>;
>>>> +               cdns,phy-type = <PHY_TYPE_USB3>;
>>>> +               #phy-cells = <0>;
>>>> +               resets = <&serdes_wiz0 1>;
>>>> +       };
>>>> +};
>>>> +
>>>> +&serdes_ln_ctrl {
>>>> +       idle-states = <AM64_SERDES0_LANE0_USB>;
>>>> +};
>>>> +
>>>> +&serdes_mux {
>>>> +       idle-state = <0>;
>>>> +};
>>>> +
>>>> +&usbss0 {
>>>> +       /delete-property/ ti,usb2-only;
>>> /delete-property/ (and /delete-node/) to delete something in the base DTS
>>> does not work.
> 
> My understanding is that flags are equivalent to boolean, i.e:
> 
> ti,usb2-only = <true>;
> ti,usb2-only;
> 
> are equivalent.
> 
> If so, can we assign <false> within the overlay?
> 
>> Geert,
>>
>> Thanks for the catching
> Excellent spotting indeed.
> I noticed this in passing about a week ago when I requested
> Debian enable the necessary drivers in their distro kernel
> (without understanding the root cause).
>>
>> Joshua,
>>
>> This overlay is pretty useless in light of above issue.  I intend to
>> just drop this file unless you convince me otherwise?
>>
> I would really prefer to fix it, or somehow replace with equivalent functionality.
> 
> My original proposal was having the board a dtsi, and the pci and usb3
> variants as standalone dts.
> 


Yeah, you would need a separate board dts for this profile. Please
propose a patch

-- 
Regards
Vignesh

Re: [PATCH v7 4/4] arm64: dts: ti: hummingboard-t: add overlays for m.2 pci-e and usb-3

[Josua Mayer]

Am 28.10.24 um 19:44 schrieb Vignesh Raghavendra:
>
> On 28/10/24 22:49, Josua Mayer wrote:
>> Hi Geert, Vignesh,
>>
>> Am 28.10.24 um 16:31 schrieb Vignesh Raghavendra:
>>> On 25/10/24 19:27, Geert Uytterhoeven wrote:
>>>> Hi Josua,
>>>>
>>>> On Mon, Feb 19, 2024 at 4:05 PM Josua Mayer <josua@solid-run.com> wrote:
>>>>> HummingBoard-T features two M.2 connectors labeled "M1" and "M2".
>>>>> The single SerDes lane of the SoC can be routed to either M1 pci-e
>>>>> signals, or M2 usb-3 signals by a gpio-controlled mux.
>>>>>
>>>>> Add overlays for each configuration.
>>>>>
>>>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>>>> Thanks for your patch, which is now commit bbef42084cc170cb ("arm64:
>>>> dts: ti: hummingboard-t: add overlays for m.2 pci-e and usb-3") in v6.9.
>>>>
>>>>> --- /dev/null
>>>>> +++ b/arch/arm64/boot/dts/ti/k3-am642-hummingboard-t-usb3.dtso
>>>>> @@ -0,0 +1,44 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>>> +/*
>>>>> + * Copyright (C) 2023 Josua Mayer <josua@solid-run.com>
>>>>> + *
>>>>> + * Overlay for SolidRun AM642 HummingBoard-T to enable USB-3.1.
>>>>> + */
>>>>> +
>>>>> +/dts-v1/;
>>>>> +/plugin/;
>>>>> +
>>>>> +#include <dt-bindings/phy/phy.h>
>>>>> +
>>>>> +#include "k3-serdes.h"
>>>>> +
>>>>> +&serdes0 {
>>>>> +       #address-cells = <1>;
>>>>> +       #size-cells = <0>;
>>>>> +
>>>>> +       serdes0_link: phy@0 {
>>>>> +               reg = <0>;
>>>>> +               cdns,num-lanes = <1>;
>>>>> +               cdns,phy-type = <PHY_TYPE_USB3>;
>>>>> +               #phy-cells = <0>;
>>>>> +               resets = <&serdes_wiz0 1>;
>>>>> +       };
>>>>> +};
>>>>> +
>>>>> +&serdes_ln_ctrl {
>>>>> +       idle-states = <AM64_SERDES0_LANE0_USB>;
>>>>> +};
>>>>> +
>>>>> +&serdes_mux {
>>>>> +       idle-state = <0>;
>>>>> +};
>>>>> +
>>>>> +&usbss0 {
>>>>> +       /delete-property/ ti,usb2-only;
>>>> /delete-property/ (and /delete-node/) to delete something in the base DTS
>>>> does not work.
>> My understanding is that flags are equivalent to boolean, i.e:
>>
>> ti,usb2-only = <true>;
>> ti,usb2-only;
>>
>> are equivalent.
>>
>> If so, can we assign <false> within the overlay?
>>
>>> Geert,
>>>
>>> Thanks for the catching
>> Excellent spotting indeed.
>> I noticed this in passing about a week ago when I requested
>> Debian enable the necessary drivers in their distro kernel
>> (without understanding the root cause).
>>> Joshua,
>>>
>>> This overlay is pretty useless in light of above issue.  I intend to
>>> just drop this file unless you convince me otherwise?
>>>
>> I would really prefer to fix it, or somehow replace with equivalent functionality.
>>
>> My original proposal was having the board a dtsi, and the pci and usb3
>> variants as standalone dts.
>>
>
> Yeah, you would need a separate board dts for this profile. Please
> propose a patch
Is there any chance of reassigning <false> and making an argument
that this should be fixed?

I find it frustrating that overlays can't override boolean properties,
and for consistency reasons I would otherwise change both
pcie and usb3 overlays to standalone dts.

Re: [PATCH v7 4/4] arm64: dts: ti: hummingboard-t: add overlays for m.2 pci-e and usb-3

[Geert Uytterhoeven]

Hi Josua,

On Wed, Oct 30, 2024 at 1:18 PM Josua Mayer <josua@solid-run.com> wrote:
> Am 28.10.24 um 19:44 schrieb Vignesh Raghavendra:
> > On 28/10/24 22:49, Josua Mayer wrote:
> >> Am 28.10.24 um 16:31 schrieb Vignesh Raghavendra:
> >>> On 25/10/24 19:27, Geert Uytterhoeven wrote:
> >>>> On Mon, Feb 19, 2024 at 4:05 PM Josua Mayer <josua@solid-run.com> wrote:
> >>>>> HummingBoard-T features two M.2 connectors labeled "M1" and "M2".
> >>>>> The single SerDes lane of the SoC can be routed to either M1 pci-e
> >>>>> signals, or M2 usb-3 signals by a gpio-controlled mux.
> >>>>>
> >>>>> Add overlays for each configuration.
> >>>>>
> >>>>> Signed-off-by: Josua Mayer <josua@solid-run.com>

> >>>>> --- /dev/null
> >>>>> +++ b/arch/arm64/boot/dts/ti/k3-am642-hummingboard-t-usb3.dtso

> >>>>> +&usbss0 {
> >>>>> +       /delete-property/ ti,usb2-only;
> >>>> /delete-property/ (and /delete-node/) to delete something in the base DTS
> >>>> does not work.
> >> My understanding is that flags are equivalent to boolean, i.e:
> >>
> >> ti,usb2-only = <true>;
> >> ti,usb2-only;
> >>
> >> are equivalent.
> >>
> >> If so, can we assign <false> within the overlay?

> Is there any chance of reassigning <false> and making an argument
> that this should be fixed?

In theory, it can be done, if (1) all code that checks boolean
properties would use of_property_read_bool() instead of
of_property_present(), and (2) of_property_read_bool() would be changed
to actually read the boolean value instead of just checking for the
presence of the property.  And of course we have to do that in all
software that uses DT (i.e. not just Linux).
See [1][2][3] below for caveats.

Using a similar solution for /delete-node/ is more complex, but still
feasible, by setting its "status" property to "disabled" . I think
that can be made to work if all DT core code that looks up nodes would
just ignore any node that has a disabled status. I.e. callers would
no longer see disabled nodes at all.

> I find it frustrating that overlays can't override boolean properties,
> and for consistency reasons I would otherwise change both
> pcie and usb3 overlays to standalone dts.

OK.

[1] The example in Documentation/devicetree/bindings/sound/rt5651.txt has:

        realtek,dmic-en = "true";
        realtek,in2-diff = "false";

    Obviously the second line doesn't really work with the current
    code, but fortunately there are no actual users of that (in
    upstream DTS).
    Note: "realtek,in2-diff" is a typo for "realtek,in2-differential".

[2] The example in Documentation/devicetree/bindings/sound/pcm3060.txt has:

        ti,out-single-ended = "true";

    Again, this is an open invitation for replacing "true" by "false".
    Fortunately there are no such users (in upstream DTS).

[3] arch/arm/boot/dts/ti/omap/am335x-baltos.dtsi has:

        gpmc,device-nand = "true";

    But "gpmc,device-nand" does not exist. Oh, it is under removal:
    https://lore.kernel.org/all/20241009-gpmc-omap-dtbx-v2-2-fc68124a090a@kernel.org/



Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v7 4/4] arm64: dts: ti: hummingboard-t: add overlays for m.2 pci-e and usb-3

[Josua Mayer]

Hi Geert,

Am 04.11.24 um 17:56 schrieb Geert Uytterhoeven:
> Hi Josua,
>
> On Wed, Oct 30, 2024 at 1:18 PM Josua Mayer <josua@solid-run.com> wrote:
>> Am 28.10.24 um 19:44 schrieb Vignesh Raghavendra:
>>> On 28/10/24 22:49, Josua Mayer wrote:
>>>> Am 28.10.24 um 16:31 schrieb Vignesh Raghavendra:
>>>>> On 25/10/24 19:27, Geert Uytterhoeven wrote:
>>>>>> On Mon, Feb 19, 2024 at 4:05 PM Josua Mayer <josua@solid-run.com> wrote:
>>>>>>> HummingBoard-T features two M.2 connectors labeled "M1" and "M2".
>>>>>>> The single SerDes lane of the SoC can be routed to either M1 pci-e
>>>>>>> signals, or M2 usb-3 signals by a gpio-controlled mux.
>>>>>>>
>>>>>>> Add overlays for each configuration.
>>>>>>>
>>>>>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>>>>>>> --- /dev/null
>>>>>>> +++ b/arch/arm64/boot/dts/ti/k3-am642-hummingboard-t-usb3.dtso
>>>>>>> +&usbss0 {
>>>>>>> +       /delete-property/ ti,usb2-only;
>>>>>> /delete-property/ (and /delete-node/) to delete something in the base DTS
>>>>>> does not work.
>>>> My understanding is that flags are equivalent to boolean, i.e:
>>>>
>>>> ti,usb2-only = <true>;
>>>> ti,usb2-only;
>>>>
>>>> are equivalent.
>>>>
>>>> If so, can we assign <false> within the overlay?
>> Is there any chance of reassigning <false> and making an argument
>> that this should be fixed?
> In theory, it can be done, if (1) all code that checks boolean
> properties would use of_property_read_bool() instead of
> of_property_present(), and (2) of_property_read_bool() would be changed
> to actually read the boolean value instead of just checking for the
> presence of the property.  And of course we have to do that in all
> software that uses DT (i.e. not just Linux).
> See [1][2][3] below for caveats.
Thank you for all those pointers!
>
> Using a similar solution for /delete-node/ is more complex, but still
> feasible, by setting its "status" property to "disabled" . I think
> that can be made to work if all DT core code that looks up nodes would
> just ignore any node that has a disabled status.
Agreed!
> I.e. callers would
> no longer see disabled nodes at all.
>
>> I find it frustrating that overlays can't override boolean properties,
>> and for consistency reasons I would otherwise change both
>> pcie and usb3 overlays to standalone dts.
> OK.

Setting a property = "false" would be quite preferential in my opinion,
over dropping the overlays.

Unfortunately I am preoccupied, and can't submit tested patches
within current merge window.

I will draft a minimal untested patch, and submit it to the list,
to get some comments.
For next window I could prepare a tested version.

>
> [1] The example in Documentation/devicetree/bindings/sound/rt5651.txt has:
>
>          realtek,dmic-en = "true";
>          realtek,in2-diff = "false";
>
>      Obviously the second line doesn't really work with the current
>      code, but fortunately there are no actual users of that (in
>      upstream DTS).
>      Note: "realtek,in2-diff" is a typo for "realtek,in2-differential".
>
> [2] The example in Documentation/devicetree/bindings/sound/pcm3060.txt has:
>
>          ti,out-single-ended = "true";
>
>      Again, this is an open invitation for replacing "true" by "false".
>      Fortunately there are no such users (in upstream DTS).
>
> [3] arch/arm/boot/dts/ti/omap/am335x-baltos.dtsi has:
>
>          gpmc,device-nand = "true";
>
>      But "gpmc,device-nand" does not exist. Oh, it is under removal:
>      https://lore.kernel.org/all/20241009-gpmc-omap-dtbx-v2-2-fc68124a090a@kernel.org/
>
>
>
> Gr{oetje,eeting}s,
>
>                          Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                  -- Linus Torvalds