Re: [PATCH] kbuild: Add DTC_CPP_FLAGS

Re: [PATCH] kbuild: Add DTC_CPP_FLAGS

[Masahiro Yamada]

(+CC: DT maintainers)

On Wed, Dec 18, 2019 at 8:55 PM Frank Wunderlich
<frank-w@public-files.de> wrote:
>
> With this Patch we are able to change cpp-preprocessor options
> (e.g. -D...) for using conditions (#ifdef) in devicetree files
> and dynamically add include searchpaths

I think this is questionable
since DT is supposed to describe hardware.
Does it depends on #ifdef or some external defines?

If you want to add variations,
you can add a new DTS which includes other DTS(I) files.



>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>  scripts/Makefile.lib | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 3fa32f83b2d7..c722a6c7786a 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -177,7 +177,7 @@ DTC_INCLUDE    := $(srctree)/scripts/dtc/include-prefixes
>
>  dtc_cpp_flags  = -Wp,-MD,$(depfile).pre.tmp -nostdinc                    \
>                  $(addprefix -I,$(DTC_INCLUDE))                          \
> -                -undef -D__DTS__
> +                -undef -D__DTS__ $(DTC_CPP_FLAGS)
>
>  # Useful for describing the dependency of composite objects
>  # Usage:
> --
> 2.17.1
>


--
Best Regards


Masahiro Yamada

Re: [PATCH] kbuild: Add DTC_CPP_FLAGS

[Rob Herring]

On Wed, Dec 18, 2019 at 7:37 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> (+CC: DT maintainers)
>
> On Wed, Dec 18, 2019 at 8:55 PM Frank Wunderlich
> <frank-w@public-files.de> wrote:
> >
> > With this Patch we are able to change cpp-preprocessor options
> > (e.g. -D...) for using conditions (#ifdef) in devicetree files
> > and dynamically add include searchpaths
>
> I think this is questionable
> since DT is supposed to describe hardware.
> Does it depends on #ifdef or some external defines?

NAK. #ifdefs and complex macros in particular are features we don't
want in dts files.

Rob

Aw: Re: [PATCH] kbuild: Add DTC_CPP_FLAGS

[Frank Wunderlich]

Hi Rob,

so the way you prefer is this one (use new file for additions and including the board dts):

arch/arm64/boot/dts/mediatek/mt7622-bpi-r64-mt7531.dts (example for mt7531-phy)

/*
 * Copyright (c) 2018 MediaTek Inc.
 * Author:
 *
 * SPDX-License-Identifier: (GPL-2.0 OR MIT)
 */

/dts-v1/;

#include "mt7622-bananapi-bpi-r64.dts"

/ {
        gsw: gsw@0 {
                compatible = "mediatek,mt753x";
                mediatek,ethsys = <&ethsys>;
                #address-cells = <1>;
                #size-cells = <0>;
        };
};

&gsw {
        mediatek,mdio = <&mdio>;
        mediatek,portmap = "wllll";
        mediatek,mdio_master_pinmux = <0>;
        reset-gpios = <&pio 54 0>;
        interrupt-parent = <&pio>;
        interrupts = <53 IRQ_TYPE_LEVEL_HIGH>;
        status = "okay";

        port5: port@5 {
                compatible = "mediatek,mt753x-port";
                reg = <5>;
                phy-mode = "rgmii";
                fixed-link {
                        speed = <1000>;
                        full-duplex;
                };
        };

        port6: port@6 {
                compatible = "mediatek,mt753x-port";
                reg = <6>;
                phy-mode = "2500base-x";
                fixed-link {
                        speed = <2500>;
                        full-duplex;
                };
        };
};

at least it compiles and after decompile the new nodes are visible...

is there any way to drop nodes (in case dsa-driver gets merged i don't need it in the other 2)? i can disable them, but they will be present.

regards Frank


> Gesendet: Mittwoch, 18. Dezember 2019 um 14:51 Uhr
> Von: "Rob Herring" <robh+dt@kernel.org>
> NAK. #ifdefs and complex macros in particular are features we don't
> want in dts files.
>
> Rob

Re: Re: [PATCH] kbuild: Add DTC_CPP_FLAGS

[Rob Herring]

On Thu, Dec 19, 2019 at 6:05 AM Frank Wunderlich
<frank-w@public-files.de> wrote:
>
> Hi Rob,
>
> so the way you prefer is this one (use new file for additions and including the board dts):

Yes, or you could use an overlay, but that will raise other issues
(current lack of support building them in the kernel).

> arch/arm64/boot/dts/mediatek/mt7622-bpi-r64-mt7531.dts (example for mt7531-phy)
>
> /*
>  * Copyright (c) 2018 MediaTek Inc.
>  * Author:
>  *
>  * SPDX-License-Identifier: (GPL-2.0 OR MIT)
>  */
>
> /dts-v1/;
>
> #include "mt7622-bananapi-bpi-r64.dts"
>
> / {
>         gsw: gsw@0 {
>                 compatible = "mediatek,mt753x";
>                 mediatek,ethsys = <&ethsys>;
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>         };
> };
>
> &gsw {
>         mediatek,mdio = <&mdio>;
>         mediatek,portmap = "wllll";
>         mediatek,mdio_master_pinmux = <0>;
>         reset-gpios = <&pio 54 0>;
>         interrupt-parent = <&pio>;
>         interrupts = <53 IRQ_TYPE_LEVEL_HIGH>;
>         status = "okay";
>
>         port5: port@5 {
>                 compatible = "mediatek,mt753x-port";
>                 reg = <5>;
>                 phy-mode = "rgmii";
>                 fixed-link {
>                         speed = <1000>;
>                         full-duplex;
>                 };
>         };
>
>         port6: port@6 {
>                 compatible = "mediatek,mt753x-port";
>                 reg = <6>;
>                 phy-mode = "2500base-x";
>                 fixed-link {
>                         speed = <2500>;
>                         full-duplex;
>                 };
>         };
> };
>
> at least it compiles and after decompile the new nodes are visible...
>
> is there any way to drop nodes (in case dsa-driver gets merged i don't need it in the other 2)? i can disable them, but they will be present.

Yes, use '/delete-node/'.

Rob

Aw: Re: [PATCH] kbuild: Add DTC_CPP_FLAGS

[Frank Wunderlich]

Hi,

thank you for your answer... i waited for this argument ;) so here an example (don't wanted to add in patch itself):

i have 2 different Bananapi-r64 boards with different switch-chips (v0.x rtl8367,v1.x xmt7531). for 1 of the switches (mt7531) there are 2 drivers (phy-driver + dsa driver) available and so i have 3 different configs in DTS.

Imho dtsi should contain only config for 1 chip, but the board has only 1 dts. Till now i have copied the whole dts twice, so i have 3 independent dts. if i Patch any other in one like for bluetooth, wifi, which is same on each board i have to do it on all 3 dts files.

so i tried to make the switch-part conditionally, include the right dtsi for switch to 1 bpi-r64 dts based on -D param passed to the build-chain.

#ifdef __SWITCH_RTL8367__
       #include "rtl8367.dtsi"
#endif
#ifdef __SWITCH_MT7531_DSA__
       #include "mt7531.dtsi"
#endif

imho i can/should not include a dtsi to a dtsi and no dts to dts, so i cannot have a small dts (with the switch-config) including the whole r64 board. Am i right?

and this change should be less interferring than other ideas i'm thinking about.

Another example: I had tried adding the include/generated/autoconf.h to dtsi (same reason) which needs a change in scripts/dtc/include-prefixes/ (add symlink) or an odd include in the dtsi (#include "../../../../../include/generated/autoconf.h"). Both ways not nice...

regards Frank


> Gesendet: Mittwoch, 18. Dezember 2019 um 14:31 Uhr
> Von: "Masahiro Yamada" <masahiroy@kernel.org>
> I think this is questionable
> since DT is supposed to describe hardware.
> Does it depends on #ifdef or some external defines?
>
> If you want to add variations,
> you can add a new DTS which includes other DTS(I) files.